RE: [PATCH v5 4/9] iommufd: Add fault and response message definitions

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@xxxxxxxx>
> Sent: Friday, May 24, 2024 10:15 PM
> 
> On Mon, May 20, 2024 at 04:59:18AM +0000, Tian, Kevin wrote:
> > > From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> > > Sent: Monday, May 20, 2024 11:33 AM
> > >
> > > On 5/20/24 11:24 AM, Tian, Kevin wrote:
> > > >> From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> > > >> Sent: Sunday, May 19, 2024 10:38 PM
> > > >>
> > > >> On 2024/5/15 15:43, Tian, Kevin wrote:
> > > >>>> From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > > >>>> Sent: Tuesday, April 30, 2024 10:57 PM
> > > >>>>
> > > >>>> + * @length: a hint of how much data the requestor is expecting to
> > > fetch.
> > > >> For
> > > >>>> + *          example, if the PRI initiator knows it is going to do a 10MB
> > > >>>> + *          transfer, it could fill in 10MB and the OS could pre-fault in
> > > >>>> + *          10MB of IOVA. It's default to 0 if there's no such hint.
> > > >>>
> > > >>> This is not clear to me and I don't remember PCIe spec defines such
> > > >>> mechanism.
> > > >>
> > > >> This came up in a previous discussion. While it's not currently part of
> > > >
> > > > Can you provide a link to that discussion?
> > >
> > > https://lore.kernel.org/linux-
> iommu/20240322170410.GH66976@xxxxxxxx/
> > >
> >
> > We can always extend uAPI for new usages, e.g. having a new flag
> > bit to indicate the additional filed for carrying the number of pages.
> > But requiring the user to handle non-zero length now (though trivial)
> > is unnecessary burden.
> 
> It is tricky to extend this stuff since it comes out in read().. We'd
> have to have userspace negotiate a new format most likely.
> 
> > Do we want the response message to also carry a length field i.e.
> > allowing the user to partially fix the fault?
> 
> No, the device will discover this when it gets another fault  :)
> 

My worry was that we cannot predict how the spec will define this
extension for multi-pages request/response. It could ask for
additional things provided in the response message (though length
may not be a good example) then we will have to extend the uAPI
anyway.

So I'm fine to keep this room if we can find an usage other than
relying on future spec change.

Otherwise IMHO it tries to guess the semantics of a future spec
change for no good at this point. Then conservatively I'd vote
for not doing it now. 😊