>-----Original Message----- >From: Liu, Yi L <yi.l....@intel.com> >Subject: RE: [PATCH] intel_iommu: Use the latest fault reasons defined by >spec > >> From: CLEMENT MATHIEU--DRIF <clement.mathieu--d...@eviden.com> >> Sent: Friday, May 17, 2024 9:13 PM >> >> Hi Zhenzhong >> >> On 17/05/2024 12: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. >> > >> > >> > From: Yu Zhang <yu.c.zh...@linux.intel.com> >> > >> > Currently we use only VTD_FR_PASID_TABLE_INV as fault reason. >> > Update with more detailed fault reasons listed in VT-d spec 7.2.3. >> > >> > Signed-off-by: Yu Zhang <yu.c.zh...@linux.intel.com> >> > Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com> >> > --- >> > hw/i386/intel_iommu_internal.h | 8 +++++++- >> > hw/i386/intel_iommu.c | 25 ++++++++++++++++--------- >> > 2 files changed, 23 insertions(+), 10 deletions(-) >> > >> > diff --git a/hw/i386/intel_iommu_internal.h >b/hw/i386/intel_iommu_internal.h >> > index f8cf99bddf..666e2cf2ce 100644 >> > --- a/hw/i386/intel_iommu_internal.h >> > +++ b/hw/i386/intel_iommu_internal.h >> > @@ -311,7 +311,13 @@ typedef enum VTDFaultReason { >> > * request while disabled */ >> > VTD_FR_IR_SID_ERR = 0x26, /* Invalid Source-ID */ >> > >> > - VTD_FR_PASID_TABLE_INV = 0x58, /*Invalid PASID table entry */ >> > + /* PASID directory entry access failure */ >> > + VTD_FR_PASID_DIR_ACCESS_ERR = 0x50, >> > + /* The Present(P) field of pasid directory entry is 0 */ >> > + VTD_FR_PASID_DIR_ENTRY_P = 0x51, >> > + VTD_FR_PASID_TABLE_ACCESS_ERR = 0x58, /* PASID table entry >access failure */ >> > + VTD_FR_PASID_ENTRY_P = 0x59, /* The Present(P) field of pasidt- >entry is 0 */ >> s/pasidt/pasid > >Per spec, it is pasid table entry. So Zhenzhong may need to use the same >word >With the line below. E.g. PASID Table entry.
Yes, will fix. Thanks Zhenzhong > >Regards, >Yi Liu > >> > + VTD_FR_PASID_TABLE_ENTRY_INV = 0x5b, /*Invalid PASID table >entry */ >> > >> > /* Output address in the interrupt address range for scalable mode >*/ >> > VTD_FR_SM_INTERRUPT_ADDR = 0x87, >> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> > index cc8e59674e..0951ebb71d 100644 >> > --- a/hw/i386/intel_iommu.c >> > +++ b/hw/i386/intel_iommu.c >> > @@ -771,7 +771,7 @@ static int >vtd_get_pdire_from_pdir_table(dma_addr_t >> pasid_dir_base, >> > addr = pasid_dir_base + index * entry_size; >> > if (dma_memory_read(&address_space_memory, addr, >> > pdire, entry_size, MEMTXATTRS_UNSPECIFIED)) { >> > - return -VTD_FR_PASID_TABLE_INV; >> > + return -VTD_FR_PASID_DIR_ACCESS_ERR; >> > } >> > >> > pdire->val = le64_to_cpu(pdire->val); >> > @@ -789,6 +789,7 @@ static int >vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState >> *s, >> > dma_addr_t addr, >> > VTDPASIDEntry *pe) >> > { >> > + uint8_t pgtt; >> > uint32_t index; >> > dma_addr_t entry_size; >> > X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s); >> > @@ -798,7 +799,7 @@ static int >vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState >> *s, >> > addr = addr + index * entry_size; >> > if (dma_memory_read(&address_space_memory, addr, >> > pe, entry_size, MEMTXATTRS_UNSPECIFIED)) { >> > - return -VTD_FR_PASID_TABLE_INV; >> > + return -VTD_FR_PASID_TABLE_ACCESS_ERR; >> > } >> > for (size_t i = 0; i < ARRAY_SIZE(pe->val); i++) { >> > pe->val[i] = le64_to_cpu(pe->val[i]); >> > @@ -806,11 +807,13 @@ static int >vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState >> *s, >> > >> > /* Do translation type check */ >> > if (!vtd_pe_type_check(x86_iommu, pe)) { >> > - return -VTD_FR_PASID_TABLE_INV; >> > + return -VTD_FR_PASID_TABLE_ENTRY_INV; >> > } >> > >> > - if (!vtd_is_level_supported(s, VTD_PE_GET_LEVEL(pe))) { >> > - return -VTD_FR_PASID_TABLE_INV; >> > + pgtt = VTD_PE_GET_TYPE(pe); >> > + if (pgtt == VTD_SM_PASID_ENTRY_SLT && >> > + !vtd_is_level_supported(s, VTD_PE_GET_LEVEL(pe))) { >> > + return -VTD_FR_PASID_TABLE_ENTRY_INV; >> > } >> > >> > return 0; >> > @@ -851,7 +854,7 @@ static int >vtd_get_pe_from_pasid_table(IntelIOMMUState *s, >> > } >> > >> > if (!vtd_pdire_present(&pdire)) { >> > - return -VTD_FR_PASID_TABLE_INV; >> > + return -VTD_FR_PASID_DIR_ENTRY_P; >> > } >> > >> > ret = vtd_get_pe_from_pdire(s, pasid, &pdire, pe); >> > @@ -860,7 +863,7 @@ static int >vtd_get_pe_from_pasid_table(IntelIOMMUState *s, >> > } >> > >> > if (!vtd_pe_present(pe)) { >> > - return -VTD_FR_PASID_TABLE_INV; >> > + return -VTD_FR_PASID_ENTRY_P; >> > } >> > >> > return 0; >> > @@ -913,7 +916,7 @@ static int >vtd_ce_get_pasid_fpd(IntelIOMMUState *s, >> > } >> > >> > if (!vtd_pdire_present(&pdire)) { >> > - return -VTD_FR_PASID_TABLE_INV; >> > + return -VTD_FR_PASID_DIR_ENTRY_P; >> > } >> > >> > /* >> > @@ -1770,7 +1773,11 @@ static const bool vtd_qualified_faults[] = { >> > [VTD_FR_ROOT_ENTRY_RSVD] = false, >> > [VTD_FR_PAGING_ENTRY_RSVD] = true, >> > [VTD_FR_CONTEXT_ENTRY_TT] = true, >> > - [VTD_FR_PASID_TABLE_INV] = false, >> > + [VTD_FR_PASID_DIR_ACCESS_ERR] = false, >> > + [VTD_FR_PASID_DIR_ENTRY_P] = true, >> > + [VTD_FR_PASID_TABLE_ACCESS_ERR] = false, >> > + [VTD_FR_PASID_ENTRY_P] = true, >> > + [VTD_FR_PASID_TABLE_ENTRY_INV] = true, >> > [VTD_FR_SM_INTERRUPT_ADDR] = true, >> > [VTD_FR_MAX] = false, >> > }; >> > -- >> > 2.34.1 >> > >> > >> lgtm