[Xen-ia64-devel] Re: [patch] fix zero extending for mmio ld1/2/4 emulation in KVM

[Jes Sorensen <jes@xxxxxxx>]

Isaku Yamahata wrote:
> Hi Jes.
>
> Good catch.
> I thought similar fix is necessary for xen/ia64 and checked the code.
> It was fixed differently. I think the unnecessary divergence is undesirable.
> What do you think the following fix according?
Hi Isaku,

I tried this fix for KVM, but it didn't work since the data returned is
a full word (64 bit) and it seems to get crippled in the process, so
we cannot use your patch :(

Cheers,
Jes


> Only copy in the data actually requested by the instruction emulation
> and zero pad the destination register first. This avoids the problem
> where emulated mmio access got garbled data from ld2.acq instructions
> in the vga console driver.
>
> Signed-off-by: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
> Cc: Jes Sorensen <jes@xxxxxxx>
>
> diff --git a/arch/ia64/kvm/mmio.c b/arch/ia64/kvm/mmio.c
> index 351bf70..e6f194a 100644
> --- a/arch/ia64/kvm/mmio.c
> +++ b/arch/ia64/kvm/mmio.c
> @@ -154,6 +154,9 @@ static void mmio_access(struct kvm_vcpu *vcpu, u64 src_pa, 
> u64 *dest,
>         p->u.ioreq.dir = dir;
>         if (dir == IOREQ_WRITE)
>                 p->u.ioreq.data = *dest;
> +       else
> +               /* it's necessary to ensure zero extending */
> +               p->u.ioreq.data = 0;
>         p->u.ioreq.state = STATE_IOREQ_READY;
>         vmm_transition(vcpu);
>  
>
> On Tue, May 20, 2008 at 01:13:50PM +0200, Jes Sorensen wrote:
> > Matthew Chapman wrote:
> > > Jes,
> > >
> > > Glad you tracked it down.  Can I suggest rather than using memcpy, a
> > > more efficient way might be something like...
> > >
> > > #define ZERO_EXTEND(x,bits) ((x) & (~0UL >> (64-(bits))))
> > >
> > > *dest = ZERO_EXTEND(p->u.ioreq.data, 8*s);
> > Much nicer indeed!
> >
> > Here's a pretty version - Tony will you apply this one instead.
> >
> > Cheers,
> > Jes
> > Only copy in the data actually requested by the instruction emulation
> > and zero pad the destination register first. This avoids the problem
> > where emulated mmio access got garbled data from ld2.acq instructions
> > in the vga console driver.
> >
> > Signed-off-by: Jes Sorensen <jes@xxxxxxx>
> >
> > ---
> >  arch/ia64/kvm/mmio.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Index: linux-2.6.git/arch/ia64/kvm/mmio.c
> > ===================================================================
> > --- linux-2.6.git.orig/arch/ia64/kvm/mmio.c
> > +++ linux-2.6.git/arch/ia64/kvm/mmio.c
> > @@ -159,7 +159,8 @@
> >  
> >  	if (p->u.ioreq.state == STATE_IORESP_READY) {
> >                 if (dir == IOREQ_READ)
> > -                       *dest = p->u.ioreq.data;
> > +                       /* it's necessary to ensure zero extending */
> > +                       *dest = p->u.ioreq.data & (~0UL >> (64-(s*8)));
> >         } else
> >                 panic_vm(vcpu);
> >  out:
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel