Re: [PATCH 3/9] x86/vvmx: Extract operand reading logic into operand_read()

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/11/17 17:01, Jan Beulich wrote:
>>>> On 26.10.17 at 19:03, <euan.harris@citrix.com> wrote:
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -361,6 +361,40 @@ static void reg_write(struct cpu_user_regs *regs,
>>      *pval = value;
>>  }
>>  
>> +static int operand_read(void *buf, struct vmx_inst_op *op,
>> +                        struct cpu_user_regs *regs, unsigned int bytes)
>> +{
>> +    if ( op->type == VMX_INST_MEMREG_TYPE_REG )
>> +    {
>> +        switch ( bytes )
>> +        {
>> +        case 4:
>> +            *(uint32_t *)buf = reg_read(regs, op->reg_idx);
>> +
>> +        case 8:
>> +            *(uint64_t *)buf = reg_read(regs, op->reg_idx);
>> +
>> +        default:
>> +            ASSERT_UNREACHABLE();
>> +            return X86EMUL_UNHANDLEABLE;
>> +        }
> Looks like there are two missing break statements up there.

I fixed these up x86-next.  (Although it was only because Coverity
noticed and complained at me...)

>
>> +        return X86EMUL_OKAY;
> This and ...
>
>> +    }
>> +    else
>> +    {
>> +        pagefault_info_t pfinfo;
>> +        int rc = hvm_copy_from_guest_linear(buf, op->mem, bytes, 0, &pfinfo);
>> +
>> +        if ( rc == HVMTRANS_bad_linear_to_gfn )
>> +            hvm_inject_page_fault(pfinfo.ec, pfinfo.linear);
>> +        if ( rc != HVMTRANS_okay )
>> +            return X86EMUL_EXCEPTION;
>> +
>> +        return X86EMUL_OKAY;
> ... this should become a uniform ...
>
>> +    }
> ... return here.

I tried this, but later patches in the series need them to move back. 
Overall, this layout reduces the code churn across the series.

>
>> @@ -440,7 +474,12 @@ static int decode_vmx_inst(struct cpu_user_regs *regs,
>>          decode->op[0].type = VMX_INST_MEMREG_TYPE_REG;
>>          decode->op[0].reg_idx = info.fields.reg1;
>>          if ( poperandS != NULL )
>> -            *poperandS = reg_read(regs, decode->op[0].reg_idx);
>> +        {
>> +            int rc = operand_read(poperandS, &decode->op[0], regs,
>> +                                  decode->op[0].len);
>> +            if ( rc != X86EMUL_OKAY )
> Blank line between declaration(s) and statement(s) please.

I can fix this.

~Andrew

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