linux-kernel - Re: [PATCH v6 21/31] irqchip/gic-v5: Add GICv5 IRS/SPI 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250702140459.000063ef@huawei.com>
Date: Wed, 2 Jul 2025 14:04:59 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Lorenzo Pieralisi <lpieralisi@...nel.org>
CC: Marc Zyngier <maz@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, "Rob
 Herring" <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, "Conor
 Dooley" <conor+dt@...nel.org>, Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>, Arnd Bergmann <arnd@...db.de>, "Sascha
 Bischoff" <sascha.bischoff@....com>, Timothy Hayes <timothy.hayes@....com>,
	Bjorn Helgaas <bhelgaas@...gle.com>, "Liam R. Howlett"
	<Liam.Howlett@...cle.com>, Peter Maydell <peter.maydell@...aro.org>, "Mark
 Rutland" <mark.rutland@....com>, Jiri Slaby <jirislaby@...nel.org>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
	<devicetree@...r.kernel.org>, <linux-pci@...r.kernel.org>
Subject: Re: [PATCH v6 21/31] irqchip/gic-v5: Add GICv5 IRS/SPI support



There are more ID masks not using FIELD_PREP() in here, but
as mentioned in earlier reply, even if we decide those are worth
cleaning up, no reason it can't be as a trivial patch on top of the
series.

Otherwise just one comment walking back some feedback on an earlier
patch (as what you had there now makes sense)

Jonathan

> diff --git a/drivers/irqchip/irq-gic-v5-irs.c b/drivers/irqchip/irq-gic-v5-irs.c
> new file mode 100644
> index 000000000000..fba8efceb26e
> --- /dev/null
> +++ b/drivers/irqchip/irq-gic-v5-irs.c
> @@ -0,0 +1,434 @@

> -static int gicv5_irq_ppi_domain_translate(struct irq_domain *d,
> -					  struct irq_fwspec *fwspec,
> -					  irq_hw_number_t *hwirq,
> -					  unsigned int *type)
> +static const struct irq_chip gicv5_spi_irq_chip = {
> +	.name			= "GICv5-SPI",
> +	.irq_mask		= gicv5_spi_irq_mask,
> +	.irq_unmask		= gicv5_spi_irq_unmask,
> +	.irq_eoi		= gicv5_spi_irq_eoi,
> +	.irq_set_type		= gicv5_spi_irq_set_type,
> +	.irq_set_affinity	= gicv5_spi_irq_set_affinity,
> +	.irq_retrigger		= gicv5_spi_irq_retrigger,
> +	.irq_get_irqchip_state	= gicv5_spi_irq_get_irqchip_state,
> +	.irq_set_irqchip_state	= gicv5_spi_irq_set_irqchip_state,
> +	.flags			= IRQCHIP_SET_TYPE_MASKED |
> +				  IRQCHIP_SKIP_SET_WAKE	  |
> +				  IRQCHIP_MASK_ON_SUSPEND,
> +};
> +
> +static __always_inline int gicv5_irq_domain_translate(struct irq_domain *d,
> +						      struct irq_fwspec *fwspec,
> +						      irq_hw_number_t *hwirq,
> +						      unsigned int *type,
> +						      const u8 hwirq_typ)

>  {
>  	if (!is_of_node(fwspec->fwnode))
>  		return -EINVAL;
> @@ -235,20 +428,39 @@ static int gicv5_irq_ppi_domain_translate(struct irq_domain *d,
>  	if (fwspec->param_count < 3)
>  		return -EINVAL;
>  
> -	if (fwspec->param[0] != GICV5_HWIRQ_TYPE_PPI)
> +	if (fwspec->param[0] != hwirq_type)
>  		return -EINVAL;
>  
>  	*hwirq = fwspec->param[1];
>  
> -	/*
> -	 * Handling mode is hardcoded for PPIs, set the type using
> -	 * HW reported value.
> -	 */
> -	*type = gicv5_ppi_irq_is_level(*hwirq) ? IRQ_TYPE_LEVEL_LOW : IRQ_TYPE_EDGE_RISING;
> +	switch (hwirq_type) {
> +	case GICV5_HWIRQ_TYPE_PPI:
> +		/*
> +		 * Handling mode is hardcoded for PPIs, set the type using
> +		 * HW reported value.
> +		 */
> +		*type = gicv5_ppi_irq_is_level(*hwirq) ? IRQ_TYPE_LEVEL_LOW :
> +							 IRQ_TYPE_EDGE_RISING;
> +		break;
> +	case GICV5_HWIRQ_TYPE_SPI:
> +		*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;

Ah fair enough in earlier patches on just enforcing 3 parameters for all cases.
Seems like a sensible simplification once this is taken into account. So ignore that one!

> +		break;
> +	default:
> +		BUILD_BUG_ON(1);
> +	}
>  
>  	return 0;
>  }