Message ID | 1580483390-131164-39-git-send-email-imammedo@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | refactor main RAM allocation to use hostmem backend | expand |
On Fri, 31 Jan 2020 16:09:08 +0100 Igor Mammedov <imammedo@redhat.com> 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. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Reviewed-by: David Hildenbrand <david@redhat.com> > Acked-by: Cornelia Huck <cohuck@redhat.com> Hi Igor! I gave -object memory-backend-file,id=mem -machine type=s390-ccw-virtio,memory-backend=mem a spin on s390x. Seems to largely work a expected. So I guess it is: Tested-by: Halil Pasic <pasic@linux.ibm.com> Acked-by: Halil Pasic <pasic@linux.ibm.com> Thanks! Halil > --- > CC: pasic@linux.ibm.com > --- > hw/s390x/s390-virtio-ccw.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index e759eb5..a89cf4c 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -154,14 +154,12 @@ static void virtio_ccw_register_hcalls(void) > virtio_ccw_hcall_early_printk); > } > > -static void s390_memory_init(ram_addr_t mem_size) > +static void s390_memory_init(MemoryRegion *ram) > { > MemoryRegion *sysmem = get_system_memory(); > - MemoryRegion *ram = g_new(MemoryRegion, 1); > Error *local_err = NULL; > > /* allocate RAM for core */ > - memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size); > memory_region_add_subregion(sysmem, 0, ram); > > /* > @@ -245,7 +243,7 @@ static void ccw_init(MachineState *machine) > > s390_sclp_init(); > /* init memory + setup max page size. Required for the CPU model */ > - s390_memory_init(machine->ram_size); > + s390_memory_init(machine->ram); > > /* init CPUs (incl. CPU model) early so s390_has_feature() works */ > s390_init_cpus(machine); > @@ -471,6 +469,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) > hc->plug = s390_machine_device_plug; > hc->unplug_request = s390_machine_device_unplug_request; > nc->nmi_monitor_handler = s390_nmi; > + mc->default_ram_id = "s390.ram"; > } > > static inline bool machine_get_aes_key_wrap(Object *obj, Error **errp)
On Wed, 5 Feb 2020 21:11:23 +0100 Halil Pasic <pasic@linux.ibm.com> wrote: > On Fri, 31 Jan 2020 16:09:08 +0100 > Igor Mammedov <imammedo@redhat.com> 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. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > Reviewed-by: David Hildenbrand <david@redhat.com> > > Acked-by: Cornelia Huck <cohuck@redhat.com> > > Hi Igor! > > I gave > -object memory-backend-file,id=mem > -machine type=s390-ccw-virtio,memory-backend=mem > a spin on s390x. Seems to largely work a expected. So I guess it is: > > Tested-by: Halil Pasic <pasic@linux.ibm.com> > Acked-by: Halil Pasic <pasic@linux.ibm.com> Thanks, Could you also take a look at patches 3-7/8o that makes this possible? (it never hurts to have second pair of eyes on a code that affects everyone). > > Thanks! > > Halil > > > --- > > CC: pasic@linux.ibm.com > > --- > > hw/s390x/s390-virtio-ccw.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > > index e759eb5..a89cf4c 100644 > > --- a/hw/s390x/s390-virtio-ccw.c > > +++ b/hw/s390x/s390-virtio-ccw.c > > @@ -154,14 +154,12 @@ static void virtio_ccw_register_hcalls(void) > > virtio_ccw_hcall_early_printk); > > } > > > > -static void s390_memory_init(ram_addr_t mem_size) > > +static void s390_memory_init(MemoryRegion *ram) > > { > > MemoryRegion *sysmem = get_system_memory(); > > - MemoryRegion *ram = g_new(MemoryRegion, 1); > > Error *local_err = NULL; > > > > /* allocate RAM for core */ > > - memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size); > > memory_region_add_subregion(sysmem, 0, ram); > > > > /* > > @@ -245,7 +243,7 @@ static void ccw_init(MachineState *machine) > > > > s390_sclp_init(); > > /* init memory + setup max page size. Required for the CPU model */ > > - s390_memory_init(machine->ram_size); > > + s390_memory_init(machine->ram); > > > > /* init CPUs (incl. CPU model) early so s390_has_feature() works */ > > s390_init_cpus(machine); > > @@ -471,6 +469,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) > > hc->plug = s390_machine_device_plug; > > hc->unplug_request = s390_machine_device_unplug_request; > > nc->nmi_monitor_handler = s390_nmi; > > + mc->default_ram_id = "s390.ram"; > > } > > > > static inline bool machine_get_aes_key_wrap(Object *obj, Error **errp) >
On Thu, 6 Feb 2020 14:15:53 +0100 Igor Mammedov <imammedo@redhat.com> wrote: > > Tested-by: Halil Pasic <pasic@linux.ibm.com> > > Acked-by: Halil Pasic <pasic@linux.ibm.com> > Thanks, > > Could you also take a look at patches 3-7/8o that makes this possible? > (it never hurts to have second pair of eyes on a code that affects > everyone). Sorry, I'm in the middle of debugging something, and it does not look like I will be able to do a proper review on this soon. I did a quick scan through the code and I did not see anything fishy. Regards, Halil
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index e759eb5..a89cf4c 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -154,14 +154,12 @@ static void virtio_ccw_register_hcalls(void) virtio_ccw_hcall_early_printk); } -static void s390_memory_init(ram_addr_t mem_size) +static void s390_memory_init(MemoryRegion *ram) { MemoryRegion *sysmem = get_system_memory(); - MemoryRegion *ram = g_new(MemoryRegion, 1); Error *local_err = NULL; /* allocate RAM for core */ - memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size); memory_region_add_subregion(sysmem, 0, ram); /* @@ -245,7 +243,7 @@ static void ccw_init(MachineState *machine) s390_sclp_init(); /* init memory + setup max page size. Required for the CPU model */ - s390_memory_init(machine->ram_size); + s390_memory_init(machine->ram); /* init CPUs (incl. CPU model) early so s390_has_feature() works */ s390_init_cpus(machine); @@ -471,6 +469,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) hc->plug = s390_machine_device_plug; hc->unplug_request = s390_machine_device_unplug_request; nc->nmi_monitor_handler = s390_nmi; + mc->default_ram_id = "s390.ram"; } static inline bool machine_get_aes_key_wrap(Object *obj, Error **errp)