1. Patchew
  2. QEMU
  3. View series

[RFC PATCH] hw/misc: add a user creatable dummy device for debugging

Alex Bennée posted 1 patch 9 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240215141055.55173-1-alex.bennee@linaro.org
hw/misc/dummy.c     | 97 +++++++++++++++++++++++++++++++++++++++++++++
hw/misc/meson.build |  2 +-
2 files changed, 98 insertions(+), 1 deletion(-)
create mode 100644 hw/misc/dummy.c
[RFC PATCH] hw/misc: add a user creatable dummy device for debugging
Posted by Alex Bennée 9 months, 2 weeks ago 
This is an experimental patch to allow the creation of unimp regions
from the command line. I'm not sure we would ever merge it as
generally unimp regions are created by the machine definition and
adding your own regions into existing device spaces is likely to break
things pretty badly.

However what I was looking for was a simple way to test that a Xen XFT
test was accessing and trapping where I thought it was without hacking
up the machine models themselves. It allows the addition of unimp
regions to specific address spaces as in the future we shouldn't rely
on sysbus being the default but it's pretty clunky as you need to
intuit the full QOM path of the region, e.g.:

  /qemu-system-aarch64 -machine fby35 --display none \
    -serial mon:stdio -d unimp \
    --device dummy-device,name=dummy1,offset=0x10001000,size=0x1000,x-address-space="/machine/unattached/fsi.opb[0]"

Then running info mtree -f shows:

  ...
  FlatView #15
   AS "fsi.opb", root: fsi.opb
   Root memory region: fsi.opb
    0000000010001000-0000000010001fff (prio 0, i/o): dummy1
    0000000080000000-000000008fffffff (prio 0, i/o): fsi.master
    00000000a0000000-00000000a00003ff (prio 0, i/o): cfam.config
    00000000a0000400-00000000a00007ff (prio 0, i/o): cfam @0000000000000400
    00000000a0000800-00000000a0000bff (prio 0, i/o): fsi.slave
    00000000a0000c00-00000000a0000fff (prio 0, i/o): fsi.scratchpad
    00000000a0001000-00000000a01fffff (prio 0, i/o): cfam @0000000000001000

No need to review this RFC unless you particularly think this would be
useful to develop further.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/misc/dummy.c     | 97 +++++++++++++++++++++++++++++++++++++++++++++
 hw/misc/meson.build |  2 +-
 2 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100644 hw/misc/dummy.c

diff --git a/hw/misc/dummy.c b/hw/misc/dummy.c
new file mode 100644
index 00000000000..701c3468951
--- /dev/null
+++ b/hw/misc/dummy.c
@@ -0,0 +1,97 @@
+/*
+ * user creatable dummy device wrapper
+ *
+ * This is a simple wrapper around the unimp device
+ *
+ * Copyright (c) 2024 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "exec/address-spaces.h"
+#include "hw/misc/unimp.h"
+
+#define TYPE_DUMMY_DEVICE "dummy-device"
+
+OBJECT_DECLARE_SIMPLE_TYPE(DummyDeviceState, DUMMY_DEVICE)
+
+struct DummyDeviceState {
+    DeviceState parent_obj;
+
+    DeviceState *unimp;
+
+    MemoryRegion *mr;
+
+    char *name;
+    char *as_name;
+    uint64_t offset;
+    uint64_t size;
+};
+
+static void dummy_realize(DeviceState *dev, Error **errp)
+{
+    DummyDeviceState *s = DUMMY_DEVICE(dev);
+
+    if (s->size == 0) {
+        error_setg(errp, "property 'size' not specified or zero");
+        return;
+    }
+
+    if (s->name == NULL) {
+        error_setg(errp, "property 'name' not specified");
+        return;
+    }
+
+    if (s->as_name == NULL) {
+        s->mr = get_system_memory();
+    } else {
+        bool ambiguous = false;
+        /* this needs to be a full path. e.g. /machine/unattached/foo[0] */
+        Object *obj = object_resolve_path_type(s->as_name, TYPE_MEMORY_REGION, &ambiguous);
+        if (!obj || ambiguous) {
+            error_setg(errp, "Unable to find %s to locate region", s->as_name);
+            return;
+        }
+        s->mr = MEMORY_REGION(obj);
+    }
+
+    /*
+     * While this is a test device we don't want to make it too easy
+     * to shoot yourself in the foot.
+     */
+    s->unimp = qdev_new(TYPE_UNIMPLEMENTED_DEVICE);
+    qdev_prop_set_string(s->unimp, "name", s->name);
+    qdev_prop_set_uint64(s->unimp, "size", s->size);
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(s->unimp), &error_fatal);
+
+    /* Now lets map it to memory */
+    memory_region_add_subregion_overlap(s->mr, s->offset, &UNIMPLEMENTED_DEVICE(s->unimp)->iomem, 0);
+}
+
+static Property dummy_properties[] = {
+    DEFINE_PROP_UINT64("offset", DummyDeviceState, offset, 0),
+    DEFINE_PROP_UINT64("size", DummyDeviceState, size, 0),
+    DEFINE_PROP_STRING("name", DummyDeviceState, name),
+    DEFINE_PROP_STRING("x-address-space", DummyDeviceState, as_name),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void dummy_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = dummy_realize;
+    device_class_set_props(dc, dummy_properties);
+}
+
+static const TypeInfo dummy_devices[]  = {
+    {
+        .name = TYPE_DUMMY_DEVICE,
+        .parent = TYPE_DEVICE,
+        .instance_size = sizeof(DummyDeviceState),
+        .class_init = dummy_class_init,
+    }
+};
+
+DEFINE_TYPES(dummy_devices)
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index e4ef1da5a53..309314e2398 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -5,7 +5,7 @@ system_ss.add(when: 'CONFIG_ISA_DEBUG', if_true: files('debugexit.c'))
 system_ss.add(when: 'CONFIG_ISA_TESTDEV', if_true: files('pc-testdev.c'))
 system_ss.add(when: 'CONFIG_PCA9552', if_true: files('pca9552.c'))
 system_ss.add(when: 'CONFIG_PCI_TESTDEV', if_true: files('pci-testdev.c'))
