| Message ID | 20240718081636.879544-17-zhenzhong.duan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | intel_iommu: Enable stage-1 translation for emulated device | expand |
On 18/07/2024 10:16, Zhenzhong Duan wrote: > Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe. > > > From: Yi Liu <yi.l.liu@intel.com> > > 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 to 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"|"off"]" > > - "legacy": gives support for stage-2 page table > - "modern": gives support for stage-1 page table > - "off": no scalable mode support > - if not configured, means no scalable mode support, if not proper > configured, will throw error s/proper/properly Maybe we could split and rephrase the last bullet point to make it clear that "off" is equivalent to not using the option at all > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > --- > include/hw/i386/intel_iommu.h | 1 + > hw/i386/intel_iommu.c | 24 +++++++++++++++++++++++- > 2 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h > index 48134bda11..650641544c 100644 > --- a/include/hw/i386/intel_iommu.h > +++ b/include/hw/i386/intel_iommu.h > @@ -263,6 +263,7 @@ 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? */ > bool snoop_control; /* RO - is SNP filed supported? */ > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 2804c3628a..14d05fce1d 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -3770,7 +3770,7 @@ static Property vtd_properties[] = { > DEFINE_PROP_UINT8("aw-bits", IntelIOMMUState, aw_bits, > VTD_HOST_AW_AUTO), > 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("snoop-control", IntelIOMMUState, snoop_control, false), > DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false), > DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true), > @@ -4686,6 +4686,28 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp) > } > } > > + if (s->scalable_mode_str && > + (strcmp(s->scalable_mode_str, "off") && > + strcmp(s->scalable_mode_str, "modern") && > + strcmp(s->scalable_mode_str, "legacy"))) { > + error_setg(errp, "Invalid x-scalable-mode config," > + "Please use \"modern\", \"legacy\" or \"off\""); > + 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; > + } > + > if (s->aw_bits == VTD_HOST_AW_AUTO) { > if (s->scalable_modern) { > s->aw_bits = VTD_HOST_AW_48BIT; > -- > 2.34.1 > LGTM
>-----Original Message----- >From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com> >Subject: Re: [PATCH v1 16/17] intel_iommu: Modify x-scalable-mode to be >string option > > > >On 18/07/2024 10:16, Zhenzhong Duan wrote: >> Caution: External email. Do not open attachments or click links, unless this >email comes from a known sender and you know the content is safe. >> >> >> From: Yi Liu <yi.l.liu@intel.com> >> >> 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 to 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"|"off"]" >> >> - "legacy": gives support for stage-2 page table >> - "modern": gives support for stage-1 page table >> - "off": no scalable mode support >> - if not configured, means no scalable mode support, if not proper >> configured, will throw error >s/proper/properly >Maybe we could split and rephrase the last bullet point to make it clear >that "off" is equivalent to not using the option at all You mean split last bullet as a separate paragraph? Then what about below: - "legacy": gives support for stage-2 page table - "modern": gives support for stage-1 page table - "off": no scalable mode support - any other string, will throw error If x-scalable-mode is not configured, it is equivalent to x-scalable-mode=off. Thanks Zhenzhong >> >> Signed-off-by: Yi Liu <yi.l.liu@intel.com> >> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >> --- >> include/hw/i386/intel_iommu.h | 1 + >> hw/i386/intel_iommu.c | 24 +++++++++++++++++++++++- >> 2 files changed, 24 insertions(+), 1 deletion(-) >> >> diff --git a/include/hw/i386/intel_iommu.h >b/include/hw/i386/intel_iommu.h >> index 48134bda11..650641544c 100644 >> --- a/include/hw/i386/intel_iommu.h >> +++ b/include/hw/i386/intel_iommu.h >> @@ -263,6 +263,7 @@ 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? */ >> bool snoop_control; /* RO - is SNP filed supported? */ >> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> index 2804c3628a..14d05fce1d 100644 >> --- a/hw/i386/intel_iommu.c >> +++ b/hw/i386/intel_iommu.c >> @@ -3770,7 +3770,7 @@ static Property vtd_properties[] = { >> DEFINE_PROP_UINT8("aw-bits", IntelIOMMUState, aw_bits, >> VTD_HOST_AW_AUTO), >> 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("snoop-control", IntelIOMMUState, >snoop_control, false), >> DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false), >> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true), >> @@ -4686,6 +4686,28 @@ static bool >vtd_decide_config(IntelIOMMUState *s, Error **errp) >> } >> } >> >> + if (s->scalable_mode_str && >> + (strcmp(s->scalable_mode_str, "off") && >> + strcmp(s->scalable_mode_str, "modern") && >> + strcmp(s->scalable_mode_str, "legacy"))) { >> + error_setg(errp, "Invalid x-scalable-mode config," >> + "Please use \"modern\", \"legacy\" or \"off\""); >> + 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; >> + } >> + >> if (s->aw_bits == VTD_HOST_AW_AUTO) { >> if (s->scalable_modern) { >> s->aw_bits = VTD_HOST_AW_48BIT; >> -- >> 2.34.1 >> >LGTM
On 19/07/2024 04:53, Duan, Zhenzhong wrote: > Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe. > > >> -----Original Message----- >> From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com> >> Subject: Re: [PATCH v1 16/17] intel_iommu: Modify x-scalable-mode to be >> string option >> >> >> >> On 18/07/2024 10:16, Zhenzhong Duan wrote: >>> Caution: External email. Do not open attachments or click links, unless this >> email comes from a known sender and you know the content is safe. >>> >>> From: Yi Liu <yi.l.liu@intel.com> >>> >>> 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 to 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"|"off"]" >>> >>> - "legacy": gives support for stage-2 page table >>> - "modern": gives support for stage-1 page table >>> - "off": no scalable mode support >>> - if not configured, means no scalable mode support, if not proper >>> configured, will throw error >> s/proper/properly >> Maybe we could split and rephrase the last bullet point to make it clear >> that "off" is equivalent to not using the option at all > You mean split last bullet as a separate paragraph? > Then what about below: > > - "legacy": gives support for stage-2 page table > - "modern": gives support for stage-1 page table > - "off": no scalable mode support > - any other string, will throw error > > If x-scalable-mode is not configured, it is equivalent to x-scalable-mode=off. Yes, lgtm > > Thanks > Zhenzhong > >>> Signed-off-by: Yi Liu <yi.l.liu@intel.com> >>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> >>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >>> --- >>> include/hw/i386/intel_iommu.h | 1 + >>> hw/i386/intel_iommu.c | 24 +++++++++++++++++++++++- >>> 2 files changed, 24 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/hw/i386/intel_iommu.h >> b/include/hw/i386/intel_iommu.h >>> index 48134bda11..650641544c 100644 >>> --- a/include/hw/i386/intel_iommu.h >>> +++ b/include/hw/i386/intel_iommu.h >>> @@ -263,6 +263,7 @@ 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? */ >>> bool snoop_control; /* RO - is SNP filed supported? */ >>> >>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >>> index 2804c3628a..14d05fce1d 100644 >>> --- a/hw/i386/intel_iommu.c >>> +++ b/hw/i386/intel_iommu.c >>> @@ -3770,7 +3770,7 @@ static Property vtd_properties[] = { >>> DEFINE_PROP_UINT8("aw-bits", IntelIOMMUState, aw_bits, >>> VTD_HOST_AW_AUTO), >>> 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("snoop-control", IntelIOMMUState, >> snoop_control, false), >>> DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false), >>> DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true), >>> @@ -4686,6 +4686,28 @@ static bool >> vtd_decide_config(IntelIOMMUState *s, Error **errp) >>> } >>> } >>> >>> + if (s->scalable_mode_str && >>> + (strcmp(s->scalable_mode_str, "off") && >>> + strcmp(s->scalable_mode_str, "modern") && >>> + strcmp(s->scalable_mode_str, "legacy"))) { >>> + error_setg(errp, "Invalid x-scalable-mode config," >>> + "Please use \"modern\", \"legacy\" or \"off\""); >>> + 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; >>> + } >>> + >>> if (s->aw_bits == VTD_HOST_AW_AUTO) { >>> if (s->scalable_modern) { >>> s->aw_bits = VTD_HOST_AW_48BIT; >>> -- >>> 2.34.1 >>> >> LGTM
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h index 48134bda11..650641544c 100644 --- a/include/hw/i386/intel_iommu.h +++ b/include/hw/i386/intel_iommu.h @@ -263,6 +263,7 @@ 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? */ bool snoop_control; /* RO - is SNP filed supported? */ diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 2804c3628a..14d05fce1d 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -3770,7 +3770,7 @@ static Property vtd_properties[] = { DEFINE_PROP_UINT8("aw-bits", IntelIOMMUState, aw_bits, VTD_HOST_AW_AUTO), 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("snoop-control", IntelIOMMUState, snoop_control, false), DEFINE_PROP_BOOL("x-pasid-mode", IntelIOMMUState, pasid, false), DEFINE_PROP_BOOL("dma-drain", IntelIOMMUState, dma_drain, true), @@ -4686,6 +4686,28 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp) } } + if (s->scalable_mode_str && + (strcmp(s->scalable_mode_str, "off") && + strcmp(s->scalable_mode_str, "modern") && + strcmp(s->scalable_mode_str, "legacy"))) { + error_setg(errp, "Invalid x-scalable-mode config," + "Please use \"modern\", \"legacy\" or \"off\""); + 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; + } + if (s->aw_bits == VTD_HOST_AW_AUTO) { if (s->scalable_modern) { s->aw_bits = VTD_HOST_AW_48BIT;