[Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801

[Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801

[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(-)

[Qemu-devel] [PULL 1/2] hw/misc/macio: Fix device introspection problems in macio devices

[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

[Qemu-devel] [PULL 2/2] sam460ex: Fix PCI interrupts with multiple devices

[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

Re: [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801

[Peter Maydell]

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

Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801

[BALATON Zoltan]

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

Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801

[Peter Maydell]

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

Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801

[Peter Maydell]

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

Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801

[Thomas Huth]

[-- 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 --]

Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801

[David Gibson]

[-- 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 --]

Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801

[BALATON Zoltan]

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

Re: [Qemu-devel] [Qemu-ppc] [PULL 0/2] ppc-for-3.0 queue 20180801

[Peter Maydell]

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

Re: [Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801

[Peter Maydell]

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