[RFC PATCH v2 12/16] add x-sysbus-mmio-map qmp command

Damien Hedde posted 16 patches 3 years, 2 months ago
There is a newer version of this series
[RFC PATCH v2 12/16] add x-sysbus-mmio-map qmp command
Posted by Damien Hedde 3 years, 2 months ago
This command allows to map an mmio region of sysbus device onto
the system memory. Its behavior mimics the sysbus_mmio_map()
function apart from the automatic unmap (the C function unmaps
the region if it is already mapped).
For the qmp function we consider it is an error to try to map
an already mapped function. If unmapping is required, it is
probably better to add a sysbus-mmip-unmap function.

This command is still experimental (hence the x prefix), as it
is related to the sysbus device creation through qmp commands.

In future, we expect to have to handle the overlap/priority
parameter but also multiple mapping of one mmio. For some
devices, one mmio is mapped several times at different addresses on
the bus (which is not supported by sysbus_mmio_map() function and
requires the use of memory region aliases).

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---

Note: this qmp command is required to be able to build a machine from
scratch as there is no qmp-way of doing a memory mapping today.

We've added the command into qapi/qdev.json section. It does not seem to
have any really adequate section yet. Any idea ? should we create for
example a new one: qapi/sysbus.json or qapi/memory.json ?
---
 qapi/qdev.json   | 21 +++++++++++++++++++++
 hw/core/sysbus.c | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/qapi/qdev.json b/qapi/qdev.json
index ad669ae175..dfc1104197 100644
--- a/qapi/qdev.json
+++ b/qapi/qdev.json
@@ -125,3 +125,24 @@
 ##
 { 'event': 'DEVICE_DELETED',
   'data': { '*device': 'str', 'path': 'str' } }
+
+##
+# @x-sysbus-mmio-map:
+#
+# Map a sysbus device mmio onto the main system bus.
+#
+# @device: the device's QOM path
+#
+# @mmio: The mmio number to be mapped (defaults to 0).
+#
+# @addr: The base address for the mapping.
+#
+# Since: 6.2
+#
+# Returns: Nothing on success
+#
+##
+
+{ 'command': 'x-sysbus-mmio-map',
+  'data': {'device': 'str', '*mmio': 'uint8', 'addr': 'uint64'},
+  'allow-preconfig' : true }
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index aaae8e23cc..b0891f37b6 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -23,6 +23,7 @@
 #include "hw/sysbus.h"
 #include "monitor/monitor.h"
 #include "exec/address-spaces.h"
+#include "qapi/qapi-commands-qdev.h"
 
 static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent);
 static char *sysbus_get_fw_dev_path(DeviceState *dev);
@@ -154,6 +155,46 @@ static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
     }
 }
 
+void qmp_x_sysbus_mmio_map(const char *device, bool has_mmio, uint8_t mmio,
+                           uint64_t addr, Error **errp)
+{
+    Object *obj = object_resolve_path_type(device, TYPE_SYS_BUS_DEVICE, NULL);
+    SysBusDevice *dev;
+
+    if (phase_get() != MACHINE_INIT_PHASE_INITIALIZED) {
+        error_setg(errp, "The command is permitted only when "
+                         "the machine is in initialized phase");
+        return;
+    }
+
+    if (obj == NULL) {
+        error_setg(errp, "Device '%s' not found", device);
+        return;
+    }
+
+    dev = SYS_BUS_DEVICE(obj);
+    if (!has_mmio) {
+        mmio = 0;
+    }
+    if (mmio >= dev->num_mmio) {
+        error_setg(errp, "MMIO index '%u' is out of range", mmio);
+        return;
+    }
+
+    if (dev->mmio[mmio].addr != (hwaddr)-1) {
+        error_setg(errp, "MMIO index '%u' is already mapped", mmio);
+        return;
+    }
+
+    if (!memory_region_try_add_subregion(get_system_memory(), addr,
+                                         dev->mmio[mmio].memory, 0,
+                                         errp)) {
+        return;
+    }
+
+    dev->mmio[mmio].addr = addr;
+}
+
 void sysbus_mmio_unmap(SysBusDevice *dev, int n)
 {
     assert(n >= 0 && n < dev->num_mmio);
-- 
2.33.0


Re: [RFC PATCH v2 12/16] add x-sysbus-mmio-map qmp command
Posted by Alistair Francis 3 years, 1 month ago
On Thu, Sep 23, 2021 at 2:26 AM Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> This command allows to map an mmio region of sysbus device onto
> the system memory. Its behavior mimics the sysbus_mmio_map()
> function apart from the automatic unmap (the C function unmaps
> the region if it is already mapped).
> For the qmp function we consider it is an error to try to map
> an already mapped function. If unmapping is required, it is
> probably better to add a sysbus-mmip-unmap function.
>
> This command is still experimental (hence the x prefix), as it
> is related to the sysbus device creation through qmp commands.
>
> In future, we expect to have to handle the overlap/priority
> parameter but also multiple mapping of one mmio. For some
> devices, one mmio is mapped several times at different addresses on
> the bus (which is not supported by sysbus_mmio_map() function and
> requires the use of memory region aliases).

I think as is this is a good start. This is a useful feature!

>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

> ---
>
> Note: this qmp command is required to be able to build a machine from
> scratch as there is no qmp-way of doing a memory mapping today.
>
> We've added the command into qapi/qdev.json section. It does not seem to
> have any really adequate section yet. Any idea ? should we create for
> example a new one: qapi/sysbus.json or qapi/memory.json ?

I would say leave it in qdev. We don't really want many more sysbus
commands, so qapi/sysbus.json doesn't need it's own file.

Alistair

> ---
>  qapi/qdev.json   | 21 +++++++++++++++++++++
>  hw/core/sysbus.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+)
>
> diff --git a/qapi/qdev.json b/qapi/qdev.json
> index ad669ae175..dfc1104197 100644
> --- a/qapi/qdev.json
> +++ b/qapi/qdev.json
> @@ -125,3 +125,24 @@
>  ##
>  { 'event': 'DEVICE_DELETED',
>    'data': { '*device': 'str', 'path': 'str' } }
> +
> +##
> +# @x-sysbus-mmio-map:
> +#
> +# Map a sysbus device mmio onto the main system bus.
> +#
> +# @device: the device's QOM path
> +#
> +# @mmio: The mmio number to be mapped (defaults to 0).
> +#
> +# @addr: The base address for the mapping.
> +#
> +# Since: 6.2
> +#
> +# Returns: Nothing on success
> +#
> +##
> +
> +{ 'command': 'x-sysbus-mmio-map',
> +  'data': {'device': 'str', '*mmio': 'uint8', 'addr': 'uint64'},
> +  'allow-preconfig' : true }
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index aaae8e23cc..b0891f37b6 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -23,6 +23,7 @@
>  #include "hw/sysbus.h"
>  #include "monitor/monitor.h"
>  #include "exec/address-spaces.h"
> +#include "qapi/qapi-commands-qdev.h"
>
>  static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent);
>  static char *sysbus_get_fw_dev_path(DeviceState *dev);
> @@ -154,6 +155,46 @@ static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
>      }
>  }
>
> +void qmp_x_sysbus_mmio_map(const char *device, bool has_mmio, uint8_t mmio,
> +                           uint64_t addr, Error **errp)
> +{
> +    Object *obj = object_resolve_path_type(device, TYPE_SYS_BUS_DEVICE, NULL);
> +    SysBusDevice *dev;
> +
> +    if (phase_get() != MACHINE_INIT_PHASE_INITIALIZED) {
> +        error_setg(errp, "The command is permitted only when "
> +                         "the machine is in initialized phase");
> +        return;
> +    }
> +
> +    if (obj == NULL) {
> +        error_setg(errp, "Device '%s' not found", device);
> +        return;
> +    }
> +
> +    dev = SYS_BUS_DEVICE(obj);
> +    if (!has_mmio) {
> +        mmio = 0;
> +    }
> +    if (mmio >= dev->num_mmio) {
> +        error_setg(errp, "MMIO index '%u' is out of range", mmio);
> +        return;
> +    }
> +
> +    if (dev->mmio[mmio].addr != (hwaddr)-1) {
> +        error_setg(errp, "MMIO index '%u' is already mapped", mmio);
> +        return;
> +    }
> +
> +    if (!memory_region_try_add_subregion(get_system_memory(), addr,
> +                                         dev->mmio[mmio].memory, 0,
> +                                         errp)) {
> +        return;
> +    }
> +
> +    dev->mmio[mmio].addr = addr;
> +}
> +
>  void sysbus_mmio_unmap(SysBusDevice *dev, int n)
>  {
>      assert(n >= 0 && n < dev->num_mmio);
> --
> 2.33.0
>
>