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,
--
2.33.0
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>
--
Thanks,
David / dhildenb
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
>
>
© 2016 - 2024 Red Hat, Inc.