1. Patchew
  2. QEMU
  3. [PATCH 00/13] single-binary: compile target/arm twice
  4. View patch

[PATCH 05/13] target/arm/kvm_arm: copy definitions from kvm headers

Pierrick Bouvier posted 13 patches 5 months, 2 weeks ago
There is a newer version of this series
[PATCH 05/13] target/arm/kvm_arm: copy definitions from kvm headers
Posted by Pierrick Bouvier 5 months, 2 weeks ago 
"linux/kvm.h" is not included for code compiled without
COMPILING_PER_TARGET, and headers are different depending architecture
(arm, arm64).
Thus we need to manually expose some definitions that will
be used by target/arm, ensuring they are the same for arm amd aarch64.

As well, we must but prudent to not redefine things if code is already
including linux/kvm.h, thus the #ifndef COMPILING_PER_TARGET guard.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 target/arm/kvm_arm.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index c8ddf8beb2e..eedd081064c 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -16,6 +16,21 @@
 #define KVM_ARM_VGIC_V2   (1 << 0)
 #define KVM_ARM_VGIC_V3   (1 << 1)
 
+#ifndef COMPILING_PER_TARGET
+
+/* we copy those definitions from asm-arm and asm-aarch64, as they are the same
+ * for both architectures */
+#define KVM_ARM_IRQ_CPU_IRQ 0
+#define KVM_ARM_IRQ_CPU_FIQ 1
+#define KVM_ARM_IRQ_TYPE_CPU 0
+typedef unsigned int __u32;
+struct kvm_vcpu_init {
+    __u32 target;
+    __u32 features[7];
+};
+
+#endif /* COMPILING_PER_TARGET */
+
 /**
  * kvm_arm_register_device:
  * @mr: memory region for this device
-- 
2.47.2
Re: [PATCH 05/13] target/arm/kvm_arm: copy definitions from kvm headers
Posted by Alex Bennée 5 months, 2 weeks ago 
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> "linux/kvm.h" is not included for code compiled without
> COMPILING_PER_TARGET, and headers are different depending architecture
> (arm, arm64).
> Thus we need to manually expose some definitions that will
> be used by target/arm, ensuring they are the same for arm amd aarch64.
>
> As well, we must but prudent to not redefine things if code is already
> including linux/kvm.h, thus the #ifndef COMPILING_PER_TARGET guard.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  target/arm/kvm_arm.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index c8ddf8beb2e..eedd081064c 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -16,6 +16,21 @@
>  #define KVM_ARM_VGIC_V2   (1 << 0)
>  #define KVM_ARM_VGIC_V3   (1 << 1)
>  
> +#ifndef COMPILING_PER_TARGET
> +
> +/* we copy those definitions from asm-arm and asm-aarch64, as they are the same
> + * for both architectures */
> +#define KVM_ARM_IRQ_CPU_IRQ 0
> +#define KVM_ARM_IRQ_CPU_FIQ 1
> +#define KVM_ARM_IRQ_TYPE_CPU 0
> +typedef unsigned int __u32;
> +struct kvm_vcpu_init {
> +    __u32 target;
> +    __u32 features[7];
> +};
> +
> +#endif /* COMPILING_PER_TARGET */
> +

I'm not keen on the duplication. It seems to be the only reason we have
struct kvm_vcpu_init is for kvm_arm_create_scratch_host_vcpu() where the
only *external* user passes in a NULL.

If kvm_arm_create_scratch_host_vcpu() is made internal static to
target/arm/kvm.c which will should always include the real linux headers
you just need a QMP helper.

For the IRQ types is this just a sign of target/arm/cpu.c needing
splitting into TCG and KVM bits?


