linux-kernel - Re: [PATCH RESEND v1 1/2] ACPI/PCI: Make

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220705172931.GA71865@bhelgaas>
Date:   Tue, 5 Jul 2022 12:29:31 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Pierre Gondois <pierre.gondois@....com>
Cc:     linux-kernel@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>, linux-pci@...r.kernel.org,
        linux-acpi@...r.kernel.org
Subject: Re: [PATCH RESEND v1 1/2] ACPI/PCI: Make _SRS optional for link
 device

On Fri, Jul 01, 2022 at 06:16:23PM +0200, Pierre Gondois wrote:
> From: Pierre Gondois <Pierre.Gondois@....com>
> 
> In ACPI 6.4, s6.2.13 "_PRT (PCI Routing Table)", PCI legacy
> interrupts can be described though a link device (first model).
> From s6.2.16 "_SRS (Set Resource Settings)":
> "This optional control method [...]"
> 
> Make it optional to have a _SRS method for link devices.

I think it would be helpful to outline the reason for wanting these
changes in the commit log.  Otherwise we don't know the benefit and
it's harder to justify making the change since it's not an obvious
cleanup.

IIRC from [1] there *is* a good reason: you need to use Interrupt Link
devices so you can specify "level triggered, active high".

Without an Interrupt Link, you would get the default "level triggered,
active low" setting, which apparently isn't compatible with GICv2.

I assume this fixes a device that previously didn't work correctly,
but I don't see the details of that in the bugzilla.  I'm a little
confused about this.  Isn't GICv2 widely used already?  How are things
working now?  Or are there just a lot of broken devices?

[1] https://lore.kernel.org/r/e2ae06ba-de8f-2cae-60fa-fe9a215d779b@arm.com

> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=215560
> Signed-off-by: Pierre Gondois <pierre.gondois@....com>
> ---
>  drivers/acpi/pci_link.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> index 58647051c948..129e3e7e80ee 100644
> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c
> @@ -288,6 +288,13 @@ static int acpi_pci_link_set(struct acpi_pci_link *link, int irq)
>  	if (!irq)
>  		return -EINVAL;
>  
> +	if (!acpi_has_method(handle, METHOD_NAME__SRS)) {
> +		if (link->irq.active == irq)
> +			return 0;
> +		acpi_handle_err(handle, "Unable to set IRQ %d: No _SRS.\n", irq);
> +		return -ENODEV;
> +	}
> +
>  	resource = kzalloc(sizeof(*resource) + 1, irqs_disabled() ? GFP_ATOMIC: GFP_KERNEL);
>  	if (!resource)
>  		return -ENOMEM;
> -- 
> 2.25.1
>