From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id gfX3DoAuBGXOjxYAWB0awg (envelope-from ) for ; Fri, 15 Sep 2023 06:14:24 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=AHvmqvxv; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 33DCC1E0C3; Fri, 15 Sep 2023 06:14:24 -0400 (EDT) Received: from server2.sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (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 273651E028 for ; Fri, 15 Sep 2023 06:14:22 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C13B238582BD for ; Fri, 15 Sep 2023 10:14:21 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C13B238582BD DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1694772861; bh=roZtFyw7gXOyGWIds6Ft/IHTh+RksY9gTYUfI+7WGJQ=; h=Date:To:Cc:Subject:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=AHvmqvxvLBndZPrSK4bNdE09OasOMJs2q/Sn4eIUYRv/ohrl/GXZ1i4U0oqg3ahMh WkCZnOTBzZyh91OubhxJnParQwkyB8LRFtXj8zVdx0oZarYK7kfH8V71NLYhdAZFeM 0Olzs7sRAcrgF3NsCkc+UI9cNuME3z4SXDhICijQ= Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111]) by sourceware.org (Postfix) with ESMTPS id BAA8B3858D1E for ; Fri, 15 Sep 2023 10:13:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BAA8B3858D1E Received: from octopus (cust120-dsl54.idnet.net [212.69.54.120]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 81A2480AE9; Fri, 15 Sep 2023 10:13:58 +0000 (UTC) Date: Fri, 15 Sep 2023 11:13:53 +0100 To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v2 3/3] gdb/amdgpu: add precise-memory support Message-ID: <20230915101353.h6z7fv4e5ekn3jn5@octopus> References: <20230914161433.432600-1-simon.marchi@efficios.com> <20230914161433.432600-3-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20230914161433.432600-3-simon.marchi@efficios.com> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.6.2 (lndn.lancelotsix.com [0.0.0.0]); Fri, 15 Sep 2023 10:13:58 +0000 (UTC) X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, 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 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: , From: Lancelot SIX via Gdb-patches Reply-To: Lancelot SIX Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" Hi Simon, > diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c > index 22c269b7992c..d38b790f53f7 100644 > --- a/gdb/amd-dbgapi-target.c > +++ b/gdb/amd-dbgapi-target.c > @@ -1326,6 +1338,29 @@ amd_dbgapi_target::stopped_by_hw_breakpoint () > return false; > } > > +/* Try to set the process's memory access reporting precision mode. > + > + Warn if the requested mode is not supported on at least one agent in the > + process. > + > + Error out if setting the requested mode failed for some other reason. */ > + > +static void > +try_set_process_memory_precision (amd_dbgapi_inferior_info &info) > +{ > + auto mode = (info.precise_memory.requested > + ? AMD_DBGAPI_MEMORY_PRECISION_PRECISE > + : AMD_DBGAPI_MEMORY_PRECISION_NONE); > + amd_dbgapi_status_t status > + = amd_dbgapi_set_memory_precision (info.process_id, mode); > + > + if (status == AMD_DBGAPI_STATUS_ERROR_NOT_SUPPORTED) > + warning (_("AMDGPU precise memory access reporting could not be enabled.")); > + else if (status != AMD_DBGAPI_STATUS_SUCCESS) > + error (_("amd_dbgapi_set_memory_precision failed (%s)"), > + get_status_string (status)); I think you forgot something like else info.precise_memory.enabled = info.precise_memory.requested; here. > +} > + > /* Make the amd-dbgapi library attach to the process behind INF. > > Note that this is unrelated to the "attach" GDB concept / command. > @@ -1399,6 +1434,8 @@ attach_amd_dbgapi (inferior *inf) > amd_dbgapi_debug_printf ("process_id = %" PRIu64 ", notifier fd = %d", > info->process_id.handle, info->notifier); > > + try_set_process_memory_precision (*info); > + > /* If GDB is attaching to a process that has the runtime loaded, there will > already be a "runtime loaded" event available. Consume it and push the > target. */ > @@ -1443,8 +1480,10 @@ detach_amd_dbgapi (inferior *inf) > for (auto &&value : info->breakpoint_map) > delete_breakpoint (value.second); > > - /* Reset the amd_dbgapi_inferior_info. */ > + /* Reset the amd_dbgapi_inferior_info, except for precise_memory_mode. */ > + bool precise_memory_requested = info->precise_memory.requested; > *info = amd_dbgapi_inferior_info (inf); Not really significant, but I am wonedring if adding a request_precise_memory bool parameter to the amd_dbgapi_inferior_info ctor (maybe with default value to false) would make sense. That being said, I'm happy either way. > + info->precise_memory.requested = precise_memory_requested; > > maybe_reset_amd_dbgapi (); > } I have tested this patch (with the fix suggested above) on gfx900 (where precise memory is not supported) and gfx90a (where precise memory is supported). With the correction, the patch looks good to me. Best, Lancelot.