Re: [Xen-devel] [PATCH] x86/mm: Consolidate all Xen L4 slot writing into init_xen_l4_slots()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.09.17 at 19:07, <andrew.cooper3@xxxxxxxxxx> wrote:
> Having all of this logic together makes it easier to follow Xen's virtual
> setup across the whole system.
> 
> No practical changes to the resulting L4, although the logic has been
> rearanged to avoid rewriting some slots.  This changes the zap_ro_mpt
> parameter to simply ro_mpt.  Another side effect is that highmem-start= is
> applied consistently to all L4 tables, not just PV ones.

Is this side effect really a good idea to have? I.e. are you certain
HVM-only code (i.e. such known to only ever run in HVM or idle
vCPU context) is all prepared to not have everything direct-
mapped, and is using map_domain_page() et al everywhere?

> +void init_xen_l4_slots(l4_pgentry_t *l4t, mfn_t l4mfn,
> +                       const struct domain *d, mfn_t sl4mfn, bool ro_mpt)
> +{
> +    /* Slot 256: RO M2P (if applicable). */
> +    l4t[l4_table_offset(RO_MPT_VIRT_START)] =
> +        ro_mpt ? idle_pg_table[l4_table_offset(RO_MPT_VIRT_START)]
> +               : l4e_empty();

While the patch in general looks correct, I'm also not convinced
having the slot numbers here as well as doing the operation in an
open-coded slot-by-slot fashion is a good idea: The comments
and the intended ordering here can easily go stale with future
adjustments to the virtual address layout.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel