| Message ID | 1668624097-14884-3-git-send-email-mikelley@microsoft.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Add PCI pass-thru support to Hyper-V Confidential VMs | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 11/16/22 10:41 AM, Michael Kelley wrote: > Current code always maps the IOAPIC as shared (decrypted) in a > confidential VM. But Hyper-V guest VMs on AMD SEV-SNP with vTOM > enabled use a paravisor running in VMPL0 to emulate the IOAPIC. > In such a case, the IOAPIC must be accessed as private (encrypted). > > Fix this by gating the IOAPIC decrypted mapping on a new > cc_platform_has() attribute that a subsequent patch in the series > will set only for Hyper-V guests. > > Signed-off-by: Michael Kelley <mikelley@microsoft.com> > Reviewed-by: Wei Liu <wei.liu@kernel.org> > --- Looks fine to me. Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > arch/x86/kernel/apic/io_apic.c | 3 ++- > include/linux/cc_platform.h | 12 ++++++++++++ > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c > index a868b76..c65e0cc 100644 > --- a/arch/x86/kernel/apic/io_apic.c > +++ b/arch/x86/kernel/apic/io_apic.c > @@ -2686,7 +2686,8 @@ static void io_apic_set_fixmap(enum fixed_addresses idx, phys_addr_t phys) > * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot > * bits, just like normal ioremap(): > */ > - flags = pgprot_decrypted(flags); > + if (!cc_platform_has(CC_ATTR_EMULATED_IOAPIC)) > + flags = pgprot_decrypted(flags); > > __set_fixmap(idx, phys, flags); > } > diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h > index cb0d6cd..7a0da75 100644 > --- a/include/linux/cc_platform.h > +++ b/include/linux/cc_platform.h > @@ -90,6 +90,18 @@ enum cc_attr { > * Examples include TDX Guest. > */ > CC_ATTR_HOTPLUG_DISABLED, > + > + /** > + * @CC_ATTR_EMULATED_IOAPIC: Guest VM has an emulated I/O APIC > + * > + * The platform/OS is running as a guest/virtual machine with > + * an I/O APIC that is emulated by a paravisor running in the > + * guest VM context. As such, the I/O APIC is accessed in the > + * encrypted portion of the guest physical address space. > + * > + * Examples include Hyper-V SEV-SNP guests using vTOM. > + */ > + CC_ATTR_EMULATED_IOAPIC, > }; > > #ifdef CONFIG_ARCH_HAS_CC_PLATFORM
On Wed, Nov 16, 2022 at 10:41:25AM -0800, Michael Kelley wrote: > Current code always maps the IOAPIC as shared (decrypted) in a > confidential VM. But Hyper-V guest VMs on AMD SEV-SNP with vTOM > enabled use a paravisor running in VMPL0 to emulate the IOAPIC. "IO-APIC" I guess, in all your text. > In such a case, the IOAPIC must be accessed as private (encrypted). So the condition for the IO-APIC is pretty specific but the naming CC_ATTR_EMULATED_IOAPIC too generic. Other HVs emulate IO-APICs too, right? If you have to be precise, the proper check should be (pseudo code): if (cc_vendor(HYPERV) && SNP enabled && SNP features has vTOM && paravisor in use) so I guess you're probably better off calling it CC_ATTR_ACCESS_IOAPIC_ENCRYPTED which then gets set on exactly those guests and nothing else. I'd say. Thx.
From: Borislav Petkov <bp@alien8.de> Sent: Monday, November 21, 2022 5:51 AM > > On Wed, Nov 16, 2022 at 10:41:25AM -0800, Michael Kelley wrote: > > Current code always maps the IOAPIC as shared (decrypted) in a > > confidential VM. But Hyper-V guest VMs on AMD SEV-SNP with vTOM > > enabled use a paravisor running in VMPL0 to emulate the IOAPIC. > > "IO-APIC" I guess, in all your text. > > > In such a case, the IOAPIC must be accessed as private (encrypted). > > So the condition for the IO-APIC is pretty specific but the naming > CC_ATTR_EMULATED_IOAPIC too generic. Other HVs emulate IO-APICs too, > right? > > If you have to be precise, the proper check should be (pseudo code): > > if (cc_vendor(HYPERV) && > SNP enabled && > SNP features has vTOM && > paravisor in use) > > so I guess you're probably better off calling it > > CC_ATTR_ACCESS_IOAPIC_ENCRYPTED > > which then gets set on exactly those guests and nothing else. > > I'd say. > I'm OK with naming it very narrowly. When/if there's a more general case later, we can generalize to whatever degree is appropriate. Michael
On Mon, Nov 21, 2022 at 04:43:01PM +0000, Michael Kelley (LINUX) wrote: > I'm OK with naming it very narrowly. When/if there's a more general > case later, we can generalize to whatever degree is appropriate. Exactly. Those defines are free to change when we see fit.
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c index a868b76..c65e0cc 100644 --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -2686,7 +2686,8 @@ static void io_apic_set_fixmap(enum fixed_addresses idx, phys_addr_t phys) * Ensure fixmaps for IOAPIC MMIO respect memory encryption pgprot * bits, just like normal ioremap(): */ - flags = pgprot_decrypted(flags); + if (!cc_platform_has(CC_ATTR_EMULATED_IOAPIC)) + flags = pgprot_decrypted(flags); __set_fixmap(idx, phys, flags); } diff --git a/include/linux/cc_platform.h b/include/linux/cc_platform.h index cb0d6cd..7a0da75 100644 --- a/include/linux/cc_platform.h +++ b/include/linux/cc_platform.h @@ -90,6 +90,18 @@ enum cc_attr { * Examples include TDX Guest. */ CC_ATTR_HOTPLUG_DISABLED, + + /** + * @CC_ATTR_EMULATED_IOAPIC: Guest VM has an emulated I/O APIC + * + * The platform/OS is running as a guest/virtual machine with + * an I/O APIC that is emulated by a paravisor running in the + * guest VM context. As such, the I/O APIC is accessed in the + * encrypted portion of the guest physical address space. + * + * Examples include Hyper-V SEV-SNP guests using vTOM. + */ + CC_ATTR_EMULATED_IOAPIC, }; #ifdef CONFIG_ARCH_HAS_CC_PLATFORM