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

[Artyom Tarasenko]

Hi Andreas,

On Mon, Mar 17, 2014 at 8:59 PM, Andreas Färber <address@hidden> wrote:
>> 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:
>[...]
> I.e. we might change 1 -> 4 in the SysBus API, but would that work given
> that endianness is being changed alongside?

Yes, and that's the point of this patch. PCI configuration space is
little-endian. With 1 byte access size, no byte swapping happens, so
the bug is hidden.
But on 32- and 16- bit accesses the bytes are swapped.

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

No refactoring is necessary: only be->le and 1->4, and this is a pure
bugfix, which has no side effects because OHW seems to use 1 byte
accesses only.

> 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

That's fair. I don't have any preference here though, as long as the
necessary cpu can be selected via the command line.

> rather than just happens to work better with OpenBIOS.

Oh, there is a compatible version of OpenBIOS available?! Are the
binaries shared somewhere?
BTW is there any Linux distribution newer than Debian Woody available for PReP?

Artyom

>> 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);
>



-- 
Regards,
Artyom Tarasenko

linux/sparc and solaris/sparc under qemu blog:
http://tyom.blogspot.com/search/label/qemu