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

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

[Swetha Joshi]

Signed-off-by: Swetha Joshi <swethajoshi139@gmail.com>
---
 target/arm/kvm64.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index dff85f6db9..47a4d9d831 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 = virt_is_acpi_enabled(vms);
+    bool acpi_enabled = false;
+#ifdef CONFIG_ARM_VIRT
+    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 {
+#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;
         }
-- 
2.25.1

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Richard Henderson]

On 5/24/21 7:58 PM, Swetha Joshi wrote:
> Signed-off-by: Swetha Joshi <swethajoshi139@gmail.com>
> ---
>   target/arm/kvm64.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)

You're still missing the commit message.

> 
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index dff85f6db9..47a4d9d831 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 = virt_is_acpi_enabled(vms);
> +    bool acpi_enabled = false;
> +#ifdef CONFIG_ARM_VIRT
> +    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 {
> +#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);
>               }

Otherwise the actual patch looks ok.

I'm a bit surprised that you need the second hunk.  I would have expected the 
entire block to be optimized away with the 'acpi_enabled = false' being 
propagated into the outermost IF.


r~

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Richard Henderson]

On 5/24/21 8:23 PM, Swetha Joshi wrote:
> Where do I add the commit message?

When you use git commit?

> About the outer if, so if config_arm_virt was enabled and if acpi_enabled Is 
> true, we still need to make sure config_acpi_apei was enabled as well.

Done in hw/arm/Kconfig:

config ARM_VIRT
...
     select ACPI_APEI


r~

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Philippe Mathieu-Daudé]

On 5/25/21 5:20 AM, Richard Henderson wrote:
> On 5/24/21 7:58 PM, Swetha Joshi wrote:
>> Signed-off-by: Swetha Joshi <swethajoshi139@gmail.com>
>> ---
>>   target/arm/kvm64.c | 12 ++++++++----
>>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> You're still missing the commit message.
> 
>>
>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
>> index dff85f6db9..47a4d9d831 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 = virt_is_acpi_enabled(vms);
>> +    bool acpi_enabled = false;

Oh, IIUC this is an user request to build QEMU *with* KVM but
*without* the Virt machine? Presumably sbsa-ref or Xilinx
Versal/ZynqMP?

Interesting, I never tested that config. Swetha, do you mind
providing more information on the use case?

Thanks,

Phil.

>> +#ifdef CONFIG_ARM_VIRT
>> +    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 {
>> +#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);
>>               }
> 
> Otherwise the actual patch looks ok.
> 
> I'm a bit surprised that you need the second hunk.  I would have
> expected the entire block to be optimized away with the 'acpi_enabled =
> false' being propagated into the outermost IF.
> 
> 
> r~
> 

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Peter Maydell]

On Tue, 25 May 2021 at 04:21, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 5/24/21 7:58 PM, Swetha Joshi wrote:
> > Signed-off-by: Swetha Joshi <swethajoshi139@gmail.com>
> > ---
> >   target/arm/kvm64.c | 12 ++++++++----
> >   1 file changed, 8 insertions(+), 4 deletions(-)
>
> You're still missing the commit message.
>
> >
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index dff85f6db9..47a4d9d831 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 = virt_is_acpi_enabled(vms);
> > +    bool acpi_enabled = false;
> > +#ifdef CONFIG_ARM_VIRT
> > +    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 {
> > +#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);
> >               }
>
> Otherwise the actual patch looks ok.

I feel like the underlying problem here is that we let a virt-board-specific
bit of code slip into what should be generic Arm/KVM code here. We do
need to know "does the board actually have an ACPI table we can record the
memory error into", but that shouldn't be a specific query to virt board
code. I think (but have not tested) that a nicer solution would look like:

bool acpi_ghes_present(void)
{
    return object_resolve_path_type("", TYPE_ACPI_GED, NULL) != NULL;
}

in hw/acpi/ghes.c, and then using that function instead of
virt_is_acpi_enabled().

That avoids the CONFIG_ARM_VIRT specific reference and the need for ifdefs,
and means that any future Arm board that wants to can support memory errors
via ACPI tables by creating the ACPI_GED device and setting up the ACPI
tables as virt does.

(You will also need: a stub "return false" version in a new ghes-stub.c file,
in an if_false clause in the meson.build line for ghes.c similar to the way
ipmi.c/ipmi-stub.c are done; a prototype in include/hw/acpi/ghes.h with a
doc-comment block documenting the function; possibly to add a stub for
acpi_ghes_record_errors() in the new ghes-stub.c.)

thanks
-- PMM

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Swetha Joshi]

[-- Attachment #1: Type: text/plain, Size: 3688 bytes --]

Hey Peter, Phil,

Yeah like Peter mentioned, when KVM is enabled and we don't want VIRT
enabled, there are a couple of routines that are being called from virt.h
and ghes.h, which is resulting in errors. I came up with this simple fix
but if you think there is a better solution to it I'll let you/ other
developers who own it decide and fix it because I don't have much
experience or visibility into what happens internally, my knowledge is
restricted to just using the configs.

Thank you for the feedback.

On Tue, May 25, 2021 at 2:05 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Tue, 25 May 2021 at 04:21, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> >
> > On 5/24/21 7:58 PM, Swetha Joshi wrote:
> > > Signed-off-by: Swetha Joshi <swethajoshi139@gmail.com>
> > > ---
> > >   target/arm/kvm64.c | 12 ++++++++----
> > >   1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > You're still missing the commit message.
> >
> > >
> > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > > index dff85f6db9..47a4d9d831 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 = virt_is_acpi_enabled(vms);
> > > +    bool acpi_enabled = false;
> > > +#ifdef CONFIG_ARM_VIRT
> > > +    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 {
> > > +#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);
> > >               }
> >
> > Otherwise the actual patch looks ok.
>
> I feel like the underlying problem here is that we let a
> virt-board-specific
> bit of code slip into what should be generic Arm/KVM code here. We do
> need to know "does the board actually have an ACPI table we can record the
> memory error into", but that shouldn't be a specific query to virt board
> code. I think (but have not tested) that a nicer solution would look like:
>
> bool acpi_ghes_present(void)
> {
>     return object_resolve_path_type("", TYPE_ACPI_GED, NULL) != NULL;
> }
>
> in hw/acpi/ghes.c, and then using that function instead of
> virt_is_acpi_enabled().
>
> That avoids the CONFIG_ARM_VIRT specific reference and the need for ifdefs,
> and means that any future Arm board that wants to can support memory errors
> via ACPI tables by creating the ACPI_GED device and setting up the ACPI
> tables as virt does.
>
> (You will also need: a stub "return false" version in a new ghes-stub.c
> file,
> in an if_false clause in the meson.build line for ghes.c similar to the way
> ipmi.c/ipmi-stub.c are done; a prototype in include/hw/acpi/ghes.h with a
> doc-comment block documenting the function; possibly to add a stub for
> acpi_ghes_record_errors() in the new ghes-stub.c.)
>
> thanks
> -- PMM
>


-- 
Regards

Swetha Joshi.

[-- Attachment #2: Type: text/html, Size: 4931 bytes --]

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Peter Maydell]

On Tue, 25 May 2021 at 17:28, Swetha Joshi <swethajoshi139@gmail.com> wrote:
>
> Hey Peter, Phil,
>
> Yeah like Peter mentioned, when KVM is enabled and we don't want VIRT enabled, there are a couple of routines that are being called from virt.h and ghes.h, which is resulting in errors. I came up with this simple fix but if you think there is a better solution to it I'll let you/ other developers who own it decide and fix it because I don't have much experience or visibility into what happens internally, my knowledge is restricted to just using the configs.

Well, QEMU builds fine for me as-is, because the default config
always enables the virt board. Do you have repro instructions for
reproducing the build failure ?

thanks
-- PMM

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Swetha Joshi]

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

Hello,

