Adding ifdefs to call the respective routines only when their configs are enabled

Commit Message

Swetha Joshi May 24, 2021, 10:56 p.m. UTC

From: Swetha <[e-mail]>

Signed-off-by: Swetha <[e-mail]>
---
 target/arm/kvm64.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

Comments

[e-mail] May 25, 2021, 12:48 a.m. UTC | #1

Patchew URL: https://patchew.org/QEMU/[e-mail]/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: [e-mail]
Subject: [PATCH] Adding ifdefs to call the respective routines only when their configs are enabled

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/[e-mail] -> patchew/[e-mail]
Switched to a new branch 'test'
18cab39 Adding ifdefs to call the respective routines only when their configs are enabled

=== OUTPUT BEGIN ===
ERROR: suspect code indent for conditional statements (16, 18)
#38: FILE: target/arm/kvm64.c:1433:
+                if (acpi_ghes_record_errors(ACPI_HEST_SRC_ID_SEA, paddr)) {
+                  error_report("failed to record the error");

total: 1 errors, 0 warnings, 28 lines checked

Commit 18cab39bcd46 (Adding ifdefs to call the respective routines only when their configs are enabled) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/[e-mail]/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to [e-mail]

Richard Henderson May 25, 2021, 1:53 a.m. UTC | #2

On 5/24/21 3:56 PM, Swetha Joshi wrote:
> From: Swetha <[e-mail]>
> 
> Signed-off-by: Swetha <[e-mail]>

What are you trying to accomplish?
That's what belongs in the commit message that you omitted.

> +    bool acpi_enabled = false;
> +#ifdef CONFIG_ARM_VIRT
>       bool acpi_enabled = virt_is_acpi_enabled(vms);

Of course this doesn't compile, having declared acpi_enabled twice.  So you 
have clearly not tested this patch sufficiently.

The cc to qemu-trivial is unwarranted.


r~

Swetha Joshi May 25, 2021, 2:14 a.m. UTC | #3

Hey Richard,

I think I submitted the wrong patch, sorry about that. I will go ahead and
submit the correct commit id now. What I was trying to do was, when kvm is
enabled and if we don't want to include CONFIG_ARM_VIRT or
CONFIG_ACPI_APEI, compilation fails as virt_is_acpi_enabled() routine is
defined in virt.h and acpi_ghes_record_errors() is defined in ghes.h.

~ Swetha

On Mon, May 24, 2021 at 6:53 PM Richard Henderson <
[e-mail]> wrote:

> On 5/24/21 3:56 PM, Swetha Joshi wrote:
> > From: Swetha <[e-mail]>
> >
> > Signed-off-by: Swetha <[e-mail]>
>
> What are you trying to accomplish?
> That's what belongs in the commit message that you omitted.
>
> > +    bool acpi_enabled = false;
> > +#ifdef CONFIG_ARM_VIRT
> >       bool acpi_enabled = virt_is_acpi_enabled(vms);
>
> Of course this doesn't compile, having declared acpi_enabled twice.  So
> you
> have clearly not tested this patch sufficiently.
>
> The cc to qemu-trivial is unwarranted.
>
>
> r~
>

Patch

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index dff85f6db9..724ce78265 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -1403,7 +1403,10 @@  void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
     hwaddr paddr;
     Object *obj = qdev_get_machine();
     VirtMachineState *vms = VIRT_MACHINE(obj);
+    bool acpi_enabled = false;
+#ifdef CONFIG_ARM_VIRT
     bool acpi_enabled = virt_is_acpi_enabled(vms);
+    #endif /* CONFIG_ARM_VIRT */
 
     assert(code == BUS_MCEERR_AR || code == BUS_MCEERR_AO);
 
@@ -1426,12 +1429,13 @@  void kvm_arch_on_sigbus_vcpu(CPUState *c, int code, void *addr)
              */
             if (code == BUS_MCEERR_AR) {
                 kvm_cpu_synchronize_state(c);
-                if (!acpi_ghes_record_errors(ACPI_HEST_SRC_ID_SEA, paddr)) {
-                    kvm_inject_arm_sea(c);
-                } else {
-                    error_report("failed to record the error");
-                    abort();
+#ifdef CONFIG_ACPI_APEI
+                if (acpi_ghes_record_errors(ACPI_HEST_SRC_ID_SEA, paddr)) {
+                  error_report("failed to record the error");
+                  abort();
                 }
+#endif /* CONFIG_ACPI_APEI */
+                kvm_inject_arm_sea(c);
             }
             return;
         }