| Message ID | 20250311094225.35129-2-sgarzare@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enlightened vTPM support for SVSM on SEV-SNP | expand |
On Tue, Mar 11, 2025 at 10:42:22AM +0100, Stefano Garzarella wrote: > Add two new functions to probe and send commands to the SVSM vTPM. > They leverage the two calls defined by the AMD SVSM specification [1] > for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD. > > Expose these functions to be used by other modules such as a tpm > driver. > > [1] "Secure VM Service Module for SEV-SNP Guests" > Publication # 58019 Revision: 1.00 > > Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com> > Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > v3: > - removed link to the spec because those URLs are unstable [Borislav] > - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch > in this one [Borislav] > - slimmed down snp_svsm_vtpm_probe() [Borislav] > - removed features check and any print related [Tom] > --- > arch/x86/include/asm/sev.h | 7 +++++++ > arch/x86/coco/sev/core.c | 31 +++++++++++++++++++++++++++++++ > 2 files changed, 38 insertions(+) > > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h > index ba7999f66abe..09471d058ce5 100644 > --- a/arch/x86/include/asm/sev.h > +++ b/arch/x86/include/asm/sev.h > @@ -384,6 +384,10 @@ struct svsm_call { > #define SVSM_ATTEST_SERVICES 0 > #define SVSM_ATTEST_SINGLE_SERVICE 1 > > +#define SVSM_VTPM_CALL(x) ((2ULL << 32) | (x)) > +#define SVSM_VTPM_QUERY 0 > +#define SVSM_VTPM_CMD 1 > + > #ifdef CONFIG_AMD_MEM_ENCRYPT > > extern u8 snp_vmpl; > @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc); > int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req, > struct snp_guest_request_ioctl *rio); > > +bool snp_svsm_vtpm_probe(void); > +int snp_svsm_vtpm_send_command(u8 *buffer); > + > void __init snp_secure_tsc_prepare(void); > void __init snp_secure_tsc_init(void); > > diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c > index 96c7bc698e6b..2166bdff88b7 100644 > --- a/arch/x86/coco/sev/core.c > +++ b/arch/x86/coco/sev/core.c > @@ -2628,6 +2628,37 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat > return ret; > } > Since this is an exported symbol, it'd be a good practice document snp_svsm_vtpm_probe(). > +bool snp_svsm_vtpm_probe(void) > +{ > + struct svsm_call call = {}; > + > + /* The vTPM device is available only if a SVSM is present */ > + if (!snp_vmpl) > + return false; > + > + call.caa = svsm_get_caa(); > + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY); I supposed CAA is some kind of shared memory area for host and VM? > + > + if (svsm_perform_call_protocol(&call)) > + return false; > + > + /* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */ > + return (call.rcx_out & BIT_ULL(8)) == BIT_ULL(8); > +} > +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_probe); > + Ditto. > +int snp_svsm_vtpm_send_command(u8 *buffer) > +{ > + struct svsm_call call = {}; > + > + call.caa = svsm_get_caa(); > + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD); > + call.rcx = __pa(buffer); > + > + return svsm_perform_call_protocol(&call); > +} > +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_send_command); > + > static struct platform_device sev_guest_device = { > .name = "sev-guest", > .id = -1, > -- > 2.48.1 > That said, these are rather self-documenting (i.e, nice and clean). BR, Jarkko
On Tue, Mar 11, 2025 at 11:56:23AM +0200, Jarkko Sakkinen wrote: >On Tue, Mar 11, 2025 at 10:42:22AM +0100, Stefano Garzarella wrote: >> Add two new functions to probe and send commands to the SVSM vTPM. >> They leverage the two calls defined by the AMD SVSM specification [1] >> for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD. >> >> Expose these functions to be used by other modules such as a tpm >> driver. >> >> [1] "Secure VM Service Module for SEV-SNP Guests" >> Publication # 58019 Revision: 1.00 >> >> Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com> >> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> >> Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com> >> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> --- >> v3: >> - removed link to the spec because those URLs are unstable [Borislav] >> - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch >> in this one [Borislav] >> - slimmed down snp_svsm_vtpm_probe() [Borislav] >> - removed features check and any print related [Tom] >> --- >> arch/x86/include/asm/sev.h | 7 +++++++ >> arch/x86/coco/sev/core.c | 31 +++++++++++++++++++++++++++++++ >> 2 files changed, 38 insertions(+) >> >> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h >> index ba7999f66abe..09471d058ce5 100644 >> --- a/arch/x86/include/asm/sev.h >> +++ b/arch/x86/include/asm/sev.h >> @@ -384,6 +384,10 @@ struct svsm_call { >> #define SVSM_ATTEST_SERVICES 0 >> #define SVSM_ATTEST_SINGLE_SERVICE 1 >> >> +#define SVSM_VTPM_CALL(x) ((2ULL << 32) | (x)) >> +#define SVSM_VTPM_QUERY 0 >> +#define SVSM_VTPM_CMD 1 >> + >> #ifdef CONFIG_AMD_MEM_ENCRYPT >> >> extern u8 snp_vmpl; >> @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc); >> int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req, >> struct snp_guest_request_ioctl *rio); >> >> +bool snp_svsm_vtpm_probe(void); >> +int snp_svsm_vtpm_send_command(u8 *buffer); >> + >> void __init snp_secure_tsc_prepare(void); >> void __init snp_secure_tsc_init(void); >> >> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c >> index 96c7bc698e6b..2166bdff88b7 100644 >> --- a/arch/x86/coco/sev/core.c >> +++ b/arch/x86/coco/sev/core.c >> @@ -2628,6 +2628,37 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat >> return ret; >> } >> > >Since this is an exported symbol, it'd be a good practice document >snp_svsm_vtpm_probe(). Yes, you are right, since the others were not documented, I had not added it, but I agree with you, I'll do in v4. > >> +bool snp_svsm_vtpm_probe(void) >> +{ >> + struct svsm_call call = {}; >> + >> + /* The vTPM device is available only if a SVSM is present */ >> + if (!snp_vmpl) >> + return false; >> + >> + call.caa = svsm_get_caa(); >> + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY); > >I supposed CAA is some kind of shared memory area for host and VM? Not with the host, but with SVSM, which is the firmware running in the guest, but at a higher privilege level (VMPL) than the kernel, where, for example, the vTPM is emulated. BTW, yep it is a shared memory defined by the SVSM calling convention. From AMD SVSM specification: 5 Calling Convention Each call to the SVSM conveys data through a combination of the SVSM Calling Area (whose address was first configured through the SVSM_CAA field of the secrets page) and registers. Use of the Calling Area is necessary for the SVSM to detect the difference between a call that was issued by the guest and a spurious invocation by a poorly behaved host. Registers are used for all other parameters. The initially configured SVSM Calling Area is a page of memory that lies outside the initial SVSM memory range and has not had its VMPL permissions restricted in any way. The address is guaranteed to be aligned to a 4 KB boundary, so the remainder of the page may be used by the guest for memory-based parameter passing if desired. The contents of the Calling Area are described in the following table: Table 2: Calling Area Byte Size Name Description Offset 0x000 1 byte SVSM_CALL_PENDING Indicates whether a call has been requested by the guest (0=no call requested, 1=call requested). 0x001 1 byte SVSM_MEM_AVAILABLE Free memory is available to be withdrawn. 0x002 6 byte Reserved. The SVSM is not required to verify that these bytes are 0. > >> + >> + if (svsm_perform_call_protocol(&call)) >> + return false; >> + >> + /* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */ >> + return (call.rcx_out & BIT_ULL(8)) == BIT_ULL(8); >> +} >> +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_probe); >> + > >Ditto. Ack. > >> +int snp_svsm_vtpm_send_command(u8 *buffer) >> +{ >> + struct svsm_call call = {}; >> + >> + call.caa = svsm_get_caa(); >> + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD); >> + call.rcx = __pa(buffer); >> + >> + return svsm_perform_call_protocol(&call); >> +} >> +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_send_command); >> + >> static struct platform_device sev_guest_device = { >> .name = "sev-guest", >> .id = -1, >> -- >> 2.48.1 >> > >That said, these are rather self-documenting (i.e, nice and clean). Thanks for the review! Stefano
On Wed, Mar 12, 2025 at 11:56:06AM +0100, Stefano Garzarella wrote: > On Tue, Mar 11, 2025 at 11:56:23AM +0200, Jarkko Sakkinen wrote: > > On Tue, Mar 11, 2025 at 10:42:22AM +0100, Stefano Garzarella wrote: > > > Add two new functions to probe and send commands to the SVSM vTPM. > > > They leverage the two calls defined by the AMD SVSM specification [1] > > > for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD. > > > > > > Expose these functions to be used by other modules such as a tpm > > > driver. > > > > > > [1] "Secure VM Service Module for SEV-SNP Guests" > > > Publication # 58019 Revision: 1.00 > > > > > > Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > > Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com> > > > Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > > --- > > > v3: > > > - removed link to the spec because those URLs are unstable [Borislav] > > > - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch > > > in this one [Borislav] > > > - slimmed down snp_svsm_vtpm_probe() [Borislav] > > > - removed features check and any print related [Tom] > > > --- > > > arch/x86/include/asm/sev.h | 7 +++++++ > > > arch/x86/coco/sev/core.c | 31 +++++++++++++++++++++++++++++++ > > > 2 files changed, 38 insertions(+) > > > > > > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h > > > index ba7999f66abe..09471d058ce5 100644 > > > --- a/arch/x86/include/asm/sev.h > > > +++ b/arch/x86/include/asm/sev.h > > > @@ -384,6 +384,10 @@ struct svsm_call { > > > #define SVSM_ATTEST_SERVICES 0 > > > #define SVSM_ATTEST_SINGLE_SERVICE 1 > > > > > > +#define SVSM_VTPM_CALL(x) ((2ULL << 32) | (x)) > > > +#define SVSM_VTPM_QUERY 0 > > > +#define SVSM_VTPM_CMD 1 > > > + > > > #ifdef CONFIG_AMD_MEM_ENCRYPT > > > > > > extern u8 snp_vmpl; > > > @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc); > > > int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req, > > > struct snp_guest_request_ioctl *rio); > > > > > > +bool snp_svsm_vtpm_probe(void); > > > +int snp_svsm_vtpm_send_command(u8 *buffer); > > > + > > > void __init snp_secure_tsc_prepare(void); > > > void __init snp_secure_tsc_init(void); > > > > > > diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c > > > index 96c7bc698e6b..2166bdff88b7 100644 > > > --- a/arch/x86/coco/sev/core.c > > > +++ b/arch/x86/coco/sev/core.c > > > @@ -2628,6 +2628,37 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat > > > return ret; > > > } > > > > > > > Since this is an exported symbol, it'd be a good practice document > > snp_svsm_vtpm_probe(). > > Yes, you are right, since the others were not documented, I had not added > it, but I agree with you, I'll do in v4. > > > > > > +bool snp_svsm_vtpm_probe(void) > > > +{ > > > + struct svsm_call call = {}; > > > + > > > + /* The vTPM device is available only if a SVSM is present */ > > > + if (!snp_vmpl) > > > + return false; > > > + > > > + call.caa = svsm_get_caa(); > > > + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY); > > > > I supposed CAA is some kind of shared memory area for host and VM? > > Not with the host, but with SVSM, which is the firmware running in the > guest, but at a higher privilege level (VMPL) than the kernel, where, for > example, the vTPM is emulated. > > BTW, yep it is a shared memory defined by the SVSM calling convention. > From AMD SVSM specification: > > 5 Calling Convention > > Each call to the SVSM conveys data through a combination of the > SVSM Calling Area (whose address was first configured through the > SVSM_CAA field of the secrets page) and registers. Use of the > Calling Area is necessary for the SVSM to detect the difference > between a call that was issued by the guest and a spurious > invocation by a poorly behaved host. Registers are used for all > other parameters. > The initially configured SVSM Calling Area is a page of memory that > lies outside the initial SVSM memory range and has not had its VMPL > permissions restricted in any way. The address is guaranteed to be > aligned to a 4 KB boundary, so the remainder of the page may be used > by the guest for memory-based parameter passing if desired. > The contents of the Calling Area are described in the following > table: > > Table 2: Calling Area > Byte Size Name Description > Offset > > 0x000 1 byte SVSM_CALL_PENDING Indicates whether a call has > been requested by the guest > (0=no call requested, 1=call > requested). > 0x001 1 byte SVSM_MEM_AVAILABLE Free memory is available to > be withdrawn. > 0x002 6 byte Reserved. The SVSM is not > required to verify that > these bytes are 0. > > > > > > + > > > + if (svsm_perform_call_protocol(&call)) > > > + return false; > > > + > > > + /* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */ > > > + return (call.rcx_out & BIT_ULL(8)) == BIT_ULL(8); > > > +} > > > +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_probe); > > > + > > > > Ditto. > > Ack. > > > > > > +int snp_svsm_vtpm_send_command(u8 *buffer) > > > +{ > > > + struct svsm_call call = {}; > > > + > > > + call.caa = svsm_get_caa(); > > > + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD); > > > + call.rcx = __pa(buffer); > > > + > > > + return svsm_perform_call_protocol(&call); > > > +} > > > +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_send_command); > > > + > > > static struct platform_device sev_guest_device = { > > > .name = "sev-guest", > > > .id = -1, > > > -- > > > 2.48.1 > > > > > > > That said, these are rather self-documenting (i.e, nice and clean). > > Thanks for the review! Sure, don't worry about it! Let's just cycle this enough rounds that it fits well... > Stefano BR, Jarkko
On 3/11/25 04:42, Stefano Garzarella wrote: > Add two new functions to probe and send commands to the SVSM vTPM. > They leverage the two calls defined by the AMD SVSM specification [1] > for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD. > > Expose these functions to be used by other modules such as a tpm > driver. > > [1] "Secure VM Service Module for SEV-SNP Guests" > Publication # 58019 Revision: 1.00 > > Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com> > Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> One minor nit below, otherwise: Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > v3: > - removed link to the spec because those URLs are unstable [Borislav] > - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch > in this one [Borislav] > - slimmed down snp_svsm_vtpm_probe() [Borislav] > - removed features check and any print related [Tom] > --- > arch/x86/include/asm/sev.h | 7 +++++++ > arch/x86/coco/sev/core.c | 31 +++++++++++++++++++++++++++++++ > 2 files changed, 38 insertions(+) > > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h > index ba7999f66abe..09471d058ce5 100644 > --- a/arch/x86/include/asm/sev.h > +++ b/arch/x86/include/asm/sev.h > @@ -384,6 +384,10 @@ struct svsm_call { > #define SVSM_ATTEST_SERVICES 0 > #define SVSM_ATTEST_SINGLE_SERVICE 1 > > +#define SVSM_VTPM_CALL(x) ((2ULL << 32) | (x)) > +#define SVSM_VTPM_QUERY 0 > +#define SVSM_VTPM_CMD 1 > + > #ifdef CONFIG_AMD_MEM_ENCRYPT > > extern u8 snp_vmpl; > @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc); > int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req, > struct snp_guest_request_ioctl *rio); > > +bool snp_svsm_vtpm_probe(void); > +int snp_svsm_vtpm_send_command(u8 *buffer); > + > void __init snp_secure_tsc_prepare(void); > void __init snp_secure_tsc_init(void); > > diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c > index 96c7bc698e6b..2166bdff88b7 100644 > --- a/arch/x86/coco/sev/core.c > +++ b/arch/x86/coco/sev/core.c > @@ -2628,6 +2628,37 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat > return ret; > } > > +bool snp_svsm_vtpm_probe(void) > +{ > + struct svsm_call call = {}; > + > + /* The vTPM device is available only if a SVSM is present */ > + if (!snp_vmpl) > + return false; > + > + call.caa = svsm_get_caa(); > + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY); > + > + if (svsm_perform_call_protocol(&call)) > + return false; > + > + /* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */ > + return (call.rcx_out & BIT_ULL(8)) == BIT_ULL(8); It's a bool function, so this could simplified to just: return call.rcx_out & BIT_ULL(8); Thanks, Tom > +} > +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_probe); > + > +int snp_svsm_vtpm_send_command(u8 *buffer) > +{ > + struct svsm_call call = {}; > + > + call.caa = svsm_get_caa(); > + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD); > + call.rcx = __pa(buffer); > + > + return svsm_perform_call_protocol(&call); > +} > +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_send_command); > + > static struct platform_device sev_guest_device = { > .name = "sev-guest", > .id = -1,
On Fri, Mar 14, 2025 at 10:27:07AM -0500, Tom Lendacky wrote: > On 3/11/25 04:42, Stefano Garzarella wrote: > > Add two new functions to probe and send commands to the SVSM vTPM. > > They leverage the two calls defined by the AMD SVSM specification [1] > > for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD. > > > > Expose these functions to be used by other modules such as a tpm > > driver. > > > > [1] "Secure VM Service Module for SEV-SNP Guests" > > Publication # 58019 Revision: 1.00 > > > > Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com> > > Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > One minor nit below, otherwise: > > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> > > > --- > > v3: > > - removed link to the spec because those URLs are unstable [Borislav] > > - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch > > in this one [Borislav] > > - slimmed down snp_svsm_vtpm_probe() [Borislav] > > - removed features check and any print related [Tom] > > --- > > arch/x86/include/asm/sev.h | 7 +++++++ > > arch/x86/coco/sev/core.c | 31 +++++++++++++++++++++++++++++++ > > 2 files changed, 38 insertions(+) > > > > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h > > index ba7999f66abe..09471d058ce5 100644 > > --- a/arch/x86/include/asm/sev.h > > +++ b/arch/x86/include/asm/sev.h > > @@ -384,6 +384,10 @@ struct svsm_call { > > #define SVSM_ATTEST_SERVICES 0 > > #define SVSM_ATTEST_SINGLE_SERVICE 1 > > > > +#define SVSM_VTPM_CALL(x) ((2ULL << 32) | (x)) > > +#define SVSM_VTPM_QUERY 0 > > +#define SVSM_VTPM_CMD 1 > > + > > #ifdef CONFIG_AMD_MEM_ENCRYPT > > > > extern u8 snp_vmpl; > > @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc); > > int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req, > > struct snp_guest_request_ioctl *rio); > > > > +bool snp_svsm_vtpm_probe(void); > > +int snp_svsm_vtpm_send_command(u8 *buffer); > > + > > void __init snp_secure_tsc_prepare(void); > > void __init snp_secure_tsc_init(void); > > > > diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c > > index 96c7bc698e6b..2166bdff88b7 100644 > > --- a/arch/x86/coco/sev/core.c > > +++ b/arch/x86/coco/sev/core.c > > @@ -2628,6 +2628,37 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat > > return ret; > > } > > > > +bool snp_svsm_vtpm_probe(void) > > +{ > > + struct svsm_call call = {}; > > + > > + /* The vTPM device is available only if a SVSM is present */ > > + if (!snp_vmpl) > > + return false; > > + > > + call.caa = svsm_get_caa(); > > + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY); > > + > > + if (svsm_perform_call_protocol(&call)) > > + return false; > > + > > + /* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */ > > + return (call.rcx_out & BIT_ULL(8)) == BIT_ULL(8); > > It's a bool function, so this could simplified to just: > > return call.rcx_out & BIT_ULL(8); Or perhaps even just "call.rcx_out & 0x100". I don't think BIT_ULL() here brings much additional clarity or anything useful... > > Thanks, > Tom BR, Jarkko
On Mon, Mar 17, 2025 at 03:36:26PM +0200, Jarkko Sakkinen wrote: >On Fri, Mar 14, 2025 at 10:27:07AM -0500, Tom Lendacky wrote: >> On 3/11/25 04:42, Stefano Garzarella wrote: >> > Add two new functions to probe and send commands to the SVSM vTPM. >> > They leverage the two calls defined by the AMD SVSM specification [1] >> > for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD. >> > >> > Expose these functions to be used by other modules such as a tpm >> > driver. >> > >> > [1] "Secure VM Service Module for SEV-SNP Guests" >> > Publication # 58019 Revision: 1.00 >> > >> > Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com> >> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> >> > Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com> >> > Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> >> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> >> One minor nit below, otherwise: >> >> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> Thanks! >> >> > --- >> > v3: >> > - removed link to the spec because those URLs are unstable [Borislav] >> > - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch >> > in this one [Borislav] >> > - slimmed down snp_svsm_vtpm_probe() [Borislav] >> > - removed features check and any print related [Tom] >> > --- >> > arch/x86/include/asm/sev.h | 7 +++++++ >> > arch/x86/coco/sev/core.c | 31 +++++++++++++++++++++++++++++++ >> > 2 files changed, 38 insertions(+) >> > >> > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h >> > index ba7999f66abe..09471d058ce5 100644 >> > --- a/arch/x86/include/asm/sev.h >> > +++ b/arch/x86/include/asm/sev.h >> > @@ -384,6 +384,10 @@ struct svsm_call { >> > #define SVSM_ATTEST_SERVICES 0 >> > #define SVSM_ATTEST_SINGLE_SERVICE 1 >> > >> > +#define SVSM_VTPM_CALL(x) ((2ULL << 32) | (x)) >> > +#define SVSM_VTPM_QUERY 0 >> > +#define SVSM_VTPM_CMD 1 >> > + >> > #ifdef CONFIG_AMD_MEM_ENCRYPT >> > >> > extern u8 snp_vmpl; >> > @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc); >> > int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req, >> > struct snp_guest_request_ioctl *rio); >> > >> > +bool snp_svsm_vtpm_probe(void); >> > +int snp_svsm_vtpm_send_command(u8 *buffer); >> > + >> > void __init snp_secure_tsc_prepare(void); >> > void __init snp_secure_tsc_init(void); >> > >> > diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c >> > index 96c7bc698e6b..2166bdff88b7 100644 >> > --- a/arch/x86/coco/sev/core.c >> > +++ b/arch/x86/coco/sev/core.c >> > @@ -2628,6 +2628,37 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat >> > return ret; >> > } >> > >> > +bool snp_svsm_vtpm_probe(void) >> > +{ >> > + struct svsm_call call = {}; >> > + >> > + /* The vTPM device is available only if a SVSM is present */ >> > + if (!snp_vmpl) >> > + return false; >> > + >> > + call.caa = svsm_get_caa(); >> > + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY); >> > + >> > + if (svsm_perform_call_protocol(&call)) >> > + return false; >> > + >> > + /* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */ >> > + return (call.rcx_out & BIT_ULL(8)) == BIT_ULL(8); >> >> It's a bool function, so this could simplified to just: >> >> return call.rcx_out & BIT_ULL(8); Sure. > >Or perhaps even just "call.rcx_out & 0x100". I don't think BIT_ULL() >here brings much additional clarity or anything useful... I can do that, I slightly prefer BIT_ULL() macro, but I don't have a strong opinion on my side. @Borislav since you suggested it, WDYT? Thanks, Stefano
On Tue, Mar 18, 2025 at 11:07:57AM +0100, Stefano Garzarella wrote: > On Mon, Mar 17, 2025 at 03:36:26PM +0200, Jarkko Sakkinen wrote: > > On Fri, Mar 14, 2025 at 10:27:07AM -0500, Tom Lendacky wrote: > > > On 3/11/25 04:42, Stefano Garzarella wrote: > > > > Add two new functions to probe and send commands to the SVSM vTPM. > > > > They leverage the two calls defined by the AMD SVSM specification [1] > > > > for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD. > > > > > > > > Expose these functions to be used by other modules such as a tpm > > > > driver. > > > > > > > > [1] "Secure VM Service Module for SEV-SNP Guests" > > > > Publication # 58019 Revision: 1.00 > > > > > > > > Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > > > Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com> > > > > Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> > > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > > > > > One minor nit below, otherwise: > > > > > > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> > > Thanks! > > > > > > > > --- > > > > v3: > > > > - removed link to the spec because those URLs are unstable [Borislav] > > > > - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch > > > > in this one [Borislav] > > > > - slimmed down snp_svsm_vtpm_probe() [Borislav] > > > > - removed features check and any print related [Tom] > > > > --- > > > > arch/x86/include/asm/sev.h | 7 +++++++ > > > > arch/x86/coco/sev/core.c | 31 +++++++++++++++++++++++++++++++ > > > > 2 files changed, 38 insertions(+) > > > > > > > > diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h > > > > index ba7999f66abe..09471d058ce5 100644 > > > > --- a/arch/x86/include/asm/sev.h > > > > +++ b/arch/x86/include/asm/sev.h > > > > @@ -384,6 +384,10 @@ struct svsm_call { > > > > #define SVSM_ATTEST_SERVICES 0 > > > > #define SVSM_ATTEST_SINGLE_SERVICE 1 > > > > > > > > +#define SVSM_VTPM_CALL(x) ((2ULL << 32) | (x)) > > > > +#define SVSM_VTPM_QUERY 0 > > > > +#define SVSM_VTPM_CMD 1 > > > > + > > > > #ifdef CONFIG_AMD_MEM_ENCRYPT > > > > > > > > extern u8 snp_vmpl; > > > > @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc); > > > > int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req, > > > > struct snp_guest_request_ioctl *rio); > > > > > > > > +bool snp_svsm_vtpm_probe(void); > > > > +int snp_svsm_vtpm_send_command(u8 *buffer); > > > > + > > > > void __init snp_secure_tsc_prepare(void); > > > > void __init snp_secure_tsc_init(void); > > > > > > > > diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c > > > > index 96c7bc698e6b..2166bdff88b7 100644 > > > > --- a/arch/x86/coco/sev/core.c > > > > +++ b/arch/x86/coco/sev/core.c > > > > @@ -2628,6 +2628,37 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat > > > > return ret; > > > > } > > > > > > > > +bool snp_svsm_vtpm_probe(void) > > > > +{ > > > > + struct svsm_call call = {}; > > > > + > > > > + /* The vTPM device is available only if a SVSM is present */ > > > > + if (!snp_vmpl) > > > > + return false; > > > > + > > > > + call.caa = svsm_get_caa(); > > > > + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY); > > > > + > > > > + if (svsm_perform_call_protocol(&call)) > > > > + return false; > > > > + > > > > + /* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */ > > > > + return (call.rcx_out & BIT_ULL(8)) == BIT_ULL(8); > > > > > > It's a bool function, so this could simplified to just: > > > > > > return call.rcx_out & BIT_ULL(8); > > Sure. > > > > > Or perhaps even just "call.rcx_out & 0x100". I don't think BIT_ULL() > > here brings much additional clarity or anything useful... > > I can do that, I slightly prefer BIT_ULL() macro, but I don't have a strong > opinion on my side. > @Borislav since you suggested it, WDYT? Either goes for me. Sorry for nitpicking that :-) The first comment stil applies. > > Thanks, > Stefano > BR, Jarkko
On Thu, Mar 20, 2025 at 05:03:09PM +0200, Jarkko Sakkinen wrote: > > I can do that, I slightly prefer BIT_ULL() macro, but I don't have a strong > > opinion on my side. > > @Borislav since you suggested it, WDYT? > > Either goes for me. Sorry for nitpicking that :-) The first comment > stil applies. Bit 8 is a lot better than 0x100. Let's give a better example: 0x0000000008000000 or BIT_ULL(27) :-) While I'm here: I'm guessing I'll route patches 1 and 4 through tip once they're ready to go and give Jarkko an immutable branch he can base the other two ontop. Agreed? Thx.
On Thu, Mar 20, 2025 at 06:16:19PM +0100, Borislav Petkov wrote: > On Thu, Mar 20, 2025 at 05:03:09PM +0200, Jarkko Sakkinen wrote: > > > I can do that, I slightly prefer BIT_ULL() macro, but I don't have a strong > > > opinion on my side. > > > @Borislav since you suggested it, WDYT? > > > > Either goes for me. Sorry for nitpicking that :-) The first comment > > stil applies. > > Bit 8 is a lot better than 0x100. > > Let's give a better example: > > 0x0000000008000000 > > or > > BIT_ULL(27) > > :-) Sure, I'm fine with using BIT_ULL() :-) > > While I'm here: I'm guessing I'll route patches 1 and 4 through tip once > they're ready to go and give Jarkko an immutable branch he can base the other > two ontop. > > Agreed? Works for me. > > Thx. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette BR, Jarkko
On Thu, Mar 20, 2025 at 07:30:43PM +0200, Jarkko Sakkinen wrote: >On Thu, Mar 20, 2025 at 06:16:19PM +0100, Borislav Petkov wrote: >> On Thu, Mar 20, 2025 at 05:03:09PM +0200, Jarkko Sakkinen wrote: >> > > I can do that, I slightly prefer BIT_ULL() macro, but I don't have a strong >> > > opinion on my side. >> > > @Borislav since you suggested it, WDYT? >> > >> > Either goes for me. Sorry for nitpicking that :-) The first comment >> > stil applies. >> >> Bit 8 is a lot better than 0x100. >> >> Let's give a better example: >> >> 0x0000000008000000 >> >> or >> >> BIT_ULL(27) >> >> :-) > >Sure, I'm fine with using BIT_ULL() :-) Yeah, we all agree :-) > >> >> While I'm here: I'm guessing I'll route patches 1 and 4 through tip once >> they're ready to go and give Jarkko an immutable branch he can base the other >> two ontop. >> >> Agreed? > >Works for me. Just a note, patch 2 adds `include/linux/svsm_vtpm.h`, that file is basically a translation of the AMD SVSM specification into structures and functions used to communicate with SVSM in the way it is defined by the specification. I realized that the file does not fall under any section of MAINTAINERS. How do you suggest we proceed? Should we create an SVSM section to maintain it, including the TPM driver and future other drivers,etc.? Or include it in other sections? Which one in this case? I'm willing to help both as a sub-maintainer and reviewer of course, but I would like your advice. Thanks, Stefano
On Fri, Mar 21, 2025 at 10:01:17AM +0100, Stefano Garzarella wrote: > Just a note, patch 2 adds `include/linux/svsm_vtpm.h`, that file is > basically a translation of the AMD SVSM specification into structures and > functions used to communicate with SVSM in the way it is defined by the > specification. > > I realized that the file does not fall under any section of MAINTAINERS. > How do you suggest we proceed? > > Should we create an SVSM section to maintain it, including the TPM driver > and future other drivers,etc.? This all belongs to the TPM drivers, right? I.e., drivers/char/tpm/ So I guess add that header to the TPM DEVICE DRIVER section if the gents there are fine with it...
On Fri, Mar 21, 2025 at 11:05:20PM +0100, Borislav Petkov wrote: > On Fri, Mar 21, 2025 at 10:01:17AM +0100, Stefano Garzarella wrote: > > Just a note, patch 2 adds `include/linux/svsm_vtpm.h`, that file is > > basically a translation of the AMD SVSM specification into structures and > > functions used to communicate with SVSM in the way it is defined by the > > specification. > > > > I realized that the file does not fall under any section of MAINTAINERS. > > How do you suggest we proceed? > > > > Should we create an SVSM section to maintain it, including the TPM driver > > and future other drivers,etc.? > > This all belongs to the TPM drivers, right? > > I.e., drivers/char/tpm/ > > So I guess add that header to the TPM DEVICE DRIVER section if the gents there > are fine with it... It's fine for me but I'd suggest to rename the header as "tpm_svsm.h". Then this will already provide coverage: https://web.git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=a2fbcecc7027944a2ce447d4dd72725c5822321f > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette > BR, Jarkko
On Sat, Mar 22, 2025 at 10:17:03PM +0200, Jarkko Sakkinen wrote: >On Fri, Mar 21, 2025 at 11:05:20PM +0100, Borislav Petkov wrote: >> On Fri, Mar 21, 2025 at 10:01:17AM +0100, Stefano Garzarella wrote: >> > Just a note, patch 2 adds `include/linux/svsm_vtpm.h`, that file is >> > basically a translation of the AMD SVSM specification into structures and >> > functions used to communicate with SVSM in the way it is defined by the >> > specification. >> > >> > I realized that the file does not fall under any section of MAINTAINERS. >> > How do you suggest we proceed? >> > >> > Should we create an SVSM section to maintain it, including the TPM driver >> > and future other drivers,etc.? >> >> This all belongs to the TPM drivers, right? For now yes, we may have other devices in the future, but we can think about that later. >> >> I.e., drivers/char/tpm/ >> >> So I guess add that header to the TPM DEVICE DRIVER section if the gents there >> are fine with it... > >It's fine for me but I'd suggest to rename the header as "tpm_svsm.h". >Then this will already provide coverage: > >https://web.git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?id=a2fbcecc7027944a2ce447d4dd72725c5822321f > Great, I'll rename it and send v4. Thanks, Stefano
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h index ba7999f66abe..09471d058ce5 100644 --- a/arch/x86/include/asm/sev.h +++ b/arch/x86/include/asm/sev.h @@ -384,6 +384,10 @@ struct svsm_call { #define SVSM_ATTEST_SERVICES 0 #define SVSM_ATTEST_SINGLE_SERVICE 1 +#define SVSM_VTPM_CALL(x) ((2ULL << 32) | (x)) +#define SVSM_VTPM_QUERY 0 +#define SVSM_VTPM_CMD 1 + #ifdef CONFIG_AMD_MEM_ENCRYPT extern u8 snp_vmpl; @@ -481,6 +485,9 @@ void snp_msg_free(struct snp_msg_desc *mdesc); int snp_send_guest_request(struct snp_msg_desc *mdesc, struct snp_guest_req *req, struct snp_guest_request_ioctl *rio); +bool snp_svsm_vtpm_probe(void); +int snp_svsm_vtpm_send_command(u8 *buffer); + void __init snp_secure_tsc_prepare(void); void __init snp_secure_tsc_init(void); diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c index 96c7bc698e6b..2166bdff88b7 100644 --- a/arch/x86/coco/sev/core.c +++ b/arch/x86/coco/sev/core.c @@ -2628,6 +2628,37 @@ static int snp_issue_guest_request(struct snp_guest_req *req, struct snp_req_dat return ret; } +bool snp_svsm_vtpm_probe(void) +{ + struct svsm_call call = {}; + + /* The vTPM device is available only if a SVSM is present */ + if (!snp_vmpl) + return false; + + call.caa = svsm_get_caa(); + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY); + + if (svsm_perform_call_protocol(&call)) + return false; + + /* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */ + return (call.rcx_out & BIT_ULL(8)) == BIT_ULL(8); +} +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_probe); + +int snp_svsm_vtpm_send_command(u8 *buffer) +{ + struct svsm_call call = {}; + + call.caa = svsm_get_caa(); + call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD); + call.rcx = __pa(buffer); + + return svsm_perform_call_protocol(&call); +} +EXPORT_SYMBOL_GPL(snp_svsm_vtpm_send_command); + static struct platform_device sev_guest_device = { .name = "sev-guest", .id = -1,
Add two new functions to probe and send commands to the SVSM vTPM. They leverage the two calls defined by the AMD SVSM specification [1] for the vTPM protocol: SVSM_VTPM_QUERY and SVSM_VTPM_CMD. Expose these functions to be used by other modules such as a tpm driver. [1] "Secure VM Service Module for SEV-SNP Guests" Publication # 58019 Revision: 1.00 Co-developed-by: James Bottomley <James.Bottomley@HansenPartnership.com> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> Co-developed-by: Claudio Carvalho <cclaudio@linux.ibm.com> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- v3: - removed link to the spec because those URLs are unstable [Borislav] - squashed "x86/sev: add SVSM call macros for the vTPM protocol" patch in this one [Borislav] - slimmed down snp_svsm_vtpm_probe() [Borislav] - removed features check and any print related [Tom] --- arch/x86/include/asm/sev.h | 7 +++++++ arch/x86/coco/sev/core.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+)