On 11.06.2018 09:45, Janosch Frank wrote:
> On 11.06.2018 09:34, Thomas Huth wrote:
>> On 08.06.2018 16:01, Janosch Frank wrote:
>>> From: Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
>>>
>>> Tests no-execute (Instruction Execution Protection) DAT protection.
>>>
>>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
>>> ---
>>> lib/s390x/interrupt.c | 4 ++++
>>> lib/s390x/mmu.c | 46 ++++++++++++++++++++++++++++++++++--
>>> lib/s390x/mmu.h | 20 ++++++++++++++++
>>> s390x/Makefile | 1 +
>>> s390x/iep.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>> s390x/unittests.cfg | 4 ++++
>>> 6 files changed, 138 insertions(+), 2 deletions(-)
>>> create mode 100644 lib/s390x/mmu.h
>>> create mode 100644 s390x/iep.c
[...]
>>> + 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);
>>> + mb();
>>> +
>>> + /* Code branches into r14 which contains the return address. */
>>> + code = (uint16_t *)iepbuf;
>>> + *code = 0x07fe;
>>> + asm volatile("" : : "m" (code));
>>
>> What's this asm-volatile statement here good for? Mybe add a comment to
>> the code?
>>
>>> + fn = (void *)code;
>>> + mb();
>>
>> (having done a lot of ppc stuff in the past, let me ask just to be sure:
>> We don't have to flush the data cache on s390x before doing stuff like
>> this, do we?)
>
> This was not done out of knowledge, I had some crashes without the
> barriers a few weeks ago and sprinkled mbs over the code until it
> worked. Some problems I figured out later and I removed the barriers, I
> haven't yet tested if I can remove this one.
The mb() is fine here, I think. I just wondered whether an additional
cache flush is necessary (it would be necessary on ppc for example). But
I guess s390x is strict enough that you don't need an additional cache
flush here.
Thomas
Attachment:
signature.asc
Description: OpenPGP digital signature
