[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <efc3a224-27b3-4259-a9a3-0296ccbf3e8d@intel.com>
Date: Tue, 18 Jun 2024 10:59:06 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: Marco Cavenati <cavenati.marco@...il.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: peterz@...radead.org, mingo@...hat.com, acme@...nel.org,
namhyung@...nel.org, mark.rutland@....com,
alexander.shishkin@...ux.intel.com, jolsa@...nel.org, irogers@...gle.com,
adrian.hunter@...el.com, kan.liang@...ux.intel.com, tglx@...utronix.de,
bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com
Subject: Re: [PATCH] perf/x86/intel/pt: Update topa_entry base len to support
52-bit physical addresses
On 6/18/24 04:06, Marco Cavenati wrote:
> Increase topa_entry base to 40 bits to accommodate page addresses in
> systems with 52-bit physical addresses.
> The Base Physical Address field (base) has a length of MAXPHYADDR - 12 as
> stated in Intel's SDM chapter 33.2.7.2.
> The maximum MAXPHYADDR is 52 as stated in SDM 4.1.4.
> Therefore, the maximum base bit length is 40.
This makes it sound like it's _adding_ support for larger physical
addresses. It really was a bug from day one. MAXPHYADDR has been
defined to be "at most 52" for a long, long time. I think it was well
before 5-level paging came on the scene and actual MAXPHYADDR=52 systems
came along.
It probably needs to say something more along the lines of:
topa_entry->base needs to store a pfn. It obviously needs to be
large enough to store the largest possible x86 pfn which is
MAXPHYADDR-PAGE_SIZE (52-12). So it is 4 bits too small.
This isn't the only bug in the area:
> static void *pt_buffer_region(struct pt_buffer *buf)
> {
> return phys_to_virt(TOPA_ENTRY(buf->cur, buf->cur_idx)->base << TOPA_SHIFT);
> }
At this point, ->base is still a 40-bit (or 36-bit before this patch)
type. If it has anything in the high 12 bits, a <<TOPA_SHIFT will just
lose those bits.
But maybe I'm reading it wrong. If I'm right, this malfunctions at pfns
over 36-12=24 bits, or 64GB of RAM. Is it possible nobody has ever
allocated a 'struct pt_buffer' over 64GB? Or is this somehow tolerant
of reading garbage?
Powered by blists - more mailing lists