All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] x86: more flexible alignment handling for xen binary
@ 2019-03-18 14:57 Wei Liu
  2019-03-18 14:57 ` [PATCH 1/2] x86: decouple xen alignment setting from EFI/non-EFI build Wei Liu
  2019-03-18 14:57 ` [PATCH 2/2] x86/pvshim: use 2M alignment Wei Liu
  0 siblings, 2 replies; 6+ messages in thread
From: Wei Liu @ 2019-03-18 14:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu

Wei Liu (2):
  x86: decouple xen alignment setting from EFI/non-EFI build
  x86/pvshim: use 2M alignment

 xen/arch/x86/Kconfig                  | 26 ++++++++++++++++++++++++++
 xen/arch/x86/configs/pvshim_defconfig |  1 +
 xen/arch/x86/xen.lds.S                | 16 ++++++++++++++--
 3 files changed, 41 insertions(+), 2 deletions(-)

-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] x86: decouple xen alignment setting from EFI/non-EFI build
  2019-03-18 14:57 [PATCH 0/2] x86: more flexible alignment handling for xen binary Wei Liu
@ 2019-03-18 14:57 ` Wei Liu
  2019-03-19 10:45   ` Jan Beulich
  2019-03-18 14:57 ` [PATCH 2/2] x86/pvshim: use 2M alignment Wei Liu
  1 sibling, 1 reply; 6+ messages in thread
From: Wei Liu @ 2019-03-18 14:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné

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(-)

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)
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] x86/pvshim: use 2M alignment
  2019-03-18 14:57 [PATCH 0/2] x86: more flexible alignment handling for xen binary Wei Liu
  2019-03-18 14:57 ` [PATCH 1/2] x86: decouple xen alignment setting from EFI/non-EFI build Wei Liu
@ 2019-03-18 14:57 ` Wei Liu
  1 sibling, 0 replies; 6+ messages in thread
From: Wei Liu @ 2019-03-18 14:57 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné

The pvshim is loaded directly by toolstack. Use 2M alignment for
potential better performance.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/configs/pvshim_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/arch/x86/configs/pvshim_defconfig b/xen/arch/x86/configs/pvshim_defconfig
index a12e3d0465..8e50b60cda 100644
--- a/xen/arch/x86/configs/pvshim_defconfig
+++ b/xen/arch/x86/configs/pvshim_defconfig
@@ -5,6 +5,7 @@ CONFIG_PVH_GUEST=y
 CONFIG_PV_SHIM=y
 CONFIG_PV_SHIM_EXCLUSIVE=y
 CONFIG_NR_CPUS=32
+CONFIG_XEN_ALIGN_2M=y
 # Disable features not used by the PV shim
 # CONFIG_SHADOW_PAGING is not set
 # CONFIG_BIGMEM is not set
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] x86: decouple xen alignment setting from EFI/non-EFI build
  2019-03-18 14:57 ` [PATCH 1/2] x86: decouple xen alignment setting from EFI/non-EFI build Wei Liu
@ 2019-03-19 10:45   ` Jan Beulich
  2019-03-19 11:24     ` Wei Liu
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2019-03-19 10:45 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, xen-devel, Roger Pau Monne

>>> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] x86: decouple xen alignment setting from EFI/non-EFI build
  2019-03-19 10:45   ` Jan Beulich
@ 2019-03-19 11:24     ` Wei Liu
  2019-03-19 12:48       ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Wei Liu @ 2019-03-19 11:24 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel, Roger Pau Monne

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
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] x86: decouple xen alignment setting from EFI/non-EFI build
  2019-03-19 11:24     ` Wei Liu
@ 2019-03-19 12:48       ` Jan Beulich
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2019-03-19 12:48 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, xen-devel, Roger Pau Monne

>>> 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



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-03-19 12:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-18 14:57 [PATCH 0/2] x86: more flexible alignment handling for xen binary Wei Liu
2019-03-18 14:57 ` [PATCH 1/2] x86: decouple xen alignment setting from EFI/non-EFI build Wei Liu
2019-03-19 10:45   ` Jan Beulich
2019-03-19 11:24     ` Wei Liu
2019-03-19 12:48       ` Jan Beulich
2019-03-18 14:57 ` [PATCH 2/2] x86/pvshim: use 2M alignment Wei Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.