diff mbox series

[5/7] x86/hyperv: Mark ACPI wakeup mailbox page as private

Message ID 20240806221237.1634126-6-yunhong.jiang@linux.intel.com (mailing list archive)
State Superseded, archived
Headers show
Series x86/acpi: Move ACPI MADT wakeup to generic code | expand

Commit Message

Yunhong Jiang Aug. 6, 2024, 10:12 p.m. UTC 
The ACPI wakeup mailbox is accessed by the OS and the firmware, both are
in the guest's context, instead of the hypervisor/VMM context. Mark the
address private explicitly.

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

Comments

Thomas Gleixner Aug. 7, 2024, 4:59 p.m. UTC | #1 
On Tue, Aug 06 2024 at 15:12, Yunhong Jiang wrote:
> The ACPI wakeup mailbox is accessed by the OS and the firmware, both are
> in the guest's context, instead of the hypervisor/VMM context. Mark the
> address private explicitly.

This lacks information why the realmode area must be reserved and
initialized, which is what the change is doing implicitely.

> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
>  
> +/*
> + * 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)
> +{
> +	if (wakeup_mailbox_addr && (addr >= wakeup_mailbox_addr &&
> +	    addr < (wakeup_mailbox_addr + PAGE_SIZE)))
> +		return true;
> +	return false;
> +}

static inline bool within_page(u64 addr, u64 start)
{
	return addr >= start && addr < (start + PAGE_SIZE);
}

static bool hv_is_private_mmio_tdx(u64 addr)
{
        return wakeup_mailbox_addr && within_page(addr, wakeup_mailbox_addr)
}

Hmm?

> +
>  void __init hv_vtl_init_platform(void)
>  {
>  	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
>  
> -	x86_platform.realmode_reserve = x86_init_noop;
> -	x86_platform.realmode_init = x86_init_noop;
> +	if (wakeup_mailbox_addr) {

Wants a comment vs. realmode here.

> +		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
> +	} else {
> +		x86_platform.realmode_reserve = x86_init_noop;
> +		x86_platform.realmode_init = x86_init_noop;
> +	}
>  	x86_init.irqs.pre_vector_init = x86_init_noop;
>  	x86_init.timers.timer_init = x86_init_noop;

Thanks,

        tglx
Yunhong Jiang Aug. 9, 2024, 11:17 p.m. UTC | #2 
On Wed, Aug 07, 2024 at 06:59:28PM +0200, Thomas Gleixner wrote:
> On Tue, Aug 06 2024 at 15:12, Yunhong Jiang wrote:
> > The ACPI wakeup mailbox is accessed by the OS and the firmware, both are
> > in the guest's context, instead of the hypervisor/VMM context. Mark the
> > address private explicitly.
> 
> This lacks information why the realmode area must be reserved and
> initialized, which is what the change is doing implicitely.
> 
> > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> >  
> > +/*
> > + * 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)
> > +{
> > +	if (wakeup_mailbox_addr && (addr >= wakeup_mailbox_addr &&
> > +	    addr < (wakeup_mailbox_addr + PAGE_SIZE)))
> > +		return true;
> > +	return false;
> > +}
> 
> static inline bool within_page(u64 addr, u64 start)
> {
> 	return addr >= start && addr < (start + PAGE_SIZE);
> }
> 
> static bool hv_is_private_mmio_tdx(u64 addr)
> {
>         return wakeup_mailbox_addr && within_page(addr, wakeup_mailbox_addr)
> }
> 
> Hmm?
> 
> > +
> >  void __init hv_vtl_init_platform(void)
> >  {
> >  	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
> >  
> > -	x86_platform.realmode_reserve = x86_init_noop;
> > -	x86_platform.realmode_init = x86_init_noop;
> > +	if (wakeup_mailbox_addr) {
> 
> Wants a comment vs. realmode here.

Sorry for the confusion. This patch is not related to real mode. This change is
similar to 88e378d400fa0544d51cf62037e7774d8a4b4379. Current code maps MMIO
devices as shared (decrypted) by default in a confidential computing VM. But the
wakeup mailbox must be accessed as private (encrypted) because all the access to
it are encrypted.

It's my fault to leave the realmode_reserve/realmode_init change here and cause
this confusion. Originally this patch and the real mode patch were included in
one patch. When I splitted them, I wrongly left this change here. Will put it to
the realmode patch on my next submission.

Will address all your other comments on my next submission.

Thank you
--jyh

> 
> > +		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
> > +	} else {
> > +		x86_platform.realmode_reserve = x86_init_noop;
> > +		x86_platform.realmode_init = x86_init_noop;
> > +	}
> >  	x86_init.irqs.pre_vector_init = x86_init_noop;
> >  	x86_init.timers.timer_init = x86_init_noop;
> 
> Thanks,
> 
>         tglx
diff mbox series

Patch

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 04775346369c..bfe54afcdf1d 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -22,12 +22,28 @@  static bool __init hv_vtl_msi_ext_dest_id(void)
 	return true;
 }
 
+/*
+ * 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)
+{
+	if (wakeup_mailbox_addr && (addr >= wakeup_mailbox_addr &&
+	    addr < (wakeup_mailbox_addr + PAGE_SIZE)))
+		return true;
+	return false;
+}
+
 void __init hv_vtl_init_platform(void)
 {
 	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
 
-	x86_platform.realmode_reserve = x86_init_noop;
-	x86_platform.realmode_init = x86_init_noop;
+	if (wakeup_mailbox_addr) {
+		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
+	} else {
+		x86_platform.realmode_reserve = x86_init_noop;
+		x86_platform.realmode_init = x86_init_noop;
+	}
 	x86_init.irqs.pre_vector_init = x86_init_noop;
 	x86_init.timers.timer_init = x86_init_noop;