On 13/08/19 16:33, Adalbert Lazăr wrote:
> On Tue, 13 Aug 2019 10:47:34 +0200, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>> On 09/08/19 18:00, Adalbert Lazăr wrote:
>>> If the EPT violation was caused by an execute restriction imposed by the
>>> introspection tool, gpa_available will point to the instruction pointer,
>>> not the to the read/write location that has to be used to emulate the
>>> current instruction.
>>>
>>> This optimization should be disabled only when the VM is introspected,
>>> not just because the introspection subsystem is present.
>>>
>>> Signed-off-by: Adalbert Lazăr <alazar@xxxxxxxxxxxxxxx>
>>
>> The right thing to do is to not set gpa_available for fetch failures in
>> kvm_mmu_page_fault instead:
>>
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index 24843cf49579..1bdca40fa831 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -5364,8 +5364,12 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code,
>> enum emulation_result er;
>> bool direct = vcpu->arch.mmu->direct_map;
>>
>> - /* With shadow page tables, fault_address contains a GVA or nGPA. */
>> - if (vcpu->arch.mmu->direct_map) {
>> + /*
>> + * With shadow page tables, fault_address contains a GVA or nGPA.
>> + * On a fetch fault, fault_address contains the instruction pointer.
>> + */
>> + if (vcpu->arch.mmu->direct_map &&
>> + likely(!(error_code & PFERR_FETCH_MASK)) {
>> vcpu->arch.gpa_available = true;
>> vcpu->arch.gpa_val = cr2;
>> }
>
> Sure, but I think we'll have to extend the check.
>
> Searching the logs I've found:
>
> kvm/x86: re-translate broken translation that caused EPT violation
>
> Signed-off-by: Mircea Cirjaliu <mcirjaliu@xxxxxxxxxxxxxxx>
>
> arch/x86/kvm/x86.c | 1 +
> 1 file changed, 1 insertion(+)
>
> /home/b/kvmi@9cad844~1/arch/x86/kvm/x86.c:4757,4762 - /home/b/kvmi@9cad844/arch/x86/kvm/x86.c:4757,4763
> */
> if (vcpu->arch.gpa_available &&
> emulator_can_use_gpa(ctxt) &&
> + (vcpu->arch.error_code & PFERR_GUEST_FINAL_MASK) &&
> (addr & ~PAGE_MASK) == (vcpu->arch.gpa_val & ~PAGE_MASK)) {
> gpa = vcpu->arch.gpa_val;
> ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write);
>
Yes, adding that check makes sense as well (still in kvm_mmu_page_fault).
Paolo
