[PATCH V5 00/14] powerpc/vas: Page fault handling for user space NX requests

[Haren Myneni]

On power9, Virtual Accelerator Switchboard (VAS) allows user space or
kernel to communicate with Nest Accelerator (NX) directly using COPY/PASTE
instructions. NX provides verious functionalities such as compression,
encryption and etc. But only compression (842 and GZIP formats) is
supported in Linux kernel on power9.

842 compression driver (drivers/crypto/nx/nx-842-powernv.c)
is already included in Linux. Only GZIP support will be available from
user space.

Applications can issue GZIP compression / decompression requests to NX with
COPY/PASTE instructions. When NX is processing these requests, can hit
fault on the request buffer (not in memory). It issues an interrupt and
pastes fault CRB in fault FIFO. Expects kernel to handle this fault and
return credits for both send and fault windows after processing.

This patch series adds IRQ and fault window setup, and NX fault handling:
- Alloc IRQ and trigger port address, and configure IRQ per VAS instance.
- Set port# for each window to generate an interrupt when noticed fault.
- Set fault window and FIFO on which NX paste fault CRB.
- Setup IRQ thread fault handler per VAS instance.
- When receiving an interrupt, Read CRBs from fault FIFO and update
  coprocessor_status_block (CSB) in the corresponding CRB with translation
  failure (CSB_CC_TRANSLATION). After issuing NX requests, process polls
  on CSB address. When it sees translation error, can touch the request
  buffer to bring the page in to memory and reissue NX request.
- If copy_to_user fails on user space CSB address, OS sends SEGV signal.

Tested these patches with NX-GZIP support and will be posting this series
soon.

Patches 1 & 2: Define alloc IRQ and get port address per chip which are needed
   to alloc IRQ per VAS instance.
Patch 3: Define nx_fault_stamp on which NX writes fault status for the fault
 CRB
Patch 4: Alloc and setup IRQ and trigger port address for each VAS instance
Patch 5: Setup fault window per each VAS instance. This window is used for
 NX to paste fault CRB in FIFO.
Patches 6 & 7: Setup threaded IRQ per VAS and register NX with fault window
 ID and port number for each send window so that NX paste fault CRB
 in this window.
Patch 8: Reference to pid and mm so that pid is not used until window closed.
 Needed for multi thread application where child can open a window
 and can be used by parent later.
Patches 9 and 10: Process CRBs from fault FIFO and notify tasks by
 updating CSB or through signals.
Patches 11 and 12: Return credits for send and fault windows after handling
faults.
Patch 14:Fix closing send window after all credits are returned. This issue
 happens only for user space requests. No page faults on kernel
 request buffer.

Changelog:
V2:
  - Use threaded IRQ instead of own kernel thread handler
  - Use pswid insted of user space CSB address to find valid CRB
  - Removed unused macros and other changes as suggested by Christoph Hellwig

V3:
  - Rebased to 5.5-rc2
  - Use struct pid * instead of pid_t for vas_window tgid
  - Code cleanup as suggested by Christoph Hellwig

V4:
  - Define xive alloc and get IRQ info based on chip ID and use these
functions for IRQ setup per VAS instance. It eliminates skiboot
dependency as suggested by Oliver.

V5:
  - Do not update CSB if the process is exiting (patch9)

Haren Myneni (14):
  powerpc/xive: Define xive_native_alloc_irq_on_chip()
  powerpc/xive: Define xive_native_alloc_get_irq_info()
  powerpc/vas: Define nx_fault_stamp in coprocessor_request_block
  powerpc/vas: Alloc and setup IRQ and trigger port address
  powerpc/vas: Setup fault window per VAS instance
  powerpc/vas: Setup thread IRQ handler per VAS instance
  powerpc/vas: Register NX with fault window ID and IRQ port value
  powerpc/vas: Take reference to PID and mm for user space windows
  powerpc/vas: Update CSB and notify process for fault CRBs
  powerpc/vas: Print CRB and FIFO values
  powerpc/vas: Do not use default credits for receive window
  powerpc/VAS: Return credits after handling fault
  powerpc/vas: Display process stuck message
  powerpc/vas: Free send window in VAS instance after credits returned

 arch/powerpc/include/asm/icswx.h|  18 +-
 arch/powerpc/include/asm/xive.h |  11 +-
 arch/powerpc/platforms/powernv/Makefile |   2 +-
 arch/powerpc/platforms/powernv/ocxl.c   |  20 +-
 arch/powerpc/platforms/powernv/vas-debug.c  |   2 +-
 arch/powerpc/platforms/powernv/vas-fault.c  | 325 
 arch/powerpc/platforms/powernv/vas-window.c | 184 ++--
 arch/powerpc/platforms/powernv/vas.c|  73 ++-
 arch/powerpc/platforms/powernv/vas.h|  38 +++-
 arch/powerpc/sysdev/xive/native.c   |  29 ++-
 10 files changed, 655 insertions(+), 47 deletions(-)
 create mode 100644 arch/powerpc/platforms/powernv/vas-fault.c

-- 
1.8.3.1

