| Message ID | 20230906091647.1667171-11-dbarboza@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: split TCG/KVM accelerators from cpu.c | expand |
On 6/9/23 11:16, Daniel Henrique Barboza wrote: > This file is not needed for some time now. All the stubs implemented in > it (kvm_riscv_reset_vcpu() and kvm_riscv_set_irq()) are wrapped in 'if > kvm_enabled()' blocks that the compiler will rip it out in non-KVM > builds. > > We'll also add non-KVM stubs for all functions declared in kvm_riscv.h. > All stubs are implemented as g_asserted_not_reached(), meaning that we > won't support them in non-KVM builds. This is done by other kvm headers > like kvm_arm.h and kvm_ppc.h. Aren't them also protected by kvm_enabled()? Otherwise shouldn't they? > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > target/riscv/kvm-stub.c | 30 ------------------------------ > target/riscv/kvm_riscv.h | 31 +++++++++++++++++++++++++++++++ > target/riscv/meson.build | 2 +- > 3 files changed, 32 insertions(+), 31 deletions(-) > delete mode 100644 target/riscv/kvm-stub.c > diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h > index f6501e68e2..c9ecd9a967 100644 > --- a/target/riscv/kvm_riscv.h > +++ b/target/riscv/kvm_riscv.h > @@ -19,6 +19,7 @@ > #ifndef QEMU_KVM_RISCV_H > #define QEMU_KVM_RISCV_H > > +#ifdef CONFIG_KVM > void kvm_riscv_cpu_add_kvm_properties(Object *obj); At a glance kvm_riscv_cpu_add_kvm_properties() is. Keep the prototype declared (before #ifdef CONFIG_KVM) is enough for the compiler to elide it. > void kvm_riscv_reset_vcpu(RISCVCPU *cpu); > void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level); > @@ -27,5 +28,35 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift, > uint64_t aplic_base, uint64_t imsic_base, > uint64_t guest_num); > void riscv_kvm_aplic_request(void *opaque, int irq, int level); > +#else > +static inline void kvm_riscv_cpu_add_kvm_properties(Object *obj) > +{ > + g_assert_not_reached(); > +} > + > +static inline void kvm_riscv_reset_vcpu(RISCVCPU *cpu) > +{ > + g_assert_not_reached(); > +} > + > +static inline void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level) > +{ > + g_assert_not_reached(); > +} > + > +static inline void kvm_riscv_aia_create(MachineState *machine, > + uint64_t group_shift, uint64_t aia_irq_num, > + uint64_t aia_msi_num, uint64_t aplic_base, > + uint64_t imsic_base, uint64_t guest_num) > +{ > + g_assert_not_reached(); > +} > + > +static inline void riscv_kvm_aplic_request(void *opaque, int irq, int level) > +{ > + g_assert_not_reached(); > +} > + > +#endif /* CONFIG_KVM */
On Wed, Sep 06, 2023 at 12:23:19PM +0200, Philippe Mathieu-Daudé wrote: > On 6/9/23 11:16, Daniel Henrique Barboza wrote: > > This file is not needed for some time now. All the stubs implemented in > > it (kvm_riscv_reset_vcpu() and kvm_riscv_set_irq()) are wrapped in 'if > > kvm_enabled()' blocks that the compiler will rip it out in non-KVM > > builds. > > > > We'll also add non-KVM stubs for all functions declared in kvm_riscv.h. > > All stubs are implemented as g_asserted_not_reached(), meaning that we > > won't support them in non-KVM builds. This is done by other kvm headers > > like kvm_arm.h and kvm_ppc.h. > > Aren't them also protected by kvm_enabled()? Otherwise shouldn't they? Yes, I think your earlier suggestion that we always invoke kvm functions from non-kvm files with a kvm_enabled() guard makes sense. > > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > > --- > > target/riscv/kvm-stub.c | 30 ------------------------------ > > target/riscv/kvm_riscv.h | 31 +++++++++++++++++++++++++++++++ > > target/riscv/meson.build | 2 +- > > 3 files changed, 32 insertions(+), 31 deletions(-) > > delete mode 100644 target/riscv/kvm-stub.c > > > > diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h > > index f6501e68e2..c9ecd9a967 100644 > > --- a/target/riscv/kvm_riscv.h > > +++ b/target/riscv/kvm_riscv.h > > @@ -19,6 +19,7 @@ > > #ifndef QEMU_KVM_RISCV_H > > #define QEMU_KVM_RISCV_H > > +#ifdef CONFIG_KVM > > void kvm_riscv_cpu_add_kvm_properties(Object *obj); > > At a glance kvm_riscv_cpu_add_kvm_properties() is. > Keep the prototype declared (before #ifdef CONFIG_KVM) is enough for the > compiler to elide it. Yes, when building without CONFIG_KVM enabled it's actually better to not have the stubs, since the compiler will catch an unguarded kvm function call (assuming the kvm function is defined in a file which is only built with CONFIG_KVM). Unfortunately we don't have anything to protect developers from forgetting the kvm_enabled() guard when building a QEMU which supports both TCG and KVM. We could try to remember to put 'assert(kvm_enabled())' at the start of each of these types of functions. It looks like mips does that for a couple functions. Thanks, drew
On Mon, Sep 11, 2023 at 09:49:06AM +0200, Andrew Jones wrote: > On Wed, Sep 06, 2023 at 12:23:19PM +0200, Philippe Mathieu-Daudé wrote: > > On 6/9/23 11:16, Daniel Henrique Barboza wrote: > > > This file is not needed for some time now. All the stubs implemented in > > > it (kvm_riscv_reset_vcpu() and kvm_riscv_set_irq()) are wrapped in 'if > > > kvm_enabled()' blocks that the compiler will rip it out in non-KVM > > > builds. > > > > > > We'll also add non-KVM stubs for all functions declared in kvm_riscv.h. > > > All stubs are implemented as g_asserted_not_reached(), meaning that we > > > won't support them in non-KVM builds. This is done by other kvm headers > > > like kvm_arm.h and kvm_ppc.h. > > > > Aren't them also protected by kvm_enabled()? Otherwise shouldn't they? > > Yes, I think your earlier suggestion that we always invoke kvm functions > from non-kvm files with a kvm_enabled() guard makes sense. > > > > > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > > > --- > > > target/riscv/kvm-stub.c | 30 ------------------------------ > > > target/riscv/kvm_riscv.h | 31 +++++++++++++++++++++++++++++++ > > > target/riscv/meson.build | 2 +- > > > 3 files changed, 32 insertions(+), 31 deletions(-) > > > delete mode 100644 target/riscv/kvm-stub.c > > > > > > > diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h > > > index f6501e68e2..c9ecd9a967 100644 > > > --- a/target/riscv/kvm_riscv.h > > > +++ b/target/riscv/kvm_riscv.h > > > @@ -19,6 +19,7 @@ > > > #ifndef QEMU_KVM_RISCV_H > > > #define QEMU_KVM_RISCV_H > > > +#ifdef CONFIG_KVM > > > void kvm_riscv_cpu_add_kvm_properties(Object *obj); > > > > At a glance kvm_riscv_cpu_add_kvm_properties() is. > > Keep the prototype declared (before #ifdef CONFIG_KVM) is enough for the > > compiler to elide it. > > Yes, when building without CONFIG_KVM enabled it's actually better to not > have the stubs, since the compiler will catch an unguarded kvm function > call (assuming the kvm function is defined in a file which is only built > with CONFIG_KVM). > > Unfortunately we don't have anything to protect developers from forgetting > the kvm_enabled() guard when building a QEMU which supports both TCG and > KVM. We could try to remember to put 'assert(kvm_enabled())' at the start > of each of these types of functions. It looks like mips does that for a > couple functions. Eh, ignore this suggestion. We don't need asserts, because we have CI. As long as our CI does a CONFIG_KVM=n build and all KVM functions are in kvm- only files, then we'll always catch calls of KVM functions which are missing their kvm_enabled() guards. Thanks, drew
On 9/11/23 06:04, Andrew Jones wrote: > On Mon, Sep 11, 2023 at 09:49:06AM +0200, Andrew Jones wrote: >> On Wed, Sep 06, 2023 at 12:23:19PM +0200, Philippe Mathieu-Daudé wrote: >>> On 6/9/23 11:16, Daniel Henrique Barboza wrote: >>>> This file is not needed for some time now. All the stubs implemented in >>>> it (kvm_riscv_reset_vcpu() and kvm_riscv_set_irq()) are wrapped in 'if >>>> kvm_enabled()' blocks that the compiler will rip it out in non-KVM >>>> builds. >>>> >>>> We'll also add non-KVM stubs for all functions declared in kvm_riscv.h. >>>> All stubs are implemented as g_asserted_not_reached(), meaning that we >>>> won't support them in non-KVM builds. This is done by other kvm headers >>>> like kvm_arm.h and kvm_ppc.h. >>> >>> Aren't them also protected by kvm_enabled()? Otherwise shouldn't they? >> >> Yes, I think your earlier suggestion that we always invoke kvm functions >> from non-kvm files with a kvm_enabled() guard makes sense. >> >>> >>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >>>> --- >>>> target/riscv/kvm-stub.c | 30 ------------------------------ >>>> target/riscv/kvm_riscv.h | 31 +++++++++++++++++++++++++++++++ >>>> target/riscv/meson.build | 2 +- >>>> 3 files changed, 32 insertions(+), 31 deletions(-) >>>> delete mode 100644 target/riscv/kvm-stub.c >>> >>> >>>> diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h >>>> index f6501e68e2..c9ecd9a967 100644 >>>> --- a/target/riscv/kvm_riscv.h >>>> +++ b/target/riscv/kvm_riscv.h >>>> @@ -19,6 +19,7 @@ >>>> #ifndef QEMU_KVM_RISCV_H >>>> #define QEMU_KVM_RISCV_H >>>> +#ifdef CONFIG_KVM >>>> void kvm_riscv_cpu_add_kvm_properties(Object *obj); >>> >>> At a glance kvm_riscv_cpu_add_kvm_properties() is. >>> Keep the prototype declared (before #ifdef CONFIG_KVM) is enough for the >>> compiler to elide it. >> >> Yes, when building without CONFIG_KVM enabled it's actually better to not >> have the stubs, since the compiler will catch an unguarded kvm function >> call (assuming the kvm function is defined in a file which is only built >> with CONFIG_KVM). >> >> Unfortunately we don't have anything to protect developers from forgetting >> the kvm_enabled() guard when building a QEMU which supports both TCG and >> KVM. We could try to remember to put 'assert(kvm_enabled())' at the start >> of each of these types of functions. It looks like mips does that for a >> couple functions. > > Eh, ignore this suggestion. We don't need asserts, because we have CI. As > long as our CI does a CONFIG_KVM=n build and all KVM functions are in kvm- > only files, then we'll always catch calls of KVM functions which are > missing their kvm_enabled() guards. So, to sum up, get rid of the 'g_assert_not_reached()' stubs since they should be cropped by well placed kvm_enabled() calls in the code and, if that's not the case, a non-KVM build failure will let us know that there's something wrong. I can live with that. It's less code to deal with in the header as well. Thanks, Daniel > > Thanks, > drew
On 9/11/23 09:23, Daniel Henrique Barboza wrote: > > > On 9/11/23 06:04, Andrew Jones wrote: >> On Mon, Sep 11, 2023 at 09:49:06AM +0200, Andrew Jones wrote: >>> On Wed, Sep 06, 2023 at 12:23:19PM +0200, Philippe Mathieu-Daudé wrote: >>>> On 6/9/23 11:16, Daniel Henrique Barboza wrote: >>>>> This file is not needed for some time now. All the stubs implemented in >>>>> it (kvm_riscv_reset_vcpu() and kvm_riscv_set_irq()) are wrapped in 'if >>>>> kvm_enabled()' blocks that the compiler will rip it out in non-KVM >>>>> builds. >>>>> >>>>> We'll also add non-KVM stubs for all functions declared in kvm_riscv.h. >>>>> All stubs are implemented as g_asserted_not_reached(), meaning that we >>>>> won't support them in non-KVM builds. This is done by other kvm headers >>>>> like kvm_arm.h and kvm_ppc.h. >>>> >>>> Aren't them also protected by kvm_enabled()? Otherwise shouldn't they? >>> >>> Yes, I think your earlier suggestion that we always invoke kvm functions >>> from non-kvm files with a kvm_enabled() guard makes sense. >>> >>>> >>>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >>>>> --- >>>>> target/riscv/kvm-stub.c | 30 ------------------------------ >>>>> target/riscv/kvm_riscv.h | 31 +++++++++++++++++++++++++++++++ >>>>> target/riscv/meson.build | 2 +- >>>>> 3 files changed, 32 insertions(+), 31 deletions(-) >>>>> delete mode 100644 target/riscv/kvm-stub.c >>>> >>>> >>>>> diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h >>>>> index f6501e68e2..c9ecd9a967 100644 >>>>> --- a/target/riscv/kvm_riscv.h >>>>> +++ b/target/riscv/kvm_riscv.h >>>>> @@ -19,6 +19,7 @@ >>>>> #ifndef QEMU_KVM_RISCV_H >>>>> #define QEMU_KVM_RISCV_H >>>>> +#ifdef CONFIG_KVM >>>>> void kvm_riscv_cpu_add_kvm_properties(Object *obj); >>>> >>>> At a glance kvm_riscv_cpu_add_kvm_properties() is. >>>> Keep the prototype declared (before #ifdef CONFIG_KVM) is enough for the >>>> compiler to elide it. >>> >>> Yes, when building without CONFIG_KVM enabled it's actually better to not >>> have the stubs, since the compiler will catch an unguarded kvm function >>> call (assuming the kvm function is defined in a file which is only built >>> with CONFIG_KVM). >>> >>> Unfortunately we don't have anything to protect developers from forgetting >>> the kvm_enabled() guard when building a QEMU which supports both TCG and >>> KVM. We could try to remember to put 'assert(kvm_enabled())' at the start >>> of each of these types of functions. It looks like mips does that for a >>> couple functions. >> >> Eh, ignore this suggestion. We don't need asserts, because we have CI. As >> long as our CI does a CONFIG_KVM=n build and all KVM functions are in kvm- >> only files, then we'll always catch calls of KVM functions which are >> missing their kvm_enabled() guards. > > So, to sum up, get rid of the 'g_assert_not_reached()' stubs since they should > be cropped by well placed kvm_enabled() calls in the code and, if that's not > the case, a non-KVM build failure will let us know that there's something wrong. > > I can live with that. It's less code to deal with in the header as well. Thanks, I changed my mind after talking with Michael in this thread: https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02567.html There are differences in how gcc and clang (and probably other toolchains) behave when choosing to crop code gated with 'if (kvm_enabled && something)'. It is better to have stubs for all KVM functions to avoid relying on compiler behavior. We should keep the stubs in this patch as is. Thanks, Daniel > > > Daniel > > >> >> Thanks, >> drew
On 12/9/23 12:48, Daniel Henrique Barboza wrote: > On 9/11/23 09:23, Daniel Henrique Barboza wrote: >> On 9/11/23 06:04, Andrew Jones wrote: >>> On Mon, Sep 11, 2023 at 09:49:06AM +0200, Andrew Jones wrote: >>>> On Wed, Sep 06, 2023 at 12:23:19PM +0200, Philippe Mathieu-Daudé wrote: >>>>> On 6/9/23 11:16, Daniel Henrique Barboza wrote: >>>>>> This file is not needed for some time now. All the stubs >>>>>> implemented in >>>>>> it (kvm_riscv_reset_vcpu() and kvm_riscv_set_irq()) are wrapped in >>>>>> 'if >>>>>> kvm_enabled()' blocks that the compiler will rip it out in non-KVM >>>>>> builds. >>>>>> >>>>>> We'll also add non-KVM stubs for all functions declared in >>>>>> kvm_riscv.h. >>>>>> All stubs are implemented as g_asserted_not_reached(), meaning >>>>>> that we >>>>>> won't support them in non-KVM builds. This is done by other kvm >>>>>> headers >>>>>> like kvm_arm.h and kvm_ppc.h. >>>>> >>>>> Aren't them also protected by kvm_enabled()? Otherwise shouldn't they? >>>> >>>> Yes, I think your earlier suggestion that we always invoke kvm >>>> functions >>>> from non-kvm files with a kvm_enabled() guard makes sense. >>>> >>>>> >>>>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >>>>>> --- >>>>>> target/riscv/kvm-stub.c | 30 ------------------------------ >>>>>> target/riscv/kvm_riscv.h | 31 +++++++++++++++++++++++++++++++ >>>>>> target/riscv/meson.build | 2 +- >>>>>> 3 files changed, 32 insertions(+), 31 deletions(-) >>>>>> delete mode 100644 target/riscv/kvm-stub.c >>>>> >>>>> >>>>>> diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h >>>>>> index f6501e68e2..c9ecd9a967 100644 >>>>>> --- a/target/riscv/kvm_riscv.h >>>>>> +++ b/target/riscv/kvm_riscv.h >>>>>> @@ -19,6 +19,7 @@ >>>>>> #ifndef QEMU_KVM_RISCV_H >>>>>> #define QEMU_KVM_RISCV_H >>>>>> +#ifdef CONFIG_KVM >>>>>> void kvm_riscv_cpu_add_kvm_properties(Object *obj); >>>>> >>>>> At a glance kvm_riscv_cpu_add_kvm_properties() is. >>>>> Keep the prototype declared (before #ifdef CONFIG_KVM) is enough >>>>> for the >>>>> compiler to elide it. >>>> >>>> Yes, when building without CONFIG_KVM enabled it's actually better >>>> to not >>>> have the stubs, since the compiler will catch an unguarded kvm function >>>> call (assuming the kvm function is defined in a file which is only >>>> built >>>> with CONFIG_KVM). >>>> >>>> Unfortunately we don't have anything to protect developers from >>>> forgetting >>>> the kvm_enabled() guard when building a QEMU which supports both TCG >>>> and >>>> KVM. We could try to remember to put 'assert(kvm_enabled())' at the >>>> start >>>> of each of these types of functions. It looks like mips does that for a >>>> couple functions. >>> >>> Eh, ignore this suggestion. We don't need asserts, because we have >>> CI. As >>> long as our CI does a CONFIG_KVM=n build and all KVM functions are in >>> kvm- >>> only files, then we'll always catch calls of KVM functions which are >>> missing their kvm_enabled() guards. >> >> So, to sum up, get rid of the 'g_assert_not_reached()' stubs since >> they should >> be cropped by well placed kvm_enabled() calls in the code and, if >> that's not >> the case, a non-KVM build failure will let us know that there's >> something wrong. >> >> I can live with that. It's less code to deal with in the header as >> well. Thanks, > > > I changed my mind after talking with Michael in this thread: > > https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02567.html > > > There are differences in how gcc and clang (and probably other > toolchains) behave > when choosing to crop code gated with 'if (kvm_enabled && something)'. > It is > better to have stubs for all KVM functions to avoid relying on compiler > behavior. > > We should keep the stubs in this patch as is. Thanks, Don't get stuck at this; get your series merged, we'll have a look at that later, this is not important. Regards, Phil.
On 9/12/23 08:15, Philippe Mathieu-Daudé wrote: > On 12/9/23 12:48, Daniel Henrique Barboza wrote: >> On 9/11/23 09:23, Daniel Henrique Barboza wrote: >>> On 9/11/23 06:04, Andrew Jones wrote: >>>> On Mon, Sep 11, 2023 at 09:49:06AM +0200, Andrew Jones wrote: >>>>> On Wed, Sep 06, 2023 at 12:23:19PM +0200, Philippe Mathieu-Daudé wrote: >>>>>> On 6/9/23 11:16, Daniel Henrique Barboza wrote: >>>>>>> This file is not needed for some time now. All the stubs implemented in >>>>>>> it (kvm_riscv_reset_vcpu() and kvm_riscv_set_irq()) are wrapped in 'if >>>>>>> kvm_enabled()' blocks that the compiler will rip it out in non-KVM >>>>>>> builds. >>>>>>> >>>>>>> We'll also add non-KVM stubs for all functions declared in kvm_riscv.h. >>>>>>> All stubs are implemented as g_asserted_not_reached(), meaning that we >>>>>>> won't support them in non-KVM builds. This is done by other kvm headers >>>>>>> like kvm_arm.h and kvm_ppc.h. >>>>>> >>>>>> Aren't them also protected by kvm_enabled()? Otherwise shouldn't they? >>>>> >>>>> Yes, I think your earlier suggestion that we always invoke kvm functions >>>>> from non-kvm files with a kvm_enabled() guard makes sense. >>>>> >>>>>> >>>>>>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >>>>>>> --- >>>>>>> target/riscv/kvm-stub.c | 30 ------------------------------ >>>>>>> target/riscv/kvm_riscv.h | 31 +++++++++++++++++++++++++++++++ >>>>>>> target/riscv/meson.build | 2 +- >>>>>>> 3 files changed, 32 insertions(+), 31 deletions(-) >>>>>>> delete mode 100644 target/riscv/kvm-stub.c >>>>>> >>>>>> >>>>>>> diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h >>>>>>> index f6501e68e2..c9ecd9a967 100644 >>>>>>> --- a/target/riscv/kvm_riscv.h >>>>>>> +++ b/target/riscv/kvm_riscv.h >>>>>>> @@ -19,6 +19,7 @@ >>>>>>> #ifndef QEMU_KVM_RISCV_H >>>>>>> #define QEMU_KVM_RISCV_H >>>>>>> +#ifdef CONFIG_KVM >>>>>>> void kvm_riscv_cpu_add_kvm_properties(Object *obj); >>>>>> >>>>>> At a glance kvm_riscv_cpu_add_kvm_properties() is. >>>>>> Keep the prototype declared (before #ifdef CONFIG_KVM) is enough for the >>>>>> compiler to elide it. >>>>> >>>>> Yes, when building without CONFIG_KVM enabled it's actually better to not >>>>> have the stubs, since the compiler will catch an unguarded kvm function >>>>> call (assuming the kvm function is defined in a file which is only built >>>>> with CONFIG_KVM). >>>>> >>>>> Unfortunately we don't have anything to protect developers from forgetting >>>>> the kvm_enabled() guard when building a QEMU which supports both TCG and >>>>> KVM. We could try to remember to put 'assert(kvm_enabled())' at the start >>>>> of each of these types of functions. It looks like mips does that for a >>>>> couple functions. >>>> >>>> Eh, ignore this suggestion. We don't need asserts, because we have CI. As >>>> long as our CI does a CONFIG_KVM=n build and all KVM functions are in kvm- >>>> only files, then we'll always catch calls of KVM functions which are >>>> missing their kvm_enabled() guards. >>> >>> So, to sum up, get rid of the 'g_assert_not_reached()' stubs since they should >>> be cropped by well placed kvm_enabled() calls in the code and, if that's not >>> the case, a non-KVM build failure will let us know that there's something wrong. >>> >>> I can live with that. It's less code to deal with in the header as well. Thanks, >> >> >> I changed my mind after talking with Michael in this thread: >> >> https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02567.html >> >> >> There are differences in how gcc and clang (and probably other toolchains) behave >> when choosing to crop code gated with 'if (kvm_enabled && something)'. It is >> better to have stubs for all KVM functions to avoid relying on compiler behavior. >> >> We should keep the stubs in this patch as is. Thanks, > > Don't get stuck at this; get your series merged, we'll have a look > at that later, this is not important. Ok, so we'll simplify here and avoid the unneeded stubs for now as we initially discussed, while making sure we're not breaking builds with --enable-debug and clang. After this is merged we can reevaluate the best approach to take. Thanks, Daniel > > Regards, > > Phil.
diff --git a/target/riscv/kvm-stub.c b/target/riscv/kvm-stub.c deleted file mode 100644 index 4e8fc31a21..0000000000 --- a/target/riscv/kvm-stub.c +++ /dev/null @@ -1,30 +0,0 @@ -/* - * QEMU KVM RISC-V specific function stubs - * - * Copyright (c) 2020 Huawei Technologies Co., Ltd - * - * This program is free software; you can redistribute it and/or modify it - * under the terms and conditions of the GNU General Public License, - * version 2 or later, as published by the Free Software Foundation. - * - * This program is distributed in the hope it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along with - * this program. If not, see <http://www.gnu.org/licenses/>. - */ -#include "qemu/osdep.h" -#include "cpu.h" -#include "kvm_riscv.h" - -void kvm_riscv_reset_vcpu(RISCVCPU *cpu) -{ - abort(); -} - -void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level) -{ - abort(); -} diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h index f6501e68e2..c9ecd9a967 100644 --- a/target/riscv/kvm_riscv.h +++ b/target/riscv/kvm_riscv.h @@ -19,6 +19,7 @@ #ifndef QEMU_KVM_RISCV_H #define QEMU_KVM_RISCV_H +#ifdef CONFIG_KVM void kvm_riscv_cpu_add_kvm_properties(Object *obj); void kvm_riscv_reset_vcpu(RISCVCPU *cpu); void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level); @@ -27,5 +28,35 @@ void kvm_riscv_aia_create(MachineState *machine, uint64_t group_shift, uint64_t aplic_base, uint64_t imsic_base, uint64_t guest_num); void riscv_kvm_aplic_request(void *opaque, int irq, int level); +#else +static inline void kvm_riscv_cpu_add_kvm_properties(Object *obj) +{ + g_assert_not_reached(); +} + +static inline void kvm_riscv_reset_vcpu(RISCVCPU *cpu) +{ + g_assert_not_reached(); +} + +static inline void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level) +{ + g_assert_not_reached(); +} + +static inline void kvm_riscv_aia_create(MachineState *machine, + uint64_t group_shift, uint64_t aia_irq_num, + uint64_t aia_msi_num, uint64_t aplic_base, + uint64_t imsic_base, uint64_t guest_num) +{ + g_assert_not_reached(); +} + +static inline void riscv_kvm_aplic_request(void *opaque, int irq, int level) +{ + g_assert_not_reached(); +} + +#endif /* CONFIG_KVM */ #endif diff --git a/target/riscv/meson.build b/target/riscv/meson.build index f0486183fa..3323b78b84 100644 --- a/target/riscv/meson.build +++ b/target/riscv/meson.build @@ -24,7 +24,7 @@ riscv_ss.add(files( 'zce_helper.c', 'vcrypto_helper.c' )) -riscv_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: files('kvm-stub.c')) +riscv_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c')) riscv_system_ss = ss.source_set() riscv_system_ss.add(files(
This file is not needed for some time now. All the stubs implemented in it (kvm_riscv_reset_vcpu() and kvm_riscv_set_irq()) are wrapped in 'if kvm_enabled()' blocks that the compiler will rip it out in non-KVM builds. We'll also add non-KVM stubs for all functions declared in kvm_riscv.h. All stubs are implemented as g_asserted_not_reached(), meaning that we won't support them in non-KVM builds. This is done by other kvm headers like kvm_arm.h and kvm_ppc.h. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/kvm-stub.c | 30 ------------------------------ target/riscv/kvm_riscv.h | 31 +++++++++++++++++++++++++++++++ target/riscv/meson.build | 2 +- 3 files changed, 32 insertions(+), 31 deletions(-) delete mode 100644 target/riscv/kvm-stub.c