>  /**
>   * kvm_arm_register_device:
>   * @mr: memory region for this device

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH 05/13] target/arm/kvm_arm: copy definitions from kvm headers
Posted by Pierrick Bouvier 5 months, 2 weeks ago 
On 4/29/25 3:28 AM, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> "linux/kvm.h" is not included for code compiled without
>> COMPILING_PER_TARGET, and headers are different depending architecture
>> (arm, arm64).
>> Thus we need to manually expose some definitions that will
>> be used by target/arm, ensuring they are the same for arm amd aarch64.
>>
>> As well, we must but prudent to not redefine things if code is already
>> including linux/kvm.h, thus the #ifndef COMPILING_PER_TARGET guard.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   target/arm/kvm_arm.h | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
>> index c8ddf8beb2e..eedd081064c 100644
>> --- a/target/arm/kvm_arm.h
>> +++ b/target/arm/kvm_arm.h
>> @@ -16,6 +16,21 @@
>>   #define KVM_ARM_VGIC_V2   (1 << 0)
>>   #define KVM_ARM_VGIC_V3   (1 << 1)
>>   
>> +#ifndef COMPILING_PER_TARGET
>> +
>> +/* we copy those definitions from asm-arm and asm-aarch64, as they are the same
>> + * for both architectures */
>> +#define KVM_ARM_IRQ_CPU_IRQ 0
>> +#define KVM_ARM_IRQ_CPU_FIQ 1
>> +#define KVM_ARM_IRQ_TYPE_CPU 0
>> +typedef unsigned int __u32;
>> +struct kvm_vcpu_init {
>> +    __u32 target;
>> +    __u32 features[7];
>> +};
>> +
>> +#endif /* COMPILING_PER_TARGET */
>> +
> 
> I'm not keen on the duplication. It seems to be the only reason we have
> struct kvm_vcpu_init is for kvm_arm_create_scratch_host_vcpu() where the
> only *external* user passes in a NULL.
>

I'm not keen about it either, so thanks for pointing it.

> If kvm_arm_create_scratch_host_vcpu() is made internal static to
> target/arm/kvm.c which will should always include the real linux headers
> you just need a QMP helper.
>

Yes, sounds like the good approach! Thanks.

> For the IRQ types is this just a sign of target/arm/cpu.c needing
> splitting into TCG and KVM bits?
> 

I'll move relevant functions to target/arm/kvm.c, so cpu.c can be 
isolated from this.

> 
>>   /**
>>    * kvm_arm_register_device:
>>    * @mr: memory region for this device
>
Re: [PATCH 05/13] target/arm/kvm_arm: copy definitions from kvm headers
Posted by Pierrick Bouvier 5 months, 2 weeks ago 
On 4/29/25 2:14 PM, Pierrick Bouvier wrote:
> On 4/29/25 3:28 AM, Alex Bennée wrote:
>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>
>>> "linux/kvm.h" is not included for code compiled without
>>> COMPILING_PER_TARGET, and headers are different depending architecture
>>> (arm, arm64).
>>> Thus we need to manually expose some definitions that will
>>> be used by target/arm, ensuring they are the same for arm amd aarch64.
>>>
>>> As well, we must but prudent to not redefine things if code is already
>>> including linux/kvm.h, thus the #ifndef COMPILING_PER_TARGET guard.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>>    target/arm/kvm_arm.h | 15 +++++++++++++++
>>>    1 file changed, 15 insertions(+)
>>>
>>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
>>> index c8ddf8beb2e..eedd081064c 100644
>>> --- a/target/arm/kvm_arm.h
>>> +++ b/target/arm/kvm_arm.h
>>> @@ -16,6 +16,21 @@
>>>    #define KVM_ARM_VGIC_V2   (1 << 0)
>>>    #define KVM_ARM_VGIC_V3   (1 << 1)
>>>    
>>> +#ifndef COMPILING_PER_TARGET
>>> +
>>> +/* we copy those definitions from asm-arm and asm-aarch64, as they are the same
>>> + * for both architectures */
>>> +#define KVM_ARM_IRQ_CPU_IRQ 0
>>> +#define KVM_ARM_IRQ_CPU_FIQ 1
>>> +#define KVM_ARM_IRQ_TYPE_CPU 0
>>> +typedef unsigned int __u32;
>>> +struct kvm_vcpu_init {
>>> +    __u32 target;
>>> +    __u32 features[7];
>>> +};
>>> +
>>> +#endif /* COMPILING_PER_TARGET */
>>> +
>>
>> I'm not keen on the duplication. It seems to be the only reason we have
>> struct kvm_vcpu_init is for kvm_arm_create_scratch_host_vcpu() where the
>> only *external* user passes in a NULL.
>>
> 
> I'm not keen about it either, so thanks for pointing it.
> 
>> If kvm_arm_create_scratch_host_vcpu() is made internal static to
>> target/arm/kvm.c which will should always include the real linux headers
>> you just need a QMP helper.
>>
> 
> Yes, sounds like the good approach! Thanks.
>

