[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PULL 19/48] spapr: allocate the ICPState ob
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PULL 19/48] spapr: allocate the ICPState object from under sPAPRCPUCore |
Date: |
Tue, 16 May 2017 14:55:38 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 16/05/2017 14:50, Cédric Le Goater wrote:
> On 05/16/2017 02:03 PM, Laurent Vivier wrote:
>> On 26/04/2017 09:00, David Gibson wrote:
>>> From: Cédric Le Goater <address@hidden>
>>>
>>> Today, all the ICPs are created before the CPUs, stored in an array
>>> under the sPAPR machine and linked to the CPU when the core threads
>>> are realized. This modeling brings some complexity when a lookup in
>>> the array is required and it can be simplified by allocating the ICPs
>>> when the CPUs are.
>>>
>>> This is the purpose of this proposal which introduces a new 'icp_type'
>>> field under the machine and creates the ICP objects of the right type
>>> (KVM or not) before the PowerPCCPU object are.
>>>
>>> This change allows more cleanups : the removal of the icps array under
>>> the sPAPR machine and the removal of the xics_get_cpu_index_by_dt_id()
>>> helper.
>>>
>>> Signed-off-by: Cédric Le Goater <address@hidden>
>>> Reviewed-by: David Gibson <address@hidden>
>>> Signed-off-by: David Gibson <address@hidden>
>>> ---
>>> hw/intc/xics.c | 11 -----------
>>> hw/ppc/spapr.c | 47
>>> ++++++++++++++---------------------------------
>>> hw/ppc/spapr_cpu_core.c | 18 ++++++++++++++----
>>> include/hw/ppc/spapr.h | 2 +-
>>> include/hw/ppc/xics.h | 2 --
>>> 5 files changed, 29 insertions(+), 51 deletions(-)
>>>
>>
>> This commit breaks CPU re-hotplugging with KVM
>>
>> the sequence "device_add, device_del, device_add" brings to the
>> following error message:
>>
>> Unable to connect CPUx to kernel XICS: Device or resource busy
>>
>> It comes from icp_kvm_cpu_setup():
>>
>> ...
>> ret = kvm_vcpu_enable_cap(cs, KVM_CAP_IRQ_XICS, 0, kernel_xics_fd,
>> kvm_arch_vcpu_id(cs));
>> if (ret < 0) {
>> error_report("Unable to connect CPU%ld to kernel XICS: %s",
>> kvm_arch_vcpu_id(cs), strerror(errno));
>> exit(1);
>> }
>> ..
>>
>> It should be protected by cap_irq_xics_enabled:
>>
>> ...
>> /*
>> * If we are reusing a parked vCPU fd corresponding to the CPU
>> * which was hot-removed earlier we don't have to renable
>> * KVM_CAP_IRQ_XICS capability again.
>> */
>> if (icp->cap_irq_xics_enabled) {
>> return;
>> }
>>
>> ...
>> ret = kvm_vcpu_enable_cap(...);
>> ...
>> icp->cap_irq_xics_enabled = true;
>> ...
>>
>> But since this commit, "icp" is a new object on each call:
>>
>> spapr_cpu_core_realize_child()
>> ...
>> obj = object_new(spapr->icp_type);
>> ...
>> xics_cpu_setup(XICS_FABRIC(spapr), cpu, ICP(obj));
>> ...
>> icpc->cpu_setup(icp, cpu); -> icp_kvm_cpu_setup()
>> ...
>> ...
>>
>> and "cap_irq_xics_enabled" is reinitialized.
>>
>> Any idea how to fix that?
>
> it seems that a cleanup is not done in the kernel. We are missing
> a way to call kvmppc_xics_free_icp() from QEMU. Today the only
> way is to destroy the vcpu.
The commit introducing this hack, for reference:
commit a45863bda90daa8ec39e5a312b9734fd4665b016
Author: Bharata B Rao <address@hidden>
Date: Thu Jul 2 16:23:20 2015 +1000
xics_kvm: Don't enable KVM_CAP_IRQ_XICS if already enabled
When supporting CPU hot removal by parking the vCPU fd and reusing
it during hotplug again, there can be cases where we try to reenable
KVM_CAP_IRQ_XICS CAP for the vCPU for which it was already enabled.
Introduce a boolean member in ICPState to track this and don't
reenable the CAP if it was already enabled earlier.
Re-enabling this CAP should ideally work, but currently it results in
kernel trying to create and associate ICP with this vCPU and that
fails since there is already an ICP associated with it. Hence this
patch is needed to work around this problem in the kernel.
This change allows CPU hot removal to work for sPAPR.
Signed-off-by: Bharata B Rao <address@hidden>
Reviewed-by: David Gibson <address@hidden>
Signed-off-by: David Gibson <address@hidden>
Signed-off-by: Alexander Graf <address@hidden>
> Else we need to reintroduce the array of icps (again) to keep some
> xics state ... but that just sucks :/ Let me think about it.
>
Thanks,
Laurent
> C.
>
- Re: [Qemu-ppc] [Qemu-devel] [PULL 19/48] spapr: allocate the ICPState object from under sPAPRCPUCore, Laurent Vivier, 2017/05/16
- Re: [Qemu-ppc] [Qemu-devel] [PULL 19/48] spapr: allocate the ICPState object from under sPAPRCPUCore, Cédric Le Goater, 2017/05/16
- Re: [Qemu-ppc] [Qemu-devel] [PULL 19/48] spapr: allocate the ICPState object from under sPAPRCPUCore,
Laurent Vivier <=
- Re: [Qemu-ppc] [Qemu-devel] [PULL 19/48] spapr: allocate the ICPState object from under sPAPRCPUCore, Cédric Le Goater, 2017/05/16
- Re: [Qemu-ppc] [Qemu-devel] [PULL 19/48] spapr: allocate the ICPState object from under sPAPRCPUCore, Greg Kurz, 2017/05/16
- Re: [Qemu-ppc] [Qemu-devel] [PULL 19/48] spapr: allocate the ICPState object from under sPAPRCPUCore, Cédric Le Goater, 2017/05/17
- Re: [Qemu-ppc] [Qemu-devel] [PULL 19/48] spapr: allocate the ICPState object from under sPAPRCPUCore, David Gibson, 2017/05/17
- Re: [Qemu-ppc] [Qemu-devel] [PULL 19/48] spapr: allocate the ICPState object from under sPAPRCPUCore, Greg Kurz, 2017/05/17
- Re: [Qemu-ppc] [Qemu-devel] [PULL 19/48] spapr: allocate the ICPState object from under sPAPRCPUCore, Greg Kurz, 2017/05/17