diff mbox series

[v2,07/17] intel_iommu: Check if the input address is canonical

Message ID 20240805062727.2307552-8-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 
From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>

First stage translation must fail if the address to translate is
not canonical.

Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/i386/intel_iommu_internal.h |  2 ++
 hw/i386/intel_iommu.c          | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+)

Comments

Yi Liu Aug. 14, 2024, 12:02 p.m. UTC | #1 
On 2024/8/5 14:27, Zhenzhong Duan wrote:
> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> 
> First stage translation must fail if the address to translate is
> not canonical.
> 
> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/i386/intel_iommu_internal.h |  2 ++
>   hw/i386/intel_iommu.c          | 21 +++++++++++++++++++++
>   2 files changed, 23 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 51e9b1fc43..668583aeca 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -320,6 +320,8 @@ typedef enum VTDFaultReason {
>       VTD_FR_PASID_ENTRY_P = 0x59,
>       VTD_FR_PASID_TABLE_ENTRY_INV = 0x5b,  /*Invalid PASID table entry */
>   
> +    VTD_FR_FS_NON_CANONICAL = 0x80, /* SNG.1 : Address for FS not canonical.*/
> +
>       /* Output address in the interrupt address range for scalable mode */
>       VTD_FR_SM_INTERRUPT_ADDR = 0x87,
>       VTD_FR_MAX,                 /* Guard */
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 0bcbd5b777..6121cca4cd 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1821,6 +1821,7 @@ static const bool vtd_qualified_faults[] = {
>       [VTD_FR_PASID_ENTRY_P] = true,
>       [VTD_FR_PASID_TABLE_ENTRY_INV] = true,
>       [VTD_FR_SM_INTERRUPT_ADDR] = true,
> +    [VTD_FR_FS_NON_CANONICAL] = true,
>       [VTD_FR_MAX] = false,
>   };
>   
> @@ -1924,6 +1925,20 @@ static inline bool vtd_flpte_present(uint64_t flpte)
>       return !!(flpte & VTD_FL_P);
>   }
>   
> +/* Return true if IOVA is canonical, otherwise false. */
> +static bool vtd_iova_fl_check_canonical(IntelIOMMUState *s, uint64_t iova,
> +                                        VTDContextEntry *ce, uint32_t pasid)
> +{
> +    uint64_t iova_limit = vtd_iova_limit(s, ce, s->aw_bits, pasid);
> +    uint64_t upper_bits_mask = ~(iova_limit - 1);
> +    uint64_t upper_bits = iova & upper_bits_mask;
> +    bool msb = ((iova & (iova_limit >> 1)) != 0);
> +    return !(
> +             (!msb && (upper_bits != 0)) ||
> +             (msb && (upper_bits != upper_bits_mask))
> +            );
> +}
> +

will the below be clearer?

     if (msb)
         return upper_bits == upper_bits_mask;
     else
         return !upper_bits;

