[Qemu-devel] [PATCH 0/7] Openrisc misc features / fixes

[Qemu-devel] [PATCH 0/7] Openrisc misc features / fixes

[Stafford Horne]

Hello,

The first patch here I sent a few weeks ago and got go response.  Sending
again.

The second was requested by the uclibc-ng maintainer as when he is testing
his builds he uses qemu and would like to have a way for the system to shut
down.  This models how our other simulators shutdown, so no changes are
needed on the kernel side.

The others I added while working on upcoming openrisc smp support.

-Stafford

Stafford Horne (7):
  target/openrisc: Fixes for memory debugging
  target/openrisc: add shutdown logic
  target/openrisc: add numcores and coreid support
  target/openrisc: implement shadow registers
  migration: Add VMSTATE_UINTTL_2DARRAY()
  migration: Add VMSTATE_STRUCT_2DARRAY()
  target/openrisc: Implement full vmstate serialization

 include/migration/cpu.h      |  7 ++++
 include/migration/vmstate.h  | 15 +++++++++
 target/openrisc/cpu.c        |  5 ++-
 target/openrisc/cpu.h        |  4 ++-
 target/openrisc/helper.h     |  1 +
 target/openrisc/machine.c    | 76 ++++++++++++++++++++++++++++++++++++++++++--
 target/openrisc/mmu.c        | 23 +++++++++++---
 target/openrisc/sys_helper.c | 32 +++++++++++++++++++
 target/openrisc/translate.c  |  8 +++--
 9 files changed, 160 insertions(+), 11 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH 1/7] target/openrisc: Fixes for memory debugging

[Stafford Horne]

When debugging in gdb you might want to inspect instructions in mapped
pages or in exception vectors like 0x800 etc.  This was previously not
possible in qemu since the *get_phys_page_debug() routine only looked
into the data tlb.

