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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.12.16 at 17:53, <andrew.cooper3@xxxxxxxxxx> wrote:
> 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.

Oh, sure - I should have extended the similar change to patch 1
to 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>

Thanks, Jan


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