Re: [PATCH v3 0/6] Add virtio-iommu built-in topology

2020-09-25 Thread Michael S. Tsirkin
On Fri, Sep 25, 2020 at 01:26:29PM +0200, Jean-Philippe Brucker wrote:
> On Fri, Sep 25, 2020 at 06:22:57AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Sep 25, 2020 at 10:48:06AM +0200, Jean-Philippe Brucker wrote:
> > > On Fri, Aug 21, 2020 at 03:15:34PM +0200, Jean-Philippe Brucker wrote:
> > > > Add a topology description to the virtio-iommu driver and enable x86
> > > > platforms.
> > > > 
> > > > Since [v2] we have made some progress on adding ACPI support for
> > > > virtio-iommu, which is the preferred boot method on x86. It will be a
> > > > new vendor-agnostic table describing para-virtual topologies in a
> > > > minimal format. However some platforms don't use either ACPI or DT for
> > > > booting (for example microvm), and will need the alternative topology
> > > > description method proposed here. In addition, since the process to get
> > > > a new ACPI table will take a long time, this provides a boot method even
> > > > to ACPI-based platforms, if only temporarily for testing and
> > > > development.
> > > > 
> > > > v3:
> > > > * Add patch 1 that moves virtio-iommu to a subfolder.
> > > > * Split the rest:
> > > >   * Patch 2 adds topology-helper.c, which will be shared with the ACPI
> > > > support.
> > > >   * Patch 4 adds definitions.
> > > >   * Patch 5 adds parser in topology.c.
> > > > * Address other comments.
> > > > 
> > > > Linux and QEMU patches available at:
> > > > https://jpbrucker.net/git/linux virtio-iommu/devel
> > > > https://jpbrucker.net/git/qemu virtio-iommu/devel
> > > 
> > > I'm parking this work again, until we make progress on the ACPI table, or
> > > until a platform without ACPI and DT needs it. Until then, I've pushed v4
> > > to my virtio-iommu/topo branch and will keep it rebased on master.
> > > 
> > > Thanks,
> > > Jean
> > 
> > I think you guys need to work on virtio spec too, not too much left to
> > do there ...
> 
> I know it's ready and I'd really like to move on with this, but I'd rather
> not commit it to the spec until we know it's going to be used at all. As
> Gerd pointed out the one example we had, microvm, now supports ACPI. Since
> we've kicked off the ACPI work anyway it isn't clear that the built-in
> topology will be useful.
> 
> Thanks,
> Jean

Many power platforms are OF based, thus without ACPI or DT support.

-- 
MST

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


Re: [PATCH v3 0/6] Add virtio-iommu built-in topology

2020-09-25 Thread Michael S. Tsirkin
On Fri, Sep 25, 2020 at 10:48:06AM +0200, Jean-Philippe Brucker wrote:
> On Fri, Aug 21, 2020 at 03:15:34PM +0200, Jean-Philippe Brucker wrote:
> > Add a topology description to the virtio-iommu driver and enable x86
> > platforms.
> > 
> > Since [v2] we have made some progress on adding ACPI support for
> > virtio-iommu, which is the preferred boot method on x86. It will be a
> > new vendor-agnostic table describing para-virtual topologies in a
> > minimal format. However some platforms don't use either ACPI or DT for
> > booting (for example microvm), and will need the alternative topology
> > description method proposed here. In addition, since the process to get
> > a new ACPI table will take a long time, this provides a boot method even
> > to ACPI-based platforms, if only temporarily for testing and
> > development.
> > 
> > v3:
> > * Add patch 1 that moves virtio-iommu to a subfolder.
> > * Split the rest:
> >   * Patch 2 adds topology-helper.c, which will be shared with the ACPI
> > support.
> >   * Patch 4 adds definitions.
> >   * Patch 5 adds parser in topology.c.
> > * Address other comments.
> > 
> > Linux and QEMU patches available at:
> > https://jpbrucker.net/git/linux virtio-iommu/devel
> > https://jpbrucker.net/git/qemu virtio-iommu/devel
> 
> I'm parking this work again, until we make progress on the ACPI table, or
> until a platform without ACPI and DT needs it. Until then, I've pushed v4
> to my virtio-iommu/topo branch and will keep it rebased on master.
> 
> Thanks,
> Jean

I think you guys need to work on virtio spec too, not too much left to
do there ...

-- 
MST

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


Re: [PATCH v3 0/6] Add virtio-iommu built-in topology

2020-09-25 Thread Michael S. Tsirkin
On Thu, Sep 24, 2020 at 02:50:46PM +0200, Joerg Roedel wrote:
> On Thu, Sep 24, 2020 at 08:41:21AM -0400, Michael S. Tsirkin wrote:
> > But this has nothing to do with Linux.  There is also no guarantee that
> > the two committees will decide to use exactly the same format. Once one
> > of them sets the format in stone, we can add support for that format to
> > linux. If another one is playing nice and uses the same format, we can
> > use the same parsers. If it doesn't linux will have to follow suit.
> 
> Or Linux decides to support only one of the formats, which would then be
> ACPI.
> 
> Regards,
> 
>   Joerg

It's really up to hypervisors not guests, linux as a guest can for sure
refuse to work somewhere, but that's normally not very attractive.

-- 
MST

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


Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors

2020-09-25 Thread Michael S. Tsirkin
On Thu, Sep 24, 2020 at 01:24:13PM +0300, Eli Cohen wrote:
> On Thu, Sep 24, 2020 at 05:30:55AM -0400, Michael S. Tsirkin wrote:
> > > > --- linux-next-20200917.orig/drivers/vdpa/Kconfig
> > > > +++ linux-next-20200917/drivers/vdpa/Kconfig
> > > > @@ -31,7 +31,7 @@ config IFCVF
> > > >
> > > >  config MLX5_VDPA
> > > > bool "MLX5 VDPA support library for ConnectX devices"
> > > > -   depends on MLX5_CORE
> > > > +   depends on VHOST_IOTLB && MLX5_CORE
> > > > default n
> > > 
> > > While we are here, can anyone who apply this patch delete the "default n" 
> > > line?
> > > It is by default "n".
> 
> I can do that
> 
> > > 
> > > Thanks
> > 
> > Hmm other drivers select VHOST_IOTLB, why not do the same?
> 
> I can't see another driver doing that.

Well grep VHOST_IOTLB and you will see some examples.

> Perhaps I can set dependency on
> VHOST which by itself depends on VHOST_IOTLB?

VHOST is processing virtio in the kernel. You don't really need that
for mlx, do you?

> > 
> > 
> > > > help
> > > >   Support library for Mellanox VDPA drivers. Provides code that 
> > > > is
> > > >
> > 

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


Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors

2020-09-25 Thread Michael S. Tsirkin
On Fri, Sep 25, 2020 at 10:20:05AM +0300, Leon Romanovsky wrote:
> On Thu, Sep 24, 2020 at 12:02:43PM -0400, Michael S. Tsirkin wrote:
> > On Thu, Sep 24, 2020 at 08:47:05AM -0700, Randy Dunlap wrote:
> > > On 9/24/20 3:24 AM, Eli Cohen wrote:
> > > > On Thu, Sep 24, 2020 at 05:30:55AM -0400, Michael S. Tsirkin wrote:
> > > >>>> --- linux-next-20200917.orig/drivers/vdpa/Kconfig
> > > >>>> +++ linux-next-20200917/drivers/vdpa/Kconfig
> > > >>>> @@ -31,7 +31,7 @@ config IFCVF
> > > >>>>
> > > >>>>  config MLX5_VDPA
> > > >>>>  bool "MLX5 VDPA support library for ConnectX devices"
> > > >>>> -depends on MLX5_CORE
> > > >>>> +depends on VHOST_IOTLB && MLX5_CORE
> > > >>>>  default n
> > > >>>
> > > >>> While we are here, can anyone who apply this patch delete the 
> > > >>> "default n" line?
> > > >>> It is by default "n".
> > > >
> > > > I can do that
> > > >
> > > >>>
> > > >>> Thanks
> > > >>
> > > >> Hmm other drivers select VHOST_IOTLB, why not do the same?
> > >
> > > v1 used select, but Saeed requested use of depends instead because
> > > select can cause problems.
> > >
> > > > I can't see another driver doing that. Perhaps I can set dependency on
> > > > VHOST which by itself depends on VHOST_IOTLB?
> > > >>
> > > >>
> > > >>>>  help
> > > >>>>Support library for Mellanox VDPA drivers. Provides code that 
> > > >>>> is
> > > >>>>
> > > >>
> > >
> >
> > Saeed what kind of problems? It's used with select in other places,
> > isn't it?
> 
> IMHO, "depends" is much more explicit than "select".
> 
> Thanks

This is now how VHOST_IOTLB has been designed though.
If you want to change VHOST_IOTLB to depends I think
we should do it consistently all over.


config VHOST_IOTLB
tristate
help
  Generic IOTLB implementation for vhost and vringh.
  This option is selected by any driver which needs to support
  an IOMMU in software.


> >
> > > --
> > > ~Randy
> >

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


Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors

2020-09-24 Thread Michael S. Tsirkin
On Thu, Sep 24, 2020 at 08:47:05AM -0700, Randy Dunlap wrote:
> On 9/24/20 3:24 AM, Eli Cohen wrote:
> > On Thu, Sep 24, 2020 at 05:30:55AM -0400, Michael S. Tsirkin wrote:
> >>>> --- linux-next-20200917.orig/drivers/vdpa/Kconfig
> >>>> +++ linux-next-20200917/drivers/vdpa/Kconfig
> >>>> @@ -31,7 +31,7 @@ config IFCVF
> >>>>
> >>>>  config MLX5_VDPA
> >>>>  bool "MLX5 VDPA support library for ConnectX devices"
> >>>> -depends on MLX5_CORE
> >>>> +depends on VHOST_IOTLB && MLX5_CORE
> >>>>  default n
> >>>
> >>> While we are here, can anyone who apply this patch delete the "default n" 
> >>> line?
> >>> It is by default "n".
> > 
> > I can do that
> > 
> >>>
> >>> Thanks
> >>
> >> Hmm other drivers select VHOST_IOTLB, why not do the same?
> 
> v1 used select, but Saeed requested use of depends instead because
> select can cause problems.
> 
> > I can't see another driver doing that. Perhaps I can set dependency on
> > VHOST which by itself depends on VHOST_IOTLB?
> >>
> >>
> >>>>  help
> >>>>Support library for Mellanox VDPA drivers. Provides code that 
> >>>> is
> >>>>
> >>
> 

Saeed what kind of problems? It's used with select in other places,
isn't it?

> -- 
> ~Randy

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


Re: [PATCH v3 0/6] Add virtio-iommu built-in topology

2020-09-24 Thread Michael S. Tsirkin
On Thu, Sep 24, 2020 at 12:02:55PM +0200, Joerg Roedel wrote:
> On Thu, Sep 24, 2020 at 05:38:13AM -0400, Michael S. Tsirkin wrote:
> > On Thu, Sep 24, 2020 at 11:21:29AM +0200, Joerg Roedel wrote:
> > > On Thu, Sep 24, 2020 at 05:00:35AM -0400, Michael S. Tsirkin wrote:
> > > > OK so this looks good. Can you pls repost with the minor tweak
> > > > suggested and all acks included, and I will queue this?
> > > 
> > > My NACK still stands, as long as a few questions are open:
> > > 
> > >   1) The format used here will be the same as in the ACPI table? I
> > >  think the answer to this questions must be Yes, so this leads
> > >  to the real question:
> > 
> > I am not sure it's a must.
> 
> It is, having only one parser for the ACPI and MMIO descriptions was one
> of the selling points for MMIO in past discussions and I think it makes
> sense to keep them in sync.
> 
> > We can always tweak the parser if there are slight differences
> > between ACPI and virtio formats.
> 
> There is no guarantee that there only need to be "tweaks" until the
> ACPI table format is stablized.
> 
> Regards,
> 
>   Joerg

But this has nothing to do with Linux.  There is also no guarantee that
the two committees will decide to use exactly the same format. Once one
of them sets the format in stone, we can add support for that format to
linux. If another one is playing nice and uses the same format, we can
use the same parsers. If it doesn't linux will have to follow suit.

-- 
MST

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


Re: [PATCH v3 0/6] Add virtio-iommu built-in topology

2020-09-24 Thread Michael S. Tsirkin
On Thu, Sep 24, 2020 at 11:21:29AM +0200, Joerg Roedel wrote:
> On Thu, Sep 24, 2020 at 05:00:35AM -0400, Michael S. Tsirkin wrote:
> > OK so this looks good. Can you pls repost with the minor tweak
> > suggested and all acks included, and I will queue this?
> 
> My NACK still stands, as long as a few questions are open:
> 
>   1) The format used here will be the same as in the ACPI table? I
>  think the answer to this questions must be Yes, so this leads
>  to the real question:

I am not sure it's a must.
We can always tweak the parser if there are slight differences
between ACPI and virtio formats.

But we do want the virtio format used here to be approved by the virtio
TC, so it won't change.

Eric, Jean-Philippe, does one of you intend to create a github issue
and request a ballot for the TC? It's been posted end of August with no
changes ...

>   2) Has the ACPI table format stabalized already? If and only if
>  the answer is Yes I will Ack these patches. We don't need to
>  wait until the ACPI table format is published in a
>  specification update, but at least some certainty that it
>  will not change in incompatible ways anymore is needed.
> 

Not that I know, but I don't see why it's a must.

> So what progress has been made with the ACPI table specification, is it
> just a matter of time to get it approved or are there concerns?
> 
> Regards,
> 
>   Joerg

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


Re: [RFC PATCH 02/24] vhost-vdpa: fix vqs leak in vhost_vdpa_open()

2020-09-24 Thread Michael S. Tsirkin
On Thu, Sep 24, 2020 at 11:21:03AM +0800, Jason Wang wrote:
> We need to free vqs during the err path after it has been allocated
> since vhost won't do that for us.
> 
> Signed-off-by: Jason Wang 

This is a bugfix too right? I don't see it posted separately ...

> ---
>  drivers/vhost/vdpa.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 796fe979f997..9c641274b9f3 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -764,6 +764,12 @@ static void vhost_vdpa_free_domain(struct vhost_vdpa *v)
>   v->domain = NULL;
>  }
>  
> +static void vhost_vdpa_cleanup(struct vhost_vdpa *v)
> +{
> + vhost_dev_cleanup(>vdev);
> + kfree(v->vdev.vqs);
> +}
> +
>  static int vhost_vdpa_open(struct inode *inode, struct file *filep)
>  {
>   struct vhost_vdpa *v;
> @@ -809,7 +815,7 @@ static int vhost_vdpa_open(struct inode *inode, struct 
> file *filep)
>   return 0;
>  
>  err_init_iotlb:
> - vhost_dev_cleanup(>vdev);
> + vhost_vdpa_cleanup(v);
>  err:
>   atomic_dec(>opened);
>   return r;
> @@ -840,8 +846,7 @@ static int vhost_vdpa_release(struct inode *inode, struct 
> file *filep)
>   vhost_vdpa_free_domain(v);
>   vhost_vdpa_config_put(v);
>   vhost_vdpa_clean_irq(v);
> - vhost_dev_cleanup(>vdev);
> - kfree(v->vdev.vqs);
> + vhost_vdpa_cleanup(v);
>   mutex_unlock(>mutex);
>  
>   atomic_dec(>opened);
> -- 
> 2.20.1

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


Re: [PATCH v3 -next] vdpa: mlx5: change Kconfig depends to fix build errors

2020-09-24 Thread Michael S. Tsirkin
On Fri, Sep 18, 2020 at 11:22:45AM +0300, Leon Romanovsky wrote:
> On Thu, Sep 17, 2020 at 07:35:03PM -0700, Randy Dunlap wrote:
> > From: Randy Dunlap 
> >
> > drivers/vdpa/mlx5/ uses vhost_iotlb*() interfaces, so add a dependency
> > on VHOST to eliminate build errors.
> >
> > ld: drivers/vdpa/mlx5/core/mr.o: in function `add_direct_chain':
> > mr.c:(.text+0x106): undefined reference to `vhost_iotlb_itree_first'
> > ld: mr.c:(.text+0x1cf): undefined reference to `vhost_iotlb_itree_next'
> > ld: mr.c:(.text+0x30d): undefined reference to `vhost_iotlb_itree_first'
> > ld: mr.c:(.text+0x3e8): undefined reference to `vhost_iotlb_itree_next'
> > ld: drivers/vdpa/mlx5/core/mr.o: in function `_mlx5_vdpa_create_mr':
> > mr.c:(.text+0x908): undefined reference to `vhost_iotlb_itree_first'
> > ld: mr.c:(.text+0x9e6): undefined reference to `vhost_iotlb_itree_next'
> > ld: drivers/vdpa/mlx5/core/mr.o: in function `mlx5_vdpa_handle_set_map':
> > mr.c:(.text+0xf1d): undefined reference to `vhost_iotlb_itree_first'
> >
> > Signed-off-by: Randy Dunlap 
> > Cc: "Michael S. Tsirkin" 
> > Cc: Jason Wang 
> > Cc: virtualization@lists.linux-foundation.org
> > Cc: Saeed Mahameed 
> > Cc: Leon Romanovsky 
> > Cc: net...@vger.kernel.org
> > ---
> > v2: change from select to depends on VHOST (Saeed)
> > v3: change to depends on VHOST_IOTLB (Jason)
> >
> >  drivers/vdpa/Kconfig |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > --- linux-next-20200917.orig/drivers/vdpa/Kconfig
> > +++ linux-next-20200917/drivers/vdpa/Kconfig
> > @@ -31,7 +31,7 @@ config IFCVF
> >
> >  config MLX5_VDPA
> > bool "MLX5 VDPA support library for ConnectX devices"
> > -   depends on MLX5_CORE
> > +   depends on VHOST_IOTLB && MLX5_CORE
> > default n
> 
> While we are here, can anyone who apply this patch delete the "default n" 
> line?
> It is by default "n".
> 
> Thanks

Hmm other drivers select VHOST_IOTLB, why not do the same?


> > help
> >   Support library for Mellanox VDPA drivers. Provides code that is
> >

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


Re: [PATCH v3 0/6] Add virtio-iommu built-in topology

2020-09-24 Thread Michael S. Tsirkin
On Fri, Sep 04, 2020 at 06:24:12PM +0200, Auger Eric wrote:
> Hi,
> 
> On 8/21/20 3:15 PM, Jean-Philippe Brucker wrote:
> > Add a topology description to the virtio-iommu driver and enable x86
> > platforms.
> > 
> > Since [v2] we have made some progress on adding ACPI support for
> > virtio-iommu, which is the preferred boot method on x86. It will be a
> > new vendor-agnostic table describing para-virtual topologies in a
> > minimal format. However some platforms don't use either ACPI or DT for
> > booting (for example microvm), and will need the alternative topology
> > description method proposed here. In addition, since the process to get
> > a new ACPI table will take a long time, this provides a boot method even
> > to ACPI-based platforms, if only temporarily for testing and
> > development.
> > 
> > v3:
> > * Add patch 1 that moves virtio-iommu to a subfolder.
> > * Split the rest:
> >   * Patch 2 adds topology-helper.c, which will be shared with the ACPI
> > support.
> >   * Patch 4 adds definitions.
> >   * Patch 5 adds parser in topology.c.
> > * Address other comments.
> > 
> > Linux and QEMU patches available at:
> > https://jpbrucker.net/git/linux virtio-iommu/devel
> > https://jpbrucker.net/git/qemu virtio-iommu/devel
> I have tested that series with above QEMU branch on ARM with virtio-net
> and virtio-blk translated devices in non DT mode.
> 
> It works for me:
> Tested-by: Eric Auger 
> 
> Thanks
> 
> Eric

OK so this looks good. Can you pls repost with the minor tweak
suggested and all acks included, and I will queue this?

Thanks!

> > 
> > [spec] https://lists.oasis-open.org/archives/virtio-dev/202008/msg00067.html
> > [v2] 
> > https://lore.kernel.org/linux-iommu/20200228172537.377327-1-jean-phili...@linaro.org/
> > [v1] 
> > https://lore.kernel.org/linux-iommu/20200214160413.1475396-1-jean-phili...@linaro.org/
> > [rfc] 
> > https://lore.kernel.org/linux-iommu/20191122105000.800410-1-jean-phili...@linaro.org/
> > 
> > Jean-Philippe Brucker (6):
> >   iommu/virtio: Move to drivers/iommu/virtio/
> >   iommu/virtio: Add topology helpers
> >   PCI: Add DMA configuration for virtual platforms
> >   iommu/virtio: Add topology definitions
> >   iommu/virtio: Support topology description in config space
> >   iommu/virtio: Enable x86 support
> > 
> >  drivers/iommu/Kconfig |  18 +-
> >  drivers/iommu/Makefile|   3 +-
> >  drivers/iommu/virtio/Makefile |   4 +
> >  drivers/iommu/virtio/topology-helpers.h   |  50 +
> >  include/linux/virt_iommu.h|  15 ++
> >  include/uapi/linux/virtio_iommu.h |  44 
> >  drivers/iommu/virtio/topology-helpers.c   | 196 
> >  drivers/iommu/virtio/topology.c   | 259 ++
> >  drivers/iommu/{ => virtio}/virtio-iommu.c |   4 +
> >  drivers/pci/pci-driver.c  |   5 +
> >  MAINTAINERS   |   3 +-
> >  11 files changed, 597 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/iommu/virtio/Makefile
> >  create mode 100644 drivers/iommu/virtio/topology-helpers.h
> >  create mode 100644 include/linux/virt_iommu.h
> >  create mode 100644 drivers/iommu/virtio/topology-helpers.c
> >  create mode 100644 drivers/iommu/virtio/topology.c
> >  rename drivers/iommu/{ => virtio}/virtio-iommu.c (99%)
> > 

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


Re: [RFC PATCH 01/24] vhost-vdpa: fix backend feature ioctls

2020-09-24 Thread Michael S. Tsirkin
On Thu, Sep 24, 2020 at 11:21:02AM +0800, Jason Wang wrote:
> Commit 653055b9acd4 ("vhost-vdpa: support get/set backend features")
> introduces two malfunction backend features ioctls:
> 
> 1) the ioctls was blindly added to vring ioctl instead of vdpa device
>ioctl
> 2) vhost_set_backend_features() was called when dev mutex has already
>been held which will lead a deadlock
> 
> This patch fixes the above issues.
> 
> Cc: Eli Cohen 
> Reported-by: Zhu Lingshan 
> Fixes: 653055b9acd4 ("vhost-vdpa: support get/set backend features")
> Signed-off-by: Jason Wang 

Don't we want the fixes queued right now, as opposed to the rest of the
RFC?

> ---
>  drivers/vhost/vdpa.c | 30 --
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 3fab94f88894..796fe979f997 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -353,8 +353,6 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, 
> unsigned int cmd,
>   struct vdpa_callback cb;
>   struct vhost_virtqueue *vq;
>   struct vhost_vring_state s;
> - u64 __user *featurep = argp;
> - u64 features;
>   u32 idx;
>   long r;
>  
> @@ -381,18 +379,6 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, 
> unsigned int cmd,
>  
>   vq->last_avail_idx = vq_state.avail_index;
>   break;
> - case VHOST_GET_BACKEND_FEATURES:
> - features = VHOST_VDPA_BACKEND_FEATURES;
> - if (copy_to_user(featurep, , sizeof(features)))
> - return -EFAULT;
> - return 0;
> - case VHOST_SET_BACKEND_FEATURES:
> - if (copy_from_user(, featurep, sizeof(features)))
> - return -EFAULT;
> - if (features & ~VHOST_VDPA_BACKEND_FEATURES)
> - return -EOPNOTSUPP;
> - vhost_set_backend_features(>vdev, features);
> - return 0;
>   }
>  
>   r = vhost_vring_ioctl(>vdev, cmd, argp);
> @@ -440,8 +426,20 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>   struct vhost_vdpa *v = filep->private_data;
>   struct vhost_dev *d = >vdev;
>   void __user *argp = (void __user *)arg;
> + u64 __user *featurep = argp;
> + u64 features;
>   long r;
>  
> + if (cmd == VHOST_SET_BACKEND_FEATURES) {
> + r = copy_from_user(, featurep, sizeof(features));
> + if (r)
> + return r;
> + if (features & ~VHOST_VDPA_BACKEND_FEATURES)
> + return -EOPNOTSUPP;
> + vhost_set_backend_features(>vdev, features);
> + return 0;
> + }
> +
>   mutex_lock(>mutex);
>  
>   switch (cmd) {
> @@ -476,6 +474,10 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>   case VHOST_VDPA_SET_CONFIG_CALL:
>   r = vhost_vdpa_set_config_call(v, argp);
>   break;
> + case VHOST_GET_BACKEND_FEATURES:
> + features = VHOST_VDPA_BACKEND_FEATURES;
> + r = copy_to_user(featurep, , sizeof(features));
> + break;
>   default:
>   r = vhost_dev_ioctl(>vdev, cmd, argp);
>   if (r == -ENOIOCTLCMD)
> -- 
> 2.20.1

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


Re: [PATCH 3/8] vhost scsi: alloc cmds per vq instead of session

2020-09-24 Thread Michael S. Tsirkin
On Mon, Sep 21, 2020 at 01:23:03PM -0500, Mike Christie wrote:
> We currently are limited to 256 cmds per session. This leads to problems
> where if the user has increased virtqueue_size to more than 2 or
> cmd_per_lun to more than 256 vhost_scsi_get_tag can fail and the guest
> will get IO errors.
> 
> This patch moves the cmd allocation to per vq so we can easily match
> whatever the user has specified for num_queues and
> virtqueue_size/cmd_per_lun. It also makes it easier to control how much
> memory we preallocate. For cases, where perf is not as important and
> we can use the current defaults (1 vq and 128 cmds per vq) memory use
> from preallocate cmds is cut in half. For cases, where we are willing
> to use more memory for higher perf, cmd mem use will now increase as
> the num queues and queue depth increases.
> 
> Signed-off-by: Mike Christie 
> ---
>  drivers/vhost/scsi.c | 204 
> ---
>  1 file changed, 127 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index b22adf0..13311b8 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -52,7 +52,6 @@
>  #define VHOST_SCSI_VERSION  "v0.1"
>  #define VHOST_SCSI_NAMELEN 256
>  #define VHOST_SCSI_MAX_CDB_SIZE 32
> -#define VHOST_SCSI_DEFAULT_TAGS 256
>  #define VHOST_SCSI_PREALLOC_SGLS 2048
>  #define VHOST_SCSI_PREALLOC_UPAGES 2048
>  #define VHOST_SCSI_PREALLOC_PROT_SGLS 2048
> @@ -189,6 +188,9 @@ struct vhost_scsi_virtqueue {
>* Writers must also take dev mutex and flush under it.
>*/
>   int inflight_idx;
> + struct vhost_scsi_cmd *scsi_cmds;
> + struct sbitmap scsi_tags;
> + int max_cmds;
>  };
>  
>  struct vhost_scsi {
> @@ -324,7 +326,9 @@ static void vhost_scsi_release_cmd(struct se_cmd *se_cmd)
>  {
>   struct vhost_scsi_cmd *tv_cmd = container_of(se_cmd,
>   struct vhost_scsi_cmd, tvc_se_cmd);
> - struct se_session *se_sess = tv_cmd->tvc_nexus->tvn_se_sess;
> + struct vhost_scsi_virtqueue *svq = container_of(tv_cmd->tvc_vq,
> + struct vhost_scsi_virtqueue, vq);
> + struct vhost_scsi_inflight *inflight = tv_cmd->inflight;
>   int i;
>  
>   if (tv_cmd->tvc_sgl_count) {
> @@ -336,8 +340,8 @@ static void vhost_scsi_release_cmd(struct se_cmd *se_cmd)
>   put_page(sg_page(_cmd->tvc_prot_sgl[i]));
>   }
>  
> - vhost_scsi_put_inflight(tv_cmd->inflight);
> - target_free_tag(se_sess, se_cmd);
> + sbitmap_clear_bit(>scsi_tags, se_cmd->map_tag);
> + vhost_scsi_put_inflight(inflight);
>  }
>  
>  static u32 vhost_scsi_sess_get_index(struct se_session *se_sess)
> @@ -566,13 +570,14 @@ static void vhost_scsi_complete_cmd_work(struct 
> vhost_work *work)
>  }
>  
>  static struct vhost_scsi_cmd *
> -vhost_scsi_get_tag(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg,
> +vhost_scsi_get_cmd(struct vhost_virtqueue *vq, struct vhost_scsi_tpg *tpg,
>  unsigned char *cdb, u64 scsi_tag, u16 lun, u8 task_attr,
>  u32 exp_data_len, int data_direction)
>  {
> + struct vhost_scsi_virtqueue *svq = container_of(vq,
> + struct vhost_scsi_virtqueue, vq);
>   struct vhost_scsi_cmd *cmd;
>   struct vhost_scsi_nexus *tv_nexus;
> - struct se_session *se_sess;
>   struct scatterlist *sg, *prot_sg;
>   struct page **pages;
>   int tag, cpu;
> @@ -582,15 +587,14 @@ static void vhost_scsi_complete_cmd_work(struct 
> vhost_work *work)
>   pr_err("Unable to locate active struct vhost_scsi_nexus\n");
>   return ERR_PTR(-EIO);
>   }
> - se_sess = tv_nexus->tvn_se_sess;
>  
> - tag = sbitmap_queue_get(_sess->sess_tag_pool, );
> + tag = sbitmap_get(>scsi_tags, 0, false);
>   if (tag < 0) {
>   pr_err("Unable to obtain tag for vhost_scsi_cmd\n");
>   return ERR_PTR(-ENOMEM);
>   }


After this change, cpu is uninitialized.


>  
> - cmd = &((struct vhost_scsi_cmd *)se_sess->sess_cmd_map)[tag];
> + cmd = >scsi_cmds[tag];
>   sg = cmd->tvc_sgl;
>   prot_sg = cmd->tvc_prot_sgl;
>   pages = cmd->tvc_upages;
> @@ -1065,11 +1069,11 @@ static void vhost_scsi_submission_work(struct 
> work_struct *work)
>   scsi_command_size(cdb), 
> VHOST_SCSI_MAX_CDB_SIZE);
>   goto err;
>   }
> - cmd = vhost_scsi_get_tag(vq, tpg, cdb, tag, lun, task_attr,
> + cmd = vhost_scsi_get_cmd(vq, tpg, cdb, tag, lun, task_attr,
>exp_data_len + prot_bytes,
>data_direction);
>   if (IS_ERR(cmd)) {
> - vq_err(vq, "vhost_scsi_get_tag failed %ld\n",
> + vq_err(vq, "vhost_scsi_get_cmd failed %ld\n",
>  PTR_ERR(cmd));
>   

Re: [PATCH v12 0/2] s390: virtio: let arch validate VIRTIO features

2020-09-22 Thread Michael S. Tsirkin
Will do for the next Linux.

On Tue, Sep 22, 2020 at 02:15:17PM +0200, Christian Borntraeger wrote:
> Michael,
> 
> are you going to pick this series?
> 
> 
> On 10.09.20 10:53, Pierre Morel wrote:
> > Hi all,
> > 
> > The goal of the series is to give a chance to the architecture
> > to validate VIRTIO device features.
> > 
> > I changed VIRTIO_F_IOMMU_PLATFORM to VIRTIO_F_ACCESS_PLATFORM
> > I forgot in drivers/virtio/Kconfig, and put back the inclusion
> > of virtio_config.h for the definition of the callback in
> > arch/s390/mm/init.c I wrongly removed in the last series.
> > 
> > Regards,
> > Pierre
> > 
> > 
> > Pierre Morel (2):
> >   virtio: let arch advertise guest's memory access restrictions
> >   s390: virtio: PV needs VIRTIO I/O device protection
> > 
> >  arch/s390/Kconfig |  1 +
> >  arch/s390/mm/init.c   | 11 +++
> >  drivers/virtio/Kconfig|  6 ++
> >  drivers/virtio/virtio.c   | 15 +++
> >  include/linux/virtio_config.h | 10 ++
> >  5 files changed, 43 insertions(+)
> > 

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


Re: [PATCH 1/2] vhost: remove mutex ops in vhost_set_backend_features

2020-09-21 Thread Michael S. Tsirkin
On Tue, Sep 08, 2020 at 09:00:19PM +0800, Zhu, Lingshan wrote:
> 
> On 9/8/2020 8:05 PM, Michael S. Tsirkin wrote:
> 
> On Mon, Sep 07, 2020 at 06:52:19PM +0800, Zhu Lingshan wrote:
> 
> In vhost_vdpa ioctl SET_BACKEND_FEATURES path, currect code
> would try to acquire vhost dev mutex twice
> (first shown in vhost_vdpa_unlocked_ioctl), which can lead
> to a dead lock issue.
> This commit removed mutex operations in vhost_set_backend_features.
> As a compensation for vhost_net, a followinig commit will add
> needed mutex lock/unlock operations in a new function
> vhost_net_set_backend_features() which is a wrap of
> vhost_set_backend_features().
> 
> Signed-off-by: Zhu Lingshan 
> 
> I think you need to squash these two or reorder, we can't first
> make code racy then fix it up.
> 
> OK, I will send a V2 series with Jason's fixes tomorrow (handle 
> SET/GET_BACKEND_FEATURES in vhost_vdpa ioctl than vring ioctl).
> 
> Thanks,
> BR
> Zhu Lingshan


this never materialized ...

> 
> 
> ---
>  drivers/vhost/vhost.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index b45519ca66a7..e03c9e6f058f 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2591,14 +2591,12 @@ void vhost_set_backend_features(struct 
> vhost_dev *dev, u64 features)
> struct vhost_virtqueue *vq;
> int i;
> 
> -   mutex_lock(>mutex);
> for (i = 0; i < dev->nvqs; ++i) {
> vq = dev->vqs[i];
> mutex_lock(>mutex);
> vq->acked_backend_features = features;
> mutex_unlock(>mutex);
> }
> -   mutex_unlock(>mutex);
>  }
>  EXPORT_SYMBOL_GPL(vhost_set_backend_features);
> 
> --
> 2.18.4
> 

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


Re: [PATCH v5 0/4] Add a vhost RPMsg API

2020-09-08 Thread Michael S. Tsirkin
On Wed, Aug 26, 2020 at 07:46:32PM +0200, Guennadi Liakhovetski wrote:
> Hi,
> 
> Next update:

OK could we get some acks from rpmsg folks on this please?
It's been quite a while, patchset is not huge.


> v5:
> - don't hard-code message layout
> 
> v4:
> - add endianness conversions to comply with the VirtIO standard
> 
> v3:
> - address several checkpatch warnings
> - address comments from Mathieu Poirier
> 
> v2:
> - update patch #5 with a correct vhost_dev_init() prototype
> - drop patch #6 - it depends on a different patch, that is currently
>   an RFC
> - address comments from Pierre-Louis Bossart:
>   * remove "default n" from Kconfig
> 
> Linux supports RPMsg over VirtIO for "remote processor" / AMP use
> cases. It can however also be used for virtualisation scenarios,
> e.g. when using KVM to run Linux on both the host and the guests.
> This patch set adds a wrapper API to facilitate writing vhost
> drivers for such RPMsg-based solutions. The first use case is an
> audio DSP virtualisation project, currently under development, ready
> for review and submission, available at
> https://github.com/thesofproject/linux/pull/1501/commits
> 
> Thanks
> Guennadi
> 
> Guennadi Liakhovetski (4):
>   vhost: convert VHOST_VSOCK_SET_RUNNING to a generic ioctl
>   rpmsg: move common structures and defines to headers
>   rpmsg: update documentation
>   vhost: add an RPMsg API
> 
>  Documentation/rpmsg.txt  |   6 +-
>  drivers/rpmsg/virtio_rpmsg_bus.c |  78 +--
>  drivers/vhost/Kconfig|   7 +
>  drivers/vhost/Makefile   |   3 +
>  drivers/vhost/rpmsg.c| 373 +++
>  drivers/vhost/vhost_rpmsg.h  |  74 ++
>  include/linux/virtio_rpmsg.h |  83 +++
>  include/uapi/linux/rpmsg.h   |   3 +
>  include/uapi/linux/vhost.h   |   4 +-
>  9 files changed, 551 insertions(+), 80 deletions(-)
>  create mode 100644 drivers/vhost/rpmsg.c
>  create mode 100644 drivers/vhost/vhost_rpmsg.h
>  create mode 100644 include/linux/virtio_rpmsg.h
> 
> -- 
> 2.28.0

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


Re: [PATCH] Rescan the entire target on transport reset when LUN is 0

2020-09-08 Thread Michael S. Tsirkin
On Fri, Aug 28, 2020 at 12:21:35PM +, Matej Genci wrote:
> VirtIO 1.0 spec says
> The removed and rescan events ... when sent for LUN 0, they MAY
> apply to the entire target so the driver can ask the initiator
> to rescan the target to detect this.
> 
> This change introduces the behaviour described above by scanning the
> entire scsi target when LUN is set to 0. This is both a functional and a
> performance fix. It aligns the driver with the spec and allows control
> planes to hotplug targets with large numbers of LUNs without having to
> request a RESCAN for each one of them.
> 
> Signed-off-by: Matej Genci 
> Suggested-by: Felipe Franciosi 

Stefan, Paolo, could you review this pls?

> ---
>  drivers/scsi/virtio_scsi.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index bfec84aacd90..a4b9bc7b4b4a 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -284,7 +284,12 @@ static void virtscsi_handle_transport_reset(struct 
> virtio_scsi *vscsi,
>  
>   switch (virtio32_to_cpu(vscsi->vdev, event->reason)) {
>   case VIRTIO_SCSI_EVT_RESET_RESCAN:
> - scsi_add_device(shost, 0, target, lun);
> + if (lun == 0) {
> + scsi_scan_target(>shost_gendev, 0, target,
> +  SCAN_WILD_CARD, SCSI_SCAN_INITIAL);
> + } else {
> + scsi_add_device(shost, 0, target, lun);
> + }
>   break;
>   case VIRTIO_SCSI_EVT_RESET_REMOVED:
>   sdev = scsi_device_lookup(shost, 0, target, lun);
> -- 
> 2.20.1

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


Re: [PATCH v3] virtio-rng: return available data with O_NONBLOCK

2020-09-08 Thread Michael S. Tsirkin
On Mon, Aug 31, 2020 at 02:37:26PM +0200, Laurent Vivier wrote:
> On 28/08/2020 23:34, Martin Wilck wrote:
> > On Wed, 2020-08-26 at 08:26 -0400, Michael S. Tsirkin wrote:
> >> On Tue, Aug 11, 2020 at 04:42:32PM +0200, Laurent Vivier wrote:
> >>> On 11/08/2020 16:28, mwi...@suse.com wrote:
> >>>> From: Martin Wilck 
> >>>>
> >>>> If a program opens /dev/hwrng with O_NONBLOCK and uses poll() and
> >>>> non-blocking read() to retrieve random data, it ends up in a
> >>>> tight
> >>>> loop with poll() always returning POLLIN and read() returning
> >>>> EAGAIN.
> >>>> This repeats forever until some process makes a blocking read()
> >>>> call.
> >>>> The reason is that virtio_read() always returns 0 in non-blocking 
> >>>> mode,
> >>>> even if data is available. Worse, it fetches random data from the
> >>>> hypervisor after every non-blocking call, without ever using this
> >>>> data.
> >>>>
> >>>> The following test program illustrates the behavior and can be
> >>>> used
> >>>> for testing and experiments. The problem will only be seen if all
> >>>> tasks use non-blocking access; otherwise the blocking reads will
> >>>> "recharge" the random pool and cause other, non-blocking reads to
> >>>> succeed at least sometimes.
> >>>>
> >>>> /* Whether to use non-blocking mode in a task, problem occurs if
> >>>> CONDITION is 1 */
> >>>> //#define CONDITION (getpid() % 2 != 0)
> >>>>
> >>>> static volatile sig_atomic_t stop;
> >>>> static void handler(int sig __attribute__((unused))) { stop = 1;
> >>>> }
> >>>>
> >>>> static void loop(int fd, int sec)
> >>>> {
> >>>>  struct pollfd pfd = { .fd = fd, .events  = POLLIN, };
> >>>>  unsigned long errors = 0, eagains = 0, bytes = 0, succ = 0;
> >>>>  int size, rc, rd;
> >>>>
> >>>>  srandom(getpid());
> >>>>  if (CONDITION && fcntl(fd, F_SETFL, fcntl(fd, F_GETFL) |
> >>>> O_NONBLOCK) == -1)
> >>>>  perror("fcntl");
> >>>>  size = MINBUFSIZ + random() % (MAXBUFSIZ - MINBUFSIZ + 1);
> >>>>
> >>>>  for(;;) {
> >>>>  char buf[size];
> >>>>
> >>>>  if (stop)
> >>>>  break;
> >>>>  rc = poll(, 1, sec);
> >>>>  if (rc > 0) {
> >>>>  rd = read(fd, buf, sizeof(buf));
> >>>>  if (rd == -1 && errno == EAGAIN)
> >>>>  eagains++;
> >>>>  else if (rd == -1)
> >>>>  errors++;
> >>>>  else {
> >>>>  succ++;
> >>>>  bytes += rd;
> >>>>  write(1, buf, sizeof(buf));
> >>>>  }
> >>>>  } else if (rc == -1) {
> >>>>  if (errno != EINTR)
> >>>>  perror("poll");
> >>>>  break;
> >>>>  } else
> >>>>  fprintf(stderr, "poll: timeout\n");
> >>>>  }
> >>>>  fprintf(stderr,
> >>>>  "pid %d %sblocking, bufsize %d, %d seconds, %lu bytes
> >>>> read, %lu success, %lu eagain, %lu errors\n",
> >>>>  getpid(), CONDITION ? "non-" : "", size, sec, bytes,
> >>>> succ, eagains, errors);
> >>>> }
> >>>>
> >>>> int main(void)
> >>>> {
> >>>>  int fd;
> >>>>
> >>>>  fork(); fork();
> >>>>  fd = open("/dev/hwrng", O_RDONLY);
> >>>>  if (fd == -1) {
> >>>>  perror("open");
> >>>>  return 1;
> >>>>  };
> >>>>  signal(SIGALRM, handler);
> >>>>  alarm(SECONDS);
> >>>>  loop(fd, SECONDS);
> >>>>  close(fd);
> >>>>  wait(NULL);
> >>>>  return 0;
> >>>> }
> >>>>
> >>&

Re: [PATCH 1/2] vhost: remove mutex ops in vhost_set_backend_features

2020-09-08 Thread Michael S. Tsirkin
On Mon, Sep 07, 2020 at 06:52:19PM +0800, Zhu Lingshan wrote:
> In vhost_vdpa ioctl SET_BACKEND_FEATURES path, currect code
> would try to acquire vhost dev mutex twice
> (first shown in vhost_vdpa_unlocked_ioctl), which can lead
> to a dead lock issue.
> This commit removed mutex operations in vhost_set_backend_features.
> As a compensation for vhost_net, a followinig commit will add
> needed mutex lock/unlock operations in a new function
> vhost_net_set_backend_features() which is a wrap of
> vhost_set_backend_features().
> 
> Signed-off-by: Zhu Lingshan 

I think you need to squash these two or reorder, we can't first
make code racy then fix it up.

> ---
>  drivers/vhost/vhost.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index b45519ca66a7..e03c9e6f058f 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2591,14 +2591,12 @@ void vhost_set_backend_features(struct vhost_dev 
> *dev, u64 features)
>   struct vhost_virtqueue *vq;
>   int i;
>  
> - mutex_lock(>mutex);
>   for (i = 0; i < dev->nvqs; ++i) {
>   vq = dev->vqs[i];
>   mutex_lock(>mutex);
>   vq->acked_backend_features = features;
>   mutex_unlock(>mutex);
>   }
> - mutex_unlock(>mutex);
>  }
>  EXPORT_SYMBOL_GPL(vhost_set_backend_features);
>  
> -- 
> 2.18.4

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


Re: [PATCH] vdpa/mlx5: Setup driver only if VIRTIO_CONFIG_S_DRIVER_OK

2020-09-08 Thread Michael S. Tsirkin
On Mon, Sep 07, 2020 at 02:43:51PM +0300, Eli Cohen wrote:
> On Mon, Sep 07, 2020 at 07:34:00AM -0400, Michael S. Tsirkin wrote:
> > On Mon, Sep 07, 2020 at 10:51:36AM +0300, Eli Cohen wrote:
> > > If the memory map changes before the driver status is
> > > VIRTIO_CONFIG_S_DRIVER_OK, don't attempt to create resources because it
> > > may fail. For example, if the VQ is not ready there is no point in
> > > creating resources.
> > > 
> > > Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 
> > > devices")
> > > Signed-off-by: Eli Cohen 
> > 
> > 
> > Could you add a bit more data about the problem to the log?
> > To be more exact, what exactly happens right now?
> >
> 
> Sure I can.
> 
> set_map() is used by mlx5 vdpa to create a memory region based on the
> address map passed by the iotlb argument. If I get successive calls, I
> will destroy the current memory region and build another one based on
> the new address mapping. I also need to setup the hardware resources
> since they depend on the memory region.
> 
> If these calls happen before DRIVER_OK, It means it that driver VQs may
> also not been setup and I may not create them yet. In this case I want
> to avoid setting up the other resources and defer this till I get DRIVER
> OK.
> 
> Let me know if that answers your question so I can post another patch.

it does, pls do.

> > > ---
> > >  drivers/vdpa/mlx5/net/mlx5_vnet.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
> > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > index 9df69d5efe8c..c89cd48a0aab 100644
> > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > @@ -1645,6 +1645,9 @@ static int mlx5_vdpa_change_map(struct 
> > > mlx5_vdpa_net *ndev, struct vhost_iotlb *
> > >   if (err)
> > >   goto err_mr;
> > >  
> > > + if (!(ndev->mvdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
> > > + return 0;
> > > +
> > >   restore_channels_info(ndev);
> > >   err = setup_driver(ndev);
> > >   if (err)
> > > -- 
> > > 2.26.0
> > 

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


Re: [PATCH v11 0/2] s390: virtio: let arch validate VIRTIO features

2020-09-08 Thread Michael S. Tsirkin
On Tue, Sep 08, 2020 at 08:55:21AM +0200, Cornelia Huck wrote:
> On Tue, 8 Sep 2020 00:39:51 +0200
> Halil Pasic  wrote:
> 
> > On Mon,  7 Sep 2020 11:39:05 +0200
> > Pierre Morel  wrote:
> > 
> > > Hi all,
> > > 
> > > The goal of the series is to give a chance to the architecture
> > > to validate VIRTIO device features.  
> > 
> > Michael, is this going in via your tree?
> > 
> 
> I believe Michael's tree is the right place for this, but I can also
> queue it if I get an ack on patch 1.

I think Halil pointed out some minor issues, so a new version is in
order.

-- 
MST

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


Re: [PATCH v11 0/2] s390: virtio: let arch validate VIRTIO features

2020-09-08 Thread Michael S. Tsirkin
On Tue, Sep 08, 2020 at 12:39:51AM +0200, Halil Pasic wrote:
> On Mon,  7 Sep 2020 11:39:05 +0200
> Pierre Morel  wrote:
> 
> > Hi all,
> > 
> > The goal of the series is to give a chance to the architecture
> > to validate VIRTIO device features.
> 
> Michael, is this going in via your tree?

I guess so. Still not really happy about second-guessing
the hypervisor, but this got acks from others
so maybe I'm wrong in this instance. Won't be the first time.

-- 
MST

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


Re: [PATCH] vdpa/mlx5: Setup driver only if VIRTIO_CONFIG_S_DRIVER_OK

2020-09-07 Thread Michael S. Tsirkin
On Mon, Sep 07, 2020 at 10:51:36AM +0300, Eli Cohen wrote:
> If the memory map changes before the driver status is
> VIRTIO_CONFIG_S_DRIVER_OK, don't attempt to create resources because it
> may fail. For example, if the VQ is not ready there is no point in
> creating resources.
> 
> Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
> Signed-off-by: Eli Cohen 


Could you add a bit more data about the problem to the log?
To be more exact, what exactly happens right now?

> ---
>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 9df69d5efe8c..c89cd48a0aab 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1645,6 +1645,9 @@ static int mlx5_vdpa_change_map(struct mlx5_vdpa_net 
> *ndev, struct vhost_iotlb *
>   if (err)
>   goto err_mr;
>  
> + if (!(ndev->mvdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
> + return 0;
> +
>   restore_channels_info(ndev);
>   err = setup_driver(ndev);
>   if (err)
> -- 
> 2.26.0

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


Re: [PATCH] i2c: virtio: add a virtio i2c frontend driver

2020-09-03 Thread Michael S. Tsirkin
On Thu, Sep 03, 2020 at 01:34:45PM +0800, Jie Deng wrote:
> Add an I2C bus driver for virtio para-virtualization.
> 
> The controller can be emulated by the backend driver in
> any device model software by following the virtio protocol.
> 
> This driver communicates with the backend driver through a
> virtio I2C message structure which includes following parts:
> 
> - Header: i2c_msg addr, flags, len.
> - Data buffer: the pointer to the i2c msg data.
> - Status: the processing result from the backend.
> 
> People may implement different backend drivers to emulate
> different controllers according to their needs. A backend
> example can be found in the device model of the open source
> project ACRN. For more information, please refer to
> https://projectacrn.org.
> 
> The virtio device ID 34 is used for this I2C adpter since IDs
> before 34 have been reserved by other virtio devices.

Please reserve the ID with the virtio tc so no one conflicts.


> Co-developed-by: Conghui Chen 
> Signed-off-by: Conghui Chen 
> Signed-off-by: Jie Deng 
> Reviewed-by: Shuo Liu 
> Reviewed-by: Andy Shevchenko 
> ---
>  drivers/i2c/busses/Kconfig  |  11 ++
>  drivers/i2c/busses/Makefile |   3 +
>  drivers/i2c/busses/i2c-virtio.c | 276 
> 
>  include/uapi/linux/virtio_ids.h |   1 +
>  4 files changed, 291 insertions(+)
>  create mode 100644 drivers/i2c/busses/i2c-virtio.c
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 293e7a0..70c8e30 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -21,6 +21,17 @@ config I2C_ALI1535
> This driver can also be built as a module.  If so, the module
> will be called i2c-ali1535.
>  
> +config I2C_VIRTIO
> + tristate "Virtio I2C Adapter"
> + depends on VIRTIO
> + help
> +   If you say yes to this option, support will be included for the virtio
> +   i2c adapter driver. The hardware can be emulated by any device model
> +   software according to the virtio protocol.
> +
> +   This driver can also be built as a module. If so, the module
> +   will be called i2c-virtio.
> +
>  config I2C_ALI1563
>   tristate "ALI 1563"
>   depends on PCI
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 19aff0e..821acfa 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -6,6 +6,9 @@
>  # ACPI drivers
>  obj-$(CONFIG_I2C_SCMI)   += i2c-scmi.o
>  
> +# VIRTIO I2C host controller driver
> +obj-$(CONFIG_I2C_VIRTIO) += i2c-virtio.o
> +
>  # PC SMBus host controller drivers
>  obj-$(CONFIG_I2C_ALI1535)+= i2c-ali1535.o
>  obj-$(CONFIG_I2C_ALI1563)+= i2c-ali1563.o
> diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> new file mode 100644
> index 000..47f9fd1
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-virtio.c
> @@ -0,0 +1,276 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Virtio I2C Bus Driver
> + *
> + * Copyright (c) 2020 Intel Corporation. All rights reserved.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#define VIRTIO_I2C_MSG_OK0
> +#define VIRTIO_I2C_MSG_ERR   1
> +
> +/**
> + * struct virtio_i2c_hdr - the virtio I2C message header structure
> + * @addr: i2c_msg addr, the slave address
> + * @flags: i2c_msg flags
> + * @len: i2c_msg len
> + */
> +struct virtio_i2c_hdr {
> + __virtio16 addr;
> + __virtio16 flags;
> + __virtio16 len;
> +} __packed;

virtio16 is for legacy devices, modern ones should be __le.
and  we don't really need to pack it I think.

> +
> +/**
> + * struct virtio_i2c_msg - the virtio I2C message structure
> + * @hdr: the virtio I2C message header
> + * @buf: virtio I2C message data buffer
> + * @status: the processing result from the backend
> + */
> +struct virtio_i2c_msg {
> + struct virtio_i2c_hdr hdr;
> + char *buf;
> + u8 status;
> +};
> +
> +/**
> + * struct virtio_i2c - virtio I2C data
> + * @vdev: virtio device for this controller
> + * @completion: completion of virtio I2C message
> + * @adap: I2C adapter for this controller
> + * @i2c_lock: lock for virtqueue processing
> + * @vq: the virtio virtqueue for communication
> + */
> +struct virtio_i2c {
> + struct virtio_device *vdev;
> + struct completion completion;
> + struct i2c_adapter adap;
> + struct mutex i2c_lock;
> + struct virtqueue *vq;
> +};
> +
> +static void virtio_i2c_msg_done(struct virtqueue *vq)
> +{
> + struct virtio_i2c *vi = vq->vdev->priv;
> +
> + complete(>completion);
> +}
> +
> +static int virtio_i2c_add_msg(struct virtqueue *vq,
> +   struct virtio_i2c_msg *vmsg,
> +   struct i2c_msg *msg)
> +{
> + struct scatterlist *sgs[3], hdr, bout, bin, status;
> + int 

Re: [PATCH net-next] vhost: fix typo in error message

2020-09-01 Thread Michael S. Tsirkin
On Tue, Sep 01, 2020 at 10:39:09AM +0800, Yunsheng Lin wrote:
> "enable" should be "disable" when the function name is
> vhost_disable_notify(), which does the disabling work.
> 
> Signed-off-by: Yunsheng Lin 

Acked-by: Michael S. Tsirkin 

Why net-next though? It's a bugfix, can go into net.


> ---
>  drivers/vhost/vhost.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 5857d4e..b45519c 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2537,7 +2537,7 @@ void vhost_disable_notify(struct vhost_dev *dev, struct 
> vhost_virtqueue *vq)
>   if (!vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX)) {
>   r = vhost_update_used_flags(vq);
>   if (r)
> - vq_err(vq, "Failed to enable notification at %p: %d\n",
> + vq_err(vq, "Failed to disable notification at %p: %d\n",
>  >used->flags, r);
>   }
>  }
> -- 
> 2.8.1

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


[GIT PULL] virtio: bugfixes

2020-08-26 Thread Michael S. Tsirkin
The following changes since commit d012a7190fc1fd72ed48911e77ca97ba4521bccd:

  Linux 5.9-rc2 (2020-08-23 14:08:43 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

for you to fetch changes up to cbb523594eb718944b726ba52bb43a1d66188a17:

  vdpa/mlx5: Avoid warnings about shifts on 32-bit platforms (2020-08-26 
08:13:59 -0400)


virtio: bugfixes

A couple vdpa and vhost bugfixes

Signed-off-by: Michael S. Tsirkin 


Jason Wang (2):
  vdpa: ifcvf: return err when fail to request config irq
  vdpa: ifcvf: free config irq in ifcvf_free_irq()

Nathan Chancellor (1):
  vdpa/mlx5: Avoid warnings about shifts on 32-bit platforms

Stefano Garzarella (1):
  vhost-iotlb: fix vhost_iotlb_itree_next() documentation

 drivers/vdpa/ifcvf/ifcvf_base.h   |  2 +-
 drivers/vdpa/ifcvf/ifcvf_main.c   |  9 +--
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 50 +++
 drivers/vhost/iotlb.c |  4 ++--
 4 files changed, 35 insertions(+), 30 deletions(-)

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


Re: [PATCH v3 0/6] Add virtio-iommu built-in topology

2020-08-26 Thread Michael S. Tsirkin
On Fri, Aug 21, 2020 at 03:15:34PM +0200, Jean-Philippe Brucker wrote:
> Add a topology description to the virtio-iommu driver and enable x86
> platforms.
> 
> Since [v2] we have made some progress on adding ACPI support for
> virtio-iommu, which is the preferred boot method on x86. It will be a
> new vendor-agnostic table describing para-virtual topologies in a
> minimal format. However some platforms don't use either ACPI or DT for
> booting (for example microvm), and will need the alternative topology
> description method proposed here. In addition, since the process to get
> a new ACPI table will take a long time, this provides a boot method even
> to ACPI-based platforms, if only temporarily for testing and
> development.

OK should I park this in next now? Seems appropriate ...

> v3:
> * Add patch 1 that moves virtio-iommu to a subfolder.
> * Split the rest:
>   * Patch 2 adds topology-helper.c, which will be shared with the ACPI
> support.
>   * Patch 4 adds definitions.
>   * Patch 5 adds parser in topology.c.
> * Address other comments.
> 
> Linux and QEMU patches available at:
> https://jpbrucker.net/git/linux virtio-iommu/devel
> https://jpbrucker.net/git/qemu virtio-iommu/devel
> 
> [spec] https://lists.oasis-open.org/archives/virtio-dev/202008/msg00067.html
> [v2] 
> https://lore.kernel.org/linux-iommu/20200228172537.377327-1-jean-phili...@linaro.org/
> [v1] 
> https://lore.kernel.org/linux-iommu/20200214160413.1475396-1-jean-phili...@linaro.org/
> [rfc] 
> https://lore.kernel.org/linux-iommu/20191122105000.800410-1-jean-phili...@linaro.org/
> 
> Jean-Philippe Brucker (6):
>   iommu/virtio: Move to drivers/iommu/virtio/
>   iommu/virtio: Add topology helpers
>   PCI: Add DMA configuration for virtual platforms
>   iommu/virtio: Add topology definitions
>   iommu/virtio: Support topology description in config space
>   iommu/virtio: Enable x86 support
> 
>  drivers/iommu/Kconfig |  18 +-
>  drivers/iommu/Makefile|   3 +-
>  drivers/iommu/virtio/Makefile |   4 +
>  drivers/iommu/virtio/topology-helpers.h   |  50 +
>  include/linux/virt_iommu.h|  15 ++
>  include/uapi/linux/virtio_iommu.h |  44 
>  drivers/iommu/virtio/topology-helpers.c   | 196 
>  drivers/iommu/virtio/topology.c   | 259 ++
>  drivers/iommu/{ => virtio}/virtio-iommu.c |   4 +
>  drivers/pci/pci-driver.c  |   5 +
>  MAINTAINERS   |   3 +-
>  11 files changed, 597 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/iommu/virtio/Makefile
>  create mode 100644 drivers/iommu/virtio/topology-helpers.h
>  create mode 100644 include/linux/virt_iommu.h
>  create mode 100644 drivers/iommu/virtio/topology-helpers.c
>  create mode 100644 drivers/iommu/virtio/topology.c
>  rename drivers/iommu/{ => virtio}/virtio-iommu.c (99%)
> 
> -- 
> 2.28.0
> 
> ___
> iommu mailing list
> io...@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

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


Re: [PATCH v3] virtio-rng: return available data with O_NONBLOCK

2020-08-26 Thread Michael S. Tsirkin
On Tue, Aug 11, 2020 at 04:42:32PM +0200, Laurent Vivier wrote:
> On 11/08/2020 16:28, mwi...@suse.com wrote:
> > From: Martin Wilck 
> > 
> > If a program opens /dev/hwrng with O_NONBLOCK and uses poll() and
> > non-blocking read() to retrieve random data, it ends up in a tight
> > loop with poll() always returning POLLIN and read() returning EAGAIN.
> > This repeats forever until some process makes a blocking read() call.
> > The reason is that virtio_read() always returns 0 in non-blocking mode,
> > even if data is available. Worse, it fetches random data from the
> > hypervisor after every non-blocking call, without ever using this data.
> > 
> > The following test program illustrates the behavior and can be used
> > for testing and experiments. The problem will only be seen if all
> > tasks use non-blocking access; otherwise the blocking reads will
> > "recharge" the random pool and cause other, non-blocking reads to
> > succeed at least sometimes.
> > 
> > /* Whether to use non-blocking mode in a task, problem occurs if CONDITION 
> > is 1 */
> > //#define CONDITION (getpid() % 2 != 0)
> > 
> > static volatile sig_atomic_t stop;
> > static void handler(int sig __attribute__((unused))) { stop = 1; }
> > 
> > static void loop(int fd, int sec)
> > {
> > struct pollfd pfd = { .fd = fd, .events  = POLLIN, };
> > unsigned long errors = 0, eagains = 0, bytes = 0, succ = 0;
> > int size, rc, rd;
> > 
> > srandom(getpid());
> > if (CONDITION && fcntl(fd, F_SETFL, fcntl(fd, F_GETFL) | O_NONBLOCK) == 
> > -1)
> > perror("fcntl");
> > size = MINBUFSIZ + random() % (MAXBUFSIZ - MINBUFSIZ + 1);
> > 
> > for(;;) {
> > char buf[size];
> > 
> > if (stop)
> > break;
> > rc = poll(, 1, sec);
> > if (rc > 0) {
> > rd = read(fd, buf, sizeof(buf));
> > if (rd == -1 && errno == EAGAIN)
> > eagains++;
> > else if (rd == -1)
> > errors++;
> > else {
> > succ++;
> > bytes += rd;
> > write(1, buf, sizeof(buf));
> > }
> > } else if (rc == -1) {
> > if (errno != EINTR)
> > perror("poll");
> > break;
> > } else
> > fprintf(stderr, "poll: timeout\n");
> > }
> > fprintf(stderr,
> > "pid %d %sblocking, bufsize %d, %d seconds, %lu bytes read, %lu 
> > success, %lu eagain, %lu errors\n",
> > getpid(), CONDITION ? "non-" : "", size, sec, bytes, succ, 
> > eagains, errors);
> > }
> > 
> > int main(void)
> > {
> > int fd;
> > 
> > fork(); fork();
> > fd = open("/dev/hwrng", O_RDONLY);
> > if (fd == -1) {
> > perror("open");
> > return 1;
> > };
> > signal(SIGALRM, handler);
> > alarm(SECONDS);
> > loop(fd, SECONDS);
> > close(fd);
> > wait(NULL);
> > return 0;
> > }
> > 
> > void loop(int fd)
> > {
> > struct pollfd pfd0 = { .fd = fd, .events  = POLLIN, };
> > int rc;
> > unsigned int n;
> > 
> > for (n = LOOPS; n > 0; n--) {
> > struct pollfd pfd = pfd0;
> > char buf[SIZE];
> > 
> > rc = poll(, 1, 1);
> > if (rc > 0) {
> > int rd = read(fd, buf, sizeof(buf));
> > 
> > if (rd == -1)
> > perror("read");
> > else
> > printf("read %d bytes\n", rd);
> > } else if (rc == -1)
> > perror("poll");
> > else
> > fprintf(stderr, "timeout\n");
> > 
> > }
> > }
> > 
> > int main(void)
> > {
> > int fd;
> > 
> > fd = open("/dev/hwrng", O_RDONLY|O_NONBLOCK);
> > if (fd == -1) {
> > perror("open");
> > return 1;
> > };
> > loop(fd);
> > close(fd);
> > return 0;
> > }
> > 
> > This can be observed in the real word e.g. with nested qemu/KVM virtual
> > machines, if both the "outer" and "inner" VMs have a virtio-rng device.
> > If the "inner" VM requests random data, qemu running in the "outer" VM
> > uses this device in a non-blocking manner like the test program above.
> > 
> > Fix it by returning available data if a previous hypervisor call has
> > completed. I tested this patch with the program above, and with rng-tools.
> > 
> > v2 -> v3: Simplified the implementation as suggested by Laurent Vivier
> > 
> > Signed-off-by: Martin Wilck 
> > ---
> >  drivers/char/hw_random/virtio-rng.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/char/hw_random/virtio-rng.c 

Re: [PATCH 1/2] vdpa: ifcvf: return err when fail to request config irq

2020-08-26 Thread Michael S. Tsirkin
On Fri, Aug 07, 2020 at 11:52:09AM +0800, Jason Wang wrote:
> 
> On 2020/7/23 下午5:12, Jason Wang wrote:
> > We ignore the err of requesting config interrupt, fix this.
> > 
> > Fixes: e7991f376a4d ("ifcvf: implement config interrupt in IFCVF")
> > Cc: Zhu Lingshan 
> > Signed-off-by: Jason Wang 
> > ---
> >   drivers/vdpa/ifcvf/ifcvf_main.c | 4 
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 
> > b/drivers/vdpa/ifcvf/ifcvf_main.c
> > index f5a60c14b979..ae7110955a44 100644
> > --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> > @@ -76,6 +76,10 @@ static int ifcvf_request_irq(struct ifcvf_adapter 
> > *adapter)
> > ret = devm_request_irq(>dev, irq,
> >ifcvf_config_changed, 0,
> >vf->config_msix_name, vf);
> > +   if (ret) {
> > +   IFCVF_ERR(pdev, "Failed to request config irq\n");
> > +   return ret;
> > +   }
> > for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) {
> > snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n",
> 
> 
> Hi Michael:
> 
> Any comments on this series?
> 
> Thanks
> 

Applied, thanks!

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

Re: [PATCH v4 4/4] vhost: add an RPMsg API

2020-08-25 Thread Michael S. Tsirkin
On Tue, Aug 25, 2020 at 03:41:10PM +0200, Guennadi Liakhovetski wrote:
> Hi Michael,
> 
> ...back from holidays and still unsure what your preferred solution 
> for the message layout would be:
> 
> On Wed, Aug 12, 2020 at 02:32:43PM +0200, Guennadi Liakhovetski wrote:
> > Hi Michael,
> > 
> > Thanks for a review.
> > 
> > On Mon, Aug 10, 2020 at 09:44:15AM -0400, Michael S. Tsirkin wrote:
> > > On Tue, Aug 04, 2020 at 05:19:17PM +0200, Guennadi Liakhovetski wrote:
> 
> [snip]
> 
> > > > > > +static int vhost_rpmsg_get_single(struct vhost_virtqueue *vq)
> > > > > > +{
> > > > > > +   struct vhost_rpmsg *vr = container_of(vq->dev, struct 
> > > > > > vhost_rpmsg, dev);
> > > > > > +   unsigned int out, in;
> > > > > > +   int head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov), 
> > > > > > , ,
> > > > > > +NULL, NULL);
> > > > > > +   if (head < 0) {
> > > > > > +   vq_err(vq, "%s(): error %d getting buffer\n",
> > > > > > +  __func__, head);
> > > > > > +   return head;
> > > > > > +   }
> > > > > > +
> > > > > > +   /* Nothing new? */
> > > > > > +   if (head == vq->num)
> > > > > > +   return head;
> > > > > > +
> > > > > > +   if (vq == >vq[VIRTIO_RPMSG_RESPONSE] && (out || in != 1)) {
> > > > > 
> > > > > This in != 1 looks like a dependency on a specific message layout.
> > > > > virtio spec says to avoid these. Using iov iters it's not too hard to 
> > > > > do
> > > > > ...
> > > > 
> > > > This is an RPMsg VirtIO implementation, and it has to match the 
> > > > virtio_rpmsg_bus.c 
> > > > driver, and that one has specific VirtIO queue and message usage 
> > > > patterns.
> > > 
> > > That could be fine for legacy virtio, but now you are claiming support
> > > for virtio 1, so need to fix these assumptions in the device.
> > 
> > I can just deop these checks without changing anything else, that still 
> > would work. 
> > I could also make this work with "any" layout - either ignoring any 
> > left-over 
> > buffers or maybe even getting them one by one. But I wouldn't even be able 
> > to test 
> > those modes without modifying / breaking the current virtio-rpmsg driver. 
> > What's 
> > the preferred solution?
> 
> Could you elaborate a bit please?
> 
> Thanks
> Guennadi

I'd prefer it that we make it work with any layout.
For testing, there was a hack for virtio ring which
split up descriptors at a random boundary.
I'll try to locate it and post, sounds like something
we might want upstream for debugging.

-- 
MST

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


Re: [PATCH v2] virtio-rng: return available data with O_NONBLOCK

2020-08-11 Thread Michael S. Tsirkin
On Tue, Aug 11, 2020 at 03:53:54PM +0200, Laurent Vivier wrote:
> On 11/08/2020 15:14, Michael S. Tsirkin wrote:
> > On Tue, Aug 11, 2020 at 03:00:14PM +0200, Laurent Vivier wrote:
> >> No problem. This code is tricky and it took me several months to really
> >> start to understand it ...
> > 
> > Oh great, we actually have someone who understands the code!
> > Maybe you can help me understand: virtio_read
> > takes the buf pointer and puts it in the vq.
> > It can then return to caller (e.g. on a signal).
> > Device can meanwhile write into the buffer.
> > 
> > It looks like if another call then happens, and that
> > other call uses a different buffer, virtio rng
> > will happily return the data written into the
> > original buf pointer, confusing the caller.
> > 
> > Is that right?
> > 
> 
> Yes.
> 
> hw_random core uses two bufers:
> 
> - rng_fillbuf that is used with a blocking access and protected by the
> reading_mutex. I think this cannot be interrupted by a kill because it's
> in  hwrng_fillfn() and it's kthread.
> 
> - rng_buffer that is used in rng_dev_read() and can be interrupted (it
> is also protected by reading_mutex)
> 
> But if rng_dev_read() is called with O_NONBLOCK or interrupted and then
> rng_fillbuf starts they can be mixed.
> 
> We have also the first use of rng_buffer in add_early_randomness() that
> use a different size than in rng_dev_read() with the same buffer (and
> this size is 16 whereas the hwrng read API says it must be at least 32...).
> 
> The problem here is core has been developped with synchronicity in mind,
> whereas virtio is asynchronous by definition.
> 
> I think we should add some internal buffers in virtio-rng backend. This
> would improve performance (we are at 1 MB/s, I sent a patch to improve
> that, but this doesn't fix the problems above), and allows hw_random
> core to use memory that doesn't need to be compatible with virt_to_page().
> 
> Thanks,
> Laurent

OK so just add a bunch of 32 bit buffers and pass them to hardware,
as they data gets consumed pass them to hardware again?


-- 
MST

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


Re: [PATCH v2] virtio-rng: return available data with O_NONBLOCK

2020-08-11 Thread Michael S. Tsirkin
On Tue, Aug 11, 2020 at 03:00:14PM +0200, Laurent Vivier wrote:
> No problem. This code is tricky and it took me several months to really
> start to understand it ...

Oh great, we actually have someone who understands the code!
Maybe you can help me understand: virtio_read
takes the buf pointer and puts it in the vq.
It can then return to caller (e.g. on a signal).
Device can meanwhile write into the buffer.

It looks like if another call then happens, and that
other call uses a different buffer, virtio rng
will happily return the data written into the
original buf pointer, confusing the caller.

Is that right?

-- 
MST

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


Re: [PATCH v2] virtio-rng: return available data with O_NONBLOCK

2020-08-11 Thread Michael S. Tsirkin
On Tue, Aug 11, 2020 at 02:07:23PM +0200, Martin Wilck wrote:
> On Tue, 2020-08-11 at 07:26 -0400, Michael S. Tsirkin wrote:
> > On Wed, Jul 15, 2020 at 03:32:55PM +0200, mwi...@suse.com wrote:
> > >  drivers/char/hw_random/virtio-rng.c | 14 ++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/drivers/char/hw_random/virtio-rng.c
> > > b/drivers/char/hw_random/virtio-rng.c
> > > index 79a6e47b5fbc..984713b35892 100644
> > > --- a/drivers/char/hw_random/virtio-rng.c
> > > +++ b/drivers/char/hw_random/virtio-rng.c
> > > @@ -59,6 +59,20 @@ static int virtio_read(struct hwrng *rng, void
> > > *buf, size_t size, bool wait)
> > >   if (vi->hwrng_removed)
> > >   return -ENODEV;
> > >  
> > > + /*
> > > +  * If the previous call was non-blocking, we may have got some
> > > +  * randomness already.
> > > +  */
> > > + if (vi->busy && completion_done(>have_data)) {
> > > + unsigned int len;
> > > +
> > > + vi->busy = false;
> > > + len = vi->data_avail > size ? size : vi->data_avail;
> > > + vi->data_avail -= len;
> > 
> > I wonder what purpose does this line serve: busy is false
> > which basically means data_avail is invalid, right?
> > A following non blocking call will not enter here.
> 
> Well, I thought this is just how reading data normally works. But
> you're right, the remainder will not be used. I can remove the line, or
> reset data_avail to 0 at this point. What do you prefer?
> 
> Regards,
> Martin

Removing seems cleaner.

But looking at it, it is using the API in a very strange way:
a buffer is placed in the ring by one call, and *assumed*
to still be there in the next call.

which it might not be if one call is from userspace and the
next one is from fill kthread.

I guess this is why it's returning 0: yes it knows there's
data, but it does not know where it is.

-- 
MST

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


Re: VDPA Debug/Statistics

2020-08-11 Thread Michael S. Tsirkin
On Tue, Aug 11, 2020 at 11:58:23AM +, Eli Cohen wrote:
> On Tue, Aug 11, 2020 at 11:26:20AM +, Eli Cohen wrote:
> > Hi All
> > 
> > Currently, the only statistics we get for a VDPA instance comes from the 
> > virtio_net device instance. Since VDPA involves hardware acceleration, 
> > there can be quite a lot of information that can be fetched from the 
> > underlying device. Currently there is no generic method to fetch this 
> > information.
> > 
> > One way of doing this can be to create a the host, a net device for 
> > each VDPA instance, and use it to get this information or do some 
> > configuration. Ethtool can be used in such a case
> > 
> > I would like to hear what you think about this or maybe you have some other 
> > ideas to address this topic.
> > 
> > Thanks,
> > Eli
> 
> Something I'm not sure I understand is how are vdpa instances created on 
> mellanox cards? There's a devlink command for that, is that right?
> Can that be extended for stats?
> 
> Currently any VF will be probed as VDPA device. We're adding devlink support 
> but I am not sure if devlink is suitable for displaying statistics. We will 
> discuss internally but I wanted to know why you guys think.

OK still things like specifying the mac are managed through rtnetlink,
right?

Right now it does not look like you can mix stats and vf, they are
handled separately:

if (rtnl_fill_stats(skb, dev))
goto nla_put_failure;

if (rtnl_fill_vf(skb, dev, ext_filter_mask))
goto nla_put_failure;

but ability to query vf stats on the host sounds useful generally.

As another option, we could use a vdpa specific way to retrieve stats,
and teach qemu to report them.




> --
> MST

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


Re: VDPA Debug/Statistics

2020-08-11 Thread Michael S. Tsirkin
On Tue, Aug 11, 2020 at 11:26:20AM +, Eli Cohen wrote:
> Hi All
> 
> Currently, the only statistics we get for a VDPA instance comes from the 
> virtio_net device instance. Since VDPA involves hardware acceleration, there 
> can be quite a lot of information that can be fetched from the underlying 
> device. Currently there is no generic method to fetch this information.
> 
> One way of doing this can be to create a the host, a net device for each VDPA 
> instance, and use it to get this information or do some configuration. 
> Ethtool can be used in such a case
> 
> I would like to hear what you think about this or maybe you have some other 
> ideas to address this topic.
> 
> Thanks,
> Eli

Something I'm not sure I understand is how are vdpa instances created
on mellanox cards? There's a devlink command for that, is that right?
Can that be extended for stats?

-- 
MST

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


Re: [PATCH v2] virtio-rng: return available data with O_NONBLOCK

2020-08-11 Thread Michael S. Tsirkin
On Wed, Jul 15, 2020 at 03:32:55PM +0200, mwi...@suse.com wrote:
> From: Martin Wilck 
> 
> If a program opens /dev/hwrng with O_NONBLOCK and uses poll() and
> non-blocking read() to retrieve random data, it ends up in a tight
> loop with poll() always returning POLLIN and read() returning EAGAIN.
> This repeats forever until some process makes a blocking read() call.
> The reason is that virtio_read() always returns 0 in non-blocking mode,
> even if data is available. Worse, it fetches random data from the
> hypervisor after every non-blocking call, without ever using this data.
> 
> The following test program illustrates the behavior and can be used
> for testing and experiments. The problem will only be seen if all
> tasks use non-blocking access; otherwise the blocking reads will
> "recharge" the random pool and cause other, non-blocking reads to
> succeed at least sometimes.
> 
> /* Whether to use non-blocking mode in a task, problem occurs if CONDITION is 
> 1 */
> //#define CONDITION (getpid() % 2 != 0)
> 
> static volatile sig_atomic_t stop;
> static void handler(int sig __attribute__((unused))) { stop = 1; }
> 
> static void loop(int fd, int sec)
> {
>   struct pollfd pfd = { .fd = fd, .events  = POLLIN, };
>   unsigned long errors = 0, eagains = 0, bytes = 0, succ = 0;
>   int size, rc, rd;
> 
>   srandom(getpid());
>   if (CONDITION && fcntl(fd, F_SETFL, fcntl(fd, F_GETFL) | O_NONBLOCK) == 
> -1)
>   perror("fcntl");
>   size = MINBUFSIZ + random() % (MAXBUFSIZ - MINBUFSIZ + 1);
> 
>   for(;;) {
>   char buf[size];
> 
>   if (stop)
>   break;
>   rc = poll(, 1, sec);
>   if (rc > 0) {
>   rd = read(fd, buf, sizeof(buf));
>   if (rd == -1 && errno == EAGAIN)
>   eagains++;
>   else if (rd == -1)
>   errors++;
>   else {
>   succ++;
>   bytes += rd;
>   write(1, buf, sizeof(buf));
>   }
>   } else if (rc == -1) {
>   if (errno != EINTR)
>   perror("poll");
>   break;
>   } else
>   fprintf(stderr, "poll: timeout\n");
>   }
>   fprintf(stderr,
>   "pid %d %sblocking, bufsize %d, %d seconds, %lu bytes read, %lu 
> success, %lu eagain, %lu errors\n",
>   getpid(), CONDITION ? "non-" : "", size, sec, bytes, succ, 
> eagains, errors);
> }
> 
> int main(void)
> {
>   int fd;
> 
>   fork(); fork();
>   fd = open("/dev/hwrng", O_RDONLY);
>   if (fd == -1) {
>   perror("open");
>   return 1;
>   };
>   signal(SIGALRM, handler);
>   alarm(SECONDS);
>   loop(fd, SECONDS);
>   close(fd);
>   wait(NULL);
>   return 0;
> }
> 
> void loop(int fd)
> {
> struct pollfd pfd0 = { .fd = fd, .events  = POLLIN, };
> int rc;
> unsigned int n;
> 
> for (n = LOOPS; n > 0; n--) {
> struct pollfd pfd = pfd0;
> char buf[SIZE];
> 
> rc = poll(, 1, 1);
> if (rc > 0) {
> int rd = read(fd, buf, sizeof(buf));
> 
> if (rd == -1)
> perror("read");
> else
> printf("read %d bytes\n", rd);
> } else if (rc == -1)
> perror("poll");
> else
> fprintf(stderr, "timeout\n");
> 
> }
> }
> 
> int main(void)
> {
> int fd;
> 
> fd = open("/dev/hwrng", O_RDONLY|O_NONBLOCK);
> if (fd == -1) {
> perror("open");
> return 1;
> };
> loop(fd);
> close(fd);
> return 0;
> }
> 
> This can be observed in the real word e.g. with nested qemu/KVM virtual
> machines, if both the "outer" and "inner" VMs have a virtio-rng device.
> If the "inner" VM requests random data, qemu running in the "outer" VM
> uses this device in a non-blocking manner like the test program above.
> 
> Fix it by returning available data if a previous hypervisor call has
> completed in the meantime. I tested the patch with the program above,
> and with rng-tools.
> 
> Signed-off-by: Martin Wilck 
> ---
>  drivers/char/hw_random/virtio-rng.c | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/char/hw_random/virtio-rng.c 
> b/drivers/char/hw_random/virtio-rng.c
> index 79a6e47b5fbc..984713b35892 100644
> --- a/drivers/char/hw_random/virtio-rng.c
> +++ b/drivers/char/hw_random/virtio-rng.c
> @@ -59,6 +59,20 @@ static int virtio_read(struct hwrng *rng, void *buf, 
> size_t size, bool wait)
>   if 

Re: [PATCH] vhost: vdpa: remove per device feature whitelist

2020-08-11 Thread Michael S. Tsirkin
On Mon, Jul 20, 2020 at 04:50:43PM +0800, Jason Wang wrote:
> We used to have a per device feature whitelist to filter out the
> unsupported virtio features. But this seems unnecessary since:
> 
> - the main idea behind feature whitelist is to block control vq
>   feature until we finalize the control virtqueue API. But the current
>   vhost-vDPA uAPI is sufficient to support control virtqueue. For
>   device that has hardware control virtqueue, the vDPA device driver
>   can just setup the hardware virtqueue and let userspace to use
>   hardware virtqueue directly. For device that doesn't have a control
>   virtqueue, the vDPA device driver need to use e.g vringh to emulate
>   a software control virtqueue.
> - we don't do it in virtio-vDPA driver
> 
> So remove this limitation.
> 
> Signed-off-by: Jason Wang 


Thinking about it, should we block some bits?
E.g. access_platform?
they depend on qemu not vdpa ...

> ---
>  drivers/vhost/vdpa.c | 37 -
>  1 file changed, 37 deletions(-)
> 
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 77a0c9fb6cc3..f7f6ddd681ce 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -26,35 +26,6 @@
>  
>  #include "vhost.h"
>  
> -enum {
> - VHOST_VDPA_FEATURES =
> - (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |
> - (1ULL << VIRTIO_F_ANY_LAYOUT) |
> - (1ULL << VIRTIO_F_VERSION_1) |
> - (1ULL << VIRTIO_F_IOMMU_PLATFORM) |
> - (1ULL << VIRTIO_F_RING_PACKED) |
> - (1ULL << VIRTIO_F_ORDER_PLATFORM) |
> - (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
> - (1ULL << VIRTIO_RING_F_EVENT_IDX),
> -
> - VHOST_VDPA_NET_FEATURES = VHOST_VDPA_FEATURES |
> - (1ULL << VIRTIO_NET_F_CSUM) |
> - (1ULL << VIRTIO_NET_F_GUEST_CSUM) |
> - (1ULL << VIRTIO_NET_F_MTU) |
> - (1ULL << VIRTIO_NET_F_MAC) |
> - (1ULL << VIRTIO_NET_F_GUEST_TSO4) |
> - (1ULL << VIRTIO_NET_F_GUEST_TSO6) |
> - (1ULL << VIRTIO_NET_F_GUEST_ECN) |
> - (1ULL << VIRTIO_NET_F_GUEST_UFO) |
> - (1ULL << VIRTIO_NET_F_HOST_TSO4) |
> - (1ULL << VIRTIO_NET_F_HOST_TSO6) |
> - (1ULL << VIRTIO_NET_F_HOST_ECN) |
> - (1ULL << VIRTIO_NET_F_HOST_UFO) |
> - (1ULL << VIRTIO_NET_F_MRG_RXBUF) |
> - (1ULL << VIRTIO_NET_F_STATUS) |
> - (1ULL << VIRTIO_NET_F_SPEED_DUPLEX),
> -};
> -
>  /* Currently, only network backend w/o multiqueue is supported. */
>  #define VHOST_VDPA_VQ_MAX2
>  
> @@ -79,10 +50,6 @@ static DEFINE_IDA(vhost_vdpa_ida);
>  
>  static dev_t vhost_vdpa_major;
>  
> -static const u64 vhost_vdpa_features[] = {
> - [VIRTIO_ID_NET] = VHOST_VDPA_NET_FEATURES,
> -};
> -
>  static void handle_vq_kick(struct vhost_work *work)
>  {
>   struct vhost_virtqueue *vq = container_of(work, struct vhost_virtqueue,
> @@ -255,7 +222,6 @@ static long vhost_vdpa_get_features(struct vhost_vdpa *v, 
> u64 __user *featurep)
>   u64 features;
>  
>   features = ops->get_features(vdpa);
> - features &= vhost_vdpa_features[v->virtio_id];
>  
>   if (copy_to_user(featurep, , sizeof(features)))
>   return -EFAULT;
> @@ -279,9 +245,6 @@ static long vhost_vdpa_set_features(struct vhost_vdpa *v, 
> u64 __user *featurep)
>   if (copy_from_user(, featurep, sizeof(features)))
>   return -EFAULT;
>  
> - if (features & ~vhost_vdpa_features[v->virtio_id])
> - return -EINVAL;
> -
>   if (ops->set_features(vdpa, features))
>   return -EINVAL;
>  
> -- 
> 2.20.1

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


vdpa: handling of VIRTIO_F_ACCESS_PLATFORM/VIRTIO_F_ORDER_PLATFORM

2020-08-11 Thread Michael S. Tsirkin
Hi!
I'd like to raise the question of whether we can drop the requirement
of VIRTIO_F_ACCESS_PLATFORM from vdpa?
As far as I can see, it is merely required for virtio vdpa -
so should we not enforce it there?

The point is support for legacy guests - which mostly just works
on x86.

Also, what is the plan for VIRTIO_F_ORDER_PLATFORM?

-- 
MST

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


[GIT PULL] virtio: features, fixes

2020-08-11 Thread Michael S. Tsirkin
OK, some patches in the series add buggy code which is then fixed by
follow-up patches, but none of the bugs fixed are severe regressions on
common configs (e.g. compiler warnings, lockdep/rt errors, or bugs in
new drivers). So I thought it's more important to preserve the credit
for the fixes.

I had to pull 5 patches from 
git://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux mlx5-next
to get the mlx5 things to work, this seems to be how mellanox guys are
always managing things, and they told me they are ok with it.

The following changes since commit bcf876870b95592b52519ed4aafcf9d95999bc9c:

  Linux 5.8 (2020-08-02 14:21:45 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

for you to fetch changes up to 8a7c3213db068135e816a6a517157de6443290d6:

  vdpa/mlx5: fix up endian-ness for mtu (2020-08-10 10:38:55 -0400)


virtio: fixes, features

IRQ bypass support for vdpa and IFC
MLX5 vdpa driver
Endian-ness fixes for virtio drivers
Misc other fixes

Signed-off-by: Michael S. Tsirkin 


Alex Dewar (1):
  vdpa/mlx5: Fix uninitialised variable in core/mr.c

Colin Ian King (1):
  vdpa/mlx5: fix memory allocation failure checks

Dan Carpenter (2):
  vdpa/mlx5: Fix pointer math in mlx5_vdpa_get_config()
  vdpa: Fix pointer math bug in vdpasim_get_config()

Eli Cohen (9):
  net/mlx5: Support setting access rights of dma addresses
  net/mlx5: Add VDPA interface type to supported enumerations
  net/mlx5: Add interface changes required for VDPA
  net/vdpa: Use struct for set/get vq state
  vdpa: Modify get_vq_state() to return error code
  vdpa/mlx5: Add hardware descriptive header file
  vdpa/mlx5: Add support library for mlx5 VDPA implementation
  vdpa/mlx5: Add shared memory registration code
  vdpa/mlx5: Add VDPA driver for supported mlx5 devices

Gustavo A. R. Silva (1):
  vhost: Use flex_array_size() helper in copy_from_user()

Jason Wang (6):
  vhost: vdpa: remove per device feature whitelist
  vhost-vdpa: refine ioctl pre-processing
  vhost: generialize backend features setting/getting
  vhost-vdpa: support get/set backend features
  vhost-vdpa: support IOTLB batching hints
  vdpasim: support batch updating

Liao Pingfang (1):
  virtio_pci_modern: Fix the comment of virtio_pci_find_capability()

Mao Wenan (1):
  virtio_ring: Avoid loop when vq is broken in virtqueue_poll

Maor Gottlieb (2):
  net/mlx5: Export resource dump interface
  net/mlx5: Add support in query QP, CQ and MKEY segments

Max Gurtovoy (2):
  vdpasim: protect concurrent access to iommu iotlb
  vdpa: remove hard coded virtq num

Meir Lichtinger (1):
  RDMA/mlx5: ConnectX-7 new capabilities to set relaxed ordering by UMR

Michael Guralnik (2):
  net/mlx5: Enable QP number request when creating IPoIB underlay QP
  net/mlx5: Enable count action for rules with allow action

Michael S. Tsirkin (44):
  virtio: VIRTIO_F_IOMMU_PLATFORM -> VIRTIO_F_ACCESS_PLATFORM
  virtio: virtio_has_iommu_quirk -> virtio_has_dma_quirk
  virtio_balloon: fix sparse warning
  virtio_ring: sparse warning fixup
  virtio: allow __virtioXX, __leXX in config space
  virtio_9p: correct tags for config space fields
  virtio_balloon: correct tags for config space fields
  virtio_blk: correct tags for config space fields
  virtio_console: correct tags for config space fields
  virtio_crypto: correct tags for config space fields
  virtio_fs: correct tags for config space fields
  virtio_gpu: correct tags for config space fields
  virtio_input: correct tags for config space fields
  virtio_iommu: correct tags for config space fields
  virtio_mem: correct tags for config space fields
  virtio_net: correct tags for config space fields
  virtio_pmem: correct tags for config space fields
  virtio_scsi: correct tags for config space fields
  virtio_config: disallow native type fields
  mlxbf-tmfifo: sparse tags for config access
  vdpa: make sure set_features is invoked for legacy
  vhost/vdpa: switch to new helpers
  virtio_vdpa: legacy features handling
  vdpa_sim: fix endian-ness of config space
  virtio_config: cread/write cleanup
  virtio_config: rewrite using _Generic
  virtio_config: disallow native type fields (again)
  virtio_config: LE config space accessors
  virtio_caif: correct tags for config space fields
  virtio_config: add virtio_cread_le_feature
  virtio_balloon: use LE config space accesses
  virtio_input: convert to LE accessors
  virtio_fs: convert to LE accessors
  virtio_crypto: convert to LE accessors
  virtio_pmem: convert to LE accessors
  drm/virtio: convert to LE accessors
  virt

Re: [PATCH 1/4] vdpa: introduce config op to get valid iova range

2020-08-11 Thread Michael S. Tsirkin
On Tue, Aug 11, 2020 at 10:53:09AM +0800, Jason Wang wrote:
> 
> On 2020/8/10 下午8:05, Michael S. Tsirkin wrote:
> > On Thu, Aug 06, 2020 at 03:43:54PM +0300, Eli Cohen wrote:
> > > On Thu, Aug 06, 2020 at 08:29:22AM -0400, Michael S. Tsirkin wrote:
> > > > On Thu, Aug 06, 2020 at 03:03:55PM +0300, Eli Cohen wrote:
> > > > > On Wed, Aug 05, 2020 at 08:51:56AM -0400, Michael S. Tsirkin wrote:
> > > > > > On Wed, Jun 17, 2020 at 11:29:44AM +0800, Jason Wang wrote:
> > > > > > > This patch introduce a config op to get valid iova range from the 
> > > > > > > vDPA
> > > > > > > device.
> > > > > > > 
> > > > > > > Signed-off-by: Jason Wang 
> > > > > > > ---
> > > > > > >   include/linux/vdpa.h | 14 ++
> > > > > > >   1 file changed, 14 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > > > > > > index 239db794357c..b7633ed2500c 100644
> > > > > > > --- a/include/linux/vdpa.h
> > > > > > > +++ b/include/linux/vdpa.h
> > > > > > > @@ -41,6 +41,16 @@ struct vdpa_device {
> > > > > > >   unsigned int index;
> > > > > > >   };
> > > > > > > +/**
> > > > > > > + * vDPA IOVA range - the IOVA range support by the device
> > > > > > > + * @start: start of the IOVA range
> > > > > > > + * @end: end of the IOVA range
> > > > > > > + */
> > > > > > > +struct vdpa_iova_range {
> > > > > > > + u64 start;
> > > > > > > + u64 end;
> > > > > > > +};
> > > > > > > +
> > > > > > 
> > > > > > This is ambiguous. Is end in the range or just behind it?
> > > > > > How about first/last?
> > > > > It is customary in the kernel to use start-end where end corresponds 
> > > > > to
> > > > > the byte following the last in the range. See struct vm_area_struct
> > > > > vm_start and vm_end fields
> > > > Exactly my point:
> > > > 
> > > > include/linux/mm_types.h:   unsigned long vm_end;   /* The 
> > > > first byte after our end address
> > > > 
> > > > in this case Jason wants it to be the last byte, not one behind.
> > > > 
> > > > 
> > > Maybe start, size? Not ambiguous, and you don't need to do annoying
> > > calculations like size = last - start + 1
> > Size has a bunch of issues: can overlap, can not cover the entire 64 bit
> > range. The requisite checks are arguably easier to get wrong than
> > getting the size if you need it.
> 
> 
> Yes, so do you still prefer first/last or just begin/end which is consistent
> with iommu_domain_geometry?
> 
> Thanks

I prefer first/last I think, these are unambiguous.
E.g.

dma_addr_t aperture_start; /* First address that can be mapped*/
dma_addr_t aperture_end;   /* Last address that can be mapped */

instead of addressing ambiguity with a comment, let's just name the field well.



> 
> > 

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

Re: [PATCH] vdpa/mlx5: Fix pointer math in mlx5_vdpa_get_config()

2020-08-10 Thread Michael S. Tsirkin
On Mon, Aug 10, 2020 at 01:31:47PM +0300, Dan Carpenter wrote:
> On Sun, Aug 09, 2020 at 06:34:04AM +, Eli Cohen wrote:
> > Acked-by: Eli Cohen 
> > 
> > BTW, vdpa_sim has the same bug.
> > 
> 
> I sent a patch for that on April 6.
> 
> [PATCH 2/2] vdpa: Fix pointer math bug in vdpasim_get_config()
> 
> Jason acked the patch but it wasn't applied.
> 
> regards,
> dan carpenter

Oh sorry. I'll drop my patch and queue yours then.

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


Re: [PATCH v4 4/4] vhost: add an RPMsg API

2020-08-10 Thread Michael S. Tsirkin
On Tue, Aug 04, 2020 at 05:19:17PM +0200, Guennadi Liakhovetski wrote:
> On Tue, Aug 04, 2020 at 10:27:08AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Jul 22, 2020 at 05:09:27PM +0200, Guennadi Liakhovetski wrote:
> > > Linux supports running the RPMsg protocol over the VirtIO transport
> > > protocol, but currently there is only support for VirtIO clients and
> > > no support for a VirtIO server. This patch adds a vhost-based RPMsg
> > > server implementation.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski 
> > > 
> > > ---
> > >  drivers/vhost/Kconfig   |   7 +
> > >  drivers/vhost/Makefile  |   3 +
> > >  drivers/vhost/rpmsg.c   | 375 
> > >  drivers/vhost/vhost_rpmsg.h |  74 +++
> > >  4 files changed, 459 insertions(+)
> > >  create mode 100644 drivers/vhost/rpmsg.c
> > >  create mode 100644 drivers/vhost/vhost_rpmsg.h
> > > 
> > > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> > > index d3688c6afb87..602421bf1d03 100644
> > > --- a/drivers/vhost/Kconfig
> > > +++ b/drivers/vhost/Kconfig
> > > @@ -38,6 +38,13 @@ config VHOST_NET
> > > To compile this driver as a module, choose M here: the module will
> > > be called vhost_net.
> > >  
> > > +config VHOST_RPMSG
> > > + tristate
> > 
> > So this lacks a description line so it does not appear
> > in menuconfig. How is user supposed to set it?
> > I added a one-line description.
> 
> That was on purpose. I don't think there's any value in this API stand-alone, 
> so I let users select it as needed. But we can change that too, id desired.

I guess the patches actually selecting this 
are separate then?

> > > + depends on VHOST
> > 
> > Other drivers select VHOST instead. Any reason not to
> > do it like this here?
> 
> I have
> 
> + select VHOST
> + select VHOST_RPMSG
> 
> in my client driver patch.

Any issues selecting from here so others get it for free?
If this is selected then dependencies are ignored ...

> > > + help
> > > +   Vhost RPMsg API allows vhost drivers to communicate with VirtIO
> > > +   drivers, using the RPMsg over VirtIO protocol.
> > > +
> > 
> > >  config VHOST_SCSI
> > >   tristate "VHOST_SCSI TCM fabric driver"
> > >   depends on TARGET_CORE && EVENTFD
> > > diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
> > > index f3e1897cce85..9cf459d59f97 100644
> > > --- a/drivers/vhost/Makefile
> > > +++ b/drivers/vhost/Makefile
> > > @@ -2,6 +2,9 @@
> > >  obj-$(CONFIG_VHOST_NET) += vhost_net.o
> > >  vhost_net-y := net.o
> > >  
> > > +obj-$(CONFIG_VHOST_RPMSG) += vhost_rpmsg.o
> > > +vhost_rpmsg-y := rpmsg.o
> > > +
> > >  obj-$(CONFIG_VHOST_SCSI) += vhost_scsi.o
> > >  vhost_scsi-y := scsi.o
> > >  
> > > diff --git a/drivers/vhost/rpmsg.c b/drivers/vhost/rpmsg.c
> > > new file mode 100644
> > > index ..d7ab48414224
> > > --- /dev/null
> > > +++ b/drivers/vhost/rpmsg.c
> > > @@ -0,0 +1,375 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright(c) 2020 Intel Corporation. All rights reserved.
> > > + *
> > > + * Author: Guennadi Liakhovetski 
> > > + *
> > > + * Vhost RPMsg VirtIO interface. It provides a set of functions to match 
> > > the
> > > + * guest side RPMsg VirtIO API, provided by 
> > > drivers/rpmsg/virtio_rpmsg_bus.c
> > > + * These functions handle creation of 2 virtual queues, handling of 
> > > endpoint
> > > + * addresses, sending a name-space announcement to the guest as well as 
> > > any
> > > + * user messages. This API can be used by any vhost driver to handle 
> > > RPMsg
> > > + * specific processing.
> > > + * Specific vhost drivers, using this API will use their own VirtIO 
> > > device
> > > + * IDs, that should then also be added to the ID table in 
> > > virtio_rpmsg_bus.c
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +#include "vhost.h"
> > > +#include "vhost_rpmsg.h"
> > > +
> > > +/*
> > > + * All virtio-rpmsg virtua

Re: [PATCH V5 1/6] vhost: introduce vhost_vring_call

2020-08-10 Thread Michael S. Tsirkin
On Wed, Aug 05, 2020 at 10:16:16AM +0800, Jason Wang wrote:
> 
> On 2020/8/4 下午5:21, Michael S. Tsirkin wrote:
> > > > > >    +struct vhost_vring_call {
> > > > > > +    struct eventfd_ctx *ctx;
> > > > > > +    struct irq_bypass_producer producer;
> > > > > > +    spinlock_t ctx_lock;
> > > > > It's not clear to me why we need ctx_lock here.
> > > > > 
> > > > > Thanks
> > > > Hi Jason,
> > > > 
> > > > we use this lock to protect the eventfd_ctx and irq from race 
> > > > conditions,
> > > We don't support irq notification from vDPA device driver in this version,
> > > do we still have race condition?
> > > 
> > > Thanks
> > Jason I'm not sure what you are trying to say here.
> 
> 
> I meant we change the API from V4 so driver won't notify us if irq is
> changed.
> 
> Then it looks to me there's no need for the ctx_lock, everyhing could be
> synchronized with vq mutex.
> 
> Thanks

Jason do you want to post a cleanup patch simplifying code along these
lines?

Thanks,


> > 
> > 

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

[PATCH] vdpa/mlx5: fix up endian-ness for mtu

2020-08-10 Thread Michael S. Tsirkin
VDPA mlx5 accesses config space as native endian - this is
wrong since it's a modern device and actually uses LE.

It only supports modern guests so we could punt and
just force LE, but let's use the full virtio APIs since people
tend to copy/paste code, and this is not data path anyway.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index c6b9ec47e51d..9df69d5efe8c 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -137,6 +137,7 @@ struct mlx5_vdpa_net {
struct mlx5_fc *rx_counter;
struct mlx5_flow_handle *rx_rule;
bool setup;
+   u16 mtu;
 };
 
 static void free_resources(struct mlx5_vdpa_net *ndev);
@@ -1506,6 +1507,13 @@ static void teardown_virtqueues(struct mlx5_vdpa_net 
*ndev)
}
 }
 
+/* TODO: cross-endian support */
+static inline bool mlx5_vdpa_is_little_endian(struct mlx5_vdpa_dev *mvdev)
+{
+   return virtio_legacy_is_little_endian() ||
+   (mvdev->actual_features & (1ULL << VIRTIO_F_VERSION_1));
+}
+
 static int mlx5_vdpa_set_features(struct vdpa_device *vdev, u64 features)
 {
struct mlx5_vdpa_dev *mvdev = to_mvdev(vdev);
@@ -1519,6 +1527,8 @@ static int mlx5_vdpa_set_features(struct vdpa_device 
*vdev, u64 features)
return err;
 
ndev->mvdev.actual_features = features & ndev->mvdev.mlx_features;
+   ndev->config.mtu = __cpu_to_virtio16(mlx5_vdpa_is_little_endian(mvdev),
+ndev->mtu);
return err;
 }
 
@@ -1925,7 +1935,7 @@ void *mlx5_vdpa_add_dev(struct mlx5_core_dev *mdev)
init_mvqs(ndev);
mutex_init(>reslock);
config = >config;
-   err = mlx5_query_nic_vport_mtu(mdev, >mtu);
+   err = mlx5_query_nic_vport_mtu(mdev, >mtu);
if (err)
goto err_mtu;
 
-- 
MST

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


[PATCH] vdpa_sim: fix pointer math in get_config

2020-08-10 Thread Michael S. Tsirkin
There is a pointer math bug here: the variable cast is a struct so the
offset is in units of struct size.  If "offset" is non-zero this will
copy memory from beyond the end of the array.

fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
Reported-by: Dan Carpenter 
Signed-off-by: Michael S. Tsirkin 
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 604d9d25ca47..62d640327145 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -558,7 +558,7 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, 
unsigned int offset,
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 
if (offset + len < sizeof(struct virtio_net_config))
-   memcpy(buf, >config + offset, len);
+   memcpy(buf, (u8 *)>config + offset, len);
 }
 
 static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
-- 
MST

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


Re: [PATCH][next] vdpa/mlx5: fix memory allocation failure checks

2020-08-10 Thread Michael S. Tsirkin
On Sun, Aug 09, 2020 at 09:03:47AM +0300, Eli Cohen wrote:
> On Thu, Aug 06, 2020 at 05:08:28PM +0100, Colin King wrote:
> Acked by: Eli Cohen 

That should be Acked-by: (with a dash).

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


[PATCH] vdpa_sim: init iommu lock

2020-08-10 Thread Michael S. Tsirkin
The patch adding the iommu lock did not initialize it.
The struct is zero-initialized so this is mostly a problem
when using lockdep.

Reported-by: kernel test robot 
Cc: Max Gurtovoy 
Fixes: 0ea9ee430e74 ("vdpasim: protect concurrent access to iommu iotlb")
Signed-off-by: Michael S. Tsirkin 
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index df3224b138ee..604d9d25ca47 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -358,6 +358,7 @@ static struct vdpasim *vdpasim_create(void)
 
INIT_WORK(>work, vdpasim_work);
spin_lock_init(>lock);
+   spin_lock_init(>iommu_lock);
 
dev = >vdpa.dev;
dev->coherent_dma_mask = DMA_BIT_MASK(64);
-- 
MST

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


Re: [PATCH 1/4] vdpa: introduce config op to get valid iova range

2020-08-10 Thread Michael S. Tsirkin
On Thu, Aug 06, 2020 at 03:43:54PM +0300, Eli Cohen wrote:
> On Thu, Aug 06, 2020 at 08:29:22AM -0400, Michael S. Tsirkin wrote:
> > On Thu, Aug 06, 2020 at 03:03:55PM +0300, Eli Cohen wrote:
> > > On Wed, Aug 05, 2020 at 08:51:56AM -0400, Michael S. Tsirkin wrote:
> > > > On Wed, Jun 17, 2020 at 11:29:44AM +0800, Jason Wang wrote:
> > > > > This patch introduce a config op to get valid iova range from the vDPA
> > > > > device.
> > > > > 
> > > > > Signed-off-by: Jason Wang 
> > > > > ---
> > > > >  include/linux/vdpa.h | 14 ++
> > > > >  1 file changed, 14 insertions(+)
> > > > > 
> > > > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > > > > index 239db794357c..b7633ed2500c 100644
> > > > > --- a/include/linux/vdpa.h
> > > > > +++ b/include/linux/vdpa.h
> > > > > @@ -41,6 +41,16 @@ struct vdpa_device {
> > > > >   unsigned int index;
> > > > >  };
> > > > >  
> > > > > +/**
> > > > > + * vDPA IOVA range - the IOVA range support by the device
> > > > > + * @start: start of the IOVA range
> > > > > + * @end: end of the IOVA range
> > > > > + */
> > > > > +struct vdpa_iova_range {
> > > > > + u64 start;
> > > > > + u64 end;
> > > > > +};
> > > > > +
> > > > 
> > > > 
> > > > This is ambiguous. Is end in the range or just behind it?
> > > > How about first/last?
> > > 
> > > It is customary in the kernel to use start-end where end corresponds to
> > > the byte following the last in the range. See struct vm_area_struct
> > > vm_start and vm_end fields
> > 
> > Exactly my point:
> > 
> > include/linux/mm_types.h:   unsigned long vm_end;   /* The 
> > first byte after our end address
> > 
> > in this case Jason wants it to be the last byte, not one behind.
> > 
> > 
> Maybe start, size? Not ambiguous, and you don't need to do annoying
> calculations like size = last - start + 1

Size has a bunch of issues: can overlap, can not cover the entire 64 bit
range. The requisite checks are arguably easier to get wrong than
getting the size if you need it.

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


Re: [PATCH 1/4] vdpa: introduce config op to get valid iova range

2020-08-06 Thread Michael S. Tsirkin
On Thu, Aug 06, 2020 at 03:03:55PM +0300, Eli Cohen wrote:
> On Wed, Aug 05, 2020 at 08:51:56AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Jun 17, 2020 at 11:29:44AM +0800, Jason Wang wrote:
> > > This patch introduce a config op to get valid iova range from the vDPA
> > > device.
> > > 
> > > Signed-off-by: Jason Wang 
> > > ---
> > >  include/linux/vdpa.h | 14 ++
> > >  1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > > index 239db794357c..b7633ed2500c 100644
> > > --- a/include/linux/vdpa.h
> > > +++ b/include/linux/vdpa.h
> > > @@ -41,6 +41,16 @@ struct vdpa_device {
> > >   unsigned int index;
> > >  };
> > >  
> > > +/**
> > > + * vDPA IOVA range - the IOVA range support by the device
> > > + * @start: start of the IOVA range
> > > + * @end: end of the IOVA range
> > > + */
> > > +struct vdpa_iova_range {
> > > + u64 start;
> > > + u64 end;
> > > +};
> > > +
> > 
> > 
> > This is ambiguous. Is end in the range or just behind it?
> > How about first/last?
> 
> It is customary in the kernel to use start-end where end corresponds to
> the byte following the last in the range. See struct vm_area_struct
> vm_start and vm_end fields

Exactly my point:

include/linux/mm_types.h:   unsigned long vm_end;   /* The first 
byte after our end address

in this case Jason wants it to be the last byte, not one behind.


> > 
> > 
> > 
> > >  /**
> > >   * vDPA_config_ops - operations for configuring a vDPA device.
> > >   * Note: vDPA device drivers are required to implement all of the
> > > @@ -134,6 +144,9 @@ struct vdpa_device {
> > >   * @get_generation:  Get device config generation (optional)
> > >   *   @vdev: vdpa device
> > >   *   Returns u32: device generation
> > > + * @get_iova_range:  Get supported iova range (on-chip IOMMU)
> > > + *   @vdev: vdpa device
> > > + *   Returns the iova range supported by the 
> > > device
> > >   * @set_map: Set device memory mapping (optional)
> > >   *   Needed for device that using device
> > >   *   specific DMA translation (on-chip IOMMU)
> > > @@ -195,6 +208,7 @@ struct vdpa_config_ops {
> > >   void (*set_config)(struct vdpa_device *vdev, unsigned int offset,
> > >  const void *buf, unsigned int len);
> > >   u32 (*get_generation)(struct vdpa_device *vdev);
> > > + struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev);
> > >  
> > >   /* DMA ops */
> > >   int (*set_map)(struct vdpa_device *vdev, struct vhost_iotlb *iotlb);
> > > -- 
> > > 2.20.1
> > 

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


Re: [PATCH v2 19/24] vdpa: make sure set_features in invoked for legacy

2020-08-06 Thread Michael S. Tsirkin
On Thu, Aug 06, 2020 at 03:27:38PM +0800, Jason Wang wrote:
> 
> On 2020/8/6 下午1:53, Michael S. Tsirkin wrote:
> > On Thu, Aug 06, 2020 at 11:23:05AM +0800, Jason Wang wrote:
> > > On 2020/8/5 下午7:40, Michael S. Tsirkin wrote:
> > > > On Wed, Aug 05, 2020 at 02:14:07PM +0800, Jason Wang wrote:
> > > > > On 2020/8/4 上午5:00, Michael S. Tsirkin wrote:
> > > > > > Some legacy guests just assume features are 0 after reset.
> > > > > > We detect that config space is accessed before features are
> > > > > > set and set features to 0 automatically.
> > > > > > Note: some legacy guests might not even access config space, if 
> > > > > > this is
> > > > > > reported in the field we might need to catch a kick to handle these.
> > > > > I wonder whether it's easier to just support modern device?
> > > > > 
> > > > > Thanks
> > > > Well hardware vendors are I think interested in supporting legacy
> > > > guests. Limiting vdpa to modern only would make it uncompetitive.
> > > 
> > > My understanding is that, IOMMU_PLATFORM is mandatory for hardware vDPA to
> > > work.
> > Hmm I don't really see why. Assume host maps guest memory properly,
> > VM does not have an IOMMU, legacy guest can just work.
> 
> 
> Yes, guest may not set IOMMU_PLATFORM.
> 
> 
> > 
> > Care explaining what's wrong with this picture?
> 
> 
> The problem is virtio_vdpa, without IOMMU_PLATFORM it uses PA which can not
> work if IOMMU is enabled.
> 
> Thanks

So that's a virtio_vdpa limitation. In the same way, if a device
does not have an on-device iommu *and* is not behind an iommu,
then vdpa can't bind to it.

But this virtio_vdpa specific hack does not belong in a generic vdpa code.

> 
> > 
> > 
> > > So it can only work for modern device ...
> > > 
> > > Thanks
> > > 
> > > 
> > > > 
> > > > 

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

Re: [PATCH v2 03/24] virtio: allow __virtioXX, __leXX in config space

2020-08-05 Thread Michael S. Tsirkin
On Thu, Aug 06, 2020 at 11:37:38AM +0800, Jason Wang wrote:
> 
> On 2020/8/5 下午7:45, Michael S. Tsirkin wrote:
> > > >#define virtio_cread(vdev, structname, member, ptr)  
> > > > \
> > > > do {
> > > > \
> > > > might_sleep();  
> > > > \
> > > > /* Must match the member's type, and be integer */  
> > > > \
> > > > -   if (!typecheck(typeofstructname*)0)->member)), 
> > > > *(ptr))) \
> > > > +   if (!__virtio_typecheck(structname, member, *(ptr)))
> > > > \
> > > > (*ptr) = 1; 
> > > > \
> > > A silly question,  compare to using set()/get() directly, what's the value
> > > of the accessors macro here?
> > > 
> > > Thanks
> > get/set don't convert to the native endian, I guess that's why
> > drivers use cread/cwrite. It is also nice that there's type
> > safety, checking the correct integer width is used.
> 
> 
> Yes, but this is simply because a macro is used here, how about just doing
> things similar like virtio_cread_bytes():
> 
> static inline void virtio_cread(struct virtio_device *vdev,
>                   unsigned int offset,
>                   void *buf, size_t len)
> 
> 
> And do the endian conversion inside?
> 
> Thanks
> 

Then you lose type safety. It's very easy to have an le32 field
and try to read it into a u16 by mistake.

These macros are all about preventing bugs: and the whole patchset
is about several bugs sparse found - that is what prompted me to make
type checks more strict.


> > 

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

Re: [PATCH 4/4] vhost: vdpa: report iova range

2020-08-05 Thread Michael S. Tsirkin
On Thu, Aug 06, 2020 at 11:29:16AM +0800, Jason Wang wrote:
> 
> On 2020/8/5 下午8:58, Michael S. Tsirkin wrote:
> > On Wed, Jun 17, 2020 at 11:29:47AM +0800, Jason Wang wrote:
> > > This patch introduces a new ioctl for vhost-vdpa device that can
> > > report the iova range by the device. For device that depends on
> > > platform IOMMU, we fetch the iova range via DOMAIN_ATTR_GEOMETRY. For
> > > devices that has its own DMA translation unit, we fetch it directly
> > > from vDPA bus operation.
> > > 
> > > Signed-off-by: Jason Wang 
> > > ---
> > >   drivers/vhost/vdpa.c | 27 +++
> > >   include/uapi/linux/vhost.h   |  4 
> > >   include/uapi/linux/vhost_types.h |  5 +
> > >   3 files changed, 36 insertions(+)
> > > 
> > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> > > index 77a0c9fb6cc3..ad23e66cbf57 100644
> > > --- a/drivers/vhost/vdpa.c
> > > +++ b/drivers/vhost/vdpa.c
> > > @@ -332,6 +332,30 @@ static long vhost_vdpa_set_config_call(struct 
> > > vhost_vdpa *v, u32 __user *argp)
> > >   return 0;
> > >   }
> > > +
> > > +static long vhost_vdpa_get_iova_range(struct vhost_vdpa *v, u32 __user 
> > > *argp)
> > > +{
> > > + struct iommu_domain_geometry geo;
> > > + struct vdpa_device *vdpa = v->vdpa;
> > > + const struct vdpa_config_ops *ops = vdpa->config;
> > > + struct vhost_vdpa_iova_range range;
> > > + struct vdpa_iova_range vdpa_range;
> > > +
> > > + if (!ops->set_map && !ops->dma_map) {
> > Why not just check if (ops->get_iova_range) directly?
> 
> 
> Because set_map || dma_ops is a hint that the device has its own DMA
> translation logic.
> 
> Device without get_iova_range does not necessarily meant it use IOMMU
> driver.
> 
> Thanks

OK let's add some code comments please, and check get_iova_range
is actually there before calling.

> 
> > 
> > 
> > 
> > 
> > > + iommu_domain_get_attr(v->domain,
> > > +   DOMAIN_ATTR_GEOMETRY, );
> > > + range.start = geo.aperture_start;
> > > + range.end = geo.aperture_end;
> > > + } else {
> > > + vdpa_range = ops->get_iova_range(vdpa);
> > > + range.start = vdpa_range.start;
> > > + range.end = vdpa_range.end;
> > > + }
> > > +
> > > + return copy_to_user(argp, , sizeof(range));
> > > +
> > > +}
> > > +
> > >   static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int 
> > > cmd,
> > >  void __user *argp)
> > >   {
> > > @@ -442,6 +466,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file 
> > > *filep,
> > >   case VHOST_VDPA_SET_CONFIG_CALL:
> > >   r = vhost_vdpa_set_config_call(v, argp);
> > >   break;
> > > + case VHOST_VDPA_GET_IOVA_RANGE:
> > > + r = vhost_vdpa_get_iova_range(v, argp);
> > > + break;
> > >   default:
> > >   r = vhost_dev_ioctl(>vdev, cmd, argp);
> > >   if (r == -ENOIOCTLCMD)
> > > diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> > > index 0c2349612e77..850956980e27 100644
> > > --- a/include/uapi/linux/vhost.h
> > > +++ b/include/uapi/linux/vhost.h
> > > @@ -144,4 +144,8 @@
> > >   /* Set event fd for config interrupt*/
> > >   #define VHOST_VDPA_SET_CONFIG_CALL  _IOW(VHOST_VIRTIO, 0x77, int)
> > > +
> > > +/* Get the valid iova range */
> > > +#define VHOST_VDPA_GET_IOVA_RANGE_IOW(VHOST_VIRTIO, 0x78, \
> > > +  struct vhost_vdpa_iova_range)
> > >   #endif
> > > diff --git a/include/uapi/linux/vhost_types.h 
> > > b/include/uapi/linux/vhost_types.h
> > > index 669457ce5c48..4025b5a36177 100644
> > > --- a/include/uapi/linux/vhost_types.h
> > > +++ b/include/uapi/linux/vhost_types.h
> > > @@ -127,6 +127,11 @@ struct vhost_vdpa_config {
> > >   __u8 buf[0];
> > >   };
> > > +struct vhost_vdpa_iova_range {
> > > + __u64 start;
> > > + __u64 end;
> > > +};
> > > +
> > 
> > Pls document fields. And I think first/last is a better API ...
> > 
> > >   /* Feature bits */
> > >   /* Log all write descriptors. Can be changed while device is active. */
> > >   #define VHOST_F_LOG_ALL 26
> > > -- 
> > > 2.20.1

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

Re: [PATCH 1/4] vdpa: introduce config op to get valid iova range

2020-08-05 Thread Michael S. Tsirkin
On Thu, Aug 06, 2020 at 11:25:11AM +0800, Jason Wang wrote:
> 
> On 2020/8/5 下午8:51, Michael S. Tsirkin wrote:
> > On Wed, Jun 17, 2020 at 11:29:44AM +0800, Jason Wang wrote:
> > > This patch introduce a config op to get valid iova range from the vDPA
> > > device.
> > > 
> > > Signed-off-by: Jason Wang
> > > ---
> > >   include/linux/vdpa.h | 14 ++
> > >   1 file changed, 14 insertions(+)
> > > 
> > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > > index 239db794357c..b7633ed2500c 100644
> > > --- a/include/linux/vdpa.h
> > > +++ b/include/linux/vdpa.h
> > > @@ -41,6 +41,16 @@ struct vdpa_device {
> > >   unsigned int index;
> > >   };
> > > +/**
> > > + * vDPA IOVA range - the IOVA range support by the device
> > > + * @start: start of the IOVA range
> > > + * @end: end of the IOVA range
> > > + */
> > > +struct vdpa_iova_range {
> > > + u64 start;
> > > + u64 end;
> > > +};
> > > +
> > This is ambiguous. Is end in the range or just behind it?
> 
> 
> In the range.

OK I guess we can treat it as a bugfix and merge after rc1,
but pls add a bit more in the commit log about what's
currently broken.

> 
> > How about first/last?
> 
> 
> Sure.
> 
> Thanks
> 
> 
> > 
> > 
> > 

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

Re: [PATCH v2 19/24] vdpa: make sure set_features in invoked for legacy

2020-08-05 Thread Michael S. Tsirkin
On Thu, Aug 06, 2020 at 11:23:05AM +0800, Jason Wang wrote:
> 
> On 2020/8/5 下午7:40, Michael S. Tsirkin wrote:
> > On Wed, Aug 05, 2020 at 02:14:07PM +0800, Jason Wang wrote:
> > > On 2020/8/4 上午5:00, Michael S. Tsirkin wrote:
> > > > Some legacy guests just assume features are 0 after reset.
> > > > We detect that config space is accessed before features are
> > > > set and set features to 0 automatically.
> > > > Note: some legacy guests might not even access config space, if this is
> > > > reported in the field we might need to catch a kick to handle these.
> > > I wonder whether it's easier to just support modern device?
> > > 
> > > Thanks
> > Well hardware vendors are I think interested in supporting legacy
> > guests. Limiting vdpa to modern only would make it uncompetitive.
> 
> 
> My understanding is that, IOMMU_PLATFORM is mandatory for hardware vDPA to
> work.

Hmm I don't really see why. Assume host maps guest memory properly,
VM does not have an IOMMU, legacy guest can just work.

Care explaining what's wrong with this picture?


> So it can only work for modern device ...
> 
> Thanks
> 
> 
> > 
> > 
> > 

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

Re: [vhost:vhost 32/52] include/linux/typecheck.h:12:18: warning: comparison of distinct pointer types lacks a cast

2020-08-05 Thread Michael S. Tsirkin
On Thu, Aug 06, 2020 at 04:17:13AM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
> head:   4c05433bc6fb4ae172270f0279be8ba89a3da64f
> commit: b025584098e621d88894d28e80af686958e273af [32/52] virtio_input: 
> convert to LE accessors
> config: parisc-randconfig-r003-20200805 (attached as .config)
> compiler: hppa-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> git checkout b025584098e621d88894d28e80af686958e273af
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross 
> ARCH=parisc 

Weird. So the following fixes it:


diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index ecb166c824bb..8fe857e27ef3 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -357,10 +357,10 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
  */
 #define virtio_le_to_cpu(x) \
_Generic((x), \
-   __u8: (x), \
-__le16: le16_to_cpu(x), \
-__le32: le32_to_cpu(x), \
-__le64: le64_to_cpu(x) \
+   __u8: (u8)(x), \
+__le16: (u16)le16_to_cpu(x), \
+__le32: (u32)le32_to_cpu(x), \
+__le64: (u64)le64_to_cpu(x) \
)
 
 #define virtio_cpu_to_le(x, m) \
@@ -400,7 +400,6 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
*(ptr) = virtio_le_to_cpu(virtio_cread_v);  \
} while(0)
 
-/* Config space accessors. */
 #define virtio_cwrite_le(vdev, structname, member, ptr)
\
do {\
typeof(((structname*)0)->member) virtio_cwrite_v =  \


How could this be? le16_to_cpu doesn't return a u16?
I suspect this compiler gets confused by _Generic.
Let's hope it does not also miscompile the code :)


> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot 
> 
> All warnings (new ones prefixed by >>):
> 
>In file included from include/linux/irqflags.h:15,
> from include/asm-generic/cmpxchg-local.h:6,
> from arch/parisc/include/asm/cmpxchg.h:89,
> from arch/parisc/include/asm/atomic.h:10,
> from include/linux/atomic.h:7,
> from arch/parisc/include/asm/bitops.h:13,
> from include/linux/bitops.h:29,
> from include/linux/kernel.h:12,
> from include/linux/list.h:9,
> from include/linux/module.h:12,
> from drivers/virtio/virtio_input.c:2:
>drivers/virtio/virtio_input.c: In function 'virtinput_probe':
> >> include/linux/typecheck.h:12:18: warning: comparison of distinct pointer 
> >> types lacks a cast
>   12 |  (void)(&__dummy == &__dummy2); \
>  |  ^~
>include/linux/virtio_config.h:405:3: note: in expansion of macro 
> 'typecheck'
>  405 |   typecheck(typeof(virtio_le_to_cpu(virtio_cread_v)), *(ptr)); \
>  |   ^
>drivers/virtio/virtio_input.c:247:3: note: in expansion of macro 
> 'virtio_cread_le'
>  247 |   virtio_cread_le(vi->vdev, struct virtio_input_config,
>  |   ^~~
> >> include/linux/typecheck.h:12:18: warning: comparison of distinct pointer 
> >> types lacks a cast
>   12 |  (void)(&__dummy == &__dummy2); \
>  |  ^~
>include/linux/virtio_config.h:405:3: note: in expansion of macro 
> 'typecheck'
>  405 |   typecheck(typeof(virtio_le_to_cpu(virtio_cread_v)), *(ptr)); \
>  |   ^
>drivers/virtio/virtio_input.c:249:3: note: in expansion of macro 
> 'virtio_cread_le'
>  249 |   virtio_cread_le(vi->vdev, struct virtio_input_config,
>  |   ^~~
> >> include/linux/typecheck.h:12:18: warning: comparison of distinct pointer 
> >> types lacks a cast
>   12 |  (void)(&__dummy == &__dummy2); \
>  |  ^~
>include/linux/virtio_config.h:405:3: note: in expansion of macro 
> 'typecheck'
>  405 |   typecheck(typeof(virtio_le_to_cpu(virtio_cread_v)), *(ptr)); \
>  |   ^
>drivers/virtio/virtio_input.c:251:3: note: in expansion of macro 
> 'virtio_cread_le'
>  251 |   virtio_cread_le(vi->vdev, struct virtio_input_config,
>  |   ^~~
> >> include/linux/typecheck.h:12:18: warning: comparison of distinct pointer 
> >> types lacks a cast
>   12 |  (void)(&__dummy == &__dummy2); \
>  |  ^~
>include/linux/virtio_config.h:405:3: note: in expansion of macro 
> 'typecheck'
>  

Re: [PATCH V4 linux-next 00/12] VDPA support for Mellanox ConnectX devices

2020-08-05 Thread Michael S. Tsirkin
On Wed, Aug 05, 2020 at 04:46:46PM -0300, Jason Gunthorpe wrote:
> On Wed, Aug 05, 2020 at 07:01:52PM +, Saeed Mahameed wrote:
> > On Wed, 2020-08-05 at 09:12 -0400, Michael S. Tsirkin wrote:
> > > On Wed, Aug 05, 2020 at 04:01:58PM +0300, Eli Cohen wrote:
> > > > On Wed, Aug 05, 2020 at 08:48:52AM -0400, Michael S. Tsirkin wrote:
> > > > > > Did you merge this?:
> > > > > > git pull
> > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux.gi
> > > > > > t mlx5-next
> > > > > 
> > > > > I can only merge this tree if no one else will. Linus does not
> > > > > like getting same patches through two trees.
> 
> This is not quite the case
> 
> Linus does not like multiple *copies* of the same patches. The same
> actual git commits can be OK.
> 
> Linus also does not like unnecessarily cross linking trees, mlx5-next
> is designed to be small enough and approved enough that it is not
> controversial.
> 
> Linus really doesn't like it when people jams stuff together in rc7 or
> the weeks of the merge window. He wants to see stuff be in linux-next
> for at least a bit. So it may be too late regardless.

I'll try, let's see what happens.

> > We do this all the time with net-next and rdma,
> > mlx5-next is a very small branch based on a very early rc that includes
> > mlx5 shared stuff between rdma and net-next, and now virtio as well.
> 
> Yes, going on two years now? Been working well
> 
> Jason

OK, I'll merge it then. Thanks!

-- 
MST

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


[PATCH v3 34/38] drm/virtio: convert to LE accessors

2020-08-05 Thread Michael S. Tsirkin
Virtgpu is modern-only. Use LE accessors for config space.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/gpu/drm/virtio/virtgpu_kms.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c 
b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 0a5c8cf409fb..4d944a0dff3e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -39,8 +39,8 @@ static void virtio_gpu_config_changed_work_func(struct 
work_struct *work)
u32 events_read, events_clear = 0;
 
/* read the config space */
-   virtio_cread(vgdev->vdev, struct virtio_gpu_config,
-events_read, _read);
+   virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
+   events_read, _read);
if (events_read & VIRTIO_GPU_EVENT_DISPLAY) {
if (vgdev->has_edid)
virtio_gpu_cmd_get_edids(vgdev);
@@ -49,8 +49,8 @@ static void virtio_gpu_config_changed_work_func(struct 
work_struct *work)
drm_helper_hpd_irq_event(vgdev->ddev);
events_clear |= VIRTIO_GPU_EVENT_DISPLAY;
}
-   virtio_cwrite(vgdev->vdev, struct virtio_gpu_config,
- events_clear, _clear);
+   virtio_cwrite_le(vgdev->vdev, struct virtio_gpu_config,
+events_clear, _clear);
 }
 
 static void virtio_gpu_init_vq(struct virtio_gpu_queue *vgvq,
@@ -165,8 +165,8 @@ int virtio_gpu_init(struct drm_device *dev)
}
 
/* get display info */
-   virtio_cread(vgdev->vdev, struct virtio_gpu_config,
-num_scanouts, _scanouts);
+   virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
+   num_scanouts, _scanouts);
vgdev->num_scanouts = min_t(uint32_t, num_scanouts,
VIRTIO_GPU_MAX_SCANOUTS);
if (!vgdev->num_scanouts) {
@@ -176,8 +176,8 @@ int virtio_gpu_init(struct drm_device *dev)
}
DRM_INFO("number of scanouts: %d\n", num_scanouts);
 
-   virtio_cread(vgdev->vdev, struct virtio_gpu_config,
-num_capsets, _capsets);
+   virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
+   num_capsets, _capsets);
DRM_INFO("number of cap sets: %d\n", num_capsets);
 
virtio_gpu_modeset_init(vgdev);
-- 
MST

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


[PATCH v3 27/38] virtio_caif: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Tag config space fields as having virtio endian-ness.

Signed-off-by: Michael S. Tsirkin 
---
 include/linux/virtio_caif.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/virtio_caif.h b/include/linux/virtio_caif.h
index 5d2d3124ca3d..ea722479510c 100644
--- a/include/linux/virtio_caif.h
+++ b/include/linux/virtio_caif.h
@@ -11,9 +11,9 @@
 
 #include 
 struct virtio_caif_transf_config {
-   u16 headroom;
-   u16 tailroom;
-   u32 mtu;
+   __virtio16 headroom;
+   __virtio16 tailroom;
+   __virtio32 mtu;
u8 reserved[4];
 };
 
-- 
MST

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


[PATCH v3 31/38] virtio_fs: convert to LE accessors

2020-08-05 Thread Michael S. Tsirkin
Virtio fs is modern-only. Use LE accessors for config space.

Signed-off-by: Michael S. Tsirkin 
---
 fs/fuse/virtio_fs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 4c4ef5d69298..104f35de5270 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -606,8 +606,8 @@ static int virtio_fs_setup_vqs(struct virtio_device *vdev,
unsigned int i;
int ret = 0;
 
-   virtio_cread(vdev, struct virtio_fs_config, num_request_queues,
->num_request_queues);
+   virtio_cread_le(vdev, struct virtio_fs_config, num_request_queues,
+   >num_request_queues);
if (fs->num_request_queues == 0)
return -EINVAL;
 
-- 
MST

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


[PATCH v3 35/38] virtio_mem: convert to LE accessors

2020-08-05 Thread Michael S. Tsirkin
Virtio mem is modern-only. Use LE accessors for config space.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/virtio/virtio_mem.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index f26f5f64ae82..c08512fcea90 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -1530,21 +1530,21 @@ static void virtio_mem_refresh_config(struct virtio_mem 
*vm)
uint64_t new_plugged_size, usable_region_size, end_addr;
 
/* the plugged_size is just a reflection of what _we_ did previously */
-   virtio_cread(vm->vdev, struct virtio_mem_config, plugged_size,
-_plugged_size);
+   virtio_cread_le(vm->vdev, struct virtio_mem_config, plugged_size,
+   _plugged_size);
if (WARN_ON_ONCE(new_plugged_size != vm->plugged_size))
vm->plugged_size = new_plugged_size;
 
/* calculate the last usable memory block id */
-   virtio_cread(vm->vdev, struct virtio_mem_config,
-usable_region_size, _region_size);
+   virtio_cread_le(vm->vdev, struct virtio_mem_config,
+   usable_region_size, _region_size);
end_addr = vm->addr + usable_region_size;
end_addr = min(end_addr, phys_limit);
vm->last_usable_mb_id = virtio_mem_phys_to_mb_id(end_addr) - 1;
 
/* see if there is a request to change the size */
-   virtio_cread(vm->vdev, struct virtio_mem_config, requested_size,
->requested_size);
+   virtio_cread_le(vm->vdev, struct virtio_mem_config, requested_size,
+   >requested_size);
 
dev_info(>vdev->dev, "plugged size: 0x%llx", vm->plugged_size);
dev_info(>vdev->dev, "requested size: 0x%llx", vm->requested_size);
@@ -1677,16 +1677,16 @@ static int virtio_mem_init(struct virtio_mem *vm)
}
 
/* Fetch all properties that can't change. */
-   virtio_cread(vm->vdev, struct virtio_mem_config, plugged_size,
->plugged_size);
-   virtio_cread(vm->vdev, struct virtio_mem_config, block_size,
->device_block_size);
-   virtio_cread(vm->vdev, struct virtio_mem_config, node_id,
-_id);
+   virtio_cread_le(vm->vdev, struct virtio_mem_config, plugged_size,
+   >plugged_size);
+   virtio_cread_le(vm->vdev, struct virtio_mem_config, block_size,
+   >device_block_size);
+   virtio_cread_le(vm->vdev, struct virtio_mem_config, node_id,
+   _id);
vm->nid = virtio_mem_translate_node_id(vm, node_id);
-   virtio_cread(vm->vdev, struct virtio_mem_config, addr, >addr);
-   virtio_cread(vm->vdev, struct virtio_mem_config, region_size,
->region_size);
+   virtio_cread_le(vm->vdev, struct virtio_mem_config, addr, >addr);
+   virtio_cread_le(vm->vdev, struct virtio_mem_config, region_size,
+   >region_size);
 
/*
 * We always hotplug memory in memory block granularity. This way,
-- 
MST

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


[PATCH v3 30/38] virtio_input: convert to LE accessors

2020-08-05 Thread Michael S. Tsirkin
Virtio input is modern-only. Use LE accessors for config space.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/virtio/virtio_input.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
index efaf65b0f42d..877b2ea3ed05 100644
--- a/drivers/virtio/virtio_input.c
+++ b/drivers/virtio/virtio_input.c
@@ -113,9 +113,9 @@ static u8 virtinput_cfg_select(struct virtio_input *vi,
 {
u8 size;
 
-   virtio_cwrite(vi->vdev, struct virtio_input_config, select, );
-   virtio_cwrite(vi->vdev, struct virtio_input_config, subsel, );
-   virtio_cread(vi->vdev, struct virtio_input_config, size, );
+   virtio_cwrite_le(vi->vdev, struct virtio_input_config, select, );
+   virtio_cwrite_le(vi->vdev, struct virtio_input_config, subsel, );
+   virtio_cread_le(vi->vdev, struct virtio_input_config, size, );
return size;
 }
 
@@ -158,11 +158,11 @@ static void virtinput_cfg_abs(struct virtio_input *vi, 
int abs)
u32 mi, ma, re, fu, fl;
 
virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ABS_INFO, abs);
-   virtio_cread(vi->vdev, struct virtio_input_config, u.abs.min, );
-   virtio_cread(vi->vdev, struct virtio_input_config, u.abs.max, );
-   virtio_cread(vi->vdev, struct virtio_input_config, u.abs.res, );
-   virtio_cread(vi->vdev, struct virtio_input_config, u.abs.fuzz, );
-   virtio_cread(vi->vdev, struct virtio_input_config, u.abs.flat, );
+   virtio_cread_le(vi->vdev, struct virtio_input_config, u.abs.min, );
+   virtio_cread_le(vi->vdev, struct virtio_input_config, u.abs.max, );
+   virtio_cread_le(vi->vdev, struct virtio_input_config, u.abs.res, );
+   virtio_cread_le(vi->vdev, struct virtio_input_config, u.abs.fuzz, );
+   virtio_cread_le(vi->vdev, struct virtio_input_config, u.abs.flat, );
input_set_abs_params(vi->idev, abs, mi, ma, fu, fl);
input_abs_set_res(vi->idev, abs, re);
 }
@@ -244,14 +244,14 @@ static int virtinput_probe(struct virtio_device *vdev)
 
size = virtinput_cfg_select(vi, VIRTIO_INPUT_CFG_ID_DEVIDS, 0);
if (size >= sizeof(struct virtio_input_devids)) {
-   virtio_cread(vi->vdev, struct virtio_input_config,
-u.ids.bustype, >idev->id.bustype);
-   virtio_cread(vi->vdev, struct virtio_input_config,
-u.ids.vendor, >idev->id.vendor);
-   virtio_cread(vi->vdev, struct virtio_input_config,
-u.ids.product, >idev->id.product);
-   virtio_cread(vi->vdev, struct virtio_input_config,
-u.ids.version, >idev->id.version);
+   virtio_cread_le(vi->vdev, struct virtio_input_config,
+   u.ids.bustype, >idev->id.bustype);
+   virtio_cread_le(vi->vdev, struct virtio_input_config,
+   u.ids.vendor, >idev->id.vendor);
+   virtio_cread_le(vi->vdev, struct virtio_input_config,
+   u.ids.product, >idev->id.product);
+   virtio_cread_le(vi->vdev, struct virtio_input_config,
+   u.ids.version, >idev->id.version);
} else {
vi->idev->id.bustype = BUS_VIRTUAL;
}
-- 
MST

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


[PATCH v3 38/38] virtio_net: use LE accessors for speed/duplex

2020-08-05 Thread Michael S. Tsirkin
Speed and duplex config fields depend on VIRTIO_NET_F_SPEED_DUPLEX
which being 63>31 depends on VIRTIO_F_VERSION_1.

Accordingly, use LE accessors for these fields.

Reported-by: Cornelia Huck 
Signed-off-by: Michael S. Tsirkin 
---
 drivers/net/virtio_net.c| 9 +
 include/uapi/linux/virtio_net.h | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ba38765dc490..0934b1ec5320 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2264,12 +2264,13 @@ static void virtnet_update_settings(struct virtnet_info 
*vi)
if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX))
return;
 
-   speed = virtio_cread32(vi->vdev, offsetof(struct virtio_net_config,
- speed));
+   virtio_cread_le(vi->vdev, struct virtio_net_config, speed, );
+
if (ethtool_validate_speed(speed))
vi->speed = speed;
-   duplex = virtio_cread8(vi->vdev, offsetof(struct virtio_net_config,
- duplex));
+
+   virtio_cread_le(vi->vdev, struct virtio_net_config, duplex, );
+
if (ethtool_validate_duplex(duplex))
vi->duplex = duplex;
 }
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 27d996f29dd1..3f55a4215f11 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -99,7 +99,7 @@ struct virtio_net_config {
 * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
 * Any other value stands for unknown.
 */
-   __virtio32 speed;
+   __le32 speed;
/*
 * 0x00 - half duplex
 * 0x01 - full duplex
-- 
MST

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


[PATCH v3 37/38] virtio_config: drop LE option from config space

2020-08-05 Thread Michael S. Tsirkin
All drivers now use virtio_cread/write_le for LE config
space fields. Drop LE option from virtio_cread/write, only leaving
the option to access transitional fields.

Signed-off-by: Michael S. Tsirkin 
---
 include/linux/virtio_config.h | 28 ++--
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index cc7a2b1fd7b2..ecb166c824bb 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -293,19 +293,7 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
__u8: (x), \
__virtio16: virtio16_to_cpu((vdev), (x)), \
__virtio32: virtio32_to_cpu((vdev), (x)), \
-   __virtio64: virtio64_to_cpu((vdev), (x)), \
-   /*
-* Why define a default? checker can distinguish between
-* e.g. __u16, __le16 and __virtio16, but GCC can't so
-* attempts to define variants for both look like a duplicate
-* variant to it.
-*/ \
-   default: _Generic((x), \
-__u8: (x), \
-__le16: virtio16_to_cpu((vdev), (__force 
__virtio16)(x)), \
-__le32: virtio32_to_cpu((vdev), (__force 
__virtio32)(x)), \
-__le64: virtio64_to_cpu((vdev), (__force 
__virtio64)(x)) \
-) \
+   __virtio64: virtio64_to_cpu((vdev), (x)) \
)
 
 #define cpu_to_virtio(vdev, x, m) \
@@ -313,19 +301,7 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
__u8: (x), \
__virtio16: cpu_to_virtio16((vdev), (x)), \
__virtio32: cpu_to_virtio32((vdev), (x)), \
-   __virtio64: cpu_to_virtio64((vdev), (x)), \
-   /*
-* Why define a default? checker can distinguish between
-* e.g. __u16, __le16 and __virtio16, but GCC can't so
-* attempts to define variants for both look like a duplicate
-* variant to it.
-*/ \
-   default: _Generic((m), \
-__u8: (x), \
-__le16: (__force 
__le16)cpu_to_virtio16((vdev), (x)), \
-__le32: (__force 
__le32)cpu_to_virtio32((vdev), (x)), \
-__le64: (__force 
__le64)cpu_to_virtio64((vdev), (x)) \
-) \
+   __virtio64: cpu_to_virtio64((vdev), (x)) \
)
 
 #define __virtio_native_type(structname, member) \
-- 
MST

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


[PATCH v3 32/38] virtio_crypto: convert to LE accessors

2020-08-05 Thread Michael S. Tsirkin
Virtio crypto is modern-only. Use LE accessors for config space.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/crypto/virtio/virtio_crypto_core.c | 46 +++---
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/crypto/virtio/virtio_crypto_core.c 
b/drivers/crypto/virtio/virtio_crypto_core.c
index c8a962c62663..aeecce27fe8f 100644
--- a/drivers/crypto/virtio/virtio_crypto_core.c
+++ b/drivers/crypto/virtio/virtio_crypto_core.c
@@ -204,8 +204,8 @@ static int virtcrypto_update_status(struct virtio_crypto 
*vcrypto)
u32 status;
int err;
 
-   virtio_cread(vcrypto->vdev,
-   struct virtio_crypto_config, status, );
+   virtio_cread_le(vcrypto->vdev,
+   struct virtio_crypto_config, status, );
 
/*
 * Unknown status bits would be a host error and the driver
@@ -323,31 +323,31 @@ static int virtcrypto_probe(struct virtio_device *vdev)
if (!vcrypto)
return -ENOMEM;
 
-   virtio_cread(vdev, struct virtio_crypto_config,
+   virtio_cread_le(vdev, struct virtio_crypto_config,
max_dataqueues, _data_queues);
if (max_data_queues < 1)
max_data_queues = 1;
 
-   virtio_cread(vdev, struct virtio_crypto_config,
-   max_cipher_key_len, _cipher_key_len);
-   virtio_cread(vdev, struct virtio_crypto_config,
-   max_auth_key_len, _auth_key_len);
-   virtio_cread(vdev, struct virtio_crypto_config,
-   max_size, _size);
-   virtio_cread(vdev, struct virtio_crypto_config,
-   crypto_services, _services);
-   virtio_cread(vdev, struct virtio_crypto_config,
-   cipher_algo_l, _algo_l);
-   virtio_cread(vdev, struct virtio_crypto_config,
-   cipher_algo_h, _algo_h);
-   virtio_cread(vdev, struct virtio_crypto_config,
-   hash_algo, _algo);
-   virtio_cread(vdev, struct virtio_crypto_config,
-   mac_algo_l, _algo_l);
-   virtio_cread(vdev, struct virtio_crypto_config,
-   mac_algo_h, _algo_h);
-   virtio_cread(vdev, struct virtio_crypto_config,
-   aead_algo, _algo);
+   virtio_cread_le(vdev, struct virtio_crypto_config,
+   max_cipher_key_len, _cipher_key_len);
+   virtio_cread_le(vdev, struct virtio_crypto_config,
+   max_auth_key_len, _auth_key_len);
+   virtio_cread_le(vdev, struct virtio_crypto_config,
+   max_size, _size);
+   virtio_cread_le(vdev, struct virtio_crypto_config,
+   crypto_services, _services);
+   virtio_cread_le(vdev, struct virtio_crypto_config,
+   cipher_algo_l, _algo_l);
+   virtio_cread_le(vdev, struct virtio_crypto_config,
+   cipher_algo_h, _algo_h);
+   virtio_cread_le(vdev, struct virtio_crypto_config,
+   hash_algo, _algo);
+   virtio_cread_le(vdev, struct virtio_crypto_config,
+   mac_algo_l, _algo_l);
+   virtio_cread_le(vdev, struct virtio_crypto_config,
+   mac_algo_h, _algo_h);
+   virtio_cread_le(vdev, struct virtio_crypto_config,
+   aead_algo, _algo);
 
/* Add virtio crypto device to global table */
err = virtcrypto_devmgr_add_dev(vcrypto);
-- 
MST

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


[PATCH v3 36/38] virtio-iommu: convert to LE accessors

2020-08-05 Thread Michael S. Tsirkin
Virtio iommu is modern-only. Use LE accessors for config space.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/iommu/virtio-iommu.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index f6f07489a9aa..b4da396cce60 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -1010,8 +1010,8 @@ static int viommu_probe(struct virtio_device *vdev)
if (ret)
return ret;
 
-   virtio_cread(vdev, struct virtio_iommu_config, page_size_mask,
->pgsize_bitmap);
+   virtio_cread_le(vdev, struct virtio_iommu_config, page_size_mask,
+   >pgsize_bitmap);
 
if (!viommu->pgsize_bitmap) {
ret = -EINVAL;
@@ -1022,25 +1022,25 @@ static int viommu_probe(struct virtio_device *vdev)
viommu->last_domain = ~0U;
 
/* Optional features */
-   virtio_cread_feature(vdev, VIRTIO_IOMMU_F_INPUT_RANGE,
-struct virtio_iommu_config, input_range.start,
-_start);
+   virtio_cread_le_feature(vdev, VIRTIO_IOMMU_F_INPUT_RANGE,
+   struct virtio_iommu_config, input_range.start,
+   _start);
 
-   virtio_cread_feature(vdev, VIRTIO_IOMMU_F_INPUT_RANGE,
-struct virtio_iommu_config, input_range.end,
-_end);
+   virtio_cread_le_feature(vdev, VIRTIO_IOMMU_F_INPUT_RANGE,
+   struct virtio_iommu_config, input_range.end,
+   _end);
 
-   virtio_cread_feature(vdev, VIRTIO_IOMMU_F_DOMAIN_RANGE,
-struct virtio_iommu_config, domain_range.start,
->first_domain);
+   virtio_cread_le_feature(vdev, VIRTIO_IOMMU_F_DOMAIN_RANGE,
+   struct virtio_iommu_config, domain_range.start,
+   >first_domain);
 
-   virtio_cread_feature(vdev, VIRTIO_IOMMU_F_DOMAIN_RANGE,
-struct virtio_iommu_config, domain_range.end,
->last_domain);
+   virtio_cread_le_feature(vdev, VIRTIO_IOMMU_F_DOMAIN_RANGE,
+   struct virtio_iommu_config, domain_range.end,
+   >last_domain);
 
-   virtio_cread_feature(vdev, VIRTIO_IOMMU_F_PROBE,
-struct virtio_iommu_config, probe_size,
->probe_size);
+   virtio_cread_le_feature(vdev, VIRTIO_IOMMU_F_PROBE,
+   struct virtio_iommu_config, probe_size,
+   >probe_size);
 
viommu->geometry = (struct iommu_domain_geometry) {
.aperture_start = input_start,
-- 
MST

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


[PATCH v3 29/38] virtio_balloon: use LE config space accesses

2020-08-05 Thread Michael S. Tsirkin
Balloon is LE, it's cleaner to access it as such directly.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/virtio/virtio_balloon.c | 26 +-
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 8bc1704ffdf3..31cc97f2f515 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -398,12 +398,9 @@ static inline s64 towards_target(struct virtio_balloon *vb)
s64 target;
u32 num_pages;
 
-   virtio_cread(vb->vdev, struct virtio_balloon_config, num_pages,
-_pages);
-
/* Legacy balloon config space is LE, unlike all other devices. */
-   if (!virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1))
-   num_pages = le32_to_cpu((__force __le32)num_pages);
+   virtio_cread_le(vb->vdev, struct virtio_balloon_config, num_pages,
+   _pages);
 
target = num_pages;
return target - vb->num_pages;
@@ -462,11 +459,8 @@ static void update_balloon_size(struct virtio_balloon *vb)
u32 actual = vb->num_pages;
 
/* Legacy balloon config space is LE, unlike all other devices. */
-   if (!virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1))
-   actual = (__force u32)cpu_to_le32(actual);
-
-   virtio_cwrite(vb->vdev, struct virtio_balloon_config, actual,
- );
+   virtio_cwrite_le(vb->vdev, struct virtio_balloon_config, actual,
+);
 }
 
 static void update_balloon_stats_func(struct work_struct *work)
@@ -579,12 +573,10 @@ static u32 virtio_balloon_cmd_id_received(struct 
virtio_balloon *vb)
 {
if (test_and_clear_bit(VIRTIO_BALLOON_CONFIG_READ_CMD_ID,
   >config_read_bitmap)) {
-   virtio_cread(vb->vdev, struct virtio_balloon_config,
-free_page_hint_cmd_id,
->cmd_id_received_cache);
/* Legacy balloon config space is LE, unlike all other devices. 
*/
-   if (!virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1))
-   vb->cmd_id_received_cache = le32_to_cpu((__force 
__le32)vb->cmd_id_received_cache);
+   virtio_cread_le(vb->vdev, struct virtio_balloon_config,
+   free_page_hint_cmd_id,
+   >cmd_id_received_cache);
}
 
return vb->cmd_id_received_cache;
@@ -987,8 +979,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
if (!want_init_on_free())
memset(_val, PAGE_POISON, sizeof(poison_val));
 
-   virtio_cwrite(vb->vdev, struct virtio_balloon_config,
- poison_val, _val);
+   virtio_cwrite_le(vb->vdev, struct virtio_balloon_config,
+poison_val, _val);
}
 
vb->pr_dev_info.report = virtballoon_free_page_report;
-- 
MST

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


[PATCH v3 25/38] virtio_config: disallow native type fields (again)

2020-08-05 Thread Michael S. Tsirkin
_Generic version allowed __uXX types but that is no longer necessary:

Transitional devices should all use __virtioXX types (and __leXX for
fields not present in the legacy devices).
Modern ones should use __leXX.
_uXX type would be a bug.
Let's prevent that.

Signed-off-by: Michael S. Tsirkin 
---
 include/linux/virtio_config.h | 16 ++--
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 7fa000f02721..441fd6dd42ab 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -304,13 +304,7 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
 __u8: (x), \
 __le16: virtio16_to_cpu((vdev), (__force 
__virtio16)(x)), \
 __le32: virtio32_to_cpu((vdev), (__force 
__virtio32)(x)), \
-__le64: virtio64_to_cpu((vdev), (__force 
__virtio64)(x)), \
-default: _Generic((x), \
- __u8: (x), \
- __u16: 
virtio16_to_cpu((vdev), (__force __virtio16)(x)), \
- __u32: 
virtio32_to_cpu((vdev), (__force __virtio32)(x)), \
- __u64: 
virtio64_to_cpu((vdev), (__force __virtio64)(x)) \
- ) \
+__le64: virtio64_to_cpu((vdev), (__force 
__virtio64)(x)) \
 ) \
)
 
@@ -330,13 +324,7 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
 __u8: (x), \
 __le16: (__force 
__le16)cpu_to_virtio16((vdev), (x)), \
 __le32: (__force 
__le32)cpu_to_virtio32((vdev), (x)), \
-__le64: (__force 
__le64)cpu_to_virtio64((vdev), (x)), \
-default: _Generic((m), \
- __u8: (x), \
- __u16: (__force 
__u16)cpu_to_virtio16((vdev), (x)), \
- __u32: (__force 
__u32)cpu_to_virtio32((vdev), (x)), \
- __u64: (__force 
__u64)cpu_to_virtio64((vdev), (x)) \
- ) \
+__le64: (__force 
__le64)cpu_to_virtio64((vdev), (x)) \
 ) \
)
 
-- 
MST

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


[PATCH v3 28/38] virtio_config: add virtio_cread_le_feature

2020-08-05 Thread Michael S. Tsirkin
Mirrors virtio_cread_feature but for LE fields.

Signed-off-by: Michael S. Tsirkin 
---
 include/linux/virtio_config.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 5b5196fec899..cc7a2b1fd7b2 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -555,4 +555,14 @@ static inline void virtio_cwrite64(struct virtio_device 
*vdev,
_r; \
})
 
+/* Conditional config space accessors. */
+#define virtio_cread_le_feature(vdev, fbit, structname, member, ptr)   \
+   ({  \
+   int _r = 0; \
+   if (!virtio_has_feature(vdev, fbit))\
+   _r = -ENOENT;   \
+   else\
+   virtio_cread_le((vdev), structname, member, ptr); \
+   _r; \
+   })
 #endif /* _LINUX_VIRTIO_CONFIG_H */
-- 
MST

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


[PATCH v3 24/38] virtio_config: rewrite using _Generic

2020-08-05 Thread Michael S. Tsirkin
Min compiler version has been raised, so that's ok now.

Signed-off-by: Michael S. Tsirkin 
---
 include/linux/virtio_config.h | 163 --
 1 file changed, 77 insertions(+), 86 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 5c3b02245ecd..7fa000f02721 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -288,112 +288,103 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
return __cpu_to_virtio64(virtio_is_little_endian(vdev), val);
 }
 
-/*
- * Only the checker differentiates between __virtioXX and __uXX types. But we
- * try to share as much code as we can with the regular GCC build.
- */
-#if !defined(CONFIG_CC_IS_GCC) && !defined(__CHECKER__)
+#define virtio_to_cpu(vdev, x) \
+   _Generic((x), \
+   __u8: (x), \
+   __virtio16: virtio16_to_cpu((vdev), (x)), \
+   __virtio32: virtio32_to_cpu((vdev), (x)), \
+   __virtio64: virtio64_to_cpu((vdev), (x)), \
+   /*
+* Why define a default? checker can distinguish between
+* e.g. __u16, __le16 and __virtio16, but GCC can't so
+* attempts to define variants for both look like a duplicate
+* variant to it.
+*/ \
+   default: _Generic((x), \
+__u8: (x), \
+__le16: virtio16_to_cpu((vdev), (__force 
__virtio16)(x)), \
+__le32: virtio32_to_cpu((vdev), (__force 
__virtio32)(x)), \
+__le64: virtio64_to_cpu((vdev), (__force 
__virtio64)(x)), \
+default: _Generic((x), \
+ __u8: (x), \
+ __u16: 
virtio16_to_cpu((vdev), (__force __virtio16)(x)), \
+ __u32: 
virtio32_to_cpu((vdev), (__force __virtio32)(x)), \
+ __u64: 
virtio64_to_cpu((vdev), (__force __virtio64)(x)) \
+ ) \
+) \
+   )
 
-/* Not a checker - we can keep things simple */
-#define __virtio_native_typeof(x) typeof(x)
-
-#else
-
-/*
- * We build this out of a couple of helper macros in a vain attempt to
- * help you keep your lunch down while reading it.
- */
-#define __virtio_pick_value(x, type, then, otherwise)  \
-   __builtin_choose_expr(__same_type(x, type), then, otherwise)
-
-#define __virtio_pick_type(x, type, then, otherwise)   \
-   __virtio_pick_value(x, type, (then)0, otherwise)
-
-#define __virtio_pick_endian(x, x16, x32, x64, otherwise)  
\
-   __virtio_pick_type(x, x16, __u16,   
\
-   __virtio_pick_type(x, x32, __u32,   
\
-   __virtio_pick_type(x, x64, __u64,   
\
-   otherwise)))
-
-#define __virtio_native_typeof(x) typeof(  
\
-   __virtio_pick_type(x, __u8, __u8,   
\
-   __virtio_pick_endian(x, __virtio16, __virtio32, __virtio64, 
\
-   __virtio_pick_endian(x, __le16, __le32, __le64, 
\
-   /* No other type allowed */ 
\
-   (void)0
-
-#endif
+#define cpu_to_virtio(vdev, x, m) \
+   _Generic((m), \
+   __u8: (x), \
+   __virtio16: cpu_to_virtio16((vdev), (x)), \
+   __virtio32: cpu_to_virtio32((vdev), (x)), \
+   __virtio64: cpu_to_virtio64((vdev), (x)), \
+   /*
+* Why define a default? checker can distinguish between
+* e.g. __u16, __le16 and __virtio16, but GCC can't so
+* attempts to define variants for both look like a duplicate
+* variant to it.
+*/ \
+   default: _Generic((m), \
+__u8: (x), \
+__le16: (__force 
__le16)cpu_to_virtio16((vdev), (x)), \
+__le32: (__force 
__le32)cpu_to_virtio32((vdev), (x)), \
+__le64: (__force 
__le64)cpu_to_virtio64((vdev), (x)), \
+default: _Generic((m), \
+ __u8: (x), \
+ __u16: (__force 
__u16)cpu_to_virtio16((vdev), (x)), \
+ __u32: (__force 
__u32)cpu_to_virtio32((vdev), (x)), \
+ __u64: (__forc

[PATCH v3 10/38] virtio_gpu: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Since gpu is a modern-only device,
tag config space fields as having little endian-ness.

Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Cornelia Huck 
---
 include/uapi/linux/virtio_gpu.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
index 0c85914d9369..ccbd174ef321 100644
--- a/include/uapi/linux/virtio_gpu.h
+++ b/include/uapi/linux/virtio_gpu.h
@@ -320,10 +320,10 @@ struct virtio_gpu_resp_edid {
 #define VIRTIO_GPU_EVENT_DISPLAY (1 << 0)
 
 struct virtio_gpu_config {
-   __u32 events_read;
-   __u32 events_clear;
-   __u32 num_scanouts;
-   __u32 num_capsets;
+   __le32 events_read;
+   __le32 events_clear;
+   __le32 num_scanouts;
+   __le32 num_capsets;
 };
 
 /* simple formats for fbcon/X use */
-- 
MST

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


[PATCH v3 09/38] virtio_fs: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Since fs is a modern-only device,
tag config space fields as having little endian-ness.

Signed-off-by: Michael S. Tsirkin 
Acked-by: Vivek Goyal 
Acked-by: Vivek Goyal 
Reviewed-by: Cornelia Huck 
---
 include/uapi/linux/virtio_fs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/virtio_fs.h b/include/uapi/linux/virtio_fs.h
index b02eb2ac3d99..3056b6e9f8ce 100644
--- a/include/uapi/linux/virtio_fs.h
+++ b/include/uapi/linux/virtio_fs.h
@@ -13,7 +13,7 @@ struct virtio_fs_config {
__u8 tag[36];
 
/* Number of request queues */
-   __u32 num_request_queues;
+   __le32 num_request_queues;
 } __attribute__((packed));
 
 #endif /* _UAPI_LINUX_VIRTIO_FS_H */
-- 
MST

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


[PATCH v3 13/38] virtio_mem: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Since this is a modern-only device,
tag config space fields as having little endian-ness.

TODO: check other uses of __virtioXX types in this header,
should probably be __leXX.

Signed-off-by: Michael S. Tsirkin 
Acked-by: David Hildenbrand 
Reviewed-by: Cornelia Huck 
---
 include/uapi/linux/virtio_mem.h | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/virtio_mem.h b/include/uapi/linux/virtio_mem.h
index a9ffe041843c..70e01c687d5e 100644
--- a/include/uapi/linux/virtio_mem.h
+++ b/include/uapi/linux/virtio_mem.h
@@ -185,27 +185,27 @@ struct virtio_mem_resp {
 
 struct virtio_mem_config {
/* Block size and alignment. Cannot change. */
-   __u64 block_size;
+   __le64 block_size;
/* Valid with VIRTIO_MEM_F_ACPI_PXM. Cannot change. */
-   __u16 node_id;
+   __le16 node_id;
__u8 padding[6];
/* Start address of the memory region. Cannot change. */
-   __u64 addr;
+   __le64 addr;
/* Region size (maximum). Cannot change. */
-   __u64 region_size;
+   __le64 region_size;
/*
 * Currently usable region size. Can grow up to region_size. Can
 * shrink due to VIRTIO_MEM_REQ_UNPLUG_ALL (in which case no config
 * update will be sent).
 */
-   __u64 usable_region_size;
+   __le64 usable_region_size;
/*
 * Currently used size. Changes due to plug/unplug requests, but no
 * config updates will be sent.
 */
-   __u64 plugged_size;
+   __le64 plugged_size;
/* Requested size. New plug requests cannot exceed it. Can change. */
-   __u64 requested_size;
+   __le64 requested_size;
 };
 
 #endif /* _LINUX_VIRTIO_MEM_H */
-- 
MST

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


[PATCH v3 19/38] vdpa: make sure set_features is invoked for legacy

2020-08-05 Thread Michael S. Tsirkin
Some legacy guests just assume features are 0 after reset.
We detect that config space is accessed before features are
set and set features to 0 automatically.
Note: some legacy guests might not even access config space, if this is
reported in the field we might need to catch a kick to handle these.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/vdpa/vdpa.c  |  1 +
 include/linux/vdpa.h | 34 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index de211ef3738c..7105265e4793 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -96,6 +96,7 @@ struct vdpa_device *__vdpa_alloc_device(struct device *parent,
vdev->dev.release = vdpa_release_dev;
vdev->index = err;
vdev->config = config;
+   vdev->features_valid = false;
 
err = dev_set_name(>dev, "vdpa%u", vdev->index);
if (err)
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 239db794357c..29b8296f1414 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -33,12 +33,14 @@ struct vdpa_notification_area {
  * @dma_dev: the actual device that is performing DMA
  * @config: the configuration ops for this device.
  * @index: device index
+ * @features_valid: were features initialized? for legacy guests
  */
 struct vdpa_device {
struct device dev;
struct device *dma_dev;
const struct vdpa_config_ops *config;
unsigned int index;
+   bool features_valid;
 };
 
 /**
@@ -266,4 +268,36 @@ static inline struct device *vdpa_get_dma_dev(struct 
vdpa_device *vdev)
 {
return vdev->dma_dev;
 }
+
+static inline void vdpa_reset(struct vdpa_device *vdev)
+{
+const struct vdpa_config_ops *ops = vdev->config;
+
+   vdev->features_valid = false;
+ops->set_status(vdev, 0);
+}
+
+static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)
+{
+const struct vdpa_config_ops *ops = vdev->config;
+
+   vdev->features_valid = true;
+return ops->set_features(vdev, features);
+}
+
+
+static inline void vdpa_get_config(struct vdpa_device *vdev, unsigned offset,
+  void *buf, unsigned int len)
+{
+const struct vdpa_config_ops *ops = vdev->config;
+
+   /*
+* Config accesses aren't supposed to trigger before features are set.
+* If it does happen we assume a legacy guest.
+*/
+   if (!vdev->features_valid)
+   vdpa_set_features(vdev, 0);
+   ops->get_config(vdev, offset, buf, len);
+}
+
 #endif /* _LINUX_VDPA_H */
-- 
MST

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


[PATCH v3 12/38] virtio_iommu: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Since this is a modern-only device,
tag config space fields as having little endian-ness.

Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Jean-Philippe Brucker 
Reviewed-by: Jean-Philippe Brucker 
Reviewed-by: Cornelia Huck 
---
 include/uapi/linux/virtio_iommu.h | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/virtio_iommu.h 
b/include/uapi/linux/virtio_iommu.h
index 48e3c29223b5..237e36a280cb 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -18,24 +18,24 @@
 #define VIRTIO_IOMMU_F_MMIO5
 
 struct virtio_iommu_range_64 {
-   __u64   start;
-   __u64   end;
+   __le64  start;
+   __le64  end;
 };
 
 struct virtio_iommu_range_32 {
-   __u32   start;
-   __u32   end;
+   __le32  start;
+   __le32  end;
 };
 
 struct virtio_iommu_config {
/* Supported page sizes */
-   __u64   page_size_mask;
+   __le64  page_size_mask;
/* Supported IOVA range */
struct virtio_iommu_range_64input_range;
/* Max domain ID size */
struct virtio_iommu_range_32domain_range;
/* Probe buffer size */
-   __u32   probe_size;
+   __le32  probe_size;
 };
 
 /* Request types */
-- 
MST

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


[PATCH v3 23/38] virtio_config: cread/write cleanup

2020-08-05 Thread Michael S. Tsirkin
Use vars of the correct type instead of casting.

Signed-off-by: Michael S. Tsirkin 
---
 include/linux/virtio_config.h | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index c68f58f3bf34..5c3b02245ecd 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -444,53 +444,60 @@ static inline void virtio_cwrite8(struct virtio_device 
*vdev,
 static inline u16 virtio_cread16(struct virtio_device *vdev,
 unsigned int offset)
 {
-   u16 ret;
+   __virtio16 ret;
 
might_sleep();
vdev->config->get(vdev, offset, , sizeof(ret));
-   return virtio16_to_cpu(vdev, (__force __virtio16)ret);
+   return virtio16_to_cpu(vdev, ret);
 }
 
 static inline void virtio_cwrite16(struct virtio_device *vdev,
   unsigned int offset, u16 val)
 {
+   __virtio16 v;
+
might_sleep();
-   val = (__force u16)cpu_to_virtio16(vdev, val);
-   vdev->config->set(vdev, offset, , sizeof(val));
+   v = cpu_to_virtio16(vdev, val);
+   vdev->config->set(vdev, offset, , sizeof(v));
 }
 
 static inline u32 virtio_cread32(struct virtio_device *vdev,
 unsigned int offset)
 {
-   u32 ret;
+   __virtio32 ret;
 
might_sleep();
vdev->config->get(vdev, offset, , sizeof(ret));
-   return virtio32_to_cpu(vdev, (__force __virtio32)ret);
+   return virtio32_to_cpu(vdev, ret);
 }
 
 static inline void virtio_cwrite32(struct virtio_device *vdev,
   unsigned int offset, u32 val)
 {
+   __virtio32 v;
+
might_sleep();
-   val = (__force u32)cpu_to_virtio32(vdev, val);
-   vdev->config->set(vdev, offset, , sizeof(val));
+   v = cpu_to_virtio32(vdev, val);
+   vdev->config->set(vdev, offset, , sizeof(v));
 }
 
 static inline u64 virtio_cread64(struct virtio_device *vdev,
 unsigned int offset)
 {
-   u64 ret;
+   __virtio64 ret;
+
__virtio_cread_many(vdev, offset, , 1, sizeof(ret));
-   return virtio64_to_cpu(vdev, (__force __virtio64)ret);
+   return virtio64_to_cpu(vdev, ret);
 }
 
 static inline void virtio_cwrite64(struct virtio_device *vdev,
   unsigned int offset, u64 val)
 {
+   __virtio64 v;
+
might_sleep();
-   val = (__force u64)cpu_to_virtio64(vdev, val);
-   vdev->config->set(vdev, offset, , sizeof(val));
+   v = cpu_to_virtio64(vdev, val);
+   vdev->config->set(vdev, offset, , sizeof(v));
 }
 
 /* Conditional config space accessors. */
-- 
MST

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


[PATCH v3 08/38] virtio_crypto: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Since crypto is a modern-only device,
tag config space fields as having little endian-ness.

Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Cornelia Huck 
---
 include/uapi/linux/virtio_crypto.h | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/virtio_crypto.h 
b/include/uapi/linux/virtio_crypto.h
index 50cdc8aebfcf..a03932f10565 100644
--- a/include/uapi/linux/virtio_crypto.h
+++ b/include/uapi/linux/virtio_crypto.h
@@ -414,33 +414,33 @@ struct virtio_crypto_op_data_req {
 
 struct virtio_crypto_config {
/* See VIRTIO_CRYPTO_OP_* above */
-   __u32  status;
+   __le32  status;
 
/*
 * Maximum number of data queue
 */
-   __u32  max_dataqueues;
+   __le32  max_dataqueues;
 
/*
 * Specifies the services mask which the device support,
 * see VIRTIO_CRYPTO_SERVICE_* above
 */
-   __u32 crypto_services;
+   __le32 crypto_services;
 
/* Detailed algorithms mask */
-   __u32 cipher_algo_l;
-   __u32 cipher_algo_h;
-   __u32 hash_algo;
-   __u32 mac_algo_l;
-   __u32 mac_algo_h;
-   __u32 aead_algo;
+   __le32 cipher_algo_l;
+   __le32 cipher_algo_h;
+   __le32 hash_algo;
+   __le32 mac_algo_l;
+   __le32 mac_algo_h;
+   __le32 aead_algo;
/* Maximum length of cipher key */
-   __u32 max_cipher_key_len;
+   __le32 max_cipher_key_len;
/* Maximum length of authenticated key */
-   __u32 max_auth_key_len;
-   __u32 reserve;
+   __le32 max_auth_key_len;
+   __le32 reserve;
/* Maximum size of each crypto request's content */
-   __u64 max_size;
+   __le64 max_size;
 };
 
 struct virtio_crypto_inhdr {
-- 
MST

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


[PATCH v3 21/38] virtio_vdpa: legacy features handling

2020-08-05 Thread Michael S. Tsirkin
We normally expect vdpa to use the modern interface.
However for consistency, let's use same APIs as vhost
for legacy guests.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/virtio/virtio_vdpa.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index c30eb55030be..4a9ddb44b2a7 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -57,9 +57,8 @@ static void virtio_vdpa_get(struct virtio_device *vdev, 
unsigned offset,
void *buf, unsigned len)
 {
struct vdpa_device *vdpa = vd_get_vdpa(vdev);
-   const struct vdpa_config_ops *ops = vdpa->config;
 
-   ops->get_config(vdpa, offset, buf, len);
+   vdpa_get_config(vdpa, offset, buf, len);
 }
 
 static void virtio_vdpa_set(struct virtio_device *vdev, unsigned offset,
@@ -101,9 +100,8 @@ static void virtio_vdpa_set_status(struct virtio_device 
*vdev, u8 status)
 static void virtio_vdpa_reset(struct virtio_device *vdev)
 {
struct vdpa_device *vdpa = vd_get_vdpa(vdev);
-   const struct vdpa_config_ops *ops = vdpa->config;
 
-   return ops->set_status(vdpa, 0);
+   vdpa_reset(vdpa);
 }
 
 static bool virtio_vdpa_notify(struct virtqueue *vq)
@@ -294,12 +292,11 @@ static u64 virtio_vdpa_get_features(struct virtio_device 
*vdev)
 static int virtio_vdpa_finalize_features(struct virtio_device *vdev)
 {
struct vdpa_device *vdpa = vd_get_vdpa(vdev);
-   const struct vdpa_config_ops *ops = vdpa->config;
 
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
 
-   return ops->set_features(vdpa, vdev->features);
+   return vdpa_set_features(vdpa, vdev->features);
 }
 
 static const char *virtio_vdpa_bus_name(struct virtio_device *vdev)
-- 
MST

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


[PATCH v3 20/38] vhost/vdpa: switch to new helpers

2020-08-05 Thread Michael S. Tsirkin
For new helpers handling legacy features to be effective,
vhost needs to invoke them. Tie them in.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/vhost/vdpa.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 18869a35d408..3674404688f5 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -118,9 +118,8 @@ static irqreturn_t vhost_vdpa_config_cb(void *private)
 static void vhost_vdpa_reset(struct vhost_vdpa *v)
 {
struct vdpa_device *vdpa = v->vdpa;
-   const struct vdpa_config_ops *ops = vdpa->config;
 
-   ops->set_status(vdpa, 0);
+   vdpa_reset(vdpa);
 }
 
 static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user *argp)
@@ -196,7 +195,6 @@ static long vhost_vdpa_get_config(struct vhost_vdpa *v,
  struct vhost_vdpa_config __user *c)
 {
struct vdpa_device *vdpa = v->vdpa;
-   const struct vdpa_config_ops *ops = vdpa->config;
struct vhost_vdpa_config config;
unsigned long size = offsetof(struct vhost_vdpa_config, buf);
u8 *buf;
@@ -209,7 +207,7 @@ static long vhost_vdpa_get_config(struct vhost_vdpa *v,
if (!buf)
return -ENOMEM;
 
-   ops->get_config(vdpa, config.off, buf, config.len);
+   vdpa_get_config(vdpa, config.off, buf, config.len);
 
if (copy_to_user(c->buf, buf, config.len)) {
kvfree(buf);
@@ -282,7 +280,7 @@ static long vhost_vdpa_set_features(struct vhost_vdpa *v, 
u64 __user *featurep)
if (features & ~vhost_vdpa_features[v->virtio_id])
return -EINVAL;
 
-   if (ops->set_features(vdpa, features))
+   if (vdpa_set_features(vdpa, features))
return -EINVAL;
 
return 0;
-- 
MST

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


[PATCH v3 17/38] virtio_config: disallow native type fields

2020-08-05 Thread Michael S. Tsirkin
Transitional devices should all use __virtioXX types (and __leXX for
fields not present in legacy devices).
Modern ones should use __leXX.
_uXX type would be a bug.
Let's prevent that.

Signed-off-by: Michael S. Tsirkin 
---
 include/linux/virtio_config.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 64da491936f7..c68f58f3bf34 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -319,9 +319,8 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
__virtio_pick_type(x, __u8, __u8,   
\
__virtio_pick_endian(x, __virtio16, __virtio32, __virtio64, 
\
__virtio_pick_endian(x, __le16, __le32, __le64, 
\
-   __virtio_pick_endian(x, __u16, __u32, __u64,
\
-   /* No other type allowed */ 
\
-   (void)0)
+   /* No other type allowed */ 
\
+   (void)0
 
 #endif
 
-- 
MST

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


[PATCH v3 16/38] virtio_scsi: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Tag config space fields as having virtio endian-ness.

Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Cornelia Huck 
---
 drivers/scsi/virtio_scsi.c   |  4 ++--
 include/uapi/linux/virtio_scsi.h | 20 ++--
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 0e0910c5b942..c36aeb9a1330 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -746,14 +746,14 @@ static struct scsi_host_template virtscsi_host_template = 
{
 
 #define virtscsi_config_get(vdev, fld) \
({ \
-   typeof(((struct virtio_scsi_config *)0)->fld) __val; \
+   __virtio_native_type(struct virtio_scsi_config, fld) __val; \
virtio_cread(vdev, struct virtio_scsi_config, fld, &__val); \
__val; \
})
 
 #define virtscsi_config_set(vdev, fld, val) \
do { \
-   typeof(((struct virtio_scsi_config *)0)->fld) __val = (val); \
+   __virtio_native_type(struct virtio_scsi_config, fld) __val = 
(val); \
virtio_cwrite(vdev, struct virtio_scsi_config, fld, &__val); \
} while(0)
 
diff --git a/include/uapi/linux/virtio_scsi.h b/include/uapi/linux/virtio_scsi.h
index cc18ef8825c0..0abaae4027c0 100644
--- a/include/uapi/linux/virtio_scsi.h
+++ b/include/uapi/linux/virtio_scsi.h
@@ -103,16 +103,16 @@ struct virtio_scsi_event {
 } __attribute__((packed));
 
 struct virtio_scsi_config {
-   __u32 num_queues;
-   __u32 seg_max;
-   __u32 max_sectors;
-   __u32 cmd_per_lun;
-   __u32 event_info_size;
-   __u32 sense_size;
-   __u32 cdb_size;
-   __u16 max_channel;
-   __u16 max_target;
-   __u32 max_lun;
+   __virtio32 num_queues;
+   __virtio32 seg_max;
+   __virtio32 max_sectors;
+   __virtio32 cmd_per_lun;
+   __virtio32 event_info_size;
+   __virtio32 sense_size;
+   __virtio32 cdb_size;
+   __virtio16 max_channel;
+   __virtio16 max_target;
+   __virtio32 max_lun;
 } __attribute__((packed));
 
 /* Feature Bits */
-- 
MST

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


[PATCH v3 14/38] virtio_net: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Tag config space fields as having virtio endian-ness.

Signed-off-by: Michael S. Tsirkin 
---
 include/uapi/linux/virtio_net.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 19d23e5baa4e..27d996f29dd1 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -87,19 +87,19 @@ struct virtio_net_config {
/* The config defining mac address (if VIRTIO_NET_F_MAC) */
__u8 mac[ETH_ALEN];
/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
-   __u16 status;
+   __virtio16 status;
/* Maximum number of each of transmit and receive queues;
 * see VIRTIO_NET_F_MQ and VIRTIO_NET_CTRL_MQ.
 * Legal values are between 1 and 0x8000
 */
-   __u16 max_virtqueue_pairs;
+   __virtio16 max_virtqueue_pairs;
/* Default maximum transmit unit advice */
-   __u16 mtu;
+   __virtio16 mtu;
/*
 * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
 * Any other value stands for unknown.
 */
-   __u32 speed;
+   __virtio32 speed;
/*
 * 0x00 - half duplex
 * 0x01 - full duplex
-- 
MST

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


[PATCH v3 22/38] vdpa_sim: fix endian-ness of config space

2020-08-05 Thread Michael S. Tsirkin
VDPA sim accesses config space as native endian - this is
wrong since it's a modern device and actually uses LE.

It only supports modern guests so we could punt and
just force LE, but let's use the full virtio APIs since people
tend to copy/paste code, and this is not data path anyway.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 +++-
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index a9bc5e0fb353..b7d5727fde4c 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -72,6 +73,23 @@ struct vdpasim {
u64 features;
 };
 
+/* TODO: cross-endian support */
+static inline bool vdpasim_is_little_endian(struct vdpasim *vdpasim)
+{
+   return virtio_legacy_is_little_endian() ||
+   (vdpasim->features & (1ULL << VIRTIO_F_VERSION_1));
+}
+
+static inline u16 vdpasim16_to_cpu(struct vdpasim *vdpasim, __virtio16 val)
+{
+   return __virtio16_to_cpu(vdpasim_is_little_endian(vdpasim), val);
+}
+
+static inline __virtio16 cpu_to_vdpasim16(struct vdpasim *vdpasim, u16 val)
+{
+   return __cpu_to_virtio16(vdpasim_is_little_endian(vdpasim), val);
+}
+
 static struct vdpasim *vdpasim_dev;
 
 static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa)
@@ -306,7 +324,6 @@ static const struct vdpa_config_ops vdpasim_net_config_ops;
 
 static struct vdpasim *vdpasim_create(void)
 {
-   struct virtio_net_config *config;
struct vdpasim *vdpasim;
struct device *dev;
int ret = -ENOMEM;
@@ -331,10 +348,7 @@ static struct vdpasim *vdpasim_create(void)
if (!vdpasim->buffer)
goto err_iommu;
 
-   config = >config;
-   config->mtu = 1500;
-   config->status = VIRTIO_NET_S_LINK_UP;
-   eth_random_addr(config->mac);
+   eth_random_addr(vdpasim->config.mac);
 
vringh_set_iotlb(>vqs[0].vring, vdpasim->iommu);
vringh_set_iotlb(>vqs[1].vring, vdpasim->iommu);
@@ -448,6 +462,7 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
 static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
 {
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
+   struct virtio_net_config *config = >config;
 
/* DMA mapping must be done by driver */
if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
@@ -455,6 +470,14 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, 
u64 features)
 
vdpasim->features = features & vdpasim_features;
 
+   /* We generally only know whether guest is using the legacy interface
+* here, so generally that's the earliest we can set config fields.
+* Note: We actually require VIRTIO_F_ACCESS_PLATFORM above which
+* implies VIRTIO_F_VERSION_1, but let's not try to be clever here.
+*/
+
+   config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
+   config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
return 0;
 }
 
-- 
MST

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


[PATCH v3 26/38] virtio_config: LE config space accessors

2020-08-05 Thread Michael S. Tsirkin
To be used by modern code, as well as to handle LE only fields such as
balloon.

Signed-off-by: Michael S. Tsirkin 
---
 include/linux/virtio_config.h | 65 +++
 1 file changed, 65 insertions(+)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 441fd6dd42ab..5b5196fec899 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -375,6 +375,71 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
  sizeof(virtio_cwrite_v)); \
} while(0)
 
+/*
+ * Nothing virtio-specific about these, but let's worry about generalizing
+ * these later.
+ */
+#define virtio_le_to_cpu(x) \
+   _Generic((x), \
+   __u8: (x), \
+__le16: le16_to_cpu(x), \
+__le32: le32_to_cpu(x), \
+__le64: le64_to_cpu(x) \
+   )
+
+#define virtio_cpu_to_le(x, m) \
+   _Generic((m), \
+__u8: (x), \
+__le16: cpu_to_le16(x), \
+__le32: cpu_to_le32(x), \
+__le64: cpu_to_le64(x) \
+   )
+
+/* LE (e.g. modern) Config space accessors. */
+#define virtio_cread_le(vdev, structname, member, ptr) \
+   do {\
+   typeof(((structname*)0)->member) virtio_cread_v;\
+   \
+   might_sleep();  \
+   /* Sanity check: must match the member's type */\
+   typecheck(typeof(virtio_le_to_cpu(virtio_cread_v)), *(ptr)); \
+   \
+   switch (sizeof(virtio_cread_v)) {   \
+   case 1: \
+   case 2: \
+   case 4: \
+   vdev->config->get((vdev),   \
+ offsetof(structname, member), \
+ _cread_v,  \
+ sizeof(virtio_cread_v));  \
+   break;  \
+   default:\
+   __virtio_cread_many((vdev), \
+ offsetof(structname, member), \
+ _cread_v,  \
+ 1,\
+ sizeof(virtio_cread_v));  \
+   break;  \
+   }   \
+   *(ptr) = virtio_le_to_cpu(virtio_cread_v);  \
+   } while(0)
+
+/* Config space accessors. */
+#define virtio_cwrite_le(vdev, structname, member, ptr)
\
+   do {\
+   typeof(((structname*)0)->member) virtio_cwrite_v =  \
+   virtio_cpu_to_le(*(ptr), ((structname*)0)->member); \
+   \
+   might_sleep();  \
+   /* Sanity check: must match the member's type */\
+   typecheck(typeof(virtio_le_to_cpu(virtio_cwrite_v)), *(ptr)); \
+   \
+   vdev->config->set((vdev), offsetof(structname, member), \
+ _cwrite_v, \
+ sizeof(virtio_cwrite_v)); \
+   } while(0)
+
+
 /* Read @count fields, @bytes each. */
 static inline void __virtio_cread_many(struct virtio_device *vdev,
   unsigned int offset,
-- 
MST

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


[PATCH v3 06/38] virtio_blk: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Tag config space fields as having virtio endian-ness.

Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Cornelia Huck 
---
 include/uapi/linux/virtio_blk.h | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/virtio_blk.h b/include/uapi/linux/virtio_blk.h
index 0f99d7b49ede..d888f013d9ff 100644
--- a/include/uapi/linux/virtio_blk.h
+++ b/include/uapi/linux/virtio_blk.h
@@ -57,20 +57,20 @@
 
 struct virtio_blk_config {
/* The capacity (in 512-byte sectors). */
-   __u64 capacity;
+   __virtio64 capacity;
/* The maximum segment size (if VIRTIO_BLK_F_SIZE_MAX) */
-   __u32 size_max;
+   __virtio32 size_max;
/* The maximum number of segments (if VIRTIO_BLK_F_SEG_MAX) */
-   __u32 seg_max;
+   __virtio32 seg_max;
/* geometry of the device (if VIRTIO_BLK_F_GEOMETRY) */
struct virtio_blk_geometry {
-   __u16 cylinders;
+   __virtio16 cylinders;
__u8 heads;
__u8 sectors;
} geometry;
 
/* block size of device (if VIRTIO_BLK_F_BLK_SIZE) */
-   __u32 blk_size;
+   __virtio32 blk_size;
 
/* the next 4 entries are guarded by VIRTIO_BLK_F_TOPOLOGY  */
/* exponent for physical block per logical block. */
@@ -78,42 +78,42 @@ struct virtio_blk_config {
/* alignment offset in logical blocks. */
__u8 alignment_offset;
/* minimum I/O size without performance penalty in logical blocks. */
-   __u16 min_io_size;
+   __virtio16 min_io_size;
/* optimal sustained I/O size in logical blocks. */
-   __u32 opt_io_size;
+   __virtio32 opt_io_size;
 
/* writeback mode (if VIRTIO_BLK_F_CONFIG_WCE) */
__u8 wce;
__u8 unused;
 
/* number of vqs, only available when VIRTIO_BLK_F_MQ is set */
-   __u16 num_queues;
+   __virtio16 num_queues;
 
/* the next 3 entries are guarded by VIRTIO_BLK_F_DISCARD */
/*
 * The maximum discard sectors (in 512-byte sectors) for
 * one segment.
 */
-   __u32 max_discard_sectors;
+   __virtio32 max_discard_sectors;
/*
 * The maximum number of discard segments in a
 * discard command.
 */
-   __u32 max_discard_seg;
+   __virtio32 max_discard_seg;
/* Discard commands must be aligned to this number of sectors. */
-   __u32 discard_sector_alignment;
+   __virtio32 discard_sector_alignment;
 
/* the next 3 entries are guarded by VIRTIO_BLK_F_WRITE_ZEROES */
/*
 * The maximum number of write zeroes sectors (in 512-byte sectors) in
 * one segment.
 */
-   __u32 max_write_zeroes_sectors;
+   __virtio32 max_write_zeroes_sectors;
/*
 * The maximum number of segments in a write zeroes
 * command.
 */
-   __u32 max_write_zeroes_seg;
+   __virtio32 max_write_zeroes_seg;
/*
 * Set if a VIRTIO_BLK_T_WRITE_ZEROES request may result in the
 * deallocation of one or more of the sectors.
-- 
MST

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


[PATCH v3 07/38] virtio_console: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Tag config space fields as having virtio endian-ness.

Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Cornelia Huck 
---
 include/uapi/linux/virtio_console.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/virtio_console.h 
b/include/uapi/linux/virtio_console.h
index b7fb108c9310..7e6ec2ff0560 100644
--- a/include/uapi/linux/virtio_console.h
+++ b/include/uapi/linux/virtio_console.h
@@ -45,13 +45,13 @@
 
 struct virtio_console_config {
/* colums of the screens */
-   __u16 cols;
+   __virtio16 cols;
/* rows of the screens */
-   __u16 rows;
+   __virtio16 rows;
/* max. number of ports this device can hold */
-   __u32 max_nr_ports;
+   __virtio32 max_nr_ports;
/* emergency write register */
-   __u32 emerg_wr;
+   __virtio32 emerg_wr;
 } __attribute__((packed));
 
 /*
-- 
MST

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


[PATCH v3 11/38] virtio_input: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Since this is a modern-only device,
tag config space fields as having little endian-ness.

Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Gerd Hoffmann 
Reviewed-by: Cornelia Huck 
---
 include/uapi/linux/virtio_input.h | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/uapi/linux/virtio_input.h 
b/include/uapi/linux/virtio_input.h
index a7fe5c8fb135..52084b1fb965 100644
--- a/include/uapi/linux/virtio_input.h
+++ b/include/uapi/linux/virtio_input.h
@@ -40,18 +40,18 @@ enum virtio_input_config_select {
 };
 
 struct virtio_input_absinfo {
-   __u32 min;
-   __u32 max;
-   __u32 fuzz;
-   __u32 flat;
-   __u32 res;
+   __le32 min;
+   __le32 max;
+   __le32 fuzz;
+   __le32 flat;
+   __le32 res;
 };
 
 struct virtio_input_devids {
-   __u16 bustype;
-   __u16 vendor;
-   __u16 product;
-   __u16 version;
+   __le16 bustype;
+   __le16 vendor;
+   __le16 product;
+   __le16 version;
 };
 
 struct virtio_input_config {
-- 
MST

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


[PATCH v3 15/38] virtio_pmem: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Since this is a modern-only device,
tag config space fields as having little endian-ness.

Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Cornelia Huck 
---
 include/uapi/linux/virtio_pmem.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/virtio_pmem.h b/include/uapi/linux/virtio_pmem.h
index b022787ffb94..d676b3620383 100644
--- a/include/uapi/linux/virtio_pmem.h
+++ b/include/uapi/linux/virtio_pmem.h
@@ -15,8 +15,8 @@
 #include 
 
 struct virtio_pmem_config {
-   __u64 start;
-   __u64 size;
+   __le64 start;
+   __le64 size;
 };
 
 #define VIRTIO_PMEM_REQ_TYPE_FLUSH  0
-- 
MST

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


[PATCH v3 03/38] virtio: allow __virtioXX, __leXX in config space

2020-08-05 Thread Michael S. Tsirkin
Currently all config space fields are of the type __uXX.
This confuses people and some drivers (notably vdpa)
access them using CPU endian-ness - which only
works well for legacy or LE platforms.

Update virtio_cread/virtio_cwrite macros to allow __virtioXX
and __leXX field types. Follow-up patches will convert
config space to use these types.

Signed-off-by: Michael S. Tsirkin 
Acked-by: Cornelia Huck 
---
 include/linux/virtio_config.h | 50 +--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 3b4eae5ac5e3..64da491936f7 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 struct irq_affinity;
@@ -287,12 +288,57 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
return __cpu_to_virtio64(virtio_is_little_endian(vdev), val);
 }
 
+/*
+ * Only the checker differentiates between __virtioXX and __uXX types. But we
+ * try to share as much code as we can with the regular GCC build.
+ */
+#if !defined(CONFIG_CC_IS_GCC) && !defined(__CHECKER__)
+
+/* Not a checker - we can keep things simple */
+#define __virtio_native_typeof(x) typeof(x)
+
+#else
+
+/*
+ * We build this out of a couple of helper macros in a vain attempt to
+ * help you keep your lunch down while reading it.
+ */
+#define __virtio_pick_value(x, type, then, otherwise)  \
+   __builtin_choose_expr(__same_type(x, type), then, otherwise)
+
+#define __virtio_pick_type(x, type, then, otherwise)   \
+   __virtio_pick_value(x, type, (then)0, otherwise)
+
+#define __virtio_pick_endian(x, x16, x32, x64, otherwise)  
\
+   __virtio_pick_type(x, x16, __u16,   
\
+   __virtio_pick_type(x, x32, __u32,   
\
+   __virtio_pick_type(x, x64, __u64,   
\
+   otherwise)))
+
+#define __virtio_native_typeof(x) typeof(  
\
+   __virtio_pick_type(x, __u8, __u8,   
\
+   __virtio_pick_endian(x, __virtio16, __virtio32, __virtio64, 
\
+   __virtio_pick_endian(x, __le16, __le32, __le64, 
\
+   __virtio_pick_endian(x, __u16, __u32, __u64,
\
+   /* No other type allowed */ 
\
+   (void)0)
+
+#endif
+
+#define __virtio_native_type(structname, member) \
+   __virtio_native_typeof(((structname*)0)->member)
+
+#define __virtio_typecheck(structname, member, val) \
+   /* Must match the member's type, and be integer */ \
+   typecheck(__virtio_native_type(structname, member), (val))
+
+
 /* Config space accessors. */
 #define virtio_cread(vdev, structname, member, ptr)\
do {\
might_sleep();  \
/* Must match the member's type, and be integer */  \
-   if (!typecheck(typeofstructname*)0)->member)), *(ptr))) \
+   if (!__virtio_typecheck(structname, member, *(ptr)))\
(*ptr) = 1; \
\
switch (sizeof(*ptr)) { \
@@ -322,7 +368,7 @@ static inline __virtio64 cpu_to_virtio64(struct 
virtio_device *vdev, u64 val)
do {\
might_sleep();  \
/* Must match the member's type, and be integer */  \
-   if (!typecheck(typeofstructname*)0)->member)), *(ptr))) \
+   if (!__virtio_typecheck(structname, member, *(ptr)))\
BUG_ON((*ptr) == 1);\
\
switch (sizeof(*ptr)) { \
-- 
MST

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


[PATCH v3 04/38] virtio_9p: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Tag config space fields as having virtio endian-ness.

Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Cornelia Huck 
---
 include/uapi/linux/virtio_9p.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/virtio_9p.h b/include/uapi/linux/virtio_9p.h
index 277c4ad44e84..441047432258 100644
--- a/include/uapi/linux/virtio_9p.h
+++ b/include/uapi/linux/virtio_9p.h
@@ -25,7 +25,7 @@
  * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE. */
-#include 
+#include 
 #include 
 #include 
 
@@ -36,7 +36,7 @@
 
 struct virtio_9p_config {
/* length of the tag name */
-   __u16 tag_len;
+   __virtio16 tag_len;
/* non-NULL terminated tag name */
__u8 tag[0];
 } __attribute__((packed));
-- 
MST

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


[PATCH v3 05/38] virtio_balloon: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
Tag config space fields as having little endian-ness.
Note that balloon is special: LE even when using
the legacy interface.

Signed-off-by: Michael S. Tsirkin 
Acked-by: David Hildenbrand 
Reviewed-by: Cornelia Huck 
---
 include/uapi/linux/virtio_balloon.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/virtio_balloon.h 
b/include/uapi/linux/virtio_balloon.h
index dc3e656470dd..ddaa45e723c4 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -45,20 +45,20 @@
 #define VIRTIO_BALLOON_CMD_ID_DONE 1
 struct virtio_balloon_config {
/* Number of pages host wants Guest to give up. */
-   __u32 num_pages;
+   __le32 num_pages;
/* Number of pages we've actually got in balloon. */
-   __u32 actual;
+   __le32 actual;
/*
 * Free page hint command id, readonly by guest.
 * Was previously named free_page_report_cmd_id so we
 * need to carry that name for legacy support.
 */
union {
-   __u32 free_page_hint_cmd_id;
-   __u32 free_page_report_cmd_id;  /* deprecated */
+   __le32 free_page_hint_cmd_id;
+   __le32 free_page_report_cmd_id; /* deprecated */
};
/* Stores PAGE_POISON if page poisoning is in use */
-   __u32 poison_val;
+   __le32 poison_val;
 };
 
 #define VIRTIO_BALLOON_S_SWAP_IN  0   /* Amount of memory swapped in */
-- 
MST

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


[PATCH v3 01/38] virtio_balloon: fix sparse warning

2020-08-05 Thread Michael S. Tsirkin
balloon uses virtio32_to_cpu instead of cpu_to_virtio32
to convert a native endian number to virtio.
No practical difference but makes sparse warn.
Fix it up.

Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Cornelia Huck 
Acked-by: David Hildenbrand 
Reviewed-by: Cornelia Huck 
---
 drivers/virtio/virtio_balloon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 54fd989f9353..8bc1704ffdf3 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -600,7 +600,7 @@ static int send_cmd_id_start(struct virtio_balloon *vb)
while (virtqueue_get_buf(vq, ))
;
 
-   vb->cmd_id_active = virtio32_to_cpu(vb->vdev,
+   vb->cmd_id_active = cpu_to_virtio32(vb->vdev,
virtio_balloon_cmd_id_received(vb));
sg_init_one(, >cmd_id_active, sizeof(vb->cmd_id_active));
err = virtqueue_add_outbuf(vq, , 1, >cmd_id_active, GFP_KERNEL);
-- 
MST

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


[PATCH v3 02/38] virtio_ring: sparse warning fixup

2020-08-05 Thread Michael S. Tsirkin
virtio_store_mb was built with split ring in mind so it accepts
__virtio16 arguments. Packed ring uses __le16 values, so sparse
complains.  It's just a store with some barriers so let's convert it to
a macro, we don't loose too much type safety by doing that.

Signed-off-by: Michael S. Tsirkin 
Acked-by: Cornelia Huck 
---
 include/linux/virtio_ring.h | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 3dc70adfe5f5..b485b13fa50b 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -46,16 +46,15 @@ static inline void virtio_wmb(bool weak_barriers)
dma_wmb();
 }
 
-static inline void virtio_store_mb(bool weak_barriers,
-  __virtio16 *p, __virtio16 v)
-{
-   if (weak_barriers) {
-   virt_store_mb(*p, v);
-   } else {
-   WRITE_ONCE(*p, v);
-   mb();
-   }
-}
+#define virtio_store_mb(weak_barriers, p, v) \
+do { \
+   if (weak_barriers) { \
+   virt_store_mb(*p, v); \
+   } else { \
+   WRITE_ONCE(*p, v); \
+   mb(); \
+   } \
+} while (0) \
 
 struct virtio_device;
 struct virtqueue;
-- 
MST

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


[PATCH] virtio_net: use LE accessors for speed/duplex

2020-08-05 Thread Michael S. Tsirkin
Speed and duplex config fields depend on VIRTIO_NET_F_SPEED_DUPLEX
which being 63>31 depends on VIRTIO_F_VERSION_1.

Accordingly, use LE accessors for these fields.

Reported-by: Cornelia Huck 
Signed-off-by: Michael S. Tsirkin 
---
 drivers/net/virtio_net.c| 9 +
 include/uapi/linux/virtio_net.h | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index ba38765dc490..0934b1ec5320 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2264,12 +2264,13 @@ static void virtnet_update_settings(struct virtnet_info 
*vi)
if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX))
return;
 
-   speed = virtio_cread32(vi->vdev, offsetof(struct virtio_net_config,
- speed));
+   virtio_cread_le(vi->vdev, struct virtio_net_config, speed, );
+
if (ethtool_validate_speed(speed))
vi->speed = speed;
-   duplex = virtio_cread8(vi->vdev, offsetof(struct virtio_net_config,
- duplex));
+
+   virtio_cread_le(vi->vdev, struct virtio_net_config, duplex, );
+
if (ethtool_validate_duplex(duplex))
vi->duplex = duplex;
 }
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 27d996f29dd1..3f55a4215f11 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -99,7 +99,7 @@ struct virtio_net_config {
 * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
 * Any other value stands for unknown.
 */
-   __virtio32 speed;
+   __le32 speed;
/*
 * 0x00 - half duplex
 * 0x01 - full duplex
-- 
MST

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


Re: [PATCH v2 17/24] virtio_config: disallow native type fields

2020-08-05 Thread Michael S. Tsirkin
On Tue, Aug 04, 2020 at 04:50:39PM +0200, Cornelia Huck wrote:
> On Mon, 3 Aug 2020 16:59:57 -0400
> "Michael S. Tsirkin"  wrote:
> 
> > Transitional devices should all use __virtioXX types.
> 
> I think they should use __leXX for those fields that are not present
> with legacy devices?

Will correct.

> > Modern ones should use __leXX.
> > _uXX type would be a bug.
> > Let's prevent that.
> 
> That sounds right, though.
> 
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  include/linux/virtio_config.h | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > index 64da491936f7..c68f58f3bf34 100644
> > --- a/include/linux/virtio_config.h
> > +++ b/include/linux/virtio_config.h
> > @@ -319,9 +319,8 @@ static inline __virtio64 cpu_to_virtio64(struct 
> > virtio_device *vdev, u64 val)
> > __virtio_pick_type(x, __u8, __u8,   
> > \
> > __virtio_pick_endian(x, __virtio16, __virtio32, __virtio64, 
> > \
> > __virtio_pick_endian(x, __le16, __le32, __le64, 
> > \
> > -   __virtio_pick_endian(x, __u16, __u32, __u64,
> > \
> > -   /* No other type allowed */ 
> > \
> > -   (void)0)
> > +   /* No other type allowed */ 
> > \
> > +   (void)0
> >  
> >  #endif
> >  

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


Re: [PATCH v2 18/24] mlxbf-tmfifo: sparse tags for config access

2020-08-05 Thread Michael S. Tsirkin
On Tue, Aug 04, 2020 at 04:56:34PM +0200, Cornelia Huck wrote:
> On Mon, 3 Aug 2020 17:00:01 -0400
> "Michael S. Tsirkin"  wrote:
> 
> > mlxbf-tmfifo accesses config space using native types -
> > which works for it since the legacy virtio native types.
> > 
> > This will break if it ever needs to support modern virtio,
> > so with new tags previously introduced for virtio net config,
> > sparse now warns for this in drivers.
> > 
> > Since this is a legacy only device, fix it up using
> > virtio_legacy_is_little_endian for now.
> 
> I'm wondering if the driver should make this more explicit?


Not sure how though.

> No issues with the patch, though.
> 
> > 
> > No functional changes.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  drivers/platform/mellanox/mlxbf-tmfifo.c | 13 ++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> Acked-by: Cornelia Huck 

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


Re: [PATCH v2 14/24] virtio_net: correct tags for config space fields

2020-08-05 Thread Michael S. Tsirkin
On Tue, Aug 04, 2020 at 04:44:44PM +0200, Cornelia Huck wrote:
> On Mon, 3 Aug 2020 16:59:37 -0400
> "Michael S. Tsirkin"  wrote:
> 
> > Tag config space fields as having virtio endian-ness.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  include/uapi/linux/virtio_net.h | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/uapi/linux/virtio_net.h 
> > b/include/uapi/linux/virtio_net.h
> > index 19d23e5baa4e..27d996f29dd1 100644
> > --- a/include/uapi/linux/virtio_net.h
> > +++ b/include/uapi/linux/virtio_net.h
> > @@ -87,19 +87,19 @@ struct virtio_net_config {
> > /* The config defining mac address (if VIRTIO_NET_F_MAC) */
> > __u8 mac[ETH_ALEN];
> > /* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
> > -   __u16 status;
> > +   __virtio16 status;
> > /* Maximum number of each of transmit and receive queues;
> >  * see VIRTIO_NET_F_MQ and VIRTIO_NET_CTRL_MQ.
> >  * Legal values are between 1 and 0x8000
> >  */
> > -   __u16 max_virtqueue_pairs;
> > +   __virtio16 max_virtqueue_pairs;
> > /* Default maximum transmit unit advice */
> > -   __u16 mtu;
> > +   __virtio16 mtu;
> > /*
> >  * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
> >  * Any other value stands for unknown.
> >  */
> > -   __u32 speed;
> > +   __virtio32 speed;
> 
> Hm... VIRTIO_NET_F_SPEED_DUPLEX can only be negotiated if VERSION_1 has
> also been negotiated; I think this should be __le32?

OK APIs for this are in a separate patch.
I'll add conversion as a patch on top.


> > /*
> >  * 0x00 - half duplex
> >  * 0x01 - full duplex

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


Re: [PATCH V4 linux-next 00/12] VDPA support for Mellanox ConnectX devices

2020-08-05 Thread Michael S. Tsirkin
On Wed, Aug 05, 2020 at 04:01:58PM +0300, Eli Cohen wrote:
> On Wed, Aug 05, 2020 at 08:48:52AM -0400, Michael S. Tsirkin wrote:
> > > 
> > > Did you merge this?:
> > > git pull git://git.kernel.org/pub/scm/linux/kernel/git/mellanox/linux.git 
> > > mlx5-next
> > 
> > 
> > I can only merge this tree if no one else will. Linus does not like
> > getting same patches through two trees.
> > 
> > Is this the case? Is mlx5-next going to be merged through
> > my tree in this cycle?
> > 
> 
> Saeed Mahameed from Mellanox (located in California) usuaally sends out
> net patches. So he's supposed to send that to Dave Miller.
> 
> I think Saeed should answer this. Let's wait a few more hours till he
> wakes up.

Alternatives:
- merge vdpa through Saeed's tree. I can ack that, we'll need to
  resolve any conflicts by merging the two trees and show the
  result to Linus so he can resolve the merge in the same way.
- extract just the necessary patches that are needed for vdpa and
  merge through my tree.
- if Saeed sends his pull today, it's likely it will be merged
  early next week. Then I can rebase and send a pull with your patches
  on top. A bit risky.
- do some tricks with build. E.g. disable build of your code,
  and enable in Saeed's tree when everything is merged together.
  Can be somewhat hard.

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


Re: [PATCH 4/4] vhost: vdpa: report iova range

2020-08-05 Thread Michael S. Tsirkin
On Wed, Jun 17, 2020 at 11:29:47AM +0800, Jason Wang wrote:
> This patch introduces a new ioctl for vhost-vdpa device that can
> report the iova range by the device. For device that depends on
> platform IOMMU, we fetch the iova range via DOMAIN_ATTR_GEOMETRY. For
> devices that has its own DMA translation unit, we fetch it directly
> from vDPA bus operation.
> 
> Signed-off-by: Jason Wang 
> ---
>  drivers/vhost/vdpa.c | 27 +++
>  include/uapi/linux/vhost.h   |  4 
>  include/uapi/linux/vhost_types.h |  5 +
>  3 files changed, 36 insertions(+)
> 
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 77a0c9fb6cc3..ad23e66cbf57 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -332,6 +332,30 @@ static long vhost_vdpa_set_config_call(struct vhost_vdpa 
> *v, u32 __user *argp)
>  
>   return 0;
>  }
> +
> +static long vhost_vdpa_get_iova_range(struct vhost_vdpa *v, u32 __user *argp)
> +{
> + struct iommu_domain_geometry geo;
> + struct vdpa_device *vdpa = v->vdpa;
> + const struct vdpa_config_ops *ops = vdpa->config;
> + struct vhost_vdpa_iova_range range;
> + struct vdpa_iova_range vdpa_range;
> +
> + if (!ops->set_map && !ops->dma_map) {

Why not just check if (ops->get_iova_range) directly?




> + iommu_domain_get_attr(v->domain,
> +   DOMAIN_ATTR_GEOMETRY, );
> + range.start = geo.aperture_start;
> + range.end = geo.aperture_end;
> + } else {
> + vdpa_range = ops->get_iova_range(vdpa);
> + range.start = vdpa_range.start;
> + range.end = vdpa_range.end;
> + }
> +
> + return copy_to_user(argp, , sizeof(range));
> +
> +}
> +
>  static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
>  void __user *argp)
>  {
> @@ -442,6 +466,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
>   case VHOST_VDPA_SET_CONFIG_CALL:
>   r = vhost_vdpa_set_config_call(v, argp);
>   break;
> + case VHOST_VDPA_GET_IOVA_RANGE:
> + r = vhost_vdpa_get_iova_range(v, argp);
> + break;
>   default:
>   r = vhost_dev_ioctl(>vdev, cmd, argp);
>   if (r == -ENOIOCTLCMD)
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index 0c2349612e77..850956980e27 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -144,4 +144,8 @@
>  
>  /* Set event fd for config interrupt*/
>  #define VHOST_VDPA_SET_CONFIG_CALL   _IOW(VHOST_VIRTIO, 0x77, int)
> +
> +/* Get the valid iova range */
> +#define VHOST_VDPA_GET_IOVA_RANGE_IOW(VHOST_VIRTIO, 0x78, \
> +  struct vhost_vdpa_iova_range)
>  #endif
> diff --git a/include/uapi/linux/vhost_types.h 
> b/include/uapi/linux/vhost_types.h
> index 669457ce5c48..4025b5a36177 100644
> --- a/include/uapi/linux/vhost_types.h
> +++ b/include/uapi/linux/vhost_types.h
> @@ -127,6 +127,11 @@ struct vhost_vdpa_config {
>   __u8 buf[0];
>  };
>  
> +struct vhost_vdpa_iova_range {
> + __u64 start;
> + __u64 end;
> +};
> +


Pls document fields. And I think first/last is a better API ...

>  /* Feature bits */
>  /* Log all write descriptors. Can be changed while device is active. */
>  #define VHOST_F_LOG_ALL 26
> -- 
> 2.20.1

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


Re: [PATCH 3/4] vdpa: get_iova_range() is mandatory for device specific DMA translation

2020-08-05 Thread Michael S. Tsirkin
On Wed, Jun 17, 2020 at 11:29:46AM +0800, Jason Wang wrote:
> In order to let userspace work correctly, get_iova_range() is a must
> for the device that has its own DMA translation logic.

I guess you mean for a device.

However in absence of ths op, I don't see what is wrong with just
assuming device can access any address.

> 
> Signed-off-by: Jason Wang 
> ---
>  drivers/vdpa/vdpa.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index de211ef3738c..ab7af978ef70 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -82,6 +82,10 @@ struct vdpa_device *__vdpa_alloc_device(struct device 
> *parent,
>   if (!!config->dma_map != !!config->dma_unmap)
>   goto err;
>  
> + if ((config->dma_map || config->set_map) &&
> + !config->get_iova_range)
> + goto err;
> +
>   err = -ENOMEM;
>   vdev = kzalloc(size, GFP_KERNEL);
>   if (!vdev)

What about devices using an IOMMU for translation?
IOMMUs generally have a limited IOVA range too, right?



> -- 
> 2.20.1

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


Re: [PATCH 1/4] vdpa: introduce config op to get valid iova range

2020-08-05 Thread Michael S. Tsirkin
On Wed, Jun 17, 2020 at 11:29:44AM +0800, Jason Wang wrote:
> This patch introduce a config op to get valid iova range from the vDPA
> device.
> 
> Signed-off-by: Jason Wang 
> ---
>  include/linux/vdpa.h | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 239db794357c..b7633ed2500c 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -41,6 +41,16 @@ struct vdpa_device {
>   unsigned int index;
>  };
>  
> +/**
> + * vDPA IOVA range - the IOVA range support by the device
> + * @start: start of the IOVA range
> + * @end: end of the IOVA range
> + */
> +struct vdpa_iova_range {
> + u64 start;
> + u64 end;
> +};
> +


This is ambiguous. Is end in the range or just behind it?
How about first/last?



>  /**
>   * vDPA_config_ops - operations for configuring a vDPA device.
>   * Note: vDPA device drivers are required to implement all of the
> @@ -134,6 +144,9 @@ struct vdpa_device {
>   * @get_generation:  Get device config generation (optional)
>   *   @vdev: vdpa device
>   *   Returns u32: device generation
> + * @get_iova_range:  Get supported iova range (on-chip IOMMU)
> + *   @vdev: vdpa device
> + *   Returns the iova range supported by the device
>   * @set_map: Set device memory mapping (optional)
>   *   Needed for device that using device
>   *   specific DMA translation (on-chip IOMMU)
> @@ -195,6 +208,7 @@ struct vdpa_config_ops {
>   void (*set_config)(struct vdpa_device *vdev, unsigned int offset,
>  const void *buf, unsigned int len);
>   u32 (*get_generation)(struct vdpa_device *vdev);
> + struct vdpa_iova_range (*get_iova_range)(struct vdpa_device *vdev);
>  
>   /* DMA ops */
>   int (*set_map)(struct vdpa_device *vdev, struct vhost_iotlb *iotlb);
> -- 
> 2.20.1

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


  1   2   3   4   5   6   7   8   9   10   >