[PATCH 1/2] KVM: X86: remove read buffer for mmio read

[PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Xiao Guangrong]

After commit f78146b0f9230765c6315b2e14f56112513389ad:

 KVM: Fix page-crossing MMIO

    MMIO that are split across a page boundary are currently broken - the
    code does not expect to be aborted by the exit to userspace for the
    first MMIO fragment.

    This patch fixes the problem by generalizing the current code for handling
    16-byte MMIOs to handle a number of "fragments", and changes the MMIO
    code to create those fragments.

    Signed-off-by: Avi Kivity <avi@redhat.com>
    Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Multiple MMIO reads can be merged into mmio_fragments, the read buffer is not
needed anymore

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/x86/include/asm/kvm_emulate.h |    1 -
 arch/x86/kvm/emulate.c             |   43 ++++-------------------------------
 arch/x86/kvm/x86.c                 |    2 -
 3 files changed, 5 insertions(+), 41 deletions(-)

diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index 1ac46c22..339d7c6 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -286,7 +286,6 @@ struct x86_emulate_ctxt {
 	struct operand *memopp;
 	struct fetch_cache fetch;
 	struct read_cache io_read;
-	struct read_cache mem_read;
 };

 /* Repeat String Operation Prefix */
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index f95d242..aa455da 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1128,33 +1128,6 @@ static void fetch_bit_operand(struct x86_emulate_ctxt *ctxt)
 	ctxt->src.val &= (ctxt->dst.bytes << 3) - 1;
 }

-static int read_emulated(struct x86_emulate_ctxt *ctxt,
-			 unsigned long addr, void *dest, unsigned size)
-{
-	int rc;
-	struct read_cache *mc = &ctxt->mem_read;
-
-	while (size) {
-		int n = min(size, 8u);
-		size -= n;
-		if (mc->pos < mc->end)
-			goto read_cached;
-
-		rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, n,
-					      &ctxt->exception);
-		if (rc != X86EMUL_CONTINUE)
-			return rc;
-		mc->end += n;
-
-	read_cached:
-		memcpy(dest, mc->data + mc->pos, n);
-		mc->pos += n;
-		dest += n;
-		addr += n;
-	}
-	return X86EMUL_CONTINUE;
-}
-
 static int segmented_read(struct x86_emulate_ctxt *ctxt,
 			  struct segmented_address addr,
 			  void *data,
@@ -1166,7 +1139,9 @@ static int segmented_read(struct x86_emulate_ctxt *ctxt,
 	rc = linearize(ctxt, addr, size, false, &linear);
 	if (rc != X86EMUL_CONTINUE)
 		return rc;
-	return read_emulated(ctxt, linear, data, size);
+
+	return ctxt->ops->read_emulated(ctxt, linear, data, size,
+					&ctxt->exception);
 }

 static int segmented_write(struct x86_emulate_ctxt *ctxt,
@@ -4122,8 +4097,6 @@ int x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
 	int rc = X86EMUL_CONTINUE;
 	int saved_dst_type = ctxt->dst.type;

-	ctxt->mem_read.pos = 0;
-
 	if (ctxt->mode == X86EMUL_MODE_PROT64 && (ctxt->d & No64)) {
 		rc = emulate_ud(ctxt);
 		goto done;
@@ -4364,15 +4337,9 @@ writeback:
 			 * or, if it is not used, after each 1024 iteration.
 			 */
 			if ((r->end != 0 || ctxt->regs[VCPU_REGS_RCX] & 0x3ff) &&
-			    (r->end == 0 || r->end != r->pos)) {
-				/*
-				 * Reset read cache. Usually happens before
-				 * decode, but since instruction is restarted
-				 * we have to do it here.
-				 */
-				ctxt->mem_read.end = 0;
+			    (r->end == 0 || r->end != r->pos))
 				return EMULATION_RESTART;
-			}
+
 			goto done; /* skip rip writeback */
 		}
 	}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a01a424..7445545 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4399,8 +4399,6 @@ static void init_decode_cache(struct x86_emulate_ctxt *ctxt,
 	ctxt->fetch.end = 0;
 	ctxt->io_read.pos = 0;
 	ctxt->io_read.end = 0;
-	ctxt->mem_read.pos = 0;
-	ctxt->mem_read.end = 0;
 }

 static void init_emulate_ctxt(struct kvm_vcpu *vcpu)
-- 
1.7.7.6

[PATCH 2/2] KVM: X86: introduce set_mmio_exit_info

[Xiao Guangrong]

Introduce set_mmio_exit_info to cleanup the common code

Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
 arch/x86/kvm/x86.c |   33 +++++++++++++++++----------------
 1 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7445545..7771f45 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3755,9 +3755,6 @@ static int read_exit_mmio(struct kvm_vcpu *vcpu, gpa_t gpa,
 static int write_exit_mmio(struct kvm_vcpu *vcpu, gpa_t gpa,
 			   void *val, int bytes)
 {
-	struct kvm_mmio_fragment *frag = &vcpu->mmio_fragments[0];
-
-	memcpy(vcpu->run->mmio.data, frag->data, frag->len);
 	return X86EMUL_CONTINUE;
 }

@@ -3825,6 +3822,20 @@ mmio:
 	return X86EMUL_CONTINUE;
 }

+static void set_mmio_exit_info(struct kvm_vcpu *vcpu,
+			       struct kvm_mmio_fragment *frag, bool write)
+{
+	struct kvm_run *run = vcpu->run;
+
+	run->exit_reason = KVM_EXIT_MMIO;
+	run->mmio.phys_addr = frag->gpa;
+	run->mmio.len = frag->len;
+	run->mmio.is_write = vcpu->mmio_is_write = write;
+
+	if (write)
+		memcpy(run->mmio.data, frag->data, frag->len);
+}
+
 int emulator_read_write(struct x86_emulate_ctxt *ctxt, unsigned long addr,
 			void *val, unsigned int bytes,
 			struct x86_exception *exception,
@@ -3864,14 +3875,10 @@ int emulator_read_write(struct x86_emulate_ctxt *ctxt, unsigned long addr,
 		return rc;

 	gpa = vcpu->mmio_fragments[0].gpa;
-
 	vcpu->mmio_needed = 1;
 	vcpu->mmio_cur_fragment = 0;

-	vcpu->run->mmio.len = vcpu->mmio_fragments[0].len;
-	vcpu->run->mmio.is_write = vcpu->mmio_is_write = ops->write;
-	vcpu->run->exit_reason = KVM_EXIT_MMIO;
-	vcpu->run->mmio.phys_addr = gpa;
+	set_mmio_exit_info(vcpu, &vcpu->mmio_fragments[0], ops->write);

 	return ops->read_write_exit_mmio(vcpu, gpa, val, bytes);
 }
@@ -5490,7 +5497,6 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
  */
 static int complete_mmio(struct kvm_vcpu *vcpu)
 {
-	struct kvm_run *run = vcpu->run;
 	struct kvm_mmio_fragment *frag;
 	int r;

@@ -5501,7 +5507,7 @@ static int complete_mmio(struct kvm_vcpu *vcpu)
 		/* Complete previous fragment */
 		frag = &vcpu->mmio_fragments[vcpu->mmio_cur_fragment++];
 		if (!vcpu->mmio_is_write)
-			memcpy(frag->data, run->mmio.data, frag->len);
+			memcpy(frag->data, vcpu->run->mmio.data, frag->len);
 		if (vcpu->mmio_cur_fragment == vcpu->mmio_nr_fragments) {
 			vcpu->mmio_needed = 0;
 			if (vcpu->mmio_is_write)
@@ -5511,12 +5517,7 @@ static int complete_mmio(struct kvm_vcpu *vcpu)
 		}
 		/* Initiate next fragment */
 		++frag;
-		run->exit_reason = KVM_EXIT_MMIO;
-		run->mmio.phys_addr = frag->gpa;
-		if (vcpu->mmio_is_write)
-			memcpy(run->mmio.data, frag->data, frag->len);
-		run->mmio.len = frag->len;
-		run->mmio.is_write = vcpu->mmio_is_write;
+		set_mmio_exit_info(vcpu, frag, vcpu->mmio_is_write);
 		return 0;

 	}
-- 
1.7.7.6

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Avi Kivity]

