* Re: [patch V3 34/35] soc: ti: ti_sci_inta_msi: Get rid of ti_sci_inta_msi_get_virq()
From: Nishanth Menon @ 2021-12-13 17:57 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Mark Rutland, Stuart Yoder, Will Deacon, Peter Ujfalusi,
Ashok Raj, Joerg Roedel, Jassi Brar, Sinan Kaya, iommu,
Bjorn Helgaas, linux-arm-kernel, Jason Gunthorpe, linux-pci,
xen-devel, Kevin Tian, Arnd Bergmann, Robin Murphy,
Alex Williamson, Cedric Le Goater, Santosh Shilimkar,
Bjorn Helgaas, Megha Dey, Laurentiu Tudor, Juergen Gross,
Tero Kristo, Greg Kroah-Hartman, LKML, Vinod Koul, Marc Zygnier,
dmaengine, linuxppc-dev
In-Reply-To: <20211210221815.269468319@linutronix.de>
On 23:19-20211210, Thomas Gleixner wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> Just use the core function msi_get_virq().
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Cc: Peter Ujfalusi <peter.ujfalusi@gmail.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: dmaengine@vger.kernel.org
Acked-by: Nishanth Menon <nm@ti.com>
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D)/Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
^ permalink raw reply
* Re: [PATCH v1 0/5] Implement livepatch on PPC32
From: Christophe Leroy @ 2021-12-13 19:33 UTC (permalink / raw)
To: Steven Rostedt
Cc: Petr Mladek, Joe Lawrence, linux-s390@vger.kernel.org,
Jiri Kosina, linux-kernel@vger.kernel.org, Ingo Molnar,
Josh Poimboeuf, live-patching@vger.kernel.org, Naveen N . Rao,
Miroslav Benes, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20211213135410.12642d8f@gandalf.local.home>
Le 13/12/2021 à 19:54, Steven Rostedt a écrit :
> On Mon, 13 Dec 2021 17:50:52 +0000
> Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>
>> @@ -958,6 +942,12 @@ unsigned long prepare_ftrace_return(unsigned long
>> parent, unsigned long ip,
>> out:
>> return parent;
>> }
>> +
>> +void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>> + struct ftrace_ops *op, struct ftrace_regs *fregs)
>> +{
>> + prepare_ftrace_return(ip, kernel_stack_pointer(&fregs->regs), 0);
>> +}
>
> I have for powerpc prepare_ftrace_return as:
>
>
> unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip,
> unsigned long sp)
> {
> unsigned long return_hooker;
>
> if (unlikely(ftrace_graph_is_dead()))
> goto out;
>
> if (unlikely(atomic_read(¤t->tracing_graph_pause)))
> goto out;
>
> return_hooker = ppc_function_entry(return_to_handler);
>
> if (!function_graph_enter(parent, ip, 0, (unsigned long *)sp))
> parent = return_hooker;
> out:
> return parent;
> }
>
> Which means you'll need different parameters to it than what x86 has, which
> has the prototype of:
>
> void prepare_ftrace_return(unsigned long ip, unsigned long *parent,
> unsigned long frame_pointer)
>
> and it does not use the frame_pointer for this case, which is why it is
> zero.
>
> For powerpc though, it uses the stack pointer, so you parameters are
> incorrect. Looks like it should be:
>
> prepare_ftrace_return(parent_ip, ip, kernel_stack_pointer(&fregs->regs));
>
> And that will likely not be enough. I'll need to update the ctr register,
> as that is where the return address is saved. So you'll probably need it to be:
>
> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> struct ftrace_ops *op, struct ftrace_regs *fregs)
> {
> unsigned long parent;
>
> parent = prepare_ftrace_return(parent_ip, ip, kernel_stack_pointer(&fregs->regs));
> fregs->regs.ctr = parent;
> }
>
STill the same Oops, below
I will look more closely tomorrow.
[ 8.018219] Testing tracer function_graph:
[ 8.043884] Kernel attempted to read user page (4) - exploit attempt?
(uid: 0)
[ 8.055074] Kernel attempted to read user page (4) - exploit attempt?
(uid: 0)
[ 8.062204] BUG: Kernel NULL pointer dereference on read at 0x00000004
[ 8.068643] Faulting instruction address: 0xc0014694
[ 8.073556] Oops: Kernel access of bad area, sig: 11 [#1]
[ 8.078884] BE PAGE_SIZE=16K PREEMPT CMPC885
[ 8.083109] Modules linked in:
[ 8.086120] CPU: 0 PID: 1 Comm: swapper Not tainted
5.16.0-rc3-s3k-dev-02295-g0bd6d618bcd8-dirty #733
[ 8.095240] NIP: c0014694 LR: c00c8434 CTR: c0014674
[ 8.100227] REGS: c902b9e0 TRAP: 0300 Not tainted
(5.16.0-rc3-s3k-dev-02295-g0bd6d618bcd8-dirty)
[ 8.109178] MSR: 00001032 <ME,IR,DR,RI> CR: 88022242 XER: 20000000
[ 8.115632] DAR: 00000004 DSISR: c0000000
[ 8.115632] GPR00: c00c8434 c902baa0 c2140000 c0015278 c0003ac4
c122db78 00000000 00000300
[ 8.115632] GPR08: c2140000 c0014674 c0015278 00000000 2802b242
00000000 c0004f38 00000000
[ 8.115632] GPR16: 00000000 00000000 00000000 00000000 00000000
00000010 c1037d1c c12d0000
[ 8.115632] GPR24: c121c440 c12b5380 c12b0000 c0003ac4 c0015278
00000000 00000000 c122db78
[ 8.154272] NIP [c0014694] ftrace_graph_func+0x20/0x8c
[ 8.159351] LR [c00c8434] arch_ftrace_ops_list_func+0x118/0x230
[ 8.165208] Call Trace:
[ 8.167616] [c902baa0] [c006c048] vprintk_emit+0x188/0x2a4 (unreliable)
[ 8.174158] [c902bac0] [c00c8434] arch_ftrace_ops_list_func+0x118/0x230
[ 8.180699] [c902bb10] [c0014774] ftrace_call+0x4/0x44
[ 8.185776] [c902bb40] [c0003ac4] DataTLBError_virt+0x114/0x118
[ 8.191627] --- interrupt: 300 at ftrace_graph_func+0x20/0x8c
[ 8.197306] NIP: c0014694 LR: c00c8434 CTR: c0014674
[ 8.202296] REGS: c902bb50 TRAP: 0300 Not tainted
(5.16.0-rc3-s3k-dev-02295-g0bd6d618bcd8-dirty)
[ 8.211245] MSR: 00001032 <ME,IR,DR,RI> CR: 82002842 XER: 20000000
[ 8.217699] DAR: 00000004 DSISR: c0000000
[ 8.217699] GPR00: c00c8434 c902bc10 c2140000 c0015754 c0016264
c122db78 00000000 00000100
[ 8.217699] GPR08: c2140000 c0014674 c0015754 00000000 22004842
00000000 c0004f38 00000000
[ 8.217699] GPR16: 00000000 00000000 00000000 00000000 00000000
00000010 c1037d1c c12d0000
[ 8.217699] GPR24: c121c440 c12b5380 c12b0000 c0016264 c0015754
00000000 00000000 c122db78
[ 8.256340] NIP [c0014694] ftrace_graph_func+0x20/0x8c
[ 8.261418] LR [c00c8434] arch_ftrace_ops_list_func+0x118/0x230
[ 8.267270] --- interrupt: 300
[ 8.270288] [c902bc10] [c00adb98]
clockevents_program_event+0x108/0x254 (unreliable)
[ 8.277947] [c902bc30] [c00c8434] arch_ftrace_ops_list_func+0x118/0x230
[ 8.284488] [c902bc80] [c0014774] ftrace_call+0x4/0x44
[ 8.289565] [c902bcb0] [c0016264] map_kernel_page+0xc8/0x12c
[ 8.295159] [c902bd00] [c0019cc8] patch_instruction+0xbc/0x278
[ 8.300926] [c902bd30] [c0013964] ftrace_modify_code+0x38/0xc4
[ 8.306691] [c902bd70] [c00c2c2c] ftrace_replace_code+0x78/0xec
[ 8.312543] [c902bd90] [c00c2e50] ftrace_modify_all_code+0xd0/0x148
[ 8.318740] [c902bdb0] [c00c2f58] ftrace_run_update_code+0x28/0x88
[ 8.324850] [c902bdc0] [c00c75fc] ftrace_startup+0x118/0x1e0
[ 8.330443] [c902bde0] [c00e8330] register_ftrace_graph+0x334/0x3c0
[ 8.336640] [c902be20] [c100ccf4]
trace_selftest_startup_function_graph+0x64/0x164
[ 8.344127] [c902be50] [c00debe0] run_tracer_selftest+0x120/0x1b4
[ 8.350152] [c902be70] [c100c74c] register_tracer+0x14c/0x218
[ 8.355832] [c902be90] [c0004a30] do_one_initcall+0x44/0x1e8
[ 8.361425] [c902bef0] [c10011f4] kernel_init_freeable+0x1a8/0x250
[ 8.367536] [c902bf20] [c0004f68] kernel_init+0x30/0x150
[ 8.372785] [c902bf30] [c001322c] ret_from_kernel_thread+0x5c/0x64
[ 8.378898] Instruction dump:
[ 8.381821] 386348b4 7c0803a6 38210020 4e800020 9421ffe0 7c0802a6
93a10014 93c10018
[ 8.389737] 93e1001c 90010024 93810010 7cde3378 <83860004> 7c7d1b78
7c9f2378 480d344d
[ 8.397859] ---[ end trace 93333951fba49ac1 ]---
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH v1 0/5] Implement livepatch on PPC32
From: Steven Rostedt @ 2021-12-13 19:46 UTC (permalink / raw)
To: Christophe Leroy
Cc: Petr Mladek, Joe Lawrence, linux-s390@vger.kernel.org,
Jiri Kosina, linux-kernel@vger.kernel.org, Ingo Molnar,
Josh Poimboeuf, live-patching@vger.kernel.org, Naveen N . Rao,
Miroslav Benes, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <8df90f94-9939-0178-b92b-6ae6ea81784c@csgroup.eu>
On Mon, 13 Dec 2021 19:33:47 +0000
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> STill the same Oops, below
Unfortunately, I don't have a PPC machine (32 nor 64 bit) to help debug
this.
> I will look more closely tomorrow.
OK, thanks.
-- Steve
^ permalink raw reply
* Re: [PATCH] PCI/AER: potential dereference of null pointer
From: Bjorn Helgaas @ 2021-12-13 20:07 UTC (permalink / raw)
To: Rajat Jain
Cc: linux-pci, Jiasheng Jiang, linux-kernel, oohall, bhelgaas,
linuxppc-dev
In-Reply-To: <CACK8Z6EntHP0TWfq=JdQ7QS8vCgXoRUJOWqbGYLvD6R2Mq0paw@mail.gmail.com>
On Mon, Dec 13, 2021 at 11:57:52AM -0800, Rajat Jain wrote:
> Hello
>
> On Sat, Dec 11, 2021 at 9:54 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > [+cc Rajat, author of aer_stats:
> > db89ccbe52c7 ("PCI/AER: Define aer_stats structure for AER capable devices"
> > 81aa5206f9a7 ("PCI/AER: Add sysfs attributes to provide AER stats and breakdown"]
> >
> > On Thu, Dec 09, 2021 at 05:45:56PM +0800, Jiasheng Jiang wrote:
> > > he return value of kzalloc() needs to be checked.
> > > To avoid use of null pointer in case of the failure of alloc.
> > >
> > > Fixes: db89ccbe52c7 ("PCI/AER: Define aer_stats structure for AER capable devices")
> > > Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> > > ---
> > > drivers/pci/pcie/aer.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> > > index ec943cee5ecc..d04303edf468 100644
> > > --- a/drivers/pci/pcie/aer.c
> > > +++ b/drivers/pci/pcie/aer.c
> > > @@ -376,6 +376,8 @@ void pci_aer_init(struct pci_dev *dev)
> > > return;
> > >
> > > dev->aer_stats = kzalloc(sizeof(struct aer_stats), GFP_KERNEL);
> > > + if (!dev->aer_stats)
> > > + return;
> >
> > Did you actually trip over a null pointer dereference, and if so,
> > where was it?
> >
> > I think the intent here was that aer_stats is a non-essential feature,
> > and if we can't allocate space to keep the statistics, we can still
> > use the device without the stats.
>
> Ack. Yes, this was my intention.
>
> Practically speaking though, if a system cannot allocate space for
> aer_stats (608 bytes), I would think it is under an extreme memory
> pressure so not really sure if will be able to really do anything with
> the PCI device (probe the driver which may want to allocate more
> memory etc). But that isn't anything that we can help with.
>
> The current patch AFAICS looks like a no-op to me.
Well, it's a no-op except for the fact that we should continue on to
add a save buffer and clear the AER status, so if we applied this
patch, I think an alloc failure (unlikely though it is) would mean we
would continue using the device, but the AER init for it wouldn't be
done correctly.
I think we need to drop this patch, and if there is a null pointer
dereference elsewhere, fix that.
> > I *think* all the users of dev->aer_stats check for NULL before
> > dereferencing it, but if you found a case that doesn't do that, we
> > should definitely fix it.
> >
> > In a few cases (aer_stats_dev_attr, aer_stats_rootport_attr), the
> > check isn't obvious -- it happens in aer_stats_attrs_are_visible().
> > If aer_stats_attrs_are_visible() finds that aer_stats is NULL, those
> > sysfs attributes should not be visible, and the corresponding *_show()
> > functions should never be called.
> >
> > > /*
> > > * We save/restore PCI_ERR_UNCOR_MASK, PCI_ERR_UNCOR_SEVER,
> > > --
> > > 2.25.1
> > >
^ permalink raw reply
* Re: [PATCH 0/4] fsl-asoc-card: Add optional dt property for setting mclk-id
From: Rob Herring @ 2021-12-13 20:35 UTC (permalink / raw)
To: Ariel D'Alessandro
Cc: devicetree, alsa-devel, michael, kuninori.morimoto.gx, Xiubo.Lee,
tony, festevam, shengjiu.wang, tiwai, linux-kernel, lgirdwood,
nicoleotsuka, broonie, bcousson, perex, linux-omap, linuxppc-dev
In-Reply-To: <20211203134930.128703-1-ariel.dalessandro@collabora.com>
On Fri, Dec 03, 2021 at 10:49:26AM -0300, Ariel D'Alessandro wrote:
> This is a follow up of patchset:
>
> [RFC patch 0/5] Support BCLK input clock in tlv320aic31xx
Link?
> Sound cards may allow using different main clock inputs. In the generic
> fsl-asoc-card driver, these values are hardcoded for each specific card
> configuration.
>
> Let's make it more flexible, allowing setting mclk-id from the
> device-tree node.
>
> Ariel D'Alessandro (4):
> dt-bindings: sound: Rename tlv320aic31xx-micbias as tlv320aic31xx
> dt-bindings: tlv320aic31xx: Define PLL clock inputs
> ASoC: fsl-asoc-card: Add optional dt property for setting mclk-id
'mclk-id' is not documented.
> ASoC: fsl-asoc-card: Remove BCLK default value for tlv320aic31xx card
>
> .../devicetree/bindings/sound/fsl-asoc-card.txt | 1 +
> .../devicetree/bindings/sound/tlv320aic31xx.txt | 2 +-
> arch/arm/boot/dts/am43x-epos-evm.dts | 2 +-
> include/dt-bindings/sound/tlv320aic31xx-micbias.h | 9 ---------
> include/dt-bindings/sound/tlv320aic31xx.h | 14 ++++++++++++++
> sound/soc/codecs/tlv320aic31xx.c | 2 +-
> sound/soc/fsl/fsl-asoc-card.c | 7 ++++++-
> 7 files changed, 24 insertions(+), 13 deletions(-)
> delete mode 100644 include/dt-bindings/sound/tlv320aic31xx-micbias.h
> create mode 100644 include/dt-bindings/sound/tlv320aic31xx.h
>
> --
> 2.30.2
>
>
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: sound: Rename tlv320aic31xx-micbias as tlv320aic31xx
From: Rob Herring @ 2021-12-13 20:35 UTC (permalink / raw)
To: Ariel D'Alessandro
Cc: devicetree, alsa-devel, linuxppc-dev, kuninori.morimoto.gx,
Xiubo.Lee, tony, shengjiu.wang, tiwai, lgirdwood, robh+dt, perex,
nicoleotsuka, linux-omap, broonie, bcousson, michael, festevam,
linux-kernel
In-Reply-To: <20211203134930.128703-2-ariel.dalessandro@collabora.com>
On Fri, 03 Dec 2021 10:49:27 -0300, Ariel D'Alessandro wrote:
> Let's use a more generic name, so other definitions for tlv320aic31xx
> can be included.
>
> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> ---
> .../devicetree/bindings/sound/tlv320aic31xx.txt | 2 +-
> arch/arm/boot/dts/am43x-epos-evm.dts | 2 +-
> include/dt-bindings/sound/tlv320aic31xx-micbias.h | 9 ---------
> include/dt-bindings/sound/tlv320aic31xx.h | 9 +++++++++
> sound/soc/codecs/tlv320aic31xx.c | 2 +-
> 5 files changed, 12 insertions(+), 12 deletions(-)
> delete mode 100644 include/dt-bindings/sound/tlv320aic31xx-micbias.h
> create mode 100644 include/dt-bindings/sound/tlv320aic31xx.h
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 3/4] ASoC: fsl-asoc-card: Add optional dt property for setting mclk-id
From: Rob Herring @ 2021-12-13 20:37 UTC (permalink / raw)
To: Ariel D'Alessandro
Cc: devicetree, alsa-devel, michael, kuninori.morimoto.gx, Xiubo.Lee,
tony, festevam, shengjiu.wang, tiwai, linux-kernel, lgirdwood,
nicoleotsuka, broonie, bcousson, perex, linux-omap, linuxppc-dev
In-Reply-To: <20211203134930.128703-4-ariel.dalessandro@collabora.com>
On Fri, Dec 03, 2021 at 10:49:29AM -0300, Ariel D'Alessandro wrote:
> Sound cards may allow using different main clock inputs. In the generic
> fsl-asoc-card driver, these values are hardcoded for each specific card
> configuration.
>
> Let's make it more flexible, allowing setting mclk-id from the
> device-tree node. Otherwise, the default value for each card
> configuration is used.
>
> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> ---
> Documentation/devicetree/bindings/sound/fsl-asoc-card.txt | 1 +
> sound/soc/fsl/fsl-asoc-card.c | 6 ++++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
> index 23d83fa7609f..b219626a5403 100644
> --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
> +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
> @@ -82,6 +82,7 @@ Optional properties:
> - dai-format : audio format, for details see simple-card.yaml.
> - frame-inversion : dai-link uses frame clock inversion, for details see simple-card.yaml.
> - bitclock-inversion : dai-link uses bit clock inversion, for details see simple-card.yaml.
> + - mclk-id : main clock id, specific for each card configuration.
Ahh, there it is. This change and the header defines should be 1 patch
and driver changes another.
>
> Optional unless SSI is selected as a CPU DAI:
>
> diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
> index 90cbed496f98..bb962e04d40b 100644
> --- a/sound/soc/fsl/fsl-asoc-card.c
> +++ b/sound/soc/fsl/fsl-asoc-card.c
> @@ -693,6 +693,12 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
> goto asrc_fail;
> }
>
> + /*
> + * Allow setting mclk-id from the device-tree node. Otherwise, the
> + * default value for each card configuration is used.
> + */
> + of_property_read_u32(np, "mclk-id", &priv->codec_priv.mclk_id);
> +
> /* Format info from DT is optional. */
> snd_soc_daifmt_parse_clock_provider_as_phandle(np, NULL, &bitclkprovider, &frameprovider);
> if (bitclkprovider || frameprovider) {
> --
> 2.30.2
>
>
^ permalink raw reply
* Re: [PATCH 0/4] fsl-asoc-card: Add optional dt property for setting mclk-id
From: Ariel D'Alessandro @ 2021-12-13 22:43 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, alsa-devel, michael, kuninori.morimoto.gx, Xiubo.Lee,
tony, festevam, shengjiu.wang, tiwai, linux-kernel, lgirdwood,
nicoleotsuka, broonie, bcousson, perex, linux-omap, linuxppc-dev
In-Reply-To: <YbeukcwXQueEquJZ@robh.at.kernel.org>
Hi Rob,
Thanks for the review.
On 12/13/21 5:35 PM, Rob Herring wrote:
> On Fri, Dec 03, 2021 at 10:49:26AM -0300, Ariel D'Alessandro wrote:
>> This is a follow up of patchset:
>>
>> [RFC patch 0/5] Support BCLK input clock in tlv320aic31xx
>
> Link?
Link to the datasheet?
http://www.ti.com/lit/ds/symlink/tlv320aic3100.pdf
>
>> Sound cards may allow using different main clock inputs. In the generic
>> fsl-asoc-card driver, these values are hardcoded for each specific card
>> configuration.
>>
>> Let's make it more flexible, allowing setting mclk-id from the
>> device-tree node.
>>
>> Ariel D'Alessandro (4):
>> dt-bindings: sound: Rename tlv320aic31xx-micbias as tlv320aic31xx
>> dt-bindings: tlv320aic31xx: Define PLL clock inputs
>> ASoC: fsl-asoc-card: Add optional dt property for setting mclk-id
>
> 'mclk-id' is not documented.
>
>> ASoC: fsl-asoc-card: Remove BCLK default value for tlv320aic31xx card
>>
>> .../devicetree/bindings/sound/fsl-asoc-card.txt | 1 +
>> .../devicetree/bindings/sound/tlv320aic31xx.txt | 2 +-
>> arch/arm/boot/dts/am43x-epos-evm.dts | 2 +-
>> include/dt-bindings/sound/tlv320aic31xx-micbias.h | 9 ---------
>> include/dt-bindings/sound/tlv320aic31xx.h | 14 ++++++++++++++
>> sound/soc/codecs/tlv320aic31xx.c | 2 +-
>> sound/soc/fsl/fsl-asoc-card.c | 7 ++++++-
>> 7 files changed, 24 insertions(+), 13 deletions(-)
>> delete mode 100644 include/dt-bindings/sound/tlv320aic31xx-micbias.h
>> create mode 100644 include/dt-bindings/sound/tlv320aic31xx.h
>>
>> --
>> 2.30.2
Regards,
Ariel
^ permalink raw reply
* Re: [PATCH 3/4] ASoC: fsl-asoc-card: Add optional dt property for setting mclk-id
From: Ariel D'Alessandro @ 2021-12-13 22:44 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, alsa-devel, michael, kuninori.morimoto.gx, Xiubo.Lee,
tony, festevam, shengjiu.wang, tiwai, linux-kernel, lgirdwood,
nicoleotsuka, broonie, bcousson, perex, linux-omap, linuxppc-dev
In-Reply-To: <Ybeu/1vsUzvAg2Ya@robh.at.kernel.org>
Hi Rob,
On 12/13/21 5:37 PM, Rob Herring wrote:
> On Fri, Dec 03, 2021 at 10:49:29AM -0300, Ariel D'Alessandro wrote:
>> Sound cards may allow using different main clock inputs. In the generic
>> fsl-asoc-card driver, these values are hardcoded for each specific card
>> configuration.
>>
>> Let's make it more flexible, allowing setting mclk-id from the
>> device-tree node. Otherwise, the default value for each card
>> configuration is used.
>>
>> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
>> ---
>> Documentation/devicetree/bindings/sound/fsl-asoc-card.txt | 1 +
>> sound/soc/fsl/fsl-asoc-card.c | 6 ++++++
>> 2 files changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
>> index 23d83fa7609f..b219626a5403 100644
>> --- a/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
>> +++ b/Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
>> @@ -82,6 +82,7 @@ Optional properties:
>> - dai-format : audio format, for details see simple-card.yaml.
>> - frame-inversion : dai-link uses frame clock inversion, for details see simple-card.yaml.
>> - bitclock-inversion : dai-link uses bit clock inversion, for details see simple-card.yaml.
>> + - mclk-id : main clock id, specific for each card configuration.
>
> Ahh, there it is. This change and the header defines should be 1 patch
> and driver changes another.
Ah, I see. I'll split it and send a patchset v2 for this.
Thanks,
Ariel
^ permalink raw reply
* Re: [PATCH] PCI/AER: potential dereference of null pointer
From: Rajat Jain @ 2021-12-13 19:57 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Jiasheng Jiang, linux-kernel, oohall, bhelgaas,
linuxppc-dev
In-Reply-To: <20211211175450.GA398271@bhelgaas>
Hello
On Sat, Dec 11, 2021 at 9:54 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> [+cc Rajat, author of aer_stats:
> db89ccbe52c7 ("PCI/AER: Define aer_stats structure for AER capable devices"
> 81aa5206f9a7 ("PCI/AER: Add sysfs attributes to provide AER stats and breakdown"]
>
> On Thu, Dec 09, 2021 at 05:45:56PM +0800, Jiasheng Jiang wrote:
> > he return value of kzalloc() needs to be checked.
> > To avoid use of null pointer in case of the failure of alloc.
> >
> > Fixes: db89ccbe52c7 ("PCI/AER: Define aer_stats structure for AER capable devices")
> > Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> > ---
> > drivers/pci/pcie/aer.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> > index ec943cee5ecc..d04303edf468 100644
> > --- a/drivers/pci/pcie/aer.c
> > +++ b/drivers/pci/pcie/aer.c
> > @@ -376,6 +376,8 @@ void pci_aer_init(struct pci_dev *dev)
> > return;
> >
> > dev->aer_stats = kzalloc(sizeof(struct aer_stats), GFP_KERNEL);
> > + if (!dev->aer_stats)
> > + return;
>
> Did you actually trip over a null pointer dereference, and if so,
> where was it?
>
> I think the intent here was that aer_stats is a non-essential feature,
> and if we can't allocate space to keep the statistics, we can still
> use the device without the stats.
Ack. Yes, this was my intention.
Practically speaking though, if a system cannot allocate space for
aer_stats (608 bytes), I would think it is under an extreme memory
pressure so not really sure if will be able to really do anything with
the PCI device (probe the driver which may want to allocate more
memory etc). But that isn't anything that we can help with.
The current patch AFAICS looks like a no-op to me.
Thanks,
Rajat
>
> I *think* all the users of dev->aer_stats check for NULL before
> dereferencing it, but if you found a case that doesn't do that, we
> should definitely fix it.
>
> In a few cases (aer_stats_dev_attr, aer_stats_rootport_attr), the
> check isn't obvious -- it happens in aer_stats_attrs_are_visible().
> If aer_stats_attrs_are_visible() finds that aer_stats is NULL, those
> sysfs attributes should not be visible, and the corresponding *_show()
> functions should never be called.
>
> > /*
> > * We save/restore PCI_ERR_UNCOR_MASK, PCI_ERR_UNCOR_SEVER,
> > --
> > 2.25.1
> >
^ permalink raw reply
* [Bug 215217] Kernel fails to boot at an early stage when built with GCC_PLUGIN_LATENT_ENTROPY=y (PowerMac G4 3,6)
From: bugzilla-daemon @ 2021-12-14 0:45 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-215217-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=215217
--- Comment #10 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 300015
--> https://bugzilla.kernel.org/attachment.cgi?id=300015&action=edit
kernel zImage (5.16-rc5 + CFLAGS_setup_32.o/early_32.o +=... , PowerMac G4 DP)
Unfortunately still no success.
Relevant section in the Makefile now looks like this:
CFLAGS_early_32.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_setup_32.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
I'll attach the generated zImage, maybe you can make something out of it.
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH]selftests/powerpc: skip tests for unavailable mitigations.
From: Nageswara Sastry @ 2021-12-14 4:27 UTC (permalink / raw)
To: Sachin Sant, linuxppc-dev
In-Reply-To: <163941374362.36967.18016981579099073379.sendpatchset@1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa>
On 13/12/21 10:12 pm, Sachin Sant wrote:
> Mitigation patching test iterates over a set of mitigations irrespective
> of whether a certain mitigation is supported/available in the kernel.
> This causes following messages on a kernel where some mitigations
> are unavailable:
>
> Spawned threads enabling/disabling mitigations ...
> cat: entry_flush: No such file or directory
> cat: uaccess_flush: No such file or directory
> Waiting for timeout ...
> OK
>
> This patch adds a check for available mitigations in the kernel.
>
> Reported-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> Signed-off-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> ---
> diff -Naurp aa/tools/testing/selftests/powerpc/security/mitigation-patching.sh bb/tools/testing/selftests/powerpc/security/mitigation-patching.sh
> --- aa/tools/testing/selftests/powerpc/security/mitigation-patching.sh 2021-12-13 10:17:05.714127154 -0500
> +++ bb/tools/testing/selftests/powerpc/security/mitigation-patching.sh 2021-12-13 10:19:32.575315913 -0500
> @@ -44,7 +44,10 @@ mitigations="barrier_nospec stf_barrier
>
> for m in $mitigations
> do
> - do_one "$m" &
> + if [[ -f /sys/kernel/debug/powerpc/$m ]]
> + then
> + do_one "$m" &
> + fi
> done
>
> echo "Spawned threads enabling/disabling mitigations ..."
--
Thanks and Regards
R.Nageswara Sastry
^ permalink raw reply
* [Bug 215217] Kernel fails to boot at an early stage when built with GCC_PLUGIN_LATENT_ENTROPY=y (PowerMac G4 3,6)
From: bugzilla-daemon @ 2021-12-14 5:41 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-215217-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=215217
--- Comment #11 from Christophe Leroy (christophe.leroy@csgroup.eu) ---
Ok, so that's not enough, must be something else.
I guess you are right next step should be analysis of the image.
zImage however can hardly be used for that.
Could you provide vmlinux file ?
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v1 0/5] Implement livepatch on PPC32
From: Christophe Leroy @ 2021-12-14 6:09 UTC (permalink / raw)
To: Steven Rostedt
Cc: Petr Mladek, Joe Lawrence, linux-s390@vger.kernel.org,
Jiri Kosina, linux-kernel@vger.kernel.org, Ingo Molnar,
Josh Poimboeuf, live-patching@vger.kernel.org, Naveen N . Rao,
Miroslav Benes, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20211213144603.47d7c908@gandalf.local.home>
Le 13/12/2021 à 20:46, Steven Rostedt a écrit :
> On Mon, 13 Dec 2021 19:33:47 +0000
> Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>
>> STill the same Oops, below
>
> Unfortunately, I don't have a PPC machine (32 nor 64 bit) to help debug
> this.
>
>
>> I will look more closely tomorrow.
>
> OK, thanks.
>
The Oops was due to ftrace_caller() setting the regs argument to NULL.
After fixing that, I'm back into a situation where I get "Testing tracer
function_graph: FAILED!"
Will continue investigating.
Christophe
^ permalink raw reply
* Re: [BUG] mtd: cfi_cmdset_0002: write regression since v4.17-rc1
From: Thorsten Leemhuis @ 2021-12-14 7:23 UTC (permalink / raw)
To: Ahmad Fatoum, linux-mtd, ikegami, Joakim.Tjernlund, miquel.raynal,
vigneshr, richard, ikegami.t, regressions@lists.linux.dev
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, marek.vasut,
Chris Packham, Pengutronix Kernel Team, cyrille.pitchen,
Shaohui.Xie, Brian Norris, David Woodhouse
In-Reply-To: <b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de>
[TLDR: adding this regression to regzbot; most of this mail is compiled
from a few templates paragraphs some of you might have seen already.]
Hi, this is your Linux kernel regression tracker speaking.
Top-posting for once, to make this easy accessible to everyone.
Thanks for the report.
Adding the regression mailing list to the list of recipients, as it
should be in the loop for all regressions, as explained here:
https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html
To be sure this issue doesn't fall through the cracks unnoticed, I'm
adding it to regzbot, my Linux kernel regression tracking bot:
#regzbot ^introduced dfeae1073583
#regzbot title mtd: cfi_cmdset_0002: flash write accesses on the
hardware fail on a PowerPC MPC8313 to a 8-bit-parallel S29GL064N flash
#regzbot ignore-activity
Reminder: when fixing the issue, please add a 'Link:' tag with the URL
to the report (the parent of this mail), then regzbot will automatically
mark the regression as resolved once the fix lands in the appropriate
tree. For more details about regzbot see footer.
Sending this to everyone that got the initial report, to make all aware
of the tracking. I also hope that messages like this motivate people to
directly get at least the regression mailing list and ideally even
regzbot involved when dealing with regressions, as messages like this
wouldn't be needed then.
Don't worry, I'll send further messages wrt to this regression just to
the lists (with a tag in the subject so people can filter them away), as
long as they are intended just for regzbot. With a bit of luck no such
messages will be needed anyway.
Ciao, Thorsten (wearing his 'Linux kernel regression tracker' hat).
P.S.: As a Linux kernel regression tracker I'm getting a lot of reports
on my table. I can only look briefly into most of them. Unfortunately
therefore I sometimes will get things wrong or miss something important.
I hope that's not the case here; if you think it is, don't hesitate to
tell me about it in a public reply. That's in everyone's interest, as
what I wrote above might be misleading to everyone reading this; any
suggestion I gave thus might sent someone reading this down the wrong
rabbit hole, which none of us wants.
BTW, I have no personal interest in this issue, which is tracked using
regzbot, my Linux kernel regression tracking bot
(https://linux-regtracking.leemhuis.info/regzbot/). I'm only posting
this mail to get things rolling again and hence don't need to be CC on
all further activities wrt to this regression.
On 13.12.21 14:24, Ahmad Fatoum wrote:
> Hi,
>
> I've been investigating a breakage on a PowerPC MPC8313: The SoC is connected
> via the "Enhanced Local Bus Controller" to a 8-bit-parallel S29GL064N flash,
> which is represented as a memory-mapped cfi-flash.
>
> The regression began in v4.17-rc1 with
>
> dfeae1073583 ("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
>
> and causes all flash write accesses on the hardware to fail. Example output
> after v5.1-rc2[1]:
>
> root@host:~# mount -t jffs2 /dev/mtdblock0 /mnt
> MTD do_write_buffer_wait(): software timeout, address:0x000c000b.
> jffs2: Write clean marker to block at 0x000c0000 failed: -5
>
> This issue still persists with v5.16-rc. Reverting aforementioned patch fixes
> it, but I am still looking for a change that keeps both Tokunori's and my
> hardware happy.
>
> What Tokunori's patch did is that it strengthened the success condition
> for flash writes:
>
> - Prior to the patch, DQ polling was done until bits
> stopped toggling. This was taken as an indicator that the write succeeded
> and was reported up the stack. i.e. success condition is chip_ready()
>
> - After the patch, polling continues until the just written data is
> actually read back, i.e. success condition is chip_good()
>
> This new condition never holds for me, when DQ stabilizes, it reads 0xFF,
> never the just written data. The data is still written and can be read back
> on subsequent reads, just not at that point of time in the poll loop.
>
> We haven't had write issues for the years predating that patch. As the
> regression has been mainline for a while, I am wondering what about my setup
> that makes it pop up here, but not elsewhere?
>
> I consulted the data sheet[2] and found Figure 27, which describes DQ polling
> during embedded algorithms. DQ switches from status output to "True" (I assume
> True == all bits set == 0xFF) until CS# is reasserted.
>
> I compared with another chip's datasheet, and it (Figure 8.4) doesn't describe
> such an intermittent "True" state. In any case, the driver polls a few hundred
> times, however, before giving up, so there should be enough CS# toggles.
>
>
> Locally, I'll revert this patch for now. I think accepting 0xFF as a success
> condition may be appropriate, but I don't yet have the rationale to back it up.
>
> I am investigating this some more, probably with a logic trace, but I wanted
> to report this in case someone has pointers and in case other people run into
> the same issue.
>
>
> Cheers,
> Ahmad
>
> [1] Prior to d9b8a67b3b95 ("mtd: cfi: fix deadloop in cfi_cmdset_0002.c do_write_buffer")
> first included with v5.1-rc2, failing writes just hung indefinitely in kernel space.
> That's fixed, but the writes still fail.
>
> [2]: 001-98525 Rev. *B, https://www.infineon.com/dgdl/Infineon-S29GL064N_S29GL032N_64_Mbit_32_Mbit_3_V_Page_Mode_MirrorBit_Flash-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ed556fd548b
>
> [3]: https://www.mouser.com/datasheet/2/268/SST39VF1601C-SST39VF1602C-16-Mbit-x16-Multi-Purpos-709008.pdf
> Note that "true data" means valid data here, not all bits one.
>
^ permalink raw reply
* Re: [PATCH v1 0/5] Implement livepatch on PPC32
From: Christophe Leroy @ 2021-12-14 7:35 UTC (permalink / raw)
To: Steven Rostedt
Cc: Petr Mladek, Joe Lawrence, linux-s390@vger.kernel.org,
Jiri Kosina, linux-kernel@vger.kernel.org, Ingo Molnar,
Josh Poimboeuf, live-patching@vger.kernel.org, Naveen N . Rao,
Miroslav Benes, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <76ce2dd7-691e-df73-727c-110713c07cda@csgroup.eu>
Le 14/12/2021 à 07:09, Christophe Leroy a écrit :
>
>
> Le 13/12/2021 à 20:46, Steven Rostedt a écrit :
>> On Mon, 13 Dec 2021 19:33:47 +0000
>> Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>>
>>> STill the same Oops, below
>>
>> Unfortunately, I don't have a PPC machine (32 nor 64 bit) to help debug
>> this.
>>
>>
>>> I will look more closely tomorrow.
>>
>> OK, thanks.
>>
>
> The Oops was due to ftrace_caller() setting the regs argument to NULL.
>
> After fixing that, I'm back into a situation where I get "Testing tracer
> function_graph: FAILED!"
>
> Will continue investigating.
>
trace_selftest_startup_function_graph() calls register_ftrace_direct()
which returns -ENOSUPP because powerpc doesn't select
CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS.
Should TEST_DIRECT_TRAMP depend on CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS ?
Christophe
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3S HV P9: Use kvm_arch_vcpu_get_wait() to get rcuwait object
From: Sachin Sant @ 2021-12-14 8:07 UTC (permalink / raw)
To: Sean Christopherson
Cc: linuxppc-dev, linux-kernel, Nicholas Piggin, Paolo Bonzini
In-Reply-To: <20211213174556.3871157-1-seanjc@google.com>
> On 13-Dec-2021, at 11:15 PM, Sean Christopherson <seanjc@google.com> wrote:
>
> Use kvm_arch_vcpu_get_wait() to get a vCPU's rcuwait object instead of
> using vcpu->wait directly in kvmhv_run_single_vcpu(). Functionally, this
> is a nop as vcpu->arch.waitp is guaranteed to point at vcpu->wait. But
> that is not obvious at first glance, and a future change coming in via
> the KVM tree, commit 510958e99721 ("KVM: Force PPC to define its own
> rcuwait object"), will hide vcpu->wait from architectures that define
> __KVM_HAVE_ARCH_WQP to prevent generic KVM from attepting to wake a vCPU
> with the wrong rcuwait object.
>
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> —
>
Thanks for the patch.
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
^ permalink raw reply
* Re: [patch V3 00/35] genirq/msi, PCI/MSI: Spring cleaning - Part 2
From: Thomas Gleixner @ 2021-12-14 9:41 UTC (permalink / raw)
To: Nishanth Menon
Cc: Mark Rutland, Stuart Yoder, Will Deacon, Ashok Raj, Joerg Roedel,
Jassi Brar, Sinan Kaya, iommu, Peter Ujfalusi, Bjorn Helgaas,
linux-arm-kernel, Jason Gunthorpe, linux-pci, xen-devel,
Kevin Tian, Arnd Bergmann, Robin Murphy, Johannes Berg,
Alex Williamson, Cedric Le Goater, Santosh Shilimkar,
Bjorn Helgaas, Megha Dey, Laurentiu Tudor, Juergen Gross,
Tero Kristo, Greg Kroah-Hartman, linux-wireless, LKML, Vinod Koul,
Marc Zygnier, dmaengine, linuxppc-dev
In-Reply-To: <20211213182958.ytj4m6gsg35u77cv@detonator>
On Mon, Dec 13 2021 at 12:29, Nishanth Menon wrote:
> On 23:18-20211210, Thomas Gleixner wrote:
> Also while testing on TI K3 platforms, I noticed:
>
> msi_device_data_release/msi_device_destroy_sysfs in am64xx-evm / j7200
The warning complains about a device being released with MSI descriptors
still attached to the device. This was added by:
5b012cede0f7 ("device: Add device::msi_data pointer and struct msi_device_data")
That's not a regression caused by this commit. The warning is just
exposing an already existing problem in the iwlwifi driver, which seems
to do:
probe()
setup_pci_msi[x]_interrupts()
start_drv()
if (try_to_load_firmware() == FAIL)
device_release_driver()
...
msi_device_data_release()
WARN()
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH v2 0/2] kdump: simplify code
From: Tiezhu Yang @ 2021-12-14 10:03 UTC (permalink / raw)
To: Matthew Wilcox, David Laight
Cc: linux-ia64@vger.kernel.org, Baoquan He, linux-sh@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, x86@kernel.org,
kexec@lists.infradead.org, linux-mips@vger.kernel.org,
linux-kernel@vger.kernel.org, Vivek Goyal, Xuefeng Li,
linux-fsdevel@vger.kernel.org, Andrew Morton, Dave Young,
linux-riscv@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <YbdcKK3Cq6ITTg/l@casper.infradead.org>
On 12/13/2021 10:43 PM, Matthew Wilcox wrote:
> On Mon, Dec 13, 2021 at 08:30:33AM +0000, David Laight wrote:
>> From: Matthew Wilcox
>>> Sent: 12 December 2021 11:48
>>>
>>> On Sat, Dec 11, 2021 at 05:53:46PM +0000, David Laight wrote:
>>>> From: Tiezhu Yang
>>>>> Sent: 11 December 2021 03:33
>>>>>
>>>>> v2:
>>>>> -- add copy_to_user_or_kernel() in lib/usercopy.c
>>>>> -- define userbuf as bool type
>>>>
>>>> Instead of having a flag to indicate whether the buffer is user or kernel,
>>>> would it be better to have two separate buffer pointers.
>>>> One for a user space buffer, the other for a kernel space buffer.
>>>> Exactly one of the buffers should always be NULL.
>>>
>>> No. You should be using an iov_iter instead. See
>>> https://lore.kernel.org/all/Ya4bdB0UBJCZhUSo@casper.infradead.org/
>>> for a start on this.
>>
>> iov_iter gets horribly expensive...
>
> Oh, right. Reading the kcore is a high-performance path, my mistake.
>
Hi,
Thank you for your discussions.
The intention of this patchset is to simplify the related code with no
functional changes and no side effects.
At this moment, if you are OK, I will send v3 used with inline function
copy_to_user_or_kernel() to keep it simple, maybe other more changes can
be done in the future if no any side effect.
The v3 will contain the following three patches to make the changes
more clear:
kdump: vmcore: remove copy_to() and add copy_to_user_or_kernel()
kdump: crashdump: use copy_to_user_or_kernel() to simplify code
kdump: vmcore: crashdump: make variable type of userbuf as bool
^ permalink raw reply
* Re: [PATCH v2] of/fdt: Rework early_init_dt_scan_memory() to call directly
From: Michael Ellerman @ 2021-12-14 11:18 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Thomas Bogendoerfer, linuxppc-dev, open list:MIPS,
linux-kernel@vger.kernel.org, Paul Mackerras, John Crispin,
Frank Rowand
In-Reply-To: <CAL_JsqLpq7fx0pyQiJFa0P5C3JXijiVe_fr84x9RML1aDJ7vDQ@mail.gmail.com>
Rob Herring <robh@kernel.org> writes:
> On Mon, Dec 13, 2021 at 6:47 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Rob Herring <robh@kernel.org> writes:
>> > Use of the of_scan_flat_dt() function predates libfdt and is discouraged
>> > as libfdt provides a nicer set of APIs. Rework
>> > early_init_dt_scan_memory() to be called directly and use libfdt.
>> ...
>> > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> > index 6e1a106f02eb..63762a3b75e8 100644
>> > --- a/arch/powerpc/kernel/prom.c
>> > +++ b/arch/powerpc/kernel/prom.c
>> > @@ -532,19 +532,19 @@ static int __init early_init_drmem_lmb(struct drmem_lmb *lmb,
>> > }
>> > #endif /* CONFIG_PPC_PSERIES */
>> >
>> > -static int __init early_init_dt_scan_memory_ppc(unsigned long node,
>> > - const char *uname,
>> > - int depth, void *data)
>> > +static int __init early_init_dt_scan_memory_ppc(void)
>> > {
>> > #ifdef CONFIG_PPC_PSERIES
>> > - if (depth == 1 &&
>> > - strcmp(uname, "ibm,dynamic-reconfiguration-memory") == 0) {
>> > + const void *fdt = initial_boot_params;
>> > + int node = fdt_path_offset(fdt, "/ibm,dynamic-reconfiguration-memory");
>> > +
>> > + if (node > 0) {
>> > walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb);
>> > return 0;
>> > }
It's that return that is the problem.
Now that early_init_dt_scan_memory_ppc() is only called once, that
return causes us to skip scanning regular memory nodes if there is an
"ibm,dynamic-reconfiguration-memory" property present.
So the fix is just:
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 1098de3b172f..125661e5fcf3 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -538,10 +538,8 @@ static int __init early_init_dt_scan_memory_ppc(void)
const void *fdt = initial_boot_params;
int node = fdt_path_offset(fdt, "/ibm,dynamic-reconfiguration-memory");
- if (node > 0) {
+ if (node > 0)
walk_drmem_lmbs_early(node, NULL, early_init_drmem_lmb);
- return 0;
- }
#endif
return early_init_dt_scan_memory();
> The only thing I see is now there is an assumption that 'memory' nodes
> are off the root node only. Before they could be anywhere.
I don't know of any machines where that would be a problem. But given
all the wild and wonderful device trees out there, who really knows :)
Maybe we should continue to allow memory nodes to be anywhere, and print
a warning for any that aren't at the root. Then if no one reports any
hits for the warning we could switch to only allowing them at the root?
cheers
> index a835c458f50a..97d7607625ec 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -1083,16 +1083,13 @@ int __init early_init_dt_scan_memory(void)
> int node;
> const void *fdt = initial_boot_params;
>
> - fdt_for_each_subnode(node, fdt, 0) {
> - const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> + for (node = fdt_node_offset_by_prop_value(fdt, -1, "device_type", "memory", 6);
> + node != -FDT_ERR_NOTFOUND;
> + node = fdt_node_offset_by_prop_value(fdt, node, "device_type", "memory", 6)) {
> const __be32 *reg, *endp;
> int l;
> bool hotpluggable;
>
> - /* We are scanning "memory" nodes only */
> - if (type == NULL || strcmp(type, "memory") != 0)
> - continue;
> -
> reg = of_get_flat_dt_prop(node, "linux,usable-memory", &l);
> if (reg == NULL)
> reg = of_get_flat_dt_prop(node, "reg", &l);
>
> Rob
^ permalink raw reply related
* [PATCH v2] powerpc/module_64: Fix livepatching for RO modules
From: Michael Ellerman @ 2021-12-14 12:12 UTC (permalink / raw)
To: linuxppc-dev
From: Russell Currey <ruscur@russell.cc>
Livepatching a loaded module involves applying relocations through
apply_relocate_add(), which attempts to write to read-only memory when
CONFIG_STRICT_MODULE_RWX=y. Work around this by performing these
writes through the text poke area by using patch_instruction().
R_PPC_REL24 is the only relocation type generated by the kpatch-build
userspace tool or klp-convert kernel tree that I observed applying a
relocation to a post-init module.
A more comprehensive solution is planned, but using patch_instruction()
for R_PPC_REL24 on should serve as a sufficient fix.
This does have a performance impact, I observed ~15% overhead in
module_load() on POWER8 bare metal with checksum verification off.
Fixes: c35717c71e98 ("powerpc: Set ARCH_HAS_STRICT_MODULE_RWX")
Cc: stable@vger.kernel.org # v5.14+
Reported-by: Joe Lawrence <joe.lawrence@redhat.com>
Signed-off-by: Russell Currey <ruscur@russell.cc>
Tested-by: Joe Lawrence <joe.lawrence@redhat.com>
[mpe: Check return codes from patch_instruction()]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/module_64.c | 42 ++++++++++++++++++++++++++-------
1 file changed, 34 insertions(+), 8 deletions(-)
v2: mpe: Check return codes from patch_instruction()
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 6baa676e7cb6..5d77d3f5fbb5 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -422,11 +422,17 @@ static inline int create_stub(const Elf64_Shdr *sechdrs,
const char *name)
{
long reladdr;
+ func_desc_t desc;
+ int i;
if (is_mprofile_ftrace_call(name))
return create_ftrace_stub(entry, addr, me);
- memcpy(entry->jump, ppc64_stub_insns, sizeof(ppc64_stub_insns));
+ for (i = 0; i < sizeof(ppc64_stub_insns) / sizeof(u32); i++) {
+ if (patch_instruction(&entry->jump[i],
+ ppc_inst(ppc64_stub_insns[i])))
+ return 0;
+ }
/* Stub uses address relative to r2. */
reladdr = (unsigned long)entry - my_r2(sechdrs, me);
@@ -437,10 +443,24 @@ static inline int create_stub(const Elf64_Shdr *sechdrs,
}
pr_debug("Stub %p get data from reladdr %li\n", entry, reladdr);
- entry->jump[0] |= PPC_HA(reladdr);
- entry->jump[1] |= PPC_LO(reladdr);
- entry->funcdata = func_desc(addr);
- entry->magic = STUB_MAGIC;
+ if (patch_instruction(&entry->jump[0],
+ ppc_inst(entry->jump[0] | PPC_HA(reladdr))))
+ return 0;
+
+ if (patch_instruction(&entry->jump[1],
+ ppc_inst(entry->jump[1] | PPC_LO(reladdr))))
+ return 0;
+
+ // func_desc_t is 8 bytes if ABIv2, else 16 bytes
+ desc = func_desc(addr);
+ for (i = 0; i < sizeof(func_desc_t) / sizeof(u32); i++) {
+ if (patch_instruction(((u32 *)&entry->funcdata) + i,
+ ppc_inst(((u32 *)(&desc))[i])))
+ return 0;
+ }
+
+ if (patch_instruction(&entry->magic, ppc_inst(STUB_MAGIC)))
+ return 0;
return 1;
}
@@ -495,8 +515,11 @@ static int restore_r2(const char *name, u32 *instruction, struct module *me)
me->name, *instruction, instruction);
return 0;
}
+
/* ld r2,R2_STACK_OFFSET(r1) */
- *instruction = PPC_INST_LD_TOC;
+ if (patch_instruction(instruction, ppc_inst(PPC_INST_LD_TOC)))
+ return 0;
+
return 1;
}
@@ -636,9 +659,12 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
}
/* Only replace bits 2 through 26 */
- *(uint32_t *)location
- = (*(uint32_t *)location & ~0x03fffffc)
+ value = (*(uint32_t *)location & ~0x03fffffc)
| (value & 0x03fffffc);
+
+ if (patch_instruction((u32 *)location, ppc_inst(value)))
+ return -EFAULT;
+
break;
case R_PPC64_REL64:
--
2.31.1
^ permalink raw reply related
* Re: ppc64le STRICT_MODULE_RWX and livepatch apply_relocate_add() crashes
From: Christophe Leroy @ 2021-12-14 12:44 UTC (permalink / raw)
To: Joe Lawrence, Russell Currey, live-patching@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
Cc: Peter Zijlstra, Jordan Niethe, Jessica Yu, Josh Poimboeuf
In-Reply-To: <8a68ffef-7e0d-b1ff-1102-2e6f2c999455@redhat.com>
Le 13/12/2021 à 18:26, Joe Lawrence a écrit :
> On 12/13/21 11:36 AM, Christophe Leroy wrote:
>>
>>
>> Le 13/12/2021 à 15:47, Joe Lawrence a écrit :
>>> On 12/13/21 2:42 AM, Christophe Leroy wrote:
>>>>
>>>> Hello Joe,
>>>>
>>>> I'm implementing LIVEPATCH on PPC32 and I wanted to test with
>>>> STRICT_MODULE_RWX enabled so I took your branch as suggested, but I'm
>>>> getting the following errors on build. What shall I do ?
>>>>
>>>> CALL scripts/checksyscalls.sh
>>>> CALL scripts/atomic/check-atomics.sh
>>>> CHK include/generated/compile.h
>>>> KLP lib/livepatch/test_klp_convert1.ko
>>>> klp-convert: section .rela.klp.module_relocs.test_klp_convert_mod length
>>>> beyond nr_entries
>>>>
>>>> klp-convert: Unable to load user-provided sympos
>>>> make[2]: *** [scripts/Makefile.modfinal:79:
>>>> lib/livepatch/test_klp_convert1.ko] Error 255
>>>> KLP lib/livepatch/test_klp_convert2.ko
>>>> klp-convert: section .rela.klp.module_relocs.test_klp_convert_mod length
>>>> beyond nr_entries
>>>>
>>>> klp-convert: Unable to load user-provided sympos
>>>> make[2]: *** [scripts/Makefile.modfinal:79:
>>>> lib/livepatch/test_klp_convert2.ko] Error 255
>>>> KLP lib/livepatch/test_klp_convert_sections.ko
>>>> klp-convert: section .rela.klp.module_relocs.test_klp_convert_mod length
>>>> beyond nr_entries
>>>>
>>>> klp-convert: Unable to load user-provided sympos
>>>> make[2]: *** [scripts/Makefile.modfinal:79:
>>>> lib/livepatch/test_klp_convert_sections.ko] Error 255
>>>> make[2]: Target '__modfinal' not remade because of errors.
>>>> make[1]: *** [scripts/Makefile.modpost:145: __modpost] Error 2
>>>> make: *** [Makefile:1770: modules] Error 2
>>>>
>>>
>>> Hi Christophe,
>>>
>>> Interesting failure mode. That's klp-convert complaining that it found
>>> more relocations in a .klp.module_relocs.<objname> section than
>>> expected, i.e. nr_entries = sec->size / sizeof(struct klp_module_reloc).
>>>
>>> A few possibilities: the ELF sec->size was incorrectly set/read by
>>> build/libelf (I doubt that). Or maybe the layout/size of struct
>>> klp_module_reloc is not consistent between kernel and userspace (I'm
>>> more suspicious of this).
>>>
>>> Can you post a copy of the build's symbols.klp and
>>> lib/livepatch/test_klp_convert1.tmp.ko somewhere? I should be able to
>>> start debug with those files.
>>>
>>
>> I sent you both files off list.
>>
>> It looks like klp-convert doesn't use the correct size. It finds a
>> struct of size 12 hence 3 entries for a section of size 40.
>>
>> On PPC32 the struct has size 8 (void * is 4 and int is 4).
>>
>> But I'm cross-building from x86_64 where the struct is 8 + 4 = 12.
>>
>> Can it be the reason ?
>>
>
> I'm pretty sure that is it. I haven't had much runtime with klp-convert
> and cross-building (I've only found one big/little endian bug with
> x86_64->s390x) and was going to ask you how you were testing :)
>
> Do you know if there are other kernel build tools that deal with similar
> situations? This seems like a tricky job for the userspace build tool
> to determine non-native target struct layout.
>
> In the meantime, hacking in:
>
> struct klp_module_reloc {
> - void *sym;
> + uint32_t sym;
> unsigned int sympos;
> } __packed;
>
> gets me generating an output .ko file, but the readelf output doesn't
> look right.
>
> I'll add this to the patchset TODO list, but may not get to it for a
> while -- is there any chance the above hack works or could you test a
> local non-cross build?
>
No I have no way to do a non-cross build. My target is an embedded board
with slow CPU and little memory.
I tested with your hack, I get:
root@vgoip:~# insmod /lib/modules/test_klp_convert1.ko
insmod: can't insert '/lib/modules/test_klp_convert1.ko': unknown symbol
in module, or unknown parameter
root@vgoip:~# insmod /lib/modules/test_klp_livepatch.ko
insmod: can't insert '/lib/modules/test_klp_livepatch.ko': unknown
symbol in module, or unknown parameter
I agree with you readelf shows something went wrong with relocations.
Christophe
^ permalink raw reply
* Re: ppc64le STRICT_MODULE_RWX and livepatch apply_relocate_add() crashes
From: Joe Lawrence @ 2021-12-14 13:00 UTC (permalink / raw)
To: Christophe Leroy, Russell Currey, live-patching@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
Cc: Peter Zijlstra, Jordan Niethe, Jessica Yu, Josh Poimboeuf
In-Reply-To: <27cee0a4-aa34-7a52-f98c-ab8c13aafb12@csgroup.eu>
On 12/14/21 7:44 AM, Christophe Leroy wrote:
>
>
> Le 13/12/2021 à 18:26, Joe Lawrence a écrit :
>> On 12/13/21 11:36 AM, Christophe Leroy wrote:
>>>
>>>
>>> Le 13/12/2021 à 15:47, Joe Lawrence a écrit :
>>>> On 12/13/21 2:42 AM, Christophe Leroy wrote:
>>>>>
>>>>> Hello Joe,
>>>>>
>>>>> I'm implementing LIVEPATCH on PPC32 and I wanted to test with
>>>>> STRICT_MODULE_RWX enabled so I took your branch as suggested, but I'm
>>>>> getting the following errors on build. What shall I do ?
>>>>>
>>>>> CALL scripts/checksyscalls.sh
>>>>> CALL scripts/atomic/check-atomics.sh
>>>>> CHK include/generated/compile.h
>>>>> KLP lib/livepatch/test_klp_convert1.ko
>>>>> klp-convert: section .rela.klp.module_relocs.test_klp_convert_mod length
>>>>> beyond nr_entries
>>>>>
>>>>> klp-convert: Unable to load user-provided sympos
>>>>> make[2]: *** [scripts/Makefile.modfinal:79:
>>>>> lib/livepatch/test_klp_convert1.ko] Error 255
>>>>> KLP lib/livepatch/test_klp_convert2.ko
>>>>> klp-convert: section .rela.klp.module_relocs.test_klp_convert_mod length
>>>>> beyond nr_entries
>>>>>
>>>>> klp-convert: Unable to load user-provided sympos
>>>>> make[2]: *** [scripts/Makefile.modfinal:79:
>>>>> lib/livepatch/test_klp_convert2.ko] Error 255
>>>>> KLP lib/livepatch/test_klp_convert_sections.ko
>>>>> klp-convert: section .rela.klp.module_relocs.test_klp_convert_mod length
>>>>> beyond nr_entries
>>>>>
>>>>> klp-convert: Unable to load user-provided sympos
>>>>> make[2]: *** [scripts/Makefile.modfinal:79:
>>>>> lib/livepatch/test_klp_convert_sections.ko] Error 255
>>>>> make[2]: Target '__modfinal' not remade because of errors.
>>>>> make[1]: *** [scripts/Makefile.modpost:145: __modpost] Error 2
>>>>> make: *** [Makefile:1770: modules] Error 2
>>>>>
>>>>
>>>> Hi Christophe,
>>>>
>>>> Interesting failure mode. That's klp-convert complaining that it found
>>>> more relocations in a .klp.module_relocs.<objname> section than
>>>> expected, i.e. nr_entries = sec->size / sizeof(struct klp_module_reloc).
>>>>
>>>> A few possibilities: the ELF sec->size was incorrectly set/read by
>>>> build/libelf (I doubt that). Or maybe the layout/size of struct
>>>> klp_module_reloc is not consistent between kernel and userspace (I'm
>>>> more suspicious of this).
>>>>
>>>> Can you post a copy of the build's symbols.klp and
>>>> lib/livepatch/test_klp_convert1.tmp.ko somewhere? I should be able to
>>>> start debug with those files.
>>>>
>>>
>>> I sent you both files off list.
>>>
>>> It looks like klp-convert doesn't use the correct size. It finds a
>>> struct of size 12 hence 3 entries for a section of size 40.
>>>
>>> On PPC32 the struct has size 8 (void * is 4 and int is 4).
>>>
>>> But I'm cross-building from x86_64 where the struct is 8 + 4 = 12.
>>>
>>> Can it be the reason ?
>>>
>>
>> I'm pretty sure that is it. I haven't had much runtime with klp-convert
>> and cross-building (I've only found one big/little endian bug with
>> x86_64->s390x) and was going to ask you how you were testing :)
>>
>> Do you know if there are other kernel build tools that deal with similar
>> situations? This seems like a tricky job for the userspace build tool
>> to determine non-native target struct layout.
>>
>> In the meantime, hacking in:
>>
>> struct klp_module_reloc {
>> - void *sym;
>> + uint32_t sym;
>> unsigned int sympos;
>> } __packed;
>>
>> gets me generating an output .ko file, but the readelf output doesn't
>> look right.
>>
>> I'll add this to the patchset TODO list, but may not get to it for a
>> while -- is there any chance the above hack works or could you test a
>> local non-cross build?
>>
>
> No I have no way to do a non-cross build. My target is an embedded board
> with slow CPU and little memory.
>
> I tested with your hack, I get:
>
> root@vgoip:~# insmod /lib/modules/test_klp_convert1.ko
> insmod: can't insert '/lib/modules/test_klp_convert1.ko': unknown symbol
> in module, or unknown parameter
> root@vgoip:~# insmod /lib/modules/test_klp_livepatch.ko
> insmod: can't insert '/lib/modules/test_klp_livepatch.ko': unknown
> symbol in module, or unknown parameter
>
>
> I agree with you readelf shows something went wrong with relocations.
>
Thanks for trying that. Can you point me to the cross-compiler suite
that you are using for build and readelf? Kernel .config would be handy
too and I can try to reproduce locally for debugging.
Thanks,
--
Joe
^ permalink raw reply
* Re: [PATCH v2 0/2] kdump: simplify code
From: Matthew Wilcox @ 2021-12-14 13:14 UTC (permalink / raw)
To: Tiezhu Yang
Cc: linux-ia64@vger.kernel.org, Baoquan He, linux-sh@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, x86@kernel.org,
kexec@lists.infradead.org, linux-mips@vger.kernel.org,
linux-kernel@vger.kernel.org, David Laight, Vivek Goyal,
Xuefeng Li, linux-fsdevel@vger.kernel.org, Andrew Morton,
Dave Young, linux-riscv@lists.infradead.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <36440c0f-8b8a-c401-684f-6f54ce9c061e@loongson.cn>
On Tue, Dec 14, 2021 at 06:03:11PM +0800, Tiezhu Yang wrote:
> On 12/13/2021 10:43 PM, Matthew Wilcox wrote:
> > On Mon, Dec 13, 2021 at 08:30:33AM +0000, David Laight wrote:
> > > From: Matthew Wilcox
> > > > Sent: 12 December 2021 11:48
> > > >
> > > > On Sat, Dec 11, 2021 at 05:53:46PM +0000, David Laight wrote:
> > > > > From: Tiezhu Yang
> > > > > > Sent: 11 December 2021 03:33
> > > > > >
> > > > > > v2:
> > > > > > -- add copy_to_user_or_kernel() in lib/usercopy.c
> > > > > > -- define userbuf as bool type
> > > > >
> > > > > Instead of having a flag to indicate whether the buffer is user or kernel,
> > > > > would it be better to have two separate buffer pointers.
> > > > > One for a user space buffer, the other for a kernel space buffer.
> > > > > Exactly one of the buffers should always be NULL.
> > > >
> > > > No. You should be using an iov_iter instead. See
> > > > https://lore.kernel.org/all/Ya4bdB0UBJCZhUSo@casper.infradead.org/
> > > > for a start on this.
> > >
> > > iov_iter gets horribly expensive...
> >
> > Oh, right. Reading the kcore is a high-performance path, my mistake.
> >
>
> Hi,
>
> Thank you for your discussions.
>
> The intention of this patchset is to simplify the related code with no
> functional changes and no side effects.
>
> At this moment, if you are OK, I will send v3 used with inline function
> copy_to_user_or_kernel() to keep it simple, maybe other more changes can
> be done in the future if no any side effect.
That would be pointless. I already sent a series to remove this,
which you were cc'd on.
^ permalink raw reply
* Re: ppc64le STRICT_MODULE_RWX and livepatch apply_relocate_add() crashes
From: Christophe Leroy @ 2021-12-14 13:35 UTC (permalink / raw)
To: Joe Lawrence, Russell Currey, live-patching@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
Cc: Peter Zijlstra, Jordan Niethe, Jessica Yu, Josh Poimboeuf
In-Reply-To: <af1eb0c3-4beb-bbc4-39da-da42f104f6c7@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 5223 bytes --]
Le 14/12/2021 à 14:00, Joe Lawrence a écrit :
> On 12/14/21 7:44 AM, Christophe Leroy wrote:
>>
>>
>> Le 13/12/2021 à 18:26, Joe Lawrence a écrit :
>>> On 12/13/21 11:36 AM, Christophe Leroy wrote:
>>>>
>>>>
>>>> Le 13/12/2021 à 15:47, Joe Lawrence a écrit :
>>>>> On 12/13/21 2:42 AM, Christophe Leroy wrote:
>>>>>>
>>>>>> Hello Joe,
>>>>>>
>>>>>> I'm implementing LIVEPATCH on PPC32 and I wanted to test with
>>>>>> STRICT_MODULE_RWX enabled so I took your branch as suggested, but I'm
>>>>>> getting the following errors on build. What shall I do ?
>>>>>>
>>>>>> CALL scripts/checksyscalls.sh
>>>>>> CALL scripts/atomic/check-atomics.sh
>>>>>> CHK include/generated/compile.h
>>>>>> KLP lib/livepatch/test_klp_convert1.ko
>>>>>> klp-convert: section .rela.klp.module_relocs.test_klp_convert_mod length
>>>>>> beyond nr_entries
>>>>>>
>>>>>> klp-convert: Unable to load user-provided sympos
>>>>>> make[2]: *** [scripts/Makefile.modfinal:79:
>>>>>> lib/livepatch/test_klp_convert1.ko] Error 255
>>>>>> KLP lib/livepatch/test_klp_convert2.ko
>>>>>> klp-convert: section .rela.klp.module_relocs.test_klp_convert_mod length
>>>>>> beyond nr_entries
>>>>>>
>>>>>> klp-convert: Unable to load user-provided sympos
>>>>>> make[2]: *** [scripts/Makefile.modfinal:79:
>>>>>> lib/livepatch/test_klp_convert2.ko] Error 255
>>>>>> KLP lib/livepatch/test_klp_convert_sections.ko
>>>>>> klp-convert: section .rela.klp.module_relocs.test_klp_convert_mod length
>>>>>> beyond nr_entries
>>>>>>
>>>>>> klp-convert: Unable to load user-provided sympos
>>>>>> make[2]: *** [scripts/Makefile.modfinal:79:
>>>>>> lib/livepatch/test_klp_convert_sections.ko] Error 255
>>>>>> make[2]: Target '__modfinal' not remade because of errors.
>>>>>> make[1]: *** [scripts/Makefile.modpost:145: __modpost] Error 2
>>>>>> make: *** [Makefile:1770: modules] Error 2
>>>>>>
>>>>>
>>>>> Hi Christophe,
>>>>>
>>>>> Interesting failure mode. That's klp-convert complaining that it found
>>>>> more relocations in a .klp.module_relocs.<objname> section than
>>>>> expected, i.e. nr_entries = sec->size / sizeof(struct klp_module_reloc).
>>>>>
>>>>> A few possibilities: the ELF sec->size was incorrectly set/read by
>>>>> build/libelf (I doubt that). Or maybe the layout/size of struct
>>>>> klp_module_reloc is not consistent between kernel and userspace (I'm
>>>>> more suspicious of this).
>>>>>
>>>>> Can you post a copy of the build's symbols.klp and
>>>>> lib/livepatch/test_klp_convert1.tmp.ko somewhere? I should be able to
>>>>> start debug with those files.
>>>>>
>>>>
>>>> I sent you both files off list.
>>>>
>>>> It looks like klp-convert doesn't use the correct size. It finds a
>>>> struct of size 12 hence 3 entries for a section of size 40.
>>>>
>>>> On PPC32 the struct has size 8 (void * is 4 and int is 4).
>>>>
>>>> But I'm cross-building from x86_64 where the struct is 8 + 4 = 12.
>>>>
>>>> Can it be the reason ?
>>>>
>>>
>>> I'm pretty sure that is it. I haven't had much runtime with klp-convert
>>> and cross-building (I've only found one big/little endian bug with
>>> x86_64->s390x) and was going to ask you how you were testing :)
>>>
>>> Do you know if there are other kernel build tools that deal with similar
>>> situations? This seems like a tricky job for the userspace build tool
>>> to determine non-native target struct layout.
>>>
>>> In the meantime, hacking in:
>>>
>>> struct klp_module_reloc {
>>> - void *sym;
>>> + uint32_t sym;
>>> unsigned int sympos;
>>> } __packed;
>>>
>>> gets me generating an output .ko file, but the readelf output doesn't
>>> look right.
>>>
>>> I'll add this to the patchset TODO list, but may not get to it for a
>>> while -- is there any chance the above hack works or could you test a
>>> local non-cross build?
>>>
>>
>> No I have no way to do a non-cross build. My target is an embedded board
>> with slow CPU and little memory.
>>
>> I tested with your hack, I get:
>>
>> root@vgoip:~# insmod /lib/modules/test_klp_convert1.ko
>> insmod: can't insert '/lib/modules/test_klp_convert1.ko': unknown symbol
>> in module, or unknown parameter
>> root@vgoip:~# insmod /lib/modules/test_klp_livepatch.ko
>> insmod: can't insert '/lib/modules/test_klp_livepatch.ko': unknown
>> symbol in module, or unknown parameter
>>
>>
>> I agree with you readelf shows something went wrong with relocations.
>>
>
> Thanks for trying that. Can you point me to the cross-compiler suite
> that you are using for build and readelf? Kernel .config would be handy
> too and I can try to reproduce locally for debugging.
>
Livepatch on PPC32 is WIP, branch 'livepatch' on
https://github.com/chleroy/linux.git.
You can use branch 'joe' on https://github.com/chleroy/linux.git which
is a merge of your branch klp-convert-v5-expanded-v5.16-rebase1 with my
livepatch branch.
Crosstool:
https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/x86_64-gcc-11.1.0-nolibc-powerpc64-linux.tar.xz
defconfig is attached
Thanks for your help
Christophe
[-- Attachment #2: defconfig --]
[-- Type: text/plain, Size: 1974 bytes --]
# CONFIG_SWAP is not set
CONFIG_SYSVIPC=y
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y
CONFIG_BPF_JIT=y
CONFIG_VIRT_CPU_ACCOUNTING_NATIVE=y
CONFIG_LOG_BUF_SHIFT=14
CONFIG_EXPERT=y
# CONFIG_ELF_CORE is not set
# CONFIG_BASE_FULL is not set
# CONFIG_FUTEX is not set
CONFIG_KALLSYMS_ALL=y
CONFIG_PERF_EVENTS=y
# CONFIG_VM_EVENT_COUNTERS is not set
CONFIG_PPC_8xx=y
CONFIG_8xx_GPIO=y
CONFIG_SMC_UCODE_PATCH=y
CONFIG_PIN_TLB=y
CONFIG_GEN_RTC=y
CONFIG_HZ_100=y
CONFIG_MATH_EMULATION=y
CONFIG_PPC_16K_PAGES=y
CONFIG_ADVANCED_OPTIONS=y
CONFIG_LIVEPATCH=y
# CONFIG_SECCOMP is not set
CONFIG_MODULES=y
CONFIG_PARTITION_ADVANCED=y
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_UNIX=y
CONFIG_INET=y
CONFIG_IP_MULTICAST=y
CONFIG_IP_PNP=y
CONFIG_SYN_COOKIES=y
# CONFIG_FW_LOADER is not set
CONFIG_MTD=y
CONFIG_MTD_BLOCK=y
CONFIG_MTD_JEDECPROBE=y
CONFIG_MTD_CFI_ADV_OPTIONS=y
CONFIG_MTD_CFI_GEOMETRY=y
# CONFIG_MTD_MAP_BANK_WIDTH_1 is not set
# CONFIG_MTD_MAP_BANK_WIDTH_2 is not set
# CONFIG_MTD_CFI_I1 is not set
# CONFIG_MTD_CFI_I2 is not set
CONFIG_MTD_CFI_I4=y
CONFIG_MTD_CFI_AMDSTD=y
CONFIG_MTD_PHYSMAP=y
CONFIG_MTD_PHYSMAP_OF=y
# CONFIG_BLK_DEV is not set
CONFIG_NETDEVICES=y
CONFIG_FS_ENET=y
# CONFIG_FS_ENET_HAS_SCC is not set
CONFIG_DAVICOM_PHY=y
# CONFIG_INPUT is not set
# CONFIG_SERIO is not set
# CONFIG_VT is not set
# CONFIG_LEGACY_PTYS is not set
CONFIG_SERIAL_CPM=y
CONFIG_SERIAL_CPM_CONSOLE=y
CONFIG_SPI=y
CONFIG_SPI_FSL_SPI=y
# CONFIG_HWMON is not set
CONFIG_WATCHDOG=y
CONFIG_8xxx_WDT=y
# CONFIG_USB_SUPPORT is not set
# CONFIG_DNOTIFY is not set
CONFIG_TMPFS=y
CONFIG_CRAMFS=y
CONFIG_NFS_FS=y
CONFIG_ROOT_NFS=y
CONFIG_CRYPTO=y
CONFIG_CRYPTO_DEV_TALITOS=y
CONFIG_CRC32_SLICEBY4=y
CONFIG_DYNAMIC_DEBUG=y
CONFIG_DEBUG_INFO=y
CONFIG_MAGIC_SYSRQ=y
CONFIG_DEBUG_FS=y
CONFIG_DEBUG_VM_PGTABLE=y
CONFIG_DETECT_HUNG_TASK=y
CONFIG_FUNCTION_TRACER=y
CONFIG_FTRACE_STARTUP_TEST=y
# CONFIG_EVENT_TRACE_STARTUP_TEST is not set
CONFIG_BDI_SWITCH=y
CONFIG_PPC_EARLY_DEBUG=y
CONFIG_TEST_LIVEPATCH=m
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox