Re: [Qemu-devel] [PATCH] PPC: fix PCI configuration space MemoryRegions for grackle/uninorth

[Alexander Graf]

On 08.11.2013, at 23:18, Mark Cave-Ayland <address@hidden> wrote:

> On 08/11/13 03:20, Alexander Graf wrote:
> 
>> On 11.10.2013, at 12:53, Mark Cave-Ayland<address@hidden>  wrote:
>> 
>>> OpenBIOS prior to SVN r1225 had a horrible bug when accessing PCI
>>> configuration space for PPC Mac architectures - instead of writing the PCI
>>> configuration data value to the data register address, it would instead 
>>> write
>>> it to the data register address plus the PCI configuration address.
>>> 
>>> For this reason, the MemoryRegions for the PCI data register for
>>> grackle/uninorth were extremely large in order to accomodate the entire PCI
>>> configuration space being accessed during OpenBIOS PCI bus enumeration. Now
>>> that the OpenBIOS images have been updated, reduce the MemoryRegion sizes 
>>> down
>>> to a single 32-bit register as intended.
>>> 
>>> Signed-off-by: Mark Cave-Ayland<address@hidden>
>>> CC: Hervé Poussineau<address@hidden>
>>> CC: Andreas Färber<address@hidden>
>>> CC: Alexander Graf<address@hidden>
>> 
>> With this patch applied, mac99 emulation seems to break:
>> 
>>   
>> http://award.ath.cx/results/288-alex/x86/kvm.qemu-git-tcg.ppc-debian.mac99-G4.etch.e1000.reboot/debug/serial-vm1.log
>> 
>> 
>> Alex
> 
> Hi Alex,
> 
> Thanks for the heads-up - with the information above I was able to reproduce 
> this fairly easily. I had look at some of the uninorth drivers, and while 
> it's not particularly apparent from Linux that the PCI configuration data is 
> accessed via MMIO rather than ioport access, FreeBSD seems to suggest that 
> this is the case: 
> http://code.google.com/p/freebsd-head/source/browse/sys/powerpc/powermac/uninorth.c?spec=svnc6989e24706228678e454517dad4ad465a36e556&r=c6989e24706228678e454517dad4ad465a36e556#274.
> 
> The key is that the QEMU uninorth host bridge contains a hack to allow PCI 
> configuration mechanism #1 as used by OpenBIOS to work at all (see 
> unin_get_config_reg() in hw/pci-host/uninorth.c) which is why I didn't notice 
> it in my OpenBIOS boot tests; and in fact, the name of the uninorth PCI 
> configuration data MemoryRegions have a "-data" rather than a "-idx" suffix 
> is also a big clue.
> 
> Hence please find a revised version of the patch which is unaltered for 
> grackle, and only changes the MemoryRegion size for the PCI configuration 
> address register for uninorth so that the PCI configuration data space is 
> still accessible using MMIO. This resolves the issue for me, so if you're 
> satisifed that it works for you then I'll post a revised version to the list.

Hrm. Are you 100% sure this correct? This UniNorth is a real headache. The 
closest thing to a spec for it is the U4 spec which is generations ahead:

  http://www.datasheetarchive.com/dl/Datasheets-SW3/DSASW0048084.pdf

On that at page 109 you can see that you do indeed have a range of registers 
and a few fancy modes that can even be used to directly access config space 
registers without the usual index/data cycle.

Ben, do you have any more insight into how the original Uninorth worked?


Alex