diff mbox series

[for-2.13,1/4] arm: reuse arm_boot_address_space() in armv7m_load_kernel()

Message ID 1523551221-11612-2-git-send-email-imammedo@redhat.com
State New
Headers show
Series arm: isolate and clean up dtb generation | expand

Commit Message

Igor Mammedov April 12, 2018, 4:40 p.m. UTC 
reduce code duplication by resusing arm_boot_address_space()

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/arm/arm.h |  2 ++
 hw/arm/armv7m.c      | 10 +---------
 hw/arm/boot.c        | 16 ++++++++--------
 3 files changed, 11 insertions(+), 17 deletions(-)

Comments

Peter Maydell April 12, 2018, 6:22 p.m. UTC | #1 
On 12 April 2018 at 17:40, Igor Mammedov <imammedo@redhat.com> wrote:
> reduce code duplication by resusing arm_boot_address_space()
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/arm/arm.h |  2 ++
>  hw/arm/armv7m.c      | 10 +---------
>  hw/arm/boot.c        | 16 ++++++++--------
>  3 files changed, 11 insertions(+), 17 deletions(-)

This minor duplication was deliberate, because hw/arm/armv7m.c
is for M profile cores only, and hw/arm/boot.c is for
A/R profile cores only. I don't think that saving a
single if/else in armv7m.c really justifies blurring
that dividing line.

thanks
-- PMM
Igor Mammedov April 13, 2018, 1:41 p.m. UTC | #2 
On Thu, 12 Apr 2018 19:22:13 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 12 April 2018 at 17:40, Igor Mammedov <imammedo@redhat.com> wrote:
> > reduce code duplication by resusing arm_boot_address_space()
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  include/hw/arm/arm.h |  2 ++
> >  hw/arm/armv7m.c      | 10 +---------
> >  hw/arm/boot.c        | 16 ++++++++--------
> >  3 files changed, 11 insertions(+), 17 deletions(-)  
> 
> This minor duplication was deliberate, because hw/arm/armv7m.c
> is for M profile cores only, and hw/arm/boot.c is for
> A/R profile cores only. I don't think that saving a
> single if/else in armv7m.c really justifies blurring
> that dividing line.
I'll drop patch on respin

> thanks
> -- PMM
diff mbox series

Patch

diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index ce769bd..188d18b 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -143,6 +143,8 @@  struct arm_boot_info {
  */
 void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info);
 
+AddressSpace *arm_boot_address_space(ARMCPU *cpu, bool secure_boot);
+
 /* Write a secure board setup routine with a dummy handler for SMCs */
 void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
                                             const struct arm_boot_info *info,
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index f123cc7..d372d9c 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -289,8 +289,6 @@  void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
     uint64_t lowaddr;
     int big_endian;
     AddressSpace *as;
-    int asidx;
-    CPUState *cs = CPU(cpu);
 
 #ifdef TARGET_WORDS_BIGENDIAN
     big_endian = 1;
@@ -303,13 +301,7 @@  void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
         exit(1);
     }
 
-    if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
-        asidx = ARMASIdx_S;
-    } else {
-        asidx = ARMASIdx_NS;
-    }
-    as = cpu_get_address_space(cs, asidx);
-
+    as = arm_boot_address_space(cpu, true);
     if (kernel_filename) {
         image_size = load_elf_as(kernel_filename, NULL, NULL, &entry, &lowaddr,
                                  NULL, big_endian, EM_ARM, 1, 0, as);
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 26184bc..2f464ca 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -36,8 +36,7 @@ 
 #define ARM64_TEXT_OFFSET_OFFSET    8
 #define ARM64_MAGIC_OFFSET          56
 
-static AddressSpace *arm_boot_address_space(ARMCPU *cpu,
-                                            const struct arm_boot_info *info)
+AddressSpace *arm_boot_address_space(ARMCPU *cpu, bool secure_boot)
 {
     /* Return the address space to use for bootloader reads and writes.
      * We prefer the secure address space if the CPU has it and we're
@@ -46,7 +45,7 @@  static AddressSpace *arm_boot_address_space(ARMCPU *cpu,
     int asidx;
     CPUState *cs = CPU(cpu);
 
-    if (arm_feature(&cpu->env, ARM_FEATURE_EL3) && info->secure_boot) {
+    if (arm_feature(&cpu->env, ARM_FEATURE_EL3) && secure_boot) {
         asidx = ARMASIdx_S;
     } else {
         asidx = ARMASIdx_NS;
@@ -193,7 +192,7 @@  static void default_write_secondary(ARMCPU *cpu,
                                     const struct arm_boot_info *info)
 {
     uint32_t fixupcontext[FIXUP_MAX];
-    AddressSpace *as = arm_boot_address_space(cpu, info);
+    AddressSpace *as = arm_boot_address_space(cpu, info->secure_boot);
 
     fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr;
     fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
@@ -211,7 +210,7 @@  void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
                                             const struct arm_boot_info *info,
                                             hwaddr mvbar_addr)
 {
-    AddressSpace *as = arm_boot_address_space(cpu, info);
+    AddressSpace *as = arm_boot_address_space(cpu, info->secure_boot);
     int n;
     uint32_t mvbar_blob[] = {
         /* mvbar_addr: secure monitor vectors
@@ -262,7 +261,7 @@  void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
 static void default_reset_secondary(ARMCPU *cpu,
                                     const struct arm_boot_info *info)
 {
-    AddressSpace *as = arm_boot_address_space(cpu, info);
+    AddressSpace *as = arm_boot_address_space(cpu, info->secure_boot);
     CPUState *cs = CPU(cpu);
 
     address_space_stl_notdirty(as, info->smp_bootreg_addr,
@@ -753,7 +752,8 @@  static void do_cpu_reset(void *opaque)
             }
 
             if (cs == first_cpu) {
-                AddressSpace *as = arm_boot_address_space(cpu, info);
+                AddressSpace *as =
+                    arm_boot_address_space(cpu, info->secure_boot);
 
                 cpu_set_pc(cs, info->loader_start);
 
@@ -950,7 +950,7 @@  static void arm_load_kernel_notify(Notifier *notifier, void *data)
     ARMCPU *cpu = n->cpu;
     struct arm_boot_info *info =
         container_of(n, struct arm_boot_info, load_kernel_notifier);
-    AddressSpace *as = arm_boot_address_space(cpu, info);
+    AddressSpace *as = arm_boot_address_space(cpu, info->secure_boot);
 
     /* The board code is not supposed to set secure_board_setup unless
      * running its code in secure mode is actually possible, and KVM