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;
> }
>
>
> Paolo
>
> > ---
> > arch/x86/kvm/x86.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 965c4f0108eb..3975331230b9 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -5532,7 +5532,7 @@ static int emulator_read_write_onepage(unsigned long addr, void *val,
> > * operation using rep will only have the initial GPA from the NPF
> > * occurred.
> > */
> > - if (vcpu->arch.gpa_available &&
> > + if (vcpu->arch.gpa_available && !kvmi_is_present() &&
> > emulator_can_use_gpa(ctxt) &&
> > (addr & ~PAGE_MASK) == (vcpu->arch.gpa_val & ~PAGE_MASK)) {
> > gpa = vcpu->arch.gpa_val;
> >
>
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);
