Re: [PATCH 1/2] x86/coco/sev: Move SEV SYSFS group to /sys/hypervisor/

[Liam Merwick <liam.merwick@oracle.com>]

On 12/03/2025 14:41, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Move the SYSFS information about SEV to the /sys/hypervisor/ directory and link
> to it from the old location. The /sys/hypervisor/ hierarchy makes more
> sense for this information, as it is only relevant in a virtualized
> environment and contains values influenced by the hypervisor.
> 
> Signed-off-by: Joerg Roedel <jroedel@suse.de>

one suggestion below but either way,

Reviewed-by: Liam Merwick <liam.merwick@oracle.com>

> ---
>   .../ABI/testing/sysfs-devices-system-cpu      | 11 ++--------
>   Documentation/ABI/testing/sysfs-hypervisor    | 10 +++++++++
>   arch/x86/Kconfig                              |  1 +
>   arch/x86/coco/sev/core.c                      | 21 +++++++++++++------
>   4 files changed, 28 insertions(+), 15 deletions(-)
>   create mode 100644 Documentation/ABI/testing/sysfs-hypervisor
> 
> diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
> index 206079d3bd5b..f056c401a550 100644
> --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
> +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
> @@ -607,16 +607,9 @@ Description:	Umwait control
>   			  Low order two bits must be zero.
>   
>   What:		/sys/devices/system/cpu/sev
> -		/sys/devices/system/cpu/sev/vmpl
>   Date:		May 2024
> -Contact:	Linux kernel mailing list <linux-kernel@vger.kernel.org>
> -Description:	Secure Encrypted Virtualization (SEV) information
> -
> -		This directory is only present when running as an SEV-SNP guest.
> -
> -		vmpl: Reports the Virtual Machine Privilege Level (VMPL) at which
> -		      the SEV-SNP guest is running.
> -
> +Description:	This symbolic link to /sys/hypervisor/sev/ is only present when
> +		running as an SEV-SNP guest.
>   
>   What:		/sys/devices/system/cpu/svm
>   Date:		August 2019
> diff --git a/Documentation/ABI/testing/sysfs-hypervisor b/Documentation/ABI/testing/sysfs-hypervisor
> new file mode 100644
> index 000000000000..aca8b02c878c
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-hypervisor
> @@ -0,0 +1,10 @@
> +What:		/sys/devices/system/cpu/sev
> +		/sys/devices/system/cpu/sev/vmpl
> +Date:		May 2024
> +Contact:	Linux kernel mailing list <linux-kernel@vger.kernel.org>
> +Description:	Secure Encrypted Virtualization (SEV) information
> +
> +		This directory is only present when running as an SEV-SNP guest.
> +
> +		vmpl: Reports the Virtual Machine Privilege Level (VMPL) at which
> +		      the SEV-SNP guest is running.
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 1665ebaba251..5b717f6ccbbb 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1497,6 +1497,7 @@ config AMD_MEM_ENCRYPT
>   	select X86_MEM_ENCRYPT
>   	select UNACCEPTED_MEMORY
>   	select CRYPTO_LIB_AESGCM
> +	select SYS_HYPERVISOR
>   	help
>   	  Say yes to enable support for the encryption of system memory.
>   	  This requires an AMD processor that supports Secure Memory
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index 96c7bc698e6b..51a04a19449b 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -2698,12 +2698,10 @@ static int __init sev_sysfs_init(void)
>   	if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
>   		return -ENODEV;
>   
> -	dev_root = bus_get_dev_root(&cpu_subsys);
> -	if (!dev_root)
> -		return -ENODEV;
> -
> -	sev_kobj = kobject_create_and_add("sev", &dev_root->kobj);
> -	put_device(dev_root);
> +	/*
> +	 * Create /sys/hypervisor/sev/ with attributes
> +	 */
> +	sev_kobj = kobject_create_and_add("sev", hypervisor_kobj);

Given hypervisor_kobj is created elsewhere, and the caller of 
hypervisor_init() doesn't check for ENOMEM, would it be worth
adding a check here that it exists before using it?

>   
>   	if (!sev_kobj)
>   		return -ENOMEM;
> @@ -2712,6 +2710,17 @@ static int __init sev_sysfs_init(void)
>   	if (ret)
>   		kobject_put(sev_kobj);
>   
> +	/*
> +	 * Link from /sys/devices/system/cpu/sev to /sys/hypervisor/sev/ for
> +	 * compatibility reasons.
> +	 */
> +	dev_root = bus_get_dev_root(&cpu_subsys);
> +	if (!dev_root)
> +		return -ENODEV;
> +
> +	ret = compat_only_sysfs_link_entry_to_kobj(&dev_root->kobj, hypervisor_kobj, "sev", NULL);
> +	put_device(dev_root);
> +
>   	return ret;
>   }
>   arch_initcall(sev_sysfs_init);