mbox series

[RFC,0/1] hw/arm: use -cpu max by default on sbsa-ref

Message ID 20221109133525.762667-1-quic_llindhol@quicinc.com (mailing list archive)
Headers show
Series hw/arm: use -cpu max by default on sbsa-ref | expand

Message

Leif Lindholm Nov. 9, 2022, 1:35 p.m. UTC
From: Leif Lindholm <Leif Lindholm quic_llindhol@quicinc.com>

We have mainly (well, as will become clear, in fact "exclusively") been using
sbsa-ref with the "max" CPU. But sbsa-ref was created with a default CPU of
Cortex-A57, which we have not updated along the way.

However, the "max" cpu has seen a bug where Linux boot fails around UEFI
ExitBootServices. Marcin Juszkiewicz has found the cause for that, but that
requires a patch to TF-A. (Has that been submitted upstream?)

Turns out that due to a change in upstream TF-A last year, all supported cpus
other than "max" fail to even boot UEFI fully, due to the top-level (TF-A)
Makefile defaulting to enabling the maximum ARM architectural version
(currently 8.6), in combination with not verifying all features at runtime
using the ID registers.

Since the *point* of sbsa-ref is to serve as a continuously evolving platform
tracking (with some obvious lag) the evolution of the ARM architecture and the
SystemReady specifications, I don't really want to restrict the enabled
feature set in TF-A to the Cortex-A57 one.

My preferred course of action would be to change the default cpu to max -
maybe even dropping support for other cpus. I would then step the version
field that was added to the DT. *But* this would break existing boots with
old TF-A that can currently boot Linux.

I did contemplate weaving this into the platform versioning, but truth is
I'm not convinced that would help ... and it would delay getting the
reworked memory map out.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
Cc: Shashi Mallela <shashi.mallela@linaro.org>
Cc: Radoslaw Biernacki <rad@semihalf.com>

Leif Lindholm (1):
  hw/arm: use -cpu max by default for sbsa-ref

 hw/arm/sbsa-ref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Peter Maydell Nov. 9, 2022, 2:02 p.m. UTC | #1
On Wed, 9 Nov 2022 at 13:35, Leif Lindholm <quic_llindhol@quicinc.com> wrote:
>
> From: Leif Lindholm <Leif Lindholm quic_llindhol@quicinc.com>
>
> We have mainly (well, as will become clear, in fact "exclusively") been using
> sbsa-ref with the "max" CPU. But sbsa-ref was created with a default CPU of
> Cortex-A57, which we have not updated along the way.

So, I don't have a strong opinion on what sbsa-ref's CPU type
should be: if 'max' works better for the use case we should
change to it.

> However, the "max" cpu has seen a bug where Linux boot fails around UEFI
> ExitBootServices. Marcin Juszkiewicz has found the cause for that, but that
> requires a patch to TF-A. (Has that been submitted upstream?)
>
> Turns out that due to a change in upstream TF-A last year, all supported cpus
> other than "max" fail to even boot UEFI fully, due to the top-level (TF-A)
> Makefile defaulting to enabling the maximum ARM architectural version
> (currently 8.6), in combination with not verifying all features at runtime
> using the ID registers.

This seems to me straightforwardly to be a problem that should be
fixed in TF-A. "Default to the newest possible architecture version
and don't check ID registers" is a recipe for "can't boot on
anything". Many *hardware* CPUs aren't that new yet!

Part of the purpose of sbsa-ref is to find problems with the
software stack -- so we should expect that sometimes the answer
is "fix the software stack", not "change the model's behaviour".

> Since the *point* of sbsa-ref is to serve as a continuously evolving platform
> tracking (with some obvious lag) the evolution of the ARM architecture and the
> SystemReady specifications, I don't really want to restrict the enabled
> feature set in TF-A to the Cortex-A57 one.
>
> My preferred course of action would be to change the default cpu to max -
> maybe even dropping support for other cpus. I would then step the version
> field that was added to the DT. *But* this would break existing boots with
> old TF-A that can currently boot Linux.

An intermediate option would be to move forward to for instance
the neoverse-n1 CPU type.

If you want to use 'max' for sbsa-ref then you probably also want to
look at making sure that TF-A is doing the same thing Linux does
where it checks ID registers for feature presence before enabling
feature use, because 'max' is a moving target and relies on the
guest code doing the right thing with feature checks. In particular
it does not honour the nominal architectural requirements about
v8.x/v9.x levels, but may implement features from further in the
"future" than is strictly permitted given the absence of some
other older features.

thanks
-- PMM
Marcin Juszkiewicz Nov. 13, 2022, 2:27 p.m. UTC | #2
W dniu 9.11.2022 o 14:35, Leif Lindholm pisze:

> We have mainly (well, as will become clear, in fact "exclusively") been using
> sbsa-ref with the "max" CPU. But sbsa-ref was created with a default CPU of
> Cortex-A57, which we have not updated along the way.
> 
> However, the "max" cpu has seen a bug where Linux boot fails around UEFI
> ExitBootServices. Marcin Juszkiewicz has found the cause for that, but that
> requires a patch to TF-A. (Has that been submitted upstream?)

TF-A gerrit instance refuses to handle any of my SSH keys. RSA, ED25519 
ones.

The only change I did to TF-A was removal of coherent memory stuff as it 
was for a57/72 and blocked me from building it for a76/n1.

Attached it.