On Thu, Oct 17, 2019 at 03:42:06PM +0200, Cédric Le Goater wrote: > On 17/10/2019 10:43, Cédric Le Goater wrote: > > On 17/10/2019 07:42, David Gibson wrote: > >> Traditional PCI INTx for vfio devices can only perform well if using > >> an in-kernel irqchip. Therefore, vfio_intx_update() issues a warning > >> if an in kernel irqchip is not available. > >> > >> We usually do have an in-kernel irqchip available for pseries machines > >> on POWER hosts. However, because the platform allows feature > >> negotiation of what interrupt controller model to use, we don't > >> currently initialize it until machine reset. vfio_intx_update() is > >> called (first) from vfio_realize() before that, so it can issue a > >> spurious warning, even if we will have an in kernel irqchip by the > >> time we need it. > >> > >> To workaround this, make a call to spapr_irq_update_active_intc() from > >> spapr_irq_init() which is called at machine realize time, before the > >> vfio realize. This call will be pretty much obsoleted by the later > >> call at reset time, but it serves to suppress the spurious warning > >> from VFIO. > >> > >> Cc: Alex Williamson <alex.william...@redhat.com> > >> Cc: Alexey Kardashevskiy <a...@ozlabs.ru> > >> > >> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > >> --- > >> hw/ppc/spapr_irq.c | 11 ++++++++++- > >> 1 file changed, 10 insertions(+), 1 deletion(-) > >> > >> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > >> index 45544b8976..bb91c61fa0 100644 > >> --- a/hw/ppc/spapr_irq.c > >> +++ b/hw/ppc/spapr_irq.c > >> @@ -345,6 +345,14 @@ void spapr_irq_init(SpaprMachineState *spapr, Error > >> **errp) > >> > >> spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr, > >> smc->nr_xirqs + SPAPR_XIRQ_BASE); > >> + > >> + /* > >> + * Mostly we don't actually need this until reset, except that not > >> + * having this set up can cause VFIO devices to issue a > >> + * false-positive warning during realize(), because they don't yet > >> + * have an in-kernel irq chip. > >> + */ > >> + spapr_irq_update_active_intc(spapr); > > > > This will call the de/activate hooks of the irq chip very early > > before reset and CAS, and won't call them at reset if the intc > > pointers are the same (xive only for instance). It breaks very > > obviously the emulated XIVE device which won't reset the OS CAM > > line with the CPU id values and CPU notification will be broken. > > > > We have to find something else. > > Here is a possible fix for the (re)setting of the OS CAM line. > > Removing spapr_xive_set_tctx_os_cam() is a good thing but this solution > shows some issues in our modeling of hot-plugged CPUS with a reset() > being called at realize().
Ok, I've applied the patch below now. Does that mean that my 5/5 patch should be good now? -- 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
signature.asc
Description: PGP signature