Re: [RFC PATCH] x86/xen: Fix PVH dom0 xen_hypercall detection

[Alejandro Vallejo <agarciav@xxxxxxx>]

On Fri Apr 11, 2025 at 2:08 PM BST, Jan Beulich wrote:
> On 11.04.2025 14:46, Alejandro Vallejo wrote:
>> On Thu Apr 10, 2025 at 8:50 PM BST, Jason Andryuk wrote:
>>> A Xen PVH dom0 on an AMD processor triple faults early in boot on
>>> 6.6.86.  CPU detection appears to fail, as the faulting instruction is
>>> vmcall in xen_hypercall_intel() and not vmmcall in xen_hypercall_amd().
>>>
>>> Detection fails because __xen_hypercall_setfunc() returns the full
>>> kernel mapped address of xen_hypercall_amd() or xen_hypercall_intel() -
>>> e.g. 0xffffffff815b93f0.  But this is compared against the rip-relative
>>> xen_hypercall_amd(%rip), which when running from identity mapping, is
>>> only 0x015b93f0.
>>>
>>> Replace the rip-relative address with just loading the actual address to
>>> restore the proper comparision.
>>>
>>> This only seems to affect PVH dom0 boot.  This is probably because the
>>> XENMEM_memory_map hypercall is issued early on from the identity
>>> mappings.  With a domU, the memory map is provided via hvm_start_info
>>> and the hypercall is skipped.  The domU is probably running from the
>>> kernel high mapping when it issues hypercalls.
>>>
>>> Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
>>> ---
>>> I think this sort of address mismatch would be addresed by
>>> e8fbc0d9cab6 ("x86/pvh: Call C code via the kernel virtual mapping")
>>>
>>> That could be backported instead, but it depends on a fair number of
>>> patches.
>>>
>>> Not sure on how getting a patch just into 6.6 would work.  This patch
>>> could go into upstream Linux though it's not strictly necessary when the
>>> rip-relative address is a high address.
>>> ---
>>>  arch/x86/xen/xen-head.S | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
>>> index 059f343da76d..71a0eda2da60 100644
>>> --- a/arch/x86/xen/xen-head.S
>>> +++ b/arch/x86/xen/xen-head.S
>>> @@ -117,7 +117,7 @@ SYM_FUNC_START(xen_hypercall_hvm)
>>>     pop %ebx
>>>     pop %eax
>>>  #else
>>> -   lea xen_hypercall_amd(%rip), %rcx
>>> +   mov $xen_hypercall_amd, %rcx
>> 
>> (Now that this is known to be the fix upstream) This probably wants to
>> be plain lea without RIP-relative addressing, like the x86_32 branch
>> above?
>
> Why would you want to use LEA there? It's functionally identical, but the
> MOV can be encoded without ModR/M byte.
>
> Jan

It's not the using of a particular encoding that I meant, but not using
the same on both 32 and 64 bit paths. Surely whatever argument in favour
of either would hold for both 32 and 64 bits.

Cheers,
Alejandro