| Message ID | 20220331115312.30018-2-damien.hedde@greensocs.com |
|---|---|
| State | New |
| Headers | show |
| Series | Generalize the sysbus device machine allowance | expand |
On Thu, Mar 31, 2022 at 01:53:08PM +0200, Damien Hedde wrote: > This flag will be used in device_add to check if > the device needs special allowance from the machine > model. > > It will replace the current check based only on the > device being a TYPE_SYB_BUS_DEVICE. > Looks good to me! Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com> > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > > v2: > + change the flag name and put it just below user_creatable > --- > include/hw/qdev-core.h | 9 +++++++++ > hw/core/qdev.c | 1 + > hw/core/sysbus.c | 1 + > 3 files changed, 11 insertions(+) > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 92c3d65208..6a040fcd3b 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -122,6 +122,15 @@ struct DeviceClass { > * TODO remove once we're there > */ > bool user_creatable; > + /* > + * Some devices can be user created under certain conditions (eg: > + * specific machine support for sysbus devices), but it is > + * preferable to prevent global allowance for the reasons > + * described above. > + * This flag is an additional constraint over user_creatable: > + * user_creatable still needs to be set to true. > + */ > + bool user_creatable_requires_machine_allowance; > bool hotpluggable; > > /* callbacks */ > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 84f3019440..0844c85a21 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -833,6 +833,7 @@ static void device_class_init(ObjectClass *class, void *data) > */ > dc->hotpluggable = true; > dc->user_creatable = true; > + dc->user_creatable_requires_machine_allowance = false; > vc->get_id = device_vmstate_if_get_id; > rc->get_state = device_get_reset_state; > rc->child_foreach = device_reset_child_foreach; > diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c > index 05c1da3d31..5f771ed1e9 100644 > --- a/hw/core/sysbus.c > +++ b/hw/core/sysbus.c > @@ -325,6 +325,7 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data) > * subclass needs to override it and set user_creatable=true. > */ > k->user_creatable = false; > + k->user_creatable_requires_machine_allowance = true; > } > > static const TypeInfo sysbus_device_type_info = { > -- > 2.35.1 > >
On Thu, 31 Mar 2022 at 13:19, Damien Hedde <damien.hedde@greensocs.com> wrote: > > This flag will be used in device_add to check if > the device needs special allowance from the machine > model. > > It will replace the current check based only on the > device being a TYPE_SYB_BUS_DEVICE. > > Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> > --- > > v2: > + change the flag name and put it just below user_creatable > --- > include/hw/qdev-core.h | 9 +++++++++ > hw/core/qdev.c | 1 + > hw/core/sysbus.c | 1 + > 3 files changed, 11 insertions(+) > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 92c3d65208..6a040fcd3b 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -122,6 +122,15 @@ struct DeviceClass { > * TODO remove once we're there > */ > bool user_creatable; > + /* > + * Some devices can be user created under certain conditions (eg: > + * specific machine support for sysbus devices), but it is > + * preferable to prevent global allowance for the reasons > + * described above. > + * This flag is an additional constraint over user_creatable: > + * user_creatable still needs to be set to true. > + */ > + bool user_creatable_requires_machine_allowance; "allowance" doesn't have the meaning you seem to be trying to give it here. (It means "the amount of something you're allowed to have", like a baggage allowance, or "an amount of money you're given for something", like a travel allowance.) Do you mean "user creatable only if the machine permits it" ? More generally, the pluggable-sysbus stuff is an awful hack that I wish we didn't have to have. I'm not sure I want to see us expanding the use of it to other device types... thanks -- PMM
On 4/21/22 17:59, Peter Maydell wrote: > On Thu, 31 Mar 2022 at 13:19, Damien Hedde <damien.hedde@greensocs.com> wrote: >> >> This flag will be used in device_add to check if >> the device needs special allowance from the machine >> model. >> >> It will replace the current check based only on the >> device being a TYPE_SYB_BUS_DEVICE. >> >> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> >> --- >> >> v2: >> + change the flag name and put it just below user_creatable >> --- >> include/hw/qdev-core.h | 9 +++++++++ >> hw/core/qdev.c | 1 + >> hw/core/sysbus.c | 1 + >> 3 files changed, 11 insertions(+) >> >> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h >> index 92c3d65208..6a040fcd3b 100644 >> --- a/include/hw/qdev-core.h >> +++ b/include/hw/qdev-core.h >> @@ -122,6 +122,15 @@ struct DeviceClass { >> * TODO remove once we're there >> */ >> bool user_creatable; >> + /* >> + * Some devices can be user created under certain conditions (eg: >> + * specific machine support for sysbus devices), but it is >> + * preferable to prevent global allowance for the reasons >> + * described above. >> + * This flag is an additional constraint over user_creatable: >> + * user_creatable still needs to be set to true. >> + */ >> + bool user_creatable_requires_machine_allowance; > > "allowance" doesn't have the meaning you seem to be trying to give it here. > (It means "the amount of something you're allowed to have", like > a baggage allowance, or "an amount of money you're given for something", > like a travel allowance.) > > Do you mean "user creatable only if the machine permits it" ? Yes. > > More generally, the pluggable-sysbus stuff is an awful hack > that I wish we didn't have to have. I'm not sure I want to see > us expanding the use of it to other device types... I not really trying to trigger code when adding devices. I'm just trying to use the related per-machine allow-list as a way to prevent the user to add such devices (specifically cpu group/cluster) on any random machine which would most probably not "work". Thanks, Damien
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 92c3d65208..6a040fcd3b 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -122,6 +122,15 @@ struct DeviceClass { * TODO remove once we're there */ bool user_creatable; + /* + * Some devices can be user created under certain conditions (eg: + * specific machine support for sysbus devices), but it is + * preferable to prevent global allowance for the reasons + * described above. + * This flag is an additional constraint over user_creatable: + * user_creatable still needs to be set to true. + */ + bool user_creatable_requires_machine_allowance; bool hotpluggable; /* callbacks */ diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 84f3019440..0844c85a21 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -833,6 +833,7 @@ static void device_class_init(ObjectClass *class, void *data) */ dc->hotpluggable = true; dc->user_creatable = true; + dc->user_creatable_requires_machine_allowance = false; vc->get_id = device_vmstate_if_get_id; rc->get_state = device_get_reset_state; rc->child_foreach = device_reset_child_foreach; diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c index 05c1da3d31..5f771ed1e9 100644 --- a/hw/core/sysbus.c +++ b/hw/core/sysbus.c @@ -325,6 +325,7 @@ static void sysbus_device_class_init(ObjectClass *klass, void *data) * subclass needs to override it and set user_creatable=true. */ k->user_creatable = false; + k->user_creatable_requires_machine_allowance = true; } static const TypeInfo sysbus_device_type_info = {
This flag will be used in device_add to check if the device needs special allowance from the machine model. It will replace the current check based only on the device being a TYPE_SYB_BUS_DEVICE. Signed-off-by: Damien Hedde <damien.hedde@greensocs.com> --- v2: + change the flag name and put it just below user_creatable --- include/hw/qdev-core.h | 9 +++++++++ hw/core/qdev.c | 1 + hw/core/sysbus.c | 1 + 3 files changed, 11 insertions(+)