| 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
| ||
|
Date: Sat, 18 Jun 2022 18:22:03 +0100
From: Marc Zyngier <maz@...nel.org>
To: Jianmin Lv <lvjianmin@...ngson.cn>
Cc: Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org,
Hanjun Guo <guohanjun@...wei.com>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Jiaxun Yang <jiaxun.yang@...goat.com>,
Huacai Chen <chenhuacai@...ngson.cn>
Subject: Re: [PATCH V12 04/10] irqchip: create library file for LoongArch irqchip driver
On Wed, 15 Jun 2022 07:07:24 +0100,
Jianmin Lv <lvjianmin@...ngson.cn> wrote:
>
> The library file contains following content:
> - Implement acpi_get_gsi_domain_id callback.
> - Implement initialization of vector group entries and APIs
> for building hierachy irqdomains.
>
> Signed-off-by: Jianmin Lv <lvjianmin@...ngson.cn>
> ---
> drivers/irqchip/Makefile | 2 +-
> drivers/irqchip/irq-loongarch-pic-common.c | 122 +++++++++++++++++++++++++++++
> drivers/irqchip/irq-loongarch-pic-common.h | 39 +++++++++
> 3 files changed, 162 insertions(+), 1 deletion(-)
> create mode 100644 drivers/irqchip/irq-loongarch-pic-common.c
> create mode 100644 drivers/irqchip/irq-loongarch-pic-common.h
>
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 6894a13..2d0d871 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -103,7 +103,7 @@ obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
> obj-$(CONFIG_TI_SCI_INTR_IRQCHIP) += irq-ti-sci-intr.o
> obj-$(CONFIG_TI_SCI_INTA_IRQCHIP) += irq-ti-sci-inta.o
> obj-$(CONFIG_TI_PRUSS_INTC) += irq-pruss-intc.o
> -obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o
> +obj-$(CONFIG_IRQ_LOONGARCH_CPU) += irq-loongarch-cpu.o irq-loongarch-pic-common.o
> obj-$(CONFIG_LOONGSON_LIOINTC) += irq-loongson-liointc.o
> obj-$(CONFIG_LOONGSON_HTPIC) += irq-loongson-htpic.o
> obj-$(CONFIG_LOONGSON_HTVEC) += irq-loongson-htvec.o
> diff --git a/drivers/irqchip/irq-loongarch-pic-common.c b/drivers/irqchip/irq-loongarch-pic-common.c
> new file mode 100644
> index 0000000..2f75362
> --- /dev/null
> +++ b/drivers/irqchip/irq-loongarch-pic-common.c
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 Loongson Limited, All Rights Reserved.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/pci.h>
> +#include <linux/acpi.h>
> +#include "irq-loongarch-pic-common.h"
> +
> +static struct acpi_vector_group vector_group[MAX_IO_PICS];
> +
> +struct acpi_madt_bio_pic *acpi_pchpic[MAX_IO_PICS];
> +
> +struct fwnode_handle *liointc_handle;
> +struct fwnode_handle *pch_lpc_handle;
> +struct fwnode_handle *pch_msi_handle[MAX_IO_PICS];
> +struct fwnode_handle *pch_pic_handle[MAX_IO_PICS];
Why aren't these in individual drivers, and then have accessors to
retrieve them?
> +
> +static int find_pch_pic(u32 gsi)
> +{
> + int i, start, end;
> +
> + /* Find the PCH_PIC that manages this GSI. */
> + for (i = 0; i < MAX_IO_PICS; i++) {
> + struct acpi_madt_bio_pic *irq_cfg = acpi_pchpic[i];
> +
> + if (!irq_cfg)
> + return -1;
> +
> + start = irq_cfg->gsi_base;
> + end = irq_cfg->gsi_base + irq_cfg->size;
> + if (gsi >= start && gsi < end)
> + return i;
> + }
> +
> + pr_err("ERROR: Unable to locate PCH_PIC for GSI %d\n", gsi);
> + return -1;
> +}
Same thing. This really should be in the PCH driver, and be called by
lpic_get_gsi_domain().
> +
> +struct fwnode_handle *lpic_get_gsi_domain_id(u32 gsi)
> +{
> + int id;
> + struct fwnode_handle *domain_handle = NULL;
> +
> + switch (gsi) {
> + case GSI_MIN_CPU_IRQ ... GSI_MAX_CPU_IRQ:
> + if (liointc_handle)
> + domain_handle = liointc_handle;
> + break;
> +
> + case GSI_MIN_LPC_IRQ ... GSI_MAX_LPC_IRQ:
> + if (pch_lpc_handle)
> + domain_handle = pch_lpc_handle;
> + break;
> +
> + case GSI_MIN_PCH_IRQ ... GSI_MAX_PCH_IRQ:
> + id = find_pch_pic(gsi);
> + if (id >= 0 && pch_pic_handle[id])
> + domain_handle = pch_pic_handle[id];
> +
> + break;
> + }
> +
> + return domain_handle;
> +}
> +
> +static int pci_mcfg_parse(struct acpi_table_header *header)
> +{
> + struct acpi_table_mcfg *mcfg;
> + struct acpi_mcfg_allocation *mptr;
> + int i, n;
> +
> + if (header->length < sizeof(struct acpi_table_mcfg))
> + return -EINVAL;
> +
> + n = (header->length - sizeof(struct acpi_table_mcfg)) /
> + sizeof(struct acpi_mcfg_allocation);
> + mcfg = (struct acpi_table_mcfg *)header;
> + mptr = (struct acpi_mcfg_allocation *) &mcfg[1];
> +
> + for (i = 0; i < n; i++, mptr++)
> + vector_group[mptr->pci_segment].node = (mptr->address >> 44) & 0xf;
> +
> + return 0;
> +}
> +
> +void __init init_vector_parent_group(void)
> +{
> + acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse);
> +}
I really don't think the PCI code should be anywhere near
this. Frankly, this file looks like a dumping ground for totally
unrelated stuff.
> +
> +void acpi_set_vector_parent(int node, struct irq_domain *parent)
> +{
> + int i;
> +
> + if (cpu_has_flatmode)
> + node = cpu_to_node(node * CORES_PER_EIO_NODE);
> +
> + for (i = 0; i < MAX_IO_PICS; i++) {
> + if (node == vector_group[i].node) {
> + vector_group[i].parent = parent;
> + return;
> + }
> + }
> +}
> +
> +struct irq_domain *acpi_get_msi_parent(int index)
> +{
> + return vector_group[index].parent;
> +}
> +
> +struct irq_domain *acpi_get_pch_parent(int node)
> +{
> + int i;
> +
> + for (i = 0; i < MAX_IO_PICS; i++) {
> + if (node == vector_group[i].node)
> + return vector_group[i].parent;
> + }
> + return NULL;
> +}
Same thing. There is nothing "common" here. All this should be split
in the various drivers, where they belong.
M.
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists