[PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Current user-space tooling which needs access to the SEV_STATUS MSR is
using the MSR module. The use of this module poses a security risk in
any trusted execution environment and is generally discouraged.

Instead, provide an file in SYSFS in the already existing
/sys/devices/system/cpu/sev/ directory to provide the value of the
SEV_STATUS MSR to user-space.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 arch/x86/coco/sev/core.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 82492efc5d94..7b23fb803610 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -2678,10 +2678,19 @@ static ssize_t vmpl_show(struct kobject *kobj,
 	return sysfs_emit(buf, "%d\n", snp_vmpl);
 }
 
+static ssize_t sev_status_show(struct kobject *kobj,
+			       struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%llx\n", sev_status);
+}
+
 static struct kobj_attribute vmpl_attr = __ATTR_RO(vmpl);
+static struct kobj_attribute sev_status_attr = __ATTR_RO(sev_status);
+
 
 static struct attribute *vmpl_attrs[] = {
 	&vmpl_attr.attr,
+	&sev_status_attr.attr,
 	NULL
 };
 
-- 
2.48.1

[tip: x86/sev] x86/sev: Make SEV_STATUS available via SYSFS

[tip-bot2 for Joerg Roedel]

The following commit has been merged into the x86/sev branch of tip:

Commit-ID:     1d307efcf3b75d1d3aa2f8e7e932eae182d5323a
Gitweb:        https://git.kernel.org/tip/1d307efcf3b75d1d3aa2f8e7e932eae182d5323a
Author:        Joerg Roedel <jroedel@suse.de>
AuthorDate:    Wed, 05 Mar 2025 11:52:34 +01:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 05 Mar 2025 12:05:42 +01:00

x86/sev: Make SEV_STATUS available via SYSFS

Current user-space tooling which needs access to the SEV_STATUS MSR is
using the MSR module. The use of this module poses a security risk in
any trusted execution environment and is generally discouraged.

Instead, provide an file in SYSFS in the already existing
/sys/devices/system/cpu/sev/ directory to provide the value of the
SEV_STATUS MSR to user-space.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20250305105234.235553-1-joro@8bytes.org
---
 arch/x86/coco/sev/core.c |  9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 82492ef..7b23fb8 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -2678,10 +2678,19 @@ static ssize_t vmpl_show(struct kobject *kobj,
 	return sysfs_emit(buf, "%d\n", snp_vmpl);
 }
 
+static ssize_t sev_status_show(struct kobject *kobj,
+			       struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%llx\n", sev_status);
+}
+
 static struct kobj_attribute vmpl_attr = __ATTR_RO(vmpl);
+static struct kobj_attribute sev_status_attr = __ATTR_RO(sev_status);
+
 
 static struct attribute *vmpl_attrs[] = {
 	&vmpl_attr.attr,
+	&sev_status_attr.attr,
 	NULL
 };
 

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Wed, Mar 05, 2025 at 11:52:34AM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
> 
> Current user-space tooling which needs access to the SEV_STATUS MSR is
> using the MSR module. The use of this module poses a security risk in
> any trusted execution environment and is generally discouraged.
> 
> Instead, provide an file in SYSFS in the already existing
> /sys/devices/system/cpu/sev/ directory to provide the value of the
> SEV_STATUS MSR to user-space.

Right, to continue this discussion on the ML, like we said yesterday, I think
that dumping a raw MSR value is not really user-friendly.

We could stick a

Coco:

line in /proc/cpuinfo and simply dump SEV_STATUS there and TDX can put the
respective TDX-specific feature flags of what is enabled there and then we
have a good tested and well-known interface to communicate such things to
userspace through.

I'd say...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Ingo Molnar]

* Borislav Petkov <bp@alien8.de> wrote:

> On Wed, Mar 05, 2025 at 11:52:34AM +0100, Joerg Roedel wrote:
> > From: Joerg Roedel <jroedel@suse.de>
> > 
> > Current user-space tooling which needs access to the SEV_STATUS MSR is
> > using the MSR module. The use of this module poses a security risk in
> > any trusted execution environment and is generally discouraged.
> > 
> > Instead, provide an file in SYSFS in the already existing
> > /sys/devices/system/cpu/sev/ directory to provide the value of the
> > SEV_STATUS MSR to user-space.
> 
> Right, to continue this discussion on the ML, like we said yesterday, I think
> that dumping a raw MSR value is not really user-friendly.
> 
> We could stick a
> 
> Coco:
> 
> line in /proc/cpuinfo and simply dump SEV_STATUS there and TDX can put the
> respective TDX-specific feature flags of what is enabled there and then we
> have a good tested and well-known interface to communicate such things to
> userspace through.
> 
> I'd say...

It's *far* better to expose this via a targeted sysfs entry than 
polluting /proc/cpuinfo with it that everyone and their dog is parsing 
all the time ...

Thanks,

	Ingo

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Wed, Mar 05, 2025 at 12:26:13PM +0100, Ingo Molnar wrote:
> It's *far* better to expose this via a targeted sysfs entry than 
> polluting /proc/cpuinfo with it that everyone and their dog is parsing 
> all the time ...

Pasting what we're talking on IRC:

- we don't want to expose a naked MSR u64 to userspace. Might as well use
  msr-tools

- the backstory is, there are a bunch of tools which wanna know this so we
  need to agree on how to supply it to them

- I think /proc/cpuinfo is the best option right now

- and then TDX can use the same thing too

- we have a general need to expose what a confidential guest supports

- a .../sev sysfs file clearly doesn't cut it because TDX doesn't have "sev"
  - it is the Intel version of a confidential guest

- and I don't want to have "0xdeadbeef" in some sys file but "SEV SEV-ES TDX
  SecureTSC" and so on user-readable strings

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 1103 bytes --]

On 05.03.25 12:31, Borislav Petkov wrote:
> On Wed, Mar 05, 2025 at 12:26:13PM +0100, Ingo Molnar wrote:
>> It's *far* better to expose this via a targeted sysfs entry than
>> polluting /proc/cpuinfo with it that everyone and their dog is parsing
>> all the time ...
> 
> Pasting what we're talking on IRC:
> 
> - we don't want to expose a naked MSR u64 to userspace. Might as well use
>    msr-tools
> 
> - the backstory is, there are a bunch of tools which wanna know this so we
>    need to agree on how to supply it to them
> 
> - I think /proc/cpuinfo is the best option right now
> 
> - and then TDX can use the same thing too
> 
> - we have a general need to expose what a confidential guest supports
> 
> - a .../sev sysfs file clearly doesn't cut it because TDX doesn't have "sev"
>    - it is the Intel version of a confidential guest
> 
> - and I don't want to have "0xdeadbeef" in some sys file but "SEV SEV-ES TDX
>    SecureTSC" and so on user-readable strings

There is /sys/hypervisor/

Why don't put it there? Maybe under /sys/hypervisor/coco.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Wed, Mar 05, 2025 at 12:35:56PM +0100, Juergen Gross wrote:
> There is /sys/hypervisor/
> 
> Why don't put it there? Maybe under /sys/hypervisor/coco.

It's a guest features thing.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 385 bytes --]

On 05.03.25 12:41, Borislav Petkov wrote:
> On Wed, Mar 05, 2025 at 12:35:56PM +0100, Juergen Gross wrote:
>> There is /sys/hypervisor/
>>
>> Why don't put it there? Maybe under /sys/hypervisor/coco.
> 
> It's a guest features thing.
> 

And?

Under Xen there is e.g. /sys/hypervisor/guest_type which shows in which
mode the guest is running ("PV" or "HVM").


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Wed, Mar 05, 2025 at 12:48:11PM +0100, Jürgen Groß wrote:
> And?
> 
> Under Xen there is e.g. /sys/hypervisor/guest_type which shows in which
> mode the guest is running ("PV" or "HVM").

And looking for guest features in /sys/hypervisor - especially for
confidential guests which do not trust the hypervisor - is ironic and wrong to
say the least. :)

/sys/guest

or 

sys/guest/coco

Now that's more like it.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Ingo Molnar]

* Borislav Petkov <bp@alien8.de> wrote:

> On Wed, Mar 05, 2025 at 12:26:13PM +0100, Ingo Molnar wrote:
> > It's *far* better to expose this via a targeted sysfs entry than 
> > polluting /proc/cpuinfo with it that everyone and their dog is parsing 
> > all the time ...
> 
> Pasting what we're talking on IRC:
> 
> - we don't want to expose a naked MSR u64 to userspace.

As long as it's architected values that won't change randomly, I don't 
see the harm, and we expose raw feature bits all the time in sysfs.

User-space tooling would just unnecessarily parse and decode it anyway.

So if the convenience of tooling is the argument, the raw feature mask 
exposed is the best option overall.

