Openrisc misc features / fixes

PATCH Superseded Stafford Horne 5 participants 35 messages Apr 16, 2017 – Apr 28, 2022

lore

Share & tools

lore

Stafford Horne

author

2017-04-16 23:23 UTC #0 lore raw

cover Openrisc misc features / fixes

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(-)

Signature · 1 line

2.9.3

Stafford Horne

author

2017-04-16 23:23 UTC #1 lore raw

PATCH 1/7 target/openrisc: Fixes for memory debugging

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 <[email protected]>

---
 target/openrisc/mmu.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

target/openrisc/mmu.c +19 -4

index 56b11d3..a6d7bcd 100644
@@ -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)

Signature · 1 line

2.9.3

Stafford Horne

author

2017-04-16 23:23 UTC #6 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

In reply to 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 <[email protected]>

Signed-off-by Stafford Horne <[email protected]>

---
 target/openrisc/helper.h     |  1 +
 target/openrisc/sys_helper.c | 17 +++++++++++++++++
 target/openrisc/translate.c  |  5 ++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

target/openrisc/helper.h +1 -0

index 4fd1a6b..b7838f5 100644
@@ -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)

target/openrisc/sys_helper.c +17 -0

index 60c3193..2eaff87 100644
@@ -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
+}

target/openrisc/translate.c +4 -1

index 7c4cbf2..74df245 100644
@@ -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;

Signature · 1 line

2.9.3

Stafford Horne

author

2017-04-16 23:23 UTC #24 lore raw

PATCH 3/7 target/openrisc: add numcores and coreid support

In reply to 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 <[email protected]>

---
 target/openrisc/sys_helper.c | 6 ++++++
 1 file changed, 6 insertions(+)

target/openrisc/sys_helper.c +6 -0

index 2eaff87..bd5051b 100644
@@ -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;

Signature · 1 line

2.9.3

Stafford Horne

author

2017-04-16 23:23 UTC #26 lore raw

PATCH 4/7 target/openrisc: implement shadow registers

In reply to 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 <[email protected]>

---
 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(-)

target/openrisc/cpu.c +4 -1

index 7fd2b9a..dc8be21 100644
@@ -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);
 }

target/openrisc/cpu.h +3 -1

index 418a0e6..78675e0 100644
@@ -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 */

target/openrisc/machine.c +13 -3

index 686eaa3..27cc751 100644
@@ -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),

target/openrisc/sys_helper.c +9 -0

index bd5051b..04ff302 100644
@@ -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;

target/openrisc/translate.c +2 -1

index 74df245..c7261cb 100644
@@ -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];

Signature · 1 line

2.9.3

Stafford Horne

author

2017-04-16 23:23 UTC #29 lore raw

PATCH 5/7 migration: Add VMSTATE_UINTTL_2DARRAY()

In reply to 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 <[email protected]>

---
 include/migration/cpu.h | 7 +++++++
 1 file changed, 7 insertions(+)

include/migration/cpu.h +7 -0

index f3d5dfc..a40bd35 100644
@@ -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

Signature · 1 line

2.9.3

Stafford Horne

author

2017-04-16 23:23 UTC #30 lore raw

PATCH 6/7 migration: Add VMSTATE_STRUCT_2DARRAY()

In reply to 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 <[email protected]>

---
 include/migration/vmstate.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

include/migration/vmstate.h +15 -0

index f2dbf84..9b7dcdc 100644
@@ -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)

Signature · 1 line

2.9.3

Stafford Horne

author

2017-04-16 23:23 UTC #31 lore raw

PATCH 7/7 target/openrisc: Implement full vmstate serialization

In reply to 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 <[email protected]>

---
 target/openrisc/machine.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

target/openrisc/machine.c +60 -0

index 27cc751..2adcda8 100644
@@ -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()
     }
 };

Signature · 1 line

2.9.3

[email protected]

bot

2017-04-16 23:33 UTC #34 lore raw

In reply to Stafford Horne

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

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 at 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 at gmail.com -> patchew/cover.1492384862.git.shorne at 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 at freelists.org

