On 14/10/16 18:05, Kyle Huey wrote:
> On Fri, Oct 14, 2016 at 7:46 AM, Andrew Cooper
> <andrew.coop...@citrix.com> wrote:
>> On 14/10/16 13:04, Jan Beulich wrote:
>>>>>> On 13.10.16 at 23:09, <m...@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
>> The test can be found
>> 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.
> Thanks for the test. It almost works out of the box (will post a diff
:) I am now curious as to which bit I missed.
> It also found another bug: my patches currently advance the
> ip when cpuid faults in an HVM guest. Will fix that too.
Ah of course. Looks like you need to return 1 avoid the eip update, but
I have to admit that this is quite a poor interface.
In some copious free time, I will see about updating it to use the
On a slightly separate note, as you have just been a successful
guinea-pig for XTF, how did you find it? It is a very new (still
somewhat in development) system but the project is looking to try and
improve regression testing in this way, especially for new features. I
welcome any feedback.
Xen-devel mailing list