Re: [Qemu-devel] [PATCH 03/12] e500: note possible bug with host bridge

[Michael Davidsaver]

On 11/21/2017 09:46 PM, David Gibson wrote:
> On Sun, Nov 19, 2017 at 09:24:11PM -0600, Michael Davidsaver wrote:
>> Signed-off-by: Michael Davidsaver 
> 
> I'm not sure if you're saying you think there is a hardware bug which
> we're faithfully emulating, or a software bug.

I mean that the emulation is incorrect in that it just sets
config[PCI_HEADER_TYPE]==PCI_HEADER_TYPE_BRIDGE but does none of the
other initialization of the base-pci-bridge class.

I specifically observed Linux being confused by the fact that the
primary, secondary, and subordinate bus registers don't work right
because they're actually the BAR2 address register.

Further, it seems odd that a host bridge would identify itself as a
pci-to-pci bridge.  The mpc8540 doesn't.  The mpc8544 docs aren't clear,
and I don't have a real one to test.  My inclination is to remove the
line changing PCI_HEADER_TYPE, but I'm hesitant about breaking things.
Especially since this doesn't trigger mis-behavior in Linux or RTEMS.

>> ---
>>  hw/pci-host/ppce500.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
>> index f2d108bc8a..0e2833bd98 100644
>> --- a/hw/pci-host/ppce500.c
>> +++ b/hw/pci-host/ppce500.c
>> @@ -424,6 +424,9 @@ static void e500_pcihost_bridge_realize(PCIDevice *d, 
>> Error **errp)
>>  MemoryRegion *ccsr_mr = sysbus_mmio_get_region(ccsr, 0);
>>  
>>  pci_config_set_class(d->config, PCI_CLASS_BRIDGE_PCI);
>> +/* BUG? identifies as PCI_HEADER_TYPE_BRIDGE but uses
>> + * standard device config read/write
>> + */
>>  d->config[PCI_HEADER_TYPE] =
>>  (d->config[PCI_HEADER_TYPE] & PCI_HEADER_TYPE_MULTI_FUNCTION) |
>>  PCI_HEADER_TYPE_BRIDGE;
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 03/12] e500: note possible bug with host bridge

[David Gibson]

On Sun, Nov 19, 2017 at 09:24:11PM -0600, Michael Davidsaver wrote:
> Signed-off-by: Michael Davidsaver 

I'm not sure if you're saying you think there is a hardware bug which
we're faithfully emulating, or a software bug.

> ---
>  hw/pci-host/ppce500.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> index f2d108bc8a..0e2833bd98 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -424,6 +424,9 @@ static void e500_pcihost_bridge_realize(PCIDevice *d, 
> Error **errp)
>  MemoryRegion *ccsr_mr = sysbus_mmio_get_region(ccsr, 0);
>  
>  pci_config_set_class(d->config, PCI_CLASS_BRIDGE_PCI);
> +/* BUG? identifies as PCI_HEADER_TYPE_BRIDGE but uses
> + * standard device config read/write
> + */
>  d->config[PCI_HEADER_TYPE] =
>  (d->config[PCI_HEADER_TYPE] & PCI_HEADER_TYPE_MULTI_FUNCTION) |
>  PCI_HEADER_TYPE_BRIDGE;

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH 03/12] e500: note possible bug with host bridge

[Michael Davidsaver]

Signed-off-by: Michael Davidsaver 
---
 hw/pci-host/ppce500.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index f2d108bc8a..0e2833bd98 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -424,6 +424,9 @@ static void e500_pcihost_bridge_realize(PCIDevice *d, Error 
**errp)
 MemoryRegion *ccsr_mr = sysbus_mmio_get_region(ccsr, 0);
 
 pci_config_set_class(d->config, PCI_CLASS_BRIDGE_PCI);
+/* BUG? identifies as PCI_HEADER_TYPE_BRIDGE but uses
+ * standard device config read/write
+ */
 d->config[PCI_HEADER_TYPE] =
 (d->config[PCI_HEADER_TYPE] & PCI_HEADER_TYPE_MULTI_FUNCTION) |
 PCI_HEADER_TYPE_BRIDGE;
-- 
2.11.0