| Message ID | 20200505142810.14827-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/pv: Fix Clang build with !CONFIG_PV32 | expand |
On Tue, May 05, 2020 at 03:28:10PM +0100, Andrew Cooper wrote: > Clang 3.5 doesn't do enough dead-code-elimination to drop the compat_gdt > reference, resulting in a linker failure: > > hidden symbol `per_cpu__compat_gdt' isn't defined > > Drop the local variable, and move evaluation of this_cpu(compat_gdt) to within > the guarded region. > > Reported-by: Roger Pau Monné <roger.pau@citrix.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Tested-and-reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks!
On 05.05.2020 16:28, Andrew Cooper wrote: > @@ -753,8 +751,9 @@ void load_system_tables(void) > _set_tssldt_desc(gdt + TSS_ENTRY, (unsigned long)tss, > sizeof(*tss) - 1, SYS_DESC_tss_avail); > if ( IS_ENABLED(CONFIG_PV32) ) > - _set_tssldt_desc(compat_gdt + TSS_ENTRY, (unsigned long)tss, > - sizeof(*tss) - 1, SYS_DESC_tss_busy); > + _set_tssldt_desc( > + this_cpu(compat_gdt) - FIRST_RESERVED_GDT_ENTRY + TSS_ENTRY, > + (unsigned long)tss, sizeof(*tss) - 1, SYS_DESC_tss_busy); Isn't indentation here off by 4 compared to what we normally do with extremely large argument expressions? Other than this lgtm. Jan
On 05/05/2020 15:52, Jan Beulich wrote: > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. > > On 05.05.2020 16:28, Andrew Cooper wrote: >> @@ -753,8 +751,9 @@ void load_system_tables(void) >> _set_tssldt_desc(gdt + TSS_ENTRY, (unsigned long)tss, >> sizeof(*tss) - 1, SYS_DESC_tss_avail); >> if ( IS_ENABLED(CONFIG_PV32) ) >> - _set_tssldt_desc(compat_gdt + TSS_ENTRY, (unsigned long)tss, >> - sizeof(*tss) - 1, SYS_DESC_tss_busy); >> + _set_tssldt_desc( >> + this_cpu(compat_gdt) - FIRST_RESERVED_GDT_ENTRY + TSS_ENTRY, >> + (unsigned long)tss, sizeof(*tss) - 1, SYS_DESC_tss_busy); > Isn't indentation here off by 4 compared to what we > normally do with extremely large argument expressions? No. This is Linux style (therefore 8-space tabs), not Xen style (4 spaces). ~Andrew
On 05.05.2020 17:05, Andrew Cooper wrote: > On 05/05/2020 15:52, Jan Beulich wrote: >> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. >> >> On 05.05.2020 16:28, Andrew Cooper wrote: >>> @@ -753,8 +751,9 @@ void load_system_tables(void) >>> _set_tssldt_desc(gdt + TSS_ENTRY, (unsigned long)tss, >>> sizeof(*tss) - 1, SYS_DESC_tss_avail); >>> if ( IS_ENABLED(CONFIG_PV32) ) >>> - _set_tssldt_desc(compat_gdt + TSS_ENTRY, (unsigned long)tss, >>> - sizeof(*tss) - 1, SYS_DESC_tss_busy); >>> + _set_tssldt_desc( >>> + this_cpu(compat_gdt) - FIRST_RESERVED_GDT_ENTRY + TSS_ENTRY, >>> + (unsigned long)tss, sizeof(*tss) - 1, SYS_DESC_tss_busy); >> Isn't indentation here off by 4 compared to what we >> normally do with extremely large argument expressions? > > No. This is Linux style (therefore 8-space tabs), not Xen style (4 spaces). Oh, right - din't pay attention at all to this being tabs, sorry. Jan
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c index 131ff03fcf..63f3893c7a 100644 --- a/xen/arch/x86/cpu/common.c +++ b/xen/arch/x86/cpu/common.c @@ -711,8 +711,6 @@ void load_system_tables(void) struct tss64 *tss = &this_cpu(tss_page).tss; seg_desc_t *gdt = this_cpu(gdt) - FIRST_RESERVED_GDT_ENTRY; - seg_desc_t *compat_gdt = - this_cpu(compat_gdt) - FIRST_RESERVED_GDT_ENTRY; const struct desc_ptr gdtr = { .base = (unsigned long)gdt, @@ -753,8 +751,9 @@ void load_system_tables(void) _set_tssldt_desc(gdt + TSS_ENTRY, (unsigned long)tss, sizeof(*tss) - 1, SYS_DESC_tss_avail); if ( IS_ENABLED(CONFIG_PV32) ) - _set_tssldt_desc(compat_gdt + TSS_ENTRY, (unsigned long)tss, - sizeof(*tss) - 1, SYS_DESC_tss_busy); + _set_tssldt_desc( + this_cpu(compat_gdt) - FIRST_RESERVED_GDT_ENTRY + TSS_ENTRY, + (unsigned long)tss, sizeof(*tss) - 1, SYS_DESC_tss_busy); per_cpu(full_gdt_loaded, cpu) = false; lgdt(&gdtr);
Clang 3.5 doesn't do enough dead-code-elimination to drop the compat_gdt reference, resulting in a linker failure: hidden symbol `per_cpu__compat_gdt' isn't defined Drop the local variable, and move evaluation of this_cpu(compat_gdt) to within the guarded region. Reported-by: Roger Pau Monné <roger.pau@citrix.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wl@xen.org> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/cpu/common.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)