| Message ID | 20190318145719.11396-2-wei.liu2@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: more flexible alignment handling for xen binary | expand |
>>> On 18.03.19 at 15:57, <wei.liu2@citrix.com> wrote: > Introduce a new Kconfig option to pick the alignment for xen binary. > To retain original behaviour, the default pick for EFI build is 2M and > non-EFI build 4K. Is this a worthwhile step to take, considering that we mean to switch to 2M main section boundaries anyway? I realize it's still not necessarily clear how to get there without re-introducing the boot regression with syslinux that was observed back then, but perhaps time would better be spent there than into introducing configurability? (To be honest I don't recall whether it was determined that there was no way out of this dilemma, but it seems to me there would have been a plan.) > --- a/xen/arch/x86/Kconfig > +++ b/xen/arch/x86/Kconfig > @@ -138,6 +138,32 @@ config TBOOT > > If unsure, say Y. > > +choice > + prompt "Alignment of Xen binary" > + depends on X86 > + default XEN_ALIGN_DEFAULT > + ---help--- > + Specify alignment for Xen binary. > + > + If unsure, choose "default". > + > +config XEN_ALIGN_DEFAULT > + bool "Default alignment" > + ---help--- > + Pick alignment according to build variants. > + > + For EFI build the default alignment is 2M. For non-EFI build > + the default alignment is 4K due to syslinux failing to handle > + 2M alignment. Wasn't it the resulting image size rather than the alignment itself which did cause the problem? Has the issue been fixed in newer syslinux? > +config XEN_ALIGN_4K > + bool "4K alignment" > + > +config XEN_ALIGN_2M > + bool "2M alignment" In patch 2 you only need the latter - is there really much value in allowing explicitly requesting 4k? > @@ -20,13 +19,26 @@ ENTRY(efi_start) > #else /* !EFI */ > > #define FORMAT "elf64-x86-64" > -#define SECTION_ALIGN PAGE_SIZE > #define DECL_SECTION(x) x : AT(ADDR(x) - __XEN_VIRT_START) > > ENTRY(start_pa) > > #endif /* EFI */ > > +#if defined CONFIG_XEN_ALIGN_2M > +#define SECTION_ALIGN MB(2) > +#elif defined CONFIG_XEN_ALIGN_4K > +#define SECTION_ALIGN PAGE_SIZE > +#elif defined CONFIG_XEN_ALIGN_DEFAULT > + #ifdef EFI > + #define SECTION_ALIGN MB(2) > + #else > + #define SECTION_ALIGN PAGE_SIZE > + #endif > +#else > +#error "Section alignment undefined" > +#endif How about converting the last #elif to #else and omitting the #error? Also would you mind using the defined() style (i.e. with parentheses), as we commonly do elsewhere? And please use uniform indentation (or not) of directives. I also find the indentation itself quite unusual - we have almost no examples of it being like this outside of files we've imported from elsewhere. Typically we retain the # in column 1 and indent only the actual directive keyword. Jan
On Tue, Mar 19, 2019 at 04:45:35AM -0600, Jan Beulich wrote: > >>> On 18.03.19 at 15:57, <wei.liu2@citrix.com> wrote: > > Introduce a new Kconfig option to pick the alignment for xen binary. > > To retain original behaviour, the default pick for EFI build is 2M and > > non-EFI build 4K. > > Is this a worthwhile step to take, considering that we mean to > switch to 2M main section boundaries anyway? I realize it's still not > necessarily clear how to get there without re-introducing the boot > regression with syslinux that was observed back then, but perhaps > time would better be spent there than into introducing configurability? > (To be honest I don't recall whether it was determined that there > was no way out of this dilemma, but it seems to me there would > have been a plan.) I think it would definitely be sensible to fix it there. But we still have older syslinux to deal with. I don't think they're going away any time soon. > > > --- a/xen/arch/x86/Kconfig > > +++ b/xen/arch/x86/Kconfig > > @@ -138,6 +138,32 @@ config TBOOT > > > > If unsure, say Y. > > > > +choice > > + prompt "Alignment of Xen binary" > > + depends on X86 > > + default XEN_ALIGN_DEFAULT > > + ---help--- > > + Specify alignment for Xen binary. > > + > > + If unsure, choose "default". > > + > > +config XEN_ALIGN_DEFAULT > > + bool "Default alignment" > > + ---help--- > > + Pick alignment according to build variants. > > + > > + For EFI build the default alignment is 2M. For non-EFI build > > + the default alignment is 4K due to syslinux failing to handle > > + 2M alignment. > > Wasn't it the resulting image size rather than the alignment itself > which did cause the problem? Has the issue been fixed in newer > syslinux? > Right. I can make this clearer. "due to syslinux failing to handle the image size increment induced by 2M alignment". Also, I think s/binary/image/ in the description would be better. Looking at syslinux.git I don't immediately see patches to fix the issue (if there is such issue in the first place). > > +config XEN_ALIGN_4K > > + bool "4K alignment" > > + > > +config XEN_ALIGN_2M > > + bool "2M alignment" > > In patch 2 you only need the latter - is there really much value in > allowing explicitly requesting 4k? > I thought there would be a case in which users want 4K explicitly? I'm certainly fine with dropping the 4K alignment option. > > @@ -20,13 +19,26 @@ ENTRY(efi_start) > > #else /* !EFI */ > > > > #define FORMAT "elf64-x86-64" > > -#define SECTION_ALIGN PAGE_SIZE > > #define DECL_SECTION(x) x : AT(ADDR(x) - __XEN_VIRT_START) > > > > ENTRY(start_pa) > > > > #endif /* EFI */ > > > > +#if defined CONFIG_XEN_ALIGN_2M > > +#define SECTION_ALIGN MB(2) > > +#elif defined CONFIG_XEN_ALIGN_4K > > +#define SECTION_ALIGN PAGE_SIZE > > +#elif defined CONFIG_XEN_ALIGN_DEFAULT > > + #ifdef EFI > > + #define SECTION_ALIGN MB(2) > > + #else > > + #define SECTION_ALIGN PAGE_SIZE > > + #endif > > +#else > > +#error "Section alignment undefined" > > +#endif > > How about converting the last #elif to #else and omitting the > #error? Also would you mind using the defined() style (i.e. with > parentheses), as we commonly do elsewhere? And please use > uniform indentation (or not) of directives. I also find the > indentation itself quite unusual - we have almost no examples of > it being like this outside of files we've imported from elsewhere. > Typically we retain the # in column 1 and indent only the actual > directive keyword. Will fix (after we determine what to do with 4K option). Wei. > > Jan > >
>>> On 19.03.19 at 12:24, <wei.liu2@citrix.com> wrote: > On Tue, Mar 19, 2019 at 04:45:35AM -0600, Jan Beulich wrote: >> >>> On 18.03.19 at 15:57, <wei.liu2@citrix.com> wrote: >> > --- a/xen/arch/x86/Kconfig >> > +++ b/xen/arch/x86/Kconfig >> > @@ -138,6 +138,32 @@ config TBOOT >> > >> > If unsure, say Y. >> > >> > +choice >> > + prompt "Alignment of Xen binary" >> > + depends on X86 >> > + default XEN_ALIGN_DEFAULT >> > + ---help--- >> > + Specify alignment for Xen binary. >> > + >> > + If unsure, choose "default". >> > + >> > +config XEN_ALIGN_DEFAULT >> > + bool "Default alignment" >> > + ---help--- >> > + Pick alignment according to build variants. >> > + >> > + For EFI build the default alignment is 2M. For non-EFI build >> > + the default alignment is 4K due to syslinux failing to handle >> > + 2M alignment. >> >> Wasn't it the resulting image size rather than the alignment itself >> which did cause the problem? Has the issue been fixed in newer >> syslinux? >> > > Right. I can make this clearer. > > "due to syslinux failing to handle the image size increment induced by > 2M alignment". > > Also, I think s/binary/image/ in the description would be better. Agreed. >> > +config XEN_ALIGN_4K >> > + bool "4K alignment" >> > + >> > +config XEN_ALIGN_2M >> > + bool "2M alignment" >> >> In patch 2 you only need the latter - is there really much value in >> allowing explicitly requesting 4k? > > I thought there would be a case in which users want 4K explicitly? I'm > certainly fine with dropping the 4K alignment option. Well, without a specific need or request I'd prefer not to see extra options introduced. Jan
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index 5c2d1070b6..b15053cfbe 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -138,6 +138,32 @@ config TBOOT If unsure, say Y. +choice + prompt "Alignment of Xen binary" + depends on X86 + default XEN_ALIGN_DEFAULT + ---help--- + Specify alignment for Xen binary. + + If unsure, choose "default". + +config XEN_ALIGN_DEFAULT + bool "Default alignment" + ---help--- + Pick alignment according to build variants. + + For EFI build the default alignment is 2M. For non-EFI build + the default alignment is 4K due to syslinux failing to handle + 2M alignment. + +config XEN_ALIGN_4K + bool "4K alignment" + +config XEN_ALIGN_2M + bool "2M alignment" + +endchoice + config XEN_GUEST def_bool n prompt "Xen Guest" diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index 6e9bda5109..163de31574 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -12,7 +12,6 @@ #define FORMAT "pei-x86-64" #undef __XEN_VIRT_START #define __XEN_VIRT_START __image_base__ -#define SECTION_ALIGN MB(2) #define DECL_SECTION(x) x : ENTRY(efi_start) @@ -20,13 +19,26 @@ ENTRY(efi_start) #else /* !EFI */ #define FORMAT "elf64-x86-64" -#define SECTION_ALIGN PAGE_SIZE #define DECL_SECTION(x) x : AT(ADDR(x) - __XEN_VIRT_START) ENTRY(start_pa) #endif /* EFI */ +#if defined CONFIG_XEN_ALIGN_2M +#define SECTION_ALIGN MB(2) +#elif defined CONFIG_XEN_ALIGN_4K +#define SECTION_ALIGN PAGE_SIZE +#elif defined CONFIG_XEN_ALIGN_DEFAULT + #ifdef EFI + #define SECTION_ALIGN MB(2) + #else + #define SECTION_ALIGN PAGE_SIZE + #endif +#else +#error "Section alignment undefined" +#endif + OUTPUT_FORMAT(FORMAT, FORMAT, FORMAT) OUTPUT_ARCH(i386:x86-64)
Introduce a new Kconfig option to pick the alignment for xen binary. To retain original behaviour, the default pick for EFI build is 2M and non-EFI build 4K. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- xen/arch/x86/Kconfig | 26 ++++++++++++++++++++++++++ xen/arch/x86/xen.lds.S | 16 ++++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-)