xen/arch/arm/arm64/vsysreg.c | 82 ++++++++++++++++++++ xen/arch/arm/cpufeature.c | 113 ++++++++++++++++++++++------ xen/arch/arm/traps.c | 7 +- xen/arch/arm/vcpreg.c | 102 +++++++++++++++++++++++++ xen/include/asm-arm/arm64/hsr.h | 37 +++++++++ xen/include/asm-arm/arm64/sysregs.h | 28 +++++++ xen/include/asm-arm/cpregs.h | 15 ++++ xen/include/asm-arm/cpufeature.h | 58 +++++++++++--- xen/include/asm-arm/perfc_defn.h | 1 + xen/include/asm-arm/traps.h | 1 + 10 files changed, 409 insertions(+), 35 deletions(-)
The goal of this serie is to emulate coprocessor ID registers so that Xen only publish to guest features that are supported by Xen and can actually be used by guests. One practical example where this is required are SVE support which is forbidden by Xen as it is not supported, but if Linux is compiled with it, it will crash on boot. An other one is AMU which is also forbidden by Xen but one Linux compiled with it would crash if the platform supports it. To be able to emulate the coprocessor registers defining what features are supported by the hardware, the TID3 bit of HCR must be disabled and Xen must emulated the values of those registers when an exception is catched when a guest is accessing it. This serie is first creating a guest cpuinfo structure which will contain the values that we want to publish to the guests and then provides the proper emulationg for those registers when Xen is getting an exception due to an access to any of those registers. This is a first simple implementation to solve the problem and the way to define the values that we provide to guests and which features are disabled will be in a future patchset enhance so that we could decide per guest what can be used or not and depending on this deduce the bits to activate in HCR and the values that we must publish on ID registers. --- Changes in V2: Fix First patch to properly handle DFR1 register and increase dbg32 size. Other patches have just been rebased. Changes in V3: Add handling of reserved registers as RAZ Minor fixes described in each patch Changes in V4: Add a patch to switch implementation to use READ_SYSREG instead of the 32/64 bit version of it. Move cases for reserved register handling from macros to the code itself. Various typos fixes. Bertrand Marquis (8): xen/arm: Use READ_SYSREG instead of 32/64 versions xen/arm: Add ID registers and complete cpuinfo xen/arm: Add arm64 ID registers definitions xen/arm: create a cpuinfo structure for guest xen/arm: Add handler for ID registers on arm64 xen/arm: Add handler for cp15 ID registers xen/arm: Add CP10 exception support to handle MVFR xen/arm: Activate TID3 in HCR_EL2 xen/arch/arm/arm64/vsysreg.c | 82 ++++++++++++++++++++ xen/arch/arm/cpufeature.c | 113 ++++++++++++++++++++++------ xen/arch/arm/traps.c | 7 +- xen/arch/arm/vcpreg.c | 102 +++++++++++++++++++++++++ xen/include/asm-arm/arm64/hsr.h | 37 +++++++++ xen/include/asm-arm/arm64/sysregs.h | 28 +++++++ xen/include/asm-arm/cpregs.h | 15 ++++ xen/include/asm-arm/cpufeature.h | 58 +++++++++++--- xen/include/asm-arm/perfc_defn.h | 1 + xen/include/asm-arm/traps.h | 1 + 10 files changed, 409 insertions(+), 35 deletions(-) -- 2.17.1
On Thu, 17 Dec 2020, Bertrand Marquis wrote:
> The goal of this serie is to emulate coprocessor ID registers so that
> Xen only publish to guest features that are supported by Xen and can
> actually be used by guests.
> One practical example where this is required are SVE support which is
> forbidden by Xen as it is not supported, but if Linux is compiled with
> it, it will crash on boot. An other one is AMU which is also forbidden
> by Xen but one Linux compiled with it would crash if the platform
> supports it.
>
> To be able to emulate the coprocessor registers defining what features
> are supported by the hardware, the TID3 bit of HCR must be disabled and
> Xen must emulated the values of those registers when an exception is
> catched when a guest is accessing it.
>
> This serie is first creating a guest cpuinfo structure which will
> contain the values that we want to publish to the guests and then
> provides the proper emulationg for those registers when Xen is getting
> an exception due to an access to any of those registers.
>
> This is a first simple implementation to solve the problem and the way
> to define the values that we provide to guests and which features are
> disabled will be in a future patchset enhance so that we could decide
> per guest what can be used or not and depending on this deduce the bits
> to activate in HCR and the values that we must publish on ID registers.
As per our discussion I think we want to add this to the series.
---
xen/arm: clarify support status for various ARMv8.x CPUs
ARMv8.1+ is not security supported for now, as it would require more
investigation on hardware features that Xen has to hide from the guest.
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
diff --git a/SUPPORT.md b/SUPPORT.md
index ab02aca5f4..d95ce3a411 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -37,7 +37,8 @@ supported in this document.
### ARM v8
- Status: Supported
+ Status, ARMv8.0: Supported
+ Status, ARMv8.1+: Supported, not security supported
Status, Cortex A57 r0p0-r1p1: Supported, not security supported
For the Cortex A57 r0p0 - r1p1, see Errata 832075.
Hi Stefano, > On 17 Dec 2020, at 23:47, Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Thu, 17 Dec 2020, Bertrand Marquis wrote: >> The goal of this serie is to emulate coprocessor ID registers so that >> Xen only publish to guest features that are supported by Xen and can >> actually be used by guests. >> One practical example where this is required are SVE support which is >> forbidden by Xen as it is not supported, but if Linux is compiled with >> it, it will crash on boot. An other one is AMU which is also forbidden >> by Xen but one Linux compiled with it would crash if the platform >> supports it. >> >> To be able to emulate the coprocessor registers defining what features >> are supported by the hardware, the TID3 bit of HCR must be disabled and >> Xen must emulated the values of those registers when an exception is >> catched when a guest is accessing it. >> >> This serie is first creating a guest cpuinfo structure which will >> contain the values that we want to publish to the guests and then >> provides the proper emulationg for those registers when Xen is getting >> an exception due to an access to any of those registers. >> >> This is a first simple implementation to solve the problem and the way >> to define the values that we provide to guests and which features are >> disabled will be in a future patchset enhance so that we could decide >> per guest what can be used or not and depending on this deduce the bits >> to activate in HCR and the values that we must publish on ID registers. > > As per our discussion I think we want to add this to the series. Fully agree. > > --- > > xen/arm: clarify support status for various ARMv8.x CPUs > > ARMv8.1+ is not security supported for now, as it would require more > investigation on hardware features that Xen has to hide from the guest. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Cheers Bertrand > > diff --git a/SUPPORT.md b/SUPPORT.md > index ab02aca5f4..d95ce3a411 100644 > --- a/SUPPORT.md > +++ b/SUPPORT.md > @@ -37,7 +37,8 @@ supported in this document. > > ### ARM v8 > > - Status: Supported > + Status, ARMv8.0: Supported > + Status, ARMv8.1+: Supported, not security supported > Status, Cortex A57 r0p0-r1p1: Supported, not security supported > > For the Cortex A57 r0p0 - r1p1, see Errata 832075.
(+ Ian and Andre) Hi Bertrand, On IRC, Ian pointed out that the smoke test was failing on Cubietruck. The only patches because the last success and the failure are your series. This seems to be a very early failure as there is no output from Xen [1]. I originally thought the problem was because some of the ID registers (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7. But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be RAZ. So it would result to a crash. Andre confirmed that PFR2 can be accessed by writing a small baremetal code and booted on Cortex-A7 and Cortex-A20. So I am not entirely sure what's the problem. Andre kindly accepted to try to boot Xen on his board. Hopefully, this will give us a clue on the problem. If not, I will borrow a Cubietruck in OssTest and see if I can reproduce it and debug it. Cheers, On 17/12/2020 15:38, Bertrand Marquis wrote: > The goal of this serie is to emulate coprocessor ID registers so that > Xen only publish to guest features that are supported by Xen and can > actually be used by guests. > One practical example where this is required are SVE support which is > forbidden by Xen as it is not supported, but if Linux is compiled with > it, it will crash on boot. An other one is AMU which is also forbidden > by Xen but one Linux compiled with it would crash if the platform > supports it. > > To be able to emulate the coprocessor registers defining what features > are supported by the hardware, the TID3 bit of HCR must be disabled and > Xen must emulated the values of those registers when an exception is > catched when a guest is accessing it. > > This serie is first creating a guest cpuinfo structure which will > contain the values that we want to publish to the guests and then > provides the proper emulationg for those registers when Xen is getting > an exception due to an access to any of those registers. > > This is a first simple implementation to solve the problem and the way > to define the values that we provide to guests and which features are > disabled will be in a future patchset enhance so that we could decide > per guest what can be used or not and depending on this deduce the bits > to activate in HCR and the values that we must publish on ID registers. > > --- > Changes in V2: > Fix First patch to properly handle DFR1 register and increase dbg32 > size. Other patches have just been rebased. > > Changes in V3: > Add handling of reserved registers as RAZ > Minor fixes described in each patch > > Changes in V4: > Add a patch to switch implementation to use READ_SYSREG instead of the > 32/64 bit version of it. > Move cases for reserved register handling from macros to the code > itself. > Various typos fixes. > > Bertrand Marquis (8): > xen/arm: Use READ_SYSREG instead of 32/64 versions > xen/arm: Add ID registers and complete cpuinfo > xen/arm: Add arm64 ID registers definitions > xen/arm: create a cpuinfo structure for guest > xen/arm: Add handler for ID registers on arm64 > xen/arm: Add handler for cp15 ID registers > xen/arm: Add CP10 exception support to handle MVFR > xen/arm: Activate TID3 in HCR_EL2 > > xen/arch/arm/arm64/vsysreg.c | 82 ++++++++++++++++++++ > xen/arch/arm/cpufeature.c | 113 ++++++++++++++++++++++------ > xen/arch/arm/traps.c | 7 +- > xen/arch/arm/vcpreg.c | 102 +++++++++++++++++++++++++ > xen/include/asm-arm/arm64/hsr.h | 37 +++++++++ > xen/include/asm-arm/arm64/sysregs.h | 28 +++++++ > xen/include/asm-arm/cpregs.h | 15 ++++ > xen/include/asm-arm/cpufeature.h | 58 +++++++++++--- > xen/include/asm-arm/perfc_defn.h | 1 + > xen/include/asm-arm/traps.h | 1 + > 10 files changed, 409 insertions(+), 35 deletions(-) > [1] http://logs.test-lab.xenproject.org/osstest/logs/158152/test-armhf-armhf-xl/info.html -- Julien Grall
On 05/01/2021 16:06, Julien Grall wrote:
> (+ Ian and Andre)
>
> Hi Bertrand,
>
> On IRC, Ian pointed out that the smoke test was failing on Cubietruck.
> The only patches because the last success and the failure are your series.
>
> This seems to be a very early failure as there is no output from Xen [1].
>
> I originally thought the problem was because some of the ID registers
> (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
>
> But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be
> RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
> accessed by writing a small baremetal code and booted on Cortex-A7 and
> Cortex-A20.
>
> So I am not entirely sure what's the problem. Andre kindly accepted to
> try to boot Xen on his board. Hopefully, this will give us a clue on the
> problem.
>
> If not, I will borrow a Cubietruck in OssTest and see if I can reproduce
> it and debug it.
So I just compiled master and staging and ran just that on an Allwinner
H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
missing Dom0, as expected). However staging indeed fails:
(XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
(arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y Tue Jan 5
16:09:40 GMT 2021
(XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
(XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
(XEN) CPU0: Unexpected Trap: Undefined Instruction
(XEN) ----[ Xen-4.15-unstable arm32 debug=y Not tainted ]----
(XEN) CPU: 0
(XEN) PC: 0026b8c8 identify_cpu+0xc0/0xd4
(XEN) CPSR: 600001da MODE:Hypervisor
(XEN) R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
(XEN) R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
(XEN) R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c R12:00000080
(XEN) HYP: SP: 002f7e68 LR: 002c419c
(XEN)
(XEN) VTCR_EL2: 80002646
(XEN) VTTBR_EL2: 00000018e628bb80
(XEN)
(XEN) SCTLR_EL2: 30cd187f
(XEN) HCR_EL2: 00000038
(XEN) TTBR0_EL2: 000000004013a000
(XEN)
(XEN) ESR_EL2: 00000000
(XEN) HPFAR_EL2: 0003fff0
(XEN) HDFAR: 9d110000
(XEN) HIFAR: 0000a04a
(XEN)
(XEN) Xen stack trace from sp=002f7e68:
(XEN) 00000000 002f7f54 00008000 00000000 00002000 002a4584 00000000
00000000
(XEN) 00000000 00008000 49ff5000 002d81f0 40000000 00000000 00002000
00000001
(XEN) 00000000 50000000 49ffd000 00000000 50000000 00000000 00000000
50000000
(XEN) 4c000000 00000000 4e000000 00000000 ffffffff ffffffff 50000000
00000000
(XEN) 50000000 00000000 50000000 00000000 00000000 00000000 00000000
00000000
(XEN) 00000000 00000003 00000000 00000000 ffffffff 00000040 ffffffff
00000000
(XEN) 00000000 00000000 00000000 002a7000 40008050 0000001a 00000000
49ff5000
(XEN) 40008000 3fe08000 00000004 0020006c 00000000 00000000 00000000
00000000
(XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
(XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
(XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
(XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
(XEN) 00000000 00000000 00000000 00000000 00000000 00000000
(XEN) Xen call trace:
(XEN) [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
(XEN) [<002c419c>] start_xen+0x778/0xe50 (LR)
(XEN) [<002f7f54>] 002f7f54
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) CPU0: Unexpected Trap: Undefined Instruction
(XEN) ****************************************
(XEN)
(XEN) Reboot in five seconds...
The code in question:
26b8c0: eef63a10 vmrs r3, mvfr1
26b8c4: e5803058 str r3, [r0, #88] ; 0x58
> 26b8c8: eef53a10 vmrs r3, mvfr2
26b8cc: e580305c str r3, [r0, #92] ; 0x5c
26b8d0: e28bd000 add sp, fp, #0
26b8d4: e49db004 pop {fp} ; (ldr fp, [sp], #4)
26b8d8: e12fff1e bx lr
And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
the CP15 CPUID registers this is using the VMRS instruction, so it's not
protected by future-proof CPUID register scheme.
Not sure what to do about this, maybe #ifdef'ing this register access
with arm64?
I guess this comes from the slightly too optimistic code-sharing between
arm32 and arm64?
Cheers,
Andre
> On 17/12/2020 15:38, Bertrand Marquis wrote:
>> The goal of this serie is to emulate coprocessor ID registers so that
>> Xen only publish to guest features that are supported by Xen and can
>> actually be used by guests.
>> One practical example where this is required are SVE support which is
>> forbidden by Xen as it is not supported, but if Linux is compiled with
>> it, it will crash on boot. An other one is AMU which is also forbidden
>> by Xen but one Linux compiled with it would crash if the platform
>> supports it.
>>
>> To be able to emulate the coprocessor registers defining what features
>> are supported by the hardware, the TID3 bit of HCR must be disabled and
>> Xen must emulated the values of those registers when an exception is
>> catched when a guest is accessing it.
>>
>> This serie is first creating a guest cpuinfo structure which will
>> contain the values that we want to publish to the guests and then
>> provides the proper emulationg for those registers when Xen is getting
>> an exception due to an access to any of those registers.
>>
>> This is a first simple implementation to solve the problem and the way
>> to define the values that we provide to guests and which features are
>> disabled will be in a future patchset enhance so that we could decide
>> per guest what can be used or not and depending on this deduce the bits
>> to activate in HCR and the values that we must publish on ID registers.
>>
>> ---
>> Changes in V2:
>> Fix First patch to properly handle DFR1 register and increase dbg32
>> size. Other patches have just been rebased.
>>
>> Changes in V3:
>> Add handling of reserved registers as RAZ
>> Minor fixes described in each patch
>>
>> Changes in V4:
>> Add a patch to switch implementation to use READ_SYSREG instead of the
>> 32/64 bit version of it.
>> Move cases for reserved register handling from macros to the code
>> itself.
>> Various typos fixes.
>>
>> Bertrand Marquis (8):
>> xen/arm: Use READ_SYSREG instead of 32/64 versions
>> xen/arm: Add ID registers and complete cpuinfo
>> xen/arm: Add arm64 ID registers definitions
>> xen/arm: create a cpuinfo structure for guest
>> xen/arm: Add handler for ID registers on arm64
>> xen/arm: Add handler for cp15 ID registers
>> xen/arm: Add CP10 exception support to handle MVFR
>> xen/arm: Activate TID3 in HCR_EL2
>>
>> xen/arch/arm/arm64/vsysreg.c | 82 ++++++++++++++++++++
>> xen/arch/arm/cpufeature.c | 113 ++++++++++++++++++++++------
>> xen/arch/arm/traps.c | 7 +-
>> xen/arch/arm/vcpreg.c | 102 +++++++++++++++++++++++++
>> xen/include/asm-arm/arm64/hsr.h | 37 +++++++++
>> xen/include/asm-arm/arm64/sysregs.h | 28 +++++++
>> xen/include/asm-arm/cpregs.h | 15 ++++
>> xen/include/asm-arm/cpufeature.h | 58 +++++++++++---
>> xen/include/asm-arm/perfc_defn.h | 1 +
>> xen/include/asm-arm/traps.h | 1 +
>> 10 files changed, 409 insertions(+), 35 deletions(-)
>>
>
> [1]
> http://logs.test-lab.xenproject.org/osstest/logs/158152/test-armhf-armhf-xl/info.html
>
>
On Tue, 5 Jan 2021, André Przywara wrote:
> On 05/01/2021 16:06, Julien Grall wrote:
> > (+ Ian and Andre)
> >
> > Hi Bertrand,
> >
> > On IRC, Ian pointed out that the smoke test was failing on Cubietruck.
> > The only patches because the last success and the failure are your series.
> >
> > This seems to be a very early failure as there is no output from Xen [1].
> >
> > I originally thought the problem was because some of the ID registers
> > (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
> >
> > But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be
> > RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
> > accessed by writing a small baremetal code and booted on Cortex-A7 and
> > Cortex-A20.
> >
> > So I am not entirely sure what's the problem. Andre kindly accepted to
> > try to boot Xen on his board. Hopefully, this will give us a clue on the
> > problem.
> >
> > If not, I will borrow a Cubietruck in OssTest and see if I can reproduce
> > it and debug it.
>
>
> So I just compiled master and staging and ran just that on an Allwinner
> H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
> missing Dom0, as expected). However staging indeed fails:
>
> (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
> (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y Tue Jan 5
> 16:09:40 GMT 2021
> (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
> (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
> (XEN) CPU0: Unexpected Trap: Undefined Instruction
> (XEN) ----[ Xen-4.15-unstable arm32 debug=y Not tainted ]----
> (XEN) CPU: 0
> (XEN) PC: 0026b8c8 identify_cpu+0xc0/0xd4
> (XEN) CPSR: 600001da MODE:Hypervisor
> (XEN) R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
> (XEN) R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
> (XEN) R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c R12:00000080
> (XEN) HYP: SP: 002f7e68 LR: 002c419c
> (XEN)
> (XEN) VTCR_EL2: 80002646
> (XEN) VTTBR_EL2: 00000018e628bb80
> (XEN)
> (XEN) SCTLR_EL2: 30cd187f
> (XEN) HCR_EL2: 00000038
> (XEN) TTBR0_EL2: 000000004013a000
> (XEN)
> (XEN) ESR_EL2: 00000000
> (XEN) HPFAR_EL2: 0003fff0
> (XEN) HDFAR: 9d110000
> (XEN) HIFAR: 0000a04a
> (XEN)
> (XEN) Xen stack trace from sp=002f7e68:
> (XEN) 00000000 002f7f54 00008000 00000000 00002000 002a4584 00000000
> 00000000
> (XEN) 00000000 00008000 49ff5000 002d81f0 40000000 00000000 00002000
> 00000001
> (XEN) 00000000 50000000 49ffd000 00000000 50000000 00000000 00000000
> 50000000
> (XEN) 4c000000 00000000 4e000000 00000000 ffffffff ffffffff 50000000
> 00000000
> (XEN) 50000000 00000000 50000000 00000000 00000000 00000000 00000000
> 00000000
> (XEN) 00000000 00000003 00000000 00000000 ffffffff 00000040 ffffffff
> 00000000
> (XEN) 00000000 00000000 00000000 002a7000 40008050 0000001a 00000000
> 49ff5000
> (XEN) 40008000 3fe08000 00000004 0020006c 00000000 00000000 00000000
> 00000000
> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
> (XEN) Xen call trace:
> (XEN) [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
> (XEN) [<002c419c>] start_xen+0x778/0xe50 (LR)
> (XEN) [<002f7f54>] 002f7f54
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) CPU0: Unexpected Trap: Undefined Instruction
> (XEN) ****************************************
> (XEN)
> (XEN) Reboot in five seconds...
>
>
> The code in question:
> 26b8c0: eef63a10 vmrs r3, mvfr1
> 26b8c4: e5803058 str r3, [r0, #88] ; 0x58
> > 26b8c8: eef53a10 vmrs r3, mvfr2
> 26b8cc: e580305c str r3, [r0, #92] ; 0x5c
> 26b8d0: e28bd000 add sp, fp, #0
> 26b8d4: e49db004 pop {fp} ; (ldr fp, [sp], #4)
> 26b8d8: e12fff1e bx lr
>
> And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
> the CP15 CPUID registers this is using the VMRS instruction, so it's not
> protected by future-proof CPUID register scheme.
>
> Not sure what to do about this, maybe #ifdef'ing this register access
> with arm64?
> I guess this comes from the slightly too optimistic code-sharing between
> arm32 and arm64?
Yes and #ifdef'ing is what we have been doing with the other registers.
It looks OK in cpufeature.c; it looks ugly in cpufeature.h but I
couldn't come up with something nicer at the moment.
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 1f6a85aafe..9e3377eae3 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -150,7 +150,9 @@ void identify_cpu(struct cpuinfo_arm *c)
c->mvfr.bits[0] = READ_SYSREG(MVFR0_EL1);
c->mvfr.bits[1] = READ_SYSREG(MVFR1_EL1);
+#ifdef CONFIG_ARM_64
c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
+#endif
}
/*
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 6058744c18..29a63a91c8 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -271,9 +271,15 @@ struct cpuinfo_arm {
uint32_t bits[7];
} isa32;
+#ifdef CONFIG_ARM_64
struct {
register_t bits[3];
} mvfr;
+#else
+ struct {
+ register_t bits[2];
+ } mvfr;
+#endif
};
extern struct cpuinfo_arm boot_cpu_data;
Hi Stefano,
On 05/01/2021 18:44, Stefano Stabellini wrote:
> On Tue, 5 Jan 2021, André Przywara wrote:
>> On 05/01/2021 16:06, Julien Grall wrote:
>>> (+ Ian and Andre)
>>>
>>> Hi Bertrand,
>>>
>>> On IRC, Ian pointed out that the smoke test was failing on Cubietruck.
>>> The only patches because the last success and the failure are your series.
>>>
>>> This seems to be a very early failure as there is no output from Xen [1].
>>>
>>> I originally thought the problem was because some of the ID registers
>>> (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
>>>
>>> But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be
>>> RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
>>> accessed by writing a small baremetal code and booted on Cortex-A7 and
>>> Cortex-A20.
>>>
>>> So I am not entirely sure what's the problem. Andre kindly accepted to
>>> try to boot Xen on his board. Hopefully, this will give us a clue on the
>>> problem.
>>>
>>> If not, I will borrow a Cubietruck in OssTest and see if I can reproduce
>>> it and debug it.
>>
>>
>> So I just compiled master and staging and ran just that on an Allwinner
>> H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
>> missing Dom0, as expected). However staging indeed fails:
>>
>> (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
>> (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y Tue Jan 5
>> 16:09:40 GMT 2021
>> (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
>> (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>> (XEN) ----[ Xen-4.15-unstable arm32 debug=y Not tainted ]----
>> (XEN) CPU: 0
>> (XEN) PC: 0026b8c8 identify_cpu+0xc0/0xd4
>> (XEN) CPSR: 600001da MODE:Hypervisor
>> (XEN) R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
>> (XEN) R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
>> (XEN) R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c R12:00000080
>> (XEN) HYP: SP: 002f7e68 LR: 002c419c
>> (XEN)
>> (XEN) VTCR_EL2: 80002646
>> (XEN) VTTBR_EL2: 00000018e628bb80
>> (XEN)
>> (XEN) SCTLR_EL2: 30cd187f
>> (XEN) HCR_EL2: 00000038
>> (XEN) TTBR0_EL2: 000000004013a000
>> (XEN)
>> (XEN) ESR_EL2: 00000000
>> (XEN) HPFAR_EL2: 0003fff0
>> (XEN) HDFAR: 9d110000
>> (XEN) HIFAR: 0000a04a
>> (XEN)
>> (XEN) Xen stack trace from sp=002f7e68:
>> (XEN) 00000000 002f7f54 00008000 00000000 00002000 002a4584 00000000
>> 00000000
>> (XEN) 00000000 00008000 49ff5000 002d81f0 40000000 00000000 00002000
>> 00000001
>> (XEN) 00000000 50000000 49ffd000 00000000 50000000 00000000 00000000
>> 50000000
>> (XEN) 4c000000 00000000 4e000000 00000000 ffffffff ffffffff 50000000
>> 00000000
>> (XEN) 50000000 00000000 50000000 00000000 00000000 00000000 00000000
>> 00000000
>> (XEN) 00000000 00000003 00000000 00000000 ffffffff 00000040 ffffffff
>> 00000000
>> (XEN) 00000000 00000000 00000000 002a7000 40008050 0000001a 00000000
>> 49ff5000
>> (XEN) 40008000 3fe08000 00000004 0020006c 00000000 00000000 00000000
>> 00000000
>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 00000000
>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 00000000
>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 00000000
>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 00000000
>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
>> (XEN) Xen call trace:
>> (XEN) [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
>> (XEN) [<002c419c>] start_xen+0x778/0xe50 (LR)
>> (XEN) [<002f7f54>] 002f7f54
>> (XEN)
>> (XEN)
>> (XEN) ****************************************
>> (XEN) Panic on CPU 0:
>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>> (XEN) ****************************************
>> (XEN)
>> (XEN) Reboot in five seconds...
>>
>>
>> The code in question:
>> 26b8c0: eef63a10 vmrs r3, mvfr1
>> 26b8c4: e5803058 str r3, [r0, #88] ; 0x58
>>> 26b8c8: eef53a10 vmrs r3, mvfr2
>> 26b8cc: e580305c str r3, [r0, #92] ; 0x5c
>> 26b8d0: e28bd000 add sp, fp, #0
>> 26b8d4: e49db004 pop {fp} ; (ldr fp, [sp], #4)
>> 26b8d8: e12fff1e bx lr
>>
>> And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
>> the CP15 CPUID registers this is using the VMRS instruction, so it's not
>> protected by future-proof CPUID register scheme.
>>
>> Not sure what to do about this, maybe #ifdef'ing this register access
>> with arm64?
>> I guess this comes from the slightly too optimistic code-sharing between
>> arm32 and arm64?
>
> Yes and #ifdef'ing is what we have been doing with the other registers.
There is a catch here. This register is accessible from AArch32 on all
Armv8 HW. It is just not accessible on Armv7.
So hiding the MVFR2 behind #ifdef CONFIG_ARM_64 is technically not correct.
I know that we said that we don't officially support Xen on Arm32 on
Armv8 HW (I can't find where it is written though). So we could argue
that shadowing MVFR2 is not worth it.
I do use Armv8 HW to test 32-bit, so I would at least like to get Xen
booting. In addition to that, Linux 32-bit doesn't access MVFR2 at the
moment.
Therefore, a #ifdef may be acceptable for now. However, I would suggest
to introduce name it MAY_BE_UNDEFINED (or similar) that will be used to
avoid reading the system register on 32-bit.
For the 32-bit case, I would just hardcode the value based on the Arm
(it looks like for Armv8-A there is only one valid value).
IOW, the hack would be self-contained in cpufeature.c.
Cheers,
--
Julien Grall
On Tue, 5 Jan 2021, Julien Grall wrote:
> Hi Stefano,
>
> On 05/01/2021 18:44, Stefano Stabellini wrote:
> > On Tue, 5 Jan 2021, André Przywara wrote:
> > > On 05/01/2021 16:06, Julien Grall wrote:
> > > > (+ Ian and Andre)
> > > >
> > > > Hi Bertrand,
> > > >
> > > > On IRC, Ian pointed out that the smoke test was failing on Cubietruck.
> > > > The only patches because the last success and the failure are your
> > > > series.
> > > >
> > > > This seems to be a very early failure as there is no output from Xen
> > > > [1].
> > > >
> > > > I originally thought the problem was because some of the ID registers
> > > > (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
> > > >
> > > > But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be
> > > > RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
> > > > accessed by writing a small baremetal code and booted on Cortex-A7 and
> > > > Cortex-A20.
> > > >
> > > > So I am not entirely sure what's the problem. Andre kindly accepted to
> > > > try to boot Xen on his board. Hopefully, this will give us a clue on the
> > > > problem.
> > > >
> > > > If not, I will borrow a Cubietruck in OssTest and see if I can reproduce
> > > > it and debug it.
> > >
> > >
> > > So I just compiled master and staging and ran just that on an Allwinner
> > > H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
> > > missing Dom0, as expected). However staging indeed fails:
> > >
> > > (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
> > > (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y Tue Jan 5
> > > 16:09:40 GMT 2021
> > > (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
> > > (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
> > > (XEN) CPU0: Unexpected Trap: Undefined Instruction
> > > (XEN) ----[ Xen-4.15-unstable arm32 debug=y Not tainted ]----
> > > (XEN) CPU: 0
> > > (XEN) PC: 0026b8c8 identify_cpu+0xc0/0xd4
> > > (XEN) CPSR: 600001da MODE:Hypervisor
> > > (XEN) R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
> > > (XEN) R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
> > > (XEN) R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c
> > > R12:00000080
> > > (XEN) HYP: SP: 002f7e68 LR: 002c419c
> > > (XEN)
> > > (XEN) VTCR_EL2: 80002646
> > > (XEN) VTTBR_EL2: 00000018e628bb80
> > > (XEN)
> > > (XEN) SCTLR_EL2: 30cd187f
> > > (XEN) HCR_EL2: 00000038
> > > (XEN) TTBR0_EL2: 000000004013a000
> > > (XEN)
> > > (XEN) ESR_EL2: 00000000
> > > (XEN) HPFAR_EL2: 0003fff0
> > > (XEN) HDFAR: 9d110000
> > > (XEN) HIFAR: 0000a04a
> > > (XEN)
> > > (XEN) Xen stack trace from sp=002f7e68:
> > > (XEN) 00000000 002f7f54 00008000 00000000 00002000 002a4584 00000000
> > > 00000000
> > > (XEN) 00000000 00008000 49ff5000 002d81f0 40000000 00000000 00002000
> > > 00000001
> > > (XEN) 00000000 50000000 49ffd000 00000000 50000000 00000000 00000000
> > > 50000000
> > > (XEN) 4c000000 00000000 4e000000 00000000 ffffffff ffffffff 50000000
> > > 00000000
> > > (XEN) 50000000 00000000 50000000 00000000 00000000 00000000 00000000
> > > 00000000
> > > (XEN) 00000000 00000003 00000000 00000000 ffffffff 00000040 ffffffff
> > > 00000000
> > > (XEN) 00000000 00000000 00000000 002a7000 40008050 0000001a 00000000
> > > 49ff5000
> > > (XEN) 40008000 3fe08000 00000004 0020006c 00000000 00000000 00000000
> > > 00000000
> > > (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000
> > > (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000
> > > (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000
> > > (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000
> > > (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
> > > (XEN) Xen call trace:
> > > (XEN) [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
> > > (XEN) [<002c419c>] start_xen+0x778/0xe50 (LR)
> > > (XEN) [<002f7f54>] 002f7f54
> > > (XEN)
> > > (XEN)
> > > (XEN) ****************************************
> > > (XEN) Panic on CPU 0:
> > > (XEN) CPU0: Unexpected Trap: Undefined Instruction
> > > (XEN) ****************************************
> > > (XEN)
> > > (XEN) Reboot in five seconds...
> > >
> > >
> > > The code in question:
> > > 26b8c0: eef63a10 vmrs r3, mvfr1
> > > 26b8c4: e5803058 str r3, [r0, #88] ; 0x58
> > > > 26b8c8: eef53a10 vmrs r3, mvfr2
> > > 26b8cc: e580305c str r3, [r0, #92] ; 0x5c
> > > 26b8d0: e28bd000 add sp, fp, #0
> > > 26b8d4: e49db004 pop {fp} ; (ldr fp, [sp], #4)
> > > 26b8d8: e12fff1e bx lr
> > >
> > > And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
> > > the CP15 CPUID registers this is using the VMRS instruction, so it's not
> > > protected by future-proof CPUID register scheme.
> > >
> > > Not sure what to do about this, maybe #ifdef'ing this register access
> > > with arm64?
> > > I guess this comes from the slightly too optimistic code-sharing between
> > > arm32 and arm64?
> >
> > Yes and #ifdef'ing is what we have been doing with the other registers.
>
> There is a catch here. This register is accessible from AArch32 on all Armv8
> HW. It is just not accessible on Armv7.
>
> So hiding the MVFR2 behind #ifdef CONFIG_ARM_64 is technically not correct.
>
> I know that we said that we don't officially support Xen on Arm32 on Armv8 HW
> (I can't find where it is written though). So we could argue that shadowing
> MVFR2 is not worth it.
>
> I do use Armv8 HW to test 32-bit, so I would at least like to get Xen booting.
Yep, me too.
> In addition to that, Linux 32-bit doesn't access MVFR2 at the moment.
>
> Therefore, a #ifdef may be acceptable for now. However, I would suggest to
> introduce name it MAY_BE_UNDEFINED (or similar) that will be used to avoid
> reading the system register on 32-bit.
>
> For the 32-bit case, I would just hardcode the value based on the Arm (it
> looks like for Armv8-A there is only one valid value).
>
> IOW, the hack would be self-contained in cpufeature.c.
I think it makes sense that the hack should be self-contained in
cpufeature.c, leaving the definition of struct mvfr to 3 register_t also
on arm32 as it is today. Also leaving vcpreg.c as it is today so that a
guest can try to read mvfr2 without crashing thanks to
GENERATE_TID3_INFO(MVFR2, mvfr, 2).
For the arm32 case in cpufeature.c:identify_cpu, the two permitted
values are 0 and 0b0100, which one did you have in mind? I take you
meant 0 which stands for "not implemented, or no support for
miscellaneous features"?
In regards to:
> I would suggest to introduce name it MAY_BE_UNDEFINED (or similar)
> that will be used to avoid reading the system register on 32-bit.
Did you mean something like the following on arm32 (maybe to
xen/include/asm-arm/arm32/sysregs.h):
#define MVFR2_MAYBE_UNDEFINED
Then in identify_cpu:
#ifndef MVFR2_MAYBE_UNDEFINED
c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
#endif
Hi Stefano,
On 05/01/2021 22:43, Stefano Stabellini wrote:
> On Tue, 5 Jan 2021, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 05/01/2021 18:44, Stefano Stabellini wrote:
>>> On Tue, 5 Jan 2021, André Przywara wrote:
>>>> On 05/01/2021 16:06, Julien Grall wrote:
>>>>> (+ Ian and Andre)
>>>>>
>>>>> Hi Bertrand,
>>>>>
>>>>> On IRC, Ian pointed out that the smoke test was failing on Cubietruck.
>>>>> The only patches because the last success and the failure are your
>>>>> series.
>>>>>
>>>>> This seems to be a very early failure as there is no output from Xen
>>>>> [1].
>>>>>
>>>>> I originally thought the problem was because some of the ID registers
>>>>> (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
>>>>>
>>>>> But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should be
>>>>> RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
>>>>> accessed by writing a small baremetal code and booted on Cortex-A7 and
>>>>> Cortex-A20.
>>>>>
>>>>> So I am not entirely sure what's the problem. Andre kindly accepted to
>>>>> try to boot Xen on his board. Hopefully, this will give us a clue on the
>>>>> problem.
>>>>>
>>>>> If not, I will borrow a Cubietruck in OssTest and see if I can reproduce
>>>>> it and debug it.
>>>>
>>>>
>>>> So I just compiled master and staging and ran just that on an Allwinner
>>>> H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
>>>> missing Dom0, as expected). However staging indeed fails:
>>>>
>>>> (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
>>>> (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y Tue Jan 5
>>>> 16:09:40 GMT 2021
>>>> (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
>>>> (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
>>>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>>>> (XEN) ----[ Xen-4.15-unstable arm32 debug=y Not tainted ]----
>>>> (XEN) CPU: 0
>>>> (XEN) PC: 0026b8c8 identify_cpu+0xc0/0xd4
>>>> (XEN) CPSR: 600001da MODE:Hypervisor
>>>> (XEN) R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
>>>> (XEN) R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
>>>> (XEN) R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c
>>>> R12:00000080
>>>> (XEN) HYP: SP: 002f7e68 LR: 002c419c
>>>> (XEN)
>>>> (XEN) VTCR_EL2: 80002646
>>>> (XEN) VTTBR_EL2: 00000018e628bb80
>>>> (XEN)
>>>> (XEN) SCTLR_EL2: 30cd187f
>>>> (XEN) HCR_EL2: 00000038
>>>> (XEN) TTBR0_EL2: 000000004013a000
>>>> (XEN)
>>>> (XEN) ESR_EL2: 00000000
>>>> (XEN) HPFAR_EL2: 0003fff0
>>>> (XEN) HDFAR: 9d110000
>>>> (XEN) HIFAR: 0000a04a
>>>> (XEN)
>>>> (XEN) Xen stack trace from sp=002f7e68:
>>>> (XEN) 00000000 002f7f54 00008000 00000000 00002000 002a4584 00000000
>>>> 00000000
>>>> (XEN) 00000000 00008000 49ff5000 002d81f0 40000000 00000000 00002000
>>>> 00000001
>>>> (XEN) 00000000 50000000 49ffd000 00000000 50000000 00000000 00000000
>>>> 50000000
>>>> (XEN) 4c000000 00000000 4e000000 00000000 ffffffff ffffffff 50000000
>>>> 00000000
>>>> (XEN) 50000000 00000000 50000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN) 00000000 00000003 00000000 00000000 ffffffff 00000040 ffffffff
>>>> 00000000
>>>> (XEN) 00000000 00000000 00000000 002a7000 40008050 0000001a 00000000
>>>> 49ff5000
>>>> (XEN) 40008000 3fe08000 00000004 0020006c 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
>>>> (XEN) Xen call trace:
>>>> (XEN) [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
>>>> (XEN) [<002c419c>] start_xen+0x778/0xe50 (LR)
>>>> (XEN) [<002f7f54>] 002f7f54
>>>> (XEN)
>>>> (XEN)
>>>> (XEN) ****************************************
>>>> (XEN) Panic on CPU 0:
>>>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>>>> (XEN) ****************************************
>>>> (XEN)
>>>> (XEN) Reboot in five seconds...
>>>>
>>>>
>>>> The code in question:
>>>> 26b8c0: eef63a10 vmrs r3, mvfr1
>>>> 26b8c4: e5803058 str r3, [r0, #88] ; 0x58
>>>>> 26b8c8: eef53a10 vmrs r3, mvfr2
>>>> 26b8cc: e580305c str r3, [r0, #92] ; 0x5c
>>>> 26b8d0: e28bd000 add sp, fp, #0
>>>> 26b8d4: e49db004 pop {fp} ; (ldr fp, [sp], #4)
>>>> 26b8d8: e12fff1e bx lr
>>>>
>>>> And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
>>>> the CP15 CPUID registers this is using the VMRS instruction, so it's not
>>>> protected by future-proof CPUID register scheme.
>>>>
>>>> Not sure what to do about this, maybe #ifdef'ing this register access
>>>> with arm64?
>>>> I guess this comes from the slightly too optimistic code-sharing between
>>>> arm32 and arm64?
>>>
>>> Yes and #ifdef'ing is what we have been doing with the other registers.
>>
>> There is a catch here. This register is accessible from AArch32 on all Armv8
>> HW. It is just not accessible on Armv7.
>>
>> So hiding the MVFR2 behind #ifdef CONFIG_ARM_64 is technically not correct.
>>
>> I know that we said that we don't officially support Xen on Arm32 on Armv8 HW
>> (I can't find where it is written though). So we could argue that shadowing
>> MVFR2 is not worth it.
>>
>> I do use Armv8 HW to test 32-bit, so I would at least like to get Xen booting.
>
> Yep, me too.
>
>
>> In addition to that, Linux 32-bit doesn't access MVFR2 at the moment.
>>
>> Therefore, a #ifdef may be acceptable for now. However, I would suggest to
>> introduce name it MAY_BE_UNDEFINED (or similar) that will be used to avoid
>> reading the system register on 32-bit.
>>
>> For the 32-bit case, I would just hardcode the value based on the Arm (it
>> looks like for Armv8-A there is only one valid value).
>>
>> IOW, the hack would be self-contained in cpufeature.c.
>
> I think it makes sense that the hack should be self-contained in
> cpufeature.c, leaving the definition of struct mvfr to 3 register_t also
> on arm32 as it is today. Also leaving vcpreg.c as it is today so that a
> guest can try to read mvfr2 without crashing thanks to
> GENERATE_TID3_INFO(MVFR2, mvfr, 2).
>
> For the arm32 case in cpufeature.c:identify_cpu, the two permitted
> values are 0 and 0b0100, which one did you have in mind? I take you
> meant 0 which stands for "not implemented, or no support for
> miscellaneous features"?
It looks like I misread the spec. :/ I thought it would only be a single
possible value.
Hmmm... I am not sure about the value here as I don't know what would be
the impact.
It might be better to detect whether we are running on Armv8 HW or Armv7
HW. I haven't figured out an obvious way for that yet.
On IRC, Andre suggested a couple of options on IRC
1) read ISAR5: This doesn't seem to work as it is UNK on Armv7
2) detect the VFP subarchitecture: It is not clear how this can be
leveraged
While skimming through the spec, I noticed that ID_DFR0.CopDbg requires
a specific value for Armv8.x spec. This is the debug architecture, but
maybe we can leverage it?
Any better ideas?
>
>
> In regards to:
>> I would suggest to introduce name it MAY_BE_UNDEFINED (or similar)
>> that will be used to avoid reading the system register on 32-bit.
>
> Did you mean something like the following on arm32 (maybe to
> xen/include/asm-arm/arm32/sysregs.h):
>
> #define MVFR2_MAYBE_UNDEFINED
>
>
> Then in identify_cpu:
>
> #ifndef MVFR2_MAYBE_UNDEFINED
> c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
> #endif
>
Yes. But this may not work if we can't decide on the fixed value (see
above).
Cheers,
--
Julien Grall
On Wed, 6 Jan 2021, Julien Grall wrote:
> On 05/01/2021 22:43, Stefano Stabellini wrote:
> > On Tue, 5 Jan 2021, Julien Grall wrote:
> > > Hi Stefano,
> > >
> > > On 05/01/2021 18:44, Stefano Stabellini wrote:
> > > > On Tue, 5 Jan 2021, André Przywara wrote:
> > > > > On 05/01/2021 16:06, Julien Grall wrote:
> > > > > > (+ Ian and Andre)
> > > > > >
> > > > > > Hi Bertrand,
> > > > > >
> > > > > > On IRC, Ian pointed out that the smoke test was failing on
> > > > > > Cubietruck.
> > > > > > The only patches because the last success and the failure are your
> > > > > > series.
> > > > > >
> > > > > > This seems to be a very early failure as there is no output from Xen
> > > > > > [1].
> > > > > >
> > > > > > I originally thought the problem was because some of the ID
> > > > > > registers
> > > > > > (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
> > > > > >
> > > > > > But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should
> > > > > > be
> > > > > > RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
> > > > > > accessed by writing a small baremetal code and booted on Cortex-A7
> > > > > > and
> > > > > > Cortex-A20.
> > > > > >
> > > > > > So I am not entirely sure what's the problem. Andre kindly accepted
> > > > > > to
> > > > > > try to boot Xen on his board. Hopefully, this will give us a clue on
> > > > > > the
> > > > > > problem.
> > > > > >
> > > > > > If not, I will borrow a Cubietruck in OssTest and see if I can
> > > > > > reproduce
> > > > > > it and debug it.
> > > > >
> > > > >
> > > > > So I just compiled master and staging and ran just that on an
> > > > > Allwinner
> > > > > H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
> > > > > missing Dom0, as expected). However staging indeed fails:
> > > > >
> > > > > (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
> > > > > (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y Tue Jan 5
> > > > > 16:09:40 GMT 2021
> > > > > (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
> > > > > (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
> > > > > (XEN) CPU0: Unexpected Trap: Undefined Instruction
> > > > > (XEN) ----[ Xen-4.15-unstable arm32 debug=y Not tainted ]----
> > > > > (XEN) CPU: 0
> > > > > (XEN) PC: 0026b8c8 identify_cpu+0xc0/0xd4
> > > > > (XEN) CPSR: 600001da MODE:Hypervisor
> > > > > (XEN) R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
> > > > > (XEN) R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
> > > > > (XEN) R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c
> > > > > R12:00000080
> > > > > (XEN) HYP: SP: 002f7e68 LR: 002c419c
> > > > > (XEN)
> > > > > (XEN) VTCR_EL2: 80002646
> > > > > (XEN) VTTBR_EL2: 00000018e628bb80
> > > > > (XEN)
> > > > > (XEN) SCTLR_EL2: 30cd187f
> > > > > (XEN) HCR_EL2: 00000038
> > > > > (XEN) TTBR0_EL2: 000000004013a000
> > > > > (XEN)
> > > > > (XEN) ESR_EL2: 00000000
> > > > > (XEN) HPFAR_EL2: 0003fff0
> > > > > (XEN) HDFAR: 9d110000
> > > > > (XEN) HIFAR: 0000a04a
> > > > > (XEN)
> > > > > (XEN) Xen stack trace from sp=002f7e68:
> > > > > (XEN) 00000000 002f7f54 00008000 00000000 00002000 002a4584
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN) 00000000 00008000 49ff5000 002d81f0 40000000 00000000
> > > > > 00002000
> > > > > 00000001
> > > > > (XEN) 00000000 50000000 49ffd000 00000000 50000000 00000000
> > > > > 00000000
> > > > > 50000000
> > > > > (XEN) 4c000000 00000000 4e000000 00000000 ffffffff ffffffff
> > > > > 50000000
> > > > > 00000000
> > > > > (XEN) 50000000 00000000 50000000 00000000 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN) 00000000 00000003 00000000 00000000 ffffffff 00000040
> > > > > ffffffff
> > > > > 00000000
> > > > > (XEN) 00000000 00000000 00000000 002a7000 40008050 0000001a
> > > > > 00000000
> > > > > 49ff5000
> > > > > (XEN) 40008000 3fe08000 00000004 0020006c 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > 00000000
> > > > > 00000000
> > > > > (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
> > > > > (XEN) Xen call trace:
> > > > > (XEN) [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
> > > > > (XEN) [<002c419c>] start_xen+0x778/0xe50 (LR)
> > > > > (XEN) [<002f7f54>] 002f7f54
> > > > > (XEN)
> > > > > (XEN)
> > > > > (XEN) ****************************************
> > > > > (XEN) Panic on CPU 0:
> > > > > (XEN) CPU0: Unexpected Trap: Undefined Instruction
> > > > > (XEN) ****************************************
> > > > > (XEN)
> > > > > (XEN) Reboot in five seconds...
> > > > >
> > > > >
> > > > > The code in question:
> > > > > 26b8c0: eef63a10 vmrs r3, mvfr1
> > > > > 26b8c4: e5803058 str r3, [r0, #88] ; 0x58
> > > > > > 26b8c8: eef53a10 vmrs r3, mvfr2
> > > > > 26b8cc: e580305c str r3, [r0, #92] ; 0x5c
> > > > > 26b8d0: e28bd000 add sp, fp, #0
> > > > > 26b8d4: e49db004 pop {fp} ; (ldr fp, [sp],
> > > > > #4)
> > > > > 26b8d8: e12fff1e bx lr
> > > > >
> > > > > And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
> > > > > the CP15 CPUID registers this is using the VMRS instruction, so it's
> > > > > not
> > > > > protected by future-proof CPUID register scheme.
> > > > >
> > > > > Not sure what to do about this, maybe #ifdef'ing this register access
> > > > > with arm64?
> > > > > I guess this comes from the slightly too optimistic code-sharing
> > > > > between
> > > > > arm32 and arm64?
> > > >
> > > > Yes and #ifdef'ing is what we have been doing with the other registers.
> > >
> > > There is a catch here. This register is accessible from AArch32 on all
> > > Armv8
> > > HW. It is just not accessible on Armv7.
> > >
> > > So hiding the MVFR2 behind #ifdef CONFIG_ARM_64 is technically not
> > > correct.
> > >
> > > I know that we said that we don't officially support Xen on Arm32 on Armv8
> > > HW
> > > (I can't find where it is written though). So we could argue that
> > > shadowing
> > > MVFR2 is not worth it.
> > >
> > > I do use Armv8 HW to test 32-bit, so I would at least like to get Xen
> > > booting.
> >
> > Yep, me too.
> >
> >
> > > In addition to that, Linux 32-bit doesn't access MVFR2 at the moment.
> > >
> > > Therefore, a #ifdef may be acceptable for now. However, I would suggest to
> > > introduce name it MAY_BE_UNDEFINED (or similar) that will be used to avoid
> > > reading the system register on 32-bit.
> > >
> > > For the 32-bit case, I would just hardcode the value based on the Arm (it
> > > looks like for Armv8-A there is only one valid value).
> > >
> > > IOW, the hack would be self-contained in cpufeature.c.
> >
> > I think it makes sense that the hack should be self-contained in
> > cpufeature.c, leaving the definition of struct mvfr to 3 register_t also
> > on arm32 as it is today. Also leaving vcpreg.c as it is today so that a
> > guest can try to read mvfr2 without crashing thanks to
> > GENERATE_TID3_INFO(MVFR2, mvfr, 2).
> >
> > For the arm32 case in cpufeature.c:identify_cpu, the two permitted
> > values are 0 and 0b0100, which one did you have in mind? I take you
> > meant 0 which stands for "not implemented, or no support for
> > miscellaneous features"?
>
> It looks like I misread the spec. :/ I thought it would only be a single
> possible value.
>
> Hmmm... I am not sure about the value here as I don't know what would be the
> impact.
>
> It might be better to detect whether we are running on Armv8 HW or Armv7 HW. I
> haven't figured out an obvious way for that yet.
>
> On IRC, Andre suggested a couple of options on IRC
>
> 1) read ISAR5: This doesn't seem to work as it is UNK on Armv7
> 2) detect the VFP subarchitecture: It is not clear how this can be leveraged
>
> While skimming through the spec, I noticed that ID_DFR0.CopDbg requires a
> specific value for Armv8.x spec. This is the debug architecture, but maybe we
> can leverage it?
>
> Any better ideas?
One thing to note is that if Xen has difficulties in distinguishing
between armv7 and armv8/aarch32 a guest will have the same issue. Given
that mvfr2 is not available on armv7, how can a guest safely attempt to
read it?
If somehow the guest figures out that it is running on armv8/aarch32,
then it could read mvfr2, but we could return "not implemented" in that
case, right? In other words, part of me thinks that maybe we are
overcomplicating this, especially because we don't officially support
Xen on armv8/aarch32 as you wrote (we need to update SUPPORT.md).
So I would be tempted to only do the following:
- read mvfr2 on arm64
- never read mvfr2 on aarch32 (no matter armv7 or armv8)
- keep c->mvfr.bits[2] as zero on armv7 and armv8/aarch32 so that if the
guest attempts to read it, it won't crash but simply return zero
The most important things in regards to armv8/aarch32 are:
- be able to boot Xen armv8/aarch32 for our own arm32 testing of Xen
- be able to boot an armv7 and armv8/aarch32 guest on Xen armv8/aarch64
Both these things should work correctly with the simple approach above.
Your idea of trying to figure out if we are running on armv8/aarch32 via
ID_DFR0.CopDbg is clever but I think it is a bit overkill for a mode of
execution that we don't support. I don't think is worth the effort.
But if you think differently, I can invest some time in investigating
this further, following your line of thought on finding a register to
detect the mode of execution. (However I don't have any hardware to test
this anymore.) Otherwise, if you agree that the simpler approach is
sufficient, then I'll send a patch as described above.
On 06/01/2021 20:55, Stefano Stabellini wrote:
> On Wed, 6 Jan 2021, Julien Grall wrote:
>> On 05/01/2021 22:43, Stefano Stabellini wrote:
>>> On Tue, 5 Jan 2021, Julien Grall wrote:
>>>> Hi Stefano,
>>>>
>>>> On 05/01/2021 18:44, Stefano Stabellini wrote:
>>>>> On Tue, 5 Jan 2021, André Przywara wrote:
>>>>>> On 05/01/2021 16:06, Julien Grall wrote:
>>>>>>> (+ Ian and Andre)
>>>>>>>
>>>>>>> Hi Bertrand,
>>>>>>>
>>>>>>> On IRC, Ian pointed out that the smoke test was failing on
>>>>>>> Cubietruck.
>>>>>>> The only patches because the last success and the failure are your
>>>>>>> series.
>>>>>>>
>>>>>>> This seems to be a very early failure as there is no output from Xen
>>>>>>> [1].
>>>>>>>
>>>>>>> I originally thought the problem was because some of the ID
>>>>>>> registers
>>>>>>> (such as ID_PFR2) introduced in patch #2 doesn't exist in Armv7.
>>>>>>>
>>>>>>> But per B7.2.1 in ARM DDI 0406C.d, unallocated ID registers should
>>>>>>> be
>>>>>>> RAZ. So it would result to a crash. Andre confirmed that PFR2 can be
>>>>>>> accessed by writing a small baremetal code and booted on Cortex-A7
>>>>>>> and
>>>>>>> Cortex-A20.
>>>>>>>
>>>>>>> So I am not entirely sure what's the problem. Andre kindly accepted
>>>>>>> to
>>>>>>> try to boot Xen on his board. Hopefully, this will give us a clue on
>>>>>>> the
>>>>>>> problem.
>>>>>>>
>>>>>>> If not, I will borrow a Cubietruck in OssTest and see if I can
>>>>>>> reproduce
>>>>>>> it and debug it.
>>>>>>
>>>>>>
>>>>>> So I just compiled master and staging and ran just that on an
>>>>>> Allwinner
>>>>>> H3 (Cortex-A7 r0p5). Master boots fine (till it complains about the
>>>>>> missing Dom0, as expected). However staging indeed fails:
>>>>>>
>>>>>> (XEN) Xen version 4.15-unstable (andprz01@slackpad.lan)
>>>>>> (arm-slackware-linux-gnueabihf-gcc (GCC) 8.2.0) debug=y Tue Jan 5
>>>>>> 16:09:40 GMT 2021
>>>>>> (XEN) Latest ChangeSet: Sun Nov 8 15:59:42 2020 +0100 git:c992efd06a
>>>>>> (XEN) build-id: 85d361b8565b90d4e0defe2beb2419e191fd76b4
>>>>>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>>>>>> (XEN) ----[ Xen-4.15-unstable arm32 debug=y Not tainted ]----
>>>>>> (XEN) CPU: 0
>>>>>> (XEN) PC: 0026b8c8 identify_cpu+0xc0/0xd4
>>>>>> (XEN) CPSR: 600001da MODE:Hypervisor
>>>>>> (XEN) R0: 002acb20 R1: 00000000 R2: 00000000 R3: 11111111
>>>>>> (XEN) R4: 002acb1c R5: 002acb20 R6: 4e000000 R7: 00000000
>>>>>> (XEN) R8: 00000002 R9: 002d8200 R10:00008000 R11:002f7e6c
>>>>>> R12:00000080
>>>>>> (XEN) HYP: SP: 002f7e68 LR: 002c419c
>>>>>> (XEN)
>>>>>> (XEN) VTCR_EL2: 80002646
>>>>>> (XEN) VTTBR_EL2: 00000018e628bb80
>>>>>> (XEN)
>>>>>> (XEN) SCTLR_EL2: 30cd187f
>>>>>> (XEN) HCR_EL2: 00000038
>>>>>> (XEN) TTBR0_EL2: 000000004013a000
>>>>>> (XEN)
>>>>>> (XEN) ESR_EL2: 00000000
>>>>>> (XEN) HPFAR_EL2: 0003fff0
>>>>>> (XEN) HDFAR: 9d110000
>>>>>> (XEN) HIFAR: 0000a04a
>>>>>> (XEN)
>>>>>> (XEN) Xen stack trace from sp=002f7e68:
>>>>>> (XEN) 00000000 002f7f54 00008000 00000000 00002000 002a4584
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN) 00000000 00008000 49ff5000 002d81f0 40000000 00000000
>>>>>> 00002000
>>>>>> 00000001
>>>>>> (XEN) 00000000 50000000 49ffd000 00000000 50000000 00000000
>>>>>> 00000000
>>>>>> 50000000
>>>>>> (XEN) 4c000000 00000000 4e000000 00000000 ffffffff ffffffff
>>>>>> 50000000
>>>>>> 00000000
>>>>>> (XEN) 50000000 00000000 50000000 00000000 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN) 00000000 00000003 00000000 00000000 ffffffff 00000040
>>>>>> ffffffff
>>>>>> 00000000
>>>>>> (XEN) 00000000 00000000 00000000 002a7000 40008050 0000001a
>>>>>> 00000000
>>>>>> 49ff5000
>>>>>> (XEN) 40008000 3fe08000 00000004 0020006c 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
>>>>>> 00000000
>>>>>> 00000000
>>>>>> (XEN) 00000000 00000000 00000000 00000000 00000000 00000000
>>>>>> (XEN) Xen call trace:
>>>>>> (XEN) [<0026b8c8>] identify_cpu+0xc0/0xd4 (PC)
>>>>>> (XEN) [<002c419c>] start_xen+0x778/0xe50 (LR)
>>>>>> (XEN) [<002f7f54>] 002f7f54
>>>>>> (XEN)
>>>>>> (XEN)
>>>>>> (XEN) ****************************************
>>>>>> (XEN) Panic on CPU 0:
>>>>>> (XEN) CPU0: Unexpected Trap: Undefined Instruction
>>>>>> (XEN) ****************************************
>>>>>> (XEN)
>>>>>> (XEN) Reboot in five seconds...
>>>>>>
>>>>>>
>>>>>> The code in question:
>>>>>> 26b8c0: eef63a10 vmrs r3, mvfr1
>>>>>> 26b8c4: e5803058 str r3, [r0, #88] ; 0x58
>>>>>>> 26b8c8: eef53a10 vmrs r3, mvfr2
>>>>>> 26b8cc: e580305c str r3, [r0, #92] ; 0x5c
>>>>>> 26b8d0: e28bd000 add sp, fp, #0
>>>>>> 26b8d4: e49db004 pop {fp} ; (ldr fp, [sp],
>>>>>> #4)
>>>>>> 26b8d8: e12fff1e bx lr
>>>>>>
>>>>>> And indeed MVFR2 is not mentioned in the ARMv7 ARM, and in contrast to
>>>>>> the CP15 CPUID registers this is using the VMRS instruction, so it's
>>>>>> not
>>>>>> protected by future-proof CPUID register scheme.
>>>>>>
>>>>>> Not sure what to do about this, maybe #ifdef'ing this register access
>>>>>> with arm64?
>>>>>> I guess this comes from the slightly too optimistic code-sharing
>>>>>> between
>>>>>> arm32 and arm64?
>>>>>
>>>>> Yes and #ifdef'ing is what we have been doing with the other registers.
>>>>
>>>> There is a catch here. This register is accessible from AArch32 on all
>>>> Armv8
>>>> HW. It is just not accessible on Armv7.
>>>>
>>>> So hiding the MVFR2 behind #ifdef CONFIG_ARM_64 is technically not
>>>> correct.
>>>>
>>>> I know that we said that we don't officially support Xen on Arm32 on Armv8
>>>> HW
>>>> (I can't find where it is written though). So we could argue that
>>>> shadowing
>>>> MVFR2 is not worth it.
>>>>
>>>> I do use Armv8 HW to test 32-bit, so I would at least like to get Xen
>>>> booting.
>>>
>>> Yep, me too.
>>>
>>>
>>>> In addition to that, Linux 32-bit doesn't access MVFR2 at the moment.
>>>>
>>>> Therefore, a #ifdef may be acceptable for now. However, I would suggest to
>>>> introduce name it MAY_BE_UNDEFINED (or similar) that will be used to avoid
>>>> reading the system register on 32-bit.
>>>>
>>>> For the 32-bit case, I would just hardcode the value based on the Arm (it
>>>> looks like for Armv8-A there is only one valid value).
>>>>
>>>> IOW, the hack would be self-contained in cpufeature.c.
>>>
>>> I think it makes sense that the hack should be self-contained in
>>> cpufeature.c, leaving the definition of struct mvfr to 3 register_t also
>>> on arm32 as it is today. Also leaving vcpreg.c as it is today so that a
>>> guest can try to read mvfr2 without crashing thanks to
>>> GENERATE_TID3_INFO(MVFR2, mvfr, 2).
>>>
>>> For the arm32 case in cpufeature.c:identify_cpu, the two permitted
>>> values are 0 and 0b0100, which one did you have in mind? I take you
>>> meant 0 which stands for "not implemented, or no support for
>>> miscellaneous features"?
>>
>> It looks like I misread the spec. :/ I thought it would only be a single
>> possible value.
>>
>> Hmmm... I am not sure about the value here as I don't know what would be the
>> impact.
>>
>> It might be better to detect whether we are running on Armv8 HW or Armv7 HW. I
>> haven't figured out an obvious way for that yet.
>>
>> On IRC, Andre suggested a couple of options on IRC
>>
>> 1) read ISAR5: This doesn't seem to work as it is UNK on Armv7
>> 2) detect the VFP subarchitecture: It is not clear how this can be leveraged
>>
>> While skimming through the spec, I noticed that ID_DFR0.CopDbg requires a
>> specific value for Armv8.x spec. This is the debug architecture, but maybe we
>> can leverage it?
>>
>> Any better ideas?
>
> One thing to note is that if Xen has difficulties in distinguishing
> between armv7 and armv8/aarch32 a guest will have the same issue. Given
> that mvfr2 is not available on armv7, how can a guest safely attempt to
> read it?
I am not sure I would call it difficulties yet. I would say it is more
that I haven't looked too much in the spec to understand if there is a
trivial way to distinguish between v7 and v8.
Maybe André or Bertrand can help here?
>
> If somehow the guest figures out that it is running on armv8/aarch32,
> then it could read mvfr2, but we could return "not implemented" in that
> case, right?
Nothing in the Armv8 say the register is optional. So we technically
cannot return "not implemented".
However, we could consider to return "not implemented" (which would
likely result to a crash) if we are not confident with the fixed value
returned.
> In other words, part of me thinks that maybe we are
> overcomplicating this, especially because we don't officially support
> Xen on armv8/aarch32 as you wrote (we need to update SUPPORT.md).
Right, at the same time we don't want to introduce problem hard to
diagnostic.
This is quite similar to the discussion about IS{ACT,PEND}R GIC register
we had in the past. If we return dummy value, then the guest may
continue to run until a point but we don't know for how long. An error
may come up a really long time after and it would be difficult to find
the exact root cause.
So we need to be careful in our choice.
> So I would be tempted to only do the following:
>
> - read mvfr2 on arm64
> - never read mvfr2 on aarch32 (no matter armv7 or armv8)
> - keep c->mvfr.bits[2] as zero on armv7 and armv8/aarch32 so that if the
> guest attempts to read it, it won't crash but simply return zero
I agree that we should return 0, but I disagree with the justification
(see more below).
Reading the Armv8 spec again, 0 means there is no support for
miscellaneous features. So the guest should behave correctly if we
return 0. Therefore returning 0 for AArch32 should be fine.
>
> The most important things in regards to armv8/aarch32 are:
>
> - be able to boot Xen armv8/aarch32 for our own arm32 testing of Xen
> - be able to boot an armv7 and armv8/aarch32 guest on Xen armv8/aarch64
The most important things here is to not shoot ourself in the foot by
returning value we don't understand :). It is best to crash the guest
rather than continuing until it become nearly impossible to track the
root cause of an issue.
>
> Both these things should work correctly with the simple approach above.
>
>
> Your idea of trying to figure out if we are running on armv8/aarch32 via
> ID_DFR0.CopDbg is clever but I think it is a bit overkill for a mode of
> execution that we don't support. I don't think is worth the effort.
I made this suggestion because I originally thought it would not be
possible to find a sensible value. I believe we found one, so this
suggestion can be discarded.
Cheers,
--
Julien Grall
It seems there is still something wrong with ARM in staging. Whatever
change was made passed the smoke test but now the main flights are
failing:
osstest service owner writes:
> flight 158303 xen-unstable real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/158303/
>
> Regressions :-(
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-arm64-arm64-xl-thunderx 8 xen-boot fail REGR. vs. 158290
> test-arm64-arm64-examine 8 reboot fail REGR. vs. 158290
> test-arm64-arm64-xl-xsm 8 xen-boot fail REGR. vs. 158290
> test-arm64-arm64-xl-credit1 8 xen-boot fail REGR. vs. 158290
The bisector has fingered the same commit in unstable as it did in the
smoke tests. That might be because the fix made to staging, to get
the smoke tests to pass, was not complete enough. It also might be
because something different broke the other tests and the tree was
briefly working in between.
fx: puts on Release Manager hat. Can one of you ARM folks please take
a look at this and fix it ASAP ?
Thanks,
Ian.
osstest service owner writes ("[xen-unstable bisection] complete test-arm64-arm64-xl-xsm"):
> branch xen-unstable
> xenbranch xen-unstable
> job test-arm64-arm64-xl-xsm
> testid xen-boot
>
> Tree: linux git://xenbits.xen.org/linux-pvops.git
> Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
> Tree: qemuu git://xenbits.xen.org/qemu-xen.git
> Tree: xen git://xenbits.xen.org/xen.git
>
> *** Found and reproduced problem changeset ***
>
> Bug is in tree: xen git://xenbits.xen.org/xen.git
> Bug introduced: 9cfdb489af810f71acb7dcdb87075dc7b3b313a0
> Bug not present: a9f1f03b2710f5ce84f69c1c4516349531053fac
> Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/158347/
>
>
> commit 9cfdb489af810f71acb7dcdb87075dc7b3b313a0
> Author: Bertrand Marquis <bertrand.marquis@arm.com>
> Date: Thu Dec 17 15:38:02 2020 +0000
>
> xen/arm: Add ID registers and complete cpuinfo
>
> Add definition and entries in cpuinfo for ID registers introduced in
> newer Arm Architecture reference manual:
> - ID_PFR2: processor feature register 2
> - ID_DFR1: debug feature register 1
> - ID_MMFR4 and ID_MMFR5: Memory model feature registers 4 and 5
> - ID_ISA6: ISA Feature register 6
> Add more bitfield definitions in PFR fields of cpuinfo.
> Add MVFR2 register definition for aarch32.
> Add MVFRx_EL1 defines for aarch32.
> Add mvfr values in cpuinfo.
> Add some registers definition for arm64 in sysregs as some are not
> always know by compilers.
> Initialize the new values added in cpuinfo in identify_cpu during init.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Hi Ian, On 11/01/2021 11:19, Ian Jackson wrote: > It seems there is still something wrong with ARM in staging. Whatever > change was made passed the smoke test but now the main flights are > failing: > > osstest service owner writes: >> flight 158303 xen-unstable real [real] >> http://logs.test-lab.xenproject.org/osstest/logs/158303/ >> >> Regressions :-( >> >> Tests which did not succeed and are blocking, >> including tests which could not be run: >> test-arm64-arm64-xl-thunderx 8 xen-boot fail REGR. vs. 158290 >> test-arm64-arm64-examine 8 reboot fail REGR. vs. 158290 >> test-arm64-arm64-xl-xsm 8 xen-boot fail REGR. vs. 158290 >> test-arm64-arm64-xl-credit1 8 xen-boot fail REGR. vs. 158290 > > The bisector has fingered the same commit in unstable as it did in the > smoke tests. That might be because the fix made to staging, to get > the smoke tests to pass, was not complete enough. It also might be > because something different broke the other tests and the tree was > briefly working in between. The smoke test ran on laxton0 (AMD Seattle) while the unstable test ran on rochester0 (Cavium Thunder-X). The issue seems to be processor specific. > > fx: puts on Release Manager hat. Can one of you ARM folks please take > a look at this and fix it ASAP ? Jan sent an e-mail this morning about this. See [1]. Cheers, [1] <dce8bed2-db66-b032-06a9-86c8f80d26aa@xen.org> -- Julien Grall
MVFR2 is not available on arm32. Don't try to read it. Make MVFR2 arm64
only.
Fixes: 9cfdb489af81 ("xen/arm: Add ID registers and complete cpuinfo")
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
xen/arch/arm/cpufeature.c | 2 ++
xen/include/asm-arm/cpufeature.h | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 1f6a85aafe..9e3377eae3 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -150,7 +150,9 @@ void identify_cpu(struct cpuinfo_arm *c)
c->mvfr.bits[0] = READ_SYSREG(MVFR0_EL1);
c->mvfr.bits[1] = READ_SYSREG(MVFR1_EL1);
+#ifdef CONFIG_ARM_64
c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
+#endif
}
/*
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 6058744c18..fa20cb493a 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -272,7 +272,7 @@ struct cpuinfo_arm {
} isa32;
struct {
- register_t bits[3];
+ register_t bits[2 + IS_ENABLED(CONFIG_ARM_64)];
} mvfr;
};
--
2.17.1
On 05/01/2021 19:05, Stefano Stabellini wrote:
> MVFR2 is not available on arm32. Don't try to read it. Make MVFR2 arm64
> only.
Not really, MVFR2 is allocated when running in AArch32 mode on Armv8. It
just doesn't exist on Armv7. See my answer your previous e-mail for more
details.
>
> Fixes: 9cfdb489af81 ("xen/arm: Add ID registers and complete cpuinfo")
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> ---
> xen/arch/arm/cpufeature.c | 2 ++
> xen/include/asm-arm/cpufeature.h | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 1f6a85aafe..9e3377eae3 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -150,7 +150,9 @@ void identify_cpu(struct cpuinfo_arm *c)
>
> c->mvfr.bits[0] = READ_SYSREG(MVFR0_EL1);
> c->mvfr.bits[1] = READ_SYSREG(MVFR1_EL1);
> +#ifdef CONFIG_ARM_64
> c->mvfr.bits[2] = READ_SYSREG(MVFR2_EL1);
> +#endif
> }
>
> /*
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index 6058744c18..fa20cb493a 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -272,7 +272,7 @@ struct cpuinfo_arm {
> } isa32;
>
> struct {
> - register_t bits[3];
> + register_t bits[2 + IS_ENABLED(CONFIG_ARM_64)];
mvfr.bits[2] will be accessed from arch/arm/vcpreg.c, so you will
provide garbagge to guest if the user happen to run Xen on Arm32 on Armv8.
Given that MVFR2 exists, I think it would be best to keep the definition
in cpuinfo_arm around and only hardcode the value in cpufeature.c.
Please see my previous e-mail for more rationale on this approach.
Cheers,
--
Julien Grall
© 2016 - 2024 Red Hat, Inc.