> Might as well use msr-tools
> 
> - the backstory is, there are a bunch of tools which wanna know this so we
>   need to agree on how to supply it to them
> 
> - I think /proc/cpuinfo is the best option right now

So I disagree with that placement: /proc/cpuinfo is fundamentally 
per-CPU, while sev_status is a machine-wide word in .data. Also, 
something that is needed infrequently should not be put into the 
frequently used /proc/cpuinfo file.

> - and then TDX can use the same thing too
> 
> - we have a general need to expose what a confidential guest supports
> 
> - a .../sev sysfs file clearly doesn't cut it because TDX doesn't have "sev"
>   - it is the Intel version of a confidential guest
> 
> - and I don't want to have "0xdeadbeef" in some sys file but "SEV SEV-ES TDX
>   SecureTSC" and so on user-readable strings

So the /sys/devices/system/cpu/sev/ directory already exists and your 
arguments already apply to that, don't they?

As to the hex numbers - do you prefer to put string versions of these 
into the sysfs file:

 MSR_AMD64_SEV_ENABLED
 MSR_AMD64_SEV_ES_ENABLED
 MSR_AMD64_SEV_SNP_ENABLED
 MSR_AMD64_SNP_DEBUG_SWAP
 MSR_AMD64_SNP_SECURE_TSC
 MSR_AMD64_SNP_VTOM

?

Thanks,

	Ingo

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Wed, Mar 05, 2025 at 12:42:41PM +0100, Ingo Molnar wrote:
> So if the convenience of tooling is the argument, the raw feature mask 
> exposed is the best option overall.

The convenience of tooling *and* user. I want both. I want to be able to boot
a guest and see what features are enabled without needing a tool.

And, at the same time, tools should be able to use the same interface.

Exactly like we *and glibc* use /proc/cpuinfo today. Now think the same thing
but for confidential guests.

> So the /sys/devices/system/cpu/sev/ directory already exists and your 
> arguments already apply to that, don't they?

Lemme repeat:

>> - a .../sev sysfs file clearly doesn't cut it because TDX doesn't have "sev"
>>   - it is the Intel version of a confidential guest

>> - we have a general need to expose what a confidential guest supports

> As to the hex numbers - do you prefer to put string versions of these 
> into the sysfs file:

See above.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Wed, Mar 05, 2025 at 12:50:35PM +0100, Borislav Petkov wrote:
> On Wed, Mar 05, 2025 at 12:42:41PM +0100, Ingo Molnar wrote:
> > So if the convenience of tooling is the argument, the raw feature mask 
> > exposed is the best option overall.
> 
> The convenience of tooling *and* user. I want both. I want to be able to boot
> a guest and see what features are enabled without needing a tool.
> 
> And, at the same time, tools should be able to use the same interface.
> 
> Exactly like we *and glibc* use /proc/cpuinfo today. Now think the same thing
> but for confidential guests.

So this question boils down to whether the parsing of the bits happens
in kernel- or user-space. Actually there is already parsing in
kernel-space to print the status bits into the kernel log:

	SEV: Status: SEV SEV-ES SEV-SNP

... which is great for a quick glance without needing any tools. The
user-space tools which already exist have their own parsing of the bits
and for them it is much easier to consume the raw value of the
SEV_STATUS MSR. See my changes to snpguest:

	https://github.com/virtee/snpguest/pull/88/files

Btw, what is the equivalent on the Intel TDX side for these feature
bits?

Regards,

-- 
Jörg Rödel
jroedel@suse.de

SUSE Software Solutions Germany GmbH
Frankenstraße 146
90461 Nürnberg
Germany
https://www.suse.com/

Geschäftsführer: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Wed, Mar 05, 2025 at 02:56:34PM +0100, Joerg Roedel wrote:
> On Wed, Mar 05, 2025 at 12:50:35PM +0100, Borislav Petkov wrote:
> > On Wed, Mar 05, 2025 at 12:42:41PM +0100, Ingo Molnar wrote:
> > > So if the convenience of tooling is the argument, the raw feature mask 
> > > exposed is the best option overall.
> > 
> > The convenience of tooling *and* user. I want both. I want to be able to boot
> > a guest and see what features are enabled without needing a tool.
> > 
> > And, at the same time, tools should be able to use the same interface.
> > 
> > Exactly like we *and glibc* use /proc/cpuinfo today. Now think the same thing
> > but for confidential guests.
> 
> So this question boils down to whether the parsing of the bits happens
> in kernel- or user-space. Actually there is already parsing in
> kernel-space to print the status bits into the kernel log:
> 
> 	SEV: Status: SEV SEV-ES SEV-SNP
> 
> ... which is great for a quick glance without needing any tools. The
> user-space tools which already exist have their own parsing of the bits
> and for them it is much easier to consume the raw value of the
> SEV_STATUS MSR. See my changes to snpguest:
> 
> 	https://github.com/virtee/snpguest/pull/88/files

Well, I guess we can do both:

cat /sys/...

SEV_STATUS(0xdeadbeef): SEV SEV-ES SEV-SNP

So that people don't have to pick apart the MSR hex value.

> Btw, what is the equivalent on the Intel TDX side for these feature
> bits?

There is none, AFAICT. That's why the whole discussion.

Lemme add TDX folks.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Dave Hansen]

On 3/5/25 07:37, Borislav Petkov wrote:
>> So this question boils down to whether the parsing of the bits happens
>> in kernel- or user-space. Actually there is already parsing in
>> kernel-space to print the status bits into the kernel log:
>>
>> 	SEV: Status: SEV SEV-ES SEV-SNP
>>
>> ... which is great for a quick glance without needing any tools. The
>> user-space tools which already exist have their own parsing of the bits
>> and for them it is much easier to consume the raw value of the
>> SEV_STATUS MSR. See my changes to snpguest:
>>
>> 	https://github.com/virtee/snpguest/pull/88/files
> Well, I guess we can do both:
> 
> cat /sys/...
> 
> SEV_STATUS(0xdeadbeef): SEV SEV-ES SEV-SNP
> 
> So that people don't have to pick apart the MSR hex value.
> 
>> Btw, what is the equivalent on the Intel TDX side for these feature
>> bits?
> There is none, AFAICT. That's why the whole discussion.

TDX's history isn't as exciting as SEV.

TDX guests have CPUID to tell them that they're running that way.

TDX hosts are much more arcane. You can't _actually_ know that it's a
TDX host until you actually start making successful SEAMCALLs and the
TDX module answers them. But we fudge it by just looking at
MSR_IA32_MKTME_KEYID_PARTITIONING at boot and assuming that anything
with that MSR will be able to be a TDX host.

We've just got X86_FEATUREs for hosts and guests:

	#define X86_FEATURE_TDX_HOST_PLATFORM ( 7*32+ 7)
	#define X86_FEATURE_TDX_GUEST ( 8*32+22)

and that's it.

ks wa

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Dave Hansen]

On 3/5/25 07:37, Borislav Petkov wrote:
>> So this question boils down to whether the parsing of the bits happens
>> in kernel- or user-space. Actually there is already parsing in
>> kernel-space to print the status bits into the kernel log:
>>
>> 	SEV: Status: SEV SEV-ES SEV-SNP
>>
>> ... which is great for a quick glance without needing any tools. The
>> user-space tools which already exist have their own parsing of the bits
>> and for them it is much easier to consume the raw value of the
>> SEV_STATUS MSR. See my changes to snpguest:
>>
>> 	https://github.com/virtee/snpguest/pull/88/files
> Well, I guess we can do both:
> 
> cat /sys/...
> 
> SEV_STATUS(0xdeadbeef): SEV SEV-ES SEV-SNP
> 
> So that people don't have to pick apart the MSR hex value.
> 
>> Btw, what is the equivalent on the Intel TDX side for these feature
>> bits?
> There is none, AFAICT. That's why the whole discussion.

TDX's history isn't as exciting as SEV.

TDX guests have CPUID to tell them that they're running that way.

TDX hosts are much more arcane. You can't _actually_ know that it's a
TDX host until you actually start making successful SEAMCALLs and the
TDX module answers them. But we fudge it by just looking at
MSR_IA32_MKTME_KEYID_PARTITIONING at boot and assuming that anything
with that MSR will be able to be a TDX host.

We've just got X86_FEATUREs for hosts and guests:

	#define X86_FEATURE_TDX_HOST_PLATFORM ( 7*32+ 7)
	#define X86_FEATURE_TDX_GUEST ( 8*32+22)

and that's it.

Folks certainly _want_ something in sysfs to dump the TDX module version
and so forth, but we've resisted the urge so far.

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Wed, Mar 05, 2025 at 08:40:29AM -0800, Dave Hansen wrote:
> TDX guests have CPUID to tell them that they're running that way.

And those CPUID leafs cannot be modified or intercepted or so by the
hypervisor?

