Re: [PATCH 3/3] x86/sev: Add guest support for Enhanced SMT Protection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hyper-V SEV-SNP AP boot path entirely ignores ESMT, leaving VMSA fields as zeroes.
- [Low] The tools header for msr-index.h was not synchronized with the kernel header changes.
--

commit 8604f276ddd2e9fc69ba0b3ddf175ec09324c70a
Author: Pratik R. Sampat <prsampat@amd.com>

x86/sev: Add guest support for Enhanced SMT Protection

This commit enables guest support for Enhanced SMT Protection by
populating the vCPU's identity and SMT sibling mask in the VMSA during
AP bringup. The sibling mask is fully set to allow threads of the same
guest to be co-resident.

> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index ecd77d3217f3c..acf25eca1a5c9 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
[ ... ]
> @@ -849,6 +850,18 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip, unsigned
>  	vmsa->vmpl		= snp_vmpl;
>  	vmsa->sev_features	= sev_status >> 2;
>  
> +	if (cc_platform_has(CC_ATTR_GUEST_SEV_SNP) &&
> +	    (sev_status & MSR_AMD64_SNP_ESMT_PROT)) {
> +		vmsa->vcpu_id = apic_id;
> +		/*
> +		 * The mask fully set puts every vCPU in one group, so any two
> +		 * of them may be co-resident. A legal ESMTP sibling must
> +		 * also match on ASID, so the sibling of a vCPU in guest mode is
> +		 * always either another vCPU of this same guest or idle.
> +		 */
> +		vmsa->vcpu_sibling_mask = U32_MAX;
> +	}
> +
>  	/* Populate AP's TSC scale/offset to get accurate TSC values. */
>  	if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC)) {

[Severity: High]
Does the Hyper-V SEV-SNP AP boot path also need to initialize ESMT fields?

Looking at hv_snp_boot_ap() in arch/x86/hyperv/ivm.c, the VMSA is initialized
without the new ESMT fields:

arch/x86/hyperv/ivm.c:hv_snp_boot_ap() {
    ...
	vmsa->vmpl = 0;
	vmsa->sev_features = sev_status >> 2;

	ret = snp_set_vmsa(vmsa, true);
    ...
}

If a Hyper-V SEV-SNP guest with >1 vCPUs is booted with ESMT enabled, the APs
will be launched with a sibling mask of 0. A mask of 0 does not intersect with
any other mask, which could prevent the AP from being co-scheduled with any
sibling, leading to a hardware violation or a failure to boot APs.

> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index ee9fb820a1cd7..638867e7b4ec2 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -745,7 +745,9 @@
>  #define MSR_AMD64_SNP_SMT_PROT		BIT_ULL(MSR_AMD64_SNP_SMT_PROT_BIT)
>  #define MSR_AMD64_SNP_SECURE_AVIC_BIT	18
>  #define MSR_AMD64_SNP_SECURE_AVIC	BIT_ULL(MSR_AMD64_SNP_SECURE_AVIC_BIT)
> -#define MSR_AMD64_SNP_RESERVED_BITS19_22 GENMASK_ULL(22, 19)
> +#define MSR_AMD64_SNP_ESMT_PROT_BIT	19
> +#define MSR_AMD64_SNP_ESMT_PROT	BIT_ULL(MSR_AMD64_SNP_ESMT_PROT_BIT)
> +#define MSR_AMD64_SNP_RESERVED_BITS20_22 GENMASK_ULL(22, 20)

[Severity: Low]
Does tools/arch/x86/include/asm/msr-index.h need to be synchronized with
these changes?

Currently, MSR_AMD64_SNP_RESERVED_BITS19_22 remains defined in the tools
header without the ESMT_PROT definitions. This missed synchronization could
lead to inconsistencies or warnings when building tools that depend on this
header.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789399214.git.prsampat@amd.com?part=3