QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH for-5.2] spapr: Avoid some integer conversions in spapr_phb_realize()
@ 2020-07-28  9:14 Greg Kurz
  2020-07-28 11:51 ` Markus Armbruster
  2020-07-29  2:54 ` David Gibson
  0 siblings, 2 replies; 5+ messages in thread
From: Greg Kurz @ 2020-07-28  9:14 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, Markus Armbruster, qemu-devel

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);




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH for-5.2] spapr: Avoid some integer conversions in spapr_phb_realize()
  2020-07-28  9:14 [PATCH for-5.2] spapr: Avoid some integer conversions in spapr_phb_realize() Greg Kurz
@ 2020-07-28 11:51 ` Markus Armbruster
  2020-07-29  2:54 ` David Gibson
  1 sibling, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2020-07-28 11:51 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-devel, qemu-ppc, Markus Armbruster, David Gibson

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>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH for-5.2] spapr: Avoid some integer conversions in spapr_phb_realize()
  2020-07-28  9:14 [PATCH for-5.2] spapr: Avoid some integer conversions in spapr_phb_realize() Greg Kurz
  2020-07-28 11:51 ` Markus Armbruster
@ 2020-07-29  2:54 ` David Gibson
  2020-07-30 16:55   ` Greg Kurz
  1 sibling, 1 reply; 5+ messages in thread
From: David Gibson @ 2020-07-29  2:54 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-ppc, Markus Armbruster, qemu-devel


[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]

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);
> 
> 

-- 
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] 5+ messages in thread

* Re: [PATCH for-5.2] spapr: Avoid some integer conversions in spapr_phb_realize()
  2020-07-29  2:54 ` David Gibson
@ 2020-07-30 16:55   ` Greg Kurz
  2020-07-31  3:22     ` David Gibson
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kurz @ 2020-07-30 16:55 UTC (permalink / raw)
  To: David Gibson
  Cc: Daniel Henrique Barboza, qemu-ppc, Markus Armbruster, qemu-devel


[-- Attachment #1: Type: text/plain, Size: 2192 bytes --]

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);
> > 
> > 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH for-5.2] spapr: Avoid some integer conversions in spapr_phb_realize()
  2020-07-30 16:55   ` Greg Kurz
@ 2020-07-31  3:22     ` David Gibson
  0 siblings, 0 replies; 5+ messages in thread
From: David Gibson @ 2020-07-31  3:22 UTC (permalink / raw)
  To: Greg Kurz
  Cc: Daniel Henrique Barboza, qemu-ppc, Markus Armbruster, qemu-devel


[-- Attachment #1: Type: text/plain, Size: 1732 bytes --]

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.

-- 
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] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-28  9:14 [PATCH for-5.2] spapr: Avoid some integer conversions in spapr_phb_realize() Greg Kurz
2020-07-28 11:51 ` Markus Armbruster
2020-07-29  2:54 ` David Gibson
2020-07-30 16:55   ` Greg Kurz
2020-07-31  3:22     ` David Gibson

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git