* [PATCH net v3] ibmvnic fix NULL tx_pools and rx_tools issue at do_reset
From: Dany Madden @ 2020-08-25 17:26 UTC (permalink / raw)
To: davem; +Cc: Dany Madden, netdev, Mingming Cao, linuxppc-dev
From: Mingming Cao <mmc@linux.vnet.ibm.com>
At the time of do_rest, ibmvnic tries to re-initalize the tx_pools
and rx_pools to avoid re-allocating the long term buffer. However
there is a window inside do_reset that the tx_pools and
rx_pools were freed before re-initialized making it possible to deference
null pointers.
This patch fix this issue by always check the tx_pool
and rx_pool are not NULL after ibmvnic_login. If so, re-allocating
the pools. This will avoid getting into calling reset_tx/rx_pools with
NULL adapter tx_pools/rx_pools pointer. Also add null pointer check in
reset_tx_pools and reset_rx_pools to safe handle NULL pointer case.
Signed-off-by: Mingming Cao <mmc@linux.vnet.ibm.com>
Signed-off-by: Dany Madden <drt@linux.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5afb3c9c52d2..d3a774331afc 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -479,6 +479,9 @@ static int reset_rx_pools(struct ibmvnic_adapter *adapter)
int i, j, rc;
u64 *size_array;
+ if (!adapter->rx_pool)
+ return -1;
+
size_array = (u64 *)((u8 *)(adapter->login_rsp_buf) +
be32_to_cpu(adapter->login_rsp_buf->off_rxadd_buff_size));
@@ -649,6 +652,9 @@ static int reset_tx_pools(struct ibmvnic_adapter *adapter)
int tx_scrqs;
int i, rc;
+ if (!adapter->tx_pool)
+ return -1;
+
tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
for (i = 0; i < tx_scrqs; i++) {
rc = reset_one_tx_pool(adapter, &adapter->tso_pool[i]);
@@ -2011,7 +2017,10 @@ static int do_reset(struct ibmvnic_adapter *adapter,
adapter->req_rx_add_entries_per_subcrq !=
old_num_rx_slots ||
adapter->req_tx_entries_per_subcrq !=
- old_num_tx_slots) {
+ old_num_tx_slots ||
+ !adapter->rx_pool ||
+ !adapter->tso_pool ||
+ !adapter->tx_pool) {
release_rx_pools(adapter);
release_tx_pools(adapter);
release_napi(adapter);
@@ -2024,10 +2033,14 @@ static int do_reset(struct ibmvnic_adapter *adapter,
} else {
rc = reset_tx_pools(adapter);
if (rc)
+ netdev_dbg(adapter->netdev, "reset tx pools failed (%d)\n",
+ rc);
goto out;
rc = reset_rx_pools(adapter);
if (rc)
+ netdev_dbg(adapter->netdev, "reset rx pools failed (%d)\n",
+ rc);
goto out;
}
ibmvnic_disable_irqs(adapter);
--
2.18.2
^ permalink raw reply related
* Re: fsl_espi errors on v5.7.15
From: Chris Packham @ 2020-08-25 22:22 UTC (permalink / raw)
To: Heiner Kallweit, broonie@kernel.org, mpe@ellerman.id.au,
benh@kernel.crashing.org, paulus@samba.org
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
linux-spi@vger.kernel.org
In-Reply-To: <f97dc3af-d1f0-6974-ec2d-ace8d7e73993@gmail.com>
On 25/08/20 7:22 pm, Heiner Kallweit wrote:
<snip>
> I've been staring at spi-fsl-espi.c for while now and I think I've
>> identified a couple of deficiencies that may or may not be related to my
>> issue.
>>
>> First I think the 'Transfer done but SPIE_DON isn't set' message can be
>> generated spuriously. In fsl_espi_irq() we read the ESPI_SPIE register.
>> We also write back to it to clear the current events. We re-read it in
>> fsl_espi_cpu_irq() and complain when SPIE_DON is not set. But we can
>> naturally end up in that situation if we're doing a large read. Consider
>> the messages for reading a block of data from a spi-nor chip
>>
>> tx = READ_OP + ADDR
>> rx = data
>>
>> We setup the transfer and pump out the tx_buf. The first interrupt goes
>> off and ESPI_SPIE has SPIM_DON and SPIM_RXT set. We empty the rx fifo,
>> clear ESPI_SPIE and wait for the next interrupt. The next interrupt
>> fires and this time we have ESPI_SPIE with just SPIM_RXT set. This
>> continues until we've received all the data and we finish with ESPI_SPIE
>> having only SPIM_RXT set. When we re-read it we complain that SPIE_DON
>> isn't set.
>>
>> The other deficiency is that we only get an interrupt when the amount of
>> data in the rx fifo is above FSL_ESPI_RXTHR. If there are fewer than
>> FSL_ESPI_RXTHR left to be received we will never pull them out of the fifo.
>>
> SPIM_DON will trigger an interrupt once the last characters have been
> transferred, and read the remaining characters from the FIFO.
The T2080RM that I have says the following about the DON bit
"Last character was transmitted. The last character was transmitted and
a new command can be written for the next frame."
That does at least seem to fit with my assertion that it's all about the
TX direction. But the fact that it doesn't happen all the time throws
some doubt on it.
> I think the reason I'm seeing some variability is because of how fast
>> (or slow) the interrupts get processed and how fast the spi-nor chip can
>> fill the CPUs rx fifo.
>>
> To rule out timing issues at high bus frequencies I initially asked
> for re-testing at lower frequencies. If you e.g. limit the bus to 1 MHz
> or even less, then timing shouldn't be an issue.
Yes I've currently got spi-max-frequency = <1000000>; in my dts. I would
also expect a slower frequency would fit my "DON is for TX" narrative.
> Last relevant functional changes have been done almost 4 years ago.
> And yours is the first such report I see. So question is what could be so
> special with your setup that it seems you're the only one being affected.
> The scenarios you describe are standard, therefore much more people
> should be affected in case of a driver bug.
Agreed. But even on my hardware (which may have a latent issue despite
being in the field for going on 5 years) the issue only triggers under
some fairly specific circumstances.
> You said that kernel config impacts how frequently the issue happens.
> Therefore question is what's the diff in kernel config, and how could
> the differences be related to SPI.
It did seem to be somewhat random. Things like CONFIG_PREEMPT have an
impact but every time I found something that seemed to be having an
impact I've been able to disprove it. I actually think its about how
busy the system is which may or may not affect when we get round to
processing the interrupts.
I have managed to get the 'Transfer done but SPIE_DON isn't set!' to
occur on the T2080RDB.
I've had to add the following to expose the environment as a mtd partition
diff --git a/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
b/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
index ff87e67c70da..fbf95fc1fd68 100644
--- a/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
@@ -116,6 +116,15 @@ flash@0 {
compatible = "micron,n25q512ax3",
"jedec,spi-nor";
reg = <0>;
spi-max-frequency = <10000000>; /*
input clock */
+
+ partition@u-boot {
+ reg = <0x00000000 0x00100000>;
+ label = "u-boot";
+ };
+ partition@u-boot-env {
+ reg = <0x00100000 0x00010000>;
+ label = "u-boot-env";
+ };
};
};
And I'm using the following script to poke at the environment (warning
if anyone does try this and the bug hits it can render your u-boot
environment invalid).
cat flash/fw_env_test.sh
#!/bin/sh
generate_fw_env_config()
{
cat /proc/mtd | sed 's/[:"]//g' | while read dev size erasesize name ; do
echo "$dev $size $erasesize $name"
[ "$name" = "u-boot-env" ] && echo "/dev/$dev 0x0000 0x2000
$erasesize" >/flash/fw_env.config
done
}
cycles=10
[ $# -ge 1 ] && cycles=$1
generate_fw_env_config
fw_printenv -c /flash/fw_env.config
dmesg -c >/dev/null
x=0
while [ $x -lt $cycles ]; do
fw_printenv -c /flash/fw_env.config >/dev/null || break
fw_setenv -c /flash/fw_env.config foo $RANDOM || break;
dmesg -c | grep -q fsl_espi && break;
let x=x+1
done
echo "Ran $x cycles"
^ permalink raw reply related
* Please apply commit 0828137e8f16 ("powerpc/64s: Don't init FSCR_DSCR in __init_FSCR()") to v4.14.y, v4.19.y, v5.4.y, v5.7.y
From: Thadeu Lima de Souza Cascardo @ 2020-08-25 22:44 UTC (permalink / raw)
To: stable; +Cc: sashal, linuxppc-dev, gregkh
After commit 912c0a7f2b5daa3cbb2bc10f303981e493de73bd ("powerpc/64s: Save FSCR
to init_task.thread.fscr after feature init"), which has been applied to the
referred branches, when userspace sets the user DSCR MSR, it won't be inherited
or restored during context switch, because the facility unavailable interrupt
won't trigger.
Applying 0828137e8f16721842468e33df0460044a0c588b ("powerpc/64s: Don't init
FSCR_DSCR in __init_FSCR()") will fix it.
Cascardo.
^ permalink raw reply
* Re: [RFT][PATCH 0/7] Avoid overflow at boundary_size
From: Nicolin Chen @ 2020-08-25 23:19 UTC (permalink / raw)
To: Niklas Schnelle
Cc: linux-ia64, James.Bottomley, paulus, hpa, sparclinux, hch, sfr,
deller, x86, borntraeger, mingo, mattst88, fenghua.yu, gor,
linux-s390, hca, ink, tglx, gerald.schaefer, rth, tony.luck,
linux-parisc, linux-kernel, linux-alpha, bp, linuxppc-dev, davem
In-Reply-To: <4321af30-9554-6897-5281-05afd88f2631@linux.ibm.com>
Hi Niklas,
On Tue, Aug 25, 2020 at 12:16:27PM +0200, Niklas Schnelle wrote:
> On 8/21/20 1:19 AM, Nicolin Chen wrote:
> > We are expending the default DMA segmentation boundary to its
> > possible maximum value (ULONG_MAX) to indicate that a device
> > doesn't specify a boundary limit. So all dma_get_seg_boundary
> > callers should take a precaution with the return values since
> > it would easily get overflowed.
> >
> > I scanned the entire kernel tree for all the existing callers
> > and found that most of callers may get overflowed in two ways:
> > either "+ 1" or passing it to ALIGN() that does "+ mask".
> >
> > According to kernel defines:
> > #define ALIGN_MASK(x, mask) (((x) + (mask)) & ~(mask))
> > #define ALIGN(x, a) ALIGN_MASK(x, (typeof(x))(a) - 1)
> >
> > We can simplify the logic here:
> > ALIGN(boundary + 1, 1 << shift) >> shift
> > = ALIGN_MASK(b + 1, (1 << s) - 1) >> s
> > = {[b + 1 + (1 << s) - 1] & ~[(1 << s) - 1]} >> s
> > = [b + 1 + (1 << s) - 1] >> s
> > = [b + (1 << s)] >> s
> > = (b >> s) + 1
> >
> > So this series of patches fix the potential overflow with this
> > overflow-free shortcut.
> haven't seen any other feedback from other maintainers,
I am wondering this too...whether I sent correctly or not.
> so I guess you will resend this?
Do I need to? Though I won't mind doing so if it's necessary..
> On first glance it seems to make sense.
> I'm a little confused why it is only a "potential overflow"
> while this part
>
> "We are expending the default DMA segmentation boundary to its
> possible maximum value (ULONG_MAX) to indicate that a device
> doesn't specify a boundary limit"
>
> sounds to me like ULONG_MAX is actually used, does that
> mean there are currently no devices which do not specify a
> boundary limit?
Sorry for the confusion. We actually applied ULONG_MAX change
last week but reverted it right after, due to a bug report at
one of these "potential" overflows. So at this moment the top
of the tree doesn't set default boundary to ULONG_MAX yet.
Thanks
Nic
^ permalink raw reply
* Re: [PATCH net v3] ibmvnic fix NULL tx_pools and rx_tools issue at do_reset
From: David Miller @ 2020-08-26 0:31 UTC (permalink / raw)
To: drt; +Cc: netdev, mmc, linuxppc-dev
In-Reply-To: <20200825172641.806912-1-drt@linux.ibm.com>
From: Dany Madden <drt@linux.ibm.com>
Date: Tue, 25 Aug 2020 13:26:41 -0400
> From: Mingming Cao <mmc@linux.vnet.ibm.com>
>
> At the time of do_rest, ibmvnic tries to re-initalize the tx_pools
> and rx_pools to avoid re-allocating the long term buffer. However
> there is a window inside do_reset that the tx_pools and
> rx_pools were freed before re-initialized making it possible to deference
> null pointers.
>
> This patch fix this issue by always check the tx_pool
> and rx_pool are not NULL after ibmvnic_login. If so, re-allocating
> the pools. This will avoid getting into calling reset_tx/rx_pools with
> NULL adapter tx_pools/rx_pools pointer. Also add null pointer check in
> reset_tx_pools and reset_rx_pools to safe handle NULL pointer case.
>
> Signed-off-by: Mingming Cao <mmc@linux.vnet.ibm.com>
> Signed-off-by: Dany Madden <drt@linux.ibm.com>
Applied, but:
> + if (!adapter->rx_pool)
> + return -1;
> +
This driver has poor error code usage, it's a random mix of hypervisor
error codes, normal error codes like -EINVAL, and internal error codes.
Sometimes used all in the same function.
For example:
static int ibmvnic_send_crq(struct ibmvnic_adapter *adapter,
union ibmvnic_crq *crq)
...
if (!adapter->crq.active &&
crq->generic.first != IBMVNIC_CRQ_INIT_CMD) {
dev_warn(dev, "Invalid request detected while CRQ is inactive, possible device state change during reset\n");
return -EINVAL;
}
...
rc = plpar_hcall_norets(H_SEND_CRQ, ua,
cpu_to_be64(u64_crq[0]),
cpu_to_be64(u64_crq[1]));
if (rc) {
if (rc == H_CLOSED) {
...
return rc;
So obviously this function returns a mix of negative erro codes
and Hypervisor codes such as H_CLOSED.
And stuff like:
rc = __ibmvnic_open(netdev);
if (rc)
return IBMVNIC_OPEN_FAILED;
^ permalink raw reply
* Re: [PATCH net v3] ibmvnic fix NULL tx_pools and rx_tools issue at do_reset
From: Mingming Cao @ 2020-08-26 1:14 UTC (permalink / raw)
To: David Miller; +Cc: drt, netdev, linuxppc-dev
In-Reply-To: <20200825.173104.1541443546408114168.davem@davemloft.net>
> On Aug 25, 2020, at 5:31 PM, David Miller <davem@davemloft.net> wrote:
>
> From: Dany Madden <drt@linux.ibm.com>
> Date: Tue, 25 Aug 2020 13:26:41 -0400
>
>> From: Mingming Cao <mmc@linux.vnet.ibm.com>
>>
>> At the time of do_rest, ibmvnic tries to re-initalize the tx_pools
>> and rx_pools to avoid re-allocating the long term buffer. However
>> there is a window inside do_reset that the tx_pools and
>> rx_pools were freed before re-initialized making it possible to deference
>> null pointers.
>>
>> This patch fix this issue by always check the tx_pool
>> and rx_pool are not NULL after ibmvnic_login. If so, re-allocating
>> the pools. This will avoid getting into calling reset_tx/rx_pools with
>> NULL adapter tx_pools/rx_pools pointer. Also add null pointer check in
>> reset_tx_pools and reset_rx_pools to safe handle NULL pointer case.
>>
>> Signed-off-by: Mingming Cao <mmc@linux.vnet.ibm.com>
>> Signed-off-by: Dany Madden <drt@linux.ibm.com>
>
> Applied, but:
>
>> + if (!adapter->rx_pool)
>> + return -1;
>> +
>
> This driver has poor error code usage, it's a random mix of hypervisor
> error codes, normal error codes like -EINVAL, and internal error codes.
> Sometimes used all in the same function.
>
Agree need to improve. For this patch/fix, -1 is chosen to follow other part of the driver that check NULL pointer and return -1 . We should go through all of -1 cases and replace with normal proper error code. That should be a seperate patch.
> For example:
>
> static int ibmvnic_send_crq(struct ibmvnic_adapter *adapter,
> union ibmvnic_crq *crq)
> ...
> if (!adapter->crq.active &&
> crq->generic.first != IBMVNIC_CRQ_INIT_CMD) {
> dev_warn(dev, "Invalid request detected while CRQ is inactive, possible device state change during reset\n");
> return -EINVAL;
> }
> ...
> rc = plpar_hcall_norets(H_SEND_CRQ, ua,
> cpu_to_be64(u64_crq[0]),
> cpu_to_be64(u64_crq[1]));
>
> if (rc) {
> if (rc == H_CLOSED) {
> ...
> return rc;
>
> So obviously this function returns a mix of negative erro codes
> and Hypervisor codes such as H_CLOSED.
>
> And stuff like:
>
> rc = __ibmvnic_open(netdev);
> if (rc)
> return IBMVNIC_OPEN_FAILED;
Agree.
Mingming
^ permalink raw reply
* Re: fsl_espi errors on v5.7.15
From: Chris Packham @ 2020-08-26 1:48 UTC (permalink / raw)
To: Heiner Kallweit, broonie@kernel.org, mpe@ellerman.id.au,
benh@kernel.crashing.org, paulus@samba.org
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
linux-spi@vger.kernel.org
In-Reply-To: <31e91237-4f87-66da-dac3-2304779e5749@alliedtelesis.co.nz>
On 26/08/20 10:22 am, Chris Packham wrote:
> On 25/08/20 7:22 pm, Heiner Kallweit wrote:
>
> <snip>
>> I've been staring at spi-fsl-espi.c for while now and I think I've
>>> identified a couple of deficiencies that may or may not be related
>>> to my
>>> issue.
>>>
>>> First I think the 'Transfer done but SPIE_DON isn't set' message can be
>>> generated spuriously. In fsl_espi_irq() we read the ESPI_SPIE register.
>>> We also write back to it to clear the current events. We re-read it in
>>> fsl_espi_cpu_irq() and complain when SPIE_DON is not set. But we can
>>> naturally end up in that situation if we're doing a large read.
>>> Consider
>>> the messages for reading a block of data from a spi-nor chip
>>>
>>> tx = READ_OP + ADDR
>>> rx = data
>>>
>>> We setup the transfer and pump out the tx_buf. The first interrupt goes
>>> off and ESPI_SPIE has SPIM_DON and SPIM_RXT set. We empty the rx fifo,
>>> clear ESPI_SPIE and wait for the next interrupt. The next interrupt
>>> fires and this time we have ESPI_SPIE with just SPIM_RXT set. This
>>> continues until we've received all the data and we finish with
>>> ESPI_SPIE
>>> having only SPIM_RXT set. When we re-read it we complain that SPIE_DON
>>> isn't set.
>>>
>>> The other deficiency is that we only get an interrupt when the
>>> amount of
>>> data in the rx fifo is above FSL_ESPI_RXTHR. If there are fewer than
>>> FSL_ESPI_RXTHR left to be received we will never pull them out of
>>> the fifo.
>>>
>> SPIM_DON will trigger an interrupt once the last characters have been
>> transferred, and read the remaining characters from the FIFO.
>
> The T2080RM that I have says the following about the DON bit
>
> "Last character was transmitted. The last character was transmitted
> and a new command can be written for the next frame."
>
> That does at least seem to fit with my assertion that it's all about
> the TX direction. But the fact that it doesn't happen all the time
> throws some doubt on it.
>
>> I think the reason I'm seeing some variability is because of how fast
>>> (or slow) the interrupts get processed and how fast the spi-nor chip
>>> can
>>> fill the CPUs rx fifo.
>>>
>> To rule out timing issues at high bus frequencies I initially asked
>> for re-testing at lower frequencies. If you e.g. limit the bus to 1 MHz
>> or even less, then timing shouldn't be an issue.
> Yes I've currently got spi-max-frequency = <1000000>; in my dts. I
> would also expect a slower frequency would fit my "DON is for TX"
> narrative.
>> Last relevant functional changes have been done almost 4 years ago.
>> And yours is the first such report I see. So question is what could
>> be so
>> special with your setup that it seems you're the only one being
>> affected.
>> The scenarios you describe are standard, therefore much more people
>> should be affected in case of a driver bug.
> Agreed. But even on my hardware (which may have a latent issue despite
> being in the field for going on 5 years) the issue only triggers under
> some fairly specific circumstances.
>> You said that kernel config impacts how frequently the issue happens.
>> Therefore question is what's the diff in kernel config, and how could
>> the differences be related to SPI.
>
> It did seem to be somewhat random. Things like CONFIG_PREEMPT have an
> impact but every time I found something that seemed to be having an
> impact I've been able to disprove it. I actually think its about how
> busy the system is which may or may not affect when we get round to
> processing the interrupts.
>
> I have managed to get the 'Transfer done but SPIE_DON isn't set!' to
> occur on the T2080RDB.
>
> I've had to add the following to expose the environment as a mtd
> partition
>
> diff --git a/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
> b/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
> index ff87e67c70da..fbf95fc1fd68 100644
> --- a/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
> @@ -116,6 +116,15 @@ flash@0 {
> compatible = "micron,n25q512ax3",
> "jedec,spi-nor";
> reg = <0>;
> spi-max-frequency = <10000000>; /*
> input clock */
> +
> + partition@u-boot {
> + reg = <0x00000000 0x00100000>;
> + label = "u-boot";
> + };
> + partition@u-boot-env {
> + reg = <0x00100000 0x00010000>;
> + label = "u-boot-env";
> + };
> };
> };
>
> And I'm using the following script to poke at the environment (warning
> if anyone does try this and the bug hits it can render your u-boot
> environment invalid).
>
> cat flash/fw_env_test.sh
> #!/bin/sh
>
> generate_fw_env_config()
> {
> cat /proc/mtd | sed 's/[:"]//g' | while read dev size erasesize name
> ; do
> echo "$dev $size $erasesize $name"
> [ "$name" = "u-boot-env" ] && echo "/dev/$dev 0x0000 0x2000
> $erasesize" >/flash/fw_env.config
> done
> }
>
> cycles=10
> [ $# -ge 1 ] && cycles=$1
>
> generate_fw_env_config
>
> fw_printenv -c /flash/fw_env.config
>
> dmesg -c >/dev/null
> x=0
> while [ $x -lt $cycles ]; do
> fw_printenv -c /flash/fw_env.config >/dev/null || break
> fw_setenv -c /flash/fw_env.config foo $RANDOM || break;
> dmesg -c | grep -q fsl_espi && break;
> let x=x+1
> done
>
> echo "Ran $x cycles"
I've also now seen the RX FIFO not empty error on the T2080RDB
fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
With my current workaround of emptying the RX FIFO. It seems survivable.
Interestingly it only ever seems to be 1 extra byte in the RX FIFO and
it seems to be after either a READ_SR or a READ_FSR.
fsl_espi ffe110000.spi: tx 70
fsl_espi ffe110000.spi: rx 03
fsl_espi ffe110000.spi: Extra RX 00
fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
fsl_espi ffe110000.spi: tx 05
fsl_espi ffe110000.spi: rx 00
fsl_espi ffe110000.spi: Extra RX 03
fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
fsl_espi ffe110000.spi: tx 05
fsl_espi ffe110000.spi: rx 00
fsl_espi ffe110000.spi: Extra RX 03
From all the Micron SPI-NOR datasheets I've got access to it is
possible to continually read the SR/FSR. But I've no idea why it happens
some times and not others.
^ permalink raw reply
* Re: fsl_espi errors on v5.7.15
From: Chris Packham @ 2020-08-26 6:07 UTC (permalink / raw)
To: Heiner Kallweit, broonie@kernel.org, mpe@ellerman.id.au,
benh@kernel.crashing.org, paulus@samba.org
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
linux-spi@vger.kernel.org
In-Reply-To: <519c3068-6c73-c17a-2016-1afe2a1d12f7@alliedtelesis.co.nz>
On 26/08/20 1:48 pm, Chris Packham wrote:
>
> On 26/08/20 10:22 am, Chris Packham wrote:
>> On 25/08/20 7:22 pm, Heiner Kallweit wrote:
>>
>> <snip>
>>> I've been staring at spi-fsl-espi.c for while now and I think I've
>>>> identified a couple of deficiencies that may or may not be related
>>>> to my
>>>> issue.
>>>>
>>>> First I think the 'Transfer done but SPIE_DON isn't set' message
>>>> can be
>>>> generated spuriously. In fsl_espi_irq() we read the ESPI_SPIE
>>>> register.
>>>> We also write back to it to clear the current events. We re-read it in
>>>> fsl_espi_cpu_irq() and complain when SPIE_DON is not set. But we can
>>>> naturally end up in that situation if we're doing a large read.
>>>> Consider
>>>> the messages for reading a block of data from a spi-nor chip
>>>>
>>>> tx = READ_OP + ADDR
>>>> rx = data
>>>>
>>>> We setup the transfer and pump out the tx_buf. The first interrupt
>>>> goes
>>>> off and ESPI_SPIE has SPIM_DON and SPIM_RXT set. We empty the rx fifo,
>>>> clear ESPI_SPIE and wait for the next interrupt. The next interrupt
>>>> fires and this time we have ESPI_SPIE with just SPIM_RXT set. This
>>>> continues until we've received all the data and we finish with
>>>> ESPI_SPIE
>>>> having only SPIM_RXT set. When we re-read it we complain that SPIE_DON
>>>> isn't set.
>>>>
>>>> The other deficiency is that we only get an interrupt when the
>>>> amount of
>>>> data in the rx fifo is above FSL_ESPI_RXTHR. If there are fewer than
>>>> FSL_ESPI_RXTHR left to be received we will never pull them out of
>>>> the fifo.
>>>>
>>> SPIM_DON will trigger an interrupt once the last characters have been
>>> transferred, and read the remaining characters from the FIFO.
>>
>> The T2080RM that I have says the following about the DON bit
>>
>> "Last character was transmitted. The last character was transmitted
>> and a new command can be written for the next frame."
>>
>> That does at least seem to fit with my assertion that it's all about
>> the TX direction. But the fact that it doesn't happen all the time
>> throws some doubt on it.
>>
>>> I think the reason I'm seeing some variability is because of how fast
>>>> (or slow) the interrupts get processed and how fast the spi-nor
>>>> chip can
>>>> fill the CPUs rx fifo.
>>>>
>>> To rule out timing issues at high bus frequencies I initially asked
>>> for re-testing at lower frequencies. If you e.g. limit the bus to 1 MHz
>>> or even less, then timing shouldn't be an issue.
>> Yes I've currently got spi-max-frequency = <1000000>; in my dts. I
>> would also expect a slower frequency would fit my "DON is for TX"
>> narrative.
>>> Last relevant functional changes have been done almost 4 years ago.
>>> And yours is the first such report I see. So question is what could
>>> be so
>>> special with your setup that it seems you're the only one being
>>> affected.
>>> The scenarios you describe are standard, therefore much more people
>>> should be affected in case of a driver bug.
>> Agreed. But even on my hardware (which may have a latent issue
>> despite being in the field for going on 5 years) the issue only
>> triggers under some fairly specific circumstances.
>>> You said that kernel config impacts how frequently the issue happens.
>>> Therefore question is what's the diff in kernel config, and how could
>>> the differences be related to SPI.
>>
>> It did seem to be somewhat random. Things like CONFIG_PREEMPT have an
>> impact but every time I found something that seemed to be having an
>> impact I've been able to disprove it. I actually think its about how
>> busy the system is which may or may not affect when we get round to
>> processing the interrupts.
>>
>> I have managed to get the 'Transfer done but SPIE_DON isn't set!' to
>> occur on the T2080RDB.
>>
>> I've had to add the following to expose the environment as a mtd
>> partition
>>
>> diff --git a/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
>> b/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
>> index ff87e67c70da..fbf95fc1fd68 100644
>> --- a/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
>> +++ b/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
>> @@ -116,6 +116,15 @@ flash@0 {
>> compatible = "micron,n25q512ax3",
>> "jedec,spi-nor";
>> reg = <0>;
>> spi-max-frequency = <10000000>; /*
>> input clock */
>> +
>> + partition@u-boot {
>> + reg = <0x00000000 0x00100000>;
>> + label = "u-boot";
>> + };
>> + partition@u-boot-env {
>> + reg = <0x00100000 0x00010000>;
>> + label = "u-boot-env";
>> + };
>> };
>> };
>>
>> And I'm using the following script to poke at the environment
>> (warning if anyone does try this and the bug hits it can render your
>> u-boot environment invalid).
>>
>> cat flash/fw_env_test.sh
>> #!/bin/sh
>>
>> generate_fw_env_config()
>> {
>> cat /proc/mtd | sed 's/[:"]//g' | while read dev size erasesize
>> name ; do
>> echo "$dev $size $erasesize $name"
>> [ "$name" = "u-boot-env" ] && echo "/dev/$dev 0x0000 0x2000
>> $erasesize" >/flash/fw_env.config
>> done
>> }
>>
>> cycles=10
>> [ $# -ge 1 ] && cycles=$1
>>
>> generate_fw_env_config
>>
>> fw_printenv -c /flash/fw_env.config
>>
>> dmesg -c >/dev/null
>> x=0
>> while [ $x -lt $cycles ]; do
>> fw_printenv -c /flash/fw_env.config >/dev/null || break
>> fw_setenv -c /flash/fw_env.config foo $RANDOM || break;
>> dmesg -c | grep -q fsl_espi && break;
>> let x=x+1
>> done
>>
>> echo "Ran $x cycles"
>
> I've also now seen the RX FIFO not empty error on the T2080RDB
>
> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>
> With my current workaround of emptying the RX FIFO. It seems
> survivable. Interestingly it only ever seems to be 1 extra byte in the
> RX FIFO and it seems to be after either a READ_SR or a READ_FSR.
>
> fsl_espi ffe110000.spi: tx 70
> fsl_espi ffe110000.spi: rx 03
> fsl_espi ffe110000.spi: Extra RX 00
> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
> fsl_espi ffe110000.spi: tx 05
> fsl_espi ffe110000.spi: rx 00
> fsl_espi ffe110000.spi: Extra RX 03
> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
> fsl_espi ffe110000.spi: tx 05
> fsl_espi ffe110000.spi: rx 00
> fsl_espi ffe110000.spi: Extra RX 03
>
> From all the Micron SPI-NOR datasheets I've got access to it is
> possible to continually read the SR/FSR. But I've no idea why it
> happens some times and not others.
So I think I've got a reproduction and I think I've bisected the problem
to commit 3282a3da25bd ("powerpc/64: Implement soft interrupt replay in
C"). My day is just finishing now so I haven't applied too much scrutiny
to this result. Given the various rabbit holes I've been down on this
issue already I'd take this information with a good degree of skepticism.
Thanks,
Chris
^ permalink raw reply
* Re: fsl_espi errors on v5.7.15
From: Heiner Kallweit @ 2020-08-26 6:38 UTC (permalink / raw)
To: Chris Packham, broonie@kernel.org, mpe@ellerman.id.au,
benh@kernel.crashing.org, paulus@samba.org
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
linux-spi@vger.kernel.org
In-Reply-To: <42107721-614b-96e8-68d9-4b888206562e@alliedtelesis.co.nz>
On 26.08.2020 08:07, Chris Packham wrote:
>
> On 26/08/20 1:48 pm, Chris Packham wrote:
>>
>> On 26/08/20 10:22 am, Chris Packham wrote:
>>> On 25/08/20 7:22 pm, Heiner Kallweit wrote:
>>>
>>> <snip>
>>>> I've been staring at spi-fsl-espi.c for while now and I think I've
>>>>> identified a couple of deficiencies that may or may not be related
>>>>> to my
>>>>> issue.
>>>>>
>>>>> First I think the 'Transfer done but SPIE_DON isn't set' message
>>>>> can be
>>>>> generated spuriously. In fsl_espi_irq() we read the ESPI_SPIE
>>>>> register.
>>>>> We also write back to it to clear the current events. We re-read it in
>>>>> fsl_espi_cpu_irq() and complain when SPIE_DON is not set. But we can
>>>>> naturally end up in that situation if we're doing a large read.
>>>>> Consider
>>>>> the messages for reading a block of data from a spi-nor chip
>>>>>
>>>>> tx = READ_OP + ADDR
>>>>> rx = data
>>>>>
>>>>> We setup the transfer and pump out the tx_buf. The first interrupt
>>>>> goes
>>>>> off and ESPI_SPIE has SPIM_DON and SPIM_RXT set. We empty the rx fifo,
>>>>> clear ESPI_SPIE and wait for the next interrupt. The next interrupt
>>>>> fires and this time we have ESPI_SPIE with just SPIM_RXT set. This
>>>>> continues until we've received all the data and we finish with
>>>>> ESPI_SPIE
>>>>> having only SPIM_RXT set. When we re-read it we complain that SPIE_DON
>>>>> isn't set.
>>>>>
>>>>> The other deficiency is that we only get an interrupt when the
>>>>> amount of
>>>>> data in the rx fifo is above FSL_ESPI_RXTHR. If there are fewer than
>>>>> FSL_ESPI_RXTHR left to be received we will never pull them out of
>>>>> the fifo.
>>>>>
>>>> SPIM_DON will trigger an interrupt once the last characters have been
>>>> transferred, and read the remaining characters from the FIFO.
>>>
>>> The T2080RM that I have says the following about the DON bit
>>>
>>> "Last character was transmitted. The last character was transmitted
>>> and a new command can be written for the next frame."
>>>
>>> That does at least seem to fit with my assertion that it's all about
>>> the TX direction. But the fact that it doesn't happen all the time
>>> throws some doubt on it.
>>>
>>>> I think the reason I'm seeing some variability is because of how fast
>>>>> (or slow) the interrupts get processed and how fast the spi-nor
>>>>> chip can
>>>>> fill the CPUs rx fifo.
>>>>>
>>>> To rule out timing issues at high bus frequencies I initially asked
>>>> for re-testing at lower frequencies. If you e.g. limit the bus to 1 MHz
>>>> or even less, then timing shouldn't be an issue.
>>> Yes I've currently got spi-max-frequency = <1000000>; in my dts. I
>>> would also expect a slower frequency would fit my "DON is for TX"
>>> narrative.
>>>> Last relevant functional changes have been done almost 4 years ago.
>>>> And yours is the first such report I see. So question is what could
>>>> be so
>>>> special with your setup that it seems you're the only one being
>>>> affected.
>>>> The scenarios you describe are standard, therefore much more people
>>>> should be affected in case of a driver bug.
>>> Agreed. But even on my hardware (which may have a latent issue
>>> despite being in the field for going on 5 years) the issue only
>>> triggers under some fairly specific circumstances.
>>>> You said that kernel config impacts how frequently the issue happens.
>>>> Therefore question is what's the diff in kernel config, and how could
>>>> the differences be related to SPI.
>>>
>>> It did seem to be somewhat random. Things like CONFIG_PREEMPT have an
>>> impact but every time I found something that seemed to be having an
>>> impact I've been able to disprove it. I actually think its about how
>>> busy the system is which may or may not affect when we get round to
>>> processing the interrupts.
>>>
>>> I have managed to get the 'Transfer done but SPIE_DON isn't set!' to
>>> occur on the T2080RDB.
>>>
>>> I've had to add the following to expose the environment as a mtd
>>> partition
>>>
>>> diff --git a/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
>>> b/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
>>> index ff87e67c70da..fbf95fc1fd68 100644
>>> --- a/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
>>> +++ b/arch/powerpc/boot/dts/fsl/t208xrdb.dtsi
>>> @@ -116,6 +116,15 @@ flash@0 {
>>> compatible = "micron,n25q512ax3",
>>> "jedec,spi-nor";
>>> reg = <0>;
>>> spi-max-frequency = <10000000>; /*
>>> input clock */
>>> +
>>> + partition@u-boot {
>>> + reg = <0x00000000 0x00100000>;
>>> + label = "u-boot";
>>> + };
>>> + partition@u-boot-env {
>>> + reg = <0x00100000 0x00010000>;
>>> + label = "u-boot-env";
>>> + };
>>> };
>>> };
>>>
>>> And I'm using the following script to poke at the environment
>>> (warning if anyone does try this and the bug hits it can render your
>>> u-boot environment invalid).
>>>
>>> cat flash/fw_env_test.sh
>>> #!/bin/sh
>>>
>>> generate_fw_env_config()
>>> {
>>> cat /proc/mtd | sed 's/[:"]//g' | while read dev size erasesize
>>> name ; do
>>> echo "$dev $size $erasesize $name"
>>> [ "$name" = "u-boot-env" ] && echo "/dev/$dev 0x0000 0x2000
>>> $erasesize" >/flash/fw_env.config
>>> done
>>> }
>>>
>>> cycles=10
>>> [ $# -ge 1 ] && cycles=$1
>>>
>>> generate_fw_env_config
>>>
>>> fw_printenv -c /flash/fw_env.config
>>>
>>> dmesg -c >/dev/null
>>> x=0
>>> while [ $x -lt $cycles ]; do
>>> fw_printenv -c /flash/fw_env.config >/dev/null || break
>>> fw_setenv -c /flash/fw_env.config foo $RANDOM || break;
>>> dmesg -c | grep -q fsl_espi && break;
>>> let x=x+1
>>> done
>>>
>>> echo "Ran $x cycles"
>>
>> I've also now seen the RX FIFO not empty error on the T2080RDB
>>
>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>>
>> With my current workaround of emptying the RX FIFO. It seems
>> survivable. Interestingly it only ever seems to be 1 extra byte in the
>> RX FIFO and it seems to be after either a READ_SR or a READ_FSR.
>>
>> fsl_espi ffe110000.spi: tx 70
>> fsl_espi ffe110000.spi: rx 03
>> fsl_espi ffe110000.spi: Extra RX 00
>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>> fsl_espi ffe110000.spi: tx 05
>> fsl_espi ffe110000.spi: rx 00
>> fsl_espi ffe110000.spi: Extra RX 03
>> fsl_espi ffe110000.spi: Transfer done but SPIE_DON isn't set!
>> fsl_espi ffe110000.spi: Transfer done but rx/tx fifo's aren't empty!
>> fsl_espi ffe110000.spi: SPIE_RXCNT = 1, SPIE_TXCNT = 32
>> fsl_espi ffe110000.spi: tx 05
>> fsl_espi ffe110000.spi: rx 00
>> fsl_espi ffe110000.spi: Extra RX 03
>>
>> From all the Micron SPI-NOR datasheets I've got access to it is
>> possible to continually read the SR/FSR. But I've no idea why it
>> happens some times and not others.
>
> So I think I've got a reproduction and I think I've bisected the problem
> to commit 3282a3da25bd ("powerpc/64: Implement soft interrupt replay in
> C"). My day is just finishing now so I haven't applied too much scrutiny
> to this result. Given the various rabbit holes I've been down on this
> issue already I'd take this information with a good degree of skepticism.
>
OK, so an easy test should be to re-test with a 5.4 kernel.
It doesn't have yet the change you're referring to, and the fsl-espi driver
is basically the same as in 5.7 (just two small changes in 5.7).
> Thanks,
> Chris
>
^ permalink raw reply
* [PATCH] powerpc/perf: Fix reading of MSR[HV PR] bits in trace-imc
From: Athira Rajeev @ 2020-08-26 6:40 UTC (permalink / raw)
To: mpe; +Cc: maddy, linuxppc-dev
IMC trace-mode uses MSR[HV PR] bits to set the cpumode
for the instruction pointer captured in each sample.
The bits are fetched from third DW of the trace record.
Reading third DW from IMC trace record should use be64_to_cpu
along with READ_ONCE inorder to fetch correct MSR[HV PR] bits.
Patch addresses this change.
Currently we are using `PERF_RECORD_MISC_HYPERVISOR` as
cpumode if MSR HV is 1 and PR is 0 which means the address is from
host counter. But using `PERF_RECORD_MISC_HYPERVISOR` for host
counter data will fail to resolve the `address -> symbol` during
`perf report` because perf tools side uses `PERF_RECORD_MISC_KERNEL`
to represent the host counter data. Therefore, fix the trace imc
sample data to use `PERF_RECORD_MISC_KERNEL` as cpumode for
host kernel information.
Fixes: 77ca3951cc37 ("powerpc/perf: Add kernel support for new
MSR[HV PR] bits in trace-imc")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
arch/powerpc/perf/imc-pmu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index a45d694..62d0b54 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -1289,7 +1289,7 @@ static int trace_imc_prepare_sample(struct trace_imc_data *mem,
header->misc = 0;
if (cpu_has_feature(CPU_FTR_ARCH_31)) {
- switch (IMC_TRACE_RECORD_VAL_HVPR(mem->val)) {
+ switch (IMC_TRACE_RECORD_VAL_HVPR(be64_to_cpu(READ_ONCE(mem->val)))) {
case 0:/* when MSR HV and PR not set in the trace-record */
header->misc |= PERF_RECORD_MISC_GUEST_KERNEL;
break;
@@ -1297,7 +1297,7 @@ static int trace_imc_prepare_sample(struct trace_imc_data *mem,
header->misc |= PERF_RECORD_MISC_GUEST_USER;
break;
case 2: /* MSR HV is 1 and PR is 0 */
- header->misc |= PERF_RECORD_MISC_HYPERVISOR;
+ header->misc |= PERF_RECORD_MISC_KERNEL;
break;
case 3: /* MSR HV is 1 and PR is 1 */
header->misc |= PERF_RECORD_MISC_USER;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] selftests/powerpc: Fix prefixes in alignment_handler signal handler
From: Jordan Niethe @ 2020-08-26 7:27 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20200824131231.14008-1-jniethe5@gmail.com>
On Mon, Aug 24, 2020 at 11:12 PM Jordan Niethe <jniethe5@gmail.com> wrote:
>
> 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;
Oh this should be befine __powerpc64__/CONFIG_PPC64 (thank you patchwork).
>
> 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
* [PATCH] Revert "powerpc/powernv/idle: Replace CPU feature check with PVR check"
From: Pratik Rajesh Sampat @ 2020-08-26 8:29 UTC (permalink / raw)
To: linuxppc-dev, mpe, npiggin, mikey, ego, svaidy, linux-kernel,
pratik.r.sampat, psampat
Cpuidle stop state implementation has minor optimizations for P10
where hardware preserves more SPR registers compared to P9.
The current P9 driver works for P10, although does few extra
save-restores. P9 driver can provide the required power management
features like SMT thread folding and core level power savings
on a P10 platform.
Until the P10 stop driver is available, revert the commit which
allows for only P9 systems to utilize cpuidle and blocks all
idle stop states for P10.
Cpu idle states are enabled and tested on the P10 platform
with this fix.
This reverts commit 8747bf36f312356f8a295a0c39ff092d65ce75ae.
Fixes: 8747bf36f312 ("powerpc/powernv/idle: Replace CPU feature check with PVR check")
Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
---
@mpe: This revert would resolve a staging issue wherein the P10 stop
driver is not yet ready while cpuidle stop states need not be blocked
on 5.9 for Power10 systems which could cause SMT folding related
performance issues.
The P10 stop driver is in the works here:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216773.html
arch/powerpc/platforms/powernv/idle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 77513a80cef9..345ab062b21a 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -1223,7 +1223,7 @@ static void __init pnv_probe_idle_states(void)
return;
}
- if (pvr_version_is(PVR_POWER9))
+ if (cpu_has_feature(CPU_FTR_ARCH_300))
pnv_power9_idle_init();
for (i = 0; i < nr_pnv_idle_states; i++)
--
2.25.4
^ permalink raw reply related
* Re: [PATCH] Revert "powerpc/powernv/idle: Replace CPU feature check with PVR check"
From: Christophe Leroy @ 2020-08-26 8:37 UTC (permalink / raw)
To: Pratik Rajesh Sampat, linuxppc-dev, mpe, npiggin, mikey, ego,
svaidy, linux-kernel, pratik.r.sampat
In-Reply-To: <20200826082918.89306-1-psampat@linux.ibm.com>
Le 26/08/2020 à 10:29, Pratik Rajesh Sampat a écrit :
> Cpuidle stop state implementation has minor optimizations for P10
> where hardware preserves more SPR registers compared to P9.
> The current P9 driver works for P10, although does few extra
> save-restores. P9 driver can provide the required power management
> features like SMT thread folding and core level power savings
> on a P10 platform.
>
> Until the P10 stop driver is available, revert the commit which
> allows for only P9 systems to utilize cpuidle and blocks all
> idle stop states for P10.
> Cpu idle states are enabled and tested on the P10 platform
> with this fix.
>
> This reverts commit 8747bf36f312356f8a295a0c39ff092d65ce75ae.
>
> Fixes: 8747bf36f312 ("powerpc/powernv/idle: Replace CPU feature check with PVR check")
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
> ---
> @mpe: This revert would resolve a staging issue wherein the P10 stop
> driver is not yet ready while cpuidle stop states need not be blocked
> on 5.9 for Power10 systems which could cause SMT folding related
> performance issues.
>
> The P10 stop driver is in the works here:
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216773.html
>
> arch/powerpc/platforms/powernv/idle.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 77513a80cef9..345ab062b21a 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -1223,7 +1223,7 @@ static void __init pnv_probe_idle_states(void)
> return;
> }
>
> - if (pvr_version_is(PVR_POWER9))
> + if (cpu_has_feature(CPU_FTR_ARCH_300))
Why not something like:
if (pvr_version_is(PVR_POWER9) || pvr_version_is(PVR_POWER10))
pnv_power9_idle_init();
> pnv_power9_idle_init();
>
> for (i = 0; i < nr_pnv_idle_states; i++)
>
Christophe
^ permalink raw reply
* Re: [PATCH] Revert "powerpc/powernv/idle: Replace CPU feature check with PVR check"
From: Pratik Sampat @ 2020-08-26 9:14 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev, mpe, npiggin, mikey, ego, svaidy,
linux-kernel, pratik.r.sampat
In-Reply-To: <1fb7fcef-a39d-d36e-35d5-021a5c9ea82c@csgroup.eu>
On 26/08/20 2:07 pm, Christophe Leroy wrote:
>
>
> Le 26/08/2020 à 10:29, Pratik Rajesh Sampat a écrit :
>> Cpuidle stop state implementation has minor optimizations for P10
>> where hardware preserves more SPR registers compared to P9.
>> The current P9 driver works for P10, although does few extra
>> save-restores. P9 driver can provide the required power management
>> features like SMT thread folding and core level power savings
>> on a P10 platform.
>>
>> Until the P10 stop driver is available, revert the commit which
>> allows for only P9 systems to utilize cpuidle and blocks all
>> idle stop states for P10.
>> Cpu idle states are enabled and tested on the P10 platform
>> with this fix.
>>
>> This reverts commit 8747bf36f312356f8a295a0c39ff092d65ce75ae.
>>
>> Fixes: 8747bf36f312 ("powerpc/powernv/idle: Replace CPU feature check
>> with PVR check")
>> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
>> ---
>> @mpe: This revert would resolve a staging issue wherein the P10 stop
>> driver is not yet ready while cpuidle stop states need not be blocked
>> on 5.9 for Power10 systems which could cause SMT folding related
>> performance issues.
>>
>> The P10 stop driver is in the works here:
>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216773.html
>>
>> arch/powerpc/platforms/powernv/idle.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/idle.c
>> b/arch/powerpc/platforms/powernv/idle.c
>> index 77513a80cef9..345ab062b21a 100644
>> --- a/arch/powerpc/platforms/powernv/idle.c
>> +++ b/arch/powerpc/platforms/powernv/idle.c
>> @@ -1223,7 +1223,7 @@ static void __init pnv_probe_idle_states(void)
>> return;
>> }
>> - if (pvr_version_is(PVR_POWER9))
>> + if (cpu_has_feature(CPU_FTR_ARCH_300))
>
> Why not something like:
>
> if (pvr_version_is(PVR_POWER9) || pvr_version_is(PVR_POWER10))
> pnv_power9_idle_init();
In order to use PVR_POWER10 I would need to define it under
arch/powerpc/include/asm/reg.h, which is not present in 5.9 yet.
However, if it okay with @mpe I could split out Nick's P10 stop driver
(https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216773.html)
into two parts:
1. This could include minimal code to introduce the P10 PVR and the
stop wrappers for it. Although this patch internally still calls into
the P9 path. This should gracefully fix the issue.
2. Then later in this patch we could introduce the p10 callback
methods as they are in Nick's series.
---
Thanks
Pratik
>
>> pnv_power9_idle_init();
>> for (i = 0; i < nr_pnv_idle_states; i++)
>>
>
> Christophe
^ permalink raw reply
* Re: [PATCH] Revert "powerpc/powernv/idle: Replace CPU feature check with PVR check"
From: Vaidyanathan Srinivasan @ 2020-08-26 9:40 UTC (permalink / raw)
To: Pratik Rajesh Sampat
Cc: ego, mikey, pratik.r.sampat, linux-kernel, npiggin, linuxppc-dev
In-Reply-To: <20200826082918.89306-1-psampat@linux.ibm.com>
* Pratik Rajesh Sampat <psampat@linux.ibm.com> [2020-08-26 13:59:18]:
> Cpuidle stop state implementation has minor optimizations for P10
> where hardware preserves more SPR registers compared to P9.
> The current P9 driver works for P10, although does few extra
> save-restores. P9 driver can provide the required power management
> features like SMT thread folding and core level power savings
> on a P10 platform.
>
> Until the P10 stop driver is available, revert the commit which
> allows for only P9 systems to utilize cpuidle and blocks all
> idle stop states for P10.
> Cpu idle states are enabled and tested on the P10 platform
> with this fix.
>
> This reverts commit 8747bf36f312356f8a295a0c39ff092d65ce75ae.
>
> Fixes: 8747bf36f312 ("powerpc/powernv/idle: Replace CPU feature check with PVR check")
> Signed-off-by: Pratik Rajesh Sampat <psampat@linux.ibm.com>
Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.ibm.com>
> ---
> @mpe: This revert would resolve a staging issue wherein the P10 stop
> driver is not yet ready while cpuidle stop states need not be blocked
> on 5.9 for Power10 systems which could cause SMT folding related
> performance issues.
>
> The P10 stop driver is in the works here:
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216773.html
>
> arch/powerpc/platforms/powernv/idle.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 77513a80cef9..345ab062b21a 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -1223,7 +1223,7 @@ static void __init pnv_probe_idle_states(void)
> return;
> }
>
> - if (pvr_version_is(PVR_POWER9))
> + if (cpu_has_feature(CPU_FTR_ARCH_300))
> pnv_power9_idle_init();
>
> for (i = 0; i < nr_pnv_idle_states; i++)
This revert solves the stated problem and makes kernel v5.9 work
reasonable well on P10 with stop states which are required for SMT
mode changes.
Complete P10 driver has been in the works and will build on this fix
and complete the required platform support and optimizations.
--Vaidy
^ permalink raw reply
* Re: [PATCH v7 07/12] arm64: inline huge vmap supported functions
From: Catalin Marinas @ 2020-08-26 10:02 UTC (permalink / raw)
To: Nicholas Piggin
Cc: linux-arch, Will Deacon, linux-kernel, Christoph Hellwig,
linux-mm, Zefan Li, Jonathan Cameron, Andrew Morton, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20200825145753.529284-8-npiggin@gmail.com>
On Wed, Aug 26, 2020 at 12:57:48AM +1000, Nicholas Piggin wrote:
> This allows unsupported levels to be constant folded away, and so
> p4d_free_pud_page can be removed because it's no longer linked to.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>
> Ack or objection if this goes via the -mm tree?
No objections.
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* Re: Please apply commit 0828137e8f16 ("powerpc/64s: Don't init FSCR_DSCR in __init_FSCR()") to v4.14.y, v4.19.y, v5.4.y, v5.7.y
From: Greg KH @ 2020-08-26 10:29 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo; +Cc: sashal, linuxppc-dev, stable
In-Reply-To: <20200825224408.GB6060@mussarela>
On Tue, Aug 25, 2020 at 07:44:08PM -0300, Thadeu Lima de Souza Cascardo wrote:
> After commit 912c0a7f2b5daa3cbb2bc10f303981e493de73bd ("powerpc/64s: Save FSCR
> to init_task.thread.fscr after feature init"), which has been applied to the
> referred branches, when userspace sets the user DSCR MSR, it won't be inherited
> or restored during context switch, because the facility unavailable interrupt
> won't trigger.
>
> Applying 0828137e8f16721842468e33df0460044a0c588b ("powerpc/64s: Don't init
> FSCR_DSCR in __init_FSCR()") will fix it.
Now queued up, thanks.
greg k-h
^ permalink raw reply
* Re: Please apply commit 0828137e8f16 ("powerpc/64s: Don't init FSCR_DSCR in __init_FSCR()") to v4.14.y, v4.19.y, v5.4.y, v5.7.y
From: Michael Ellerman @ 2020-08-26 11:58 UTC (permalink / raw)
To: Greg KH, Thadeu Lima de Souza Cascardo; +Cc: sashal, linuxppc-dev, stable
In-Reply-To: <20200826102929.GA3356257@kroah.com>
Greg KH <gregkh@linuxfoundation.org> writes:
> On Tue, Aug 25, 2020 at 07:44:08PM -0300, Thadeu Lima de Souza Cascardo wrote:
>> After commit 912c0a7f2b5daa3cbb2bc10f303981e493de73bd ("powerpc/64s: Save FSCR
>> to init_task.thread.fscr after feature init"), which has been applied to the
>> referred branches, when userspace sets the user DSCR MSR, it won't be inherited
>> or restored during context switch, because the facility unavailable interrupt
>> won't trigger.
>>
>> Applying 0828137e8f16721842468e33df0460044a0c588b ("powerpc/64s: Don't init
>> FSCR_DSCR in __init_FSCR()") will fix it.
Oops, thanks for catching it.
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> Now queued up, thanks.
Thanks.
cheers
^ permalink raw reply
* Re: [PATCH v7 06/12] powerpc: inline huge vmap supported functions
From: Michael Ellerman @ 2020-08-26 11:59 UTC (permalink / raw)
To: Nicholas Piggin, linux-mm, Andrew Morton
Cc: linux-arch, linux-kernel, Nicholas Piggin, Christoph Hellwig,
Zefan Li, Jonathan Cameron, linuxppc-dev
In-Reply-To: <20200825145753.529284-7-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> This allows unsupported levels to be constant folded away, and so
> p4d_free_pud_page can be removed because it's no longer linked to.
>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>
> Ack or objection if this goes via the -mm tree?
Fine by me if it builds.
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
> diff --git a/arch/powerpc/include/asm/vmalloc.h b/arch/powerpc/include/asm/vmalloc.h
> index 105abb73f075..3f0c153befb0 100644
> --- a/arch/powerpc/include/asm/vmalloc.h
> +++ b/arch/powerpc/include/asm/vmalloc.h
> @@ -1,12 +1,25 @@
> #ifndef _ASM_POWERPC_VMALLOC_H
> #define _ASM_POWERPC_VMALLOC_H
>
> +#include <asm/mmu.h>
> #include <asm/page.h>
>
> #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
> -bool arch_vmap_p4d_supported(pgprot_t prot);
> -bool arch_vmap_pud_supported(pgprot_t prot);
> -bool arch_vmap_pmd_supported(pgprot_t prot);
> +static inline bool arch_vmap_p4d_supported(pgprot_t prot)
> +{
> + return false;
> +}
> +
> +static inline bool arch_vmap_pud_supported(pgprot_t prot)
> +{
> + /* HPT does not cope with large pages in the vmalloc area */
> + return radix_enabled();
> +}
> +
> +static inline bool arch_vmap_pmd_supported(pgprot_t prot)
> +{
> + return radix_enabled();
> +}
> #endif
>
> #endif /* _ASM_POWERPC_VMALLOC_H */
> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
> index eca83a50bf2e..27f5837cf145 100644
> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> @@ -1134,22 +1134,6 @@ void radix__ptep_modify_prot_commit(struct vm_area_struct *vma,
> set_pte_at(mm, addr, ptep, pte);
> }
>
> -bool arch_vmap_pud_supported(pgprot_t prot)
> -{
> - /* HPT does not cope with large pages in the vmalloc area */
> - return radix_enabled();
> -}
> -
> -bool arch_vmap_pmd_supported(pgprot_t prot)
> -{
> - return radix_enabled();
> -}
> -
> -int p4d_free_pud_page(p4d_t *p4d, unsigned long addr)
> -{
> - return 0;
> -}
> -
> int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
> {
> pte_t *ptep = (pte_t *)pud;
> @@ -1233,8 +1217,3 @@ int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
>
> return 1;
> }
> -
> -bool arch_vmap_p4d_supported(pgprot_t prot)
> -{
> - return false;
> -}
> --
> 2.23.0
^ permalink raw reply
* Re: kernel since 5.6 do not boot anymore on Apple PowerBook
From: Christophe Leroy @ 2020-08-26 13:53 UTC (permalink / raw)
To: Giuseppe Sacco, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <e7a620fa7521e84e2010660b87f20dd24a3b0cd4.camel@sguazz.it>
Hello Giuseppe,
Le 24/08/2020 à 22:48, Giuseppe Sacco a écrit :
> Hello Christophe,
>
> Il giorno lun, 24/08/2020 alle 07.17 +0200, Christophe Leroy ha
> scritto:
>> Hello Giuseppe,
> [...]
>> The Oopses in the video are fixed in 5.9-rc2, see my response to your
>> other mail.
>
> Right, I just updated from git and rebuilt the kernel whith
> CONFIG_VMAP_STACK not set and the machine boots correctly.
>
>> So now we know that your kernel doesn't boot when CONFIG_VMAP_STACK
>> is set.
>> Can you remind the exact problem ?
>
> latest kernel with CONFIG_VMAP_STACK set stops after writing:
> pmac32_cpufreq: registering PowerMac CPU frequency driver
> pmac32_cpufreq: Low: 667 MHz, High: 867 Mhz, Boot: 667 MHz
>
>> 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 ?
>
> I've just tried: it boots without any special warning. What should I
> look for? This is an excerpt of dmesg output about the line it would
> otherwise stop:
If there is no warning, then the issue is something else, bad luck.
Could you increase the loglevel and try again both with and without
VMAP_STACK ? Maybe we'll get more information on where it stops.
Christophe
>
> [...]
> [ 6.566984] PowerMac i2c bus pmu 2 registered
> [ 6.574879] PowerMac i2c bus pmu 1 registered
> [ 6.582634] PowerMac i2c bus mac-io 0 registered
> [ 6.590323] i2c i2c-2: No i2c address for
> /pci@f2000000/mac-io@17/i2c@18000/i2c-modem
> [ 6.598290] PowerMac i2c bus uni-n 1 registered
> [ 6.606196] i2c i2c-3: i2c-powermac: modalias failure on
> /uni-n@f8000000/i2c@f8001000/cereal@1c0
> [ 6.614320] PowerMac i2c bus uni-n 0 registered
> [ 6.622501] pmac32_cpufreq: Registering PowerMac CPU frequency driver
> [ 6.630580] pmac32_cpufreq: Low: 667 Mhz, High: 867 Mhz, Boot: 667 Mhz
> [ 6.639518] ledtrig-cpu: registered to indicate activity on CPUs
> [ 6.647894] NET: Registered protocol family 10
> [ 6.656492] Segment Routing with IPv6
> [ 6.664490] mip6: Mobile IPv6
> [ 6.672337] NET: Registered protocol family 17
> [ 6.680213] mpls_gso: MPLS GSO support
> [...]
>
> Bye,
> Giuseppe
>
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc/rtas: Restrict RTAS requests from userspace
From: Sasha Levin @ 2020-08-26 13:53 UTC (permalink / raw)
To: Sasha Levin, Andrew Donnellan, linuxppc-dev
Cc: nathanl, leobras.c, stable, dja
In-Reply-To: <20200820044512.7543-1-ajd@linux.ibm.com>
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.8.2, v5.7.16, v5.4.59, v4.19.140, v4.14.193, v4.9.232, v4.4.232.
v5.8.2: Build OK!
v5.7.16: Build OK!
v5.4.59: Failed to apply! Possible dependencies:
1a8916ee3ac2 ("powerpc: Detect the secure boot mode of the system")
4238fad366a6 ("powerpc/ima: Add support to initialize ima policy rules")
9155e2341aa8 ("powerpc/powernv: Add OPAL API interface to access secure variable")
bd5d9c743d38 ("powerpc: expose secure variables to userspace via sysfs")
v4.19.140: Failed to apply! Possible dependencies:
0261a508c9fc ("powerpc/mm: dump segment registers on book3s/32")
136bc0397ae2 ("powerpc/pseries: Introduce option to build secure virtual machines")
1a8916ee3ac2 ("powerpc: Detect the secure boot mode of the system")
75d9fc7fd94e ("powerpc/powernv: move OPAL call wrapper tracing and interrupt handling to C")
7c91efce1608 ("powerpc/mm: dump block address translation on book3s/32")
97026b5a5ac2 ("powerpc/mm: Split dump_pagelinuxtables flag_array table")
a49dddbdb0cc ("powerpc/kernel: Add ucall_norets() ultravisor call handler")
b2133bd7a553 ("powerpc/book3s/32: do not include pte-common.h")
bd5d9c743d38 ("powerpc: expose secure variables to userspace via sysfs")
cbcbbf4afd6d ("powerpc/mm: Define platform default caches related flags")
d81e6f8b7c66 ("powerpc/mm: don't use _PAGE_EXEC in book3s/32")
d82fd29c5a8c ("powerpc/mm: Distribute platform specific PAGE and PMD flags and definitions")
e66c3209c7fd ("powerpc: Move page table dump files in a dedicated subdirectory")
fb0b0a73b223 ("powerpc: Enable kcov")
ff00552578ba ("powerpc/8xx: change name of a few page flags to avoid confusion")
v4.14.193: Failed to apply! Possible dependencies:
136bc0397ae2 ("powerpc/pseries: Introduce option to build secure virtual machines")
1a8916ee3ac2 ("powerpc: Detect the secure boot mode of the system")
4e56207130ed ("kbuild: Cache a few more calls to the compiler")
4fa8bc949de1 ("kbuild: rename *-asn1.[ch] to *.asn1.[ch]")
74ce1896c6c6 ("kbuild: clean up *.dtb and *.dtb.S patterns from top-level Makefile")
75d9fc7fd94e ("powerpc/powernv: move OPAL call wrapper tracing and interrupt handling to C")
8438ee76b004 ("Makefile: disable PIE before testing asm goto")
8f2133cc0e1f ("powerpc/pseries: hcall_exit tracepoint retval should be signed")
92e3da3cf193 ("powerpc: initial pkey plumbing")
9a8dfb394c04 ("kbuild: clean up *.lex.c and *.tab.[ch] patterns from top-level Makefile")
9ce285cfe360 (".gitignore: move *-asn1.[ch] patterns to the top-level .gitignore")
a49dddbdb0cc ("powerpc/kernel: Add ucall_norets() ultravisor call handler")
bd5d9c743d38 ("powerpc: expose secure variables to userspace via sysfs")
c64ba044ed57 ("kbuild: gcov: enable -fno-tree-loop-im if supported")
d677a4d60193 ("Makefile: support flag -fsanitizer-coverage=trace-cmp")
d682026dd3c5 (".gitignore: ignore ASN.1 auto generated files")
e08d6de4e532 ("kbuild: remove kbuild cache")
e501ce957a78 ("x86: Force asm-goto")
e9666d10a567 ("jump_label: move 'asm goto' support test to Kconfig")
ef46d9b3dc01 ("kbuild: clean up *.i and *.lst patterns by make clean")
v4.9.232: Failed to apply! Possible dependencies:
1515ab932156 ("powerpc/mm: Dump hash table")
1a8916ee3ac2 ("powerpc: Detect the secure boot mode of the system")
6cc89bad60a6 ("powerpc/kprobes: Invoke handlers directly")
7644d5819cf8 ("powerpc: Create asm/debugfs.h and move powerpc_debugfs_root there")
7c0f6ba682b9 ("Replace <asm/uaccess.h> with <linux/uaccess.h> globally")
8eb07b187000 ("powerpc/mm: Dump linux pagetables")
92e3da3cf193 ("powerpc: initial pkey plumbing")
bd5d9c743d38 ("powerpc: expose secure variables to userspace via sysfs")
da6658859b9c ("powerpc: Change places using CONFIG_KEXEC to use CONFIG_KEXEC_CORE instead.")
dd5ac03e0955 ("powerpc/mm: Fix page table dump build on non-Book3S")
v4.4.232: Failed to apply! Possible dependencies:
019132ff3daf ("x86/mm/pkeys: Fill in pkey field in siginfo")
0e749e54244e ("dax: increase granularity of dax_clear_blocks() operations")
1a8916ee3ac2 ("powerpc: Detect the secure boot mode of the system")
33a709b25a76 ("mm/gup, x86/mm/pkeys: Check VMAs and PTEs for protection keys")
34c0fd540e79 ("mm, dax, pmem: introduce pfn_t")
3565fce3a659 ("mm, x86: get_user_pages() for dax mappings")
52db400fcd50 ("pmem, dax: clean up clear_pmem()")
5c1d90f51027 ("x86/mm/pkeys: Add PTE bits for storing protection key")
63c17fb8e5a4 ("mm/core, x86/mm/pkeys: Store protection bits in high VMA flags")
69660fd797c3 ("x86, mm: introduce _PAGE_DEVMAP")
7b2d0dbac489 ("x86/mm/pkeys: Pass VMA down in to fault signal generation code")
8f62c883222c ("x86/mm/pkeys: Add arch-specific VMA protection bits")
92e3da3cf193 ("powerpc: initial pkey plumbing")
b2e0d1625e19 ("dax: fix lifetime of in-kernel dax mappings with dax_map_atomic()")
b95f5f4391fa ("libnvdimm: convert to statically allocated badblocks")
bd5d9c743d38 ("powerpc: expose secure variables to userspace via sysfs")
f25748e3c34e ("mm, dax: convert vmf_insert_pfn_pmd() to pfn_t")
fe683adabfe6 ("dax: guarantee page aligned results from bdev_direct_access()")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks
Sasha
^ permalink raw reply
* Re: [PATCH v2 3/4] powerpc/memhotplug: Make lmb size 64bit
From: Sasha Levin @ 2020-08-26 13:53 UTC (permalink / raw)
To: Sasha Levin, Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Nathan Lynch, stable
In-Reply-To: <20200806162329.276534-3-aneesh.kumar@linux.ibm.com>
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.8.2, v5.7.16, v5.4.59, v4.19.140, v4.14.193, v4.9.232, v4.4.232.
v5.8.2: Build OK!
v5.7.16: Build OK!
v5.4.59: Build OK!
v4.19.140: Failed to apply! Possible dependencies:
Unable to calculate
v4.14.193: Failed to apply! Possible dependencies:
Unable to calculate
v4.9.232: Failed to apply! Possible dependencies:
1a367063ca0c ("powerpc/pseries: Check memory device state before onlining/offlining")
25b587fba9a4 ("powerpc/pseries: Correct possible read beyond dlpar sysfs buffer")
333f7b76865b ("powerpc/pseries: Implement indexed-count hotplug memory add")
753843471cbb ("powerpc/pseries: Implement indexed-count hotplug memory remove")
943db62c316c ("powerpc/pseries: Revert 'Auto-online hotplugged memory'")
c21f515c7436 ("powerpc/pseries: Make the acquire/release of the drc for memory a seperate step")
e70d59700fc3 ("powerpc/pseries: Introduce memory hotplug READD operation")
f84775c2d5d9 ("powerpc/pseries: Fix build break when MEMORY_HOTREMOVE=n")
v4.4.232: Failed to apply! Possible dependencies:
183deeea5871 ("powerpc/pseries: Consolidate CPU hotplug code to hotplug-cpu.c")
1a367063ca0c ("powerpc/pseries: Check memory device state before onlining/offlining")
1dc759566636 ("powerpc/pseries: Use kernel hotplug queue for PowerVM hotplug events")
1f859adb9253 ("powerpc/pseries: Verify CPU doesn't exist before adding")
25b587fba9a4 ("powerpc/pseries: Correct possible read beyond dlpar sysfs buffer")
333f7b76865b ("powerpc/pseries: Implement indexed-count hotplug memory add")
4a4bdfea7cb7 ("powerpc/pseries: Refactor dlpar_add_lmb() code")
753843471cbb ("powerpc/pseries: Implement indexed-count hotplug memory remove")
9054619ef54a ("powerpc/pseries: Add pseries hotplug workqueue")
943db62c316c ("powerpc/pseries: Revert 'Auto-online hotplugged memory'")
9dc512819e4b ("powerpc: Fix unused function warning 'lmb_to_memblock'")
bdf5fc633804 ("powerpc/pseries: Update LMB associativity index during DLPAR add/remove")
c21f515c7436 ("powerpc/pseries: Make the acquire/release of the drc for memory a seperate step")
e70d59700fc3 ("powerpc/pseries: Introduce memory hotplug READD operation")
e9d764f80396 ("powerpc/pseries: Enable kernel CPU dlpar from sysfs")
ec999072442a ("powerpc/pseries: Auto-online hotplugged memory")
f84775c2d5d9 ("powerpc/pseries: Fix build break when MEMORY_HOTREMOVE=n")
fdb4f6e99ffa ("powerpc/pseries: Remove call to memblock_add()")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks
Sasha
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Do not initiate shutdown when system is running on UPS
From: Sasha Levin @ 2020-08-26 13:54 UTC (permalink / raw)
To: Sasha Levin, Vasant Hegde, linuxppc-dev; +Cc: Vasant Hegde, stable
In-Reply-To: <20200818105424.234108-1-hegdevasant@linux.vnet.ibm.com>
Hi
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag
fixing commit: 79872e35469b ("powerpc/pseries: All events of EPOW_SYSTEM_SHUTDOWN must initiate shutdown").
The bot has tested the following trees: v5.8.2, v5.7.16, v5.4.59, v4.19.140, v4.14.193, v4.9.232, v4.4.232.
v5.8.2: Build OK!
v5.7.16: Build OK!
v5.4.59: Build OK!
v4.19.140: Build OK!
v4.14.193: Build OK!
v4.9.232: Build OK!
v4.4.232: Failed to apply! Possible dependencies:
b4af279a7cba ("powerpc/pseries: Limit EPOW reset event warnings")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks
Sasha
^ permalink raw reply
* Re: [PATCH v2 1/4] powerpc/drmem: Make lmb_size 64 bit
From: Sasha Levin @ 2020-08-26 13:54 UTC (permalink / raw)
To: Sasha Levin, Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Nathan Lynch, stable
In-Reply-To: <20200806162329.276534-1-aneesh.kumar@linux.ibm.com>
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.8.2, v5.7.16, v5.4.59, v4.19.140, v4.14.193, v4.9.232, v4.4.232.
v5.8.2: Build OK!
v5.7.16: Build OK!
v5.4.59: Build OK!
v4.19.140: Build OK!
v4.14.193: Failed to apply! Possible dependencies:
22508f3dc985 ("powerpc/numa: Look up device node in of_get_usable_memory()")
2c77721552e5 ("powerpc: Move of_drconf_cell struct to asm/drmem.h")
35f80debaef0 ("powerpc/numa: Look up device node in of_get_assoc_arrays()")
514a9cb3316a ("powerpc/numa: Update numa code use walk_drmem_lmbs")
6195a5001f1d ("powerpc/pseries: Update memory hotplug code to use drmem LMB array")
6c6ea53725b3 ("powerpc/mm: Separate ibm, dynamic-memory data from DT format")
b6eca183e23e ("powerpc/kernel: Enables memory hot-remove after reboot on pseries guests")
b88fc309d6ad ("powerpc/numa: Look up associativity array in of_drconf_to_nid_single")
v4.9.232: Failed to apply! Possible dependencies:
3a2df3798d4d ("powerpc/mm: Make switch_mm_irqs_off() out of line")
43ed84a891b7 ("powerpc/mm: Move pgdir setting into a helper")
5a61ef74f269 ("powerpc/64s: Support new device tree binding for discovering CPU features")
5d451a87e5eb ("powerpc/64: Retrieve number of L1 cache sets from device-tree")
6c6ea53725b3 ("powerpc/mm: Separate ibm, dynamic-memory data from DT format")
70cd4c10b290 ("KVM: PPC: Book3S HV: Fix software walk of guest process page tables")
9b081e10805c ("powerpc: port 64 bits pgtable_cache to 32 bits")
a25bd72badfa ("powerpc/mm/radix: Workaround prefetch issue with KVM")
bd067f83b084 ("powerpc/64: Fix naming of cache block vs. cache line")
dbcbfee0c81c ("powerpc/64: More definitions for POWER9")
e2827fe5c156 ("powerpc/64: Clean up ppc64_caches using a struct per cache")
f4329f2ecb14 ("powerpc/64s: Reduce exception alignment")
v4.4.232: Failed to apply! Possible dependencies:
11a6f6abd74a ("powerpc/mm: Move radix/hash common data structures to book3s64 headers")
26b6a3d9bb48 ("powerpc/mm: move pte headers to book3s directory")
3808a88985b4 ("powerpc: Move FW feature probing out of pseries probe()")
3dfcb315d81e ("powerpc/mm: make a separate copy for book3s")
5a61ef74f269 ("powerpc/64s: Support new device tree binding for discovering CPU features")
5d31a96e6c01 ("powerpc/livepatch: Add livepatch stack to struct thread_info")
6574ba950bbe ("powerpc/kernel: Convert cpu_has_feature() to returning bool")
6c6ea53725b3 ("powerpc/mm: Separate ibm, dynamic-memory data from DT format")
a141cca3892b ("powerpc/mm: Add early_[cpu|mmu]_has_feature()")
a8ed87c92adf ("powerpc/mm/radix: Add MMU_FTR_RADIX")
b92a226e5284 ("powerpc: Move cpu_has_feature() to a separate file")
da6a97bf12d5 ("powerpc: Move epapr_paravirt_early_init() to early_init_devtree()")
f63e6d898760 ("powerpc/livepatch: Add livepatch header")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks
Sasha
^ permalink raw reply
* Re: [PATCH v8 2/8] powerpc/vdso: Remove __kernel_datapage_offset and simplify __get_datapage()
From: Michael Ellerman @ 2020-08-26 13:58 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, nathanl
Cc: linux-arch, arnd, Will Deacon, linux-kernel, luto, tglx,
vincenzo.frascino, linuxppc-dev
In-Reply-To: <6862421a-5a14-2e38-b825-e39e6ad3d51d@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 04/08/2020 à 13:17, Christophe Leroy a écrit :
>> On 07/16/2020 02:59 AM, Michael Ellerman wrote:
>>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>>> The VDSO datapage and the text pages are always located immediately
>>>> next to each other, so it can be hardcoded without an indirection
>>>> through __kernel_datapage_offset
>>>>
>>>> In order to ease things, move the data page in front like other
>>>> arches, that way there is no need to know the size of the library
>>>> to locate the data page.
>>>>
> [...]
>
>>>
>>> I merged this but then realised it breaks the display of the vdso in
>>> /proc/self/maps.
>>>
>>> ie. the vdso vma gets no name:
>>>
>>> # cat /proc/self/maps
>
> [...]
>
>>> And it's also going to break the logic in arch_unmap() to detect if
>>> we're unmapping (part of) the VDSO. And it will break arch_remap() too.
>>>
>>> And the logic to recognise the signal trampoline in
>>> arch/powerpc/perf/callchain_*.c as well.
>>
>> I don't think it breaks that one, because ->vdsobase is still the start
>> of text.
>>
>>>
>>> So I'm going to rebase and drop this for now.
>>>
>>> Basically we have a bunch of places that assume that vdso_base is == the
>>> start of the VDSO vma, and also that the code starts there. So that will
>>> need some work to tease out all those assumptions and make them work
>>> with this change.
>>
>> Ok, one day I need to look at it in more details and see how other
>> architectures handle it etc ...
>>
>
> I just sent out a series which switches powerpc to the new
> _install_special_mapping() API, the one powerpc uses being deprecated
> since commit a62c34bd2a8a ("x86, mm: Improve _install_special_mapping
> and fix x86 vdso naming")
>
> arch_remap() gets replaced by vdso_remap()
>
> For arch_unmap(), I'm wondering how/what other architectures do, because
> powerpc seems to be the only one to erase the vdso context pointer when
> unmapping the vdso.
Yeah. The original unmap/remap stuff was added for CRIU, which I thought
people tested on other architectures (more than powerpc even).
Possibly no one really cares about vdso unmap though, vs just moving the
vdso.
We added a test for vdso unmap recently because it happened to trigger a
KAUP failure, and someone actually hit it & reported it.
Running that test on arm64 segfaults:
# ./sigreturn_vdso
VDSO is at 0xffff8191f000-0xffff8191ffff (4096 bytes)
Signal delivered OK with VDSO mapped
VDSO moved to 0xffff8191a000-0xffff8191afff (4096 bytes)
Signal delivered OK with VDSO moved
Unmapped VDSO
Remapped the stack executable
[ 48.556191] potentially unexpected fatal signal 11.
[ 48.556752] CPU: 0 PID: 140 Comm: sigreturn_vdso Not tainted 5.9.0-rc2-00057-g2ac69819ba9e #190
[ 48.556990] Hardware name: linux,dummy-virt (DT)
[ 48.557336] pstate: 60001000 (nZCv daif -PAN -UAO BTYPE=--)
[ 48.557475] pc : 0000ffff8191a7bc
[ 48.557603] lr : 0000ffff8191a7bc
[ 48.557697] sp : 0000ffffc13c9e90
[ 48.557873] x29: 0000ffffc13cb0e0 x28: 0000000000000000
[ 48.558201] x27: 0000000000000000 x26: 0000000000000000
[ 48.558337] x25: 0000000000000000 x24: 0000000000000000
[ 48.558754] x23: 0000000000000000 x22: 0000000000000000
[ 48.558893] x21: 00000000004009b0 x20: 0000000000000000
[ 48.559046] x19: 0000000000400ff0 x18: 0000000000000000
[ 48.559180] x17: 0000ffff817da300 x16: 0000000000412010
[ 48.559312] x15: 0000000000000000 x14: 000000000000001c
[ 48.559443] x13: 656c626174756365 x12: 7865206b63617473
[ 48.559625] x11: 0000000000000003 x10: 0101010101010101
[ 48.559828] x9 : 0000ffff818afda8 x8 : 0000000000000081
[ 48.559973] x7 : 6174732065687420 x6 : 64657070616d6552
[ 48.560115] x5 : 000000000e0388bd x4 : 000000000040135d
[ 48.560270] x3 : 0000000000000000 x2 : 0000000000000001
[ 48.560412] x1 : 0000000000000003 x0 : 00000000004120b8
Segmentation fault
#
So I think we need to keep the unmap hook. Maybe it should be handled by
the special_mapping stuff generically.
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