Re: [Xen-devel] [PATCH] QEMU/helper2.c: Fix multiply issue for int and uint types

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

> -----Original Message-----
> From: Stefano Stabellini [mailto:stefano.stabellini@xxxxxxxxxxxxx]
> Sent: Monday, August 20, 2012 8:18 PM
> To: Xu, Dongxiao
> Cc: xen-devel@xxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH] QEMU/helper2.c: Fix multiply issue for int 
> and
> uint types
> 
> On Mon, 20 Aug 2012, Xu, Dongxiao wrote:
> > Hi,
> >
> > The following patch fixes an issue of (uint * int) multiply in QEMU.
> > The bug phenomenon is that, it causes the Level 1 (L1) Xen boots more than
> 30 minutes on Level 0 (L0) Xen (The nested virtualization case).
> > Please help to review and pull.
> >
> > I saw the upstream QEMU also have a similar code piece, I will also send a
> patch there.
> >
> > Thanks,
> > Dongxiao
> >
> >
> > >From 442297c3f5073b11033e6af2338f571418eff024 Mon Sep 17 00:00:00
> > >2001
> > From: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> > Date: Mon, 20 Aug 2012 16:45:04 +0800
> > Subject: [PATCH] helper2: fix multiply issue for int and uint types
> >
> > If the two multiply operands are int and uint types separately, the
> > int type will be transformed to uint firstly, which is not the intent
> > in our code piece. The fix is to add (int) transform for the uint type
> > before the multiply.
> 
> well spotted!
> 
> 
> > This helps to fix the Xen hypevisor slow booting issue (boots more
> > than 30 minutes) on another Xen hypervisor (the nested virtualization
> > case).
> 
> If we do that we suddenly restrict the maximum req->size from UINT_MAX to
> INT_MAX.
> I would rather cast req->size to int64_t instead.

Yes, this is reasonable. I will update my patch.

Thanks,
Dongxiao

> 
> 
> > Signed-off-by: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> > ---
> >  i386-dm/helper2.c |   16 ++++++++--------
> >  1 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/i386-dm/helper2.c b/i386-dm/helper2.c index
> > c6d049c..2a66086 100644
> > --- a/i386-dm/helper2.c
> > +++ b/i386-dm/helper2.c
> > @@ -364,7 +364,7 @@ static void cpu_ioreq_pio(CPUState *env, ioreq_t
> *req)
> >              for (i = 0; i < req->count; i++) {
> >                  tmp = do_inp(env, req->addr, req->size);
> >                  write_physical((target_phys_addr_t) req->data
> > -                  + (sign * i * req->size),
> > +                  + (sign * i * (int)req->size),
> >                    req->size, &tmp);
> >              }
> >          }
> > @@ -376,7 +376,7 @@ static void cpu_ioreq_pio(CPUState *env, ioreq_t
> *req)
> >                  unsigned long tmp = 0;
> >
> >                  read_physical((target_phys_addr_t) req->data
> > -                  + (sign * i * req->size),
> > +                  + (sign * i * (int)req->size),
> >                    req->size, &tmp);
> >                  do_outp(env, req->addr, req->size, tmp);
> >              }
> > @@ -394,13 +394,13 @@ static void cpu_ioreq_move(CPUState *env,
> ioreq_t *req)
> >          if (req->dir == IOREQ_READ) {
> >              for (i = 0; i < req->count; i++) {
> >                  read_physical(req->addr
> > -                  + (sign * i * req->size),
> > +                  + (sign * i * (int)req->size),
> >                    req->size, &req->data);
> >              }
> >          } else if (req->dir == IOREQ_WRITE) {
> >              for (i = 0; i < req->count; i++) {
> >                  write_physical(req->addr
> > -                  + (sign * i * req->size),
> > +                  + (sign * i * (int)req->size),
> >                    req->size, &req->data);
> >              }
> >          }
> > @@ -410,19 +410,19 @@ static void cpu_ioreq_move(CPUState *env,
> ioreq_t *req)
> >          if (req->dir == IOREQ_READ) {
> >              for (i = 0; i < req->count; i++) {
> >                  read_physical(req->addr
> > -                  + (sign * i * req->size),
> > +                  + (sign * i * (int)req->size),
> >                    req->size, &tmp);
> >                  write_physical((target_phys_addr_t )req->data
> > -                  + (sign * i * req->size),
> > +                  + (sign * i * (int)req->size),
> >                    req->size, &tmp);
> >              }
> >          } else if (req->dir == IOREQ_WRITE) {
> >              for (i = 0; i < req->count; i++) {
> >                  read_physical((target_phys_addr_t) req->data
> > -                  + (sign * i * req->size),
> > +                  + (sign * i * (int)req->size),
> >                    req->size, &tmp);
> >                  write_physical(req->addr
> > -                  + (sign * i * req->size),
> > +                  + (sign * i * (int)req->size),
> >                    req->size, &tmp);
> >              }
> >          }
> > --
> > 1.7.1
> >
> >

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel