| Message ID | 20200217173452.15243-28-imammedo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | refactor main RAM allocation to use hostmem backend | expand |
On 2/17/20 9:34 AM, 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: > while at it add check for user supplied RAM size and error > out if it mismatches board expected value. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > Reviewed-by: Andrew Jones <drjones@redhat.com> > --- Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 2/17/20 6:34 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: > while at it add check for user supplied RAM size and error > out if it mismatches board expected value. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > Reviewed-by: Andrew Jones <drjones@redhat.com> > --- > v2: > * fix format string causing build failure on 32-bit host > (Philippe Mathieu-Daudé <philmd@redhat.com>) > > CC: balrogg@gmail.com > --- > hw/arm/palm.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/hw/arm/palm.c b/hw/arm/palm.c > index 72eca8cc55..388b2629a5 100644 > --- a/hw/arm/palm.c > +++ b/hw/arm/palm.c > @@ -31,6 +31,7 @@ > #include "hw/loader.h" > #include "exec/address-spaces.h" > #include "cpu.h" > +#include "qemu/cutils.h" > > static uint64_t static_read(void *opaque, hwaddr offset, unsigned size) > { > @@ -181,7 +182,6 @@ static void palmte_gpio_setup(struct omap_mpu_state_s *cpu) > > static struct arm_boot_info palmte_binfo = { > .loader_start = OMAP_EMIFF_BASE, > - .ram_size = 0x02000000, Again, this is incorrect. Used by hw/arm/boot::do_cpu_reset(). > .board_id = 0x331, > }; > > @@ -195,15 +195,21 @@ static void palmte_init(MachineState *machine) > static uint32_t cs2val = 0x0000e1a0; > static uint32_t cs3val = 0xe1a0e1a0; > int rom_size, rom_loaded = 0; > - MemoryRegion *dram = g_new(MemoryRegion, 1); > + MachineClass *mc = MACHINE_GET_CLASS(machine); > MemoryRegion *flash = g_new(MemoryRegion, 1); > MemoryRegion *cs = g_new(MemoryRegion, 4); > > - memory_region_allocate_system_memory(dram, NULL, "omap1.dram", > - palmte_binfo.ram_size); > - memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, dram); > + if (machine->ram_size != mc->default_ram_size) { > + char *sz = size_to_str(mc->default_ram_size); > + error_report("Invalid RAM size, should be %s", sz); > + g_free(sz); > + exit(EXIT_FAILURE); > + } > + > + memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, > + machine->ram); > > - mpu = omap310_mpu_init(dram, machine->cpu_type); > + mpu = omap310_mpu_init(machine->ram, machine->cpu_type); > > /* External Flash (EMIFS) */ > memory_region_init_ram(flash, NULL, "palmte.flash", flash_size, > @@ -265,6 +271,8 @@ static void palmte_machine_init(MachineClass *mc) > mc->init = palmte_init; > mc->ignore_memory_transaction_failures = true; > mc->default_cpu_type = ARM_CPU_TYPE_NAME("ti925t"); > + mc->default_ram_size = 0x02000000; > + mc->default_ram_id = "omap1.dram"; > } > > DEFINE_MACHINE("cheetah", palmte_machine_init) >
On Tue, 18 Feb 2020 18:22:06 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > On 2/17/20 6:34 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: > > while at it add check for user supplied RAM size and error > > out if it mismatches board expected value. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > Reviewed-by: Andrew Jones <drjones@redhat.com> > > --- > > v2: > > * fix format string causing build failure on 32-bit host > > (Philippe Mathieu-Daudé <philmd@redhat.com>) > > > > CC: balrogg@gmail.com > > --- > > hw/arm/palm.c | 20 ++++++++++++++------ > > 1 file changed, 14 insertions(+), 6 deletions(-) > > > > diff --git a/hw/arm/palm.c b/hw/arm/palm.c > > index 72eca8cc55..388b2629a5 100644 > > --- a/hw/arm/palm.c > > +++ b/hw/arm/palm.c > > @@ -31,6 +31,7 @@ > > #include "hw/loader.h" > > #include "exec/address-spaces.h" > > #include "cpu.h" > > +#include "qemu/cutils.h" > > > > static uint64_t static_read(void *opaque, hwaddr offset, unsigned size) > > { > > @@ -181,7 +182,6 @@ static void palmte_gpio_setup(struct omap_mpu_state_s *cpu) > > > > static struct arm_boot_info palmte_binfo = { > > .loader_start = OMAP_EMIFF_BASE, > > - .ram_size = 0x02000000, > > Again, this is incorrect. Used by hw/arm/boot::do_cpu_reset(). Thanks, fixed in v6 > > > .board_id = 0x331, > > }; > > > > @@ -195,15 +195,21 @@ static void palmte_init(MachineState *machine) > > static uint32_t cs2val = 0x0000e1a0; > > static uint32_t cs3val = 0xe1a0e1a0; > > int rom_size, rom_loaded = 0; > > - MemoryRegion *dram = g_new(MemoryRegion, 1); > > + MachineClass *mc = MACHINE_GET_CLASS(machine); > > MemoryRegion *flash = g_new(MemoryRegion, 1); > > MemoryRegion *cs = g_new(MemoryRegion, 4); > > > > - memory_region_allocate_system_memory(dram, NULL, "omap1.dram", > > - palmte_binfo.ram_size); > > - memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, dram); > > + if (machine->ram_size != mc->default_ram_size) { > > + char *sz = size_to_str(mc->default_ram_size); > > + error_report("Invalid RAM size, should be %s", sz); > > + g_free(sz); > > + exit(EXIT_FAILURE); > > + } > > + > > + memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, > > + machine->ram); > > > > - mpu = omap310_mpu_init(dram, machine->cpu_type); > > + mpu = omap310_mpu_init(machine->ram, machine->cpu_type); > > > > /* External Flash (EMIFS) */ > > memory_region_init_ram(flash, NULL, "palmte.flash", flash_size, > > @@ -265,6 +271,8 @@ static void palmte_machine_init(MachineClass *mc) > > mc->init = palmte_init; > > mc->ignore_memory_transaction_failures = true; > > mc->default_cpu_type = ARM_CPU_TYPE_NAME("ti925t"); > > + mc->default_ram_size = 0x02000000; > > + mc->default_ram_id = "omap1.dram"; > > } > > > > DEFINE_MACHINE("cheetah", palmte_machine_init) > > >
On 2/19/20 2:44 PM, Igor Mammedov wrote: > On Tue, 18 Feb 2020 18:22:06 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > >> On 2/17/20 6:34 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: >>> while at it add check for user supplied RAM size and error >>> out if it mismatches board expected value. >>> >>> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >>> Reviewed-by: Andrew Jones <drjones@redhat.com> >>> --- >>> v2: >>> * fix format string causing build failure on 32-bit host >>> (Philippe Mathieu-Daudé <philmd@redhat.com>) >>> >>> CC: balrogg@gmail.com >>> --- >>> hw/arm/palm.c | 20 ++++++++++++++------ >>> 1 file changed, 14 insertions(+), 6 deletions(-) >>> >>> diff --git a/hw/arm/palm.c b/hw/arm/palm.c >>> index 72eca8cc55..388b2629a5 100644 >>> --- a/hw/arm/palm.c >>> +++ b/hw/arm/palm.c >>> @@ -31,6 +31,7 @@ >>> #include "hw/loader.h" >>> #include "exec/address-spaces.h" >>> #include "cpu.h" >>> +#include "qemu/cutils.h" >>> >>> static uint64_t static_read(void *opaque, hwaddr offset, unsigned size) >>> { >>> @@ -181,7 +182,6 @@ static void palmte_gpio_setup(struct omap_mpu_state_s *cpu) >>> >>> static struct arm_boot_info palmte_binfo = { >>> .loader_start = OMAP_EMIFF_BASE, >>> - .ram_size = 0x02000000, >> >> Again, this is incorrect. Used by hw/arm/boot::do_cpu_reset(). > Thanks, > fixed in v6 With the arm_boot_info ram_size unmodified, please add Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> >>> .board_id = 0x331, >>> }; >>> >>> @@ -195,15 +195,21 @@ static void palmte_init(MachineState *machine) >>> static uint32_t cs2val = 0x0000e1a0; >>> static uint32_t cs3val = 0xe1a0e1a0; >>> int rom_size, rom_loaded = 0; >>> - MemoryRegion *dram = g_new(MemoryRegion, 1); >>> + MachineClass *mc = MACHINE_GET_CLASS(machine); >>> MemoryRegion *flash = g_new(MemoryRegion, 1); >>> MemoryRegion *cs = g_new(MemoryRegion, 4); >>> >>> - memory_region_allocate_system_memory(dram, NULL, "omap1.dram", >>> - palmte_binfo.ram_size); >>> - memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, dram); >>> + if (machine->ram_size != mc->default_ram_size) { >>> + char *sz = size_to_str(mc->default_ram_size); >>> + error_report("Invalid RAM size, should be %s", sz); >>> + g_free(sz); >>> + exit(EXIT_FAILURE); >>> + } >>> + >>> + memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, >>> + machine->ram); >>> >>> - mpu = omap310_mpu_init(dram, machine->cpu_type); >>> + mpu = omap310_mpu_init(machine->ram, machine->cpu_type); >>> >>> /* External Flash (EMIFS) */ >>> memory_region_init_ram(flash, NULL, "palmte.flash", flash_size, >>> @@ -265,6 +271,8 @@ static void palmte_machine_init(MachineClass *mc) >>> mc->init = palmte_init; >>> mc->ignore_memory_transaction_failures = true; >>> mc->default_cpu_type = ARM_CPU_TYPE_NAME("ti925t"); >>> + mc->default_ram_size = 0x02000000; >>> + mc->default_ram_id = "omap1.dram"; >>> } >>> >>> DEFINE_MACHINE("cheetah", palmte_machine_init) >>> >> >
diff --git a/hw/arm/palm.c b/hw/arm/palm.c index 72eca8cc55..388b2629a5 100644 --- a/hw/arm/palm.c +++ b/hw/arm/palm.c @@ -31,6 +31,7 @@ #include "hw/loader.h" #include "exec/address-spaces.h" #include "cpu.h" +#include "qemu/cutils.h" static uint64_t static_read(void *opaque, hwaddr offset, unsigned size) { @@ -181,7 +182,6 @@ static void palmte_gpio_setup(struct omap_mpu_state_s *cpu) static struct arm_boot_info palmte_binfo = { .loader_start = OMAP_EMIFF_BASE, - .ram_size = 0x02000000, .board_id = 0x331, }; @@ -195,15 +195,21 @@ static void palmte_init(MachineState *machine) static uint32_t cs2val = 0x0000e1a0; static uint32_t cs3val = 0xe1a0e1a0; int rom_size, rom_loaded = 0; - MemoryRegion *dram = g_new(MemoryRegion, 1); + MachineClass *mc = MACHINE_GET_CLASS(machine); MemoryRegion *flash = g_new(MemoryRegion, 1); MemoryRegion *cs = g_new(MemoryRegion, 4); - memory_region_allocate_system_memory(dram, NULL, "omap1.dram", - palmte_binfo.ram_size); - memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, dram); + if (machine->ram_size != mc->default_ram_size) { + char *sz = size_to_str(mc->default_ram_size); + error_report("Invalid RAM size, should be %s", sz); + g_free(sz); + exit(EXIT_FAILURE); + } + + memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, + machine->ram); - mpu = omap310_mpu_init(dram, machine->cpu_type); + mpu = omap310_mpu_init(machine->ram, machine->cpu_type); /* External Flash (EMIFS) */ memory_region_init_ram(flash, NULL, "palmte.flash", flash_size, @@ -265,6 +271,8 @@ static void palmte_machine_init(MachineClass *mc) mc->init = palmte_init; mc->ignore_memory_transaction_failures = true; mc->default_cpu_type = ARM_CPU_TYPE_NAME("ti925t"); + mc->default_ram_size = 0x02000000; + mc->default_ram_id = "omap1.dram"; } DEFINE_MACHINE("cheetah", palmte_machine_init)