Re: [Xen-devel] [PATCH v2 5/9] x86/pagewalk: Helpers for reserved bit handling

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/03/2017 07:47, Jan Beulich wrote:
>>>> On 23.03.17 at 18:35, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 23/03/17 17:12, Tim Deegan wrote:
>>> At 17:02 +0000 on 23 Mar (1490288548), Andrew Cooper wrote:
>>>> On 23/03/17 16:55, Tim Deegan wrote:
>>>>> At 16:31 +0000 on 16 Mar (1489681899), Andrew Cooper wrote:
>>>>>> Some bits are unconditionally reserved in pagetable entries, or reserved
>>>>>> because of alignment restrictions.  Other bits are reserved because of 
>> control
>>>>>> register configuration.
>>>>>>
>>>>>> Introduce helpers which take an individual vcpu and guest pagetable 
>>>>>> entry, 
>> and
>>>>>> calculates whether any reserved bits are set.
>>>>>>
>>>>>> While here, add a couple of newlines to aid readability.
>>>>>>
>>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>>> Reviewed-by: Tim Deegan <tim@xxxxxxx>, although:
>>>>>
>>>>>> +/* Mask covering the reserved bits from superpage alignment. */
>>>>>> +#define SUPERPAGE_RSVD(bit)                                             
>>>>>> \
>>>>>> +    (((1ULL << (bit)) - 1) & ~(_PAGE_PSE_PAT | (_PAGE_PSE_PAT - 1)))
>>>>> I think this will be wrong if we ever get l4 superpages, as the mask
>>>>> is only 32 bits wide.
>>>> What is 32 bits wide?  1ULL should cause everything else to be suitably
>>>> promoted, no?
>>> ~(_PAGE_PSE_PAT | (_PAGE_PSE_PAT - 1)) == 0xffffe000.  Promotion comes
>>> too late.
>> Oh - good point.  Best fix this right now.
>>
>> Would you prefer ~((uint64_t)_PAGE_PSE_PAT | (_PAGE_PSE_PAT - 1)) or
>> ~(_PAGE_PSE_PAT | (_PAGE_PSE_PAT - 1) | 0ULL)
> Why not simply tag the suffix on a constant already there, i.e.
>
> ~(_PAGE_PSE_PAT | (_PAGE_PSE_PAT - 1ULL))

Oh - good point.  I will do that.

> (And btw, we commonly use only UL these days.)

Sorry - force of habit.  (XTF, being 32bit as well, really does need ULL
over UL)

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel