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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.08.17 at 16:14, <andrew.cooper3@xxxxxxxxxx> wrote:
> 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.

If it is, then with the casts dropped
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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