Re: [Xen-devel] [PATCH v2 06/19] x86/pv: Implement pv_inject_{event, page_fault, hw_exception}()

[Tim Deegan <tim@xxxxxxx>]

At 11:13 +0000 on 28 Nov (1480331603), Andrew Cooper wrote:
> To help with event injection improvements for the PV uses of x86_emulate(),
> implement a event injection API which matches its hvm counterpart.
> 
> This is started with taking do_guest_trap() and modifying its calling API to
> pv_inject_event(), subsequentally implementing the former in terms of the
> latter.
> 
> The existing propagate_page_fault() is fairly similar to
> pv_inject_page_fault(), although it has a return value.  Only a single caller
> makes use of the return value, and non-NULL is only returned if the passed cr2
> is non-canonical.  Opencode this single case in
> handle_gdt_ldt_mapping_fault(), allowing propagate_page_fault() to become
> void.
> 
> The #PF specific bits are moved into pv_inject_event(), and
> pv_inject_page_fault() is implemented as a static inline wrapper.
> reserved_bit_page_fault() is pure code motion.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Acked-by: Tim Deegan <tim@xxxxxxx>

with one note:

> +    if ( vector == TRAP_page_fault )
> +    {
> +        v->arch.pv_vcpu.ctrlreg[2] = event->cr2;
> +        arch_set_cr2(v, event->cr2);
> +
> +        /* Re-set error_code.user flag appropriately for the guest. */
> +        error_code &= ~PFEC_user_mode;
> +        if ( !guest_kernel_mode(v, regs) )
> +            error_code |= PFEC_user_mode;

I can see that you're just moving this code, but isn't it wrong for
what are now called "implicit" accesses?  My Ack stands on this patch
regardless.

Cheers,

Tim.



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