One of the qemu machines we use has KVM enabled, but we don't want the
CONFIG_ARM_VIRT enabled as it pulls in emulation of a variety of physical
hardware that we don't need. The compilation errors I mentioned are not in
the qemu mainline per say but we see them in one of the qemu derived
machines we use.

Thanks,
Swetha.

On Tue, May 25, 2021 at 10:16 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Tue, 25 May 2021 at 17:28, Swetha Joshi <swethajoshi139@gmail.com>
> wrote:
> >
> > Hey Peter, Phil,
> >
> > Yeah like Peter mentioned, when KVM is enabled and we don't want VIRT
> enabled, there are a couple of routines that are being called from virt.h
> and ghes.h, which is resulting in errors. I came up with this simple fix
> but if you think there is a better solution to it I'll let you/ other
> developers who own it decide and fix it because I don't have much
> experience or visibility into what happens internally, my knowledge is
> restricted to just using the configs.
>
> Well, QEMU builds fine for me as-is, because the default config
> always enables the virt board. Do you have repro instructions for
> reproducing the build failure ?
>
> thanks
> -- PMM
>


-- 
Regards

Swetha Joshi.

[-- Attachment #2: Type: text/html, Size: 1810 bytes --]

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Peter Maydell]

On Wed, 26 May 2021 at 18:32, Swetha Joshi <swethajoshi139@gmail.com> wrote:
>
> Hello,
>
> One of the qemu machines we use has KVM enabled, but we don't want the CONFIG_ARM_VIRT enabled as it pulls in emulation of a variety of physical hardware that we don't need. The compilation errors I mentioned are not in the qemu mainline per say but we see them in one of the qemu derived machines we use.

Sure, but unless you can give me a recipe for reproducing the
build failure in mainline I can't really help...

thanks
-- PMM

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Dongjiu Geng]

Peter Maydell <peter.maydell@linaro.org> 于2021年5月27日周四 上午2:19写道：
>
> On Wed, 26 May 2021 at 18:32, Swetha Joshi <swethajoshi139@gmail.com> wrote:
> >
> > Hello,
> >
> > One of the qemu machines we use has KVM enabled, but we don't want the CONFIG_ARM_VIRT enabled as it pulls in emulation of a variety of physical hardware that we don't need. The compilation errors I mentioned are not in the qemu mainline per say but we see them in one of the qemu derived machines we use.
>
> Sure, but unless you can give me a recipe for reproducing the
> build failure in mainline I can't really help...

Hi Swetha，
     Yes,  Can you give a method that how to reproduce the build
failure issues? Thanks

>
> thanks
> -- PMM

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Swetha Joshi]

[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]

I apologize for the delay, here are the repro steps:
1. Remove CONFIG_ARM_VIRT=y from arm-softmmu.mak
2. In .gitlab-ci.yml, crossbuild.yml and in tests/vm/Makefile.include, in
all the places where we disable kvm using -disable-kvm, replace this
with -enable-kvm
3. Build

You should see errors pointing to these routines: virt_is_acpi_enabled,
acpi_ghes_record_errors

Thanks,
Swetha.

On Fri, May 28, 2021 at 12:08 AM Dongjiu Geng <gengdongjiu1@gmail.com>
wrote:

> Peter Maydell <peter.maydell@linaro.org> 于2021年5月27日周四 上午2:19写道：
> >
> > On Wed, 26 May 2021 at 18:32, Swetha Joshi <swethajoshi139@gmail.com>
> wrote:
> > >
> > > Hello,
> > >
> > > One of the qemu machines we use has KVM enabled, but we don't want the
> CONFIG_ARM_VIRT enabled as it pulls in emulation of a variety of physical
> hardware that we don't need. The compilation errors I mentioned are not in
> the qemu mainline per say but we see them in one of the qemu derived
> machines we use.
> >
> > Sure, but unless you can give me a recipe for reproducing the
> > build failure in mainline I can't really help...
>
> Hi Swetha，
>      Yes,  Can you give a method that how to reproduce the build
> failure issues? Thanks
>
> >
> > thanks
> > -- PMM
>


