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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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.


> 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