Re: [PATCH v2 07/22] intel_iommu: add set/unset_iommu_context callback
Hi Yi, On 3/31/20 2:25 PM, Liu, Yi L wrote: > Hi Eric, > >> From: Auger Eric < eric.au...@redhat.com> >> Sent: Tuesday, March 31, 2020 4:24 AM >> To: Liu, Yi L ; qemu-devel@nongnu.org; >> Subject: Re: [PATCH v2 07/22] intel_iommu: add set/unset_iommu_context >> callback >> >> Yi, >> >> On 3/30/20 6:24 AM, Liu Yi L wrote: >>> This patch adds set/unset_iommu_context() impelementation in Intel >> This patch implements the set/unset_iommu_context() ops for Intel vIOMMU. >>> vIOMMU. For Intel platform, pass-through modules (e.g. VFIO) could >>> set HostIOMMUContext to Intel vIOMMU emulator. >>> >>> Cc: Kevin Tian >>> Cc: Jacob Pan >>> Cc: Peter Xu >>> Cc: Yi Sun >>> Cc: Paolo Bonzini >>> Cc: Richard Henderson >>> Cc: Eduardo Habkost >>> Signed-off-by: Liu Yi L >>> --- >>> hw/i386/intel_iommu.c | 71 >> --- >>> include/hw/i386/intel_iommu.h | 21 ++--- >>> 2 files changed, 83 insertions(+), 9 deletions(-) >>> >>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >>> index 4b22910..fd349c6 100644 >>> --- a/hw/i386/intel_iommu.c >>> +++ b/hw/i386/intel_iommu.c >>> @@ -3354,23 +3354,33 @@ static const MemoryRegionOps vtd_mem_ir_ops = { >>> }, >>> }; >>> >>> -VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int >>> devfn) >>> +/** >>> + * Fetch a VTDBus instance for given PCIBus. If no existing instance, >>> + * allocate one. >>> + */ >>> +static VTDBus *vtd_find_add_bus(IntelIOMMUState *s, PCIBus *bus) >>> { >>> uintptr_t key = (uintptr_t)bus; >>> VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key); >>> -VTDAddressSpace *vtd_dev_as; >>> -char name[128]; >>> >>> if (!vtd_bus) { >>> uintptr_t *new_key = g_malloc(sizeof(*new_key)); >>> *new_key = (uintptr_t)bus; >>> /* No corresponding free() */ >>> -vtd_bus = g_malloc0(sizeof(VTDBus) + sizeof(VTDAddressSpace *) * \ >>> -PCI_DEVFN_MAX); >>> +vtd_bus = g_malloc0(sizeof(VTDBus)); >>> vtd_bus->bus = bus; >>> g_hash_table_insert(s->vtd_as_by_busptr, new_key, vtd_bus); >>> } >>> +return vtd_bus; >>> +} >>> >>> +VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int >>> devfn) >>> +{ >>> +VTDBus *vtd_bus; >>> +VTDAddressSpace *vtd_dev_as; >>> +char name[128]; >>> + >>> +vtd_bus = vtd_find_add_bus(s, bus); >>> vtd_dev_as = vtd_bus->dev_as[devfn]; >>> >>> if (!vtd_dev_as) { >>> @@ -3436,6 +3446,55 @@ VTDAddressSpace >> *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) >>> return vtd_dev_as; >>> } >>> >>> +static int vtd_dev_set_iommu_context(PCIBus *bus, void *opaque, >>> + int devfn, >>> + HostIOMMUContext *iommu_ctx) >>> +{ >>> +IntelIOMMUState *s = opaque; >>> +VTDBus *vtd_bus; >>> +VTDHostIOMMUContext *vtd_dev_icx; >>> + >>> +assert(0 <= devfn && devfn < PCI_DEVFN_MAX); >>> + >>> +vtd_bus = vtd_find_add_bus(s, bus); >>> + >>> +vtd_iommu_lock(s); >>> + >>> +vtd_dev_icx = vtd_bus->dev_icx[devfn]; >>> + >>> +assert(!vtd_dev_icx); >>> + >>> +vtd_bus->dev_icx[devfn] = vtd_dev_icx = >>> +g_malloc0(sizeof(VTDHostIOMMUContext)); >>> +vtd_dev_icx->vtd_bus = vtd_bus; >>> +vtd_dev_icx->devfn = (uint8_t)devfn; >>> +vtd_dev_icx->iommu_state = s; >>> +vtd_dev_icx->iommu_ctx = iommu_ctx; >>> + >>> +vtd_iommu_unlock(s); >>> + >>> +return 0; >>> +} >>> + >>> +static void vtd_dev_unset_iommu_context(PCIBus *bus, void *opaque, int >>> devfn) >>> +{ >>> +IntelIOMMUState *s = opaque; >>> +VTDBus *vtd_bus; >>> +VTDHostIOMMUContext *vtd_dev_icx; >>> + >>> +assert(0 <= devfn && devfn < PCI_DEVFN_MAX); >>> + >>> +vtd_bus = vtd_find_add_bus(s,
RE: [PATCH v2 07/22] intel_iommu: add set/unset_iommu_context callback
Hi Eric, > From: Auger Eric < eric.au...@redhat.com> > Sent: Tuesday, March 31, 2020 4:24 AM > To: Liu, Yi L ; qemu-devel@nongnu.org; > Subject: Re: [PATCH v2 07/22] intel_iommu: add set/unset_iommu_context > callback > > Yi, > > On 3/30/20 6:24 AM, Liu Yi L wrote: > > This patch adds set/unset_iommu_context() impelementation in Intel > This patch implements the set/unset_iommu_context() ops for Intel vIOMMU. > > vIOMMU. For Intel platform, pass-through modules (e.g. VFIO) could > > set HostIOMMUContext to Intel vIOMMU emulator. > > > > Cc: Kevin Tian > > Cc: Jacob Pan > > Cc: Peter Xu > > Cc: Yi Sun > > Cc: Paolo Bonzini > > Cc: Richard Henderson > > Cc: Eduardo Habkost > > Signed-off-by: Liu Yi L > > --- > > hw/i386/intel_iommu.c | 71 > --- > > include/hw/i386/intel_iommu.h | 21 ++--- > > 2 files changed, 83 insertions(+), 9 deletions(-) > > > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > > index 4b22910..fd349c6 100644 > > --- a/hw/i386/intel_iommu.c > > +++ b/hw/i386/intel_iommu.c > > @@ -3354,23 +3354,33 @@ static const MemoryRegionOps vtd_mem_ir_ops = { > > }, > > }; > > > > -VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int > > devfn) > > +/** > > + * Fetch a VTDBus instance for given PCIBus. If no existing instance, > > + * allocate one. > > + */ > > +static VTDBus *vtd_find_add_bus(IntelIOMMUState *s, PCIBus *bus) > > { > > uintptr_t key = (uintptr_t)bus; > > VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key); > > -VTDAddressSpace *vtd_dev_as; > > -char name[128]; > > > > if (!vtd_bus) { > > uintptr_t *new_key = g_malloc(sizeof(*new_key)); > > *new_key = (uintptr_t)bus; > > /* No corresponding free() */ > > -vtd_bus = g_malloc0(sizeof(VTDBus) + sizeof(VTDAddressSpace *) * \ > > -PCI_DEVFN_MAX); > > +vtd_bus = g_malloc0(sizeof(VTDBus)); > > vtd_bus->bus = bus; > > g_hash_table_insert(s->vtd_as_by_busptr, new_key, vtd_bus); > > } > > +return vtd_bus; > > +} > > > > +VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int > > devfn) > > +{ > > +VTDBus *vtd_bus; > > +VTDAddressSpace *vtd_dev_as; > > +char name[128]; > > + > > +vtd_bus = vtd_find_add_bus(s, bus); > > vtd_dev_as = vtd_bus->dev_as[devfn]; > > > > if (!vtd_dev_as) { > > @@ -3436,6 +3446,55 @@ VTDAddressSpace > *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) > > return vtd_dev_as; > > } > > > > +static int vtd_dev_set_iommu_context(PCIBus *bus, void *opaque, > > + int devfn, > > + HostIOMMUContext *iommu_ctx) > > +{ > > +IntelIOMMUState *s = opaque; > > +VTDBus *vtd_bus; > > +VTDHostIOMMUContext *vtd_dev_icx; > > + > > +assert(0 <= devfn && devfn < PCI_DEVFN_MAX); > > + > > +vtd_bus = vtd_find_add_bus(s, bus); > > + > > +vtd_iommu_lock(s); > > + > > +vtd_dev_icx = vtd_bus->dev_icx[devfn]; > > + > > +assert(!vtd_dev_icx); > > + > > +vtd_bus->dev_icx[devfn] = vtd_dev_icx = > > +g_malloc0(sizeof(VTDHostIOMMUContext)); > > +vtd_dev_icx->vtd_bus = vtd_bus; > > +vtd_dev_icx->devfn = (uint8_t)devfn; > > +vtd_dev_icx->iommu_state = s; > > +vtd_dev_icx->iommu_ctx = iommu_ctx; > > + > > +vtd_iommu_unlock(s); > > + > > +return 0; > > +} > > + > > +static void vtd_dev_unset_iommu_context(PCIBus *bus, void *opaque, int > > devfn) > > +{ > > +IntelIOMMUState *s = opaque; > > +VTDBus *vtd_bus; > > +VTDHostIOMMUContext *vtd_dev_icx; > > + > > +assert(0 <= devfn && devfn < PCI_DEVFN_MAX); > > + > > +vtd_bus = vtd_find_add_bus(s, bus); > > + > > +vtd_iommu_lock(s); > > + > > +vtd_dev_icx = vtd_bus->dev_icx[devfn]; > > +g_free(vtd_dev_icx); > > +vtd_bus->dev_icx[devfn] = NULL; > > + > > +vtd_iommu_unlock(s); > > +} > > + > > static uint64_t get_naturally_aligned_size(uint64_t start, > > uint64_t size, int gaw)
Re: [PATCH v2 07/22] intel_iommu: add set/unset_iommu_context callback
Yi, On 3/30/20 6:24 AM, Liu Yi L wrote: > This patch adds set/unset_iommu_context() impelementation in Intel This patch implements the set/unset_iommu_context() ops for Intel vIOMMU. > vIOMMU. For Intel platform, pass-through modules (e.g. VFIO) could > set HostIOMMUContext to Intel vIOMMU emulator. > > Cc: Kevin Tian > Cc: Jacob Pan > Cc: Peter Xu > Cc: Yi Sun > Cc: Paolo Bonzini > Cc: Richard Henderson > Cc: Eduardo Habkost > Signed-off-by: Liu Yi L > --- > hw/i386/intel_iommu.c | 71 > --- > include/hw/i386/intel_iommu.h | 21 ++--- > 2 files changed, 83 insertions(+), 9 deletions(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 4b22910..fd349c6 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -3354,23 +3354,33 @@ static const MemoryRegionOps vtd_mem_ir_ops = { > }, > }; > > -VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) > +/** > + * Fetch a VTDBus instance for given PCIBus. If no existing instance, > + * allocate one. > + */ > +static VTDBus *vtd_find_add_bus(IntelIOMMUState *s, PCIBus *bus) > { > uintptr_t key = (uintptr_t)bus; > VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key); > -VTDAddressSpace *vtd_dev_as; > -char name[128]; > > if (!vtd_bus) { > uintptr_t *new_key = g_malloc(sizeof(*new_key)); > *new_key = (uintptr_t)bus; > /* No corresponding free() */ > -vtd_bus = g_malloc0(sizeof(VTDBus) + sizeof(VTDAddressSpace *) * \ > -PCI_DEVFN_MAX); > +vtd_bus = g_malloc0(sizeof(VTDBus)); > vtd_bus->bus = bus; > g_hash_table_insert(s->vtd_as_by_busptr, new_key, vtd_bus); > } > +return vtd_bus; > +} > > +VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) > +{ > +VTDBus *vtd_bus; > +VTDAddressSpace *vtd_dev_as; > +char name[128]; > + > +vtd_bus = vtd_find_add_bus(s, bus); > vtd_dev_as = vtd_bus->dev_as[devfn]; > > if (!vtd_dev_as) { > @@ -3436,6 +3446,55 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, > PCIBus *bus, int devfn) > return vtd_dev_as; > } > > +static int vtd_dev_set_iommu_context(PCIBus *bus, void *opaque, > + int devfn, > + HostIOMMUContext *iommu_ctx) > +{ > +IntelIOMMUState *s = opaque; > +VTDBus *vtd_bus; > +VTDHostIOMMUContext *vtd_dev_icx; > + > +assert(0 <= devfn && devfn < PCI_DEVFN_MAX); > + > +vtd_bus = vtd_find_add_bus(s, bus); > + > +vtd_iommu_lock(s); > + > +vtd_dev_icx = vtd_bus->dev_icx[devfn]; > + > +assert(!vtd_dev_icx); > + > +vtd_bus->dev_icx[devfn] = vtd_dev_icx = > +g_malloc0(sizeof(VTDHostIOMMUContext)); > +vtd_dev_icx->vtd_bus = vtd_bus; > +vtd_dev_icx->devfn = (uint8_t)devfn; > +vtd_dev_icx->iommu_state = s; > +vtd_dev_icx->iommu_ctx = iommu_ctx; > + > +vtd_iommu_unlock(s); > + > +return 0; > +} > + > +static void vtd_dev_unset_iommu_context(PCIBus *bus, void *opaque, int devfn) > +{ > +IntelIOMMUState *s = opaque; > +VTDBus *vtd_bus; > +VTDHostIOMMUContext *vtd_dev_icx; > + > +assert(0 <= devfn && devfn < PCI_DEVFN_MAX); > + > +vtd_bus = vtd_find_add_bus(s, bus); > + > +vtd_iommu_lock(s); > + > +vtd_dev_icx = vtd_bus->dev_icx[devfn]; > +g_free(vtd_dev_icx); > +vtd_bus->dev_icx[devfn] = NULL; > + > +vtd_iommu_unlock(s); > +} > + > static uint64_t get_naturally_aligned_size(uint64_t start, > uint64_t size, int gaw) > { > @@ -3731,6 +3790,8 @@ static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, > void *opaque, int devfn) > > static PCIIOMMUOps vtd_iommu_ops = { > .get_address_space = vtd_host_dma_iommu, > +.set_iommu_context = vtd_dev_set_iommu_context, > +.unset_iommu_context = vtd_dev_unset_iommu_context, > }; > > static bool vtd_decide_config(IntelIOMMUState *s, Error **errp) > diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h > index 3870052..b5fefb9 100644 > --- a/include/hw/i386/intel_iommu.h > +++ b/include/hw/i386/intel_iommu.h > @@ -64,6 +64,7 @@ typedef union VTD_IR_TableEntry VTD_IR_TableEntry; > typedef union VTD_IR_MSIAddress VTD_IR_MSIAddress; > typedef struct VTDPASIDDirEntry VTDPASIDDirEntry; > typedef struct VTDPASIDEntry VTDPASIDEntry; > +typedef struct VTDHostIOMMUContext VTDHostIOMMUContext; > > /* Context-Entry */ > struct VTDContextEntry { > @@ -112,10 +113,20 @@ struct VTDAddressSpace { > IOVATree *iova_tree; /* Traces mapped IOVA ranges */ > }; > > +struct VTDHostIOMMUContext { > +VTDBus *vtd_bus; > +uint8_t devfn; > +HostIOMMUContext *iommu_ctx; I don't get why we don't have standard QOM inheritance instead of this handle? VTDHostContext parent_obj; like IOMMUMemoryReg
[PATCH v2 07/22] intel_iommu: add set/unset_iommu_context callback
This patch adds set/unset_iommu_context() impelementation in Intel vIOMMU. For Intel platform, pass-through modules (e.g. VFIO) could set HostIOMMUContext to Intel vIOMMU emulator. Cc: Kevin Tian Cc: Jacob Pan Cc: Peter Xu Cc: Yi Sun Cc: Paolo Bonzini Cc: Richard Henderson Cc: Eduardo Habkost Signed-off-by: Liu Yi L --- hw/i386/intel_iommu.c | 71 --- include/hw/i386/intel_iommu.h | 21 ++--- 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 4b22910..fd349c6 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -3354,23 +3354,33 @@ static const MemoryRegionOps vtd_mem_ir_ops = { }, }; -VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) +/** + * Fetch a VTDBus instance for given PCIBus. If no existing instance, + * allocate one. + */ +static VTDBus *vtd_find_add_bus(IntelIOMMUState *s, PCIBus *bus) { uintptr_t key = (uintptr_t)bus; VTDBus *vtd_bus = g_hash_table_lookup(s->vtd_as_by_busptr, &key); -VTDAddressSpace *vtd_dev_as; -char name[128]; if (!vtd_bus) { uintptr_t *new_key = g_malloc(sizeof(*new_key)); *new_key = (uintptr_t)bus; /* No corresponding free() */ -vtd_bus = g_malloc0(sizeof(VTDBus) + sizeof(VTDAddressSpace *) * \ -PCI_DEVFN_MAX); +vtd_bus = g_malloc0(sizeof(VTDBus)); vtd_bus->bus = bus; g_hash_table_insert(s->vtd_as_by_busptr, new_key, vtd_bus); } +return vtd_bus; +} +VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) +{ +VTDBus *vtd_bus; +VTDAddressSpace *vtd_dev_as; +char name[128]; + +vtd_bus = vtd_find_add_bus(s, bus); vtd_dev_as = vtd_bus->dev_as[devfn]; if (!vtd_dev_as) { @@ -3436,6 +3446,55 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn) return vtd_dev_as; } +static int vtd_dev_set_iommu_context(PCIBus *bus, void *opaque, + int devfn, + HostIOMMUContext *iommu_ctx) +{ +IntelIOMMUState *s = opaque; +VTDBus *vtd_bus; +VTDHostIOMMUContext *vtd_dev_icx; + +assert(0 <= devfn && devfn < PCI_DEVFN_MAX); + +vtd_bus = vtd_find_add_bus(s, bus); + +vtd_iommu_lock(s); + +vtd_dev_icx = vtd_bus->dev_icx[devfn]; + +assert(!vtd_dev_icx); + +vtd_bus->dev_icx[devfn] = vtd_dev_icx = +g_malloc0(sizeof(VTDHostIOMMUContext)); +vtd_dev_icx->vtd_bus = vtd_bus; +vtd_dev_icx->devfn = (uint8_t)devfn; +vtd_dev_icx->iommu_state = s; +vtd_dev_icx->iommu_ctx = iommu_ctx; + +vtd_iommu_unlock(s); + +return 0; +} + +static void vtd_dev_unset_iommu_context(PCIBus *bus, void *opaque, int devfn) +{ +IntelIOMMUState *s = opaque; +VTDBus *vtd_bus; +VTDHostIOMMUContext *vtd_dev_icx; + +assert(0 <= devfn && devfn < PCI_DEVFN_MAX); + +vtd_bus = vtd_find_add_bus(s, bus); + +vtd_iommu_lock(s); + +vtd_dev_icx = vtd_bus->dev_icx[devfn]; +g_free(vtd_dev_icx); +vtd_bus->dev_icx[devfn] = NULL; + +vtd_iommu_unlock(s); +} + static uint64_t get_naturally_aligned_size(uint64_t start, uint64_t size, int gaw) { @@ -3731,6 +3790,8 @@ static AddressSpace *vtd_host_dma_iommu(PCIBus *bus, void *opaque, int devfn) static PCIIOMMUOps vtd_iommu_ops = { .get_address_space = vtd_host_dma_iommu, +.set_iommu_context = vtd_dev_set_iommu_context, +.unset_iommu_context = vtd_dev_unset_iommu_context, }; static bool vtd_decide_config(IntelIOMMUState *s, Error **errp) diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h index 3870052..b5fefb9 100644 --- a/include/hw/i386/intel_iommu.h +++ b/include/hw/i386/intel_iommu.h @@ -64,6 +64,7 @@ typedef union VTD_IR_TableEntry VTD_IR_TableEntry; typedef union VTD_IR_MSIAddress VTD_IR_MSIAddress; typedef struct VTDPASIDDirEntry VTDPASIDDirEntry; typedef struct VTDPASIDEntry VTDPASIDEntry; +typedef struct VTDHostIOMMUContext VTDHostIOMMUContext; /* Context-Entry */ struct VTDContextEntry { @@ -112,10 +113,20 @@ struct VTDAddressSpace { IOVATree *iova_tree; /* Traces mapped IOVA ranges */ }; +struct VTDHostIOMMUContext { +VTDBus *vtd_bus; +uint8_t devfn; +HostIOMMUContext *iommu_ctx; +IntelIOMMUState *iommu_state; +}; + struct VTDBus { -PCIBus* bus; /* A reference to the bus to provide translation for */ +/* A reference to the bus to provide translation for */ +PCIBus *bus; /* A table of VTDAddressSpace objects indexed by devfn */ -VTDAddressSpace *dev_as[]; +VTDAddressSpace *dev_as[PCI_DEVFN_MAX]; +/* A table of VTDHostIOMMUContext objects indexed by devfn */ +VTDHostIOMMUContext *dev_icx[PCI_DEVFN_MAX]; }; struct VTDIOTLBEntry { @@ -269,8 +280,10 @@ struct