linux-kernel - Re: [PATCH v3 06/10] irqchip: irq-mips-gic: Switch to ipi

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: <871q2fghu8.ffs@tglx>
Date: Fri, 23 Aug 2024 21:27:43 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Jiaxun Yang <jiaxun.yang@...goat.com>, Thomas Bogendoerfer
 <tsbogend@...ha.franken.de>, Florian Fainelli
 <florian.fainelli@...adcom.com>, Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>, Huacai Chen
 <chenhuacai@...nel.org>, Serge Semin <fancer.lancer@...il.com>, Paul
 Burton <paulburton@...nel.org>
Cc: linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org, Jiaxun Yang
 <jiaxun.yang@...goat.com>
Subject: Re: [PATCH v3 06/10] irqchip: irq-mips-gic: Switch to ipi_mux

On Sat, Aug 10 2024 at 13:39, Jiaxun Yang wrote:

The interrupt subsystem uses irqchip/irq....: as prefix.

> Use ipi_mux to implement IPI interrupts instead of
> allocating vector for each individual IPI messages.

  allocating separate vectors for ... IPI message.

> This can reduce number of reserved GIC shared vectors,

Can? Does it or does it not?

Also: the number

> which is a huge problem on MSI enabled GIC systems.
>
> It also allowed us to easily expand number of IPIs.

s/allowed us/allows/           the number

>  int gic_get_c0_compare_int(void)
>  {
>  	if (!gic_local_irq_is_routable(GIC_LOCAL_INT_TIMER))
> @@ -181,6 +176,11 @@ static void gic_mask_irq(struct irq_data *d)
>  	unsigned int intr = GIC_HWIRQ_TO_SHARED(d->hwirq);
>  
>  	write_gic_rmask(intr);
> +
> +#ifdef CONFIG_GENERIC_IRQ_IPI
> +	if (test_bit(intr, ipi_resrv))
> +		return;
> +#endif

Can you please wrap this into a inline function so that you don't have
to sprinkle all these #ifdefs into the code?

#ifdef CONFIG_GENERIC_IRQ_IPI
static inline bool gic_is_reserved(unsigned int intr)
{
        return test_bit(intr, ipi_resrv);
}
#else
static inline bool gic_is_reserved(unsigned int intr) { return false; }
#endif

Hmm?

> +static int gic_ipi_mux_init(struct device_node *node, struct irq_domain *d)
>  {
> -	struct irq_domain *gic_ipi_domain;
> -	unsigned int v[2], num_ipis;
> -
> -	gic_ipi_domain = irq_domain_add_hierarchy(gic_irq_domain,
> -						  IRQ_DOMAIN_FLAG_IPI_PER_CPU,
> -						  GIC_NUM_LOCAL_INTRS + gic_shared_intrs,
> -						  node, &gic_ipi_domain_ops, NULL);
> -	if (!gic_ipi_domain) {
> -		pr_err("Failed to add IPI domain");
> -		return -ENXIO;
> -	}
> -
> -	irq_domain_update_bus_token(gic_ipi_domain, DOMAIN_BUS_IPI);
> +	unsigned int i, v[2], num_ipis;
> +	int ipi_virq, cpu = 0;
>  
>  	if (node &&
>  	    !of_property_read_u32_array(node, "mti,reserved-ipi-vectors", v, 2)) {

Please use the full 100 characters width here and get rid of the line break.

>  		bitmap_set(ipi_resrv, v[0], v[1]);
>  	} else {
>  		/*
> -		 * Reserve 2 interrupts per possible CPU/VP for use as IPIs,
> -		 * meeting the requirements of arch/mips SMP.
> +		 * Reserve 1 interrupts per possible CPU/VP for use as IPIs
>  		 */
> -		num_ipis = 2 * num_possible_cpus();
> +		num_ipis = num_possible_cpus();
>  		bitmap_set(ipi_resrv, gic_shared_intrs - num_ipis, num_ipis);
>  	}

Thanks,

        tglx