On 09/28/17 09:31, Mark Cave-Ayland wrote: > On 25/09/17 09:11, Dr. David Alan Gilbert wrote: > >> * Marcel Apfelbaum (mar...@redhat.com) 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 <mark.cave-ayl...@ilande.co.uk> >>>>>> --- >>>>>> 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?
None from my side. (I didn't "object" to begin with :) , I was just curious about any possible migration impact.) Thanks! Laszlo
