2022-07-26 13:49:22

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH 2/2] iommu/amd: Add support for AVIC when SNP is enabled

In order to support AVIC on SNP-enabled system, The IOMMU driver needs to
check EFR2[SNPAVICSup] and enables the support by setting SNPAVICEn bit
in the IOMMU control register (MMIO offset 18h).

For detail, please see section "SEV-SNP Guest Virtual APIC Support" of the
AMD I/O Virtualization Technology (IOMMU) Specification.
(https://www.amd.com/system/files/TechDocs/48882_IOMMU.pdf)

Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/iommu/amd/amd_iommu_types.h | 7 +++++++
drivers/iommu/amd/init.c | 11 ++++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 3c1205ba636a..5b1019dab328 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -103,6 +103,12 @@
#define FEATURE_GLXVAL_SHIFT 14
#define FEATURE_GLXVAL_MASK (0x03ULL << FEATURE_GLXVAL_SHIFT)

+/* Extended Feature 2 Bits */
+#define FEATURE_SNPAVICSUP_SHIFT 5
+#define FEATURE_SNPAVICSUP_MASK (0x07ULL << FEATURE_SNPAVICSUP_SHIFT)
+#define FEATURE_SNPAVICSUP_GAM(x) \
+ ((x & FEATURE_SNPAVICSUP_MASK) >> FEATURE_SNPAVICSUP_SHIFT == 0x1)
+
/* Note:
* The current driver only support 16-bit PASID.
* Currently, hardware only implement upto 16-bit PASID
@@ -165,6 +171,7 @@
#define CONTROL_GAINT_EN 29
#define CONTROL_XT_EN 50
#define CONTROL_INTCAPXT_EN 51
+#define CONTROL_SNPAVIC_EN 61

#define CTRL_INV_TO_MASK (7 << CONTROL_INV_TIMEOUT)
#define CTRL_INV_TO_NONE 0
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 4cd94d716122..6bbaf6b971e8 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -2794,13 +2794,22 @@ static void enable_iommus_vapic(void)
return;
}

- /* Enabling GAM support */
+ if (amd_iommu_snp_en &&
+ !FEATURE_SNPAVICSUP_GAM(amd_iommu_efr2)) {
+ pr_warn("Force to disable Virtual APIC due to SNP\n");
+ amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY_GA;
+ return;
+ }
+
+ /* Enabling GAM and SNPAVIC support */
for_each_iommu(iommu) {
if (iommu_init_ga_log(iommu) ||
iommu_ga_log_enable(iommu))
return;

iommu_feature_enable(iommu, CONTROL_GAM_EN);
+ if (amd_iommu_snp_en)
+ iommu_feature_enable(iommu, CONTROL_SNPAVIC_EN);
}

amd_iommu_irq_ops.capability |= (1 << IRQ_POSTING_CAP);
--
2.34.1


2022-07-26 15:38:42

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: [PATCH 2/2] iommu/amd: Add support for AVIC when SNP is enabled

On Tue, Jul 26, 2022 at 08:43:48AM -0500, Suravee Suthikulpanit wrote:
> In order to support AVIC on SNP-enabled system, The IOMMU driver needs to
> check EFR2[SNPAVICSup] and enables the support by setting SNPAVICEn bit
> in the IOMMU control register (MMIO offset 18h).
>
> For detail, please see section "SEV-SNP Guest Virtual APIC Support" of the
> AMD I/O Virtualization Technology (IOMMU) Specification.
> (https://www.amd.com/system/files/TechDocs/48882_IOMMU.pdf)
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>

Is this a typo in the 3.06 spec for SNPAVIC_EN?

"001b-111b = Reserved."

Or I guess maybe there is a newer revision that isn't available yet
that describes 001b?

Reviewed-by: Jerry Snitselaar <[email protected]>

> ---
> drivers/iommu/amd/amd_iommu_types.h | 7 +++++++
> drivers/iommu/amd/init.c | 11 ++++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 3c1205ba636a..5b1019dab328 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -103,6 +103,12 @@
> #define FEATURE_GLXVAL_SHIFT 14
> #define FEATURE_GLXVAL_MASK (0x03ULL << FEATURE_GLXVAL_SHIFT)
>
> +/* Extended Feature 2 Bits */
> +#define FEATURE_SNPAVICSUP_SHIFT 5
> +#define FEATURE_SNPAVICSUP_MASK (0x07ULL << FEATURE_SNPAVICSUP_SHIFT)
> +#define FEATURE_SNPAVICSUP_GAM(x) \
> + ((x & FEATURE_SNPAVICSUP_MASK) >> FEATURE_SNPAVICSUP_SHIFT == 0x1)
> +
> /* Note:
> * The current driver only support 16-bit PASID.
> * Currently, hardware only implement upto 16-bit PASID
> @@ -165,6 +171,7 @@
> #define CONTROL_GAINT_EN 29
> #define CONTROL_XT_EN 50
> #define CONTROL_INTCAPXT_EN 51
> +#define CONTROL_SNPAVIC_EN 61
>
> #define CTRL_INV_TO_MASK (7 << CONTROL_INV_TIMEOUT)
> #define CTRL_INV_TO_NONE 0
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 4cd94d716122..6bbaf6b971e8 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -2794,13 +2794,22 @@ static void enable_iommus_vapic(void)
> return;
> }
>
> - /* Enabling GAM support */
> + if (amd_iommu_snp_en &&
> + !FEATURE_SNPAVICSUP_GAM(amd_iommu_efr2)) {
> + pr_warn("Force to disable Virtual APIC due to SNP\n");
> + amd_iommu_guest_ir = AMD_IOMMU_GUEST_IR_LEGACY_GA;
> + return;
> + }
> +
> + /* Enabling GAM and SNPAVIC support */
> for_each_iommu(iommu) {
> if (iommu_init_ga_log(iommu) ||
> iommu_ga_log_enable(iommu))
> return;
>
> iommu_feature_enable(iommu, CONTROL_GAM_EN);
> + if (amd_iommu_snp_en)
> + iommu_feature_enable(iommu, CONTROL_SNPAVIC_EN);
> }
>
> amd_iommu_irq_ops.capability |= (1 << IRQ_POSTING_CAP);
> --
> 2.34.1
>

2022-07-27 03:02:56

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH 2/2] iommu/amd: Add support for AVIC when SNP is enabled



On 7/26/22 10:32 PM, Jerry Snitselaar wrote:
> On Tue, Jul 26, 2022 at 08:43:48AM -0500, Suravee Suthikulpanit wrote:
>> In order to support AVIC on SNP-enabled system, The IOMMU driver needs to
>> check EFR2[SNPAVICSup] and enables the support by setting SNPAVICEn bit
>> in the IOMMU control register (MMIO offset 18h).
>>
>> For detail, please see section "SEV-SNP Guest Virtual APIC Support" of the
>> AMD I/O Virtualization Technology (IOMMU) Specification.
>> (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F48882_IOMMU.pdf&amp;data=05%7C01%7Csuravee.suthikulpanit%40amd.com%7C844cc78ad4674484b1f208da6f1c0ee1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637944463702725122%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=U%2Fe%2FxqqQ14UmlF5ThaB55Col4mbNzuqNj8IqEyxmsxo%3D&amp;reserved=0)
>>
>> Signed-off-by: Suravee Suthikulpanit<[email protected]>
> Is this a typo in the 3.06 spec for SNPAVIC_EN?
>
> "001b-111b = Reserved."
>
> Or I guess maybe there is a newer revision that isn't available yet
> that describes 001b?

That's correct. The newer version should be coming out soon.

> Reviewed-by: Jerry Snitselaar<[email protected]>

Thank you,
Suravee