* Re: [PATCH v4 2/6] printk: honor the max_reason field in kmsg_dumper
From: Kees Cook @ 2020-05-22 17:34 UTC (permalink / raw)
To: Petr Mladek
Cc: devicetree, Tony Luck, Pavel Tatashin, Jonathan Corbet,
Anton Vorontsov, linux-doc, linux-kernel, Steven Rostedt,
Sergey Senozhatsky, Rob Herring, Paul Mackerras, Colin Cross,
Enric Balletbo i Serra, linuxppc-dev, Benson Leung
In-Reply-To: <20200522165120.GL3464@linux-b0ei>
On Fri, May 22, 2020 at 06:51:20PM +0200, Petr Mladek wrote:
> On Fri 2020-05-15 11:44:30, Kees Cook wrote:
> > From: Pavel Tatashin <pasha.tatashin@soleen.com>
> >
> > kmsg_dump() allows to dump kmesg buffer for various system events: oops,
> > panic, reboot, etc. It provides an interface to register a callback call
> > for clients, and in that callback interface there is a field "max_reason"
> > which gets ignored unless always_kmsg_dump is passed as kernel parameter.
>
> Strictly speaking, this is not fully true. "max_reason" field is not
> ignored when set to KMSG_DUMP_PANIC even when always_kmsg_dump was not set.
>
> It should be something like:
>
> "which gets ignored for reason higher than KMSG_DUMP_OOPS unless
> always_kmsg_dump is passed as kernel parameter".
>
> Heh, I wonder if anyone will be able to parse this ;-)
Ah yeah, good point. I've reworded things like this:
kmsg_dump() allows to dump kmesg buffer for various system events: oops,
panic, reboot, etc. It provides an interface to register a callback
call for clients, and in that callback interface there is a field
"max_reason", but it was getting ignored when set to any "reason"
higher than KMSG_DUMP_OOPS unless "always_kmsg_dump" was passed as
kernel parameter.
Allow clients to actually control their "max_reason", and keep the
current behavior when "max_reason" is not set.
> Otherwise, it looks good to me. With the updated commit message:
>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
Thanks!
--
Kees Cook
^ permalink raw reply
* Re: [PATCH] kbuild: reuse vmlinux.o in vmlinux_link
From: Masahiro Yamada @ 2020-05-22 17:44 UTC (permalink / raw)
To: Sami Tolvanen, Michael Ellerman, linuxppc-dev
Cc: Linux Kernel Mailing List, Michal Marek, Kees Cook,
Linux Kbuild mailing list
In-Reply-To: <CAK7LNARq3g5vA6vy9449SHsKQmbwJrQDSBz4ZbH1pBEvPmusuA@mail.gmail.com>
+ Michael, and PPC ML.
They may know something about the reason of failure.
On Sat, May 23, 2020 at 2:41 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, May 22, 2020 at 5:27 AM Sami Tolvanen <samitolvanen@google.com> wrote:
> >
> > Instead of linking all compilation units again each time vmlinux_link is
> > called, reuse vmlinux.o from modpost_link.
> >
> > With x86_64 allyesconfig, vmlinux_link is called three times and reusing
> > vmlinux.o reduces the build time ~38 seconds on my system (59% reduction
> > in the time spent in vmlinux_link).
> >
> > Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> > ---
> > scripts/link-vmlinux.sh | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index d09ab4afbda4..c6cc4305950c 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -77,11 +77,8 @@ vmlinux_link()
> >
> > if [ "${SRCARCH}" != "um" ]; then
> > objects="--whole-archive \
> > - ${KBUILD_VMLINUX_OBJS} \
> > + vmlinux.o \
> > --no-whole-archive \
> > - --start-group \
> > - ${KBUILD_VMLINUX_LIBS} \
> > - --end-group \
> > ${@}"
> >
> > ${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux} \
> >
> > base-commit: b85051e755b0e9d6dd8f17ef1da083851b83287d
> > --
> > 2.27.0.rc0.183.gde8f92d652-goog
> >
>
>
> I like this patch irrespective of CLANG_LTO, but
> unfortunately, my build test failed.
>
>
> ARCH=powerpc failed to build as follows:
>
>
>
> MODPOST vmlinux.o
> MODINFO modules.builtin.modinfo
> GEN modules.builtin
> LD .tmp_vmlinux.kallsyms1
> vmlinux.o:(__ftr_alt_97+0x20): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+4b1c
> vmlinux.o:(__ftr_alt_97+0x164): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+1cf78
> vmlinux.o:(__ftr_alt_97+0x288): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+1dac4
> vmlinux.o:(__ftr_alt_97+0x2f0): relocation truncated to fit:
> R_PPC64_REL14 against `.text'+1e254
> make: *** [Makefile:1125: vmlinux] Error 1
>
>
>
> I used powerpc-linux-gcc
> available at
> https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/9.2.0/
>
>
> Build command:
>
> make -j24 ARCH=powerpc CROSS_COMPILE=powerpc-linux- defconfig all
>
>
> Could you check it please?
>
>
>
> I will apply it to my test branch.
> Perhaps, 0-day bot may find more failure cases.
>
>
> --
> Best Regards
> Masahiro Yamada
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [v2 1/2] dts: ppc: t4240rdb: remove interrupts property
From: Li Yang @ 2020-05-22 19:20 UTC (permalink / raw)
To: Biwen Li
Cc: linux-rtc, a.zummo, Alexandre Belloni,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Biwen Li, lkml, Rob Herring, linuxppc-dev
In-Reply-To: <20200520091543.44692-1-biwen.li@oss.nxp.com>
On Wed, May 20, 2020 at 4:21 AM Biwen Li <biwen.li@oss.nxp.com> wrote:
>
> From: Biwen Li <biwen.li@nxp.com>
>
> This removes interrupts property to drop warning as follows:
> - $ hwclock.util-linux
> hwclock.util-linux: select() to /dev/rtc0
> to wait for clock tick timed out
>
> My case:
> - RTC ds1374's INT pin is connected to VCC on T4240RDB,
> then the RTC cannot inform cpu about the alarm interrupt
The commit message need a little bit improvement. Something like:
Since the interrupt pin for RTC DS1374 is not connected to the CPU on
T4240RDB, remove
the interrupt property from the device tree.
This also fix the following warning for hwclock.util-linux:
$ hwclock.util-linux
hwclock.util-linux: select() to /dev/rtc0
to wait for clock tick timed out
>
> Signed-off-by: Biwen Li <biwen.li@nxp.com>
> ---
> arch/powerpc/boot/dts/fsl/t4240rdb.dts | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t4240rdb.dts b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> index a56a705d41f7..145896f2eef6 100644
> --- a/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> @@ -144,7 +144,6 @@
> rtc@68 {
> compatible = "dallas,ds1374";
> reg = <0x68>;
> - interrupts = <0x1 0x1 0 0>;
> };
> };
>
> --
> 2.17.1
>
^ permalink raw reply
* Re: [v2 2/2] dts: ppc: t1024rdb: remove interrupts property
From: Li Yang @ 2020-05-22 19:22 UTC (permalink / raw)
To: Biwen Li
Cc: linux-rtc, a.zummo, Alexandre Belloni,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Biwen Li, lkml, Rob Herring, linuxppc-dev
In-Reply-To: <20200520091543.44692-2-biwen.li@oss.nxp.com>
On Wed, May 20, 2020 at 4:21 AM Biwen Li <biwen.li@oss.nxp.com> wrote:
>
> From: Biwen Li <biwen.li@nxp.com>
>
> This removes interrupts property to drop warning as follows:
> - $ hwclock.util-linux
> hwclock.util-linux: select() to /dev/rtc0
> to wait for clock tick timed out
>
> My case:
> - RTC ds1339s INT pin isn't connected to cpus INT pin on T1024RDB,
> then the RTC cannot inform cpu about alarm interrupt
>
> How to fix it?
> - remove IRQ line
This style is not the recommended style for commit message. Please
see my comment for the other patch.
>
> Signed-off-by: Biwen Li <biwen.li@nxp.com>
> ---
> arch/powerpc/boot/dts/fsl/t1024rdb.dts | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t1024rdb.dts b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> index 645caff98ed1..605ceec66af3 100644
> --- a/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> @@ -161,7 +161,6 @@
> rtc@68 {
> compatible = "dallas,ds1339";
> reg = <0x68>;
> - interrupts = <0x1 0x1 0 0>;
> };
> };
>
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v3 0/2] PCI/ERR: Allow Native AER/DPC using _OSC
From: Bjorn Helgaas @ 2020-05-22 19:46 UTC (permalink / raw)
To: Derrick, Jonathan
Cc: sathyanarayanan.kuppuswamy@linux.intel.com, Patel, Mayurkumar,
fred@fredlawl.com, sbobroff@linux.ibm.com,
linuxppc-dev@lists.ozlabs.org, Wysocki, Rafael J,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
andriy.shevchenko@linux.intel.com, bhelgaas@google.com,
alex.williamson@redhat.com, oohall@gmail.com, olof@lixom.net,
rajatja@google.com, mika.westerberg@linux.intel.com
In-Reply-To: <bbd1e71fc069389b96351490881fe43c4a5cb25f.camel@intel.com>
On Fri, May 22, 2020 at 05:23:31PM +0000, Derrick, Jonathan wrote:
> On Fri, 2020-05-01 at 11:35 -0600, Jonathan Derrick wrote:
> > On Fri, 2020-05-01 at 12:16 -0500, Bjorn Helgaas wrote:
> > > On Thu, Apr 30, 2020 at 12:46:07PM -0600, Jon Derrick wrote:
> > > > Hi Bjorn & Kuppuswamy,
> > > >
> > > > I see a problem in the DPC ECN [1] to _OSC in that it doesn't
> > > > give us a way to determine if firmware supports _OSC DPC
> > > > negotation, and therefore how to handle DPC.
> > > >
> > > > Here is the wording of the ECN that implies that Firmware
> > > > without _OSC DPC negotiation support should have the OSPM rely
> > > > on _OSC AER negotiation when determining DPC control:
> > > >
> > > > PCIe Base Specification suggests that Downstream Port
> > > > Containment may be controlled either by the Firmware or the
> > > > Operating System. It also suggests that the Firmware retain
> > > > ownership of Downstream Port Containment if it also owns
> > > > AER. When the Firmware owns Downstream Port Containment, it
> > > > is expected to use the new "Error Disconnect Recover"
> > > > notification to alert OSPM of a Downstream Port Containment
> > > > event.
> > > >
> > > > In legacy platforms, as bits in _OSC are reserved prior to
> > > > implementation, ACPI Root Bus enumeration will mark these Host
> > > > Bridges as without Native DPC support, even though the
> > > > specification implies it's expected that AER _OSC negotiation
> > > > determines DPC control for these platforms. There seems to be
> > > > a need for a way to determine if the DPC control bit in _OSC
> > > > is supported and fallback on AER otherwise.
> > > >
> > > >
> > > > Currently portdrv assumes DPC control if the port has Native
> > > > AER services:
> > > >
> > > > static int get_port_device_capability(struct pci_dev *dev)
> > > > ...
> > > > if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
> > > > pci_aer_available() &&
> > > > (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
> > > > services |= PCIE_PORT_SERVICE_DPC;
> > > >
> > > > Newer firmware may not grant OSPM DPC control, if for
> > > > instance, it expects to use Error Disconnect Recovery. However
> > > > it looks like ACPI will use DPC services via the EDR driver,
> > > > without binding the full DPC port service driver.
> > > >
> > > >
> > > > If we change portdrv to probe based on host->native_dpc and
> > > > not AER, then we break instances with legacy firmware where
> > > > OSPM will clear host->native_dpc solely due to _OSC bits being
> > > > reserved:
> > > >
> > > > struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
> > > > ...
> > > > if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
> > > > host_bridge->native_dpc = 0;
> > > >
> > > >
> > > >
> > > > So my assumption instead is that host->native_dpc can be 0 and
> > > > expect Native DPC services if AER is used. In other words, if
> > > > and only if DPC probe is invoked from portdrv, then it needs
> > > > to rely on the AER dependency. Otherwise it should be assumed
> > > > that ACPI set up DPC via EDR. This covers legacy firmware.
> > > >
> > > > However it seems like that could be trouble with newer
> > > > firmware that might give OSPM control of AER but not DPC, and
> > > > would result in both Native DPC and EDR being in effect.
> > > >
> > > >
> > > > Anyways here are two patches that give control of AER and DPC
> > > > on the results of _OSC. They don't mess with the HEST parser
> > > > as I expect those to be removed at some point. I need these
> > > > for VMD support which doesn't even rely on _OSC, but I suspect
> > > > this won't be the last effort as we detangle Firmware First.
> > > >
> > > > [1] https://members.pcisig.com/wg/PCI-SIG/document/12888
> > >
> > > Hi Jon, I think we need to sort out the _OSC/FIRMWARE_FIRST patches
> > > from Alex and Sathy first, then see what needs to be done on top of
> > > those, so I'm going to push these off for a few days and they'll
> > > probably need a refresh.
> > >
> > > Bjorn
> >
> > Agreed, no need to merge now. Just wanted to bring up the DPC
> > ambiguity, which I think was first addressed by dpc-native:
> >
> > commit 35a0b2378c199d4f26e458b2ca38ea56aaf2d9b8
> > Author: Olof Johansson <olof@lixom.net>
> > Date: Wed Oct 23 12:22:05 2019 -0700
> >
> > PCI/DPC: Add "pcie_ports=dpc-native" to allow DPC without AER control
> >
> > Prior to eed85ff4c0da7 ("PCI/DPC: Enable DPC only if AER is available"),
> > Linux handled DPC events regardless of whether firmware had granted it
> > ownership of AER or DPC, e.g., via _OSC.
> >
> > PCIe r5.0, sec 6.2.10, recommends that the OS link control of DPC to
> > control of AER, so after eed85ff4c0da7, Linux handles DPC events only if it
> > has control of AER.
> >
> > On platforms that do not grant OS control of AER via _OSC, Linux DPC
> > handling worked before eed85ff4c0da7 but not after.
> >
> > To make Linux DPC handling work on those platforms the same way they did
> > before, add a "pcie_ports=dpc-native" kernel parameter that makes Linux
> > handle DPC events regardless of whether it has control of AER.
> >
> > [bhelgaas: commit log, move pcie_ports_dpc_native to drivers/pci/]
> > Link: https://lore.kernel.org/r/20191023192205.97024-1-olof@lixom.net
> > Signed-off-by: Olof Johansson <olof@lixom.net>
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
> Are you still thinking about removing the HEST parser?
>
> For VMD we still need the ability to bind DPC if native_dpc==1.
> I think if we can do that, this set should still pretty much still
> apply with a modification to patch 2 to allow matching
> pcie_ports_dpc_native in dpc_probe.
Yes, I think we should remove the HEST firmware-first parsing, because
IIRC the spec really doesn't specify any action the OS should take
based on it. I was thinking Sathy might update the patch, and it fell
off my radar.
Bjorn
^ permalink raw reply
* Re: [PATCH v3 0/2] PCI/ERR: Allow Native AER/DPC using _OSC
From: Kuppuswamy, Sathyanarayanan @ 2020-05-22 20:48 UTC (permalink / raw)
To: Bjorn Helgaas, Derrick, Jonathan
Cc: Patel, Mayurkumar, fred@fredlawl.com, sbobroff@linux.ibm.com,
linuxppc-dev@lists.ozlabs.org, Wysocki, Rafael J,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
andriy.shevchenko@linux.intel.com, bhelgaas@google.com,
alex.williamson@redhat.com, oohall@gmail.com, olof@lixom.net,
rajatja@google.com, mika.westerberg@linux.intel.com
In-Reply-To: <20200522194616.GA11359@bjorn-Precision-5520>
Hi Bjorn, Derrick,
On 5/22/20 12:46 PM, Bjorn Helgaas wrote:
> On Fri, May 22, 2020 at 05:23:31PM +0000, Derrick, Jonathan wrote:
>> On Fri, 2020-05-01 at 11:35 -0600, Jonathan Derrick wrote:
>>> On Fri, 2020-05-01 at 12:16 -0500, Bjorn Helgaas wrote:
>>>> On Thu, Apr 30, 2020 at 12:46:07PM -0600, Jon Derrick wrote:
>>>>> Hi Bjorn & Kuppuswamy,
>>>>>
>>>>> I see a problem in the DPC ECN [1] to _OSC in that it doesn't
>>>>> give us a way to determine if firmware supports _OSC DPC
>>>>> negotation, and therefore how to handle DPC.
>>>>>
>>>>> Here is the wording of the ECN that implies that Firmware
>>>>> without _OSC DPC negotiation support should have the OSPM rely
>>>>> on _OSC AER negotiation when determining DPC control:
>>>>>
>>>>> PCIe Base Specification suggests that Downstream Port
>>>>> Containment may be controlled either by the Firmware or the
>>>>> Operating System. It also suggests that the Firmware retain
>>>>> ownership of Downstream Port Containment if it also owns
>>>>> AER. When the Firmware owns Downstream Port Containment, it
>>>>> is expected to use the new "Error Disconnect Recover"
>>>>> notification to alert OSPM of a Downstream Port Containment
>>>>> event.
>>>>>
>>>>> In legacy platforms, as bits in _OSC are reserved prior to
>>>>> implementation, ACPI Root Bus enumeration will mark these Host
>>>>> Bridges as without Native DPC support, even though the
>>>>> specification implies it's expected that AER _OSC negotiation
>>>>> determines DPC control for these platforms. There seems to be
>>>>> a need for a way to determine if the DPC control bit in _OSC
>>>>> is supported and fallback on AER otherwise.
>>>>>
>>>>>
>>>>> Currently portdrv assumes DPC control if the port has Native
>>>>> AER services:
>>>>>
>>>>> static int get_port_device_capability(struct pci_dev *dev)
>>>>> ...
>>>>> if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC) &&
>>>>> pci_aer_available() &&
>>>>> (pcie_ports_dpc_native || (services & PCIE_PORT_SERVICE_AER)))
>>>>> services |= PCIE_PORT_SERVICE_DPC;
>>>>>
>>>>> Newer firmware may not grant OSPM DPC control, if for
>>>>> instance, it expects to use Error Disconnect Recovery. However
>>>>> it looks like ACPI will use DPC services via the EDR driver,
>>>>> without binding the full DPC port service driver.
>>>>>
>>>>>
>>>>> If we change portdrv to probe based on host->native_dpc and
>>>>> not AER, then we break instances with legacy firmware where
>>>>> OSPM will clear host->native_dpc solely due to _OSC bits being
>>>>> reserved:
>>>>>
>>>>> struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>>>>> ...
>>>>> if (!(root->osc_control_set & OSC_PCI_EXPRESS_DPC_CONTROL))
>>>>> host_bridge->native_dpc = 0;
>>>>>
>>>>>
>>>>>
>>>>> So my assumption instead is that host->native_dpc can be 0 and
>>>>> expect Native DPC services if AER is used. In other words, if
>>>>> and only if DPC probe is invoked from portdrv, then it needs
>>>>> to rely on the AER dependency. Otherwise it should be assumed
>>>>> that ACPI set up DPC via EDR. This covers legacy firmware.
>>>>>
>>>>> However it seems like that could be trouble with newer
>>>>> firmware that might give OSPM control of AER but not DPC, and
>>>>> would result in both Native DPC and EDR being in effect.
>>>>>
>>>>>
>>>>> Anyways here are two patches that give control of AER and DPC
>>>>> on the results of _OSC. They don't mess with the HEST parser
>>>>> as I expect those to be removed at some point. I need these
>>>>> for VMD support which doesn't even rely on _OSC, but I suspect
>>>>> this won't be the last effort as we detangle Firmware First.
>>>>>
>>>>> [1] https://members.pcisig.com/wg/PCI-SIG/document/12888
>>>>
>>>> Hi Jon, I think we need to sort out the _OSC/FIRMWARE_FIRST patches
>>>> from Alex and Sathy first, then see what needs to be done on top of
>>>> those, so I'm going to push these off for a few days and they'll
>>>> probably need a refresh.
>>>>
>>>> Bjorn
>>>
>>> Agreed, no need to merge now. Just wanted to bring up the DPC
>>> ambiguity, which I think was first addressed by dpc-native:
>>>
>>> commit 35a0b2378c199d4f26e458b2ca38ea56aaf2d9b8
>>> Author: Olof Johansson <olof@lixom.net>
>>> Date: Wed Oct 23 12:22:05 2019 -0700
>>>
>>> PCI/DPC: Add "pcie_ports=dpc-native" to allow DPC without AER control
>>>
>>> Prior to eed85ff4c0da7 ("PCI/DPC: Enable DPC only if AER is available"),
>>> Linux handled DPC events regardless of whether firmware had granted it
>>> ownership of AER or DPC, e.g., via _OSC.
>>>
>>> PCIe r5.0, sec 6.2.10, recommends that the OS link control of DPC to
>>> control of AER, so after eed85ff4c0da7, Linux handles DPC events only if it
>>> has control of AER.
>>>
>>> On platforms that do not grant OS control of AER via _OSC, Linux DPC
>>> handling worked before eed85ff4c0da7 but not after.
>>>
>>> To make Linux DPC handling work on those platforms the same way they did
>>> before, add a "pcie_ports=dpc-native" kernel parameter that makes Linux
>>> handle DPC events regardless of whether it has control of AER.
>>>
>>> [bhelgaas: commit log, move pcie_ports_dpc_native to drivers/pci/]
>>> Link: https://lore.kernel.org/r/20191023192205.97024-1-olof@lixom.net
>>> Signed-off-by: Olof Johansson <olof@lixom.net>
>>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>
>> Are you still thinking about removing the HEST parser?
>>
>> For VMD we still need the ability to bind DPC if native_dpc==1.
>> I think if we can do that, this set should still pretty much still
>> apply with a modification to patch 2 to allow matching
>> pcie_ports_dpc_native in dpc_probe.
>
> Yes, I think we should remove the HEST firmware-first parsing, because
> IIRC the spec really doesn't specify any action the OS should take
> based on it. I was thinking Sathy might update the patch, and it fell
> off my radar.
Sorry for the delay.
I was just waiting to see whether we get any issues with merging
following commit.
commit c100beb9ccfb98e2474586a4006483cbf770c823
Author: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Date: Mon Apr 27 18:25:13 2020 -0500
PCI/AER: Use only _OSC to determine AER ownership
Since I did not see any email reporting any issues about it,
I will work on follow up patch.
>
> Bjorn
>
^ permalink raw reply
* Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper
From: Li Yang @ 2020-05-22 21:21 UTC (permalink / raw)
To: Kees Cook
Cc: Gustavo A. R. Silva, lkml, Gustavo A. R. Silva, linuxppc-dev,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Qiang Zhao
In-Reply-To: <202005202022.588918E61@keescook>
On Wed, May 20, 2020 at 10:24 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Wed, May 20, 2020 at 06:52:21PM -0500, Li Yang wrote:
> > On Mon, May 18, 2020 at 5:57 PM Kees Cook <keescook@chromium.org> wrote:
> > > Hm, looking at this code, I see a few other things that need to be
> > > fixed:
> > >
> > > 1) drivers/tty/serial/ucc_uart.c does not do a be32_to_cpu() conversion
> > > on the length test (understandably, a little-endian system has never run
> > > this code since it's ppc specific), but it's still wrong:
> > >
> > > if (firmware->header.length != fw->size) {
> > >
> > > compare to the firmware loader:
> > >
> > > length = be32_to_cpu(hdr->length);
> > >
> > > 2) drivers/soc/fsl/qe/qe.c does not perform bounds checking on the
> > > per-microcode offsets, so the uploader might send data outside the
> > > firmware buffer. Perhaps:
> >
> > We do validate the CRC for each microcode, it is unlikely the CRC
> > check can pass if the offset or length is not correct. But you are
> > probably right that it will be safer to check the boundary and fail
>
> Right, but a malicious firmware file could still match CRC but trick the
> kernel code.
>
> > quicker before we actually start the CRC check. Will you come up with
> > a formal patch or you want us to deal with it?
>
> It sounds like Gustavo will be sending one, though I don't think either
> of us have the hardware to test it with, so if you could do that part,
> that would be great! :)
That will be great. I think Zhao Qiang can help with the testing part.
Regards,
Leo
^ permalink raw reply
* Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper
From: Li Yang @ 2020-05-22 21:23 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Kees Cook, Gustavo A. R. Silva, lkml, linuxppc-dev,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Qiang Zhao
In-Reply-To: <20200518221904.GA22274@embeddedor>
On Mon, May 18, 2020 at 5:16 PM Gustavo A. R. Silva
<gustavoars@kernel.org> wrote:
>
> The current codebase makes use of one-element arrays in the following
> form:
>
> struct something {
> int length;
> u8 data[1];
> };
>
> struct something *instance;
>
> instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
> instance->length = size;
> memcpy(instance->data, source, size);
>
> but the preferred mechanism to declare variable-length types such as
> these ones is a flexible array member[1][2], introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on. So, replace
> the one-element array with a flexible-array member.
>
> Also, make use of the new struct_size() helper to properly calculate the
> size of struct qe_firmware.
>
> This issue was found with the help of Coccinelle and, audited and fixed
> _manually_.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> drivers/soc/fsl/qe/qe.c | 4 ++--
> include/soc/fsl/qe/qe.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
Applied for next. Thanks.
Regards,
Leo
^ permalink raw reply
* Re: [PATCH] treewide: Replace zero-length array with flexible-array
From: Li Yang @ 2020-05-22 21:24 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: linuxppc-dev, lkml,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20200507185301.GA14333@embeddedor>
On Thu, May 7, 2020 at 1:49 PM Gustavo A. R. Silva
<gustavoars@kernel.org> wrote:
>
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> sizeof(flexible-array-member) triggers a warning because flexible array
> members have incomplete type[1]. There are some instances of code in
> which the sizeof operator is being incorrectly/erroneously applied to
> zero-length arrays and the result is zero. Such instances may be hiding
> some bugs. So, this work (flexible-array member conversions) will also
> help to get completely rid of those sorts of issues.
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> include/linux/fsl/bestcomm/bestcomm.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied for next. Thanks.
Regards,
Leo
>
> diff --git a/include/linux/fsl/bestcomm/bestcomm.h b/include/linux/fsl/bestcomm/bestcomm.h
> index a0e2e6b19b57..154e541ce57e 100644
> --- a/include/linux/fsl/bestcomm/bestcomm.h
> +++ b/include/linux/fsl/bestcomm/bestcomm.h
> @@ -27,7 +27,7 @@
> */
> struct bcom_bd {
> u32 status;
> - u32 data[0]; /* variable payload size */
> + u32 data[]; /* variable payload size */
> };
>
> /* ======================================================================== */
>
^ permalink raw reply
* Re: [PATCH -next] soc: fsl: qbman: Remove unused inline function qm_eqcr_get_ci_stashing
From: Li Yang @ 2020-05-22 21:54 UTC (permalink / raw)
To: YueHaibing
Cc: Roy Pledge, linuxppc-dev, lkml,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20200508140846.47608-1-yuehaibing@huawei.com>
On Fri, May 8, 2020 at 9:11 AM YueHaibing <yuehaibing@huawei.com> wrote:
>
> There's no callers in-tree anymore.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied for next. Thanks.
Regards,
Leo
> ---
> drivers/soc/fsl/qbman/qman.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index 1e164e03410a..9888a7061873 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -449,11 +449,6 @@ static inline int qm_eqcr_init(struct qm_portal *portal,
> return 0;
> }
>
> -static inline unsigned int qm_eqcr_get_ci_stashing(struct qm_portal *portal)
> -{
> - return (qm_in(portal, QM_REG_CFG) >> 28) & 0x7;
> -}
> -
> static inline void qm_eqcr_finish(struct qm_portal *portal)
> {
> struct qm_eqcr *eqcr = &portal->eqcr;
> --
> 2.17.1
>
>
^ permalink raw reply
* Re: [PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()
From: Li Yang @ 2020-05-22 22:40 UTC (permalink / raw)
To: Tang Bin
Cc: Felipe Balbi, Greg Kroah-Hartman, linux-usb, kernel-janitors,
lkml, Markus Elfring, linuxppc-dev, Dan Carpenter
In-Reply-To: <f712918c-2f61-0ba5-2ba8-b5ca3cce9a35@cmss.chinamobile.com>
On Tue, Apr 14, 2020 at 4:13 AM Tang Bin <tangbin@cmss.chinamobile.com> wrote:
>
> Hi
>
> On 2020/4/14 16:30, Dan Carpenter wrote:
> > On Fri, Apr 10, 2020 at 04:05:06PM +0800, Tang Bin wrote:
> >>>
> >>>> Thus it must be fixed.
> >>> Wording alternative:
> >>> Thus adjust the error detection and corresponding exception handling.
> >> Got it.
> > Wow...
> >
> > No, don't listen to Markus when it comes to writing commit messages.
> > You couldn't find worse advice anywhere. :P
>
> I'm actively waiting for a reply from the maintainer. Thank you very much.
Sorry for the late response.
Acked-by: Li Yang <leoyang.li@nxp.com>
Regards,
Leo
^ permalink raw reply
* Re: [PATCH] net/ethernet/freescale: rework quiesce/activate for ucc_geth
From: David Miller @ 2020-05-22 22:50 UTC (permalink / raw)
To: valentin; +Cc: matteo.ghidoni, netdev, hkallweit1, linuxppc-dev, kuba
In-Reply-To: <20200520155350.1372-1-valentin@longchamp.me>
From: Valentin Longchamp <valentin@longchamp.me>
Date: Wed, 20 May 2020 17:53:50 +0200
> ugeth_quiesce/activate are used to halt the controller when there is a
> link change that requires to reconfigure the mac.
>
> The previous implementation called netif_device_detach(). This however
> causes the initial activation of the netdevice to fail precisely because
> it's detached. For details, see [1].
>
> A possible workaround was the revert of commit
> net: linkwatch: add check for netdevice being present to linkwatch_do_dev
> However, the check introduced in the above commit is correct and shall be
> kept.
>
> The netif_device_detach() is thus replaced with
> netif_tx_stop_all_queues() that prevents any tranmission. This allows to
> perform mac config change required by the link change, without detaching
> the corresponding netdevice and thus not preventing its initial
> activation.
>
> [1] https://lists.openwall.net/netdev/2020/01/08/201
>
> Signed-off-by: Valentin Longchamp <valentin@longchamp.me>
> Acked-by: Matteo Ghidoni <matteo.ghidoni@ch.abb.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] soc: fsl: qe: clean up an indentation issue
From: Li Yang @ 2020-05-22 23:01 UTC (permalink / raw)
To: Colin King
Cc: kernel-janitors, linuxppc-dev, lkml,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Qiang Zhao
In-Reply-To: <20200327161349.284679-1-colin.king@canonical.com>
On Fri, Mar 27, 2020 at 11:15 AM Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> There is a statement that not indented correctly, remove the
> extraneous space.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied for next. Thanks.
> ---
> drivers/soc/fsl/qe/ucc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
> index d6c93970df4d..cac0fb7693a0 100644
> --- a/drivers/soc/fsl/qe/ucc.c
> +++ b/drivers/soc/fsl/qe/ucc.c
> @@ -519,7 +519,7 @@ int ucc_set_tdm_rxtx_clk(u32 tdm_num, enum qe_clock clock,
> int clock_bits;
> u32 shift;
> struct qe_mux __iomem *qe_mux_reg;
> - __be32 __iomem *cmxs1cr;
> + __be32 __iomem *cmxs1cr;
>
> qe_mux_reg = &qe_immr->qmx;
>
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH -next] soc: fsl: dpio: Remove unused inline function qbman_write_eqcr_am_rt_register
From: Li Yang @ 2020-05-22 23:11 UTC (permalink / raw)
To: YueHaibing
Cc: Roy Pledge, linuxppc-dev, Youri Querry,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, lkml
In-Reply-To: <20200508140947.28712-1-yuehaibing@huawei.com>
On Fri, May 8, 2020 at 9:13 AM YueHaibing <yuehaibing@huawei.com> wrote:
>
> There's no callers in-tree anymore since commit
> 3b2abda7d28c ("soc: fsl: dpio: Replace QMAN array mode with ring mode enqueue")
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied to next. Thanks.
Regards,
Leo
> ---
> drivers/soc/fsl/dpio/qbman-portal.c | 12 ------------
> 1 file changed, 12 deletions(-)
>
> diff --git a/drivers/soc/fsl/dpio/qbman-portal.c b/drivers/soc/fsl/dpio/qbman-portal.c
> index 804b8ba9bf5c..e2e9fbb58a72 100644
> --- a/drivers/soc/fsl/dpio/qbman-portal.c
> +++ b/drivers/soc/fsl/dpio/qbman-portal.c
> @@ -572,18 +572,6 @@ void qbman_eq_desc_set_qd(struct qbman_eq_desc *d, u32 qdid,
> #define EQAR_VB(eqar) ((eqar) & 0x80)
> #define EQAR_SUCCESS(eqar) ((eqar) & 0x100)
>
> -static inline void qbman_write_eqcr_am_rt_register(struct qbman_swp *p,
> - u8 idx)
> -{
> - if (idx < 16)
> - qbman_write_register(p, QBMAN_CINH_SWP_EQCR_AM_RT + idx * 4,
> - QMAN_RT_MODE);
> - else
> - qbman_write_register(p, QBMAN_CINH_SWP_EQCR_AM_RT2 +
> - (idx - 16) * 4,
> - QMAN_RT_MODE);
> -}
> -
> #define QB_RT_BIT ((u32)0x100)
> /**
> * qbman_swp_enqueue_direct() - Issue an enqueue command
> --
> 2.17.1
>
>
^ permalink raw reply
* Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
From: Thiago Jung Bauermann @ 2020-05-23 4:08 UTC (permalink / raw)
To: Prakhar Srivastava
Cc: Mark Rutland, kstewart, gregkh, bhsharma, tao.li, zohar, paulus,
vincenzo.frascino, will, Rob Herring, nramas, frowand.list,
masahiroy, jmorris, takahiro.akashi, linux-arm-kernel,
catalin.marinas, serge, devicetree, pasha.tatashin, hsinyi,
tusharsu, tglx, allison, christophe.leroy, mbrugger, balajib,
dmitry.kasatkin, linux-kernel, linux-security-module, james.morse,
linux-integrity, linuxppc-dev
In-Reply-To: <7701df90-a68b-b710-4279-9d64e45ee792@linux.microsoft.com>
Hello Prakhar,
Prakhar Srivastava <prsriva@linux.microsoft.com> writes:
> On 5/12/20 4:05 PM, Rob Herring wrote:
>> On Wed, May 06, 2020 at 10:50:04PM -0700, Prakhar Srivastava wrote:
>>> Hi Mark,
>>
>> Please don't top post.
>>
>>> This patch set currently only address the Pure DT implementation.
>>> EFI and ACPI implementations will be posted in subsequent patchsets.
>>>
>>> The logs are intended to be carried over the kexec and once read the
>>> logs are no longer needed and in prior conversation with James(
>>> https://lore.kernel.org/linux-arm-kernel/0053eb68-0905-4679-c97a-00c5cb6f1abb@arm.com/)
>>> the apporach of using a chosen node doesn't
>>> support the case.
>>>
>>> The DT entries make the reservation permanent and thus doesnt need kernel
>>> segments to be used for this, however using a chosen-node with
>>> reserved memory only changes the node information but memory still is
>>> reserved via reserved-memory section.
>>
>> I think Mark's point was whether it needs to be permanent. We don't
>> hardcode the initrd address for example.
>>
> Thankyou for clarifying my misunderstanding, i am modelling this keeping to the
> TPM log implementation that uses a reserved memory. I will rev up the version
> with chosen-node support.
> That will make the memory reservation free after use.
Nice. Do you intend to use the same property that powerpc uses
(linux,ima-kexec-buffer)?
>>> On 5/5/20 2:59 AM, Mark Rutland wrote:
>>>> Hi Prakhar,
>>>>
>>>> On Mon, May 04, 2020 at 01:38:27PM -0700, Prakhar Srivastava wrote:
>>>>> IMA during kexec(kexec file load) verifies the kernel signature and measures
>>
>> What's IMAIMA is a LSM attempting to detect if files have been accidentally or
> maliciously altered, both remotely and locally, it can also be used
> to appraise a file's measurement against a "good" value stored as an extended
> attribute, and enforce local file integrity.
>
> IMA also validates and measures the signers of the kernel and initrd
> during kexec. The measurements are extended to PCR 10(configurable) and the logs
> stored in memory, however once kexec'd the logs are lost. Kexec is used as
> secondary boot loader in may use cases and loosing the signer
> creates a security hole.
>
> This patch is an implementation to carry over the logs and making it
> possible to remotely validate the signers of the kernel and initrd. Such a
> support exits only in powerpc.
> This patch makes the carry over of logs architecture independent and puts the
> complexity in a driver.
If I'm not mistaken, the code at arch/powerpc/kexec/ima.c isn't actually
powerpc-specific. It could be moved to an arch-independent directory and
used by any other architecture which supports device trees.
I think that's the simplest way forward. And to be honest I'm still
trying to understand why you didn't take that approach. Did you try it
and hit some obstacle or noticed a disadvantage for your use case?
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH] kbuild: reuse vmlinux.o in vmlinux_link
From: Nicholas Piggin @ 2020-05-23 10:06 UTC (permalink / raw)
To: linuxppc-dev, Masahiro Yamada, Michael Ellerman, Sami Tolvanen
Cc: Linux Kernel Mailing List, Michal Marek, Kees Cook,
Linux Kbuild mailing list
In-Reply-To: <CAK7LNASm2t-Dkr+p_EWvqf_eoKn5R2iXWuBHnTB9n6MUxr3-pQ@mail.gmail.com>
Excerpts from Masahiro Yamada's message of May 23, 2020 3:44 am:
> + Michael, and PPC ML.
>
> They may know something about the reason of failure.
Because the linker can't put branch stubs within object code sections,
so when you incrementally link them too large, the linker can't resolve
branches into other object files.
This is why we added incremental linking in the first place. I suppose
it could be made conditional for platforms that can use this
optimization.
What'd be really nice is if we could somehow build and link kallsyms
without relinking everything twice, and if we could do section mismatch
analysis without making that vmlinux.o as well. I had a few ideas but
not enough time to do much work on it.
Thanks,
Nick
>
>
> On Sat, May 23, 2020 at 2:41 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>
>> On Fri, May 22, 2020 at 5:27 AM Sami Tolvanen <samitolvanen@google.com> wrote:
>> >
>> > Instead of linking all compilation units again each time vmlinux_link is
>> > called, reuse vmlinux.o from modpost_link.
>> >
>> > With x86_64 allyesconfig, vmlinux_link is called three times and reusing
>> > vmlinux.o reduces the build time ~38 seconds on my system (59% reduction
>> > in the time spent in vmlinux_link).
>> >
>> > Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
>> > ---
>> > scripts/link-vmlinux.sh | 5 +----
>> > 1 file changed, 1 insertion(+), 4 deletions(-)
>> >
>> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
>> > index d09ab4afbda4..c6cc4305950c 100755
>> > --- a/scripts/link-vmlinux.sh
>> > +++ b/scripts/link-vmlinux.sh
>> > @@ -77,11 +77,8 @@ vmlinux_link()
>> >
>> > if [ "${SRCARCH}" != "um" ]; then
>> > objects="--whole-archive \
>> > - ${KBUILD_VMLINUX_OBJS} \
>> > + vmlinux.o \
>> > --no-whole-archive \
>> > - --start-group \
>> > - ${KBUILD_VMLINUX_LIBS} \
>> > - --end-group \
>> > ${@}"
>> >
>> > ${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux} \
>> >
>> > base-commit: b85051e755b0e9d6dd8f17ef1da083851b83287d
>> > --
>> > 2.27.0.rc0.183.gde8f92d652-goog
>> >
>>
>>
>> I like this patch irrespective of CLANG_LTO, but
>> unfortunately, my build test failed.
>>
>>
>> ARCH=powerpc failed to build as follows:
>>
>>
>>
>> MODPOST vmlinux.o
>> MODINFO modules.builtin.modinfo
>> GEN modules.builtin
>> LD .tmp_vmlinux.kallsyms1
>> vmlinux.o:(__ftr_alt_97+0x20): relocation truncated to fit:
>> R_PPC64_REL14 against `.text'+4b1c
>> vmlinux.o:(__ftr_alt_97+0x164): relocation truncated to fit:
>> R_PPC64_REL14 against `.text'+1cf78
>> vmlinux.o:(__ftr_alt_97+0x288): relocation truncated to fit:
>> R_PPC64_REL14 against `.text'+1dac4
>> vmlinux.o:(__ftr_alt_97+0x2f0): relocation truncated to fit:
>> R_PPC64_REL14 against `.text'+1e254
>> make: *** [Makefile:1125: vmlinux] Error 1
>>
>>
>>
>> I used powerpc-linux-gcc
>> available at
>> https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/9.2.0/
>>
>>
>> Build command:
>>
>> make -j24 ARCH=powerpc CROSS_COMPILE=powerpc-linux- defconfig all
>>
>>
>> Could you check it please?
>>
>>
>>
>> I will apply it to my test branch.
>> Perhaps, 0-day bot may find more failure cases.
>>
>>
>> --
>> Best Regards
>> Masahiro Yamada
>
>
>
> --
> Best Regards
> Masahiro Yamada
>
^ permalink raw reply
* Re: [PATCH v4 1/6] printk: Collapse shutdown types into a single dump reason
From: Michael Ellerman @ 2020-05-23 11:16 UTC (permalink / raw)
To: Kees Cook, Pavel Tatashin
Cc: Petr Mladek, Tony Luck, Kees Cook, Jonathan Corbet,
Anton Vorontsov, linux-doc, linux-kernel, Steven Rostedt,
Sergey Senozhatsky, devicetree, Rob Herring, Paul Mackerras,
Colin Cross, Enric Balletbo i Serra, linuxppc-dev, Benson Leung
In-Reply-To: <20200515184434.8470-2-keescook@chromium.org>
Kees Cook <keescook@chromium.org> writes:
> To turn the KMSG_DUMP_* reasons into a more ordered list, collapse
> the redundant KMSG_DUMP_(RESTART|HALT|POWEROFF) reasons into
> KMSG_DUMP_SHUTDOWN. The current users already don't meaningfully
> distinguish between them, so there's no need to, as discussed here:
> https://lore.kernel.org/lkml/CA+CK2bAPv5u1ih5y9t5FUnTyximtFCtDYXJCpuyjOyHNOkRdqw@mail.gmail.com/
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/powerpc/kernel/nvram_64.c | 4 +---
> fs/pstore/platform.c | 8 ++------
> include/linux/kmsg_dump.h | 4 +---
> kernel/reboot.c | 6 +++---
> 4 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index fb4f61096613..0cd1c88bfc8b 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -655,9 +655,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
> int rc = -1;
>
> switch (reason) {
> - case KMSG_DUMP_RESTART:
> - case KMSG_DUMP_HALT:
> - case KMSG_DUMP_POWEROFF:
> + case KMSG_DUMP_SHUTDOWN:
> /* These are almost always orderly shutdowns. */
> return;
> case KMSG_DUMP_OOPS:
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
^ permalink raw reply
* Re: [PATCH] kbuild: reuse vmlinux.o in vmlinux_link
From: Masahiro Yamada @ 2020-05-23 15:12 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Michal Marek, Kees Cook, Sam Ravnborg, Linux Kbuild mailing list,
Linux Kernel Mailing List, Sami Tolvanen, linuxppc-dev
In-Reply-To: <1590226253.lnkg0jun9x.astroid@bobo.none>
Hi Nicholas,
(+CC: Sam Ravnborg)
On Sat, May 23, 2020 at 7:06 PM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> Excerpts from Masahiro Yamada's message of May 23, 2020 3:44 am:
> > + Michael, and PPC ML.
> >
> > They may know something about the reason of failure.
>
> Because the linker can't put branch stubs within object code sections,
> so when you incrementally link them too large, the linker can't resolve
> branches into other object files.
Ah, you are right.
So, this is a problem not only for PPC
but also for ARM (both 32 and 64 bit), etc.
ARM needs to insert a veneer to jump far.
Prior to thin archive, we could not compile
ARCH=arm allyesconfig because
drivers/built-in.o was too large.
This patch gets us back to the too large
incremental object situation.
With my quick compile-testing,
ARCH=arm allyesconfig
and ARCH=arm64 allyesconfig are broken.
> This is why we added incremental linking in the first place. I suppose
> it could be made conditional for platforms that can use this
> optimization.
>
> What'd be really nice is if we could somehow build and link kallsyms
> without relinking everything twice, and if we could do section mismatch
> analysis without making that vmlinux.o as well. I had a few ideas but
> not enough time to do much work on it.
Right, kallsyms links 3 times. (not twice)
Hmm, I think Sami's main motivation is Clang LTO.
LTO is very time-consuming.
So, the android common kernel implements Clang LTO
in the pre modpost stage:
1) LTO against vmlinux.o
2) modpost against vmlinux.o
3) Link vmlinux.o + kallsyms into vmlinux
(this requires linking 3 times)
If we move LTO to 3), we need to do LTO 3 times.
And, this was how GCC LTO was implemented in 2014,
(then rejected by Linus).
How to do modpost without making vmlinux.o ?
In old days, the section mismatch analysis was done
against the final vmlinux.
85bd2fddd68e757da8e1af98f857f61a3c9ce647 changed
it to run modpost for individual .o files.
Then, 741f98fe298a73c9d47ed53703c1279a29718581
introduced vmlinux.o to use it for modpost.
The following two commits.
I did not fully understand the background, though.
I CC'ed Sam in case he may add some comments.
commit 85bd2fddd68e757da8e1af98f857f61a3c9ce647
Author: Sam Ravnborg <sam@ravnborg.org>
Date: Mon Feb 26 15:33:52 2007 +0100
kbuild: fix section mismatch check for vmlinux
vmlinux does not contain relocation entries which is
used by the section mismatch checks.
Reported by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Use the individual objects as inputs to overcome
this limitation.
In modpost check the .o files and skip non-ELF files.
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
commit 741f98fe298a73c9d47ed53703c1279a29718581
Author: Sam Ravnborg <sam@ravnborg.org>
Date: Tue Jul 17 10:54:06 2007 +0200
kbuild: do section mismatch check on full vmlinux
Previously we did do the check on the .o files used to link
vmlinux but that failed to find questionable references across
the .o files.
Create a dedicated vmlinux.o file used only for section mismatch checks
that uses the defualt linker script so section does not get renamed.
The vmlinux.o may later be used as part of the the final link of vmlinux
but for now it is used fo section mismatch only.
For a defconfig build this is instant but for an allyesconfig this
add two minutes to a full build (that anyways takes ~2 hours).
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
>
> Thanks,
> Nick
>
> >
> >
> > On Sat, May 23, 2020 at 2:41 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >>
> >> On Fri, May 22, 2020 at 5:27 AM Sami Tolvanen <samitolvanen@google.com> wrote:
> >> >
> >> > Instead of linking all compilation units again each time vmlinux_link is
> >> > called, reuse vmlinux.o from modpost_link.
> >> >
> >> > With x86_64 allyesconfig, vmlinux_link is called three times and reusing
> >> > vmlinux.o reduces the build time ~38 seconds on my system (59% reduction
> >> > in the time spent in vmlinux_link).
> >> >
> >> > Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> >> > ---
> >> > scripts/link-vmlinux.sh | 5 +----
> >> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >> >
> >> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> >> > index d09ab4afbda4..c6cc4305950c 100755
> >> > --- a/scripts/link-vmlinux.sh
> >> > +++ b/scripts/link-vmlinux.sh
> >> > @@ -77,11 +77,8 @@ vmlinux_link()
> >> >
> >> > if [ "${SRCARCH}" != "um" ]; then
> >> > objects="--whole-archive \
> >> > - ${KBUILD_VMLINUX_OBJS} \
> >> > + vmlinux.o \
> >> > --no-whole-archive \
> >> > - --start-group \
> >> > - ${KBUILD_VMLINUX_LIBS} \
> >> > - --end-group \
> >> > ${@}"
> >> >
> >> > ${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux} \
> >> >
> >> > base-commit: b85051e755b0e9d6dd8f17ef1da083851b83287d
> >> > --
> >> > 2.27.0.rc0.183.gde8f92d652-goog
> >> >
> >>
> >>
> >> I like this patch irrespective of CLANG_LTO, but
> >> unfortunately, my build test failed.
> >>
> >>
> >> ARCH=powerpc failed to build as follows:
> >>
> >>
> >>
> >> MODPOST vmlinux.o
> >> MODINFO modules.builtin.modinfo
> >> GEN modules.builtin
> >> LD .tmp_vmlinux.kallsyms1
> >> vmlinux.o:(__ftr_alt_97+0x20): relocation truncated to fit:
> >> R_PPC64_REL14 against `.text'+4b1c
> >> vmlinux.o:(__ftr_alt_97+0x164): relocation truncated to fit:
> >> R_PPC64_REL14 against `.text'+1cf78
> >> vmlinux.o:(__ftr_alt_97+0x288): relocation truncated to fit:
> >> R_PPC64_REL14 against `.text'+1dac4
> >> vmlinux.o:(__ftr_alt_97+0x2f0): relocation truncated to fit:
> >> R_PPC64_REL14 against `.text'+1e254
> >> make: *** [Makefile:1125: vmlinux] Error 1
> >>
> >>
> >>
> >> I used powerpc-linux-gcc
> >> available at
> >> https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/9.2.0/
> >>
> >>
> >> Build command:
> >>
> >> make -j24 ARCH=powerpc CROSS_COMPILE=powerpc-linux- defconfig all
> >>
> >>
> >> Could you check it please?
> >>
> >>
> >>
> >> I will apply it to my test branch.
> >> Perhaps, 0-day bot may find more failure cases.
> >>
> >>
> >> --
> >> Best Regards
> >> Masahiro Yamada
> >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
> >
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* [linux-next RFC] mm/gup.c: Convert to use get_user_pages_fast_only()
From: Souptick Joarder @ 2020-05-23 16:41 UTC (permalink / raw)
To: paulus, mpe, benh, akpm, peterz, mingo, acme, mark.rutland,
alexander.shishkin, namhyung, pbonzini, sfr, rppt, msuchanek,
aneesh.kumar
Cc: Matthew Wilcox, kvm, John Hubbard, linux-kernel, kvm-ppc,
linux-mm, Souptick Joarder, linuxppc-dev
Renaming the API __get_user_pages_fast() to get_user_pages_
fast_only() to align with pin_user_pages_fast_only().
As part of this we will get rid of write parameter.
Instead caller will pass FOLL_WRITE to get_user_pages_fast_only().
This will not change any existing functionality of the API.
All the callers are changed to pass FOLL_WRITE.
Updated the documentation of the API.
Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Matthew Wilcox <willy@infradead.org>
---
arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 2 +-
arch/powerpc/perf/callchain_64.c | 2 +-
include/linux/mm.h | 4 ++--
kernel/events/core.c | 4 ++--
mm/gup.c | 29 ++++++++++++++++-------------
virt/kvm/kvm_main.c | 6 +++---
7 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 18aed97..34fc5c8 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -581,7 +581,7 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
* We always ask for write permission since the common case
* is that the page is writable.
*/
- if (__get_user_pages_fast(hva, 1, 1, &page) == 1) {
+ if (get_user_pages_fast_only(hva, 1, FOLL_WRITE, &page) == 1) {
write_ok = true;
} else {
/* Call KVM generic code to do the slow-path check */
diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
index 3248f78..3b6e342 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
@@ -795,7 +795,7 @@ int kvmppc_book3s_instantiate_page(struct kvm_vcpu *vcpu,
* is that the page is writable.
*/
hva = gfn_to_hva_memslot(memslot, gfn);
- if (!kvm_ro && __get_user_pages_fast(hva, 1, 1, &page) == 1) {
+ if (!kvm_ro && get_user_pages_fast_only(hva, 1, FOLL_WRITE, &page) == 1) {
upgrade_write = true;
} else {
unsigned long pfn;
diff --git a/arch/powerpc/perf/callchain_64.c b/arch/powerpc/perf/callchain_64.c
index 1bff896d..f719a74 100644
--- a/arch/powerpc/perf/callchain_64.c
+++ b/arch/powerpc/perf/callchain_64.c
@@ -32,7 +32,7 @@ int read_user_stack_slow(void __user *ptr, void *buf, int nb)
int nrpages;
void *kaddr;
- nrpages = __get_user_pages_fast(addr, 1, 1, &page);
+ nrpages = get_user_pages_fast_only(addr, 1, FOLL_WRITE, &page);
if (nrpages == 1) {
kaddr = page_address(page);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 93d93bd..10a6758 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1817,8 +1817,8 @@ extern int mprotect_fixup(struct vm_area_struct *vma,
/*
* doesn't attempt to fault and will return short.
*/
-int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
- struct page **pages);
+int get_user_pages_fast_only(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages);
int pin_user_pages_fast_only(unsigned long start, int nr_pages,
unsigned int gup_flags, struct page **pages);
/*
diff --git a/kernel/events/core.c b/kernel/events/core.c
index c94eb27..81d6e73 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6934,12 +6934,12 @@ static u64 perf_virt_to_phys(u64 virt)
* Walking the pages tables for user address.
* Interrupts are disabled, so it prevents any tear down
* of the page tables.
- * Try IRQ-safe __get_user_pages_fast first.
+ * Try IRQ-safe get_user_pages_fast_only first.
* If failed, leave phys_addr as 0.
*/
if (current->mm != NULL) {
pagefault_disable();
- if (__get_user_pages_fast(virt, 1, 0, &p) == 1)
+ if (get_user_pages_fast_only(virt, 1, 0, &p) == 1)
phys_addr = page_to_phys(p) + virt % PAGE_SIZE;
pagefault_enable();
}
diff --git a/mm/gup.c b/mm/gup.c
index 80f51a36..d8aabc0 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2278,7 +2278,7 @@ static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
* to be special.
*
* For a futex to be placed on a THP tail page, get_futex_key requires a
- * __get_user_pages_fast implementation that can pin pages. Thus it's still
+ * get_user_pages_fast_only implementation that can pin pages. Thus it's still
* useful to have gup_huge_pmd even if we can't operate on ptes.
*/
static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
@@ -2683,7 +2683,7 @@ static inline void gup_pgd_range(unsigned long addr, unsigned long end,
#ifndef gup_fast_permitted
/*
- * Check if it's allowed to use __get_user_pages_fast() for the range, or
+ * Check if it's allowed to use get_user_pages_fast_only() for the range, or
* we need to fall back to the slow version:
*/
static bool gup_fast_permitted(unsigned long start, unsigned long end)
@@ -2776,8 +2776,14 @@ static int internal_get_user_pages_fast(unsigned long start, int nr_pages,
return ret;
}
-
-/*
+/**
+ * get_user_pages_fast_only() - pin user pages in memory
+ * @start: starting user address
+ * @nr_pages: number of pages from start to pin
+ * @gup_flags: flags modifying pin behaviour
+ * @pages: array that receives pointers to the pages pinned.
+ * Should be at least nr_pages long.
+ *
* Like get_user_pages_fast() except it's IRQ-safe in that it won't fall back to
* the regular GUP.
* Note a difference with get_user_pages_fast: this always returns the
@@ -2786,8 +2792,8 @@ static int internal_get_user_pages_fast(unsigned long start, int nr_pages,
* If the architecture does not support this function, simply return with no
* pages pinned.
*/
-int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
- struct page **pages)
+int get_user_pages_fast_only(unsigned long start, int nr_pages,
+ unsigned int gup_flags, struct page **pages)
{
int nr_pinned;
/*
@@ -2797,10 +2803,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
* FOLL_FAST_ONLY is required in order to match the API description of
* this routine: no fall back to regular ("slow") GUP.
*/
- unsigned int gup_flags = FOLL_GET | FOLL_FAST_ONLY;
-
- if (write)
- gup_flags |= FOLL_WRITE;
+ gup_flags = FOLL_GET | FOLL_FAST_ONLY;
nr_pinned = internal_get_user_pages_fast(start, nr_pages, gup_flags,
pages);
@@ -2815,7 +2818,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
return nr_pinned;
}
-EXPORT_SYMBOL_GPL(__get_user_pages_fast);
+EXPORT_SYMBOL_GPL(get_user_pages_fast_only);
/**
* get_user_pages_fast() - pin user pages in memory
@@ -2886,8 +2889,8 @@ int pin_user_pages_fast(unsigned long start, int nr_pages,
EXPORT_SYMBOL_GPL(pin_user_pages_fast);
/*
- * This is the FOLL_PIN equivalent of __get_user_pages_fast(). Behavior is the
- * same, except that this one sets FOLL_PIN instead of FOLL_GET.
+ * This is the FOLL_PIN equivalent of get_user_pages_fast_only(). Behavior
+ * is the same, except that this one sets FOLL_PIN instead of FOLL_GET.
*
* The API rules are the same, too: no negative values may be returned.
*/
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index fc38d63..cec7919 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1750,7 +1750,7 @@ static bool hva_to_pfn_fast(unsigned long addr, bool write_fault,
if (!(write_fault || writable))
return false;
- npages = __get_user_pages_fast(addr, 1, 1, page);
+ npages = get_user_pages_fast_only(addr, 1, FOLL_WRITE, page);
if (npages == 1) {
*pfn = page_to_pfn(page[0]);
@@ -1791,7 +1791,7 @@ static int hva_to_pfn_slow(unsigned long addr, bool *async, bool write_fault,
if (unlikely(!write_fault) && writable) {
struct page *wpage;
- if (__get_user_pages_fast(addr, 1, 1, &wpage) == 1) {
+ if (get_user_pages_fast_only(addr, 1, FOLL_WRITE, &wpage) == 1) {
*writable = true;
put_page(page);
page = wpage;
@@ -1998,7 +1998,7 @@ int gfn_to_page_many_atomic(struct kvm_memory_slot *slot, gfn_t gfn,
if (entry < nr_pages)
return 0;
- return __get_user_pages_fast(addr, nr_pages, 1, pages);
+ return get_user_pages_fast(addr, nr_pages, FOLL_WRITE, pages);
}
EXPORT_SYMBOL_GPL(gfn_to_page_many_atomic);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] kbuild: reuse vmlinux.o in vmlinux_link
From: Sam Ravnborg @ 2020-05-23 16:53 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Michal Marek, Kees Cook, Linux Kbuild mailing list,
Linux Kernel Mailing List, Nicholas Piggin, Sami Tolvanen,
linuxppc-dev
In-Reply-To: <CAK7LNAR_-q3jhaUzDpkC3ej_DpAerzMsORT-tFw_3AwX7xM0Yw@mail.gmail.com>
Hi Masahiro.
On Sun, May 24, 2020 at 12:12:35AM +0900, Masahiro Yamada wrote:
> Hi Nicholas,
> (+CC: Sam Ravnborg)
>
>
> On Sat, May 23, 2020 at 7:06 PM Nicholas Piggin <npiggin@gmail.com> wrote:
> >
> > Excerpts from Masahiro Yamada's message of May 23, 2020 3:44 am:
> > > + Michael, and PPC ML.
> > >
> > > They may know something about the reason of failure.
> >
> > Because the linker can't put branch stubs within object code sections,
> > so when you incrementally link them too large, the linker can't resolve
> > branches into other object files.
>
>
> Ah, you are right.
>
> So, this is a problem not only for PPC
> but also for ARM (both 32 and 64 bit), etc.
>
> ARM needs to insert a veneer to jump far.
>
> Prior to thin archive, we could not compile
> ARCH=arm allyesconfig because
> drivers/built-in.o was too large.
>
> This patch gets us back to the too large
> incremental object situation.
>
> With my quick compile-testing,
> ARCH=arm allyesconfig
> and ARCH=arm64 allyesconfig are broken.
>
>
> > This is why we added incremental linking in the first place. I suppose
> > it could be made conditional for platforms that can use this
> > optimization.
> >
> > What'd be really nice is if we could somehow build and link kallsyms
> > without relinking everything twice, and if we could do section mismatch
> > analysis without making that vmlinux.o as well. I had a few ideas but
> > not enough time to do much work on it.
>
>
> Right, kallsyms links 3 times. (not twice)
>
>
> Hmm, I think Sami's main motivation is Clang LTO.
>
> LTO is very time-consuming.
> So, the android common kernel implements Clang LTO
> in the pre modpost stage:
>
>
> 1) LTO against vmlinux.o
>
> 2) modpost against vmlinux.o
>
> 3) Link vmlinux.o + kallsyms into vmlinux
> (this requires linking 3 times)
We have kallsyms we had to link three times because the linking
increased the object a little in size so symbols did not match.
The last time was added more or less only to check that we did
have stable symbol addresses.
All this predates LTO stuff which we only introduced later.
The reason for doing modpost on vmlinux.o was that we had cases
where everything in drivers/ was fine but there was section mismatch
references from arch/* to drivers/*
This is back when there were much more drivers in arch/ than what we
have today.
And back then we also had much more to check ad we had cPU hotplug
that could really cause section mismatches - this is no longer the case
which is a good thing.
...
>
> The following two commits.
> I did not fully understand the background, though.
>
> I CC'ed Sam in case he may add some comments.
>
> commit 85bd2fddd68e757da8e1af98f857f61a3c9ce647
> Author: Sam Ravnborg <sam@ravnborg.org>
> Date: Mon Feb 26 15:33:52 2007 +0100
>
> kbuild: fix section mismatch check for vmlinux
>
> vmlinux does not contain relocation entries which is
> used by the section mismatch checks.
> Reported by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
>
> Use the individual objects as inputs to overcome
> this limitation.
> In modpost check the .o files and skip non-ELF files.
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
So we checked vmlinx - but vmlinx did have too much stripped away.
so in reality nothing was checked.
To allow the warnings to be as precise as possible move the checks
out to the indovidual .o files.
Sometimes the names was mangled a little so if warnigns was only
reported on vmlinx level in could be difficult to track down the
offender.
This would then also do the check on .o files that had all the
relocation symbols rtequired.
>
> commit 741f98fe298a73c9d47ed53703c1279a29718581
> Author: Sam Ravnborg <sam@ravnborg.org>
> Date: Tue Jul 17 10:54:06 2007 +0200
>
> kbuild: do section mismatch check on full vmlinux
>
> Previously we did do the check on the .o files used to link
> vmlinux but that failed to find questionable references across
> the .o files.
> Create a dedicated vmlinux.o file used only for section mismatch checks
> that uses the defualt linker script so section does not get renamed.
>
> The vmlinux.o may later be used as part of the the final link of vmlinux
> but for now it is used fo section mismatch only.
> For a defconfig build this is instant but for an allyesconfig this
> add two minutes to a full build (that anyways takes ~2 hours).
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
But when we introduced check of the individual .o fiules we missed when
the references spanned outside the .o files as explained previously.
So included a link of vmlinx.o that did NOT drop the relocations
so we could use it to check for the remaining section mismatch warnings.
Remember - back when we started this we had many hundred warnings
and it was a fight to keep that number low.
But we also wanted to report as much as possible.
There was back then several discussions if this was really worth the
effort. How much was gained from discarding the memory where the
section mismatch warnigns was triggered.
In other words - how about just keeping the init code in memory so there
were no illegal references anymore.
That is something that is maybe worth to consiuder again as we have even
less memory we save by throwing away the init code.
But I think this is a topic for another mail thread.
Sam
^ permalink raw reply
* Kernel bug in 5.7 for PPC32 on PowerBook G4 - bisected to commit 697ece7
From: Larry Finger @ 2020-05-23 17:24 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Mackerras, ppc-dev, LKML
Hi Christophe,
Although kernel 5.7.0-rc2 appeared to boot cleanly, it failed on my G4 when I
tried to generate a new kernel. The following BUG message is logged:
[ 336.148935] ------------[ cut here ]------------
[ 336.148950] kernel BUG at ./include/linux/swapops.h:195!
[ 336.148971] Oops: Exception in kernel mode, sig: 5 [#1]
[ 336.148975] BE PAGE_SIZE=4K MMU=Hash PowerMac
[ 336.148978] Modules linked in: cpufreq_ondemand fuse ctr ccm b43 mac80211
sha256_generic libsha256 cfg80211 hid_apple hid_generic joydev rfkill libarc4
rng_core cordic uinput radeon evdev
ttm drm_kms_helper usbhid hid appletouch drm rack_meter ehci_pci ehci_hcd
drm_panel_orientation_quirks ssb fb_sys_fops yenta_socket sysimgblt sysfillrect
pcmcia_rsrc syscopyarea pcmcia pcmcia
_core i2c_powermac therm_adt746x loop ohci_pci ohci_hcd usbcore sungem
sungem_phy usb_common
[ 336.149052] CPU: 0 PID: 8346 Comm: ld Not tainted 5.6.0-rc2-00086-g36b7840 #249
[ 336.149056] NIP: c0166624 LR: c016661c CTR: 00000000
[ 336.149062] REGS: f42ddb08 TRAP: 0700 Not tainted (5.6.0-rc2-00086-g36b7840)
[ 336.149064] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 24000424 XER: 00000000
[ 336.149072]
[ 336.149072] GPR00: 00000000 f42ddbc0 c24fcb80 00000001 ef438f00 c1957b1c
f42ddc4c 00000004
[ 336.149072] GPR08: 00000050 00000100 00000000 edb9d000 00000000 100cba68
10051b10 10051b08
[ 336.149072] GPR16: 000001be ee68c078 0000105a 00000000 00000000 c1957b1c
00000000 00000000
[ 336.149072] GPR24: ec5d2540 00000000 7c002bf8 c1957ae0 00000000 ec5d2540
ef438f00 ef438f00
[ 336.149107] NIP [c0166624] _einittext+0x3f9d38a8/0x3fe4a284
[ 336.149111] LR [c016661c] _einittext+0x3f9d38a0/0x3fe4a284
[ 336.149114] Call Trace:
[ 336.149118] [f42ddbc0] [c07b9b60] 0xc07b9b60 (unreliable)
[ 336.149123] [f42ddbd0] [c013ff64] _einittext+0x3f9ad1e8/0x3fe4a284
[ 336.149128] [f42ddc10] [c0140d4c] _einittext+0x3f9adfd0/0x3fe4a284
[ 336.149133] [f42ddc90] [c002aadc] _einittext+0x3f897d60/0x3fe4a284
[ 336.149137] [f42ddce0] [c00153a4] _einittext+0x3f882628/0x3fe4a284
[ 336.149144] --- interrupt: 301 at _einittext+0x3fb52a50/0x3fe4a284
[ 336.149144] LR = _einittext+0x3fb52a4c/0x3fe4a284
[ 336.149148] [f42ddda8] [c02e56c0] _einittext+0x3fb52944/0x3fe4a284 (unreliable)
[ 336.149153] [f42ddde8] [c011644c] _einittext+0x3f9836d0/0x3fe4a284
[ 336.149158] [f42dde38] [c01f5950] _einittext+0x3fa62bd4/0x3fe4a284
[ 336.149163] [f42dde58] [c016b98c] _einittext+0x3f9d8c10/0x3fe4a284
[ 336.149167] [f42ddec8] [c016ba60] _einittext+0x3f9d8ce4/0x3fe4a284
[ 336.149172] [f42ddef8] [c016bd00] _einittext+0x3f9d8f84/0x3fe4a284
[ 336.149177] [f42ddf38] [c0015174] _einittext+0x3f8823f8/0x3fe4a284
[ 336.149182] --- interrupt: c01 at 0xfdf99fc
[ 336.149182] LR = 0xfd9cce0
[ 336.149184] Instruction dump:
[ 336.149189] 40be0018 4bffe359 3c80c06a 3884e48f 4bfd4c9d 0fe00000 4bffe345
7c641b78
[ 336.149196] 38600000 4bffe045 7c630034 5463d97e <0f030000> 39400000 393f001c
39600001
[ 336.149208] ---[ end trace d08833cae9c66ce3 ]---
[ 336.149210]
[ 336.193729] ------------[ cut here ]------------
This problem was bisected to commit 697ece7 ("powerpc/32s: reorder Linux PTE
bits to better match Hash PTE bits").
If I had done more rigorous tests earlier, I would have found the bug with more
time to fix it before release of 5.7, but every other problem I have found
happened at boot, not when the machine had to swap.
Thanks,
Larry
^ permalink raw reply
* Re: [linux-next RFC] mm/gup.c: Convert to use get_user_pages_fast_only()
From: Matthew Wilcox @ 2020-05-23 17:25 UTC (permalink / raw)
To: Souptick Joarder
Cc: mark.rutland, sfr, kvm, pbonzini, linux-mm, peterz, kvm-ppc,
linux-kernel, acme, rppt, alexander.shishkin, mingo, aneesh.kumar,
John Hubbard, namhyung, akpm, msuchanek, linuxppc-dev
In-Reply-To: <1590252072-2793-1-git-send-email-jrdr.linux@gmail.com>
On Sat, May 23, 2020 at 10:11:12PM +0530, Souptick Joarder wrote:
> Renaming the API __get_user_pages_fast() to get_user_pages_
> fast_only() to align with pin_user_pages_fast_only().
Please don't split a function name across lines. That messes
up people who are grepping for the function name in the changelog.
> As part of this we will get rid of write parameter.
> Instead caller will pass FOLL_WRITE to get_user_pages_fast_only().
> This will not change any existing functionality of the API.
>
> All the callers are changed to pass FOLL_WRITE.
>
> Updated the documentation of the API.
Everything you have done here is an improvement, and I'd be happy to
see it go in (after fixing the bug I note below).
But in reading through it, I noticed almost every user ...
> - if (__get_user_pages_fast(hva, 1, 1, &page) == 1) {
> + if (get_user_pages_fast_only(hva, 1, FOLL_WRITE, &page) == 1) {
passes '1' as the second parameter. So do we want to add:
static inline bool get_user_page_fast_only(unsigned long addr,
unsigned int gup_flags, struct page **pagep)
{
return get_user_pages_fast_only(addr, 1, gup_flags, pagep) == 1;
}
> @@ -2797,10 +2803,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> * FOLL_FAST_ONLY is required in order to match the API description of
> * this routine: no fall back to regular ("slow") GUP.
> */
> - unsigned int gup_flags = FOLL_GET | FOLL_FAST_ONLY;
> -
> - if (write)
> - gup_flags |= FOLL_WRITE;
> + gup_flags = FOLL_GET | FOLL_FAST_ONLY;
Er ... gup_flags |=, surely?
^ permalink raw reply
* Re: Kernel bug in 5.7 for PPC32 on PowerBook G4 - bisected to commit 697ece7
From: Christophe Leroy @ 2020-05-23 17:30 UTC (permalink / raw)
To: Larry Finger; +Cc: Paul Mackerras, ppc-dev, LKML
In-Reply-To: <2c361d8e-5e2a-cdd9-da8e-aa49a4f93cfd@lwfinger.net>
Hi Larry,
Le 23/05/2020 à 19:24, Larry Finger a écrit :
> Hi Christophe,
>
> Although kernel 5.7.0-rc2 appeared to boot cleanly, it failed on my G4
> when I tried to generate a new kernel. The following BUG message is logged:
>
[...]
>
> This problem was bisected to commit 697ece7 ("powerpc/32s: reorder Linux
> PTE bits to better match Hash PTE bits").
Being reversed in new -rc , see
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/87sgfsf4hs.fsf@mpe.ellerman.id.au/
>
> If I had done more rigorous tests earlier, I would have found the bug
> with more time to fix it before release of 5.7, but every other problem
> I have found happened at boot, not when the machine had to swap.
>
> Thanks,
>
> Larry
Christophe
^ permalink raw reply
* Re: [linux-next RFC] mm/gup.c: Convert to use get_user_pages_fast_only()
From: Souptick Joarder @ 2020-05-23 19:35 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Mark Rutland, Stephen Rothwell, kvm, pbonzini, Linux-MM,
Peter Zijlstra, kvm-ppc, linux-kernel, acme, Mike Rapoport,
Alexander Shishkin, Ingo Molnar, Aneesh Kumar K.V, John Hubbard,
namhyung, Andrew Morton, msuchanek, linuxppc-dev
In-Reply-To: <20200523172519.GA17206@bombadil.infradead.org>
On Sat, May 23, 2020 at 10:55 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Sat, May 23, 2020 at 10:11:12PM +0530, Souptick Joarder wrote:
> > Renaming the API __get_user_pages_fast() to get_user_pages_
> > fast_only() to align with pin_user_pages_fast_only().
>
> Please don't split a function name across lines. That messes
> up people who are grepping for the function name in the changelog.
Ok.
>
> > As part of this we will get rid of write parameter.
> > Instead caller will pass FOLL_WRITE to get_user_pages_fast_only().
> > This will not change any existing functionality of the API.
> >
> > All the callers are changed to pass FOLL_WRITE.
> >
> > Updated the documentation of the API.
>
> Everything you have done here is an improvement, and I'd be happy to
> see it go in (after fixing the bug I note below).
>
> But in reading through it, I noticed almost every user ...
>
> > - if (__get_user_pages_fast(hva, 1, 1, &page) == 1) {
> > + if (get_user_pages_fast_only(hva, 1, FOLL_WRITE, &page) == 1) {
>
> passes '1' as the second parameter. So do we want to add:
>
> static inline bool get_user_page_fast_only(unsigned long addr,
> unsigned int gup_flags, struct page **pagep)
> {
> return get_user_pages_fast_only(addr, 1, gup_flags, pagep) == 1;
> }
>
Yes, this can be added. Does get_user_page_fast_only() naming is fine ?
> > @@ -2797,10 +2803,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > * FOLL_FAST_ONLY is required in order to match the API description of
> > * this routine: no fall back to regular ("slow") GUP.
> > */
> > - unsigned int gup_flags = FOLL_GET | FOLL_FAST_ONLY;
> > -
> > - if (write)
> > - gup_flags |= FOLL_WRITE;
> > + gup_flags = FOLL_GET | FOLL_FAST_ONLY;
>
> Er ... gup_flags |=, surely?
Poor mistake.
@@ -1998,7 +1998,7 @@ int gfn_to_page_many_atomic(struct
kvm_memory_slot *slot, gfn_t gfn,
if (entry < nr_pages)
return 0;
- return __get_user_pages_fast(addr, nr_pages, 1, pages);
+ return get_user_pages_fast(addr, nr_pages, FOLL_WRITE, pages);
Also this needs to be corrected.
^ permalink raw reply
* Re: Kernel bug in 5.7 for PPC32 on PowerBook G4 - bisected to commit 697ece7
From: Larry Finger @ 2020-05-23 19:49 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Mackerras, ppc-dev, LKML
In-Reply-To: <3e3e2343-d674-02e0-7b23-81636b472641@csgroup.eu>
On 5/23/20 12:30 PM, Christophe Leroy wrote:
> Hi Larry,
>
> Le 23/05/2020 à 19:24, Larry Finger a écrit :
>> Hi Christophe,
>>
>> Although kernel 5.7.0-rc2 appeared to boot cleanly, it failed on my G4 when I
>> tried to generate a new kernel. The following BUG message is logged:
>>
>
> [...]
>
>>
>> This problem was bisected to commit 697ece7 ("powerpc/32s: reorder Linux PTE
>> bits to better match Hash PTE bits").
>
> Being reversed in new -rc , see
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/87sgfsf4hs.fsf@mpe.ellerman.id.au/
Christophe,
Thanks for the update.
Larry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox