Re: [PATCH 02/14] lmb: Use CONFIG_LMB_*_REGIONS only if they are defined

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 4071 bytes --]

On Sat, Sep 04, 2021 at 09:37:39PM +0200, Marek Vasut wrote:
> On 9/4/21 7:01 PM, Tom Rini wrote:
> > [trimming the CC list]
> > 
> > On Sat, Sep 04, 2021 at 06:49:03PM +0200, Marek Vasut wrote:
> > > On 9/4/21 6:09 PM, Tom Rini wrote:
> > > > On Sat, Sep 04, 2021 at 06:05:50PM +0200, Marek Vasut wrote:
> > > > > On 9/4/21 5:17 PM, Tom Rini wrote:
> > > > > > On Sat, Sep 04, 2021 at 05:15:45PM +0200, Marek Vasut wrote:
> > > > > > > On 9/4/21 4:10 PM, Tom Rini wrote:
> > > > > > > [...]
> > > > > > > 
> > > > > > > > > > > > At this point, I think you should rework things to stop making
> > > > > > > > > > > > CONFIG_LMB be optional, it should be a def_bool y.
> > > > > > > > > > > 
> > > > > > > > > > > I disagree, see above.
> > > > > > > > > > 
> > > > > > > > > > The only reason "tools-only_defconfig" builds a useless u-boot binary
> > > > > > > > > > today is in CI where it would be more work than it's worth to make CI
> > > > > > > > > > exclude that from the build list.  But if you want to just do that
> > > > > > > > > > instead, I'll also accept adding -x tools-only to the azure/gitlab jobs
> > > > > > > > > > that build all other architectures, as tools-only is tested in its own
> > > > > > > > > > build job, for it's only valid build target.
> > > > > > > > > 
> > > > > > > > > The tools-only build is also used elsewhere, to build just that, tools.
> > > > > > > > 
> > > > > > > > I've repeatedly explained myself and what I'm looking for in v2 of this
> > > > > > > > series.  I will summarize one last time.  The "tools-only_defconfig" is
> > > > > > > > for tools, only.  Building anything other than the "tools-only" target
> > > > > > > > isn't useful.  In U-Boot itself, LMB is required as that is how we
> > > > > > > > prevent a number of CVEs from being trivial to exploit.  v2 of this
> > > > > > > > series needs to drop patches 1 and 2 of v1 of this series.  It can
> > > > > > > > further do any of:
> > > > > > > > 1. Nothing else.
> > > > > > > > 2. Add tools-only to the exclude list in the "build everything else" CI
> > > > > > > >        job.
> > > > > > > > 3. Make CONFIG_LMB be def_bool y.
> > > > > > > 
> > > > > > > If tools-only is for tools, only, then why should it enable LMB ?
> > > > > > > The tools are userspace tools, they do not need LMB, and so LMB can be
> > > > > > > disabled.
> > > > > > > 
> > > > > > > This is the part which is unclear to me.
> > > > > > 
> > > > > > I don't know why it's unclear to you at this point, sorry.
> > > > > 
> > > > > Well why exactly does a userspace program require LMB enabled ?
> > > > > What does LMB protect in there ? obviously not U-Boot.
> > > > 
> > > > I feel like you've lost the thread.
> > > 
> > > Can you please answer my questions above ?
> > 
> > I have.
> 
> This attitude is not helpful. Please answer my questions, if necessary
> please reiterate, otherwise this discussion cannot be resolved and will only
> lead to frustration.

One last time then.  The only reason tools-only_defconfig is ever built
for a target other than "tools-only" is because CI does not exclude it
from the world build stage.  You can fix this by doing option #2 still
quoted above.

The only CONFIG options that are at all valid for "tools-only" and so
the host tools related, are LOCALVERSION (which is why there's a
tools-only defconfig at all) and now TOOLS_LIBCRYPTO.  Nothing else at
all should matter as the tools should always be the same.  So your point
about "what does userspace need LMB for" is irrelevant.  The host tools
should need NO option be enabled/disabled.

Further, "disabling FOO breaks the build" means we need to investigate
what the correct resolution is.  In this case, LMB needs to be def_bool
y.  This is option #3 above.  Why does u-boot-as-sandbox need LMB?
Because that's how we ensure that the tests that check for overlap fail
as expected.

Finally, you can just drop the first two patches and call me too
stubborn.  This is option #1 above.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]