diff mbox

[kvm-unit-tests,v2] s390x: IEP tests

Message ID 1528707706-16105-1-git-send-email-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Janosch Frank June 11, 2018, 9:01 a.m. UTC
From: Janosch Frank <frankja@linux.vnet.ibm.com>

Tests no-execute (Instruction Execution Protection) DAT protection.

Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
 lib/s390x/interrupt.c |  6 +++++
 lib/s390x/mmu.c       | 46 ++++++++++++++++++++++++++++++++++++--
 lib/s390x/mmu.h       | 20 +++++++++++++++++
 s390x/Makefile        |  1 +
 s390x/iep.c           | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
 s390x/unittests.cfg   |  4 ++++
 6 files changed, 137 insertions(+), 2 deletions(-)
 create mode 100644 lib/s390x/mmu.h
 create mode 100644 s390x/iep.c

Comments

David Hildenbrand June 11, 2018, 10:37 a.m. UTC | #1
On 11.06.2018 11:01, Janosch Frank wrote:
> From: Janosch Frank <frankja@linux.vnet.ibm.com>
> 
> Tests no-execute (Instruction Execution Protection) DAT protection.
> 
> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
>  lib/s390x/interrupt.c |  6 +++++
>  lib/s390x/mmu.c       | 46 ++++++++++++++++++++++++++++++++++++--
>  lib/s390x/mmu.h       | 20 +++++++++++++++++
>  s390x/Makefile        |  1 +
>  s390x/iep.c           | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg   |  4 ++++
>  6 files changed, 137 insertions(+), 2 deletions(-)
>  create mode 100644 lib/s390x/mmu.h
>  create mode 100644 s390x/iep.c
> 
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 56c7603..bc44e3a 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -51,6 +51,12 @@ static void fixup_pgm_int(void)
>  		 */
>  		lc->pgm_old_psw.mask &= ~PSW_MASK_PSTATE;
>  		break;
> +	case PGM_INT_CODE_PROTECTION:
> +		/* Handling for iep.c test case. */
> +		if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & 0x04UL &&
> +		    !(lc->trans_exc_id & 0x08UL))
> +			lc->pgm_old_psw.addr = lc->sw_int_grs[14];
> +		break;
>  	case PGM_INT_CODE_SEGMENT_TRANSLATION:
>  	case PGM_INT_CODE_PAGE_TRANSLATION:
>  	case PGM_INT_CODE_TRACE_TABLE:
> diff --git a/lib/s390x/mmu.c b/lib/s390x/mmu.c
> index 288f835..70753c3 100644
> --- a/lib/s390x/mmu.c
> +++ b/lib/s390x/mmu.c
> @@ -16,6 +16,8 @@
>  #include <asm/barrier.h>
>  #include <vmalloc.h>
>  
> +static pgd_t *table_root;
> +
>  void configure_dat(int enable)
>  {
>  	uint64_t mask;
> @@ -62,7 +64,7 @@ phys_addr_t virt_to_pte_phys(pgd_t *pgtable, void *vaddr)
>  	       ((unsigned long)vaddr & ~PAGE_MASK);
>  }
>  
> -pteval_t *install_page(pgd_t *pgtable, phys_addr_t phys, void *vaddr)
> +static pteval_t *set_pte(pgd_t *pgtable, pteval_t val, void *vaddr)
>  {
>  	pteval_t *p_pte = get_pte(pgtable, (uintptr_t)vaddr);
>  
> @@ -70,10 +72,49 @@ pteval_t *install_page(pgd_t *pgtable, phys_addr_t phys, void *vaddr)
>  	if (!(*p_pte & PAGE_ENTRY_I))
>  		ipte((uintptr_t)vaddr, p_pte);
>  
> -	*p_pte = __pa(phys);
> +	*p_pte = val;
>  	return p_pte;
>  }
>  
> +pteval_t *install_page(pgd_t *pgtable, phys_addr_t phys, void *vaddr)
> +{
> +	return set_pte(pgtable, __pa(phys), vaddr);
> +}
> +
> +void protect_page(void *vaddr, unsigned long prot)
> +{
> +	pteval_t *p_pte = get_pte(table_root, (uintptr_t)vaddr);
> +	pteval_t n_pte = *p_pte | prot;
> +
> +	set_pte(table_root, n_pte, vaddr);
> +}
> +
> +void unprotect_page(void *vaddr, unsigned long prot)
> +{
> +	pteval_t *p_pte = get_pte(table_root, (uintptr_t)vaddr);
> +	pteval_t n_pte = *p_pte & ~prot;
> +
> +	set_pte(table_root, n_pte, vaddr);
> +}
> +
> +void protect_range(void *start, unsigned long len, unsigned long prot)
> +{
> +	uintptr_t curr = (uintptr_t)start & PAGE_MASK;
> +
> +	len &= PAGE_MASK;
> +	for (; len; len -= PAGE_SIZE, curr += PAGE_SIZE)
> +		protect_page((void *)curr, prot);
> +}
> +
> +void unprotect_range(void *start, unsigned long len, unsigned long prot)
> +{
> +	uintptr_t curr = (uintptr_t)start & PAGE_MASK;
> +
> +	len &= PAGE_MASK;
> +	for (; len; len -= PAGE_SIZE, curr += PAGE_SIZE)
> +		unprotect_page((void *)curr, prot);
> +}
> +
>  static void setup_identity(pgd_t *pgtable, phys_addr_t start_addr,
>  			   phys_addr_t end_addr)
>  {
> @@ -104,5 +145,6 @@ void *setup_mmu(phys_addr_t phys_end){
>  
>  	/* finally enable DAT with the new table */
>  	mmu_enable(page_root);
> +	table_root = page_root;
>  	return page_root;
>  }
> diff --git a/lib/s390x/mmu.h b/lib/s390x/mmu.h
> new file mode 100644
> index 0000000..f5095fa
> --- /dev/null
> +++ b/lib/s390x/mmu.h
> @@ -0,0 +1,20 @@
> +/*
> + * s390x mmu functions
> + *
> + * Copyright (c) 2018 IBM Corp
> + *
> + * Authors:
> + *	Janosch Frank <frankja@de.ibm.com>
> + *
> + * 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.
> + */
> +#ifndef _ASMS390X_MMU_H_
> +#define _ASMS390X_MMU_H_
> +
> +void protect_page(void *vaddr, unsigned long prot);
> +void protect_range(void *start, unsigned long len, unsigned long prot);
> +void unprotect_page(void *vaddr, unsigned long prot);
> +void unprotect_range(void *start, unsigned long len, unsigned long prot);
> +
> +#endif /* _ASMS390X_MMU_H_ */
> 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..e4abc72
> --- /dev/null
> +++ b/s390x/iep.c
> @@ -0,0 +1,62 @@
> +/*
> + * Instruction Execution Prevention (IEP) DAT test.
> + *
> + * Copyright (c) 2018 IBM Corp
> + *
> + * Authors:
> + *	Janosch Frank <frankja@de.ibm.com>
> + *
> + * 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 <mmu.h>
> +#include <asm/pgtable.h>
> +#include <asm-generic/barrier.h>
> +
> +static void test_iep(void)
> +{
> +	uint16_t *code;
> +	uint8_t *iepbuf = NULL;
> +	void (*fn)(void);
> +
> +	/* Enable IEP */
> +	ctl_set_bit(0, 20);
> +
> +	/* Get and protect a page with the IEP bit */
> +	iepbuf = alloc_page();
> +	protect_page(iepbuf, PAGE_ENTRY_IEP);
> +
> +	/* Code branches into r14 which contains the return address. */
> +	code = (uint16_t *)iepbuf;
> +	*code = 0x07fe;
> +	fn = (void *)code;
> +
> +	expect_pgm_int();
> +	/* Jump into protected page */
> +	fn();
> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
> +	unprotect_page(iepbuf, PAGE_ENTRY_IEP);
> +	ctl_clear_bit(0, 20);
> +}
> +
> +int main(void)
> +{
> +	bool has_iep = test_facility(130);
> +
> +	report_prefix_push("iep");
> +	report_xfail("DAT IEP available", !has_iep, has_iep);
> +	if (!has_iep)
> +		goto done;
> +
> +	/* Setup DAT 1:1 mapping and memory management */
> +	setup_vm();
> +	test_iep();
> +
> +done:
> +	report_prefix_pop();
> +	return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index ff7eea1..760402e 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -55,3 +55,7 @@ file = vector.elf
>  
>  [gs]
>  file = gs.elf
> +
> +[iep]
> +file = iep.elf
> +
> 

Acked-by: David Hildenbrand <david@redhat.com>

We might hold the range protection functions back and include once we're
actually using them.

Are you planning to extend the IEP test case? (e.g. not a call into
protected memory but let code run into a protected page)

Instead of using a hand crafted function, you could use a real function,
make sure it sits on a single page (isolated from other code), and call
it before protection, after protection and after unprotection.

noinline __attribute__((aligned(4096)))
static void testf(void) {...}

noinline __attribute__((aligned(4096)))
static void test_iep(void)

I used something similar once to make sure my target code resides on a
separate page, maybe it could work here too.
Thomas Huth June 11, 2018, 10:41 a.m. UTC | #2
On 11.06.2018 11:01, Janosch Frank wrote:
> From: Janosch Frank <frankja@linux.vnet.ibm.com>
> 
> Tests no-execute (Instruction Execution Protection) DAT protection.
> 
> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> ---
>  lib/s390x/interrupt.c |  6 +++++
>  lib/s390x/mmu.c       | 46 ++++++++++++++++++++++++++++++++++++--
>  lib/s390x/mmu.h       | 20 +++++++++++++++++
>  s390x/Makefile        |  1 +
>  s390x/iep.c           | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  s390x/unittests.cfg   |  4 ++++
>  6 files changed, 137 insertions(+), 2 deletions(-)
>  create mode 100644 lib/s390x/mmu.h
>  create mode 100644 s390x/iep.c
> 
> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> index 56c7603..bc44e3a 100644
> --- a/lib/s390x/interrupt.c
> +++ b/lib/s390x/interrupt.c
> @@ -51,6 +51,12 @@ static void fixup_pgm_int(void)
>  		 */
>  		lc->pgm_old_psw.mask &= ~PSW_MASK_PSTATE;
>  		break;
> +	case PGM_INT_CODE_PROTECTION:
> +		/* Handling for iep.c test case. */
> +		if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & 0x04UL &&
> +		    !(lc->trans_exc_id & 0x08UL))

