[PATCH] sev-snp: parse MP tables for VMware hypervisor

[PATCH] sev-snp: parse MP tables for VMware hypervisor

[Ajay Kaher]

For VMware hypervisor, SEV-SNP enabled VM's could boot without UEFI.
In this case, mpparse_find_mptable() has to be called to parse MP
tables which contains boot information.

Fixes: 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for SEV-SNP guests")
Signed-off-by: Ajay Kaher <ajay.kaher@broadcom.com>
Signed-off-by: Ye Li <ye.li@broadcom.com>
Tested-by: Ye Li <ye.li@broadcom.com>
---
 arch/x86/kernel/cpu/vmware.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 00189cd..3e2594d 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -26,6 +26,7 @@
 #include <linux/export.h>
 #include <linux/clocksource.h>
 #include <linux/cpu.h>
+#include <linux/efi.h>
 #include <linux/reboot.h>
 #include <linux/static_call.h>
 #include <asm/div64.h>
@@ -35,6 +36,8 @@
 #include <asm/apic.h>
 #include <asm/vmware.h>
 #include <asm/svm.h>
+#include <asm/mem_encrypt.h>
+#include <asm/efi.h>
 
 #undef pr_fmt
 #define pr_fmt(fmt)	"vmware: " fmt
@@ -429,6 +432,10 @@ static void __init vmware_platform_setup(void)
 		pr_warn("Failed to get TSC freq from the hypervisor\n");
 	}
 
+	if (sev_status & MSR_AMD64_SEV_SNP_ENABLED &&
+	    !efi_enabled(EFI_BOOT))
+		x86_init.mpparse.find_mptable = mpparse_find_mptable;
+
 	vmware_paravirt_ops_setup();
 
 #ifdef CONFIG_X86_IO_APIC
-- 
2.39.4

Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor

[Kevin Loughlin]

On Thu, Dec 19, 2024 at 6:44 AM Ajay Kaher <ajay.kaher@broadcom.com> wrote:
>
> For VMware hypervisor, SEV-SNP enabled VM's could boot without UEFI.
> In this case, mpparse_find_mptable() has to be called to parse MP
> tables which contains boot information.
>
> Fixes: 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for SEV-SNP guests")
> Signed-off-by: Ajay Kaher <ajay.kaher@broadcom.com>
> Signed-off-by: Ye Li <ye.li@broadcom.com>
> Tested-by: Ye Li <ye.li@broadcom.com>
> ---
>  arch/x86/kernel/cpu/vmware.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index 00189cd..3e2594d 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -26,6 +26,7 @@
>  #include <linux/export.h>
>  #include <linux/clocksource.h>
>  #include <linux/cpu.h>
> +#include <linux/efi.h>
>  #include <linux/reboot.h>
>  #include <linux/static_call.h>
>  #include <asm/div64.h>
> @@ -35,6 +36,8 @@
>  #include <asm/apic.h>
>  #include <asm/vmware.h>
>  #include <asm/svm.h>
> +#include <asm/mem_encrypt.h>
> +#include <asm/efi.h>
>
>  #undef pr_fmt
>  #define pr_fmt(fmt)    "vmware: " fmt
> @@ -429,6 +432,10 @@ static void __init vmware_platform_setup(void)
>                 pr_warn("Failed to get TSC freq from the hypervisor\n");
>         }
>
> +       if (sev_status & MSR_AMD64_SEV_SNP_ENABLED &&
> +           !efi_enabled(EFI_BOOT))
> +               x86_init.mpparse.find_mptable = mpparse_find_mptable;

As far I know, Linux itself currently doesn't PVALIDATE the address
ranges scanned in mpparse_find_mptable(), and Linux accesses these
addresses as encrypted during early boot. Given this, am I correct
that the guest firmware that you're using is doing the PVALIDATE of
these ranges before starting Linux (else there would be a crash upon
scan)? And then presumably the firmware is also making sure that this
memory is encrypted so that Linux isn't reading unencrypted data as
encrypted (i.e., garbage)?

If so, does that mean all the ROM region scans removed in [0] are
permissible for SEV-SNP guests booting on whichever guest firmware
this is? But you only want/need the mptable info here?

[0] 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for
SEV-SNP guests")

More broadly, ideally the guest firmware would communicate to Linux
that these ranges are safe to access (perhaps via the e820 table),
rather than Linux making the assumption that the ranges are safe for
non-EFI SEV-SNP guest boots in this scenario. However, since you're
only changing vmware_platform_setup() for such boots, I don't think we
need to hold up this patch on that generality.

Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor

[Ajay Kaher]

[-- Attachment #1: Type: text/plain, Size: 3523 bytes --]

On Thu, Dec 26, 2024 at 9:26 PM Kevin Loughlin <kevinloughlin@google.com> wrote:
>
> On Thu, Dec 19, 2024 at 6:44 AM Ajay Kaher <ajay.kaher@broadcom.com> wrote:
> >
> > For VMware hypervisor, SEV-SNP enabled VM's could boot without UEFI.
> > In this case, mpparse_find_mptable() has to be called to parse MP
> > tables which contains boot information.
> >
> > Fixes: 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for SEV-SNP guests")
> > Signed-off-by: Ajay Kaher <ajay.kaher@broadcom.com>
> > Signed-off-by: Ye Li <ye.li@broadcom.com>
> > Tested-by: Ye Li <ye.li@broadcom.com>
> > ---
> >  arch/x86/kernel/cpu/vmware.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> > index 00189cd..3e2594d 100644
> > --- a/arch/x86/kernel/cpu/vmware.c
> > +++ b/arch/x86/kernel/cpu/vmware.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/export.h>
> >  #include <linux/clocksource.h>
> >  #include <linux/cpu.h>
> > +#include <linux/efi.h>
> >  #include <linux/reboot.h>
> >  #include <linux/static_call.h>
> >  #include <asm/div64.h>
> > @@ -35,6 +36,8 @@
> >  #include <asm/apic.h>
> >  #include <asm/vmware.h>
> >  #include <asm/svm.h>
> > +#include <asm/mem_encrypt.h>
> > +#include <asm/efi.h>
> >
> >  #undef pr_fmt
> >  #define pr_fmt(fmt)    "vmware: " fmt
> > @@ -429,6 +432,10 @@ static void __init vmware_platform_setup(void)
> >                 pr_warn("Failed to get TSC freq from the hypervisor\n");
> >         }
> >
> > +       if (sev_status & MSR_AMD64_SEV_SNP_ENABLED &&
> > +           !efi_enabled(EFI_BOOT))
> > +               x86_init.mpparse.find_mptable = mpparse_find_mptable;
>
> As far I know, Linux itself currently doesn't PVALIDATE the address
> ranges scanned in mpparse_find_mptable(), and Linux accesses these
> addresses as encrypted during early boot. Given this, am I correct
> that the guest firmware that you're using is doing the PVALIDATE of
> these ranges before starting Linux (else there would be a crash upon
> scan)? And then presumably the firmware is also making sure that this
> memory is encrypted so that Linux isn't reading unencrypted data as
> encrypted (i.e., garbage)?

Yes, Linux (as a guest) receives valid data.

> If so, does that mean all the ROM region scans removed in [0] are
> permissible for SEV-SNP guests booting on whichever guest firmware
> this is? But you only want/need the mptable info here?

Yes, VMware firmware validates MPTABLE info when 'non-EFI guest boots
on VMware Hypervisor'. The other ROM regions removed in [0] were not
encrypted and validated. This is a specific use case for the VMware platform.
Linux today assumes SNP VMs will be booted with UEFI.

> [0] 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for
> SEV-SNP guests")
>
> More broadly, ideally the guest firmware would communicate to Linux
> that these ranges are safe to access (perhaps via the e820 table),
> rather than Linux making the assumption that the ranges are safe for
> non-EFI SEV-SNP guest boots in this scenario. However, since you're
> only changing vmware_platform_setup() for such boots, I don't think we
> need to hold up this patch on that generality.

This is a VMware specific scenario. VMware Hypervisor makes the MPTABLE
memory available in e820 when 'non-EFI guest boots on VMware Hypervisor'.
So it makes sense to do the changes only for vmware_platform_setup().

- Ajay

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5427 bytes --]

Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor

[Ajay Kaher]

[-- Attachment #1: Type: text/plain, Size: 3756 bytes --]

On Fri, Jan 3, 2025 at 10:01 AM Ajay Kaher <ajay.kaher@broadcom.com> wrote:
> > > For VMware hypervisor, SEV-SNP enabled VM's could boot without UEFI.
> > > In this case, mpparse_find_mptable() has to be called to parse MP
> > > tables which contains boot information.
> > >
> > > Fixes: 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for SEV-SNP guests")
> > > Signed-off-by: Ajay Kaher <ajay.kaher@broadcom.com>
> > > Signed-off-by: Ye Li <ye.li@broadcom.com>
> > > Tested-by: Ye Li <ye.li@broadcom.com>
> > > ---
> > >  arch/x86/kernel/cpu/vmware.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> > > index 00189cd..3e2594d 100644
> > > --- a/arch/x86/kernel/cpu/vmware.c
> > > +++ b/arch/x86/kernel/cpu/vmware.c
> > > @@ -26,6 +26,7 @@
> > >  #include <linux/export.h>
> > >  #include <linux/clocksource.h>
> > >  #include <linux/cpu.h>
> > > +#include <linux/efi.h>
> > >  #include <linux/reboot.h>
> > >  #include <linux/static_call.h>
> > >  #include <asm/div64.h>
> > > @@ -35,6 +36,8 @@
> > >  #include <asm/apic.h>
> > >  #include <asm/vmware.h>
> > >  #include <asm/svm.h>
> > > +#include <asm/mem_encrypt.h>
> > > +#include <asm/efi.h>
> > >
> > >  #undef pr_fmt
> > >  #define pr_fmt(fmt)    "vmware: " fmt
> > > @@ -429,6 +432,10 @@ static void __init vmware_platform_setup(void)
> > >                 pr_warn("Failed to get TSC freq from the hypervisor\n");
> > >         }
> > >
> > > +       if (sev_status & MSR_AMD64_SEV_SNP_ENABLED &&
> > > +           !efi_enabled(EFI_BOOT))
> > > +               x86_init.mpparse.find_mptable = mpparse_find_mptable;
> >
> > As far I know, Linux itself currently doesn't PVALIDATE the address
> > ranges scanned in mpparse_find_mptable(), and Linux accesses these
> > addresses as encrypted during early boot. Given this, am I correct
> > that the guest firmware that you're using is doing the PVALIDATE of
> > these ranges before starting Linux (else there would be a crash upon
> > scan)? And then presumably the firmware is also making sure that this
> > memory is encrypted so that Linux isn't reading unencrypted data as
> > encrypted (i.e., garbage)?
>
> Yes, Linux (as a guest) receives valid data.
>
> > If so, does that mean all the ROM region scans removed in [0] are
> > permissible for SEV-SNP guests booting on whichever guest firmware
> > this is? But you only want/need the mptable info here?
>
> Yes, VMware firmware validates MPTABLE info when 'non-EFI guest boots
> on VMware Hypervisor'. The other ROM regions removed in [0] were not
> encrypted and validated. This is a specific use case for the VMware platform.
> Linux today assumes SNP VMs will be booted with UEFI.
>
> > [0] 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for
> > SEV-SNP guests")
> >
> > More broadly, ideally the guest firmware would communicate to Linux
> > that these ranges are safe to access (perhaps via the e820 table),
> > rather than Linux making the assumption that the ranges are safe for
> > non-EFI SEV-SNP guest boots in this scenario. However, since you're
> > only changing vmware_platform_setup() for such boots, I don't think we
> > need to hold up this patch on that generality.
>
> This is a VMware specific scenario. VMware Hypervisor makes the MPTABLE
> memory available in e820 when 'non-EFI guest boots on VMware Hypervisor'.
> So it makes sense to do the changes only for vmware_platform_setup().

Kevin, thanks for your time and sharing your thoughts. Please let me know
if you have any updates or need further information from my side.

Looking forward to hearing from you.

- Ajay

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5427 bytes --]

Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor

[Kevin Loughlin]

On Thu, Jan 2, 2025 at 8:31 PM Ajay Kaher <ajay.kaher@broadcom.com> wrote:
>
> On Thu, Dec 26, 2024 at 9:26 PM Kevin Loughlin <kevinloughlin@google.com> wrote:
> >
> > On Thu, Dec 19, 2024 at 6:44 AM Ajay Kaher <ajay.kaher@broadcom.com> wrote:
> > >
> > > For VMware hypervisor, SEV-SNP enabled VM's could boot without UEFI.
> > > In this case, mpparse_find_mptable() has to be called to parse MP
> > > tables which contains boot information.
> > >
> > > Fixes: 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for SEV-SNP guests")
> > > Signed-off-by: Ajay Kaher <ajay.kaher@broadcom.com>
> > > Signed-off-by: Ye Li <ye.li@broadcom.com>
> > > Tested-by: Ye Li <ye.li@broadcom.com>
> > > ---
> > >  arch/x86/kernel/cpu/vmware.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> > > index 00189cd..3e2594d 100644
> > > --- a/arch/x86/kernel/cpu/vmware.c
> > > +++ b/arch/x86/kernel/cpu/vmware.c
> > > @@ -26,6 +26,7 @@
> > >  #include <linux/export.h>
> > >  #include <linux/clocksource.h>
> > >  #include <linux/cpu.h>
> > > +#include <linux/efi.h>
> > >  #include <linux/reboot.h>
> > >  #include <linux/static_call.h>
> > >  #include <asm/div64.h>
> > > @@ -35,6 +36,8 @@
> > >  #include <asm/apic.h>
> > >  #include <asm/vmware.h>
> > >  #include <asm/svm.h>
> > > +#include <asm/mem_encrypt.h>
> > > +#include <asm/efi.h>
> > >
> > >  #undef pr_fmt
> > >  #define pr_fmt(fmt)    "vmware: " fmt
> > > @@ -429,6 +432,10 @@ static void __init vmware_platform_setup(void)
> > >                 pr_warn("Failed to get TSC freq from the hypervisor\n");
> > >         }
> > >
> > > +       if (sev_status & MSR_AMD64_SEV_SNP_ENABLED &&
> > > +           !efi_enabled(EFI_BOOT))
> > > +               x86_init.mpparse.find_mptable = mpparse_find_mptable;
> >
> > As far I know, Linux itself currently doesn't PVALIDATE the address
> > ranges scanned in mpparse_find_mptable(), and Linux accesses these
> > addresses as encrypted during early boot. Given this, am I correct
> > that the guest firmware that you're using is doing the PVALIDATE of
> > these ranges before starting Linux (else there would be a crash upon
> > scan)? And then presumably the firmware is also making sure that this
> > memory is encrypted so that Linux isn't reading unencrypted data as
> > encrypted (i.e., garbage)?
>
> Yes, Linux (as a guest) receives valid data.
>
> > If so, does that mean all the ROM region scans removed in [0] are
> > permissible for SEV-SNP guests booting on whichever guest firmware
> > this is? But you only want/need the mptable info here?
>
> Yes, VMware firmware validates MPTABLE info when 'non-EFI guest boots
> on VMware Hypervisor'. The other ROM regions removed in [0] were not
> encrypted and validated. This is a specific use case for the VMware platform.
> Linux today assumes SNP VMs will be booted with UEFI.
>
> > [0] 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for
> > SEV-SNP guests")
> >
> > More broadly, ideally the guest firmware would communicate to Linux
> > that these ranges are safe to access (perhaps via the e820 table),
> > rather than Linux making the assumption that the ranges are safe for
> > non-EFI SEV-SNP guest boots in this scenario. However, since you're
> > only changing vmware_platform_setup() for such boots, I don't think we
> > need to hold up this patch on that generality.
>
> This is a VMware specific scenario. VMware Hypervisor makes the MPTABLE
> memory available in e820 when 'non-EFI guest boots on VMware Hypervisor'.
> So it makes sense to do the changes only for vmware_platform_setup().

Works for me. Just wanted to make sure we don't regress Linux to
making assumptions about the safety of accessing memory regions in
CoCo guests without having entries in the e820 table for them.

