Re: [Qemu-devel] [Qemu-ppc] [for-2.11 PATCH 00/26] spapr: add support for PHB hotplug

[Greg Kurz]

On Fri, 28 Jul 2017 07:35:34 +0200
Cédric Le Goater <address@hidden> wrote:

> On 07/28/2017 05:40 AM, David Gibson wrote:
> > On Fri, Jul 28, 2017 at 01:27:05PM +1000, Alexey Kardashevskiy wrote:  
> >> On 28/07/17 02:39, Greg Kurz wrote:  
> >>> On Wed, 26 Jul 2017 17:31:17 -0300
> >>> Daniel Henrique Barboza <address@hidden> wrote:
> >>>  
> >>>> I've tested the patch set using Greg's Github branch. It worked fine in 
> >>>> my tests
> >>>> using a Fedora 26 and an Ubuntu 17.04 guests. I have two observations
> >>>> though:
> >>>>
> >>>> 1 - This is not related to this patch set per se because it is 
> >>>> reproducible on master, but
> >>>> I think it is interfering with this new feature.  There is a 
> >>>> warning/error message in
> >>>> the kernel right after SLOF that goes:
> >>>>
> >>>> (...)  
> >>>>   -> smp_release_cpus()    
> >>>> spinning_secondaries = 0
> >>>>   <- smp_release_cpus()
> >>>> Linux ppc64le
> >>>> #1 SMP Mon Jul 1[    0.030450] pci 0000:00:02.0: of_irq_parse_pci: 
> >>>> failed with rc=-22
> >>>> [    0.030552] pci 0000:00:0f.0: of_irq_parse_pci: failed with rc=-22
> >>>> [  OK  ] Started Security Auditing Service.
> >>>> (...)
> >>>>  
> >>>
> >>> This is a regression in QEMU master introduced by this commit:
> >>>
> >>> commit b87680427e8a3ff682f66514e99a8344e7437247
> >>> Author: Cédric Le Goater <address@hidden>
> >>> Date:   Wed Jul 5 19:13:15 2017 +0200
> >>>
> >>>     spapr: populate device tree depending on XIVE_EXPLOIT option
> >>>     
> >>>     When XIVE is supported, the device tree should be populated
> >>>     accordingly and the XIVE memory regions mapped to activate MMIOs.
> >>>     
> >>>     Depending on the design we choose, we could also allocate different
> >>>     ICS and ICP objects, or switch between objects. This needs to be
> >>>     discussed.
> >>>     
> >>>     Signed-off-by: Cédric Le Goater <address@hidden>
> >>>     Signed-off-by: David Gibson <address@hidden>
> >>>
> >>> It is very similar to the issue that motivated the new 
> >>> KVMPPC_H_UPDATE_PHANDLE
> >>> hcall (see patch 24 and 26 in this series):
> >>>
> >>> - QEMU creates an "interrupt-controller" node with a phandle property
> >>>   with the value 0x1111
> >>> - QEMU passes the FDT to SLOF
> >>> - SLOF converts all references to the phandle to an SLOF internal value
> >>>  
> >>> => from now on (ie, until the next machine reset), the guest only knows  
> >>>    the OF phandle.
> >>>
> >>> - during CAS, if we go XICS, we send back an updated FDT with the
> >>>   phandle of the "interrupt-controller" node reverted to 0x1111
> >>>  
> >>> => the guest complains because all cold-plugged devices nodes refer  
> >>>    to the OF phandle, not 0x1111
> >>>
> >>> A solution is to use the value set by KVMPPC_H_UPDATE_PHANDLE during CAS
> >>> instead of 0x1111. I could verify it makes the guest warning disappear.
> >>>
> >>> I'll send a dedicated patchset to fix this in 2.10.  
> >>
> >>
> >> The SLOF I pushed for 2.10 does not have it though. And the rest of XIVE is
> >> not targeted for 2.10 anyway. So imho the solution is reverting "spapr:
> >> populate device tree depending on XIVE_EXPLOIT option" for 2.10.  
> > 
> > I agree, I've applied the revert to ppc-for-2.10 now.  
> 
> Sure. 
> 
> Greg, have you found a solution to get around this problem or do we need
> to populate the device tree with the interrupt controller node for SLOF ? 
> 

With commit b87680427e8a, spapr_dt_xics() is called twice actually: first
time during machine reset and a second time during CAS.

The first call means we start in XICS mode by default. But I don't understand
why it is also called during CAS: if the guest didn't advertise XIVE, then
we don't have anything to do, do we ?

Cheers,

--
Greg

> Thanks,
> 
> C.  
>

pgp1ofWlWES0c.pgp
Description: OpenPGP digital signature