Re: [PATCH 02/12] arm64: Add PAR_EL1 field description

[Marc Zyngier <maz@kernel.org>]

On Fri, 12 Jul 2024 08:06:31 +0100,
Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> 
> 
> 
> On 6/25/24 19:05, Marc Zyngier wrote:
> > As KVM is about to grow a full emulation for the AT instructions,
> > add the layout of the PAR_EL1 register in its non-D128 configuration.
> 
> Right, there are two variants for PAR_EL1 i.e D128 and non-D128. Probably it makes
> sense to define all these PAR_EL1 fields in arch/arm64/include/asm/sysreg.h, until
> arch/arm64/tools/sysreg evolves to accommodate different bit field layouts for the
> same register.

This is really sorely needed, because we can't describe any of the
registers that changes layout depending on another control bit. Take
for example any of the EL2 registers affected by HCR_EL2.E2H.

However, I have no interest in defining *any* D128 format. I take it
that whoever will eventually add D128 support to the kernel (and KVM)
will take care of that.

> 
> > 
> > Note that the constants are a bit ugly, as the register has two
> > layouts, based on the state of the F bit.
> 
> Just wondering if it would be better to append 'VALID/INVALID' suffix
> for the fields to differentiate between when F = 0 and when F = 1 ?
> 
> s/SYS_PAR_EL1_FST/SYS_PAR_INVALID_FST_EL1
> s/SYS_PAR_EL1_SH/SYS_PAR_VALID_SH_EL1
> 
> Or something similar.

I find it pretty horrible.

If anything, because "VALID/INVALID" doesn't say anything of *what* is
invalid. Also, there is no "VALID" definition in the register, and an
aborted translation does not make the register invalid, quite the
opposite -- it is full of crucial information.

Which is why I used the F0/F1 prefixes, making it clear (at least in
my view) that the description is tied to a particular value of the
PAR_EL1.F bit.

Finally, most of the bit layouts are unambiguous: a field of any given
name only exists in a given layout of the register. This means we can
safely have names that match the ARM ARM description without any
visual pollution.

The only ambiguities are with generic names such as RES0 and IMPDEF.
Given that we almost never use these bits for anything, I don't think
the use of a F-specific prefix is a problem.

But yeah, naming is hard.

	M.

-- 
Without deviation from the norm, progress is not possible.