Re: [Qemu-devel] [PULL 13/28] hw/arm/virt: Use 256MB ECAM region by default

[Eric Auger <eric.auger@redhat.com>]

Hi Peter,

On 1/8/24 16:52, Peter Maydell wrote:
> On Fri, 22 Jun 2018 at 14:09, Peter Maydell <peter.maydell@linaro.org> wrote:
>> From: Eric Auger <eric.auger@redhat.com>
>>
>> With this patch, virt-3.0 machine uses a new 256MB ECAM region
>> by default instead of the legacy 16MB one, if highmem is set
>> (LPAE supported by the guest) and (!firmware_loaded || aarch64).
>>
>> Indeed aarch32 mode FW may not support this high ECAM region.
> This is a rather old change by now, but I've been looking
> at it because it exposes an issue which was previously
> masked by a different bug...
>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index d8abf89e8c8..0f8bfa57d7e 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1318,6 +1318,7 @@ static void machvirt_init(MachineState *machine)
>>      int n, virt_max_cpus;
>>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>>      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
>> +    bool aarch64 = true;
>>
>>      /* We can probe only here because during property set
>>       * KVM is not available yet
>> @@ -1433,6 +1434,8 @@ static void machvirt_init(MachineState *machine)
>>          numa_cpu_pre_plug(&possible_cpus->cpus[cs->cpu_index], DEVICE(cpuobj),
>>                            &error_fatal);
>>
>> +        aarch64 &= object_property_get_bool(cpuobj, "aarch64", NULL);
>> +
>>          if (!vms->secure) {
>>              object_property_set_bool(cpuobj, false, "has_el3", NULL);
>>          }
>> @@ -1491,6 +1494,8 @@ static void machvirt_init(MachineState *machine)
>>          create_uart(vms, pic, VIRT_SECURE_UART, secure_sysmem, serial_hd(1));
>>      }
>>
>> +    vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64);
> Do you remember why this conditional is the way it is?
It comes from Laszlo's suggestion at
http://patchwork.ozlabs.org/project/qemu-devel/cover/1527091418-11874-1-git-send-email-eric.auger@redhat.com/#1920422

"I'd rather restrict the large/high ECAM feature to 64-bit guests (with or without
firmware), and to 32-bit LPAE kernels that are launched without firmware
(which, I think, has been the case for most of their history)."


See the associated thread too.

Hope this helps

Eric
> As it stands, it will disable the high-memory ECAM for
> an AArch32 VM that's loaded firmware, but leaves it enabled
> if we're direct booting Linux. That's a problem because 32-bit
> Linux falls over if you pass it a highmem-ECAM, even if LPAE
> is enabled (somewhere along the line it discards the high 32
> bits of the address of the ECAM in the dtb, so it thinks the
> ECAM overlaps with another memory region, and won't recognize
> the pci controller; I have a feeling this is a regression in
> the kernel). Plus, we have no way to tell if the guest
> kernel has LPAE enabled at all.
>
> Maybe it would be safer to insist that the guest is aarch64
> before we enable highmem ECAM?
>
> thanks
> -- PMM
>