[RFC,v1,0/8] vhost-vdpa: add support for iommufd

  Hi All
This code provides the iommufd support for vdpa device
This code fixes the bugs from the last version and also add the asid support. rebase on kernel
v6,6-rc3
Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net), 
I will continue working on it

The kernel code is
https://gitlab.com/lulu6/vhost/-/tree/iommufdRFC_v1

Signed-off-by: Cindy Lu <lulu@redhat.com>


Cindy Lu (8):
  vhost/iommufd: Add the functions support iommufd
  Kconfig: Add the new file vhost/iommufd
  vhost: Add 3 new uapi to support iommufd
  vdpa: Add new vdpa_config_ops to support iommufd
  vdpa_sim :Add support for iommufd
  vdpa: change the map/unmap process to support iommufd
  vp_vdpa::Add support for iommufd
  iommu: expose the function iommu_device_use_default_domain

 drivers/iommu/iommu.c             |   2 +
 drivers/vdpa/vdpa_sim/vdpa_sim.c  |   8 ++
 drivers/vdpa/virtio_pci/vp_vdpa.c |   4 +
 drivers/vhost/Kconfig             |   1 +
 drivers/vhost/Makefile            |   1 +
 drivers/vhost/iommufd.c           | 178 +++++++++++++++++++++++++
 drivers/vhost/vdpa.c              | 210 +++++++++++++++++++++++++++++-
 drivers/vhost/vhost.h             |  21 +++
 include/linux/vdpa.h              |  38 +++++-
 include/uapi/linux/vhost.h        |  66 ++++++++++
 10 files changed, 525 insertions(+), 4 deletions(-)
 create mode 100644 drivers/vhost/iommufd.c
  

On Sat, Nov 4, 2023 at 1:16 AM Cindy Lu <lulu@redhat.com> wrote:
>
>
> Hi All
> This code provides the iommufd support for vdpa device
> This code fixes the bugs from the last version and also add the asid support. rebase on kernel
> v6,6-rc3
> Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net),
> I will continue working on it
>
> The kernel code is
> https://gitlab.com/lulu6/vhost/-/tree/iommufdRFC_v1
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>

It would be better to have a change summary here.

Thanks

>
>
> Cindy Lu (8):
>   vhost/iommufd: Add the functions support iommufd
>   Kconfig: Add the new file vhost/iommufd
>   vhost: Add 3 new uapi to support iommufd
>   vdpa: Add new vdpa_config_ops to support iommufd
>   vdpa_sim :Add support for iommufd
>   vdpa: change the map/unmap process to support iommufd
>   vp_vdpa::Add support for iommufd
>   iommu: expose the function iommu_device_use_default_domain
>
>  drivers/iommu/iommu.c             |   2 +
>  drivers/vdpa/vdpa_sim/vdpa_sim.c  |   8 ++
>  drivers/vdpa/virtio_pci/vp_vdpa.c |   4 +
>  drivers/vhost/Kconfig             |   1 +
>  drivers/vhost/Makefile            |   1 +
>  drivers/vhost/iommufd.c           | 178 +++++++++++++++++++++++++
>  drivers/vhost/vdpa.c              | 210 +++++++++++++++++++++++++++++-
>  drivers/vhost/vhost.h             |  21 +++
>  include/linux/vdpa.h              |  38 +++++-
>  include/uapi/linux/vhost.h        |  66 ++++++++++
>  10 files changed, 525 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/vhost/iommufd.c
>
> --
> 2.34.3
>
  

On 2023/11/6 12:11, Jason Wang wrote:
> On Sat, Nov 4, 2023 at 1:16 AM Cindy Lu <lulu@redhat.com> wrote:
>>
>>
>> Hi All
>> This code provides the iommufd support for vdpa device
>> This code fixes the bugs from the last version and also add the asid support. rebase on kernel
>> v6,6-rc3
>> Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net),
>> I will continue working on it
>>
>> The kernel code is
>> https://gitlab.com/lulu6/vhost/-/tree/iommufdRFC_v1
>>
>> Signed-off-by: Cindy Lu <lulu@redhat.com>
> 
> It would be better to have a change summary here.

yeah, the version number is also incorrect.

