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

[Catalin Marinas]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/