>>> On 14.02.18 at 13:34, <andrew.coop...@citrix.com> wrote: > On 14/02/18 09:14, Jan Beulich wrote: >>>>> On 13.02.18 at 20:45, <andrew.coop...@citrix.com> wrote: >>> @@ -651,9 +654,6 @@ static int clone_mapping(const void *ptr, >>> root_pgentry_t *rpt) >>> l2_pgentry_t *pl2e; >>> l1_pgentry_t *pl1e; >>> >>> - if ( linear < DIRECTMAP_VIRT_START ) >>> - return 0; >> Isn't outright removing this going a little too far? > > Considering it is buggy, no. Retuning success after doing nothing is > very antisocial. > > If anything, it should return -EINVAL so the caller knows that something > expected happened, but it should also reject attempts to clone mappings > beyond the end of the directmap (as those will explode when we switch to > PV context), and the XEN_VIRT_* range needs white-listing as ok to clone > for this usecase to work.
Returning -EINVAL would have been wrong without excluding the XEN_VIRT_* range - the boot CPU has at least its IDT there. Tightening the check would be fine with me. >>> --- a/xen/arch/x86/x86_64/compat/entry.S >>> +++ b/xen/arch/x86/x86_64/compat/entry.S >>> @@ -13,6 +13,8 @@ >>> #include <public/xen.h> >>> #include <irq_vectors.h> >>> >>> + .section .text.entry, "ax", @progbits >>> + >>> ENTRY(entry_int82) >>> ASM_CLAC >>> pushq $0 >> This also puts compat_create_bounce_frame into the entry section, >> which surely isn't needed. Same for the non-compat variant. > > I considered that. Excluding those ranges won't reduce the number of > allocations we make, but it does complicate backports. Complicate backports? The context of the section directive may change some, but that's not a significant complication imo. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
