| Message ID | 20241002103052.1797237-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/kexec: Separate code and data by at least 1 cacheline | expand |
On 02.10.2024 12:30, Andrew Cooper wrote: > No functional change, but it performs a bit better. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> A question nevertheless: > --- a/xen/arch/x86/x86_64/kexec_reloc.S > +++ b/xen/arch/x86/x86_64/kexec_reloc.S > @@ -19,6 +19,7 @@ > #include <xen/kimage.h> > > #include <asm/asm_defns.h> > +#include <asm/cache.h> > #include <asm/msr-index.h> > #include <asm/page.h> > #include <asm/machine_kexec.h> > @@ -174,6 +175,9 @@ FUNC_LOCAL(compatibility_mode) > ud2 > END(compatibility_mode) > > + /* Separate code and data into into different cache lines */ > + .balign L1_CACHE_BYTES > + > DATA_LOCAL(compat_mode_gdt_desc, 4) > .word .Lcompat_mode_gdt_end - compat_mode_gdt -1 > .quad 0x0000000000000000 /* set in call_32_bit above */ Because of L1_CACHE_BYTES being 128, you indeed put at least 1 cache line in between. Is that necessary, though? Just starting data on the next cache line ought to be enough? IOW if and when we adjust L1_CACHE_BYTES, we won't need to touch this again, just that the title here then would end up slightly misleading. Jan
On 02/10/2024 1:08 pm, Jan Beulich wrote: > On 02.10.2024 12:30, Andrew Cooper wrote: >> No functional change, but it performs a bit better. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> Thanks. > > A question nevertheless: > >> --- a/xen/arch/x86/x86_64/kexec_reloc.S >> +++ b/xen/arch/x86/x86_64/kexec_reloc.S >> @@ -19,6 +19,7 @@ >> #include <xen/kimage.h> >> >> #include <asm/asm_defns.h> >> +#include <asm/cache.h> >> #include <asm/msr-index.h> >> #include <asm/page.h> >> #include <asm/machine_kexec.h> >> @@ -174,6 +175,9 @@ FUNC_LOCAL(compatibility_mode) >> ud2 >> END(compatibility_mode) >> >> + /* Separate code and data into into different cache lines */ >> + .balign L1_CACHE_BYTES >> + >> DATA_LOCAL(compat_mode_gdt_desc, 4) >> .word .Lcompat_mode_gdt_end - compat_mode_gdt -1 >> .quad 0x0000000000000000 /* set in call_32_bit above */ > Because of L1_CACHE_BYTES being 128, you indeed put at least 1 cache line in > between. Is that necessary, though? Just starting data on the next cache line > ought to be enough? Correct. Specifically, we want the write into compat_mode_gdt_desc not hitting a line in L1i. > IOW if and when we adjust L1_CACHE_BYTES, we won't need > to touch this again, just that the title here then would end up slightly > misleading. Oh - I'll just copy the comment and say "different". That is slightly poor grammar. ~Andrew
diff --git a/xen/arch/x86/x86_64/kexec_reloc.S b/xen/arch/x86/x86_64/kexec_reloc.S index 9f40c80d7c4b..50ba454abd48 100644 --- a/xen/arch/x86/x86_64/kexec_reloc.S +++ b/xen/arch/x86/x86_64/kexec_reloc.S @@ -19,6 +19,7 @@ #include <xen/kimage.h> #include <asm/asm_defns.h> +#include <asm/cache.h> #include <asm/msr-index.h> #include <asm/page.h> #include <asm/machine_kexec.h> @@ -174,6 +175,9 @@ FUNC_LOCAL(compatibility_mode) ud2 END(compatibility_mode) + /* Separate code and data into into different cache lines */ + .balign L1_CACHE_BYTES + DATA_LOCAL(compat_mode_gdt_desc, 4) .word .Lcompat_mode_gdt_end - compat_mode_gdt -1 .quad 0x0000000000000000 /* set in call_32_bit above */
No functional change, but it performs a bit better. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> v2: * Rebase over metadata changes. --- xen/arch/x86/x86_64/kexec_reloc.S | 4 ++++ 1 file changed, 4 insertions(+)