On 29/05/2024 8:55 pm, Oleksii Kurochko wrote: > Signed-off-by: Oleksii Kurochko <oleksii.kuroc...@gmail.com> > Acked-by: Jan Beulich <jbeul...@suse.com>
This patch looks like it can go in independently? Or does it depend on having bitops.h working in practice? However, one very strong suggestion... > diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h > index 07c7a0abba..cc4a07a71c 100644 > --- a/xen/arch/riscv/include/asm/mm.h > +++ b/xen/arch/riscv/include/asm/mm.h > @@ -3,11 +3,246 @@ > <snip> > +/* PDX of the first page in the frame table. */ > +extern unsigned long frametable_base_pdx; > + > +/* Convert between machine frame numbers and page-info structures. */ > +#define mfn_to_page(mfn) \ > + (frame_table + (mfn_to_pdx(mfn) - frametable_base_pdx)) > +#define page_to_mfn(pg) \ > + pdx_to_mfn((unsigned long)((pg) - frame_table) + frametable_base_pdx) Do yourself a favour and not introduce frametable_base_pdx to begin with. Every RISC-V board I can find has things starting from 0 in physical address space, with RAM starting immediately after. Taking the microchip board as an example, RAM actually starts at 0x8000000, which means that having frametable_base_pdx and assuming it does get set to 0x8000 (which isn't even a certainty, given that I think you'll need struct pages covering the PLICs), then what you are trading off is: * Saving 32k of virtual address space only (no need to even allocate memory for this range of the framtable), by * Having an extra memory load and add/sub in every page <-> mfn conversion, which is a screaming hotpath all over Xen. It's a terribly short-sighted tradeoff. 32k of VA space might be worth saving in a 32bit build (I personally wouldn't - especially as there's no need to share Xen's VA space with guests, given no PV guests on ARM/RISC-V), but it's absolutely not at all in an a 64bit build with TB of VA space available. Even if we do find a board with the first interesting thing in the frametable starting sufficiently away from 0 that it might be worth considering this slide, then it should still be Kconfig-able in a similar way to PDX_COMPRESSION. You don't want to be penalising everyone because of a theoretical/weird corner case. ~Andrew
