linux-kernel - Re: [RFC PATCH 05/17] net: ipa: Check interrupts for availability

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]

Message-ID: <445bb258-c07c-349c-e482-78b7b560e0eb@ieee.org>
Date:   Wed, 13 Oct 2021 17:29:39 -0500
From:   Alex Elder <elder@...e.org>
To:     Sireesh Kodali <sireeshkodali1@...il.com>,
        phone-devel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, elder@...nel.org
Cc:     Vladimir Lypak <vladimir.lypak@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: [RFC PATCH 05/17] net: ipa: Check interrupts for availability

On 9/19/21 10:07 PM, Sireesh Kodali wrote:
> From: Vladimir Lypak <vladimir.lypak@...il.com>
> 
> Make ipa_interrupt_add/ipa_interrupt_remove no-operation if requested
> interrupt is not supported by IPA hardware.
> 
> Signed-off-by: Vladimir Lypak <vladimir.lypak@...il.com>
> Signed-off-by: Sireesh Kodali <sireeshkodali1@...il.com>

I'm not sure why this is important.  Callers shouldn't add an
interrupt type that isn't supported by the hardware.  The check
here would be for sanity.

And there's no point in checking in the interrupt remove
function, the only interrupts removed will have already
been added.

Anyway, maybe I'll see you're adding support for these
IPA interrupt types later on?

					-Alex

> ---
>   drivers/net/ipa/ipa_interrupt.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/net/ipa/ipa_interrupt.c b/drivers/net/ipa/ipa_interrupt.c
> index b35170a93b0f..94708a23a597 100644
> --- a/drivers/net/ipa/ipa_interrupt.c
> +++ b/drivers/net/ipa/ipa_interrupt.c
> @@ -48,6 +48,25 @@ static bool ipa_interrupt_uc(struct ipa_interrupt *interrupt, u32 irq_id)
>   	return irq_id == IPA_IRQ_UC_0 || irq_id == IPA_IRQ_UC_1;
>   }
>   
> +static bool ipa_interrupt_check_fixup(enum ipa_irq_id *irq_id, enum ipa_version version)
> +{
> +	switch (*irq_id) {
> +	case IPA_IRQ_EOT_COAL:
> +		return version < IPA_VERSION_3_5;
> +	case IPA_IRQ_DCMP:
> +		return version < IPA_VERSION_4_5;
> +	case IPA_IRQ_TLV_LEN_MIN_DSM:
> +		return version >= IPA_VERSION_4_5;
> +	default:
> +		break;
> +	}
> +
> +	if (*irq_id >= IPA_IRQ_DRBIP_PKT_EXCEED_MAX_SIZE_EN)
> +		return version >= IPA_VERSION_4_9;
> +
> +	return true;
> +}
> +
>   /* Process a particular interrupt type that has been received */
>   static void ipa_interrupt_process(struct ipa_interrupt *interrupt, u32 irq_id)
>   {
> @@ -191,6 +210,9 @@ void ipa_interrupt_add(struct ipa_interrupt *interrupt,
>   	struct ipa *ipa = interrupt->ipa;
>   	u32 offset;
>   
> +	if (!ipa_interrupt_check_fixup(&ipa_irq, ipa->version))
> +		return;
> +
>   	WARN_ON(ipa_irq >= IPA_IRQ_COUNT);
>   
>   	interrupt->handler[ipa_irq] = handler;
> @@ -208,6 +230,9 @@ ipa_interrupt_remove(struct ipa_interrupt *interrupt, enum ipa_irq_id ipa_irq)
>   	struct ipa *ipa = interrupt->ipa;
>   	u32 offset;
>   
> +	if (!ipa_interrupt_check_fixup(&ipa_irq, ipa->version))
> +		return;
> +
>   	WARN_ON(ipa_irq >= IPA_IRQ_COUNT);
>   
>   	/* Update the IPA interrupt mask to disable it */
>