| Message ID | 159592765385.99837.12059368746532345109.stgit@bahia.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for-5.2] spapr: Avoid some integer conversions in spapr_phb_realize() | expand |
Greg Kurz <groug@kaod.org> writes: > Without this patch, the irq number gets converted uselessly from int > to int32_t, back and forth. > > This doesn't fix an actual issue, it's just to make the code neater. > > Suggested-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > > This is a follow-up to my previous "spapr: Simplify error handling in > spapr_phb_realize()" patch. Maybe worth squashing it there ? > --- > hw/ppc/spapr_pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 59441e2117f3..0a418f1e6711 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -1964,7 +1964,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > > /* Initialize the LSI table */ > for (i = 0; i < PCI_NUM_PINS; i++) { > - int32_t irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i; > + int irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i; > > if (smc->legacy_irq_allocation) { > irq = spapr_irq_findone(spapr, errp); Reviewed-by: Markus Armbruster <armbru@redhat.com>
On Tue, Jul 28, 2020 at 11:14:13AM +0200, Greg Kurz wrote: > Without this patch, the irq number gets converted uselessly from int > to int32_t, back and forth. > > This doesn't fix an actual issue, it's just to make the code neater. > > Suggested-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Greg Kurz <groug@kaod.org> Applied to ppc-for-5.2, thanks. > --- > > This is a follow-up to my previous "spapr: Simplify error handling in > spapr_phb_realize()" patch. Maybe worth squashing it there ? > --- > hw/ppc/spapr_pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 59441e2117f3..0a418f1e6711 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -1964,7 +1964,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > > /* Initialize the LSI table */ > for (i = 0; i < PCI_NUM_PINS; i++) { > - int32_t irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i; > + int irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i; > > if (smc->legacy_irq_allocation) { > irq = spapr_irq_findone(spapr, errp); > >
On Wed, 29 Jul 2020 12:54:41 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > On Tue, Jul 28, 2020 at 11:14:13AM +0200, Greg Kurz wrote: > > Without this patch, the irq number gets converted uselessly from int > > to int32_t, back and forth. > > > > This doesn't fix an actual issue, it's just to make the code neater. > > > > Suggested-by: Markus Armbruster <armbru@redhat.com> > > Signed-off-by: Greg Kurz <groug@kaod.org> > > Applied to ppc-for-5.2, thanks. > Daniel reported a crash that happens systematically on some systems that don't support KVM XIVE (aka. bostons) since the patch "spapr: Simplify error handling in spapr_phb_realize()" landed in the ppc-for-5.2 tree. The patch is good but it uncovered an issue we have in the KVM XIVE code in QEMU (basically we should ignore the absence of KVM XIVE device when claiming IRQ numbers). The fix is trivial but to avoid breaking bisect, it should rather go before the patch mentioned above. Also I want to consolidate the error handling a bit more so, in the meantime, for others to be able to use the ppc-for-5.2 branch, I suggest you simply drop: spapr: Simplify error handling in spapr_phb_realize() and the current patch as well since it's a follow-up. I'll send a new patchset later. > > --- > > > > This is a follow-up to my previous "spapr: Simplify error handling in > > spapr_phb_realize()" patch. Maybe worth squashing it there ? > > --- > > hw/ppc/spapr_pci.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > > index 59441e2117f3..0a418f1e6711 100644 > > --- a/hw/ppc/spapr_pci.c > > +++ b/hw/ppc/spapr_pci.c > > @@ -1964,7 +1964,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > > > > /* Initialize the LSI table */ > > for (i = 0; i < PCI_NUM_PINS; i++) { > > - int32_t irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i; > > + int irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i; > > > > if (smc->legacy_irq_allocation) { > > irq = spapr_irq_findone(spapr, errp); > > > > >
On Thu, Jul 30, 2020 at 06:55:18PM +0200, Greg Kurz wrote: > On Wed, 29 Jul 2020 12:54:41 +1000 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > On Tue, Jul 28, 2020 at 11:14:13AM +0200, Greg Kurz wrote: > > > Without this patch, the irq number gets converted uselessly from int > > > to int32_t, back and forth. > > > > > > This doesn't fix an actual issue, it's just to make the code neater. > > > > > > Suggested-by: Markus Armbruster <armbru@redhat.com> > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > > > Applied to ppc-for-5.2, thanks. > > > > Daniel reported a crash that happens systematically on some systems that > don't support KVM XIVE (aka. bostons) since the patch "spapr: Simplify > error handling in spapr_phb_realize()" landed in the ppc-for-5.2 tree. > > The patch is good but it uncovered an issue we have in the KVM XIVE code > in QEMU (basically we should ignore the absence of KVM XIVE device when > claiming IRQ numbers). > > The fix is trivial but to avoid breaking bisect, it should rather go > before the patch mentioned above. Also I want to consolidate the error > handling a bit more so, in the meantime, for others to be able to use > the ppc-for-5.2 branch, I suggest you simply drop: > > spapr: Simplify error handling in spapr_phb_realize() > > and the current patch as well since it's a follow-up. > > I'll send a new patchset later. Ok, done, I've removed both those patches from ppc-for-5.2, resend the new version whenever you're ready.
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 59441e2117f3..0a418f1e6711 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1964,7 +1964,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) /* Initialize the LSI table */ for (i = 0; i < PCI_NUM_PINS; i++) { - int32_t irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i; + int irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i; if (smc->legacy_irq_allocation) { irq = spapr_irq_findone(spapr, errp);
Without this patch, the irq number gets converted uselessly from int to int32_t, back and forth. This doesn't fix an actual issue, it's just to make the code neater. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Greg Kurz <groug@kaod.org> --- This is a follow-up to my previous "spapr: Simplify error handling in spapr_phb_realize()" patch. Maybe worth squashing it there ? --- hw/ppc/spapr_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)