From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01on2045.outbound.protection.outlook.com [40.107.15.45]) by sourceware.org (Postfix) with ESMTPS id 44D68384E397 for ; Mon, 28 Nov 2022 12:06:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 44D68384E397 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=D230ZKuQG/dZyks9Agb6KxFTXW7xAnd1gFHf2qiDA1BR8h3oeBXxlddvA0ENrbL3Evvp3Ux/M1dIqpEGuk1CJfLkn0HnsnhvNwE49VRwzJlYkMX+YUr5YN5tSEj3IVJa9429WzHTP4WiW/9V9SAh0p9dEWzjqlK0hCEjccq05fs1soZc1rplR0LIY10hpkvwFq14X1Rj1fSJGymA8mW4PYvgvw8OIJuoTmb/st+KDZGnzyNhx4bLZWy4lUAj7/UrpeGyBg5xzSYL5eMUJx55wnJ+GtAGKK2jNoftTDFyVCwdXwEwcTv1dtZBeF6Ei7Ph27DH6qEMThBoso/1gArU/w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=SNlzr6mDx+VeP1Ft1bmhbKY2Lijtlnwv6nYKfSiGGmQ=; b=JlKkVDgwbpSZ31uCp+gVw3lw5BoNC2peuo6L8rMHnU74cDnqVIMlaxi3CRHK1ppJIeXhJAJcr5vRYh/GpldRgQaetmbtsiNRVrwjtGxJLGiaXHyY1MyQbCmBTR36xc5Zkfdgm2RrCGcCmAaJeBFe+eH510FPqdII4zddVDvVjBp0g1B60qU19lYqQL5Wo2ozHDoyc4l9VGTZavrI51f48b45CHm4V/dDIMoMlsF/Qsmee/ZM8DcGv56KopqLHVa6lcMjl5+qV+y4HDV18JOH22XWFZywqUTcRbuYidPmt8uZjKuMwI+iCjUG3kaODjpT0Be8MtqS/a/Iw5ipXfIuHA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=SNlzr6mDx+VeP1Ft1bmhbKY2Lijtlnwv6nYKfSiGGmQ=; b=HszT1jloIWmaKeW7QuF4jpLs0SbqbLpVJ58OXcC8cYtJhVJ9V+kQnG+rrAgOkpa4NKNk/6+wP9391nhCsqDvbafEIq9L7BPtuDS/ARZXXrl0j2Tbsy5AWBEk1kK+HrcKkkWMvsrR/wOFvbyCuppM/udSY8UNw1cqrvw0/WMs8vg= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from VI1PR08MB3919.eurprd08.prod.outlook.com (2603:10a6:803:c4::31) by GV2PR08MB8003.eurprd08.prod.outlook.com (2603:10a6:150:af::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5880.8; Mon, 28 Nov 2022 12:06:48 +0000 Received: from VI1PR08MB3919.eurprd08.prod.outlook.com ([fe80::fe5c:b195:a2ad:b19c]) by VI1PR08MB3919.eurprd08.prod.outlook.com ([fe80::fe5c:b195:a2ad:b19c%4]) with mapi id 15.20.5880.008; Mon, 28 Nov 2022 12:06:47 +0000 Message-ID: Date: Mon, 28 Nov 2022 12:06:45 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH v2 4/6] gdbserver/linux-aarch64: When thread stops, update its target description Content-Language: en-US To: Thiago Jung Bauermann , gdb-patches@sourceware.org References: <20221126020452.1686509-1-thiago.bauermann@linaro.org> <20221126020452.1686509-5-thiago.bauermann@linaro.org> From: Luis Machado In-Reply-To: <20221126020452.1686509-5-thiago.bauermann@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: LO2P265CA0001.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:62::13) To VI1PR08MB3919.eurprd08.prod.outlook.com (2603:10a6:803:c4::31) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: VI1PR08MB3919:EE_|GV2PR08MB8003:EE_ X-MS-Office365-Filtering-Correlation-Id: 7dfb8a26-0b99-4f11-8deb-08dad1390617 NoDisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: b3o+K4VRQw+6fKdfUqBnLzS94smH/PmNTJbMHsD7QjtU2LTo9tLKmKkKPDwT5nCCQlEExUuNhwQHlVkBoNnihBdiCJitlve+z33p3AK/vbm8XhnLnUSGloO6MfbuYLuAFEkUja7pYhRdVW0DxYWw/XoQnPyET9z+QV5HDXTGy6ykpbqCL4DZSg1Cuqj1iJXIrAo7sarbgaWZBDpirS8gDipnqF9jDDr/xITTINZBwx6e3khGDEgd2C3lLGWY4Zz4Pbm0ZRT6N5b8T3uNNipMzYEKzxa2fT/qlTAtBq+6JNp/4MpUMxphYIU0OpAks1FWk57HJ3ZihM+AJZX2gxBHLdeGAziyYv1pa++yeYCu3CschrFwkXI9HY8CUnXKfNMTciyE69OnpiBUD8TWSCf6awI9PXl8Ax3Wg5noBvBintg0KjF0MeI0m1mA/Rf5/AXhDoZeOPwBzWiZbtdm7FWD+BAC5PC4IjLD9Fw9mD/DOxB9bY6mcucS95z9+kdaLQtJTEtyDLeotntLBKVCREeB7Kscyjeo7OhT5hBzjpqJAWYd0KJe0jAAGf24us8ZqjZ4QQugHfUVeKPhSSy5QBSL4qczGQ8QXO6HWc7lwkGth4qat7A+IiSwV9Peg2A/DvwaebgZlkvirrUQhLkgvfaH6+i1yXttE+7E3eSV8AjlriHuDE05STMDsiMPH6LF3NK/U5HpvBOmgCURJE4wuJOlm/V8v/uDEky4FyKZLLyFxwQ= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR08MB3919.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(4636009)(136003)(346002)(376002)(396003)(366004)(39860400002)(451199015)(53546011)(2906002)(6506007)(186003)(6486002)(6512007)(478600001)(36756003)(26005)(38100700002)(31696002)(86362001)(83380400001)(2616005)(41300700001)(31686004)(44832011)(66556008)(66476007)(8676002)(5660300002)(66946007)(8936002)(316002)(15650500001)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?S2ZsMWZaUDkxQ0Q0V2IyYkV0cnFKOTdTUjFQYUJDQ3hpRXpaYmNOTXlxT0dS?= =?utf-8?B?SXV2ai9DNzhPSEl3ZnZRWFIvVXI1M1BCc2xyWTA4WU9OK2RsSzh4RHNJT1pi?= =?utf-8?B?YmR0a0l5SkcxdGRBVzM0SmJDSUhsQzdmTjNMLzZ3Q1FwWFU1bWJzamhMQkhz?= =?utf-8?B?QTdvQjFQd2h6eUFXTHVCeWFVazVFSUhHSkV6TVVoSVpqK1pYSmVneFNPemVQ?= =?utf-8?B?NjZJZlVLd2NEd1hrT2ltTE85UzNqd2JCUS9xb3pmS3kvL3RFdkZURDdobmhS?= =?utf-8?B?cjN3K0U0Q2xQUDdocVpVbFo2ditwUmNsVWxoQjU1cUJPZU41ZmlUNkwyNXgx?= =?utf-8?B?NFhVZ0F0U2h1aC9vQ0NxVUNZQ3RIV0NwOHcxVllHcGtoaXJTbjRCWVQvNUt4?= =?utf-8?B?aWQyWEJCazR3bkNNeHBoMUN4dWNNVTF0cXFHeHpGcHVrcjhMczhwdk5VUStK?= =?utf-8?B?UUNBeVo2c2hMc2VlNXNBMHBtVGdTdUtHY0IxSzRDQlZtSit1SEJCYTI1MEUy?= =?utf-8?B?VXJlQXBPNTZJa29QNG5SNWhvZ2pyMlRBTnU3aVphNENuRXRGWVVjSUhnb1A4?= =?utf-8?B?cmFBWmFkRUVVQ0hhb3VJUDVHTUM1RUdTaGY4K2JFTG4wNU5kSWZSK0xZeUFM?= =?utf-8?B?LzJoM0lNUGZzRXY4cmV0NWdWSnYrdlhlVG5TbUthOUFBY3BvRktDNjlyTnFQ?= =?utf-8?B?czVrMEpIOU5KcU81Yno3QVJuUTI1V1pnNGNUTWQyclF5R2FBcXROWmxjSGR2?= =?utf-8?B?VEYyaEdXUmlxdXNkTEV3WnRNMVZ0MUFkU3prSkxucloxSkVCSnI0aEJhQkRZ?= =?utf-8?B?T3ZDbEtDVWVNR0RiYnBtVk4ySjd6M05aaENXVEdMNnExU1J0WnlhU29JRlBE?= =?utf-8?B?N2VQcmlkZTNYWER4UE55NHk4UXJ5WmFDTEY1T29vU3VyY3VNUC94blhvcDNT?= =?utf-8?B?czNOSWEybkFybUpGZ09ubURlM0UrbDlVK2pOMFZCVEVYU2pXKzN5Wm5sOFNL?= =?utf-8?B?RHdPRFY1L0xvS0lWd1NHRFNTemNDMDgraVVXMlB3T2krMzI1N3lnUGxIYjJn?= =?utf-8?B?RE5Fb05xRWxvbk5wTURZaTExYnRxUGpRa0htQU9acUI3L2NxMVA2WWlUd0VR?= =?utf-8?B?Y3k4Zmw1NUVJVDRqMEl5QXRWQUdXT3E1SUErcVFKUE43SFY5alhyeE0zRkRz?= =?utf-8?B?Ymw5S0JKTjd2NTAyVEQ0cmdRRjB1dnRyRjd2cW82UW50ck1DVTZYSlRmMFpE?= =?utf-8?B?aDJ1TnFCWk04M1hVZ3hBMEg3cjRJMFdaNWl2UDRHcDk4emRDc05lTmk0S2FN?= =?utf-8?B?UStuVEhjVDNaUWRiR0tZVEFFZXYzWUJnRnBzaXpEV2cveDJEdjhTQld1bE0r?= =?utf-8?B?dWxSTHZLRDNmOHFnOWFlVm1VT3JKVTZKZ2g4NUN5eWRIUFNmMUlBVThUdUQ4?= =?utf-8?B?bFp1bEIwazZrb3FLSmRsblZJVHduRjA4Y0daV1I4T2wxQjFpS2g5eFJLVXNZ?= =?utf-8?B?MnRQNlFIZ3BnT1BWcmcrZUdKRUkzcWhtVWFQWnBIaGduaHZhdk1FbWM0ZU1L?= =?utf-8?B?NWhhVUh4Q1QxKy96NTQ3UWhLdk9ySGJTKzRJUFdZaGxtbVVnMWhxcnVabjJt?= =?utf-8?B?eUdNUjlzTWFDdU9yY3MxamZKakUzRzMya29xV1BRaEIxbTYzZGNaUFZSd3VI?= =?utf-8?B?MmtIVUhNbkx3Vmg5b0V3VjlkVEo4VUxobHVhaDczeU9WeUFsNjBPY1JhV0FC?= =?utf-8?B?QjJqdGFiSU04N01kMk1xSndzUWFIRFZnSkVxcGsranVOaDVPcG5JZXVWU1JK?= =?utf-8?B?eVZWdVNyeUp2aUtGZXV1VkRtWk4zZ3QvcThWNDJEMGhwV1hZZFVUYVVGN1ZB?= =?utf-8?B?UHZ0NEdOQm80V0tuQjNld0djcDdLZmJJOVNTTWFDSUFnanRmazN5bkd1R0Rv?= =?utf-8?B?b3F0bURHV2VqcXNya3J1S2JUTW5tWHY3SU4vMDRFZ0MxYXI0UzN2TTY2V2Jz?= =?utf-8?B?b2wxZFBtLzlvakIyY2gySVBCTXljdFFxWnJqdk8rWlRPcitCTlBVeTVyN3VB?= =?utf-8?B?N21JS3Z4enJEWkhDN1kzYUFWUXhvRERCUU9pSjFnaWs4RXYrdGdCcXJNMjBi?= =?utf-8?Q?/WuwjPb+udTn1OUCXqiYj+6O+?= X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7dfb8a26-0b99-4f11-8deb-08dad1390617 X-MS-Exchange-CrossTenant-AuthSource: VI1PR08MB3919.eurprd08.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Nov 2022 12:06:47.2831 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 0Q2ciUZHkSHO5cRtacbCvnuit2XC4QBVh90vD0SXl7O24Fiu3XTgz6l+cLTk16kYAoAvb+kmj/PPMUSpRRlj/Q== X-MS-Exchange-Transport-CrossTenantHeadersStamped: GV2PR08MB8003 X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,FORGED_SPF_HELO,GIT_PATCH_0,KAM_DMARC_NONE,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Thanks for the patch. On 11/26/22 02:04, Thiago Jung Bauermann wrote: > This change allows aarch64-linux to support debugging programs where > different threads have different SVE vector lengths. It requires > gdbserver to support different inferior threads having different target > descriptions. > > The arch_update_tdesc method is added to the linux_process_target class to > allow aarch64-linux to probe the inferior's vq register and provide an > updated thread target description reflecting the new vector length. > > After this change, all targets except SVE-supporting aarch64-linux will > still use per-process target descriptions. > --- > gdbserver/gdbthread.h | 2 ++ > gdbserver/linux-aarch64-low.cc | 37 +++++++++++++++++++++++++++++++++- > gdbserver/linux-low.cc | 18 +++++++++++++++++ > gdbserver/linux-low.h | 5 +++++ > gdbserver/regcache.cc | 11 +++++++--- > gdbserver/tdesc.cc | 3 +++ > 6 files changed, 72 insertions(+), 4 deletions(-) > > diff --git a/gdbserver/gdbthread.h b/gdbserver/gdbthread.h > index 8b897e73d33b..47b44d03b8e0 100644 > --- a/gdbserver/gdbthread.h > +++ b/gdbserver/gdbthread.h > @@ -80,6 +80,8 @@ struct thread_info > > /* Branch trace target information for this thread. */ > struct btrace_target_info *btrace = nullptr; > + > + const struct target_desc *tdesc = nullptr; Should we add a bit of information on how this new field is used, through a comment? > }; > > extern std::list all_threads; > diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc > index cab4fc0a4674..786ce4071279 100644 > --- a/gdbserver/linux-aarch64-low.cc > +++ b/gdbserver/linux-aarch64-low.cc > @@ -99,6 +99,9 @@ protected: > > void low_arch_setup () override; > > + gdb::optional > + arch_update_tdesc (const thread_info *thread) override; > + > bool low_cannot_fetch_register (int regno) override; > > bool low_cannot_store_register (int regno) override; > @@ -184,6 +187,8 @@ struct arch_process_info > same for each thread, it is reasonable for the data to live here. > */ > struct aarch64_debug_reg_state debug_reg_state; > + > + bool has_sve; > }; Though obvious, adding a comment like "has_sve" in gdb/aarch64-tdep.h will clarify the use of this field. > > /* Return true if the size of register 0 is 8 byte. */ > @@ -840,8 +845,16 @@ aarch64_target::low_arch_setup () > { > struct aarch64_features features > = aarch64_get_arch_features (current_thread); > + const target_desc *tdesc = aarch64_linux_read_description (features); > > - current_process ()->tdesc = aarch64_linux_read_description (features); > + /* Only SVE-enabled inferiors need per-thread target descriptions. */ > + if (features.vq > 0) > + { > + current_thread->tdesc = tdesc; > + current_process ()->priv->arch_private->has_sve = true; > + } > + > + current_process ()->tdesc = tdesc; > > /* Adjust the register sets we should use for this particular set of > features. */ > @@ -853,6 +866,28 @@ aarch64_target::low_arch_setup () > aarch64_linux_get_debug_reg_capacity (lwpid_of (current_thread)); > } > > +/* Implementation of linux target ops method "arch_update_tdesc". */ > + > +gdb::optional > +aarch64_target::arch_update_tdesc (const thread_info *thread) > +{ > + /* Only processes using SVE need to update the thread's target description. */ > + if (!get_thread_process (thread)->priv->arch_private->has_sve) > + return {}; > + > + const struct aarch64_features features = aarch64_get_arch_features (thread); > + const struct target_desc *tdesc = aarch64_linux_read_description (features); > + > + if (tdesc == thread->tdesc) > + return {}; > + > + /* Adjust the register sets we should use for this particular set of > + features. */ > + aarch64_adjust_register_sets(features); > + > + return tdesc; > +} > + > /* Implementation of linux target ops method "get_regs_info". */ > > const regs_info * > diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc > index fc496275d6a3..7c510e26f0f5 100644 > --- a/gdbserver/linux-low.cc > +++ b/gdbserver/linux-low.cc > @@ -483,6 +483,12 @@ linux_process_target::arch_setup_thread (thread_info *thread) > low_arch_setup (); > } > > +gdb::optional > +linux_process_target::arch_update_tdesc (const thread_info *thread) > +{ > + return {}; > +} > + > int > linux_process_target::handle_extended_wait (lwp_info **orig_event_lwp, > int wstat) > @@ -2348,6 +2354,18 @@ linux_process_target::filter_event (int lwpid, int wstat) > return; > } > } > + else > + { > + /* Give the arch code an opportunity to update the thread's target > + description. */ > + gdb::optional new_tdesc > + = arch_update_tdesc (thread); > + if (new_tdesc.has_value ()) > + { > + regcache_release (); > + thread->tdesc = *new_tdesc; > + } > + } > } > > if (WIFSTOPPED (wstat) && child->must_set_ptrace_flags) > diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h > index 182a540f3bb3..ff14423e9e07 100644 > --- a/gdbserver/linux-low.h > +++ b/gdbserver/linux-low.h > @@ -604,6 +604,11 @@ class linux_process_target : public process_stratum_target > /* Architecture-specific setup for the current thread. */ > virtual void low_arch_setup () = 0; > > + /* Allows arch-specific code to update the thread's target description when > + the inferior stops. */ I'd also mention sometimes we don't need to update the target description if nothing's changed. So an empty return is expected. > + virtual gdb::optional > + arch_update_tdesc (const thread_info *thread); > + > /* Return false if we can fetch/store the register, true if we cannot > fetch/store the register. */ > virtual bool low_cannot_fetch_register (int regno) = 0; > diff --git a/gdbserver/regcache.cc b/gdbserver/regcache.cc > index 14236069f712..03ee88b3cfd1 100644 > --- a/gdbserver/regcache.cc > +++ b/gdbserver/regcache.cc > @@ -39,11 +39,16 @@ get_thread_regcache (struct thread_info *thread, int fetch) > have. */ > if (regcache == NULL) > { > - struct process_info *proc = get_thread_process (thread); > + /* First see if there's a thread-specific target description. */ > + const target_desc *tdesc = thread->tdesc; > > - gdb_assert (proc->tdesc != NULL); > + /* If not, get it from the process instead. */ > + if (tdesc == nullptr) > + tdesc = get_thread_process (thread)->tdesc; Just a suggestion. Your call. We could abstract away trying to fetch a tdesc from a thread and then from a process by having a function "get_tdesc ()" and calling it here. Possibly with a better name. > > - regcache = new_register_cache (proc->tdesc); > + gdb_assert (tdesc != nullptr); > + > + regcache = new_register_cache (tdesc); > set_thread_regcache_data (thread, regcache); > } > > diff --git a/gdbserver/tdesc.cc b/gdbserver/tdesc.cc > index 5693cc6626fb..3665ab0540d5 100644 > --- a/gdbserver/tdesc.cc > +++ b/gdbserver/tdesc.cc > @@ -129,6 +129,9 @@ current_target_desc (void) > if (current_thread == NULL) > return &default_description; > > + if (current_thread->tdesc != nullptr) > + return current_thread->tdesc; > + > return current_process ()->tdesc; We'd use the above function in here as well. > } > Otherwise LGTM. Reviewed-by: Luis Machado