Re: [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running

[Heiner Kallweit]

On 22.01.2020 05:04, Florian Fainelli wrote:
> 
> 
> On 1/21/2020 1:09 PM, Heiner Kallweit wrote:
>> Convert suitable drivers to use new helper phy_do_ioctl_running.
>>
>> Signed-off-by: Heiner Kallweit 
> The vast majority of drivers that you are converting use the following
> convention:
> 
> - !netif_running -> return -EINVAL
> - !dev->phydev -> return -ENODEV
> 
> so it may make sense to change the helper to accommodate the majority
> here, not that I believe this is going to make much practical
> difference, but if there were test cases that were specifically looking
> for such an error code, they could be failing after this changeset.
> 
Right, I also stumbled across the different error codes, mainly as you
say -EINVAL. However there is no "wrong value", if netdev isn't running,
then typically the PHY is not attached, and from a netdev point of view
it's not there. So ENODEV seems to be best suited.
In kernel code the changed return code doesn't make a difference,
but yes, in theory there could be userspace programs checking for
-EINVAL. However such userspace programs should check for ENODEV too
anyway to cover the second check that already returns -ENODEV.

> For bgmac.c, bcmgenet.c and cpmac.c:
> 
> Acked-by: Florian Fainelli 
> 
> Whether you decide to spin another version or not.
> 
Heiner

Re: GCC bug ? Re: [PATCH v2 10/10] powerpc/32s: Implement Kernel Userspace Access Protection

[Christophe Leroy]

Le 21/01/2020 à 20:55, Segher Boessenkool a écrit :

On Tue, Jan 21, 2020 at 05:22:32PM +, Christophe Leroy wrote:

g1() should return 3, not 5.


What makes you say that?

"A return of 0 does not indicate that the
  value is _not_ a constant, but merely that GCC cannot prove it is a
  constant with the specified value of the '-O' option."



GCC doc also says:

"if you use it in an inlined function and pass an argument of the 
function as the argument to the built-in, GCC never returns 1 when you 
call the inline function with a string constant"


Does GCC considers (void*)0 as a string constant ?

Christophe

Re: [PATCH FIX] KVM: PPC: Book3S HV: Release lock on page-out failure path

[Kamalesh Babulal]

On 1/22/20 10:25 AM, Bharata B Rao wrote:
> When migrate_vma_setup() fails in kvmppc_svm_page_out(),
> release kvm->arch.uvmem_lock before returning.
> 
> Fixes: ca9f4942670 ("KVM: PPC: Book3S HV: Support for running secure guests")
> Signed-off-by: Bharata B Rao 

Reviewed-by: Kamalesh Babulal 

-- 
Kamalesh

Re: GCC bug ? Re: [PATCH v2 10/10] powerpc/32s: Implement Kernel Userspace Access Protection

[Christophe Leroy]

Le 21/01/2020 à 20:55, Segher Boessenkool a écrit :

On Tue, Jan 21, 2020 at 05:22:32PM +, Christophe Leroy wrote:

g1() should return 3, not 5.


What makes you say that?


What makes me say that is that NULL is obviously a constant pointer and 
I think we are all expecting gcc to see it as a constant during kernel 
build, ie at -O2




"A return of 0 does not indicate that the
  value is _not_ a constant, but merely that GCC cannot prove it is a
  constant with the specified value of the '-O' option."

(And the rules it uses for this are *not* the same as C "constant
expressions" or C "integer constant expression" or C "arithmetic
constant expression" or anything like that -- which should be already
obvious from that it changes with different -Ox).

You can use builtin_constant_p to have the compiler do something better
if the compiler feels like it, but not anything more.  Often people
want stronger guarantees, but when they see how much less often it then
returns "true", they do not want that either.



in asm/book3s/64/kup-radix.h we have:

static inline void allow_user_access(void __user *to, const void __user 
*from,

 unsigned long size)
{
// This is written so we can resolve to a single case at build time
if (__builtin_constant_p(to) && to == NULL)
set_kuap(AMR_KUAP_BLOCK_WRITE);
else if (__builtin_constant_p(from) && from == NULL)
set_kuap(AMR_KUAP_BLOCK_READ);
else
set_kuap(0);
}

and in asm/kup.h we have:

static inline void allow_read_from_user(const void __user *from, 
unsigned long size)

{
allow_user_access(NULL, from, size);
}

static inline void allow_write_to_user(void __user *to, unsigned long size)
{
allow_user_access(to, NULL, size);
}


If GCC doesn't see NULL as a constant, then the above doesn't work as 
expected.


What's surprising and frustrating is that if you remove the 
__builtin_constant_p() and only leave the NULL check, then GCC sees it 
as a constant and drops the other leg.


So if we remove the __builtin_constant_p(to) and leave only the (to == 
NULL), it will work as expected for allow_read_from_user(). But for the 
others where (to) is not a constant, the NULL test will remain together 
with the associated leg.


Christophe

[PATCH v4 1/2] powerpc/pseries/svm: Use FW_FEATURE to detect SVM

[Sukadev Bhattiprolu]

Use FW_FEATURE_SVM to detect a secure guest (SVM). This would be
more efficient than calling mfmsr() frequently.

Suggested-by: Michael Ellerman 
Signed-off-by: Sukadev Bhattiprolu 
---
 arch/powerpc/include/asm/firmware.h   | 3 ++-
 arch/powerpc/include/asm/svm.h| 6 +-
 arch/powerpc/kernel/paca.c| 6 +-
 arch/powerpc/platforms/pseries/firmware.c | 3 +++
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/firmware.h 
b/arch/powerpc/include/asm/firmware.h
index b3e214a97f3a..23cffcec8a55 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -51,6 +51,7 @@
 #define FW_FEATURE_BLOCK_REMOVE ASM_CONST(0x0010)
 #define FW_FEATURE_PAPR_SCMASM_CONST(0x0020)
 #define FW_FEATURE_ULTRAVISOR  ASM_CONST(0x0040)
+#define FW_FEATURE_SVM ASM_CONST(0x0080)
 
 #ifndef __ASSEMBLY__
 
@@ -69,7 +70,7 @@ enum {
FW_FEATURE_TYPE1_AFFINITY | FW_FEATURE_PRRN |
FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRMEM_V2 |
FW_FEATURE_DRC_INFO | FW_FEATURE_BLOCK_REMOVE |
-   FW_FEATURE_PAPR_SCM | FW_FEATURE_ULTRAVISOR,
+   FW_FEATURE_PAPR_SCM | FW_FEATURE_ULTRAVISOR | FW_FEATURE_SVM,
FW_FEATURE_PSERIES_ALWAYS = 0,
FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL | FW_FEATURE_ULTRAVISOR,
FW_FEATURE_POWERNV_ALWAYS = 0,
diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
index 85580b30aba4..1d056c70fa87 100644
--- a/arch/powerpc/include/asm/svm.h
+++ b/arch/powerpc/include/asm/svm.h
@@ -10,9 +10,13 @@
 
 #ifdef CONFIG_PPC_SVM
 
+/*
+ * Note that this is not usable in early boot - before FW
+ * features were probed
+ */
 static inline bool is_secure_guest(void)
 {
-   return mfmsr() & MSR_S;
+   return firmware_has_feature(FW_FEATURE_SVM);
 }
 
 void dtl_cache_ctor(void *addr);
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 949eceb254d8..3cba33a99549 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -120,7 +120,11 @@ static struct lppaca * __init new_lppaca(int cpu, unsigned 
long limit)
if (early_cpu_has_feature(CPU_FTR_HVMODE))
return NULL;
 
-   if (is_secure_guest())
+   /*
+* Firmware features may not have been probed yet, so check
+* MSR rather than FW_FEATURE_SVM in is_secure_guest().
+*/
+   if (mfmsr() & MSR_S)
lp = alloc_shared_lppaca(LPPACA_SIZE, 0x400, limit, cpu);
else
lp = alloc_paca_data(LPPACA_SIZE, 0x400, limit, cpu);
diff --git a/arch/powerpc/platforms/pseries/firmware.c 
b/arch/powerpc/platforms/pseries/firmware.c
index d4a8f1702417..c98527fb4937 100644
--- a/arch/powerpc/platforms/pseries/firmware.c
+++ b/arch/powerpc/platforms/pseries/firmware.c
@@ -175,4 +175,7 @@ static int __init probe_fw_features(unsigned long node, 
const char *uname, int
 void __init pseries_probe_fw_features(void)
 {
of_scan_flat_dt(probe_fw_features, NULL);
+
+   if (mfmsr() & MSR_S)
+   powerpc_firmware_features |= FW_FEATURE_SVM;
 }
-- 
2.17.2

[PATCH v4 2/2] powerpc/pseries/svm: Disable BHRB/EBB/PMU access

[Sukadev Bhattiprolu]

Ultravisor disables some CPU features like BHRB, EBB and PMU in secure
virtual machines (SVMs) for now. Skip accessing those registers in
SVMs to avoid getting a Program Interrupt.

Basic performance monitoring in SVMs is likely to be enabled in the future
after adding the necessary security mechanisms in Ultravisor. Some features,
like BHRB or monitoring event counts in HV-mode (e.g: perf stat -e cycles:h)
may still be restricted for the longer term.

Signed-off-by: Sukadev Bhattiprolu 
Acked-by: Madhavan Srinivasan 
---
Changelog[v4]
- [Paul Mackerras] Drop is_secure_guest() checks in HV-only code
  and indicate if the disabling of PMU is temporary.
- For consistency, also skip registering PMUs in secure guests.

Changelog[v2]
- [Michael Ellerman] Optimize the code using FW_FEATURE_SVM
- Merged EBB/BHRB and PMU patches into one and reorganized code.
- Fix some build errors reported by 
---
 arch/powerpc/kernel/cpu_setup_power.S | 21 +++
 arch/powerpc/kernel/process.c | 23 
 arch/powerpc/perf/power9-pmu.c| 10 +
 arch/powerpc/xmon/xmon.c  | 30 +--
 4 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/kernel/cpu_setup_power.S 
b/arch/powerpc/kernel/cpu_setup_power.S
index a460298c7ddb..9e895d8db468 100644
--- a/arch/powerpc/kernel/cpu_setup_power.S
+++ b/arch/powerpc/kernel/cpu_setup_power.S
@@ -206,14 +206,35 @@ __init_PMU_HV_ISA207:
blr
 
 __init_PMU:
+#ifdef CONFIG_PPC_SVM
+   /*
+* SVM's are restricted from accessing PMU, so skip.
+*/
+   mfmsr   r5
+   rldicl  r5, r5, 64-MSR_S_LG, 62
+   cmpwi   r5,1
+   beq skip1
+#endif
li  r5,0
mtspr   SPRN_MMCRA,r5
mtspr   SPRN_MMCR0,r5
mtspr   SPRN_MMCR1,r5
mtspr   SPRN_MMCR2,r5
+skip1:
blr
 
 __init_PMU_ISA207:
+
+#ifdef CONFIG_PPC_SVM
+   /*
+* SVM's are restricted from accessing PMU, so skip.
+   */
+   mfmsr   r5
+   rldicl  r5, r5, 64-MSR_S_LG, 62
+   cmpwi   r5,1
+   beq skip2
+#endif
li  r5,0
mtspr   SPRN_MMCRS,r5
+skip2:
blr
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 639ceae7da9d..83c7c4119305 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -64,6 +64,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1059,9 +1060,11 @@ static inline void save_sprs(struct thread_struct *t)
t->dscr = mfspr(SPRN_DSCR);
 
if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
-   t->bescr = mfspr(SPRN_BESCR);
-   t->ebbhr = mfspr(SPRN_EBBHR);
-   t->ebbrr = mfspr(SPRN_EBBRR);
+   if (!is_secure_guest()) {
+   t->bescr = mfspr(SPRN_BESCR);
+   t->ebbhr = mfspr(SPRN_EBBHR);
+   t->ebbrr = mfspr(SPRN_EBBRR);
+   }
 
t->fscr = mfspr(SPRN_FSCR);
 
@@ -1097,12 +1100,14 @@ static inline void restore_sprs(struct thread_struct 
*old_thread,
}
 
if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
-   if (old_thread->bescr != new_thread->bescr)
-   mtspr(SPRN_BESCR, new_thread->bescr);
-   if (old_thread->ebbhr != new_thread->ebbhr)
-   mtspr(SPRN_EBBHR, new_thread->ebbhr);
-   if (old_thread->ebbrr != new_thread->ebbrr)
-   mtspr(SPRN_EBBRR, new_thread->ebbrr);
+   if (!is_secure_guest()) {
+   if (old_thread->bescr != new_thread->bescr)
+   mtspr(SPRN_BESCR, new_thread->bescr);
+   if (old_thread->ebbhr != new_thread->ebbhr)
+   mtspr(SPRN_EBBHR, new_thread->ebbhr);
+   if (old_thread->ebbrr != new_thread->ebbrr)
+   mtspr(SPRN_EBBRR, new_thread->ebbrr);
+   }
 
if (old_thread->fscr != new_thread->fscr)
mtspr(SPRN_FSCR, new_thread->fscr);
diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
index 08c3ef796198..c6eca682180d 100644
--- a/arch/powerpc/perf/power9-pmu.c
+++ b/arch/powerpc/perf/power9-pmu.c
@@ -10,6 +10,7 @@
 #define pr_fmt(fmt)"power9-pmu: " fmt
 
 #include "isa207-common.h"
+#include 
 
 /*
  * Raw event encoding for Power9:
@@ -446,6 +447,15 @@ int init_power9_pmu(void)
strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power9"))
return -ENODEV;
 
+   /*
+* Disable PMUs in secure guests until we evaluate security
+* exposure and add relevant functionality in Ultravisor.
+*/
+   if (is_secure_guest()) {
+   printk("Not registering Performance Monitor in secure guest\n");
+   return 0;
+   }

[PATCH FIX] KVM: PPC: Book3S HV: Release lock on page-out failure path

[Bharata B Rao]

When migrate_vma_setup() fails in kvmppc_svm_page_out(),
release kvm->arch.uvmem_lock before returning.

Fixes: ca9f4942670 ("KVM: PPC: Book3S HV: Support for running secure guests")
Signed-off-by: Bharata B Rao 
---
Applies on paulus/kvm-ppc-next branch

 arch/powerpc/kvm/book3s_hv_uvmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c 
b/arch/powerpc/kvm/book3s_hv_uvmem.c
index 4d1f25a3959a..79b1202b1c62 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -571,7 +571,7 @@ kvmppc_svm_page_out(struct vm_area_struct *vma, unsigned 
long start,
 
ret = migrate_vma_setup();
if (ret)
-   return ret;
+   goto out;
 
spage = migrate_pfn_to_page(*mig.src);
if (!spage || !(*mig.src & MIGRATE_PFN_MIGRATE))
-- 
2.21.0

Re: [PATCH v5 0/4] KASAN for powerpc64 radix

[Daniel Axtens]

Christophe Leroy  writes:

> Le 09/01/2020 à 08:08, Daniel Axtens a écrit :
>> Building on the work of Christophe, Aneesh and Balbir, I've ported
>> KASAN to 64-bit Book3S kernels running on the Radix MMU.
>> 
>> This provides full inline instrumentation on radix, but does require
>> that you be able to specify the amount of physically contiguous memory
>> on the system at compile time. More details in patch 4.
>
> This might be a stupid idea as I don't know ppc64 much. IIUC, PPC64 
> kernel can be relocated, there is no requirement to have it at address 
> 0. Therefore, would it be possible to put the KASAN shadow mem at the 
> begining of the physical memory, instead of putting it at the end ?
> That way, you wouldn't need to know the amount of memory at compile time 
> because KASAN shadow mem would always be at address 0.

Good question! I've had a look. Bearing in mind that I'm not an expert
in ppc64 early load, I think it would be possible, but a large chunk of
work.

One challenge is that - as I understand it - the early relocation code
in head_64.S currently allows the kernel to either:
 - run at the address it's loaded at by kexec/the bootloader, or
 - relocate the kernel to 0

As far as I can tell book3s 64bit doesn't have code to arbitrarily
relocate the kernel.

It's possible I'm wrong about this, in which case I'm happy to reasses!

If I'm right, I think we'd want to implement KASLR for book3s first,
along the lines of how book3e does it. That would allow the kernel to be
put at an arbitrary location at runtime. We could then leverage that.

Another challenge is that some of the interrupt vectors are not easy to
relocate, so we'd have to work around that. That's probably not too big
an issue and we'd pick that up in KASLR implementation.

So I think this is something we could come back to once we have KASLR.

Regards,
Daniel

>
> Christophe

Re: [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running

[Florian Fainelli]

On 1/21/2020 1:09 PM, Heiner Kallweit wrote:
> Convert suitable drivers to use new helper phy_do_ioctl_running.
> 
> Signed-off-by: Heiner Kallweit 
The vast majority of drivers that you are converting use the following
convention:

- !netif_running -> return -EINVAL
- !dev->phydev -> return -ENODEV

so it may make sense to change the helper to accommodate the majority
here, not that I believe this is going to make much practical
difference, but if there were test cases that were specifically looking
for such an error code, they could be failing after this changeset.

For bgmac.c, bcmgenet.c and cpmac.c:

Acked-by: Florian Fainelli 

Whether you decide to spin another version or not.
-- 
Florian

[PATCH] selftests/eeh: Bump EEH wait time to 60s

[Oliver O'Halloran]

Some newer cards supported by aacraid can take up to 40s to recover
after an EEH event. This causes spurious failures in the basic EEH
self-test since the current maximim timeout is only 30s.

Fix the immediate issue by bumping the timeout to a default of 60s,
and allow the wait time to be specified via an environmental variable
(EEH_MAX_WAIT).

Reported-by: Steve Best 
Suggested-by: Douglas Miller 
Signed-off-by: Oliver O'Halloran 
---
 tools/testing/selftests/powerpc/eeh/eeh-functions.sh | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/powerpc/eeh/eeh-functions.sh 
b/tools/testing/selftests/powerpc/eeh/eeh-functions.sh
index 26112ab5cdf4..f52ed92b53e7 100755
--- a/tools/testing/selftests/powerpc/eeh/eeh-functions.sh
+++ b/tools/testing/selftests/powerpc/eeh/eeh-functions.sh
@@ -53,9 +53,13 @@ eeh_one_dev() {
# is a no-op.
echo $dev >/sys/kernel/debug/powerpc/eeh_dev_check
 
-   # Enforce a 30s timeout for recovery. Even the IPR, which is infamously
-   # slow to reset, should recover within 30s.
-   max_wait=30
+   # Default to a 60s timeout when waiting for a device to recover. This
+   # is an arbitrary default which can be overridden by setting the
+   # EEH_MAX_WAIT environmental variable when required.
+
+   # The current record holder for longest recovery time is:
+   #  "Adaptec Series 8 12G SAS/PCIe 3" at 39 seconds
+   max_wait=${EEH_MAX_WAIT:=60}
 
for i in `seq 0 ${max_wait}` ; do
if pe_ok $dev ; then
-- 
2.21.1

Re: [PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running

[Timur Tabi]

On 1/21/20 3:09 PM, Heiner Kallweit wrote:

  drivers/net/ethernet/qualcomm/emac/emac.c  | 14 +-


Acked-by: Timur Tabi 

[PATCH v2 net-next] net: convert suitable drivers to use phy_do_ioctl_running

[Heiner Kallweit]

Convert suitable drivers to use new helper phy_do_ioctl_running.

Signed-off-by: Heiner Kallweit 
---
v2: I forgot the netdev mailing list
---
 drivers/net/ethernet/allwinner/sun4i-emac.c| 15 +--
 drivers/net/ethernet/amd/au1000_eth.c  | 13 +
 drivers/net/ethernet/arc/emac_main.c   | 14 +-
 drivers/net/ethernet/broadcom/bgmac.c  | 10 +-
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 14 +-
 drivers/net/ethernet/dnet.c| 15 +--
 .../ethernet/freescale/fs_enet/fs_enet-main.c  | 10 +-
 drivers/net/ethernet/hisilicon/hisi_femac.c| 14 +-
 drivers/net/ethernet/hisilicon/hns/hns_enet.c  | 16 +---
 drivers/net/ethernet/nxp/lpc_eth.c | 15 +--
 drivers/net/ethernet/qualcomm/emac/emac.c  | 14 +-
 drivers/net/ethernet/renesas/sh_eth.c  | 18 ++
 drivers/net/ethernet/smsc/smsc911x.c   | 11 +--
 drivers/net/ethernet/smsc/smsc9420.c   | 11 +--
 drivers/net/ethernet/ti/cpmac.c| 12 +---
 drivers/net/ethernet/toshiba/tc35815.c | 12 +---
 drivers/net/ethernet/xilinx/ll_temac_main.c| 13 +
 drivers/net/usb/ax88172a.c | 13 +
 drivers/net/usb/lan78xx.c  | 10 +-
 19 files changed, 20 insertions(+), 230 deletions(-)

diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c 
b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 5ea806423..22cadfbee 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -207,19 +207,6 @@ static void emac_inblk_32bit(void __iomem *reg, void 
*data, int count)
readsl(reg, data, round_up(count, 4) / 4);
 }
 
-static int emac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-   struct phy_device *phydev = dev->phydev;
-
-   if (!netif_running(dev))
-   return -EINVAL;
-
-   if (!phydev)
-   return -ENODEV;
-
-   return phy_mii_ioctl(phydev, rq, cmd);
-}
-
 /* ethtool ops */
 static void emac_get_drvinfo(struct net_device *dev,
  struct ethtool_drvinfo *info)
@@ -791,7 +778,7 @@ static const struct net_device_ops emac_netdev_ops = {
.ndo_start_xmit = emac_start_xmit,
.ndo_tx_timeout = emac_timeout,
.ndo_set_rx_mode= emac_set_rx_mode,
-   .ndo_do_ioctl   = emac_ioctl,
+   .ndo_do_ioctl   = phy_do_ioctl_running,
.ndo_validate_addr  = eth_validate_addr,
.ndo_set_mac_address= emac_set_mac_address,
 #ifdef CONFIG_NET_POLL_CONTROLLER
diff --git a/drivers/net/ethernet/amd/au1000_eth.c 
b/drivers/net/ethernet/amd/au1000_eth.c
index 6acf5aa99..089a4fbc6 100644
--- a/drivers/net/ethernet/amd/au1000_eth.c
+++ b/drivers/net/ethernet/amd/au1000_eth.c
@@ -1053,23 +1053,12 @@ static void au1000_multicast_list(struct net_device 
*dev)
writel(reg, >mac->control);
 }
 
-static int au1000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-   if (!netif_running(dev))
-   return -EINVAL;
-
-   if (!dev->phydev)
-   return -EINVAL; /* PHY not controllable */
-
-   return phy_mii_ioctl(dev->phydev, rq, cmd);
-}
-
 static const struct net_device_ops au1000_netdev_ops = {
.ndo_open   = au1000_open,
.ndo_stop   = au1000_close,
.ndo_start_xmit = au1000_tx,
.ndo_set_rx_mode= au1000_multicast_list,
-   .ndo_do_ioctl   = au1000_ioctl,
+   .ndo_do_ioctl   = phy_do_ioctl_running,
.ndo_tx_timeout = au1000_tx_timeout,
.ndo_set_mac_address= eth_mac_addr,
.ndo_validate_addr  = eth_validate_addr,
diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index 6f2c86778..17bda4e8c 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -781,18 +781,6 @@ static int arc_emac_set_address(struct net_device *ndev, 
void *p)
return 0;
 }
 
-static int arc_emac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-   if (!netif_running(dev))
-   return -EINVAL;
-
-   if (!dev->phydev)
-   return -ENODEV;
-
-   return phy_mii_ioctl(dev->phydev, rq, cmd);
-}
-
-
 /**
  * arc_emac_restart - Restart EMAC
  * @ndev:  Pointer to net_device structure.
@@ -857,7 +845,7 @@ static const struct net_device_ops arc_emac_netdev_ops = {
.ndo_set_mac_address= arc_emac_set_address,
.ndo_get_stats  = arc_emac_stats,
.ndo_set_rx_mode= arc_emac_set_rx_mode,
-   .ndo_do_ioctl   = arc_emac_ioctl,
+   .ndo_do_ioctl   = phy_do_ioctl_running,
 #ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller  

[PATCH net-next] net: convert suitable drivers to use phy_do_ioctl_running

[Heiner Kallweit]

Convert suitable drivers to use new helper phy_do_ioctl_running.

Signed-off-by: Heiner Kallweit 
---
 drivers/net/ethernet/allwinner/sun4i-emac.c| 15 +--
 drivers/net/ethernet/amd/au1000_eth.c  | 13 +
 drivers/net/ethernet/arc/emac_main.c   | 14 +-
 drivers/net/ethernet/broadcom/bgmac.c  | 10 +-
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 14 +-
 drivers/net/ethernet/dnet.c| 15 +--
 .../ethernet/freescale/fs_enet/fs_enet-main.c  | 10 +-
 drivers/net/ethernet/hisilicon/hisi_femac.c| 14 +-
 drivers/net/ethernet/hisilicon/hns/hns_enet.c  | 16 +---
 drivers/net/ethernet/nxp/lpc_eth.c | 15 +--
 drivers/net/ethernet/qualcomm/emac/emac.c  | 14 +-
 drivers/net/ethernet/renesas/sh_eth.c  | 18 ++
 drivers/net/ethernet/smsc/smsc911x.c   | 11 +--
 drivers/net/ethernet/smsc/smsc9420.c   | 11 +--
 drivers/net/ethernet/ti/cpmac.c| 12 +---
 drivers/net/ethernet/toshiba/tc35815.c | 12 +---
 drivers/net/ethernet/xilinx/ll_temac_main.c| 13 +
 drivers/net/usb/ax88172a.c | 13 +
 drivers/net/usb/lan78xx.c  | 10 +-
 19 files changed, 20 insertions(+), 230 deletions(-)

diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c 
b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 5ea806423..22cadfbee 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -207,19 +207,6 @@ static void emac_inblk_32bit(void __iomem *reg, void 
*data, int count)
readsl(reg, data, round_up(count, 4) / 4);
 }
 
-static int emac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-   struct phy_device *phydev = dev->phydev;
-
-   if (!netif_running(dev))
-   return -EINVAL;
-
-   if (!phydev)
-   return -ENODEV;
-
-   return phy_mii_ioctl(phydev, rq, cmd);
-}
-
 /* ethtool ops */
 static void emac_get_drvinfo(struct net_device *dev,
  struct ethtool_drvinfo *info)
@@ -791,7 +778,7 @@ static const struct net_device_ops emac_netdev_ops = {
.ndo_start_xmit = emac_start_xmit,
.ndo_tx_timeout = emac_timeout,
.ndo_set_rx_mode= emac_set_rx_mode,
-   .ndo_do_ioctl   = emac_ioctl,
+   .ndo_do_ioctl   = phy_do_ioctl_running,
.ndo_validate_addr  = eth_validate_addr,
.ndo_set_mac_address= emac_set_mac_address,
 #ifdef CONFIG_NET_POLL_CONTROLLER
diff --git a/drivers/net/ethernet/amd/au1000_eth.c 
b/drivers/net/ethernet/amd/au1000_eth.c
index 6acf5aa99..089a4fbc6 100644
--- a/drivers/net/ethernet/amd/au1000_eth.c
+++ b/drivers/net/ethernet/amd/au1000_eth.c
@@ -1053,23 +1053,12 @@ static void au1000_multicast_list(struct net_device 
*dev)
writel(reg, >mac->control);
 }
 
-static int au1000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-   if (!netif_running(dev))
-   return -EINVAL;
-
-   if (!dev->phydev)
-   return -EINVAL; /* PHY not controllable */
-
-   return phy_mii_ioctl(dev->phydev, rq, cmd);
-}
-
 static const struct net_device_ops au1000_netdev_ops = {
.ndo_open   = au1000_open,
.ndo_stop   = au1000_close,
.ndo_start_xmit = au1000_tx,
.ndo_set_rx_mode= au1000_multicast_list,
-   .ndo_do_ioctl   = au1000_ioctl,
+   .ndo_do_ioctl   = phy_do_ioctl_running,
.ndo_tx_timeout = au1000_tx_timeout,
.ndo_set_mac_address= eth_mac_addr,
.ndo_validate_addr  = eth_validate_addr,
diff --git a/drivers/net/ethernet/arc/emac_main.c 
b/drivers/net/ethernet/arc/emac_main.c
index 6f2c86778..17bda4e8c 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -781,18 +781,6 @@ static int arc_emac_set_address(struct net_device *ndev, 
void *p)
return 0;
 }
 
-static int arc_emac_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
-{
-   if (!netif_running(dev))
-   return -EINVAL;
-
-   if (!dev->phydev)
-   return -ENODEV;
-
-   return phy_mii_ioctl(dev->phydev, rq, cmd);
-}
-
-
 /**
  * arc_emac_restart - Restart EMAC
  * @ndev:  Pointer to net_device structure.
@@ -857,7 +845,7 @@ static const struct net_device_ops arc_emac_netdev_ops = {
.ndo_set_mac_address= arc_emac_set_address,
.ndo_get_stats  = arc_emac_stats,
.ndo_set_rx_mode= arc_emac_set_rx_mode,
-   .ndo_do_ioctl   = arc_emac_ioctl,
+   .ndo_do_ioctl   = phy_do_ioctl_running,
 #ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller= arc_emac_poll_controller,
 #endif

Re: GCC bug ? Re: [PATCH v2 10/10] powerpc/32s: Implement Kernel Userspace Access Protection

[Segher Boessenkool]

On Tue, Jan 21, 2020 at 05:22:32PM +, Christophe Leroy wrote:
> g1() should return 3, not 5.

What makes you say that?

"A return of 0 does not indicate that the
 value is _not_ a constant, but merely that GCC cannot prove it is a
 constant with the specified value of the '-O' option."

(And the rules it uses for this are *not* the same as C "constant
expressions" or C "integer constant expression" or C "arithmetic
constant expression" or anything like that -- which should be already
obvious from that it changes with different -Ox).

You can use builtin_constant_p to have the compiler do something better
if the compiler feels like it, but not anything more.  Often people
want stronger guarantees, but when they see how much less often it then
returns "true", they do not want that either.


Segher

Re: [RFC 5/6] perf/tools: Enhance JSON/metric infrastructure to handle "?"

[Andi Kleen]

> Here, '?' will be replaced with a runtime value and metric expression will
> be replicated.

Okay seems reasonable to me.

Thanks,

-Andi

Re: [PATCH v5 01/10] capabilities: introduce CAP_PERFMON to kernel and user space

[Alexey Budankov]

On 21.01.2020 20:55, Alexei Starovoitov wrote:
> On Tue, Jan 21, 2020 at 9:31 AM Alexey Budankov
>  wrote:
>>
>>
>> On 21.01.2020 17:43, Stephen Smalley wrote:
>>> On 1/20/20 6:23 AM, Alexey Budankov wrote:

 Introduce CAP_PERFMON capability designed to secure system performance
 monitoring and observability operations so that CAP_PERFMON would assist
 CAP_SYS_ADMIN capability in its governing role for perf_events, i915_perf
 and other performance monitoring and observability subsystems.

 CAP_PERFMON intends to harden system security and integrity during system
 performance monitoring and observability operations by decreasing attack
 surface that is available to a CAP_SYS_ADMIN privileged process [1].
 Providing access to system performance monitoring and observability
 operations under CAP_PERFMON capability singly, without the rest of
 CAP_SYS_ADMIN credentials, excludes chances to misuse the credentials and
 makes operation more secure.

 CAP_PERFMON intends to take over CAP_SYS_ADMIN credentials related to
 system performance monitoring and observability operations and balance
 amount of CAP_SYS_ADMIN credentials following the recommendations in the
 capabilities man page [1] for CAP_SYS_ADMIN: "Note: this capability is
 overloaded; see Notes to kernel developers, below."

 Although the software running under CAP_PERFMON can not ensure avoidance
 of related hardware issues, the software can still mitigate these issues
 following the official embargoed hardware issues mitigation procedure [2].
 The bugs in the software itself could be fixed following the standard
 kernel development process [3] to maintain and harden security of system
 performance monitoring and observability operations.

 [1] http://man7.org/linux/man-pages/man7/capabilities.7.html
 [2] 
 https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
 [3] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html

 Signed-off-by: Alexey Budankov 
 ---
   include/linux/capability.h  | 12 
   include/uapi/linux/capability.h |  8 +++-
   security/selinux/include/classmap.h |  4 ++--
   3 files changed, 21 insertions(+), 3 deletions(-)

 diff --git a/include/linux/capability.h b/include/linux/capability.h
 index ecce0f43c73a..8784969d91e1 100644
 --- a/include/linux/capability.h
 +++ b/include/linux/capability.h
 @@ -251,6 +251,18 @@ extern bool privileged_wrt_inode_uidgid(struct 
 user_namespace *ns, const struct
   extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
   extern bool file_ns_capable(const struct file *file, struct 
 user_namespace *ns, int cap);
   extern bool ptracer_capable(struct task_struct *tsk, struct 
 user_namespace *ns);
 +static inline bool perfmon_capable(void)
 +{
 +struct user_namespace *ns = _user_ns;
 +
 +if (ns_capable_noaudit(ns, CAP_PERFMON))
 +return ns_capable(ns, CAP_PERFMON);
 +
 +if (ns_capable_noaudit(ns, CAP_SYS_ADMIN))
 +return ns_capable(ns, CAP_SYS_ADMIN);
 +
 +return false;
 +}
>>>
>>> Why _noaudit()?  Normally only used when a permission failure is non-fatal 
>>> to the operation.  Otherwise, we want the audit message.
>>
>> Some of ideas from v4 review.
> 
> well, in the requested changes form v4 I wrote:
> return capable(CAP_PERFMON);
> instead of
> return false;

Aww, indeed. I was concerning exactly about it when updating the patch
and simply put false, missing the fact that capable() also logs.

I suppose the idea is originally from here [1].
BTW, Has it already seen any _more optimal_ implementation?
Anyway, original or optimized version could be reused for CAP_PERFMON.

~Alexey

[1] https://patchwork.ozlabs.org/patch/1159243/

> 
> That's what Andy suggested earlier for CAP_BPF.
> I think that should resolve Stephen's concern.
> 

Re: [PATCH v5 01/10] capabilities: introduce CAP_PERFMON to kernel and user space

[Alexei Starovoitov]

On Tue, Jan 21, 2020 at 9:31 AM Alexey Budankov
 wrote:
>
>
> On 21.01.2020 17:43, Stephen Smalley wrote:
> > On 1/20/20 6:23 AM, Alexey Budankov wrote:
> >>
> >> Introduce CAP_PERFMON capability designed to secure system performance
> >> monitoring and observability operations so that CAP_PERFMON would assist
> >> CAP_SYS_ADMIN capability in its governing role for perf_events, i915_perf
> >> and other performance monitoring and observability subsystems.
> >>
> >> CAP_PERFMON intends to harden system security and integrity during system
> >> performance monitoring and observability operations by decreasing attack
> >> surface that is available to a CAP_SYS_ADMIN privileged process [1].
> >> Providing access to system performance monitoring and observability
> >> operations under CAP_PERFMON capability singly, without the rest of
> >> CAP_SYS_ADMIN credentials, excludes chances to misuse the credentials and
> >> makes operation more secure.
> >>
> >> CAP_PERFMON intends to take over CAP_SYS_ADMIN credentials related to
> >> system performance monitoring and observability operations and balance
> >> amount of CAP_SYS_ADMIN credentials following the recommendations in the
> >> capabilities man page [1] for CAP_SYS_ADMIN: "Note: this capability is
> >> overloaded; see Notes to kernel developers, below."
> >>
> >> Although the software running under CAP_PERFMON can not ensure avoidance
> >> of related hardware issues, the software can still mitigate these issues
> >> following the official embargoed hardware issues mitigation procedure [2].
> >> The bugs in the software itself could be fixed following the standard
> >> kernel development process [3] to maintain and harden security of system
> >> performance monitoring and observability operations.
> >>
> >> [1] http://man7.org/linux/man-pages/man7/capabilities.7.html
> >> [2] 
> >> https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
> >> [3] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html
> >>
> >> Signed-off-by: Alexey Budankov 
> >> ---
> >>   include/linux/capability.h  | 12 
> >>   include/uapi/linux/capability.h |  8 +++-
> >>   security/selinux/include/classmap.h |  4 ++--
> >>   3 files changed, 21 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/linux/capability.h b/include/linux/capability.h
> >> index ecce0f43c73a..8784969d91e1 100644
> >> --- a/include/linux/capability.h
> >> +++ b/include/linux/capability.h
> >> @@ -251,6 +251,18 @@ extern bool privileged_wrt_inode_uidgid(struct 
> >> user_namespace *ns, const struct
> >>   extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
> >>   extern bool file_ns_capable(const struct file *file, struct 
> >> user_namespace *ns, int cap);
> >>   extern bool ptracer_capable(struct task_struct *tsk, struct 
> >> user_namespace *ns);
> >> +static inline bool perfmon_capable(void)
> >> +{
> >> +struct user_namespace *ns = _user_ns;
> >> +
> >> +if (ns_capable_noaudit(ns, CAP_PERFMON))
> >> +return ns_capable(ns, CAP_PERFMON);
> >> +
> >> +if (ns_capable_noaudit(ns, CAP_SYS_ADMIN))
> >> +return ns_capable(ns, CAP_SYS_ADMIN);
> >> +
> >> +return false;
> >> +}
> >
> > Why _noaudit()?  Normally only used when a permission failure is non-fatal 
> > to the operation.  Otherwise, we want the audit message.
>
> Some of ideas from v4 review.

well, in the requested changes form v4 I wrote:
return capable(CAP_PERFMON);
instead of
return false;

That's what Andy suggested earlier for CAP_BPF.
I think that should resolve Stephen's concern.

Re: [PATCH v5 01/10] capabilities: introduce CAP_PERFMON to kernel and user space

[Alexey Budankov]

On 21.01.2020 17:43, Stephen Smalley wrote:
> On 1/20/20 6:23 AM, Alexey Budankov wrote:
>>
>> Introduce CAP_PERFMON capability designed to secure system performance
>> monitoring and observability operations so that CAP_PERFMON would assist
>> CAP_SYS_ADMIN capability in its governing role for perf_events, i915_perf
>> and other performance monitoring and observability subsystems.
>>
>> CAP_PERFMON intends to harden system security and integrity during system
>> performance monitoring and observability operations by decreasing attack
>> surface that is available to a CAP_SYS_ADMIN privileged process [1].
>> Providing access to system performance monitoring and observability
>> operations under CAP_PERFMON capability singly, without the rest of
>> CAP_SYS_ADMIN credentials, excludes chances to misuse the credentials and
>> makes operation more secure.
>>
>> CAP_PERFMON intends to take over CAP_SYS_ADMIN credentials related to
>> system performance monitoring and observability operations and balance
>> amount of CAP_SYS_ADMIN credentials following the recommendations in the
>> capabilities man page [1] for CAP_SYS_ADMIN: "Note: this capability is
>> overloaded; see Notes to kernel developers, below."
>>
>> Although the software running under CAP_PERFMON can not ensure avoidance
>> of related hardware issues, the software can still mitigate these issues
>> following the official embargoed hardware issues mitigation procedure [2].
>> The bugs in the software itself could be fixed following the standard
>> kernel development process [3] to maintain and harden security of system
>> performance monitoring and observability operations.
>>
>> [1] http://man7.org/linux/man-pages/man7/capabilities.7.html
>> [2] 
>> https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
>> [3] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html
>>
>> Signed-off-by: Alexey Budankov 
>> ---
>>   include/linux/capability.h  | 12 
>>   include/uapi/linux/capability.h |  8 +++-
>>   security/selinux/include/classmap.h |  4 ++--
>>   3 files changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/capability.h b/include/linux/capability.h
>> index ecce0f43c73a..8784969d91e1 100644
>> --- a/include/linux/capability.h
>> +++ b/include/linux/capability.h
>> @@ -251,6 +251,18 @@ extern bool privileged_wrt_inode_uidgid(struct 
>> user_namespace *ns, const struct
>>   extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
>>   extern bool file_ns_capable(const struct file *file, struct user_namespace 
>> *ns, int cap);
>>   extern bool ptracer_capable(struct task_struct *tsk, struct user_namespace 
>> *ns);
>> +static inline bool perfmon_capable(void)
>> +{
>> +    struct user_namespace *ns = _user_ns;
>> +
>> +    if (ns_capable_noaudit(ns, CAP_PERFMON))
>> +    return ns_capable(ns, CAP_PERFMON);
>> +
>> +    if (ns_capable_noaudit(ns, CAP_SYS_ADMIN))
>> +    return ns_capable(ns, CAP_SYS_ADMIN);
>> +
>> +    return false;
>> +}
> 
> Why _noaudit()?  Normally only used when a permission failure is non-fatal to 
> the operation.  Otherwise, we want the audit message.

Some of ideas from v4 review.
Well, on the second sight, it defenitly should be logged for CAP_SYS_ADMIN.
Probably it is not so fatal for CAP_PERFMON, but personally 
I would unconditionally log it for CAP_PERFMON as well.
Good catch, thank you.

~Alexey

GCC bug ? Re: [PATCH v2 10/10] powerpc/32s: Implement Kernel Userspace Access Protection

[Christophe Leroy]

On 04/18/2019 06:55 AM, Michael Ellerman wrote:

Christophe Leroy  writes:

diff --git a/arch/powerpc/include/asm/book3s/32/kup.h 
b/arch/powerpc/include/asm/book3s/32/kup.h
index 5f97c742ca71..b3560b2de435 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -37,6 +37,113 @@

...

+
+static inline void allow_user_access(void __user *to, const void __user *from, 
u32 size)
+{
+   u32 addr = (__force u32)to;
+   u32 end = min(addr + size, TASK_SIZE);
+
+   if (!addr || addr >= TASK_SIZE || !size)
+   return;
+
+   current->thread.kuap = (addr & 0xf000) | end - 1) >> 28) + 1) & 
0xf);
+   kuap_update_sr(mfsrin(addr) & ~SR_KS, addr, end);   /* Clear Ks */
+}


When rebasing on my v6 I changed the above to:

static inline void allow_user_access(void __user *to, const void __user *from, 
u32 size)
{
u32 addr, end;

if (__builtin_constant_p(to) && to == NULL)
return;


Look like the above doesn't work: gcc bug ?

#define NULL (void*)0

static inline int f1(void *to)
{
if (__builtin_constant_p(to) && to == NULL)
return 3;
return 5;
}

int g1(void)
{
return f1(NULL);
}

static inline int f2(int x)
{
if (__builtin_constant_p(x) && x == 0)
return 7;
return 9;
}

int g2(void)
{
return f2(0);
}



toto.o: file format elf32-powerpc


Disassembly of section .text:

 :
   0:   38 60 00 05 li  r3,5
   4:   4e 80 00 20 blr

0008 :
   8:   38 60 00 07 li  r3,7
   c:   4e 80 00 20 blr



It works for the int const, but not for the pointer const:

g1() should return 3, not 5. GCC bug ?

Christophe

Re: [PATCH v5 01/10] capabilities: introduce CAP_PERFMON to kernel and user space

[Stephen Smalley]

On 1/20/20 6:23 AM, Alexey Budankov wrote:


Introduce CAP_PERFMON capability designed to secure system performance
monitoring and observability operations so that CAP_PERFMON would assist
CAP_SYS_ADMIN capability in its governing role for perf_events, i915_perf
and other performance monitoring and observability subsystems.

CAP_PERFMON intends to harden system security and integrity during system
performance monitoring and observability operations by decreasing attack
surface that is available to a CAP_SYS_ADMIN privileged process [1].
Providing access to system performance monitoring and observability
operations under CAP_PERFMON capability singly, without the rest of
CAP_SYS_ADMIN credentials, excludes chances to misuse the credentials and
makes operation more secure.

CAP_PERFMON intends to take over CAP_SYS_ADMIN credentials related to
system performance monitoring and observability operations and balance
amount of CAP_SYS_ADMIN credentials following the recommendations in the
capabilities man page [1] for CAP_SYS_ADMIN: "Note: this capability is
overloaded; see Notes to kernel developers, below."

Although the software running under CAP_PERFMON can not ensure avoidance
of related hardware issues, the software can still mitigate these issues
following the official embargoed hardware issues mitigation procedure [2].
The bugs in the software itself could be fixed following the standard
kernel development process [3] to maintain and harden security of system
performance monitoring and observability operations.

[1] http://man7.org/linux/man-pages/man7/capabilities.7.html
[2] 
https://www.kernel.org/doc/html/latest/process/embargoed-hardware-issues.html
[3] https://www.kernel.org/doc/html/latest/admin-guide/security-bugs.html

Signed-off-by: Alexey Budankov 
---
  include/linux/capability.h  | 12 
  include/uapi/linux/capability.h |  8 +++-
  security/selinux/include/classmap.h |  4 ++--
  3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/include/linux/capability.h b/include/linux/capability.h
index ecce0f43c73a..8784969d91e1 100644
--- a/include/linux/capability.h
+++ b/include/linux/capability.h
@@ -251,6 +251,18 @@ extern bool privileged_wrt_inode_uidgid(struct 
user_namespace *ns, const struct
  extern bool capable_wrt_inode_uidgid(const struct inode *inode, int cap);
  extern bool file_ns_capable(const struct file *file, struct user_namespace 
*ns, int cap);
  extern bool ptracer_capable(struct task_struct *tsk, struct user_namespace 
*ns);
+static inline bool perfmon_capable(void)
+{
+   struct user_namespace *ns = _user_ns;
+
+   if (ns_capable_noaudit(ns, CAP_PERFMON))
+   return ns_capable(ns, CAP_PERFMON);
+
+   if (ns_capable_noaudit(ns, CAP_SYS_ADMIN))
+   return ns_capable(ns, CAP_SYS_ADMIN);
+
+   return false;
+}


Why _noaudit()?  Normally only used when a permission failure is 
non-fatal to the operation.  Otherwise, we want the audit message.

Re: [PATCH 1/3] powerpc/tm: Clear the current thread's MSR[TS] after treclaim

[kbuild test robot]

Hi Gustavo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on scottwood/next v5.5-rc7 next-20200120]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:
https://github.com/0day-ci/linux/commits/Gustavo-Luiz-Duarte/powerpc-tm-Clear-the-current-thread-s-MSR-TS-after-treclaim/20200118-034925
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-a001-20200121 (attached as .config)
compiler: powerpc64le-linux-gcc (GCC) 7.5.0
reproduce:
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
GCC_VERSION=7.5.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 

All errors (new ones prefixed by >>):

   arch/powerpc/kernel/signal_32.c: In function 'handle_rt_signal32':
>> arch/powerpc/kernel/signal_32.c:908:16: error: unused variable 'msr' 
>> [-Werror=unused-variable]
 unsigned long msr = regs->msr;
   ^~~
   arch/powerpc/kernel/signal_32.c: In function 'handle_signal32':
   arch/powerpc/kernel/signal_32.c:1367:16: error: unused variable 'msr' 
[-Werror=unused-variable]
 unsigned long msr = regs->msr;
   ^~~
   cc1: all warnings being treated as errors
--
   arch/powerpc/kernel/signal_64.c: In function 'handle_rt_signal64':
>> arch/powerpc/kernel/signal_64.c:821:16: error: unused variable 'msr' 
>> [-Werror=unused-variable]
 unsigned long msr = regs->msr;
   ^~~
   cc1: all warnings being treated as errors

vim +/msr +908 arch/powerpc/kernel/signal_32.c

   891  
   892  /*
   893   * Set up a signal frame for a "real-time" signal handler
   894   * (one which gets siginfo).
   895   */
   896  int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
   897 struct task_struct *tsk)
   898  {
   899  struct rt_sigframe __user *rt_sf;
   900  struct mcontext __user *frame;
   901  struct mcontext __user *tm_frame = NULL;
   902  void __user *addr;
   903  unsigned long newsp = 0;
   904  int sigret;
   905  unsigned long tramp;
   906  struct pt_regs *regs = tsk->thread.regs;
   907  /* Save the thread's msr before get_tm_stackpointer() changes 
it */
 > 908  unsigned long msr = regs->msr;
   909  
   910  BUG_ON(tsk != current);
   911  
   912  /* Set up Signal Frame */
   913  /* Put a Real Time Context onto stack */
   914  rt_sf = get_sigframe(ksig, get_tm_stackpointer(tsk), 
sizeof(*rt_sf), 1);
   915  addr = rt_sf;
   916  if (unlikely(rt_sf == NULL))
   917  goto badframe;
   918  
   919  /* Put the siginfo & fill in most of the ucontext */
   920  if (copy_siginfo_to_user(_sf->info, >info)
   921  || __put_user(0, _sf->uc.uc_flags)
   922  || __save_altstack(_sf->uc.uc_stack, regs->gpr[1])
   923  || __put_user(to_user_ptr(_sf->uc.uc_mcontext),
   924  _sf->uc.uc_regs)
   925  || put_sigset_t(_sf->uc.uc_sigmask, oldset))
   926  goto badframe;
   927  
   928  /* Save user registers on the stack */
   929  frame = _sf->uc.uc_mcontext;
   930  addr = frame;
   931  if (vdso32_rt_sigtramp && tsk->mm->context.vdso_base) {
   932  sigret = 0;
   933  tramp = tsk->mm->context.vdso_base + vdso32_rt_sigtramp;
   934  } else {
   935  sigret = __NR_rt_sigreturn;
   936  tramp = (unsigned long) frame->tramp;
   937  }
   938  

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org Intel Corporation


.config.gz
Description: application/gzip

[Bug 205099] KASAN hit at raid6_pq: BUG: Unable to handle kernel data access at 0x00f0fd0d

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=205099

--- Comment #24 from Erhard F. (erhar...@mailbox.org) ---
Created attachment 286931
  --> https://bugzilla.kernel.org/attachment.cgi?id=286931=edit
screenshot (5.5-rc6+, KASAN_VMALLOC + VMAP_STACK + INLINE KASAN, PowerMac G4
DP)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 205099] KASAN hit at raid6_pq: BUG: Unable to handle kernel data access at 0x00f0fd0d

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=205099

--- Comment #23 from Erhard F. (erhar...@mailbox.org) ---
Created attachment 286929
  --> https://bugzilla.kernel.org/attachment.cgi?id=286929=edit
dmesg (kernel 5.5-rc6+, KASAN_VMALLOC + CONFIG_THREAD_SHIFT=14 + OUTLINE KASAN,
PowerMac G4 DP)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 205099] KASAN hit at raid6_pq: BUG: Unable to handle kernel data access at 0x00f0fd0d

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=205099

--- Comment #22 from Erhard F. (erhar...@mailbox.org) ---
(In reply to Christophe Leroy from comment #19)
> Can you tell exactly where it stops during the boot ? Or take a photo of the
> screen ?
I'll attach a photo shortly.

> In parallele, could you try (without VMAP_STACK) increasing
> CONFIG_THREAD_SHIFT to 14 ? It will double the size of the stacks.
I can confirm that with CONFIG_THREAD_SHIFT=14 the G4 runs good again with
INLINE KASAN + KASAN_VMALLOC enabled (as seen in attached dmesg). No KASAN hit
at raid6_pq, no poblem with btrfs.

Though with CONFIG_THREAD_SHIFT=14 + OUTLINE KASAN + KASAN_VMALLOC I still get
this KASAN hit at raid6_pq.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 205099] KASAN hit at raid6_pq: BUG: Unable to handle kernel data access at 0x00f0fd0d

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=205099

--- Comment #21 from Erhard F. (erhar...@mailbox.org) ---
Created attachment 286927
  --> https://bugzilla.kernel.org/attachment.cgi?id=286927=edit
dmesg (kernel 5.5-rc6+, KASAN_VMALLOC + CONFIG_THREAD_SHIFT=14 + INLINE KASAN,
PowerMac G4 DP)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 205099] KASAN hit at raid6_pq: BUG: Unable to handle kernel data access at 0x00f0fd0d

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=205099

Erhard F. (erhar...@mailbox.org) changed:

   What|Removed |Added

 Attachment #286871|0   |1
is obsolete||

--- Comment #20 from Erhard F. (erhar...@mailbox.org) ---
Created attachment 286925
  --> https://bugzilla.kernel.org/attachment.cgi?id=286925=edit
kernel .config (5.5-rc6+, KASAN_VMALLOC + VMAP_STACK + INLINE KASAN, PowerMac
G4 DP)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

Re: [PATCH RFC v1] mm: is_mem_section_removable() overhaul

[Michal Hocko]

On Mon 20-01-20 10:14:44, David Hildenbrand wrote:
> On 20.01.20 08:48, Michal Hocko wrote:
> > On Fri 17-01-20 08:57:51, Dan Williams wrote:
> > [...]
> >> Unless the user is willing to hold the device_hotplug_lock over the
> >> evaluation then the result is unreliable.
> > 
> > Do we want to hold the device_hotplug_lock from this user readable file
> > in the first place? My book says that this just waits to become a
> > problem.
> 
> It was the "big hammer" solution for this RFC.
> 
> I think we could do with a try_lock() on the device_lock() paired with a
> device->removed flag. The latter is helpful for properly catching zombie
> devices on the onlining/offlining path either way (and on my todo list).

try_lock would be more considerate. It would at least make any potential
hammering a bit harder.

> > Really, the interface is flawed and should have never been merged in the
> > first place. We cannot simply remove it altogether I am afraid so let's
> > at least remove the bogus code and pretend that the world is a better
> > place where everything is removable except the reality sucks...
> 
> As I expressed already, the interface works as designed/documented and
> has been used like that for years.

It seems we do differ in the usefulness though. Using a crappy interface
for years doesn't make it less crappy. I do realize we cannot remove the
interface but we can remove issues with the implementation and I dare to
say that most existing users wouldn't really notice.

> I tend to agree that it never should have been merged like that.
> 
> We have (at least) two places that are racy (with concurrent memory
> hotplug):
> 
> 1. /sys/.../memoryX/removable
> - a) make it always return yes and make the interface useless
> - b) add proper locking and keep it running as is (e.g., so David can
>  identify offlineable memory blocks :) ).
> 
> 2. /sys/.../memoryX/valid_zones
> - a) always return "none" if the memory is online
> - b) add proper locking and keep it running as is
> - c) cache the result ("zone") when a block is onlined (e.g., in
> mem->zone. If it is NULL, either mixed zones or unknown)
> 
> At least 2. already scream for a proper device_lock() locking as the
> mem->state is not stable across the function call.
> 
> 1a and 2a are the easiest solutions but remove all ways to identify if a
> memory block could theoretically be offlined - without trying
> (especially, also to identify the MOVABLE zone).
> 
> I tend to prefer 1b) and 2c), paired with proper device_lock() locking.
> We don't affect existing use cases but are able to simplify the code +
> fix the races.
> 
> What's your opinion? Any alternatives?

1a) and 2c) if you ask me.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH -next] powerpc/maple: fix comparing pointer to 0

[Joe Perches]

On Tue, 2020-01-21 at 01:47 -0600, Segher Boessenkool wrote:
> On Mon, Jan 20, 2020 at 05:52:15PM -0800, Joe Perches wrote:
> > On Tue, 2020-01-21 at 09:31 +0800, Chen Zhou wrote:
> > > Fixes coccicheck warning:
> > > ./arch/powerpc/platforms/maple/setup.c:232:15-16:
> > >   WARNING comparing pointer to 0
> > 
> > Does anyone have or use these powerpc maple boards anymore?
> > 
> > Maybe the whole codebase should just be deleted instead.
> 
> This is used for *all* non-Apple 970 systems (not running virtualized),
> not just actual Maple.

OK, then likely this Kconfig description should be updated
(and the http://www.970eval.com link is no longer about powerpc)

$ cat arch/powerpc/platforms/maple/Kconfig
# SPDX-License-Identifier: GPL-2.0
config PPC_MAPLE
depends on PPC64 && PPC_BOOK3S && CPU_BIG_ENDIAN
bool "Maple 970FX Evaluation Board"
select FORCE_PCI
select MPIC
select U3_DART
select MPIC_U3_HT_IRQS
select GENERIC_TBSYNC
select PPC_UDBG_16550
select PPC_970_NAP
select PPC_NATIVE
select PPC_RTAS
select MMIO_NVRAM
select ATA_NONSTANDARD if ATA
help
  This option enables support for the Maple 970FX Evaluation Board.
  For more information, refer to 

Re: [PATCH] powerpc/mm/hash: Fix the min context value used by userspace.

[Nicholas Piggin]

Aneesh Kumar K.V's on January 8, 2020 3:44 pm:
> Without this kernel can endup with SLB entries as below
> 
> 04 c00c0800 00066bde000a7510 256M ESID=c00c0  VSID=   66bde000a7 
> LLP:110
> 12 0800 00066bde000a7d90 256M ESID=0  VSID=   66bde000a7 
> LLP:110
> 
> Such SLB entries can result in machine check.
> 
> We noticed this with 256MB segments because that resulted in the duplicate 
> VSID
> with first vmemmap segment and first user segement. With 1TB segments we 
> observe
> duplication with EAs like
> 
> 0x100e64b vsid for EA 0xc00db500 context 7
> 0x100e64b vsid for user EA 0x1b500 context 7
> 
> and those high addresses are not common and the kernel mapping in the above 
> case
> is I/O remap range.
> 
> [0.00] vmalloc start = 0xc008
> [0.00] IO start  = 0xc00a
> [0.00] vmemmap start = 0xc00c
> 
> Fixes: 0034d395f89d ("powerpc/mm/hash64: Map all the kernel regions in the 
> same 0xc range")
> Signed-off-by: Aneesh Kumar K.V 
> ---
>  arch/powerpc/include/asm/book3s/64/mmu-hash.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h 
> b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> index 15b75005bc34..516db8a2e6ca 100644
> --- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> +++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
> @@ -601,7 +601,7 @@ extern void slb_set_size(u16 size);
>   */
>  #define MAX_USER_CONTEXT ((ASM_CONST(1) << CONTEXT_BITS) - 2)
>  #define MIN_USER_CONTEXT (MAX_KERNEL_CTX_CNT + MAX_VMALLOC_CTX_CNT + \
> -  MAX_IO_CTX_CNT + MAX_VMEMMAP_CTX_CNT)
> +  MAX_IO_CTX_CNT + MAX_VMEMMAP_CTX_CNT + 1)

