| Message ID | 20240513134046.82605-9-eliasely@amazon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Remove the directmap | expand |
On Mon, May 13, 2024 at 01:40:35PM +0000, Elias El Yandouzi wrote: > The early fixed addresses must all fit into the static L1 table. > Introduce a build assertion to this end. > > Signed-off-by: Elias El Yandouzi <eliasely@amazon.com> > > ---- > > Changes in v2: > * New patch > > diff --git a/xen/arch/x86/include/asm/fixmap.h b/xen/arch/x86/include/asm/fixmap.h > index a7ac365fc6..904bee0480 100644 > --- a/xen/arch/x86/include/asm/fixmap.h > +++ b/xen/arch/x86/include/asm/fixmap.h > @@ -77,6 +77,11 @@ enum fixed_addresses { > #define FIXADDR_SIZE (__end_of_fixed_addresses << PAGE_SHIFT) > #define FIXADDR_START (FIXADDR_TOP - FIXADDR_SIZE) > > +static inline void fixaddr_build_assertion(void) > +{ > + BUILD_BUG_ON(FIX_PMAP_END > L1_PAGETABLE_ENTRIES - 1); > +} Just introduce the BUILD_BUG_ON somewhere else, no need for a new function just for this. Adding the BUILD_BUG_ON() to pmap_map() would be perfectly fine. Thanks, Roger.
On 14.05.2024 11:42, Roger Pau Monné wrote: > On Mon, May 13, 2024 at 01:40:35PM +0000, Elias El Yandouzi wrote: >> The early fixed addresses must all fit into the static L1 table. >> Introduce a build assertion to this end. >> >> Signed-off-by: Elias El Yandouzi <eliasely@amazon.com> >> >> ---- >> >> Changes in v2: >> * New patch >> >> diff --git a/xen/arch/x86/include/asm/fixmap.h b/xen/arch/x86/include/asm/fixmap.h >> index a7ac365fc6..904bee0480 100644 >> --- a/xen/arch/x86/include/asm/fixmap.h >> +++ b/xen/arch/x86/include/asm/fixmap.h >> @@ -77,6 +77,11 @@ enum fixed_addresses { >> #define FIXADDR_SIZE (__end_of_fixed_addresses << PAGE_SHIFT) >> #define FIXADDR_START (FIXADDR_TOP - FIXADDR_SIZE) >> >> +static inline void fixaddr_build_assertion(void) >> +{ >> + BUILD_BUG_ON(FIX_PMAP_END > L1_PAGETABLE_ENTRIES - 1); >> +} > > Just introduce the BUILD_BUG_ON somewhere else, no need for a new > function just for this. And especially not in an inline function (thus triggering the potential error perhaps several dozen times in a highly parallel build). > Adding the BUILD_BUG_ON() to pmap_map() would be perfectly fine. Nevertheless we have build_assertions() functions in a couple of places, so yes, there are precedents to doing so rather that putting the constructs at more or less random places. Jan
On 13.05.2024 15:40, Elias El Yandouzi wrote: > The early fixed addresses must all fit into the static L1 table. > Introduce a build assertion to this end. > > Signed-off-by: Elias El Yandouzi <eliasely@amazon.com> > > ---- > > Changes in v2: > * New patch > > diff --git a/xen/arch/x86/include/asm/fixmap.h b/xen/arch/x86/include/asm/fixmap.h > index a7ac365fc6..904bee0480 100644 > --- a/xen/arch/x86/include/asm/fixmap.h > +++ b/xen/arch/x86/include/asm/fixmap.h > @@ -77,6 +77,11 @@ enum fixed_addresses { > #define FIXADDR_SIZE (__end_of_fixed_addresses << PAGE_SHIFT) > #define FIXADDR_START (FIXADDR_TOP - FIXADDR_SIZE) > > +static inline void fixaddr_build_assertion(void) > +{ > + BUILD_BUG_ON(FIX_PMAP_END > L1_PAGETABLE_ENTRIES - 1); > +} > + > extern void __set_fixmap( > enum fixed_addresses idx, unsigned long mfn, unsigned long flags); > In addition to earlier comments that were given - this looks like it wants to be part of another patch (the previous one? a subsequent one?), rather than being standalone. Such an check makes sense in connection with code actually leveraging the assumption checked. Jan
diff --git a/xen/arch/x86/include/asm/fixmap.h b/xen/arch/x86/include/asm/fixmap.h index a7ac365fc6..904bee0480 100644 --- a/xen/arch/x86/include/asm/fixmap.h +++ b/xen/arch/x86/include/asm/fixmap.h @@ -77,6 +77,11 @@ enum fixed_addresses { #define FIXADDR_SIZE (__end_of_fixed_addresses << PAGE_SHIFT) #define FIXADDR_START (FIXADDR_TOP - FIXADDR_SIZE) +static inline void fixaddr_build_assertion(void) +{ + BUILD_BUG_ON(FIX_PMAP_END > L1_PAGETABLE_ENTRIES - 1); +} + extern void __set_fixmap( enum fixed_addresses idx, unsigned long mfn, unsigned long flags);
The early fixed addresses must all fit into the static L1 table. Introduce a build assertion to this end. Signed-off-by: Elias El Yandouzi <eliasely@amazon.com> ---- Changes in v2: * New patch