> From: Peter Xu [mailto:peterx@xxxxxxxxxx] > Sent: Friday, November 1, 2019 10:58 PM > To: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Subject: Re: [RFC v2 03/22] intel_iommu: modify x-scalable-mode to be string option > > On Thu, Oct 24, 2019 at 08:34:24AM -0400, 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 > > "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 > > > > Cc: Kevin Tian <kevin.tian@xxxxxxxxx> > > Cc: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx> > > Cc: Peter Xu <peterx@xxxxxxxxxx> > > Cc: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> > > Signed-off-by: Liu Yi L <yi.l.liu@xxxxxxxxx> > > Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> > > --- > > hw/i386/intel_iommu.c | 15 +++++++++++++-- > > hw/i386/intel_iommu_internal.h | 3 +++ > > include/hw/i386/intel_iommu.h | 2 +- > > 3 files changed, 17 insertions(+), 3 deletions(-) > > > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index > > 771bed2..4a1a07a 100644 > > --- a/hw/i386/intel_iommu.c > > +++ b/hw/i386/intel_iommu.c > > @@ -3019,7 +3019,7 @@ static Property vtd_properties[] = { > > DEFINE_PROP_UINT8("aw-bits", IntelIOMMUState, aw_bits, > > VTD_HOST_ADDRESS_WIDTH), > > DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, > FALSE), > > - DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode, > FALSE), > > + DEFINE_PROP_STRING("x-scalable-mode", IntelIOMMUState, > > + scalable_mode), > > DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true), > > DEFINE_PROP_END_OF_LIST(), > > }; > > @@ -3581,7 +3581,12 @@ static void vtd_init(IntelIOMMUState *s) > > > > /* TODO: read cap/ecap from host to decide which cap to be exposed. */ > > if (s->scalable_mode) { > > - s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS; > > + if (!strcmp(s->scalable_mode, "legacy")) { > > + s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS; > > + } else if (!strcmp(s->scalable_mode, "modern")) { > > + s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_PASID > > + | VTD_ECAP_FLTS | VTD_ECAP_PSS; > > + } > > Shall we do this string op only once in vtd_decide_config() then keep it somewhere? Agreed. I'll move it to vtd_decide_config(). > Something like: > > - s->scalable_mode_str to keep the string > - s->scalable_mode still as a bool to cache the global enablement > - s->scalable_modern as a bool to keep the mode > > ? So x-scalable-mode is still a string option, just to have a new field to store it? > > These could be used in some MMIO path (I think) and parsing strings always could be > a bit overkill. I think so. Let's just align on the direction above. Regards, Yi Liu
