|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 1/2] xen/pdx: account for frametable_base_pdx in generic pdx_to_page/page_to_pdx
On 05-May-26 09:13, Roger Pau Monné wrote: > On Tue, May 05, 2026 at 08:48:15AM +0200, Orzel, Michal wrote: >> >> >> On 04-May-26 17:28, Roger Pau Monné wrote: >>> On Thu, Apr 30, 2026 at 02:51:02PM +0200, Michal Orzel wrote: >>>> The generic pdx_to_page() and page_to_pdx() macros in xen/pdx.h assume >>>> the frame table starts at PDX 0, which is only true on x86. ARM >>>> uses a non-zero frametable_base_pdx to offset into the frame table (PPC >>>> also >>>> defines it). >>>> >>>> Fix the generic macros to subtract/add frametable_base_pdx, defaulting >>>> to 0 when the arch does not define it. This makes the generic macros >>>> correct for all architectures, even though they are only used on x86 >>>> today. >>> >>> Hm, I assume this offset was added because the original mask PDX >>> compression won't (usually) compress the gap between 0 and the start >>> of RAM. However the newish offset PDX compression should be able to >>> compress from 0 to start of RAM, and hence you don't need to apply >>> an extra PDX offset there? >>> >>> If that's indeed the case it might be better to integrate >>> frametable_base_pdx into the mask compression algorithm itself, so >>> that on some arches it's a mask plus a decrease. >> The offset is needed regardless of whether compression is used. With >> CONFIG_PDX_NONE (no compression, PDX == MFN), if RAM starts at e.g. >> 0x80000000, the first valid PDX is 0x80000. > > OK, so you are doing some (kind of) address space compression (removing > the leading empty range to the first RAM region) even when PDX is > disabled. > >> Without frametable_base_pdx >> the frame table would have to be indexed from 0, wasting >> 0x80000 * sizeof(page_info) of memory just to cover the hole before RAM. > > But you don't really "waste" memory, just address space? Oh, maybe > not on ARM as it doesn't use pdx_group_valid? And so you > unconditionally populate the frametable from PDX 0 to max PDX. With pdx_group_valid (which this series adds) we wouldn't waste physical memory for the leading gap. But we'd still waste virtual address space and the FRAMETABLE_NR check (max_pdx > FRAMETABLE_NR) becomes tighter because the full range from PDX 0 must fit. For example with RAM starting at 5TB the virtual offset before the first usable entry would be ~70GB — more than the entire 32GB FRAMETABLE_SIZE on ARM64. > >> So frametable_base_pdx is really a frame table indexing offset, not >> something tied to the compression algorithm. > > Right, it just seems odd to do that extra subtraction when using > offset compression, as in that case the compression logic itself > should remove that leading gap when RAM doesn't start at 0. > > Instead of generalizing and expanding the usage of frametable_base_pdx > it might be better to implement support for pdx_group_valid when > populating the frame table, and switch by default to the offset > compression method that will already remove any leading unpopulated > spaces? Switching the compression method would be a bigger change, and with feature freeze on Friday I'd prefer not to get into that now. The current approach is minimal and self-contained and works with mask and no-pdx which is what we use nowadays: frametable_base_pdx already existed on ARM and PPC, we're just making the generic macros aware of it as Julien requested (in v1 I just overwrote the macro in local file). We can revisit the compression strategy as a follow-up next release. ~Michal
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |