LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 3/9] powerpc/pseries: remove the ppc-cmm file system
From: Christoph Hellwig @ 2021-03-11  8:42 UTC (permalink / raw)
  To: Al Viro
  Cc: Jason Gunthorpe, Michael S. Tsirkin, VMware, Inc.,
	David Hildenbrand, linux-kernel, dri-devel, virtualization,
	linux-mm, Minchan Kim, Alex Williamson, Nadav Amit, Daniel Vetter,
	linux-fsdevel, Andrew Morton, linuxppc-dev, Christoph Hellwig,
	Nitin Gupta
In-Reply-To: <YEjz/+HfILCUwKwb@zeniv-ca.linux.org.uk>

On Wed, Mar 10, 2021 at 04:29:51PM +0000, Al Viro wrote:
> On Tue, Mar 09, 2021 at 04:53:42PM +0100, Christoph Hellwig wrote:
> > Just use the generic anon_inode file system.
> 
> Umm...  The only problem I see here is the lifetime rules for
> that module, and that's not something introduced in this patchset.
> Said that, looks like the logics around that place is duplicated in
> cmm.c, vmw_balloon.c and virtion_balloon.c and I wonder if it would
> be better off with a helper in mm/balloon.c to be used for that setup...

Independ of all other discussions untangling that mess does seem
very useful.

^ permalink raw reply

* [PATCH v5] powerpc/uprobes: Validation for prefixed instruction
From: Ravi Bangoria @ 2021-03-11  9:15 UTC (permalink / raw)
  To: mpe
  Cc: ravi.bangoria, jniethe5, oleg, rostedt, linux-kernel, paulus,
	sandipan, naveen.n.rao, linuxppc-dev

As per ISA 3.1, prefixed instruction should not cross 64-byte
boundary. So don't allow Uprobe on such prefixed instruction.

There are two ways probed instruction is changed in mapped pages.
First, when Uprobe is activated, it searches for all the relevant
pages and replace instruction in them. In this case, if that probe
is on the 64-byte unaligned prefixed instruction, error out
directly. Second, when Uprobe is already active and user maps a
relevant page via mmap(), instruction is replaced via mmap() code
path. But because Uprobe is invalid, entire mmap() operation can
not be stopped. In this case just print an error and continue.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Acked-by: Sandipan Das <sandipan@linux.ibm.com>
---
v4: https://lore.kernel.org/r/20210305115433.140769-1-ravi.bangoria@linux.ibm.com
v4->v5:
  - Replace SZ_ macros with numbers

 arch/powerpc/kernel/uprobes.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index e8a63713e655..186f69b11e94 100644
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -41,6 +41,13 @@ int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe,
 	if (addr & 0x03)
 		return -EINVAL;
 
+	if (cpu_has_feature(CPU_FTR_ARCH_31) &&
+	    ppc_inst_prefixed(auprobe->insn) &&
+	    (addr & 0x3f) == 60) {
+		pr_info_ratelimited("Cannot register a uprobe on 64 byte unaligned prefixed instruction\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.27.0


^ permalink raw reply related

* [PATCH] powerpc/prom: remove unneeded semicolon
From: Jiapeng Chong @ 2021-03-11  9:40 UTC (permalink / raw)
  To: mpe; +Cc: Jiapeng Chong, paulus, linuxppc-dev, linux-kernel

Fix the following coccicheck warnings:

./arch/powerpc/kernel/prom_init.c:2986:2-3: Unneeded semicolon.

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
 arch/powerpc/kernel/prom_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index ccf77b9..41ed7e3 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -2983,7 +2983,7 @@ static void __init fixup_device_tree_efika_add_phy(void)
 				" 0x3 encode-int encode+"
 				" s\" interrupts\" property"
 			" finish-device");
-	};
+	}
 
 	/* Check for a PHY device node - if missing then create one and
 	 * give it's phandle to the ethernet node */
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH] ide: fix warning comparing pointer to 0
From: Jiapeng Chong @ 2021-03-11  9:48 UTC (permalink / raw)
  To: davem; +Cc: Jiapeng Chong, linux-kernel, linux-ide, paulus, linuxppc-dev

Fix the following coccicheck warning:

./drivers/ide/pmac.c:1680:38-39: WARNING comparing pointer to 0.

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
 drivers/ide/pmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ide/pmac.c b/drivers/ide/pmac.c
index ea0b064..d5171e0 100644
--- a/drivers/ide/pmac.c
+++ b/drivers/ide/pmac.c
@@ -1677,7 +1677,7 @@ static int pmac_ide_init_dma(ide_hwif_t *hwif, const struct ide_port_info *d)
 	/* We won't need pci_dev if we switch to generic consistent
 	 * DMA routines ...
 	 */
-	if (dev == NULL || pmif->dma_regs == 0)
+	if (!dev || pmif->dma_regs)
 		return -ENODEV;
 	/*
 	 * Allocate space for the DBDMA commands.
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH] powerpc/prom: remove unneeded semicolon
From: Christophe Leroy @ 2021-03-11 10:04 UTC (permalink / raw)
  To: Jiapeng Chong, mpe; +Cc: linuxppc-dev, paulus, linux-kernel
In-Reply-To: <1615455601-117447-1-git-send-email-jiapeng.chong@linux.alibaba.com>

Hi,

Le 11/03/2021 à 10:40, Jiapeng Chong a écrit :
> Fix the following coccicheck warnings:
> 
> ./arch/powerpc/kernel/prom_init.c:2986:2-3: Unneeded semicolon.
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>

You already sent this patch in February and it is under Review, see 
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=&submitter=81048&state=&q=&archive=&delegate=

Any reason for resending it ?


> ---
>   arch/powerpc/kernel/prom_init.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index ccf77b9..41ed7e3 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -2983,7 +2983,7 @@ static void __init fixup_device_tree_efika_add_phy(void)
>   				" 0x3 encode-int encode+"
>   				" s\" interrupts\" property"
>   			" finish-device");
> -	};
> +	}
>   
>   	/* Check for a PHY device node - if missing then create one and
>   	 * give it's phandle to the ethernet node */
> 

^ permalink raw reply

* [PATCH v2] ide: fix warning comparing pointer to 0
From: Jiapeng Chong @ 2021-03-11 10:18 UTC (permalink / raw)
  To: davem; +Cc: Jiapeng Chong, linux-kernel, linux-ide, paulus, linuxppc-dev

Fix the following coccicheck warning:

./drivers/ide/pmac.c:1680:38-39: WARNING comparing pointer to 0.

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
Changes in v2:
  - Modified if condition.

 drivers/ide/pmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ide/pmac.c b/drivers/ide/pmac.c
index ea0b064..8e1388b 100644
--- a/drivers/ide/pmac.c
+++ b/drivers/ide/pmac.c
@@ -1677,7 +1677,7 @@ static int pmac_ide_init_dma(ide_hwif_t *hwif, const struct ide_port_info *d)
 	/* We won't need pci_dev if we switch to generic consistent
 	 * DMA routines ...
 	 */
-	if (dev == NULL || pmif->dma_regs == 0)
+	if (!dev || !pmif->dma_regs)
 		return -ENODEV;
 	/*
 	 * Allocate space for the DBDMA commands.
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH v2 36/43] powerpc/32: Set current->thread.regs in C interrupt entry
From: Christophe Leroy @ 2021-03-11 10:38 UTC (permalink / raw)
  To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
	Paul Mackerras
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1615339900.vmbtzuirqw.astroid@bobo.none>



Le 10/03/2021 à 02:33, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of March 9, 2021 10:10 pm:
>> No need to do that is assembly, do it in C.
> 
> Hmm. No issues with the patch as such, but why does ppc32 need this but
> not 64? AFAIKS 64 sets this when a thread is created.

Looks like ppc64 was doing the same in function save_remaining_regs() in arch/ppc64/kernel/head.S 
until commit https://github.com/mpe/linux-fullhistory/commit/e5bb080d

But I can't find what happend to it in that commit.

Where is it done now ? Maybe that's also already done for ppc32.

Thanks
Christophe


> 
> Thanks,
> Nick
> 
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>>   arch/powerpc/include/asm/interrupt.h | 4 +++-
>>   arch/powerpc/kernel/entry_32.S       | 3 +--
>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
>> index 861e6eadc98c..e6d71c2e3aa2 100644
>> --- a/arch/powerpc/include/asm/interrupt.h
>> +++ b/arch/powerpc/include/asm/interrupt.h
>> @@ -33,8 +33,10 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
>>   	if (!arch_irq_disabled_regs(regs))
>>   		trace_hardirqs_off();
>>   
>> -	if (user_mode(regs))
>> +	if (user_mode(regs)) {
>> +		current->thread.regs = regs;
>>   		account_cpu_user_entry();
>> +	}
>>   #endif
>>   	/*
>>   	 * Book3E reconciles irq soft mask in asm
>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
>> index 8fe1c3fdfa6e..815a4ff1ba76 100644
>> --- a/arch/powerpc/kernel/entry_32.S
>> +++ b/arch/powerpc/kernel/entry_32.S
>> @@ -52,8 +52,7 @@
>>   prepare_transfer_to_handler:
>>   	andi.	r0,r9,MSR_PR
>>   	addi	r12, r2, THREAD
>> -	beq	2f			/* if from user, fix up THREAD.regs */
>> -	stw	r3,PT_REGS(r12)
>> +	beq	2f
>>   #ifdef CONFIG_PPC_BOOK3S_32
>>   	kuep_lock r11, r12
>>   #endif
>> -- 
>> 2.25.0
>>
>>

^ permalink raw reply

* Re: [PATCH] ALSA: ppc: keywest: remove outdated comment
From: Takashi Iwai @ 2021-03-11 10:47 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: alsa-devel, linuxppc-dev
In-Reply-To: <20210310193227.333140-1-wsa@kernel.org>

On Wed, 10 Mar 2021 20:32:27 +0100,
Wolfram Sang wrote:
> 
> The I2C attach_adapter callback is gone. Remove this reference.
> 
> Signed-off-by: Wolfram Sang <wsa@kernel.org>

Applied, thanks.


Takashi

^ permalink raw reply

* Re: [PATCH v4 3/6] ASoC: dt-bindings: fsl_rpmsg: Add binding doc for rpmsg cpu dai driver
From: Shengjiu Wang @ 2021-03-11 10:58 UTC (permalink / raw)
  To: Rob Herring
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
	Takashi Iwai, Liam Girdwood, linux-kernel, Nicolin Chen,
	Mark Brown, linuxppc-dev
In-Reply-To: <CAL_Jsq+NcXHtDo+HEFVOEcGqYTx9Heo8dc_R5Nfz1Rr-sAu6YA@mail.gmail.com>

Hi Rob

On Thu, Mar 11, 2021 at 5:12 AM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Mar 10, 2021 at 6:33 AM Shengjiu Wang <shengjiu.wang@gmail.com> wrote:
> >
> > Hi Rob
> >
> > On Wed, Mar 10, 2021 at 10:49 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Mon, Mar 08, 2021 at 09:22:27PM +0800, Shengjiu Wang wrote:
> > > > fsl_rpmsg cpu dai driver is driver for rpmsg audio, which is mainly used
> > >
> > > Bindings describe h/w blocks, not drivers.
> >
> > I will modify the descriptions. but here it is a virtual device.  the
> > mapping in real h/w is cortex M core, Cortex M core controls the SAI,
> > DMA interface. What we see from Linux side is a audio service
> > through rpmsg channel.
>
> It's describing the h/w from the view of the OS. It's not important
> that it's a Cortex-M, but how you interface to it whether that's
> shared registers, mailbox, etc. And it's what resources the block uses
> that the OS controls.

ok.

>
> > > > for getting the user's configuration from device tree and configure the
> > > > clocks which is used by Cortex-M core. So in this document define the
> > > > needed property.
> > > >
> > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > ---
> > > >  .../devicetree/bindings/sound/fsl,rpmsg.yaml  | 118 ++++++++++++++++++
> > > >  1 file changed, 118 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > > > new file mode 100644
> > > > index 000000000000..5731c1fbc0a6
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/sound/fsl,rpmsg.yaml
> > > > @@ -0,0 +1,118 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/sound/fsl,rpmsg.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: NXP Audio RPMSG CPU DAI Controller
> > > > +
> > > > +maintainers:
> > > > +  - Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > +
> > > > +description: |
> > > > +  fsl_rpmsg cpu dai driver is virtual driver for rpmsg audio, which doesn't
> > > > +  touch hardware. It is mainly used for getting the user's configuration
> > > > +  from device tree and configure the clocks which is used by Cortex-M core.
> > > > +  So in this document define the needed property.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - fsl,imx7ulp-rpmsg
> > > > +      - fsl,imx8mn-rpmsg
> > > > +      - fsl,imx8mm-rpmsg
> > > > +      - fsl,imx8mp-rpmsg
> > > > +
> > > > +  model:
> > > > +    $ref: /schemas/types.yaml#/definitions/string
> > > > +    description: User specified audio sound card name
> > > > +
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: Peripheral clock for register access
> > > > +      - description: Master clock
> > > > +      - description: DMA clock for DMA register access
> > > > +      - description: Parent clock for multiple of 8kHz sample rates
> > > > +      - description: Parent clock for multiple of 11kHz sample rates
> > > > +    minItems: 5
> > >
> > > If this doesn't touch hardware, what are these clocks for?
> >
> > When the cortex-M core support audio service, these clock
> > needed prepared & enabled by ALSA driver.
> >
> > >
> > > You don't need 'minItems' unless it's less than the number of 'items'.
> >
> > Ok, I will remove this minItems.
> >
> > >
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: ipg
> > > > +      - const: mclk
> > > > +      - const: dma
> > > > +      - const: pll8k
> > > > +      - const: pll11k
> > > > +    minItems: 5
> > > > +
> > > > +  power-domains:
> > > > +    maxItems: 1
> > > > +
> > > > +  fsl,audioindex:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [0, 1]
> > > > +    default: 0
> > > > +    description: Instance index for sound card in
> > > > +                 M core side, which share one rpmsg
> > > > +                 channel.
> > >
> > > We don't do indexes in DT. What's this numbering tied to?
> >
> > I will remove it. it is not necessary
> >
> > >
> > > > +
> > > > +  fsl,version:
> > >
> > > version of what?
> > >
> > > This seems odd at best.
> > >
> >
> > I will remove it.  it is not necessary
> >
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [1, 2]
> > >
> > > You're going to update this with every new firmware version?
> > >
> > > > +    default: 2
> > > > +    description: The version of M core image, which is
> > > > +                 to make driver compatible with different image.
> > > > +
> > > > +  fsl,buffer-size:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    description: pre allocate dma buffer size
> > >
> > > How can you have DMA, this doesn't touch h/w?
> >
> > The DMA is handled by M core image for sound playback
> > and capture. we need to allocated buffer in Linux side.
> > here just make the buffer size to be configurable.
>
> Do we set audio buffer sizes for other audio devices in DT? If not,
> why is this special? If so, why is it not common.

No. I will move it to driver.

>
> > > > +  fsl,enable-lpa:
> > > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > > +    description: enable low power audio path.
> > > > +
> > > > +  fsl,rpmsg-out:
> > > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > > +    description: |
> > > > +      This is a boolean property. If present, the transmitting function
> > > > +      will be enabled.
> > > > +
> > > > +  fsl,rpmsg-in:
> > > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > > +    description: |
> > > > +      This is a boolean property. If present, the receiving function
> > > > +      will be enabled.
> > > > +
> > > > +  fsl,codec-type:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    enum: [0, 1, 2]
> > > > +    default: 0
> > > > +    description: Sometimes the codec is registered by
> > > > +                 driver not by the device tree, this items
> > > > +                 can be used to distinguish codecs.
> > >
> > > How does one decide what value to use?
> >
> > I will add more description:
> > 0: dummy codec
> > 1: WM8960 codec
> > 2: AK4497 codec
>
> I assume the last 2 cases have nodes in DT (pointed to by
> 'audio-codec'), so this is redundant.

Ok, will remove it.

>
> > > > +
> > > > +  audio-codec:
> > > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > > +    description: The phandle of the audio codec
> > >
> > > The codec is controlled from the Linux side?
> >
> > yes.
> >
> > >
> > > > +
> > > > +  memory-region:
> > > > +    $ref: /schemas/types.yaml#/definitions/phandle
> > > > +    description: phandle to the reserved memory nodes
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - fsl,audioindex
> > > > +  - fsl,version
> > > > +  - fsl,buffer-size
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    rpmsg_audio: rpmsg_audio {
> > > > +        compatible = "fsl,imx8mn-rpmsg";
> > > > +        fsl,audioindex = <0> ;
> > > > +        fsl,version = <2>;
> > > > +        fsl,buffer-size = <0x6000000>;
> > > > +        fsl,enable-lpa;
> > >
> > > How does this work? Don't you need somewhere to put the 'rpmsg' data?
> >
> > The rpmsg data is not handled in this "rpmsg_audio" device, it is just to
> > prepare the resource for rpmsg audio function, the clock, the memory
> > the power...
> >
> > The rpmsg data is handled in imx-pcm-rpmsg.c and imx-audio-rpmsg.c
> > These devices is registered by imx remoteproc driver.
>
> Then what is 'memory-region' for? You need that, a mailbox, or ???
> somewhere in DT.
>
The M core can't access all the DDR memory space on some platform,
so use 'memory-region' reserve a specific memory for dma buffer
which M core can access.

best regards
wang shengjiu

^ permalink raw reply

* Re: [PATCH v2 36/43] powerpc/32: Set current->thread.regs in C interrupt entry
From: Christophe Leroy @ 2021-03-11 12:38 UTC (permalink / raw)
  To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
	Paul Mackerras
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <035298ad-4d0b-5e74-6f5c-e03677580924@csgroup.eu>



Le 11/03/2021 à 11:38, Christophe Leroy a écrit :
> 
> 
> Le 10/03/2021 à 02:33, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of March 9, 2021 10:10 pm:
>>> No need to do that is assembly, do it in C.
>>
>> Hmm. No issues with the patch as such, but why does ppc32 need this but
>> not 64? AFAIKS 64 sets this when a thread is created.
> 
> Looks like ppc64 was doing the same in function save_remaining_regs() in arch/ppc64/kernel/head.S 
> until commit https://github.com/mpe/linux-fullhistory/commit/e5bb080d
> 
> But I can't find what happend to it in that commit.
> 
> Where is it done now ? Maybe that's also already done for ppc32.
> 

I digged a bit more and found a later bug fix which adds that setting of current->thread.regs at 
task creation: https://github.com/mpe/linux-fullhistory/commit/3eac1897

That was in the ppc64 tree only at that time, and was merged into the common powerpc tree via commit 
https://github.com/mpe/linux-fullhistory/commit/06d67d54

So we have it for both ppc32 and ppc64 and ppc32 doesn't need to do it at exception entry anymore. 
I'll remove it.

Thanks
Christophe

^ permalink raw reply

* Re: [PATCH v2 25/43] powerpc/32: Replace ASM exception exit by C exception exit from ppc64
From: Michael Ellerman @ 2021-03-11 13:46 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, npiggin
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <a9a50f475db97fc53795dd778bc14f58029fdd55.1615291473.git.christophe.leroy@csgroup.eu>

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> This patch replaces the PPC32 ASM exception exit by C exception exit.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/powerpc/kernel/entry_32.S  | 481 +++++++++-----------------------
>  arch/powerpc/kernel/interrupt.c |   4 +
>  2 files changed, 132 insertions(+), 353 deletions(-)

Bisect points to this breaking qemu mac99 for me, with pmac32_defconfig.

I haven't had time to dig any deeper sorry.

cheers


Freeing unused kernel memory: 1132K
This architecture does not have kernel memory protection.
Run /init as init process
init[1]: User access of kernel address (fffffd20) - exploit attempt? (uid: 0)
init[1]: segfault (11) at fffffd20 nip b7e78638 lr b7e845e4 code 1 in ld-2.27.so[b7e6b000+22000]
init[1]: code: 92010080 92210084 92410088 92810090 92a10094 92c10098 930100a0 932100a4
init[1]: code: 934100a8 936100ac 93a100b4 91810074 <7d41496e> 39400000 3b810017 579c0036
Kernel panic - not syncing: Attempted to kill init! exitcode=0x00ERROR: Error: saw oops/warning etc. while expecting
00000b
CPU: 0 PID: 1 Comm: init Not tainted 5.12.0-rc2+ #1
Call Trace:
[f1019d80] [c004f1ec] panic+0x138/0x328 (unreliable)
[f1019de0] [c0051c8c] do_exit+0x880/0x8f4
[f1019e30] [c0052bdc] do_group_exit+0x40/0xa4
[f1019e50] [c0060d04] get_signal+0x1e8/0x834
[f1019eb0] [c000b624] do_notify_resume+0xc8/0x314
[f1019f10] [c0010da8] interrupt_exit_user_prepare+0xa4/0xdc
[f1019f30] [c0018228] interrupt_return+0x14/0x14c
--- interrupt: 300 at 0xb7e78638
NIP:  b7e78638 LR: b7e845e4 CTR: c01ea2d8
REGS: f1019f40 TRAP: 0300   Not tainted  (5.12.0-rc2+)
MSR:  0000d032 <EE,PR,ME,IR,DR,RI>  CR: 28004422  XER: 20000000
DAR: fffffd20 DSISR: 42000000
GPR00: b7e845e4 bf951440 00000000 bf951460 00000000 bf951718 fefefeff 7f7f7f7f
GPR08: bf9516b0 406ae8e0 b7eac1d4 00000000 0a12247b 00000000 b7e8a0d0 b7e78554
GPR16: bf951730 bf9516f0 b7eaaf40 bf9516f0 00000001 b7eaa688 10002178 bf951460
GPR24: 00000000 00000000 b7eac200 100cff38 bf9516f0 10002179 b7e845e4 bf951440
NIP [b7e78638] 0xb7e78638
LR [b7e845e4] 0xb7e845e4
--- interrupt: 300
Rebooting in 180 seconds..

^ permalink raw reply

* Re: [PATCH] ide: fix warning comparing pointer to 0
From: Jens Axboe @ 2021-03-11 15:12 UTC (permalink / raw)
  To: Jiapeng Chong, davem; +Cc: linux-kernel, linux-ide, paulus, linuxppc-dev
In-Reply-To: <1615456086-127803-1-git-send-email-jiapeng.chong@linux.alibaba.com>

On 3/11/21 2:48 AM, Jiapeng Chong wrote:
> Fix the following coccicheck warning:
> 
> ./drivers/ide/pmac.c:1680:38-39: WARNING comparing pointer to 0.
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> ---
>  drivers/ide/pmac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ide/pmac.c b/drivers/ide/pmac.c
> index ea0b064..d5171e0 100644
> --- a/drivers/ide/pmac.c
> +++ b/drivers/ide/pmac.c
> @@ -1677,7 +1677,7 @@ static int pmac_ide_init_dma(ide_hwif_t *hwif, const struct ide_port_info *d)
>  	/* We won't need pci_dev if we switch to generic consistent
>  	 * DMA routines ...
>  	 */
> -	if (dev == NULL || pmif->dma_regs == 0)
> +	if (!dev || pmif->dma_regs)
>  		return -ENODEV;

This looks utterly broken - the warning is most definitely about
dma_regs, not dev, and you swapped the condition (failing on dma_regs
being set, not NULL).

I'd just leave this one alone, drivers/ide/ should be going away soon.

-- 
Jens Axboe


^ permalink raw reply

* Re: Errant readings on LM81 with T2080 SoC
From: Guenter Roeck @ 2021-03-11 15:19 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-hwmon@vger.kernel.org, jdelvare@suse.com,
	linux-kernel@vger.kernel.org, Chris Packham,
	linux-i2c@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20210311081842.GA1070@ninjato>


[-- Attachment #1.1: Type: text/plain, Size: 763 bytes --]

On 3/11/21 12:18 AM, Wolfram Sang wrote:
> 
>> Bummer. What is really weird is that you see clock stretching under
>> CPU load. Normally clock stretching is triggered by the device, not
>> by the host.
> 
> One example: Some hosts need an interrupt per byte to know if they
> should send ACK or NACK. If that interrupt is delayed, they stretch the
> clock.
> 

Indeed, the i2c-mpc driver sends TXAK (only) after receiving
that interrupt. Since that is running in the context of the user
process, that may well be delayed substantially on a loaded system.

Maybe the interrupt handler will need to play a more active role
in the i2c-mpc driver. Alternatively, the transfer function could
be handled by a high priority kernel thread.

Guenter


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: swiotlb cleanups v2
From: Christoph Hellwig @ 2021-03-11 16:52 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: xen-devel, iommu, Dongli Zhang, Claire Chang, linuxppc-dev
In-Reply-To: <20210301074436.919889-1-hch@lst.de>

Any comments? I would be good to make some progress on this series
as the base for the various additional pools.

On Mon, Mar 01, 2021 at 08:44:22AM +0100, Christoph Hellwig wrote:
> Hi Konrad,
> 
> this series contains a bunch of swiotlb cleanups, mostly to reduce the
> amount of internals exposed to code outside of swiotlb.c, which should
> helper to prepare for supporting multiple different bounce buffer pools.
> 
> Changes since v1:
>  - rebased to v5.12-rc1
>  - a few more cleanups
>  - merge and forward port the patch from Claire to move all the global
>    variables into a struct to prepare for multiple instances
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
---end quoted text---

^ permalink raw reply

* Re: [PATCH v2] powerpc/kexec_file: Restore FDT size estimation for kdump kernel
From: Rob Herring @ 2021-03-11 16:53 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: kexec, linux-kernel, Mimi Zohar, Lakshmi Ramasubramanian,
	linuxppc-dev, Hari Bathini
In-Reply-To: <20210220005204.1417200-1-bauerman@linux.ibm.com>

On Fri, 19 Feb 2021 21:52:04 -0300, Thiago Jung Bauermann wrote:
> Commit 2377c92e37fe ("powerpc/kexec_file: fix FDT size estimation for kdump
> kernel") fixed how elf64_load() estimates the FDT size needed by the
> crashdump kernel.
> 
> At the same time, commit 130b2d59cec0 ("powerpc: Use common
> of_kexec_alloc_and_setup_fdt()") changed the same code to use the generic
> function of_kexec_alloc_and_setup_fdt() to calculate the FDT size. That
> change made the code overestimate it a bit by counting twice the space
> required for the kernel command line and /chosen properties.
> 
> Therefore change kexec_fdt_totalsize_ppc64() to calculate just the extra
> space needed by the kdump kernel, and change the function name so that it
> better reflects what the function is now doing.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
>  arch/powerpc/include/asm/kexec.h  |  2 +-
>  arch/powerpc/kexec/elf_64.c       |  2 +-
>  arch/powerpc/kexec/file_load_64.c | 26 ++++++++------------------
>  3 files changed, 10 insertions(+), 20 deletions(-)
> 
> Applies on top of next-20210219.
> 
> Changes since v1:
> 
> - Adjusted comment describing kexec_extra_fdt_size_ppc64() as suggested
>   by Lakshmi.
> 

Applied, thanks!

^ permalink raw reply

* [PATCH] [backport for 5.4] powerpc/603: Fix protection of user pages mapped with PROT_NONE
From: Christophe Leroy @ 2021-03-11 18:24 UTC (permalink / raw)
  To: stable, gregkh; +Cc: linuxppc-dev, linux-kernel

(cherry picked from commit c119565a15a628efdfa51352f9f6c5186e506a1c)

On book3s/32, page protection is defined by the PP bits in the PTE
which provide the following protection depending on the access
keys defined in the matching segment register:
- PP 00 means RW with key 0 and N/A with key 1.
- PP 01 means RW with key 0 and RO with key 1.
- PP 10 means RW with both key 0 and key 1.
- PP 11 means RO with both key 0 and key 1.

Since the implementation of kernel userspace access protection,
PP bits have been set as follows:
- PP00 for pages without _PAGE_USER
- PP01 for pages with _PAGE_USER and _PAGE_RW
- PP11 for pages with _PAGE_USER and without _PAGE_RW

For kernelspace segments, kernel accesses are performed with key 0
and user accesses are performed with key 1. As PP00 is used for
non _PAGE_USER pages, user can't access kernel pages not flagged
_PAGE_USER while kernel can.

For userspace segments, both kernel and user accesses are performed
with key 0, therefore pages not flagged _PAGE_USER are still
accessible to the user.

This shouldn't be an issue, because userspace is expected to be
accessible to the user. But unlike most other architectures, powerpc
implements PROT_NONE protection by removing _PAGE_USER flag instead of
flagging the page as not valid. This means that pages in userspace
that are not flagged _PAGE_USER shall remain inaccessible.

To get the expected behaviour, just mimic other architectures in the
TLB miss handler by checking _PAGE_USER permission on userspace
accesses as if it was the _PAGE_PRESENT bit.

Note that this problem only is only for 603 cores. The 604+ have
an hash table, and hash_page() function already implement the
verification of _PAGE_USER permission on userspace pages.

Fixes: f342adca3afc ("powerpc/32s: Prepare Kernel Userspace Access Protection")
Change-Id: I68bc5e5ff4542bdfcdcd12923fa96a5811707475
Cc: stable@vger.kernel.org # v5.2+
Reported-by: Christoph Plattner <christoph.plattner@thalesgroup.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/4a0c6e3bb8f0c162457bf54d9bc6fd8d7b55129f.1612160907.git.christophe.leroy@csgroup.eu
---
 arch/powerpc/kernel/head_32.S | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index 126ba5438430..edaab1142498 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -418,10 +418,11 @@ InstructionTLBMiss:
 	cmplw	0,r1,r3
 #endif
 	mfspr	r2, SPRN_SPRG_PGDIR
-	li	r1,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC
+	li	r1,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC | _PAGE_USER
 #if defined(CONFIG_MODULES) || defined(CONFIG_DEBUG_PAGEALLOC)
 	bge-	112f
 	lis	r2, (swapper_pg_dir - PAGE_OFFSET)@ha	/* if kernel address, use */
+	li	r1,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC
 	addi	r2, r2, (swapper_pg_dir - PAGE_OFFSET)@l	/* kernel page table */
 #endif
 112:	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
@@ -480,9 +481,10 @@ DataLoadTLBMiss:
 	lis	r1,PAGE_OFFSET@h		/* check if kernel address */
 	cmplw	0,r1,r3
 	mfspr	r2, SPRN_SPRG_PGDIR
-	li	r1, _PAGE_PRESENT | _PAGE_ACCESSED
+	li	r1, _PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_USER
 	bge-	112f
 	lis	r2, (swapper_pg_dir - PAGE_OFFSET)@ha	/* if kernel address, use */
+	li	r1, _PAGE_PRESENT | _PAGE_ACCESSED
 	addi	r2, r2, (swapper_pg_dir - PAGE_OFFSET)@l	/* kernel page table */
 112:	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
 	lwz	r2,0(r2)		/* get pmd entry */
@@ -556,9 +558,10 @@ DataStoreTLBMiss:
 	lis	r1,PAGE_OFFSET@h		/* check if kernel address */
 	cmplw	0,r1,r3
 	mfspr	r2, SPRN_SPRG_PGDIR
-	li	r1, _PAGE_RW | _PAGE_DIRTY | _PAGE_PRESENT | _PAGE_ACCESSED
+	li	r1, _PAGE_RW | _PAGE_DIRTY | _PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_USER
 	bge-	112f
 	lis	r2, (swapper_pg_dir - PAGE_OFFSET)@ha	/* if kernel address, use */
+	li	r1, _PAGE_RW | _PAGE_DIRTY | _PAGE_PRESENT | _PAGE_ACCESSED
 	addi	r2, r2, (swapper_pg_dir - PAGE_OFFSET)@l	/* kernel page table */
 112:	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
 	lwz	r2,0(r2)		/* get pmd entry */
-- 
2.25.0


^ permalink raw reply related

* [PATCH] [backport for 5.10] powerpc/603: Fix protection of user pages mapped with PROT_NONE
From: Christophe Leroy @ 2021-03-11 18:24 UTC (permalink / raw)
  To: stable, gregkh; +Cc: linuxppc-dev, linux-kernel

(cherry picked from commit c119565a15a628efdfa51352f9f6c5186e506a1c)

On book3s/32, page protection is defined by the PP bits in the PTE
which provide the following protection depending on the access
keys defined in the matching segment register:
- PP 00 means RW with key 0 and N/A with key 1.
- PP 01 means RW with key 0 and RO with key 1.
- PP 10 means RW with both key 0 and key 1.
- PP 11 means RO with both key 0 and key 1.

Since the implementation of kernel userspace access protection,
PP bits have been set as follows:
- PP00 for pages without _PAGE_USER
- PP01 for pages with _PAGE_USER and _PAGE_RW
- PP11 for pages with _PAGE_USER and without _PAGE_RW

For kernelspace segments, kernel accesses are performed with key 0
and user accesses are performed with key 1. As PP00 is used for
non _PAGE_USER pages, user can't access kernel pages not flagged
_PAGE_USER while kernel can.

For userspace segments, both kernel and user accesses are performed
with key 0, therefore pages not flagged _PAGE_USER are still
accessible to the user.

This shouldn't be an issue, because userspace is expected to be
accessible to the user. But unlike most other architectures, powerpc
implements PROT_NONE protection by removing _PAGE_USER flag instead of
flagging the page as not valid. This means that pages in userspace
that are not flagged _PAGE_USER shall remain inaccessible.

To get the expected behaviour, just mimic other architectures in the
TLB miss handler by checking _PAGE_USER permission on userspace
accesses as if it was the _PAGE_PRESENT bit.

Note that this problem only is only for 603 cores. The 604+ have
an hash table, and hash_page() function already implement the
verification of _PAGE_USER permission on userspace pages.

Fixes: f342adca3afc ("powerpc/32s: Prepare Kernel Userspace Access Protection")
Change-Id: I68bc5e5ff4542bdfcdcd12923fa96a5811707475
Cc: stable@vger.kernel.org # v5.2+
Reported-by: Christoph Plattner <christoph.plattner@thalesgroup.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/4a0c6e3bb8f0c162457bf54d9bc6fd8d7b55129f.1612160907.git.christophe.leroy@csgroup.eu
---
 arch/powerpc/kernel/head_book3s_32.S | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/head_book3s_32.S b/arch/powerpc/kernel/head_book3s_32.S
index 2729d8fa6e77..96b45901da64 100644
--- a/arch/powerpc/kernel/head_book3s_32.S
+++ b/arch/powerpc/kernel/head_book3s_32.S
@@ -461,10 +461,11 @@ InstructionTLBMiss:
 	cmplw	0,r1,r3
 #endif
 	mfspr	r2, SPRN_SPRG_PGDIR
-	li	r1,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC
+	li	r1,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC | _PAGE_USER
 #if defined(CONFIG_MODULES) || defined(CONFIG_DEBUG_PAGEALLOC)
 	bgt-	112f
 	lis	r2, (swapper_pg_dir - PAGE_OFFSET)@ha	/* if kernel address, use */
+	li	r1,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC
 	addi	r2, r2, (swapper_pg_dir - PAGE_OFFSET)@l	/* kernel page table */
 #endif
 112:	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
@@ -523,9 +524,10 @@ DataLoadTLBMiss:
 	lis	r1, TASK_SIZE@h		/* check if kernel address */
 	cmplw	0,r1,r3
 	mfspr	r2, SPRN_SPRG_PGDIR
-	li	r1, _PAGE_PRESENT | _PAGE_ACCESSED
+	li	r1, _PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_USER
 	bgt-	112f
 	lis	r2, (swapper_pg_dir - PAGE_OFFSET)@ha	/* if kernel address, use */
+	li	r1, _PAGE_PRESENT | _PAGE_ACCESSED
 	addi	r2, r2, (swapper_pg_dir - PAGE_OFFSET)@l	/* kernel page table */
 112:	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
 	lwz	r2,0(r2)		/* get pmd entry */
@@ -599,9 +601,10 @@ DataStoreTLBMiss:
 	lis	r1, TASK_SIZE@h		/* check if kernel address */
 	cmplw	0,r1,r3
 	mfspr	r2, SPRN_SPRG_PGDIR
-	li	r1, _PAGE_RW | _PAGE_DIRTY | _PAGE_PRESENT | _PAGE_ACCESSED
+	li	r1, _PAGE_RW | _PAGE_DIRTY | _PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_USER
 	bgt-	112f
 	lis	r2, (swapper_pg_dir - PAGE_OFFSET)@ha	/* if kernel address, use */
+	li	r1, _PAGE_RW | _PAGE_DIRTY | _PAGE_PRESENT | _PAGE_ACCESSED
 	addi	r2, r2, (swapper_pg_dir - PAGE_OFFSET)@l	/* kernel page table */
 112:	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
 	lwz	r2,0(r2)		/* get pmd entry */
-- 
2.25.0


^ permalink raw reply related

* Re: [PATCH v2 25/43] powerpc/32: Replace ASM exception exit by C exception exit from ppc64
From: Christophe Leroy @ 2021-03-11 19:39 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, npiggin
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <87tuphkdkz.fsf@mpe.ellerman.id.au>



Le 11/03/2021 à 14:46, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> This patch replaces the PPC32 ASM exception exit by C exception exit.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>>   arch/powerpc/kernel/entry_32.S  | 481 +++++++++-----------------------
>>   arch/powerpc/kernel/interrupt.c |   4 +
>>   2 files changed, 132 insertions(+), 353 deletions(-)
> 
> Bisect points to this breaking qemu mac99 for me, with pmac32_defconfig.
> 
> I haven't had time to dig any deeper sorry.

Embarrasing ...

I don't get this problem on the 8xx (nohash/32) or the 83xx (book3s/32).
I don't get this problem with qemu mac99 when using my klibc-based initramfs.

I managed to reproduce it with the rootfs.cpio that I got some time ago from linuxppc github Wiki.

I'll investigate it tomorrow.

Thanks
Christophe


> 
> cheers
> 
> 
> Freeing unused kernel memory: 1132K
> This architecture does not have kernel memory protection.
> Run /init as init process
> init[1]: User access of kernel address (fffffd20) - exploit attempt? (uid: 0)
> init[1]: segfault (11) at fffffd20 nip b7e78638 lr b7e845e4 code 1 in ld-2.27.so[b7e6b000+22000]
> init[1]: code: 92010080 92210084 92410088 92810090 92a10094 92c10098 930100a0 932100a4
> init[1]: code: 934100a8 936100ac 93a100b4 91810074 <7d41496e> 39400000 3b810017 579c0036
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x00ERROR: Error: saw oops/warning etc. while expecting
> 00000b
> CPU: 0 PID: 1 Comm: init Not tainted 5.12.0-rc2+ #1
> Call Trace:
> [f1019d80] [c004f1ec] panic+0x138/0x328 (unreliable)
> [f1019de0] [c0051c8c] do_exit+0x880/0x8f4
> [f1019e30] [c0052bdc] do_group_exit+0x40/0xa4
> [f1019e50] [c0060d04] get_signal+0x1e8/0x834
> [f1019eb0] [c000b624] do_notify_resume+0xc8/0x314
> [f1019f10] [c0010da8] interrupt_exit_user_prepare+0xa4/0xdc
> [f1019f30] [c0018228] interrupt_return+0x14/0x14c
> --- interrupt: 300 at 0xb7e78638
> NIP:  b7e78638 LR: b7e845e4 CTR: c01ea2d8
> REGS: f1019f40 TRAP: 0300   Not tainted  (5.12.0-rc2+)
> MSR:  0000d032 <EE,PR,ME,IR,DR,RI>  CR: 28004422  XER: 20000000
> DAR: fffffd20 DSISR: 42000000
> GPR00: b7e845e4 bf951440 00000000 bf951460 00000000 bf951718 fefefeff 7f7f7f7f
> GPR08: bf9516b0 406ae8e0 b7eac1d4 00000000 0a12247b 00000000 b7e8a0d0 b7e78554
> GPR16: bf951730 bf9516f0 b7eaaf40 bf9516f0 00000001 b7eaa688 10002178 bf951460
> GPR24: 00000000 00000000 b7eac200 100cff38 bf9516f0 10002179 b7e845e4 bf951440
> NIP [b7e78638] 0xb7e78638
> LR [b7e845e4] 0xb7e845e4
> --- interrupt: 300
> Rebooting in 180 seconds..
> 

^ permalink raw reply

* Re: Errant readings on LM81 with T2080 SoC
From: Chris Packham @ 2021-03-11 21:17 UTC (permalink / raw)
  To: Wolfram Sang, Guenter Roeck
  Cc: linux-hwmon@vger.kernel.org, jdelvare@suse.com,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	linux-i2c@vger.kernel.org
In-Reply-To: <20210311081842.GA1070@ninjato>


On 11/03/21 9:18 pm, Wolfram Sang wrote:
>> Bummer. What is really weird is that you see clock stretching under
>> CPU load. Normally clock stretching is triggered by the device, not
>> by the host.
> One example: Some hosts need an interrupt per byte to know if they
> should send ACK or NACK. If that interrupt is delayed, they stretch the
> clock.
>
It feels like something like that is happening. Looking at the T2080 
Reference manual there is an interesting timing diagram (Figure 14-2 if 
someone feels like looking it up). It shows SCL low between the ACK for 
the address and the data byte. I think if we're delayed in sending the 
next byte we could violate Ttimeout or Tlow:mext from the SMBUS spec.

^ permalink raw reply

* Re: Errant readings on LM81 with T2080 SoC
From: Guenter Roeck @ 2021-03-11 21:34 UTC (permalink / raw)
  To: Chris Packham, Wolfram Sang
  Cc: linux-hwmon@vger.kernel.org, jdelvare@suse.com,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	linux-i2c@vger.kernel.org
In-Reply-To: <94dfa9dc-a80c-98ba-4169-44cce3d810f7@alliedtelesis.co.nz>

On 3/11/21 1:17 PM, Chris Packham wrote:
> 
> On 11/03/21 9:18 pm, Wolfram Sang wrote:
>>> Bummer. What is really weird is that you see clock stretching under
>>> CPU load. Normally clock stretching is triggered by the device, not
>>> by the host.
>> One example: Some hosts need an interrupt per byte to know if they
>> should send ACK or NACK. If that interrupt is delayed, they stretch the
>> clock.
>>
> It feels like something like that is happening. Looking at the T2080 
> Reference manual there is an interesting timing diagram (Figure 14-2 if 
> someone feels like looking it up). It shows SCL low between the ACK for 
> the address and the data byte. I think if we're delayed in sending the 
> next byte we could violate Ttimeout or Tlow:mext from the SMBUS spec.
> 

I think that really leaves you only two options that I can see:
Rework the driver to handle critical actions (such as setting TXAK,
and everything else that might result in clock stretching) in the
interrupt handler, or rework the driver to handle everything in
a high priority kernel thread.

Guenter

^ permalink raw reply

* Re: [PATCH v2 25/43] powerpc/32: Replace ASM exception exit by C exception exit from ppc64
From: Michael Ellerman @ 2021-03-11 23:26 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, npiggin
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0296d1bc-b37e-43c8-06cf-00ec458fb74e@csgroup.eu>

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 11/03/2021 à 14:46, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> This patch replaces the PPC32 ASM exception exit by C exception exit.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>> ---
>>>   arch/powerpc/kernel/entry_32.S  | 481 +++++++++-----------------------
>>>   arch/powerpc/kernel/interrupt.c |   4 +
>>>   2 files changed, 132 insertions(+), 353 deletions(-)
>> 
>> Bisect points to this breaking qemu mac99 for me, with pmac32_defconfig.
>> 
>> I haven't had time to dig any deeper sorry.
>
> Embarrasing ...

Nah, these things happen.

> I don't get this problem on the 8xx (nohash/32) or the 83xx (book3s/32).
> I don't get this problem with qemu mac99 when using my klibc-based initramfs.
>
> I managed to reproduce it with the rootfs.cpio that I got some time ago from linuxppc github Wiki.

OK.

I'm using the ppc-rootfs.cpio.gz from here:

  https://github.com/linuxppc/ci-scripts/blob/master/root-disks/Makefile

And the boot script is:

  https://github.com/linuxppc/ci-scripts/blob/master/scripts/boot/qemu-mac99

I've been meaning to write docs on how to use those scripts, but haven't
got around to it.

There's nothing really special though it's just a wrapper around qemu -M mac99.

> I'll investigate it tomorrow.

Thanks.

cheers

^ permalink raw reply

* Re: PowerPC64 future proof kernel toc, revised for lld
From: Michael Ellerman @ 2021-03-11 23:32 UTC (permalink / raw)
  To: Alan Modra, Christophe Leroy; +Cc: alexey, Alexey Kardashevskiy, linuxppc-dev
In-Reply-To: <20210310234135.GC29645@bubble.grove.modra.org>

Alan Modra <amodra@gmail.com> writes:
> On Wed, Mar 10, 2021 at 01:44:57PM +0100, Christophe Leroy wrote:
>> 
>> Le 10/03/2021 à 13:25, Alan Modra a écrit :
>> > On Wed, Mar 10, 2021 at 08:33:37PM +1100, Alexey Kardashevskiy wrote:
>> > > One more question - the older version had a construct "DEFINED (.TOC.) ?
>> > > .TOC. : ..." in case .TOC. is not defined (too old ld? too old gcc?) but the
>> > > newer patch seems assuming it is always defined, when was it added? I have
>> > > the same check in SLOF, for example, do I still need it?
>> > 
>> > .TOC. symbol support was first added 2012-11-06, so you need
>> > binutils-2.24 or later to use .TOC. as a symbol.
>> > 
>> 
>> As of today, minimum requirement to build kernel is binutils 2.23, see https://urldefense.proofpoint.com/v2/url?u=https-3A__www.kernel.org_doc_html_latest_process_changes.html-23current-2Dminimal-2Drequirements&d=DwIDAw&c=jf_iaSHvJObTbx-siA1ZOg&r=uzpscot8Q8p-51o1Gp1vnzKV94bfny2qmUdVe821lv0&m=SYi605mn0I1hf1QoHuvHXtS_Z-R6JJHbzS34cEtV2Tk&s=47ckf3yxVcP6RwRb8D9viYOQSWpf6rXrnWj4YM4OTJ0&e= 
>
> Yes, and arch/powerpc/Makefile complains about 2.24.  So for powerpc
> that means you need to go to at least 2.25.  

Not quite. It only complains for little endian builds, and only if you
have stock 2.24, it will allow a 2.24.<something>.

I do most of my builds with 2.34, so I have no issue with newer
binutils. But we try not to increase the minimum version too rapidly to
accommodate folks using older and/or "Enterprise" distros that are stuck
on old toolchains.

I think we are within our rights to increase the minimum requirement for
powerpc builds, if it brings advantages we can identify.

The way to do that would be to add a new check in our arch Makefile that
rejects the older versions.

cheers

^ permalink raw reply

* Re: Errant readings on LM81 with T2080 SoC
From: Chris Packham @ 2021-03-11 23:47 UTC (permalink / raw)
  To: Guenter Roeck, Wolfram Sang
  Cc: linux-hwmon@vger.kernel.org, jdelvare@suse.com,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	linux-i2c@vger.kernel.org
In-Reply-To: <725c5e51-65df-e17d-e2da-0982efacf2d2@roeck-us.net>


On 12/03/21 10:34 am, Guenter Roeck wrote:
> On 3/11/21 1:17 PM, Chris Packham wrote:
>> On 11/03/21 9:18 pm, Wolfram Sang wrote:
>>>> Bummer. What is really weird is that you see clock stretching under
>>>> CPU load. Normally clock stretching is triggered by the device, not
>>>> by the host.
>>> One example: Some hosts need an interrupt per byte to know if they
>>> should send ACK or NACK. If that interrupt is delayed, they stretch the
>>> clock.
>>>
>> It feels like something like that is happening. Looking at the T2080
>> Reference manual there is an interesting timing diagram (Figure 14-2 if
>> someone feels like looking it up). It shows SCL low between the ACK for
>> the address and the data byte. I think if we're delayed in sending the
>> next byte we could violate Ttimeout or Tlow:mext from the SMBUS spec.
>>
> I think that really leaves you only two options that I can see:
> Rework the driver to handle critical actions (such as setting TXAK,
> and everything else that might result in clock stretching) in the
> interrupt handler, or rework the driver to handle everything in
> a high priority kernel thread.
One thing I've found that does seem to avoid the problem is to disable 
preemption, use polling and replace the schedule() in i2c_wait() with 
udelay(50). That's kind of like the kernel thread option.
> Guenter

^ permalink raw reply

* Re: Errant readings on LM81 with T2080 SoC
From: Guenter Roeck @ 2021-03-12  0:07 UTC (permalink / raw)
  To: Chris Packham, Wolfram Sang
  Cc: linux-hwmon@vger.kernel.org, jdelvare@suse.com,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	linux-i2c@vger.kernel.org
In-Reply-To: <309f94fa-40ec-c3be-7cdf-78a910a5b209@alliedtelesis.co.nz>

On 3/11/21 3:47 PM, Chris Packham wrote:
> 
> On 12/03/21 10:34 am, Guenter Roeck wrote:
>> On 3/11/21 1:17 PM, Chris Packham wrote:
>>> On 11/03/21 9:18 pm, Wolfram Sang wrote:
>>>>> Bummer. What is really weird is that you see clock stretching under
>>>>> CPU load. Normally clock stretching is triggered by the device, not
>>>>> by the host.
>>>> One example: Some hosts need an interrupt per byte to know if they
>>>> should send ACK or NACK. If that interrupt is delayed, they stretch the
>>>> clock.
>>>>
>>> It feels like something like that is happening. Looking at the T2080
>>> Reference manual there is an interesting timing diagram (Figure 14-2 if
>>> someone feels like looking it up). It shows SCL low between the ACK for
>>> the address and the data byte. I think if we're delayed in sending the
>>> next byte we could violate Ttimeout or Tlow:mext from the SMBUS spec.
>>>
>> I think that really leaves you only two options that I can see:
>> Rework the driver to handle critical actions (such as setting TXAK,
>> and everything else that might result in clock stretching) in the
>> interrupt handler, or rework the driver to handle everything in
>> a high priority kernel thread.
> One thing I've found that does seem to avoid the problem is to disable 
> preemption, use polling and replace the schedule() in i2c_wait() with 
> udelay(50). That's kind of like the kernel thread option.

It is kind of hackish, though, especially since it makes the "loaded system"
situation even worse by adding even more active wait loops.

Guenter

^ permalink raw reply

* Re: Errant readings on LM81 with T2080 SoC
From: Chris Packham @ 2021-03-12  0:19 UTC (permalink / raw)
  To: Guenter Roeck, Wolfram Sang
  Cc: linux-hwmon@vger.kernel.org, jdelvare@suse.com,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	linux-i2c@vger.kernel.org
In-Reply-To: <62ee2a1c-19ea-8287-a438-ef7bdf5472de@roeck-us.net>


On 12/03/21 1:07 pm, Guenter Roeck wrote:
> On 3/11/21 3:47 PM, Chris Packham wrote:
>> On 12/03/21 10:34 am, Guenter Roeck wrote:
>>> On 3/11/21 1:17 PM, Chris Packham wrote:
>>>> On 11/03/21 9:18 pm, Wolfram Sang wrote:
>>>>>> Bummer. What is really weird is that you see clock stretching under
>>>>>> CPU load. Normally clock stretching is triggered by the device, not
>>>>>> by the host.
>>>>> One example: Some hosts need an interrupt per byte to know if they
>>>>> should send ACK or NACK. If that interrupt is delayed, they stretch the
>>>>> clock.
>>>>>
>>>> It feels like something like that is happening. Looking at the T2080
>>>> Reference manual there is an interesting timing diagram (Figure 14-2 if
>>>> someone feels like looking it up). It shows SCL low between the ACK for
>>>> the address and the data byte. I think if we're delayed in sending the
>>>> next byte we could violate Ttimeout or Tlow:mext from the SMBUS spec.
>>>>
>>> I think that really leaves you only two options that I can see:
>>> Rework the driver to handle critical actions (such as setting TXAK,
>>> and everything else that might result in clock stretching) in the
>>> interrupt handler, or rework the driver to handle everything in
>>> a high priority kernel thread.
>> One thing I've found that does seem to avoid the problem is to disable
>> preemption, use polling and replace the schedule() in i2c_wait() with
>> udelay(50). That's kind of like the kernel thread option.
> It is kind of hackish, though, especially since it makes the "loaded system"
> situation even worse by adding even more active wait loops.
No -ish about it :). But it might put out one fire for me while I'm 
looking at doing some kind of interrupt driven state machine.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox