Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/19] mpic: Unify numbering scheme

2012-12-11 Thread Alexander Graf


On 11.12.2012, at 00:34, Scott Wood scottw...@freescale.com wrote:

 On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
 MPIC interrupt numbers in Linux (device tree) and in QEMU are different,
 because QEMU takes the sparseness of the IRQ number space into account.
 Remove that cleverness and instead assume a flat number space. This makes
 the code easier to understand, because we are actually aligned with Linux
 on the view of our worlds.
 Signed-off-by: Alexander Graf ag...@suse.de
 ---
 hw/openpic.c  |  287 
 hw/ppc/e500.c |4 +-
 2 files changed, 43 insertions(+), 248 deletions(-)
 
 Thanks!
 
 This also helps accommodate Linux's (and probably other OSes') MPIC
 driver, which on boot will initialize all vectors, whether the actual
 interrupt is valid or not.  Currently QEMU rejects those accesses as
 illegal MMIO; we only survive it now because we don't yet have support for
 injecting a machine check on e500.
 
 diff --git a/hw/openpic.c b/hw/openpic.c
 index b30c853..0d858cc 100644
 --- a/hw/openpic.c
 +++ b/hw/openpic.c
 @@ -47,7 +47,7 @@
 #endif
 #define MAX_CPU15
 -#define MAX_IRQ   128
 +#define MAX_IRQ   256
 #define MAX_TMR 4
 #define VECTOR_BITS 8
 #define MAX_IPI 4
 @@ -82,32 +82,25 @@ enum {
 #define MPIC_MAX_CPU  1
 #define MPIC_MAX_EXT 12
 #define MPIC_MAX_INT 64
 -#define MPIC_MAX_MSG  4
 -#define MPIC_MAX_MSI  8
 -#define MPIC_MAX_TMR  MAX_TMR
 -#define MPIC_MAX_IPI  MAX_IPI
 -#define MPIC_MAX_IRQ (MPIC_MAX_EXT + MPIC_MAX_INT + MPIC_MAX_TMR + 
 MPIC_MAX_MSG + MPIC_MAX_MSI + (MPIC_MAX_IPI * MPIC_MAX_CPU))
 +#define MPIC_MAX_IRQ MAX_IRQ
 /* Interrupt definitions */
 -#define MPIC_EXT_IRQ  0
 -#define MPIC_INT_IRQ  (MPIC_EXT_IRQ + MPIC_MAX_EXT)
 -#define MPIC_TMR_IRQ  (MPIC_INT_IRQ + MPIC_MAX_INT)
 -#define MPIC_MSG_IRQ  (MPIC_TMR_IRQ + MPIC_MAX_TMR)
 -#define MPIC_MSI_IRQ  (MPIC_MSG_IRQ + MPIC_MAX_MSG)
 -#define MPIC_IPI_IRQ  (MPIC_MSI_IRQ + MPIC_MAX_MSI)
 +/* IRQs, accessible through the IRQ region */
 +#define MPIC_EXT_IRQ  0x00
 +#define MPIC_INT_IRQ  0x10
 +#define MPIC_MSG_IRQ  0xb0
 +#define MPIC_MSI_IRQ  0xe0
 
 Where are MPIC_EXT/INT/MSG/MSI_IRQ used now?  Note that these are
 specific to Freescale's MPIC.
 
 +/* These are available through separate regions, but
 +   for simplicity's sake mapped into the same number space */
 +#define MPIC_TMR_IRQ  0xf3
 +#define MPIC_IPI_IRQ  0xfb
 
 Please don't do this, or at least choose different numbers.  0xf3 is a
 valid MSI on p4080 (not to mention T4240 which goes beyond 256).

Ah, that's where I was wondering myself too. I copied the above logic from 
Linux, which maps tmr and ipi to max_irq-x. But I agree that it sounds off.

I'll rework this one again to distinguish between src and internal interrupt 
lines.


Alex




Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/19] mpic: Unify numbering scheme

2012-12-11 Thread Scott Wood

On 12/11/2012 02:14:42 AM, Alexander Graf wrote:



On 11.12.2012, at 00:34, Scott Wood scottw...@freescale.com wrote:

 On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
 +/* These are available through separate regions, but
 +   for simplicity's sake mapped into the same number space */
 +#define MPIC_TMR_IRQ  0xf3
 +#define MPIC_IPI_IRQ  0xfb

 Please don't do this, or at least choose different numbers.  0xf3  
is a

 valid MSI on p4080 (not to mention T4240 which goes beyond 256).

Ah, that's where I was wondering myself too. I copied the above logic  
from Linux, which maps tmr and ipi to max_irq-x. But I agree that it  
sounds off.


Yeah, Linux was buggy and has since been fixed (specifically, we turned  
on MPIC_LARGE_VECTORS so that max_irq is large enough to not conflict).


-Scott



Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/19] mpic: Unify numbering scheme

2012-12-10 Thread Scott Wood

On 12/10/2012 05:34:19 PM, Scott Wood wrote:

On 12/08/2012 07:44:25 AM, Alexander Graf wrote:

 /* Interrupt definitions */
