Mailing List Archive

1 2 3  View All
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Fri, Feb 10, 2023, Michael Kelley (LINUX) wrote:
> From: Sean Christopherson <seanjc@google.com> Sent: Friday, February 10, 2023 12:58 PM
> >
> > On Fri, Feb 10, 2023, Sean Christopherson wrote:
> > > On Fri, Feb 10, 2023, Dave Hansen wrote:
> > > > On 2/10/23 11:36, Borislav Petkov wrote:
> > > > >> One approach is to go with the individual device attributes for now.>> If the list
> > does grow significantly, there will probably be patterns
> > > > >> or groupings that we can't discern now. We could restructure into
> > > > >> larger buckets at that point based on those patterns/groupings.
> > > > > There's a reason the word "platform" is in cc_platform_has(). Initially
> > > > > we wanted to distinguish attributes of the different platforms. So even
> > > > > if y'all don't like CC_ATTR_PARAVISOR, that is what distinguishes this
> > > > > platform and it *is* one platform.
> > > > >
> > > > > So call it CC_ATTR_SEV_VTOM as it uses that technology or whatever. But
> > > > > call it like the platform, not to mean "I need this functionality".
> > > >
> > > > I can live with that. There's already a CC_ATTR_GUEST_SEV_SNP, so it
> > > > would at least not be too much of a break from what we already have.
> > >
> > > I'm fine with CC_ATTR_SEV_VTOM, assuming the proposal is to have something like:
> > >
> > > static inline bool is_address_range_private(resource_size_t addr)
> > > {
> > > if (cc_platform_has(CC_ATTR_SEV_VTOM))
> > > return is_address_below_vtom(addr);
> > >
> > > return false;
> > > }
> > >
> > > i.e. not have SEV_VTOM mean "I/O APIC and vTPM are private". Though I don't see
> > > the point in making it SEV vTOM specific or using a flag. Despite what any of us
> > > think about TDX paravisors, it's completely doable within the confines of TDX to
> > > have an emulated device reside in the private address space. E.g. why not
> > > something like this?
> > >
> > > static inline bool is_address_range_private(resource_size_t addr)
> > > {
> > > return addr < cc_platform_private_end;
> > > }
> > >
> > > where SEV fills in "cc_platform_private_end" when vTOM is enabled, and TDX does
> > > the same. Or wrap cc_platform_private_end in a helper, etc.
> >
> > Gah, forgot that the intent with TDX is to enumerate devices in their legacy
> > address spaces. So a TDX guest couldn't do this by default, but if/when Hyper-V
> > or some other hypervisor moves I/O APIC, vTPM, etc... into the TCB, the common
> > code would just work and only the hypervisor-specific paravirt code would need
> > to change.
> >
> > Probably need a more specific name than is_address_range_private() though, e.g.
> > is_mmio_address_range_private()?
>
> Maybe I'm not understanding what you are proposing, but in an SEV-SNP
> VM using vTOM, devices like the IO-APIC and TPM live at their usual guest
> physical addresses.

Ah, so as the cover letter says, the intent really is to treat vTOM as an
attribute bit. Sorry, I got confused by Boris's comment:

: What happens if the next silly HV guest scheme comes along and they do
: need more and different ones?

Based on that comment, I assumed the proposal to use CC_ATTR_SEV_VTOM was intended
to be a generic range-based thing, but it sounds like that's not the case.

IMO, using CC_ATTR_SEV_VTOM to infer anything about the state of I/O APIC or vTPM
is wrong. vTOM as a platform feature effectively says "physical address bit X
controls private vs. shared" (ignoring weird usage of vTOM). vTOM does not mean
I/O APIC and vTPM are private, that's very much a property of Hyper-V's current
generation of vTOM-based VMs.

Hardcoding this in the guest feels wrong. Ideally, we would have a way to enumerate
that a device is private/trusted, e.g. through ACPI. I'm guessing we already
missed the boat on that, so the next best thing is to do something like Michael
originally proposed in this patch and shove the "which devices are private" logic
into hypervisor-specific code, i.e. let Hyper-V figure out how to enumerate to its
guests which devices are shared.

I agree with Boris' comment that a one-off "other encrypted range" is a hack, but
that's just an API problem. The kernel already has hypervisor specific hooks (and
for SEV-ES even), why not expand that? That way figuring out which devices are
private is wholly contained in Hyper-V code, at least until there's a generic
solution for enumerating private devices, though that seems unlikely to happen
and will be a happy problem to solve if it does come about.

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index a868b76cd3d4..08f65ed439d9 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2682,11 +2682,16 @@ static void io_apic_set_fixmap(enum fixed_addresses idx, phys_addr_t phys)
{
pgprot_t flags = FIXMAP_PAGE_NOCACHE;

- /*
- * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
- * bits, just like normal ioremap():
- */
- flags = pgprot_decrypted(flags);
+ if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
+ /*
+ * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
+ * bits, just like normal ioremap():
+ */
+ if (x86_platform.hyper.is_private_mmio(phys))
+ flags = pgprot_encrypted(flags);
+ else
+ flags = pgprot_decrypted(flags);
+ }

__set_fixmap(idx, phys, flags);
}
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 6453fbaedb08..0baec766b921 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -116,6 +116,9 @@ static void __ioremap_check_other(resource_size_t addr, struct ioremap_desc *des
if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
return;

+ if (x86_platform.hyper.is_private_mmio(addr))
+ desc->flags |= IORES_MAP_ENCRYPTED;
+
if (!IS_ENABLED(CONFIG_EFI))
return;
RE: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
From: Sean Christopherson <seanjc@google.com> Sent: Friday, February 10, 2023 3:47 PM
>
> On Fri, Feb 10, 2023, Michael Kelley (LINUX) wrote:
> > From: Sean Christopherson <seanjc@google.com> Sent: Friday, February 10, 2023
> 12:58 PM
> > >
> > > On Fri, Feb 10, 2023, Sean Christopherson wrote:
> > > > On Fri, Feb 10, 2023, Dave Hansen wrote:
> > > > > On 2/10/23 11:36, Borislav Petkov wrote:
> > > > > >> One approach is to go with the individual device attributes for now.>> If the list
> > > does grow significantly, there will probably be patterns
> > > > > >> or groupings that we can't discern now. We could restructure into
> > > > > >> larger buckets at that point based on those patterns/groupings.
> > > > > > There's a reason the word "platform" is in cc_platform_has(). Initially
> > > > > > we wanted to distinguish attributes of the different platforms. So even
> > > > > > if y'all don't like CC_ATTR_PARAVISOR, that is what distinguishes this
> > > > > > platform and it *is* one platform.
> > > > > >
> > > > > > So call it CC_ATTR_SEV_VTOM as it uses that technology or whatever. But
> > > > > > call it like the platform, not to mean "I need this functionality".
> > > > >
> > > > > I can live with that. There's already a CC_ATTR_GUEST_SEV_SNP, so it
> > > > > would at least not be too much of a break from what we already have.
> > > >
> > > > I'm fine with CC_ATTR_SEV_VTOM, assuming the proposal is to have something
> like:
> > > >
> > > > static inline bool is_address_range_private(resource_size_t addr)
> > > > {
> > > > if (cc_platform_has(CC_ATTR_SEV_VTOM))
> > > > return is_address_below_vtom(addr);
> > > >
> > > > return false;
> > > > }
> > > >
> > > > i.e. not have SEV_VTOM mean "I/O APIC and vTPM are private". Though I don't
> see
> > > > the point in making it SEV vTOM specific or using a flag. Despite what any of us
> > > > think about TDX paravisors, it's completely doable within the confines of TDX to
> > > > have an emulated device reside in the private address space. E.g. why not
> > > > something like this?
> > > >
> > > > static inline bool is_address_range_private(resource_size_t addr)
> > > > {
> > > > return addr < cc_platform_private_end;
> > > > }
> > > >
> > > > where SEV fills in "cc_platform_private_end" when vTOM is enabled, and TDX does
> > > > the same. Or wrap cc_platform_private_end in a helper, etc.
> > >
> > > Gah, forgot that the intent with TDX is to enumerate devices in their legacy
> > > address spaces. So a TDX guest couldn't do this by default, but if/when Hyper-V
> > > or some other hypervisor moves I/O APIC, vTPM, etc... into the TCB, the common
> > > code would just work and only the hypervisor-specific paravirt code would need
> > > to change.
> > >
> > > Probably need a more specific name than is_address_range_private() though, e.g.
> > > is_mmio_address_range_private()?
> >
> > Maybe I'm not understanding what you are proposing, but in an SEV-SNP
> > VM using vTOM, devices like the IO-APIC and TPM live at their usual guest
> > physical addresses.
>
> Ah, so as the cover letter says, the intent really is to treat vTOM as an
> attribute bit. Sorry, I got confused by Boris's comment:
>
> : What happens if the next silly HV guest scheme comes along and they do
> : need more and different ones?
>
> Based on that comment, I assumed the proposal to use CC_ATTR_SEV_VTOM was
> intended
> to be a generic range-based thing, but it sounds like that's not the case.
>
> IMO, using CC_ATTR_SEV_VTOM to infer anything about the state of I/O APIC or vTPM
> is wrong. vTOM as a platform feature effectively says "physical address bit X
> controls private vs. shared" (ignoring weird usage of vTOM). vTOM does not mean
> I/O APIC and vTPM are private, that's very much a property of Hyper-V's current
> generation of vTOM-based VMs.
>
> Hardcoding this in the guest feels wrong. Ideally, we would have a way to enumerate
> that a device is private/trusted, e.g. through ACPI. I'm guessing we already
> missed the boat on that, so the next best thing is to do something like Michael
> originally proposed in this patch and shove the "which devices are private" logic
> into hypervisor-specific code, i.e. let Hyper-V figure out how to enumerate to its
> guests which devices are shared.
>
> I agree with Boris' comment that a one-off "other encrypted range" is a hack, but
> that's just an API problem. The kernel already has hypervisor specific hooks (and
> for SEV-ES even), why not expand that? That way figuring out which devices are
> private is wholly contained in Hyper-V code, at least until there's a generic
> solution for enumerating private devices, though that seems unlikely to happen
> and will be a happy problem to solve if it does come about.

Yes, this is definitely a cleaner way to implement what I was originally proposing.
I can make it work if there's agreement to take this approach.

Michael

>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index a868b76cd3d4..08f65ed439d9 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -2682,11 +2682,16 @@ static void io_apic_set_fixmap(enum fixed_addresses idx,
> phys_addr_t phys)
> {
> pgprot_t flags = FIXMAP_PAGE_NOCACHE;
>
> - /*
> - * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
> - * bits, just like normal ioremap():
> - */
> - flags = pgprot_decrypted(flags);
> + if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
> + /*
> + * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
> + * bits, just like normal ioremap():
> + */
> + if (x86_platform.hyper.is_private_mmio(phys))
> + flags = pgprot_encrypted(flags);
> + else
> + flags = pgprot_decrypted(flags);
> + }
>
> __set_fixmap(idx, phys, flags);
> }
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 6453fbaedb08..0baec766b921 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -116,6 +116,9 @@ static void __ioremap_check_other(resource_size_t addr, struct
> ioremap_desc *des
> if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> return;
>
> + if (x86_platform.hyper.is_private_mmio(addr))
> + desc->flags |= IORES_MAP_ENCRYPTED;
> +
> if (!IS_ENABLED(CONFIG_EFI))
> return;
>
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Fri, Feb 10, 2023 at 11:47:27PM +0000, Sean Christopherson wrote:
> I agree with Boris' comment that a one-off "other encrypted range" is a hack, but
> that's just an API problem. The kernel already has hypervisor specific hooks (and
> for SEV-ES even), why not expand that? That way figuring out which devices are
> private is wholly contained in Hyper-V code, at least until there's a generic
> solution for enumerating private devices, though that seems unlikely to happen
> and will be a happy problem to solve if it does come about.

I feel ya and this all makes sense and your proposals look clean enough
to me but we still need some way of determining whether this is a vTOM
on hyperv because there's the next crapola with

https://lore.kernel.org/r/20230209072220.6836-4-jgross@suse.com

because apparently hyperv does PAT but disables MTRRs for such vTOM
SEV-SNP guests and ... madness.

But that's not the only example - Xen has been doing this thing too.

And Jürgen has been trying to address this in a clean way but it is
a pain.

What I don't want to have is a gazillion ways to check what needs to
happen for which guest type. Because people who change the kernel to run
on baremetal, will break them. And I can't blame them. We try to support
all kinds of guests in the x86 code but this support should be plain and
simple.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette
RE: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
From: Borislav Petkov <bp@alien8.de> Sent: Thursday, February 16, 2023 5:33 AM
>
> On Fri, Feb 10, 2023 at 11:47:27PM +0000, Sean Christopherson wrote:
> > I agree with Boris' comment that a one-off "other encrypted range" is a hack, but
> > that's just an API problem. The kernel already has hypervisor specific hooks (and
> > for SEV-ES even), why not expand that? That way figuring out which devices are
> > private is wholly contained in Hyper-V code, at least until there's a generic
> > solution for enumerating private devices, though that seems unlikely to happen
> > and will be a happy problem to solve if it does come about.
>
> I feel ya and this all makes sense and your proposals look clean enough
> to me but we still need some way of determining whether this is a vTOM
> on hyperv

Historically, callbacks like Sean proposed default to NULL and do nothing
unless they are explicitly set. The Hyper-V vTOM code would set the callback.
Is that not sufficient? Or in the two places where the callback would
be made, do you want to bracket with a test for being in a Hyper-V vTOM
VM? If so, then we're back to needing something like CC_ATTR_PARAVISOR
on which to gate the callbacks.

Or do you mean something else entirely?

Michael

> because there's the next crapola with
>
> https://lore.kernel.org/all/20230209072220.6836-4-jgross@suse.com/
>
> because apparently hyperv does PAT but disables MTRRs for such vTOM
> SEV-SNP guests and ... madness.
>
> But that's not the only example - Xen has been doing this thing too.
>
> And J?rgen has been trying to address this in a clean way but it is
> a pain.
>
> What I don't want to have is a gazillion ways to check what needs to
> happen for which guest type. Because people who change the kernel to run
> on baremetal, will break them. And I can't blame them. We try to support
> all kinds of guests in the x86 code but this support should be plain and
> simple.
>
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Thu, Feb 16, 2023 at 04:16:16PM +0000, Michael Kelley (LINUX) wrote:
> Historically, callbacks like Sean proposed default to NULL and do nothing
> unless they are explicitly set. The Hyper-V vTOM code would set the callback.
> Is that not sufficient? Or in the two places where the callback would
> be made, do you want to bracket with a test for being in a Hyper-V vTOM
> VM? If so, then we're back to needing something like CC_ATTR_PARAVISOR
> on which to gate the callbacks.
>
> Or do you mean something else entirely?

See the second part of my reply.

This thing...

> > because there's the next crapola with
> >
> > https://lore.kernel.org/all/20230209072220.6836-4-jgross@suse.com/
> >
> > because apparently hyperv does PAT but disables MTRRs for such vTOM
> > SEV-SNP guests and ... madness.
> >
> > But that's not the only example - Xen has been doing this thing too.
> >
> > And Jürgen has been trying to address this in a clean way but it is
> > a pain.
> >
> > What I don't want to have is a gazillion ways to check what needs to
> > happen for which guest type. Because people who change the kernel to run
> > on baremetal, will break them. And I can't blame them. We try to support
> > all kinds of guests in the x86 code but this support should be plain and
> > simple.

... here.

We need a single way to test for this guest type and stick with it.

I'd like for all guest types we support to be queried in a plain and
simple way.

Not:

* CC_ATTR_GUEST_MEM_ENCRYPT

* x86_platform.hyper.is_private_mmio(addr)

* CC_ATTR_PARAVISOR

to mean three different aspects of SEV-SNP guests using vTOM on Hyper-V.

This is going to be a major mess which we won't support.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette
RE: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
From: Borislav Petkov <bp@alien8.de> Sent: Thursday, February 16, 2023 9:07 AM
>
> ... here.
>
> We need a single way to test for this guest type and stick with it.
>
> I'd like for all guest types we support to be queried in a plain and
> simple way.
>
> Not:
>
> * CC_ATTR_GUEST_MEM_ENCRYPT
>
> * x86_platform.hyper.is_private_mmio(addr)
>
> * CC_ATTR_PARAVISOR
>
> to mean three different aspects of SEV-SNP guests using vTOM on Hyper-V.
>
> This is going to be a major mess which we won't support.
>

