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 <raywang@linux.vnet.ibm.com>]

On 9/26/2011 5:46 PM, Peter Maydell wrote:
> On 26 September 2011 07:22, Ray Wang<raywang@linux.vnet.ibm.com>  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
xianleiw@cn.ibm.com