Alas this function is used in target/arm/arm-qmp-cmds.c, and if we move 
the code using it, it pulls QAPI, which is target dependent at this time.

Since struct kvm_vcpu_init is only used by pointer, I could workaround 
this by doing a simple forward declaration in kvm_arm.h.

>> For the IRQ types is this just a sign of target/arm/cpu.c needing
>> splitting into TCG and KVM bits?
>>
> 
> I'll move relevant functions to target/arm/kvm.c, so cpu.c can be
> isolated from this.
> 
>>
>>>    /**
>>>     * kvm_arm_register_device:
>>>     * @mr: memory region for this device
>>
>
Re: [PATCH 05/13] target/arm/kvm_arm: copy definitions from kvm headers
Posted by Philippe Mathieu-Daudé 5 months, 2 weeks ago 
On 30/4/25 00:02, Pierrick Bouvier wrote:
> On 4/29/25 2:14 PM, Pierrick Bouvier wrote:
>> On 4/29/25 3:28 AM, Alex Bennée wrote:
>>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>>
>>>> "linux/kvm.h" is not included for code compiled without
>>>> COMPILING_PER_TARGET, and headers are different depending architecture
>>>> (arm, arm64).
>>>> Thus we need to manually expose some definitions that will
>>>> be used by target/arm, ensuring they are the same for arm amd aarch64.
>>>>
>>>> As well, we must but prudent to not redefine things if code is already
>>>> including linux/kvm.h, thus the #ifndef COMPILING_PER_TARGET guard.
>>>>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> ---
>>>>    target/arm/kvm_arm.h | 15 +++++++++++++++
>>>>    1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
>>>> index c8ddf8beb2e..eedd081064c 100644
>>>> --- a/target/arm/kvm_arm.h
>>>> +++ b/target/arm/kvm_arm.h
>>>> @@ -16,6 +16,21 @@
>>>>    #define KVM_ARM_VGIC_V2   (1 << 0)
>>>>    #define KVM_ARM_VGIC_V3   (1 << 1)
>>>> +#ifndef COMPILING_PER_TARGET
>>>> +
>>>> +/* we copy those definitions from asm-arm and asm-aarch64, as they 
>>>> are the same
>>>> + * for both architectures */
>>>> +#define KVM_ARM_IRQ_CPU_IRQ 0
>>>> +#define KVM_ARM_IRQ_CPU_FIQ 1
>>>> +#define KVM_ARM_IRQ_TYPE_CPU 0
>>>> +typedef unsigned int __u32;
>>>> +struct kvm_vcpu_init {
>>>> +    __u32 target;
>>>> +    __u32 features[7];
>>>> +};
>>>> +
>>>> +#endif /* COMPILING_PER_TARGET */
>>>> +
>>>
>>> I'm not keen on the duplication. It seems to be the only reason we have
>>> struct kvm_vcpu_init is for kvm_arm_create_scratch_host_vcpu() where the
>>> only *external* user passes in a NULL.
>>>
>>
>> I'm not keen about it either, so thanks for pointing it.
>>
>>> If kvm_arm_create_scratch_host_vcpu() is made internal static to
>>> target/arm/kvm.c which will should always include the real linux headers
>>> you just need a QMP helper.
>>>
>>
>> Yes, sounds like the good approach! Thanks.
>>
> 
> Alas this function is used in target/arm/arm-qmp-cmds.c, and if we move 
> the code using it, it pulls QAPI, which is target dependent at this time.
> 
> Since struct kvm_vcpu_init is only used by pointer, I could workaround 
> this by doing a simple forward declaration in kvm_arm.h.

Correct, great!

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.