To support the new AMD Secure Nested Paging (SNP) feature, AMD IOMMU driver
needs to be modified to comply with new restrictions enforced by the SNP
feature.
The SNP feature detection needs to happen early in the IOMMU driver
initialization, and the feature must be supported across all IOMMUs.
To simplify the detection process, this series introduces global variables
for tracking IOMMU Extended Feature Registers (EFR and EFR2), which store
common feature bits across all IOMMUs. These global variables are available
as soon as the IVRS table is parsed, which happens at the beginning of
the driver initialization. Therefore, they can be used for early detection
of SNP feature. (See patch 2 - 5)
Once the feature is detected, IOMMU driver needs to be informed when the
feature system-wide. Therefor, the function amd_iommu_snp_enable() is
introduced in patch 6, and will be called by SEV-SNP driver.
When IOMMU driver initializing the device table entries (DTEs), care must
be taken when setting up the DTE[TV] bit on SNP-enabled system.
(See patch 7)
Lastly, an SNP-enabled system requires IOMMU v1 page table to be configured
with non-zero DTE[Mode] for DMA-capable devices. This affects a number of
use cases such as IOMMU pass-through mode and AMD IOMMUv2 APIs for binding/
unbinding pasid cannot be supported with SNP. These are handled in patch 8
and 9.
Testing:
- Tested booting and verify dmesg.
- Tested booting with iommu=pt
- Tested changing the iommu domain to identity at runtime
- Tested loading amd_iommu_v2 driver
- Tested booting SEV/SNP-enabled guest
- Tested when CONFIG_AMD_MEM_ENCRYPT is not set
Chanages from v3:
(https://www.spinics.net/lists/kernel/msg4409539.html)
- Patch 1, 2, and 5 are new.
- Patch 3: Modify to use global common EFR/EFR2 vaiable
when tracking supported features.
Best Regards,
Suravee
Brijesh Singh (1):
iommu/amd: Introduce function to check and enable SNP
Suravee Suthikulpanit (8):
iommu/amd: Change macro for IOMMU control register bit shift to
decimal value
iommu/amd: Introduce Support for Extended Feature 2 Register
iommu/amd: Introduce global variable for storing common EFR and EFR2
iommu/amd: Process all IVHDs before enabling IOMMU features
iommu/amd: Globally detect SNP support
iommu/amd: Set translation valid bit only when IO page tables are in
use
iommu/amd: Do not support IOMMU_DOMAIN_IDENTITY after SNP is enabled
iommu/amd: Do not support IOMMUv2 APIs when SNP is enabled
drivers/iommu/amd/amd_iommu.h | 5 +
drivers/iommu/amd/amd_iommu_types.h | 46 +++++----
drivers/iommu/amd/init.c | 153 +++++++++++++++++++++++-----
drivers/iommu/amd/iommu.c | 24 ++++-
include/linux/amd-iommu.h | 4 +
5 files changed, 183 insertions(+), 49 deletions(-)
--
2.32.0
On AMD system with SNP enabled, IOMMU hardware checks the host translation
valid (TV) and guest translation valid (GV) bits in the device table entry
(DTE) before accessing the corresponded page tables.
However, current IOMMU driver sets the TV bit for all devices regardless
of whether the host page table is in use. This results in
ILLEGAL_DEV_TABLE_ENTRY event for devices, which do not the host page
table root pointer set up.
Thefore, when SNP is enabled, only set TV bit when DMA remapping is not
used, which is when domain ID in the AMD IOMMU device table entry (DTE)
is zero.
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/iommu/amd/init.c | 3 ++-
drivers/iommu/amd/iommu.c | 16 ++++++++++++++--
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 1dd1c2b25898..959595c49656 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -2566,7 +2566,8 @@ static void init_device_table_dma(struct amd_iommu_pci_seg *pci_seg)
for (devid = 0; devid <= pci_seg->last_bdf; ++devid) {
__set_dev_entry_bit(dev_table, devid, DEV_ENTRY_VALID);
- __set_dev_entry_bit(dev_table, devid, DEV_ENTRY_TRANSLATION);
+ if (!amd_iommu_snp_en)
+ __set_dev_entry_bit(dev_table, devid, DEV_ENTRY_TRANSLATION);
}
}
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index a56a9ad3273e..aedeff8af929 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1552,7 +1552,15 @@ static void set_dte_entry(struct amd_iommu *iommu, u16 devid,
pte_root |= (domain->iop.mode & DEV_ENTRY_MODE_MASK)
<< DEV_ENTRY_MODE_SHIFT;
- pte_root |= DTE_FLAG_IR | DTE_FLAG_IW | DTE_FLAG_V | DTE_FLAG_TV;
+
+ pte_root |= DTE_FLAG_IR | DTE_FLAG_IW | DTE_FLAG_V;
+
+ /*
+ * When SNP is enabled, Only set TV bit when IOMMU
+ * page translation is in use.
+ */
+ if (!amd_iommu_snp_en || (domain->id != 0))
+ pte_root |= DTE_FLAG_TV;
flags = dev_table[devid].data[1];
@@ -1612,7 +1620,11 @@ static void clear_dte_entry(struct amd_iommu *iommu, u16 devid)
struct dev_table_entry *dev_table = get_dev_table(iommu);
/* remove entry from the device table seen by the hardware */
- dev_table[devid].data[0] = DTE_FLAG_V | DTE_FLAG_TV;
+ dev_table[devid].data[0] = DTE_FLAG_V;
+
+ if (!amd_iommu_snp_en)
+ dev_table[devid].data[0] |= DTE_FLAG_TV;
+
dev_table[devid].data[1] &= DTE_FLAG_MASK;
amd_iommu_apply_erratum_63(iommu, devid);
--
2.32.0
AMD IOMMU spec introduces additional extended feature register
in the IVRS IVHD offset 80h (for IVHD type 11h and 40h) and MMIO
offset 1A0h.
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/iommu/amd/amd_iommu_types.h | 4 ++++
drivers/iommu/amd/init.c | 24 ++++++++++++++++--------
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 7c830e3c7a91..3c1205ba636a 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -67,6 +67,7 @@
#define MMIO_INTCAPXT_EVT_OFFSET 0x0170
#define MMIO_INTCAPXT_PPR_OFFSET 0x0178
#define MMIO_INTCAPXT_GALOG_OFFSET 0x0180
+#define MMIO_EXT_FEATURES2 0x01A0
#define MMIO_CMD_HEAD_OFFSET 0x2000
#define MMIO_CMD_TAIL_OFFSET 0x2008
#define MMIO_EVT_HEAD_OFFSET 0x2010
@@ -645,6 +646,9 @@ struct amd_iommu {
/* Extended features */
u64 features;
+ /* Extended features 2 */
+ u64 features2;
+
/* IOMMUv2 */
bool is_iommu_v2;
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 3c82d9c5f1c0..92ab02f6c5dc 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -114,7 +114,7 @@ struct ivhd_header {
/* Following only valid on IVHD type 11h and 40h */
u64 efr_reg; /* Exact copy of MMIO_EXT_FEATURES */
- u64 res;
+ u64 efr_reg2;
} __attribute__((packed));
/*
@@ -283,8 +283,10 @@ static bool check_feature_on_all_iommus(u64 mask)
static void __init early_iommu_features_init(struct amd_iommu *iommu,
struct ivhd_header *h)
{
- if (amd_iommu_ivinfo & IOMMU_IVINFO_EFRSUP)
+ if (amd_iommu_ivinfo & IOMMU_IVINFO_EFRSUP) {
iommu->features = h->efr_reg;
+ iommu->features2 = h->efr_reg2;
+ }
if (amd_iommu_ivinfo & IOMMU_IVINFO_DMA_REMAP)
amdr_ivrs_remap_support = true;
}
@@ -1912,7 +1914,7 @@ static ssize_t amd_iommu_show_features(struct device *dev,
char *buf)
{
struct amd_iommu *iommu = dev_to_amd_iommu(dev);
- return sprintf(buf, "%llx\n", iommu->features);
+ return sprintf(buf, "%llx:%llx\n", iommu->features2, iommu->features);
}
static DEVICE_ATTR(features, S_IRUGO, amd_iommu_show_features, NULL);
@@ -1939,16 +1941,18 @@ static const struct attribute_group *amd_iommu_groups[] = {
*/
static void __init late_iommu_features_init(struct amd_iommu *iommu)
{
- u64 features;
+ u64 features, features2;
if (!(iommu->cap & (1 << IOMMU_CAP_EFR)))
return;
/* read extended feature bits */
features = readq(iommu->mmio_base + MMIO_EXT_FEATURES);
+ features2 = readq(iommu->mmio_base + MMIO_EXT_FEATURES2);
if (!iommu->features) {
iommu->features = features;
+ iommu->features2 = features2;
return;
}
@@ -1956,9 +1960,13 @@ static void __init late_iommu_features_init(struct amd_iommu *iommu)
* Sanity check and warn if EFR values from
* IVHD and MMIO conflict.
*/
- if (features != iommu->features)
- pr_warn(FW_WARN "EFR mismatch. Use IVHD EFR (%#llx : %#llx).\n",
- features, iommu->features);
+ if (features != iommu->features ||
+ features2 != iommu->features2) {
+ pr_warn(FW_WARN
+ "EFR mismatch. Use IVHD EFR (%#llx : %#llx), EFR2 (%#llx : %#llx).\n",
+ features, iommu->features,
+ features2, iommu->features2);
+ }
}
static int __init iommu_init_pci(struct amd_iommu *iommu)
@@ -2080,7 +2088,7 @@ static void print_iommu_info(void)
pci_info(pdev, "Found IOMMU cap 0x%x\n", iommu->cap_ptr);
if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
- pr_info("Extended features (%#llx):", iommu->features);
+ pr_info("Extended features (%#llx, %#llx):", iommu->features, iommu->features2);
for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
if (iommu_feature(iommu, (1ULL << i)))
--
2.32.0
There is no functional change.
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/iommu/amd/amd_iommu_types.h | 42 ++++++++++++++---------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 40f52d02c5b9..7c830e3c7a91 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -143,27 +143,27 @@
#define EVENT_FLAG_I 0x008
/* feature control bits */
-#define CONTROL_IOMMU_EN 0x00ULL
-#define CONTROL_HT_TUN_EN 0x01ULL
-#define CONTROL_EVT_LOG_EN 0x02ULL
-#define CONTROL_EVT_INT_EN 0x03ULL
-#define CONTROL_COMWAIT_EN 0x04ULL
-#define CONTROL_INV_TIMEOUT 0x05ULL
-#define CONTROL_PASSPW_EN 0x08ULL
-#define CONTROL_RESPASSPW_EN 0x09ULL
-#define CONTROL_COHERENT_EN 0x0aULL
-#define CONTROL_ISOC_EN 0x0bULL
-#define CONTROL_CMDBUF_EN 0x0cULL
-#define CONTROL_PPRLOG_EN 0x0dULL
-#define CONTROL_PPRINT_EN 0x0eULL
-#define CONTROL_PPR_EN 0x0fULL
-#define CONTROL_GT_EN 0x10ULL
-#define CONTROL_GA_EN 0x11ULL
-#define CONTROL_GAM_EN 0x19ULL
-#define CONTROL_GALOG_EN 0x1CULL
-#define CONTROL_GAINT_EN 0x1DULL
-#define CONTROL_XT_EN 0x32ULL
-#define CONTROL_INTCAPXT_EN 0x33ULL
+#define CONTROL_IOMMU_EN 0
+#define CONTROL_HT_TUN_EN 1
+#define CONTROL_EVT_LOG_EN 2
+#define CONTROL_EVT_INT_EN 3
+#define CONTROL_COMWAIT_EN 4
+#define CONTROL_INV_TIMEOUT 5
+#define CONTROL_PASSPW_EN 8
+#define CONTROL_RESPASSPW_EN 9
+#define CONTROL_COHERENT_EN 10
+#define CONTROL_ISOC_EN 11
+#define CONTROL_CMDBUF_EN 12
+#define CONTROL_PPRLOG_EN 13
+#define CONTROL_PPRINT_EN 14
+#define CONTROL_PPR_EN 15
+#define CONTROL_GT_EN 16
+#define CONTROL_GA_EN 17
+#define CONTROL_GAM_EN 25
+#define CONTROL_GALOG_EN 28
+#define CONTROL_GAINT_EN 29
+#define CONTROL_XT_EN 50
+#define CONTROL_INTCAPXT_EN 51
#define CTRL_INV_TO_MASK (7 << CONTROL_INV_TIMEOUT)
#define CTRL_INV_TO_NONE 0
--
2.32.0
Modify existing SNP feature check to use the helper function
check_feature_on_all_iommus() to ensure consistency among all IOMMUs.
Also report IOMMU SNP support information for each IOMMU.
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/iommu/amd/init.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index a0f4b5bbd98c..2f338c8c3d10 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -389,7 +389,7 @@ static void iommu_set_cwwb_range(struct amd_iommu *iommu)
u64 start = iommu_virt_to_phys((void *)iommu->cmd_sem);
u64 entry = start & PM_ADDR_MASK;
- if (!iommu_feature(iommu, FEATURE_SNP))
+ if (!check_feature_on_all_iommus(FEATURE_SNP))
return;
/* Note:
@@ -804,7 +804,7 @@ static void *__init iommu_alloc_4k_pages(struct amd_iommu *iommu,
void *buf = (void *)__get_free_pages(gfp, order);
if (buf &&
- iommu_feature(iommu, FEATURE_SNP) &&
+ check_feature_on_all_iommus(FEATURE_SNP) &&
set_memory_4k((unsigned long)buf, (1 << order))) {
free_pages((unsigned long)buf, order);
buf = NULL;
@@ -2140,6 +2140,9 @@ static void print_iommu_info(void)
if (iommu->features & FEATURE_GAM_VAPIC)
pr_cont(" GA_vAPIC");
+ if (iommu->features & FEATURE_SNP)
+ pr_cont(" SNP");
+
pr_cont("\n");
}
}
--
2.32.0
On Wed, Jul 13, 2022 at 05:56:42PM -0500, Suravee Suthikulpanit wrote:
> Brijesh Singh (1):
> iommu/amd: Introduce function to check and enable SNP
>
> Suravee Suthikulpanit (8):
> iommu/amd: Change macro for IOMMU control register bit shift to
> decimal value
> iommu/amd: Introduce Support for Extended Feature 2 Register
> iommu/amd: Introduce global variable for storing common EFR and EFR2
> iommu/amd: Process all IVHDs before enabling IOMMU features
> iommu/amd: Globally detect SNP support
> iommu/amd: Set translation valid bit only when IO page tables are in
> use
> iommu/amd: Do not support IOMMU_DOMAIN_IDENTITY after SNP is enabled
> iommu/amd: Do not support IOMMUv2 APIs when SNP is enabled
Applied, thanks Suravee.