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

[Liu, Yi L]

> From: Peter Xu [mailto:address@hidden]
> Sent: Thursday, July 11, 2019 9:04 AM
> To: Liu, Yi L <address@hidden>
> Subject: Re: [RFC v1 04/18] intel_iommu: add "sm_model" option
> 
> 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.

you are welcome. :-)

> 
> >
> > 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.

hmmm, it's a good idea. If we agree to change x-scalable-mode to be a string
parameter. I think I can change it.

> >
> > > >
> > > >  - "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". :)

got it. if we modify x-scalable-mode to be string, then sm-model would be
removed.

Regards,
Yi Liu

> Regards,
> 
> --
> Peter Xu