[v2,2/3] lkdtm/powerpc: Add SLB multihit test

Add support to inject slb multihit errors, to test machine
check handling.

Based on work by Mahesh Salgaonkar and Michal Suchánek.

Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Cc: Michal Suchánek <msuchanek@suse.de>
Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
---
 drivers/misc/lkdtm/Makefile  |   4 ++
 drivers/misc/lkdtm/core.c    |   3 +
 drivers/misc/lkdtm/lkdtm.h   |   3 +
 drivers/misc/lkdtm/powerpc.c | 132 +++++++++++++++++++++++++++++++++++
 4 files changed, 142 insertions(+)
 create mode 100644 drivers/misc/lkdtm/powerpc.c

Hi Ganesh,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on powerpc/next v5.9-rc6 next-20200925]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ganesh-Goudar/powerpc-mce-Fix-mce-handler-and-add-selftest/20200925-184048
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git 9eb29f2ed95edda511ce28651b1d7cdef3614c12
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c066c175c2699a6aec1b0a25f6b95746590d802a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ganesh-Goudar/powerpc-mce-Fix-mce-handler-and-add-selftest/20200925-184048
        git checkout c066c175c2699a6aec1b0a25f6b95746590d802a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/misc/lkdtm/powerpc.c:124:6: warning: no previous prototype for 'lkdtm_PPC_SLB_MULTIHIT' [-Wmissing-prototypes]
     124 | void lkdtm_PPC_SLB_MULTIHIT(void)
         |      ^~~~~~~~~~~~~~~~~~~~~~

vim +/lkdtm_PPC_SLB_MULTIHIT +124 drivers/misc/lkdtm/powerpc.c

   123	
 > 124	void lkdtm_PPC_SLB_MULTIHIT(void)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

On Fri, Sep 25, 2020 at 04:01:22PM +0530, Ganesh Goudar wrote:
> Add support to inject slb multihit errors, to test machine
> check handling.

Thank you for more tests in here!

> 
> Based on work by Mahesh Salgaonkar and Michal Suchánek.
> 
> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Cc: Michal Suchánek <msuchanek@suse.de>

Should these be Co-developed-by: with S-o-b?

> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
> ---
>  drivers/misc/lkdtm/Makefile  |   4 ++
>  drivers/misc/lkdtm/core.c    |   3 +
>  drivers/misc/lkdtm/lkdtm.h   |   3 +
>  drivers/misc/lkdtm/powerpc.c | 132 +++++++++++++++++++++++++++++++++++
>  4 files changed, 142 insertions(+)
>  create mode 100644 drivers/misc/lkdtm/powerpc.c
> 
> diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
> index c70b3822013f..6a82f407fbcd 100644
> --- a/drivers/misc/lkdtm/Makefile
> +++ b/drivers/misc/lkdtm/Makefile
> @@ -11,6 +11,10 @@ lkdtm-$(CONFIG_LKDTM)		+= usercopy.o
>  lkdtm-$(CONFIG_LKDTM)		+= stackleak.o
>  lkdtm-$(CONFIG_LKDTM)		+= cfi.o
>  
> +ifeq ($(CONFIG_PPC64),y)
> +lkdtm-$(CONFIG_LKDTM)		+= powerpc.o
> +endif

This can just be:

lkdtm-$(CONFIG_PPC64)		+= powerpc.o

> +
>  KASAN_SANITIZE_stackleak.o	:= n
>  KCOV_INSTRUMENT_rodata.o	:= n
>  
> diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
> index a5e344df9166..8d5db42baa90 100644
> --- a/drivers/misc/lkdtm/core.c
> +++ b/drivers/misc/lkdtm/core.c
> @@ -178,6 +178,9 @@ static const struct crashtype crashtypes[] = {
>  #ifdef CONFIG_X86_32
>  	CRASHTYPE(DOUBLE_FAULT),
>  #endif
> +#ifdef CONFIG_PPC64
> +	CRASHTYPE(PPC_SLB_MULTIHIT),
> +#endif
>  };
>  
>  
> diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
> index 8878538b2c13..b305bd511ee5 100644
> --- a/drivers/misc/lkdtm/lkdtm.h
> +++ b/drivers/misc/lkdtm/lkdtm.h
> @@ -104,4 +104,7 @@ void lkdtm_STACKLEAK_ERASING(void);
>  /* cfi.c */
>  void lkdtm_CFI_FORWARD_PROTO(void);
>  
> +/* powerpc.c */
> +void lkdtm_PPC_SLB_MULTIHIT(void);
> +
>  #endif
> diff --git a/drivers/misc/lkdtm/powerpc.c b/drivers/misc/lkdtm/powerpc.c
> new file mode 100644
> index 000000000000..d6db18444757
> --- /dev/null
> +++ b/drivers/misc/lkdtm/powerpc.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0
> +

Please #include "lkdtm.h" here to get the correct pr_fmt heading (and
any future header adjustments).

> +#include <linux/slab.h>
> +#include <linux/vmalloc.h>
> +
> +static inline unsigned long get_slb_index(void)
> +{
> +	unsigned long index;
> +
> +	index = get_paca()->stab_rr;
> +
> +	/*
> +	 * simple round-robin replacement of slb starting at SLB_NUM_BOLTED.
> +	 */
> +	if (index < (mmu_slb_size - 1))
> +		index++;
> +	else
> +		index = SLB_NUM_BOLTED;
> +	get_paca()->stab_rr = index;
> +	return index;
> +}
> +
> +#define slb_esid_mask(ssize)	\
> +	(((ssize) == MMU_SEGSIZE_256M) ? ESID_MASK : ESID_MASK_1T)
> +
> +static inline unsigned long mk_esid_data(unsigned long ea, int ssize,
> +					 unsigned long slot)
> +{
> +	return (ea & slb_esid_mask(ssize)) | SLB_ESID_V | slot;
> +}
> +
> +#define slb_vsid_shift(ssize)	\
> +	((ssize) == MMU_SEGSIZE_256M ? SLB_VSID_SHIFT : SLB_VSID_SHIFT_1T)
> +
> +static inline unsigned long mk_vsid_data(unsigned long ea, int ssize,
> +					 unsigned long flags)
> +{
> +	return (get_kernel_vsid(ea, ssize) << slb_vsid_shift(ssize)) | flags |
> +		((unsigned long)ssize << SLB_VSID_SSIZE_SHIFT);
> +}
> +
> +static void insert_slb_entry(char *p, int ssize)
> +{
> +	unsigned long flags, entry;
> +
> +	flags = SLB_VSID_KERNEL | mmu_psize_defs[MMU_PAGE_64K].sllp;
> +	preempt_disable();
> +
> +	entry = get_slb_index();
> +	asm volatile("slbmte %0,%1" :
> +			: "r" (mk_vsid_data((unsigned long)p, ssize, flags)),
> +			  "r" (mk_esid_data((unsigned long)p, ssize, entry))
> +			: "memory");
> +
> +	entry = get_slb_index();
> +	asm volatile("slbmte %0,%1" :
> +			: "r" (mk_vsid_data((unsigned long)p, ssize, flags)),
> +			  "r" (mk_esid_data((unsigned long)p, ssize, entry))
> +			: "memory");
> +	preempt_enable();
> +	p[0] = '!';
> +}

Can you add some comments to these helpers? It'll help people (me) with
understanding what is actually being done here (and more importantly,
what is _expected_ to happen).

> +
> +static void inject_vmalloc_slb_multihit(void)
> +{
> +	char *p;
> +
> +	p = vmalloc(2048);
> +	if (!p)
> +		return;
> +
> +	insert_slb_entry(p, MMU_SEGSIZE_1T);
> +	vfree(p);
> +}
> +
> +static void inject_kmalloc_slb_multihit(void)
> +{
> +	char *p;
> +
> +	p = kmalloc(2048, GFP_KERNEL);
> +	if (!p)
> +		return;
> +
> +	insert_slb_entry(p, MMU_SEGSIZE_1T);
> +	kfree(p);
> +}

