mbox series

[0/8] add generic builtin command line

Message ID 1538067309-5711-1-git-send-email-maksym.kokhan@globallogic.com
Headers show
Series add generic builtin command line | expand

Message

Maksym Kokhan Sept. 27, 2018, 4:55 p.m. UTC
There were series of patches [1] for 4.3.0-rc3, that allowed
architectures to use a generic builtin command line. I have rebased
these patches on kernel 4.19.0-rc4.

Things, modified in comparison with original patches:                            
* There was some bug for mips, in the case when CONFIG_CMDLINE_PREPEND
and CONFIG_CMDLINE_APPEND are empty and CMDLINE_OVERRIDE is not set,
command line from bootloader was ignored, so I fixed it, modifying
patch "add generic builtin command line".

* Implemented new patch to resolve conflict with new kernel, which
modify EFI stub code. Unfortunately, I don't have capability to test
this modification on real arm board with EFI.

* Removed new realisation of mips builtin command line, which was
created after 4.3.0-rc3.

* Kernel 4.3.0-rc3 with original patches could not be compiled for
powerpc due to prom_init.c checking by prom_init_check.sh. So I added
strlcat (which is used by cmdline_add_builtin macro) to
prom_init_check.sh whitelist.

Patches have been tested in QEMU for x86, arm (little-endian), arm64
(little-endian), mips (little-endian, 32-bit) and powerpc
(big-endian, 64-bit), everything works perfectly. Also it was tested
on linux-next (next-20180924 tag) for all listed above architectures.

[1] : https://lore.kernel.org/patchwork/patch/604992/

Daniel Walker (7):
  add generic builtin command line
  drivers: of: ifdef out cmdline section
  x86: convert to generic builtin command line
  arm: convert to generic builtin command line
  arm64: convert to generic builtin command line
  mips: convert to generic builtin command line
  powerpc: convert to generic builtin command line

Maksym Kokhan (1):
  efi: modify EFI stub code for arm/arm64

 arch/arm/Kconfig                        | 38 +-----------------
 arch/arm/kernel/atags_parse.c           | 14 ++-----
 arch/arm/kernel/devtree.c               |  2 +
 arch/arm64/Kconfig                      | 17 +-------
 arch/arm64/kernel/setup.c               |  3 ++
 arch/mips/Kconfig                       | 24 +----------
 arch/mips/Kconfig.debug                 | 47 ----------------------
 arch/mips/kernel/setup.c                | 41 ++-----------------
 arch/powerpc/Kconfig                    | 23 +----------
 arch/powerpc/kernel/prom.c              |  4 ++
 arch/powerpc/kernel/prom_init.c         |  8 ++--
 arch/powerpc/kernel/prom_init_check.sh  |  2 +-
 arch/x86/Kconfig                        | 44 +--------------------
 arch/x86/kernel/setup.c                 | 19 ++-------
 drivers/firmware/efi/libstub/arm-stub.c | 10 ++---
 drivers/of/fdt.c                        |  2 +-
 include/linux/cmdline.h                 | 70 +++++++++++++++++++++++++++++++++
 init/Kconfig                            | 68 ++++++++++++++++++++++++++++++++
 18 files changed, 173 insertions(+), 263 deletions(-)
 create mode 100644 include/linux/cmdline.h

Comments

Ard Biesheuvel Sept. 27, 2018, 5:05 p.m. UTC | #1
On 27 September 2018 at 18:55, Maksym Kokhan
<maksym.kokhan@globallogic.com> wrote:
> There were series of patches [1] for 4.3.0-rc3, that allowed
> architectures to use a generic builtin command line. I have rebased
> these patches on kernel 4.19.0-rc4.
>

Could you please elaborate on the purpose of this series? Is it simply
to align between architectures? Does it solve an actual problem?

