On Wed, 8 Aug 2012 15:58:11 +0100, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote:
> On 12 July 2012 06:54, Rusty Russell <rusty.russell@xxxxxxxxxx> wrote:
> > +/* If you need to interpret the index values, here's the key. */
> > +#define KVM_ARM_MSR_COPROC_MASK 0xFFFF0000
> > +#define KVM_ARM_MSR_64_BIT_MASK 0x00008000
> > +#define KVM_ARM_MSR_64_OPC1_MASK 0x000000F0
> > +#define KVM_ARM_MSR_64_CRM_MASK 0x0000000F
> > +#define KVM_ARM_MSR_32_CRM_MASK 0x0000000F
> > +#define KVM_ARM_MSR_32_OPC2_MASK 0x00000070
> > +#define KVM_ARM_MSR_32_CRN_MASK 0x00000780
> > +#define KVM_ARM_MSR_32_OPC1_MASK 0x00003800
>
> >From userspace's point of view, these defines are not very helpful,
> since they don't make it easy to construct an index from a
> (copro,crn,opc1,crm,opc2) tuple. Your QEMU patchset includes:
>
> #define MSR32_INDEX_OF(coproc, crn, opc1, crm, opc2) \
> (((coproc)<<16) | ((opc1)<<11) | ((crn)<<7) | ((opc2)<<4) | (crm))
>
> ...but it would be nice if those magic numbers were actually
> defined constants from the kernel headers I think.
Hmm, you should have ffs(), which makes this easy:
/* Given a simple mask, get those bits. */
static inline u32 get_bits(u32 index, u32 mask)
{
return (index & mask) >> (ffs(mask) - 1);
}
Since your ffs probably ends up as __builtin_ffs(), it should evaporate
nicely with this constant.
Consider it my little grip against foolishly redundant #defines in
headers :)
Cheers,
Rusty.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
