Re: [RFC v2 32/32] x86/tdx: ioapic: Add shared bit for IOAPIC base address

[Dave Hansen]

On 4/26/21 11:01 AM, Kuppuswamy Sathyanarayanan wrote:
> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> 
> IOAPIC is emulated by KVM which means its MMIO address is shared
> by host. Add shared bit for base address of IOAPIC.
> Most MMIO region is handled by ioremap which is already marked
> as shared for TDX guest platform, but IOAPIC is an exception which
> uses fixed map.

Ho hum...  I guess I'll rewrite the changelog:

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.

>  arch/x86/kernel/apic/io_apic.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 73ff4dd426a8..2a01d4a82be7 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -2675,6 +2675,14 @@ 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;
> +	if (is_tdx_guest())
> +		flags = pgprot_tdg_shared(flags);
> +	__set_fixmap(idx, phys, flags);
> +}

^ This seems like it could at least use a one-liner comment.

>  void __init io_apic_init_mappings(void)
>  {
>  	unsigned long ioapic_phys, idx = FIX_IO_APIC_BASE_0;
> @@ -2707,7 +2715,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);
> @@ -2836,7 +2844,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;
>