From: Isaku Yamahata <[email protected]>
The kernel interacts with each bare-metal IOAPIC with a special
MMIO page. When running under KVM, the guest's IOAPICs are
emulated by KVM.
When running as a TDX guest, the guest needs to mark each IOAPIC
mapping as "shared" with the host. This ensures that TDX private
protections are not applied to the page, which allows the TDX host
emulation to work.
Earlier patches in this series modified ioremap() so that
ioremap()-created mappings such as virtio will be marked as
shared. However, the IOAPIC code does not use ioremap() and instead
uses the fixmap mechanism.
Introduce a special fixmap helper just for the IOAPIC code. Ensure
that it marks IOAPIC pages as "shared". This replaces
set_fixmap_nocache() with __set_fixmap() since __set_fixmap()
allows custom 'prot' values.
Signed-off-by: Isaku Yamahata <[email protected]>
Reviewed-by: Andi Kleen <[email protected]>
Reviewed-by: Tony Luck <[email protected]>
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
Signed-off-by: Kirill A. Shutemov <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index c1bb384935b0..d2fef5893e41 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -49,6 +49,7 @@
#include <linux/slab.h>
#include <linux/memblock.h>
#include <linux/msi.h>
+#include <linux/cc_platform.h>
#include <asm/irqdomain.h>
#include <asm/io.h>
@@ -65,6 +66,7 @@
#include <asm/irq_remapping.h>
#include <asm/hw_irq.h>
#include <asm/apic.h>
+#include <asm/pgtable.h>
#define for_each_ioapic(idx) \
for ((idx) = 0; (idx) < nr_ioapics; (idx)++)
@@ -2677,6 +2679,18 @@ static struct resource * __init ioapic_setup_resources(void)
return res;
}
+static void io_apic_set_fixmap_nocache(enum fixed_addresses idx,
+ phys_addr_t phys)
+{
+ pgprot_t flags = FIXMAP_PAGE_NOCACHE;
+
+ /* Set TDX guest shared bit in pgprot flags */
+ if (cc_platform_has(CC_ATTR_GUEST_TDX))
+ flags = pgprot_decrypted(flags);
+
+ __set_fixmap(idx, phys, flags);
+}
+
void __init io_apic_init_mappings(void)
{
unsigned long ioapic_phys, idx = FIX_IO_APIC_BASE_0;
@@ -2709,7 +2723,7 @@ void __init io_apic_init_mappings(void)
__func__, PAGE_SIZE, PAGE_SIZE);
ioapic_phys = __pa(ioapic_phys);
}
- set_fixmap_nocache(idx, ioapic_phys);
+ io_apic_set_fixmap_nocache(idx, ioapic_phys);
apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08lx (%08lx)\n",
__fix_to_virt(idx) + (ioapic_phys & ~PAGE_MASK),
ioapic_phys);
@@ -2838,7 +2852,7 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
ioapics[idx].mp_config.flags = MPC_APIC_USABLE;
ioapics[idx].mp_config.apicaddr = address;
- set_fixmap_nocache(FIX_IO_APIC_BASE_0 + idx, address);
+ io_apic_set_fixmap_nocache(FIX_IO_APIC_BASE_0 + idx, address);
if (bad_ioapic_register(idx)) {
clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
return -ENODEV;
--
2.34.1
On Mon, Jan 24 2022 at 18:02, Kirill A. Shutemov wrote:
> From: Isaku Yamahata <[email protected]>
>
> The kernel interacts with each bare-metal IOAPIC with a special
> MMIO page. When running under KVM, the guest's IOAPICs are
> emulated by KVM.
>
> When running as a TDX guest, the guest needs to mark each IOAPIC
> mapping as "shared" with the host. This ensures that TDX private
> protections are not applied to the page, which allows the TDX host
> emulation to work.
>
> Earlier patches in this series modified ioremap() so that
The concept of earlier patches does not exist.
> ioremap()-created mappings such as virtio will be marked as
> shared. However, the IOAPIC code does not use ioremap() and instead
> uses the fixmap mechanism.
>
> Introduce a special fixmap helper just for the IOAPIC code. Ensure
> that it marks IOAPIC pages as "shared". This replaces
> set_fixmap_nocache() with __set_fixmap() since __set_fixmap()
> allows custom 'prot' values.
Why is this a TDX only issue and SEV does not suffer from that?
Thanks,
tglx
> > ioremap()-created mappings such as virtio will be marked as
> > shared. However, the IOAPIC code does not use ioremap() and instead
> > uses the fixmap mechanism.
> >
> > Introduce a special fixmap helper just for the IOAPIC code. Ensure
> > that it marks IOAPIC pages as "shared". This replaces
> > set_fixmap_nocache() with __set_fixmap() since __set_fixmap()
> > allows custom 'prot' values.
>
> Why is this a TDX only issue and SEV does not suffer from that?
The bit meaning is opposite.
TDX: set: shared, cleared: private
SEV: set: private, cleared: shared
Without this patch, it happens to work for SEV. (or any emulated MMIO can work)
But for TDX, IOAPIC emulation doesn't work.
Thanks,
Isaku Yamahata
On Wed, Feb 02, 2022 at 02:33:16AM +0100, Thomas Gleixner wrote:
> On Mon, Jan 24 2022 at 18:02, Kirill A. Shutemov wrote:
> > ioremap()-created mappings such as virtio will be marked as
> > shared. However, the IOAPIC code does not use ioremap() and instead
> > uses the fixmap mechanism.
> >
> > Introduce a special fixmap helper just for the IOAPIC code. Ensure
> > that it marks IOAPIC pages as "shared". This replaces
> > set_fixmap_nocache() with __set_fixmap() since __set_fixmap()
> > allows custom 'prot' values.
>
> Why is this a TDX only issue and SEV does not suffer from that?
Hm. Good question.
I think it is because FIXMAP_PAGE_NOCACHE does not have __ENC bit set so
the mapping is accessible to host. With TDX the logic is oposit:
everything is private if the bit is not set.
Tom, does it sound right?
BTW, I will drop 'if (cc_platform_has(CC_ATTR_GUEST_TDX))'.
pgprot_decrypted() is nop on AMD in this case.
--
Kirill A. Shutemov
On 2/4/22 16:31, Kirill A. Shutemov wrote:
> On Wed, Feb 02, 2022 at 02:33:16AM +0100, Thomas Gleixner wrote:
>> On Mon, Jan 24 2022 at 18:02, Kirill A. Shutemov wrote:
>>> ioremap()-created mappings such as virtio will be marked as
>>> shared. However, the IOAPIC code does not use ioremap() and instead
>>> uses the fixmap mechanism.
>>>
>>> Introduce a special fixmap helper just for the IOAPIC code. Ensure
>>> that it marks IOAPIC pages as "shared". This replaces
>>> set_fixmap_nocache() with __set_fixmap() since __set_fixmap()
>>> allows custom 'prot' values.
>>
>> Why is this a TDX only issue and SEV does not suffer from that?
>
> Hm. Good question.
>
> I think it is because FIXMAP_PAGE_NOCACHE does not have __ENC bit set so
> the mapping is accessible to host. With TDX the logic is oposit:
> everything is private if the bit is not set.
>
> Tom, does it sound right?
Correct, FIXMAP_PAGE_NOCACHE => PAGE_KERNEL_IO_NOCACHE, which does not
have the encryption bit set, so it is mapped as shared under SEV.
Thanks,
Tom
>
> BTW, I will drop 'if (cc_platform_has(CC_ATTR_GUEST_TDX))'.
> pgprot_decrypted() is nop on AMD in this case.
>