Re: [PATCH 59.5/65] x86: Introduce helpers/checks for endbr64 instructions

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.12.2021 15:10, Andrew Cooper wrote:
> On 03/12/2021 13:59, Jan Beulich wrote:
>> On 26.11.2021 17:33, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/Makefile
>>> +++ b/xen/arch/x86/Makefile
>>> @@ -190,6 +190,10 @@ $(TARGET)-syms: prelink.o xen.lds
>>>     $(MAKE) -f $(BASEDIR)/Rules.mk efi-y= $(@D)/.$(@F).1.o
>>>     $(LD) $(XEN_LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
>>>         $(@D)/.$(@F).1.o -o $@
>>> +ifeq ($(CONFIG_XEN_IBT),y)
>>> +   $(OBJDUMP) -d $@ | grep 0xfa1e0ff3 >/dev/null && \
>>> +           { echo "Found embedded endbr64 instructions" >&2; false; } || :
>> I guess I'm confused: The "false;" suggests to me you want to make the
>> build fail in such a case. The "|| :" otoh suggests you want to silence
>> errors (and not just the one from grep when not finding the pattern
>> aiui).
> 
> The exit code of grep needs inverting for the build to proceed
> correctly.  Without || :, all builds fail when they've not got the pattern.

But doesn't this invert not only failure of grep, but also the
unconditional "failure" of "false"? IOW doesn't this step therefore
always succeed, making the message merely a warning-like one? Or if
that's the intended behavior, what's the "false" good for?

>> Also isn't passing -q to grep standard enough (and shorter) to use in
>> place of redirecting its output to /dev/null?
> 
> That caused problems on the BSDs.  c/s e632d56f0f5 went through several
> iterations before settling on this pattern.

Odd. The commit message doesn't mention any of this. v2 of that patch
already didn't use -q, and also doesn't identify a respective change
from v1. I wasn't able to locate v1.

Jan