On 28/09/17 10:19, Marcel Apfelbaum wrote: > On 28/09/2017 10:31, Mark Cave-Ayland wrote: >> On 25/09/17 09:11, Dr. David Alan Gilbert wrote: >> >>> * Marcel Apfelbaum ([email protected]) 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 <[email protected]> >>>>>>> --- >>>>>>> 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? >> > > Since we don't support 32-bit IO accesses (yet) in x86 > and the address space is not visible to guest, it should be OK. > I'll run a simple migration test to be sure, I'll let you > know if something goes wrong.
Fantastic! In that case I'll assume that no news is good news :) Do you know approximately how long it will be before the patch gets merged? I'm just trying to get a feel for when I can send the pull request for the sun4u changes which I'd like to do soon if possible given that it's such a large change. ATB, Mark.
