[Qemu-devel] Re: [PATCH] pci: fix info pci with host bridge.

[Blue Swirl]

On Tue, Feb 9, 2010 at 5:02 AM, Isaku Yamahata <address@hidden> wrote:
> On Mon, Feb 08, 2010 at 07:23:34PM +0200, Blue Swirl wrote:
>> On Mon, Feb 8, 2010 at 12:37 PM, Michael S. Tsirkin <address@hidden> wrote:
>> > On Mon, Feb 08, 2010 at 03:40:38PM +0900, Isaku Yamahata wrote:
>> >> This patch fixes 525e05147d5a3bdc08caa422d108c1ef71b584b5.
>> >> pci host bridge doesn't have header type of bridge.
>> >> The check should be by header type, instead of pci class device.
>> >>
>> >> Cc: Blue Swirl <address@hidden>
>> >> Cc: "Michael S. Tsirkin" <address@hidden>
>> >> Signed-off-by: Isaku Yamahata <address@hidden>
>> >
>> > So the effect of this will be that info pci won't report
>> > host bridge, right? IOW, it kind of reverts
>> > 525e05147d5a3bdc08caa422d108c1ef71b584b5, or am I
>> > missing something?
>>
>> Yes, it breaks info pci. PBM/APB does not use PCI_HEADER_TYPE_BRIDGE.
>
> Devices of pci host bridge class (!= PCI-PCI bridge) don't have
> header type PCI_HEADER_TYPE_BRIDGE (= 1), but PCI_HEADER_TYPE_NORMAL (= 0).
> In fact, i440fx pci host bridge is of PCI_HEADER_TYPE_NORMAL.
> You can see it in piix_pci.c.
> Registers of offset 0x10-0x3F in configuration space are used
> differently depending on header type.
> For example, PCI_HEADER_TYPE_NORMAL device don't have primary bus register,
> secondary bus register and so on. Those registers are used as BAR2.
> It doesn't make senses to show BAR2 as bus numbers.
>
> Having said that, I'm confused. So I downloaded
> "UltraSPARC IIi User's Manual", 805-0087.pdf.
> Page 301, table 19-12, section 19l.3.1 says that its header type is 0x0.
> (= PCI_HEADER_TYPE_Normal).
>
> cited from table 19-12.
>
> offset
> 0x10-0x27       Base Address
> 0x28-0x2F       Reserved
> 0x30-0x34       Expansion ROM
> 0x34-0x3b       Reserved
> 0x3e            MIN_GNT
> 0x3f            MAX_LAT
> ...
>
> So it doesn't make sense to access those registers as
> primary bus number, etc...
> However the manual also says that those shaded registers aren't implemented.
> Maybe it would make sense to use those unused/unimplemented registers
> for registers of header type 1 in PBM emulation.
> This is what you want to do, Right?
>
> Probably what you want in pci_info_device() is something like
> "if (type == PCI_HEADER_TYPE_BRIDGE ||
>    (vendorid == PCI_VENDOR_ID_SUN && deviceid == PCI_DEVICE_ID_SUN_SABRE))"
>
> This is ugly, so introducing device specific info callback?

Or maybe header should be of normal type and the secondary/subordinate
registers should not be used like they would be with a PCI-PCI bridge.
In that case maybe the bus number checks are not correct, or perhaps
OpenBIOS PCI code is wrong.