On 10/28/2016 03:00 AM, David Gibson wrote:
> On Thu, Oct 27, 2016 at 07:43:10PM +0200, Cédric Le Goater wrote:
>> On 10/27/2016 05:09 AM, David Gibson wrote:
>>> On Wed, Oct 26, 2016 at 09:13:18AM +0200, Cédric Le Goater wrote:
>>>> On 10/25/2016 07:08 AM, David Gibson wrote:
>>>>> On Sat, Oct 22, 2016 at 11:46:44AM +0200, Cédric Le Goater wrote:
>>>>>> This provides access to the MMIO based Interrupt Presentation
>>>>>> Controllers (ICP) as found on a POWER8 system.
>>>>>>
>>>>>> A new XICSNative class is introduced to hold the MMIO region of the
>>>>>> ICPs. Each thread of the system has a subregion, indexed by its PIR
>>>>>> number, holding a XIVE (External Interrupt Vector Entry). This
>>>>>> provides a mean to make the link with the ICPState of the CPU.
>>>>>>
>>>>>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>>>>>> ---
>>>>>>
>>>>>> Changes since v4:
>>>>>>
>>>>>> - replaced the pir_able by memory subregions using an ICP.
>>>>>> - removed the find_icp() and cpu_setup() handlers which became
>>>>>> useless with the memory regions.
>>>>>> - removed the superfluous inits done in xics_native_initfn. This is
>>>>>> covered in the parent class init.
>>>>>> - took ownership of the patch.
>>>>>>
>>>>>> default-configs/ppc64-softmmu.mak | 3 +-
>>>>>> hw/intc/Makefile.objs | 1 +
>>>>>> hw/intc/xics_native.c | 304
>>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>> include/hw/ppc/pnv.h | 19 +++
>>>>>> include/hw/ppc/xics.h | 24 +++
>>>>>> 5 files changed, 350 insertions(+), 1 deletion(-)
>>>>>> create mode 100644 hw/intc/xics_native.c
>>>>>>
>>>>>> diff --git a/default-configs/ppc64-softmmu.mak
>>>>>> b/default-configs/ppc64-softmmu.mak
>>>>>> index 67a9bcaa67fa..a22c93a48686 100644
>>>>>> --- a/default-configs/ppc64-softmmu.mak
>>>>>> +++ b/default-configs/ppc64-softmmu.mak
>>>>>> @@ -48,8 +48,9 @@ CONFIG_PLATFORM_BUS=y
>>>>>> CONFIG_ETSEC=y
>>>>>> CONFIG_LIBDECNUMBER=y
>>>>>> # For pSeries
>>>>>> -CONFIG_XICS=$(CONFIG_PSERIES)
>>>>>> +CONFIG_XICS=$(or $(CONFIG_PSERIES),$(CONFIG_POWERNV))
>>>>>> CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
>>>>>> +CONFIG_XICS_NATIVE=$(CONFIG_POWERNV)
>>>>>> CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
>>>>>> # For PReP
>>>>>> CONFIG_MC146818RTC=y
>>>>>> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
>>>>>> index 2f44a2da26e9..e44a29d75b32 100644
>>>>>> --- a/hw/intc/Makefile.objs
>>>>>> +++ b/hw/intc/Makefile.objs
>>>>>> @@ -34,6 +34,7 @@ obj-$(CONFIG_RASPI) += bcm2835_ic.o bcm2836_control.o
>>>>>> obj-$(CONFIG_SH4) += sh_intc.o
>>>>>> obj-$(CONFIG_XICS) += xics.o
>>>>>> obj-$(CONFIG_XICS_SPAPR) += xics_spapr.o
>>>>>> +obj-$(CONFIG_XICS_NATIVE) += xics_native.o
>>>>>> obj-$(CONFIG_XICS_KVM) += xics_kvm.o
>>>>>> obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o
>>>>>> obj-$(CONFIG_S390_FLIC) += s390_flic.o
>>>>>> diff --git a/hw/intc/xics_native.c b/hw/intc/xics_native.c
>>>>>> new file mode 100644
>>>>>> index 000000000000..bbdd786aeb50
>>>>>> --- /dev/null
>>>>>> +++ b/hw/intc/xics_native.c
>>>>>> @@ -0,0 +1,304 @@
>>>>>> +/*
>>>>>> + * QEMU PowerPC PowerNV machine model
>>>>>> + *
>>>>>> + * Native version of ICS/ICP
>>>>>> + *
>>>>>> + * Copyright (c) 2016, IBM Corporation.
>>>>>> + *
>>>>>> + * This library is free software; you can redistribute it and/or
>>>>>> + * modify it under the terms of the GNU Lesser General Public
>>>>>> + * License as published by the Free Software Foundation; either
>>>>>> + * version 2 of the License, or (at your option) any later version.
>>>>>> + *
>>>>>> + * This library is distributed in the hope that it will be useful,
>>>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>>>>> + * Lesser General Public License for more details.
>>>>>> + *
>>>>>> + * You should have received a copy of the GNU Lesser General Public
>>>>>> + * License along with this library; if not, see
>>>>>> <http://www.gnu.org/licenses/>.
>>>>>> + */
>>>>>> +
>>>>>> +#include "qemu/osdep.h"
>>>>>> +#include "qapi/error.h"
>>>>>> +#include "qemu-common.h"
>>>>>> +#include "cpu.h"
>>>>>> +#include "hw/hw.h"
>>>>>> +#include "qemu/log.h"
>>>>>> +#include "qapi/error.h"
>>>>>> +
>>>>>> +#include "hw/ppc/fdt.h"
>>>>>> +#include "hw/ppc/xics.h"
>>>>>> +#include "hw/ppc/pnv.h"
>>>>>> +
>>>>>> +#include <libfdt.h>
>>>>>> +
>>>>>> +static void xics_native_reset(void *opaque)
>>>>>> +{
>>>>>> + device_reset(DEVICE(opaque));
>>>>>> +}
>>>>>> +
>>>>>> +static void xics_native_initfn(Object *obj)
>>>>>> +{
>>>>>> + qemu_register_reset(xics_native_reset, obj);
>>>>>> +}
>>>>>
>>>>> I think we need to investigate why the xics native is not showing up
>>>>> on the SysBus. As a "raw" MMIO device, it really should.
>>>>
>>>> Well, it has sysbus mmio region, but it is not created with
>>>> qdev_create(...)
>>>> so it is not under sysbus and the reset does not get called. That is my
>>>> understanding of the problem.
>>>>
>>>> May be we shouldn't be using a sysbus mmio region ?
>>>
>>> Yeah, maybe not. We don't really fit the sysbus model well.
>>>
>>> I do kind of wonder if the xics object should be an mmio device at
>>> all, or if just the individual ICPs should be. But that might make
>>> for more trouble.
>>
>> A cleanup brings another :) It is true that xics native does not
>> have any controls. It is just a container to hold the array of ICPs
>> and so each of these could well be a child object of PnvCore. Well,
>> of a PnvThread but we don't have that today.
>
> Ok.
>
>> I am going to move the container region to PnvChip to start with and
>> if I can the ICP regions to PnvCore. When we realize the PnvCore, we
>> have the xics, but I need to find a way to grab the ICPState from it.
>> I might need to use the cpu_index for that or could I change
>> xics_cpu_setup() to return an 'ICPState *' ?
>
>> I would prefer the ICP to be a PnvCore/Thread child object but that
>> is too early I think.
>
> Right, that makes sense.
>
>> So if that comes well together, we will get rid of XICSNative and we
>> will use a XICSState for its ICP array.
>
> So, I just had a thought about this that I think might work, though it
> would require cleaning up the existing stuff in spapr before extending
> for powernv:
>
> Abolish the (overall) XICS as a fully realized object, and instead
> make it a QOM interface, which is implemented by the machine. ICPs
> and ICSes remain real devices, which (as now) would have a link back
> to the XICS interface object.
OK. I will take a look at that for 2.9.
Here is the current status for pnv. XICSNative is gone. The ICP container
region is under the chip and the ICP-per-thread subregions under PnvCore.
The machine uses a XICS_COMMON object to hold the array of ICPs and the
list of ICS. The result is much better but I had to modify a few things
in xics to be able to instantiate a XICS_COMMON object :
* add a new ops to XICSStateClass :
void (*realize)(DeviceState *dev, Error **errp)
to clean up xics_kvm_realize() and xics_spapr_realize() which are
doing the same thing. That's a good cleanup going in the direction
you are talking about. So maybe I could send for 2.8
* add a xics_common_set_nr_servers() routine to create the ICPs when
a XICS_COMMON object is instantiated. That is more a work around.
That business around the "nr_irqs" and "nr_servers" properties is
confusing, we should clean it up.
So if you think this is worth 2.8, I can send a couple of patches. Else
I can start by some xics cleanups and aim 2.9
> The XICS interface would provide server-number-to-ICP-pointer and
> irq-number-to-ICS-pointer callbacks. That puts the "fabric" logic -
> how the IPCs and ICSes find each other back in the machine, which I
> think is where it belongs. Obviously we could still provide
> xics_system_init() or similar helpers to make it easier for the
> machines to implement the xics interface.
Rough comments and questions :
* ICS :
They don't belong to XICS but we do need to maintain a list as we
have a few loops on the ICSs. Should we maintain the list under the
machine ?
* ICP :
The server-number-to-ICP-pointer handler should cover most of the
needs. If we can use the Core object to hold the ICP, that would
be even better. It whould greatly simplify cpu_setup() which is
here just for KVM_CAP_IRQ_XICS and the list of cpus would provide
the array.
ICP reset would be handled at the Core level
xics_common_pic_print_info() would be a machine handler.
I am not sure what to do with ics_simple_post_load().
C.
> The overall XICS has no migrated state, which means that change
> shouldn't fundamentally break things. There might be issues with the
> qom paths of ICS or ICP changing, we'd have to check that.
>
>
