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 7B8621B810 for ; Fri, 16 Feb 2024 09:11:07 +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=1708074667; cv=none; b=LdImBNkP6Gwa9U648/1Z7UpZpRb3x7Kpz5T0RHLOr638BZNgtueZ1zkpkfrAJvTlzuGVz4fg7i5eQoKvHBuOVVAu00Rq7hF29MnOOEf/PLHumVNA/6VJPwRgzEkbT+AOhuwc/47PGyzv9eCy8Q7hLPmedyxjitBgng4XKEpgy0E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708074667; c=relaxed/simple; bh=QgtH4bIBgu4fS0Ue3Be6a0VMQOCIjHMpzSUvoQDq7tA=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=qDhjvDAV/fUSOMBJ9Jj054RNHXrZeF/buoe8ynTtjfzGAk1Kg+d7QAh8EI21y8BZURgP80lAJy/KjUtLzg9wDury4FamoN6G0NHZ2sEWloxpmmqAdWe4siHrs5yUhTJRNFzMmOuJ4BbKT2zsbia1rXBSq3reZBSwHeSxZzaGVRk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=cLbYEou5; 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="cLbYEou5" Received: by smtp.kernel.org (Postfix) with ESMTPSA id EB62BC433F1; Fri, 16 Feb 2024 09:11:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1708074667; bh=QgtH4bIBgu4fS0Ue3Be6a0VMQOCIjHMpzSUvoQDq7tA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=cLbYEou5g+bVS+9EvhbfiAdpe/HCKmRkW2bDfwmNf5U1IO6C4K0MaxMhezfWIHkzA AypHGmSIMCmlPhNbutSUsnIy/3RKjen1+lxi4luK1lSje2AR9BSGhUFtp4jbWb1owT OVcirVRFCtzsgj2dqYSe10xNXELkBS9egWYe8xL6q4D+IBLr9fim85M5k9dxUHpBDX Jr94YDgT3nJBC8JsYGOVW9TPljsNjVhTgot506zJGei+EFxokMiHLhg9fHyIQ8M1cg SP45Ty5Ih0+PpJtL74wieBZ4KLpkuZJ3YuY38IDu/pBRMaPwohwUnvrov5oKTY+49y 0y7BNN1hQQUHA== 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 1rauFM-003mA4-5n; Fri, 16 Feb 2024 09:11:04 +0000 Date: Fri, 16 Feb 2024 09:11:03 +0000 Message-ID: <86il2o4vvc.wl-maz@kernel.org> From: Marc Zyngier To: Yoshihiro Shimoda Cc: "catalin.marinas@arm.com" , "will@kernel.org" , "tglx@linutronix.de" , "linux-arm-kernel@lists.infradead.org" , "linux-renesas-soc@vger.kernel.org" , Geert Uytterhoeven Subject: Re: [PATCH] irqchip/gic-v3-its: Workaround Renesas R-Car GICv3 ITS In-Reply-To: References: <20240214052050.1966439-1-yoshihiro.shimoda.uh@renesas.com> <865xyr5n3s.wl-maz@kernel.org> 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: linux-renesas-soc@vger.kernel.org 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: yoshihiro.shimoda.uh@renesas.com, catalin.marinas@arm.com, will@kernel.org, tglx@linutronix.de, linux-arm-kernel@lists.infradead.org, linux-renesas-soc@vger.kernel.org, geert@linux-m68k.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Fri, 16 Feb 2024 08:47:04 +0000, Yoshihiro Shimoda wrote: > > Hi Marc, > > Thank you for your review! > > > > @@ -2201,7 +2202,7 @@ static struct page *its_allocate_prop_table(gfp_t gfp_flags) > > > { > > > struct page *prop_page; > > > > > > - prop_page = alloc_pages(gfp_flags, get_order(LPI_PROPBASE_SZ)); > > > + prop_page = alloc_pages(gfp_flags | its_gfp_quirk, get_order(LPI_PROPBASE_SZ)); > > > > This only affects the redistributors. Why should we constraint it? > > To be honest, I don't know why, but this is required on my environment. > Otherwise, the MSI interrupt never happens. > Anyway, I should have learned the hardware bug in detail... So the redistributors are also affected by this bug, which makes sense given the monolithic nature of GIC600. This should be handled as a separate allocation constraints (i.e. not using your ITS-specific GFP quirk). [...] > > > @@ -2971,7 +2972,7 @@ static struct page *its_allocate_pending_table(gfp_t gfp_flags) > > > { > > > struct page *pend_page; > > > > > > - pend_page = alloc_pages(gfp_flags | __GFP_ZERO, > > > + pend_page = alloc_pages(gfp_flags | __GFP_ZERO | its_gfp_quirk, > > > get_order(LPI_PENDBASE_SZ)); > > > > This is a redistributor-private allocation. If it is the ITSs that are > > affected by this bug, why are the pending tables constrained? > > Thank you for the comment. This is not needed on my environment. > So, I'll drop it. If GICR_PROPBASER needs to be in the low 4GB, it is likely that GICR_PENDBASER has the same constraint. It is just that the GIC600 caches are big enough that evictions are rare, and that you don't see a problem. But it is still very likely to exist. [...] > > I suggest that you start reading the architecture spec and understand > > what is the memory that is accessible by the GIC, because at least > > half of this patch is completely unnecessary. > > > > You also need to be clear about what is affected by this bug: ITS > > only? or also the RDs? If both are affected, they need to be handled > > separately. > > > > In any case, you must not assume that all ITSs in a system are > > subjected to this bug, which means that you must have per-ITS tracking > > of the GFP flags. > > > > Finally, the DMA story itself needs to be sorted out, because you are > > relying on something that is incredibly fragile. > > Thank you very much for your suggestion. I'll learn the architecture spec > and reconsider the implementation, especially DMA related. Using the DMA allocator is an interesting prospect. The only issue with that is that the ITS isn't currently represented as a device, which the DMA allocator requires IIRC. You will either have to plumb something in a low-level allocator, or convert the ITS code to implement a platform device. The latter won't be fun. Thanks, M. -- Without deviation from the norm, progress is not possible.