-- 
Regards

Swetha Joshi.

[-- Attachment #2: Type: text/html, Size: 2991 bytes --]

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Dongjiu Geng]

Swetha Joshi <swethajoshi139@gmail.com> 于2021年5月29日周六 上午3:41写道：
>
> I apologize for the delay, here are the repro steps:
> 1. Remove CONFIG_ARM_VIRT=y from arm-softmmu.mak
> 2. In .gitlab-ci.yml, crossbuild.yml and in tests/vm/Makefile.include, in all the places where we disable kvm using -disable-kvm, replace this with -enable-kvm
> 3. Build

According to your steps, I can not see such errors，also your change is
odd. I suggested you do not this change until you indeed encounter
errors

diff --git a/default-configs/devices/arm-softmmu.mak
b/default-configs/devices/arm-softmmu.mak
index 0500156a0c..f47ab0f3b1 100644
--- a/default-configs/devices/arm-softmmu.mak
+++ b/default-configs/devices/arm-softmmu.mak
@@ -6,7 +6,6 @@ CONFIG_ARM_V7M=y
 # CONFIG_PCI_DEVICES=n
 # CONFIG_TEST_DEVICES=n

-CONFIG_ARM_VIRT=y
 CONFIG_CUBIEBOARD=y
 CONFIG_EXYNOS4=y
 CONFIG_HIGHBANK=y
diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index e94d95ec54..95387c3e5a 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -110,7 +110,7 @@ vm-build-%: $(IMAGES_DIR)/%.img
                "  VM-BUILD $*")

 vm-boot-serial-%: $(IMAGES_DIR)/%.img
-       qemu-system-x86_64 -enable-kvm -m 4G -smp 2 -nographic \
+       qemu-system-x86_64 -disable-kvm -m 4G -smp 2 -nographic \
                -drive if=none,id=vblk,cache=writeback,file="$<" \
                -netdev user,id=vnet \
                -device virtio-blk-pci,drive=vblk \


>
> You should see errors pointing to these routines: virt_is_acpi_enabled, acpi_ghes_record_errors
>
> Thanks,
> Swetha.
>
> On Fri, May 28, 2021 at 12:08 AM Dongjiu Geng <gengdongjiu1@gmail.com> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> 于2021年5月27日周四 上午2:19写道：
>> >
>> > On Wed, 26 May 2021 at 18:32, Swetha Joshi <swethajoshi139@gmail.com> wrote:
>> > >
>> > > Hello,
>> > >
>> > > One of the qemu machines we use has KVM enabled, but we don't want the CONFIG_ARM_VIRT enabled as it pulls in emulation of a variety of physical hardware that we don't need. The compilation errors I mentioned are not in the qemu mainline per say but we see them in one of the qemu derived machines we use.
>> >
>> > Sure, but unless you can give me a recipe for reproducing the
>> > build failure in mainline I can't really help...
>>
>> Hi Swetha，
>>      Yes,  Can you give a method that how to reproduce the build
>> failure issues? Thanks
>>
>> >
>> > thanks
>> > -- PMM
>
>
>
> --
> Regards
>
> Swetha Joshi.

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Swetha Joshi]

[-- Attachment #1: Type: text/plain, Size: 3026 bytes --]

Basically, you need to remove CONFIG_VIRT_ARM=y from arm-soft memu.mak and
then enable KVM, I might have missed some places where you need to enable
kvm.

On Wed, Jun 2, 2021 at 6:44 AM Dongjiu Geng <gengdongjiu1@gmail.com> wrote:

> Swetha Joshi <swethajoshi139@gmail.com> 于2021年5月29日周六 上午3:41写道：
> >
> > I apologize for the delay, here are the repro steps:
> > 1. Remove CONFIG_ARM_VIRT=y from arm-softmmu.mak
> > 2. In .gitlab-ci.yml, crossbuild.yml and in tests/vm/Makefile.include,
> in all the places where we disable kvm using -disable-kvm, replace this
> with -enable-kvm
> > 3. Build
>
> According to your steps, I can not see such errors，also your change is
> odd. I suggested you do not this change until you indeed encounter
> errors
>
> diff --git a/default-configs/devices/arm-softmmu.mak
> b/default-configs/devices/arm-softmmu.mak
> index 0500156a0c..f47ab0f3b1 100644
> --- a/default-configs/devices/arm-softmmu.mak
> +++ b/default-configs/devices/arm-softmmu.mak
> @@ -6,7 +6,6 @@ CONFIG_ARM_V7M=y
>  # CONFIG_PCI_DEVICES=n
>  # CONFIG_TEST_DEVICES=n
>
> -CONFIG_ARM_VIRT=y
>  CONFIG_CUBIEBOARD=y
>  CONFIG_EXYNOS4=y
>  CONFIG_HIGHBANK=y
> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
> index e94d95ec54..95387c3e5a 100644
> --- a/tests/vm/Makefile.include
> +++ b/tests/vm/Makefile.include
> @@ -110,7 +110,7 @@ vm-build-%: $(IMAGES_DIR)/%.img
>                 "  VM-BUILD $*")
>
>  vm-boot-serial-%: $(IMAGES_DIR)/%.img
> -       qemu-system-x86_64 -enable-kvm -m 4G -smp 2 -nographic \
> +       qemu-system-x86_64 -disable-kvm -m 4G -smp 2 -nographic \
>                 -drive if=none,id=vblk,cache=writeback,file="$<" \
>                 -netdev user,id=vnet \
>                 -device virtio-blk-pci,drive=vblk \
>
>
> >
> > You should see errors pointing to these routines: virt_is_acpi_enabled,
> acpi_ghes_record_errors
> >
> > Thanks,
> > Swetha.
> >
> > On Fri, May 28, 2021 at 12:08 AM Dongjiu Geng <gengdongjiu1@gmail.com>
> wrote:
> >>
> >> Peter Maydell <peter.maydell@linaro.org> 于2021年5月27日周四 上午2:19写道：
> >> >
> >> > On Wed, 26 May 2021 at 18:32, Swetha Joshi <swethajoshi139@gmail.com>
> wrote:
> >> > >
> >> > > Hello,
> >> > >
> >> > > One of the qemu machines we use has KVM enabled, but we don't want
> the CONFIG_ARM_VIRT enabled as it pulls in emulation of a variety of
> physical hardware that we don't need. The compilation errors I mentioned
> are not in the qemu mainline per say but we see them in one of the qemu
> derived machines we use.
> >> >
> >> > Sure, but unless you can give me a recipe for reproducing the
> >> > build failure in mainline I can't really help...
> >>
> >> Hi Swetha，
> >>      Yes,  Can you give a method that how to reproduce the build
> >> failure issues? Thanks
> >>
> >> >
> >> > thanks
> >> > -- PMM
> >
> >
> >
> > --
> > Regards
> >
> > Swetha Joshi.
>
-- 
Regards

Swetha Joshi.

[-- Attachment #2: Type: text/html, Size: 4174 bytes --]

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Peter Maydell]

On Fri, 28 May 2021 at 20:41, Swetha Joshi <swethajoshi139@gmail.com> wrote:
>
> I apologize for the delay, here are the repro steps:
> 1. Remove CONFIG_ARM_VIRT=y from arm-softmmu.mak
> 2. In .gitlab-ci.yml, crossbuild.yml and in tests/vm/Makefile.include, in all the places where we disable kvm using -disable-kvm, replace this with -enable-kvm
> 3. Build

Thanks; I reproduced the link errors and have sent a patchset
that I hope fixes this:
https://patchew.org/QEMU/20210603171259.27962-1-peter.maydell@linaro.org/

If you could test that it works for you that would be great.

-- PMM

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Swetha Joshi]

[-- Attachment #1: Type: text/plain, Size: 783 bytes --]

Oh okay, thank you. I will test this by eod today!


On Thu, Jun 3, 2021 at 10:22 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Fri, 28 May 2021 at 20:41, Swetha Joshi <swethajoshi139@gmail.com>
> wrote:
> >
> > I apologize for the delay, here are the repro steps:
> > 1. Remove CONFIG_ARM_VIRT=y from arm-softmmu.mak
> > 2. In .gitlab-ci.yml, crossbuild.yml and in tests/vm/Makefile.include,
> in all the places where we disable kvm using -disable-kvm, replace this
> with -enable-kvm
> > 3. Build
>
> Thanks; I reproduced the link errors and have sent a patchset
> that I hope fixes this:
> https://patchew.org/QEMU/20210603171259.27962-1-peter.maydell@linaro.org/
>
> If you could test that it works for you that would be great.
>
> -- PMM
>
-- 
Regards

Swetha Joshi.

[-- Attachment #2: Type: text/html, Size: 1551 bytes --]

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Swetha Joshi]

[-- Attachment #1: Type: text/plain, Size: 1104 bytes --]

Hello, I have tested this patch with our qemu and it works, thank you.

What are the next steps for this patch? So is it approved and ready to go
in mainline?

Thanks,
Swetha.

On Thu, Jun 3, 2021 at 10:30 AM Swetha Joshi <swethajoshi139@gmail.com>
wrote:

>
> Oh okay, thank you. I will test this by eod today!
>
>
> On Thu, Jun 3, 2021 at 10:22 AM Peter Maydell <peter.maydell@linaro.org>
> wrote:
>
>> On Fri, 28 May 2021 at 20:41, Swetha Joshi <swethajoshi139@gmail.com>
>> wrote:
>> >
>> > I apologize for the delay, here are the repro steps:
>> > 1. Remove CONFIG_ARM_VIRT=y from arm-softmmu.mak
>> > 2. In .gitlab-ci.yml, crossbuild.yml and in tests/vm/Makefile.include,
>> in all the places where we disable kvm using -disable-kvm, replace this
>> with -enable-kvm
>> > 3. Build
>>
>> Thanks; I reproduced the link errors and have sent a patchset
>> that I hope fixes this:
>> https://patchew.org/QEMU/20210603171259.27962-1-peter.maydell@linaro.org/
>>
>> If you could test that it works for you that would be great.
>>
>> -- PMM
>>
> --
> Regards
>
> Swetha Joshi.
>
-- 
Regards

Swetha Joshi.

[-- Attachment #2: Type: text/html, Size: 2405 bytes --]

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Peter Maydell]

On Fri, 4 Jun 2021 at 06:26, Swetha Joshi <swethajoshi139@gmail.com> wrote:
>
> Hello, I have tested this patch with our qemu and it works, thank you.

Thanks for testing.

> What are the next steps for this patch? So is it approved and ready to go in mainline?

It will go in once it has been code-reviewed.

-- PMM

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Swetha Joshi]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

Can y oh I let me know once it goes in mainline? Thanks!

On Fri, Jun 4, 2021 at 2:03 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Fri, 4 Jun 2021 at 06:26, Swetha Joshi <swethajoshi139@gmail.com>
> wrote:
> >
> > Hello, I have tested this patch with our qemu and it works, thank you.
>
> Thanks for testing.
>
> > What are the next steps for this patch? So is it approved and ready to
> go in mainline?
>
> It will go in once it has been code-reviewed.
>
> -- PMM
>
-- 
Regards

Swetha Joshi.

[-- Attachment #2: Type: text/html, Size: 1079 bytes --]

Re: [PATCH_V3] Adding ifdefs to call the respective routines only when their configs are enabled

[Peter Maydell]

On Fri, 4 Jun 2021 at 16:06, Swetha Joshi <swethajoshi139@gmail.com> wrote:
>
> Can y oh I let me know once it goes in mainline? Thanks!

Sorry for having forgotten about this -- I was just clearing
out some of my old email backlog and found your email again.
You probably discovered this long ago, but this patchset to remove
the dependency between arm kvm and the virt board went into mainline
back in late June and was in the 6.1 release.

thanks
-- PMM