Re: [PATCH RFC v12 16/21] pvh: Use PV handlers for emulated forced invalid ops, cpuid, and IO

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

On 19/09/13 11:09, Jan Beulich wrote:
>>>> On 19.09.13 at 03:02, Mukesh Rathor <mukesh.rathor@oracle.com> wrote:
>> On Wed, 18 Sep 2013 16:31:17 +0100 "Jan Beulich" <JBeulich@suse.com> wrote:
>>>>>> On 13.09.13 at 18:25, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>>>> @@ -1624,6 +1631,13 @@ static int guest_io_okay(
>>>>       int user_mode = !(v->arch.flags & TF_kernel_mode);
>>>>   #define TOGGLE_MODE() if ( user_mode ) toggle_guest_mode(v)
>>>>   
>>>> +    /*
>>>> +     * For PVH we check this in vmexit for
>>>> EXIT_REASON_IO_INSTRUCTION
>>>> +     * and so don't need to check again here.
>>>> +     */
>>>> +    if ( is_pvh_vcpu(v) )
>>>> +        return 1;
>>>> +
>>>>       if ( !vm86_mode(regs) &&
>>>>            (v->arch.pv_vcpu.iopl >= (guest_kernel_mode(v, regs) ?
>>>> 1 : 3)) ) return 1;
>>> Hmm, am I missing something here? The check in the VMEXIT
>>> handler is just a privilege level one - where's the bitmap being
>>> consulted? _If_ the bitmap is being maintained properly for the
>>> guest (which I don't recall having seen), anything leading here
>>> would be for ports the guest was not permitted access to. Yet
>>> we would happily emulate the access for it then.
>> Not sure I understand which bitmap needs to be consulted. The bitmap
>> hvm_io_bitmap is used to set the intercepts which PVH also uses, with
>> HVM defaults.
> Actually I think I got confused by the IOPL checking in your
> VM exit handling code: _If_ you need to check the IOPL, then
> you also need to check the bitmap hanging off of the TSS. But
> aiui all the privilege level checks get done inside the guest, and
> only if those pass would the I/O bitmap hanging off of the VMCS
> be consulted. Hence neither the IOPL check nor the TSS-based
> bitmap check ought to be necessary here.

I'm not quite following this.  Are you saying that the iopl checks are 
done by the hardware, and if failed it will already have delivered a 
GPF; and if we've managed to get an IO_INSTRUCTION exit then they have 
passed, so we don't need the iopl check in vmx.c?

If not, what needs to change here?

  -George