RE: [RFC v3 13/25] intel_iommu: modify x-scalable-mode to be string option
> From: Peter Xu > Sent: Thursday, February 13, 2020 12:06 AM > To: Liu, Yi L > Subject: Re: [RFC v3 13/25] intel_iommu: modify x-scalable-mode to be string > option > > On Wed, Feb 12, 2020 at 07:28:24AM +, Liu, Yi L wrote: > > > From: Peter Xu > > > Sent: Wednesday, February 12, 2020 3:44 AM > > > To: Liu, Yi L > > > Subject: Re: [RFC v3 13/25] intel_iommu: modify x-scalable-mode to > > > be string option > > > > > > On Wed, Jan 29, 2020 at 04:16:44AM -0800, Liu, Yi L wrote: > > > > From: Liu Yi L > > > > > > > > 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"]" > > > > > > Maybe also "off" when someone wants to explicitly disable it? > > > > emmm, I think x-scalable-mode should be disabled by default. It is > > enabled only when "legacy" or "modern" is configured. I'm fine to add > > "off" as an explicit way to turn it off if you think it is necessary. > > :-) > > It's not necessary. It'll be necessary when we remove "x-" and change the > default value. However it'll always be good to provide all options > explicitly in the > parameter starting from when we design it, imho. It's still experimental, > so... > Your call. :) Got it. Let me add it in next version. 😊 Regards, Yi Liu
Re: [RFC v3 13/25] intel_iommu: modify x-scalable-mode to be string option
On Wed, Feb 12, 2020 at 07:28:24AM +, Liu, Yi L wrote: > > From: Peter Xu > > Sent: Wednesday, February 12, 2020 3:44 AM > > To: Liu, Yi L > > Subject: Re: [RFC v3 13/25] intel_iommu: modify x-scalable-mode to be string > > option > > > > On Wed, Jan 29, 2020 at 04:16:44AM -0800, Liu, Yi L wrote: > > > From: Liu Yi L > > > > > > 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"]" > > > > Maybe also "off" when someone wants to explicitly disable it? > > emmm, I think x-scalable-mode should be disabled by default. It is enabled > only when "legacy" or "modern" is configured. I'm fine to add "off" as an > explicit way to turn it off if you think it is necessary. :-) It's not necessary. It'll be necessary when we remove "x-" and change the default value. However it'll always be good to provide all options explicitly in the parameter starting from when we design it, imho. It's still experimental, so... Your call. :) -- Peter Xu
RE: [RFC v3 13/25] intel_iommu: modify x-scalable-mode to be string option
> From: Peter Xu
> Sent: Wednesday, February 12, 2020 3:44 AM
> To: Liu, Yi L
> Subject: Re: [RFC v3 13/25] intel_iommu: modify x-scalable-mode to be string
> option
>
> On Wed, Jan 29, 2020 at 04:16:44AM -0800, Liu, Yi L wrote:
> > From: Liu Yi L
> >
> > 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"]"
>
> Maybe also "off" when someone wants to explicitly disable it?
emmm, I think x-scalable-mode should be disabled by default. It is enabled
only when "legacy" or "modern" is configured. I'm fine to add "off" as an
explicit way to turn it off if you think it is necessary. :-)
> >
> > - "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
> > Cc: Jacob Pan
> > Cc: Peter Xu
> > Cc: Yi Sun
> > Cc: Paolo Bonzini
> > Cc: Richard Henderson
> > Cc: Eduardo Habkost
> > Signed-off-by: Liu Yi L
> > Signed-off-by: Yi Sun
> > ---
> > hw/i386/intel_iommu.c | 27 +--
> > hw/i386/intel_iommu_internal.h | 3 +++
> > include/hw/i386/intel_iommu.h | 2 ++
> > 3 files changed, 30 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index
> > 1c1eb7f..33be40c 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -3078,7 +3078,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_str),
> > DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> > @@ -3708,8 +3708,11 @@ static void vtd_init(IntelIOMMUState *s)
> > }
> >
> > /* TODO: read cap/ecap from host to decide which cap to be exposed. */
> > -if (s->scalable_mode) {
> > +if (s->scalable_mode && !s->scalable_modern) {
> > s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
> > +} else if (s->scalable_mode && s->scalable_modern) {
> > +s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_PASID
> > + | VTD_ECAP_FLTS | VTD_ECAP_PSS;
>
> This patch might be good to be the last one after all the impls are ready.
Oh, yes. Let me reorder it in next version.
> > }
> >
> > vtd_reset_caches(s);
> > @@ -3845,6 +3848,26 @@ static bool vtd_decide_config(IntelIOMMUState *s,
> Error **errp)
> > return false;
> > }
> >
> > +if (s->scalable_mode_str &&
> > +(strcmp(s->scalable_mode_str, "modern") &&
> > + strcmp(s->scalable_mode_str, "legacy"))) {
> > +error_setg(errp, "Invalid x-scalable-mode config");
>
> Maybe "..., Please use 'modern', 'legacy', or 'off'." to show options.
Got it.
Thanks,
Yi Liu
Re: [RFC v3 13/25] intel_iommu: modify x-scalable-mode to be string option
On Wed, Jan 29, 2020 at 04:16:44AM -0800, Liu, Yi L wrote:
> From: Liu Yi L
>
> 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"]"
Maybe also "off" when someone wants to explicitly disable it?
>
> - "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
> Cc: Jacob Pan
> Cc: Peter Xu
> Cc: Yi Sun
> Cc: Paolo Bonzini
> Cc: Richard Henderson
> Cc: Eduardo Habkost
> Signed-off-by: Liu Yi L
> Signed-off-by: Yi Sun
> ---
> hw/i386/intel_iommu.c | 27 +--
> hw/i386/intel_iommu_internal.h | 3 +++
> include/hw/i386/intel_iommu.h | 2 ++
> 3 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 1c1eb7f..33be40c 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3078,7 +3078,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_str),
> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true),
> DEFINE_PROP_END_OF_LIST(),
> };
> @@ -3708,8 +3708,11 @@ static void vtd_init(IntelIOMMUState *s)
> }
>
> /* TODO: read cap/ecap from host to decide which cap to be exposed. */
> -if (s->scalable_mode) {
> +if (s->scalable_mode && !s->scalable_modern) {
> s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_SLTS;
> +} else if (s->scalable_mode && s->scalable_modern) {
> +s->ecap |= VTD_ECAP_SMTS | VTD_ECAP_SRS | VTD_ECAP_PASID
> + | VTD_ECAP_FLTS | VTD_ECAP_PSS;
This patch might be good to be the last one after all the impls are
ready.
> }
>
> vtd_reset_caches(s);
> @@ -3845,6 +3848,26 @@ static bool vtd_decide_config(IntelIOMMUState *s,
> Error **errp)
> return false;
> }
>
> +if (s->scalable_mode_str &&
> +(strcmp(s->scalable_mode_str, "modern") &&
> + strcmp(s->scalable_mode_str, "legacy"))) {
> +error_setg(errp, "Invalid x-scalable-mode config");
Maybe "..., Please use 'modern', 'legacy', or 'off'." to show options.
> +return false;
> +}
> +
> +if (s->scalable_mode_str &&
> +!strcmp(s->scalable_mode_str, "legacy")) {
> +s->scalable_mode = true;
> +s->scalable_modern = false;
> +} else if (s->scalable_mode_str &&
> +!strcmp(s->scalable_mode_str, "modern")) {
> +s->scalable_mode = true;
> +s->scalable_modern = true;
> +} else {
> +s->scalable_mode = false;
> +s->scalable_modern = false;
> +}
> +
> return true;
> }
>
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 862033e..c4dbb2c 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -190,8 +190,11 @@
> #define VTD_ECAP_PT (1ULL << 6)
> #define VTD_ECAP_MHMV (15ULL << 20)
> #define VTD_ECAP_SRS(1ULL << 31)
> +#define VTD_ECAP_PSS(19ULL << 35)
> +#define VTD_ECAP_PASID (1ULL << 40)
> #define VTD_ECAP_SMTS (1ULL << 43)
> #define VTD_ECAP_SLTS (1ULL << 46)
> +#define VTD_ECAP_FLTS (1ULL << 47)
>
> /* CAP_REG */
> /* (offset >> 4) << 24 */
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index 8571a85..1ef2917 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -244,6 +244,8 @@ struct IntelIOMMUState {
>
> bool caching_mode; /* RO - is cap CM enabled? */
> bool scalable_mode; /* RO - is Scalable Mode supported? */
> +char *scalable_mode_str;/* RO - admin's Scalable Mode config */
> +bool scalable_modern; /* RO - is modern SM supported? */
>
> dma_addr_t root;/* Current root table pointer */
> bool root_scalable; /* Type of root table (scalable or not)
> */
> --
> 2.7.4
>
--
Peter Xu
