Re: [PATCH v3 02/13] xen: harmonize return types of hypercall handlers

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 17 Dec 2021, Julien Grall wrote:
> Hi Stefano,
> 
> On 16/12/2021 21:15, Stefano Stabellini wrote:
> > On Thu, 16 Dec 2021, Stefano Stabellini wrote:
> > > On Thu, 16 Dec 2021, Juergen Gross wrote:
> > > > On 16.12.21 03:10, Stefano Stabellini wrote:
> > > > > On Wed, 15 Dec 2021, Juergen Gross wrote:
> > > > > > On 14.12.21 18:36, Julien Grall wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > On 08/12/2021 15:55, Juergen Gross wrote:
> > > > > > > > Today most hypercall handlers have a return type of long, while
> > > > > > > > the
> > > > > > > > compat ones return an int. There are a few exceptions from that
> > > > > > > > rule,
> > > > > > > > however.
> > > > > > > 
> > > > > > > So on Arm64, I don't think you can make use of the full 64-bit
> > > > > > > because a
> > > > > > > 32-bit domain would not be able to see the top 32-bit.
> > > > > > > 
> > > > > > > In fact, this could potentially cause us some trouble (see [1]) in
> > > > > > > Xen.
> > > > > > > So it feels like the hypercalls should always return a 32-bit
> > > > > > > signed
> > > > > > > value
> > > > > > > on Arm.
> > > > > > 
> > > > > > This would break hypercalls like XENMEM_maximum_ram_page which are
> > > > > > able
> > > > > > to return larger values, right?
> > > > > > 
> > > > > > > The other advantage is it would be clear that the top 32-bit are
> > > > > > > not
> > > > > > > usuable. Stefano, what do you think?
> > > > > > 
> > > > > > Wouldn't it make more sense to check the return value to be a sign
> > > > > > extended 32-bit value for 32-bit guests in do_trap_hypercall()
> > > > > > instead?
> > > > > > 
> > > > > > The question is what to return if this is not the case. -EDOM?
> > > > > 
> > > > > 
> > > > > I can see where Julien is coming from: we have been trying to keep the
> > > > > arm32 and arm64 ABIs identical since the beginning of the project. So,
> > > > > like Julien, my preference would be to always return 32-bit on ARM,
> > > > > both
> > > > > aarch32 and aarch64. It would make things simple.
> > > > > 
> > > > > The case of XENMEM_maximum_ram_page is interesting but it is not a
> > > > > problem in reality because the max physical address size is only
> > > > > 40-bit
> > > > > for aarch32 guests, so 32-bit are always enough to return the highest
> > > > > page in memory for 32-bit guests.
> > > > 
> > > > You are aware that this isn't the guest's max page, but the host's?
> > 
> > I can see now that you meant to say that, no matter what is the max
> > pseudo-physical address supported by the VM, XENMEM_maximum_ram_page is
> > supposed to return the max memory page, which could go above the
> > addressibility limit of the VM.
> > 
> > So XENMEM_maximum_ram_page should potentially be able to return (1<<44)
> > even when called by an aarch32 VM, with max IPA 40-bit.
> 
> I am a bit confused with what you wrote. Yes, 32-bit VM can only address
> 40-bit, but this is only limiting its own (guest) physical address space. Such
> VM would still be able to map any host physical address (assuming GFN != MFN).
 
I meant to say the same thing that you wrote, sorry it wasn't clear

 
> > I would imagine it could be useful if dom0 is 32-bit but domUs are
> > 64-bit on a 64-bit hypervisor (which I think it would be a very rare
> > configuration on ARM.)
> 
> Looking at the implementation, the hypercall is accessible by any domain. IOW
> a domU 32-bit could read a wrong value.
> 
> That said, it is not clear to me why an Arm or HVM x86 guest would want to
> read the value.

Right, indeed. AFAICT it is currently unused on ARM.

Going through the code, the only caller that could potentially use it on
ARM is libxl_domain_core_dump and xc_maximum_ram_page is called on the
if ( !auto_translated_physmap ) code path.