| Message ID | 20240913113705.419146-2-Neeraj.Upadhyay@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | AMD: Add Secure AVIC Guest Support | expand |
On Fri, Sep 13, 2024 at 05:06:52PM +0530, Neeraj Upadhyay wrote: > diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c > index cd44e120fe53..ec038be0a048 100644 > --- a/arch/x86/boot/compressed/sev.c > +++ b/arch/x86/boot/compressed/sev.c > @@ -394,6 +394,7 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code) > MSR_AMD64_SNP_VMSA_REG_PROT | \ > MSR_AMD64_SNP_RESERVED_BIT13 | \ > MSR_AMD64_SNP_RESERVED_BIT15 | \ > + MSR_AMD64_SNP_SECURE_AVIC_ENABLED | \ > MSR_AMD64_SNP_RESERVED_MASK) > > /* Shouldn't this hunk be in the last patch of the series, after all the sAVIC enablement has happened?
On 10/9/2024 12:45 AM, Borislav Petkov wrote: > On Fri, Sep 13, 2024 at 05:06:52PM +0530, Neeraj Upadhyay wrote: >> diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c >> index cd44e120fe53..ec038be0a048 100644 >> --- a/arch/x86/boot/compressed/sev.c >> +++ b/arch/x86/boot/compressed/sev.c >> @@ -394,6 +394,7 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code) >> MSR_AMD64_SNP_VMSA_REG_PROT | \ >> MSR_AMD64_SNP_RESERVED_BIT13 | \ >> MSR_AMD64_SNP_RESERVED_BIT15 | \ >> + MSR_AMD64_SNP_SECURE_AVIC_ENABLED | \ >> MSR_AMD64_SNP_RESERVED_MASK) >> >> /* > > Shouldn't this hunk be in the last patch of the series, after all the sAVIC > enablement has happened? > As SECURE_AVIC feature is not supported (as reported by snp_get_unsupported_features()) by guest at this patch in the series, it is added to SNP_FEATURES_IMPL_REQ here. The bit value within SNP_FEATURES_IMPL_REQ hasn't changed with this change as the same bit pos was part of MSR_AMD64_SNP_RESERVED_MASK before this patch. In patch 14 SECURE_AVIC guest support is indicated by guest. - Neeraj
On Wed, Oct 09, 2024 at 07:26:55AM +0530, Neeraj Upadhyay wrote: > As SECURE_AVIC feature is not supported (as reported by snp_get_unsupported_features()) > by guest at this patch in the series, it is added to SNP_FEATURES_IMPL_REQ here. The bit > value within SNP_FEATURES_IMPL_REQ hasn't changed with this change as the same bit pos > was part of MSR_AMD64_SNP_RESERVED_MASK before this patch. In patch 14 SECURE_AVIC guest > support is indicated by guest. So what's the point of adding it to SNP_FEATURES_IMPL_REQ here? What does that do at all in this patch alone? Why is this change needed in here? IOW, why don't you do all the feature bit handling in the last patch, where it all belongs logically? In the last patch you can start *testing* for MSR_AMD64_SNP_SECURE_AVIC_ENABLED *and* enforce it with SNP_FEATURES_PRESENT.
On 10/9/2024 10:53 AM, Borislav Petkov wrote: > On Wed, Oct 09, 2024 at 07:26:55AM +0530, Neeraj Upadhyay wrote: >> As SECURE_AVIC feature is not supported (as reported by snp_get_unsupported_features()) >> by guest at this patch in the series, it is added to SNP_FEATURES_IMPL_REQ here. The bit >> value within SNP_FEATURES_IMPL_REQ hasn't changed with this change as the same bit pos >> was part of MSR_AMD64_SNP_RESERVED_MASK before this patch. In patch 14 SECURE_AVIC guest >> support is indicated by guest. > > So what's the point of adding it to SNP_FEATURES_IMPL_REQ here? What does that > do at all in this patch alone? Why is this change needed in here? > Before this patch, if hypervisor enables Secure AVIC (reported in sev_status), guest would terminate in snp_check_features(). The reason for this is, SNP_FEATURES_IMPL_REQ had the Secure AVIC bit set before this patch, as that bit was part of MSR_AMD64_SNP_RESERVED_MASK GENMASK_ULL(63, 18). #define SNP_FEATURES_IMPL_REQ (MSR_AMD64_SNP_VTOM | \ ... MSR_AMD64_SNP_RESERVED_MASK) Adding MSR_AMD64_SNP_SECURE_AVIC_BIT (bit 18) to SNP_FEATURES_IMPL_REQ in this patch keeps that behavior intact as now with this change MSR_AMD64_SNP_RESERVED_MASK becomes GENMASK_ULL(63, 19). > IOW, why don't you do all the feature bit handling in the last patch, where it > all belongs logically? > If we do that, then hypervisor could have enabled Secure AVIC support and the guest code at this patch won't catch the missing guest-support early and it can result in some unknown failures at later point during guest boot. - Neeraj > In the last patch you can start *testing* for > MSR_AMD64_SNP_SECURE_AVIC_ENABLED *and* enforce it with SNP_FEATURES_PRESENT. >
On Fri, Sep 13, 2024 at 05:06:52PM +0530, Neeraj Upadhyay wrote: > diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h > index caa4b4430634..801208678450 100644 > --- a/include/linux/cc_platform.h > +++ b/include/linux/cc_platform.h > @@ -88,6 +88,14 @@ enum cc_attr { > * enabled to run SEV-SNP guests. > */ > CC_ATTR_HOST_SEV_SNP, > + > + /** > + * @CC_ATTR_SNP_SECURE_AVIC: Secure AVIC mode is active. > + * > + * The host kernel is running with the necessary features enabled > + * to run SEV-SNP guests with full Secure AVIC capabilities. > + */ > + CC_ATTR_SNP_SECURE_AVIC, I don't think CC attributes is the right way to track this kind of features. My understanding of cc_platform interface is that it has to be used to advertise some kind of property of the platform that generic code and be interested in, not a specific implementation. For the same reason, I think CC_ATTR_GUEST/HOST_SEV_SNP is also a bad use of the interface. Borislav, I know we had different view on this. What is your criteria on what should and shouldn't be a CC attribute? I don't think we want a parallel X86_FEATURE_*.
On Wed, Oct 09, 2024 at 11:31:07AM +0530, Neeraj Upadhyay wrote: > Before this patch, if hypervisor enables Secure AVIC (reported in sev_status), guest would > terminate in snp_check_features(). We want the guest to terminate at this patch too. The only case where the guest should not terminate is when the *full* sAVIC support is in. I.e., at patch 14.
On Wed, Oct 09, 2024 at 01:10:58PM +0300, Kirill A. Shutemov wrote: > I don't think CC attributes is the right way to track this kind of > features. My understanding of cc_platform interface is that it has to be > used to advertise some kind of property of the platform that generic code > and be interested in, not a specific implementation. Yes. > > For the same reason, I think CC_ATTR_GUEST/HOST_SEV_SNP is also a bad use > of the interface. > > Borislav, I know we had different view on this. What is your criteria on > what should and shouldn't be a CC attribute? I don't think we want a > parallel X86_FEATURE_*. Yes, we don't. Do you have a better idea which is cleaner than what we do now? Yes yes, cc_platform reports aspects of the coco platform to generic code but nothing stops the x86 code from calling those interfaces too, for simplicity reasons. Because the coco platform being a SNP guest or having an SAVIC are also two aspects of that same confidential computing platform. So we might as well use it this way too. I'd say.
On 10/9/2024 4:08 PM, Borislav Petkov wrote: > On Wed, Oct 09, 2024 at 11:31:07AM +0530, Neeraj Upadhyay wrote: >> Before this patch, if hypervisor enables Secure AVIC (reported in sev_status), guest would >> terminate in snp_check_features(). > > We want the guest to terminate at this patch too. > If I understand it correctly, you are fine with adding MSR_AMD64_SNP_SECURE_AVIC_ENABLED to SNP_FEATURES_IMPL_REQ in this patch. > The only case where the guest should not terminate is when the *full* sAVIC > support is in. I.e., at patch 14. > Got it. This version of the patch series is following that. - Neeraj
On Wed, Oct 09, 2024 at 12:38:21PM +0200, Borislav Petkov wrote: > On Wed, Oct 09, 2024 at 11:31:07AM +0530, Neeraj Upadhyay wrote: > > Before this patch, if hypervisor enables Secure AVIC (reported in sev_status), guest would > > terminate in snp_check_features(). > > We want the guest to terminate at this patch too. > > The only case where the guest should not terminate is when the *full* sAVIC > support is in. I.e., at patch 14. I went and did a small C program doing all that - I see what you mean now - you want to *enforce* the guest termination when SAVIC bit is not in SNP_FEATURES_PRESENT. Basically what I want you do to. Please call that out in the commit message as it is important. Thx.
On Wed, Oct 09, 2024 at 12:42:34PM +0200, Borislav Petkov wrote: > Do you have a better idea which is cleaner than what we do now? I would rather convert these three attributes to synthetic X86_FEATUREs next to X86_FEATURE_TDX_GUEST. I suggested it once. > Yes yes, cc_platform reports aspects of the coco platform to generic code but > nothing stops the x86 code from calling those interfaces too, for simplicity > reasons. I don't see why it is any simpler than having a synthetic X86_FEATURE.
On Wed, Oct 09, 2024 at 02:03:48PM +0300, Kirill A. Shutemov wrote: > I would rather convert these three attributes to synthetic X86_FEATUREs > next to X86_FEATURE_TDX_GUEST. I suggested it once. And back then I answered that splitting the coco checks between a X86_FEATURE and a cc_platform ones is confusing. Which ones do I use, X86_FEATURE or cc_platform? Oh, for SNP or TDX I use cpu_feature_enabled() but in generic code I use cc_platform. Sounds confusing to me.
On Wed, Oct 09, 2024 at 01:22:16PM +0200, Borislav Petkov wrote: > On Wed, Oct 09, 2024 at 02:03:48PM +0300, Kirill A. Shutemov wrote: > > I would rather convert these three attributes to synthetic X86_FEATUREs > > next to X86_FEATURE_TDX_GUEST. I suggested it once. > > And back then I answered that splitting the coco checks between a X86_FEATURE > and a cc_platform ones is confusing. Which ones do I use, X86_FEATURE or > cc_platform? > > Oh, for SNP or TDX I use cpu_feature_enabled() but in generic code I use > cc_platform. > > Sounds confusing to me. If you use SNP or TDX check in generic code something is wrong. Abstraction is broken somewhere. Generic code doesn't need to know concrete implementation.
On 10/9/2024 4:32 PM, Borislav Petkov wrote: > On Wed, Oct 09, 2024 at 12:38:21PM +0200, Borislav Petkov wrote: >> On Wed, Oct 09, 2024 at 11:31:07AM +0530, Neeraj Upadhyay wrote: >>> Before this patch, if hypervisor enables Secure AVIC (reported in sev_status), guest would >>> terminate in snp_check_features(). >> >> We want the guest to terminate at this patch too. >> >> The only case where the guest should not terminate is when the *full* sAVIC >> support is in. I.e., at patch 14. > > I went and did a small C program doing all that - I see what you mean now > - you want to *enforce* the guest termination when SAVIC bit is not in > SNP_FEATURES_PRESENT. > Yes. > Basically what I want you do to. > Cool! > Please call that out in the commit message as it is important. > Sure, will update in next version. Thanks! - Neeraj > Thx. >
On 10/9/24 01:01, Neeraj Upadhyay wrote: > > > On 10/9/2024 10:53 AM, Borislav Petkov wrote: >> On Wed, Oct 09, 2024 at 07:26:55AM +0530, Neeraj Upadhyay wrote: >>> As SECURE_AVIC feature is not supported (as reported by snp_get_unsupported_features()) >>> by guest at this patch in the series, it is added to SNP_FEATURES_IMPL_REQ here. The bit >>> value within SNP_FEATURES_IMPL_REQ hasn't changed with this change as the same bit pos >>> was part of MSR_AMD64_SNP_RESERVED_MASK before this patch. In patch 14 SECURE_AVIC guest >>> support is indicated by guest. >> >> So what's the point of adding it to SNP_FEATURES_IMPL_REQ here? What does that >> do at all in this patch alone? Why is this change needed in here? >> > > Before this patch, if hypervisor enables Secure AVIC (reported in sev_status), guest would > terminate in snp_check_features(). The reason for this is, SNP_FEATURES_IMPL_REQ had the Secure > AVIC bit set before this patch, as that bit was part of MSR_AMD64_SNP_RESERVED_MASK > GENMASK_ULL(63, 18). > > #define SNP_FEATURES_IMPL_REQ (MSR_AMD64_SNP_VTOM | \ > ... > MSR_AMD64_SNP_RESERVED_MASK) > > > > Adding MSR_AMD64_SNP_SECURE_AVIC_BIT (bit 18) to SNP_FEATURES_IMPL_REQ in this patch > keeps that behavior intact as now with this change MSR_AMD64_SNP_RESERVED_MASK becomes > GENMASK_ULL(63, 19). > > >> IOW, why don't you do all the feature bit handling in the last patch, where it >> all belongs logically? >> > > If we do that, then hypervisor could have enabled Secure AVIC support and the guest > code at this patch won't catch the missing guest-support early and it can result in some > unknown failures at later point during guest boot. Won't the SNP_RESERVED_MASK catch it? You are just renaming the bit position value, right? It was a 1 before and is still a 1. So the guest will terminate if the hypervisor sets the Secure AVIC bit both before and after this patch, right? Thanks, Tom > > > - Neeraj > >> In the last patch you can start *testing* for >> MSR_AMD64_SNP_SECURE_AVIC_ENABLED *and* enforce it with SNP_FEATURES_PRESENT. >>
On 10/9/2024 6:45 PM, Tom Lendacky wrote: > On 10/9/24 01:01, Neeraj Upadhyay wrote: >> >> >> On 10/9/2024 10:53 AM, Borislav Petkov wrote: >>> On Wed, Oct 09, 2024 at 07:26:55AM +0530, Neeraj Upadhyay wrote: >>>> As SECURE_AVIC feature is not supported (as reported by snp_get_unsupported_features()) >>>> by guest at this patch in the series, it is added to SNP_FEATURES_IMPL_REQ here. The bit >>>> value within SNP_FEATURES_IMPL_REQ hasn't changed with this change as the same bit pos >>>> was part of MSR_AMD64_SNP_RESERVED_MASK before this patch. In patch 14 SECURE_AVIC guest >>>> support is indicated by guest. >>> >>> So what's the point of adding it to SNP_FEATURES_IMPL_REQ here? What does that >>> do at all in this patch alone? Why is this change needed in here? >>> >> >> Before this patch, if hypervisor enables Secure AVIC (reported in sev_status), guest would >> terminate in snp_check_features(). The reason for this is, SNP_FEATURES_IMPL_REQ had the Secure >> AVIC bit set before this patch, as that bit was part of MSR_AMD64_SNP_RESERVED_MASK >> GENMASK_ULL(63, 18). >> >> #define SNP_FEATURES_IMPL_REQ (MSR_AMD64_SNP_VTOM | \ >> ... >> MSR_AMD64_SNP_RESERVED_MASK) >> >> >> >> Adding MSR_AMD64_SNP_SECURE_AVIC_BIT (bit 18) to SNP_FEATURES_IMPL_REQ in this patch >> keeps that behavior intact as now with this change MSR_AMD64_SNP_RESERVED_MASK becomes >> GENMASK_ULL(63, 19). >> >> >>> IOW, why don't you do all the feature bit handling in the last patch, where it >>> all belongs logically? >>> >> >> If we do that, then hypervisor could have enabled Secure AVIC support and the guest >> code at this patch won't catch the missing guest-support early and it can result in some >> unknown failures at later point during guest boot. > > Won't the SNP_RESERVED_MASK catch it? You are just renaming the bit > position value, right? It was a 1 before and is still a 1. So the guest > will terminate if the hypervisor sets the Secure AVIC bit both before > and after this patch, right? > Yes that is right. SNP_RESERVED_MASK catches it before this patch. My reply to Boris above was for the case if we move setting of MSR_AMD64_SNP_SECURE_AVIC_ENABLED in SNP_FEATURES_IMPL_REQ from this patch to patch 14. - Neeraj > Thanks, > Tom > >> >> >> - Neeraj >> >>> In the last patch you can start *testing* for >>> MSR_AMD64_SNP_SECURE_AVIC_ENABLED *and* enforce it with SNP_FEATURES_PRESENT. >>>
On Wed, Oct 09, 2024 at 03:12:41PM +0300, Kirill A. Shutemov wrote: > If you use SNP or TDX check in generic code something is wrong. Abstraction > is broken somewhere. Generic code doesn't need to know concrete > implementation. That's perhaps because you're thinking that the *actual* coco implementation type should be hidden away from generic code. But SNP and TDX are pretty different so we might as well ask for them by their name. But I can see why you'd think there might be some abstraction violation there. My goal here - even though there might be some bad taste of abstraction violation here - is simplicity. As expressed a bunch of times already, having cc_platform *and* X86_FEATURE* things used in relation to coco code can be confusing. So I'd prefer to avoid that confusion. Nothing says anywhere that arch code cannot use cc_platform interfaces. Absolutely nothing. So for the sake of KISS I'm going in that direction. If it turns out later that this was a bad idea and we need to change it, we can always can. As we do for other interfaces in the kernel. If you're still not convinced, I already asked you: "Do you have a better idea which is cleaner than what we do now?" Your turn. Thx.
On Wed, Oct 09, 2024 at 03:53:35PM +0200, Borislav Petkov wrote: > On Wed, Oct 09, 2024 at 03:12:41PM +0300, Kirill A. Shutemov wrote: > > If you use SNP or TDX check in generic code something is wrong. Abstraction > > is broken somewhere. Generic code doesn't need to know concrete > > implementation. > > That's perhaps because you're thinking that the *actual* coco implementation type > should be hidden away from generic code. But SNP and TDX are pretty different > so we might as well ask for them by their name. > > But I can see why you'd think there might be some abstraction violation there. > > My goal here - even though there might be some bad taste of abstraction > violation here - is simplicity. As expressed a bunch of times already, having > cc_platform *and* X86_FEATURE* things used in relation to coco code can be > confusing. So I'd prefer to avoid that confusion. > > Nothing says anywhere that arch code cannot use cc_platform interfaces. > Absolutely nothing. So for the sake of KISS I'm going in that direction. > > If it turns out later that this was a bad idea and we need to change it, we > can always can. As we do for other interfaces in the kernel. > > If you're still not convinced, I already asked you: > > "Do you have a better idea which is cleaner than what we do now?" > > Your turn. Okay, I've got your point. It is not what I would do, but I don't have sufficient argument to change what is already there.
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 007bab9f2a0e..b05b4e9d2e49 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -469,6 +469,18 @@ config X86_X2APIC If you don't know what to do here, say N. +config AMD_SECURE_AVIC + bool "AMD Secure AVIC" + depends on X86_X2APIC && AMD_MEM_ENCRYPT + help + This enables AMD Secure AVIC support on guests that have this feature. + + AMD Secure AVIC provides hardware acceleration for performance sensitive + APIC accesses and support for managing guest owned APIC state for SEV-SNP + guests. + + If you don't know what to do here, say N. + config X86_POSTED_MSI bool "Enable MSI and MSI-x delivery by posted interrupts" depends on X86_64 && IRQ_REMAP diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c index cd44e120fe53..ec038be0a048 100644 --- a/arch/x86/boot/compressed/sev.c +++ b/arch/x86/boot/compressed/sev.c @@ -394,6 +394,7 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code) MSR_AMD64_SNP_VMSA_REG_PROT | \ MSR_AMD64_SNP_RESERVED_BIT13 | \ MSR_AMD64_SNP_RESERVED_BIT15 | \ + MSR_AMD64_SNP_SECURE_AVIC_ENABLED | \ MSR_AMD64_SNP_RESERVED_MASK) /* diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c index 0f81f70aca82..4c3bc031e9a9 100644 --- a/arch/x86/coco/core.c +++ b/arch/x86/coco/core.c @@ -100,6 +100,9 @@ static bool noinstr amd_cc_platform_has(enum cc_attr attr) case CC_ATTR_HOST_SEV_SNP: return cc_flags.host_sev_snp; + case CC_ATTR_SNP_SECURE_AVIC: + return sev_status & MSR_AMD64_SNP_SECURE_AVIC_ENABLED; + default: return false; } diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h index 82c6a4d350e0..d0583619c978 100644 --- a/arch/x86/include/asm/msr-index.h +++ b/arch/x86/include/asm/msr-index.h @@ -658,7 +658,9 @@ #define MSR_AMD64_SNP_VMSA_REG_PROT BIT_ULL(MSR_AMD64_SNP_VMSA_REG_PROT_BIT) #define MSR_AMD64_SNP_SMT_PROT_BIT 17 #define MSR_AMD64_SNP_SMT_PROT BIT_ULL(MSR_AMD64_SNP_SMT_PROT_BIT) -#define MSR_AMD64_SNP_RESV_BIT 18 +#define MSR_AMD64_SNP_SECURE_AVIC_BIT 18 +#define MSR_AMD64_SNP_SECURE_AVIC_ENABLED BIT_ULL(MSR_AMD64_SNP_SECURE_AVIC_BIT) +#define MSR_AMD64_SNP_RESV_BIT 19 #define MSR_AMD64_SNP_RESERVED_MASK GENMASK_ULL(63, MSR_AMD64_SNP_RESV_BIT) #define MSR_AMD64_VIRT_SPEC_CTRL 0xc001011f diff --git a/arch/x86/kernel/apic/Makefile b/arch/x86/kernel/apic/Makefile index 3bf0487cf3b7..12153993c12b 100644 --- a/arch/x86/kernel/apic/Makefile +++ b/arch/x86/kernel/apic/Makefile @@ -18,6 +18,7 @@ ifeq ($(CONFIG_X86_64),y) # APIC probe will depend on the listing order here obj-$(CONFIG_X86_NUMACHIP) += apic_numachip.o obj-$(CONFIG_X86_UV) += x2apic_uv_x.o +obj-$(CONFIG_AMD_SECURE_AVIC) += x2apic_savic.o obj-$(CONFIG_X86_X2APIC) += x2apic_phys.o obj-$(CONFIG_X86_X2APIC) += x2apic_cluster.o obj-y += apic_flat_64.o diff --git a/arch/x86/kernel/apic/x2apic_savic.c b/arch/x86/kernel/apic/x2apic_savic.c new file mode 100644 index 000000000000..97dac09a7f42 --- /dev/null +++ b/arch/x86/kernel/apic/x2apic_savic.c @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * AMD Secure AVIC Support (SEV-SNP Guests) + * + * Copyright (C) 2024 Advanced Micro Devices, Inc. + * + * Author: Kishon Vijay Abraham I <kvijayab@amd.com> + */ + +#include <linux/cpumask.h> +#include <linux/cc_platform.h> + +#include <asm/apic.h> +#include <asm/sev.h> + +#include "local.h" + +static int x2apic_savic_acpi_madt_oem_check(char *oem_id, char *oem_table_id) +{ + return x2apic_enabled() && cc_platform_has(CC_ATTR_SNP_SECURE_AVIC); +} + +static void x2apic_savic_send_IPI(int cpu, int vector) +{ + u32 dest = per_cpu(x86_cpu_to_apicid, cpu); + + /* x2apic MSRs are special and need a special fence: */ + weak_wrmsr_fence(); + __x2apic_send_IPI_dest(dest, vector, APIC_DEST_PHYSICAL); +} + +static void +__send_IPI_mask(const struct cpumask *mask, int vector, int apic_dest) +{ + unsigned long query_cpu; + unsigned long this_cpu; + unsigned long flags; + + /* x2apic MSRs are special and need a special fence: */ + weak_wrmsr_fence(); + + local_irq_save(flags); + + this_cpu = smp_processor_id(); + for_each_cpu(query_cpu, mask) { + if (apic_dest == APIC_DEST_ALLBUT && this_cpu == query_cpu) + continue; + __x2apic_send_IPI_dest(per_cpu(x86_cpu_to_apicid, query_cpu), + vector, APIC_DEST_PHYSICAL); + } + local_irq_restore(flags); +} + +static void x2apic_savic_send_IPI_mask(const struct cpumask *mask, int vector) +{ + __send_IPI_mask(mask, vector, APIC_DEST_ALLINC); +} + +static void x2apic_savic_send_IPI_mask_allbutself(const struct cpumask *mask, int vector) +{ + __send_IPI_mask(mask, vector, APIC_DEST_ALLBUT); +} + +static int x2apic_savic_probe(void) +{ + if (!cc_platform_has(CC_ATTR_SNP_SECURE_AVIC)) + return 0; + + if (!x2apic_mode) { + pr_err("Secure AVIC enabled in non x2APIC mode\n"); + snp_abort(); + } + + pr_info("Secure AVIC Enabled\n"); + + return 1; +} + +static struct apic apic_x2apic_savic __ro_after_init = { + + .name = "secure avic x2apic", + .probe = x2apic_savic_probe, + .acpi_madt_oem_check = x2apic_savic_acpi_madt_oem_check, + + .dest_mode_logical = false, + + .disable_esr = 0, + + .cpu_present_to_apicid = default_cpu_present_to_apicid, + + .max_apic_id = UINT_MAX, + .x2apic_set_max_apicid = true, + .get_apic_id = x2apic_get_apic_id, + + .calc_dest_apicid = apic_default_calc_apicid, + + .send_IPI = x2apic_savic_send_IPI, + .send_IPI_mask = x2apic_savic_send_IPI_mask, + .send_IPI_mask_allbutself = x2apic_savic_send_IPI_mask_allbutself, + .send_IPI_allbutself = x2apic_send_IPI_allbutself, + .send_IPI_all = x2apic_send_IPI_all, + .send_IPI_self = x2apic_send_IPI_self, + .nmi_to_offline_cpu = true, + + .read = native_apic_msr_read, + .write = native_apic_msr_write, + .eoi = native_apic_msr_eoi, + .icr_read = native_x2apic_icr_read, + .icr_write = native_x2apic_icr_write, +}; + +apic_driver(apic_x2apic_savic); diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h index caa4b4430634..801208678450 100644 --- a/include/linux/cc_platform.h +++ b/include/linux/cc_platform.h @@ -88,6 +88,14 @@ enum cc_attr { * enabled to run SEV-SNP guests. */ CC_ATTR_HOST_SEV_SNP, + + /** + * @CC_ATTR_SNP_SECURE_AVIC: Secure AVIC mode is active. + * + * The host kernel is running with the necessary features enabled + * to run SEV-SNP guests with full Secure AVIC capabilities. + */ + CC_ATTR_SNP_SECURE_AVIC, }; #ifdef CONFIG_ARCH_HAS_CC_PLATFORM