* [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge @ 2017-09-22 12:18 Mark Cave-Ayland 2017-09-22 21:21 ` Richard Henderson 2017-09-22 22:18 ` Laszlo Ersek 0 siblings, 2 replies; 14+ messages in thread From: Mark Cave-Ayland @ 2017-09-22 12:18 UTC (permalink / raw) To: qemu-devel, mst, marcel 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-ayland@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); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-09-22 12:18 [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge Mark Cave-Ayland @ 2017-09-22 21:21 ` Richard Henderson 2017-09-22 22:18 ` Laszlo Ersek 1 sibling, 0 replies; 14+ messages in thread From: Richard Henderson @ 2017-09-22 21:21 UTC (permalink / raw) To: Mark Cave-Ayland, qemu-devel, mst, marcel On 09/22/2017 07:18 AM, 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-ayland@ilande.co.uk> > --- > hw/pci/pci_bridge.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-09-22 12:18 [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge Mark Cave-Ayland 2017-09-22 21:21 ` Richard Henderson @ 2017-09-22 22:18 ` Laszlo Ersek 2017-09-23 8:23 ` Mark Cave-Ayland 1 sibling, 1 reply; 14+ messages in thread From: Laszlo Ersek @ 2017-09-22 22:18 UTC (permalink / raw) To: Mark Cave-Ayland, qemu-devel, mst, marcel 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-ayland@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); > 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? Thanks, Laszlo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-09-22 22:18 ` Laszlo Ersek @ 2017-09-23 8:23 ` Mark Cave-Ayland 2017-09-24 15:43 ` Marcel Apfelbaum 0 siblings, 1 reply; 14+ messages in thread From: Mark Cave-Ayland @ 2017-09-23 8:23 UTC (permalink / raw) To: Laszlo Ersek, qemu-devel, mst, marcel 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-ayland@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); >> > > 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. ATB, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-09-23 8:23 ` Mark Cave-Ayland @ 2017-09-24 15:43 ` Marcel Apfelbaum 2017-09-24 16:56 ` Mark Cave-Ayland 2017-09-25 8:11 ` Dr. David Alan Gilbert 0 siblings, 2 replies; 14+ messages in thread From: Marcel Apfelbaum @ 2017-09-24 15:43 UTC (permalink / raw) To: Mark Cave-Ayland, Laszlo Ersek, qemu-devel, mst, Dr. David Alan Gilbert 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-ayland@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) Thanks, Marcel > > ATB, > > Mark. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-09-24 15:43 ` Marcel Apfelbaum @ 2017-09-24 16:56 ` Mark Cave-Ayland 2017-09-25 8:11 ` Dr. David Alan Gilbert 1 sibling, 0 replies; 14+ messages in thread From: Mark Cave-Ayland @ 2017-09-24 16:56 UTC (permalink / raw) To: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, mst, Dr. David Alan Gilbert On 24/09/17 16:43, Marcel Apfelbaum wrote: > 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) Thanks Marcel. I should add that qemu-system-sparc64 isn't currently migratable anyhow, so if with my upcoming patch qemu-system-sparc64 is still the only user of PCI_IO_RANGE_TYPE_32 then that won't cause me any particular issue trying to migrate to earlier versions. Also in my local tests without the patch applied, the guest always panics during boot trying to access the IO space above 64K so I can't see there's a way that an older guest could boot in order to migrate forward either. ATB, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-09-24 15:43 ` Marcel Apfelbaum 2017-09-24 16:56 ` Mark Cave-Ayland @ 2017-09-25 8:11 ` Dr. David Alan Gilbert 2017-09-28 7:31 ` Mark Cave-Ayland 1 sibling, 1 reply; 14+ messages in thread From: Dr. David Alan Gilbert @ 2017-09-25 8:11 UTC (permalink / raw) To: Marcel Apfelbaum; +Cc: Mark Cave-Ayland, Laszlo Ersek, qemu-devel, mst * Marcel Apfelbaum (marcel@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-ayland@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. Dave > Thanks, > Marcel > > > > > ATB, > > > > Mark. > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-09-25 8:11 ` Dr. David Alan Gilbert @ 2017-09-28 7:31 ` Mark Cave-Ayland 2017-09-28 7:56 ` Laszlo Ersek 2017-09-28 9:19 ` Marcel Apfelbaum 0 siblings, 2 replies; 14+ messages in thread From: Mark Cave-Ayland @ 2017-09-28 7:31 UTC (permalink / raw) To: Dr. David Alan Gilbert, Marcel Apfelbaum; +Cc: Laszlo Ersek, qemu-devel, mst On 25/09/17 09:11, Dr. David Alan Gilbert wrote: > * Marcel Apfelbaum (marcel@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-ayland@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? ATB, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-09-28 7:31 ` Mark Cave-Ayland @ 2017-09-28 7:56 ` Laszlo Ersek 2017-10-01 21:44 ` Mark Cave-Ayland 2017-09-28 9:19 ` Marcel Apfelbaum 1 sibling, 1 reply; 14+ messages in thread From: Laszlo Ersek @ 2017-09-28 7:56 UTC (permalink / raw) To: Mark Cave-Ayland, Dr. David Alan Gilbert, Marcel Apfelbaum Cc: qemu-devel, mst On 09/28/17 09:31, Mark Cave-Ayland wrote: > On 25/09/17 09:11, Dr. David Alan Gilbert wrote: > >> * Marcel Apfelbaum (marcel@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-ayland@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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-09-28 7:56 ` Laszlo Ersek @ 2017-10-01 21:44 ` Mark Cave-Ayland 2017-10-11 6:35 ` Mark Cave-Ayland 0 siblings, 1 reply; 14+ messages in thread From: Mark Cave-Ayland @ 2017-10-01 21:44 UTC (permalink / raw) To: Laszlo Ersek, Dr. David Alan Gilbert, Marcel Apfelbaum; +Cc: qemu-devel, mst On 28/09/17 08:56, Laszlo Ersek wrote: > On 09/28/17 09:31, Mark Cave-Ayland wrote: >> On 25/09/17 09:11, Dr. David Alan Gilbert wrote: >> >>> * Marcel Apfelbaum (marcel@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-ayland@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.) Okay great! I guess I mis-read your query as being a NACK for the patch. ATB, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-10-01 21:44 ` Mark Cave-Ayland @ 2017-10-11 6:35 ` Mark Cave-Ayland 2017-10-19 9:38 ` Marcel Apfelbaum 0 siblings, 1 reply; 14+ messages in thread From: Mark Cave-Ayland @ 2017-10-11 6:35 UTC (permalink / raw) To: Laszlo Ersek, Dr. David Alan Gilbert, Marcel Apfelbaum; +Cc: qemu-devel, mst On 01/10/17 22:44, Mark Cave-Ayland wrote: > On 28/09/17 08:56, Laszlo Ersek wrote: > >> On 09/28/17 09:31, Mark Cave-Ayland wrote: >>> On 25/09/17 09:11, Dr. David Alan Gilbert wrote: >>> >>>> * Marcel Apfelbaum (marcel@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-ayland@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.) > > Okay great! I guess I mis-read your query as being a NACK for the patch. Hi Marcel, Did you manage to get any further with your testing? It appears that freeze is coming soon and my remaining changes for the sun4u machine are currently blocked on this patch :/ ATB, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-10-11 6:35 ` Mark Cave-Ayland @ 2017-10-19 9:38 ` Marcel Apfelbaum 0 siblings, 0 replies; 14+ messages in thread From: Marcel Apfelbaum @ 2017-10-19 9:38 UTC (permalink / raw) To: Mark Cave-Ayland, Laszlo Ersek, Dr. David Alan Gilbert; +Cc: qemu-devel, mst On 11/10/2017 9:35, Mark Cave-Ayland wrote: > On 01/10/17 22:44, Mark Cave-Ayland wrote: > >> On 28/09/17 08:56, Laszlo Ersek wrote: >> >>> On 09/28/17 09:31, Mark Cave-Ayland wrote: >>>> On 25/09/17 09:11, Dr. David Alan Gilbert wrote: >>>> >>>>> * Marcel Apfelbaum (marcel@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-ayland@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.) >> >> Okay great! I guess I mis-read your query as being a NACK for the patch. > > Hi Marcel, > > Did you manage to get any further with your testing? It appears that > freeze is coming soon and my remaining changes for the sun4u machine are > currently blocked on this patch :/ > Not yet, but I see your patch is already merged. I'll let you know if there will be any problems. Thanks, Marcel > > ATB, > > Mark. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-09-28 7:31 ` Mark Cave-Ayland 2017-09-28 7:56 ` Laszlo Ersek @ 2017-09-28 9:19 ` Marcel Apfelbaum 2017-10-01 21:49 ` Mark Cave-Ayland 1 sibling, 1 reply; 14+ messages in thread From: Marcel Apfelbaum @ 2017-09-28 9:19 UTC (permalink / raw) To: Mark Cave-Ayland, Dr. David Alan Gilbert; +Cc: Laszlo Ersek, qemu-devel, mst On 28/09/2017 10:31, Mark Cave-Ayland wrote: > On 25/09/17 09:11, Dr. David Alan Gilbert wrote: > >> * Marcel Apfelbaum (marcel@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-ayland@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? > 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. Thanks, Marcel > > ATB, > > Mark. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge 2017-09-28 9:19 ` Marcel Apfelbaum @ 2017-10-01 21:49 ` Mark Cave-Ayland 0 siblings, 0 replies; 14+ messages in thread From: Mark Cave-Ayland @ 2017-10-01 21:49 UTC (permalink / raw) To: Marcel Apfelbaum, Dr. David Alan Gilbert; +Cc: Laszlo Ersek, qemu-devel, mst 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 (marcel@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-ayland@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? >> > > 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-10-19 9:38 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-22 12:18 [Qemu-devel] [PATCH] pci: allow 32-bit PCI IO accesses to pass through the PCI bridge Mark Cave-Ayland 2017-09-22 21:21 ` Richard Henderson 2017-09-22 22:18 ` Laszlo Ersek 2017-09-23 8:23 ` Mark Cave-Ayland 2017-09-24 15:43 ` Marcel Apfelbaum 2017-09-24 16:56 ` Mark Cave-Ayland 2017-09-25 8:11 ` Dr. David Alan Gilbert 2017-09-28 7:31 ` Mark Cave-Ayland 2017-09-28 7:56 ` Laszlo Ersek 2017-10-01 21:44 ` Mark Cave-Ayland 2017-10-11 6:35 ` Mark Cave-Ayland 2017-10-19 9:38 ` Marcel Apfelbaum 2017-09-28 9:19 ` Marcel Apfelbaum 2017-10-01 21:49 ` Mark Cave-Ayland
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.