[Xen-devel] Re: [PATCH] xen/x86: replace order-based range checking of M2P table by linear one

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> On 16.08.11 at 19:45, Jeremy Fitzhardinge <jeremy@xxxxxxxx> wrote:
> On 08/16/2011 07:07 AM, Jan Beulich wrote:
>> +#ifdef CONFIG_X86_32
>> +    if (machine_to_phys_mapping + machine_to_phys_nr
>> +        < machine_to_phys_mapping)
> 
> I'd prefer extra parens around the + just to make it very clear.  Is
> this kind of overflow check fully defined, or could the compiler find
> some way of screwing it up?

This check is fully defined afaik, ...

>> +            machine_to_phys_nr = (unsigned long *)NULL
>> +                                 - machine_to_phys_mapping;
> 
> Is the machine_to_phys_mapping array guaranteed to end at the end of the
> address space?  And I think a literal '0' there would make it a bit
> clearer what's going on, rather than invoking all the stuff that NULL
> implies.

... and this operation is as long as machine_to_phys_mapping is
aligned to a long boundary (which we know it is).

machine_to_phys_mapping not necessarily ends at the end of address
space (it never will on a 32-bit hypervisor, and the 64-bit one puts a
2Mb guard page at the end), but the purpose here is not to determine
a precise machine_to_phys_nr, but just to avoid overflow, since the
number reported by the hypervisor isn't necessarily precise.

But wait, only on x86-64 it's possibly much larger than the actual
number of entries, while what a 32-bit kernel gets to see (no matter
whether running on a 32- or 64-bit hypervisor) should really never
itself result in an overflow (only the previous order-based calculation
could), so it should even be okay to make this a BUG_ON(). I may
give this a try, and come up with an incremental patch (but I don't
see a pressing need to re-spin this one).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel