Skip to contentSkip to conversation

64bit PowerPC little endian support

PATCH Applied Anton Blanchard 5 participants 19 messages Aug 7, 2013 – Aug 28, 2013

lore
Share & tools
lore
#0 lore raw

cover 64bit PowerPC little endian support

This patchset adds support for 64bit PowerPC little endian on POWER7.

Linux kernel patches to support this were sent out earlier today:

https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-August/109849.html

Anton
--

Anton Blanchard (4):
target-ppc: POWER7 supports the MSR_LE bit
target-ppc: USE LPCR_ILE to control exception endian on POWER7
pseries: Add H_SET_MODE hcall to change guest exception endianness disas/ppc.c: Fix little endian disassembly

Benjamin Herrenschmidt (1):
pseries: Fix loading of little endian kernels

 disas/ppc.c                 |  3 ++-
 hw/ppc/spapr.c              | 15 +++++++++++++--
 hw/ppc/spapr_hcall.c        | 44 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h      | 12 +++++++++++-
 target-ppc/cpu.h            |  2 ++
 target-ppc/excp_helper.c    | 10 ++++++++++
 target-ppc/translate_init.c |  2 +-
 7 files changed, 83 insertions(+), 5 deletions(-)
Signature · 1 line
1.8.1.2
#1 lore raw

PATCH 1/5 target-ppc: POWER7 supports the MSR_LE bit

Add MSR_LE to the msr_mask for POWER7.

---
 target-ppc/translate_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
target-ppc/translate_init.c +1 -1
index b14aec8..33914bc 100644
@@ -7122,7 +7122,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
                        PPC_SEGMENT_64B | PPC_SLBI |
                        PPC_POPCNTB | PPC_POPCNTWD;
     pcc->insns_flags2 = PPC2_VSX | PPC2_DFP | PPC2_DBRX | PPC2_ISA205;
-    pcc->msr_mask = 0x800000000204FF36ULL;
+    pcc->msr_mask = 0x800000000204FF37ULL;
     pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
     pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
Signature · 1 line
1.8.1.2
Reviewed-by on v1 1/5
#2 lore raw

Re: [Qemu-devel] [PATCH 1/5] target-ppc: POWER7 supports the MSR_LE bit

Quoted 3 lines · Anton Blanchard

Add MSR_LE to the msr_mask for POWER7.

Regards,

Anthony Liguori

Quoted 21 lines
---
 target-ppc/translate_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
target-ppc/translate_init.c +1 -2
index b14aec8..33914bc 100644
@@ -7122,7 +7122,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
                        PPC_SEGMENT_64B | PPC_SLBI |
                        PPC_POPCNTB | PPC_POPCNTWD;
     pcc->insns_flags2 = PPC2_VSX | PPC2_DFP | PPC2_DBRX | PPC2_ISA205;
-    pcc->msr_mask = 0x800000000204FF36ULL;
+    pcc->msr_mask = 0x800000000204FF37ULL;
     pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
     pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
--

1.8.1.2

#4 lore raw

PATCH 2/5 target-ppc: USE LPCR_ILE to control exception endian on POWER7

In reply to Anton Blanchard

On POWER7, LPCR_ILE is used to control what endian guests take their exceptions in so use it instead of MSR_ILE.

---
 target-ppc/cpu.h         |  2 ++
 target-ppc/excp_helper.c | 10 ++++++++++
 2 files changed, 12 insertions(+)
target-ppc/cpu.h +2 -0
index 711db08..422a6bb 100644
@@ -453,6 +453,8 @@ struct ppc_slb_t {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define LPCR_ILE (1 << (63-38))
+
 #define msr_sf   ((env->msr >> MSR_SF)   & 1)
 #define msr_isf  ((env->msr >> MSR_ISF)  & 1)
 #define msr_shv  ((env->msr >> MSR_SHV)  & 1)
target-ppc/excp_helper.c +10 -0
index e9fcad8..e957761 100644
@@ -611,9 +611,19 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
         tlb_flush(env, 1);
     }
 
+#ifdef TARGET_PPC64
+    if (excp_model == POWERPC_EXCP_POWER7) {
+        if (env->spr[SPR_LPCR] & LPCR_ILE) {
+            new_msr |= (target_ulong)1 << MSR_LE;
+        }
+    } else if (msr_ile) {
+        new_msr |= (target_ulong)1 << MSR_LE;
+    }
+#else
     if (msr_ile) {
         new_msr |= (target_ulong)1 << MSR_LE;
     }
+#endif
 
     /* Jump to handler */
     vector = env->excp_vectors[excp];
Signature · 1 line
1.8.1.2
Reviewed-by on v1 2/5
#5 lore raw

Re: [Qemu-devel] [PATCH 2/5] target-ppc: USE LPCR_ILE to control exception endian on POWER7

Quoted 4 lines · Anton Blanchard

On POWER7, LPCR_ILE is used to control what endian guests take their exceptions in so use it instead of MSR_ILE.

Regards,

Anthony Liguori

Quoted 46 lines
---
 target-ppc/cpu.h         |  2 ++
 target-ppc/excp_helper.c | 10 ++++++++++
 2 files changed, 12 insertions(+)
target-ppc/cpu.h +2 -0
index 711db08..422a6bb 100644
@@ -453,6 +453,8 @@ struct ppc_slb_t {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define LPCR_ILE (1 << (63-38))
+
 #define msr_sf   ((env->msr >> MSR_SF)   & 1)
 #define msr_isf  ((env->msr >> MSR_ISF)  & 1)
 #define msr_shv  ((env->msr >> MSR_SHV)  & 1)
target-ppc/excp_helper.c +10 -1
index e9fcad8..e957761 100644
@@ -611,9 +611,19 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
         tlb_flush(env, 1);
     }
 
+#ifdef TARGET_PPC64
+    if (excp_model == POWERPC_EXCP_POWER7) {
+        if (env->spr[SPR_LPCR] & LPCR_ILE) {
+            new_msr |= (target_ulong)1 << MSR_LE;
+        }
+    } else if (msr_ile) {
+        new_msr |= (target_ulong)1 << MSR_LE;
+    }
+#else
     if (msr_ile) {
         new_msr |= (target_ulong)1 << MSR_LE;
     }
+#endif
 
     /* Jump to handler */
     vector = env->excp_vectors[excp];
--

1.8.1.2

#7 lore raw

Re: [Qemu-devel] [PATCH 2/5] target-ppc: USE LPCR_ILE to control exception endian on POWER7

In reply to Anton Blanchard

Am 07.08.2013 02:47, schrieb Anton Blanchard:

Quoted 29 lines

On POWER7, LPCR_ILE is used to control what endian guests take their exceptions in so use it instead of MSR_ILE.

---
 target-ppc/cpu.h         |  2 ++
 target-ppc/excp_helper.c | 10 ++++++++++
 2 files changed, 12 insertions(+)
target-ppc/cpu.h +2 -0
index 711db08..422a6bb 100644
@@ -453,6 +453,8 @@ struct ppc_slb_t {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define LPCR_ILE (1 << (63-38))
+
 #define msr_sf   ((env->msr >> MSR_SF)   & 1)
 #define msr_isf  ((env->msr >> MSR_ISF)  & 1)
 #define msr_shv  ((env->msr >> MSR_SHV)  & 1)
target-ppc/excp_helper.c +0 -0
index e9fcad8..e957761 100644
@@ -611,9 +611,19 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
         tlb_flush(env, 1);
     }

target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 622

Quoted 10 lines
target-ppc/excp_helper.c +9 -0
 
+#ifdef TARGET_PPC64
+    if (excp_model == POWERPC_EXCP_POWER7) {
+        if (env->spr[SPR_LPCR] & LPCR_ILE) {
+            new_msr |= (target_ulong)1 << MSR_LE;
+        }
+    } else if (msr_ile) {
+        new_msr |= (target_ulong)1 << MSR_LE;
+    }
+#else

You could just do

...

    } else

#endif

to spare duplicating the msr_ile path.

Andreas

Quoted 8 lines
target-ppc/excp_helper.c +1 -0
     if (msr_ile) {
         new_msr |= (target_ulong)1 << MSR_LE;
     }
+#endif
 
     /* Jump to handler */
     vector = env->excp_vectors[excp];
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
#8 lore raw

Re: [Qemu-devel] [PATCH 2/5] target-ppc: USE LPCR_ILE to control exception endian on POWER7

Quoted 43 lines · Andreas Färber

Am 07.08.2013 02:47, schrieb Anton Blanchard:

Quoted 29 lines

On POWER7, LPCR_ILE is used to control what endian guests take their exceptions in so use it instead of MSR_ILE.

