Re: [PATCH 2/5] arm64: cpufeature: Track unsigned fields

Catalin Marinas
2015-11-19T18:45:41Z

On Thu, Nov 19, 2015 at 10:03:13AM +0000, Suzuki K. Poulose wrote:
> On 19/11/15 04:57, AKASHI Takahiro wrote:
> > From my curiosity,
> >can you please clarify your criteria regarding which fields of a register should be signed or unsigned?
> >I guessed that the fields marked with FTR_LOWER_SAFE or FTR_HIGHER_SAFE could be unsigned,
> >but it seems to be not always true looking at your patch.
> >Anyhow, for example,
>
> ...
>
> >>- ARM64_FTR_BITS(FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_PARANGE_SHIFT, 4, 0),
> >>+ U_ARM64_FTR_BITS(FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64MMFR0_PARANGE_SHIFT, 4, 0),
> >> ARM64_FTR_END,
> >> };
> >
> >BigEnd, bits[11:8], is 0b0000 for "No mixed-endian support", and 0b0001 for
> >"Mixed-endian support". But can any other value be possible? If not, why signed?
> >If there are some hidden (or undocumented) specifications, as Ard mentioned, that's fine.
> >Please ignore my comments.
>
> There are no hidden specifications, but just that they are
> undocumented. To be precise, the criteria I selected was based on the
> meaning of their values.
>
> 1) If value represents something which cannot be negative and hence
> should be treated as unsigned. e.g, number of break points
> ID_AA64DFR0:BRPs.
>
> 2) If the individual values are mapped to some other values which
> cannot be negative, but have LOWER_SAFE/HIGHER_SAFE relation. e.g,
> CTR_EL0:WRG - Log2 of the write granule size. ID_AA64MMFR0:PARANGE
> - Supported PA Size

I asked for a clarification to be added to the ARM ARM but, in the
meantime, we have three types of fields:

a) A precise value (number of breakpoint registers) or a value from
which you derive some precise value. You mentioned these above

b) Fields defining the presence of a feature (1, 2, 3). These would
always be positive since the absence of such feature would mean a
value of 0

c) Fields defining the absence of a feature by setting 0xf. These are
usually fields that were initial RAZ and turned to -1. I don't expect
such field be greater than 0, nor smaller than -1.

So I think we can treat (a) and (b) as unsigned permanently. We could
treat (c) as unsigned as well with a value of 0xf though I'm not sure
how it matches your LOWER/HIGHER_SAFE definitions.

If we go for all unsigned, we no longer need the sign extension of a
4-bit value.

--
Catalin