It looks like the expected failure injection is actually the "p[0] = '!'" line in the
"insert" helper? I would expect pr_info/pr_err wrappers, etc, as in
other lkdtm tests.

If this is the negative test, what does the positive test look like?
e.g. in other lkdtm tests, first a positive test is done, then a
negative: first run a legit function, then run a function from a bad
location.

> +
> +static void insert_dup_slb_entry_0(void)
> +{
> +	unsigned long test_address = 0xC000000000000000;
> +	volatile unsigned long *test_ptr;
> +	unsigned long entry, i = 0;
> +	unsigned long esid, vsid;
> +
> +	test_ptr = (unsigned long *)test_address;
> +	preempt_disable();
> +
> +	asm volatile("slbmfee  %0,%1" : "=r" (esid) : "r" (i));
> +	asm volatile("slbmfev  %0,%1" : "=r" (vsid) : "r" (i));
> +	entry = get_slb_index();
> +
> +	/* for i !=0 we would need to mask out the old entry number */
> +	asm volatile("slbmte %0,%1" :
> +			: "r" (vsid),
> +			  "r" (esid | entry)
> +			: "memory");
> +
> +	asm volatile("slbmfee  %0,%1" : "=r" (esid) : "r" (i));
> +	asm volatile("slbmfev  %0,%1" : "=r" (vsid) : "r" (i));
> +	entry = get_slb_index();
> +
> +	/* for i !=0 we would need to mask out the old entry number */
> +	asm volatile("slbmte %0,%1" :
> +			: "r" (vsid),
> +			  "r" (esid | entry)
> +			: "memory");
> +
> +	pr_info("lkdtm: %s accessing test address 0x%lx: 0x%lx\n",
> +		__func__, test_address, *test_ptr);
> +
> +	preempt_enable();
> +}

What does this do?

> +
> +void lkdtm_PPC_SLB_MULTIHIT(void)
> +{
> +	if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
> +		inject_vmalloc_slb_multihit();
> +		inject_kmalloc_slb_multihit();
> +		insert_dup_slb_entry_0();
> +	}
> +	pr_info("Recovered from SLB multihit. (Ignore this message on non HPTE machines)");

Is this bad? If so, I'd expect pr_err("FAIL: ...") Can HPTE machines be
detected so that an XFAIL can be emitted instead?

Since there are three (two?) distinct regions being tested, should these
be separate tests? Right now there is no way to separate a vmalloc
failure from a kmalloc failure, and no way to fail the first without
hiding the result from the latter (or maybe the machine cannot survive
this test? ... which should also be a comment.)

And finally, assuming a successful test (or testing from a separate
thread later), so there any state that needs to be restored (or cleaned
up before doing the "insert" calls)?

Thanks!

