[PATCH 0/2] Kconfig symbol fixes on powerpc
Dear powerpc maintainers, The script ./scripts/checkkconfigsymbols.py warns on invalid references to Kconfig symbols (often, minor typos, name confusions or outdated references). This patch series addresses all issues reported by ./scripts/checkkconfigsymbols.py in ./drivers/usb/ for Kconfig and Makefile files. Issues in the Kconfig and Makefile files indicate some shortcomings in the overall build definitions, and often are true actionable issues to address. These issues can be identified and filtered by: ./scripts/checkkconfigsymbols.py | grep -E "arch/powerpc/.*(Kconfig|Makefile)" -B 1 -A 1 After applying this patch series on linux-next (next-20210817), the command above yields just two false positives (SHELL, r13) due to tool shortcomings. As these two patches are fixes, please consider if they are suitable for backporting to stable. Lukas Lukas Bulwahn (2): powerpc: kvm: rectify selection to PPC_DAWR powerpc: rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK arch/powerpc/kvm/Kconfig | 2 +- arch/powerpc/platforms/Kconfig.cputype | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.26.2
[PATCH 1/2] powerpc: kvm: rectify selection to PPC_DAWR
Commit a278e7ea608b ("powerpc: Fix compile issue with force DAWR")
selects the non-existing config PPC_DAWR_FORCE_ENABLE for config
KVM_BOOK3S_64_HANDLER. As this commit also introduces a config PPC_DAWR,
it probably intends to select PPC_DAWR instead.
Rectify the selection in config KVM_BOOK3S_64_HANDLER to PPC_DAWR.
The issue was identified with ./scripts/checkkconfigsymbols.py.
Fixes: a278e7ea608b ("powerpc: Fix compile issue with force DAWR")
Signed-off-by: Lukas Bulwahn
---
arch/powerpc/kvm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
index e45644657d49..aa29ea56c80a 100644
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -38,7 +38,7 @@ config KVM_BOOK3S_32_HANDLER
config KVM_BOOK3S_64_HANDLER
bool
select KVM_BOOK3S_HANDLER
- select PPC_DAWR_FORCE_ENABLE
+ select PPC_DAWR
config KVM_BOOK3S_PR_POSSIBLE
bool
--
2.26.2
[PATCH 2/2] powerpc: rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK
Commit 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
selects the non-existing config ARCH_ENABLE_PMD_SPLIT_PTLOCK in
./arch/powerpc/platforms/Kconfig.cputype, but clearly it intends to select
ARCH_ENABLE_SPLIT_PMD_PTLOCK here (notice the word swapping!), as this
commit does select that for all other architectures.
Rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK instead.
Fixes: 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
Signed-off-by: Lukas Bulwahn
---
arch/powerpc/platforms/Kconfig.cputype | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/Kconfig.cputype
b/arch/powerpc/platforms/Kconfig.cputype
index 6794145603de..a208997ade88 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -98,7 +98,7 @@ config PPC_BOOK3S_64
select PPC_HAVE_PMU_SUPPORT
select HAVE_ARCH_TRANSPARENT_HUGEPAGE
select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
- select ARCH_ENABLE_PMD_SPLIT_PTLOCK
+ select ARCH_ENABLE_SPLIT_PMD_PTLOCK
select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
select ARCH_SUPPORTS_HUGETLBFS
select ARCH_SUPPORTS_NUMA_BALANCING
--
2.26.2
Re: [PATCH 1/2] powerpc: kvm: rectify selection to PPC_DAWR
Le 19/08/2021 à 11:32, Lukas Bulwahn a écrit :
Commit a278e7ea608b ("powerpc: Fix compile issue with force DAWR")
selects the non-existing config PPC_DAWR_FORCE_ENABLE for config
KVM_BOOK3S_64_HANDLER. As this commit also introduces a config PPC_DAWR,
it probably intends to select PPC_DAWR instead.
Rectify the selection in config KVM_BOOK3S_64_HANDLER to PPC_DAWR.
The issue was identified with ./scripts/checkkconfigsymbols.py.
Fixes: a278e7ea608b ("powerpc: Fix compile issue with force DAWR")
Signed-off-by: Lukas Bulwahn
---
arch/powerpc/kvm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
index e45644657d49..aa29ea56c80a 100644
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -38,7 +38,7 @@ config KVM_BOOK3S_32_HANDLER
config KVM_BOOK3S_64_HANDLER
bool
select KVM_BOOK3S_HANDLER
- select PPC_DAWR_FORCE_ENABLE
+ select PPC_DAWR
That's useless, see
https://elixir.bootlin.com/linux/v5.14-rc6/source/arch/powerpc/Kconfig#L267
In arch/powerpc/Kconfig, you already have:
select PPC_DAWR if PPC64
config KVM_BOOK3S_PR_POSSIBLE
bool
Re: [PATCH v2 02/12] mm: Introduce a function to check for virtualization protection features
On Fri, Aug 13, 2021 at 11:59:21AM -0500, Tom Lendacky wrote: > +#define PATTR_MEM_ENCRYPT0 /* Encrypted memory */ > +#define PATTR_HOST_MEM_ENCRYPT 1 /* Host encrypted > memory */ > +#define PATTR_GUEST_MEM_ENCRYPT 2 /* Guest encrypted > memory */ > +#define PATTR_GUEST_PROT_STATE 3 /* Guest encrypted > state */ Please write an actual detailed explanaton of what these mean, that is what implications it has on the kernel.
Re: [PATCH 1/2] powerpc: kvm: rectify selection to PPC_DAWR
On Thu, Aug 19, 2021 at 11:45 AM Christophe Leroy
wrote:
>
>
>
> Le 19/08/2021 à 11:32, Lukas Bulwahn a écrit :
> > Commit a278e7ea608b ("powerpc: Fix compile issue with force DAWR")
> > selects the non-existing config PPC_DAWR_FORCE_ENABLE for config
> > KVM_BOOK3S_64_HANDLER. As this commit also introduces a config PPC_DAWR,
> > it probably intends to select PPC_DAWR instead.
> >
> > Rectify the selection in config KVM_BOOK3S_64_HANDLER to PPC_DAWR.
> >
> > The issue was identified with ./scripts/checkkconfigsymbols.py.
> >
> > Fixes: a278e7ea608b ("powerpc: Fix compile issue with force DAWR")
> > Signed-off-by: Lukas Bulwahn
> > ---
> > arch/powerpc/kvm/Kconfig | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
> > index e45644657d49..aa29ea56c80a 100644
> > --- a/arch/powerpc/kvm/Kconfig
> > +++ b/arch/powerpc/kvm/Kconfig
> > @@ -38,7 +38,7 @@ config KVM_BOOK3S_32_HANDLER
> > config KVM_BOOK3S_64_HANDLER
> > bool
> > select KVM_BOOK3S_HANDLER
> > - select PPC_DAWR_FORCE_ENABLE
> > + select PPC_DAWR
>
> That's useless, see
> https://elixir.bootlin.com/linux/v5.14-rc6/source/arch/powerpc/Kconfig#L267
>
> In arch/powerpc/Kconfig, you already have:
>
> select PPC_DAWR if PPC64
>
Ah, I see. Then, it is just a needless and non-effective select here,
and then select can be deleted completely.
I will send a patch series v2.
Lukas
Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
On Fri, Aug 13, 2021 at 11:59:22AM -0500, Tom Lendacky wrote:
> While the name suggests this is intended mainly for guests, it will
> also be used for host memory encryption checks in place of sme_active().
Which suggest that the name is not good to start with. Maybe protected
hardware, system or platform might be a better choice?
> +static inline bool prot_guest_has(unsigned int attr)
> +{
> +#ifdef CONFIG_AMD_MEM_ENCRYPT
> + if (sme_me_mask)
> + return amd_prot_guest_has(attr);
> +#endif
> +
> + return false;
> +}
Shouldn't this be entirely out of line?
> +/* 0x800 - 0x8ff reserved for AMD */
> +#define PATTR_SME0x800
> +#define PATTR_SEV0x801
> +#define PATTR_SEV_ES 0x802
Why do we need reservations for a purely in-kernel namespace?
And why are you overoading a brand new generic API with weird details
of a specific implementation like this?
Re: [PATCH 2/2] powerpc: rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK
On 8/19/21 3:02 PM, Lukas Bulwahn wrote:
> Commit 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
> selects the non-existing config ARCH_ENABLE_PMD_SPLIT_PTLOCK in
> ./arch/powerpc/platforms/Kconfig.cputype, but clearly it intends to select
> ARCH_ENABLE_SPLIT_PMD_PTLOCK here (notice the word swapping!), as this
> commit does select that for all other architectures.
Right, indeed the words here got swapped. They look very similar and also
a cross compile would not even detect the problem because the non-existent
config option would simply evaluate to 0. Thanks for catching this.
>
> Rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK instead.
>
> Fixes: 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
> Signed-off-by: Lukas Bulwahn
> ---
> arch/powerpc/platforms/Kconfig.cputype | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/Kconfig.cputype
> b/arch/powerpc/platforms/Kconfig.cputype
> index 6794145603de..a208997ade88 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -98,7 +98,7 @@ config PPC_BOOK3S_64
> select PPC_HAVE_PMU_SUPPORT
> select HAVE_ARCH_TRANSPARENT_HUGEPAGE
> select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
> - select ARCH_ENABLE_PMD_SPLIT_PTLOCK
> + select ARCH_ENABLE_SPLIT_PMD_PTLOCK
> select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
> select ARCH_SUPPORTS_HUGETLBFS
> select ARCH_SUPPORTS_NUMA_BALANCING
>
Re: [PATCH v2 04/12] powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
On Fri, Aug 13, 2021 at 11:59:23AM -0500, Tom Lendacky wrote: > +static inline bool prot_guest_has(unsigned int attr) No reall need to have this inline. In fact I'd suggest we havea the prototype in a common header so that everyone must implement it out of line.
[PATCH v2 1/2] powerpc: kvm: remove obsolete and unneeded select
Commit a278e7ea608b ("powerpc: Fix compile issue with force DAWR")
selects the non-existing config PPC_DAWR_FORCE_ENABLE for config
KVM_BOOK3S_64_HANDLER. As this commit also introduces a config PPC_DAWR
and this config PPC_DAWR is selected with PPC if PPC64, there is no
need for any further select in the KVM_BOOK3S_64_HANDLER.
Remove an obsolete and unneeded select in config KVM_BOOK3S_64_HANDLER.
The issue was identified with ./scripts/checkkconfigsymbols.py.
Fixes: a278e7ea608b ("powerpc: Fix compile issue with force DAWR")
Signed-off-by: Lukas Bulwahn
---
arch/powerpc/kvm/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
index e45644657d49..ff581d70f20c 100644
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -38,7 +38,6 @@ config KVM_BOOK3S_32_HANDLER
config KVM_BOOK3S_64_HANDLER
bool
select KVM_BOOK3S_HANDLER
- select PPC_DAWR_FORCE_ENABLE
config KVM_BOOK3S_PR_POSSIBLE
bool
--
2.26.2
[PATCH v2 0/2] Kconfig symbol fixes on powerpc
Dear powerpc maintainers, The script ./scripts/checkkconfigsymbols.py warns on invalid references to Kconfig symbols (often, minor typos, name confusions or outdated references). This patch series addresses all issues reported by ./scripts/checkkconfigsymbols.py in ./drivers/usb/ for Kconfig and Makefile files. Issues in the Kconfig and Makefile files indicate some shortcomings in the overall build definitions, and often are true actionable issues to address. These issues can be identified and filtered by: ./scripts/checkkconfigsymbols.py | grep -E "arch/powerpc/.*(Kconfig|Makefile)" -B 1 -A 1 After applying this patch series on linux-next (next-20210817), the command above yields just two false positives (SHELL, r13) due to tool shortcomings. As these two patches are fixes, please consider if they are suitable for backporting to stable. v1 -> v2: Followed Christophe Leroy's comment and drop the obsolete select. Lukas Lukas Bulwahn (2): powerpc: kvm: remove obsolete and unneeded select powerpc: rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK arch/powerpc/kvm/Kconfig | 1 - arch/powerpc/platforms/Kconfig.cputype | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) -- 2.26.2
[PATCH v2 2/2] powerpc: rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK
Commit 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
selects the non-existing config ARCH_ENABLE_PMD_SPLIT_PTLOCK in
./arch/powerpc/platforms/Kconfig.cputype, but clearly it intends to select
ARCH_ENABLE_SPLIT_PMD_PTLOCK here (notice the word swapping!), as this
commit does select that for all other architectures.
Rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK instead.
Fixes: 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
Signed-off-by: Lukas Bulwahn
---
arch/powerpc/platforms/Kconfig.cputype | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/Kconfig.cputype
b/arch/powerpc/platforms/Kconfig.cputype
index 6794145603de..a208997ade88 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -98,7 +98,7 @@ config PPC_BOOK3S_64
select PPC_HAVE_PMU_SUPPORT
select HAVE_ARCH_TRANSPARENT_HUGEPAGE
select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
- select ARCH_ENABLE_PMD_SPLIT_PTLOCK
+ select ARCH_ENABLE_SPLIT_PMD_PTLOCK
select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
select ARCH_SUPPORTS_HUGETLBFS
select ARCH_SUPPORTS_NUMA_BALANCING
--
2.26.2
Re: [PATCH v2 0/2] Kconfig symbol fixes on powerpc
Le 19/08/2021 à 13:39, Lukas Bulwahn a écrit : Dear powerpc maintainers, The script ./scripts/checkkconfigsymbols.py warns on invalid references to Kconfig symbols (often, minor typos, name confusions or outdated references). This patch series addresses all issues reported by ./scripts/checkkconfigsymbols.py in ./drivers/usb/ for Kconfig and Makefile files. Issues in the Kconfig and Makefile files indicate some shortcomings in the overall build definitions, and often are true actionable issues to address. These issues can be identified and filtered by: ./scripts/checkkconfigsymbols.py | grep -E "arch/powerpc/.*(Kconfig|Makefile)" -B 1 -A 1 After applying this patch series on linux-next (next-20210817), the command above yields just two false positives (SHELL, r13) due to tool shortcomings. As these two patches are fixes, please consider if they are suitable for backporting to stable. v1 -> v2: Followed Christophe Leroy's comment and drop the obsolete select. No need to change anything now, but as your two patches are completely independent, you could have submitted them separately. That would have avoided to resend both when changing the first one only.
[PATCH 1/6] powerpc/prom: Introduce early_reserve_mem_old()
and condition its call with IS_ENABLED(CONFIG_PPC32). This fixes a
compile error with W=1.
arch/powerpc/kernel/prom.c: In function ‘early_reserve_mem’:
arch/powerpc/kernel/prom.c:625:10: error: variable ‘reserve_map’ set but not
used [-Werror=unused-but-set-variable]
__be64 *reserve_map;
^~~
cc1: all warnings being treated as errors
Cc: Christophe Leroy
Signed-off-by: Cédric Le Goater
---
Christophe, I think you had comments on this one ? Yes, I am being a bit lazy.
arch/powerpc/kernel/prom.c | 37 -
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index f620e04dc9bf..52869d12bc1d 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -621,27 +621,14 @@ static void __init early_reserve_mem_dt(void)
}
}
-static void __init early_reserve_mem(void)
+static void __init early_reserve_mem_old(void)
{
__be64 *reserve_map;
reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
fdt_off_mem_rsvmap(initial_boot_params));
- /* Look for the new "reserved-regions" property in the DT */
- early_reserve_mem_dt();
-
-#ifdef CONFIG_BLK_DEV_INITRD
- /* Then reserve the initrd, if any */
- if (initrd_start && (initrd_end > initrd_start)) {
- memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
- ALIGN(initrd_end, PAGE_SIZE) -
- ALIGN_DOWN(initrd_start, PAGE_SIZE));
- }
-#endif /* CONFIG_BLK_DEV_INITRD */
-
-#ifdef CONFIG_PPC32
- /*
+ /*
* Handle the case where we might be booting from an old kexec
* image that setup the mem_rsvmap as pairs of 32-bit values
*/
@@ -659,9 +646,25 @@ static void __init early_reserve_mem(void)
DBG("reserving: %x -> %x\n", base_32, size_32);
memblock_reserve(base_32, size_32);
}
- return;
}
-#endif
+}
+
+static void __init early_reserve_mem(void)
+{
+ /* Look for the new "reserved-regions" property in the DT */
+ early_reserve_mem_dt();
+
+#ifdef CONFIG_BLK_DEV_INITRD
+ /* Then reserve the initrd, if any */
+ if (initrd_start && (initrd_end > initrd_start)) {
+ memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
+ ALIGN(initrd_end, PAGE_SIZE) -
+ ALIGN_DOWN(initrd_start, PAGE_SIZE));
+ }
+#endif /* CONFIG_BLK_DEV_INITRD */
+
+ if (IS_ENABLED(CONFIG_PPC32))
+ early_reserve_mem_old();
}
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
--
2.31.1
[PATCH 3/6] KVM: PPC: Book3S PR: Declare kvmppc_handle_exit_pr()
This fixes a compile error with W=1. Signed-off-by: Cédric Le Goater --- arch/powerpc/kvm/book3s.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h index 740e51def5a5..c08f93b7f523 100644 --- a/arch/powerpc/kvm/book3s.h +++ b/arch/powerpc/kvm/book3s.h @@ -24,6 +24,7 @@ extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val); extern int kvmppc_book3s_init_pr(void); extern void kvmppc_book3s_exit_pr(void); +extern int kvmppc_handle_exit_pr(struct kvm_vcpu *vcpu, unsigned int exit_nr); #ifdef CONFIG_PPC_TRANSACTIONAL_MEM extern void kvmppc_emulate_tabort(struct kvm_vcpu *vcpu, int ra_val); -- 2.31.1
[PATCH 6/6] powerpc/compat_sys: Declare syscalls
This fixes a compile error with W=1. Cc: Christophe Leroy Signed-off-by: Cédric Le Goater --- arch/powerpc/include/asm/syscalls.h | 31 + 1 file changed, 31 insertions(+) diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h index 398171fdcd9f..1d5f2abaa38a 100644 --- a/arch/powerpc/include/asm/syscalls.h +++ b/arch/powerpc/include/asm/syscalls.h @@ -6,6 +6,7 @@ #include #include #include +#include struct rtas_args; @@ -18,5 +19,35 @@ asmlinkage long sys_mmap2(unsigned long addr, size_t len, asmlinkage long ppc64_personality(unsigned long personality); asmlinkage long sys_rtas(struct rtas_args __user *uargs); +#ifdef CONFIG_COMPAT +unsigned long compat_sys_mmap2(unsigned long addr, size_t len, + unsigned long prot, unsigned long flags, + unsigned long fd, unsigned long pgoff); + +compat_ssize_t compat_sys_pread64(unsigned int fd, char __user *ubuf, compat_size_t count, + u32 reg6, u32 pos1, u32 pos2); + +compat_ssize_t compat_sys_pwrite64(unsigned int fd, const char __user *ubuf, compat_size_t count, + u32 reg6, u32 pos1, u32 pos2); + +compat_ssize_t compat_sys_readahead(int fd, u32 r4, u32 offset1, u32 offset2, u32 count); + +asmlinkage int compat_sys_truncate64(const char __user *path, u32 reg4, +unsigned long len1, unsigned long len2); + +asmlinkage long compat_sys_fallocate(int fd, int mode, u32 offset1, u32 offset2, +u32 len1, u32 len2); + +asmlinkage int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned long len1, + unsigned long len2); + +long ppc32_fadvise64(int fd, u32 unused, u32 offset1, u32 offset2, +size_t len, int advice); + +asmlinkage long compat_sys_sync_file_range2(int fd, unsigned int flags, + unsigned int offset1, unsigned int offset2, + unsigned int nbytes1, unsigned int nbytes2); +#endif + #endif /* __KERNEL__ */ #endif /* __ASM_POWERPC_SYSCALLS_H */ -- 2.31.1
[PATCH 5/6] audit: Declare ppc32_classify_syscall()
This fixes a compile error with W=1.
Cc: Christophe Leroy
Signed-off-by: Cédric Le Goater
---
I don't think this is correct. Which file could we use ?
arch/powerpc/include/asm/unistd.h | 3 +++
arch/powerpc/kernel/audit.c | 1 -
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/unistd.h
b/arch/powerpc/include/asm/unistd.h
index b541c690a31c..d9025a7e973c 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -47,6 +47,9 @@
#define __ARCH_WANT_SYS_UTIME
#define __ARCH_WANT_SYS_NEWFSTATAT
#define __ARCH_WANT_COMPAT_SYS_SENDFILE
+#ifdef CONFIG_AUDIT
+extern int ppc32_classify_syscall(unsigned int syscall);
+#endif
#endif
#define __ARCH_WANT_SYS_FORK
#define __ARCH_WANT_SYS_VFORK
diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
index a27f3d09..c3c6c6a1069b 100644
--- a/arch/powerpc/kernel/audit.c
+++ b/arch/powerpc/kernel/audit.c
@@ -41,7 +41,6 @@ int audit_classify_arch(int arch)
int audit_classify_syscall(int abi, unsigned syscall)
{
#ifdef CONFIG_PPC64
- extern int ppc32_classify_syscall(unsigned);
if (abi == AUDIT_ARCH_PPC)
return ppc32_classify_syscall(syscall);
#endif
--
2.31.1
[PATCH 2/6] powerpc/pseries/vas: Declare pseries_vas_fault_thread_fn() as static
This fixes a compile error with W=1.
Fixes: 6d0aaf5e0de0 ("powerpc/pseries/vas: Setup IRQ and fault handling")
Cc: Haren Myneni
Signed-off-by: Cédric Le Goater
---
arch/powerpc/platforms/pseries/vas.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/vas.c
b/arch/powerpc/platforms/pseries/vas.c
index b5c1cf1bc64d..b043e3936d21 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -184,7 +184,7 @@ static int h_get_nx_fault(u32 winid, u64 buffer)
* Note: The hypervisor forwards an interrupt for each fault request.
* So one fault CRB to process for each H_GET_NX_FAULT hcall.
*/
-irqreturn_t pseries_vas_fault_thread_fn(int irq, void *data)
+static irqreturn_t pseries_vas_fault_thread_fn(int irq, void *data)
{
struct pseries_vas_window *txwin = data;
struct coprocessor_request_block crb;
--
2.31.1
[PATCH 0/6] W=1 fixes
Hello, With this small series, I could compile the ppc kernel with W=1. There are certainly other configs which will need more fixes but it's a good start. The last 2 patches look hacky. Christophe, could you help with these to find a better place to include the declarations ? Thanks, C. Cédric Le Goater (6): powerpc/prom: Introduce early_reserve_mem_old() powerpc/pseries/vas: Declare pseries_vas_fault_thread_fn() as static KVM: PPC: Book3S PR: Declare kvmppc_handle_exit_pr() KVM: PPC: Book3S PR: Remove unused variable audit: Declare ppc32_classify_syscall() powerpc/compat_sys: Declare syscalls arch/powerpc/include/asm/syscalls.h | 31 +++ arch/powerpc/include/asm/unistd.h| 3 +++ arch/powerpc/kvm/book3s.h| 1 + arch/powerpc/kernel/audit.c | 1 - arch/powerpc/kernel/prom.c | 37 +++- arch/powerpc/kvm/book3s_64_mmu.c | 3 +-- arch/powerpc/platforms/pseries/vas.c | 2 +- 7 files changed, 57 insertions(+), 21 deletions(-) -- 2.31.1
[PATCH 4/6] KVM: PPC: Book3S PR: Remove unused variable
This fixes a compile error with W=1. Signed-off-by: Cédric Le Goater --- May be, this was sent already ? arch/powerpc/kvm/book3s_64_mmu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/book3s_64_mmu.c b/arch/powerpc/kvm/book3s_64_mmu.c index 26b8b27a3755..feee40cb2ba1 100644 --- a/arch/powerpc/kvm/book3s_64_mmu.c +++ b/arch/powerpc/kvm/book3s_64_mmu.c @@ -196,7 +196,7 @@ static int kvmppc_mmu_book3s_64_xlate(struct kvm_vcpu *vcpu, gva_t eaddr, hva_t ptegp; u64 pteg[16]; u64 avpn = 0; - u64 v, r; + u64 r; u64 v_val, v_mask; u64 eaddr_mask; int i; @@ -285,7 +285,6 @@ static int kvmppc_mmu_book3s_64_xlate(struct kvm_vcpu *vcpu, gva_t eaddr, goto do_second; } - v = be64_to_cpu(pteg[i]); r = be64_to_cpu(pteg[i+1]); pp = (r & HPTE_R_PP) | key; if (r & HPTE_R_PP0) -- 2.31.1
Re: [PATCH 2/3] powerpc: Refactor verification of MSR_RI
Hi Christophe, I love your patch! Perhaps something to improve: [auto build test WARNING on powerpc/next] [also build test WARNING on v5.14-rc6 next-20210819] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-Remove-MSR_PR-check-in-interrupt_exit_-user-kernel-_prepare/20210819-143251 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc-allyesconfig (attached as .config) compiler: powerpc64-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/797b527549df3f1f8e4d9f2bafeb5fe5ec810409 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Christophe-Leroy/powerpc-Remove-MSR_PR-check-in-interrupt_exit_-user-kernel-_prepare/20210819-143251 git checkout 797b527549df3f1f8e4d9f2bafeb5fe5ec810409 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=powerpc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): In file included from arch/powerpc/mm/book3s64/slb.c:13: arch/powerpc/mm/book3s64/slb.c: In function 'do_slb_fault': arch/powerpc/include/asm/interrupt.h:398:29: error: invalid storage class for function 'do_bad_slb_fault' 398 | static __always_inline void ##func(struct pt_regs *regs); \ | ^~~~ arch/powerpc/mm/book3s64/slb.c:872:1: note: in expansion of macro 'DEFINE_INTERRUPT_HANDLER' 872 | DEFINE_INTERRUPT_HANDLER(do_bad_slb_fault) | ^~~~ >> arch/powerpc/include/asm/interrupt.h:398:1: warning: ISO C90 forbids mixed >> declarations and code [-Wdeclaration-after-statement] 398 | static __always_inline void ##func(struct pt_regs *regs); \ | ^~ arch/powerpc/mm/book3s64/slb.c:872:1: note: in expansion of macro 'DEFINE_INTERRUPT_HANDLER' 872 | DEFINE_INTERRUPT_HANDLER(do_bad_slb_fault) | ^~~~ >> arch/powerpc/include/asm/interrupt.h:400:36: warning: 'externally_visible' >> attribute have effect only on public objects [-Wattributes] 400 | interrupt_handler void func(struct pt_regs *regs) \ |^~~ arch/powerpc/mm/book3s64/slb.c:872:1: note: in expansion of macro 'DEFINE_INTERRUPT_HANDLER' 872 | DEFINE_INTERRUPT_HANDLER(do_bad_slb_fault) | ^~~~ arch/powerpc/mm/book3s64/slb.c: In function 'do_bad_slb_fault': arch/powerpc/include/asm/interrupt.h:406:9: error: implicit declaration of function 'do_bad_slb_fault'; did you mean 'do_bad_slb_fault'? [-Werror=implicit-function-declaration] 406 | ##func (regs); \ | ^~~~ arch/powerpc/mm/book3s64/slb.c:872:1: note: in expansion of macro 'DEFINE_INTERRUPT_HANDLER' 872 | DEFINE_INTERRUPT_HANDLER(do_bad_slb_fault) | ^~~~ In file included from arch/powerpc/include/asm/kprobes.h:5, from arch/powerpc/include/asm/interrupt.h:73, from arch/powerpc/mm/book3s64/slb.c:13: arch/powerpc/mm/book3s64/slb.c: In function 'do_slb_fault': include/asm-generic/kprobes.h:14:29: error: initializer element is not constant 14 | _kbl_addr_##fname = (unsigned long)fname; | ^ include/asm-generic/kprobes.h:15:33: note: in expansion of macro '__NOKPROBE_SYMBOL' 15 | # define NOKPROBE_SYMBOL(fname) __NOKPROBE_SYMBOL(fname) | ^ arch/powerpc/include/asm/interrupt.h:410:1: note: in expansion of macro 'NOKPROBE_SYMBOL' 410 | NOKPROBE_SYMBOL(func); \ | ^~~ arch/powerpc/mm/book3s64/slb.c:872:1: note: in expansion of macro 'DEFINE_INTERRUPT_HANDLER' 872 | DEFINE_INTERRUPT_HANDLER(do_bad_slb_fault) | ^~~~ In file included from arch/powerpc/mm/book3s64/slb.c:13: arch/powerpc/include/asm/interrupt.h:412:29: error: invalid storage class for function 'do_bad_slb_fault' 412 | static __always_inline void ##func(struct pt_regs *regs)
Re: [PATCH 1/6] powerpc/prom: Introduce early_reserve_mem_old()
Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
and condition its call with IS_ENABLED(CONFIG_PPC32). This fixes a
compile error with W=1.
arch/powerpc/kernel/prom.c: In function ‘early_reserve_mem’:
arch/powerpc/kernel/prom.c:625:10: error: variable ‘reserve_map’ set but not
used [-Werror=unused-but-set-variable]
__be64 *reserve_map;
^~~
cc1: all warnings being treated as errors
Cc: Christophe Leroy
Signed-off-by: Cédric Le Goater
---
Christophe, I think you had comments on this one ? Yes, I am being a bit lazy.
Yeah, my comment was to leave thing almost as is, just drop the #ifdef CONFIG_PPC32 and instead put
something like:
if (!IS_ENABLED(CONFIG_PPC32))
return;
arch/powerpc/kernel/prom.c | 37 -
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index f620e04dc9bf..52869d12bc1d 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -621,27 +621,14 @@ static void __init early_reserve_mem_dt(void)
}
}
-static void __init early_reserve_mem(void)
+static void __init early_reserve_mem_old(void)
Why old ? Because ppc32 ?
I think that's more changes than needed.
{
__be64 *reserve_map;
reserve_map = (__be64 *)(((unsigned long)initial_boot_params) +
fdt_off_mem_rsvmap(initial_boot_params));
- /* Look for the new "reserved-regions" property in the DT */
- early_reserve_mem_dt();
-
-#ifdef CONFIG_BLK_DEV_INITRD
- /* Then reserve the initrd, if any */
- if (initrd_start && (initrd_end > initrd_start)) {
- memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
- ALIGN(initrd_end, PAGE_SIZE) -
- ALIGN_DOWN(initrd_start, PAGE_SIZE));
- }
-#endif /* CONFIG_BLK_DEV_INITRD */
-
-#ifdef CONFIG_PPC32
- /*
+ /*
* Handle the case where we might be booting from an old kexec
* image that setup the mem_rsvmap as pairs of 32-bit values
*/
@@ -659,9 +646,25 @@ static void __init early_reserve_mem(void)
DBG("reserving: %x -> %x\n", base_32, size_32);
memblock_reserve(base_32, size_32);
}
- return;
}
-#endif
+}
+
+static void __init early_reserve_mem(void)
+{
+ /* Look for the new "reserved-regions" property in the DT */
+ early_reserve_mem_dt();
+
+#ifdef CONFIG_BLK_DEV_INITRD
+ /* Then reserve the initrd, if any */
+ if (initrd_start && (initrd_end > initrd_start)) {
+ memblock_reserve(ALIGN_DOWN(__pa(initrd_start), PAGE_SIZE),
+ ALIGN(initrd_end, PAGE_SIZE) -
+ ALIGN_DOWN(initrd_start, PAGE_SIZE));
+ }
+#endif /* CONFIG_BLK_DEV_INITRD */
+
+ if (IS_ENABLED(CONFIG_PPC32))
+ early_reserve_mem_old();
}
#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
Re: [PATCH 3/6] KVM: PPC: Book3S PR: Declare kvmppc_handle_exit_pr()
Le 19/08/2021 à 14:56, Cédric Le Goater a écrit : This fixes a compile error with W=1. Signed-off-by: Cédric Le Goater --- arch/powerpc/kvm/book3s.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h index 740e51def5a5..c08f93b7f523 100644 --- a/arch/powerpc/kvm/book3s.h +++ b/arch/powerpc/kvm/book3s.h @@ -24,6 +24,7 @@ extern int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val); extern int kvmppc_book3s_init_pr(void); extern void kvmppc_book3s_exit_pr(void); +extern int kvmppc_handle_exit_pr(struct kvm_vcpu *vcpu, unsigned int exit_nr); Don't add new 'extern' keywords, they are useless and pointless for functions prototypes. #ifdef CONFIG_PPC_TRANSACTIONAL_MEM extern void kvmppc_emulate_tabort(struct kvm_vcpu *vcpu, int ra_val);
Re: [PATCH 5/6] audit: Declare ppc32_classify_syscall()
Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
This fixes a compile error with W=1.
Cc: Christophe Leroy
Signed-off-by: Cédric Le Goater
---
I don't think this is correct. Which file could we use ?
I think you can completely remove ppc32_classify_syscall(), and instead add the following in the
default case in audit_classify_syscall():
default:
+ if (IS_ENABLED(CONFIG_PPC64) && abi == AUDIT_ARCH_PPC)
+ return 1;
return 0;
arch/powerpc/include/asm/unistd.h | 3 +++
arch/powerpc/kernel/audit.c | 1 -
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/unistd.h
b/arch/powerpc/include/asm/unistd.h
index b541c690a31c..d9025a7e973c 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -47,6 +47,9 @@
#define __ARCH_WANT_SYS_UTIME
#define __ARCH_WANT_SYS_NEWFSTATAT
#define __ARCH_WANT_COMPAT_SYS_SENDFILE
+#ifdef CONFIG_AUDIT
+extern int ppc32_classify_syscall(unsigned int syscall);
+#endif
#endif
#define __ARCH_WANT_SYS_FORK
#define __ARCH_WANT_SYS_VFORK
diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
index a27f3d09..c3c6c6a1069b 100644
--- a/arch/powerpc/kernel/audit.c
+++ b/arch/powerpc/kernel/audit.c
@@ -41,7 +41,6 @@ int audit_classify_arch(int arch)
int audit_classify_syscall(int abi, unsigned syscall)
{
#ifdef CONFIG_PPC64
- extern int ppc32_classify_syscall(unsigned);
if (abi == AUDIT_ARCH_PPC)
return ppc32_classify_syscall(syscall);
#endif
[PATCH v2 1/3] powerpc: Remove MSR_PR check in interrupt_exit_{user/kernel}_prepare()
In those hot functions that are called at every interrupt, any saved cycle is worth it. interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare() are called from three places: - From entry_32.S - From interrupt_64.S - From interrupt_exit_user_restart() and interrupt_exit_kernel_restart() In entry_32.S, there are inambiguously called based on MSR_PR: interrupt_return: lwz r4,_MSR(r1) addir3,r1,STACK_FRAME_OVERHEAD andi. r0,r4,MSR_PR beq .Lkernel_interrupt_return bl interrupt_exit_user_prepare ... .Lkernel_interrupt_return: bl interrupt_exit_kernel_prepare In interrupt_64.S, that's similar: interrupt_return_\srr\(): ld r4,_MSR(r1) andi. r0,r4,MSR_PR beq interrupt_return_\srr\()_kernel interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */ addir3,r1,STACK_FRAME_OVERHEAD bl interrupt_exit_user_prepare ... interrupt_return_\srr\()_kernel: addir3,r1,STACK_FRAME_OVERHEAD bl interrupt_exit_kernel_prepare In interrupt_exit_user_restart() and interrupt_exit_kernel_restart(), MSR_PR is verified respectively by BUG_ON(!user_mode(regs)) and BUG_ON(user_mode(regs)) prior to calling interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare(). The verification in interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare() are therefore useless and can be removed. Signed-off-by: Christophe Leroy Acked-by: Nicholas Piggin --- arch/powerpc/kernel/interrupt.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c index 21bbd615ca41..f26caf911ab5 100644 --- a/arch/powerpc/kernel/interrupt.c +++ b/arch/powerpc/kernel/interrupt.c @@ -465,7 +465,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs) if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x)) BUG_ON(!(regs->msr & MSR_RI)); - BUG_ON(!(regs->msr & MSR_PR)); BUG_ON(arch_irq_disabled_regs(regs)); CT_WARN_ON(ct_state() == CONTEXT_USER); @@ -499,7 +498,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs) if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) && unlikely(!(regs->msr & MSR_RI))) unrecoverable_exception(regs); - BUG_ON(regs->msr & MSR_PR); /* * CT_WARN_ON comes here via program_check_exception, * so avoid recursion. -- 2.25.0
[PATCH v2 2/3] powerpc: Refactor verification of MSR_RI
40x and BOOKE don't have MSR_RI therefore all tests involving
MSR_RI may be problematic on those plateforms.
Create helpers to check or set MSR_RI in regs, and use them
in common code.
Signed-off-by: Christophe Leroy
---
v2: Remove superflous {
---
arch/powerpc/include/asm/ptrace.h | 23 +++
arch/powerpc/kernel/interrupt.c | 9 +++-
arch/powerpc/kernel/traps.c | 8 +++
arch/powerpc/mm/book3s64/slb.c| 2 +-
arch/powerpc/platforms/embedded6xx/holly.c| 2 +-
.../platforms/embedded6xx/mpc7448_hpc2.c | 2 +-
arch/powerpc/platforms/pasemi/idle.c | 2 +-
arch/powerpc/platforms/powernv/opal.c | 2 +-
arch/powerpc/platforms/pseries/ras.c | 2 +-
arch/powerpc/sysdev/fsl_rio.c | 2 +-
arch/powerpc/xmon/xmon.c | 16 +++--
11 files changed, 40 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h
b/arch/powerpc/include/asm/ptrace.h
index fd60538737a0..0cdb7b9c2c9c 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#ifndef __ASSEMBLY__
struct pt_regs
@@ -282,6 +283,28 @@ static inline void regs_set_return_value(struct pt_regs
*regs, unsigned long rc)
regs->gpr[3] = rc;
}
+static inline bool cpu_has_msr_ri(void)
+{
+ return !IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x);
+}
+
+static inline bool regs_is_unrecoverable(struct pt_regs *regs)
+{
+ return unlikely(cpu_has_msr_ri() && !(regs->msr & MSR_RI));
+}
+
+static inline void regs_set_recoverable(struct pt_regs *regs)
+{
+ if (cpu_has_msr_ri())
+ regs_set_return_msr(regs, regs->msr | MSR_RI);
+}
+
+static inline void regs_set_unrecoverable(struct pt_regs *regs)
+{
+ if (cpu_has_msr_ri())
+ regs_set_return_msr(regs, regs->msr & ~MSR_RI);
+}
+
#define arch_has_single_step() (1)
#define arch_has_block_step() (true)
#define ARCH_HAS_USER_SINGLE_STEP_REPORT
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index f26caf911ab5..f06c38e8fe36 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -93,8 +93,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
CT_WARN_ON(ct_state() == CONTEXT_KERNEL);
user_exit_irqoff();
- if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
- BUG_ON(!(regs->msr & MSR_RI));
+ BUG_ON(regs_is_unrecoverable(regs));
BUG_ON(!(regs->msr & MSR_PR));
BUG_ON(arch_irq_disabled_regs(regs));
@@ -463,8 +462,7 @@ notrace unsigned long interrupt_exit_user_prepare(struct
pt_regs *regs)
{
unsigned long ret;
- if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
- BUG_ON(!(regs->msr & MSR_RI));
+ BUG_ON(regs_is_unrecoverable(regs));
BUG_ON(arch_irq_disabled_regs(regs));
CT_WARN_ON(ct_state() == CONTEXT_USER);
@@ -495,8 +493,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct
pt_regs *regs)
bool stack_store = current_thread_info()->flags &
_TIF_EMULATE_STACK_STORE;
- if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
- unlikely(!(regs->msr & MSR_RI)))
+ if (regs_is_unrecoverable(regs))
unrecoverable_exception(regs);
/*
* CT_WARN_ON comes here via program_check_exception,
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 3e2adb3487e7..8310147b5e7b 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -428,7 +428,7 @@ void hv_nmi_check_nonrecoverable(struct pt_regs *regs)
return;
nonrecoverable:
- regs_set_return_msr(regs, regs->msr & ~MSR_RI);
+ regs_set_unrecoverable(regs);
#endif
}
DEFINE_INTERRUPT_HANDLER_NMI(system_reset_exception)
@@ -498,7 +498,7 @@ DEFINE_INTERRUPT_HANDLER_NMI(system_reset_exception)
die("Unrecoverable nested System Reset", regs, SIGABRT);
#endif
/* Must die if the interrupt is not recoverable */
- if (!(regs->msr & MSR_RI)) {
+ if (regs_is_unrecoverable(regs)) {
/* For the reason explained in die_mce, nmi_exit before die */
nmi_exit();
die("Unrecoverable System Reset", regs, SIGABRT);
@@ -550,7 +550,7 @@ static inline int check_io_access(struct pt_regs *regs)
printk(KERN_DEBUG "%s bad port %lx at %p\n",
(*nip & 0x100)? "OUT to": "IN from",
regs->gpr[rb] - _IO_BASE, nip);
- regs_set_return_msr(regs, regs->msr | MSR_RI);
+ regs_set_recoverable(regs);
regs_set_return_ip(regs, extable_fixup(entry));
return 1;
[PATCH v2 3/3] powerpc: Define and use MSR_RI only on non booke/40x
40x and BOOKE don't have MSR_RI.
Define MSR_RI only for platforms where it exists. For the other ones,
defines it as BUILD_BUG for C and do not define it for ASM.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/reg.h | 4
arch/powerpc/include/asm/reg_booke.h | 6 +++---
arch/powerpc/kernel/head_32.h| 4
arch/powerpc/kernel/process.c| 2 +-
arch/powerpc/lib/sstep.c | 2 +-
5 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index be85cf156a1f..656a9aaa1e8e 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -109,7 +109,11 @@
#ifndef MSR_PMM
#define MSR_PMM__MASK(MSR_PMM_LG) /* Performance monitor
*/
#endif
+#if !defined(CONFIG_BOOKE) && !defined(CONFIG_40x)
#define MSR_RI __MASK(MSR_RI_LG) /* Recoverable Exception */
+#elif !defined(__ASSEMBLY__)
+#define MSR_RI ({BUILD_BUG(); 0; })
+#endif
#define MSR_LE __MASK(MSR_LE_LG) /* Little Endian */
#define MSR_TM __MASK(MSR_TM_LG) /* Transactional Mem Available
*/
diff --git a/arch/powerpc/include/asm/reg_booke.h
b/arch/powerpc/include/asm/reg_booke.h
index 17b8dcd9a40d..6f40a8420ad0 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -38,15 +38,15 @@
#if defined(CONFIG_PPC_BOOK3E_64)
#define MSR_64BIT MSR_CM
-#define MSR_ (MSR_ME | MSR_RI | MSR_CE)
+#define MSR_ (MSR_ME | MSR_CE)
#define MSR_KERNEL (MSR_ | MSR_64BIT)
#define MSR_USER32 (MSR_ | MSR_PR | MSR_EE)
#define MSR_USER64 (MSR_USER32 | MSR_64BIT)
#elif defined (CONFIG_40x)
-#define MSR_KERNEL (MSR_ME|MSR_RI|MSR_IR|MSR_DR|MSR_CE)
+#define MSR_KERNEL (MSR_ME|MSR_IR|MSR_DR|MSR_CE)
#define MSR_USER (MSR_KERNEL|MSR_PR|MSR_EE)
#else
-#define MSR_KERNEL (MSR_ME|MSR_RI|MSR_CE)
+#define MSR_KERNEL (MSR_ME|MSR_CE)
#define MSR_USER (MSR_KERNEL|MSR_PR|MSR_EE)
#endif
diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index 6b1ec9e3541b..6c5f4183dc8e 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -63,7 +63,11 @@
mtspr SPRN_DAR, r11 /* Tag DAR, to be used in DTLB Error */
.endif
#endif
+#ifdef CONFIG_40x
+ LOAD_REG_IMMEDIATE(r11, MSR_KERNEL) /* re-enable MMU */
+#else
LOAD_REG_IMMEDIATE(r11, MSR_KERNEL & ~MSR_RI) /* re-enable MMU */
+#endif
mtspr SPRN_SRR1, r11
lis r11, 1f@h
ori r11, r11, 1f@l
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 185beb290580..5ba72e31de28 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1420,7 +1420,7 @@ static struct regbit msr_bits[] = {
{MSR_IR,"IR"},
{MSR_DR,"DR"},
{MSR_PMM, "PMM"},
-#ifndef CONFIG_BOOKE
+#if !defined(CONFIG_BOOKE) && !defined(CONFIG_40x)
{MSR_RI,"RI"},
{MSR_LE,"LE"},
#endif
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index d8d5f901cee1..357cc1fb4f67 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -3559,7 +3559,7 @@ int emulate_step(struct pt_regs *regs, struct ppc_inst
instr)
case MTMSR:
val = regs->gpr[op.reg];
- if ((val & MSR_RI) == 0)
+ if (cpu_has_msr_ri() && (val & MSR_RI) == 0)
/* can't step mtmsr[d] that would clear MSR_RI */
return -1;
/* here op.val is the mask of bits to change */
--
2.25.0
Re: [PATCH v2 02/12] mm: Introduce a function to check for virtualization protection features
On 8/19/21 4:46 AM, Christoph Hellwig wrote: > On Fri, Aug 13, 2021 at 11:59:21AM -0500, Tom Lendacky wrote: >> +#define PATTR_MEM_ENCRYPT 0 /* Encrypted memory */ >> +#define PATTR_HOST_MEM_ENCRYPT 1 /* Host encrypted >> memory */ >> +#define PATTR_GUEST_MEM_ENCRYPT 2 /* Guest encrypted >> memory */ >> +#define PATTR_GUEST_PROT_STATE 3 /* Guest encrypted >> state */ > > Please write an actual detailed explanaton of what these mean, that > is what implications it has on the kernel. Will do. Thanks, Tom >
Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
On Thu, Aug 19, 2021 at 10:52:53AM +0100, Christoph Hellwig wrote: > Which suggest that the name is not good to start with. Maybe protected > hardware, system or platform might be a better choice? Yah, coming up with a proper name here hasn't been easy. prot_guest_has() is not the first variant. >From all three things you suggest above, I guess calling it a "platform" is the closest. As in, this is a confidential computing platform which provides host and guest facilities etc. So calling it confidential_computing_platform_has() is obviously too long. ccp_has() clashes with the namespace of drivers/crypto/ccp/ which is used by the technology too. coco_platform_has() is too unserious. So I guess cc_platform_has() ain't all that bad. Unless you have a better idea, ofc. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
On 8/19/21 4:52 AM, Christoph Hellwig wrote:
> On Fri, Aug 13, 2021 at 11:59:22AM -0500, Tom Lendacky wrote:
>> While the name suggests this is intended mainly for guests, it will
>> also be used for host memory encryption checks in place of sme_active().
>
> Which suggest that the name is not good to start with. Maybe protected
> hardware, system or platform might be a better choice?
>
>> +static inline bool prot_guest_has(unsigned int attr)
>> +{
>> +#ifdef CONFIG_AMD_MEM_ENCRYPT
>> +if (sme_me_mask)
>> +return amd_prot_guest_has(attr);
>> +#endif
>> +
>> +return false;
>> +}
>
> Shouldn't this be entirely out of line?
I did it as inline originally because the presence of the function will be
decided based on the ARCH_HAS_PROTECTED_GUEST config. For now, that is
only selected by the AMD memory encryption support, so if I went out of
line I could put in mem_encrypt.c. But with TDX wanting to also use it, it
would have to be in an always built file with some #ifdefs or in its own
file that is conditionally built based on the ARCH_HAS_PROTECTED_GUEST
setting (they've already tried building with ARCH_HAS_PROTECTED_GUEST=y
and AMD_MEM_ENCRYPT not set).
To take it out of line, I'm leaning towards the latter, creating a new
file that is built based on the ARCH_HAS_PROTECTED_GUEST setting.
>
>> +/* 0x800 - 0x8ff reserved for AMD */
>> +#define PATTR_SME 0x800
>> +#define PATTR_SEV 0x801
>> +#define PATTR_SEV_ES0x802
>
> Why do we need reservations for a purely in-kernel namespace?
>
> And why are you overoading a brand new generic API with weird details
> of a specific implementation like this?
There was some talk about this on the mailing list where TDX and SEV may
need to be differentiated, so we wanted to reserve a range of values per
technology. I guess I can remove them until they are actually needed.
Thanks,
Tom
>
Re: [PATCH v2 04/12] powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
On 8/19/21 4:55 AM, Christoph Hellwig wrote: > On Fri, Aug 13, 2021 at 11:59:23AM -0500, Tom Lendacky wrote: >> +static inline bool prot_guest_has(unsigned int attr) > > No reall need to have this inline. In fact I'd suggest we havea the > prototype in a common header so that everyone must implement it out > of line. I'll do the same thing I end up doing for x86. Thanks, Tom >
Re: [PATCH 5/6] audit: Declare ppc32_classify_syscall()
Le 19/08/2021 à 16:56, Christophe Leroy a écrit :
Le 19/08/2021 à 14:56, Cédric Le Goater a écrit :
This fixes a compile error with W=1.
Cc: Christophe Leroy
Signed-off-by: Cédric Le Goater
---
I don't think this is correct. Which file could we use ?
I think you can completely remove ppc32_classify_syscall(), and instead add the following in the
default case in audit_classify_syscall():
default:
+ if (IS_ENABLED(CONFIG_PPC64) && abi == AUDIT_ARCH_PPC)
+ return 1;
return 0;
After looking more in details, in fact I think we should convert powerpc to
CONFIG_AUDIT_ARCH_COMPAT_GENERIC
arch/powerpc/include/asm/unistd.h | 3 +++
arch/powerpc/kernel/audit.c | 1 -
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/unistd.h
b/arch/powerpc/include/asm/unistd.h
index b541c690a31c..d9025a7e973c 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -47,6 +47,9 @@
#define __ARCH_WANT_SYS_UTIME
#define __ARCH_WANT_SYS_NEWFSTATAT
#define __ARCH_WANT_COMPAT_SYS_SENDFILE
+#ifdef CONFIG_AUDIT
+extern int ppc32_classify_syscall(unsigned int syscall);
+#endif
#endif
#define __ARCH_WANT_SYS_FORK
#define __ARCH_WANT_SYS_VFORK
diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
index a27f3d09..c3c6c6a1069b 100644
--- a/arch/powerpc/kernel/audit.c
+++ b/arch/powerpc/kernel/audit.c
@@ -41,7 +41,6 @@ int audit_classify_arch(int arch)
int audit_classify_syscall(int abi, unsigned syscall)
{
#ifdef CONFIG_PPC64
- extern int ppc32_classify_syscall(unsigned);
if (abi == AUDIT_ARCH_PPC)
return ppc32_classify_syscall(syscall);
#endif
Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()
On 8/19/21 11:33 AM, Tom Lendacky wrote: There was some talk about this on the mailing list where TDX and SEV may need to be differentiated, so we wanted to reserve a range of values per technology. I guess I can remove them until they are actually needed. In TDX also we have similar requirements and we need some flags for TDX specific checks. So I think it is fine to leave some space for vendor flags. -- Sathyanarayanan Kuppuswamy Linux Kernel Developer
Re: [PATCH 3/7] powerpc: replace cc-option-yn uses with cc-option
On Tue, Aug 17, 2021 at 11:31 AM Michael Ellerman wrote: > > Nick Desaulniers writes: > > cc-option-yn can be replaced with cc-option. ie. > > Checking for support: > > ifeq ($(call cc-option-yn,$(FLAG)),y) > > becomes: > > ifneq ($(call cc-option,$(FLAG)),) > > > > Checking for lack of support: > > ifeq ($(call cc-option-yn,$(FLAG)),n) > > becomes: > > ifeq ($(call cc-option,$(FLAG)),) > > > > This allows us to pursue removing cc-option-yn. > > > > Cc: Michael Ellerman > > Cc: Benjamin Herrenschmidt > > Cc: Paul Mackerras > > Cc: linuxppc-dev@lists.ozlabs.org > > Signed-off-by: Nick Desaulniers > > --- > > arch/powerpc/Makefile | 12 ++-- > > arch/powerpc/boot/Makefile | 5 + > > 2 files changed, 7 insertions(+), 10 deletions(-) > > > > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile > > index 9aaf1abbc641..85e224536cf7 100644 > > --- a/arch/powerpc/Makefile > > +++ b/arch/powerpc/Makefile > > @@ -12,12 +12,12 @@ > > # Rewritten by Cort Dougan and Paul Mackerras > > # > > > > -HAS_BIARCH := $(call cc-option-yn, -m32) > > +HAS_BIARCH := $(call cc-option,-m32) > > > > # Set default 32 bits cross compilers for vdso and boot wrapper > > CROSS32_COMPILE ?= > > > > -ifeq ($(HAS_BIARCH),y) > > +ifeq ($(HAS_BIARCH),-m32) > > I don't love that we have to repeat "-m32" in each check. > > I'm pretty sure you can use ifdef here, because HAS_BIARCH is a simple > variable (assigned with ":="). > > ie, this can be: > > ifdef HAS_BIARCH > > > And that avoids having to spell out "-m32" everywhere. > > cheers Yes. Comments from Nathan and Michael both sound good. -- Best Regards Masahiro Yamada
Re: [PATCH 2/5] entry: rseq: Call rseq_handle_notify_resume() in tracehook_notify_resume()
- On Aug 17, 2021, at 8:12 PM, Sean Christopherson sea...@google.com wrote:
> Invoke rseq_handle_notify_resume() from tracehook_notify_resume() now
> that the two function are always called back-to-back by architectures
> that have rseq. The rseq helper is stubbed out for architectures that
> don't support rseq, i.e. this is a nop across the board.
>
> Note, tracehook_notify_resume() is horribly named and arguably does not
> belong in tracehook.h as literally every line of code in it has nothing
> to do with tracing. But, that's been true since commit a42c6ded827d
> ("move key_repace_session_keyring() into tracehook_notify_resume()")
> first usurped tracehook_notify_resume() back in 2012. Punt cleaning that
> mess up to future patches.
>
> No functional change intended.
This will make it harder to introduce new code paths which consume the
NOTIFY_RESUME without calling the rseq callback, which introduces issues.
Agreed.
Acked-by: Mathieu Desnoyers
>
> Signed-off-by: Sean Christopherson
> ---
> arch/arm/kernel/signal.c | 1 -
> arch/arm64/kernel/signal.c | 1 -
> arch/csky/kernel/signal.c| 4 +---
> arch/mips/kernel/signal.c| 4 +---
> arch/powerpc/kernel/signal.c | 4 +---
> arch/s390/kernel/signal.c| 1 -
> include/linux/tracehook.h| 2 ++
> kernel/entry/common.c| 4 +---
> kernel/entry/kvm.c | 4 +---
> 9 files changed, 7 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
> index a3a38d0a4c85..9df68d139965 100644
> --- a/arch/arm/kernel/signal.c
> +++ b/arch/arm/kernel/signal.c
> @@ -670,7 +670,6 @@ do_work_pending(struct pt_regs *regs, unsigned int
> thread_flags, int syscall)
> uprobe_notify_resume(regs);
> } else {
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
> }
> }
> local_irq_disable();
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> index 23036334f4dc..22b55db13da6 100644
> --- a/arch/arm64/kernel/signal.c
> +++ b/arch/arm64/kernel/signal.c
> @@ -951,7 +951,6 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
>
> if (thread_flags & _TIF_NOTIFY_RESUME) {
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
>
> /*
>* If we reschedule after checking the affinity
> diff --git a/arch/csky/kernel/signal.c b/arch/csky/kernel/signal.c
> index 312f046d452d..bc4238b9f709 100644
> --- a/arch/csky/kernel/signal.c
> +++ b/arch/csky/kernel/signal.c
> @@ -260,8 +260,6 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
> if (thread_info_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> do_signal(regs);
>
> - if (thread_info_flags & _TIF_NOTIFY_RESUME) {
> + if (thread_info_flags & _TIF_NOTIFY_RESUME)
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
> - }
> }
> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
> index f1e985109da0..c9b2a75563e1 100644
> --- a/arch/mips/kernel/signal.c
> +++ b/arch/mips/kernel/signal.c
> @@ -906,10 +906,8 @@ asmlinkage void do_notify_resume(struct pt_regs *regs,
> void
> *unused,
> if (thread_info_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
> do_signal(regs);
>
> - if (thread_info_flags & _TIF_NOTIFY_RESUME) {
> + if (thread_info_flags & _TIF_NOTIFY_RESUME)
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
> - }
>
> user_enter();
> }
> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> index e600764a926c..b93b87df499d 100644
> --- a/arch/powerpc/kernel/signal.c
> +++ b/arch/powerpc/kernel/signal.c
> @@ -293,10 +293,8 @@ void do_notify_resume(struct pt_regs *regs, unsigned long
> thread_info_flags)
> do_signal(current);
> }
>
> - if (thread_info_flags & _TIF_NOTIFY_RESUME) {
> + if (thread_info_flags & _TIF_NOTIFY_RESUME)
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
> - }
> }
>
> static unsigned long get_tm_stackpointer(struct task_struct *tsk)
> diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c
> index 78ef53b29958..b307db26bf2d 100644
> --- a/arch/s390/kernel/signal.c
> +++ b/arch/s390/kernel/signal.c
> @@ -537,5 +537,4 @@ void arch_do_signal_or_restart(struct pt_regs *regs, bool
> has_signal)
> void do_notify_resume(struct pt_regs *regs)
> {
> tracehook_notify_resume(regs);
> - rseq_handle_notify_resume(NULL, regs);
> }
> diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
> index 3e80c4bc66f7..2564b7434b4d 100644
> --- a/include/linux/tracehook
Re: [PATCH 1/5] KVM: rseq: Update rseq when processing NOTIFY_RESUME on xfer to KVM guest
- On Aug 17, 2021, at 8:12 PM, Sean Christopherson sea...@google.com wrote:
> Invoke rseq's NOTIFY_RESUME handler when processing the flag prior to
> transferring to a KVM guest, which is roughly equivalent to an exit to
> userspace and processes many of the same pending actions. While the task
> cannot be in an rseq critical section as the KVM path is reachable only
> via ioctl(KVM_RUN), the side effects that apply to rseq outside of a
> critical section still apply, e.g. the CPU ID needs to be updated if the
> task is migrated.
>
> Clearing TIF_NOTIFY_RESUME without informing rseq can lead to segfaults
> and other badness in userspace VMMs that use rseq in combination with KVM,
> e.g. due to the CPU ID being stale after task migration.
I agree with the problem assessment, but I would recommend a small change
to this fix.
>
> Fixes: 72c3c0fe54a3 ("x86/kvm: Use generic xfer to guest work function")
> Reported-by: Peter Foley
> Bisected-by: Doug Evans
> Cc: Shakeel Butt
> Cc: Thomas Gleixner
> Cc: sta...@vger.kernel.org
> Signed-off-by: Sean Christopherson
> ---
> kernel/entry/kvm.c | 4 +++-
> kernel/rseq.c | 4 ++--
> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/entry/kvm.c b/kernel/entry/kvm.c
> index 49972ee99aff..049fd06b4c3d 100644
> --- a/kernel/entry/kvm.c
> +++ b/kernel/entry/kvm.c
> @@ -19,8 +19,10 @@ static int xfer_to_guest_mode_work(struct kvm_vcpu *vcpu,
> unsigned long ti_work)
> if (ti_work & _TIF_NEED_RESCHED)
> schedule();
>
> - if (ti_work & _TIF_NOTIFY_RESUME)
> + if (ti_work & _TIF_NOTIFY_RESUME) {
> tracehook_notify_resume(NULL);
> + rseq_handle_notify_resume(NULL, NULL);
> + }
>
> ret = arch_xfer_to_guest_mode_handle_work(vcpu, ti_work);
> if (ret)
> diff --git a/kernel/rseq.c b/kernel/rseq.c
> index 35f7bd0fced0..58c79a7918cd 100644
> --- a/kernel/rseq.c
> +++ b/kernel/rseq.c
> @@ -236,7 +236,7 @@ static bool in_rseq_cs(unsigned long ip, struct rseq_cs
> *rseq_cs)
>
> static int rseq_ip_fixup(struct pt_regs *regs)
> {
> - unsigned long ip = instruction_pointer(regs);
> + unsigned long ip = regs ? instruction_pointer(regs) : 0;
> struct task_struct *t = current;
> struct rseq_cs rseq_cs;
> int ret;
> @@ -250,7 +250,7 @@ static int rseq_ip_fixup(struct pt_regs *regs)
>* If not nested over a rseq critical section, restart is useless.
>* Clear the rseq_cs pointer and return.
>*/
> - if (!in_rseq_cs(ip, &rseq_cs))
> + if (!regs || !in_rseq_cs(ip, &rseq_cs))
I think clearing the thread's rseq_cs unconditionally here when regs is NULL
is not the behavior we want when this is called from xfer_to_guest_mode_work.
If we have a scenario where userspace ends up calling this ioctl(KVM_RUN)
from within a rseq c.s., we really want a CONFIG_DEBUG_RSEQ=y kernel to
kill this application in the rseq_syscall handler when exiting back to usermode
when the ioctl eventually returns.
However, clearing the thread's rseq_cs will prevent this from happening.
So I would favor an approach where we simply do:
if (!regs)
return 0;
Immediately at the beginning of rseq_ip_fixup, before getting the instruction
pointer, so effectively skip all side-effects of the ip fixup code. Indeed, it
is not relevant to do any fixup here, because it is nested in a ioctl system
call.
Effectively, this would preserve the SIGSEGV behavior when this ioctl is
erroneously called by user-space from a rseq critical section.
Thanks for looking into this !
Mathieu
> return clear_rseq_cs(t);
> ret = rseq_need_restart(t, rseq_cs.flags);
> if (ret <= 0)
> --
> 2.33.0.rc1.237.g0d66db33f3-goog
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Re: [PATCH 4/5] KVM: selftests: Add a test for KVM_RUN+rseq to detect task migration bugs
- On Aug 17, 2021, at 8:12 PM, Sean Christopherson sea...@google.com wrote:
> Add a test to verify an rseq's CPU ID is updated correctly if the task is
> migrated while the kernel is handling KVM_RUN. This is a regression test
> for a bug introduced by commit 72c3c0fe54a3 ("x86/kvm: Use generic xfer
> to guest work function"), where TIF_NOTIFY_RESUME would be cleared by KVM
> without updating rseq, leading to a stale CPU ID and other badness.
>
> Signed-off-by: Sean Christopherson
> ---
[...]
> +
> +static void *migration_worker(void *ign)
> +{
> + cpu_set_t allowed_mask;
> + int r, i, nr_cpus, cpu;
> +
> + CPU_ZERO(&allowed_mask);
> +
> + nr_cpus = CPU_COUNT(&possible_mask);
> +
> + for (i = 0; i < 2; i++) {
> + cpu = i % nr_cpus;
> + if (!CPU_ISSET(cpu, &possible_mask))
> + continue;
> +
> + CPU_SET(cpu, &allowed_mask);
> +
> + r = sched_setaffinity(0, sizeof(allowed_mask), &allowed_mask);
> + TEST_ASSERT(!r, "sched_setaffinity failed, errno = %d (%s)",
> errno,
> + strerror(errno));
> +
> + CPU_CLR(cpu, &allowed_mask);
> +
> + usleep(10);
> + }
> + done = true;
> + return NULL;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + struct kvm_vm *vm;
> + u32 cpu, rseq_cpu;
> + int r;
> +
> + /* Tell stdout not to buffer its content */
> + setbuf(stdout, NULL);
> +
> + r = sched_getaffinity(0, sizeof(possible_mask), &possible_mask);
> + TEST_ASSERT(!r, "sched_getaffinity failed, errno = %d (%s)", errno,
> + strerror(errno));
> +
> + if (CPU_COUNT(&possible_mask) < 2) {
> + print_skip("Only one CPU, task migration not possible\n");
> + exit(KSFT_SKIP);
> + }
> +
> + sys_rseq(0);
> +
> + /*
> + * Create and run a dummy VM that immediately exits to userspace via
> + * GUEST_SYNC, while concurrently migrating the process by setting its
> + * CPU affinity.
> + */
> + vm = vm_create_default(VCPU_ID, 0, guest_code);
> +
> + pthread_create(&migration_thread, NULL, migration_worker, 0);
> +
> + while (!done) {
> + vcpu_run(vm, VCPU_ID);
> + TEST_ASSERT(get_ucall(vm, VCPU_ID, NULL) == UCALL_SYNC,
> + "Guest failed?");
> +
> + cpu = sched_getcpu();
> + rseq_cpu = READ_ONCE(__rseq.cpu_id);
> +
> + /*
> + * Verify rseq's CPU matches sched's CPU, and that sched's CPU
> + * is stable. This doesn't handle the case where the task is
> + * migrated between sched_getcpu() and reading rseq, and again
> + * between reading rseq and sched_getcpu(), but in practice no
> + * false positives have been observed, while on the other hand
> + * blocking migration while this thread reads CPUs messes with
> + * the timing and prevents hitting failures on a buggy kernel.
> + */
I think you could get a stable cpu id between sched_getcpu and __rseq_abi.cpu_id
if you add a pthread mutex to protect:
sched_getcpu and __rseq_abi.cpu_id reads
vs
sched_setaffinity calls within the migration thread.
Thoughts ?
Thanks,
Mathieu
> + TEST_ASSERT(rseq_cpu == cpu || cpu != sched_getcpu(),
> + "rseq CPU = %d, sched CPU = %d\n", rseq_cpu, cpu);
> + }
> +
> + pthread_join(migration_thread, NULL);
> +
> + kvm_vm_free(vm);
> +
> + sys_rseq(RSEQ_FLAG_UNREGISTER);
> +
> + return 0;
> +}
> --
> 2.33.0.rc1.237.g0d66db33f3-goog
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Re: [PATCH v2 1/2] powerpc: kvm: remove obsolete and unneeded select
Hi Lukas, > diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig > index e45644657d49..ff581d70f20c 100644 > --- a/arch/powerpc/kvm/Kconfig > +++ b/arch/powerpc/kvm/Kconfig > @@ -38,7 +38,6 @@ config KVM_BOOK3S_32_HANDLER > config KVM_BOOK3S_64_HANDLER > bool > select KVM_BOOK3S_HANDLER > - select PPC_DAWR_FORCE_ENABLE I looked at some of the history here. It looks like this select was left over from an earlier version of the patch series that added PPC_DAWR: v2 of the series has a new symbol PPC_DAWR_FORCE_ENABLE but by version 4 that new symbol had disappeared but the select had not. v2: https://lore.kernel.org/linuxppc-dev/20190513071703.25243-1-mi...@neuling.org/ v5: https://lore.kernel.org/linuxppc-dev/20190604030037.9424-2-mi...@neuling.org/ The rest of the patch reasoning makes sense to me: DAWR support will be selected anyway by virtue of PPC64->PPC_DAWR so there's no need to try to select it again anyway. Reviewed-by: Daniel Axtens Kind regards, Daniel > > config KVM_BOOK3S_PR_POSSIBLE > bool > -- > 2.26.2
Re: [PATCH v2 2/2] powerpc: rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK
Lukas Bulwahn writes:
> Commit 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
> selects the non-existing config ARCH_ENABLE_PMD_SPLIT_PTLOCK in
> ./arch/powerpc/platforms/Kconfig.cputype, but clearly it intends to select
> ARCH_ENABLE_SPLIT_PMD_PTLOCK here (notice the word swapping!), as this
> commit does select that for all other architectures.
>
> Rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK instead.
>
Yikes, yes, 66f24fa766e3 does seem to have got that wrong. It looks like
that went into 5.13.
I think we want to specifically target this for stable so that we don't
lose the perfomance and scalability benefits of split pmd ptlocks:
Cc: sta...@vger.kernel.org # v5.13+
(I don't think you need to do another revision for this, I think mpe
could add it when merging.)
I tried to check whether we accidentally broke SPLIT_PMD_PTLOCKs while
they were disabled:
- There hasn't been any change to the pgtable_pmd_page_ctor or _dtor
prototypes, and we haven't made any relevant changes to any of the
files in arch/powerpc that called it.
- I checked out v5.13 and powerpc/merge, applied this patch, built a
pseries_le_defconfig and boot tested it in qemu. It didn't crash on
boot or with /bin/sh and some shell commands, but I didn't exactly
stress test the VM subsystem either.
This gives me some confidence it's both good for powerpc and stable-worthy.
Overall:
Reviewed-by: Daniel Axtens
Kind regards,
Daniel
> Fixes: 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
> Signed-off-by: Lukas Bulwahn
> ---
> arch/powerpc/platforms/Kconfig.cputype | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/Kconfig.cputype
> b/arch/powerpc/platforms/Kconfig.cputype
> index 6794145603de..a208997ade88 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -98,7 +98,7 @@ config PPC_BOOK3S_64
> select PPC_HAVE_PMU_SUPPORT
> select HAVE_ARCH_TRANSPARENT_HUGEPAGE
> select ARCH_ENABLE_HUGEPAGE_MIGRATION if HUGETLB_PAGE && MIGRATION
> - select ARCH_ENABLE_PMD_SPLIT_PTLOCK
> + select ARCH_ENABLE_SPLIT_PMD_PTLOCK
> select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
> select ARCH_SUPPORTS_HUGETLBFS
> select ARCH_SUPPORTS_NUMA_BALANCING
> --
> 2.26.2
[Bug 213079] [bisected] IRQ problems and crashes on a PowerMac G5 with 5.12.3
https://bugzilla.kernel.org/show_bug.cgi?id=213079 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Attachment #297473|0 |1 is obsolete|| --- Comment #16 from Erhard F. (erhar...@mailbox.org) --- Created attachment 298371 --> https://bugzilla.kernel.org/attachment.cgi?id=298371&action=edit dmesg (5.14-rc6, PowerMac G5 11,2) As there is a fix now for bug #213803 I was able to build v5.14-rc6 and gave it a testride. Looks like the issue persists: [...] irq 63: nobody cared (try booting with the "irqpoll" option) CPU: 0 PID: 10732 Comm: emerge Tainted: GW 5.14.0-rc6-PowerMacG5+ #2 Call Trace: [cfff7af0] [c054de24] .dump_stack_lvl+0x98/0xe0 (unreliable) [cfff7b80] [c00e1724] .__report_bad_irq+0x34/0xf0 [cfff7c20] [c00e160c] .note_interrupt+0x258/0x300 [cfff7ce0] [c00dd840] .handle_irq_event_percpu+0x5c/0x88 [cfff7d70] [c00dd8b0] .handle_irq_event+0x44/0x70 [cfff7e00] [c00e2d34] .handle_fasteoi_irq+0xac/0x158 [cfff7ea0] [c00dc8bc] .handle_irq_desc+0x34/0x54 [cfff7f10] [c0012058] .__do_irq+0x15c/0x238 [cfff7f90] [c0012978] .__do_IRQ+0xac/0xb4 [c0001e9cfcf0] [c0001e9cfd90] 0xc0001e9cfd90 [c0001e9cfd90] [c0012ac4] .do_IRQ+0x144/0x194 [c0001e9cfe10] [c0008050] hardware_interrupt_common_virt+0x210/0x220 --- interrupt: 500 at 0x3fffb9b25d9c NIP: 3fffb9b25d9c LR: 3fffb9b2811c CTR: 3fffb9b25d9c REGS: c0001e9cfe80 TRAP: 0500 Tainted: GW (5.14.0-rc6-PowerMacG5+) MSR: 9000f032 CR: 22482822 XER: 2000 IRQMASK: 0 GPR00: 3fffb9b28100 3d4e7550 3fffb9ef6200 3fffb7977790 GPR04: 3fffb7977790 3fffb55e8b80 3fffb9eccac0 GPR08: 3fffb9b25d9c 000f GPR12: 3fffb9b7eeb0 3fffb9fc8890 3d4e7658 3fffb395c548 GPR16: 3d4e7670 3fffb7902480 GPR20: 3fffb395c528 00014b8f7878 GPR24: 3fffb7969a80 00014b8f7830 3fffb7a750d0 000a GPR28: 3fffb7a750dc 007c 00014b8f9420 3fffb395c3c0 NIP [3fffb9b25d9c] 0x3fffb9b25d9c LR [3fffb9b2811c] 0x3fffb9b2811c --- interrupt: 500 handlers: [] .nvme_irq [] .nvme_irq Disabling IRQ #63 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 213079] [bisected] IRQ problems and crashes on a PowerMac G5 with 5.12.3
https://bugzilla.kernel.org/show_bug.cgi?id=213079 Erhard F. (erhar...@mailbox.org) changed: What|Removed |Added Attachment #297439|0 |1 is obsolete|| --- Comment #17 from Erhard F. (erhar...@mailbox.org) --- Created attachment 298373 --> https://bugzilla.kernel.org/attachment.cgi?id=298373&action=edit kernel .config (5.14-rc6, PowerMac G5 11,2) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[powerpc:fixes-test] BUILD SUCCESS 9f7853d7609d59172eecfc5e7ccf503bc1b690bd
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test branch HEAD: 9f7853d7609d59172eecfc5e7ccf503bc1b690bd powerpc/mm: Fix set_memory_*() against concurrent accesses elapsed time: 1357m configs tested: 129 configs skipped: 123 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig i386 randconfig-c001-20210819 i386 randconfig-c001-20210818 powerpc pcm030_defconfig arm socfpga_defconfig m68k m5275evb_defconfig sh se7619_defconfig powerpc iss476-smp_defconfig mips rt305x_defconfig arc allyesconfig powerpc linkstation_defconfig arm netwinder_defconfig alpha defconfig arm hackkit_defconfig mips maltasmvp_eva_defconfig mipsar7_defconfig x86_64 defconfig arm ixp4xx_defconfig powerpc mpc834x_mds_defconfig powerpc ps3_defconfig sh se7724_defconfig pariscgeneric-32bit_defconfig powerpc pmac32_defconfig sh rsk7203_defconfig arm stm32_defconfig powerpc ppc64_defconfig mips decstation_r4k_defconfig powerpcsam440ep_defconfig powerpc pq2fads_defconfig m68km5407c3_defconfig sh j2_defconfig m68k amcore_defconfig arm viper_defconfig riscvnommu_k210_defconfig powerpc ep88xc_defconfig armcerfcube_defconfig arm aspeed_g4_defconfig nios2 10m50_defconfig m68k m5249evb_defconfig m68k bvme6000_defconfig microblaze defconfig um defconfig x86_64 allyesconfig mips rm200_defconfig sh sh7770_generic_defconfig arm iop32x_defconfig powerpcicon_defconfig x86_64allnoconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig nds32 allnoconfig nds32 defconfig nios2allyesconfig cskydefconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig s390 allmodconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig x86_64 randconfig-a004-20210818 x86_64 randconfig-a006-20210818 x86_64 randconfig-a003-20210818 x86_64 randconfig-a005-20210818 x86_64 randconfig-a002-20210818 x86_64 randconfig-a001-20210818 i386 randconfig-a004-20210818 i386 randconfig-a006-20210818 i386 randconfig-a002-20210818 i386 randconfig-a001-20210818 i386 randconfig-a003-20210818 i386 randconfig-a005-20210818 i386 randconfig-a015-20210819 i386 randconfig-a011-20210819 i386 randconfig-a014-20210819 i386 randconfig-a013-20210819 i386 randconfig-a016-20210819 i386 randconfig-a012-20210819 riscv
[PATCH linux-next] macintosh: fix warning comparing pointer to 0
Fix the following coccicheck warning:
./drivers/macintosh/windfarm_pm91.c:152:12-13:WARNING comparing pointer to 0
Reported-by: Zeal Robot
Signed-off-by: jing yangyang
---
drivers/macintosh/windfarm_pm91.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/macintosh/windfarm_pm91.c
b/drivers/macintosh/windfarm_pm91.c
index 3f346af..568f8a2 100644
--- a/drivers/macintosh/windfarm_pm91.c
+++ b/drivers/macintosh/windfarm_pm91.c
@@ -149,7 +149,7 @@ static void wf_smu_create_cpu_fans(void)
/* First, locate the PID params in SMU SBD */
hdr = smu_get_sdb_partition(SMU_SDB_CPUPIDDATA_ID, NULL);
- if (hdr == 0) {
+ if (!hdr) {
printk(KERN_WARNING "windfarm: CPU PID fan config not found "
"max fan speed\n");
goto fail;
--
1.8.3.1
[PATCH linux-next] ps3: remove unneeded semicolon
Eliminate the following coccicheck warning: ./arch/powerpc/platforms/ps3/system-bus.c:606:2-3: Unneeded semicolon ./arch/powerpc/platforms/ps3/system-bus.c:765:2-3: Unneeded semicolon Reported-by: Zeal Robot Signed-off-by: jing yangyang --- arch/powerpc/platforms/ps3/system-bus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/ps3/system-bus.c b/arch/powerpc/platforms/ps3/system-bus.c index c8b50fe..b637bf2 100644 --- a/arch/powerpc/platforms/ps3/system-bus.c +++ b/arch/powerpc/platforms/ps3/system-bus.c @@ -603,7 +603,7 @@ static dma_addr_t ps3_ioc0_map_page(struct device *_dev, struct page *page, default: /* not happned */ BUG(); - }; + } result = ps3_dma_map(dev->d_region, (unsigned long)ptr, size, &bus_addr, iopte_flag); @@ -762,7 +762,7 @@ int ps3_system_bus_device_register(struct ps3_system_bus_device *dev) break; default: BUG(); - }; + } dev->core.of_node = NULL; set_dev_node(&dev->core, 0); -- 1.8.3.1
Re: [PATCH 4/5] KVM: selftests: Add a test for KVM_RUN+rseq to detect task migration bugs
On Thu, Aug 19, 2021, Mathieu Desnoyers wrote:
> - On Aug 17, 2021, at 8:12 PM, Sean Christopherson sea...@google.com
> wrote:
>
> > Add a test to verify an rseq's CPU ID is updated correctly if the task is
> > migrated while the kernel is handling KVM_RUN. This is a regression test
> > for a bug introduced by commit 72c3c0fe54a3 ("x86/kvm: Use generic xfer
> > to guest work function"), where TIF_NOTIFY_RESUME would be cleared by KVM
> > without updating rseq, leading to a stale CPU ID and other badness.
> >
> > Signed-off-by: Sean Christopherson
> > ---
>
> [...]
>
> > + while (!done) {
> > + vcpu_run(vm, VCPU_ID);
> > + TEST_ASSERT(get_ucall(vm, VCPU_ID, NULL) == UCALL_SYNC,
> > + "Guest failed?");
> > +
> > + cpu = sched_getcpu();
> > + rseq_cpu = READ_ONCE(__rseq.cpu_id);
> > +
> > + /*
> > +* Verify rseq's CPU matches sched's CPU, and that sched's CPU
> > +* is stable. This doesn't handle the case where the task is
> > +* migrated between sched_getcpu() and reading rseq, and again
> > +* between reading rseq and sched_getcpu(), but in practice no
> > +* false positives have been observed, while on the other hand
> > +* blocking migration while this thread reads CPUs messes with
> > +* the timing and prevents hitting failures on a buggy kernel.
> > +*/
>
> I think you could get a stable cpu id between sched_getcpu and
> __rseq_abi.cpu_id
> if you add a pthread mutex to protect:
>
> sched_getcpu and __rseq_abi.cpu_id reads
>
> vs
>
> sched_setaffinity calls within the migration thread.
>
> Thoughts ?
I tried that and couldn't reproduce the bug. That's what I attempted to call
out
in the blurb "blocking migration while this thread reads CPUs ... prevents
hitting
failures on a buggy kernel".
I considered adding arbitrary delays around the mutex to try and hit the bug,
but
I was worried that even if I got it "working" for this bug, the test would be
too
tailored to this bug and potentially miss future regression. Letting the two
threads run wild seemed like it would provide the best coverage, at the cost of
potentially causing to false failures.
Re: [PATCH 1/5] KVM: rseq: Update rseq when processing NOTIFY_RESUME on xfer to KVM guest
On Thu, Aug 19, 2021, Mathieu Desnoyers wrote:
> - On Aug 17, 2021, at 8:12 PM, Sean Christopherson sea...@google.com
> wrote:
> > @@ -250,7 +250,7 @@ static int rseq_ip_fixup(struct pt_regs *regs)
> > * If not nested over a rseq critical section, restart is useless.
> > * Clear the rseq_cs pointer and return.
> > */
> > - if (!in_rseq_cs(ip, &rseq_cs))
> > + if (!regs || !in_rseq_cs(ip, &rseq_cs))
>
> I think clearing the thread's rseq_cs unconditionally here when regs is NULL
> is not the behavior we want when this is called from xfer_to_guest_mode_work.
>
> If we have a scenario where userspace ends up calling this ioctl(KVM_RUN)
> from within a rseq c.s., we really want a CONFIG_DEBUG_RSEQ=y kernel to
> kill this application in the rseq_syscall handler when exiting back to
> usermode
> when the ioctl eventually returns.
>
> However, clearing the thread's rseq_cs will prevent this from happening.
>
> So I would favor an approach where we simply do:
>
> if (!regs)
> return 0;
>
> Immediately at the beginning of rseq_ip_fixup, before getting the instruction
> pointer, so effectively skip all side-effects of the ip fixup code. Indeed, it
> is not relevant to do any fixup here, because it is nested in a ioctl system
> call.
>
> Effectively, this would preserve the SIGSEGV behavior when this ioctl is
> erroneously called by user-space from a rseq critical section.
Ha, that's effectively what I implemented first, but I changed it because of the
comment in clear_rseq_cs() that says:
The rseq_cs field is set to NULL on preemption or signal delivery ... as well
as well as on top of code outside of the rseq assembly block.
Which makes it sound like something might rely on clearing rseq_cs?
Ah, or is it the case that rseq_cs is non-NULL if and only if userspace is in an
rseq critical section, and because syscalls in critical sections are illegal, by
definition clearing rseq_cs is a nop unless userspace is misbehaving.
If that's true, what about explicitly checking that at NOTIFY_RESUME? Or is it
not worth the extra code to detect an error that will likely be caught anyways?
diff --git a/kernel/rseq.c b/kernel/rseq.c
index 35f7bd0fced0..28b8342290b0 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c
@@ -282,6 +282,13 @@ void __rseq_handle_notify_resume(struct ksignal *ksig,
struct pt_regs *regs)
if (unlikely(t->flags & PF_EXITING))
return;
+ if (!regs) {
+#ifdef CONFIG_DEBUG_RSEQ
+ if (t->rseq && rseq_get_rseq_cs(t, &rseq_cs))
+ goto error;
+#endif
+ return;
+ }
ret = rseq_ip_fixup(regs);
if (unlikely(ret < 0))
goto error;
> Thanks for looking into this !
>
> Mathieu
>
> > return clear_rseq_cs(t);
> > ret = rseq_need_restart(t, rseq_cs.flags);
> > if (ret <= 0)
> > --
> > 2.33.0.rc1.237.g0d66db33f3-goog
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
Re: [PATCH v2 2/2] powerpc: rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK
Daniel Axtens writes:
> Lukas Bulwahn writes:
>
>> Commit 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
>> selects the non-existing config ARCH_ENABLE_PMD_SPLIT_PTLOCK in
>> ./arch/powerpc/platforms/Kconfig.cputype, but clearly it intends to select
>> ARCH_ENABLE_SPLIT_PMD_PTLOCK here (notice the word swapping!), as this
>> commit does select that for all other architectures.
>>
>> Rectify selection to ARCH_ENABLE_SPLIT_PMD_PTLOCK instead.
>>
>
> Yikes, yes, 66f24fa766e3 does seem to have got that wrong. It looks like
> that went into 5.13.
>
> I think we want to specifically target this for stable so that we don't
> lose the perfomance and scalability benefits of split pmd ptlocks:
>
> Cc: sta...@vger.kernel.org # v5.13+
>
> (I don't think you need to do another revision for this, I think mpe
> could add it when merging.)
Yeah. I rewrote the change log a bit to make it clear this is a bug fix,
not a harmless cleanup.
cheers
powerpc: Re-enable ARCH_ENABLE_SPLIT_PMD_PTLOCK
Commit 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
broke PMD split page table lock for powerpc.
It selects the non-existent config ARCH_ENABLE_PMD_SPLIT_PTLOCK in
arch/powerpc/platforms/Kconfig.cputype, but clearly intended to
select ARCH_ENABLE_SPLIT_PMD_PTLOCK (notice the word swapping!), as
that commit did for all other architectures.
Fix it by selecting the correct symbol ARCH_ENABLE_SPLIT_PMD_PTLOCK.
Fixes: 66f24fa766e3 ("mm: drop redundant ARCH_ENABLE_SPLIT_PMD_PTLOCK")
Cc: sta...@vger.kernel.org # v5.13+
Signed-off-by: Lukas Bulwahn
Reviewed-by: Daniel Axtens
[mpe: Reword change log to make it clear this is a bug fix]
Signed-off-by: Michael Ellerman
Link: https://lore.kernel.org/r/20210819113954.17515-3-lukas.bulw...@gmail.com
[PATCH] powerpc/32: indirect function call use bctrl rather than blrl in ret_from_kernel_thread
Copied from commit 89bbe4c798bc ("powerpc/64: indirect function call
use bctrl rather than blrl in ret_from_kernel_thread")
blrl is not recommended to use as an indirect function call, as it may
corrupt the link stack predictor.
This is not a performance critical path but this should be fixed for
consistency.
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/entry_32.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 0273a1349006..61fdd53cdd9a 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -161,10 +161,10 @@ ret_from_fork:
ret_from_kernel_thread:
REST_NVGPRS(r1)
bl schedule_tail
- mtlrr14
+ mtctr r14
mr r3,r15
PPC440EP_ERR42
- blrl
+ bctrl
li r3,0
b ret_from_syscall
--
2.25.0
[PATCH] powerpc: Avoid link stack corruption in {add_}reloc_offset
reloc_offset() / add_reloc_offset() used bl;mflr to get code
position.
Use bcl 20,31,+4 instead of bl in order to preserve link stack.
See commit c974809a26a1 ("powerpc/vdso: Avoid link stack corruption
in __get_datapage()") for details.
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/misc.S | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S
index 5be96feccb55..acd557637a3c 100644
--- a/arch/powerpc/kernel/misc.S
+++ b/arch/powerpc/kernel/misc.S
@@ -29,7 +29,7 @@ _GLOBAL(reloc_offset)
li r3, 0
_GLOBAL(add_reloc_offset)
mflrr0
- bl 1f
+ bcl 20,31,1f
1: mflrr5
PPC_LL r4,(2f-1b)(r5)
subfr5,r4,r5
--
2.25.0
Re: [PATCH v8 2/3] tty: hvc: pass DMA capable memory to put_chars()
Xianting Tian writes:
> As well known, hvc backend driver(eg, virtio-console) can register its
> operations to hvc framework. The operations can contain put_chars(),
> get_chars() and so on.
>
> Some hvc backend may do dma in its operations. eg, put_chars() of
> virtio-console. But in the code of hvc framework, it may pass DMA
> incapable memory to put_chars() under a specific configuration, which
> is explained in commit c4baad5029(virtio-console: avoid DMA from stack):
We could also run into issues on powerpc where Andrew is working on
adding vmap-stack but the opal hvc driver assumes that it is passed a
buffer which is not in vmalloc space but in the linear mapping. So it
would be good to fix this (or more clearly document what drivers can
expect).
> 1, c[] is on stack,
>hvc_console_print():
> char c[N_OUTBUF] __ALIGNED__;
> cons_ops[index]->put_chars(vtermnos[index], c, i);
> 2, ch is on stack,
>static void hvc_poll_put_char(,,char ch)
>{
> struct tty_struct *tty = driver->ttys[0];
> struct hvc_struct *hp = tty->driver_data;
> int n;
>
> do {
> n = hp->ops->put_chars(hp->vtermno, &ch, 1);
> } while (n <= 0);
>}
>
> Commit c4baad5029 is just the fix to avoid DMA from stack memory, which
> is passed to virtio-console by hvc framework in above code. But I think
> the fix is aggressive, it directly uses kmemdup() to alloc new buffer
> from kmalloc area and do memcpy no matter the memory is in kmalloc area
> or not. But most importantly, it should better be fixed in the hvc
> framework, by changing it to never pass stack memory to the put_chars()
> function in the first place. Otherwise, we still face the same issue if
> a new hvc backend using dma added in the future.
>
> In this patch, we make 'char out_buf[N_OUTBUF]' and 'chat out_ch' part
> of 'struct hvc_struct', so both two buf are no longer the stack memory.
> we can use it in above two cases separately.
>
> Introduce another array(cons_outbufs[]) for buffer pointers next to
> the cons_ops[] and vtermnos[] arrays. With the array, we can easily find
> the buffer, instead of traversing hp list.
>
> With the patch, we can remove the fix c4baad5029.
>
> Signed-off-by: Xianting Tian
> Reviewed-by: Shile Zhang
> struct hvc_struct {
> struct tty_port port;
> spinlock_t lock;
> int index;
> int do_wakeup;
> - char *outbuf;
> - int outbuf_size;
> int n_outbuf;
> uint32_t vtermno;
> const struct hv_ops *ops;
> @@ -48,6 +56,10 @@ struct hvc_struct {
> struct work_struct tty_resize;
> struct list_head next;
> unsigned long flags;
> + char out_ch;
> + char out_buf[N_OUTBUF] __ALIGNED__;
> + int outbuf_size;
> + char outbuf[0] __ALIGNED__;
I'm trying to understand this patch but I am finding it very difficult
to understand what the difference between `out_buf` and `outbuf`
(without the underscore) is supposed to be. `out_buf` is statically
sized and the size of `outbuf` is supposed to depend on the arguments to
hvc_alloc(), but I can't quite figure out what the roles of each one are
and their names are confusingly similiar!
I looked briefly at the older revisions of the series but it didn't make
things much clearer.
Could you give them clearer names?
Also, looking at Documentation/process/deprecated.rst, it looks like
maybe we want to use a 'flexible array member' instead:
.. note:: If you are using struct_size() on a structure containing a zero-length
or a one-element array as a trailing array member, please refactor such
array usage and switch to a `flexible array member
<#zero-length-and-one-element-arrays>`_ instead.
I think we want:
> + char outbuf[] __ALIGNED__;
Kind regards,
Daniel
