diff mbox series

[rfcv2,15/17] intel_iommu: Set default aw_bits to 48 in scalable modren mode

Message ID 20240522062313.453317-16-zhenzhong.duan@intel.com
State New
Headers show
Series intel_iommu: Enable stage-1 translation for emulated device | expand

Commit Message

Duan, Zhenzhong May 22, 2024, 6:23 a.m. UTC 
According to VTD spec, stage-1 page table could support 4-level and
5-level paging.

However, 5-level paging translation emulation is unsupported yet.
That means the only supported value for aw_bits is 48.

So default aw_bits to 48 in scalable modern mode. In other cases,
it is still default to 39 for compatibility.

Add a check to ensure user specified value is 48 in modern mode
for now.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/i386/intel_iommu.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

CLEMENT MATHIEU--DRIF May 24, 2024, 1:56 p.m. UTC | #1 
Hi Zhenzhong

On 22/05/2024 08:23, 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.
>
>
> According to VTD spec, stage-1 page table could support 4-level and
> 5-level paging.
>
> However, 5-level paging translation emulation is unsupported yet.
> That means the only supported value for aw_bits is 48.
>
> So default aw_bits to 48 in scalable modern mode. In other cases,
> it is still default to 39 for compatibility.
>
> Add a check to ensure user specified value is 48 in modern mode
> for now.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/i386/intel_iommu.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index e07daaba99..a4c241ea96 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3748,7 +3748,7 @@ static Property vtd_properties[] = {
>                               ON_OFF_AUTO_AUTO),
>       DEFINE_PROP_BOOL("x-buggy-eim", IntelIOMMUState, buggy_eim, false),
>       DEFINE_PROP_UINT8("aw-bits", IntelIOMMUState, aw_bits,
> -                      VTD_HOST_ADDRESS_WIDTH),
> +                      0xff),
you could define a constant for this invalid value
>       DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, FALSE),
>       DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode, FALSE),
>       DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState, snoop_control, false),
> @@ -4663,6 +4663,14 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
>           }
>       }
>
> +    if (s->aw_bits == 0xff) {
> +        if (s->scalable_modern) {
> +            s->aw_bits = VTD_HOST_AW_48BIT;
> +        } else {
> +            s->aw_bits = VTD_HOST_AW_39BIT;
> +        }
> +    }
> +
>       if ((s->aw_bits != VTD_HOST_AW_39BIT) &&
>           (s->aw_bits != VTD_HOST_AW_48BIT) &&
>           !s->scalable_modern) {
> @@ -4671,6 +4679,12 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
>           return false;
>       }
>
> +    if ((s->aw_bits != VTD_HOST_AW_48BIT) && s->scalable_modern) {
> +        error_setg(errp, "Supported values for aw-bits are: %d",
specify 'in modern mode' in the message?
> +                   VTD_HOST_AW_48BIT);
> +        return false;
> +    }
> +
>       if (s->scalable_mode && !s->dma_drain) {
>           error_setg(errp, "Need to set dma_drain for scalable mode");
>           return false;
> --
> 2.34.1
>
#cmd
Duan, Zhenzhong May 27, 2024, 3:16 a.m. UTC | #2 
Hi Clement,

>-----Original Message-----
>From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>Sent: Friday, May 24, 2024 9:57 PM
>To: Duan, Zhenzhong <zhenzhong.duan@intel.com>; qemu-
>devel@nongnu.org
>Cc: alex.williamson@redhat.com; clg@redhat.com; eric.auger@redhat.com;
>mst@redhat.com; peterx@redhat.com; jasowang@redhat.com;
>jgg@nvidia.com; nicolinc@nvidia.com; joao.m.martins@oracle.com; Tian,
>Kevin <kevin.tian@intel.com>; Liu, Yi L <yi.l.liu@intel.com>; Peng, Chao P
><chao.p.peng@intel.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>Henderson <richard.henderson@linaro.org>; Eduardo Habkost
><eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>Subject: Re: [PATCH rfcv2 15/17] intel_iommu: Set default aw_bits to 48 in
>scalable modren mode
>
>Hi Zhenzhong
>
>On 22/05/2024 08:23, 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.
>>
>>
>> According to VTD spec, stage-1 page table could support 4-level and
>> 5-level paging.
>>
>> However, 5-level paging translation emulation is unsupported yet.
>> That means the only supported value for aw_bits is 48.
>>
>> So default aw_bits to 48 in scalable modern mode. In other cases,
>> it is still default to 39 for compatibility.
>>
>> Add a check to ensure user specified value is 48 in modern mode
>> for now.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   hw/i386/intel_iommu.c | 16 +++++++++++++++-
>>   1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index e07daaba99..a4c241ea96 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -3748,7 +3748,7 @@ static Property vtd_properties[] = {
>>                               ON_OFF_AUTO_AUTO),
>>       DEFINE_PROP_BOOL("x-buggy-eim", IntelIOMMUState, buggy_eim,
>false),
>>       DEFINE_PROP_UINT8("aw-bits", IntelIOMMUState, aw_bits,
>> -                      VTD_HOST_ADDRESS_WIDTH),
>> +                      0xff),
>you could define a constant for this invalid value

Sure, maybe VTD_HOST_ADDRESS_WIDTH_UNDEFINED?

Thanks
Zhenzhong

>>       DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode,
>FALSE),
>>       DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState,
>scalable_mode, FALSE),
>>       DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState,
>snoop_control, false),
>> @@ -4663,6 +4663,14 @@ static bool
>vtd_decide_config(IntelIOMMUState *s, Error **errp)
>>           }
>>       }
>>
>> +    if (s->aw_bits == 0xff) {
>> +        if (s->scalable_modern) {
>> +            s->aw_bits = VTD_HOST_AW_48BIT;
>> +        } else {
>> +            s->aw_bits = VTD_HOST_AW_39BIT;
>> +        }
>> +    }
>> +
>>       if ((s->aw_bits != VTD_HOST_AW_39BIT) &&
>>           (s->aw_bits != VTD_HOST_AW_48BIT) &&
>>           !s->scalable_modern) {
>> @@ -4671,6 +4679,12 @@ static bool
>vtd_decide_config(IntelIOMMUState *s, Error **errp)
>>           return false;
>>       }
>>
>> +    if ((s->aw_bits != VTD_HOST_AW_48BIT) && s->scalable_modern) {
>> +        error_setg(errp, "Supported values for aw-bits are: %d",
>specify 'in modern mode' in the message?
>> +                   VTD_HOST_AW_48BIT);
>> +        return false;
>> +    }
>> +
>>       if (s->scalable_mode && !s->dma_drain) {
>>           error_setg(errp, "Need to set dma_drain for scalable mode");
>>           return false;
>> --
>> 2.34.1
>>
>#cmd
CLEMENT MATHIEU--DRIF May 27, 2024, 5:14 a.m. UTC | #3 
On 27/05/2024 05:16, 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.
>
>
> Hi Clement,
>
>> -----Original Message-----
>> From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>> Sent: Friday, May 24, 2024 9:57 PM
>> To: Duan, Zhenzhong <zhenzhong.duan@intel.com>; qemu-
>> devel@nongnu.org
>> Cc: alex.williamson@redhat.com; clg@redhat.com; eric.auger@redhat.com;
>> mst@redhat.com; peterx@redhat.com; jasowang@redhat.com;
>> jgg@nvidia.com; nicolinc@nvidia.com; joao.m.martins@oracle.com; Tian,
>> Kevin <kevin.tian@intel.com>; Liu, Yi L <yi.l.liu@intel.com>; Peng, Chao P
>> <chao.p.peng@intel.com>; Paolo Bonzini <pbonzini@redhat.com>; Richard
>> Henderson <richard.henderson@linaro.org>; Eduardo Habkost
>> <eduardo@habkost.net>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Subject: Re: [PATCH rfcv2 15/17] intel_iommu: Set default aw_bits to 48 in
>> scalable modren mode
>>
>> Hi Zhenzhong
>>
>> On 22/05/2024 08:23, 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.
>>>
>>> According to VTD spec, stage-1 page table could support 4-level and
>>> 5-level paging.
>>>
>>> However, 5-level paging translation emulation is unsupported yet.
>>> That means the only supported value for aw_bits is 48.
>>>
>>> So default aw_bits to 48 in scalable modern mode. In other cases,
>>> it is still default to 39 for compatibility.
>>>
>>> Add a check to ensure user specified value is 48 in modern mode
>>> for now.
>>>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>    hw/i386/intel_iommu.c | 16 +++++++++++++++-
>>>    1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>> index e07daaba99..a4c241ea96 100644
>>> --- a/hw/i386/intel_iommu.c
>>> +++ b/hw/i386/intel_iommu.c
>>> @@ -3748,7 +3748,7 @@ static Property vtd_properties[] = {
>>>                                ON_OFF_AUTO_AUTO),
>>>        DEFINE_PROP_BOOL("x-buggy-eim", IntelIOMMUState, buggy_eim,
>> false),
>>>        DEFINE_PROP_UINT8("aw-bits", IntelIOMMUState, aw_bits,
>>> -                      VTD_HOST_ADDRESS_WIDTH),
>>> +                      0xff),
>> you could define a constant for this invalid value
> Sure, maybe VTD_HOST_ADDRESS_WIDTH_UNDEFINED?
Yes, fine for me
>
> Thanks
> Zhenzhong
>
>>>        DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode,
>> FALSE),
>>>        DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState,
>> scalable_mode, FALSE),
>>>        DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState,
>> snoop_control, false),
>>> @@ -4663,6 +4663,14 @@ static bool
>> vtd_decide_config(IntelIOMMUState *s, Error **errp)
>>>            }
>>>        }
>>>
>>> +    if (s->aw_bits == 0xff) {
>>> +        if (s->scalable_modern) {
>>> +            s->aw_bits = VTD_HOST_AW_48BIT;
>>> +        } else {
>>> +            s->aw_bits = VTD_HOST_AW_39BIT;
>>> +        }
>>> +    }
>>> +
>>>        if ((s->aw_bits != VTD_HOST_AW_39BIT) &&
>>>            (s->aw_bits != VTD_HOST_AW_48BIT) &&
>>>            !s->scalable_modern) {
>>> @@ -4671,6 +4679,12 @@ static bool
>> vtd_decide_config(IntelIOMMUState *s, Error **errp)
>>>            return false;
>>>        }
>>>
>>> +    if ((s->aw_bits != VTD_HOST_AW_48BIT) && s->scalable_modern) {
>>> +        error_setg(errp, "Supported values for aw-bits are: %d",
>> specify 'in modern mode' in the message?
>>> +                   VTD_HOST_AW_48BIT);
>>> +        return false;
>>> +    }
>>> +
>>>        if (s->scalable_mode && !s->dma_drain) {
>>>            error_setg(errp, "Need to set dma_drain for scalable mode");
>>>            return false;
>>> --
>>> 2.34.1
>>>
>> #cmd
diff mbox series

Patch

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index e07daaba99..a4c241ea96 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3748,7 +3748,7 @@  static Property vtd_properties[] = {
                             ON_OFF_AUTO_AUTO),
     DEFINE_PROP_BOOL("x-buggy-eim", IntelIOMMUState, buggy_eim, false),
     DEFINE_PROP_UINT8("aw-bits", IntelIOMMUState, aw_bits,
-                      VTD_HOST_ADDRESS_WIDTH),
+                      0xff),
     DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, FALSE),
     DEFINE_PROP_BOOL("x-scalable-mode", IntelIOMMUState, scalable_mode, FALSE),
     DEFINE_PROP_BOOL("snoop-control", IntelIOMMUState, snoop_control, false),
@@ -4663,6 +4663,14 @@  static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
         }
     }
 
+    if (s->aw_bits == 0xff) {
+        if (s->scalable_modern) {
+            s->aw_bits = VTD_HOST_AW_48BIT;
+        } else {
+            s->aw_bits = VTD_HOST_AW_39BIT;
+        }
+    }
+
     if ((s->aw_bits != VTD_HOST_AW_39BIT) &&
         (s->aw_bits != VTD_HOST_AW_48BIT) &&
         !s->scalable_modern) {
@@ -4671,6 +4679,12 @@  static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
         return false;
     }
 
+    if ((s->aw_bits != VTD_HOST_AW_48BIT) && s->scalable_modern) {
+        error_setg(errp, "Supported values for aw-bits are: %d",
+                   VTD_HOST_AW_48BIT);
+        return false;
+    }
+
     if (s->scalable_mode && !s->dma_drain) {
         error_setg(errp, "Need to set dma_drain for scalable mode");
         return false;