From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH v2 2/2] KVM: Fix writeback on page boundary that propagate changes in spite of #PF Date: Thu, 26 Jan 2012 13:54:09 +0200 Message-ID: <20120126115409.GE30469@redhat.com> References: <1327000617-4283-1-git-send-email-namit@cs.technion.ac.il> <1327000617-4283-2-git-send-email-namit@cs.technion.ac.il> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , Nadav Amit , Marcelo Tosatti , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org To: Nadav Amit Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On Sun, Jan 22, 2012 at 01:01:18PM +0200, Nadav Amit wrote: > Consider the case in which an instruction emulation writeback is performed on a page boundary. > In such case, if a #PF occurs on the second page, the write to the first page already occurred and cannot be retracted. > Therefore, validation of the second page access must be performed prior to writeback. > Patch is corrupted. > Signed-off-by: Nadav Amit > --- > arch/x86/kvm/x86.c | 60 +++++++++++++++++++++++++++++---------------------- > 1 files changed, 34 insertions(+), 26 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 05fd3d7..0e86f3a 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3689,33 +3689,27 @@ static struct read_write_emulator_ops write_emultor = { > .write = true, > }; > > -static int emulator_read_write_onepage(unsigned long addr, void *val, > +static int emulator_read_write_onepage(gpa_t gpa, void *val, > unsigned int bytes, > - struct x86_exception *exception, > struct kvm_vcpu *vcpu, > - struct read_write_emulator_ops *ops) > + struct read_write_emulator_ops *ops, > + bool mmio) > { > - gpa_t gpa; > - int handled, ret; > + int handled; > bool write = ops->write; > > if (ops->read_write_prepare && > ops->read_write_prepare(vcpu, val, bytes)) > return X86EMUL_CONTINUE; > > - ret = vcpu_mmio_gva_to_gpa(vcpu, addr, &gpa, exception, write); > - > - if (ret < 0) > - return X86EMUL_PROPAGATE_FAULT; > - > /* For APIC access vmexit */ > - if (ret) > - goto mmio; > + if (mmio) > + goto do_mmio; > > if (ops->read_write_emulate(vcpu, gpa, val, bytes)) > return X86EMUL_CONTINUE; > > -mmio: > +do_mmio: > /* > * Is this MMIO handled locally? > */ > @@ -3744,24 +3738,38 @@ int emulator_read_write(struct x86_emulate_ctxt *ctxt, unsigned long addr, > struct read_write_emulator_ops *ops) > { > struct kvm_vcpu *vcpu = emul_to_vcpu(ctxt); > + int i, rc; > + bool write = ops->write; > + gpa_t gpa[2]; > + int npages = (((addr + bytes - 1) ^ addr) & PAGE_MASK) ? 2 : 1; > + unsigned int offset[2] = {0}; > + unsigned int p_bytes[2] = {bytes, 0}; > + bool mmio[2]; > + > + if (npages == 2) { > + p_bytes[0] = offset[1] = -addr & ~PAGE_MASK; > + p_bytes[1] = bytes - p_bytes[0]; > + } > + > + /* First check there is no page-fault on the next page */ > + for (i = 0; i < npages; i++) { > + rc = vcpu_mmio_gva_to_gpa(vcpu, addr + offset[i], &gpa[i], > + exception, write); > + if (rc < 0) > + return X86EMUL_PROPAGATE_FAULT; > + mmio[i] = (rc > 0); > + } > > - /* Crossing a page boundary? */ > - if (((addr + bytes - 1) ^ addr) & PAGE_MASK) { > - int rc, now; > - > - now = -addr & ~PAGE_MASK; > - rc = emulator_read_write_onepage(addr, val, now, exception, > - vcpu, ops); > - > + /* Actual read/write */ > + for (i = 0; i < npages; i++) { > + rc = emulator_read_write_onepage(gpa[i], val + offset[i], > + p_bytes[i], vcpu, ops, > + mmio[i]); > if (rc != X86EMUL_CONTINUE) > return rc; > - addr += now; > - val += now; > - bytes -= now; > } > > - return emulator_read_write_onepage(addr, val, bytes, exception, > - vcpu, ops); > + return X86EMUL_CONTINUE; > } > > static int emulator_read_emulated(struct x86_emulate_ctxt *ctxt, > -- > 1.7.4.1 > -- Gleb.