> TDX hosts are much more arcane. You can't _actually_ know that it's a
> TDX host until you actually start making successful SEAMCALLs and the
> TDX module answers them. But we fudge it by just looking at
> MSR_IA32_MKTME_KEYID_PARTITIONING at boot and assuming that anything
> with that MSR will be able to be a TDX host.

Fun. :)

> We've just got X86_FEATUREs for hosts and guests:
> 
> 	#define X86_FEATURE_TDX_HOST_PLATFORM ( 7*32+ 7)
> 	#define X86_FEATURE_TDX_GUEST ( 8*32+22)
> 
> and that's it.

And there are no new ones coming down the pipe?

> Folks certainly _want_ something in sysfs to dump the TDX module version
> and so forth, but we've resisted the urge so far.

Perhaps now is the time do design something together...

I was thinking

/sys/guest/...

or something tied to the x86_platform gunk so that we can stick always some
info there about any platform arch/x86/ has detected and is running on...

Hmmm.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Dave Hansen]

On 3/5/25 08:55, Borislav Petkov wrote:
> On Wed, Mar 05, 2025 at 08:40:29AM -0800, Dave Hansen wrote:
>> TDX guests have CPUID to tell them that they're running that way.
> 
> And those CPUID leafs cannot be modified or intercepted or so by the
> hypervisor?

They are documented as coming straight from the TDX module when TDX is
in place. But there's nothing stopping an evil hypervisor from faking
them, except attestation.

>> We've just got X86_FEATUREs for hosts and guests:
>>
>> 	#define X86_FEATURE_TDX_HOST_PLATFORM ( 7*32+ 7)
>> 	#define X86_FEATURE_TDX_GUEST ( 8*32+22)
>>
>> and that's it.
> 
> And there are no new ones coming down the pipe?

Not really. There are always new features in the pipeline, but no real
fundamental changes to the threat model like SEV has had throughout its
iterations.

>> Folks certainly _want_ something in sysfs to dump the TDX module version
>> and so forth, but we've resisted the urge so far.
> 
> Perhaps now is the time do design something together...
> 
> I was thinking
> 
> /sys/guest/...
> 
> or something tied to the x86_platform gunk so that we can stick always some
> info there about any platform arch/x86/ has detected and is running on...

Xen has a bunch of gunk in:

	/sys/hypervisor

Joerg, why do folks care if they're running under SEV? What kind of
stuff are they doing after they do the rdmsr and see that SEV is in play?

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Wed, Mar 05, 2025 at 09:09:47AM -0800, Dave Hansen wrote:
> They are documented as coming straight from the TDX module when TDX is
> in place. But there's nothing stopping an evil hypervisor from faking
> them, except attestation.

That is the same for MSR_SEV_STATUS. A malicious HV can emulate it on
older hardware and give fake values to the guest. The only trusted proof
is attestation.

> Joerg, why do folks care if they're running under SEV? What kind of
> stuff are they doing after they do the rdmsr and see that SEV is in play?

So people started to use tooling which tells them whether the VM runs
with SEV (and what level of SEV). One of these tools is snpguest which has
the 'ok' subcommand that basically takes the SEV_GUEST MSR and dumps its
bits in a human readable form. The tool can also do full attestation,
but for a first check it only relies on the MSR. I think the tool is
also called by some remote management tooling to fetch information about
the VM.

Regards,

	Joerg

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Wed, Mar 05, 2025 at 09:09:47AM -0800, Dave Hansen wrote:
> Not really. There are always new features in the pipeline, but no real
> fundamental changes to the threat model like SEV has had throughout its
> iterations.

I'd call that more a iterative design with adding more features with each
gen.

Regardless, we should think about some sane mechanism of communicating guest
coco attributes to its userspace...

> Xen has a bunch of gunk in:
> 
> 	/sys/hypervisor

Yeah, see uptread.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Kirill A. Shutemov]

On Wed, Mar 05, 2025 at 08:40:29AM -0800, Dave Hansen wrote:
> On 3/5/25 07:37, Borislav Petkov wrote:
> >> So this question boils down to whether the parsing of the bits happens
> >> in kernel- or user-space. Actually there is already parsing in
> >> kernel-space to print the status bits into the kernel log:
> >>
> >> 	SEV: Status: SEV SEV-ES SEV-SNP
> >>
> >> ... which is great for a quick glance without needing any tools. The
> >> user-space tools which already exist have their own parsing of the bits
> >> and for them it is much easier to consume the raw value of the
> >> SEV_STATUS MSR. See my changes to snpguest:
> >>
> >> 	https://github.com/virtee/snpguest/pull/88/files
> > Well, I guess we can do both:
> > 
> > cat /sys/...
> > 
> > SEV_STATUS(0xdeadbeef): SEV SEV-ES SEV-SNP
> > 
> > So that people don't have to pick apart the MSR hex value.
> > 
> >> Btw, what is the equivalent on the Intel TDX side for these feature
> >> bits?
> > There is none, AFAICT. That's why the whole discussion.
> 
> TDX's history isn't as exciting as SEV.
> 
> TDX guests have CPUID to tell them that they're running that way.
> 
> TDX hosts are much more arcane. You can't _actually_ know that it's a
> TDX host until you actually start making successful SEAMCALLs and the
> TDX module answers them. But we fudge it by just looking at
> MSR_IA32_MKTME_KEYID_PARTITIONING at boot and assuming that anything
> with that MSR will be able to be a TDX host.
> 
> We've just got X86_FEATUREs for hosts and guests:
> 
> 	#define X86_FEATURE_TDX_HOST_PLATFORM ( 7*32+ 7)
> 	#define X86_FEATURE_TDX_GUEST ( 8*32+22)
> 
> and that's it.
> 
> Folks certainly _want_ something in sysfs to dump the TDX module version
> and so forth, but we've resisted the urge so far.

Alexey looking into exposing TDX module version in sysfs for both guest
and host.

I think it would be useful for guest to make attributes and TD_CTLS
available via sysfs. So far, we only dump them in dmesg on boot (see
564ea84c8c14).

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Thu, Mar 06, 2025 at 10:01:17AM +0200, Kirill A. Shutemov wrote:
> Alexey looking into exposing TDX module version in sysfs for both guest
> and host.
> 
> I think it would be useful for guest to make attributes and TD_CTLS
> available via sysfs. So far, we only dump them in dmesg on boot (see
> 564ea84c8c14).

Okay, do you have ideas already on where to put this information in
SYSFS?

Regards,

	Joerg

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Thu, Mar 06, 2025 at 09:38:21AM +0100, Joerg Roedel wrote:
> On Thu, Mar 06, 2025 at 10:01:17AM +0200, Kirill A. Shutemov wrote:
> > Alexey looking into exposing TDX module version in sysfs for both guest
> > and host.
> > 
> > I think it would be useful for guest to make attributes and TD_CTLS
> > available via sysfs. So far, we only dump them in dmesg on boot (see
> > 564ea84c8c14).
> 
> Okay, do you have ideas already on where to put this information in
> SYSFS?

Right, I was thinking about it: sysfs does a one-datum-per-file thing and that
would go nuts very quickly.

I'm thinking we probably should design something like /proc/cpuinfo where we
have a single file with coco-specific infos per line:

SEV: ...
 * fw_info: ...
 * features: ...
TDX: ...
 * TD_CTLS: ...
 * features: ...

I mean, we'll agree on what the most optimal format is but having a single
file would make this a lot saner for everyone involved.

So maybe stick it in /proc...?

I'm not sure if /proc accepts any new files - I can't find anything after
a quick search but this would be the perfect place for that...

Hmmm.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Kirill A. Shutemov]

On Thu, Mar 06, 2025 at 11:31:19AM +0100, Borislav Petkov wrote:
> On Thu, Mar 06, 2025 at 09:38:21AM +0100, Joerg Roedel wrote:
> > On Thu, Mar 06, 2025 at 10:01:17AM +0200, Kirill A. Shutemov wrote:
> > > Alexey looking into exposing TDX module version in sysfs for both guest
> > > and host.
> > > 
> > > I think it would be useful for guest to make attributes and TD_CTLS
> > > available via sysfs. So far, we only dump them in dmesg on boot (see
> > > 564ea84c8c14).
> > 
> > Okay, do you have ideas already on where to put this information in
> > SYSFS?
> 
> Right, I was thinking about it: sysfs does a one-datum-per-file thing and that
> would go nuts very quickly.

I think we should be pretty use to one-datum-per-file thing by now.
I don't see why coco case is special.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Thu, Mar 06, 2025 at 03:36:02PM +0200, Kirill A. Shutemov wrote:
> I think we should be pretty use to one-datum-per-file thing by now.
> I don't see why coco case is special.

I want to avoid all the boilerplate code for each sysfs node. And it doesn't
matter if you do:

cat /proc/coco

or

grep -r . /sys/.../coco/

And you can't format sysfs output in a human-readable fashion if you have to
dump registers like SEV_STATUS, which started that whole conversation.

But if everybody is crazy about sysfs, I ...

  [ /me goes and looks up the proverb people fancy using nowadays... ]

... I won't die on that hill.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Alexey Gladkov (Intel)]

On Thu, Mar 06, 2025 at 09:38:21AM +0100, Joerg Roedel wrote:
> On Thu, Mar 06, 2025 at 10:01:17AM +0200, Kirill A. Shutemov wrote:
> > Alexey looking into exposing TDX module version in sysfs for both guest
> > and host.
> > 
> > I think it would be useful for guest to make attributes and TD_CTLS
> > available via sysfs. So far, we only dump them in dmesg on boot (see
> > 564ea84c8c14).
> 
> Okay, do you have ideas already on where to put this information in
> SYSFS?

I was thinking to suggest something like that

/sys/firmware/coco/tdx/...
/sys/firmware/coco/sev/...

-- 
Rgrds, legion

---------------------------------------------------------------------
Intel Czech Tradings, Inc., organizacni slozka 
Registered Office: Prague 8, Pobrezni Street, No. 620/3, 
Postal Code 186 00, Czech Republic
Identification No: 60165898
Entered in the Commercial Register 
kept by the Municipal Court in Prague, Section A, Inset 8332

The above is a branch office of:

Intel Czech Tradings, Inc.
A Corporation registered in California, USA
Registered Office: 818 West 7th Street, Los Angeles, CA 90017, USA
Registered No: 1828841

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Thu, Mar 06, 2025 at 11:37:28AM +0100, Alexey Gladkov (Intel) wrote:
> I was thinking to suggest something like that
> 
> /sys/firmware/coco/tdx/...
> /sys/firmware/coco/sev/...

So on a second thought I'd like to vote for the /sys/hypervisor/
hierarchy. The `firmware` term is a bit amibious here, the TDX module
can be seen as a kind of firmware for the guest OS, but realistically it
is more like another hypervisor sitting between KVM and the guest.

Also the settings on the SEV side that need to be exposed (VMPL and
SEV_STATUS) are CPU properties, but on the other side also set by some
form of hypervisor (either KVM/QEMU, the SVSM, or some other paravisor
in-between).

Overall /sys/hypervisor/ seems to be the best-fitting location for all
this data. To avoid ambiguation I propose:

	/sys/hypervisor/common/[coco/]tdx/
	/sys/hypervisor/common/[coco/]sev/

Using `common` makes it clear that this directory does not point to some
sort of Hypervisor, but to data common to all hypervisors. Using another
`coco` subdirectory is not necessary in my view, but if people think it
should exist I am fine with that.

Is this something we can agree on?

Regards,

	Joerg

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Mon, Mar 10, 2025 at 11:28:46AM +0100, Joerg Roedel wrote:
> So on a second thought I'd like to vote for the /sys/hypervisor/
> hierarchy. The `firmware` term is a bit amibious here, the TDX module
> can be seen as a kind of firmware for the guest OS, but realistically it
> is more like another hypervisor sitting between KVM and the guest.
> 
> Also the settings on the SEV side that need to be exposed (VMPL and
> SEV_STATUS) are CPU properties, but on the other side also set by some
> form of hypervisor (either KVM/QEMU, the SVSM, or some other paravisor
> in-between).

Dunno, it still reads weird because if you wanna put guest-specific things in
there, /sys/hypervisor really sounds like the wrong place...

> Overall /sys/hypervisor/ seems to be the best-fitting location for all
> this data. To avoid ambiguation I propose:
> 
> 	/sys/hypervisor/common/[coco/]tdx/
> 	/sys/hypervisor/common/[coco/]sev/
> 
> Using `common` makes it clear that this directory does not point to some
> sort of Hypervisor, but to data common to all hypervisors. Using another
> `coco` subdirectory is not necessary in my view, but if people think it
> should exist I am fine with that.

... or you can drop the "common" thing and use only the "coco":

/sys/hypervisor/coco

and then you kinda denote that while it is the hypervisor hierarchy, it is
related to confidential computing so it could be consumed by guests too.

But I still don't see why we can't simply do

/sys/guest

It is just another sysfs node. Or is there a particular reason to stick to
/sys/hypervisor?

And putting it in sysfs still doesn't solve the human-readable aspect: dumping
a raw SEV_STATUS might as well be simply reading the MSR and if someone wants
to read it, someone would need to go count bits. Imagine the following
scenario: a user reports a bug, you say, ok, send me

/sys/hypervisor/coco/sev/sev_status

you get it and you dump it through your script or start looking at the bits.
Yeah, we all have scripts for that but it ain't too user-friendly...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Mon, Mar 10, 2025 at 12:02:02PM +0100, Borislav Petkov wrote:
> ... or you can drop the "common" thing and use only the "coco":
>
> /sys/hypervisor/coco

Common is less likely to be mistaken for a hypervisor name than coco.
But if there is agreement on that naming I can live with that.

> and then you kinda denote that while it is the hypervisor hierarchy, it is
> related to confidential computing so it could be consumed by guests too.
> 
> But I still don't see why we can't simply do
> 
> /sys/guest
> 
> It is just another sysfs node. Or is there a particular reason to stick to
> /sys/hypervisor?

/sys/hypervisor/ has the best-fitting name imho. Unfortunately it is
taken in a very non-generic way by Xen, with no clean way to make it
more generic without breaking Xen or increase the mess. So /sys/guest
might be a viable alternative. /sys/guest/xen/ could then link to
/sys/hypervisor/.

> And putting it in sysfs still doesn't solve the human-readable aspect: dumping
> a raw SEV_STATUS might as well be simply reading the MSR and if someone wants
> to read it, someone would need to go count bits. Imagine the following
> scenario: a user reports a bug, you say, ok, send me
> 
> /sys/hypervisor/coco/sev/sev_status
> 
> you get it and you dump it through your script or start looking at the bits.
> Yeah, we all have scripts for that but it ain't too user-friendly...

Right, it is not really a good human-readable interface. On the other
side SYSFS was always an interface targeted more towards tooling than
humans, therefore the one-datum-per-file rule. The use-case I want to
target with this patch is also tooling-related.

We can add a human-readable version of the coco-features somewhere else,
if wanted.  You already suggested /proc/cpuinfo, which in itself is
designed for direct human consumption.

Regards,

	Joerg

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Mon, Mar 10, 2025 at 01:46:13PM +0100, Joerg Roedel wrote:
> Right, it is not really a good human-readable interface. On the other
> side SYSFS was always an interface targeted more towards tooling than
> humans, therefore the one-datum-per-file rule. The use-case I want to
> target with this patch is also tooling-related.
> 
> We can add a human-readable version of the coco-features somewhere else,
> if wanted.  You already suggested /proc/cpuinfo, which in itself is
> designed for direct human consumption.

Right, except mingo thinks /proc/cpuinfo is per CPU and there's "duplication".
I say meh but sure, we can design a special one of really needed. /proc/guest
maybe :-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Alexey Gladkov]

On Mon, Mar 10, 2025 at 11:28:46AM +0100, Joerg Roedel wrote:
> On Thu, Mar 06, 2025 at 11:37:28AM +0100, Alexey Gladkov (Intel) wrote:
> > I was thinking to suggest something like that
> > 
> > /sys/firmware/coco/tdx/...
> > /sys/firmware/coco/sev/...
> 
> So on a second thought I'd like to vote for the /sys/hypervisor/
> hierarchy. The `firmware` term is a bit amibious here, the TDX module
> can be seen as a kind of firmware for the guest OS, but realistically it
> is more like another hypervisor sitting between KVM and the guest.
> 
> Also the settings on the SEV side that need to be exposed (VMPL and
> SEV_STATUS) are CPU properties, but on the other side also set by some
> form of hypervisor (either KVM/QEMU, the SVSM, or some other paravisor
> in-between).
> 
> Overall /sys/hypervisor/ seems to be the best-fitting location for all
> this data. To avoid ambiguation I propose:
> 
> 	/sys/hypervisor/common/[coco/]tdx/
> 	/sys/hypervisor/common/[coco/]sev/

The /sys/hypervisor requires CONFIG_SYS_HYPERVISOR=y. Now, this parameter
is not required for the minimum TDX guest configuration.

As I can see right now [1] this directory is used exclusively by xen team.
It's part of their ABI stable. I'm not sure we can go in there.

