* [PATCH v2] powerpc/signal32: Fix Oops on sigreturn with unmapped VDSO
From: Christophe Leroy @ 2021-03-31 14:07 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
PPC32 encounters a KUAP fault when trying to handle a signal with
VDSO unmapped.
Kernel attempted to read user page (7fc07ec0) - exploit attempt? (uid: 0)
BUG: Unable to handle kernel data access on read at 0x7fc07ec0
Faulting instruction address: 0xc00111d4
Oops: Kernel access of bad area, sig: 11 [#1]
BE PAGE_SIZE=16K PREEMPT CMPC885
CPU: 0 PID: 353 Comm: sigreturn_vdso Not tainted 5.12.0-rc4-s3k-dev-01553-gb30c310ea220 #4814
NIP: c00111d4 LR: c0005a28 CTR: 00000000
REGS: cadb3dd0 TRAP: 0300 Not tainted (5.12.0-rc4-s3k-dev-01553-gb30c310ea220)
MSR: 00009032 <EE,ME,IR,DR,RI> CR: 48000884 XER: 20000000
DAR: 7fc07ec0 DSISR: 88000000
GPR00: c0007788 cadb3e90 c28d4a40 7fc07ec0 7fc07ed0 000004e0 7fc07ce0 00000000
GPR08: 00000001 00000001 7fc07ec0 00000000 28000282 1001b828 100a0920 00000000
GPR16: 100cac0c 100b0000 105c43a4 105c5685 100d0000 100d0000 100d0000 100b2e9e
GPR24: ffffffff 105c43c8 00000000 7fc07ec8 cadb3f40 cadb3ec8 c28d4a40 00000000
NIP [c00111d4] flush_icache_range+0x90/0xb4
LR [c0005a28] handle_signal32+0x1bc/0x1c4
Call Trace:
[cadb3e90] [100d0000] 0x100d0000 (unreliable)
[cadb3ec0] [c0007788] do_notify_resume+0x260/0x314
[cadb3f20] [c000c764] syscall_exit_prepare+0x120/0x184
[cadb3f30] [c00100b4] ret_from_syscall+0xc/0x28
--- interrupt: c00 at 0xfe807f8
NIP: 0fe807f8 LR: 10001060 CTR: c0139378
REGS: cadb3f40 TRAP: 0c00 Not tainted (5.12.0-rc4-s3k-dev-01553-gb30c310ea220)
MSR: 0000d032 <EE,PR,ME,IR,DR,RI> CR: 28000482 XER: 20000000
GPR00: 00000025 7fc081c0 77bb1690 00000000 0000000a 28000482 00000001 0ff03a38
GPR08: 0000d032 00006de5 c28d4a40 00000009 88000482 1001b828 100a0920 00000000
GPR16: 100cac0c 100b0000 105c43a4 105c5685 100d0000 100d0000 100d0000 100b2e9e
GPR24: ffffffff 105c43c8 00000000 77ba7628 10002398 10010000 10002124 00024000
NIP [0fe807f8] 0xfe807f8
LR [10001060] 0x10001060
--- interrupt: c00
Instruction dump:
38630010 7c001fac 38630010 4200fff0 7c0004ac 4c00012c 4e800020 7c001fac
2c0a0000 38630010 4082ffcc 4bffffe4 <7c00186c> 2c070000 39430010 4082ff8c
---[ end trace 3973fb72b049cb06 ]---
This is because flush_icache_range() is called on user addresses.
The same problem was detected some time ago on PPC64. It was fixed by
enabling KUAP in commit 59bee45b9712 ("powerpc/mm: Fix missing KUAP
disable in flush_coherent_icache()").
PPC32 doesn't use flush_coherent_icache() and fallbacks on
clean_dcache_range() and invalidate_icache_range().
We could fix it similarly by enabling user access in those functions,
but this is overkill for just flushing two instructions.
The two instructions are 8 bytes aligned, so a single dcbst/icbi is
enough to flush them. Do like __patch_instruction() and inline
a dcbst followed by an icbi just after the write of the instructions,
while user access is still allowed. The isync is not required because
rfi will be used to return to user.
icbi() is handled as a read so read-write user access is needed.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: Do read-write user access.
---
arch/powerpc/kernel/signal_32.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index c505b444a613..09884af693aa 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -775,7 +775,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
else
prepare_save_user_regs(1);
- if (!user_write_access_begin(frame, sizeof(*frame)))
+ if (!user_access_begin(frame, sizeof(*frame)))
goto badframe;
/* Put the siginfo & fill in most of the ucontext */
@@ -809,17 +809,15 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
unsafe_put_user(PPC_INST_ADDI + __NR_rt_sigreturn, &mctx->mc_pad[0],
failed);
unsafe_put_user(PPC_INST_SC, &mctx->mc_pad[1], failed);
+ asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
}
unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
- user_write_access_end();
+ user_access_end();
if (copy_siginfo_to_user(&frame->info, &ksig->info))
goto badframe;
- if (tramp == (unsigned long)mctx->mc_pad)
- flush_icache_range(tramp, tramp + 2 * sizeof(unsigned long));
-
regs->link = tramp;
#ifdef CONFIG_PPC_FPU_REGS
@@ -844,7 +842,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
return 0;
failed:
- user_write_access_end();
+ user_access_end();
badframe:
signal_fault(tsk, regs, "handle_rt_signal32", frame);
@@ -879,7 +877,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
else
prepare_save_user_regs(1);
- if (!user_write_access_begin(frame, sizeof(*frame)))
+ if (!user_access_begin(frame, sizeof(*frame)))
goto badframe;
sc = (struct sigcontext __user *) &frame->sctx;
@@ -908,11 +906,9 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
/* Set up the sigreturn trampoline: li r0,sigret; sc */
unsafe_put_user(PPC_INST_ADDI + __NR_sigreturn, &mctx->mc_pad[0], failed);
unsafe_put_user(PPC_INST_SC, &mctx->mc_pad[1], failed);
+ asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
}
- user_write_access_end();
-
- if (tramp == (unsigned long)mctx->mc_pad)
- flush_icache_range(tramp, tramp + 2 * sizeof(unsigned long));
+ user_access_end();
regs->link = tramp;
@@ -935,7 +931,7 @@ int handle_signal32(struct ksignal *ksig, sigset_t *oldset,
return 0;
failed:
- user_write_access_end();
+ user_access_end();
badframe:
signal_fault(tsk, regs, "handle_signal32", frame);
--
2.25.0
^ permalink raw reply related
* [PATCH] selftests: timens: Fix gettime_perf to work on powerpc
From: Christophe Leroy @ 2021-03-31 13:59 UTC (permalink / raw)
To: Shuah Khan, Andrei Vagin, Dmitry Safonov
Cc: linuxppc-dev, linux-kernel, linux-kselftest
On powerpc:
- VDSO library is named linux-vdso32.so.1 or linux-vdso64.so.1
- clock_gettime is named __kernel_clock_gettime()
Ensure gettime_perf tries these names before giving up.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
tools/testing/selftests/timens/gettime_perf.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tools/testing/selftests/timens/gettime_perf.c b/tools/testing/selftests/timens/gettime_perf.c
index 7bf841a3967b..6b13dc277724 100644
--- a/tools/testing/selftests/timens/gettime_perf.c
+++ b/tools/testing/selftests/timens/gettime_perf.c
@@ -25,12 +25,20 @@ static void fill_function_pointers(void)
if (!vdso)
vdso = dlopen("linux-gate.so.1",
RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD);
+ if (!vdso)
+ vdso = dlopen("linux-vdso32.so.1",
+ RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD);
+ if (!vdso)
+ vdso = dlopen("linux-vdso64.so.1",
+ RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD);
if (!vdso) {
pr_err("[WARN]\tfailed to find vDSO\n");
return;
}
vdso_clock_gettime = (vgettime_t)dlsym(vdso, "__vdso_clock_gettime");
+ if (!vdso_clock_gettime)
+ vdso_clock_gettime = (vgettime_t)dlsym(vdso, "__kernel_clock_gettime");
if (!vdso_clock_gettime)
pr_err("Warning: failed to find clock_gettime in vDSO\n");
--
2.25.0
^ permalink raw reply related
* Re: [PATCH] powerpc/8xx: Load modules closer to kernel text
From: Michael Ellerman @ 2021-03-31 13:39 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <db20abbecb129c49d3c663de67dac6a87501ac0b.1617001615.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> On the 8xx, TASK_SIZE is 0x80000000. The space between TASK_SIZE and
> PAGE_OFFSET is not used.
>
> Use it to load modules in order to minimise the distance between
> kernel text and modules and avoid trampolines in modules to access
> kernel functions or other module functions.
>
> Define a 16Mbytes area for modules, that's more than enough.
16MB seems kind of small.
At least on 64-bit we could potentially have hundreds of MBs of modules.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/signal32: Fix Oops on sigreturn with unmapped VDSO
From: Michael Ellerman @ 2021-03-31 13:23 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <79ee6fe13b267b0c81d7626c471a99140f0c9e4a.1616939989.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> PPC32 encounters a KUAP fault when trying to handle a signal with
> VDSO unmapped.
>
> Kernel attempted to read user page (7fc07ec0) - exploit attempt? (uid: 0)
> BUG: Unable to handle kernel data access on read at 0x7fc07ec0
> Faulting instruction address: 0xc00111d4
> Oops: Kernel access of bad area, sig: 11 [#1]
> BE PAGE_SIZE=16K PREEMPT CMPC885
> CPU: 0 PID: 353 Comm: sigreturn_vdso Not tainted 5.12.0-rc4-s3k-dev-01553-gb30c310ea220 #4814
> NIP: c00111d4 LR: c0005a28 CTR: 00000000
> REGS: cadb3dd0 TRAP: 0300 Not tainted (5.12.0-rc4-s3k-dev-01553-gb30c310ea220)
> MSR: 00009032 <EE,ME,IR,DR,RI> CR: 48000884 XER: 20000000
> DAR: 7fc07ec0 DSISR: 88000000
> GPR00: c0007788 cadb3e90 c28d4a40 7fc07ec0 7fc07ed0 000004e0 7fc07ce0 00000000
> GPR08: 00000001 00000001 7fc07ec0 00000000 28000282 1001b828 100a0920 00000000
> GPR16: 100cac0c 100b0000 105c43a4 105c5685 100d0000 100d0000 100d0000 100b2e9e
> GPR24: ffffffff 105c43c8 00000000 7fc07ec8 cadb3f40 cadb3ec8 c28d4a40 00000000
> NIP [c00111d4] flush_icache_range+0x90/0xb4
> LR [c0005a28] handle_signal32+0x1bc/0x1c4
> Call Trace:
> [cadb3e90] [100d0000] 0x100d0000 (unreliable)
> [cadb3ec0] [c0007788] do_notify_resume+0x260/0x314
> [cadb3f20] [c000c764] syscall_exit_prepare+0x120/0x184
> [cadb3f30] [c00100b4] ret_from_syscall+0xc/0x28
> --- interrupt: c00 at 0xfe807f8
> NIP: 0fe807f8 LR: 10001060 CTR: c0139378
> REGS: cadb3f40 TRAP: 0c00 Not tainted (5.12.0-rc4-s3k-dev-01553-gb30c310ea220)
> MSR: 0000d032 <EE,PR,ME,IR,DR,RI> CR: 28000482 XER: 20000000
>
> GPR00: 00000025 7fc081c0 77bb1690 00000000 0000000a 28000482 00000001 0ff03a38
> GPR08: 0000d032 00006de5 c28d4a40 00000009 88000482 1001b828 100a0920 00000000
> GPR16: 100cac0c 100b0000 105c43a4 105c5685 100d0000 100d0000 100d0000 100b2e9e
> GPR24: ffffffff 105c43c8 00000000 77ba7628 10002398 10010000 10002124 00024000
> NIP [0fe807f8] 0xfe807f8
> LR [10001060] 0x10001060
> --- interrupt: c00
> Instruction dump:
> 38630010 7c001fac 38630010 4200fff0 7c0004ac 4c00012c 4e800020 7c001fac
> 2c0a0000 38630010 4082ffcc 4bffffe4 <7c00186c> 2c070000 39430010 4082ff8c
> ---[ end trace 3973fb72b049cb06 ]---
>
> This is because flush_icache_range() is called on user addresses.
>
> The same problem was detected some time ago on PPC64. It was fixed by
> enabling KUAP in commit 59bee45b9712 ("powerpc/mm: Fix missing KUAP
> disable in flush_coherent_icache()").
>
> PPC32 doesn't use flush_coherent_icache() and fallbacks on
> clean_dcache_range() and invalidate_icache_range().
But this code is also used for compat tasks on 64-bit.
> We could fix it similarly by enabling user access in those functions,
> but this is overkill for just flushing two instructions.
>
> The two instructions are 8 bytes aligned, so a single dcbst/icbi is
> enough to flush them. Do like __patch_instruction() and inline
> a dcbst followed by an icbi just after the write of the instructions,
> while user access is still allowed. The isync is not required because
> rfi will be used to return to user.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/signal_32.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index 75ee918a120a..5b2ba2731957 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -809,6 +809,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
> unsafe_put_user(PPC_INST_ADDI + __NR_rt_sigreturn, &mctx->mc_pad[0],
> failed);
> unsafe_put_user(PPC_INST_SC, &mctx->mc_pad[1], failed);
> + asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
If I'm reading that right you're pointing the icbi at the user address.
That's going to cause a KUAP fault just like we fixed in commit
59bee45b9712 ("powerpc/mm: Fix missing KUAP disable in flush_coherent_icache()").
We have user write access enabled, but the icbi is treated as a load.
So I don't think that's going to work.
cheers
^ permalink raw reply
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
From: Robin Murphy @ 2021-03-31 13:09 UTC (permalink / raw)
To: Will Deacon
Cc: kvm, linux-arm-msm, linuxppc-dev, dri-devel, Li Yang, iommu,
Christoph Hellwig, netdev, virtualization, freedreno,
David Woodhouse, linux-arm-kernel
In-Reply-To: <20210331114947.GA7626@willie-the-truck>
On 2021-03-31 12:49, Will Deacon wrote:
> On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote:
>> On 2021-03-30 14:58, Will Deacon wrote:
>>> On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote:
>>>> On 2021-03-30 14:11, Will Deacon wrote:
>>>>> On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote:
>>>>>> From: Robin Murphy <robin.murphy@arm.com>
>>>>>>
>>>>>> Instead make the global iommu_dma_strict paramete in iommu.c canonical by
>>>>>> exporting helpers to get and set it and use those directly in the drivers.
>>>>>>
>>>>>> This make sure that the iommu.strict parameter also works for the AMD and
>>>>>> Intel IOMMU drivers on x86. As those default to lazy flushing a new
>>>>>> IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to
>>>>>> represent the default if not overriden by an explicit parameter.
>>>>>>
>>>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>.
>>>>>> [ported on top of the other iommu_attr changes and added a few small
>>>>>> missing bits]
>>>>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>>>>> ---
>>>>>> drivers/iommu/amd/iommu.c | 23 +-------
>>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +---------------
>>>>>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 -
>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +--------
>>>>>> drivers/iommu/dma-iommu.c | 9 +--
>>>>>> drivers/iommu/intel/iommu.c | 64 ++++-----------------
>>>>>> drivers/iommu/iommu.c | 27 ++++++---
>>>>>> include/linux/iommu.h | 4 +-
>>>>>> 8 files changed, 40 insertions(+), 165 deletions(-)
>>>>>
>>>>> I really like this cleanup, but I can't help wonder if it's going in the
>>>>> wrong direction. With SoCs often having multiple IOMMU instances and a
>>>>> distinction between "trusted" and "untrusted" devices, then having the
>>>>> flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound
>>>>> unreasonable to me, but this change makes it a global property.
>>>>
>>>> The intent here was just to streamline the existing behaviour of stuffing a
>>>> global property into a domain attribute then pulling it out again in the
>>>> illusion that it was in any way per-domain. We're still checking
>>>> dev_is_untrusted() before making an actual decision, and it's not like we
>>>> can't add more factors at that point if we want to.
>>>
>>> Like I say, the cleanup is great. I'm just wondering whether there's a
>>> better way to express the complicated logic to decide whether or not to use
>>> the flush queue than what we end up with:
>>>
>>> if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) &&
>>> domain->ops->flush_iotlb_all && !iommu_get_dma_strict())
>>>
>>> which is mixing up globals, device properties and domain properties. The
>>> result is that the driver code ends up just using the global to determine
>>> whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code,
>>> which is a departure from the current way of doing things.
>>
>> But previously, SMMU only ever saw the global policy piped through the
>> domain attribute by iommu_group_alloc_default_domain(), so there's no
>> functional change there.
>
> For DMA domains sure, but I don't think that's the case for unmanaged
> domains such as those used by VFIO.
Eh? This is only relevant to DMA domains anyway. Flush queues are part
of the IOVA allocator that VFIO doesn't even use. It's always been the
case that unmanaged domains only use strict invalidation.
>> Obviously some of the above checks could be factored out into some kind of
>> iommu_use_flush_queue() helper that IOMMU drivers can also call if they need
>> to keep in sync. Or maybe we just allow iommu-dma to set
>> IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're
>> treating that as a generic thing now.
>
> I think a helper that takes a domain would be a good starting point.
You mean device, right? The one condition we currently have is at the
device level, and there's really nothing inherent to the domain itself
that matters (since the type is implicitly IOMMU_DOMAIN_DMA to even care
about this).
Another idea that's just come to mind is now that IOMMU_DOMAIN_DMA has a
standard meaning, maybe we could split out a separate
IOMMU_DOMAIN_DMA_STRICT type such that it can all propagate from
iommu_get_def_domain_type()? That feels like it might be quite
promising, but I'd still do it as an improvement on top of this patch,
since it's beyond just cleaning up the abuse of domain attributes to
pass a command-line option around.
Robin.
^ permalink raw reply
* Re: [PATCH -next] powerpc/eeh: Remove unused inline function eeh_dev_phb_init_dynamic()
From: YueHaibing @ 2021-03-31 12:58 UTC (permalink / raw)
To: Daniel Axtens, ruscur, oohall, mpe, benh, paulus
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <874kgy8ptt.fsf@linkitivity.dja.id.au>
On 2021/3/26 13:08, Daniel Axtens wrote:
> Hi,
>
>> commit 475028efc708 ("powerpc/eeh: Remove eeh_dev_phb_init_dynamic()")
>> left behind this, so can remove it.
>
> I had a look: the inline that you are removing here is for the
> !CONFIG_EEH case, which explains why it was missed the first time.
>
> This looks like a good change. Out of interest, what tool are you using
> to find these unused inlines? If there are many more, it might make
> sense to combine future patches removing them into a single patch, but
> I'm not sure.
Just use some grep skill, will do that if any.
>
> checkpatch likes this patch, so that's also good :)
>
> Reviewed-by: Daniel Axtens <dja@axtens.net>
>
> Kind regards,
> Daniel
> .
>
^ permalink raw reply
* [PATCH -next] powerpc/eeh: Add correct inline functions
From: YueHaibing @ 2021-03-31 12:53 UTC (permalink / raw)
To: ruscur, oohall, mpe, benh, paulus, sbobroff
Cc: YueHaibing, linuxppc-dev, linux-kernel
pseries_eeh_add_device_early()/pseries_eeh_add_device_tree_early() is
never used since adding, however pseries_eeh_init_edev() and
pseries_eeh_init_edev_recursive() need their inline versions.
Fixes: b6eebb093cad ("powerpc/eeh: Make early EEH init pseries specific")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
arch/powerpc/include/asm/eeh.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index b1a5bba2e0b9..0b6c2a6711d3 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -357,8 +357,8 @@ static inline int eeh_phb_pe_create(struct pci_controller *phb) { return 0; }
void pseries_eeh_init_edev(struct pci_dn *pdn);
void pseries_eeh_init_edev_recursive(struct pci_dn *pdn);
#else
-static inline void pseries_eeh_add_device_early(struct pci_dn *pdn) { }
-static inline void pseries_eeh_add_device_tree_early(struct pci_dn *pdn) { }
+static inline void pseries_eeh_init_edev(struct pci_dn *pdn) { }
+static inline void pseries_eeh_init_edev_recursive(struct pci_dn *pdn) { }
#endif
#ifdef CONFIG_PPC64
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v10 01/10] powerpc/mm: Implement set_memory() routines
From: Christophe Leroy @ 2021-03-31 12:03 UTC (permalink / raw)
To: Michael Ellerman, Jordan Niethe, linuxppc-dev
Cc: ajd, npiggin, cmr, naveen.n.rao, dja
In-Reply-To: <87lfa37euw.fsf@mpe.ellerman.id.au>
Le 31/03/2021 à 13:16, Michael Ellerman a écrit :
> 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
>> ---
>
> This ↓ should have a comment explaining what it's doing:
>
>> +#ifdef CONFIG_PPC_BOOK3S_64
>> + if (WARN_ON_ONCE(!radix_enabled() &&
>> + get_region_id(addr) == LINEAR_MAP_REGION_ID)) {
>> + return -1;
>> + }
>> +#endif
>
> Maybe:
>
> if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) &&
> WARN_ON_ONCE(!radix_enabled() && get_region_id(addr) == LINEAR_MAP_REGION_ID)) {
> return -1;
> }
get_region_id() only exists for book3s/64 at the time being, and LINEAR_MAP_REGION_ID as well.
>
> But then Aneesh pointed out that we should also block VMEMMAP_REGION_ID.
>
> It might be better to just check for the permitted regions.
>
> 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 v2 3/7] powerpc: convert config files to generic cmdline
From: Will Deacon @ 2021-03-31 11:52 UTC (permalink / raw)
To: Daniel Walker
Cc: Rob Herring, Daniel Gimpelevich, X86 ML, open list:MIPS,
linux-kernel@vger.kernel.org, Paul Mackerras, xe-linux-external,
Andrew Morton, linuxppc-dev
In-Reply-To: <20210330173521.GT109100@zorba>
On Tue, Mar 30, 2021 at 10:35:21AM -0700, Daniel Walker wrote:
> On Mon, Mar 29, 2021 at 11:07:51AM +0100, Will Deacon wrote:
> > On Thu, Mar 25, 2021 at 12:59:56PM -0700, Daniel Walker wrote:
> > > On Thu, Mar 25, 2021 at 01:03:55PM +0100, Christophe Leroy wrote:
> > > >
> > > > Ok, so you agree we don't need to provide two CMDLINE, one to be appended and one to be prepended.
> > > >
> > > > Let's only provide once CMDLINE as of today, and ask the user to select
> > > > whether he wants it appended or prepended or replacee. Then no need to
> > > > change all existing config to rename CONFIG_CMDLINE into either of the new
> > > > ones.
> > > >
> > > > That's the main difference between my series and Daniel's series. So I'll
> > > > finish taking Will's comment into account and we'll send out a v3 soon.
> > >
> > > It doesn't solve the needs of Cisco, I've stated many times your changes have
> > > little value. Please stop submitting them.
> >
> > FWIW, they're useful for arm64 and I will gladly review the updated series.
> >
> > I don't think asking people to stop submitting patches is ever the right
> > answer. Please don't do that.
>
> Why ? It's me nacking his series, is that not allowed anymore ?
If you're that way inclined then you can "nack" whatever you want, but
please allow the rest of us to continue reviewing the patches. You don't
have any basis on which to veto other people's contributions and so
demanding that somebody stops posting code is neither constructive nor
meaningful.
Will
^ permalink raw reply
* Re: [PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
From: Will Deacon @ 2021-03-31 11:49 UTC (permalink / raw)
To: Robin Murphy
Cc: freedreno, kvm, Joerg Roedel, linuxppc-dev, dri-devel, Li Yang,
iommu, netdev, linux-arm-msm, virtualization, David Woodhouse,
Christoph Hellwig, linux-arm-kernel, Lu Baolu
In-Reply-To: <578d6aa5-4239-f5d7-2e9f-686b18e52bba@arm.com>
On Tue, Mar 30, 2021 at 05:28:19PM +0100, Robin Murphy wrote:
> On 2021-03-30 14:58, Will Deacon wrote:
> > On Tue, Mar 30, 2021 at 02:19:38PM +0100, Robin Murphy wrote:
> > > On 2021-03-30 14:11, Will Deacon wrote:
> > > > On Tue, Mar 16, 2021 at 04:38:22PM +0100, Christoph Hellwig wrote:
> > > > > From: Robin Murphy <robin.murphy@arm.com>
> > > > >
> > > > > Instead make the global iommu_dma_strict paramete in iommu.c canonical by
> > > > > exporting helpers to get and set it and use those directly in the drivers.
> > > > >
> > > > > This make sure that the iommu.strict parameter also works for the AMD and
> > > > > Intel IOMMU drivers on x86. As those default to lazy flushing a new
> > > > > IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to
> > > > > represent the default if not overriden by an explicit parameter.
> > > > >
> > > > > Signed-off-by: Robin Murphy <robin.murphy@arm.com>.
> > > > > [ported on top of the other iommu_attr changes and added a few small
> > > > > missing bits]
> > > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > > ---
> > > > > drivers/iommu/amd/iommu.c | 23 +-------
> > > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +---------------
> > > > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 -
> > > > > drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +--------
> > > > > drivers/iommu/dma-iommu.c | 9 +--
> > > > > drivers/iommu/intel/iommu.c | 64 ++++-----------------
> > > > > drivers/iommu/iommu.c | 27 ++++++---
> > > > > include/linux/iommu.h | 4 +-
> > > > > 8 files changed, 40 insertions(+), 165 deletions(-)
> > > >
> > > > I really like this cleanup, but I can't help wonder if it's going in the
> > > > wrong direction. With SoCs often having multiple IOMMU instances and a
> > > > distinction between "trusted" and "untrusted" devices, then having the
> > > > flush-queue enabled on a per-IOMMU or per-domain basis doesn't sound
> > > > unreasonable to me, but this change makes it a global property.
> > >
> > > The intent here was just to streamline the existing behaviour of stuffing a
> > > global property into a domain attribute then pulling it out again in the
> > > illusion that it was in any way per-domain. We're still checking
> > > dev_is_untrusted() before making an actual decision, and it's not like we
> > > can't add more factors at that point if we want to.
> >
> > Like I say, the cleanup is great. I'm just wondering whether there's a
> > better way to express the complicated logic to decide whether or not to use
> > the flush queue than what we end up with:
> >
> > if (!cookie->fq_domain && (!dev || !dev_is_untrusted(dev)) &&
> > domain->ops->flush_iotlb_all && !iommu_get_dma_strict())
> >
> > which is mixing up globals, device properties and domain properties. The
> > result is that the driver code ends up just using the global to determine
> > whether or not to pass IO_PGTABLE_QUIRK_NON_STRICT to the page-table code,
> > which is a departure from the current way of doing things.
>
> But previously, SMMU only ever saw the global policy piped through the
> domain attribute by iommu_group_alloc_default_domain(), so there's no
> functional change there.
For DMA domains sure, but I don't think that's the case for unmanaged
domains such as those used by VFIO.
> Obviously some of the above checks could be factored out into some kind of
> iommu_use_flush_queue() helper that IOMMU drivers can also call if they need
> to keep in sync. Or maybe we just allow iommu-dma to set
> IO_PGTABLE_QUIRK_NON_STRICT directly via iommu_set_pgtable_quirks() if we're
> treating that as a generic thing now.
I think a helper that takes a domain would be a good starting point.
Will
^ permalink raw reply
* Re: [PATCH v10 06/10] powerpc/mm/ptdump: debugfs handler for W+X checks at runtime
From: Michael Ellerman @ 2021-03-31 11:24 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev
Cc: ajd, Kees Cook, Jordan Niethe, cmr, npiggin, naveen.n.rao, dja
In-Reply-To: <20210330045132.722243-7-jniethe5@gmail.com>
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.
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.
cheers
^ permalink raw reply
* Re: [PATCH v10 01/10] powerpc/mm: Implement set_memory() routines
From: Michael Ellerman @ 2021-03-31 11:16 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev
Cc: ajd, Jordan Niethe, cmr, npiggin, naveen.n.rao, dja
In-Reply-To: <20210330045132.722243-2-jniethe5@gmail.com>
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 ?
> 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.
> + */
> +
> +#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".
> + * 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?
> + */
> +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.
> + 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.
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:
> +#ifdef CONFIG_PPC_BOOK3S_64
> + if (WARN_ON_ONCE(!radix_enabled() &&
> + get_region_id(addr) == LINEAR_MAP_REGION_ID)) {
> + return -1;
> + }
> +#endif
Maybe:
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.
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 05/10] powerpc/bpf: Write protect JIT code
From: Christophe Leroy @ 2021-03-31 10:39 UTC (permalink / raw)
To: Michael Ellerman, Jordan Niethe, linuxppc-dev
Cc: naveen.n.rao, cmr, ajd, npiggin, dja
In-Reply-To: <87wntnwqw9.fsf@mpe.ellerman.id.au>
Le 31/03/2021 à 12:37, Michael Ellerman a écrit :
> 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.
I was about to do the same comment, but ....
CONFIG_STRICT_MODULE_RWX is not CONFIG_ARCH_HAS_SET_MEMORY
>
>> 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.
Same
>
>> @@ -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?) ?
>
> 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 05/10] powerpc/bpf: Write protect JIT code
From: Michael Ellerman @ 2021-03-31 10:37 UTC (permalink / raw)
To: Jordan Niethe, linuxppc-dev
Cc: ajd, Jordan Niethe, cmr, npiggin, naveen.n.rao, dja
In-Reply-To: <20210330045132.722243-6-jniethe5@gmail.com>
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.
> 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?) ?
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 v3] powerpc/papr_scm: Implement support for H_SCM_FLUSH hcall
From: Michael Ellerman @ 2021-03-31 10:20 UTC (permalink / raw)
To: Aneesh Kumar K.V, Shivaprasad G Bhat, sbhat, linuxppc-dev,
kvm-ppc, linux-nvdimm
Cc: vaibhav, linux-doc
In-Reply-To: <87mtul6xzj.fsf@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> Shivaprasad G Bhat <sbhat@linux.ibm.com> writes:
>
>> Add support for ND_REGION_ASYNC capability if the device tree
>> indicates 'ibm,hcall-flush-required' property in the NVDIMM node.
>> Flush is done by issuing H_SCM_FLUSH hcall to the hypervisor.
>>
>> If the flush request failed, the hypervisor is expected to
>> to reflect the problem in the subsequent nvdimm H_SCM_HEALTH call.
>>
>> This patch prevents mmap of namespaces with MAP_SYNC flag if the
>> nvdimm requires an explicit flush[1].
>>
>> References:
>> [1] https://github.com/avocado-framework-tests/avocado-misc-tests/blob/master/memory/ndctl.py.data/map_sync.c
>
>
> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Do we need an ack from nvdimm folks on this?
Or is it entirely powerpc internal (seems like it from the diffstat)?
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/vdso: Separate vvar vma from vdso
From: Michael Ellerman @ 2021-03-31 9:59 UTC (permalink / raw)
To: Christophe Leroy, Dmitry Safonov, linux-kernel
Cc: Dmitry Safonov, stable, Andrei Vagin, Paul Mackerras,
Andy Lutomirski, Laurent Dufour, linuxppc-dev
In-Reply-To: <09e8d68d-54fe-e327-b44f-8f68543edba1@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 26/03/2021 à 20:17, Dmitry Safonov a écrit :
>> Since commit 511157ab641e ("powerpc/vdso: Move vdso datapage up front")
>> VVAR page is in front of the VDSO area. In result it breaks CRIU
>> (Checkpoint Restore In Userspace) [1], where CRIU expects that "[vdso]"
>> from /proc/../maps points at ELF/vdso image, rather than at VVAR data page.
>> Laurent made a patch to keep CRIU working (by reading aux vector).
>> But I think it still makes sence to separate two mappings into different
>> VMAs. It will also make ppc64 less "special" for userspace and as
>> a side-bonus will make VVAR page un-writable by debugger (which previously
>> would COW page and can be unexpected).
>>
>> I opportunistically Cc stable on it: I understand that usually such
>> stuff isn't a stable material, but that will allow us in CRIU have
>> one workaround less that is needed just for one release (v5.11) on
>> one platform (ppc64), which we otherwise have to maintain.
>> I wouldn't go as far as to say that the commit 511157ab641e is ABI
>> regression as no other userspace got broken, but I'd really appreciate
>> if it gets backported to v5.11 after v5.12 is released, so as not
>> to complicate already non-simple CRIU-vdso code. Thanks!
>>
>> Cc: Andrei Vagin <avagin@gmail.com>
>> Cc: Andy Lutomirski <luto@kernel.org>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Cc: Laurent Dufour <ldufour@linux.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: stable@vger.kernel.org # v5.11
>> [1]: https://github.com/checkpoint-restore/criu/issues/1417
>> Signed-off-by: Dmitry Safonov <dima@arista.com>
>> Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/include/asm/mmu_context.h | 2 +-
>> arch/powerpc/kernel/vdso.c | 54 +++++++++++++++++++-------
>> 2 files changed, 40 insertions(+), 16 deletions(-)
>>
>
>> @@ -133,7 +135,13 @@ static int __arch_setup_additional_pages(struct linux_binprm *bprm, int uses_int
>> * install_special_mapping or the perf counter mmap tracking code
>> * will fail to recognise it as a vDSO.
>> */
>> - mm->context.vdso = (void __user *)vdso_base + PAGE_SIZE;
>> + mm->context.vdso = (void __user *)vdso_base + vvar_size;
>> +
>> + vma = _install_special_mapping(mm, vdso_base, vvar_size,
>> + VM_READ | VM_MAYREAD | VM_IO |
>> + VM_DONTDUMP | VM_PFNMAP, &vvar_spec);
>> + if (IS_ERR(vma))
>> + return PTR_ERR(vma);
>>
>> /*
>> * our vma flags don't have VM_WRITE so by default, the process isn't
>
>
> IIUC, VM_PFNMAP is for when we have a vvar_fault handler.
Some of the other flags seem odd too.
eg. VM_IO ? VM_DONTDUMP ?
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/traps: Enhance readability for trap types
From: Michael Ellerman @ 2021-03-31 9:58 UTC (permalink / raw)
To: Xiongwei Song, benh, paulus, oleg, npiggin, christophe.leroy,
msuchanek, aneesh.kumar, ravi.bangoria, mikey, haren, alistair,
jniethe5, peterz, leobras.c, akpm, rppt, peterx, atrajeev, maddy,
kjain, kan.liang, aik, pmladek, john.ogness
Cc: Xiongwei Song, linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <20210330150425.10145-1-sxwjean@me.com>
Xiongwei Song <sxwjean@me.com> writes:
> From: Xiongwei Song <sxwjean@gmail.com>
>
> Create a new header named traps.h, define macros to list ppc exception
> types in traps.h, replace the reference of the real trap values with
> these macros.
Personally I find the hex values easier to recognise, but I realise
that's probably not true of other people :)
...
> diff --git a/arch/powerpc/include/asm/traps.h b/arch/powerpc/include/asm/traps.h
> new file mode 100644
> index 000000000000..a31b6122de23
> --- /dev/null
> +++ b/arch/powerpc/include/asm/traps.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_PPC_TRAPS_H
> +#define _ASM_PPC_TRAPS_H
> +
> +#define TRAP_RESET 0x100 /* System reset */
> +#define TRAP_MCE 0x200 /* Machine check */
> +#define TRAP_DSI 0x300 /* Data storage */
> +#define TRAP_DSEGI 0x380 /* Data segment */
> +#define TRAP_ISI 0x400 /* Instruction storage */
> +#define TRAP_ISEGI 0x480 /* Instruction segment */
> +#define TRAP_ALIGN 0x600 /* Alignment */
> +#define TRAP_PROG 0x700 /* Program */
> +#define TRAP_DEC 0x900 /* Decrementer */
> +#define TRAP_SYSCALL 0xc00 /* System call */
> +#define TRAP_TRACEI 0xd00 /* Trace */
> +#define TRAP_FPA 0xe00 /* Floating-point Assist */
> +#define TRAP_PMI 0xf00 /* Performance monitor */
I know the macro is called TRAP and the field in pt_regs is called trap,
but the terminology in the architecture is "exception", and we already
have many uses of that. In particular we have a lot of uses of "exc" as
an abbreviation for "exception". So I think I'd rather we use that than
"TRAP".
I think we should probably use the names from the ISA, unless they are
really over long.
Which are:
0x100 System Reset
0x200 Machine Check
0x300 Data Storage
0x380 Data Segment
0x400 Instruction Storage
0x480 Instruction Segment
0x500 External
0x600 Alignment
0x700 Program
0x800 Floating-Point Unavailable
0x900 Decrementer
0x980 Hypervisor Decrementer
0xA00 Directed Privileged Doorbell
0xC00 System Call
0xD00 Trace
0xE00 Hypervisor Data Storage
0xE20 Hypervisor Instruction Storage
0xE40 Hypervisor Emulation Assistance
0xE60 Hypervisor Maintenance
0xE80 Directed Hypervisor Doorbell
0xEA0 Hypervisor Virtualization
0xF00 Performance Monitor
0xF20 Vector Unavailable
0xF40 VSX Unavailable
0xF60 Facility Unavailable
0xF80 Hypervisor Facility Unavailable
0xFA0 Directed Ultravisor Doorbell
So perhaps:
EXC_SYSTEM_RESET
EXC_MACHINE_CHECK
EXC_DATA_STORAGE
EXC_DATA_SEGMENT
EXC_INST_STORAGE
EXC_INST_SEGMENT
EXC_EXTERNAL_INTERRUPT
EXC_ALIGNMENT
EXC_PROGRAM_CHECK
EXC_FP_UNAVAILABLE
EXC_DECREMENTER
EXC_HV_DECREMENTER
EXC_SYSTEM_CALL
EXC_HV_DATA_STORAGE
EXC_PERF_MONITOR
cheers
^ permalink raw reply
* Re: [PATCH v2] mm: Move mem_init_print_info() into mm_init()
From: Russell King - ARM Linux admin @ 2021-03-31 9:34 UTC (permalink / raw)
To: Kefeng Wang
Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
Dave Hansen, linux-mips, linux-mm, Guo Ren, sparclinux,
linux-riscv, Jonas Bonn, linux-s390, Yoshinori Sato,
linux-hexagon, Huacai Chen, linux-csky, Ingo Molnar,
linux-snps-arc, linux-xtensa, Heiko Carstens, linux-um,
linux-m68k, openrisc, linux-arm-kernel, Richard Henderson,
linux-parisc, linux-kernel, Palmer Dabbelt, linux-alpha,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20210317015210.33641-1-wangkefeng.wang@huawei.com>
On Wed, Mar 17, 2021 at 09:52:10AM +0800, Kefeng Wang wrote:
> mem_init_print_info() is called in mem_init() on each architecture,
> and pass NULL argument, so using void argument and move it into mm_init().
>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk> # for arm bits
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH v2] mm: Move mem_init_print_info() into mm_init()
From: Mike Rapoport @ 2021-03-31 9:28 UTC (permalink / raw)
To: Kefeng Wang
Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
Dave Hansen, linux-mips, linux-mm, Guo Ren, sparclinux,
linux-riscv, Jonas Bonn, linux-s390, Yoshinori Sato,
linux-hexagon, Huacai Chen, Russell King, linux-csky, Ingo Molnar,
linux-snps-arc, linux-xtensa, Heiko Carstens, linux-um,
linux-m68k, openrisc, linux-arm-kernel, Richard Henderson,
linux-parisc, linux-kernel, Palmer Dabbelt, linux-alpha,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20210317015210.33641-1-wangkefeng.wang@huawei.com>
On Wed, Mar 17, 2021 at 09:52:10AM +0800, Kefeng Wang wrote:
> mem_init_print_info() is called in mem_init() on each architecture,
> and pass NULL argument, so using void argument and move it into mm_init().
>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Acked-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
> v2:
> - Cleanup 'str' line suggested by Christophe and ACK
>
^ permalink raw reply
* [PATCH -next] powerpc: Remove duplicated include from time.c
From: Qiheng Lin @ 2021-03-31 8:58 UTC (permalink / raw)
To: linqiheng, Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Ingo Molnar, Geert Uytterhoeven, Peter Zijlstra,
Frederic Weisbecker
Cc: kernel-janitors, linuxppc-dev, linux-kernel, Hulk Robot
Remove duplicated include.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Qiheng Lin <linqiheng@huawei.com>
---
arch/powerpc/kernel/time.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index b67d93a609a2..2c8762002b21 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -53,7 +53,6 @@
#include <linux/of_clk.h>
#include <linux/suspend.h>
#include <linux/sched/cputime.h>
-#include <linux/sched/clock.h>
#include <linux/processor.h>
#include <asm/trace.h>
^ permalink raw reply related
* Re: WARNING: CPU: 0 PID: 1 at arch/powerpc/lib/feature-fixups.c:109 do_feature_fixups+0xb0/0xf0
From: Christophe Leroy @ 2021-03-31 9:15 UTC (permalink / raw)
To: Paul Menzel; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <bc1d3a33-2499-994e-860a-b50cf72b9619@molgen.mpg.de>
Hi Paul,
Le 30/03/2021 à 12:37, Paul Menzel a écrit :
> Dear Linux folks,
>
>
> On the POWER8 system IBM S822LC, Linux 5.12-rc5+ logs the warning below.
>
> ```
> [ 0.724118] Unable to patch feature section at (____ptrval____) - (____ptrval____) with
> (____ptrval____) - (____ptrval____)
> [ 0.724185] pstore: Registered nvram as persistent store backend
> ```
>
> Please find the output of `dmesg` attached.
>
>
Did you do a 'make clean' before building ?
If not, can you try patch
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/8bb015bc98c51d8ced581415b7e3d157e18da7c9.1617181918.git.christophe.leroy@csgroup.eu/
Thanks
Christophe
^ permalink raw reply
* [PATCH] powerpc/vdso: Make sure vdso_wrapper.o is rebuilt everytime vdso.so is rebuilt
From: Christophe Leroy @ 2021-03-31 9:12 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
masahiroy
Cc: linuxppc-dev, linux-kernel
Commit bce74491c300 ("powerpc/vdso: fix unnecessary rebuilds of
vgettimeofday.o") moved vdso32_wrapper.o and vdso64_wrapper.o out
of arch/powerpc/kernel/vdso[32/64]/ and removed the dependencies in
the Makefile. This leads to the wrappers not being re-build hence the
kernel embedding the old vdso library.
Add back missing dependencies to ensure vdso32_wrapper.o and vdso64_wrapper.o
are rebuilt when vdso32.so.dbg and vdso64.so.dbg are changed.
Fixes: bce74491c300 ("powerpc/vdso: fix unnecessary rebuilds of vgettimeofday.o")
Cc: stable@vger.kernel.org
Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 6084fa499aa3..f66b63e81c3b 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -191,3 +191,7 @@ $(obj)/prom_init_check: $(src)/prom_init_check.sh $(obj)/prom_init.o FORCE
targets += prom_init_check
clean-files := vmlinux.lds
+
+# Force dependency (incbin is bad)
+$(obj)/vdso32_wrapper.o : $(obj)/vdso32/vdso32.so.dbg
+$(obj)/vdso64_wrapper.o : $(obj)/vdso64/vdso64.so.dbg
--
2.25.0
^ permalink raw reply related
* Re: [PATCH 1/2] powerpc/vdso: fix unnecessary rebuilds of vgettimeofday.o
From: Christophe Leroy @ 2021-03-31 9:03 UTC (permalink / raw)
To: Masahiro Yamada, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev
Cc: Ravi Bangoria, Linux Kernel Mailing List, Nicholas Piggin,
Oliver O'Halloran, Greentime Hu, Michal Suchanek,
Ard Biesheuvel, Daniel Axtens
In-Reply-To: <CAK7LNASEVM8e5hohV4jbXOvMxSJ_Prm3es+fhezPkRc6UL=vdw@mail.gmail.com>
Le 28/01/2021 à 05:01, Masahiro Yamada a écrit :
> On Thu, Dec 24, 2020 at 2:12 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>
>> vgettimeofday.o is unnecessarily rebuilt. Adding it to 'targets' is not
>> enough to fix the issue. Kbuild is correctly rebuilding it because the
>> command line is changed.
>>
>> PowerPC builds each vdso directory twice; first in vdso_prepare to
>> generate vdso{32,64}-offsets.h, second as part of the ordinary build
>> process to embed vdso{32,64}.so.dbg into the kernel.
>>
>> The problem shows up when CONFIG_PPC_WERROR=y due to the following line
>> in arch/powerpc/Kbuild:
>>
>> subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
>>
>> In the preparation stage, Kbuild directly visits the vdso directories,
>> hence it does not inherit subdir-ccflags-y. In the second descend,
>> Kbuild adds -Werror, which results in the command line flipping
>> with/without -Werror.
>>
>> It implies a potential danger; if a more critical flag that would impact
>> the resulted vdso, the offsets recorded in the headers might be different
>> from real offsets in the embedded vdso images.
>>
>> Removing the unneeded second descend solves the problem.
>>
>> Link: https://lore.kernel.org/linuxppc-dev/87tuslxhry.fsf@mpe.ellerman.id.au/
>> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>> ---
>
>
> Michael, please take a look at this.
>
> The unneeded rebuild problem is still remaining.
Seems like with this patch, vdso32_wrapper.o is not rebuilt when vdso32.so.dbg is rebuilt.
Leading to ... disasters.
I'll send a patch
>
>
>>
>> arch/powerpc/kernel/Makefile | 4 ++--
>> arch/powerpc/kernel/vdso32/Makefile | 5 +----
>> arch/powerpc/kernel/{vdso32 => }/vdso32_wrapper.S | 0
>> arch/powerpc/kernel/vdso64/Makefile | 6 +-----
>> arch/powerpc/kernel/{vdso64 => }/vdso64_wrapper.S | 0
>> 5 files changed, 4 insertions(+), 11 deletions(-)
>> rename arch/powerpc/kernel/{vdso32 => }/vdso32_wrapper.S (100%)
>> rename arch/powerpc/kernel/{vdso64 => }/vdso64_wrapper.S (100%)
>>
>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
>> index fe2ef598e2ea..79ee7750937d 100644
>> --- a/arch/powerpc/kernel/Makefile
>> +++ b/arch/powerpc/kernel/Makefile
>> @@ -51,7 +51,7 @@ obj-y += ptrace/
>> obj-$(CONFIG_PPC64) += setup_64.o \
>> paca.o nvram_64.o note.o syscall_64.o
>> obj-$(CONFIG_COMPAT) += sys_ppc32.o signal_32.o
>> -obj-$(CONFIG_VDSO32) += vdso32/
>> +obj-$(CONFIG_VDSO32) += vdso32_wrapper.o
>> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
>> obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
>> obj-$(CONFIG_PPC_DAWR) += dawr.o
>> @@ -60,7 +60,7 @@ obj-$(CONFIG_PPC_BOOK3S_64) += cpu_setup_power.o
>> obj-$(CONFIG_PPC_BOOK3S_64) += mce.o mce_power.o
>> obj-$(CONFIG_PPC_BOOK3E_64) += exceptions-64e.o idle_book3e.o
>> obj-$(CONFIG_PPC_BARRIER_NOSPEC) += security.o
>> -obj-$(CONFIG_PPC64) += vdso64/
>> +obj-$(CONFIG_PPC64) += vdso64_wrapper.o
>> obj-$(CONFIG_ALTIVEC) += vecemu.o
>> obj-$(CONFIG_PPC_BOOK3S_IDLE) += idle_book3s.o
>> procfs-y := proc_powerpc.o
>> diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
>> index 59aa2944ecae..42fc3de89b39 100644
>> --- a/arch/powerpc/kernel/vdso32/Makefile
>> +++ b/arch/powerpc/kernel/vdso32/Makefile
>> @@ -30,7 +30,7 @@ CC32FLAGS += -m32
>> KBUILD_CFLAGS := $(filter-out -mcmodel=medium,$(KBUILD_CFLAGS))
>> endif
>>
>> -targets := $(obj-vdso32) vdso32.so.dbg
>> +targets := $(obj-vdso32) vdso32.so.dbg vgettimeofday.o
>> obj-vdso32 := $(addprefix $(obj)/, $(obj-vdso32))
>>
>> GCOV_PROFILE := n
>> @@ -46,9 +46,6 @@ obj-y += vdso32_wrapper.o
>> targets += vdso32.lds
>> CPPFLAGS_vdso32.lds += -P -C -Upowerpc
>>
>> -# Force dependency (incbin is bad)
>> -$(obj)/vdso32_wrapper.o : $(obj)/vdso32.so.dbg
>> -
>> # link rule for the .so file, .lds has to be first
>> $(obj)/vdso32.so.dbg: $(src)/vdso32.lds $(obj-vdso32) $(obj)/vgettimeofday.o FORCE
>> $(call if_changed,vdso32ld_and_check)
>> diff --git a/arch/powerpc/kernel/vdso32/vdso32_wrapper.S b/arch/powerpc/kernel/vdso32_wrapper.S
>> similarity index 100%
>> rename from arch/powerpc/kernel/vdso32/vdso32_wrapper.S
>> rename to arch/powerpc/kernel/vdso32_wrapper.S
>> diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
>> index d365810a689a..b50b39fedf74 100644
>> --- a/arch/powerpc/kernel/vdso64/Makefile
>> +++ b/arch/powerpc/kernel/vdso64/Makefile
>> @@ -17,7 +17,7 @@ endif
>>
>> # Build rules
>>
>> -targets := $(obj-vdso64) vdso64.so.dbg
>> +targets := $(obj-vdso64) vdso64.so.dbg vgettimeofday.o
>> obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
>>
>> GCOV_PROFILE := n
>> @@ -29,15 +29,11 @@ ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
>> -Wl,-soname=linux-vdso64.so.1 -Wl,--hash-style=both
>> asflags-y := -D__VDSO64__ -s
>>
>> -obj-y += vdso64_wrapper.o
>> targets += vdso64.lds
>> CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)
>>
>> $(obj)/vgettimeofday.o: %.o: %.c FORCE
>>
>> -# Force dependency (incbin is bad)
>> -$(obj)/vdso64_wrapper.o : $(obj)/vdso64.so.dbg
>> -
>> # link rule for the .so file, .lds has to be first
>> $(obj)/vdso64.so.dbg: $(src)/vdso64.lds $(obj-vdso64) $(obj)/vgettimeofday.o FORCE
>> $(call if_changed,vdso64ld_and_check)
>> diff --git a/arch/powerpc/kernel/vdso64/vdso64_wrapper.S b/arch/powerpc/kernel/vdso64_wrapper.S
>> similarity index 100%
>> rename from arch/powerpc/kernel/vdso64/vdso64_wrapper.S
>> rename to arch/powerpc/kernel/vdso64_wrapper.S
>> --
>> 2.27.0
>>
>
>
^ permalink raw reply
* Re: [PATCH] PCI: Try to find two continuous regions for child resource
From: Kai-Heng Feng @ 2021-03-31 8:53 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Qian Cai, Michal Simek, Chen Zhou, Arnd Bergmann,
open list:SPARC + UltraSPARC (sparc/sparc64),
Alexey Kardashevskiy, open list:PCI SUBSYSTEM, open list,
Dominik Brodowski, Frederic Barrat, Qinglang Miao, Paul Mackerras,
Cédric Le Goater, Bjorn Helgaas, Oliver O'Halloran,
Andrew Morton, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
David S. Miller, Mike Rapoport
In-Reply-To: <20210329162250.GA1167957@bjorn-Precision-5520>
On Tue, Mar 30, 2021 at 12:23 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Mar 29, 2021 at 04:47:59PM +0800, Kai-Heng Feng wrote:
> > Built-in grahpics on HP EliteDesk 805 G6 doesn't work because graphics
> > can't get the BAR it needs:
> > [ 0.611504] pci_bus 0000:00: root bus resource [mem 0x10020200000-0x100303fffff window]
> > [ 0.611505] pci_bus 0000:00: root bus resource [mem 0x10030400000-0x100401fffff window]
> > ...
> > [ 0.638083] pci 0000:00:08.1: bridge window [mem 0xd2000000-0xd23fffff]
> > [ 0.638086] pci 0000:00:08.1: bridge window [mem 0x10030000000-0x100401fffff 64bit pref]
> > [ 0.962086] pci 0000:00:08.1: can't claim BAR 15 [mem 0x10030000000-0x100401fffff 64bit pref]: no compatible bridge window
> > [ 0.962086] pci 0000:00:08.1: [mem 0x10030000000-0x100401fffff 64bit pref] clipped to [mem 0x10030000000-0x100303fffff 64bit pref]
> > [ 0.962086] pci 0000:00:08.1: bridge window [mem 0x10030000000-0x100303fffff 64bit pref]
> > [ 0.962086] pci 0000:07:00.0: can't claim BAR 0 [mem 0x10030000000-0x1003fffffff 64bit pref]: no compatible bridge window
> > [ 0.962086] pci 0000:07:00.0: can't claim BAR 2 [mem 0x10040000000-0x100401fffff 64bit pref]: no compatible bridge window
> >
> > However, the root bus has two continuous regions that can contain the
> > child resource requested.
> >
> > So try to find another parent region if two regions are continuous and
> > can contain child resource. This change makes the grahpics works on the
> > system in question.
>
> The BIOS description of PCI0 is interesting:
>
> pci_bus 0000:00: root bus resource [mem 0x10000000000-0x100201fffff window]
> pci_bus 0000:00: root bus resource [mem 0x10020200000-0x100303fffff window]
> pci_bus 0000:00: root bus resource [mem 0x10030400000-0x100401fffff window]
>
> So the PCI0 _CRS apparently gave us:
>
> [mem 0x10000000000-0x100201fffff] size 0x20200000 (512MB + 2MB)
> [mem 0x10020200000-0x100303fffff] size 0x10200000 (256MB + 2MB)
> [mem 0x10030400000-0x100401fffff] size 0x0fe00000 (254MB)
>
> These are all contiguous, so we'd have no problem if we coalesced them
> into a single window:
>
> [mem 0x10000000000-0x100401fffff window] size 0x40200000 (1GB + 2MB)
>
> I think we currently keep these root bus resources separate because if
> we ever support _SRS for host bridges, the argument we give to _SRS
> must be exactly the same format as what we got from _CRS (see ACPI
> v6.3, sec 6.2.16, and pnpacpi_set_resources()).
>
> pnpacpi_encode_resources() is currently very simple-minded and copies
> each device resource back into a single _SRS entry. But (1) we don't
> support _SRS for host bridges, and (2) if we ever do, we can make
> pnpacpi_encode_resources() smarter so it breaks things back up.
>
> So I think we should try to fix this by coalescing these adjacent
> resources from _CRS so we end up with a single root bus resource that
> covers all contiguous regions.
Thanks for the tip! Working on v2 patch.
>
> Typos, etc:
> - No need for the timestamps; they're not relevant to the problem.
> - s/grahpics/graphics/ (two occurrences above)
> - s/continuous/contiguous/ (three occurrences above)
Will also update those in v2.
Kai-Heng
>
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=212013
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > arch/microblaze/pci/pci-common.c | 4 +--
> > arch/powerpc/kernel/pci-common.c | 8 ++---
> > arch/sparc/kernel/pci.c | 4 +--
> > drivers/pci/pci.c | 60 +++++++++++++++++++++++++++-----
> > drivers/pci/setup-res.c | 21 +++++++----
> > drivers/pcmcia/rsrc_nonstatic.c | 4 +--
> > include/linux/pci.h | 6 ++--
> > 7 files changed, 80 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
> > index 557585f1be41..8e65832fb510 100644
> > --- a/arch/microblaze/pci/pci-common.c
> > +++ b/arch/microblaze/pci/pci-common.c
> > @@ -669,7 +669,7 @@ static void pcibios_allocate_bus_resources(struct pci_bus *bus)
> > {
> > struct pci_bus *b;
> > int i;
> > - struct resource *res, *pr;
> > + struct resource *res, *pr = NULL;
> >
> > pr_debug("PCI: Allocating bus resources for %04x:%02x...\n",
> > pci_domain_nr(bus), bus->number);
> > @@ -688,7 +688,7 @@ static void pcibios_allocate_bus_resources(struct pci_bus *bus)
> > * and as such ensure proper re-allocation
> > * later.
> > */
> > - pr = pci_find_parent_resource(bus->self, res);
> > + pci_find_parent_resource(bus->self, res, &pr, NULL);
> > if (pr == res) {
> > /* this happens when the generic PCI
> > * code (wrongly) decides that this
> > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> > index 001e90cd8948..f865354b746d 100644
> > --- a/arch/powerpc/kernel/pci-common.c
> > +++ b/arch/powerpc/kernel/pci-common.c
> > @@ -1196,7 +1196,7 @@ static void pcibios_allocate_bus_resources(struct pci_bus *bus)
> > {
> > struct pci_bus *b;
> > int i;
> > - struct resource *res, *pr;
> > + struct resource *res, *pr = NULL;
> >
> > pr_debug("PCI: Allocating bus resources for %04x:%02x...\n",
> > pci_domain_nr(bus), bus->number);
> > @@ -1213,7 +1213,7 @@ static void pcibios_allocate_bus_resources(struct pci_bus *bus)
> > pr = (res->flags & IORESOURCE_IO) ?
> > &ioport_resource : &iomem_resource;
> > else {
> > - pr = pci_find_parent_resource(bus->self, res);
> > + pci_find_parent_resource(bus->self, res, &pr, NULL);
> > if (pr == res) {
> > /* this happens when the generic PCI
> > * code (wrongly) decides that this
> > @@ -1265,12 +1265,12 @@ static void pcibios_allocate_bus_resources(struct pci_bus *bus)
> >
> > static inline void alloc_resource(struct pci_dev *dev, int idx)
> > {
> > - struct resource *pr, *r = &dev->resource[idx];
> > + struct resource *pr = NULL, *r = &dev->resource[idx];
> >
> > pr_debug("PCI: Allocating %s: Resource %d: %pR\n",
> > pci_name(dev), idx, r);
> >
> > - pr = pci_find_parent_resource(dev, r);
> > + pci_find_parent_resource(dev, r, &pr, NULL);
> > if (!pr || (pr->flags & IORESOURCE_UNSET) ||
> > request_resource(pr, r) < 0) {
> > printk(KERN_WARNING "PCI: Cannot allocate resource region %d"
> > diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
> > index 9c2b720bfd20..b4006798e4e1 100644
> > --- a/arch/sparc/kernel/pci.c
> > +++ b/arch/sparc/kernel/pci.c
> > @@ -621,7 +621,7 @@ static void pci_bus_register_of_sysfs(struct pci_bus *bus)
> > static void pci_claim_legacy_resources(struct pci_dev *dev)
> > {
> > struct pci_bus_region region;
> > - struct resource *p, *root, *conflict;
> > + struct resource *p, *root = NULL, *conflict;
> >
> > if ((dev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> > return;
> > @@ -637,7 +637,7 @@ static void pci_claim_legacy_resources(struct pci_dev *dev)
> > region.end = region.start + 0x1ffffUL;
> > pcibios_bus_to_resource(dev->bus, p, ®ion);
> >
> > - root = pci_find_parent_resource(dev, p);
> > + pci_find_parent_resource(dev, p, &root, NULL);
> > if (!root) {
> > pci_info(dev, "can't claim VGA legacy %pR: no compatible bridge window\n", p);
> > goto err;
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 16a17215f633..abbcd2dcdc02 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -693,20 +693,25 @@ u8 pci_find_ht_capability(struct pci_dev *dev, int ht_cap)
> > EXPORT_SYMBOL_GPL(pci_find_ht_capability);
> >
> > /**
> > - * pci_find_parent_resource - return resource region of parent bus of given
> > + * pci_find_parent_resource - find resource region of parent bus of given
> > * region
> > * @dev: PCI device structure contains resources to be searched
> > * @res: child resource record for which parent is sought
> > + * @first: the first region that contains the child resource
> > + * @second: the second region that combines with the first region to fully
> > + * contains the child resource
> > *
> > * For given resource region of given device, return the resource region of
> > * parent bus the given region is contained in.
> > */
> > -struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> > - struct resource *res)
> > +void pci_find_parent_resource(const struct pci_dev *dev,
> > + struct resource *res,
> > + struct resource **first,
> > + struct resource **second)
> > {
> > const struct pci_bus *bus = dev->bus;
> > struct resource *r;
> > - int i;
> > + int i, overlaps = 0;
> >
> > pci_bus_for_each_resource(bus, r, i) {
> > if (!r)
> > @@ -718,8 +723,10 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> > * not, the allocator made a mistake.
> > */
> > if (r->flags & IORESOURCE_PREFETCH &&
> > - !(res->flags & IORESOURCE_PREFETCH))
> > - return NULL;
> > + !(res->flags & IORESOURCE_PREFETCH)) {
> > + *first = NULL;
> > + return;
> > + }
> >
> > /*
> > * If we're below a transparent bridge, there may
> > @@ -729,10 +736,47 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> > * on pci_bus_for_each_resource() giving us those
> > * first.
> > */
> > - return r;
> > + *first = r;
> > + return;
> > }
> > }
> > - return NULL;
> > +
> > + if (!second)
> > + return;
> > +
> > + pci_bus_for_each_resource(bus, r, i) {
> > + if (!r)
> > + continue;
> > + if (resource_overlaps(r, res)) {
> > + if (r->flags & IORESOURCE_PREFETCH &&
> > + !(res->flags & IORESOURCE_PREFETCH))
> > + continue;
> > +
> > + if (!overlaps++)
> > + *first = r;
> > + else {
> > + *second = r;
> > + break;
> > + }
> > + }
> > + }
> > +
> > + if (overlaps != 2)
> > + goto out;
> > +
> > + if ((*first)->start > (*second)->start)
> > + swap(*first, *second);
> > +
> > + if ((*first)->end + 1 != (*second)->start)
> > + goto out;
> > +
> > + if ((*first)->start <= res->start && (*second)->end >= res->end)
> > + return;
> > +out:
> > +
> > + *first = NULL;
> > + *second = NULL;
> > + return;
> > }
> > EXPORT_SYMBOL(pci_find_parent_resource);
> >
> > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> > index 7f1acb3918d0..e39615321d81 100644
> > --- a/drivers/pci/setup-res.c
> > +++ b/drivers/pci/setup-res.c
> > @@ -131,7 +131,7 @@ void pci_update_resource(struct pci_dev *dev, int resno)
> > int pci_claim_resource(struct pci_dev *dev, int resource)
> > {
> > struct resource *res = &dev->resource[resource];
> > - struct resource *root, *conflict;
> > + struct resource *first = NULL, *second = NULL, *conflict;
> >
> > if (res->flags & IORESOURCE_UNSET) {
> > pci_info(dev, "can't claim BAR %d %pR: no address assigned\n",
> > @@ -147,21 +147,28 @@ int pci_claim_resource(struct pci_dev *dev, int resource)
> > if (res->flags & IORESOURCE_ROM_SHADOW)
> > return 0;
> >
> > - root = pci_find_parent_resource(dev, res);
> > - if (!root) {
> > + pci_find_parent_resource(dev, res, &first, &second);
> > + if (!first) {
> > pci_info(dev, "can't claim BAR %d %pR: no compatible bridge window\n",
> > resource, res);
> > res->flags |= IORESOURCE_UNSET;
> > return -EINVAL;
> > }
> >
> > - conflict = request_resource_conflict(root, res);
> > + if (second)
> > + first->end = second->end;
> > +
> > + conflict = request_resource_conflict(first, res);
> > if (conflict) {
> > + if (second)
> > + first->end = second->start - 1;
> > +
> > pci_info(dev, "can't claim BAR %d %pR: address conflict with %s %pR\n",
> > resource, res, conflict->name, conflict);
> > res->flags |= IORESOURCE_UNSET;
> > return -EBUSY;
> > - }
> > + } else if (second)
> > + second->start = second->end = 0;
> >
> > return 0;
> > }
> > @@ -195,7 +202,7 @@ resource_size_t __weak pcibios_retrieve_fw_addr(struct pci_dev *dev, int idx)
> > static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev,
> > int resno, resource_size_t size)
> > {
> > - struct resource *root, *conflict;
> > + struct resource *root = NULL, *conflict;
> > resource_size_t fw_addr, start, end;
> >
> > fw_addr = pcibios_retrieve_fw_addr(dev, resno);
> > @@ -208,7 +215,7 @@ static int pci_revert_fw_address(struct resource *res, struct pci_dev *dev,
> > res->end = res->start + size - 1;
> > res->flags &= ~IORESOURCE_UNSET;
> >
> > - root = pci_find_parent_resource(dev, res);
> > + pci_find_parent_resource(dev, res, &root, NULL);
> > if (!root) {
> > if (res->flags & IORESOURCE_IO)
> > root = &ioport_resource;
> > diff --git a/drivers/pcmcia/rsrc_nonstatic.c b/drivers/pcmcia/rsrc_nonstatic.c
> > index 3b05760e69d6..2fba42d7486e 100644
> > --- a/drivers/pcmcia/rsrc_nonstatic.c
> > +++ b/drivers/pcmcia/rsrc_nonstatic.c
> > @@ -73,7 +73,7 @@ static struct resource *
> > claim_region(struct pcmcia_socket *s, resource_size_t base,
> > resource_size_t size, int type, char *name)
> > {
> > - struct resource *res, *parent;
> > + struct resource *res, *parent = NULL;
> >
> > parent = type & IORESOURCE_MEM ? &iomem_resource : &ioport_resource;
> > res = pcmcia_make_resource(base, size, type | IORESOURCE_BUSY, name);
> > @@ -81,7 +81,7 @@ claim_region(struct pcmcia_socket *s, resource_size_t base,
> > if (res) {
> > #ifdef CONFIG_PCI
> > if (s && s->cb_dev)
> > - parent = pci_find_parent_resource(s->cb_dev, res);
> > + pci_find_parent_resource(s->cb_dev, res, &parent, NULL);
> > #endif
> > if (!parent || request_resource(parent, res)) {
> > kfree(res);
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 86c799c97b77..dd1455be5247 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1049,8 +1049,10 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus);
> > unsigned int pci_scan_child_bus(struct pci_bus *bus);
> > void pci_bus_add_device(struct pci_dev *dev);
> > void pci_read_bridge_bases(struct pci_bus *child);
> > -struct resource *pci_find_parent_resource(const struct pci_dev *dev,
> > - struct resource *res);
> > +void pci_find_parent_resource(const struct pci_dev *dev,
> > + struct resource *res,
> > + struct resource **first,
> > + struct resource **second);
> > u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin);
> > int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge);
> > u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp);
> > --
> > 2.30.2
> >
^ permalink raw reply
* Re: [PATCH printk v2 2/5] printk: remove safe buffers
From: John Ogness @ 2021-03-31 7:59 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Alexey Kardashevskiy, Paul Mackerras,
Tiezhu Yang, Rafael Aquini, Aneesh Kumar K.V, Peter Zijlstra,
Yue Hu, Jordan Niethe, Kees Cook, Paul E. McKenney,
Alistair Popple, Guilherme G. Piccoli, Nicholas Piggin,
Steven Rostedt, Thomas Gleixner, kexec, linux-kernel,
Sergey Senozhatsky, Eric Biederman, Andrew Morton, linuxppc-dev
In-Reply-To: <20210330153512.1182-3-john.ogness@linutronix.de>
On 2021-03-30, John Ogness <john.ogness@linutronix.de> wrote:
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index e971c0a9ec9e..f090d6a1b39e 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1772,16 +1759,21 @@ static struct task_struct *console_owner;
> static bool console_waiter;
>
> /**
> - * console_lock_spinning_enable - mark beginning of code where another
> + * console_lock_spinning_enable_irqsave - mark beginning of code where another
> * thread might safely busy wait
> *
> * This basically converts console_lock into a spinlock. This marks
> * the section where the console_lock owner can not sleep, because
> * there may be a waiter spinning (like a spinlock). Also it must be
> * ready to hand over the lock at the end of the section.
> + *
> + * This disables interrupts because the hand over to a waiter must not be
> + * interrupted until the hand over is completed (@console_waiter is cleared).
> */
> -static void console_lock_spinning_enable(void)
> +static void console_lock_spinning_enable_irqsave(unsigned long *flags)
I missed the prototype change for the !CONFIG_PRINTK case, resulting in:
linux/kernel/printk/printk.c:2707:3: error: implicit declaration of function ‘console_lock_spinning_enable_irqsave’; did you mean ‘console_lock_spinning_enable’? [-Werror=implicit-function-declaration]
console_lock_spinning_enable_irqsave(&flags);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
console_lock_spinning_enable
Will be fixed for v3.
(I have now officially added !CONFIG_PRINTK to my CI tests.)
John Ogness
^ 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