| Message ID | 1456160247-15209-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 22.02.16 at 17:57, <andrew.cooper3@citrix.com> wrote: > --- a/xen/arch/x86/alternative.c > +++ b/xen/arch/x86/alternative.c > @@ -38,7 +38,7 @@ static const unsigned char k8nops[] __initconst = { > K8_NOP7, > K8_NOP8 > }; > -static const unsigned char * const k8_nops[ASM_NOP_MAX+1] = { > +static const unsigned char * const k8_nops[ASM_NOP_MAX+1] __initconst = { > NULL, > k8nops, > k8nops + 1, > @@ -62,7 +62,7 @@ static const unsigned char p6nops[] __initconst = { > P6_NOP7, > P6_NOP8 > }; > -static const unsigned char * const p6_nops[ASM_NOP_MAX+1] = { > +static const unsigned char * const p6_nops[ASM_NOP_MAX+1] __initconst = { > NULL, > p6nops, > p6nops + 1, Afaict this will cause the same section type conflict issue as did the command line parameter constification change that I needed to revert yesterday. I'm afraid there's no way around introducing a sibling to __initconst (e.g. __initrelro or __initconst_r) specifying another section name (e.g. .init.rodata.rel), which then needs to be used on data objects incurring relocations. And iirc that was also the reason why these annotation have got left off originally here. The issue is that with -fPIC these sections needing relocations need to be marked writable even if the objects are "const". Jan
On 23/02/16 09:47, Jan Beulich wrote: >>>> On 22.02.16 at 17:57, <andrew.cooper3@citrix.com> wrote: >> --- a/xen/arch/x86/alternative.c >> +++ b/xen/arch/x86/alternative.c >> @@ -38,7 +38,7 @@ static const unsigned char k8nops[] __initconst = { >> K8_NOP7, >> K8_NOP8 >> }; >> -static const unsigned char * const k8_nops[ASM_NOP_MAX+1] = { >> +static const unsigned char * const k8_nops[ASM_NOP_MAX+1] __initconst = { >> NULL, >> k8nops, >> k8nops + 1, >> @@ -62,7 +62,7 @@ static const unsigned char p6nops[] __initconst = { >> P6_NOP7, >> P6_NOP8 >> }; >> -static const unsigned char * const p6_nops[ASM_NOP_MAX+1] = { >> +static const unsigned char * const p6_nops[ASM_NOP_MAX+1] __initconst = { >> NULL, >> p6nops, >> p6nops + 1, > Afaict this will cause the same section type conflict issue as did > the command line parameter constification change that I needed > to revert yesterday. I'm afraid there's no way around introducing > a sibling to __initconst (e.g. __initrelro or __initconst_r) specifying > another section name (e.g. .init.rodata.rel), which then needs to > be used on data objects incurring relocations. And iirc that was > also the reason why these annotation have got left off originally > here. > > The issue is that with -fPIC these sections needing relocations > need to be marked writable even if the objects are "const". Surely we would be better seeing about fixing the build not to use -fPIC. Linux doesn't, so it is clearly possible. ~Andrew
>>> On 23.02.16 at 11:10, <andrew.cooper3@citrix.com> wrote: > On 23/02/16 09:47, Jan Beulich wrote: >>>>> On 22.02.16 at 17:57, <andrew.cooper3@citrix.com> wrote: >>> --- a/xen/arch/x86/alternative.c >>> +++ b/xen/arch/x86/alternative.c >>> @@ -38,7 +38,7 @@ static const unsigned char k8nops[] __initconst = { >>> K8_NOP7, >>> K8_NOP8 >>> }; >>> -static const unsigned char * const k8_nops[ASM_NOP_MAX+1] = { >>> +static const unsigned char * const k8_nops[ASM_NOP_MAX+1] __initconst = { >>> NULL, >>> k8nops, >>> k8nops + 1, >>> @@ -62,7 +62,7 @@ static const unsigned char p6nops[] __initconst = { >>> P6_NOP7, >>> P6_NOP8 >>> }; >>> -static const unsigned char * const p6_nops[ASM_NOP_MAX+1] = { >>> +static const unsigned char * const p6_nops[ASM_NOP_MAX+1] __initconst = { >>> NULL, >>> p6nops, >>> p6nops + 1, >> Afaict this will cause the same section type conflict issue as did >> the command line parameter constification change that I needed >> to revert yesterday. I'm afraid there's no way around introducing >> a sibling to __initconst (e.g. __initrelro or __initconst_r) specifying >> another section name (e.g. .init.rodata.rel), which then needs to >> be used on data objects incurring relocations. And iirc that was >> also the reason why these annotation have got left off originally >> here. >> >> The issue is that with -fPIC these sections needing relocations >> need to be marked writable even if the objects are "const". > > Surely we would be better seeing about fixing the build not to use > -fPIC. Linux doesn't, so it is clearly possible. Linux runs in the top 2Gb of address space, using -mcmodel=kernel. We can't do that. Jan
diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c index 46ac0fd..02b5e92 100644 --- a/xen/arch/x86/alternative.c +++ b/xen/arch/x86/alternative.c @@ -38,7 +38,7 @@ static const unsigned char k8nops[] __initconst = { K8_NOP7, K8_NOP8 }; -static const unsigned char * const k8_nops[ASM_NOP_MAX+1] = { +static const unsigned char * const k8_nops[ASM_NOP_MAX+1] __initconst = { NULL, k8nops, k8nops + 1, @@ -62,7 +62,7 @@ static const unsigned char p6nops[] __initconst = { P6_NOP7, P6_NOP8 }; -static const unsigned char * const p6_nops[ASM_NOP_MAX+1] = { +static const unsigned char * const p6_nops[ASM_NOP_MAX+1] __initconst = { NULL, p6nops, p6nops + 1, diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index 53c7452..e0fdf40 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -241,53 +241,33 @@ static void __init noreturn blexit(const CHAR16 *str) /* generic routine for printing error messages */ static void __init PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode) { + static const CHAR16* const ErrCodeToStr[] __initconst = { + [~EFI_ERROR_MASK & EFI_NOT_FOUND] = L"Not found", + [~EFI_ERROR_MASK & EFI_NO_MEDIA] = L"The device has no media", + [~EFI_ERROR_MASK & EFI_MEDIA_CHANGED] = L"Media changed", + [~EFI_ERROR_MASK & EFI_DEVICE_ERROR] = L"Device error", + [~EFI_ERROR_MASK & EFI_VOLUME_CORRUPTED] = L"Volume corrupted", + [~EFI_ERROR_MASK & EFI_ACCESS_DENIED] = L"Access denied", + [~EFI_ERROR_MASK & EFI_OUT_OF_RESOURCES] = L"Out of resources", + [~EFI_ERROR_MASK & EFI_VOLUME_FULL] = L"Volume is full", + [~EFI_ERROR_MASK & EFI_SECURITY_VIOLATION] = L"Security violation", + [~EFI_ERROR_MASK & EFI_CRC_ERROR] = L"CRC error", + [~EFI_ERROR_MASK & EFI_COMPROMISED_DATA] = L"Compromised data", + [~EFI_ERROR_MASK & EFI_BUFFER_TOO_SMALL] = L"Buffer too small", + }; + EFI_STATUS ErrIdx = ErrCode & ~EFI_ERROR_MASK; + StdOut = StdErr; PrintErr((CHAR16 *)mesg); PrintErr(L": "); - switch (ErrCode) + if( (ErrIdx < ARRAY_SIZE(ErrCodeToStr)) && ErrCodeToStr[ErrIdx] ) + mesg = ErrCodeToStr[ErrIdx]; + else { - case EFI_NOT_FOUND: - mesg = L"Not found"; - break; - case EFI_NO_MEDIA: - mesg = L"The device has no media"; - break; - case EFI_MEDIA_CHANGED: - mesg = L"Media changed"; - break; - case EFI_DEVICE_ERROR: - mesg = L"Device error"; - break; - case EFI_VOLUME_CORRUPTED: - mesg = L"Volume corrupted"; - break; - case EFI_ACCESS_DENIED: - mesg = L"Access denied"; - break; - case EFI_OUT_OF_RESOURCES: - mesg = L"Out of resources"; - break; - case EFI_VOLUME_FULL: - mesg = L"Volume is full"; - break; - case EFI_SECURITY_VIOLATION: - mesg = L"Security violation"; - break; - case EFI_CRC_ERROR: - mesg = L"CRC error"; - break; - case EFI_COMPROMISED_DATA: - mesg = L"Compromised data"; - break; - case EFI_BUFFER_TOO_SMALL: - mesg = L"Buffer too small"; - break; - default: PrintErr(L"ErrCode: "); DisplayUint(ErrCode, 0); mesg = NULL; - break; } blexit(mesg); }
Clang-3.8 generates several .data.rel.ro sections when compiling Xen. As these contain no global symbols, they should be .data.rel.ro.local. This breaks the SPECIAL_DATA_SECTIONS check when converting the transition units to being init-only. For alternatives.c, explicitly move the nops arrays into __initconst. For efi boot.c, manually create the optimisation performed by Clang by collapsing the switch statement into a lookup table. The double use of const is required to avoid breaking the ARM build by creating a section type conflict with fdt_guid. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> v2: Fix ARM build --- xen/arch/x86/alternative.c | 4 ++-- xen/common/efi/boot.c | 58 +++++++++++++++------------------------------- 2 files changed, 21 insertions(+), 41 deletions(-)