-system_ss.add(when: 'CONFIG_UNIMP', if_true: files('unimp.c'))
+system_ss.add(when: 'CONFIG_UNIMP', if_true: files('unimp.c', 'dummy.c'))
 system_ss.add(when: 'CONFIG_EMPTY_SLOT', if_true: files('empty_slot.c'))
 system_ss.add(when: 'CONFIG_LED', if_true: files('led.c'))
 system_ss.add(when: 'CONFIG_PVPANIC_COMMON', if_true: files('pvpanic.c'))
-- 
2.39.2
Re: [RFC PATCH] hw/misc: add a user creatable dummy device for debugging
Posted by Philippe Mathieu-Daudé 9 months, 2 weeks ago 
On 15/2/24 15:10, Alex Bennée wrote:
> This is an experimental patch to allow the creation of unimp regions
> from the command line. I'm not sure we would ever merge it as
> generally unimp regions are created by the machine definition and
> adding your own regions into existing device spaces is likely to break
> things pretty badly.
> 
> However what I was looking for was a simple way to test that a Xen XFT
> test was accessing and trapping where I thought it was without hacking
> up the machine models themselves. It allows the addition of unimp
> regions to specific address spaces as in the future we shouldn't rely
> on sysbus being the default but it's pretty clunky as you need to
> intuit the full QOM path of the region, e.g.:
> 
>    /qemu-system-aarch64 -machine fby35 --display none \
>      -serial mon:stdio -d unimp \
>      --device dummy-device,name=dummy1,offset=0x10001000,size=0x1000,x-address-space="/machine/unattached/fsi.opb[0]"
> 
> Then running info mtree -f shows:
> 
>    ...
>    FlatView #15
>     AS "fsi.opb", root: fsi.opb
>     Root memory region: fsi.opb
>      0000000010001000-0000000010001fff (prio 0, i/o): dummy1
>      0000000080000000-000000008fffffff (prio 0, i/o): fsi.master
>      00000000a0000000-00000000a00003ff (prio 0, i/o): cfam.config
>      00000000a0000400-00000000a00007ff (prio 0, i/o): cfam @0000000000000400
>      00000000a0000800-00000000a0000bff (prio 0, i/o): fsi.slave
>      00000000a0000c00-00000000a0000fff (prio 0, i/o): fsi.scratchpad
>      00000000a0001000-00000000a01fffff (prio 0, i/o): cfam @0000000000001000
> 
> No need to review this RFC unless you particularly think this would be
> useful to develop further.

Some devices are only useful for QEMU hardness test suite,
but still are very useful in-tree.

https://lore.kernel.org/qemu-devel/20200817161853.593247-6-f4bug@amsat.org/

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/misc/dummy.c     | 97 +++++++++++++++++++++++++++++++++++++++++++++
>   hw/misc/meson.build |  2 +-
>   2 files changed, 98 insertions(+), 1 deletion(-)
>   create mode 100644 hw/misc/dummy.c
> 
> diff --git a/hw/misc/dummy.c b/hw/misc/dummy.c
> new file mode 100644
> index 00000000000..701c3468951
> --- /dev/null
> +++ b/hw/misc/dummy.c
> @@ -0,0 +1,97 @@
> +/*
> + * user creatable dummy device wrapper
> + *
> + * This is a simple wrapper around the unimp device
> + *
> + * Copyright (c) 2024 Linaro Ltd
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "exec/address-spaces.h"
> +#include "hw/misc/unimp.h"
> +
> +#define TYPE_DUMMY_DEVICE "dummy-device"
> +
> +OBJECT_DECLARE_SIMPLE_TYPE(DummyDeviceState, DUMMY_DEVICE)
> +
> +struct DummyDeviceState {
> +    DeviceState parent_obj;
> +
> +    DeviceState *unimp;
> +
> +    MemoryRegion *mr;
> +
> +    char *name;
> +    char *as_name;
> +    uint64_t offset;
> +    uint64_t size;
> +};
> +
> +static void dummy_realize(DeviceState *dev, Error **errp)
> +{
> +    DummyDeviceState *s = DUMMY_DEVICE(dev);
> +
> +    if (s->size == 0) {
> +        error_setg(errp, "property 'size' not specified or zero");
> +        return;
> +    }
> +
> +    if (s->name == NULL) {
> +        error_setg(errp, "property 'name' not specified");
> +        return;
> +    }
> +
> +    if (s->as_name == NULL) {
> +        s->mr = get_system_memory();
> +    } else {
> +        bool ambiguous = false;
> +        /* this needs to be a full path. e.g. /machine/unattached/foo[0] */
> +        Object *obj = object_resolve_path_type(s->as_name, TYPE_MEMORY_REGION, &ambiguous);

OMG yet another attempt of device which automatically map itself in
its REALIZE()...

> +        if (!obj || ambiguous) {
> +            error_setg(errp, "Unable to find %s to locate region", s->as_name);
> +            return;
> +        }
> +        s->mr = MEMORY_REGION(obj);
> +    }
> +
> +    /*
> +     * While this is a test device we don't want to make it too easy
> +     * to shoot yourself in the foot.
> +     */
> +    s->unimp = qdev_new(TYPE_UNIMPLEMENTED_DEVICE);
> +    qdev_prop_set_string(s->unimp, "name", s->name);
> +    qdev_prop_set_uint64(s->unimp, "size", s->size);
> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(s->unimp), &error_fatal);
> +
> +    /* Now lets map it to memory */
> +    memory_region_add_subregion_overlap(s->mr, s->offset, &UNIMPLEMENTED_DEVICE(s->unimp)->iomem, 0);

... here!

> +}
> +
> +static Property dummy_properties[] = {
> +    DEFINE_PROP_UINT64("offset", DummyDeviceState, offset, 0),
> +    DEFINE_PROP_UINT64("size", DummyDeviceState, size, 0),
> +    DEFINE_PROP_STRING("name", DummyDeviceState, name),
> +    DEFINE_PROP_STRING("x-address-space", DummyDeviceState, as_name),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void dummy_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = dummy_realize;
> +    device_class_set_props(dc, dummy_properties);
> +}
> +
> +static const TypeInfo dummy_devices[]  = {
> +    {
> +        .name = TYPE_DUMMY_DEVICE,
> +        .parent = TYPE_DEVICE,
> +        .instance_size = sizeof(DummyDeviceState),
> +        .class_init = dummy_class_init,
> +    }
> +};
> +
> +DEFINE_TYPES(dummy_devices)
> diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> index e4ef1da5a53..309314e2398 100644
> --- a/hw/misc/meson.build
> +++ b/hw/misc/meson.build
> @@ -5,7 +5,7 @@ system_ss.add(when: 'CONFIG_ISA_DEBUG', if_true: files('debugexit.c'))
>   system_ss.add(when: 'CONFIG_ISA_TESTDEV', if_true: files('pc-testdev.c'))
>   system_ss.add(when: 'CONFIG_PCA9552', if_true: files('pca9552.c'))
>   system_ss.add(when: 'CONFIG_PCI_TESTDEV', if_true: files('pci-testdev.c'))
> -system_ss.add(when: 'CONFIG_UNIMP', if_true: files('unimp.c'))
> +system_ss.add(when: 'CONFIG_UNIMP', if_true: files('unimp.c', 'dummy.c'))
>   system_ss.add(when: 'CONFIG_EMPTY_SLOT', if_true: files('empty_slot.c'))
>   system_ss.add(when: 'CONFIG_LED', if_true: files('led.c'))
>   system_ss.add(when: 'CONFIG_PVPANIC_COMMON', if_true: files('pvpanic.c'))

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.