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