| Message ID | 1585542301-84087-7-git-send-email-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | intel_iommu: expose Shared Virtual Addressing to VMs | expand |
Yi, On 3/30/20 6:24 AM, Liu Yi L wrote: > This patch adds pci_device_set/unset_iommu_context() to set/unset > host_iommu_context for a given device. New callback is added in > PCIIOMMUOps. As such, vIOMMU could make use of host IOMMU capability. > e.g setup nested translation. I think you need to explain what this practically is supposed to do. such as: by attaching such context to a PCI device (for example VFIO assigned?), you tell the host that this PCIe device is protected by a FL stage controlled by the guest or something like that - if this is correct understanding (?) - > > Cc: Kevin Tian <kevin.tian@intel.com> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com> > Cc: Peter Xu <peterx@redhat.com> > Cc: Eric Auger <eric.auger@redhat.com> > Cc: Yi Sun <yi.y.sun@linux.intel.com> > Cc: David Gibson <david@gibson.dropbear.id.au> > Cc: Michael S. Tsirkin <mst@redhat.com> > Reviewed-by: Peter Xu <peterx@redhat.com> > Signed-off-by: Liu Yi L <yi.l.liu@intel.com> > --- > hw/pci/pci.c | 49 ++++++++++++++++++++++++++++++++++++++++++++----- > include/hw/pci/pci.h | 10 ++++++++++ > 2 files changed, 54 insertions(+), 5 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index aa9025c..af3c1a1 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -2638,7 +2638,8 @@ static void pci_device_class_base_init(ObjectClass *klass, void *data) > } > } > > -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, > + PCIBus **pbus, uint8_t *pdevfn) > { > PCIBus *bus = pci_get_bus(dev); > PCIBus *iommu_bus = bus; > @@ -2683,14 +2684,52 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > > iommu_bus = parent_bus; > } > - if (iommu_bus && iommu_bus->iommu_ops && > - iommu_bus->iommu_ops->get_address_space) { > - return iommu_bus->iommu_ops->get_address_space(bus, > - iommu_bus->iommu_opaque, devfn); > + *pbus = iommu_bus; > + *pdevfn = devfn; > +} > + > +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > +{ > + PCIBus *bus; > + uint8_t devfn; > + > + pci_device_get_iommu_bus_devfn(dev, &bus, &devfn); > + if (bus && bus->iommu_ops && > + bus->iommu_ops->get_address_space) { > + return bus->iommu_ops->get_address_space(bus, > + bus->iommu_opaque, devfn); > } > return &address_space_memory; > } > > +int pci_device_set_iommu_context(PCIDevice *dev, > + HostIOMMUContext *iommu_ctx) > +{ > + PCIBus *bus; > + uint8_t devfn; > + > + pci_device_get_iommu_bus_devfn(dev, &bus, &devfn); > + if (bus && bus->iommu_ops && > + bus->iommu_ops->set_iommu_context) { > + return bus->iommu_ops->set_iommu_context(bus, > + bus->iommu_opaque, devfn, iommu_ctx); > + } > + return -ENOENT; > +} > + > +void pci_device_unset_iommu_context(PCIDevice *dev) > +{ > + PCIBus *bus; > + uint8_t devfn; > + > + pci_device_get_iommu_bus_devfn(dev, &bus, &devfn); > + if (bus && bus->iommu_ops && > + bus->iommu_ops->unset_iommu_context) { > + bus->iommu_ops->unset_iommu_context(bus, > + bus->iommu_opaque, devfn); > + } > +} > + > void pci_setup_iommu(PCIBus *bus, const PCIIOMMUOps *ops, void *opaque) > { > bus->iommu_ops = ops; > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index ffe192d..0ec5680 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -9,6 +9,8 @@ > > #include "hw/pci/pcie.h" > > +#include "hw/iommu/host_iommu_context.h" > + > extern bool pci_available; > > /* PCI bus */ > @@ -489,9 +491,17 @@ typedef struct PCIIOMMUOps PCIIOMMUOps; > struct PCIIOMMUOps { > AddressSpace * (*get_address_space)(PCIBus *bus, > void *opaque, int32_t devfn); > + int (*set_iommu_context)(PCIBus *bus, void *opaque, > + int32_t devfn, > + HostIOMMUContext *iommu_ctx); > + void (*unset_iommu_context)(PCIBus *bus, void *opaque, > + int32_t devfn); > }; > > AddressSpace *pci_device_iommu_address_space(PCIDevice *dev); > +int pci_device_set_iommu_context(PCIDevice *dev, > + HostIOMMUContext *iommu_ctx); > +void pci_device_unset_iommu_context(PCIDevice *dev); > void pci_setup_iommu(PCIBus *bus, const PCIIOMMUOps *iommu_ops, void *opaque); > > static inline void > Thanks Eric
Hi Eric, > From: Auger Eric < eric.auger@redhat.com> > Sent: Tuesday, March 31, 2020 1:30 AM > To: Liu, Yi L <yi.l.liu@intel.com>; qemu-devel@nongnu.org; > Subject: Re: [PATCH v2 06/22] hw/pci: introduce > pci_device_set/unset_iommu_context() > > Yi, > On 3/30/20 6:24 AM, Liu Yi L wrote: > > This patch adds pci_device_set/unset_iommu_context() to set/unset > > host_iommu_context for a given device. New callback is added in > > PCIIOMMUOps. As such, vIOMMU could make use of host IOMMU capability. > > e.g setup nested translation. > > I think you need to explain what this practically is supposed to do. > such as: by attaching such context to a PCI device (for example VFIO > assigned?), you tell the host that this PCIe device is protected by a FL > stage controlled by the guest or something like that - if this is > correct understanding (?) - I'd like to say by attaching such context to a PCI device (for example VFIO assigned), this PCIe device is protected by a host IOMMU w/ nested-translation capability. Its DMA would be protected either through the FL stage controlled by the guest together with a SL stage page table owned by host or a single stage page table owned by host (e.g. shadow solution). It depends on the choice of vIOMMU the pci_device_set/unset_iommu_context() finally pass the context to vIOMMU. If vIOMMU binds guest FL stage page table to host, then it is prior case. If vIOMMU doesn't, do bind, then it is the latter case. Regards, Yi Liu
diff --git a/hw/pci/pci.c b/hw/pci/pci.c index aa9025c..af3c1a1 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2638,7 +2638,8 @@ static void pci_device_class_base_init(ObjectClass *klass, void *data) } } -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, + PCIBus **pbus, uint8_t *pdevfn) { PCIBus *bus = pci_get_bus(dev); PCIBus *iommu_bus = bus; @@ -2683,14 +2684,52 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) iommu_bus = parent_bus; } - if (iommu_bus && iommu_bus->iommu_ops && - iommu_bus->iommu_ops->get_address_space) { - return iommu_bus->iommu_ops->get_address_space(bus, - iommu_bus->iommu_opaque, devfn); + *pbus = iommu_bus; + *pdevfn = devfn; +} + +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) +{ + PCIBus *bus; + uint8_t devfn; + + pci_device_get_iommu_bus_devfn(dev, &bus, &devfn); + if (bus && bus->iommu_ops && + bus->iommu_ops->get_address_space) { + return bus->iommu_ops->get_address_space(bus, + bus->iommu_opaque, devfn); } return &address_space_memory; } +int pci_device_set_iommu_context(PCIDevice *dev, + HostIOMMUContext *iommu_ctx) +{ + PCIBus *bus; + uint8_t devfn; + + pci_device_get_iommu_bus_devfn(dev, &bus, &devfn); + if (bus && bus->iommu_ops && + bus->iommu_ops->set_iommu_context) { + return bus->iommu_ops->set_iommu_context(bus, + bus->iommu_opaque, devfn, iommu_ctx); + } + return -ENOENT; +} + +void pci_device_unset_iommu_context(PCIDevice *dev) +{ + PCIBus *bus; + uint8_t devfn; + + pci_device_get_iommu_bus_devfn(dev, &bus, &devfn); + if (bus && bus->iommu_ops && + bus->iommu_ops->unset_iommu_context) { + bus->iommu_ops->unset_iommu_context(bus, + bus->iommu_opaque, devfn); + } +} + void pci_setup_iommu(PCIBus *bus, const PCIIOMMUOps *ops, void *opaque) { bus->iommu_ops = ops; diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index ffe192d..0ec5680 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -9,6 +9,8 @@ #include "hw/pci/pcie.h" +#include "hw/iommu/host_iommu_context.h" + extern bool pci_available; /* PCI bus */ @@ -489,9 +491,17 @@ typedef struct PCIIOMMUOps PCIIOMMUOps; struct PCIIOMMUOps { AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int32_t devfn); + int (*set_iommu_context)(PCIBus *bus, void *opaque, + int32_t devfn, + HostIOMMUContext *iommu_ctx); + void (*unset_iommu_context)(PCIBus *bus, void *opaque, + int32_t devfn); }; AddressSpace *pci_device_iommu_address_space(PCIDevice *dev); +int pci_device_set_iommu_context(PCIDevice *dev, + HostIOMMUContext *iommu_ctx); +void pci_device_unset_iommu_context(PCIDevice *dev); void pci_setup_iommu(PCIBus *bus, const PCIIOMMUOps *iommu_ops, void *opaque); static inline void