Re: [PATCH 02/10] memory-device: Introduce memory_devices_init()

[Philippe Mathieu-Daudé]

Hi David,

On 30/5/23 13:38, David Hildenbrand wrote:
> Let's intrduce a new helper that we will use to replace existing memory
> device setup code during machine initialization. We'll enforce that the
> size has to be > 0.
>
> Once all machines were converted, we'll only allocate ms->device_memory
> if the size > 0.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   hw/mem/memory-device.c         | 14 ++++++++++++++
>   include/hw/mem/memory-device.h |  2 ++
>   2 files changed, 16 insertions(+)


> diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
> index 48d2611fc5..6e8a10e2f5 100644
> --- a/include/hw/mem/memory-device.h
> +++ b/include/hw/mem/memory-device.h
> @@ -16,6 +16,7 @@
>   #include "hw/qdev-core.h"
>   #include "qapi/qapi-types-machine.h"
>   #include "qom/object.h"
> +#include "exec/hwaddr.h"
>   
>   #define TYPE_MEMORY_DEVICE "memory-device"
>   
> @@ -113,5 +114,6 @@ void memory_device_plug(MemoryDeviceState *md, MachineState *ms);
>   void memory_device_unplug(MemoryDeviceState *md, MachineState *ms);
>   uint64_t memory_device_get_region_size(const MemoryDeviceState *md,
>                                          Error **errp);
> +void memory_devices_init(MachineState *ms, hwaddr base, uint64_t size);


While hw/mem/memory-device.c contains the implementation, all callers
are expected to be around Machine object, right? Thus maybe this _init()
could be declared in "hw/boards.h", already included by machines
(eventually renaming as machine_init_memory_devices() ). Then machines
implementation don't have to all include "hw/mem/memory-device.h".

Alternatively, keep memory_devices_init() declared here, but implement
machine_init_memory_devices() in hw/core/machine.c, declaring it in
"hw/boards.h", so again machines don't have to include
"hw/mem/memory-device.h".

What do you think?