> I am able to reproduce it on AMD HW using kvm-unit-tests. Looking at > test, the initial thought is "push mem" has two operands (the memory > being pushed and the stack pointer). The provided GPA could be either > one of those.
Aha, this makes sense---and it's easy to handle too, since you can just add a flag to the decode table and extend the string op case to cover that flag too. Detecting cross-page MMIO is more problematic, I don't have an idea offhand of how to solve it. > If we can detect those cases, we should not set the gpa_available on > them (like what we do with string move). It would forbid usage of "push/pop mem" instructions with MMIO for SEV, right? It probably doesn't happen much in practice, but it's unfortunate. Paolo > We probably haven't hit this case in guest booting. Will investigate bit > further and provide a updated patch to handle it. > > > -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 majord...@vger.kernel.org > >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > >>> >