On 3/17/20 10:09 AM, Philippe Mathieu-Daudé wrote:
On 3/16/20 9:16 PM, Richard Henderson wrote:
On 3/16/20 9:06 AM, Philippe Mathieu-Daudé wrote:
+++ b/target/arm/kvm32.c
@@ -22,7 +22,7 @@
#include "internals.h"
#include "qemu/log.h"
-static inline void set_feature(uint64_t *features, int feature)
+static inline void kvm_set_feature(uint64_t *features, int feature)
Why, what's wrong with the existing name?
Peter suggested the rename here:
https://www.mail-archive.com/qemu-devel@xxxxxxxxxx/msg641931.html
Plus, with patch 2, you can just remove these.
Since they don't have the same prototype, they clash:
target/arm/kvm64.c:450:20: error: conflicting types for ‘set_feature’
static inline void set_feature(uint64_t *features, int feature)
^~~~~~~~~~~
In file included from target/arm/kvm64.c:30:0:
target/arm/internals.h:30:20: note: previous definition of ‘set_feature’
was here
static inline void set_feature(CPUARMState *env, int feature)
^~~~~~~~~~~
target/arm/kvm64.c:455:20: error: conflicting types for ‘unset_feature’
static inline void unset_feature(uint64_t *features, int feature)
^~~~~~~~~~~~~
In file included from target/arm/kvm64.c:30:0:
target/arm/internals.h:35:20: note: previous definition of
‘unset_feature’ was here
static inline void unset_feature(CPUARMState *env, int feature)
^~~~~~~~~~~~~
rules.mak:69: recipe for target 'target/arm/kvm64.o' failed
make[1]: *** [target/arm/kvm64.o] Error 1
The prototypes are different:
void set_feature(uint64_t *features, int feature)
void set_feature(CPUARMState *env, int feature)
Anyway you are right, I'll use the later prototype instead, thanks.
There are ~180 uses of set_feature(CPUARMState,...) and 10 of
set_feature(uint64_t,...) (kvm32:4 kvm64:6).
We are going to remove kvm32, so replacing 180 set_feature(env) by
set_feature(env->features) seems a waste.
If you prefer to avoid renaming as kvm_set_feature() another option is
to move the declaration in a local "features.h" header that would not be
included by kvm*.c.
The main problem is the use of the ARMHostCPUFeatures structure which
apparently was introduced similar to a CPUClass (commit a96c0514ab7)
then lost this in commit c4487d76d52.
