This message generated a parse failure. Raw output follows here. Please use 'back' to navigate. From devnull@lkml.org Sat Apr 27 03:35:04 2024 >From mailfetcher Mon Jun 27 09:33:15 2022 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pi4 X-Spam-Level: X-Spam-Status: No, score=-2.3 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,DMARC_PASS,MAILING_LIST_MULTI, RCVD_IN_ZEN_BLOCKED_OPENDNS,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 Authentication-Results: pi4.bmw12.nl; dmarc=pass (p=none dis=none) header.from=kernel.org Authentication-Results: pi4.bmw12.nl; dkim=pass (Good 2048 bit rsa-sha256 signature) header.d=kernel.org header.a=rsa-sha256 Authentication-Results: pi4.bmw12.nl; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=2620:137:e000::1:20; helo=out1.vger.email; envelope-from=linux-kernel-owner@vger.kernel.org; receiver=) Received: from secure.jasper.es [188.166.10.231] by 1dc7d414a5be with IMAP (fetchmail-6.3.26) for (single-drop); Mon, 27 Jun 2022 09:33:15 +0200 (CEST) Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by pi4.bmw12.nl (Postfix) with ESMTP id 5DFB03F04F for ; Mon, 27 Jun 2022 09:33:09 +0200 (CEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232776AbiF0Hco (ORCPT ); Mon, 27 Jun 2022 03:32:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42836 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232850AbiF0Hcm (ORCPT ); Mon, 27 Jun 2022 03 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 32B5A5FE6 for ; Mon, 27 Jun 2022 00:32:41 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id D4DD5B80CA5 for Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6C426C341C8; Mon, 27 Jun 2022 07:32:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1656315158; bh=oRzFw+QoSeFZcHWgfFlSW6JTW6TExChSidIjg0mxE2Q=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=RFkda938osRp68VLJuwyRXvZr5IggLtnmxt4WQeICb Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1o5jEZ-003LKN- Date: Mon, 27 Jun 2022 08:32:35 +0100 Message-Id: <87czeu36jw.wl-maz@kernel.org> From: Marc Zyngier To: Jianmin Lv Cc: Thomas Gleixner , linux-kernel@vger.kernel.org, Hanjun Guo , Lorenzo Pieralisi , Jiaxun Yang , Huacai Chen References: <1655273250-23495-1-git-send-email-lvjianmin@loongson.cn> <1655273250-23495-2-git-send-email-lvjianmin@loongson.cn> <87k09ipfe2.wl-maz@kernel.org> <0247b7d5-aca9-5db1-e712-4783ee672110@loongson.cn> <87fsk2p8b5.wl-maz@kernel.or User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) Mime-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: lvjianmin@loongson.cn, tglx@linutronix.de, linux-kernel@vger.kernel.org, guohanjun@huawei.com, lorenzo.pieralisi@arm.com, jiaxun.yang@flygoat.com, chenhuacai@loongson.cn X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-Id: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 25 Jun 2022 10:34:34 +0100, Jianmin Lv wrote: >=20 >=20 >=20 > On 2022/6/18 =E4=B8=8B=E5=8D=886:36, Marc Zyngier wrote: > > On Wed, 15 Jun 2022 10:28:47 +0100, > > Jianmin Lv wrote: > >>=20 > >>=20 > >>=20 > >> On 2022/6/15 =E4=B8=8B=E5=8D=883:14, Marc Zyngier wrote: > >>> On Wed, 15 Jun 2022 07:07:21 +0100, > >>> Jianmin Lv wrote: > >>>>=20 > >>>> From: Marc Zyngier > >>>>=20 > >>>> In an unfortunate departure from the ACPI spec, the LoongArch > >>>> architecture split its GSI space across multiple interrupt > >>>> controllers. > >>>>=20 > >>>> In order to be able to reuse sthe core code and prevent > >>>> architectures from reinventing an already square wheel, offer > >>>> the arch code the ability to register a dispatcher function > >>>> that will return the domain fwnode for a given GSI. > >>>>=20 > >>>> The ARM GIC drivers are updated to support this (with a single > >>>> domain, as intended). > >>>>=20 > >>>> Co-developed-by: Jianmin Lv > >>>=20 > >>> I don't think this tag is appropriate here. > >>>=20 > >>>> Signed-off-by: Marc Zyngier > >>>> Cc: Hanjun Guo > >>>> Cc: Lorenzo Pieralisi > >>>> Signed-off-by: Jianmin Lv > >>>> --- > >>>> drivers/acpi/irq.c | 40 +++++++++++++++++++++++--------= --------- > >>>> drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------ > >>>> drivers/irqchip/irq-gic.c | 18 ++++++++++++------ > >>>> include/linux/acpi.h | 2 +- > >>>> 4 files changed, 48 insertions(+), 30 deletions(-) > >>>>=20 > >>>> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c > >>>> index c68e694..b7460ab 100644 > >>>> --- a/drivers/acpi/irq.c > >>>> +++ b/drivers/acpi/irq.c > >>>> @@ -12,7 +12,7 @@ > >>>> enum acpi_irq_model_id acpi_irq_model; > >>>> -static struct fwnode_handle *acpi_gsi_domain_id; > >>>> +static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi); > >>>> /** > >>>> * acpi_gsi_to_irq() - Retrieve the linux irq number for a given = GSI > >>>> @@ -26,10 +26,7 @@ > >>>> */ > >>>> int acpi_gsi_to_irq(u32 gsi, unsigned int *irq) > >>>> { > >>>> - struct irq_domain *d =3D irq_find_matching_fwnode(acpi_gsi_domain_= id, > >>>> - DOMAIN_BUS_ANY); > >>>> - > >>>> - *irq =3D irq_find_mapping(d, gsi); > >>>> + *irq =3D acpi_register_gsi(NULL, gsi, -1, -1); > >>>=20 > >>> What is this? > >>>=20 > >>> - This wasn't part of my initial patch, and randomly changing patches > >>> without mentioning it isn't acceptable > >>>=20 > >>> - you *cannot* trigger a registration here, as this isn't what the API > >>> advertises > >>>=20 > >>> - what makes you think that passing random values (NULL, -1... )to > >>> acpi_register_gsi() is an acceptable thing to do? > >>>=20 > >>> The original patch had: > >>>=20 > >>> @@ -26,8 +26,10 @@ static struct fwnode_handle *acpi_gsi_domain_id; > >>> */ > >>> int acpi_gsi_to_irq(u32 gsi, unsigned int *irq) > >>> { > >>> - struct irq_domain *d =3D irq_find_matching_fwnode(acpi_gsi_domain_i= d, > >>> - DOMAIN_BUS_ANY); > >>> + struct irq_domain *d; > >>> + > >>> + d =3D irq_find_matching_fwnode(acpi_get_gsi_domain_id(gsi), > >>> + DOMAIN_BUS_ANY); > >>> *irq =3D irq_find_mapping(d, gsi); > >>> /* > >>>=20 > >>> and I don't think it needs anything else. If something breaks, let's > >>> discuss it, but don't abuse the API nor the fact that I usually don't > >>> review my own patches to sneak things in... > >>>=20 > >>=20 > >> Sorry, Marc, I don't know how to communicate with you for my change > >> here before submitting the patch, maybe I should mention it in the > >> patch commit or code. > >=20 > > It should at least be discussed first, like you are doing it here. > >=20 > >> When I use the patch, I found that acpi_gsi_to_irq in driver/acpi/irq.c > >> only handle existed mapping and will return -EINVAL if mapping not > >> found. When I test on my machine, a calling stack is as following: > >>=20 > >>=20 > >> acpi_bus_init > >> ->acpi_enable_subsystem > >> ->acpi_ev_install_xrupt_handlers > >> ->acpi_ev_install_sci_handler > >> ->acpi_os_install_interrupt_handler > >> ->acpi_gsi_to_irq > >>=20 > >>=20 > >> the acpi_gsi_to_irq returned -EINVAL because of no mapping found. I > >> looked into acpi_gsi_to_irq of x86, acpi_register_gsi is called in it > >> so that new mapping for gsi is created if no mapping is found. > >=20 > > So it looks like we have a discrepancy between the x86 and ARM on that > > front. > >=20 > > Lorenzo, Hanjun, can you please have a look at this and shed some > > light on what the expected behaviour is? It looks like we never > > encountered an issue with this on arm64, which tends to indicate that > > we don't usually use the above path. > >=20 > >> I looked into generic acpi_register_gsi, the existed mapping will be > >> checked first by calling irq_find_mapping, so I think calling > >> acpi_register_gsi in acpi_gsi_to_irq can address the problem. > >>=20 > >> But you're right, I'm wrong that I passed random value of -1 to > >> acpi_register_gsi. I don't find a right way to address the problem > >> without changing acpi_gsi_to_irq. I'll continue to work for the > >> problem. > >=20 > > At the very least, this should be indirected so that the existing > > behaviour isn't affected, no matter how badly broken arm64 may or may > > not be here. Please have a look at the patch below that should help > > you with this. > >=20 > > Thanks, > >=20 > > M. > >=20 > > From 3e6b87ea49473d0eb384f42e76d584a1495a538c Mon Sep 17 00:00:00 2001 > > From: Marc Zyngier > > Date: Sat, 18 Jun 2022 11:29:33 +0100 > > Subject: [PATCH] ACPI: irq: Allow acpi_gsi_to_irq() to have an arch-spe= cific > > fallback > >=20 > > It appears that the generic version of acpi_gsi_to_irq() doesn't > > fallback to establishing a mapping if there is no pre-existing > > one while the x86 version does. > >=20 > > While arm64 seems unaffected by it, LoongArch is relying on the x86 > > behaviour. In an effort to prevent new architectures from reinventing > > the proverbial wheel, provide an optional callback that the arch code > > can set to restore the x86 behaviour. > >=20 > > Hopefully we can eventually get rid of this in the future once > > the expected behaviour has been clarified. > >=20 > > Reported-by: Jianmin Lv > > Signed-off-by: Marc Zyngier > > --- > > drivers/acpi/irq.c | 8 ++++++-- > > include/linux/acpi.h | 1 + > > 2 files changed, 7 insertions(+), 2 deletions(-) > >=20 > > diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c > > index 6e1633ac1756..66c5f01995d0 100644 > > --- a/drivers/acpi/irq.c > > +++ b/drivers/acpi/irq.c > > @@ -13,6 +13,7 @@ > > enum acpi_irq_model_id acpi_irq_model; > > static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi); > > +static int (*acpi_gsi_to_irq_fallback)(u32 gsi); > > /** > > * acpi_gsi_to_irq() - Retrieve the linux irq number for a given GSI > > @@ -33,9 +34,12 @@ int acpi_gsi_to_irq(u32 gsi, unsigned int *irq) > > *irq =3D irq_find_mapping(d, gsi); > > /* > > - * *irq =3D=3D 0 means no mapping, that should > > - * be reported as a failure > > + * *irq =3D=3D 0 means no mapping, that should be reported as a > > + * failure, unless there is an arch-specific fallback handler. > > */ > > + if (!*irq && acpi_gsi_to_irq_fallback) > > + *irq =3D acpi_gsi_to_irq_fallback(gsi); > > + > > return (*irq > 0) ? 0 : -EINVAL; > > } > > EXPORT_SYMBOL_GPL(acpi_gsi_to_irq); > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > > index 957e23f727ea..71d3719e3ec4 100644 > > --- a/include/linux/acpi.h > > +++ b/include/linux/acpi.h > > @@ -357,6 +357,7 @@ int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi= ); > > void acpi_set_irq_model(enum acpi_irq_model_id model, > > struct fwnode_handle *(*)(u32)); > > +void acpi_set_gsi_to_irq_fallback(int (*)(u32)); > > =20 >=20 > Hi, Marc >=20 > I want to make sure that if acpi_set_gsi_to_irq_fallback should be > implemented in driver/acpi/irq.c as acpi_set_irq_model, e.g.: >=20 > void __init acpi_set_gsi_to_irq_fallback(int (*fn)(u32)) > { > acpi_gsi_to_irq_fallback =3D fn; > } >=20 > And then, arch related code can call acpi_set_gsi_to_irq_fallback > to register a callback. Yes. I had something like that, but forgot to add it to the patch, apparently. =17 M. --=20 Without deviation from the norm, progress is not possible.