1. Patchew
  2. QEMU
  3. [PATCH v1 00/17] intel_iommu: Enable stage-1 translation for emulated device
  4. View patch

[PATCH v1 16/17] intel_iommu: Modify x-scalable-mode to be string option

Zhenzhong Duan posted 17 patches 2 months ago
Only 14 patches received!
[PATCH v1 16/17] intel_iommu: Modify x-scalable-mode to be string option
Posted by Zhenzhong Duan 2 months ago 
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

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
Re: [PATCH v1 16/17] intel_iommu: Modify x-scalable-mode to be string option
Posted by CLEMENT MATHIEU--DRIF 2 months ago 

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
RE: [PATCH v1 16/17] intel_iommu: Modify x-scalable-mode to be string option
Posted by Duan, Zhenzhong 1 month, 4 weeks ago 

>-----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
Re: [PATCH v1 16/17] intel_iommu: Modify x-scalable-mode to be string option
Posted by CLEMENT MATHIEU--DRIF 1 month, 4 weeks ago 

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

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.