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

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 15/12/2021 07:03, 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?
It looks like there was a lot of discussion afterwards. I will read 
everything and answer on the last part of the thread :).
> > > Get rid of the exceptions by letting compat handlers always return int
> > > and others always return long.
> > >
> > > For the compat hvm case use eax instead of rax for the stored result as
> > > it should have been from the beginning.
> > >
> > > Additionally move some prototypes to include/asm-x86/hypercall.h
> > > as they are x86 specific. Move the do_physdev_op() prototype from both
> > > architecture dependant headers to the common one. Move the
> > > compat_platform_op() prototype to the common header.
> > >
> > > Switch some non style compliant types (u32, s32, s64) to style compliant
> > > ones.
> > TBH, I think this should have been split because the modifications are 
> > done on lines that are untouched.
> > The extra patch would have made the review easier (even if this patch 
> > is still quite small).
> I can split the patch if you want.
I have already reviewed this patch. So it is not going to help me :). 
However, I would appreciate if in the future the coding style changes 
are separated at least when they are not meant to be untouched.
Cheers,

--
Julien Grall