OK, I'm trying to figure out where to go next. I've been following the pattern
set by the SEV/SEV-ES/SEV-SNP and TDX platforms in the cc_platform_has()
function. Each platform returns "true" for multiple CC_ATTR_* values,
and those CC_ATTR_* values are tested in multiple places throughout
kernel code. Some CC_ATTR_* values are shared by multiple platforms
(like CC_ATTR_GUEST_MEM_ENCRYPT) and some are unique to a particular
platform (like CC_ATTR_HOTPLUG_DISABLED). For the CC_ATTR_* values
that are shared, the logic of which platforms they apply to occurs once in
cc_platform_has() rather than scattered and duplicated throughout the
kernel, which makes sense. Any given platform is not represented by a
single CC_ATTR_* value, but by multiple ones. Each CC_ATTR_* value
essentially represents a chunk of kernel functionality that one or more
platforms need, and the platform indicates its need by cc_platform_has()
returning "true" for that value.

So I've been trying to apply the same pattern to the SNP vTOM sub-case
of SEV-SNP. Is that consistent with your thinking, or is the whole
cc_platform_has() approach problematic, including for the existing
SEV flavors and for TDX?

Michael
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Fri, Feb 17, 2023 at 06:16:56AM +0000, Michael Kelley (LINUX) wrote:
> Is that consistent with your thinking, or is the whole
> cc_platform_has() approach problematic, including for the existing SEV
> flavors and for TDX?

The confidential computing attributes are, yes, features. I've been
preaching since the very beginning that vTOM *is* *also* one such
feature. It is a feature bit in sev_features, for chrissakes. So by that
logic, those SEV-SNP HyperV guests should return true when

cc_platform_has(CC_ATTR_GUEST_SEV_SNP_VTOM);

is tested.

But Sean doesn't like that.

If the access method to the IO-APIC and vTPM are specific to the
HyperV's vTOM implementation, then I don't mind if this were called

cc_platform_has(CC_ATTR_GUEST_HYPERV_VTOM);

Frankly, I don't see any other enlightened guest using vTOM except
HyperV's but virt folks have managed to surprise me in the past too.

In any case, a single flag which is specific to that guest type is fine
too.

It feels like we're running in circles by now... ;-\

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Fri, Feb 17, 2023, Borislav Petkov wrote:
> On Fri, Feb 17, 2023 at 06:16:56AM +0000, Michael Kelley (LINUX) wrote:
> > Is that consistent with your thinking, or is the whole
> > cc_platform_has() approach problematic, including for the existing SEV
> > flavors and for TDX?
>
> The confidential computing attributes are, yes, features. I've been
> preaching since the very beginning that vTOM *is* *also* one such
> feature. It is a feature bit in sev_features, for chrissakes. So by that
> logic, those SEV-SNP HyperV guests should return true when
>
> cc_platform_has(CC_ATTR_GUEST_SEV_SNP_VTOM);
>
> is tested.
>
> But Sean doesn't like that.

Because vTOM is a hardware feature, whereas the IO-APIC and vTPM being accessible
via private memory are software features. It's very possible to emulate the
IO-APIC in trusted code without vTOM.

> If the access method to the IO-APIC and vTPM are specific to the
> HyperV's vTOM implementation, then I don't mind if this were called
>
> cc_platform_has(CC_ATTR_GUEST_HYPERV_VTOM);

I still think that's likely to caused problems in the future, e.g. if Hyper-V
moves more stuff into the paravisor or if Hyper-V ends up with similar functionality
for TDX. But it's not a sticking point, the only thing I'm fiercely resistant to
is conflating hardware features with software features.
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Wed, Feb 22, 2023 at 02:13:44PM -0800, Sean Christopherson wrote:
> Because vTOM is a hardware feature, whereas the IO-APIC and vTPM being accessible
> via private memory are software features. It's very possible to emulate the
> IO-APIC in trusted code without vTOM.

I know, but their use case is dictated by the fact that they're using
a SNP guest *with* vTOM as a SEV feature. And so their guest does
IO-APIC and vTPM *with* the vTOM SEV feature. That's what I'm trying to
model.

> > If the access method to the IO-APIC and vTPM are specific to the
> > HyperV's vTOM implementation, then I don't mind if this were called
> >
> > cc_platform_has(CC_ATTR_GUEST_HYPERV_VTOM);
>
> I still think that's likely to caused problems in the future, e.g. if Hyper-V
> moves more stuff into the paravisor or if Hyper-V ends up with similar functionality
> for TDX.

Yah, reportedly, TDX folks are not very interested in this case.

> But it's not a sticking point, the only thing I'm fiercely resistant to
> is conflating hardware features with software features.

So you and I need to find a common ground...

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Wed, Feb 22, 2023, Borislav Petkov wrote:
> On Wed, Feb 22, 2023 at 02:13:44PM -0800, Sean Christopherson wrote:
> > Because vTOM is a hardware feature, whereas the IO-APIC and vTPM being accessible
> > via private memory are software features. It's very possible to emulate the
> > IO-APIC in trusted code without vTOM.
>
> I know, but their use case is dictated by the fact that they're using
> a SNP guest *with* vTOM as a SEV feature. And so their guest does
> IO-APIC and vTPM *with* the vTOM SEV feature. That's what I'm trying to
> model.

Why? I genuinely don't understand the motivation for bundling all of this stuff
under a single "feature". To me, that's like saying Haswell or Zen2 is a "feature",
but outside of a very few cases where the exact uarch truly matters, nothing pivots
on FMS because the CPU type is not a single feature.
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Wed, Feb 22, 2023 at 02:54:47PM -0800, Sean Christopherson wrote:
> Why? I genuinely don't understand the motivation for bundling all of this stuff
> under a single "feature".

It is called "sanity".

See here:

https://lore.kernel.org/r/Y%2B5immKTXCsjSysx@zn.tnic

We support SEV, SEV-ES, SEV-SNP, TDX, HyperV... guests and whatever's
coming down the pipe. And all that goes into arch/x86/ kernel proper
code.

The CC_ATTR stuff is clean-ish in the sense that we have separation by
confidential computing platform - AMD's and Intel's. Hyper-V comes along
and wants to define a different subset of that. And that's only the
SEV-SNP side - there's a TDX patchset too.

And then some other hypervisor will come along and say, but but, I wanna
have X and Y and a pink pony too.

Oh, and there's this other fun with MTRRs where each HV decides to do
whatever it wants.

So, we have a zoo brewing on the horizon already!

If there's no clean definition of what each guest is and requires and
that stuff isn't documented properly and if depending on which "feature"
I need to check, I need to call a different function or query
a different variable, then it won't go anywhere as far as guest support
goes.

The cc_platform_has() thing gives us a relatively clean way to abstract
all those differences away and keep the code sane-ish.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Thu, Feb 23, 2023, Borislav Petkov wrote:
> On Wed, Feb 22, 2023 at 02:54:47PM -0800, Sean Christopherson wrote:
> > Why? I genuinely don't understand the motivation for bundling all of this stuff
> > under a single "feature".
>
> It is called "sanity".
>
> See here:
>
> https://lore.kernel.org/r/Y%2B5immKTXCsjSysx@zn.tnic
>
> We support SEV, SEV-ES, SEV-SNP, TDX, HyperV... guests and whatever's
> coming down the pipe. And all that goes into arch/x86/ kernel proper
> code.
>
> The CC_ATTR stuff is clean-ish in the sense that we have separation by
> confidential computing platform - AMD's and Intel's. Hyper-V comes along
> and wants to define a different subset of that. And that's only the
> SEV-SNP side - there's a TDX patchset too.
>
> And then some other hypervisor will come along and say, but but, I wanna
> have X and Y and a pink pony too.
>
> Oh, and there's this other fun with MTRRs where each HV decides to do
> whatever it wants.

The MTRR mess isn't unique to coco guests, e.g. KVM explicitly "supports" VMMs
hiding MTTRs from the guest by defaulting to WB if MTTRs aren't exposed to the
guest. Why on earth Hyper-V suddenly needs to enlighten the guest is beyond me,
but whatever the reason, it's not unique to coco VMs.

> So, we have a zoo brewing on the horizon already!
>
> If there's no clean definition of what each guest is and requires and
> that stuff isn't documented properly and if depending on which "feature"
> I need to check, I need to call a different function or query
> a different variable, then it won't go anywhere as far as guest support
> goes.
>
> The cc_platform_has() thing gives us a relatively clean way to abstract
> all those differences away and keep the code sane-ish.

For features that are inherent to the platform, I agree, or at least I don't hate
the interface. But defining a platform to have specific devices runs counter to
pretty much the entire x86 ecosystem. At some point, there _will_ be more devices
in private memory than just IO-APIC and TPM, and conversely there will be "platforms"
that support a trusted TPM but not a trusted IO-APIC, and probably even vice versa.

All I'm advocating is that for determining whether or not a device should be mapped
private vs. shared, provide an API so that the hypervisor-specific enlightened code
can manage that insanity without polluting common code. If we are ever fortunate
enough to have common enumeration, e.g. through ACPI or something, the enlightened
code can simply reroute to the common code. This is a well established pattern for
many paravirt features, I don't see why it wouldn't work here.
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Wed, Feb 22, 2023 at 05:21:27PM -0800, Sean Christopherson wrote:
> The MTRR mess isn't unique to coco guests, e.g. KVM explicitly "supports" VMMs
> hiding MTTRs from the guest by defaulting to WB if MTTRs aren't exposed to the
> guest. Why on earth Hyper-V suddenly needs to enlighten the guest is beyond me,
> but whatever the reason, it's not unique to coco VMs.

Well, TDX can't stomach MTRRs either, reportedly, and I hear we should
try to avoid #VEs for them too.

And this is the problem: all those guest "enlightening" efforts come up
with the weirdest stuff they need to sprinkle around arch/x86/. And if
we let that without paying attention to the big picture, that will
become an unmaintanable mess.

And I'm not proud of some of the stuff we did in arch/x86/ already and
some day they'll get on my nerves just enough...

> All I'm advocating is that for determining whether or not a device should be mapped
> private vs. shared, provide an API so that the hypervisor-specific enlightened code
> can manage that insanity without polluting common code. If we are ever fortunate
> enough to have common enumeration, e.g. through ACPI or something, the enlightened
> code can simply reroute to the common code. This is a well established pattern for
> many paravirt features, I don't see why it wouldn't work here.

Yah, that would be good. If the device can know upfront how it needs to
ioremap its address range, then that is fine - we already have
ioremap_encrypted() for example.

What I don't like is hooking conditionals into the common code to figure
out what to do depending on what we're running on.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette
RE: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
From: Borislav Petkov <bp@alien8.de> Sent: Thursday, February 23, 2023 2:45 AM
>
> On Wed, Feb 22, 2023 at 05:21:27PM -0800, Sean Christopherson wrote:
>
> > All I'm advocating is that for determining whether or not a device should be mapped
> > private vs. shared, provide an API so that the hypervisor-specific enlightened code
> > can manage that insanity without polluting common code. If we are ever fortunate
> > enough to have common enumeration, e.g. through ACPI or something, the enlightened
> > code can simply reroute to the common code. This is a well established pattern for
> > many paravirt features, I don't see why it wouldn't work here.
>
> Yah, that would be good.