Nit / bikeshedding: It's IMHO a little bit nicer to test all three bits
at once:

    if ((lc->trans_exc_id & 0x8c) == 0x84)

[...]
> diff --git a/s390x/iep.c b/s390x/iep.c
> new file mode 100644
> index 0000000..e4abc72
> --- /dev/null
> +++ b/s390x/iep.c
> @@ -0,0 +1,62 @@
> +/*
> + * Instruction Execution Prevention (IEP) DAT test.
> + *
> + * Copyright (c) 2018 IBM Corp
> + *
> + * Authors:
> + *	Janosch Frank <frankja@de.ibm.com>
> + *
> + * 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 <mmu.h>
> +#include <asm/pgtable.h>
> +#include <asm-generic/barrier.h>
> +
> +static void test_iep(void)
> +{
> +	uint16_t *code;
> +	uint8_t *iepbuf = NULL;
> +	void (*fn)(void);
> +
> +	/* Enable IEP */
> +	ctl_set_bit(0, 20);
> +
> +	/* Get and protect a page with the IEP bit */
> +	iepbuf = alloc_page();
> +	protect_page(iepbuf, PAGE_ENTRY_IEP);
> +
> +	/* Code branches into r14 which contains the return address. */
> +	code = (uint16_t *)iepbuf;
> +	*code = 0x07fe;
> +	fn = (void *)code;

