qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Cédric Le Goater
Subject: Re: [Qemu-devel] [PATCH 20/21] ppc/pnv: Add model for Power8 PHB3 PCIe Host bridge
Date: Tue, 11 Apr 2017 18:03:12 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0

On 04/11/2017 05:05 AM, Benjamin Herrenschmidt wrote:
> 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.

Yeah that works. I ran a PowerNV guest under a Pseries BE guest under
a PowerNV P8 host. Network was running on the final guest so I suppose 
that's enough to check the PCI layer.  

> 
> 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.

No that's fine. I will add some helper routines to explain what is
happening. I have had to do some thing similar with the Aspeed flash
controller tests when using the Command mode.  

Thanks,

C.
 
>>> +        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.
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]