On 04/05/2020 15:24, Jan Beulich wrote:
> On 02.05.2020 00:58, Andrew Cooper wrote:
>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -732,14 +732,14 @@ void load_system_tables(void)
>> .rsp2 = 0x8600111111111111ul,
>>
>> /*
>> - * MCE, NMI and Double Fault handlers get their own stacks.
>> + * #DB, NMI, DF and #MCE handlers get their own stacks.
> Then also #DF and #MC?
Ok.
>
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -6002,25 +6002,18 @@ void memguard_unguard_range(void *p, unsigned long l)
>>
>> void memguard_guard_stack(void *p)
>> {
>> - /* IST_MAX IST pages + at least 1 guard page + primary stack. */
>> - BUILD_BUG_ON((IST_MAX + 1) * PAGE_SIZE + PRIMARY_STACK_SIZE >
>> STACK_SIZE);
>> + map_pages_to_xen((unsigned long)p, virt_to_mfn(p), 1, _PAGE_NONE);
>>
>> - memguard_guard_range(p + IST_MAX * PAGE_SIZE,
>> - STACK_SIZE - PRIMARY_STACK_SIZE - IST_MAX *
>> PAGE_SIZE);
>> + p += 5 * PAGE_SIZE;
> The literal 5 here and ...
>
>> + map_pages_to_xen((unsigned long)p, virt_to_mfn(p), 1, _PAGE_NONE);
>> }
>>
>> void memguard_unguard_stack(void *p)
>> {
>> - memguard_unguard_range(p + IST_MAX * PAGE_SIZE,
>> - STACK_SIZE - PRIMARY_STACK_SIZE - IST_MAX *
>> PAGE_SIZE);
>> -}
>> -
>> -bool memguard_is_stack_guard_page(unsigned long addr)
>> -{
>> - addr &= STACK_SIZE - 1;
>> + map_pages_to_xen((unsigned long)p, virt_to_mfn(p), 1,
>> PAGE_HYPERVISOR_RW);
>>
>> - return addr >= IST_MAX * PAGE_SIZE &&
>> - addr < STACK_SIZE - PRIMARY_STACK_SIZE;
>> + p += 5 * PAGE_SIZE;
> ... here could do with macro-izing: IST_MAX + 1 would already be
> a little better, I guess.
The problem is that "IST_MAX + 1" is now less meaningful than a literal
5, because at least 5 obviously matches up with the comment describing
which page does what.
~Andrew
>
> Preferably with adjustments along these lines
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
>
> Jan
