RE: [PATCH 2/2] powerpc: Make context bits depend on virtual addr size.

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:

> David Laight <David.Laight@aculab.com> writes:
>
>>> +#define CONTEXT_BITS		19
>>> +#define USER_ESID_BITS		18
>>> +#define USER_ESID_BITS_1T	6
>>> +
>>> +/*
>>> + * 256MB segment
>>> + * The proto-VSID space has 2^(CONTEX_BITS + USER_ESID_BITS) - 1 segments
>>> + * available for user + kernel mapping. The top 4 contexts are used for
>>> + * kernel mapping. Each segment contains 2^28 bytes. Each
>>> + * context maps 2^46 bytes (64TB) so we can support 2^19-1 contexts
>>> + * (19 == 37 + 28 - 46).
>>> + */
>>
>> I can't help feeling this would be easier to understand if a full
>> 64? 80? bit address was shown with the various bit ranges identified.
>>
>> Given the comment, I'd have expected CONTEXT_BITS to be calculated
>> from three other named constants - rather than being set to 19.
>>
>
> May be the comments were misleading. We build proto vsid using a
> combination of context and ea bits.
>
> Current code does the below:
>
> for kernel:
> proto_vsid = ea >> SID_SHIFT;
> proto_vsid |= (1UL << (CONTEXT_BITS + USER_ESID_BITS));
> for user:
> proto_vsid = ea >> SID_SHIFT
> proto_vsid |= context << USER_ESID_BITS 
>
> context range is 0 - (2^19 -1)
>
> With this patch we _don't_ give kernel half the proto vsid range.
> Instead, we reduce the proto vsid range and then the kernel is given
> top 4 context. ie, kernel proto vsid is now
>
> for kenel:
> proto_vsid = ea >> SID_SHIFT;
> context = (MAX_CONTEXT - 4) +  ((effective address >> 60) - 0xc);
> proto_vsid |= context << USER_ESID_BITS 
>

Hmm that may be an issue, considering ESID for kernel is 36 bits.
We have overlapping bits between shifted value of context and kernel
ESID.

-aneesh