On 17.05.2018 11:22, Janosch Frank wrote:
> On 16.05.2018 16:45, David Hildenbrand wrote:
>> On 16.05.2018 16:22, Janosch Frank wrote:
>>> On 16.05.2018 16:13, David Hildenbrand wrote:
>>>> On 16.05.2018 11:10, Janosch Frank wrote:
>>>>> From: Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
>>>>>
>>>>> Tests no-execute (Instruction Execution Protection) DAT protection.
>>>>>
>>>>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>>>>> ---
>>>>>
>>>>> This was originally part of my first patchset, but was pushed back
>>>>> after David introduced virtual memory allocation. It's more a RFC, as
>>>>> I'm not completely happy with the memory management manipulation I do.
>>>>>
>>>>> ---
>>>>> lib/s390x/asm/interrupt.h | 1 +
>>>>> lib/s390x/interrupt.c | 9 +++++
>>>>> s390x/Makefile | 1 +
>>>>> s390x/iep.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>> s390x/unittests.cfg | 4 +++
>>>>> 5 files changed, 104 insertions(+)
>>>>> create mode 100644 s390x/iep.c
>>>>>
>>>>> diff --git a/lib/s390x/asm/interrupt.h b/lib/s390x/asm/interrupt.h
>>>>> index 3ccc8e3..39ae534 100644
>>>>> --- a/lib/s390x/asm/interrupt.h
>>>>> +++ b/lib/s390x/asm/interrupt.h
>>>>> @@ -15,6 +15,7 @@ void handle_pgm_int(void);
>>>>> void expect_pgm_int(void);
>>>>> uint16_t clear_pgm_int(void);
>>>>> void check_pgm_int_code(uint16_t code);
>>>>> +void register_pgm_handler(void (*handler)(struct lowcore *));
>>>>>
>>>>> /* Activate low-address protection */
>>>>> static inline void low_prot_enable(void)
>>>>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>>>>> index 56c7603..fe6cdba 100644
>>>>> --- a/lib/s390x/interrupt.c
>>>>> +++ b/lib/s390x/interrupt.c
>>>>> @@ -15,6 +15,7 @@
>>>>>
>>>>> static bool pgm_int_expected;
>>>>> static struct lowcore *lc;
>>>>> +static void (*custom_pgm_handler)(struct lowcore *);
>>>>>
>>>>> void expect_pgm_int(void)
>>>>> {
>>>>> @@ -41,8 +42,16 @@ void check_pgm_int_code(uint16_t code)
>>>>> code == lc->pgm_int_code, code, lc->pgm_int_code);
>>>>> }
>>>>>
>>>>> +void register_pgm_handler(void (*handler)(struct lowcore *))
>>>>> +{
>>>>> + custom_pgm_handler = handler;
>>>>> +}
>>>>> +
>>>>> static void fixup_pgm_int(void)
>>>>> {
>>>>> + if (custom_pgm_handler)
>>>>> + return custom_pgm_handler(lc);
>>>>> +
>>>>> switch (lc->pgm_int_code) {
>>>>> case PGM_INT_CODE_PRIVILEGED_OPERATION:
>>>>> /* Normal operation is in supervisor state, so this exception
>>>>> diff --git a/s390x/Makefile b/s390x/Makefile
>>>>> index abc3242..d4275a1 100644
>>>>> --- a/s390x/Makefile
>>>>> +++ b/s390x/Makefile
>>>>> @@ -9,6 +9,7 @@ tests += $(TEST_DIR)/pfmf.elf
>>>>> tests += $(TEST_DIR)/cmm.elf
>>>>> tests += $(TEST_DIR)/vector.elf
>>>>> tests += $(TEST_DIR)/gs.elf
>>>>> +tests += $(TEST_DIR)/iep.elf
>>>>>
>>>>> all: directories test_cases
>>>>>
>>>>> diff --git a/s390x/iep.c b/s390x/iep.c
>>>>> new file mode 100644
>>>>> index 0000000..87f3007
>>>>> --- /dev/null
>>>>> +++ b/s390x/iep.c
>>>>> @@ -0,0 +1,89 @@
>>>>> +/*
>>>>> + * Instruction Execution Prevention (IEP) DAT test.
>>>>> + *
>>>>> + * Copyright (c) 2018 IBM Corp
>>>>> + *
>>>>> + * Authors:
>>>>> + * Janosch Frank <frankja@xxxxxxxxxxxxx>
>>>>> + *
>>>>> + * This code is free software; you can redistribute it and/or modify it
>>>>> + * under the terms of the GNU Library General Public License version 2.
>>>>> + */
>>>>> +#include <libcflat.h>
>>>>> +#include <vmalloc.h>
>>>>> +#include <asm/facility.h>
>>>>> +#include <asm/interrupt.h>
>>>>> +#include <asm/page.h>
>>>>> +#include <asm/pgtable.h>
>>>>> +#include <asm-generic/barrier.h>
>>>>> +
>>>>> +static pteval_t *get_pte(pgd_t *pgtable, uintptr_t vaddr)
>>>>> +{
>>>>> + pgd_t *pgd = pgd_offset(pgtable, vaddr);
>>>>> + p4d_t *p4d = p4d_offset(pgd, vaddr);
>>>>> + pud_t *pud = pud_offset(p4d, vaddr);
>>>>> + pmd_t *pmd = pmd_offset(pud, vaddr);
>>>>> + pte_t *pte = pte_offset(pmd, vaddr);
>>>>> +
>>>>> + return &pte_val(*pte);
>>>>> +}
>>>>
>>>> You could factor that out into some sort of
>>>>
>>>> protect_range() / protect_page()
>>>> unprotect_range() / unprotect_page()
>>>
>>> Would the page protection be enough for now?
>>> Then I'd add that to our mmu.c
>>
>> Sure, we can later add protect_segment ... to keep it simple.
>>
>>>
>>>>
>>>> And implement iep handling directly in the core. The handling of
>>>> resetting the old psw seems to be generic enough.
>>>
>>> It will break two tests, that's why I added a custom handler.
>>
>> We should be able to easily check which type of protection it was, no?
>> (IEP vs. DAT vs. lowprot)
>>
>> Or what exactly makes these fail?
>
> Testing the right bits helps not jumping where we don't want to be...
> Alright, I'll send a v2 after my vacation, thanks for the guidance.
>
Okay, we can discuss once you're back. Have a nice vacation!
--
Thanks,
David / dhildenb
