diff mbox series

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

Message ID 20240805062727.2307552-15-zhenzhong.duan@intel.com (mailing list archive)
State New, archived
Headers show
Series intel_iommu: Enable stage-1 translation for emulated device | expand

Commit Message

Duan, Zhenzhong Aug. 5, 2024, 6:27 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>
Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>
---
 include/hw/i386/intel_iommu.h |  2 +-
 hw/i386/intel_iommu.c         | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

Comments

CLEMENT MATHIEU--DRIF Aug. 6, 2024, 6:35 a.m. UTC | #1
Typo in the title : s/modren/modern

Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>


On 05/08/2024 08:27, 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>
> Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>
> ---
>   include/hw/i386/intel_iommu.h |  2 +-
>   hw/i386/intel_iommu.c         | 16 +++++++++++++++-
>   2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index b843d069cc..48134bda11 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -45,7 +45,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IntelIOMMUState, INTEL_IOMMU_DEVICE)
>   #define DMAR_REG_SIZE               0x230
>   #define VTD_HOST_AW_39BIT           39
>   #define VTD_HOST_AW_48BIT           48
> -#define VTD_HOST_ADDRESS_WIDTH      VTD_HOST_AW_39BIT
> +#define VTD_HOST_AW_AUTO            0xff
>   #define VTD_HAW_MASK(aw)            ((1ULL << (aw)) - 1)
>
>   #define DMAR_REPORT_F_INTR          (1)
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 317e630e08..5469ab4f9b 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3770,7 +3770,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),
> +                      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_BOOL("snoop-control", IntelIOMMUState, snoop_control, false),
> @@ -4685,6 +4685,14 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
>           }
>       }
>
> +    if (s->aw_bits == VTD_HOST_AW_AUTO) {
> +        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) {
> @@ -4693,6 +4701,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;
> --
> 2.34.1
>
Yi Liu Aug. 14, 2024, 12:26 p.m. UTC | #2
On 2024/8/5 14:27, Zhenzhong Duan wrote:
> 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>
> Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>
> ---
>   include/hw/i386/intel_iommu.h |  2 +-
>   hw/i386/intel_iommu.c         | 16 +++++++++++++++-
>   2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index b843d069cc..48134bda11 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -45,7 +45,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IntelIOMMUState, INTEL_IOMMU_DEVICE)
>   #define DMAR_REG_SIZE               0x230
>   #define VTD_HOST_AW_39BIT           39
>   #define VTD_HOST_AW_48BIT           48
> -#define VTD_HOST_ADDRESS_WIDTH      VTD_HOST_AW_39BIT
> +#define VTD_HOST_AW_AUTO            0xff
>   #define VTD_HAW_MASK(aw)            ((1ULL << (aw)) - 1)
>   
>   #define DMAR_REPORT_F_INTR          (1)
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 317e630e08..5469ab4f9b 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3770,7 +3770,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),
> +                      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_BOOL("snoop-control", IntelIOMMUState, snoop_control, false),
> @@ -4685,6 +4685,14 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
>           }
>       }
>   
> +    if (s->aw_bits == VTD_HOST_AW_AUTO) {
> +        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) {
> @@ -4693,6 +4701,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);

call out it is for scalable modern.:)

> +        return false;
> +    }
> +
>       if (s->scalable_mode && !s->dma_drain) {
>           error_setg(errp, "Need to set dma_drain for scalable mode");
>           return false;
Duan, Zhenzhong Aug. 15, 2024, 3:39 a.m. UTC | #3
>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v2 14/17] intel_iommu: Set default aw_bits to 48 in
>scalable modren mode
>
>On 2024/8/5 14:27, Zhenzhong Duan wrote:
>> 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>
>> Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>
>> ---
>>   include/hw/i386/intel_iommu.h |  2 +-
>>   hw/i386/intel_iommu.c         | 16 +++++++++++++++-
>>   2 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/i386/intel_iommu.h
>b/include/hw/i386/intel_iommu.h
>> index b843d069cc..48134bda11 100644
>> --- a/include/hw/i386/intel_iommu.h
>> +++ b/include/hw/i386/intel_iommu.h
>> @@ -45,7 +45,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IntelIOMMUState,
>INTEL_IOMMU_DEVICE)
>>   #define DMAR_REG_SIZE               0x230
>>   #define VTD_HOST_AW_39BIT           39
>>   #define VTD_HOST_AW_48BIT           48
>> -#define VTD_HOST_ADDRESS_WIDTH      VTD_HOST_AW_39BIT
>> +#define VTD_HOST_AW_AUTO            0xff
>>   #define VTD_HAW_MASK(aw)            ((1ULL << (aw)) - 1)
>>
>>   #define DMAR_REPORT_F_INTR          (1)
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 317e630e08..5469ab4f9b 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -3770,7 +3770,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),
>> +                      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_BOOL("snoop-control", IntelIOMMUState,
>snoop_control, false),
>> @@ -4685,6 +4685,14 @@ static bool
>vtd_decide_config(IntelIOMMUState *s, Error **errp)
>>           }
>>       }
>>
>> +    if (s->aw_bits == VTD_HOST_AW_AUTO) {
>> +        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) {
>> @@ -4693,6 +4701,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);
>
>call out it is for scalable modern.:)

Sure, will be:

    if ((s->aw_bits != VTD_HOST_AW_39BIT) &&
        (s->aw_bits != VTD_HOST_AW_48BIT) &&
        !s->scalable_modern) {
        error_setg(errp, "%s mode: supported values for aw-bits are: %d, %d",
                   s->scalable_mode ? "Scalable legacy" : "Legacy",
                   VTD_HOST_AW_39BIT, VTD_HOST_AW_48BIT);
        return false;
    }

    if ((s->aw_bits != VTD_HOST_AW_48BIT) && s->scalable_modern) {
        error_setg(errp,
                   "Scalable modern mode: supported values for aw-bits is: %d",
                   VTD_HOST_AW_48BIT);
        return false;
    }

Thanks
Zhenzhong
diff mbox series

Patch

diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index b843d069cc..48134bda11 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -45,7 +45,7 @@  OBJECT_DECLARE_SIMPLE_TYPE(IntelIOMMUState, INTEL_IOMMU_DEVICE)
 #define DMAR_REG_SIZE               0x230
 #define VTD_HOST_AW_39BIT           39
 #define VTD_HOST_AW_48BIT           48
-#define VTD_HOST_ADDRESS_WIDTH      VTD_HOST_AW_39BIT
+#define VTD_HOST_AW_AUTO            0xff
 #define VTD_HAW_MASK(aw)            ((1ULL << (aw)) - 1)
 
 #define DMAR_REPORT_F_INTR          (1)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 317e630e08..5469ab4f9b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3770,7 +3770,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),
+                      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_BOOL("snoop-control", IntelIOMMUState, snoop_control, false),
@@ -4685,6 +4685,14 @@  static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
         }
     }
 
+    if (s->aw_bits == VTD_HOST_AW_AUTO) {
+        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) {
@@ -4693,6 +4701,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;