* Re: [PATCH v5 0/6] Add audio driver base on rpmsg on i.MX platform
From: Mark Brown @ 2021-03-25 17:37 UTC (permalink / raw)
To: tiwai, perex, festevam, Xiubo.Lee, devicetree, nicoleotsuka,
linuxppc-dev, linux-kernel, timur, alsa-devel, Shengjiu Wang,
lgirdwood, robh+dt
Cc: Mark Brown
In-Reply-To: <1615516725-4975-1-git-send-email-shengjiu.wang@nxp.com>
On Fri, 12 Mar 2021 10:38:39 +0800, Shengjiu Wang wrote:
> On Asymmetric multiprocessor, there is Cortex-A core and Cortex-M core,
> Linux is running on A core, RTOS is running on M core.
> The audio hardware device can be controlled by Cortex-M device,
> So audio playback/capture can be handled by M core.
>
> Rpmsg is the interface for sending and receiving msg to and from M
> core, that we can create a virtual sound on Cortex-A core side.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/6] ASoC: soc-component: Add snd_soc_pcm_component_ack
commit: 8bdfc0455e3a59e2c1207a56be22e910fae0e0d5
[2/6] ASoC: fsl_rpmsg: Add CPU DAI driver for audio base on rpmsg
commit: b73d9e6225e86492f6a901223a34ecfa7b55c178
[3/6] ASoC: dt-bindings: fsl_rpmsg: Add binding doc for rpmsg audio device
commit: 49c6bf62498344fa8f8af2314231f3eb37e0e150
[4/6] ASoC: imx-audio-rpmsg: Add rpmsg_driver for audio channel
commit: 1935050de0b6c6c961e9de51d5b5d05642f861f1
[5/6] ASoC: imx-pcm-rpmsg: Add platform driver for audio base on rpmsg
commit: 3c00eceb2a5391ed1ca6703b71cad35ab8cd4352
[6/6] ASoC: imx-rpmsg: Add machine driver for audio base on rpmsg
commit: 39f8405c3e502e7b9d0533fa0b0bfe715b3e89c1
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH] docs: powerpc: Fix a typo
From: Jonathan Corbet @ 2021-03-25 17:48 UTC (permalink / raw)
To: Bhaskar Chowdhury, mpe, benh, paulus, linuxppc-dev, linux-doc,
linux-kernel
Cc: rdunlap, Bhaskar Chowdhury
In-Reply-To: <20210322062237.2971314-1-unixbhaskar@gmail.com>
Bhaskar Chowdhury <unixbhaskar@gmail.com> writes:
> s/struture/structure/
>
> Signed-off-by: Bhaskar Chowdhury <unixbhaskar@gmail.com>
> ---
> Documentation/powerpc/firmware-assisted-dump.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/powerpc/firmware-assisted-dump.rst b/Documentation/powerpc/firmware-assisted-dump.rst
> index 20ea8cdee0aa..6c0ae070ba67 100644
> --- a/Documentation/powerpc/firmware-assisted-dump.rst
> +++ b/Documentation/powerpc/firmware-assisted-dump.rst
> @@ -171,7 +171,7 @@ that were present in CMA region::
> (meta area) |
> |
> |
> - Metadata: This area holds a metadata struture whose
> + Metadata: This area holds a metadata structure whose
> address is registered with f/w and retrieved in the
> second kernel after crash, on platforms that support
Applied, thanks.
jon
^ permalink raw reply
* Re: VDSO ELF header
From: Laurent Dufour @ 2021-03-25 19:02 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev
In-Reply-To: <5b03e966-2cfd-5f0c-c48d-dea5e0001833@linux.ibm.com>
Le 25/03/2021 à 17:56, Laurent Dufour a écrit :
> Le 25/03/2021 à 17:46, Christophe Leroy a écrit :
>> Hi Laurent
>>
>> Le 25/03/2021 à 17:11, Laurent Dufour a écrit :
>>> Hi Christophe,
>>>
>>> Since v5.11 and the changes you made to the VDSO code, it no more exposing
>>> the ELF header at the beginning of the VDSO mapping in user space.
>>>
>>> This is confusing CRIU which is checking for this ELF header cookie
>>> (https://github.com/checkpoint-restore/criu/issues/1417).
>>
>> How does it do on other architectures ?
>
> Good question, I'll double check the CRIU code.
On x86, there are 2 VDSO entries:
7ffff7fcb000-7ffff7fce000 r--p 00000000 00:00 0 [vvar]
7ffff7fce000-7ffff7fcf000 r-xp 00000000 00:00 0 [vdso]
And the VDSO is starting with the ELF header.
>
>>
>>>
>>> I'm not an expert in loading and ELF part and reading the change you made, I
>>> can't identify how this could work now as I'm expecting the loader to need
>>> that ELF header to do the relocation.
>>
>> I think the loader is able to find it at the expected place.
>
> Actually, it seems the loader relies on the AUX vector AT_SYSINFO_EHDR. I guess
> CRIU should do the same.
>
>>>
>>> From my investigation it seems that the first bytes of the VDSO area are now
>>> the vdso_arch_data.
>>>
>>> Is the ELF header put somewhere else?
>>> How could the loader process the VDSO without that ELF header?
>>>
>>
>> Like most other architectures, we now have the data section as first page and
>> the text section follows. So you will likely find the elf header on the second
>> page.
I'm wondering if the data section you're refering to is the vvar section I can
see on x86.
>>
>> Done in this commit:
>> https://github.com/linuxppc/linux/commit/511157ab641eb6bedd00d62673388e78a4f871cf
>
> I'll double check on x86, but anyway, I think CRIU should rely on
> AT_SYSINFO_EHDR and not assume that the ELF header is at the beginning of VDSO
> mapping.
>
> Thanks for your help.
> Laurent.
>
^ permalink raw reply
* Re: [PATCH v2 6/7] cmdline: Gives architectures opportunity to use generically defined boot cmdline manipulation
From: Will Deacon @ 2021-03-25 19:32 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, robh, daniel, devicetree, linux-kernel,
Paul Mackerras, linuxppc-dev, danielwa
In-Reply-To: <8db81511-3f28-4ef1-5e66-188cf7cafad1@csgroup.eu>
On Thu, Mar 25, 2021 at 12:18:38PM +0100, Christophe Leroy wrote:
>
>
> Le 03/03/2021 à 18:57, Will Deacon a écrit :
> > On Tue, Mar 02, 2021 at 05:25:22PM +0000, Christophe Leroy wrote:
> > > Most architectures have similar boot command line manipulation
> > > options. This patchs adds the definition in init/Kconfig, gated by
> > > CONFIG_HAVE_CMDLINE that the architectures can select to use them.
> > >
> > > In order to use this, a few architectures will have to change their
> > > CONFIG options:
> > > - riscv has to replace CMDLINE_FALLBACK by CMDLINE_FROM_BOOTLOADER
> > > - architectures using CONFIG_CMDLINE_OVERRIDE or
> > > CONFIG_CMDLINE_OVERWRITE have to replace them by CONFIG_CMDLINE_FORCE.
> > >
> > > Architectures also have to define CONFIG_DEFAULT_CMDLINE.
> > >
> > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > > ---
> > > init/Kconfig | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 56 insertions(+)
> > >
> > > diff --git a/init/Kconfig b/init/Kconfig
> > > index 22946fe5ded9..a0f2ad9467df 100644
> > > --- a/init/Kconfig
> > > +++ b/init/Kconfig
> > > @@ -117,6 +117,62 @@ config INIT_ENV_ARG_LIMIT
> > > Maximum of each of the number of arguments and environment
> > > variables passed to init from the kernel command line.
> > > +config HAVE_CMDLINE
> > > + bool
> > > +
> > > +config CMDLINE_BOOL
> > > + bool "Default bootloader kernel arguments"
> > > + depends on HAVE_CMDLINE
> > > + help
> > > + On some platforms, there is currently no way for the boot loader to
> > > + pass arguments to the kernel. For these platforms, you can supply
> > > + some command-line options at build time by entering them here. In
> > > + most cases you will need to specify the root device here.
> >
> > Why is this needed as well as CMDLINE_FROM_BOOTLOADER? IIUC, the latter
> > will use CONFIG_CMDLINE if it fails to get anything from the bootloader,
> > which sounds like the same scenario.
> >
> > > +config CMDLINE
> > > + string "Initial kernel command string"
> >
> > s/Initial/Default
> >
> > which is then consistent with the rest of the text here.
> >
> > > + depends on CMDLINE_BOOL
> >
> > Ah, so this is a bit different and I don't think lines-up with the
> > CMDLINE_BOOL help text.
> >
> > > + default DEFAULT_CMDLINE
> > > + help
> > > + On some platforms, there is currently no way for the boot loader to
> > > + pass arguments to the kernel. For these platforms, you can supply
> > > + some command-line options at build time by entering them here. In
> > > + most cases you will need to specify the root device here.
> >
> > (same stale text)
> >
> > > +choice
> > > + prompt "Kernel command line type" if CMDLINE != ""
> > > + default CMDLINE_FROM_BOOTLOADER
> > > + help
> > > + Selects the way you want to use the default kernel arguments.
> >
> > How about:
> >
> > "Determines how the default kernel arguments are combined with any
> > arguments passed by the bootloader"
> >
> > > +config CMDLINE_FROM_BOOTLOADER
> > > + bool "Use bootloader kernel arguments if available"
> > > + help
> > > + Uses the command-line options passed by the boot loader. If
> > > + the boot loader doesn't provide any, the default kernel command
> > > + string provided in CMDLINE will be used.
> > > +
> > > +config CMDLINE_EXTEND
> >
> > Can we rename this to CMDLINE_APPEND, please? There is code in the tree
> > which disagrees about what CMDLINE_EXTEND means, so that will need be
> > to be updated to be consistent (e.g. the EFI stub parsing order). Having
> > the generic option with a different name means we won't accidentally end
> > up with the same inconsistent behaviours.
>
> Argh, yes. Seems like the problem is even larger than that IIUC:
>
> - For ARM it means to append the bootloader arguments to the CONFIG_CMDLINE
> - For Powerpc it means to append the CONFIG_CMDLINE to the bootloader arguments
> - For SH it means to append the CONFIG_CMDLINE to the bootloader arguments
> - For EFI it means to append the bootloader arguments to the CONFIG_CMDLINE
> - For OF it means to append the CONFIG_CMDLINE to the bootloader arguments
>
> So what happens on ARM for instance when it selects CONFIG_OF for instance ?
I think ARM gets different behaviour depending on whether it uses ATAGs or
FDT.
> Or should we consider that EXTEND means APPEND or PREPEND, no matter which ?
> Because EXTEND is for instance used for:
>
> config INITRAMFS_FORCE
> bool "Ignore the initramfs passed by the bootloader"
> depends on CMDLINE_EXTEND || CMDLINE_FORCE
Oh man, I didn't spot that one :(
I think I would make the generic options explicit: either APPEND or PREPEND.
Then architectures which choose to define CMDLINE_EXTEND in their Kconfigs
can select the generic option that matches their behaviour.
INITRAMFS_FORCE sounds like it should depend on APPEND (assuming that means
CONFIG_CMDLINE is appended to the bootloader arguments).
Will
^ permalink raw reply
* Re: [PATCH v2 3/7] powerpc: convert config files to generic cmdline
From: Daniel Walker @ 2021-03-25 19:56 UTC (permalink / raw)
To: Christophe Leroy
Cc: Rob Herring, Daniel Gimpelevich, linuxppc-dev, x86, linux-mips,
linux-kernel, Paul Mackerras, xe-linux-external, Andrew Morton,
Will Deacon
In-Reply-To: <e4899874-1684-fa1b-443e-f4e478e05e31@csgroup.eu>
On Wed, Mar 24, 2021 at 05:59:59PM +0100, Christophe Leroy wrote:
> > I think my changes maintain most of this due to the override of
> > CONFIG_CMDLINE_PREPEND. This is an upgrade and the inflexibility in powerpc is
> > an example of why these changes were created in the first place.
>
> "inflexibility in powerpc" : Can you elaborate ?
the prom environment.
> >
> > For example , say the default command line is "root=/dev/issblk0" from iss476
> > platform. And the bootloader adds "root=/dev/sda1"
> >
> > The result is <prepend><bootloader><append>.
>
>
> I'm still having hard time understanding the benefit of having both <prepend> and <append>.
> Could you please provide a complete exemple from real life, ie what exactly
> the problem is and what it solves ?
Say the boot loader of an old product is released with a command line of
"root=/dev/sda" and per the needs of the company or product the boot loader can
not be upgraded to change this command line. To change this behavior you would
need append or EXTEND.
Below I detail an example of PREPEND due to your list question.
> >
> > Then you have,
> >
> > root=/dev/issblk0 root=/dev/sda1
> >
> > and the bootloader has precedent over the default command line. So root= in the
> > above cases is defined by the bootloader.
A person could input a command line into a boot loader, and it would override
the PREPEND values.
Can you imagine you have a default command line which makes root=/dev/issblk0 ,
but that doesn't work for you testing purpose. So you input into the boot loader
root=/dev/sda1 , since you have the default input in the bootloader OVERRIDEABLE
you can do this without re-compiling and just input the single root= command
into the bootloader.
Daniel
^ permalink raw reply
* Re: [PATCH v2 3/7] powerpc: convert config files to generic cmdline
From: Daniel Walker @ 2021-03-25 19:59 UTC (permalink / raw)
To: Christophe Leroy
Cc: Rob Herring, Daniel Gimpelevich, linuxppc-dev, X86 ML,
open list:MIPS, linux-kernel@vger.kernel.org, Paul Mackerras,
xe-linux-external, Andrew Morton, Will Deacon
In-Reply-To: <20fd7d44-8c39-48bc-25c3-990be9d9d911@csgroup.eu>
On Thu, Mar 25, 2021 at 01:03:55PM +0100, Christophe Leroy wrote:
>
> Ok, so you agree we don't need to provide two CMDLINE, one to be appended and one to be prepended.
>
> Let's only provide once CMDLINE as of today, and ask the user to select
> whether he wants it appended or prepended or replacee. Then no need to
> change all existing config to rename CONFIG_CMDLINE into either of the new
> ones.
>
> That's the main difference between my series and Daniel's series. So I'll
> finish taking Will's comment into account and we'll send out a v3 soon.
It doesn't solve the needs of Cisco, I've stated many times your changes have
little value. Please stop submitting them.
Daniel
^ permalink raw reply
* Re: [PATCH v2 4/7] CMDLINE: powerpc: convert to generic builtin command line
From: Daniel Walker @ 2021-03-25 20:03 UTC (permalink / raw)
To: Christophe Leroy
Cc: Rob Herring, Ruslan Ruslichenko, Ruslan Bilovol,
Daniel Gimpelevich, linuxppc-dev, x86, linux-mips, linux-kernel,
Paul Mackerras, xe-linux-external, Andrew Morton, Will Deacon
In-Reply-To: <9c5b8e33-026e-c9d6-c267-a5dd4a2b999c@csgroup.eu>
On Wed, Mar 24, 2021 at 04:31:35PM +0100, Christophe Leroy wrote:
>
>
> Le 09/03/2021 à 22:40, Daniel Walker a écrit :
> > On Tue, Mar 09, 2021 at 08:56:47AM +0100, Christophe Leroy wrote:
> > >
> > >
> > > Le 09/03/2021 à 01:02, Daniel Walker a écrit :
> > > > This updates the powerpc code to use the CONFIG_GENERIC_CMDLINE
> > > > option.
> > > >
> > > > Cc: xe-linux-external@cisco.com
> > > > Signed-off-by: Ruslan Ruslichenko <rruslich@cisco.com>
> > > > Signed-off-by: Ruslan Bilovol <rbilovol@cisco.com>
> > > > Signed-off-by: Daniel Walker <danielwa@cisco.com>
> > > > ---
> > > > arch/powerpc/Kconfig | 37 +--------------------------------
> > > > arch/powerpc/kernel/prom.c | 1 +
> > > > arch/powerpc/kernel/prom_init.c | 35 ++++++++++++++++++-------------
> > > > 3 files changed, 23 insertions(+), 50 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > > > index 107bb4319e0e..276b06d5c961 100644
> > > > --- a/arch/powerpc/Kconfig
> > > > +++ b/arch/powerpc/Kconfig
> > > > @@ -167,6 +167,7 @@ config PPC
> > > > select EDAC_SUPPORT
> > > > select GENERIC_ATOMIC64 if PPC32
> > > > select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> > > > + select GENERIC_CMDLINE
> > > > select GENERIC_CMOS_UPDATE
> > > > select GENERIC_CPU_AUTOPROBE
> > > > select GENERIC_CPU_VULNERABILITIES if PPC_BARRIER_NOSPEC
> > > > @@ -906,42 +907,6 @@ config PPC_DENORMALISATION
> > > > Add support for handling denormalisation of single precision
> > > > values. Useful for bare metal only. If unsure say Y here.
> > > > -config CMDLINE
> > > > - string "Initial kernel command string"
> > > > - default ""
> > > > - help
> > > > - On some platforms, there is currently no way for the boot loader to
> > > > - pass arguments to the kernel. For these platforms, you can supply
> > > > - some command-line options at build time by entering them here. In
> > > > - most cases you will need to specify the root device here.
> > > > -
> > > > -choice
> > > > - prompt "Kernel command line type" if CMDLINE != ""
> > > > - default CMDLINE_FROM_BOOTLOADER
> > > > -
> > > > -config CMDLINE_FROM_BOOTLOADER
> > > > - bool "Use bootloader kernel arguments if available"
> > > > - help
> > > > - Uses the command-line options passed by the boot loader. If
> > > > - the boot loader doesn't provide any, the default kernel command
> > > > - string provided in CMDLINE will be used.
>
> I can't see how the above is supported in the generic builtin.
>
> Taking into account that it is the default on powerpc, I'm having hardtime with that.
Hmm, so this ignores the built in changes. You just don't enable it, or you
don't add PREPEND or APPEND.
> Any feedback on the proposed changes I made on the 13th ? I know it is
> partly buggy but that was more for the principle. I can make clean working
> patch if it helps.
The reason I added it into the function parameters is because I can get free
type checking on the functions. If you use macro's then you don't know if the
function is compatible.
Daniel
^ permalink raw reply
* [PATCH] selftests: powerpc: unmark non-kernel-doc comments
From: Randy Dunlap @ 2021-03-25 20:08 UTC (permalink / raw)
To: linux-kernel; +Cc: Randy Dunlap, linuxppc-dev
Drop the 'beginning of kernel-doc' notation markers (/**)
in places that are not in kernel-doc format.
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
---
tools/testing/selftests/powerpc/tm/tm-trap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- linux-next-20210323.orig/tools/testing/selftests/powerpc/tm/tm-trap.c
+++ linux-next-20210323/tools/testing/selftests/powerpc/tm/tm-trap.c
@@ -66,7 +66,7 @@ void trap_signal_handler(int signo, sigi
/* Get thread endianness: extract bit LE from MSR */
thread_endianness = MSR_LE & ucp->uc_mcontext.gp_regs[PT_MSR];
- /***
+ /*
* Little-Endian Machine
*/
@@ -126,7 +126,7 @@ void trap_signal_handler(int signo, sigi
}
}
- /***
+ /*
* Big-Endian Machine
*/
^ permalink raw reply
* Re: [PATCH v2 05/15] powerpc/uaccess: Move get_user_instr helpers in asm/inst.h
From: Daniel Axtens @ 2021-03-25 21:59 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <2c6e83581b4fa434aa7cf2fa7714c41e98f57007.1615398265.git.christophe.leroy@csgroup.eu>
Hi Christophe,
> Those helpers use get_user helpers but they don't participate
> in their implementation, so they do not belong to asm/uaccess.h
>
> Move them in asm/inst.h
Hmm, is asm/inst.h the right place for this?
asm/inst.h seems to be entirely concerned with the ppc_inst type:
converting things to and from ppc_inst, print ppc_inst as a string,
dealing with prefixed instructs, etc., etc. The only things currently
that look at memory are the probe_user_read_inst and
probe_kernel_read_inst prototypes...
Having said that, I'm not sure quite where else to put it, and none of
the other places in arch/powerpc/include that currently reference
ppc_inst seem any better...
If we do use asm/inst.h, I think maybe it makes sense to put the
code towards the end rather than at the start, as uses structs and calls
macros that are defined later on in the function.
Kind regards,
Daniel
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/inst.h | 34 ++++++++++++++++++++++++++++++
> arch/powerpc/include/asm/uaccess.h | 34 ------------------------------
> 2 files changed, 34 insertions(+), 34 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index cc73c1267572..19e18af2fac9 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -4,6 +4,40 @@
>
> #include <asm/ppc-opcode.h>
>
> +#ifdef CONFIG_PPC64
> +
> +#define ___get_user_instr(gu_op, dest, ptr) \
> +({ \
> + long __gui_ret = 0; \
> + unsigned long __gui_ptr = (unsigned long)ptr; \
> + struct ppc_inst __gui_inst; \
> + unsigned int __prefix, __suffix; \
> + __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \
> + if (__gui_ret == 0) { \
> + if ((__prefix >> 26) == OP_PREFIX) { \
> + __gui_ret = gu_op(__suffix, \
> + (unsigned int __user *)__gui_ptr + 1); \
> + __gui_inst = ppc_inst_prefix(__prefix, \
> + __suffix); \
> + } else { \
> + __gui_inst = ppc_inst(__prefix); \
> + } \
> + if (__gui_ret == 0) \
> + (dest) = __gui_inst; \
> + } \
> + __gui_ret; \
> +})
> +#else /* !CONFIG_PPC64 */
> +#define ___get_user_instr(gu_op, dest, ptr) \
> + gu_op((dest).val, (u32 __user *)(ptr))
> +#endif /* CONFIG_PPC64 */
> +
> +#define get_user_instr(x, ptr) \
> + ___get_user_instr(get_user, x, ptr)
> +
> +#define __get_user_instr(x, ptr) \
> + ___get_user_instr(__get_user, x, ptr)
> +
> /*
> * Instruction data type for POWER
> */
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 01aea0df4dd0..eaa828a6a419 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -53,40 +53,6 @@ static inline bool __access_ok(unsigned long addr, unsigned long size)
> #define __put_user(x, ptr) \
> __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
>
> -#ifdef CONFIG_PPC64
> -
> -#define ___get_user_instr(gu_op, dest, ptr) \
> -({ \
> - long __gui_ret = 0; \
> - unsigned long __gui_ptr = (unsigned long)ptr; \
> - struct ppc_inst __gui_inst; \
> - unsigned int __prefix, __suffix; \
> - __gui_ret = gu_op(__prefix, (unsigned int __user *)__gui_ptr); \
> - if (__gui_ret == 0) { \
> - if ((__prefix >> 26) == OP_PREFIX) { \
> - __gui_ret = gu_op(__suffix, \
> - (unsigned int __user *)__gui_ptr + 1); \
> - __gui_inst = ppc_inst_prefix(__prefix, \
> - __suffix); \
> - } else { \
> - __gui_inst = ppc_inst(__prefix); \
> - } \
> - if (__gui_ret == 0) \
> - (dest) = __gui_inst; \
> - } \
> - __gui_ret; \
> -})
> -#else /* !CONFIG_PPC64 */
> -#define ___get_user_instr(gu_op, dest, ptr) \
> - gu_op((dest).val, (u32 __user *)(ptr))
> -#endif /* CONFIG_PPC64 */
> -
> -#define get_user_instr(x, ptr) \
> - ___get_user_instr(get_user, x, ptr)
> -
> -#define __get_user_instr(x, ptr) \
> - ___get_user_instr(__get_user, x, ptr)
> -
> extern long __put_user_bad(void);
>
> #define __put_user_size(x, ptr, size, retval) \
> --
> 2.25.0
^ permalink raw reply
* Re: [PATCH v2 06/15] powerpc/align: Don't use __get_user_instr() on kernel addresses
From: Daniel Axtens @ 2021-03-25 22:12 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <d9ecbce00178484e66ca7adec2ff210058037704.1615398265.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> In the old days, when we didn't have kernel userspace access
> protection and had set_fs(), it was wise to use __get_user()
> and friends to read kernel memory.
>
> Nowadays, get_user() is granting userspace access and is exclusively
> for userspace access.
>
> In alignment exception handler, use probe_kernel_read_inst()
> instead of __get_user_instr() for reading instructions in kernel.
>
> This will allow to remove the is_kernel_addr() check in
> __get/put_user() in a following patch.
>
Looks good to me!
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/align.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
> index c4d7b445b459..8d4c7af262e2 100644
> --- a/arch/powerpc/kernel/align.c
> +++ b/arch/powerpc/kernel/align.c
> @@ -310,7 +310,11 @@ int fix_alignment(struct pt_regs *regs)
> */
> CHECK_FULL_REGS(regs);
>
> - if (unlikely(__get_user_instr(instr, (void __user *)regs->nip)))
> + if (is_kernel_addr(regs->nip))
> + r = probe_kernel_read_inst(&instr, (void *)regs->nip);
> + else
> + r = __get_user_instr(instr, (void __user *)regs->nip);
> + if (unlikely(r))
> return -EFAULT;
> if ((regs->msr & MSR_LE) != (MSR_KERNEL & MSR_LE)) {
> /* We don't handle PPC little-endian any more... */
> --
> 2.25.0
^ permalink raw reply
* Re: [PATCH v2 07/15] powerpc/uaccess: Call might_fault() inconditionaly
From: Daniel Axtens @ 2021-03-25 22:38 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <e0a980a4dc7a2551183dd5cb30f46eafdbee390c.1615398265.git.christophe.leroy@csgroup.eu>
Hi Christophe,
> Commit 6bfd93c32a50 ("powerpc: Fix incorrect might_sleep in
> __get_user/__put_user on kernel addresses") added a check to not call
> might_sleep() on kernel addresses. This was to enable the use of
> __get_user() in the alignment exception handler for any address.
>
> Then commit 95156f0051cb ("lockdep, mm: fix might_fault() annotation")
> added a check of the address space in might_fault(), based on
> set_fs() logic. But this didn't solve the powerpc alignment exception
> case as it didn't call set_fs(KERNEL_DS).
>
> Nowadays, set_fs() is gone, previous patch fixed the alignment
> exception handler and __get_user/__put_user are not supposed to be
> used anymore to read kernel memory.
>
> Therefore the is_kernel_addr() check has become useless and can be
> removed.
While I agree that __get_user/__put_user should not be used on kernel
memory, I'm not sure that we have covered every case where they might be
used on kernel memory yet. I did a git grep for __get_user - there are
several callers in arch/powerpc and it looks like at least lib/sstep.c
might be using __get_user to read kernel memory while single-stepping.
I am not sure if might_sleep has got logic to cover the powerpc case -
it uses uaccess_kernel, but we don't supply a definition for that on
powerpc, so if we do end up calling __get_user on a kernel address, I
think we might now throw a warning. (Unless we are saved by
pagefault_disabled()?)
(But I haven't tested this yet, so it's possible I misunderstood
something.)
Do you expect any consequences if we've missed a case where
__(get|put)_user is called on a kernel address because it hasn't been
converted to use better helpers yet?
Kind regards,
Daniel
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/uaccess.h | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index eaa828a6a419..c4bbc64758a0 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -77,8 +77,7 @@ __pu_failed: \
> __typeof__(*(ptr)) __pu_val = (x); \
> __typeof__(size) __pu_size = (size); \
> \
> - if (!is_kernel_addr((unsigned long)__pu_addr)) \
> - might_fault(); \
> + might_fault(); \
> __chk_user_ptr(__pu_addr); \
> __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
> \
> @@ -238,12 +237,12 @@ do { \
> __typeof__(size) __gu_size = (size); \
> \
> __chk_user_ptr(__gu_addr); \
> - if (do_allow && !is_kernel_addr((unsigned long)__gu_addr)) \
> + if (do_allow) { \
> might_fault(); \
> - if (do_allow) \
> __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
> - else \
> + } else { \
> __get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
> + } \
> (x) = (__typeof__(*(ptr)))__gu_val; \
> \
> __gu_err; \
> --
> 2.25.0
^ permalink raw reply
* Re: [PATCH v2 07/15] powerpc/uaccess: Call might_fault() inconditionaly
From: Daniel Axtens @ 2021-03-25 22:44 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <874kgykgfk.fsf@dja-thinkpad.axtens.net>
Daniel Axtens <dja@axtens.net> writes:
> Hi Christophe,
>
>> Commit 6bfd93c32a50 ("powerpc: Fix incorrect might_sleep in
>> __get_user/__put_user on kernel addresses") added a check to not call
>> might_sleep() on kernel addresses. This was to enable the use of
>> __get_user() in the alignment exception handler for any address.
>>
>> Then commit 95156f0051cb ("lockdep, mm: fix might_fault() annotation")
>> added a check of the address space in might_fault(), based on
>> set_fs() logic. But this didn't solve the powerpc alignment exception
>> case as it didn't call set_fs(KERNEL_DS).
>>
>> Nowadays, set_fs() is gone, previous patch fixed the alignment
>> exception handler and __get_user/__put_user are not supposed to be
>> used anymore to read kernel memory.
>>
>> Therefore the is_kernel_addr() check has become useless and can be
>> removed.
>
> While I agree that __get_user/__put_user should not be used on kernel
> memory, I'm not sure that we have covered every case where they might be
> used on kernel memory yet. I did a git grep for __get_user - there are
> several callers in arch/powerpc and it looks like at least lib/sstep.c
> might be using __get_user to read kernel memory while single-stepping.
>
> I am not sure if might_sleep has got logic to cover the powerpc case -
> it uses uaccess_kernel, but we don't supply a definition for that on
> powerpc, so if we do end up calling __get_user on a kernel address, I
> think we might now throw a warning. (Unless we are saved by
> pagefault_disabled()?)
Ah, I just re-read some of my earlier emails and was reminded that yes,
if we are calling __get/put, we must have disabled page faults.
So yes, this is good.
>
> (But I haven't tested this yet, so it's possible I misunderstood
> something.)
>
> Do you expect any consequences if we've missed a case where
> __(get|put)_user is called on a kernel address because it hasn't been
> converted to use better helpers yet?
>
> Kind regards,
> Daniel
>
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/include/asm/uaccess.h | 9 ++++-----
>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
>> index eaa828a6a419..c4bbc64758a0 100644
>> --- a/arch/powerpc/include/asm/uaccess.h
>> +++ b/arch/powerpc/include/asm/uaccess.h
>> @@ -77,8 +77,7 @@ __pu_failed: \
>> __typeof__(*(ptr)) __pu_val = (x); \
>> __typeof__(size) __pu_size = (size); \
>> \
>> - if (!is_kernel_addr((unsigned long)__pu_addr)) \
>> - might_fault(); \
>> + might_fault(); \
>> __chk_user_ptr(__pu_addr); \
>> __put_user_size(__pu_val, __pu_addr, __pu_size, __pu_err); \
>> \
>> @@ -238,12 +237,12 @@ do { \
>> __typeof__(size) __gu_size = (size); \
>> \
>> __chk_user_ptr(__gu_addr); \
>> - if (do_allow && !is_kernel_addr((unsigned long)__gu_addr)) \
>> + if (do_allow) { \
>> might_fault(); \
>> - if (do_allow) \
>> __get_user_size(__gu_val, __gu_addr, __gu_size, __gu_err); \
>> - else \
>> + } else { \
>> __get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
>> + } \
One microscopic nit: these changes throw the '\'s further out of
alignment.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
>> (x) = (__typeof__(*(ptr)))__gu_val; \
>> \
>> __gu_err; \
>> --
>> 2.25.0
^ permalink raw reply
* Re: [PATCH v2 3/7] powerpc: convert config files to generic cmdline
From: Rob Herring @ 2021-03-25 23:29 UTC (permalink / raw)
To: Daniel Walker
Cc: Daniel Gimpelevich, linuxppc-dev, X86 ML, open list:MIPS,
linux-kernel@vger.kernel.org, Paul Mackerras, xe-linux-external,
Andrew Morton, Will Deacon
In-Reply-To: <20210325195956.GM109100@zorba>
On Thu, Mar 25, 2021 at 2:00 PM Daniel Walker <danielwa@cisco.com> wrote:
>
> On Thu, Mar 25, 2021 at 01:03:55PM +0100, Christophe Leroy wrote:
> >
> > Ok, so you agree we don't need to provide two CMDLINE, one to be appended and one to be prepended.
> >
> > Let's only provide once CMDLINE as of today, and ask the user to select
> > whether he wants it appended or prepended or replacee. Then no need to
> > change all existing config to rename CONFIG_CMDLINE into either of the new
> > ones.
> >
> > That's the main difference between my series and Daniel's series. So I'll
> > finish taking Will's comment into account and we'll send out a v3 soon.
>
> It doesn't solve the needs of Cisco, I've stated many times your changes have
> little value. Please stop submitting them.
Can you please outline what those needs are which aren't met?
Rob
^ permalink raw reply
* Re: [PATCH v4 24/46] KVM: PPC: Book3S HV P9: Use large decrementer for HDEC
From: Alexey Kardashevskiy @ 2021-03-26 2:05 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <20210323010305.1045293-25-npiggin@gmail.com>
On 23/03/2021 12:02, Nicholas Piggin wrote:
> On processors that don't suppress the HDEC exceptions when LPCR[HDICE]=0,
> this could help reduce needless guest exits due to leftover exceptions on
> entering the guest.
>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
ERROR: modpost: "decrementer_max" [arch/powerpc/kvm/kvm-hv.ko] undefined!
need this:
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -89,6 +89,7 @@ static struct clocksource clocksource_timebase = {
#define DECREMENTER_DEFAULT_MAX 0x7FFFFFFF
u64 decrementer_max = DECREMENTER_DEFAULT_MAX;
+EXPORT_SYMBOL_GPL(decrementer_max);
> ---
> arch/powerpc/include/asm/time.h | 2 ++
> arch/powerpc/kvm/book3s_hv.c | 3 ++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 8dd3cdb25338..68d94711811e 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -18,6 +18,8 @@
> #include <asm/vdso/timebase.h>
>
> /* time.c */
> +extern u64 decrementer_max;
> +
> extern unsigned long tb_ticks_per_jiffy;
> extern unsigned long tb_ticks_per_usec;
> extern unsigned long tb_ticks_per_sec;
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 8215430e6d5e..bb30c5ab53d1 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3658,7 +3658,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> vc->tb_offset_applied = 0;
> }
>
> - mtspr(SPRN_HDEC, 0x7fffffff);
> + /* HDEC must be at least as large as DEC, so decrementer_max fits */
> + mtspr(SPRN_HDEC, decrementer_max);
>
> switch_mmu_to_host_radix(kvm, host_pidr);
>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH] soc/fsl: qbman: fix conflicting alignment attributes
From: Li Yang @ 2021-03-26 2:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Arnd Bergmann, Madalin-cristian Bucur, soc, Lee Jones, Roy Pledge,
YueHaibing, lkml, Scott Wood, Claudiu Manoil, Jakub Kicinski,
linuxppc-dev, Sebastian Andrzej Siewior,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20210323131530.2619900-1-arnd@kernel.org>
On Tue, Mar 23, 2021 at 8:17 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> When building with W=1, gcc points out that the __packed attribute
> on struct qm_eqcr_entry conflicts with the 8-byte alignment
> attribute on struct qm_fd inside it:
>
> drivers/soc/fsl/qbman/qman.c:189:1: error: alignment 1 of 'struct qm_eqcr_entry' is less than 8 [-Werror=packed-not-aligned]
>
> I assume that the alignment attribute is the correct one, and
> that qm_eqcr_entry cannot actually be unaligned in memory,
> so add the same alignment on the outer struct.
>
> Fixes: c535e923bb97 ("soc/fsl: Introduce DPAA 1.x QMan device driver")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/soc/fsl/qbman/qman.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index a1b9be1d105a..fde4edd83c14 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -186,7 +186,7 @@ struct qm_eqcr_entry {
> __be32 tag;
> struct qm_fd fd;
> u8 __reserved3[32];
> -} __packed;
> +} __packed __aligned(8);
The EQCR structure is actually aligned on 64-byte from the manual.
But probably 8 is enough to let the compiler not complain.
> #define QM_EQCR_VERB_VBIT 0x80
> #define QM_EQCR_VERB_CMD_MASK 0x61 /* but only one value; */
> #define QM_EQCR_VERB_CMD_ENQUEUE 0x01
> --
> 2.29.2
>
^ permalink raw reply
* Re: [PATCH] selftests: powerpc: unmark non-kernel-doc comments
From: Daniel Axtens @ 2021-03-26 4:50 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel; +Cc: Randy Dunlap, linuxppc-dev
In-Reply-To: <20210325200820.16594-1-rdunlap@infradead.org>
Randy Dunlap <rdunlap@infradead.org> writes:
> Drop the 'beginning of kernel-doc' notation markers (/**)
> in places that are not in kernel-doc format.
This looks good to me. Arguably we don't need the comments at all, but
it doesn't seem to hurt to keep them.
checkpatch is OK with the entire file, so there's nothing else we'd
really want to clean up while you're doing cleanups.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
> tools/testing/selftests/powerpc/tm/tm-trap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> --- linux-next-20210323.orig/tools/testing/selftests/powerpc/tm/tm-trap.c
> +++ linux-next-20210323/tools/testing/selftests/powerpc/tm/tm-trap.c
> @@ -66,7 +66,7 @@ void trap_signal_handler(int signo, sigi
> /* Get thread endianness: extract bit LE from MSR */
> thread_endianness = MSR_LE & ucp->uc_mcontext.gp_regs[PT_MSR];
>
> - /***
> + /*
> * Little-Endian Machine
> */
>
> @@ -126,7 +126,7 @@ void trap_signal_handler(int signo, sigi
> }
> }
>
> - /***
> + /*
> * Big-Endian Machine
> */
>
^ permalink raw reply
* Re: [PATCH -next] powerpc/smp: Remove unused inline functions
From: Daniel Axtens @ 2021-03-26 5:01 UTC (permalink / raw)
To: YueHaibing, mpe, benh, paulus; +Cc: YueHaibing, linuxppc-dev, linux-kernel
In-Reply-To: <20210324140752.11320-1-yuehaibing@huawei.com>
Hi,
> commit 441c19c8a290 ("powerpc/kvm/book3s_hv: Rework the secondary inhibit code")
> left behind this, so can remove it.
>
Interesting: that commit removed some instances of
(un)inhibit_secondary_onlining, but it seems to have missed the ones for
the uni-processor case, which your patch removes. This seems like a good
change.
Checkpatch does have one small complaint about your commit message:
| WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
| #6:
| commit 441c19c8a290 ("powerpc/kvm/book3s_hv: Rework the secondary inhibit code")
I don't think this warrants another revision, I think leaving the commit
name on one line makes sense.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> arch/powerpc/include/asm/smp.h | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index 7a13bc20f0a0..ad7129a19e8f 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -189,8 +189,6 @@ extern void __cpu_die(unsigned int cpu);
> #define hard_smp_processor_id() get_hard_smp_processor_id(0)
> #define smp_setup_cpu_maps()
> #define thread_group_shares_l2 0
> -static inline void inhibit_secondary_onlining(void) {}
> -static inline void uninhibit_secondary_onlining(void) {}
> static inline const struct cpumask *cpu_sibling_mask(int cpu)
> {
> return cpumask_of(cpu);
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH -next] powerpc/eeh: Remove unused inline function eeh_dev_phb_init_dynamic()
From: Daniel Axtens @ 2021-03-26 5:08 UTC (permalink / raw)
To: YueHaibing, ruscur, oohall, mpe, benh, paulus
Cc: YueHaibing, linuxppc-dev, linux-kernel
In-Reply-To: <20210324140714.19612-1-yuehaibing@huawei.com>
Hi,
> commit 475028efc708 ("powerpc/eeh: Remove eeh_dev_phb_init_dynamic()")
> left behind this, so can remove it.
I had a look: the inline that you are removing here is for the
!CONFIG_EEH case, which explains why it was missed the first time.
This looks like a good change. Out of interest, what tool are you using
to find these unused inlines? If there are many more, it might make
sense to combine future patches removing them into a single patch, but
I'm not sure.
checkpatch likes this patch, so that's also good :)
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH v2 1/1] hotplug-cpu.c: show 'last online CPU' error in dlpar_cpu_offline()
From: Daniel Axtens @ 2021-03-26 5:24 UTC (permalink / raw)
To: Daniel Henrique Barboza, linuxppc-dev; +Cc: Daniel Henrique Barboza
In-Reply-To: <20210323205056.52768-2-danielhb413@gmail.com>
Hi Daniel,
Two small nitpicks:
> This patch adds a 'last online' check in dlpar_cpu_offline() to catch
> the 'last online CPU' offline error, eturning a more informative error
^--- s/eturning/returning/;
> + /* device_offline() will return -EBUSY (via cpu_down())
> + * if there is only one CPU left. Check it here to fail
> + * earlier and with a more informative error message,
> + * while also retaining the cpu_add_remove_lock to be sure
> + * that no CPUs are being online/offlined during this
> + * check. */
Checkpatch has a small issue with this comment:
WARNING: Block comments use a trailing */ on a separate line
#50: FILE: arch/powerpc/platforms/pseries/hotplug-cpu.c:279:
+ * check. */
Apart from that, this patch seems sane to me, but I haven't been able to
test it.
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH] [v2] arch: powerpc: Remove duplicate includes
From: Daniel Axtens @ 2021-03-26 5:38 UTC (permalink / raw)
To: Wan Jiabing, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Nicholas Piggin, Aneesh Kumar K.V,
Cédric Le Goater, Randy Dunlap, Wan Jiabing, Ganesh Goudar,
Christophe Leroy, Michal Suchanek, Geert Uytterhoeven,
Pingfan Liu, Frederic Weisbecker, linuxppc-dev, linux-kernel
Cc: kael_w
In-Reply-To: <20210323062916.295346-1-wanjiabing@vivo.com>
Wan Jiabing <wanjiabing@vivo.com> writes:
> mmu-hash.h: asm/bug.h has been included at line 12, so remove
> the duplicate one at line 21.
Looking at the file I had wondered if this was due to a #ifdef being
removed, but no, the second one was just added in commit 891121e6c02c
("powerpc/mm: Differentiate between hugetlb and THP during page
walk"). How odd!
Anyway, all of these look good to me, and the automated checks at
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210323062916.295346-1-wanjiabing@vivo.com/
have all passed.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH] [v2] tools: testing: Remove duplicate includes
From: Daniel Axtens @ 2021-03-26 5:45 UTC (permalink / raw)
To: Wan Jiabing, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Shuah Khan, Wan Jiabing, linuxppc-dev,
linux-kselftest, linux-kernel
Cc: kael_w
In-Reply-To: <20210323061604.289958-1-wanjiabing@vivo.com>
Wan Jiabing <wanjiabing@vivo.com> writes:
> sched.h has been included at line 33, so remove the
> duplicate one at line 36.
> pthread.h has been included at line 17,so remove the
> duplicate one at line 20.
I can see that both of these are correct from the diff.
> inttypes.h has been included at line 19, so remove the
> duplicate one at line 23.
For this one I checked the file. Indeed there is another inttypes.h, so
this is also correct.
Again, all the automated checks pass. (although I don't think any of the
automated builds include selftests.)
So:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
>
> Signed-off-by: Wan Jiabing <wanjiabing@vivo.com>
> ---
> tools/testing/selftests/powerpc/mm/tlbie_test.c | 1 -
> tools/testing/selftests/powerpc/tm/tm-poison.c | 1 -
> tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c | 1 -
> 3 files changed, 3 deletions(-)
>
> diff --git a/tools/testing/selftests/powerpc/mm/tlbie_test.c b/tools/testing/selftests/powerpc/mm/tlbie_test.c
> index f85a0938ab25..48344a74b212 100644
> --- a/tools/testing/selftests/powerpc/mm/tlbie_test.c
> +++ b/tools/testing/selftests/powerpc/mm/tlbie_test.c
> @@ -33,7 +33,6 @@
> #include <sched.h>
> #include <time.h>
> #include <stdarg.h>
> -#include <sched.h>
> #include <pthread.h>
> #include <signal.h>
> #include <sys/prctl.h>
> diff --git a/tools/testing/selftests/powerpc/tm/tm-poison.c b/tools/testing/selftests/powerpc/tm/tm-poison.c
> index 29e5f26af7b9..27c083a03d1f 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-poison.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-poison.c
> @@ -20,7 +20,6 @@
> #include <sched.h>
> #include <sys/types.h>
> #include <signal.h>
> -#include <inttypes.h>
>
> #include "tm.h"
>
> diff --git a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> index e2a0c07e8362..9ef37a9836ac 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
> @@ -17,7 +17,6 @@
> #include <pthread.h>
> #include <sys/mman.h>
> #include <unistd.h>
> -#include <pthread.h>
>
> #include "tm.h"
> #include "utils.h"
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH] powerpc/iommu/debug: Remove redundant NULL check
From: Daniel Axtens @ 2021-03-26 5:53 UTC (permalink / raw)
To: Jiapeng Chong, mpe; +Cc: Jiapeng Chong, paulus, linuxppc-dev, linux-kernel
In-Reply-To: <1616406217-94635-1-git-send-email-jiapeng.chong@linux.alibaba.com>
Hi Jiapeng Chong, <jiapeng.chong@linux.alibaba.com> writes:
> Fix the following coccicheck warnings:
>
> ./fs/io_uring.c:5989:4-9: WARNING: NULL check before some freeing
> functions is not needed.
This looks correct to me, and matches the description of debugfs_remove
in Documentation/filesystems/debugfs.rst.
If you have a number of similar fixes it might be helpful to do them in
a single bigger patch, but I'm not sure if coccicheck reports much else
as I don't have coccinelle installed at the moment.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
^ permalink raw reply
* Re: [PATCH] crypto: vmx: fix incorrect kernel-doc comment syntax in files
From: Daniel Axtens @ 2021-03-26 6:02 UTC (permalink / raw)
To: Aditya Srivastava, leitao
Cc: rdunlap, herbert, corbet, linux-kernel-mentees, nayna,
linux-kernel, yashsri421, pfsmorigo, linux-crypto, lukas.bulwahn,
linuxppc-dev, davem, paulus
In-Reply-To: <20210320202525.12562-1-yashsri421@gmail.com>
Hi Aditya,
Thanks for your patch!
> The opening comment mark '/**' is used for highlighting the beginning of
> kernel-doc comments.
> There are certain files in drivers/crypto/vmx, which follow this syntax,
> but the content inside does not comply with kernel-doc.
> Such lines were probably not meant for kernel-doc parsing, but are parsed
> due to the presence of kernel-doc like comment syntax(i.e, '/**'), which
> causes unexpected warnings from kernel-doc.
>
> E.g., presence of kernel-doc like comment in the header line for
> drivers/crypto/vmx/vmx.c causes this warning by kernel-doc:
>
> "warning: expecting prototype for Routines supporting VMX instructions on the Power 8(). Prototype was for p8_init() instead"
checkpatch (scripts/checkpatch.pl --strict -g HEAD) complains about this line:
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
but checkpatch should be ignored here, as you did the right thing by not
breaking an error message across multiple lines.
> Similarly for other files too.
>
> Provide a simple fix by replacing such occurrences with general comment
> format, i.e. '/*', to prevent kernel-doc from parsing it.
This makes sense.
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> * Applies perfectly on next-20210319
>
> drivers/crypto/vmx/aes.c | 2 +-
> drivers/crypto/vmx/aes_cbc.c | 2 +-
> drivers/crypto/vmx/aes_ctr.c | 2 +-
> drivers/crypto/vmx/aes_xts.c | 2 +-
> drivers/crypto/vmx/ghash.c | 2 +-
> drivers/crypto/vmx/vmx.c | 2 +-
> 6 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/crypto/vmx/aes.c b/drivers/crypto/vmx/aes.c
> index d05c02baebcf..ec06189fbf99 100644
> --- a/drivers/crypto/vmx/aes.c
> +++ b/drivers/crypto/vmx/aes.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/**
> +/*
> * AES routines supporting VMX instructions on the Power 8
> *
> * Copyright (C) 2015 International Business Machines Inc.
> diff --git a/drivers/crypto/vmx/aes_cbc.c b/drivers/crypto/vmx/aes_cbc.c
> index d88084447f1c..ed0debc7acb5 100644
> --- a/drivers/crypto/vmx/aes_cbc.c
> +++ b/drivers/crypto/vmx/aes_cbc.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/**
> +/*
> * AES CBC routines supporting VMX instructions on the Power 8
> *
> * Copyright (C) 2015 International Business Machines Inc.
> diff --git a/drivers/crypto/vmx/aes_ctr.c b/drivers/crypto/vmx/aes_ctr.c
> index 79ba062ee1c1..9a3da8cd62f3 100644
> --- a/drivers/crypto/vmx/aes_ctr.c
> +++ b/drivers/crypto/vmx/aes_ctr.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/**
> +/*
> * AES CTR routines supporting VMX instructions on the Power 8
> *
> * Copyright (C) 2015 International Business Machines Inc.
> diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c
> index 9fee1b1532a4..dabbccb41550 100644
> --- a/drivers/crypto/vmx/aes_xts.c
> +++ b/drivers/crypto/vmx/aes_xts.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/**
> +/*
> * AES XTS routines supporting VMX In-core instructions on Power 8
> *
> * Copyright (C) 2015 International Business Machines Inc.
> diff --git a/drivers/crypto/vmx/ghash.c b/drivers/crypto/vmx/ghash.c
> index 14807ac2e3b9..5bc5710a6de0 100644
> --- a/drivers/crypto/vmx/ghash.c
> +++ b/drivers/crypto/vmx/ghash.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0
> -/**
> +/*
> * GHASH routines supporting VMX instructions on the Power 8
> *
> * Copyright (C) 2015, 2019 International Business Machines Inc.
> diff --git a/drivers/crypto/vmx/vmx.c b/drivers/crypto/vmx/vmx.c
> index a40d08e75fc0..7eb713cc87c8 100644
> --- a/drivers/crypto/vmx/vmx.c
> +++ b/drivers/crypto/vmx/vmx.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/**
> +/*
> * Routines supporting VMX instructions on the Power 8
> *
> * Copyright (C) 2015 International Business Machines Inc.
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH] powerpc/iommu/debug: Remove redundant NULL check
From: Daniel Axtens @ 2021-03-26 6:12 UTC (permalink / raw)
To: Jiapeng Chong, mpe; +Cc: Jiapeng Chong, paulus, linuxppc-dev, linux-kernel
In-Reply-To: <87r1k2795x.fsf@linkitivity.dja.id.au>
Daniel Axtens <dja@axtens.net> writes:
It looks like the kernel test robot also reported this:
"[PATCH] powerpc/iommu/debug: fix ifnullfree.cocci warnings"
Weirdly I don't see it in patchwork.
I'm not sure which one mpe will want to take but either would do.
>> Fix the following coccicheck warnings:
>>
>> ./fs/io_uring.c:5989:4-9: WARNING: NULL check before some freeing
>> functions is not needed.
(Also, while unimportant, that's technically not the error you fix here
as it's for a different file!)
>
> This looks correct to me, and matches the description of debugfs_remove
> in Documentation/filesystems/debugfs.rst.
>
> If you have a number of similar fixes it might be helpful to do them in
> a single bigger patch, but I'm not sure if coccicheck reports much else
> as I don't have coccinelle installed at the moment.
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
>
> Kind regards,
> Daniel
^ permalink raw reply
* remove the nvlink2 pci_vfio subdriver v2
From: Christoph Hellwig @ 2021-03-26 6:13 UTC (permalink / raw)
To: Michael Ellerman, Alex Williamson
Cc: Jason Gunthorpe, kvm, David Airlie, linux-kernel, dri-devel,
Paul Mackerras, Daniel Vetter, Greg Kroah-Hartman, linux-api,
linuxppc-dev
Hi all,
the nvlink2 vfio subdriver is a weird beast. It supports a hardware
feature without any open source component - what would normally be
the normal open source userspace that we require for kernel drivers,
although in this particular case user space could of course be a
kernel driver in a VM. It also happens to be a complete mess that
does not properly bind to PCI IDs, is hacked into the vfio_pci driver
and also pulles in over 1000 lines of code always build into powerpc
kernels that have Power NV support enabled. Because of all these
issues and the lack of breaking userspace when it is removed I think
the best idea is to simply kill.
Changes since v1:
- document the removed subtypes as reserved
- add the ACK from Greg
Diffstat:
arch/powerpc/platforms/powernv/npu-dma.c | 705 ---------------------------
b/arch/powerpc/include/asm/opal.h | 3
b/arch/powerpc/include/asm/pci-bridge.h | 1
b/arch/powerpc/include/asm/pci.h | 7
b/arch/powerpc/platforms/powernv/Makefile | 2
b/arch/powerpc/platforms/powernv/opal-call.c | 2
b/arch/powerpc/platforms/powernv/pci-ioda.c | 185 -------
b/arch/powerpc/platforms/powernv/pci.c | 11
b/arch/powerpc/platforms/powernv/pci.h | 17
b/arch/powerpc/platforms/pseries/pci.c | 23
b/drivers/vfio/pci/Kconfig | 6
b/drivers/vfio/pci/Makefile | 1
b/drivers/vfio/pci/vfio_pci.c | 18
b/drivers/vfio/pci/vfio_pci_private.h | 14
b/include/uapi/linux/vfio.h | 38 -
drivers/vfio/pci/vfio_pci_nvlink2.c | 490 ------------------
16 files changed, 12 insertions(+), 1511 deletions(-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox