[Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Alexander Graf]

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 <agraf@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;
+
         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
 
     /* Those resources are used only during code translation */
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);
+        }
         goto store_next;
     case POWERPC_EXCP_ALIGN:     /* Alignment exception                      */
         if (lpes1 == 0) {
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index d2e9a55..83139d5 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -405,7 +405,6 @@ DEF_HELPER_2(store_40x_dbcr0, void, env, tl)
 DEF_HELPER_2(store_40x_sler, void, env, tl)
 DEF_HELPER_2(store_booke_tcr, void, env, tl)
 DEF_HELPER_2(store_booke_tsr, void, env, tl)
-DEF_HELPER_1(load_epr, tl, env)
 DEF_HELPER_3(store_ibatl, void, env, i32, tl)
 DEF_HELPER_3(store_ibatu, void, env, i32, tl)
 DEF_HELPER_3(store_dbatl, void, env, i32, tl)
diff --git a/target-ppc/mpic_helper.c b/target-ppc/mpic_helper.c
deleted file mode 100644
index 2c6a4d3..0000000
--- a/target-ppc/mpic_helper.c
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- *  PowerPC emulation helpers for QEMU.
- *
- *  Copyright (c) 2003-2007 Jocelyn Mayer
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see <http://www.gnu.org/licenses/>.
- */
-#include "cpu.h"
-#include "helper.h"
-
-/*****************************************************************************/
-/* SPR accesses */
-
-#if !defined(CONFIG_USER_ONLY)
-/*
- * This is an ugly helper for EPR, which is basically the same as accessing
- * the IACK (PIAC) register on the MPIC. Because we model the MPIC as a device
- * that can only talk to the CPU through MMIO, let's access it that way!
- */
-target_ulong helper_load_epr(CPUPPCState *env)
-{
-    return ldl_phys(env->mpic_cpu_base + 0xA0);
-}
-#endif
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 42ed748..e2eeb87 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -4493,11 +4493,6 @@ static void spr_read_mas73(void *opaque, int gprn, int sprn)
     tcg_temp_free(mas7);
 }
 
-static void spr_load_epr(void *opaque, int gprn, int sprn)
-{
-    gen_helper_load_epr(cpu_gpr[gprn], cpu_env);
-}
-
 #endif
 
 enum fsl_e500_version {
@@ -4656,7 +4651,7 @@ static void init_proc_e500 (CPUPPCState *env, int version)
                  0x00000000);
     spr_register(env, SPR_BOOKE_EPR, "EPR",
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_load_epr, SPR_NOACCESS,
+                 &spr_read_generic, SPR_NOACCESS,
                  0x00000000);
     /* XXX better abstract into Emb.xxx features */
     if (version == fsl_e5500) {
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Scott Wood]

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 <agraf@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.

>          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?

> 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?

-Scott

Re: [Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Alexander Graf]

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 <agraf@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

Re: [Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Scott Wood]

On 01/04/2013 12:54:51 PM, Alexander Graf wrote:
> 
> On 04.01.2013, at 19:50, Scott Wood wrote:
> 
> > On 01/04/2013 04:24:59 AM, Alexander Graf wrote:
> >>         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.

I see bool handlers in savevm.c.  I'm not sure how a non-fixed-size  
"int" is better.

BTW, docs/migration.txt says that savevm is the "legacy" approach.

-Scott

Re: [Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Anthony Liguori]

Hi,

This is an automated message generated from the QEMU Patches.
Thank you for submitting this patch.  This patch no longer applies to qemu.git.

This may have occurred due to:
 
  1) Changes in mainline requiring your patch to be rebased and re-tested.

  2) Sending the mail using a tool other than git-send-email.  Please use
     git-send-email to send patches to QEMU.

  3) Basing this patch off of a branch that isn't tracking the QEMU
     master branch.  If that was done purposefully, please include the name
     of the tree in the subject line in the future to prevent this message.

     For instance: "[PATCH block-next 1/10] qcow3: add fancy new feature"

  4) You no longer wish for this patch to be applied to QEMU.  No additional
     action is required on your part.

Nacked-by: QEMU Patches <aliguori@us.ibm.com>

Below is the output from git-am:

    Applying: PPC: Bring EPR support closer to reality
    fatal: sha1 information is lacking or useless (hw/openpic.c).
    Repository lacks necessary blobs to fall back on 3-way merge.
    Cannot fall back to three-way merge.
    Patch failed at 0001 PPC: Bring EPR support closer to reality
    The copy of the patch that failed is found in:
       /home/aliguori/.patches/qemu-working/.git/rebase-apply/patch
    When you have resolved this problem run "git am --resolved".
    If you would prefer to skip this patch, instead run "git am --skip".
    To restore the original branch and stop patching run "git am --abort".

Re: [Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Alexander Graf]

On 04.01.2013, at 20:07, Scott Wood wrote:

> On 01/04/2013 12:54:51 PM, Alexander Graf wrote:
>> On 04.01.2013, at 19:50, Scott Wood wrote:
>> > On 01/04/2013 04:24:59 AM, Alexander Graf wrote:
>> >>         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.
> 
> I see bool handlers in savevm.c.

I mean vmstate of course. But you're right - there are bool handlers. Awesome :).


Alex

> I'm not sure how a non-fixed-size "int" is better.
> 
> BTW, docs/migration.txt says that savevm is the "legacy" approach.
> 
> -Scott

[Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Alexander Graf]

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 <agraf@suse.de>

---

v1 -> v2:

  - use bool to indicate whether to use the mpic proxy
  - add comment saying that MPIC 4.0 is the first one to implement EPR
  - remove open-coded 0xA0 offset
---
 hw/openpic.c                |   21 +++++++++++++++++++++
 hw/ppc/e500.c               |    4 ++--
 target-ppc/Makefile.objs    |    1 -
 target-ppc/cpu.h            |    4 +++-
 target-ppc/excp_helper.c    |    4 ++++
 target-ppc/helper.h         |    1 -
 target-ppc/mpic_helper.c    |   35 -----------------------------------
 target-ppc/translate_init.c |    7 +------
 8 files changed, 31 insertions(+), 46 deletions(-)
 delete mode 100644 target-ppc/mpic_helper.c

diff --git a/hw/openpic.c b/hw/openpic.c
index e773d68..3b20a39 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,9 @@ 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;
+        /* XXX really only available as of MPIC 4.0 */
+        opp->mpic_mode_mask = GCR_MODE_PROXY;
+
         msi_supported = true;
         list = list_be;
 
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 5d70618..3a9e1c7 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -497,8 +497,8 @@ void ppce500_init(PPCE500Params *params)
         irqs[i][OPENPIC_OUTPUT_INT] = input[PPCE500_INPUT_INT];
         irqs[i][OPENPIC_OUTPUT_CINT] = input[PPCE500_INPUT_CINT];
         env->spr[SPR_BOOKE_PIR] = env->cpu_index = i;
-        env->mpic_cpu_base = MPC8544_CCSRBAR_BASE +
-                              MPC8544_MPIC_REGS_OFFSET + 0x20000;
+        env->mpic_iack = MPC8544_CCSRBAR_BASE +
+                         MPC8544_MPIC_REGS_OFFSET + 0x200A0;
 
         ppc_booke_timers_init(cpu, 400000000, PPC_TIMER_E500);
 
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..dc5145b 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1067,7 +1067,9 @@ struct CPUPPCState {
     target_ulong ivor_mask;
     target_ulong ivpr_mask;
     target_ulong hreset_vector;
-    hwaddr mpic_cpu_base;
+    hwaddr mpic_iack;
+    /* true when the external proxy facility mode is enabled */
+    bool mpic_proxy;
 #endif
 
     /* Those resources are used only during code translation */
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 41037a7..0a1ac86 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_iack);
+        }
         goto store_next;
     case POWERPC_EXCP_ALIGN:     /* Alignment exception                      */
         if (lpes1 == 0) {
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index d2e9a55..83139d5 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -405,7 +405,6 @@ DEF_HELPER_2(store_40x_dbcr0, void, env, tl)
 DEF_HELPER_2(store_40x_sler, void, env, tl)
 DEF_HELPER_2(store_booke_tcr, void, env, tl)
 DEF_HELPER_2(store_booke_tsr, void, env, tl)
-DEF_HELPER_1(load_epr, tl, env)
 DEF_HELPER_3(store_ibatl, void, env, i32, tl)
 DEF_HELPER_3(store_ibatu, void, env, i32, tl)
 DEF_HELPER_3(store_dbatl, void, env, i32, tl)
diff --git a/target-ppc/mpic_helper.c b/target-ppc/mpic_helper.c
deleted file mode 100644
index 2c6a4d3..0000000
--- a/target-ppc/mpic_helper.c
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- *  PowerPC emulation helpers for QEMU.
- *
- *  Copyright (c) 2003-2007 Jocelyn Mayer
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see <http://www.gnu.org/licenses/>.
- */
-#include "cpu.h"
-#include "helper.h"
-
-/*****************************************************************************/
-/* SPR accesses */
-
-#if !defined(CONFIG_USER_ONLY)
-/*
- * This is an ugly helper for EPR, which is basically the same as accessing
- * the IACK (PIAC) register on the MPIC. Because we model the MPIC as a device
- * that can only talk to the CPU through MMIO, let's access it that way!
- */
-target_ulong helper_load_epr(CPUPPCState *env)
-{
-    return ldl_phys(env->mpic_cpu_base + 0xA0);
-}
-#endif
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 42ed748..e2eeb87 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -4493,11 +4493,6 @@ static void spr_read_mas73(void *opaque, int gprn, int sprn)
     tcg_temp_free(mas7);
 }
 
