On Wed, 15 Jan 2020 20:07:34 +0100
Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
> On 1/15/20 4:06 PM, Igor Mammedov wrote:
> > memory_region_allocate_system_memory() API is going away, so
> > replace it with memdev allocated MemoryRegion. The later is
> > initialized by generic code, so board only needs to opt in
> > to memdev scheme by providing
> > MachineClass::default_ram_id
> > and using MachineState::ram instead of manually initializing
> > RAM memory region.
> >
> > PS:
> > remove no longer needed RasPiState
> >
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > ---
> > CC: andrew.baum...@microsoft.com
> > CC: phi...@redhat.com
> > CC: peter.mayd...@linaro.org
> > CC: qemu-...@nongnu.org
> > ---
> > hw/arm/raspi.c | 34 +++++++++++++---------------------
> > 1 file changed, 13 insertions(+), 21 deletions(-)
> >
> > diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> > index 6a510aa..33ace66 100644
> > --- a/hw/arm/raspi.c
> > +++ b/hw/arm/raspi.c
> > @@ -32,11 +32,6 @@
> > /* Table of Linux board IDs for different Pi versions */
> > static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] =
> > 0xc44};
> >
> > -typedef struct RasPiState {
> > - BCM283XState soc;
> > - MemoryRegion ram;
> > -} RasPiState;
> > -
> > static void write_smpboot(ARMCPU *cpu, const struct arm_boot_info *info)
> > {
> > static const uint32_t smpboot[] = {
> > @@ -166,7 +161,7 @@ static void setup_boot(MachineState *machine, int
> > version, size_t ram_size)
> >
> > static void raspi_init(MachineState *machine, int version)
> > {
> > - RasPiState *s = g_new0(RasPiState, 1);
> > + Object *soc;
> > uint32_t vcram_size;
> > DriveInfo *di;
> > BlockBackend *blk;
> > @@ -179,30 +174,26 @@ static void raspi_init(MachineState *machine, int
> > version)
> > exit(1);
> > }
> >
> > - object_initialize_child(OBJECT(machine), "soc", &s->soc,
> > sizeof(s->soc),
> > - version == 3 ? TYPE_BCM2837 : TYPE_BCM2836,
> > - &error_abort, NULL);
> > + soc = object_new(version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
> > + object_property_add_child(OBJECT(machine), "soc", soc, &error_fatal);
> >
> > - /* Allocate and map RAM */
> > - memory_region_allocate_system_memory(&s->ram, OBJECT(machine), "ram",
> > - machine->ram_size);
> > /* FIXME: Remove when we have custom CPU address space support */
> > - memory_region_add_subregion_overlap(get_system_memory(), 0, &s->ram,
> > 0);
> > + memory_region_add_subregion_overlap(get_system_memory(), 0,
> > + machine->ram, 0);
> >
> > /* Setup the SOC */
> > - object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
> > + object_property_add_const_link(soc, "ram", OBJECT(machine->ram),
> > &error_abort);
> > - object_property_set_int(OBJECT(&s->soc), machine->smp.cpus,
> > "enabled-cpus",
> > + object_property_set_int(soc, machine->smp.cpus, "enabled-cpus",
> > &error_abort);
> > int board_rev = version == 3 ? 0xa02082 : 0xa21041;
> > - object_property_set_int(OBJECT(&s->soc), board_rev, "board-rev",
> > - &error_abort);
> > - object_property_set_bool(OBJECT(&s->soc), true, "realized",
> > &error_abort);
> > + object_property_set_int(soc, board_rev, "board-rev", &error_abort);
> > + object_property_set_bool(soc, true, "realized", &error_abort);
> >
> > /* Create and plug in the SD cards */
> > di = drive_get_next(IF_SD);
> > blk = di ? blk_by_legacy_dinfo(di) : NULL;
> > - bus = qdev_get_child_bus(DEVICE(&s->soc), "sd-bus");
> > + bus = qdev_get_child_bus(DEVICE(soc), "sd-bus");
> > if (bus == NULL) {
> > error_report("No SD bus found in SOC object");
> > exit(1);
> > @@ -211,8 +202,7 @@ static void raspi_init(MachineState *machine, int
> > version)
> > qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
> > object_property_set_bool(OBJECT(carddev), true, "realized",
> > &error_fatal);
> >
> > - vcram_size = object_property_get_uint(OBJECT(&s->soc), "vcram-size",
> > - &error_abort);
> > + vcram_size = object_property_get_uint(soc, "vcram-size", &error_abort);
> > setup_boot(machine, version, machine->ram_size - vcram_size);
> > }
> >
> > @@ -233,6 +223,7 @@ static void raspi2_machine_init(MachineClass *mc)
> > mc->min_cpus = BCM283X_NCPUS;
> > mc->default_cpus = BCM283X_NCPUS;
> > mc->default_ram_size = 1 * GiB;
> > + mc->default_ram_id = "ram";
> > mc->ignore_memory_transaction_failures = true;
> > };
> > DEFINE_MACHINE("raspi2", raspi2_machine_init)
> > @@ -255,6 +246,7 @@ static void raspi3_machine_init(MachineClass *mc)
> > mc->min_cpus = BCM283X_NCPUS;
> > mc->default_cpus = BCM283X_NCPUS;
> > mc->default_ram_size = 1 * GiB;
> > + mc->default_ram_id = "ram";
> > }
> > DEFINE_MACHINE("raspi3", raspi3_machine_init)
> > #endif
> >
>
> This patch diverges a lot from my current work:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg653818.html
perhaps we could generalize size checks on top of that.
there were some ideas in that direction in
"[PATCH v2 66/86] ppc/{ppc440_bamboo,sam460x}: drop RAM size fixup"
thread
> So I'm not very happy about it. Maybe my bad I should ping more
> aggressively my patches. I can respin mine preparing for your series on top.
this patch is trivial,
if your patches merged before this, than I'll just rebase.
>
> Meanwhile if you are in a hurry I tested yours, so:
> Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
> Tested-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>
>