/sys/hypervisor/compilation/compile_date
/sys/hypervisor/compilation/compiled_by
/sys/hypervisor/compilation/compiler
/sys/hypervisor/properties/capabilities
/sys/hypervisor/properties/changeset
/sys/hypervisor/properties/features
/sys/hypervisor/properties/pagesize
/sys/hypervisor/properties/virtual_start
/sys/hypervisor/type
/sys/hypervisor/uuid
/sys/hypervisor/version/extra
/sys/hypervisor/version/major
/sys/hypervisor/version/minor
/sys/hypervisor/start_flags/*
/sys/hypervisor/guest_type
/sys/hypervisor/pmu/pmu_mode
/sys/hypervisor/pmu/pmu_features
/sys/hypervisor/properties/buildid

[1] Documentation/ABI/stable/sysfs-hypervisor-xen

> Using `common` makes it clear that this directory does not point to some
> sort of Hypervisor, but to data common to all hypervisors. Using another
> `coco` subdirectory is not necessary in my view, but if people think it
> should exist I am fine with that.
> 
> Is this something we can agree on?
> 
> Regards,
> 
> 	Joerg

-- 
Rgrds, legion

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 1536 bytes --]

On 10.03.25 12:24, Alexey Gladkov wrote:
> On Mon, Mar 10, 2025 at 11:28:46AM +0100, Joerg Roedel wrote:
>> On Thu, Mar 06, 2025 at 11:37:28AM +0100, Alexey Gladkov (Intel) wrote:
>>> I was thinking to suggest something like that
>>>
>>> /sys/firmware/coco/tdx/...
>>> /sys/firmware/coco/sev/...
>>
>> So on a second thought I'd like to vote for the /sys/hypervisor/
>> hierarchy. The `firmware` term is a bit amibious here, the TDX module
>> can be seen as a kind of firmware for the guest OS, but realistically it
>> is more like another hypervisor sitting between KVM and the guest.
>>
>> Also the settings on the SEV side that need to be exposed (VMPL and
>> SEV_STATUS) are CPU properties, but on the other side also set by some
>> form of hypervisor (either KVM/QEMU, the SVSM, or some other paravisor
>> in-between).
>>
>> Overall /sys/hypervisor/ seems to be the best-fitting location for all
>> this data. To avoid ambiguation I propose:
>>
>> 	/sys/hypervisor/common/[coco/]tdx/
>> 	/sys/hypervisor/common/[coco/]sev/
> 
> The /sys/hypervisor requires CONFIG_SYS_HYPERVISOR=y. Now, this parameter
> is not required for the minimum TDX guest configuration.
> 
> As I can see right now [1] this directory is used exclusively by xen team.
> It's part of their ABI stable. I'm not sure we can go in there.

We can (saying that with my Xen maintainer hat on).

There is /sys/hypervisor/type which should return the used virtualization
environment ("xen" when running as a Xen guest).


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Mon, Mar 10, 2025 at 01:28:38PM +0100, Juergen Gross wrote:
> We can (saying that with my Xen maintainer hat on).
> 
> There is /sys/hypervisor/type which should return the used virtualization
> environment ("xen" when running as a Xen guest).

In CoCo environments there can be more than one hypervisor beneath the
guest. For example KVM as the untrusted host, SVSM or another para-visor
as the trusted in-guest hypervisor. On TDX there is also the TDX module
in-between, which is another level of hypervisors. ARM and Risc-V will
have similar architectures.


Regards,

	Joerg

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 1144 bytes --]

On 10.03.25 13:35, Joerg Roedel wrote:
> On Mon, Mar 10, 2025 at 01:28:38PM +0100, Juergen Gross wrote:
>> We can (saying that with my Xen maintainer hat on).
>>
>> There is /sys/hypervisor/type which should return the used virtualization
>> environment ("xen" when running as a Xen guest).
> 
> In CoCo environments there can be more than one hypervisor beneath the
> guest. For example KVM as the untrusted host, SVSM or another para-visor
> as the trusted in-guest hypervisor. On TDX there is also the TDX module
> in-between, which is another level of hypervisors. ARM and Risc-V will
> have similar architectures.

There are multiple possible approaches here:

1. Only name the hypervisor nearest to the guest (similar to running Xen on
    top of another hypervisor in nested virtualization, which would still
    say "xen").

2. Add another entry for naming the outer hypervisor(s) (if possible).

3. Name all known hypervisor levels, like "kvm,svsm" or "svsm,kvm").

BTW, I've found another user of /sys/hypervisor: s390 running as a z/VM
guest is saying "z/VM Hypervisor" in /sys/hypervisor/type.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Mon, Mar 10, 2025 at 01:49:46PM +0100, Juergen Gross wrote:
> 1. Only name the hypervisor nearest to the guest (similar to running Xen on
>    top of another hypervisor in nested virtualization, which would still
>    say "xen").
> 
> 2. Add another entry for naming the outer hypervisor(s) (if possible).
> 
> 3. Name all known hypervisor levels, like "kvm,svsm" or "svsm,kvm").

/sys/guest it is then.

Let's just keep /sys/hypervisor alone and as Joerg said, we can link to it
from /sys/guest.

I guess that sounds ok...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Tom Lendacky]

On 3/10/25 08:38, Borislav Petkov wrote:
> On Mon, Mar 10, 2025 at 01:49:46PM +0100, Juergen Gross wrote:
>> 1. Only name the hypervisor nearest to the guest (similar to running Xen on
>>    top of another hypervisor in nested virtualization, which would still
>>    say "xen").
>>
>> 2. Add another entry for naming the outer hypervisor(s) (if possible).
>>
>> 3. Name all known hypervisor levels, like "kvm,svsm" or "svsm,kvm").
> 
> /sys/guest it is then.
> 
> Let's just keep /sys/hypervisor alone and as Joerg said, we can link to it
> from /sys/guest.
> 
> I guess that sounds ok...

And we should link the VMPL setting currently under
/sys/devices/system/cpu/sev/

Thanks,
Tom

> 
> Thx.
> 

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Alexey Gladkov]

On Mon, Mar 10, 2025 at 02:38:33PM +0100, Borislav Petkov wrote:
> On Mon, Mar 10, 2025 at 01:49:46PM +0100, Juergen Gross wrote:
> > 1. Only name the hypervisor nearest to the guest (similar to running Xen on
> >    top of another hypervisor in nested virtualization, which would still
> >    say "xen").
> > 
> > 2. Add another entry for naming the outer hypervisor(s) (if possible).
> > 
> > 3. Name all known hypervisor levels, like "kvm,svsm" or "svsm,kvm").
> 
> /sys/guest it is then.
> 
> Let's just keep /sys/hypervisor alone and as Joerg said, we can link to it
> from /sys/guest.
> 
> I guess that sounds ok...

Am I understand correctly that you and Joerg are proposing

/sys/guest/tdx/...
/sys/guest/sev/...

?

Which path to use for the host side ?

For guest: /sys/coco/guest/{tdx,sev}/...
For host:  /sys/coco/host/{tdx,sev}/...

Maybe it would be better to add the "coco" subdirectory or something like
that ?

-- 
Rgrds, legion

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Mon, Mar 10, 2025 at 03:50:09PM +0100, Alexey Gladkov wrote:
> Am I understand correctly that you and Joerg are proposing
> 
> /sys/guest/tdx/...
> /sys/guest/sev/...
> 
> ?
> 
> Which path to use for the host side ?
> 
> For guest: /sys/coco/guest/{tdx,sev}/...
> For host:  /sys/coco/host/{tdx,sev}/...
> 
> Maybe it would be better to add the "coco" subdirectory or something like
> that ?

Hmm, so we can do

/sys/guest

and extend

/sys/hypervisor

Or we can do what you're suggesting.

If we do /sys/coco/host, then we'll have two different places to read HV info.

Or we can stick *everything* coco needs in

/sys/coco/{sev,tdx}

but then it is coco-specific and if other guest types want to put stuff in
sysfs, it'll get ugly.

So I guess having

/sys/guest
and
/sys/hypervisor

kinda keeps it all clean, hierarchy-wise...

Right?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 1330 bytes --]

On 10.03.25 16:11, Borislav Petkov wrote:
> On Mon, Mar 10, 2025 at 03:50:09PM +0100, Alexey Gladkov wrote:
>> Am I understand correctly that you and Joerg are proposing
>>
>> /sys/guest/tdx/...
>> /sys/guest/sev/...
>>
>> ?
>>
>> Which path to use for the host side ?
>>
>> For guest: /sys/coco/guest/{tdx,sev}/...
>> For host:  /sys/coco/host/{tdx,sev}/...
>>
>> Maybe it would be better to add the "coco" subdirectory or something like
>> that ?
> 
> Hmm, so we can do
> 
> /sys/guest
> 
> and extend
> 
> /sys/hypervisor
> 
> Or we can do what you're suggesting.
> 
> If we do /sys/coco/host, then we'll have two different places to read HV info.
> 
> Or we can stick *everything* coco needs in
> 
> /sys/coco/{sev,tdx}
> 
> but then it is coco-specific and if other guest types want to put stuff in
> sysfs, it'll get ugly.
> 
> So I guess having
> 
> /sys/guest
> and
> /sys/hypervisor
> 
> kinda keeps it all clean, hierarchy-wise...
> 
> Right?

Kind of.

/sys/hypervisor is meant to provide data for running under a hypervisor.

It is NOT meant to provide data for running as a hypervisor.

So far when running either under Xen or under z/VM /sys/hypervisor is being
populated.

I'm not feeling really strong here. I just want to state the status quo.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Mon, Mar 10, 2025 at 04:33:08PM +0100, Jürgen Groß wrote:
> Kind of.
> 
> /sys/hypervisor is meant to provide data for running under a hypervisor.
> 
> It is NOT meant to provide data for running as a hypervisor.

Hah.

Which would mean that

/sys/hypervisor == /sys/guest

the way we envision it.

Do we need to dump all the HV levels in it?

I think Joerg wants to dump the fact that there's an SVSM present. I guess we
can dump the "stack" of "things" running under Linux as you suggest it.

Hmmm.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Alexey Gladkov]

On Mon, Mar 10, 2025 at 04:41:52PM +0100, Borislav Petkov wrote:
> On Mon, Mar 10, 2025 at 04:33:08PM +0100, Jürgen Groß wrote:
> > Kind of.
> > 
> > /sys/hypervisor is meant to provide data for running under a hypervisor.
> > 
> > It is NOT meant to provide data for running as a hypervisor.
> 
> Hah.
> 
> Which would mean that
> 
> /sys/hypervisor == /sys/guest
> 
> the way we envision it.
> 
> Do we need to dump all the HV levels in it?

Hm. We already have:

/sys/hypervisor/type        (Type of hypervisor)
/sys/hypervisor/guest_type  (Type of guest)

I don't really understand how you propose to locate the files for host and
guest and what you propose to put in /sys/guest.

> I think Joerg wants to dump the fact that there's an SVSM present. I guess we
> can dump the "stack" of "things" running under Linux as you suggest it.
> 
> Hmmm.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette
> 

-- 
Rgrds, legion

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Alexey Gladkov]

On Mon, Mar 10, 2025 at 04:33:08PM +0100, Jürgen Groß wrote:
> On 10.03.25 16:11, Borislav Petkov wrote:
> > On Mon, Mar 10, 2025 at 03:50:09PM +0100, Alexey Gladkov wrote:
> >> Am I understand correctly that you and Joerg are proposing
> >>
> >> /sys/guest/tdx/...
> >> /sys/guest/sev/...
> >>
> >> ?
> >>
> >> Which path to use for the host side ?
> >>
> >> For guest: /sys/coco/guest/{tdx,sev}/...
> >> For host:  /sys/coco/host/{tdx,sev}/...
> >>
> >> Maybe it would be better to add the "coco" subdirectory or something like
> >> that ?
> > 
> > Hmm, so we can do
> > 
> > /sys/guest
> > 
> > and extend
> > 
> > /sys/hypervisor
> > 
> > Or we can do what you're suggesting.
> > 
> > If we do /sys/coco/host, then we'll have two different places to read HV info.
> > 
> > Or we can stick *everything* coco needs in
> > 
> > /sys/coco/{sev,tdx}
> > 
> > but then it is coco-specific and if other guest types want to put stuff in
> > sysfs, it'll get ugly.
> > 
> > So I guess having
> > 
> > /sys/guest
> > and
> > /sys/hypervisor
> > 
> > kinda keeps it all clean, hierarchy-wise...
> > 
> > Right?
> 
> Kind of.
> 
> /sys/hypervisor is meant to provide data for running under a hypervisor.
> 
> It is NOT meant to provide data for running as a hypervisor.
> 
> So far when running either under Xen or under z/VM /sys/hypervisor is being
> populated.
> 
> I'm not feeling really strong here. I just want to state the status quo.

OK, so I misunderstood.

If in the /sys/hypervisor we have information for guest (for running under
a hypervisor), where do you propose to put the information for the
host-side (for running as a hypervisor) ?

-- 
Rgrds, legion

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 1825 bytes --]

On 10.03.25 16:43, Alexey Gladkov wrote:
> On Mon, Mar 10, 2025 at 04:33:08PM +0100, Jürgen Groß wrote:
>> On 10.03.25 16:11, Borislav Petkov wrote:
>>> On Mon, Mar 10, 2025 at 03:50:09PM +0100, Alexey Gladkov wrote:
>>>> Am I understand correctly that you and Joerg are proposing
>>>>
>>>> /sys/guest/tdx/...
>>>> /sys/guest/sev/...
>>>>
>>>> ?
>>>>
>>>> Which path to use for the host side ?
>>>>
>>>> For guest: /sys/coco/guest/{tdx,sev}/...
>>>> For host:  /sys/coco/host/{tdx,sev}/...
>>>>
>>>> Maybe it would be better to add the "coco" subdirectory or something like
>>>> that ?
>>>
>>> Hmm, so we can do
>>>
>>> /sys/guest
>>>
>>> and extend
>>>
>>> /sys/hypervisor
>>>
>>> Or we can do what you're suggesting.
>>>
>>> If we do /sys/coco/host, then we'll have two different places to read HV info.
>>>
>>> Or we can stick *everything* coco needs in
>>>
>>> /sys/coco/{sev,tdx}
>>>
>>> but then it is coco-specific and if other guest types want to put stuff in
>>> sysfs, it'll get ugly.
>>>
>>> So I guess having
>>>
>>> /sys/guest
>>> and
>>> /sys/hypervisor
>>>
>>> kinda keeps it all clean, hierarchy-wise...
>>>
>>> Right?
>>
>> Kind of.
>>
>> /sys/hypervisor is meant to provide data for running under a hypervisor.
>>
>> It is NOT meant to provide data for running as a hypervisor.
>>
>> So far when running either under Xen or under z/VM /sys/hypervisor is being
>> populated.
>>
>> I'm not feeling really strong here. I just want to state the status quo.
> 
> OK, so I misunderstood.
> 
> If in the /sys/hypervisor we have information for guest (for running under
> a hypervisor), where do you propose to put the information for the
> host-side (for running as a hypervisor) ?
> 

/sys/kvm/ might be a good fit when using KVM?


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Mon, Mar 10, 2025 at 04:52:37PM +0100, Juergen Gross wrote:
> /sys/kvm/ might be a good fit when using KVM?

What are we going to do when Xen runs coco guests? Or there's no such danger
in the near future?

:-P

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 568 bytes --]

On 10.03.25 16:55, Borislav Petkov wrote:
> On Mon, Mar 10, 2025 at 04:52:37PM +0100, Juergen Gross wrote:
>> /sys/kvm/ might be a good fit when using KVM?
> 
> What are we going to do when Xen runs coco guests? Or there's no such danger
> in the near future?

There is work in progress for SEV guests at least.

Shouldn't the coco related information be the same regardless of the
hypervisor beneath? IOW: do you envision the coding for populating the
coco related sysfs guest nodes to be different when running as a KVM
or a Hyper-V guest?


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Mon, Mar 10, 2025 at 05:00:50PM +0100, Juergen Gross wrote:
> There is work in progress for SEV guests at least.

Nice.
 
> Shouldn't the coco related information be the same regardless of the
> hypervisor beneath?

Right, but...

> IOW: do you envision the coding for populating the coco related sysfs guest
> nodes to be different when running as a KVM or a Hyper-V guest?

... if you have some coco tools which need to read out HV info - for whatever
reason - then they'll have to do:

	if (HV == KVM)
		read /sys/kvm
	else if (HV == Xen)
		read /sys/xen
	else
		...

which we might save them upfront...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 1103 bytes --]

On 10.03.25 17:06, Borislav Petkov wrote:
> On Mon, Mar 10, 2025 at 05:00:50PM +0100, Juergen Gross wrote:
>> There is work in progress for SEV guests at least.
> 
> Nice.
>   
>> Shouldn't the coco related information be the same regardless of the
>> hypervisor beneath?
> 
> Right, but...
> 
>> IOW: do you envision the coding for populating the coco related sysfs guest
>> nodes to be different when running as a KVM or a Hyper-V guest?
> 
> ... if you have some coco tools which need to read out HV info - for whatever
> reason - then they'll have to do:
> 
> 	if (HV == KVM)
> 		read /sys/kvm
> 	else if (HV == Xen)
> 		read /sys/xen
> 	else
> 		...
> 
> which we might save them upfront...

In case there is the need for such information in a guest (and it can only
be in a guest, as every Linux under Xen is a guest, same applies to Hyper-V
and VMWare), the information should be under /sys/hypervisor.

If Linux supports running as a hypervisor using something different than KVM
then we should add something like /sys/virt-platform for such data.


Juergen


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Alexey Gladkov]

On Mon, Mar 10, 2025 at 04:52:37PM +0100, Juergen Gross wrote:
> On 10.03.25 16:43, Alexey Gladkov wrote:
> > On Mon, Mar 10, 2025 at 04:33:08PM +0100, Jürgen Groß wrote:
> >> On 10.03.25 16:11, Borislav Petkov wrote:
> >>> On Mon, Mar 10, 2025 at 03:50:09PM +0100, Alexey Gladkov wrote:
> >>>> Am I understand correctly that you and Joerg are proposing
> >>>>
> >>>> /sys/guest/tdx/...
> >>>> /sys/guest/sev/...
> >>>>
> >>>> ?
> >>>>
> >>>> Which path to use for the host side ?
> >>>>
> >>>> For guest: /sys/coco/guest/{tdx,sev}/...
> >>>> For host:  /sys/coco/host/{tdx,sev}/...
> >>>>
> >>>> Maybe it would be better to add the "coco" subdirectory or something like
> >>>> that ?
> >>>
> >>> Hmm, so we can do
> >>>
> >>> /sys/guest
> >>>
> >>> and extend
> >>>
> >>> /sys/hypervisor
> >>>
> >>> Or we can do what you're suggesting.
> >>>
> >>> If we do /sys/coco/host, then we'll have two different places to read HV info.
> >>>
> >>> Or we can stick *everything* coco needs in
> >>>
> >>> /sys/coco/{sev,tdx}
> >>>
> >>> but then it is coco-specific and if other guest types want to put stuff in
> >>> sysfs, it'll get ugly.
> >>>
> >>> So I guess having
> >>>
> >>> /sys/guest
> >>> and
> >>> /sys/hypervisor
> >>>
> >>> kinda keeps it all clean, hierarchy-wise...
> >>>
> >>> Right?
> >>
> >> Kind of.
> >>
> >> /sys/hypervisor is meant to provide data for running under a hypervisor.
> >>
> >> It is NOT meant to provide data for running as a hypervisor.
> >>
> >> So far when running either under Xen or under z/VM /sys/hypervisor is being
> >> populated.
> >>
> >> I'm not feeling really strong here. I just want to state the status quo.
> > 
> > OK, so I misunderstood.
> > 
> > If in the /sys/hypervisor we have information for guest (for running under
> > a hypervisor), where do you propose to put the information for the
> > host-side (for running as a hypervisor) ?
> > 
> 
> /sys/kvm/ might be a good fit when using KVM?

As far as I can see /sys/hypervisor should contain information about both
host-side and guest-side. If of course we would use the same files as
described in the ABI xen team.

Why do we need /sys/kvm/{tdx,sev}/ ?

I keep mentioning tdx and sev because they will have a different set of
files.

-- 
Rgrds, legion

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Mon, Mar 10, 2025 at 04:43:59PM +0100, Alexey Gladkov wrote:
> If in the /sys/hypervisor we have information for guest (for running under
> a hypervisor), where do you propose to put the information for the
> host-side (for running as a hypervisor) ?

Okay, so let's not mix things up too much here. The only (upstream) case
where Linux _is_ the hypervisor is when running KVM guests. What
information needs to be provided for this case in SYSFS that is not
already provided elsewhere, e.g. by the KVM modules or, in case of SEV,
by /dev/sev? What does Intel expose for TDX?

Back to the guest-side, I agree with introducing a new directory in
SYSFS with sub-directories for each detected hypervisor. To maximise
confusion, it can be called '/sys/hypervisors/', or just /sys/guest/ (as
Boris suggested).

Regards,

-- 
Jörg Rödel
jroedel@suse.de

SUSE Software Solutions Germany GmbH
Frankenstraße 146
90461 Nürnberg
Germany
https://www.suse.com/

Geschäftsführer: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 1157 bytes --]

On 11.03.25 10:43, Joerg Roedel wrote:
> On Mon, Mar 10, 2025 at 04:43:59PM +0100, Alexey Gladkov wrote:
>> If in the /sys/hypervisor we have information for guest (for running under
>> a hypervisor), where do you propose to put the information for the
>> host-side (for running as a hypervisor) ?
> 
> Okay, so let's not mix things up too much here. The only (upstream) case
> where Linux _is_ the hypervisor is when running KVM guests. What
> information needs to be provided for this case in SYSFS that is not
> already provided elsewhere, e.g. by the KVM modules or, in case of SEV,
> by /dev/sev? What does Intel expose for TDX?
> 
> Back to the guest-side, I agree with introducing a new directory in
> SYSFS with sub-directories for each detected hypervisor. To maximise
> confusion, it can be called '/sys/hypervisors/', or just /sys/guest/ (as
> Boris suggested).

I can live with that, as long as we make it possible to make e.g.
/sys/guest/xen a link to /sys/hypervisor (if xen is the hypervisor
the guest is directly running on). This means that /sys/guest/*/type
should not conflict with /sys/hypervisor/type.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Borislav Petkov]

On Tue, Mar 11, 2025 at 11:22:23AM +0100, Jürgen Groß wrote:
> I can live with that, as long as we make it possible to make e.g.
> /sys/guest/xen a link to /sys/hypervisor (if xen is the hypervisor
> the guest is directly running on). This means that /sys/guest/*/type
> should not conflict with /sys/hypervisor/type.

Yeah, so Joerg and I came up with this on IRC:

/sys/hypervisor/{sev,tdx}

* It should not disturb the current hierarchy there

* No need for a new hierarchy like /sys/guest - we haz enough and besides,
/sys/hypervisor sounds like the right place already

* ...

So yeah, I guess we can try this.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 751 bytes --]

On 11.03.25 12:07, Borislav Petkov wrote:
> On Tue, Mar 11, 2025 at 11:22:23AM +0100, Jürgen Groß wrote:
>> I can live with that, as long as we make it possible to make e.g.
>> /sys/guest/xen a link to /sys/hypervisor (if xen is the hypervisor
>> the guest is directly running on). This means that /sys/guest/*/type
>> should not conflict with /sys/hypervisor/type.
> 
> Yeah, so Joerg and I came up with this on IRC:
> 
> /sys/hypervisor/{sev,tdx}
> 
> * It should not disturb the current hierarchy there
> 
> * No need for a new hierarchy like /sys/guest - we haz enough and besides,
> /sys/hypervisor sounds like the right place already
> 
> * ...
> 
> So yeah, I guess we can try this.

Full ack for this plan.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Alexey Gladkov]

On Tue, Mar 11, 2025 at 12:07:48PM +0100, Borislav Petkov wrote:
> On Tue, Mar 11, 2025 at 11:22:23AM +0100, Jürgen Groß wrote:
> > I can live with that, as long as we make it possible to make e.g.
> > /sys/guest/xen a link to /sys/hypervisor (if xen is the hypervisor
> > the guest is directly running on). This means that /sys/guest/*/type
> > should not conflict with /sys/hypervisor/type.
> 
> Yeah, so Joerg and I came up with this on IRC:
> 
> /sys/hypervisor/{sev,tdx}

This directory is for guest-side information, right ?

> 
> * It should not disturb the current hierarchy there
> 
> * No need for a new hierarchy like /sys/guest - we haz enough and besides,
> /sys/hypervisor sounds like the right place already

The /sys/hypervisor is for host-side information ?

> 
> * ...
> 
> So yeah, I guess we can try this.

If the answers to both questions are yes. Then I have understood
everything correctly and I agree with it.

-- 
Rgrds, legion

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Tue, Mar 11, 2025 at 07:24:09PM +0100, Alexey Gladkov wrote:
> On Tue, Mar 11, 2025 at 12:07:48PM +0100, Borislav Petkov wrote:
> > On Tue, Mar 11, 2025 at 11:22:23AM +0100, Jürgen Groß wrote:
> > > I can live with that, as long as we make it possible to make e.g.
> > > /sys/guest/xen a link to /sys/hypervisor (if xen is the hypervisor
> > > the guest is directly running on). This means that /sys/guest/*/type
> > > should not conflict with /sys/hypervisor/type.
> > 
> > Yeah, so Joerg and I came up with this on IRC:
> > 
> > /sys/hypervisor/{sev,tdx}
> 
> This directory is for guest-side information, right ?
> 
> > 
> > * It should not disturb the current hierarchy there
> > 
> > * No need for a new hierarchy like /sys/guest - we haz enough and besides,
> > /sys/hypervisor sounds like the right place already
> 
> The /sys/hypervisor is for host-side information ?

No, all under /sys/hypervisor is for guest-side information. There is
not directory for host-side information yet. The question is whether a
directory in SYSFS is needed, or whether there are other means to expose
the same information.

Regards,

-- 
Jörg Rödel
jroedel@suse.de

SUSE Software Solutions Germany GmbH
Frankenstraße 146
90461 Nürnberg
Germany
https://www.suse.com/

Geschäftsführer: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Alexey Gladkov]

