Re: [RFC PATCH 5/6] pkeys: Up level pkey_free() checks
Le 11/06/2022 à 01:35, ira.we...@intel.com a écrit :
> From: Ira Weiny
>
> x86 is missing a hardware check for pkey support in pkey_free(). While
> the net result is the same (-EINVAL returned), pkey_free() has well
> defined behavior which will be easier to maintain in one place.
>
> For powerpc the return code is -1 rather than -EINVAL. This changes
> that behavior slightly but this is very unlikely to break any user
> space.
>
> Lift the checks for pkey_free() to the core mm code and ensure
> consistency with returning -EINVAL.
>
> Cc: ah...@chromium.org
> Cc: cleme...@chromium.org
> Cc: gdee...@chromium.org
> Cc: jkumme...@chromium.org
> Cc: manosk...@chromium.org
> Cc: thiba...@chromium.org
> Cc: Florian Weimer
> Cc: Andrew Morton
> Cc: linux-...@vger.kernel.org
> Cc: Sohil Mehta
> Cc: Dave Hansen
> Cc: Aneesh Kumar K.V
> Signed-off-by: Ira Weiny
>
> ---
> Thanks to Sohil for suggesting I mention the powerpc return value in the
> commit message.
>
> Also Sohil suggested changing mm_pkey_free() from int to void. This is
> added as a separate patch with his suggested by.
> ---
> arch/powerpc/include/asm/pkeys.h | 6 --
> arch/x86/include/asm/pkeys.h | 3 ---
> mm/mprotect.c| 8 ++--
> 3 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pkeys.h
> b/arch/powerpc/include/asm/pkeys.h
> index 2c8351248793..e96aa91f817b 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -107,12 +107,6 @@ static inline int mm_pkey_alloc(struct mm_struct *mm)
>
> static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
> {
> - if (!mmu_has_feature(MMU_FTR_PKEY))
> - return -1;
> -
> - if (!mm_pkey_is_allocated(mm, pkey))
> - return -EINVAL;
> -
> __mm_pkey_free(mm, pkey);
>
> return 0;
If it returns always 0, the return value is pointless and the function
mm_pkey_free() should be changed to return void.
> diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
> index 2e6c04d8a45b..da02737cc4d1 100644
> --- a/arch/x86/include/asm/pkeys.h
> +++ b/arch/x86/include/asm/pkeys.h
> @@ -107,9 +107,6 @@ int mm_pkey_alloc(struct mm_struct *mm)
> static inline
> int mm_pkey_free(struct mm_struct *mm, int pkey)
> {
> - if (!mm_pkey_is_allocated(mm, pkey))
> - return -EINVAL;
> -
> mm_set_pkey_free(mm, pkey);
>
> return 0;
Same.
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 56d35de33725..41458e729c27 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -803,10 +803,14 @@ SYSCALL_DEFINE2(pkey_alloc, unsigned long, flags,
> unsigned long, init_val)
>
> SYSCALL_DEFINE1(pkey_free, int, pkey)
> {
> - int ret;
> + int ret = -EINVAL;
Don't initialise 'ret'
> +
> + if (!arch_pkeys_enabled())
> + return ret;
Make it explicit, do 'return -EINVAL'
Once that is done, is there any point in having a fallback version of
mm_pkey_free() which returns -EINVAL ?
>
> mmap_write_lock(current->mm);
> - ret = mm_pkey_free(current->mm, pkey);
> + if (mm_pkey_is_allocated(current->mm, pkey))
> + ret = mm_pkey_free(current->mm, pkey);
Add:
else
ret = -EINVAL;
> mmap_write_unlock(current->mm);
>
> /*
Re: [RFC PATCH 6/6] pkeys: Change mm_pkey_free() to void
Le 11/06/2022 à 01:35, ira.we...@intel.com a écrit :
> From: Ira Weiny
>
> Now that the pkey arch support is no longer checked in mm_pkey_free()
> there is no reason to have it return int.
Right, I see this is doing what I commented in previous patch.
>
> Change the return value to void.
>
> Cc: Dave Hansen
> Cc: Aneesh Kumar K.V
> Suggested-by: Sohil Mehta
> Signed-off-by: Ira Weiny
> ---
> arch/powerpc/include/asm/pkeys.h | 4 +---
> arch/x86/include/asm/pkeys.h | 4 +---
> include/linux/pkeys.h| 5 +
> mm/mprotect.c| 6 --
> 4 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pkeys.h
> b/arch/powerpc/include/asm/pkeys.h
> index e96aa91f817b..4d01a48ab941 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -105,11 +105,9 @@ static inline int mm_pkey_alloc(struct mm_struct *mm)
> return ret;
> }
>
> -static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
> +static inline void mm_pkey_free(struct mm_struct *mm, int pkey)
> {
> __mm_pkey_free(mm, pkey);
> -
> - return 0;
> }
>
> /*
> diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
> index da02737cc4d1..1f408f46fa9a 100644
> --- a/arch/x86/include/asm/pkeys.h
> +++ b/arch/x86/include/asm/pkeys.h
> @@ -105,11 +105,9 @@ int mm_pkey_alloc(struct mm_struct *mm)
> }
>
> static inline
> -int mm_pkey_free(struct mm_struct *mm, int pkey)
> +void mm_pkey_free(struct mm_struct *mm, int pkey)
> {
> mm_set_pkey_free(mm, pkey);
> -
> - return 0;
> }
>
> static inline int vma_pkey(struct vm_area_struct *vma)
> diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
> index 86be8bf27b41..bf98c50a3437 100644
> --- a/include/linux/pkeys.h
> +++ b/include/linux/pkeys.h
> @@ -30,10 +30,7 @@ static inline int mm_pkey_alloc(struct mm_struct *mm)
> return -1;
> }
>
> -static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
> -{
> - return -EINVAL;
> -}
> +static inline void mm_pkey_free(struct mm_struct *mm, int pkey) { }
>
> static inline int arch_set_user_pkey_access(struct task_struct *tsk, int
> pkey,
> unsigned long init_val)
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 41458e729c27..e872bdd2e228 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -809,8 +809,10 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
> return ret;
>
> mmap_write_lock(current->mm);
> - if (mm_pkey_is_allocated(current->mm, pkey))
> - ret = mm_pkey_free(current->mm, pkey);
> + if (mm_pkey_is_allocated(current->mm, pkey)) {
> + mm_pkey_free(current->mm, pkey);
> + ret = 0;
> + }
Or you could have ret = 0 by default and do
if (mm_pkey_is_allocated(current->mm, pkey))
mm_pkey_free(current->mm, pkey);
else
ret = -EINVAL;
> mmap_write_unlock(current->mm);
>
> /*
[PATCH v4.14] powerpc/32: Fix overread/overwrite of thread_struct via ptrace
commit 8e127846fc97778a5e5c99bca1ce0bbc5ec9 upstream.
The ptrace PEEKUSR/POKEUSR (aka PEEKUSER/POKEUSER) API allows a process
to read/write registers of another process.
To get/set a register, the API takes an index into an imaginary address
space called the "USER area", where the registers of the process are
laid out in some fashion.
The kernel then maps that index to a particular register in its own data
structures and gets/sets the value.
The API only allows a single machine-word to be read/written at a time.
So 4 bytes on 32-bit kernels and 8 bytes on 64-bit kernels.
The way floating point registers (FPRs) are addressed is somewhat
complicated, because double precision float values are 64-bit even on
32-bit CPUs. That means on 32-bit kernels each FPR occupies two
word-sized locations in the USER area. On 64-bit kernels each FPR
occupies one word-sized location in the USER area.
Internally the kernel stores the FPRs in an array of u64s, or if VSX is
enabled, an array of pairs of u64s where one half of each pair stores
the FPR. Which half of the pair stores the FPR depends on the kernel's
endianness.
To handle the different layouts of the FPRs depending on VSX/no-VSX and
big/little endian, the TS_FPR() macro was introduced.
Unfortunately the TS_FPR() macro does not take into account the fact
that the addressing of each FPR differs between 32-bit and 64-bit
kernels. It just takes the index into the "USER area" passed from
userspace and indexes into the fp_state.fpr array.
On 32-bit there are 64 indexes that address FPRs, but only 32 entries in
the fp_state.fpr array, meaning the user can read/write 256 bytes past
the end of the array. Because the fp_state sits in the middle of the
thread_struct there are various fields than can be overwritten,
including some pointers. As such it may be exploitable.
It has also been observed to cause systems to hang or otherwise
misbehave when using gdbserver, and is probably the root cause of this
report which could not be easily reproduced:
https://lore.kernel.org/linuxppc-dev/dc38afe9-6b78-f3f5-666b-986939e40...@keymile.com/
Rather than trying to make the TS_FPR() macro even more complicated to
fix the bug, or add more macros, instead add a special-case for 32-bit
kernels. This is more obvious and hopefully avoids a similar bug
happening again in future.
Note that because 32-bit kernels never have VSX enabled the code doesn't
need to consider TS_FPRWIDTH/OFFSET at all.
Fixes: 87fec0514f61 ("powerpc: PTRACE_PEEKUSR/PTRACE_POKEUSER of FPR registers
in little endian builds")
Cc: sta...@vger.kernel.org # v3.13+
Reported-by: Ariel Miculas
Tested-by: Christophe Leroy
Signed-off-by: Michael Ellerman
Link: https://lore.kernel.org/r/20220609133245.573565-1-...@ellerman.id.au
---
arch/powerpc/kernel/ptrace.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index bfc5f59d9f1b..ef5875f83692 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -2920,8 +2920,13 @@ long arch_ptrace(struct task_struct *child, long request,
flush_fp_to_thread(child);
if (fpidx < (PT_FPSCR - PT_FPR0))
- memcpy(&tmp, &child->thread.TS_FPR(fpidx),
- sizeof(long));
+ if (IS_ENABLED(CONFIG_PPC32)) {
+ // On 32-bit the index we are passed
refers to 32-bit words
+ tmp = ((u32
*)child->thread.fp_state.fpr)[fpidx];
+ } else {
+ memcpy(&tmp,
&child->thread.TS_FPR(fpidx),
+ sizeof(long));
+ }
else
tmp = child->thread.fp_state.fpscr;
}
@@ -2953,8 +2958,13 @@ long arch_ptrace(struct task_struct *child, long request,
flush_fp_to_thread(child);
if (fpidx < (PT_FPSCR - PT_FPR0))
- memcpy(&child->thread.TS_FPR(fpidx), &data,
- sizeof(long));
+ if (IS_ENABLED(CONFIG_PPC32)) {
+ // On 32-bit the index we are passed
refers to 32-bit words
+ ((u32
*)child->thread.fp_state.fpr)[fpidx] = data;
+ } else {
+ memcpy(&child->thread.TS_FPR(fpidx),
&data,
+ sizeof(long));
+ }
else
child->thread.fp_state.fpscr = data;
ret = 0;
--
2.35.3
Re: [PATCH v4 15/18] PCI: dwc: Add dw_ prefix to the pcie_port structure name
gkai Hu , "linux-arm-ker...@lists.infradead.org"
, Roy Zang , Jingoo Han
, "linuxppc-dev@lists.ozlabs.org"
, Heiko Stuebner ,
"linux-ker...@vger.kernel.org" , Serge Semin
, Stanimir Varbanov , Krzysztof
Kozlowski , Masami Hiramatsu
, Pengutronix Kernel Team , Gustavo
Pimentel , Shawn Guo ,
Lucas Stach
Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org
Sender: "Linuxppc-dev"
On Fri, Jun 10, 2022 at 10:25:31AM +0200, Serge Semin wrote:
> All of the DW PCIe core driver entities have names with the dw_ prefix in
> order to easily distinguish local and common PCIe name spaces. All except
> the pcie_port structure which contains the DW PCIe Root Port descriptor.
> For historical reason the structure has retained the original name since
> commit 340cba6092c2 ("pci: Add PCIe driver for Samsung Exynos") when
> the DW PCIe IP-core support was added to the kernel. Let's finally fix
> that by adding the dw_ prefix to the structure name and by adding the _rp
> suffix to be similar to the EP counterpart. Thus the name will be coherent
> with the common driver naming policy. It shall make the driver code more
> readable eliminating visual confusion between the local and generic PCI
> name spaces.
Hi Serge,
I think that most variable and parameters of this type is named "pp" for
"pcie_port".
If this is the way we want to go, those should be changed also to "rp", right?
/Jesper
> Signed-off-by: Serge Semin
>
> ---
>
> Changelog v4:
> - This is a new patch created on the v4 lap of the series.
> ---
> drivers/pci/controller/dwc/pci-dra7xx.c | 12 +++
> drivers/pci/controller/dwc/pci-exynos.c | 6 ++--
> drivers/pci/controller/dwc/pci-imx6.c | 6 ++--
> drivers/pci/controller/dwc/pci-keystone.c | 20 +--
> drivers/pci/controller/dwc/pci-layerscape.c | 2 +-
> drivers/pci/controller/dwc/pci-meson.c| 2 +-
> drivers/pci/controller/dwc/pcie-al.c | 6 ++--
> drivers/pci/controller/dwc/pcie-armada8k.c| 4 +--
> drivers/pci/controller/dwc/pcie-artpec6.c | 4 +--
> .../pci/controller/dwc/pcie-designware-host.c | 36 +--
> .../pci/controller/dwc/pcie-designware-plat.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.h | 30
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 4 +--
> drivers/pci/controller/dwc/pcie-fu740.c | 2 +-
> drivers/pci/controller/dwc/pcie-histb.c | 10 +++---
> drivers/pci/controller/dwc/pcie-intel-gw.c| 6 ++--
> drivers/pci/controller/dwc/pcie-keembay.c | 4 +--
> drivers/pci/controller/dwc/pcie-kirin.c | 2 +-
> drivers/pci/controller/dwc/pcie-qcom.c| 4 +--
> drivers/pci/controller/dwc/pcie-spear13xx.c | 6 ++--
> drivers/pci/controller/dwc/pcie-tegra194.c| 22 ++--
> drivers/pci/controller/dwc/pcie-uniphier.c| 10 +++---
> drivers/pci/controller/dwc/pcie-visconti.c| 6 ++--
> 23 files changed, 103 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c
> b/drivers/pci/controller/dwc/pci-dra7xx.c
> index dfcdeb432dc8..a174b680b2a7 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -178,7 +178,7 @@ static void dra7xx_pcie_enable_interrupts(struct
> dra7xx_pcie *dra7xx)
> dra7xx_pcie_enable_msi_interrupts(dra7xx);
> }
>
> -static int dra7xx_pcie_host_init(struct pcie_port *pp)
> +static int dra7xx_pcie_host_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
> @@ -202,7 +202,7 @@ static const struct irq_domain_ops intx_domain_ops = {
> .xlate = pci_irqd_intx_xlate,
> };
>
> -static int dra7xx_pcie_handle_msi(struct pcie_port *pp, int index)
> +static int dra7xx_pcie_handle_msi(struct dw_pcie_rp *pp, int index)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> unsigned long val;
> @@ -224,7 +224,7 @@ static int dra7xx_pcie_handle_msi(struct pcie_port *pp,
> int index)
> return 1;
> }
>
> -static void dra7xx_pcie_handle_msi_irq(struct pcie_port *pp)
> +static void dra7xx_pcie_handle_msi_irq(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> int ret, i, count, num_ctrls;
> @@ -255,8 +255,8 @@ static void dra7xx_pcie_msi_irq_handler(struct irq_desc
> *desc)
> {
> struct irq_chip *chip = irq_desc_get_chip(desc);
> struct dra7xx_pcie *dra7xx;
> + struct dw_pcie_rp *pp;
> struct dw_pcie *pci;
> - struct pcie_port *pp;
> unsigned long reg;
> u32 bit;
>
> @@ -344,7 +344,7 @@ static irqreturn_t dra7xx_pcie_irq_handler(int irq, void
> *arg)
> return IRQ_HANDLED;
> }
>
> -static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp)
> +static int dra7xx_pcie_init_irq_domain(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct device *dev = pci->dev
Re: [PATCH 24/36] printk: Remove trace_.*_rcuidle() usage
il.com, vgu...@kernel.org, linux-...@vger.kernel.org, mon...@monstr.eu, rost...@goodmis.org, r...@vger.kernel.org, b...@alien8.de, bc...@quicinc.com, tsbog...@alpha.franken.de, linux-par...@vger.kernel.org, sudeep.ho...@arm.com, shawn...@kernel.org, da...@davemloft.net, dal...@libc.org, Peter Zijlstra , amakha...@vmware.com, bjorn.anders...@linaro.org, h...@zytor.com, sparcli...@vger.kernel.org, linux-hexa...@vger.kernel.org, linux-ri...@lists.infradead.org, anton.iva...@cambridgegreys.com, jo...@southpole.se, Arnd Bergmann , rich...@nod.at, x...@kernel.org, li...@armlinux.org.uk, mi...@redhat.com, a...@eecs.berkeley.edu, paul...@kernel.org, h...@linux.ibm.com, stefan.kristians...@saunalahti.fi, openr...@lists.librecores.org, paul.walms...@sifive.com, linux-te...@vger.kernel.org, namhy...@kernel.org, andriy.shevche...@linux.intel.com, jpoim...@kernel.org, jgr...@suse.com, pv-driv...@vmware.com, linux-m...@vger.kernel.org, pal...@dabbelt.com, anup@brainfa ult.org, i...@jurassic.park.msu.ru, johan...@sipsolutions.net, linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org Sender: "Linuxppc-dev" On Thu, Jun 9, 2022 at 10:06 PM Petr Mladek wrote: > > Makes sense. Feel free to use for this patch: > > Acked-by: Petr Mladek Reviewed-by: Sergey Senozhatsky
Re: [PATCH 24/36] printk: Remove trace_.*_rcuidle() usage
il.com, vgu...@kernel.org, linux-...@vger.kernel.org, mon...@monstr.eu, rost...@goodmis.org, r...@vger.kernel.org, b...@alien8.de, bc...@quicinc.com, tsbog...@alpha.franken.de, linux-par...@vger.kernel.org, sudeep.ho...@arm.com, shawn...@kernel.org, da...@davemloft.net, dal...@libc.org, Peter Zijlstra , amakha...@vmware.com, bjorn.anders...@linaro.org, h...@zytor.com, sparcli...@vger.kernel.org, linux-hexa...@vger.kernel.org, linux-ri...@lists.infradead.org, anton.iva...@cambridgegreys.com, jo...@southpole.se, Arnd Bergmann , rich...@nod.at, x...@kernel.org, li...@armlinux.org.uk, mi...@redhat.com, a...@eecs.berkeley.edu, paul...@kernel.org, h...@linux.ibm.com, stefan.kristians...@saunalahti.fi, openr...@lists.librecores.org, paul.walms...@sifive.com, linux-te...@vger.kernel.org, namhy...@kernel.org, andriy.shevche...@linux.intel.com, jpoim...@kernel.org, jgr...@suse.com, pv-driv...@vmware.com, linux-m...@vger.kernel.org, pal...@dabbelt.com, anup@brainfa ult.org, i...@jurassic.park.msu.ru, johan...@sipsolutions.net, linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org Sender: "Linuxppc-dev" On Thu, Jun 9, 2022 at 10:02 PM Petr Mladek wrote: > > On Thu 2022-06-09 20:30:58, Sergey Senozhatsky wrote: > > My emails are getting rejected... Let me try web-interface > > Bad day for mail sending. I have problems as well ;-) For me the problem is still there and apparently it's an "too many recipients" error. > > I'm somewhat curious whether we can actually remove that trace event. > > Good question. > > Well, I think that it might be useful. It allows to see trace and > printk messages together. Fair enough. Seems that back in 2011 people were pretty happy with it https://lore.kernel.org/all/1322161388.5366.54.ca...@jlt3.sipsolutions.net/T/#m7bf6416f469119372191f22a6ecf653c5f7331d2 but... reportedly, one of the folks who Ack-ed it (*cough cough* PeterZ) has never used it. > It was ugly when it was in the console code. The new location > in vprintk_store() allows to have it even "correctly" sorted > (timestamp) against other tracing messages. That's true.
[PATCH] soc: fsl: qe: Check of ioremap return value in qe_reset
As the possible failure of the ioremap(), the qe_immr could be NULL.
Therefore it should be better to check it and print error message.
Signed-off-by: Li Qiong
---
drivers/soc/fsl/qe/qe.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index b3c226eb5292..3c0948c2440e 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -88,6 +88,11 @@ void qe_reset(void)
if (qe_immr == NULL)
qe_immr = ioremap(get_qe_base(), QE_IMMAP_SIZE);
+ if (!qe_immr) {
+ pr_err("%s: failed to ioremap()\n", __func__);
+ return;
+ }
+
qe_snums_init();
qe_issue_cmd(QE_RESET, QE_CR_SUBBLOCK_INVALID,
--
2.11.0
Re: [PATCH] kprobes: Enable tracing for mololithic kernel images
, Will Deacon , Masahiro Yamada , Jarkko Sakkinen , Sami Tolvanen , "Naveen N. Rao" , Marco Elver , Kees Cook , Steven Rostedt , Nathan Chancellor , "Russell King \(Oracle\)" , Mark Brown , Borislav Petkov , Alexander Egorenkov , Thomas Bogendoerfer , Parisc List , Nathaniel McCallum , Dmitry Torokhov , "David S. Miller" , "Kirill A. Shutemov" , Tobias Huschle , "Peter Zijlstra \(Intel\)" , "H. Peter Anvin" , sparclinux , Tiezhu Yang , Miroslav Benes , Chen Zhongjin , linux-riscv , the arch/x86 maintainers , Russell King , Ingo Molnar , Aaron Tomlin , Albert Ou , Heiko Carstens , Liao Chang , Paul Walmsley , Josh Poimboeuf , Thomas Richter , "open list:BROADCOM NVRAM DRIVER" , Changbin Du , Palmer Dabbelt , linuxppc-dev , linux-modu...@vger.kernel.org Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org Sender: "Linuxppc-dev" On Thu, 9 Jun 2022 15:23:16 +0200 Ard Biesheuvel wrote: > On Thu, 9 Jun 2022 at 15:14, Jarkko Sakkinen wrote: > > > > On Wed, Jun 08, 2022 at 09:12:34AM -0700, Song Liu wrote: > > > On Wed, Jun 8, 2022 at 7:21 AM Masami Hiramatsu > > > wrote: > > > > > > > > Hi Jarkko, > > > > > > > > On Wed, 8 Jun 2022 08:25:38 +0300 > > > > Jarkko Sakkinen wrote: > > > > > > > > > On Wed, Jun 08, 2022 at 10:35:42AM +0800, Guo Ren wrote: > > > > > > . > > > > > > > > > > > > On Wed, Jun 8, 2022 at 8:02 AM Jarkko Sakkinen > > > > > > wrote: > > > > > > > > > > > > > > Tracing with kprobes while running a monolithic kernel is > > > > > > > currently > > > > > > > impossible because CONFIG_KPROBES is dependent of CONFIG_MODULES. > > > > > > > This > > > > > > > dependency is a result of kprobes code using the module allocator > > > > > > > for the > > > > > > > trampoline code. > > > > > > > > > > > > > > Detaching kprobes from modules helps to squeeze down the user > > > > > > > space, > > > > > > > e.g. when developing new core kernel features, while still having > > > > > > > all > > > > > > > the nice tracing capabilities. > > > > > > > > > > > > > > For kernel/ and arch/*, move module_alloc() and module_memfree() > > > > > > > to > > > > > > > module_alloc.c, and compile as part of vmlinux when either > > > > > > > CONFIG_MODULES > > > > > > > or CONFIG_KPROBES is enabled. In addition, flag kernel module > > > > > > > specific > > > > > > > code with CONFIG_MODULES. > > > > > > > > > > > > > > As the result, kprobes can be used with a monolithic kernel. > > > > > > It's strange when MODULES is n, but vmlinux still obtains > > > > > > module_alloc. > > > > > > > > > > > > Maybe we need a kprobe_alloc, right? > > > > > > > > > > Perhaps not the best name but at least it documents the fact that > > > > > they use the same allocator. > > > > > > > > > > Few years ago I carved up something "half-way there" for kprobes, > > > > > and I used the name text_alloc() [*]. > > > > > > > > > > [*] > > > > > https://lore.kernel.org/all/20200724050553.1724168-1-jarkko.sakki...@linux.intel.com/ > > > > > > > > Yeah, I remember that. Thank you for updating your patch! > > > > I think the idea (split module_alloc() from CONFIG_MODULE) is good to > > > > me. > > > > If module support maintainers think this name is not good, you may be > > > > able to rename it as text_alloc() and make the module_alloc() as a > > > > wrapper of it. > > > > > > IIUC, most users of module_alloc() use it to allocate memory for text, > > > except > > > that module code uses it for both text and data. Therefore, I guess > > > calling it > > > text_alloc() is not 100% accurate until we change the module code (to use > > > a different API to allocate memory for data). > > > > After reading the feedback, I'd stay on using module_alloc() because > > it has arch-specific quirks baked in. Easier to deal with them in one > > place. > > > > In that case, please ensure that you enable this only on architectures > where it is needed. arm64 implements alloc_insn_page() without relying > on module_alloc() so I would not expect to see any changes there. Hmm, what about adding CONFIG_ARCH_HAVE_ALLOC_INSN_PAGE and check it? If it is defined, kprobes will not define the __weak function, but if not, it will use module_alloc()? Thank you, -- Masami Hiramatsu (Google)
Re: [PATCH] kprobes: Enable tracing for mololithic kernel images
hiro Yamada , Jarkko Sakkinen , Sami Tolvanen , "Naveen N. Rao" , Marco Elver , Kees Cook , Steven Rostedt , Nathan Chancellor , "Russell King \(Oracle\)" , Mark Brown , Borislav Petkov , Alexander Egorenkov , Thomas Bogendoerfer , linux-par...@vger.kernel.org, Nathaniel McCallum , Dmitry Torokhov , "David S. Miller" , "Kirill A. Shutemov" , Tobias Huschle , "Peter Zijlstra \(Intel\)" , "H. Peter Anvin" , sparcli...@vger.kernel.org, Tiezhu Yang , Miroslav Benes , Chen Zhongjin , Ard Biesheuvel , X86 ML , Russell King , linux-ri...@lists.infradead.org, Ingo Molnar , Aaron Tomlin , Albert Ou , Heiko Carstens , Liao Chang , Paul Walmsley , Josh Poimboeuf , Thomas Richter , linux-m...@vger.kernel.org, Changbin Du , Palmer Dabbelt , linuxppc-dev@lists.ozlabs.org, linux-modu...@vger.kernel.org Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org Sender: "Linuxppc-dev" On Wed, 8 Jun 2022 11:19:19 -0700 Song Liu wrote: > On Wed, Jun 8, 2022 at 9:28 AM Ard Biesheuvel wrote: > > > > Hello Jarkko, > > > > On Wed, 8 Jun 2022 at 02:02, Jarkko Sakkinen wrote: > > > > > > Tracing with kprobes while running a monolithic kernel is currently > > > impossible because CONFIG_KPROBES is dependent of CONFIG_MODULES. This > > > dependency is a result of kprobes code using the module allocator for the > > > trampoline code. > > > > > > Detaching kprobes from modules helps to squeeze down the user space, > > > e.g. when developing new core kernel features, while still having all > > > the nice tracing capabilities. > > > > > > For kernel/ and arch/*, move module_alloc() and module_memfree() to > > > module_alloc.c, and compile as part of vmlinux when either CONFIG_MODULES > > > or CONFIG_KPROBES is enabled. In addition, flag kernel module specific > > > code with CONFIG_MODULES. > > > > > > As the result, kprobes can be used with a monolithic kernel. > > > > I think I may have mentioned this the previous time as well, but I > > don't think this is the right approach. > > > > Kprobes uses alloc_insn_page() to allocate executable memory, but the > > requirements for this memory are radically different compared to > > loadable modules, which need to be within an arch-specific distance of > > the core kernel, need KASAN backing etc etc. > > I think the distance of core kernel requirement is the same for kprobe > alloc_insn_page and modules, no? This strongly depends on how kprobes (software breakpoint and single-step) is implemented on the arch. For example, x86 implements the so-called "kprobe-booster" which jumps back from the single stepping trampoline buffer. Then the buffer address must be within the range where it can jump to the original address. However, if the arch implements single-step as an instruction emulation, it has no such limitation. As far as I know, arm64 will do emulation for the instructions which change PC register and will do direct execution with another software breakpoint for other instructions. Why I'm using module_alloc() for a generic function, is that can cover the limitation most widely. Thus, if we have CONFIG_ARCH_HAVE_ALLOC_INSN_PAGE flag and kprobes can check it instead of using __weak function, the kprobes may not need to depend on module_alloc() in general. Thank you, > > Thanks, > Song > > > > > This is why arm64, for instance, does not implement alloc_insn_page() > > in terms of module_alloc() [and likely does not belong in this patch > > for that reason] > > > > > > > Is there any reason kprobes cannot simply use vmalloc()? > > -- Masami Hiramatsu (Google)
Re: [PATCH] kprobes: Enable tracing for mololithic kernel images
���zZ+�Ƭji֦jơ��2���zZ+JjI"���j�$���~&�r��jh��[ڝ��jh��[ڝ�࢈%y�&5��zsQj�ڽ秞���)��&�r��j�([ޭ�oz�(�
^r��y���$y� �� ��&�騮�z�F�-y�k��^v�(�٢���5�ajp�jw �Z+��ajy �w����� �R���i�^�i>���zV��X��+��
j���'n�(�'�z�ޖ����V�=�d���jX���^W�jw^�H(�j�b��bng(�8h����v��}��
zwZ��Z~�x�� ��e�{��z� �w���
j�Z�'�1���[���ajx~&�r���+k�:+�Hh�٢�$��r��j�Hȥ���j�u�ޚZ �w�**�P��^��䊸���n�騾X�����\�d�n&� � �W���!��b��bng(��z�b�[-��'��iz�)߭�^i�+�s��+rܖ)���Z��b��o���z�ޖ��N'����
�� b{8n��'��'rs"��%j�^��&m�ˬy�
�ن��)܅���)Ṭ ��&�)�Ƹ�r�b��k��/�+-�)߭�^i�+��^j�!� :��z�1� �w����� �R��
b��Z�ib��h� �"x(2�gj���
+y�Z��&��:&�)ڶ���z�v�r��뚢�r�ޮG�{'��w��^��\jX�����&.&�
��X���Z�
a�� ��&=��Y�f�W��f�W��'��(��,��"�� ���)��� �w������j�re�{�j)zyb��Q8 �8�UDD�DFX��z� �w������n)ù�Z��ۢ�ץr��jY��6�m�m��fz�Zm&�)�ƚ\u��{��� z�b��(�V)��j �W��zZ+�X���v�^�� �G���h�+���N�b��i��^��.�Ǭ�܆+�
Re: [PATCH] kprobes: Enable tracing for mololithic kernel images
el.org>, Masahiro Yamada , Jarkko Sakkinen
, Sami Tolvanen , "Naveen N. Rao"
, Marco Elver , Kees Cook
, Steven Rostedt , Nathan
Chancellor , "Russell King \(Oracle\)"
, Mark Brown , Borislav Petkov
, Alexander Egorenkov , Thomas
Bogendoerfer , Parisc List
, Nathaniel McCallum ,
Dmitry Torokhov , "David S. Miller"
, "Kirill A. Shutemov" ,
Tobias Huschle , "Peter Zijlstra \(Intel\)"
, "H. Peter Anvin" , sparclinux
, Tiezhu Yang , Miroslav
Benes , Chen Zhongjin , Ard Biesheuvel ,
>the arch/x86 maintainers , Russell King
>, linux-riscv , Ingo
>Molnar , Aaron Tomlin , Albert Ou
>, Heiko Carstens , Liao Chang
>, Paul Walmsley , Josh
>Poimboeuf , Thomas Richter ,
>"open list:BROADCOM NVRAM DRIVER" , Changbin Du
>, Palmer Dabbelt , linuxppc-dev
>, "linux-modu...@vger.kernel.org"
>
Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org
Sender: "Linuxppc-dev"
On Sun, 12 Jun 2022 15:59:29 +
Christophe Leroy wrote:
>
>
> Le 12/06/2022 à 14:18, Masami Hiramatsu (Google) a écrit :
> > [You don't often get email from mhira...@kernel.org. Learn why this is
> > important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > On Thu, 9 Jun 2022 15:23:16 +0200
> > Ard Biesheuvel wrote:
> >
> >> On Thu, 9 Jun 2022 at 15:14, Jarkko Sakkinen wrote:
> >>>
> >>> On Wed, Jun 08, 2022 at 09:12:34AM -0700, Song Liu wrote:
> On Wed, Jun 8, 2022 at 7:21 AM Masami Hiramatsu
> wrote:
> >
> > Hi Jarkko,
> >
> > On Wed, 8 Jun 2022 08:25:38 +0300
> > Jarkko Sakkinen wrote:
> >
> >> On Wed, Jun 08, 2022 at 10:35:42AM +0800, Guo Ren wrote:
> >>> .
> >>>
> >>> On Wed, Jun 8, 2022 at 8:02 AM Jarkko Sakkinen
> >>> wrote:
>
> Tracing with kprobes while running a monolithic kernel is currently
> impossible because CONFIG_KPROBES is dependent of CONFIG_MODULES.
> This
> dependency is a result of kprobes code using the module allocator
> for the
> trampoline code.
>
> Detaching kprobes from modules helps to squeeze down the user space,
> e.g. when developing new core kernel features, while still having all
> the nice tracing capabilities.
>
> For kernel/ and arch/*, move module_alloc() and module_memfree() to
> module_alloc.c, and compile as part of vmlinux when either
> CONFIG_MODULES
> or CONFIG_KPROBES is enabled. In addition, flag kernel module
> specific
> code with CONFIG_MODULES.
>
> As the result, kprobes can be used with a monolithic kernel.
> >>> It's strange when MODULES is n, but vmlinux still obtains
> >>> module_alloc.
> >>>
> >>> Maybe we need a kprobe_alloc, right?
> >>
> >> Perhaps not the best name but at least it documents the fact that
> >> they use the same allocator.
> >>
> >> Few years ago I carved up something "half-way there" for kprobes,
> >> and I used the name text_alloc() [*].
> >>
> >> [*]
> >> https://lore.kernel.org/all/20200724050553.1724168-1-jarkko.sakki...@linux.intel.com/
> >
> > Yeah, I remember that. Thank you for updating your patch!
> > I think the idea (split module_alloc() from CONFIG_MODULE) is good to
> > me.
> > If module support maintainers think this name is not good, you may be
> > able to rename it as text_alloc() and make the module_alloc() as a
> > wrapper of it.
>
> IIUC, most users of module_alloc() use it to allocate memory for text,
> except
> that module code uses it for both text and data. Therefore, I guess
> calling it
> text_alloc() is not 100% accurate until we change the module code (to use
> a different API to allocate memory for data).
> >>>
> >>> After reading the feedback, I'd stay on using module_alloc() because
> >>> it has arch-specific quirks baked in. Easier to deal with them in one
> >>> place.
> >>>
> >>
> >> In that case, please ensure that you enable this only on architectures
> >> where it is needed. arm64 implements alloc_insn_page() without relying
> >> on module_alloc() so I would not expect to see any changes there.
> >
> > Hmm, what about adding CONFIG_ARCH_HAVE_ALLOC_INSN_PAGE and check it?
> > If it is defined, kprobes will not define the __weak function, but
> > if not, it will use module_alloc()?
> >
>
> I'm not sure I understand. What's the problem with the __weak function
> here ?
>
> If we don't define the __weak alloc_insn_page() when arch has
> CONFIG_ARCH_HAVE_ALLOC_INSN_PAGE, then what's the point in making it weak ?
>
> powerpc has it's own alloc_insn_page(), but calls module_alloc(). So how
> will it work ?
Good point! In that case, it will need to separate the module_alloc()
from kmodule support even without the __weak.
Thank you,
>
> void *alloc_insn_page(void)
> {
> void *page;
>
> page = module_a
Re: [PATCH 21/36] x86/tdx: Remove TDX_HCALL_ISSUE_STI
kernel.org, VMware Inc , linux-snps-...@lists.infradead.org, mgor...@suse.de, jacob.jun@linux.intel.com, Arnd Bergmann , ulli.kr...@googlemail.com, vgu...@kernel.org, linux-...@vger.kernel.org, j...@joshtriplett.org, rost...@goodmis.org, r...@vger.kernel.org, Borislav Petkov , bc...@quicinc.com, tsbog...@alpha.franken.de, linux-par...@vger.kernel.org, sudeep.ho...@arm.com, shawn...@kernel.org, da...@davemloft.net, kirill.shute...@linux.intel.com, dal...@libc.org, t...@atomide.com, amakha...@vmware.com, bjorn.anders...@linaro.org, "H. Peter Anvin" , sparcli...@vger.kernel.org, linux-hexa...@vger.kernel.org, linux-ri...@lists.infradead.org, Isaku Yamahata , anton.iva...@cambridgegreys.com, jo...@southpole.se, yury.no...@gmail.com, Richard Weinberger , X86 ML , li...@armlinux.org.uk, Ingo Molnar , Albert Ou , paulmck@ker nel.org, h...@linux.ibm.com, stefan.kristians...@saunalahti.fi, openr...@lists.librecores.org, Paul Walmsley , linux-te...@vger.kernel.org, Namhyung Kim , Andy Shevchenko , jpoim...@kernel.org, Juergen Gross , mon...@monstr.eu, linux-m...@vger.kernel.org, Palmer Dabbelt , Anup Patel , i...@jurassic.park.msu.ru, Johannes Berg , linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org Sender: "Linuxppc-dev" On Wed, Jun 8, 2022 at 10:48 PM Peter Zijlstra wrote: > > Now that arch_cpu_idle() is expected to return with IRQs disabled, > avoid the useless STI/CLI dance. > > Per the specs this is supposed to work, but nobody has yet relied up > this behaviour so broken implementations are possible. I'm totally newbie here. The point of safe_halt() is that STI must be used and be used directly before HLT to enable IRQ during the halting and stop the halting if there is any IRQ. In TDX case, STI must be used directly before the hypercall. Otherwise, no IRQ can come and the vcpu would be stalled forever. Although the hypercall has an "irq_disabled" argument. But the hypervisor doesn't (and can't) touch the IRQ flags no matter what the "irq_disabled" argument is. The IRQ is not enabled during the halting if the IRQ is disabled before the hypercall even if irq_disabled=false. The "irq_disabled" argument is used for workaround purposes: https://lore.kernel.org/kvm/c020ee0b90c424a7010e979c9b32a28e9c488a51.1651774251.git.isaku.yamah...@intel.com/ Hope my immature/incorrect reply elicits a real response from others. Thanks Lai
Re: [PATCH 21/36] x86/tdx: Remove TDX_HCALL_ISSUE_STI
kernel.org, VMware Inc , linux-snps-...@lists.infradead.org, mgor...@suse.de, jacob.jun@linux.intel.com, Arnd Bergmann , ulli.kr...@googlemail.com, vgu...@kernel.org, linux-...@vger.kernel.org, j...@joshtriplett.org, rost...@goodmis.org, r...@vger.kernel.org, Borislav Petkov , bc...@quicinc.com, tsbog...@alpha.franken.de, linux-par...@vger.kernel.org, sudeep.ho...@arm.com, shawn...@kernel.org, da...@davemloft.net, kirill.shute...@linux.intel.com, dal...@libc.org, t...@atomide.com, amakha...@vmware.com, bjorn.anders...@linaro.org, "H. Peter Anvin" , sparcli...@vger.kernel.org, linux-hexa...@vger.kernel.org, linux-ri...@lists.infradead.org, Isaku Yamahata , anton.iva...@cambridgegreys.com, jo...@southpole.se, yury.no...@gmail.com, Richard Weinberger , X86 ML , li...@armlinux.org.uk, Ingo Molnar , Albert Ou , paulmck@ker nel.org, h...@linux.ibm.com, stefan.kristians...@saunalahti.fi, openr...@lists.librecores.org, Paul Walmsley , linux-te...@vger.kernel.org, Namhyung Kim , Andy Shevchenko , jpoim...@kernel.org, Juergen Gross , mon...@monstr.eu, linux-m...@vger.kernel.org, Palmer Dabbelt , Anup Patel , i...@jurassic.park.msu.ru, Johannes Berg , linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org Sender: "Linuxppc-dev" On Mon, Jun 13, 2022 at 04:26:01PM +0800, Lai Jiangshan wrote: > On Wed, Jun 8, 2022 at 10:48 PM Peter Zijlstra wrote: > > > > Now that arch_cpu_idle() is expected to return with IRQs disabled, > > avoid the useless STI/CLI dance. > > > > Per the specs this is supposed to work, but nobody has yet relied up > > this behaviour so broken implementations are possible. > > I'm totally newbie here. > > The point of safe_halt() is that STI must be used and be used > directly before HLT to enable IRQ during the halting and stop > the halting if there is any IRQ. Correct; on real hardware. But this is virt... > In TDX case, STI must be used directly before the hypercall. > Otherwise, no IRQ can come and the vcpu would be stalled forever. > > Although the hypercall has an "irq_disabled" argument. > But the hypervisor doesn't (and can't) touch the IRQ flags no matter > what the "irq_disabled" argument is. The IRQ is not enabled during > the halting if the IRQ is disabled before the hypercall even if > irq_disabled=false. All we need the VMM to do is wake the vCPU, and it can do that, irrespective of the guest's IF. So the VMM can (and does) know if there's an interrupt pending, and that's all that's needed to wake from this hypercall. Once the vCPU is back up and running again, we'll eventually set IF again and the pending interrupt will get delivered and all's well. Think of this like MWAIT with ECX[0] set if you will.
Re: [PATCH 04/36] cpuidle,intel_idle: Fix CPUIDLE_FLAG_IRQ_ENABLE
, ulli.kr...@googlemail.com, vgu...@kernel.org, linux-...@vger.kernel.org,
j...@joshtriplett.org, rost...@goodmis.org, r...@vger.kernel.org,
b...@alien8.de, bc...@quicinc.com, tsbog...@alpha.franken.de,
linux-par...@vger.kernel.org, sudeep.ho...@arm.com, shawn...@kernel.org,
da...@davemloft.net, dal...@libc.org, t...@atomide.com, amakha...@vmware.com,
bjorn.anders...@linaro.org, h...@zytor.com, sparcli...@vger.kernel.org,
linux-hexa...@vger.kernel.org, linux-ri...@lists.infradead.org,
anton.iva...@cambridgegreys.com, jo...@southpole.se, yury.no...@gmail.com,
rich...@nod.at, x...@kernel.org, li...@armlinux.org.uk, mi...@redhat.com,
a...@eecs.berkeley.edu, paul...@kernel.org, h...@linux.ibm.com,
stefan.kristians...@saunalahti.fi, openr...@lists.librecores.org,
paul.walms...@sifive.com, linux-te...@vger.kernel.org, namhy...@kernel.org,
andriy.shevche...@linux.intel.com, jpoim...@kernel.org, jgr...@suse.com,
mon...@monstr.eu, linux-m...@vger.kernel.org, pal...@dabbelt.com,
a...@brainfault.org,
i...@jurassic.park.msu.ru, johan...@sipsolutions.net,
linuxppc-dev@lists.ozlabs.org
Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org
Sender: "Linuxppc-dev"
On Thu, Jun 09, 2022 at 04:49:21PM -0700, Jacob Pan wrote:
> Hi Peter,
>
> On Wed, 08 Jun 2022 16:27:27 +0200, Peter Zijlstra
> wrote:
>
> > Commit c227233ad64c ("intel_idle: enable interrupts before C1 on
> > Xeons") wrecked intel_idle in two ways:
> >
> > - must not have tracing in idle functions
> > - must return with IRQs disabled
> >
> > Additionally, it added a branch for no good reason.
> >
> > Fixes: c227233ad64c ("intel_idle: enable interrupts before C1 on Xeons")
> > Signed-off-by: Peter Zijlstra (Intel)
> > ---
> > drivers/idle/intel_idle.c | 48
> > +++--- 1 file changed, 37
> > insertions(+), 11 deletions(-)
> >
> > --- a/drivers/idle/intel_idle.c
> > +++ b/drivers/idle/intel_idle.c
> > @@ -129,21 +137,37 @@ static unsigned int mwait_substates __in
> > *
> > * Must be called under local_irq_disable().
> > */
> nit: this comment is no long true, right?
It still is, all the idle routines are called with interrupts disabled,
but must also exit with interrupts disabled.
If the idle method requires interrupts to be enabled, it must be sure to
disable them again before returning. Given all the RCU/tracing concerns
it must use raw_local_irq_*() for this though.
[PATCH] spi: mpc52xx-psc: Switch to using core message queue
We deprecated open coding of the transfer queue back in 2017 so it's high
time we finished up converting drivers to use the standard message queue
code. The mpc52xx-psc driver is fairly straightforward so convert to use
transfer_one_message(), it looks like the driver would be a good fit for
transfer_one() with a little bit of updating but this smaller change seems
safer.
The driver seems like a good candidate for transfer_one() but the chip
select function is actually doing rather more than just updating the chip
select and both transfer_one() and transfer_one_message() are current APIs
so leave that refactoring for another day, ideally by someone with the
hardware.
Signed-off-by: Mark Brown
---
drivers/spi/spi-mpc52xx-psc.c | 116 ++
1 file changed, 34 insertions(+), 82 deletions(-)
diff --git a/drivers/spi/spi-mpc52xx-psc.c b/drivers/spi/spi-mpc52xx-psc.c
index 7654736c2c0e..609311231e64 100644
--- a/drivers/spi/spi-mpc52xx-psc.c
+++ b/drivers/spi/spi-mpc52xx-psc.c
@@ -37,12 +37,6 @@ struct mpc52xx_psc_spi {
struct mpc52xx_psc_fifo __iomem *fifo;
unsigned int irq;
u8 bits_per_word;
- u8 busy;
-
- struct work_struct work;
-
- struct list_head queue;
- spinlock_t lock;
struct completion done;
};
@@ -198,69 +192,53 @@ static int mpc52xx_psc_spi_transfer_rxtx(struct
spi_device *spi,
return 0;
}
-static void mpc52xx_psc_spi_work(struct work_struct *work)
+int mpc52xx_psc_spi_transfer_one_message(struct spi_controller *ctlr,
+struct spi_message *m)
{
- struct mpc52xx_psc_spi *mps =
- container_of(work, struct mpc52xx_psc_spi, work);
-
- spin_lock_irq(&mps->lock);
- mps->busy = 1;
- while (!list_empty(&mps->queue)) {
- struct spi_message *m;
- struct spi_device *spi;
- struct spi_transfer *t = NULL;
- unsigned cs_change;
- int status;
-
- m = container_of(mps->queue.next, struct spi_message, queue);
- list_del_init(&m->queue);
- spin_unlock_irq(&mps->lock);
-
- spi = m->spi;
- cs_change = 1;
- status = 0;
- list_for_each_entry (t, &m->transfers, transfer_list) {
- if (t->bits_per_word || t->speed_hz) {
- status = mpc52xx_psc_spi_transfer_setup(spi, t);
- if (status < 0)
- break;
- }
-
- if (cs_change)
- mpc52xx_psc_spi_activate_cs(spi);
- cs_change = t->cs_change;
-
- status = mpc52xx_psc_spi_transfer_rxtx(spi, t);
- if (status)
+ struct spi_device *spi;
+ struct spi_transfer *t = NULL;
+ unsigned cs_change;
+ int status;
+
+ spi = m->spi;
+ cs_change = 1;
+ status = 0;
+ list_for_each_entry (t, &m->transfers, transfer_list) {
+ if (t->bits_per_word || t->speed_hz) {
+ status = mpc52xx_psc_spi_transfer_setup(spi, t);
+ if (status < 0)
break;
- m->actual_length += t->len;
+ }
- spi_transfer_delay_exec(t);
+ if (cs_change)
+ mpc52xx_psc_spi_activate_cs(spi);
+ cs_change = t->cs_change;
- if (cs_change)
- mpc52xx_psc_spi_deactivate_cs(spi);
- }
+ status = mpc52xx_psc_spi_transfer_rxtx(spi, t);
+ if (status)
+ break;
+ m->actual_length += t->len;
- m->status = status;
- if (m->complete)
- m->complete(m->context);
+ spi_transfer_delay_exec(t);
- if (status || !cs_change)
+ if (cs_change)
mpc52xx_psc_spi_deactivate_cs(spi);
+ }
- mpc52xx_psc_spi_transfer_setup(spi, NULL);
+ m->status = status;
+ if (status || !cs_change)
+ mpc52xx_psc_spi_deactivate_cs(spi);
- spin_lock_irq(&mps->lock);
- }
- mps->busy = 0;
- spin_unlock_irq(&mps->lock);
+ mpc52xx_psc_spi_transfer_setup(spi, NULL);
+
+ spi_finalize_current_message(ctlr);
+
+ return 0;
}
static int mpc52xx_psc_spi_setup(struct spi_device *spi)
{
- struct mpc52xx_psc_spi *mps = spi_master_get_devdata(spi->master);
struct mpc52xx_psc_spi_cs *cs = spi->controller_state;
- unsigned long flags;
if (spi->bits_per_word%8)
return -EINVAL;
@@ -275,28 +253,6 @@ static int mpc52xx_psc_spi_setup(struct spi_device *spi)
cs->bits_p
Re: [FSL P50x0] Keyboard and mouse don't work anymore after the devicetree updates for 5.19
On Thu, Jun 9, 2022 at 12:03 PM Christian Zigotzky
wrote:
>
> On 06 June 2022 at 07:06 pm, Rob Herring wrote:
> > On Mon, Jun 6, 2022 at 11:14 AM Christian Zigotzky
> > wrote:
> >> On 06 June 2022 at 04:58 pm, Rob Herring wrote:
> >>> On Fri, May 27, 2022 at 9:23 AM Rob Herring wrote:
> On Fri, May 27, 2022 at 3:33 AM Christian Zigotzky
> wrote:
> > On 27 May 2022 at 10:14 am, Prabhakar Mahadev Lad wrote:
> >> Hi,
> >>
> >>> -Original Message-
> >>> From: Christian Zigotzky
> >>>
> >>> On 27 May 2022 at 09:56 am, Prabhakar Mahadev Lad wrote:
> Hi,
>
> > -Original Message-
> > From: Christophe Leroy
> >>> [...]
> >>>
> Looks like the driver which you are using has not been converted to
> use
> >>> platform_get_irq(), could you please check that.
> Cheers,
> Prabhakar
> >>> Do you mean the mouse and keyboard driver?
> >>>
> >> No it could be your gpio/pinctrl driver assuming the keyboard/mouse
> >> are using GPIO's. If you are using interrupts then it might be some
> >> hierarchal irqc driver in drivers/irqchip/.
> >>
> >> Cheers,
> >> Prabhakar
> > Good to know. I only use unmodified drivers from the official Linux
> > kernel so it's not an issue of the Cyrus+ board.
> The issue is in drivers/usb/host/fsl-mph-dr-of.c which copies the
> resources to a child platform device. Can you try the following
> change:
>
> diff --git a/drivers/usb/host/fsl-mph-dr-of.c
> b/drivers/usb/host/fsl-mph-dr-of.c
> index 44a7e58a26e3..47d9b7be60da 100644
> --- a/drivers/usb/host/fsl-mph-dr-of.c
> +++ b/drivers/usb/host/fsl-mph-dr-of.c
> @@ -80,8 +80,6 @@ static struct platform_device
> *fsl_usb2_device_register(
> const char *name, int id)
> {
> struct platform_device *pdev;
> - const struct resource *res = ofdev->resource;
> - unsigned int num = ofdev->num_resources;
> int retval;
>
> pdev = platform_device_alloc(name, id);
> @@ -106,11 +104,7 @@ static struct platform_device
> *fsl_usb2_device_register(
> if (retval)
> goto error;
>
> - if (num) {
> - retval = platform_device_add_resources(pdev, res, num);
> - if (retval)
> - goto error;
> - }
> + pdev->dev.of_node = ofdev->dev.of_node;
> >>> >From the log, I think you also need to add this line:
> >>>
> >>> pdev->dev.of_node_reused = true;
> >>>
> retval = platform_device_add(pdev);
> if (retval)
> >> Hello Rob,
> >>
> >> Thanks a lot for your answer.
> >>
> >> Is the following patch correct?
> > Yes
> >
> >> --- a/drivers/usb/host/fsl-mph-dr-of.c2022-05-28 09:10:26.797688422
> >> +0200
> >> +++ b/drivers/usb/host/fsl-mph-dr-of.c2022-05-28 09:15:01.668594809
> >> +0200
> >> @@ -80,8 +80,6 @@ static struct platform_device *fsl_usb2_
> >>const char *name, int id)
> >>{
> >>struct platform_device *pdev;
> >> -const struct resource *res = ofdev->resource;
> >> -unsigned int num = ofdev->num_resources;
> >>int retval;
> >>
> >>pdev = platform_device_alloc(name, id);
> >> @@ -106,11 +104,7 @@ static struct platform_device *fsl_usb2_
> >>if (retval)
> >>goto error;
> >>
> >> -if (num) {
> >> -retval = platform_device_add_resources(pdev, res, num);
> >> -if (retval)
> >> -goto error;
> >> -}
> >> +pdev->dev.of_node = ofdev->dev.of_node;
> >> +pdev->dev.of_node_reused = true;
> >>
> >>retval = platform_device_add(pdev);
> >>if (retval)
> >>
> >> ---
> >>
> >> Thanks,
> >> Christian
> Hello Rob,
>
> I tested this patch today and unfortunately the issue still exists.
The log is the same?
Rob
Re: [RFC PATCH 6/6] pkeys: Change mm_pkey_free() to void
On Mon, Jun 13, 2022 at 09:17:06AM +, Christophe Leroy wrote:
>
>
> Le 11/06/2022 à 01:35, ira.we...@intel.com a écrit :
> > From: Ira Weiny
> >
> > Now that the pkey arch support is no longer checked in mm_pkey_free()
> > there is no reason to have it return int.
>
> Right, I see this is doing what I commented in previous patch.
Yes because it was suggested by Sohil I decided to make it a separate patch to
make the credit easier.
> > diff --git a/mm/mprotect.c b/mm/mprotect.c
> > index 41458e729c27..e872bdd2e228 100644
> > --- a/mm/mprotect.c
> > +++ b/mm/mprotect.c
> > @@ -809,8 +809,10 @@ SYSCALL_DEFINE1(pkey_free, int, pkey)
> > return ret;
> >
> > mmap_write_lock(current->mm);
> > - if (mm_pkey_is_allocated(current->mm, pkey))
> > - ret = mm_pkey_free(current->mm, pkey);
> > + if (mm_pkey_is_allocated(current->mm, pkey)) {
> > + mm_pkey_free(current->mm, pkey);
> > + ret = 0;
> > + }
>
> Or you could have ret = 0 by default and do
>
> if (mm_pkey_is_allocated(current->mm, pkey))
> mm_pkey_free(current->mm, pkey);
> else
> ret = -EINVAL;
Yes that fits the kernel style better.
Thanks for the review!
Ira
>
> > mmap_write_unlock(current->mm);
> >
> > /*
Re: [PATCH] powerpc: Restore CONFIG_DEBUG_INFO in defconfigs
On Sat, Jun 11, 2022 at 08:51:57AM +0200, Christophe Leroy wrote:
> Commit f9b3cd245784 ("Kconfig.debug: make DEBUG_INFO selectable from a
> choice") broke the selection of CONFIG_DEBUG_INFO by powerpc defconfigs.
>
> It is now necessary to select one of the three DEBUG_INFO_DWARF*
> options to get DEBUG_INFO enabled.
>
> Replace DEBUG_INFO=y by DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y in all
> defconfigs using the following command:
>
> sed -i s/DEBUG_INFO=y/DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y/g `git grep -l
> DEBUG_INFO arch/powerpc/configs/`
>
> Fixes: f9b3cd245784 ("Kconfig.debug: make DEBUG_INFO selectable from a
> choice")
> Cc: sta...@vger.kernel.org
> Cc: Kees Cook
> Signed-off-by: Christophe Leroy
Reviewed-by: Kees Cook
--
Kees Cook
Re: [PATCH 1/6] powerpc: Add ZERO_GPRS macros for register clears
On Sat, Jun 11, 2022 at 08:42:27AM +, Christophe Leroy wrote: > Le 10/06/2022 à 05:32, Rohan McLure a écrit : > > .macro ZERO_REGS start, end > > .Lreg=\start > > .rept (\end - \start + 1) > > li .Lreg, 0 > > .Lreg=.Lreg+1 > > .endr > > .endm > > I'd have a preference for using a verb, for instance ZEROISE_REGS or > CLEAR_REGS "Zero" is a verb as well (as well as a noun and an adjective) :-) Segher
Re: [PATCH v2 4/5] bpf ppc32: add support for BPF_ATOMIC bitwise operations
On 11/06/22 10:44 pm, Christophe Leroy wrote: Le 10/06/2022 à 17:55, Hari Bathini a écrit : Adding instructions for ppc32 for atomic_and atomic_or atomic_xor atomic_fetch_add atomic_fetch_and atomic_fetch_or atomic_fetch_xor Signed-off-by: Hari Bathini --- Changes in v2: * Used an additional register (BPF_REG_AX) - to avoid clobbering src_reg. - to keep the lwarx reservation as intended. - to avoid the odd switch/goto construct. Might be a stupid question as I don't know the internals of BPF: Are we sure BPF_REG_AX cannot be the src reg or the dst reg ? AFAICS, BPF_REG_AX wouldn't be used as src_reg or dst_reg unless this code is reused internally, by arch-specific code, for JIT'ing some other instruction(s) using BPF_REG_AX as either src or dst reg.. Thanks Hari
Re: [PATCH v2 5/5] bpf ppc32: Add instructions for atomic_[cmp]xchg
On 11/06/22 11:04 pm, Christophe Leroy wrote:
Le 10/06/2022 à 17:55, Hari Bathini a écrit :
This adds two atomic opcodes BPF_XCHG and BPF_CMPXCHG on ppc32, both
of which include the BPF_FETCH flag. The kernel's atomic_cmpxchg
operation fundamentally has 3 operands, but we only have two register
fields. Therefore the operand we compare against (the kernel's API
calls it 'old') is hard-coded to be BPF_REG_R0. Also, kernel's
atomic_cmpxchg returns the previous value at dst_reg + off. JIT the
same for BPF too with return value put in BPF_REG_0.
BPF_REG_R0 = atomic_cmpxchg(dst_reg + off, BPF_REG_R0, src_reg);
Signed-off-by: Hari Bathini
---
Changes in v2:
* Moved variable declaration to avoid late declaration error on
some compilers.
* Tried to make code readable and compact.
arch/powerpc/net/bpf_jit_comp32.c | 25 ++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit_comp32.c
b/arch/powerpc/net/bpf_jit_comp32.c
index 28dc6a1a8f2f..43f1c76d48ce 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -297,6 +297,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
struct codegen_context *
u32 ax_reg = bpf_to_ppc(BPF_REG_AX);
u32 tmp_reg = bpf_to_ppc(TMP_REG);
u32 size = BPF_SIZE(code);
+ u32 save_reg, ret_reg;
s16 off = insn[i].off;
s32 imm = insn[i].imm;
bool func_addr_fixed;
@@ -799,6 +800,9 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
struct codegen_context *
* BPF_STX ATOMIC (atomic ops)
*/
case BPF_STX | BPF_ATOMIC | BPF_W:
+ save_reg = _R0;
+ ret_reg = src_reg;
+
bpf_set_seen_register(ctx, tmp_reg);
bpf_set_seen_register(ctx, ax_reg);
@@ -829,6 +833,21 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
case BPF_XOR | BPF_FETCH:
EMIT(PPC_RAW_XOR(_R0, _R0, src_reg));
break;
+ case BPF_CMPXCHG:
+ /*
+* Return old value in BPF_REG_0 for BPF_CMPXCHG
&
+* in src_reg for other cases.
+*/
+ ret_reg = bpf_to_ppc(BPF_REG_0);
+
+ /* Compare with old value in BPF_REG_0 */
+ EMIT(PPC_RAW_CMPW(bpf_to_ppc(BPF_REG_0), _R0));
+ /* Don't set if different from old value */
+ PPC_BCC_SHORT(COND_NE, (ctx->idx + 3) * 4);
+ fallthrough;
+ case BPF_XCHG:
+ save_reg = src_reg;
I'm a bit lost, when save_reg is src_reg, don't we expect the upper part
(ie src_reg - 1) to be explicitely zeroised ?
For BPF_FETCH variants, old value is returned in src_reg (ret_reg).
In all such cases, higher 32-bit is zero'ed. But in case of BPF_CMPXCHG,
src_reg is untouched as BPF_REG_0 is used instead. So, higher 32-bit
remains untouched for that case alone..
+ break;
default:
pr_err_ratelimited("eBPF filter atomic op code %02x
(@%d) unsupported\n",
code, i);
@@ -836,15 +855,15 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
struct codegen_context *
}
/* store new value */
- EMIT(PPC_RAW_STWCX(_R0, tmp_reg, dst_reg));
+ EMIT(PPC_RAW_STWCX(save_reg, tmp_reg, dst_reg));
/* we're done if this succeeded */
PPC_BCC_SHORT(COND_NE, tmp_idx);
/* For the BPF_FETCH variant, get old data into src_reg
*/
With this commit, this comment is not true for BPF_CMPXCHG. So, this
comment should not be removed..
Thanks
Hari
Re: [PATCH v2 5/5] bpf ppc32: Add instructions for atomic_[cmp]xchg
On 14/06/22 12:41 am, Hari Bathini wrote:
On 11/06/22 11:04 pm, Christophe Leroy wrote:
Le 10/06/2022 à 17:55, Hari Bathini a écrit :
This adds two atomic opcodes BPF_XCHG and BPF_CMPXCHG on ppc32, both
of which include the BPF_FETCH flag. The kernel's atomic_cmpxchg
operation fundamentally has 3 operands, but we only have two register
fields. Therefore the operand we compare against (the kernel's API
calls it 'old') is hard-coded to be BPF_REG_R0. Also, kernel's
atomic_cmpxchg returns the previous value at dst_reg + off. JIT the
same for BPF too with return value put in BPF_REG_0.
BPF_REG_R0 = atomic_cmpxchg(dst_reg + off, BPF_REG_R0, src_reg);
Signed-off-by: Hari Bathini
---
Changes in v2:
* Moved variable declaration to avoid late declaration error on
some compilers.
* Tried to make code readable and compact.
arch/powerpc/net/bpf_jit_comp32.c | 25 ++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit_comp32.c
b/arch/powerpc/net/bpf_jit_comp32.c
index 28dc6a1a8f2f..43f1c76d48ce 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -297,6 +297,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32
*image, struct codegen_context *
u32 ax_reg = bpf_to_ppc(BPF_REG_AX);
u32 tmp_reg = bpf_to_ppc(TMP_REG);
u32 size = BPF_SIZE(code);
+ u32 save_reg, ret_reg;
s16 off = insn[i].off;
s32 imm = insn[i].imm;
bool func_addr_fixed;
@@ -799,6 +800,9 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32
*image, struct codegen_context *
* BPF_STX ATOMIC (atomic ops)
*/
case BPF_STX | BPF_ATOMIC | BPF_W:
+ save_reg = _R0;
+ ret_reg = src_reg;
+
bpf_set_seen_register(ctx, tmp_reg);
bpf_set_seen_register(ctx, ax_reg);
@@ -829,6 +833,21 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32
*image, struct codegen_context *
case BPF_XOR | BPF_FETCH:
EMIT(PPC_RAW_XOR(_R0, _R0, src_reg));
break;
+ case BPF_CMPXCHG:
+ /*
+ * Return old value in BPF_REG_0 for BPF_CMPXCHG &
+ * in src_reg for other cases.
+ */
+ ret_reg = bpf_to_ppc(BPF_REG_0);
+
+ /* Compare with old value in BPF_REG_0 */
+ EMIT(PPC_RAW_CMPW(bpf_to_ppc(BPF_REG_0), _R0));
+ /* Don't set if different from old value */
+ PPC_BCC_SHORT(COND_NE, (ctx->idx + 3) * 4);
+ fallthrough;
+ case BPF_XCHG:
+ save_reg = src_reg;
I'm a bit lost, when save_reg is src_reg, don't we expect the upper part
(ie src_reg - 1) to be explicitely zeroised ?
For BPF_FETCH variants, old value is returned in src_reg (ret_reg).
In all such cases, higher 32-bit is zero'ed. But in case of BPF_CMPXCHG,
src_reg is untouched as BPF_REG_0 is used instead. So, higher 32-bit
remains untouched for that case alone..
+ break;
default:
pr_err_ratelimited("eBPF filter atomic op code
%02x (@%d) unsupported\n",
code, i);
@@ -836,15 +855,15 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32
*image, struct codegen_context *
}
/* store new value */
- EMIT(PPC_RAW_STWCX(_R0, tmp_reg, dst_reg));
+ EMIT(PPC_RAW_STWCX(save_reg, tmp_reg, dst_reg));
/* we're done if this succeeded */
PPC_BCC_SHORT(COND_NE, tmp_idx);
/* For the BPF_FETCH variant, get old data into
src_reg */
With this commit, this comment is not true for BPF_CMPXCHG. So, this
comment should not be removed..
Sorry, the above should read:
"should be removed" instead of "should not be removed"..
Re: [PATCH v4 13/18] PCI: dwc: Add start_link/stop_link inliners
On Fri, Jun 10, 2022 at 11:25:29AM +0300, Serge Semin wrote: > There are several places in the generic DW PCIe code where the > platform-specific PCIe link start/stop methods are called after making > sure the ops handler and the callbacks are specified. Instead of repeating > the same pattern over and over let's define the static-inline methods in > the DW PCIe header file and use them in the relevant parts of the driver. > > Note returning a negative error from the EP link start procedure if the > start_link pointer isn't specified doesn't really make much sense since it > perfectly normal to have such platform. Moreover even pci_epc_start() > doesn't fail if no epc->ops->start callback is spotted. As a side-effect > of this modification we can set the generic DW PCIe and Layerscape EP > platform drivers free from the empty start_link callbacks and as such > entirely dummy dw_pcie_ops instances. > > Signed-off-by: Serge Semin > > --- > > Changelog v4: > - This is a new patch created on the v4 lap of the series. > --- > drivers/pci/controller/dwc/pci-layerscape-ep.c| 12 > drivers/pci/controller/dwc/pcie-designware-ep.c | 8 ++-- > drivers/pci/controller/dwc/pcie-designware-host.c | 10 -- > drivers/pci/controller/dwc/pcie-designware-plat.c | 10 -- > drivers/pci/controller/dwc/pcie-designware.h | 14 ++ > 5 files changed, 20 insertions(+), 34 deletions(-) Reviewed-by: Rob Herring
Re: [PATCH v4 15/18] PCI: dwc: Add dw_ prefix to the pcie_port structure name
o...@mm-sol.com>, Krzysztof Kozlowski , Masami
Hiramatsu , Pengutronix Kernel Team
, Gustavo Pimentel ,
Shawn Guo , Lucas Stach
Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org
Sender: "Linuxppc-dev"
On Fri, Jun 10, 2022 at 11:25:31AM +0300, Serge Semin wrote:
> All of the DW PCIe core driver entities have names with the dw_ prefix in
> order to easily distinguish local and common PCIe name spaces. All except
> the pcie_port structure which contains the DW PCIe Root Port descriptor.
> For historical reason the structure has retained the original name since
> commit 340cba6092c2 ("pci: Add PCIe driver for Samsung Exynos") when
> the DW PCIe IP-core support was added to the kernel. Let's finally fix
> that by adding the dw_ prefix to the structure name and by adding the _rp
> suffix to be similar to the EP counterpart. Thus the name will be coherent
> with the common driver naming policy. It shall make the driver code more
> readable eliminating visual confusion between the local and generic PCI
> name spaces.
>
> Signed-off-by: Serge Semin
>
> ---
>
> Changelog v4:
> - This is a new patch created on the v4 lap of the series.
> ---
> drivers/pci/controller/dwc/pci-dra7xx.c | 12 +++
> drivers/pci/controller/dwc/pci-exynos.c | 6 ++--
> drivers/pci/controller/dwc/pci-imx6.c | 6 ++--
> drivers/pci/controller/dwc/pci-keystone.c | 20 +--
> drivers/pci/controller/dwc/pci-layerscape.c | 2 +-
> drivers/pci/controller/dwc/pci-meson.c| 2 +-
> drivers/pci/controller/dwc/pcie-al.c | 6 ++--
> drivers/pci/controller/dwc/pcie-armada8k.c| 4 +--
> drivers/pci/controller/dwc/pcie-artpec6.c | 4 +--
> .../pci/controller/dwc/pcie-designware-host.c | 36 +--
> .../pci/controller/dwc/pcie-designware-plat.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.h | 30
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 4 +--
> drivers/pci/controller/dwc/pcie-fu740.c | 2 +-
> drivers/pci/controller/dwc/pcie-histb.c | 10 +++---
> drivers/pci/controller/dwc/pcie-intel-gw.c| 6 ++--
> drivers/pci/controller/dwc/pcie-keembay.c | 4 +--
> drivers/pci/controller/dwc/pcie-kirin.c | 2 +-
> drivers/pci/controller/dwc/pcie-qcom.c| 4 +--
> drivers/pci/controller/dwc/pcie-spear13xx.c | 6 ++--
> drivers/pci/controller/dwc/pcie-tegra194.c| 22 ++--
> drivers/pci/controller/dwc/pcie-uniphier.c| 10 +++---
> drivers/pci/controller/dwc/pcie-visconti.c| 6 ++--
> 23 files changed, 103 insertions(+), 103 deletions(-)
Reviewed-by: Rob Herring
Re: [RFC PATCH 1/6] testing/pkeys: Add command line options
On Mon, Jun 13, 2022 at 03:31:02PM -0700, Mehta, Sohil wrote: > On 6/10/2022 4:35 PM, ira.we...@intel.com wrote: > > > Add command line options for debug level and number of iterations. > > > > $ ./protection_keys_64 -h > > Usage: ./protection_keys_64 [-h,-d,-i ] > > --help,-h This help > > --debug,-d Increase debug level for each -d > > Is this mechanism (of counting d's) commonplace in other selftests as well? > Looking at the test code for pkeys the debug levels run from 1-5. That feels > like quite a few d's to input :) I've seen (and used) it before yes. See ibnetdiscover. ... # Debugging flags -d raise the IB debugging level. May be used several times (-ddd or -d -d -d). ... -v increase the application verbosity level. May be used several times (-vv or -v -v -v) ... - https://linux.die.net/man/8/ibnetdiscover But a much more mainstream example I can think of is verbosity level with lspci. 16:29:12 > lspci -h ... Display options: -v Be verbose (-vv or -vvv for higher verbosity) ... > > Would it be easier to input the number in the command line directly? > > Either way it would be useful to know the debug range in the help. > Maybe something like: > --debug,-d Increase debug level for each -d (1-5) I'm inclined not to do this because it would encode the max debug level. On the other hand I'm not sure why there are 5 levels now. ;-) Having the multiple options specified was an easy way to maintain the large number of levels. Ira > > The patch seems fine to me otherwise. > > > --iterations,-i repeate test times > > default: 22 > > > > Thanks, > Sohil
Re: [RFC PATCH 2/6] testing/pkeys: Don't use uninitialized variable
On Mon, Jun 13, 2022 at 03:48:56PM -0700, Mehta, Sohil wrote:
> On 6/10/2022 4:35 PM, ira.we...@intel.com wrote:
> > diff --git a/tools/testing/selftests/vm/protection_keys.c
> > b/tools/testing/selftests/vm/protection_keys.c
> > index d0183c381859..43e47de19c0d 100644
> > --- a/tools/testing/selftests/vm/protection_keys.c
> > +++ b/tools/testing/selftests/vm/protection_keys.c
> > @@ -1225,9 +1225,9 @@ void test_pkey_alloc_exhaust(int *ptr, u16 pkey)
> > int new_pkey;
> > dprintf1("%s() alloc loop: %d\n", __func__, i);
> > new_pkey = alloc_pkey();
> > - dprintf4("%s()::%d, err: %d pkey_reg: 0x%016llx"
> > + dprintf4("%s()::%d, errno: %d pkey_reg: 0x%016llx"
>
> What is errno referring to over here? There are a few things happening in
> alloc_pkey().
Good point, but the only system call in alloc_pkey() is pkey_alloc() so it will
be the errno from there.
In test_pkey_alloc_exhaust() we are expecting the errno to be from pkey_alloc()
...
if ((new_pkey == -1) && (errno == ENOSPC)) {
...
> I guess it would show the latest error that happened. Does
> errno need to be set to 0 before the call?
Maybe. Now that I look again errno is printed just below at level 2.
dprintf2("%s() errno: %d ENOSPC: %d\n", __func__, errno,
ENOSPC);
I missed that.
>
> Also, would it be useful to print the return value (new_pkey) from
> alloc_pkey() here?
Yea that might be useful. Perhaps change err to new_pkey instead since errno
is already printed.
Ira
>
> > " shadow: 0x%016llx\n",
> > - __func__, __LINE__, err, __read_pkey_reg(),
> > + __func__, __LINE__, errno, __read_pkey_reg(),
> > shadow_pkey_reg);
> > read_pkey_reg(); /* for shadow checking */
> > dprintf2("%s() errno: %d ENOSPC: %d\n", __func__, errno,
> > ENOSPC);
>
> Sohil
[PATCH v4.9] powerpc/32: Fix overread/overwrite of thread_struct via ptrace
commit 8e127846fc97778a5e5c99bca1ce0bbc5ec9 upstream.
The ptrace PEEKUSR/POKEUSR (aka PEEKUSER/POKEUSER) API allows a process
to read/write registers of another process.
To get/set a register, the API takes an index into an imaginary address
space called the "USER area", where the registers of the process are
laid out in some fashion.
The kernel then maps that index to a particular register in its own data
structures and gets/sets the value.
The API only allows a single machine-word to be read/written at a time.
So 4 bytes on 32-bit kernels and 8 bytes on 64-bit kernels.
The way floating point registers (FPRs) are addressed is somewhat
complicated, because double precision float values are 64-bit even on
32-bit CPUs. That means on 32-bit kernels each FPR occupies two
word-sized locations in the USER area. On 64-bit kernels each FPR
occupies one word-sized location in the USER area.
Internally the kernel stores the FPRs in an array of u64s, or if VSX is
enabled, an array of pairs of u64s where one half of each pair stores
the FPR. Which half of the pair stores the FPR depends on the kernel's
endianness.
To handle the different layouts of the FPRs depending on VSX/no-VSX and
big/little endian, the TS_FPR() macro was introduced.
Unfortunately the TS_FPR() macro does not take into account the fact
that the addressing of each FPR differs between 32-bit and 64-bit
kernels. It just takes the index into the "USER area" passed from
userspace and indexes into the fp_state.fpr array.
On 32-bit there are 64 indexes that address FPRs, but only 32 entries in
the fp_state.fpr array, meaning the user can read/write 256 bytes past
the end of the array. Because the fp_state sits in the middle of the
thread_struct there are various fields than can be overwritten,
including some pointers. As such it may be exploitable.
It has also been observed to cause systems to hang or otherwise
misbehave when using gdbserver, and is probably the root cause of this
report which could not be easily reproduced:
https://lore.kernel.org/linuxppc-dev/dc38afe9-6b78-f3f5-666b-986939e40...@keymile.com/
Rather than trying to make the TS_FPR() macro even more complicated to
fix the bug, or add more macros, instead add a special-case for 32-bit
kernels. This is more obvious and hopefully avoids a similar bug
happening again in future.
Note that because 32-bit kernels never have VSX enabled the code doesn't
need to consider TS_FPRWIDTH/OFFSET at all.
Fixes: 87fec0514f61 ("powerpc: PTRACE_PEEKUSR/PTRACE_POKEUSER of FPR registers
in little endian builds")
Cc: sta...@vger.kernel.org # v3.13+
Reported-by: Ariel Miculas
Tested-by: Christophe Leroy
Signed-off-by: Michael Ellerman
Link: https://lore.kernel.org/r/20220609133245.573565-1-...@ellerman.id.au
---
arch/powerpc/kernel/ptrace.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 4f2829634d79..88947f5fd778 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -2938,8 +2938,13 @@ long arch_ptrace(struct task_struct *child, long request,
flush_fp_to_thread(child);
if (fpidx < (PT_FPSCR - PT_FPR0))
- memcpy(&tmp, &child->thread.TS_FPR(fpidx),
- sizeof(long));
+ if (IS_ENABLED(CONFIG_PPC32)) {
+ // On 32-bit the index we are passed
refers to 32-bit words
+ tmp = ((u32
*)child->thread.fp_state.fpr)[fpidx];
+ } else {
+ memcpy(&tmp,
&child->thread.TS_FPR(fpidx),
+ sizeof(long));
+ }
else
tmp = child->thread.fp_state.fpscr;
}
@@ -2971,8 +2976,13 @@ long arch_ptrace(struct task_struct *child, long request,
flush_fp_to_thread(child);
if (fpidx < (PT_FPSCR - PT_FPR0))
- memcpy(&child->thread.TS_FPR(fpidx), &data,
- sizeof(long));
+ if (IS_ENABLED(CONFIG_PPC32)) {
+ // On 32-bit the index we are passed
refers to 32-bit words
+ ((u32
*)child->thread.fp_state.fpr)[fpidx] = data;
+ } else {
+ memcpy(&child->thread.TS_FPR(fpidx),
&data,
+ sizeof(long));
+ }
else
child->thread.fp_state.fpscr = data;
ret = 0;
--
2.35.3
[PATCH AUTOSEL 5.18 01/47] powerpc/kasan: Silence KASAN warnings in __get_wchan()
From: He Ying
[ Upstream commit a1b29ba2f2c171b9bea73be993bfdf0a62d37d15 ]
The following KASAN warning was reported in our kernel.
BUG: KASAN: stack-out-of-bounds in get_wchan+0x188/0x250
Read of size 4 at addr d216f958 by task ps/14437
CPU: 3 PID: 14437 Comm: ps Tainted: G O 5.10.0 #1
Call Trace:
[daa63858] [c0654348] dump_stack+0x9c/0xe4 (unreliable)
[daa63888] [c035cf0c] print_address_description.constprop.3+0x8c/0x570
[daa63908] [c035d6bc] kasan_report+0x1ac/0x218
[daa63948] [c00496e8] get_wchan+0x188/0x250
[daa63978] [c0461ec8] do_task_stat+0xce8/0xe60
[daa63b98] [c0455ac8] proc_single_show+0x98/0x170
[daa63bc8] [c03cab8c] seq_read_iter+0x1ec/0x900
[daa63c38] [c03cb47c] seq_read+0x1dc/0x290
[daa63d68] [c037fc94] vfs_read+0x164/0x510
[daa63ea8] [c03808e4] ksys_read+0x144/0x1d0
[daa63f38] [c005b1dc] ret_from_syscall+0x0/0x38
--- interrupt: c00 at 0x8fa8f4
LR = 0x8fa8cc
The buggy address belongs to the page:
page:98ebcdd2 refcount:0 mapcount:0 mapping: index:0x2 pfn:0x1216f
flags: 0x0()
raw: 01010122 0002
raw:
page dumped because: kasan: bad access detected
Memory state around the buggy address:
d216f800: 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00
d216f880: f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>d216f900: 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00
^
d216f980: f2 f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00
d216fa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
After looking into this issue, I find the buggy address belongs
to the task stack region. It seems KASAN has something wrong.
I look into the code of __get_wchan in x86 architecture and
find the same issue has been resolved by the commit
f7d27c35ddff ("x86/mm, kasan: Silence KASAN warnings in get_wchan()").
The solution could be applied to powerpc architecture too.
As Andrey Ryabinin said, get_wchan() is racy by design, it may
access volatile stack of running task, thus it may access
redzone in a stack frame and cause KASAN to warn about this.
Use READ_ONCE_NOCHECK() to silence these warnings.
Reported-by: Wanming Hu
Signed-off-by: He Ying
Signed-off-by: Chen Jingwen
Reviewed-by: Kees Cook
Signed-off-by: Michael Ellerman
Link: https://lore.kernel.org/r/20220121014418.155675-1-heyin...@huawei.com
Signed-off-by: Sasha Levin
---
arch/powerpc/kernel/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 984813a4d5dc..a75d20f23dac 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2160,12 +2160,12 @@ static unsigned long ___get_wchan(struct task_struct *p)
return 0;
do {
- sp = *(unsigned long *)sp;
+ sp = READ_ONCE_NOCHECK(*(unsigned long *)sp);
if (!validate_sp(sp, p, STACK_FRAME_OVERHEAD) ||
task_is_running(p))
return 0;
if (count > 0) {
- ip = ((unsigned long *)sp)[STACK_FRAME_LR_SAVE];
+ ip = READ_ONCE_NOCHECK(((unsigned long
*)sp)[STACK_FRAME_LR_SAVE]);
if (!in_sched_functions(ip))
return ip;
}
--
2.35.1
[PATCH AUTOSEL 5.17 01/43] powerpc/kasan: Silence KASAN warnings in __get_wchan()
From: He Ying
[ Upstream commit a1b29ba2f2c171b9bea73be993bfdf0a62d37d15 ]
The following KASAN warning was reported in our kernel.
BUG: KASAN: stack-out-of-bounds in get_wchan+0x188/0x250
Read of size 4 at addr d216f958 by task ps/14437
CPU: 3 PID: 14437 Comm: ps Tainted: G O 5.10.0 #1
Call Trace:
[daa63858] [c0654348] dump_stack+0x9c/0xe4 (unreliable)
[daa63888] [c035cf0c] print_address_description.constprop.3+0x8c/0x570
[daa63908] [c035d6bc] kasan_report+0x1ac/0x218
[daa63948] [c00496e8] get_wchan+0x188/0x250
[daa63978] [c0461ec8] do_task_stat+0xce8/0xe60
[daa63b98] [c0455ac8] proc_single_show+0x98/0x170
[daa63bc8] [c03cab8c] seq_read_iter+0x1ec/0x900
[daa63c38] [c03cb47c] seq_read+0x1dc/0x290
[daa63d68] [c037fc94] vfs_read+0x164/0x510
[daa63ea8] [c03808e4] ksys_read+0x144/0x1d0
[daa63f38] [c005b1dc] ret_from_syscall+0x0/0x38
--- interrupt: c00 at 0x8fa8f4
LR = 0x8fa8cc
The buggy address belongs to the page:
page:98ebcdd2 refcount:0 mapcount:0 mapping: index:0x2 pfn:0x1216f
flags: 0x0()
raw: 01010122 0002
raw:
page dumped because: kasan: bad access detected
Memory state around the buggy address:
d216f800: 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00
d216f880: f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>d216f900: 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00
^
d216f980: f2 f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00
d216fa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
After looking into this issue, I find the buggy address belongs
to the task stack region. It seems KASAN has something wrong.
I look into the code of __get_wchan in x86 architecture and
find the same issue has been resolved by the commit
f7d27c35ddff ("x86/mm, kasan: Silence KASAN warnings in get_wchan()").
The solution could be applied to powerpc architecture too.
As Andrey Ryabinin said, get_wchan() is racy by design, it may
access volatile stack of running task, thus it may access
redzone in a stack frame and cause KASAN to warn about this.
Use READ_ONCE_NOCHECK() to silence these warnings.
Reported-by: Wanming Hu
Signed-off-by: He Ying
Signed-off-by: Chen Jingwen
Reviewed-by: Kees Cook
Signed-off-by: Michael Ellerman
Link: https://lore.kernel.org/r/20220121014418.155675-1-heyin...@huawei.com
Signed-off-by: Sasha Levin
---
arch/powerpc/kernel/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 984813a4d5dc..a75d20f23dac 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2160,12 +2160,12 @@ static unsigned long ___get_wchan(struct task_struct *p)
return 0;
do {
- sp = *(unsigned long *)sp;
+ sp = READ_ONCE_NOCHECK(*(unsigned long *)sp);
if (!validate_sp(sp, p, STACK_FRAME_OVERHEAD) ||
task_is_running(p))
return 0;
if (count > 0) {
- ip = ((unsigned long *)sp)[STACK_FRAME_LR_SAVE];
+ ip = READ_ONCE_NOCHECK(((unsigned long
*)sp)[STACK_FRAME_LR_SAVE]);
if (!in_sched_functions(ip))
return ip;
}
--
2.35.1
[PATCH AUTOSEL 5.15 01/41] powerpc/kasan: Silence KASAN warnings in __get_wchan()
From: He Ying
[ Upstream commit a1b29ba2f2c171b9bea73be993bfdf0a62d37d15 ]
The following KASAN warning was reported in our kernel.
BUG: KASAN: stack-out-of-bounds in get_wchan+0x188/0x250
Read of size 4 at addr d216f958 by task ps/14437
CPU: 3 PID: 14437 Comm: ps Tainted: G O 5.10.0 #1
Call Trace:
[daa63858] [c0654348] dump_stack+0x9c/0xe4 (unreliable)
[daa63888] [c035cf0c] print_address_description.constprop.3+0x8c/0x570
[daa63908] [c035d6bc] kasan_report+0x1ac/0x218
[daa63948] [c00496e8] get_wchan+0x188/0x250
[daa63978] [c0461ec8] do_task_stat+0xce8/0xe60
[daa63b98] [c0455ac8] proc_single_show+0x98/0x170
[daa63bc8] [c03cab8c] seq_read_iter+0x1ec/0x900
[daa63c38] [c03cb47c] seq_read+0x1dc/0x290
[daa63d68] [c037fc94] vfs_read+0x164/0x510
[daa63ea8] [c03808e4] ksys_read+0x144/0x1d0
[daa63f38] [c005b1dc] ret_from_syscall+0x0/0x38
--- interrupt: c00 at 0x8fa8f4
LR = 0x8fa8cc
The buggy address belongs to the page:
page:98ebcdd2 refcount:0 mapcount:0 mapping: index:0x2 pfn:0x1216f
flags: 0x0()
raw: 01010122 0002
raw:
page dumped because: kasan: bad access detected
Memory state around the buggy address:
d216f800: 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00
d216f880: f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>d216f900: 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00
^
d216f980: f2 f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00
d216fa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
After looking into this issue, I find the buggy address belongs
to the task stack region. It seems KASAN has something wrong.
I look into the code of __get_wchan in x86 architecture and
find the same issue has been resolved by the commit
f7d27c35ddff ("x86/mm, kasan: Silence KASAN warnings in get_wchan()").
The solution could be applied to powerpc architecture too.
As Andrey Ryabinin said, get_wchan() is racy by design, it may
access volatile stack of running task, thus it may access
redzone in a stack frame and cause KASAN to warn about this.
Use READ_ONCE_NOCHECK() to silence these warnings.
Reported-by: Wanming Hu
Signed-off-by: He Ying
Signed-off-by: Chen Jingwen
Reviewed-by: Kees Cook
Signed-off-by: Michael Ellerman
Link: https://lore.kernel.org/r/20220121014418.155675-1-heyin...@huawei.com
Signed-off-by: Sasha Levin
---
arch/powerpc/kernel/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 50436b52c213..39a0a13a3a27 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2124,12 +2124,12 @@ static unsigned long __get_wchan(struct task_struct *p)
return 0;
do {
- sp = *(unsigned long *)sp;
+ sp = READ_ONCE_NOCHECK(*(unsigned long *)sp);
if (!validate_sp(sp, p, STACK_FRAME_OVERHEAD) ||
task_is_running(p))
return 0;
if (count > 0) {
- ip = ((unsigned long *)sp)[STACK_FRAME_LR_SAVE];
+ ip = READ_ONCE_NOCHECK(((unsigned long
*)sp)[STACK_FRAME_LR_SAVE]);
if (!in_sched_functions(ip))
return ip;
}
--
2.35.1
[PATCH AUTOSEL 5.10 01/29] powerpc/kasan: Silence KASAN warnings in __get_wchan()
From: He Ying
[ Upstream commit a1b29ba2f2c171b9bea73be993bfdf0a62d37d15 ]
The following KASAN warning was reported in our kernel.
BUG: KASAN: stack-out-of-bounds in get_wchan+0x188/0x250
Read of size 4 at addr d216f958 by task ps/14437
CPU: 3 PID: 14437 Comm: ps Tainted: G O 5.10.0 #1
Call Trace:
[daa63858] [c0654348] dump_stack+0x9c/0xe4 (unreliable)
[daa63888] [c035cf0c] print_address_description.constprop.3+0x8c/0x570
[daa63908] [c035d6bc] kasan_report+0x1ac/0x218
[daa63948] [c00496e8] get_wchan+0x188/0x250
[daa63978] [c0461ec8] do_task_stat+0xce8/0xe60
[daa63b98] [c0455ac8] proc_single_show+0x98/0x170
[daa63bc8] [c03cab8c] seq_read_iter+0x1ec/0x900
[daa63c38] [c03cb47c] seq_read+0x1dc/0x290
[daa63d68] [c037fc94] vfs_read+0x164/0x510
[daa63ea8] [c03808e4] ksys_read+0x144/0x1d0
[daa63f38] [c005b1dc] ret_from_syscall+0x0/0x38
--- interrupt: c00 at 0x8fa8f4
LR = 0x8fa8cc
The buggy address belongs to the page:
page:98ebcdd2 refcount:0 mapcount:0 mapping: index:0x2 pfn:0x1216f
flags: 0x0()
raw: 01010122 0002
raw:
page dumped because: kasan: bad access detected
Memory state around the buggy address:
d216f800: 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00
d216f880: f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>d216f900: 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00
^
d216f980: f2 f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00
d216fa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
After looking into this issue, I find the buggy address belongs
to the task stack region. It seems KASAN has something wrong.
I look into the code of __get_wchan in x86 architecture and
find the same issue has been resolved by the commit
f7d27c35ddff ("x86/mm, kasan: Silence KASAN warnings in get_wchan()").
The solution could be applied to powerpc architecture too.
As Andrey Ryabinin said, get_wchan() is racy by design, it may
access volatile stack of running task, thus it may access
redzone in a stack frame and cause KASAN to warn about this.
Use READ_ONCE_NOCHECK() to silence these warnings.
Reported-by: Wanming Hu
Signed-off-by: He Ying
Signed-off-by: Chen Jingwen
Reviewed-by: Kees Cook
Signed-off-by: Michael Ellerman
Link: https://lore.kernel.org/r/20220121014418.155675-1-heyin...@huawei.com
Signed-off-by: Sasha Levin
---
arch/powerpc/kernel/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 3064694afea1..cfb8fd76afb4 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2108,12 +2108,12 @@ static unsigned long __get_wchan(struct task_struct *p)
return 0;
do {
- sp = *(unsigned long *)sp;
+ sp = READ_ONCE_NOCHECK(*(unsigned long *)sp);
if (!validate_sp(sp, p, STACK_FRAME_OVERHEAD) ||
p->state == TASK_RUNNING)
return 0;
if (count > 0) {
- ip = ((unsigned long *)sp)[STACK_FRAME_LR_SAVE];
+ ip = READ_ONCE_NOCHECK(((unsigned long
*)sp)[STACK_FRAME_LR_SAVE]);
if (!in_sched_functions(ip))
return ip;
}
--
2.35.1
[PATCH AUTOSEL 5.4 01/23] powerpc/kasan: Silence KASAN warnings in __get_wchan()
From: He Ying
[ Upstream commit a1b29ba2f2c171b9bea73be993bfdf0a62d37d15 ]
The following KASAN warning was reported in our kernel.
BUG: KASAN: stack-out-of-bounds in get_wchan+0x188/0x250
Read of size 4 at addr d216f958 by task ps/14437
CPU: 3 PID: 14437 Comm: ps Tainted: G O 5.10.0 #1
Call Trace:
[daa63858] [c0654348] dump_stack+0x9c/0xe4 (unreliable)
[daa63888] [c035cf0c] print_address_description.constprop.3+0x8c/0x570
[daa63908] [c035d6bc] kasan_report+0x1ac/0x218
[daa63948] [c00496e8] get_wchan+0x188/0x250
[daa63978] [c0461ec8] do_task_stat+0xce8/0xe60
[daa63b98] [c0455ac8] proc_single_show+0x98/0x170
[daa63bc8] [c03cab8c] seq_read_iter+0x1ec/0x900
[daa63c38] [c03cb47c] seq_read+0x1dc/0x290
[daa63d68] [c037fc94] vfs_read+0x164/0x510
[daa63ea8] [c03808e4] ksys_read+0x144/0x1d0
[daa63f38] [c005b1dc] ret_from_syscall+0x0/0x38
--- interrupt: c00 at 0x8fa8f4
LR = 0x8fa8cc
The buggy address belongs to the page:
page:98ebcdd2 refcount:0 mapcount:0 mapping: index:0x2 pfn:0x1216f
flags: 0x0()
raw: 01010122 0002
raw:
page dumped because: kasan: bad access detected
Memory state around the buggy address:
d216f800: 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00
d216f880: f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>d216f900: 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00
^
d216f980: f2 f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00
d216fa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
After looking into this issue, I find the buggy address belongs
to the task stack region. It seems KASAN has something wrong.
I look into the code of __get_wchan in x86 architecture and
find the same issue has been resolved by the commit
f7d27c35ddff ("x86/mm, kasan: Silence KASAN warnings in get_wchan()").
The solution could be applied to powerpc architecture too.
As Andrey Ryabinin said, get_wchan() is racy by design, it may
access volatile stack of running task, thus it may access
redzone in a stack frame and cause KASAN to warn about this.
Use READ_ONCE_NOCHECK() to silence these warnings.
Reported-by: Wanming Hu
Signed-off-by: He Ying
Signed-off-by: Chen Jingwen
Reviewed-by: Kees Cook
Signed-off-by: Michael Ellerman
Link: https://lore.kernel.org/r/20220121014418.155675-1-heyin...@huawei.com
Signed-off-by: Sasha Levin
---
arch/powerpc/kernel/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index c94bba9142e7..832663f21422 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2001,12 +2001,12 @@ static unsigned long __get_wchan(struct task_struct *p)
return 0;
do {
- sp = *(unsigned long *)sp;
+ sp = READ_ONCE_NOCHECK(*(unsigned long *)sp);
if (!validate_sp(sp, p, STACK_FRAME_OVERHEAD) ||
p->state == TASK_RUNNING)
return 0;
if (count > 0) {
- ip = ((unsigned long *)sp)[STACK_FRAME_LR_SAVE];
+ ip = READ_ONCE_NOCHECK(((unsigned long
*)sp)[STACK_FRAME_LR_SAVE]);
if (!in_sched_functions(ip))
return ip;
}
--
2.35.1
[PATCH AUTOSEL 4.19 01/18] powerpc/kasan: Silence KASAN warnings in __get_wchan()
From: He Ying
[ Upstream commit a1b29ba2f2c171b9bea73be993bfdf0a62d37d15 ]
The following KASAN warning was reported in our kernel.
BUG: KASAN: stack-out-of-bounds in get_wchan+0x188/0x250
Read of size 4 at addr d216f958 by task ps/14437
CPU: 3 PID: 14437 Comm: ps Tainted: G O 5.10.0 #1
Call Trace:
[daa63858] [c0654348] dump_stack+0x9c/0xe4 (unreliable)
[daa63888] [c035cf0c] print_address_description.constprop.3+0x8c/0x570
[daa63908] [c035d6bc] kasan_report+0x1ac/0x218
[daa63948] [c00496e8] get_wchan+0x188/0x250
[daa63978] [c0461ec8] do_task_stat+0xce8/0xe60
[daa63b98] [c0455ac8] proc_single_show+0x98/0x170
[daa63bc8] [c03cab8c] seq_read_iter+0x1ec/0x900
[daa63c38] [c03cb47c] seq_read+0x1dc/0x290
[daa63d68] [c037fc94] vfs_read+0x164/0x510
[daa63ea8] [c03808e4] ksys_read+0x144/0x1d0
[daa63f38] [c005b1dc] ret_from_syscall+0x0/0x38
--- interrupt: c00 at 0x8fa8f4
LR = 0x8fa8cc
The buggy address belongs to the page:
page:98ebcdd2 refcount:0 mapcount:0 mapping: index:0x2 pfn:0x1216f
flags: 0x0()
raw: 01010122 0002
raw:
page dumped because: kasan: bad access detected
Memory state around the buggy address:
d216f800: 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00
d216f880: f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>d216f900: 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00
^
d216f980: f2 f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00
d216fa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
After looking into this issue, I find the buggy address belongs
to the task stack region. It seems KASAN has something wrong.
I look into the code of __get_wchan in x86 architecture and
find the same issue has been resolved by the commit
f7d27c35ddff ("x86/mm, kasan: Silence KASAN warnings in get_wchan()").
The solution could be applied to powerpc architecture too.
As Andrey Ryabinin said, get_wchan() is racy by design, it may
access volatile stack of running task, thus it may access
redzone in a stack frame and cause KASAN to warn about this.
Use READ_ONCE_NOCHECK() to silence these warnings.
Reported-by: Wanming Hu
Signed-off-by: He Ying
Signed-off-by: Chen Jingwen
Reviewed-by: Kees Cook
Signed-off-by: Michael Ellerman
Link: https://lore.kernel.org/r/20220121014418.155675-1-heyin...@huawei.com
Signed-off-by: Sasha Levin
---
arch/powerpc/kernel/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 02b69a68139c..56c33285b1df 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2017,12 +2017,12 @@ unsigned long get_wchan(struct task_struct *p)
return 0;
do {
- sp = *(unsigned long *)sp;
+ sp = READ_ONCE_NOCHECK(*(unsigned long *)sp);
if (!validate_sp(sp, p, STACK_FRAME_OVERHEAD) ||
p->state == TASK_RUNNING)
return 0;
if (count > 0) {
- ip = ((unsigned long *)sp)[STACK_FRAME_LR_SAVE];
+ ip = READ_ONCE_NOCHECK(((unsigned long
*)sp)[STACK_FRAME_LR_SAVE]);
if (!in_sched_functions(ip))
return ip;
}
--
2.35.1
[Bug 203699] Kernel 5.2-rc1 fails to boot on a Mac Mini G4: dt_headr_start=0x01501000
https://bugzilla.kernel.org/show_bug.cgi?id=203699 Michael Ellerman (mich...@ellerman.id.au) changed: What|Removed |Added Status|NEEDINFO|CLOSED Resolution|--- |CODE_FIX --- Comment #3 from Michael Ellerman (mich...@ellerman.id.au) --- v5.19-rc2 is booting on my mac mini with KASAN enabled, so I'm going to close this as fixed. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 1/6] powerpc: Add ZERO_GPRS macros for register clears
Segher Boessenkool writes: > On Sat, Jun 11, 2022 at 08:42:27AM +, Christophe Leroy wrote: >> Le 10/06/2022 à 05:32, Rohan McLure a écrit : >> > .macro ZERO_REGS start, end >> >.Lreg=\start >> >.rept (\end - \start + 1) >> >li .Lreg, 0 >> >.Lreg=.Lreg+1 >> >.endr >> > .endm >> >> I'd have a preference for using a verb, for instance ZEROISE_REGS or >> CLEAR_REGS > > "Zero" is a verb as well (as well as a noun and an adjective) :-) And "clear" is also a verb and an adjective, though helpfully the noun is "clearing" :D We could use "nullify", that has some existing usage in the kernel, although I don't really mind, "zeroise" sounds kind of cool :) cheers
回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
Hi Greg, thanks for the comments.
The questions are replied specifically below.
I have figured out and tested the patch v2, which is to be posted later.
>发件人: Greg KH
>发送时间: 2022年6月9日 21:17
>收件人: Wang Wenhu
>抄送: christophe.le...@csgroup.eu ;
>m...@ellerman.id.au ; linuxppc-dev@lists.ozlabs.org
>; linux-ker...@vger.kernel.org
>
>主题: Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver
>implementation
>
>On Thu, Jun 09, 2022 at 03:28:55AM -0700, Wang Wenhu wrote:
>> The l2-cache could be optionally configured as SRAM partly or fully.
>> Users can make use of it as a block of independent memory that offers
>> special usage, such as for debuging or other cratical status info
>> storage which keeps consistently even when the whole system crashed.
>>
>> The hardware related configuration process utilized the work of the
>> earlier implementation, which has been removed now.
>> See:
>> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
>>
>> Cc: Christophe Leroy
>> Signed-off-by: Wang Wenhu
>> ---
>> drivers/uio/Kconfig | 10 +
>> drivers/uio/Makefile | 1 +
>> drivers/uio/uio_fsl_85xx_cache_sram.c | 286 ++
>> 3 files changed, 297 insertions(+)
>> create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
>>
>> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
>> index 2e16c5338e5b..9199ced03880 100644
>> --- a/drivers/uio/Kconfig
>> +++ b/drivers/uio/Kconfig
>> @@ -105,6 +105,16 @@ config UIO_NETX
>> To compile this driver as a module, choose M here; the module
>> will be called uio_netx.
>>
>> +config UIO_FSL_85XX_CACHE_SRAM
>> + tristate "Freescale 85xx Cache-Sram driver"
>> + depends on FSL_SOC_BOOKE && PPC32
>> + help
>> + Generic driver for accessing the Cache-Sram form user level. This
>> + is extremely helpful for some user-space applications that require
>> + high performance memory accesses.
>> +
>> + If you don't know what to do here, say N.
>
>Module name information?
>
More detailed and clearer info in v2
>> +
>> config UIO_FSL_ELBC_GPCM
>> tristate "eLBC/GPCM driver"
>> depends on FSL_LBC
>> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
>> index f2f416a14228..1ba07d92a1b1 100644
>> --- a/drivers/uio/Makefile
>> +++ b/drivers/uio/Makefile
>> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624) += uio_mf624.o
>> obj-$(CONFIG_UIO_FSL_ELBC_GPCM) += uio_fsl_elbc_gpcm.o
>> obj-$(CONFIG_UIO_HV_GENERIC) += uio_hv_generic.o
>> obj-$(CONFIG_UIO_DFL) += uio_dfl.o
>> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM) += uio_fsl_85xx_cache_sram.o
>> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c
>> b/drivers/uio/uio_fsl_85xx_cache_sram.c
>> new file mode 100644
>> index ..d363f9d2b179
>> --- /dev/null
>> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
>> @@ -0,0 +1,286 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2022 Wang Wenhu
>> + * All rights reserved.
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#define DRIVER_NAME "uio_mpc85xx_cache_sram"
>> +#define UIO_INFO_VER "0.0.1"
>> +#define UIO_NAME "uio_cache_sram"
>> +
>> +#define L2CR_L2FI 0x4000 /* L2 flash
>> invalidate */
>> +#define L2CR_L2IO 0x0020 /* L2
>> instruction only */
>> +#define L2CR_SRAM_ZERO 0x /* L2SRAM zero
>> size */
>> +#define L2CR_SRAM_FULL 0x0001 /* L2SRAM full
>> size */
>> +#define L2CR_SRAM_HALF 0x0002 /* L2SRAM half
>> size */
>> +#define L2CR_SRAM_TWO_HALFS 0x0003 /* L2SRAM two half
>> sizes */
>> +#define L2CR_SRAM_QUART 0x0004 /* L2SRAM one
>> quarter size */
>> +#define L2CR_SRAM_TWO_QUARTS 0x0005 /* L2SRAM two quarter size */
>> +#define L2CR_SRAM_EIGHTH 0x0006 /* L2SRAM one eighth
>> size */
>> +#define L2CR_SRAM_TWO_EIGHTH 0x0007 /* L2SRAM two eighth size */
>> +
>> +#define L2SRAM_OPTIMAL_SZ_SHIFT 0x0003 /* Optimum size for
>> L2SRAM */
>> +
>> +#define L2SRAM_BAR_MSK_LO18 0xC000 /* Lower 18 bits */
>> +#define L2SRAM_BARE_MSK_HI4 0x000F /* Upper 4 bits */
>> +
>> +enum cache_sram_lock_ways {
>> + LOCK_WAYS_ZERO,
>> + LOCK_WAYS_EIGHTH,
>> + LOCK_WAYS_TWO_EIGHTH,
>
>Why not have values for these?
>
full values given in v2
>> + LOCK_WAYS_HALF = 4,
>> + LOCK_WAYS_FULL = 8,
>> +};
>> +
>> +struct mpc85xx_l2ctlr {
>> + u32 ctl; /* 0x000 - L2 control */
>
>What is the endian of these u32 values? You map them directly to
>memory, so they must be specified some way, right? Please make it
>obvious what th
Re: 回复: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver implementation
On Tue, Jun 14, 2022 at 06:09:35AM +, Wenhu Wang wrote:
> Hi Greg, thanks for the comments.
> The questions are replied specifically below.
> I have figured out and tested the patch v2, which is to be posted later.
> >发件人: Greg KH
> >发送时间: 2022年6月9日 21:17
> >收件人: Wang Wenhu
> >抄送: christophe.le...@csgroup.eu ;
> >m...@ellerman.id.au ; linuxppc-dev@lists.ozlabs.org
> >; linux-ker...@vger.kernel.org
> >
> >主题: Re: [PATCH 2/2] uio:powerpc:mpc85xx: l2-cache-sram uio driver
> >implementation
> >
> >On Thu, Jun 09, 2022 at 03:28:55AM -0700, Wang Wenhu wrote:
> >> The l2-cache could be optionally configured as SRAM partly or fully.
> >> Users can make use of it as a block of independent memory that offers
> >> special usage, such as for debuging or other cratical status info
> >> storage which keeps consistently even when the whole system crashed.
> >>
> >> The hardware related configuration process utilized the work of the
> >> earlier implementation, which has been removed now.
> >> See:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dc21ed2aef4150fc2fcf58227a4ff24502015c03
> >>
> >> Cc: Christophe Leroy
> >> Signed-off-by: Wang Wenhu
> >> ---
> >> drivers/uio/Kconfig | 10 +
> >> drivers/uio/Makefile | 1 +
> >> drivers/uio/uio_fsl_85xx_cache_sram.c | 286 ++
> >> 3 files changed, 297 insertions(+)
> >> create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c
> >>
> >> diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
> >> index 2e16c5338e5b..9199ced03880 100644
> >> --- a/drivers/uio/Kconfig
> >> +++ b/drivers/uio/Kconfig
> >> @@ -105,6 +105,16 @@ config UIO_NETX
> >> To compile this driver as a module, choose M here; the module
> >> will be called uio_netx.
> >>
> >> +config UIO_FSL_85XX_CACHE_SRAM
> >> + tristate "Freescale 85xx Cache-Sram driver"
> >> + depends on FSL_SOC_BOOKE && PPC32
> >> + help
> >> + Generic driver for accessing the Cache-Sram form user level. This
> >> + is extremely helpful for some user-space applications that require
> >> + high performance memory accesses.
> >> +
> >> + If you don't know what to do here, say N.
> >
> >Module name information?
> >
>
> More detailed and clearer info in v2
>
> >> +
> >> config UIO_FSL_ELBC_GPCM
> >> tristate "eLBC/GPCM driver"
> >> depends on FSL_LBC
> >> diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> >> index f2f416a14228..1ba07d92a1b1 100644
> >> --- a/drivers/uio/Makefile
> >> +++ b/drivers/uio/Makefile
> >> @@ -12,3 +12,4 @@ obj-$(CONFIG_UIO_MF624) += uio_mf624.o
> >> obj-$(CONFIG_UIO_FSL_ELBC_GPCM) += uio_fsl_elbc_gpcm.o
> >> obj-$(CONFIG_UIO_HV_GENERIC) += uio_hv_generic.o
> >> obj-$(CONFIG_UIO_DFL) += uio_dfl.o
> >> +obj-$(CONFIG_UIO_FSL_85XX_CACHE_SRAM) += uio_fsl_85xx_cache_sram.o
> >> diff --git a/drivers/uio/uio_fsl_85xx_cache_sram.c
> >> b/drivers/uio/uio_fsl_85xx_cache_sram.c
> >> new file mode 100644
> >> index ..d363f9d2b179
> >> --- /dev/null
> >> +++ b/drivers/uio/uio_fsl_85xx_cache_sram.c
> >> @@ -0,0 +1,286 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) 2022 Wang Wenhu
> >> + * All rights reserved.
> >> + */
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +#define DRIVER_NAME "uio_mpc85xx_cache_sram"
> >> +#define UIO_INFO_VER "0.0.1"
> >> +#define UIO_NAME "uio_cache_sram"
> >> +
> >> +#define L2CR_L2FI 0x4000 /* L2 flash
> >> invalidate */
> >> +#define L2CR_L2IO 0x0020 /* L2
> >> instruction only */
> >> +#define L2CR_SRAM_ZERO 0x /* L2SRAM
> >> zero size */
> >> +#define L2CR_SRAM_FULL 0x0001 /* L2SRAM
> >> full size */
> >> +#define L2CR_SRAM_HALF 0x0002 /* L2SRAM
> >> half size */
> >> +#define L2CR_SRAM_TWO_HALFS 0x0003 /* L2SRAM two half
> >> sizes */
> >> +#define L2CR_SRAM_QUART 0x0004 /* L2SRAM
> >> one quarter size */
> >> +#define L2CR_SRAM_TWO_QUARTS 0x0005 /* L2SRAM two quarter size */
> >> +#define L2CR_SRAM_EIGHTH 0x0006 /* L2SRAM one eighth
> >> size */
> >> +#define L2CR_SRAM_TWO_EIGHTH 0x0007 /* L2SRAM two eighth size */
> >> +
> >> +#define L2SRAM_OPTIMAL_SZ_SHIFT 0x0003 /* Optimum size for
> >> L2SRAM */
> >> +
> >> +#define L2SRAM_BAR_MSK_LO18 0xC000 /* Lower 18 bits */
> >> +#define L2SRAM_BARE_MSK_HI4 0x000F /* Upper 4 bits */
> >> +
> >> +enum cache_sram_lock_ways {
> >> + LOCK_WAYS_ZERO,
> >> + LOCK_WAYS_EIGHTH,
> >> + LOCK_WAYS_TWO_EIGHTH,
> >
> >Why not have values for these?
> >
>
> full values given
