On Fri, 12 May 2017 15:15:33 +0530 Hari Bathini <hbath...@linux.vnet.ibm.com> wrote:
> On Thursday 11 May 2017 06:46 PM, Michal Suchánek wrote: > > On Thu, 11 May 2017 02:00:11 +0530 > > Hari Bathini <hbath...@linux.vnet.ibm.com> wrote: > > > >> Hello Michal, > >> > >> On Wednesday 10 May 2017 09:31 PM, Michal Suchánek wrote: > >>> Hello, > >>> > >>> On Wed, 03 May 2017 23:52:52 +0530 > >>> Hari Bathini <hbath...@linux.vnet.ibm.com> wrote: > >>> > >>>> With the introduction of 'fadump_append=' parameter to pass > >>>> additional parameters to fadump (capture) kernel, update > >>>> documentation about it. > >>>> > >>>> Signed-off-by: Hari Bathini <hbath...@linux.vnet.ibm.com> > >>>> --- > >>>> > >>>> Changes from v4: > >>>> * Based on top of patchset that includes > >>>> > >>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=akpm&id=05f383cdfba8793240e73f9a9fbff4e25d66003f > >>>> > >>>> > >>>> Documentation/powerpc/firmware-assisted-dump.txt | 10 > >>>> +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/Documentation/powerpc/firmware-assisted-dump.txt > >>>> b/Documentation/powerpc/firmware-assisted-dump.txt index > >>>> 8394bc8..6327193 100644 --- > >>>> a/Documentation/powerpc/firmware-assisted-dump.txt +++ > >>>> b/Documentation/powerpc/firmware-assisted-dump.txt @@ -162,7 > >>>> +162,15 @@ How to enable firmware-assisted dump (fadump): > >>>> 1. Set config option CONFIG_FA_DUMP=y and build kernel. > >>>> 2. Boot into linux kernel with 'fadump=on' kernel cmdline > >>>> option. -3. Optionally, user can also set 'crashkernel=' kernel > >>>> cmdline +3. A user can pass additional command line parameters > >>>> as a comma > >>>> + separated list through 'fadump_append=' parameter, to be > >>>> enforced > >>>> + when fadump is active. For example, if parameters like > >>>> nr_cpus=1, > >>>> + numa=off & udev.children-max=2 are to be enforced when fadump > >>>> is > >>>> + active, > >>>> 'fadump_append=nr_cpus=1,numa=off,udev.children-max=2' > >>>> + can be passed in command line, which will be replaced with > >>>> + "nr_cpus=1 numa=off udev.children-max=2" when fadump is > >>>> active. > >>>> + This helps in reducing memory consumption during dump > >>>> capture. +4. Optionally, user can also set 'crashkernel=' kernel > >>>> cmdline to specify size of the memory to reserve for boot memory > >>>> dump preservation. > >>>> > >>>> > >>> Writing your own deficient parser for comma separated arguments > >>> when perfectly fine parser for space separated quoted arguments > >>> exists in the kernel and the bootloader does not seem like a good > >>> idea to me. > >> > >> Couple of things that prompted me for v5 are: > >> 1. Using parse_early_options() limits the kind of parameters > >> that can be passed to fadump capture kernel. Passing > >> parameters like systemd.unit= & udev.childern.max= has no effect > >> with v4. Updating > >> boot_command_line parameter, when fadump is active, seems a > >> better alternative. > >> > >> 2. Passing space-separated quoted arguments is not working > >> as intended with lilo. Updating bootloader with the below > >> entry in /etc/lilo.conf file results in a missing append > >> entry in /etc/yaboot.conf file. > >> > >> append = "quiet sysrq=1 insmod=sym53c8xx insmod=ipr > >> crashkernel=512M-:256M fadump_append=\"nr_cpus=1 numa=off > >> udev.children-max=2\"" > > Meaning that a script that emulates LILO semantics on top of yaboot > > which is completely capable of passing qouted space separated > > arguments fails. IMHO it is more reasonable to fix the script or > > whatever adaptation layer or use yaboot directly than working > > around bug in said script by introducing a new argument parser in > > the kernel. > > > > > > Hmmm.. while trying to implement space-separated parameter list with > quotes as syntax for fadump_append parameter, noticed that it can > make implemenation > more vulnerable. Here are some problems I am facing while > implementing this.. How so? presumably you can reuse parse_args even if you do not register with early_param and call it yourself. Then your parsing of fadump_append is as vulnerable as parsing any other kernel argument since it is using same parser. Thanks Michal
