From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14979 invoked by alias); 23 Nov 2017 20:26:11 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 14965 invoked by uid 89); 23 Nov 2017 20:26:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=BAYES_00,KB_WAM_FROM_NAME_SINGLEWORD,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=this! X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Nov 2017 20:26:09 +0000 Received: from pps.filterd (m0098417.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id vANKOQmt054501 for ; Thu, 23 Nov 2017 15:26:07 -0500 Received: from e06smtp14.uk.ibm.com (e06smtp14.uk.ibm.com [195.75.94.110]) by mx0a-001b2d01.pphosted.com with ESMTP id 2ee1fc1brt-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 23 Nov 2017 15:26:07 -0500 Received: from localhost by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 23 Nov 2017 20:26:05 -0000 Received: from b06cxnps4074.portsmouth.uk.ibm.com (9.149.109.196) by e06smtp14.uk.ibm.com (192.168.101.144) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 23 Nov 2017 20:26:04 -0000 Received: from d06av26.portsmouth.uk.ibm.com (d06av26.portsmouth.uk.ibm.com [9.149.105.62]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id vANKQ3CR41877604; Thu, 23 Nov 2017 20:26:03 GMT Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id A0A26AE04D; Thu, 23 Nov 2017 20:19:17 +0000 (GMT) Received: from d06av26.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 8558BAE045; Thu, 23 Nov 2017 20:19:17 +0000 (GMT) Received: from oc3748833570.ibm.com (unknown [9.164.187.199]) by d06av26.portsmouth.uk.ibm.com (Postfix) with ESMTP; Thu, 23 Nov 2017 20:19:17 +0000 (GMT) Received: by oc3748833570.ibm.com (Postfix, from userid 1000) id 2FDEDD803CD; Thu, 23 Nov 2017 21:26:03 +0100 (CET) Subject: Re: [PATCH 1/4] Split up s390-linux-tdep.c into two files To: prudo@linux.vnet.ibm.com (Philipp Rudo) Date: Thu, 23 Nov 2017 20:26:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org, arnez@linux.vnet.ibm.com (Andreas Arnez) In-Reply-To: <20171123153733.31261-2-prudo@linux.vnet.ibm.com> from "Philipp Rudo" at Nov 23, 2017 04:37:30 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 17112320-0016-0000-0000-00000504E7B3 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17112320-0017-0000-0000-00002840BE24 Message-Id: <20171123202603.2FDEDD803CD@oc3748833570.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-11-23_08:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1711230276 X-SW-Source: 2017-11/txt/msg00578.txt.bz2 Philipp Rudo wrote: > Currently all target dependent code for s390 is in one file > (s390-linux-tdep.c). This includes code general for the architecture as > well as code specific for uses in GNU/Linux (user space). Up until now > this was ok as GNU/Linux was the only supported OS. In preparation to > support the new Linux kernel 'OS' split up the existing s390 code into a > general s390-tdep and a GNU/Linux specific s390-linux-tdep. Thanks for working on this! A couple of comments on the particular split: > -static void > -s390_write_pc (struct regcache *regcache, CORE_ADDR pc) > -{ > - struct gdbarch *gdbarch = regcache->arch (); > - struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); > - > - regcache_cooked_write_unsigned (regcache, tdep->pc_regnum, pc); > - > - /* Set special SYSTEM_CALL register to 0 to prevent the kernel from > - messing with the PC we just installed, if we happen to be within > - an interrupted system call that the kernel wants to restart. > - > - Note that after we return from the dummy call, the SYSTEM_CALL and > - ORIG_R2 registers will be automatically restored, and the kernel > - continues to restart the system call at this point. */ > - if (register_size (gdbarch, S390_SYSTEM_CALL_REGNUM) > 0) > - regcache_cooked_write_unsigned (regcache, S390_SYSTEM_CALL_REGNUM, 0); > -} This seems a typical Linux issue. Other OSes won't have the ORIG_R2 and SYSTEM_CALL registers and may not need this particular handling. > -/* Maps for register sets. */ > - > -static const struct regcache_map_entry s390_gregmap[] = > - { > - { 1, S390_PSWM_REGNUM }, > - { 1, S390_PSWA_REGNUM }, > - { 16, S390_R0_REGNUM }, > - { 16, S390_A0_REGNUM }, > - { 1, S390_ORIG_R2_REGNUM }, > - { 0 } > - }; > - > -static const struct regcache_map_entry s390_fpregmap[] = > - { > - { 1, S390_FPC_REGNUM, 8 }, > - { 16, S390_F0_REGNUM, 8 }, > - { 0 } > - }; > - > static const struct regcache_map_entry s390_regmap_upper[] = > { > { 16, S390_R0_UPPER_REGNUM, 4 }, Not sure I understand your split here. "ORIG_R2" is not a real register and only exists on Linux, so it shouldn't move to s390-tdep.c. On the other hand, the split between upper halves and lower halves seems more generic and could happen elsewhere ... > +/* Set up GNU/Linux gdbarch. Allocates struct gdbarch if needed. */ > > static struct gdbarch * > -s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > +s390_linux_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > { This is wrong. The way the split setup between generic part and Linux part is supposed to be working is this: - The generic part (and only it) has a _initialize routine that calls register_gdbarch_init (bfd_arch_s390, s390_gdbarch_init); - In the s390_gdbarch_init routine, the generic setup is done, and at the point where the platform-specific setup should begin, we call gdbarch_init_osabi (info, gdbarch); - The OS-specific part also has a _initialize routine, which calls gdbarch_register_osabi (bfd_arch_s390, bfd_mach_s390_31, GDB_OSABI_LINUX, s390_linux_init_abi); gdbarch_register_osabi (bfd_arch_s390, bfd_mach_s390_64, GDB_OSABI_LINUX, s390_linux_init_abi); (may be the same or two different handlers for 31- vs 64-bit, whatever is more practical) - The common gdbarch_init_osabi detects which OS handler to use and calls it. This method is same across all Linux (and really all) targets; there is no point for us to try to do it differently, that would just cause confusion. > - /* Optional GNU/Linux-specific "registers". */ > - feature = tdesc_find_feature (tdesc, "org.gnu.gdb.s390.linux"); > - if (feature) > - { > - tdesc_numbered_register (feature, tdesc_data, > - S390_ORIG_R2_REGNUM, "orig_r2"); > - > - if (tdesc_numbered_register (feature, tdesc_data, > - S390_LAST_BREAK_REGNUM, "last_break")) > - have_linux_v1 = 1; > - > - if (tdesc_numbered_register (feature, tdesc_data, > - S390_SYSTEM_CALL_REGNUM, "system_call")) > - have_linux_v2 = 1; > - > - if (have_linux_v2 > have_linux_v1) > - valid_p = 0; > - } The Linux specific feature should be handled here, the others in the generic part. > -/* GNU/Linux-specific optional registers. */ > -#define S390_ORIG_R2_REGNUM 67 > -#define S390_LAST_BREAK_REGNUM 68 > -#define S390_SYSTEM_CALL_REGNUM 69 Ideally, these should also remain here as Linux specific registers. This can be done e.g. like ppc-linux-tdep.h does it. Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain Ulrich.Weigand@de.ibm.com