spapr/vio: deprecate the "irq" property

[Qemu-devel] [PATCH] spapr/vio: deprecate the "irq" property

Posted by Cédric Le Goater 5 years, 10 months ago

VIO devices have an "irq" property that can be used by the sPAPR IRQ
allocator as an IRQ number hint. But it is not set in QEMU nor in
libvirt. It brings unnecessary complexity to the underlying layers
managing the IRQ number space and it is in full opposition with the
new static IRQ allocator we want to introduce in sPAPR.

Let's deprecate it to simplify the spapr_irq_alloc routine in the
future.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 qemu-doc.texi      | 12 ++++++++++--
 hw/ppc/spapr_vio.c | 19 ++++++++++++++++++-
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index f00706b9996c..fd25ddf7a06a 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2970,13 +2970,21 @@ support page sizes < 4096 any longer.
 The ``xlnx-ep108'' machine has been replaced by the ``xlnx-zcu102'' machine.
 The ``xlnx-zcu102'' machine has the same features and capabilites in QEMU.
 
-@section Block device options
+@section Device options
 
-@subsection "backing": "" (since 2.12.0)
+@subsection Block device options
+
+@subsubsection "backing": "" (since 2.12.0)
 
 In order to prevent QEMU from automatically opening an image's backing
 chain, use ``"backing": null'' instead.
 
+@subsection vio-spapr-device device options
+
+@subsubsection "irq": "" (since 3.0.0)
+
+The ``irq'' property is obsoleted.
+
 @node Supported build platforms
 @appendix Supported build platforms
 
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 472dd6f33a96..28bb3b849b9b 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -22,6 +22,7 @@
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
+#include "qapi/visitor.h"
 #include "hw/hw.h"
 #include "qemu/log.h"
 #include "sysemu/sysemu.h"
@@ -41,8 +42,24 @@
 
 #include <libfdt.h>
 
+static void spapr_vio_getset_irq(Object *obj, Visitor *v, const char *name,
+                              void *opaque, Error **errp)
+{
+    Property *prop = opaque;
+    uint32_t *ptr = qdev_get_prop_ptr(DEVICE(obj), prop);
+
+    warn_report(TYPE_VIO_SPAPR_DEVICE " '%s' property is deprecated", name);
+    visit_type_uint32(v, name, ptr, errp);
+}
+
+static const PropertyInfo spapr_vio_irq_propinfo = {
+    .name = "irq",
+    .get = spapr_vio_getset_irq,
+    .set = spapr_vio_getset_irq,
+};
+
 static Property spapr_vio_props[] = {
-    DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, irq, 0), \
+    DEFINE_PROP("irq", VIOsPAPRDevice, irq, spapr_vio_irq_propinfo, uint32_t),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.13.6

Re: [Qemu-devel] [PATCH] spapr/vio: deprecate the "irq" property

Posted by Thomas Huth 5 years, 10 months ago

On 05.06.2018 08:56, Cédric Le Goater wrote:
> VIO devices have an "irq" property that can be used by the sPAPR IRQ
> allocator as an IRQ number hint. But it is not set in QEMU nor in
> libvirt. It brings unnecessary complexity to the underlying layers
> managing the IRQ number space and it is in full opposition with the
> new static IRQ allocator we want to introduce in sPAPR.
> 
> Let's deprecate it to simplify the spapr_irq_alloc routine in the
> future.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  qemu-doc.texi      | 12 ++++++++++--
>  hw/ppc/spapr_vio.c | 19 ++++++++++++++++++-
>  2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index f00706b9996c..fd25ddf7a06a 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2970,13 +2970,21 @@ support page sizes < 4096 any longer.
>  The ``xlnx-ep108'' machine has been replaced by the ``xlnx-zcu102'' machine.
>  The ``xlnx-zcu102'' machine has the same features and capabilites in QEMU.
>  
> -@section Block device options
> +@section Device options
>  
> -@subsection "backing": "" (since 2.12.0)
> +@subsection Block device options
> +
> +@subsubsection "backing": "" (since 2.12.0)
>  
>  In order to prevent QEMU from automatically opening an image's backing
>  chain, use ``"backing": null'' instead.
>  
> +@subsection vio-spapr-device device options
> +
> +@subsubsection "irq": "" (since 3.0.0)
> +
> +The ``irq'' property is obsoleted.
> +
>  @node Supported build platforms
>  @appendix Supported build platforms
>  
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index 472dd6f33a96..28bb3b849b9b 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -22,6 +22,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
> +#include "qapi/visitor.h"
>  #include "hw/hw.h"
>  #include "qemu/log.h"
>  #include "sysemu/sysemu.h"
> @@ -41,8 +42,24 @@
>  
>  #include <libfdt.h>
>  
> +static void spapr_vio_getset_irq(Object *obj, Visitor *v, const char *name,
> +                              void *opaque, Error **errp)
> +{
> +    Property *prop = opaque;
> +    uint32_t *ptr = qdev_get_prop_ptr(DEVICE(obj), prop);
> +
> +    warn_report(TYPE_VIO_SPAPR_DEVICE " '%s' property is deprecated", name);
> +    visit_type_uint32(v, name, ptr, errp);
> +}
> +
> +static const PropertyInfo spapr_vio_irq_propinfo = {
> +    .name = "irq",
> +    .get = spapr_vio_getset_irq,
> +    .set = spapr_vio_getset_irq,
> +};
> +
>  static Property spapr_vio_props[] = {
> -    DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, irq, 0), \
> +    DEFINE_PROP("irq", VIOsPAPRDevice, irq, spapr_vio_irq_propinfo, uint32_t),
>      DEFINE_PROP_END_OF_LIST(),
>  };

Looks fine to me.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH] spapr/vio: deprecate the "irq" property

Posted by Greg Kurz 5 years, 10 months ago

On Tue,  5 Jun 2018 08:56:26 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> VIO devices have an "irq" property that can be used by the sPAPR IRQ
> allocator as an IRQ number hint. But it is not set in QEMU nor in
> libvirt. It brings unnecessary complexity to the underlying layers
> managing the IRQ number space and it is in full opposition with the
> new static IRQ allocator we want to introduce in sPAPR.
> 
> Let's deprecate it to simplify the spapr_irq_alloc routine in the
> future.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  qemu-doc.texi      | 12 ++++++++++--
>  hw/ppc/spapr_vio.c | 19 ++++++++++++++++++-
>  2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index f00706b9996c..fd25ddf7a06a 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2970,13 +2970,21 @@ support page sizes < 4096 any longer.
>  The ``xlnx-ep108'' machine has been replaced by the ``xlnx-zcu102'' machine.
>  The ``xlnx-zcu102'' machine has the same features and capabilites in QEMU.
>  
> -@section Block device options
> +@section Device options
>  
> -@subsection "backing": "" (since 2.12.0)
> +@subsection Block device options
> +
> +@subsubsection "backing": "" (since 2.12.0)
>  
>  In order to prevent QEMU from automatically opening an image's backing
>  chain, use ``"backing": null'' instead.
>  
> +@subsection vio-spapr-device device options
> +
> +@subsubsection "irq": "" (since 3.0.0)
> +
> +The ``irq'' property is obsoleted.

"obsoleted" or "deprecated" ? Also maybe you could provide some
more context, so that users don't have to run git log to know
what's going on here ?

No big deal anyway, so:

Reviewed-by: Greg Kurz <groug@kaod.org>

> +
>  @node Supported build platforms
>  @appendix Supported build platforms
>  
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index 472dd6f33a96..28bb3b849b9b 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -22,6 +22,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
> +#include "qapi/visitor.h"
>  #include "hw/hw.h"
>  #include "qemu/log.h"
>  #include "sysemu/sysemu.h"
> @@ -41,8 +42,24 @@
>  
>  #include <libfdt.h>
>  
> +static void spapr_vio_getset_irq(Object *obj, Visitor *v, const char *name,
> +                              void *opaque, Error **errp)
> +{
> +    Property *prop = opaque;
> +    uint32_t *ptr = qdev_get_prop_ptr(DEVICE(obj), prop);
> +
> +    warn_report(TYPE_VIO_SPAPR_DEVICE " '%s' property is deprecated", name);
> +    visit_type_uint32(v, name, ptr, errp);
> +}
> +
> +static const PropertyInfo spapr_vio_irq_propinfo = {
> +    .name = "irq",
> +    .get = spapr_vio_getset_irq,
> +    .set = spapr_vio_getset_irq,
> +};
> +
>  static Property spapr_vio_props[] = {
> -    DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, irq, 0), \
> +    DEFINE_PROP("irq", VIOsPAPRDevice, irq, spapr_vio_irq_propinfo, uint32_t),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>

Re: [Qemu-devel] [PATCH] spapr/vio: deprecate the "irq" property

Posted by David Gibson 5 years, 10 months ago

On Tue, Jun 05, 2018 at 08:56:26AM +0200, Cédric Le Goater wrote:
> VIO devices have an "irq" property that can be used by the sPAPR IRQ
> allocator as an IRQ number hint. But it is not set in QEMU nor in
> libvirt. It brings unnecessary complexity to the underlying layers
> managing the IRQ number space and it is in full opposition with the
> new static IRQ allocator we want to introduce in sPAPR.
> 
> Let's deprecate it to simplify the spapr_irq_alloc routine in the
> future.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-3.0.

Should we also add a section for deprecated properties to
https://wiki.qemu.org/Features/LegacyRemoval ?

> ---
>  qemu-doc.texi      | 12 ++++++++++--
>  hw/ppc/spapr_vio.c | 19 ++++++++++++++++++-
>  2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index f00706b9996c..fd25ddf7a06a 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2970,13 +2970,21 @@ support page sizes < 4096 any longer.
>  The ``xlnx-ep108'' machine has been replaced by the ``xlnx-zcu102'' machine.
>  The ``xlnx-zcu102'' machine has the same features and capabilites in QEMU.
>  
> -@section Block device options
> +@section Device options
>  
> -@subsection "backing": "" (since 2.12.0)
> +@subsection Block device options
> +
> +@subsubsection "backing": "" (since 2.12.0)
>  
>  In order to prevent QEMU from automatically opening an image's backing
>  chain, use ``"backing": null'' instead.
>  
> +@subsection vio-spapr-device device options
> +
> +@subsubsection "irq": "" (since 3.0.0)
> +
> +The ``irq'' property is obsoleted.
> +
>  @node Supported build platforms
>  @appendix Supported build platforms
>  
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index 472dd6f33a96..28bb3b849b9b 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -22,6 +22,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
> +#include "qapi/visitor.h"
>  #include "hw/hw.h"
>  #include "qemu/log.h"
>  #include "sysemu/sysemu.h"
> @@ -41,8 +42,24 @@
>  
>  #include <libfdt.h>
>  
> +static void spapr_vio_getset_irq(Object *obj, Visitor *v, const char *name,
> +                              void *opaque, Error **errp)
> +{
> +    Property *prop = opaque;
> +    uint32_t *ptr = qdev_get_prop_ptr(DEVICE(obj), prop);
> +
> +    warn_report(TYPE_VIO_SPAPR_DEVICE " '%s' property is deprecated", name);
> +    visit_type_uint32(v, name, ptr, errp);
> +}
> +
> +static const PropertyInfo spapr_vio_irq_propinfo = {
> +    .name = "irq",
> +    .get = spapr_vio_getset_irq,
> +    .set = spapr_vio_getset_irq,
> +};
> +
>  static Property spapr_vio_props[] = {
> -    DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, irq, 0), \
> +    DEFINE_PROP("irq", VIOsPAPRDevice, irq, spapr_vio_irq_propinfo, uint32_t),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  

-- 
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

Re: [Qemu-devel] [PATCH] spapr/vio: deprecate the "irq" property

Posted by Greg Kurz 5 years, 10 months ago

On Wed, 6 Jun 2018 10:18:52 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Jun 05, 2018 at 08:56:26AM +0200, Cédric Le Goater wrote:
> > VIO devices have an "irq" property that can be used by the sPAPR IRQ
> > allocator as an IRQ number hint. But it is not set in QEMU nor in
> > libvirt. It brings unnecessary complexity to the underlying layers
> > managing the IRQ number space and it is in full opposition with the
> > new static IRQ allocator we want to introduce in sPAPR.
> > 
> > Let's deprecate it to simplify the spapr_irq_alloc routine in the
> > future.
> > 
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>  
> 
> Applied to ppc-for-3.0.
> 
> Should we also add a section for deprecated properties to
> https://wiki.qemu.org/Features/LegacyRemoval ?
> 

It isn't formally required but it wouldn't hurt.

"Note: This page is used to gather ideas about deprecated features and interfaces,
 the lists here are by no means mandatory!"

> > ---
> >  qemu-doc.texi      | 12 ++++++++++--
> >  hw/ppc/spapr_vio.c | 19 ++++++++++++++++++-
> >  2 files changed, 28 insertions(+), 3 deletions(-)
> > 
> > diff --git a/qemu-doc.texi b/qemu-doc.texi
> > index f00706b9996c..fd25ddf7a06a 100644
> > --- a/qemu-doc.texi
> > +++ b/qemu-doc.texi
> > @@ -2970,13 +2970,21 @@ support page sizes < 4096 any longer.
> >  The ``xlnx-ep108'' machine has been replaced by the ``xlnx-zcu102'' machine.
> >  The ``xlnx-zcu102'' machine has the same features and capabilites in QEMU.
> >  
> > -@section Block device options
> > +@section Device options
> >  
> > -@subsection "backing": "" (since 2.12.0)
> > +@subsection Block device options
> > +
> > +@subsubsection "backing": "" (since 2.12.0)
> >  
> >  In order to prevent QEMU from automatically opening an image's backing
> >  chain, use ``"backing": null'' instead.
> >  
> > +@subsection vio-spapr-device device options
> > +
> > +@subsubsection "irq": "" (since 3.0.0)
> > +
> > +The ``irq'' property is obsoleted.
> > +
> >  @node Supported build platforms
> >  @appendix Supported build platforms
> >  
> > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> > index 472dd6f33a96..28bb3b849b9b 100644
> > --- a/hw/ppc/spapr_vio.c
> > +++ b/hw/ppc/spapr_vio.c
> > @@ -22,6 +22,7 @@
> >  #include "qemu/osdep.h"
> >  #include "qemu/error-report.h"
> >  #include "qapi/error.h"
> > +#include "qapi/visitor.h"
> >  #include "hw/hw.h"
> >  #include "qemu/log.h"
> >  #include "sysemu/sysemu.h"
> > @@ -41,8 +42,24 @@
> >  
> >  #include <libfdt.h>
> >  
> > +static void spapr_vio_getset_irq(Object *obj, Visitor *v, const char *name,
> > +                              void *opaque, Error **errp)
> > +{
> > +    Property *prop = opaque;
> > +    uint32_t *ptr = qdev_get_prop_ptr(DEVICE(obj), prop);
> > +
> > +    warn_report(TYPE_VIO_SPAPR_DEVICE " '%s' property is deprecated", name);
> > +    visit_type_uint32(v, name, ptr, errp);
> > +}
> > +
> > +static const PropertyInfo spapr_vio_irq_propinfo = {
> > +    .name = "irq",
> > +    .get = spapr_vio_getset_irq,
> > +    .set = spapr_vio_getset_irq,
> > +};
> > +
> >  static Property spapr_vio_props[] = {
> > -    DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, irq, 0), \
> > +    DEFINE_PROP("irq", VIOsPAPRDevice, irq, spapr_vio_irq_propinfo, uint32_t),
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >    
>