[PATCH 0/2] sysemu/kvm: Header cleanups around "cpu.h"

[PATCH 0/2] sysemu/kvm: Header cleanups around "cpu.h"

[Philippe Mathieu-Daudé]

These patches are part of a big refactor cleanup
around "cpu.h". Most changes should be trivial IMHO.

Philippe Mathieu-Daudé (2):
  sysemu/kvm: Remove CONFIG_USER_ONLY guard
  sysemu/kvm: Reduce target-specific declarations

 include/sysemu/kvm.h | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

-- 
2.38.1

[PATCH 1/2] sysemu/kvm: Remove CONFIG_USER_ONLY guard

[Philippe Mathieu-Daudé]

User emulation shouldn't really include this header; if included
these declarations are guarded by CONFIG_KVM_IS_POSSIBLE.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/kvm.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index e9a97eda8c..c8281c07a7 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -471,10 +471,8 @@ uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index);
 
 void kvm_set_sigmask_len(KVMState *s, unsigned int sigmask_len);
 
-#if !defined(CONFIG_USER_ONLY)
 int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr,
                                        hwaddr *phys_addr);
-#endif
 
 #endif /* NEED_CPU_H */
 
-- 
2.38.1

[PATCH 2/2] sysemu/kvm: Reduce target-specific declarations

[Philippe Mathieu-Daudé]

Only the declarations using the target_ulong type are
target specific.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/sysemu/kvm.h | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index c8281c07a7..a53d6dab49 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -242,9 +242,6 @@ bool kvm_arm_supports_user_irq(void);
 int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
 int kvm_on_sigbus(int code, void *addr);
 
-#ifdef NEED_CPU_H
-#include "cpu.h"
-
 void kvm_flush_coalesced_mmio_buffer(void);
 
 /**
@@ -410,6 +407,9 @@ void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic);
 struct kvm_guest_debug;
 struct kvm_debug_exit_arch;
 
+#ifdef NEED_CPU_H
+#include "cpu.h"
+
 struct kvm_sw_breakpoint {
     target_ulong pc;
     target_ulong saved_insn;
@@ -436,6 +436,15 @@ void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg);
 
 bool kvm_arch_stop_on_emulation_error(CPUState *cpu);
 
+uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
+                                      uint32_t index, int reg);
+uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index);
+
+int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr,
+                                       hwaddr *phys_addr);
+
+#endif /* NEED_CPU_H */
+
 int kvm_check_extension(KVMState *s, unsigned int extension);
 
 int kvm_vm_check_extension(KVMState *s, unsigned int extension);
@@ -464,18 +473,8 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension);
         kvm_vcpu_ioctl(cpu, KVM_ENABLE_CAP, &cap);                   \
     })
 
-uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
-                                      uint32_t index, int reg);
-uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index);
-
-
 void kvm_set_sigmask_len(KVMState *s, unsigned int sigmask_len);
 
-int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr,
-                                       hwaddr *phys_addr);
-
-#endif /* NEED_CPU_H */
-
 void kvm_cpu_synchronize_state(CPUState *cpu);
 
 void kvm_init_cpu_signals(CPUState *cpu);
-- 
2.38.1

Re: [PATCH 1/2] sysemu/kvm: Remove CONFIG_USER_ONLY guard

[Richard Henderson]

On 12/16/22 14:07, Philippe Mathieu-Daudé wrote:
> User emulation shouldn't really include this header; if included
> these declarations are guarded by CONFIG_KVM_IS_POSSIBLE.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/sysemu/kvm.h | 2 --
>   1 file changed, 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 2/2] sysemu/kvm: Reduce target-specific declarations

[Richard Henderson]

On 12/16/22 14:07, Philippe Mathieu-Daudé wrote:
> Only the declarations using the target_ulong type are
> target specific.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/sysemu/kvm.h | 25 ++++++++++++-------------
>   1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index c8281c07a7..a53d6dab49 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -242,9 +242,6 @@ bool kvm_arm_supports_user_irq(void);
>   int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
>   int kvm_on_sigbus(int code, void *addr);
>   
> -#ifdef NEED_CPU_H
> -#include "cpu.h"
> -
>   void kvm_flush_coalesced_mmio_buffer(void);
>   
>   /**
> @@ -410,6 +407,9 @@ void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic);
>   struct kvm_guest_debug;
>   struct kvm_debug_exit_arch;
>   
> +#ifdef NEED_CPU_H
> +#include "cpu.h"
> +
>   struct kvm_sw_breakpoint {
>       target_ulong pc;
>       target_ulong saved_insn;
> @@ -436,6 +436,15 @@ void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg);
>   
>   bool kvm_arch_stop_on_emulation_error(CPUState *cpu);
>   
> +uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
> +                                      uint32_t index, int reg);
> +uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index);
> +
> +int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr,
> +                                       hwaddr *phys_addr);

Why did these need to move?


r~

> +
> +#endif /* NEED_CPU_H */
> +
>   int kvm_check_extension(KVMState *s, unsigned int extension);
>   
>   int kvm_vm_check_extension(KVMState *s, unsigned int extension);
> @@ -464,18 +473,8 @@ int kvm_vm_check_extension(KVMState *s, unsigned int extension);
>           kvm_vcpu_ioctl(cpu, KVM_ENABLE_CAP, &cap);                   \
>       })
>   
> -uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
> -                                      uint32_t index, int reg);
> -uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t index);
> -
> -
>   void kvm_set_sigmask_len(KVMState *s, unsigned int sigmask_len);
>   
> -int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr,
> -                                       hwaddr *phys_addr);
> -
> -#endif /* NEED_CPU_H */
> -
>   void kvm_cpu_synchronize_state(CPUState *cpu);
>   
>   void kvm_init_cpu_signals(CPUState *cpu);

Re: [PATCH 2/2] sysemu/kvm: Reduce target-specific declarations

[Philippe Mathieu-Daudé]

On 17/12/22 01:28, Richard Henderson wrote:
> On 12/16/22 14:07, Philippe Mathieu-Daudé wrote:
>> Only the declarations using the target_ulong type are
>> target specific.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/sysemu/kvm.h | 25 ++++++++++++-------------
>>   1 file changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>> index c8281c07a7..a53d6dab49 100644
>> --- a/include/sysemu/kvm.h
>> +++ b/include/sysemu/kvm.h
>> @@ -242,9 +242,6 @@ bool kvm_arm_supports_user_irq(void);
>>   int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
>>   int kvm_on_sigbus(int code, void *addr);
>> -#ifdef NEED_CPU_H
>> -#include "cpu.h"
>> -
>>   void kvm_flush_coalesced_mmio_buffer(void);
>>   /**
>> @@ -410,6 +407,9 @@ void kvm_get_apic_state(DeviceState *d, struct 
>> kvm_lapic_state *kapic);
>>   struct kvm_guest_debug;
>>   struct kvm_debug_exit_arch;
>> +#ifdef NEED_CPU_H
>> +#include "cpu.h"
>> +
>>   struct kvm_sw_breakpoint {
>>       target_ulong pc;
>>       target_ulong saved_insn;
>> @@ -436,6 +436,15 @@ void kvm_arch_update_guest_debug(CPUState *cpu, 
>> struct kvm_guest_debug *dbg);
>>   bool kvm_arch_stop_on_emulation_error(CPUState *cpu);
>> +uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
>> +                                      uint32_t index, int reg);
>> +uint64_t kvm_arch_get_supported_msr_feature(KVMState *s, uint32_t 
>> index);
>> +
>> +int kvm_physical_memory_addr_from_host(KVMState *s, void *ram_addr,
>> +                                       hwaddr *phys_addr);
> 
> Why did these need to move?

kvm_arch_get_XXX() don't need to move, but they are only called from
target-specific code, so there is no point in declaring them for
target-agnostic part; besides that helps catching unnecessary
target-specific stuff built within target-agnostic code.


Normally kvm_enabled() shouldn't be use by user-mode code; we could
poison it. In practice we have few common files calling it, so this
header is included, declaring kvm_physical_memory_addr_from_host()
which uses hwaddr. I'm trying to not define hwaddr in user-mode.

I thought this patch would be trivial :/ I'll split it with tiny /
better justified ones.

Thanks,

Phil.