[v2,06/22] hw/pci: introduce pci_device_set/unset_iommu_context()

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.

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(-)

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