New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 868003 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

ChromeOS: parent-child overlays cannot add kernel command line args independently

Project Member Reported by rajatja@google.com, Jul 26

Issue description

Currently:

src/scripts/build_kernel_image.sh:
    ....
    load_board_specific_script "build_kernel_image.sh"
         [sources "build_kernel_image.sh" from all the overlays
          each of which may have its own modify_kernel_command_line()]
    ....
    modify_kernel_command_line 

This was perhaps designed to have a board specific modify_kernel_command_line() override the default one. But now that we have chipset-* and baseboard-* overlays, each may want to mangle kernel command line parameters independently. E.g. for icelake:

* We want to have chipset-icl specify i915.alpha_support=1 (to indicate that the graphics chipset needs alpha support).

* baseboard-dragonegg to specify some other board specific args.

But what I see is that the baseboard or board specific version will hide the chipset version, essentially removing that command line instead of adding to what has already been added.

May be we should have a way to call a couple of different functions for chipset, baseboard kernel command line mangling functions.

Until then, we can just remember to include all the kernel command line changes in one place (i.e. in 1 build_kernel_image.sh)
 
Cc: sjg@chromium.org
modify_kernel_command_line has probably metastasized beyond its original intentions and could do with a redesign

Simon: you think it's appropriate to add this to the unibuild config ?
Summary: ChromeOS: parent-child overlays cannot add kernel command line args independently (was: ChromeOS: Allow kernel command line mangling from different overlays)
Yes that sounds like a good idea to me.
Components: -Infra>Client>ChromeOS Infra>Client>ChromeOS>Build
Going to move this out of Infra>Client>ChromeOS so it's not sitting in the test infra deputy's triage queue.
we'll only assume accumulation of settings ?  or should we support a syntax that allows for removal of options ?  i don't think chromeos-config has that concept currently ?
No it doesn't have that concept. I can imagine it getting a bit out of hand since the ordering of overlays would affect it. Do we need that?

I suppose one option (if we do) is:

kernel-params:
   add:
     - param1 param2
     - param3
   remove
     - bad-param
     - starting-with*

but it would be nice to start simple if we can.
i'm OK if we just start with concatenating everything together.  we're going to need a rule anyways to guarantee stability in output ... it'd be bad if we'd end up randomly with <common args><baseboard><chipset> or <common args><chipset><baseboard>.  it'd just be introducing a level of unnecessary confusion.

so we only have kernel-params::add initially
Cc: gmeinke@chromium.org
SGTM
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/7c30bca5b0be5c60824c8b4b957c18e056923fae

commit 7c30bca5b0be5c60824c8b4b957c18e056923fae
Author: Rajat Jain <rajatja@google.com>
Date: Fri Jul 27 06:59:50 2018

Dragonegg: Move ramoops.ecc arg to chipset overlay

Ref to the description of this bug for details:
https://bugs.chromium.org/p/chromium/issues/detail?id=868003

I found that all kernel parameter changes need to be specified in
1 overlay, i.e. cannot be split across multiple overlays. I chose
the chipset overlay to house all parameters for dragon egg because
there is no board specific parameter (so far). In future, if there is
a board specific parameter, we'll have to move all the parameters to
board specific overlay.

BUG=chromium:868003
TEST=check that the kernel arguments are present

Change-Id: Ib11abbc03c0ef23e7782d18f8b3644750d77e081
Reviewed-on: https://chromium-review.googlesource.com/1151900
Commit-Ready: Rajat Jain <rajatja@chromium.org>
Tested-by: Rajat Jain <rajatja@chromium.org>
Reviewed-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-by: Rajat Jain <rajatja@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/7c30bca5b0be5c60824c8b4b957c18e056923fae/chipset-icl/scripts/build_kernel_image.sh
[delete] https://crrev.com/e41398666d4c15fb9268e5d3f54409eeeefd2dac/baseboard-dragonegg/scripts/build_kernel_image.sh

Status: Available (was: Untriaged)

Sign in to add a comment