Re: [Xen-devel] [PATCH] EPT: utilize GLA->GPA translation known for certain faults

[Tim Deegan <tim@xxxxxxx>]

At 15:40 +0100 on 28 Aug (1409236857), Jan Beulich wrote:
> Rather than doing the translation ourselves in __hvmemul_{read,write}()
> leverage that we know the association for faults other than such having
> occurred when translating addresses of page tables.
> 
> There is one intentional but not necessarily obvious (and possibly
> subtle) adjustment to behavior: __hvmemul_read() no longer blindly
> bails on instruction fetches matching the MMIO GVA (the callers of
> handle_mmio_with_translation() now control the behavior via the struct
> npfec they pass, and it didn't seem right to bail here rather than just
> falling through to the unaccelerated path)

Understood.  I think the test you have is not quite right:

> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -481,10 +481,11 @@ static int __hvmemul_read(
>          while ( off & (chunk - 1) )
>              chunk >>= 1;
>  
> -    if ( unlikely(vio->mmio_gva == (addr & PAGE_MASK)) && vio->mmio_gva )
> +    if ( ((access_type != hvm_access_insn_fetch
> +           ? vio->mmio_access.read_access
> +           : vio->mmio_access.insn_fetch)) &&
> +         (vio->mmio_gva == (addr & PAGE_MASK)) )

This seems like it will follow the fast path for some instruction
fetches, which might be OK, but doesn't quite match your description.

If that's the behaviour you want then I think we can simplify the test
a lot because we don't care whether this is an ifetch or not; only
whether the mapping is valid (assuming we model a coherent
icache/dcache and itlb/dtlb here).

But IIRC we don't really support fetching instructions from MMIO
addresses anyway (raising questions about, e.g. operation sizes,
idempotence and readahead) so we probably want:

       if ( access_type != hvm_access_insn_fetch &&
            vio->mmio_access.read_access &&
            vio->mmio_gva == (addr & PAGE_MASK) )

For the rest, Reviewed-by: Tim Deegan <tim@xxxxxxx>

Cheers,

Tim.

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