On 06/11/2018 09:58 AM, Thomas Huth wrote:
> 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.
There is no cache flush for s390. We only need to care about flushing the
old TLB (so what needs to be done is an ipte (seems that install_page does
that)
