[RFC PATCH v2 09/16] hw/core/machine: Remove the dynamic sysbus devices type check

Damien Hedde posted 16 patches 3 years, 2 months ago
There is a newer version of this series
[RFC PATCH v2 09/16] hw/core/machine: Remove the dynamic sysbus devices type check
Posted by Damien Hedde 3 years, 2 months ago
Now that we check sysbus device types during device creation, we
can remove the check done in the machine init done notifier.
This was the only thing done by this notifier, so we remove the
whole sysbus_notifier structure of the MachineState.

Note: This notifier was checking all /peripheral and /peripheral-anon
sysbus devices. Now we only check those added by -device cli option or
device_add qmp command when handling the command/option. So if there
are some devices added in one of these containers manually (eg in
machine C code), these will not be checked anymore.
This use case does not seem to appear apart from
hw/xen/xen-legacy-backend.c (it uses qdev_set_id() and in this case,
not for a sysbus device, so it's ok).

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
 include/hw/boards.h |  1 -
 hw/core/machine.c   | 27 ---------------------------
 2 files changed, 28 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 934443c1cd..ccbc40355a 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -311,7 +311,6 @@ typedef struct CpuTopology {
 struct MachineState {
     /*< private >*/
     Object parent_obj;
-    Notifier sysbus_notifier;
 
     /*< public >*/
 
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1a18912dc8..521438e90a 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -571,18 +571,6 @@ bool machine_class_is_dynamic_sysbus_dev_allowed(MachineClass *mc,
     return allowed;
 }
 
-static void validate_sysbus_device(SysBusDevice *sbdev, void *opaque)
-{
-    MachineState *machine = opaque;
-    MachineClass *mc = MACHINE_GET_CLASS(machine);
-
-    if (!device_is_dynamic_sysbus(mc, DEVICE(sbdev))) {
-        error_report("Option '-device %s' cannot be handled by this machine",
-                     object_class_get_name(object_get_class(OBJECT(sbdev))));
-        exit(1);
-    }
-}
-
 static char *machine_get_memdev(Object *obj, Error **errp)
 {
     MachineState *ms = MACHINE(obj);
@@ -598,17 +586,6 @@ static void machine_set_memdev(Object *obj, const char *value, Error **errp)
     ms->ram_memdev_id = g_strdup(value);
 }
 
-static void machine_init_notify(Notifier *notifier, void *data)
-{
-    MachineState *machine = MACHINE(qdev_get_machine());
-
-    /*
-     * Loop through all dynamically created sysbus devices and check if they are
-     * all allowed.  If a device is not allowed, error out.
-     */
-    foreach_dynamic_sysbus_device(validate_sysbus_device, machine);
-}
-
 HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
 {
     int i;
@@ -1030,10 +1007,6 @@ static void machine_initfn(Object *obj)
                                         "Table (HMAT)");
     }
 
-    /* Register notifier when init is done for sysbus sanity checks */
-    ms->sysbus_notifier.notify = machine_init_notify;
-    qemu_add_machine_init_done_notifier(&ms->sysbus_notifier);
-
     /* default to mc->default_cpus */
     ms->smp.cpus = mc->default_cpus;
     ms->smp.max_cpus = mc->default_cpus;
-- 
2.33.0


Re: [RFC PATCH v2 09/16] hw/core/machine: Remove the dynamic sysbus devices type check
Posted by Alistair Francis 3 years, 1 month ago
On Thu, Sep 23, 2021 at 2:23 AM Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> Now that we check sysbus device types during device creation, we
> can remove the check done in the machine init done notifier.
> This was the only thing done by this notifier, so we remove the
> whole sysbus_notifier structure of the MachineState.
>
> Note: This notifier was checking all /peripheral and /peripheral-anon
> sysbus devices. Now we only check those added by -device cli option or
> device_add qmp command when handling the command/option. So if there
> are some devices added in one of these containers manually (eg in
> machine C code), these will not be checked anymore.
> This use case does not seem to appear apart from
> hw/xen/xen-legacy-backend.c (it uses qdev_set_id() and in this case,
> not for a sysbus device, so it's ok).
>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>

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

Alistair

> ---
>  include/hw/boards.h |  1 -
>  hw/core/machine.c   | 27 ---------------------------
>  2 files changed, 28 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 934443c1cd..ccbc40355a 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -311,7 +311,6 @@ typedef struct CpuTopology {
>  struct MachineState {
>      /*< private >*/
>      Object parent_obj;
> -    Notifier sysbus_notifier;
>
>      /*< public >*/
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 1a18912dc8..521438e90a 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -571,18 +571,6 @@ bool machine_class_is_dynamic_sysbus_dev_allowed(MachineClass *mc,
>      return allowed;
>  }
>
> -static void validate_sysbus_device(SysBusDevice *sbdev, void *opaque)
> -{
> -    MachineState *machine = opaque;
> -    MachineClass *mc = MACHINE_GET_CLASS(machine);
> -
> -    if (!device_is_dynamic_sysbus(mc, DEVICE(sbdev))) {
> -        error_report("Option '-device %s' cannot be handled by this machine",
> -                     object_class_get_name(object_get_class(OBJECT(sbdev))));
> -        exit(1);
> -    }
> -}
> -
>  static char *machine_get_memdev(Object *obj, Error **errp)
>  {
>      MachineState *ms = MACHINE(obj);
> @@ -598,17 +586,6 @@ static void machine_set_memdev(Object *obj, const char *value, Error **errp)
>      ms->ram_memdev_id = g_strdup(value);
>  }
>
> -static void machine_init_notify(Notifier *notifier, void *data)
> -{
> -    MachineState *machine = MACHINE(qdev_get_machine());
> -
> -    /*
> -     * Loop through all dynamically created sysbus devices and check if they are
> -     * all allowed.  If a device is not allowed, error out.
> -     */
> -    foreach_dynamic_sysbus_device(validate_sysbus_device, machine);
> -}
> -
>  HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
>  {
>      int i;
> @@ -1030,10 +1007,6 @@ static void machine_initfn(Object *obj)
>                                          "Table (HMAT)");
>      }
>
> -    /* Register notifier when init is done for sysbus sanity checks */
> -    ms->sysbus_notifier.notify = machine_init_notify;
> -    qemu_add_machine_init_done_notifier(&ms->sysbus_notifier);
> -
>      /* default to mc->default_cpus */
>      ms->smp.cpus = mc->default_cpus;
>      ms->smp.max_cpus = mc->default_cpus;
> --
> 2.33.0
>
>