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,URIBL_BLOCKED autolearn=ham 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 34DF6CA9ECF for ; Fri, 1 Nov 2019 13:26:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0936F2067D for ; Fri, 1 Nov 2019 13:26:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1572614806; bh=rqtvRstnFB/aFpNsK1qvt8/Qu32GiuMfuYACmi92Ik8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=kWPsCkw1Z/9EL4kDXhtC3LLM+R4Tz1bkUKKvwj14JrCUOQrkZfPPa5tZuoSt8ffkO VtSMuZJWAgQvFnHnXQt1qsUweX2PSQNk5g/9TuAGJ29YLXWweRhbvOijxXH3VSvLIl WfZowQkWejz06AkRZ2zKET7fhZ6OnCl+IHCq5Ahk= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727162AbfKAN0o (ORCPT ); Fri, 1 Nov 2019 09:26:44 -0400 Received: from inca-roads.misterjones.org ([213.251.177.50]:38312 "EHLO inca-roads.misterjones.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726860AbfKAN0o (ORCPT ); Fri, 1 Nov 2019 09:26:44 -0400 Received: from [91.217.168.176] (helo=big-swifty.misterjones.org) by cheepnis.misterjones.org with esmtpsa (TLSv1.2:AES256-GCM-SHA384:256) (Exim 4.80) (envelope-from ) id 1iQWwo-0006n0-LS; Fri, 01 Nov 2019 14:26:38 +0100 Date: Fri, 01 Nov 2019 13:26:37 +0000 Message-ID: <86ftj7ybg2.wl-maz@kernel.org> From: Marc Zyngier To: Zenghui Yu Cc: , , Eric Auger , James Morse , Julien Thierry , Suzuki K Poulose , Thomas Gleixner , Jason Cooper , Lorenzo Pieralisi , "Andrew Murray" , Jayachandran C , "Robert Richter" Subject: Re: [PATCH v2 03/36] irqchip/gic-v3-its: Allow LPI invalidation via the DirectLPI interface In-Reply-To: References: <20191027144234.8395-1-maz@kernel.org> <20191027144234.8395-4-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 EasyPG/1.0.0 Emacs/26 (aarch64-unknown-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: 91.217.168.176 X-SA-Exim-Rcpt-To: yuzenghui@huawei.com, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org, eric.auger@redhat.com, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, tglx@linutronix.de, jason@lakedaemon.net, lorenzo.pieralisi@arm.com, Andrew.Murray@arm.com, jnair@marvell.com, rrichter@marvell.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on cheepnis.misterjones.org); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 31 Oct 2019 08:49:32 +0000, Zenghui Yu wrote: > > Hi Marc, > > On 2019/10/27 22:42, Marc Zyngier wrote: > > We currently don't make much use of the DirectLPI feature, and it would > > be beneficial to do this more, if only because it becomes a mandatory > > feature for GICv4.1. > > > > Signed-off-by: Marc Zyngier > > I have no objection to this patch, which says: > > Reviewed-by: Zenghui Yu > > > But this patch really drives me to look through all callsites of > dev_event_to_col(), the abstraction which can be used _only_ with > physical LPI mappings. > > I find that when building the INV command, we use dev_event_to_col() > to find the "sync_obj" and then pass it to the following SYNC command. > But the "INV+SYNC" will be performed both on physical LPI and *VLPI* > (lpi_update_config/its_send_inv). > So I have two questions about the way we sending INV on VLPI: > > 1) Which "sync" command should be followed? SYNC or VSYNC? > (we currently use SYNC, while the spec says, SYNC "ensures all > outstanding ITS operations associated with *physical* interrupts > for the Redistributor are globally observed ...") > > 2) The "sync_obj" we are currently using seems to be wrong. I think you're right on both counts (where were you when I wrote the initial GICv4 support? ;-). I think the confusion stems from the fact that there is no 'VINV' command, and we simply overlooked the sync object issue. It is quite likely that existing implementations don't care much about the difference (otherwise we'd have seen the problem before), but it doesn't hurt to do the right thing. I have the following patch as part of a set of fixes that I'm about to post (once I get a chance to test them), let me know what you think. M. diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index a47ed2ba2907..75ab3716a870 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -702,6 +702,24 @@ static struct its_vpe *its_build_vmovp_cmd(struct its_node *its, return valid_vpe(its, desc->its_vmovp_cmd.vpe); } +static struct its_vpe *its_build_vinv_cmd(struct its_node *its, + struct its_cmd_block *cmd, + struct its_cmd_desc *desc) +{ + struct its_vlpi_map *map; + + map = dev_event_to_vlpi_map(desc->its_inv_cmd.dev, + desc->its_inv_cmd.event_id); + + its_encode_cmd(cmd, GITS_CMD_INV); + its_encode_devid(cmd, desc->its_inv_cmd.dev->device_id); + its_encode_event_id(cmd, desc->its_inv_cmd.event_id); + + its_fixup_cmd(cmd); + + return valid_vpe(its, map->vpe); +} + static u64 its_cmd_ptr_to_offset(struct its_node *its, struct its_cmd_block *ptr) { @@ -1068,6 +1086,20 @@ static void its_send_vinvall(struct its_node *its, struct its_vpe *vpe) its_send_single_vcommand(its, its_build_vinvall_cmd, &desc); } +static void its_send_vinv(struct its_device *dev, u32 event_id) +{ + struct its_cmd_desc desc; + + /* + * There is no real VINV command. This is just a normal INV, + * with a VSYNC instead of a SYNC. + */ + desc.its_inv_cmd.dev = dev; + desc.its_inv_cmd.event_id = event_id; + + its_send_single_vcommand(dev->its, its_build_vinv_cmd, &desc); +} + /* * irqchip functions - assumes MSI, mostly. */ @@ -1142,8 +1174,10 @@ static void lpi_update_config(struct irq_data *d, u8 clr, u8 set) lpi_write_config(d, clr, set); if (gic_rdists->has_direct_lpi && !irqd_is_forwarded_to_vcpu(d)) direct_lpi_inv(d); - else + else if (!irqd_is_forwarded_to_vcpu(d)) its_send_inv(its_dev, its_get_event_id(d)); + else + its_send_vinv(its_dev, its_get_event_id(d)); } static void its_vlpi_set_doorbell(struct irq_data *d, bool enable) -- Jazz is not dead, it just smells funny.