Just so I'm clear, are you saying you are good with the proposal that Sean
sketched out with code here? [1] With his proposal, the device driver
is not involved in deciding whether to map encrypted or decrypted. That
decision is made in the hypervisor-specific callback function based on
the physical address. The callback has to be made in two places in common
code. But then as Sean said, " the hypervisor-specific enlightened code
can manage that insanity without polluting common code". :-)

I like Sean's proposal, so if you are good with it, I'll do v6 of the patch
set with that approach.

Dave Hansen: Are you also OK with Sean's proposal? Looking for consensus
here ....

> If the device can know upfront how it needs to
> ioremap its address range, then that is fine - we already have
> ioremap_encrypted() for example.
>

Again, the device driver would not be involved in Sean's proposal.

Michael

[1] https://lore.kernel.org/linux-hyperv/Y+bXjxUtSf71E5SS@google.com/
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On 2/10/23 15:47, Sean Christopherson wrote:
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index a868b76cd3d4..08f65ed439d9 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -2682,11 +2682,16 @@ static void io_apic_set_fixmap(enum fixed_addresses idx, phys_addr_t phys)
> {
> pgprot_t flags = FIXMAP_PAGE_NOCACHE;
>
> - /*
> - * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
> - * bits, just like normal ioremap():
> - */
> - flags = pgprot_decrypted(flags);
> + if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
> + /*
> + * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
> + * bits, just like normal ioremap():
> + */
> + if (x86_platform.hyper.is_private_mmio(phys))
> + flags = pgprot_encrypted(flags);
> + else
> + flags = pgprot_decrypted(flags);
> + }

I don't completely hate this. Thinking to the future, I'd hope that
future platforms will include information about which physical addresses
are shared or private. This might even vary per device, but this
interface would still work.

I _think_ it would be nicer to wrap both checks up in a helper where the
comments can be more detailed, like:

if (cc_private_mmio(phys))
flags = pgprot_encrypted(flags);
else
flags = pgprot_decrypted(flags);

but I honestly don't feel that strongly about it.

It does seem a bit odd that there's a new CC_ATTR_GUEST_MEM_ENCRYPT
check wrapping this whole thing. I guess the trip through
pgprot_decrypted() is harmless on normal platforms, though.

> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 6453fbaedb08..0baec766b921 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -116,6 +116,9 @@ static void __ioremap_check_other(resource_size_t addr, struct ioremap_desc *des
> if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
> return;
>
> + if (x86_platform.hyper.is_private_mmio(addr))
> + desc->flags |= IORES_MAP_ENCRYPTED;
> +
> if (!IS_ENABLED(CONFIG_EFI))
> return;
>
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On 2/23/23 12:01, Michael Kelley (LINUX) wrote:
> Dave Hansen: Are you also OK with Sean's proposal? Looking for consensus
> here ....

Yeah, I'm generally OK with it as long as Borislav is.
RE: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
From: Dave Hansen <dave.hansen@intel.com> Sent: Thursday, February 23, 2023 12:42 PM
>
> On 2/23/23 12:26, Dave Hansen wrote:
> >> + if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
> >> + /*
> >> + * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
> >> + * bits, just like normal ioremap():
> >> + */
> >> + if (x86_platform.hyper.is_private_mmio(phys))
> >> + flags = pgprot_encrypted(flags);
> >> + else
> >> + flags = pgprot_decrypted(flags);
> >> + }
> ...
> > It does seem a bit odd that there's a new CC_ATTR_GUEST_MEM_ENCRYPT
> > check wrapping this whole thing. I guess the trip through
> > pgprot_decrypted() is harmless on normal platforms, though.
>
> Yeah, that's _really_ odd. Sean, were you trying to optimize away the
> indirect call or something?
>
> I would just expect the Hyper-V/vTOM code to leave
> x86_platform.hyper.is_private_mmio alone unless
> it *knows* the platform has private MMIO *and* CC_ATTR_GUEST_MEM_ENCRYPT.

Agreed.

>
> Is there ever a case where CC_ATTR_GUEST_MEM_ENCRYPT==0 and he
> Hyper-V/vTOM code would need to set x86_platform.hyper.is_private_mmio?

There's no such case.

I agree that gating with CC_ATTR_GUEST_MEM_ENCRYPT isn't really necessary.
Current upstream code always does the pgprot_decrypted(), and as you said,
that's a no-op on platforms with no memory encryption.

Michael
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On 2/23/23 12:26, Dave Hansen wrote:
>> + if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
>> + /*
>> + * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
>> + * bits, just like normal ioremap():
>> + */
>> + if (x86_platform.hyper.is_private_mmio(phys))
>> + flags = pgprot_encrypted(flags);
>> + else
>> + flags = pgprot_decrypted(flags);
>> + }
...
> It does seem a bit odd that there's a new CC_ATTR_GUEST_MEM_ENCRYPT
> check wrapping this whole thing. I guess the trip through
> pgprot_decrypted() is harmless on normal platforms, though.

Yeah, that's _really_ odd. Sean, were you trying to optimize away the
indirect call or something?

I would just expect the Hyper-V/vTOM code to leave
x86_platform.hyper.is_private_mmio alone unless
it *knows* the platform has private MMIO *and* CC_ATTR_GUEST_MEM_ENCRYPT.

