From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4579E12D776 for ; Thu, 7 Mar 2024 13:50:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709819421; cv=none; b=DTKPAp+2gXOZX5QIA1uBxW/9Ge+4TbzXa7uF4cWVJP48FraZD2Y0p6/fazFZTkmDIZykxjOSQATv+dv4n6dgnG2MJyQv7UG7r0Nx6G1zULd2TQdZzdvb2KBBkAzkdwGLqugFVnu7YHSVPU5pyPbtZJbOg8BwqgolpdTAlNFlFeI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709819421; c=relaxed/simple; bh=HiqIhTVJ214R9Vb2kkfXbavmR7C6iP6xiPmPUyFhWis=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=cgdPWWQR+WbkSC6KinuHlTIQI3JD42gmNjr0xCLdgLeACOIzb3EJJEeAE3WAu7SSQOOOd2pMlvDsRR38HWIHaKYKSMV9RD1LTVnjsOVnkQSnMl/wUkG9QHvj+N6P18j8EGdEFYX21QRxgFDWjkgL6pdAry6QdcdxKD2BZDUz9A4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=o0iSnY3p; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="o0iSnY3p" Received: by smtp.kernel.org (Postfix) with ESMTPSA id C20EBC433C7; Thu, 7 Mar 2024 13:50:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1709819420; bh=HiqIhTVJ214R9Vb2kkfXbavmR7C6iP6xiPmPUyFhWis=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=o0iSnY3pz/mnQEslFaxfUOzGEfm+zOF1o5xpNc6YAwm+3VcKH8NshshD2X3XGZIKr 0ouAs+4l6RyybNDol3OgmVLD8FAgXIPGGhZcbaN8HDz7gW/iYy1nz9RknzhEyhfVJz fOLDOlW8gh/iU9AclT5mLT2zjCnJiDFvQ3zOydrg+KP916gjM+X1rjxK+Ne3PC6X8h RxEWAwB+QRHDYONxx+6ub+HNYCynDTCQN/TI3IEK2uKrO18WxiGAJzU4aGqR4kdUS0 9cryFdF6k5rJCRHd58PDAu+5EL1DBq7HGxqRpDCVxbmY0GtWjRBG/hknLskr2jPlky bwZRJgsQXUDNg== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.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 1riE8X-00AK3C-UP; Thu, 07 Mar 2024 13:50:18 +0000 Date: Thu, 07 Mar 2024 13:50:17 +0000 Message-ID: <86le6u178m.wl-maz@kernel.org> From: Marc Zyngier To: Zenghui Yu Cc: Oliver Upton , , James Morse , Suzuki K Poulose , Eric Auger Subject: Re: [PATCH] KVM: arm64: vgic-v3: Don't load pending state when enabling LPIs on RD In-Reply-To: <453f7cef-2b53-ccc6-40d2-6e147353f220@huawei.com> References: <20240228000117.2297982-1-oliver.upton@linux.dev> <86o7bq1hhd.wl-maz@kernel.org> <453f7cef-2b53-ccc6-40d2-6e147353f220@huawei.com> 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/29.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Precedence: bulk X-Mailing-List: kvmarm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: yuzenghui@huawei.com, oliver.upton@linux.dev, kvmarm@lists.linux.dev, james.morse@arm.com, suzuki.poulose@arm.com, eric.auger@redhat.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Thu, 07 Mar 2024 12:13:43 +0000, Zenghui Yu wrote: > > Hi Marc, > > On 2024/3/7 18:09, Marc Zyngier wrote: > > Hi Zenghui, > > > > On Thu, 07 Mar 2024 09:00:42 +0000, > > Zenghui Yu wrote: > >> > >> So I've tested it with kvm-unit-tests and got failure with the > >> its-pending-migration case: > >> > >> INFO: gicv3: its-pending-migration: Migration complete > >> INFO: gicv3: its-pending-migration: expected 128 LPIs on PE #30, 0 observed > >> FAIL: gicv3: its-pending-migration: 128 LPIs on both PE0 and PE1 after > >> migration > >> SUMMARY: 1 tests, 1 unexpected failures > >> > >> where the guest SW directly writes to the pending table when > >> GICR_CTLR.EnableLPIs == 0. I seriously doubt there is any use case like > >> that in real world. But not sure whether it is a funky behaviour from > >> the architectural perspective. > > > > Right, so this is *exactly* the thing I was worried about. A mapping > > has been established, the interrupt wasn't pending, all good. Now, an > > interrupt lands while GICR_CTLR.EnableLPIs == 0. > > > > The spec says (4.7.3 "Effect of disabling interrupts") > > > > "When GICR_CTLR.EnableLPIs == 0, LPIs are never set pending." > > > > which to me is a pretty damning indication that we shouldn't take > > these bits into account. > > Ah, thanks for pointing it out! And I believe this is the rationale of > Oliver's patch. > > What confused me is that the spec also says (5.1.2 "LPI Pending tables") > > | "For physical LPIs, when GICR_CTLR.EnableLPIs is changed to 1, the > | Redistributor must read the pending status of the physical LPIs from > | the physical LPI Pending table." > > which implicitly indicates that the pending table can contain some > pending bits (=1) when EnableLPIs == 0, which would be loaded by RD and > make the relevant LPIs pending when EnableLPIs is written from 0 to 1. > > I have no idea how to interpret these rules ;-) . Yeah, even after all this time, the GICv3 spec never fails to entertain. My take on this is that it is there to support the different ways to build a GICv3: - either RDs are completely independent of the ITSs, and only deal with the forwarding of enabled interrupts. In this case, setting EnableLPIs to 1 would naturally lead to interrupt being forwarded to the CPU. New bits are not allowed to be made pending while it is set to 0 though, which matches 4.7.3. - or RDs and ITSs are pretty monolithic, and the GIC infrastructure as a whole maintain the pending state as a result of a translation, not as a set of bits in the pending tables. In this case, EnableLPIs gates the translation process and there are no new pending bits being produced when it is set to 0. We can easily satisfy 5.1.2 by not having created new pending bits. So the two rules are more or less correct. They just are saying that it is IMPDEF what happens in this case, which is pretty damming for an interrupt controller architecture. I feel dirty just having to think of these things! Thanks, M. -- Without deviation from the norm, progress is not possible.