---
 target-ppc/cpu.h         |  2 ++
 target-ppc/excp_helper.c | 10 ++++++++++
 2 files changed, 12 insertions(+)
target-ppc/cpu.h +2 -0
index 711db08..422a6bb 100644
@@ -453,6 +453,8 @@ struct ppc_slb_t {
 #define MSR_RI   1  /* Recoverable interrupt                        1        */
 #define MSR_LE   0  /* Little-endian mode                           1 hflags */
 
+#define LPCR_ILE (1 << (63-38))
+
 #define msr_sf   ((env->msr >> MSR_SF)   & 1)
 #define msr_isf  ((env->msr >> MSR_ISF)  & 1)
 #define msr_shv  ((env->msr >> MSR_SHV)  & 1)
target-ppc/excp_helper.c +0 -0
index e9fcad8..e957761 100644
@@ -611,9 +611,19 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
         tlb_flush(env, 1);
     }

target-ppc/excp_helper.c static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) · line 622

Quoted 10 lines
target-ppc/excp_helper.c +9 -0
 
+#ifdef TARGET_PPC64
+    if (excp_model == POWERPC_EXCP_POWER7) {
+        if (env->spr[SPR_LPCR] & LPCR_ILE) {
+            new_msr |= (target_ulong)1 << MSR_LE;
+        }
+    } else if (msr_ile) {
+        new_msr |= (target_ulong)1 << MSR_LE;
+    }
+#else

You could just do

Quoted 5 lines

...

    } else

#endif

to spare duplicating the msr_ile path.

If the condition was more complicated, I'd agree, but I really dislike splitting statements up via an #ifdef.

Regards,

Anthony Liguori

Quoted 16 lines

Andreas

Quoted 8 lines
     if (msr_ile) {
         new_msr |= (target_ulong)1 << MSR_LE;
     }

+#endif

     /* Jump to handler */
     vector = env->excp_vectors[excp];
Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
#9 lore raw

PATCH 3/5 pseries: Add H_SET_MODE hcall to change guest exception endianness

In reply to Anton Blanchard

H_SET_MODE is used for controlling various partition settings. One of these settings is the endianness a guest takes its exceptions in.

---
 hw/ppc/spapr.c         |  2 +-
 hw/ppc/spapr_hcall.c   | 44 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h | 12 +++++++++++-
 3 files changed, 56 insertions(+), 2 deletions(-)
hw/ppc/spapr.c +1 -1
index 16bfab9..de639f6 100644
@@ -262,7 +262,7 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
     uint32_t start_prop = cpu_to_be32(initrd_base);
     uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
     char hypertas_prop[] = "hcall-pft\0hcall-term\0hcall-dabr\0hcall-interrupt"
-        "\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk";
+        "\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk\0hcall-set-mode";
     char qemu_hypertas_prop[] = "hcall-memop1";
     uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
     uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(smp_cpus)};
hw/ppc/spapr_hcall.c +44 -0

56 lines not on this page · raw message