Not sure if I've got Christian's comment wrt to ipte right, but if I did
(Christian, please correct me if I'm wrong), I think it's better to move
the "protect_page(iepbuf, PAGE_ENTRY_IEP)" here, so that the ipte is
called after you've modified the contents of the page.

> +	expect_pgm_int();
> +	/* Jump into protected page */
> +	fn();
> +	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
> +	unprotect_page(iepbuf, PAGE_ENTRY_IEP);
> +	ctl_clear_bit(0, 20);
> +}
> +
> +int main(void)
> +{
> +	bool has_iep = test_facility(130);
> +
> +	report_prefix_push("iep");
> +	report_xfail("DAT IEP available", !has_iep, has_iep);
> +	if (!has_iep)
> +		goto done;
> +
> +	/* Setup DAT 1:1 mapping and memory management */
> +	setup_vm();
> +	test_iep();
> +
> +done:
> +	report_prefix_pop();
> +	return report_summary();
> +}
> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> index ff7eea1..760402e 100644
> --- a/s390x/unittests.cfg
> +++ b/s390x/unittests.cfg
> @@ -55,3 +55,7 @@ file = vector.elf
>  
>  [gs]
>  file = gs.elf
> +
> +[iep]
> +file = iep.elf
> +

Nit: "git am" complains about the trailing empty line here.
(No need to resend just because of this nit, but in case Christian
confirms the issue with ipte, please fix this as well).

 Thomas
