Re: [Xen-devel] [PATCH v4 4/5] x86/PV: use generic emulator for privileged instruction handling

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/12/16 11:28, Jan Beulich wrote:
> +static int priv_op_insn_fetch(enum x86_segment seg,
> +                              unsigned long offset,
> +                              void *p_data,
> +                              unsigned int bytes,
> +                              struct x86_emulate_ctxt *ctxt)
> +{
> +    const struct priv_op_ctxt *poc =
> +        container_of(ctxt, struct priv_op_ctxt, ctxt);
> +    unsigned int rc;
> +    unsigned long addr = poc->cs.base + offset;
> +
> +    ASSERT(seg == x86_seg_cs);
> +
> +    /* We don't mean to emulate any branches. */
> +    if ( !bytes )
> +        return X86EMUL_UNHANDLEABLE;
> +
> +    rc = pv_emul_virt_to_linear(poc->cs.base, offset, bytes, poc->cs.limit,
> +                                x86_seg_cs, ctxt, &addr);
> +    if ( rc != X86EMUL_OKAY )
> +        return rc;
> +
> +    if ( (rc = __copy_from_user(p_data, (void *)addr, bytes)) != 0 )
> +    {
> +        x86_emul_pagefault(cpu_has_nx ? PFEC_insn_fetch : 0,
> +                           addr + bytes - rc, ctxt);

Please can we retain the 0 and comment here.

> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -1185,7 +1185,7 @@ static int ioport_access_check(
>  
>      fail_if(ops->read_segment == NULL);
>      if ( (rc = ops->read_segment(x86_seg_tr, &tr, ctxt)) != 0 )
> -        return rc;
> +        return rc == X86EMUL_DONE ? X86EMUL_OKAY : rc;

Please have at least a comment here

/* Used by the PV path to defer the port permission check to the ioport
hooks. */

>  
>      /* Ensure the TSS has an io-bitmap-offset field. */
>      generate_exception_if(tr.attr.fields.type != 0xb, EXC_GP, 0);
>

Other than that, subject to double checking the IOPL behaviour,
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

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