diff mbox series

[v4,01/13] x86/ioapic: Gate decrypted mapping on cc_platform_has() attribute

Message ID 1669951831-4180-2-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

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Michael Kelley (LINUX) Dec. 2, 2022, 3:30 a.m. UTC 
Current code always maps the IO-APIC 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 IO-APIC.
In such a case, the IO-APIC must be accessed as private (encrypted).

Fix this by gating the IO-APIC decrypted mapping on a new
cc_platform_has() attribute that a subsequent patch in the series
will set only for guests using vTOM.

Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Reviewed-by: Wei Liu <wei.liu@kernel.org>
---
 arch/x86/kernel/apic/io_apic.c |  3 ++-
 include/linux/cc_platform.h    | 12 ++++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

Comments

Borislav Petkov Dec. 6, 2022, 7:22 p.m. UTC | #1 
On Thu, Dec 01, 2022 at 07:30:19PM -0800, Michael Kelley wrote:
> Current code always maps the IO-APIC 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 IO-APIC.
> In such a case, the IO-APIC must be accessed as private (encrypted).

Lemme see I understand this correctly:

the paravisor is emulating the IO-APIC in the lower range of the address
space, under the vTOM which is accessed encrypted.

That's why you need to access it encrypted in the guest.

Close?

Thx.
Kuppuswamy Sathyanarayanan Dec. 6, 2022, 7:27 p.m. UTC | #2 
On 12/1/22 7:30 PM, Michael Kelley wrote:
> Current code always maps the IO-APIC 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 IO-APIC.
> In such a case, the IO-APIC must be accessed as private (encrypted).
> 
> Fix this by gating the IO-APIC decrypted mapping on a new
> cc_platform_has() attribute that a subsequent patch in the series
> will set only for guests using vTOM.
> 
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> Reviewed-by: Wei Liu <wei.liu@kernel.org>
> ---

Looks good 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..2b70e2e 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_ACCESS_IOAPIC_ENCRYPTED))
> +		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..7b63a7d 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_ACCESS_IOAPIC_ENCRYPTED: Guest VM IO-APIC is encrypted
> +	 *
> +	 * The platform/OS is running as a guest/virtual machine with
> +	 * an IO-APIC that is emulated by a paravisor running in the
> +	 * guest VM context. As such, the IO-APIC is accessed in the
> +	 * encrypted portion of the guest physical address space.
> +	 *
> +	 * Examples include Hyper-V SEV-SNP guests using vTOM.
> +	 */
> +	CC_ATTR_ACCESS_IOAPIC_ENCRYPTED,
>  };
>  
>  #ifdef CONFIG_ARCH_HAS_CC_PLATFORM
Michael Kelley (LINUX) Dec. 6, 2022, 7:54 p.m. UTC | #3 
From: Borislav Petkov <bp@alien8.de> Sent: Tuesday, December 6, 2022 11:23 AM
> 
> On Thu, Dec 01, 2022 at 07:30:19PM -0800, Michael Kelley wrote:
> > Current code always maps the IO-APIC 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 IO-APIC.
> > In such a case, the IO-APIC must be accessed as private (encrypted).
> 
> Lemme see I understand this correctly:
> 
> the paravisor is emulating the IO-APIC in the lower range of the address
> space, under the vTOM which is accessed encrypted.
> 
> That's why you need to access it encrypted in the guest.
> 
> Close?
> 

Exactly correct.

Michael
Borislav Petkov Dec. 29, 2022, 11:39 a.m. UTC | #4 
On Tue, Dec 06, 2022 at 07:54:02PM +0000, Michael Kelley (LINUX) wrote:
> Exactly correct.

Ok, thanks.

Let's put that in the commit message and get rid of the "subsequent
patch" wording as patch order in git is ambiguous.

IOW, something like this:

    Current code always maps the IO-APIC 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 IO-APIC.

    In such a case, the IO-APIC must be accessed as private (encrypted)
    because the paravisor emulates the IO-APIC in the lower half of the vTOM
    where all accesses must be encrypted.

    Add a new CC attribute which determines how the IO-APIC MMIO mapping
    should be established depending on the platform the kernel is running on
    as a guest.

Thx.
Michael Kelley (LINUX) Dec. 29, 2022, 4:02 p.m. UTC | #5 
From: Borislav Petkov <bp@alien8.de> Sent: Thursday, December 29, 2022 3:39 AM
> 
> On Tue, Dec 06, 2022 at 07:54:02PM +0000, Michael Kelley (LINUX) wrote:
> > Exactly correct.
> 
> Ok, thanks.
> 
> Let's put that in the commit message and get rid of the "subsequent
> patch" wording as patch order in git is ambiguous.
> 
> IOW, something like this:
> 
>     Current code always maps the IO-APIC 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 IO-APIC.
> 
>     In such a case, the IO-APIC must be accessed as private (encrypted)
>     because the paravisor emulates the IO-APIC in the lower half of the vTOM
>     where all accesses must be encrypted.
> 
>     Add a new CC attribute which determines how the IO-APIC MMIO mapping
>     should be established depending on the platform the kernel is running on
>     as a guest.
>
 
Works for me.  I'll adopt this wording in v5.

Michael
diff mbox series

Patch

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index a868b76..2b70e2e 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_ACCESS_IOAPIC_ENCRYPTED))
+		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..7b63a7d 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_ACCESS_IOAPIC_ENCRYPTED: Guest VM IO-APIC is encrypted
+	 *
+	 * The platform/OS is running as a guest/virtual machine with
+	 * an IO-APIC that is emulated by a paravisor running in the
+	 * guest VM context. As such, the IO-APIC is accessed in the
+	 * encrypted portion of the guest physical address space.
+	 *
+	 * Examples include Hyper-V SEV-SNP guests using vTOM.
+	 */
+	CC_ATTR_ACCESS_IOAPIC_ENCRYPTED,
 };
 
 #ifdef CONFIG_ARCH_HAS_CC_PLATFORM