From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4B94E154BE2 for ; Wed, 18 Dec 2024 21:02:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.169 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734555758; cv=none; b=R2ipZMFiMPwO+JVsSE251TcUUAZsbvS5gfFsHx8wfi06wTaS++8Vf0LOG5ZmQ+9v1vSjBR4kzALo7bzrjD40GJgZ1+CxilGCcCFqL0FUkpCtscFz7jaDpuKLBLUkAsLhvuoQm9dkFltv/N0uQCWSKISC+kGVJx84f8oe2nUxwXE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734555758; c=relaxed/simple; bh=Wmg6V9Kp+8xcAMfN5JLKf/Zv/txUVGtU/0FK9QGmSg8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=cgcS+IAGgmG1JzsrbF2imNUqEGDYwi7V1pgiEkZLDlbz/SBg25PV6VL+EeH2K0uRtkaILv+VjvOqW3Dg7EFX5KbMS2+brn3br10TTdBgrUXnRtYT0BRb26HsC63/KgBuwH1884S4n9Zg3p8V/5/Fxk7zZI8O2YliAN/52uvoD00= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=rivosinc.com; spf=pass smtp.mailfrom=rivosinc.com; dkim=pass (2048-bit key) header.d=rivosinc-com.20230601.gappssmtp.com header.i=@rivosinc-com.20230601.gappssmtp.com header.b=2TUL8CoU; arc=none smtp.client-ip=209.85.210.169 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rivosinc.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=rivosinc-com.20230601.gappssmtp.com header.i=@rivosinc-com.20230601.gappssmtp.com header.b="2TUL8CoU" Received: by mail-pf1-f169.google.com with SMTP id d2e1a72fcca58-728e78c4d7bso104069b3a.0 for ; Wed, 18 Dec 2024 13:02:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1734555754; x=1735160554; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=VEvOuS9GjCc8R1kkciZzUfnJPop9QAlnrlUsyAlmMgg=; b=2TUL8CoUEJ3AnQl782k/BHnn3PpPbOiF0hc7lccK5l7j2H0QX4gBfaoFQoxAKQQLU+ NgcacYRySsEA25xO018IkyVGOvhoLNbL33g6T6dKoO6Ny+wpz4LcTSqPSx2x38ryIl7Q L68dhfuCBx5sDZG9ms2inv2kNHE61GpzaWWFmgCPzcBF1VOMRElDg0b+87Tk/BfRYVgU vvPG1+Q1WSoUTPQhhXMFzma1MNzDDEFGG713IlHAg28aB9AK+LQ/SgTA7czfaBe9GoRw QXcnjHRjcck0JCJakA6a7UucBmyLhY5jc4ciKhtxDnDZbu3ZfkzLs5Wmbgdy6uWr0KPC 4myg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1734555754; x=1735160554; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=VEvOuS9GjCc8R1kkciZzUfnJPop9QAlnrlUsyAlmMgg=; b=f8cNmEDYmyL73rKs9loGNf1fwnHLP2IC0mv//iKjbMbFHdnYVcR2SehoSwYCQ7F+9b 4AUKQvF8MjgTssX0b7UDpfzD998b9B9buUuVcRXV1jNiwTV8z1j3tj3Say9mrkbDCmkv UoprBXy7sV3baY+H6teegxB7Q0AE8T55Pe2JrKC0fALlSCLqbO5bvD4AXh0J/u0PPS4F Br0OHS8Xn4XqRD0V34KwnpCi4K9rkRgdxfl6Y2c7tKRs/lMbjwARBEyq4RGmPIj6/lPR rfJeqS5tkxA0F+ZJUaJhctjOG1cR/tAt76PqIUQLBmFybEm5VXjSqkCsxcoyKQDWvEY9 0gwQ== X-Forwarded-Encrypted: i=1; AJvYcCV9SvUaRwhw+MkHkU0EiDW2/o6b9IITPEWQleu0uEhT9akoG57HAw55Iwb3QZPOEfRyMtEv47JKNOkg4pg=@vger.kernel.org X-Gm-Message-State: AOJu0Yzkn38l/Oa8wNrc8LPIWddRYuI7757XVLSkNkDmwbNzaDozN/DQ lmLB+FYvbwre+SYECk5p5pQLctGoz8kA29S4QspGL/zc5xTN84FpxzjtgBbgeZU= X-Gm-Gg: ASbGncsoh5KaccbTwOyXPxNH8hSh3BbSdq7ltFWWHEux5/pIaG9dfp96pb7FCebniFD dSP8NPPfS2RuhlpkQqpyCVhah+lG6RlN68m/yWVQYrHVCjAovGOqgp2B1kHoQRRT2MbdwO8RRdz /KfrFLqHWNOd5pK6YT97qNF3PhMiriMFGteMXidsOJICJtr2wg0UNgzBrz3GiubY7dZH645ox3S QAcsVfLUnQNP3mv4VhfG9MTRGjvvR5wF5sWSF3Bviwh+QA= X-Google-Smtp-Source: AGHT+IGGTepBnro/Yv+Td9aOesliGxiq1HfALi+NYhCuWSXz+YG9fqxb1BuJcfYw6mbiXYYJ+gAnug== X-Received: by 2002:a05:6a00:7428:b0:725:4a1b:38ec with SMTP id d2e1a72fcca58-72aa9a10440mr1146368b3a.3.1734555753073; Wed, 18 Dec 2024 13:02:33 -0800 (PST) Received: from ghost ([50.145.13.30]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72918b77290sm8969134b3a.113.2024.12.18.13.02.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Dec 2024 13:02:32 -0800 (PST) Date: Wed, 18 Dec 2024 13:02:30 -0800 From: Charlie Jenkins To: Ian Rogers Cc: Arnaldo Carvalho de Melo , Peter Zijlstra , Ingo Molnar , Namhyung Kim , Mark Rutland , Alexander Shishkin , Jiri Olsa , Adrian Hunter , Paul Walmsley , Palmer Dabbelt , Albert Ou , Nathan Chancellor , Nick Desaulniers , Bill Wendling , Justin Stitt , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org Subject: Re: [PATCH v2] tools: perf: tests: Fix code reading for riscv Message-ID: References: <20241217-perf_fix_riscv_obj_reading-v2-1-58f81b7b4c7d@rivosinc.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Wed, Dec 18, 2024 at 11:23:51AM -0800, Ian Rogers wrote: > On Wed, Dec 18, 2024 at 10:41 AM Arnaldo Carvalho de Melo > wrote: > > > > On Tue, Dec 17, 2024 at 04:30:15PM -0800, Charlie Jenkins wrote: > > > On Tue, Dec 17, 2024 at 04:18:32PM -0800, Ian Rogers wrote: > > > > On Tue, Dec 17, 2024 at 3:52 PM Charlie Jenkins wrote: > > > > > After binutils commit e43d876 which was first included in binutils 2.41, > > > > > riscv no longer supports dumping in the middle of instructions. Increase > > > > > the objdump window by 2-bytes to ensure that any instruction that sits > > > > > on the boundary of the specified stop-address is not cut in half. > > > > > > > Signed-off-by: Charlie Jenkins > > > > > > Reviewed-by: Ian Rogers > > > > > > > A binutils patch has been sent as well to fix this in objdump [1]. > > > > > > > Link: https://sourceware.org/pipermail/binutils/2024-December/138139.html [1] > > > > > > > Changes in v2: > > > > > - Do objdump version detection at runtime (Ian) > > > > > - Link to v1: https://lore.kernel.org/r/20241216-perf_fix_riscv_obj_reading-v1-0-b75962660a9b@rivosinc.com > > > > > > > --- a/tools/perf/tests/code-reading.c > > > > > @@ -183,9 +244,30 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf, > > > > > const char *fmt; > > > > > FILE *f; > > > > > int ret; > > > > > + u64 stop_address = addr + len; > > > > > + > > > > > + if (IS_ENABLED(__riscv)) { > > > > > > Not sure if there is a consistency issue here. Elsewhere we're just > > > > using ifdef, such as: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/include/dwarf-regs.h?h=perf-tools-next#n69 > > > > > I don't have any strong feelings about that. I can change it to be an > > > ifdef. On other lists I have been told to use IS_ENABLED whenever > > > possible, but it's only a small difference. > > > > Can't we just use uname here? > > > > So that we don't use kconfig.h since its not used in tools/perf/ and > > makes it looks like perf is in lockstep with the kernel source tree > > version it was compiled from? > > > > $ git grep kconfig.h tools/perf/ > > $ > > > > BTW, what would happen if I collected a perf.data file on x86_64 and > > would read it in a RiscV machine with such a objdump version? The same > > problem? > > This code is in tests hence thinking that a separate fix is needed for > that problem. Hopefully the use of elf machine/flags tackles it: > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/include/dwarf-regs.h?h=perf-tools-next#n25 > We are getting somewhat disassembler heavy. We have llvm as a library, > capstone as a library, binutils objdump and llvm objdump. Given the > pain with parsing text, could we lose the objdumps? Similarly for > addr2line? Are you suggesting to remove this test case entirely to get rid of the objdump dependency? The goal of this test case seems to be to check objdump and perf return the same data, so it doesn't seem like there would be an alternative to using objdump. - Charlie > > Thanks, > Ian