| Message ID | 20170330124707.28142-3-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
On 30/03/2017 14:47, Alexey Kardashevskiy wrote: > +static int spapr_tce_get_fd(IOMMUMemoryRegion *iommu) > +{ > + sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu); > + > + return tcet->fd; > +} > + > static int spapr_tce_table_post_load(void *opaque, int version_id) > { > sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); > @@ -266,7 +273,7 @@ static int spapr_tce_table_realize(DeviceState *dev) > memory_region_init(&tcet->root, tcetobj, tmp, UINT64_MAX); > > snprintf(tmp, sizeof(tmp), "tce-iommu-%x", tcet->liobn); > - memory_region_init_iommu_type(TYPE_IOMMU_MEMORY_REGION, > + memory_region_init_iommu_type(TYPE_SPAPR_IOMMU_MEMORY_REGION, > &tcet->iommu, tcetobj, &spapr_iommu_ops, > tmp, 0); > > @@ -634,9 +641,25 @@ static TypeInfo spapr_tce_table_info = { > .class_init = spapr_tce_table_class_init, > }; > > +static void spapr_iommu_memory_region_class_init(ObjectClass *k, void *data) > +{ > + sPAPRIOMMUMemoryRegionClass *smrc = SPAPR_IOMMU_MEMORY_REGION_CLASS(k); > + > + smrc->get_fd = spapr_tce_get_fd; > +} > + You don't even need the virtual function. Rather, make spapr_tce_get_fd public and give it a sPAPRTCETable argument. Then vfio_spapr_notify_kvm can use SPAPR_IOMMU_MEMORY_REGION(iommumr). Paolo
On 31/03/17 00:00, Paolo Bonzini wrote: > > > On 30/03/2017 14:47, Alexey Kardashevskiy wrote: >> +static int spapr_tce_get_fd(IOMMUMemoryRegion *iommu) >> +{ >> + sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu); >> + >> + return tcet->fd; >> +} >> + >> static int spapr_tce_table_post_load(void *opaque, int version_id) >> { >> sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); >> @@ -266,7 +273,7 @@ static int spapr_tce_table_realize(DeviceState *dev) >> memory_region_init(&tcet->root, tcetobj, tmp, UINT64_MAX); >> >> snprintf(tmp, sizeof(tmp), "tce-iommu-%x", tcet->liobn); >> - memory_region_init_iommu_type(TYPE_IOMMU_MEMORY_REGION, >> + memory_region_init_iommu_type(TYPE_SPAPR_IOMMU_MEMORY_REGION, >> &tcet->iommu, tcetobj, &spapr_iommu_ops, >> tmp, 0); >> >> @@ -634,9 +641,25 @@ static TypeInfo spapr_tce_table_info = { >> .class_init = spapr_tce_table_class_init, >> }; >> >> +static void spapr_iommu_memory_region_class_init(ObjectClass *k, void *data) >> +{ >> + sPAPRIOMMUMemoryRegionClass *smrc = SPAPR_IOMMU_MEMORY_REGION_CLASS(k); >> + >> + smrc->get_fd = spapr_tce_get_fd; >> +} >> + > > You don't even need the virtual function. Rather, make spapr_tce_get_fd > public and give it a sPAPRTCETable argument. Then vfio_spapr_notify_kvm > can use SPAPR_IOMMU_MEMORY_REGION(iommumr). Well, if I make spapr_tce_get_fd() public, vfio_spapr_notify_kvm() could just take MemoryRegion and cast it to sPAPRTCETable without all these dances with MemoryRegion, IOMMUMemoryRegion, what do I miss here? I have made a big patch with IOMMUMemoryRegion though, post it?
On 31/03/2017 13:47, Alexey Kardashevskiy wrote: >>> >>> +static void spapr_iommu_memory_region_class_init(ObjectClass *k, void *data) >>> +{ >>> + sPAPRIOMMUMemoryRegionClass *smrc = SPAPR_IOMMU_MEMORY_REGION_CLASS(k); >>> + >>> + smrc->get_fd = spapr_tce_get_fd; >>> +} >>> + >> You don't even need the virtual function. Rather, make spapr_tce_get_fd >> public and give it a sPAPRTCETable argument. Then vfio_spapr_notify_kvm >> can use SPAPR_IOMMU_MEMORY_REGION(iommumr). > Well, if I make spapr_tce_get_fd() public, vfio_spapr_notify_kvm() could > just take MemoryRegion and cast it to sPAPRTCETable without all these > dances with MemoryRegion, IOMMUMemoryRegion, what do I miss here? I have > made a big patch with IOMMUMemoryRegion though, post it? *I* was missing that the call was inside an "if (container->iommu_type == TCE)". So the hierarchy is good to have, but the virtual function can be removed. Paolo
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 6997ed7e98..5d5ce4dd2b 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -573,11 +573,25 @@ void spapr_load_rtas(sPAPRMachineState *spapr, void *fdt, hwaddr addr); #define RTAS_EVENT_SCAN_RATE 1 typedef struct sPAPRTCETable sPAPRTCETable; +typedef struct sPAPRIOMMUMemoryRegionClass sPAPRIOMMUMemoryRegionClass; #define TYPE_SPAPR_TCE_TABLE "spapr-tce-table" #define SPAPR_TCE_TABLE(obj) \ OBJECT_CHECK(sPAPRTCETable, (obj), TYPE_SPAPR_TCE_TABLE) +#define TYPE_SPAPR_IOMMU_MEMORY_REGION "qemu:spapr_iommu-memory-region" +#define SPAPR_IOMMU_MEMORY_REGION(obj) \ + OBJECT_CHECK(IOMMUMemoryRegion, (obj), \ + TYPE_SPAPR_IOMMU_MEMORY_REGION) + +#define SPAPR_IOMMU_MEMORY_REGION_CLASS(k) \ + OBJECT_CLASS_CHECK(sPAPRIOMMUMemoryRegionClass, (k), \ + TYPE_SPAPR_IOMMU_MEMORY_REGION) + +#define SPAPR_IOMMU_MEMORY_REGION_GET_CLASS(obj) \ + OBJECT_GET_CLASS(sPAPRIOMMUMemoryRegionClass, (obj), \ + TYPE_SPAPR_IOMMU_MEMORY_REGION) + struct sPAPRTCETable { DeviceState parent; uint32_t liobn; @@ -596,6 +610,14 @@ struct sPAPRTCETable { QLIST_ENTRY(sPAPRTCETable) list; }; +struct sPAPRIOMMUMemoryRegionClass { + /*< private >*/ + ObjectClass parent_class; + /*< public >*/ + + int (*get_fd)(IOMMUMemoryRegion *iommu); +}; + sPAPRTCETable *spapr_tce_find_by_liobn(target_ulong liobn); struct sPAPREventLogEntry { diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c index 5051110b9d..5b0eee1be4 100644 --- a/hw/ppc/spapr_iommu.c +++ b/hw/ppc/spapr_iommu.c @@ -171,6 +171,13 @@ static void spapr_tce_notify_flag_changed(IOMMUMemoryRegion *iommu, } } +static int spapr_tce_get_fd(IOMMUMemoryRegion *iommu) +{ + sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu); + + return tcet->fd; +} + static int spapr_tce_table_post_load(void *opaque, int version_id) { sPAPRTCETable *tcet = SPAPR_TCE_TABLE(opaque); @@ -266,7 +273,7 @@ static int spapr_tce_table_realize(DeviceState *dev) memory_region_init(&tcet->root, tcetobj, tmp, UINT64_MAX); snprintf(tmp, sizeof(tmp), "tce-iommu-%x", tcet->liobn); - memory_region_init_iommu_type(TYPE_IOMMU_MEMORY_REGION, + memory_region_init_iommu_type(TYPE_SPAPR_IOMMU_MEMORY_REGION, &tcet->iommu, tcetobj, &spapr_iommu_ops, tmp, 0); @@ -634,9 +641,25 @@ static TypeInfo spapr_tce_table_info = { .class_init = spapr_tce_table_class_init, }; +static void spapr_iommu_memory_region_class_init(ObjectClass *k, void *data) +{ + sPAPRIOMMUMemoryRegionClass *smrc = SPAPR_IOMMU_MEMORY_REGION_CLASS(k); + + smrc->get_fd = spapr_tce_get_fd; +} + +static const TypeInfo spapr_iommu_memory_region_info = { + .parent = TYPE_IOMMU_MEMORY_REGION, + .name = TYPE_SPAPR_IOMMU_MEMORY_REGION, + .instance_size = sizeof(IOMMUMemoryRegion), + .class_size = sizeof(sPAPRIOMMUMemoryRegionClass), + .class_init = spapr_iommu_memory_region_class_init, +}; + static void register_types(void) { type_register_static(&spapr_tce_table_info); + type_register_static(&spapr_iommu_memory_region_info); } type_init(register_types);
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- include/hw/ppc/spapr.h | 22 ++++++++++++++++++++++ hw/ppc/spapr_iommu.c | 25 ++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-)