Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 09/10] raven: fix PCI bus accesses with size > 1

[Andreas Färber]

Hi Artyom,

Am 16.03.2014 23:27, schrieb Artyom Tarasenko:
> Hi Andreas, Hervé,
> 
> this patch seems still be missing in master. Is it causing any problems?

It does not apply without the preceding patches. Here's my cherry-pick
result:

diff --cc hw/pci-host/prep.c
index 94fdffa,fed6c26..0000000
--- a/hw/pci-host/prep.c
+++ b/hw/pci-host/prep.c
@@@ -133,17 -219,17 +133,27 @@@ static void raven_pcihost_realizefn(Dev
          sysbus_init_irq(dev, &s->irq[i]);
      }

 -    qdev_init_gpio_in(d, raven_change_gpio, 1);
 -
      pci_bus_irqs(&s->pci_bus, prep_set_irq, prep_map_irq, s->irq,
PCI_NUM_PINS
);

++<<<<<<< HEAD
 +    memory_region_init_io(&h->conf_mem, OBJECT(h),
&pci_host_conf_be_ops, s,
 +                          "pci-conf-idx", 1);
 +    sysbus_add_io(dev, 0xcf8, &h->conf_mem);
 +    sysbus_init_ioports(&h->busdev, 0xcf8, 1);
 +
 +    memory_region_init_io(&h->data_mem, OBJECT(h),
&pci_host_data_be_ops, s,
 +                          "pci-conf-data", 1);
 +    sysbus_add_io(dev, 0xcfc, &h->data_mem);
 +    sysbus_init_ioports(&h->busdev, 0xcfc, 1);
++=======
+     memory_region_init_io(&h->conf_mem, OBJECT(h),
&pci_host_conf_le_ops, s,
+                           "pci-conf-idx", 4);
+     memory_region_add_subregion(&s->pci_io, 0xcf8, &h->conf_mem);
+
+     memory_region_init_io(&h->data_mem, OBJECT(h),
&pci_host_data_le_ops, s,
+                           "pci-conf-data", 4);
+     memory_region_add_subregion(&s->pci_io, 0xcfc, &h->data_mem);
++>>>>>>> 67472dc... raven: fix PCI bus accesses with size > 1

      memory_region_init_io(&h->mmcfg, OBJECT(s), &PPC_PCIIO_ops, s,
"pciio", 0x00400000);
      memory_region_add_subregion(address_space_mem, 0x80800000, &h->mmcfg);

I.e. we might change 1 -> 4 in the SysBus API, but would that work given
that endianness is being changed alongside?

If either of you could submit a version limited to bug fixes or explain
why the whole refactoring is needed as bug fix and provide a bisectable
version, I can certainly apply it for -rc1 if my test cases continue
working.

BTW another unresolved issue that's been discussed is whether we should
change the default CPU for -M prep. I've been open to doing so for 2.0
but would like some pointer that such a machine did exist rather than
just happens to work better with OpenBIOS.

Regards,
Andreas


> On Mon, Feb 10, 2014 at 11:46 PM, Artyom Tarasenko <address@hidden> wrote:
>> On Tue, Nov 5, 2013 at 12:09 AM, Hervé Poussineau <address@hidden> wrote:
>>> Signed-off-by: Hervé Poussineau <address@hidden>
>>
>> Without this patch PReP is broken really bad. Was going to submit the
>> same fix, and then found that the bug was already fixed 4 months ago.
>>
>> Hope it helps getting it closer to master:
>>
>> Tested-by: Artyom Tarasenko <address@hidden>
>>
>>> ---
>>>  hw/pci-host/prep.c |    8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
>>> index c11679a..4eabe31 100644
>>> --- a/hw/pci-host/prep.c
>>> +++ b/hw/pci-host/prep.c
>>> @@ -222,12 +222,12 @@ static void raven_pcihost_realizefn(DeviceState *d, 
>>> Error **errp)
>>>
>>>      pci_bus_irqs(&s->pci_bus, prep_set_irq, prep_map_irq, s->irq, 
>>> PCI_NUM_PINS);
>>>
>>> -    memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_be_ops, 
>>> s,
>>> -                          "pci-conf-idx", 1);
>>> +    memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_le_ops, 
>>> s,
>>> +                          "pci-conf-idx", 4);
>>>      memory_region_add_subregion(&s->pci_io, 0xcf8, &h->conf_mem);
>>>
>>> -    memory_region_init_io(&h->data_mem, OBJECT(h), &pci_host_data_be_ops, 
>>> s,
>>> -                          "pci-conf-data", 1);
>>> +    memory_region_init_io(&h->data_mem, OBJECT(h), &pci_host_data_le_ops, 
>>> s,
>>> +                          "pci-conf-data", 4);
>>>      memory_region_add_subregion(&s->pci_io, 0xcfc, &h->data_mem);
>>>
>>>      memory_region_init_io(&h->mmcfg, OBJECT(s), &PPC_PCIIO_ops, s, 
>>> "pciio", 0x00400000);