LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v7 1/2] tty: hvc: pass DMA capable memory to put_chars()
From: Greg KH @ 2021-08-17 15:48 UTC (permalink / raw)
  To: Xianting Tian
  Cc: arnd, amit, jirislaby, linux-kernel, virtualization, linuxppc-dev,
	osandov
In-Reply-To: <20210817132300.165014-2-xianting.tian@linux.alibaba.com>

On Tue, Aug 17, 2021 at 09:22:59PM +0800, Xianting Tian wrote:
> We tested the patch, it worked normally.

Who is "we"?

> Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com>

Like I said before, I need another developer from your company to review
and sign-off on this patch (and the other one), before I am willing to
look at it, based on the previous mistakes that have happened here.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH v2 09/12] mm: Remove the now unused mem_encrypt_active() function
From: Tom Lendacky @ 2021-08-17 15:30 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-s390, Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh,
	kvm, Joerg Roedel, x86, kexec, linux-kernel, amd-gfx,
	platform-driver-x86, iommu, Andi Kleen, linux-graphics-maintainer,
	dri-devel, Joerg Roedel, linux-fsdevel, Tianyu Lan, linuxppc-dev
In-Reply-To: <YRuOVOdxOZm0S86j@zn.tnic>

On 8/17/21 5:24 AM, Borislav Petkov wrote:
> On Tue, Aug 17, 2021 at 12:22:33PM +0200, Borislav Petkov wrote:
>> This one wants to be part of the previous patch.
> 
> ... and the three following patches too - the treewide patch does a
> single atomic :) replacement and that's it.

Ok, I'll squash those all together.

Thanks,
Tom

> 

^ permalink raw reply

* Re: [PATCH v2 06/12] x86/sev: Replace occurrences of sev_active() with prot_guest_has()
From: Tom Lendacky @ 2021-08-17 15:26 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh, kvm,
	Peter Zijlstra, Dave Hansen, dri-devel, platform-driver-x86,
	Ard Biesheuvel, linux-s390, Andi Kleen, Joerg Roedel, x86,
	amd-gfx, Ingo Molnar, linux-graphics-maintainer, Joerg Roedel,
	Tianyu Lan, Andy Lutomirski, Thomas Gleixner, kexec, linux-kernel,
	iommu, linux-fsdevel, linuxppc-dev
In-Reply-To: <YRuJPqxFZ6ItZd++@zn.tnic>

