diff mbox series

[v2,5/9] x86/hyperv: Mark ACPI wakeup mailbox page as private

Message ID 20240823232327.2408869-6-yunhong.jiang@linux.intel.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series x86/hyperv: Support wakeup mailbox for VTL2 TDX guest | expand

Commit Message

Yunhong Jiang Aug. 23, 2024, 11:23 p.m. UTC
Current code maps MMIO devices as shared (decrypted) by default in a
confidential computing VM. However, the wakeup mailbox must be accessed
as private (encrypted) because it's accessed by the OS and the firmware,
both are in the guest's context and encrypted. Set the wakeup mailbox
range as private explicitly.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 arch/x86/hyperv/hv_vtl.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Michael Kelley Sept. 2, 2024, 3:35 a.m. UTC | #1
From: Yunhong Jiang <yunhong.jiang@linux.intel.com> Sent: Friday, August 23, 2024 4:23 PM
> 
> Current code maps MMIO devices as shared (decrypted) by default in a
> confidential computing VM. However, the wakeup mailbox must be accessed
> as private (encrypted) because it's accessed by the OS and the firmware,
> both are in the guest's context and encrypted. Set the wakeup mailbox
> range as private explicitly.
> 
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> ---
>  arch/x86/hyperv/hv_vtl.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> index 04775346369c..987a6a1200b0 100644
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -22,10 +22,26 @@ static bool __init hv_vtl_msi_ext_dest_id(void)
>  	return true;
>  }
> 
> +static inline bool within_page(u64 addr, u64 start)
> +{
> +	return addr >= start && addr < (start + PAGE_SIZE);
> +}
> +
> +/*
> + * The ACPI wakeup mailbox are accessed by the OS and the BIOS, both are in the
> + * guest's context, instead of the hypervisor/VMM context.
> + */
> +static bool hv_is_private_mmio_tdx(u64 addr)
> +{
> +	return wakeup_mailbox_addr && within_page(addr, wakeup_mailbox_addr);
> +}
> +
>  void __init hv_vtl_init_platform(void)
>  {
>  	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
> 
> +	if (hv_isolation_type_tdx())
> +		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;

hv_vtl_init_platform() is unconditionally called in
ms_hyperv_init_platform(). So in the case of a normal TDX guest
running with a paravisor on Hyper-V, the above code will overwrite
the is_private_mmio function that was set in hv_vtom_init(). Then
the mapping of the emulated IOAPIC and TPM provided by the
paravisor won't be correct.

Michael

>  	x86_platform.realmode_reserve = x86_init_noop;
>  	x86_platform.realmode_init = x86_init_noop;
>  	x86_init.irqs.pre_vector_init = x86_init_noop;
> --
> 2.25.1
>
Saurabh Singh Sengar Sept. 2, 2024, 6:38 p.m. UTC | #2
On Mon, Sep 02, 2024 at 03:35:18AM +0000, Michael Kelley wrote:
> From: Yunhong Jiang <yunhong.jiang@linux.intel.com> Sent: Friday, August 23, 2024 4:23 PM
> > 
> > Current code maps MMIO devices as shared (decrypted) by default in a
> > confidential computing VM. However, the wakeup mailbox must be accessed
> > as private (encrypted) because it's accessed by the OS and the firmware,
> > both are in the guest's context and encrypted. Set the wakeup mailbox
> > range as private explicitly.
> > 
> > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > ---
> >  arch/x86/hyperv/hv_vtl.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> > index 04775346369c..987a6a1200b0 100644
> > --- a/arch/x86/hyperv/hv_vtl.c
> > +++ b/arch/x86/hyperv/hv_vtl.c
> > @@ -22,10 +22,26 @@ static bool __init hv_vtl_msi_ext_dest_id(void)
> >  	return true;
> >  }
> > 
> > +static inline bool within_page(u64 addr, u64 start)
> > +{
> > +	return addr >= start && addr < (start + PAGE_SIZE);
> > +}
> > +
> > +/*
> > + * The ACPI wakeup mailbox are accessed by the OS and the BIOS, both are in the
> > + * guest's context, instead of the hypervisor/VMM context.
> > + */
> > +static bool hv_is_private_mmio_tdx(u64 addr)
> > +{
> > +	return wakeup_mailbox_addr && within_page(addr, wakeup_mailbox_addr);
> > +}
> > +
> >  void __init hv_vtl_init_platform(void)
> >  {
> >  	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
> > 
> > +	if (hv_isolation_type_tdx())
> > +		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
> 
> hv_vtl_init_platform() is unconditionally called in
> ms_hyperv_init_platform(). So in the case of a normal TDX guest
> running with a paravisor on Hyper-V, the above code will overwrite
> the is_private_mmio function that was set in hv_vtom_init(). Then
> the mapping of the emulated IOAPIC and TPM provided by the
> paravisor won't be correct.
> 
> Michael

non-VTL Hyper-V platforms are expected to disable CONFIG_HYPERV_VTL_MODE,
that means for a normal TDX guest hv_vtl_init_platform will be an empty
stub. Have I missed anything ?

- Saurabh
Michael Kelley Sept. 2, 2024, 8:24 p.m. UTC | #3
From: Saurabh Singh Sengar <ssengar@linux.microsoft.com> Sent: Monday, September 2, 2024 11:39 AM
> 
> On Mon, Sep 02, 2024 at 03:35:18AM +0000, Michael Kelley wrote:
> > From: Yunhong Jiang <yunhong.jiang@linux.intel.com> Sent: Friday, August 23, 2024
> 4:23 PM
> > >
> > > Current code maps MMIO devices as shared (decrypted) by default in a
> > > confidential computing VM. However, the wakeup mailbox must be accessed
> > > as private (encrypted) because it's accessed by the OS and the firmware,
> > > both are in the guest's context and encrypted. Set the wakeup mailbox
> > > range as private explicitly.
> > >
> > > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > > ---
> > >  arch/x86/hyperv/hv_vtl.c | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > >
> > > diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> > > index 04775346369c..987a6a1200b0 100644
> > > --- a/arch/x86/hyperv/hv_vtl.c
> > > +++ b/arch/x86/hyperv/hv_vtl.c
> > > @@ -22,10 +22,26 @@ static bool __init hv_vtl_msi_ext_dest_id(void)
> > >  	return true;
> > >  }
> > >
> > > +static inline bool within_page(u64 addr, u64 start)
> > > +{
> > > +	return addr >= start && addr < (start + PAGE_SIZE);
> > > +}
> > > +
> > > +/*
> > > + * The ACPI wakeup mailbox are accessed by the OS and the BIOS, both are in the
> > > + * guest's context, instead of the hypervisor/VMM context.
> > > + */
> > > +static bool hv_is_private_mmio_tdx(u64 addr)
> > > +{
> > > +	return wakeup_mailbox_addr && within_page(addr, wakeup_mailbox_addr);
> > > +}
> > > +
> > >  void __init hv_vtl_init_platform(void)
> > >  {
> > >  	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
> > >
> > > +	if (hv_isolation_type_tdx())
> > > +		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
> >
> > hv_vtl_init_platform() is unconditionally called in
> > ms_hyperv_init_platform(). So in the case of a normal TDX guest
> > running with a paravisor on Hyper-V, the above code will overwrite
> > the is_private_mmio function that was set in hv_vtom_init(). Then
> > the mapping of the emulated IOAPIC and TPM provided by the
> > paravisor won't be correct.
> >
> > Michael
> 
> non-VTL Hyper-V platforms are expected to disable CONFIG_HYPERV_VTL_MODE,
> that means for a normal TDX guest hv_vtl_init_platform will be an empty
> stub. Have I missed anything ?
> 

Ah, you are right. My concern is misplaced and can be ignored. :-)

Michael
diff mbox series

Patch

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 04775346369c..987a6a1200b0 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -22,10 +22,26 @@  static bool __init hv_vtl_msi_ext_dest_id(void)
 	return true;
 }
 
+static inline bool within_page(u64 addr, u64 start)
+{
+	return addr >= start && addr < (start + PAGE_SIZE);
+}
+
+/*
+ * The ACPI wakeup mailbox are accessed by the OS and the BIOS, both are in the
+ * guest's context, instead of the hypervisor/VMM context.
+ */
+static bool hv_is_private_mmio_tdx(u64 addr)
+{
+	return wakeup_mailbox_addr && within_page(addr, wakeup_mailbox_addr);
+}
+
 void __init hv_vtl_init_platform(void)
 {
 	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
 
+	if (hv_isolation_type_tdx())
+		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
 	x86_platform.realmode_reserve = x86_init_noop;
 	x86_platform.realmode_init = x86_init_noop;
 	x86_init.irqs.pre_vector_init = x86_init_noop;