Re: [V10 PATCH 23/23] PVH xen: introduce vmexit handler for PVH

[George Dunlap <george.dunlap@eu.citrix.com>]

On 15/08/13 17:37, Tim Deegan wrote:
> At 16:51 +0100 on 15 Aug (1376585469), George Dunlap wrote:
>> On Wed, Aug 7, 2013 at 10:54 AM, Tim Deegan <tim@xen.org> wrote:
>>> At 17:37 -0700 on 06 Aug (1375810631), Mukesh Rathor wrote:
>>>> On Fri, 26 Jul 2013 11:45:19 +0100
>>>> Tim Deegan <tim@xen.org> wrote:
>>>>
>>>>> At 19:30 -0700 on 25 Jul (1374780657), Mukesh Rathor wrote:
>>>>>> On Thu, 25 Jul 2013 17:28:40 +0100
>>>>>> Tim Deegan <tim@xen.org> wrote:
>>>>>>
>>>>>>> At 18:59 -0700 on 23 Jul (1374605971), Mukesh Rathor wrote:
>>>>>>>> +/* Just like HVM, PVH should be using "cpuid" from the kernel
>>>>>>>> mode. */ +static int vmxit_invalid_op(struct cpu_user_regs
>>>>>>>> *regs) +{
>>>>>>>> +    if ( guest_kernel_mode(current, regs)
>>>>>>>> || !emulate_forced_invalid_op(regs) )
>>>>>>>> +        hvm_inject_hw_exception(TRAP_invalid_op,
>>>>>>>> HVM_DELIVER_NO_ERROR_CODE);
>>>>>>> Was this discussed before?  It seems harsh to stop kernel-mode
>>>>>>> code from using the pv cpuid operation if it wants to.  In
>>>>>>> particular, what about loadable kernel modules?
>>>>>> Yes, few times on the xen mailing list. The only PVH guest, linux
>>>>>> as of now, the pv ops got rewired to use native cpuid, which is
>>>>>> how hvm does it.
>>>>> Yes, but presumably you want to make it easy for other PV guests to
>>>>> port to PVH too?
>>>> True, but how would not allowing kernel mode emulation impede that?
>>>> I fail to understand why a new kernel would wanna use xen signature
>>>> emulation over just plain cpuid instruction?
>>> I'm talking about existing PV kernel code that already uses PV CPUID.
>>> And in particular what if that kernel code is in a device driver, or
>>> even a third-party loadable module?  Porting the core kernel from PV to
>>> PVH shouldn't break that code if it doesn't have to.
>>>
>>> But TBH my objection is really more aesthetic than anything else.
>>> Restricting the PV CPUID instruction here adds another ragged edge in
>>> the ABI that all kernel writers have to think about, and for little or
>>> no benfit.  And, to be clear, I object to it and this patch does not
>>> have my Ack.
>> Are you in particular saying that you think PVH guests should be
>> allowed to use the PV CPUID as a prerequisite for the patch series to
>> go in?
>>
>> Correct me if I'm wrong, but:
>> 1. CPUID is the only forced_emulated op at the moment
>> 2. The kernel is the only caller we can reasonably expect to use the
>> forced_emulated ops
> No - the forced-invalid-op CPUID is used from userspace too, e.g. in
> tools/misc/xen-detect.c.  The current patch keeps support for userspace
> but not for kernel users.  I would prefer to keep it for both, on the
> grounds that even in the kernel there may be users of it that the person
> porting the kernel does not control (e.g. in third-party modules).

Ah, right.  That makes sense then -- we'd want to have the same 
toolstack binaries for PV and PVH.  But then if we're going to have it, 
I agree that there's no reason not to just go ahead and make it 
available in all modes.  We can just deprecate it in kernel mode.

  -George