LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v5 19/26] powerpc/book3s64/kuap: Move KUAP related function outside radix
From: Aneesh Kumar K.V @ 2020-07-06 14:41 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87eepo6cjm.fsf@mpe.ellerman.id.au>

On 7/6/20 6:11 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> The next set of patches adds support for kuap with hash translation.
> 
> That's no longer true of this series.
> 
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index 0d72c0246052..e93b65a0e6e7 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -199,6 +200,24 @@ void __init pkey_early_init_devtree(void)
>>   	return;
>>   }
>>   
>> +#ifdef CONFIG_PPC_KUAP
>> +void __init setup_kuap(bool disabled)
>> +{
>> +	if (disabled || !early_radix_enabled())
>> +		return;
>> +
>> +	if (smp_processor_id() == boot_cpuid) {
>> +		pr_info("Activating Kernel Userspace Access Prevention\n");
>> +		cur_cpu_spec->mmu_features |= MMU_FTR_RADIX_KUAP;
>> +	}
>> +
>> +	/* Make sure userspace can't change the AMR */
>> +	mtspr(SPRN_UAMOR, 0);
>> +	mtspr(SPRN_AMR, AMR_KUAP_BLOCKED);
>> +	isync();
>> +}
>> +#endif
> 
> This makes this code depend on CONFIG_PPC_MEM_KEYS=y, which it didn't
> used to.
> 
> That risks breaking people's existing .configs, if they have
> PPC_MEM_KEYS=n they will now lose KUAP.
> 
> And I'm not convinced the two features should be tied together, at least
> at the user-visible Kconfig level.
> 

That simplifies the addition of hash kuap a lot. Especially in the 
exception entry and return paths.  I did try to consider them as 
independent options. But then the feature fixup in asm code gets 
unnecessarily complicated. Also the UAMOR handling also get complicated.

-aneesh

^ permalink raw reply

* Re: [PATCH v5 18/26] powerpc/book3s64/keys/kuap: Reset AMR/IAMR values on kexec
From: Aneesh Kumar K.V @ 2020-07-06 14:39 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87h7uk6d3m.fsf@mpe.ellerman.id.au>

On 7/6/20 5:59 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> As we kexec across kernels that use AMR/IAMR for different purposes
>> we need to ensure that new kernels get kexec'd with a reset value
>> of AMR/IAMR. For ex: the new kernel can use key 0 for kernel mapping and the old
>> AMR value prevents access to key 0.
>>
>> This patch also removes reset if IAMR and AMOR in kexec_sequence. Reset of AMOR
>> is not needed and the IAMR reset is partial (it doesn't do the reset
>> on secondary cpus) and is redundant with this patch.
> 
> I like the idea of cleaning this stuff up.
> 
> But I think tying it into kup/kuep/kuap and the MMU features and ifdefs
> and so on is overly complicated.
> 
> We should just have eg:
> 
> void reset_sprs(void)
> {
> 	if (early_cpu_has_feature(CPU_FTR_ARCH_206)) {
>          	mtspr(SPRN_AMR, 0);
>          	mtspr(SPRN_UAMOR, 0);
>          }
> 
> 	if (early_cpu_has_feature(CPU_FTR_ARCH_207S)) {
>          	mtspr(SPRN_IAMR, 0);
>          }
> }
> 
> And call that from the kexec paths.
>

Will fix. Should that be early_cpu_has_feature()? cpu_has_feature() 
should work there right?

-aneesh

^ permalink raw reply

* Re: [PATCH v5 13/26] powerpc/book3s64/pkeys: Enable MMU_FTR_PKEY
From: Aneesh Kumar K.V @ 2020-07-06 14:09 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <878sfw6b7v.fsf@mpe.ellerman.id.au>

On 7/6/20 6:40 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> Parse storage keys related device tree entry in early_init_devtree
>> and enable MMU feature MMU_FTR_PKEY if pkeys are supported.
>>
>> MMU feature is used instead of CPU feature because this enables us
>> to group MMU_FTR_KUAP and MMU_FTR_PKEY in asm feature fixup code.
> 
> Subject should probably be "Add MMU_FTR_PKEY".
> 
>> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
>> index f4ac25d4df05..72966d3d8f64 100644
>> --- a/arch/powerpc/include/asm/mmu.h
>> +++ b/arch/powerpc/include/asm/mmu.h
>> @@ -23,6 +23,7 @@
>>   
>>   /* Radix page table supported and enabled */
>>   #define MMU_FTR_TYPE_RADIX		ASM_CONST(0x00000040)
>> +#define MMU_FTR_PKEY			ASM_CONST(0x00000080)
> 
> It's not a type, so it should be with the individual feature bits below:
> 


We don't have free bit in the other group. For now i will move this to

modified   arch/powerpc/include/asm/mmu.h
@@ -23,12 +23,15 @@

  /* Radix page table supported and enabled */
  #define MMU_FTR_TYPE_RADIX		ASM_CONST(0x00000040)
-#define MMU_FTR_PKEY			ASM_CONST(0x00000080)

  /*
   * Individual features below.
   */

+/*
+ * Support for memory protection keys.
+ */
+#define MMU_FTR_PKEY			ASM_CONST(0x00001000)
  /*
   * Support for 68 bit VA space. We added that from ISA 2.05
   */



>>   /*
>>    * Individual features below.
> 
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index 6a3bac357e24..6d70797352d8 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -815,6 +815,11 @@ void __init early_init_devtree(void *params)
>>   	/* Now try to figure out if we are running on LPAR and so on */
>>   	pseries_probe_fw_features();
>>   
>> +	/*
>> +	 * Initialize pkey features and default AMR/IAMR values
>> +	 */
>> +	pkey_early_init_devtree();
> 
> Not your fault, but the fact that we're having to do more and more
> initialisation this early, based on the flat device tree, makes me
> wonder if we need to rethink how we're doing the CPU/MMU feature setup.
> 
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index 0ff59acdbb84..bbba9c601e14 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -38,38 +39,45 @@ static int execute_only_key = 2;
>>   #define PKEY_REG_BITS (sizeof(u64) * 8)
>>   #define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY))
>>   
>> +static int __init dt_scan_storage_keys(unsigned long node,
>> +				       const char *uname, int depth,
>> +				       void *data)
>> +{
>> +	const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
>> +	const __be32 *prop;
>> +	int pkeys_total;
>> +
>> +	/* We are scanning "cpu" nodes only */
>> +	if (type == NULL || strcmp(type, "cpu") != 0)
>> +		return 0;
>> +
>> +	prop = of_get_flat_dt_prop(node, "ibm,processor-storage-keys", NULL);
>> +	if (!prop)
>> +		return 0;
>> +	pkeys_total = be32_to_cpu(prop[0]);
>> +	return pkeys_total;
> 
> That's not really the way the return value is meant to be used for these
> scanning functions.
> 
> The usual return values are 0 to keep scanning and !0 means we've found
> the node we're looking for and we should stop scanning.
> 
> Doing it this way means if you find 0 pkeys it will keep scanning.
> 
> Instead you should pass &pkeys_total as the data pointer and set that.
> 
>> +}
>> +


done

