On 04.01.2013, at 19:50, Scott Wood wrote:
> On 01/04/2013 04:24:59 AM, Alexander Graf wrote:
>> We already used to support the external proxy facility of FSL MPICs,
>> but only implemented it halfway correctly.
>> This patch adds support for
>> * dynamic enablement of the EPR facility
>> * interrupt acknowledgement only when the interrupt is delivered
>> This way the implementation now is closer to real hardware.
>> Signed-off-by: Alexander Graf <ag...@suse.de>
>> ---
>> hw/openpic.c | 20 ++++++++++++++++++++
>> target-ppc/Makefile.objs | 1 -
>> target-ppc/cpu.h | 2 ++
>> target-ppc/excp_helper.c | 4 ++++
>> target-ppc/helper.h | 1 -
>> target-ppc/mpic_helper.c | 35 -----------------------------------
>> target-ppc/translate_init.c | 7 +------
>> 7 files changed, 27 insertions(+), 43 deletions(-)
>> delete mode 100644 target-ppc/mpic_helper.c
>> diff --git a/hw/openpic.c b/hw/openpic.c
>> index e773d68..6447a47 100644
>> --- a/hw/openpic.c
>> +++ b/hw/openpic.c
>> @@ -131,6 +131,9 @@ static const int debug_openpic = 0;
>> #define VIR_GENERIC 0x00000000 /* Generic Vendor ID */
>> #define GCR_RESET 0x80000000
>> +#define GCR_MODE_PASS 0x00000000
>> +#define GCR_MODE_MIXED 0x20000000
>> +#define GCR_MODE_PROXY 0x60000000
>> #define TBCR_CI 0x80000000 /* count inhibit */
>> #define TCCR_TOG 0x80000000 /* toggles when decrement to zero */
>> @@ -233,6 +236,7 @@ typedef struct OpenPICState {
>> uint32_t ivpr_reset;
>> uint32_t idr_reset;
>> uint32_t brr1;
>> + uint32_t mpic_mode_mask;
>> /* Sub-regions */
>> MemoryRegion sub_io_mem[5];
>> @@ -667,6 +671,20 @@ static void openpic_gbl_write(void *opaque, hwaddr
>> addr, uint64_t val,
>> case 0x1020: /* GCR */
>> if (val & GCR_RESET) {
>> openpic_reset(&opp->busdev.qdev);
>> + } else if (opp->mpic_mode_mask) {
>> + CPUArchState *env;
>> + int mpic_proxy = 0;
>> +
>> + opp->gcr &= ~opp->mpic_mode_mask;
>> + opp->gcr |= val & opp->mpic_mode_mask;
>> +
>> + /* Set external proxy mode */
>> + if ((val & opp->mpic_mode_mask) == GCR_MODE_PROXY) {
>> + mpic_proxy = 1;
>> + }
>> + for (env = first_cpu; env != NULL; env = env->next_cpu) {
>> + env->mpic_proxy = mpic_proxy;
>> + }
>> }
>> break;
>> case 0x1080: /* VIR */
>> @@ -1407,6 +1425,8 @@ static int openpic_init(SysBusDevice *dev)
>> opp->irq_tim0 = FSL_MPIC_20_TMR_IRQ;
>> opp->irq_msi = FSL_MPIC_20_MSI_IRQ;
>> opp->brr1 = FSL_BRR1_IPID | FSL_BRR1_IPMJ | FSL_BRR1_IPMN;
>> + opp->mpic_mode_mask = GCR_MODE_PROXY;
>> +
>
> Technically this should only be available starting with v4.0, but I guess
> that can wait until we have more general support for a newer MPIC model.
> Should at least have a comment, though.
Phew. That one's tricky. I'll just add a comment for now, yeah.
>
>> msi_supported = true;
>> list = list_be;
>> diff --git a/target-ppc/Makefile.objs b/target-ppc/Makefile.objs
>> index 237a0ed..6c11ef8 100644
>> --- a/target-ppc/Makefile.objs
>> +++ b/target-ppc/Makefile.objs
>> @@ -9,4 +9,3 @@ obj-y += mmu_helper.o
>> obj-y += timebase_helper.o
>> obj-y += misc_helper.o
>> obj-y += mem_helper.o
>> -obj-y += mpic_helper.o
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index e88ebe0..0db06d6 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -1068,6 +1068,8 @@ struct CPUPPCState {
>> target_ulong ivpr_mask;
>> target_ulong hreset_vector;
>> hwaddr mpic_cpu_base;
>> + /* true when the external proxy facility mode is enabled */
>> + int mpic_proxy;
>> #endif
>
> bool?
IIRC we can't save/restore bools easily using savevm, and this one should be
saved/restored.
>
>> diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
>> index 41037a7..2b80164 100644
>> --- a/target-ppc/excp_helper.c
>> +++ b/target-ppc/excp_helper.c
>> @@ -178,6 +178,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int
>> excp_model, int excp)
>> if (lpes0 == 1) {
>> new_msr |= (target_ulong)MSR_HVB;
>> }
>> + if (env->mpic_proxy) {
>> + /* IACK the IRQ on delivery */
>> + env->spr[SPR_BOOKE_EPR] = ldl_phys(env->mpic_cpu_base + 0xA0);
>> + }
>
> Can we avoid the opencoded 0xA0? QEMU has too many open-coded magic numbers
> as is.
>
> I don't see where mpic_cpu_base is used other than for EPR; maybe just change
> it to mpic_iack? Or are there plans to do other things with it?
Not really. I'll change it accordingly.
Alex
