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 20/09/13 18:03, George Dunlap wrote:
> 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?

It does look as if the HVM path doesn't do any checking at all.

  -George