On Tue, Jul 30, 2013 at 05:22:32PM +0300, Gleb Natapov wrote: > > >> > > >> BUILD_BUG_ON(!!PT_GUEST_ACCESSED_MASK != !!PT_GUEST_DIRTY_MASK); > > >> #if PT_GUEST_ACCESSED_MASK > > >> BUILD_BUG_ON(PT_GUEST_DIRTY_SHIFT <= PT_GUEST_ACCESSED_SHIFT); > > >> BUILD_BUG_ON(PT_GUEST_DIRTY_SHIFT <= PT_WRITABLE_SHIFT); > > >> BUILD_BUG_ON(PT_GUEST_DIRTY_MASK != (1 << PT_GUEST_DIRTY_SHIFT)); > > >> BUILD_BUG_ON(PT_GUEST_ACCESSED_MASK != (1 << PT_GUEST_ACCESSED_SHIFT)); > > > > > > This will not work if I define _SHIFT to be 8/9 now. > > > > They will because I put them under an appropriate "#if". :) > > > True, missed that. > > > OTOH if you define _SHIFT to be 8/9 you can move the #if so that it only > > covers the last two checks. > > > > > But we do not use > > > BUILD_BUG_ON() to check values from the same define "namespace". It is > > > implied that they are correct and when they change all "namespace" > > > remains to be consistent. If you look at BUILD_BUG_ON() that we have > > > (and this patch adds) they are from the form: > > > PT_WRITABLE_MASK != ACC_WRITE_MASK > > > ACC_WRITE_MASK != VMX_EPT_WRITABLE_MASK > > > VMX_EPT_READABLE_MASK != PT_PRESENT_MASK > > > VMX_EPT_WRITABLE_MASK != PT_WRITABLE_MASK > > > > > > i.e they compare value from different "namespaces". > > > > Yes, all these BUILD_BUG_ONs make sense. > > > > But I think BUILD_BUG_ON() is more generically for consistency checks > > and enforcing invariants that the code expects. Our invariants are: > > > > * A/D bits are enabled or disabled in pairs > > > > * dirty is the left of accessed and writable > > > > * masks should either be zero or match the corresponding shifts > > > > The alternative to BUILD_BUG_ON would be a comment that explains the > > invariants, but there's no need to use English if C can do it better! :) > > > OK, will add this in separate patch. > Not really, BUILD_BUG_ON() is not meant to be used outside of a function. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
