Re: [RFC PATCH] xen/memory: Introduce a hypercall to provide unallocated space

[Julien Grall <julien@xxxxxxx>]

On 09/08/2021 18:14, Oleksandr wrote:
> On 09.08.21 17:51, Julien Grall wrote:
> Hi Julien.
Hi Oleksandr,

> > I am writing down here what we discussed on another thread and on IRC. 
> > This will be easier to track in a single thread.
> > On 04/08/2021 23:00, Julien Grall wrote:
> > > On 04/08/2021 21:56, Oleksandr wrote:
> > > > Now, I am wondering, would it be possible to update/clarify the 
> > > > current "reg" purpose and use it to pass a safe unallocated space 
> > > > for any Xen specific mappings (grant, foreign, whatever) instead of 
> > > > just for the grant table region. In case, it is not allowed for any 
> > > > reason (compatibility PoV, etc), would it be possible to extend a 
> > > > property by passing an extra range separately, something similar to 
> > > > how I described above?
> > > I think it should be fine to re-use the same region so long the size 
> > > of the first bank is at least the size of the original region.
> > While answering to the DT binding question on the DT ML, I realized 
> > that this is probably not going to be fine because there is a bug in 
> > Xen when mapping grant-table frame.
> > The function gnttab_map_frame() is used to map the grant table frame. 
> > If there is an old mapping, it will first remove it.
> > The function is using the helper gnttab_map_frame() to find the 
> > corresponding GFN or return INVALID_GFN if not mapped.
> > On Arm, gnttab_map_frame() is implementing using an array index by the 
> > grant table frame number. The trouble is we don't update the array 
> > when the page is unmapped. So if the GFN is re-used before the 
> > grant-table is remapped, then we will end up to remove whatever was 
> > mapped there (this could be a foreign page...).
> > This behavior already happens today as the toolstack will use the 
> > first GFN of the region if Linux doesn't support the acquire resource 
> > interface. We are getting away in the Linux because the toolstack only 
> > map the first grant table frame and:
> >  - Newer Linux will not used the region provided by the DT and nothing 
> > will be mapped there.
> >  - Older Linux will use the region but still map the grant table frame 
> > 0 to the same GFN.
> > I am not sure about U-boot and other OSes here.
> >
> > This is not new but it is going to be become a bigger source of 
> > problem (read more chance to hit it) as we try to re-use the first 
> > region.
> > This means the first region should exclusively used for the 
> > grant-table (in a specific order) until the issue is properly fixed.
> Thank you for the explanation, it is clear now.
>
>
> > A potential fix is to update the array in p2m_put_l3_page(). The 
> > default max size of the array is 1024, so it might be fine to just 
> > walk it (it would be simply a comparison).
> I think, this would work. Looks like we don't need to walk for each gfn 
> which is being freed, we could just filter it by p2m_is_ram() ...
Well. This would still potentially result to a few unnecessary walk. I 
would consider to introduce a new P2M type or possibly add a check if 
the page is in xenheap (grant-table are xenheap pages so far).
Cheers,

--
Julien Grall