Re: [Qemu-devel] [PATCH V2 07/11] virtio-pci: address space translation service (ATS) support

[Michael S. Tsirkin]

On Thu, Nov 03, 2016 at 05:27:19PM +0800, Jason Wang wrote:
> This patches enable the Address Translation Service support for virtio
> pci devices. This is needed for a guest visible Device IOTLB
> implementation and will be required by vhost device IOTLB API
> implementation for intel IOMMU.
> 
> Cc: Michael S. Tsirkin <address@hidden>
> Signed-off-by: Jason Wang <address@hidden>

I'd like to understand why do you think this is strictly required.
Won't setting CM bit in the IOMMU do the trick.

Also, could you remind me pls - can guests just disable ATS?

What happens then?


> ---
>  hw/pci/pcie.c                             | 16 ++++++++++++++++
>  hw/virtio/virtio-pci.c                    |  7 +++++++
>  hw/virtio/virtio-pci.h                    |  4 ++++
>  include/hw/pci/pcie.h                     |  4 ++++
>  include/standard-headers/linux/pci_regs.h |  1 +
>  5 files changed, 32 insertions(+)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 99cfb45..02195d9 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -717,3 +717,19 @@ void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t 
> offset, uint64_t ser_num)
>                          PCI_EXT_CAP_DSN_SIZEOF);
>      pci_set_quad(dev->config + offset + pci_dsn_cap, ser_num);
>  }
> +
> +void pcie_ats_init(PCIDevice *dev, uint16_t offset)
> +{
> +    pcie_add_capability(dev, PCI_EXT_CAP_ID_ATS, 0x1,
> +                        offset, PCI_EXT_CAP_ATS_SIZEOF);
> +
> +    dev->exp.ats_cap = offset;
> +
> +    /* Invalidate Queue Depth 0, Page Aligned Request 0 */
> +    pci_set_word(dev->config + offset + PCI_ATS_CAP, 0);
> +    /* STU 0, Disabled by default */
> +    pci_set_word(dev->config + offset + PCI_ATS_CTRL, 0);
> +
> +    pci_set_word(dev->wmask + dev->exp.ats_cap + PCI_ATS_CTRL, 0x800f);
> +}
> +
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 6ceb43e..e357bdf 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1838,6 +1838,11 @@ static void virtio_pci_realize(PCIDevice *pci_dev, 
> Error **errp)
>           * PCI Power Management Interface Specification.
>           */
>          pci_set_word(pci_dev->config + pos + PCI_PM_PMC, 0x3);
> +
> +        if (proxy->flags & VIRTIO_PCI_FLAG_ATS) {
> +            pcie_ats_init(pci_dev, 256);
> +        }
> +
>      } else {
>          /*
>           * make future invocations of pci_is_express() return false
> @@ -1889,6 +1894,8 @@ static Property virtio_pci_properties[] = {
>                      VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false),
>      DEFINE_PROP_BIT("page-per-vq", VirtIOPCIProxy, flags,
>                      VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT, false),
> +    DEFINE_PROP_BIT("ats", VirtIOPCIProxy, flags,
> +                    VIRTIO_PCI_FLAG_ATS_BIT, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> index b4edea6..057d49d 100644
> --- a/hw/virtio/virtio-pci.h
> +++ b/hw/virtio/virtio-pci.h
> @@ -69,6 +69,7 @@ enum {
>      VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT,
>      VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT,
>      VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT,
> +    VIRTIO_PCI_FLAG_ATS_BIT,
>  };
>  
>  /* Need to activate work-arounds for buggy guests at vmstate load. */
> @@ -93,6 +94,9 @@ enum {
>  #define VIRTIO_PCI_FLAG_PAGE_PER_VQ \
>      (1 << VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT)
>  
> +/* address space translation service */
> +#define VIRTIO_PCI_FLAG_ATS (1 << VIRTIO_PCI_FLAG_ATS_BIT)
> +
>  typedef struct {
>      MSIMessage msg;
>      int virq;
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index 056d25e..b08451d 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -74,6 +74,9 @@ struct PCIExpressDevice {
>      /* AER */
>      uint16_t aer_cap;
>      PCIEAERLog aer_log;
> +
> +    /* Offset of ATS capability in config space */
> +    uint16_t ats_cap;
>  };
>  
>  #define COMPAT_PROP_PCP "power_controller_present"
> @@ -120,6 +123,7 @@ void pcie_add_capability(PCIDevice *dev,
>  
>  void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
>  void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t 
> ser_num);
> +void pcie_ats_init(PCIDevice *dev, uint16_t offset);
>  
>  extern const VMStateDescription vmstate_pcie_device;
>  
> diff --git a/include/standard-headers/linux/pci_regs.h 
> b/include/standard-headers/linux/pci_regs.h
> index 4040951..ac426a0 100644
> --- a/include/standard-headers/linux/pci_regs.h
> +++ b/include/standard-headers/linux/pci_regs.h
> @@ -674,6 +674,7 @@
>  #define PCI_EXT_CAP_ID_MAX   PCI_EXT_CAP_ID_DPC
>  
>  #define PCI_EXT_CAP_DSN_SIZEOF       12
> +#define PCI_EXT_CAP_ATS_SIZEOF       8
>  #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
>  
>  /* Advanced Error Reporting */
> -- 
> 2.7.4