| Message ID | 20210922161405.140018-12-damien.hedde@greensocs.com |
|---|---|
| State | New |
| Headers | show |
| Series | Initial support for machine creation via QMP | expand |
On 9/22/21 18:14, Damien Hedde wrote: > It allows to try to add a subregion to a memory region with error > handling. Like memory_region_add_subregion_overlap, it handles > priority as well. > Apart the error handling, the behavior is the same. It can be used > to do the simple memory_region_add_subregion() (with no overlap) by > setting the priority parameter to 0. > > This commit is a preparation to further use this function in the > context of qmp command which needs error handling support. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > > Adding a new function is obviously not ideal. But there is ~900 > occurrences of memory_region_add_subregion[_overlap] calls in the code > base. We do not really see an alternative here. > --- > include/exec/memory.h | 22 ++++++++++++++++++++++ > softmmu/memory.c | 22 ++++++++++++++-------- > 2 files changed, 36 insertions(+), 8 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On 22.09.21 18:14, Damien Hedde wrote: > It allows to try to add a subregion to a memory region with error > handling. Like memory_region_add_subregion_overlap, it handles > priority as well. > Apart the error handling, the behavior is the same. It can be used > to do the simple memory_region_add_subregion() (with no overlap) by > setting the priority parameter to 0. > > This commit is a preparation to further use this function in the > context of qmp command which needs error handling support. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > > Adding a new function is obviously not ideal. But there is ~900 > occurrences of memory_region_add_subregion[_overlap] calls in the code > base. We do not really see an alternative here. > --- > include/exec/memory.h | 22 ++++++++++++++++++++++ > softmmu/memory.c | 22 ++++++++++++++-------- > 2 files changed, 36 insertions(+), 8 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index c3d417d317..422e1eda67 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -2162,6 +2162,28 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr, > MemoryRegion *subregion, > int priority); > > +/** > + * memory_region_try_add_subregion: Add a subregion to a container > + * with error handling. > + * > + * Behaves like memory_region_add_subregion_overlap(), but errors are > + * reported if the subregion cannot be added. > + * > + * @mr: the region to contain the new subregion; must be a container > + * initialized with memory_region_init(). > + * @offset: the offset relative to @mr where @subregion is added. > + * @subregion: the subregion to be added. > + * @priority: used for resolving overlaps; highest priority wins. > + * @errp: pointer to Error*, to store an error if it happens. > + * > + * Returns: True in case of success, false otherwise. > + */ > +bool memory_region_try_add_subregion(MemoryRegion *mr, > + hwaddr offset, > + MemoryRegion *subregion, > + int priority, > + Error **errp); > + > /** > * memory_region_get_ram_addr: Get the ram address associated with a memory > * region > diff --git a/softmmu/memory.c b/softmmu/memory.c > index bfedaf9c4d..eac61f8236 100644 > --- a/softmmu/memory.c > +++ b/softmmu/memory.c > @@ -2513,22 +2513,28 @@ done: > memory_region_transaction_commit(); > } > > -static void memory_region_add_subregion_common(MemoryRegion *mr, > - hwaddr offset, > - MemoryRegion *subregion) > +bool memory_region_try_add_subregion(MemoryRegion *mr, > + hwaddr offset, > + MemoryRegion *subregion, > + int priority, > + Error **errp) > { > - assert(!subregion->container); > + if (subregion->container) { > + error_setg(errp, "The memory region is already in another region"); > + return false; > + } > + subregion->priority = priority; > subregion->container = mr; > subregion->addr = offset; > memory_region_update_container_subregions(subregion); > + return true; > } > > void memory_region_add_subregion(MemoryRegion *mr, > hwaddr offset, > MemoryRegion *subregion) > { > - subregion->priority = 0; > - memory_region_add_subregion_common(mr, offset, subregion); > + memory_region_try_add_subregion(mr, offset, subregion, 0, &error_abort); > } > > void memory_region_add_subregion_overlap(MemoryRegion *mr, > @@ -2536,8 +2542,8 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr, > MemoryRegion *subregion, > int priority) > { > - subregion->priority = priority; > - memory_region_add_subregion_common(mr, offset, subregion); > + memory_region_try_add_subregion(mr, offset, subregion, priority, > + &error_abort); > } > > void memory_region_del_subregion(MemoryRegion *mr, > Reviewed-by: David Hildenbrand <david@redhat.com>
On Thu, Sep 23, 2021 at 2:29 AM Damien Hedde <damien.hedde@greensocs.com> wrote: > > It allows to try to add a subregion to a memory region with error > handling. Like memory_region_add_subregion_overlap, it handles > priority as well. > Apart the error handling, the behavior is the same. It can be used > to do the simple memory_region_add_subregion() (with no overlap) by > setting the priority parameter to 0. > > This commit is a preparation to further use this function in the > context of qmp command which needs error handling support. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > > Adding a new function is obviously not ideal. But there is ~900 > occurrences of memory_region_add_subregion[_overlap] calls in the code > base. We do not really see an alternative here. > --- > include/exec/memory.h | 22 ++++++++++++++++++++++ > softmmu/memory.c | 22 ++++++++++++++-------- > 2 files changed, 36 insertions(+), 8 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index c3d417d317..422e1eda67 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -2162,6 +2162,28 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr, > MemoryRegion *subregion, > int priority); > > +/** > + * memory_region_try_add_subregion: Add a subregion to a container > + * with error handling. > + * > + * Behaves like memory_region_add_subregion_overlap(), but errors are > + * reported if the subregion cannot be added. > + * > + * @mr: the region to contain the new subregion; must be a container > + * initialized with memory_region_init(). > + * @offset: the offset relative to @mr where @subregion is added. > + * @subregion: the subregion to be added. > + * @priority: used for resolving overlaps; highest priority wins. > + * @errp: pointer to Error*, to store an error if it happens. > + * > + * Returns: True in case of success, false otherwise. > + */ > +bool memory_region_try_add_subregion(MemoryRegion *mr, > + hwaddr offset, > + MemoryRegion *subregion, > + int priority, > + Error **errp); > + > /** > * memory_region_get_ram_addr: Get the ram address associated with a memory > * region > diff --git a/softmmu/memory.c b/softmmu/memory.c > index bfedaf9c4d..eac61f8236 100644 > --- a/softmmu/memory.c > +++ b/softmmu/memory.c > @@ -2513,22 +2513,28 @@ done: > memory_region_transaction_commit(); > } > > -static void memory_region_add_subregion_common(MemoryRegion *mr, > - hwaddr offset, > - MemoryRegion *subregion) > +bool memory_region_try_add_subregion(MemoryRegion *mr, > + hwaddr offset, > + MemoryRegion *subregion, > + int priority, > + Error **errp) > { > - assert(!subregion->container); > + if (subregion->container) { > + error_setg(errp, "The memory region is already in another region"); > + return false; > + } > + subregion->priority = priority; > subregion->container = mr; > subregion->addr = offset; > memory_region_update_container_subregions(subregion); > + return true; > } > > void memory_region_add_subregion(MemoryRegion *mr, > hwaddr offset, > MemoryRegion *subregion) > { > - subregion->priority = 0; > - memory_region_add_subregion_common(mr, offset, subregion); > + memory_region_try_add_subregion(mr, offset, subregion, 0, &error_abort); > } > > void memory_region_add_subregion_overlap(MemoryRegion *mr, > @@ -2536,8 +2542,8 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr, > MemoryRegion *subregion, > int priority) > { > - subregion->priority = priority; > - memory_region_add_subregion_common(mr, offset, subregion); > + memory_region_try_add_subregion(mr, offset, subregion, priority, > + &error_abort); > } > > void memory_region_del_subregion(MemoryRegion *mr, > -- > 2.33.0 > >
diff --git a/include/exec/memory.h b/include/exec/memory.h index c3d417d317..422e1eda67 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -2162,6 +2162,28 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr, MemoryRegion *subregion, int priority); +/** + * memory_region_try_add_subregion: Add a subregion to a container + * with error handling. + * + * Behaves like memory_region_add_subregion_overlap(), but errors are + * reported if the subregion cannot be added. + * + * @mr: the region to contain the new subregion; must be a container + * initialized with memory_region_init(). + * @offset: the offset relative to @mr where @subregion is added. + * @subregion: the subregion to be added. + * @priority: used for resolving overlaps; highest priority wins. + * @errp: pointer to Error*, to store an error if it happens. + * + * Returns: True in case of success, false otherwise. + */ +bool memory_region_try_add_subregion(MemoryRegion *mr, + hwaddr offset, + MemoryRegion *subregion, + int priority, + Error **errp); + /** * memory_region_get_ram_addr: Get the ram address associated with a memory * region diff --git a/softmmu/memory.c b/softmmu/memory.c index bfedaf9c4d..eac61f8236 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -2513,22 +2513,28 @@ done: memory_region_transaction_commit(); } -static void memory_region_add_subregion_common(MemoryRegion *mr, - hwaddr offset, - MemoryRegion *subregion) +bool memory_region_try_add_subregion(MemoryRegion *mr, + hwaddr offset, + MemoryRegion *subregion, + int priority, + Error **errp) { - assert(!subregion->container); + if (subregion->container) { + error_setg(errp, "The memory region is already in another region"); + return false; + } + subregion->priority = priority; subregion->container = mr; subregion->addr = offset; memory_region_update_container_subregions(subregion); + return true; } void memory_region_add_subregion(MemoryRegion *mr, hwaddr offset, MemoryRegion *subregion) { - subregion->priority = 0; - memory_region_add_subregion_common(mr, offset, subregion); + memory_region_try_add_subregion(mr, offset, subregion, 0, &error_abort); } void memory_region_add_subregion_overlap(MemoryRegion *mr, @@ -2536,8 +2542,8 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr, MemoryRegion *subregion, int priority) { - subregion->priority = priority; - memory_region_add_subregion_common(mr, offset, subregion); + memory_region_try_add_subregion(mr, offset, subregion, priority, + &error_abort); } void memory_region_del_subregion(MemoryRegion *mr,
It allows to try to add a subregion to a memory region with error handling. Like memory_region_add_subregion_overlap, it handles priority as well. Apart the error handling, the behavior is the same. It can be used to do the simple memory_region_add_subregion() (with no overlap) by setting the priority parameter to 0. This commit is a preparation to further use this function in the context of qmp command which needs error handling support. Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> --- Adding a new function is obviously not ideal. But there is ~900 occurrences of memory_region_add_subregion[_overlap] calls in the code base. We do not really see an alternative here. --- include/exec/memory.h | 22 ++++++++++++++++++++++ softmmu/memory.c | 22 ++++++++++++++-------- 2 files changed, 36 insertions(+), 8 deletions(-)