From mboxrd@z Thu Jan 1 00:00:00 1970 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 smtp.subspace.kernel.org (Postfix) with ESMTPS id DE75A427A1A for ; Tue, 28 Jul 2026 14:20:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.92.199 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785248416; cv=none; b=jv/slOnymvPaJf3ReovuxK73u2i6mLEa06oraBzvi7xa790b/mmF7Yc+Ki3V8o+/ZTBQ1sxW35s5uct4Ky5suFbLQ+xmxT/PHWkA59EMwRlywj/A+0/4WleV/K8PyIo/g+kAqjAmfiLN+MeWGCqt1y6NtLExo212B7TS2LNO0+c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785248416; c=relaxed/simple; bh=j6jDJd85cvWXAXzs4ZJdaFyoaLZ/kvrpIg/r7otZ8nw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lWpKz79a5F72yVMj+2vk9kRfor4J68HbRHHiDZKMk8F61aIB3BYnTjhRtcMKKn0FOxh/d4ul32AWVxhGtHCEXevkwr2AwVSYN6IsIX6vvRZBe9icPAvE0ZIFSo4L3LjV9VcNjL/V4SRUFzHTMjyPzwcH74lpnPyf96q55LmWxlY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=pass smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=G1v2FUtu; arc=none smtp.client-ip=90.155.92.199 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="G1v2FUtu" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=fhlajuHyoPf54wxfOla7I4/BG+IlIRWbYYSGKFish8g=; b=G1v2FUtuHLemnMjYcdQbqS+W+b UBIxWfD8YWMkPme2QDY770d+1X2pSHPsLhlPdf6v3JYRbNw8lubyqIgXBP1iqcJzLMmBP9vkLuUf5 Vn0kM1ZmONru8EEF8n98h8O5blwDFwQEvtCAXzxmoXb2Q3bN10CEV4s6Yg5Qs9M7UWEJb+PPDUPh7 wUr+6qabWa7UEPpf/tXEvsv/xnwq5jricWGs9kQfOG5Mjthv5leaDGImUFOCfAy7VOJe0jOuXldqf 5DJ1Lcuha9/xZLeVNZ8m+2V6B+u1vsf9zfqh+6lG5AUHYGoK9NAvBOz8H9L7ybtC4mhX6MJ0q4q3c 3k5aa7kQ==; Received: from 77-249-17-252.cable.dynamic.v4.ziggo.nl ([77.249.17.252] helo=noisy.programming.kicks-ass.net) by desiato.infradead.org with esmtpsa (Exim 4.99.2 #2 (Red Hat Linux)) id 1woif1-000000050fK-2lPx; Tue, 28 Jul 2026 14:19:59 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 58358300969; Tue, 28 Jul 2026 16:19:58 +0200 (CEST) Date: Tue, 28 Jul 2026 16:19:58 +0200 From: Peter Zijlstra To: Mark Rutland Cc: linux-arm-kernel@lists.infradead.org, ada.coupriediaz@arm.com, ardb@kernel.org, catalin.marinas@arm.com, hca@linux.ibm.com, linux-kernel@vger.kernel.org, maz@kernel.org, ruanjinjie@huawei.com, vladimir.murzin@arm.com, will@kernel.org, yang@os.amperecomputing.com Subject: Re: [RFC PATCH 06/13] arm64: percpu: Add infrastructure for preemptible this_cpu_*() ops Message-ID: <20260728141958.GV751831@noisy.programming.kicks-ass.net> References: <20260728123859.2911495-1-mark.rutland@arm.com> <20260728123859.2911495-7-mark.rutland@arm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260728123859.2911495-7-mark.rutland@arm.com> On Tue, Jul 28, 2026 at 01:38:52PM +0100, Mark Rutland wrote: > With the scheme added in this patch, this can be compiled as: > > | : > | mrs x2, sp_el0 > | mov x4, #0xc80 Perhaps add a comment to that like: | mov x4, #0xc80 # {x0, x4, x3} to make the literal less magical? > | strh w4, [x2, #20] Anyway, interrupt can hit here, at which point it will: x4 := this_cpu_offset x3 := x0 + x4 > | mrs x4, tpidr_el1 > | add x3, x0, x4 Which it then recomputes, and so is harmless. > | 1: ldxr x6, [x3] > | add x6, x6, x1 > | stxr w5, x6, [x3] IIRC any interrupt in between LL/SC will cause the stxr to fail anyway, irrespective of the fixup, so goto 1b we go. > | cbnz w5, 1b If we get here and interrupts happens, we recompute pointlessly, no harm done. Anyway, per this sequence there is no point in ever doing the fixup. So perhaps give a better example? > | strh wzr, [x2, #20] > | ret > > TODO: Save/restore the PCPU GPRs in __sdei_asm_handler(). This will > require some mechanical rework to the __sdei_asm_handler() assembly. > > Signed-off-by: Mark Rutland > @@ -51,6 +53,47 @@ static inline unsigned long __kern_my_cpu_offset(void) > return off; > } > > +#define PCPU_GPR_PCP GENMASK(4, 0) > +#define PCPU_GPR_OFF GENMASK(9, 5) > +#define PCPU_GPR_ADDR GENMASK(14, 10) I hate those macros, I can never remember what they all do. Now, esp. since you then not use them here: > +#define __VAL_PCPU_GPRS(pcp, off, addr) \ > + "(" \ > + "(.L__gpr_num_" pcp " << 0) | " \ > + "(.L__gpr_num_" off " << 5) | " \ > + "(.L__gpr_num_" addr " << 10)" \ > + ")" ... continue comment at irqentry_exit_pcpu_adjust(). > +#define ____PCPU_GPRS_BEGIN(gprs, pcp, off, addr) \ > + __DEFINE_ASM_GPR_NUMS \ > + __DEFINE_ASM_GPR_ALIASES \ > + " mov w" off ", #" __VAL_PCPU_GPRS(pcp, off, addr) "\n" \ > + " strh w" off ", " gprs "\n" \ > + __KERN_ASM_CPU_OFFSET(off) "\n" Can this macro also generate a readable comment for those few of us building the .i file ? > +/* > + * Where the context being returned to had an active percpu GPR critical > + * section, ensure that the offset and address GPRs are updated to match the > + * current CPU. > + * > + * For simplicity we always update the GPRs when a critical section is active > + * and preemption was *possible*, regardless of whether preemption actually > + * occurred. Where preemption did not occur, the updates are redundant but not > + * harmful. > + */ > +static __always_inline void irqentry_exit_pcpu_adjust(struct pt_regs *regs) > +{ > + int reg_pcp, reg_off, reg_addr; > + unsigned long pcp, off, addr; > + u16 gprs = regs->pcpu_gprs; > + > + /* > + * Zero means no active PCPU GPRs. As the PCPU GPRs must be distinct, > + * a PCPU critical section cannot possibly use {x0,x0,x0}. > + */ > + if (likely(!gprs)) > + return; > + > + reg_pcp = FIELD_GET(PCPU_GPR_PCP, gprs); > + reg_off = FIELD_GET(PCPU_GPR_OFF, gprs); > + reg_addr = FIELD_GET(PCPU_GPR_ADDR, gprs); Would it not be more readable to write this like: reg_pcp = (grps >> 0) & 0x1f; reg_off = (grps >> 5) & 0x1f; reg_addr = (grps >> 10) & 0x1f; To better match __VAL_PCPU_GRPS() ? > + pcp = pt_regs_read_reg(regs, reg_pcp); > + > + off = __kern_my_cpu_offset(); > + pt_regs_write_reg(regs, reg_off, off); > + > + addr = pcp + off; > + pt_regs_write_reg(regs, reg_addr, addr); > +}