On 07/09/2012 12:02 PM, Xiao Guangrong wrote:
> After commit f78146b0f9230765c6315b2e14f56112513389ad:
> 
>  KVM: Fix page-crossing MMIO
> 
>     MMIO that are split across a page boundary are currently broken - the
>     code does not expect to be aborted by the exit to userspace for the
>     first MMIO fragment.
> 
>     This patch fixes the problem by generalizing the current code for handling
>     16-byte MMIOs to handle a number of "fragments", and changes the MMIO
>     code to create those fragments.
> 
>     Signed-off-by: Avi Kivity <avi@redhat.com>
>     Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> 
> Multiple MMIO reads can be merged into mmio_fragments, the read buffer is not
> needed anymore
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
>  arch/x86/include/asm/kvm_emulate.h |    1 -
>  arch/x86/kvm/emulate.c             |   43 ++++-------------------------------
>  arch/x86/kvm/x86.c                 |    2 -
>  3 files changed, 5 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
> index 1ac46c22..339d7c6 100644
> --- a/arch/x86/include/asm/kvm_emulate.h
> +++ b/arch/x86/include/asm/kvm_emulate.h
> @@ -286,7 +286,6 @@ struct x86_emulate_ctxt {
>  	struct operand *memopp;
>  	struct fetch_cache fetch;
>  	struct read_cache io_read;
> -	struct read_cache mem_read;
>  };
> 

Suppose we have a RMW instruction.  On the first entry to
x86_emulate_insn() we'll drop to userspace and perform the read, and the
seconds we'll read from the cache and complete the write.

Without the read cache this cannot work.

kvm-unit-tests.git has a test for xchg to mmio.  Does it still work?

I agree this code has to go, but it needs to be replaced by something.
Maybe a .valid flag in struct operand.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Gleb Natapov]

On Mon, Jul 09, 2012 at 02:15:37PM +0300, Avi Kivity wrote:
> On 07/09/2012 12:02 PM, Xiao Guangrong wrote:
> > After commit f78146b0f9230765c6315b2e14f56112513389ad:
> > 
> >  KVM: Fix page-crossing MMIO
> > 
> >     MMIO that are split across a page boundary are currently broken - the
> >     code does not expect to be aborted by the exit to userspace for the
> >     first MMIO fragment.
> > 
> >     This patch fixes the problem by generalizing the current code for handling
> >     16-byte MMIOs to handle a number of "fragments", and changes the MMIO
> >     code to create those fragments.
> > 
> >     Signed-off-by: Avi Kivity <avi@redhat.com>
> >     Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > 
> > Multiple MMIO reads can be merged into mmio_fragments, the read buffer is not
> > needed anymore
> > 
> > Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> > ---
> >  arch/x86/include/asm/kvm_emulate.h |    1 -
> >  arch/x86/kvm/emulate.c             |   43 ++++-------------------------------
> >  arch/x86/kvm/x86.c                 |    2 -
> >  3 files changed, 5 insertions(+), 41 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
> > index 1ac46c22..339d7c6 100644
> > --- a/arch/x86/include/asm/kvm_emulate.h
> > +++ b/arch/x86/include/asm/kvm_emulate.h
> > @@ -286,7 +286,6 @@ struct x86_emulate_ctxt {
> >  	struct operand *memopp;
> >  	struct fetch_cache fetch;
> >  	struct read_cache io_read;
> > -	struct read_cache mem_read;
> >  };
> > 
> 
> Suppose we have a RMW instruction.  On the first entry to
> x86_emulate_insn() we'll drop to userspace and perform the read, and the
> seconds we'll read from the cache and complete the write.
> 
> Without the read cache this cannot work.
> 
Cache is needed to emulate instructions that need more than one read
that can go to MMIO.

