* Re: [PATCH 1/2] mm: Fix struct page layout on 32-bit systems
From: Vineet Gupta @ 2021-04-20 21:14 UTC (permalink / raw)
To: Arnd Bergmann, Matthew Wilcox
Cc: grygorii.strashko@ti.com, linux-snps-arc@lists.infradead.org,
netdev@vger.kernel.org, Vineet Gupta, ilias.apalodimas@linaro.org,
linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org,
linux-mm@kvack.org, mgorman@suse.de, brouer@redhat.com,
mcroce@linux.microsoft.com, mhocko@kernel.org,
linuxppc-dev@lists.ozlabs.org, hch@lst.de,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAK8P3a0KUwf1Z0bHiUaHC2nHztevkxg5_FBSzHddNeSsBayWUA@mail.gmail.com>
On 4/20/21 12:07 AM, Arnd Bergmann wrote:
> On Tue, Apr 20, 2021 at 5:10 AM Matthew Wilcox <willy@infradead.org> wrote:
>> On Tue, Apr 20, 2021 at 02:48:17AM +0000, Vineet Gupta wrote:
>>>> 32-bit architectures which expect 8-byte alignment for 8-byte integers
>>>> and need 64-bit DMA addresses (arc, arm, mips, ppc) had their struct
>>>> page inadvertently expanded in 2019.
>>> FWIW, ARC doesn't require 8 byte alignment for 8 byte integers. This is
>>> only needed for 8-byte atomics due to the requirements of LLOCKD/SCOND
>>> instructions.
>> Ah, like x86? OK, great, I'll drop your arch from the list of
>> affected. Thanks!
> I mistakenly assumed that i386 and m68k were the only supported
> architectures with 32-bit alignment on u64. I checked it now and found
>
> $ for i in /home/arnd/cross/x86_64/gcc-10.1.0-nolibc/*/bin/*-gcc ; do
> echo `echo 'int a = __alignof__(long long);' | $i -xc - -Wall -S -o- |
> grep -A1 a: | tail -n 1 | cut -f 3 -d\ `
> ${i#/home/arnd/cross/x86_64/gcc-10.1.0-nolibc/*/bin/} ; done
> 8 aarch64-linux-gcc
> 8 alpha-linux-gcc
> 4 arc-linux-gcc
> 8 arm-linux-gnueabi-gcc
> 8 c6x-elf-gcc
> 4 csky-linux-gcc
> 4 h8300-linux-gcc
> 8 hppa-linux-gcc
> 8 hppa64-linux-gcc
> 8 i386-linux-gcc
> 8 ia64-linux-gcc
> 2 m68k-linux-gcc
> 4 microblaze-linux-gcc
> 8 mips-linux-gcc
> 8 mips64-linux-gcc
> 8 nds32le-linux-gcc
> 4 nios2-linux-gcc
> 4 or1k-linux-gcc
> 8 powerpc-linux-gcc
> 8 powerpc64-linux-gcc
> 8 riscv32-linux-gcc
> 8 riscv64-linux-gcc
> 8 s390-linux-gcc
> 4 sh2-linux-gcc
> 4 sh4-linux-gcc
> 8 sparc-linux-gcc
> 8 sparc64-linux-gcc
> 8 x86_64-linux-gcc
> 8 xtensa-linux-gcc
>
> which means that half the 32-bit architectures do this. This may
> cause more problems when arc and/or microblaze want to support
> 64-bit kernels and compat mode in the future on their latest hardware,
> as that means duplicating the x86 specific hacks we have for compat.
>
> What is alignof(u64) on 64-bit arc?
$ echo 'int a = __alignof__(long long);' | arc64-linux-gnu-gcc -xc -
-Wall -S -o - | grep -A1 a: | tail -n 1 | cut -f 3
8
Yeah ARCv2 alignment of 4 for 64-bit data was a bit of surprise finding
for me as well. When 64-bit load/stores were initially targeted by the
internal Metaware compiler (llvm based) they decided to keep alignment
to 4 still (granted hardware allowed this) and then gcc guys decided to
follow the same ABI. I only found this by accident :-)
Can you point me to some specifics on the compat issue. For better of
worse, arc64 does''t have a compat 32-bit mode, so everything is
64-on-64 or 32-on-32 (ARC32 flavor of ARCv3)
Thx,
-Vineet
^ permalink raw reply
* [PATCH V2 net] ibmvnic: Continue with reset if set link down failed
From: Dany Madden @ 2021-04-20 21:35 UTC (permalink / raw)
To: davem, kuba; +Cc: tlfalcon, netdev, paulus, drt, sukadev, linuxppc-dev
When ibmvnic gets a FATAL error message from the vnicserver, it marks
the Command Respond Queue (CRQ) inactive and resets the adapter. If this
FATAL reset fails and a transmission timeout reset follows, the CRQ is
still inactive, ibmvnic's attempt to set link down will also fail. If
ibmvnic abandons the reset because of this failed set link down and this
is the last reset in the workqueue, then this adapter will be left in an
inoperable state.
Instead, make the driver ignore this link down failure and continue to
free and re-register CRQ so that the adapter has an opportunity to
recover.
Fixes: ed651a10875f ("ibmvnic: Updated reset handling")
Signed-off-by: Dany Madden <drt@linux.ibm.com>
Reviewed-by: Rick Lindsley <ricklind@linux.ibm.com>
Reviewed-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
---
Changes in V2:
- Update description to clarify background for the patch
- Include Reviewed-by tags
---
drivers/net/ethernet/ibm/ibmvnic.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index ffb2a91750c7..4bd8c5d1a275 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1970,8 +1970,10 @@ static int do_reset(struct ibmvnic_adapter *adapter,
rtnl_unlock();
rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
rtnl_lock();
- if (rc)
- goto out;
+ if (rc) {
+ netdev_dbg(netdev,
+ "Setting link down failed rc=%d. Continue anyway\n", rc);
+ }
if (adapter->state == VNIC_OPEN) {
/* When we dropped rtnl, ibmvnic_open() got
--
2.26.2
^ permalink raw reply related
* Re: [PATCH V2 net] ibmvnic: Continue with reset if set link down failed
From: Lijun Pan @ 2021-04-20 21:42 UTC (permalink / raw)
To: Dany Madden
Cc: netdev, Tom Falcon, paulus, Jakub Kicinski, Sukadev Bhattiprolu,
linuxppc-dev, David Miller
In-Reply-To: <20210420213517.24171-1-drt@linux.ibm.com>
> On Apr 20, 2021, at 4:35 PM, Dany Madden <drt@linux.ibm.com> wrote:
>
> When ibmvnic gets a FATAL error message from the vnicserver, it marks
> the Command Respond Queue (CRQ) inactive and resets the adapter. If this
> FATAL reset fails and a transmission timeout reset follows, the CRQ is
> still inactive, ibmvnic's attempt to set link down will also fail. If
> ibmvnic abandons the reset because of this failed set link down and this
> is the last reset in the workqueue, then this adapter will be left in an
> inoperable state.
>
> Instead, make the driver ignore this link down failure and continue to
> free and re-register CRQ so that the adapter has an opportunity to
> recover.
This v2 does not adddress the concerns mentioned in v1.
And I think it is better to exit with error from do_reset, and schedule a thorough
do_hard_reset if the the adapter is already in unstable state.
>
> Fixes: ed651a10875f ("ibmvnic: Updated reset handling")
> Signed-off-by: Dany Madden <drt@linux.ibm.com>
> Reviewed-by: Rick Lindsley <ricklind@linux.ibm.com>
> Reviewed-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
> ---
> Changes in V2:
> - Update description to clarify background for the patch
> - Include Reviewed-by tags
> ---
> drivers/net/ethernet/ibm/ibmvnic.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index ffb2a91750c7..4bd8c5d1a275 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -1970,8 +1970,10 @@ static int do_reset(struct ibmvnic_adapter *adapter,
> rtnl_unlock();
> rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_DN);
> rtnl_lock();
> - if (rc)
> - goto out;
> + if (rc) {
> + netdev_dbg(netdev,
> + "Setting link down failed rc=%d. Continue anyway\n", rc);
> + }
>
> if (adapter->state == VNIC_OPEN) {
> /* When we dropped rtnl, ibmvnic_open() got
> --
> 2.26.2
>
^ permalink raw reply
* Re: [RFC v1 PATCH 1/3] drivers: soc: add support for soc_device_match returning -EPROBE_DEFER
From: Dan Carpenter @ 2021-04-20 11:21 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: ulf.hansson, aymen.sghaier, geert+renesas, rafael, airlied,
mturquette, dri-devel, linux-kernel, a.hajda, netdev, linux-phy,
peter.ujfalusi, linux-clk, linux-renesas-soc, wim, herbert,
horia.geanta, khilman, joro, narmstrong, linux-staging, iommu,
kishon, tony, linux-omap, stern, kuba, linus.walleij, linux,
linux-media, linux-watchdog, will, linux-pm, linuxppc-dev,
edubezval, linux-gpio, linux-mediatek, ssantosh, matthias.bgg,
linux-amlogic, mchehab, linux-arm-kernel, Alice Guo (OSS), balbi,
tomba, sboyd, gregkh, linux-usb, linux-mmc, adrian.hunter,
robert.foss, leoyang.li, linux, vkoul, Arnd Bergmann,
linux-crypto, daniel, j-keerthy, dmaengine, Roy.Pledge,
jyri.sarha, davem
In-Reply-To: <CAMuHMdUbrPxtJ9DCP0_nFrReuuO4vFY2J79LrKY82D7bCOfzRw@mail.gmail.com>
On Mon, Apr 19, 2021 at 10:20:13AM +0200, Geert Uytterhoeven wrote:
> Hi Alice,
>
> CC Arnd (soc_device_match() author)
>
> On Mon, Apr 19, 2021 at 6:28 AM Alice Guo (OSS) <alice.guo@oss.nxp.com> wrote:
> > From: Alice Guo <alice.guo@nxp.com>
> >
> > In i.MX8M boards, the registration of SoC device is later than caam
> > driver which needs it. Caam driver needs soc_device_match to provide
> > -EPROBE_DEFER when no SoC device is registered and no
> > early_soc_dev_attr.
>
> I'm wondering if this is really a good idea: soc_device_match() is a
> last-resort low-level check, and IMHO should be made available early on,
> so there is no need for -EPROBE_DEFER.
>
> >
> > Signed-off-by: Alice Guo <alice.guo@nxp.com>
>
> Thanks for your patch!
>
> > --- a/drivers/base/soc.c
> > +++ b/drivers/base/soc.c
> > @@ -110,6 +110,7 @@ static void soc_release(struct device *dev)
> > }
> >
> > static struct soc_device_attribute *early_soc_dev_attr;
> > +static bool soc_dev_attr_init_done = false;
>
> Do you need this variable?
>
> >
> > struct soc_device *soc_device_register(struct soc_device_attribute *soc_dev_attr)
> > {
> > @@ -157,6 +158,7 @@ struct soc_device *soc_device_register(struct soc_device_attribute *soc_dev_attr
> > return ERR_PTR(ret);
> > }
> >
> > + soc_dev_attr_init_done = true;
> > return soc_dev;
> >
> > out3:
> > @@ -246,6 +248,9 @@ const struct soc_device_attribute *soc_device_match(
> > if (!matches)
> > return NULL;
> >
> > + if (!soc_dev_attr_init_done && !early_soc_dev_attr)
>
> if (!soc_bus_type.p && !early_soc_dev_attr)
There is one place checking this already. We could wrap it in a helper
function:
static bool device_init_done(void)
{
return soc_bus_type.p ? true : false;
}
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 1/1] of/pci: Add IORESOURCE_MEM_64 to resource flags for 64-bit memory addresses
From: Rob Herring @ 2021-04-20 22:34 UTC (permalink / raw)
To: Leonardo Bras
Cc: devicetree, Alexey Kardashevskiy, Frank Rowand,
linux-kernel@vger.kernel.org, PCI, linuxppc-dev
In-Reply-To: <b56b8a5c8f02a2afea9554ebf16a423c182a9fc3.camel@gmail.com>
On Mon, Apr 19, 2021 at 9:03 PM Leonardo Bras <leobras.c@gmail.com> wrote:
>
> On Mon, 2021-04-19 at 20:39 -0500, Rob Herring wrote:
> > On Mon, Apr 19, 2021 at 7:35 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> > >
> > > On Mon, 2021-04-19 at 10:44 -0500, Rob Herring wrote:
> > > > On Fri, Apr 16, 2021 at 3:58 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> > > > >
> > > > > Hello Rob, thanks for this feedback!
> > > > >
> > > > > On Thu, 2021-04-15 at 13:59 -0500, Rob Herring wrote:
> > > > > > +PPC and PCI lists
> > > > > >
> > > > > > On Thu, Apr 15, 2021 at 1:01 PM Leonardo Bras <leobras.c@gmail.com> wrote:
> > > > > > >
> > > > > > > Many other resource flag parsers already add this flag when the input
> > > > > > > has bits 24 & 25 set, so update this one to do the same.
> > > > > >
> > > > > > Many others? Looks like sparc and powerpc to me.
> > > > > >
> > > > >
> > > > > s390 also does that, but it look like it comes from a device-tree.
> > > >
> > > > I'm only looking at DT based platforms, and s390 doesn't use DT.
> > >
> > > Correct.
> > > Sorry, I somehow write above the opposite of what I was thinking.
> > >
> > > >
> > > > > > Those would be the
> > > > > > ones I worry about breaking. Sparc doesn't use of/address.c so it's
> > > > > > fine. Powerpc version of the flags code was only fixed in 2019, so I
> > > > > > don't think powerpc will care either.
> > > > >
> > > > > In powerpc I reach this function with this stack, while configuring a
> > > > > virtio-net device for a qemu/KVM pseries guest:
> > > > >
> > > > > pci_process_bridge_OF_ranges+0xac/0x2d4
> > > > > pSeries_discover_phbs+0xc4/0x158
> > > > > discover_phbs+0x40/0x60
> > > > > do_one_initcall+0x60/0x2d0
> > > > > kernel_init_freeable+0x308/0x3a8
> > > > > kernel_init+0x2c/0x168
> > > > > ret_from_kernel_thread+0x5c/0x70
> > > > >
> > > > > For this, both MMIO32 and MMIO64 resources will have flags 0x200.
> > > >
> > > > Oh good, powerpc has 2 possible flags parsing functions. So in the
> > > > above path, do we need to set PCI_BASE_ADDRESS_MEM_TYPE_64?
> > > >
> > > > Does pci_parse_of_flags() get called in your case?
> > > >
> > >
> > > It's called in some cases, but not for the device I am debugging
> > > (virtio-net pci@800000020000000).
> > >
> > > For the above device, here is an expanded stack trace:
> > >
> > > of_bus_pci_get_flags() (from parser->bus->get_flags())
> > > of_pci_range_parser_one() (from macro for_each_of_pci_range)
> > > pci_process_bridge_OF_ranges+0xac/0x2d4
> > > pSeries_discover_phbs+0xc4/0x158
> > > discover_phbs+0x40/0x60
> > > do_one_initcall+0x60/0x2d0
> > > kernel_init_freeable+0x308/0x3a8
> > > kernel_init+0x2c/0x168
> > > ret_from_kernel_thread+0x5c/0x70
> > >
> > > For other devices, I could also see the following stack trace:
> > > ## device ethernet@8
> > >
> > > pci_parse_of_flags()
> > > of_create_pci_dev+0x7f0/0xa40
> > > __of_scan_bus+0x248/0x320
> > > pcibios_scan_phb+0x370/0x3b0
> > > pcibios_init+0x8c/0x12c
> > > do_one_initcall+0x60/0x2d0
> > > kernel_init_freeable+0x308/0x3a8
> > > kernel_init+0x2c/0x168
> > > ret_from_kernel_thread+0x5c/0x70
> > >
> > > Devices that get parsed with of_bus_pci_get_flags() appears first at
> > > dmesg (around 0.015s in my test), while devices that get parsed by
> > > pci_parse_of_flags() appears later (0.025s in my test).
> > >
> > > I am not really used to this code, but having the term "discover phbs"
> > > in the first trace and the term "scan phb" in the second, makes me
> > > wonder if the first trace is seen on devices that are seen/described in
> > > the device-tree and the second trace is seen in devices not present in
> > > the device-tree and found scanning pci bus.
> >
> > That was my guess as well. I think on pSeries that most PCI devices
> > are in the DT whereas on Arm and other flattened DT (non OpenFirmware)
> > platforms PCI devices are not in DT.
> >
>
> It makes sense to me.
>
> > Of course, for virtio devices,
> > they would not be in DT in either case.
>
> I don't get this part... in pseries it looks like virtio devices can be
> in device-tree.
>
> Oh, I think I get it... this pci@800000020000000 looks like a bus
> (described in device-tree, so discovered), and then the devices are
> inside it, getting scanned.
>
> The virtio device gets the correct flags (from pci_parse_of_flags), but
> the bus (pci@800000020000000) does not seem to get it correctly,
> because it comes from of_bus_pci_get_flags() which makes sense
> according to the name of the function.
>
> (see lspci bellow, output without patch)
>
>
> 00:08.0 Ethernet controller: Red Hat, Inc. Virtio network device (rev
> 01)
> Subsystem: Red Hat, Inc. Device 1100
> Device tree node:
> /sys/firmware/devicetree/base/pci@800000020000000/ethernet@8
> Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
> ParErr- Stepping- SERR+ FastB2B- DisINTx+
> Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
> <TAbort- <MAbort- >SERR- <PERR- INTx-
> Latency: 0
> Interrupt: pin A routed to IRQ 19
> IOMMU group: 0
> Region 1: Memory at 200080020000 (32-bit, non-prefetchable)
> [size=4K]
> Region 4: Memory at 210000010000 (64-bit, prefetchable)
> [size=16K]
> Expansion ROM at 200080040000 [disabled] [size=256K]
> Capabilities: [98] MSI-X: Enable+ Count=3 Masked-
> Vector table: BAR=1 offset=00000000
> PBA: BAR=1 offset=00000800
> Capabilities: [84] Vendor Specific Information: VirtIO:
> <unknown>
> BAR=0 offset=00000000 size=00000000
> Capabilities: [70] Vendor Specific Information: VirtIO: Notify
> BAR=4 offset=00003000 size=00001000 multiplier=00000004
> Capabilities: [60] Vendor Specific Information: VirtIO:
> DeviceCfg
> BAR=4 offset=00002000 size=00001000
> Capabilities: [50] Vendor Specific Information: VirtIO: ISR
> BAR=4 offset=00001000 size=00001000
> Capabilities: [40] Vendor Specific Information: VirtIO:
> CommonCfg
> BAR=4 offset=00000000 size=00001000
> Kernel driver in use: virtio-pci
>
>
> >
> > > > > > I noticed both sparc and powerpc set PCI_BASE_ADDRESS_MEM_TYPE_64 in
> > > > > > the flags. AFAICT, that's not set anywhere outside of arch code. So
> > > > > > never for riscv, arm and arm64 at least. That leads me to
> > > > > > pci_std_update_resource() which is where the PCI code sets BARs and
> > > > > > just copies the flags in PCI_BASE_ADDRESS_MEM_MASK ignoring
> > > > > > IORESOURCE_* flags. So it seems like 64-bit is still not handled and
> > > > > > neither is prefetch.
> > > > > >
> > > > >
> > > > > I am not sure if you mean here:
> > > > > a) it's ok to add IORESOURCE_MEM_64 here, because it does not affect
> > > > > anything else, or
> > > > > b) it should be using PCI_BASE_ADDRESS_MEM_TYPE_64
> > > > > (or IORESOURCE_MEM_64 | PCI_BASE_ADDRESS_MEM_TYPE_64) instead, since
> > > > > it's how it's added in powerpc/sparc, and else there is no point.
> > > >
> > > > I'm wondering if a) is incomplete and PCI_BASE_ADDRESS_MEM_TYPE_64
> > > > also needs to be set. The question is ultimately are BARs getting set
> > > > correctly for 64-bit? It looks to me like they aren't.
> > >
> > > I am not used to these terms, does BAR means 'Base Address Register'?
> >
> > Yes. Standard PCI thing.
>
> Nice :)
>
> >
> > > If so, those are the addresses stored in pci->phb->mem_resources[i] and
> > > pci->phb->mem_offset[i], printed from enable_ddw() (which takes place a
> > > lot after discovering the device (0.17s in my run)).
> > >
> > > resource #1 pci@800000020000000: start=0x200080000000
> > > end=0x2000ffffffff flags=0x200 desc=0x0 offset=0x200000000000
> > > resource #2 pci@800000020000000: start=0x210000000000
> > > end=0x21ffffffffff flags=0x200 desc=0x0 offset=0x0
> > >
> > > The message above was printed without this patch.
> > > With the patch, the flags for memory resource #2 gets ORed with
> > > 0x00100000.
> >
> > Right, as expected.
> >
> > > Is it enough to know if BARs are correctly set for 64-bit?
> >
> > No, because AFAICT, bit 2 in the BAR would not be set.
> >
> > > If it's not, how can I check?
> >
> > Can you try 'lspci -vv' and look at the 'Region X:' lines which will
> > say 32 or 64-bit. I *think* that should reflect what actually got
> > written into the BARs.
>
> As seen in the lspci from above comment:
> Region 1: Memory at 200080020000 (32-bit, non-prefetchable) [size=4K]
> Region 4: Memory at 210000010000 (64-bit, prefetchable) [size=16K]
>
> So it seems to be getting configured properly.
>
> I think the point here is bus resources not getting the MEM_64 flag,
> but device resources getting it correctly. Is that supposed to happen?
I experimented with this on Arm with qemu and it seems fine there too.
Looks like the BARs are first read and will have bit 2 set by default
(or hardwired?). Now I'm just wondering why powerpc needs the code it
has...
Anyways, I'll apply the patch.
Rob
^ permalink raw reply
* powerpc{32,64} randconfigs
From: Randy Dunlap @ 2021-04-20 23:31 UTC (permalink / raw)
To: PowerPC; +Cc: linux-kbuild
Hi,
Is there a way to do this?
$ make ARCH=powerpc randconfig # and force PPC32
and separately
$ make ARCH=powerpc randconfig # and force PPC64
I suppose that I could do something like this (for PPC64):
$ make ARCH=powerpc randconfig
$ ./scripts/config -d PPC32 -e PPC64
$ make ARCH=powerpc olddefconfig
$ make ARCH=powerpc all
but that's awkward and sort of defeats the randomness -- i.e.,
the randomness is done before the ARCH=PPC{32,64} is forced,
and then some of the config items won't be valid during the
'make olddefconfig'.
If the answer is No, then I will try to make a patch (at least
for local use) that does something like x86 does for
i386 | x86_64, or sparc does for sparc32 | sparc64.
[time passes]
OK, I have a patch that seems for work as far as setting
PPC32=y or PPC64=y... but it has a problem during linking
of vmlinux:
crosstool/gcc-9.3.0-nolibc/powerpc-linux/bin/powerpc-linux-ld:./arch/powerpc/kernel/vmlinux.lds:6: syntax error
and the (bad) generated vmlinux.lds file says (at line 6):
OUTPUT_ARCH(1:common)
while it should say:
OUTPUT_ARCH(powerpc:common)
Does anyone have any ideas about this problem?
The ARCH=ppc32 / ARCH=ppc64 patch is below FYI.
It is missing an update to Documentation/kbuild/kbuild.rst (aliases).
thanks.
--
---
Makefile | 8 ++++++++
arch/powerpc/Makefile | 14 +++++++-------
arch/powerpc/kernel/vdso64/Makefile | 2 +-
arch/powerpc/platforms/Kconfig.cputype | 5 +++--
4 files changed, 19 insertions(+), 10 deletions(-)
--- linux-next-20210420.orig/Makefile
+++ linux-next-20210420/Makefile
@@ -411,6 +411,14 @@ ifeq ($(ARCH),sh64)
SRCARCH := sh
endif
+# Additional ARCH settings for powerpc
+ifeq ($(ARCH),ppc32)
+ SRCARCH := powerpc
+endif
+ifeq ($(ARCH),ppc64)
+ SRCARCH := powerpc
+endif
+
export cross_compiling :=
ifneq ($(SRCARCH),$(SUBARCH))
cross_compiling := 1
--- linux-next-20210420.orig/arch/powerpc/platforms/Kconfig.cputype
+++ linux-next-20210420/arch/powerpc/platforms/Kconfig.cputype
@@ -1,11 +1,12 @@
# SPDX-License-Identifier: GPL-2.0
config PPC32
bool
- default y if !PPC64
+ default y if !PPC64 || "$(ARCH)" = "ppc32"
select KASAN_VMALLOC if KASAN && MODULES
config PPC64
- bool "64-bit kernel"
+ bool "64-bit kernel" if "$(ARCH)" = "powerpc"
+ default y if "$(ARCH)" = "ppc64"
select ZLIB_DEFLATE
help
This option selects whether a 32-bit or a 64-bit kernel
--- linux-next-20210420.orig/arch/powerpc/kernel/vdso64/Makefile
+++ linux-next-20210420/arch/powerpc/kernel/vdso64/Makefile
@@ -30,7 +30,7 @@ ccflags-y := -shared -fno-common -fno-bu
asflags-y := -D__VDSO64__ -s
targets += vdso64.lds
-CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)
+CPPFLAGS_vdso64.lds += -P -C -U$(SRCARCH)
# link rule for the .so file, .lds has to be first
$(obj)/vdso64.so.dbg: $(src)/vdso64.lds $(obj-vdso64) $(obj)/vgettimeofday.o FORCE
--- linux-next-20210420.orig/arch/powerpc/Makefile
+++ linux-next-20210420/arch/powerpc/Makefile
@@ -204,7 +204,7 @@ endif
asinstr := $(call as-instr,lis 9$(comma)foo@high,-DHAVE_AS_ATHIGH=1)
-KBUILD_CPPFLAGS += -I $(srctree)/arch/$(ARCH) $(asinstr)
+KBUILD_CPPFLAGS += -I $(srctree)/arch/$(SRCARCH) $(asinstr)
KBUILD_AFLAGS += $(AFLAGS-y)
KBUILD_CFLAGS += $(call cc-option,-msoft-float)
KBUILD_CFLAGS += -pipe $(CFLAGS-y)
@@ -279,7 +279,7 @@ BOOT_TARGETS2 := zImage% dtbImage% treeI
PHONY += $(BOOT_TARGETS1) $(BOOT_TARGETS2)
-boot := arch/$(ARCH)/boot
+boot := arch/$(SRCARCH)/boot
$(BOOT_TARGETS1): vmlinux
$(Q)$(MAKE) $(build)=$(boot) $(patsubst %,$(boot)/%,$@)
@@ -297,8 +297,8 @@ bootwrapper_install:
# without the .config suffix.
define merge_into_defconfig
$(Q)$(CONFIG_SHELL) $(srctree)/scripts/kconfig/merge_config.sh \
- -m -O $(objtree) $(srctree)/arch/$(ARCH)/configs/$(1) \
- $(foreach config,$(2),$(srctree)/arch/$(ARCH)/configs/$(config).config)
+ -m -O $(objtree) $(srctree)/arch/$(SRCARCH)/configs/$(1) \
+ $(foreach config,$(2),$(srctree)/arch/$(SRCARCH)/configs/$(config).config)
+$(Q)$(MAKE) -f $(srctree)/Makefile olddefconfig
endef
@@ -378,7 +378,7 @@ ppc64_book3e_allmodconfig:
define archhelp
@echo '* zImage - Build default images selected by kernel config'
- @echo ' zImage.* - Compressed kernel image (arch/$(ARCH)/boot/zImage.*)'
+ @echo ' zImage.* - Compressed kernel image (arch/$(SRCARCH)/boot/zImage.*)'
@echo ' uImage - U-Boot native image format'
@echo ' cuImage.<dt> - Backwards compatible U-Boot image for older'
@echo ' versions which do not support device trees'
@@ -389,12 +389,12 @@ define archhelp
@echo ' (your) ~/bin/$(INSTALLKERNEL) or'
@echo ' (distribution) /sbin/$(INSTALLKERNEL) or'
@echo ' install to $$(INSTALL_PATH) and run lilo'
- @echo ' *_defconfig - Select default config from arch/$(ARCH)/configs'
+ @echo ' *_defconfig - Select default config from arch/$(SRCARCH)/configs'
@echo ''
@echo ' Targets with <dt> embed a device tree blob inside the image'
@echo ' These targets support board with firmware that does not'
@echo ' support passing a device tree directly. Replace <dt> with the'
- @echo ' name of a dts file from the arch/$(ARCH)/boot/dts/ directory'
+ @echo ' name of a dts file from the arch/$(SRCARCH)/boot/dts/ directory'
@echo ' (minus the .dts extension).'
endef
^ permalink raw reply
* [PATCH] powerpc/64s: Add load address to plt branch targets before moved to linked location for non-relocatable kernels
From: Jordan Niethe @ 2021-04-21 2:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Jordan Niethe
Large branches will go through the plt which includes a stub that loads
a target address from the .branch_lt section. On a relocatable kernel the
targets in .branch_lt have relocations so they will be fixed up for
where the kernel is running by relocate().
For a non-relocatable kernel obviously there are no relocations.
However, until the kernel is moved down to its linked address it is
expected to be able to run where ever it is loaded. For pseries machines
prom_init() is called before running at the linked address.
Certain configs result in a large kernel such as STRICT_KERNEL_RWX
(because of the larger data shift):
config DATA_SHIFT
int "Data shift" if DATA_SHIFT_BOOL
default 24 if STRICT_KERNEL_RWX && PPC64
These large kernels lead to prom_init()'s final call to __start()
generating a plt branch:
bl c000000002000018 <00000078.plt_branch.__start>
This results in the kernel jumping to the linked address of __start,
0xc000000000000000, when really it needs to jump to the
0xc000000000000000 + the runtime address because the kernel is still
running at the load address.
The first 256 bytes are already copied to address 0 so the kernel will
run until
b __start_initialization_multiplatform
because there is nothing yet at __start_initialization_multiplatform
this will inevitably crash. At this point the exception handlers are
still OF's.
On phyp this will look like:
OF stdout device is: /vdevice/vty@30000000
Preparing to boot Linux version 5.12.0-rc3-63029-gada7d7e600c0 (gcc (GCC) 8.4.1 20200928 (Red Hat 8.4.1-1), GNU ld version 2.30-93.el8) #1 SMP Wed Apr 7 07:24:20 EDT 2021
Detected machine type: 0000000000000101
command line: BOOT_IMAGE=/vmlinuz-5.12.0-rc3-63029-gada7d7e600c0
Max number of cores passed to firmware: 256 (NR_CPUS = 2048)
Calling ibm,client-architecture-support... done
memory layout at init:
memory_limit : 0000000000000000 (16 MB aligned)
alloc_bottom : 000000000edc0000
alloc_top : 0000000020000000
alloc_top_hi : 0000000020000000
rmo_top : 0000000020000000
ram_top : 0000000020000000
instantiating rtas at 0x000000001ec30000... done
prom_hold_cpus: skipped
copying OF device tree...
Building dt strings...
Building dt structure...
Device tree strings 0x000000000edd0000 -> 0x000000000edd1809
Device tree struct 0x000000000ede0000 -> 0x000000000edf0000
Quiescing Open Firmware ...
Booting Linux via __start() @ 0x000000000a710000 ...
DEFAULT CATCH!, exception-handler=fffffffffffffff6
at %SRR0: 0000000000000f20 %SRR1: 8000000000081000
Open Firmware exception handler entered from non-OF code
Client's Fix Pt Regs:
00 000000000c713134 0000000008a9fc00 000000000caf9c00 000000000edc0000
04 000000000a710000 0000000000000000 0000000000000000 0000000000000000
08 0000000000000000 0000000000000000 000000000a7200fc 0000000000003003
0c c000000000000000 0000000000000000 0000000000000000 000000000b5a9820
10 000000000b5a9b38 000000000b5a9988 000000000b5a9f38 000000000b660c10
14 000000000b5a9f60 00000000013d0000 000000001ec30000 000000001ec30000
18 000000000b5a9840 000000000a710000 0000000000000028 000000000edc0008
1c 000000000edc0000 000000000cb60000 0000000000000000 000000000edc0000
Special Regs:
%IV: 00000700 %CR: 44000202 %XER: 00000000 %DSISR: 00000000
%SRR0: 0000000000000f20 %SRR1: 8000000000081000
%LR: 000000000c71326c %CTR: c000000000000000
%DAR: 0000000000000000
Virtual PID = 0
DEFAULT CATCH!, throw-code=fffffffffffffff6
Call History
------------
throw - c3f05c
$call-method - c4f0b4
(poplocals) - c40a00
key-fillq - c4f4cc
?xoff - c4f5b4
(poplocals) - c40a00
(stdout-write) - c4fa64
(emit) - c4fb3c
space - c4dfc8
quit - c5336c
quit - c53100
My Fix Pt Regs:
00 800000000000b002 0000000000000000 00000000deadbeef 0000000000c4f0b0
04 0000000008bfff80 00000000deadbeef 0000000000000004 0000000000c09010
08 0000000000000005 0000000000000000 0000000000000000 0000000000000000
0c 80000000072a40a8 0000000000000000 0000000000000000 0000000008d2cf30
10 0000000000e7d968 0000000000e7d968 0000000000c4f0a8 0000000000c4f0b4
14 fffffffffffffff6 0000000008bfff80 c8ff21fbd0ff41fb f8ffe1fbb1fd21f8
18 0000000000c19000 0000000000c3e000 0000000000c1af80 0000000000c1cfc0
1c 0000000000c26000 0000000000c460f0 0000000000c17fa8 0000000000c16fe0
Special Regs:
%IV: 00000900 %CR: 84800208 %XER: 00040010 %DSISR: 00000000
%SRR0: 0000000000c3eec8 %SRR1: 800000000000b002
%LR: 0000000000c3f05c %CTR: 0000000000c4f0b0
%DAR: 0000000000000000
...
On qemu it will just appear to be stuck after
Booting Linux via __start() @ 0x0000000000400000 ...:
SLOF **********************************************************************
QEMU Starting
Build Date = Apr 9 2021 14:13:31
FW Version = git-33a7322de13e9dca
Press "s" to enter Open Firmware.
Populating /vdevice methods
Populating /vdevice/vty@71000000
Populating /vdevice/nvram@71000001
Populating /vdevice/l-lan@71000002
Populating /vdevice/v-scsi@71000003
SCSI: Looking for devices
8200000000000000 CD-ROM : "QEMU QEMU CD-ROM 2.5+"
Populating /pci@800000020000000
Scanning USB
Using default console: /vdevice/vty@71000000
Detected RAM kernel at 400000 (25baa08 bytes)
Welcome to Open Firmware
Copyright (c) 2004, 2017 IBM Corporation All rights reserved.
This program and the accompanying materials are made available
under the terms of the BSD License available at
http://www.opensource.org/licenses/bsd-license.php
Booting from memory...
OF stdout device is: /vdevice/vty@71000000
Preparing to boot Linux version 5.12.0-rc3-00128-g87a8d2180282 (powerpc64le-linux-gnu-gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0, GNU ld (GNU Binutils for Ubuntu) 2.34) #85 SMP Sun Apr 18 19:30:55 AEST 2021
Detected machine type: 0000000000000101
command line: nokaslr
Max number of cores passed to firmware: 2048 (NR_CPUS = 2048)
Calling ibm,client-architecture-support... done
memory layout at init:
memory_limit : 0000000000000000 (16 MB aligned)
alloc_bottom : 0000000003df0000
alloc_top : 0000000030000000
alloc_top_hi : 0000000080000000
rmo_top : 0000000030000000
ram_top : 0000000080000000
instantiating rtas at 0x000000002fff0000... done
prom_hold_cpus: skipped
copying OF device tree...
Building dt strings...
Building dt structure...
Device tree strings 0x0000000003e00000 -> 0x0000000003e00ab2
Device tree struct 0x0000000003e10000 -> 0x0000000003e20000
Quiescing Open Firmware ...
Booting Linux via __start() @ 0x0000000000400000 ...
To fix this do some "relocation" of the plt target addresses on
non-relocatable before running at the linked address. Before calling
prom_init() add the runtime address to all the targets in .branch_lt
with relocate_plt(). Have relocate_plt() save the offset added in
p_branch_lt_off. After prom_init() calls __start() remove the offset
saved in p_branch_lt_off to return the targets to their original
addresses.
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
arch/powerpc/include/asm/sections.h | 2 +
arch/powerpc/kernel/head_64.S | 66 +++++++++++++++++++++++++++++
arch/powerpc/kernel/vmlinux.lds.S | 2 +
3 files changed, 70 insertions(+)
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index 324d7b298ec3..f087f5cd5a50 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -30,6 +30,8 @@ extern char __end_interrupts[];
extern char __prom_init_toc_start[];
extern char __prom_init_toc_end[];
+extern char __branch_lt_start[];
+extern char __branch_lt_end[];
#ifdef CONFIG_PPC_POWERNV
extern char start_real_trampolines[];
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index ece7f97bafff..28a6c2abd3ab 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -560,8 +560,11 @@ __boot_from_prom:
/* Relocate code for where we are now */
mr r3,r26
bl relocate
+#else
+ bl relocate_plt
#endif
+
/* Restore parameters */
mr r3,r31
mr r4,r30
@@ -600,6 +603,8 @@ __after_prom_start:
/* IVPR needs to be set after relocation. */
bl init_core_book3e
#endif
+#else
+ bl unrelocate_plt
#endif
/*
@@ -901,6 +906,67 @@ _GLOBAL(relative_toc)
.balign 8
p_toc: .8byte __toc_start + 0x8000 - 0b
+/*
+ * A large non relocatable kernel may generate branches that go though the plt,
+ * before the kernel is copied down to its link location, the target address in
+ * the .branch_lt section need to be offset with the run time address. The
+ * offset then needs to be removed before the kernel is running at the correct
+ * address. When relocate_plt is called the current runtime address is added
+ * to all of the target address in .branch_lt and that address is stored in
+ * p_branch_lt_off. When unrelocate_plt is called if there is an offset saved
+ * in p_branch_lt_off it is subtracted from the addresses in .branch_lt to
+ * return them to their original targets.
+ */
+#ifndef CONFIG_RELOCATABLE
+#define RELOCATE_MODE 0
+#define UNRELOCATE_MODE 1
+unrelocate_plt:
+ li r16,UNRELOCATE_MODE
+ b +8
+relocate_plt:
+ li r16,RELOCATE_MODE
+ mflr r0
+ bcl 20,31,$+4
+0: mflr r11
+
+ ld r12,(p_branch_lt_start - 0b)(r11)
+ add r12,r12,r11
+ ld r14,(p_branch_lt_end - 0b)(r11)
+ add r14,r14,r11
+ ld r15,(p_branch_lt_off - 0b)(r11)
+
+ /* Adding runtime address or subtracting p_branch_lt_off? */
+ cmpdi r16,UNRELOCATE_MODE
+ bne 5f
+ cmpdi r15,0
+ beq 4f
+ mr r10,r15
+ neg r10,r10
+ b 2f
+5: mr r10,r26
+
+ /* Iterate over all targets in .branch_lt */
+2: cmpd r12,r14
+ bge 6f
+ ld r13,0(r12)
+ add r13,r13, r10
+ std r13,0(r12)
+ addi r12,r12, 8
+ b 2b
+
+6: cmpdi r16,RELOCATE_MODE
+ bne 4f
+ std r26,(p_branch_lt_off - 0b)(r11)
+
+4: mtlr r0
+ blr
+
+.balign 8
+p_branch_lt_start: .8byte __branch_lt_start - 0b
+p_branch_lt_end: .8byte __branch_lt_end - 0b
+p_branch_lt_off: .8byte 0
+#endif
+
/*
* This is where the main kernel code starts.
*/
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index 72fa3c00229a..99085558ad3a 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -317,7 +317,9 @@ SECTIONS
#endif
*(.data.rel*)
*(.toc1)
+ __branch_lt_start = .;
*(.branch_lt)
+ __branch_lt_end = .;
}
.opd : AT(ADDR(.opd) - LOAD_OFFSET) {
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v10 06/10] powerpc/mm/ptdump: debugfs handler for W+X checks at runtime
From: Jordan Niethe @ 2021-04-21 2:23 UTC (permalink / raw)
To: Michael Ellerman
Cc: ajd, Kees Cook, cmr, Nicholas Piggin, naveen.n.rao, linuxppc-dev,
Daniel Axtens
In-Reply-To: <87im577eh3.fsf@mpe.ellerman.id.au>
On Wed, Mar 31, 2021 at 10:24 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Jordan Niethe <jniethe5@gmail.com> writes:
> > From: Russell Currey <ruscur@russell.cc>
> >
> > Optionally run W+X checks when dumping pagetable information to
> > debugfs' kernel_page_tables.
> >
> > To use:
> > $ echo 1 > /sys/kernel/debug/check_wx_pages
> > $ cat /sys/kernel/debug/kernel_page_tables
> >
> > and check the kernel log. Useful for testing strict module RWX.
> >
> > To disable W+X checks:
> > $ echo 0 > /sys/kernel/debug/check_wx_pages
> >
> > Update the Kconfig entry to reflect this.
> >
> > Also fix a typo.
> >
> > Reviewed-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > [jpn: Change check_wx_pages to act as mode bit affecting
> > kernel_page_tables instead of triggering action on its own]
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v10: check_wx_pages now affects kernel_page_tables rather then triggers
> > its own action.
>
> Hmm. I liked the old version better :)
>
> I think you changed it based on Christophe's comment:
>
> Why not just perform the test everytime someone dumps kernel_page_tables ?
>
>
> But I think he meant *always* do the check when someone dumps
> kernel_page_tables, not have another file to enable checking and then
> require someone to dump kernel_page_tables to do the actual check.
Yes, I guess I misinterpreted that.
>
> Still I like the previous version where you can do the checks
> separately, without having to dump the page tables, because dumping can
> sometimes take quite a while.
>
> What would be even better is if ptdump_check_wx() returned an error when
> wx pages were found, and that was plumbed out to the debugs file. That
> way you can script around it.
Ok I'll go back to how it was and add in returning an error.
>
> cheers
^ permalink raw reply
* Re: [PATCH v10 05/10] powerpc/bpf: Write protect JIT code
From: Jordan Niethe @ 2021-04-21 2:35 UTC (permalink / raw)
To: Michael Ellerman
Cc: ajd, cmr, Nicholas Piggin, naveen.n.rao, linuxppc-dev,
Daniel Axtens
In-Reply-To: <87wntnwqw9.fsf@mpe.ellerman.id.au>
On Wed, Mar 31, 2021 at 9:37 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Jordan Niethe <jniethe5@gmail.com> writes:
>
> > Once CONFIG_STRICT_MODULE_RWX is enabled there will be no need to
> > override bpf_jit_free() because it is now possible to set images
> > read-only. So use the default implementation.
> >
> > Also add the necessary call to bpf_jit_binary_lock_ro() which will
> > remove write protection and add exec protection to the JIT image after
> > it has finished being written.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v10: New to series
> > ---
> > arch/powerpc/net/bpf_jit_comp.c | 5 ++++-
> > arch/powerpc/net/bpf_jit_comp64.c | 4 ++++
> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
> > index e809cb5a1631..8015e4a7d2d4 100644
> > --- a/arch/powerpc/net/bpf_jit_comp.c
> > +++ b/arch/powerpc/net/bpf_jit_comp.c
> > @@ -659,12 +659,15 @@ void bpf_jit_compile(struct bpf_prog *fp)
> > bpf_jit_dump(flen, proglen, pass, code_base);
> >
> > bpf_flush_icache(code_base, code_base + (proglen/4));
> > -
> > #ifdef CONFIG_PPC64
> > /* Function descriptor nastiness: Address + TOC */
> > ((u64 *)image)[0] = (u64)code_base;
> > ((u64 *)image)[1] = local_paca->kernel_toc;
> > #endif
> > + if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) {
> > + set_memory_ro((unsigned long)image, alloclen >> PAGE_SHIFT);
> > + set_memory_x((unsigned long)image, alloclen >> PAGE_SHIFT);
> > + }
>
> You don't need to check the ifdef in a caller, there are stubs that
> compile to nothing when CONFIG_ARCH_HAS_SET_MEMORY=n.
As Christophe pointed out we could have !CONFIG_STRICT_MODULE_RWX and
CONFIG_ARCH_HAS_SET_MEMORY which would then be wrong here.
Probably we could make CONFIG_ARCH_HAS_SET_MEMORY depend on
CONFIG_STRICT_MODULE_RWX?
>
> > diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> > index aaf1a887f653..1484ad588685 100644
> > --- a/arch/powerpc/net/bpf_jit_comp64.c
> > +++ b/arch/powerpc/net/bpf_jit_comp64.c
> > @@ -1240,6 +1240,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> > fp->jited_len = alloclen;
> >
> > bpf_flush_icache(bpf_hdr, (u8 *)bpf_hdr + (bpf_hdr->pages * PAGE_SIZE));
> > + if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
> > + bpf_jit_binary_lock_ro(bpf_hdr);
>
> Do we need the ifdef here either? Looks like it should be safe to call
> due to the stubs.
>
> > @@ -1262,6 +1264,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> > }
> >
> > /* Overriding bpf_jit_free() as we don't set images read-only. */
> > +#ifndef CONFIG_STRICT_MODULE_RWX
>
> Did you test without this and notice something broken?
>
> Looking at the generic version I can't tell why we need to override
> this. Maybe we don't (anymore?) ?
Yeah we don't.
>
> cheers
>
> > void bpf_jit_free(struct bpf_prog *fp)
> > {
> > unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
> > @@ -1272,3 +1275,4 @@ void bpf_jit_free(struct bpf_prog *fp)
> >
> > bpf_prog_unlock_free(fp);
> > }
> > +#endif
> > --
> > 2.25.1
^ permalink raw reply
* Re: [PATCH v10 08/10] powerpc/configs: Enable STRICT_MODULE_RWX in skiroot_defconfig
From: Jordan Niethe @ 2021-04-21 2:37 UTC (permalink / raw)
To: Christophe Leroy
Cc: ajd, Joel Stanley, Nicholas Piggin, cmr, naveen.n.rao,
linuxppc-dev, Daniel Axtens
In-Reply-To: <c511660d-64e3-30fa-9324-8019928545d5@csgroup.eu>
On Tue, Mar 30, 2021 at 4:27 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 30/03/2021 à 06:51, Jordan Niethe a écrit :
> > From: Russell Currey <ruscur@russell.cc>
> >
> > skiroot_defconfig is the only powerpc defconfig with STRICT_KERNEL_RWX
> > enabled, and if you want memory protection for kernel text you'd want it
> > for modules too, so enable STRICT_MODULE_RWX there.
>
> Maybe we could now selectt ARCH_OPTIONAL_KERNEL_RWX_DEFAULT in arch/powerpc/Kconfig.
>
> Then this change would not be necessary.
>
> Would be in line with https://github.com/linuxppc/issues/issues/223
Yes, I think that is the way to go.
>
>
> >
> > Acked-by: Joel Stanley <joel@joel.id.au>
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > arch/powerpc/configs/skiroot_defconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/powerpc/configs/skiroot_defconfig b/arch/powerpc/configs/skiroot_defconfig
> > index b806a5d3a695..50fe06cb3a31 100644
> > --- a/arch/powerpc/configs/skiroot_defconfig
> > +++ b/arch/powerpc/configs/skiroot_defconfig
> > @@ -50,6 +50,7 @@ CONFIG_CMDLINE="console=tty0 console=hvc0 ipr.fast_reboot=1 quiet"
> > # CONFIG_PPC_MEM_KEYS is not set
> > CONFIG_JUMP_LABEL=y
> > CONFIG_STRICT_KERNEL_RWX=y
> > +CONFIG_STRICT_MODULE_RWX=y
> > CONFIG_MODULES=y
> > CONFIG_MODULE_UNLOAD=y
> > CONFIG_MODULE_SIG_FORCE=y
> >
^ permalink raw reply
* Re: [PATCH v10 04/10] powerpc/kprobes: Mark newly allocated probes as ROX
From: Jordan Niethe @ 2021-04-21 2:39 UTC (permalink / raw)
To: Christophe Leroy
Cc: ajd, Nicholas Piggin, cmr, naveen.n.rao, linuxppc-dev,
Daniel Axtens
In-Reply-To: <d61a1b2f-4a22-fb43-e8fe-a7612431c4ad@csgroup.eu>
On Tue, Mar 30, 2021 at 4:05 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 30/03/2021 à 06:51, Jordan Niethe a écrit :
> > From: Russell Currey <ruscur@russell.cc>
> >
> > Add the arch specific insn page allocator for powerpc. This allocates
> > ROX pages if STRICT_KERNEL_RWX is enabled. These pages are only written
> > to with patch_instruction() which is able to write RO pages.
> >
> > Reviewed-by: Daniel Axtens <dja@axtens.net>
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > [jpn: Reword commit message, switch to __vmalloc_node_range()]
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v9: - vmalloc_exec() no longer exists
> > - Set the page to RW before freeing it
> > v10: - use __vmalloc_node_range()
> > ---
> > arch/powerpc/kernel/kprobes.c | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> > index 01ab2163659e..3ae27af9b094 100644
> > --- a/arch/powerpc/kernel/kprobes.c
> > +++ b/arch/powerpc/kernel/kprobes.c
> > @@ -25,6 +25,7 @@
> > #include <asm/sections.h>
> > #include <asm/inst.h>
> > #include <linux/uaccess.h>
> > +#include <linux/vmalloc.h>
> >
> > DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
> > DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
> > @@ -103,6 +104,19 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
> > return addr;
> > }
> >
> > +void *alloc_insn_page(void)
> > +{
> > + if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) {
> > + return __vmalloc_node_range(PAGE_SIZE, 1, MODULES_VADDR, MODULES_END,
> > + GFP_KERNEL, PAGE_KERNEL_ROX, VM_FLUSH_RESET_PERMS,
> > + NUMA_NO_NODE, __builtin_return_address(0));
> > + } else {
> > + return __vmalloc_node_range(PAGE_SIZE, 1, MODULES_VADDR, MODULES_END,
> > + GFP_KERNEL, PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS,
> > + NUMA_NO_NODE, __builtin_return_address(0));
> > + }
> > +}
> > +
>
> What about
>
> void *alloc_insn_page(void)
> {
> pgprot_t prot = IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
>
> return __vmalloc_node_range(PAGE_SIZE, 1, MODULES_VADDR, MODULES_END,
> GFP_KERNEL, prot, VM_FLUSH_RESET_PERMS,
> NUMA_NO_NODE, __builtin_return_address(0));
> }
Yes, that is better.
>
> > int arch_prepare_kprobe(struct kprobe *p)
> > {
> > int ret = 0;
> >
^ permalink raw reply
* Re: [PATCH v10 03/10] powerpc: Always define MODULES_{VADDR,END}
From: Jordan Niethe @ 2021-04-21 2:46 UTC (permalink / raw)
To: Christophe Leroy
Cc: ajd, Nicholas Piggin, cmr, naveen.n.rao, linuxppc-dev,
Daniel Axtens
In-Reply-To: <c4977cfb-6d9d-063e-b74b-ba0ffd9feaeb@csgroup.eu>
On Fri, Apr 2, 2021 at 12:36 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 30/03/2021 à 06:51, Jordan Niethe a écrit :
> > If MODULES_{VADDR,END} are not defined set them to VMALLOC_START and
> > VMALLOC_END respectively. This reduces the need for special cases. For
> > example, powerpc's module_alloc() was previously predicated on
> > MODULES_VADDR being defined but now is unconditionally defined.
> >
> > This will be useful reducing conditional code in other places that need
> > to allocate from the module region (i.e., kprobes).
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v10: New to series
> > ---
> > arch/powerpc/include/asm/pgtable.h | 5 +++++
> > arch/powerpc/kernel/module.c | 5 +----
>
> You probably also have changes to do in kernel/ptdump.c
>
> In mm/book3s32/mmu.c and mm/kasan/kasan_init_32.c as well allthough that's harmless here.
>
> > 2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> > index 4eed82172e33..014c2921f26a 100644
> > --- a/arch/powerpc/include/asm/pgtable.h
> > +++ b/arch/powerpc/include/asm/pgtable.h
> > @@ -167,6 +167,11 @@ struct seq_file;
> > void arch_report_meminfo(struct seq_file *m);
> > #endif /* CONFIG_PPC64 */
> >
> > +#ifndef MODULES_VADDR
> > +#define MODULES_VADDR VMALLOC_START
> > +#define MODULES_END VMALLOC_END
> > +#endif
> > +
> > #endif /* __ASSEMBLY__ */
> >
> > #endif /* _ASM_POWERPC_PGTABLE_H */
> > diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
> > index a211b0253cdb..f1fb58389d58 100644
> > --- a/arch/powerpc/kernel/module.c
> > +++ b/arch/powerpc/kernel/module.c
> > @@ -14,6 +14,7 @@
> > #include <asm/firmware.h>
> > #include <linux/sort.h>
> > #include <asm/setup.h>
> > +#include <linux/mm.h>
> >
> > static LIST_HEAD(module_bug_list);
> >
> > @@ -87,13 +88,9 @@ int module_finalize(const Elf_Ehdr *hdr,
> > return 0;
> > }
> >
> > -#ifdef MODULES_VADDR
> > void *module_alloc(unsigned long size)
> > {
> > - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
> > -
>
> The above check is needed somewhere, if you remove it from here you have to perform the check
> somewhere else.
This also introduces this warning:
fs/proc/kcore.c:626:52: warning: self-comparison always evaluates to
false [-Wtautological-compare]
626 | if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
I might leave this patch out of this series and use an #ifdef for now
and make this change separately as a follow up.
>
> > return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL,
> > PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
> > __builtin_return_address(0));
> > }
> > -#endif
> >
^ permalink raw reply
* Re: [PATCH v10 01/10] powerpc/mm: Implement set_memory() routines
From: Jordan Niethe @ 2021-04-21 2:51 UTC (permalink / raw)
To: Christophe Leroy
Cc: ajd, Nicholas Piggin, cmr, naveen.n.rao, linuxppc-dev,
Daniel Axtens
In-Reply-To: <1503da9e-b74e-b6d0-b5f3-a1648270e930@csgroup.eu>
On Tue, Mar 30, 2021 at 4:16 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 30/03/2021 à 06:51, Jordan Niethe a écrit :
> > From: Russell Currey <ruscur@russell.cc>
> >
> > The set_memory_{ro/rw/nx/x}() functions are required for STRICT_MODULE_RWX,
> > and are generally useful primitives to have. This implementation is
> > designed to be completely generic across powerpc's many MMUs.
> >
> > It's possible that this could be optimised to be faster for specific
> > MMUs, but the focus is on having a generic and safe implementation for
> > now.
> >
> > This implementation does not handle cases where the caller is attempting
> > to change the mapping of the page it is executing from, or if another
> > CPU is concurrently using the page being altered. These cases likely
> > shouldn't happen, but a more complex implementation with MMU-specific code
> > could safely handle them, so that is left as a TODO for now.
> >
> > On hash the linear mapping is not kept in the linux pagetable, so this
> > will not change the protection if used on that range. Currently these
> > functions are not used on the linear map so just WARN for now.
> >
> > These functions do nothing if STRICT_KERNEL_RWX is not enabled.
> >
> > Reviewed-by: Daniel Axtens <dja@axtens.net>
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > [jpn: -rebase on next plus "powerpc/mm/64s: Allow STRICT_KERNEL_RWX again"
> > - WARN on hash linear map]
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v10: WARN if trying to change the hash linear map
> > ---
> > arch/powerpc/Kconfig | 1 +
> > arch/powerpc/include/asm/set_memory.h | 32 ++++++++++
> > arch/powerpc/mm/Makefile | 2 +-
> > arch/powerpc/mm/pageattr.c | 88 +++++++++++++++++++++++++++
> > 4 files changed, 122 insertions(+), 1 deletion(-)
> > create mode 100644 arch/powerpc/include/asm/set_memory.h
> > create mode 100644 arch/powerpc/mm/pageattr.c
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index fc7f5c5933e6..4498a27ac9db 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -135,6 +135,7 @@ config PPC
> > select ARCH_HAS_MEMBARRIER_CALLBACKS
> > select ARCH_HAS_MEMBARRIER_SYNC_CORE
> > select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
> > + select ARCH_HAS_SET_MEMORY
> > select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !HIBERNATION)
> > select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> > select ARCH_HAS_UACCESS_FLUSHCACHE
> > diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
> > new file mode 100644
> > index 000000000000..64011ea444b4
> > --- /dev/null
> > +++ b/arch/powerpc/include/asm/set_memory.h
> > @@ -0,0 +1,32 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _ASM_POWERPC_SET_MEMORY_H
> > +#define _ASM_POWERPC_SET_MEMORY_H
> > +
> > +#define SET_MEMORY_RO 0
> > +#define SET_MEMORY_RW 1
> > +#define SET_MEMORY_NX 2
> > +#define SET_MEMORY_X 3
> > +
> > +int change_memory_attr(unsigned long addr, int numpages, long action);
> > +
> > +static inline int set_memory_ro(unsigned long addr, int numpages)
> > +{
> > + return change_memory_attr(addr, numpages, SET_MEMORY_RO);
> > +}
> > +
> > +static inline int set_memory_rw(unsigned long addr, int numpages)
> > +{
> > + return change_memory_attr(addr, numpages, SET_MEMORY_RW);
> > +}
> > +
> > +static inline int set_memory_nx(unsigned long addr, int numpages)
> > +{
> > + return change_memory_attr(addr, numpages, SET_MEMORY_NX);
> > +}
> > +
> > +static inline int set_memory_x(unsigned long addr, int numpages)
> > +{
> > + return change_memory_attr(addr, numpages, SET_MEMORY_X);
> > +}
> > +
> > +#endif
> > diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> > index 3b4e9e4e25ea..d8a08abde1ae 100644
> > --- a/arch/powerpc/mm/Makefile
> > +++ b/arch/powerpc/mm/Makefile
> > @@ -5,7 +5,7 @@
> >
> > ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
> >
> > -obj-y := fault.o mem.o pgtable.o mmap.o maccess.o \
> > +obj-y := fault.o mem.o pgtable.o mmap.o maccess.o pageattr.o \
> > init_$(BITS).o pgtable_$(BITS).o \
> > pgtable-frag.o ioremap.o ioremap_$(BITS).o \
> > init-common.o mmu_context.o drmem.o
> > diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
> > new file mode 100644
> > index 000000000000..9efcb01088da
> > --- /dev/null
> > +++ b/arch/powerpc/mm/pageattr.c
> > @@ -0,0 +1,88 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * MMU-generic set_memory implementation for powerpc
> > + *
> > + * Copyright 2019, IBM Corporation.
> > + */
> > +
> > +#include <linux/mm.h>
> > +#include <linux/set_memory.h>
> > +
> > +#include <asm/mmu.h>
> > +#include <asm/page.h>
> > +#include <asm/pgtable.h>
> > +
> > +
> > +/*
> > + * Updates the attributes of a page in three steps:
> > + *
> > + * 1. invalidate the page table entry
> > + * 2. flush the TLB
> > + * 3. install the new entry with the updated attributes
> > + *
> > + * This is unsafe if the caller is attempting to change the mapping of the
> > + * page it is executing from, or if another CPU is concurrently using the
> > + * page being altered.
> > + *
> > + * TODO make the implementation resistant to this.
> > + *
> > + * NOTE: can be dangerous to call without STRICT_KERNEL_RWX
> > + */
> > +static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
> > +{
> > + long action = (long)data;
> > + pte_t pte;
> > +
> > + spin_lock(&init_mm.page_table_lock);
> > +
> > + /* invalidate the PTE so it's safe to modify */
> > + pte = ptep_get_and_clear(&init_mm, addr, ptep);
> > + flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> > +
> > + /* modify the PTE bits as desired, then apply */
> > + switch (action) {
> > + case SET_MEMORY_RO:
> > + pte = pte_wrprotect(pte);
> > + break;
> > + case SET_MEMORY_RW:
> > + pte = pte_mkwrite(pte);
> > + break;
> > + case SET_MEMORY_NX:
> > + pte = pte_exprotect(pte);
> > + break;
> > + case SET_MEMORY_X:
> > + pte = pte_mkexec(pte);
> > + break;
> > + default:
> > + WARN_ON_ONCE(1);
> > + break;
> > + }
> > +
> > + set_pte_at(&init_mm, addr, ptep, pte);
> > + spin_unlock(&init_mm.page_table_lock);
> > +
> > + return 0;
> > +}
> > +
> > +int change_memory_attr(unsigned long addr, int numpages, long action)
> > +{
> > + unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
> > + unsigned long sz = numpages * PAGE_SIZE;
> > +
> > + if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> > + return 0;
>
> You should do this in the header file in order to get it optimised out completely when
> CONFIG_STRICT_KERNEL_RWX is not set.
>
> In asm/set_memory.h you could have:
>
> #ifdef CONFIG_STRICT_KERNEL_RWX
> int change_memory_attr(unsigned long addr, int numpages, long action);
> #else
> static inline int change_memory_attr(unsigned long addr, int numpages, long action) { return 0; }
> #endif
>
> Or another solution is to only define ARCH_HAS_SET_MEMORY when CONFIG_STRICT_KERNEL_RWX is selected.
I think making ARCH_HAS_SET_MEMORY depend on CONFIG_STRICT_KERNEL_RWX
is the way to go.
>
> > +
> > + if (numpages <= 0)
> > + return 0;
> > +
> > +#ifdef CONFIG_PPC_BOOK3S_64
> > + if (WARN_ON_ONCE(!radix_enabled() &&
> > + get_region_id(addr) == LINEAR_MAP_REGION_ID)) {
> > + return -1;
> > + }
> > +#endif
> > +
> > + return apply_to_existing_page_range(&init_mm, start, sz,
> > + change_page_attr, (void *)action);
> > +}
> >
^ permalink raw reply
* [PATCH v2] powerpc: make ALTIVEC select PPC_FPU
From: Randy Dunlap @ 2021-04-21 2:56 UTC (permalink / raw)
To: linux-kernel; +Cc: kernel test robot, Randy Dunlap, linuxppc-dev
On a kernel config with ALTIVEC=y and PPC_FPU not set/enabled,
there are build errors:
drivers/cpufreq/pmac32-cpufreq.c:262:2: error: implicit declaration of function 'enable_kernel_fp' [-Werror,-Wimplicit-function-declaration]
enable_kernel_fp();
../arch/powerpc/lib/sstep.c: In function 'do_vec_load':
../arch/powerpc/lib/sstep.c:637:3: error: implicit declaration of function 'put_vr' [-Werror=implicit-function-declaration]
637 | put_vr(rn, &u.v);
| ^~~~~~
../arch/powerpc/lib/sstep.c: In function 'do_vec_store':
../arch/powerpc/lib/sstep.c:660:3: error: implicit declaration of function 'get_vr'; did you mean 'get_oc'? [-Werror=implicit-function-declaration]
660 | get_vr(rn, &u.v);
| ^~~~~~
In theory ALTIVEC is independent of PPC_FPU but in practice nobody
is going to build such a machine, so make ALTIVEC require PPC_FPU
by selecting it.
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: kernel test robot <lkp@intel.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Segher Boessenkool <segher@kernel.crashing.org>
Cc: lkp@intel.com
---
v2: change ALTIVEC depends on PPC_FPU to select (Christophe and Michael)
arch/powerpc/platforms/86xx/Kconfig | 1 +
arch/powerpc/platforms/Kconfig.cputype | 2 ++
2 files changed, 3 insertions(+)
--- linux-next-20210416.orig/arch/powerpc/platforms/86xx/Kconfig
+++ linux-next-20210416/arch/powerpc/platforms/86xx/Kconfig
@@ -4,6 +4,7 @@ menuconfig PPC_86xx
bool "86xx-based boards"
depends on PPC_BOOK3S_32
select FSL_SOC
+ select PPC_FPU
select ALTIVEC
help
The Freescale E600 SoCs have 74xx cores.
--- linux-next-20210416.orig/arch/powerpc/platforms/Kconfig.cputype
+++ linux-next-20210416/arch/powerpc/platforms/Kconfig.cputype
@@ -186,6 +186,7 @@ config E300C3_CPU
config G4_CPU
bool "G4 (74xx)"
depends on PPC_BOOK3S_32
+ select PPC_FPU
select ALTIVEC
endchoice
@@ -310,6 +311,7 @@ config PHYS_64BIT
config ALTIVEC
bool "AltiVec Support"
depends on PPC_BOOK3S_32 || PPC_BOOK3S_64 || (PPC_E500MC && PPC64)
+ select PPC_FPU
help
This option enables kernel support for the Altivec extensions to the
PowerPC processor. The kernel currently supports saving and restoring
^ permalink raw reply
* Re: [PATCH v10 01/10] powerpc/mm: Implement set_memory() routines
From: Jordan Niethe @ 2021-04-21 5:03 UTC (permalink / raw)
To: Michael Ellerman
Cc: ajd, cmr, Nicholas Piggin, naveen.n.rao, linuxppc-dev,
Daniel Axtens
In-Reply-To: <87lfa37euw.fsf@mpe.ellerman.id.au>
On Wed, Mar 31, 2021 at 10:16 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Hi Jordan,
>
> A few nits below ...
>
> Jordan Niethe <jniethe5@gmail.com> writes:
> > From: Russell Currey <ruscur@russell.cc>
> >
> > The set_memory_{ro/rw/nx/x}() functions are required for STRICT_MODULE_RWX,
> > and are generally useful primitives to have. This implementation is
> > designed to be completely generic across powerpc's many MMUs.
> >
> > It's possible that this could be optimised to be faster for specific
> > MMUs, but the focus is on having a generic and safe implementation for
> > now.
> >
> > This implementation does not handle cases where the caller is attempting
> > to change the mapping of the page it is executing from, or if another
> > CPU is concurrently using the page being altered. These cases likely
> > shouldn't happen, but a more complex implementation with MMU-specific code
> > could safely handle them, so that is left as a TODO for now.
> >
> > On hash the linear mapping is not kept in the linux pagetable, so this
> > will not change the protection if used on that range. Currently these
> > functions are not used on the linear map so just WARN for now.
> >
> > These functions do nothing if STRICT_KERNEL_RWX is not enabled.
> >
> > Reviewed-by: Daniel Axtens <dja@axtens.net>
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > [jpn: -rebase on next plus "powerpc/mm/64s: Allow STRICT_KERNEL_RWX again"
> > - WARN on hash linear map]
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v10: WARN if trying to change the hash linear map
> > ---
> > arch/powerpc/Kconfig | 1 +
> > arch/powerpc/include/asm/set_memory.h | 32 ++++++++++
> > arch/powerpc/mm/Makefile | 2 +-
> > arch/powerpc/mm/pageattr.c | 88 +++++++++++++++++++++++++++
> > 4 files changed, 122 insertions(+), 1 deletion(-)
> > create mode 100644 arch/powerpc/include/asm/set_memory.h
> > create mode 100644 arch/powerpc/mm/pageattr.c
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index fc7f5c5933e6..4498a27ac9db 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -135,6 +135,7 @@ config PPC
> > select ARCH_HAS_MEMBARRIER_CALLBACKS
> > select ARCH_HAS_MEMBARRIER_SYNC_CORE
> > select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
> > + select ARCH_HAS_SET_MEMORY
>
> Below you do:
>
> if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> return 0;
>
> Which suggests we should instead just only select ARCH_HAS_SET_MEMORY if
> STRICT_KERNEL_RWX ?
Yeah, I'm just going to do that.
>
>
> > diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> > index 3b4e9e4e25ea..d8a08abde1ae 100644
> > --- a/arch/powerpc/mm/Makefile
> > +++ b/arch/powerpc/mm/Makefile
> > @@ -5,7 +5,7 @@
> >
> > ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
> >
> > -obj-y := fault.o mem.o pgtable.o mmap.o maccess.o \
> > +obj-y := fault.o mem.o pgtable.o mmap.o maccess.o pageattr.o \
>
> .. and then the file should only be built if ARCH_HAS_SET_MEMORY = y.
>
> > init_$(BITS).o pgtable_$(BITS).o \
> > pgtable-frag.o ioremap.o ioremap_$(BITS).o \
> > init-common.o mmu_context.o drmem.o
> > diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
> > new file mode 100644
> > index 000000000000..9efcb01088da
> > --- /dev/null
> > +++ b/arch/powerpc/mm/pageattr.c
> > @@ -0,0 +1,88 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * MMU-generic set_memory implementation for powerpc
> > + *
> > + * Copyright 2019, IBM Corporation.
>
> Should be 2019-2021.
Right.
>
> > + */
> > +
> > +#include <linux/mm.h>
> > +#include <linux/set_memory.h>
> > +
> > +#include <asm/mmu.h>
> > +#include <asm/page.h>
> > +#include <asm/pgtable.h>
> > +
> > +
> > +/*
> > + * Updates the attributes of a page in three steps:
> > + *
> > + * 1. invalidate the page table entry
> > + * 2. flush the TLB
> > + * 3. install the new entry with the updated attributes
> > + *
> > + * This is unsafe if the caller is attempting to change the mapping of the
> > + * page it is executing from, or if another CPU is concurrently using the
> > + * page being altered.
>
> Is the 2nd part of that statement true?
>
> Or, I guess maybe it is true depending on what "unsafe" means.
>
> AIUI it's unsafe to use this on the page you're executing from, and by
> unsafe we mean the kernel will potentially crash because it will lose
> the mapping for the currently executing text.
>
> Using this on a page that another CPU is accessing could be safe, if eg.
> the other CPU is reading from the page and we are just changing it from
> RW->RO.
>
> So I'm not sure they're the same type of "unsafe".
I think the comment was prompted by your message here:
https://lore.kernel.org/linuxppc-dev/87pnio5fva.fsf@mpe.ellerman.id.au/
So I'll rewrite the comment to separate the two cases and indicate the
2nd case only might be an issue.
>
> > + * TODO make the implementation resistant to this.
> > + *
> > + * NOTE: can be dangerous to call without STRICT_KERNEL_RWX
>
> I don't think we need that anymore?
No we don't, change_memory_attr() won't call it without STRICT_KERNEL_RWX.
>
> > + */
> > +static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
> > +{
> > + long action = (long)data;
> > + pte_t pte;
> > +
> > + spin_lock(&init_mm.page_table_lock);
> > +
> > + /* invalidate the PTE so it's safe to modify */
> > + pte = ptep_get_and_clear(&init_mm, addr, ptep);
> > + flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> > +
> > + /* modify the PTE bits as desired, then apply */
> > + switch (action) {
> > + case SET_MEMORY_RO:
> > + pte = pte_wrprotect(pte);
> > + break;
>
> So set_memory_ro() removes write, but doesn't remove execute.
>
> That doesn't match my mental model of what "set to ro" means, but I
> guess I'm wrong because the other implementations seem to do something
> similar.
Hm, looking at arm and riscv it does seem to make it just RO.
>
>
> > + case SET_MEMORY_RW:
> > + pte = pte_mkwrite(pte);
>
> I think we want to add pte_mkdirty() here also to avoid a fault when the
> mapping is written to.
Right.
>
> eg. pmd_mkwrite(pmd_mkdirty(pte));
>
> > + break;
> > + case SET_MEMORY_NX:
> > + pte = pte_exprotect(pte);
> > + break;
> > + case SET_MEMORY_X:
> > + pte = pte_mkexec(pte);
> > + break;
> > + default:
> > + WARN_ON_ONCE(1);
> > + break;
> > + }
> > +
> > + set_pte_at(&init_mm, addr, ptep, pte);
> > + spin_unlock(&init_mm.page_table_lock);
> > +
> > + return 0;
> > +}
> > +
> > +int change_memory_attr(unsigned long addr, int numpages, long action)
> > +{
> > + unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
> > + unsigned long sz = numpages * PAGE_SIZE;
> > +
> > + if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> > + return 0;
> > +
> > + if (numpages <= 0)
> > + return 0;
> > +
>
> This ↓ should have a comment explaining what it's doing:
Sure.
>
> > +#ifdef CONFIG_PPC_BOOK3S_64
> > + if (WARN_ON_ONCE(!radix_enabled() &&
> > + get_region_id(addr) == LINEAR_MAP_REGION_ID)) {
> > + return -1;
> > + }
> > +#endif
>
> Maybe:
As Chrisophe says, we can't do that because those symbols aren't
defined for !CONFIG_PPC_BOOK3S_64.
>
> if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) &&
> WARN_ON_ONCE(!radix_enabled() && get_region_id(addr) == LINEAR_MAP_REGION_ID)) {
> return -1;
> }
>
> But then Aneesh pointed out that we should also block VMEMMAP_REGION_ID.
>
> It might be better to just check for the permitted regions.
That would probably work better.
>
> if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !radix_enabled()) {
> int region = get_region_id(addr);
>
> if (WARN_ON_ONCE(region != VMALLOC_REGION_ID && region != IO_REGION_ID))
> return -1;
> }
>
> > +
> > + return apply_to_existing_page_range(&init_mm, start, sz,
> > + change_page_attr, (void *)action);
> > +}
>
>
> cheers
^ permalink raw reply
* Re: [PATCH v10 03/10] powerpc: Always define MODULES_{VADDR,END}
From: Christophe Leroy @ 2021-04-21 5:14 UTC (permalink / raw)
To: Jordan Niethe
Cc: ajd, Nicholas Piggin, cmr, naveen.n.rao, linuxppc-dev,
Daniel Axtens
In-Reply-To: <CACzsE9oNQuymvv5FRUvzQ9uiytvFOcke5RsXA_urfKS1mtMnLg@mail.gmail.com>
Le 21/04/2021 à 04:46, Jordan Niethe a écrit :
> On Fri, Apr 2, 2021 at 12:36 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 30/03/2021 à 06:51, Jordan Niethe a écrit :
>>> If MODULES_{VADDR,END} are not defined set them to VMALLOC_START and
>>> VMALLOC_END respectively. This reduces the need for special cases. For
>>> example, powerpc's module_alloc() was previously predicated on
>>> MODULES_VADDR being defined but now is unconditionally defined.
>>>
>>> This will be useful reducing conditional code in other places that need
>>> to allocate from the module region (i.e., kprobes).
>>>
>>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
>>> ---
>>> v10: New to series
>>> ---
>>> arch/powerpc/include/asm/pgtable.h | 5 +++++
>>> arch/powerpc/kernel/module.c | 5 +----
>>
>> You probably also have changes to do in kernel/ptdump.c
>>
>> In mm/book3s32/mmu.c and mm/kasan/kasan_init_32.c as well allthough that's harmless here.
>>
>>> 2 files changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
>>> index 4eed82172e33..014c2921f26a 100644
>>> --- a/arch/powerpc/include/asm/pgtable.h
>>> +++ b/arch/powerpc/include/asm/pgtable.h
>>> @@ -167,6 +167,11 @@ struct seq_file;
>>> void arch_report_meminfo(struct seq_file *m);
>>> #endif /* CONFIG_PPC64 */
>>>
>>> +#ifndef MODULES_VADDR
>>> +#define MODULES_VADDR VMALLOC_START
>>> +#define MODULES_END VMALLOC_END
>>> +#endif
>>> +
>>> #endif /* __ASSEMBLY__ */
>>>
>>> #endif /* _ASM_POWERPC_PGTABLE_H */
>>> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
>>> index a211b0253cdb..f1fb58389d58 100644
>>> --- a/arch/powerpc/kernel/module.c
>>> +++ b/arch/powerpc/kernel/module.c
>>> @@ -14,6 +14,7 @@
>>> #include <asm/firmware.h>
>>> #include <linux/sort.h>
>>> #include <asm/setup.h>
>>> +#include <linux/mm.h>
>>>
>>> static LIST_HEAD(module_bug_list);
>>>
>>> @@ -87,13 +88,9 @@ int module_finalize(const Elf_Ehdr *hdr,
>>> return 0;
>>> }
>>>
>>> -#ifdef MODULES_VADDR
>>> void *module_alloc(unsigned long size)
>>> {
>>> - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
>>> -
>>
>> The above check is needed somewhere, if you remove it from here you have to perform the check
>> somewhere else.
>
> This also introduces this warning:
> fs/proc/kcore.c:626:52: warning: self-comparison always evaluates to
> false [-Wtautological-compare]
> 626 | if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
> I might leave this patch out of this series and use an #ifdef for now
> and make this change separately as a follow up.
x86/32 at least does the same (see
https://elixir.bootlin.com/linux/v5.12-rc8/source/arch/x86/include/asm/pgtable_32_areas.h#L47)
They probably also get the warning, so I think would shouldn't bother.
One day someone will fix fs/proc/kcore.c , that's not a powerpc problem.
>
>>
>>> return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL,
>>> PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>>> __builtin_return_address(0));
>>> }
>>> -#endif
>>>
^ permalink raw reply
* Re: [PATCH v10 06/10] powerpc/mm/ptdump: debugfs handler for W+X checks at runtime
From: Christophe Leroy @ 2021-04-21 5:16 UTC (permalink / raw)
To: Jordan Niethe, Michael Ellerman
Cc: ajd, Kees Cook, Nicholas Piggin, cmr, naveen.n.rao, linuxppc-dev,
Daniel Axtens
In-Reply-To: <CACzsE9pgZRaoHT0Ca=NB3srEMJMvLuiFkYA98xFDiUPyfmoV5Q@mail.gmail.com>
Le 21/04/2021 à 04:23, Jordan Niethe a écrit :
> On Wed, Mar 31, 2021 at 10:24 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Jordan Niethe <jniethe5@gmail.com> writes:
>>> From: Russell Currey <ruscur@russell.cc>
>>>
>>> Optionally run W+X checks when dumping pagetable information to
>>> debugfs' kernel_page_tables.
>>>
>>> To use:
>>> $ echo 1 > /sys/kernel/debug/check_wx_pages
>>> $ cat /sys/kernel/debug/kernel_page_tables
>>>
>>> and check the kernel log. Useful for testing strict module RWX.
>>>
>>> To disable W+X checks:
>>> $ echo 0 > /sys/kernel/debug/check_wx_pages
>>>
>>> Update the Kconfig entry to reflect this.
>>>
>>> Also fix a typo.
>>>
>>> Reviewed-by: Kees Cook <keescook@chromium.org>
>>> Signed-off-by: Russell Currey <ruscur@russell.cc>
>>> [jpn: Change check_wx_pages to act as mode bit affecting
>>> kernel_page_tables instead of triggering action on its own]
>>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
>>> ---
>>> v10: check_wx_pages now affects kernel_page_tables rather then triggers
>>> its own action.
>>
>> Hmm. I liked the old version better :)
>>
>> I think you changed it based on Christophe's comment:
>>
>> Why not just perform the test everytime someone dumps kernel_page_tables ?
>>
>>
>> But I think he meant *always* do the check when someone dumps
>> kernel_page_tables, not have another file to enable checking and then
>> require someone to dump kernel_page_tables to do the actual check.
> Yes, I guess I misinterpreted that.
>>
>> Still I like the previous version where you can do the checks
>> separately, without having to dump the page tables, because dumping can
>> sometimes take quite a while.
>>
>> What would be even better is if ptdump_check_wx() returned an error when
>> wx pages were found, and that was plumbed out to the debugs file. That
>> way you can script around it.
> Ok I'll go back to how it was and add in returning an error.
I have a series to convert PPC_PTDUMP into GENERIC_PTDUMP, see
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=239795
>>
>> cheers
^ permalink raw reply
* Re: [PATCH v10 01/10] powerpc/mm: Implement set_memory() routines
From: Jordan Niethe @ 2021-04-21 5:19 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: ajd, Nicholas Piggin, cmr, naveen.n.rao, linuxppc-dev,
Daniel Axtens
In-Reply-To: <87czve7h8r.fsf@linux.ibm.com>
On Thu, Apr 1, 2021 at 3:37 PM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> Jordan Niethe <jniethe5@gmail.com> writes:
>
> > From: Russell Currey <ruscur@russell.cc>
> >
> > The set_memory_{ro/rw/nx/x}() functions are required for STRICT_MODULE_RWX,
> > and are generally useful primitives to have. This implementation is
> > designed to be completely generic across powerpc's many MMUs.
> >
> > It's possible that this could be optimised to be faster for specific
> > MMUs, but the focus is on having a generic and safe implementation for
> > now.
> >
> > This implementation does not handle cases where the caller is attempting
> > to change the mapping of the page it is executing from, or if another
> > CPU is concurrently using the page being altered. These cases likely
> > shouldn't happen, but a more complex implementation with MMU-specific code
> > could safely handle them, so that is left as a TODO for now.
> >
> > On hash the linear mapping is not kept in the linux pagetable, so this
> > will not change the protection if used on that range. Currently these
> > functions are not used on the linear map so just WARN for now.
> >
> > These functions do nothing if STRICT_KERNEL_RWX is not enabled.
> >
> > Reviewed-by: Daniel Axtens <dja@axtens.net>
> > Signed-off-by: Russell Currey <ruscur@russell.cc>
> > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > [jpn: -rebase on next plus "powerpc/mm/64s: Allow STRICT_KERNEL_RWX again"
> > - WARN on hash linear map]
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v10: WARN if trying to change the hash linear map
> > ---
> > arch/powerpc/Kconfig | 1 +
> > arch/powerpc/include/asm/set_memory.h | 32 ++++++++++
> > arch/powerpc/mm/Makefile | 2 +-
> > arch/powerpc/mm/pageattr.c | 88 +++++++++++++++++++++++++++
> > 4 files changed, 122 insertions(+), 1 deletion(-)
> > create mode 100644 arch/powerpc/include/asm/set_memory.h
> > create mode 100644 arch/powerpc/mm/pageattr.c
> >
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index fc7f5c5933e6..4498a27ac9db 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -135,6 +135,7 @@ config PPC
> > select ARCH_HAS_MEMBARRIER_CALLBACKS
> > select ARCH_HAS_MEMBARRIER_SYNC_CORE
> > select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
> > + select ARCH_HAS_SET_MEMORY
> > select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !HIBERNATION)
> > select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> > select ARCH_HAS_UACCESS_FLUSHCACHE
> > diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
> > new file mode 100644
> > index 000000000000..64011ea444b4
> > --- /dev/null
> > +++ b/arch/powerpc/include/asm/set_memory.h
> > @@ -0,0 +1,32 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _ASM_POWERPC_SET_MEMORY_H
> > +#define _ASM_POWERPC_SET_MEMORY_H
> > +
> > +#define SET_MEMORY_RO 0
> > +#define SET_MEMORY_RW 1
> > +#define SET_MEMORY_NX 2
> > +#define SET_MEMORY_X 3
> > +
> > +int change_memory_attr(unsigned long addr, int numpages, long action);
> > +
> > +static inline int set_memory_ro(unsigned long addr, int numpages)
> > +{
> > + return change_memory_attr(addr, numpages, SET_MEMORY_RO);
> > +}
> > +
> > +static inline int set_memory_rw(unsigned long addr, int numpages)
> > +{
> > + return change_memory_attr(addr, numpages, SET_MEMORY_RW);
> > +}
> > +
> > +static inline int set_memory_nx(unsigned long addr, int numpages)
> > +{
> > + return change_memory_attr(addr, numpages, SET_MEMORY_NX);
> > +}
> > +
> > +static inline int set_memory_x(unsigned long addr, int numpages)
> > +{
> > + return change_memory_attr(addr, numpages, SET_MEMORY_X);
> > +}
> > +
> > +#endif
> > diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> > index 3b4e9e4e25ea..d8a08abde1ae 100644
> > --- a/arch/powerpc/mm/Makefile
> > +++ b/arch/powerpc/mm/Makefile
> > @@ -5,7 +5,7 @@
> >
> > ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
> >
> > -obj-y := fault.o mem.o pgtable.o mmap.o maccess.o \
> > +obj-y := fault.o mem.o pgtable.o mmap.o maccess.o pageattr.o \
> > init_$(BITS).o pgtable_$(BITS).o \
> > pgtable-frag.o ioremap.o ioremap_$(BITS).o \
> > init-common.o mmu_context.o drmem.o
> > diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
> > new file mode 100644
> > index 000000000000..9efcb01088da
> > --- /dev/null
> > +++ b/arch/powerpc/mm/pageattr.c
> > @@ -0,0 +1,88 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * MMU-generic set_memory implementation for powerpc
> > + *
> > + * Copyright 2019, IBM Corporation.
> > + */
> > +
> > +#include <linux/mm.h>
> > +#include <linux/set_memory.h>
> > +
> > +#include <asm/mmu.h>
> > +#include <asm/page.h>
> > +#include <asm/pgtable.h>
> > +
> > +
> > +/*
> > + * Updates the attributes of a page in three steps:
> > + *
> > + * 1. invalidate the page table entry
> > + * 2. flush the TLB
> > + * 3. install the new entry with the updated attributes
> > + *
> > + * This is unsafe if the caller is attempting to change the mapping of the
> > + * page it is executing from, or if another CPU is concurrently using the
> > + * page being altered.
> > + *
> > + * TODO make the implementation resistant to this.
> > + *
> > + * NOTE: can be dangerous to call without STRICT_KERNEL_RWX
> > + */
> > +static int change_page_attr(pte_t *ptep, unsigned long addr, void *data)
> > +{
> > + long action = (long)data;
> > + pte_t pte;
> > +
> > + spin_lock(&init_mm.page_table_lock);
> > +
> > + /* invalidate the PTE so it's safe to modify */
> > + pte = ptep_get_and_clear(&init_mm, addr, ptep);
> > + flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
> > +
> > + /* modify the PTE bits as desired, then apply */
> > + switch (action) {
> > + case SET_MEMORY_RO:
> > + pte = pte_wrprotect(pte);
> > + break;
> > + case SET_MEMORY_RW:
> > + pte = pte_mkwrite(pte);
> > + break;
> > + case SET_MEMORY_NX:
> > + pte = pte_exprotect(pte);
> > + break;
> > + case SET_MEMORY_X:
> > + pte = pte_mkexec(pte);
> > + break;
> > + default:
> > + WARN_ON_ONCE(1);
> > + break;
> > + }
> > +
> > + set_pte_at(&init_mm, addr, ptep, pte);
> > + spin_unlock(&init_mm.page_table_lock);
> > +
> > + return 0;
> > +}
> > +
> > +int change_memory_attr(unsigned long addr, int numpages, long action)
> > +{
> > + unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
> > + unsigned long sz = numpages * PAGE_SIZE;
> > +
> > + if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
> > + return 0;
>
> What restrictions imposed by that config are we dependent on here?
So the reasons given here
https://lore.kernel.org/linuxppc-dev/20200226062403.63790-9-ruscur@russell.cc/
were:
"
- The linear mapping is a different size and apply_to_page_range()
may modify a giant section, breaking everything
- patch_instruction() doesn't know to work around a page being marked
RO, and will subsequently crash
"
but now I'm not 100% sure about it... we might not actually need to
have that restriction.
>
>
> > +
> > + if (numpages <= 0)
> > + return 0;
> > +
> > +#ifdef CONFIG_PPC_BOOK3S_64
> > + if (WARN_ON_ONCE(!radix_enabled() &&
> > + get_region_id(addr) == LINEAR_MAP_REGION_ID)) {
> > + return -1;
> > + }
> > +#endif
>
> What about VMEMMAP_REGIOND_ID
True.
>
> > +
> > + return apply_to_existing_page_range(&init_mm, start, sz,
> > + change_page_attr, (void *)action);
>
>
> That handles on 64K mapping. What about linear map? Also there is a
> patchset implementing hugepage for vmalloc mapping.
At least for now there is nothing that calls the set memory functions
on the linear map.
Is that this series:
https://lore.kernel.org/linuxppc-dev/20210317062402.533919-15-npiggin@gmail.com/
?
I will test on top of that.
>
> > +}
> > --
> > 2.25.1
^ permalink raw reply
* Re: [PATCH v2] powerpc: make ALTIVEC select PPC_FPU
From: Christophe Leroy @ 2021-04-21 5:19 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel; +Cc: linuxppc-dev, kernel test robot
In-Reply-To: <20210421025605.10868-1-rdunlap@infradead.org>
Le 21/04/2021 à 04:56, Randy Dunlap a écrit :
> On a kernel config with ALTIVEC=y and PPC_FPU not set/enabled,
> there are build errors:
>
> drivers/cpufreq/pmac32-cpufreq.c:262:2: error: implicit declaration of function 'enable_kernel_fp' [-Werror,-Wimplicit-function-declaration]
> enable_kernel_fp();
> ../arch/powerpc/lib/sstep.c: In function 'do_vec_load':
> ../arch/powerpc/lib/sstep.c:637:3: error: implicit declaration of function 'put_vr' [-Werror=implicit-function-declaration]
> 637 | put_vr(rn, &u.v);
> | ^~~~~~
> ../arch/powerpc/lib/sstep.c: In function 'do_vec_store':
> ../arch/powerpc/lib/sstep.c:660:3: error: implicit declaration of function 'get_vr'; did you mean 'get_oc'? [-Werror=implicit-function-declaration]
> 660 | get_vr(rn, &u.v);
> | ^~~~~~
>
> In theory ALTIVEC is independent of PPC_FPU but in practice nobody
> is going to build such a machine, so make ALTIVEC require PPC_FPU
> by selecting it.
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: Segher Boessenkool <segher@kernel.crashing.org>
> Cc: lkp@intel.com
> ---
> v2: change ALTIVEC depends on PPC_FPU to select (Christophe and Michael)
>
> arch/powerpc/platforms/86xx/Kconfig | 1 +
> arch/powerpc/platforms/Kconfig.cputype | 2 ++
> 2 files changed, 3 insertions(+)
>
> --- linux-next-20210416.orig/arch/powerpc/platforms/86xx/Kconfig
> +++ linux-next-20210416/arch/powerpc/platforms/86xx/Kconfig
> @@ -4,6 +4,7 @@ menuconfig PPC_86xx
> bool "86xx-based boards"
> depends on PPC_BOOK3S_32
> select FSL_SOC
> + select PPC_FPU
Now that ALTIVEC selects PPC_FPU by itself, I don't think you need that.
> select ALTIVEC
> help
> The Freescale E600 SoCs have 74xx cores.
> --- linux-next-20210416.orig/arch/powerpc/platforms/Kconfig.cputype
> +++ linux-next-20210416/arch/powerpc/platforms/Kconfig.cputype
> @@ -186,6 +186,7 @@ config E300C3_CPU
> config G4_CPU
> bool "G4 (74xx)"
> depends on PPC_BOOK3S_32
> + select PPC_FPU
Same
> select ALTIVEC
>
> endchoice
> @@ -310,6 +311,7 @@ config PHYS_64BIT
> config ALTIVEC
> bool "AltiVec Support"
> depends on PPC_BOOK3S_32 || PPC_BOOK3S_64 || (PPC_E500MC && PPC64)
> + select PPC_FPU
> help
> This option enables kernel support for the Altivec extensions to the
> PowerPC processor. The kernel currently supports saving and restoring
>
^ permalink raw reply
* Re: [PATCH v10 03/10] powerpc: Always define MODULES_{VADDR,END}
From: Jordan Niethe @ 2021-04-21 5:22 UTC (permalink / raw)
To: Christophe Leroy
Cc: ajd, Nicholas Piggin, cmr, naveen.n.rao, linuxppc-dev,
Daniel Axtens
In-Reply-To: <8e0e850d-79b5-03ab-56c7-3d92ec72c7dc@csgroup.eu>
On Wed, Apr 21, 2021 at 3:14 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 21/04/2021 à 04:46, Jordan Niethe a écrit :
> > On Fri, Apr 2, 2021 at 12:36 AM Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >>
> >>
> >>
> >> Le 30/03/2021 à 06:51, Jordan Niethe a écrit :
> >>> If MODULES_{VADDR,END} are not defined set them to VMALLOC_START and
> >>> VMALLOC_END respectively. This reduces the need for special cases. For
> >>> example, powerpc's module_alloc() was previously predicated on
> >>> MODULES_VADDR being defined but now is unconditionally defined.
> >>>
> >>> This will be useful reducing conditional code in other places that need
> >>> to allocate from the module region (i.e., kprobes).
> >>>
> >>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> >>> ---
> >>> v10: New to series
> >>> ---
> >>> arch/powerpc/include/asm/pgtable.h | 5 +++++
> >>> arch/powerpc/kernel/module.c | 5 +----
> >>
> >> You probably also have changes to do in kernel/ptdump.c
> >>
> >> In mm/book3s32/mmu.c and mm/kasan/kasan_init_32.c as well allthough that's harmless here.
> >>
> >>> 2 files changed, 6 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> >>> index 4eed82172e33..014c2921f26a 100644
> >>> --- a/arch/powerpc/include/asm/pgtable.h
> >>> +++ b/arch/powerpc/include/asm/pgtable.h
> >>> @@ -167,6 +167,11 @@ struct seq_file;
> >>> void arch_report_meminfo(struct seq_file *m);
> >>> #endif /* CONFIG_PPC64 */
> >>>
> >>> +#ifndef MODULES_VADDR
> >>> +#define MODULES_VADDR VMALLOC_START
> >>> +#define MODULES_END VMALLOC_END
> >>> +#endif
> >>> +
> >>> #endif /* __ASSEMBLY__ */
> >>>
> >>> #endif /* _ASM_POWERPC_PGTABLE_H */
> >>> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
> >>> index a211b0253cdb..f1fb58389d58 100644
> >>> --- a/arch/powerpc/kernel/module.c
> >>> +++ b/arch/powerpc/kernel/module.c
> >>> @@ -14,6 +14,7 @@
> >>> #include <asm/firmware.h>
> >>> #include <linux/sort.h>
> >>> #include <asm/setup.h>
> >>> +#include <linux/mm.h>
> >>>
> >>> static LIST_HEAD(module_bug_list);
> >>>
> >>> @@ -87,13 +88,9 @@ int module_finalize(const Elf_Ehdr *hdr,
> >>> return 0;
> >>> }
> >>>
> >>> -#ifdef MODULES_VADDR
> >>> void *module_alloc(unsigned long size)
> >>> {
> >>> - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
> >>> -
> >>
> >> The above check is needed somewhere, if you remove it from here you have to perform the check
> >> somewhere else.
> >
> > This also introduces this warning:
> > fs/proc/kcore.c:626:52: warning: self-comparison always evaluates to
> > false [-Wtautological-compare]
> > 626 | if (MODULES_VADDR != VMALLOC_START && MODULES_END != VMALLOC_END) {
> > I might leave this patch out of this series and use an #ifdef for now
> > and make this change separately as a follow up.
>
> x86/32 at least does the same (see
> https://elixir.bootlin.com/linux/v5.12-rc8/source/arch/x86/include/asm/pgtable_32_areas.h#L47)
>
> They probably also get the warning, so I think would shouldn't bother.
> One day someone will fix fs/proc/kcore.c , that's not a powerpc problem.
Yeah you are right. I'll add the BUILD_BUG_ON() check to
asm/task_size_32.h and keep the patch.
>
> >
> >>
> >>> return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL,
> >>> PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
> >>> __builtin_return_address(0));
> >>> }
> >>> -#endif
> >>>
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: Free fdt on error in elf64_load()
From: Santosh Sivaraj @ 2021-04-21 5:35 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, robh, dan.carpenter, mpe
Cc: devicetree, kbuild-all, lkp, nramas, linuxppc-dev, bauerman, dja
In-Reply-To: <20210420190355.10059-1-nramas@linux.microsoft.com>
Hi Lakshmi,
Lakshmi Ramasubramanian <nramas@linux.microsoft.com> writes:
> There are a few "goto out;" statements before the local variable "fdt"
> is initialized through the call to of_kexec_alloc_and_setup_fdt() in
> elf64_load(). This will result in an uninitialized "fdt" being passed
> to kvfree() in this function if there is an error before the call to
> of_kexec_alloc_and_setup_fdt().
>
> If there is any error after fdt is allocated, but before it is
> saved in the arch specific kimage struct, free the fdt.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/kexec/elf_64.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
> index 5a569bb51349..02662e72c53d 100644
> --- a/arch/powerpc/kexec/elf_64.c
> +++ b/arch/powerpc/kexec/elf_64.c
> @@ -114,7 +114,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> ret = setup_new_fdt_ppc64(image, fdt, initrd_load_addr,
> initrd_len, cmdline);
> if (ret)
> - goto out;
> + goto out_free_fdt;
Shouldn't there be a goto out_free_fdt if fdt_open_into fails?
>
> fdt_pack(fdt);
>
> @@ -125,7 +125,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
> ret = kexec_add_buffer(&kbuf);
> if (ret)
> - goto out;
> + goto out_free_fdt;
>
> /* FDT will be freed in arch_kimage_file_post_load_cleanup */
> image->arch.fdt = fdt;
> @@ -140,18 +140,14 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
> if (ret)
> pr_err("Error setting up the purgatory.\n");
>
> + goto out;
> +
> +out_free_fdt:
> + kvfree(fdt);
Can just use kfree here?
Thanks,
Santosh
> out:
> kfree(modified_cmdline);
> kexec_free_elf_info(&elf_info);
>
> - /*
> - * Once FDT buffer has been successfully passed to kexec_add_buffer(),
> - * the FDT buffer address is saved in image->arch.fdt. In that case,
> - * the memory cannot be freed here in case of any other error.
> - */
> - if (ret && !image->arch.fdt)
> - kvfree(fdt);
> -
> return ret ? ERR_PTR(ret) : NULL;
> }
>
> --
> 2.31.0
^ permalink raw reply
* Re: [PATCH 1/2] mm: Fix struct page layout on 32-bit systems
From: hch @ 2021-04-21 5:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: grygorii.strashko@ti.com, linux-snps-arc@lists.infradead.org,
netdev@vger.kernel.org, Vineet Gupta, ilias.apalodimas@linaro.org,
linux-kernel@vger.kernel.org, Matthew Wilcox,
linux-mips@vger.kernel.org, linux-mm@kvack.org, mgorman@suse.de,
brouer@redhat.com, mcroce@linux.microsoft.com, mhocko@kernel.org,
linuxppc-dev@lists.ozlabs.org, hch@lst.de,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAK8P3a3rzz1gfNLoGC8aZJiAC-tgZYD6P8pQsoEfgCAmQK=FAw@mail.gmail.com>
On Tue, Apr 20, 2021 at 11:20:19PM +0200, Arnd Bergmann wrote:
> In that case, there should be no problem for you.
>
> The main issue is with system calls and ioctls that contain a misaligned
> struct member like
>
> struct s {
> u32 a;
> u64 b;
> };
>
> Passing this structure by reference from a 32-bit user space application
> to a 64-bit kernel with different alignment constraints means that the
> kernel has to convert the structure layout. See
> compat_ioctl_preallocate() in fs/ioctl.c for one such example.
We've also had this problem with some on-disk structures in the past,
but hopefully people desining those have learnt the lesson by now.
^ permalink raw reply
* Re: powerpc{32,64} randconfigs
From: Christophe Leroy @ 2021-04-21 6:03 UTC (permalink / raw)
To: Randy Dunlap, PowerPC; +Cc: linux-kbuild
In-Reply-To: <45afab41-cc5f-dffa-712a-d5fb1a9d0a23@infradead.org>
Hi
Le 21/04/2021 à 01:31, Randy Dunlap a écrit :
> [time passes]
>
> OK, I have a patch that seems for work as far as setting
> PPC32=y or PPC64=y... but it has a problem during linking
> of vmlinux:
>
> crosstool/gcc-9.3.0-nolibc/powerpc-linux/bin/powerpc-linux-ld:./arch/powerpc/kernel/vmlinux.lds:6: syntax error
>
> and the (bad) generated vmlinux.lds file says (at line 6):
>
> OUTPUT_ARCH(1:common)
>
> while it should say:
>
> OUTPUT_ARCH(powerpc:common)
>
> Does anyone have any ideas about this problem?
The following change fixes the problem, allthough I guess that's not what you want to do at the end.
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 1b6094a13034..00f72699c518 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -358,7 +358,7 @@ targets += $(lib-y) $(always-y) $(MAKECMDGOALS)
# Linker scripts preprocessor (.lds.S -> .lds)
# ---------------------------------------------------------------------------
quiet_cmd_cpp_lds_S = LDS $@
- cmd_cpp_lds_S = $(CPP) $(cpp_flags) -P -U$(ARCH) \
+ cmd_cpp_lds_S = $(CPP) $(cpp_flags) -P -U$(SRCARCH) \
-D__ASSEMBLY__ -DLINKER_SCRIPT -o $@ $<
$(obj)/%.lds: $(src)/%.lds.S FORCE
>
> The ARCH=ppc32 / ARCH=ppc64 patch is below FYI.
> It is missing an update to Documentation/kbuild/kbuild.rst (aliases).
>
>
> thanks.
>
^ permalink raw reply related
* Re: powerpc{32,64} randconfigs
From: Christophe Leroy @ 2021-04-21 6:12 UTC (permalink / raw)
To: Randy Dunlap, PowerPC; +Cc: linux-kbuild
In-Reply-To: <45afab41-cc5f-dffa-712a-d5fb1a9d0a23@infradead.org>
Le 21/04/2021 à 01:31, Randy Dunlap a écrit :
> --- linux-next-20210420.orig/arch/powerpc/platforms/Kconfig.cputype
> +++ linux-next-20210420/arch/powerpc/platforms/Kconfig.cputype
> @@ -1,11 +1,12 @@
> # SPDX-License-Identifier: GPL-2.0
> config PPC32
> bool
> - default y if !PPC64
> + default y if !PPC64 || "$(ARCH)" = "ppc32"
I don't think you need that change. If the ARCH is neither 'powerpc' not 'ppc64' CONFIG_PPC64 isn't
set so CONFIG_PPC32 is set.
> select KASAN_VMALLOC if KASAN && MODULES
>
> config PPC64
> - bool "64-bit kernel"
> + bool "64-bit kernel" if "$(ARCH)" = "powerpc"
> + default y if "$(ARCH)" = "ppc64"
> select ZLIB_DEFLATE
> help
> This option selects whether a 32-bit or a 64-bit kernel
^ permalink raw reply
* Re: powerpc{32,64} randconfigs
From: Christophe Leroy @ 2021-04-21 6:37 UTC (permalink / raw)
To: Randy Dunlap, PowerPC; +Cc: linux-kbuild
In-Reply-To: <45afab41-cc5f-dffa-712a-d5fb1a9d0a23@infradead.org>
Le 21/04/2021 à 01:31, Randy Dunlap a écrit :
> --- linux-next-20210420.orig/arch/powerpc/kernel/vdso64/Makefile
> +++ linux-next-20210420/arch/powerpc/kernel/vdso64/Makefile
> @@ -30,7 +30,7 @@ ccflags-y := -shared -fno-common -fno-bu
> asflags-y := -D__VDSO64__ -s
>
> targets += vdso64.lds
> -CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)
> +CPPFLAGS_vdso64.lds += -P -C -U$(SRCARCH)
Maybe it would be better to do -Upowerpc like in VDSO32
>
> # link rule for the .so file, .lds has to be first
> $(obj)/vdso64.so.dbg: $(src)/vdso64.lds $(obj-vdso64) $(obj)/vgettimeofday.o FORCE
> --- linux-next-20210420.orig/arch/powerpc/Makefile
^ 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