Good find and fix, but the changelog is a bit difficult to read.

The bug is an off-by-one error which means the first user context ID
allocated is the vmemmap ID, right? I would lead with that.

I'm not sure that machine checks are a primary symptom, different ESID
mapping the same VSID is allowed. My guess is the machine check happens
a little later, after the vmemmap gets corrupted via its new mapping.

Guessing this hasn't immediately resulted in wholesale mayhem because
- Init is pretty small, doesn't use many segments or pages.
- Low 1TB is mapped with 256MB segments which get a different VA hash
  than the 1TB vmemmap segment.
- init tends to load itself at 256MB, so even with disable_1tb_segments,
  it's clashing with the second vmmemap segment, which is for like the
  second 256GB of memory on the first node, so not going to hit many
  systems.

Anyway good find.

Reviewed-by: Nicholas Piggin 

Thanks,
Nick

[PATCH v2 3/5] powerpc/perf: Add an interface sub-folder to imc pmu

[Anju T Sudhakar]

From: Madhavan Srinivasan 

Patch adds an interface attribute folder to imc pmu.
This is intended to include pmu intreface capabilities
which will be useful to userspace likes selftest
testcases. Patch adds a "glob_lck" file to notify to
userspace of global lock mechanism added to imc devices
like core, thread and trace.

"glob_lck" will be used by selftest file to execute
interface test for the global lock mechanism.

Signed-off-by: Madhavan Srinivasan 
---
 arch/powerpc/include/asm/imc-pmu.h | 11 ++-
 arch/powerpc/perf/imc-pmu.c| 19 +++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/imc-pmu.h 
b/arch/powerpc/include/asm/imc-pmu.h
index 4da4fcba0684..1b2c33c30e7c 100644
--- a/arch/powerpc/include/asm/imc-pmu.h
+++ b/arch/powerpc/include/asm/imc-pmu.h
@@ -87,8 +87,9 @@ struct trace_imc_data {
 /* Event attribute array index */
 #define IMC_FORMAT_ATTR0
 #define IMC_EVENT_ATTR 1
-#define IMC_CPUMASK_ATTR   2
-#define IMC_NULL_ATTR  3
+#define IMC_INTERFACE_ATTR 2
+#define IMC_CPUMASK_ATTR   3
+#define IMC_NULL_ATTR  4
 
 /* PMU Format attribute macros */
 #define IMC_EVENT_OFFSET_MASK  0xULL
@@ -114,10 +115,10 @@ struct imc_pmu {
/*
 * Attribute groups for the PMU. Slot 0 used for
 * format attribute, slot 1 used for cpusmask attribute,
-* slot 2 used for event attribute. Slot 3 keep as
-* NULL.
+* slot 2 used for event attribute. Slot 3 used for interface
+* attribute and Slot 4 is NULL.
 */
-   const struct attribute_group *attr_groups[4];
+   const struct attribute_group *attr_groups[5];
u32 counter_mem_size;
int domain;
/*
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index 2e220f199530..3f49664f29f1 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -54,6 +54,24 @@ static struct imc_pmu_ref imc_global_refc = {
.refc = 0,
 };
 
+static ssize_t glob_lck_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   return sprintf(buf, "%d\n", 1);
+}
+
+static DEVICE_ATTR_RO(glob_lck);
+
+static struct attribute *imc_interface_attrs[] = {
+   _attr_glob_lck.attr,
+   NULL,
+};
+
+static struct attribute_group imc_interface_group = {
+   .name = "interface",
+   .attrs = imc_interface_attrs,
+};
+
 static struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
 {
return container_of(event->pmu, struct imc_pmu, pmu);
@@ -1462,6 +1480,7 @@ static int update_pmu_ops(struct imc_pmu *pmu)
pmu->pmu.attr_groups = pmu->attr_groups;
pmu->pmu.capabilities = PERF_PMU_CAP_NO_EXCLUDE;
pmu->attr_groups[IMC_FORMAT_ATTR] = _format_group;
+   pmu->attr_groups[IMC_INTERFACE_ATTR] = _interface_group;
 
switch (pmu->domain) {
case IMC_DOMAIN_NEST:
-- 
2.20.1

Re: [RFC 5/6] perf/tools: Enhance JSON/metric infrastructure to handle "?"

[kajoljain]

On 1/17/20 9:58 PM, Andi Kleen wrote:

On Fri, Jan 17, 2020 at 06:16:19PM +0530, Kajol Jain wrote:

Patch enhances current metric infrastructure to handle "?" in the metric
expression. The "?" can be use for parameters whose value not known while
creating metric events and which can be replace later at runtime to
the proper value. It also add flexibility to create multiple events out
of single metric event added in json file.

Please add a proper specification how this ? thing is supposed to work,
what the exact semantics are, how it is different from the existing
# mechanism etc.

The standard way to do similar things before was to define an explicit
# name and let the expr code take care of it.


Hi Andi,

    Thanks for reviewing my patchset.

I did review the existing '#' mechanism. It is quiet useful for runtime
dependency (like ex. event selection based on a return value). But I want
to handle couple of more things. Meaning, filling in parameter of event code
based on runtime value and replicating events at runtime.

So the '?' can handle, 1) replacement of a value in the event code based 
on runtime


and 2) replicate the event (metric expr) at runtime (handled in new 
metricgroup__add_metric()).


This patchset add an arch specific function called 
'arch_get_runtimeparam'. Ideally,
arch_get_runtimeparm() returns the number of metric exps one want to 
create (it defaults to 1).

The loop added in 'metricgroup__add_metric' will handle the event addition.

Example ppc64 hv_24x7 pmu metric event using '?':

(hv_24x7@PM_MCS01_128B_RD_DISP_PORT01\\,chip\\=?@ + 
hv_24x7@PM_MCS01_128B_RD_DISP_PORT23\\,chip\\=?@)


Here, '?' will be replaced with a runtime value and metric expression 
will be replicated.


Thanks,

Kajol


-Andi

[PATCH v2 2/5] powerpc/perf: Implement a global lock to avoid races between trace, core and thread imc events.

[Anju T Sudhakar]

IMC(In-memory Collection Counters) does performance monitoring in
two different modes, i.e accumulation mode(core-imc and thread-imc events),
and trace mode(trace-imc events). A cpu thread can either be in
accumulation-mode or trace-mode at a time and this is done via the LDBAR
register in POWER architecture. The current design does not address the
races between thread-imc and trace-imc events.

Patch implements a global id and lock to avoid the races between
core, trace and thread imc events. With this global id-lock
implementation, the system can either run core, thread or trace imc
events at a time. i.e. to run any core-imc events, thread/trace imc events
should not be enabled/monitored.

Signed-off-by: Anju T Sudhakar 
---
 arch/powerpc/perf/imc-pmu.c | 177 +++-
 1 file changed, 153 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index cb50a9e1fd2d..2e220f199530 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -44,6 +44,16 @@ static DEFINE_PER_CPU(u64 *, trace_imc_mem);
 static struct imc_pmu_ref *trace_imc_refc;
 static int trace_imc_mem_size;
 
+/*
+ * Global data structure used to avoid races between thread,
+ * core and trace-imc
+ */
+static struct imc_pmu_ref imc_global_refc = {
+   .lock = __MUTEX_INITIALIZER(imc_global_refc.lock),
+   .id = 0,
+   .refc = 0,
+};
+
 static struct imc_pmu *imc_event_to_pmu(struct perf_event *event)
 {
return container_of(event->pmu, struct imc_pmu, pmu);
@@ -759,6 +769,20 @@ static void core_imc_counters_release(struct perf_event 
*event)
ref->refc = 0;
}
mutex_unlock(>lock);
+
+   mutex_lock(_global_refc.lock);
+   if (imc_global_refc.id == IMC_DOMAIN_CORE) {
+   imc_global_refc.refc--;
+   /*
+* If no other thread is running any core-imc
+* event, set the global id to zero.
+*/
+   if (imc_global_refc.refc <= 0) {
+   imc_global_refc.refc = 0;
+   imc_global_refc.id = 0;
+   }
+   }
+   mutex_unlock(_global_refc.lock);
 }
 
 static int core_imc_event_init(struct perf_event *event)
@@ -779,6 +803,22 @@ static int core_imc_event_init(struct perf_event *event)
if (event->cpu < 0)
return -EINVAL;
 
+   /*
+* Take the global lock, and make sure
+* no other thread is running any trace OR thread imc event
+*/
+   mutex_lock(_global_refc.lock);
+   if (imc_global_refc.id == 0) {
+   imc_global_refc.id = IMC_DOMAIN_CORE;
+   imc_global_refc.refc++;
+   } else if (imc_global_refc.id == IMC_DOMAIN_CORE) {
+   imc_global_refc.refc++;
+   } else {
+   mutex_unlock(_global_refc.lock);
+   return -EBUSY;
+   }
+   mutex_unlock(_global_refc.lock);
+
event->hw.idx = -1;
pmu = imc_event_to_pmu(event);
 
@@ -877,7 +917,16 @@ static int ppc_thread_imc_cpu_online(unsigned int cpu)
 
 static int ppc_thread_imc_cpu_offline(unsigned int cpu)
 {
-   mtspr(SPRN_LDBAR, 0);
+   /*
+* Toggle the bit 0 of LDBAR.
+*
+* If bit 0 of LDBAR is unset, it will stop posting
+* the counetr data to memory.
+* For thread-imc, bit 0 of LDBAR will be set to 1 in the
+* event_add function. So toggle this bit here, to stop the updates
+* to memory in the cpu_offline path.
+*/
+   mtspr(SPRN_LDBAR, (mfspr(SPRN_LDBAR) ^ (1UL << 63)));
return 0;
 }
 
@@ -889,6 +938,24 @@ static int thread_imc_cpu_init(void)
  ppc_thread_imc_cpu_offline);
 }
 
+static void thread_imc_counters_release(struct perf_event *event)
+{
+
+   mutex_lock(_global_refc.lock);
+   if (imc_global_refc.id == IMC_DOMAIN_THREAD) {
+   imc_global_refc.refc--;
+   /*
+* If no other thread is running any thread-imc
+* event, set the global id to zero.
+*/
+   if (imc_global_refc.refc <= 0) {
+   imc_global_refc.refc = 0;
+   imc_global_refc.id = 0;
+   }
+   }
+   mutex_unlock(_global_refc.lock);
+}
+
 static int thread_imc_event_init(struct perf_event *event)
 {
u32 config = event->attr.config;
@@ -905,6 +972,27 @@ static int thread_imc_event_init(struct perf_event *event)
if (event->hw.sample_period)
return -EINVAL;
 
+   mutex_lock(_global_refc.lock);
+   /*
+* Check if any other thread is running
+* core-engine, if not set the global id to
+* thread-imc.
+*/
+   if (imc_global_refc.id == 0) {
+   imc_global_refc.id = IMC_DOMAIN_THREAD;
+   imc_global_refc.refc++;
+   } else if (imc_global_refc.id == IMC_DOMAIN_THREAD) {

[PATCH v2 5/5] selftest/powerpc/pmu: Testcase for imc global lock mechanism

[Anju T Sudhakar]

From: Madhavan Srinivasan 

Signed-off-by: Madhavan Srinivasan 
---
 .../pmu/mem_counters/imc_global_lock_test.c   | 49 ++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git 
a/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c 
b/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
index ea687ffc1990..f643dba8ecc0 100644
--- a/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
+++ b/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
@@ -5,9 +5,56 @@
 
 #include "mem_counters.h"
 
+static  bool check_imc_interface_glob_lck(void)
+{
+   if (!access("/sys/devices/thread_imc/interface/glob_lck", F_OK))
+   return true;
+
+   return false;
+}
+
 static int testcase(void)
 {
-   return 0;
+   struct event events[2];
+
+   if (!check_imc_interface_glob_lck()) {
+   printf("Test not supported\n");
+   return MAGIC_SKIP_RETURN_VALUE;
+   }
+
+   if (!is_mem_counters_device_enabled(CORE) || 
!is_mem_counters_device_enabled(THREAD)) {
+   printf("%s: IMC device not found. So exiting the test\n", 
__FUNCTION__);
+   return -1;
+   }
+
+   if (setup_mem_counters_event(THREAD, [0], 0xe0, 
"thread_imc/cycles")) {
+   printf("%s setup_mem_counters_event for thread_imc failed\n", 
__FUNCTION__);
+   return -1;
+   }
+
+   if (setup_mem_counters_event(CORE, [1], 0xe0, 
"core_imc/cycles")) {
+   printf("%s setup_mem_counters_event for core_imc failed\n", 
__FUNCTION__);
+   return -1;
+   }
+
+   if (event_open([0])) {
+   perror("thread_imc: perf_event_open");
+   return -1;
+   }
+
+   /*
+* If we have the Global lock patchset applied to kernel
+* event_open for events[1] should fail with resource busy
+*/
+   if (event_open_with_cpu([1], 0)) {
+   /*
+* Check for the errno to certify the test result
+*/
+   if (errno == 16) // Resource busy (EBUSY)
+   return 0;
+   }
+
+   return -1;
 }
 
 static int imc_global_lock_test(void)
-- 
2.20.1

[PATCH v2 4/5] selftest/powerpc/pmc: Support to include interface test for Memory Counter PMUs

[Anju T Sudhakar]

From: Madhavan Srinivasan 

Patch to add support to include interface tests for
memory counter PMUs as part of selftest.
These PMUs are primarily used to understand socket/chip/core
resourage usage. In PowerNV envirnoment, the perf interface
registered to access these counters are called "In Memory Collection"
(IMC) and in PowerVM, the perf interface registered to access
these counters are called "hv_24x7".

New folder "mem_counters" added under selftest/powerpc/pmu.
This will include interface tests for both "imc" and "hv_24x7"
pmus. Patch adds base/common functioned needed.
To make blame easier, a place-holder test function added to
this patch. Subsequent patch will fill in the actual test
content.

Signed-off-by: Madhavan Srinivasan 
---
 tools/testing/selftests/powerpc/pmu/Makefile  |  7 +-
 .../powerpc/pmu/mem_counters/Makefile | 21 
 .../pmu/mem_counters/imc_global_lock_test.c   | 21 
 .../powerpc/pmu/mem_counters/mem_counters.c   | 99 +++
 .../powerpc/pmu/mem_counters/mem_counters.h   | 36 +++
 5 files changed, 182 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/powerpc/pmu/mem_counters/Makefile
 create mode 100644 
tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
 create mode 100644 
tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.c
 create mode 100644 
tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.h

diff --git a/tools/testing/selftests/powerpc/pmu/Makefile 
b/tools/testing/selftests/powerpc/pmu/Makefile
index 19046db995fe..e352eceac0a9 100644
--- a/tools/testing/selftests/powerpc/pmu/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/Makefile
@@ -8,7 +8,7 @@ EXTRA_SOURCES := ../harness.c event.c lib.c ../utils.c
 top_srcdir = ../../../../..
 include ../../lib.mk
 
-all: $(TEST_GEN_PROGS) ebb
+all: $(TEST_GEN_PROGS) ebb mem_counters
 
 $(TEST_GEN_PROGS): $(EXTRA_SOURCES)
 
@@ -43,4 +43,7 @@ clean:
 ebb:
TARGET=$@; BUILD_TARGET=$$OUTPUT/$$TARGET; mkdir -p $$BUILD_TARGET; 
$(MAKE) OUTPUT=$$BUILD_TARGET -k -C $$TARGET all
 
-.PHONY: all run_tests clean ebb
+mem_counters:
+   TARGET=$@; BUILD_TARGET=$$OUTPUT/$$TARGET; mkdir -p $$BUILD_TARGET; 
$(MAKE) OUTPUT=$$BUILD_TARGET -k -C $$TARGET all
+
+.PHONY: all run_tests clean ebb mem_counters
diff --git a/tools/testing/selftests/powerpc/pmu/mem_counters/Makefile 
b/tools/testing/selftests/powerpc/pmu/mem_counters/Makefile
new file mode 100644
index ..f39ebe30ab70
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/mem_counters/Makefile
@@ -0,0 +1,21 @@
+# SPDX-License-Identifier: GPL-2.0
+include ../../../../../../scripts/Kbuild.include
+
+noarg:
+   $(MAKE) -C ../../
+
+CFLAGS += -m64
+
+# Toolchains may build PIE by default which breaks the assembly
+no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
+$(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -no-pie -x c - -o 
"$$TMP", -no-pie)
+
+LDFLAGS += $(no-pie-option)
+
+TEST_GEN_PROGS := imc_global_lock_test
+
+top_srcdir = ../../../../../..
+include ../../../lib.mk
+
+$(TEST_GEN_PROGS): ../../harness.c ../../utils.c ../event.c ../lib.c 
./mem_counters.c \
+   imc_global_lock_test.c
diff --git 
a/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c 
b/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
new file mode 100644
index ..ea687ffc1990
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020, Madhavan Srinivasan, IBM Corp.
+ */
+
+#include "mem_counters.h"
+
+static int testcase(void)
+{
+   return 0;
+}
+
+static int imc_global_lock_test(void)
+{
+   return eat_cpu(testcase);
+}
+
+int main(void)
+{
+   return test_harness(imc_global_lock_test, "imc_global_lock_test");
+}
diff --git a/tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.c 
b/tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.c
new file mode 100644
index ..b0ee1319f018
--- /dev/null
+++ b/tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020, Madhavan Srinivasan, IBM Corp.
+ */
+
+#include "mem_counters.h"
+
+/*
+ * mem_counters.c will contain common/basic functions
+ * to support testcases for both In Memory Collection (IMC)
+ * and hv_24x7 counters.
+ */
+
+
+/*
+ * Since device type enum starts with 1,
+ * have the first entry in the array as a placeholder.
+ */
+const char mem_counters_dev_path[][30] = {
+   "",
+   "/sys/devices/thread_imc",
+   "/sys/devices/trace_imc",
+   "/sys/devices/core_imc",
+   "/sys/devices/hv_24x7",
+   "",
+};
+
+const char mem_counters_dev_type_path[][35] = {
+   "",
+   "/sys/devices/thread_imc/type",
+   "/sys/devices/trace_imc/type",
+   

[PATCH v2 1/5] powerpc/powernv: Re-enable imc trace-mode in kernel

[Anju T Sudhakar]

commit <249fad734a25> ""powerpc/perf: Disable trace_imc pmu"
disables IMC(In-Memory Collection) trace-mode in kernel, since frequent
mode switching between accumulation mode and trace mode via the spr LDBAR
in the hardware can trigger a checkstop(system crash).

Patch to re-enable imc-trace mode in kernel.

The following patch in this series will address the mode switching issue
by implementing a global lock, and will restrict the usage of
accumulation and trace-mode at a time.

Signed-off-by: Anju T Sudhakar 
---
 arch/powerpc/platforms/powernv/opal-imc.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-imc.c 
b/arch/powerpc/platforms/powernv/opal-imc.c
index 000b350d4060..3b4518f4b643 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -278,14 +278,7 @@ static int opal_imc_counters_probe(struct platform_device 
*pdev)
domain = IMC_DOMAIN_THREAD;
break;
case IMC_TYPE_TRACE:
-   /*
-* FIXME. Using trace_imc events to monitor application
-* or KVM thread performance can cause a checkstop
-* (system crash).
-* Disable it for now.
-*/
-   pr_info_once("IMC: disabling trace_imc PMU\n");
-   domain = -1;
+   domain = IMC_DOMAIN_TRACE;
break;
default:
pr_warn("IMC Unknown Device type \n");
-- 
2.20.1

[PATCH v2 0/5] Re-enable IMC trace-mode

[Anju T Sudhakar]

commit <249fad734a25> ""powerpc/perf: Disable trace_imc pmu"   
disables IMC(In-Memory Collection) trace-mode in kernel, since frequent   
mode switching between accumulation mode and trace mode via the spr LDBAR  
in the hardware can trigger a checkstop(system crash).

This patch series re-enables IMC trace mode and fixes the mode switching
issue by global lock mechanism.

Patch 3/5,4/5 and 5/5 provides a selftest to verify the global-lock
mechanism.

Changes from v1 -> v2:
-
- Added self test patches to the series.

Anju T Sudhakar (2):
  powerpc/powernv: Re-enable imc trace-mode in kernel
  powerpc/perf: Implement a global lock to avoid races between trace,
core and thread imc events.

Madhavan Srinivasan (3):
  powerpc/perf: Add an interface sub-folder to imc pmu
  selftest/powerpc/pmc: Support to include interface test for Memory
Counter PMUs
  selftest/powerpc/pmu: Testcase for imc global lock mechanism


 arch/powerpc/include/asm/imc-pmu.h|  11 +-
 arch/powerpc/perf/imc-pmu.c   | 196 +++---
 arch/powerpc/platforms/powernv/opal-imc.c |   9 +-
 tools/testing/selftests/powerpc/pmu/Makefile  |   7 +-
 .../powerpc/pmu/mem_counters/Makefile |  21 ++
 .../pmu/mem_counters/imc_global_lock_test.c   |  68 ++
 .../powerpc/pmu/mem_counters/mem_counters.c   |  99 +
 .../powerpc/pmu/mem_counters/mem_counters.h   |  36 
 8 files changed, 408 insertions(+), 39 deletions(-)
 create mode 100644 tools/testing/selftests/powerpc/pmu/mem_counters/Makefile
 create mode 100644 
tools/testing/selftests/powerpc/pmu/mem_counters/imc_global_lock_test.c
 create mode 100644 
tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.c
 create mode 100644 
tools/testing/selftests/powerpc/pmu/mem_counters/mem_counters.h

-- 
2.18.1

Re: [PATCH] powerpc/sysdev: fix compile errors

[Christophe Leroy]

Le 21/01/2020 à 07:59, 王文虎 a écrit :

发件人：Andrew Donnellan 
发送日期：2020-01-21 14:13:07
收件人：wangwenhu ,Benjamin Herrenschmidt ,Paul Mackerras 
,Michael Ellerman ,Kate Stewart ,Greg 
Kroah-Hartman ,Richard Fontana ,Thomas Gleixner 
,linuxppc-dev@lists.ozlabs.org,linux-ker...@vger.kernel.org
抄送人：triv...@kernel.org,loneh...@hotmail.com,wenhu.w...@vivo.com
主题：Re: [PATCH] powerpc/sysdev: fix compile errors>On 21/1/20 4:31 pm, wangwenhu 
wrote:

From: wangwenhu 

Include arch/powerpc/include/asm/io.h into fsl_85xx_cache_sram.c to
fix the implicit declaration compile errors when building Cache-Sram.

arch/powerpc/sysdev/fsl_85xx_cache_sram.c: In function ‘instantiate_cache_sram’:
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:26: error: implicit declaration of 
function ‘ioremap_coherent’; did you mean ‘bitmap_complement’? 
[-Werror=implicit-function-declaration]
cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
^~~~
bitmap_complement
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:24: error: assignment makes 
pointer from integer without a cast [-Werror=int-conversion]
cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
  ^
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:123:2: error: implicit declaration of 
function ‘iounmap’; did you mean ‘roundup’? 
[-Werror=implicit-function-declaration]
iounmap(cache_sram->base_virt);
^~~
roundup
cc1: all warnings being treated as errors

Signed-off-by: wangwenhu 


How long has this code been broken for?


It's been broken almost 15 months since the commit below:
"commit aa91796ec46339f2ed53da311bd3ea77a3e4dfe1
Author: Christophe Leroy 
Date:   Tue Oct 9 13:51:41 2018 +

 powerpc: don't use ioremap_prot() nor __ioremap() unless really needed."

And we are working on it now for further development.


That's pretty surprising. That commit didn't change the iounmap(). It 
only replaced ioremap_prot() by ioremap_coherent(). Both are defined in io.h


Christophe






---
   arch/powerpc/sysdev/fsl_85xx_cache_sram.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c 
b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
index f6c665dac725..29b6868eff7d 100644
--- a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
+++ b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
@@ -17,6 +17,7 @@
   #include 
   #include 
   #include 
+#include 

   #include "fsl_85xx_cache_ctlr.h"



--
Andrew Donnellan  OzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited



Wenhu

Re: [PATCH v2 05/27] powerpc: Map & release OpenCAPI LPC memory

[Greg Kurz]

On Tue, 21 Jan 2020 17:46:12 +1100
Andrew Donnellan  wrote:

> On 3/12/19 2:46 pm, Alastair D'Silva wrote:
> > From: Alastair D'Silva 
> > 
> > This patch adds platform support to map & release LPC memory.
> 
> Might want to explain what LPC is.
> 
> Otherwise:
> 
> Reviewed-by: Andrew Donnellan 
> 
> > 
> > Signed-off-by: Alastair D'Silva 
> > ---
> >   arch/powerpc/include/asm/pnv-ocxl.h   |  2 ++
> >   arch/powerpc/platforms/powernv/ocxl.c | 42 +++
> >   2 files changed, 44 insertions(+)
> > 
> > diff --git a/arch/powerpc/include/asm/pnv-ocxl.h 
> > b/arch/powerpc/include/asm/pnv-ocxl.h
> > index 7de82647e761..f8f8ffb48aa8 100644
> > --- a/arch/powerpc/include/asm/pnv-ocxl.h
> > +++ b/arch/powerpc/include/asm/pnv-ocxl.h
> > @@ -32,5 +32,7 @@ extern int pnv_ocxl_spa_remove_pe_from_cache(void 
> > *platform_data, int pe_handle)
> >   
> >   extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
> >   extern void pnv_ocxl_free_xive_irq(u32 irq);
> > +extern u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size);
> > +extern void pnv_ocxl_platform_lpc_release(struct pci_dev *pdev);
> 
> nit: I don't think these need to be extern?
> 
> 

And even if they were, as verified by checkpatch:

"extern prototypes should be avoided in .h files"

Re: [PATCH] powerpc/sysdev: fix compile errors

[Christophe Leroy]

Le 21/01/2020 à 07:59, 王文虎 a écrit :

发件人：Andrew Donnellan 
发送日期：2020-01-21 14:13:07
收件人：wangwenhu ,Benjamin Herrenschmidt ,Paul Mackerras 
,Michael Ellerman ,Kate Stewart ,Greg 
Kroah-Hartman ,Richard Fontana ,Thomas Gleixner 
,linuxppc-dev@lists.ozlabs.org,linux-ker...@vger.kernel.org
抄送人：triv...@kernel.org,loneh...@hotmail.com,wenhu.w...@vivo.com
主题：Re: [PATCH] powerpc/sysdev: fix compile errors>On 21/1/20 4:31 pm, wangwenhu 
wrote:

From: wangwenhu 

Include arch/powerpc/include/asm/io.h into fsl_85xx_cache_sram.c to
fix the implicit declaration compile errors when building Cache-Sram.

arch/powerpc/sysdev/fsl_85xx_cache_sram.c: In function ‘instantiate_cache_sram’:
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:26: error: implicit declaration of 
function ‘ioremap_coherent’; did you mean ‘bitmap_complement’? 
[-Werror=implicit-function-declaration]
cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
^~~~
bitmap_complement
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:97:24: error: assignment makes 
pointer from integer without a cast [-Werror=int-conversion]
cache_sram->base_virt = ioremap_coherent(cache_sram->base_phys,
  ^
arch/powerpc/sysdev/fsl_85xx_cache_sram.c:123:2: error: implicit declaration of 
function ‘iounmap’; did you mean ‘roundup’? 
[-Werror=implicit-function-declaration]
iounmap(cache_sram->base_virt);
^~~
roundup
cc1: all warnings being treated as errors

Signed-off-by: wangwenhu 


How long has this code been broken for?


It's been broken almost 15 months since the commit below:
"commit aa91796ec46339f2ed53da311bd3ea77a3e4dfe1


Can you then add a Fixes: tag ?

Thanks
Christophe


Author: Christophe Leroy 
Date:   Tue Oct 9 13:51:41 2018 +

 powerpc: don't use ioremap_prot() nor __ioremap() unless really needed."

And we are working on it now for further development.




---
   arch/powerpc/sysdev/fsl_85xx_cache_sram.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c 
b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
index f6c665dac725..29b6868eff7d 100644
--- a/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
+++ b/arch/powerpc/sysdev/fsl_85xx_cache_sram.c
@@ -17,6 +17,7 @@
   #include 
   #include 
   #include 
+#include 

   #include "fsl_85xx_cache_ctlr.h"



--
Andrew Donnellan  OzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited



Wenhu