-static void spr_load_epr(void *opaque, int gprn, int sprn)
-{
-    gen_helper_load_epr(cpu_gpr[gprn], cpu_env);
-}
-
 #endif
 
 enum fsl_e500_version {
@@ -4656,7 +4651,7 @@ static void init_proc_e500 (CPUPPCState *env, int version)
                  0x00000000);
     spr_register(env, SPR_BOOKE_EPR, "EPR",
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_load_epr, SPR_NOACCESS,
+                 &spr_read_generic, SPR_NOACCESS,
                  0x00000000);
     /* XXX better abstract into Emb.xxx features */
     if (version == fsl_e5500) {
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Scott Wood]

On 01/04/2013 05:36:42 PM, Alexander Graf wrote:
> @@ -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 */

Why is "if (opp->mpic_mode_mask)" needed?  The code should already be a
no-op if mpic_mode_mask is zero.

Can get rid of the "else" by having the reset code do a "break".

> @@ -1407,6 +1425,9 @@ 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;
> +        /* XXX really only available as of MPIC 4.0 */
> +        opp->mpic_mode_mask = GCR_MODE_PROXY;
> +

Shouldn't mpic_mode_mask be set to GCR_MODE_MIXED for Raven?

-Scott

Re: [Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Alexander Graf]

On 07.01.2013, at 18:42, Scott Wood wrote:

> On 01/04/2013 05:36:42 PM, Alexander Graf wrote:
>> @@ -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 */
> 
> Why is "if (opp->mpic_mode_mask)" needed?  The code should already be a
> no-op if mpic_mode_mask is zero.

It is today, but it wouldn't be for pass-through mode which we don't implement today. I wasn't sure whether raven implements pass-through / mixed mode, so I wanted to maintain the status quo as much as possible.

> Can get rid of the "else" by having the reset code do a "break".

Yes. Not sure it's worth another patch though :).

> 
>> @@ -1407,6 +1425,9 @@ 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;
>> +        /* XXX really only available as of MPIC 4.0 */
>> +        opp->mpic_mode_mask = GCR_MODE_PROXY;
>> +
> 
> Shouldn't mpic_mode_mask be set to GCR_MODE_MIXED for Raven?

Does Raven support the MIXED bit? Do you have a pointer to the spec?


Alex

Re: [Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Scott Wood]

On 01/07/2013 11:50:02 AM, Alexander Graf wrote:
> >> @@ -1407,6 +1425,9 @@ 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;
> >> +        /* XXX really only available as of MPIC 4.0 */
> >> +        opp->mpic_mode_mask = GCR_MODE_PROXY;
> >> +
> >
> > Shouldn't mpic_mode_mask be set to GCR_MODE_MIXED for Raven?
> 
> Does Raven support the MIXED bit?

Yes.  It's part of the base openpic spec (though it calls the bit "8259  
pass through disable").

> Do you have a pointer to the spec?

http://www.slac.stanford.edu/grp/cd/soft/vxworks/doc/cpu/pci/ppc/mcp750/mcp750pg.pdf

-Scott