Subject: Re: [PATCH v2 3/3] kvm: svm: Use the hardware provided GPA instead of
 page walk
To: Paolo Bonzini <pbonzini@redhat.com>, <kvm@vger.kernel.org>
References: <147992048887.27638.17559991037474542240.stgit@brijesh-build-machine>
 <147992052008.27638.18095073174935903705.stgit@brijesh-build-machine>
 <65a10dd8-5fae-350f-b597-f8f0261da766@redhat.com>
CC: <brijesh.singh@amd.com>, <thomas.lendacky@amd.com>,
        <rkrcmar@redhat.com>, <joro@8bytes.org>, <x86@kernel.org>,
        <linux-kernel@vger.kernel.org>, <mingo@redhat.com>, <hpa@zytor.com>,
        <tglx@linutronix.de>, <bp@suse.de>
From: Brijesh Singh <brijesh.singh@amd.com>
Message-ID: <9820037d-e3ca-0131-3b04-2e51f2abc883@amd.com>
Date: Thu, 8 Dec 2016 09:39:59 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <65a10dd8-5fae-350f-b597-f8f0261da766@redhat.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Dec 2016 15:40:02.9791 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR12MB0665
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 5496
Lines: 163

Hi Paolo,

On 12/08/2016 08:52 AM, Paolo Bonzini wrote:
>
>
> On 23/11/2016 18:02, Brijesh Singh wrote:
>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>
>> When a guest causes a NPF which requires emulation, KVM sometimes walks
>> the guest page tables to translate the GVA to a GPA. This is unnecessary
>> most of the time on AMD hardware since the hardware provides the GPA in
>> EXITINFO2.
>>
>> The only exception cases involve string operations involving rep or
>> operations that use two memory locations. With rep, the GPA will only be
>> the value of the initial NPF and with dual memory locations we won't know
>> which memory address was translated into EXITINFO2.
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> Reviewed-by: Borislav Petkov <bp@suse.de>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>
> Tom, Brijesh,
>
> I would like to confirm that you have run kvm-unit-tests with this patch?
> I haven't yet tried AMD (will do before sending the pull request to Linus),
> but a similar patch for Intel gives me these 4 failures on emulator.flat:
>
> FAIL: push mem
> FAIL: pop mem
> FAIL: cross-page mmio read
> FAIL: cross-page mmio write
>

I did not ran kvm-unit-tests. However, I was able to boot Linux
guest and run some stress test. I will run kvm-unit-tests and let
you know soon.

-Brijesh
> The VMX patch to set gpa_available is just this:
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 25d48380c312..5d7b60d4795b 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6393,6 +6393,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
>  	/* ept page table is present? */
>  	error_code |= (exit_qualification & 0x38) != 0;
>
> +	vcpu->arch.gpa_available = true;
>  	vcpu->arch.exit_qualification = exit_qualification;
>
>  	return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
> @@ -6410,6 +6411,7 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
>  	}
>
>  	ret = handle_mmio_page_fault(vcpu, gpa, true);
> +	vcpu->arch.gpa_available = true;
>  	if (likely(ret == RET_MMIO_PF_EMULATE))
>  		return x86_emulate_instruction(vcpu, gpa, 0, NULL, 0) ==
>  					      EMULATE_DONE;
> @@ -8524,6 +8526,7 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
>  	u32 vectoring_info = vmx->idt_vectoring_info;
>
>  	trace_kvm_exit(exit_reason, vcpu, KVM_ISA_VMX);
> +	vcpu->arch.gpa_available = false;
>
>  	/*
>  	 * Flush logged GPAs PML buffer, this will make dirty_bitmap more
>
> Thanks,
>
> Paolo
>
>> ---
>>  arch/x86/include/asm/kvm_emulate.h |    3 +++
>>  arch/x86/include/asm/kvm_host.h    |    3 +++
>>  arch/x86/kvm/svm.c                 |    2 ++
>>  arch/x86/kvm/x86.c                 |   17 ++++++++++++++++-
>>  4 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
>> index e9cd7be..2d1ac09 100644
>> --- a/arch/x86/include/asm/kvm_emulate.h
>> +++ b/arch/x86/include/asm/kvm_emulate.h
>> @@ -344,6 +344,9 @@ struct x86_emulate_ctxt {
>>  	struct read_cache mem_read;
>>  };
>>
>> +/* String operation identifier (matches the definition in emulate.c) */
>> +#define CTXT_STRING_OP	(1 << 13)
>> +
>>  /* Repeat String Operation Prefix */
>>  #define REPE_PREFIX	0xf3
>>  #define REPNE_PREFIX	0xf2
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 77cb3f9..fd5b1c8 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -668,6 +668,9 @@ struct kvm_vcpu_arch {
>>
>>  	int pending_ioapic_eoi;
>>  	int pending_external_vector;
>> +
>> +	/* GPA available (AMD only) */
>> +	bool gpa_available;
>>  };
>>
>>  struct kvm_lpage_info {
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index 5e64e656..1bbd04c 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -4246,6 +4246,8 @@ static int handle_exit(struct kvm_vcpu *vcpu)
>>  		return 1;
>>  	}
>>
>> +	vcpu->arch.gpa_available = (exit_code == SVM_EXIT_NPF);
>> +
>>  	return svm_exit_handlers[exit_code](svm);
>>  }
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index c30f62dc..5002eea 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -4441,7 +4441,19 @@ static int vcpu_mmio_gva_to_gpa(struct kvm_vcpu *vcpu, unsigned long gva,
>>  		return 1;
>>  	}
>>
>> -	*gpa = vcpu->arch.walk_mmu->gva_to_gpa(vcpu, gva, access, exception);
>> +	/*
>> +	 * 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 when the NPF occurred.
>> +	 */
>> +	if (vcpu->arch.gpa_available &&
>> +	    !(vcpu->arch.emulate_ctxt.d & CTXT_STRING_OP))
>> +		*gpa = exception->address;
>> +	else
>> +		*gpa = vcpu->arch.walk_mmu->gva_to_gpa(vcpu, gva, access,
>> +						       exception);
>>
>>  	if (*gpa == UNMAPPED_GVA)
>>  		return -1;
>> @@ -5563,6 +5575,9 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
>>  	}
>>
>>  restart:
>> +	/* Save the faulting GPA (cr2) in the address field */
>> +	ctxt->exception.address = cr2;
>> +
>>  	r = x86_emulate_insn(ctxt);
>>
>>  	if (r == EMULATION_INTERCEPTED)
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>