Re: [Qemu-devel] [RFC v1 04/18] intel_iommu: add "sm_model" option

[Peter Xu]

On Wed, Jul 10, 2019 at 12:14:44PM +0000, Liu, Yi L wrote:
> > From: Peter Xu [mailto:address@hidden]
> > Sent: Tuesday, July 9, 2019 10:16 AM
> > To: Liu, Yi L <address@hidden>
> > Subject: Re: [RFC v1 04/18] intel_iommu: add "sm_model" option
> > 
> > On Fri, Jul 05, 2019 at 07:01:37PM +0800, Liu Yi L wrote:
> > > 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 "sm_model" option. The usage is as
> > > below:
> > >
> > > "-device intel-iommu,x-scalable-mode=on,sm_model=["legacy"|"scalable"]"
> > 
> > Is it a requirement to split into two parameters, instead of just
> > exposing everything about scalable mode when x-scalable-mode is set?
> 
> yes, it is. Scalable mode has multiple capabilities. And we want to support
> the most typical combinations to simplify software. e.g. current scalable mode
> vIOMMU exposes only 2nd level translation to guest, and guest IOVA support
> is via shadowing guest 2nd level page table. We have plan to move IOVA from
> 2nd level page table to 1st level page table, thus guest IOVA can be supported
> with nested translation. And this also addresses the co-existence issue of 
> guest
> SVA and guest IOVA. So in future we will have scalable mode vIOMMU expose
> 1st level translation only. To differentiate this config with current vIOMMU,
> we need an extra option to control it. But yes, it is still scalable mode 
> vIOMMU.
> just has different capability exposed to guest.

I see.  Thanks for explaining.

> 
> BTW. do you know if I can add sub-options under "x-scalable-mode"? I think
> that may demonstrate the dependency better.

I'm not an expert of that, but I think at least we can make it a
string parameter depends on what you prefer, then we can do
"x-scalable-mode=legacy|modern".  Or keep this would be fine too.

> 
> > >
> > >  - "legacy": gives support for SL page table
> > >  - "scalable": gives support for FL page table, pasid, virtual command
> > >  - default to be "legacy" if "x-scalable-mode=on while no sm_model is
> > >    configured
> > >
> > > Cc: Kevin Tian <address@hidden>
> > > Cc: Jacob Pan <address@hidden>
> > > Cc: Peter Xu <address@hidden>
> > > Cc: Yi Sun <address@hidden>
> > > Signed-off-by: Liu Yi L <address@hidden>
> > > Signed-off-by: Yi Sun <address@hidden>
> > > ---
> > >  hw/i386/intel_iommu.c          | 28 +++++++++++++++++++++++++++-
> > >  hw/i386/intel_iommu_internal.h |  2 ++
> > >  include/hw/i386/intel_iommu.h  |  1 +
> > >  3 files changed, 30 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > > index 44b1231..3160a05 100644
> > > --- a/hw/i386/intel_iommu.c
> > > +++ b/hw/i386/intel_iommu.c
> > > @@ -3014,6 +3014,7 @@ static Property vtd_properties[] = {
> > >      DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode,
> > FALSE),
> > >      DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode,
> > FALSE),
> > >      DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
> > > +    DEFINE_PROP_STRING("sm_model", IntelIOMMUState, sm_model),
> > 
> > Can do 's/-/_/' to follow the rest if we need it.
> 
> Do you mean sub-options after "x-scalable-mode"?

No, I only mean "sm-model". :)

Regards,

-- 
Peter Xu