On 9/26/20 1:27 AM, Kees Cook wrote:
> On Fri, Sep 25, 2020 at 04:01:22PM +0530, Ganesh Goudar wrote:
>> Add support to inject slb multihit errors, to test machine
>> check handling.
> Thank you for more tests in here!
>
>> Based on work by Mahesh Salgaonkar and Michal Suchánek.
>>
>> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
>> Cc: Michal Suchánek <msuchanek@suse.de>
> Should these be Co-developed-by: with S-o-b?
Sure, ill add.
>> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
>> ---
>>   drivers/misc/lkdtm/Makefile  |   4 ++
>>   drivers/misc/lkdtm/core.c    |   3 +
>>   drivers/misc/lkdtm/lkdtm.h   |   3 +
>>   drivers/misc/lkdtm/powerpc.c | 132 +++++++++++++++++++++++++++++++++++
>>   4 files changed, 142 insertions(+)
>>   create mode 100644 drivers/misc/lkdtm/powerpc.c
>>
>> diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
>> index c70b3822013f..6a82f407fbcd 100644
>> --- a/drivers/misc/lkdtm/Makefile
>> +++ b/drivers/misc/lkdtm/Makefile
>> @@ -11,6 +11,10 @@ lkdtm-$(CONFIG_LKDTM)		+= usercopy.o
>>   lkdtm-$(CONFIG_LKDTM)		+= stackleak.o
>>   lkdtm-$(CONFIG_LKDTM)		+= cfi.o
>>   
>> +ifeq ($(CONFIG_PPC64),y)
>> +lkdtm-$(CONFIG_LKDTM)		+= powerpc.o
>> +endif
> This can just be:
>
> lkdtm-$(CONFIG_PPC64)		+= powerpc.o
ok
>> +
>>   KASAN_SANITIZE_stackleak.o	:= n
>>   KCOV_INSTRUMENT_rodata.o	:= n
>>   
>> diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
>> index a5e344df9166..8d5db42baa90 100644
>> --- a/drivers/misc/lkdtm/core.c
>> +++ b/drivers/misc/lkdtm/core.c
>> @@ -178,6 +178,9 @@ static const struct crashtype crashtypes[] = {
>>   #ifdef CONFIG_X86_32
>>   	CRASHTYPE(DOUBLE_FAULT),
>>   #endif
>> +#ifdef CONFIG_PPC64
>> +	CRASHTYPE(PPC_SLB_MULTIHIT),
>> +#endif
>>   };
>>   
>>   
>> diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
>> index 8878538b2c13..b305bd511ee5 100644
>> --- a/drivers/misc/lkdtm/lkdtm.h
>> +++ b/drivers/misc/lkdtm/lkdtm.h
>> @@ -104,4 +104,7 @@ void lkdtm_STACKLEAK_ERASING(void);
>>   /* cfi.c */
>>   void lkdtm_CFI_FORWARD_PROTO(void);
>>   
>> +/* powerpc.c */
>> +void lkdtm_PPC_SLB_MULTIHIT(void);
>> +
>>   #endif
>> diff --git a/drivers/misc/lkdtm/powerpc.c b/drivers/misc/lkdtm/powerpc.c
>> new file mode 100644
>> index 000000000000..d6db18444757
>> --- /dev/null
>> +++ b/drivers/misc/lkdtm/powerpc.c
>> @@ -0,0 +1,132 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
> Please #include "lkdtm.h" here to get the correct pr_fmt heading (and
> any future header adjustments).
Sure
>> +#include <linux/slab.h>
>> +#include <linux/vmalloc.h>
>> +
>> +static inline unsigned long get_slb_index(void)
>> +{
>> +	unsigned long index;
>> +
>> +	index = get_paca()->stab_rr;
>> +
>> +	/*
>> +	 * simple round-robin replacement of slb starting at SLB_NUM_BOLTED.
>> +	 */
>> +	if (index < (mmu_slb_size - 1))
>> +		index++;
>> +	else
>> +		index = SLB_NUM_BOLTED;
>> +	get_paca()->stab_rr = index;
>> +	return index;
>> +}
>> +
>> +#define slb_esid_mask(ssize)	\
>> +	(((ssize) == MMU_SEGSIZE_256M) ? ESID_MASK : ESID_MASK_1T)
>> +
>> +static inline unsigned long mk_esid_data(unsigned long ea, int ssize,
>> +					 unsigned long slot)
>> +{
>> +	return (ea & slb_esid_mask(ssize)) | SLB_ESID_V | slot;
>> +}
>> +
>> +#define slb_vsid_shift(ssize)	\
>> +	((ssize) == MMU_SEGSIZE_256M ? SLB_VSID_SHIFT : SLB_VSID_SHIFT_1T)
>> +
>> +static inline unsigned long mk_vsid_data(unsigned long ea, int ssize,
>> +					 unsigned long flags)
>> +{
>> +	return (get_kernel_vsid(ea, ssize) << slb_vsid_shift(ssize)) | flags |
>> +		((unsigned long)ssize << SLB_VSID_SSIZE_SHIFT);
>> +}
>> +
>> +static void insert_slb_entry(char *p, int ssize)
>> +{
>> +	unsigned long flags, entry;
>> +
>> +	flags = SLB_VSID_KERNEL | mmu_psize_defs[MMU_PAGE_64K].sllp;
>> +	preempt_disable();
>> +
>> +	entry = get_slb_index();
>> +	asm volatile("slbmte %0,%1" :
>> +			: "r" (mk_vsid_data((unsigned long)p, ssize, flags)),
>> +			  "r" (mk_esid_data((unsigned long)p, ssize, entry))
>> +			: "memory");
>> +
>> +	entry = get_slb_index();
>> +	asm volatile("slbmte %0,%1" :
>> +			: "r" (mk_vsid_data((unsigned long)p, ssize, flags)),
>> +			  "r" (mk_esid_data((unsigned long)p, ssize, entry))
>> +			: "memory");
>> +	preempt_enable();
>> +	p[0] = '!';
>> +}
> Can you add some comments to these helpers? It'll help people (me) with
> understanding what is actually being done here (and more importantly,
> what is _expected_ to happen).
Sure, ill add comments.
>> +
>> +static void inject_vmalloc_slb_multihit(void)
>> +{
>> +	char *p;
>> +
>> +	p = vmalloc(2048);
>> +	if (!p)
>> +		return;
>> +
>> +	insert_slb_entry(p, MMU_SEGSIZE_1T);
>> +	vfree(p);
>> +}
>> +
>> +static void inject_kmalloc_slb_multihit(void)
>> +{
>> +	char *p;
>> +
>> +	p = kmalloc(2048, GFP_KERNEL);
>> +	if (!p)
>> +		return;
>> +
>> +	insert_slb_entry(p, MMU_SEGSIZE_1T);
>> +	kfree(p);
>> +}
> It looks like the expected failure injection is actually the "p[0] = '!'" line in the
> "insert" helper? I would expect pr_info/pr_err wrappers, etc, as in
> other lkdtm tests.
Yes "p[0] = '!'" is the final step in this error injection, ill add 
comments.
> If this is the negative test, what does the positive test look like?
> e.g. in other lkdtm tests, first a positive test is done, then a
> negative: first run a legit function, then run a function from a bad
> location.

