[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAD2QZ9acg0Z6dT90saG7D9U+oNMCnoskZY=+0pxaDXSXbauTLw@mail.gmail.com>
Date: Sun, 9 Mar 2025 13:05:21 +0530
From: Ajay Kaher <ajay.kaher@...adcom.com>
To: Kevin Loughlin <kevinloughlin@...gle.com>, Borislav Petkov <bp@...e.de>
Cc: bp@...en8.de, bcm-kernel-feedback-list@...adcom.com, tglx@...utronix.de,
mingo@...hat.com, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org,
ye.li@...adcom.com, bo.gan@...adcom.com,
vamsi-krishna.brahmajosyula@...adcom.com, alexey.makhalov@...adcom.com,
vasavi.sirnapalli@...adcom.com, florian.fainelli@...adcom.com
Subject: Re: [PATCH] sev-snp: parse MP tables for VMware hypervisor
On Sat, Jan 18, 2025 at 4:26 AM Kevin Loughlin <kevinloughlin@...gle.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@...adcom.com>
> > > > Signed-off-by: Ye Li <ye.li@...adcom.com>
> > > > Tested-by: Ye Li <ye.li@...adcom.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@...gle.com>
Adding 'Borislav Petkov <bp@...e.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
Download attachment "smime.p7s" of type "application/pkcs7-signature" (5414 bytes)
Powered by blists - more mailing lists