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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
