|
|
ad078f9 |
From 5b2ccb60ff22fbff44dd66214c2956a434ee6271 Mon Sep 17 00:00:00 2001
|
|
|
ad078f9 |
From: Roger Pau Monne <roger.pau@citrix.com>
|
|
|
ad078f9 |
Date: Tue, 13 Jun 2023 15:01:05 +0200
|
|
|
ad078f9 |
Subject: [PATCH] iommu/amd-vi: flush IOMMU TLB when flushing the DTE
|
|
|
ad078f9 |
MIME-Version: 1.0
|
|
|
ad078f9 |
Content-Type: text/plain; charset=UTF-8
|
|
|
ad078f9 |
Content-Transfer-Encoding: 8bit
|
|
|
ad078f9 |
|
|
|
ad078f9 |
The caching invalidation guidelines from the AMD-Vi specification (48882—Rev
|
|
|
ad078f9 |
3.07-PUB—Oct 2022) seem to be misleading on some hardware, as devices will
|
|
|
ad078f9 |
malfunction (see stale DMA mappings) if some fields of the DTE are updated but
|
|
|
ad078f9 |
the IOMMU TLB is not flushed. This has been observed in practice on AMD
|
|
|
ad078f9 |
systems. Due to the lack of guidance from the currently published
|
|
|
ad078f9 |
specification this patch aims to increase the flushing done in order to prevent
|
|
|
ad078f9 |
device malfunction.
|
|
|
ad078f9 |
|
|
|
ad078f9 |
In order to fix, issue an INVALIDATE_IOMMU_PAGES command from
|
|
|
ad078f9 |
amd_iommu_flush_device(), flushing all the address space. Note this requires
|
|
|
ad078f9 |
callers to be adjusted in order to pass the DomID on the DTE previous to the
|
|
|
ad078f9 |
modification.
|
|
|
ad078f9 |
|
|
|
ad078f9 |
Some call sites don't provide a valid DomID to amd_iommu_flush_device() in
|
|
|
ad078f9 |
order to avoid the flush. That's because the device had address translations
|
|
|
ad078f9 |
disabled and hence the previous DomID on the DTE is not valid. Note the
|
|
|
ad078f9 |
current logic relies on the entity disabling address translations to also flush
|
|
|
ad078f9 |
the TLB of the in use DomID.
|
|
|
ad078f9 |
|
|
|
ad078f9 |
Device I/O TLB flushing when ATS are enabled is not covered by the current
|
|
|
ad078f9 |
change, as ATS usage is not security supported.
|
|
|
ad078f9 |
|
|
|
ad078f9 |
This is XSA-442 / CVE-2023-34326
|
|
|
ad078f9 |
|
|
|
ad078f9 |
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
|
|
|
ad078f9 |
Reviewed-by: Jan Beulich <jbeulich@suse.com>
|
|
|
ad078f9 |
---
|
|
|
ad078f9 |
xen/drivers/passthrough/amd/iommu.h | 3 ++-
|
|
|
ad078f9 |
xen/drivers/passthrough/amd/iommu_cmd.c | 10 +++++++++-
|
|
|
ad078f9 |
xen/drivers/passthrough/amd/iommu_guest.c | 5 +++--
|
|
|
ad078f9 |
xen/drivers/passthrough/amd/iommu_init.c | 6 +++++-
|
|
|
ad078f9 |
xen/drivers/passthrough/amd/pci_amd_iommu.c | 14 ++++++++++----
|
|
|
ad078f9 |
5 files changed, 29 insertions(+), 9 deletions(-)
|
|
|
ad078f9 |
|
|
|
ad078f9 |
diff --git a/xen/drivers/passthrough/amd/iommu.h b/xen/drivers/passthrough/amd/iommu.h
|
|
|
ad078f9 |
index 5429ada58ef5..a58be28bf96d 100644
|
|
|
ad078f9 |
--- a/xen/drivers/passthrough/amd/iommu.h
|
|
|
ad078f9 |
+++ b/xen/drivers/passthrough/amd/iommu.h
|
|
|
ad078f9 |
@@ -283,7 +283,8 @@ void amd_iommu_flush_pages(struct domain *d, unsigned long dfn,
|
|
|
ad078f9 |
unsigned int order);
|
|
|
ad078f9 |
void amd_iommu_flush_iotlb(u8 devfn, const struct pci_dev *pdev,
|
|
|
ad078f9 |
uint64_t gaddr, unsigned int order);
|
|
|
ad078f9 |
-void amd_iommu_flush_device(struct amd_iommu *iommu, uint16_t bdf);
|
|
|
ad078f9 |
+void amd_iommu_flush_device(struct amd_iommu *iommu, uint16_t bdf,
|
|
|
ad078f9 |
+ domid_t domid);
|
|
|
ad078f9 |
void amd_iommu_flush_intremap(struct amd_iommu *iommu, uint16_t bdf);
|
|
|
ad078f9 |
void amd_iommu_flush_all_caches(struct amd_iommu *iommu);
|
|
|
ad078f9 |
|
|
|
ad078f9 |
diff --git a/xen/drivers/passthrough/amd/iommu_cmd.c b/xen/drivers/passthrough/amd/iommu_cmd.c
|
|
|
ad078f9 |
index 40ddf366bb4d..cb28b36abc38 100644
|
|
|
ad078f9 |
--- a/xen/drivers/passthrough/amd/iommu_cmd.c
|
|
|
ad078f9 |
+++ b/xen/drivers/passthrough/amd/iommu_cmd.c
|
|
|
ad078f9 |
@@ -363,10 +363,18 @@ void amd_iommu_flush_pages(struct domain *d,
|
|
|
ad078f9 |
_amd_iommu_flush_pages(d, __dfn_to_daddr(dfn), order);
|
|
|
ad078f9 |
}
|
|
|
ad078f9 |
|
|
|
ad078f9 |
-void amd_iommu_flush_device(struct amd_iommu *iommu, uint16_t bdf)
|
|
|
ad078f9 |
+void amd_iommu_flush_device(struct amd_iommu *iommu, uint16_t bdf,
|
|
|
ad078f9 |
+ domid_t domid)
|
|
|
ad078f9 |
{
|
|
|
ad078f9 |
invalidate_dev_table_entry(iommu, bdf);
|
|
|
ad078f9 |
flush_command_buffer(iommu, 0);
|
|
|
ad078f9 |
+
|
|
|
ad078f9 |
+ /* Also invalidate IOMMU TLB entries when flushing the DTE. */
|
|
|
ad078f9 |
+ if ( domid != DOMID_INVALID )
|
|
|
ad078f9 |
+ {
|
|
|
ad078f9 |
+ invalidate_iommu_pages(iommu, INV_IOMMU_ALL_PAGES_ADDRESS, domid, 0);
|
|
|
ad078f9 |
+ flush_command_buffer(iommu, 0);
|
|
|
ad078f9 |
+ }
|
|
|
ad078f9 |
}
|
|
|
ad078f9 |
|
|
|
ad078f9 |
void amd_iommu_flush_intremap(struct amd_iommu *iommu, uint16_t bdf)
|
|
|
ad078f9 |
diff --git a/xen/drivers/passthrough/amd/iommu_guest.c b/xen/drivers/passthrough/amd/iommu_guest.c
|
|
|
ad078f9 |
index 80a331f546ed..be86bce6fb03 100644
|
|
|
ad078f9 |
--- a/xen/drivers/passthrough/amd/iommu_guest.c
|
|
|
ad078f9 |
+++ b/xen/drivers/passthrough/amd/iommu_guest.c
|
|
|
ad078f9 |
@@ -385,7 +385,7 @@ static int do_completion_wait(struct domain *d, cmd_entry_t *cmd)
|
|
|
ad078f9 |
|
|
|
ad078f9 |
static int do_invalidate_dte(struct domain *d, cmd_entry_t *cmd)
|
|
|
ad078f9 |
{
|
|
|
ad078f9 |
- uint16_t gbdf, mbdf, req_id, gdom_id, hdom_id;
|
|
|
ad078f9 |
+ uint16_t gbdf, mbdf, req_id, gdom_id, hdom_id, prev_domid;
|
|
|
ad078f9 |
struct amd_iommu_dte *gdte, *mdte, *dte_base;
|
|
|
ad078f9 |
struct amd_iommu *iommu = NULL;
|
|
|
ad078f9 |
struct guest_iommu *g_iommu;
|
|
|
ad078f9 |
@@ -445,13 +445,14 @@ static int do_invalidate_dte(struct domain *d, cmd_entry_t *cmd)
|
|
|
ad078f9 |
req_id = get_dma_requestor_id(iommu->seg, mbdf);
|
|
|
ad078f9 |
dte_base = iommu->dev_table.buffer;
|
|
|
ad078f9 |
mdte = &dte_base[req_id];
|
|
|
ad078f9 |
+ prev_domid = mdte->domain_id;
|
|
|
ad078f9 |
|
|
|
ad078f9 |
spin_lock_irqsave(&iommu->lock, flags);
|
|
|
ad078f9 |
dte_set_gcr3_table(mdte, hdom_id, gcr3_mfn << PAGE_SHIFT, gv, glx);
|
|
|
ad078f9 |
|
|
|
ad078f9 |
spin_unlock_irqrestore(&iommu->lock, flags);
|
|
|
ad078f9 |
|
|
|
ad078f9 |
- amd_iommu_flush_device(iommu, req_id);
|
|
|
ad078f9 |
+ amd_iommu_flush_device(iommu, req_id, prev_domid);
|
|
|
ad078f9 |
|
|
|
ad078f9 |
return 0;
|
|
|
ad078f9 |
}
|
|
|
ad078f9 |
diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
|
|
|
ad078f9 |
index 166570648d26..101a60ce1794 100644
|
|
|
ad078f9 |
--- a/xen/drivers/passthrough/amd/iommu_init.c
|
|
|
ad078f9 |
+++ b/xen/drivers/passthrough/amd/iommu_init.c
|
|
|
ad078f9 |
@@ -1547,7 +1547,11 @@ static int cf_check _invalidate_all_devices(
|
|
|
ad078f9 |
req_id = ivrs_mappings[bdf].dte_requestor_id;
|
|
|
ad078f9 |
if ( iommu )
|
|
|
ad078f9 |
{
|
|
|
ad078f9 |
- amd_iommu_flush_device(iommu, req_id);
|
|
|
ad078f9 |
+ /*
|
|
|
ad078f9 |
+ * IOMMU TLB flush performed separately (see
|
|
|
ad078f9 |
+ * invalidate_all_domain_pages()).
|
|
|
ad078f9 |
+ */
|
|
|
ad078f9 |
+ amd_iommu_flush_device(iommu, req_id, DOMID_INVALID);
|
|
|
ad078f9 |
amd_iommu_flush_intremap(iommu, req_id);
|
|
|
ad078f9 |
}
|
|
|
ad078f9 |
}
|
|
|
ad078f9 |
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
|
|
|
ad078f9 |
index 94e37755064b..8641b84712a0 100644
|
|
|
ad078f9 |
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
|
|
|
ad078f9 |
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
|
|
|
ad078f9 |
@@ -192,10 +192,13 @@ static int __must_check amd_iommu_setup_domain_device(
|
|
|
ad078f9 |
|
|
|
ad078f9 |
spin_unlock_irqrestore(&iommu->lock, flags);
|
|
|
ad078f9 |
|
|
|
ad078f9 |
- amd_iommu_flush_device(iommu, req_id);
|
|
|
ad078f9 |
+ /* DTE didn't have DMA translations enabled, do not flush the TLB. */
|
|
|
ad078f9 |
+ amd_iommu_flush_device(iommu, req_id, DOMID_INVALID);
|
|
|
ad078f9 |
}
|
|
|
ad078f9 |
else if ( dte->pt_root != mfn_x(page_to_mfn(root_pg)) )
|
|
|
ad078f9 |
{
|
|
|
ad078f9 |
+ domid_t prev_domid = dte->domain_id;
|
|
|
ad078f9 |
+
|
|
|
ad078f9 |
/*
|
|
|
ad078f9 |
* Strictly speaking if the device is the only one with this requestor
|
|
|
ad078f9 |
* ID, it could be allowed to be re-assigned regardless of unity map
|
|
|
ad078f9 |
@@ -252,7 +255,7 @@ static int __must_check amd_iommu_setup_domain_device(
|
|
|
ad078f9 |
|
|
|
ad078f9 |
spin_unlock_irqrestore(&iommu->lock, flags);
|
|
|
ad078f9 |
|
|
|
ad078f9 |
- amd_iommu_flush_device(iommu, req_id);
|
|
|
ad078f9 |
+ amd_iommu_flush_device(iommu, req_id, prev_domid);
|
|
|
ad078f9 |
}
|
|
|
ad078f9 |
else
|
|
|
ad078f9 |
spin_unlock_irqrestore(&iommu->lock, flags);
|
|
|
ad078f9 |
@@ -421,6 +424,8 @@ static void amd_iommu_disable_domain_device(const struct domain *domain,
|
|
|
ad078f9 |
spin_lock_irqsave(&iommu->lock, flags);
|
|
|
ad078f9 |
if ( dte->tv || dte->v )
|
|
|
ad078f9 |
{
|
|
|
ad078f9 |
+ domid_t prev_domid = dte->domain_id;
|
|
|
ad078f9 |
+
|
|
|
ad078f9 |
/* See the comment in amd_iommu_setup_device_table(). */
|
|
|
ad078f9 |
dte->int_ctl = IOMMU_DEV_TABLE_INT_CONTROL_ABORTED;
|
|
|
ad078f9 |
smp_wmb();
|
|
|
ad078f9 |
@@ -439,7 +444,7 @@ static void amd_iommu_disable_domain_device(const struct domain *domain,
|
|
|
ad078f9 |
|
|
|
ad078f9 |
spin_unlock_irqrestore(&iommu->lock, flags);
|
|
|
ad078f9 |
|
|
|
ad078f9 |
- amd_iommu_flush_device(iommu, req_id);
|
|
|
ad078f9 |
+ amd_iommu_flush_device(iommu, req_id, prev_domid);
|
|
|
ad078f9 |
|
|
|
ad078f9 |
AMD_IOMMU_DEBUG("Disable: device id = %#x, "
|
|
|
ad078f9 |
"domain = %d, paging mode = %d\n",
|
|
|
ad078f9 |
@@ -610,7 +615,8 @@ static int cf_check amd_iommu_add_device(u8 devfn, struct pci_dev *pdev)
|
|
|
ad078f9 |
|
|
|
ad078f9 |
spin_unlock_irqrestore(&iommu->lock, flags);
|
|
|
ad078f9 |
|
|
|
ad078f9 |
- amd_iommu_flush_device(iommu, bdf);
|
|
|
ad078f9 |
+ /* DTE didn't have DMA translations enabled, do not flush the TLB. */
|
|
|
ad078f9 |
+ amd_iommu_flush_device(iommu, bdf, DOMID_INVALID);
|
|
|
ad078f9 |
}
|
|
|
ad078f9 |
|
|
|
ad078f9 |
if ( amd_iommu_reserve_domain_unity_map(
|
|
|
ad078f9 |
--
|
|
|
ad078f9 |
2.42.0
|
|
|
ad078f9 |
|