All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xive: Make some device types not user creatable
@ 2019-10-04  7:38 Greg Kurz
  2019-10-04  8:27 ` Cédric Le Goater
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Greg Kurz @ 2019-10-04  7:38 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, Cédric Le Goater, qemu-devel

Some device types of the XIVE model are exposed to the QEMU command
line:

$ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive
name "xive-end-source", desc "XIVE END Source"
name "xive-source", desc "XIVE Interrupt Source"
name "xive-tctx", desc "XIVE Interrupt Thread Context"

These are internal devices that shouldn't be instantiable by the
user. By the way, they can't be because their respective realize
functions expect link properties that can't be set from the command
line:

qemu-system-ppc64: -device xive-source: required link 'xive' not found:
 Property '.xive' not found
qemu-system-ppc64: -device xive-end-source: required link 'xive' not found:
 Property '.xive' not found
qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found:
 Property '.cpu' not found

Hide them by setting dc->user_creatable to false in their respective
class init functions.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/intc/xive.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 29df06df1136..6c54a35fd4bb 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data)
     dc->realize = xive_tctx_realize;
     dc->unrealize = xive_tctx_unrealize;
     dc->vmsd = &vmstate_xive_tctx;
+    dc->user_creatable = false;
 }
 
 static const TypeInfo xive_tctx_info = {
@@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data)
     dc->props   = xive_source_properties;
     dc->realize = xive_source_realize;
     dc->vmsd    = &vmstate_xive_source;
+    dc->user_creatable = false;
 }
 
 static const TypeInfo xive_source_info = {
@@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data)
     dc->desc    = "XIVE END Source";
     dc->props   = xive_end_source_properties;
     dc->realize = xive_end_source_realize;
+    dc->user_creatable = false;
 }
 
 static const TypeInfo xive_end_source_info = {



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xive: Make some device types not user creatable
  2019-10-04  7:38 [PATCH] xive: Make some device types not user creatable Greg Kurz
@ 2019-10-04  8:27 ` Cédric Le Goater
  2019-10-04  9:17 ` Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2019-10-04  8:27 UTC (permalink / raw)
  To: Greg Kurz, David Gibson; +Cc: qemu-ppc, qemu-devel

On 04/10/2019 09:38, Greg Kurz wrote:
> Some device types of the XIVE model are exposed to the QEMU command
> line:
> 
> $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive
> name "xive-end-source", desc "XIVE END Source"
> name "xive-source", desc "XIVE Interrupt Source"
> name "xive-tctx", desc "XIVE Interrupt Thread Context"
> 
> These are internal devices that shouldn't be instantiable by the
> user. By the way, they can't be because their respective realize
> functions expect link properties that can't be set from the command
> line:
> 
> qemu-system-ppc64: -device xive-source: required link 'xive' not found:
>  Property '.xive' not found
> qemu-system-ppc64: -device xive-end-source: required link 'xive' not found:
>  Property '.xive' not found
> qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found:
>  Property '.cpu' not found
> 
> Hide them by setting dc->user_creatable to false in their respective
> class init functions.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

> ---
>  hw/intc/xive.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 29df06df1136..6c54a35fd4bb 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data)
>      dc->realize = xive_tctx_realize;
>      dc->unrealize = xive_tctx_unrealize;
>      dc->vmsd = &vmstate_xive_tctx;
> +    dc->user_creatable = false;
>  }
>  
>  static const TypeInfo xive_tctx_info = {
> @@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data)
>      dc->props   = xive_source_properties;
>      dc->realize = xive_source_realize;
>      dc->vmsd    = &vmstate_xive_source;
> +    dc->user_creatable = false;
>  }
>  
>  static const TypeInfo xive_source_info = {
> @@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data)
>      dc->desc    = "XIVE END Source";
>      dc->props   = xive_end_source_properties;
>      dc->realize = xive_end_source_realize;
> +    dc->user_creatable = false;
>  }
>  
>  static const TypeInfo xive_end_source_info = {
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xive: Make some device types not user creatable
  2019-10-04  7:38 [PATCH] xive: Make some device types not user creatable Greg Kurz
  2019-10-04  8:27 ` Cédric Le Goater
@ 2019-10-04  9:17 ` Philippe Mathieu-Daudé
  2019-10-04 10:00   ` Greg Kurz
  2019-10-05 10:21 ` David Gibson
  2019-10-07  9:02 ` Markus Armbruster
  3 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-04  9:17 UTC (permalink / raw)
  To: Greg Kurz, David Gibson, Thomas Huth, Eduardo Habkost
  Cc: Markus Armbruster, qemu-ppc, Cédric Le Goater, qemu-devel

Hi Greg,

On 10/4/19 9:38 AM, Greg Kurz wrote:
> Some device types of the XIVE model are exposed to the QEMU command
> line:
> 
> $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive
> name "xive-end-source", desc "XIVE END Source"
> name "xive-source", desc "XIVE Interrupt Source"
> name "xive-tctx", desc "XIVE Interrupt Thread Context"
> 
> These are internal devices that shouldn't be instantiable by the
> user. By the way, they can't be because their respective realize
> functions expect link properties that can't be set from the command
> line:
> 
> qemu-system-ppc64: -device xive-source: required link 'xive' not found:
>   Property '.xive' not found
> qemu-system-ppc64: -device xive-end-source: required link 'xive' not found:
>   Property '.xive' not found
> qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found:
>   Property '.cpu' not found

Why do you have to test that manually, isn't it what 
tests/device-introspect-test.c::test_one_device does?

> Hide them by setting dc->user_creatable to false in their respective
> class init functions.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>   hw/intc/xive.c |    3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 29df06df1136..6c54a35fd4bb 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data)
>       dc->realize = xive_tctx_realize;
>       dc->unrealize = xive_tctx_unrealize;
>       dc->vmsd = &vmstate_xive_tctx;
> +    dc->user_creatable = false;
>   }
>   
>   static const TypeInfo xive_tctx_info = {
> @@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data)
>       dc->props   = xive_source_properties;
>       dc->realize = xive_source_realize;
>       dc->vmsd    = &vmstate_xive_source;
> +    dc->user_creatable = false;
>   }
>   
>   static const TypeInfo xive_source_info = {
> @@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data)
>       dc->desc    = "XIVE END Source";
>       dc->props   = xive_end_source_properties;
>       dc->realize = xive_end_source_realize;
> +    dc->user_creatable = false;
>   }
>   
>   static const TypeInfo xive_end_source_info = {
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xive: Make some device types not user creatable
  2019-10-04  9:17 ` Philippe Mathieu-Daudé
@ 2019-10-04 10:00   ` Greg Kurz
  0 siblings, 0 replies; 6+ messages in thread
From: Greg Kurz @ 2019-10-04 10:00 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Eduardo Habkost, qemu-devel, Markus Armbruster,
	qemu-ppc, Cédric Le Goater, David Gibson

On Fri, 4 Oct 2019 11:17:30 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Hi Greg,
> 
> On 10/4/19 9:38 AM, Greg Kurz wrote:
> > Some device types of the XIVE model are exposed to the QEMU command
> > line:
> > 
> > $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive
> > name "xive-end-source", desc "XIVE END Source"
> > name "xive-source", desc "XIVE Interrupt Source"
> > name "xive-tctx", desc "XIVE Interrupt Thread Context"
> > 
> > These are internal devices that shouldn't be instantiable by the
> > user. By the way, they can't be because their respective realize
> > functions expect link properties that can't be set from the command
> > line:
> > 
> > qemu-system-ppc64: -device xive-source: required link 'xive' not found:
> >   Property '.xive' not found
> > qemu-system-ppc64: -device xive-end-source: required link 'xive' not found:
> >   Property '.xive' not found
> > qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found:
> >   Property '.cpu' not found
> 
> Why do you have to test that manually, isn't it what 
> tests/device-introspect-test.c::test_one_device does?
> 

Heh probably because I wasn't aware of it :)

And BTW, test_one_device() can't help here since it only cares
about 'device_add foo,help' not crashing QEMU:

    help = qtest_hmp(qts, "device_add \"%s,help\"", type);

as explained in a comment at the beginning of the file.

/*
 * Covers QMP device-list-properties and HMP device_add help.  We
 * currently don't check that their output makes sense, only that QEMU
 * survives.  Useful since we've had an astounding number of crash
 * bugs around here.
 */

> > Hide them by setting dc->user_creatable to false in their respective
> > class init functions.
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >   hw/intc/xive.c |    3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> > index 29df06df1136..6c54a35fd4bb 100644
> > --- a/hw/intc/xive.c
> > +++ b/hw/intc/xive.c
> > @@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data)
> >       dc->realize = xive_tctx_realize;
> >       dc->unrealize = xive_tctx_unrealize;
> >       dc->vmsd = &vmstate_xive_tctx;
> > +    dc->user_creatable = false;
> >   }
> >   
> >   static const TypeInfo xive_tctx_info = {
> > @@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data)
> >       dc->props   = xive_source_properties;
> >       dc->realize = xive_source_realize;
> >       dc->vmsd    = &vmstate_xive_source;
> > +    dc->user_creatable = false;
> >   }
> >   
> >   static const TypeInfo xive_source_info = {
> > @@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data)
> >       dc->desc    = "XIVE END Source";
> >       dc->props   = xive_end_source_properties;
> >       dc->realize = xive_end_source_realize;
> > +    dc->user_creatable = false;
> >   }
> >   
> >   static const TypeInfo xive_end_source_info = {
> > 
> > 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xive: Make some device types not user creatable
  2019-10-04  7:38 [PATCH] xive: Make some device types not user creatable Greg Kurz
  2019-10-04  8:27 ` Cédric Le Goater
  2019-10-04  9:17 ` Philippe Mathieu-Daudé
@ 2019-10-05 10:21 ` David Gibson
  2019-10-07  9:02 ` Markus Armbruster
  3 siblings, 0 replies; 6+ messages in thread
From: David Gibson @ 2019-10-05 10:21 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-ppc, Cédric Le Goater, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]

On Fri, Oct 04, 2019 at 09:38:50AM +0200, Greg Kurz wrote:
65;5603;1c> Some device types of the XIVE model are exposed to the QEMU command
> line:
> 
> $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive
> name "xive-end-source", desc "XIVE END Source"
> name "xive-source", desc "XIVE Interrupt Source"
> name "xive-tctx", desc "XIVE Interrupt Thread Context"
> 
> These are internal devices that shouldn't be instantiable by the
> user. By the way, they can't be because their respective realize
> functions expect link properties that can't be set from the command
> line:
> 
> qemu-system-ppc64: -device xive-source: required link 'xive' not found:
>  Property '.xive' not found
> qemu-system-ppc64: -device xive-end-source: required link 'xive' not found:
>  Property '.xive' not found
> qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found:
>  Property '.cpu' not found
> 
> Hide them by setting dc->user_creatable to false in their respective
> class init functions.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-4.2.

> ---
>  hw/intc/xive.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 29df06df1136..6c54a35fd4bb 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data)
>      dc->realize = xive_tctx_realize;
>      dc->unrealize = xive_tctx_unrealize;
>      dc->vmsd = &vmstate_xive_tctx;
> +    dc->user_creatable = false;
>  }
>  
>  static const TypeInfo xive_tctx_info = {
> @@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data)
>      dc->props   = xive_source_properties;
>      dc->realize = xive_source_realize;
>      dc->vmsd    = &vmstate_xive_source;
> +    dc->user_creatable = false;
>  }
>  
>  static const TypeInfo xive_source_info = {
> @@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data)
>      dc->desc    = "XIVE END Source";
>      dc->props   = xive_end_source_properties;
>      dc->realize = xive_end_source_realize;
> +    dc->user_creatable = false;
>  }
>  
>  static const TypeInfo xive_end_source_info = {
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xive: Make some device types not user creatable
  2019-10-04  7:38 [PATCH] xive: Make some device types not user creatable Greg Kurz
                   ` (2 preceding siblings ...)
  2019-10-05 10:21 ` David Gibson
@ 2019-10-07  9:02 ` Markus Armbruster
  3 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2019-10-07  9:02 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-devel, qemu-ppc, Cédric Le Goater, David Gibson

Greg Kurz <groug@kaod.org> writes:

> Some device types of the XIVE model are exposed to the QEMU command
> line:
>
> $ ppc64-softmmu/qemu-system-ppc64 -device help | grep xive
> name "xive-end-source", desc "XIVE END Source"
> name "xive-source", desc "XIVE Interrupt Source"
> name "xive-tctx", desc "XIVE Interrupt Thread Context"
>
> These are internal devices that shouldn't be instantiable by the
> user. By the way, they can't be because their respective realize
> functions expect link properties that can't be set from the command
> line:
>
> qemu-system-ppc64: -device xive-source: required link 'xive' not found:
>  Property '.xive' not found
> qemu-system-ppc64: -device xive-end-source: required link 'xive' not found:
>  Property '.xive' not found
> qemu-system-ppc64: -device xive-tctx: required link 'cpu' not found:
>  Property '.cpu' not found
>
> Hide them by setting dc->user_creatable to false in their respective
> class init functions.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/intc/xive.c |    3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index 29df06df1136..6c54a35fd4bb 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -670,6 +670,7 @@ static void xive_tctx_class_init(ObjectClass *klass, void *data)
>      dc->realize = xive_tctx_realize;
>      dc->unrealize = xive_tctx_unrealize;
>      dc->vmsd = &vmstate_xive_tctx;
> +    dc->user_creatable = false;
>  }
>  
>  static const TypeInfo xive_tctx_info = {
> @@ -1118,6 +1119,7 @@ static void xive_source_class_init(ObjectClass *klass, void *data)
>      dc->props   = xive_source_properties;
>      dc->realize = xive_source_realize;
>      dc->vmsd    = &vmstate_xive_source;
> +    dc->user_creatable = false;
>  }
>  
>  static const TypeInfo xive_source_info = {
> @@ -1853,6 +1855,7 @@ static void xive_end_source_class_init(ObjectClass *klass, void *data)
>      dc->desc    = "XIVE END Source";
>      dc->props   = xive_end_source_properties;
>      dc->realize = xive_end_source_realize;
> +    dc->user_creatable = false;
>  }
>  
>  static const TypeInfo xive_end_source_info = {

These all need a comment like the existing ->user_creatable = false
have.

Your commit message mentions link properties.  Based on that:

    /*
     * Reason: link property 'NAME-OF-PROP' needs to be wired up.
     */

Rather minimal, though.  Several existing similar cases are a bit more
specific, which is nice:

    /*
     * Reason: part of WHATEVER, needs to be wired up by FUNCTION().
     */

or if there is or could be more than one FUNCTION():

    /*
     * Reason: part of WHATEVER, needs to be wired up, e.g. by FUNCTION().
     */

David queued your patch already.  If it goes into master without such
comments, please post them as a follow-up patch.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-10-07  9:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-04  7:38 [PATCH] xive: Make some device types not user creatable Greg Kurz
2019-10-04  8:27 ` Cédric Le Goater
2019-10-04  9:17 ` Philippe Mathieu-Daudé
2019-10-04 10:00   ` Greg Kurz
2019-10-05 10:21 ` David Gibson
2019-10-07  9:02 ` Markus Armbruster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.