[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251227175728.4358-4-dmaluka@chromium.org>
Date: Sat, 27 Dec 2025 18:57:26 +0100
From: Dmytro Maluka <dmaluka@...omium.org>
To: David Woodhouse <dwmw2@...radead.org>,
Lu Baolu <baolu.lu@...ux.intel.com>,
iommu@...ts.linux.dev
Cc: Joerg Roedel <joro@...tes.org>,
Will Deacon <will@...nel.org>,
Robin Murphy <robin.murphy@....com>,
linux-kernel@...r.kernel.org,
"Vineeth Pillai (Google)" <vineeth@...byteword.org>,
Aashish Sharma <aashish@...hishsharma.net>,
Grzegorz Jaszczyk <jaszczyk@...omium.org>,
Chuanxiao Dong <chuanxiao.dong@...el.com>,
Kevin Tian <kevin.tian@...el.com>,
Dmytro Maluka <dmaluka@...omium.org>
Subject: [PATCH v2 3/5] iommu/vt-d: Ensure memory ordering in context entry updates
We do take care to not set the present bit in a context table entry via
context_set_present() earlier than setting up all other bits in it.
However, we don't do anything to actually ensure this order, i.e. to
prevent the compiler from reordering it. And since context entries may
be updated at runtime when translation is already enabled, this is a
potential source of bugs or security issues.
To easily fix this, convert the context_set_*() and context_clear_*()
helpers to use entry_set_bits() which uses READ_ONCE/WRITE_ONCE, to
ensure that the ordering between updates of individual bits in context
entries matches the order of calling those helpers, just like we already
do that for PASID table entries.
Link: https://lore.kernel.org/all/aTG7gc7I5wExai3S@google.com/
Signed-off-by: Dmytro Maluka <dmaluka@...omium.org>
---
drivers/iommu/intel/iommu.h | 30 ++++++++++++++----------------
drivers/iommu/intel/pasid.c | 3 ++-
2 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 2fab7ff4b932..5bc69ffc7c8e 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -869,7 +869,7 @@ static inline bool dma_pte_superpage(struct dma_pte *pte)
static inline bool context_present(struct context_entry *context)
{
- return (context->lo & 1);
+ return READ_ONCE(context->lo) & 1;
}
#define LEVEL_STRIDE (9)
@@ -909,43 +909,41 @@ static inline void entry_set_bits(u64 *ptr, u64 mask, u64 bits)
static inline void context_set_present(struct context_entry *context)
{
- context->lo |= 1;
+ entry_set_bits(&context->lo, 1ULL << 0, 1ULL);
}
static inline void context_set_fault_enable(struct context_entry *context)
{
- context->lo &= (((u64)-1) << 2) | 1;
+ entry_set_bits(&context->lo, 1ULL << 1, 0ULL);
}
static inline void context_set_translation_type(struct context_entry *context,
unsigned long value)
{
- context->lo &= (((u64)-1) << 4) | 3;
- context->lo |= (value & 3) << 2;
+ entry_set_bits(&context->lo, GENMASK_ULL(3, 2), value << 2);
}
static inline void context_set_address_root(struct context_entry *context,
unsigned long value)
{
- context->lo &= ~VTD_PAGE_MASK;
- context->lo |= value & VTD_PAGE_MASK;
+ entry_set_bits(&context->lo, VTD_PAGE_MASK, value);
}
static inline void context_set_address_width(struct context_entry *context,
unsigned long value)
{
- context->hi |= value & 7;
+ entry_set_bits(&context->hi, GENMASK_ULL(2, 0), value);
}
static inline void context_set_domain_id(struct context_entry *context,
unsigned long value)
{
- context->hi |= (value & ((1 << 16) - 1)) << 8;
+ entry_set_bits(&context->hi, GENMASK_ULL(23, 8), value << 8);
}
static inline void context_set_pasid(struct context_entry *context)
{
- context->lo |= CONTEXT_PASIDE;
+ entry_set_bits(&context->lo, CONTEXT_PASIDE, CONTEXT_PASIDE);
}
static inline int context_domain_id(struct context_entry *c)
@@ -955,8 +953,8 @@ static inline int context_domain_id(struct context_entry *c)
static inline void context_clear_entry(struct context_entry *context)
{
- context->lo = 0;
- context->hi = 0;
+ WRITE_ONCE(context->lo, 0);
+ WRITE_ONCE(context->hi, 0);
}
#ifdef CONFIG_INTEL_IOMMU
@@ -989,7 +987,7 @@ clear_context_copied(struct intel_iommu *iommu, u8 bus, u8 devfn)
static inline void
context_set_sm_rid2pasid(struct context_entry *context, unsigned long pasid)
{
- context->hi |= pasid & ((1 << 20) - 1);
+ entry_set_bits(&context->hi, GENMASK_ULL(19, 0), pasid);
}
/*
@@ -998,7 +996,7 @@ context_set_sm_rid2pasid(struct context_entry *context, unsigned long pasid)
*/
static inline void context_set_sm_dte(struct context_entry *context)
{
- context->lo |= BIT_ULL(2);
+ entry_set_bits(&context->lo, BIT_ULL(2), BIT_ULL(2));
}
/*
@@ -1007,7 +1005,7 @@ static inline void context_set_sm_dte(struct context_entry *context)
*/
static inline void context_set_sm_pre(struct context_entry *context)
{
- context->lo |= BIT_ULL(4);
+ entry_set_bits(&context->lo, BIT_ULL(4), BIT_ULL(4));
}
/*
@@ -1016,7 +1014,7 @@ static inline void context_set_sm_pre(struct context_entry *context)
*/
static inline void context_clear_sm_pre(struct context_entry *context)
{
- context->lo &= ~BIT_ULL(4);
+ entry_set_bits(&context->lo, BIT_ULL(4), 0);
}
/* Returns a number of VTD pages, but aligned to MM page size */
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 3e2255057079..10bc1908d892 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -983,7 +983,8 @@ static int context_entry_set_pasid_table(struct context_entry *context,
context_clear_entry(context);
pds = context_get_sm_pds(table);
- context->lo = (u64)virt_to_phys(table->table) | context_pdts(pds);
+ WRITE_ONCE(context->lo,
+ (u64)virt_to_phys(table->table) | context_pdts(pds));
context_set_sm_rid2pasid(context, IOMMU_NO_PASID);
if (info->ats_supported)
--
2.47.3
Powered by blists - more mailing lists