Re: [RFC 00/14] AMD: Add Secure AVIC Guest Support

[Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>]

On 10/29/2024 3:17 PM, Borislav Petkov wrote:
> On Fri, Oct 18, 2024 at 10:54:21AM +0300, Kirill A. Shutemov wrote:
>> I think it has to be addressed before it got merged. Or we will get a
>> regression.
> 
> ... or temporarily disable kexec when SAVIC is present.
> 

Thanks! I plan to do something like below patch for the next version.
Verified Secure AVIC guest kexec with this.



- Neeraj

-----------------------------------------------------------------------

From 80a4901644fa8a9ed2c6f690fbba4b8a6176b215 Mon Sep 17 00:00:00 2001
From: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>
Date: Tue, 29 Oct 2024 15:38:21 +0530
Subject: [RFC 15/14] x86/apic: Add kexec support for Secure AVIC

Add a ->teardown callback to disable Secure AVIC before
rebooting into the new kernel. This ensures that the new
kernel does not access the old APIC backing page which was
allocated by the previous kernel. This can happen if there
are any APIC accesses done during guest boot before Secure
AVIC driver probe is done by the new kernel (as Secure AVIC
remained enabled in control msr).

Signed-off-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>
---

This is dependent on SNP guest supports patches [1]


[1] https://lore.kernel.org/lkml/cover.1722520012.git.ashish.kalra@amd.com/

 arch/x86/include/asm/apic.h         | 1 +
 arch/x86/kernel/apic/apic.c         | 3 +++
 arch/x86/kernel/apic/x2apic_savic.c | 7 +++++++
 3 files changed, 11 insertions(+)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 2d5400372470..ec332afd0277 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -303,6 +303,7 @@ struct apic {
        /* Probe, setup and smpboot functions */
        int     (*probe)(void);
        void    (*setup)(void);
+       void    (*teardown)(void);
        int     (*acpi_madt_oem_check)(char *oem_id, char *oem_table_id);

        void    (*init_apic_ldr)(void);
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index aeda74bf15e6..08156ac4ec6c 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1163,6 +1163,9 @@ void disable_local_APIC(void)
        if (!apic_accessible())
                return;

+       if (apic->teardown)
+               apic->teardown();
+
        apic_soft_disable();

 #ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/apic/x2apic_savic.c b/arch/x86/kernel/apic/x2apic_savic.c
index a3f0ddc6b5b6..bb7a28f9646a 100644
--- a/arch/x86/kernel/apic/x2apic_savic.c
+++ b/arch/x86/kernel/apic/x2apic_savic.c
@@ -391,6 +391,12 @@ static void init_backing_page(void *backing_page)
        set_reg(backing_page, APIC_ID, apic_id);
 }

+static void x2apic_savic_teardown(void)
+{
+       /* Disable Secure AVIC */
+       native_wrmsr(MSR_AMD64_SECURE_AVIC_CONTROL, 0, 0);
+}
+
 static void x2apic_savic_setup(void)
 {
        void *backing_page;
@@ -447,6 +453,7 @@ static struct apic apic_x2apic_savic __ro_after_init = {
        .probe                          = x2apic_savic_probe,
        .acpi_madt_oem_check            = x2apic_savic_acpi_madt_oem_check,
        .setup                          = x2apic_savic_setup,
+       .teardown                       = x2apic_savic_teardown,

        .dest_mode_logical              = false,

--