Richard Henderson

2017-04-18 07:47 UTC #2 lore raw

PATCH 1/7 target/openrisc: Fixes for memory debugging

In reply to Stafford Horne

Quoted 9 lines · 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 <[email protected]>

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 <[email protected]>

Richard Henderson

2017-04-18 07:52 UTC #7 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

In reply to Stafford Horne

Quoted 5 lines · 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 <[email protected]>
Signed-off-by Stafford Horne <[email protected]>

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.

target/openrisc/translate.c static void dec_misc(DisasContext *dc, uint32_t insn) · line 761

Quoted 6 lines

         case 0x01:    /* l.nop */
             LOG_DIS("l.nop %d\n", I16);

target/openrisc/translate.c +4 -0

+            {
+                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.

Richard Henderson

2017-04-18 08:01 UTC #25 lore raw

PATCH 3/7 target/openrisc: add numcores and coreid support

In reply to Stafford Horne

Quoted 8 lines · 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 <[email protected]>

Reviewed-by Richard Henderson <[email protected]>

Richard Henderson

2017-04-18 08:11 UTC #27 lore raw

PATCH 4/7 target/openrisc: implement shadow registers

In reply to Stafford Horne

Quoted 9 lines · 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.

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

target/openrisc/cpu.h typedef struct CPUOpenRISCTLBContext { · line 273

Quoted 3 lines

target/openrisc/cpu.h +2 -1

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

Richard Henderson

2017-04-18 08:14 UTC #32 lore raw

PATCH 7/7 target/openrisc: Implement full vmstate serialization

In reply to Stafford Horne

Quoted 7 lines · 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.

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

Stafford Horne

author

2017-04-18 14:18 UTC #3 lore raw

PATCH 1/7 target/openrisc: Fixes for memory debugging

In reply to Richard Henderson

Quoted 13 lines · Richard Henderson

Quoted 9 lines · 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 <[email protected]>
Oh the horrors of a software managed TLB.

Quoted 8 lines

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

Quoted 3 lines

That said,
Reviewed-by Richard Henderson <[email protected]>

Thanks for the review.

-Stafford

Quoted 2 lines

r~

Stafford Horne

author

2017-04-18 14:20 UTC #8 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

In reply to Richard Henderson

Quoted 11 lines · Richard Henderson

Quoted 5 lines · 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 <[email protected]>
Signed-off-by Stafford Horne <[email protected]>
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.

Quoted 10 lines

target/openrisc/translate.c static void dec_misc(DisasContext *dc, uint32_t insn) · line 761
Quoted 6 lines
         case 0x01:    /* l.nop */
             LOG_DIS("l.nop %d\n", I16);
target/openrisc/translate.c +4 -0
+            {
+                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

Quoted 2 lines

r~

Stafford Horne

author

2017-04-18 14:26 UTC #28 lore raw

PATCH 4/7 target/openrisc: implement shadow registers

In reply to Richard Henderson

Quoted 23 lines · Richard Henderson

Quoted 9 lines · 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.
I'm not especially keen on this half-conversion.
If CID cannot be changed, then
target/openrisc/cpu.h typedef struct CPUOpenRISCTLBContext { · line 273
Quoted 3 lines
target/openrisc/cpu.h +2 -1
-    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];

Quoted 7 lines

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

Quoted 5 lines

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

Stafford Horne

author

2017-04-18 14:27 UTC #33 lore raw

PATCH 7/7 target/openrisc: Implement full vmstate serialization

In reply to Richard Henderson

Quoted 10 lines · Richard Henderson

Quoted 7 lines · 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.
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

Quoted 2 lines

r~

Richard Henderson

2017-04-18 15:00 UTC #4 lore raw

PATCH 1/7 target/openrisc: Fixes for memory debugging

In reply to Stafford Horne

Quoted 18 lines · Stafford Horne

Quoted 17 lines · Richard Henderson
Quoted 9 lines · 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 <[email protected]>
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.

Quoted 7 lines

Quoted 4 lines
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?

Stafford Horne

author

2017-04-19 20:06 UTC #5 lore raw

PATCH 1/7 target/openrisc: Fixes for memory debugging

Quoted 24 lines · Richard Henderson

Quoted 23 lines · Stafford Horne
Quoted 21 lines · Richard Henderson
Quoted 9 lines · 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 <[email protected]>
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.

Quoted 6 lines

Quoted 2 lines
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

Stafford Horne

author

2017-04-22 10:09 UTC #9 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

In reply to Stafford Horne

Quoted 22 lines · Stafford Horne

Quoted 11 lines · Richard Henderson
Quoted 5 lines · 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 <[email protected]>
Signed-off-by Stafford Horne <[email protected]>
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.
Quoted 7 lines
Quoted 6 lines
         case 0x01:    /* l.nop */
             LOG_DIS("l.nop %d\n", I16);
target/openrisc/translate.c +4 -0
+            {
+                TCGv_i32 arg = tcg_const_i32(I16);
+                gen_helper_nop(arg);
+            }

Quoted 5 lines

Quoted 3 lines
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

Richard Henderson

2017-04-22 15:25 UTC #10 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

Quoted 3 lines · Stafford Horne

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.

Quoted 3 lines

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.

Stafford Horne

author

2017-04-23 21:28 UTC #11 lore raw

PATCH 1/1 target/openrisc: Support non-busy idle state using PMR SPR

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 <[email protected]>

---

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(-)

target/openrisc/cpu.c +2 -1

index c9b3f22..1d6330c 100644
@@ -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));

target/openrisc/cpu.h +10 -0

index 938ccc3..2721432 100644
@@ -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;

target/openrisc/interrupt.c +2 -0

index 2c91fab..3959671 100644
@@ -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;

target/openrisc/machine.c +1 -0

index a82be62..a20cce7 100644
@@ -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),

target/openrisc/sys_helper.c +12 -0

index fa3d6a4..cb1e085 100644
@@ -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;

Signature · 1 line

2.9.3

Stafford Horne

author

2017-04-23 21:54 UTC #12 lore raw

RFC PATCH 1/1 target/openrisc: Support non-busy idle state using PMR SPR

In reply to Richard Henderson

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 <[email protected]>

---
(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(-)

target/openrisc/cpu.c +2 -1

index c9b3f22..1d6330c 100644
@@ -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));

target/openrisc/cpu.h +10 -0

index 938ccc3..2721432 100644
@@ -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;

target/openrisc/interrupt.c +2 -0

index 2c91fab..3959671 100644
@@ -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;

target/openrisc/machine.c +1 -0

index a82be62..a20cce7 100644
@@ -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),

target/openrisc/sys_helper.c +12 -0

index fa3d6a4..cb1e085 100644
@@ -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;

Signature · 1 line

2.9.3

Richard Henderson

2017-04-25 10:11 UTC #13 lore raw

RFC PATCH 1/1 target/openrisc: Support non-busy idle state using PMR SPR

Quoted 9 lines · 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.

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.

Quoted 6 lines

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>

...

Quoted 1 line

+ 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.

Stafford Horne

author

2017-04-25 14:10 UTC #14 lore raw

RFC PATCH v2 1/1 target/openrisc: Support non-busy idle state using PMR SPR

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 <[email protected]>

---
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(-)

hw/openrisc/cputimer.c +1 -0

index a98c799..febc469 100644
@@ -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)

target/openrisc/cpu.c +2 -1

index c9b3f22..1d6330c 100644
@@ -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));

target/openrisc/cpu.h +10 -0

index 938ccc3..2721432 100644
@@ -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;

target/openrisc/interrupt.c +2 -0

index 2c91fab..3959671 100644
@@ -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;

target/openrisc/machine.c +1 -0

index a82be62..a20cce7 100644
@@ -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),

target/openrisc/sys_helper.c +13 -0

index fa3d6a4..44acf0d 100644
@@ -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;

Signature · 1 line

2.9.3

Stafford Horne

author

2017-04-25 14:18 UTC #15 lore raw

RFC PATCH 1/1 target/openrisc: Support non-busy idle state using PMR SPR

In reply to Richard Henderson

Quoted 9 lines · Richard Henderson

Quoted 8 lines · 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

Quoted 5 lines

Quoted 1 line
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.

Quoted 9 lines

Quoted 6 lines
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>
...
Quoted 1 line
+ cpu_restore_state(cs, GETPC() + 4);

Quoted 7 lines

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

Richard Henderson

2017-04-25 14:51 UTC #16 lore raw

RFC PATCH 1/1 target/openrisc: Support non-busy idle state using PMR SPR

Quoted 30 lines · Stafford Horne

Quoted 9 lines · Richard Henderson
Quoted 8 lines · 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
Quoted 5 lines
Quoted 1 line
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.
Quoted 11 lines
Quoted 6 lines
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>
...
Quoted 1 line
+ cpu_restore_state(cs, GETPC() + 4);
This isn't correct. You want

Quoted 8 lines

Quoted 5 lines
	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.

Quoted 5 lines

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.

Jason A. Donenfeld

2022-04-27 17:44 UTC #17 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

In reply to Stafford Horne

Hey Stafford,

Quoted 5 lines · 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 <[email protected]>
Signed-off-by Stafford Horne <[email protected]>

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

Peter Maydell

2022-04-27 18:47 UTC #18 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

Quoted 7 lines · Jason A. Donenfeld

Hey Stafford,
Quoted 3 lines · Stafford Horne
In openrisc simulators we use hooks like 'l.nop 1' to cause the simulator to exit. Implement that for qemu too.

Quoted 6 lines

Quoted 2 lines
Reported-by Waldemar Brodkorb <[email protected]>
Signed-off-by Stafford Horne <[email protected]>
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

Stafford Horne

author

2022-04-27 21:48 UTC #19 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

Quoted 30 lines · Peter Maydell

Quoted 7 lines · Jason A. Donenfeld
Hey Stafford,
Quoted 3 lines · Stafford Horne
In openrisc simulators we use hooks like 'l.nop 1' to cause the simulator to exit. Implement that for qemu too.
Quoted 6 lines
Quoted 2 lines
Reported-by Waldemar Brodkorb <[email protected]>
Signed-off-by Stafford Horne <[email protected]>
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.)

Quoted 2 lines

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

Jason A. Donenfeld

2022-04-28 00:04 UTC #20 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

Hi Stafford,

Quoted 38 lines · Stafford Horne

Quoted 30 lines · Peter Maydell
Quoted 7 lines · Jason A. Donenfeld
Hey Stafford,
Quoted 3 lines · Stafford Horne
In openrisc simulators we use hooks like 'l.nop 1' to cause the simulator to exit. Implement that for qemu too.
Quoted 6 lines
Quoted 2 lines
Reported-by Waldemar Brodkorb <[email protected]>
Signed-off-by Stafford Horne <[email protected]>
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.)
Quoted 2 lines
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?

Quoted 5 lines

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

Peter Maydell

2022-04-28 09:19 UTC #23 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

In reply to Stafford Horne

Quoted 3 lines · Stafford Horne

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

Quoted 5 lines

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

Jason A. Donenfeld

2022-04-28 11:16 UTC #21 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

In reply to Jason A. Donenfeld

Hey again,

Quoted 5 lines · Jason A. Donenfeld

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

Quoted 8 lines

    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

Stafford Horne

author

2022-04-28 11:47 UTC #22 lore raw

PATCH 2/7 target/openrisc: add shutdown logic

Quoted 17 lines · Jason A. Donenfeld

Hey again,
Quoted 5 lines · Jason A. Donenfeld
By the way, would this also help the reboot case? That's reboot(RB_AUTOBOOT);, which does:
machine_restart() ->
do_kernel_restart() ->
Quoted 8 lines
    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.

Quoted 6 lines

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