qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 01/19] target/arm: Rename KVM set_feature() as kvm_set_fea


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v3 01/19] target/arm: Rename KVM set_feature() as kvm_set_feature()
Date: Sun, 19 Apr 2020 18:31:48 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0

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/address@hidden/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.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]