David Hildenbrand June 11, 2018, 10:42 a.m. UTC | #3
>> +static void test_iep(void)
>> +{
>> +	uint16_t *code;
>> +	uint8_t *iepbuf = NULL;
>> +	void (*fn)(void);
>> +
>> +	/* Enable IEP */
>> +	ctl_set_bit(0, 20);
>> +
>> +	/* Get and protect a page with the IEP bit */
>> +	iepbuf = alloc_page();
>> +	protect_page(iepbuf, PAGE_ENTRY_IEP);
>> +
>> +	/* Code branches into r14 which contains the return address. */
>> +	code = (uint16_t *)iepbuf;
>> +	*code = 0x07fe;
>> +	fn = (void *)code;
> 
> Not sure if I've got Christian's comment wrt to ipte right, but if I did
> (Christian, please correct me if I'm wrong), I think it's better to move
> the "protect_page(iepbuf, PAGE_ENTRY_IEP)" here, so that the ipte is
> called after you've modified the contents of the page.

Why? When we replace the entry (protect), we do an ipte. Next access
will reload the right page table entry including protection. What am I
missing?
Christian Borntraeger June 11, 2018, 10:52 a.m. UTC | #4
On 06/11/2018 12:41 PM, Thomas Huth wrote:
> On 11.06.2018 11:01, Janosch Frank wrote:
>> From: Janosch Frank <frankja@linux.vnet.ibm.com>
>>
>> Tests no-execute (Instruction Execution Protection) DAT protection.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
>> ---
>>  lib/s390x/interrupt.c |  6 +++++
>>  lib/s390x/mmu.c       | 46 ++++++++++++++++++++++++++++++++++++--
>>  lib/s390x/mmu.h       | 20 +++++++++++++++++
>>  s390x/Makefile        |  1 +
>>  s390x/iep.c           | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  s390x/unittests.cfg   |  4 ++++
>>  6 files changed, 137 insertions(+), 2 deletions(-)
>>  create mode 100644 lib/s390x/mmu.h
>>  create mode 100644 s390x/iep.c
>>
>> diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
>> index 56c7603..bc44e3a 100644
>> --- a/lib/s390x/interrupt.c
>> +++ b/lib/s390x/interrupt.c
>> @@ -51,6 +51,12 @@ static void fixup_pgm_int(void)
>>  		 */
>>  		lc->pgm_old_psw.mask &= ~PSW_MASK_PSTATE;
>>  		break;
>> +	case PGM_INT_CODE_PROTECTION:
>> +		/* Handling for iep.c test case. */
>> +		if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & 0x04UL &&
>> +		    !(lc->trans_exc_id & 0x08UL))
> 
> Nit / bikeshedding: It's IMHO a little bit nicer to test all three bits
> at once:
> 
>     if ((lc->trans_exc_id & 0x8c) == 0x84)
> 
> [...]
>> diff --git a/s390x/iep.c b/s390x/iep.c
>> new file mode 100644
>> index 0000000..e4abc72
>> --- /dev/null
>> +++ b/s390x/iep.c
>> @@ -0,0 +1,62 @@
>> +/*
>> + * Instruction Execution Prevention (IEP) DAT test.
>> + *
>> + * Copyright (c) 2018 IBM Corp
>> + *
>> + * Authors:
>> + *	Janosch Frank <frankja@de.ibm.com>
>> + *
>> + * 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 <mmu.h>
>> +#include <asm/pgtable.h>
>> +#include <asm-generic/barrier.h>
>> +
>> +static void test_iep(void)
>> +{
>> +	uint16_t *code;
>> +	uint8_t *iepbuf = NULL;
>> +	void (*fn)(void);
>> +
>> +	/* Enable IEP */
>> +	ctl_set_bit(0, 20);
>> +
>> +	/* Get and protect a page with the IEP bit */
>> +	iepbuf = alloc_page();
>> +	protect_page(iepbuf, PAGE_ENTRY_IEP);
>> +
>> +	/* Code branches into r14 which contains the return address. */
>> +	code = (uint16_t *)iepbuf;
>> +	*code = 0x07fe;
>> +	fn = (void *)code;
> 
> Not sure if I've got Christian's comment wrt to ipte right, but if I did
> (Christian, please correct me if I'm wrong), I think it's better to move
> the "protect_page(iepbuf, PAGE_ENTRY_IEP)" here, so that the ipte is
> called after you've modified the contents of the page.

Not necessary. As I said we only need to flush the TLB while changing the 
page table. The page content is fine as is.
Thomas Huth June 11, 2018, 11:15 a.m. UTC | #5
On 11.06.2018 12:42, David Hildenbrand wrote:
> 
>>> +static void test_iep(void)
>>> +{
>>> +	uint16_t *code;
>>> +	uint8_t *iepbuf = NULL;
>>> +	void (*fn)(void);
>>> +
>>> +	/* Enable IEP */
>>> +	ctl_set_bit(0, 20);
>>> +
>>> +	/* Get and protect a page with the IEP bit */
>>> +	iepbuf = alloc_page();
>>> +	protect_page(iepbuf, PAGE_ENTRY_IEP);
>>> +
>>> +	/* Code branches into r14 which contains the return address. */
>>> +	code = (uint16_t *)iepbuf;
>>> +	*code = 0x07fe;
>>> +	fn = (void *)code;
>>
>> Not sure if I've got Christian's comment wrt to ipte right, but if I did
>> (Christian, please correct me if I'm wrong), I think it's better to move
>> the "protect_page(iepbuf, PAGE_ENTRY_IEP)" here, so that the ipte is
>> called after you've modified the contents of the page.
> 
> Why? When we replace the entry (protect), we do an ipte. Next access
> will reload the right page table entry including protection. What am I
> missing?

Never mind, as explained by Christian, it's not necessary here. I'm just
too paranoid after running into cache-flush issues on ppc in the past ;-)

 Thomas
Janosch Frank June 11, 2018, 12:29 p.m. UTC | #6
On 11.06.2018 12:37, David Hildenbrand wrote:
[...]
>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>> index ff7eea1..760402e 100644
>> --- a/s390x/unittests.cfg
>> +++ b/s390x/unittests.cfg
>> @@ -55,3 +55,7 @@ file = vector.elf
>>  
>>  [gs]
>>  file = gs.elf
>> +
>> +[iep]
>> +file = iep.elf
>> +
>>
> 
> Acked-by: David Hildenbrand <david@redhat.com>

Thanks

> 
> We might hold the range protection functions back and include once we're
> actually using them.

I'm currently not too happy to have the protection functions in mmu.c
and tracking our current root table together with the general setup
functions. It seems like we mix functionality, but I also don't want the
test(s) to retrieve the ASCE and then pass it to the protection functions.

> 
> Are you planning to extend the IEP test case? (e.g. not a call into
> protected memory but let code run into a protected page)

Not currently, but that sounds like a nice extension.
For now I see no real use-case, as this is hardware functionality and I
assume, that someone already tests this. I just want to make sure, that
it works, if the stfle bit is set.

> 
> Instead of using a hand crafted function, you could use a real function,
> make sure it sits on a single page (isolated from other code), and call
> it before protection, after protection and after unprotection.
> 
> noinline __attribute__((aligned(4096)))
> static void testf(void) {...}
> 
> noinline __attribute__((aligned(4096)))
> static void test_iep(void)
> 
> I used something similar once to make sure my target code resides on a
> separate page, maybe it could work here too.
David Hildenbrand June 11, 2018, 12:31 p.m. UTC | #7
On 11.06.2018 14:29, Janosch Frank wrote:
> On 11.06.2018 12:37, David Hildenbrand wrote:
> [...]
>>> diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
>>> index ff7eea1..760402e 100644
>>> --- a/s390x/unittests.cfg
>>> +++ b/s390x/unittests.cfg
>>> @@ -55,3 +55,7 @@ file = vector.elf
>>>  
>>>  [gs]
>>>  file = gs.elf
>>> +
>>> +[iep]
>>> +file = iep.elf
>>> +
>>>
>>
>> Acked-by: David Hildenbrand <david@redhat.com>
> 
> Thanks
> 
>>
>> We might hold the range protection functions back and include once we're
>> actually using them.
> 
> I'm currently not too happy to have the protection functions in mmu.c
> and tracking our current root table together with the general setup
> functions. It seems like we mix functionality, but I also don't want the
> test(s) to retrieve the ASCE and then pass it to the protection functions.
> 
>>
>> Are you planning to extend the IEP test case? (e.g. not a call into
>> protected memory but let code run into a protected page)
> 
> Not currently, but that sounds like a nice extension.
> For now I see no real use-case, as this is hardware functionality and I
> assume, that someone already tests this. I just want to make sure, that
> it works, if the stfle bit is set.

Guess I'm thinking too TCGy :)

> 
>>
>> Instead of using a hand crafted function, you could use a real function,
>> make sure it sits on a single page (isolated from other code), and call
>> it before protection, after protection and after unprotection.
>>
>> noinline __attribute__((aligned(4096)))
>> static void testf(void) {...}
>>
>> noinline __attribute__((aligned(4096)))
>> static void test_iep(void)
>>
>> I used something similar once to make sure my target code resides on a
>> separate page, maybe it could work here too.
> 
>
diff mbox

Patch

diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
index 56c7603..bc44e3a 100644
--- a/lib/s390x/interrupt.c
+++ b/lib/s390x/interrupt.c
@@ -51,6 +51,12 @@  static void fixup_pgm_int(void)
 		 */
 		lc->pgm_old_psw.mask &= ~PSW_MASK_PSTATE;
 		break;
+	case PGM_INT_CODE_PROTECTION:
+		/* Handling for iep.c test case. */
+		if (lc->trans_exc_id & 0x80UL && lc->trans_exc_id & 0x04UL &&
+		    !(lc->trans_exc_id & 0x08UL))
+			lc->pgm_old_psw.addr = lc->sw_int_grs[14];
+		break;
 	case PGM_INT_CODE_SEGMENT_TRANSLATION:
 	case PGM_INT_CODE_PAGE_TRANSLATION:
 	case PGM_INT_CODE_TRACE_TABLE:
diff --git a/lib/s390x/mmu.c b/lib/s390x/mmu.c
index 288f835..70753c3 100644
--- a/lib/s390x/mmu.c
+++ b/lib/s390x/mmu.c
@@ -16,6 +16,8 @@ 
 #include <asm/barrier.h>
 #include <vmalloc.h>
 
+static pgd_t *table_root;
+
 void configure_dat(int enable)
 {
 	uint64_t mask;
@@ -62,7 +64,7 @@  phys_addr_t virt_to_pte_phys(pgd_t *pgtable, void *vaddr)
 	       ((unsigned long)vaddr & ~PAGE_MASK);
 }
 
-pteval_t *install_page(pgd_t *pgtable, phys_addr_t phys, void *vaddr)
+static pteval_t *set_pte(pgd_t *pgtable, pteval_t val, void *vaddr)
 {
 	pteval_t *p_pte = get_pte(pgtable, (uintptr_t)vaddr);
 
@@ -70,10 +72,49 @@  pteval_t *install_page(pgd_t *pgtable, phys_addr_t phys, void *vaddr)
 	if (!(*p_pte & PAGE_ENTRY_I))
 		ipte((uintptr_t)vaddr, p_pte);
 
-	*p_pte = __pa(phys);
+	*p_pte = val;
 	return p_pte;
 }
 
+pteval_t *install_page(pgd_t *pgtable, phys_addr_t phys, void *vaddr)
+{
+	return set_pte(pgtable, __pa(phys), vaddr);
+}
+
+void protect_page(void *vaddr, unsigned long prot)
+{
+	pteval_t *p_pte = get_pte(table_root, (uintptr_t)vaddr);
+	pteval_t n_pte = *p_pte | prot;
+
+	set_pte(table_root, n_pte, vaddr);
+}
+
+void unprotect_page(void *vaddr, unsigned long prot)
+{
+	pteval_t *p_pte = get_pte(table_root, (uintptr_t)vaddr);
+	pteval_t n_pte = *p_pte & ~prot;
+
+	set_pte(table_root, n_pte, vaddr);
+}
+
+void protect_range(void *start, unsigned long len, unsigned long prot)
+{
+	uintptr_t curr = (uintptr_t)start & PAGE_MASK;
+
+	len &= PAGE_MASK;
+	for (; len; len -= PAGE_SIZE, curr += PAGE_SIZE)
+		protect_page((void *)curr, prot);
+}
+
+void unprotect_range(void *start, unsigned long len, unsigned long prot)
+{
+	uintptr_t curr = (uintptr_t)start & PAGE_MASK;
+
+	len &= PAGE_MASK;
+	for (; len; len -= PAGE_SIZE, curr += PAGE_SIZE)
+		unprotect_page((void *)curr, prot);
+}
+
 static void setup_identity(pgd_t *pgtable, phys_addr_t start_addr,
 			   phys_addr_t end_addr)
 {
@@ -104,5 +145,6 @@  void *setup_mmu(phys_addr_t phys_end){
 
 	/* finally enable DAT with the new table */
 	mmu_enable(page_root);
+	table_root = page_root;
 	return page_root;
 }
diff --git a/lib/s390x/mmu.h b/lib/s390x/mmu.h
new file mode 100644
index 0000000..f5095fa
--- /dev/null
+++ b/lib/s390x/mmu.h
@@ -0,0 +1,20 @@ 
+/*
+ * s390x mmu functions
+ *
+ * Copyright (c) 2018 IBM Corp
+ *
+ * Authors:
+ *	Janosch Frank <frankja@de.ibm.com>
+ *
+ * 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.
+ */
+#ifndef _ASMS390X_MMU_H_
+#define _ASMS390X_MMU_H_
+
+void protect_page(void *vaddr, unsigned long prot);
+void protect_range(void *start, unsigned long len, unsigned long prot);
+void unprotect_page(void *vaddr, unsigned long prot);
+void unprotect_range(void *start, unsigned long len, unsigned long prot);
+
+#endif /* _ASMS390X_MMU_H_ */
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..e4abc72
--- /dev/null
+++ b/s390x/iep.c
@@ -0,0 +1,62 @@ 
+/*
+ * Instruction Execution Prevention (IEP) DAT test.
+ *
+ * Copyright (c) 2018 IBM Corp
+ *
+ * Authors:
+ *	Janosch Frank <frankja@de.ibm.com>
+ *
+ * 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 <mmu.h>
+#include <asm/pgtable.h>
+#include <asm-generic/barrier.h>
+
+static void test_iep(void)
+{
+	uint16_t *code;
+	uint8_t *iepbuf = NULL;
+	void (*fn)(void);
+
+	/* Enable IEP */
+	ctl_set_bit(0, 20);
+
+	/* Get and protect a page with the IEP bit */
+	iepbuf = alloc_page();
+	protect_page(iepbuf, PAGE_ENTRY_IEP);
+
+	/* Code branches into r14 which contains the return address. */
+	code = (uint16_t *)iepbuf;
+	*code = 0x07fe;
+	fn = (void *)code;
+
+	expect_pgm_int();
+	/* Jump into protected page */
+	fn();
+	check_pgm_int_code(PGM_INT_CODE_PROTECTION);
+	unprotect_page(iepbuf, PAGE_ENTRY_IEP);
+	ctl_clear_bit(0, 20);
+}
+
+int main(void)
+{
+	bool has_iep = test_facility(130);
+
+	report_prefix_push("iep");
+	report_xfail("DAT IEP available", !has_iep, has_iep);
+	if (!has_iep)
+		goto done;
+
+	/* Setup DAT 1:1 mapping and memory management */
+	setup_vm();
+	test_iep();
+
+done:
+	report_prefix_pop();
+	return report_summary();
+}
diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
index ff7eea1..760402e 100644
--- a/s390x/unittests.cfg
+++ b/s390x/unittests.cfg
@@ -55,3 +55,7 @@  file = vector.elf
 
 [gs]
 file = gs.elf
+
+[iep]
+file = iep.elf
+