Re: [PATCH V2 4/4] intel-iommu: PASID support

[Michael S. Tsirkin]

On Fri, Apr 22, 2022 at 11:03:51AM -0400, Peter Xu wrote:
> On Sat, Apr 02, 2022 at 07:27:15AM +0000, Tian, Kevin wrote:
> > > > > > Earlier when Yi proposed Qemu changes for guest SVA [1] he aimed for
> > > a
> > > > > > coarse-grained knob design:
> > > > > > --
> > > > > >   Intel VT-d 3.0 introduces scalable mode, and it has a bunch of
> > > capabilities
> > > > > >   related to scalable mode translation, thus there are multiple
> > > combinations.
> > > > > >   While this vIOMMU implementation wants simplify it for user by
> > > providing
> > > > > >   typical combinations. User could config it by "x-scalable-mode" 
> > > > > > option.
> > > > > The
> > > > > >   usage is as below:
> > > > > >     "-device intel-iommu,x-scalable-mode=["legacy"|"modern"]"
> > > > > >
> > > > > >     - "legacy": gives support for SL page table
> > > > > >     - "modern": gives support for FL page table, pasid, virtual 
> > > > > > command
> > > > > >     -  if not configured, means no scalable mode support, if not 
> > > > > > proper
> > > > > >        configured, will throw error
> > > > > > --
> > > > > >
> > > > > > Which way do you prefer to?
> > > > > >
> > > > > > [1] https://lists.gnu.org/archive/html/qemu-devel/2020-
> > > 02/msg02805.html
> > > > >
> > > > > My understanding is that, if we want to deploy Qemu in a production
> > > > > environment, we can't use the "x-" prefix. We need a full
> > > > > implementation of each cap.
> > > > >
> > > > > E.g
> > > > > -device intel-iommu,first-level=on,scalable-mode=on etc.
> > > > >
> > > >
> > > > You meant each cap will get a separate control option?
> > > >
> > > > But that way requires the management stack or admin to have deep
> > > > knowledge about how combinations of different capabilities work, e.g.
> > > > if just turning on scalable mode w/o first-level cannot support vSVA
> > > > on assigned devices. Is this a common practice when defining Qemu
> > > > parameters?
> > > 
> > > We can have a safe and good default value for each cap. E.g
> > > 
> > > In qemu 8.0 we think scalable is mature, we can make scalable to be
> > > enabled by default
> > > in qemu 8.1 we think first-level is mature, we can make first level to
> > > be enabled by default.
> > > 
> > 
> > OK, that is a workable way.
> 
> Sorry again for a very late comment, I think I agree with both of you. :-)
> 
> For debugging purpose parameters like pasid-mode, I'd suggest we make the
> default value to be always depend on the parmaeters that we'll expose to
> the user at last always with the coarse-grained way.
> 
> Assuming that's scalable-mode to be exported by plan (which sounds
> reasonable), then we by default have pasid mode on if scalable-mode is
> modern, otherwise off.  IMHO we don't even need to bother with turning it
> on/off in machine types since we don't even declare these debugging
> parameters supported, do we? :)
> 
> But these debugging parameters could be useful for debugging and triaging
> for sure, keeping them always prefixed with x-.  So it makes sense to have
> them when we're making intermediate steps for the whole building.
> 
> Then at some point all things stable we export scalable-mode to replace
> x-scalable-mode, with an initial versioning alongside (and it doesn't need
> to be started with vt-d 3.0, maybe rev3.3 or even later).
> 
> How's that sound?
> 
> Thanks,
> 
> -- 
> Peter Xu

Sounds good.

-- 
MST