| Message ID | 20190719111451.12406-1-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PATCH-for-4.1] target/arm: Add missing break statement for Hypervisor Trap Exception | expand |
On Fri, 19 Jul 2019 at 12:15, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > Reported by GCC9 when building with -Wimplicit-fallthrough=2: > > target/arm/helper.c: In function ‘arm_cpu_do_interrupt_aarch32_hyp’: > target/arm/helper.c:7958:14: error: this statement may fall through [-Werror=implicit-fallthrough=] > 7958 | addr = 0x14; > | ~~~~~^~~~~~ > target/arm/helper.c:7959:5: note: here > 7959 | default: > | ^~~~~~~ > cc1: all warnings being treated as errors > > Fixes: b9bc21ff9f9 > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > target/arm/helper.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 20f8728be1..b74c23a9bc 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -7956,6 +7956,7 @@ static void arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs) > break; > case EXCP_HYP_TRAP: > addr = 0x14; > + break; > default: > cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); > } I think this is right, but EXCP_HYP_TRAP is a bit odd -- we appear to use this only for the case of "SMC instruction is trapped from NS EL1 to EL2 by HCR.TSC". I was expecting more traps-to-EL2 to use this EXCP_ variable... Mostly we seem to use EXCP_UDEF, eg for CP_ACCESS_TRAP_UNCATEGORIZED_EL2 coprocessor/sysreg accesses: this has the same behaviour as EXCP_HYP_TRAP as long as we know we are going from an EL below 2 to EL2. Which I think we could also use in the one place we use EXCP_HYP_TRAP; or we could make wider use of EXCP_HYP_TRAP, since feeding everything through EXCP_UDEF is rather confusing. Anyway, for 4.1 we should do this. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 7/19/19 1:47 PM, Peter Maydell wrote: > On Fri, 19 Jul 2019 at 12:15, Philippe Mathieu-Daudé <philmd@redhat.com> wrote: >> >> Reported by GCC9 when building with -Wimplicit-fallthrough=2: >> >> target/arm/helper.c: In function ‘arm_cpu_do_interrupt_aarch32_hyp’: >> target/arm/helper.c:7958:14: error: this statement may fall through [-Werror=implicit-fallthrough=] >> 7958 | addr = 0x14; >> | ~~~~~^~~~~~ >> target/arm/helper.c:7959:5: note: here >> 7959 | default: >> | ^~~~~~~ >> cc1: all warnings being treated as errors >> >> Fixes: b9bc21ff9f9 >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> target/arm/helper.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/target/arm/helper.c b/target/arm/helper.c >> index 20f8728be1..b74c23a9bc 100644 >> --- a/target/arm/helper.c >> +++ b/target/arm/helper.c >> @@ -7956,6 +7956,7 @@ static void arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs) >> break; >> case EXCP_HYP_TRAP: >> addr = 0x14; >> + break; >> default: >> cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); >> } > > I think this is right, but EXCP_HYP_TRAP is a bit odd -- we appear > to use this only for the case of "SMC instruction is trapped from > NS EL1 to EL2 by HCR.TSC". I was expecting more traps-to-EL2 > to use this EXCP_ variable... Mostly we seem to use EXCP_UDEF, > eg for CP_ACCESS_TRAP_UNCATEGORIZED_EL2 coprocessor/sysreg accesses: > this has the same behaviour as EXCP_HYP_TRAP as long as we know > we are going from an EL below 2 to EL2. Which I think we could > also use in the one place we use EXCP_HYP_TRAP; or we could make > wider use of EXCP_HYP_TRAP, since feeding everything through > EXCP_UDEF is rather confusing. > > Anyway, for 4.1 we should do this. > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Thanks, if you take this I forgot to mention: Reported-by: Stefan Weil <sw@weilnetz.de> Regards, Phil.
diff --git a/target/arm/helper.c b/target/arm/helper.c index 20f8728be1..b74c23a9bc 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -7956,6 +7956,7 @@ static void arm_cpu_do_interrupt_aarch32_hyp(CPUState *cs) break; case EXCP_HYP_TRAP: addr = 0x14; + break; default: cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); }
Reported by GCC9 when building with -Wimplicit-fallthrough=2: target/arm/helper.c: In function ‘arm_cpu_do_interrupt_aarch32_hyp’: target/arm/helper.c:7958:14: error: this statement may fall through [-Werror=implicit-fallthrough=] 7958 | addr = 0x14; | ~~~~~^~~~~~ target/arm/helper.c:7959:5: note: here 7959 | default: | ^~~~~~~ cc1: all warnings being treated as errors Fixes: b9bc21ff9f9 Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- target/arm/helper.c | 1 + 1 file changed, 1 insertion(+)