| Message ID | 1384456020-12251-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
Il 14/11/2013 20:07, Peter Maydell ha scritto: > diff --git a/target-arm/kvm-consts.h b/target-arm/kvm-consts.h > new file mode 100644 > index 0000000..42ffb50 > --- /dev/null > +++ b/target-arm/kvm-consts.h > @@ -0,0 +1,34 @@ > +/* > + * Provide versions of KVM constant defines that can be used even > + * when CONFIG_KVM is not set and we don't have access to the > + * KVM headers. If CONFIG_KVM is set, we do a compile-time check > + * that we haven't got out of sync somehow. > + */ > +#ifndef ARM_KVM_CONSTS_H > +#define ARM_KVM_CONSTS_H > + > +#ifdef CONFIG_KVM > +#include "qemu/compiler.h" > +#include <linux/kvm.h> > + > +#define MISMATCH_CHECK(X, Y) QEMU_BUILD_BUG_ON(X != Y) > + > +#else > +#define MISMATCH_CHECK(X, Y) > +#endif > + > +#define CP_REG_SIZE_SHIFT 52 > +#define CP_REG_SIZE_MASK 0x00f0000000000000ULL > +#define CP_REG_SIZE_U32 0x0020000000000000ULL > +#define CP_REG_SIZE_U64 0x0030000000000000ULL > +#define CP_REG_ARM 0x4000000000000000ULL > + > +MISMATCH_CHECK(CP_REG_SIZE_SHIFT, KVM_REG_SIZE_SHIFT) > +MISMATCH_CHECK(CP_REG_SIZE_MASK, KVM_REG_SIZE_MASK) > +MISMATCH_CHECK(CP_REG_SIZE_U32, KVM_REG_SIZE_U32) > +MISMATCH_CHECK(CP_REG_SIZE_U64, KVM_REG_SIZE_U64) > +MISMATCH_CHECK(CP_REG_ARM, KVM_REG_ARM) > + > +#undef MISMATCH_CHECK > + > +#endif > It's okay. There are indeed advantages to putting this together with the definitions, instead of splitting it between target-arm/cpu.h and target-arm/kvm.c. The patch is missing the removal of the check from kvm.c though. Paolo
On 15 November 2013 10:54, Paolo Bonzini <pbonzini@redhat.com> wrote: > It's okay. There are indeed advantages to putting this together with > the definitions, instead of splitting it between target-arm/cpu.h and > target-arm/kvm.c. Cool. I just wanted to check I wasn't missing some clever approach to this that might have avoided the need to duplicate all the definitions. > The patch is missing the removal of the check from kvm.c though. Yeah, I put this out in a bit of a hurry last thing in the evening. You'll notice I forgot the copyright/license boilerplate in the header comment too. thanks -- PMM
Il 15/11/2013 12:26, Peter Maydell ha scritto: >> > It's okay. There are indeed advantages to putting this together with >> > the definitions, instead of splitting it between target-arm/cpu.h and >> > target-arm/kvm.c. > Cool. I just wanted to check I wasn't missing some > clever approach to this that might have avoided the > need to duplicate all the definitions. If you call your constants KVM_FOO, the compiler should not warn for a redefinition with the exact same content. But I find that more gross than clever... Paolo
On 15 November 2013 11:33, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 15/11/2013 12:26, Peter Maydell ha scritto: >>> > It's okay. There are indeed advantages to putting this together with >>> > the definitions, instead of splitting it between target-arm/cpu.h and >>> > target-arm/kvm.c. >> Cool. I just wanted to check I wasn't missing some >> clever approach to this that might have avoided the >> need to duplicate all the definitions. > > If you call your constants KVM_FOO, the compiler should not warn for a > redefinition with the exact same content. But I find that more gross > than clever... Yeah, using different names seemed like a better idea to me too. -- PMM
diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 9f110f1..c3f007f 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -21,6 +21,8 @@ #include "config.h" +#include "kvm-consts.h" + #if defined(TARGET_AARCH64) /* AArch64 definitions */ # define TARGET_LONG_BITS 64 @@ -497,17 +499,6 @@ void armv7m_nvic_complete_irq(void *opaque, int irq); (((cp) << 16) | ((is64) << 15) | ((crn) << 11) | \ ((crm) << 7) | ((opc1) << 3) | (opc2)) -/* Note that these must line up with the KVM/ARM register - * ID field definitions (kvm.c will check this, but we - * can't just use the KVM defines here as the kvm headers - * are unavailable to non-KVM-specific files) - */ -#define CP_REG_SIZE_SHIFT 52 -#define CP_REG_SIZE_MASK 0x00f0000000000000ULL -#define CP_REG_SIZE_U32 0x0020000000000000ULL -#define CP_REG_SIZE_U64 0x0030000000000000ULL -#define CP_REG_ARM 0x4000000000000000ULL - /* Convert a full 64 bit KVM register ID to the truncated 32 bit * version used as a key for the coprocessor register hashtable */ diff --git a/target-arm/kvm-consts.h b/target-arm/kvm-consts.h new file mode 100644 index 0000000..42ffb50 --- /dev/null +++ b/target-arm/kvm-consts.h @@ -0,0 +1,34 @@ +/* + * Provide versions of KVM constant defines that can be used even + * when CONFIG_KVM is not set and we don't have access to the + * KVM headers. If CONFIG_KVM is set, we do a compile-time check + * that we haven't got out of sync somehow. + */ +#ifndef ARM_KVM_CONSTS_H +#define ARM_KVM_CONSTS_H + +#ifdef CONFIG_KVM +#include "qemu/compiler.h" +#include <linux/kvm.h> + +#define MISMATCH_CHECK(X, Y) QEMU_BUILD_BUG_ON(X != Y) + +#else +#define MISMATCH_CHECK(X, Y) +#endif + +#define CP_REG_SIZE_SHIFT 52 +#define CP_REG_SIZE_MASK 0x00f0000000000000ULL +#define CP_REG_SIZE_U32 0x0020000000000000ULL +#define CP_REG_SIZE_U64 0x0030000000000000ULL +#define CP_REG_ARM 0x4000000000000000ULL + +MISMATCH_CHECK(CP_REG_SIZE_SHIFT, KVM_REG_SIZE_SHIFT) +MISMATCH_CHECK(CP_REG_SIZE_MASK, KVM_REG_SIZE_MASK) +MISMATCH_CHECK(CP_REG_SIZE_U32, KVM_REG_SIZE_U32) +MISMATCH_CHECK(CP_REG_SIZE_U64, KVM_REG_SIZE_U64) +MISMATCH_CHECK(CP_REG_ARM, KVM_REG_ARM) + +#undef MISMATCH_CHECK + +#endif
There are a number of places where it would be convenient for ARM code to have working definitions of KVM constants even in code which is compiled with CONFIG_KVM not set. In this situation we can't simply include the kernel KVM headers (which might conflict with host header definitions or not even compile on the compiler we're using) so we have to redefine equivalent constants. Provide a mechanism for doing this and checking that the values match, and use it for the constants we're currently exposing via an ad-hoc mechanism. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- A series I'm currently working on wants another couple of KVM constants outside CONFIG_KVM code (in that case it's the KVM_ARM_TARGET_* values), and hw/arm/virt.c will want KVM_PSCI_* constants. This is my attempt at at least putting all of our redefinitions of things in one header file. Does anybody think this is horribly ugly or have a better idea? target-arm/cpu.h | 13 ++----------- target-arm/kvm-consts.h | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 11 deletions(-) create mode 100644 target-arm/kvm-consts.h