RE: [PATCH] xen: introduce xen_vring_use_dma

2020-06-28 Thread Peng Fan
> Subject: Re: [PATCH] xen: introduce xen_vring_use_dma
> 
> On Wed, 24 Jun 2020, Michael S. Tsirkin wrote:
> > On Wed, Jun 24, 2020 at 02:53:54PM -0700, Stefano Stabellini wrote:
> > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote:
> > > > On Wed, Jun 24, 2020 at 10:59:47AM -0700, Stefano Stabellini wrote:
> > > > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote:
> > > > > > On Wed, Jun 24, 2020 at 05:17:32PM +0800, Peng Fan wrote:
> > > > > > > Export xen_swiotlb for all platforms using xen swiotlb
> > > > > > >
> > > > > > > Use xen_swiotlb to determine when vring should use dma APIs
> > > > > > > to map the
> > > > > > > ring: when xen_swiotlb is enabled the dma API is required.
> > > > > > > When it is disabled, it is not required.
> > > > > > >
> > > > > > > Signed-off-by: Peng Fan 
> > > > > >
> > > > > > Isn't there some way to use VIRTIO_F_IOMMU_PLATFORM for this?
> > > > > > Xen was there first, but everyone else is using that now.
> > > > >
> > > > > Unfortunately it is complicated and it is not related to
> > > > > VIRTIO_F_IOMMU_PLATFORM :-(
> > > > >
> > > > >
> > > > > The Xen subsystem in Linux uses dma_ops via swiotlb_xen to
> > > > > translate foreign mappings (memory coming from other VMs) to
> physical addresses.
> > > > > On x86, it also uses dma_ops to translate Linux's idea of a
> > > > > physical address into a real physical address (this is unneeded
> > > > > on ARM.)
> > > > >
> > > > >
> > > > > So regardless of VIRTIO_F_IOMMU_PLATFORM, dma_ops should be
> used
> > > > > on Xen/x86 always and on Xen/ARM if Linux is Dom0 (because it
> > > > > has foreign
> > > > > mappings.) That is why we have the if (xen_domain) return true;
> > > > > in vring_use_dma_api.
> > > >
> > > > VIRTIO_F_IOMMU_PLATFORM makes guest always use DMA ops.
> > > >
> > > > Xen hack predates VIRTIO_F_IOMMU_PLATFORM so it *also* forces
> DMA
> > > > ops even if VIRTIO_F_IOMMU_PLATFORM is clear.
> > > >
> > > > Unfortunately as a result Xen never got around to properly setting
> > > > VIRTIO_F_IOMMU_PLATFORM.
> > >
> > > I don't think VIRTIO_F_IOMMU_PLATFORM would be correct for this
> > > because the usage of swiotlb_xen is not a property of virtio,
> >
> >
> > Basically any device without VIRTIO_F_ACCESS_PLATFORM (that is it's
> > name in latest virtio spec, VIRTIO_F_IOMMU_PLATFORM is what linux
> > calls it) is declared as "special, don't follow normal rules for
> > access".
> >
> > So yes swiotlb_xen is not a property of virtio, but what *is* a
> > property of virtio is that it's not special, just a regular device from DMA 
> > POV.
> 
> I am trying to understand what you meant but I think I am missing something.
> 
> Are you saying that modern virtio should always have
> VIRTIO_F_ACCESS_PLATFORM, hence use normal dma_ops as any other
> devices?
> 
> If that is the case, how is it possible that virtio breaks on ARM using the
> default dma_ops? The breakage is not Xen related (except that Xen turns
> dma_ops on). The original message from Peng was:
> 
>   vring_map_one_sg -> vring_use_dma_api
>-> dma_map_page
>  -> __swiotlb_map_page
>   ->swiotlb_map_page
>   ->__dma_map_area(phys_to_virt(dma_to_phys(dev,
> dev_addr)), size, dir);
>   However we are using per device dma area for rpmsg, phys_to_virt
>   could not return a correct virtual address for virtual address in
>   vmalloc area. Then kernel panic.
> 
> I must be missing something. Maybe it is because it has to do with RPMesg?

I am not going to fix the rpmsg issue with this patch. It is when ARM DomU
Android os communicate with secure world trusty os using virtio, the
vring_use_dma_api will return true for xen domu, but I no need it return
true and fall into swiotlb.

Thanks,
Peng.

> 
> 
> > > > > You might have noticed that I missed one possible case above:
> > > > > Xen/ARM DomU :-)
> > > > >
> > > > > Xen/ARM domUs don't need swiotlb_xen, it is not even
> > > > > initialized. So if
> > > > > (xen_domain) return true; would give the wrong answer in that case.
> > > > > Linux would end up calling the "normal" dma_ops, not
> > > > > swiotlb-xen, and the "normal" dma_ops fail.
> > > > >
> > > > >
> > > > > The solution I suggested was to make the check in
> > > > > vring_use_dma_api more flexible by returning true if the
> > > > > swiotlb_xen is supposed to be used, not in general for all Xen
> > > > > domains, because that is what the check was really meant to do.
> > > >
> > > > Why not fix DMA ops so they DTRT (nop) on Xen/ARM DomU? What is
> wrong with that?
> > >
> > > swiotlb-xen is not used on Xen/ARM DomU, the default dma_ops are the
> > > ones that are used. So you are saying, why don't we fix the default
> > > dma_ops to work with virtio?
> > >
> > > It is bad that the default dma_ops crash with virtio, so yes I think
> > > it would be good to fix that. However, even if we fixed that, the if
> > > (xen_domain()) check in vring_use_dma_api is still 

