Re: [PATCH] arch/x86/setup.c: Ignore early boot parameters like no-real-mode

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 12/08/2020 20:06, Trammell Hudson wrote:
> On Wednesday, August 12, 2020 8:16 PM, Andrew Cooper 
> <andrew.cooper3@xxxxxxxxxx> wrote:
>
>> However, the use of LINE creates problems for livepatch builds, as
>> it causes the binary diffing tools to believe these changed, based on a
>> change earlier in the file.
> Ah, I hadn't considered that.  Makes sense that the
> deterministic __COUNTER__ would be better for many uses.
> However...
>
> One concern is that the __COUNTER__ is per compilation unit,
> which I think would mean that every file would conflict by
> creating __setup_str_ign_0 for the first one, __setup_str_ign_1
> for the next, etc.  Unless they are static scoped or have a
> variable-name-friendly unique prefix, they aren't
> suitable for globals that share a namespace.

That's fine.  Something else which exists in our tangle of a build
system is some symbol munging (behind CONFIG_ENFORCE_UNIQUE_SYMBOLS)
which takes any static variable and prepends the relative filename, so
the end result is something which is properly unique.

In some copious free, this wants refining to "every ambiguous static
variable", seeing as most static variables aren't ambiguous, but that is
an incremental improvement.

>
> Another is that each evaluation increments it, so the macro
> would need some tricks to avoid double-evaluation since it
> both defines __setup_str_ign_XYZ and references it in the
> __kparam structure.  This is in contrast to __LINE__,
> which is constant in the macro and based on the line where
> it was invoked so the double evaluation is not a problem.
>
>> Instead of opencoding TEMP_NAME(), can we borrow Linux's __UNIQUE_ID()
>> infrastructure?  COUNTER appears to have existed for ages, and
>> exists in all of our supported compilers.
> I'm definitely in favor of borrowing it from Linux, although
> subject to those two caveats.
>
>> If you want, I can sort that out as a prereq patch, and rebase this one
>> on top?
> That sounds fine to me. They really are two separate patches.

I think we're fine caveat wise.  I'll try and find some time tomorrow.

~Andrew