From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id VgVPKt2Rt2pEuwYAWB0awg (envelope-from ) for ; Sat, 26 Sep 2026 05:35:25 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=ibm.com header.i=@ibm.com header.a=rsa-sha256 header.s=pp1 header.b=ewJyNXTm; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 958091E033; Sat, 26 Sep 2026 05:35:25 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-5.3 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,HTML_MESSAGE,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [IPv6:2620:52:6:3111::32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 060C01E033 for ; Sat, 26 Sep 2026 05:35:24 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AC3844BB58ED for ; Sat, 26 Sep 2026 09:35:21 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AC3844BB58ED Authentication-Results: sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=ibm.com header.i=@ibm.com header.a=rsa-sha256 header.s=pp1 header.b=ewJyNXTm Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id C86BD4BB3BAE for ; Sat, 26 Sep 2026 09:34:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C86BD4BB3BAE Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C86BD4BB3BAE Authentication-Results: sourceware.org; arc=none smtp.remote-ip=148.163.158.5 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1790415293; cv=none; b=BwRUVrFwMlx9Q9/iy4KQaKxvbntE91w36/OvOR872JQCyaJZ0mBl16U4P51o408H+akSxSdNfDrhgRb/QHpCUPRnO7Q4C4igK/FqmzUSRb1EUmF/MSaXEnSDbMUZGjxeJ9EPENi7tN1CYt9uCgfRNglWLCbEn6jJOE+D5jaavzY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1790415293; c=relaxed/simple; bh=hR5o+CdkMUrp161c199A7XI4PjlR+MdrjFZ6Wo1eDqQ=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=ALoAaWJsFh9px/J7pXB/cO8WMsaKQjqUt38FSBwhsJtUajGe9NWynoQwPP9OJSTIkWRbRF6P4ZvVZ97aJFw+ZOsDVRm54W7jTK6oNsqgLvjT7ecGP+LvAFWvTe3Wc0L1aShJ+BM2xu/SbPJXaF+dkMrxfgXC+1fvXgRNfWgvduo= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (2048-bit key, unprotected) header.d=ibm.com header.i=@ibm.com header.a=rsa-sha256 header.s=pp1 header.b=ewJyNXTm DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C86BD4BB3BAE Received: from pps.filterd (m0353725.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id 68Q6a0lg2031530 for ; Sat, 26 Sep 2026 09:34:51 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=cc :content-type:date:from:in-reply-to:message-id:mime-version :references:subject:to; s=pp1; bh=bgEcqZjb9RbLOAcx7Slv6usojRQyzw pDDj1eI31n8ms=; b=ewJyNXTmZ7iWq/SUaRmeyRVapzvl7OE/QlqTafhC2DaRrF Dg49Qnc1vzgHZ3xg+g1EW+1wowmZOdE4khN84RKiXT4wvDaHl5RFMthTEA1+4d/t LzxtbCLdcA47brg+ZNpqU5Y1OJS83JGW9k/B1G1NkhT/t+Wjtj7wDByI2i2ZHYTy /lLkozTELmpMzXHmCw/JNjJBBxmxWWesoQIVKVqlca/BiQP59DXOHrfa4uchyzxR ZG2ogBXfV3eVIlHiXbv6KaT7X7AvaC4uBjTdrJSbvdfFgAs7R5Mq8tfKsyKODiG3 mUMJuZfmAc75MI4GaWpQGxbtZwRbzsSsWRNDXjKg== Received: from ppma22.wdc07v.mail.ibm.com (5c.69.3da9.ip4.static.sl-reverse.com [169.61.105.92]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 4gx4fds2p1-1 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT) for ; Sat, 26 Sep 2026 09:34:51 +0000 (GMT) Received: from pps.filterd (ppma22.wdc07v.mail.ibm.com [127.0.0.1]) by ppma22.wdc07v.mail.ibm.com (8.18.1.11/8.18.1.11) with ESMTP id 68Q6WeIG880579 for ; Sat, 26 Sep 2026 09:34:51 GMT Received: from smtprelay06.wdc07v.mail.ibm.com ([172.16.1.73]) by ppma22.wdc07v.mail.ibm.com (PPS) with ESMTPS id 4gvbu95qxk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Sat, 26 Sep 2026 09:34:51 +0000 (GMT) Received: from smtpav05.wdc07v.mail.ibm.com (smtpav05.wdc07v.mail.ibm.com [10.39.53.232]) by smtprelay06.wdc07v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 68Q9YlKd27853394 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Sat, 26 Sep 2026 09:34:48 GMT Received: from smtpav05.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E13B0580CC; Sat, 26 Sep 2026 09:34:47 +0000 (GMT) Received: from smtpav05.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 9FCAC580CF; Sat, 26 Sep 2026 09:34:45 +0000 (GMT) Received: from [9.111.132.219] (unknown [9.111.132.219]) by smtpav05.wdc07v.mail.ibm.com (Postfix) with ESMTP; Sat, 26 Sep 2026 09:34:45 +0000 (GMT) Content-Type: multipart/alternative; boundary="------------jBhyrQJSxSvPpPE9sHPmJ0KE" Message-ID: <687032cb-729a-437e-b1cc-0fa01772e0ac@linux.ibm.com> Date: Sat, 26 Sep 2026 15:04:42 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1] PowerPC: Add support for Dense Math registers (RFC02653) To: Ulrich Weigand , "gdb-patches@sourceware.org" Cc: "cel@linux.ibm.com" , Abhay Kandpal References: <20260924063013.288753-1-abhay@linux.ibm.com> <4b8f1c34708d1b5373966c7995f53a5e7f1d548f.camel@de.ibm.com> From: Abhay Kandpal Content-Language: en-GB In-Reply-To: <4b8f1c34708d1b5373966c7995f53a5e7f1d548f.camel@de.ibm.com> X-TM-AS-GCONF: 00 X-Authority-Analysis: v=2.4 cv=FYWiV5+6 c=1 sm=1 tr=0 ts=6ab791bb cx=c_pps a=5BHTudwdYE3Te8bg5FgnPg==:117 a=5BHTudwdYE3Te8bg5FgnPg==:17 a=VdqzKS8jKosA:10 a=VkNPw1HP01LnGYTKEx00:22 a=RnoormkPH1_aCDwRdu11:22 a=V8glGbnc2Ofi9Qvn3v5h:22 a=r77TgQKjGQsHNAKrUKIA:9 a=VnNF1IyMAAAA:8 a=90F9bexvJ6Wg_EdeNj8A:9 a=3ZKOabzyN94A:10 a=QEXdDO2ut3YA:10 a=IUQYIl3xDchs3za-K-gA:9 a=uO-OeOHBoYQ_-tTI:21 a=_W_S_7VecoQA:10 a=lqcHg5cX4UMA:10 X-Proofpoint-Spam-Info: AW1haW4tMjYwOTI2MDAzNiBTYWx0ZWRfX9sD/rCLXNhCt dcvp9CG5B2clQyEOWFeaXUL6Vb6xLVYhp85VVYzlmt/uQbJHudJPFakCI6nDHY/4/j3wtzukuIE 89vKmwUJDe3nBf1Z+V/t9d0nK1XtvQM= X-Proofpoint-ORIG-GUID: Cb2BxAuGbtYouneuQpoV90Yabk4njlt_ X-Proofpoint-Spam-Details-Enc: AW1haW4tMjYwOTI2MDAzNiBTYWx0ZWRfXy2D9dpCgFxBc a67k+6DFQAa6iRNw6MZz/13KaSEJMV/yfFSm4JHaoqTlHxvjKaEpQC45tavTPBdwJkXTMNKzqQf 2eYEGMp5DJce7se1nxnIvInL0oVyJsXzmOHEFA5o6dEFXFNoZ0Z0pY2zcH8kNXLRyc9CWGql30+ +zomJFQSjlVV11Cu/gfc/G4+eRdP2CJwl53FlQ5te8Ztog0ym543Y8UP4U4LUAeQ1ChXC8vBwHX MarCYnJ4/MZKr2mWWfqmCC9hWUFwB1tWhXJMOe2bCGjSh8/9aIJ0pWJ+fZX3vdM8YnuNzTURRo8 EyOewi3wJpXPm38vhRfmf/6NvmGZPm/qm4lRTHJgua1rpPAPFa9yeQdVFfU1qjsUWo2HD00ahtP Ns0oHsCMI5p0cZdRfNGNBz+9plhX8cogHZWxeMhv88Eu53HE6yPSTkTe5w0x2FH9NHhk3zErSq1 K6dWexjx6gkD3J9/qEw== X-Proofpoint-GUID: Cb2BxAuGbtYouneuQpoV90Yabk4njlt_ X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1176,Hydra:6.1.134,FMLib:17.12.100.49 definitions=2026-09-26_02,2026-09-21_02,2025-10-01_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 clxscore=1015 impostorscore=0 spamscore=0 phishscore=0 priorityscore=1501 malwarescore=0 bulkscore=0 lowpriorityscore=0 suspectscore=0 classifier=typeunknown authscore=0 authtc= authcc= route=outbound adjust=0 reason=mlx scancount=1 engine=8.22.0-2609040000 definitions=main-2609260036 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org This is a multi-part message in MIME format. --------------jBhyrQJSxSvPpPE9sHPmJ0KE Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi Ulrich, Thanks very much for the review. Responses below. On 25/09/26 17:35, Ulrich Weigand wrote: > Abhay Kandpal wrote: > >> A new target description (powerpc-dmr-vsx32l/64l) is introduced for >> systems where DMR registers are available. The existing isa207 target >> description is left unchanged so that systems without DMR support are >> not affected. DMR availability is detected at runtime by probing the >> kernel with PTRACE_GETDMREGS. On kernels without DMR support the >> registers are gracefully skipped and show as unavailable. > This looks good in general. One question about naming: most recent > features have names that identify the base Power ISA level (isa205, > isa207, etc.). Should we follow this precedent for the new target > description name? The existing names encode the base ISA level, but DMR is a later ISA feature layered on an isa207-derived description, so|isa207-dmr| would misstate the ISA level. A few options: |1. powerpc-isa207-dmr-vsx64l| - names the base feature set the description extends |2. powerpc-isa32-dmr-vsx64l| - names DMR's own ISA level (ISA 3.2) |3. powerpc-dmr-vsx64l| - as posted, no ISA level > > A few other comments below. > > >>    if (features.wordsize == 8) >>      { >> +      /* HTM was disabled on POWER9 and later hardware following the >> + transactional-memory erratum, so it does not coexist with the >> + Dense Math facility introduced on later processors.  The htm >> + and has_dmr cases below are therefore mutually exclusive in >> + practice.  */ >>        if (features.vsx) >>   tdesc = (features.htm ? tdesc_powerpc_isa207_htm_vsx64l >> + : features.has_dmr ? tdesc_powerpc_dmr_vsx64l > Even so, it would be preferable to add the newest feature first. Will do. The comment then becomes unnecessary. > > >> @@ -58,6 +59,7 @@ struct ppc_linux_features >>    bool ppr_dscr; >>    bool isa207; >>    bool htm; >> +  bool has_dmr; > For this struct, precedent is not to use the "has_" prefix. Will rename to dmr. > > >> +  >> +  > "uint1024" is a surprising name for a vector type. I would have > expected something like "v8uint128" ? Agreed. I'll rename it and regenerate the description files. > > >> @@ -159,6 +159,11 @@ >>  #define NT_PPC_TM_CDSCR 0x10f >>  #endif >> >> +#ifndef PTRACE_GETDMREGS >> +#define PTRACE_GETDMREGS 0x1f >> +#define PTRACE_SETDMREGS 0x20 >> +#endif > Please move this block higher up, together with the > other PTRACE_ defines in this file. > > That said, it would have been preferable to use the > regset mechanism like for all other recently added > registers, rather than a completely new PTRACE_ call. > I see in the gdbserver you actually do that! So please > use the same method in gdb itself as well. The dedicated calls were used on the native side because that is what the kernel team pointed tooling at for DMR, but I agree the inconsistency between gdb and gdbserver isn't justified. In the kernel,|PTRACE_GETDMREGS| is a thin wrapper around|copy_regset_to_user()| on the same regset that |NT_PPC_DMR| reaches, and I've confirmed|PTRACE_GETREGSET| with |NT_PPC_DMR| works on the enabled kernel. So in v2 I'll convert the native side to the regset mechanism throughout:|fetch_regset|/|store_regset| with|NT_PPC_DMR| in |fetch_register|,|fetch_ppc_registers|,|store_register| and |store_ppc_registers|, and a regset-based availability check in |read_description|. That removes the need for the|PTRACE_GETDMREGS|/|PTRACE_SETDMREGS| defines, so I'll drop the block rather than move it - unless something still needs them, in which case I'll relocate it as you suggest. > > >> +  /* Check whether the kernel supports Dense Math registers.  Native >> +     GDB accesses DMR through the dedicated PTRACE_GETDMREGS > request.  */ >> +  { >> +    gdb_byte buf[PPC_LINUX_SIZEOF_DMRREGSET]; >> +    if (ptrace (PTRACE_GETDMREGS, tid, 0, buf) >= 0) >> +      features.has_dmr = true; >> +  } > Again it would be preferable to use a regset check instead. > > Is there a HWCAP bit we should check in addition, to indicate that > the kernel and hypervisor support context-swapping these registers? Yes -|PPC_FEATURE2_DMF| (0x00008000, Dense Math Facility). I'll add that check. The kernel does save and restore these registers across context switches. The hwcap bit and the DMR ptrace support are currently in separate kernel branches here, so I'm assembling a tree with both before validating v2. > > >> +    /* Dense Math registers.  */ >> +    int have_dmr = 0; >> +    /* Register number of dmr0, or -1 if DMR is not available.  Set >> +       from PPC_DMR0_REGNUM when the target provides the dmr feature. >> +       Provided for register-offset computations (e.g. core file and >> +       pseudo-register handling) added in follow-up DMR work.  */ >> +    int ppc_dmr0_regnum = 0; > It seems odd to have *both* a have_ flag and a regnum. > Usually, where we have regnum field, all tests are written > as "if (tdep->ppc_dmr0_regnum != -1)" instead. Agreed. I'll drop|have_dmr| and test|tdep->ppc_dmr0_regnum != -1| instead. > > >> +  /* Check whether the kernel supports Dense Math registers.  Native >> +     GDB accesses DMR through the dedicated PTRACE_GETDMREGS > request.  */ >> +  { >> +    gdb_byte buf[PPC_LINUX_SIZEOF_DMRREGSET]; >> +    if (ptrace (PTRACE_GETDMREGS, tid, 0, buf) >= 0) >> +      features.has_dmr = true; >> +  } > Why this mix of new PTRACE_ and regsets? Covered above - I'll move the native side to regsets so there's no mix. Regards, Abhay > > > Bye, > Ulrich --------------jBhyrQJSxSvPpPE9sHPmJ0KE Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit 
Hi Ulrich,
Thanks very much for the review. Responses below.
On 25/09/26 17:35, Ulrich Weigand wrote:
Abhay Kandpal <abhay@linux.ibm.com> wrote:


A new target description (powerpc-dmr-vsx32l/64l) is introduced for
systems where DMR registers are available. The existing isa207 target
description is left unchanged so that systems without DMR support are
not affected. DMR availability is detected at runtime by probing the
kernel with PTRACE_GETDMREGS. On kernels without DMR support the
registers are gracefully skipped and show as unavailable.

This looks good in general.  One question about naming: most recent
features have names that identify the base Power ISA level (isa205,
isa207, etc.).   Should we follow this precedent for the new target
description name?
The existing names encode the base ISA level, but DMR is a later ISA
feature layered on an isa207-derived description, so isa207-dmr would 
misstate the ISA level. A few options:

1. powerpc-isa207-dmr-vsx64l - names the base feature set the description extends
2. powerpc-isa32-dmr-vsx64l - names DMR's own ISA level (ISA 3.2)
3. powerpc-dmr-vsx64l - as posted, no ISA level

A few other comments below.



   if (features.wordsize == 8)
     {
+      /* HTM was disabled on POWER9 and later hardware following the
+	 transactional-memory erratum, so it does not coexist with the
+	 Dense Math facility introduced on later processors.  The htm
+	 and has_dmr cases below are therefore mutually exclusive in
+	 practice.  */
       if (features.vsx)
 	tdesc = (features.htm ? tdesc_powerpc_isa207_htm_vsx64l
+		 : features.has_dmr ? tdesc_powerpc_dmr_vsx64l

Even so, it would be preferable to add the newest feature first.
Will do. The comment then becomes unnecessary.



@@ -58,6 +59,7 @@ struct ppc_linux_features
   bool ppr_dscr;
   bool isa207;
   bool htm;
+  bool has_dmr;

For this struct, precedent is not to use the "has_" prefix.
Will rename to dmr.



+  <!-- Define one 1024-bit vector composed of eight 128-bit lanes -->
+  <vector id="uint1024" type="uint128" count="8"/>

"uint1024" is a surprising name for a vector type.  I would have
expected something like "v8uint128" ?
Agreed. I'll rename it and regenerate the description files.



@@ -159,6 +159,11 @@
 #define NT_PPC_TM_CDSCR 0x10f
 #endif
 
+#ifndef PTRACE_GETDMREGS
+#define PTRACE_GETDMREGS 0x1f
+#define PTRACE_SETDMREGS 0x20
+#endif

Please move this block higher up, together with the
other PTRACE_ defines in this file.

That said, it would have been preferable to use the
regset mechanism like for all other recently added
registers, rather than a completely new PTRACE_ call.
I see in the gdbserver you actually do that!  So please
use the same method in gdb itself as well.
The dedicated calls were used on the native side because
that is what the kernel team pointed tooling at for DMR,
but I agree the inconsistency between gdb and gdbserver 
isn't justified. In the kernel, PTRACE_GETDMREGS is a thin
wrapper around copy_regset_to_user() on the same regset that
NT_PPC_DMR reaches, and I've confirmed PTRACE_GETREGSET with
NT_PPC_DMR works on the enabled kernel.
So in v2 I'll convert the native side to the regset mechanism
throughout: fetch_regset/store_regset with NT_PPC_DMR in
fetch_register, fetch_ppc_registers, store_register and
store_ppc_registers, and a regset-based availability check in
read_description. That removes the need for the 
PTRACE_GETDMREGS/PTRACE_SETDMREGS defines, so I'll drop the 
block rather than move it - unless something still needs them,
in which case I'll relocate it as you suggest.



+  /* Check whether the kernel supports Dense Math registers.  Native
+     GDB accesses DMR through the dedicated PTRACE_GETDMREGS

request.  */

+  {
+    gdb_byte buf[PPC_LINUX_SIZEOF_DMRREGSET];
+    if (ptrace (PTRACE_GETDMREGS, tid, 0, buf) >= 0)
+      features.has_dmr = true;
+  }

Again it would be preferable to use a regset check instead.

Is there a HWCAP bit we should check in addition, to indicate that
the kernel and hypervisor support context-swapping these registers?
Yes - PPC_FEATURE2_DMF (0x00008000, Dense Math Facility). I'll add that
check. The kernel does save and restore these registers across context 
switches. The hwcap bit and the DMR ptrace support are currently in 
separate kernel branches here, so I'm assembling a tree with both before
validating v2.



+    /* Dense Math registers.  */
+    int have_dmr = 0;
+    /* Register number of dmr0, or -1 if DMR is not available.  Set
+       from PPC_DMR0_REGNUM when the target provides the dmr feature.
+       Provided for register-offset computations (e.g. core file and
+       pseudo-register handling) added in follow-up DMR work.  */
+    int ppc_dmr0_regnum = 0;

It seems odd to have *both* a have_ flag and a regnum.
Usually, where we have regnum field, all tests are written
as "if (tdep->ppc_dmr0_regnum != -1)" instead.
Agreed. I'll drop have_dmr and test tdep->ppc_dmr0_regnum != -1 instead.



+  /* Check whether the kernel supports Dense Math registers.  Native
+     GDB accesses DMR through the dedicated PTRACE_GETDMREGS

request.  */

+  {
+    gdb_byte buf[PPC_LINUX_SIZEOF_DMRREGSET];
+    if (ptrace (PTRACE_GETDMREGS, tid, 0, buf) >= 0)
+      features.has_dmr = true;
+  }

Why this mix of new PTRACE_ and regsets?
Covered above - I'll move the native side to regsets so there's no mix.

Regards,
Abhay


Bye,
Ulrich
--------------jBhyrQJSxSvPpPE9sHPmJ0KE--