Re: [Qemu-devel] [PATCH] Changed the type of val argument of the function gt64120_writel() from uint32_t to uint64_t, so change the corresponding bswap32() to bswap64().

[Ray Wang]

On 9/26/2011 5:46 PM, Peter Maydell wrote:
> On 26 September 2011 07:22, Ray Wang<address@hidden>  wrote:
> > diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
> > index 1c34253..d0a31d2 100644
> > --- a/hw/gt64xxx.c
> > +++ b/hw/gt64xxx.c
> > @@ -312,7 +312,7 @@ static void gt64120_writel (void *opaque, 
> > target_phys_addr_t addr,
> >      uint32_t saddr;
> >
> >      if (!(s->regs[GT_CPU]&  0x00001000))
> > -        val = bswap32(val);
> > +        val = bswap64(val);
> >
> >      saddr = (addr&  0xfff)>>  2;
> >      switch (saddr) {
> > @@ -533,7 +533,7 @@ static void gt64120_writel (void *opaque, 
> > target_phys_addr_t addr,
> >          break;
> >      case GT_PCI0_CFGDATA:
> >          if (!(s->regs[GT_PCI0_CMD]&  1)&&  (s->pci.config_reg&  0x00fff800))
> > -            val = bswap32(val);
> > +            val = bswap64(val);
> >          if (s->pci.config_reg&  (1u<<  31))
> >              pci_data_write(s->pci.bus, s->pci.config_reg, val, 4);
> >          break;
> I don't know this device, but this looks a bit suspicious. If
> you do a bswap64() on the value for a 32-bit write this will put
> the data into the high 32 bits and zeros into the low half; then
> storing into s->regs[] will just write a zero (since regs[] is
> 32 bits), won't it? Changing only writel and not readl also looks
> odd.
   Yes, you're right. However, if a 64-bit value with effective high 32 
bits is considered as a 32-bit one, some significant information will be 
lost.
> What is the bug this change is trying to fix?
   It is always a potential bug to process a 64-bit value as 32-bit 
one, isn't it?
> -- PMM


--
Best Regards,
-------------------------------------------------
Ray Wang
Linux Technology Center, KVM China
IBM Corp., Beijing, China
address@hidden