include/hw/ppc/spapr.h +11 -1
index 9fc1972..3ceec7a 100644
@@ -109,7 +109,16 @@ typedef struct sPAPREnvironment {
 #define H_NOT_ENOUGH_RESOURCES -44
 #define H_R_STATE         -45
 #define H_RESCINDEND      -46
+#define H_P2              -55
+#define H_P3              -56
+#define H_P4              -57
+#define H_P5              -58
+#define H_P6              -59
+#define H_P7              -60
+#define H_P8              -61
+#define H_P9              -62
 #define H_MULTI_THREADS_ACTIVE -9005
+#define H_UNSUPPORTED_FLAG -256
 
 
 /* Long Busy is a condition that can be returned by the firmware
@@ -267,7 +276,8 @@ typedef struct sPAPREnvironment {
 #define H_GET_EM_PARMS          0x2B8
 #define H_SET_MPP               0x2D0
 #define H_GET_MPP               0x2D4
-#define MAX_HCALL_OPCODE        H_GET_MPP
+#define H_SET_MODE              0x31C
+#define MAX_HCALL_OPCODE        H_SET_MODE
 
 /* The hcalls above are standardized in PAPR and implemented by pHyp
  * as well.
Signature · 1 line
1.8.1.2
#10 lore raw

Re: [Qemu-devel] [PATCH 3/5] pseries: Add H_SET_MODE hcall to change guest exception endianness

Quoted 31 lines · Anton Blanchard

H_SET_MODE is used for controlling various partition settings. One of these settings is the endianness a guest takes its exceptions in.

---
 hw/ppc/spapr.c         |  2 +-
 hw/ppc/spapr_hcall.c   | 44 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h | 12 +++++++++++-
 3 files changed, 56 insertions(+), 2 deletions(-)
hw/ppc/spapr.c +1 -1
index 16bfab9..de639f6 100644
@@ -262,7 +262,7 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
     uint32_t start_prop = cpu_to_be32(initrd_base);
     uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
     char hypertas_prop[] = "hcall-pft\0hcall-term\0hcall-dabr\0hcall-interrupt"
-        "\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk";
+        "\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk\0hcall-set-mode";
     char qemu_hypertas_prop[] = "hcall-memop1";
     uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
     uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(smp_cpus)};
hw/ppc/spapr_hcall.c +0 -0
index 67d6cd9..79e1b61 100644
@@ -657,6 +657,48 @@ static target_ulong h_logical_dcbf(PowerPCCPU *cpu, sPAPREnvironment *spapr,
     return H_SUCCESS;
 }
 

hw/ppc/spapr_hcall.c static target_ulong h_logical_dcbf(PowerPCCPU *cpu, sPAPREnvironment *spapr, · line 669

Quoted 10 lines
hw/ppc/spapr_hcall.c +10 -0
+static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+                               target_ulong opcode, target_ulong *args)
+{
+    CPUState *cs;
+    target_ulong mflags = args[0];
+    target_ulong resource = args[1];
+    target_ulong value1 = args[2];
+    target_ulong value2 = args[3];
+
+    if (resource == 4) {

This ought to be a #define. There's no else here, is that expected? Should you return failure for a different resource?

Quoted 13 lines
hw/ppc/spapr_hcall.c +13 -0
+        if (value1) {
+            return H_P3;
+        }
+        if (value2) {
+            return H_P4;
+        }
+
+        switch (mflags) {
+        case 0:
+            for (cs = first_cpu; cs != NULL; cs = cs->next_cpu) {
+                PowerPCCPU *cp = POWERPC_CPU(cs);
+                CPUPPCState *env = &cp->env;
+                env->spr[SPR_LPCR] &= ~LPCR_ILE;

hw/ppc/spapr_hcall.c static target_ulong h_logical_dcbf(PowerPCCPU *cpu, sPAPREnvironment *spapr, · line 692

Quoted 10 lines
hw/ppc/spapr_hcall.c +10 -0
+            }
+            return H_SUCCESS;
+
+        case 1:
+            for (cs = first_cpu; cs != NULL; cs = cs->next_cpu) {
+                PowerPCCPU *cp = POWERPC_CPU(cs);
+                CPUPPCState *env = &cp->env;
+                env->spr[SPR_LPCR] |= LPCR_ILE;
+            }
+            return H_SUCCESS;

Without knowing this interface better, a few things come to mind.

Is mflags a boolean? If so, you can reduce this to a single loop and drop the switch() statement. If mflags is truly a set of flags, it would be nice to use #define to give the flags a proper symbolic name.

Regards,

Anthony Liguori

Quoted 55 lines
hw/ppc/spapr_hcall.c +10 -0
+        default:
+            return H_UNSUPPORTED_FLAG;
+        }
+    }
+
+    return H_P2;
+}
+
 static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1];
 static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - KVMPPC_HCALL_BASE + 1];
 
@@ -734,6 +776,8 @@ static void hypercall_register_types(void)
 
     /* qemu/KVM-PPC specific hcalls */
     spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas);
+
+    spapr_register_hypercall(H_SET_MODE, h_set_mode);
 }
 
 type_init(hypercall_register_types)
include/hw/ppc/spapr.h +11 -2
index 9fc1972..3ceec7a 100644
@@ -109,7 +109,16 @@ typedef struct sPAPREnvironment {
 #define H_NOT_ENOUGH_RESOURCES -44
 #define H_R_STATE         -45
 #define H_RESCINDEND      -46
+#define H_P2              -55
+#define H_P3              -56
+#define H_P4              -57
+#define H_P5              -58
+#define H_P6              -59
+#define H_P7              -60
+#define H_P8              -61
+#define H_P9              -62
 #define H_MULTI_THREADS_ACTIVE -9005
+#define H_UNSUPPORTED_FLAG -256
 
 
 /* Long Busy is a condition that can be returned by the firmware
@@ -267,7 +276,8 @@ typedef struct sPAPREnvironment {
 #define H_GET_EM_PARMS          0x2B8
 #define H_SET_MPP               0x2D0
 #define H_GET_MPP               0x2D4
-#define MAX_HCALL_OPCODE        H_GET_MPP
+#define H_SET_MODE              0x31C
+#define MAX_HCALL_OPCODE        H_SET_MODE
 
 /* The hcalls above are standardized in PAPR and implemented by pHyp
  * as well.
--

1.8.1.2

#11 lore raw

PATCH 1/1 pseries: Add H_SET_MODE hcall to change guest exception endianness

Hi Anthony,

Quoted 4 lines
Quoted 1 line

+ if (resource == 4) {

This ought to be a #define. There's no else here, is that expected? Should you return failure for a different resource?

Good point, I made it a define. We were returning H_P2 for a different resource, but it was a bit of a twisted maze of return statements. I tried to clear it up in this version.

Quoted 5 lines

Without knowing this interface better, a few things come to mind.

Is mflags a boolean? If so, you can reduce this to a single loop and drop the switch() statement. If mflags is truly a set of flags, it would be nice to use #define to give the flags a proper symbolic name.

Unfortunately it isn't a boolean, but yes it should have be made clearer with a #define.

Anton
--

pseries: Add H_SET_MODE hcall to change guest exception endianness

H_SET_MODE is used for controlling various partition settings. One of these settings is the endianness a guest takes its exceptions in.

---

hw/ppc/spapr.c +1 -1
index 16bfab9..de639f6 100644
@@ -262,7 +262,7 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
     uint32_t start_prop = cpu_to_be32(initrd_base);
     uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
     char hypertas_prop[] = "hcall-pft\0hcall-term\0hcall-dabr\0hcall-interrupt"
-        "\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk";
+        "\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk\0hcall-set-mode";
     char qemu_hypertas_prop[] = "hcall-memop1";
     uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
     uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(smp_cpus)};
hw/ppc/spapr_hcall.c +50 -0

62 lines not on this page · raw message

include/hw/ppc/spapr.h +16 -1
index 9fc1972..ab42813 100644
@@ -109,6 +109,15 @@ typedef struct sPAPREnvironment {
 #define H_NOT_ENOUGH_RESOURCES -44
 #define H_R_STATE         -45
 #define H_RESCINDEND      -46
+#define H_P2              -55
+#define H_P3              -56
+#define H_P4              -57
+#define H_P5              -58
+#define H_P6              -59
+#define H_P7              -60
+#define H_P8              -61
+#define H_P9              -62
+#define H_UNSUPPORTED_FLAG -256
 #define H_MULTI_THREADS_ACTIVE -9005
 
 
@@ -143,6 +152,11 @@ typedef struct sPAPREnvironment {
 #define H_PP1             (1ULL<<(63-62))
 #define H_PP2             (1ULL<<(63-63))
 
+/* H_SET_MODE flags */
+#define H_SET_MODE_ENDIAN		4
+#define  H_SET_MODE_ENDIAN_BIG		0
+#define  H_SET_MODE_ENDIAN_LITTLE	1
+
 /* VASI States */
 #define H_VASI_INVALID    0
 #define H_VASI_ENABLED    1
@@ -267,7 +281,8 @@ typedef struct sPAPREnvironment {
 #define H_GET_EM_PARMS          0x2B8
 #define H_SET_MPP               0x2D0
 #define H_GET_MPP               0x2D4
-#define MAX_HCALL_OPCODE        H_GET_MPP
+#define H_SET_MODE              0x31C
+#define MAX_HCALL_OPCODE        H_SET_MODE
 
 /* The hcalls above are standardized in PAPR and implemented by pHyp
  * as well.
#12 lore raw

Re: [Qemu-devel] [PATCH] pseries: Add H_SET_MODE hcall to change guest exception endianness

Quoted 24 lines · Anton Blanchard

Hi Anthony,

Quoted 4 lines
Quoted 1 line

+ if (resource == 4) {

This ought to be a #define. There's no else here, is that expected? Should you return failure for a different resource?

Good point, I made it a define. We were returning H_P2 for a different resource, but it was a bit of a twisted maze of return statements. I tried to clear it up in this version.

Quoted 5 lines

Without knowing this interface better, a few things come to mind.

Is mflags a boolean? If so, you can reduce this to a single loop and drop the switch() statement. If mflags is truly a set of flags, it would be nice to use #define to give the flags a proper symbolic name.

Unfortunately it isn't a boolean, but yes it should have be made clearer with a #define.

Anton
--

Quoted 6 lines

pseries: Add H_SET_MODE hcall to change guest exception endianness

H_SET_MODE is used for controlling various partition settings. One of these settings is the endianness a guest takes its exceptions in.

Thanks, applied after I replaced tabs with spaces ;).

Alex

#13 lore raw

PATCH 4/5 disas/ppc.c: Fix little endian disassembly

In reply to Anton Blanchard

Use info->endian to select the endian of the instruction to be disassembled.

---
 disas/ppc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
disas/ppc.c +2 -1
index c149506..99c4cbc 100644
@@ -5157,7 +5157,8 @@ int
 print_insn_ppc (bfd_vma memaddr, struct disassemble_info *info)
 {
   int dialect = (char *) info->private_data - (char *) 0;
-  return print_insn_powerpc (memaddr, info, 1, dialect);
+  return print_insn_powerpc (memaddr, info, info->endian == BFD_ENDIAN_BIG,
+                             dialect);
 }
 
 /* Print a big endian PowerPC instruction.  */
Signature · 1 line
1.8.1.2
Reviewed-by on v1 4/5
#14 lore raw

Re: [Qemu-devel] [PATCH 4/5] disas/ppc.c: Fix little endian disassembly

Quoted 4 lines · Anton Blanchard

Use info->endian to select the endian of the instruction to be disassembled.

Regards,

Anthony Liguori

Quoted 22 lines
---
 disas/ppc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
disas/ppc.c +2 -2
index c149506..99c4cbc 100644
@@ -5157,7 +5157,8 @@ int
 print_insn_ppc (bfd_vma memaddr, struct disassemble_info *info)
 {
   int dialect = (char *) info->private_data - (char *) 0;
-  return print_insn_powerpc (memaddr, info, 1, dialect);
+  return print_insn_powerpc (memaddr, info, info->endian == BFD_ENDIAN_BIG,
+                             dialect);
 }
 
 /* Print a big endian PowerPC instruction.  */
--

1.8.1.2

#16 lore raw

PATCH 5/5 pseries: Fix loading of little endian kernels

In reply to Anton Blanchard

From: Benjamin Herrenschmidt <[email protected]>

Try loading the kernel as little endian if it fails big endian.

---
 hw/ppc/spapr.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
hw/ppc/spapr.c +12 -1
index de639f6..639b719 100644
@@ -253,6 +253,7 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
                                    hwaddr initrd_base,
                                    hwaddr initrd_size,
                                    hwaddr kernel_size,
+                                   bool little_endian,
                                    const char *boot_device,
                                    const char *kernel_cmdline,
                                    uint32_t epow_irq)
@@ -306,6 +307,9 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
                               cpu_to_be64(kernel_size) };
 
         _FDT((fdt_property(fdt, "qemu,boot-kernel", &kprop, sizeof(kprop))));
+        if (little_endian) {
+            _FDT((fdt_property(fdt, "qemu,boot-kernel-le", NULL, 0)));
+        }
     }
     if (boot_device) {
         _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device)));
@@ -1082,6 +1086,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
     uint32_t initrd_base = 0;
     long kernel_size = 0, initrd_size = 0;
     long load_limit, rtas_limit, fw_size;
+    bool kernel_le = false;
     char *filename;
 
     msi_supported = true;
@@ -1261,6 +1266,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
         kernel_size = load_elf(kernel_filename, translate_kernel_address, NULL,
                                NULL, &lowaddr, NULL, 1, ELF_MACHINE, 0);
         if (kernel_size < 0) {
+            kernel_size = load_elf(kernel_filename,
+                                   translate_kernel_address, NULL,
+                                   NULL, &lowaddr, NULL, 0, ELF_MACHINE, 0);
+            kernel_le = kernel_size > 0;
+        }
+        if (kernel_size < 0) {
             kernel_size = load_image_targphys(kernel_filename,
                                               KERNEL_LOAD_ADDR,
                                               load_limit - KERNEL_LOAD_ADDR);
@@ -1310,7 +1321,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
     /* Prepare the device tree */
     spapr->fdt_skel = spapr_create_fdt_skel(cpu_model,
                                             initrd_base, initrd_size,
-                                            kernel_size,
+                                            kernel_size, kernel_le,
                                             boot_device, kernel_cmdline,
                                             spapr->epow_irq);
     assert(spapr->fdt_skel != NULL);
Signature · 1 line
1.8.1.2
Reviewed-by on v1 5/5
#17 lore raw

Re: [Qemu-devel] [PATCH 5/5] pseries: Fix loading of little endian kernels

Quoted 6 lines · Anton Blanchard

From: Benjamin Herrenschmidt <[email protected]>

Try loading the kernel as little endian if it fails big endian.

Regards,

Anthony Liguori

Quoted 60 lines
---
 hw/ppc/spapr.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
hw/ppc/spapr.c +12 -2
index de639f6..639b719 100644
@@ -253,6 +253,7 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
                                    hwaddr initrd_base,
                                    hwaddr initrd_size,
                                    hwaddr kernel_size,
+                                   bool little_endian,
                                    const char *boot_device,
                                    const char *kernel_cmdline,
                                    uint32_t epow_irq)
@@ -306,6 +307,9 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
                               cpu_to_be64(kernel_size) };
 
         _FDT((fdt_property(fdt, "qemu,boot-kernel", &kprop, sizeof(kprop))));
+        if (little_endian) {
+            _FDT((fdt_property(fdt, "qemu,boot-kernel-le", NULL, 0)));
+        }
     }
     if (boot_device) {
         _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device)));
@@ -1082,6 +1086,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
     uint32_t initrd_base = 0;
     long kernel_size = 0, initrd_size = 0;
     long load_limit, rtas_limit, fw_size;
+    bool kernel_le = false;
     char *filename;
 
     msi_supported = true;
@@ -1261,6 +1266,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
         kernel_size = load_elf(kernel_filename, translate_kernel_address, NULL,
                                NULL, &lowaddr, NULL, 1, ELF_MACHINE, 0);
         if (kernel_size < 0) {
+            kernel_size = load_elf(kernel_filename,
+                                   translate_kernel_address, NULL,
+                                   NULL, &lowaddr, NULL, 0, ELF_MACHINE, 0);
+            kernel_le = kernel_size > 0;
+        }
+        if (kernel_size < 0) {
             kernel_size = load_image_targphys(kernel_filename,
                                               KERNEL_LOAD_ADDR,
                                               load_limit - KERNEL_LOAD_ADDR);
@@ -1310,7 +1321,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
     /* Prepare the device tree */
     spapr->fdt_skel = spapr_create_fdt_skel(cpu_model,
                                             initrd_base, initrd_size,
-                                            kernel_size,
+                                            kernel_size, kernel_le,
                                             boot_device, kernel_cmdline,
                                             spapr->epow_irq);
     assert(spapr->fdt_skel != NULL);
--

1.8.1.2

#18 lore raw
In reply to Anton Blanchard
Quoted 5 lines · Anton Blanchard

This patchset adds support for 64bit PowerPC little endian on POWER7.

Linux kernel patches to support this were sent out earlier today:

https://lists.ozlabs.org/pipermail/linuxppc-dev/2013-August/109849.html

Nice :-)

This will need to wait until 1.7 opens up in a couple weeks, but other than the cosmetics on 3/5, the series looks good.

Regards,

Anthony Liguori

Quoted 26 lines

Anton
--

Anton Blanchard (4):
target-ppc: POWER7 supports the MSR_LE bit
target-ppc: USE LPCR_ILE to control exception endian on POWER7
pseries: Add H_SET_MODE hcall to change guest exception endianness disas/ppc.c: Fix little endian disassembly

Benjamin Herrenschmidt (1):
pseries: Fix loading of little endian kernels

 disas/ppc.c                 |  3 ++-
 hw/ppc/spapr.c              | 15 +++++++++++++--
 hw/ppc/spapr_hcall.c        | 44 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h      | 12 +++++++++++-
 target-ppc/cpu.h            |  2 ++
 target-ppc/excp_helper.c    | 10 ++++++++++
 target-ppc/translate_init.c |  2 +-
 7 files changed, 83 insertions(+), 5 deletions(-)

--
1.8.1.2