Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge

[Mark Cave-Ayland]

On 25/09/17 09:11, Dr. David Alan Gilbert wrote:

> * Marcel Apfelbaum (address@hidden) wrote:
>> On 23/09/2017 11:23, Mark Cave-Ayland wrote:
>>> On 22/09/17 23:18, Laszlo Ersek wrote:
>>>
>>>> On 09/22/17 14:18, Mark Cave-Ayland wrote:
>>>>> Whilst the underlying PCI bridge implementation supports 32-bit PCI IO
>>>>> accesses, unfortunately they are truncated at the legacy 64K limit.
>>>>>
>>>>> Signed-off-by: Mark Cave-Ayland <address@hidden>
>>>>> ---
>>>>>   hw/pci/pci_bridge.c |    3 ++-
>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
>>>>> index 17feae5..a47d257 100644
>>>>> --- a/hw/pci/pci_bridge.c
>>>>> +++ b/hw/pci/pci_bridge.c
>>>>> @@ -379,7 +379,8 @@ void pci_bridge_initfn(PCIDevice *dev, const char 
>>>>> *typename)
>>>>>       sec_bus->address_space_mem = &br->address_space_mem;
>>>>>       memory_region_init(&br->address_space_mem, OBJECT(br), 
>>>>> "pci_bridge_pci", UINT64_MAX);
>>>>>       sec_bus->address_space_io = &br->address_space_io;
>>>>> -    memory_region_init(&br->address_space_io, OBJECT(br), 
>>>>> "pci_bridge_io", 65536);
>>>>> +    memory_region_init(&br->address_space_io, OBJECT(br), 
>>>>> "pci_bridge_io",
>>>>> +                       UINT32_MAX);
>>>>>       br->windows = pci_bridge_region_init(br);
>>>>>       QLIST_INIT(&sec_bus->child);
>>>>>       QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling);
>>>>>
>>>>
>>
>> Hi Mark,
>>
>>>> Based on the commit message, I assume this change is guest-visible. If
>>>> so, should it be made dependent on a compat property, so that it doesn't
>>>> cause problems with migration?
>>>
>>> In order to enable 32-bit IO accesses the PCI bridge needs to set bit 0
>>> in the IO_LIMIT and IO_BASE registers - this bit is read-only to guests,
>>> so unless a PCI bridge has this bit set then it's impossible for this
>>> change to be guest visible.
>>>
>>> I did a grep for PCI_IO_RANGE_TYPE_32 and didn't see any existing users
>>> (other than an upcoming patchset from me!), so this combined with the
>>> fact that without this patch the feature is broken makes me think that I
>>> am the first user and so existing guests won't have a problem.
>>>
>>
>> (adding Dave for his expertise)
>>
>> Do you know how the migration code will behave if it will have
>> a 65k address space on source and MAX UINT on destination?
>> (and the other way around for rolling back)
> 
> Hmm not sure; we don't migrate regions explicitly; just RAMBlocks
> and devices that back them.  If the change is visible in the IO
> addresses allocated to the PCI devices or in the config space then
> it might fail.

For reference here is the link to the sun4u patch I posted yesterday
that requires this fix if anyone else would like to test:
https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg07355.html.

Other than that are there any further objections to this patch?


ATB,

Mark.