RE: [PATCH] xen: introduce xen_vring_use_dma

2020-06-28 Thread Peng Fan
> Subject: Re: [PATCH] xen: introduce xen_vring_use_dma
> 
> On Thu, Jun 25, 2020 at 10:31:27AM -0700, Stefano Stabellini wrote:
> > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote:
> > > On Wed, Jun 24, 2020 at 02:53:54PM -0700, Stefano Stabellini wrote:
> > > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote:
> > > > > On Wed, Jun 24, 2020 at 10:59:47AM -0700, Stefano Stabellini wrote:
> > > > > > On Wed, 24 Jun 2020, Michael S. Tsirkin wrote:
> > > > > > > On Wed, Jun 24, 2020 at 05:17:32PM +0800, Peng Fan wrote:
> > > > > > > > Export xen_swiotlb for all platforms using xen swiotlb
> > > > > > > >
> > > > > > > > Use xen_swiotlb to determine when vring should use dma
> > > > > > > > APIs to map the
> > > > > > > > ring: when xen_swiotlb is enabled the dma API is required.
> > > > > > > > When it is disabled, it is not required.
> > > > > > > >
> > > > > > > > Signed-off-by: Peng Fan 
> > > > > > >
> > > > > > > Isn't there some way to use VIRTIO_F_IOMMU_PLATFORM for
> this?
> > > > > > > Xen was there first, but everyone else is using that now.
> > > > > >
> > > > > > Unfortunately it is complicated and it is not related to
> > > > > > VIRTIO_F_IOMMU_PLATFORM :-(
> > > > > >
> > > > > >
> > > > > > The Xen subsystem in Linux uses dma_ops via swiotlb_xen to
> > > > > > translate foreign mappings (memory coming from other VMs) to
> physical addresses.
> > > > > > On x86, it also uses dma_ops to translate Linux's idea of a
> > > > > > physical address into a real physical address (this is
> > > > > > unneeded on ARM.)
> > > > > >
> > > > > >
> > > > > > So regardless of VIRTIO_F_IOMMU_PLATFORM, dma_ops should be
> > > > > > used on Xen/x86 always and on Xen/ARM if Linux is Dom0
> > > > > > (because it has foreign
> > > > > > mappings.) That is why we have the if (xen_domain) return
> > > > > > true; in vring_use_dma_api.
> > > > >
> > > > > VIRTIO_F_IOMMU_PLATFORM makes guest always use DMA ops.
> > > > >
> > > > > Xen hack predates VIRTIO_F_IOMMU_PLATFORM so it *also* forces
> > > > > DMA ops even if VIRTIO_F_IOMMU_PLATFORM is clear.
> > > > >
> > > > > Unfortunately as a result Xen never got around to properly
> > > > > setting VIRTIO_F_IOMMU_PLATFORM.
> > > >
> > > > I don't think VIRTIO_F_IOMMU_PLATFORM would be correct for this
> > > > because the usage of swiotlb_xen is not a property of virtio,
> > >
> > >
> > > Basically any device without VIRTIO_F_ACCESS_PLATFORM (that is it's
> > > name in latest virtio spec, VIRTIO_F_IOMMU_PLATFORM is what linux
> > > calls it) is declared as "special, don't follow normal rules for
> > > access".
> > >
> > > So yes swiotlb_xen is not a property of virtio, but what *is* a
> > > property of virtio is that it's not special, just a regular device from 
> > > DMA
> POV.
> >
> > I am trying to understand what you meant but I think I am missing
> > something.
> >
> > Are you saying that modern virtio should always have
> > VIRTIO_F_ACCESS_PLATFORM, hence use normal dma_ops as any other
> devices?
> 
> I am saying it's a safe default. Clear VIRTIO_F_ACCESS_PLATFORM if you have
> some special needs e.g. you are very sure it's ok to bypass DMA ops, or you
> need to support a legacy guest (produced in the window between virtio 1
> support in 2014 and support for VIRTIO_F_ACCESS_PLATFORM in 2016).
> 
> 
> > If that is the case, how is it possible that virtio breaks on ARM
> > using the default dma_ops? The breakage is not Xen related (except
> > that Xen turns dma_ops on). The original message from Peng was:
> >
> >   vring_map_one_sg -> vring_use_dma_api
> >-> dma_map_page
> >-> __swiotlb_map_page
> > ->swiotlb_map_page
> > ->__dma_map_area(phys_to_virt(dma_to_phys(dev,
> dev_addr)), size, dir);
> >   However we are using per device dma area for rpmsg, phys_to_virt
> >   could not return a correct virtual address for virtual address in
> >   vmalloc area. Then kernel panic.
> >
> > I must be missing something. Maybe it is because it has to do with RPMesg?
> 
> I think it's an RPMesg bug, yes

rpmsg bug is another issue, it should not use dma_alloc_coherent for reserved 
area,
and use vmalloc_to_page.

Anyway here using dma api will also trigger issue.

> 
> >
> > > > > > You might have noticed that I missed one possible case above:
> > > > > > Xen/ARM DomU :-)
> > > > > >
> > > > > > Xen/ARM domUs don't need swiotlb_xen, it is not even
> > > > > > initialized. So if
> > > > > > (xen_domain) return true; would give the wrong answer in that case.
> > > > > > Linux would end up calling the "normal" dma_ops, not
> > > > > > swiotlb-xen, and the "normal" dma_ops fail.
> > > > > >
> > > > > >
> > > > > > The solution I suggested was to make the check in
> > > > > > vring_use_dma_api more flexible by returning true if the
> > > > > > swiotlb_xen is supposed to be used, not in general for all Xen
> > > > > > domains, because that is what the check was really meant to do.
> > > > >
> > > > 

Re: [PATCH RFC 4/5] vhost-vdpa: support IOTLB batching hints

2020-06-28 Thread Michael S. Tsirkin
On Thu, Jun 18, 2020 at 01:56:25PM +0800, Jason Wang wrote:
> This patches extend the vhost IOTLB API to accept batch updating hints
> form userspace. When userspace wants update the device IOTLB in a
> batch, it may do:
> 
> 1) Write vhost_iotlb_msg with VHOST_IOTLB_BATCH_BEGIN flag
> 2) Perform a batch of IOTLB updating via VHOST_IOTLB_UPDATE/INVALIDATE
> 3) Write vhost_iotlb_msg with VHOST_IOTLB_BATCH_END flag

As long as we are extending the interface,
is there some way we could cut down the number of system calls needed
here?


> 
> Vhost-vdpa may decide to batch the IOMMU/IOTLB updating in step 3 when
> vDPA device support set_map() ops. This is useful for the vDPA device
> that want to know all the mappings to tweak their own DMA translation
> logic.
> 
> For vDPA device that doesn't require set_map(), no behavior changes.
> 
> This capability is advertised via VHOST_BACKEND_F_IOTLB_BATCH capability.
> 
> Signed-off-by: Jason Wang 
> ---
>  drivers/vhost/vdpa.c | 30 +++---
>  include/uapi/linux/vhost.h   |  2 ++
>  include/uapi/linux/vhost_types.h |  7 +++
>  3 files changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 453057421f80..8f624bbafee7 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -56,7 +56,9 @@ enum {
>  };
>  
>  enum {
> - VHOST_VDPA_BACKEND_FEATURES = (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2)
> + VHOST_VDPA_BACKEND_FEATURES =
> + (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2) |
> + (1ULL << VHOST_BACKEND_F_IOTLB_BATCH),
>  };
>  
>  /* Currently, only network backend w/o multiqueue is supported. */
> @@ -77,6 +79,7 @@ struct vhost_vdpa {
>   int virtio_id;
>   int minor;
>   struct eventfd_ctx *config_ctx;
> + int in_batch;
>  };
>  
>  static DEFINE_IDA(vhost_vdpa_ida);
> @@ -125,6 +128,7 @@ static void vhost_vdpa_reset(struct vhost_vdpa *v)
>   const struct vdpa_config_ops *ops = vdpa->config;
>  
>   ops->set_status(vdpa, 0);
> + v->in_batch = 0;
>  }
>  
>  static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user *argp)
> @@ -540,9 +544,10 @@ static int vhost_vdpa_map(struct vhost_vdpa *v,
>  
>   if (ops->dma_map)
>   r = ops->dma_map(vdpa, iova, size, pa, perm);
> - else if (ops->set_map)
> - r = ops->set_map(vdpa, dev->iotlb);
> - else
> + else if (ops->set_map) {
> + if (!v->in_batch)
> + r = ops->set_map(vdpa, dev->iotlb);
> + } else
>   r = iommu_map(v->domain, iova, pa, size,
> perm_to_iommu_flags(perm));
>  
> @@ -559,9 +564,10 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v, u64 
> iova, u64 size)
>  
>   if (ops->dma_map)
>   ops->dma_unmap(vdpa, iova, size);
> - else if (ops->set_map)
> - ops->set_map(vdpa, dev->iotlb);
> - else
> + else if (ops->set_map) {
> + if (!v->in_batch)
> + ops->set_map(vdpa, dev->iotlb);
> + } else
>   iommu_unmap(v->domain, iova, size);
>  }
>  
> @@ -655,6 +661,8 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev 
> *dev,
>   struct vhost_iotlb_msg *msg)
>  {
>   struct vhost_vdpa *v = container_of(dev, struct vhost_vdpa, vdev);
> + struct vdpa_device *vdpa = v->vdpa;
> + const struct vdpa_config_ops *ops = vdpa->config;
>   int r = 0;
>  
>   r = vhost_dev_check_owner(dev);
> @@ -668,6 +676,14 @@ static int vhost_vdpa_process_iotlb_msg(struct vhost_dev 
> *dev,
>   case VHOST_IOTLB_INVALIDATE:
>   vhost_vdpa_unmap(v, msg->iova, msg->size);
>   break;
> + case VHOST_IOTLB_BATCH_BEGIN:
> + v->in_batch = true;
> + break;
> + case VHOST_IOTLB_BATCH_END:
> + if (v->in_batch && ops->set_map)
> + ops->set_map(vdpa, dev->iotlb);
> + v->in_batch = false;
> + break;
>   default:
>   r = -EINVAL;
>   break;
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index 0c2349612e77..565da96f55d5 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -91,6 +91,8 @@
>  
>  /* Use message type V2 */
>  #define VHOST_BACKEND_F_IOTLB_MSG_V2 0x1
> +/* IOTLB can accpet batching hints */

typo

> +#define VHOST_BACKEND_F_IOTLB_BATCH  0x2
>  
>  #define VHOST_SET_BACKEND_FEATURES _IOW(VHOST_VIRTIO, 0x25, __u64)
>  #define VHOST_GET_BACKEND_FEATURES _IOR(VHOST_VIRTIO, 0x26, __u64)
> diff --git a/include/uapi/linux/vhost_types.h 
> b/include/uapi/linux/vhost_types.h
> index 669457ce5c48..5c12faffdde9 100644
> --- a/include/uapi/linux/vhost_types.h
> +++ b/include/uapi/linux/vhost_types.h
> @@ -60,6 +60,13 @@ struct vhost_iotlb_msg {
>  #define VHOST_IOTLB_UPDATE 2
>  #define VHOST_IOTLB_INVALIDATE 3
>  #define 

Re: [RFC 0/3] virtio: NUMA-aware memory allocation

2020-06-28 Thread Jason Wang


On 2020/6/25 下午9:57, Stefan Hajnoczi wrote:

These patches are not ready to be merged because I was unable to measure a
performance improvement. I'm publishing them so they are archived in case
someone picks up this work again in the future.

The goal of these patches is to allocate virtqueues and driver state from the
device's NUMA node for optimal memory access latency. Only guests with a vNUMA
topology and virtio devices spread across vNUMA nodes benefit from this.  In
other cases the memory placement is fine and we don't need to take NUMA into
account inside the guest.

These patches could be extended to virtio_net.ko and other devices in the
future. I only tested virtio_blk.ko.

The benchmark configuration was designed to trigger worst-case NUMA placement:
  * Physical NVMe storage controller on host NUMA node 0
  * IOThread pinned to host NUMA node 0
  * virtio-blk-pci device in vNUMA node 1
  * vCPU 0 on host NUMA node 1 and vCPU 1 on host NUMA node 0
  * vCPU 0 in vNUMA node 0 and vCPU 1 in vNUMA node 1

The intent is to have .probe() code run on vCPU 0 in vNUMA node 0 (host NUMA
node 1) so that memory is in the wrong NUMA node for the virtio-blk-pci devic=
e.
Applying these patches fixes memory placement so that virtqueues and driver
state is allocated in vNUMA node 1 where the virtio-blk-pci device is located.

The fio 4KB randread benchmark results do not show a significant improvement:

Name  IOPS   Error
virtio-blk42373.79 =C2=B1 0.54%
virtio-blk-numa   42517.07 =C2=B1 0.79%



I remember I did something similar in vhost by using page_to_nid() for 
descriptor ring. And I get little improvement as shown here.


Michael reminds that it was probably because all data were cached. So I 
doubt if the test lacks sufficient stress on the cache ...


Thanks




Stefan Hajnoczi (3):
   virtio-pci: use NUMA-aware memory allocation in probe
   virtio_ring: use NUMA-aware memory allocation in probe
   virtio-blk: use NUMA-aware memory allocation in probe

  include/linux/gfp.h|  2 +-
  drivers/block/virtio_blk.c |  7 +--
  drivers/virtio/virtio_pci_common.c | 16 
  drivers/virtio/virtio_ring.c   | 26 +-
  mm/page_alloc.c|  2 +-
  5 files changed, 36 insertions(+), 17 deletions(-)

--=20
2.26.2



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 2/2] virtio: virtio_has_iommu_quirk -> virtio_has_dma_quirk

2020-06-28 Thread Jason Wang


On 2020/6/25 上午7:21, Michael S. Tsirkin wrote:

Now that the corresponding feature bit has been renamed,
rename the quirk too - it's about special ways to
do DMA, not necessarily about the IOMMU.

Signed-off-by: Michael S. Tsirkin 
---
  drivers/gpu/drm/virtio/virtgpu_object.c | 2 +-
  drivers/gpu/drm/virtio/virtgpu_vq.c | 4 ++--
  drivers/virtio/virtio_ring.c| 2 +-
  include/linux/virtio_config.h   | 4 ++--
  tools/virtio/linux/virtio_config.h  | 4 ++--
  5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c 
b/drivers/gpu/drm/virtio/virtgpu_object.c
index 6ccbd01cd888..e8799ab0c753 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -141,7 +141,7 @@ static int virtio_gpu_object_shmem_init(struct 
virtio_gpu_device *vgdev,
struct virtio_gpu_mem_entry **ents,
unsigned int *nents)
  {
-   bool use_dma_api = !virtio_has_iommu_quirk(vgdev->vdev);
+   bool use_dma_api = !virtio_has_dma_quirk(vgdev->vdev);
struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
struct scatterlist *sg;
int si, ret;
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c 
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 9e663a5d9952..53af60d484a4 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -599,7 +599,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct 
virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(objs->objs[0]);
struct virtio_gpu_transfer_to_host_2d *cmd_p;
struct virtio_gpu_vbuffer *vbuf;
-   bool use_dma_api = !virtio_has_iommu_quirk(vgdev->vdev);
+   bool use_dma_api = !virtio_has_dma_quirk(vgdev->vdev);
struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
  
  	if (use_dma_api)

@@ -1015,7 +1015,7 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct 
virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(objs->objs[0]);
struct virtio_gpu_transfer_host_3d *cmd_p;
struct virtio_gpu_vbuffer *vbuf;
-   bool use_dma_api = !virtio_has_iommu_quirk(vgdev->vdev);
+   bool use_dma_api = !virtio_has_dma_quirk(vgdev->vdev);
struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
  
  	if (use_dma_api)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index a1a5c2a91426..34253cb69cb8 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -240,7 +240,7 @@ static inline bool virtqueue_use_indirect(struct virtqueue 
*_vq,
  
  static bool vring_use_dma_api(struct virtio_device *vdev)

  {
-   if (!virtio_has_iommu_quirk(vdev))
+   if (!virtio_has_dma_quirk(vdev))
return true;
  
  	/* Otherwise, we are left to guess. */

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index f2cc2a0df174..3b4eae5ac5e3 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -162,10 +162,10 @@ static inline bool virtio_has_feature(const struct 
virtio_device *vdev,
  }
  
  /**

- * virtio_has_iommu_quirk - determine whether this device has the iommu quirk
+ * virtio_has_dma_quirk - determine whether this device has the DMA quirk
   * @vdev: the device
   */
-static inline bool virtio_has_iommu_quirk(const struct virtio_device *vdev)
+static inline bool virtio_has_dma_quirk(const struct virtio_device *vdev)
  {
/*
 * Note the reverse polarity of the quirk feature (compared to most
diff --git a/tools/virtio/linux/virtio_config.h 
b/tools/virtio/linux/virtio_config.h
index f99ae42668e0..f2640e505c4e 100644
--- a/tools/virtio/linux/virtio_config.h
+++ b/tools/virtio/linux/virtio_config.h
@@ -42,10 +42,10 @@ static inline void __virtio_clear_bit(struct virtio_device 
*vdev,
(__virtio_test_bit((dev), feature))
  
  /**

- * virtio_has_iommu_quirk - determine whether this device has the iommu quirk
+ * virtio_has_dma_quirk - determine whether this device has the DMA quirk
   * @vdev: the device
   */
-static inline bool virtio_has_iommu_quirk(const struct virtio_device *vdev)
+static inline bool virtio_has_dma_quirk(const struct virtio_device *vdev)
  {
/*
 * Note the reverse polarity of the quirk feature (compared to most



Acked-by: Jason Wang 


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio: VIRTIO_F_IOMMU_PLATFORM -> VIRTIO_F_ACCESS_PLATFORM

2020-06-28 Thread Jason Wang


On 2020/6/25 上午6:25, Michael S. Tsirkin wrote:

Rename the bit to match latest virtio spec.
Add a compat macro to avoid breaking existing userspace.

Signed-off-by: Michael S. Tsirkin 
---
  arch/um/drivers/virtio_uml.c   |  2 +-
  drivers/vdpa/ifcvf/ifcvf_base.h|  2 +-
  drivers/vdpa/vdpa_sim/vdpa_sim.c   |  4 ++--
  drivers/vhost/net.c|  4 ++--
  drivers/vhost/vdpa.c   |  2 +-
  drivers/virtio/virtio_balloon.c|  2 +-
  drivers/virtio/virtio_ring.c   |  2 +-
  include/linux/virtio_config.h  |  2 +-
  include/uapi/linux/virtio_config.h | 10 +++---
  tools/virtio/linux/virtio_config.h |  2 +-
  10 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c
index 351aee52aca6..a6c4bb6c2c01 100644
--- a/arch/um/drivers/virtio_uml.c
+++ b/arch/um/drivers/virtio_uml.c
@@ -385,7 +385,7 @@ static irqreturn_t vu_req_interrupt(int irq, void *data)
}
break;
case VHOST_USER_SLAVE_IOTLB_MSG:
-   /* not supported - VIRTIO_F_IOMMU_PLATFORM */
+   /* not supported - VIRTIO_F_ACCESS_PLATFORM */
case VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG:
/* not supported - VHOST_USER_PROTOCOL_F_HOST_NOTIFIER */
default:
diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
index f4554412e607..24af422b5a3e 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.h
+++ b/drivers/vdpa/ifcvf/ifcvf_base.h
@@ -29,7 +29,7 @@
 (1ULL << VIRTIO_F_VERSION_1) | \
 (1ULL << VIRTIO_NET_F_STATUS)| \
 (1ULL << VIRTIO_F_ORDER_PLATFORM)| \
-(1ULL << VIRTIO_F_IOMMU_PLATFORM)| \
+(1ULL << VIRTIO_F_ACCESS_PLATFORM)   | \
 (1ULL << VIRTIO_NET_F_MRG_RXBUF))
  
  /* Only one queue pair for now. */

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index c7334cc65bb2..a9bc5e0fb353 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -55,7 +55,7 @@ struct vdpasim_virtqueue {
  
  static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |

  (1ULL << VIRTIO_F_VERSION_1)  |
- (1ULL << VIRTIO_F_IOMMU_PLATFORM);
+ (1ULL << VIRTIO_F_ACCESS_PLATFORM);
  
  /* State of each vdpasim device */

  struct vdpasim {
@@ -450,7 +450,7 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, 
u64 features)
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
  
  	/* DMA mapping must be done by driver */

-   if (!(features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)))
+   if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
return -EINVAL;
  
  	vdpasim->features = features & vdpasim_features;

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index e992decfec53..8e0921d3805d 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -73,7 +73,7 @@ enum {
VHOST_NET_FEATURES = VHOST_FEATURES |
 (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) |
 (1ULL << VIRTIO_NET_F_MRG_RXBUF) |
-(1ULL << VIRTIO_F_IOMMU_PLATFORM)
+(1ULL << VIRTIO_F_ACCESS_PLATFORM)
  };
  
  enum {

@@ -1653,7 +1653,7 @@ static int vhost_net_set_features(struct vhost_net *n, 
u64 features)
!vhost_log_access_ok(>dev))
goto out_unlock;
  
-	if ((features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))) {

+   if ((features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) {
if (vhost_init_device_iotlb(>dev, true))
goto out_unlock;
}
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index a54b60d6623f..18869a35d408 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -31,7 +31,7 @@ enum {
(1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |
(1ULL << VIRTIO_F_ANY_LAYOUT) |
(1ULL << VIRTIO_F_VERSION_1) |
-   (1ULL << VIRTIO_F_IOMMU_PLATFORM) |
+   (1ULL << VIRTIO_F_ACCESS_PLATFORM) |
(1ULL << VIRTIO_F_RING_PACKED) |
(1ULL << VIRTIO_F_ORDER_PLATFORM) |
(1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 1f157d2f4952..fc7301406540 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -1120,7 +1120,7 @@ static int virtballoon_validate(struct virtio_device 
*vdev)
else if (!virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON))
__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_REPORTING);
  
-	__virtio_clear_bit(vdev, VIRTIO_F_IOMMU_PLATFORM);

+   __virtio_clear_bit(vdev, VIRTIO_F_ACCESS_PLATFORM);
return 0;
  }
  
diff --git