Re: [PATCH v2 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 3/7/25 21:41, Philippe Mathieu-Daudé wrote:
> On 7/3/25 23:39, Philippe Mathieu-Daudé wrote:
>> Use qemu_arch_available() to check at runtime if a target
>> architecture is built in.
>>
>> Consider the maximum extent size of any architecture built in.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>    hw/virtio/virtio-mem.c | 28 ++++++++++++++++++++--------
>>    1 file changed, 20 insertions(+), 8 deletions(-)
> 
> 
>> @@ -170,13 +171,24 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
>>     * necessary (as the section size can change). But it's more likely that the
>>     * section size will rather get smaller and not bigger over time.
>>     */
>> -#if defined(TARGET_X86_64) || defined(TARGET_I386) || defined(TARGET_S390X)
>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB))
>> -#elif defined(TARGET_ARM)
>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (512 * MiB))
>> -#else
>> -#error VIRTIO_MEM_USABLE_EXTENT not defined
>> -#endif
>> +static uint64_t virtio_mem_usable_extent_size(void)
>> +{
>> +    uint64_t size = 0;
>> +
>> +    assert(qemu_arch_available(QEMU_ARCH_ARM | QEMU_ARCH_I386 | QEMU_ARCH_S390X));
> 
> I'm not sure this assertion is doing what I thought it'd do.
> 
> For example, building with --target-list=aarch64-softmmu,riscv32-softmmu,
> this device is now linked in. However, riscv32 machines won't be able
> to plug it until they allow TYPE_VIRTIO_MD_PCI in some of their
> HotplugHandlerClass handlers. Still I'd like to catch this case here
> to avoid bad surprises.
> 

With the single binary, a lot of devices will be linked in, even though 
they won't be used. There is no specific very specific here, and 
definitely something we can tackle later, once we are able to link a 
single binary.

>> +    /*
>> +     * FIXME: We should use the maximum of instantiated vCPUs ARCH, but
>> +     *        for now it is easier to take the maximum of any ARCH built in.
>> +     */
>> +    if (qemu_arch_available(QEMU_ARCH_I386 | QEMU_ARCH_S390X)) {
>> +        size = MAX(size, 2 * 128 * MiB);
>> +    }
>> +    if (qemu_arch_available(QEMU_ARCH_ARM)) {
>> +        size = MAX(size, 2 * 512 * MiB);
>> +    }
>> +
>> +    return size;
>> +}
>