Re: [PATCH v2 5/6] spapr/xive: Activate StoreEOI at the source level

[Greg Kurz]

On Tue, 6 Oct 2020 19:41:28 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> On 10/6/20 7:06 PM, Greg Kurz wrote:
> > On Mon, 5 Oct 2020 18:51:46 +0200
> > Cédric Le Goater <clg@kaod.org> wrote:
> > 
> >> When the StoreEOI capability is "on", the H_INT_GET_SOURCE_INFO will
> >> set the StoreEOI flag for all sources. This could be an issue if
> >> StoreEOI is not supported on a specific source, of a passthrough
> >> device for instance. In that case, we could either introduce a new KVM
> >> ioctl to query the characteristics of the source at the HW level or
> >> deactivate StoreEOI on the machine.
> >>
> >> This is theoretically unsafe on a POWER9 host but it still runs.
> >>
> > 
> > Patch looks good but as said before, what is the likeliness of something
> > really painful to happen on a POWER9 host ?
> 
> Nothing will happen because POWER9 systems deactivate StoreEOI. You need
> a custom skiboot to add it back.
> 

Ok, then:

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

> C.
> 
> 
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> ---
> >>  hw/intc/spapr_xive.c | 1 +
> >>  hw/ppc/spapr_irq.c   | 6 ++++++
> >>  2 files changed, 7 insertions(+)
> >>
> >> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> >> index 1fa09f287ac0..41f2719ff93a 100644
> >> --- a/hw/intc/spapr_xive.c
> >> +++ b/hw/intc/spapr_xive.c
> >> @@ -280,6 +280,7 @@ static void spapr_xive_instance_init(Object *obj)
> >>      SpaprXive *xive = SPAPR_XIVE(obj);
> >>  
> >>      object_initialize_child(obj, "source", &xive->source, 
> >> TYPE_XIVE_SOURCE);
> >> +    object_property_add_alias(obj, "flags", OBJECT(&xive->source), 
> >> "flags");
> >>  
> >>      object_initialize_child(obj, "end_source", &xive->end_source,
> >>                              TYPE_XIVE_END_SOURCE);
> >> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> >> index f59960339ec3..cdf9f9df4173 100644
> >> --- a/hw/ppc/spapr_irq.c
> >> +++ b/hw/ppc/spapr_irq.c
> >> @@ -325,9 +325,14 @@ void spapr_irq_init(SpaprMachineState *spapr, Error 
> >> **errp)
> >>  
> >>      if (spapr->irq->xive) {
> >>          uint32_t nr_servers = spapr_max_server_number(spapr);
> >> +        uint64_t flags = 0;
> >>          DeviceState *dev;
> >>          int i;
> >>  
> >> +        if (spapr_get_cap(spapr, SPAPR_CAP_STOREEOI) == SPAPR_CAP_ON) {
> >> +            flags |= XIVE_SRC_STORE_EOI;
> >> +        }
> >> +
> >>          dev = qdev_new(TYPE_SPAPR_XIVE);
> >>          qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + 
> >> SPAPR_XIRQ_BASE);
> >>          /*
> >> @@ -337,6 +342,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error 
> >> **errp)
> >>          qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3);
> >>          object_property_set_link(OBJECT(dev), "xive-fabric", 
> >> OBJECT(spapr),
> >>                                   &error_abort);
> >> +        object_property_set_int(OBJECT(dev), "flags", flags, 
> >> &error_abort);
> >>          sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> >>  
> >>          spapr->xive = SPAPR_XIVE(dev);
> > 
>