Hi Clement,
>-----Original Message-----
>From: CLEMENT MATHIEU--DRIF <[email protected]>
>Subject: [PATCH ats_vtd v1 03/24] intel_iommu: check if the input address
>is canonical
>
>First stage translation must fail if the address to translate is
>not canonical.
>
>Signed-off-by: Clément Mathieu--Drif <[email protected]>
>---
> hw/i386/intel_iommu.c | 22 ++++++++++++++++++++++
> hw/i386/intel_iommu_internal.h | 2 ++
> 2 files changed, 24 insertions(+)
>
>diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>index 80cdf37870..240ecb8f72 100644
>--- a/hw/i386/intel_iommu.c
>+++ b/hw/i386/intel_iommu.c
>@@ -1912,6 +1912,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,
> };
>
>@@ -2023,6 +2024,21 @@ static inline uint64_t
>vtd_get_flpte_addr(uint64_t flpte, uint8_t aw)
> return flpte & VTD_FL_PT_BASE_ADDR_MASK(aw);
> }
>
>+/* Return true if IOVA is canonical, otherwise false. */
>+static bool vtd_iova_fl_check_canonical(IntelIOMMUState *s,
>+ uint64_t iova, VTDContextEntry *ce,
>+ uint8_t aw, uint32_t pasid)
>+{
>+ uint64_t iova_limit = vtd_iova_limit(s, ce, aw, pasid);
According to spec:
"Input-address in the request subjected to first-stage translation is not
canonical (i.e., address bits 63:N are not same value as address bits [N-
1], where N is 48 bits with 4-level paging and 57 bits with 5-level paging)."
So it looks not correct to use aw filed in pasid entry to calculate iova_limit.
Aw can be a value configured by guest and it's used for stage-2 table. See spec:
" This field is treated as Reserved(0) for implementations not supporting
Second-stage
Translation (SSTS=0 in the Extended Capability Register).
This field indicates the adjusted guest-address-width (AGAW) to be used by
hardware
for second-stage translation through paging structures referenced through the
SSPTPTR field.
• The following encodings are defined for this field:
• 001b: 39-bit AGAW (3-level page table)
• 010b: 48-bit AGAW (4-level page table)
• 011b: 57-bit AGAW (5-level page table)
• 000b,100b-111b: Reserved
When not treated as Reserved(0), hardware ignores this field for
first-stage-only
(PGTT=001b) and pass-through (PGTT=100b) translations."
Thanks
Zhenzhong
>+ 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.
>@@ -2038,6 +2054,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, aw_bits, 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_fl_level_offset(iova, level);
> flpte = vtd_get_flpte(addr, offset);
>diff --git a/hw/i386/intel_iommu_internal.h
>b/hw/i386/intel_iommu_internal.h
>index 901691afb9..e9448291a4 100644
>--- a/hw/i386/intel_iommu_internal.h
>+++ b/hw/i386/intel_iommu_internal.h
>@@ -324,6 +324,8 @@ typedef enum VTDFaultReason {
> VTD_FR_PASID_ENTRY_P = 0x59, /* The Present(P) field of pasidt-entry is
>0 */
> 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 */
>--
>2.44.0