modified   arch/powerpc/mm/book3s64/pkeys.c
@@ -52,7 +52,7 @@ static int __init dt_scan_storage_keys(unsigned long node,
  {
  	const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
  	const __be32 *prop;
-	int pkeys_total;
+	int *pkeys_total = (int *) data;

  	/* We are scanning "cpu" nodes only */
  	if (type == NULL || strcmp(type, "cpu") != 0)
@@ -61,12 +61,13 @@ static int __init dt_scan_storage_keys(unsigned long 
node,
  	prop = of_get_flat_dt_prop(node, "ibm,processor-storage-keys", NULL);
  	if (!prop)
  		return 0;
-	pkeys_total = be32_to_cpu(prop[0]);
-	return pkeys_total;
+	*pkeys_total = be32_to_cpu(prop[0]);
+	return 1;
  }

  static int scan_pkey_feature(void)
  {
+	int ret;
  	int pkeys_total;

  	/*
@@ -75,8 +76,8 @@ static int scan_pkey_feature(void)
  	if (early_radix_enabled())
  		return 0;

-	pkeys_total = of_scan_flat_dt(dt_scan_storage_keys, NULL);
-	if (pkeys_total == 0) {
+	ret = of_scan_flat_dt(dt_scan_storage_keys, &pkeys_total);
+	if (ret == 0) {

  		/*
  		 * Let's assume 32 pkeys on P8/P9 bare metal, if its not defined by 
device
@@ -87,7 +88,6 @@ static int scan_pkey_feature(void)
  		    (early_cpu_has_feature(CPU_FTR_ARCH_207S) ||
  		     early_cpu_has_feature(CPU_FTR_ARCH_300)))
  			pkeys_total = 32;
-
  	}

  	/*


>>   static int scan_pkey_feature(void)
>>   {
>> -	u32 vals[2];
>> -	int pkeys_total = 0;
>> -	struct device_node *cpu;
>> +	int pkeys_total;
>>   
>>   	/*
>>   	 * Pkey is not supported with Radix translation.
>>   	 */
>> -	if (radix_enabled())
>> +	if (early_radix_enabled())
>>   		return 0;
>>   
>> -	cpu = of_find_node_by_type(NULL, "cpu");
>> -	if (!cpu)
>> -		return 0;
>> +	pkeys_total = of_scan_flat_dt(dt_scan_storage_keys, NULL);
>> +	if (pkeys_total == 0) {
>>   
>> -	if (of_property_read_u32_array(cpu,
>> -				       "ibm,processor-storage-keys", vals, 2) == 0) {
>> -		/*
>> -		 * Since any pkey can be used for data or execute, we will
>> -		 * just treat all keys as equal and track them as one entity.
>> -		 */
>> -		pkeys_total = vals[0];
>> -		/*  Should we check for IAMR support FIXME!! */
> 
> ???

The device tree allows us to have different count for both AMR and IAMR.
The current code skip that. I guess i added a comment in earlier patch 
to check that whether we need to handle different AMR and IAMR counts. 
The same comment get dropped here.


> 
>> -	} else {
> 
> This loses the ability to differentiate between no storage-keys property
> at all vs a property that specifies zero keys, which I don't think is a
> good change.
> 
>>   		/*
>>   		 * Let's assume 32 pkeys on P8 bare metal, if its not defined by device
>>   		 * tree. We make this exception since skiboot forgot to expose this
>>   		 * property on power8.
>>   		 */
>>   		if (!firmware_has_feature(FW_FEATURE_LPAR) &&
>> -		    cpu_has_feature(CPU_FTRS_POWER8))
>> +		    early_cpu_has_feature(CPU_FTRS_POWER8))
>>   			pkeys_total = 32;
> 
> That's not how cpu_has_feature() works, we'll need to fix that.
> 
> cheers
> 

I did a separate patch to handle that which switch the above to

		/*
		 * Let's assume 32 pkeys on P8/P9 bare metal, if its not defined by device
		 * tree. We make this exception since skiboot forgot to expose this
		 * property on power8/9.
		 */
		if (!firmware_has_feature(FW_FEATURE_LPAR) &&
		    (early_cpu_has_feature(CPU_FTR_ARCH_207S) ||
		     early_cpu_has_feature(CPU_FTR_ARCH_300)))
			pkeys_total = 32;
	

^ permalink raw reply

* [PATCH] powerpc/spufs: add CONFIG_COREDUMP dependency
From: Arnd Bergmann @ 2020-07-06 13:22 UTC (permalink / raw)
  To: Arnd Bergmann, Michael Ellerman
  Cc: kernel test robot, linux-kernel, Paul Mackerras, Jeremy Kerr,
	linuxppc-dev, Christoph Hellwig

The kernel test robot pointed out a slightly different error message
after recent commit 5456ffdee666 ("powerpc/spufs: simplify spufs core
dumping") to spufs for a configuration that never worked:

   powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in function `.spufs_proxydma_info_dump':
>> file.c:(.text+0x4c68): undefined reference to `.dump_emit'
   powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in function `.spufs_dma_info_dump':
   file.c:(.text+0x4d70): undefined reference to `.dump_emit'
   powerpc64-linux-ld: arch/powerpc/platforms/cell/spufs/file.o: in function `.spufs_wbox_info_dump':
   file.c:(.text+0x4df4): undefined reference to `.dump_emit'

Add a Kconfig dependency to prevent this from happening again.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/powerpc/platforms/cell/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/cell/Kconfig b/arch/powerpc/platforms/cell/Kconfig
index 0f7c8241912b..f2ff359041ee 100644
--- a/arch/powerpc/platforms/cell/Kconfig
+++ b/arch/powerpc/platforms/cell/Kconfig
@@ -44,6 +44,7 @@ config SPU_FS
 	tristate "SPU file system"
 	default m
 	depends on PPC_CELL
+	depends on COREDUMP
 	select SPU_BASE
 	help
 	  The SPU file system is used to access Synergistic Processing
-- 
2.27.0


^ permalink raw reply related

* Re: [PATCH v5 13/26] powerpc/book3s64/pkeys: Enable MMU_FTR_PKEY
From: Michael Ellerman @ 2020-07-06 13:10 UTC (permalink / raw)
  To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200619135850.47155-14-aneesh.kumar@linux.ibm.com>

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Parse storage keys related device tree entry in early_init_devtree
> and enable MMU feature MMU_FTR_PKEY if pkeys are supported.
>
> MMU feature is used instead of CPU feature because this enables us
> to group MMU_FTR_KUAP and MMU_FTR_PKEY in asm feature fixup code.

Subject should probably be "Add MMU_FTR_PKEY".

> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
> index f4ac25d4df05..72966d3d8f64 100644
> --- a/arch/powerpc/include/asm/mmu.h
> +++ b/arch/powerpc/include/asm/mmu.h
> @@ -23,6 +23,7 @@
>  
>  /* Radix page table supported and enabled */
>  #define MMU_FTR_TYPE_RADIX		ASM_CONST(0x00000040)
> +#define MMU_FTR_PKEY			ASM_CONST(0x00000080)

It's not a type, so it should be with the individual feature bits below:

>  /*
>   * Individual features below.

> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 6a3bac357e24..6d70797352d8 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -815,6 +815,11 @@ void __init early_init_devtree(void *params)
>  	/* Now try to figure out if we are running on LPAR and so on */
>  	pseries_probe_fw_features();
>  
> +	/*
> +	 * Initialize pkey features and default AMR/IAMR values
> +	 */
> +	pkey_early_init_devtree();

Not your fault, but the fact that we're having to do more and more
initialisation this early, based on the flat device tree, makes me
wonder if we need to rethink how we're doing the CPU/MMU feature setup.

> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index 0ff59acdbb84..bbba9c601e14 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -38,38 +39,45 @@ static int execute_only_key = 2;
>  #define PKEY_REG_BITS (sizeof(u64) * 8)
>  #define pkeyshift(pkey) (PKEY_REG_BITS - ((pkey+1) * AMR_BITS_PER_PKEY))
>  
> +static int __init dt_scan_storage_keys(unsigned long node,
> +				       const char *uname, int depth,
> +				       void *data)
> +{
> +	const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> +	const __be32 *prop;
> +	int pkeys_total;
> +
> +	/* We are scanning "cpu" nodes only */
> +	if (type == NULL || strcmp(type, "cpu") != 0)
> +		return 0;
> +
> +	prop = of_get_flat_dt_prop(node, "ibm,processor-storage-keys", NULL);
> +	if (!prop)
> +		return 0;
> +	pkeys_total = be32_to_cpu(prop[0]);
> +	return pkeys_total;

That's not really the way the return value is meant to be used for these
scanning functions.

The usual return values are 0 to keep scanning and !0 means we've found
the node we're looking for and we should stop scanning.

Doing it this way means if you find 0 pkeys it will keep scanning.

Instead you should pass &pkeys_total as the data pointer and set that.

> +}
> +
>  static int scan_pkey_feature(void)
>  {
> -	u32 vals[2];
> -	int pkeys_total = 0;
> -	struct device_node *cpu;
> +	int pkeys_total;
>  
>  	/*
>  	 * Pkey is not supported with Radix translation.
>  	 */
> -	if (radix_enabled())
> +	if (early_radix_enabled())
>  		return 0;
>  
> -	cpu = of_find_node_by_type(NULL, "cpu");
> -	if (!cpu)
> -		return 0;
> +	pkeys_total = of_scan_flat_dt(dt_scan_storage_keys, NULL);
> +	if (pkeys_total == 0) {
>  
> -	if (of_property_read_u32_array(cpu,
> -				       "ibm,processor-storage-keys", vals, 2) == 0) {
> -		/*
> -		 * Since any pkey can be used for data or execute, we will
> -		 * just treat all keys as equal and track them as one entity.
> -		 */
> -		pkeys_total = vals[0];
> -		/*  Should we check for IAMR support FIXME!! */

???

> -	} else {

This loses the ability to differentiate between no storage-keys property
at all vs a property that specifies zero keys, which I don't think is a
good change.

>  		/*
>  		 * Let's assume 32 pkeys on P8 bare metal, if its not defined by device
>  		 * tree. We make this exception since skiboot forgot to expose this
>  		 * property on power8.
>  		 */
>  		if (!firmware_has_feature(FW_FEATURE_LPAR) &&
> -		    cpu_has_feature(CPU_FTRS_POWER8))
> +		    early_cpu_has_feature(CPU_FTRS_POWER8))
>  			pkeys_total = 32;

That's not how cpu_has_feature() works, we'll need to fix that.

cheers

^ permalink raw reply

* Re: [PATCH v5 19/26] powerpc/book3s64/kuap: Move KUAP related function outside radix
From: Michael Ellerman @ 2020-07-06 12:41 UTC (permalink / raw)
  To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200619135850.47155-20-aneesh.kumar@linux.ibm.com>

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> The next set of patches adds support for kuap with hash translation.

That's no longer true of this series.

> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index 0d72c0246052..e93b65a0e6e7 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -199,6 +200,24 @@ void __init pkey_early_init_devtree(void)
>  	return;
>  }
>  
> +#ifdef CONFIG_PPC_KUAP
> +void __init setup_kuap(bool disabled)
> +{
> +	if (disabled || !early_radix_enabled())
> +		return;
> +
> +	if (smp_processor_id() == boot_cpuid) {
> +		pr_info("Activating Kernel Userspace Access Prevention\n");
> +		cur_cpu_spec->mmu_features |= MMU_FTR_RADIX_KUAP;
> +	}
> +
> +	/* Make sure userspace can't change the AMR */
> +	mtspr(SPRN_UAMOR, 0);
> +	mtspr(SPRN_AMR, AMR_KUAP_BLOCKED);
> +	isync();
> +}
> +#endif

This makes this code depend on CONFIG_PPC_MEM_KEYS=y, which it didn't
used to.

That risks breaking people's existing .configs, if they have
PPC_MEM_KEYS=n they will now lose KUAP.

And I'm not convinced the two features should be tied together, at least
at the user-visible Kconfig level.

cheers

^ permalink raw reply

* Re: [PATCH v12 00/31] Speculative page faults
From: Laurent Dufour @ 2020-07-06 12:27 UTC (permalink / raw)
  To: Chinwen Chang, Haiyan Song
  Cc: jack, sergey.senozhatsky.work, peterz, Will Deacon, mhocko,
	linux-mm, paulus, Punit Agrawal, hpa, Michel Lespinasse,
	Alexei Starovoitov, Andrea Arcangeli, ak, Minchan Kim,
	aneesh.kumar, x86, Matthew Wilcox, Daniel Jordan, Ingo Molnar,
	David Rientjes, paulmck, npiggin, sj38.park, Jerome Glisse, dave,
	kemi.wang, kirill, Thomas Gleixner, zhong jiang, Ganesh Mahendran,
	Yang Shi, Mike Rapoport, linuxppc-dev, linux-kernel,
	Sergey Senozhatsky, miles.chen, vinayak menon, akpm, Tim Chen,
	haren
In-Reply-To: <1594027500.30360.32.camel@mtkswgap22>

Le 06/07/2020 à 11:25, Chinwen Chang a écrit :
> On Thu, 2019-06-20 at 16:19 +0800, Haiyan Song wrote:
>> Hi Laurent,
>>
>> I downloaded your script and run it on Intel 2s skylake platform with spf-v12 patch
>> serials.
>>
>> Here attached the output results of this script.
>>
>> The following comparison result is statistics from the script outputs.
>>
>> a). Enable THP
>>                                              SPF_0          change       SPF_1
>> will-it-scale.page_fault2.per_thread_ops    2664190.8      -11.7%       2353637.6
>> will-it-scale.page_fault3.per_thread_ops    4480027.2      -14.7%       3819331.9
>>
>>
>> b). Disable THP
>>                                              SPF_0           change      SPF_1
>> will-it-scale.page_fault2.per_thread_ops    2653260.7       -10%        2385165.8
>> will-it-scale.page_fault3.per_thread_ops    4436330.1       -12.4%      3886734.2
>>
>>
>> Thanks,
>> Haiyan Song
>>
>>
>> On Fri, Jun 14, 2019 at 10:44:47AM +0200, Laurent Dufour wrote:
>>> Le 14/06/2019 à 10:37, Laurent Dufour a écrit :
>>>> Please find attached the script I run to get these numbers.
>>>> This would be nice if you could give it a try on your victim node and share the result.
>>>
>>> Sounds that the Intel mail fitering system doesn't like the attached shell script.
>>> Please find it there: https://gist.github.com/ldu4/a5cc1a93f293108ea387d43d5d5e7f44
>>>
>>> Thanks,
>>> Laurent.
>>>
> 
> Hi Laurent,
> 
> We merged SPF v11 and some patches from v12 into our platforms. After
> several experiments, we observed SPF has obvious improvements on the
> launch time of applications, especially for those high-TLP ones,
> 
> # launch time of applications(s):
> 
> package           version      w/ SPF      w/o SPF      improve(%)
> ------------------------------------------------------------------
> Baidu maps        10.13.3      0.887       0.98         9.49
> Taobao            8.4.0.35     1.227       1.293        5.10
> Meituan           9.12.401     1.107       1.543        28.26
> WeChat            7.0.3        2.353       2.68         12.20
> Honor of Kings    1.43.1.6     6.63        6.713        1.24

That's great news, thanks for reporting this!

> 
> By the way, we have verified our platforms with those patches and
> achieved the goal of mass production.

Another good news!
For my information, what is your targeted hardware?

Cheers,
Laurent.


^ permalink raw reply

* Re: [PATCH v5 18/26] powerpc/book3s64/keys/kuap: Reset AMR/IAMR values on kexec
From: Michael Ellerman @ 2020-07-06 12:29 UTC (permalink / raw)
  To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200619135850.47155-19-aneesh.kumar@linux.ibm.com>

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> As we kexec across kernels that use AMR/IAMR for different purposes
> we need to ensure that new kernels get kexec'd with a reset value
> of AMR/IAMR. For ex: the new kernel can use key 0 for kernel mapping and the old
> AMR value prevents access to key 0.
>
> This patch also removes reset if IAMR and AMOR in kexec_sequence. Reset of AMOR
> is not needed and the IAMR reset is partial (it doesn't do the reset
> on secondary cpus) and is redundant with this patch.

I like the idea of cleaning this stuff up.

But I think tying it into kup/kuep/kuap and the MMU features and ifdefs
and so on is overly complicated.

We should just have eg:

void reset_sprs(void)
{
	if (early_cpu_has_feature(CPU_FTR_ARCH_206)) {
        	mtspr(SPRN_AMR, 0);
        	mtspr(SPRN_UAMOR, 0);
        }

	if (early_cpu_has_feature(CPU_FTR_ARCH_207S)) {
        	mtspr(SPRN_IAMR, 0);
        }
}

And call that from the kexec paths.

cheers

> diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h
> index 3ee1ec60be84..c57063c35833 100644
> --- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
> @@ -180,6 +180,26 @@ static inline unsigned long kuap_get_and_check_amr(void)
>  }
>  #endif /* CONFIG_PPC_KUAP */
>  
> +#define reset_kuap reset_kuap
> +static inline void reset_kuap(void)
> +{
> +	if (mmu_has_feature(MMU_FTR_RADIX_KUAP)) {
> +		mtspr(SPRN_AMR, 0);
> +		/*  Do we need isync()? We are going via a kexec reset */
> +		isync();
> +	}
> +}
> +
> +#define reset_kuep reset_kuep
> +static inline void reset_kuep(void)
> +{
> +	if (mmu_has_feature(MMU_FTR_KUEP)) {
> +		mtspr(SPRN_IAMR, 0);
> +		/*  Do we need isync()? We are going via a kexec reset */
> +		isync();
> +	}
> +}
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* _ASM_POWERPC_BOOK3S_64_KUP_RADIX_H */
> diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
> index c745ee41ad66..4dc23a706910 100644
> --- a/arch/powerpc/include/asm/kup.h
> +++ b/arch/powerpc/include/asm/kup.h
> @@ -113,6 +113,20 @@ static inline void prevent_current_write_to_user(void)
>  	prevent_user_access(NULL, NULL, ~0UL, KUAP_CURRENT_WRITE);
>  }
>  
> +#ifndef reset_kuap
> +#define reset_kuap reset_kuap
> +static inline void reset_kuap(void)
> +{
> +}
> +#endif
> +
> +#ifndef reset_kuep
> +#define reset_kuep reset_kuep
> +static inline void reset_kuep(void)
> +{
> +}
> +#endif
> +
>  #endif /* !__ASSEMBLY__ */
>  
>  #endif /* _ASM_POWERPC_KUAP_H_ */
> diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
> index 1864605eca29..7bb46ad98207 100644
> --- a/arch/powerpc/kernel/misc_64.S
> +++ b/arch/powerpc/kernel/misc_64.S
> @@ -413,20 +413,6 @@ _GLOBAL(kexec_sequence)
>  	li	r0,0
>  	std	r0,16(r1)
>  
> -BEGIN_FTR_SECTION
> -	/*
> -	 * This is the best time to turn AMR/IAMR off.
> -	 * key 0 is used in radix for supervisor<->user
> -	 * protection, but on hash key 0 is reserved
> -	 * ideally we want to enter with a clean state.
> -	 * NOTE, we rely on r0 being 0 from above.
> -	 */
> -	mtspr	SPRN_IAMR,r0
> -BEGIN_FTR_SECTION_NESTED(42)
> -	mtspr	SPRN_AMOR,r0
> -END_FTR_SECTION_NESTED_IFSET(CPU_FTR_HVMODE, 42)
> -END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> -
>  	/* save regs for local vars on new stack.
>  	 * yes, we won't go back, but ...
>  	 */
> diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
> index b4184092172a..a124715f33ea 100644
> --- a/arch/powerpc/kexec/core_64.c
> +++ b/arch/powerpc/kexec/core_64.c
> @@ -152,6 +152,9 @@ static void kexec_smp_down(void *arg)
>  	if (ppc_md.kexec_cpu_down)
>  		ppc_md.kexec_cpu_down(0, 1);
>  
> +	reset_kuap();
> +	reset_kuep();
> +
>  	kexec_smp_wait();
>  	/* NOTREACHED */
>  }
> diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
> index c58ad1049909..9673f4b74c9a 100644
> --- a/arch/powerpc/mm/book3s64/pgtable.c
> +++ b/arch/powerpc/mm/book3s64/pgtable.c
> @@ -165,6 +165,9 @@ void mmu_cleanup_all(void)
>  		radix__mmu_cleanup_all();
>  	else if (mmu_hash_ops.hpte_clear_all)
>  		mmu_hash_ops.hpte_clear_all();
> +
> +	reset_kuap();
> +	reset_kuep();
>  }
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -- 
> 2.26.2