Yes, this is negative test, As SLB search is very fundamental thing in 
address translation and

used all the time, no positive test required here.

>> +
>> +static void insert_dup_slb_entry_0(void)
>> +{
>> +	unsigned long test_address = 0xC000000000000000;
>> +	volatile unsigned long *test_ptr;
>> +	unsigned long entry, i = 0;
>> +	unsigned long esid, vsid;
>> +
>> +	test_ptr = (unsigned long *)test_address;
>> +	preempt_disable();
>> +
>> +	asm volatile("slbmfee  %0,%1" : "=r" (esid) : "r" (i));
>> +	asm volatile("slbmfev  %0,%1" : "=r" (vsid) : "r" (i));
>> +	entry = get_slb_index();
>> +
>> +	/* for i !=0 we would need to mask out the old entry number */
>> +	asm volatile("slbmte %0,%1" :
>> +			: "r" (vsid),
>> +			  "r" (esid | entry)
>> +			: "memory");
>> +
>> +	asm volatile("slbmfee  %0,%1" : "=r" (esid) : "r" (i));
>> +	asm volatile("slbmfev  %0,%1" : "=r" (vsid) : "r" (i));
>> +	entry = get_slb_index();
>> +
>> +	/* for i !=0 we would need to mask out the old entry number */
>> +	asm volatile("slbmte %0,%1" :
>> +			: "r" (vsid),
>> +			  "r" (esid | entry)
>> +			: "memory");
>> +
>> +	pr_info("lkdtm: %s accessing test address 0x%lx: 0x%lx\n",
>> +		__func__, test_address, *test_ptr);
>> +
>> +	preempt_enable();
>> +}
> What does this do?

