diff mbox series

[01/10] x86/mm: no-one passes a NULL domain to init_xen_l4_slots()

Message ID 19d7ad4f-c653-b7b6-59a8-90c9700c9200@suse.com
State Superseded
Headers show
Series x86: mm (mainly shadow) adjustments | expand

Commit Message

Jan Beulich April 17, 2020, 2:25 p.m. UTC
Drop the NULL checks - they've been introduced by commit 8d7b633ada
("x86/mm: Consolidate all Xen L4 slot writing into
init_xen_l4_slots()") for no apparent reason.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Comments

Andrew Cooper April 17, 2020, 7:46 p.m. UTC | #1
On 17/04/2020 15:25, Jan Beulich wrote:
> Drop the NULL checks - they've been introduced by commit 8d7b633ada
> ("x86/mm: Consolidate all Xen L4 slot writing into
> init_xen_l4_slots()") for no apparent reason.

:) I'll take this as conformation that all my sudden pagetable work in
Xen manage ended up being rather more subtle than Linux's equivalent
work for KPTI.

https://lists.xenproject.org/archives/html/xen-devel/2018-01/msg00281.html

Specifically, this was part of trying to arrange for fully per-pcpu
private mappings, and was used to construct the pagetables for the idle
vcpu which specifically don't have a perdomain mapping.

Seeing as this is still an outstanding task in the secret-free-Xen
plans, any dropping of it now will have to be undone at some point in
the future.  Is there a specific reason for the cleanup?

> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -1696,7 +1696,7 @@ void init_xen_l4_slots(l4_pgentry_t *l4t

If we continue with this patch, this comment, just out of context, needs
adjusting.

~Andrew

>       * PV vcpus need a shortened directmap.  HVM and Idle vcpus get the full
>       * directmap.
>       */
> -    bool short_directmap = d && !paging_mode_external(d);
> +    bool short_directmap = !paging_mode_external(d);
>  
>      /* Slot 256: RO M2P (if applicable). */
>      l4t[l4_table_offset(RO_MPT_VIRT_START)] =
>
Jan Beulich April 20, 2020, 5:48 a.m. UTC | #2
On 17.04.2020 21:46, Andrew Cooper wrote:
> On 17/04/2020 15:25, Jan Beulich wrote:
>> Drop the NULL checks - they've been introduced by commit 8d7b633ada
>> ("x86/mm: Consolidate all Xen L4 slot writing into
>> init_xen_l4_slots()") for no apparent reason.
> 
> :) I'll take this as conformation that all my sudden pagetable work in
> Xen manage ended up being rather more subtle than Linux's equivalent
> work for KPTI.
> 
> https://lists.xenproject.org/archives/html/xen-devel/2018-01/msg00281.html
> 
> Specifically, this was part of trying to arrange for fully per-pcpu
> private mappings, and was used to construct the pagetables for the idle
> vcpu which specifically don't have a perdomain mapping.
> 
> Seeing as this is still an outstanding task in the secret-free-Xen
> plans, any dropping of it now will have to be undone at some point in
> the future.

s/will/may/ I suppose - we don't know for sure how this will look
like at this point.

>  Is there a specific reason for the cleanup?

Elimination of effectively dead code. We avoid arbitrary NULL checks
elsewhere as well; iirc I've seen you (amongst others) comment on
people trying to introduce such in their patches.

>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -1696,7 +1696,7 @@ void init_xen_l4_slots(l4_pgentry_t *l4t
> 
> If we continue with this patch, this comment, just out of context, needs
> adjusting.

Will do.

Jan
Andrew Cooper April 22, 2020, 12:20 p.m. UTC | #3
On 20/04/2020 06:48, Jan Beulich wrote:
> On 17.04.2020 21:46, Andrew Cooper wrote:
>> On 17/04/2020 15:25, Jan Beulich wrote:
>>> Drop the NULL checks - they've been introduced by commit 8d7b633ada
>>> ("x86/mm: Consolidate all Xen L4 slot writing into
>>> init_xen_l4_slots()") for no apparent reason.
>> :) I'll take this as conformation that all my sudden pagetable work in
>> Xen manage ended up being rather more subtle than Linux's equivalent
>> work for KPTI.
>>
>> https://lists.xenproject.org/archives/html/xen-devel/2018-01/msg00281.html
>>
>> Specifically, this was part of trying to arrange for fully per-pcpu
>> private mappings, and was used to construct the pagetables for the idle
>> vcpu which specifically don't have a perdomain mapping.
>>
>> Seeing as this is still an outstanding task in the secret-free-Xen
>> plans, any dropping of it now will have to be undone at some point in
>> the future.
> s/will/may/ I suppose - we don't know for sure how this will look
> like at this point.

Will.

The only reason we don't need it right now is because idle_pg_table[]
gets constructed at boot time.  As soon as we're creating one (or more)
per pcpu, we need a way of writing an L4 without a perdomain mapping.

~Andrew
diff mbox series

Patch

--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1696,7 +1696,7 @@  void init_xen_l4_slots(l4_pgentry_t *l4t
      * PV vcpus need a shortened directmap.  HVM and Idle vcpus get the full
      * directmap.
      */
-    bool short_directmap = d && !paging_mode_external(d);
+    bool short_directmap = !paging_mode_external(d);
 
     /* Slot 256: RO M2P (if applicable). */
     l4t[l4_table_offset(RO_MPT_VIRT_START)] =
@@ -1717,10 +1717,9 @@  void init_xen_l4_slots(l4_pgentry_t *l4t
         mfn_eq(sl4mfn, INVALID_MFN) ? l4e_empty() :
         l4e_from_mfn(sl4mfn, __PAGE_HYPERVISOR_RW);
 
-    /* Slot 260: Per-domain mappings (if applicable). */
+    /* Slot 260: Per-domain mappings. */
     l4t[l4_table_offset(PERDOMAIN_VIRT_START)] =
-        d ? l4e_from_page(d->arch.perdomain_l3_pg, __PAGE_HYPERVISOR_RW)
-          : l4e_empty();
+        l4e_from_page(d->arch.perdomain_l3_pg, __PAGE_HYPERVISOR_RW);
 
     /* Slot 261-: text/data/bss, RW M2P, vmap, frametable, directmap. */
 #ifndef NDEBUG