On 18/11/2019 00:20, Greg Kurz wrote:
> The ICS object has both a pointer and an ICS_PROP_XICS property pointing
> to the XICS fabric. Confusing bugs could arise if these ever go out of
> sync.
>
> Change the property definition so that it explicitely sets the pointer.
> The property isn't optional : not being able to set the link is a bug
> and QEMU should rather abort than exit in this case.
>
> Signed-off-by: Greg Kurz <gr...@kaod.org>
Reviewed-by: Cédric Le Goater <c...@kaod.org>
> ---
> hw/intc/xics.c | 13 +++----------
> hw/ppc/pnv_psi.c | 3 +--
> hw/ppc/spapr_irq.c | 9 ++-------
> 3 files changed, 6 insertions(+), 19 deletions(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index e7ac9ba618fa..f7a454808992 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -609,17 +609,8 @@ static void ics_reset_handler(void *dev)
> static void ics_realize(DeviceState *dev, Error **errp)
> {
> ICSState *ics = ICS(dev);
> - Error *local_err = NULL;
> - Object *obj;
>
> - obj = object_property_get_link(OBJECT(dev), ICS_PROP_XICS, &local_err);
> - if (!obj) {
> - error_propagate_prepend(errp, local_err,
> - "required link '" ICS_PROP_XICS
> - "' not found: ");
> - return;
> - }
> - ics->xics = XICS_FABRIC(obj);
> + assert(ics->xics);
>
> if (!ics->nr_irqs) {
> error_setg(errp, "Number of interrupts needs to be greater 0");
> @@ -699,6 +690,8 @@ static const VMStateDescription vmstate_ics = {
>
> static Property ics_properties[] = {
> DEFINE_PROP_UINT32("nr-irqs", ICSState, nr_irqs, 0),
> + DEFINE_PROP_LINK(ICS_PROP_XICS, ICSState, xics, TYPE_XICS_FABRIC,
> + XICSFabric *),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
> index a360515a86f8..7e725aaf2bd5 100644
> --- a/hw/ppc/pnv_psi.c
> +++ b/hw/ppc/pnv_psi.c
> @@ -497,8 +497,7 @@ static void pnv_psi_power8_realize(DeviceState *dev,
> Error **errp)
> }
>
> /* Create PSI interrupt control source */
> - object_property_add_const_link(OBJECT(ics), ICS_PROP_XICS, obj,
> - &error_abort);
> + object_property_set_link(OBJECT(ics), obj, ICS_PROP_XICS, &error_abort);
> object_property_set_int(OBJECT(ics), PSI_NUM_INTERRUPTS, "nr-irqs",
> &err);
> if (err) {
> error_propagate(errp, err);
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 168044be853a..487c8ceb35a3 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -319,13 +319,8 @@ void spapr_irq_init(SpaprMachineState *spapr, Error
> **errp)
> return;
> }
>
> - object_property_add_const_link(obj, ICS_PROP_XICS, OBJECT(spapr),
> - &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> - return;
> - }
> -
> + object_property_set_link(obj, OBJECT(spapr), ICS_PROP_XICS,
> + &error_abort);
> object_property_set_int(obj, smc->nr_xirqs, "nr-irqs", &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
>
