Re: [Xen-devel] [PATCH] x86/mm: Reduce debug overhead of __virt_to_maddr()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/08/17 15:11, Jan Beulich wrote:
>>>> On 16.08.17 at 15:58, <andrew.cooper3@xxxxxxxxxx> wrote:
>> --- a/xen/include/asm-x86/x86_64/page.h
>> +++ b/xen/include/asm-x86/x86_64/page.h
>> @@ -51,13 +51,15 @@ extern unsigned long xen_virt_end;
>>  
>>  static inline unsigned long __virt_to_maddr(unsigned long va)
>>  {
>> -    ASSERT(va >= XEN_VIRT_START);
>>      ASSERT(va < DIRECTMAP_VIRT_END);
>>      if ( va >= DIRECTMAP_VIRT_START )
>>          va -= DIRECTMAP_VIRT_START;
>>      else
>>      {
>> -        ASSERT(va < XEN_VIRT_END);
>> +        BUILD_BUG_ON(XEN_VIRT_END - XEN_VIRT_START != GB(1));
>> +        ASSERT(((long)va >> (PAGE_ORDER_1G + PAGE_SHIFT)) ==
>> +               ((long)XEN_VIRT_START >> (PAGE_ORDER_1G + PAGE_SHIFT)));
> Do you really need the casts here? I.e. what's wrong here with
> doing unsigned long arithmetic?

Oh - good point.  This took more than one attempt to get right, and I
first thought I had a sign extension problem.  The actual problem was a
(lack of) + PAGE_SHIFT.

The other thing to know is that  __virt_to_maddr() is used before the
IDT is set up, so your only signal of something being wrong is a triple
fault.  Let me double check without the casts, but I think it should be
fine.

~Andrew

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