* Re: [PATCH] powerpc/64s: Include <asm/nmi.h> header file to fix a warning
From: Christophe Leroy @ 2019-03-12 21:11 UTC (permalink / raw)
To: Mathieu Malaterre, Michael Ellerman
Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <20190312201823.29371-1-malat@debian.org>
Le 12/03/2019 à 21:18, Mathieu Malaterre a écrit :
> Make sure to include <asm/nmi.h> to provide the following prototype:
> hv_nmi_check_nonrecoverable.
>
> Remove the following warning treated as error (W=1):
>
> arch/powerpc/kernel/traps.c:393:6: error: no previous prototype for 'hv_nmi_check_nonrecoverable' [-Werror=missing-prototypes]
>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/kernel/traps.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index a21200c6aaea..1fd45a8650e1 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -71,6 +71,7 @@
> #include <sysdev/fsl_pci.h>
> #include <asm/kprobes.h>
> #include <asm/stacktrace.h>
> +#include <asm/nmi.h>
>
> #if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC_CORE)
> int (*__debugger)(struct pt_regs *regs) __read_mostly;
>
^ permalink raw reply
* Re: [PATCH] kmemleak: skip scanning holes in the .bss section
From: Andrew Morton @ 2019-03-12 21:10 UTC (permalink / raw)
To: Qian Cai
Cc: catalin.marinas, agraf, kvm-ppc, linux-kernel, linux-mm, pe,
linuxppc-dev
In-Reply-To: <20190312191412.28656-1-cai@lca.pw>
On Tue, 12 Mar 2019 15:14:12 -0400 Qian Cai <cai@lca.pw> wrote:
> The commit 2d4f567103ff ("KVM: PPC: Introduce kvm_tmp framework") adds
> kvm_tmp[] into the .bss section and then free the rest of unused spaces
> back to the page allocator.
>
> kernel_init
> kvm_guest_init
> kvm_free_tmp
> free_reserved_area
> free_unref_page
> free_unref_page_prepare
>
> With DEBUG_PAGEALLOC=y, it will unmap those pages from kernel. As the
> result, kmemleak scan will trigger a panic below when it scans the .bss
> section with unmapped pages.
>
> Since this is done way before the first kmemleak_scan(), just go
> lockless to make the implementation simple and skip those pages when
> scanning the .bss section. Later, those pages could be tracked by
> kmemleak again once allocated by the page allocator. Overall, this is
> such a special case, so no need to make it a generic to let kmemleak
> gain an ability to skip blocks in scan_large_block().
>
> BUG: Unable to handle kernel data access at 0xc000000001610000
> Faulting instruction address: 0xc0000000003cc178
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA pSeries
> CPU: 3 PID: 130 Comm: kmemleak Kdump: loaded Not tainted 5.0.0+ #9
> REGS: c0000004b05bf940 TRAP: 0300 Not tainted (5.0.0+)
> NIP [c0000000003cc178] scan_block+0xa8/0x190
> LR [c0000000003cc170] scan_block+0xa0/0x190
> Call Trace:
> [c0000004b05bfbd0] [c0000000003cc170] scan_block+0xa0/0x190 (unreliable)
> [c0000004b05bfc30] [c0000000003cc2c0] scan_large_block+0x60/0xa0
> [c0000004b05bfc70] [c0000000003ccc64] kmemleak_scan+0x254/0x960
> [c0000004b05bfd40] [c0000000003cdd50] kmemleak_scan_thread+0xec/0x12c
> [c0000004b05bfdb0] [c000000000104388] kthread+0x1b8/0x1c0
> [c0000004b05bfe20] [c00000000000b364] ret_from_kernel_thread+0x5c/0x78
> Instruction dump:
> 7fa3eb78 4844667d 60000000 60000000 60000000 60000000 3bff0008 7fbcf840
> 409d00b8 4bfffeed 2fa30000 409e00ac <e87f0000> e93e0128 7fa91840
> 419dffdc
>
hm, yes, this is super crude. I guess we can turn it into something
more sophisticated if another caller is identified.
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -237,6 +237,10 @@ static int kmemleak_skip_disable;
> /* If there are leaks that can be reported */
> static bool kmemleak_found_leaks;
>
> +/* Skip scanning of a range in the .bss section. */
> +static void *bss_hole_start;
> +static void *bss_hole_stop;
> +
> static bool kmemleak_verbose;
> module_param_named(verbose, kmemleak_verbose, bool, 0600);
>
> @@ -1265,6 +1269,18 @@ void __ref kmemleak_ignore_phys(phys_addr_t phys)
> }
> EXPORT_SYMBOL(kmemleak_ignore_phys);
>
> +/**
> + * kmemleak_bss_hole - skip scanning a range in the .bss section
> + *
> + * @start: start of the range
> + * @stop: end of the range
> + */
> +void kmemleak_bss_hole(void *start, void *stop)
> +{
> + bss_hole_start = start;
> + bss_hole_stop = stop;
> +}
I'll make this __init.
> /*
> * Update an object's checksum and return true if it was modified.
> */
> @@ -1531,7 +1547,14 @@ static void kmemleak_scan(void)
>
> /* data/bss scanning */
> scan_large_block(_sdata, _edata);
> - scan_large_block(__bss_start, __bss_stop);
> +
> + if (bss_hole_start) {
> + scan_large_block(__bss_start, bss_hole_start);
> + scan_large_block(bss_hole_stop, __bss_stop);
> + } else {
> + scan_large_block(__bss_start, __bss_stop);
> + }
> +
> scan_large_block(__start_ro_after_init, __end_ro_after_init);
>
> #ifdef CONFIG_SMP
^ permalink raw reply
* Re: [PATCH] powerpc/64s: Mark 'dummy_copy_buffer' as used
From: Christophe Leroy @ 2019-03-12 21:06 UTC (permalink / raw)
To: Mathieu Malaterre, Michael Ellerman
Cc: linuxppc-dev, Paul Mackerras, linux-kernel, Nicholas Piggin
In-Reply-To: <20190312202921.31331-1-malat@debian.org>
On 03/12/2019 08:29 PM, Mathieu Malaterre wrote:
> In commit 07d2a628bc00 ("powerpc/64s: Avoid cpabort in context switch
> when possible") a buffer 'dummy_copy_buffer' was introduced. gcc does
> not see this buffer being used in the inline assembly within function
> '__switch_to', explicitly marked this variable as being used.
>
> Prefer using '__aligned' to get passed line over 80 characters warning
> in checkpatch.
Powerpc accepts 90 characters, use arch/powerpc/tools/checkpatch.sh
>
> This remove the following warning:
>
> arch/powerpc/kernel/process.c:1156:17: error: 'dummy_copy_buffer' defined but not used [-Werror=unused-const-variable=]
commit 2bf1071a8d50 ("powerpc/64s: Remove POWER9 DD1 support") has
removed the fonction using 'dummy_copy_buffer' so you should remove it
completely.
Christophe
>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
> arch/powerpc/kernel/process.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 77e44275d025..5acf63d45802 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1153,7 +1153,7 @@ static inline void restore_sprs(struct thread_struct *old_thread,
>
> #ifdef CONFIG_PPC_BOOK3S_64
> #define CP_SIZE 128
> -static const u8 dummy_copy_buffer[CP_SIZE] __attribute__((aligned(CP_SIZE)));
> +static const u8 dummy_copy_buffer[CP_SIZE] __aligned(CP_SIZE) __used;
> #endif
>
> struct task_struct *__switch_to(struct task_struct *prev,
>
^ permalink raw reply
* Re: [PATCH] powerpc: sstep: Mark variable `rc` as unused in function 'analyse_instr'
From: Christophe Leroy @ 2019-03-12 20:56 UTC (permalink / raw)
To: Mathieu Malaterre, Michael Ellerman
Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <20190312202008.29681-1-malat@debian.org>
Le 12/03/2019 à 21:20, Mathieu Malaterre a écrit :
> Add gcc attribute unused for `rc` variable.
>
> Fix warnings treated as errors with W=1:
>
> arch/powerpc/lib/sstep.c:1172:31: error: variable 'rc' set but not used [-Werror=unused-but-set-variable]
>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
> arch/powerpc/lib/sstep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 3d33fb509ef4..32d092f62ae0 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -1169,7 +1169,7 @@ static nokprobe_inline int trap_compare(long v1, long v2)
> int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> unsigned int instr)
> {
> - unsigned int opcode, ra, rb, rc, rd, spr, u;
> + unsigned int opcode, ra, rb, rc __maybe_unused, rd, spr, u;
I think it would be better to enclose 'rc' inside a #ifdef CONFIG_PPC64
Christophe
> unsigned long int imm;
> unsigned long int val, val2;
> unsigned int mb, me, sh;
>
^ permalink raw reply
* [PATCH] powerpc: Make some functions static
From: Mathieu Malaterre @ 2019-03-12 20:31 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mathieu Malaterre, Paul Mackerras, linuxppc-dev, linux-kernel
In commit cb9e4d10c448 ("[POWERPC] Add support for 750CL Holly board")
new functions were added. Since these functions can be made static,
make it so. While doing so, it turns out that holly_power_off and
holly_halt are unused, so remove them.
Silence the following warnings triggered using W=1:
arch/powerpc/platforms/embedded6xx/holly.c:47:5: error: no previous prototype for 'holly_exclude_device' [-Werror=missing-prototypes]
arch/powerpc/platforms/embedded6xx/holly.c:190:6: error: no previous prototype for 'holly_show_cpuinfo' [-Werror=missing-prototypes]
arch/powerpc/platforms/embedded6xx/holly.c:196:17: error: no previous prototype for 'holly_restart' [-Werror=missing-prototypes]
arch/powerpc/platforms/embedded6xx/holly.c:236:6: error: no previous prototype for 'holly_power_off' [-Werror=missing-prototypes]
arch/powerpc/platforms/embedded6xx/holly.c:243:6: error: no previous prototype for 'holly_halt' [-Werror=missing-prototypes]
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
arch/powerpc/platforms/embedded6xx/holly.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/platforms/embedded6xx/holly.c b/arch/powerpc/platforms/embedded6xx/holly.c
index 0409714e8070..829bf3697dc9 100644
--- a/arch/powerpc/platforms/embedded6xx/holly.c
+++ b/arch/powerpc/platforms/embedded6xx/holly.c
@@ -44,7 +44,8 @@
#define HOLLY_PCI_CFG_PHYS 0x7c000000
-int holly_exclude_device(struct pci_controller *hose, u_char bus, u_char devfn)
+static int holly_exclude_device(struct pci_controller *hose, u_char bus,
+ u_char devfn)
{
if (bus == 0 && PCI_SLOT(devfn) == 0)
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -187,13 +188,13 @@ static void __init holly_init_IRQ(void)
tsi108_write_reg(TSI108_MPIC_OFFSET + 0x30c, 0);
}
-void holly_show_cpuinfo(struct seq_file *m)
+static void holly_show_cpuinfo(struct seq_file *m)
{
seq_printf(m, "vendor\t\t: IBM\n");
seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
}
-void __noreturn holly_restart(char *cmd)
+static void __noreturn holly_restart(char *cmd)
{
__be32 __iomem *ocn_bar1 = NULL;
unsigned long bar;
@@ -233,18 +234,6 @@ void __noreturn holly_restart(char *cmd)
for (;;) ;
}
-void holly_power_off(void)
-{
- local_irq_disable();
- /* No way to shut power off with software */
- for (;;) ;
-}
-
-void holly_halt(void)
-{
- holly_power_off();
-}
-
/*
* Called very early, device-tree isn't unflattened
*/
--
2.20.1
^ permalink raw reply related
* [PATCH] powerpc/64s: Mark 'dummy_copy_buffer' as used
From: Mathieu Malaterre @ 2019-03-12 20:29 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mathieu Malaterre, linux-kernel, Nicholas Piggin, Paul Mackerras,
linuxppc-dev
In commit 07d2a628bc00 ("powerpc/64s: Avoid cpabort in context switch
when possible") a buffer 'dummy_copy_buffer' was introduced. gcc does
not see this buffer being used in the inline assembly within function
'__switch_to', explicitly marked this variable as being used.
Prefer using '__aligned' to get passed line over 80 characters warning
in checkpatch.
This remove the following warning:
arch/powerpc/kernel/process.c:1156:17: error: 'dummy_copy_buffer' defined but not used [-Werror=unused-const-variable=]
Cc: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
arch/powerpc/kernel/process.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 77e44275d025..5acf63d45802 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1153,7 +1153,7 @@ static inline void restore_sprs(struct thread_struct *old_thread,
#ifdef CONFIG_PPC_BOOK3S_64
#define CP_SIZE 128
-static const u8 dummy_copy_buffer[CP_SIZE] __attribute__((aligned(CP_SIZE)));
+static const u8 dummy_copy_buffer[CP_SIZE] __aligned(CP_SIZE) __used;
#endif
struct task_struct *__switch_to(struct task_struct *prev,
--
2.20.1
^ permalink raw reply related
* [PATCH] powerpc: sstep: Mark variable `rc` as unused in function 'analyse_instr'
From: Mathieu Malaterre @ 2019-03-12 20:20 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mathieu Malaterre, Paul Mackerras, linuxppc-dev, linux-kernel
Add gcc attribute unused for `rc` variable.
Fix warnings treated as errors with W=1:
arch/powerpc/lib/sstep.c:1172:31: error: variable 'rc' set but not used [-Werror=unused-but-set-variable]
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
arch/powerpc/lib/sstep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 3d33fb509ef4..32d092f62ae0 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1169,7 +1169,7 @@ static nokprobe_inline int trap_compare(long v1, long v2)
int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
unsigned int instr)
{
- unsigned int opcode, ra, rb, rc, rd, spr, u;
+ unsigned int opcode, ra, rb, rc __maybe_unused, rd, spr, u;
unsigned long int imm;
unsigned long int val, val2;
unsigned int mb, me, sh;
--
2.20.1
^ permalink raw reply related
* [PATCH] powerpc/64s: Include <asm/nmi.h> header file to fix a warning
From: Mathieu Malaterre @ 2019-03-12 20:18 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mathieu Malaterre, Paul Mackerras, linuxppc-dev, linux-kernel
Make sure to include <asm/nmi.h> to provide the following prototype:
hv_nmi_check_nonrecoverable.
Remove the following warning treated as error (W=1):
arch/powerpc/kernel/traps.c:393:6: error: no previous prototype for 'hv_nmi_check_nonrecoverable' [-Werror=missing-prototypes]
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
arch/powerpc/kernel/traps.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index a21200c6aaea..1fd45a8650e1 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -71,6 +71,7 @@
#include <sysdev/fsl_pci.h>
#include <asm/kprobes.h>
#include <asm/stacktrace.h>
+#include <asm/nmi.h>
#if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC_CORE)
int (*__debugger)(struct pt_regs *regs) __read_mostly;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] kmemleak: skip scanning holes in the .bss section
From: Qian Cai @ 2019-03-12 19:19 UTC (permalink / raw)
To: akpm; +Cc: catalin.marinas, linux-kernel, kvm-ppc, linux-mm, linuxppc-dev
In-Reply-To: <20190312191412.28656-1-cai@lca.pw>
Fixing some email addresses.
On Tue, 2019-03-12 at 15:14 -0400, Qian Cai wrote:
> The commit 2d4f567103ff ("KVM: PPC: Introduce kvm_tmp framework") adds
> kvm_tmp[] into the .bss section and then free the rest of unused spaces
> back to the page allocator.
>
> kernel_init
> kvm_guest_init
> kvm_free_tmp
> free_reserved_area
> free_unref_page
> free_unref_page_prepare
>
> With DEBUG_PAGEALLOC=y, it will unmap those pages from kernel. As the
> result, kmemleak scan will trigger a panic below when it scans the .bss
> section with unmapped pages.
>
> Since this is done way before the first kmemleak_scan(), just go
> lockless to make the implementation simple and skip those pages when
> scanning the .bss section. Later, those pages could be tracked by
> kmemleak again once allocated by the page allocator. Overall, this is
> such a special case, so no need to make it a generic to let kmemleak
> gain an ability to skip blocks in scan_large_block().
>
> BUG: Unable to handle kernel data access at 0xc000000001610000
> Faulting instruction address: 0xc0000000003cc178
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA pSeries
> CPU: 3 PID: 130 Comm: kmemleak Kdump: loaded Not tainted 5.0.0+ #9
> REGS: c0000004b05bf940 TRAP: 0300 Not tainted (5.0.0+)
> NIP [c0000000003cc178] scan_block+0xa8/0x190
> LR [c0000000003cc170] scan_block+0xa0/0x190
> Call Trace:
> [c0000004b05bfbd0] [c0000000003cc170] scan_block+0xa0/0x190 (unreliable)
> [c0000004b05bfc30] [c0000000003cc2c0] scan_large_block+0x60/0xa0
> [c0000004b05bfc70] [c0000000003ccc64] kmemleak_scan+0x254/0x960
> [c0000004b05bfd40] [c0000000003cdd50] kmemleak_scan_thread+0xec/0x12c
> [c0000004b05bfdb0] [c000000000104388] kthread+0x1b8/0x1c0
> [c0000004b05bfe20] [c00000000000b364] ret_from_kernel_thread+0x5c/0x78
> Instruction dump:
> 7fa3eb78 4844667d 60000000 60000000 60000000 60000000 3bff0008 7fbcf840
> 409d00b8 4bfffeed 2fa30000 409e00ac <e87f0000> e93e0128 7fa91840
> 419dffdc
>
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
> arch/powerpc/kernel/kvm.c | 3 +++
> include/linux/kmemleak.h | 4 ++++
> mm/kmemleak.c | 25 ++++++++++++++++++++++++-
> 3 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/kvm.c b/arch/powerpc/kernel/kvm.c
> index 683b5b3805bd..5cddc8fc56bb 100644
> --- a/arch/powerpc/kernel/kvm.c
> +++ b/arch/powerpc/kernel/kvm.c
> @@ -26,6 +26,7 @@
> #include <linux/slab.h>
> #include <linux/of.h>
> #include <linux/pagemap.h>
> +#include <linux/kmemleak.h>
>
> #include <asm/reg.h>
> #include <asm/sections.h>
> @@ -712,6 +713,8 @@ static void kvm_use_magic_page(void)
>
> static __init void kvm_free_tmp(void)
> {
> + kmemleak_bss_hole(&kvm_tmp[kvm_tmp_index],
> + &kvm_tmp[ARRAY_SIZE(kvm_tmp)]);
> free_reserved_area(&kvm_tmp[kvm_tmp_index],
> &kvm_tmp[ARRAY_SIZE(kvm_tmp)], -1, NULL);
> }
> diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h
> index 5ac416e2d339..3d8949b9c6f5 100644
> --- a/include/linux/kmemleak.h
> +++ b/include/linux/kmemleak.h
> @@ -46,6 +46,7 @@ extern void kmemleak_alloc_phys(phys_addr_t phys, size_t
> size, int min_count,
> extern void kmemleak_free_part_phys(phys_addr_t phys, size_t size) __ref;
> extern void kmemleak_not_leak_phys(phys_addr_t phys) __ref;
> extern void kmemleak_ignore_phys(phys_addr_t phys) __ref;
> +extern void kmemleak_bss_hole(void *start, void *stop);
>
> static inline void kmemleak_alloc_recursive(const void *ptr, size_t size,
> int min_count, slab_flags_t
> flags,
> @@ -131,6 +132,9 @@ static inline void kmemleak_not_leak_phys(phys_addr_t
> phys)
> static inline void kmemleak_ignore_phys(phys_addr_t phys)
> {
> }
> +static inline void kmemleak_bss_hole(void *start, void *stop)
> +{
> +}
>
> #endif /* CONFIG_DEBUG_KMEMLEAK */
>
> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 707fa5579f66..42349cd9ef7a 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -237,6 +237,10 @@ static int kmemleak_skip_disable;
> /* If there are leaks that can be reported */
> static bool kmemleak_found_leaks;
>
> +/* Skip scanning of a range in the .bss section. */
> +static void *bss_hole_start;
> +static void *bss_hole_stop;
> +
> static bool kmemleak_verbose;
> module_param_named(verbose, kmemleak_verbose, bool, 0600);
>
> @@ -1265,6 +1269,18 @@ void __ref kmemleak_ignore_phys(phys_addr_t phys)
> }
> EXPORT_SYMBOL(kmemleak_ignore_phys);
>
> +/**
> + * kmemleak_bss_hole - skip scanning a range in the .bss section
> + *
> + * @start: start of the range
> + * @stop: end of the range
> + */
> +void kmemleak_bss_hole(void *start, void *stop)
> +{
> + bss_hole_start = start;
> + bss_hole_stop = stop;
> +}
> +
> /*
> * Update an object's checksum and return true if it was modified.
> */
> @@ -1531,7 +1547,14 @@ static void kmemleak_scan(void)
>
> /* data/bss scanning */
> scan_large_block(_sdata, _edata);
> - scan_large_block(__bss_start, __bss_stop);
> +
> + if (bss_hole_start) {
> + scan_large_block(__bss_start, bss_hole_start);
> + scan_large_block(bss_hole_stop, __bss_stop);
> + } else {
> + scan_large_block(__bss_start, __bss_stop);
> + }
> +
> scan_large_block(__start_ro_after_init, __end_ro_after_init);
>
> #ifdef CONFIG_SMP
^ permalink raw reply
* [PATCH] kmemleak: skip scanning holes in the .bss section
From: Qian Cai @ 2019-03-12 19:14 UTC (permalink / raw)
To: akpm
Cc: catalin.marinas, agraf, kvm-ppc, linux-kernel, linux-mm, Qian Cai,
pe, linuxppc-dev
The commit 2d4f567103ff ("KVM: PPC: Introduce kvm_tmp framework") adds
kvm_tmp[] into the .bss section and then free the rest of unused spaces
back to the page allocator.
kernel_init
kvm_guest_init
kvm_free_tmp
free_reserved_area
free_unref_page
free_unref_page_prepare
With DEBUG_PAGEALLOC=y, it will unmap those pages from kernel. As the
result, kmemleak scan will trigger a panic below when it scans the .bss
section with unmapped pages.
Since this is done way before the first kmemleak_scan(), just go
lockless to make the implementation simple and skip those pages when
scanning the .bss section. Later, those pages could be tracked by
kmemleak again once allocated by the page allocator. Overall, this is
such a special case, so no need to make it a generic to let kmemleak
gain an ability to skip blocks in scan_large_block().
BUG: Unable to handle kernel data access at 0xc000000001610000
Faulting instruction address: 0xc0000000003cc178
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA pSeries
CPU: 3 PID: 130 Comm: kmemleak Kdump: loaded Not tainted 5.0.0+ #9
REGS: c0000004b05bf940 TRAP: 0300 Not tainted (5.0.0+)
NIP [c0000000003cc178] scan_block+0xa8/0x190
LR [c0000000003cc170] scan_block+0xa0/0x190
Call Trace:
[c0000004b05bfbd0] [c0000000003cc170] scan_block+0xa0/0x190 (unreliable)
[c0000004b05bfc30] [c0000000003cc2c0] scan_large_block+0x60/0xa0
[c0000004b05bfc70] [c0000000003ccc64] kmemleak_scan+0x254/0x960
[c0000004b05bfd40] [c0000000003cdd50] kmemleak_scan_thread+0xec/0x12c
[c0000004b05bfdb0] [c000000000104388] kthread+0x1b8/0x1c0
[c0000004b05bfe20] [c00000000000b364] ret_from_kernel_thread+0x5c/0x78
Instruction dump:
7fa3eb78 4844667d 60000000 60000000 60000000 60000000 3bff0008 7fbcf840
409d00b8 4bfffeed 2fa30000 409e00ac <e87f0000> e93e0128 7fa91840
419dffdc
Signed-off-by: Qian Cai <cai@lca.pw>
---
arch/powerpc/kernel/kvm.c | 3 +++
include/linux/kmemleak.h | 4 ++++
mm/kmemleak.c | 25 ++++++++++++++++++++++++-
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/kvm.c b/arch/powerpc/kernel/kvm.c
index 683b5b3805bd..5cddc8fc56bb 100644
--- a/arch/powerpc/kernel/kvm.c
+++ b/arch/powerpc/kernel/kvm.c
@@ -26,6 +26,7 @@
#include <linux/slab.h>
#include <linux/of.h>
#include <linux/pagemap.h>
+#include <linux/kmemleak.h>
#include <asm/reg.h>
#include <asm/sections.h>
@@ -712,6 +713,8 @@ static void kvm_use_magic_page(void)
static __init void kvm_free_tmp(void)
{
+ kmemleak_bss_hole(&kvm_tmp[kvm_tmp_index],
+ &kvm_tmp[ARRAY_SIZE(kvm_tmp)]);
free_reserved_area(&kvm_tmp[kvm_tmp_index],
&kvm_tmp[ARRAY_SIZE(kvm_tmp)], -1, NULL);
}
diff --git a/include/linux/kmemleak.h b/include/linux/kmemleak.h
index 5ac416e2d339..3d8949b9c6f5 100644
--- a/include/linux/kmemleak.h
+++ b/include/linux/kmemleak.h
@@ -46,6 +46,7 @@ extern void kmemleak_alloc_phys(phys_addr_t phys, size_t size, int min_count,
extern void kmemleak_free_part_phys(phys_addr_t phys, size_t size) __ref;
extern void kmemleak_not_leak_phys(phys_addr_t phys) __ref;
extern void kmemleak_ignore_phys(phys_addr_t phys) __ref;
+extern void kmemleak_bss_hole(void *start, void *stop);
static inline void kmemleak_alloc_recursive(const void *ptr, size_t size,
int min_count, slab_flags_t flags,
@@ -131,6 +132,9 @@ static inline void kmemleak_not_leak_phys(phys_addr_t phys)
static inline void kmemleak_ignore_phys(phys_addr_t phys)
{
}
+static inline void kmemleak_bss_hole(void *start, void *stop)
+{
+}
#endif /* CONFIG_DEBUG_KMEMLEAK */
diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 707fa5579f66..42349cd9ef7a 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -237,6 +237,10 @@ static int kmemleak_skip_disable;
/* If there are leaks that can be reported */
static bool kmemleak_found_leaks;
+/* Skip scanning of a range in the .bss section. */
+static void *bss_hole_start;
+static void *bss_hole_stop;
+
static bool kmemleak_verbose;
module_param_named(verbose, kmemleak_verbose, bool, 0600);
@@ -1265,6 +1269,18 @@ void __ref kmemleak_ignore_phys(phys_addr_t phys)
}
EXPORT_SYMBOL(kmemleak_ignore_phys);
+/**
+ * kmemleak_bss_hole - skip scanning a range in the .bss section
+ *
+ * @start: start of the range
+ * @stop: end of the range
+ */
+void kmemleak_bss_hole(void *start, void *stop)
+{
+ bss_hole_start = start;
+ bss_hole_stop = stop;
+}
+
/*
* Update an object's checksum and return true if it was modified.
*/
@@ -1531,7 +1547,14 @@ static void kmemleak_scan(void)
/* data/bss scanning */
scan_large_block(_sdata, _edata);
- scan_large_block(__bss_start, __bss_stop);
+
+ if (bss_hole_start) {
+ scan_large_block(__bss_start, bss_hole_start);
+ scan_large_block(bss_hole_stop, __bss_stop);
+ } else {
+ scan_large_block(__bss_start, __bss_stop);
+ }
+
scan_large_block(__start_ro_after_init, __end_ro_after_init);
#ifdef CONFIG_SMP
--
2.17.2 (Apple Git-113)
^ permalink raw reply related
* Re: [PATCH v2 4/7] dt-bindings: counter: ftm-quaddec
From: Rob Herring @ 2019-03-12 19:09 UTC (permalink / raw)
To: Patrick Havelange
Cc: Mark Rutland, devicetree, linuxppc-dev, Jonathan Cameron,
linux-pwm, linux-iio, linux-kernel, Daniel Lezcano,
William Breathitt Gray, Li Yang, Thierry Reding, linux-arm-kernel,
Thomas Gleixner, Shawn Guo, Esben Haabendal
In-Reply-To: <20190306111208.7454-5-patrick.havelange@essensium.com>
On Wed, Mar 06, 2019 at 12:12:05PM +0100, Patrick Havelange wrote:
> FlexTimer quadrature decoder driver.
>
> Signed-off-by: Patrick Havelange <patrick.havelange@essensium.com>
> Reviewed-by: Esben Haabendal <esben@haabendal.dk>
> ---
> Changes v2
> - None
> ---
> .../bindings/counter/ftm-quaddec.txt | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/counter/ftm-quaddec.txt
>
> diff --git a/Documentation/devicetree/bindings/counter/ftm-quaddec.txt b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt
> new file mode 100644
> index 000000000000..4d18cd722074
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/counter/ftm-quaddec.txt
> @@ -0,0 +1,18 @@
> +FlexTimer Quadrature decoder counter
> +
> +This driver exposes a simple counter for the quadrature decoder mode.
Seems like this is more a mode of a h/w block than describing a h/w
block. Bindings should do the latter.
> +
> +Required properties:
> +- compatible: Must be "fsl,ftm-quaddec".
> +- reg: Must be set to the memory region of the flextimer.
> +
> +Optional property:
> +- big-endian: Access the device registers in big-endian mode.
> +
> +Example:
> + counter0: counter@29d0000 {
> + compatible = "fsl,ftm-quaddec";
> + reg = <0x0 0x29d0000 0x0 0x10000>;
> + big-endian;
> + status = "disabled";
> + };
> --
> 2.19.1
>
^ permalink raw reply
* Re: [PATCH v2 01/16] powerpc/xive: add OPAL extensions for the XIVE native exploitation support
From: Cédric Le Goater @ 2019-03-12 18:25 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, Paul Mackerras, Stewart Smith, linuxppc-dev
In-Reply-To: <20190226042145.GP6872@umbus.fritz.box>
On 2/26/19 5:21 AM, David Gibson wrote:
> On Mon, Feb 25, 2019 at 11:11:58AM +0100, Cédric Le Goater wrote:
>> On 2/25/19 4:50 AM, Michael Ellerman wrote:
>>> Cédric Le Goater <clg@kaod.org> writes:
>>>
>>>> The support for XIVE native exploitation mode in Linux/KVM needs a
>>>> couple more OPAL calls to configure the sPAPR guest and to get/set the
>>>> state of the XIVE internal structures.
>>>>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>>> arch/powerpc/include/asm/opal-api.h | 11 ++-
>>>> arch/powerpc/include/asm/opal.h | 7 ++
>>>> arch/powerpc/include/asm/xive.h | 14 +++
>>>> arch/powerpc/sysdev/xive/native.c | 99 +++++++++++++++++++
>>>> .../powerpc/platforms/powernv/opal-wrappers.S | 3 +
>>>> 5 files changed, 130 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
>>>> index 870fb7b239ea..cdfc54f78101 100644
>>>> --- a/arch/powerpc/include/asm/opal-api.h
>>>> +++ b/arch/powerpc/include/asm/opal-api.h
>>>> @@ -186,8 +186,8 @@
>>>> #define OPAL_XIVE_FREE_IRQ 140
>>>> #define OPAL_XIVE_SYNC 141
>>>> #define OPAL_XIVE_DUMP 142
>>>> -#define OPAL_XIVE_RESERVED3 143
>>>> -#define OPAL_XIVE_RESERVED4 144
>>>> +#define OPAL_XIVE_GET_QUEUE_STATE 143
>>>> +#define OPAL_XIVE_SET_QUEUE_STATE 144
>>>> #define OPAL_SIGNAL_SYSTEM_RESET 145
>>>> #define OPAL_NPU_INIT_CONTEXT 146
>>>> #define OPAL_NPU_DESTROY_CONTEXT 147
>>>> @@ -209,8 +209,11 @@
>>>> #define OPAL_SENSOR_GROUP_ENABLE 163
>>>> #define OPAL_PCI_GET_PBCQ_TUNNEL_BAR 164
>>>> #define OPAL_PCI_SET_PBCQ_TUNNEL_BAR 165
>>>> -#define OPAL_NX_COPROC_INIT 167
>>>> -#define OPAL_LAST 167
>>>> +#define OPAL_HANDLE_HMI2 166
>>>> +#define OPAL_NX_COPROC_INIT 167
>>>> +#define OPAL_NPU_SET_RELAXED_ORDER 168
>>>> +#define OPAL_NPU_GET_RELAXED_ORDER 169
>>>> +#define OPAL_XIVE_GET_VP_STATE 170
>>>
>>> You should only be defining the calls you need, leaving gaps for other
>>> things, and you need to retain OPAL_LAST. So it should look more like:
>>>
>>> -#define OPAL_LAST 167
>>> +#define OPAL_XIVE_GET_VP_STATE 170
>>> +#define OPAL_LAST 170
>>>
>>>
>>> Also I can't merge this until it's merged into skiboot.
>>
>> OK. Let's start with skiboot.
>
> Yeah.. where are we at with skiboot in general. We can't test this
> downstream until we have a released skiboot with the necessary
> support.
If we add a flag to remove the OPAL call when setting the EQ, you could
test, without migration though.
C.
^ permalink raw reply
* Re: [PATCH 00/14] entry: preempt_schedule_irq() callers scrub
From: Vineet Gupta @ 2019-03-12 18:03 UTC (permalink / raw)
To: Valentin Schneider, linux-kernel
Cc: uclinux-h8-devel, linux-xtensa, linux-ia64, linux-c6x-dev,
Julien Thierry, Peter Zijlstra, linux-s390, x86, linux-mips,
linux-m68k, Ingo Molnar, linux-sh, sparclinux, nios2-dev,
Thomas Gleixner, linux-snps-arc, linuxppc-dev, linux-riscv
In-Reply-To: <20190311224752.8337-1-valentin.schneider@arm.com>
On 3/11/19 3:47 PM, Valentin Schneider wrote:
> Hi,
>
> This is the continuation of [1] where I'm hunting down
> preempt_schedule_irq() callers because of [2].
>
> I told myself the best way to get this moving forward wouldn't be to write
> doc about it, but to go write some fixes and get some discussions going,
> which is what this patch-set is about.
>
> I've looked at users of preempt_schedule_irq(), and made sure they didn't
> have one of those useless loops. The list of offenders is:
>
> $ grep -r -I "preempt_schedule_irq" arch/ | cut -d/ -f2 | sort | uniq
>
...
>
> Regarding that loop, archs seem to fall in 3 categories:
> A) Those that don't have the loop
Please clarify that this is the right thing to do (since core code already has the
loop) hence no fixing is required for this "category"
> B) Those that have a small need_resched() loop around the
> preempt_schedule_irq() callsite
> C) Those that branch to some more generic code further up the entry code
> and eventually branch back to preempt_schedule_irq()
>
> arc, m68k, nios2 fall in A)
> sparc, ia64, s390 fall in C)
> all the others fall in B)
>
> I've written patches for B) and C) EXCEPT for ia64 and s390 because I
> haven't been able to tell if it's actually fine to kill that "long jump"
> (and maybe I'm wrong on sparc). Hopefully folks who understand what goes on
> in there might be able to shed some light.
^ permalink raw reply
* Re: [PATCH v2 06/16] KVM: PPC: Book3S HV: XIVE: add controls for the EQ configuration
From: Cédric Le Goater @ 2019-03-12 17:00 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190225023955.GJ7668@umbus.fritz.box>
On 2/25/19 3:39 AM, David Gibson wrote:
> On Fri, Feb 22, 2019 at 12:28:30PM +0100, Cédric Le Goater wrote:
>> These controls will be used by the H_INT_SET_QUEUE_CONFIG and
>> H_INT_GET_QUEUE_CONFIG hcalls from QEMU. They will also be used to
>> restore the configuration of the XIVE EQs in the KVM device and to
>> capture the internal runtime state of the EQs. Both 'get' and 'set'
>> rely on an OPAL call to access from the XIVE interrupt controller the
>> EQ toggle bit and EQ index which are updated by the HW when event
>> notifications are enqueued in the EQ.
>>
>> The value of the guest physical address of the event queue is saved in
>> the XIVE internal xive_q structure for later use. That is when
>> migration needs to mark the EQ pages dirty to capture a consistent
>> memory state of the VM.
>>
>> To be noted that H_INT_SET_QUEUE_CONFIG does not require the extra
>> OPAL call setting the EQ toggle bit and EQ index to configure the EQ,
>> but restoring the EQ state will.
I think we need to add some kind of flags to differentiate the hcall
H_INT_SET_QUEUE_CONFIG from the restore of the EQ. The hcall does
not need OPAL support call and this could help in the code transition.
But without OPAL support, we won't have migration. Would that be of
any use ?
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> arch/powerpc/include/asm/xive.h | 2 +
>> arch/powerpc/include/uapi/asm/kvm.h | 21 +++
>> arch/powerpc/kvm/book3s_xive.h | 2 +
>> arch/powerpc/kvm/book3s_xive.c | 15 +-
>> arch/powerpc/kvm/book3s_xive_native.c | 207 +++++++++++++++++++++
>> Documentation/virtual/kvm/devices/xive.txt | 29 +++
>> 6 files changed, 270 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
>> index b579a943407b..46891f321606 100644
>> --- a/arch/powerpc/include/asm/xive.h
>> +++ b/arch/powerpc/include/asm/xive.h
>> @@ -73,6 +73,8 @@ struct xive_q {
>> u32 esc_irq;
>> atomic_t count;
>> atomic_t pending_count;
>> + u64 guest_qpage;
>> + u32 guest_qsize;
>> };
>>
>> /* Global enable flags for the XIVE support */
>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>> index 91899c7f9abd..177e43f3edaf 100644
>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>> @@ -679,6 +679,7 @@ struct kvm_ppc_cpu_char {
>> #define KVM_DEV_XIVE_GRP_CTRL 1
>> #define KVM_DEV_XIVE_GRP_SOURCE 2 /* 64-bit source attributes */
>> #define KVM_DEV_XIVE_GRP_SOURCE_CONFIG 3 /* 64-bit source attributes */
>> +#define KVM_DEV_XIVE_GRP_EQ_CONFIG 4 /* 64-bit eq attributes */
>>
>> /* Layout of 64-bit XIVE source attribute values */
>> #define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
>> @@ -694,4 +695,24 @@ struct kvm_ppc_cpu_char {
>> #define KVM_XIVE_SOURCE_EISN_SHIFT 33
>> #define KVM_XIVE_SOURCE_EISN_MASK 0xfffffffe00000000ULL
>>
>> +/* Layout of 64-bit eq attribute */
>> +#define KVM_XIVE_EQ_PRIORITY_SHIFT 0
>> +#define KVM_XIVE_EQ_PRIORITY_MASK 0x7
>> +#define KVM_XIVE_EQ_SERVER_SHIFT 3
>> +#define KVM_XIVE_EQ_SERVER_MASK 0xfffffff8ULL
>> +
>> +/* Layout of 64-bit eq attribute values */
>> +struct kvm_ppc_xive_eq {
>> + __u32 flags;
>> + __u32 qsize;
>> + __u64 qpage;
>> + __u32 qtoggle;
>> + __u32 qindex;
>> + __u8 pad[40];
>> +};
>> +
>> +#define KVM_XIVE_EQ_FLAG_ENABLED 0x00000001
>> +#define KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY 0x00000002
>> +#define KVM_XIVE_EQ_FLAG_ESCALATE 0x00000004
>> +
>> #endif /* __LINUX_KVM_POWERPC_H */
>> diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
>> index ab3ac152980d..6660d138c6b7 100644
>> --- a/arch/powerpc/kvm/book3s_xive.h
>> +++ b/arch/powerpc/kvm/book3s_xive.h
>> @@ -267,6 +267,8 @@ struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
>> struct kvmppc_xive *xive, int irq);
>> void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb);
>> int kvmppc_xive_select_target(struct kvm *kvm, u32 *server, u8 prio);
>> +int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio,
>> + bool single_escalation);
>>
>> #endif /* CONFIG_KVM_XICS */
>> #endif /* _KVM_PPC_BOOK3S_XICS_H */
>> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
>> index 086da91d7c6e..7431e31bc541 100644
>> --- a/arch/powerpc/kvm/book3s_xive.c
>> +++ b/arch/powerpc/kvm/book3s_xive.c
>> @@ -166,7 +166,8 @@ static irqreturn_t xive_esc_irq(int irq, void *data)
>> return IRQ_HANDLED;
>> }
>>
>> -static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
>> +int kvmppc_xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio,
>> + bool single_escalation)
>> {
>> struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> struct xive_q *q = &xc->queues[prio];
>> @@ -185,7 +186,7 @@ static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
>> return -EIO;
>> }
>>
>> - if (xc->xive->single_escalation)
>> + if (single_escalation)
>> name = kasprintf(GFP_KERNEL, "kvm-%d-%d",
>> vcpu->kvm->arch.lpid, xc->server_num);
>> else
>> @@ -217,7 +218,7 @@ static int xive_attach_escalation(struct kvm_vcpu *vcpu, u8 prio)
>> * interrupt, thus leaving it effectively masked after
>> * it fires once.
>> */
>> - if (xc->xive->single_escalation) {
>> + if (single_escalation) {
>> struct irq_data *d = irq_get_irq_data(xc->esc_virq[prio]);
>> struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
>>
>> @@ -291,7 +292,8 @@ static int xive_check_provisioning(struct kvm *kvm, u8 prio)
>> continue;
>> rc = xive_provision_queue(vcpu, prio);
>> if (rc == 0 && !xive->single_escalation)
>> - xive_attach_escalation(vcpu, prio);
>> + kvmppc_xive_attach_escalation(vcpu, prio,
>> + xive->single_escalation);
>> if (rc)
>> return rc;
>> }
>> @@ -1219,7 +1221,8 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
>> if (xive->qmap & (1 << i)) {
>> r = xive_provision_queue(vcpu, i);
>> if (r == 0 && !xive->single_escalation)
>> - xive_attach_escalation(vcpu, i);
>> + kvmppc_xive_attach_escalation(
>> + vcpu, i, xive->single_escalation);
>> if (r)
>> goto bail;
>> } else {
>> @@ -1234,7 +1237,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
>> }
>>
>> /* If not done above, attach priority 0 escalation */
>> - r = xive_attach_escalation(vcpu, 0);
>> + r = kvmppc_xive_attach_escalation(vcpu, 0, xive->single_escalation);
>> if (r)
>> goto bail;
>>
>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
>> index cb5a5c6e05af..34a35bcf550c 100644
>> --- a/arch/powerpc/kvm/book3s_xive_native.c
>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
>> @@ -341,6 +341,201 @@ static int kvmppc_xive_native_set_source_config(struct kvmppc_xive *xive,
>> priority, eisn);
>> }
>>
>> +static int xive_native_validate_queue_size(u32 qsize)
>> +{
>> + switch (qsize) {
>> + case 12:
>> + case 16:
>> + case 21:
>> + case 24:
>> + case 0:
>> + return 0;
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static int kvmppc_xive_native_set_queue_config(struct kvmppc_xive *xive,
>> + long eq_idx, u64 addr)
>> +{
>> + struct kvm *kvm = xive->kvm;
>> + struct kvm_vcpu *vcpu;
>> + struct kvmppc_xive_vcpu *xc;
>> + void __user *ubufp = (u64 __user *) addr;
>> + u32 server;
>> + u8 priority;
>> + struct kvm_ppc_xive_eq kvm_eq;
>> + int rc;
>> + __be32 *qaddr = 0;
>> + struct page *page;
>> + struct xive_q *q;
>> +
>> + /*
>> + * Demangle priority/server tuple from the EQ index
>> + */
>> + priority = (eq_idx & KVM_XIVE_EQ_PRIORITY_MASK) >>
>> + KVM_XIVE_EQ_PRIORITY_SHIFT;
>> + server = (eq_idx & KVM_XIVE_EQ_SERVER_MASK) >>
>> + KVM_XIVE_EQ_SERVER_SHIFT;
>> +
>> + if (copy_from_user(&kvm_eq, ubufp, sizeof(kvm_eq)))
>> + return -EFAULT;
>> +
>> + vcpu = kvmppc_xive_find_server(kvm, server);
>> + if (!vcpu) {
>> + pr_err("Can't find server %d\n", server);
>> + return -ENOENT;
>> + }
>> + xc = vcpu->arch.xive_vcpu;
>> +
>> + if (priority != xive_prio_from_guest(priority)) {
>> + pr_err("Trying to restore invalid queue %d for VCPU %d\n",
>> + priority, server);
>> + return -EINVAL;
>> + }
>> + q = &xc->queues[priority];
>
> You need to validate the 'flags' field (AFAICT we don't actually have
> any flags yet, so it's only valid it if is 0.
Well, we should set the ESCALATE flags and ALWAYS_NOTIFY also but this
is already set by xive_native_configure_queue(). I will take a closer look
and document.
>> +
>> + pr_devel("%s VCPU %d priority %d fl:%x sz:%d addr:%llx g:%d idx:%d\n",
>> + __func__, server, priority, kvm_eq.flags,
>> + kvm_eq.qsize, kvm_eq.qpage, kvm_eq.qtoggle, kvm_eq.qindex);
>> +
>> + rc = xive_native_validate_queue_size(kvm_eq.qsize);
>> + if (rc) {
>> + pr_err("invalid queue size %d\n", kvm_eq.qsize);
>> + return rc;
>> + }
>> +
>> + /* reset queue and disable queueing */
>> + if (!kvm_eq.qsize) {
>> + q->guest_qpage = 0;
>> + q->guest_qsize = 0;
>> +
>> + rc = xive_native_configure_queue(xc->vp_id, q, priority,
>> + NULL, 0, true);
>> + if (rc) {
>> + pr_err("Failed to reset queue %d for VCPU %d: %d\n",
>> + priority, xc->server_num, rc);
>> + return rc;
>> + }
>> +
>> + if (q->qpage) {
>> + put_page(virt_to_page(q->qpage));
>> + q->qpage = NULL;
>> + }
>> +
>> + return 0;
>> + }
>> +
>> +
>> + page = gfn_to_page(kvm, gpa_to_gfn(kvm_eq.qpage));
>> + if (is_error_page(page)) {
>> + pr_warn("Couldn't get guest page for %llx!\n", kvm_eq.qpage);
>> + return -ENOMEM;
>
> Nit: I don't think ENOMEM is the right error here. ENOMEM indicates
> that the kernel couldn't allocate enough memory to complete whatever
> you asked. Here the problem is the user supplied a bad guest address,
> which is a rather different error. EFAULT is closer, but still not
> quite right, since it could be a valid user address but not a valid
> guest address. There are probably existing KVM calls that could hit
> this problem, I wonder what they use.
I have seen a -EINVAL (s390) and a -EFAULT (x86)
Thanks,
C.
>> + }
>> + qaddr = page_to_virt(page) + (kvm_eq.qpage & ~PAGE_MASK);
>> +
>> + /* Backup queue page guest address for migration */
>> + q->guest_qpage = kvm_eq.qpage;
>> + q->guest_qsize = kvm_eq.qsize;
>> +
>> + rc = xive_native_configure_queue(xc->vp_id, q, priority,
>> + (__be32 *) qaddr, kvm_eq.qsize, true);
>> + if (rc) {
>> + pr_err("Failed to configure queue %d for VCPU %d: %d\n",
>> + priority, xc->server_num, rc);
>> + put_page(page);
>> + return rc;
>> + }
>> +
>> + rc = xive_native_set_queue_state(xc->vp_id, priority, kvm_eq.qtoggle,
>> + kvm_eq.qindex);
>> + if (rc)
>> + goto error;
>> +
>> + rc = kvmppc_xive_attach_escalation(vcpu, priority,
>> + xive->single_escalation);
>> +error:
>> + if (rc)
>> + kvmppc_xive_native_cleanup_queue(vcpu, priority);
>> + return rc;
>> +}
>> +
>> +static int kvmppc_xive_native_get_queue_config(struct kvmppc_xive *xive,
>> + long eq_idx, u64 addr)
>> +{
>> + struct kvm *kvm = xive->kvm;
>> + struct kvm_vcpu *vcpu;
>> + struct kvmppc_xive_vcpu *xc;
>> + struct xive_q *q;
>> + void __user *ubufp = (u64 __user *) addr;
>> + u32 server;
>> + u8 priority;
>> + struct kvm_ppc_xive_eq kvm_eq;
>> + u64 qpage;
>> + u64 qsize;
>> + u64 qeoi_page;
>> + u32 escalate_irq;
>> + u64 qflags;
>> + int rc;
>> +
>> + /*
>> + * Demangle priority/server tuple from the EQ index
>> + */
>> + priority = (eq_idx & KVM_XIVE_EQ_PRIORITY_MASK) >>
>> + KVM_XIVE_EQ_PRIORITY_SHIFT;
>> + server = (eq_idx & KVM_XIVE_EQ_SERVER_MASK) >>
>> + KVM_XIVE_EQ_SERVER_SHIFT;
>> +
>> + vcpu = kvmppc_xive_find_server(kvm, server);
>> + if (!vcpu) {
>> + pr_err("Can't find server %d\n", server);
>> + return -ENOENT;
>> + }
>> + xc = vcpu->arch.xive_vcpu;
>> +
>> + if (priority != xive_prio_from_guest(priority)) {
>> + pr_err("invalid priority for queue %d for VCPU %d\n",
>> + priority, server);
>> + return -EINVAL;
>> + }
>> + q = &xc->queues[priority];
>> +
>> + memset(&kvm_eq, 0, sizeof(kvm_eq));
>> +
>> + if (!q->qpage)
>> + return 0;
>> +
>> + rc = xive_native_get_queue_info(xc->vp_id, priority, &qpage, &qsize,
>> + &qeoi_page, &escalate_irq, &qflags);
>> + if (rc)
>> + return rc;
>> +
>> + kvm_eq.flags = 0;
>> + if (qflags & OPAL_XIVE_EQ_ENABLED)
>> + kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ENABLED;
>> + if (qflags & OPAL_XIVE_EQ_ALWAYS_NOTIFY)
>> + kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY;
>> + if (qflags & OPAL_XIVE_EQ_ESCALATE)
>> + kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ESCALATE;
>> +
>> + kvm_eq.qsize = q->guest_qsize;
>> + kvm_eq.qpage = q->guest_qpage;
>> +
>> + rc = xive_native_get_queue_state(xc->vp_id, priority, &kvm_eq.qtoggle,
>> + &kvm_eq.qindex);
>> + if (rc)
>> + return rc;
>> +
>> + pr_devel("%s VCPU %d priority %d fl:%x sz:%d addr:%llx g:%d idx:%d\n",
>> + __func__, server, priority, kvm_eq.flags,
>> + kvm_eq.qsize, kvm_eq.qpage, kvm_eq.qtoggle, kvm_eq.qindex);
>> +
>> + if (copy_to_user(ubufp, &kvm_eq, sizeof(kvm_eq)))
>> + return -EFAULT;
>> +
>> + return 0;
>> +}
>> +
>> static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>> struct kvm_device_attr *attr)
>> {
>> @@ -355,6 +550,9 @@ static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>> case KVM_DEV_XIVE_GRP_SOURCE_CONFIG:
>> return kvmppc_xive_native_set_source_config(xive, attr->attr,
>> attr->addr);
>> + case KVM_DEV_XIVE_GRP_EQ_CONFIG:
>> + return kvmppc_xive_native_set_queue_config(xive, attr->attr,
>> + attr->addr);
>> }
>> return -ENXIO;
>> }
>> @@ -362,6 +560,13 @@ static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>> static int kvmppc_xive_native_get_attr(struct kvm_device *dev,
>> struct kvm_device_attr *attr)
>> {
>> + struct kvmppc_xive *xive = dev->private;
>> +
>> + switch (attr->group) {
>> + case KVM_DEV_XIVE_GRP_EQ_CONFIG:
>> + return kvmppc_xive_native_get_queue_config(xive, attr->attr,
>> + attr->addr);
>> + }
>> return -ENXIO;
>> }
>>
>> @@ -377,6 +582,8 @@ static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
>> attr->attr < KVMPPC_XIVE_NR_IRQS)
>> return 0;
>> break;
>> + case KVM_DEV_XIVE_GRP_EQ_CONFIG:
>> + return 0;
>> }
>> return -ENXIO;
>> }
>> diff --git a/Documentation/virtual/kvm/devices/xive.txt b/Documentation/virtual/kvm/devices/xive.txt
>> index 4f513a1880c7..c0b5d9bd43fb 100644
>> --- a/Documentation/virtual/kvm/devices/xive.txt
>> +++ b/Documentation/virtual/kvm/devices/xive.txt
>> @@ -52,3 +52,32 @@ the legacy interrupt mode, referred as XICS (POWER7/8).
>> -ENXIO: CPU event queues not configured or configuration of the
>> underlying HW interrupt failed
>> -EBUSY: No CPU available to serve interrupt
>> +
>> + 4. KVM_DEV_XIVE_GRP_EQ_CONFIG (read-write)
>> + Configures an event queue of a CPU
>> + Attributes:
>> + EQ descriptor identifier (64-bit)
>> + The EQ descriptor identifier is a tuple (server, priority) :
>> + bits: | 63 .... 32 | 31 .. 3 | 2 .. 0
>> + values: | unused | server | priority
>> + The kvm_device_attr.addr points to :
>> + struct kvm_ppc_xive_eq {
>> + __u32 flags;
>> + __u32 qsize;
>> + __u64 qpage;
>> + __u32 qtoggle;
>> + __u32 qindex;
>> + __u8 pad[40];
>> + };
>> + - flags: queue flags
>> + - qsize: queue size (power of 2)
>> + - qpage: real address of queue
>> + - qtoggle: current queue toggle bit
>> + - qindex: current queue index
>> + - pad: reserved for future use
>> + Errors:
>> + -ENOENT: Invalid CPU number
>> + -EINVAL: Invalid priority or invalid queue size
>> + -EFAULT: Invalid user pointer for attr->addr.
>> + -ENOMEM: Invalid queue address
>> + -EIO: Configuration of the underlying HW failed
>
^ permalink raw reply
* Re: [PATCH v3] powerpc/pseries: Only wait for dying CPU after call to rtas_stop_self()
From: Thiago Jung Bauermann @ 2019-03-12 16:30 UTC (permalink / raw)
To: ego; +Cc: Michael Bringmann, linuxppc-dev, linux-kernel, Tyrel Datwyler
In-Reply-To: <20190312093246.GA18313@in.ibm.com>
Gautham R Shenoy <ego@linux.vnet.ibm.com> writes:
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
> Thanks for this version. I have tested the patch and we no longer see
> the "Querying DEAD? cpu X (Y) shows 2" message.
>
>
> Tested-and-Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Thanks for reviewing and testing the patch!
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v2 04/16] KVM: PPC: Book3S HV: XIVE: add a control to initialize a source
From: Cédric Le Goater @ 2019-03-12 15:19 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190225021012.GH7668@umbus.fritz.box>
On 2/25/19 3:10 AM, David Gibson wrote:
> On Fri, Feb 22, 2019 at 12:28:28PM +0100, Cédric Le Goater wrote:
>> The associated HW interrupt source is simply allocated at the OPAL/HW
>> level and then MASKED. KVM only needs to know about its type: LSI or
>> MSI.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> arch/powerpc/include/uapi/asm/kvm.h | 5 +
>> arch/powerpc/kvm/book3s_xive.h | 10 ++
>> arch/powerpc/kvm/book3s_xive.c | 8 +-
>> arch/powerpc/kvm/book3s_xive_native.c | 114 +++++++++++++++++++++
>> Documentation/virtual/kvm/devices/xive.txt | 15 +++
>> 5 files changed, 148 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>> index b002c0c67787..a9ad99f2a11b 100644
>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>> @@ -677,5 +677,10 @@ struct kvm_ppc_cpu_char {
>>
>> /* POWER9 XIVE Native Interrupt Controller */
>> #define KVM_DEV_XIVE_GRP_CTRL 1
>> +#define KVM_DEV_XIVE_GRP_SOURCE 2 /* 64-bit source attributes */
>> +
>> +/* Layout of 64-bit XIVE source attribute values */
>> +#define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
>> +#define KVM_XIVE_LEVEL_ASSERTED (1ULL << 1)
>>
>> #endif /* __LINUX_KVM_POWERPC_H */
>> diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
>> index bcb1bbcf0359..f22f2d46d0f0 100644
>> --- a/arch/powerpc/kvm/book3s_xive.h
>> +++ b/arch/powerpc/kvm/book3s_xive.h
>> @@ -12,6 +12,13 @@
>> #ifdef CONFIG_KVM_XICS
>> #include "book3s_xics.h"
>>
>> +/*
>> + * The XIVE IRQ number space is aligned with the XICS IRQ number
>> + * space, CPU IPIs being allocated in the first 4K.
>
> We do align these in qemu, but I don't see that the kernel part
> cares: as far as it's concerned only one of XICS or XIVE is active at
> a time, and the irq numbers are chosen by userspace.
There is some relation with userspace nevertheless. The KVM device does
not remap the numbers to some other range today and the limits are fixed
values. Checks are being done in the has_attr() and the set_attr().
>> + */
>> +#define KVMPPC_XIVE_FIRST_IRQ 0
>> +#define KVMPPC_XIVE_NR_IRQS KVMPPC_XICS_NR_IRQS
>> +
>> /*
>> * State for one guest irq source.
>> *
>> @@ -253,6 +260,9 @@ extern int (*__xive_vm_h_eoi)(struct kvm_vcpu *vcpu, unsigned long xirr);
>> */
>> void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu);
>> int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu);
>> +struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
>> + struct kvmppc_xive *xive, int irq);
>> +void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb);
>>
>> #endif /* CONFIG_KVM_XICS */
>> #endif /* _KVM_PPC_BOOK3S_XICS_H */
>> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
>> index d1cc18a5b1c4..6f950ecb3592 100644
>> --- a/arch/powerpc/kvm/book3s_xive.c
>> +++ b/arch/powerpc/kvm/book3s_xive.c
>
> I wonder if we should rename this book3s_xics_on_xive.c or something
> at some point, I keep getting confused because I forget that this is
> only dealing with host xive, not guest xive.
I am fine with renaming. Any objections ? book3s_xics_p9.c ?
>> @@ -1485,8 +1485,8 @@ static int xive_get_source(struct kvmppc_xive *xive, long irq, u64 addr)
>> return 0;
>> }
>>
>> -static struct kvmppc_xive_src_block *xive_create_src_block(struct kvmppc_xive *xive,
>> - int irq)
>> +struct kvmppc_xive_src_block *kvmppc_xive_create_src_block(
>> + struct kvmppc_xive *xive, int irq)
>> {
>> struct kvm *kvm = xive->kvm;
>> struct kvmppc_xive_src_block *sb;
>
> It's odd that this function, now used from the xive-on-xive path as
> well as the xics-on-xive path references KVMPPC_XICS_ICS_SHIFT a few
> lines down from this change.
Yes. This is because of the definition of the struct kvmppc_xive_src_block.
We could introduce new defines for XIVE or a common set of defines for
XICS and XIVE.
>> @@ -1565,7 +1565,7 @@ static int xive_set_source(struct kvmppc_xive *xive, long irq, u64 addr)
>> sb = kvmppc_xive_find_source(xive, irq, &idx);
>> if (!sb) {
>> pr_devel("No source, creating source block...\n");
>> - sb = xive_create_src_block(xive, irq);
>> + sb = kvmppc_xive_create_src_block(xive, irq);
>> if (!sb) {
>> pr_devel("Failed to create block...\n");
>> return -ENOMEM;
>> @@ -1789,7 +1789,7 @@ static void kvmppc_xive_cleanup_irq(u32 hw_num, struct xive_irq_data *xd)
>> xive_cleanup_irq_data(xd);
>> }
>>
>> -static void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
>> +void kvmppc_xive_free_sources(struct kvmppc_xive_src_block *sb)
>> {
>> int i;
>>
>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
>> index 1f3da47a4a6a..a9b2d2d9af99 100644
>> --- a/arch/powerpc/kvm/book3s_xive_native.c
>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
>> @@ -31,6 +31,29 @@
>>
>> #include "book3s_xive.h"
>>
>> +/*
>> + * TODO: introduce a common template file with the XIVE native layer
>> + * and the XICS-on-XIVE glue for the utility functions
>> + */
>> +#define __x_eoi_page(xd) ((void __iomem *)((xd)->eoi_mmio))
>> +#define __x_trig_page(xd) ((void __iomem *)((xd)->trig_mmio))
>> +#define __x_readq __raw_readq
>> +#define __x_writeq __raw_writeq
>> +
>> +static u8 xive_vm_esb_load(struct xive_irq_data *xd, u32 offset)
>> +{
>> + u64 val;
>> +
>> + if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
>> + offset |= offset << 4;
>> +
>> + val = __x_readq(__x_eoi_page(xd) + offset);
>> +#ifdef __LITTLE_ENDIAN__
>> + val >>= 64-8;
>> +#endif
>> + return (u8)val;
>> +}
>> +
>> static void kvmppc_xive_native_cleanup_queue(struct kvm_vcpu *vcpu, int prio)
>> {
>> struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> @@ -153,12 +176,89 @@ int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
>> return rc;
>> }
>>
>> +static int kvmppc_xive_native_set_source(struct kvmppc_xive *xive, long irq,
>> + u64 addr)
>> +{
>> + struct kvmppc_xive_src_block *sb;
>> + struct kvmppc_xive_irq_state *state;
>> + u64 __user *ubufp = (u64 __user *) addr;
>> + u64 val;
>> + u16 idx;
>> +
>> + pr_devel("%s irq=0x%lx\n", __func__, irq);
>> +
>> + if (irq < KVMPPC_XIVE_FIRST_IRQ || irq >= KVMPPC_XIVE_NR_IRQS)
>> + return -E2BIG;
>> +
>> + sb = kvmppc_xive_find_source(xive, irq, &idx);
>> + if (!sb) {
>> + pr_debug("No source, creating source block...\n");
>> + sb = kvmppc_xive_create_src_block(xive, irq);
>> + if (!sb) {
>> + pr_err("Failed to create block...\n");
>> + return -ENOMEM;
>> + }
>> + }
>> + state = &sb->irq_state[idx];
>> +
>> + if (get_user(val, ubufp)) {
>> + pr_err("fault getting user info !\n");
>> + return -EFAULT;
>> + }
>
> You should validate the value loaded here to check it doesn't have any
> bits set we don't know about.
ok
>
>> +
>> + /*
>> + * If the source doesn't already have an IPI, allocate
>> + * one and get the corresponding data
>> + */
>> + if (!state->ipi_number) {
>> + state->ipi_number = xive_native_alloc_irq();
>> + if (state->ipi_number == 0) {
>> + pr_err("Failed to allocate IRQ !\n");
>> + return -ENXIO;
>> + }
>> + xive_native_populate_irq_data(state->ipi_number,
>> + &state->ipi_data);
>> + pr_debug("%s allocated hw_irq=0x%x for irq=0x%lx\n", __func__,
>> + state->ipi_number, irq);
>> + }
>> +
>> + arch_spin_lock(&sb->lock);
>
> Why the direct call to arch_spin_lock() rather than just spin_lock()?
Paul answered this question but may be I should make the effort to
decouple both devices on this aspect.
Thanks,
C.
>> +
>> + /* Restore LSI state */
>> + if (val & KVM_XIVE_LEVEL_SENSITIVE) {
>> + state->lsi = true;
>> + if (val & KVM_XIVE_LEVEL_ASSERTED)
>> + state->asserted = true;
>> + pr_devel(" LSI ! Asserted=%d\n", state->asserted);
>> + }
>> +
>> + /* Mask IRQ to start with */
>> + state->act_server = 0;
>> + state->act_priority = MASKED;
>> + xive_vm_esb_load(&state->ipi_data, XIVE_ESB_SET_PQ_01);
>> + xive_native_configure_irq(state->ipi_number, 0, MASKED, 0);
>> +
>> + /* Increment the number of valid sources and mark this one valid */
>> + if (!state->valid)
>> + xive->src_count++;
>> + state->valid = true;
>> +
>> + arch_spin_unlock(&sb->lock);
>> +
>> + return 0;
>> +}
>> +
>> static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>> struct kvm_device_attr *attr)
>> {
>> + struct kvmppc_xive *xive = dev->private;
>> +
>> switch (attr->group) {
>> case KVM_DEV_XIVE_GRP_CTRL:
>> break;
>> + case KVM_DEV_XIVE_GRP_SOURCE:
>> + return kvmppc_xive_native_set_source(xive, attr->attr,
>> + attr->addr);
>> }
>> return -ENXIO;
>> }
>> @@ -175,6 +275,11 @@ static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
>> switch (attr->group) {
>> case KVM_DEV_XIVE_GRP_CTRL:
>> break;
>> + case KVM_DEV_XIVE_GRP_SOURCE:
>> + if (attr->attr >= KVMPPC_XIVE_FIRST_IRQ &&
>> + attr->attr < KVMPPC_XIVE_NR_IRQS)
>> + return 0;
>> + break;
>> }
>> return -ENXIO;
>> }
>> @@ -183,6 +288,7 @@ static void kvmppc_xive_native_free(struct kvm_device *dev)
>> {
>> struct kvmppc_xive *xive = dev->private;
>> struct kvm *kvm = xive->kvm;
>> + int i;
>>
>> debugfs_remove(xive->dentry);
>>
>> @@ -191,6 +297,14 @@ static void kvmppc_xive_native_free(struct kvm_device *dev)
>> if (kvm)
>> kvm->arch.xive = NULL;
>>
>> + /* Mask and free interrupts */
>> + for (i = 0; i <= xive->max_sbid; i++) {
>> + if (xive->src_blocks[i])
>> + kvmppc_xive_free_sources(xive->src_blocks[i]);
>> + kfree(xive->src_blocks[i]);
>> + xive->src_blocks[i] = NULL;
>> + }
>> +
>> if (xive->vp_base != XIVE_INVALID_VP)
>> xive_native_free_vp_block(xive->vp_base);
>>
>> diff --git a/Documentation/virtual/kvm/devices/xive.txt b/Documentation/virtual/kvm/devices/xive.txt
>> index fdbd2ff92a88..cd8bfc37b72e 100644
>> --- a/Documentation/virtual/kvm/devices/xive.txt
>> +++ b/Documentation/virtual/kvm/devices/xive.txt
>> @@ -17,3 +17,18 @@ the legacy interrupt mode, referred as XICS (POWER7/8).
>>
>> 1. KVM_DEV_XIVE_GRP_CTRL
>> Provides global controls on the device
>> +
>> + 2. KVM_DEV_XIVE_GRP_SOURCE (write only)
>> + Initializes a new source in the XIVE device and mask it.
>> + Attributes:
>> + Interrupt source number (64-bit)
>> + The kvm_device_attr.addr points to a __u64 value:
>> + bits: | 63 .... 2 | 1 | 0
>> + values: | unused | level | type
>> + - type: 0:MSI 1:LSI
>> + - level: assertion level in case of an LSI.
>> + Errors:
>> + -E2BIG: Interrupt source number is out of range
>> + -ENOMEM: Could not create a new source block
>> + -EFAULT: Invalid user pointer for attr->addr.
>> + -ENXIO: Could not allocate underlying HW interrupt
>
^ permalink raw reply
* Re: [PATCH v2 03/16] KVM: PPC: Book3S HV: XIVE: introduce a new capability KVM_CAP_PPC_IRQ_XIVE
From: Cédric Le Goater @ 2019-03-12 14:10 UTC (permalink / raw)
To: Paul Mackerras, David Gibson; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <20190225045931.GB20501@blackberry>
On 2/25/19 5:59 AM, Paul Mackerras wrote:
> On Mon, Feb 25, 2019 at 11:35:27AM +1100, David Gibson wrote:
>> On Fri, Feb 22, 2019 at 12:28:27PM +0100, Cédric Le Goater wrote:
>>> + xc->xive = xive;
>>> + xc->vcpu = vcpu;
>>> + xc->server_num = cpu;
>>> + xc->vp_id = xive->vp_base + cpu;
>>
>> Hrm. This ties the internal VP id to the userspace chosen server
>> number, which isn't ideal. It puts a constraint on those server
>> numbers that you wouldn't otherwise have.
>
> We should probably do the same as the xics-on-xive code, which is to
> put the server number through kvmppc_pack_vcpu_id(), which is a
> folding function that maps the QEMU vcpu id (which is the server
> number) down to the range 0..KVM_MAX_VCPUS-1, and works for the
> allocation patterns used in the various vSMT modes.
yes. I will see how it goes.
Thanks,
C.
^ permalink raw reply
* Re: [PATCH v2 0/2] Append new variables to vmcoreinfo (PTRS_PER_PGD for arm64 and MAX_PHYSMEM_BITS for all archs)
From: Bhupesh Sharma @ 2019-03-12 14:24 UTC (permalink / raw)
To: Dave Young
Cc: Mark Rutland, Kazuhito Hagio, x86, Will Deacon, linux-kernel,
Ingo Molnar, Paul Mackerras, James Morse, Boris Petkov,
Thomas Gleixner, bhupesh.linux, linuxppc-dev, kexec,
linux-arm-kernel, Dave Anderson
In-Reply-To: <20190311090553.GA12130@dhcp-128-65.nay.redhat.com>
Hi Dave,
On 03/11/2019 02:35 PM, Dave Young wrote:
> Hi Bhupesh,
> On 03/10/19 at 03:34pm, Bhupesh Sharma wrote:
>> Changes since v1:
>> ----------------
>> - v1 was sent out as a single patch which can be seen here:
>> http://lists.infradead.org/pipermail/kexec/2019-February/022411.html
>>
>> - v2 breaks the single patch into two independent patches:
>> [PATCH 1/2] appends 'PTRS_PER_PGD' to vmcoreinfo for arm64 arch, whereas
>> [PATCH 2/2] appends 'MAX_PHYSMEM_BITS' to vmcoreinfo in core kernel code (all archs)
>>
>> This patchset primarily fixes the regression reported in user-space
>> utilities like 'makedumpfile' and 'crash-utility' on arm64 architecture
>> with the availability of 52-bit address space feature in underlying
>> kernel. These regressions have been reported both on CPUs which don't
>> support ARMv8.2 extensions (i.e. LVA, LPA) and are running newer kernels
>> and also on prototype platforms (like ARMv8 FVP simulator model) which
>> support ARMv8.2 extensions and are running newer kernels.
>>
>> The reason for these regressions is that right now user-space tools
>> have no direct access to these values (since these are not exported
>> from the kernel) and hence need to rely on a best-guess method of
>> determining value of 'PTRS_PER_PGD' and 'MAX_PHYSMEM_BITS' supported
>> by underlying kernel.
>>
>> Exporting these values via vmcoreinfo will help user-land in such cases.
>> In addition, as per suggestion from makedumpfile maintainer (Kazu)
>> during v1 review, it makes more sense to append 'MAX_PHYSMEM_BITS' to
>> vmcoreinfo in the core code itself rather than in arm64 arch-specific
>> code, so that the user-space code for other archs can also benefit from
>> this addition to the vmcoreinfo and use it as a standard way of
>> determining 'SECTIONS_SHIFT' value in user-land.
>>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: James Morse <james.morse@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Boris Petkov <bp@alien8.de>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Dave Anderson <anderson@redhat.com>
>> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
>> Cc: x86@kernel.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: kexec@lists.infradead.org
>>
>> Bhupesh Sharma (2):
>> arm64, vmcoreinfo : Append 'PTRS_PER_PGD' to vmcoreinfo
>> crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
>>
>> arch/arm64/kernel/crash_core.c | 1 +
>> kernel/crash_core.c | 1 +
>> 2 files changed, 2 insertions(+)
>>
>
> Lianbo's document patch has been merged, would you mind to add vmcoreinfo doc
> patch as well in your series?
Thanks for the inputs. Will add it to the v3.
Let's wait for other comments/reviews, before I spin a version 3.
Regards,
Bhupesh
^ permalink raw reply
* Re: [PATCH v2 03/16] KVM: PPC: Book3S HV: XIVE: introduce a new capability KVM_CAP_PPC_IRQ_XIVE
From: Cédric Le Goater @ 2019-03-12 14:03 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190225003527.GG7668@umbus.fritz.box>
On 2/25/19 1:35 AM, David Gibson wrote:
> On Fri, Feb 22, 2019 at 12:28:27PM +0100, Cédric Le Goater wrote:
>> The user interface exposes a new capability to let QEMU connect the
>> vCPU to the XIVE KVM device if required. The capability is only
>> advertised on a PowerNV Hypervisor as support for nested guests
>> (pseries KVM Hypervisor) is not yet available.
>>
>> Internally, the interface to the new KVM device is protected with a
>> new interrupt mode: KVMPPC_IRQ_XIVE.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> arch/powerpc/include/asm/kvm_host.h | 1 +
>> arch/powerpc/include/asm/kvm_ppc.h | 13 +++
>> arch/powerpc/kvm/book3s_xive.h | 6 ++
>> include/uapi/linux/kvm.h | 1 +
>> arch/powerpc/kvm/book3s_xive.c | 67 +++++++-----
>> arch/powerpc/kvm/book3s_xive_native.c | 144 ++++++++++++++++++++++++++
>> arch/powerpc/kvm/powerpc.c | 33 ++++++
>> Documentation/virtual/kvm/api.txt | 9 ++
>> 8 files changed, 246 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>> index 9f75a75a07f2..eb8581be0ee8 100644
>> --- a/arch/powerpc/include/asm/kvm_host.h
>> +++ b/arch/powerpc/include/asm/kvm_host.h
>> @@ -448,6 +448,7 @@ struct kvmppc_passthru_irqmap {
>> #define KVMPPC_IRQ_DEFAULT 0
>> #define KVMPPC_IRQ_MPIC 1
>> #define KVMPPC_IRQ_XICS 2 /* Includes a XIVE option */
>> +#define KVMPPC_IRQ_XIVE 3 /* XIVE native exploitation mode */
>>
>> #define MMIO_HPTE_CACHE_SIZE 4
>>
>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
>> index 4b72ddde7dc1..1e61877fe147 100644
>> --- a/arch/powerpc/include/asm/kvm_ppc.h
>> +++ b/arch/powerpc/include/asm/kvm_ppc.h
>> @@ -594,6 +594,14 @@ extern int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 irq,
>> int level, bool line_status);
>> extern void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu);
>>
>> +static inline int kvmppc_xive_enabled(struct kvm_vcpu *vcpu)
>> +{
>> + return vcpu->arch.irq_type == KVMPPC_IRQ_XIVE;
>> +}
>> +
>> +extern int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
>> + struct kvm_vcpu *vcpu, u32 cpu);
>> +extern void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu);
>> extern void kvmppc_xive_native_init_module(void);
>> extern void kvmppc_xive_native_exit_module(void);
>>
>> @@ -621,6 +629,11 @@ static inline int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 ir
>> int level, bool line_status) { return -ENODEV; }
>> static inline void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu) { }
>>
>> +static inline int kvmppc_xive_enabled(struct kvm_vcpu *vcpu)
>> + { return 0; }
>> +static inline int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
>> + struct kvm_vcpu *vcpu, u32 cpu) { return -EBUSY; }
>> +static inline void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu) { }
>> static inline void kvmppc_xive_native_init_module(void) { }
>> static inline void kvmppc_xive_native_exit_module(void) { }
>>
>> diff --git a/arch/powerpc/kvm/book3s_xive.h b/arch/powerpc/kvm/book3s_xive.h
>> index a08ae6fd4c51..bcb1bbcf0359 100644
>> --- a/arch/powerpc/kvm/book3s_xive.h
>> +++ b/arch/powerpc/kvm/book3s_xive.h
>> @@ -248,5 +248,11 @@ extern int (*__xive_vm_h_ipi)(struct kvm_vcpu *vcpu, unsigned long server,
>> extern int (*__xive_vm_h_cppr)(struct kvm_vcpu *vcpu, unsigned long cppr);
>> extern int (*__xive_vm_h_eoi)(struct kvm_vcpu *vcpu, unsigned long xirr);
>>
>> +/*
>> + * Common Xive routines for XICS-over-XIVE and XIVE native
>> + */
>> +void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu);
>> +int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu);
>> +
>> #endif /* CONFIG_KVM_XICS */
>> #endif /* _KVM_PPC_BOOK3S_XICS_H */
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index e6368163d3a0..52bf74a1616e 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -988,6 +988,7 @@ struct kvm_ppc_resize_hpt {
>> #define KVM_CAP_ARM_VM_IPA_SIZE 165
>> #define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166
>> #define KVM_CAP_HYPERV_CPUID 167
>> +#define KVM_CAP_PPC_IRQ_XIVE 168
>>
>> #ifdef KVM_CAP_IRQ_ROUTING
>>
>> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
>> index f78d002f0fe0..d1cc18a5b1c4 100644
>> --- a/arch/powerpc/kvm/book3s_xive.c
>> +++ b/arch/powerpc/kvm/book3s_xive.c
>> @@ -1049,7 +1049,7 @@ int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,
>> }
>> EXPORT_SYMBOL_GPL(kvmppc_xive_clr_mapped);
>>
>> -static void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu)
>> +void kvmppc_xive_disable_vcpu_interrupts(struct kvm_vcpu *vcpu)
>> {
>> struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> struct kvm *kvm = vcpu->kvm;
>> @@ -1883,6 +1883,43 @@ static int kvmppc_xive_create(struct kvm_device *dev, u32 type)
>> return 0;
>> }
>>
>> +int kvmppc_xive_debug_show_queues(struct seq_file *m, struct kvm_vcpu *vcpu)
>> +{
>> + struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> + unsigned int i;
>> +
>> + for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
>> + struct xive_q *q = &xc->queues[i];
>> + u32 i0, i1, idx;
>> +
>> + if (!q->qpage && !xc->esc_virq[i])
>> + continue;
>> +
>> + seq_printf(m, " [q%d]: ", i);
>> +
>> + if (q->qpage) {
>> + idx = q->idx;
>> + i0 = be32_to_cpup(q->qpage + idx);
>> + idx = (idx + 1) & q->msk;
>> + i1 = be32_to_cpup(q->qpage + idx);
>> + seq_printf(m, "T=%d %08x %08x...\n", q->toggle,
>> + i0, i1);
>> + }
>> + if (xc->esc_virq[i]) {
>> + struct irq_data *d = irq_get_irq_data(xc->esc_virq[i]);
>> + struct xive_irq_data *xd =
>> + irq_data_get_irq_handler_data(d);
>> + u64 pq = xive_vm_esb_load(xd, XIVE_ESB_GET);
>> +
>> + seq_printf(m, "E:%c%c I(%d:%llx:%llx)",
>> + (pq & XIVE_ESB_VAL_P) ? 'P' : 'p',
>> + (pq & XIVE_ESB_VAL_Q) ? 'Q' : 'q',
>> + xc->esc_virq[i], pq, xd->eoi_page);
>> + seq_puts(m, "\n");
>> + }
>> + }
>> + return 0;
>> +}
>>
>> static int xive_debug_show(struct seq_file *m, void *private)
>> {
>> @@ -1908,7 +1945,6 @@ static int xive_debug_show(struct seq_file *m, void *private)
>>
>> kvm_for_each_vcpu(i, vcpu, kvm) {
>> struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> - unsigned int i;
>>
>> if (!xc)
>> continue;
>> @@ -1918,33 +1954,8 @@ static int xive_debug_show(struct seq_file *m, void *private)
>> xc->server_num, xc->cppr, xc->hw_cppr,
>> xc->mfrr, xc->pending,
>> xc->stat_rm_h_xirr, xc->stat_vm_h_xirr);
>> - for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
>> - struct xive_q *q = &xc->queues[i];
>> - u32 i0, i1, idx;
>>
>> - if (!q->qpage && !xc->esc_virq[i])
>> - continue;
>> -
>> - seq_printf(m, " [q%d]: ", i);
>> -
>> - if (q->qpage) {
>> - idx = q->idx;
>> - i0 = be32_to_cpup(q->qpage + idx);
>> - idx = (idx + 1) & q->msk;
>> - i1 = be32_to_cpup(q->qpage + idx);
>> - seq_printf(m, "T=%d %08x %08x... \n", q->toggle, i0, i1);
>> - }
>> - if (xc->esc_virq[i]) {
>> - struct irq_data *d = irq_get_irq_data(xc->esc_virq[i]);
>> - struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
>> - u64 pq = xive_vm_esb_load(xd, XIVE_ESB_GET);
>> - seq_printf(m, "E:%c%c I(%d:%llx:%llx)",
>> - (pq & XIVE_ESB_VAL_P) ? 'P' : 'p',
>> - (pq & XIVE_ESB_VAL_Q) ? 'Q' : 'q',
>> - xc->esc_virq[i], pq, xd->eoi_page);
>> - seq_printf(m, "\n");
>> - }
>> - }
>> + kvmppc_xive_debug_show_queues(m, vcpu);
>>
>> t_rm_h_xirr += xc->stat_rm_h_xirr;
>> t_rm_h_ipoll += xc->stat_rm_h_ipoll;
>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
>> index e475ce83ad14..1f3da47a4a6a 100644
>> --- a/arch/powerpc/kvm/book3s_xive_native.c
>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
>> @@ -31,6 +31,128 @@
>>
>> #include "book3s_xive.h"
>>
>> +static void kvmppc_xive_native_cleanup_queue(struct kvm_vcpu *vcpu, int prio)
>> +{
>> + struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> + struct xive_q *q = &xc->queues[prio];
>> +
>> + xive_native_disable_queue(xc->vp_id, q, prio);
>> + if (q->qpage) {
>> + put_page(virt_to_page(q->qpage));
>> + q->qpage = NULL;
>> + }
>> +}
>> +
>> +void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu)
>> +{
>> + struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> + int i;
>> +
>> + if (!kvmppc_xive_enabled(vcpu))
>> + return;
>> +
>> + if (!xc)
>> + return;
>> +
>> + pr_devel("native_cleanup_vcpu(cpu=%d)\n", xc->server_num);
>> +
>> + /* Ensure no interrupt is still routed to that VP */
>> + xc->valid = false;
>> + kvmppc_xive_disable_vcpu_interrupts(vcpu);
>> +
>> + /* Disable the VP */
>> + xive_native_disable_vp(xc->vp_id);
>> +
>> + /* Free the queues & associated interrupts */
>> + for (i = 0; i < KVMPPC_XIVE_Q_COUNT; i++) {
>> + /* Free the escalation irq */
>> + if (xc->esc_virq[i]) {
>> + free_irq(xc->esc_virq[i], vcpu);
>> + irq_dispose_mapping(xc->esc_virq[i]);
>> + kfree(xc->esc_virq_names[i]);
>> + xc->esc_virq[i] = 0;
>> + }
>> +
>> + /* Free the queue */
>> + kvmppc_xive_native_cleanup_queue(vcpu, i);
>> + }
>> +
>> + /* Free the VP */
>> + kfree(xc);
>> +
>> + /* Cleanup the vcpu */
>> + vcpu->arch.irq_type = KVMPPC_IRQ_DEFAULT;
>> + vcpu->arch.xive_vcpu = NULL;
>> +}
>> +
>> +int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
>> + struct kvm_vcpu *vcpu, u32 cpu)
>> +{
>> + struct kvmppc_xive *xive = dev->private;
>> + struct kvmppc_xive_vcpu *xc;
>> + int rc;
>> +
>> + pr_devel("native_connect_vcpu(cpu=%d)\n", cpu);
>> +
>> + if (dev->ops != &kvm_xive_native_ops) {
>> + pr_devel("Wrong ops !\n");
>> + return -EPERM;
>> + }
>> + if (xive->kvm != vcpu->kvm)
>> + return -EPERM;
>> + if (vcpu->arch.irq_type != KVMPPC_IRQ_DEFAULT)
>> + return -EBUSY;
>> + if (kvmppc_xive_find_server(vcpu->kvm, cpu)) {
>
> You haven't taken the kvm->lock yet, so couldn't a race mean a
> duplicate server gets inserted after you make this check?
>
>> + pr_devel("Duplicate !\n");
>> + return -EEXIST;
>> + }
>> + if (cpu >= KVM_MAX_VCPUS) {
>> + pr_devel("Out of bounds !\n");
>> + return -EINVAL;
>> + }
>> + xc = kzalloc(sizeof(*xc), GFP_KERNEL);
>> + if (!xc)
>> + return -ENOMEM;
>> +
>> + mutex_lock(&vcpu->kvm->lock);
>> + vcpu->arch.xive_vcpu = xc;
>
> Similarly you don't verify this is NULL after taking the lock, so
> couldn't another thread race and make a connect which gets clobbered
> here?
Yes. this is not very safe ... We need to clean up all the KVM device
methods doing the connection of the presenter to the vCPU AFAICT.
I will fix the XIVE native one for now.
And also, this CPU parameter is useless. There is no reason to connect
a vCPU from another vCPU.
>> + xc->xive = xive;
>> + xc->vcpu = vcpu;
>> + xc->server_num = cpu;
>> + xc->vp_id = xive->vp_base + cpu;
>
> Hrm. This ties the internal VP id to the userspace chosen server
> number, which isn't ideal. It puts a constraint on those server
> numbers that you wouldn't otherwise have.
Ah yes. I should be using the kvmppc_pack_vcpu_id() like we do for
the XICS-over-XIVE device probably. I need to check that it is correct
in this mode.
Thanks,
C.
>> + xc->valid = true;
>> +
>> + rc = xive_native_get_vp_info(xc->vp_id, &xc->vp_cam, &xc->vp_chip_id);
>> + if (rc) {
>> + pr_err("Failed to get VP info from OPAL: %d\n", rc);
>> + goto bail;
>> + }
>> +
>> + /*
>> + * Enable the VP first as the single escalation mode will
>> + * affect escalation interrupts numbering
>> + */
>> + rc = xive_native_enable_vp(xc->vp_id, xive->single_escalation);
>> + if (rc) {
>> + pr_err("Failed to enable VP in OPAL: %d\n", rc);
>> + goto bail;
>> + }
>> +
>> + /* Configure VCPU fields for use by assembly push/pull */
>> + vcpu->arch.xive_saved_state.w01 = cpu_to_be64(0xff000000);
>> + vcpu->arch.xive_cam_word = cpu_to_be32(xc->vp_cam | TM_QW1W2_VO);
>> +
>> + /* TODO: initialize queues ? */
>> +
>> +bail:
>> + vcpu->arch.irq_type = KVMPPC_IRQ_XIVE;
>> + mutex_unlock(&vcpu->kvm->lock);
>> + if (rc)
>> + kvmppc_xive_native_cleanup_vcpu(vcpu);
>> +
>> + return rc;
>> +}
>> +
>> static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>> struct kvm_device_attr *attr)
>> {
>> @@ -126,10 +248,32 @@ static int xive_native_debug_show(struct seq_file *m, void *private)
>> {
>> struct kvmppc_xive *xive = m->private;
>> struct kvm *kvm = xive->kvm;
>> + struct kvm_vcpu *vcpu;
>> + unsigned int i;
>>
>> if (!kvm)
>> return 0;
>>
>> + seq_puts(m, "=========\nVCPU state\n=========\n");
>> +
>> + kvm_for_each_vcpu(i, vcpu, kvm) {
>> + struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>> +
>> + if (!xc)
>> + continue;
>> +
>> + seq_printf(m, "cpu server %#x NSR=%02x CPPR=%02x IBP=%02x PIPR=%02x w01=%016llx w2=%08x\n",
>> + xc->server_num,
>> + vcpu->arch.xive_saved_state.nsr,
>> + vcpu->arch.xive_saved_state.cppr,
>> + vcpu->arch.xive_saved_state.ipb,
>> + vcpu->arch.xive_saved_state.pipr,
>> + vcpu->arch.xive_saved_state.w01,
>> + (u32) vcpu->arch.xive_cam_word);
>> +
>> + kvmppc_xive_debug_show_queues(m, vcpu);
>> + }
>> +
>> return 0;
>> }
>>
>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index 8c69af10f91d..a38a643a24dd 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -570,6 +570,12 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>> case KVM_CAP_PPC_GET_CPU_CHAR:
>> r = 1;
>> break;
>> +#ifdef CONFIG_KVM_XIVE
>> + case KVM_CAP_PPC_IRQ_XIVE:
>> + /* only for PowerNV */
>> + r = !!cpu_has_feature(CPU_FTR_HVMODE);
>> + break;
>> +#endif
>>
>> case KVM_CAP_PPC_ALLOC_HTAB:
>> r = hv_enabled;
>> @@ -753,6 +759,9 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
>> else
>> kvmppc_xics_free_icp(vcpu);
>> break;
>> + case KVMPPC_IRQ_XIVE:
>> + kvmppc_xive_native_cleanup_vcpu(vcpu);
>> + break;
>> }
>>
>> kvmppc_core_vcpu_free(vcpu);
>> @@ -1941,6 +1950,30 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
>> break;
>> }
>> #endif /* CONFIG_KVM_XICS */
>> +#ifdef CONFIG_KVM_XIVE
>> + case KVM_CAP_PPC_IRQ_XIVE: {
>> + struct fd f;
>> + struct kvm_device *dev;
>> +
>> + r = -EBADF;
>> + f = fdget(cap->args[0]);
>> + if (!f.file)
>> + break;
>> +
>> + r = -ENXIO;
>> + if (!xive_enabled())
>> + break;
>> +
>> + r = -EPERM;
>> + dev = kvm_device_from_filp(f.file);
>> + if (dev)
>> + r = kvmppc_xive_native_connect_vcpu(dev, vcpu,
>> + cap->args[1]);
>> +
>> + fdput(f);
>> + break;
>> + }
>> +#endif /* CONFIG_KVM_XIVE */
>> #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
>> case KVM_CAP_PPC_FWNMI:
>> r = -EINVAL;
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index 356156f5c52d..1db1435769b4 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -4458,6 +4458,15 @@ struct kvm_sync_regs {
>> struct kvm_vcpu_events events;
>> };
>>
>> +6.75 KVM_CAP_PPC_IRQ_XIVE
>> +
>> +Architectures: ppc
>> +Target: vcpu
>> +Parameters: args[0] is the XIVE device fd
>> + args[1] is the XIVE CPU number (server ID) for this vcpu
>> +
>> +This capability connects the vcpu to an in-kernel XIVE device.
>> +
>> 7. Capabilities that can be enabled on VMs
>> ------------------------------------------
>>
>
^ permalink raw reply
* Re: [PATCH 3/6] x86: clean up _TIF_SYSCALL_EMU handling using ptrace_syscall_enter hook
From: Sudeep Holla @ 2019-03-12 12:09 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Haibo Xu (Arm Technology China), Steve Capper, Catalin Marinas,
jdike@addtoit.com, x86@kernel.org, Will Deacon,
linux-kernel@vger.kernel.org, Oleg Nesterov, Richard Weinberger,
Ingo Molnar, Paul Mackerras, Borislav Petkov, Thomas Gleixner,
Bin Lu (Arm Technology China), linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CALCETrWiF5FKUTmt4jOZJxJr6B_66NFj9TaA3Tnv1uorfkwTMg@mail.gmail.com>
On Mon, Mar 11, 2019 at 08:04:39PM -0700, Andy Lutomirski wrote:
> On Mon, Mar 11, 2019 at 6:35 PM Haibo Xu (Arm Technology China)
> <Haibo.Xu@arm.com> wrote:
> >
[...]
> > For the PTRACE_SYSEMU_SINGLESTEP request, ptrace only need to report(send
> > SIGTRAP) at the entry of a system call, no need to report at the exit of a
> > system call.That's why the old logic-{step = ((flags & (_TIF_SINGLESTEP |
> > _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP)} here try to filter out the special
> > case(PTRACE_SYSEMU_SINGLESTEP).
> >
> > Another way to make sure the logic is fine, you can run some tests with
> > respect to both logic, and to check whether they have the same behavior.
>
> tools/testing/selftests/x86/ptrace_syscall.c has a test intended to
> exercise this. Can one of you either confirm that it does exercise it
> and that it still passes or can you improve the test?
>
I did run the tests which didn't flag anything. I haven't looked at the
details of test implementation, but seem to miss this case. I will see
what can be improved(if it's possible). Also I think single_step_syscall
is the one I need to look for this particular one. Both single_step_syscall
ptrace_syscall reported no errors.
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH 3/6] x86: clean up _TIF_SYSCALL_EMU handling using ptrace_syscall_enter hook
From: Sudeep Holla @ 2019-03-12 12:05 UTC (permalink / raw)
To: Haibo Xu (Arm Technology China)
Cc: Steve Capper, Catalin Marinas, jdike@addtoit.com, x86@kernel.org,
Will Deacon, linux-kernel@vger.kernel.org, Oleg Nesterov,
Richard Weinberger, Ingo Molnar, Paul Mackerras, Andy Lutomirski,
Borislav Petkov, Thomas Gleixner, Bin Lu (Arm Technology China),
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <65b00ea1-f784-4fb4-2a98-49fa44d9fa8f@arm.com>
On Tue, Mar 12, 2019 at 01:34:44AM +0000, Haibo Xu (Arm Technology China) wrote:
> On 2019/3/12 2:34, Sudeep Holla wrote:
> > (I thought I had sent this email, last Tuesday itself, but saw this in my
> > draft today, something went wrong, sorry for the delay)
> >
> > On Tue, Mar 05, 2019 at 02:14:47AM +0000, Haibo Xu (Arm Technology China) wrote:
> >> On 2019/3/4 18:12, Sudeep Holla wrote:
> >>> On Mon, Mar 04, 2019 at 08:25:28AM +0000, Haibo Xu (Arm Technology China) wrote:
> >>>> On 2019/3/1 2:32, Sudeep Holla wrote:
> >>>>> Now that we have a new hook ptrace_syscall_enter that can be called from
> >>>>> syscall entry code and it handles PTRACE_SYSEMU in generic code, we
> >>>>> can do some cleanup using the same in syscall_trace_enter.
> >>>>>
> >>>>> Further the extra logic to find single stepping PTRACE_SYSEMU_SINGLESTEP
> >>>>> in syscall_slow_exit_work seems unnecessary. Let's remove the same.
> >>>>
> >>>> I think we should not change the logic here. Is so, it will double the report of syscall
> >>>> when PTRACE_SYSEMU_SINGLESTEP is enabled.
> >>>>
> >>>
> >>> I don't think that should happen, but I may be missing something.
> >>> Can you explain how ?
> >>>
> >>
> >> When PTRACE_SYSEMU_SINGLESTEP is enabled, both the _TIF_SYSCALL_EMU and
> >> _TIF_SINGLESTEP flags are set, but ptrace only need to report(send SIGTRAP)
> >> at the entry of a system call, no need to report at the exit of a system
> >> call.
> >>
> > Sorry, but I still not get it, we have:
> >
> > step = ((flags & (_TIF_SINGLESTEP | _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP);
> >
> > For me, this is same as:
> > step = ((flags & _TIF_SINGLESTEP) == _TIF_SINGLESTEP)
> > or
> > if (flags & _TIF_SINGLESTEP)
> > step = true;
> >
>
> I don't think so! As I mentioned in the last email loop, when
> PTRACE_SYSEMU_SINGLESTE is enabled, both the _TIF_SYSCALL_EMU and
> _TIF_SINGLESTEP flags are set, in which case the step should be "false" for
> the old logic. But with the new logic, the step is "true".
>
Ah right, sorry I missed that.
> > So when PTRACE_SYSEMU_SINGLESTEP, _TIF_SYSCALL_EMU and _TIF_SINGLESTEP
> > are set and step evaluates to true.
> >
> > So dropping _TIF_SYSCALL_EMU here should be fine. Am I still missing
> > something ?
> >
> > --
> > Regards,
> > Sudeep
> >
>
> For the PTRACE_SYSEMU_SINGLESTEP request, ptrace only need to report(send
> SIGTRAP) at the entry of a system call, no need to report at the exit of a
> system call.That's why the old logic-{step = ((flags & (_TIF_SINGLESTEP |
> _TIF_SYSCALL_EMU)) == _TIF_SINGLESTEP)} here try to filter out the special
> case(PTRACE_SYSEMU_SINGLESTEP).
>
Understood
> Another way to make sure the logic is fine, you can run some tests with
> respect to both logic, and to check whether they have the same behavior.
>
I did run selftests after Andy Lutomirski pointed out. Nothing got flagged,
I haven't looked at the tests themselves yet, but it clearly misses this
case.
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH v2 02/16] KVM: PPC: Book3S HV: add a new KVM device for the XIVE native exploitation mode
From: Cédric Le Goater @ 2019-03-12 11:14 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190225000848.GF7668@umbus.fritz.box>
On 2/25/19 1:08 AM, David Gibson wrote:
> On Fri, Feb 22, 2019 at 12:28:26PM +0100, Cédric Le Goater wrote:
>> This is the basic framework for the new KVM device supporting the XIVE
>> native exploitation mode. The user interface exposes a new KVM device
>> to be created by QEMU when running on a L0 hypervisor only. Support
>> for nested guests is not available yet.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> arch/powerpc/include/asm/kvm_host.h | 1 +
>> arch/powerpc/include/asm/kvm_ppc.h | 8 +
>> arch/powerpc/include/uapi/asm/kvm.h | 3 +
>> include/uapi/linux/kvm.h | 2 +
>> arch/powerpc/kvm/book3s.c | 7 +-
>> arch/powerpc/kvm/book3s_xive_native.c | 191 +++++++++++++++++++++
>> Documentation/virtual/kvm/devices/xive.txt | 19 ++
>> arch/powerpc/kvm/Makefile | 2 +-
>> 8 files changed, 231 insertions(+), 2 deletions(-)
>> create mode 100644 arch/powerpc/kvm/book3s_xive_native.c
>> create mode 100644 Documentation/virtual/kvm/devices/xive.txt
>>
>> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
>> index 091430339db1..9f75a75a07f2 100644
>> --- a/arch/powerpc/include/asm/kvm_host.h
>> +++ b/arch/powerpc/include/asm/kvm_host.h
>> @@ -220,6 +220,7 @@ extern struct kvm_device_ops kvm_xics_ops;
>> struct kvmppc_xive;
>> struct kvmppc_xive_vcpu;
>> extern struct kvm_device_ops kvm_xive_ops;
>> +extern struct kvm_device_ops kvm_xive_native_ops;
>>
>> struct kvmppc_passthru_irqmap;
>>
>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
>> index b3bf4f61b30c..4b72ddde7dc1 100644
>> --- a/arch/powerpc/include/asm/kvm_ppc.h
>> +++ b/arch/powerpc/include/asm/kvm_ppc.h
>> @@ -593,6 +593,10 @@ extern int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval);
>> extern int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 irq,
>> int level, bool line_status);
>> extern void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu);
>> +
>> +extern void kvmppc_xive_native_init_module(void);
>> +extern void kvmppc_xive_native_exit_module(void);
>> +
>> #else
>> static inline int kvmppc_xive_set_xive(struct kvm *kvm, u32 irq, u32 server,
>> u32 priority) { return -1; }
>> @@ -616,6 +620,10 @@ static inline int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval) { retur
>> static inline int kvmppc_xive_set_irq(struct kvm *kvm, int irq_source_id, u32 irq,
>> int level, bool line_status) { return -ENODEV; }
>> static inline void kvmppc_xive_push_vcpu(struct kvm_vcpu *vcpu) { }
>> +
>> +static inline void kvmppc_xive_native_init_module(void) { }
>> +static inline void kvmppc_xive_native_exit_module(void) { }
>> +
>> #endif /* CONFIG_KVM_XIVE */
>>
>> #ifdef CONFIG_PPC_POWERNV
>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>> index 8c876c166ef2..b002c0c67787 100644
>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>> @@ -675,4 +675,7 @@ struct kvm_ppc_cpu_char {
>> #define KVM_XICS_PRESENTED (1ULL << 43)
>> #define KVM_XICS_QUEUED (1ULL << 44)
>>
>> +/* POWER9 XIVE Native Interrupt Controller */
>> +#define KVM_DEV_XIVE_GRP_CTRL 1
>> +
>> #endif /* __LINUX_KVM_POWERPC_H */
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 6d4ea4b6c922..e6368163d3a0 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -1211,6 +1211,8 @@ enum kvm_device_type {
>> #define KVM_DEV_TYPE_ARM_VGIC_V3 KVM_DEV_TYPE_ARM_VGIC_V3
>> KVM_DEV_TYPE_ARM_VGIC_ITS,
>> #define KVM_DEV_TYPE_ARM_VGIC_ITS KVM_DEV_TYPE_ARM_VGIC_ITS
>> + KVM_DEV_TYPE_XIVE,
>> +#define KVM_DEV_TYPE_XIVE KVM_DEV_TYPE_XIVE
>> KVM_DEV_TYPE_MAX,
>> };
>>
>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>> index 601c094f15ab..96d43f091255 100644
>> --- a/arch/powerpc/kvm/book3s.c
>> +++ b/arch/powerpc/kvm/book3s.c
>> @@ -1040,6 +1040,9 @@ static int kvmppc_book3s_init(void)
>> if (xics_on_xive()) {
>> kvmppc_xive_init_module();
>> kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);
>> + kvmppc_xive_native_init_module();
>> + kvm_register_device_ops(&kvm_xive_native_ops,
>> + KVM_DEV_TYPE_XIVE);
>> } else
>> #endif
>> kvm_register_device_ops(&kvm_xics_ops, KVM_DEV_TYPE_XICS);
>> @@ -1050,8 +1053,10 @@ static int kvmppc_book3s_init(void)
>> static void kvmppc_book3s_exit(void)
>> {
>> #ifdef CONFIG_KVM_XICS
>> - if (xics_on_xive())
>> + if (xics_on_xive()) {
>> kvmppc_xive_exit_module();
>> + kvmppc_xive_native_exit_module();
>> + }
>> #endif
>> #ifdef CONFIG_KVM_BOOK3S_32_HANDLER
>> kvmppc_book3s_exit_pr();
>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
>> new file mode 100644
>> index 000000000000..e475ce83ad14
>> --- /dev/null
>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
>> @@ -0,0 +1,191 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2017-2019, IBM Corporation.
>> + */
>> +
>> +#define pr_fmt(fmt) "xive-kvm: " fmt
>> +
>> +#include <linux/anon_inodes.h>
>> +#include <linux/kernel.h>
>> +#include <linux/kvm_host.h>
>> +#include <linux/err.h>
>> +#include <linux/gfp.h>
>> +#include <linux/spinlock.h>
>> +#include <linux/delay.h>
>> +#include <linux/percpu.h>
>> +#include <linux/cpumask.h>
>> +#include <asm/uaccess.h>
>> +#include <asm/kvm_book3s.h>
>> +#include <asm/kvm_ppc.h>
>> +#include <asm/hvcall.h>
>> +#include <asm/xics.h>
>> +#include <asm/xive.h>
>> +#include <asm/xive-regs.h>
>> +#include <asm/debug.h>
>> +#include <asm/debugfs.h>
>> +#include <asm/time.h>
>> +#include <asm/opal.h>
>> +
>> +#include <linux/debugfs.h>
>> +#include <linux/seq_file.h>
>> +
>> +#include "book3s_xive.h"
>> +
>> +static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>> + struct kvm_device_attr *attr)
>> +{
>> + switch (attr->group) {
>> + case KVM_DEV_XIVE_GRP_CTRL:
>> + break;
>> + }
>> + return -ENXIO;
>> +}
>> +
>> +static int kvmppc_xive_native_get_attr(struct kvm_device *dev,
>> + struct kvm_device_attr *attr)
>> +{
>> + return -ENXIO;
>> +}
>> +
>> +static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
>> + struct kvm_device_attr *attr)
>> +{
>> + switch (attr->group) {
>> + case KVM_DEV_XIVE_GRP_CTRL:
>> + break;
>> + }
>> + return -ENXIO;
>> +}
>> +
>> +static void kvmppc_xive_native_free(struct kvm_device *dev)
>> +{
>> + struct kvmppc_xive *xive = dev->private;
>> + struct kvm *kvm = xive->kvm;
>> +
>> + debugfs_remove(xive->dentry);
>> +
>> + pr_devel("Destroying xive native device\n");
>> +
>> + if (kvm)
>> + kvm->arch.xive = NULL;
>> +
>> + if (xive->vp_base != XIVE_INVALID_VP)
>> + xive_native_free_vp_block(xive->vp_base);
>> +
>> + kfree(xive);
>> + kfree(dev);
>> +}
>> +
>> +static int kvmppc_xive_native_create(struct kvm_device *dev, u32 type)
>> +{
>> + struct kvmppc_xive *xive;
>> + struct kvm *kvm = dev->kvm;
>> + int ret = 0;
>> +
>> + pr_devel("Creating xive native device\n");
>> +
>> + if (kvm->arch.xive)
>> + return -EEXIST;
>> +
>> + xive = kzalloc(sizeof(*xive), GFP_KERNEL);
>> + if (!xive)
>> + return -ENOMEM;
>> +
>> + dev->private = xive;
>> + xive->dev = dev;
>> + xive->kvm = kvm;
>> + kvm->arch.xive = xive;
>> +
>> + /* We use the default queue size set by the host */
>
> IIUC the queue is examined directly by the guest, so the guest must
> know its size. In which case letting the host decide the size would
> be a problem for migration.
yes. This is a left over from the XICS-over-XIVE KVM device. I will
remove the code, we don't use it.
Thanks,
C.
>> + xive->q_order = xive_native_default_eq_shift();
>> + if (xive->q_order < PAGE_SHIFT)
>> + xive->q_page_order = 0;
>> + else
>> + xive->q_page_order = xive->q_order - PAGE_SHIFT;
>> +
>> + /*
>> + * Allocate a bunch of VPs. KVM_MAX_VCPUS is a large value for
>> + * a default. Getting the max number of CPUs the VM was
>> + * configured with would improve our usage of the XIVE VP space.
>> + */
>> + xive->vp_base = xive_native_alloc_vp_block(KVM_MAX_VCPUS);
>> + pr_devel("VP_Base=%x\n", xive->vp_base);
>> +
>> + if (xive->vp_base == XIVE_INVALID_VP)
>> + ret = -ENOMEM;
>> +
>> + xive->single_escalation = xive_native_has_single_escalation();
>> +
>> + if (ret)
>> + kfree(xive);
>> +
>> + return ret;
>> +}
>> +
>> +static int xive_native_debug_show(struct seq_file *m, void *private)
>> +{
>> + struct kvmppc_xive *xive = m->private;
>> + struct kvm *kvm = xive->kvm;
>> +
>> + if (!kvm)
>> + return 0;
>> +
>> + return 0;
>> +}
>> +
>> +static int xive_native_debug_open(struct inode *inode, struct file *file)
>> +{
>> + return single_open(file, xive_native_debug_show, inode->i_private);
>> +}
>> +
>> +static const struct file_operations xive_native_debug_fops = {
>> + .open = xive_native_debug_open,
>> + .read = seq_read,
>> + .llseek = seq_lseek,
>> + .release = single_release,
>> +};
>> +
>> +static void xive_native_debugfs_init(struct kvmppc_xive *xive)
>> +{
>> + char *name;
>> +
>> + name = kasprintf(GFP_KERNEL, "kvm-xive-%p", xive);
>> + if (!name) {
>> + pr_err("%s: no memory for name\n", __func__);
>> + return;
>> + }
>> +
>> + xive->dentry = debugfs_create_file(name, 0444, powerpc_debugfs_root,
>> + xive, &xive_native_debug_fops);
>> +
>> + pr_debug("%s: created %s\n", __func__, name);
>> + kfree(name);
>> +}
>> +
>> +static void kvmppc_xive_native_init(struct kvm_device *dev)
>> +{
>> + struct kvmppc_xive *xive = (struct kvmppc_xive *)dev->private;
>> +
>> + /* Register some debug interfaces */
>> + xive_native_debugfs_init(xive);
>> +}
>> +
>> +struct kvm_device_ops kvm_xive_native_ops = {
>> + .name = "kvm-xive-native",
>> + .create = kvmppc_xive_native_create,
>> + .init = kvmppc_xive_native_init,
>> + .destroy = kvmppc_xive_native_free,
>> + .set_attr = kvmppc_xive_native_set_attr,
>> + .get_attr = kvmppc_xive_native_get_attr,
>> + .has_attr = kvmppc_xive_native_has_attr,
>> +};
>> +
>> +void kvmppc_xive_native_init_module(void)
>> +{
>> + ;
>> +}
>> +
>> +void kvmppc_xive_native_exit_module(void)
>> +{
>> + ;
>> +}
>> diff --git a/Documentation/virtual/kvm/devices/xive.txt b/Documentation/virtual/kvm/devices/xive.txt
>> new file mode 100644
>> index 000000000000..fdbd2ff92a88
>> --- /dev/null
>> +++ b/Documentation/virtual/kvm/devices/xive.txt
>> @@ -0,0 +1,19 @@
>> +POWER9 eXternal Interrupt Virtualization Engine (XIVE Gen1)
>> +==========================================================
>> +
>> +Device types supported:
>> + KVM_DEV_TYPE_XIVE POWER9 XIVE Interrupt Controller generation 1
>> +
>> +This device acts as a VM interrupt controller. It provides the KVM
>> +interface to configure the interrupt sources of a VM in the underlying
>> +POWER9 XIVE interrupt controller.
>> +
>> +Only one XIVE instance may be instantiated. A guest XIVE device
>> +requires a POWER9 host and the guest OS should have support for the
>> +XIVE native exploitation interrupt mode. If not, it should run using
>> +the legacy interrupt mode, referred as XICS (POWER7/8).
>> +
>> +* Groups:
>> +
>> + 1. KVM_DEV_XIVE_GRP_CTRL
>> + Provides global controls on the device
>> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
>> index 64f1135e7732..806cbe488410 100644
>> --- a/arch/powerpc/kvm/Makefile
>> +++ b/arch/powerpc/kvm/Makefile
>> @@ -99,7 +99,7 @@ endif
>> kvm-book3s_64-objs-$(CONFIG_KVM_XICS) += \
>> book3s_xics.o
>>
>> -kvm-book3s_64-objs-$(CONFIG_KVM_XIVE) += book3s_xive.o
>> +kvm-book3s_64-objs-$(CONFIG_KVM_XIVE) += book3s_xive.o book3s_xive_native.o
>> kvm-book3s_64-objs-$(CONFIG_SPAPR_TCE_IOMMU) += book3s_64_vio.o
>>
>> kvm-book3s_64-module-objs := \
>
^ permalink raw reply
* Re: [PATCH v3] powerpc/pseries: Only wait for dying CPU after call to rtas_stop_self()
From: Gautham R Shenoy @ 2019-03-12 9:32 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Gautham R Shenoy, linux-kernel, Michael Bringmann, Tyrel Datwyler,
linuxppc-dev
In-Reply-To: <20190311193517.23756-1-bauerman@linux.ibm.com>
Hello Thiago,
On Mon, Mar 11, 2019 at 04:35:17PM -0300, Thiago Jung Bauermann wrote:
> When testing DLPAR CPU add/remove on a system under stress,
> pseries_cpu_die() doesn't wait long enough for a CPU to die:
>
> [ 446.983944] cpu 148 (hwid 148) Ready to die...
> [ 446.984062] cpu 149 (hwid 149) Ready to die...
> [ 446.993518] cpu 150 (hwid 150) Ready to die...
> [ 446.993543] Querying DEAD? cpu 150 (150) shows 2
> [ 446.994098] cpu 151 (hwid 151) Ready to die...
> [ 447.133726] cpu 136 (hwid 136) Ready to die...
> [ 447.403532] cpu 137 (hwid 137) Ready to die...
> [ 447.403772] cpu 138 (hwid 138) Ready to die...
> [ 447.403839] cpu 139 (hwid 139) Ready to die...
> [ 447.403887] cpu 140 (hwid 140) Ready to die...
> [ 447.403937] cpu 141 (hwid 141) Ready to die...
> [ 447.403979] cpu 142 (hwid 142) Ready to die...
> [ 447.404038] cpu 143 (hwid 143) Ready to die...
> [ 447.513546] cpu 128 (hwid 128) Ready to die...
> [ 447.693533] cpu 129 (hwid 129) Ready to die...
> [ 447.693999] cpu 130 (hwid 130) Ready to die...
> [ 447.703530] cpu 131 (hwid 131) Ready to die...
> [ 447.704087] Querying DEAD? cpu 132 (132) shows 2
> [ 447.704102] cpu 132 (hwid 132) Ready to die...
> [ 447.713534] cpu 133 (hwid 133) Ready to die...
> [ 447.714064] Querying DEAD? cpu 134 (134) shows 2
>
> This is a race between one CPU stopping and another one calling
> pseries_cpu_die() to wait for it to stop. That function does a short busy
> loop calling RTAS query-cpu-stopped-state on the stopping CPU to verify
> that it is stopped, but I think there's a lot for the stopping CPU to do
> which may take longer than this loop allows.
>
> As can be seen in the dmesg right before or after the "Querying DEAD?"
> messages, if pseries_cpu_die() waited a little longer it would have seen
> the CPU in the stopped state.
>
> What I think is going on is that CPU 134 was inactive at the time it was
> unplugged. In that case, dlpar_offline_cpu() calls H_PROD on that CPU and
> immediately calls pseries_cpu_die(). Meanwhile, the prodded CPU activates
> and start the process of stopping itself. The busy loop is not long enough
> to allow for the CPU to wake up and complete the stopping process.
>
> This can be a problem because if the busy loop finishes too early, then the
> kernel may offline another CPU before the previous one finished dying,
> which would lead to two concurrent calls to rtas-stop-self, which is
> prohibited by the PAPR.
>
> We can make the race a lot more even if we only start querying if the CPU
> is stopped when the stopping CPU is close to call rtas_stop_self(). Since
> pseries_mach_cpu_die() sets the CPU current state to offline almost
> immediately before calling rtas_stop_self(), we use that as a signal that
> it is either already stopped or very close to that point, and we can start
> the busy loop.
>
> As suggested by Michael Ellerman, this patch also changes the busy loop to
> wait for a fixed amount of wall time. Based on the measurements that
> Gautham did on a POWER9 system, in successful cases of
> smp_query_cpu_stopped(cpu) returning affirmative, the maximum time spent
> inside the loop was was 10 ms. This patch loops for 20 ms just be sure.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Thanks for this version. I have tested the patch and we no longer see
the "Querying DEAD? cpu X (Y) shows 2" message.
Tested-and-Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
--
Thanks and Regards
gautham.
^ permalink raw reply
* Re: [RFCv2 PATCH 4/4] powerpc: KASAN for 64bit Book3E
From: Christophe Leroy @ 2019-03-12 8:30 UTC (permalink / raw)
To: Daniel Axtens, aneesh.kumar, bsingharora
Cc: linuxppc-dev, Aneesh Kumar K . V, kasan-dev
In-Reply-To: <20190312012348.4854-5-dja@axtens.net>
Hi,
Build failure with pmac32_defconfig.
CC arch/powerpc/kernel/asm-offsets.s
In file included from ./arch/powerpc/include/asm/book3s/32/pgtable.h:149:0,
from ./arch/powerpc/include/asm/book3s/pgtable.h:8,
from ./arch/powerpc/include/asm/pgtable.h:18,
from ./arch/powerpc/include/asm/kasan.h:18,
from ./include/linux/kasan.h:14,
from ./include/linux/slab.h:129,
from ./include/linux/crypto.h:24,
from ./include/crypto/hash.h:16,
from ./include/linux/uio.h:14,
from ./include/linux/socket.h:8,
from ./include/linux/compat.h:15,
from arch/powerpc/kernel/asm-offsets.c:18:
./include/asm-generic/fixmap.h: In function ‘fix_to_virt’:
./arch/powerpc/include/asm/fixmap.h:27:22: error: ‘KASAN_SHADOW_START’
undeclared (first use in this function)
#define FIXADDR_TOP (KASAN_SHADOW_START - PAGE_SIZE)
^
./include/asm-generic/fixmap.h:21:27: note: in expansion of macro
‘FIXADDR_TOP’
#define __fix_to_virt(x) (FIXADDR_TOP - ((x) << PAGE_SHIFT))
^
./include/asm-generic/fixmap.h:33:9: note: in expansion of macro
‘__fix_to_virt’
return __fix_to_virt(idx);
^
./arch/powerpc/include/asm/fixmap.h:27:22: note: each undeclared
identifier is reported only once for each function it appears in
#define FIXADDR_TOP (KASAN_SHADOW_START - PAGE_SIZE)
^
./include/asm-generic/fixmap.h:21:27: note: in expansion of macro
‘FIXADDR_TOP’
#define __fix_to_virt(x) (FIXADDR_TOP - ((x) << PAGE_SHIFT))
^
./include/asm-generic/fixmap.h:33:9: note: in expansion of macro
‘__fix_to_virt’
return __fix_to_virt(idx);
^
In file included from ./include/linux/bug.h:5:0,
from ./include/linux/thread_info.h:12,
from ./include/asm-generic/preempt.h:5,
from ./arch/powerpc/include/generated/asm/preempt.h:1,
from ./include/linux/preempt.h:78,
from ./include/linux/spinlock.h:51,
from ./include/linux/seqlock.h:36,
from ./include/linux/time.h:6,
from ./include/linux/compat.h:10,
from arch/powerpc/kernel/asm-offsets.c:18:
./include/asm-generic/fixmap.h: In function ‘virt_to_fix’:
./arch/powerpc/include/asm/fixmap.h:27:22: error: ‘KASAN_SHADOW_START’
undeclared (first use in this function)
#define FIXADDR_TOP (KASAN_SHADOW_START - PAGE_SIZE)
^
./arch/powerpc/include/asm/bug.h:76:27: note: in definition of macro
‘BUG_ON’
if (__builtin_constant_p(x)) { \
^
./include/asm-generic/fixmap.h:38:18: note: in expansion of macro
‘FIXADDR_TOP’
BUG_ON(vaddr >= FIXADDR_TOP || vaddr < FIXADDR_START);
^
make[1]: *** [arch/powerpc/kernel/asm-offsets.s] Error 1
make: *** [prepare0] Error 2
Christophe
On 03/12/2019 01:23 AM, Daniel Axtens wrote:
> Wire up KASAN. Only outline instrumentation is supported.
>
> The KASAN shadow area is mapped into vmemmap space:
> 0x8000 0400 0000 0000 to 0x8000 0600 0000 0000.
> To do this we require that vmemmap be disabled. (This is the default
> in the kernel config that QorIQ provides for the machine in their
> SDK anyway - they use flat memory.)
>
> Only the kernel linear mapping (0xc000...) is checked. The vmalloc and
> ioremap areas (also in 0x800...) are all mapped to the zero page. As
> with the Book3S hash series, this requires overriding the memory <->
> shadow mapping.
>
> Also, as with both previous 64-bit series, early instrumentation is not
> supported. It would allow us to drop the check_return_arch_not_ready()
> hook in the KASAN core, but it's tricky to get it set up early enough:
> we need it setup before the first call to instrumented code like printk().
> Perhaps in the future.
>
> Only KASAN_MINIMAL works.
>
> Tested on e6500. KVM, kexec and xmon have not been tested.
>
> The test_kasan module fires warnings as expected, except for the
> following tests:
>
> - Expected/by design:
> kasan test: memcg_accounted_kmem_cache allocate memcg accounted object
>
> - Due to only supporting KASAN_MINIMAL:
> kasan test: kasan_stack_oob out-of-bounds on stack
> kasan test: kasan_global_oob out-of-bounds global variable
> kasan test: kasan_alloca_oob_left out-of-bounds to left on alloca
> kasan test: kasan_alloca_oob_right out-of-bounds to right on alloca
> kasan test: use_after_scope_test use-after-scope on int
> kasan test: use_after_scope_test use-after-scope on array
>
> Thanks to those who have done the heavy lifting over the past several years:
> - Christophe's 32 bit series: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-February/185379.html
> - Aneesh's Book3S hash series: https://lwn.net/Articles/655642/
> - Balbir's Book3S radix series: https://patchwork.ozlabs.org/patch/795211/
>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Cc: Balbir Singh <bsingharora@gmail.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
>
> ---
>
> While useful if you have a book3e device, this is mostly intended
> as a warm-up exercise for reviving Aneesh's series for book3s hash.
> In particular, changes to the kasan core are going to be required
> for hash and radix as well.
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/Kconfig.debug | 2 +-
> arch/powerpc/include/asm/kasan.h | 73 +++++++++++++++++++-
> arch/powerpc/mm/Makefile | 2 +
> arch/powerpc/mm/kasan/Makefile | 1 +
> arch/powerpc/mm/kasan/kasan_init_book3e_64.c | 53 ++++++++++++++
> 6 files changed, 129 insertions(+), 3 deletions(-)
> create mode 100644 arch/powerpc/mm/kasan/kasan_init_book3e_64.c
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 8d6108c83299..01540873a79f 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -173,6 +173,7 @@ config PPC
> select HAVE_ARCH_AUDITSYSCALL
> select HAVE_ARCH_JUMP_LABEL
> select HAVE_ARCH_KASAN if PPC32
> + select HAVE_ARCH_KASAN if PPC_BOOK3E_64 && !SPARSEMEM_VMEMMAP
> select HAVE_ARCH_KGDB
> select HAVE_ARCH_MMAP_RND_BITS
> select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> index 61febbbdd02b..fc1f5fa7554e 100644
> --- a/arch/powerpc/Kconfig.debug
> +++ b/arch/powerpc/Kconfig.debug
> @@ -369,5 +369,5 @@ config PPC_FAST_ENDIAN_SWITCH
>
> config KASAN_SHADOW_OFFSET
> hex
> - depends on KASAN
> + depends on KASAN && PPC32
> default 0xe0000000
> diff --git a/arch/powerpc/include/asm/kasan.h b/arch/powerpc/include/asm/kasan.h
> index e4adc6bc1e8f..661a5700869b 100644
> --- a/arch/powerpc/include/asm/kasan.h
> +++ b/arch/powerpc/include/asm/kasan.h
> @@ -15,14 +15,16 @@
> #ifndef __ASSEMBLY__
>
> #include <asm/page.h>
> +#include <asm/pgtable.h>
>
> #define KASAN_SHADOW_SCALE_SHIFT 3
>
> -#define KASAN_SHADOW_OFFSET ASM_CONST(CONFIG_KASAN_SHADOW_OFFSET)
> -
> #define KASAN_SHADOW_START (KASAN_SHADOW_OFFSET + \
> (PAGE_OFFSET >> KASAN_SHADOW_SCALE_SHIFT))
>
> +#ifdef CONFIG_PPC32
> +#define KASAN_SHADOW_OFFSET ASM_CONST(CONFIG_KASAN_SHADOW_OFFSET)
> +
> #define KASAN_SHADOW_END 0UL
>
> #define KASAN_SHADOW_SIZE (KASAN_SHADOW_END - KASAN_SHADOW_START)
> @@ -30,6 +32,73 @@
> #ifdef CONFIG_KASAN
> void kasan_early_init(void);
> void kasan_mmu_init(void);
> +#endif
> +#endif /* CONFIG_PPC32 */
> +
> +#ifdef CONFIG_PPC_BOOK3E_64
> +
> +/* we don't put this in Kconfig as we only support KASAN_MINIMAL, and
> + * that will be disabled if the symbol is availabe in Kconfig */
> +#define KASAN_SHADOW_OFFSET ASM_CONST(0x6800040000000000)
> +
> +#define KASAN_SHADOW_SIZE (KERN_VIRT_SIZE >> KASAN_SHADOW_SCALE_SHIFT)
> +
> +extern struct static_key_false powerpc_kasan_enabled_key;
> +static inline bool kasan_arch_is_ready_book3e(void) {
> + if (static_branch_likely(&powerpc_kasan_enabled_key))
> + return true;
> + return false;
> +}
> +#define kasan_arch_is_ready kasan_arch_is_ready_book3e
> +
> +extern unsigned char kasan_zero_page[PAGE_SIZE];
> +static inline void *kasan_mem_to_shadow_book3e(const void *addr)
> +{
> + if ((unsigned long)addr >= KERN_VIRT_START &&
> + (unsigned long)addr < (KERN_VIRT_START + KERN_VIRT_SIZE)) {
> + return (void *)kasan_zero_page;
> + }
> +
> + return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT)
> + + KASAN_SHADOW_OFFSET;
> +}
> +#define kasan_mem_to_shadow kasan_mem_to_shadow_book3e
> +
> +static inline void *kasan_shadow_to_mem_book3e(const void *shadow_addr)
> +{
> + /*
> + * We map the entire non-linear virtual mapping onto the zero page so if
> + * we are asked to map the zero page back just pick the beginning of that
> + * area.
> + */
> + if (shadow_addr >= (void *)kasan_zero_page &&
> + shadow_addr < (void *)(kasan_zero_page + PAGE_SIZE)) {
> + return (void *)KERN_VIRT_START;
> + }
> +
> + return (void *)(((unsigned long)shadow_addr - KASAN_SHADOW_OFFSET)
> + << KASAN_SHADOW_SCALE_SHIFT);
> +}
> +#define kasan_shadow_to_mem kasan_shadow_to_mem_book3e
> +
> +static inline bool kasan_addr_has_shadow_book3e(const void *addr)
> +{
> + /*
> + * We want to specifically assert that the addresses in the 0x8000...
> + * region have a shadow, otherwise they are considered by the kasan
> + * core to be wild pointers
> + */
> + if ((unsigned long)addr >= KERN_VIRT_START &&
> + (unsigned long)addr < (KERN_VIRT_START + KERN_VIRT_SIZE)) {
> + return true;
> + }
> + return (addr >= kasan_shadow_to_mem((void *)KASAN_SHADOW_START));
> +}
> +#define kasan_addr_has_shadow kasan_addr_has_shadow_book3e
> +
> +#endif /* CONFIG_PPC_BOOK3E_64 */
> +
> +#ifdef CONFIG_KASAN
> void kasan_init(void);
> #else
> static inline void kasan_init(void) { }
> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> index 80382a2d169b..fc49231f807c 100644
> --- a/arch/powerpc/mm/Makefile
> +++ b/arch/powerpc/mm/Makefile
> @@ -8,9 +8,11 @@ ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
> CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
>
> KASAN_SANITIZE_ppc_mmu_32.o := n
> +KASAN_SANITIZE_fsl_booke_mmu.o := n
>
> ifdef CONFIG_KASAN
> CFLAGS_ppc_mmu_32.o += -DDISABLE_BRANCH_PROFILING
> +CFLAGS_fsl_booke_mmu.o += -DDISABLE_BRANCH_PROFILING
> endif
>
> obj-y := fault.o mem.o pgtable.o mmap.o \
> diff --git a/arch/powerpc/mm/kasan/Makefile b/arch/powerpc/mm/kasan/Makefile
> index 6577897673dd..f8f164ad8ade 100644
> --- a/arch/powerpc/mm/kasan/Makefile
> +++ b/arch/powerpc/mm/kasan/Makefile
> @@ -3,3 +3,4 @@
> KASAN_SANITIZE := n
>
> obj-$(CONFIG_PPC32) += kasan_init_32.o
> +obj-$(CONFIG_PPC_BOOK3E_64) += kasan_init_book3e_64.o
> diff --git a/arch/powerpc/mm/kasan/kasan_init_book3e_64.c b/arch/powerpc/mm/kasan/kasan_init_book3e_64.c
> new file mode 100644
> index 000000000000..93b9afcf1020
> --- /dev/null
> +++ b/arch/powerpc/mm/kasan/kasan_init_book3e_64.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define DISABLE_BRANCH_PROFILING
> +
> +#include <linux/kasan.h>
> +#include <linux/printk.h>
> +#include <linux/memblock.h>
> +#include <linux/sched/task.h>
> +#include <asm/pgalloc.h>
> +
> +DEFINE_STATIC_KEY_FALSE(powerpc_kasan_enabled_key);
> +EXPORT_SYMBOL(powerpc_kasan_enabled_key);
> +unsigned char kasan_zero_page[PAGE_SIZE] __page_aligned_bss;
> +
> +static void __init kasan_init_region(struct memblock_region *reg)
> +{
> + void *start = __va(reg->base);
> + void *end = __va(reg->base + reg->size);
> + unsigned long k_start, k_end, k_cur;
> +
> + if (start >= end)
> + return;
> +
> + k_start = (unsigned long)kasan_mem_to_shadow(start);
> + k_end = (unsigned long)kasan_mem_to_shadow(end);
> +
> + for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) {
> + void *va = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> + map_kernel_page(k_cur, __pa(va), PAGE_KERNEL);
> + }
> + flush_tlb_kernel_range(k_start, k_end);
> +}
> +
> +void __init kasan_init(void)
> +{
> + struct memblock_region *reg;
> +
> + for_each_memblock(memory, reg)
> + kasan_init_region(reg);
> +
> + /* map the zero page RO */
> + map_kernel_page((unsigned long)kasan_zero_page,
> + __pa(kasan_zero_page), PAGE_KERNEL_RO);
> +
> + kasan_init_tags();
> +
> + /* Turn on checking */
> + static_branch_inc(&powerpc_kasan_enabled_key);
> +
> + /* Enable error messages */
> + init_task.kasan_depth = 0;
> + pr_info("KASAN init done (64-bit Book3E)\n");
> +}
>
^ permalink raw reply
* [PATCH kernel] powerpc/powernv: Fix compile without CONFIG_TRACEPOINTS
From: Alexey Kardashevskiy @ 2019-03-12 5:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Nicholas Piggin
The functions returns s64 but the return statement is missing.
This adds the missing return statement.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Just in case if this has not been caught just yet :)
---
arch/powerpc/platforms/powernv/opal-call.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 578757d403ab..daad8c45c8e7 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -86,6 +86,7 @@ static s64 __opal_call_trace(s64 a0, s64 a1, s64 a2, s64 a3,
s64 a4, s64 a5, s64 a6, s64 a7,
unsigned long opcode, unsigned long msr)
{
+ return 0;
}
#define DO_TRACE false
--
2.17.1
^ permalink raw reply related
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