On 15/12/2016 13:42, David Hildenbrand wrote:
>
>> +++ b/arch/x86/kvm/x86.c
>> @@ -4420,6 +4420,21 @@ int kvm_write_guest_virt_system(struct
>> x86_emulate_ctxt *ctxt,
>> }
>> EXPORT_SYMBOL_GPL(kvm_write_guest_virt_system);
>>
>> +static int vcpu_is_mmio_gpa(struct kvm_vcpu *vcpu, unsigned long gva,
>> + gpa_t gpa, bool write)
>> +{
>> + /* For APIC access vmexit */
>> + if ((gpa & PAGE_MASK) == APIC_DEFAULT_PHYS_BASE)
>> + return 1;
>> +
>> + if (vcpu_match_mmio_gpa(vcpu, gpa)) {
>> + trace_vcpu_match_mmio(gva, gpa, write, true);
>> + return 1;
>> + }
>> +
>> + return 0;
>> +}
>
> I think I'd prefer that in a separate patch. But I don't have any
> strong feelings about this.
>
>> +
>> static int vcpu_mmio_gva_to_gpa(struct kvm_vcpu *vcpu, unsigned long
>> gva,
>> gpa_t *gpa, struct x86_exception *exception,
>> bool write)
>> @@ -4446,16 +4461,7 @@ static int vcpu_mmio_gva_to_gpa(struct kvm_vcpu
>> *vcpu, unsigned long gva,
>> if (*gpa == UNMAPPED_GVA)
>> return -1;
>>
>> - /* For APIC access vmexit */
>> - if ((*gpa & PAGE_MASK) == APIC_DEFAULT_PHYS_BASE)
>> - return 1;
>> -
>> - if (vcpu_match_mmio_gpa(vcpu, *gpa)) {
>> - trace_vcpu_match_mmio(gva, *gpa, write, true);
>> - return 1;
>> - }
>> -
>> - return 0;
>> + return vcpu_is_mmio_gpa(vcpu, gva, *gpa, write);
>> }
>>
>> int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
>> @@ -4552,6 +4558,22 @@ static int emulator_read_write_onepage(unsigned
>> long addr, void *val,
>> int handled, ret;
>> bool write = ops->write;
>> struct kvm_mmio_fragment *frag;
>> + struct x86_emulate_ctxt *ctxt = &vcpu->arch.emulate_ctxt;
>> +
>> + /*
>> + * If the exit was due to a NPF we may already have a GPA.
>> + * If the GPA is present, use it to avoid the GVA to GPA table walk.
>> + * Note, this cannot be used on string operations since string
>> + * operation using rep will only have the initial GPA from the NPF
>> + * occurred.
>> + */
>
> I was wondering if it would make sense to get rid of gpa_available and
> rather define a new function:
>
> bool exception_gpa_valid(struct kvm_vcpu)
> {
> // check if svm
> // check if exit code is NPF
> // check ctxt
> }
No, this would be a layering violation. The emulator ops don't know
about svm and exit codes (and in fact it's trivial to implement this
optimization for vmx, with a slightly different logic), so we need to
have gpa_available.
Paolo
> Then you could move the whole comment into that function.
>
>
> Looks good to me in general.
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
