Further use the generic framework from xen/linkage.h. While there drop
excess alignment and move to .bss.
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Of course alongside ASM_INT() we could introduce ASM_QUAD() and
ASM_QUAD_LOCAL() (only the latter needed right here) to aid readability.
Thoughts?
--- a/xen/arch/x86/acpi/wakeup_prot.S
+++ b/xen/arch/x86/acpi/wakeup_prot.S
@@ -1,3 +1,5 @@
+#define DATA_FILL 0 /* For the .bss contributions at the bottom. */
+
#include <asm/asm_defns.h>
#include <asm/msr-index.h>
#include <asm/page.h>
@@ -134,13 +136,20 @@ LABEL(s3_resume)
ret
END(do_suspend_lowlevel)
-.data
- .align 16
+ .bss
-saved_rsp: .quad 0
-saved_cr0: .quad 0
+DATA_LOCAL(saved_rsp, 8)
+ .quad 0
+END(saved_rsp)
+DATA_LOCAL(saved_cr0, 8)
+ .quad 0
+END(saved_cr0)
#ifdef CONFIG_XEN_SHSTK
-saved_ssp: .quad 0
+DATA_LOCAL(saved_ssp, 8)
+ .quad 0
+END(saved_ssp)
#endif
+ .data
+
ASM_INT(saved_magic, 0x9abcdef0)
On 02/10/2024 8:41 am, Jan Beulich wrote: > Further use the generic framework from xen/linkage.h. While there drop > excess alignment and move to .bss. > > Requested-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > Of course alongside ASM_INT() we could introduce ASM_QUAD() and > ASM_QUAD_LOCAL() (only the latter needed right here) to aid readability. > Thoughts? Honestly, ASM_INT() hiding a .long is confusing enough already. ASM_C_{INT,LONG}() wouldn't be as bad. At least they're clear about being a particular type in another language. > --- a/xen/arch/x86/acpi/wakeup_prot.S > +++ b/xen/arch/x86/acpi/wakeup_prot.S > @@ -1,3 +1,5 @@ > +#define DATA_FILL 0 /* For the .bss contributions at the bottom. */ > + I really feel that here is the wrong place for this to live. Why isn't it in xen/linkage.h? When is data typically padded with anything other than 0's? ~Andrew
On 02.10.2024 11:07, Andrew Cooper wrote: > On 02/10/2024 8:41 am, Jan Beulich wrote: >> Further use the generic framework from xen/linkage.h. While there drop >> excess alignment and move to .bss. >> >> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> Of course alongside ASM_INT() we could introduce ASM_QUAD() and >> ASM_QUAD_LOCAL() (only the latter needed right here) to aid readability. >> Thoughts? > > Honestly, ASM_INT() hiding a .long is confusing enough already. > > ASM_C_{INT,LONG}() wouldn't be as bad. At least they're clear about > being a particular type in another language. I don't think the _C_ would add much; we all know C is the language Xen is written in. ASM_LONG() / ASM_C_LONG() would not be generalizable, i.e. couldn't be put in xen/linkage.h without arch customization, as that can neither expand uniformly to .long nor to .quad. It's all solvable, but would be getting involved. >> --- a/xen/arch/x86/acpi/wakeup_prot.S >> +++ b/xen/arch/x86/acpi/wakeup_prot.S >> @@ -1,3 +1,5 @@ >> +#define DATA_FILL 0 /* For the .bss contributions at the bottom. */ >> + > > I really feel that here is the wrong place for this to live. > > Why isn't it in xen/linkage.h? When is data typically padded with > anything other than 0's? As per what we currently have, the default data padding is ~0. Personally I consider this marginally better than 0, but it could of course be changed. Jan
© 2016 - 2024 Red Hat, Inc.