> Things, modified in comparison with original patches:
> * There was some bug for mips, in the case when CONFIG_CMDLINE_PREPEND
> and CONFIG_CMDLINE_APPEND are empty and CMDLINE_OVERRIDE is not set,
> command line from bootloader was ignored, so I fixed it, modifying
> patch "add generic builtin command line".
>
> * Implemented new patch to resolve conflict with new kernel, which
> modify EFI stub code. Unfortunately, I don't have capability to test
> this modification on real arm board with EFI.
>
> * Removed new realisation of mips builtin command line, which was
> created after 4.3.0-rc3.
>
> * Kernel 4.3.0-rc3 with original patches could not be compiled for
> powerpc due to prom_init.c checking by prom_init_check.sh. So I added
> strlcat (which is used by cmdline_add_builtin macro) to
> prom_init_check.sh whitelist.
>
> Patches have been tested in QEMU for x86, arm (little-endian), arm64
> (little-endian), mips (little-endian, 32-bit) and powerpc
> (big-endian, 64-bit), everything works perfectly. Also it was tested
> on linux-next (next-20180924 tag) for all listed above architectures.
>
> [1] : https://lore.kernel.org/patchwork/patch/604992/
>
> Daniel Walker (7):
>   add generic builtin command line
>   drivers: of: ifdef out cmdline section
>   x86: convert to generic builtin command line
>   arm: convert to generic builtin command line
>   arm64: convert to generic builtin command line
>   mips: convert to generic builtin command line
>   powerpc: convert to generic builtin command line
>
> Maksym Kokhan (1):
>   efi: modify EFI stub code for arm/arm64
>
>  arch/arm/Kconfig                        | 38 +-----------------
>  arch/arm/kernel/atags_parse.c           | 14 ++-----
>  arch/arm/kernel/devtree.c               |  2 +
>  arch/arm64/Kconfig                      | 17 +-------
>  arch/arm64/kernel/setup.c               |  3 ++
>  arch/mips/Kconfig                       | 24 +----------
>  arch/mips/Kconfig.debug                 | 47 ----------------------
>  arch/mips/kernel/setup.c                | 41 ++-----------------
>  arch/powerpc/Kconfig                    | 23 +----------
>  arch/powerpc/kernel/prom.c              |  4 ++
>  arch/powerpc/kernel/prom_init.c         |  8 ++--
>  arch/powerpc/kernel/prom_init_check.sh  |  2 +-
>  arch/x86/Kconfig                        | 44 +--------------------
>  arch/x86/kernel/setup.c                 | 19 ++-------
>  drivers/firmware/efi/libstub/arm-stub.c | 10 ++---
>  drivers/of/fdt.c                        |  2 +-
>  include/linux/cmdline.h                 | 70 +++++++++++++++++++++++++++++++++
>  init/Kconfig                            | 68 ++++++++++++++++++++++++++++++++
>  18 files changed, 173 insertions(+), 263 deletions(-)
>  create mode 100644 include/linux/cmdline.h
>
> --
> 2.7.4
>
Daniel Walker \(danielwa\) Sept. 27, 2018, 6:08 p.m. UTC | #2
On 09/27/2018 10:05 AM, Ard Biesheuvel wrote:
> On 27 September 2018 at 18:55, Maksym Kokhan
> <maksym.kokhan@globallogic.com> wrote:
>> There were series of patches [1] for 4.3.0-rc3, that allowed
>> architectures to use a generic builtin command line. I have rebased
>> these patches on kernel 4.19.0-rc4.
>>
> 
> Could you please elaborate on the purpose of this series? Is it simply
> to align between architectures? Does it solve an actual problem?

1) It removed a lot of code duplication between architecture

2) At Cisco we have issues where our bootloaders having default boot 
arguments. Some platforms we can't update the boot loader and it's 
helpful to be able to have boot arguments which are prepended to the 
bootloader arguments , and some parameters which are appended. These 
changes allow that.

Daniel
Daniel Walker Sept. 29, 2018, 6:17 p.m. UTC | #3
On Thu, Sep 27, 2018 at 07:55:08PM +0300, Maksym Kokhan wrote:
> Daniel Walker (7):
>   add generic builtin command line
>   drivers: of: ifdef out cmdline section
>   x86: convert to generic builtin command line
>   arm: convert to generic builtin command line
>   arm64: convert to generic builtin command line
>   mips: convert to generic builtin command line
>   powerpc: convert to generic builtin command line
> 

When I originally submitted these I had a very good conversion with Rob Herring
on the device tree changes. It seemed fairly clear that my approach in these
changes could be done better. It effected specifically arm64, but a lot of other
platforms use the device tree integrally. With arm64 you can reduce the changes
down to only Kconfig changes, and that would likely be the case for many of the
other architecture. I made patches to do this a while back, but have not had
time to test them and push them out.

In terms of mips I think there's a fair amount of work needed to pull out their
architecture specific mangling into something generic. Part of my motivation for
these was to take the architecture specific feature and open that up for all the
architecture. So it makes sense that the mips changes should become part of
that.

The only changes which have no comments are the generic changes, x86, and
powerpc. Those patches have been used at Cisco for years with no issues.
I added those changes into my -next tree for a round of testing. Assuming there
are no issues I can work out the merging with the architecture maintainers.
As for the other changes I think they can be done in time, as long as the
generic parts of upstream the rest can be worked on by any of the architecture
developers.

Daniel
Maksym Kokhan Oct. 8, 2018, 6:01 p.m. UTC | #4
Hi, Daniel

On Sat, Sep 29, 2018 at 9:17 PM <dwalker@fifo99.com> wrote:
>
> On Thu, Sep 27, 2018 at 07:55:08PM +0300, Maksym Kokhan wrote:
> > Daniel Walker (7):
> >   add generic builtin command line
> >   drivers: of: ifdef out cmdline section
> >   x86: convert to generic builtin command line
> >   arm: convert to generic builtin command line
> >   arm64: convert to generic builtin command line
> >   mips: convert to generic builtin command line
> >   powerpc: convert to generic builtin command line
> >
>
> When I originally submitted these I had a very good conversion with Rob Herring
> on the device tree changes. It seemed fairly clear that my approach in these
> changes could be done better. It effected specifically arm64, but a lot of other
> platforms use the device tree integrally. With arm64 you can reduce the changes
> down to only Kconfig changes, and that would likely be the case for many of the
> other architecture. I made patches to do this a while back, but have not had
> time to test them and push them out.

Can you please share this patches? I could test them and use to improve this
generic command line implementation.

> In terms of mips I think there's a fair amount of work needed to pull out their
> architecture specific mangling into something generic. Part of my motivation for
> these was to take the architecture specific feature and open that up for all the
> architecture. So it makes sense that the mips changes should become part of
> that.

This is really makes sense, and we have intentions to implement it
afterward. It would be easier to initially merge this simple
implementation and then develop it step by step.

> The only changes which have no comments are the generic changes, x86, and
> powerpc. Those patches have been used at Cisco for years with no issues.
> I added those changes into my -next tree for a round of testing. Assuming there
> are no issues I can work out the merging with the architecture maintainers.
> As for the other changes I think they can be done in time, as long as the
> generic parts of upstream the rest can be worked on by any of the architecture
> developers.

Thanks,
Maksym
Maksym Kokhan Oct. 23, 2018, 2:43 p.m. UTC | #5
On Mon, Oct 8, 2018 at 9:01 PM Maksym Kokhan
<maksym.kokhan@globallogic.com> wrote:
>
> Hi, Daniel
>
> On Sat, Sep 29, 2018 at 9:17 PM <dwalker@fifo99.com> wrote:
> >
> > On Thu, Sep 27, 2018 at 07:55:08PM +0300, Maksym Kokhan wrote:
> > > Daniel Walker (7):
> > >   add generic builtin command line
> > >   drivers: of: ifdef out cmdline section
> > >   x86: convert to generic builtin command line
> > >   arm: convert to generic builtin command line
> > >   arm64: convert to generic builtin command line
> > >   mips: convert to generic builtin command line
> > >   powerpc: convert to generic builtin command line
> > >
> >
> > When I originally submitted these I had a very good conversion with Rob Herring
> > on the device tree changes. It seemed fairly clear that my approach in these
> > changes could be done better. It effected specifically arm64, but a lot of other
> > platforms use the device tree integrally. With arm64 you can reduce the changes
> > down to only Kconfig changes, and that would likely be the case for many of the
> > other architecture. I made patches to do this a while back, but have not had
> > time to test them and push them out.
>
> Can you please share this patches? I could test them and use to improve this
> generic command line implementation.
>
> > In terms of mips I think there's a fair amount of work needed to pull out their
> > architecture specific mangling into something generic. Part of my motivation for
> > these was to take the architecture specific feature and open that up for all the
> > architecture. So it makes sense that the mips changes should become part of
> > that.
>
> This is really makes sense, and we have intentions to implement it
> afterward. It would be easier to initially merge this simple
> implementation and then develop it step by step.
>
> > The only changes which have no comments are the generic changes, x86, and
> > powerpc. Those patches have been used at Cisco for years with no issues.
> > I added those changes into my -next tree for a round of testing. Assuming there
> > are no issues I can work out the merging with the architecture maintainers.
> > As for the other changes I think they can be done in time, as long as the
> > generic parts of upstream the rest can be worked on by any of the architecture
> > developers.
>
> Thanks,
> Maksym

We still have no response to patches for x86, arm, arm64 and powerpc.
Is current generic command line implementation appropriate for these
architectures?
Is it possible to merge these patches in the current form (for x86,
arm, arm64 and powerpc)?

Thanks,
Maksym
Russell King - ARM Linux admin Oct. 23, 2018, 2:48 p.m. UTC | #6
On Tue, Oct 23, 2018 at 05:43:18PM +0300, Maksym Kokhan wrote:
> We still have no response to patches for x86, arm, arm64 and powerpc.
> Is current generic command line implementation appropriate for these
> architectures?
> Is it possible to merge these patches in the current form (for x86,
> arm, arm64 and powerpc)?

You may wish to consider your recipients - I seem to only have received
the cover and patch 1 (which doesn't include any ARM specific bits).
It may be that you're not getting responses because people haven't seen
your patches.

Thanks.
Maksym Kokhan Oct. 24, 2018, 8:57 a.m. UTC | #7
Do you mean, that you haven't seen patch for ARM, which I sent on
September 27 along with cover and patch 1? It is strange, because
you was the one from recipients. If so, you can see this patch here:
https://lore.kernel.org/patchwork/patch/992779/
On Tue, Oct 23, 2018 at 5:48 PM Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
>
> On Tue, Oct 23, 2018 at 05:43:18PM +0300, Maksym Kokhan wrote:
> > We still have no response to patches for x86, arm, arm64 and powerpc.
> > Is current generic command line implementation appropriate for these
> > architectures?
> > Is it possible to merge these patches in the current form (for x86,
> > arm, arm64 and powerpc)?
>
> You may wish to consider your recipients - I seem to only have received
> the cover and patch 1 (which doesn't include any ARM specific bits).
> It may be that you're not getting responses because people haven't seen
> your patches.
>
> Thanks.
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
Russell King - ARM Linux admin Oct. 24, 2018, 9:07 a.m. UTC | #8
On Wed, Oct 24, 2018 at 11:57:44AM +0300, Maksym Kokhan wrote:
> Do you mean, that you haven't seen patch for ARM, which I sent on
> September 27 along with cover and patch 1? It is strange, because
> you was the one from recipients. If so, you can see this patch here:
> https://lore.kernel.org/patchwork/patch/992779/

It seems that I have received patch 5, _but_ it's not threaded with
the cover message and patch 1.  With 50k messages in my inbox, and 3k
messages since you sent the series, it's virtually impossible to find
it (I only found it by looking at my mail server logs from September
to find the subject, and then searching my mailbox for that subject.)

This is unnecessarily difficult.

> On Tue, Oct 23, 2018 at 5:48 PM Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> >
> > On Tue, Oct 23, 2018 at 05:43:18PM +0300, Maksym Kokhan wrote:
> > > We still have no response to patches for x86, arm, arm64 and powerpc.
> > > Is current generic command line implementation appropriate for these
> > > architectures?
> > > Is it possible to merge these patches in the current form (for x86,
> > > arm, arm64 and powerpc)?
> >
> > You may wish to consider your recipients - I seem to only have received
> > the cover and patch 1 (which doesn't include any ARM specific bits).
> > It may be that you're not getting responses because people haven't seen
> > your patches.
> >
> > Thanks.
> >
> > --
> > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> > According to speedtest.net: 11.9Mbps down 500kbps up
Will Deacon Oct. 29, 2018, 10:29 a.m. UTC | #9
On Wed, Oct 24, 2018 at 10:07:32AM +0100, Russell King - ARM Linux wrote:
> On Wed, Oct 24, 2018 at 11:57:44AM +0300, Maksym Kokhan wrote:
> > Do you mean, that you haven't seen patch for ARM, which I sent on
> > September 27 along with cover and patch 1? It is strange, because
> > you was the one from recipients. If so, you can see this patch here:
> > https://lore.kernel.org/patchwork/patch/992779/
> 
> It seems that I have received patch 5, _but_ it's not threaded with
> the cover message and patch 1.  With 50k messages in my inbox, and 3k
> messages since you sent the series, it's virtually impossible to find
> it (I only found it by looking at my mail server logs from September
> to find the subject, and then searching my mailbox for that subject.)
> 
> This is unnecessarily difficult.

This comes up surprisingly often, and I think part of the issue is that
different maintainers have different preferences. I also prefer to receive
the entire series and cover-letter, but I've seen people object to being
CC'd on the whole series as well (how they manage to review things in
isolation is another question...!)

I wonder if we could have an entry in MAINTAINERS for this sort of
preference?

Maksym: in the short term, please just stick me and Russell on CC for the
entire thing.

Will
Russell King - ARM Linux admin Oct. 29, 2018, 10:44 a.m. UTC | #10
On Mon, Oct 29, 2018 at 10:29:15AM +0000, Will Deacon wrote:
> On Wed, Oct 24, 2018 at 10:07:32AM +0100, Russell King - ARM Linux wrote:
> > On Wed, Oct 24, 2018 at 11:57:44AM +0300, Maksym Kokhan wrote:
> > > Do you mean, that you haven't seen patch for ARM, which I sent on
> > > September 27 along with cover and patch 1? It is strange, because
> > > you was the one from recipients. If so, you can see this patch here:
> > > https://lore.kernel.org/patchwork/patch/992779/
> > 
> > It seems that I have received patch 5, _but_ it's not threaded with
> > the cover message and patch 1.  With 50k messages in my inbox, and 3k
> > messages since you sent the series, it's virtually impossible to find
> > it (I only found it by looking at my mail server logs from September
> > to find the subject, and then searching my mailbox for that subject.)
> > 
> > This is unnecessarily difficult.
> 
> This comes up surprisingly often, and I think part of the issue is that
> different maintainers have different preferences. I also prefer to receive
> the entire series and cover-letter, but I've seen people object to being
> CC'd on the whole series as well (how they manage to review things in
> isolation is another question...!)

This series has the odd situation where patch 1 is threaded to the
cover letter, but nothing else is - that makes it inconsistent.

Where I've seen people disagree with threading is when sending
follow-up series - whether that should be threaded to the previous
series or not - some people want it others hate it.

However, I haven't seen any disagreement is about having the patches
threaded to the cover.