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

[Yunhong Jiang <yunhong.jiang@linux.intel.com>]

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