> 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 majordomo@xxxxxxxxxxxxxxx
> >>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>
>
--
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
