>>> 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
>>> 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.
Xen-devel mailing list