| Message ID | 20250429050010.971128-6-pierrick.bouvier@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | single-binary: compile target/arm twice | expand |
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
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 >
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 >> >
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!
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
"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(+)