On 8/17/21 5:02 AM, Borislav Petkov wrote:
> On Fri, Aug 13, 2021 at 11:59:25AM -0500, Tom Lendacky wrote:
>> diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
>> index 8e7b517ad738..66ff788b79c9 100644
>> --- a/arch/x86/kernel/machine_kexec_64.c
>> +++ b/arch/x86/kernel/machine_kexec_64.c
>> @@ -167,7 +167,7 @@ static int init_transition_pgtable(struct kimage *image, pgd_t *pgd)
>>  	}
>>  	pte = pte_offset_kernel(pmd, vaddr);
>>  
>> -	if (sev_active())
>> +	if (prot_guest_has(PATTR_GUEST_MEM_ENCRYPT))
>>  		prot = PAGE_KERNEL_EXEC;
>>  
>>  	set_pte(pte, pfn_pte(paddr >> PAGE_SHIFT, prot));
>> @@ -207,7 +207,7 @@ static int init_pgtable(struct kimage *image, unsigned long start_pgtable)
>>  	level4p = (pgd_t *)__va(start_pgtable);
>>  	clear_page(level4p);
>>  
>> -	if (sev_active()) {
>> +	if (prot_guest_has(PATTR_GUEST_MEM_ENCRYPT)) {
>>  		info.page_flag   |= _PAGE_ENC;
>>  		info.kernpg_flag |= _PAGE_ENC;
>>  	}
>> @@ -570,12 +570,12 @@ void arch_kexec_unprotect_crashkres(void)
>>   */
>>  int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, gfp_t gfp)
>>  {
>> -	if (sev_active())
>> +	if (!prot_guest_has(PATTR_HOST_MEM_ENCRYPT))
>>  		return 0;
>>  
>>  	/*
>> -	 * If SME is active we need to be sure that kexec pages are
>> -	 * not encrypted because when we boot to the new kernel the
>> +	 * If host memory encryption is active we need to be sure that kexec
>> +	 * pages are not encrypted because when we boot to the new kernel the
>>  	 * pages won't be accessed encrypted (initially).
>>  	 */
> 
> That hunk belongs logically into the previous patch which removes
> sme_active().

I was trying to keep the sev_active() changes separate... so even though
it's an SME thing, I kept it here. But I can move it to the previous
patch, it just might look strange.

> 
>>  	return set_memory_decrypted((unsigned long)vaddr, pages);
>> @@ -583,12 +583,12 @@ int arch_kexec_post_alloc_pages(void *vaddr, unsigned int pages, gfp_t gfp)
>>  
>>  void arch_kexec_pre_free_pages(void *vaddr, unsigned int pages)
>>  {
>> -	if (sev_active())
>> +	if (!prot_guest_has(PATTR_HOST_MEM_ENCRYPT))
>>  		return;
>>  
>>  	/*
>> -	 * If SME is active we need to reset the pages back to being
>> -	 * an encrypted mapping before freeing them.
>> +	 * If host memory encryption is active we need to reset the pages back
>> +	 * to being an encrypted mapping before freeing them.
>>  	 */
>>  	set_memory_encrypted((unsigned long)vaddr, pages);
>>  }
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index e8ccab50ebf6..b69f5ac622d5 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
>> @@ -25,6 +25,7 @@
>>  #include <linux/pagemap.h>
>>  #include <linux/swap.h>
>>  #include <linux/rwsem.h>
>> +#include <linux/protected_guest.h>
>>  
>>  #include <asm/apic.h>
>>  #include <asm/perf_event.h>
>> @@ -457,7 +458,7 @@ static int has_svm(void)
>>  		return 0;
>>  	}
>>  
>> -	if (sev_active()) {
>> +	if (prot_guest_has(PATTR_SEV)) {
>>  		pr_info("KVM is unsupported when running as an SEV guest\n");
>>  		return 0;
> 
> Same question as for PATTR_SME. PATTR_GUEST_MEM_ENCRYPT should be enough.

Yup, I'll change them all.

> 
>> @@ -373,7 +373,7 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
>>   * up under SME the trampoline area cannot be encrypted, whereas under SEV
>>   * the trampoline area must be encrypted.
>>   */
>> -bool sev_active(void)
>> +static bool sev_active(void)
>>  {
>>  	return sev_status & MSR_AMD64_SEV_ENABLED;
>>  }
>> @@ -382,7 +382,6 @@ static bool sme_active(void)
>>  {
>>  	return sme_me_mask && !sev_active();
>>  }
>> -EXPORT_SYMBOL_GPL(sev_active);
> 
> Just get rid of it altogether.

Ok.

Thanks,
Tom

> 
> Thx.
> 

^ permalink raw reply

* Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
From: Tom Lendacky @ 2021-08-17 15:22 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh, kvm,
	Peter Zijlstra, Dave Hansen, dri-devel, platform-driver-x86,
	linux-s390, Andi Kleen, Joerg Roedel, x86, amd-gfx, Ingo Molnar,
	linux-graphics-maintainer, Joerg Roedel, Tianyu Lan,
	Andy Lutomirski, Thomas Gleixner, kexec, linux-kernel, iommu,
	linux-fsdevel, linuxppc-dev
In-Reply-To: <YRknDQGUJJ/j9pth@zn.tnic>

On 8/15/21 9:39 AM, Borislav Petkov wrote:
> On Sun, Aug 15, 2021 at 08:53:31AM -0500, Tom Lendacky wrote:
>> It's not a cross-vendor thing as opposed to a KVM or other hypervisor
>> thing where the family doesn't have to be reported as AMD or HYGON.
> 
> What would be the use case? A HV starts a guest which is supposed to be
> encrypted using the AMD's confidential guest technology but the HV tells
> the guest that it is not running on an AMD SVM HV but something else?
> 
> Is that even an actual use case?
> 
> Or am I way off?
> 
> I know we have talked about this in the past but this still sounds
> insane.

Maybe the KVM folks have a better understanding of it...

I can change it to be an AMD/HYGON check...  although, I'll have to check
to see if any (very) early use of the function will work with that.

At a minimum, the check in arch/x86/kernel/head64.c will have to be
changed or removed. I'll take a closer look.

Thanks,
Tom

> 

^ permalink raw reply

* Re: [PATCH v2 05/12] x86/sme: Replace occurrences of sme_active() with prot_guest_has()
From: Tom Lendacky @ 2021-08-17 14:46 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh, kvm,
	Peter Zijlstra, Dave Hansen, dri-devel, platform-driver-x86,
	Will Deacon, linux-s390, Andi Kleen, Joerg Roedel, x86, amd-gfx,
	Ingo Molnar, linux-graphics-maintainer, Joerg Roedel, Tianyu Lan,
	Andy Lutomirski, Thomas Gleixner, kexec, linux-kernel, iommu,
	linux-fsdevel, linuxppc-dev
In-Reply-To: <YRt6yCNCBLwyyx5X@zn.tnic>

On 8/17/21 4:00 AM, Borislav Petkov wrote:
> On Fri, Aug 13, 2021 at 11:59:24AM -0500, Tom Lendacky wrote:
>> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
>> index edc67ddf065d..5635ca9a1fbe 100644
>> --- a/arch/x86/mm/mem_encrypt.c
>> +++ b/arch/x86/mm/mem_encrypt.c
>> @@ -144,7 +144,7 @@ void __init sme_unmap_bootdata(char *real_mode_data)
>>  	struct boot_params *boot_data;
>>  	unsigned long cmdline_paddr;
>>  
>> -	if (!sme_active())
>> +	if (!amd_prot_guest_has(PATTR_SME))
>>  		return;
>>  
>>  	/* Get the command line address before unmapping the real_mode_data */
>> @@ -164,7 +164,7 @@ void __init sme_map_bootdata(char *real_mode_data)
>>  	struct boot_params *boot_data;
>>  	unsigned long cmdline_paddr;
>>  
>> -	if (!sme_active())
>> +	if (!amd_prot_guest_has(PATTR_SME))
>>  		return;
>>  
>>  	__sme_early_map_unmap_mem(real_mode_data, sizeof(boot_params), true);
>> @@ -378,7 +378,7 @@ bool sev_active(void)
>>  	return sev_status & MSR_AMD64_SEV_ENABLED;
>>  }
>>  
>> -bool sme_active(void)
>> +static bool sme_active(void)
> 
> Just get rid of it altogether. Also, there's an
> 
> EXPORT_SYMBOL_GPL(sev_active);
> > which needs to go under the actual function. Here's a diff ontop:

Will do.

> 
> ---
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index 5635ca9a1fbe..a3a2396362a5 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -364,8 +364,9 @@ int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size)
>  /*
>   * SME and SEV are very similar but they are not the same, so there are
>   * times that the kernel will need to distinguish between SME and SEV. The
> - * sme_active() and sev_active() functions are used for this.  When a
> - * distinction isn't needed, the mem_encrypt_active() function can be used.
> + * PATTR_HOST_MEM_ENCRYPT and PATTR_GUEST_MEM_ENCRYPT flags to
> + * amd_prot_guest_has() are used for this. When a distinction isn't needed,
> + * the mem_encrypt_active() function can be used.
>   *
>   * The trampoline code is a good example for this requirement.  Before
>   * paging is activated, SME will access all memory as decrypted, but SEV
> @@ -377,11 +378,6 @@ bool sev_active(void)
>  {
>  	return sev_status & MSR_AMD64_SEV_ENABLED;
>  }
> -
> -static bool sme_active(void)
> -{
> -	return sme_me_mask && !sev_active();
> -}
>  EXPORT_SYMBOL_GPL(sev_active);
>  
>  /* Needs to be called from non-instrumentable code */
> @@ -398,7 +394,7 @@ bool amd_prot_guest_has(unsigned int attr)
>  
>  	case PATTR_SME:
>  	case PATTR_HOST_MEM_ENCRYPT:
> -		return sme_active();
> +		return sme_me_mask && !sev_active();
>  
>  	case PATTR_SEV:
>  	case PATTR_GUEST_MEM_ENCRYPT:
> 
>>  {
>>  	return sme_me_mask && !sev_active();
>>  }
>> @@ -428,7 +428,7 @@ bool force_dma_unencrypted(struct device *dev)
>>  	 * device does not support DMA to addresses that include the
>>  	 * encryption mask.
>>  	 */
>> -	if (sme_active()) {
>> +	if (amd_prot_guest_has(PATTR_SME)) {
> 
> So I'm not sure: you add PATTR_SME which you call with
> amd_prot_guest_has() and PATTR_HOST_MEM_ENCRYPT which you call with
> prot_guest_has() and they both end up being the same thing on AMD.
> 
> So why even bother with PATTR_SME?
> 
> This is only going to cause confusion later and I'd say let's simply use
> prot_guest_has(PATTR_HOST_MEM_ENCRYPT) everywhere...

Ok, I can do that. I was trying to ensure that anything that is truly SME
or SEV specific would be called out now.

I'm ok with letting the TDX folks make changes to these calls to be SME or
SEV specific, if necessary, later.

Thanks,
Tom

> 

^ permalink raw reply

* [PATCH] powerpc/32s: Fix random crashes by adding isync() after locking/unlocking KUEP
From: Christophe Leroy @ 2021-08-17 14:43 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, userm57,
	fthain
  Cc: linuxppc-dev, linux-kernel

Commit b5efec00b671 ("powerpc/32s: Move KUEP locking/unlocking in C")
removed the 'isync' instruction after adding/removing NX bit in user
segments. The reasoning behind this change was that when setting the
NX bit we don't mind it taking effect with delay as the kernel never
executes text from userspace, and when clearing the NX bit this is
to return to userspace and then the 'rfi' should synchronise the
context.

However, it looks like on book3s/32 having a hash page table, at least
on the G3 processor, we get an unexpected fault from userspace, then
this is followed by something wrong in the verification of MSR_PR
at end of another interrupt.

This is fixed by adding back the removed isync() following update
of NX bit in user segment registers. Only do it for cores with an
hash table, as 603 cores don't exhibit that problem and the two isync
increase ./null_syscall selftest by 6 cycles on an MPC 832x.

First problem: unexpected PROTFAULT

	[   62.896426] WARNING: CPU: 0 PID: 1660 at arch/powerpc/mm/fault.c:354 do_page_fault+0x6c/0x5b0
	[   62.918111] Modules linked in:
	[   62.923350] CPU: 0 PID: 1660 Comm: Xorg Not tainted 5.13.0-pmac-00028-gb3c15b60339a #40
	[   62.943476] NIP:  c001b5c8 LR: c001b6f8 CTR: 00000000
	[   62.954714] REGS: e2d09e40 TRAP: 0700   Not tainted  (5.13.0-pmac-00028-gb3c15b60339a)
	[   62.974581] MSR:  00021032 <ME,IR,DR,RI>  CR: 42d04f30  XER: 20000000
	[   62.990009]
        	       GPR00: c000424c e2d09f00 c301b680 e2d09f40 0000001e 42000000 00cba028 00000000
        	       GPR08: 08000000 48000010 c301b680 e2d09f30 22d09f30 00c1fff0 00cba000 a7b7ba4c
        	       GPR16: 00000031 00000000 00000000 00000000 00000000 00000000 a7b7b0d0 00c5c010
        	       GPR24: a7b7b64c a7b7d2f0 00000004 00000000 c1efa6c0 00cba02c 00000300 e2d09f40
	[   63.075238] NIP [c001b5c8] do_page_fault+0x6c/0x5b0
	[   63.085952] LR [c001b6f8] do_page_fault+0x19c/0x5b0
	[   63.096678] Call Trace:
	[   63.100075] [e2d09f00] [e2d09f04] 0xe2d09f04 (unreliable)
	[   63.112359] [e2d09f30] [c000424c] DataAccess_virt+0xd4/0xe4
	[   63.125168] --- interrupt: 300 at 0xa7a261dc
	[   63.134060] NIP:  a7a261dc LR: a7a253bc CTR: 00000000
	[   63.145302] REGS: e2d09f40 TRAP: 0300   Not tainted  (5.13.0-pmac-00028-gb3c15b60339a)
	[   63.165167] MSR:  0000d032 <EE,PR,ME,IR,DR,RI>  CR: 228428e2  XER: 20000000
	[   63.182162] DAR: 00cba02c DSISR: 42000000
        	       GPR00: a7a27448 afa6b0e0 a74c35c0 a7b7b614 0000001e a7b7b614 00cba028 00000000
        	       GPR08: 00020fd9 00000031 00cb9ff8 a7a273b0 220028e2 00c1fff0 00cba000 a7b7ba4c
        	       GPR16: 00000031 00000000 00000000 00000000 00000000 00000000 a7b7b0d0 00c5c010
        	       GPR24: a7b7b64c a7b7d2f0 00000004 00000002 0000001e a7b7b614 a7b7aff4 00000030
	[   63.275233] NIP [a7a261dc] 0xa7a261dc
	[   63.282291] LR [a7a253bc] 0xa7a253bc
	[   63.289087] --- interrupt: 300
	[   63.294322] Instruction dump:
	[   63.299291] 7c4a1378 810300a0 75278410 83820298 83a300a4 553b018c 551e0036 4082038c
	[   63.318630] 2e1b0000 40920228 75280800 41820220 <0fe00000> 3b600000 41920214 81420594
	[   63.338503] ---[ end trace f642a84639cba377 ]---

Second problem: MSR PR is seen unset allthough the interrupt frame shows it set

	[   63.666846] kernel BUG at arch/powerpc/kernel/interrupt.c:458!
	[   63.680633] Oops: Exception in kernel mode, sig: 5 [#1]
	[   63.692201] BE PAGE_SIZE=4K MMU=Hash SMP NR_CPUS=2 PowerMac
	[   63.705011] Modules linked in:
	[   63.710247] CPU: 0 PID: 1660 Comm: Xorg Tainted: G        W         5.13.0-pmac-00028-gb3c15b60339a #40
	[   63.734553] NIP:  c0011434 LR: c001629c CTR: 00000000
	[   63.745796] REGS: e2d09e70 TRAP: 0700   Tainted: G        W          (5.13.0-pmac-00028-gb3c15b60339a)
	[   63.769844] MSR:  00029032 <EE,ME,IR,DR,RI>  CR: 42d09f30  XER: 00000000
	[   63.786052]
        	       GPR00: 00000000 e2d09f30 c301b680 e2d09f40 83440000 c44d0e68 e2d09e8c 00000000
        	       GPR08: 00000002 00dc228a 00004000 e2d09f30 22d09f30 00c1fff0 afa6ceb4 00c26144
        	       GPR16: 00c25fb8 00c26140 afa6ceb8 90000000 00c944d8 0000001c 00000000 00200000
        	       GPR24: 00000000 000001fb afa6d1b4 00000001 00000000 a539a2a0 a530fd80 00000089
	[   63.871284] NIP [c0011434] interrupt_exit_kernel_prepare+0x10/0x70
	[   63.885922] LR [c001629c] interrupt_return+0x9c/0x144
	[   63.897168] Call Trace:
	[   63.900562] [e2d09f30] [c000424c] DataAccess_virt+0xd4/0xe4 (unreliable)
	[   63.916773] --- interrupt: 300 at 0xa09be008
	[   63.925659] NIP:  a09be008 LR: a09bdfe8 CTR: a09bdfc0
	[   63.936903] REGS: e2d09f40 TRAP: 0300   Tainted: G        W          (5.13.0-pmac-00028-gb3c15b60339a)
	[   63.960953] MSR:  0000d032 <EE,PR,ME,IR,DR,RI>  CR: 420028e2  XER: 20000000
	[   63.977948] DAR: a539a308 DSISR: 0a000000
        	       GPR00: a7b90d50 afa6b2d0 a74c35c0 a0a8b690 a0a8b698 a5365d70 a4fa82a8 00000004
        	       GPR08: 00000000 a09bdfc0 00000000 a5360000 a09bde7c 00c1fff0 afa6ceb4 00c26144
        	       GPR16: 00c25fb8 00c26140 afa6ceb8 90000000 00c944d8 0000001c 00000000 00200000
        	       GPR24: 00000000 000001fb afa6d1b4 00000001 00000000 a539a2a0 a530fd80 00000089
	[   64.071020] NIP [a09be008] 0xa09be008
	[   64.078079] LR [a09bdfe8] 0xa09bdfe8
	[   64.084874] --- interrupt: 300
	[   64.090108] Instruction dump:
	[   64.095074] 80010024 83e1001c 7c0803a6 4bffff80 3bc00800 4bffffd0 486b42fd 4bffffcc
	[   64.114419] 81430084 71480002 41820038 554a0462 <0f0a0000> 80620060 74630001 40820034
	[   64.134298] ---[ end trace f642a84639cba378 ]---

Reported-by: Stan Johnson <userm57@yahoo.com>
Fixes: b5efec00b671 ("powerpc/32s: Move KUEP locking/unlocking in C")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/book3s/32/kup.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
index 64201125a287..2658d30b223c 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -4,6 +4,8 @@
 
 #include <asm/bug.h>
 #include <asm/book3s/32/mmu-hash.h>
+#include <asm/mmu.h>
+#include <asm/synch.h>
 
 #ifndef __ASSEMBLY__
 
@@ -28,6 +30,8 @@ static inline void kuep_lock(void)
 		return;
 
 	update_user_segments(mfsr(0) | SR_NX);
+	if (mmu_has_feature(MMU_FTR_HPTE_TABLE))
+		isync();	/* Context sync required after mtsr() */
 }
 
 static inline void kuep_unlock(void)
@@ -36,6 +40,8 @@ static inline void kuep_unlock(void)
 		return;
 
 	update_user_segments(mfsr(0) & ~SR_NX);
+	if (mmu_has_feature(MMU_FTR_HPTE_TABLE))
+		isync();	/* Context sync required after mtsr() */
 }
 
 #ifdef CONFIG_PPC_KUAP
-- 
2.25.0


^ permalink raw reply related

* Re: [PATCH] powerpc/mm: Fix set_memory_*() against concurrent accesses
From: Christophe Leroy @ 2021-08-17 14:28 UTC (permalink / raw)
  To: Fabiano Rosas, Michael Ellerman, linuxppc-dev
  Cc: lvivier, jniethe5, npiggin, aneesh.kumar
In-Reply-To: <87sfz8tam3.fsf@linux.ibm.com>



Le 17/08/2021 à 16:21, Fabiano Rosas a écrit :
> Michael Ellerman <mpe@ellerman.id.au> writes:
> 
> Hi, I already mentioned these things in private, but I'll post here so
> everyone can see:
> 
>> Because pte_update() takes the set of PTE bits to set and clear we can't
>> use our existing helpers, eg. pte_wrprotect() etc. and instead have to
>> open code the set of flags. We will clean that up somehow in a future
>> commit.
> 
> I tested the following on P9 and it seems to work fine. Not sure if it
> works for CONFIG_PPC_8xx, though.
> 
> 
>   static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
>   {
>   	long action = (long)data;
>   	pte_t pte;
>   
>   	spin_lock(&init_mm.page_table_lock);
> -
> -	/* invalidate the PTE so it's safe to modify */
> -	pte = ptep_get_and_clear(&init_mm, addr, ptep);
> -	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> +	pte = *ptep;

Maybe using ptep_get() is better.

>   
>   	/* modify the PTE bits as desired, then apply */
>   	switch (action) {
> @@ -59,11 +42,9 @@ static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
>   		break;
>   	}
>   
> -	set_pte_at(&init_mm, addr, ptep, pte);
> +	pte_update(&init_mm, addr, ptep, ~0UL, pte_val(pte), 0);

Good simple idea, indeed yes it should work with much more effort.


> +	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
>   
> -	/* See ptesync comment in radix__set_pte_at() */
> -	if (radix_enabled())
> -		asm volatile("ptesync": : :"memory");
>   	spin_unlock(&init_mm.page_table_lock);
>   
>   	return 0;
> ---
> 
> For reference, the full patch is here:
> https://github.com/farosas/linux/commit/923c95c84d7081d7be9503bf5b276dd93bd17036.patch
> 
>>
>> [1]: https://lore.kernel.org/linuxppc-dev/87y318wp9r.fsf@linux.ibm.com/
>>
>> Fixes: 1f9ad21c3b38 ("powerpc/mm: Implement set_memory() routines")
>> Reported-by: Laurent Vivier <lvivier@redhat.com>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
> 
> ...
> 
>> -	set_pte_at(&init_mm, addr, ptep, pte);
>> +	pte_update(&init_mm, addr, ptep, clear, set, 0);
>>   
>>   	/* See ptesync comment in radix__set_pte_at() */
>>   	if (radix_enabled())
>>   		asm volatile("ptesync": : :"memory");
>> +
>> +	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> 
> I think there's an optimization possible here, when relaxing access, to
> skip the TLB flush. Would still need the ptesync though. Similar to what
> Nick did in e5f7cb58c2b7 ("powerpc/64s/radix: do not flush TLB when
> relaxing access"). It is out of scope for this patch but maybe worth
> thinking about.
> 
>> +
>>   	spin_unlock(&init_mm.page_table_lock);
>>   
>>   	return 0;
>>
>> base-commit: cbc06f051c524dcfe52ef0d1f30647828e226d30

^ permalink raw reply

* Re: [PATCH] powerpc/mm: Fix set_memory_*() against concurrent accesses
From: Fabiano Rosas @ 2021-08-17 14:21 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: lvivier, jniethe5, npiggin, aneesh.kumar
In-Reply-To: <20210817132552.3375738-1-mpe@ellerman.id.au>

Michael Ellerman <mpe@ellerman.id.au> writes:

Hi, I already mentioned these things in private, but I'll post here so
everyone can see:

> Because pte_update() takes the set of PTE bits to set and clear we can't
> use our existing helpers, eg. pte_wrprotect() etc. and instead have to
> open code the set of flags. We will clean that up somehow in a future
> commit.

I tested the following on P9 and it seems to work fine. Not sure if it
works for CONFIG_PPC_8xx, though.


 static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
 {
 	long action = (long)data;
 	pte_t pte;
 
 	spin_lock(&init_mm.page_table_lock);
-
-	/* invalidate the PTE so it's safe to modify */
-	pte = ptep_get_and_clear(&init_mm, addr, ptep);
-	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+	pte = *ptep;
 
 	/* modify the PTE bits as desired, then apply */
 	switch (action) {
@@ -59,11 +42,9 @@ static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
 		break;
 	}
 
-	set_pte_at(&init_mm, addr, ptep, pte);
+	pte_update(&init_mm, addr, ptep, ~0UL, pte_val(pte), 0);
+	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
 
-	/* See ptesync comment in radix__set_pte_at() */
-	if (radix_enabled())
-		asm volatile("ptesync": : :"memory");
 	spin_unlock(&init_mm.page_table_lock);
 
 	return 0;
---

For reference, the full patch is here:
https://github.com/farosas/linux/commit/923c95c84d7081d7be9503bf5b276dd93bd17036.patch

>
> [1]: https://lore.kernel.org/linuxppc-dev/87y318wp9r.fsf@linux.ibm.com/
>
> Fixes: 1f9ad21c3b38 ("powerpc/mm: Implement set_memory() routines")
> Reported-by: Laurent Vivier <lvivier@redhat.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---

...

> -	set_pte_at(&init_mm, addr, ptep, pte);
> +	pte_update(&init_mm, addr, ptep, clear, set, 0);
>  
>  	/* See ptesync comment in radix__set_pte_at() */
>  	if (radix_enabled())
>  		asm volatile("ptesync": : :"memory");
> +
> +	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);

I think there's an optimization possible here, when relaxing access, to
skip the TLB flush. Would still need the ptesync though. Similar to what
Nick did in e5f7cb58c2b7 ("powerpc/64s/radix: do not flush TLB when
relaxing access"). It is out of scope for this patch but maybe worth
thinking about.

> +
>  	spin_unlock(&init_mm.page_table_lock);
>  
>  	return 0;
>
> base-commit: cbc06f051c524dcfe52ef0d1f30647828e226d30

^ permalink raw reply

* Re: [PATCH] powerpc/mm: Fix set_memory_*() against concurrent accesses
From: Christophe Leroy @ 2021-08-17 14:20 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev
  Cc: lvivier, jniethe5, aneesh.kumar, npiggin, farosas
In-Reply-To: <20210817132552.3375738-1-mpe@ellerman.id.au>



Le 17/08/2021 à 15:25, Michael Ellerman a écrit :
> Laurent reported that STRICT_MODULE_RWX was causing intermittent crashes
> on one of his systems:
> 
>    kernel tried to execute exec-protected page (c008000004073278) - exploit attempt? (uid: 0)
>    BUG: Unable to handle kernel instruction fetch
>    Faulting instruction address: 0xc008000004073278
>    Oops: Kernel access of bad area, sig: 11 [#1]
>    LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
>    Modules linked in: drm virtio_console fuse drm_panel_orientation_quirks ...
>    CPU: 3 PID: 44 Comm: kworker/3:1 Not tainted 5.14.0-rc4+ #12
>    Workqueue: events control_work_handler [virtio_console]
>    NIP:  c008000004073278 LR: c008000004073278 CTR: c0000000001e9de0
>    REGS: c00000002e4ef7e0 TRAP: 0400   Not tainted  (5.14.0-rc4+)
>    MSR:  800000004280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24002822 XER: 200400cf
>    ...
>    NIP fill_queue+0xf0/0x210 [virtio_console]
>    LR  fill_queue+0xf0/0x210 [virtio_console]
>    Call Trace:
>      fill_queue+0xb4/0x210 [virtio_console] (unreliable)
>      add_port+0x1a8/0x470 [virtio_console]
>      control_work_handler+0xbc/0x1e8 [virtio_console]
>      process_one_work+0x290/0x590
>      worker_thread+0x88/0x620
>      kthread+0x194/0x1a0
>      ret_from_kernel_thread+0x5c/0x64
> 
> Jordan, Fabiano & Murilo were able to reproduce and identify that the
> problem is caused by the call to module_enable_ro() in do_init_module(),
> which happens after the module's init function has already been called.
> 
> Our current implementation of change_page_attr() is not safe against
> concurrent accesses, because it invalidates the PTE before flushing the
> TLB and then installing the new PTE. That leaves a window in time where
> there is no valid PTE for the page, if another CPU tries to access the
> page at that time we see something like the fault above.
> 
> We can't simply switch to set_pte_at()/flush TLB, because our hash MMU
> code doesn't handle a set_pte_at() of a valid PTE. See [1].
> 
> But we do have pte_update(), which replaces the old PTE with the new,
> meaning there's no window where the PTE is invalid. And the hash MMU
> version hash__pte_update() deals with synchronising the hash page table
> correctly.
> 
> Because pte_update() takes the set of PTE bits to set and clear we can't
> use our existing helpers, eg. pte_wrprotect() etc. and instead have to
> open code the set of flags. We will clean that up somehow in a future
> commit.
> 
> [1]: https://lore.kernel.org/linuxppc-dev/87y318wp9r.fsf@linux.ibm.com/
> 
> Fixes: 1f9ad21c3b38 ("powerpc/mm: Implement set_memory() routines")
> Reported-by: Laurent Vivier <lvivier@redhat.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>   arch/powerpc/mm/pageattr.c | 45 +++++++++++++++++++++++---------------
>   1 file changed, 27 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
> index 0876216ceee6..72425b61eb7e 100644
> --- a/arch/powerpc/mm/pageattr.c
> +++ b/arch/powerpc/mm/pageattr.c
> @@ -18,52 +18,61 @@
>   /*
>    * Updates the attributes of a page in three steps:
>    *
> - * 1. invalidate the page table entry
> - * 2. flush the TLB
> - * 3. install the new entry with the updated attributes
> - *
> - * Invalidating the pte means there are situations where this will not work
> - * when in theory it should.
> - * For example:
> - * - removing write from page whilst it is being executed
> - * - setting a page read-only whilst it is being read by another CPU
> + * 1. take the page_table_lock
> + * 2. install the new entry with the updated attributes
> + * 3. flush the TLB
>    *
> + * This sequence is safe against concurrent updates, and also allows updating the
> + * attributes of a page currently being executed or accessed.
>    */
>   static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
>   {
>   	long action = (long)data;
> -	pte_t pte;
> +	unsigned long set, clear;
>   
>   	spin_lock(&init_mm.page_table_lock);
>   
> -	/* invalidate the PTE so it's safe to modify */
> -	pte = ptep_get_and_clear(&init_mm, addr, ptep);
> -	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> +	set = clear = 0;
>   
>   	/* modify the PTE bits as desired, then apply */
>   	switch (action) {
>   	case SET_MEMORY_RO:
> -		pte = pte_wrprotect(pte);
> +#ifdef CONFIG_PPC_BOOK3S_64
> +		clear = _PAGE_WRITE;
> +#elif defined(CONFIG_PPC_8xx)
> +		set = _PAGE_RO;
> +#else
> +		clear = _PAGE_RW;
> +#endif

I think it can be handle as follows (untested):

new = pte_wrprotect(pte);

set = pte_val(new) & ~pte_val(pte);
clear = ~pte_val(new) & pte_val(pte);

So just put those two lines before the pte_update() and only change the switch cases to create a 
'new' pte instead of changing it.


Or you can do the way we do in ptep_set_wrprotect() in <asm/nohash/32/pgtable.h>

Or can __ptep_set_access_flags() be used ?

>   		break;
>   	case SET_MEMORY_RW:
> -		pte = pte_mkwrite(pte_mkdirty(pte));
> +#ifdef CONFIG_PPC_8xx
> +		clear = _PAGE_RO;
> +#elif defined(CONFIG_PPC_BOOK3S_64)
> +		set = _PAGE_RW | _PAGE_DIRTY | _PAGE_SOFT_DIRTY;
> +#else
> +		set = _PAGE_RW | _PAGE_DIRTY;
> +#endif
>   		break;
>   	case SET_MEMORY_NX:
> -		pte = pte_exprotect(pte);
> +		clear = _PAGE_EXEC;
>   		break;
>   	case SET_MEMORY_X:
> -		pte = pte_mkexec(pte);
> +		set = _PAGE_EXEC;
>   		break;
>   	default:
>   		WARN_ON_ONCE(1);
>   		break;
>   	}
>   
> -	set_pte_at(&init_mm, addr, ptep, pte);
> +	pte_update(&init_mm, addr, ptep, clear, set, 0);
>   
>   	/* See ptesync comment in radix__set_pte_at() */
>   	if (radix_enabled())
>   		asm volatile("ptesync": : :"memory");
> +
> +	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);

Can we use a page version like flush_tlb_page() in order to avoid a 'tlbia' ? (maybe another page as 
it was already there).

> +
>   	spin_unlock(&init_mm.page_table_lock);
>   
>   	return 0;
> 
> base-commit: cbc06f051c524dcfe52ef0d1f30647828e226d30
> 

^ permalink raw reply

* Re: [PATCH v2 04/12] powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
From: Tom Lendacky @ 2021-08-17 14:11 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-s390, Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh,
	kvm, Joerg Roedel, x86, kexec, linux-kernel, amd-gfx,
	platform-driver-x86, iommu, Andi Kleen, linux-graphics-maintainer,
	dri-devel, linux-fsdevel, Tianyu Lan, linuxppc-dev,
	Paul Mackerras
In-Reply-To: <YRt01F6Mw6sB+hF8@zn.tnic>

On 8/17/21 3:35 AM, Borislav Petkov wrote:
> On Fri, Aug 13, 2021 at 11:59:23AM -0500, Tom Lendacky wrote:
>> Introduce a powerpc version of the prot_guest_has() function. This will
>> be used to replace the powerpc mem_encrypt_active() implementation, so
>> the implementation will initially only support the PATTR_MEM_ENCRYPT
>> attribute.
>>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>  arch/powerpc/include/asm/protected_guest.h | 30 ++++++++++++++++++++++
>>  arch/powerpc/platforms/pseries/Kconfig     |  1 +
>>  2 files changed, 31 insertions(+)
>>  create mode 100644 arch/powerpc/include/asm/protected_guest.h
>>
>> diff --git a/arch/powerpc/include/asm/protected_guest.h b/arch/powerpc/include/asm/protected_guest.h
>> new file mode 100644
>> index 000000000000..ce55c2c7e534
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/protected_guest.h
>> @@ -0,0 +1,30 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Protected Guest (and Host) Capability checks
>> + *
>> + * Copyright (C) 2021 Advanced Micro Devices, Inc.
>> + *
>> + * Author: Tom Lendacky <thomas.lendacky@amd.com>
>> + */
>> +
>> +#ifndef _POWERPC_PROTECTED_GUEST_H
>> +#define _POWERPC_PROTECTED_GUEST_H
>> +
>> +#include <asm/svm.h>
>> +
>> +#ifndef __ASSEMBLY__
> 
> Same thing here. Pls audit the whole set whether those __ASSEMBLY__
> guards are really needed and remove them if not.

Will do.

Thanks,
Tom

> 
> Thx.
> 

^ permalink raw reply

* [PATCH] scsi: ibmvfc: Stop using scsi_cmnd.tag
From: John Garry @ 2021-08-17 13:43 UTC (permalink / raw)
  To: tyreld, mpe, benh, paulus, jejb, martin.petersen
  Cc: sfr, bvanassche, linux-scsi, John Garry, linux-kernel, linux-next,
	hare, linuxppc-dev, hch

Use scsi_cmd_to_rq(scsi_cmnd)->tag in preference to scsi_cmnd.tag.

Signed-off-by: John Garry <john.garry@huawei.com>
---
This patch was missed in a series to remove scsi_cmnd.tag, which caused
a build error on Martin's SCSI staging tree:
https://lore.kernel.org/linux-scsi/yq14kbppa42.fsf@ca-mkp.ca.oracle.com/T/#mb47909f38f35837686734369600051b278d124af

I note that scsi_cmnd.tag is/was an unsigned char, and I could not find
anywhere in the driver which limits scsi_host.can_queue to 255, so using
scsi_cmnd.tag looks odd to me.

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 7fa5e64e38c3..ba7150cb226a 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -1956,7 +1956,7 @@ static int ibmvfc_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmnd)
 	memcpy(iu->cdb, cmnd->cmnd, cmnd->cmd_len);
 
 	if (cmnd->flags & SCMD_TAGGED) {
-		vfc_cmd->task_tag = cpu_to_be64(cmnd->tag);
+		vfc_cmd->task_tag = cpu_to_be64(scsi_cmd_to_rq(cmnd)->tag);
 		iu->pri_task_attr = IBMVFC_SIMPLE_TASK;
 	}
 
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v1 2/4] powerpc/64s/perf: add power_pmu_running to query whether perf is being used
From: Michael Ellerman @ 2021-08-17 13:42 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev
  Cc: Athira Rajeev, Madhavan Srinivasan, Nicholas Piggin
In-Reply-To: <20210816072953.1165964-3-npiggin@gmail.com>

Nicholas Piggin <npiggin@gmail.com> writes:

> Interrupt handling code would like to know whether perf is enabled, to
> know whether it should enable MSR[EE] to improve PMI coverage.
>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/include/asm/hw_irq.h |  2 ++
>  arch/powerpc/perf/core-book3s.c   | 13 +++++++++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index 21cc571ea9c2..2d5c0d3ccbb6 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -306,6 +306,8 @@ static inline bool lazy_irq_pending_nocheck(void)
>  	return __lazy_irq_pending(local_paca->irq_happened);
>  }
>  
> +bool power_pmu_running(void);
> +
>  /*
>   * This is called by asynchronous interrupts to conditionally
>   * re-enable hard interrupts after having cleared the source
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index bb0ee716de91..76114a9afb2b 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2380,6 +2380,19 @@ static void perf_event_interrupt(struct pt_regs *regs)
>  	perf_sample_event_took(sched_clock() - start_clock);
>  }
>  
> +bool power_pmu_running(void)
> +{
> +	struct cpu_hw_events *cpuhw;
> +
> +	/* Could this simply test local_paca->pmcregs_in_use? */

I think it could, except that it's under an ifdef:

#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
	u8 pmcregs_in_use;		/* pseries puts this in lppaca */
#endif

cheers

^ permalink raw reply

* [PATCH] powerpc/mm: Fix set_memory_*() against concurrent accesses
From: Michael Ellerman @ 2021-08-17 13:25 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: lvivier, farosas, jniethe5, npiggin, aneesh.kumar

Laurent reported that STRICT_MODULE_RWX was causing intermittent crashes
on one of his systems:

  kernel tried to execute exec-protected page (c008000004073278) - exploit attempt? (uid: 0)
  BUG: Unable to handle kernel instruction fetch
  Faulting instruction address: 0xc008000004073278
  Oops: Kernel access of bad area, sig: 11 [#1]
  LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA pSeries
  Modules linked in: drm virtio_console fuse drm_panel_orientation_quirks ...
  CPU: 3 PID: 44 Comm: kworker/3:1 Not tainted 5.14.0-rc4+ #12
  Workqueue: events control_work_handler [virtio_console]
  NIP:  c008000004073278 LR: c008000004073278 CTR: c0000000001e9de0
  REGS: c00000002e4ef7e0 TRAP: 0400   Not tainted  (5.14.0-rc4+)
  MSR:  800000004280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24002822 XER: 200400cf
  ...
  NIP fill_queue+0xf0/0x210 [virtio_console]
  LR  fill_queue+0xf0/0x210 [virtio_console]
  Call Trace:
    fill_queue+0xb4/0x210 [virtio_console] (unreliable)
    add_port+0x1a8/0x470 [virtio_console]
    control_work_handler+0xbc/0x1e8 [virtio_console]
    process_one_work+0x290/0x590
    worker_thread+0x88/0x620
    kthread+0x194/0x1a0
    ret_from_kernel_thread+0x5c/0x64

Jordan, Fabiano & Murilo were able to reproduce and identify that the
problem is caused by the call to module_enable_ro() in do_init_module(),
which happens after the module's init function has already been called.

Our current implementation of change_page_attr() is not safe against
concurrent accesses, because it invalidates the PTE before flushing the
TLB and then installing the new PTE. That leaves a window in time where
there is no valid PTE for the page, if another CPU tries to access the
page at that time we see something like the fault above.

We can't simply switch to set_pte_at()/flush TLB, because our hash MMU
code doesn't handle a set_pte_at() of a valid PTE. See [1].

But we do have pte_update(), which replaces the old PTE with the new,
meaning there's no window where the PTE is invalid. And the hash MMU
version hash__pte_update() deals with synchronising the hash page table
correctly.

Because pte_update() takes the set of PTE bits to set and clear we can't
use our existing helpers, eg. pte_wrprotect() etc. and instead have to
open code the set of flags. We will clean that up somehow in a future
commit.

[1]: https://lore.kernel.org/linuxppc-dev/87y318wp9r.fsf@linux.ibm.com/

Fixes: 1f9ad21c3b38 ("powerpc/mm: Implement set_memory() routines")
Reported-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/mm/pageattr.c | 45 +++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
index 0876216ceee6..72425b61eb7e 100644
--- a/arch/powerpc/mm/pageattr.c
+++ b/arch/powerpc/mm/pageattr.c
@@ -18,52 +18,61 @@
 /*
  * Updates the attributes of a page in three steps:
  *
- * 1. invalidate the page table entry
- * 2. flush the TLB
- * 3. install the new entry with the updated attributes
- *
- * Invalidating the pte means there are situations where this will not work
- * when in theory it should.
- * For example:
- * - removing write from page whilst it is being executed
- * - setting a page read-only whilst it is being read by another CPU
+ * 1. take the page_table_lock
+ * 2. install the new entry with the updated attributes
+ * 3. flush the TLB
  *
+ * This sequence is safe against concurrent updates, and also allows updating the
+ * attributes of a page currently being executed or accessed.
  */
 static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
 {
 	long action = (long)data;
-	pte_t pte;
+	unsigned long set, clear;
 
 	spin_lock(&init_mm.page_table_lock);
 
-	/* invalidate the PTE so it's safe to modify */
-	pte = ptep_get_and_clear(&init_mm, addr, ptep);
-	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+	set = clear = 0;
 
 	/* modify the PTE bits as desired, then apply */
 	switch (action) {
 	case SET_MEMORY_RO:
-		pte = pte_wrprotect(pte);
+#ifdef CONFIG_PPC_BOOK3S_64
+		clear = _PAGE_WRITE;
+#elif defined(CONFIG_PPC_8xx)
+		set = _PAGE_RO;
+#else
+		clear = _PAGE_RW;
+#endif
 		break;
 	case SET_MEMORY_RW:
-		pte = pte_mkwrite(pte_mkdirty(pte));
+#ifdef CONFIG_PPC_8xx
+		clear = _PAGE_RO;
+#elif defined(CONFIG_PPC_BOOK3S_64)
+		set = _PAGE_RW | _PAGE_DIRTY | _PAGE_SOFT_DIRTY;
+#else
+		set = _PAGE_RW | _PAGE_DIRTY;
+#endif
 		break;
 	case SET_MEMORY_NX:
-		pte = pte_exprotect(pte);
+		clear = _PAGE_EXEC;
 		break;
 	case SET_MEMORY_X:
-		pte = pte_mkexec(pte);
+		set = _PAGE_EXEC;
 		break;
 	default:
 		WARN_ON_ONCE(1);
 		break;
 	}
 
-	set_pte_at(&init_mm, addr, ptep, pte);
+	pte_update(&init_mm, addr, ptep, clear, set, 0);
 
 	/* See ptesync comment in radix__set_pte_at() */
 	if (radix_enabled())
 		asm volatile("ptesync": : :"memory");
+
+	flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+
 	spin_unlock(&init_mm.page_table_lock);
 
 	return 0;

base-commit: cbc06f051c524dcfe52ef0d1f30647828e226d30
-- 
2.25.1


^ permalink raw reply related

* [PATCH v7 2/2] virtio-console: remove unnecessary kmemdup()
From: Xianting Tian @ 2021-08-17 13:23 UTC (permalink / raw)
  To: gregkh, jirislaby, amit, arnd, osandov
  Cc: Xianting Tian, linuxppc-dev, linux-kernel, virtualization
In-Reply-To: <20210817132300.165014-1-xianting.tian@linux.alibaba.com>

hvc framework will never pass stack memory to the put_chars() function,
So the calling of kmemdup() is unnecessary, we can remove it.

This revert commit c4baad5029 ("virtio-console: avoid DMA from stack")

Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com>
---
 drivers/char/virtio_console.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 7eaf303a7..4ed3ffb1d 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1117,8 +1117,6 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 {
 	struct port *port;
 	struct scatterlist sg[1];
-	void *data;
-	int ret;
 
 	if (unlikely(early_put_chars))
 		return early_put_chars(vtermno, buf, count);
@@ -1127,14 +1125,8 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 	if (!port)
 		return -EPIPE;
 
-	data = kmemdup(buf, count, GFP_ATOMIC);
-	if (!data)
-		return -ENOMEM;
-
-	sg_init_one(sg, data, count);
-	ret = __send_to_port(port, sg, 1, count, data, false);
-	kfree(data);
-	return ret;
+	sg_init_one(sg, buf, count);
+	return __send_to_port(port, sg, 1, count, (void *)buf, false);
 }
 
 /*
-- 
2.17.1


^ permalink raw reply related

* [PATCH v7 1/2] tty: hvc: pass DMA capable memory to put_chars()
From: Xianting Tian @ 2021-08-17 13:22 UTC (permalink / raw)
  To: gregkh, jirislaby, amit, arnd, osandov
  Cc: Xianting Tian, linuxppc-dev, linux-kernel, virtualization
In-Reply-To: <20210817132300.165014-1-xianting.tian@linux.alibaba.com>

As well known, hvc backend can register its opertions to hvc backend.
the opertions contain put_chars(), get_chars() and so on.

Some hvc backend may do dma in its opertions. eg, put_chars() of
virtio-console. But in the code of hvc framework, it may pass DMA
incapable memory to put_chars() under a specific configuration, which
is explained in commit c4baad5029(virtio-console: avoid DMA from stack):
1, c[] is on stack,
   hvc_console_print():
	char c[N_OUTBUF] __ALIGNED__;
	cons_ops[index]->put_chars(vtermnos[index], c, i);
2, ch is on stack,
   static void hvc_poll_put_char(,,char ch)
   {
	struct tty_struct *tty = driver->ttys[0];
	struct hvc_struct *hp = tty->driver_data;
	int n;

	do {
		n = hp->ops->put_chars(hp->vtermno, &ch, 1);
	} while (n <= 0);
   }

Commit c4baad5029 is just the fix to avoid DMA from stack memory, which
is passed to virtio-console by hvc framework in above code. But I think
the fix is aggressive, it directly uses kmemdup() to alloc new buffer
from kmalloc area and do memcpy no matter the memory is in kmalloc area
or not. But most importantly, it should better be fixed in the hvc
framework, by changing it to never pass stack memory to the put_chars()
function in the first place. Otherwise, we still face the same issue if
a new hvc backend using dma added in the furture.

We make 'char c[N_OUTBUF]' part of 'struct hvc_struct', so hp->c is no
longer the stack memory. we can use it in above two cases.

Other fix is use L1_CACHE_BYTES as the alignment, use 'sizeof(long)' as
dma alignment is wrong. And use struct_size() to calculate size of
hvc_struct.

Introduce another array(cons_hvcs[]) for hvc_struct pointers next to
the cons_ops[] and vtermnos[] arrays. With the array, we can easily find
the hp, instead of traversing hp list.

With the patch, we can remove the fix c4baad5029.

We tested the patch, it worked normally.

Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com>
---
 drivers/tty/hvc/hvc_console.c | 39 +++++++++++++++++++++--------------
 drivers/tty/hvc/hvc_console.h | 16 ++++++++++++--
 2 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 5bb8c4e44..b882ceb5f 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -41,16 +41,6 @@
  */
 #define HVC_CLOSE_WAIT (HZ/100) /* 1/10 of a second */
 
-/*
- * These sizes are most efficient for vio, because they are the
- * native transfer size. We could make them selectable in the
- * future to better deal with backends that want other buffer sizes.
- */
-#define N_OUTBUF	16
-#define N_INBUF		16
-
-#define __ALIGNED__ __attribute__((__aligned__(sizeof(long))))
-
 static struct tty_driver *hvc_driver;
 static struct task_struct *hvc_task;
 
@@ -142,6 +132,7 @@ static int hvc_flush(struct hvc_struct *hp)
 static const struct hv_ops *cons_ops[MAX_NR_HVC_CONSOLES];
 static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
 	{[0 ... MAX_NR_HVC_CONSOLES - 1] = -1};
+static struct hvc_struct *cons_hvcs[MAX_NR_HVC_CONSOLES];
 
 /*
  * Console APIs, NOT TTY.  These APIs are available immediately when
@@ -151,9 +142,11 @@ static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
 static void hvc_console_print(struct console *co, const char *b,
 			      unsigned count)
 {
-	char c[N_OUTBUF] __ALIGNED__;
+	char *c;
 	unsigned i = 0, n = 0;
 	int r, donecr = 0, index = co->index;
+	unsigned long flags;
+	struct hvc_struct *hp;
 
 	/* Console access attempt outside of acceptable console range. */
 	if (index >= MAX_NR_HVC_CONSOLES)
@@ -163,6 +156,13 @@ static void hvc_console_print(struct console *co, const char *b,
 	if (vtermnos[index] == -1)
 		return;
 
+	hp = cons_hvcs[index];
+	if (!hp || !hp->c)
+		return;
+
+	c = hp->c;
+
+	spin_lock_irqsave(&hp->c_lock, flags);
 	while (count > 0 || i > 0) {
 		if (count > 0 && i < sizeof(c)) {
 			if (b[n] == '\n' && !donecr) {
@@ -191,6 +191,7 @@ static void hvc_console_print(struct console *co, const char *b,
 			}
 		}
 	}
+	spin_unlock_irqrestore(&hp->c_lock, flags);
 	hvc_console_flush(cons_ops[index], vtermnos[index]);
 }
 
@@ -878,9 +879,16 @@ static void hvc_poll_put_char(struct tty_driver *driver, int line, char ch)
 	struct tty_struct *tty = driver->ttys[0];
 	struct hvc_struct *hp = tty->driver_data;
 	int n;
+	unsigned long flags;
+
+	if (!hp || !hp->c)
+		return;
 
 	do {
-		n = hp->ops->put_chars(hp->vtermno, &ch, 1);
+		spin_lock_irqsave(&hp->c_lock, flags);
+		hp->c[0] = ch;
+		n = hp->ops->put_chars(hp->vtermno, hp->c, 1);
+		spin_unlock_irqrestore(&hp->c_lock, flags);
 	} while (n <= 0);
 }
 #endif
@@ -922,8 +930,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 			return ERR_PTR(err);
 	}
 
-	hp = kzalloc(ALIGN(sizeof(*hp), sizeof(long)) + outbuf_size,
-			GFP_KERNEL);
+	hp = kzalloc(struct_size(hp, outbuf, outbuf_size), GFP_KERNEL);
 	if (!hp)
 		return ERR_PTR(-ENOMEM);
 
@@ -931,13 +938,13 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 	hp->data = data;
 	hp->ops = ops;
 	hp->outbuf_size = outbuf_size;
-	hp->outbuf = &((char *)hp)[ALIGN(sizeof(*hp), sizeof(long))];
 
 	tty_port_init(&hp->port);
 	hp->port.ops = &hvc_port_ops;
 
 	INIT_WORK(&hp->tty_resize, hvc_set_winsz);
 	spin_lock_init(&hp->lock);
+	spin_lock_init(&hp->c_lock);
 	mutex_lock(&hvc_structs_mutex);
 
 	/*
@@ -964,6 +971,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 	if (i < MAX_NR_HVC_CONSOLES) {
 		cons_ops[i] = ops;
 		vtermnos[i] = vtermno;
+		cons_hvcs[i] = hp;
 	}
 
 	list_add_tail(&(hp->next), &hvc_structs);
@@ -988,6 +996,7 @@ int hvc_remove(struct hvc_struct *hp)
 	if (hp->index < MAX_NR_HVC_CONSOLES) {
 		vtermnos[hp->index] = -1;
 		cons_ops[hp->index] = NULL;
+		cons_hvcs[hp->index] = NULL;
 	}
 
 	/* Don't whack hp->irq because tty_hangup() will need to free the irq. */
diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
index 18d005814..97a5f1e0f 100644
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -32,13 +32,21 @@
  */
 #define HVC_ALLOC_TTY_ADAPTERS	8
 
+/*
+ * These sizes are most efficient for vio, because they are the
+ * native transfer size. We could make them selectable in the
+ * future to better deal with backends that want other buffer sizes.
+ */
+#define N_OUTBUF	16
+#define N_INBUF		16
+
+#define __ALIGNED__ __attribute__((__aligned__(L1_CACHE_BYTES)))
+
 struct hvc_struct {
 	struct tty_port port;
 	spinlock_t lock;
 	int index;
 	int do_wakeup;
-	char *outbuf;
-	int outbuf_size;
 	int n_outbuf;
 	uint32_t vtermno;
 	const struct hv_ops *ops;
@@ -48,6 +56,10 @@ struct hvc_struct {
 	struct work_struct tty_resize;
 	struct list_head next;
 	unsigned long flags;
+	spinlock_t c_lock;
+	char c[N_OUTBUF] __ALIGNED__;
+	int outbuf_size;
+	char outbuf[0] __ALIGNED__;
 };
 
 /* implemented by a low level driver */
-- 
2.17.1


^ permalink raw reply related

* [PATCH v7 0/2] make hvc pass dma capable memory to its backend
From: Xianting Tian @ 2021-08-17 13:22 UTC (permalink / raw)
  To: gregkh, jirislaby, amit, arnd, osandov
  Cc: Xianting Tian, linuxppc-dev, linux-kernel, virtualization

Dear all,

This patch series make hvc framework pass DMA capable memory to
put_chars() of hvc backend(eg, virtio-console), and revert commit
c4baad5029 ("virtio-console: avoid DMA from stack”)

V1
virtio-console: avoid DMA from vmalloc area
https://lkml.org/lkml/2021/7/27/494

For v1 patch, Arnd Bergmann suggests to fix the issue in the first
place:
Make hvc pass DMA capable memory to put_chars()
The fix suggestion is included in v2.

V2
[PATCH 1/2] tty: hvc: pass DMA capable memory to put_chars()
https://lkml.org/lkml/2021/8/1/8
[PATCH 2/2] virtio-console: remove unnecessary kmemdup()
https://lkml.org/lkml/2021/8/1/9

For v2 patch, Arnd Bergmann suggests to make new buf part of the
hvc_struct structure, and fix the compile issue.
The fix suggestion is included in v3.

V3
[PATCH v3 1/2] tty: hvc: pass DMA capable memory to put_chars()
https://lkml.org/lkml/2021/8/3/1347
[PATCH v3 2/2] virtio-console: remove unnecessary kmemdup()
https://lkml.org/lkml/2021/8/3/1348

For v3 patch, Jiri Slaby suggests to make 'char c[N_OUTBUF]' part of
hvc_struct, and make 'hp->outbuf' aligned and use struct_size() to
calculate the size of hvc_struct. The fix suggestion is included in
v4.

V4
[PATCH v4 0/2] make hvc pass dma capable memory to its backend
https://lkml.org/lkml/2021/8/5/1350
[PATCH v4 1/2] tty: hvc: pass DMA capable memory to put_chars()
https://lkml.org/lkml/2021/8/5/1351
[PATCH v4 2/2] virtio-console: remove unnecessary kmemdup()
https://lkml.org/lkml/2021/8/5/1352

For v4 patch, Arnd Bergmann suggests to introduce another
array(cons_outbuf[]) for the buffer pointers next to the cons_ops[]
and vtermnos[] arrays. This fix included in this v5 patch.

V5
Arnd Bergmann suggests to use "L1_CACHE_BYTES" as dma alignment,
use 'sizeof(long)' as dma alignment is wrong. fix it in v6.

V6
It contains coding error, fix it in v7 and it worked normally
according to test result.

drivers/tty/hvc/hvc_console.c | 39 +++++++++++++++++++++--------------
drivers/tty/hvc/hvc_console.h | 16 ++++++++++++--
drivers/tty/hvc/hvc_console.h | 16 ++++++++++++--
3 file changed

^ permalink raw reply

* Re: [PATCH/RFC] powerpc/module_64: allow .init_array constructors to run
From: Christophe Leroy @ 2021-08-17 13:22 UTC (permalink / raw)
  To: Jan Stancek, mpe, benh, paulus, linuxppc-dev; +Cc: linux-kernel
In-Reply-To: <920acea9aa18e4f2956581a8e158bdaa376fdf63.1629203945.git.jstancek@redhat.com>



Le 17/08/2021 à 15:02, Jan Stancek a écrit :
> gcov and kasan rely on compiler generated constructor code.
> For modules, gcc-8 with gcov enabled generates .init_array section,
> but on ppc64le it doesn't get executed. find_module_sections() never
> finds .init_array section, because module_frob_arch_sections() renames
> it to _init_array.
> 
> Avoid renaming .init_array section, so do_mod_ctors() can use it.
> 
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
> I wasn't able to trace the comment:
>    "We don't handle .init for the moment: rename to _init"

Original patch there: https://github.com/mpe/linux-fullhistory/commit/d6ad6690aa72

> to original patch (it pre-dates .git). I'm not sure if it
> still applies today, so I limited patch to .init_array. This
> fixes gcov for modules for me on ppc64le 5.14.0-rc6.
> 
> Renaming issue is also mentioned in kasan patches here:
>    https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20210319144058.772525-1-dja@axtens
> 
>   arch/powerpc/kernel/module_64.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index 6baa676e7cb6..c604b13ea6bf 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -299,8 +299,16 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
>   					  sechdrs[i].sh_size);
>   
>   		/* We don't handle .init for the moment: rename to _init */
> -		while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init")))
> +		while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init"))) {
> +#ifdef CONFIG_CONSTRUCTORS

Please avoid #ifdefery as much as possible.

I think here you can do:

			if (IS_ENABLED(CONFIG_CONSTRUCTORS) &&
			    strstr(secstrings + sechdrs[i].sh_name, ".init_array"))

> +			/* find_module_sections() needs .init_array intact */
> +			if (strstr(secstrings + sechdrs[i].sh_name,
> +				".init_array")) {

No { } for single statements (See 
https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces)

> +				break;
> +			}
> +#endif
>   			p[0] = '_';
> +		}
>   
>   		if (sechdrs[i].sh_type == SHT_SYMTAB)
>   			dedotify((void *)hdr + sechdrs[i].sh_offset,
> 

^ permalink raw reply

* Re: [PATCH v1 2/4] powerpc/64s/perf: add power_pmu_running to query whether perf is being used
From: Madhavan Srinivasan @ 2021-08-17 13:06 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Athira Rajeev
In-Reply-To: <20210816072953.1165964-3-npiggin@gmail.com>


On 8/16/21 12:59 PM, Nicholas Piggin wrote:
> Interrupt handling code would like to know whether perf is enabled, to
> know whether it should enable MSR[EE] to improve PMI coverage.
>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   arch/powerpc/include/asm/hw_irq.h |  2 ++
>   arch/powerpc/perf/core-book3s.c   | 13 +++++++++++++
>   2 files changed, 15 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index 21cc571ea9c2..2d5c0d3ccbb6 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -306,6 +306,8 @@ static inline bool lazy_irq_pending_nocheck(void)
>   	return __lazy_irq_pending(local_paca->irq_happened);
>   }
>   
> +bool power_pmu_running(void);
> +
>   /*
>    * This is called by asynchronous interrupts to conditionally
>    * re-enable hard interrupts after having cleared the source
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index bb0ee716de91..76114a9afb2b 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -2380,6 +2380,19 @@ static void perf_event_interrupt(struct pt_regs *regs)
>   	perf_sample_event_took(sched_clock() - start_clock);
>   }
>   
> +bool power_pmu_running(void)
> +{
> +	struct cpu_hw_events *cpuhw;
> +
> +	/* Could this simply test local_paca->pmcregs_in_use? */
> +
> +	if (!ppmu)
> +		return false;


This covers only when perf platform driver is not registered,
but we should also check for MMCR0[32], since pmu sprs can be
accessed via sysfs.

Maddy


> +
> +	cpuhw = this_cpu_ptr(&cpu_hw_events);
> +	return cpuhw->n_events;
> +}
> +
>   static int power_pmu_prepare_cpu(unsigned int cpu)
>   {
>   	struct cpu_hw_events *cpuhw = &per_cpu(cpu_hw_events, cpu);

^ permalink raw reply

* [PATCH/RFC] powerpc/module_64: allow .init_array constructors to run
From: Jan Stancek @ 2021-08-17 13:02 UTC (permalink / raw)
  To: mpe, benh, paulus, christophe.leroy, linuxppc-dev; +Cc: linux-kernel, jstancek

gcov and kasan rely on compiler generated constructor code.
For modules, gcc-8 with gcov enabled generates .init_array section,
but on ppc64le it doesn't get executed. find_module_sections() never
finds .init_array section, because module_frob_arch_sections() renames
it to _init_array.

Avoid renaming .init_array section, so do_mod_ctors() can use it.

Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
I wasn't able to trace the comment:
  "We don't handle .init for the moment: rename to _init"
to original patch (it pre-dates .git). I'm not sure if it
still applies today, so I limited patch to .init_array. This
fixes gcov for modules for me on ppc64le 5.14.0-rc6.

Renaming issue is also mentioned in kasan patches here:
  https://patchwork.ozlabs.org/project/linuxppc-dev/cover/20210319144058.772525-1-dja@axtens

 arch/powerpc/kernel/module_64.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 6baa676e7cb6..c604b13ea6bf 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -299,8 +299,16 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
 					  sechdrs[i].sh_size);
 
 		/* We don't handle .init for the moment: rename to _init */
-		while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init")))
+		while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init"))) {
+#ifdef CONFIG_CONSTRUCTORS
+			/* find_module_sections() needs .init_array intact */
+			if (strstr(secstrings + sechdrs[i].sh_name,
+				".init_array")) {
+				break;
+			}
+#endif
 			p[0] = '_';
+		}
 
 		if (sechdrs[i].sh_type == SHT_SYMTAB)
 			dedotify((void *)hdr + sechdrs[i].sh_offset,
-- 
2.27.0


^ permalink raw reply related

* [PATCH] powerpc/head_check: Fix shellcheck errors
From: Michael Ellerman @ 2021-08-17 12:51 UTC (permalink / raw)
  To: linuxppc-dev

Replace "cat file | grep pattern" with "grep pattern file", and quote a
few variables. Together that fixes all shellcheck errors.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/tools/head_check.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/tools/head_check.sh b/arch/powerpc/tools/head_check.sh
index e477837fdc58..689907cda996 100644
--- a/arch/powerpc/tools/head_check.sh
+++ b/arch/powerpc/tools/head_check.sh
@@ -49,11 +49,11 @@ vmlinux="$2"
 $nm "$vmlinux" | grep -e " [TA] _stext$" -e " t start_first_256B$" -e " a text_start$" -e " t start_text$" > .tmp_symbols.txt
 
 
-vma=$(cat .tmp_symbols.txt | grep -e " [TA] _stext$" | cut -d' ' -f1)
+vma=$(grep -e " [TA] _stext$" .tmp_symbols.txt | cut -d' ' -f1)
 
-expected_start_head_addr=$vma
+expected_start_head_addr="$vma"
 
-start_head_addr=$(cat .tmp_symbols.txt | grep " t start_first_256B$" | cut -d' ' -f1)
+start_head_addr=$(grep " t start_first_256B$" .tmp_symbols.txt | cut -d' ' -f1)
 
 if [ "$start_head_addr" != "$expected_start_head_addr" ]; then
 	echo "ERROR: head code starts at $start_head_addr, should be $expected_start_head_addr" 1>&2
@@ -63,11 +63,11 @@ if [ "$start_head_addr" != "$expected_start_head_addr" ]; then
 	exit 1
 fi
 
-top_vma=$(echo $vma | cut -d'0' -f1)
+top_vma=$(echo "$vma" | cut -d'0' -f1)
 
-expected_start_text_addr=$(cat .tmp_symbols.txt | grep " a text_start$" | cut -d' ' -f1 | sed "s/^0/$top_vma/")
+expected_start_text_addr=$(grep " a text_start$" .tmp_symbols.txt | cut -d' ' -f1 | sed "s/^0/$top_vma/")
 
-start_text_addr=$(cat .tmp_symbols.txt | grep " t start_text$" | cut -d' ' -f1)
+start_text_addr=$(grep " t start_text$" .tmp_symbols.txt | cut -d' ' -f1)
 
 if [ "$start_text_addr" != "$expected_start_text_addr" ]; then
 	echo "ERROR: start_text address is $start_text_addr, should be $expected_start_text_addr" 1>&2
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH v2 2/2] powerpc/perf: Return regs->nip as instruction pointer value when SIAR is 0
From: Michael Ellerman @ 2021-08-17 12:49 UTC (permalink / raw)
  To: Christophe Leroy, kajoljain, linuxppc-dev
  Cc: Sukadev Bhattiprolu, atrajeev, maddy, rnsastry
In-Reply-To: <3a34c79d-b800-1a11-7a4b-1fb3babb9df1@csgroup.eu>

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 16/08/2021 à 08:44, kajoljain a écrit :
>> On 8/14/21 6:14 PM, Michael Ellerman wrote:
...
>>>
>>> eg.
>>>
>>> 	if (use_siar && siar_valid(regs) && siar)
>>> 		return siar + perf_ip_adjust(regs);
>>> 	else if (use_siar)
>>> 		return 0;		// no valid instruction pointer
>>> 	else
>>> 		return regs->nip;
>>>
>>>
>>> I'm also not sure why we have that return 0 case, I can't think of why
>>> we'd ever want to do that rather than using nip. So maybe we should do
>>> another patch to drop that case.
>> 
>> Yeah make sense. I will remove return 0 case in my next version.
>
> This was added by commit 
> https://github.com/linuxppc/linux/commit/e6878835ac4794f25385522d29c634b7bbb7cca9
>
> Are we sure it was an error to add it and it can be removed ?

I think so.

That commit added siar_valid(), and updated record_and_restart() to only
record if siar_valid() returned true.

  -                        record = 1;
  +                        record = siar_valid(regs);

It then also changed perf_instruction_pointer():

  -        if (use_siar)
  +        if (use_siar && siar_valid(regs))
                   return mfspr(SPRN_SIAR) + perf_ip_adjust(regs);
  +        else if (use_siar)
  +                return 0;                // no valid instruction pointer
           else
                   return regs->nip;


The first change means we would never even call
perf_instruction_pointer() if siar_valid() is false, so we could never
hit the use_siar && !siar_valid() case.

But even so it's always preferable to use regs->nip than 0, even if nip
is somewhat skewed due to interrupts being disabled etc.

cheers

^ permalink raw reply

* Re: [PATCH v2 04/12] powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
From: Michael Ellerman @ 2021-08-17 12:38 UTC (permalink / raw)
  To: Tom Lendacky, linux-kernel, x86, linuxppc-dev, linux-s390, iommu,
	kvm, linux-efi, platform-driver-x86, linux-graphics-maintainer,
	amd-gfx, dri-devel, kexec, linux-fsdevel
  Cc: Sathyanarayanan Kuppuswamy, Andi Kleen, Tianyu Lan, Joerg Roedel,
	Borislav Petkov, Brijesh Singh, Paul Mackerras
In-Reply-To: <000f627ce20c6504dd8d118d85bd69e7717b752f.1628873970.git.thomas.lendacky@amd.com>

Tom Lendacky <thomas.lendacky@amd.com> writes:
> Introduce a powerpc version of the prot_guest_has() function. This will
> be used to replace the powerpc mem_encrypt_active() implementation, so
> the implementation will initially only support the PATTR_MEM_ENCRYPT
> attribute.
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  arch/powerpc/include/asm/protected_guest.h | 30 ++++++++++++++++++++++
>  arch/powerpc/platforms/pseries/Kconfig     |  1 +
>  2 files changed, 31 insertions(+)
>  create mode 100644 arch/powerpc/include/asm/protected_guest.h
>
> diff --git a/arch/powerpc/include/asm/protected_guest.h b/arch/powerpc/include/asm/protected_guest.h
> new file mode 100644
> index 000000000000..ce55c2c7e534
> --- /dev/null
> +++ b/arch/powerpc/include/asm/protected_guest.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Protected Guest (and Host) Capability checks
> + *
> + * Copyright (C) 2021 Advanced Micro Devices, Inc.
> + *
> + * Author: Tom Lendacky <thomas.lendacky@amd.com>
> + */
> +
> +#ifndef _POWERPC_PROTECTED_GUEST_H
> +#define _POWERPC_PROTECTED_GUEST_H

Minor nit, we would usually use _ASM_POWERPC_PROTECTED_GUEST_H

Otherwise looks OK to me.

Acked-by: Michael Ellerman <mpe@ellerman.id.au>

cheers

^ permalink raw reply

* Re: [PATCH] macintosh: no need to initilise statics to 0
From: Christophe Leroy @ 2021-08-17 11:59 UTC (permalink / raw)
  To: Jason Wang, benh; +Cc: yukuai3, linuxppc-dev, linux-kernel
In-Reply-To: <20210817115104.30057-1-wangborong@cdjrlc.com>



Le 17/08/2021 à 13:51, Jason Wang a écrit :
> Global static variables dont need to be initialised to 0. Because
> the compiler will initilise them.

It is not the compiler, it is the Kernel. It is done here:

https://elixir.bootlin.com/linux/v5.14-rc6/source/arch/powerpc/kernel/early_32.c


> 
> Signed-off-by: Jason Wang <wangborong@cdjrlc.com>
> ---
>   drivers/macintosh/mediabay.c | 10 ++---
>   drivers/macintosh/via-pmu.c  | 78 ++++++++++++++++++------------------
>   2 files changed, 44 insertions(+), 44 deletions(-)

Among those 44 changes, only 2 are related to the commit's description.

> 
> diff --git a/drivers/macintosh/mediabay.c b/drivers/macintosh/mediabay.c
> index eab7e83c11c4..e104a95decb5 100644
> --- a/drivers/macintosh/mediabay.c
> +++ b/drivers/macintosh/mediabay.c
> @@ -70,7 +70,7 @@ struct media_bay_info {
>   #define MAX_BAYS	2
>   
>   static struct media_bay_info media_bays[MAX_BAYS];
> -static int media_bay_count = 0;
> +static int media_bay_count;
>   
>   /*
>    * Wait that number of ms between each step in normal polling mode
> @@ -129,7 +129,7 @@ enum {
>   /*
>    * Functions for polling content of media bay
>    */
> -
> +

What is that change ? Unrelated.

>   static u8
>   ohare_mb_content(struct media_bay_info *bay)
>   {
> @@ -336,7 +336,7 @@ static inline void set_mb_power(struct media_bay_info* bay, int onoff)
>   		bay->ops->power(bay, 1);
>   		bay->state = mb_powering_up;
>   		pr_debug("mediabay%d: powering up\n", bay->index);
> -	} else {
> +	} else {

What is that change ? Unrelated.

>   		/* Make sure everything is powered down & disabled */
>   		bay->ops->power(bay, 0);
>   		bay->state = mb_powering_down;
> @@ -577,7 +577,7 @@ static int media_bay_attach(struct macio_dev *mdev,
>   		macio_release_resources(mdev);
>   		return -ENOMEM;
>   	}
> -	
> +

What is that change ? Unrelated.

>   	i = media_bay_count++;
>   	bay = &media_bays[i];
>   	bay->mdev = mdev;
> @@ -745,7 +745,7 @@ static int __init media_bay_init(void)
>   	if (!machine_is(powermac))
>   		return 0;
>   
> -	macio_register_driver(&media_bay_driver);	
> +	macio_register_driver(&media_bay_driver);

What is that change ? Unrelated.

>   
>   	return 0;
>   }
> diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
> index 4bdd4c45e7a7..cba4c2464dbf 100644
> --- a/drivers/macintosh/via-pmu.c
> +++ b/drivers/macintosh/via-pmu.c
> @@ -158,7 +158,7 @@ static struct device_node *vias;
>   static struct device_node *gpio_node;
>   #endif
>   static unsigned char __iomem *gpio_reg;
> -static int gpio_irq = 0;
> +static int gpio_irq;
>   static int gpio_irq_enabled = -1;
>   static volatile int pmu_suspended;
>   static spinlock_t pmu_lock;
> @@ -313,7 +313,7 @@ int __init find_via_pmu(void)
>   			PMU_INT_SNDBRT |
>   			PMU_INT_ADB |
>   			PMU_INT_TICK;
> -	
> +

What is that change ? Unrelated.

>   	if (of_node_name_eq(vias->parent, "ohare") ||
>   	    of_device_is_compatible(vias->parent, "ohare"))
>   		pmu_kind = PMU_OHARE_BASED;
> @@ -336,7 +336,7 @@ int __init find_via_pmu(void)
>   				PMU_INT_ADB |
>   				PMU_INT_TICK |
>   				PMU_INT_ENVIRONMENT;
> -		
> +

What is that change ? Unrelated.

>   		gpiop = of_find_node_by_name(NULL, "gpio");
>   		if (gpiop) {
>   			reg = of_get_property(gpiop, "reg", NULL);
> @@ -358,7 +358,7 @@ int __init find_via_pmu(void)
>   		printk(KERN_ERR "via-pmu: Can't map address !\n");
>   		goto fail_via_remap;
>   	}
> -	
> +

What is that change ? Unrelated.

>   	out_8(&via1[IER], IER_CLR | 0x7f);	/* disable all intrs */
>   	out_8(&via1[IFR], 0x7f);			/* clear IFR */
>   
> @@ -368,7 +368,7 @@ int __init find_via_pmu(void)
>   		goto fail_init;
>   
>   	sys_ctrler = SYS_CTRLER_PMU;
> -	
> +

What is that change ? Unrelated.

>   	return 1;
>   
>    fail_init:
> @@ -623,7 +623,7 @@ init_pmu(void)
>   	pmu_wait_complete(&req);
>   	if (req.reply_len > 0)
>   		pmu_version = req.reply[0];
> -	
> +

What is that change ? Unrelated.

>   	/* Read server mode setting */
>   	if (pmu_kind == PMU_KEYLARGO_BASED) {
>   		pmu_request(&req, NULL, 2, PMU_POWER_EVENTS,
> @@ -664,11 +664,11 @@ static void pmu_set_server_mode(int server_mode)
>   	if (server_mode)
>   		pmu_request(&req, NULL, 4, PMU_POWER_EVENTS,
>   			    PMU_PWR_SET_POWERUP_EVENTS,
> -			    req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
> +			    req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);

What is that change ? Unrelated.

>   	else
>   		pmu_request(&req, NULL, 4, PMU_POWER_EVENTS,
>   			    PMU_PWR_CLR_POWERUP_EVENTS,
> -			    req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
> +			    req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);

What is that change ? Unrelated.

>   	pmu_wait_complete(&req);
>   }
>   
> @@ -684,8 +684,8 @@ done_battery_state_ohare(struct adb_request* req)
>   	 *    0x01 :  AC indicator
>   	 *    0x02 :  charging
>   	 *    0x04 :  battery exist
> -	 *    0x08 :
> -	 *    0x10 :
> +	 *    0x08 :
> +	 *    0x10 :

What is that change ? Unrelated.

>   	 *    0x20 :  full charged
>   	 *    0x40 :  pcharge reset
>   	 *    0x80 :  battery exist
> @@ -708,7 +708,7 @@ done_battery_state_ohare(struct adb_request* req)
>   		pmu_power_flags |= PMU_PWR_AC_PRESENT;
>   	else
>   		pmu_power_flags &= ~PMU_PWR_AC_PRESENT;
> -	
> +

What is that change ? Unrelated.

>   	if (mb == PMAC_TYPE_COMET) {
>   		vmax_charged = 189;
>   		vmax_charging = 213;
> @@ -771,26 +771,26 @@ done_battery_state_smart(struct adb_request* req)
>   	/* format:
>   	 *  [0] : format of this structure (known: 3,4,5)
>   	 *  [1] : flags
> -	 *
> +	 *

What is that change ? Unrelated.

>   	 *  format 3 & 4:
> -	 *
> +	 *

What is that change ? Unrelated.

>   	 *  [2] : charge
>   	 *  [3] : max charge
>   	 *  [4] : current
>   	 *  [5] : voltage
> -	 *
> +	 *

What is that change ? Unrelated.

>   	 *  format 5:
> -	 *
> +	 *

What is that change ? Unrelated.

>   	 *  [2][3] : charge
>   	 *  [4][5] : max charge
>   	 *  [6][7] : current
>   	 *  [8][9] : voltage
>   	 */
> -	
> +

What is that change ? Unrelated.

>   	unsigned int bat_flags = PMU_BATT_TYPE_SMART;
>   	int amperage;
>   	unsigned int capa, max, voltage;
> -	
> +

What is that change ? Unrelated.

>   	if (req->reply[1] & 0x01)
>   		pmu_power_flags |= PMU_PWR_AC_PRESENT;
>   	else
> @@ -798,7 +798,7 @@ done_battery_state_smart(struct adb_request* req)
>   
>   
>   	capa = max = amperage = voltage = 0;
> -	
> +

What is that change ? Unrelated.

>   	if (req->reply[1] & 0x04) {
>   		bat_flags |= PMU_BATT_PRESENT;
>   		switch(req->reply[0]) {
> @@ -897,7 +897,7 @@ static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
>   static int pmu_battery_proc_show(struct seq_file *m, void *v)
>   {
>   	long batnum = (long)m->private;
> -	
> +

What is that change ? Unrelated.

>   	seq_putc(m, '\n');
>   	seq_printf(m, "flags      : %08x\n", pmu_batteries[batnum].flags);
>   	seq_printf(m, "charge     : %d\n", pmu_batteries[batnum].charge);
> @@ -932,7 +932,7 @@ static ssize_t pmu_options_proc_write(struct file *file,
>   	char tmp[33];
>   	char *label, *val;
>   	size_t fcount = count;
> -	
> +

What is that change ? Unrelated.

>   	if (!count)
>   		return -EINVAL;
>   	if (count > 32)
> @@ -1304,7 +1304,7 @@ pmu_suspend(void)
>   
>   	if (pmu_state == uninitialized)
>   		return;
> -	
> +

What is that change ? Unrelated.

>   	spin_lock_irqsave(&pmu_lock, flags);
>   	pmu_suspended++;
>   	if (pmu_suspended > 1) {
> @@ -1430,7 +1430,7 @@ pmu_handle_data(unsigned char *data, int len)
>   			      && data[1] == 0x2c && data[3] == 0xff
>   			      && (data[2] & ~1) == 0xf4))
>   				adb_input(data+1, len-1, 1);
> -#endif /* CONFIG_ADB */		
> +#endif /* CONFIG_ADB */

What is that change ? Unrelated.

>   		}
>   		break;
>   
> @@ -1554,7 +1554,7 @@ pmu_sr_intr(void)
>   			interrupt_data_len[int_data_last] = data_len;
>   		} else {
>   			req = current_req;
> -			/*
> +			/*

What is that change ? Unrelated.

>   			 * For PMU sleep and freq change requests, we lock the
>   			 * PMU until it's explicitly unlocked. This avoids any
>   			 * spurrious event polling getting in
> @@ -1589,7 +1589,7 @@ via_pmu_interrupt(int irq, void *arg)
>   	/* This is a bit brutal, we can probably do better */
>   	spin_lock_irqsave(&pmu_lock, flags);
>   	++disable_poll;
> -	
> +

What is that change ? Unrelated.

>   	for (;;) {
>   		/* On 68k Macs, VIA interrupts are dispatched individually.
>   		 * Unless we are polling, the relevant IRQ flag has already
> @@ -1653,7 +1653,7 @@ via_pmu_interrupt(int irq, void *arg)
>   		} else if (current_req)
>   			pmu_start();
>   	}
> -no_free_slot:			
> +no_free_slot:

What is that change ? Unrelated.

>   	/* Mark the oldest buffer for flushing */
>   	if (int_data_state[!int_data_last] == int_data_ready) {
>   		int_data_state[!int_data_last] = int_data_flush;
> @@ -1670,7 +1670,7 @@ via_pmu_interrupt(int irq, void *arg)
>   		pmu_done(req);
>   		req = NULL;
>   	}
> -		
> +

What is that change ? Unrelated.

>   	/* Deal with interrupt datas outside of the lock */
>   	if (int_data >= 0) {
>   		pmu_handle_data(interrupt_data[int_data], interrupt_data_len[int_data]);
> @@ -1776,7 +1776,7 @@ pmu_restart(void)
>   	local_irq_disable();
>   
>   	drop_interrupts = 1;
> -	
> +

What is that change ? Unrelated.

>   	if (pmu_kind != PMU_KEYLARGO_BASED) {
>   		pmu_request(&req, NULL, 2, PMU_SET_INTR_MASK, PMU_INT_ADB |
>   						PMU_INT_TICK );
> @@ -1830,7 +1830,7 @@ pmu_present(void)
>   /*
>    * Put the powerbook to sleep.
>    */
> -
> +

What is that change ? Unrelated.

>   static u32 save_via[8];
>   static int __fake_sleep;
>   
> @@ -1901,7 +1901,7 @@ static int powerbook_sleep_grackle(void)
>   
>   	pci_read_config_word(grackle, 0x70, &pmcr1);
>   	/* Apparently, MacOS uses NAP mode for Grackle ??? */
> -	pmcr1 &= ~(GRACKLE_DOZE|GRACKLE_SLEEP);
> +	pmcr1 &= ~(GRACKLE_DOZE|GRACKLE_SLEEP);

What is that change ? Unrelated.

>   	pmcr1 |= GRACKLE_PM|GRACKLE_NAP;
>   	pci_write_config_word(grackle, 0x70, pmcr1);
>   
> @@ -1913,7 +1913,7 @@ static int powerbook_sleep_grackle(void)
>   
>   	/* We're awake again, stop grackle PM */
>   	pci_read_config_word(grackle, 0x70, &pmcr1);
> -	pmcr1 &= ~(GRACKLE_PM|GRACKLE_DOZE|GRACKLE_SLEEP|GRACKLE_NAP);
> +	pmcr1 &= ~(GRACKLE_PM|GRACKLE_DOZE|GRACKLE_SLEEP|GRACKLE_NAP);

What is that change ? Unrelated.

>   	pci_write_config_word(grackle, 0x70, pmcr1);
>   
>   	pci_dev_put(grackle);
> @@ -1921,11 +1921,11 @@ static int powerbook_sleep_grackle(void)
>   	/* Make sure the PMU is idle */
>   	pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,0);
>   	restore_via_state();
> -	
> +

What is that change ? Unrelated.

>   	/* Restore L2 cache */
>   	if (save_l2cr != 0xffffffff && (save_l2cr & L2CR_L2E) != 0)
>    		_set_L2CR(save_l2cr);
> -	
> +

What is that change ? Unrelated.

>   	/* Restore userland MMU context */
>   	switch_mmu_context(NULL, current->active_mm, NULL);
>   
> @@ -1949,7 +1949,7 @@ powerbook_sleep_Core99(void)
>   	unsigned long save_l2cr;
>   	unsigned long save_l3cr;
>   	struct adb_request req;
> -	
> +

What is that change ? Unrelated.

>   	if (pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,-1) < 0) {
>   		printk(KERN_ERR "Sleep mode not supported on this machine\n");
>   		return -ENOSYS;
> @@ -2014,7 +2014,7 @@ powerbook_sleep_Core99(void)
>   	/* Restore L3 cache */
>   	if (save_l3cr != 0xffffffff && (save_l3cr & L3CR_L3E) != 0)
>    		_set_L3CR(save_l3cr);
> -	
> +

What is that change ? Unrelated.

>   	/* Restore userland MMU context */
>   	switch_mmu_context(NULL, current->active_mm, NULL);
>   
> @@ -2173,7 +2173,7 @@ pmu_open(struct inode *inode, struct file *file)
>   	return 0;
>   }
>   
> -static ssize_t
> +static ssize_t

What is that change ? Unrelated.

>   pmu_read(struct file *file, char __user *buf,
>   			size_t count, loff_t *ppos)
>   {
> @@ -2219,7 +2219,7 @@ pmu_read(struct file *file, char __user *buf,
>   	__set_current_state(TASK_RUNNING);
>   	remove_wait_queue(&pp->wait, &wait);
>   	spin_unlock_irqrestore(&pp->lock, flags);
> -	
> +

What is that change ? Unrelated.

>   	return ret;
>   }
>   
> @@ -2236,7 +2236,7 @@ pmu_fpoll(struct file *filp, poll_table *wait)
>   	struct pmu_private *pp = filp->private_data;
>   	__poll_t mask = 0;
>   	unsigned long flags;
> -	
> +

What is that change ? Unrelated.

>   	if (!pp)
>   		return 0;
>   	poll_wait(filp, &pp->wait, wait);
> @@ -2500,7 +2500,7 @@ device_initcall(pmu_device_init);
>   
>   
>   #ifdef DEBUG_SLEEP
> -static inline void
> +static inline void

What is that change ? Unrelated.

>   polled_handshake(void)
>   {
>   	via2[B] &= ~TREQ; eieio();
> @@ -2511,7 +2511,7 @@ polled_handshake(void)
>   		;
>   }
>   
> -static inline void
> +static inline void

What is that change ? Unrelated.

>   polled_send_byte(int x)
>   {
>   	via1[ACR] |= SR_OUT | SR_EXT; eieio();
> 

^ permalink raw reply

* [PATCH] macintosh: no need to initilise statics to 0
From: Jason Wang @ 2021-08-17 11:51 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev, linux-kernel, Jason Wang, yukuai3

Global static variables dont need to be initialised to 0. Because
the compiler will initilise them.

Signed-off-by: Jason Wang <wangborong@cdjrlc.com>
---
 drivers/macintosh/mediabay.c | 10 ++---
 drivers/macintosh/via-pmu.c  | 78 ++++++++++++++++++------------------
 2 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/drivers/macintosh/mediabay.c b/drivers/macintosh/mediabay.c
index eab7e83c11c4..e104a95decb5 100644
--- a/drivers/macintosh/mediabay.c
+++ b/drivers/macintosh/mediabay.c
@@ -70,7 +70,7 @@ struct media_bay_info {
 #define MAX_BAYS	2
 
 static struct media_bay_info media_bays[MAX_BAYS];
-static int media_bay_count = 0;
+static int media_bay_count;
 
 /*
  * Wait that number of ms between each step in normal polling mode
@@ -129,7 +129,7 @@ enum {
 /*
  * Functions for polling content of media bay
  */
- 
+
 static u8
 ohare_mb_content(struct media_bay_info *bay)
 {
@@ -336,7 +336,7 @@ static inline void set_mb_power(struct media_bay_info* bay, int onoff)
 		bay->ops->power(bay, 1);
 		bay->state = mb_powering_up;
 		pr_debug("mediabay%d: powering up\n", bay->index);
-	} else { 
+	} else {
 		/* Make sure everything is powered down & disabled */
 		bay->ops->power(bay, 0);
 		bay->state = mb_powering_down;
@@ -577,7 +577,7 @@ static int media_bay_attach(struct macio_dev *mdev,
 		macio_release_resources(mdev);
 		return -ENOMEM;
 	}
-	
+
 	i = media_bay_count++;
 	bay = &media_bays[i];
 	bay->mdev = mdev;
@@ -745,7 +745,7 @@ static int __init media_bay_init(void)
 	if (!machine_is(powermac))
 		return 0;
 
-	macio_register_driver(&media_bay_driver);	
+	macio_register_driver(&media_bay_driver);
 
 	return 0;
 }
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 4bdd4c45e7a7..cba4c2464dbf 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -158,7 +158,7 @@ static struct device_node *vias;
 static struct device_node *gpio_node;
 #endif
 static unsigned char __iomem *gpio_reg;
-static int gpio_irq = 0;
+static int gpio_irq;
 static int gpio_irq_enabled = -1;
 static volatile int pmu_suspended;
 static spinlock_t pmu_lock;
@@ -313,7 +313,7 @@ int __init find_via_pmu(void)
 			PMU_INT_SNDBRT |
 			PMU_INT_ADB |
 			PMU_INT_TICK;
-	
+
 	if (of_node_name_eq(vias->parent, "ohare") ||
 	    of_device_is_compatible(vias->parent, "ohare"))
 		pmu_kind = PMU_OHARE_BASED;
@@ -336,7 +336,7 @@ int __init find_via_pmu(void)
 				PMU_INT_ADB |
 				PMU_INT_TICK |
 				PMU_INT_ENVIRONMENT;
-		
+
 		gpiop = of_find_node_by_name(NULL, "gpio");
 		if (gpiop) {
 			reg = of_get_property(gpiop, "reg", NULL);
@@ -358,7 +358,7 @@ int __init find_via_pmu(void)
 		printk(KERN_ERR "via-pmu: Can't map address !\n");
 		goto fail_via_remap;
 	}
-	
+
 	out_8(&via1[IER], IER_CLR | 0x7f);	/* disable all intrs */
 	out_8(&via1[IFR], 0x7f);			/* clear IFR */
 
@@ -368,7 +368,7 @@ int __init find_via_pmu(void)
 		goto fail_init;
 
 	sys_ctrler = SYS_CTRLER_PMU;
-	
+
 	return 1;
 
  fail_init:
@@ -623,7 +623,7 @@ init_pmu(void)
 	pmu_wait_complete(&req);
 	if (req.reply_len > 0)
 		pmu_version = req.reply[0];
-	
+
 	/* Read server mode setting */
 	if (pmu_kind == PMU_KEYLARGO_BASED) {
 		pmu_request(&req, NULL, 2, PMU_POWER_EVENTS,
@@ -664,11 +664,11 @@ static void pmu_set_server_mode(int server_mode)
 	if (server_mode)
 		pmu_request(&req, NULL, 4, PMU_POWER_EVENTS,
 			    PMU_PWR_SET_POWERUP_EVENTS,
-			    req.reply[0], PMU_PWR_WAKEUP_AC_INSERT); 
+			    req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
 	else
 		pmu_request(&req, NULL, 4, PMU_POWER_EVENTS,
 			    PMU_PWR_CLR_POWERUP_EVENTS,
-			    req.reply[0], PMU_PWR_WAKEUP_AC_INSERT); 
+			    req.reply[0], PMU_PWR_WAKEUP_AC_INSERT);
 	pmu_wait_complete(&req);
 }
 
@@ -684,8 +684,8 @@ done_battery_state_ohare(struct adb_request* req)
 	 *    0x01 :  AC indicator
 	 *    0x02 :  charging
 	 *    0x04 :  battery exist
-	 *    0x08 :  
-	 *    0x10 :  
+	 *    0x08 :
+	 *    0x10 :
 	 *    0x20 :  full charged
 	 *    0x40 :  pcharge reset
 	 *    0x80 :  battery exist
@@ -708,7 +708,7 @@ done_battery_state_ohare(struct adb_request* req)
 		pmu_power_flags |= PMU_PWR_AC_PRESENT;
 	else
 		pmu_power_flags &= ~PMU_PWR_AC_PRESENT;
-	
+
 	if (mb == PMAC_TYPE_COMET) {
 		vmax_charged = 189;
 		vmax_charging = 213;
@@ -771,26 +771,26 @@ done_battery_state_smart(struct adb_request* req)
 	/* format:
 	 *  [0] : format of this structure (known: 3,4,5)
 	 *  [1] : flags
-	 *  
+	 *
 	 *  format 3 & 4:
-	 *  
+	 *
 	 *  [2] : charge
 	 *  [3] : max charge
 	 *  [4] : current
 	 *  [5] : voltage
-	 *  
+	 *
 	 *  format 5:
-	 *  
+	 *
 	 *  [2][3] : charge
 	 *  [4][5] : max charge
 	 *  [6][7] : current
 	 *  [8][9] : voltage
 	 */
-	 
+
 	unsigned int bat_flags = PMU_BATT_TYPE_SMART;
 	int amperage;
 	unsigned int capa, max, voltage;
-	
+
 	if (req->reply[1] & 0x01)
 		pmu_power_flags |= PMU_PWR_AC_PRESENT;
 	else
@@ -798,7 +798,7 @@ done_battery_state_smart(struct adb_request* req)
 
 
 	capa = max = amperage = voltage = 0;
-	
+
 	if (req->reply[1] & 0x04) {
 		bat_flags |= PMU_BATT_PRESENT;
 		switch(req->reply[0]) {
@@ -897,7 +897,7 @@ static int pmu_irqstats_proc_show(struct seq_file *m, void *v)
 static int pmu_battery_proc_show(struct seq_file *m, void *v)
 {
 	long batnum = (long)m->private;
-	
+
 	seq_putc(m, '\n');
 	seq_printf(m, "flags      : %08x\n", pmu_batteries[batnum].flags);
 	seq_printf(m, "charge     : %d\n", pmu_batteries[batnum].charge);
@@ -932,7 +932,7 @@ static ssize_t pmu_options_proc_write(struct file *file,
 	char tmp[33];
 	char *label, *val;
 	size_t fcount = count;
-	
+
 	if (!count)
 		return -EINVAL;
 	if (count > 32)
@@ -1304,7 +1304,7 @@ pmu_suspend(void)
 
 	if (pmu_state == uninitialized)
 		return;
-	
+
 	spin_lock_irqsave(&pmu_lock, flags);
 	pmu_suspended++;
 	if (pmu_suspended > 1) {
@@ -1430,7 +1430,7 @@ pmu_handle_data(unsigned char *data, int len)
 			      && data[1] == 0x2c && data[3] == 0xff
 			      && (data[2] & ~1) == 0xf4))
 				adb_input(data+1, len-1, 1);
-#endif /* CONFIG_ADB */		
+#endif /* CONFIG_ADB */
 		}
 		break;
 
@@ -1554,7 +1554,7 @@ pmu_sr_intr(void)
 			interrupt_data_len[int_data_last] = data_len;
 		} else {
 			req = current_req;
-			/* 
+			/*
 			 * For PMU sleep and freq change requests, we lock the
 			 * PMU until it's explicitly unlocked. This avoids any
 			 * spurrious event polling getting in
@@ -1589,7 +1589,7 @@ via_pmu_interrupt(int irq, void *arg)
 	/* This is a bit brutal, we can probably do better */
 	spin_lock_irqsave(&pmu_lock, flags);
 	++disable_poll;
-	
+
 	for (;;) {
 		/* On 68k Macs, VIA interrupts are dispatched individually.
 		 * Unless we are polling, the relevant IRQ flag has already
@@ -1653,7 +1653,7 @@ via_pmu_interrupt(int irq, void *arg)
 		} else if (current_req)
 			pmu_start();
 	}
-no_free_slot:			
+no_free_slot:
 	/* Mark the oldest buffer for flushing */
 	if (int_data_state[!int_data_last] == int_data_ready) {
 		int_data_state[!int_data_last] = int_data_flush;
@@ -1670,7 +1670,7 @@ via_pmu_interrupt(int irq, void *arg)
 		pmu_done(req);
 		req = NULL;
 	}
-		
+
 	/* Deal with interrupt datas outside of the lock */
 	if (int_data >= 0) {
 		pmu_handle_data(interrupt_data[int_data], interrupt_data_len[int_data]);
@@ -1776,7 +1776,7 @@ pmu_restart(void)
 	local_irq_disable();
 
 	drop_interrupts = 1;
-	
+
 	if (pmu_kind != PMU_KEYLARGO_BASED) {
 		pmu_request(&req, NULL, 2, PMU_SET_INTR_MASK, PMU_INT_ADB |
 						PMU_INT_TICK );
@@ -1830,7 +1830,7 @@ pmu_present(void)
 /*
  * Put the powerbook to sleep.
  */
- 
+
 static u32 save_via[8];
 static int __fake_sleep;
 
@@ -1901,7 +1901,7 @@ static int powerbook_sleep_grackle(void)
 
 	pci_read_config_word(grackle, 0x70, &pmcr1);
 	/* Apparently, MacOS uses NAP mode for Grackle ??? */
-	pmcr1 &= ~(GRACKLE_DOZE|GRACKLE_SLEEP); 
+	pmcr1 &= ~(GRACKLE_DOZE|GRACKLE_SLEEP);
 	pmcr1 |= GRACKLE_PM|GRACKLE_NAP;
 	pci_write_config_word(grackle, 0x70, pmcr1);
 
@@ -1913,7 +1913,7 @@ static int powerbook_sleep_grackle(void)
 
 	/* We're awake again, stop grackle PM */
 	pci_read_config_word(grackle, 0x70, &pmcr1);
-	pmcr1 &= ~(GRACKLE_PM|GRACKLE_DOZE|GRACKLE_SLEEP|GRACKLE_NAP); 
+	pmcr1 &= ~(GRACKLE_PM|GRACKLE_DOZE|GRACKLE_SLEEP|GRACKLE_NAP);
 	pci_write_config_word(grackle, 0x70, pmcr1);
 
 	pci_dev_put(grackle);
@@ -1921,11 +1921,11 @@ static int powerbook_sleep_grackle(void)
 	/* Make sure the PMU is idle */
 	pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,0);
 	restore_via_state();
-	
+
 	/* Restore L2 cache */
 	if (save_l2cr != 0xffffffff && (save_l2cr & L2CR_L2E) != 0)
  		_set_L2CR(save_l2cr);
-	
+
 	/* Restore userland MMU context */
 	switch_mmu_context(NULL, current->active_mm, NULL);
 
@@ -1949,7 +1949,7 @@ powerbook_sleep_Core99(void)
 	unsigned long save_l2cr;
 	unsigned long save_l3cr;
 	struct adb_request req;
-	
+
 	if (pmac_call_feature(PMAC_FTR_SLEEP_STATE,NULL,0,-1) < 0) {
 		printk(KERN_ERR "Sleep mode not supported on this machine\n");
 		return -ENOSYS;
@@ -2014,7 +2014,7 @@ powerbook_sleep_Core99(void)
 	/* Restore L3 cache */
 	if (save_l3cr != 0xffffffff && (save_l3cr & L3CR_L3E) != 0)
  		_set_L3CR(save_l3cr);
-	
+
 	/* Restore userland MMU context */
 	switch_mmu_context(NULL, current->active_mm, NULL);
 
@@ -2173,7 +2173,7 @@ pmu_open(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static ssize_t 
+static ssize_t
 pmu_read(struct file *file, char __user *buf,
 			size_t count, loff_t *ppos)
 {
@@ -2219,7 +2219,7 @@ pmu_read(struct file *file, char __user *buf,
 	__set_current_state(TASK_RUNNING);
 	remove_wait_queue(&pp->wait, &wait);
 	spin_unlock_irqrestore(&pp->lock, flags);
-	
+
 	return ret;
 }
 
@@ -2236,7 +2236,7 @@ pmu_fpoll(struct file *filp, poll_table *wait)
 	struct pmu_private *pp = filp->private_data;
 	__poll_t mask = 0;
 	unsigned long flags;
-	
+
 	if (!pp)
 		return 0;
 	poll_wait(filp, &pp->wait, wait);
@@ -2500,7 +2500,7 @@ device_initcall(pmu_device_init);
 
 
 #ifdef DEBUG_SLEEP
-static inline void 
+static inline void
 polled_handshake(void)
 {
 	via2[B] &= ~TREQ; eieio();
@@ -2511,7 +2511,7 @@ polled_handshake(void)
 		;
 }
 
-static inline void 
+static inline void
 polled_send_byte(int x)
 {
 	via1[ACR] |= SR_OUT | SR_EXT; eieio();
-- 
2.32.0


^ permalink raw reply related

* Re: [PATCH v2 09/12] mm: Remove the now unused mem_encrypt_active() function
From: Borislav Petkov @ 2021-08-17 10:24 UTC (permalink / raw)
  To: Tom Lendacky
  Cc: linux-s390, Sathyanarayanan Kuppuswamy, linux-efi, Brijesh Singh,
	kvm, Joerg Roedel, x86, kexec, linux-kernel, amd-gfx,
	platform-driver-x86, iommu, Andi Kleen, linux-graphics-maintainer,
	dri-devel, Joerg Roedel, linux-fsdevel, Tianyu Lan, linuxppc-dev
In-Reply-To: <YRuN6QhdIQtlluUh@zn.tnic>

On Tue, Aug 17, 2021 at 12:22:33PM +0200, Borislav Petkov wrote:
> This one wants to be part of the previous patch.

... and the three following patches too - the treewide patch does a
single atomic :) replacement and that's it.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ 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