| Message ID | 20191008113318.7012-3-imammedo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | eliminate remaining places that abuse memory_region_allocate_system_memory() | expand |
On 10/8/19 1:33 PM, Igor Mammedov wrote: > rs6000mc_realize() violates memory_region_allocate_system_memory() contract > by calling it multiple times which could break -mem-path. Replace it with > plain memory_region_init_ram() instead. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/ppc/rs6000_mc.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/hw/ppc/rs6000_mc.c b/hw/ppc/rs6000_mc.c > index df7c0006fc..66b14db5fa 100644 > --- a/hw/ppc/rs6000_mc.c > +++ b/hw/ppc/rs6000_mc.c > @@ -144,6 +144,7 @@ static void rs6000mc_realize(DeviceState *dev, Error **errp) > RS6000MCState *s = RS6000MC_DEVICE(dev); > int socket = 0; > unsigned int ram_size = s->ram_size / MiB; > + Error *local_err = NULL; > > while (socket < 6) { > if (ram_size >= 64) { > @@ -165,19 +166,21 @@ static void rs6000mc_realize(DeviceState *dev, Error **errp) > if (s->simm_size[socket]) { > char name[] = "simm.?"; > name[5] = socket + '0'; > - memory_region_allocate_system_memory(&s->simm[socket], OBJECT(dev), > - name, > - s->simm_size[socket] * MiB); > + memory_region_init_ram(&s->simm[socket], OBJECT(dev), name, > + s->simm_size[socket] * MiB, &local_err); > + if (local_err) { > + goto out; > + } > memory_region_add_subregion_overlap(get_system_memory(), 0, > &s->simm[socket], socket); > } > } > if (ram_size) { > /* unable to push all requested RAM in SIMMs */ > - error_setg(errp, "RAM size incompatible with this board. " > + error_setg(&local_err, "RAM size incompatible with this board. " > "Try again with something else, like %" PRId64 " MB", > s->ram_size / MiB - ram_size); > - return; > + goto out; > } > > if (s->autoconfigure) { > @@ -193,6 +196,8 @@ static void rs6000mc_realize(DeviceState *dev, Error **errp) > > isa_register_portio_list(ISA_DEVICE(dev), &s->portio, 0x0, > rs6000mc_port_list, s, "rs6000mc"); > +out: > + error_propagate(errp, local_err); > } > > static const VMStateDescription vmstate_rs6000mc = { >
On Tue, Oct 08, 2019 at 07:33:17AM -0400, Igor Mammedov wrote: > rs6000mc_realize() violates memory_region_allocate_system_memory() contract > by calling it multiple times which could break -mem-path. Replace it with > plain memory_region_init_ram() instead. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> Acked-by: David Gibson <david@gibson.dropbear.id.au> Or would you like me to merge this through my tree?
On Wed, 9 Oct 2019 12:21:06 +1100 David Gibson <david@gibson.dropbear.id.au> wrote: > On Tue, Oct 08, 2019 at 07:33:17AM -0400, Igor Mammedov wrote: > > rs6000mc_realize() violates memory_region_allocate_system_memory() contract > > by calling it multiple times which could break -mem-path. Replace it with > > plain memory_region_init_ram() instead. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > Acked-by: David Gibson <david@gibson.dropbear.id.au> > > Or would you like me to merge this through my tree? (since it doesn't belong to a particular tree, I'd need find some to take in in) Since you volunteered :), please merge this series via your tree. Thanks!
On Wed, Oct 09, 2019 at 01:19:21PM +0200, Igor Mammedov wrote: > On Wed, 9 Oct 2019 12:21:06 +1100 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > On Tue, Oct 08, 2019 at 07:33:17AM -0400, Igor Mammedov wrote: > > > rs6000mc_realize() violates memory_region_allocate_system_memory() contract > > > by calling it multiple times which could break -mem-path. Replace it with > > > plain memory_region_init_ram() instead. > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > Acked-by: David Gibson <david@gibson.dropbear.id.au> > > > > Or would you like me to merge this through my tree? > > > (since it doesn't belong to a particular tree, I'd need find > some to take in in) > > Since you volunteered :), > please merge this series via your tree. Uhhh.. I was offering to take this patch, not the whole series. I'm not real comfortable taking the others, since they don't involve ppc code, nor are they prereqs for or motivated by any ppc change.
diff --git a/hw/ppc/rs6000_mc.c b/hw/ppc/rs6000_mc.c index df7c0006fc..66b14db5fa 100644 --- a/hw/ppc/rs6000_mc.c +++ b/hw/ppc/rs6000_mc.c @@ -144,6 +144,7 @@ static void rs6000mc_realize(DeviceState *dev, Error **errp) RS6000MCState *s = RS6000MC_DEVICE(dev); int socket = 0; unsigned int ram_size = s->ram_size / MiB; + Error *local_err = NULL; while (socket < 6) { if (ram_size >= 64) { @@ -165,19 +166,21 @@ static void rs6000mc_realize(DeviceState *dev, Error **errp) if (s->simm_size[socket]) { char name[] = "simm.?"; name[5] = socket + '0'; - memory_region_allocate_system_memory(&s->simm[socket], OBJECT(dev), - name, - s->simm_size[socket] * MiB); + memory_region_init_ram(&s->simm[socket], OBJECT(dev), name, + s->simm_size[socket] * MiB, &local_err); + if (local_err) { + goto out; + } memory_region_add_subregion_overlap(get_system_memory(), 0, &s->simm[socket], socket); } } if (ram_size) { /* unable to push all requested RAM in SIMMs */ - error_setg(errp, "RAM size incompatible with this board. " + error_setg(&local_err, "RAM size incompatible with this board. " "Try again with something else, like %" PRId64 " MB", s->ram_size / MiB - ram_size); - return; + goto out; } if (s->autoconfigure) { @@ -193,6 +196,8 @@ static void rs6000mc_realize(DeviceState *dev, Error **errp) isa_register_portio_list(ISA_DEVICE(dev), &s->portio, 0x0, rs6000mc_port_list, s, "rs6000mc"); +out: + error_propagate(errp, local_err); } static const VMStateDescription vmstate_rs6000mc = {
rs6000mc_realize() violates memory_region_allocate_system_memory() contract by calling it multiple times which could break -mem-path. Replace it with plain memory_region_init_ram() instead. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- hw/ppc/rs6000_mc.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)