* [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 @ 2018-08-01 3:53 David Gibson 2018-08-01 3:53 ` [Qemu-devel] [PULL 1/2] hw/misc/macio: Fix device introspection problems in macio devices David Gibson ` (3 more replies) 0 siblings, 4 replies; 14+ messages in thread From: David Gibson @ 2018-08-01 3:53 UTC (permalink / raw) To: peter.maydell; +Cc: qemu-devel, qemu-ppc, groug, David Gibson The following changes since commit f7502360397d291be04bc040e9f96c92ff2d8030: Update version for v3.0.0-rc3 release (2018-07-31 19:30:17 +0100) are available in the Git repository at: git://github.com/dgibson/qemu.git tags/ppc-for-3.0-20180801 for you to fetch changes up to 6484ab3dffadc79020a71376010f517d60b81b83: sam460ex: Fix PCI interrupts with multiple devices (2018-08-01 11:01:38 +1000) ---------------------------------------------------------------- ppc patch queue for 2018-08-01 Here are a final couple of fixes for the 3.0 release. ---------------------------------------------------------------- BALATON Zoltan (1): sam460ex: Fix PCI interrupts with multiple devices Thomas Huth (1): hw/misc/macio: Fix device introspection problems in macio devices hw/misc/macio/cuda.c | 5 ++--- hw/misc/macio/macio.c | 24 ++++++++---------------- hw/misc/macio/pmu.c | 5 ++--- hw/ppc/ppc440_pcix.c | 21 ++++++++------------- hw/ppc/sam460ex.c | 6 ++---- 5 files changed, 22 insertions(+), 39 deletions(-) ^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PULL 1/2] hw/misc/macio: Fix device introspection problems in macio devices 2018-08-01 3:53 [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 David Gibson @ 2018-08-01 3:53 ` David Gibson 2018-08-01 3:53 ` [Qemu-devel] [PULL 2/2] sam460ex: Fix PCI interrupts with multiple devices David Gibson ` (2 subsequent siblings) 3 siblings, 0 replies; 14+ messages in thread From: David Gibson @ 2018-08-01 3:53 UTC (permalink / raw) To: peter.maydell; +Cc: qemu-devel, qemu-ppc, groug, Thomas Huth, David Gibson From: Thomas Huth <thuth@redhat.com> Valgrind reports an error when introspecting the macio devices, e.g.: echo "{'execute':'qmp_capabilities'} {'execute':'device-list-properties'," \ "'arguments':{'typename':'macio-newworld'}}" \ "{'execute': 'human-monitor-command', " \ "'arguments': {'command-line': 'info qtree'}}" | \ valgrind -q ppc64-softmmu/qemu-system-ppc64 -M none,accel=qtest -qmp stdio [...] ==30768== Invalid read of size 8 ==30768== at 0x5BC1EA: qdev_print (qdev-monitor.c:686) ==30768== by 0x5BC1EA: qbus_print (qdev-monitor.c:719) ==30768== by 0x43E458: handle_hmp_command (monitor.c:3446) [...] Use the new function sysbus_init_child_obj() to initialize the objects here, to get the reference counting of the objects right, so that they are cleaned up correctly when the parent gets removed. Signed-off-by: Thomas Huth <thuth@redhat.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/misc/macio/cuda.c | 5 ++--- hw/misc/macio/macio.c | 24 ++++++++---------------- hw/misc/macio/pmu.c | 5 ++--- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c index 9651ed9744..c4f7a2f39b 100644 --- a/hw/misc/macio/cuda.c +++ b/hw/misc/macio/cuda.c @@ -554,9 +554,8 @@ static void cuda_init(Object *obj) CUDAState *s = CUDA(obj); SysBusDevice *sbd = SYS_BUS_DEVICE(obj); - object_initialize(&s->mos6522_cuda, sizeof(s->mos6522_cuda), - TYPE_MOS6522_CUDA); - qdev_set_parent_bus(DEVICE(&s->mos6522_cuda), sysbus_get_default()); + sysbus_init_child_obj(obj, "mos6522-cuda", &s->mos6522_cuda, + sizeof(s->mos6522_cuda), TYPE_MOS6522_CUDA); memory_region_init_io(&s->mem, obj, &mos6522_cuda_ops, s, "cuda", 0x2000); sysbus_init_mmio(sbd, &s->mem); diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c index d135e3bc2b..52aa3775f4 100644 --- a/hw/misc/macio/macio.c +++ b/hw/misc/macio/macio.c @@ -209,14 +209,11 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp) static void macio_init_ide(MacIOState *s, MACIOIDEState *ide, size_t ide_size, int index) { - gchar *name; + gchar *name = g_strdup_printf("ide[%i]", index); - object_initialize(ide, ide_size, TYPE_MACIO_IDE); - qdev_set_parent_bus(DEVICE(ide), sysbus_get_default()); + sysbus_init_child_obj(OBJECT(s), name, ide, ide_size, TYPE_MACIO_IDE); memory_region_add_subregion(&s->bar, 0x1f000 + ((index + 1) * 0x1000), &ide->mem); - name = g_strdup_printf("ide[%i]", index); - object_property_add_child(OBJECT(s), name, OBJECT(ide), NULL); g_free(name); } @@ -232,9 +229,7 @@ static void macio_oldworld_init(Object *obj) qdev_prop_allow_set_link_before_realize, 0, NULL); - object_initialize(&s->cuda, sizeof(s->cuda), TYPE_CUDA); - qdev_set_parent_bus(DEVICE(&s->cuda), sysbus_get_default()); - object_property_add_child(obj, "cuda", OBJECT(&s->cuda), NULL); + sysbus_init_child_obj(obj, "cuda", &s->cuda, sizeof(s->cuda), TYPE_CUDA); object_initialize(&os->nvram, sizeof(os->nvram), TYPE_MACIO_NVRAM); dev = DEVICE(&os->nvram); @@ -390,8 +385,8 @@ static void macio_newworld_init(Object *obj) qdev_prop_allow_set_link_before_realize, 0, NULL); - object_initialize(&ns->gpio, sizeof(ns->gpio), TYPE_MACIO_GPIO); - qdev_set_parent_bus(DEVICE(&ns->gpio), sysbus_get_default()); + sysbus_init_child_obj(obj, "gpio", &ns->gpio, sizeof(ns->gpio), + TYPE_MACIO_GPIO); for (i = 0; i < 2; i++) { macio_init_ide(s, &ns->ide[i], sizeof(ns->ide[i]), i); @@ -404,13 +399,10 @@ static void macio_instance_init(Object *obj) memory_region_init(&s->bar, obj, "macio", 0x80000); - object_initialize(&s->dbdma, sizeof(s->dbdma), TYPE_MAC_DBDMA); - qdev_set_parent_bus(DEVICE(&s->dbdma), sysbus_get_default()); - object_property_add_child(obj, "dbdma", OBJECT(&s->dbdma), NULL); + sysbus_init_child_obj(obj, "dbdma", &s->dbdma, sizeof(s->dbdma), + TYPE_MAC_DBDMA); - object_initialize(&s->escc, sizeof(s->escc), TYPE_ESCC); - qdev_set_parent_bus(DEVICE(&s->escc), sysbus_get_default()); - object_property_add_child(obj, "escc", OBJECT(&s->escc), NULL); + sysbus_init_child_obj(obj, "escc", &s->escc, sizeof(s->escc), TYPE_ESCC); } static const VMStateDescription vmstate_macio_oldworld = { diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c index e246b0fd41..d25344f888 100644 --- a/hw/misc/macio/pmu.c +++ b/hw/misc/macio/pmu.c @@ -770,9 +770,8 @@ static void pmu_init(Object *obj) qdev_prop_allow_set_link_before_realize, 0, NULL); - object_initialize(&s->mos6522_pmu, sizeof(s->mos6522_pmu), - TYPE_MOS6522_PMU); - qdev_set_parent_bus(DEVICE(&s->mos6522_pmu), sysbus_get_default()); + sysbus_init_child_obj(obj, "mos6522-pmu", &s->mos6522_pmu, + sizeof(s->mos6522_pmu), TYPE_MOS6522_PMU); memory_region_init_io(&s->mem, obj, &mos6522_pmu_ops, s, "via-pmu", 0x2000); -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [PULL 2/2] sam460ex: Fix PCI interrupts with multiple devices 2018-08-01 3:53 [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 David Gibson 2018-08-01 3:53 ` [Qemu-devel] [PULL 1/2] hw/misc/macio: Fix device introspection problems in macio devices David Gibson @ 2018-08-01 3:53 ` David Gibson 2018-08-01 10:11 ` [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 Peter Maydell 2018-08-06 10:30 ` [Qemu-devel] " Peter Maydell 3 siblings, 0 replies; 14+ messages in thread From: David Gibson @ 2018-08-01 3:53 UTC (permalink / raw) To: peter.maydell Cc: qemu-devel, qemu-ppc, groug, BALATON Zoltan, Sebastian Bauer, David Gibson From: BALATON Zoltan <balaton@eik.bme.hu> The four interrupts of the PCI bus are connected to the same UIC pin on the real Sam460ex. Evidence for this can be found in the UBoot source for the Sam460ex in the Sam460ex.c file where PCI_INTERRUPT_LINE is written. Change the ppc440_pcix model to behave more like this. This fixes the problem that can be observed when adding further PCI cards that got their interrupt rotated to other interrupts than PCI INT A. In particular, the bug was observed with an additional OHCI PCI card or an ES1370 sound device. Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Tested-by: Sebastian Bauer <mail@sebastianbauer.info> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/ppc/ppc440_pcix.c | 21 ++++++++------------- hw/ppc/sam460ex.c | 6 ++---- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c index d8af04b70f..64ed07afa6 100644 --- a/hw/ppc/ppc440_pcix.c +++ b/hw/ppc/ppc440_pcix.c @@ -57,7 +57,7 @@ typedef struct PPC440PCIXState { struct PLBOutMap pom[PPC440_PCIX_NR_POMS]; struct PLBInMap pim[PPC440_PCIX_NR_PIMS]; uint32_t sts; - qemu_irq irq[PCI_NUM_PINS]; + qemu_irq irq; AddressSpace bm_as; MemoryRegion bm; @@ -418,21 +418,20 @@ static void ppc440_pcix_reset(DeviceState *dev) * This may need further refactoring for other boards. */ static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int irq_num) { - int slot = pci_dev->devfn >> 3; - trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, slot); - return slot - 1; + trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, 0); + return 0; } static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level) { - qemu_irq *pci_irqs = opaque; + qemu_irq *pci_irq = opaque; trace_ppc440_pcix_set_irq(irq_num); if (irq_num < 0) { error_report("%s: PCI irq %d", __func__, irq_num); return; } - qemu_set_irq(pci_irqs[irq_num], level); + qemu_set_irq(*pci_irq, level); } static AddressSpace *ppc440_pcix_set_iommu(PCIBus *b, void *opaque, int devfn) @@ -471,19 +470,15 @@ static int ppc440_pcix_initfn(SysBusDevice *dev) { PPC440PCIXState *s; PCIHostState *h; - int i; h = PCI_HOST_BRIDGE(dev); s = PPC440_PCIX_HOST_BRIDGE(dev); - for (i = 0; i < ARRAY_SIZE(s->irq); i++) { - sysbus_init_irq(dev, &s->irq[i]); - } - + sysbus_init_irq(dev, &s->irq); memory_region_init(&s->busmem, OBJECT(dev), "pci bus memory", UINT64_MAX); h->bus = pci_register_root_bus(DEVICE(dev), NULL, ppc440_pcix_set_irq, - ppc440_pcix_map_irq, s->irq, &s->busmem, - get_system_io(), PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS); + ppc440_pcix_map_irq, &s->irq, &s->busmem, + get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS); s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge"); diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c index 0999efcc1e..9c77183006 100644 --- a/hw/ppc/sam460ex.c +++ b/hw/ppc/sam460ex.c @@ -515,10 +515,8 @@ static void sam460ex_init(MachineState *machine) /* PCI bus */ ppc460ex_pcie_init(env); - /* FIXME: is this correct? */ - dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000, - uic[1][0], uic[1][20], uic[1][21], uic[1][22], - NULL); + /* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */ + dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec00000, uic[1][0]); pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0"); if (!pci_bus) { error_report("couldn't create PCI controller!"); -- 2.17.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 2018-08-01 3:53 [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 David Gibson 2018-08-01 3:53 ` [Qemu-devel] [PULL 1/2] hw/misc/macio: Fix device introspection problems in macio devices David Gibson 2018-08-01 3:53 ` [Qemu-devel] [PULL 2/2] sam460ex: Fix PCI interrupts with multiple devices David Gibson @ 2018-08-01 10:11 ` Peter Maydell 2018-08-01 11:24 ` [Qemu-devel] [Qemu-ppc] " BALATON Zoltan 2018-08-06 10:30 ` [Qemu-devel] " Peter Maydell 3 siblings, 1 reply; 14+ messages in thread From: Peter Maydell @ 2018-08-01 10:11 UTC (permalink / raw) To: David Gibson; +Cc: QEMU Developers, qemu-ppc, Greg Kurz On 1 August 2018 at 04:53, David Gibson <david@gibson.dropbear.id.au> wrote: > The following changes since commit f7502360397d291be04bc040e9f96c92ff2d8030: > > Update version for v3.0.0-rc3 release (2018-07-31 19:30:17 +0100) > > are available in the Git repository at: > > git://github.com/dgibson/qemu.git tags/ppc-for-3.0-20180801 > > for you to fetch changes up to 6484ab3dffadc79020a71376010f517d60b81b83: > > sam460ex: Fix PCI interrupts with multiple devices (2018-08-01 11:01:38 +1000) > > ---------------------------------------------------------------- > ppc patch queue for 2018-08-01 > > Here are a final couple of fixes for the 3.0 release. So, we've just put out rc3, which in an ideal world is our final release candidate for 3.0. Are these bugs regressions from 2.12 ? thanks -- PMM ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801 2018-08-01 10:11 ` [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 Peter Maydell @ 2018-08-01 11:24 ` BALATON Zoltan 2018-08-01 13:04 ` Peter Maydell 0 siblings, 1 reply; 14+ messages in thread From: BALATON Zoltan @ 2018-08-01 11:24 UTC (permalink / raw) To: Peter Maydell; +Cc: David Gibson, qemu-ppc, QEMU Developers, Greg Kurz On Wed, 1 Aug 2018, Peter Maydell wrote: > On 1 August 2018 at 04:53, David Gibson <david@gibson.dropbear.id.au> wrote: >> The following changes since commit f7502360397d291be04bc040e9f96c92ff2d8030: >> >> Update version for v3.0.0-rc3 release (2018-07-31 19:30:17 +0100) >> >> are available in the Git repository at: >> >> git://github.com/dgibson/qemu.git tags/ppc-for-3.0-20180801 >> >> for you to fetch changes up to 6484ab3dffadc79020a71376010f517d60b81b83: >> >> sam460ex: Fix PCI interrupts with multiple devices (2018-08-01 11:01:38 +1000) >> >> ---------------------------------------------------------------- >> ppc patch queue for 2018-08-01 >> >> Here are a final couple of fixes for the 3.0 release. > > So, we've just put out rc3, which in an ideal world is our > final release candidate for 3.0. Are these bugs regressions from > 2.12 ? I don't know about the macio one but the sam460ex PCI interrupts were broken in 2.12 too. However it's a fix for a device only used in sam460ex which is now fixed by this patch so including it is not high risk for breaking anything else than sam460ex which is known to be not finished yet so I would not worry too much. But which is better? Releasing 3.0 with a known bug or including this fix without an rc4? In case of machines that are in QEMU for a long time, widely used and other infrastructure is dependent upon it's good to have strict rules to avoid regressions but for the newly added sam460ex which is really only expected to be first usable in 3.0 and not many people depend on it yet delaying a fix for a known problem until December just to follow a rule looks counterproductive. So I would say it's OK to include this fix but skip rc4 and if it makes sam460ex worse than it is now then at worst we would have a broken 3.0 that will need to be fixed in next release. But we have a good chance to release a better 3.0 as opposed to not include the fix and release a known broken 3.0 that surely will have to be fixed in next release. (This fix may improve sound and network support so it would be good to have it in 3.0. A lot of potential users can only use release and rc versions because they can't build QEMU from source so they will only get this fix in next release otherwise.) But if it's not possible to have this in 3.0 without rc4 and there won't be other changes needing an rc4 we can live with this not getting in 3.0 but due to the above it might delay this for the general public by a few months. Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801 2018-08-01 11:24 ` [Qemu-devel] [Qemu-ppc] " BALATON Zoltan @ 2018-08-01 13:04 ` Peter Maydell 2018-08-02 7:08 ` David Gibson 2018-08-05 15:38 ` BALATON Zoltan 0 siblings, 2 replies; 14+ messages in thread From: Peter Maydell @ 2018-08-01 13:04 UTC (permalink / raw) To: BALATON Zoltan; +Cc: David Gibson, qemu-ppc, QEMU Developers, Greg Kurz On 1 August 2018 at 12:24, BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Wed, 1 Aug 2018, Peter Maydell wrote: >> So, we've just put out rc3, which in an ideal world is our >> final release candidate for 3.0. Are these bugs regressions from >> 2.12 ? > > > I don't know about the macio one but the sam460ex PCI interrupts were broken > in 2.12 too. However it's a fix for a device only used in sam460ex which is > now fixed by this patch so including it is not high risk for breaking > anything else than sam460ex which is known to be not finished yet so I would > not worry too much. But which is better? Releasing 3.0 with a known bug or > including this fix without an rc4? The problem with continuing to delay 3.0 while we have known bugs is that bugs generally come in at an even rate, so we *always* have known bugs, and so "we found another bug, let's delay 3.0 again to put in a fix for it" is a recipe for never doing a release. That's why we gradually wind up the bar for "should this go in", from "any bug" to "regressions" to "really really serious showstopper regressions". We never do a final release without a last rc (it is too risky), so that is not an option. thanks -- PMM ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801 2018-08-01 13:04 ` Peter Maydell @ 2018-08-02 7:08 ` David Gibson 2018-08-02 9:16 ` Peter Maydell 2018-08-05 15:38 ` BALATON Zoltan 1 sibling, 1 reply; 14+ messages in thread From: David Gibson @ 2018-08-02 7:08 UTC (permalink / raw) To: Peter Maydell; +Cc: BALATON Zoltan, qemu-ppc, QEMU Developers, Greg Kurz [-- Attachment #1: Type: text/plain, Size: 2112 bytes --] On Wed, Aug 01, 2018 at 02:04:04PM +0100, Peter Maydell wrote: > On 1 August 2018 at 12:24, BALATON Zoltan <balaton@eik.bme.hu> wrote: > > On Wed, 1 Aug 2018, Peter Maydell wrote: > >> So, we've just put out rc3, which in an ideal world is our > >> final release candidate for 3.0. Are these bugs regressions from > >> 2.12 ? > > > > > > I don't know about the macio one but the sam460ex PCI interrupts were broken > > in 2.12 too. However it's a fix for a device only used in sam460ex which is > > now fixed by this patch so including it is not high risk for breaking > > anything else than sam460ex which is known to be not finished yet so I would > > not worry too much. But which is better? Releasing 3.0 with a known bug or > > including this fix without an rc4? > > The problem with continuing to delay 3.0 while we have known bugs > is that bugs generally come in at an even rate, so we *always* > have known bugs, and so "we found another bug, let's delay 3.0 > again to put in a fix for it" is a recipe for never doing a release. > That's why we gradually wind up the bar for "should this go in", > from "any bug" to "regressions" to "really really serious showstopper > regressions". > > We never do a final release without a last rc (it is too risky), > so that is not an option. Ugh, sorry about this. I made an off-by-one error in my mental calculation of whether it would be ok to include non-regression bugfixes aet this point. Also, I originally meant to send this before the last -rc, but stuff happened. So, the sam460 fix is, indeed, not a regression and I'm happy to punt it to 3.1. The macio fix, however, *is* a regression from 2.12. Whether it's severe enough to warrant another -rc, I'm not sure. It is a bad pointer access which is, well, bad. It doesn't seem to bite obviously, needing valgrind to pick it up, but possibly that's just luck. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801 2018-08-02 7:08 ` David Gibson @ 2018-08-02 9:16 ` Peter Maydell 2018-08-02 14:07 ` David Gibson 0 siblings, 1 reply; 14+ messages in thread From: Peter Maydell @ 2018-08-02 9:16 UTC (permalink / raw) To: David Gibson; +Cc: BALATON Zoltan, qemu-ppc, QEMU Developers, Greg Kurz On 2 August 2018 at 08:08, David Gibson <david@gibson.dropbear.id.au> wrote: > The macio fix, however, *is* a regression from 2.12. Whether it's > severe enough to warrant another -rc, I'm not sure. It is a bad > pointer access which is, well, bad. It doesn't seem to bite > obviously, needing valgrind to pick it up, but possibly that's just > luck. I thought those introspection-bugs like the macio ones weren't regressions ? Certainly the devices have been leaking a refcount in their init methods for a long time. I don't view these as very important bugs, though, as they only manifest if you try to introspect fixed-always-present device/SoC objects, which in practice I don't think anybody does except as part of "test every device via introspection" test cases. thanks -- PMM ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801 2018-08-02 9:16 ` Peter Maydell @ 2018-08-02 14:07 ` David Gibson 2018-08-03 5:49 ` Thomas Huth 0 siblings, 1 reply; 14+ messages in thread From: David Gibson @ 2018-08-02 14:07 UTC (permalink / raw) To: Peter Maydell; +Cc: BALATON Zoltan, qemu-ppc, QEMU Developers, Greg Kurz [-- Attachment #1: Type: text/plain, Size: 1253 bytes --] On Thu, Aug 02, 2018 at 10:16:32AM +0100, Peter Maydell wrote: > On 2 August 2018 at 08:08, David Gibson <david@gibson.dropbear.id.au> wrote: > > The macio fix, however, *is* a regression from 2.12. Whether it's > > severe enough to warrant another -rc, I'm not sure. It is a bad > > pointer access which is, well, bad. It doesn't seem to bite > > obviously, needing valgrind to pick it up, but possibly that's just > > luck. > > I thought those introspection-bugs like the macio ones weren't > regressions ? Well, I ran Thomas's testcase on master and it generates several valgrind warnings, which don't appear on either 2.12 or master+the patch. > Certainly the devices have been leaking a refcount > in their init methods for a long time. I don't view these as > very important bugs, though, as they only manifest if you > try to introspect fixed-always-present device/SoC objects, > which in practice I don't think anybody does except as > part of "test every device via introspection" test cases. > > thanks > -- PMM > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801 2018-08-02 14:07 ` David Gibson @ 2018-08-03 5:49 ` Thomas Huth 2018-08-03 6:49 ` David Gibson 0 siblings, 1 reply; 14+ messages in thread From: Thomas Huth @ 2018-08-03 5:49 UTC (permalink / raw) To: David Gibson, Peter Maydell Cc: Greg Kurz, qemu-ppc, QEMU Developers, BALATON Zoltan [-- Attachment #1: Type: text/plain, Size: 1170 bytes --] On 08/02/2018 04:07 PM, David Gibson wrote: > On Thu, Aug 02, 2018 at 10:16:32AM +0100, Peter Maydell wrote: >> On 2 August 2018 at 08:08, David Gibson <david@gibson.dropbear.id.au> wrote: >>> The macio fix, however, *is* a regression from 2.12. Whether it's >>> severe enough to warrant another -rc, I'm not sure. It is a bad >>> pointer access which is, well, bad. It doesn't seem to bite >>> obviously, needing valgrind to pick it up, but possibly that's just >>> luck. >> >> I thought those introspection-bugs like the macio ones weren't >> regressions ? > > Well, I ran Thomas's testcase on master and it generates several > valgrind warnings, which don't appear on either 2.12 or master+the > patch. Maybe the macio bug is something new, but we had plenty of these introspetion bugs in the other code (mainly the ARM code) which were clearly there since a looong time already and nobody ever complained. So it seems quite unusual that upper layer tools / the users are using the introspection feature of QEMU. Thus I'd say this bug is not important enough to block the release. We could fix it in the stable branch instead. Thomas [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801 2018-08-03 5:49 ` Thomas Huth @ 2018-08-03 6:49 ` David Gibson 0 siblings, 0 replies; 14+ messages in thread From: David Gibson @ 2018-08-03 6:49 UTC (permalink / raw) To: Thomas Huth Cc: Peter Maydell, Greg Kurz, qemu-ppc, QEMU Developers, BALATON Zoltan [-- Attachment #1: Type: text/plain, Size: 1521 bytes --] On Fri, Aug 03, 2018 at 07:49:12AM +0200, Thomas Huth wrote: > On 08/02/2018 04:07 PM, David Gibson wrote: > > On Thu, Aug 02, 2018 at 10:16:32AM +0100, Peter Maydell wrote: > >> On 2 August 2018 at 08:08, David Gibson <david@gibson.dropbear.id.au> wrote: > >>> The macio fix, however, *is* a regression from 2.12. Whether it's > >>> severe enough to warrant another -rc, I'm not sure. It is a bad > >>> pointer access which is, well, bad. It doesn't seem to bite > >>> obviously, needing valgrind to pick it up, but possibly that's just > >>> luck. > >> > >> I thought those introspection-bugs like the macio ones weren't > >> regressions ? > > > > Well, I ran Thomas's testcase on master and it generates several > > valgrind warnings, which don't appear on either 2.12 or master+the > > patch. > > Maybe the macio bug is something new, but we had plenty of these > introspetion bugs in the other code (mainly the ARM code) which were > clearly there since a looong time already and nobody ever complained. So > it seems quite unusual that upper layer tools / the users are using the > introspection feature of QEMU. Thus I'd say this bug is not important > enough to block the release. We could fix it in the stable branch > instead. Understood, I'll punt the patch to my 3.1 staging tree. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801 2018-08-01 13:04 ` Peter Maydell 2018-08-02 7:08 ` David Gibson @ 2018-08-05 15:38 ` BALATON Zoltan 2018-08-06 8:39 ` Peter Maydell 1 sibling, 1 reply; 14+ messages in thread From: BALATON Zoltan @ 2018-08-05 15:38 UTC (permalink / raw) To: Peter Maydell; +Cc: David Gibson, qemu-ppc, QEMU Developers, Greg Kurz On Wed, 1 Aug 2018, Peter Maydell wrote: > On 1 August 2018 at 12:24, BALATON Zoltan <balaton@eik.bme.hu> wrote: >> On Wed, 1 Aug 2018, Peter Maydell wrote: >>> So, we've just put out rc3, which in an ideal world is our >>> final release candidate for 3.0. Are these bugs regressions from >>> 2.12 ? >> >> >> I don't know about the macio one but the sam460ex PCI interrupts were broken >> in 2.12 too. However it's a fix for a device only used in sam460ex which is >> now fixed by this patch so including it is not high risk for breaking >> anything else than sam460ex which is known to be not finished yet so I would >> not worry too much. But which is better? Releasing 3.0 with a known bug or >> including this fix without an rc4? > > The problem with continuing to delay 3.0 while we have known bugs > is that bugs generally come in at an even rate, so we *always* > have known bugs, and so "we found another bug, let's delay 3.0 > again to put in a fix for it" is a recipe for never doing a release. > That's why we gradually wind up the bar for "should this go in", > from "any bug" to "regressions" to "really really serious showstopper > regressions". > > We never do a final release without a last rc (it is too risky), > so that is not an option. Now that it looks like we'll have an rc4 due to other fixes can these be included as well despite not being regressions? These may not have been serious enough to fix when we wouldn't have rc4 otherwise but holding on to broken implementation just because they were also broken in the previous release does not make sense if we'll have another rc anyway. Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801 2018-08-05 15:38 ` BALATON Zoltan @ 2018-08-06 8:39 ` Peter Maydell 0 siblings, 0 replies; 14+ messages in thread From: Peter Maydell @ 2018-08-06 8:39 UTC (permalink / raw) To: BALATON Zoltan; +Cc: David Gibson, qemu-ppc, QEMU Developers, Greg Kurz On 5 August 2018 at 16:38, BALATON Zoltan <balaton@eik.bme.hu> wrote: > Now that it looks like we'll have an rc4 due to other fixes can these be > included as well despite not being regressions? These may not have been > serious enough to fix when we wouldn't have rc4 otherwise but holding on to > broken implementation just because they were also broken in the previous > release does not make sense if we'll have another rc anyway. I'm considering it, yes, but it increases risk, so it is not a completely trivial decision. (All changes later in the release cycle increase the risk of something unexpected breaking as a result of the change.) thanks -- PMM ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 2018-08-01 3:53 [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 David Gibson ` (2 preceding siblings ...) 2018-08-01 10:11 ` [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 Peter Maydell @ 2018-08-06 10:30 ` Peter Maydell 3 siblings, 0 replies; 14+ messages in thread From: Peter Maydell @ 2018-08-06 10:30 UTC (permalink / raw) To: David Gibson; +Cc: QEMU Developers, qemu-ppc, Greg Kurz On 1 August 2018 at 04:53, David Gibson <david@gibson.dropbear.id.au> wrote: > The following changes since commit f7502360397d291be04bc040e9f96c92ff2d8030: > > Update version for v3.0.0-rc3 release (2018-07-31 19:30:17 +0100) > > are available in the Git repository at: > > git://github.com/dgibson/qemu.git tags/ppc-for-3.0-20180801 > > for you to fetch changes up to 6484ab3dffadc79020a71376010f517d60b81b83: > > sam460ex: Fix PCI interrupts with multiple devices (2018-08-01 11:01:38 +1000) > > ---------------------------------------------------------------- > ppc patch queue for 2018-08-01 > > Here are a final couple of fixes for the 3.0 release. > > ---------------------------------------------------------------- > BALATON Zoltan (1): > sam460ex: Fix PCI interrupts with multiple devices > > Thomas Huth (1): > hw/misc/macio: Fix device introspection problems in macio devices Since we needed an rc4, I've applied this to master (the bug fixes are not strict regressions but they are limited in scope to particular machines). thanks -- PMM ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-08-06 10:30 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-01 3:53 [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 David Gibson 2018-08-01 3:53 ` [Qemu-devel] [PULL 1/2] hw/misc/macio: Fix device introspection problems in macio devices David Gibson 2018-08-01 3:53 ` [Qemu-devel] [PULL 2/2] sam460ex: Fix PCI interrupts with multiple devices David Gibson 2018-08-01 10:11 ` [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801 Peter Maydell 2018-08-01 11:24 ` [Qemu-devel] [Qemu-ppc] " BALATON Zoltan 2018-08-01 13:04 ` Peter Maydell 2018-08-02 7:08 ` David Gibson 2018-08-02 9:16 ` Peter Maydell 2018-08-02 14:07 ` David Gibson 2018-08-03 5:49 ` Thomas Huth 2018-08-03 6:49 ` David Gibson 2018-08-05 15:38 ` BALATON Zoltan 2018-08-06 8:39 ` Peter Maydell 2018-08-06 10:30 ` [Qemu-devel] " Peter Maydell
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.