2022-08-14 14:15:03

by Conor Dooley

[permalink] [raw]
Subject: [PATCH 0/4] misc warning cleanup in arch/risc-v

From: Conor Dooley <[email protected]>

Hey all,
Couple fixes here for most of what's left of the {sparse,} warnings in
arch/riscv that are still in need of patches. Ben has sent patches
for the VDSO issue already (although they seem to need rework).

VDSO aside, With this patchset applied, we are left with:
- cpuinfo_ops missing prototype: this likely needs to go into an
asm-generic header & I'll send a separate patch for that.
- Complaints about an error in mm/init.c:
"error inarch/riscv/mm/init.c:819:2: error: "setup_vm() is <trunc>
I think this can be ignored.
- 600+ -Woverride-init warnings for syscall table setup where
overriding seems to be the whole point of the macro.
- Warnings about imported kvm core code.
- Flexible array member warnings that look like common KVM code
patterns
- An unexpected unlock in kvm_riscv_check_vcpu_requests that was added
intentionally:
https://lore.kernel.org/all/[email protected]/
Is it worth looking into whether that's a false positive or not?

Thanks,
Conor.

Conor Dooley (4):
riscv: kvm: vcpu_timer: fix unused variable warnings
riscv: kvm: move extern sbi_ext declarations to a header
riscv: signal: fix missing prototype warning
riscv: traps: add missing prototype

arch/riscv/include/asm/kvm_vcpu_sbi.h | 12 ++++++++++++
arch/riscv/include/asm/signal.h | 12 ++++++++++++
arch/riscv/include/asm/thread_info.h | 2 ++
arch/riscv/kernel/signal.c | 1 +
arch/riscv/kernel/traps.c | 3 ++-
arch/riscv/kvm/vcpu_sbi.c | 12 +-----------
arch/riscv/kvm/vcpu_timer.c | 4 ----
7 files changed, 30 insertions(+), 16 deletions(-)
create mode 100644 arch/riscv/include/asm/signal.h

--
2.37.1


2022-08-14 14:15:24

by Conor Dooley

[permalink] [raw]
Subject: [PATCH 3/4] riscv: signal: fix missing prototype warning

From: Conor Dooley <[email protected]>

