On 2/13/19 2:32 AM, David Gibson wrote:
> On Tue, Feb 12, 2019 at 08:18:19AM +0100, Cédric Le Goater wrote:
>> On 2/12/19 2:11 AM, David Gibson wrote:
>>> On Mon, Jan 07, 2019 at 07:39:46PM +0100, Cédric Le Goater wrote:
>>>> The interrupt mode is chosen by the CAS negotiation process and
>>>> activated after a reset to take into account the required changes in
>>>> the machine. This brings new constraints on how the associated KVM IRQ
>>>> device is initialized.
>>>>
>>>> Currently, each model takes care of the initialization of the KVM
>>>> device in their realize method but this is not possible anymore as the
>>>> initialization needs to be done globaly when the interrupt mode is
>>>> known, i.e. when machine is reseted. It also means that we need a way
>>>> to delete a KVM device when another mode is chosen.
>>>>
>>>> Also, to support migration, the QEMU objects holding the state to
>>>> transfer should always be available but not necessarily activated.
>>>>
>>>> The overall approach of this proposal is to initialize both interrupt
>>>> mode at the QEMU level and keep the IRQ number space in sync to allow
>>>> switching from one mode to another. For the KVM side of things, the
>>>> whole initialization of the KVM device, sources and presenters, is
>>>> grouped in a single routine. The XICS and XIVE sPAPR IRQ reset
>>>> handlers are modified accordingly to handle the init and the delete
>>>> sequences of the KVM device.
>>>>
>>>> As KVM is now initialized at reset, we loose the possiblity to
>>>> fallback to the QEMU emulated mode in case of failure and failures
>>>> become fatal to the machine.
>>>>
>>>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>>>> ---
>>>> hw/intc/spapr_xive.c | 8 +---
>>>> hw/intc/spapr_xive_kvm.c | 27 ++++++++++++++
>>>> hw/intc/xics_kvm.c | 25 +++++++++++++
>>>> hw/intc/xive.c | 4 --
>>>> hw/ppc/spapr_irq.c | 79 ++++++++++++++++++++++++++++------------
>>>> 5 files changed, 109 insertions(+), 34 deletions(-)
>>>>
>>>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>>>> index 21f3c1ef0901..0661aca35900 100644
>>>> --- a/hw/intc/spapr_xive.c
>>>> +++ b/hw/intc/spapr_xive.c
>>>> @@ -330,13 +330,7 @@ static void spapr_xive_realize(DeviceState *dev,
>>>> Error **errp)
>>>> xive->eat = g_new0(XiveEAS, xive->nr_irqs);
>>>> xive->endt = g_new0(XiveEND, xive->nr_ends);
>>>>
>>>> - if (kvmppc_xive_enabled()) {
>>>> - kvmppc_xive_connect(xive, &local_err);
>>>> - if (local_err) {
>>>> - error_propagate(errp, local_err);
>>>> - return;
>>>> - }
>>>> - } else {
>>>> + if (!kvmppc_xive_enabled()) {
>>>> /* TIMA initialization */
>>>> memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &xive_tm_ops,
>>>> xive,
>>>> "xive.tima", 4ull << TM_SHIFT);
>>>> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
>>>> index d35814c1992e..3ebc947f2be7 100644
>>>> --- a/hw/intc/spapr_xive_kvm.c
>>>> +++ b/hw/intc/spapr_xive_kvm.c
>>>> @@ -737,6 +737,15 @@ void kvmppc_xive_connect(sPAPRXive *xive, Error
>>>> **errp)
>>>> Error *local_err = NULL;
>>>> size_t esb_len;
>>>> size_t tima_len;
>>>> + CPUState *cs;
>>>> +
>>>> + /*
>>>> + * The KVM XIVE device already in use. This is the case when
>>>> + * rebooting XIVE -> XIVE
>>>
>>> Can this case actually occur? Further down you appear to
>>> unconditionally destroy both KVM devices at reset time.
>>
>> I guess you are right. I will check.
So we have to keep it for ic-mode=xive.
The number of test combinations is exploding. We now have :
3 hypervisors KVM, KVM nested, QEMU TCG
3 Interrupt modes xics, xive, dual (xics+xive)
3 kernel irqchip modes off, allow, required.
C.