It tries to inject error in address range where most important kernel 
data structure may fall.

Mahesh and Michal Suchánek correct me if I am wrong.

>> +
>> +void lkdtm_PPC_SLB_MULTIHIT(void)
>> +{
>> +	if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
>> +		inject_vmalloc_slb_multihit();
>> +		inject_kmalloc_slb_multihit();
>> +		insert_dup_slb_entry_0();
>> +	}
>> +	pr_info("Recovered from SLB multihit. (Ignore this message on non HPTE machines)");
> Is this bad? If so, I'd expect pr_err("FAIL: ...") Can HPTE machines be
> detected so that an XFAIL can be emitted instead?

Sure, ill add XFAIL print in non HPTE case. And in case of failure we 
will not return

here, so if we hit this print line we are good.

> Since there are three (two?) distinct regions being tested, should these
> be separate tests? Right now there is no way to separate a vmalloc
> failure from a kmalloc failure, and no way to fail the first without
> hiding the result from the latter (or maybe the machine cannot survive
> this test? ... which should also be a comment.)
Sure, ill comment, And yes machine cannot survive these tests in case of 
failure to handle.
> And finally, assuming a successful test (or testing from a separate
> thread later), so there any state that needs to be restored (or cleaned
> up before doing the "insert" calls)?
No, there is nothing to be restored here, Thanks!
> Thanks!
>

Hello,

On Fri, Sep 25, 2020 at 12:57:33PM -0700, Kees Cook wrote:
> On Fri, Sep 25, 2020 at 04:01:22PM +0530, Ganesh Goudar wrote:
> > Add support to inject slb multihit errors, to test machine
> > check handling.
> 
> Thank you for more tests in here!
Thanks for working on integrating this.
> 
> > 
> > Based on work by Mahesh Salgaonkar and Michal Suchánek.
> > 
> > Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> > Cc: Michal Suchánek <msuchanek@suse.de>
> 
> Should these be Co-developed-by: with S-o-b?

I don't think I wrote any of this code. I packaged it for SUSE and maybe
changed some constants based on test result discussion.

I compared this code to my saved snapshots of past versions of the test
module and this covers all the test cases I have. The only difference is that
the development modules have verbose prints showing what's going on.

It is true that without the verbose prints some explanatory comments
could be helpful.

Reviewed-by: Michal Suchánek <msuchanek@suse.de>
> 
> > Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
> > ---
> >  drivers/misc/lkdtm/Makefile  |   4 ++
> >  drivers/misc/lkdtm/core.c    |   3 +
> >  drivers/misc/lkdtm/lkdtm.h   |   3 +
> >  drivers/misc/lkdtm/powerpc.c | 132 +++++++++++++++++++++++++++++++++++
> >  4 files changed, 142 insertions(+)
> >  create mode 100644 drivers/misc/lkdtm/powerpc.c
> > 
> > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
> > index c70b3822013f..6a82f407fbcd 100644
> > --- a/drivers/misc/lkdtm/Makefile
> > +++ b/drivers/misc/lkdtm/Makefile
> > @@ -11,6 +11,10 @@ lkdtm-$(CONFIG_LKDTM)		+= usercopy.o
> >  lkdtm-$(CONFIG_LKDTM)		+= stackleak.o
> >  lkdtm-$(CONFIG_LKDTM)		+= cfi.o
> >  
> > +ifeq ($(CONFIG_PPC64),y)
> > +lkdtm-$(CONFIG_LKDTM)		+= powerpc.o
> > +endif
> 
> This can just be:
> 
> lkdtm-$(CONFIG_PPC64)		+= powerpc.o
> 
> > +
> >  KASAN_SANITIZE_stackleak.o	:= n
> >  KCOV_INSTRUMENT_rodata.o	:= n
> >  
> > diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
> > index a5e344df9166..8d5db42baa90 100644
> > --- a/drivers/misc/lkdtm/core.c
> > +++ b/drivers/misc/lkdtm/core.c
> > @@ -178,6 +178,9 @@ static const struct crashtype crashtypes[] = {
> >  #ifdef CONFIG_X86_32
> >  	CRASHTYPE(DOUBLE_FAULT),
> >  #endif
> > +#ifdef CONFIG_PPC64
> > +	CRASHTYPE(PPC_SLB_MULTIHIT),
> > +#endif
> >  };
> >  
> >  
> > diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
> > index 8878538b2c13..b305bd511ee5 100644
> > --- a/drivers/misc/lkdtm/lkdtm.h
> > +++ b/drivers/misc/lkdtm/lkdtm.h
> > @@ -104,4 +104,7 @@ void lkdtm_STACKLEAK_ERASING(void);
> >  /* cfi.c */
> >  void lkdtm_CFI_FORWARD_PROTO(void);
> >  
> > +/* powerpc.c */
> > +void lkdtm_PPC_SLB_MULTIHIT(void);
> > +
> >  #endif
> > diff --git a/drivers/misc/lkdtm/powerpc.c b/drivers/misc/lkdtm/powerpc.c
> > new file mode 100644
> > index 000000000000..d6db18444757
> > --- /dev/null
> > +++ b/drivers/misc/lkdtm/powerpc.c
> > @@ -0,0 +1,132 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> 
> Please #include "lkdtm.h" here to get the correct pr_fmt heading (and
> any future header adjustments).
> 
> > +#include <linux/slab.h>
> > +#include <linux/vmalloc.h>
> > +
> > +static inline unsigned long get_slb_index(void)
> > +{
> > +	unsigned long index;
> > +
> > +	index = get_paca()->stab_rr;
> > +
> > +	/*
> > +	 * simple round-robin replacement of slb starting at SLB_NUM_BOLTED.
> > +	 */
> > +	if (index < (mmu_slb_size - 1))
> > +		index++;
> > +	else
> > +		index = SLB_NUM_BOLTED;
> > +	get_paca()->stab_rr = index;
> > +	return index;
> > +}
> > +
> > +#define slb_esid_mask(ssize)	\
> > +	(((ssize) == MMU_SEGSIZE_256M) ? ESID_MASK : ESID_MASK_1T)
> > +
> > +static inline unsigned long mk_esid_data(unsigned long ea, int ssize,
> > +					 unsigned long slot)
> > +{
> > +	return (ea & slb_esid_mask(ssize)) | SLB_ESID_V | slot;
> > +}
> > +
> > +#define slb_vsid_shift(ssize)	\
> > +	((ssize) == MMU_SEGSIZE_256M ? SLB_VSID_SHIFT : SLB_VSID_SHIFT_1T)
> > +
> > +static inline unsigned long mk_vsid_data(unsigned long ea, int ssize,
> > +					 unsigned long flags)
> > +{
> > +	return (get_kernel_vsid(ea, ssize) << slb_vsid_shift(ssize)) | flags |
> > +		((unsigned long)ssize << SLB_VSID_SSIZE_SHIFT);
> > +}
> > +
> > +static void insert_slb_entry(char *p, int ssize)
> > +{
> > +	unsigned long flags, entry;
> > +
> > +	flags = SLB_VSID_KERNEL | mmu_psize_defs[MMU_PAGE_64K].sllp;
> > +	preempt_disable();
> > +
> > +	entry = get_slb_index();
> > +	asm volatile("slbmte %0,%1" :
> > +			: "r" (mk_vsid_data((unsigned long)p, ssize, flags)),
> > +			  "r" (mk_esid_data((unsigned long)p, ssize, entry))
> > +			: "memory");
> > +
> > +	entry = get_slb_index();
> > +	asm volatile("slbmte %0,%1" :
> > +			: "r" (mk_vsid_data((unsigned long)p, ssize, flags)),
> > +			  "r" (mk_esid_data((unsigned long)p, ssize, entry))
> > +			: "memory");
> > +	preempt_enable();
> > +	p[0] = '!';
> > +}
> 
> Can you add some comments to these helpers? It'll help people (me) with
> understanding what is actually being done here (and more importantly,
> what is _expected_ to happen).
> 
> > +
> > +static void inject_vmalloc_slb_multihit(void)
> > +{
> > +	char *p;
> > +
> > +	p = vmalloc(2048);
> > +	if (!p)
> > +		return;
> > +
> > +	insert_slb_entry(p, MMU_SEGSIZE_1T);
> > +	vfree(p);
> > +}
> > +
> > +static void inject_kmalloc_slb_multihit(void)
> > +{
> > +	char *p;
> > +
> > +	p = kmalloc(2048, GFP_KERNEL);
> > +	if (!p)
> > +		return;
> > +
> > +	insert_slb_entry(p, MMU_SEGSIZE_1T);
> > +	kfree(p);
> > +}
> 
> It looks like the expected failure injection is actually the "p[0] = '!'" line in the
> "insert" helper? I would expect pr_info/pr_err wrappers, etc, as in
> other lkdtm tests.
> 
> If this is the negative test, what does the positive test look like?
> e.g. in other lkdtm tests, first a positive test is done, then a
> negative: first run a legit function, then run a function from a bad
> location.
> 
> > +
> > +static void insert_dup_slb_entry_0(void)
> > +{
> > +	unsigned long test_address = 0xC000000000000000;
> > +	volatile unsigned long *test_ptr;
> > +	unsigned long entry, i = 0;
> > +	unsigned long esid, vsid;
> > +
> > +	test_ptr = (unsigned long *)test_address;
> > +	preempt_disable();
> > +
> > +	asm volatile("slbmfee  %0,%1" : "=r" (esid) : "r" (i));
> > +	asm volatile("slbmfev  %0,%1" : "=r" (vsid) : "r" (i));
> > +	entry = get_slb_index();
> > +
> > +	/* for i !=0 we would need to mask out the old entry number */
> > +	asm volatile("slbmte %0,%1" :
> > +			: "r" (vsid),
> > +			  "r" (esid | entry)
> > +			: "memory");
> > +
> > +	asm volatile("slbmfee  %0,%1" : "=r" (esid) : "r" (i));
> > +	asm volatile("slbmfev  %0,%1" : "=r" (vsid) : "r" (i));
> > +	entry = get_slb_index();
> > +
> > +	/* for i !=0 we would need to mask out the old entry number */
> > +	asm volatile("slbmte %0,%1" :
> > +			: "r" (vsid),
> > +			  "r" (esid | entry)
> > +			: "memory");
> > +
> > +	pr_info("lkdtm: %s accessing test address 0x%lx: 0x%lx\n",
> > +		__func__, test_address, *test_ptr);
> > +
> > +	preempt_enable();
> > +}
> 
> What does this do?
> 
> > +
> > +void lkdtm_PPC_SLB_MULTIHIT(void)
> > +{
> > +	if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
> > +		inject_vmalloc_slb_multihit();
> > +		inject_kmalloc_slb_multihit();
> > +		insert_dup_slb_entry_0();
> > +	}
> > +	pr_info("Recovered from SLB multihit. (Ignore this message on non HPTE machines)");
> 
> Is this bad? If so, I'd expect pr_err("FAIL: ...") Can HPTE machines be
> detected so that an XFAIL can be emitted instead?
> 
> Since there are three (two?) distinct regions being tested, should these
> be separate tests? Right now there is no way to separate a vmalloc
> failure from a kmalloc failure, and no way to fail the first without
> hiding the result from the latter (or maybe the machine cannot survive
> this test? ... which should also be a comment.)
> 
> And finally, assuming a successful test (or testing from a separate
> thread later), so there any state that needs to be restored (or cleaned
> up before doing the "insert" calls)?
> 
> Thanks!
> 
> -- 
> Kees Cook