Is there ever a case where CC_ATTR_GUEST_MEM_ENCRYPT==0 and he
Hyper-V/vTOM code would need to set x86_platform.hyper.is_private_mmio?
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Thu, Feb 23, 2023, Michael Kelley (LINUX) wrote:
> From: Dave Hansen <dave.hansen@intel.com> Sent: Thursday, February 23, 2023 12:42 PM
> >
> > On 2/23/23 12:26, Dave Hansen wrote:
> > >> + if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
> > >> + /*
> > >> + * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
> > >> + * bits, just like normal ioremap():
> > >> + */
> > >> + if (x86_platform.hyper.is_private_mmio(phys))
> > >> + flags = pgprot_encrypted(flags);
> > >> + else
> > >> + flags = pgprot_decrypted(flags);
> > >> + }
> > ...
> > > It does seem a bit odd that there's a new CC_ATTR_GUEST_MEM_ENCRYPT
> > > check wrapping this whole thing. I guess the trip through
> > > pgprot_decrypted() is harmless on normal platforms, though.
> >
> > Yeah, that's _really_ odd. Sean, were you trying to optimize away the
> > indirect call or something?

No, my thought was simply to require platforms that support GUEST_MEM_ENCRYPT to
implement x86_platform.hyper.is_private_mmio, e.g. to avoid having to check if
is_private_mmio is NULL, to explicit document that non-Hyper-V encrypted guests
don't (yet) support private MMIO, and to add a bit of documentation around the
{de,en}crypted logic.

> > I would just expect the Hyper-V/vTOM code to leave
> > x86_platform.hyper.is_private_mmio alone unless it *knows* the platform has
> > private MMIO *and* CC_ATTR_GUEST_MEM_ENCRYPT.
>
> Agreed.
>
> >
> > Is there ever a case where CC_ATTR_GUEST_MEM_ENCRYPT==0 and he
> > Hyper-V/vTOM code would need to set x86_platform.hyper.is_private_mmio?
>
> There's no such case.
>
> I agree that gating with CC_ATTR_GUEST_MEM_ENCRYPT isn't really necessary.
> Current upstream code always does the pgprot_decrypted(), and as you said,
> that's a no-op on platforms with no memory encryption.

Right, but since is_private_mmio can be NULL, unless I'm missing something we'll
need an extra check no matter what, i.e. the alternative would be

if (x86_platform.hyper.is_private_mmio &&
x86_platform.hyper.is_private_mmio(phys))
flags = pgprot_encrypted(flags);
else
flags = pgprot_decrypted(flags);

I have no objection to that approach. It does have the advantage of not needing
an indirect call for encrypted guests that don't support private MMIO, though
I can't imagine this code is performance sensitive.
RE: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
From: Sean Christopherson <seanjc@google.com> Sent: Thursday, February 23, 2023 1:08 PM
>
> On Thu, Feb 23, 2023, Michael Kelley (LINUX) wrote:
> > From: Dave Hansen <dave.hansen@intel.com> Sent: Thursday, February 23, 2023
> 12:42 PM
> > >
> > > On 2/23/23 12:26, Dave Hansen wrote:
> > > >> + if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
> > > >> + /*
> > > >> + * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot
> > > >> + * bits, just like normal ioremap():
> > > >> + */
> > > >> + if (x86_platform.hyper.is_private_mmio(phys))
> > > >> + flags = pgprot_encrypted(flags);
> > > >> + else
> > > >> + flags = pgprot_decrypted(flags);
> > > >> + }
> > > ...
> > > > It does seem a bit odd that there's a new CC_ATTR_GUEST_MEM_ENCRYPT
> > > > check wrapping this whole thing. I guess the trip through
> > > > pgprot_decrypted() is harmless on normal platforms, though.
> > >
> > > Yeah, that's _really_ odd. Sean, were you trying to optimize away the
> > > indirect call or something?
>
> No, my thought was simply to require platforms that support GUEST_MEM_ENCRYPT
> to
> implement x86_platform.hyper.is_private_mmio, e.g. to avoid having to check if
> is_private_mmio is NULL, to explicit document that non-Hyper-V encrypted guests
> don't (yet) support private MMIO, and to add a bit of documentation around the
> {de,en}crypted logic.
>
> > > I would just expect the Hyper-V/vTOM code to leave
> > > x86_platform.hyper.is_private_mmio alone unless it *knows* the platform has
> > > private MMIO *and* CC_ATTR_GUEST_MEM_ENCRYPT.
> >
> > Agreed.
> >
> > >
> > > Is there ever a case where CC_ATTR_GUEST_MEM_ENCRYPT==0 and he
> > > Hyper-V/vTOM code would need to set x86_platform.hyper.is_private_mmio?
> >
> > There's no such case.
> >
> > I agree that gating with CC_ATTR_GUEST_MEM_ENCRYPT isn't really necessary.
> > Current upstream code always does the pgprot_decrypted(), and as you said,
> > that's a no-op on platforms with no memory encryption.
>
> Right, but since is_private_mmio can be NULL, unless I'm missing something we'll
> need an extra check no matter what, i.e. the alternative would be
>
> if (x86_platform.hyper.is_private_mmio &&
> x86_platform.hyper.is_private_mmio(phys))
> flags = pgprot_encrypted(flags);
> else
> flags = pgprot_decrypted(flags);
>
> I have no objection to that approach. It does have the advantage of not needing
> an indirect call for encrypted guests that don't support private MMIO, though
> I can't imagine this code is performance sensitive.

Or statically set a default stub function for is_private_mmio() that returns "false".
Then there's no need to check for NULL, and only platforms that want to use it
have to code anything. Several other entries in x86_platform have such defaults.

