Re: [PATCH] KVM: arm64: vgic-v4: Use helper function irq_get_irq_data()

[Marc Zyngier <maz@kernel.org>]

On Fri, 09 Aug 2024 09:39:06 +0100,
Zhang Zekun <zhangzekun11@huawei.com> wrote:
> 
> The returned value of irq_to_desc() is only used to get the irq_data
> by calling irq_desc_get_irq_data(), and there is a helper function
> irq_get_irq_data() which has already provided the functionality. So,
> use irq_get_irq_data() to make the code more simple.

Unfortunately, simpler doesn't equate to better.

> 
> Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
> ---
>  arch/arm64/kvm/vgic/vgic-v4.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
> index 74a67ad87f29..2707ab0ab0a5 100644
> --- a/arch/arm64/kvm/vgic/vgic-v4.c
> +++ b/arch/arm64/kvm/vgic/vgic-v4.c
> @@ -124,7 +124,6 @@ static void vgic_v4_enable_vsgis(struct kvm_vcpu *vcpu)
>  	 */
>  	for (i = 0; i < VGIC_NR_SGIS; i++) {
>  		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, i);
> -		struct irq_desc *desc;
>  		unsigned long flags;
>  		int ret;
>  
> @@ -138,8 +137,7 @@ static void vgic_v4_enable_vsgis(struct kvm_vcpu *vcpu)
>  
>  		/* Transfer the full irq state to the vPE */
>  		vgic_v4_sync_sgi_config(vpe, irq);
> -		desc = irq_to_desc(irq->host_irq);
> -		ret = irq_domain_activate_irq(irq_desc_get_irq_data(desc),
> +		ret = irq_domain_activate_irq(irq_get_irq_data(irq->host_irq),
>  					      false);

And you are still performing two lookups (one in the domain, one in
the mapple tree), when everything could be solve with one:

diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
index 74a67ad87f29..9f0f15b376f2 100644
--- a/arch/arm64/kvm/vgic/vgic-v4.c
+++ b/arch/arm64/kvm/vgic/vgic-v4.c
@@ -134,11 +134,10 @@ static void vgic_v4_enable_vsgis(struct kvm_vcpu *vcpu)
 			goto unlock;
 
 		irq->hw = true;
-		irq->host_irq = irq_find_mapping(vpe->sgi_domain, i);
+		desc = __irq_resolve_mapping(vpe->sgi_domain, i, &irq->host_irq);
 
 		/* Transfer the full irq state to the vPE */
 		vgic_v4_sync_sgi_config(vpe, irq);
-		desc = irq_to_desc(irq->host_irq);
 		ret = irq_domain_activate_irq(irq_desc_get_irq_data(desc),
 					      false);
 		if (!WARN_ON(ret)) {

So no, I'm not going to change this just to drop a local variable. If
we are going to change something, it is to improve the *result*.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.