>   /*
>    * Given the @iova, get relevant @flptep. @flpte_level will be the last level
>    * of the translation, can be used for deciding the size of large page.
> @@ -1939,6 +1954,12 @@ static int vtd_iova_to_flpte(IntelIOMMUState *s, VTDContextEntry *ce,
>       uint32_t offset;
>       uint64_t flpte;
>   
> +    if (!vtd_iova_fl_check_canonical(s, iova, ce, pasid)) {
> +        error_report_once("%s: detected non canonical IOVA (iova=0x%" PRIx64 ","
> +                          "pasid=0x%" PRIx32 ")", __func__, iova, pasid);
> +        return -VTD_FR_FS_NON_CANONICAL;
> +    }
> +
>       while (true) {
>           offset = vtd_iova_level_offset(iova, level);
>           flpte = vtd_get_pte(addr, offset);
Duan, Zhenzhong Aug. 16, 2024, 2:19 a.m. UTC | #2 
>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v2 07/17] intel_iommu: Check if the input address is
>canonical
>
>On 2024/8/5 14:27, Zhenzhong Duan wrote:
>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>
>> First stage translation must fail if the address to translate is
>> not canonical.
>>
>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   hw/i386/intel_iommu_internal.h |  2 ++
>>   hw/i386/intel_iommu.c          | 21 +++++++++++++++++++++
>>   2 files changed, 23 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu_internal.h
>b/hw/i386/intel_iommu_internal.h
>> index 51e9b1fc43..668583aeca 100644
>> --- a/hw/i386/intel_iommu_internal.h
>> +++ b/hw/i386/intel_iommu_internal.h
>> @@ -320,6 +320,8 @@ typedef enum VTDFaultReason {
>>       VTD_FR_PASID_ENTRY_P = 0x59,
>>       VTD_FR_PASID_TABLE_ENTRY_INV = 0x5b,  /*Invalid PASID table entry
>*/
>>
>> +    VTD_FR_FS_NON_CANONICAL = 0x80, /* SNG.1 : Address for FS not
>canonical.*/
>> +
>>       /* Output address in the interrupt address range for scalable mode */
>>       VTD_FR_SM_INTERRUPT_ADDR = 0x87,
>>       VTD_FR_MAX,                 /* Guard */
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 0bcbd5b777..6121cca4cd 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -1821,6 +1821,7 @@ static const bool vtd_qualified_faults[] = {
>>       [VTD_FR_PASID_ENTRY_P] = true,
>>       [VTD_FR_PASID_TABLE_ENTRY_INV] = true,
>>       [VTD_FR_SM_INTERRUPT_ADDR] = true,
>> +    [VTD_FR_FS_NON_CANONICAL] = true,
>>       [VTD_FR_MAX] = false,
>>   };
>>
>> @@ -1924,6 +1925,20 @@ static inline bool vtd_flpte_present(uint64_t
>flpte)
>>       return !!(flpte & VTD_FL_P);
>>   }
>>
>> +/* Return true if IOVA is canonical, otherwise false. */
>> +static bool vtd_iova_fl_check_canonical(IntelIOMMUState *s, uint64_t
>iova,
>> +                                        VTDContextEntry *ce, uint32_t pasid)
>> +{
>> +    uint64_t iova_limit = vtd_iova_limit(s, ce, s->aw_bits, pasid);
>> +    uint64_t upper_bits_mask = ~(iova_limit - 1);
>> +    uint64_t upper_bits = iova & upper_bits_mask;
>> +    bool msb = ((iova & (iova_limit >> 1)) != 0);
>> +    return !(
>> +             (!msb && (upper_bits != 0)) ||
>> +             (msb && (upper_bits != upper_bits_mask))
>> +            );
>> +}
>> +
>
>will the below be clearer?
>
>     if (msb)
>         return upper_bits == upper_bits_mask;
>     else
>         return !upper_bits;

Yes, clearer, will do.

Thanks
Zhenzhong
diff mbox series

Patch

diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 51e9b1fc43..668583aeca 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -320,6 +320,8 @@  typedef enum VTDFaultReason {
     VTD_FR_PASID_ENTRY_P = 0x59,
     VTD_FR_PASID_TABLE_ENTRY_INV = 0x5b,  /*Invalid PASID table entry */
 
+    VTD_FR_FS_NON_CANONICAL = 0x80, /* SNG.1 : Address for FS not canonical.*/
+
     /* Output address in the interrupt address range for scalable mode */
     VTD_FR_SM_INTERRUPT_ADDR = 0x87,
     VTD_FR_MAX,                 /* Guard */
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 0bcbd5b777..6121cca4cd 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -1821,6 +1821,7 @@  static const bool vtd_qualified_faults[] = {
     [VTD_FR_PASID_ENTRY_P] = true,
     [VTD_FR_PASID_TABLE_ENTRY_INV] = true,
     [VTD_FR_SM_INTERRUPT_ADDR] = true,
+    [VTD_FR_FS_NON_CANONICAL] = true,
     [VTD_FR_MAX] = false,
 };
 
@@ -1924,6 +1925,20 @@  static inline bool vtd_flpte_present(uint64_t flpte)
     return !!(flpte & VTD_FL_P);
 }
 
+/* Return true if IOVA is canonical, otherwise false. */
+static bool vtd_iova_fl_check_canonical(IntelIOMMUState *s, uint64_t iova,
+                                        VTDContextEntry *ce, uint32_t pasid)
+{
+    uint64_t iova_limit = vtd_iova_limit(s, ce, s->aw_bits, pasid);
+    uint64_t upper_bits_mask = ~(iova_limit - 1);
+    uint64_t upper_bits = iova & upper_bits_mask;
+    bool msb = ((iova & (iova_limit >> 1)) != 0);
+    return !(
+             (!msb && (upper_bits != 0)) ||
+             (msb && (upper_bits != upper_bits_mask))
+            );
+}
+
 /*
  * Given the @iova, get relevant @flptep. @flpte_level will be the last level
  * of the translation, can be used for deciding the size of large page.
@@ -1939,6 +1954,12 @@  static int vtd_iova_to_flpte(IntelIOMMUState *s, VTDContextEntry *ce,
     uint32_t offset;
     uint64_t flpte;
 
+    if (!vtd_iova_fl_check_canonical(s, iova, ce, pasid)) {
+        error_report_once("%s: detected non canonical IOVA (iova=0x%" PRIx64 ","
+                          "pasid=0x%" PRIx32 ")", __func__, iova, pasid);
+        return -VTD_FR_FS_NON_CANONICAL;
+    }
+
     while (true) {
         offset = vtd_iova_level_offset(iova, level);
         flpte = vtd_get_pte(addr, offset);