-#define MPIC_EXT_IRQ  0
-#define MPIC_INT_IRQ  (MPIC_EXT_IRQ + MPIC_MAX_EXT)
-#define MPIC_TMR_IRQ  (MPIC_INT_IRQ + MPIC_MAX_INT)
-#define MPIC_MSG_IRQ  (MPIC_TMR_IRQ + MPIC_MAX_TMR)
-#define MPIC_MSI_IRQ  (MPIC_MSG_IRQ + MPIC_MAX_MSG)
-#define MPIC_IPI_IRQ  (MPIC_MSI_IRQ + MPIC_MAX_MSI)
+/* IRQs, accessible through the IRQ region */
+#define MPIC_EXT_IRQ  0x00
+#define MPIC_INT_IRQ  0x10
+#define MPIC_MSG_IRQ  0xb0
+#define MPIC_MSI_IRQ  0xe0


Where are MPIC_EXT/INT/MSG/MSI_IRQ used now?  Note that these are
specific to Freescale's MPIC.


+/* These are available through separate regions, but
+   for simplicity's sake mapped into the same number space */
+#define MPIC_TMR_IRQ  0xf3
+#define MPIC_IPI_IRQ  0xfb


Please don't do this, or at least choose different numbers.  0xf3 is a
valid MSI on p4080 (not to mention T4240 which goes beyond 256).

Again, what uses these defines?  Is it something later in the series?


OK, the TMR/IPI are used by existing code that this patch doesn't  
remove, but I think that's not the case with EXT/INT/MSG/MSI.


-Scott



Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/19] mpic: Unify numbering scheme

2012-12-10 Thread Scott Wood

On 12/08/2012 07:44:25 AM, Alexander Graf wrote:
MPIC interrupt numbers in Linux (device tree) and in QEMU are  
different,
because QEMU takes the sparseness of the IRQ number space into  
account.


Remove that cleverness and instead assume a flat number space. This  
makes
the code easier to understand, because we are actually aligned with  
Linux

on the view of our worlds.

Signed-off-by: Alexander Graf ag...@suse.de
---
 hw/openpic.c  |  287  


 hw/ppc/e500.c |4 +-
 2 files changed, 43 insertions(+), 248 deletions(-)


Thanks!

This also helps accommodate Linux's (and probably other OSes') MPIC
driver, which on boot will initialize all vectors, whether the actual
interrupt is valid or not.  Currently QEMU rejects those accesses as
illegal MMIO; we only survive it now because we don't yet have support  
for

injecting a machine check on e500.


diff --git a/hw/openpic.c b/hw/openpic.c
index b30c853..0d858cc 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -47,7 +47,7 @@
 #endif

 #define MAX_CPU15
-#define MAX_IRQ   128
+#define MAX_IRQ   256
 #define MAX_TMR 4
 #define VECTOR_BITS 8
 #define MAX_IPI 4
@@ -82,32 +82,25 @@ enum {
 #define MPIC_MAX_CPU  1
 #define MPIC_MAX_EXT 12
 #define MPIC_MAX_INT 64
-#define MPIC_MAX_MSG  4
-#define MPIC_MAX_MSI  8
-#define MPIC_MAX_TMR  MAX_TMR
-#define MPIC_MAX_IPI  MAX_IPI
-#define MPIC_MAX_IRQ (MPIC_MAX_EXT + MPIC_MAX_INT + MPIC_MAX_TMR  
+ MPIC_MAX_MSG + MPIC_MAX_MSI + (MPIC_MAX_IPI * MPIC_MAX_CPU))

+#define MPIC_MAX_IRQ MAX_IRQ

 /* Interrupt definitions */
-#define MPIC_EXT_IRQ  0
-#define MPIC_INT_IRQ  (MPIC_EXT_IRQ + MPIC_MAX_EXT)
-#define MPIC_TMR_IRQ  (MPIC_INT_IRQ + MPIC_MAX_INT)
-#define MPIC_MSG_IRQ  (MPIC_TMR_IRQ + MPIC_MAX_TMR)
-#define MPIC_MSI_IRQ  (MPIC_MSG_IRQ + MPIC_MAX_MSG)
-#define MPIC_IPI_IRQ  (MPIC_MSI_IRQ + MPIC_MAX_MSI)
+/* IRQs, accessible through the IRQ region */
+#define MPIC_EXT_IRQ  0x00
+#define MPIC_INT_IRQ  0x10
+#define MPIC_MSG_IRQ  0xb0
+#define MPIC_MSI_IRQ  0xe0


Where are MPIC_EXT/INT/MSG/MSI_IRQ used now?  Note that these are
specific to Freescale's MPIC.


+/* These are available through separate regions, but
+   for simplicity's sake mapped into the same number space */
+#define MPIC_TMR_IRQ  0xf3
+#define MPIC_IPI_IRQ  0xfb


Please don't do this, or at least choose different numbers.  0xf3 is a
valid MSI on p4080 (not to mention T4240 which goes beyond 256).

Again, what uses these defines?  Is it something later in the series?

-Scott