Change to fall back to look into instruction tlb and plain physical
pages.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 target/openrisc/mmu.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/target/openrisc/mmu.c b/target/openrisc/mmu.c
index 56b11d3..a6d7bcd 100644
--- a/target/openrisc/mmu.c
+++ b/target/openrisc/mmu.c
@@ -124,7 +124,7 @@ static int cpu_openrisc_get_phys_addr(OpenRISCCPU *cpu,
 {
     int ret = TLBRET_MATCH;
 
-    if (rw == 2) {    /* ITLB */
+    if (rw == MMU_INST_FETCH) {    /* ITLB */
        *physical = 0;
         ret = cpu->env.tlb->cpu_openrisc_map_address_code(cpu, physical,
                                                           prot, address, rw);
@@ -221,12 +221,27 @@ hwaddr openrisc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
     OpenRISCCPU *cpu = OPENRISC_CPU(cs);
     hwaddr phys_addr;
     int prot;
+    int miss;
 
-    if (cpu_openrisc_get_phys_addr(cpu, &phys_addr, &prot, addr, 0)) {
-        return -1;
+    /* Check memory for any kind of address, since during debug the
+       gdb can ask for anything, check data tlb for address */
+    miss = cpu_openrisc_get_phys_addr(cpu, &phys_addr, &prot, addr, 0);
+
+    /* Check instruction tlb */
+    if (miss) {
+        miss = cpu_openrisc_get_phys_addr(cpu, &phys_addr, &prot, addr, MMU_INST_FETCH);
+    }
+
+    /* Last, fall back to a plain address */
+    if (miss) {
+        miss = cpu_openrisc_get_phys_nommu(cpu, &phys_addr, &prot, addr, 0);
     }
 
-    return phys_addr;
+    if (miss) {
+        return -1;
+    } else {
+        return phys_addr;
+    }
 }
 
 void cpu_openrisc_mmu_init(OpenRISCCPU *cpu)
-- 
2.9.3

Re: [Qemu-devel] [PATCH 1/7] target/openrisc: Fixes for memory debugging

[Richard Henderson]

On 04/16/2017 04:23 PM, Stafford Horne wrote:
> When debugging in gdb you might want to inspect instructions in mapped
> pages or in exception vectors like 0x800 etc.  This was previously not
> possible in qemu since the *get_phys_page_debug() routine only looked
> into the data tlb.
>
> Change to fall back to look into instruction tlb and plain physical
> pages.
>
> Signed-off-by: Stafford Horne <shorne@gmail.com>

Oh the horrors of a software managed TLB.

You might do well to architecturally define an SPR that holds the page table 
base, even if for real hardware that's only used by the software refill to load 
up the address.

That would give qemu the option of performing a real page table walk.  This 
would fix this debug hook properly (so that you can examine pages that aren't 
in the TLB at all).  It would also optionally allow QEMU to skip the software 
refill, which *significantly* speeds up emulation.

That said,

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 1/7] target/openrisc: Fixes for memory debugging

[Stafford Horne]

On Tue, Apr 18, 2017 at 12:47:30AM -0700, Richard Henderson wrote:
> On 04/16/2017 04:23 PM, Stafford Horne wrote:
> > When debugging in gdb you might want to inspect instructions in mapped
> > pages or in exception vectors like 0x800 etc.  This was previously not
> > possible in qemu since the *get_phys_page_debug() routine only looked
> > into the data tlb.
> > 
> > Change to fall back to look into instruction tlb and plain physical
> > pages.
> > 
> > Signed-off-by: Stafford Horne <shorne@gmail.com>
> 
> Oh the horrors of a software managed TLB.
> 
> You might do well to architecturally define an SPR that holds the page table
> base, even if for real hardware that's only used by the software refill to
> load up the address.
> 
> That would give qemu the option of performing a real page table walk.  This
> would fix this debug hook properly (so that you can examine pages that
> aren't in the TLB at all).  It would also optionally allow QEMU to skip the
> software refill, which *significantly* speeds up emulation.

Understood, I guess we would also need a way to represent which paging
model we are using (1 level, 2 level etc)?

> That said,
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>

Thanks for the review.

-Stafford

> 
> r~

Re: [Qemu-devel] [PATCH 1/7] target/openrisc: Fixes for memory debugging

[Richard Henderson]

On 04/18/2017 07:18 AM, Stafford Horne wrote:
> On Tue, Apr 18, 2017 at 12:47:30AM -0700, Richard Henderson wrote:
>> On 04/16/2017 04:23 PM, Stafford Horne wrote:
>>> When debugging in gdb you might want to inspect instructions in mapped
>>> pages or in exception vectors like 0x800 etc.  This was previously not
>>> possible in qemu since the *get_phys_page_debug() routine only looked
>>> into the data tlb.
>>>
>>> Change to fall back to look into instruction tlb and plain physical
>>> pages.
>>>
>>> Signed-off-by: Stafford Horne <shorne@gmail.com>
>>
>> Oh the horrors of a software managed TLB.
>>
>> You might do well to architecturally define an SPR that holds the page table
>> base, even if for real hardware that's only used by the software refill to
>> load up the address.
>>
>> That would give qemu the option of performing a real page table walk.  This
>> would fix this debug hook properly (so that you can examine pages that
>> aren't in the TLB at all).  It would also optionally allow QEMU to skip the
>> software refill, which *significantly* speeds up emulation.
>
> Understood, I guess we would also need a way to represent which paging
> model we are using (1 level, 2 level etc)?

I suppose.  You really have a 1-level lookup?  For huge pages only, or is this 
a virtual 2-level lookup with double-faulting to handle the second level?


r~

Re: [Qemu-devel] [PATCH 1/7] target/openrisc: Fixes for memory debugging

[Stafford Horne]

On Tue, Apr 18, 2017 at 08:00:06AM -0700, Richard Henderson wrote:
> On 04/18/2017 07:18 AM, Stafford Horne wrote:
> > On Tue, Apr 18, 2017 at 12:47:30AM -0700, Richard Henderson wrote:
> > > On 04/16/2017 04:23 PM, Stafford Horne wrote:
> > > > When debugging in gdb you might want to inspect instructions in mapped
> > > > pages or in exception vectors like 0x800 etc.  This was previously not
> > > > possible in qemu since the *get_phys_page_debug() routine only looked
> > > > into the data tlb.
> > > > 
> > > > Change to fall back to look into instruction tlb and plain physical
> > > > pages.
> > > > 
> > > > Signed-off-by: Stafford Horne <shorne@gmail.com>
> > > 
> > > Oh the horrors of a software managed TLB.
> > > 
> > > You might do well to architecturally define an SPR that holds the page table
> > > base, even if for real hardware that's only used by the software refill to
> > > load up the address.
> > > 
> > > That would give qemu the option of performing a real page table walk.  This
> > > would fix this debug hook properly (so that you can examine pages that
> > > aren't in the TLB at all).  It would also optionally allow QEMU to skip the
> > > software refill, which *significantly* speeds up emulation.
> > 
> > Understood, I guess we would also need a way to represent which paging
> > model we are using (1 level, 2 level etc)?
> 
> I suppose.  You really have a 1-level lookup?  For huge pages only, or is
> this a virtual 2-level lookup with double-faulting to handle the second
> level?

We don't,  the linux kernel is implemented with basic 2-level lookup's with
single fault.  I was just thinking its not ideal for the emulator to assume
the paging model since it could change from kernel to kernel.

But I guess it will not change anytime soon.

-Stafford

[Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Stafford Horne]

In openrisc simulators we use hooks like 'l.nop 1' to cause the
simulator to exit.  Implement that for qemu too.

Reported-by: Waldemar Brodkorb <wbx@openadk.org>
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 target/openrisc/helper.h     |  1 +
 target/openrisc/sys_helper.c | 17 +++++++++++++++++
 target/openrisc/translate.c  |  5 ++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/target/openrisc/helper.h b/target/openrisc/helper.h
index 4fd1a6b..b7838f5 100644
--- a/target/openrisc/helper.h
+++ b/target/openrisc/helper.h
@@ -59,3 +59,4 @@ DEF_HELPER_FLAGS_1(rfe, 0, void, env)
 /* sys */
 DEF_HELPER_FLAGS_4(mtspr, 0, void, env, tl, tl, tl)
 DEF_HELPER_FLAGS_4(mfspr, TCG_CALL_NO_WG, tl, env, tl, tl, tl)
+DEF_HELPER_1(nop, void, i32)
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index 60c3193..2eaff87 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -22,6 +22,7 @@
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
+#include "sysemu/sysemu.h"
 
 #define TO_SPR(group, number) (((group) << 11) + (number))
 
@@ -286,3 +287,19 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
     /* for rd is passed in, if rd unchanged, just keep it back.  */
     return rd;
 }
+
+/* In openrisc simulators nop can be used to do intersting
+ * things like shut down the simulator */
+void HELPER(nop)(uint32_t arg)
+{
+#ifndef CONFIG_USER_ONLY
+    switch (arg) {
+    case 0x001: /* NOP_EXIT */
+    case 0x00c: /* NOP_EXIT_SILENT */
+        qemu_system_shutdown_request();
+        break;
+    default:
+        break;
+    }
+#endif
+}
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 7c4cbf2..74df245 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -755,8 +755,11 @@ static void dec_misc(DisasContext *dc, uint32_t insn)
         switch (op1) {
         case 0x01:    /* l.nop */
             LOG_DIS("l.nop %d\n", I16);
+            {
+                TCGv_i32 arg = tcg_const_i32(I16);
+                gen_helper_nop(arg);
+            }
             break;
-
         default:
             gen_illegal_exception(dc);
             break;
-- 
2.9.3

Re: [Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Richard Henderson]

On 04/16/2017 04:23 PM, Stafford Horne wrote:
> In openrisc simulators we use hooks like 'l.nop 1' to cause the
> simulator to exit.  Implement that for qemu too.
>
> Reported-by: Waldemar Brodkorb <wbx@openadk.org>
> Signed-off-by: Stafford Horne <shorne@gmail.com>

As I said the first time this was posted: This is horrible.

If you want to do something like this, it needs to be buried under a special 
run mode like -semihosting.

>          case 0x01:    /* l.nop */
>              LOG_DIS("l.nop %d\n", I16);
> +            {
> +                TCGv_i32 arg = tcg_const_i32(I16);
> +                gen_helper_nop(arg);
> +            }

You also really really must special-case l.nop 0 so that it doesn't generate a 
function call.  Just think of all the extra calls you're adding for every delay 
slot that couldn't be filled.


r~

Re: [Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Stafford Horne]

On Tue, Apr 18, 2017 at 12:52:52AM -0700, Richard Henderson wrote:
> On 04/16/2017 04:23 PM, Stafford Horne wrote:
> > In openrisc simulators we use hooks like 'l.nop 1' to cause the
> > simulator to exit.  Implement that for qemu too.
> > 
> > Reported-by: Waldemar Brodkorb <wbx@openadk.org>
> > Signed-off-by: Stafford Horne <shorne@gmail.com>
> 
> As I said the first time this was posted: This is horrible.
> 
> If you want to do something like this, it needs to be buried under a special
> run mode like -semihosting.

Understood,  I will revise this.  I didnt know this was posted before.

> >          case 0x01:    /* l.nop */
> >              LOG_DIS("l.nop %d\n", I16);
> > +            {
> > +                TCGv_i32 arg = tcg_const_i32(I16);
> > +                gen_helper_nop(arg);
> > +            }
> 
> You also really really must special-case l.nop 0 so that it doesn't generate
> a function call.  Just think of all the extra calls you're adding for every
> delay slot that couldn't be filled.

Yeah, that makes sense.  Ill add that for l.nop 0.

-Stafford

> 
> r~

Re: [Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Stafford Horne]

On Tue, Apr 18, 2017 at 11:20:55PM +0900, Stafford Horne wrote:
> On Tue, Apr 18, 2017 at 12:52:52AM -0700, Richard Henderson wrote:
> > On 04/16/2017 04:23 PM, Stafford Horne wrote:
> > > In openrisc simulators we use hooks like 'l.nop 1' to cause the
> > > simulator to exit.  Implement that for qemu too.
> > > 
> > > Reported-by: Waldemar Brodkorb <wbx@openadk.org>
> > > Signed-off-by: Stafford Horne <shorne@gmail.com>
> > 
> > As I said the first time this was posted: This is horrible.
> > 
> > If you want to do something like this, it needs to be buried under a special
> > run mode like -semihosting.
> 
> Understood,  I will revise this.  I didnt know this was posted before.
> 
> > >          case 0x01:    /* l.nop */
> > >              LOG_DIS("l.nop %d\n", I16);
> > > +            {
> > > +                TCGv_i32 arg = tcg_const_i32(I16);
> > > +                gen_helper_nop(arg);
> > > +            }
> > 
> > You also really really must special-case l.nop 0 so that it doesn't generate
> > a function call.  Just think of all the extra calls you're adding for every
> > delay slot that couldn't be filled.
> 
> Yeah, that makes sense.  Ill add that for l.nop 0.

FYI,

I am going to drop this patch for now.  I think Waldemar can apply this
patch for the time being.

I looked through the semihosting route and I don't think poking l.nop
through there makes much sense since that looks mainly for syscalls.  I
also considered making another flag like `-or1k-hacks`, but I figured that
wouldnt be appropriate.

I discussed a bit on #qemu and Alexander Graf suggested to properly define
shutdown semantics for openrisc. Some examples were shown from ppc, s390
and arm.

  s390x
  http://git.qemu.org/?p=qemu.git;a=blob;f=target/s390x/helper.c;hb=HEAD#l265
  Detects the cpu is in WAIT state and shutsdown qemu.

  ppc - platform
  http://git.qemu.org/?p=qemu.git;a=blob;f=hw/ppc/e500.c;hb=HEAD#l936
  Registers hardware device mpc8xxx_gpio which handles shutdown via gpio

I will have a thought about this, it will require some kernel changes.

-Stafford

Re: [Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Richard Henderson]

On 04/22/2017 12:09 PM, Stafford Horne wrote:
> I discussed a bit on #qemu and Alexander Graf suggested to properly define
> shutdown semantics for openrisc. Some examples were shown from ppc, s390
> and arm.

Yes, properly defining this in the spec goes a long way toward fixing the 
underlying problem.

It's probably worth thinking about a wait-state condition as well, so that qemu 
can avoid staying pegged at 100% host cpu for an idle guest cpu.

>    ppc - platform
>    http://git.qemu.org/?p=qemu.git;a=blob;f=hw/ppc/e500.c;hb=HEAD#l936
>    Registers hardware device mpc8xxx_gpio which handles shutdown via gpio

That's one possibility.  Another is to define an SPR.


r~

[Qemu-devel] [PATCH RFC] target/openrisc: Support non-busy idle state using PMR SPR

[Stafford Horne]

The OpenRISC architecture has the Power Management Register (PMR)
special purpose register to manage cpu power states.  The interesting
modes are:

 * Doze Mode (DME) - Stop cpu except timer & pic - wake on interrupt
 * Sleep Mode (SME) - Stop cpu and all units - wake on interrupt
 * Suspend Model (SUME) - Stop cpu and all units - wake on reset

The linux kernel will set DME when idle.

This patch implements the PMR SPR and halts the qemu cpu when there is a
change to DME or SME.  This means that openrisc qemu in no longer peggs
a host cpu at 100%.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
(Sorry, resending this again, there was something wrong with my mail setup
on the last)

Hello,

This patch seems work fine but I am not sure if it is the right way to do
trigger the halt signal.  Should I do it via raising an interrupt or
exception and exitting the cpu?

Also, I don't know if its due to this patch of an issue with the timer
interrupts.  After applying this patch the timer interrupts do not trigger
until a keypress is make.  i.e. something like this...

  $ sleep 5
  <hangs forever until a key is pressed>

It may or may not be related to this patch as I noticed sometime things
like this happened before this patch.


 target/openrisc/cpu.c        |  3 ++-
 target/openrisc/cpu.h        | 10 ++++++++++
 target/openrisc/interrupt.c  |  2 ++
 target/openrisc/machine.c    |  1 +
 target/openrisc/sys_helper.c | 12 ++++++++++++
 5 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index c9b3f22..1d6330c 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -51,7 +51,8 @@ static void openrisc_cpu_reset(CPUState *s)
     cpu->env.lock_addr = -1;
     s->exception_index = -1;
 
-    cpu->env.upr = UPR_UP | UPR_DMP | UPR_IMP | UPR_PICP | UPR_TTP;
+    cpu->env.upr = UPR_UP | UPR_DMP | UPR_IMP | UPR_PICP | UPR_TTP |
+                   UPR_PMP;
     cpu->env.dmmucfgr = (DMMUCFGR_NTW & (0 << 2)) | (DMMUCFGR_NTS & (6 << 2));
     cpu->env.immucfgr = (IMMUCFGR_NTW & (0 << 2)) | (IMMUCFGR_NTS & (6 << 2));
 
diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
index 938ccc3..2721432 100644
--- a/target/openrisc/cpu.h
+++ b/target/openrisc/cpu.h
@@ -140,6 +140,15 @@ enum {
     IMMUCFGR_HTR = (1 << 11),
 };
 
+/* Power management register */
+enum {
+    PMR_SDF = (15 << 0),
+    PMR_DME = (1 << 4),
+    PMR_SME = (1 << 5),
+    PMR_DCGE = (1 << 6),
+    PMR_SUME = (1 << 7),
+};
+
 /* Float point control status register */
 enum {
     FPCSR_FPEE = 1,
@@ -284,6 +293,7 @@ typedef struct CPUOpenRISCState {
     uint32_t immucfgr;        /* IMMU configure register */
     uint32_t esr;             /* Exception supervisor register */
     uint32_t evbar;           /* Exception vector base address register */
+    uint32_t pmr;             /* Power Management Register */
     uint32_t fpcsr;           /* Float register */
     float_status fp_status;
 
diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
index 2c91fab..3959671 100644
--- a/target/openrisc/interrupt.c
+++ b/target/openrisc/interrupt.c
@@ -60,6 +60,8 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
     env->sr |= SR_SM;
     env->sr &= ~SR_IEE;
     env->sr &= ~SR_TEE;
+    env->pmr &= ~PMR_DME;
+    env->pmr &= ~PMR_SME;
     env->tlb->cpu_openrisc_map_address_data = &cpu_openrisc_get_phys_nommu;
     env->tlb->cpu_openrisc_map_address_code = &cpu_openrisc_get_phys_nommu;
     env->lock_addr = -1;
diff --git a/target/openrisc/machine.c b/target/openrisc/machine.c
index a82be62..a20cce7 100644
--- a/target/openrisc/machine.c
+++ b/target/openrisc/machine.c
@@ -138,6 +138,7 @@ static const VMStateDescription vmstate_env = {
         VMSTATE_UINT32(dmmucfgr, CPUOpenRISCState),
         VMSTATE_UINT32(immucfgr, CPUOpenRISCState),
         VMSTATE_UINT32(evbar, CPUOpenRISCState),
+        VMSTATE_UINT32(pmr, CPUOpenRISCState),
         VMSTATE_UINT32(esr, CPUOpenRISCState),
         VMSTATE_UINT32(fpcsr, CPUOpenRISCState),
         VMSTATE_UINT64(mac, CPUOpenRISCState),
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index fa3d6a4..cb1e085 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -22,6 +22,7 @@
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
+#include "exception.h"
 
 #define TO_SPR(group, number) (((group) << 11) + (number))
 
@@ -141,6 +142,14 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
     case TO_SPR(5, 2):  /* MACHI */
         env->mac = deposit64(env->mac, 32, 32, rb);
         break;
+    case TO_SPR(8, 0):  /* PMR */
+        env->pmr = rb;
+        if (env->pmr & PMR_DME || env->pmr & PMR_SME) {
+            cpu_restore_state(cs, GETPC() + 4);
+            cs->halted = 1;
+            raise_exception(cpu, EXCP_HLT);
+        }
+        break;
     case TO_SPR(9, 0):  /* PICMR */
         env->picmr |= rb;
         break;
@@ -287,6 +296,9 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
         return env->mac >> 32;
         break;
 
+    case TO_SPR(8, 0):  /* PMR */
+        return env->pmr;
+
     case TO_SPR(9, 0):  /* PICMR */
         return env->picmr;
 
-- 
2.9.3

Re: [Qemu-devel] [PATCH RFC] target/openrisc: Support non-busy idle state using PMR SPR

[Richard Henderson]

On 04/23/2017 11:54 PM, Stafford Horne wrote:
> The OpenRISC architecture has the Power Management Register (PMR)
> special purpose register to manage cpu power states.  The interesting
> modes are:
> 
>   * Doze Mode (DME) - Stop cpu except timer & pic - wake on interrupt
>   * Sleep Mode (SME) - Stop cpu and all units - wake on interrupt
>   * Suspend Model (SUME) - Stop cpu and all units - wake on reset
> 
> The linux kernel will set DME when idle.

And SUME would be, essentially, poweroff?  Perhaps at least for the purposes of 
QEMU; on real hardware one could press a button to assert reset and reboot.

> Also, I don't know if its due to this patch of an issue with the timer
> interrupts.  After applying this patch the timer interrupts do not trigger
> until a keypress is make.  i.e. something like this...
> 
>    $ sleep 5
>    <hangs forever until a key is pressed>
...
> +            cpu_restore_state(cs, GETPC() + 4);

This isn't correct.  You want

	cpu_restore_state(cs, GETPC());
	cs->env.pc += 4;

So what's happening is that you're re-executing the MTSPR and going back to 
sleep again.  Which probably explains the hang.


r~

[Qemu-devel] [PATCH RFC v2] target/openrisc: Support non-busy idle state using PMR SPR

[Stafford Horne]

The OpenRISC architecture has the Power Management Register (PMR)
special purpose register to manage cpu power states.  The interesting
modes are:

 * Doze Mode (DME) - Stop cpu except timer & pic - wake on interrupt
 * Sleep Mode (SME) - Stop cpu and all units - wake on interrupt
 * Suspend Model (SUME) - Stop cpu and all units - wake on reset

The linux kernel will set DME when idle.

This patch implements the PMR SPR and halts the qemu cpu when there is a
change to DME or SME.  This means that openrisc qemu in no longer peggs
a host cpu at 100%.

In order for this to work we need to kick the CPU when timers are
expired.  Update the cpu timer to kick the cpu upon each timer event.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
Hello,

This patch seems work fine now.

Changes since v1:
  * Changed to kick cpu in timer
  * Increment pc separate from restore

 hw/openrisc/cputimer.c       |  1 +
 target/openrisc/cpu.c        |  3 ++-
 target/openrisc/cpu.h        | 10 ++++++++++
 target/openrisc/interrupt.c  |  2 ++
 target/openrisc/machine.c    |  1 +
 target/openrisc/sys_helper.c | 13 +++++++++++++
 6 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/hw/openrisc/cputimer.c b/hw/openrisc/cputimer.c
index a98c799..febc469 100644
--- a/hw/openrisc/cputimer.c
+++ b/hw/openrisc/cputimer.c
@@ -61,6 +61,7 @@ void cpu_openrisc_timer_update(OpenRISCCPU *cpu)
     }
     next = now + (uint64_t)wait * TIMER_PERIOD;
     timer_mod(cpu->env.timer, next);
+    qemu_cpu_kick(CPU(cpu));
 }
 
 void cpu_openrisc_count_start(OpenRISCCPU *cpu)
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index c9b3f22..1d6330c 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -51,7 +51,8 @@ static void openrisc_cpu_reset(CPUState *s)
     cpu->env.lock_addr = -1;
     s->exception_index = -1;
 
-    cpu->env.upr = UPR_UP | UPR_DMP | UPR_IMP | UPR_PICP | UPR_TTP;
+    cpu->env.upr = UPR_UP | UPR_DMP | UPR_IMP | UPR_PICP | UPR_TTP |
+                   UPR_PMP;
     cpu->env.dmmucfgr = (DMMUCFGR_NTW & (0 << 2)) | (DMMUCFGR_NTS & (6 << 2));
     cpu->env.immucfgr = (IMMUCFGR_NTW & (0 << 2)) | (IMMUCFGR_NTS & (6 << 2));
 
diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
index 938ccc3..2721432 100644
--- a/target/openrisc/cpu.h
+++ b/target/openrisc/cpu.h
@@ -140,6 +140,15 @@ enum {
     IMMUCFGR_HTR = (1 << 11),
 };
 
+/* Power management register */
+enum {
+    PMR_SDF = (15 << 0),
+    PMR_DME = (1 << 4),
+    PMR_SME = (1 << 5),
+    PMR_DCGE = (1 << 6),
+    PMR_SUME = (1 << 7),
+};
+
 /* Float point control status register */
 enum {
     FPCSR_FPEE = 1,
@@ -284,6 +293,7 @@ typedef struct CPUOpenRISCState {
     uint32_t immucfgr;        /* IMMU configure register */
     uint32_t esr;             /* Exception supervisor register */
     uint32_t evbar;           /* Exception vector base address register */
+    uint32_t pmr;             /* Power Management Register */
     uint32_t fpcsr;           /* Float register */
     float_status fp_status;
 
diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
index 2c91fab..3959671 100644
--- a/target/openrisc/interrupt.c
+++ b/target/openrisc/interrupt.c
@@ -60,6 +60,8 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
     env->sr |= SR_SM;
     env->sr &= ~SR_IEE;
     env->sr &= ~SR_TEE;
+    env->pmr &= ~PMR_DME;
+    env->pmr &= ~PMR_SME;
     env->tlb->cpu_openrisc_map_address_data = &cpu_openrisc_get_phys_nommu;
     env->tlb->cpu_openrisc_map_address_code = &cpu_openrisc_get_phys_nommu;
     env->lock_addr = -1;
diff --git a/target/openrisc/machine.c b/target/openrisc/machine.c
index a82be62..a20cce7 100644
--- a/target/openrisc/machine.c
+++ b/target/openrisc/machine.c
@@ -138,6 +138,7 @@ static const VMStateDescription vmstate_env = {
         VMSTATE_UINT32(dmmucfgr, CPUOpenRISCState),
         VMSTATE_UINT32(immucfgr, CPUOpenRISCState),
         VMSTATE_UINT32(evbar, CPUOpenRISCState),
+        VMSTATE_UINT32(pmr, CPUOpenRISCState),
         VMSTATE_UINT32(esr, CPUOpenRISCState),
         VMSTATE_UINT32(fpcsr, CPUOpenRISCState),
         VMSTATE_UINT64(mac, CPUOpenRISCState),
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index fa3d6a4..44acf0d 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -22,6 +22,7 @@
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
+#include "exception.h"
 
 #define TO_SPR(group, number) (((group) << 11) + (number))
 
@@ -141,6 +142,15 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
     case TO_SPR(5, 2):  /* MACHI */
         env->mac = deposit64(env->mac, 32, 32, rb);
         break;
+    case TO_SPR(8, 0):  /* PMR */
+        env->pmr = rb;
+        if (env->pmr & PMR_DME || env->pmr & PMR_SME) {
+            cpu_restore_state(cs, GETPC());
+            env->pc += 4;
+            cs->halted = 1;
+            raise_exception(cpu, EXCP_HLT);
+        }
+        break;
     case TO_SPR(9, 0):  /* PICMR */
         env->picmr |= rb;
         break;
@@ -287,6 +297,9 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
         return env->mac >> 32;
         break;
 
+    case TO_SPR(8, 0):  /* PMR */
+        return env->pmr;
+
     case TO_SPR(9, 0):  /* PICMR */
         return env->picmr;
 
-- 
2.9.3

Re: [Qemu-devel] [PATCH RFC] target/openrisc: Support non-busy idle state using PMR SPR

[Stafford Horne]

On Tue, Apr 25, 2017 at 12:11:00PM +0200, Richard Henderson wrote:
> On 04/23/2017 11:54 PM, Stafford Horne wrote:
> > The OpenRISC architecture has the Power Management Register (PMR)
> > special purpose register to manage cpu power states.  The interesting
> > modes are:
> > 
> >   * Doze Mode (DME) - Stop cpu except timer & pic - wake on interrupt
> >   * Sleep Mode (SME) - Stop cpu and all units - wake on interrupt
> >   * Suspend Model (SUME) - Stop cpu and all units - wake on reset
> > 
> > The linux kernel will set DME when idle.
> 
> And SUME would be, essentially, poweroff?  Perhaps at least for the purposes
> of QEMU; on real hardware one could press a button to assert reset and
> reboot.

Yes, that is what I am thinking, but I could add this later, after some
reviews with other OpenRISC folks.

> > Also, I don't know if its due to this patch of an issue with the timer
> > interrupts.  After applying this patch the timer interrupts do not trigger
> > until a keypress is make.  i.e. something like this...
> > 
> >    $ sleep 5
> >    <hangs forever until a key is pressed>
> ...
> > +            cpu_restore_state(cs, GETPC() + 4);
> 
> This isn't correct.  You want
> 
> 	cpu_restore_state(cs, GETPC());
> 	cs->env.pc += 4;
> 
> So what's happening is that you're re-executing the MTSPR and going back to
> sleep again.  Which probably explains the hang.

I have changed to the above, but I think its essentially the same.  It
resumes after the MTSPR in both cases.

I fixed this now though, you should see another patch.  The issue is the
timer events get ignored once the cpu is in halt state, I added a
qemu_cpu_kick() call in the timer hardware to wake up the cpu on timer
interrupts.  Not sure if thats the best way to do it, but it works 100%
now.

-Stafford

Re: [Qemu-devel] [PATCH RFC] target/openrisc: Support non-busy idle state using PMR SPR

[Richard Henderson]

On 04/25/2017 04:18 PM, Stafford Horne wrote:
> On Tue, Apr 25, 2017 at 12:11:00PM +0200, Richard Henderson wrote:
>> On 04/23/2017 11:54 PM, Stafford Horne wrote:
>>> The OpenRISC architecture has the Power Management Register (PMR)
>>> special purpose register to manage cpu power states.  The interesting
>>> modes are:
>>>
>>>    * Doze Mode (DME) - Stop cpu except timer & pic - wake on interrupt
>>>    * Sleep Mode (SME) - Stop cpu and all units - wake on interrupt
>>>    * Suspend Model (SUME) - Stop cpu and all units - wake on reset
>>>
>>> The linux kernel will set DME when idle.
>>
>> And SUME would be, essentially, poweroff?  Perhaps at least for the purposes
>> of QEMU; on real hardware one could press a button to assert reset and
>> reboot.
> 
> Yes, that is what I am thinking, but I could add this later, after some
> reviews with other OpenRISC folks.
> 
>>> Also, I don't know if its due to this patch of an issue with the timer
>>> interrupts.  After applying this patch the timer interrupts do not trigger
>>> until a keypress is make.  i.e. something like this...
>>>
>>>     $ sleep 5
>>>     <hangs forever until a key is pressed>
>> ...
>>> +            cpu_restore_state(cs, GETPC() + 4);
>>
>> This isn't correct.  You want
>>
>> 	cpu_restore_state(cs, GETPC());
>> 	cs->env.pc += 4;
>>
>> So what's happening is that you're re-executing the MTSPR and going back to
>> sleep again.  Which probably explains the hang.
> 
> I have changed to the above, but I think its essentially the same.  It
> resumes after the MTSPR in both cases.

It's not essentially the same.  GETPC is a host address.  Doing guest 
arithmetic on that is just wrong.

> I fixed this now though, you should see another patch.  The issue is the
> timer events get ignored once the cpu is in halt state, I added a
> qemu_cpu_kick() call in the timer hardware to wake up the cpu on timer
> interrupts.  Not sure if thats the best way to do it, but it works 100%
> now.

Ah, that could be.


r~

Re: [Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Jason A. Donenfeld]

Hey Stafford,

On Mon, Apr 17, 2017 at 08:23:51AM +0900, Stafford Horne wrote:
> In openrisc simulators we use hooks like 'l.nop 1' to cause the
> simulator to exit.  Implement that for qemu too.
> 
> Reported-by: Waldemar Brodkorb <wbx@openadk.org>
> Signed-off-by: Stafford Horne <shorne@gmail.com>

I'm curious as to why this never got merged. I noticed I'm entirely able
to shutdown or to reboot (which is mostly what I care about) Linux from
OpenRISC. It just hangs.

Thanks,
Jason

Re: [Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Peter Maydell]

On Wed, 27 Apr 2022 at 18:46, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hey Stafford,
>
> On Mon, Apr 17, 2017 at 08:23:51AM +0900, Stafford Horne wrote:
> > In openrisc simulators we use hooks like 'l.nop 1' to cause the
> > simulator to exit.  Implement that for qemu too.
> >
> > Reported-by: Waldemar Brodkorb <wbx@openadk.org>
> > Signed-off-by: Stafford Horne <shorne@gmail.com>
>
> I'm curious as to why this never got merged. I noticed I'm entirely able
> to shutdown or to reboot (which is mostly what I care about) Linux from
> OpenRISC. It just hangs.

This kind of thing needs to be either:
 (1) we're modelling real hardware and that real hardware has a
device or other mechanism guest code can prod to cause a power-off
or reboot. Then we model that device, and guest code triggers a
shutdown or reboot exactly as it would on the real hardware.
 (2) there is an architecturally defined ABI for simulators, debug
stubs, etc, that includes various operations typically including
an "exit the simulator" function. (Arm semihosting is an example
of this.) In that case we can implement that functionality,
guarded by and controlled by the appropriate command line options.
(This is generally not as nice as option 1, because the guest code
has to be compiled to have support for semihosting and also because
turning it on is usually also giving implicit permission for the
guest code to read and write arbitrary host files, etc.)

Either way, undocumented random hacks aren't a good idea, which
is why this wasn't merged.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Stafford Horne]

On Wed, Apr 27, 2022 at 07:47:33PM +0100, Peter Maydell wrote:
> On Wed, 27 Apr 2022 at 18:46, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > Hey Stafford,
> >
> > On Mon, Apr 17, 2017 at 08:23:51AM +0900, Stafford Horne wrote:
> > > In openrisc simulators we use hooks like 'l.nop 1' to cause the
> > > simulator to exit.  Implement that for qemu too.
> > >
> > > Reported-by: Waldemar Brodkorb <wbx@openadk.org>
> > > Signed-off-by: Stafford Horne <shorne@gmail.com>
> >
> > I'm curious as to why this never got merged. I noticed I'm entirely able
> > to shutdown or to reboot (which is mostly what I care about) Linux from
> > OpenRISC. It just hangs.
> 
> This kind of thing needs to be either:
>  (1) we're modelling real hardware and that real hardware has a
> device or other mechanism guest code can prod to cause a power-off
> or reboot. Then we model that device, and guest code triggers a
> shutdown or reboot exactly as it would on the real hardware.
>  (2) there is an architecturally defined ABI for simulators, debug
> stubs, etc, that includes various operations typically including
> an "exit the simulator" function. (Arm semihosting is an example
> of this.) In that case we can implement that functionality,
> guarded by and controlled by the appropriate command line options.
> (This is generally not as nice as option 1, because the guest code
> has to be compiled to have support for semihosting and also because
> turning it on is usually also giving implicit permission for the
> guest code to read and write arbitrary host files, etc.)
> 
> Either way, undocumented random hacks aren't a good idea, which
> is why this wasn't merged.

Yes, this is what was brought up before.  At that time semihosting was mentioned
and I tried to understand what it was but didn't really understand it as a general
concept.  Is this something arm specific?

Since the qemu or1k-sim defines our "simulator", I suspect I could add a
definition of our simulator ABI to the OpenRISC architecture specification.  The
simulation uses of l.nop N as ABI hooks is a de-facto standard for OpenRISC.
From the way you describe this now I take it if we document this as a
architecture simulation ABI the patch would be accepted.

-Stafford

Re: [Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Jason A. Donenfeld]

Hi Stafford,

On Thu, Apr 28, 2022 at 06:48:27AM +0900, Stafford Horne wrote:
> On Wed, Apr 27, 2022 at 07:47:33PM +0100, Peter Maydell wrote:
> > On Wed, 27 Apr 2022 at 18:46, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > >
> > > Hey Stafford,
> > >
> > > On Mon, Apr 17, 2017 at 08:23:51AM +0900, Stafford Horne wrote:
> > > > In openrisc simulators we use hooks like 'l.nop 1' to cause the
> > > > simulator to exit.  Implement that for qemu too.
> > > >
> > > > Reported-by: Waldemar Brodkorb <wbx@openadk.org>
> > > > Signed-off-by: Stafford Horne <shorne@gmail.com>
> > >
> > > I'm curious as to why this never got merged. I noticed I'm entirely able
> > > to shutdown or to reboot (which is mostly what I care about) Linux from
> > > OpenRISC. It just hangs.
> > 
> > This kind of thing needs to be either:
> >  (1) we're modelling real hardware and that real hardware has a
> > device or other mechanism guest code can prod to cause a power-off
> > or reboot. Then we model that device, and guest code triggers a
> > shutdown or reboot exactly as it would on the real hardware.
> >  (2) there is an architecturally defined ABI for simulators, debug
> > stubs, etc, that includes various operations typically including
> > an "exit the simulator" function. (Arm semihosting is an example
> > of this.) In that case we can implement that functionality,
> > guarded by and controlled by the appropriate command line options.
> > (This is generally not as nice as option 1, because the guest code
> > has to be compiled to have support for semihosting and also because
> > turning it on is usually also giving implicit permission for the
> > guest code to read and write arbitrary host files, etc.)
> > 
> > Either way, undocumented random hacks aren't a good idea, which
> > is why this wasn't merged.
> 
> Yes, this is what was brought up before.  At that time semihosting was mentioned
> and I tried to understand what it was but didn't really understand it as a general
> concept.  Is this something arm specific?
> 
> Since the qemu or1k-sim defines our "simulator", I suspect I could add a
> definition of our simulator ABI to the OpenRISC architecture specification.  The
> simulation uses of l.nop N as ABI hooks is a de-facto standard for OpenRISC.
> From the way you describe this now I take it if we document this as a
> architecture simulation ABI the patch would be accepted.

If that's what it takes, then that'd make sense.

By the way, would this also help the reboot case? That's
`reboot(RB_AUTOBOOT);`, which does:

machine_restart() ->
  do_kernel_restart() ->
    atomic_notifier_chain_register(&restart_handler_list, nb) ->
      ???

As far as I can tell, nothing is wired into the reboot case for
OpenRISC. Is this something that could be fixed in the kernel without
having to patch QEMU? If so, then I could effectively get shutdown for
my CI with the -no-reboot option, which is what I'm already doing for a
few platforms.

Jason

Re: [Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Jason A. Donenfeld]

Hey again,

On Thu, Apr 28, 2022 at 02:04:29AM +0200, Jason A. Donenfeld wrote:
> By the way, would this also help the reboot case? That's
> `reboot(RB_AUTOBOOT);`, which does:
> 
> machine_restart() ->
>   do_kernel_restart() ->
>     atomic_notifier_chain_register(&restart_handler_list, nb) ->
>       ???
> 
> As far as I can tell, nothing is wired into the reboot case for
> OpenRISC. Is this something that could be fixed in the kernel without
> having to patch QEMU? If so, then I could effectively get shutdown for
> my CI with the -no-reboot option, which is what I'm already doing for a
> few platforms.

I added 13 for this: https://lists.librecores.org/pipermail/openrisc/2022-April/003884.html

When you go add these nops to the specification, please remember to add
one for reboot too. Then, once that kernel code is merged and the
specification published, it'll be sensible to add shutdown and reboot
support to QEMU, per Peter's description.

Jason

Re: [Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Stafford Horne]

On Thu, Apr 28, 2022 at 01:16:51PM +0200, Jason A. Donenfeld wrote:
> Hey again,
> 
> On Thu, Apr 28, 2022 at 02:04:29AM +0200, Jason A. Donenfeld wrote:
> > By the way, would this also help the reboot case? That's
> > `reboot(RB_AUTOBOOT);`, which does:
> > 
> > machine_restart() ->
> >   do_kernel_restart() ->
> >     atomic_notifier_chain_register(&restart_handler_list, nb) ->
> >       ???
> > 
> > As far as I can tell, nothing is wired into the reboot case for
> > OpenRISC. Is this something that could be fixed in the kernel without
> > having to patch QEMU? If so, then I could effectively get shutdown for
> > my CI with the -no-reboot option, which is what I'm already doing for a
> > few platforms.
> 
> I added 13 for this: https://lists.librecores.org/pipermail/openrisc/2022-April/003884.html
> 
> When you go add these nops to the specification, please remember to add
> one for reboot too. Then, once that kernel code is merged and the
> specification published, it'll be sensible to add shutdown and reboot
> support to QEMU, per Peter's description.

This sounds fair.

-Stafford

Re: [Qemu-devel] [PATCH 2/7] target/openrisc: add shutdown logic

[Peter Maydell]

On Wed, 27 Apr 2022 at 23:27, Stafford Horne <shorne@gmail.com> wrote:
> Yes, this is what was brought up before.  At that time semihosting was mentioned
> and I tried to understand what it was but didn't really understand it as a general
> concept.  Is this something arm specific?

QEMU uses "semihosting" for the general concept of a syscall-like
ABI provided by the model that allows guest code written to use it
to access some facilities as if it were a program running on the host
rather than running on bare metal. (I think the term derives originally
from the Arm term for this kind of functionality, but the concept is
not Arm-specific.)

Arm defines an ABI which looks basically like a set of syscalls:
code sets up some registers and executes a specific SVC or HLT
or other magic instruction, and the implementation is supposed to
then act on that. You can do things like "open file", "read file",
"exit", etc.
 https://github.com/ARM-software/abi-aa/blob/main/semihosting/semihosting.rst
The idea is that simulators and also debug stubs or debuggers can
implement this, and then bare-metal code can be written to use it,
mainly for debugging and test case purposes.

The risc-v folks decided they needed similar functionality, and
that the easiest way to do this was to align with the Arm specification
and document the risc-v specific bits:
https://github.com/riscv/riscv-semihosting-spec

Some other architectures have an equivalent thing but which isn't
the same set of functions as the Arm version; eg the nios2 version
is documented here:
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=libgloss/nios2/nios2-semi.txt;hb=HEAD

> Since the qemu or1k-sim defines our "simulator", I suspect I could add a
> definition of our simulator ABI to the OpenRISC architecture specification.  The
> simulation uses of l.nop N as ABI hooks is a de-facto standard for OpenRISC.
> From the way you describe this now I take it if we document this as a
> architecture simulation ABI the patch would be accepted.

If it's something that (a) is documented officially somewhere and
(b) everybody is using consistently (ie guest code such as GNU newlib,
QEMU, other simulators, etc), then yes, that's OK. It sounds like
you just need to write down the details of your de-facto standard
to turn it into a de-jure one :-)

We would want to guard it behind the existing semihosting command
line option, rather than having it enabled all the time, but that
part should be straightforward.

-- PMM

[Qemu-devel] [PATCH 3/7] target/openrisc: add numcores and coreid support

[Stafford Horne]

These are used to identify the processor in SMP system.  Their
definition has been defined in verilog cores but it not yet part of the
spec but it will be soon.

The proposal for this is available:
  https://openrisc.io/proposals/core-identifier-and-number-of-cores

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 target/openrisc/sys_helper.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index 2eaff87..bd5051b 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -227,6 +227,12 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
     case TO_SPR(0, 64): /* ESR */
         return env->esr;
 
+    case TO_SPR(0, 128): /* COREID */
+        return 0;
+
+    case TO_SPR(0, 129): /* NUMCORES */
+        return 1;
+
     case TO_SPR(1, 512) ... TO_SPR(1, 512+DTLB_SIZE-1): /* DTLBW0MR 0-127 */
         idx = spr - TO_SPR(1, 512);
         return env->tlb->dtlb[0][idx].mr;
-- 
2.9.3

Re: [Qemu-devel] [PATCH 3/7] target/openrisc: add numcores and coreid support

[Richard Henderson]

On 04/16/2017 04:23 PM, Stafford Horne wrote:
> These are used to identify the processor in SMP system.  Their
> definition has been defined in verilog cores but it not yet part of the
> spec but it will be soon.
>
> The proposal for this is available:
>   https://openrisc.io/proposals/core-identifier-and-number-of-cores
>
> Signed-off-by: Stafford Horne <shorne@gmail.com>

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 4/7] target/openrisc: implement shadow registers

[Stafford Horne]

Shadow registers are part of the openrisc spec along with sr[cid], as
part of the fast context switching feature.  When exceptions occur,
instead of having to save registers to the stack if enabled the CID will
increment and a new set of registers will be available.

This patch only implements shadow registers which can be used as extra
scratch registers via the mfspr and mtspr if required.  This is
implemented in a way where it would be easy to add on the fast context
switching, currently cid is hardcoded to 0.

This is need for openrisc linux smp kernels to boot correctly.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 target/openrisc/cpu.c        |  5 ++++-
 target/openrisc/cpu.h        |  4 +++-
 target/openrisc/machine.c    | 16 +++++++++++++---
 target/openrisc/sys_helper.c |  9 +++++++++
 target/openrisc/translate.c  |  3 ++-
 5 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 7fd2b9a..dc8be21 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -46,13 +46,14 @@ static void openrisc_cpu_reset(CPUState *s)
 
     memset(&cpu->env, 0, offsetof(CPUOpenRISCState, end_reset_fields));
 
+    cpu->env.gpr = cpu->env.shadow_gpr[0];
     cpu->env.pc = 0x100;
     cpu->env.sr = SR_FO | SR_SM;
     cpu->env.lock_addr = -1;
     s->exception_index = -1;
 
     cpu->env.upr = UPR_UP | UPR_DMP | UPR_IMP | UPR_PICP | UPR_TTP;
-    cpu->env.cpucfgr = CPUCFGR_OB32S | CPUCFGR_OF32S;
+    cpu->env.cpucfgr = CPUCFGR_OB32S | CPUCFGR_OF32S | CPUCFGR_NSGF;
     cpu->env.dmmucfgr = (DMMUCFGR_NTW & (0 << 2)) | (DMMUCFGR_NTS & (6 << 2));
     cpu->env.immucfgr = (IMMUCFGR_NTW & (0 << 2)) | (IMMUCFGR_NTS & (6 << 2));
 
@@ -132,6 +133,7 @@ static void or1200_initfn(Object *obj)
 {
     OpenRISCCPU *cpu = OPENRISC_CPU(obj);
 
+    set_feature(cpu, OPENRISC_FEATURE_NSGF);
     set_feature(cpu, OPENRISC_FEATURE_OB32S);
     set_feature(cpu, OPENRISC_FEATURE_OF32S);
 }
@@ -140,6 +142,7 @@ static void openrisc_any_initfn(Object *obj)
 {
     OpenRISCCPU *cpu = OPENRISC_CPU(obj);
 
+    set_feature(cpu, OPENRISC_FEATURE_NSGF);
     set_feature(cpu, OPENRISC_FEATURE_OB32S);
 }
 
diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
index 418a0e6..78675e0 100644
--- a/target/openrisc/cpu.h
+++ b/target/openrisc/cpu.h
@@ -269,7 +269,9 @@ typedef struct CPUOpenRISCTLBContext {
 #endif
 
 typedef struct CPUOpenRISCState {
-    target_ulong gpr[32];     /* General registers */
+    target_ulong shadow_gpr[16][32]; /* Shadow registers */
+    target_ulong * gpr;       /* General registers (backed by shadow) */
+
     target_ulong pc;          /* Program counter */
     target_ulong ppc;         /* Prev PC */
     target_ulong jmp_pc;      /* Jump PC */
diff --git a/target/openrisc/machine.c b/target/openrisc/machine.c
index 686eaa3..27cc751 100644
--- a/target/openrisc/machine.c
+++ b/target/openrisc/machine.c
@@ -24,6 +24,15 @@
 #include "hw/boards.h"
 #include "migration/cpu.h"
 
+static int env_post_load(void *opaque, int version_id)
+{
+    CPUOpenRISCState *env = opaque;
+
+    env->gpr = env->shadow_gpr[0];
+
+    return 0;
+}
+
 static int get_sr(QEMUFile *f, void *opaque, size_t size, VMStateField *field)
 {
     CPUOpenRISCState *env = opaque;
@@ -47,10 +56,11 @@ static const VMStateInfo vmstate_sr = {
 
 static const VMStateDescription vmstate_env = {
     .name = "env",
-    .version_id = 4,
-    .minimum_version_id = 4,
+    .version_id = 5,
+    .minimum_version_id = 5,
+    .post_load = env_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_UINTTL_ARRAY(gpr, CPUOpenRISCState, 32),
+        VMSTATE_UINTTL_2DARRAY(shadow_gpr, CPUOpenRISCState, 16, 32),
         VMSTATE_UINTTL(pc, CPUOpenRISCState),
         VMSTATE_UINTTL(ppc, CPUOpenRISCState),
         VMSTATE_UINTTL(jmp_pc, CPUOpenRISCState),
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index bd5051b..04ff302 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -89,6 +89,11 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
     case TO_SPR(0, 64): /* ESR */
         env->esr = rb;
         break;
+
+    case TO_SPR(0, 1024) ... TO_SPR(0, 1024 + (16 * 32)): /* Shadow GPRs */
+        idx = (spr - 1024);
+        env->shadow_gpr[idx / 32][idx % 32] = rb;
+
     case TO_SPR(1, 512) ... TO_SPR(1, 512+DTLB_SIZE-1): /* DTLBW0MR 0-127 */
         idx = spr - TO_SPR(1, 512);
         if (!(rb & 1)) {
@@ -233,6 +238,10 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
     case TO_SPR(0, 129): /* NUMCORES */
         return 1;
 
+    case TO_SPR(0, 1024) ... TO_SPR(0, 1024 + (16 * 32)): /* Shadow GPRs */
+        idx = (spr - 1024);
+        return env->shadow_gpr[idx / 32][idx % 32];
+
     case TO_SPR(1, 512) ... TO_SPR(1, 512+DTLB_SIZE-1): /* DTLBW0MR 0-127 */
         idx = spr - TO_SPR(1, 512);
         return env->tlb->dtlb[0][idx].mr;
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 74df245..c7261cb 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -107,7 +107,8 @@ void openrisc_translate_init(void)
                                      "mac");
     for (i = 0; i < 32; i++) {
         cpu_R[i] = tcg_global_mem_new(cpu_env,
-                                      offsetof(CPUOpenRISCState, gpr[i]),
+                                      offsetof(CPUOpenRISCState,
+                                               shadow_gpr[0][i]),
                                       regnames[i]);
     }
     cpu_R0 = cpu_R[0];
-- 
2.9.3

Re: [Qemu-devel] [PATCH 4/7] target/openrisc: implement shadow registers

[Richard Henderson]

On 04/16/2017 04:23 PM, Stafford Horne wrote:
> Shadow registers are part of the openrisc spec along with sr[cid], as
> part of the fast context switching feature.  When exceptions occur,
> instead of having to save registers to the stack if enabled the CID will
> increment and a new set of registers will be available.
>
> This patch only implements shadow registers which can be used as extra
> scratch registers via the mfspr and mtspr if required.  This is
> implemented in a way where it would be easy to add on the fast context
> switching, currently cid is hardcoded to 0.

I'm not especially keen on this half-conversion.
If CID cannot be changed, then

> -    target_ulong gpr[32];     /* General registers */
> +    target_ulong shadow_gpr[16][32]; /* Shadow registers */
> +    target_ulong * gpr;       /* General registers (backed by shadow) */

this pointer should not be necessary.  Just use a union, or even just

     target_ulong gpr[32];
     target_ulong shadow_gpr[15][32];

for now.

Alternately, add accessor functions that take the whole ENV (which would be 
able to read CID, when needed).  C.f.

     uint64_t cpu_alpha_load_gr(CPUAlphaState *env, unsigned reg);
     void cpu_alpha_store_gr(CPUAlphaState *env, unsigned reg, uint64_t val);

If/when CID can be changed, then we can talk about various ways
that this can be modeled within TCG.


r~

Re: [Qemu-devel] [PATCH 4/7] target/openrisc: implement shadow registers

[Stafford Horne]

On Tue, Apr 18, 2017 at 01:11:53AM -0700, Richard Henderson wrote:
> On 04/16/2017 04:23 PM, Stafford Horne wrote:
> > Shadow registers are part of the openrisc spec along with sr[cid], as
> > part of the fast context switching feature.  When exceptions occur,
> > instead of having to save registers to the stack if enabled the CID will
> > increment and a new set of registers will be available.
> > 
> > This patch only implements shadow registers which can be used as extra
> > scratch registers via the mfspr and mtspr if required.  This is
> > implemented in a way where it would be easy to add on the fast context
> > switching, currently cid is hardcoded to 0.
> 
> I'm not especially keen on this half-conversion.
> If CID cannot be changed, then
> 
> > -    target_ulong gpr[32];     /* General registers */
> > +    target_ulong shadow_gpr[16][32]; /* Shadow registers */
> > +    target_ulong * gpr;       /* General registers (backed by shadow) */
> 
> this pointer should not be necessary.  Just use a union, or even just
> 
>     target_ulong gpr[32];
>     target_ulong shadow_gpr[15][32];
> 
> for now.
> 
> Alternately, add accessor functions that take the whole ENV (which would be
> able to read CID, when needed).  C.f.
> 
>     uint64_t cpu_alpha_load_gr(CPUAlphaState *env, unsigned reg);
>     void cpu_alpha_store_gr(CPUAlphaState *env, unsigned reg, uint64_t val);

Actually, I started off writing it this way, but there were a lot of places
that used env->gpr.  I had some regex's to replace everything, but in the
end I thought using the gpr pointer was just easier.

Using the union would work as well, but it wouldnt help to allow switching
of cid.  My idea with the pointer was that if cid was updates I could
update the pointer at that time.

Ill rework this to use the accessor functions.

-Stafford

> If/when CID can be changed, then we can talk about various ways
> that this can be modeled within TCG.
> 
> 
> r~

[Qemu-devel] [PATCH 5/7] migration: Add VMSTATE_UINTTL_2DARRAY()

[Stafford Horne]

In openRISC we are implementing the shadow registers as a 2d array.
Using this target long method rather than direct 32-bit alternatives is
consistent with the rest of our vm state serialization logic.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 include/migration/cpu.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/migration/cpu.h b/include/migration/cpu.h
index f3d5dfc..a40bd35 100644
--- a/include/migration/cpu.h
+++ b/include/migration/cpu.h
@@ -18,6 +18,8 @@
     VMSTATE_UINT64_EQUAL_V(_f, _s, _v)
 #define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)                        \
     VMSTATE_UINT64_ARRAY_V(_f, _s, _n, _v)
+#define VMSTATE_UINTTL_2DARRAY_V(_f, _s, _n1, _n2, _v)                \
+    VMSTATE_UINT64_2DARRAY_V(_f, _s, _n1, _n2, _v)
 #define VMSTATE_UINTTL_TEST(_f, _s, _t)                               \
     VMSTATE_UINT64_TEST(_f, _s, _t)
 #define vmstate_info_uinttl vmstate_info_uint64
@@ -37,6 +39,8 @@
     VMSTATE_UINT32_EQUAL_V(_f, _s, _v)
 #define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)                        \
     VMSTATE_UINT32_ARRAY_V(_f, _s, _n, _v)
+#define VMSTATE_UINTTL_2DARRAY_V(_f, _s, _n1, _n2, _v)                \
+    VMSTATE_UINT32_2DARRAY_V(_f, _s, _n1, _n2, _v)
 #define VMSTATE_UINTTL_TEST(_f, _s, _t)                               \
     VMSTATE_UINT32_TEST(_f, _s, _t)
 #define vmstate_info_uinttl vmstate_info_uint32
@@ -48,5 +52,8 @@
     VMSTATE_UINTTL_EQUAL_V(_f, _s, 0)
 #define VMSTATE_UINTTL_ARRAY(_f, _s, _n)                              \
     VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, 0)
+#define VMSTATE_UINTTL_2DARRAY(_f, _s, _n1, _n2)                      \
+    VMSTATE_UINTTL_2DARRAY_V(_f, _s, _n1, _n2, 0)
+
 
 #endif
-- 
2.9.3

[Qemu-devel] [PATCH 6/7] migration: Add VMSTATE_STRUCT_2DARRAY()

[Stafford Horne]

For openrisc we implement tlb state as a 2d array of tlb entry structs.
This is added to allow easy storing of state of 2d arrays.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 include/migration/vmstate.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index f2dbf84..9b7dcdc 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -499,6 +499,17 @@ extern const VMStateInfo vmstate_info_qtailq;
     .offset       = vmstate_offset_array(_state, _field, _type, _num),\
 }
 
+#define VMSTATE_STRUCT_2DARRAY_TEST(_field, _state, _n1, _n2, _test, _version, _vmsd, _type) { \
+    .name         = (stringify(_field)),                                    \
+    .num          = (_n1) * (_n2),                                          \
+    .field_exists = (_test),                                                \
+    .version_id   = (_version),                                             \
+    .vmsd         = &(_vmsd),                                               \
+    .size         = sizeof(_type),                                          \
+    .flags        = VMS_STRUCT|VMS_ARRAY,                                   \
+    .offset       = vmstate_offset_2darray(_state, _field, _type, _n1, _n2),\
+}
+
 #define VMSTATE_STRUCT_VARRAY_UINT8(_field, _state, _field_num, _version, _vmsd, _type) { \
     .name       = (stringify(_field)),                               \
     .num_offset = vmstate_offset_value(_state, _field_num, uint8_t), \
@@ -746,6 +757,10 @@ extern const VMStateInfo vmstate_info_qtailq;
     VMSTATE_STRUCT_ARRAY_TEST(_field, _state, _num, NULL, _version,   \
             _vmsd, _type)
 
+#define VMSTATE_STRUCT_2DARRAY(_field, _state, _n1, _n2, _version, _vmsd, _type) \
+    VMSTATE_STRUCT_2DARRAY_TEST(_field, _state, _n1, _n2, NULL, _version,   \
+            _vmsd, _type)
+
 #define VMSTATE_BUFFER_UNSAFE_INFO(_field, _state, _version, _info, _size) \
     VMSTATE_BUFFER_UNSAFE_INFO_TEST(_field, _state, NULL, _version, _info, \
             _size)
-- 
2.9.3

[Qemu-devel] [PATCH 7/7] target/openrisc: Implement full vmstate serialization

[Stafford Horne]

Previously serialization did not persist the tlb, timer, pic and other
key state items.  This meant snapshotting and restoring a running os
would crash. After adding these I am able to take snapshots of a
running linux os and restore at a later time.

I am currently not trying to maintain capatibility with older versions
as I do not believe this really worked before or anyone used it.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 target/openrisc/machine.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/target/openrisc/machine.c b/target/openrisc/machine.c
index 27cc751..2adcda8 100644
--- a/target/openrisc/machine.c
+++ b/target/openrisc/machine.c
@@ -30,9 +30,59 @@ static int env_post_load(void *opaque, int version_id)
 
     env->gpr = env->shadow_gpr[0];
 
+    /* Restore MMU handlers */
+    if (env->sr & SR_DME) {
+        env->tlb->cpu_openrisc_map_address_data =
+            &cpu_openrisc_get_phys_data;
+    } else {
+        env->tlb->cpu_openrisc_map_address_data =
+            &cpu_openrisc_get_phys_nommu;
+    }
+
+    if (env->sr & SR_IME) {
+        env->tlb->cpu_openrisc_map_address_code =
+            &cpu_openrisc_get_phys_code;
+    } else {
+        env->tlb->cpu_openrisc_map_address_code =
+            &cpu_openrisc_get_phys_nommu;
+    }
+
+
     return 0;
 }
 
+static const VMStateDescription vmstate_tlb_entry = {
+    .name = "tlb_entry",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINTTL(mr, OpenRISCTLBEntry),
+        VMSTATE_UINTTL(tr, OpenRISCTLBEntry),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_cpu_tlb = {
+    .name = "cpu_tlb",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT_2DARRAY(itlb, CPUOpenRISCTLBContext,
+                             ITLB_WAYS, ITLB_SIZE, 0,
+                             vmstate_tlb_entry, OpenRISCTLBEntry),
+        VMSTATE_STRUCT_2DARRAY(dtlb, CPUOpenRISCTLBContext,
+                             DTLB_WAYS, DTLB_SIZE, 0,
+                             vmstate_tlb_entry, OpenRISCTLBEntry),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+#define VMSTATE_CPU_TLB(_f, _s)                             \
+    VMSTATE_STRUCT_POINTER(_f, _s, vmstate_cpu_tlb, CPUOpenRISCTLBContext)
+
+
 static int get_sr(QEMUFile *f, void *opaque, size_t size, VMStateField *field)
 {
     CPUOpenRISCState *env = opaque;
@@ -92,6 +142,16 @@ static const VMStateDescription vmstate_env = {
         VMSTATE_UINT32(esr, CPUOpenRISCState),
         VMSTATE_UINT32(fpcsr, CPUOpenRISCState),
         VMSTATE_UINT64(mac, CPUOpenRISCState),
+
+        VMSTATE_CPU_TLB(tlb, CPUOpenRISCState),
+
+        VMSTATE_TIMER_PTR(timer, CPUOpenRISCState),
+        VMSTATE_UINT32(ttmr, CPUOpenRISCState),
+        VMSTATE_UINT32(ttcr, CPUOpenRISCState),
+
+        VMSTATE_UINT32(picmr, CPUOpenRISCState),
+        VMSTATE_UINT32(picsr, CPUOpenRISCState),
+
         VMSTATE_END_OF_LIST()
     }
 };
-- 
2.9.3

Re: [Qemu-devel] [PATCH 7/7] target/openrisc: Implement full vmstate serialization

[Richard Henderson]

On 04/16/2017 04:23 PM, Stafford Horne wrote:
> Previously serialization did not persist the tlb, timer, pic and other
> key state items.  This meant snapshotting and restoring a running os
> would crash. After adding these I am able to take snapshots of a
> running linux os and restore at a later time.
>
> I am currently not trying to maintain capatibility with older versions
> as I do not believe this really worked before or anyone used it.

That's fine, but you still have to bump the version numbers.


r~

Re: [Qemu-devel] [PATCH 7/7] target/openrisc: Implement full vmstate serialization

[Stafford Horne]

On Tue, Apr 18, 2017 at 01:14:19AM -0700, Richard Henderson wrote:
> On 04/16/2017 04:23 PM, Stafford Horne wrote:
> > Previously serialization did not persist the tlb, timer, pic and other
> > key state items.  This meant snapshotting and restoring a running os
> > would crash. After adding these I am able to take snapshots of a
> > running linux os and restore at a later time.
> > 
> > I am currently not trying to maintain capatibility with older versions
> > as I do not believe this really worked before or anyone used it.
> 
> That's fine, but you still have to bump the version numbers.

Actually, I bumped the version in the previous patch.  Ill make that more
clear an do that in this patch instead.

-Stafford

> r~
> 

Re: [Qemu-devel] [PATCH 0/7] Openrisc misc features / fixes

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH 0/7] Openrisc misc features / fixes
Message-id: cover.1492384862.git.shorne@gmail.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/cover.1492384862.git.shorne@gmail.com -> patchew/cover.1492384862.git.shorne@gmail.com
Switched to a new branch 'test'
10e376c target/openrisc: Implement full vmstate serialization
1c965d5 migration: Add VMSTATE_STRUCT_2DARRAY()
66d129f migration: Add VMSTATE_UINTTL_2DARRAY()
da6e29e target/openrisc: implement shadow registers
edd6b74 target/openrisc: add numcores and coreid support
16702b5 target/openrisc: add shutdown logic
c97a00e target/openrisc: Fixes for memory debugging

=== OUTPUT BEGIN ===
Checking PATCH 1/7: target/openrisc: Fixes for memory debugging...
WARNING: line over 80 characters
#44: FILE: target/openrisc/mmu.c:232:
+        miss = cpu_openrisc_get_phys_addr(cpu, &phys_addr, &prot, addr, MMU_INST_FETCH);

total: 0 errors, 1 warnings, 38 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/7: target/openrisc: add shutdown logic...
Checking PATCH 3/7: target/openrisc: add numcores and coreid support...
Checking PATCH 4/7: target/openrisc: implement shadow registers...
ERROR: "foo * bar" should be "foo *bar"
#67: FILE: target/openrisc/cpu.h:273:
+    target_ulong * gpr;       /* General registers (backed by shadow) */

total: 1 errors, 0 warnings, 98 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 5/7: migration: Add VMSTATE_UINTTL_2DARRAY()...
Checking PATCH 6/7: migration: Add VMSTATE_STRUCT_2DARRAY()...
ERROR: line over 90 characters
#20: FILE: include/migration/vmstate.h:502:
+#define VMSTATE_STRUCT_2DARRAY_TEST(_field, _state, _n1, _n2, _test, _version, _vmsd, _type) { \

ERROR: spaces required around that '|' (ctx:VxV)
#27: FILE: include/migration/vmstate.h:509:
+    .flags        = VMS_STRUCT|VMS_ARRAY,                                   \
                               ^

WARNING: line over 80 characters
#38: FILE: include/migration/vmstate.h:760:
+#define VMSTATE_STRUCT_2DARRAY(_field, _state, _n1, _n2, _version, _vmsd, _type) \

total: 2 errors, 1 warnings, 27 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 7/7: target/openrisc: Implement full vmstate serialization...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org