Re: [PATCH] {hyper,multi}call: further limit arguments to just 5

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 29/04/2025 9:16 am, Jan Beulich wrote:
> Multicall compat translation and hypercall continuation handling can
> also be shrunk to the processing of just (up to) 5 arguments.
>
> Take the opportunity to
> - make exceeding the limit noisy in hypercall_create_continuation(),
> - use speculation-safe array access in hypercall_create_continuation(),
> - avoid a Misra C:2012 Rule 19.1 violation in xlat_multicall_entry(),
> - further tidy xlat_multicall_entry() and __trace_multicall_call()
>   style-wise.
>
> Amends: 2f531c122e95 ("x86: limit number of hypercall parameters to 5")
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

> ---
> hypercall_xlat_continuation() uses BUG() when encountering too large an
> argument count in release builds, but I think that's too harsh. Hence in
> hypercall_create_continuation() I'm re-using the existing error path.
> Interestingly the multicall part of hypercall_xlat_continuation() has no
> check at all which would cover release builds.

This has never been the best of code.  All we can do is improve it
moving forward.

>
> With gcc14 code size grows according to my observation, due to the loops
> in xlat_multicall_entry() and __trace_multicall_call() both being
> unrolled now.

With GCC-12, it gets marginally smaller.  The loops are unrolled and the
instruction scheduling is rather odd.

mov    0x208(%r15),%rax
mov    0x228(%r15),%rdx
mov    %eax,0x1c(%rsp)
mov    0x210(%r15),%rax
mov    %edx,0x2c(%rsp)
mov    %eax,0x20(%rsp)
mov    0x218(%r15),%rax
mov    0x1c(%rsp),%rsi
mov    %edx,0x210(%r15)
mov    %eax,0x24(%rsp)
mov    0x220(%r15),%rax
mov    %rsi,0x200(%r15)
mov    %eax,0x28(%rsp)
mov    0x24(%rsp),%rsi
mov    %rsi,0x208(%r15)

This is the interleaved copy onto the stack, and back off.

Weirdly, we're doing 8 byte loads and 4 byte stores to copy on to the
stack.  It's not wrong per say, but it's also not necessary.

Then, for the copy-off, there is an overlapping store:

mov    %edx,0x210(%r15)

and

mov    %rsi,0x208(%r15)

not to mention this:

mov    %eax,0x28(%rsp)
mov    0x24(%rsp),%rsi

using an overlapping store and wider load for the copy-off.

Anyway - it looks to function correctly, but I don't exactly feel as if
the optimiser has done a great job.

~Andrew