| Message ID | 1476460950-10331-2-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 14/10/16 17:02, Andrew Cooper wrote: > Drop repeated identical BUILD_BUG_ON()'s > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: George Dunlap <george.dunlap@eu.citrix.com> > --- > xen/arch/x86/x86_64/mm.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c > index b8b6b70..0083beb 100644 > --- a/xen/arch/x86/x86_64/mm.c > +++ b/xen/arch/x86/x86_64/mm.c > @@ -540,7 +540,7 @@ void __init paging_init(void) > sizeof(*machine_to_phys_mapping)); > for ( i = 0; i < (mpt_size >> L2_PAGETABLE_SHIFT); i++ ) > { > - BUILD_BUG_ON(RO_MPT_VIRT_START & ((1UL << L3_PAGETABLE_SHIFT) - 1)); > + BUILD_BUG_ON(!IS_ALIGNED(RO_MPT_VIRT_START, L3_PAGETABLE_SHIFT)); This doesn't look right. This is what I have for IS_ALIGNED (in xen/include/xen/config.h): #define IS_ALIGNED(val, align) (((val) & ((align) - 1)) == 0) There's no shift in the #define, but you've taken it out of the calling code. Did I miss something? -George
On 20/10/16 17:37, George Dunlap wrote: > On 14/10/16 17:02, Andrew Cooper wrote: >> Drop repeated identical BUILD_BUG_ON()'s >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: George Dunlap <george.dunlap@eu.citrix.com> >> --- >> xen/arch/x86/x86_64/mm.c | 12 +++++------- >> 1 file changed, 5 insertions(+), 7 deletions(-) >> >> diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c >> index b8b6b70..0083beb 100644 >> --- a/xen/arch/x86/x86_64/mm.c >> +++ b/xen/arch/x86/x86_64/mm.c >> @@ -540,7 +540,7 @@ void __init paging_init(void) >> sizeof(*machine_to_phys_mapping)); >> for ( i = 0; i < (mpt_size >> L2_PAGETABLE_SHIFT); i++ ) >> { >> - BUILD_BUG_ON(RO_MPT_VIRT_START & ((1UL << L3_PAGETABLE_SHIFT) - 1)); >> + BUILD_BUG_ON(!IS_ALIGNED(RO_MPT_VIRT_START, L3_PAGETABLE_SHIFT)); > This doesn't look right. This is what I have for IS_ALIGNED (in > xen/include/xen/config.h): > > #define IS_ALIGNED(val, align) (((val) & ((align) - 1)) == 0) > > There's no shift in the #define, but you've taken it out of the calling > code. > > Did I miss something? No. I did. Sorry for the noise. This was originally part of a larger series, which I thought I had cherrypicked out correctly but clearly haven't. I will drop it for now. ~Andrew
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c index b8b6b70..0083beb 100644 --- a/xen/arch/x86/x86_64/mm.c +++ b/xen/arch/x86/x86_64/mm.c @@ -540,7 +540,7 @@ void __init paging_init(void) sizeof(*machine_to_phys_mapping)); for ( i = 0; i < (mpt_size >> L2_PAGETABLE_SHIFT); i++ ) { - BUILD_BUG_ON(RO_MPT_VIRT_START & ((1UL << L3_PAGETABLE_SHIFT) - 1)); + BUILD_BUG_ON(!IS_ALIGNED(RO_MPT_VIRT_START, L3_PAGETABLE_SHIFT)); va = RO_MPT_VIRT_START + (i << L2_PAGETABLE_SHIFT); memflags = MEMF_node(phys_to_nid(i << (L2_PAGETABLE_SHIFT - 3 + PAGE_SHIFT))); @@ -776,8 +776,8 @@ static int setup_frametable_chunk(void *start, void *end, unsigned long mfn; int err; - ASSERT(!(s & ((1 << L2_PAGETABLE_SHIFT) - 1))); - ASSERT(!(e & ((1 << L2_PAGETABLE_SHIFT) - 1))); + ASSERT(IS_ALIGNED(s, L2_PAGETABLE_SHIFT)); + ASSERT(IS_ALIGNED(e, L2_PAGETABLE_SHIFT)); for ( ; s < e; s += (1UL << L2_PAGETABLE_SHIFT)) { @@ -838,8 +838,8 @@ void __init subarch_init_memory(void) l3_pgentry_t l3e; l2_pgentry_t l2e; - BUILD_BUG_ON(RDWR_MPT_VIRT_START & ((1UL << L3_PAGETABLE_SHIFT) - 1)); - BUILD_BUG_ON(RDWR_MPT_VIRT_END & ((1UL << L3_PAGETABLE_SHIFT) - 1)); + BUILD_BUG_ON(!IS_ALIGNED(RDWR_MPT_VIRT_START, L3_PAGETABLE_SHIFT)); + BUILD_BUG_ON(!IS_ALIGNED(RDWR_MPT_VIRT_END, L3_PAGETABLE_SHIFT)); /* M2P table is mappable read-only by privileged domains. */ for ( v = RDWR_MPT_VIRT_START; v != RDWR_MPT_VIRT_END; @@ -934,8 +934,6 @@ long subarch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) if ( copy_from_guest(&xmml, arg, 1) ) return -EFAULT; - BUILD_BUG_ON(RDWR_MPT_VIRT_START & ((1UL << L3_PAGETABLE_SHIFT) - 1)); - BUILD_BUG_ON(RDWR_MPT_VIRT_END & ((1UL << L3_PAGETABLE_SHIFT) - 1)); for ( i = 0, v = RDWR_MPT_VIRT_START, last_mfn = 0; (i != xmml.max_extents) && (v < (unsigned long)(machine_to_phys_mapping + max_page));
Drop repeated identical BUILD_BUG_ON()'s Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: George Dunlap <george.dunlap@eu.citrix.com> --- xen/arch/x86/x86_64/mm.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)