On platforms that do not support IOMMU Extended capability bit 0 Page-walk Coherency, CPU caches are not snooped when IOMMU is accessing any translation structures. IOMMU access goes only directly to memory. Intel IOMMU code was missing a flush for the PASID table directory that resulted in the unrecoverable fault as shown below.
This patch adds clflush calls whenever allocating and updating a PASID table directory to ensure cache coherency.
On the reverse direction, there's no need to clflush the PASID directory pointer when we deactivate a context entry in that IOMMU hardware will not see the old PASID directory pointer after we clear the context entry. PASID directory entries are also never freed once allocated.
[ 0.555386] DMAR: DRHD: handling fault status reg 3 [ 0.555805] DMAR: [DMA Read NO_PASID] Request device [00:0d.2] fault addr 0x1026a4000 [fault reason 0x51] SM: Present bit in Directory Entry is clear [ 0.556348] DMAR: Dump dmar1 table entries for IOVA 0x1026a4000 [ 0.556348] DMAR: scalable mode root entry: hi 0x0000000102448001, low 0x0000000101b3e001 [ 0.556348] DMAR: context entry: hi 0x0000000000000000, low 0x0000000101b4d401 [ 0.556348] DMAR: pasid dir entry: 0x0000000101b4e001 [ 0.556348] DMAR: pasid table entry[0]: 0x0000000000000109 [ 0.556348] DMAR: pasid table entry[1]: 0x0000000000000001 [ 0.556348] DMAR: pasid table entry[2]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[3]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[4]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[5]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[6]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[7]: 0x0000000000000000 [ 0.556348] DMAR: PTE not present at level 4
Cc: stable@vger.kernel.org Fixes: 0bbeb01a4faf ("iommu/vt-d: Manage scalalble mode PASID tables") Reported-by: Sukumar Ghorai sukumar.ghorai@intel.com Signed-off-by: Ashok Raj ashok.raj@intel.com Signed-off-by: Jacob Pan jacob.jun.pan@linux.intel.com --- v3: Add clflush after PASID directory allocation to prevent malicious device attack with unauthorized PASIDs. Also flush all the PASID entries after directory updates. (Baolu) v2: Add clflush to PASID directory update case (Baolu, Kevin review) --- drivers/iommu/intel/iommu.c | 2 ++ drivers/iommu/intel/pasid.c | 7 +++++++ 2 files changed, 9 insertions(+)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 59df7e42fd53..161342e7149d 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -1976,6 +1976,8 @@ static int domain_context_mapping_one(struct dmar_domain *domain, pds = context_get_sm_pds(table); context->lo = (u64)virt_to_phys(table->table) | context_pdts(pds); + if (!ecap_coherent(iommu->ecap)) + clflush_cache_range(table->table, sizeof(u64));
/* Setup the RID_PASID field: */ context_set_sm_rid2pasid(context, PASID_RID2PASID); diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c index fb3c7020028d..979f796175b1 100644 --- a/drivers/iommu/intel/pasid.c +++ b/drivers/iommu/intel/pasid.c @@ -128,6 +128,9 @@ int intel_pasid_alloc_table(struct device *dev) pasid_table->max_pasid = 1 << (order + PAGE_SHIFT + 3); info->pasid_table = pasid_table;
+ if (!ecap_coherent(info->iommu->ecap)) + clflush_cache_range(pasid_table->table, size); + return 0; }
@@ -215,6 +218,10 @@ static struct pasid_entry *intel_pasid_get_entry(struct device *dev, u32 pasid) free_pgtable_page(entries); goto retry; } + if (!ecap_coherent(info->iommu->ecap)) { + clflush_cache_range(entries, VTD_PAGE_SIZE); + clflush_cache_range(&dir[dir_index].val, sizeof(*dir)); + } }
return &entries[index];
From: Jacob Pan jacob.jun.pan@linux.intel.com Sent: Wednesday, April 5, 2023 11:45 PM
On platforms that do not support IOMMU Extended capability bit 0 Page-walk Coherency, CPU caches are not snooped when IOMMU is accessing any translation structures. IOMMU access goes only directly to memory. Intel IOMMU code was missing a flush for the PASID table directory that resulted in the unrecoverable fault as shown below.
This patch adds clflush calls whenever allocating and updating a PASID table directory to ensure cache coherency.
On the reverse direction, there's no need to clflush the PASID directory pointer when we deactivate a context entry in that IOMMU hardware will not see the old PASID directory pointer after we clear the context entry. PASID directory entries are also never freed once allocated.
[ 0.555386] DMAR: DRHD: handling fault status reg 3 [ 0.555805] DMAR: [DMA Read NO_PASID] Request device [00:0d.2] fault addr 0x1026a4000 [fault reason 0x51] SM: Present bit in Directory Entry is clear [ 0.556348] DMAR: Dump dmar1 table entries for IOVA 0x1026a4000 [ 0.556348] DMAR: scalable mode root entry: hi 0x0000000102448001, low 0x0000000101b3e001 [ 0.556348] DMAR: context entry: hi 0x0000000000000000, low 0x0000000101b4d401 [ 0.556348] DMAR: pasid dir entry: 0x0000000101b4e001 [ 0.556348] DMAR: pasid table entry[0]: 0x0000000000000109 [ 0.556348] DMAR: pasid table entry[1]: 0x0000000000000001 [ 0.556348] DMAR: pasid table entry[2]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[3]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[4]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[5]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[6]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[7]: 0x0000000000000000 [ 0.556348] DMAR: PTE not present at level 4
Cc: stable@vger.kernel.org Fixes: 0bbeb01a4faf ("iommu/vt-d: Manage scalalble mode PASID tables") Reported-by: Sukumar Ghorai sukumar.ghorai@intel.com Signed-off-by: Ashok Raj ashok.raj@intel.com Signed-off-by: Jacob Pan jacob.jun.pan@linux.intel.com
v3: Add clflush after PASID directory allocation to prevent malicious device attack with unauthorized PASIDs. Also flush all the PASID entries after directory updates. (Baolu) v2: Add clflush to PASID directory update case (Baolu, Kevin review)
drivers/iommu/intel/iommu.c | 2 ++ drivers/iommu/intel/pasid.c | 7 +++++++ 2 files changed, 9 insertions(+)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 59df7e42fd53..161342e7149d 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -1976,6 +1976,8 @@ static int domain_context_mapping_one(struct dmar_domain *domain, pds = context_get_sm_pds(table); context->lo = (u64)virt_to_phys(table->table) | context_pdts(pds);
if (!ecap_coherent(iommu->ecap))
clflush_cache_range(table->table, sizeof(u64));
v2 of this patch was already merged w/o this change.
can you elaborate the purpose of v3? Here no flush is required as long as it's done in other two places below.
/* Setup the RID_PASID field: */ context_set_sm_rid2pasid(context, PASID_RID2PASID);
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c index fb3c7020028d..979f796175b1 100644 --- a/drivers/iommu/intel/pasid.c +++ b/drivers/iommu/intel/pasid.c @@ -128,6 +128,9 @@ int intel_pasid_alloc_table(struct device *dev) pasid_table->max_pasid = 1 << (order + PAGE_SHIFT + 3); info->pasid_table = pasid_table;
- if (!ecap_coherent(info->iommu->ecap))
clflush_cache_range(pasid_table->table, size);
- return 0;
}
@@ -215,6 +218,10 @@ static struct pasid_entry *intel_pasid_get_entry(struct device *dev, u32 pasid) free_pgtable_page(entries); goto retry; }
if (!ecap_coherent(info->iommu->ecap)) {
clflush_cache_range(entries, VTD_PAGE_SIZE);
clflush_cache_range(&dir[dir_index].val, sizeof(*dir));
}
}
return &entries[index];
-- 2.25.1
On 4/20/23 4:16 PM, Tian, Kevin wrote:
From: Jacob Pan jacob.jun.pan@linux.intel.com Sent: Wednesday, April 5, 2023 11:45 PM
On platforms that do not support IOMMU Extended capability bit 0 Page-walk Coherency, CPU caches are not snooped when IOMMU is accessing any translation structures. IOMMU access goes only directly to memory. Intel IOMMU code was missing a flush for the PASID table directory that resulted in the unrecoverable fault as shown below.
This patch adds clflush calls whenever allocating and updating a PASID table directory to ensure cache coherency.
On the reverse direction, there's no need to clflush the PASID directory pointer when we deactivate a context entry in that IOMMU hardware will not see the old PASID directory pointer after we clear the context entry. PASID directory entries are also never freed once allocated.
[ 0.555386] DMAR: DRHD: handling fault status reg 3 [ 0.555805] DMAR: [DMA Read NO_PASID] Request device [00:0d.2] fault addr 0x1026a4000 [fault reason 0x51] SM: Present bit in Directory Entry is clear [ 0.556348] DMAR: Dump dmar1 table entries for IOVA 0x1026a4000 [ 0.556348] DMAR: scalable mode root entry: hi 0x0000000102448001, low 0x0000000101b3e001 [ 0.556348] DMAR: context entry: hi 0x0000000000000000, low 0x0000000101b4d401 [ 0.556348] DMAR: pasid dir entry: 0x0000000101b4e001 [ 0.556348] DMAR: pasid table entry[0]: 0x0000000000000109 [ 0.556348] DMAR: pasid table entry[1]: 0x0000000000000001 [ 0.556348] DMAR: pasid table entry[2]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[3]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[4]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[5]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[6]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[7]: 0x0000000000000000 [ 0.556348] DMAR: PTE not present at level 4
Cc: stable@vger.kernel.org Fixes: 0bbeb01a4faf ("iommu/vt-d: Manage scalalble mode PASID tables") Reported-by: Sukumar Ghorai sukumar.ghorai@intel.com Signed-off-by: Ashok Raj ashok.raj@intel.com Signed-off-by: Jacob Pan jacob.jun.pan@linux.intel.com
v3: Add clflush after PASID directory allocation to prevent malicious device attack with unauthorized PASIDs. Also flush all the PASID entries after directory updates. (Baolu) v2: Add clflush to PASID directory update case (Baolu, Kevin review)
drivers/iommu/intel/iommu.c | 2 ++ drivers/iommu/intel/pasid.c | 7 +++++++ 2 files changed, 9 insertions(+)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 59df7e42fd53..161342e7149d 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -1976,6 +1976,8 @@ static int domain_context_mapping_one(struct dmar_domain *domain, pds = context_get_sm_pds(table); context->lo = (u64)virt_to_phys(table->table) | context_pdts(pds);
if (!ecap_coherent(iommu->ecap))
clflush_cache_range(table->table, sizeof(u64));
v2 of this patch was already merged w/o this change.
The merged patch is v4.
https://lore.kernel.org/all/20230209212843.1788125-1-jacob.jun.pan@linux.int...
can you elaborate the purpose of v3? Here no flush is required as long as it's done in other two places below.
No above code in the merged patch.
Best regards, baolu
/* Setup the RID_PASID field: */ context_set_sm_rid2pasid(context, PASID_RID2PASID);
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c index fb3c7020028d..979f796175b1 100644 --- a/drivers/iommu/intel/pasid.c +++ b/drivers/iommu/intel/pasid.c @@ -128,6 +128,9 @@ int intel_pasid_alloc_table(struct device *dev) pasid_table->max_pasid = 1 << (order + PAGE_SHIFT + 3); info->pasid_table = pasid_table;
- if (!ecap_coherent(info->iommu->ecap))
clflush_cache_range(pasid_table->table, size);
- return 0; }
@@ -215,6 +218,10 @@ static struct pasid_entry *intel_pasid_get_entry(struct device *dev, u32 pasid) free_pgtable_page(entries); goto retry; }
if (!ecap_coherent(info->iommu->ecap)) {
clflush_cache_range(entries, VTD_PAGE_SIZE);
clflush_cache_range(&dir[dir_index].val, sizeof(*dir));
}
}
return &entries[index];
-- 2.25.1
From: Baolu Lu baolu.lu@linux.intel.com Sent: Friday, April 21, 2023 10:34 AM
On 4/20/23 4:16 PM, Tian, Kevin wrote:
From: Jacob Pan jacob.jun.pan@linux.intel.com Sent: Wednesday, April 5, 2023 11:45 PM
On platforms that do not support IOMMU Extended capability bit 0 Page-walk Coherency, CPU caches are not snooped when IOMMU is accessing any translation structures. IOMMU access goes only directly to memory. Intel IOMMU code was missing a flush for the PASID table directory that resulted in the unrecoverable fault as shown below.
This patch adds clflush calls whenever allocating and updating a PASID table directory to ensure cache coherency.
On the reverse direction, there's no need to clflush the PASID directory pointer when we deactivate a context entry in that IOMMU hardware will not see the old PASID directory pointer after we clear the context entry. PASID directory entries are also never freed once allocated.
[ 0.555386] DMAR: DRHD: handling fault status reg 3 [ 0.555805] DMAR: [DMA Read NO_PASID] Request device [00:0d.2] fault addr 0x1026a4000 [fault reason 0x51] SM: Present bit in Directory Entry is clear [ 0.556348] DMAR: Dump dmar1 table entries for IOVA 0x1026a4000 [ 0.556348] DMAR: scalable mode root entry: hi 0x0000000102448001,
low
0x0000000101b3e001 [ 0.556348] DMAR: context entry: hi 0x0000000000000000, low 0x0000000101b4d401 [ 0.556348] DMAR: pasid dir entry: 0x0000000101b4e001 [ 0.556348] DMAR: pasid table entry[0]: 0x0000000000000109 [ 0.556348] DMAR: pasid table entry[1]: 0x0000000000000001 [ 0.556348] DMAR: pasid table entry[2]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[3]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[4]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[5]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[6]: 0x0000000000000000 [ 0.556348] DMAR: pasid table entry[7]: 0x0000000000000000 [ 0.556348] DMAR: PTE not present at level 4
Cc: stable@vger.kernel.org Fixes: 0bbeb01a4faf ("iommu/vt-d: Manage scalalble mode PASID tables") Reported-by: Sukumar Ghorai sukumar.ghorai@intel.com Signed-off-by: Ashok Raj ashok.raj@intel.com Signed-off-by: Jacob Pan jacob.jun.pan@linux.intel.com
v3: Add clflush after PASID directory allocation to prevent malicious device attack with unauthorized PASIDs. Also flush all the PASID entries after directory updates. (Baolu) v2: Add clflush to PASID directory update case (Baolu, Kevin review)
drivers/iommu/intel/iommu.c | 2 ++ drivers/iommu/intel/pasid.c | 7 +++++++ 2 files changed, 9 insertions(+)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index 59df7e42fd53..161342e7149d 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -1976,6 +1976,8 @@ static int domain_context_mapping_one(struct dmar_domain *domain, pds = context_get_sm_pds(table); context->lo = (u64)virt_to_phys(table->table) | context_pdts(pds);
if (!ecap_coherent(iommu->ecap))
clflush_cache_range(table->table, sizeof(u64));
v2 of this patch was already merged w/o this change.
The merged patch is v4.
https://lore.kernel.org/all/20230209212843.1788125-1- jacob.jun.pan@linux.intel.com/
can you elaborate the purpose of v3? Here no flush is required as long as it's done in other two places below.
No above code in the merged patch.
so this one might be just sent inadvertently by Jacob.
linux-stable-mirror@lists.linaro.org