On 01/28/2015 08:21 AM, Alexey Kardashevskiy wrote:
> On 01/27/2015 05:13 PM, Alexey Kardashevskiy wrote:
>> Instead of tweaking a TCE table device by adding there a bypass flag,
>> let's add an alias to RAM and IOMMU memory region, and enable/disable
>> those according to the selected bypass mode.
>> This way IOMMU memory region can have size of the actual window rather
>> than ram_size which is essential for upcoming DDW support.
>>
>> This moves bypass logic to VIO layer and keeps @bypass flag in TCE table
>> for migration compatibility only. This replaces spapr_tce_set_bypass()
>> calls with explicit assignment to avoid confusion as the function could
>> do something more that just syncing the @bypass flag.
>>
>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>> ---
>> Changes:
>> v2:
>> * kept @bypass in sPAPRTCETable not to break migration
>> ---
>> hw/ppc/spapr_iommu.c | 13 +++----------
>> hw/ppc/spapr_vio.c | 39 +++++++++++++++++++++++++++++++++++++--
>> include/hw/ppc/spapr.h | 1 -
>> include/hw/ppc/spapr_vio.h | 2 ++
>> 4 files changed, 42 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>> index da47474..a7b027a 100644
>> --- a/hw/ppc/spapr_iommu.c
>> +++ b/hw/ppc/spapr_iommu.c
>> @@ -72,9 +72,7 @@ static IOMMUTLBEntry
>> spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr,
>> .perm = IOMMU_NONE,
>> };
>>
>> - if (tcet->bypass) {
>> - ret.perm = IOMMU_RW;
>> - } else if ((addr >> tcet->page_shift) < tcet->nb_table) {
>> + if ((addr >> tcet->page_shift) < tcet->nb_table) {
>> /* Check if we are in bound */
>> hwaddr page_mask = IOMMU_PAGE_MASK(tcet->page_shift);
>>
>> @@ -131,7 +129,8 @@ static int spapr_tce_table_realize(DeviceState *dev)
>> trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd);
>>
>> memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops,
>> - "iommu-spapr", ram_size);
>> + "iommu-spapr",
>> + (uint64_t)tcet->nb_table << tcet->page_shift);
>>
>> QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list);
>>
>> @@ -191,17 +190,11 @@ MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet)
>> return &tcet->iommu;
>> }
>>
>> -void spapr_tce_set_bypass(sPAPRTCETable *tcet, bool bypass)
>> -{
>> - tcet->bypass = bypass;
>> -}
>> -
>> static void spapr_tce_reset(DeviceState *dev)
>> {
>> sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
>> size_t table_size = tcet->nb_table * sizeof(uint64_t);
>>
>> - tcet->bypass = false;
>> memset(tcet->table, 0, table_size);
>> }
>>
>> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
>> index dc9e46a..b65eb11 100644
>> --- a/hw/ppc/spapr_vio.c
>> +++ b/hw/ppc/spapr_vio.c
>> @@ -322,6 +322,18 @@ static void spapr_vio_quiesce_one(VIOsPAPRDevice *dev)
>> free_crq(dev);
>> }
>>
>> +static void spapr_vio_set_bypass(VIOsPAPRDevice *dev, bool bypass)
>> +{
>> + if (!dev->tcet) {
>> + return;
>> + }
>> +
>> + memory_region_set_enabled(&dev->mrbypass, bypass);
>> + memory_region_set_enabled(spapr_tce_get_iommu(dev->tcet), !bypass);
>> +
>> + dev->tcet->bypass = bypass;
>> +}
>> +
>> static void rtas_set_tce_bypass(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>> uint32_t token,
>> uint32_t nargs, target_ulong args,
>> @@ -348,7 +360,7 @@ static void rtas_set_tce_bypass(PowerPCCPU *cpu,
>> sPAPREnvironment *spapr,
>> return;
>> }
>>
>> - spapr_tce_set_bypass(dev->tcet, !!enable);
>> + spapr_vio_set_bypass(dev, !!enable);
>>
>> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> }
>> @@ -407,6 +419,7 @@ static void spapr_vio_busdev_reset(DeviceState *qdev)
>>
>> dev->signal_state = 0;
>>
>> + spapr_vio_set_bypass(dev, false);
>> if (pc->reset) {
>> pc->reset(dev);
>> }
>> @@ -456,12 +469,22 @@ static int spapr_vio_busdev_init(DeviceState *qdev)
>>
>> if (pc->rtce_window_size) {
>> uint32_t liobn = SPAPR_VIO_BASE_LIOBN | dev->reg;
>> +
>> + memory_region_init(&dev->mrroot, OBJECT(dev), "iommu-spapr-root",
>> + ram_size);
>> + memory_region_init_alias(&dev->mrbypass, OBJECT(dev),
>> + "iommu-spapr-bypass", get_system_memory(),
>> + 0, ram_size);
>> + memory_region_add_subregion_overlap(&dev->mrroot, 0,
>> &dev->mrbypass, 1);
>> + address_space_init(&dev->as, &dev->mrroot, qdev->id);
>> +
>> dev->tcet = spapr_tce_new_table(qdev, liobn,
>> 0,
>> SPAPR_TCE_PAGE_SHIFT,
>> pc->rtce_window_size >>
>> SPAPR_TCE_PAGE_SHIFT, false);
>> - address_space_init(&dev->as, spapr_tce_get_iommu(dev->tcet),
>> qdev->id);
>> + memory_region_add_subregion_overlap(&dev->mrroot, 0,
>> + spapr_tce_get_iommu(dev->tcet),
>> 2);
>> }
>>
>> return pc->init(dev);
>> @@ -541,6 +564,17 @@ static void spapr_vio_bridge_class_init(ObjectClass
>> *klass, void *data)
>> k->init = spapr_vio_bridge_init;
>> }
>>
>> +static int spapr_vio_post_load(void *opaque, int version_id)
>> +{
>> + VIOsPAPRDevice *dev = VIO_SPAPR_DEVICE(opaque);
>> +
>> + if (dev->tcet) {
>> + spapr_vio_set_bypass(dev, dev->tcet->bypass);
>
>
> Oh. Just realized that if VIOsPAPRDevice migrated before sPAPRTCETable,
> @bypass will always be false. So please ignore this patch for now :(
>
>
>
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static const TypeInfo spapr_vio_bridge_info = {
>> .name = "spapr-vio-bridge",
>> .parent = TYPE_SYS_BUS_DEVICE,
>> @@ -552,6 +586,7 @@ const VMStateDescription vmstate_spapr_vio = {
>> .name = "spapr_vio",
>> .version_id = 1,
>> .minimum_version_id = 1,
>> + .post_load = spapr_vio_post_load,
>> .fields = (VMStateField[]) {
>> /* Sanity check */
>> VMSTATE_UINT32_EQUAL(reg, VIOsPAPRDevice),
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 716bff4..ebc7e51 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -475,7 +475,6 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner,
>> uint32_t liobn,
>> uint32_t nb_table,
>> bool vfio_accel);
>> MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet);
>> -void spapr_tce_set_bypass(sPAPRTCETable *tcet, bool bypass);
>> int spapr_dma_dt(void *fdt, int node_off, const char *propname,
>> uint32_t liobn, uint64_t window, uint32_t size);
>> int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname,
>> diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
>> index 46edc2a..6ad55d1 100644
>> --- a/include/hw/ppc/spapr_vio.h
>> +++ b/include/hw/ppc/spapr_vio.h
>> @@ -64,6 +64,8 @@ struct VIOsPAPRDevice {
>> target_ulong signal_state;
>> VIOsPAPR_CRQ crq;
>> AddressSpace as;
>> + MemoryRegion mrroot;
>> + MemoryRegion mrbypass;
>> sPAPRTCETable *tcet;
>> };
>>
>>
>
>
I believe doing something like this is way too disguising because of
tobj->parent?
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -25,6 +25,7 @@
#include "trace.h"
#include "hw/ppc/spapr.h"
+#include "hw/ppc/spapr_vio.h"
#include <libfdt.h>
@@ -88,10 +89,26 @@ static IOMMUTLBEntry
spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr,
return ret;
}
+static int spapr_tce_table_post_load(void *opaque, int version_id)
+{
+ sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque);
+ Object *tobj = OBJECT(tcet);
+ Object *vobj = object_dynamic_cast(tobj->parent, TYPE_VIO_SPAPR_DEVICE);
+
+ if (!vobj) {
+ return 0;
+ }
+
+ spapr_vio_set_bypass((VIOsPAPRDevice *) vobj, tcet->bypass);
+
+ return 0;
+}
+
static const VMStateDescription vmstate_spapr_tce_table = {
.name = "spapr_iommu",
.version_id = 2,
.minimum_version_id = 2,
+ .post_load = spapr_tce_table_post_load,
.fields = (VMStateField []) {
/* Sanity check */
VMSTATE_UINT32_EQUAL(liobn, sPAPRTCETable),
--
Alexey