Reviewed-by: Kevin Loughlin <kevinloughlin@google.com>

Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor

[Ajay Kaher]

[-- Attachment #1: Type: text/plain, Size: 4313 bytes --]

On Sat, Jan 18, 2025 at 4:26 AM Kevin Loughlin <kevinloughlin@google.com> wrote:
>
> > > > For VMware hypervisor, SEV-SNP enabled VM's could boot without UEFI.
> > > > In this case, mpparse_find_mptable() has to be called to parse MP
> > > > tables which contains boot information.
> > > >
> > > > Fixes: 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for SEV-SNP guests")
> > > > Signed-off-by: Ajay Kaher <ajay.kaher@broadcom.com>
> > > > Signed-off-by: Ye Li <ye.li@broadcom.com>
> > > > Tested-by: Ye Li <ye.li@broadcom.com>
> > > > ---
> > > >  arch/x86/kernel/cpu/vmware.c | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> > > > index 00189cd..3e2594d 100644
> > > > --- a/arch/x86/kernel/cpu/vmware.c
> > > > +++ b/arch/x86/kernel/cpu/vmware.c
> > > > @@ -26,6 +26,7 @@
> > > >  #include <linux/export.h>
> > > >  #include <linux/clocksource.h>
> > > >  #include <linux/cpu.h>
> > > > +#include <linux/efi.h>
> > > >  #include <linux/reboot.h>
> > > >  #include <linux/static_call.h>
> > > >  #include <asm/div64.h>
> > > > @@ -35,6 +36,8 @@
> > > >  #include <asm/apic.h>
> > > >  #include <asm/vmware.h>
> > > >  #include <asm/svm.h>
> > > > +#include <asm/mem_encrypt.h>
> > > > +#include <asm/efi.h>
> > > >
> > > >  #undef pr_fmt
> > > >  #define pr_fmt(fmt)    "vmware: " fmt
> > > > @@ -429,6 +432,10 @@ static void __init vmware_platform_setup(void)
> > > >                 pr_warn("Failed to get TSC freq from the hypervisor\n");
> > > >         }
> > > >
> > > > +       if (sev_status & MSR_AMD64_SEV_SNP_ENABLED &&
> > > > +           !efi_enabled(EFI_BOOT))
> > > > +               x86_init.mpparse.find_mptable = mpparse_find_mptable;
> > >
> > > As far I know, Linux itself currently doesn't PVALIDATE the address
> > > ranges scanned in mpparse_find_mptable(), and Linux accesses these
> > > addresses as encrypted during early boot. Given this, am I correct
> > > that the guest firmware that you're using is doing the PVALIDATE of
> > > these ranges before starting Linux (else there would be a crash upon
> > > scan)? And then presumably the firmware is also making sure that this
> > > memory is encrypted so that Linux isn't reading unencrypted data as
> > > encrypted (i.e., garbage)?
> >
> > Yes, Linux (as a guest) receives valid data.
> >
> > > If so, does that mean all the ROM region scans removed in [0] are
> > > permissible for SEV-SNP guests booting on whichever guest firmware
> > > this is? But you only want/need the mptable info here?
> >
> > Yes, VMware firmware validates MPTABLE info when 'non-EFI guest boots
> > on VMware Hypervisor'. The other ROM regions removed in [0] were not
> > encrypted and validated. This is a specific use case for the VMware platform.
> > Linux today assumes SNP VMs will be booted with UEFI.
> >
> > > [0] 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for
> > > SEV-SNP guests")
> > >
> > > More broadly, ideally the guest firmware would communicate to Linux
> > > that these ranges are safe to access (perhaps via the e820 table),
> > > rather than Linux making the assumption that the ranges are safe for
> > > non-EFI SEV-SNP guest boots in this scenario. However, since you're
> > > only changing vmware_platform_setup() for such boots, I don't think we
> > > need to hold up this patch on that generality.
> >
> > This is a VMware specific scenario. VMware Hypervisor makes the MPTABLE
> > memory available in e820 when 'non-EFI guest boots on VMware Hypervisor'.
> > So it makes sense to do the changes only for vmware_platform_setup().
>
> Works for me. Just wanted to make sure we don't regress Linux to
> making assumptions about the safety of accessing memory regions in
> CoCo guests without having entries in the e820 table for them.
>
> Reviewed-by: Kevin Loughlin <kevinloughlin@google.com>

Adding 'Borislav Petkov <bp@suse.de>' to this mail.

I am looking for an update on this patch, please let me know
if there are any issues preventing it from being merged, or if
you need any additional information.

Link to patch:
https://lore.kernel.org/lkml/20241219114400.858980-1-ajay.kaher@broadcom.com/

-Ajay

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]

Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor

[Borislav Petkov]

On Thu, Dec 19, 2024 at 11:44:00AM +0000, Ajay Kaher wrote:
> For VMware hypervisor, SEV-SNP enabled VM's could boot without UEFI.
> In this case, mpparse_find_mptable() has to be called to parse MP
> tables which contains boot information.
> 
> Fixes: 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for SEV-SNP guests")
> Signed-off-by: Ajay Kaher <ajay.kaher@broadcom.com>
> Signed-off-by: Ye Li <ye.li@broadcom.com>
> Tested-by: Ye Li <ye.li@broadcom.com>

That SOB chain is wrong. Ye's SOB means, he's sending the patch but that
doesn't look like it.

> ---
>  arch/x86/kernel/cpu/vmware.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index 00189cd..3e2594d 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -26,6 +26,7 @@
>  #include <linux/export.h>
>  #include <linux/clocksource.h>
>  #include <linux/cpu.h>
> +#include <linux/efi.h>
>  #include <linux/reboot.h>
>  #include <linux/static_call.h>
>  #include <asm/div64.h>
> @@ -35,6 +36,8 @@
>  #include <asm/apic.h>
>  #include <asm/vmware.h>
>  #include <asm/svm.h>
> +#include <asm/mem_encrypt.h>
> +#include <asm/efi.h>
>  
>  #undef pr_fmt
>  #define pr_fmt(fmt)	"vmware: " fmt
> @@ -429,6 +432,10 @@ static void __init vmware_platform_setup(void)
>  		pr_warn("Failed to get TSC freq from the hypervisor\n");
>  	}
>  
> +	if (sev_status & MSR_AMD64_SEV_SNP_ENABLED &&

cpu_feature_enabled(X86_FEATURE_SEV_SNP)

> +	    !efi_enabled(EFI_BOOT))
> +		x86_init.mpparse.find_mptable = mpparse_find_mptable;
> +
>  	vmware_paravirt_ops_setup();
>  
>  #ifdef CONFIG_X86_IO_APIC
> -- 

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor

[Tom Lendacky]

On 3/10/25 05:26, Borislav Petkov wrote:
> On Thu, Dec 19, 2024 at 11:44:00AM +0000, Ajay Kaher wrote:
>> For VMware hypervisor, SEV-SNP enabled VM's could boot without UEFI.
>> In this case, mpparse_find_mptable() has to be called to parse MP
>> tables which contains boot information.
>>
>> Fixes: 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for SEV-SNP guests")
>> Signed-off-by: Ajay Kaher <ajay.kaher@broadcom.com>
>> Signed-off-by: Ye Li <ye.li@broadcom.com>
>> Tested-by: Ye Li <ye.li@broadcom.com>
> 
> That SOB chain is wrong. Ye's SOB means, he's sending the patch but that
> doesn't look like it.
> 
>> ---
>>  arch/x86/kernel/cpu/vmware.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
>> index 00189cd..3e2594d 100644
>> --- a/arch/x86/kernel/cpu/vmware.c
>> +++ b/arch/x86/kernel/cpu/vmware.c
>> @@ -26,6 +26,7 @@
>>  #include <linux/export.h>
>>  #include <linux/clocksource.h>
>>  #include <linux/cpu.h>
>> +#include <linux/efi.h>
>>  #include <linux/reboot.h>
>>  #include <linux/static_call.h>
>>  #include <asm/div64.h>
>> @@ -35,6 +36,8 @@
>>  #include <asm/apic.h>
>>  #include <asm/vmware.h>
>>  #include <asm/svm.h>
>> +#include <asm/mem_encrypt.h>
>> +#include <asm/efi.h>
>>  
>>  #undef pr_fmt
>>  #define pr_fmt(fmt)	"vmware: " fmt
>> @@ -429,6 +432,10 @@ static void __init vmware_platform_setup(void)
>>  		pr_warn("Failed to get TSC freq from the hypervisor\n");
>>  	}
>>  
>> +	if (sev_status & MSR_AMD64_SEV_SNP_ENABLED &&
> 
> cpu_feature_enabled(X86_FEATURE_SEV_SNP)

I think this should really be cc_platform_has(CC_ATTR_GUEST_SEV_SNP).

Thanks,
Tom

> 
>> +	    !efi_enabled(EFI_BOOT))
>> +		x86_init.mpparse.find_mptable = mpparse_find_mptable;
>> +
>>  	vmware_paravirt_ops_setup();
>>  
>>  #ifdef CONFIG_X86_IO_APIC
>> -- 
> 

Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor

[Borislav Petkov]

On Mon, Mar 10, 2025 at 08:12:46AM -0500, Tom Lendacky wrote:
> >> +	if (sev_status & MSR_AMD64_SEV_SNP_ENABLED &&
> > 
> > cpu_feature_enabled(X86_FEATURE_SEV_SNP)
> 
> I think this should really be cc_platform_has(CC_ATTR_GUEST_SEV_SNP).

Ah, we wanna check the guest, sure.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor

[Ajay Kaher]

[-- Attachment #1: Type: text/plain, Size: 2058 bytes --]

On Mon, Mar 10, 2025 at 6:42 PM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 3/10/25 05:26, Borislav Petkov wrote:
> > On Thu, Dec 19, 2024 at 11:44:00AM +0000, Ajay Kaher wrote:
> >> For VMware hypervisor, SEV-SNP enabled VM's could boot without UEFI.
> >> In this case, mpparse_find_mptable() has to be called to parse MP
> >> tables which contains boot information.
> >>
> >> Fixes: 0f4a1e80989a ("x86/sev: Skip ROM range scans and validation for SEV-SNP guests")
> >> Signed-off-by: Ajay Kaher <ajay.kaher@broadcom.com>
> >> Signed-off-by: Ye Li <ye.li@broadcom.com>
> >> Tested-by: Ye Li <ye.li@broadcom.com>
> >
> > That SOB chain is wrong. Ye's SOB means, he's sending the patch but that
> > doesn't look like it.

I will add CDB for Ye in v2.

> >> ---
> >>  arch/x86/kernel/cpu/vmware.c | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> >> index 00189cd..3e2594d 100644
> >> --- a/arch/x86/kernel/cpu/vmware.c
> >> +++ b/arch/x86/kernel/cpu/vmware.c
> >> @@ -26,6 +26,7 @@
> >>  #include <linux/export.h>
> >>  #include <linux/clocksource.h>
> >>  #include <linux/cpu.h>
> >> +#include <linux/efi.h>
> >>  #include <linux/reboot.h>
> >>  #include <linux/static_call.h>
> >>  #include <asm/div64.h>
> >> @@ -35,6 +36,8 @@
> >>  #include <asm/apic.h>
> >>  #include <asm/vmware.h>
> >>  #include <asm/svm.h>
> >> +#include <asm/mem_encrypt.h>
> >> +#include <asm/efi.h>
> >>
> >>  #undef pr_fmt
> >>  #define pr_fmt(fmt) "vmware: " fmt
> >> @@ -429,6 +432,10 @@ static void __init vmware_platform_setup(void)
> >>              pr_warn("Failed to get TSC freq from the hypervisor\n");
> >>      }
> >>
> >> +    if (sev_status & MSR_AMD64_SEV_SNP_ENABLED &&
> >
> > cpu_feature_enabled(X86_FEATURE_SEV_SNP)
>
> I think this should really be cc_platform_has(CC_ATTR_GUEST_SEV_SNP).

cc_platform_has(CC_ATTR_GUEST_SEV_SNP) works, I will make appropriate
changes in v2.

Thanks Borislav, Tom.

- Ajay

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]