On 11.12.2012, at 18:35, Scott Wood wrote: > On 12/11/2012 02:10:14 AM, Alexander Graf wrote: >> On 11.12.2012, at 01:36, Scott Wood <scottw...@freescale.com> wrote: >> > On 12/08/2012 07:44:39 AM, Alexander Graf wrote: >> >> The OpenPIC allows MSI access through shared MSI registers. Implement >> >> them for the MPC8544 MPIC, so we can support MSIs. >> >> Signed-off-by: Alexander Graf <ag...@suse.de> >> >> --- >> >> hw/openpic.c | 150 >> >> ++++++++++++++++++++++++++++++++++++++++++++++++++-------- >> >> 1 files changed, 130 insertions(+), 20 deletions(-) >> >> diff --git a/hw/openpic.c b/hw/openpic.c >> >> index f2f152f..f71d668 100644 >> >> --- a/hw/openpic.c >> >> +++ b/hw/openpic.c >> >> @@ -38,6 +38,7 @@ >> >> #include "pci.h" >> >> #include "openpic.h" >> >> #include "sysbus.h" >> >> +#include "msi.h" >> >> //#define DEBUG_OPENPIC >> >> @@ -52,6 +53,7 @@ >> >> #define MAX_TMR 4 >> >> #define VECTOR_BITS 8 >> >> #define MAX_IPI 4 >> >> +#define MAX_MSI 8 >> >> #define VID 0x03 /* MPIC version ID */ >> >> /* OpenPIC capability flags */ >> >> @@ -62,6 +64,8 @@ >> >> #define OPENPIC_GLB_REG_SIZE 0x10F0 >> >> #define OPENPIC_TMR_REG_START 0x10F0 >> >> #define OPENPIC_TMR_REG_SIZE 0x220 >> >> +#define OPENPIC_MSI_REG_START 0x1600 >> >> +#define OPENPIC_MSI_REG_SIZE 0x200 >> >> #define OPENPIC_SRC_REG_START 0x10000 >> >> #define OPENPIC_SRC_REG_SIZE (MAX_IRQ * 0x20) >> >> #define OPENPIC_CPU_REG_START 0x20000 >> >> @@ -126,6 +130,12 @@ >> >> #define IDR_P1_SHIFT 1 >> >> #define IDR_P0_SHIFT 0 >> >> +#define MSIIR_OFFSET 0x140 >> >> +#define MSIIR_SRS_SHIFT 29 >> >> +#define MSIIR_SRS_MASK (0x7 << MSIIR_SRS_SHIFT) >> >> +#define MSIIR_IBS_SHIFT 24 >> >> +#define MSIIR_IBS_MASK (0x1f << MSIIR_IBS_SHIFT) >> > >> > FWIW, if you want to model newer MPICs such as on p4080, they have >> > multiple banks of MSIs, so you may not want to hardcode one bank. >> The OpenPIC code was suffering a lot from attempts to generalize different >> implementations without implementing them. >> If we want to add support for p4080 MPICs later, we add a new model to the >> emulation and make the nr of msi banks a parameter, like the patch set does >> for all the other raven/mpc8544 differences. That way we don't get into the >> current mess of a halfway accurate emulation unless we really want it. > > So because the old code made a mess of it, we're saying "abstraction is bad" > in general?
No, because the old code messed up abstraction, I would like to move to a non-abstract level and then start abstracting at the correct layer. What that correct layer is is still up for discussion :). > All I'm saying is this should be done with a runtime data structure (of which > there could be more than one) rather than #defines. Yeah, and my argument was that this should be introduced as part of a new model. But if it makes you happy I can of course also make it generic right now, without a user, which means we can't even test whether the generalization works, which means we might screw it up again ;). Alex
