* [PATCH AUTOSEL 4.9 8/8] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163931.607291-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index ba49ae6625f1b..a10b67df83bae 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2042,6 +2042,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 11/11] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163914.607152-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 3188040022c4f..78f75e48dfe7f 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2096,6 +2096,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 21/21] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:38 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163845.606933-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 4004dbdab9c7b..d407b73298171 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2087,6 +2087,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 36/38] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:37 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163751.606577-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index ca92e01d0bd1b..e32f7700303bc 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2106,6 +2106,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.7 50/54] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:36 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163634.606093-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 3dcfecf858f36..5216c5a7f46da 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2099,6 +2099,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.8 59/63] powerpc/perf: Fix soft lockups due to missed interrupt accounting
From: Sasha Levin @ 2020-08-24 16:34 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Athira Rajeev, linuxppc-dev, Sasha Levin
In-Reply-To: <20200824163504.605538-1-sashal@kernel.org>
From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
[ Upstream commit 17899eaf88d689529b866371344c8f269ba79b5f ]
Performance monitor interrupt handler checks if any counter has
overflown and calls record_and_restart() in core-book3s which invokes
perf_event_overflow() to record the sample information. Apart from
creating sample, perf_event_overflow() also does the interrupt and
period checks via perf_event_account_interrupt().
Currently we record information only if the SIAR (Sampled Instruction
Address Register) valid bit is set (using siar_valid() check) and
hence the interrupt check.
But it is possible that we do sampling for some events that are not
generating valid SIAR, and hence there is no chance to disable the
event if interrupts are more than max_samples_per_tick. This leads to
soft lockup.
Fix this by adding perf_event_account_interrupt() in the invalid SIAR
code path for a sampling event. ie if SIAR is invalid, just do
interrupt check and don't record the sample information.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/1596717992-7321-1-git-send-email-atrajeev@linux.vnet.ibm.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/perf/core-book3s.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 01d70280d2872..190bc4f255b42 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2101,6 +2101,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
if (perf_event_overflow(event, &data, regs))
power_pmu_stop(event, 0);
+ } else if (period) {
+ /* Account for interrupt in case of invalid SIAR */
+ if (perf_event_account_interrupt(event))
+ power_pmu_stop(event, 0);
}
}
--
2.25.1
^ permalink raw reply related
* RE: Re：Re: [PATCH] powerpc: Fix a bug in __div64_32 if divisor is zero
From: David Laight @ 2020-08-24 15:05 UTC (permalink / raw)
To: 'Guohua Zhong', paubert@iram.es
Cc: wangle6@huawei.com, gregkh@linuxfoundation.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
paulus@samba.org, stable@vger.kernel.org, nixiaoming@huawei.com
In-Reply-To: <20200824132539.35972-1-zhongguohua1@huawei.com>
From: Guohua Zhong
> Sent: 24 August 2020 14:26
>
> >> >In generic version in lib/math/div64.c, there is no checking of 'base'
> >> >either.
> >> >Do we really want to add this check in the powerpc version only ?
> >>
> >> >The only user of __div64_32() is do_div() in
> >> >include/asm-generic/div64.h. Wouldn't it be better to do the check there ?
> >>
> >> >Christophe
> >>
> >> Yet, I have noticed that there is no checking of 'base' in these functions.
> >> But I am not sure how to check is better.As we know that the result is
> >> undefined when divisor is zero. It maybe good to print error and dump stack.
I thought that the onus was put on the caller to avoid divide by zero.
On x86 divide by zero causes an exception which (I'm pretty sure)
leads to a oops/panic.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Joakim Tjernlund @ 2020-08-24 14:12 UTC (permalink / raw)
To: yebin10@huawei.com, gregkh@linuxfoundation.org, balbi@kernel.org
Cc: linux-usb@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
leoyang.li@nxp.com
In-Reply-To: <87d03gjgbw.fsf@kernel.org>
On Mon, 2020-08-24 at 16:58 +0300, Felipe Balbi wrote:
> Joakim Tjernlund <Joakim.Tjernlund@infinera.com> writes:
>
> > On Mon, 2020-08-24 at 10:21 +0200, Greg KH wrote:
> > >
> > > On Mon, Aug 24, 2020 at 04:04:37PM +0800, Ye Bin wrote:
> > > > Signed-off-by: Ye Bin <yebin10@huawei.com>
> > >
> > > I can't take patches without any changelog text, sorry.
> >
> > Still taking patches for fsl_udc_core.c ?
> > I figured this driver was obsolete and should be moved to one of the Chipidea drivers.
>
> Nobody sent any patches to switch over the users of this driver to
> chipidea. I would love to delete this driver :-)
Me too, I got a few local patches here as the driver is quite buggy.
Got to little USB knowledge to switch it over though :(
Jocke
^ permalink raw reply
* Re: [PATCH] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Felipe Balbi @ 2020-08-24 13:58 UTC (permalink / raw)
To: Joakim Tjernlund, yebin10@huawei.com, gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
leoyang.li@nxp.com
In-Reply-To: <f61f4bc3916f852799edb6af9740afb2118ec84f.camel@infinera.com>
[-- Attachment #1: Type: text/plain, Size: 564 bytes --]
Joakim Tjernlund <Joakim.Tjernlund@infinera.com> writes:
> On Mon, 2020-08-24 at 10:21 +0200, Greg KH wrote:
>>
>> On Mon, Aug 24, 2020 at 04:04:37PM +0800, Ye Bin wrote:
>> > Signed-off-by: Ye Bin <yebin10@huawei.com>
>>
>> I can't take patches without any changelog text, sorry.
>
> Still taking patches for fsl_udc_core.c ?
> I figured this driver was obsolete and should be moved to one of the Chipidea drivers.
Nobody sent any patches to switch over the users of this driver to
chipidea. I would love to delete this driver :-)
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND] ASoC: fsl_sai: Add -EPROBE_DEFER check for regmap init
From: Mark Brown @ 2020-08-24 13:31 UTC (permalink / raw)
To: Xiubo.Lee, tiwai, timur, alsa-devel, festevam, Shengjiu Wang,
lgirdwood, perex, nicoleotsuka
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1598255887-1391-1-git-send-email-shengjiu.wang@nxp.com>
On Mon, 24 Aug 2020 15:58:07 +0800, Shengjiu Wang wrote:
> Regmap initialization may return -EPROBE_DEFER for clock
> may not be ready, so check -EPROBE_DEFER error type before
> start another Regmap initialization.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: fsl_sai: Add -EPROBE_DEFER check for regmap init
commit: c1e47e8919da525c803d1557a30e44441db1e5ee
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: Re：Re: [PATCH] powerpc: Fix a bug in __div64_32 if divisor is zero
From: Guohua Zhong @ 2020-08-24 13:25 UTC (permalink / raw)
To: paubert
Cc: zhongguohua1, wangle6, gregkh, linuxppc-dev, linux-kernel, paulus,
stable, nixiaoming
In-Reply-To: <20200822172524.GA5451@lt-gp.iram.es>
>> >In generic version in lib/math/div64.c, there is no checking of 'base'
>> >either.
>> >Do we really want to add this check in the powerpc version only ?
>>
>> >The only user of __div64_32() is do_div() in
>> >include/asm-generic/div64.h. Wouldn't it be better to do the check there ?
>>
>> >Christophe
>>
>> Yet, I have noticed that there is no checking of 'base' in these functions.
>> But I am not sure how to check is better.As we know that the result is
>> undefined when divisor is zero. It maybe good to print error and dump stack.
>> Let the process to know that the divisor is zero by sending SIGFPE.
>>
>> diff --git a/include/asm-generic/div64.h b/include/asm-generic/div64.h
>> index a3b98c86f077..161c656ee3ee 100644
>> --- a/include/asm-generic/div64.h
>> +++ b/include/asm-generic/div64.h
>> @@ -43,6 +43,11 @@
>> # define do_div(n,base) ({ \
>> uint32_t __base = (base); \
>> uint32_t __rem; \
>> + if (unlikely(base == 0)) { \
>> + pr_err("do_div base=%d\n",base); \
>> + dump_stack(); \
>> + force_sig(SIGFPE); \
>> + }
>>
> I suspect this will generate a strong reaction. SIGFPE is for user space
> instruction attempting a division by zero. A division by zero in the
> kernel is a kernel bug, period, and you don't want to kill a user
> process for this reason.
> If it happens in an interrupt, the context of the kernel may not even be
> related to the current process.
> Many other architectures (x86 for example) already trigger an exception
> on a division by zero but the handler will find that the exception
> happened in kernel context and generate an Oops, not raise a signal in a
> (possibly innocent) userland process.
OK. So just don't touch do_div functions in include/asm-generic/div64.h
But for powerpc it can not trigger an exception when divisor is 0 in __div64_32.
So the patch as below is still useful for powerpc. If this patch looks good for
you, please help to review. I will send the new patch later.
Thanks for your reply.
diff --git a/arch/powerpc/boot/div64.S b/arch/powerpc/boot/div64.S
index 4354928ed62e..1d3561cf16fa 100644
--- a/arch/powerpc/boot/div64.S
+++ b/arch/powerpc/boot/div64.S
@@ -13,8 +13,10 @@
.globl __div64_32
.globl __div64_32
__div64_32:
+ cmplwi r4,0 # check if divisor r4 is zero
lwz r5,0(r3) # get the dividend into r5/r6
lwz r6,4(r3)
+ beq 5f # jump to label 5 if r4(divisor) is zero
cmplw r5,r4
li r7,0
li r8,0
@@ -52,7 +54,7 @@ __div64_32:
4: stw r7,0(r3) # return the quotient in *r3
stw r8,4(r3)
mr r3,r6 # return the remainder in r3
- blr
+5: blr # return if divisor r4 is zero
/*
* Extended precision shifts.
diff --git a/arch/powerpc/lib/div64.S b/arch/powerpc/lib/div64.S
index 3d5426e7dcc4..570774d9782d 100644
--- a/arch/powerpc/lib/div64.S
+++ b/arch/powerpc/lib/div64.S
@@ -13,8 +13,10 @@
#include <asm/processor.h>
_GLOBAL(__div64_32)
+ cmplwi r4,0 # check if divisor r4 is zero
lwz r5,0(r3) # get the dividend into r5/r6
lwz r6,4(r3)
+ beq 5f # jump to label 5 if r4(divisor) is zero
cmplw r5,r4
li r7,0
li r8,0
@@ -52,4 +54,4 @@ _GLOBAL(__div64_32)
4: stw r7,0(r3) # return the quotient in *r3
stw r8,4(r3)
mr r3,r6 # return the remainder in r3
- blr
+5: blr # return if divisor r4 is zero
Guohua
^ permalink raw reply related
* [PATCH] selftests/powerpc: Fix prefixes in alignment_handler signal handler
From: Jordan Niethe @ 2020-08-24 13:12 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Jordan Niethe
The signal handler in the alignment handler self test has the ability to
jump over the instruction that triggered the signal. It does this by
incrementing the PT_NIP in the user context by 4. If it were a prefixed
instruction this will mean that the suffix is then executed which is
incorrect. Instead check if the major opcode indicates a prefixed
instruction (e.g. it is 1) and if so increment PT_NIP by 8.
If ISA v3.1 is not available treat it as a word instruction even if the
major opcode is 1.
Fixes: 620a6473df36 ("selftests/powerpc: Add prefixed loads/stores to
alignment_handler test")
Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
.../selftests/powerpc/alignment/alignment_handler.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/powerpc/alignment/alignment_handler.c b/tools/testing/selftests/powerpc/alignment/alignment_handler.c
index 55ef15184057..c197ff828120 100644
--- a/tools/testing/selftests/powerpc/alignment/alignment_handler.c
+++ b/tools/testing/selftests/powerpc/alignment/alignment_handler.c
@@ -64,12 +64,14 @@ int bufsize;
int debug;
int testing;
volatile int gotsig;
+bool haveprefixes;
char *cipath = "/dev/fb0";
long cioffset;
void sighandler(int sig, siginfo_t *info, void *ctx)
{
ucontext_t *ucp = ctx;
+ u32 inst;
if (!testing) {
signal(sig, SIG_DFL);
@@ -77,7 +79,12 @@ void sighandler(int sig, siginfo_t *info, void *ctx)
}
gotsig = sig;
#ifdef __powerpc64__
- ucp->uc_mcontext.gp_regs[PT_NIP] += 4;
+ if (haveprefixes) {
+ inst = *(u32 *)ucp->uc_mcontext.gp_regs[PT_NIP];
+ ucp->uc_mcontext.gp_regs[PT_NIP] += ((inst >> 26 == 1) ? 8 : 4);
+ } else {
+ ucp->uc_mcontext.gp_regs[PT_NIP] += 4;
+ }
#else
ucp->uc_mcontext.uc_regs->gregs[PT_NIP] += 4;
#endif
@@ -648,6 +655,8 @@ int main(int argc, char *argv[])
exit(1);
}
+ haveprefixes = have_hwcap2(PPC_FEATURE2_ARCH_3_1);
+
rc |= test_harness(test_alignment_handler_vsx_206,
"test_alignment_handler_vsx_206");
rc |= test_harness(test_alignment_handler_vsx_207,
--
2.17.1
^ permalink raw reply related
* Re: Re：Re: [PATCH] powerpc: Fix a bug in __div64_32 if divisor is zero
From: Guohua Zhong @ 2020-08-24 11:54 UTC (permalink / raw)
To: segher
Cc: zhongguohua1, wangle6, gregkh, linuxppc-dev, linux-kernel, paulus,
stable, nixiaoming
In-Reply-To: <20200823001101.GO28786@gate.crashing.org>
>> Yet, I have noticed that there is no checking of 'base' in these functions.
>> But I am not sure how to check is better.As we know that the result is
>> undefined when divisor is zero. It maybe good to print error and dump stack.
>> Let the process to know that the divisor is zero by sending SIGFPE.
> That is now what the PowerPC integer divide insns do: they just leave
> the result undefined (and they can set the overflow flag then, but no
> one uses that).
OK ,So just keep the patch as below. If this patch looks good for you, please
help to review. I will send the new patch later.
Thanks for your reply.
diff --git a/arch/powerpc/boot/div64.S b/arch/powerpc/boot/div64.S
index 4354928ed62e..1d3561cf16fa 100644
--- a/arch/powerpc/boot/div64.S
+++ b/arch/powerpc/boot/div64.S
@@ -13,8 +13,10 @@
.globl __div64_32
.globl __div64_32
__div64_32:
+ cmplwi r4,0 # check if divisor r4 is zero
lwz r5,0(r3) # get the dividend into r5/r6
lwz r6,4(r3)
+ beq 5f # jump to label 5 if r4(divisor) is zero
cmplw r5,r4
li r7,0
li r8,0
@@ -52,7 +54,7 @@ __div64_32:
4: stw r7,0(r3) # return the quotient in *r3
stw r8,4(r3)
mr r3,r6 # return the remainder in r3
- blr
+5: blr # return if divisor r4 is zero
/*
* Extended precision shifts.
diff --git a/arch/powerpc/lib/div64.S b/arch/powerpc/lib/div64.S
index 3d5426e7dcc4..570774d9782d 100644
--- a/arch/powerpc/lib/div64.S
+++ b/arch/powerpc/lib/div64.S
@@ -13,8 +13,10 @@
#include <asm/processor.h>
_GLOBAL(__div64_32)
+ cmplwi r4,0 # check if divisor r4 is zero
lwz r5,0(r3) # get the dividend into r5/r6
lwz r6,4(r3)
+ beq 5f # jump to label 5 if r4(divisor) is zero
cmplw r5,r4
li r7,0
li r8,0
@@ -52,4 +54,4 @@ _GLOBAL(__div64_32)
4: stw r7,0(r3) # return the quotient in *r3
stw r8,4(r3)
mr r3,r6 # return the remainder in r3
- blr
+5: blr # return if divisor r4 is zero
Guohua
^ permalink raw reply related
* Re: [PATCH] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Joakim Tjernlund @ 2020-08-24 8:49 UTC (permalink / raw)
To: yebin10@huawei.com, gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
leoyang.li@nxp.com
In-Reply-To: <20200824082122.GA336539@kroah.com>
On Mon, 2020-08-24 at 10:21 +0200, Greg KH wrote:
>
> On Mon, Aug 24, 2020 at 04:04:37PM +0800, Ye Bin wrote:
> > Signed-off-by: Ye Bin <yebin10@huawei.com>
>
> I can't take patches without any changelog text, sorry.
Still taking patches for fsl_udc_core.c ?
I figured this driver was obsolete and should be moved to one of the Chipidea drivers.
Jocke
^ permalink raw reply
* [PATCH v2] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Ye Bin @ 2020-08-24 8:42 UTC (permalink / raw)
To: leoyang.li, linux-usb, linuxppc-dev; +Cc: Ye Bin
udc_controller->irq is "unsigned int" always >= 0, but platform_get_irq may
return little than zero. So "dc_controller->irq < 0" condition is never
accessible.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
drivers/usb/gadget/udc/fsl_udc_core.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
index a6f7b2594c09..3e98740b8cfc 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2439,11 +2439,12 @@ static int fsl_udc_probe(struct platform_device *pdev)
/* DEN is bidirectional ep number, max_ep doubles the number */
udc_controller->max_ep = (dccparams & DCCPARAMS_DEN_MASK) * 2;
- udc_controller->irq = platform_get_irq(pdev, 0);
- if (udc_controller->irq <= 0) {
- ret = udc_controller->irq ? : -ENODEV;
+ ret = platform_get_irq(pdev, 0);
+ if (ret <= 0) {
+ ret = ret ? : -ENODEV;
goto err_iounmap;
}
+ udc_controller->irq = ret;
ret = request_irq(udc_controller->irq, fsl_udc_irq, IRQF_SHARED,
driver_name, udc_controller);
--
2.25.4
^ permalink raw reply related
* [PATCH] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Ye Bin @ 2020-08-24 8:04 UTC (permalink / raw)
To: leoyang.li, linux-usb, linuxppc-dev; +Cc: Ye Bin
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
drivers/usb/gadget/udc/fsl_udc_core.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
index a6f7b2594c09..3e98740b8cfc 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2439,11 +2439,12 @@ static int fsl_udc_probe(struct platform_device *pdev)
/* DEN is bidirectional ep number, max_ep doubles the number */
udc_controller->max_ep = (dccparams & DCCPARAMS_DEN_MASK) * 2;
- udc_controller->irq = platform_get_irq(pdev, 0);
- if (udc_controller->irq <= 0) {
- ret = udc_controller->irq ? : -ENODEV;
+ ret = platform_get_irq(pdev, 0);
+ if (ret <= 0) {
+ ret = ret ? : -ENODEV;
goto err_iounmap;
}
+ udc_controller->irq = ret;
ret = request_irq(udc_controller->irq, fsl_udc_irq, IRQF_SHARED,
driver_name, udc_controller);
--
2.25.4
^ permalink raw reply related
* Re: [PATCH] usb: gadget: fsl: Fix unsigned expression compared with zero in fsl_udc_probe
From: Greg KH @ 2020-08-24 8:21 UTC (permalink / raw)
To: Ye Bin; +Cc: linux-usb, linuxppc-dev, leoyang.li
In-Reply-To: <20200824080437.229826-1-yebin10@huawei.com>
On Mon, Aug 24, 2020 at 04:04:37PM +0800, Ye Bin wrote:
> Signed-off-by: Ye Bin <yebin10@huawei.com>
I can't take patches without any changelog text, sorry.
greg k-h
^ permalink raw reply
* [PATCH RESEND] ASoC: fsl_sai: Add -EPROBE_DEFER check for regmap init
From: Shengjiu Wang @ 2020-08-24 7:58 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood
Cc: linuxppc-dev, linux-kernel
Regmap initialization may return -EPROBE_DEFER for clock
may not be ready, so check -EPROBE_DEFER error type before
start another Regmap initialization.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
sound/soc/fsl/fsl_sai.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index f6969a5d49e3..62c5fdb678fc 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -959,7 +959,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
"bus", base, &fsl_sai_regmap_config);
/* Compatible with old DTB cases */
- if (IS_ERR(sai->regmap))
+ if (IS_ERR(sai->regmap) && PTR_ERR(sai->regmap) != -EPROBE_DEFER)
sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
"sai", base, &fsl_sai_regmap_config);
if (IS_ERR(sai->regmap)) {
--
2.27.0
^ permalink raw reply related
* Re: [PATCH v1 09/10] powerpc/pseries/iommu: Make use of DDW even if it does not map the partition
From: Alexey Kardashevskiy @ 2020-08-24 5:17 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Christophe Leroy, Joel Stanley,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200817234033.442511-10-leobras.c@gmail.com>
On 18/08/2020 09:40, Leonardo Bras wrote:
> As of today, if the biggest DDW that can be created can't map the whole
> partition, it's creation is skipped and the default DMA window
> "ibm,dma-window" is used instead.
>
> DDW is 16x bigger than the default DMA window,
16x only under very specific circumstances which are
1. phyp
2. sriov
3. device class in hmc (or what that priority number is in the lpar config).
> having the same amount of
> pages, but increasing the page size to 64k.
> Besides larger DMA window,
"Besides being larger"?
> it performs better for allocations over 4k,
Better how?
> so it would be nice to use it instead.
I'd rather say something like:
===
So far we assumed we can map the guest RAM 1:1 to the bus which worked
with a small number of devices. SRIOV changes it as the user can
configure hundreds VFs and since phyp preallocates TCEs and does not
allow IOMMU pages bigger than 64K, it has to limit the number of TCEs
per a PE to limit waste of physical pages.
===
>
> The DDW created will be used for direct mapping by default.
> If it's not available, indirect mapping will be used instead.
>
> For indirect mapping, it's necessary to update the iommu_table so
> iommu_alloc() can use the DDW created. For this,
> iommu_table_update_window() is called when everything else succeeds
> at enable_ddw().
>
> Removing the default DMA window for using DDW with indirect mapping
> is only allowed if there is no current IOMMU memory allocated in
> the iommu_table. enable_ddw() is aborted otherwise.
>
> As there will never have both direct and indirect mappings at the same
> time, the same property name can be used for the created DDW.
>
> So renaming
> define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
> to
> define DMA64_PROPNAME "linux,dma64-ddr-window-info"
> looks the right thing to do.
I know I suggested this but this does not look so good anymore as I
suspect it breaks kexec (from older kernel to this one) so you either
need to check for both DT names or just keep the old one. Changing the
macro name is fine.
>
> To make sure the property differentiates both cases, a new u32 for flags
> was added at the end of the property, where BIT(0) set means direct
> mapping.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/platforms/pseries/iommu.c | 108 +++++++++++++++++++------
> 1 file changed, 84 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 3a1ef02ad9d5..9544e3c91ced 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -350,8 +350,11 @@ struct dynamic_dma_window_prop {
> __be64 dma_base; /* address hi,lo */
> __be32 tce_shift; /* ilog2(tce_page_size) */
> __be32 window_shift; /* ilog2(tce_window_size) */
> + __be32 flags; /* DDW properties, see bellow */
> };
>
> +#define DDW_FLAGS_DIRECT 0x01
This is set if ((1<<window_shift) >= ddw_memory_hotplug_max()), you
could simply check window_shift and drop the flags.
> +
> struct direct_window {
> struct device_node *device;
> const struct dynamic_dma_window_prop *prop;
> @@ -377,7 +380,7 @@ static LIST_HEAD(direct_window_list);
> static DEFINE_SPINLOCK(direct_window_list_lock);
> /* protects initializing window twice for same device */
> static DEFINE_MUTEX(direct_window_init_mutex);
> -#define DIRECT64_PROPNAME "linux,direct64-ddr-window-info"
> +#define DMA64_PROPNAME "linux,dma64-ddr-window-info"
>
> static int tce_clearrange_multi_pSeriesLP(unsigned long start_pfn,
> unsigned long num_pfn, const void *arg)
> @@ -836,7 +839,7 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
> if (ret)
> return;
>
> - win = of_find_property(np, DIRECT64_PROPNAME, NULL);
> + win = of_find_property(np, DMA64_PROPNAME, NULL);
> if (!win)
> return;
>
> @@ -852,7 +855,7 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
> np, ret);
> }
>
> -static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
> +static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr, bool *direct_mapping)
> {
> struct direct_window *window;
> const struct dynamic_dma_window_prop *direct64;
> @@ -864,6 +867,7 @@ static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
> if (window->device == pdn) {
> direct64 = window->prop;
> *dma_addr = be64_to_cpu(direct64->dma_base);
> + *direct_mapping = be32_to_cpu(direct64->flags) & DDW_FLAGS_DIRECT;
> found = true;
> break;
> }
> @@ -901,8 +905,8 @@ static int find_existing_ddw_windows(void)
> if (!firmware_has_feature(FW_FEATURE_LPAR))
> return 0;
>
> - for_each_node_with_property(pdn, DIRECT64_PROPNAME) {
> - direct64 = of_get_property(pdn, DIRECT64_PROPNAME, &len);
> + for_each_node_with_property(pdn, DMA64_PROPNAME) {
> + direct64 = of_get_property(pdn, DMA64_PROPNAME, &len);
> if (!direct64)
> continue;
>
> @@ -1124,7 +1128,8 @@ static void reset_dma_window(struct pci_dev *dev, struct device_node *par_dn)
> }
>
> static int ddw_property_create(struct property **ddw_win, const char *propname,
> - u32 liobn, u64 dma_addr, u32 page_shift, u32 window_shift)
> + u32 liobn, u64 dma_addr, u32 page_shift,
> + u32 window_shift, bool direct_mapping)
> {
> struct dynamic_dma_window_prop *ddwprop;
> struct property *win64;
> @@ -1144,6 +1149,36 @@ static int ddw_property_create(struct property **ddw_win, const char *propname,
> ddwprop->dma_base = cpu_to_be64(dma_addr);
> ddwprop->tce_shift = cpu_to_be32(page_shift);
> ddwprop->window_shift = cpu_to_be32(window_shift);
> + if (direct_mapping)
> + ddwprop->flags = cpu_to_be32(DDW_FLAGS_DIRECT);
> +
> + return 0;
> +}
> +
> +static int iommu_table_update_window(struct iommu_table **tbl, int nid, unsigned long liobn,
> + unsigned long win_addr, unsigned long page_shift,
> + unsigned long window_size)
Rather strange helper imho. I'd extract the most of
iommu_table_setparms_lpar() into iommu_table_setparms() (except
of_parse_dma_window) and call new helper from where you call
iommu_table_update_window; and do
iommu_pseries_alloc_table/iommu_tce_table_put there.
> +{
> + struct iommu_table *new_tbl, *old_tbl;
> +
> + new_tbl = iommu_pseries_alloc_table(nid);
> + if (!new_tbl)
> + return -ENOMEM;
> +
> + old_tbl = *tbl;
> + new_tbl->it_index = liobn;
> + new_tbl->it_offset = win_addr >> page_shift;
> + new_tbl->it_page_shift = page_shift;
> + new_tbl->it_size = window_size >> page_shift;
> + new_tbl->it_base = old_tbl->it_base;
Should not be used in pseries.
> + new_tbl->it_busno = old_tbl->it_busno;
> + new_tbl->it_blocksize = old_tbl->it_blocksize;
16 for pseries and does not change (may be even make it a macro).
> + new_tbl->it_type = old_tbl->it_type;
TCE_PCI.
> + new_tbl->it_ops = old_tbl->it_ops;
> +
> + iommu_init_table(new_tbl, nid, old_tbl->it_reserved_start, old_tbl->it_reserved_end);
> + iommu_tce_table_put(old_tbl);
> + *tbl = new_tbl;
>
> return 0;
> }
> @@ -1171,12 +1206,16 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> struct direct_window *window;
> struct property *win64 = NULL;
> struct failed_ddw_pdn *fpdn;
> - bool default_win_removed = false;
> + bool default_win_removed = false, maps_whole_partition = false;
s/maps_whole_partition/direct_mapping/
> + struct pci_dn *pci = PCI_DN(pdn);
> + struct iommu_table *tbl = pci->table_group->tables[0];
>
> mutex_lock(&direct_window_init_mutex);
>
> - if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset))
> - goto out_unlock;
> + if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset, &maps_whole_partition)) {
> + mutex_unlock(&direct_window_init_mutex);
> + return maps_whole_partition;
> + }
>
> /*
> * If we already went through this for a previous function of
> @@ -1258,16 +1297,24 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> query.page_size);
> goto out_failed;
> }
> +
> /* verify the window * number of ptes will map the partition */
> - /* check largest block * page size > max memory hotplug addr */
> max_addr = ddw_memory_hotplug_max();
> if (query.largest_available_block < (max_addr >> page_shift)) {
> - dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu "
> - "%llu-sized pages\n", max_addr, query.largest_available_block,
> - 1ULL << page_shift);
> - goto out_failed;
> + dev_dbg(&dev->dev, "can't map partition max 0x%llx with %llu %llu-sized pages\n",
> + max_addr, query.largest_available_block,
> + 1ULL << page_shift);
> +
> + len = order_base_2(query.largest_available_block << page_shift);
> + } else {
> + maps_whole_partition = true;
> + len = order_base_2(max_addr);
> }
> - len = order_base_2(max_addr);
> +
> + /* DDW + IOMMU on single window may fail if there is any allocation */
> + if (default_win_removed && !maps_whole_partition &&
> + iommu_table_in_use(tbl))
> + goto out_failed;
>
> ret = create_ddw(dev, ddw_avail, &create, page_shift, len);
> if (ret != 0)
> @@ -1277,8 +1324,8 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> create.liobn, dn);
>
> win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
> - ret = ddw_property_create(&win64, DIRECT64_PROPNAME, create.liobn, win_addr,
> - page_shift, len);
> + ret = ddw_property_create(&win64, DMA64_PROPNAME, create.liobn, win_addr,
> + page_shift, len, maps_whole_partition);
> if (ret) {
> dev_info(&dev->dev,
> "couldn't allocate property, property name, or value\n");
> @@ -1297,12 +1344,25 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> if (!window)
> goto out_prop_del;
>
> - ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
> - win64->value, tce_setrange_multi_pSeriesLP_walk);
> - if (ret) {
> - dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
> - dn, ret);
> - goto out_free_window;
> + if (maps_whole_partition) {
> + /* DDW maps the whole partition, so enable direct DMA mapping */
> + ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
> + win64->value, tce_setrange_multi_pSeriesLP_walk);
> + if (ret) {
> + dev_info(&dev->dev, "failed to map direct window for %pOF: %d\n",
> + dn, ret);
> + goto out_free_window;
> + }
> + } else {
> + /* New table for using DDW instead of the default DMA window */
> + if (iommu_table_update_window(&tbl, pci->phb->node, create.liobn,
> + win_addr, page_shift, 1UL << len))
> + goto out_free_window;
> +
> + set_iommu_table_base(&dev->dev, tbl);
> + WARN_ON(dev->dev.archdata.dma_offset >= SZ_4G);
What is this check for exactly? Why 4G, not >= 0, for example?
> + goto out_unlock;
> +
> }
>
> dev->dev.archdata.dma_offset = win_addr;
> @@ -1340,7 +1400,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>
> out_unlock:
> mutex_unlock(&direct_window_init_mutex);
> - return win64;
> + return win64 && maps_whole_partition;
> }
>
> static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
>
--
Alexey
^ permalink raw reply
* Re: kernel since 5.6 do not boot anymore on Apple PowerBook
From: Christophe Leroy @ 2020-08-24 5:17 UTC (permalink / raw)
To: Giuseppe Sacco, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <c707f59d379a51b83ba52e796d137887219c32fc.camel@sguazz.it>
Hello Giuseppe,
Le 22/08/2020 à 10:28, Giuseppe Sacco a écrit :
> Hello Christophe,
>
> Il giorno ven, 21/08/2020 alle 16.03 +0200, Christophe Leroy ha
> scritto:
> [...]
>>
>> You also said in a previous mail that your original issue also
>> happens
>> when CONFIG_VMAP_STACK is not selected. The above bug being linked
>> to
>> CONFIG_VMAP_STACK, maybe it would be easier to bisect with
>> CONFIG_VMAP_STACK unselected.
>
> I was wrong. Disabling CONFIG_VMAP_STACK led me to all "good" compile
> and bisect ended without finding the culprit commit.
>
> So, I started from scratch: I rebuilt HEAD and found that it does show
> the original problem I am facing, then I rebuilt it without
> CONFIG_VMAP_STACK and found that it does pass (fix?) the problem, since
> kernel continue booting, but then it stops with three Oops related to
> command systemd-udevd.
>
> You may find a video that displays the complete boot, vmlinux, config,
> and system.map files here:
>
The Oopses in the video are fixed in 5.9-rc2, see my response to your
other mail.
So now we know that your kernel doesn't boot when CONFIG_VMAP_STACK is set.
Can you remind the exact problem ?
One common problem with CONFIG_VMAP_STACK is when some drivers are
invalidly using buffers in stack for DMA.
Couldn't try with CONFIG_DEBUG_VIRTUAL (without CONFIG_VMAP_STACK) and
see if it triggers some warnings ?
Thanks
Christophe
^ permalink raw reply
* Re: Oops decoding help request
From: Christophe Leroy @ 2020-08-24 5:08 UTC (permalink / raw)
To: Giuseppe Sacco, linuxppc-dev
In-Reply-To: <45033654b9663c7b7d32991249195d66f56e2f1b.camel@sguazz.it>
Hi,
Le 23/08/2020 à 19:26, Giuseppe Sacco a écrit :
> Hello,
> I am not a kernel developer and I need much help in order to understand
> a kernel Oops (the first of a series of three Oops). It is:
This is a bug in the kernel. It is fixed in 5.9-rc2.
See the following commit:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/powerpc/mm/book3s32/mmu.c?h=v5.9-rc2&id=541cebb51f3422d4f2c6cb95c1e5cc3dcc9e5021
Christophe
>
> kernel tried to execute exec-protected page (f1020000) - exploit attempt? (uid: 0)
> BUG: Unable to handle kernel instruction fetch
> Faulting instruction address: 0xf1020000
> Oops: Kernel access of bad area, sig: 11 [#1]
> BE PAGE_SIZE=4K MMU=Hash PowerMac
> Modules linked in: crct10dif_generic (+) crct10dif_common drm_panel_orientation_quirks
> CPU: 0 PID: 71 Comm: systemd-udevd Not tainted 5.9.0-rc1+ #298
> NIP: f1020000 LR: c00053a4 CTR: f1020000
> REGS: c1c6dd50 TRAP: 0400 Not tainted (5.9.0-rc1+)
> MSR: 10009032 <EE,ME,IR,DR,RI> CR: 22222284 XER: 00000000
>
> GPR00: c0005390 c1c6de08 c1c6b400 00000000 00000cc0 00000008 ef6db038 00000001
> GPR08: 0000002e 00000000 00000000 00000000 22222284 00b6fb58 00000000 00000005
> GPR16: bff0d768 bff0d770 00000000 00000000 01032cc0 00000000 00b0b31f 01020960
> GPR24: 00000000 00b70954 010206c0 ef39f4a0 00000000 00a28380 f1020000 f10193a0
> NIP [f1020000] crct10dif_mod_init+0x0/0x60 [crct10dif_generic]
> LR [c00053a4] do_one_initcall+0x50/0x1f4
> Call Trace:
> [c1c6de08] [c0005390] do_one_initcall+0x3c/0x1f4 (unreliable)
> [c1c6de78] [c0102068] do_init_module+0x6c/0x27c
> [c1c6dea8] [c01053cc] sys_finit_module+0xc0/0x12c
> [c1c6df38] [c001c11c] ret_from_syscall+0x0/0x34
> --- interrupt: c01 at 0x7a7780
> LR = 0xa1bf64
> Instruction dump:
> XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
> XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX <7c0802a6> 90010004 60000000 9421fff0
> ---[ end trace 257a4bbda691894e ]---
>
> From what I understand, this is a problem in the init function of
> module crct10dif_generic jumping at address f1020000.
>
> I think I understand that f1020000 is an address for data and not for
> code. In fact it belongs to "vmalloc & ioremap" area of the virtual
> memory layout:
> * 0xffbee000..0xfffff000 : fixmap
> * 0xff400000..0xff800000 : highmem PTEs
> * 0xfda27000..0xff400000 : early ioremap
> * 0xf1000000..0xfda27000 : vmalloc & ioremap
>
> The init function is:
>
> 00000000 <init_module>:
> 0: 7c 08 02 a6 mflr r0
> 4: 90 01 00 04 stw r0,4(r1)
> 8: 48 00 00 01 bl 8 <init_module+0x8>
> c: 94 21 ff f0 stwu r1,-16(r1)
> 10: 7c 08 02 a6 mflr r0
> 14: 3c 60 00 00 lis r3,0
> 18: 90 01 00 14 stw r0,20(r1)
> 1c: 38 63 00 00 addi r3,r3,0
> 20: 80 01 00 14 lwz r0,20(r1)
> 24: 38 21 00 10 addi r1,r1,16
> 28: 7c 08 03 a6 mtlr r0
> 2c: 48 00 00 00 b 2c <init_module+0x2c>
>
> and its source code is:
>
> static int __init crct10dif_mod_init(void)
> {
> return crypto_register_shash(&alg);
> }
>
> This is what I am not understanding. The error message seems to imply
> that code jumps to an invalid address, so the problem would be that
> address of function crypto_register_shash is calculated wrongly.
>
> About stack addresses, please note that CONFIG_VMAP_STACK is not set.
>
> Is this a correct reasoning?
>
> Thank you very much,
> Giuseppe
>
^ permalink raw reply
* Re: [PATCH v1 08/10] powerpc/pseries/iommu: Add ddw_property_create() and refactor enable_ddw()
From: Alexey Kardashevskiy @ 2020-08-24 5:07 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Christophe Leroy, Joel Stanley,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200817234033.442511-9-leobras.c@gmail.com>
On 18/08/2020 09:40, Leonardo Bras wrote:
> Code used to create a ddw property that was previously scattered in
> enable_ddw() is now gathered in ddw_property_create(), which deals with
> allocation and filling the property, letting it ready for
> of_property_add(), which now occurs in sequence.
>
> This created an opportunity to reorganize the second part of enable_ddw():
>
> Without this patch enable_ddw() does, in order:
> kzalloc() property & members, create_ddw(), fill ddwprop inside property,
> ddw_list_add(), do tce_setrange_multi_pSeriesLP_walk in all memory,
> of_add_property().
>
> With this patch enable_ddw() does, in order:
> create_ddw(), ddw_property_create(), of_add_property(), ddw_list_add(),
> do tce_setrange_multi_pSeriesLP_walk in all memory.
>
> This change requires of_remove_property() in case anything fails after
> of_add_property(), but we get to do tce_setrange_multi_pSeriesLP_walk
> in all memory, which looks the most expensive operation, only if
> everything else succeeds.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/platforms/pseries/iommu.c | 97 +++++++++++++++-----------
> 1 file changed, 57 insertions(+), 40 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 4031127c9537..3a1ef02ad9d5 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -1123,6 +1123,31 @@ static void reset_dma_window(struct pci_dev *dev, struct device_node *par_dn)
> ret);
> }
>
> +static int ddw_property_create(struct property **ddw_win, const char *propname,
@propname is always the same, do you really want to pass it every time?
> + u32 liobn, u64 dma_addr, u32 page_shift, u32 window_shift)
> +{
> + struct dynamic_dma_window_prop *ddwprop;
> + struct property *win64;
> +
> + *ddw_win = win64 = kzalloc(sizeof(*win64), GFP_KERNEL);
> + if (!win64)
> + return -ENOMEM;
> +
> + win64->name = kstrdup(propname, GFP_KERNEL);
Not clear why "win64->name = DIRECT64_PROPNAME" would not work here, the
generic OF code does not try kfree() it but it is probably out of scope
here.
> + ddwprop = kzalloc(sizeof(*ddwprop), GFP_KERNEL);
> + win64->value = ddwprop;
> + win64->length = sizeof(*ddwprop);
> + if (!win64->name || !win64->value)
> + return -ENOMEM;
Up to 2 memory leaks here. I see the cleanup at "out_free_prop:" but
still looks fragile. Instead you could simply return win64 as the only
error possible here is -ENOMEM and returning NULL is equally good.
> +
> + ddwprop->liobn = cpu_to_be32(liobn);
> + ddwprop->dma_base = cpu_to_be64(dma_addr);
> + ddwprop->tce_shift = cpu_to_be32(page_shift);
> + ddwprop->window_shift = cpu_to_be32(window_shift);
> +
> + return 0;
> +}
> +
> /*
> * If the PE supports dynamic dma windows, and there is space for a table
> * that can map all pages in a linear offset, then setup such a table,
> @@ -1140,12 +1165,11 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> struct ddw_query_response query;
> struct ddw_create_response create;
> int page_shift;
> - u64 max_addr;
> + u64 max_addr, win_addr;
> struct device_node *dn;
> u32 ddw_avail[DDW_APPLICABLE_SIZE];
> struct direct_window *window;
> - struct property *win64;
> - struct dynamic_dma_window_prop *ddwprop;
> + struct property *win64 = NULL;
> struct failed_ddw_pdn *fpdn;
> bool default_win_removed = false;
>
> @@ -1244,38 +1268,34 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> goto out_failed;
> }
> len = order_base_2(max_addr);
> - win64 = kzalloc(sizeof(struct property), GFP_KERNEL);
> - if (!win64) {
> - dev_info(&dev->dev,
> - "couldn't allocate property for 64bit dma window\n");
> +
> + ret = create_ddw(dev, ddw_avail, &create, page_shift, len);
> + if (ret != 0)
It is usually just "if (ret)"
> goto out_failed;
> - }
> - win64->name = kstrdup(DIRECT64_PROPNAME, GFP_KERNEL);
> - win64->value = ddwprop = kmalloc(sizeof(*ddwprop), GFP_KERNEL);
> - win64->length = sizeof(*ddwprop);
> - if (!win64->name || !win64->value) {
> +
> + dev_dbg(&dev->dev, "created tce table LIOBN 0x%x for %pOF\n",
> + create.liobn, dn);
> +
> + win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
> + ret = ddw_property_create(&win64, DIRECT64_PROPNAME, create.liobn, win_addr,
> + page_shift, len);
> + if (ret) {
> dev_info(&dev->dev,
> - "couldn't allocate property name and value\n");
> + "couldn't allocate property, property name, or value\n");
> goto out_free_prop;
> }
>
> - ret = create_ddw(dev, ddw_avail, &create, page_shift, len);
> - if (ret != 0)
> + ret = of_add_property(pdn, win64);
> + if (ret) {
> + dev_err(&dev->dev, "unable to add dma window property for %pOF: %d",
> + pdn, ret);
> goto out_free_prop;
> -
> - ddwprop->liobn = cpu_to_be32(create.liobn);
> - ddwprop->dma_base = cpu_to_be64(((u64)create.addr_hi << 32) |
> - create.addr_lo);
> - ddwprop->tce_shift = cpu_to_be32(page_shift);
> - ddwprop->window_shift = cpu_to_be32(len);
> -
> - dev_dbg(&dev->dev, "created tce table LIOBN 0x%x for %pOF\n",
> - create.liobn, dn);
> + }
>
> /* Add new window to existing DDW list */
> - window = ddw_list_add(pdn, ddwprop);
> + window = ddw_list_add(pdn, win64->value);
> if (!window)
> - goto out_clear_window;
> + goto out_prop_del;
>
> ret = walk_system_ram_range(0, memblock_end_of_DRAM() >> PAGE_SHIFT,
> win64->value, tce_setrange_multi_pSeriesLP_walk);
> @@ -1285,14 +1305,7 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> goto out_free_window;
> }
>
> - ret = of_add_property(pdn, win64);
> - if (ret) {
> - dev_err(&dev->dev, "unable to add dma window property for %pOF: %d",
> - pdn, ret);
> - goto out_free_window;
> - }
> -
> - dev->dev.archdata.dma_offset = be64_to_cpu(ddwprop->dma_base);
> + dev->dev.archdata.dma_offset = win_addr;
> goto out_unlock;
>
> out_free_window:
> @@ -1302,14 +1315,18 @@ static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>
> kfree(window);
>
> -out_clear_window:
> - remove_ddw(pdn, true);
> +out_prop_del:
> + of_remove_property(pdn, win64);
>
> out_free_prop:
> - kfree(win64->name);
> - kfree(win64->value);
> - kfree(win64);
> - win64 = NULL;
> + if (win64) {
> + kfree(win64->name);
> + kfree(win64->value);
> + kfree(win64);
> + win64 = NULL;
> + }
> +
> + remove_ddw(pdn, true);
>
> out_failed:
> if (default_win_removed)
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v1 06/10] powerpc/pseries/iommu: Add ddw_list_add() helper
From: Alexey Kardashevskiy @ 2020-08-24 3:46 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Christophe Leroy, Joel Stanley,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200817234033.442511-7-leobras.c@gmail.com>
On 18/08/2020 09:40, Leonardo Bras wrote:
> There are two functions adding DDW to the direct_window_list in a
> similar way, so create a ddw_list_add() to avoid duplicity and
> simplify those functions.
>
> Also, on enable_ddw(), add list_del() on out_free_window to allow
> removing the window from list if any error occurs.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/platforms/pseries/iommu.c | 42 ++++++++++++++++----------
> 1 file changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index 39617ce0ec83..fcdefcc0f365 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -872,6 +872,24 @@ static u64 find_existing_ddw(struct device_node *pdn)
> return dma_addr;
> }
>
> +static struct direct_window *ddw_list_add(struct device_node *pdn,
> + const struct dynamic_dma_window_prop *dma64)
> +{
> + struct direct_window *window;
> +
> + window = kzalloc(sizeof(*window), GFP_KERNEL);
> + if (!window)
> + return NULL;
> +
> + window->device = pdn;
> + window->prop = dma64;
> + spin_lock(&direct_window_list_lock);
> + list_add(&window->list, &direct_window_list);
> + spin_unlock(&direct_window_list_lock);
> +
> + return window;
> +}
> +
> static int find_existing_ddw_windows(void)
> {
> int len;
> @@ -887,18 +905,11 @@ static int find_existing_ddw_windows(void)
> if (!direct64)
> continue;
>
> - window = kzalloc(sizeof(*window), GFP_KERNEL);
> - if (!window || len < sizeof(struct dynamic_dma_window_prop)) {
> + window = ddw_list_add(pdn, direct64);
> + if (!window || len < sizeof(*direct64)) {
Since you are touching this code, it looks like the "len <
sizeof(*direct64)" part should go above to "if (!direct64)".
> kfree(window);
> remove_ddw(pdn, true);
> - continue;
> }
> -
> - window->device = pdn;
> - window->prop = direct64;
> - spin_lock(&direct_window_list_lock);
> - list_add(&window->list, &direct_window_list);
> - spin_unlock(&direct_window_list_lock);
> }
>
> return 0;
> @@ -1261,7 +1272,8 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> dev_dbg(&dev->dev, "created tce table LIOBN 0x%x for %pOF\n",
> create.liobn, dn);
>
> - window = kzalloc(sizeof(*window), GFP_KERNEL);
> + /* Add new window to existing DDW list */
The comment seems to duplicate what the ddw_list_add name already suggests.
> + window = ddw_list_add(pdn, ddwprop);
> if (!window)
> goto out_clear_window;
>
> @@ -1280,16 +1292,14 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> goto out_free_window;
> }
>
> - window->device = pdn;
> - window->prop = ddwprop;
> - spin_lock(&direct_window_list_lock);
> - list_add(&window->list, &direct_window_list);
> - spin_unlock(&direct_window_list_lock);
I'd leave these 3 lines here and in find_existing_ddw_windows() (which
would make ddw_list_add -> ddw_prop_alloc). In general you want to have
less stuff to do on the failure path. kmalloc may fail and needs kfree
but you can safely delay list_add (which cannot fail) and avoid having
the lock help twice in the same function (one of them is hidden inside
ddw_list_add).
Not sure if this change is really needed after all. Thanks,
> -
> dma_addr = be64_to_cpu(ddwprop->dma_base);
> goto out_unlock;
>
> out_free_window:
> + spin_lock(&direct_window_list_lock);
> + list_del(&window->list);
> + spin_unlock(&direct_window_list_lock);
> +
> kfree(window);
>
> out_clear_window:
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v1 07/10] powerpc/pseries/iommu: Allow DDW windows starting at 0x00
From: Alexey Kardashevskiy @ 2020-08-24 3:44 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Christophe Leroy, Joel Stanley,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200817234033.442511-8-leobras.c@gmail.com>
On 18/08/2020 09:40, Leonardo Bras wrote:
> enable_ddw() currently returns the address of the DMA window, which is
> considered invalid if has the value 0x00.
>
> Also, it only considers valid an address returned from find_existing_ddw
> if it's not 0x00.
>
> Changing this behavior makes sense, given the users of enable_ddw() only
> need to know if direct mapping is possible. It can also allow a DMA window
> starting at 0x00 to be used.
>
> This will be helpful for using a DDW with indirect mapping, as the window
> address will be different than 0x00, but it will not map the whole
> partition.
>
> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
> ---
> arch/powerpc/platforms/pseries/iommu.c | 30 ++++++++++++--------------
> 1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index fcdefcc0f365..4031127c9537 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -852,24 +852,25 @@ static void remove_ddw(struct device_node *np, bool remove_prop)
> np, ret);
> }
>
> -static u64 find_existing_ddw(struct device_node *pdn)
> +static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
> {
> struct direct_window *window;
> const struct dynamic_dma_window_prop *direct64;
> - u64 dma_addr = 0;
> + bool found = false;
>
> spin_lock(&direct_window_list_lock);
> /* check if we already created a window and dupe that config if so */
> list_for_each_entry(window, &direct_window_list, list) {
> if (window->device == pdn) {
> direct64 = window->prop;
> - dma_addr = be64_to_cpu(direct64->dma_base);
> + *dma_addr = be64_to_cpu(direct64->dma_base);
> + found = true;
> break;
> }
> }
> spin_unlock(&direct_window_list_lock);
>
> - return dma_addr;
> + return found;
> }
>
> static struct direct_window *ddw_list_add(struct device_node *pdn,
> @@ -1131,15 +1132,15 @@ static void reset_dma_window(struct pci_dev *dev, struct device_node *par_dn)
> * pdn: the parent pe node with the ibm,dma_window property
> * Future: also check if we can remap the base window for our base page size
> *
> - * returns the dma offset for use by the direct mapped DMA code.
> + * returns true if can map all pages (direct mapping), false otherwise..
> */
> -static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> +static bool enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> {
> int len, ret;
> struct ddw_query_response query;
> struct ddw_create_response create;
> int page_shift;
> - u64 dma_addr, max_addr;
> + u64 max_addr;
> struct device_node *dn;
> u32 ddw_avail[DDW_APPLICABLE_SIZE];
> struct direct_window *window;
> @@ -1150,8 +1151,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>
> mutex_lock(&direct_window_init_mutex);
>
> - dma_addr = find_existing_ddw(pdn);
> - if (dma_addr != 0)
> + if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset))
> goto out_unlock;
>
> /*
> @@ -1292,7 +1292,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> goto out_free_window;
> }
>
> - dma_addr = be64_to_cpu(ddwprop->dma_base);
> + dev->dev.archdata.dma_offset = be64_to_cpu(ddwprop->dma_base);
Do not you need the same chunk in the find_existing_ddw() case above as
well? Thanks,
> goto out_unlock;
>
> out_free_window:
> @@ -1309,6 +1309,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
> kfree(win64->name);
> kfree(win64->value);
> kfree(win64);
> + win64 = NULL;
>
> out_failed:
> if (default_win_removed)
> @@ -1322,7 +1323,7 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn)
>
> out_unlock:
> mutex_unlock(&direct_window_init_mutex);
> - return dma_addr;
> + return win64;
> }
>
> static void pci_dma_dev_setup_pSeriesLP(struct pci_dev *dev)
> @@ -1401,11 +1402,8 @@ static bool iommu_bypass_supported_pSeriesLP(struct pci_dev *pdev, u64 dma_mask)
> break;
> }
>
> - if (pdn && PCI_DN(pdn)) {
> - pdev->dev.archdata.dma_offset = enable_ddw(pdev, pdn);
> - if (pdev->dev.archdata.dma_offset)
> - return true;
> - }
> + if (pdn && PCI_DN(pdn))
> + return enable_ddw(pdev, pdn);
>
> return false;
> }
>
--
Alexey
^ permalink raw reply
* Re: [PATCH] powerpc/prom_init: Check display props exist before enabling btext
From: Michael Ellerman @ 2020-08-24 3:16 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev
In-Reply-To: <b0b2c5ba-4b3a-f125-d7f7-49822489c923@ozlabs.ru>
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> On 21/08/2020 20:34, Michael Ellerman wrote:
>> It's possible to enable CONFIG_PPC_EARLY_DEBUG_BOOTX for a pseries
>> kernel (maybe it shouldn't be), which is then booted with qemu/slof.
>
>
> CONFIG_BOOTX_TEXT=y
> CONFIG_PPC_EARLY_DEBUG=y
> CONFIG_PPC_EARLY_DEBUG_BOOTX=y
>
> this does not crash my VM. The changed chunk is sitting under "if
> (prom_getprop(node, "linux,boot-display", NULL, 0)" and I cannot find
> what creates this property - it is neither slof/grub/qemu, unlikely that
> it is phyp so it must be this one:
>
> arch/powerpc/platforms/powermac/bootx_init.c|244|
> bootx_dt_add_string("linux,boot-display", mem_end);
It's in prom_init.c:
static void __init prom_init_stdout(void)
{
...
stdout_node = call_prom("instance-to-package", 1, 1, prom.stdout);
if (stdout_node != PROM_ERROR) {
val = cpu_to_be32(stdout_node);
/* If it's a display, note it */
memset(type, 0, sizeof(type));
prom_getprop(stdout_node, "device_type", type, sizeof(type));
if (prom_strcmp(type, "display") == 0)
prom_setprop(stdout_node, path, "linux,boot-display", NULL, 0);
}
}
> which is powermac and not pseries. Or may be that pmac firmware.
>
> Where did you see this crash?
Qemu pseries either TCG or KVM with eg:
$ qemu-system-ppc64 -M pseries -cpu POWER8 -m 1G -kernel build~/vmlinux
>> But if you do that the kernel crashes in draw_byte(), with a DAR
>> pointing somewhere near INT_MAX.
>>
>> Adding some debug to prom_init we see that we're not able to read the
>> "address" property from OF, so we're just using whatever junk value
>> was on the stack.
>>
>> So check the properties can be read properly from OF, if not we bail
>> out before initialising btext, which avoids the crash.
>
> This is a right thing any way, just the commit log is confusing.
>
> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Thanks.
cheers
^ 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