Re: [Xen-devel] [PATCH v4 2/2] x86/Intel: virtualize support for cpuid faulting

[Kyle Huey <me@xxxxxxxxxxxx>]

On Tue, Oct 18, 2016 at 3:09 AM, Andrew Cooper
<andrew.cooper3@xxxxxxxxxx> wrote:
> On 17/10/16 19:51, Kyle Huey wrote:
>> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
>> index 6ed7486..a713ff3 100644
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -1544,16 +1544,35 @@ static int hvmemul_wbinvd(
>>
>>  static int hvmemul_cpuid(
>>      unsigned int *eax,
>>      unsigned int *ebx,
>>      unsigned int *ecx,
>>      unsigned int *edx,
>>      struct x86_emulate_ctxt *ctxt)
>>  {
>> +    /*
>> +     * x86_emulate uses this function to query CPU features for its own 
>> internal
>> +     * use. Make sure we're actually emulating CPUID before emulating CPUID
>> +     * faulting.
>
> Looking into this, it is all a complete tangle.
>
> Conceptually, the correct way to do this is to introduce
> cpuid_faulting_active() to mirror the existing umip_active().  However,
> the read_msr() infrastructure latched a #GP fault behind the back of the
> emulator, so doesn't work for speculative reads.
>
> Therefore, I am happy to accept the code in this form, because it looks
> like the least bad option available at the moment.  I will see about
> fixing it when I do the planned MSR overhaul work.
>
> Otherwise, just a few style corrections.

Ok, thanks. :)

- Kyle

>> +     */
>> +    if ( ctxt->opcode == X86EMUL_OPC(0x0f, 0xa2) &&
>> +         hvm_check_cpuid_fault(current) ) {
>
> Brace on newline please.
>
>> +        struct hvm_emulate_ctxt *hvmemul_ctxt =
>> +            container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>> +
>> +        hvmemul_ctxt->exn_pending = 1;
>> +        hvmemul_ctxt->trap.vector = TRAP_gp_fault;
>> +        hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION;
>> +        hvmemul_ctxt->trap.error_code = 0;
>> +        hvmemul_ctxt->trap.insn_len = 0;
>> +
>> +        return X86EMUL_EXCEPTION;
>> +    }
>> +
>>      hvm_funcs.cpuid_intercept(eax, ebx, ecx, edx);
>>      return X86EMUL_OKAY;
>>  }
>>
>>  static int hvmemul_inject_hw_exception(
>>      uint8_t vector,
>>      int32_t error_code,
>>      struct x86_emulate_ctxt *ctxt)
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index b9102ce..228c1b9 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -2428,16 +2428,21 @@ static void vmx_cpuid_intercept(
>>      HVMTRACE_5D (CPUID, input, *eax, *ebx, *ecx, *edx);
>>  }
>>
>>  static int vmx_do_cpuid(struct cpu_user_regs *regs)
>>  {
>>      unsigned int eax, ebx, ecx, edx;
>>      unsigned int leaf, subleaf;
>>
>> +    if ( hvm_check_cpuid_fault(current) ) {
>
> And here please.
>
>> +        hvm_inject_hw_exception(TRAP_gp_fault, 0);
>> +        return 1;  /* Don't advance the guest IP! */
>> +    }
>> +
>>      eax = regs->eax;
>>      ebx = regs->ebx;
>>      ecx = regs->ecx;
>>      edx = regs->edx;
>>
>>      leaf = regs->eax;
>>      subleaf = regs->ecx;
>>
>> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
>> index 293ff8d..12322bd 100644
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -1315,16 +1315,24 @@ static int emulate_forced_invalid_op(struct 
>> cpu_user_regs *regs)
>>      /* We only emulate CPUID. */
>>      if ( ( rc = copy_from_user(instr, (char *)eip, sizeof(instr))) != 0 )
>>      {
>>          propagate_page_fault(eip + sizeof(instr) - rc, 0);
>>          return EXCRET_fault_fixed;
>>      }
>>      if ( memcmp(instr, "\xf\xa2", sizeof(instr)) )
>>          return 0;
>> +
>> +    /* If cpuid faulting is enabled and CPL>0 inject a #GP in place of #UD. 
>> */
>> +    if ( current->arch.cpuid_fault && !guest_kernel_mode(current, regs) ) {
>
> And here.
>
> ~Andrew
>
>> +        regs->eip = eip;
>> +        do_guest_trap(TRAP_gp_fault, regs);
>> +        return EXCRET_fault_fixed;
>> +    }
>> +
>>      eip += sizeof(instr);
>>
>>      pv_cpuid(regs);
>>
>>      instruction_done(regs, eip, 0);
>>
>>      trace_trap_one_addr(TRC_PV_FORCED_INVALID_OP, regs->eip);
>>
>>

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