On 16.12.2021 15:55, Julien Grall wrote:
> Hi,
>
> On 16/12/2021 14:26, Michal Orzel wrote:
>> On 16.12.2021 14:50, Jan Beulich wrote:
>>> On 16.12.2021 10:21, Michal Orzel wrote:
>>>> to hypervisor when switching from AArch32 state.
>>>>
>>>> According to section D1.20.2 of Arm Arm(DDI 0487A.j):
>>>> "If the general-purpose register was accessible from AArch32 state the
>>>> upper 32 bits either become zero, or hold the value that the same
>>>> architectural register held before any AArch32 execution.
>>>> The choice between these two options is IMPLEMENTATION DEFINED"
>>>>
>>>> Currently Xen does not ensure that the top 32 bits are zeroed and this
>>>> needs to be fixed. The reason why is that there are places in Xen
>>>> where we assume that top 32bits are zero for AArch32 guests.
>>>> If they are not, this can lead to misinterpretation of Xen regarding
>>>> what the guest requested. For example hypercalls returning an error
>>>> encoded in a signed long like do_sched_op, do_hmv_op, do_memory_op
>>>> would return -ENOSYS if the command passed as the first argument was
>>>> clobbered.
>>>>
>>>> Create a macro clobber_gp_top_halves to clobber top 32 bits of gp
>>>> registers when hyp == 0 (guest mode) and compat == 1 (AArch32 mode).
>>>> Add a compile time check to ensure that save_x0_x1 == 1 if
>>>> compat == 1.
>>>>
>>>> Signed-off-by: Michal Orzel <michal.or...@arm.com>
>>>> ---
>>>> xen/arch/arm/arm64/entry.S | 29 +++++++++++++++++++++++++++++
>>>> 1 file changed, 29 insertions(+)
>>>>
>>>> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
>>>> index fc3811ad0a..01f32324d0 100644
>>>> --- a/xen/arch/arm/arm64/entry.S
>>>> +++ b/xen/arch/arm/arm64/entry.S
>>>> @@ -102,6 +102,30 @@
>>>> .endif
>>>> .endm
>>>> +
>>>> +/*
>>>> + * Clobber top 32 bits of gp registers when switching from AArch32
>>>> + */
>>>> + .macro clobber_gp_top_halves, compat, save_x0_x1
>>>> +
>>>> + .if \compat == 1 /* AArch32 mode */
>>>> +
>>>> + /*
>>>> + * save_x0_x1 is equal to 0 only for guest_sync (compat == 0).
>>>> + * Add a compile time check to avoid violating this rule.
>>>> + */
>>>> + .if \save_x0_x1 == 0
>>>> + .error "save_x0_x1 is 0 but compat is 1"
>>>> + .endif
>>>> +
>>>> + .irp
>>>> n,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29
>>>> + mov w\n, w\n
>>>> + .endr
>>>
>>> What about x30 (aka lr)?
>>>
>> Well the docs says about gp registers as a whole so including lr.
>> However I do not see how clobbering lr would impact Xen.
>
> Xen may not be directly impacted. However this may be used by some userspace
> application (such as for VM introspection) and could be dumped on the console.
>
> So I would cover all the GPR to give a consistent view to everyone.
>
Ok I fully understand. I will send this change as v3.
> Cheers,
>
Cheers,
Michal