Re: [PATCH RESEND v2 5/6] target/arm/cpu: Enable 'el2' to work with host/max cpu

From: Peter Maydell <peter.maydell@linaro.org>
To: Andrew Jones <drjones@redhat.com>
Cc: "Richard Henderson" <richard.henderson@linaro.org>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Andrea Bolognani" <abologna@redhat.com>,
	qemu-arm <qemu-arm@nongnu.org>, "Haibo Xu" <haibo.xu@linaro.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [PATCH RESEND v2 5/6] target/arm/cpu: Enable 'el2' to work with host/max cpu
Date: Tue, 27 Apr 2021 13:04:54 +0100	[thread overview]
Message-ID: <CAFEAcA8kH0RsS993kwbsH9+ziCbxT5151LFGnnDkcy10EODEaQ@mail.gmail.com> (raw)
In-Reply-To: <20210427094924.pgivurjrgtzovcwa@gator.home>

On Tue, 27 Apr 2021 at 10:49, Andrew Jones <drjones@redhat.com> wrote:
>
> On Tue, Apr 27, 2021 at 10:38:00AM +0100, Peter Maydell wrote:
> > For new features added we want to use ID register bits. However,
> > a lot of older pre-existing features are keyed off ARM_FEATURE_FOO
> > flag bits. Trying to convert an ARM_FEATURE flag to use ID registers
> > isn't necessarily 100% trivial (for instance, the ARM_FEATURE flag
> > is always checkable regardless of AArch64 vs AArch32, but the ID
> > register checks often need to be split up into separate ones checking
> > the AArch32 or the AArch64 ID register value). So we aren't really
> > doing conversion of existing flags. (I did a few which were easy
> > because there were only a handful of checks of them.) As a side note,
> > some features really can't be checked in ID registers, like
> > ARM_FEATURE_V8_1M, so env->features is not going to go away completely.
> >
> > The only use of cpu->has_foo should be for the QOM property -- the
> > arm_cpu_realizefn() should look at it and then either clear the
> > ARM_FEATURE flag or update the ID register bits depending on
> > which one the feature is using.
> >
> > vms->virt is for the board code (and controls more things than
> > just whether the CPU itself has EL2).
> >
>
> Thanks for the summary, Peter. For this series, do you recommend
> attempting to convert from ARM_FEATURE_EL2 to feature bits first? Or keep
> the ARM_FEATURE flag?

I think we should stick with the ARM_FEATURE flag -- there are
enough uses of it that a conversion would be complicated and
I don't think we should tie this feature work up with that.

> Also, while I agree we can't use vms->virt for the
> same purposes as the CPU feature (however that's tracked), do you agree
> vms->virt should be true when the CPU feature is enabled?

It's the other way around -- setting vms->virt should cause the
board code to set the CPU feature. (Conceptually, the board
owns the CPU so it gets to decide what its configuration should
be. The CPU doesn't get to reach out and mess with the board config.)

thanks
-- PMM