[Qemu-devel] Re: [PATCH V6 13/32] pci_host: consolidate pci config address access.

[Michael S. Tsirkin]

On Wed, Nov 04, 2009 at 04:17:46PM +0100, Aurelien Jarno wrote:
> On Wed, Nov 04, 2009 at 03:14:26PM +0900, Isaku Yamahata wrote:
> > On Tue, Nov 03, 2009 at 03:45:12PM +0200, Michael S. Tsirkin wrote:
> > > > --- a/hw/pci_host.c
> > > > +++ b/hw/pci_host.c
> > > > @@ -32,6 +32,114 @@ do { printf("pci_host_data: " fmt , ## 
> > > > __VA_ARGS__); } while (0)
> > > >  #define PCI_DPRINTF(fmt, ...)
> > > >  #endif
> > > >  
> > > > +static void pci_host_config_writel(void *opaque, target_phys_addr_t 
> > > > addr,
> > > > +                                   uint32_t val)
> > > > +{
> > > > +    PCIHostState *s = opaque;
> > > > +
> > > > +#ifdef TARGET_WORDS_BIGENDIAN
> > > > +    val = bswap32(val);
> > > > +#endif
> > > 
> > > I know you just copied it, but isn't this just
> > >   val = le32_to_cpu(val);
> > > 
> > > ?
> > 
> > Makes sense.
>  
> The original code is actually wrong, but le32_to_cpu(val), will break on
> big endian hosts.
> 
> The fact is that QEMU doesn't emulate byteswap on buses. Hopefully on all
> big endian machines we emulate, the PCI bus is always connected backward,
> so we can simply do the byteswap depending on TARGET_WORDS_BIGENDIAN.
> 
> -- 
> Aurelien Jarno                                GPG: 1024D/F1BCDB73
> address@hidden                 http://www.aurel32.net

Are you speaking about bit endian hosts with little endian guests?
Ugh ... my head hurts. bswap32 is evil because there's no way to
figure out what is converted to what. big to little? guest to host?


-- 
MST