> Thanks
> 
>>
>>
>> Cindy Lu (8):
>>    vhost/iommufd: Add the functions support iommufd
>>    Kconfig: Add the new file vhost/iommufd
>>    vhost: Add 3 new uapi to support iommufd
>>    vdpa: Add new vdpa_config_ops to support iommufd
>>    vdpa_sim :Add support for iommufd
>>    vdpa: change the map/unmap process to support iommufd
>>    vp_vdpa::Add support for iommufd
>>    iommu: expose the function iommu_device_use_default_domain
>>
>>   drivers/iommu/iommu.c             |   2 +
>>   drivers/vdpa/vdpa_sim/vdpa_sim.c  |   8 ++
>>   drivers/vdpa/virtio_pci/vp_vdpa.c |   4 +
>>   drivers/vhost/Kconfig             |   1 +
>>   drivers/vhost/Makefile            |   1 +
>>   drivers/vhost/iommufd.c           | 178 +++++++++++++++++++++++++
>>   drivers/vhost/vdpa.c              | 210 +++++++++++++++++++++++++++++-
>>   drivers/vhost/vhost.h             |  21 +++
>>   include/linux/vdpa.h              |  38 +++++-
>>   include/uapi/linux/vhost.h        |  66 ++++++++++
>>   10 files changed, 525 insertions(+), 4 deletions(-)
>>   create mode 100644 drivers/vhost/iommufd.c
>>
>> --
>> 2.34.3
>>
>
  

On Sat, Nov 04, 2023 at 01:16:33AM +0800, Cindy Lu wrote:
> 
> Hi All
> This code provides the iommufd support for vdpa device
> This code fixes the bugs from the last version and also add the asid support. rebase on kernel
> v6,6-rc3
> Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net), 

What kind of problems? Understanding that will make it easier
to figure out whether this is worth reviewing.

> I will continue working on it
> 
> The kernel code is
> https://gitlab.com/lulu6/vhost/-/tree/iommufdRFC_v1
> 
> Signed-off-by: Cindy Lu <lulu@redhat.com>

Please also Cc iommufd maintainers:

Jason Gunthorpe <jgg@ziepe.ca> (maintainer:IOMMUFD)
Kevin Tian <kevin.tian@intel.com> (maintainer:IOMMUFD)
Joerg Roedel <joro@8bytes.org> (maintainer:IOMMU SUBSYSTEM)
Will Deacon <will@kernel.org> (maintainer:IOMMU SUBSYSTEM)
Robin Murphy <robin.murphy@arm.com> (reviewer:IOMMU SUBSYSTEM)
iommu@lists.linux.dev (open list:IOMMUFD)
linux-kernel@vger.kernel.org (open list)
  

On Tue, Nov 07, 2023 at 02:30:34AM -0500, Michael S. Tsirkin wrote:
> On Sat, Nov 04, 2023 at 01:16:33AM +0800, Cindy Lu wrote:
> > 
> > Hi All
> > This code provides the iommufd support for vdpa device
> > This code fixes the bugs from the last version and also add the asid support. rebase on kernel
> > v6,6-rc3
> > Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net), 
> 
> What kind of problems? Understanding that will make it easier
> to figure out whether this is worth reviewing.

IMHO, this patch series needs to spend more time internally to Red Hat
before it is presented to the community. It is too far away from
something worth reviewing at this point.

Jason
  

On Sat, Nov 04, 2023 at 01:16:33AM +0800, Cindy Lu wrote:
> Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net), 

I'm not sure there's even value in bothering with iommufd for the
simulator. Just find a way to disable it and fail gracefully.
  

On Tue, Nov 07, 2023 at 08:49:02AM -0400, Jason Gunthorpe wrote:
> On Tue, Nov 07, 2023 at 02:30:34AM -0500, Michael S. Tsirkin wrote:
> > On Sat, Nov 04, 2023 at 01:16:33AM +0800, Cindy Lu wrote:
> > > 
> > > Hi All
> > > This code provides the iommufd support for vdpa device
> > > This code fixes the bugs from the last version and also add the asid support. rebase on kernel
> > > v6,6-rc3
> > > Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net), 
> > 
> > What kind of problems? Understanding that will make it easier
> > to figure out whether this is worth reviewing.
> 
> IMHO, this patch series needs to spend more time internally to Red Hat
> before it is presented to the community. It is too far away from
> something worth reviewing at this point.
> 
> Jason

I am always trying to convince people to post RFCs early
instead of working for months behind closed doors only
to be told to rewrite everything in Rust.

Why does it have to be internal to a specific company?
I see Yi Liu from Intel is helping Cindy get it into shape
and that's classic open source ethos.

I know some subsystems ignore the RFC tag but I didn't realize
iommu is one of these. Is that really true?
  

On Tue, Nov 07, 2023 at 08:28:32AM -0500, Michael S. Tsirkin wrote:
> On Tue, Nov 07, 2023 at 08:49:02AM -0400, Jason Gunthorpe wrote:
> > On Tue, Nov 07, 2023 at 02:30:34AM -0500, Michael S. Tsirkin wrote:
> > > On Sat, Nov 04, 2023 at 01:16:33AM +0800, Cindy Lu wrote:
> > > > 
> > > > Hi All
> > > > This code provides the iommufd support for vdpa device
> > > > This code fixes the bugs from the last version and also add the asid support. rebase on kernel
> > > > v6,6-rc3
> > > > Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net), 
> > > 
> > > What kind of problems? Understanding that will make it easier
> > > to figure out whether this is worth reviewing.
> > 
> > IMHO, this patch series needs to spend more time internally to Red Hat
> > before it is presented to the community. It is too far away from
> > something worth reviewing at this point.
> > 
> > Jason
> 
> I am always trying to convince people to post RFCs early
> instead of working for months behind closed doors only
> to be told to rewrite everything in Rust.

The community has a limited review bandwidth, things should meet a
minimum standard before there is an expectation that other people
should review it on the list.

> Why does it have to be internal to a specific company?
> I see Yi Liu from Intel is helping Cindy get it into shape
> and that's classic open source ethos.

Big company's should take the responsibility to train and provide
skill development for their own staff.

> I know some subsystems ignore the RFC tag but I didn't realize
> iommu is one of these. Is that really true?

At least I've looked at this twice now and been disappointed. Neither
have been a well thought out RFC, this is a brain dump of unfinished
work.

Jason
  

On Tue, Nov 07, 2023 at 10:12:37AM -0400, Jason Gunthorpe wrote:
> Big company's should take the responsibility to train and provide
> skill development for their own staff.

That would result in a beautiful cathedral of a patch. I know this is
how some companies work. We are doing more of a bazaar thing here,
though. In a bunch of subsystems it seems that you don't get the
necessary skills until you have been publically shouted at by
maintainers - better to start early ;). Not a nice environment for
novices, for sure.
  

On Tue, Nov 07, 2023 at 08:49:02AM -0400, Jason Gunthorpe wrote:
> IMHO, this patch series needs to spend more time internally to Red Hat
> before it is presented to the community.

Just to add an example why I think this "internal review" is a bad idea
I seem to recall that someone internal to nvidia at some point
attempted to implement this already. The only output from that
work we have is that "it's tough" - no pointers to what's tough,
no code to study even as a bad path to follow.
And while Red Hat might be big, the virt team is rather smaller.
  

On Tue, Nov 07, 2023 at 09:55:26AM -0500, Michael S. Tsirkin wrote:
> On Tue, Nov 07, 2023 at 08:49:02AM -0400, Jason Gunthorpe wrote:
> > IMHO, this patch series needs to spend more time internally to Red Hat
> > before it is presented to the community.
> 
> Just to add an example why I think this "internal review" is a bad idea
> I seem to recall that someone internal to nvidia at some point
> attempted to implement this already. The only output from that
> work we have is that "it's tough" - no pointers to what's tough,
> no code to study even as a bad path to follow.
> And while Red Hat might be big, the virt team is rather smaller.

I don't think Nicolin got to a presentable code point.

But you can start to see the issues even in this series, like
simulator is complicated. mlx5 is complicated. Deciding to omit those
is one path. Come with a proposal and justification to take it out,
not a patch with an unexplained #ifdef.

Again, I'm not talking about big impactful decisions I'm saying RH
should take it internally to get a RFC proposal to the level where it
is actually an RFC proposal and not a brain dump. Make sure it has
logical commit messages, make sure the basic thinking about the idea
is done and the proposal is self consistent and explained. Make sure
the patches and series construction meet a kernel standard.

The purpose of the RFC is to clearly articulate what it is you are
asking to do, why you want to do it, and how you intend to get
there. There is still alot of basic work to achieve this and properly
communicate it.

Training to do that should rightly come from the employeer, not the
community. We've seen some big blow ups because some companies have
been trying to externalize their training to the community.

Jason
  

On Tue, Nov 07, 2023 at 09:30:21AM -0500, Michael S. Tsirkin wrote:
> On Tue, Nov 07, 2023 at 10:12:37AM -0400, Jason Gunthorpe wrote:
> > Big company's should take the responsibility to train and provide
> > skill development for their own staff.
> 
> That would result in a beautiful cathedral of a patch. I know this is
> how some companies work. We are doing more of a bazaar thing here,
> though. In a bunch of subsystems it seems that you don't get the
> necessary skills until you have been publically shouted at by
> maintainers - better to start early ;). Not a nice environment for
> novices, for sure.

In my view the "shouting from maintainers" is harmful to the people
buidling skills and it is an unkind thing to dump employees into that
kind of situation.

They should have help to establish the basic level of competence where
they may do the wrong thing, but all the process and presentation of
the wrong thing is top notch. You get a much better reception.

Jason
  

On Tue, Nov 07, 2023 at 11:48:48AM -0400, Jason Gunthorpe wrote:
> On Tue, Nov 07, 2023 at 09:55:26AM -0500, Michael S. Tsirkin wrote:
> > On Tue, Nov 07, 2023 at 08:49:02AM -0400, Jason Gunthorpe wrote:
> > > IMHO, this patch series needs to spend more time internally to Red Hat
> > > before it is presented to the community.
> > 
> > Just to add an example why I think this "internal review" is a bad idea
> > I seem to recall that someone internal to nvidia at some point
> > attempted to implement this already. The only output from that
> > work we have is that "it's tough" - no pointers to what's tough,
> > no code to study even as a bad path to follow.
> > And while Red Hat might be big, the virt team is rather smaller.
> 
> I don't think Nicolin got to a presentable code point.
> 
> But you can start to see the issues even in this series, like
> simulator is complicated. mlx5 is complicated. Deciding to omit those
> is one path. Come with a proposal and justification to take it out,
> not a patch with an unexplained #ifdef.

Right. Simulator I don't think we need to support, or at least
not necessarily to get this merged - it does not really
benefit from any iommufd features.
  

On Tue, 7 Nov 2023 08:28:32 -0500 Michael S. Tsirkin wrote:
> I am always trying to convince people to post RFCs early
> instead of working for months behind closed doors only
> to be told to rewrite everything in Rust.
> 
> Why does it have to be internal to a specific company?
> I see Yi Liu from Intel is helping Cindy get it into shape
> and that's classic open source ethos.

+1, FWIW.
  

On Tue, Nov 07, 2023 at 11:52:17AM -0400, Jason Gunthorpe wrote:
> On Tue, Nov 07, 2023 at 09:30:21AM -0500, Michael S. Tsirkin wrote:
> > On Tue, Nov 07, 2023 at 10:12:37AM -0400, Jason Gunthorpe wrote:
> > > Big company's should take the responsibility to train and provide
> > > skill development for their own staff.
> > 
> > That would result in a beautiful cathedral of a patch. I know this is
> > how some companies work. We are doing more of a bazaar thing here,
> > though. In a bunch of subsystems it seems that you don't get the
> > necessary skills until you have been publically shouted at by
> > maintainers - better to start early ;). Not a nice environment for
> > novices, for sure.
> 
> In my view the "shouting from maintainers" is harmful to the people
> buidling skills and it is an unkind thing to dump employees into that
> kind of situation.
> 
> They should have help to establish the basic level of competence where
> they may do the wrong thing, but all the process and presentation of
> the wrong thing is top notch. You get a much better reception.
> 
> Jason

What - like e.g. mechanically fixing checkpatch warnings without
understanding? I actually very much dislike it when people take a bad
patch and just polish the presentation
- it is easier for me if I can judge patch quality quickly from the
presentation. Matter of taste I guess.
  

On Thu, Nov 09, 2023 at 06:48:46PM -0500, Michael S. Tsirkin wrote:
> On Tue, Nov 07, 2023 at 11:52:17AM -0400, Jason Gunthorpe wrote:
> > On Tue, Nov 07, 2023 at 09:30:21AM -0500, Michael S. Tsirkin wrote:
> > > On Tue, Nov 07, 2023 at 10:12:37AM -0400, Jason Gunthorpe wrote:
> > > > Big company's should take the responsibility to train and provide
> > > > skill development for their own staff.
> > > 
> > > That would result in a beautiful cathedral of a patch. I know this is
> > > how some companies work. We are doing more of a bazaar thing here,
> > > though. In a bunch of subsystems it seems that you don't get the
> > > necessary skills until you have been publically shouted at by
> > > maintainers - better to start early ;). Not a nice environment for
> > > novices, for sure.
> > 
> > In my view the "shouting from maintainers" is harmful to the people
> > buidling skills and it is an unkind thing to dump employees into that
> > kind of situation.
> > 
> > They should have help to establish the basic level of competence where
> > they may do the wrong thing, but all the process and presentation of
> > the wrong thing is top notch. You get a much better reception.
> 
> What - like e.g. mechanically fixing checkpatch warnings without
> understanding? 

No, not at all. I mean actually going through and explaining what the
idea is to another person and ensuing that the commit messages convey
that idea, that the patches reflect the idea, that everything is
convayed, and it isn't obviously internally illogical.

Like, why did this series have a giant block of #ifdef 0'd code with
no explanation at all? That isn't checkpatch nitpicks, that is not
meeting the minimum standard to convey an idea in an RFC.

Jason
  

On Sat, Nov 04, 2023 at 01:16:33AM +0800, Cindy Lu wrote:
> 
> Hi All
> This code provides the iommufd support for vdpa device
> This code fixes the bugs from the last version and also add the asid support. rebase on kernel
> v6,6-rc3
> Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net), 
> I will continue working on it
> 
> The kernel code is
> https://gitlab.com/lulu6/vhost/-/tree/iommufdRFC_v1
> 
> Signed-off-by: Cindy Lu <lulu@redhat.com>

Was this abandoned?

> 
> Cindy Lu (8):
>   vhost/iommufd: Add the functions support iommufd
>   Kconfig: Add the new file vhost/iommufd
>   vhost: Add 3 new uapi to support iommufd
>   vdpa: Add new vdpa_config_ops to support iommufd
>   vdpa_sim :Add support for iommufd
>   vdpa: change the map/unmap process to support iommufd
>   vp_vdpa::Add support for iommufd
>   iommu: expose the function iommu_device_use_default_domain
> 
>  drivers/iommu/iommu.c             |   2 +
>  drivers/vdpa/vdpa_sim/vdpa_sim.c  |   8 ++
>  drivers/vdpa/virtio_pci/vp_vdpa.c |   4 +
>  drivers/vhost/Kconfig             |   1 +
>  drivers/vhost/Makefile            |   1 +
>  drivers/vhost/iommufd.c           | 178 +++++++++++++++++++++++++
>  drivers/vhost/vdpa.c              | 210 +++++++++++++++++++++++++++++-
>  drivers/vhost/vhost.h             |  21 +++
>  include/linux/vdpa.h              |  38 +++++-
>  include/uapi/linux/vhost.h        |  66 ++++++++++
>  10 files changed, 525 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/vhost/iommufd.c
> 
> -- 
> 2.34.3
  

On Thu, Jan 11, 2024 at 6:25 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sat, Nov 04, 2023 at 01:16:33AM +0800, Cindy Lu wrote:
> >
> > Hi All
> > This code provides the iommufd support for vdpa device
> > This code fixes the bugs from the last version and also add the asid support. rebase on kernel
> > v6,6-rc3
> > Test passed in the physical device (vp_vdpa), but  there are still some problems in the emulated device (vdpa_sim_net),
> > I will continue working on it
> >
> > The kernel code is
> > https://gitlab.com/lulu6/vhost/-/tree/iommufdRFC_v1
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
>
> Was this abandoned?
>
Thanks Micheal. I'm really sorry for the delay. I will continue working on this
Thanks
Cindy
> >
> > Cindy Lu (8):
> >   vhost/iommufd: Add the functions support iommufd
> >   Kconfig: Add the new file vhost/iommufd
> >   vhost: Add 3 new uapi to support iommufd
> >   vdpa: Add new vdpa_config_ops to support iommufd
> >   vdpa_sim :Add support for iommufd
> >   vdpa: change the map/unmap process to support iommufd
> >   vp_vdpa::Add support for iommufd
> >   iommu: expose the function iommu_device_use_default_domain
> >
> >  drivers/iommu/iommu.c             |   2 +
> >  drivers/vdpa/vdpa_sim/vdpa_sim.c  |   8 ++
> >  drivers/vdpa/virtio_pci/vp_vdpa.c |   4 +
> >  drivers/vhost/Kconfig             |   1 +
> >  drivers/vhost/Makefile            |   1 +
> >  drivers/vhost/iommufd.c           | 178 +++++++++++++++++++++++++
> >  drivers/vhost/vdpa.c              | 210 +++++++++++++++++++++++++++++-
> >  drivers/vhost/vhost.h             |  21 +++
> >  include/linux/vdpa.h              |  38 +++++-
> >  include/uapi/linux/vhost.h        |  66 ++++++++++
> >  10 files changed, 525 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/vhost/iommufd.c
> >
> > --
> > 2.34.3
>