| Message ID | 19d7ad4f-c653-b7b6-59a8-90c9700c9200@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: mm (mainly shadow) adjustments | expand |
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)] = >
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
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
--- 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
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>