Re: [PATCH RFC 07/10] domain: map/unmap GADDR based shared guest areas

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 19/10/2022 8:43 am, Jan Beulich wrote:
> The registration by virtual/linear address has downsides: At least on
> x86 the access is expensive for HVM/PVH domains. Furthermore for 64-bit
> PV domains the areas are inaccessible (and hence cannot be updated by
> Xen) when in guest-user mode.

They're also inaccessible in HVM guests (x86 and ARM) when Meltdown
mitigations are in place.

And lets not get started on the multitude of layering violations that is
guest_memory_policy() for nested virt.  In fact, prohibiting any form of
map-by-va is a perquisite to any rational attempt to make nested virt work.

(In fact, that infrastructure needs purging before any other
architecture pick up stubs too.)

They're also inaccessible in general because no architecture has
hypervisor privilege in a normal user/supervisor split, and you don't
know whether the mapping is over supervisor or user mapping, and
settings like SMAP/PAN can cause the pagewalk to fail even when the
mapping is in place.


There are a lot of good reasons why map-by-va should never have happened.

Even for PV guests, map-by-gfn (and let the guest manage whatever
virtual mappings it wants on its own) would have been closer to the
status quo for how real hardware worked, and critically would have
avoided the restriction that the areas had to live at a globally fixed
position to be useful.





>
> In preparation of the introduction of new vCPU operations allowing to
> register the respective areas (one of the two is x86-specific) by
> guest-physical address, flesh out the map/unmap functions.
>
> Noteworthy differences from map_vcpu_info():
> - areas can be registered more than once (and de-registered),

When register by GFN is available, there is never a good reason to the
same area twice.

The guest maps one MMIO-like region, and then constructs all the regular
virtual addresses mapping it (or not) that it wants.

This API is new, so we can enforce sane behaviour from the outset.  In
particular, it will help with ...

> - remote vCPU-s are paused rather than checked for being down (which in
>   principle can change right after the check),
> - the domain lock is taken for a much smaller region.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> RFC: By using global domain page mappings the demand on the underlying
>      VA range may increase significantly. I did consider to use per-
>      domain mappings instead, but they exist for x86 only. Of course we
>      could have arch_{,un}map_guest_area() aliasing global domain page
>      mapping functions on Arm and using per-domain mappings on x86. Yet
>      then again map_vcpu_info() doesn't do so either (albeit that's
>      likely to be converted subsequently to use map_vcpu_area() anyway).

... this by providing a bound on the amount of vmap() space can be consumed.

>
> RFC: In map_guest_area() I'm not checking the P2M type, instead - just
>      like map_vcpu_info() - solely relying on the type ref acquisition.
>      Checking for p2m_ram_rw alone would be wrong, as at least
>      p2m_ram_logdirty ought to also be okay to use here (and in similar
>      cases, e.g. in Argo's find_ring_mfn()). p2m_is_pageable() could be
>      used here (like altp2m_vcpu_enable_ve() does) as well as in
>      map_vcpu_info(), yet then again the P2M type is stale by the time
>      it is being looked at anyway without the P2M lock held.

Again, another error caused by Xen not knowing the guest physical
address layout.  These mappings should be restricted to just RAM regions
and I think we want to enforce that right from the outset.

~Andrew