Re: [PATCH] pv32: Fix bogus cr2 on fault in emulation gate

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.05.2026 18:34, Andrew Cooper wrote:
> On 20/05/2026 4:51 pm, Teddy Astie wrote:
>> __{put,get}_guest returns -EFAULT on access faults which causes
>> the injected cr2 to be off by 14 bytes (as EFAULT is 14) which is
>> incorrect.
>>
>> Fix the computation by relying on copy_{from,to}_guest_pv which
>> reports the number of remaining bytes instead of a negative errno,
>> such that we can compute the offset properly.
>>
>> Fixes: 70ad570b2799 ("x86/64: paravirt 32-on-64 call gate support")
>> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
>> ---
>>  xen/arch/x86/pv/emul-gate-op.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/x86/pv/emul-gate-op.c b/xen/arch/x86/pv/emul-gate-op.c
>> index c2c699fbff..cacc171115 100644
>> --- a/xen/arch/x86/pv/emul-gate-op.c
>> +++ b/xen/arch/x86/pv/emul-gate-op.c
>> @@ -289,9 +289,10 @@ void pv_emulate_gate_op(struct cpu_user_regs *regs)
>>          int rc;
>>  #define push(item) do \
>>          { \
>> +            unsigned int __value = item; \

As per other comments, this wants to use uint32_t as type.

Given the number of comments, including some back and forth, I think a v2
really needs submitting (rather than one of us doing on-commit edits).

Jan

>>              --stkp; \
>>              esp -= 4; \
>> -            rc = __put_guest(item, stkp); \
>> +            rc = copy_to_guest_pv(stkp, &__value, sizeof(__value)); \
> 
> Oh, this probably violates MISRA, but you don't need to use a separate
> variable because sizeof() has no side effects.
> 
> Given that the expression is now &item, I think it needs to be &(item).
> 
> Can also be fixed on commit.
> 
> ~Andrew