Re: [Xen-devel] [PATCH] x86/ioemul: Account for ioemul_handle_quirk() in stub length check

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.01.18 at 17:47, <andrew.cooper3@xxxxxxxxxx> wrote:
> The opcode potentially written into ctxt->io_emul_stub[] in the case
> that ioemul_handle_quirk() is overriding the default logic isnt
> accounted for in the build-time check that the stubs are large enough.
> 
> Introduce IOEMUL_QUIRK_STUB_BYTES and use for both the main and quirk
> stub cases.  As a slim optimisation, avoid writing out the default stub
> when we know we are going to overwrite it.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with one suggestion:

> --- a/xen/arch/x86/pv/emul-priv-op.c
> +++ b/xen/arch/x86/pv/emul-priv-op.c
> @@ -89,19 +89,24 @@ static io_emul_stub_t *io_emul_stub_setup(struct 
> priv_op_ctxt *ctxt, u8 opcode,
>      /* callq *%rcx */
>      ctxt->io_emul_stub[10] = 0xff;
>      ctxt->io_emul_stub[11] = 0xd1;
> -    /* data16 or nop */
> -    ctxt->io_emul_stub[12] = (bytes != 2) ? 0x90 : 0x66;
> -    /* <io-access opcode> */
> -    ctxt->io_emul_stub[13] = opcode;
> -    /* imm8 or nop */
> -    ctxt->io_emul_stub[14] = !(opcode & 8) ? port : 0x90;
> -    /* ret (jumps to guest_to_host_gpr_switch) */
> -    ctxt->io_emul_stub[15] = 0xc3;
> -    BUILD_BUG_ON(STUB_BUF_SIZE / 2 < 16);
> -
> -    if ( ioemul_handle_quirk )
> +
> +    if ( likely(!ioemul_handle_quirk) )
> +    {
> +        /* data16 or nop */
> +        ctxt->io_emul_stub[12] = (bytes != 2) ? 0x90 : 0x66;
> +        /* <io-access opcode> */
> +        ctxt->io_emul_stub[13] = opcode;
> +        /* imm8 or nop */
> +        ctxt->io_emul_stub[14] = !(opcode & 8) ? port : 0x90;
> +        /* ret (jumps to guest_to_host_gpr_switch) */
> +        ctxt->io_emul_stub[15] = 0xc3;
> +    }
> +    else
>          ioemul_handle_quirk(opcode, &ctxt->io_emul_stub[12], 
> ctxt->ctxt.regs);
>  
> +    BUILD_BUG_ON(STUB_BUF_SIZE / 2 < MAX(16, /* Regular stubs */

Now that we have it available globally, would you mind using
MAX_INST_LEN + 1 here instead of the literal 16?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel