Re: [PATCH v2] xen/arm: do not read MVFR2 when is not defined

[Julien Grall <julien@xen.org>]

Hi Jan,

On 11/01/2021 08:49, Jan Beulich wrote:
> On 08.01.2021 20:22, Stefano Stabellini wrote:
>> MVFR2 is not available on ARMv7. It is available on ARMv8 aarch32 and
>> aarch64. If Xen reads MVFR2 on ARMv7 it could crash.
>>
>> Avoid the issue by doing the following:
>>
>> - define MVFR2_MAYBE_UNDEFINED on arm32
>> - if MVFR2_MAYBE_UNDEFINED, do not attempt to read MVFR2 in Xen
>> - keep the 3rd register_t in struct cpuinfo_arm.mvfr on arm32 so that a
>>    guest read to the register returns '0' instead of crashing the guest.
>>
>> '0' is an appropriate value to return to the guest because it is defined
>> as "no support for miscellaneous features".
>>
>> Aarch64 Xen is not affected by this patch.
> 
> But it looks to also be affected by ...

AFAICT, the smoke test passed on Laxton0 (AMD Seattle) [1] over the 
week-end.

>> Fixes: 9cfdb489af81 ("xen/arm: Add ID registers and complete cpuinfo")
> 
> ... this, faulting (according to osstest logs) early during boot on

The xen-unstable flight [2] ran on Rochester0 (Cavium Thunder-X). So 
this has something to do with the platform.

The main difference is AMD Seattle supports AArch32 while Cavium 
Thunder-X doesn't.

> 000000000025D314	mrs	x1, id_pfr2_el1
This register contains information for the AArch32 state.

AFAICT, the Arm Arm back to at least ARM DDI 0487A.j (published in 2016) 
described the encoding as Read-Only. So I am not sure why we receive an 
UNDEF here, the more it looks like ID_PFR{0, 1}_EL1 were correctly accessed.

Andre, Bertrand, do you have any clue?

However, most of the AArch32 ID registers are UNKNOWN on platform not 
implementing AArch32. So we may want to conditionally skip the access to 
AArch32 state.

Cheers,

[1] 
http://logs.test-lab.xenproject.org/osstest/logs/158293/test-arm64-arm64-xl-xsm/info.html

> 
> Jan
> 

[1]


-- 
Julien Grall