Re: [PATCH v2 2/2] x86/Intel: virtualize support for cpuid faulting

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

On 14/10/16 13:04, Jan Beulich wrote:
>>>> On 13.10.16 at 23:09, <me@kylehuey.com> wrote:
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -1315,16 +1315,20 @@ 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;
>> +    /* Let the guest have this one */
>> +    if ( current->arch.cpuid_fault && !guest_kernel_mode(current, regs) )
>> +        return 0;
>> +
> I think another blank line ahead of the addition would be nice. I also
> think the comment should include the conditional aspect of what is
> says (same on the second instance below).
>
> And then there is the question of state here: There's no rIP update
> ahead of here, yet I'm uncertain whether we can expect the guest
> kernel to handle both bare and Xen-prefixed CPUID instructions.
> Andrew, what do you think?

The #GP fault handed back to the guest kernel should point at the cpuid
instruction, not the ud2 of the FEP.

In reality, the only real use of this path from userspace is the
`xen-detect` utility.  Regular userspace shouldn't know or care. 
Therefore, I am not concerned about the fact that it causes an implicit
change of information source.


I have taken the liberty of writing a CPUID Faulting XTF test, which
should cover all the intended guest-side behaviour (although I did code
it without a hypervisor side implementation, so I don't guarantee it is
bug-free).

The test can be found
http://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen-test-framework.git;a=shortlog;h=refs/heads/cpuid-faulting
and general docs on using XTF can be found
http://xenbits.xen.org/docs/xtf/index.html  After cloning and building,
use "./xtf-runner cpuid-faulting" to run the test in all types of VM.

In particular, it will catch this specific issue when the exception
frame from an emulated cpuid doesn't point at the cpuid instruction.

Would you mind trying out this test?  Ideally, we would look to putting
it (or a bugfixed version of it) into our CI system at the same time as
the hypervisor support gets accepted.

~Andrew

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