* Re: [PATCH 1/4] powerpc/xive: Use XIVE_BAD_IRQ instead of zero to catch non configured IPIs
From: Cédric Le Goater @ 2020-03-10 16:07 UTC (permalink / raw)
To: Greg Kurz; +Cc: linuxppc-dev, stable, David Gibson
In-Reply-To: <20200310160916.37de59c2@bahia.home>
On 3/10/20 4:09 PM, Greg Kurz wrote:
> On Fri, 6 Mar 2020 16:01:40 +0100
> Cédric Le Goater <clg@kaod.org> wrote:
>
>> When a CPU is brought up, an IPI number is allocated and recorded
>> under the XIVE CPU structure. Invalid IPI numbers are tracked with
>> interrupt number 0x0.
>>
>> On the PowerNV platform, the interrupt number space starts at 0x10 and
>> this works fine. However, on the sPAPR platform, it is possible to
>> allocate the interrupt number 0x0 and this raises an issue when CPU 0
>> is unplugged. The XIVE spapr driver tracks allocated interrupt numbers
>> in a bitmask and it is not correctly updated when interrupt number 0x0
>> is freed. It stays allocated and it is then impossible to reallocate.
>>
>> Fix by using the XIVE_BAD_IRQ value instead of zero on both platforms.
>>
>> Reported-by: David Gibson <david@gibson.dropbear.id.au>
>> Fixes: eac1e731b59e ("powerpc/xive: guest exploitation of the XIVE interrupt controller")
>> Cc: stable@vger.kernel.org # v4.14+
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>
> This looks mostly good. I'm juste wondering about potential overlooks:
Yes. Thanks for doing so. There are some non-obvious use of interrupt
numbers under the hood.
One thing we should be adding is a comment on the different interrupt
number spaces. The HW interrupt numbers, the EISN numbers found on the
XIVE even queue and the Linux interrupt numbers are different spaces
and have different limits. XIVE_BAD_IRQ was introduced for the EISN
numbers and the patch is using this value for HW numbers. This is ok
because it's more a tag than a limit.
> $ git grep 'if.*hw_i' arch/powerpc/ | egrep -v 'xics|XIVE_BAD_IRQ'
>
>
> arch/powerpc/kvm/book3s_xive.h: if (out_hw_irq)
> arch/powerpc/kvm/book3s_xive.h: if (out_hw_irq)
> arch/powerpc/kvm/book3s_xive_template.c: else if (hw_irq && xd->flags & XIVE_IRQ_FLAG_EOI_FW)
> arch/powerpc/sysdev/xive/common.c: else if (hw_irq && xd->flags & XIVE_IRQ_FLAG_EOI_FW) {
>
> This hw_irq check in xive_do_source_eoi() for example is related to:
>
> /*
> * Note: We pass "0" to the hw_irq argument in order to
> * avoid calling into the backend EOI code which we don't
> * want to do in the case of a re-trigger. Backends typically
> * only do EOI for LSIs anyway.
> */
> xive_do_source_eoi(0, xd);
that's a hack to skip the following part of the code in case of EOI
being done through FW:
else if (hw_irq && xd->flags & XIVE_IRQ_FLAG_EOI_FW) {
/*
* The FW told us to call it. This happens for some
* interrupt sources that need additional HW whacking
* beyond the ESB manipulation. For example LPC interrupts
* on P9 DD1.0 needed a latch to be clared in the LPC bridge
* itself. The Firmware will take care of it.
*/
if (WARN_ON_ONCE(!xive_ops->eoi))
return;
xive_ops->eoi(hw_irq);
That was the case for PHB4 LSIs on P9 DD1 only. We could probably drop
the code in Linux unless similar bugs show up on new platforms.
> but it can get hw_irq from:
>
> xive_do_source_eoi(xc->hw_ipi, &xc->ipi_data);
That part is fine. It's an IPI EOI.
>
> It seems that these should use XIVE_BAD_IRQ as well or I'm missing
> something ?
>
> arch/powerpc/sysdev/xive/common.c: if (hw_irq)
This tests the XIVE IPI number which is mapped to 0 for all CPUs.
See xive_request_ipi() and xive_irq_domain_map()
C.
> arch/powerpc/sysdev/xive/common.c: if (d->domain != xive_irq_domain || hw_irq == 0)
>
>
>
>> arch/powerpc/sysdev/xive/xive-internal.h | 7 +++++++
>> arch/powerpc/sysdev/xive/common.c | 12 +++---------
>> arch/powerpc/sysdev/xive/native.c | 4 ++--
>> arch/powerpc/sysdev/xive/spapr.c | 4 ++--
>> 4 files changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/powerpc/sysdev/xive/xive-internal.h b/arch/powerpc/sysdev/xive/xive-internal.h
>> index 59cd366e7933..382980f4de2d 100644
>> --- a/arch/powerpc/sysdev/xive/xive-internal.h
>> +++ b/arch/powerpc/sysdev/xive/xive-internal.h
>> @@ -5,6 +5,13 @@
>> #ifndef __XIVE_INTERNAL_H
>> #define __XIVE_INTERNAL_H
>>
>> +/*
>> + * A "disabled" interrupt should never fire, to catch problems
>> + * we set its logical number to this
>> + */
>> +#define XIVE_BAD_IRQ 0x7fffffff
>> +#define XIVE_MAX_IRQ (XIVE_BAD_IRQ - 1)
>> +
>> /* Each CPU carry one of these with various per-CPU state */
>> struct xive_cpu {
>> #ifdef CONFIG_SMP
>> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
>> index fa49193206b6..550baba98ec9 100644
>> --- a/arch/powerpc/sysdev/xive/common.c
>> +++ b/arch/powerpc/sysdev/xive/common.c
>> @@ -68,13 +68,6 @@ static u32 xive_ipi_irq;
>> /* Xive state for each CPU */
>> static DEFINE_PER_CPU(struct xive_cpu *, xive_cpu);
>>
>> -/*
>> - * A "disabled" interrupt should never fire, to catch problems
>> - * we set its logical number to this
>> - */
>> -#define XIVE_BAD_IRQ 0x7fffffff
>> -#define XIVE_MAX_IRQ (XIVE_BAD_IRQ - 1)
>> -
>> /* An invalid CPU target */
>> #define XIVE_INVALID_TARGET (-1)
>>
>> @@ -1153,7 +1146,7 @@ static int xive_setup_cpu_ipi(unsigned int cpu)
>> xc = per_cpu(xive_cpu, cpu);
>>
>> /* Check if we are already setup */
>> - if (xc->hw_ipi != 0)
>> + if (xc->hw_ipi != XIVE_BAD_IRQ)
>> return 0;
>>
>> /* Grab an IPI from the backend, this will populate xc->hw_ipi */
>> @@ -1190,7 +1183,7 @@ static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
>> /* Disable the IPI and free the IRQ data */
>>
>> /* Already cleaned up ? */
>> - if (xc->hw_ipi == 0)
>> + if (xc->hw_ipi == XIVE_BAD_IRQ)
>> return;
>>
>> /* Mask the IPI */
>> @@ -1346,6 +1339,7 @@ static int xive_prepare_cpu(unsigned int cpu)
>> if (np)
>> xc->chip_id = of_get_ibm_chip_id(np);
>> of_node_put(np);
>> + xc->hw_ipi = XIVE_BAD_IRQ;
>>
>> per_cpu(xive_cpu, cpu) = xc;
>> }
>> diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
>> index 0ff6b739052c..50e1a8e02497 100644
>> --- a/arch/powerpc/sysdev/xive/native.c
>> +++ b/arch/powerpc/sysdev/xive/native.c
>> @@ -312,7 +312,7 @@ static void xive_native_put_ipi(unsigned int cpu, struct xive_cpu *xc)
>> s64 rc;
>>
>> /* Free the IPI */
>> - if (!xc->hw_ipi)
>> + if (xc->hw_ipi == XIVE_BAD_IRQ)
>> return;
>> for (;;) {
>> rc = opal_xive_free_irq(xc->hw_ipi);
>> @@ -320,7 +320,7 @@ static void xive_native_put_ipi(unsigned int cpu, struct xive_cpu *xc)
>> msleep(OPAL_BUSY_DELAY_MS);
>> continue;
>> }
>> - xc->hw_ipi = 0;
>> + xc->hw_ipi = XIVE_BAD_IRQ;
>> break;
>> }
>> }
>> diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
>> index 55dc61cb4867..3f15615712b5 100644
>> --- a/arch/powerpc/sysdev/xive/spapr.c
>> +++ b/arch/powerpc/sysdev/xive/spapr.c
>> @@ -560,11 +560,11 @@ static int xive_spapr_get_ipi(unsigned int cpu, struct xive_cpu *xc)
>>
>> static void xive_spapr_put_ipi(unsigned int cpu, struct xive_cpu *xc)
>> {
>> - if (!xc->hw_ipi)
>> + if (xc->hw_ipi == XIVE_BAD_IRQ)
>> return;
>>
>> xive_irq_bitmap_free(xc->hw_ipi);
>> - xc->hw_ipi = 0;
>> + xc->hw_ipi = XIVE_BAD_IRQ;
>> }
>> #endif /* CONFIG_SMP */
>>
>
^ permalink raw reply
* [PATCH v3] powerpc/32s: reorder Linux PTE bits to better match Hash PTE bits.
From: Christophe Leroy @ 2020-03-10 17:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Reorder Linux PTE bits to (almost) match Hash PTE bits.
RW Kernel : PP = 00
RO Kernel : PP = 00
RW User : PP = 01
RO User : PP = 11
So naturally, we should have
_PAGE_USER = 0x001
_PAGE_RW = 0x002
Today 0x001 and 0x002 and _PAGE_PRESENT and _PAGE_HASHPTE which
both are software only bits.
Switch _PAGE_USER and _PAGE_PRESET
Switch _PAGE_RW and _PAGE_HASHPTE
This allows to remove a few insns.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v3: rebased on today's powerpc/merge
v2: rebased on today's powerpc/merge
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/book3s/32/hash.h | 8 ++++----
arch/powerpc/kernel/head_32.S | 9 +++------
arch/powerpc/mm/book3s32/hash_low.S | 14 ++++++--------
3 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/hash.h b/arch/powerpc/include/asm/book3s/32/hash.h
index 2a0a467d2985..34a7215ae81e 100644
--- a/arch/powerpc/include/asm/book3s/32/hash.h
+++ b/arch/powerpc/include/asm/book3s/32/hash.h
@@ -17,9 +17,9 @@
* updating the accessed and modified bits in the page table tree.
*/
-#define _PAGE_PRESENT 0x001 /* software: pte contains a translation */
-#define _PAGE_HASHPTE 0x002 /* hash_page has made an HPTE for this pte */
-#define _PAGE_USER 0x004 /* usermode access allowed */
+#define _PAGE_USER 0x001 /* usermode access allowed */
+#define _PAGE_RW 0x002 /* software: user write access allowed */
+#define _PAGE_PRESENT 0x004 /* software: pte contains a translation */
#define _PAGE_GUARDED 0x008 /* G: prohibit speculative access */
#define _PAGE_COHERENT 0x010 /* M: enforce memory coherence (SMP systems) */
#define _PAGE_NO_CACHE 0x020 /* I: cache inhibit */
@@ -27,7 +27,7 @@
#define _PAGE_DIRTY 0x080 /* C: page changed */
#define _PAGE_ACCESSED 0x100 /* R: page referenced */
#define _PAGE_EXEC 0x200 /* software: exec allowed */
-#define _PAGE_RW 0x400 /* software: user write access allowed */
+#define _PAGE_HASHPTE 0x400 /* hash_page has made an HPTE for this pte */
#define _PAGE_SPECIAL 0x800 /* software: Special page */
#ifdef CONFIG_PTE_64BIT
diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index 97c887950c3c..daaa153950c2 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -348,7 +348,7 @@ BEGIN_MMU_FTR_SECTION
andis. r0, r5, (DSISR_BAD_FAULT_32S | DSISR_DABRMATCH)@h
#endif
bne handle_page_fault_tramp_2 /* if not, try to put a PTE */
- rlwinm r3, r5, 32 - 15, 21, 21 /* DSISR_STORE -> _PAGE_RW */
+ rlwinm r3, r5, 32 - 24, 30, 30 /* DSISR_STORE -> _PAGE_RW */
bl hash_page
b handle_page_fault_tramp_1
FTR_SECTION_ELSE
@@ -497,7 +497,6 @@ InstructionTLBMiss:
andc. r1,r1,r0 /* check access & ~permission */
bne- InstructionAddressInvalid /* return if access not permitted */
/* Convert linux-style PTE to low word of PPC-style PTE */
- rlwimi r0,r0,32-2,31,31 /* _PAGE_USER -> PP lsb */
ori r1, r1, 0xe06 /* clear out reserved bits */
andc r1, r0, r1 /* PP = user? 1 : 0 */
BEGIN_FTR_SECTION
@@ -565,9 +564,8 @@ DataLoadTLBMiss:
* we would need to update the pte atomically with lwarx/stwcx.
*/
/* Convert linux-style PTE to low word of PPC-style PTE */
- rlwinm r1,r0,32-9,30,30 /* _PAGE_RW -> PP msb */
- rlwimi r0,r0,32-1,30,30 /* _PAGE_USER -> PP msb */
- rlwimi r0,r0,32-1,31,31 /* _PAGE_USER -> PP lsb */
+ rlwinm r1,r0,0,30,30 /* _PAGE_RW -> PP msb */
+ rlwimi r0,r0,1,30,30 /* _PAGE_USER -> PP msb */
ori r1,r1,0xe04 /* clear out reserved bits */
andc r1,r0,r1 /* PP = user? rw? 1: 3: 0 */
BEGIN_FTR_SECTION
@@ -645,7 +643,6 @@ DataStoreTLBMiss:
* we would need to update the pte atomically with lwarx/stwcx.
*/
/* Convert linux-style PTE to low word of PPC-style PTE */
- rlwimi r0,r0,32-2,31,31 /* _PAGE_USER -> PP lsb */
li r1,0xe06 /* clear out reserved bits & PP msb */
andc r1,r0,r1 /* PP = user? 1: 0 */
BEGIN_FTR_SECTION
diff --git a/arch/powerpc/mm/book3s32/hash_low.S b/arch/powerpc/mm/book3s32/hash_low.S
index 877d880890fe..6d236080cb1a 100644
--- a/arch/powerpc/mm/book3s32/hash_low.S
+++ b/arch/powerpc/mm/book3s32/hash_low.S
@@ -35,7 +35,7 @@ mmu_hash_lock:
/*
* Load a PTE into the hash table, if possible.
* The address is in r4, and r3 contains an access flag:
- * _PAGE_RW (0x400) if a write.
+ * _PAGE_RW (0x002) if a write.
* r9 contains the SRR1 value, from which we use the MSR_PR bit.
* SPRG_THREAD contains the physical address of the current task's thread.
*
@@ -69,7 +69,7 @@ _GLOBAL(hash_page)
blt+ 112f /* assume user more likely */
lis r5, (swapper_pg_dir - PAGE_OFFSET)@ha /* if kernel address, use */
addi r5 ,r5 ,(swapper_pg_dir - PAGE_OFFSET)@l /* kernel page table */
- rlwimi r3,r9,32-12,29,29 /* MSR_PR -> _PAGE_USER */
+ rlwimi r3,r9,32-14,31,31 /* MSR_PR -> _PAGE_USER */
112:
#ifndef CONFIG_PTE_64BIT
rlwimi r5,r4,12,20,29 /* insert top 10 bits of address */
@@ -94,7 +94,7 @@ _GLOBAL(hash_page)
#else
rlwimi r8,r4,23,20,28 /* compute pte address */
#endif
- rlwinm r0,r3,32-3,24,24 /* _PAGE_RW access -> _PAGE_DIRTY */
+ rlwinm r0,r3,6,24,24 /* _PAGE_RW access -> _PAGE_DIRTY */
ori r0,r0,_PAGE_ACCESSED|_PAGE_HASHPTE
/*
@@ -310,11 +310,9 @@ Hash_msk = (((1 << Hash_bits) - 1) * 64)
_GLOBAL(create_hpte)
/* Convert linux-style PTE (r5) to low word of PPC-style PTE (r8) */
- rlwinm r8,r5,32-9,30,30 /* _PAGE_RW -> PP msb */
rlwinm r0,r5,32-6,30,30 /* _PAGE_DIRTY -> PP msb */
- and r8,r8,r0 /* writable if _RW & _DIRTY */
- rlwimi r5,r5,32-1,30,30 /* _PAGE_USER -> PP msb */
- rlwimi r5,r5,32-2,31,31 /* _PAGE_USER -> PP lsb */
+ and r8,r5,r0 /* writable if _RW & _DIRTY */
+ rlwimi r5,r5,1,30,30 /* _PAGE_USER -> PP msb */
ori r8,r8,0xe04 /* clear out reserved bits */
andc r8,r5,r8 /* PP = user? (rw&dirty? 1: 3): 0 */
BEGIN_FTR_SECTION
@@ -566,7 +564,7 @@ _GLOBAL(flush_hash_pages)
33: lwarx r8,0,r5 /* fetch the pte flags word */
andi. r0,r8,_PAGE_HASHPTE
beq 8f /* done if HASHPTE is already clear */
- rlwinm r8,r8,0,31,29 /* clear HASHPTE bit */
+ rlwinm r8,r8,0,~_PAGE_HASHPTE /* clear HASHPTE bit */
stwcx. r8,0,r5 /* update the pte */
bne- 33b
--
2.25.0
^ permalink raw reply related
* Re: [PATCH] powerpc/pseries: fix of_read_drc_info_cell() to point at next record
From: Nathan Lynch @ 2020-03-10 17:25 UTC (permalink / raw)
To: Tyrel Datwyler; +Cc: mwb, msuchanek, linuxppc-dev, Tyrel Datwyler
In-Reply-To: <20200307024547.5748-1-tyreld@linux.ibm.com>
Tyrel Datwyler <tyreld@linux.ibm.com> writes:
> The expectation is that when calling of_read_drc_info_cell()
> repeatedly to parse multiple drc-info records that the in/out curval
> parameter points at the start of the next record on return. However,
> the current behavior has curval still pointing at the final value of
> the record just parsed. The result of which is that if the
> ibm,drc-info property contains multiple properties the parsed value
> of the drc_type for any record after the first has the power_domain
> value of the previous record appended to the type string.
>
> Ex: observed the following 0xffffffff prepended to PHB
>
> [ 69.485037] drc-info: type: \xff\xff\xff\xffPHB, prefix: PHB , index_start: 0x20000001
> [ 69.485038] drc-info: suffix_start: 1, sequential_elems: 3072, sequential_inc: 1
> [ 69.485038] drc-info: power-domain: 0xffffffff, last_index: 0x20000c00
>
> Fix by incrementing curval past the power_domain value to point at
> drc_type string of next record.
>
> Fixes: a29396653b8bf ("pseries/drc-info: Search DRC properties for CPU indexes")
I have a different commit hash for that:
e83636ac3334 pseries/drc-info: Search DRC properties for CPU indexes
> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Otherwise:
Acked-by: Nathan Lynch <nathanl@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH 2/4] powerpc/xive: Fix xmon support on the PowerNV platform
From: Greg Kurz @ 2020-03-10 15:38 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev, stable
In-Reply-To: <20200306150143.5551-3-clg@kaod.org>
On Fri, 6 Mar 2020 16:01:41 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> The PowerNV platform has multiple IRQ chips and the xmon command
> dumping the state of the XIVE interrupt should only operate on the
> XIVE IRQ chip.
>
> Fixes: 5896163f7f91 ("powerpc/xmon: Improve output of XIVE interrupts")
> Cc: stable@vger.kernel.org # v5.4+
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> arch/powerpc/sysdev/xive/common.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index 550baba98ec9..8155adc2225a 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -261,11 +261,15 @@ notrace void xmon_xive_do_dump(int cpu)
>
> int xmon_xive_get_irq_config(u32 hw_irq, struct irq_data *d)
> {
> + struct irq_chip *chip = irq_data_get_irq_chip(d);
> int rc;
> u32 target;
> u8 prio;
> u32 lirq;
>
> + if (!is_xive_irq(chip))
> + return -EINVAL;
> +
> rc = xive_ops->get_irq_config(hw_irq, &target, &prio, &lirq);
> if (rc) {
> xmon_printf("IRQ 0x%08x : no config rc=%d\n", hw_irq, rc);
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/xmon: Add source flags to output of XIVE interrupts
From: Greg Kurz @ 2020-03-10 15:43 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev
In-Reply-To: <20200306150143.5551-4-clg@kaod.org>
On Fri, 6 Mar 2020 16:01:42 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> Some firmwares or hypervisors can advertise different source
> characteristics. Track their value under XMON. What we are mostly
> interested in is the StoreEOI flag.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> arch/powerpc/sysdev/xive/common.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index 8155adc2225a..c865ae554605 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -283,7 +283,10 @@ int xmon_xive_get_irq_config(u32 hw_irq, struct irq_data *d)
> struct xive_irq_data *xd = irq_data_get_irq_handler_data(d);
> u64 val = xive_esb_read(xd, XIVE_ESB_GET);
>
> - xmon_printf("PQ=%c%c",
> + xmon_printf("flags=%c%c%c PQ=%c%c",
> + xd->flags & XIVE_IRQ_FLAG_STORE_EOI ? 'S' : ' ',
> + xd->flags & XIVE_IRQ_FLAG_LSI ? 'L' : ' ',
> + xd->flags & XIVE_IRQ_FLAG_H_INT_ESB ? 'H' : ' ',
> val & XIVE_ESB_VAL_P ? 'P' : '-',
> val & XIVE_ESB_VAL_Q ? 'Q' : '-');
> }
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/xive: Use XIVE_BAD_IRQ instead of zero to catch non configured IPIs
From: Greg Kurz @ 2020-03-10 15:09 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev, stable, David Gibson
In-Reply-To: <20200306150143.5551-2-clg@kaod.org>
On Fri, 6 Mar 2020 16:01:40 +0100
Cédric Le Goater <clg@kaod.org> wrote:
> When a CPU is brought up, an IPI number is allocated and recorded
> under the XIVE CPU structure. Invalid IPI numbers are tracked with
> interrupt number 0x0.
>
> On the PowerNV platform, the interrupt number space starts at 0x10 and
> this works fine. However, on the sPAPR platform, it is possible to
> allocate the interrupt number 0x0 and this raises an issue when CPU 0
> is unplugged. The XIVE spapr driver tracks allocated interrupt numbers
> in a bitmask and it is not correctly updated when interrupt number 0x0
> is freed. It stays allocated and it is then impossible to reallocate.
>
> Fix by using the XIVE_BAD_IRQ value instead of zero on both platforms.
>
> Reported-by: David Gibson <david@gibson.dropbear.id.au>
> Fixes: eac1e731b59e ("powerpc/xive: guest exploitation of the XIVE interrupt controller")
> Cc: stable@vger.kernel.org # v4.14+
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
This looks mostly good. I'm juste wondering about potential overlooks:
$ git grep 'if.*hw_i' arch/powerpc/ | egrep -v 'xics|XIVE_BAD_IRQ'
arch/powerpc/kvm/book3s_xive.h: if (out_hw_irq)
arch/powerpc/kvm/book3s_xive.h: if (out_hw_irq)
arch/powerpc/kvm/book3s_xive_template.c: else if (hw_irq && xd->flags & XIVE_IRQ_FLAG_EOI_FW)
arch/powerpc/sysdev/xive/common.c: else if (hw_irq && xd->flags & XIVE_IRQ_FLAG_EOI_FW) {
This hw_irq check in xive_do_source_eoi() for example is related to:
/*
* Note: We pass "0" to the hw_irq argument in order to
* avoid calling into the backend EOI code which we don't
* want to do in the case of a re-trigger. Backends typically
* only do EOI for LSIs anyway.
*/
xive_do_source_eoi(0, xd);
but it can get hw_irq from:
xive_do_source_eoi(xc->hw_ipi, &xc->ipi_data);
It seems that these should use XIVE_BAD_IRQ as well or I'm missing
something ?
arch/powerpc/sysdev/xive/common.c: if (hw_irq)
arch/powerpc/sysdev/xive/common.c: if (d->domain != xive_irq_domain || hw_irq == 0)
> arch/powerpc/sysdev/xive/xive-internal.h | 7 +++++++
> arch/powerpc/sysdev/xive/common.c | 12 +++---------
> arch/powerpc/sysdev/xive/native.c | 4 ++--
> arch/powerpc/sysdev/xive/spapr.c | 4 ++--
> 4 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/xive/xive-internal.h b/arch/powerpc/sysdev/xive/xive-internal.h
> index 59cd366e7933..382980f4de2d 100644
> --- a/arch/powerpc/sysdev/xive/xive-internal.h
> +++ b/arch/powerpc/sysdev/xive/xive-internal.h
> @@ -5,6 +5,13 @@
> #ifndef __XIVE_INTERNAL_H
> #define __XIVE_INTERNAL_H
>
> +/*
> + * A "disabled" interrupt should never fire, to catch problems
> + * we set its logical number to this
> + */
> +#define XIVE_BAD_IRQ 0x7fffffff
> +#define XIVE_MAX_IRQ (XIVE_BAD_IRQ - 1)
> +
> /* Each CPU carry one of these with various per-CPU state */
> struct xive_cpu {
> #ifdef CONFIG_SMP
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index fa49193206b6..550baba98ec9 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -68,13 +68,6 @@ static u32 xive_ipi_irq;
> /* Xive state for each CPU */
> static DEFINE_PER_CPU(struct xive_cpu *, xive_cpu);
>
> -/*
> - * A "disabled" interrupt should never fire, to catch problems
> - * we set its logical number to this
> - */
> -#define XIVE_BAD_IRQ 0x7fffffff
> -#define XIVE_MAX_IRQ (XIVE_BAD_IRQ - 1)
> -
> /* An invalid CPU target */
> #define XIVE_INVALID_TARGET (-1)
>
> @@ -1153,7 +1146,7 @@ static int xive_setup_cpu_ipi(unsigned int cpu)
> xc = per_cpu(xive_cpu, cpu);
>
> /* Check if we are already setup */
> - if (xc->hw_ipi != 0)
> + if (xc->hw_ipi != XIVE_BAD_IRQ)
> return 0;
>
> /* Grab an IPI from the backend, this will populate xc->hw_ipi */
> @@ -1190,7 +1183,7 @@ static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
> /* Disable the IPI and free the IRQ data */
>
> /* Already cleaned up ? */
> - if (xc->hw_ipi == 0)
> + if (xc->hw_ipi == XIVE_BAD_IRQ)
> return;
>
> /* Mask the IPI */
> @@ -1346,6 +1339,7 @@ static int xive_prepare_cpu(unsigned int cpu)
> if (np)
> xc->chip_id = of_get_ibm_chip_id(np);
> of_node_put(np);
> + xc->hw_ipi = XIVE_BAD_IRQ;
>
> per_cpu(xive_cpu, cpu) = xc;
> }
> diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
> index 0ff6b739052c..50e1a8e02497 100644
> --- a/arch/powerpc/sysdev/xive/native.c
> +++ b/arch/powerpc/sysdev/xive/native.c
> @@ -312,7 +312,7 @@ static void xive_native_put_ipi(unsigned int cpu, struct xive_cpu *xc)
> s64 rc;
>
> /* Free the IPI */
> - if (!xc->hw_ipi)
> + if (xc->hw_ipi == XIVE_BAD_IRQ)
> return;
> for (;;) {
> rc = opal_xive_free_irq(xc->hw_ipi);
> @@ -320,7 +320,7 @@ static void xive_native_put_ipi(unsigned int cpu, struct xive_cpu *xc)
> msleep(OPAL_BUSY_DELAY_MS);
> continue;
> }
> - xc->hw_ipi = 0;
> + xc->hw_ipi = XIVE_BAD_IRQ;
> break;
> }
> }
> diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
> index 55dc61cb4867..3f15615712b5 100644
> --- a/arch/powerpc/sysdev/xive/spapr.c
> +++ b/arch/powerpc/sysdev/xive/spapr.c
> @@ -560,11 +560,11 @@ static int xive_spapr_get_ipi(unsigned int cpu, struct xive_cpu *xc)
>
> static void xive_spapr_put_ipi(unsigned int cpu, struct xive_cpu *xc)
> {
> - if (!xc->hw_ipi)
> + if (xc->hw_ipi == XIVE_BAD_IRQ)
> return;
>
> xive_irq_bitmap_free(xc->hw_ipi);
> - xc->hw_ipi = 0;
> + xc->hw_ipi = XIVE_BAD_IRQ;
> }
> #endif /* CONFIG_SMP */
>
^ permalink raw reply
* Re: [5.6.0-rc2-next-20200218/powerpc] Boot failure on POWER9
From: Michal Hocko @ 2020-03-10 15:01 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: Sachin Sant, Pekka Enberg, Linux-Next Mailing List,
Vlastimil Babka, David Rientjes, Christopher Lameter,
linuxppc-dev, Joonsoo Kim, Kirill Tkhai
In-Reply-To: <20200227182650.GG3771@dhcp22.suse.cz>
On Thu 27-02-20 19:26:54, Michal Hocko wrote:
> [Cc ppc maintainers]
[...]
> Please have a look at http://lkml.kernel.org/r/52EF4673-7292-4C4C-B459-AF583951BA48@linux.vnet.ibm.com
> for the boot log with the debugging patch which tracks set_numa_mem.
> This seems to lead to a crash in the slab allocator bebcause
> node_to_mem_node(0) for memory less node resolves to the memory less
> node http://lkml.kernel.org/r/dd450314-d428-6776-af07-f92c04c7b967@suse.cz.
> The original report is http://lkml.kernel.org/r/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com
ping
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [PATCH net v2] ibmvnic: Do not process device remove during device reset
From: Juliet Kim @ 2020-03-10 14:23 UTC (permalink / raw)
To: netdev; +Cc: julietk, tlfalcon, linuxppc-dev
The ibmvnic driver does not check the device state when the device
is removed. If the device is removed while a device reset is being
processed, the remove may free structures needed by the reset,
causing an oops.
Fix this by checking the device state before processing device remove.
Signed-off-by: Juliet Kim <julietk@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 24 ++++++++++++++++++++++--
drivers/net/ethernet/ibm/ibmvnic.h | 6 +++++-
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index c75239d8820f..4bd33245bad6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2142,6 +2142,8 @@ static void __ibmvnic_reset(struct work_struct *work)
{
struct ibmvnic_rwi *rwi;
struct ibmvnic_adapter *adapter;
+ bool saved_state = false;
+ unsigned long flags;
u32 reset_state;
int rc = 0;
@@ -2153,17 +2155,25 @@ static void __ibmvnic_reset(struct work_struct *work)
return;
}
- reset_state = adapter->state;
-
rwi = get_next_rwi(adapter);
while (rwi) {
+ spin_lock_irqsave(&adapter->state_lock, flags);
+
if (adapter->state == VNIC_REMOVING ||
adapter->state == VNIC_REMOVED) {
+ spin_unlock_irqrestore(&adapter->state_lock, flags);
kfree(rwi);
rc = EBUSY;
break;
}
+ if (!saved_state) {
+ reset_state = adapter->state;
+ adapter->state = VNIC_RESETTING;
+ saved_state = true;
+ }
+ spin_unlock_irqrestore(&adapter->state_lock, flags);
+
if (rwi->reset_reason == VNIC_RESET_CHANGE_PARAM) {
/* CHANGE_PARAM requestor holds rtnl_lock */
rc = do_change_param_reset(adapter, rwi, reset_state);
@@ -5091,6 +5101,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
__ibmvnic_delayed_reset);
INIT_LIST_HEAD(&adapter->rwi_list);
spin_lock_init(&adapter->rwi_lock);
+ spin_lock_init(&adapter->state_lock);
mutex_init(&adapter->fw_lock);
init_completion(&adapter->init_done);
init_completion(&adapter->fw_done);
@@ -5163,8 +5174,17 @@ static int ibmvnic_remove(struct vio_dev *dev)
{
struct net_device *netdev = dev_get_drvdata(&dev->dev);
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+ unsigned long flags;
+
+ spin_lock_irqsave(&adapter->state_lock, flags);
+ if (adapter->state == VNIC_RESETTING) {
+ spin_unlock_irqrestore(&adapter->state_lock, flags);
+ return -EBUSY;
+ }
adapter->state = VNIC_REMOVING;
+ spin_unlock_irqrestore(&adapter->state_lock, flags);
+
rtnl_lock();
unregister_netdevice(netdev);
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 60eccaf91b12..f8416e1d4cf0 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -941,7 +941,8 @@ enum vnic_state {VNIC_PROBING = 1,
VNIC_CLOSING,
VNIC_CLOSED,
VNIC_REMOVING,
- VNIC_REMOVED};
+ VNIC_REMOVED,
+ VNIC_RESETTING};
enum ibmvnic_reset_reason {VNIC_RESET_FAILOVER = 1,
VNIC_RESET_MOBILITY,
@@ -1090,4 +1091,7 @@ struct ibmvnic_adapter {
struct ibmvnic_tunables desired;
struct ibmvnic_tunables fallback;
+
+ /* Used for serializatin of state field */
+ spinlock_t state_lock;
};
--
2.25.0
^ permalink raw reply related
* [Bug 206669] Little-endian kernel crashing on POWER8 on heavy big-endian PowerKVM load
From: bugzilla-daemon @ 2020-03-10 12:28 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206669-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206669
--- Comment #10 from John Paul Adrian Glaubitz (glaubitz@physik.fu-berlin.de) ---
(In reply to Aneesh Kumar KV from comment #9)
> Also, can you try disabling THP. echo "never" >
> /sys/kernel/mm/transparent_hugepage/enabled
Yes. Just disabled.
FWIW, the machine just crashed some minutes ago but I didn't have a serial
console open to capture the trace.
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 206669] Little-endian kernel crashing on POWER8 on heavy big-endian PowerKVM load
From: bugzilla-daemon @ 2020-03-10 12:25 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206669-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206669
Aneesh Kumar KV (aneesh.kumar@linux.ibm.com) changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |aneesh.kumar@linux.ibm.com
--- Comment #9 from Aneesh Kumar KV (aneesh.kumar@linux.ibm.com) ---
Also, can you try disabling THP. echo "never" >
/sys/kernel/mm/transparent_hugepage/enabled
-aneesh
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v2] powerpc/Makefile: Mark phony targets as PHONY
From: Michael Ellerman @ 2020-03-10 12:00 UTC (permalink / raw)
To: Masahiro Yamada, Michael Ellerman; +Cc: linuxppc-dev, Linux Kbuild mailing list
In-Reply-To: <CAK7LNAQfREwzk4NwE5PzxOXGLcsk2BtjYNKvKx8g4_vxHmNHdw@mail.gmail.com>
Masahiro Yamada <masahiroy@kernel.org> writes:
> On Fri, Mar 6, 2020 at 9:27 AM Michael Ellerman
> <patch-notifications@ellerman.id.au> wrote:
>>
>> On Wed, 2020-02-19 at 00:04:34 UTC, Michael Ellerman wrote:
>> > Some of our phony targets are not marked as such. This can lead to
>> > confusing errors, eg:
>> >
>> > $ make clean
>> > $ touch install
>> > $ make install
>> > make: 'install' is up to date.
>> > $
>> >
>> > Fix it by adding them to the PHONY variable which is marked phony in
>> > the top-level Makefile, or in scripts/Makefile.build for the boot
>> > Makefile.
>> >
>> > Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
>> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>>
>> Applied to powerpc next.
>>
>> https://git.kernel.org/powerpc/c/d42c6d0f8d004c3661dde3c376ed637e9f292c22
>>
>
> You do not have to double your Signed-off-by.
Oops :/
My scripts don't cope with applying my own patches very well. Will try
to fix it.
cheers
^ permalink raw reply
* Re: [PATCH v3] powerpc: setup_64: set up PACA earlier to avoid kcov problems
From: Michael Ellerman @ 2020-03-10 11:41 UTC (permalink / raw)
To: Nicholas Piggin, Daniel Axtens, linuxppc-dev; +Cc: ajd
In-Reply-To: <1583543617.bp71axgtlo.astroid@bobo.none>
Nicholas Piggin <npiggin@gmail.com> writes:
> Daniel Axtens's on March 6, 2020 5:30 pm:
>> kcov instrumentation is collected the __sanitizer_cov_trace_pc hook in
>> kernel/kcov.c. The compiler inserts these hooks into every basic block
>> unless kcov is disabled for that file.
>>
>> We then have a deep call-chain:
>> - __sanitizer_cov_trace_pc calls to check_kcov_mode()
>> - check_kcov_mode() (kernel/kcov.c) calls in_task()
>> - in_task() (include/linux/preempt.h) calls preempt_count().
>> - preempt_count() (include/asm-generic/preempt.h) calls
>> current_thread_info()
>> - because powerpc has THREAD_INFO_IN_TASK, current_thread_info()
>> (include/linux/thread_info.h) is defined to 'current'
>> - current (arch/powerpc/include/asm/current.h) is defined to
>> get_current().
>> - get_current (same file) loads an offset of r13.
>> - arch/powerpc/include/asm/paca.h makes r13 a register variable
>> called local_paca - it is the PACA for the current CPU, so
>> this has the effect of loading the current task from PACA.
>> - get_current returns the current task from PACA,
>> - current_thread_info returns the task cast to a thread_info
>> - preempt_count dereferences the thread_info to load preempt_count
>> - that value is used by in_task and so on up the chain
>>
>> The problem is:
>>
>> - kcov instrumentation is enabled for arch/powerpc/kernel/dt_cpu_ftrs.c
>>
>> - even if it were not, dt_cpu_ftrs_init calls generic dt parsing code
>> which should definitely have instrumentation enabled.
>>
>> - setup_64.c calls dt_cpu_ftrs_init before it sets up a PACA.
>>
>> - If we don't set up a paca, r13 will contain unpredictable data.
>>
>> - In a zImage compiled with kcov and KASAN, we see r13 containing a value
>> that leads to dereferencing invalid memory (something like
>> 912a72603d420015).
>>
>> - Weirdly, the same kernel as a vmlinux loaded directly by qemu does not
>> crash. Investigating with gdb, it seems that in the vmlinux boot case,
>> r13 is near enough to zero that we just happen to be able to read that
>> part of memory (we're operating with translation off at this point) and
>> the current pointer also happens to land in readable memory and
>> everything just works.
>>
>> - PACA setup refers to CPU features - setup_paca() looks at
>> early_cpu_has_feature(CPU_FTR_HVMODE)
>>
>> There's no generic kill switch for kcov (as far as I can tell), and we
>> don't want to have to turn off instrumentation in the generic dt parsing
>> code (which lives outside arch/powerpc/) just because we don't have a real
>> paca or task yet.
>>
>> So:
>> - change the test when setting up a PACA to consider the actual value of
>> the MSR rather than the CPU feature.
>>
>> - move the PACA setup to before the cpu feature parsing.
>
> Hmm. Problem is that equally we want PACA to be sane before we call too
> far into the rest of the kernel ("generic dt parsing code").
But currently we call into that code with no paca at all. Or rather,
with r13 pointing somewhere random that will be interpreted as being a
paca.
This took a while for Daniel to debug because depending on how you boot
r13 contains a different junk value. That junk value may not point to
memory at all, or if it does the memory it points to may or may not send
you down the wrong path, depending on which exact bit you're looking at
in some random location.
So this is really not about kcov from my POV, that's just how we
discovered it.
cheers
^ permalink raw reply
* Re: [PATCH] macintosh: windfarm: fix MODINFO regression
From: Wolfram Sang @ 2020-03-10 11:22 UTC (permalink / raw)
To: linuxppc-dev
Cc: Erhard Furtner, Mathieu Malaterre, debian-powerpc, linux-i2c,
John Paul Adrian Glaubitz
In-Reply-To: <20200303125046.5157-1-wsa@the-dreams.de>
[-- Attachment #1: Type: text/plain, Size: 706 bytes --]
On Tue, Mar 03, 2020 at 01:50:46PM +0100, Wolfram Sang wrote:
> Commit af503716ac14 made sure OF devices get an OF style modalias with
> I2C events. It assumed all in-tree users were converted, yet it missed
> some Macintosh drivers.
>
> Add an OF module device table for all windfarm drivers to make them
> automatically load again.
>
> Fixes: af503716ac14 ("i2c: core: report OF style module alias for devices registered via OF")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=199471
> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> Tested-by: Erhard Furtner <erhard_f@mailbox.org>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Thanks, Michael! Applied to for-current.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: ppc32 panic on boot on linux-next
From: Laurentiu Tudor @ 2020-03-10 11:06 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <875zfcked5.fsf@mpe.ellerman.id.au>
Hello,
On 10.03.2020 06:44, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>> Le 07/03/2020 à 09:42, Christophe Leroy a écrit :
>>> Le 06/03/2020 à 20:05, Nick Desaulniers a écrit :
>>>> As a heads up, our CI went red last night, seems like a panic from
>>>> free_initmem? Is this a known issue?
>>>
>>> Thanks for the heads up.
>>>
>>> No such issue with either 8xx or book3s/32.
>>>
>>> I've now been able to reproduce it with bamboo QEMU.
>>>
>>> Reverting 2efc7c085f05 makes it disappear. I'll investigate.
>>>
>>
>> Ok, I found the problem. virt_to_kpte() lacks a NULL pmd check. I'll
>> send a patch for that.
>>
>> However, if there is no PMD I guess this area is mapped through some
>> kind of block mapping. Therefore it should bail out of the function through:
>>
>> if (v_block_mapped(address))
>> return 0;
>>
>>
>> Can someone who knows BOOKE investigate that ?
>
> Not sure we have anyone left?
>
> cheers
>
With latest linux-next, PPC32 Book-E boots to prompt:
https://paste.debian.net/1134272/
---
Best Regards, Laurentiu
^ permalink raw reply
* Re: [PATCH] macintosh: windfarm: fix MODINFO regression
From: Michael Ellerman @ 2020-03-10 11:05 UTC (permalink / raw)
To: Wolfram Sang, linuxppc-dev
Cc: Mathieu Malaterre, Erhard Furtner, debian-powerpc, linux-i2c,
John Paul Adrian Glaubitz
In-Reply-To: <20200310091155.GA1987@ninjato>
Wolfram Sang <wsa@the-dreams.de> writes:
> On Tue, Mar 03, 2020 at 01:50:46PM +0100, Wolfram Sang wrote:
>> Commit af503716ac14 made sure OF devices get an OF style modalias with
>> I2C events. It assumed all in-tree users were converted, yet it missed
>> some Macintosh drivers.
>>
>> Add an OF module device table for all windfarm drivers to make them
>> automatically load again.
>>
>> Fixes: af503716ac14 ("i2c: core: report OF style module alias for devices registered via OF")
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=199471
>> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
>> Tested-by: Erhard Furtner <erhard_f@mailbox.org>
>> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
>
> Michael, I can take this via I2C again, if you ack it.
Thanks, done.
cheers
^ permalink raw reply
* Re: [PATCH] macintosh: windfarm: fix MODINFO regression
From: Michael Ellerman @ 2020-03-10 11:04 UTC (permalink / raw)
To: Wolfram Sang, linuxppc-dev
Cc: Erhard Furtner, Wolfram Sang, Mathieu Malaterre, debian-powerpc,
linux-i2c, John Paul Adrian Glaubitz
In-Reply-To: <20200303125046.5157-1-wsa@the-dreams.de>
Wolfram Sang <wsa@the-dreams.de> writes:
> Commit af503716ac14 made sure OF devices get an OF style modalias with
> I2C events. It assumed all in-tree users were converted, yet it missed
> some Macintosh drivers.
>
> Add an OF module device table for all windfarm drivers to make them
> automatically load again.
>
> Fixes: af503716ac14 ("i2c: core: report OF style module alias for devices registered via OF")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=199471
> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> Tested-by: Erhard Furtner <erhard_f@mailbox.org>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> ---
>
> This should also help with this: https://lists.debian.org/debian-powerpc/2020/01/msg00062.html
> Some more testing would be appreciated because lm75 also has some code
> changes I can't test myself obviusly.
>
> By grepping, I found some more potential candidates (using a "MAC,"
> prefix but not defining a OF MODULE DEVICE TABLE). Does someone know
> about bugreports filed for those? I don't want to change them for no
> reason:
>
> drivers/macintosh/ams/ams-i2c.c
> drivers/macintosh/therm_adt746x.c
> sound/aoa/codecs/onyx.c
> sound/aoa/codecs/tas.c
> sound/ppc/keywest.c
>
> Happy hacking,
>
> Wolfram
>
> drivers/macintosh/windfarm_ad7417_sensor.c | 7 +++++++
> drivers/macintosh/windfarm_fcu_controls.c | 7 +++++++
> drivers/macintosh/windfarm_lm75_sensor.c | 16 +++++++++++++++-
> drivers/macintosh/windfarm_lm87_sensor.c | 7 +++++++
> drivers/macintosh/windfarm_max6690_sensor.c | 7 +++++++
> drivers/macintosh/windfarm_smu_sat.c | 7 +++++++
> 6 files changed, 50 insertions(+), 1 deletion(-)
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
^ permalink raw reply
* CDC ethernet gadget: complete system freeze
From: Joakim Tjernlund @ 2020-03-10 9:43 UTC (permalink / raw)
To: linuxppc-dev@ozlabs.org, linux-usb@vger.kernel.org
We have an embedded T1042 NXP CDC ethernet gadget which seems to completely freeze when an
usb0 I/F is established and one do 1 of two things:
1) reboot the connected Linux laptop -> CDC gadget appears to enter complete system freeze.
2) on laptop, ifconfig usb0 down; rmmod cdc_ether -> CDC gaget freezes
I cannot finns and OOPS or other trace in console or syslog. I could really use som ideas here.
Linux kernel 4.19.108 (4.19.76 has the same issue)
Jocke
^ permalink raw reply
* Re: [PATCH] i2c: powermac: correct comment about custom handling
From: Wolfram Sang @ 2020-03-10 9:19 UTC (permalink / raw)
To: linux-i2c; +Cc: linuxppc-dev
In-Reply-To: <20200225142613.7169-1-wsa@the-dreams.de>
[-- Attachment #1: Type: text/plain, Size: 320 bytes --]
On Tue, Feb 25, 2020 at 03:26:13PM +0100, Wolfram Sang wrote:
> The comment had some flaws which are now fixed:
> - the prefix is 'MAC' not 'AAPL'
> - no kernel coding style and too short length
> - 'we do' instead of 'we to'
>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Applied to for-next, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] macintosh: windfarm: fix MODINFO regression
From: Wolfram Sang @ 2020-03-10 9:11 UTC (permalink / raw)
To: linuxppc-dev
Cc: Erhard Furtner, Mathieu Malaterre, debian-powerpc, linux-i2c,
John Paul Adrian Glaubitz
In-Reply-To: <20200303125046.5157-1-wsa@the-dreams.de>
[-- Attachment #1: Type: text/plain, Size: 745 bytes --]
On Tue, Mar 03, 2020 at 01:50:46PM +0100, Wolfram Sang wrote:
> Commit af503716ac14 made sure OF devices get an OF style modalias with
> I2C events. It assumed all in-tree users were converted, yet it missed
> some Macintosh drivers.
>
> Add an OF module device table for all windfarm drivers to make them
> automatically load again.
>
> Fixes: af503716ac14 ("i2c: core: report OF style module alias for devices registered via OF")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=199471
> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> Tested-by: Erhard Furtner <erhard_f@mailbox.org>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Michael, I can take this via I2C again, if you ack it.
Thanks,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/xive: Use XIVE_BAD_IRQ instead of zero to catch non configured IPIs
From: David Gibson @ 2020-03-10 5:17 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev, Greg Kurz, stable
In-Reply-To: <20200306150143.5551-2-clg@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 5040 bytes --]
On Fri, Mar 06, 2020 at 04:01:40PM +0100, Cédric Le Goater wrote:
> When a CPU is brought up, an IPI number is allocated and recorded
> under the XIVE CPU structure. Invalid IPI numbers are tracked with
> interrupt number 0x0.
>
> On the PowerNV platform, the interrupt number space starts at 0x10 and
> this works fine. However, on the sPAPR platform, it is possible to
> allocate the interrupt number 0x0 and this raises an issue when CPU 0
> is unplugged. The XIVE spapr driver tracks allocated interrupt numbers
> in a bitmask and it is not correctly updated when interrupt number 0x0
> is freed. It stays allocated and it is then impossible to reallocate.
>
> Fix by using the XIVE_BAD_IRQ value instead of zero on both platforms.
>
> Reported-by: David Gibson <david@gibson.dropbear.id.au>
> Fixes: eac1e731b59e ("powerpc/xive: guest exploitation of the XIVE interrupt controller")
> Cc: stable@vger.kernel.org # v4.14+
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Tested-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> arch/powerpc/sysdev/xive/xive-internal.h | 7 +++++++
> arch/powerpc/sysdev/xive/common.c | 12 +++---------
> arch/powerpc/sysdev/xive/native.c | 4 ++--
> arch/powerpc/sysdev/xive/spapr.c | 4 ++--
> 4 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/xive/xive-internal.h b/arch/powerpc/sysdev/xive/xive-internal.h
> index 59cd366e7933..382980f4de2d 100644
> --- a/arch/powerpc/sysdev/xive/xive-internal.h
> +++ b/arch/powerpc/sysdev/xive/xive-internal.h
> @@ -5,6 +5,13 @@
> #ifndef __XIVE_INTERNAL_H
> #define __XIVE_INTERNAL_H
>
> +/*
> + * A "disabled" interrupt should never fire, to catch problems
> + * we set its logical number to this
> + */
> +#define XIVE_BAD_IRQ 0x7fffffff
> +#define XIVE_MAX_IRQ (XIVE_BAD_IRQ - 1)
> +
> /* Each CPU carry one of these with various per-CPU state */
> struct xive_cpu {
> #ifdef CONFIG_SMP
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index fa49193206b6..550baba98ec9 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -68,13 +68,6 @@ static u32 xive_ipi_irq;
> /* Xive state for each CPU */
> static DEFINE_PER_CPU(struct xive_cpu *, xive_cpu);
>
> -/*
> - * A "disabled" interrupt should never fire, to catch problems
> - * we set its logical number to this
> - */
> -#define XIVE_BAD_IRQ 0x7fffffff
> -#define XIVE_MAX_IRQ (XIVE_BAD_IRQ - 1)
> -
> /* An invalid CPU target */
> #define XIVE_INVALID_TARGET (-1)
>
> @@ -1153,7 +1146,7 @@ static int xive_setup_cpu_ipi(unsigned int cpu)
> xc = per_cpu(xive_cpu, cpu);
>
> /* Check if we are already setup */
> - if (xc->hw_ipi != 0)
> + if (xc->hw_ipi != XIVE_BAD_IRQ)
> return 0;
>
> /* Grab an IPI from the backend, this will populate xc->hw_ipi */
> @@ -1190,7 +1183,7 @@ static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
> /* Disable the IPI and free the IRQ data */
>
> /* Already cleaned up ? */
> - if (xc->hw_ipi == 0)
> + if (xc->hw_ipi == XIVE_BAD_IRQ)
> return;
>
> /* Mask the IPI */
> @@ -1346,6 +1339,7 @@ static int xive_prepare_cpu(unsigned int cpu)
> if (np)
> xc->chip_id = of_get_ibm_chip_id(np);
> of_node_put(np);
> + xc->hw_ipi = XIVE_BAD_IRQ;
>
> per_cpu(xive_cpu, cpu) = xc;
> }
> diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
> index 0ff6b739052c..50e1a8e02497 100644
> --- a/arch/powerpc/sysdev/xive/native.c
> +++ b/arch/powerpc/sysdev/xive/native.c
> @@ -312,7 +312,7 @@ static void xive_native_put_ipi(unsigned int cpu, struct xive_cpu *xc)
> s64 rc;
>
> /* Free the IPI */
> - if (!xc->hw_ipi)
> + if (xc->hw_ipi == XIVE_BAD_IRQ)
> return;
> for (;;) {
> rc = opal_xive_free_irq(xc->hw_ipi);
> @@ -320,7 +320,7 @@ static void xive_native_put_ipi(unsigned int cpu, struct xive_cpu *xc)
> msleep(OPAL_BUSY_DELAY_MS);
> continue;
> }
> - xc->hw_ipi = 0;
> + xc->hw_ipi = XIVE_BAD_IRQ;
> break;
> }
> }
> diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
> index 55dc61cb4867..3f15615712b5 100644
> --- a/arch/powerpc/sysdev/xive/spapr.c
> +++ b/arch/powerpc/sysdev/xive/spapr.c
> @@ -560,11 +560,11 @@ static int xive_spapr_get_ipi(unsigned int cpu, struct xive_cpu *xc)
>
> static void xive_spapr_put_ipi(unsigned int cpu, struct xive_cpu *xc)
> {
> - if (!xc->hw_ipi)
> + if (xc->hw_ipi == XIVE_BAD_IRQ)
> return;
>
> xive_irq_bitmap_free(xc->hw_ipi);
> - xc->hw_ipi = 0;
> + xc->hw_ipi = XIVE_BAD_IRQ;
> }
> #endif /* CONFIG_SMP */
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: ppc32 panic on boot on linux-next
From: Michael Ellerman @ 2020-03-10 4:44 UTC (permalink / raw)
To: Christophe Leroy, Nick Desaulniers; +Cc: clang-built-linux, linuxppc-dev
In-Reply-To: <c084a429-9ea1-7126-f057-8647a2e0dfac@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 07/03/2020 à 09:42, Christophe Leroy a écrit :
>> Le 06/03/2020 à 20:05, Nick Desaulniers a écrit :
>>> As a heads up, our CI went red last night, seems like a panic from
>>> free_initmem? Is this a known issue?
>>
>> Thanks for the heads up.
>>
>> No such issue with either 8xx or book3s/32.
>>
>> I've now been able to reproduce it with bamboo QEMU.
>>
>> Reverting 2efc7c085f05 makes it disappear. I'll investigate.
>>
>
> Ok, I found the problem. virt_to_kpte() lacks a NULL pmd check. I'll
> send a patch for that.
>
> However, if there is no PMD I guess this area is mapped through some
> kind of block mapping. Therefore it should bail out of the function through:
>
> if (v_block_mapped(address))
> return 0;
>
>
> Can someone who knows BOOKE investigate that ?
Not sure we have anyone left?
cheers
^ permalink raw reply
* Re: [PATCH V15] mm/debug: Add tests validating architecture page table helpers
From: Anshuman Khandual @ 2020-03-10 2:31 UTC (permalink / raw)
To: Christophe Leroy, Qian Cai
Cc: Heiko Carstens, linux-mm, Paul Mackerras, H. Peter Anvin,
linux-riscv, Will Deacon, linux-arch, linux-s390, x86,
Mike Rapoport, Christian Borntraeger, Ingo Molnar,
Catalin Marinas, linux-snps-arc, Vasily Gorbik, Borislav Petkov,
Paul Walmsley, Kirill A . Shutemov, Thomas Gleixner,
linux-arm-kernel, Vineet Gupta, linux-kernel, Palmer Dabbelt,
Andrew Morton, linuxppc-dev
In-Reply-To: <c40d907a-b64b-ae0d-e58f-33dddf0e8edc@c-s.fr>
On 03/07/2020 12:35 PM, Christophe Leroy wrote:
>
>
> Le 07/03/2020 à 01:56, Anshuman Khandual a écrit :
>>
>>
>> On 03/07/2020 06:04 AM, Qian Cai wrote:
>>>
>>>
>>>> On Mar 6, 2020, at 7:03 PM, Anshuman Khandual <Anshuman.Khandual@arm.com> wrote:
>>>>
>>>> Hmm, set_pte_at() function is not preferred here for these tests. The idea
>>>> is to avoid or atleast minimize TLB/cache flushes triggered from these sort
>>>> of 'static' tests. set_pte_at() is platform provided and could/might trigger
>>>> these flushes or some other platform specific synchronization stuff. Just
>>>
>>> Why is that important for this debugging option?
>>
>> Primarily reason is to avoid TLB/cache flush instructions on the system
>> during these tests that only involve transforming different page table
>> level entries through helpers. Unless really necessary, why should it
>> emit any TLB/cache flush instructions ?
>
> What's the problem with thoses flushes ?
>
>>
>>>
>>>> wondering is there specific reason with respect to the soft lock up problem
>>>> making it necessary to use set_pte_at() rather than a simple WRITE_ONCE() ?
>>>
>>> Looks at the s390 version of set_pte_at(), it has this comment,
>>> vmaddr);
>>>
>>> /*
>>> * Certain architectures need to do special things when PTEs
>>> * within a page table are directly modified. Thus, the following
>>> * hook is made available.
>>> */
>>>
>>> I can only guess that powerpc could be the same here.
>>
>> This comment is present in multiple platforms while defining set_pte_at().
>> Is not 'barrier()' here alone good enough ? Else what exactly set_pte_at()
>> does as compared to WRITE_ONCE() that avoids the soft lock up, just trying
>> to understand.
>>
>
>
> Argh ! I didn't realise that you were writing directly into the page tables. When it works, that's only by chance I guess.
>
> To properly set the page table entries, set_pte_at() has to be used:
> - On powerpc 8xx, with 16k pages, the page table entry must be copied four times. set_pte_at() does it, WRITE_ONCE() doesn't.
> - On powerpc book3s/32 (hash MMU), the flag _PAGE_HASHPTE must be preserved among writes. set_pte_at() preserves it, WRITE_ONCE() doesn't.
>
> set_pte_at() also does a few other mandatory things, like calling pte_mkpte()
>
> So, the WRITE_ONCE() must definitely become a set_pte_at()
Sure, will do. These are part of the clear tests that populates a given
entry with a non zero value before clearing and testing it with pxx_none().
In that context, WRITE_ONCE() seemed sufficient. But pte_clear() might be
closely tied with proper page table entry update and hence a preceding
set_pte_at() will be better.
There are still more WRITE_ONCE() for other page table levels during these
clear tests. set_pmd_at() and set_pud_at() are defined on platforms that
support (and enable) THP and PUD based THP respectively. Hence they could
not be used for clear tests as remaining helpers pmd_clear(), pud_clear(),
p4d_clear() and pgd_clear() still need to be validated with or without
THP support and enablement. We should just leave all other WRITE_ONCE()
instances unchanged. Please correct me if I am missing something here.
>
> Christophe
>
^ permalink raw reply
* Re: [PATCH net] ibmvnic: Do not process device remove during device reset
From: David Miller @ 2020-03-10 1:04 UTC (permalink / raw)
To: julietk; +Cc: netdev, linuxppc-dev, tlfalcon
In-Reply-To: <20200310000204.58596-1-julietk@linux.vnet.ibm.com>
From: Juliet Kim <julietk@linux.vnet.ibm.com>
Date: Mon, 9 Mar 2020 19:02:04 -0500
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index c75239d8820f..7ef1ae0d49bc 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -2144,6 +2144,8 @@ static void __ibmvnic_reset(struct work_struct *work)
> struct ibmvnic_adapter *adapter;
> u32 reset_state;
> int rc = 0;
> + unsigned long flags;
> + bool saved_state = false;
Please preserve the reverse christmas tree ordering of local variables.
Thank you.
^ permalink raw reply
* [PATCH v6 7/7] powerpc/32: use set_memory_attr()
From: Russell Currey @ 2020-03-10 1:03 UTC (permalink / raw)
To: linuxppc-dev; +Cc: ajd, kernel-hardening, npiggin, joel, dja
In-Reply-To: <20200310010338.21205-1-ruscur@russell.cc>
From: Christophe Leroy <christophe.leroy@c-s.fr>
Use set_memory_attr() instead of the PPC32 specific change_page_attr()
change_page_attr() was checking that the address was not mapped by
blocks and was handling highmem, but that's unneeded because the
affected pages can't be in highmem and block mapping verification
is already done by the callers.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/mm/pgtable_32.c | 95 ++++--------------------------------
1 file changed, 10 insertions(+), 85 deletions(-)
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 5fb90edd865e..3d92eaf3ee2f 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -23,6 +23,7 @@
#include <linux/highmem.h>
#include <linux/memblock.h>
#include <linux/slab.h>
+#include <linux/set_memory.h>
#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -121,99 +122,20 @@ void __init mapin_ram(void)
}
}
-/* Scan the real Linux page tables and return a PTE pointer for
- * a virtual address in a context.
- * Returns true (1) if PTE was found, zero otherwise. The pointer to
- * the PTE pointer is unmodified if PTE is not found.
- */
-static int
-get_pteptr(struct mm_struct *mm, unsigned long addr, pte_t **ptep, pmd_t **pmdp)
-{
- pgd_t *pgd;
- pud_t *pud;
- pmd_t *pmd;
- pte_t *pte;
- int retval = 0;
-
- pgd = pgd_offset(mm, addr & PAGE_MASK);
- if (pgd) {
- pud = pud_offset(pgd, addr & PAGE_MASK);
- if (pud && pud_present(*pud)) {
- pmd = pmd_offset(pud, addr & PAGE_MASK);
- if (pmd_present(*pmd)) {
- pte = pte_offset_map(pmd, addr & PAGE_MASK);
- if (pte) {
- retval = 1;
- *ptep = pte;
- if (pmdp)
- *pmdp = pmd;
- /* XXX caller needs to do pte_unmap, yuck */
- }
- }
- }
- }
- return(retval);
-}
-
-static int __change_page_attr_noflush(struct page *page, pgprot_t prot)
-{
- pte_t *kpte;
- pmd_t *kpmd;
- unsigned long address;
-
- BUG_ON(PageHighMem(page));
- address = (unsigned long)page_address(page);
-
- if (v_block_mapped(address))
- return 0;
- if (!get_pteptr(&init_mm, address, &kpte, &kpmd))
- return -EINVAL;
- __set_pte_at(&init_mm, address, kpte, mk_pte(page, prot), 0);
- pte_unmap(kpte);
-
- return 0;
-}
-
-/*
- * Change the page attributes of an page in the linear mapping.
- *
- * THIS DOES NOTHING WITH BAT MAPPINGS, DEBUG USE ONLY
- */
-static int change_page_attr(struct page *page, int numpages, pgprot_t prot)
-{
- int i, err = 0;
- unsigned long flags;
- struct page *start = page;
-
- local_irq_save(flags);
- for (i = 0; i < numpages; i++, page++) {
- err = __change_page_attr_noflush(page, prot);
- if (err)
- break;
- }
- wmb();
- local_irq_restore(flags);
- flush_tlb_kernel_range((unsigned long)page_address(start),
- (unsigned long)page_address(page));
- return err;
-}
-
void mark_initmem_nx(void)
{
- struct page *page = virt_to_page(_sinittext);
unsigned long numpages = PFN_UP((unsigned long)_einittext) -
PFN_DOWN((unsigned long)_sinittext);
if (v_block_mapped((unsigned long)_stext + 1))
mmu_mark_initmem_nx();
else
- change_page_attr(page, numpages, PAGE_KERNEL);
+ set_memory_attr((unsigned long)_sinittext, numpages, PAGE_KERNEL);
}
#ifdef CONFIG_STRICT_KERNEL_RWX
void mark_rodata_ro(void)
{
- struct page *page;
unsigned long numpages;
if (v_block_mapped((unsigned long)_sinittext)) {
@@ -222,20 +144,18 @@ void mark_rodata_ro(void)
return;
}
- page = virt_to_page(_stext);
numpages = PFN_UP((unsigned long)_etext) -
PFN_DOWN((unsigned long)_stext);
- change_page_attr(page, numpages, PAGE_KERNEL_ROX);
+ set_memory_attr((unsigned long)_stext, numpages, PAGE_KERNEL_ROX);
/*
* mark .rodata as read only. Use __init_begin rather than __end_rodata
* to cover NOTES and EXCEPTION_TABLE.
*/
- page = virt_to_page(__start_rodata);
numpages = PFN_UP((unsigned long)__init_begin) -
PFN_DOWN((unsigned long)__start_rodata);
- change_page_attr(page, numpages, PAGE_KERNEL_RO);
+ set_memory_attr((unsigned long)__start_rodata, numpages, PAGE_KERNEL_RO);
// mark_initmem_nx() should have already run by now
ptdump_check_wx();
@@ -245,9 +165,14 @@ void mark_rodata_ro(void)
#ifdef CONFIG_DEBUG_PAGEALLOC
void __kernel_map_pages(struct page *page, int numpages, int enable)
{
+ unsigned long addr = (unsigned long)page_address(page);
+
if (PageHighMem(page))
return;
- change_page_attr(page, numpages, enable ? PAGE_KERNEL : __pgprot(0));
+ if (enable)
+ set_memory_attr(addr, numpages, PAGE_KERNEL);
+ else
+ set_memory_attr(addr, numpages, __pgprot(0));
}
#endif /* CONFIG_DEBUG_PAGEALLOC */
--
2.25.1
^ permalink raw reply related
* [PATCH v6 6/7] powerpc/mm: implement set_memory_attr()
From: Russell Currey @ 2020-03-10 1:03 UTC (permalink / raw)
To: linuxppc-dev
Cc: ajd, kernel-hardening, npiggin, kbuild test robot, joel,
Russell Currey, dja
In-Reply-To: <20200310010338.21205-1-ruscur@russell.cc>
From: Christophe Leroy <christophe.leroy@c-s.fr>
In addition to the set_memory_xx() functions which allows to change
the memory attributes of not (yet) used memory regions, implement a
set_memory_attr() function to:
- set the final memory protection after init on currently used
kernel regions.
- enable/disable kernel memory regions in the scope of DEBUG_PAGEALLOC.
Unlike the set_memory_xx() which can act in three step as the regions
are unused, this function must modify 'on the fly' as the kernel is
executing from them. At the moment only PPC32 will use it and changing
page attributes on the fly is not an issue.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reported-by: kbuild test robot <lkp@intel.com>
[ruscur: cast "data" to unsigned long instead of int]
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
arch/powerpc/include/asm/set_memory.h | 2 ++
arch/powerpc/mm/pageattr.c | 33 +++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/arch/powerpc/include/asm/set_memory.h b/arch/powerpc/include/asm/set_memory.h
index 64011ea444b4..b040094f7920 100644
--- a/arch/powerpc/include/asm/set_memory.h
+++ b/arch/powerpc/include/asm/set_memory.h
@@ -29,4 +29,6 @@ static inline int set_memory_x(unsigned long addr, int numpages)
return change_memory_attr(addr, numpages, SET_MEMORY_X);
}
+int set_memory_attr(unsigned long addr, int numpages, pgprot_t prot);
+
#endif
diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
index 748fa56d9db0..60139fedc6cc 100644
--- a/arch/powerpc/mm/pageattr.c
+++ b/arch/powerpc/mm/pageattr.c
@@ -77,3 +77,36 @@ int change_memory_attr(unsigned long addr, int numpages, long action)
return apply_to_page_range(&init_mm, start, sz, change_page_attr, (void *)action);
}
+
+/*
+ * Set the attributes of a page:
+ *
+ * This function is used by PPC32 at the end of init to set final kernel memory
+ * protection. It includes changing the maping of the page it is executing from
+ * and data pages it is using.
+ */
+static int set_page_attr(pte_t *ptep, unsigned long addr, void *data)
+{
+ pgprot_t prot = __pgprot((unsigned long)data);
+
+ spin_lock(&init_mm.page_table_lock);
+
+ set_pte_at(&init_mm, addr, ptep, pte_modify(*ptep, prot));
+ flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
+
+ spin_unlock(&init_mm.page_table_lock);
+
+ return 0;
+}
+
+int set_memory_attr(unsigned long addr, int numpages, pgprot_t prot)
+{
+ unsigned long start = ALIGN_DOWN(addr, PAGE_SIZE);
+ unsigned long sz = numpages * PAGE_SIZE;
+
+ if (!numpages)
+ return 0;
+
+ return apply_to_page_range(&init_mm, start, sz, set_page_attr,
+ (void *)pgprot_val(prot));
+}
--
2.25.1
^ permalink raw reply related
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