On Tue, Mar 11, 2025 at 07:40:13PM +0100, Joerg Roedel wrote:
> On Tue, Mar 11, 2025 at 07:24:09PM +0100, Alexey Gladkov wrote:
> > On Tue, Mar 11, 2025 at 12:07:48PM +0100, Borislav Petkov wrote:
> > > On Tue, Mar 11, 2025 at 11:22:23AM +0100, Jürgen Groß wrote:
> > > > I can live with that, as long as we make it possible to make e.g.
> > > > /sys/guest/xen a link to /sys/hypervisor (if xen is the hypervisor
> > > > the guest is directly running on). This means that /sys/guest/*/type
> > > > should not conflict with /sys/hypervisor/type.
> > > 
> > > Yeah, so Joerg and I came up with this on IRC:
> > > 
> > > /sys/hypervisor/{sev,tdx}
> > 
> > This directory is for guest-side information, right ?
> > 
> > > 
> > > * It should not disturb the current hierarchy there
> > > 
> > > * No need for a new hierarchy like /sys/guest - we haz enough and besides,
> > > /sys/hypervisor sounds like the right place already
> > 
> > The /sys/hypervisor is for host-side information ?
> 
> No, all under /sys/hypervisor is for guest-side information. There is
> not directory for host-side information yet. The question is whether a
> directory in SYSFS is needed, or whether there are other means to expose
> the same information.

Ok. Now I get it. Agree.

-- 
Rgrds, legion

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Kirill A. Shutemov]

On Tue, Mar 11, 2025 at 07:40:13PM +0100, Joerg Roedel wrote:
> On Tue, Mar 11, 2025 at 07:24:09PM +0100, Alexey Gladkov wrote:
> > On Tue, Mar 11, 2025 at 12:07:48PM +0100, Borislav Petkov wrote:
> > > On Tue, Mar 11, 2025 at 11:22:23AM +0100, Jürgen Groß wrote:
> > > > I can live with that, as long as we make it possible to make e.g.
> > > > /sys/guest/xen a link to /sys/hypervisor (if xen is the hypervisor
> > > > the guest is directly running on). This means that /sys/guest/*/type
> > > > should not conflict with /sys/hypervisor/type.
> > > 
> > > Yeah, so Joerg and I came up with this on IRC:
> > > 
> > > /sys/hypervisor/{sev,tdx}
> > 
> > This directory is for guest-side information, right ?
> > 
> > > 
> > > * It should not disturb the current hierarchy there
> > > 
> > > * No need for a new hierarchy like /sys/guest - we haz enough and besides,
> > > /sys/hypervisor sounds like the right place already
> > 
> > The /sys/hypervisor is for host-side information ?
> 
> No, all under /sys/hypervisor is for guest-side information. There is
> not directory for host-side information yet. The question is whether a
> directory in SYSFS is needed, or whether there are other means to expose
> the same information.

TDX module information (version, supported features, etc) is crucial for
bug reporting. I think it makes sense to present it in sysfs.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Wed, Mar 12, 2025 at 09:19:30AM +0200, Kirill A. Shutemov wrote:
> TDX module information (version, supported features, etc) is crucial for
> bug reporting. I think it makes sense to present it in sysfs.

The SEV side presents this information via dmesg and /dev/sev, we
can also talk about a place in sysfs for it. But the place to present
host-side information is a separate discussion.

Regards,

	Joerg

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Kirill A. Shutemov]

On Wed, Mar 12, 2025 at 09:23:13AM +0100, Joerg Roedel wrote:
> On Wed, Mar 12, 2025 at 09:19:30AM +0200, Kirill A. Shutemov wrote:
> > TDX module information (version, supported features, etc) is crucial for
> > bug reporting. I think it makes sense to present it in sysfs.
> 
> The SEV side presents this information via dmesg and /dev/sev, we
> can also talk about a place in sysfs for it. But the place to present
> host-side information is a separate discussion.

There might be a value to have consistent structure for host and guest
information in sysfs, even if they are presented in different places under
/sys. There's subset of information that is common for both guest and
host, like version.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Wed, Mar 12, 2025 at 10:48:37AM +0200, Kirill A. Shutemov wrote:
> There might be a value to have consistent structure for host and guest
> information in sysfs, even if they are presented in different places under
> /sys. There's subset of information that is common for both guest and
> host, like version.

I agree for the host side, but for the guest side I am less convinced.
Any information exposed via sysfs on the guest side can not be trusted.
The only trusted way to get this information in the guest is a
successfully verified attestation report.

The same is true for exposing SEV_STATUS, btw. This can also only be
trusted together with a verified attestation. But the difference is that
SEV_STATUS is not part of the attestation report.

One might say that this does not matter much for debugging purposes, but
the question stands whether it helps the security posture of the TEE to
expose an untrusted interface which tooling then uses instead of the
trusted variant.

Regards,

	Joerg

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Kirill A. Shutemov]

On Wed, Mar 12, 2025 at 10:07:32AM +0100, Joerg Roedel wrote:
> On Wed, Mar 12, 2025 at 10:48:37AM +0200, Kirill A. Shutemov wrote:
> > There might be a value to have consistent structure for host and guest
> > information in sysfs, even if they are presented in different places under
> > /sys. There's subset of information that is common for both guest and
> > host, like version.
> 
> I agree for the host side, but for the guest side I am less convinced.
> Any information exposed via sysfs on the guest side can not be trusted.
> The only trusted way to get this information in the guest is a
> successfully verified attestation report.
> 
> The same is true for exposing SEV_STATUS, btw. This can also only be
> trusted together with a verified attestation. But the difference is that
> SEV_STATUS is not part of the attestation report.
> 
> One might say that this does not matter much for debugging purposes, but
> the question stands whether it helps the security posture of the TEE to
> expose an untrusted interface which tooling then uses instead of the
> trusted variant.

I am not sure I understand your point.

In TDX case it is as trusted as the kernel itself. If the system is
attested, this info is going to accurate too as kernel gets information
from trusted TDX module.

But nobody suggested to use this information to judge the security of the
system.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Wed, Mar 12, 2025 at 12:59:50PM +0200, Kirill A. Shutemov wrote:
> I am not sure I understand your point.
> 
> In TDX case it is as trusted as the kernel itself. If the system is
> attested, this info is going to accurate too as kernel gets information
> from trusted TDX module.
> 
> But nobody suggested to use this information to judge the security of the
> system.

Version information about the TDX module is required for the security
evaluation at the verifier. The question is whether it makes sense to
expose this information in an untrusted way in the guest (even alongside
a trusted way), or if that makes tooling prefer the untrusted source
because it is easier.

The guest kernel is also only trusted after (runtime) verification.

Regards,

	Joerg

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Alexey Gladkov]

On Tue, Mar 11, 2025 at 10:43:36AM +0100, Joerg Roedel wrote:
> On Mon, Mar 10, 2025 at 04:43:59PM +0100, Alexey Gladkov wrote:
> > If in the /sys/hypervisor we have information for guest (for running under
> > a hypervisor), where do you propose to put the information for the
> > host-side (for running as a hypervisor) ?
> 
> Okay, so let's not mix things up too much here. The only (upstream) case
> where Linux _is_ the hypervisor is when running KVM guests. What
> information needs to be provided for this case in SYSFS that is not
> already provided elsewhere, e.g. by the KVM modules or, in case of SEV,
> by /dev/sev? What does Intel expose for TDX?

Right now tdx does not export any information to userspace (neither
host-side nor guest-side). I want to change that. I want to export
version, attributes and features, maybe something else.

> Back to the guest-side, I agree with introducing a new directory in
> SYSFS with sub-directories for each detected hypervisor. To maximise
> confusion, it can be called '/sys/hypervisors/', or just /sys/guest/ (as
> Boris suggested).
> 
> Regards,
> 
> -- 
> Jörg Rödel
> jroedel@suse.de
> 
> SUSE Software Solutions Germany GmbH
> Frankenstraße 146
> 90461 Nürnberg
> Germany
> https://www.suse.com/
> 
> Geschäftsführer: Ivo Totev, Andrew McDonald, Werner Knoblich
> (HRB 36809, AG Nürnberg)
> 

-- 
Rgrds, legion

Re: [PATCH] x86/sev: Make SEV_STATUS available via SYSFS

[Joerg Roedel]

On Wed, Mar 05, 2025 at 11:52:34AM +0100, Joerg Roedel wrote:
>  arch/x86/coco/sev/core.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

And here are the changes to the snpguest tool to make use of the sysfs
as created with this patch:

	https://github.com/virtee/snpguest/pull/88

I plan to update the PR as this patch changes.

Regards,

-- 
Jörg Rödel
jroedel@suse.de

SUSE Software Solutions Germany GmbH
Frankenstraße 146
90461 Nürnberg
Germany
https://www.suse.com/

Geschäftsführer: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)