Re: [Xen-devel] [PATCH] x86/HVM: drop bogus #PF raising from linear->phys translation

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/09/17 11:21, Paul Durrant wrote:
>> -----Original Message-----
>> From: Andrew Cooper
>> Sent: 07 September 2017 11:18
>> To: Jan Beulich <JBeulich@xxxxxxxx>; xen-devel <xen-
>> devel@xxxxxxxxxxxxxxxxxxxx>
>> Cc: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
>> Subject: Re: [PATCH] x86/HVM: drop bogus #PF raising from linear->phys
>> translation
>>
>> On 07/09/17 09:14, Jan Beulich wrote:
>>> Translations spanning a page boundary not resulting in physically
>>> contiguous addresses is not a reason to raise #PF. In fact by not doing
>>> so accesses of this kind are being emulated correctly thanks to the
>>> fallback logic in the insn emulator's REP MOVS/STOS/INS/OUTS handling
>>> (non-string accesses to such locations are being split elsewhere and
>>> hence have been working fine already).
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> --- a/xen/arch/x86/hvm/emulate.c
>>> +++ b/xen/arch/x86/hvm/emulate.c
>>> @@ -566,15 +566,12 @@ static int hvmemul_linear_to_phys(
>>>              if ( pfec & (PFEC_page_paged | PFEC_page_shared) )
>>>                  return X86EMUL_RETRY;
>>>              done /= bytes_per_rep;
>>> -            *reps = done;
>>>              if ( done == 0 )
>>>              {
>>>                  ASSERT(!reverse);
>>> -                if ( npfn != gfn_x(INVALID_GFN) )
>>> -                    return X86EMUL_UNHANDLEABLE;
>>> -                x86_emul_pagefault(pfec, addr & PAGE_MASK, &hvmemul_ctxt-
>>> ctxt);
>>> -                return X86EMUL_EXCEPTION;
>>> +                return X86EMUL_UNHANDLEABLE;
>> I dont follow your reasoning.  The pagefault path is only reachable when
>> npfn is INVALID_GFN, which means a pagewalk did fail.
> But even so, does it not make more sense that a call into 
> hvmemul_linear_to_phys() simply fails and the caller decides whether an 
> exception is warranted? It seems wrong for what is essentially a utility 
> function to be deciding what to do.

The function is unfortunately complicated by its handling of reps, but
it is the architecturally appropriate place to raise pagefaults. 
linear_to_phys is "do a pagewalk and find where it goes", and callers
don't have sufficient context to recreate a correct #PF.

~Andrew

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