Re: [Qemu-ppc] [PATCH 20/21] ppc/pnv: Add model for Power8 PHB3 PCIe Host bridge

[Benjamin Herrenschmidt]

On Mon, 2017-04-10 at 18:14 +1000, David Gibson wrote:
> > +    switch (size) {
> > +    case 1:
> > +        break;
> > +    case 2:
> > +        val = bswap16(val);
> 
> An unconditional bswap() seems unlikely to be correct.  What's the
> purpose of thise?

Though it is :-) I *think* ... I did test with both LE and BE hosts I
believe but Cedric can double check.

>From memory it comes from the fact that the PHB register space is
declared to be big endian since 99% of it actually is.

However the CONFIG_DATA register used to issue config space accesses is
"special" and expect little endian accesses.

However because the whole region is marked BE, qemu will have done
the necessary swapping for BE, which thus needs to be undone for
that specific register unconditionally:

 - BE host will not swap the value, so we get the LE value written by
the guest which needs to be swapped to BE.

 - LE host will have swapped the value as if it was BE except it isn't
so we need to re-swap it.

IE. Another option would be to create a specific memory region just for
that one register but it's easier to just unconditionally swap.

> > +        break;
> > +    case 4:
> > +        val = bswap32(val);
> > +        break;
> 
> No 64-bit config registers?

No, they don't exist. (Which is also why an assert isn't a good
idea as other registers do support 64-bit accesses).

Ben.