linux-kernel - Re: [PATCH v5 3/3] irqchip/loongson-eiointc: Add extioi virt extension support

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] [day] [month] [year] [list]

Message-ID: <09d4ad7a-0548-28e6-f60e-c1b68c8c20b5@loongson.cn>
Date: Mon, 12 Aug 2024 09:10:47 +0800
From: maobibo <maobibo@...ngson.cn>
To: Thomas Gleixner <tglx@...utronix.de>,
 Tianrui Zhao <zhaotianrui@...ngson.cn>, Huacai Chen <chenhuacai@...nel.org>
Cc: WANG Xuerui <kernel@...0n.name>, kvm@...r.kernel.org,
 loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org,
 virtualization@...ts.linux.dev, x86@...nel.org,
 Song Gao <gaosong@...ngson.cn>
Subject: Re: [PATCH v5 3/3] irqchip/loongson-eiointc: Add extioi virt
 extension support



On 2024/8/11 上午4:46, Thomas Gleixner wrote:
> On Mon, Aug 05 2024 at 15:35, Bibo Mao wrote:
> 
>> Interrupts can be routed to maximal four virtual CPUs with one external
>> hardware interrupt. Add the extioi virt extension support so that
>> Interrupts can be routed to 256 vcpus on hypervisor mode.
> 
> interrupts .... 256 vCPUs in hypervisor mode.
will modify in next version.

> 
>>   static int cpu_to_eio_node(int cpu)
>>   {
>> -	return cpu_logical_map(cpu) / CORES_PER_EIO_NODE;
>> +	int cores;
>> +
>> +	if (kvm_para_available() && kvm_para_has_feature(KVM_FEATURE_VIRT_EXTIOI))
> 
> Why isn't that kvm_para_available() check inside of
> kvm_para_has_feature() instead of inflicting it on every usage site?
> That's just error prone.
I had the same idea but not sure about it, it is to follow the 
implemantion of x86.

Yeap, it is better to put kvm_para_available() in function 
kvm_para_has_feature(), one api for caller is simple and enough.

Will do in next version.
> 
>> +		cores = CORES_PER_VEIO_NODE;
>> +	else
>> +		cores = CORES_PER_EIO_NODE;
>> +	return cpu_logical_map(cpu) / cores;
>>   }
> 
>> @@ -105,18 +144,24 @@ static int eiointc_set_irq_affinity(struct irq_data *d, const struct cpumask *af
>> @@ -140,17 +185,23 @@ static int eiointc_index(int node)
>>   
>>   static int eiointc_router_init(unsigned int cpu)
>>   {
>> -	int i, bit;
>> -	uint32_t data;
>> -	uint32_t node = cpu_to_eio_node(cpu);
>> -	int index = eiointc_index(node);
>> +	uint32_t data, node;
>> +	int i, bit, cores, index;
> 
> Is it so hard to follow:
> 
>    https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations
> 
> ?
Sorry to have the same coding style issue for the second time. I am not 
familiar with it.

Will do in next version.

Regards
Bibo Mao
> 
> Thanks,
> 
>          tglx
>