From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 1F32C3858001 for ; Thu, 25 Mar 2021 17:21:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1F32C3858001 Received: from pps.filterd (m0098396.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 12PH4uVD189474 for ; Thu, 25 Mar 2021 13:21:46 -0400 Received: from ppma05wdc.us.ibm.com (1b.90.2fa9.ip4.static.sl-reverse.com [169.47.144.27]) by mx0a-001b2d01.pphosted.com with ESMTP id 37gw5jb6yh-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 25 Mar 2021 13:21:46 -0400 Received: from pps.filterd (ppma05wdc.us.ibm.com [127.0.0.1]) by ppma05wdc.us.ibm.com (8.16.0.43/8.16.0.43) with SMTP id 12PHJoP1009663 for ; Thu, 25 Mar 2021 17:21:45 GMT Received: from b01cxnp22034.gho.pok.ibm.com (b01cxnp22034.gho.pok.ibm.com [9.57.198.24]) by ppma05wdc.us.ibm.com with ESMTP id 37dycd1cyf-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Thu, 25 Mar 2021 17:21:45 +0000 Received: from b01ledav002.gho.pok.ibm.com (b01ledav002.gho.pok.ibm.com [9.57.199.107]) by b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 12PHLhDu17039826 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 25 Mar 2021 17:21:43 GMT Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 955D2124054; Thu, 25 Mar 2021 17:21:43 +0000 (GMT) Received: from b01ledav002.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 1ABC9124052; Thu, 25 Mar 2021 17:21:43 +0000 (GMT) Received: from lexx (unknown [9.211.48.66]) by b01ledav002.gho.pok.ibm.com (Postfix) with ESMTP; Thu, 25 Mar 2021 17:21:42 +0000 (GMT) Message-ID: <02a8afbd89092a7b34f5bc0e9b13404e6cdc2233.camel@vnet.ibm.com> Subject: Re: [PATCH] gdb-power10-single-step From: will schmidt To: Ulrich Weigand Cc: gdb-patches@sourceware.org Date: Thu, 25 Mar 2021 12:21:42 -0500 In-Reply-To: <20210310175015.GA4142@oc3748833570.ibm.com> References: <983294f95974a6a3572d31b077f0ca66de554655.camel@vnet.ibm.com> <20210310175015.GA4142@oc3748833570.ibm.com> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-10.el7) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.369, 18.0.761 definitions=2021-03-25_05:2021-03-24, 2021-03-25 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 mlxscore=0 bulkscore=0 spamscore=0 lowpriorityscore=0 phishscore=0 priorityscore=1501 mlxlogscore=999 suspectscore=0 clxscore=1015 malwarescore=0 impostorscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2103250124 X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Mar 2021 17:21:48 -0000 On Wed, 2021-03-10 at 18:50 +0100, Ulrich Weigand wrote: > Will Schmidt wrote: > > > This is a patch written by Alan Modra. With his permission > > I'm submitting this for review and helping get this upstream. > > > > Powerpc / Power10 ISA 3.1 adds prefixed instructions, which > > are 8 bytes in length. This is in contrast to powerpc previously > > always having 4 byte instruction length. This patch implements > > changes to allow GDB to better detect prefixed instructions, and > > handle single stepping across the 8 byte instructions. > > There's a few issues I see here: I've dug in a bit more,.. have a few questions related to the patch and the comments here. I've got a refreshed version of this patch in my queue, with a nit or two that i'm still trying to understand and squash before I post it. > > - The patch now *forces* software single-stepping for all 8-byte > instructions. I'm not sure why this is necessary; I thought > that hardware single-stepping was supported for 8-byte instructions > as well? That would certainly be preferable. Does software single-stepping go hand-in-hand with executing the instructions from a displaced location? I only see one clear return-if-prefixed change in the patch, so I am assuming the above refers to the patch chunk seen as : > @@ -1081,6 +1090,10 @@ ppc_deal_with_atomic_sequence (struct regcache *regcache) > const int atomic_sequence_length = 16; /* Instruction sequence length. */ > int bc_insn_count = 0; /* Conditional branch instruction count. */ > > + /* Power10 prefix instructions are two words in length. */ > + if ((insn & OP_MASK) == 1 << 26) > + return { pc + 8 }; I've got a local change to eliminate that return. Per the poking I've done so far, none of the prefix instructions I've run against so far allow us past the is_load_and_reserve instruction check. > if (!IS_LOAD_AND_RESERVE_INSN (insn)) > return {}; statement, so not a significant code flow behavior change. > - However, the inner loop of ppc_deal_with_atomic_sequence should > probably be updated to correctly skip 8-byte instructions; e.g. > to avoid mistakenly recognizing the second word of an 8-byte > instructions for a branch or store conditional. (Also, the > count of up to "16 instructions" is wrong if 8-byte instructions > are not handled specifically.) I've got a local change to inspect the instruction and determine if it is prefixed, so I think i've got this handled. I'm generally assuming we will never start halfway through an 8-byte prefixed instruction. > - In ppc_displaced_step_fixup 8-byte instructions are now recognized, > and this is good as far as it goes. However, there are some 8-byte > instruction that have PC-relative semantics, and those will need > an additional fixup. (Note that this same fixup is already missing > for the Power9 addpcis instruction, see bug 27525). There should be a patch on-list to start to address this. Thanks, -Will > > Bye, > Ulrich >