linux-kernel - Re: [RFC PATCH 05/13] kvm: Add #PF injection for KVM XO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <99d2bf928d1971e7cacfcfa711e82aeac5186632.camel@intel.com>
Date:   Fri, 4 Oct 2019 19:11:16 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "Christopherson, Sean J" <sean.j.christopherson@...el.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "luto@...nel.org" <luto@...nel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "Hansen, Dave" <dave.hansen@...el.com>
CC:     "kristen@...ux.intel.com" <kristen@...ux.intel.com>,
        "Dock, Deneen T" <deneen.t.dock@...el.com>
Subject: Re: [RFC PATCH 05/13] kvm: Add #PF injection for KVM XO

On Fri, 2019-10-04 at 09:42 +0200, Paolo Bonzini wrote:
> On 03/10/19 23:23, Rick Edgecombe wrote:
> > +	if (!vcpu->arch.gva_available)
> > +		return 0;
> 
> Please return RET_PF_* constants, RET_PF_EMULATE here.

Ok.

> > +	if (error_code & PFERR_WRITE_MASK)
> > +		fault_error_code |= X86_PF_WRITE;
> > +
> > +	fault.vector = PF_VECTOR;
> > +	fault.error_code_valid = true;
> > +	fault.error_code = fault_error_code;
> > +	fault.nested_page_fault = false;
> > +	fault.address = vcpu->arch.gva_val;
> > +	fault.async_page_fault = true;
> 
> Not an async page fault.

Right.

> > +	kvm_inject_page_fault(vcpu, &fault);
> > +
> > +	return 1;
> 
> Here you would return RET_PF_RETRY - you've injected the page fault and
> all that's left to do is reenter execution of the vCPU.
> 
> [...]
> 
> > +	if (unlikely(vcpu->arch.xo_fault)) {
> > +		/*
> > +		 * If not enough information to inject the fault,
> > +		 * emulate to figure it out and emulate the PF.
> > +		 */
> > +		if (!try_inject_exec_only_pf(vcpu, error_code))
> > +			return RET_PF_EMULATE;
> > +
> > +		return 1;
> > +	}
> 
> Returning 1 is wrong, it's also RET_PF_EMULATE.  If you change
> try_inject_exec_only_pf return values to RET_PF_*, you can simply return
> the value of try_inject_exec_only_pf(vcpu, error_code).

Oh right! I must have broken this at some point. Thanks. 

> That said, I wonder if it's better to just handle this in
> handle_ept_violation.  Basically, if bits 5:3 of the exit qualification
> are 100 you can bypass the whole mmu.c page fault handling and just
> inject an exec-only page fault.
> 
> Thanks,
> 
> Paolo

Hmm, that could be cleaner. I'll see how it fits together when I fix the nested
case, since some of that logic looks to be in mmu.c.

Thanks,

Rick