Re: [Xen-devel] [PATCH 14/17] xen/arm64: head: Remove ID map as soon as it is not used

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 6/26/19 9:25 PM, Stefano Stabellini wrote:
> On Mon, 10 Jun 2019, Julien Grall wrote:
> > The ID map may clash with other parts of the Xen virtual memory layout.
> > At the moment, Xen is handling the clash by only creating a mapping to
> > the runtime virtual address before enabling the MMU.
> >
> > The rest of the mappings (such as the fixmap) will be mapped after the
> > MMU is enabled. However, the code doing the mapping is not safe as it
> > replace mapping without using the Break-Before-Make sequence.
> >
> > As the ID map can be anywhere in the memory, it is easier to remove all
> > the entries added as soon as the ID map is not used rather than adding
> > the Break-Before-Make sequence everywhere.
> I think it is a good idea, but I would ask you to mention 1:1 map
> instead of "ID map" in comments and commit messages because that is the
> wording we used in all comments so far (see the one in
> create_page_tables and mm.c). It is easier to grep and refer to if we
> use the same nomenclature. Note that I don't care about which
> nomenclature we decide to use, I am only asking for consistency.
> Otherwise, it would also work if you say it both way at least once:
>
>   The ID map (1:1 map) may clash [...]
I would rather drop the wording 1:1 as this is confusing. It is also not 
trivial to find anything on google typing "1:1".
> > It is difficult to track where exactly the ID map was created without a
> > full rework of create_page_tables(). Instead, introduce a new function
> > remove_id_map() will look where is the top-level entry for the ID map
> > and remove it.
> Do you think it would be worth simplifying this code below by preserving
> where/how the ID map was created? We could repurpose x25 for that,
> carrying for instance the address of the ID map section slot or a code
> number to specify which case we are dealing with. We should be able to
> turn remove_id_map into only ~5 lines?
I thought about it but the current implementation of create_map_tables() 
is quite awful to read. So the less I touch this function, the better I 
feel :).
I have some rework for the create_page_tables() which simplify it a lot. 
Yet, I am not entirely sure it is worth to spend time trying to simplify 
remove_id_map. This is unlikely to make the boot significantly faster 
and I don't expect the function to survive more than a release as the ID 
map as to be kept in place (for secondary boot and suspend/resume).
The only reason it is removed now is because it clashes with other 
mapping we may do.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel