linux-kernel - Re: [RFC PATCH 3/3] arm/arm64: Use gic_ipi_send

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 29 Mar 2021 19:03:03 +0800
From:   Jingyi Wang <wangjingyi11@...wei.com>
To:     Marc Zyngier <maz@...nel.org>
CC:     <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <tglx@...utronix.de>,
        <wanghaibin.wang@...wei.com>, <yuzenghui@...wei.com>,
        <zhukeqian1@...wei.com>
Subject: Re: [RFC PATCH 3/3] arm/arm64: Use gic_ipi_send_single() to inject
 single IPI



On 3/29/2021 6:07 PM, Marc Zyngier wrote:
> On Mon, 29 Mar 2021 09:52:10 +0100,
> Jingyi Wang <wangjingyi11@...wei.com> wrote:
>>
>> Currently, arm use gic_ipi_send_mask() to inject single IPI, which
>> make the procedure a little complex. We use gic_ipi_send_single()
>> instead as some other archs.
>>
>> Signed-off-by: Jingyi Wang <wangjingyi11@...wei.com>
>> ---
>>   arch/arm/kernel/smp.c   | 16 +++++++++++++---
>>   arch/arm64/kernel/smp.c | 16 +++++++++++++---
>>   2 files changed, 26 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
>> index 74679240a9d8..369ce529cdd8 100644
>> --- a/arch/arm/kernel/smp.c
>> +++ b/arch/arm/kernel/smp.c
>> @@ -534,6 +534,8 @@ static const char *ipi_types[NR_IPI] __tracepoint_string = {
>>   };
>>   
>>   static void smp_cross_call(const struct cpumask *target, unsigned int ipinr);
>> +static void smp_cross_call_single(const struct cpumask *target, int cpu,
>> +				  unsigned int ipinr);
> 
> Why does this function need to take both a cpumask *and* a cpu, given
> that they represent the same thing?
> I was intended to use the extra param to reuse trace_ipi_raise_rcuidle.

>>
>>   void show_ipi_list(struct seq_file *p, int prec)
>>   {
>> @@ -564,14 +566,15 @@ void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
>>   
>>   void arch_send_call_function_single_ipi(int cpu)
>>   {
>> -	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC);
>> +	smp_cross_call_single(cpumask_of(cpu), cpu, IPI_CALL_FUNC);
>>   }
>>   
>>   #ifdef CONFIG_IRQ_WORK
>>   void arch_irq_work_raise(void)
>>   {
>> +	int cpu = smp_processor_id();
>>   	if (arch_irq_work_has_interrupt())
>> -		smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
>> +		smp_cross_call(cpumask_of(cpu), cpu, IPI_IRQ_WORK);
> 
> Why isn't that a call to smp_cross_call_single()?
> 
I ignored that, thanks.

>>   }
>>   #endif
>>   
>> @@ -707,6 +710,13 @@ static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
>>   	__ipi_send_mask(ipi_desc[ipinr], target);
>>   }
>>   
>> +static void smp_cross_call_single(const struct cpumask *target, int cpu,
>> +				  unsigned int ipinr)
>> +{
>> +	trace_ipi_raise_rcuidle(target, ipi_types[ipinr]);
> 
> Why don't you compute the cpumask here^^?
> 
>> +	__ipi_send_single(ipi_desc[ipinr], cpu);
>> +}
>> +
>>   static void ipi_setup(int cpu)
>>   {
>>   	int i;
>> @@ -744,7 +754,7 @@ void __init set_smp_ipi_range(int ipi_base, int n)
>>   
>>   void smp_send_reschedule(int cpu)
>>   {
>> -	smp_cross_call(cpumask_of(cpu), IPI_RESCHEDULE);
>> +	smp_cross_call_single(cpumask_of(cpu), cpu, IPI_RESCHEDULE);
>>   }
>>   
>>   void smp_send_stop(void)
>> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
>> index 357590beaabb..d290b6dc5a6e 100644
>> --- a/arch/arm64/kernel/smp.c
>> +++ b/arch/arm64/kernel/smp.c
> 
> Similar comments for the arm64 side.
> 
> Overall, this needs to be backed by data that indicates that there is
> an actual benefit for this extra complexity.
> 
> Thanks,
> 
> 	M.
> 

Firstly, I implemented exitless-IPIs to reduce VM trap caused by sending
IPI as what x86 does here:
https://patchwork.kernel.org/project/kvm/cover/1532327996-17619-1-git-send-email-wanpengli@tencent.com/
Then I realized that sending ipi mask usually cause sending IPIs all
but self as IRM bit defines.
So do you think we can use IRM if we can avoid extra cost like computing
mask, or using broadcast IPIs is just meaningless for now?

Thanks,
Jingyi