Michael
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On 2/23/23 13:15, Michael Kelley (LINUX) wrote:
> Or statically set a default stub function for is_private_mmio() that returns "false".
> Then there's no need to check for NULL, and only platforms that want to use it
> have to code anything. Several other entries in x86_platform have such defaults.

Yeah, that's what I was thinking too, like 'x86_op_int_noop':

> struct x86_platform_ops x86_platform __ro_after_init = {
> .calibrate_cpu = native_calibrate_cpu_early,
> .calibrate_tsc = native_calibrate_tsc,
...
> .hyper.pin_vcpu = x86_op_int_noop,

It's kinda silly to do an indirect call to a two-instruction function,
but this is a pretty slow path.
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Thu, Feb 23, 2023 at 12:27:50PM -0800, Dave Hansen wrote:
> On 2/23/23 12:01, Michael Kelley (LINUX) wrote:
> > Dave Hansen: Are you also OK with Sean's proposal? Looking for consensus
> > here ....
>
> Yeah, I'm generally OK with it as long as Borislav is.

Right, I think we're ok with the following basic rules:

- pure arch/x86/ code should use the x86_platform function pointers to
query hypervisor capabilities/peculiarities

- cc_platform_has() should be used in generic/driver code as it
abstracts away the underlying platform better. IOW, querying
x86_platform.... in generic, platform-agnostic driver code looks weird to
say the least

The hope is that those two should be enough to support most guest types
and not let the zoo get too much out of hand...

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Mon, 2023-03-06 at 22:51 +0100, Borislav Petkov wrote:
> On Thu, Feb 23, 2023 at 12:27:50PM -0800, Dave Hansen wrote:
> > On 2/23/23 12:01, Michael Kelley (LINUX) wrote:
> > > Dave Hansen:  Are you also OK with Sean's proposal?  Looking for consensus
> > > here ....
> >
> > Yeah, I'm generally OK with it as long as Borislav is.
>
> Right, I think we're ok with the following basic rules:
>
> - pure arch/x86/ code should use the x86_platform function pointers to
>   query hypervisor capabilities/peculiarities
>
> - cc_platform_has() should be used in generic/driver code as it
>   abstracts away the underlying platform better. IOW, querying
>   x86_platform.... in generic, platform-agnostic driver code looks weird to
>   say the least
>
> The hope is that those two should be enough to support most guest types
> and not let the zoo get too much out of hand...
>
> Thx.

In
https://lore.kernel.org/all/20230308171328.1562857-13-usama.arif@bytedance.com/
I added an sev_es_active() helper for x86 code.

Is that consistent with the vision here, or should I do something different?
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
First of all,

thanks for proactively pointing that out instead of simply using what's
there and we get to find out later, only by chance.

Much appreciated. :-)

On Thu, Mar 09, 2023 at 11:12:10AM +0000, David Woodhouse wrote:
> > Right, I think we're ok with the following basic rules:
> >
> > - pure arch/x86/ code should use the x86_platform function pointers to
> >   query hypervisor capabilities/peculiarities
> >
> > - cc_platform_has() should be used in generic/driver code as it
> >   abstracts away the underlying platform better. IOW, querying
> >   x86_platform.... in generic, platform-agnostic driver code looks weird to
> >   say the least
> >
> > The hope is that those two should be enough to support most guest types
> > and not let the zoo get too much out of hand...
> >
> > Thx.
>
> In
> https://lore.kernel.org/all/20230308171328.1562857-13-usama.arif@bytedance.com/
> I added an sev_es_active() helper for x86 code.
>
> Is that consistent with the vision here, or should I do something different?

So looking at sev_es_init_vc_handling() where we set that key, I'm
*thinking* that key can be removed now and the code should check

cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)

instead.

Because if some of the checks in that function below fail, the guest
will terminate anyway.

Jörg, Tom?

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v5 06/14] x86/ioremap: Support hypervisor specified range to map as encrypted [ In reply to ]
On Thu, 2023-03-09 at 12:59 +0100, Borislav Petkov wrote:
>
> So looking at sev_es_init_vc_handling() where we set that key, I'm
> *thinking* that key can be removed now and the code should check
>
>   cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT)
>
> instead.
>
> Because if some of the checks in that function below fail, the guest
> will terminate anyway.
>
> Jörg, Tom?

Hrm... the implication of that is that I should do something like this
in my own code. Is this really your intent?

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index b4265c5b46da..7ac4ec6415de 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1519,24 +1519,17 @@ void __init smp_prepare_cpus_common(void)
*/
static bool prepare_parallel_bringup(void)
{
- bool has_sev_es = sev_es_active();
+ /*
+ * The "generic" CC_ATTR_GUEST_STATE_ENCRYPT actually means specifically
+ * SEV-ES, and only SEV-ES, and always shall mean that. If it's present,
+ * that means the AP startup code should use the hard-coded SEV-ES GHCB
+ * call to find its APIC ID (STARTUP_APICID_SEV_ES).
+ */
+ bool has_sev_es = cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT);

if (IS_ENABLED(CONFIG_X86_32))
return false;

- /*
- * Encrypted guests other than SEV-ES (in the future) will need to
- * implement an early way of finding the APIC ID, since they will
- * presumably block direct CPUID too. Be kind to our future selves
- * by warning here instead of just letting them break. Parallel
- * startup doesn't have to be in the first round of enabling patches
- * for any such technology.
- */
- if (cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT) && !has_sev_es) {
- pr_info("Disabling parallel bringup due to guest memory encryption\n");
- return false;
- }
-
if (x2apic_mode || has_sev_es) {
if (boot_cpu_data.cpuid_level < 0x0b)
return false;

1 2 3  View All