Fix the warning:
arch/riscv/kernel/signal.c:316:27: warning: no previous prototype for function 'do_notify_resume' [-Wmissing-prototypes]
asmlinkage __visible void do_notify_resume(struct pt_regs *regs,

All other functions in the file are static & none of the existing
headers stood out as an obvious location. Create signal.h to hold the
declaration.

Fixes: e2c0cdfba7f6 ("RISC-V: User-facing API")
Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/include/asm/signal.h | 12 ++++++++++++
arch/riscv/kernel/signal.c | 1 +
2 files changed, 13 insertions(+)
create mode 100644 arch/riscv/include/asm/signal.h

diff --git a/arch/riscv/include/asm/signal.h b/arch/riscv/include/asm/signal.h
new file mode 100644
index 000000000000..532c29ef0376
--- /dev/null
+++ b/arch/riscv/include/asm/signal.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ASM_SIGNAL_H
+#define __ASM_SIGNAL_H
+
+#include <uapi/asm/signal.h>
+#include <uapi/asm/ptrace.h>
+
+asmlinkage __visible
+void do_notify_resume(struct pt_regs *regs, unsigned long thread_info_flags);
+
+#endif
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 38b05ca6fe66..5a2de6b6f882 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -15,6 +15,7 @@

#include <asm/ucontext.h>
#include <asm/vdso.h>
+#include <asm/signal.h>
#include <asm/signal32.h>
#include <asm/switch_to.h>
#include <asm/csr.h>
--
2.37.1

2022-08-14 14:16:27

by Conor Dooley

[permalink] [raw]
Subject: [PATCH 4/4] riscv: traps: add missing prototype

From: Conor Dooley <[email protected]>

Sparse complains:
arch/riscv/kernel/traps.c:213:6: warning: symbol 'shadow_stack' was not declared. Should it be static?

The variable is used in entry.S, so declare shadow_stack there
alongside SHADOW_OVERFLOW_STACK_SIZE.

Fixes: 31da94c25aea ("riscv: add VMAP_STACK overflow detection")
Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/include/asm/thread_info.h | 2 ++
arch/riscv/kernel/traps.c | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index 78933ac04995..67322f878e0d 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -42,6 +42,8 @@

#ifndef __ASSEMBLY__

+extern long shadow_stack[SHADOW_OVERFLOW_STACK_SIZE / sizeof(long)];
+
#include <asm/processor.h>
#include <asm/csr.h>

diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 39d0f8bba4b4..635e6ec26938 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -20,9 +20,10 @@

#include <asm/asm-prototypes.h>
#include <asm/bug.h>
+#include <asm/csr.h>
#include <asm/processor.h>
#include <asm/ptrace.h>
-#include <asm/csr.h>
+#include <asm/thread_info.h>

int show_unhandled_signals = 1;

--
2.37.1

2022-08-14 14:27:37

by Conor Dooley

[permalink] [raw]
Subject: [PATCH 1/4] riscv: kvm: vcpu_timer: fix unused variable warnings

From: Conor Dooley <[email protected]>

In two places, csr is set but never used:

arch/riscv/kvm/vcpu_timer.c:302:23: warning: variable 'csr' set but not used [-Wunused-but-set-variable]
struct kvm_vcpu_csr *csr;
^
arch/riscv/kvm/vcpu_timer.c:327:23: warning: variable 'csr' set but not used [-Wunused-but-set-variable]
struct kvm_vcpu_csr *csr;
^

Remove the variable.

Fixes: 8f5cb44b1bae ("RISC-V: KVM: Support sstc extension")
Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/kvm/vcpu_timer.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/arch/riscv/kvm/vcpu_timer.c b/arch/riscv/kvm/vcpu_timer.c
index 16f50c46ba39..185f2386a747 100644
--- a/arch/riscv/kvm/vcpu_timer.c
+++ b/arch/riscv/kvm/vcpu_timer.c
@@ -299,7 +299,6 @@ static void kvm_riscv_vcpu_update_timedelta(struct kvm_vcpu *vcpu)

void kvm_riscv_vcpu_timer_restore(struct kvm_vcpu *vcpu)
{
- struct kvm_vcpu_csr *csr;
struct kvm_vcpu_timer *t = &vcpu->arch.timer;

kvm_riscv_vcpu_update_timedelta(vcpu);
@@ -307,7 +306,6 @@ void kvm_riscv_vcpu_timer_restore(struct kvm_vcpu *vcpu)
if (!t->sstc_enabled)
return;

- csr = &vcpu->arch.guest_csr;
#if defined(CONFIG_32BIT)
csr_write(CSR_VSTIMECMP, (u32)t->next_cycles);
csr_write(CSR_VSTIMECMPH, (u32)(t->next_cycles >> 32));
@@ -324,13 +322,11 @@ void kvm_riscv_vcpu_timer_restore(struct kvm_vcpu *vcpu)

void kvm_riscv_vcpu_timer_save(struct kvm_vcpu *vcpu)
{
- struct kvm_vcpu_csr *csr;
struct kvm_vcpu_timer *t = &vcpu->arch.timer;

if (!t->sstc_enabled)
return;

- csr = &vcpu->arch.guest_csr;
t = &vcpu->arch.timer;
#if defined(CONFIG_32BIT)
t->next_cycles = csr_read(CSR_VSTIMECMP);
--
2.37.1

2022-08-14 14:45:39

by Conor Dooley

[permalink] [raw]
Subject: [PATCH 2/4] riscv: kvm: move extern sbi_ext declarations to a header

From: Conor Dooley <[email protected]>

Sparse complains about missing statics in the declarations of several
variables:
arch/riscv/kvm/vcpu_sbi_replace.c:38:37: warning: symbol 'vcpu_sbi_ext_time' was not declared. Should it be static?
arch/riscv/kvm/vcpu_sbi_replace.c:73:37: warning: symbol 'vcpu_sbi_ext_ipi' was not declared. Should it be static?
arch/riscv/kvm/vcpu_sbi_replace.c:126:37: warning: symbol 'vcpu_sbi_ext_rfence' was not declared. Should it be static?
arch/riscv/kvm/vcpu_sbi_replace.c:170:37: warning: symbol 'vcpu_sbi_ext_srst' was not declared. Should it be static?
arch/riscv/kvm/vcpu_sbi_base.c:69:37: warning: symbol 'vcpu_sbi_ext_base' was not declared. Should it be static?
arch/riscv/kvm/vcpu_sbi_base.c:90:37: warning: symbol 'vcpu_sbi_ext_experimental' was not declared. Should it be static?
arch/riscv/kvm/vcpu_sbi_base.c:96:37: warning: symbol 'vcpu_sbi_ext_vendor' was not declared. Should it be static?
arch/riscv/kvm/vcpu_sbi_hsm.c:115:37: warning: symbol 'vcpu_sbi_ext_hsm' was not declared. Should it be static?

These variables are however used in vcpu_sbi.c where they are declared
as extern. Move them to kvm_vcpu_sbi.h which is handily already
included by the three other files.

Fixes: a046c2d8578c ("RISC-V: KVM: Reorganize SBI code by moving SBI v0.1 to its own file")
Fixes: 5f862df5585c ("RISC-V: KVM: Add v0.1 replacement SBI extensions defined in v0.2")
Fixes: 3e1d86569c21 ("RISC-V: KVM: Add SBI HSM extension in KVM")
Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/include/asm/kvm_vcpu_sbi.h | 12 ++++++++++++
arch/riscv/kvm/vcpu_sbi.c | 12 +-----------
2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
index 83d6d4d2b1df..26a446a34057 100644
--- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
+++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
@@ -33,4 +33,16 @@ void kvm_riscv_vcpu_sbi_system_reset(struct kvm_vcpu *vcpu,
u32 type, u64 flags);
const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(unsigned long extid);

+#ifdef CONFIG_RISCV_SBI_V01
+extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01;
+#endif
+extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_base;
+extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_time;
+extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_ipi;
+extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_rfence;
+extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst;
+extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm;
+extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental;
+extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor;
+
#endif /* __RISCV_KVM_VCPU_SBI_H__ */
diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
index d45e7da3f0d3..f96991d230bf 100644
--- a/arch/riscv/kvm/vcpu_sbi.c
+++ b/arch/riscv/kvm/vcpu_sbi.c
@@ -32,23 +32,13 @@ static int kvm_linux_err_map_sbi(int err)
};
}

-#ifdef CONFIG_RISCV_SBI_V01
-extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01;
-#else
+#ifndef CONFIG_RISCV_SBI_V01
static const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01 = {
.extid_start = -1UL,
.extid_end = -1UL,
.handler = NULL,
};
#endif
-extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_base;
-extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_time;
-extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_ipi;
-extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_rfence;
-extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst;
-extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm;
-extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental;
-extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor;

static const struct kvm_vcpu_sbi_extension *sbi_ext[] = {
&vcpu_sbi_ext_v01,
--
2.37.1

2022-08-18 23:04:50

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 0/4] misc warning cleanup in arch/risc-v

On Sun, 14 Aug 2022 07:12:34 PDT (-0700), [email protected] wrote:
> From: Conor Dooley <[email protected]>
>
> Hey all,
> Couple fixes here for most of what's left of the {sparse,} warnings in
> arch/riscv that are still in need of patches. Ben has sent patches
> for the VDSO issue already (although they seem to need rework).
>
> VDSO aside, With this patchset applied, we are left with:
> - cpuinfo_ops missing prototype: this likely needs to go into an
> asm-generic header & I'll send a separate patch for that.
> - Complaints about an error in mm/init.c:
> "error inarch/riscv/mm/init.c:819:2: error: "setup_vm() is <trunc>
> I think this can be ignored.
> - 600+ -Woverride-init warnings for syscall table setup where
> overriding seems to be the whole point of the macro.
> - Warnings about imported kvm core code.
> - Flexible array member warnings that look like common KVM code
> patterns
> - An unexpected unlock in kvm_riscv_check_vcpu_requests that was added
> intentionally:
> https://lore.kernel.org/all/[email protected]/
> Is it worth looking into whether that's a false positive or not?
>
> Thanks,
> Conor.
>
> Conor Dooley (4):
> riscv: kvm: vcpu_timer: fix unused variable warnings
> riscv: kvm: move extern sbi_ext declarations to a header
> riscv: signal: fix missing prototype warning
> riscv: traps: add missing prototype
>
> arch/riscv/include/asm/kvm_vcpu_sbi.h | 12 ++++++++++++
> arch/riscv/include/asm/signal.h | 12 ++++++++++++
> arch/riscv/include/asm/thread_info.h | 2 ++
> arch/riscv/kernel/signal.c | 1 +
> arch/riscv/kernel/traps.c | 3 ++-
> arch/riscv/kvm/vcpu_sbi.c | 12 +-----------
> arch/riscv/kvm/vcpu_timer.c | 4 ----
> 7 files changed, 30 insertions(+), 16 deletions(-)
> create mode 100644 arch/riscv/include/asm/signal.h

These generally look good to me. Anup handles the KVM bits so I'll let
him chime in there, but

Reviewed-by: Palmer Dabbelt <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>

on all of them.

Happy to do some sort of shared tag thing, but it looks like these are
all independent enough that it'd be easier to just split them up. I've
put the non-KVM bits over at palmer/riscv-variable_fixes_without_kvm, if
you guys are all OK splitting this up then I'll go take those onto
riscv/fixes. I'll wait a bit for folks to get a chance to look, so it
won't be for tomorrow morning.

Thanks!

2022-08-19 12:03:19

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH 0/4] misc warning cleanup in arch/risc-v

On 19/08/2022 00:01, Palmer Dabbelt wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Sun, 14 Aug 2022 07:12:34 PDT (-0700), [email protected] wrote:
>> From: Conor Dooley <[email protected]>
>>
>> Hey all,
>> Couple fixes here for most of what's left of the {sparse,} warnings in
>> arch/riscv that are still in need of patches. Ben has sent patches
>> for the VDSO issue already (although they seem to need rework).
>>
>> VDSO aside, With this patchset applied, we are left with:
>> - cpuinfo_ops missing prototype: this likely needs to go into an
>>   asm-generic header & I'll send a separate patch for that.
>> - Complaints about an error in mm/init.c:
>>   "error inarch/riscv/mm/init.c:819:2: error: "setup_vm() is <trunc>
>>   I think this can be ignored.
>> - 600+ -Woverride-init warnings for syscall table setup where
>>   overriding seems to be the whole point of the macro.
>> - Warnings about imported kvm core code.
>> - Flexible array member warnings that look like common KVM code
>>   patterns
>> - An unexpected unlock in kvm_riscv_check_vcpu_requests that was added
>>   intentionally:
>>   https://lore.kernel.org/all/[email protected]/
>>   Is it worth looking into whether that's a false positive or not?
>>
>> Thanks,
>> Conor.
>>
>> Conor Dooley (4):
>>   riscv: kvm: vcpu_timer: fix unused variable warnings
>>   riscv: kvm: move extern sbi_ext declarations to a header
>>   riscv: signal: fix missing prototype warning
>>   riscv: traps: add missing prototype
>>
>>  arch/riscv/include/asm/kvm_vcpu_sbi.h | 12 ++++++++++++
>>  arch/riscv/include/asm/signal.h       | 12 ++++++++++++
>>  arch/riscv/include/asm/thread_info.h  |  2 ++
>>  arch/riscv/kernel/signal.c            |  1 +
>>  arch/riscv/kernel/traps.c             |  3 ++-
>>  arch/riscv/kvm/vcpu_sbi.c             | 12 +-----------
>>  arch/riscv/kvm/vcpu_timer.c           |  4 ----
>>  7 files changed, 30 insertions(+), 16 deletions(-)
>>  create mode 100644 arch/riscv/include/asm/signal.h
>
> These generally look good to me.  Anup handles the KVM bits so I'll let
> him chime in there, but
>
> Reviewed-by: Palmer Dabbelt <[email protected]>
> Acked-by: Palmer Dabbelt <[email protected]>
>
> on all of them.
>
> Happy to do some sort of shared tag thing, but it looks like these are
> all independent enough that it'd be easier to just split them up.  I've
> put the non-KVM bits over at palmer/riscv-variable_fixes_without_kvm, if
> you guys are all OK splitting this up then I'll go take those onto
> riscv/fixes.

