Re: [PATCH 04/15] KVM: MMU: cache mmio info on page fault path

Alexander Graf
2011-06-08T09:19:04Z

On 08.06.2011, at 10:58, Xiao Guangrong wrote:

> On 06/08/2011 04:22 PM, Alexander Graf wrote:
>
>>> +static int vcpu_gva_to_gpa(struct kvm_vcpu *vcpu, unsigned long gva,
>>> + gpa_t *gpa, struct x86_exception *exception,
>>> + bool write)
>>> +{
>>> + u32 access = (kvm_x86_ops->get_cpl(vcpu) == 3) ? PFERR_USER_MASK : 0;
>>> +
>>> + if (vcpu_match_mmio_gva(vcpu, gva) &&
>>> + check_write_user_access(vcpu, write, access,
>>> + vcpu->arch.access)) {
>>> + *gpa = vcpu->arch.mmio_gfn << PAGE_SHIFT |
>>> + (gva & (PAGE_SIZE - 1));
>>> + return 1;
>>
>
> Hi Alexander,
>
> Thanks for your review!
>
>> Hrm. Let me try to understand what you're doing.
>>
>> Whenever a guest issues an MMIO, it triggers an #NPF or #PF and then we walk either the NPT or the guest PT to resolve the GPA to the fault and send off an MMIO.
>> Within that path, you remember the GVA->GPA mapping for the last MMIO request. If the next MMIO request is on the same GVA and kernel/user permissions still apply, you simply bypass the resolution. So far so good.
>>
>
> In this patch, we also introduced vcpu_clear_mmio_info() that clears mmio cache info on the vcpu,
> and it is called when guest flush tlb (reload CR3 or INVLPG).

Ah, that one solved the SPT case then of course.

>
>> Now, what happens when the GVA is not identical to the GVA it was before? It's probably a purely theoretic case, but imagine the following:
>>
>> 1) guest issues MMIO on GVA 0x1000 (GPA 0x1000)
>> 2) guest remaps page 0x1000 to GPA 0x2000
>> 3) guest issues MMIO on GVA 0x1000
>>
>
> If guest modify the page structure, base on x86 tlb rules, we should flush tlb to ensure the cpu use
> the new mapping.
>
> When you remap GVA 0x1000 to 0x2000, you should flush tlb, then mmio cache info is cleared, so the later
> access is right.
>
>> That would break with your current implementation, right? It sounds pretty theoretic, but imagine the following:
>>
>> 1) guest user space 1 maps MMIO region A to 0x1000
>> 2) guest user space 2 maps MMIO region B to 0x1000
>> 3) guest user space 1 issues MMIO on 0x1000
>> 4) context switch; going to user space 2
>> 5) user space 2 issues MMIO on 0x1000
>>
>
> Also, when context switched, CR3 is reloaded, mmio cache info can be cleared too. right? :-)

Only when using SPT. In the NPT case, you will never see cr3 getting reloaded or INVLPG :).

Alex