^ permalink raw reply

* Re: [PATCH V4 0/4] mm/debug_vm_pgtable: Add some more tests
From: Alexander Gordeev @ 2020-07-06 12:06 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-doc, Heiko Carstens, linux-mm, Paul Mackerras,
	H. Peter Anvin, linux-riscv, Will Deacon, linux-arch, linux-s390,
	Jonathan Corbet, x86, Mike Rapoport, Christian Borntraeger,
	Ingo Molnar, Catalin Marinas, linux-snps-arc, Vasily Gorbik,
	Borislav Petkov, Paul Walmsley, Kirill A . Shutemov,
	Thomas Gleixner, linux-arm-kernel, Vineet Gupta, linux-kernel,
	Palmer Dabbelt, Andrew Morton, linuxppc-dev
In-Reply-To: <1593996516-7186-1-git-send-email-anshuman.khandual@arm.com>

On Mon, Jul 06, 2020 at 06:18:32AM +0530, Anshuman Khandual wrote:

[...]

> Tested on arm64, x86 platforms but only build tested on all other enabled
> platforms through ARCH_HAS_DEBUG_VM_PGTABLE i.e powerpc, arc, s390. The

Sorry for missing to test earlier. Works for me on s390. Also, tried with
few relevant config options to set/unset.

Thanks!

^ permalink raw reply

* Re: [PATCH] kbuild: introduce ccflags-remove-y and asflags-remove-y
From: Anders Roxell @ 2020-07-06 12:00 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Michal Marek, Yoshinori Sato, Linux Kbuild mailing list,
	Linux-sh list, Linux Kernel Mailing List, Steven Rostedt,
	Russell King, linuxppc-dev, Ingo Molnar, Paul Mackerras,
	Sami Tolvanen, Rich Felker, linux-arm-kernel
In-Reply-To: <CADYN=9JnzPC6Ja9s3_01k-CDTSuxKBMRdrqU5rqp0xw1r9XpRw@mail.gmail.com>

On Mon, 6 Jul 2020 at 13:24, Anders Roxell <anders.roxell@linaro.org> wrote:
>
> Hi,
>
> When I built an allmodconfig kernel for arm64, and boot that in qemu
> guest I see the following issues:
>
> [...]
> [   10.451561][    T1] Testing tracer function: PASSED
> [   33.087895][    T1] Testing dynamic ftrace: .. filter did not
> filter .. FAILED!
> [   51.127094][    T1] ------------[ cut here ]------------
> [   51.132387][    T1] WARNING: CPU: 0 PID: 1 at
> kernel/trace/trace.c:1814 run_tracer_selftest+0x314/0x40c
> [   51.140805][    T1] Modules linked in:
> [   51.145398][    T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 5.8.0-rc3-next-20200703-00004-g8cd4bc531754 #1
> [   51.154146][    T1] Hardware name: linux,dummy-virt (DT)
> [   51.159536][    T1] pstate: 80400005 (Nzcv daif +PAN -UAO BTYPE=--)
> [   51.165711][    T1] pc : run_tracer_selftest+0x314/0x40c
> [   51.171167][    T1] lr : run_tracer_selftest+0x308/0x40c
> [   51.176475][    T1] sp : ffff000069c07c40
> [   51.180855][    T1] x29: ffff000069c07c40 x28: ffffa00015e80320
> [   51.187187][    T1] x27: ffffa00013e074a0 x26: ffffa000150f22ee
> [   51.193520][    T1] x25: ffffa000125b35a0 x24: ffffa00013f6a000
> [   51.199868][    T1] x23: ffffa00013f6adc0 x22: 00000000ffffffff
> [   51.206225][    T1] x21: ffffa000150f2250 x20: ffffa00015e801c0
> [   51.212582][    T1] x19: ffffa00015e7fa80 x18: 0000000000002750
> [   51.218887][    T1] x17: 00000000000014c0 x16: 00000000000016f0
> [   51.225229][    T1] x15: 00000000000014f8 x14: ffffa0001000a3c8
> [   51.231584][    T1] x13: ffffa000124a5fb8 x12: ffff80000d494822
> [   51.237935][    T1] x11: 1fffe0000d494821 x10: ffff80000d494821
> [   51.244293][    T1] x9 : ffffa000101763b0 x8 : ffff000069c077d7
> [   51.250704][    T1] x7 : 0000000000000001 x6 : ffff000069c077d0
> [   51.257060][    T1] x5 : ffff00006a7fc040 x4 : 0000000000000000
> [   51.263423][    T1] x3 : ffffa00010211118 x2 : 8af0509000018b00
> [   51.269757][    T1] x1 : 0000000000000000 x0 : 0000000000000001
> [   51.276084][    T1] Call trace:
> [   51.279934][    T1]  run_tracer_selftest+0x314/0x40c
> [   51.285174][    T1]  init_trace_selftests+0x110/0x31c
> [   51.290391][    T1]  do_one_initcall+0x410/0x960
> [   51.295315][    T1]  kernel_init_freeable+0x430/0x4f0
> [   51.300595][    T1]  kernel_init+0x20/0x1d8
> [   51.305180][    T1]  ret_from_fork+0x10/0x18
> [   51.309741][    T1] irq event stamp: 1401832
> [   51.314399][    T1] hardirqs last  enabled at (1401831):
> [<ffffa0001020d10c>] console_unlock+0xc04/0xd10
> [   51.322973][    T1] hardirqs last disabled at (1401832):
> [<ffffa00010050fe4>] debug_exception_enter+0xbc/0x200
> [   51.331993][    T1] softirqs last  enabled at (1401828):
> [<ffffa000100023a4>] __do_softirq+0x95c/0x9f8
> [   51.340490][    T1] softirqs last disabled at (1401821):
> [<ffffa0001010f7d0>] irq_exit+0x118/0x198
> [   51.348717][    T1] _warn_unseeded_randomness: 5 callbacks suppressed
> [   51.349263][    T1] random: get_random_bytes called from
> print_oops_end_marker+0x48/0x80 with crng_init=0
> [   51.350502][    T1] ---[ end trace c566e8a71c933d3c ]---
> [...]
> [40709.672335][    C0] pool 2: cpus=0 flags=0x5 nice=0 hung=3s
> workers=3 manager: 1455
> [40739.960593][   T26] INFO: lockdep is turned off.
> [40775.312499][   T26] Kernel panic - not syncing: hung_task: blocked tasks
> [40775.341521][   T26] CPU: 0 PID: 26 Comm: khungtaskd Tainted: G
>   W         5.8.0-rc3-next-20200703-00004-g8cd4bc531754 #1
> [40775.352848][   T26] Hardware name: linux,dummy-virt (DT)
> [40775.359304][   T26] Call trace:
> [40775.364148][   T26]  dump_backtrace+0x0/0x418
> [40775.369918][   T26]  show_stack+0x34/0x48
> [40775.375468][   T26]  dump_stack+0x1f4/0x2b0
> [40775.381136][   T26]  panic+0x2dc/0x6ec
> [40775.386430][   T26]  watchdog+0x1400/0x1460
> [40775.392103][   T26]  kthread+0x23c/0x250
> [40775.397548][   T26]  ret_from_fork+0x10/0x18
> [40775.407039][   T26] Kernel Offset: disabled
> [40775.412634][   T26] CPU features: 0x240002,20002004
> [40775.418751][   T26] Memory Limit: none
> [40775.425823][   T26] ---[ end Kernel panic - not syncing: hung_task:
> blocked tasks ]---
>
> The full log can be found here [1].
>
> Without this patch for  'trace_selftest_dynamic' for instance, CC_FLAGS_FTRACE
> was removed from kernel/trace/*, and then added back to
> kernel/trace/trace_selftest_dynamic.
> While with this patch it looks like we add the flag (even though it is
> already there), and then
> removes the flag for all files in kernel/trace/* .

Hi again,

I think the patch below solved the issue for trace_selftest_dynamic. However,
I think there is still differences in lib/* .

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index cc5e3c6aaa20..5632a711921f 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -111,12 +111,12 @@ basename_flags = -DKBUILD_BASENAME=$(call
name-fix,$(basetarget))
 modname_flags  = -DKBUILD_MODNAME=$(call name-fix,$(modname))
 modfile_flags  = -DKBUILD_MODFILE=$(call stringify,$(modfile))

-orig_c_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) \
-                 $(ccflags-y) $(CFLAGS_$(target-stem).o)
+orig_c_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(ccflags-y)
 _c_flags       = $(filter-out $(ccflags-remove-y)
$(CFLAGS_REMOVE_$(target-stem).o), $(orig_c_flags))
-orig_a_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_AFLAGS) \
-                 $(asflags-y) $(AFLAGS_$(target-stem).o)
+_c_flags      += $(CFLAGS_$(target-stem).o)
+orig_a_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_AFLAGS) $(asflags-y)
 _a_flags       = $(filter-out $(asflags-remove-y)
$(AFLAGS_REMOVE_$(target-stem).o), $(orig_a_flags))
+_a_flags      += $(AFLAGS_$(target-stem).o)
 _cpp_flags     = $(KBUILD_CPPFLAGS) $(cppflags-y)
$(CPPFLAGS_$(target-stem).lds)

 #


>
> Cheers,
> Anders
> [1] https://people.linaro.org/~anders.roxell/output-next-20200703.log
>
> On Tue, 30 Jun 2020 at 04:09, Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Mon, Jun 29, 2020 at 2:55 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
> > >
> > > Masahiro Yamada <masahiroy@kernel.org> writes:
> > > > CFLAGS_REMOVE_<file>.o works per object, that is, there is no
> > > > convenient way to filter out flags for every object in a directory.
> > > >
> > > > Add ccflags-remove-y and asflags-remove-y to make it easily.
> > > >
> > > > Use ccflags-remove-y to clean up some Makefiles.
> > > >
> > > > Suggested-by: Sami Tolvanen <samitolvanen@google.com>
> > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > > ---
> > > >
> > > >  arch/arm/boot/compressed/Makefile | 6 +-----
> > > >  arch/powerpc/xmon/Makefile        | 3 +--
> > > >  arch/sh/boot/compressed/Makefile  | 5 +----
> > > >  kernel/trace/Makefile             | 4 ++--
> > > >  lib/Makefile                      | 5 +----
> > > >  scripts/Makefile.lib              | 4 ++--
> > > >  6 files changed, 8 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> > > > index 89c76ca35640..55cbcdd88ac0 100644
> > > > --- a/arch/powerpc/xmon/Makefile
> > > > +++ b/arch/powerpc/xmon/Makefile
> > > > @@ -7,8 +7,7 @@ UBSAN_SANITIZE := n
> > > >  KASAN_SANITIZE := n
> > > >
> > > >  # Disable ftrace for the entire directory
> > > > -ORIG_CFLAGS := $(KBUILD_CFLAGS)
> > > > -KBUILD_CFLAGS = $(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS))
> > > > +ccflags-remove-y += $(CC_FLAGS_FTRACE)
> > >
> > > This could be:
> > >
> > > ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
> > >
> > > Similar to kernel/trace/Makefile below.
> >
> >
> > I fixed it up, and applied to linux-kbuild.
> > Thanks.
> >
> >
> > > I don't mind though.
> > >
> > > Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> > >
> > > cheers
> > >
> > > > diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> > > > index 6575bb0a0434..7492844a8b1b 100644
> > > > --- a/kernel/trace/Makefile
> > > > +++ b/kernel/trace/Makefile
> > > > @@ -2,9 +2,9 @@
> > > >
> > > >  # Do not instrument the tracer itself:
> > > >
> > > > +ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
> > > > +
> > > >  ifdef CONFIG_FUNCTION_TRACER
> > > > -ORIG_CFLAGS := $(KBUILD_CFLAGS)
> > > > -KBUILD_CFLAGS = $(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS))
> > > >
> > > >  # Avoid recursion due to instrumentation.
> > > >  KCSAN_SANITIZE := n
> >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada

^ permalink raw reply related

* Re: [PATCH] kbuild: introduce ccflags-remove-y and asflags-remove-y
From: Anders Roxell @ 2020-07-06 11:24 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Michal Marek, Yoshinori Sato, Linux Kbuild mailing list,
	Linux-sh list, Linux Kernel Mailing List, Steven Rostedt,
	Russell King, linuxppc-dev, Ingo Molnar, Paul Mackerras,
	Sami Tolvanen, Rich Felker, linux-arm-kernel
In-Reply-To: <CAK7LNATusciypBJ4dYZcyrugdi_rXEV_s=zxAehDxsX+Sd5z4g@mail.gmail.com>

Hi,

When I built an allmodconfig kernel for arm64, and boot that in qemu
guest I see the following issues:

[...]
[   10.451561][    T1] Testing tracer function: PASSED
[   33.087895][    T1] Testing dynamic ftrace: .. filter did not
filter .. FAILED!
[   51.127094][    T1] ------------[ cut here ]------------
[   51.132387][    T1] WARNING: CPU: 0 PID: 1 at
kernel/trace/trace.c:1814 run_tracer_selftest+0x314/0x40c
[   51.140805][    T1] Modules linked in:
[   51.145398][    T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
5.8.0-rc3-next-20200703-00004-g8cd4bc531754 #1
[   51.154146][    T1] Hardware name: linux,dummy-virt (DT)
[   51.159536][    T1] pstate: 80400005 (Nzcv daif +PAN -UAO BTYPE=--)
[   51.165711][    T1] pc : run_tracer_selftest+0x314/0x40c
[   51.171167][    T1] lr : run_tracer_selftest+0x308/0x40c
[   51.176475][    T1] sp : ffff000069c07c40
[   51.180855][    T1] x29: ffff000069c07c40 x28: ffffa00015e80320
[   51.187187][    T1] x27: ffffa00013e074a0 x26: ffffa000150f22ee
[   51.193520][    T1] x25: ffffa000125b35a0 x24: ffffa00013f6a000
[   51.199868][    T1] x23: ffffa00013f6adc0 x22: 00000000ffffffff
[   51.206225][    T1] x21: ffffa000150f2250 x20: ffffa00015e801c0
[   51.212582][    T1] x19: ffffa00015e7fa80 x18: 0000000000002750
[   51.218887][    T1] x17: 00000000000014c0 x16: 00000000000016f0
[   51.225229][    T1] x15: 00000000000014f8 x14: ffffa0001000a3c8
[   51.231584][    T1] x13: ffffa000124a5fb8 x12: ffff80000d494822
[   51.237935][    T1] x11: 1fffe0000d494821 x10: ffff80000d494821
[   51.244293][    T1] x9 : ffffa000101763b0 x8 : ffff000069c077d7
[   51.250704][    T1] x7 : 0000000000000001 x6 : ffff000069c077d0
[   51.257060][    T1] x5 : ffff00006a7fc040 x4 : 0000000000000000
[   51.263423][    T1] x3 : ffffa00010211118 x2 : 8af0509000018b00
[   51.269757][    T1] x1 : 0000000000000000 x0 : 0000000000000001
[   51.276084][    T1] Call trace:
[   51.279934][    T1]  run_tracer_selftest+0x314/0x40c
[   51.285174][    T1]  init_trace_selftests+0x110/0x31c
[   51.290391][    T1]  do_one_initcall+0x410/0x960
[   51.295315][    T1]  kernel_init_freeable+0x430/0x4f0
[   51.300595][    T1]  kernel_init+0x20/0x1d8
[   51.305180][    T1]  ret_from_fork+0x10/0x18
[   51.309741][    T1] irq event stamp: 1401832
[   51.314399][    T1] hardirqs last  enabled at (1401831):
[<ffffa0001020d10c>] console_unlock+0xc04/0xd10
[   51.322973][    T1] hardirqs last disabled at (1401832):
[<ffffa00010050fe4>] debug_exception_enter+0xbc/0x200
[   51.331993][    T1] softirqs last  enabled at (1401828):
[<ffffa000100023a4>] __do_softirq+0x95c/0x9f8
[   51.340490][    T1] softirqs last disabled at (1401821):
[<ffffa0001010f7d0>] irq_exit+0x118/0x198
[   51.348717][    T1] _warn_unseeded_randomness: 5 callbacks suppressed
[   51.349263][    T1] random: get_random_bytes called from
print_oops_end_marker+0x48/0x80 with crng_init=0
[   51.350502][    T1] ---[ end trace c566e8a71c933d3c ]---
[...]
[40709.672335][    C0] pool 2: cpus=0 flags=0x5 nice=0 hung=3s
workers=3 manager: 1455
[40739.960593][   T26] INFO: lockdep is turned off.
[40775.312499][   T26] Kernel panic - not syncing: hung_task: blocked tasks
[40775.341521][   T26] CPU: 0 PID: 26 Comm: khungtaskd Tainted: G
  W         5.8.0-rc3-next-20200703-00004-g8cd4bc531754 #1
[40775.352848][   T26] Hardware name: linux,dummy-virt (DT)
[40775.359304][   T26] Call trace:
[40775.364148][   T26]  dump_backtrace+0x0/0x418
[40775.369918][   T26]  show_stack+0x34/0x48
[40775.375468][   T26]  dump_stack+0x1f4/0x2b0
[40775.381136][   T26]  panic+0x2dc/0x6ec
[40775.386430][   T26]  watchdog+0x1400/0x1460
[40775.392103][   T26]  kthread+0x23c/0x250
[40775.397548][   T26]  ret_from_fork+0x10/0x18
[40775.407039][   T26] Kernel Offset: disabled
[40775.412634][   T26] CPU features: 0x240002,20002004
[40775.418751][   T26] Memory Limit: none
[40775.425823][   T26] ---[ end Kernel panic - not syncing: hung_task:
blocked tasks ]---

The full log can be found here [1].

Without this patch for  'trace_selftest_dynamic' for instance, CC_FLAGS_FTRACE
was removed from kernel/trace/*, and then added back to
kernel/trace/trace_selftest_dynamic.
While with this patch it looks like we add the flag (even though it is
already there), and then
removes the flag for all files in kernel/trace/* .

Cheers,
Anders
[1] https://people.linaro.org/~anders.roxell/output-next-20200703.log

On Tue, 30 Jun 2020 at 04:09, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Mon, Jun 29, 2020 at 2:55 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> > Masahiro Yamada <masahiroy@kernel.org> writes:
> > > CFLAGS_REMOVE_<file>.o works per object, that is, there is no
> > > convenient way to filter out flags for every object in a directory.
> > >
> > > Add ccflags-remove-y and asflags-remove-y to make it easily.
> > >
> > > Use ccflags-remove-y to clean up some Makefiles.
> > >
> > > Suggested-by: Sami Tolvanen <samitolvanen@google.com>
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > ---
> > >
> > >  arch/arm/boot/compressed/Makefile | 6 +-----
> > >  arch/powerpc/xmon/Makefile        | 3 +--
> > >  arch/sh/boot/compressed/Makefile  | 5 +----
> > >  kernel/trace/Makefile             | 4 ++--
> > >  lib/Makefile                      | 5 +----
> > >  scripts/Makefile.lib              | 4 ++--
> > >  6 files changed, 8 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> > > index 89c76ca35640..55cbcdd88ac0 100644
> > > --- a/arch/powerpc/xmon/Makefile
> > > +++ b/arch/powerpc/xmon/Makefile
> > > @@ -7,8 +7,7 @@ UBSAN_SANITIZE := n
> > >  KASAN_SANITIZE := n
> > >
> > >  # Disable ftrace for the entire directory
> > > -ORIG_CFLAGS := $(KBUILD_CFLAGS)
> > > -KBUILD_CFLAGS = $(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS))
> > > +ccflags-remove-y += $(CC_FLAGS_FTRACE)
> >
> > This could be:
> >
> > ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
> >
> > Similar to kernel/trace/Makefile below.
>
>
> I fixed it up, and applied to linux-kbuild.
> Thanks.
>
>
> > I don't mind though.
> >
> > Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> >
> > cheers
> >
> > > diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> > > index 6575bb0a0434..7492844a8b1b 100644
> > > --- a/kernel/trace/Makefile
> > > +++ b/kernel/trace/Makefile
> > > @@ -2,9 +2,9 @@
> > >
> > >  # Do not instrument the tracer itself:
> > >
> > > +ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
> > > +
> > >  ifdef CONFIG_FUNCTION_TRACER
> > > -ORIG_CFLAGS := $(KBUILD_CFLAGS)
> > > -KBUILD_CFLAGS = $(subst $(CC_FLAGS_FTRACE),,$(ORIG_CFLAGS))
> > >
> > >  # Avoid recursion due to instrumentation.
> > >  KCSAN_SANITIZE := n
>
>
>
> --
> Best Regards
> Masahiro Yamada

^ permalink raw reply

* Re: [PATCH v6 00/11] Add the multiple PF support for DWC and Layerscape
From: Lorenzo Pieralisi @ 2020-07-06 10:46 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: roy.zang, devicetree, jingoohan1, Zhiqiang.Hou, linuxppc-dev,
	linux-pci, linux-kernel, leoyang.li, Minghuan.Lian, robh+dt,
	linux-arm-kernel, gustavo.pimentel, bhelgaas, andrew.murray,
	kishon, shawnguo, mingkai.hu, amurray
In-Reply-To: <20200314033038.24844-1-xiaowei.bao@nxp.com>

On Sat, Mar 14, 2020 at 11:30:27AM +0800, Xiaowei Bao wrote:
> Add the PCIe EP multiple PF support for DWC and Layerscape, add
> the doorbell MSIX function for DWC, use list to manage the PF of
> one PCIe controller, and refactor the Layerscape EP driver due to
> some platforms difference.
> 
> Xiaowei Bao (11):
>   PCI: designware-ep: Add multiple PFs support for DWC
>   PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
>   PCI: designware-ep: Move the function of getting MSI capability
>     forward
>   PCI: designware-ep: Modify MSI and MSIX CAP way of finding
>   dt-bindings: pci: layerscape-pci: Add compatible strings for ls1088a
>     and ls2088a
>   PCI: layerscape: Fix some format issue of the code
>   PCI: layerscape: Modify the way of getting capability with different
>     PEX
>   PCI: layerscape: Modify the MSIX to the doorbell mode
>   PCI: layerscape: Add EP mode support for ls1088a and ls2088a
>   arm64: dts: layerscape: Add PCIe EP node for ls1088a
>   misc: pci_endpoint_test: Add LS1088a in pci_device_id table
> 
>  .../devicetree/bindings/pci/layerscape-pci.txt     |   2 +
>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi     |  31 +++
>  drivers/misc/pci_endpoint_test.c                   |   2 +
>  drivers/pci/controller/dwc/pci-layerscape-ep.c     | 100 ++++++--
>  drivers/pci/controller/dwc/pcie-designware-ep.c    | 255 +++++++++++++++++----
>  drivers/pci/controller/dwc/pcie-designware.c       |  59 +++--
>  drivers/pci/controller/dwc/pcie-designware.h       |  48 +++-
>  7 files changed, 404 insertions(+), 93 deletions(-)

Can you rebase it against v5.8-rc1 please ?

Thanks,
Lorenzo

^ permalink raw reply

* [PATCH 11/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*()
From: Saheed Olayemi Bolarinwa @ 2020-07-06  9:31 UTC (permalink / raw)
  To: helgaas
  Cc: Mike Marciniszyn, linuxppc-dev,
	Greg Kroah-Hartman linux-rdma @ vger . kernel . org,
	Arnd Bergmann, Jason Gunthorpe, Sam Bobroff,
	Bolarinwa Olayemi Saheed, Dennis Dalessandro, skhan,
	Rafael J. Wysocki, linux-kernel, wunner.de, linux-acpi,
	Doug Ledford, linux-pci, bjorn, Oliver O'Halloran,
	linux-kernel-mentees, linux-rdma
In-Reply-To: <20200706093121.9731-1-refactormyself@gmail.com>

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

 **TODO**

Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
This patch  depends on all of the preceeding patches in this series,
otherwise it will introduce bugs as pointed out in the commit message
of each.
 drivers/pci/access.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 79c4a2ef269a..ec95edbb1ac8 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -413,13 +413,6 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val)
 
 	if (pcie_capability_reg_implemented(dev, pos)) {
 		ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val);
-		/*
-		 * Reset *val to 0 if pci_read_config_word() fails, it may
-		 * have been written as 0xFFFF if hardware error happens
-		 * during pci_read_config_word().
-		 */
-		if (ret)
-			*val = 0;
 		return ret;
 	}
 
@@ -448,13 +441,6 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val)
 
 	if (pcie_capability_reg_implemented(dev, pos)) {
 		ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val);
-		/*
-		 * Reset *val to 0 if pci_read_config_dword() fails, it may
-		 * have been written as 0xFFFFFFFF if hardware error happens
-		 * during pci_read_config_dword().
-		 */
-		if (ret)
-			*val = 0;
 		return ret;
 	}
 
-- 
2.18.2


^ permalink raw reply related

* [PATCH 0/11 RFC] PCI: Remove "*val = 0" from pcie_capability_read_*()
From: Saheed Olayemi Bolarinwa @ 2020-07-06  9:31 UTC (permalink / raw)
  To: helgaas
  Cc: Mike Marciniszyn, linuxppc-dev,
	Greg Kroah-Hartman linux-rdma @ vger . kernel . org,
	Arnd Bergmann, Jason Gunthorpe, Sam Bobroff,
	Bolarinwa Olayemi Saheed, Dennis Dalessandro, skhan,
	Rafael J. Wysocki, linux-kernel, wunner.de, linux-acpi,
	Doug Ledford, linux-pci, bjorn, Oliver O'Halloran,
	linux-kernel-mentees, linux-rdma

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

*** BLURB HERE ***

Bolarinwa Olayemi Saheed (9):
  IB/hfi1: Confirm that pcie_capability_read_dword() is successful
  misc: rtsx: Confirm that pcie_capability_read_word() is successful
  PCI/AER: Use error return value from pcie_capability_read_*()
  PCI/ASPM: Use error return value from pcie_capability_read_*()
  PCI: pciehp: Fix wrong failure check on pcie_capability_read_*()
  PCI: pciehp: Prevent wrong failure check on pcie_capability_read_*()
  PCI: pciehp: Make "Power On" the default in pciehp_get_power_status()
  PCI/ACPI: Prevent wrong failure check on pcie_capability_read_*()
  PCI: Prevent wrong failure check on pcie_capability_read_*()
  PCI: Remove "*val = 0" fom pcie_capability_read_*()

 
 drivers/infiniband/hw/hfi1/aspm.c | 7 ++++---
 drivers/misc/cardreader/rts5227.c | 5 +++--
 drivers/misc/cardreader/rts5249.c | 5 +++--
 drivers/misc/cardreader/rts5260.c | 5 +++--
 drivers/misc/cardreader/rts5261.c | 5 +++--
 drivers/pci/pcie/aer.c  |  5 +++--
 drivers/pci/pcie/aspm.c | 33 +++++++++++++++++----------------
 drivers/pci/hotplug/pciehp_hpc.c | 47 ++++++++++++++++----------------
 drivers/pci/pci-acpi.c           | 10 ++++---
 drivers/pci/probe.c              | 29 ++++++++++++--------
 drivers/pci/access.c | 14 --------------
 11 files changed, 82 insertions(+), 83 deletions(-)

-- 
2.18.2



^ permalink raw reply

* Re: [PATCH v12 00/31] Speculative page faults
From: Chinwen Chang @ 2020-07-06  9:25 UTC (permalink / raw)
  To: Haiyan Song
  Cc: jack, sergey.senozhatsky.work, peterz, Will Deacon, mhocko,
	linux-mm, paulus, Punit Agrawal, hpa, Michel Lespinasse,
	Alexei Starovoitov, Andrea Arcangeli, ak, Minchan Kim,
	aneesh.kumar, x86, Matthew Wilcox, Daniel Jordan, Ingo Molnar,
	David Rientjes, paulmck, npiggin, sj38.park, Jerome Glisse, dave,
	kemi.wang, kirill, Thomas Gleixner, Laurent Dufour, zhong jiang,
	Ganesh Mahendran, Yang Shi, Mike Rapoport, linuxppc-dev,
	linux-kernel, Sergey Senozhatsky, miles.chen, vinayak menon, akpm,
	Tim Chen, haren
In-Reply-To: <20190620081945.hwj6ruqddefnxg6z@haiyan.sh.intel.com>

On Thu, 2019-06-20 at 16:19 +0800, Haiyan Song wrote:
> Hi Laurent,
> 
> I downloaded your script and run it on Intel 2s skylake platform with spf-v12 patch
> serials.
> 
> Here attached the output results of this script.
> 
> The following comparison result is statistics from the script outputs.
> 
> a). Enable THP
>                                             SPF_0          change       SPF_1
> will-it-scale.page_fault2.per_thread_ops    2664190.8      -11.7%       2353637.6      
> will-it-scale.page_fault3.per_thread_ops    4480027.2      -14.7%       3819331.9     
> 
> 
> b). Disable THP
>                                             SPF_0           change      SPF_1
> will-it-scale.page_fault2.per_thread_ops    2653260.7       -10%        2385165.8
> will-it-scale.page_fault3.per_thread_ops    4436330.1       -12.4%      3886734.2 
> 
> 
> Thanks,
> Haiyan Song
> 
> 
> On Fri, Jun 14, 2019 at 10:44:47AM +0200, Laurent Dufour wrote:
> > Le 14/06/2019 à 10:37, Laurent Dufour a écrit :
> > > Please find attached the script I run to get these numbers.
> > > This would be nice if you could give it a try on your victim node and share the result.
> > 
> > Sounds that the Intel mail fitering system doesn't like the attached shell script.
> > Please find it there: https://gist.github.com/ldu4/a5cc1a93f293108ea387d43d5d5e7f44
> > 
> > Thanks,
> > Laurent.
> > 

Hi Laurent,

We merged SPF v11 and some patches from v12 into our platforms. After
several experiments, we observed SPF has obvious improvements on the
launch time of applications, especially for those high-TLP ones,

# launch time of applications(s):

package           version      w/ SPF      w/o SPF      improve(%)
------------------------------------------------------------------                          
Baidu maps        10.13.3      0.887       0.98         9.49
Taobao            8.4.0.35     1.227       1.293        5.10
Meituan           9.12.401     1.107       1.543        28.26
WeChat            7.0.3        2.353       2.68         12.20
Honor of Kings    1.43.1.6     6.63        6.713        1.24


By the way, we have verified our platforms with those patches and
achieved the goal of mass production.

Thanks.
Chinwen Chang

^ permalink raw reply

* Re: [PATCH] powerpc: select ARCH_HAS_MEMBARRIER_SYNC_CORE
From: Christophe Leroy @ 2020-07-06  9:53 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: linux-arch, Mathieu Desnoyers
In-Reply-To: <20200706021822.1515189-1-npiggin@gmail.com>



Le 06/07/2020 à 04:18, Nicholas Piggin a écrit :
> powerpc return from interrupt and return from system call sequences are
> context synchronising.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   .../features/sched/membarrier-sync-core/arch-support.txt      | 4 ++--
>   arch/powerpc/Kconfig                                          | 1 +
>   arch/powerpc/include/asm/exception-64s.h                      | 4 ++++
>   3 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/features/sched/membarrier-sync-core/arch-support.txt b/Documentation/features/sched/membarrier-sync-core/arch-support.txt
> index 8a521a622966..52ad74a25f54 100644
> --- a/Documentation/features/sched/membarrier-sync-core/arch-support.txt
> +++ b/Documentation/features/sched/membarrier-sync-core/arch-support.txt
> @@ -5,7 +5,7 @@
>   #
>   # Architecture requirements
>   #
> -# * arm/arm64
> +# * arm/arm64/powerpc
>   #
>   # Rely on implicit context synchronization as a result of exception return
>   # when returning from IPI handler, and when returning to user-space.
> @@ -45,7 +45,7 @@
>       |       nios2: | TODO |
>       |    openrisc: | TODO |
>       |      parisc: | TODO |
> -    |     powerpc: | TODO |
> +    |     powerpc: |  ok  |
>       |       riscv: | TODO |
>       |        s390: | TODO |
>       |          sh: | TODO |
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 9fa23eb320ff..920c4e3ca4ef 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -131,6 +131,7 @@ config PPC
>   	select ARCH_HAS_PTE_DEVMAP		if PPC_BOOK3S_64
>   	select ARCH_HAS_PTE_SPECIAL
>   	select ARCH_HAS_MEMBARRIER_CALLBACKS
> +	select ARCH_HAS_MEMBARRIER_SYNC_CORE
>   	select ARCH_HAS_SCALED_CPUTIME		if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
>   	select ARCH_HAS_STRICT_KERNEL_RWX	if (PPC32 && !HIBERNATION)
>   	select ARCH_HAS_TICK_BROADCAST		if GENERIC_CLOCKEVENTS_BROADCAST
> diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
> index 47bd4ea0837d..b88cb3a989b6 100644
> --- a/arch/powerpc/include/asm/exception-64s.h
> +++ b/arch/powerpc/include/asm/exception-64s.h
> @@ -68,6 +68,10 @@
>    *
>    * The nop instructions allow us to insert one or more instructions to flush the
>    * L1-D cache when returning to userspace or a guest.
> + *
> + * powerpc relies on return from interrupt/syscall being context synchronising
> + * (which hrfid, rfid, and rfscv are) to support ARCH_HAS_MEMBARRIER_SYNC_CORE
> + * without additional additional synchronisation instructions.

This file is dedicated to BOOK3S/64. What about other ones ?

On 32 bits, this is also valid as 'rfi' is also context synchronising, 
but then why just add some comment in exception-64s.h and only there ?

>    */
>   #define RFI_FLUSH_SLOT							\
>   	RFI_FLUSH_FIXUP_SECTION;					\
> 


Christophe

^ permalink raw reply

* Re: [PATCH 01/14] powerpc/eeh: Remove eeh_dev_phb_init_dynamic()
From: kernel test robot @ 2020-07-06  9:12 UTC (permalink / raw)
  To: Oliver O'Halloran, linuxppc-dev
  Cc: Oliver O'Halloran, kbuild-all, mahesh
In-Reply-To: <20200706013619.459420-2-oohall@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3420 bytes --]

