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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.09.17 at 12:21, <Paul.Durrant@xxxxxxxxxx> 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.

No, the layer is the right one (there's another #PF being raised
a few lines up). Already the direct caller of the function wouldn't
know what CR2 value to specify for the #PF.

Jan


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