Re: [PATCH v1 5/5] xen/riscv: map FDT

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Thu, 2024-07-11 at 13:44 +0100, Julien Grall wrote:
> Hi Oleksii,
Hi Julien,

> 
> On 11/07/2024 13:29, Oleksii wrote:
> > On Thu, 2024-07-11 at 12:54 +0100, Julien Grall wrote:
> > > Hi,
> > > 
> > > On 11/07/2024 12:28, Oleksii wrote:
> > > > Add Julien as he asked basically the same question in another
> > > > thread.
> > > 
> > > Thanks!
> > > 
> > > > 
> > > > On Thu, 2024-07-11 at 12:50 +0200, Jan Beulich wrote:
> > > > > On 11.07.2024 12:26, Oleksii wrote:
> > > > > > On Thu, 2024-07-11 at 11:54 +0200, Jan Beulich wrote:
> > > > > > > On 11.07.2024 11:40, Oleksii wrote:
> > > > > > > > On Wed, 2024-07-10 at 14:38 +0200, Jan Beulich wrote:
> > > > > > > > > On 03.07.2024 12:42, Oleksii Kurochko wrote:
> > > > > > Does it make sense now?
> > > > > 
> > > > > I think so, yet at the same time it only changes the
> > > > > question:
> > > > > Why is
> > > > > it
> > > > > that you absolutely need to use setup_initial_mapping()?
> > > > There is no strict requirement to use setup_initial_mapping().
> > > > That
> > > > function is available to me at the moment, and I haven't found
> > > > a
> > > > better
> > > > option other than reusing what I currently have.
> > > 
> > > I am not very familiar with the code base for RISC-V, but looking
> > > at
> > > the
> > > context in the patch, it seems you will still have the identity
> > > mapping
> > > mapped until start_xen().
> > We have identity mapping only for a small piece of .text section:
> >          . = ALIGN(IDENT_AREA_SIZE);
> >          _ident_start = .;
> >          *(.text.ident)
> >          _ident_end = .;
> > 
> > All other will be identically mapped only in case of linker address
> > is
> > equal to load address.
> > 
> > > 
> > > I assume we don't exactly know where the loader will put Xen in
> > > memory.
> > > Which means that the region may clash with your defined runtime
> > > regions
> > > (such as the FDT). Did I misunderstand anything?
> > I am not really get what is the issue here.
> > 
> > If we are speaking about physical regions then loader will
> > guarantee
> > that Xen and FDT regions don't overlap.
> 
> Sure. But I was referring to...
> 
> > 
> > If we are speaking about virtual regions then Xen will check that
> > nothing is overlaped. 
> 
> ... this part. The more regions you mapped with MMU off, the more
> work 
> you have to do to ensure nothing will clash.
Yes, agree here. Then I have to look at what I need now to introduce
map_pages_to_xen().

Thanks for clarifying.

> 
> > And the virtual regions are mapped so high so I
> > am not sure that loader will put something there. ( FDT in Xen is
> > mapped to 0xffffffffc0200000 )
> Never say never :). On Arm, some 64-bit HW (such as ADLink AVA
> platform) 
> has the RAM starting very high and load Xen around 8TB. For Arm, we 
> still decided to put a limit (10TB) where Xen can be loaded but this
> is 
> mainly done for convenience (otherwise it is a bit more complicated
> to 
> get off the identity mapping).
Oh, it is very high. I couldn't even expect.

~ Oleksii

> 
> We still have a check in place to ensure that Xen is not loaded above
> 10TB. If you map the FDT within the same L1.
> 
> Cheers,
>