Hi Oliver,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.8-rc4 next-20200706]
[cannot apply to scottwood/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Oliver-O-Halloran/powerpc-eeh-Remove-eeh_dev_phb_init_dynamic/20200706-103630
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-r006-20200706 (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
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc64 

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

All errors (new ones prefixed by >>):

   arch/powerpc/kernel/of_platform.c: In function 'of_pci_phb_probe':
>> arch/powerpc/kernel/of_platform.c:66:2: error: implicit declaration of function 'eeh_phb_pe_create' [-Werror=implicit-function-declaration]
      66 |  eeh_phb_pe_create(phb);
         |  ^~~~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors

vim +/eeh_phb_pe_create +66 arch/powerpc/kernel/of_platform.c

    28	
    29	/* The probing of PCI controllers from of_platform is currently
    30	 * 64 bits only, mostly due to gratuitous differences between
    31	 * the 32 and 64 bits PCI code on PowerPC and the 32 bits one
    32	 * lacking some bits needed here.
    33	 */
    34	
    35	static int of_pci_phb_probe(struct platform_device *dev)
    36	{
    37		struct pci_controller *phb;
    38	
    39		/* Check if we can do that ... */
    40		if (ppc_md.pci_setup_phb == NULL)
    41			return -ENODEV;
    42	
    43		pr_info("Setting up PCI bus %pOF\n", dev->dev.of_node);
    44	
    45		/* Alloc and setup PHB data structure */
    46		phb = pcibios_alloc_controller(dev->dev.of_node);
    47		if (!phb)
    48			return -ENODEV;
    49	
    50		/* Setup parent in sysfs */
    51		phb->parent = &dev->dev;
    52	
    53		/* Setup the PHB using arch provided callback */
    54		if (ppc_md.pci_setup_phb(phb)) {
    55			pcibios_free_controller(phb);
    56			return -ENODEV;
    57		}
    58	
    59		/* Process "ranges" property */
    60		pci_process_bridge_OF_ranges(phb, dev->dev.of_node, 0);
    61	
    62		/* Init pci_dn data structures */
    63		pci_devs_phb_init_dynamic(phb);
    64	
    65		/* Create EEH PE for the PHB */
  > 66		eeh_phb_pe_create(phb);
    67	
    68		/* Scan the bus */
    69		pcibios_scan_phb(phb);
    70		if (phb->bus == NULL)
    71			return -ENXIO;
    72	
    73		/* Claim resources. This might need some rework as well depending
    74		 * whether we are doing probe-only or not, like assigning unassigned
    75		 * resources etc...
    76		 */
    77		pcibios_claim_one_bus(phb->bus);
    78	
    79		/* Add probed PCI devices to the device model */
    80		pci_bus_add_devices(phb->bus);
    81	
    82		return 0;
    83	}
    84	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36529 bytes --]

^ permalink raw reply

* Re: [PATCH v2 01/12] kexec_file: allow archs to handle special regions while locating memory hole
From: Dave Young @ 2020-07-06  9:07 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Thiago Jung Bauermann, kernel test robot, Pingfan Liu, Kexec-ml,
	Mahesh J Salgaonkar, Petr Tesarik, lkml, linuxppc-dev, Mimi Zohar,
	Sourabh Jain, Andrew Morton, Vivek Goyal, Eric Biederman
In-Reply-To: <159371964681.21555.573193508667543223.stgit@hbathini.in.ibm.com>

On 07/03/20 at 01:24am, Hari Bathini wrote:
> Some architectures may have special memory regions, within the given
> memory range, which can't be used for the buffer in a kexec segment.
> Implement weak arch_kexec_locate_mem_hole() definition which arch code
> may override, to take care of special regions, while trying to locate
> a memory hole.
> 
> Also, add the missing declarations for arch overridable functions and
> and drop the __weak descriptors in the declarations to avoid non-weak
> definitions from becoming weak.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> [lkp: In v1, arch_kimage_file_post_load_cleanup() declaration was missing]
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
> 
> Changes in v2:
> * Introduced arch_kexec_locate_mem_hole() for override and dropped
>   weak arch_kexec_add_buffer().
> * Dropped __weak identifier for arch overridable functions.
> * Fixed the missing declaration for arch_kimage_file_post_load_cleanup()
>   reported by lkp. lkp report for reference:
>     - https://lore.kernel.org/patchwork/patch/1264418/
> 
> 
>  include/linux/kexec.h |   29 ++++++++++++++++++-----------
>  kernel/kexec_file.c   |   16 ++++++++++++++--
>  2 files changed, 32 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index ea67910..9e93bef 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -183,17 +183,24 @@ int kexec_purgatory_get_set_symbol(struct kimage *image, const char *name,
>  				   bool get_value);
>  void *kexec_purgatory_get_symbol_addr(struct kimage *image, const char *name);
>  
> -int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> -					 unsigned long buf_len);
> -void * __weak arch_kexec_kernel_image_load(struct kimage *image);
> -int __weak arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> -					    Elf_Shdr *section,
> -					    const Elf_Shdr *relsec,
> -					    const Elf_Shdr *symtab);
> -int __weak arch_kexec_apply_relocations(struct purgatory_info *pi,
> -					Elf_Shdr *section,
> -					const Elf_Shdr *relsec,
> -					const Elf_Shdr *symtab);
> +/* Architectures may override the below functions */
> +int arch_kexec_kernel_image_probe(struct kimage *image, void *buf,
> +				  unsigned long buf_len);
> +void *arch_kexec_kernel_image_load(struct kimage *image);
> +int arch_kexec_apply_relocations_add(struct purgatory_info *pi,
> +				     Elf_Shdr *section,
> +				     const Elf_Shdr *relsec,
> +				     const Elf_Shdr *symtab);
> +int arch_kexec_apply_relocations(struct purgatory_info *pi,
> +				 Elf_Shdr *section,
> +				 const Elf_Shdr *relsec,
> +				 const Elf_Shdr *symtab);
> +int arch_kimage_file_post_load_cleanup(struct kimage *image);
> +#ifdef CONFIG_KEXEC_SIG
> +int arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
> +				 unsigned long buf_len);
> +#endif
> +int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
>  
>  extern int kexec_add_buffer(struct kexec_buf *kbuf);
>  int kexec_locate_mem_hole(struct kexec_buf *kbuf);
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 09cc78d..e89912d 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -636,6 +636,19 @@ int kexec_locate_mem_hole(struct kexec_buf *kbuf)
>  }
>  
>  /**
> + * arch_kexec_locate_mem_hole - Find free memory to place the segments.
> + * @kbuf:                       Parameters for the memory search.
> + *
> + * On success, kbuf->mem will have the start address of the memory region found.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int __weak arch_kexec_locate_mem_hole(struct kexec_buf *kbuf)
> +{
> +	return kexec_locate_mem_hole(kbuf);
> +}
> +
> +/**
>   * kexec_add_buffer - place a buffer in a kexec segment
>   * @kbuf:	Buffer contents and memory parameters.
>   *
> @@ -647,7 +660,6 @@ int kexec_locate_mem_hole(struct kexec_buf *kbuf)
>   */
>  int kexec_add_buffer(struct kexec_buf *kbuf)
>  {
> -
>  	struct kexec_segment *ksegment;
>  	int ret;
>  
> @@ -675,7 +687,7 @@ int kexec_add_buffer(struct kexec_buf *kbuf)
>  	kbuf->buf_align = max(kbuf->buf_align, PAGE_SIZE);
>  
>  	/* Walk the RAM ranges and allocate a suitable range for the buffer */
> -	ret = kexec_locate_mem_hole(kbuf);
> +	ret = arch_kexec_locate_mem_hole(kbuf);
>  	if (ret)
>  		return ret;
>  
> 

Acked-by: Dave Young <dyoung@redhat.com>

Thanks
Dave


^ permalink raw reply

* Re: [PATCH v5 15/26] powerpc/book3s64/pkeys: Use execute_pkey_disable static key
From: Aneesh Kumar K.V @ 2020-07-06  8:49 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87mu4d5cu8.fsf@mpe.ellerman.id.au>

On 7/6/20 12:50 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> Use execute_pkey_disabled static key to check for execute key support instead
>> of pkey_disabled.
>>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>>   arch/powerpc/include/asm/pkeys.h | 10 +---------
>>   arch/powerpc/mm/book3s64/pkeys.c |  5 ++++-
>>   2 files changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
>> index 47c81d41ea9a..09fbaa409ac4 100644
>> --- a/arch/powerpc/include/asm/pkeys.h
>> +++ b/arch/powerpc/include/asm/pkeys.h
>> @@ -126,15 +126,7 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
>>    * Try to dedicate one of the protection keys to be used as an
>>    * execute-only protection key.
>>    */
>> -extern int __execute_only_pkey(struct mm_struct *mm);
>> -static inline int execute_only_pkey(struct mm_struct *mm)
>> -{
>> -	if (static_branch_likely(&pkey_disabled))
>> -		return -1;
>> -
>> -	return __execute_only_pkey(mm);
>> -}
>> -
>> +extern int execute_only_pkey(struct mm_struct *mm);
>>   extern int __arch_override_mprotect_pkey(struct vm_area_struct *vma,
>>   					 int prot, int pkey);
>>   static inline int arch_override_mprotect_pkey(struct vm_area_struct *vma,
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index bbba9c601e14..fed4f159011b 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -345,8 +345,11 @@ void thread_pkey_regs_init(struct thread_struct *thread)
>>   	write_uamor(default_uamor);
>>   }
>>   
>> -int __execute_only_pkey(struct mm_struct *mm)
>> +int execute_only_pkey(struct mm_struct *mm)
>>   {
>> +	if (static_branch_likely(&execute_pkey_disabled))
>> +		return -1;
>> +
>>   	return mm->context.execute_only_pkey;
>>   }
> 
> That adds the overhead of a function call, but then uses a static_key to
> avoid an easy to predict branch, which seems like a bad tradeoff. And
> it's not a performance critical path AFAICS.
> 
> Anyway this seems unnecessary:
> 
> pkey_early_init_devtree()
> {
> 	...
> 	if (unlikely(max_pkey <= execute_only_key)) {
> 		/*
> 		 * Insufficient number of keys to support
> 		 * execute only key. Mark it unavailable.
> 		 */
> 		execute_only_key = -1;
> 
> void pkey_mm_init(struct mm_struct *mm)
> {
> 	...
> 	mm->context.execute_only_pkey = execute_only_key;
> }
> 
> 
> ie. Can't it just be:
> 
> static inline int execute_only_pkey(struct mm_struct *mm)
> {
> 	return mm->context.execute_only_pkey;
> }
> 

ok updated with

modified   arch/powerpc/mm/book3s64/pkeys.c
@@ -151,7 +151,7 @@ void __init pkey_early_init_devtree(void)
  	max_pkey = pkeys_total;
  #endif

-	if (unlikely(max_pkey <= execute_only_key)) {
+	if (unlikely(max_pkey <= execute_only_key) || 
!pkey_execute_disable_supported) {
  		/*
  		 * Insufficient number of keys to support
  		 * execute only key. Mark it unavailable.
@@ -368,9 +368,6 @@ int __arch_set_user_pkey_access(struct task_struct 
*tsk, int pkey,

  int execute_only_pkey(struct mm_struct *mm)
  {
-	if (static_branch_likely(&execute_pkey_disabled))
-		return -1;
-
  	return mm->context.execute_only_pkey;
  }

-aneesh

^ permalink raw reply

* Re: [PATCH v5 11/26] powerpc/book3s64/pkeys: Make initial_allocation_mask static
From: Aneesh Kumar K.V @ 2020-07-06  8:48 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87sge55dkh.fsf@mpe.ellerman.id.au>

On 7/6/20 12:34 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> initial_allocation_mask is not used outside this file.
> 
> And never modified after init, so make it __ro_after_init as well?
> 


ok, will update reserved_allocation_maask too.

> cheers
> 
>> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
>> index 652bad7334f3..47c81d41ea9a 100644
>> --- a/arch/powerpc/include/asm/pkeys.h
>> +++ b/arch/powerpc/include/asm/pkeys.h
>> @@ -13,7 +13,6 @@
>>   
>>   DECLARE_STATIC_KEY_FALSE(pkey_disabled);
>>   extern int max_pkey;
>> -extern u32 initial_allocation_mask; /*  bits set for the initially allocated keys */
>>   extern u32 reserved_allocation_mask; /* bits set for reserved keys */
>>   
>>   #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index a4d7287082a8..73b5ef1490c8 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -15,11 +15,11 @@
>>   DEFINE_STATIC_KEY_FALSE(pkey_disabled);
>>   DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);
>>   int  max_pkey;			/* Maximum key value supported */
>> -u32  initial_allocation_mask;   /* Bits set for the initially allocated keys */
>>   /*
>>    *  Keys marked in the reservation list cannot be allocated by  userspace
>>    */
>>   u32  reserved_allocation_mask;
>> +static u32  initial_allocation_mask;   /* Bits set for the initially allocated keys */
>>   static u64 default_amr;
>>   static u64 default_iamr;
>>   /* Allow all keys to be modified by default */
>> -- 
>> 2.26.2


-aneesh

^ permalink raw reply

* Re: [PATCH v5 08/26] powerpc/book3s64/pkeys: Convert execute key support to static key
From: Aneesh Kumar K.V @ 2020-07-06  8:47 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87o8ot5cv5.fsf@mpe.ellerman.id.au>

On 7/6/20 12:49 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> Convert the bool to a static key like pkey_disabled.
> 
> I'm not convinced this is worth the added complexity of a static key.
> 
> It's not used in any performance critical paths, except for context
> switch, but that's already guarded by:
> 
> 	if (old_thread->iamr != new_thread->iamr)
> 
> Which should always be false on machines which don't have the execute
> key enabled.
> 

Ok will drop the patch.

-aneesh


^ permalink raw reply

* Re: [PATCH v8 4/4] powerpc: Book3S 64-bit "heavyweight" KASAN support
From: kernel test robot @ 2020-07-06  8:29 UTC (permalink / raw)
  To: Daniel Axtens, linux-kernel, linux-mm, linuxppc-dev, kasan-dev,
	christophe.leroy, aneesh.kumar, bsingharora
  Cc: kbuild-all, Daniel Axtens
In-Reply-To: <20200702025432.16912-5-dja@axtens.net>

[-- Attachment #1: Type: text/plain, Size: 14987 bytes --]

Hi Daniel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on linux/master linus/master v5.8-rc4 next-20200703]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Daniel-Axtens/KASAN-for-powerpc64-radix/20200702-105552
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-s031-20200706 (attached as .config)
compiler: powerpc64le-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-14-g8fce3d7a-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=powerpc64 

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


sparse warnings: (new ones prefixed by >>)

   drivers/char/tpm/tpm_ibmvtpm.c:130:9: sparse: sparse: cast removes address space '__iomem' of expression
>> drivers/char/tpm/tpm_ibmvtpm.c:131:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void *s @@     got void [noderef] __iomem *rtce_buf @@
>> drivers/char/tpm/tpm_ibmvtpm.c:131:9: sparse:     expected void *s
   drivers/char/tpm/tpm_ibmvtpm.c:131:9: sparse:     got void [noderef] __iomem *rtce_buf
   drivers/char/tpm/tpm_ibmvtpm.c:234:9: sparse: sparse: cast removes address space '__iomem' of expression
   drivers/char/tpm/tpm_ibmvtpm.c:369:30: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const * @@     got void [noderef] __iomem *rtce_buf @@
   drivers/char/tpm/tpm_ibmvtpm.c:369:30: sparse:     expected void const *
   drivers/char/tpm/tpm_ibmvtpm.c:369:30: sparse:     got void [noderef] __iomem *rtce_buf
   drivers/char/tpm/tpm_ibmvtpm.c:530:43: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void [noderef] __iomem *rtce_buf @@     got void * @@
   drivers/char/tpm/tpm_ibmvtpm.c:530:43: sparse:     expected void [noderef] __iomem *rtce_buf
   drivers/char/tpm/tpm_ibmvtpm.c:530:43: sparse:     got void *
   drivers/char/tpm/tpm_ibmvtpm.c:537:52: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected void *ptr @@     got void [noderef] __iomem *rtce_buf @@
   drivers/char/tpm/tpm_ibmvtpm.c:537:52: sparse:     expected void *ptr
   drivers/char/tpm/tpm_ibmvtpm.c:537:52: sparse:     got void [noderef] __iomem *rtce_buf
   drivers/char/tpm/tpm_ibmvtpm.c:543:46: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const * @@     got void [noderef] __iomem *rtce_buf @@
   drivers/char/tpm/tpm_ibmvtpm.c:543:46: sparse:     expected void const *
   drivers/char/tpm/tpm_ibmvtpm.c:543:46: sparse:     got void [noderef] __iomem *rtce_buf
--
   drivers/tty/vt/vt.c:233:5: sparse: sparse: symbol 'console_blank_hook' was not declared. Should it be static?
   drivers/tty/vt/vt.c:2901:19: sparse: sparse: symbol 'console_driver' was not declared. Should it be static?
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
>> arch/powerpc/include/asm/vga.h:40:21: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned short [usertype] @@     got restricted __le16 [usertype] @@
>> arch/powerpc/include/asm/vga.h:40:21: sparse:     expected unsigned short [usertype]
   arch/powerpc/include/asm/vga.h:40:21: sparse:     got restricted __le16 [usertype]
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:29:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short volatile [usertype] @@     got restricted __le16 [usertype] @@
   arch/powerpc/include/asm/vga.h:29:15: sparse:     expected unsigned short volatile [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse:     got restricted __le16 [usertype]
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:29:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short volatile [usertype] @@     got restricted __le16 [usertype] @@
   arch/powerpc/include/asm/vga.h:29:15: sparse:     expected unsigned short volatile [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse:     got restricted __le16 [usertype]
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:29:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short volatile [usertype] @@     got restricted __le16 [usertype] @@
   arch/powerpc/include/asm/vga.h:29:15: sparse:     expected unsigned short volatile [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse:     got restricted __le16 [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short volatile [usertype] @@     got restricted __le16 [usertype] @@
   arch/powerpc/include/asm/vga.h:29:15: sparse:     expected unsigned short volatile [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse:     got restricted __le16 [usertype]
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:29:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short volatile [usertype] @@     got restricted __le16 [usertype] @@
   arch/powerpc/include/asm/vga.h:29:15: sparse:     expected unsigned short volatile [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse:     got restricted __le16 [usertype]
>> arch/powerpc/include/asm/vga.h:40:21: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned short [usertype] @@     got restricted __le16 [usertype] @@
>> arch/powerpc/include/asm/vga.h:40:21: sparse:     expected unsigned short [usertype]
   arch/powerpc/include/asm/vga.h:40:21: sparse:     got restricted __le16 [usertype]
>> arch/powerpc/include/asm/vga.h:40:21: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned short [usertype] @@     got restricted __le16 [usertype] @@
>> arch/powerpc/include/asm/vga.h:40:21: sparse:     expected unsigned short [usertype]
   arch/powerpc/include/asm/vga.h:40:21: sparse:     got restricted __le16 [usertype]
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:29:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short volatile [usertype] @@     got restricted __le16 [usertype] @@
   arch/powerpc/include/asm/vga.h:29:15: sparse:     expected unsigned short volatile [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse:     got restricted __le16 [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short volatile [usertype] @@     got restricted __le16 [usertype] @@
   arch/powerpc/include/asm/vga.h:29:15: sparse:     expected unsigned short volatile [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse:     got restricted __le16 [usertype]
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:29:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short volatile [usertype] @@     got restricted __le16 [usertype] @@
   arch/powerpc/include/asm/vga.h:29:15: sparse:     expected unsigned short volatile [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse:     got restricted __le16 [usertype]
>> arch/powerpc/include/asm/vga.h:40:21: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned short [usertype] @@     got restricted __le16 [usertype] @@
>> arch/powerpc/include/asm/vga.h:40:21: sparse:     expected unsigned short [usertype]
   arch/powerpc/include/asm/vga.h:40:21: sparse:     got restricted __le16 [usertype]
>> arch/powerpc/include/asm/vga.h:40:21: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned short [usertype] @@     got restricted __le16 [usertype] @@
>> arch/powerpc/include/asm/vga.h:40:21: sparse:     expected unsigned short [usertype]
   arch/powerpc/include/asm/vga.h:40:21: sparse:     got restricted __le16 [usertype]
>> arch/powerpc/include/asm/vga.h:40:21: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned short [usertype] @@     got restricted __le16 [usertype] @@
>> arch/powerpc/include/asm/vga.h:40:21: sparse:     expected unsigned short [usertype]
   arch/powerpc/include/asm/vga.h:40:21: sparse:     got restricted __le16 [usertype]
>> arch/powerpc/include/asm/vga.h:40:21: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned short [usertype] @@     got restricted __le16 [usertype] @@
>> arch/powerpc/include/asm/vga.h:40:21: sparse:     expected unsigned short [usertype]
   arch/powerpc/include/asm/vga.h:40:21: sparse:     got restricted __le16 [usertype]
>> arch/powerpc/include/asm/vga.h:40:21: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected unsigned short [usertype] @@     got restricted __le16 [usertype] @@
>> arch/powerpc/include/asm/vga.h:40:21: sparse:     expected unsigned short [usertype]
   arch/powerpc/include/asm/vga.h:40:21: sparse:     got restricted __le16 [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short volatile [usertype] @@     got restricted __le16 [usertype] @@
   arch/powerpc/include/asm/vga.h:29:15: sparse:     expected unsigned short volatile [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse:     got restricted __le16 [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short volatile [usertype] @@     got restricted __le16 [usertype] @@
   arch/powerpc/include/asm/vga.h:29:15: sparse:     expected unsigned short volatile [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse:     got restricted __le16 [usertype]
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:34:16: sparse: sparse: cast to restricted __le16
   arch/powerpc/include/asm/vga.h:29:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned short volatile [usertype] @@     got restricted __le16 [usertype] @@
   arch/powerpc/include/asm/vga.h:29:15: sparse:     expected unsigned short volatile [usertype]
   arch/powerpc/include/asm/vga.h:29:15: sparse:     got restricted __le16 [usertype]

vim +131 drivers/char/tpm/tpm_ibmvtpm.c

132f7629474424 Ashley Lai        2012-08-22   94  
132f7629474424 Ashley Lai        2012-08-22   95  /**
132f7629474424 Ashley Lai        2012-08-22   96   * tpm_ibmvtpm_recv - Receive data after send
93c12f293f8798 Winkler, Tomas    2016-11-23   97   *
132f7629474424 Ashley Lai        2012-08-22   98   * @chip:	tpm chip struct
132f7629474424 Ashley Lai        2012-08-22   99   * @buf:	buffer to read
93c12f293f8798 Winkler, Tomas    2016-11-23  100   * @count:	size of buffer
132f7629474424 Ashley Lai        2012-08-22  101   *
93c12f293f8798 Winkler, Tomas    2016-11-23  102   * Return:
132f7629474424 Ashley Lai        2012-08-22  103   *	Number of bytes read
132f7629474424 Ashley Lai        2012-08-22  104   */
132f7629474424 Ashley Lai        2012-08-22  105  static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
132f7629474424 Ashley Lai        2012-08-22  106  {
9e0d39d8a6a0a8 Christophe Ricard 2016-03-31  107  	struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
132f7629474424 Ashley Lai        2012-08-22  108  	u16 len;
b5666502700855 Ashley Lai        2012-09-12  109  	int sig;
132f7629474424 Ashley Lai        2012-08-22  110  
132f7629474424 Ashley Lai        2012-08-22  111  	if (!ibmvtpm->rtce_buf) {
132f7629474424 Ashley Lai        2012-08-22  112  		dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n");
132f7629474424 Ashley Lai        2012-08-22  113  		return 0;
132f7629474424 Ashley Lai        2012-08-22  114  	}
132f7629474424 Ashley Lai        2012-08-22  115  
6674ff145eef1f Stefan Berger     2015-12-09  116  	sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
b5666502700855 Ashley Lai        2012-09-12  117  	if (sig)
b5666502700855 Ashley Lai        2012-09-12  118  		return -EINTR;
b5666502700855 Ashley Lai        2012-09-12  119  
b5666502700855 Ashley Lai        2012-09-12  120  	len = ibmvtpm->res_len;
132f7629474424 Ashley Lai        2012-08-22  121  
b5666502700855 Ashley Lai        2012-09-12  122  	if (count < len) {
132f7629474424 Ashley Lai        2012-08-22  123  		dev_err(ibmvtpm->dev,
37ab03414829e5 Jason Gunthorpe   2013-09-14  124  			"Invalid size in recv: count=%zd, crq_size=%d\n",
b5666502700855 Ashley Lai        2012-09-12  125  			count, len);
132f7629474424 Ashley Lai        2012-08-22  126  		return -EIO;
132f7629474424 Ashley Lai        2012-08-22  127  	}
132f7629474424 Ashley Lai        2012-08-22  128  
132f7629474424 Ashley Lai        2012-08-22  129  	spin_lock(&ibmvtpm->rtce_lock);
b5666502700855 Ashley Lai        2012-09-12  130  	memcpy((void *)buf, (void *)ibmvtpm->rtce_buf, len);
b5666502700855 Ashley Lai        2012-09-12 @131  	memset(ibmvtpm->rtce_buf, 0, len);
b5666502700855 Ashley Lai        2012-09-12  132  	ibmvtpm->res_len = 0;
132f7629474424 Ashley Lai        2012-08-22  133  	spin_unlock(&ibmvtpm->rtce_lock);
132f7629474424 Ashley Lai        2012-08-22  134  	return len;
132f7629474424 Ashley Lai        2012-08-22  135  }
132f7629474424 Ashley Lai        2012-08-22  136  

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27407 bytes --]

^ permalink raw reply

* Re: [PATCH v5 17/26] powerpc/book3s64/keys: Print information during boot.
From: Michael Ellerman @ 2020-07-06  7:52 UTC (permalink / raw)
  To: Aneesh Kumar K.V, linuxppc-dev; +Cc: Aneesh Kumar K.V, linuxram, bauerman
In-Reply-To: <20200619135850.47155-18-aneesh.kumar@linux.ibm.com>

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:

> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  arch/powerpc/mm/book3s64/pkeys.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
> index 810118123e70..0d72c0246052 100644
> --- a/arch/powerpc/mm/book3s64/pkeys.c
> +++ b/arch/powerpc/mm/book3s64/pkeys.c
> @@ -195,6 +195,7 @@ void __init pkey_early_init_devtree(void)
>  	 */
>  	initial_allocation_mask |= reserved_allocation_mask;
>  
> +	pr_info("Enabling Memory keys with max key count %d", max_pkey);
                                                           ^
                                                           missing newline

>  	return;
>  }

The syscall is called pkey_mprotect() and the manpage is "pkeys", so I
think it would make sense to use "pkey" in the message.

cheers

^ permalink raw reply

* [RFC][PATCH] avoid refcounting the lazy tlb mm struct
From: Nicholas Piggin @ 2020-07-06  7:23 UTC (permalink / raw)
  To: linux-mm; +Cc: linux-arch, linuxppc-dev

On big systems, the mm refcount can become highly contented when doing
a lot of context switching with threaded applications (particularly
switching between the idle thread and an application thread).

Not doing lazy tlb at all slows switching down quite a bit, so I wonder
if we can avoid the refcount for the lazy tlb, but have __mmdrop() IPI
all CPUs that might be using this mm lazily.

This patch has only had light testing so far, but seems to work okay.

Thanks,
Nick

--

diff --git a/arch/Kconfig b/arch/Kconfig
index 8cc35dc556c7..69ea7172db3d 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -411,6 +411,16 @@ config MMU_GATHER_NO_GATHER
 	bool
 	depends on MMU_GATHER_TABLE_FREE
 
+config MMU_LAZY_TLB_SHOOTDOWN
+	bool
+	help
+	  Instead of refcounting the "lazy tlb" mm struct, which can cause
+	  contention with multi-threaded apps on large multiprocessor systems,
+	  this option causes __mmdrop to IPI all CPUs in the mm_cpumask and
+	  switch to init_mm if they were using the to-be-freed mm as the lazy
+	  tlb. Architectures which do not track all possible lazy tlb CPUs in
+	  mm_cpumask can not use this (without modification).
+
 config ARCH_HAVE_NMI_SAFE_CMPXCHG
 	bool
 
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 920c4e3ca4ef..24ac85c868db 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -225,6 +225,7 @@ config PPC
 	select HAVE_PERF_USER_STACK_DUMP
 	select MMU_GATHER_RCU_TABLE_FREE
 	select MMU_GATHER_PAGE_SIZE
+	select MMU_LAZY_TLB_SHOOTDOWN
 	select HAVE_REGS_AND_STACK_ACCESS_API
 	select HAVE_RELIABLE_STACKTRACE		if PPC_BOOK3S_64 && CPU_LITTLE_ENDIAN
 	select HAVE_SYSCALL_TRACEPOINTS
diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
index b5cc9b23cf02..52730629b3eb 100644
--- a/arch/powerpc/mm/book3s64/radix_tlb.c
+++ b/arch/powerpc/mm/book3s64/radix_tlb.c
@@ -652,10 +652,10 @@ static void do_exit_flush_lazy_tlb(void *arg)
 		 * Must be a kernel thread because sender is single-threaded.
 		 */
 		BUG_ON(current->mm);
-		mmgrab(&init_mm);
+		mmgrab_lazy_tlb(&init_mm);
 		switch_mm(mm, &init_mm, current);
 		current->active_mm = &init_mm;
-		mmdrop(mm);
+		mmdrop_lazy_tlb(mm);
 	}
 	_tlbiel_pid(pid, RIC_FLUSH_ALL);
 }
diff --git a/fs/exec.c b/fs/exec.c
index e6e8a9a70327..6c96c8feba1f 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1119,7 +1119,7 @@ static int exec_mmap(struct mm_struct *mm)
 		mmput(old_mm);
 		return 0;
 	}
-	mmdrop(active_mm);
+	mmdrop_lazy_tlb(active_mm);
 	return 0;
 }
 
diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index 480a4d1b7dd8..ef28059086a1 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -51,6 +51,25 @@ static inline void mmdrop(struct mm_struct *mm)
 
 void mmdrop(struct mm_struct *mm);
 
+static inline void mmgrab_lazy_tlb(struct mm_struct *mm)
+{
+	if (!IS_ENABLED(CONFIG_MMU_LAZY_TLB_SHOOTDOWN))
+		mmgrab(mm);
+}
+
+static inline void mmdrop_lazy_tlb(struct mm_struct *mm)
+{
+	if (!IS_ENABLED(CONFIG_MMU_LAZY_TLB_SHOOTDOWN))
+		mmdrop(mm);
+}
+
+static inline void mmdrop_lazy_tlb_smp_mb(struct mm_struct *mm)
+{
+	mmdrop_lazy_tlb(mm);
+	if (IS_ENABLED(CONFIG_MMU_LAZY_TLB_SHOOTDOWN))
+		smp_mb();
+}
+
 /*
  * This has to be called after a get_task_mm()/mmget_not_zero()
  * followed by taking the mmap_lock for writing before modifying the
diff --git a/kernel/fork.c b/kernel/fork.c
index 142b23645d82..e3f1039cee9f 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -685,6 +685,34 @@ static void check_mm(struct mm_struct *mm)
 #define allocate_mm()	(kmem_cache_alloc(mm_cachep, GFP_KERNEL))
 #define free_mm(mm)	(kmem_cache_free(mm_cachep, (mm)))
 
+static void do_shoot_lazy_tlb(void *arg)
+{
+	struct mm_struct *mm = arg;
+
+	if (current->active_mm == mm) {
+		BUG_ON(current->mm);
+		switch_mm(mm, &init_mm, current);
+		current->active_mm = &init_mm;
+	}
+}
+
+static void do_check_lazy_tlb(void *arg)
+{
+	struct mm_struct *mm = arg;
+
+	BUG_ON(current->active_mm == mm);
+}
+
+void shoot_lazy_tlbs(struct mm_struct *mm)
+{
+	if (IS_ENABLED(CONFIG_MMU_LAZY_TLB_SHOOTDOWN)) {
+		smp_call_function_many(mm_cpumask(mm), do_shoot_lazy_tlb, (void *)mm, 1);
+		do_shoot_lazy_tlb(mm);
+	}
+	smp_call_function(do_check_lazy_tlb, (void *)mm, 1);
+	do_check_lazy_tlb(mm);
+}
+
 /*
  * Called when the last reference to the mm
  * is dropped: either by a lazy thread or by
@@ -692,6 +720,7 @@ static void check_mm(struct mm_struct *mm)
  */
 void __mmdrop(struct mm_struct *mm)
 {
+	shoot_lazy_tlbs(mm);
 	BUG_ON(mm == &init_mm);
 	WARN_ON_ONCE(mm == current->mm);
 	WARN_ON_ONCE(mm == current->active_mm);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ca5db40392d4..4d615e0be9e0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3308,7 +3308,7 @@ static struct rq *finish_task_switch(struct task_struct *prev)
 	 */
 	if (mm) {
 		membarrier_mm_sync_core_before_usermode(mm);
-		mmdrop(mm);
+		mmdrop_lazy_tlb_smp_mb(mm);
 	}
 	if (unlikely(prev_state == TASK_DEAD)) {
 		if (prev->sched_class->task_dead)
@@ -3413,9 +3413,9 @@ context_switch(struct rq *rq, struct task_struct *prev,
 
 	/*
 	 * kernel -> kernel   lazy + transfer active
-	 *   user -> kernel   lazy + mmgrab() active
+	 *   user -> kernel   lazy + mmgrab_lazy_tlb() active
 	 *
-	 * kernel ->   user   switch + mmdrop() active
+	 * kernel ->   user   switch + mmdrop_lazy_tlb() active
 	 *   user ->   user   switch
 	 */
 	if (!next->mm) {                                // to kernel
@@ -3423,7 +3423,7 @@ context_switch(struct rq *rq, struct task_struct *prev,
 
 		next->active_mm = prev->active_mm;
 		if (prev->mm)                           // from user
-			mmgrab(prev->active_mm);
+			mmgrab_lazy_tlb(prev->active_mm);
 		else
 			prev->active_mm = NULL;
 	} else {                                        // to user
@@ -3439,7 +3439,7 @@ context_switch(struct rq *rq, struct task_struct *prev,
 		switch_mm_irqs_off(prev->active_mm, next->mm, next);
 
 		if (!prev->mm) {                        // from kernel
-			/* will mmdrop() in finish_task_switch(). */
+			/* will mmdrop_lazy_tlb() in finish_task_switch(). */
 			rq->prev_mm = prev->active_mm;
 			prev->active_mm = NULL;
 		}

^ permalink raw reply related

* Re: [PATCH v5 10/26] powerpc/book3s64/pkeys: Convert pkey_total to max_pkey
From: Aneesh Kumar K.V @ 2020-07-06  7:20 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: linuxram, bauerman
In-Reply-To: <87tuyl5dko.fsf@mpe.ellerman.id.au>

On 7/6/20 12:34 PM, Michael Ellerman wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>> max_pkey now represents max key value that userspace can allocate.
>>

I guess commit message is confusing.

>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> ---
>>   arch/powerpc/include/asm/pkeys.h |  7 +++++--
>>   arch/powerpc/mm/book3s64/pkeys.c | 14 +++++++-------
>>   2 files changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
>> index 75d2a2c19c04..652bad7334f3 100644
>> --- a/arch/powerpc/include/asm/pkeys.h
>> +++ b/arch/powerpc/include/asm/pkeys.h
>> @@ -12,7 +12,7 @@
>>   #include <asm/firmware.h>
>>   
>>   DECLARE_STATIC_KEY_FALSE(pkey_disabled);
>> -extern int pkeys_total; /* total pkeys as per device tree */
>> +extern int max_pkey;
>>   extern u32 initial_allocation_mask; /*  bits set for the initially allocated keys */
>>   extern u32 reserved_allocation_mask; /* bits set for reserved keys */
>>   
>> @@ -44,7 +44,10 @@ static inline int vma_pkey(struct vm_area_struct *vma)
>>   	return (vma->vm_flags & ARCH_VM_PKEY_FLAGS) >> VM_PKEY_SHIFT;
>>   }
>>   
>> -#define arch_max_pkey() pkeys_total
>> +static inline int arch_max_pkey(void)
>> +{
>> +	return max_pkey;
>> +}
> 
> If pkeys_total = 32 then max_pkey = 31.

we have

#ifdef CONFIG_PPC_4K_PAGES
	/*
	 * The OS can manage only 8 pkeys due to its inability to represent them
	 * in the Linux 4K PTE. Mark all other keys reserved.
	 */
	max_pkey = min(8, pkeys_total);
#else
	max_pkey = pkeys_total;
#endif

so it is 32.

> 
> So we can't just substitute one for the other. ie. arch_max_pkey() must
> have been wrong, or it is wrong now.
> 
>> diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
>> index 87d882a9aaf2..a4d7287082a8 100644
>> --- a/arch/powerpc/mm/book3s64/pkeys.c
>> +++ b/arch/powerpc/mm/book3s64/pkeys.c
>> @@ -14,7 +14,7 @@
>>   
>>   DEFINE_STATIC_KEY_FALSE(pkey_disabled);
>>   DEFINE_STATIC_KEY_FALSE(execute_pkey_disabled);
>> -int  pkeys_total;		/* Total pkeys as per device tree */
>> +int  max_pkey;			/* Maximum key value supported */
>>   u32  initial_allocation_mask;   /* Bits set for the initially allocated keys */
>>   /*
>>    *  Keys marked in the reservation list cannot be allocated by  userspace
>> @@ -84,7 +84,7 @@ static int scan_pkey_feature(void)
>>   
>>   static int pkey_initialize(void)
>>   {
>> -	int os_reserved, i;
>> +	int pkeys_total, i;
>>   
>>   	/*
>>   	 * We define PKEY_DISABLE_EXECUTE in addition to the arch-neutral
>> @@ -122,12 +122,12 @@ static int pkey_initialize(void)
>>   	 * The OS can manage only 8 pkeys due to its inability to represent them
>>   	 * in the Linux 4K PTE. Mark all other keys reserved.
>>   	 */
>> -	os_reserved = pkeys_total - 8;
>> +	max_pkey = min(8, pkeys_total);
> 
> Shouldn't it be 7 ?
> 
>>   #else
>> -	os_reserved = 0;
>> +	max_pkey = pkeys_total;
>>   #endif
>>   
>> -	if (unlikely((pkeys_total - os_reserved) <= execute_only_key)) {
>> +	if (unlikely(max_pkey <= execute_only_key)) {
> 
> Isn't that an off-by-one now?
> 
> This is one-off boot time code, there's no need to clutter it with
> unlikely.
> 
>>   		/*
>>   		 * Insufficient number of keys to support
>>   		 * execute only key. Mark it unavailable.
>> @@ -174,10 +174,10 @@ static int pkey_initialize(void)
>>   	default_uamor &= ~(0x3ul << pkeyshift(1));
>>   
>>   	/*
>> -	 * Prevent the usage of OS reserved the keys. Update UAMOR
>> +	 * Prevent the usage of OS reserved keys. Update UAMOR
>>   	 * for those keys.
>>   	 */
>> -	for (i = (pkeys_total - os_reserved); i < pkeys_total; i++) {
>> +	for (i = max_pkey; i < pkeys_total; i++) {
> 
> Another off-by-one? Shouldn't we start from max_pkey + 1 ?
> 
>>   		reserved_allocation_mask |= (0x1 << i);
>>   		default_uamor &= ~(0x3ul << pkeyshift(i));
>>   	}
> 

It is the commit message. It is max pkey such that userspace can 
allocate 0 - maxp_pkey -1.

-aneesh

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox