On 02/15/2018 03:06 PM, Peter Maydell wrote:
> On 15 February 2018 at 17:57, Peter Maydell <peter.mayd...@linaro.org> wrote:
>> Instead of loading kernels, device trees, and the like to
>> the system address space, use the CPU's address space. This
>> is important if we're trying to load the file to memory or
>> via an alias memory region that is provided by an SoC
>> object and thus not mapped into the system address space.
>>
>> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
>> ---
>> hw/arm/boot.c | 119
>> +++++++++++++++++++++++++++++++++++++---------------------
>> 1 file changed, 76 insertions(+), 43 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index 9b174b982c..0eac655c98 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -35,6 +35,25 @@
>> #define ARM64_TEXT_OFFSET_OFFSET 8
>> #define ARM64_MAGIC_OFFSET 56
>>
>> +static AddressSpace *arm_boot_addressspace(ARMCPU *cpu,
>> + const struct arm_boot_info *info)
>> +{
>> + /* 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
>> + * going to boot the guest into it.
>> + */
>> + int asidx;
>> + CPUState *cs = CPU(cpu);
>> +
>> + if (arm_feature(&cpu->env, ARM_FEATURE_EL3) && info->secure_boot) {
>> + asidx = ARMASIdx_S;
>> + } else {
>> + asidx = ARMASIdx_NS;
>> + }
>> +
>> + return cpu_get_address_space(cs, asidx);
>> +}
>> +
>> typedef enum {
>> FIXUP_NONE = 0, /* do nothing */
>> FIXUP_TERMINATOR, /* end of insns */
>> @@ -124,7 +143,8 @@ static const ARMInsnFixup smpboot[] = {
>> };
>>
>> static void write_bootloader(const char *name, hwaddr addr,
>> - const ARMInsnFixup *insns, uint32_t
>> *fixupcontext)
>> + const ARMInsnFixup *insns, uint32_t
>> *fixupcontext,
>> + AddressSpace *as)
>> {
>> /* Fix up the specified bootloader fragment and write it into
>> * guest memory using rom_add_blob_fixed(). fixupcontext is
>> @@ -163,7 +183,7 @@ static void write_bootloader(const char *name, hwaddr
>> addr,
>> code[i] = tswap32(insn);
>> }
>>
>> - rom_add_blob_fixed(name, code, len * sizeof(uint32_t), addr);
>> + rom_add_blob_fixed_as(name, code, len * sizeof(uint32_t), addr, as);
>>
>> g_free(code);
>> }
>> @@ -172,6 +192,7 @@ static void default_write_secondary(ARMCPU *cpu,
>> const struct arm_boot_info *info)
>> {
>> uint32_t fixupcontext[FIXUP_MAX];
>> + AddressSpace *as = CPU(cpu)->as;
>
> Oops. This should be
> AddressSpace *as = arm_boot_addressspace(cpu, info);
I was stuck there wondering why this one were using the CPU AS :)
>
>> fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr;
>> fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
>> @@ -182,13 +203,14 @@ static void default_write_secondary(ARMCPU *cpu,
>> }
>>
>> write_bootloader("smpboot", info->smp_loader_start,
>> - smpboot, fixupcontext);
>> + smpboot, fixupcontext, as);
>> }
>>
>> void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
>> const struct arm_boot_info
>> *info,
>> hwaddr mvbar_addr)
>> {
>> + AddressSpace *as = CPU(cpu)->as;
>
> ...and so should this.
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>
> thanks
> -- PMM
>
