Re: [PATCH 1/2] virtio: fix typos of memory barriers
On Sun, 22 Jan 2012 13:47:46 +0200, "Michael S. Tsirkin" wrote: > On Fri, Jan 20, 2012 at 04:16:59PM +0800, Jason Wang wrote: > > Signed-off-by: Jason Wang > > Good catch. > Note: this fixes a bug introduced by > 7b21e34fd1c272e3a8c3846168f2f6287a4cd72b. > It's probably a good idea to mention > this is the commit log. > > Acked-by: Michael S. Tsirkin Thanks, applied! Cheers, Rusty. -- 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
Re: [PATCH 2/2] virtio: correct the memory barrier in virtqueue_kick_prepare()
On Sun, 22 Jan 2012 13:47:57 +0200, "Michael S. Tsirkin" wrote: > On Fri, Jan 20, 2012 at 04:17:08PM +0800, Jason Wang wrote: > > Use virtio_mb() to make sure the available index to be exposed before > > checking the the avail event. Otherwise we may get stale value of > > avail event in guest and never kick the host after. > > > > Signed-off-by: Jason Wang > > Good catch. > Note: this fixes a bug introduced by ee7cd8981e15bcb365fc762afe3fc47b8242f630. Indeed, great catch. It replaces the missing barrier, and after some careful thought I've convinced myself that it is necessary for both the avail index and flag cases. > > START_USE(vq); > > - /* Descriptors and available array need to be set before we expose the > > -* new available array entries. */ > > - virtio_wmb(vq); > > + /* We need expose available array entries before checking avail > > Nit: > We need expose -> Need to expose > > > +* event. */ > > + virtio_mb(vq); Fixed typo and applied. Thanks, Rusty. -- 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
Re: [Scst-devel] BUG: Process hang when kvm crashed, stacktrace cfs+kvm
On Sat, Jan 21, 2012 at 9:19 PM, kordex wrote: > I hit sysrq+t sysrq+p in order to print the tasks with stacktraces and > registers. Using ps -e -o pid,state,command will hang ps output, > while ps -e -o pid,state will not. Sorry, haven't seen such a hang ever before. Does it also occur with older kernels than 3.1 ? Is it reproducible with 3.3-rc1 ? Bart. -- 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
Re: [PATCH 2/2] virtio: correct the memory barrier in virtqueue_kick_prepare()
On Fri, Jan 20, 2012 at 04:17:08PM +0800, Jason Wang wrote: > Use virtio_mb() to make sure the available index to be exposed before > checking the the avail event. Otherwise we may get stale value of > avail event in guest and never kick the host after. > > Signed-off-by: Jason Wang Good catch. Note: this fixes a bug introduced by ee7cd8981e15bcb365fc762afe3fc47b8242f630. Acked-by: Michael S. Tsirkin > --- > drivers/virtio/virtio_ring.c |6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index 78428a8..07d9360 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -308,9 +308,9 @@ bool virtqueue_kick_prepare(struct virtqueue *_vq) > bool needs_kick; > > START_USE(vq); > - /* Descriptors and available array need to be set before we expose the > - * new available array entries. */ > - virtio_wmb(vq); > + /* We need expose available array entries before checking avail Nit: We need expose -> Need to expose > + * event. */ > + virtio_mb(vq); > > old = vq->vring.avail->idx - vq->num_added; > new = vq->vring.avail->idx; -- 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
Re: [PATCH 1/2] virtio: fix typos of memory barriers
On Fri, Jan 20, 2012 at 04:16:59PM +0800, Jason Wang wrote:
> Signed-off-by: Jason Wang
Good catch.
Note: this fixes a bug introduced by
7b21e34fd1c272e3a8c3846168f2f6287a4cd72b.
It's probably a good idea to mention
this is the commit log.
Acked-by: Michael S. Tsirkin
> ---
> drivers/virtio/virtio_ring.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 79e1b29..78428a8 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -35,7 +35,7 @@
> #define virtio_rmb(vq) \
> do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0)
> #define virtio_wmb(vq) \
> - do { if ((vq)->weak_barriers) smp_rmb(); else rmb(); } while(0)
> + do { if ((vq)->weak_barriers) smp_wmb(); else wmb(); } while(0)
> #else
> /* We must force memory ordering even if guest is UP since host could be
> * running on another CPU, but SMP barriers are defined to barrier() in that
--
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
[PATCH v2 2/2] KVM: Fix writeback on page boundary that propagate changes in spite of #PF
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.
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
--
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
[PATCH v2 1/2] KVM: Exception during emulation decode should propagate
An exception might occur during decode (e.g., #PF during fetch).
Currently, the exception is ignored and emulation is performed.
Instead, emulation should be skipped and the fault should be injected.
Skipping instruction should report a failure in this case.
Signed-off-by: Nadav Amit
---
arch/x86/kvm/emulate.c |5 +
arch/x86/kvm/x86.c |8
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 05a562b..9a07dcd 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3869,6 +3869,11 @@ done:
if (ctxt->memopp && ctxt->memopp->type == OP_MEM && ctxt->rip_relative)
ctxt->memopp->addr.mem.ea += ctxt->_eip;
+ if (rc == X86EMUL_PROPAGATE_FAULT) {
+ ctxt->have_exception = true;
+ return EMULATION_OK;
+ }
+
return (rc != X86EMUL_CONTINUE) ? EMULATION_FAILED : EMULATION_OK;
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1171def..05fd3d7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4443,10 +4443,17 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
}
if (emulation_type & EMULTYPE_SKIP) {
+ if (ctxt->have_exception)
+ return EMULATE_FAIL;
kvm_rip_write(vcpu, ctxt->_eip);
return EMULATE_DONE;
}
+ if (ctxt->have_exception) {
+ writeback = false;
+ goto post;
+ }
+
if (retry_instruction(ctxt, cr2, emulation_type))
return EMULATE_DONE;
@@ -4470,6 +4477,7 @@ restart:
return handle_emulation_failure(vcpu);
}
+post:
if (ctxt->have_exception) {
inject_emulated_exception(vcpu);
r = EMULATE_DONE;
--
1.7.4.1
--
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
