From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 70481C43462 for ; Tue, 6 Apr 2021 18:16:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4B9C9613B8 for ; Tue, 6 Apr 2021 18:16:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237311AbhDFSQc (ORCPT ); Tue, 6 Apr 2021 14:16:32 -0400 Received: from mail.kernel.org ([198.145.29.99]:44054 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236444AbhDFSQa (ORCPT ); Tue, 6 Apr 2021 14:16:30 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 20996613B3; Tue, 6 Apr 2021 18:16:22 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94) (envelope-from ) id 1lTqFQ-005uvp-1y; Tue, 06 Apr 2021 19:16:20 +0100 Date: Tue, 06 Apr 2021 19:16:18 +0100 Message-ID: <87ft03p9cd.wl-maz@kernel.org> From: Marc Zyngier To: Hector Martin Cc: linux-arm-kernel@lists.infradead.org, Rob Herring , Arnd Bergmann , Olof Johansson , Krzysztof Kozlowski , Mark Kettenis , Tony Lindgren , Mohamed Mediouni , Stan Skowronek , Alexander Graf , Will Deacon , Linus Walleij , Mark Rutland , Andy Shevchenko , Jonathan Corbet , Catalin Marinas , Christoph Hellwig , "David S. Miller" , devicetree@vger.kernel.org, linux-doc@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 15/18] irqchip/apple-aic: Add support for the Apple Interrupt Controller In-Reply-To: <20210402090542.131194-16-marcan@marcan.st> References: <20210402090542.131194-1-marcan@marcan.st> <20210402090542.131194-16-marcan@marcan.st> 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=US-ASCII X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: marcan@marcan.st, linux-arm-kernel@lists.infradead.org, robh@kernel.org, arnd@kernel.org, olof@lixom.net, krzk@kernel.org, mark.kettenis@xs4all.nl, tony@atomide.com, mohamed.mediouni@caramail.com, stan@corellium.com, graf@amazon.com, will@kernel.org, linus.walleij@linaro.org, mark.rutland@arm.com, andy.shevchenko@gmail.com, corbet@lwn.net, catalin.marinas@arm.com, hch@infradead.org, davem@davemloft.net, devicetree@vger.kernel.org, linux-doc@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org 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 Hi Hector, On Fri, 02 Apr 2021 10:05:39 +0100, Hector Martin wrote: > > This is the root interrupt controller used on Apple ARM SoCs such as the > M1. This irqchip driver performs multiple functions: > > * Handles both IRQs and FIQs > > * Drives the AIC peripheral itself (which handles IRQs) > > * Dispatches FIQs to downstream hard-wired clients (currently the ARM > timer). > > * Implements a virtual IPI multiplexer to funnel multiple Linux IPIs > into a single hardware IPI > > Signed-off-by: Hector Martin > --- > MAINTAINERS | 2 + > drivers/irqchip/Kconfig | 8 + > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-apple-aic.c | 837 ++++++++++++++++++++++++++++++++ > include/linux/cpuhotplug.h | 1 + > 5 files changed, 849 insertions(+) > create mode 100644 drivers/irqchip/irq-apple-aic.c [...] > +static int aic_irq_domain_translate(struct irq_domain *id, > + struct irq_fwspec *fwspec, > + unsigned long *hwirq, > + unsigned int *type) > +{ > + struct aic_irq_chip *ic = id->host_data; > + > + if (fwspec->param_count != 3 || !is_of_node(fwspec->fwnode)) > + return -EINVAL; > + > + switch (fwspec->param[0]) { > + case AIC_IRQ: > + if (fwspec->param[1] >= ic->nr_hw) > + return -EINVAL; > + *hwirq = fwspec->param[1]; > + break; > + case AIC_FIQ: > + if (fwspec->param[1] >= AIC_NR_FIQ) > + return -EINVAL; > + *hwirq = ic->nr_hw + fwspec->param[1]; > + > + /* > + * In EL1 the non-redirected registers are the guest's, > + * not EL2's, so remap the hwirqs to match. > + */ > + if (!is_kernel_in_hyp_mode()) { > + switch (fwspec->param[1]) { > + case AIC_TMR_GUEST_PHYS: > + *hwirq = ic->nr_hw + AIC_TMR_HV_PHYS; > + break; > + case AIC_TMR_GUEST_VIRT: > + *hwirq = ic->nr_hw + AIC_TMR_HV_VIRT; > + break; > + case AIC_TMR_HV_PHYS: > + case AIC_TMR_HV_VIRT: > + return -ENOENT; > + default: > + break; > + } > + } Urgh, this is nasty. You are internally remapping the hwirq from one timer to another in order to avoid accessing the enable register which happens to be an EL2 only register? The way we normally deal with this kind of things is by providing a different set of irq_chip callbacks. In this particular case, this would leave mask/unmask as empty stubs. Or you could move the FIQ handling to use handle_simple_irq(), because there isn't any callback that is actually applicable. It isn't a big deal for now, but that's something we should consider addressing in the future. With that in mind: Reviewed-by: Marc Zyngier Thanks, M. -- Without deviation from the norm, progress is not possible. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5BDF2C433ED for ; Tue, 6 Apr 2021 18:18:29 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id EEB9F613B8 for ; Tue, 6 Apr 2021 18:18:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EEB9F613B8 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Subject:Cc:To: From:Message-ID:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=jjFF2/q552+5M4pH/VKl7ESVhSJcROL59M+pogiCu84=; b=egCees4UmPkPKxhMtpb4e4BbI 7n3ftWMJKzsvZvcUO4vVNNBAn+TcX3cb/MNA0H/at+s4He90RhNchlk1l8HM2nerb1UTif9JhAzmG PUX5A8MfoduxrGiYYXYn2sBDlz/56+GW7LlrkbUnRw/WkBTTpTBE4JjH/apiqvM06+MXdECegfbA4 TymktxeE4GH4/92aqmDRicUPToixD5ndR+YehzKU/8+TItMb3QasRmFSpEmK6ZQXYfcNP60KED+4p 2ahYykuP3e+y3FzrNAys/lVghzGkWSkeFu1m19r4ofrgQSilo/cWSw+izPyJPjtTLU7A/CVw56RC1 RfEWTuMtw==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lTqFa-003DKJ-06; Tue, 06 Apr 2021 18:16:30 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lTqFT-003DHS-Pm for linux-arm-kernel@lists.infradead.org; Tue, 06 Apr 2021 18:16:26 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 20996613B3; Tue, 6 Apr 2021 18:16:22 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94) (envelope-from ) id 1lTqFQ-005uvp-1y; Tue, 06 Apr 2021 19:16:20 +0100 Date: Tue, 06 Apr 2021 19:16:18 +0100 Message-ID: <87ft03p9cd.wl-maz@kernel.org> From: Marc Zyngier To: Hector Martin Cc: linux-arm-kernel@lists.infradead.org, Rob Herring , Arnd Bergmann , Olof Johansson , Krzysztof Kozlowski , Mark Kettenis , Tony Lindgren , Mohamed Mediouni , Stan Skowronek , Alexander Graf , Will Deacon , Linus Walleij , Mark Rutland , Andy Shevchenko , Jonathan Corbet , Catalin Marinas , Christoph Hellwig , "David S. Miller" , devicetree@vger.kernel.org, linux-doc@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 15/18] irqchip/apple-aic: Add support for the Apple Interrupt Controller In-Reply-To: <20210402090542.131194-16-marcan@marcan.st> References: <20210402090542.131194-1-marcan@marcan.st> <20210402090542.131194-16-marcan@marcan.st> 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") X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: marcan@marcan.st, linux-arm-kernel@lists.infradead.org, robh@kernel.org, arnd@kernel.org, olof@lixom.net, krzk@kernel.org, mark.kettenis@xs4all.nl, tony@atomide.com, mohamed.mediouni@caramail.com, stan@corellium.com, graf@amazon.com, will@kernel.org, linus.walleij@linaro.org, mark.rutland@arm.com, andy.shevchenko@gmail.com, corbet@lwn.net, catalin.marinas@arm.com, hch@infradead.org, davem@davemloft.net, devicetree@vger.kernel.org, linux-doc@vger.kernel.org, linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210406_191625_001077_22DC5B89 X-CRM114-Status: GOOD ( 30.17 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Hector, On Fri, 02 Apr 2021 10:05:39 +0100, Hector Martin wrote: > > This is the root interrupt controller used on Apple ARM SoCs such as the > M1. This irqchip driver performs multiple functions: > > * Handles both IRQs and FIQs > > * Drives the AIC peripheral itself (which handles IRQs) > > * Dispatches FIQs to downstream hard-wired clients (currently the ARM > timer). > > * Implements a virtual IPI multiplexer to funnel multiple Linux IPIs > into a single hardware IPI > > Signed-off-by: Hector Martin > --- > MAINTAINERS | 2 + > drivers/irqchip/Kconfig | 8 + > drivers/irqchip/Makefile | 1 + > drivers/irqchip/irq-apple-aic.c | 837 ++++++++++++++++++++++++++++++++ > include/linux/cpuhotplug.h | 1 + > 5 files changed, 849 insertions(+) > create mode 100644 drivers/irqchip/irq-apple-aic.c [...] > +static int aic_irq_domain_translate(struct irq_domain *id, > + struct irq_fwspec *fwspec, > + unsigned long *hwirq, > + unsigned int *type) > +{ > + struct aic_irq_chip *ic = id->host_data; > + > + if (fwspec->param_count != 3 || !is_of_node(fwspec->fwnode)) > + return -EINVAL; > + > + switch (fwspec->param[0]) { > + case AIC_IRQ: > + if (fwspec->param[1] >= ic->nr_hw) > + return -EINVAL; > + *hwirq = fwspec->param[1]; > + break; > + case AIC_FIQ: > + if (fwspec->param[1] >= AIC_NR_FIQ) > + return -EINVAL; > + *hwirq = ic->nr_hw + fwspec->param[1]; > + > + /* > + * In EL1 the non-redirected registers are the guest's, > + * not EL2's, so remap the hwirqs to match. > + */ > + if (!is_kernel_in_hyp_mode()) { > + switch (fwspec->param[1]) { > + case AIC_TMR_GUEST_PHYS: > + *hwirq = ic->nr_hw + AIC_TMR_HV_PHYS; > + break; > + case AIC_TMR_GUEST_VIRT: > + *hwirq = ic->nr_hw + AIC_TMR_HV_VIRT; > + break; > + case AIC_TMR_HV_PHYS: > + case AIC_TMR_HV_VIRT: > + return -ENOENT; > + default: > + break; > + } > + } Urgh, this is nasty. You are internally remapping the hwirq from one timer to another in order to avoid accessing the enable register which happens to be an EL2 only register? The way we normally deal with this kind of things is by providing a different set of irq_chip callbacks. In this particular case, this would leave mask/unmask as empty stubs. Or you could move the FIQ handling to use handle_simple_irq(), because there isn't any callback that is actually applicable. It isn't a big deal for now, but that's something we should consider addressing in the future. With that in mind: Reviewed-by: Marc Zyngier Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel