Re: [PATCH v3 3/3] xen/riscv: introduce identity mapping

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Thu, 2023-07-20 at 16:06 +0200, Jan Beulich wrote:
> On 20.07.2023 15:34, Oleksii wrote:
> > On Thu, 2023-07-20 at 12:29 +0200, Jan Beulich wrote:
> > > On 20.07.2023 10:28, Oleksii wrote:
> > > > On Thu, 2023-07-20 at 07:58 +0200, Jan Beulich wrote:
> > > > > On 19.07.2023 18:35, Oleksii wrote:
> > > > > > Then we will have completely different L0 tables for
> > > > > > identity
> > > > > > mapping
> > > > > > and not identity and the code above will be correct.
> > > > > 
> > > > > As long as Xen won't grow beyond 2Mb total. Considering that
> > > > > at
> > > > > some point you may want to use large page mappings for .text,
> > > > > .data, and .rodata, that alone would grow Xen to 6 Mb (or
> > > > > really
> > > > > 8,
> > > > > assuming .init goes separate as well). That's leaving aside
> > > > > the
> > > > > realistic option of the mere sum of all sections being larger
> > > > > than
> > > > > 2. That said, even Arm64 with ACPI is still quite a bit below
> > > > > 2Mb.
> > > > > x86 is nearing 2.5 though in even a somewhat limited config;
> > > > > allyesconfig may well be beyond that already.
> > > > I am missing something about Xen size. Lets assume that Xen
> > > > will be
> > > > mapped using only 4k pagees ( like it is done now ). Then if
> > > > Xen
> > > > will
> > > > be more then 2Mb then only what will be changed is a number of
> > > > page
> > > > tables so it is only question of changing of
> > > > PGTBL_INITIAL_COUNT (
> > > > in
> > > > case of RISC-V).
> > > 
> > > And the way you do the tearing down of the transient 1:1 mapping.
> > It looks like removing 1:1 mapping will be the same.
> > 
> > Let's assume that the size of Xen is 4 MB and that load and linker
> > ranges don't overlap ( load and linker start address are 2Mb
> > aligned ),
> > and the difference between them isn't bigger than 1 GB. Then one L2
> > page table, one L1 page table and two L0 page tables for identity
> > mapping, and two L0 page tables for non-identity mapping are
> > needed.
> > Then at L1, we will have different indexes for load_start and
> > linker_start. So what will be needed is to clean two L1 page table
> > entries started from some index.
> > 
> > The only issue I see now is that it won't work in case if identity
> > mapping crosses a 1 Gb boundary. Then for identity mapping, it will
> > be
> > needed two L1 page tables, and only one of them identity mapping
> > will
> > be removed.
> > 
> > Do I miss anything else?
> 
> Looks correct to me.
> 
> > Wouldn't it be better to take into account that now?
> 
> Sure, it's generally better to avoid leaving traps for someone to
> fall into later.

Thanks a lot. Then it make sense to update the removing identity
mapping algo.

~ Oleksii