> kvm-unit-tests.git has a test for xchg to mmio.  Does it still work?
> 
> I agree this code has to go, but it needs to be replaced by something.
> Maybe a .valid flag in struct operand.
> 
Valid will not enough for that.

--
			Gleb.

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Avi Kivity]

On 07/09/2012 02:23 PM, Gleb Natapov wrote:
> 
>> kvm-unit-tests.git has a test for xchg to mmio.  Does it still work?
>> 
>> I agree this code has to go, but it needs to be replaced by something.
>> Maybe a .valid flag in struct operand.
>> 
> Valid will not enough for that.

If me make everything go through operands, why not?


-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Avi Kivity]

On 07/09/2012 02:23 PM, Gleb Natapov wrote:
> 
>> kvm-unit-tests.git has a test for xchg to mmio.  Does it still work?
>> 
>> I agree this code has to go, but it needs to be replaced by something.
>> Maybe a .valid flag in struct operand.
>> 
> Valid will not enough for that.

If we make everything go through operands, any reason why not?

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Xiao Guangrong]

On 07/09/2012 08:49 PM, Avi Kivity wrote:
> On 07/09/2012 02:23 PM, Gleb Natapov wrote:
>>
>>> kvm-unit-tests.git has a test for xchg to mmio.  Does it still work?
>>>
>>> I agree this code has to go, but it needs to be replaced by something.
>>> Maybe a .valid flag in struct operand.
>>>
>> Valid will not enough for that.
> 
> If we make everything go through operands, any reason why not?
> 

I noticed some instructions need to read ESP for many times (e.g, iret_real),
maybe .valid flag is not enough for this case if the stack is in MMIO, yes?

IIUC, I also noticed ESP is not reset back if it is emulated fail (mmio is needed).
If the stack located in mmio region, this kind of instruct will be broken, i know no
guest will use mmio as stack but SDM does not limit it, is it valid?

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Gleb Natapov]

On Mon, Jul 09, 2012 at 09:23:03PM +0800, Xiao Guangrong wrote:
> On 07/09/2012 08:49 PM, Avi Kivity wrote:
> > On 07/09/2012 02:23 PM, Gleb Natapov wrote:
> >>
> >>> kvm-unit-tests.git has a test for xchg to mmio.  Does it still work?
> >>>
> >>> I agree this code has to go, but it needs to be replaced by something.
> >>> Maybe a .valid flag in struct operand.
> >>>
> >> Valid will not enough for that.
> > 
> > If we make everything go through operands, any reason why not?
> > 
> 
> I noticed some instructions need to read ESP for many times (e.g, iret_real),
> maybe .valid flag is not enough for this case if the stack is in MMIO, yes?
> 
> IIUC, I also noticed ESP is not reset back if it is emulated fail (mmio is needed).
> If the stack located in mmio region, this kind of instruct will be broken, i know no
> guest will use mmio as stack but SDM does not limit it, is it valid?
> 
Good point about MMIO stack too.

--
			Gleb.

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Avi Kivity]

On 07/09/2012 04:23 PM, Xiao Guangrong wrote:
> On 07/09/2012 08:49 PM, Avi Kivity wrote:
>> On 07/09/2012 02:23 PM, Gleb Natapov wrote:
>>>
>>>> kvm-unit-tests.git has a test for xchg to mmio.  Does it still work?
>>>>
>>>> I agree this code has to go, but it needs to be replaced by something.
>>>> Maybe a .valid flag in struct operand.
>>>>
>>> Valid will not enough for that.
>> 
>> If we make everything go through operands, any reason why not?
>> 
> 
> I noticed some instructions need to read ESP for many times (e.g, iret_real),
> maybe .valid flag is not enough for this case if the stack is in MMIO, yes?

Good catch.  We either have to fix it or to restrict stack operations to
regular memory (->read_std).

> IIUC, I also noticed ESP is not reset back if it is emulated fail (mmio is needed).
> If the stack located in mmio region, this kind of instruct will be broken, i know no
> guest will use mmio as stack but SDM does not limit it, is it valid?

Stack in mmio (or task switch in mmio) is architecturally valid.  We
don't have to support it if no guests do it.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Gleb Natapov]

On Mon, Jul 09, 2012 at 04:34:50PM +0300, Avi Kivity wrote:
> On 07/09/2012 04:23 PM, Xiao Guangrong wrote:
> > On 07/09/2012 08:49 PM, Avi Kivity wrote:
> >> On 07/09/2012 02:23 PM, Gleb Natapov wrote:
> >>>
> >>>> kvm-unit-tests.git has a test for xchg to mmio.  Does it still work?
> >>>>
> >>>> I agree this code has to go, but it needs to be replaced by something.
> >>>> Maybe a .valid flag in struct operand.
> >>>>
> >>> Valid will not enough for that.
> >> 
> >> If we make everything go through operands, any reason why not?
> >> 
> > 
> > I noticed some instructions need to read ESP for many times (e.g, iret_real),
> > maybe .valid flag is not enough for this case if the stack is in MMIO, yes?
> 
> Good catch.  We either have to fix it or to restrict stack operations to
> regular memory (->read_std).
> 
> > IIUC, I also noticed ESP is not reset back if it is emulated fail (mmio is needed).
> > If the stack located in mmio region, this kind of instruct will be broken, i know no
> > guest will use mmio as stack but SDM does not limit it, is it valid?
> 
> Stack in mmio (or task switch in mmio) is architecturally valid.  We
> don't have to support it if no guests do it.
> 
But the code is already here, why drop it?

--
			Gleb.

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Avi Kivity]

On 07/10/2012 01:36 PM, Gleb Natapov wrote:
> On Mon, Jul 09, 2012 at 04:34:50PM +0300, Avi Kivity wrote:
> > On 07/09/2012 04:23 PM, Xiao Guangrong wrote:
> > > On 07/09/2012 08:49 PM, Avi Kivity wrote:
> > >> On 07/09/2012 02:23 PM, Gleb Natapov wrote:
> > >>>
> > >>>> kvm-unit-tests.git has a test for xchg to mmio.  Does it still work?
> > >>>>
> > >>>> I agree this code has to go, but it needs to be replaced by something.
> > >>>> Maybe a .valid flag in struct operand.
> > >>>>
> > >>> Valid will not enough for that.
> > >> 
> > >> If we make everything go through operands, any reason why not?
> > >> 
> > > 
> > > I noticed some instructions need to read ESP for many times (e.g, iret_real),
> > > maybe .valid flag is not enough for this case if the stack is in MMIO, yes?
> > 
> > Good catch.  We either have to fix it or to restrict stack operations to
> > regular memory (->read_std).
> > 
> > > IIUC, I also noticed ESP is not reset back if it is emulated fail (mmio is needed).
> > > If the stack located in mmio region, this kind of instruct will be broken, i know no
> > > guest will use mmio as stack but SDM does not limit it, is it valid?
> > 
> > Stack in mmio (or task switch in mmio) is architecturally valid.  We
> > don't have to support it if no guests do it.
> > 
> But the code is already here, why drop it?

The read cache is not effective for multiple disjunct reads.  The
splitting into 8-byte groups is unneeded.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Gleb Natapov]

On Tue, Jul 10, 2012 at 01:45:15PM +0300, Avi Kivity wrote:
> On 07/10/2012 01:36 PM, Gleb Natapov wrote:
> > On Mon, Jul 09, 2012 at 04:34:50PM +0300, Avi Kivity wrote:
> > > On 07/09/2012 04:23 PM, Xiao Guangrong wrote:
> > > > On 07/09/2012 08:49 PM, Avi Kivity wrote:
> > > >> On 07/09/2012 02:23 PM, Gleb Natapov wrote:
> > > >>>
> > > >>>> kvm-unit-tests.git has a test for xchg to mmio.  Does it still work?
> > > >>>>
> > > >>>> I agree this code has to go, but it needs to be replaced by something.
> > > >>>> Maybe a .valid flag in struct operand.
> > > >>>>
> > > >>> Valid will not enough for that.
> > > >> 
> > > >> If we make everything go through operands, any reason why not?
> > > >> 
> > > > 
> > > > I noticed some instructions need to read ESP for many times (e.g, iret_real),
> > > > maybe .valid flag is not enough for this case if the stack is in MMIO, yes?
> > > 
> > > Good catch.  We either have to fix it or to restrict stack operations to
> > > regular memory (->read_std).
> > > 
> > > > IIUC, I also noticed ESP is not reset back if it is emulated fail (mmio is needed).
> > > > If the stack located in mmio region, this kind of instruct will be broken, i know no
> > > > guest will use mmio as stack but SDM does not limit it, is it valid?
> > > 
> > > Stack in mmio (or task switch in mmio) is architecturally valid.  We
> > > don't have to support it if no guests do it.
> > > 
> > But the code is already here, why drop it?
> 
> The read cache is not effective for multiple disjunct reads.
What do you mean?

>                                                                The
> splitting into 8-byte groups is unneeded.
> 
Agree. Easy to fix.

--
			Gleb.

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Avi Kivity]

On 07/10/2012 01:48 PM, Gleb Natapov wrote:
> > > > 
> > > But the code is already here, why drop it?
> > 
> > The read cache is not effective for multiple disjunct reads.
> What do you mean?

If an instruction reads from several sources in mmio, then the first
read will be flushed from the cache by the second read.  So if we need a
third read, we'll have to exit for the first again.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Gleb Natapov]

On Tue, Jul 10, 2012 at 03:50:39PM +0300, Avi Kivity wrote:
> On 07/10/2012 01:48 PM, Gleb Natapov wrote:
> > > > > 
> > > > But the code is already here, why drop it?
> > > 
> > > The read cache is not effective for multiple disjunct reads.
> > What do you mean?
> 
> If an instruction reads from several sources in mmio, then the first
> read will be flushed from the cache by the second read.  So if we need a
> third read, we'll have to exit for the first again.
> 
The cache is flashed only before instruction decode, never during
emulation.

--
			Gleb.

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Avi Kivity]

On 07/10/2012 04:01 PM, Gleb Natapov wrote:
> On Tue, Jul 10, 2012 at 03:50:39PM +0300, Avi Kivity wrote:
> > On 07/10/2012 01:48 PM, Gleb Natapov wrote:
> > > > > > 
> > > > > But the code is already here, why drop it?
> > > > 
> > > > The read cache is not effective for multiple disjunct reads.
> > > What do you mean?
> > 
> > If an instruction reads from several sources in mmio, then the first
> > read will be flushed from the cache by the second read.  So if we need a
> > third read, we'll have to exit for the first again.
> > 
> The cache is flashed only before instruction decode, never during
> emulation.

Oh, I misread the code.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH 1/2] KVM: X86: remove read buffer for mmio read

[Gleb Natapov]

On Mon, Jul 09, 2012 at 03:49:05PM +0300, Avi Kivity wrote:
> On 07/09/2012 02:23 PM, Gleb Natapov wrote:
> > 
> >> kvm-unit-tests.git has a test for xchg to mmio.  Does it still work?
> >> 
> >> I agree this code has to go, but it needs to be replaced by something.
> >> Maybe a .valid flag in struct operand.
> >> 
> > Valid will not enough for that.
> 
> If we make everything go through operands, any reason why not?
> 
Ah I missed operand part. Thought about adding valid to x86.c mmio read
buffer. What about doing more complex things from MMIO, like task switch?

--
			Gleb.