Yeah, I see no reason that the kvm patches can't just go via the riscv
kvm tree. I sent the patches as a single series mostly because I wanted
to mention what was left warning wise in the cover.

> I'll wait a bit for folks to get a chance to look, so it
> won't be for tomorrow morning.

They've been there a while and nothing has gone up in flames, I am sure
we will survive a few weeks more :)

Thanks,
Conor.


2022-08-19 17:25:53

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH 1/4] riscv: kvm: vcpu_timer: fix unused variable warnings

On Sun, Aug 14, 2022 at 7:42 PM Conor Dooley <[email protected]> wrote:
>
> From: Conor Dooley <[email protected]>
>
> In two places, csr is set but never used:
>
> arch/riscv/kvm/vcpu_timer.c:302:23: warning: variable 'csr' set but not used [-Wunused-but-set-variable]
> struct kvm_vcpu_csr *csr;
> ^
> arch/riscv/kvm/vcpu_timer.c:327:23: warning: variable 'csr' set but not used [-Wunused-but-set-variable]
> struct kvm_vcpu_csr *csr;
> ^
>
> Remove the variable.
>
> Fixes: 8f5cb44b1bae ("RISC-V: KVM: Support sstc extension")
> Signed-off-by: Conor Dooley <[email protected]>

I have queued this fix for 6.0-rc1

Thanks,
Anup

> ---
> arch/riscv/kvm/vcpu_timer.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/arch/riscv/kvm/vcpu_timer.c b/arch/riscv/kvm/vcpu_timer.c
> index 16f50c46ba39..185f2386a747 100644
> --- a/arch/riscv/kvm/vcpu_timer.c
> +++ b/arch/riscv/kvm/vcpu_timer.c
> @@ -299,7 +299,6 @@ static void kvm_riscv_vcpu_update_timedelta(struct kvm_vcpu *vcpu)
>
> void kvm_riscv_vcpu_timer_restore(struct kvm_vcpu *vcpu)
> {
> - struct kvm_vcpu_csr *csr;
> struct kvm_vcpu_timer *t = &vcpu->arch.timer;
>
> kvm_riscv_vcpu_update_timedelta(vcpu);
> @@ -307,7 +306,6 @@ void kvm_riscv_vcpu_timer_restore(struct kvm_vcpu *vcpu)
> if (!t->sstc_enabled)
> return;
>
> - csr = &vcpu->arch.guest_csr;
> #if defined(CONFIG_32BIT)
> csr_write(CSR_VSTIMECMP, (u32)t->next_cycles);
> csr_write(CSR_VSTIMECMPH, (u32)(t->next_cycles >> 32));
> @@ -324,13 +322,11 @@ void kvm_riscv_vcpu_timer_restore(struct kvm_vcpu *vcpu)
>
> void kvm_riscv_vcpu_timer_save(struct kvm_vcpu *vcpu)
> {
> - struct kvm_vcpu_csr *csr;
> struct kvm_vcpu_timer *t = &vcpu->arch.timer;
>
> if (!t->sstc_enabled)
> return;
>
> - csr = &vcpu->arch.guest_csr;
> t = &vcpu->arch.timer;
> #if defined(CONFIG_32BIT)
> t->next_cycles = csr_read(CSR_VSTIMECMP);
> --
> 2.37.1
>

2022-08-19 17:26:30

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH 2/4] riscv: kvm: move extern sbi_ext declarations to a header

On Sun, Aug 14, 2022 at 7:42 PM Conor Dooley <[email protected]> wrote:
>
> From: Conor Dooley <[email protected]>
>
> Sparse complains about missing statics in the declarations of several
> variables:
> arch/riscv/kvm/vcpu_sbi_replace.c:38:37: warning: symbol 'vcpu_sbi_ext_time' was not declared. Should it be static?
> arch/riscv/kvm/vcpu_sbi_replace.c:73:37: warning: symbol 'vcpu_sbi_ext_ipi' was not declared. Should it be static?
> arch/riscv/kvm/vcpu_sbi_replace.c:126:37: warning: symbol 'vcpu_sbi_ext_rfence' was not declared. Should it be static?
> arch/riscv/kvm/vcpu_sbi_replace.c:170:37: warning: symbol 'vcpu_sbi_ext_srst' was not declared. Should it be static?
> arch/riscv/kvm/vcpu_sbi_base.c:69:37: warning: symbol 'vcpu_sbi_ext_base' was not declared. Should it be static?
> arch/riscv/kvm/vcpu_sbi_base.c:90:37: warning: symbol 'vcpu_sbi_ext_experimental' was not declared. Should it be static?
> arch/riscv/kvm/vcpu_sbi_base.c:96:37: warning: symbol 'vcpu_sbi_ext_vendor' was not declared. Should it be static?
> arch/riscv/kvm/vcpu_sbi_hsm.c:115:37: warning: symbol 'vcpu_sbi_ext_hsm' was not declared. Should it be static?
>
> These variables are however used in vcpu_sbi.c where they are declared
> as extern. Move them to kvm_vcpu_sbi.h which is handily already
> included by the three other files.
>
> Fixes: a046c2d8578c ("RISC-V: KVM: Reorganize SBI code by moving SBI v0.1 to its own file")
> Fixes: 5f862df5585c ("RISC-V: KVM: Add v0.1 replacement SBI extensions defined in v0.2")
> Fixes: 3e1d86569c21 ("RISC-V: KVM: Add SBI HSM extension in KVM")
> Signed-off-by: Conor Dooley <[email protected]>

I have queued this fix for 6.0-rc1

Thanks,
Anup

> ---
> arch/riscv/include/asm/kvm_vcpu_sbi.h | 12 ++++++++++++
> arch/riscv/kvm/vcpu_sbi.c | 12 +-----------
> 2 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> index 83d6d4d2b1df..26a446a34057 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> @@ -33,4 +33,16 @@ void kvm_riscv_vcpu_sbi_system_reset(struct kvm_vcpu *vcpu,
> u32 type, u64 flags);
> const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(unsigned long extid);
>
> +#ifdef CONFIG_RISCV_SBI_V01
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01;
> +#endif
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_base;
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_time;
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_ipi;
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_rfence;
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst;
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm;
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental;
> +extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor;
> +
> #endif /* __RISCV_KVM_VCPU_SBI_H__ */
> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> index d45e7da3f0d3..f96991d230bf 100644
> --- a/arch/riscv/kvm/vcpu_sbi.c
> +++ b/arch/riscv/kvm/vcpu_sbi.c
> @@ -32,23 +32,13 @@ static int kvm_linux_err_map_sbi(int err)
> };
> }
>
> -#ifdef CONFIG_RISCV_SBI_V01
> -extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01;
> -#else
> +#ifndef CONFIG_RISCV_SBI_V01
> static const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01 = {
> .extid_start = -1UL,
> .extid_end = -1UL,
> .handler = NULL,
> };
> #endif
> -extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_base;
> -extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_time;
> -extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_ipi;
> -extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_rfence;
> -extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_srst;
> -extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_hsm;
> -extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_experimental;
> -extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_vendor;
>
> static const struct kvm_vcpu_sbi_extension *sbi_ext[] = {
> &vcpu_sbi_ext_v01,
> --
> 2.37.1
>

2022-08-19 17:37:01

by Anup Patel

[permalink] [raw]
Subject: Re: [PATCH 0/4] misc warning cleanup in arch/risc-v

On Fri, Aug 19, 2022 at 4:32 AM Palmer Dabbelt <[email protected]> wrote:
>
> On Sun, 14 Aug 2022 07:12:34 PDT (-0700), [email protected] wrote:
> > From: Conor Dooley <[email protected]>
> >
> > Hey all,
> > Couple fixes here for most of what's left of the {sparse,} warnings in
> > arch/riscv that are still in need of patches. Ben has sent patches
> > for the VDSO issue already (although they seem to need rework).
> >
> > VDSO aside, With this patchset applied, we are left with:
> > - cpuinfo_ops missing prototype: this likely needs to go into an
> > asm-generic header & I'll send a separate patch for that.
> > - Complaints about an error in mm/init.c:
> > "error inarch/riscv/mm/init.c:819:2: error: "setup_vm() is <trunc>
> > I think this can be ignored.
> > - 600+ -Woverride-init warnings for syscall table setup where
> > overriding seems to be the whole point of the macro.
> > - Warnings about imported kvm core code.
> > - Flexible array member warnings that look like common KVM code
> > patterns
> > - An unexpected unlock in kvm_riscv_check_vcpu_requests that was added
> > intentionally:
> > https://lore.kernel.org/all/[email protected]/
> > Is it worth looking into whether that's a false positive or not?
> >
> > Thanks,
> > Conor.
> >
> > Conor Dooley (4):
> > riscv: kvm: vcpu_timer: fix unused variable warnings
> > riscv: kvm: move extern sbi_ext declarations to a header
> > riscv: signal: fix missing prototype warning
> > riscv: traps: add missing prototype
> >
> > arch/riscv/include/asm/kvm_vcpu_sbi.h | 12 ++++++++++++
> > arch/riscv/include/asm/signal.h | 12 ++++++++++++
> > arch/riscv/include/asm/thread_info.h | 2 ++
> > arch/riscv/kernel/signal.c | 1 +
> > arch/riscv/kernel/traps.c | 3 ++-
> > arch/riscv/kvm/vcpu_sbi.c | 12 +-----------
> > arch/riscv/kvm/vcpu_timer.c | 4 ----
> > 7 files changed, 30 insertions(+), 16 deletions(-)
> > create mode 100644 arch/riscv/include/asm/signal.h
>
> These generally look good to me. Anup handles the KVM bits so I'll let
> him chime in there, but
>
> Reviewed-by: Palmer Dabbelt <[email protected]>
> Acked-by: Palmer Dabbelt <[email protected]>
>
> on all of them.
>
> Happy to do some sort of shared tag thing, but it looks like these are
> all independent enough that it'd be easier to just split them up. I've
> put the non-KVM bits over at palmer/riscv-variable_fixes_without_kvm, if
> you guys are all OK splitting this up then I'll go take those onto
> riscv/fixes. I'll wait a bit for folks to get a chance to look, so it
> won't be for tomorrow morning.

Thanks Palmer. I have queued the KVM fixes (first two patches).

Regards,
Anup

>
> Thanks!
>
> --
> kvm-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kvm-riscv

2022-08-19 21:12:28

by Atish Patra

[permalink] [raw]
Subject: Re: [PATCH 1/4] riscv: kvm: vcpu_timer: fix unused variable warnings

On Sun, Aug 14, 2022 at 7:12 AM Conor Dooley <[email protected]> wrote:
>
> From: Conor Dooley <[email protected]>
>
> In two places, csr is set but never used:
>
> arch/riscv/kvm/vcpu_timer.c:302:23: warning: variable 'csr' set but not used [-Wunused-but-set-variable]
> struct kvm_vcpu_csr *csr;
> ^
> arch/riscv/kvm/vcpu_timer.c:327:23: warning: variable 'csr' set but not used [-Wunused-but-set-variable]
> struct kvm_vcpu_csr *csr;
> ^
>
> Remove the variable.
>
> Fixes: 8f5cb44b1bae ("RISC-V: KVM: Support sstc extension")
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> arch/riscv/kvm/vcpu_timer.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/arch/riscv/kvm/vcpu_timer.c b/arch/riscv/kvm/vcpu_timer.c
> index 16f50c46ba39..185f2386a747 100644
> --- a/arch/riscv/kvm/vcpu_timer.c
> +++ b/arch/riscv/kvm/vcpu_timer.c
> @@ -299,7 +299,6 @@ static void kvm_riscv_vcpu_update_timedelta(struct kvm_vcpu *vcpu)
>
> void kvm_riscv_vcpu_timer_restore(struct kvm_vcpu *vcpu)
> {
> - struct kvm_vcpu_csr *csr;
> struct kvm_vcpu_timer *t = &vcpu->arch.timer;
>
> kvm_riscv_vcpu_update_timedelta(vcpu);
> @@ -307,7 +306,6 @@ void kvm_riscv_vcpu_timer_restore(struct kvm_vcpu *vcpu)
> if (!t->sstc_enabled)
> return;
>
> - csr = &vcpu->arch.guest_csr;
> #if defined(CONFIG_32BIT)
> csr_write(CSR_VSTIMECMP, (u32)t->next_cycles);
> csr_write(CSR_VSTIMECMPH, (u32)(t->next_cycles >> 32));
> @@ -324,13 +322,11 @@ void kvm_riscv_vcpu_timer_restore(struct kvm_vcpu *vcpu)
>
> void kvm_riscv_vcpu_timer_save(struct kvm_vcpu *vcpu)
> {
> - struct kvm_vcpu_csr *csr;
> struct kvm_vcpu_timer *t = &vcpu->arch.timer;
>
> if (!t->sstc_enabled)
> return;
>
> - csr = &vcpu->arch.guest_csr;
> t = &vcpu->arch.timer;
> #if defined(CONFIG_32BIT)
> t->next_cycles = csr_read(CSR_VSTIMECMP);
> --
> 2.37.1
>

Thanks for fixing this. Sorry for missing those.


--
Regards,
Atish

2022-08-26 00:10:13

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 0/4] misc warning cleanup in arch/risc-v

On Fri, 19 Aug 2022 09:14:32 PDT (-0700), [email protected] wrote:
> On Fri, Aug 19, 2022 at 4:32 AM Palmer Dabbelt <[email protected]> wrote:
>>
>> On Sun, 14 Aug 2022 07:12:34 PDT (-0700), [email protected] wrote:
>> > From: Conor Dooley <[email protected]>
>> >
>> > Hey all,
>> > Couple fixes here for most of what's left of the {sparse,} warnings in
>> > arch/riscv that are still in need of patches. Ben has sent patches
>> > for the VDSO issue already (although they seem to need rework).
>> >
>> > VDSO aside, With this patchset applied, we are left with:
>> > - cpuinfo_ops missing prototype: this likely needs to go into an
>> > asm-generic header & I'll send a separate patch for that.
>> > - Complaints about an error in mm/init.c:
>> > "error inarch/riscv/mm/init.c:819:2: error: "setup_vm() is <trunc>
>> > I think this can be ignored.
>> > - 600+ -Woverride-init warnings for syscall table setup where
>> > overriding seems to be the whole point of the macro.
>> > - Warnings about imported kvm core code.
>> > - Flexible array member warnings that look like common KVM code
>> > patterns
>> > - An unexpected unlock in kvm_riscv_check_vcpu_requests that was added
>> > intentionally:
>> > https://lore.kernel.org/all/[email protected]/
>> > Is it worth looking into whether that's a false positive or not?
>> >
>> > Thanks,
>> > Conor.
>> >
>> > Conor Dooley (4):
>> > riscv: kvm: vcpu_timer: fix unused variable warnings
>> > riscv: kvm: move extern sbi_ext declarations to a header
>> > riscv: signal: fix missing prototype warning
>> > riscv: traps: add missing prototype
>> >
>> > arch/riscv/include/asm/kvm_vcpu_sbi.h | 12 ++++++++++++
>> > arch/riscv/include/asm/signal.h | 12 ++++++++++++
>> > arch/riscv/include/asm/thread_info.h | 2 ++
>> > arch/riscv/kernel/signal.c | 1 +
>> > arch/riscv/kernel/traps.c | 3 ++-
>> > arch/riscv/kvm/vcpu_sbi.c | 12 +-----------
>> > arch/riscv/kvm/vcpu_timer.c | 4 ----
>> > 7 files changed, 30 insertions(+), 16 deletions(-)
>> > create mode 100644 arch/riscv/include/asm/signal.h
>>
>> These generally look good to me. Anup handles the KVM bits so I'll let
>> him chime in there, but
>>
>> Reviewed-by: Palmer Dabbelt <[email protected]>
>> Acked-by: Palmer Dabbelt <[email protected]>
>>
>> on all of them.
>>
>> Happy to do some sort of shared tag thing, but it looks like these are
>> all independent enough that it'd be easier to just split them up. I've
>> put the non-KVM bits over at palmer/riscv-variable_fixes_without_kvm, if
>> you guys are all OK splitting this up then I'll go take those onto
>> riscv/fixes. I'll wait a bit for folks to get a chance to look, so it
>> won't be for tomorrow morning.
>
> Thanks Palmer. I have queued the KVM fixes (first two patches).

Sounds good. The others are in riscv/fixes.

>
> Regards,
> Anup
>
>>
>> Thanks!
>>
>> --
>> kvm-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/kvm-riscv