On Mon, Oct 27, 2014 at 12:04 PM, Preeti U Murthy
<preeti@xxxxxxxxxxxxxxxxxx> wrote:
> Hi Liu,
>
> On 10/17/2014 12:59 AM, kernelfans@xxxxxxxxx wrote:
>> When kvm is enabled on a core, we migrate all external irq to primary
>> thread. Since currently, the kvmirq logic is handled by the primary
>> hwthread.
>>
>> Todo: this patch lacks re-enable of irqbalance when kvm is disable on
>> the core
>
> Why is a sysfs file introduced to trigger irq migration? Why is it not
> done during kvm module insert ? And similarly spread interrupts when the
> module is removed? Isn't this a saner way ?
Consider the scene: coreA and coreB, we want to enable KVM on coreA,
while keeping coreB unchanged.
In fact, I try to acheive something un-symmetric on the platform. Do
you think it is an justification?
>>
>> Signed-off-by: Liu Ping Fan <pingfank@xxxxxxxxxxxxxxxxxx>
>> ---
>> arch/powerpc/kernel/sysfs.c | 39 ++++++++++++++++++++++++++++++++++
>> arch/powerpc/sysdev/xics/xics-common.c | 12 +++++++++++
>> 2 files changed, 51 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
>> index 67fd2fd..a2595dd 100644
>> --- a/arch/powerpc/kernel/sysfs.c
>> +++ b/arch/powerpc/kernel/sysfs.c
>> @@ -552,6 +552,45 @@ static void sysfs_create_dscr_default(void)
>> if (cpu_has_feature(CPU_FTR_DSCR))
>> err = device_create_file(cpu_subsys.dev_root, &dev_attr_dscr_default);
>> }
>> +
>> +#ifdef CONFIG_KVMPPC_ENABLE_SECONDARY
>> +#define NR_CORES (CONFIG_NR_CPUS/threads_per_core)
>> +static DECLARE_BITMAP(kvm_on_core, NR_CORES) __read_mostly
>> +
>> +static ssize_t show_kvm_enable(struct device *dev,
>> + struct device_attribute *attr, char *buf)
>> +{
>> +}
>> +
>> +static ssize_t __used store_kvm_enable(struct device *dev,
>> + struct device_attribute *attr, const char *buf,
>> + size_t count)
>> +{
>> + struct cpumask stop_cpus;
>> + unsigned long core, thr;
>> +
>> + sscanf(buf, "%lx", &core);
>> + if (core > NR_CORES)
>> + return -1;
>> + if (!test_bit(core, &kvm_on_core))
>> + for (thr = 1; thr< threads_per_core; thr++)
>> + if (cpu_online(thr * threads_per_core + thr))
>> + cpumask_set_cpu(thr * threads_per_core + thr, &stop_cpus);
>
> What is the above logic trying to do? Did you mean
> cpu_online(threads_per_core * core + thr) ?
>
Yeah. My mistake, should be cpumask_set_cpu(core * threads_per_core +
thr, &stop_cpus)
>> +
>> + stop_machine(xics_migrate_irqs_away_secondary, NULL, &stop_cpus);
>> + set_bit(core, &kvm_on_core);
>> + return count;
>> +}
>> +
>> +static DEVICE_ATTR(kvm_enable, 0600,
>> + show_kvm_enable, store_kvm_enable);
>> +
>> +static void sysfs_create_kvm_enable(void)
>> +{
>> + device_create_file(cpu_subsys.dev_root, &dev_attr_kvm_enable);
>> +}
>> +#endif
>> +
>> #endif /* CONFIG_PPC64 */
>>
>> #ifdef HAS_PPC_PMC_PA6T
>> diff --git a/arch/powerpc/sysdev/xics/xics-common.c b/arch/powerpc/sysdev/xics/xics-common.c
>> index fe0cca4..68b33d8 100644
>> --- a/arch/powerpc/sysdev/xics/xics-common.c
>> +++ b/arch/powerpc/sysdev/xics/xics-common.c
>> @@ -258,6 +258,18 @@ unlock:
>> raw_spin_unlock_irqrestore(&desc->lock, flags);
>> }
>> }
>> +
>> +int xics_migrate_irqs_away_secondary(void *data)
>> +{
>> + int cpu = smp_processor_id();
>> + if(cpu%thread_per_core != 0) {
>> + WARN(condition, format...);
>> + return 0;
>> + }
>> + /* In fact, if we can migrate the primary, it will be more fine */
>> + ();
>
> Isn't the aim of the patch to migrate irqs away from the secondary onto
> the primary? But from above it looks like we are returning when we find
> out that we are secondary threads, isn't it?
>
Yes, will fix in next version.
>> + return 0;
>> +}
>> #endif /* CONFIG_HOTPLUG_CPU */
>
> Note that xics_migrate_irqs_away() is defined under CONFIG_CPU_HOTPLUG.
> But we will need this option on PowerKVM even when hotplug is not
> configured in.
>
Yes, will fix the dependency in next version
Thx,
Fan
> Regards
> Preeti U Murthy
>> #ifdef CONFIG_SMP
>>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
