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=-6.8 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,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 06906C43332 for ; Thu, 19 Mar 2020 10:27:52 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 80BF520740 for ; Thu, 19 Mar 2020 10:27:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="RkCD8EU7" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 80BF520740 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 1FEC24B09A; Thu, 19 Mar 2020 06:27:51 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@kernel.org Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ozhuNf8rYr+v; Thu, 19 Mar 2020 06:27:49 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id D91884B089; Thu, 19 Mar 2020 06:27:49 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 58C8D4A500 for ; Thu, 19 Mar 2020 06:27:49 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id XP6wLuYxvD+o for ; Thu, 19 Mar 2020 06:27:48 -0400 (EDT) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 197094A4A3 for ; Thu, 19 Mar 2020 06:27:48 -0400 (EDT) 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 F38A5206D7; Thu, 19 Mar 2020 10:27:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1584613667; bh=uwoazedwTNP0+xIsOawzejd5iSeA0BwzwkyqJeruVqE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=RkCD8EU7JV2FdgiasHUyA76NEFCRtSZNlIezGQCI0JeR1C3P0bTgIZMV/jJEofEz+ alTAqUt2mV0EkoXDUnrrrSOBx8vcrswnCMUqqAdt4g0BWvgKRVRVA31AjrB/YcAAS1 w8HvzmpttbnMQBxq0GkQgopeKhARA/Lh/HsH5ql8= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1jEsOu-00Dua1-W4; Thu, 19 Mar 2020 10:27:45 +0000 MIME-Version: 1.0 Date: Thu, 19 Mar 2020 10:27:44 +0000 From: Marc Zyngier To: Auger Eric Subject: Re: [PATCH v5 11/23] irqchip/gic-v4.1: Plumb get/set_irqchip_state SGI callbacks In-Reply-To: References: <20200304203330.4967-1-maz@kernel.org> <20200304203330.4967-12-maz@kernel.org> Message-ID: <6b2fec7cdc53536997edf4db971e1d47@kernel.org> X-Sender: maz@kernel.org User-Agent: Roundcube Webmail/1.3.10 X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: eric.auger@redhat.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, lorenzo.pieralisi@arm.com, jason@lakedaemon.net, rrichter@marvell.com, tglx@linutronix.de, yuzenghui@huawei.com, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Cc: Lorenzo Pieralisi , Jason Cooper , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Robert Richter , Thomas Gleixner , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu Hi Eric, On 2020-03-16 21:43, Auger Eric wrote: > Hi Marc, > > On 3/4/20 9:33 PM, Marc Zyngier wrote: >> To implement the get/set_irqchip_state callbacks (limited to the >> PENDING state), we have to use a particular set of hacks: >> >> - Reading the pending state is done by using a pair of new >> redistributor >> registers (GICR_VSGIR, GICR_VSGIPENDR), which allow the 16 >> interrupts >> state to be retrieved. >> - Setting the pending state is done by generating it as we'd otherwise >> do >> for a guest (writing to GITS_SGIR). >> - Clearing the pending state is done by emiting a VSGI command with >> the > emitting >> "clear" bit set. >> >> This requires some interesting locking though: >> - When talking to the redistributor, we must make sure that the VPE >> affinity doesn't change, hence taking the VPE lock. >> - At the same time, we must ensure that nobody accesses the same >> redistributor's GICR_VSGI*R registers for a different VPE, which > GICR_VSGIR >> would corrupt the reading of the pending bits. We thus take the >> per-RD spinlock. Much fun. >> >> Signed-off-by: Marc Zyngier >> --- >> drivers/irqchip/irq-gic-v3-its.c | 73 >> ++++++++++++++++++++++++++++++ >> include/linux/irqchip/arm-gic-v3.h | 14 ++++++ >> 2 files changed, 87 insertions(+) >> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c >> b/drivers/irqchip/irq-gic-v3-its.c >> index c93f178914ee..fb2b836c31ff 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -3962,11 +3962,84 @@ static int its_sgi_set_affinity(struct >> irq_data *d, >> return -EINVAL; >> } >> >> +static int its_sgi_set_irqchip_state(struct irq_data *d, >> + enum irqchip_irq_state which, >> + bool state) >> +{ >> + if (which != IRQCHIP_STATE_PENDING) >> + return -EINVAL; >> + >> + if (state) { >> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d); >> + struct its_node *its = find_4_1_its(); >> + u64 val; >> + >> + val = FIELD_PREP(GITS_SGIR_VPEID, vpe->vpe_id); >> + val |= FIELD_PREP(GITS_SGIR_VINTID, d->hwirq); >> + writeq_relaxed(val, its->sgir_base + GITS_SGIR - SZ_128K); >> + } else { >> + its_configure_sgi(d, true); >> + } >> + >> + return 0; >> +} >> + >> +static int its_sgi_get_irqchip_state(struct irq_data *d, >> + enum irqchip_irq_state which, bool *val) >> +{ >> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d); >> + void __iomem *base; >> + unsigned long flags; >> + u32 count = 1000000; /* 1s! */ > where does it come from? I did not find any info in the spec about this > delay. There is no such thing in the spec. However, these timeouts have proven to be very useful in detecting broken HW (I'm lucky enough to have such wonders at hand...), as well as SW bugs. The ! second value is purely empirical (if a whole second is not enough for things to move, they're as good as dead). >> + u32 status; >> + int cpu; >> + >> + if (which != IRQCHIP_STATE_PENDING) >> + return -EINVAL; >> + >> + /* >> + * Locking galore! We can race against two different events: >> + * >> + * - Concurent vPE affinity change: we must make sure it cannot >> + * happen, or we'll talk to the wrong redistributor. This >> is >> + * identical to what happens with vLPIs. >> + * >> + * - Concurrent VSGIPENDR access: As it involves accessing two >> + * MMIO registers, this must be made atomic one way or >> another. >> + */ >> + cpu = vpe_to_cpuid_lock(vpe, &flags); >> + raw_spin_lock(&gic_data_rdist_cpu(cpu)->rd_lock); >> + base = gic_data_rdist_cpu(cpu)->rd_base + SZ_128K; >> + writel_relaxed(vpe->vpe_id, base + GICR_VSGIR); >> + do { >> + status = readl_relaxed(base + GICR_VSGIPENDR); >> + if (!(status & GICR_VSGIPENDR_BUSY)) >> + goto out; >> + >> + count--; >> + if (!count) { >> + pr_err_ratelimited("Unable to get SGI status\n"); >> + goto out; >> + } >> + cpu_relax(); >> + udelay(1); >> + } while(count); >> + >> +out: >> + raw_spin_unlock(&gic_data_rdist_cpu(cpu)->rd_lock); >> + vpe_to_cpuid_unlock(vpe, flags); >> + *val = !!(status & (1 << d->hwirq)); >> + >> + return 0; > cascade an error on timeout? Sure, that's a good idea. Thanks, M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm 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=-7.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,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 78DD4C4332E for ; Thu, 19 Mar 2020 10:27:51 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 4C9A320740 for ; Thu, 19 Mar 2020 10:27:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="lUEYnt4c"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="RkCD8EU7" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4C9A320740 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+infradead-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=bombadil.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:References:In-Reply-To:Subject:To:From: Date:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=r9Bie0ySxce5SIAs0z03HpD4raYxm6A6zdI04BJos/M=; b=lUEYnt4chZuQfd6h2v6Tk/b2V ZB4l1417OqQ5qvY0OERKsk7azV/sSWXpA1kpfeCHErQ6b8/HQ82iTslw30lcw9fihbp53vfyE+h30 UwbdNVMy5rHvixTUwsAnZbLvU9hSYs80/pkn6VLdhCqVMHzerfCjU4BV+fLaxVPHgkKFoWymoFb7k nNUlSer5b98S68Uz7pCKYkEN+42sXwxN1wxY+gfhONvzegSkGh/xIRmtXBAb+9Pq955hRLAs71AFD IQ6kBfOaJRJrY8Nq45wbfFb85BYyJnBLrxOPKHR0aG18w7a44ZDeioYGXjxmi6X9r66lP0FS/+KYv 3ONBvIZ6Q==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jEsP0-0007AX-1s; Thu, 19 Mar 2020 10:27:50 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jEsOx-0007AB-D9 for linux-arm-kernel@lists.infradead.org; Thu, 19 Mar 2020 10:27:48 +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 F38A5206D7; Thu, 19 Mar 2020 10:27:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1584613667; bh=uwoazedwTNP0+xIsOawzejd5iSeA0BwzwkyqJeruVqE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=RkCD8EU7JV2FdgiasHUyA76NEFCRtSZNlIezGQCI0JeR1C3P0bTgIZMV/jJEofEz+ alTAqUt2mV0EkoXDUnrrrSOBx8vcrswnCMUqqAdt4g0BWvgKRVRVA31AjrB/YcAAS1 w8HvzmpttbnMQBxq0GkQgopeKhARA/Lh/HsH5ql8= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1jEsOu-00Dua1-W4; Thu, 19 Mar 2020 10:27:45 +0000 MIME-Version: 1.0 Date: Thu, 19 Mar 2020 10:27:44 +0000 From: Marc Zyngier To: Auger Eric Subject: Re: [PATCH v5 11/23] irqchip/gic-v4.1: Plumb get/set_irqchip_state SGI callbacks In-Reply-To: References: <20200304203330.4967-1-maz@kernel.org> <20200304203330.4967-12-maz@kernel.org> Message-ID: <6b2fec7cdc53536997edf4db971e1d47@kernel.org> X-Sender: maz@kernel.org User-Agent: Roundcube Webmail/1.3.10 X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: eric.auger@redhat.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, lorenzo.pieralisi@arm.com, jason@lakedaemon.net, rrichter@marvell.com, tglx@linutronix.de, yuzenghui@huawei.com, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com 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-20200319_032747_483948_E654EABE X-CRM114-Status: GOOD ( 20.10 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lorenzo Pieralisi , Jason Cooper , kvm@vger.kernel.org, Suzuki K Poulose , linux-kernel@vger.kernel.org, Robert Richter , James Morse , Julien Thierry , Zenghui Yu , Thomas Gleixner , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Eric, On 2020-03-16 21:43, Auger Eric wrote: > Hi Marc, > > On 3/4/20 9:33 PM, Marc Zyngier wrote: >> To implement the get/set_irqchip_state callbacks (limited to the >> PENDING state), we have to use a particular set of hacks: >> >> - Reading the pending state is done by using a pair of new >> redistributor >> registers (GICR_VSGIR, GICR_VSGIPENDR), which allow the 16 >> interrupts >> state to be retrieved. >> - Setting the pending state is done by generating it as we'd otherwise >> do >> for a guest (writing to GITS_SGIR). >> - Clearing the pending state is done by emiting a VSGI command with >> the > emitting >> "clear" bit set. >> >> This requires some interesting locking though: >> - When talking to the redistributor, we must make sure that the VPE >> affinity doesn't change, hence taking the VPE lock. >> - At the same time, we must ensure that nobody accesses the same >> redistributor's GICR_VSGI*R registers for a different VPE, which > GICR_VSGIR >> would corrupt the reading of the pending bits. We thus take the >> per-RD spinlock. Much fun. >> >> Signed-off-by: Marc Zyngier >> --- >> drivers/irqchip/irq-gic-v3-its.c | 73 >> ++++++++++++++++++++++++++++++ >> include/linux/irqchip/arm-gic-v3.h | 14 ++++++ >> 2 files changed, 87 insertions(+) >> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c >> b/drivers/irqchip/irq-gic-v3-its.c >> index c93f178914ee..fb2b836c31ff 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -3962,11 +3962,84 @@ static int its_sgi_set_affinity(struct >> irq_data *d, >> return -EINVAL; >> } >> >> +static int its_sgi_set_irqchip_state(struct irq_data *d, >> + enum irqchip_irq_state which, >> + bool state) >> +{ >> + if (which != IRQCHIP_STATE_PENDING) >> + return -EINVAL; >> + >> + if (state) { >> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d); >> + struct its_node *its = find_4_1_its(); >> + u64 val; >> + >> + val = FIELD_PREP(GITS_SGIR_VPEID, vpe->vpe_id); >> + val |= FIELD_PREP(GITS_SGIR_VINTID, d->hwirq); >> + writeq_relaxed(val, its->sgir_base + GITS_SGIR - SZ_128K); >> + } else { >> + its_configure_sgi(d, true); >> + } >> + >> + return 0; >> +} >> + >> +static int its_sgi_get_irqchip_state(struct irq_data *d, >> + enum irqchip_irq_state which, bool *val) >> +{ >> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d); >> + void __iomem *base; >> + unsigned long flags; >> + u32 count = 1000000; /* 1s! */ > where does it come from? I did not find any info in the spec about this > delay. There is no such thing in the spec. However, these timeouts have proven to be very useful in detecting broken HW (I'm lucky enough to have such wonders at hand...), as well as SW bugs. The ! second value is purely empirical (if a whole second is not enough for things to move, they're as good as dead). >> + u32 status; >> + int cpu; >> + >> + if (which != IRQCHIP_STATE_PENDING) >> + return -EINVAL; >> + >> + /* >> + * Locking galore! We can race against two different events: >> + * >> + * - Concurent vPE affinity change: we must make sure it cannot >> + * happen, or we'll talk to the wrong redistributor. This >> is >> + * identical to what happens with vLPIs. >> + * >> + * - Concurrent VSGIPENDR access: As it involves accessing two >> + * MMIO registers, this must be made atomic one way or >> another. >> + */ >> + cpu = vpe_to_cpuid_lock(vpe, &flags); >> + raw_spin_lock(&gic_data_rdist_cpu(cpu)->rd_lock); >> + base = gic_data_rdist_cpu(cpu)->rd_base + SZ_128K; >> + writel_relaxed(vpe->vpe_id, base + GICR_VSGIR); >> + do { >> + status = readl_relaxed(base + GICR_VSGIPENDR); >> + if (!(status & GICR_VSGIPENDR_BUSY)) >> + goto out; >> + >> + count--; >> + if (!count) { >> + pr_err_ratelimited("Unable to get SGI status\n"); >> + goto out; >> + } >> + cpu_relax(); >> + udelay(1); >> + } while(count); >> + >> +out: >> + raw_spin_unlock(&gic_data_rdist_cpu(cpu)->rd_lock); >> + vpe_to_cpuid_unlock(vpe, flags); >> + *val = !!(status & (1 << d->hwirq)); >> + >> + return 0; > cascade an error on timeout? Sure, that's a good idea. Thanks, M. -- Jazz is not dead. It just smells funny... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel 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=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, 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 ED6B1C4332B for ; Thu, 19 Mar 2020 10:27:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C3ECA20781 for ; Thu, 19 Mar 2020 10:27:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1584613671; bh=uwoazedwTNP0+xIsOawzejd5iSeA0BwzwkyqJeruVqE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=jrsd14TEG/HNbk+UZRUZ6zLIeBMo/k1Gv+VAomgXO4oXOtR5RL2uywLQt29T4dIfX tlMrqCD2D0HGcP5lYGDJCNHLvJryB4fSO8KrnbfF58guN2e/UWoVbTGrPH2e+wm9ni oxRkbdZyHF+zh0HVmpXw+wxHXHcDsO8Oml2hyZVU= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727067AbgCSK1s (ORCPT ); Thu, 19 Mar 2020 06:27:48 -0400 Received: from mail.kernel.org ([198.145.29.99]:52050 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726785AbgCSK1s (ORCPT ); Thu, 19 Mar 2020 06:27:48 -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 F38A5206D7; Thu, 19 Mar 2020 10:27:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1584613667; bh=uwoazedwTNP0+xIsOawzejd5iSeA0BwzwkyqJeruVqE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=RkCD8EU7JV2FdgiasHUyA76NEFCRtSZNlIezGQCI0JeR1C3P0bTgIZMV/jJEofEz+ alTAqUt2mV0EkoXDUnrrrSOBx8vcrswnCMUqqAdt4g0BWvgKRVRVA31AjrB/YcAAS1 w8HvzmpttbnMQBxq0GkQgopeKhARA/Lh/HsH5ql8= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1jEsOu-00Dua1-W4; Thu, 19 Mar 2020 10:27:45 +0000 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Thu, 19 Mar 2020 10:27:44 +0000 From: Marc Zyngier To: Auger Eric Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, Lorenzo Pieralisi , Jason Cooper , Robert Richter , Thomas Gleixner , Zenghui Yu , James Morse , Julien Thierry , Suzuki K Poulose Subject: Re: [PATCH v5 11/23] irqchip/gic-v4.1: Plumb get/set_irqchip_state SGI callbacks In-Reply-To: References: <20200304203330.4967-1-maz@kernel.org> <20200304203330.4967-12-maz@kernel.org> Message-ID: <6b2fec7cdc53536997edf4db971e1d47@kernel.org> X-Sender: maz@kernel.org User-Agent: Roundcube Webmail/1.3.10 X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: eric.auger@redhat.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, lorenzo.pieralisi@arm.com, jason@lakedaemon.net, rrichter@marvell.com, tglx@linutronix.de, yuzenghui@huawei.com, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Eric, On 2020-03-16 21:43, Auger Eric wrote: > Hi Marc, > > On 3/4/20 9:33 PM, Marc Zyngier wrote: >> To implement the get/set_irqchip_state callbacks (limited to the >> PENDING state), we have to use a particular set of hacks: >> >> - Reading the pending state is done by using a pair of new >> redistributor >> registers (GICR_VSGIR, GICR_VSGIPENDR), which allow the 16 >> interrupts >> state to be retrieved. >> - Setting the pending state is done by generating it as we'd otherwise >> do >> for a guest (writing to GITS_SGIR). >> - Clearing the pending state is done by emiting a VSGI command with >> the > emitting >> "clear" bit set. >> >> This requires some interesting locking though: >> - When talking to the redistributor, we must make sure that the VPE >> affinity doesn't change, hence taking the VPE lock. >> - At the same time, we must ensure that nobody accesses the same >> redistributor's GICR_VSGI*R registers for a different VPE, which > GICR_VSGIR >> would corrupt the reading of the pending bits. We thus take the >> per-RD spinlock. Much fun. >> >> Signed-off-by: Marc Zyngier >> --- >> drivers/irqchip/irq-gic-v3-its.c | 73 >> ++++++++++++++++++++++++++++++ >> include/linux/irqchip/arm-gic-v3.h | 14 ++++++ >> 2 files changed, 87 insertions(+) >> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c >> b/drivers/irqchip/irq-gic-v3-its.c >> index c93f178914ee..fb2b836c31ff 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -3962,11 +3962,84 @@ static int its_sgi_set_affinity(struct >> irq_data *d, >> return -EINVAL; >> } >> >> +static int its_sgi_set_irqchip_state(struct irq_data *d, >> + enum irqchip_irq_state which, >> + bool state) >> +{ >> + if (which != IRQCHIP_STATE_PENDING) >> + return -EINVAL; >> + >> + if (state) { >> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d); >> + struct its_node *its = find_4_1_its(); >> + u64 val; >> + >> + val = FIELD_PREP(GITS_SGIR_VPEID, vpe->vpe_id); >> + val |= FIELD_PREP(GITS_SGIR_VINTID, d->hwirq); >> + writeq_relaxed(val, its->sgir_base + GITS_SGIR - SZ_128K); >> + } else { >> + its_configure_sgi(d, true); >> + } >> + >> + return 0; >> +} >> + >> +static int its_sgi_get_irqchip_state(struct irq_data *d, >> + enum irqchip_irq_state which, bool *val) >> +{ >> + struct its_vpe *vpe = irq_data_get_irq_chip_data(d); >> + void __iomem *base; >> + unsigned long flags; >> + u32 count = 1000000; /* 1s! */ > where does it come from? I did not find any info in the spec about this > delay. There is no such thing in the spec. However, these timeouts have proven to be very useful in detecting broken HW (I'm lucky enough to have such wonders at hand...), as well as SW bugs. The ! second value is purely empirical (if a whole second is not enough for things to move, they're as good as dead). >> + u32 status; >> + int cpu; >> + >> + if (which != IRQCHIP_STATE_PENDING) >> + return -EINVAL; >> + >> + /* >> + * Locking galore! We can race against two different events: >> + * >> + * - Concurent vPE affinity change: we must make sure it cannot >> + * happen, or we'll talk to the wrong redistributor. This >> is >> + * identical to what happens with vLPIs. >> + * >> + * - Concurrent VSGIPENDR access: As it involves accessing two >> + * MMIO registers, this must be made atomic one way or >> another. >> + */ >> + cpu = vpe_to_cpuid_lock(vpe, &flags); >> + raw_spin_lock(&gic_data_rdist_cpu(cpu)->rd_lock); >> + base = gic_data_rdist_cpu(cpu)->rd_base + SZ_128K; >> + writel_relaxed(vpe->vpe_id, base + GICR_VSGIR); >> + do { >> + status = readl_relaxed(base + GICR_VSGIPENDR); >> + if (!(status & GICR_VSGIPENDR_BUSY)) >> + goto out; >> + >> + count--; >> + if (!count) { >> + pr_err_ratelimited("Unable to get SGI status\n"); >> + goto out; >> + } >> + cpu_relax(); >> + udelay(1); >> + } while(count); >> + >> +out: >> + raw_spin_unlock(&gic_data_rdist_cpu(cpu)->rd_lock); >> + vpe_to_cpuid_unlock(vpe, flags); >> + *val = !!(status & (1 << d->hwirq)); >> + >> + return 0; > cascade an error on timeout? Sure, that's a good idea. Thanks, M. -- Jazz is not dead. It just smells funny...