Re: [PATCH early-RFC 4/5] xen/arm: mm: Rework switch_ttbr()

[Julien Grall <julien@xxxxxxx>]

Hi Stefano,

On 12/03/2022 01:17, Stefano Stabellini wrote:
> On Wed, 9 Mar 2022, Julien Grall wrote:
> As far as I can tell this should work for coloring too. In the case of
> coloring:
>
>      /* running on the old mapping, same as non-coloring */
>      update_identity_mapping(true);
>
>      /* jumping to the 1:1 mapping of the old Xen and switching to the
>       * new pagetable */
>      fn(ttbr);
>
>      /* new pagetable is enabled, now we are back to addresses greater
>       * than XEN_VIRT_START, which correspond to new cache-colored Xen */
>      update_identity_mapping(false);
>
>
> The only doubt that I have is: are we sure than a single page of 1:1
> mapping is enough? What if:
>
> virt_to_maddr(switch_ttbr_id) - virt_to_maddr(_start) > PAGE_SIZE
switch_ttbr_id() is placed before _end_boot (this needs to be renamed). 
We are veryfing a link time (see the check in xen.lds.S) that _end_boot 
- _start is always smaller than 4KB.
At the moment, the size is less than 2KB. So we have plenty of space to 
grow. Also, there are some code that is technically not used while using 
the ID map. So if necessary we can shrink the size to always fit in a 
PAGE_SIZE.
> We might have to do a 1:1 mapping of size = _end-_start. It would work
> with coloring too because we are doing a 1:1 mapping of the old copy of
> Xen which is non-colored and contiguous (not the new copy which is
> colored and fragmented).
I would like to keep the size of the ID mapping to the strict minimum. A 
PAGE_SIZE should be sufficient for most of what we need in Xen.
This would help to get rid of the old copy of Xen in case of the cache 
coloring. Otherwise, you technically have to keep it forever if you plan 
to support suspend/resume or even allow CPU hotplug.
Furthemore, if you keep the two copy around, it is more difficult to 
know which mapping is used and we increase the risk to use the wrong 
one. For instance, the current implementation of cache coloring is 
clearing the wrong set of boot pagetables.
Cheers,

--
Julien Grall