* Re: fsl_espi errors on v5.7.15
From: Chris Packham @ 2020-09-01 1:25 UTC (permalink / raw)
To: Heiner Kallweit, Nicholas Piggin, benh@kernel.crashing.org,
broonie@kernel.org, mpe@ellerman.id.au, paulus@samba.org
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
linux-spi@vger.kernel.org
In-Reply-To: <5bf05188-a52e-c8c0-9d2d-c87eff6f1588@gmail.com>
On 1/09/20 12:33 am, Heiner Kallweit wrote:
> On 30.08.2020 23:59, Chris Packham wrote:
>> On 31/08/20 9:41 am, Heiner Kallweit wrote:
>>> On 30.08.2020 23:00, Chris Packham wrote:
>>>> On 31/08/20 12:30 am, Nicholas Piggin wrote:
>>>>> Excerpts from Chris Packham's message of August 28, 2020 8:07 am:
>>>> <snip>
>>>>
>>>>>>>>>> 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).
>>>>>>> There's 6cc0c16d82f88 and maybe also other interrupt related patches
>>>>>>> around this time that could affect book E, so it's good if that exact
>>>>>>> patch is confirmed.
>>>>>> My confirmation is basically that I can induce the issue in a 5.4 kernel
>>>>>> by cherry-picking 3282a3da25bd. I'm also able to "fix" the issue in
>>>>>> 5.9-rc2 by reverting that one commit.
>>>>>>
>>>>>> I both cases it's not exactly a clean cherry-pick/revert so I also
>>>>>> confirmed the bisection result by building at 3282a3da25bd (which sees
>>>>>> the issue) and the commit just before (which does not).
>>>>> Thanks for testing, that confirms it well.
>>>>>
>>>>> [snip patch]
>>>>>
>>>>>> I still saw the issue with this change applied. PPC_IRQ_SOFT_MASK_DEBUG
>>>>>> didn't report anything (either with or without the change above).
>>>>> Okay, it was a bit of a shot in the dark. I still can't see what
>>>>> else has changed.
>>>>>
>>>>> What would cause this, a lost interrupt? A spurious interrupt? Or
>>>>> higher interrupt latency?
>>>>>
>>>>> I don't think the patch should cause significantly worse latency,
>>>>> (it's supposed to be a bit better if anything because it doesn't set
>>>>> up the full interrupt frame). But it's possible.
>>>> My working theory is that the SPI_DON indication is all about the TX
>>>> direction an now that the interrupts are faster we're hitting an error
>>>> because there is still RX activity going on. Heiner disagrees with my
>>>> interpretation of the SPI_DON indication and the fact that it doesn't
>>>> happen every time does throw doubt on it.
>>>>
>>> It's right that the eSPI spec can be interpreted that SPI_DON refers to
>>> TX only. However this wouldn't really make sense, because also for RX
>>> we program the frame length, and therefore want to be notified once the
>>> full frame was received. Also practical experience shows that SPI_DON
>>> is set also after RX-only transfers.
>>> Typical SPI NOR use case is that you write read command + start address,
>>> followed by a longer read. If the TX-only interpretation would be right,
>>> we'd always end up with SPI_DON not being set.
>>>
>>>> I can't really explain the extra RX byte in the fifo. We know how many
>>>> bytes to expect and we pull that many from the fifo so it's not as if
>>>> we're missing an interrupt causing us to skip the last byte. I've been
>>>> looking for some kind of off-by-one calculation but again if it were
>>>> something like that it'd happen all the time.
>>>>
>>> Maybe it helps to know what value this extra byte in the FIFO has. Is it:
>>> - a duplicate of the last read byte
>>> - or the next byte (at <end address> + 1)
>>> - or a fixed value, e.g. always 0x00 or 0xff
>> The values were up thread a bit but I'll repeat them here
>>
>> 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
>>
>>
>> The rx 00 Extra RX 03 is a bit concerning. I've only ever seen them with
>> either a READ_SR or a READ_FSR. Never a data read.
>>
> Just remembered something about SPIE_DON:
> Transfers are always full duplex, therefore in case of a read the chip
> sends dummy zero's. Having said that in case of a read SPIE_DON means
> that the last dummy zero was shifted out.
>
> READ_SR and READ_FSR are the shortest transfers, 1 byte out and 1 byte in.
> So the issue may have a dependency on the length of the transfer.
> However I see no good explanation so far. You can try adding a delay of
> a few miroseconds between the following to commands in fsl_espi_bufs().
>
> fsl_espi_write_reg(espi, ESPI_SPIM, mask);
>
> /* Prevent filling the fifo from getting interrupted */
> spin_lock_irq(&espi->lock);
>
> Maybe enabling interrupts and seeing the SPIE_DON interrupt are too close.
I think this might be heading in the right direction. Playing about with
a delay does seem to make the two symptoms less likely. Although I have
to set it quite high (i.e. msleep(100)) to completely avoid any
possibility of seeing either message.
^ permalink raw reply
* Re: [PATCH] soc: fsl: Remove bogus packed attributes from qman.h
From: Herbert Xu @ 2020-09-01 0:33 UTC (permalink / raw)
To: Li Yang, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200730125259.GA8948@gondor.apana.org.au>
On Thu, Jul 30, 2020 at 10:52:59PM +1000, Herbert Xu wrote:
> There are two __packed attributes in qman.h that are both unnecessary
> and causing compiler warnings because they're conflicting with
> explicit alignment requirements set on members within the structure.
>
> This patch removes them both.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Ping.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH v2] scsi: ibmvfc: interface updates for future FPIN and MQ support
From: Tyrel Datwyler @ 2020-09-01 0:24 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
VIOS partitions with SLI-4 enabled Emulex adapters will be capable of
driving IO in parallel through mulitple work queues or channels, and
with new hyperviosr firmware that supports multiple interrupt sources
an ibmvfc NPIV single initiator can be modified to exploit end to end
channelization in a PowerVM environment.
VIOS hosts will also be able to expose fabric perfromance impact
notifications (FPIN) via a new asynchronous event to ibmvfc clients that
advertise support via IBMVFC_CAN_HANDLE_FPIN in their capabilities flag
during NPIV_LOGIN.
This patch introduces three new Managment Datagrams (MADs) for
channelization support negotiation as well as the FPIN asynchronous
event and FPIN status flags. Follow up work is required to plumb the
ibmvfc client driver to use these new interfaces.
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
v1 -> v2:
Fixup complier errors from neglected commit --amend
---
drivers/scsi/ibmvscsi/ibmvfc.h | 66 +++++++++++++++++++++++++++++++++-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 907889f1fa9d..fe55cfc79421 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -124,6 +124,9 @@ enum ibmvfc_mad_types {
IBMVFC_PASSTHRU = 0x0200,
IBMVFC_TMF_MAD = 0x0100,
IBMVFC_NPIV_LOGOUT = 0x0800,
+ IBMVFC_CHANNEL_ENQUIRY = 0x1000,
+ IBMVFC_CHANNEL_SETUP = 0x2000,
+ IBMVFC_CONNECTION_INFO = 0x4000,
};
struct ibmvfc_mad_common {
@@ -162,6 +165,8 @@ struct ibmvfc_npiv_login {
__be32 max_cmds;
__be64 capabilities;
#define IBMVFC_CAN_MIGRATE 0x01
+#define IBMVFC_CAN_USE_CHANNELS 0x02
+#define IBMVFC_CAN_HANDLE_FPIN 0x04
__be64 node_name;
struct srp_direct_buf async;
u8 partition_name[IBMVFC_MAX_NAME];
@@ -204,6 +209,7 @@ struct ibmvfc_npiv_login_resp {
__be64 capabilities;
#define IBMVFC_CAN_FLUSH_ON_HALT 0x08
#define IBMVFC_CAN_SUPPRESS_ABTS 0x10
+#define IBMVFC_CAN_SUPPORT_CHANNELS 0x20
__be32 max_cmds;
__be32 scsi_id_sz;
__be64 max_dma_len;
@@ -482,6 +488,52 @@ struct ibmvfc_passthru_mad {
struct ibmvfc_passthru_fc_iu fc_iu;
}__attribute__((packed, aligned (8)));
+struct ibmvfc_channel_enquiry {
+ struct ibmvfc_mad_common common;
+ __be32 flags;
+#define IBMVFC_NO_CHANNELS_TO_CRQ_SUPPORT 0x01
+#define IBMVFC_SUPPORT_VARIABLE_SUBQ_MSG 0x02
+#define IBMVFC_NO_N_TO_M_CHANNELS_SUPPORT 0x04
+ __be32 num_scsi_subq_channels;
+ __be32 num_nvmeof_subq_channels;
+ __be32 num_scsi_vas_channels;
+ __be32 num_nvmeof_vas_channels;
+}__attribute__((packed, aligned (8)));
+
+struct ibmvfc_channel_setup_mad {
+ struct ibmvfc_mad_common common;
+ struct srp_direct_buf buffer;
+}__attribute__((packed, aligned (8)));
+
+#define IBMVFC_MAX_CHANNELS 502
+
+struct ibmvfc_channel_setup {
+ __be32 flags;
+#define IBMVFC_CANCEL_CHANNELS 0x01
+#define IBMVFC_USE_BUFFER 0x02
+#define IBMVFC_CHANNELS_CANCELED 0x04
+ __be32 reserved;
+ __be32 num_scsi_subq_channels;
+ __be32 num_nvmeof_subq_channels;
+ __be32 num_scsi_vas_channels;
+ __be32 num_nvmeof_vas_channels;
+ struct srp_direct_buf buffer;
+ __be64 reserved2[5];
+ __be64 channel_handles[IBMVFC_MAX_CHANNELS];
+}__attribute__((packed, aligned (8)));
+
+struct ibmvfc_connection_info {
+ struct ibmvfc_mad_common common;
+ __be64 information_bits;
+#define IBMVFC_NO_FC_IO_CHANNEL 0x01
+#define IBMVFC_NO_PHYP_VAS 0x02
+#define IBMVFC_NO_PHYP_SUBQ 0x04
+#define IBMVFC_PHYP_DEPRECATED_SUBQ 0x08
+#define IBMVFC_PHYP_PRESERVED_SUBQ 0x10
+#define IBMVFC_PHYP_FULL_SUBQ 0x20
+ __be64 reserved[16];
+}__attribute__((packed, aligned (8)));
+
struct ibmvfc_trace_start_entry {
u32 xfer_len;
}__attribute__((packed));
@@ -532,6 +584,7 @@ enum ibmvfc_async_event {
IBMVFC_AE_HALT = 0x0400,
IBMVFC_AE_RESUME = 0x0800,
IBMVFC_AE_ADAPTER_FAILED = 0x1000,
+ IBMVFC_AE_FPIN = 0x2000,
};
struct ibmvfc_async_desc {
@@ -560,10 +613,18 @@ enum ibmvfc_ae_link_state {
IBMVFC_AE_LS_LINK_DEAD = 0x08,
};
+enum ibmvfc_ae_fpin_status {
+ IBMVFC_AE_FPIN_LINK_CONGESTED = 0x1,
+ IBMVFC_AE_FPIN_PORT_CONGESTED = 0x2,
+ IBMVFC_AE_FPIN_PORT_CLEARED = 0x3,
+ IBMVFC_AE_FPIN_PORT_DEGRADED = 0x4,
+};
+
struct ibmvfc_async_crq {
volatile u8 valid;
u8 link_state;
- u8 pad[2];
+ u8 fpin_status;
+ u8 pad;
__be32 pad2;
volatile __be64 event;
volatile __be64 scsi_id;
@@ -590,6 +651,9 @@ union ibmvfc_iu {
struct ibmvfc_tmf tmf;
struct ibmvfc_cmd cmd;
struct ibmvfc_passthru_mad passthru;
+ struct ibmvfc_channel_enquiry channel_enquiry;
+ struct ibmvfc_channel_setup_mad channel_setup;
+ struct ibmvfc_connection_info connection_info;
}__attribute__((packed, aligned (8)));
enum ibmvfc_target_action {
--
2.27.0
^ permalink raw reply related
* Re: [PATCH] scsi: ibmvfc: interface updates for future FPIN and MQ support
From: kernel test robot @ 2020-08-31 23:57 UTC (permalink / raw)
To: Tyrel Datwyler, james.bottomley
Cc: Tyrel Datwyler, kbuild-all, martin.petersen, linux-scsi,
linux-kernel, brking, linuxppc-dev
In-Reply-To: <20200831171844.635729-1-tyreld@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 15835 bytes --]
Hi Tyrel,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on mkp-scsi/for-next scsi/for-next v5.9-rc3 next-20200828]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Tyrel-Datwyler/scsi-ibmvfc-interface-updates-for-future-FPIN-and-MQ-support/20200901-012005
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from drivers/scsi/ibmvscsi/ibmvfc.c:32:
>> drivers/scsi/ibmvscsi/ibmvfc.h:500:13: error: expected ':', ',', ';', '}' or '__attribute__' before 'nvmeof_vas_channels'
500 | __be32 num nvmeof_vas_channels;
| ^~~~~~~~~~~~~~~~~~~
>> drivers/scsi/ibmvscsi/ibmvfc.h:506:17: error: expected declaration specifiers or '...' before '(' token
506 | }__attrribute__((packed, aligned (8)));
| ^
>> drivers/scsi/ibmvscsi/ibmvfc.h:526:28: error: field 'common' has incomplete type
526 | struct ibmvfc_madd_common common;
| ^~~~~~
>> drivers/scsi/ibmvscsi/ibmvfc.h:627:2: error: expected ':', ',', ';', '}' or '__attribute__' before 'u8'
627 | u8 pad;
| ^~
In file included from include/linux/byteorder/big_endian.h:5,
from arch/powerpc/include/uapi/asm/byteorder.h:14,
from include/asm-generic/bitops/le.h:6,
from arch/powerpc/include/asm/bitops.h:246,
from include/linux/bitops.h:29,
from include/linux/kernel.h:12,
from include/linux/list.h:9,
from include/linux/module.h:12,
from drivers/scsi/ibmvscsi/ibmvfc.c:10:
drivers/scsi/ibmvscsi/ibmvfc.c: In function 'ibmvfc_handle_async':
>> drivers/scsi/ibmvscsi/ibmvfc.c:2665:75: error: 'struct ibmvfc_async_crq' has no member named 'event'
2665 | const struct ibmvfc_async_desc *desc = ibmvfc_get_ae_desc(be64_to_cpu(crq->event));
| ^~
include/uapi/linux/byteorder/big_endian.h:38:51: note: in definition of macro '__be64_to_cpu'
38 | #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
| ^
drivers/scsi/ibmvscsi/ibmvfc.c:2665:60: note: in expansion of macro 'be64_to_cpu'
2665 | const struct ibmvfc_async_desc *desc = ibmvfc_get_ae_desc(be64_to_cpu(crq->event));
| ^~~~~~~~~~~
>> drivers/scsi/ibmvscsi/ibmvfc.c:2669:57: error: 'struct ibmvfc_async_crq' has no member named 'scsi_id'
2669 | " node_name: %llx%s\n", desc->desc, be64_to_cpu(crq->scsi_id),
| ^~
include/uapi/linux/byteorder/big_endian.h:38:51: note: in definition of macro '__be64_to_cpu'
38 | #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
| ^
drivers/scsi/ibmvscsi/ibmvfc.h:818:4: note: in expansion of macro 'dev_err'
818 | dev_err((vhost)->dev, ##__VA_ARGS__); \
| ^~~~~~~
drivers/scsi/ibmvscsi/ibmvfc.c:2668:2: note: in expansion of macro 'ibmvfc_log'
2668 | ibmvfc_log(vhost, desc->log_level, "%s event received. scsi_id: %llx, wwpn: %llx,"
| ^~~~~~~~~~
>> drivers/scsi/ibmvscsi/ibmvfc.c:2670:21: error: 'struct ibmvfc_async_crq' has no member named 'wwpn'
2670 | be64_to_cpu(crq->wwpn), be64_to_cpu(crq->node_name),
| ^~
include/uapi/linux/byteorder/big_endian.h:38:51: note: in definition of macro '__be64_to_cpu'
38 | #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
| ^
drivers/scsi/ibmvscsi/ibmvfc.h:818:4: note: in expansion of macro 'dev_err'
818 | dev_err((vhost)->dev, ##__VA_ARGS__); \
| ^~~~~~~
drivers/scsi/ibmvscsi/ibmvfc.c:2668:2: note: in expansion of macro 'ibmvfc_log'
2668 | ibmvfc_log(vhost, desc->log_level, "%s event received. scsi_id: %llx, wwpn: %llx,"
| ^~~~~~~~~~
>> drivers/scsi/ibmvscsi/ibmvfc.c:2670:45: error: 'struct ibmvfc_async_crq' has no member named 'node_name'
2670 | be64_to_cpu(crq->wwpn), be64_to_cpu(crq->node_name),
| ^~
include/uapi/linux/byteorder/big_endian.h:38:51: note: in definition of macro '__be64_to_cpu'
38 | #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
| ^
drivers/scsi/ibmvscsi/ibmvfc.h:818:4: note: in expansion of macro 'dev_err'
818 | dev_err((vhost)->dev, ##__VA_ARGS__); \
| ^~~~~~~
drivers/scsi/ibmvscsi/ibmvfc.c:2668:2: note: in expansion of macro 'ibmvfc_log'
2668 | ibmvfc_log(vhost, desc->log_level, "%s event received. scsi_id: %llx, wwpn: %llx,"
| ^~~~~~~~~~
drivers/scsi/ibmvscsi/ibmvfc.c:2673:25: error: 'struct ibmvfc_async_crq' has no member named 'event'
2673 | switch (be64_to_cpu(crq->event)) {
| ^~
include/uapi/linux/byteorder/big_endian.h:38:51: note: in definition of macro '__be64_to_cpu'
38 | #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
| ^
drivers/scsi/ibmvscsi/ibmvfc.c:2673:10: note: in expansion of macro 'be64_to_cpu'
2673 | switch (be64_to_cpu(crq->event)) {
| ^~~~~~~~~~~
drivers/scsi/ibmvscsi/ibmvfc.c:2714:12: error: 'struct ibmvfc_async_crq' has no member named 'scsi_id'
2714 | if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
| ^~
drivers/scsi/ibmvscsi/ibmvfc.c:2714:29: error: 'struct ibmvfc_async_crq' has no member named 'wwpn'
2714 | if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
| ^~
drivers/scsi/ibmvscsi/ibmvfc.c:2714:43: error: 'struct ibmvfc_async_crq' has no member named 'node_name'
2714 | if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
| ^~
drivers/scsi/ibmvscsi/ibmvfc.c:2716:11: error: 'struct ibmvfc_async_crq' has no member named 'scsi_id'
2716 | if (crq->scsi_id && cpu_to_be64(tgt->scsi_id) != crq->scsi_id)
| ^~
drivers/scsi/ibmvscsi/ibmvfc.c:2716:56: error: 'struct ibmvfc_async_crq' has no member named 'scsi_id'
2716 | if (crq->scsi_id && cpu_to_be64(tgt->scsi_id) != crq->scsi_id)
| ^~
drivers/scsi/ibmvscsi/ibmvfc.c:2718:11: error: 'struct ibmvfc_async_crq' has no member named 'wwpn'
2718 | if (crq->wwpn && cpu_to_be64(tgt->ids.port_name) != crq->wwpn)
| ^~
drivers/scsi/ibmvscsi/ibmvfc.c:2718:59: error: 'struct ibmvfc_async_crq' has no member named 'wwpn'
2718 | if (crq->wwpn && cpu_to_be64(tgt->ids.port_name) != crq->wwpn)
| ^~
drivers/scsi/ibmvscsi/ibmvfc.c:2720:11: error: 'struct ibmvfc_async_crq' has no member named 'node_name'
2720 | if (crq->node_name && cpu_to_be64(tgt->ids.node_name) != crq->node_name)
| ^~
drivers/scsi/ibmvscsi/ibmvfc.c:2720:64: error: 'struct ibmvfc_async_crq' has no member named 'node_name'
2720 | if (crq->node_name && cpu_to_be64(tgt->ids.node_name) != crq->node_name)
| ^~
In file included from include/linux/byteorder/big_endian.h:5,
from arch/powerpc/include/uapi/asm/byteorder.h:14,
from include/asm-generic/bitops/le.h:6,
from arch/powerpc/include/asm/bitops.h:246,
from include/linux/bitops.h:29,
from include/linux/kernel.h:12,
from include/linux/list.h:9,
from include/linux/module.h:12,
from drivers/scsi/ibmvscsi/ibmvfc.c:10:
drivers/scsi/ibmvscsi/ibmvfc.c:2722:42: error: 'struct ibmvfc_async_crq' has no member named 'event'
2722 | if (tgt->need_login && be64_to_cpu(crq->event) == IBMVFC_AE_ELS_LOGO)
| ^~
include/uapi/linux/byteorder/big_endian.h:38:51: note: in definition of macro '__be64_to_cpu'
38 | #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
| ^
drivers/scsi/ibmvscsi/ibmvfc.c:2722:27: note: in expansion of macro 'be64_to_cpu'
2722 | if (tgt->need_login && be64_to_cpu(crq->event) == IBMVFC_AE_ELS_LOGO)
| ^~~~~~~~~~~
drivers/scsi/ibmvscsi/ibmvfc.c:2724:43: error: 'struct ibmvfc_async_crq' has no member named 'event'
2724 | if (!tgt->need_login || be64_to_cpu(crq->event) == IBMVFC_AE_ELS_PLOGI) {
| ^~
include/uapi/linux/byteorder/big_endian.h:38:51: note: in definition of macro '__be64_to_cpu'
38 | #define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
| ^
drivers/scsi/ibmvscsi/ibmvfc.c:2724:28: note: in expansion of macro 'be64_to_cpu'
2724 | if (!tgt->need_login || be64_to_cpu(crq->event) == IBMVFC_AE_ELS_PLOGI) {
| ^~~~~~~~~~~
In file included from include/linux/device.h:15,
from include/linux/dma-mapping.h:7,
from drivers/scsi/ibmvscsi/ibmvfc.c:12:
drivers/scsi/ibmvscsi/ibmvfc.c:2741:66: error: 'struct ibmvfc_async_crq' has no member named 'event'
2741 | dev_err(vhost->dev, "Unknown async event received: %lld\n", crq->event);
| ^~
include/linux/dev_printk.h:104:32: note: in definition of macro 'dev_err'
104 | _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~
# https://github.com/0day-ci/linux/commit/b2aced49faf50075e9a74e7a253d1ad77cce1c0c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tyrel-Datwyler/scsi-ibmvfc-interface-updates-for-future-FPIN-and-MQ-support/20200901-012005
git checkout b2aced49faf50075e9a74e7a253d1ad77cce1c0c
vim +500 drivers/scsi/ibmvscsi/ibmvfc.h
490
491 struct ibmvfc_channel_enquiry {
492 struct ibmvfc_mad_common common;
493 __be32 flags;
494 #define IBMVFC_NO_CHANNELS_TO_CRQ_SUPPORT 0x01
495 #define IBMVFC_SUPPORT_VARIABLE_SUBQ_MSG 0x02
496 #define IBMVFC_NO_N_TO_M_CHANNELS_SUPPORT 0x04
497 __be32 num_scsi_subq_channels;
498 __be32 num_nvmeof_subq_channels;
499 __be32 num_scsi_vas_channels;
> 500 __be32 num nvmeof_vas_channels;
501 }__attribute__((packed, aligned (8)));
502
503 struct ibmvfc_channel_setup_mad {
504 struct ibmvfc_mad_common common;
505 struct srp_direct_buf buffer;
> 506 }__attrribute__((packed, aligned (8)));
507
508 #define IBMVFC_MAX_CHANNELS 502
509
510 struct ibmvfc_channel_setup {
511 __be32 flags;
512 #define IBMVFC_CANCEL_CHANNELS 0x01
513 #define IBMVFC_USE_BUFFER 0x02
514 #define IBMVFC_CHANNELS_CANCELED 0x04
515 __be32 reserved;
516 __be32 num_scsi_subq_channels;
517 __be32 num_nvmeof_subq_channels;
518 __be32 num_scsi_vas_channels;
519 __be32 num_nvmeof_vas_channels;
520 struct srp_direct_buf buffer;
521 __be64 reserved2[5];
522 __be64 channel_handles[IBMVFC_MAX_CHANNELS];
523 }__attribute__((packed, aligned (8)));
524
525 struct ibmvfc_connection_info {
> 526 struct ibmvfc_madd_common common;
527 __be64 information_bits;
528 #define IBMVFC_NO_FC_IO_CHANNEL 0x01
529 #define IBMVFC_NO_PHYP_VAS 0x02
530 #define IBMVFC_NO_PHYP_SUBQ 0x04
531 #define IBMVFC_PHYP_DEPRECATED_SUBQ 0x08
532 #define IBMVFC_PHYP_PRESERVED_SUBQ 0x10
533 #define IBMVFC_PHYP_FULL_SUBQ 0x20
534 __be64 reserved[16];
535 }__attribute__((packed, aligned (8)));
536
537 struct ibmvfc_trace_start_entry {
538 u32 xfer_len;
539 }__attribute__((packed));
540
541 struct ibmvfc_trace_end_entry {
542 u16 status;
543 u16 error;
544 u8 fcp_rsp_flags;
545 u8 rsp_code;
546 u8 scsi_status;
547 u8 reserved;
548 }__attribute__((packed));
549
550 struct ibmvfc_trace_entry {
551 struct ibmvfc_event *evt;
552 u32 time;
553 u32 scsi_id;
554 u32 lun;
555 u8 fmt;
556 u8 op_code;
557 u8 tmf_flags;
558 u8 type;
559 #define IBMVFC_TRC_START 0x00
560 #define IBMVFC_TRC_END 0xff
561 union {
562 struct ibmvfc_trace_start_entry start;
563 struct ibmvfc_trace_end_entry end;
564 } u;
565 }__attribute__((packed, aligned (8)));
566
567 enum ibmvfc_crq_formats {
568 IBMVFC_CMD_FORMAT = 0x01,
569 IBMVFC_ASYNC_EVENT = 0x02,
570 IBMVFC_MAD_FORMAT = 0x04,
571 };
572
573 enum ibmvfc_async_event {
574 IBMVFC_AE_ELS_PLOGI = 0x0001,
575 IBMVFC_AE_ELS_LOGO = 0x0002,
576 IBMVFC_AE_ELS_PRLO = 0x0004,
577 IBMVFC_AE_SCN_NPORT = 0x0008,
578 IBMVFC_AE_SCN_GROUP = 0x0010,
579 IBMVFC_AE_SCN_DOMAIN = 0x0020,
580 IBMVFC_AE_SCN_FABRIC = 0x0040,
581 IBMVFC_AE_LINK_UP = 0x0080,
582 IBMVFC_AE_LINK_DOWN = 0x0100,
583 IBMVFC_AE_LINK_DEAD = 0x0200,
584 IBMVFC_AE_HALT = 0x0400,
585 IBMVFC_AE_RESUME = 0x0800,
586 IBMVFC_AE_ADAPTER_FAILED = 0x1000,
587 IBMVFC_AE_FPIN = 0x2000,
588 };
589
590 struct ibmvfc_async_desc {
591 const char *desc;
592 enum ibmvfc_async_event ae;
593 int log_level;
594 };
595
596 struct ibmvfc_crq {
597 volatile u8 valid;
598 volatile u8 format;
599 u8 reserved[6];
600 volatile __be64 ioba;
601 }__attribute__((packed, aligned (8)));
602
603 struct ibmvfc_crq_queue {
604 struct ibmvfc_crq *msgs;
605 int size, cur;
606 dma_addr_t msg_token;
607 };
608
609 enum ibmvfc_ae_link_state {
610 IBMVFC_AE_LS_LINK_UP = 0x01,
611 IBMVFC_AE_LS_LINK_BOUNCED = 0x02,
612 IBMVFC_AE_LS_LINK_DOWN = 0x04,
613 IBMVFC_AE_LS_LINK_DEAD = 0x08,
614 };
615
616 enum ibmvfc_ae_fpin_status {
617 IBMVFC_AE_FPIN_LINK_CONGESTED = 0x1,
618 IBMVFC_AE_FPIN_PORT_CONGESTED = 0x2,
619 IBMVFC_AE_FPIN_PORT_CLEARED = 0x3,
620 IBMVFC_AE_FPIN_PORT_DEGRADED = 0x4,
621 };
622
623 struct ibmvfc_async_crq {
624 volatile u8 valid;
625 u8 link_state;
626 u8 fpin_status
> 627 u8 pad;
628 __be32 pad2;
629 volatile __be64 event;
630 volatile __be64 scsi_id;
631 volatile __be64 wwpn;
632 volatile __be64 node_name;
633 __be64 reserved;
634 }__attribute__((packed, aligned (8)));
635
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26306 bytes --]
^ permalink raw reply
* [RESEND][PATCH 7/7] parisc: Avoid overflow at boundary_size
From: Nicolin Chen @ 2020-08-31 20:38 UTC (permalink / raw)
To: mpe, benh, paulus, rth, ink, mattst88, tony.luck, fenghua.yu,
schnelle, gerald.schaefer, hca, gor, borntraeger, davem, tglx,
mingo, bp, x86, hpa, James.Bottomley, deller
Cc: sfr, linux-ia64, linux-parisc, linux-s390, linux-kernel,
linux-alpha, sparclinux, linuxppc-dev, hch
In-Reply-To: <20200831203811.8494-1-nicoleotsuka@gmail.com>
The boundary_size might be as large as ULONG_MAX, which means
that a device has no specific boundary limit. So either "+ 1"
or passing it to ALIGN() would potentially overflow.
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 fixing a potential overflow with the safer shortcut.
Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
---
drivers/parisc/ccio-dma.c | 4 ++--
drivers/parisc/sba_iommu.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
index a5507f75b524..c667d6aba764 100644
--- a/drivers/parisc/ccio-dma.c
+++ b/drivers/parisc/ccio-dma.c
@@ -356,8 +356,8 @@ ccio_alloc_range(struct ioc *ioc, struct device *dev, size_t size)
** ggg sacrifices another 710 to the computer gods.
*/
- boundary_size = ALIGN((unsigned long long)dma_get_seg_boundary(dev) + 1,
- 1ULL << IOVP_SHIFT) >> IOVP_SHIFT;
+ /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
+ boundary_size = (dma_get_seg_boundary(dev) >> IOVP_SHIFT) + 1;
if (pages_needed <= 8) {
/*
diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
index d4314fba0269..96bc2c617cbd 100644
--- a/drivers/parisc/sba_iommu.c
+++ b/drivers/parisc/sba_iommu.c
@@ -342,8 +342,8 @@ sba_search_bitmap(struct ioc *ioc, struct device *dev,
unsigned long shift;
int ret;
- boundary_size = ALIGN((unsigned long long)dma_get_seg_boundary(dev) + 1,
- 1ULL << IOVP_SHIFT) >> IOVP_SHIFT;
+ /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
+ boundary_size = (dma_get_seg_boundary(dev) >> IOVP_SHIFT) + 1;
#if defined(ZX1_SUPPORT)
BUG_ON(ioc->ibase & ~IOVP_MASK);
--
2.17.1
^ permalink raw reply related
* [RESEND][PATCH 6/7] x86/amd_gart: Avoid overflow at boundary_size
From: Nicolin Chen @ 2020-08-31 20:38 UTC (permalink / raw)
To: mpe, benh, paulus, rth, ink, mattst88, tony.luck, fenghua.yu,
schnelle, gerald.schaefer, hca, gor, borntraeger, davem, tglx,
mingo, bp, x86, hpa, James.Bottomley, deller
Cc: sfr, linux-ia64, linux-parisc, linux-s390, linux-kernel,
linux-alpha, sparclinux, linuxppc-dev, hch
In-Reply-To: <20200831203811.8494-1-nicoleotsuka@gmail.com>
The boundary_size might be as large as ULONG_MAX, which means
that a device has no specific boundary limit. So either "+ 1"
or passing it to ALIGN() would potentially overflow.
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 fixing a potential overflow with the safer shortcut.
Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
---
arch/x86/kernel/amd_gart_64.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
index e89031e9c847..7fa0bb490065 100644
--- a/arch/x86/kernel/amd_gart_64.c
+++ b/arch/x86/kernel/amd_gart_64.c
@@ -96,8 +96,8 @@ static unsigned long alloc_iommu(struct device *dev, int size,
base_index = ALIGN(iommu_bus_base & dma_get_seg_boundary(dev),
PAGE_SIZE) >> PAGE_SHIFT;
- boundary_size = ALIGN((u64)dma_get_seg_boundary(dev) + 1,
- PAGE_SIZE) >> PAGE_SHIFT;
+ /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
+ boundary_size = (dma_get_seg_boundary(dev) >> PAGE_SHIFT) + 1;
spin_lock_irqsave(&iommu_bitmap_lock, flags);
offset = iommu_area_alloc(iommu_gart_bitmap, iommu_pages, next_bit,
--
2.17.1
^ permalink raw reply related
* [RESEND][PATCH 5/7] sparc: Avoid overflow at boundary_size
From: Nicolin Chen @ 2020-08-31 20:38 UTC (permalink / raw)
To: mpe, benh, paulus, rth, ink, mattst88, tony.luck, fenghua.yu,
schnelle, gerald.schaefer, hca, gor, borntraeger, davem, tglx,
mingo, bp, x86, hpa, James.Bottomley, deller
Cc: sfr, linux-ia64, linux-parisc, linux-s390, linux-kernel,
linux-alpha, sparclinux, linuxppc-dev, hch
In-Reply-To: <20200831203811.8494-1-nicoleotsuka@gmail.com>
The boundary_size might be as large as ULONG_MAX, which means
that a device has no specific boundary limit. So either "+ 1"
or passing it to ALIGN() would potentially overflow.
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 fixing a potential overflow with the safer shortcut.
Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
---
arch/sparc/kernel/iommu-common.c | 9 +++------
arch/sparc/kernel/iommu.c | 4 ++--
arch/sparc/kernel/pci_sun4v.c | 4 ++--
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/arch/sparc/kernel/iommu-common.c b/arch/sparc/kernel/iommu-common.c
index 59cb16691322..843e71894d04 100644
--- a/arch/sparc/kernel/iommu-common.c
+++ b/arch/sparc/kernel/iommu-common.c
@@ -166,13 +166,10 @@ unsigned long iommu_tbl_range_alloc(struct device *dev,
}
}
- if (dev)
- boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
- 1 << iommu->table_shift);
- else
- boundary_size = ALIGN(1ULL << 32, 1 << iommu->table_shift);
+ boundary_size = dev ? dma_get_seg_boundary(dev) : U32_MAX;
- boundary_size = boundary_size >> iommu->table_shift;
+ /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
+ boundary_size = (boundary_size >> iommu->table_shift) + 1;
/*
* if the skip_span_boundary_check had been set during init, we set
* things up so that iommu_is_span_boundary() merely checks if the
diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c
index 4ae7388b1bff..d981c37305ae 100644
--- a/arch/sparc/kernel/iommu.c
+++ b/arch/sparc/kernel/iommu.c
@@ -472,8 +472,8 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist,
outs->dma_length = 0;
max_seg_size = dma_get_max_seg_size(dev);
- seg_boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
- IO_PAGE_SIZE) >> IO_PAGE_SHIFT;
+ /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
+ seg_boundary_size = (dma_get_seg_boundary(dev) >> IO_PAGE_SHIFT) + 1;
base_shift = iommu->tbl.table_map_base >> IO_PAGE_SHIFT;
for_each_sg(sglist, s, nelems, i) {
unsigned long paddr, npages, entry, out_entry = 0, slen;
diff --git a/arch/sparc/kernel/pci_sun4v.c b/arch/sparc/kernel/pci_sun4v.c
index 14b93c5564e3..233fe8a20cec 100644
--- a/arch/sparc/kernel/pci_sun4v.c
+++ b/arch/sparc/kernel/pci_sun4v.c
@@ -508,8 +508,8 @@ static int dma_4v_map_sg(struct device *dev, struct scatterlist *sglist,
iommu_batch_start(dev, prot, ~0UL);
max_seg_size = dma_get_max_seg_size(dev);
- seg_boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
- IO_PAGE_SIZE) >> IO_PAGE_SHIFT;
+ /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
+ seg_boundary_size = (dma_get_seg_boundary(dev) >> IO_PAGE_SHIFT) + 1;
mask = *dev->dma_mask;
if (!iommu_use_atu(iommu, mask))
--
2.17.1
^ permalink raw reply related
* [RESEND][PATCH 4/7] s390/pci_dma: Avoid overflow at boundary_size
From: Nicolin Chen @ 2020-08-31 20:38 UTC (permalink / raw)
To: mpe, benh, paulus, rth, ink, mattst88, tony.luck, fenghua.yu,
schnelle, gerald.schaefer, hca, gor, borntraeger, davem, tglx,
mingo, bp, x86, hpa, James.Bottomley, deller
Cc: sfr, linux-ia64, linux-parisc, linux-s390, linux-kernel,
linux-alpha, sparclinux, linuxppc-dev, hch
In-Reply-To: <20200831203811.8494-1-nicoleotsuka@gmail.com>
The boundary_size might be as large as ULONG_MAX, which means
that a device has no specific boundary limit. So either "+ 1"
or passing it to ALIGN() would potentially overflow.
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 fixing a potential overflow with the safer shortcut.
Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
---
arch/s390/pci/pci_dma.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index 64b1399a73f0..ecb067acc6d5 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -263,8 +263,8 @@ static unsigned long __dma_alloc_iommu(struct device *dev,
struct zpci_dev *zdev = to_zpci(to_pci_dev(dev));
unsigned long boundary_size;
- boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
- PAGE_SIZE) >> PAGE_SHIFT;
+ /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
+ boundary_size = (dma_get_seg_boundary(dev) >> PAGE_SHIFT) + 1;
return iommu_area_alloc(zdev->iommu_bitmap, zdev->iommu_pages,
start, size, zdev->start_dma >> PAGE_SHIFT,
boundary_size, 0);
--
2.17.1
^ permalink raw reply related
* [RESEND][PATCH 0/7] Avoid overflow at boundary_size
From: Nicolin Chen @ 2020-08-31 20:38 UTC (permalink / raw)
To: mpe, benh, paulus, rth, ink, mattst88, tony.luck, fenghua.yu,
schnelle, gerald.schaefer, hca, gor, borntraeger, davem, tglx,
mingo, bp, x86, hpa, James.Bottomley, deller
Cc: sfr, linux-ia64, linux-parisc, linux-s390, linux-kernel,
linux-alpha, sparclinux, linuxppc-dev, hch
==== For this resend ====
The original series have not been acked at any patch. So I am
resending them, being suggested by Niklas.
==== Coverletter ====
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.
As I don't have these platforms, testings/comments are welcome.
Thanks
Nic
Nicolin Chen (7):
powerpc/iommu: Avoid overflow at boundary_size
alpha: Avoid overflow at boundary_size
ia64/sba_iommu: Avoid overflow at boundary_size
s390/pci_dma: Avoid overflow at boundary_size
sparc: Avoid overflow at boundary_size
x86/amd_gart: Avoid overflow at boundary_size
parisc: Avoid overflow at boundary_size
arch/alpha/kernel/pci_iommu.c | 10 ++++------
arch/ia64/hp/common/sba_iommu.c | 4 ++--
arch/powerpc/kernel/iommu.c | 11 +++++------
arch/s390/pci/pci_dma.c | 4 ++--
arch/sparc/kernel/iommu-common.c | 9 +++------
arch/sparc/kernel/iommu.c | 4 ++--
arch/sparc/kernel/pci_sun4v.c | 4 ++--
arch/x86/kernel/amd_gart_64.c | 4 ++--
drivers/parisc/ccio-dma.c | 4 ++--
drivers/parisc/sba_iommu.c | 4 ++--
10 files changed, 26 insertions(+), 32 deletions(-)
--
2.17.1
^ permalink raw reply
* [RESEND][PATCH 3/7] ia64/sba_iommu: Avoid overflow at boundary_size
From: Nicolin Chen @ 2020-08-31 20:38 UTC (permalink / raw)
To: mpe, benh, paulus, rth, ink, mattst88, tony.luck, fenghua.yu,
schnelle, gerald.schaefer, hca, gor, borntraeger, davem, tglx,
mingo, bp, x86, hpa, James.Bottomley, deller
Cc: sfr, linux-ia64, linux-parisc, linux-s390, linux-kernel,
linux-alpha, sparclinux, linuxppc-dev, hch
In-Reply-To: <20200831203811.8494-1-nicoleotsuka@gmail.com>
The boundary_size might be as large as ULONG_MAX, which means
that a device has no specific boundary limit. So either "+ 1"
or passing it to ALIGN() would potentially overflow.
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 fixing a potential overflow with the safer shortcut.
Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
---
arch/ia64/hp/common/sba_iommu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index 656a4888c300..945954903bb0 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -485,8 +485,8 @@ sba_search_bitmap(struct ioc *ioc, struct device *dev,
ASSERT(((unsigned long) ioc->res_hint & (sizeof(unsigned long) - 1UL)) == 0);
ASSERT(res_ptr < res_end);
- boundary_size = (unsigned long long)dma_get_seg_boundary(dev) + 1;
- boundary_size = ALIGN(boundary_size, 1ULL << iovp_shift) >> iovp_shift;
+ /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
+ boundary_size = (dma_get_seg_boundary(dev) >> iovp_shift) + 1;
BUG_ON(ioc->ibase & ~iovp_mask);
shift = ioc->ibase >> iovp_shift;
--
2.17.1
^ permalink raw reply related
* [RESEND][PATCH 1/7] powerpc/iommu: Avoid overflow at boundary_size
From: Nicolin Chen @ 2020-08-31 20:38 UTC (permalink / raw)
To: mpe, benh, paulus, rth, ink, mattst88, tony.luck, fenghua.yu,
schnelle, gerald.schaefer, hca, gor, borntraeger, davem, tglx,
mingo, bp, x86, hpa, James.Bottomley, deller
Cc: sfr, linux-ia64, linux-parisc, linux-s390, linux-kernel,
linux-alpha, sparclinux, linuxppc-dev, hch
In-Reply-To: <20200831203811.8494-1-nicoleotsuka@gmail.com>
The boundary_size might be as large as ULONG_MAX, which means
that a device has no specific boundary limit. So either "+ 1"
or passing it to ALIGN() would potentially overflow.
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 fixing a potential overflow with the safer shortcut.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
---
arch/powerpc/kernel/iommu.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 9704f3f76e63..c01ccbf8afdd 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -236,15 +236,14 @@ static unsigned long iommu_range_alloc(struct device *dev,
}
}
- if (dev)
- boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
- 1 << tbl->it_page_shift);
- else
- boundary_size = ALIGN(1UL << 32, 1 << tbl->it_page_shift);
/* 4GB boundary for iseries_hv_alloc and iseries_hv_map */
+ boundary_size = dev ? dma_get_seg_boundary(dev) : U32_MAX;
+
+ /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
+ boundary_size = (boundary_size >> tbl->it_page_shift) + 1;
n = iommu_area_alloc(tbl->it_map, limit, start, npages, tbl->it_offset,
- boundary_size >> tbl->it_page_shift, align_mask);
+ boundary_size, align_mask);
if (n == -1) {
if (likely(pass == 0)) {
/* First try the pool from the start */
--
2.17.1
^ permalink raw reply related
* [RESEND][PATCH 2/7] alpha: Avoid overflow at boundary_size
From: Nicolin Chen @ 2020-08-31 20:38 UTC (permalink / raw)
To: mpe, benh, paulus, rth, ink, mattst88, tony.luck, fenghua.yu,
schnelle, gerald.schaefer, hca, gor, borntraeger, davem, tglx,
mingo, bp, x86, hpa, James.Bottomley, deller
Cc: sfr, linux-ia64, linux-parisc, linux-s390, linux-kernel,
linux-alpha, sparclinux, linuxppc-dev, hch
In-Reply-To: <20200831203811.8494-1-nicoleotsuka@gmail.com>
The boundary_size might be as large as ULONG_MAX, which means
that a device has no specific boundary limit. So "+ 1" would
potentially overflow.
Also, by following other places in the kernel, boundary_size
should align with the PAGE_SIZE before right shifting by the
PAGE_SHIFT. However, passing it to ALIGN() would potentially
overflow too.
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 fixing a potential overflow with the safer shortcut.
Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: Christoph Hellwig <hch@lst.de>
---
arch/alpha/kernel/pci_iommu.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index 81037907268d..1ef2c647bd3e 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -141,12 +141,10 @@ iommu_arena_find_pages(struct device *dev, struct pci_iommu_arena *arena,
unsigned long boundary_size;
base = arena->dma_base >> PAGE_SHIFT;
- if (dev) {
- boundary_size = dma_get_seg_boundary(dev) + 1;
- boundary_size >>= PAGE_SHIFT;
- } else {
- boundary_size = 1UL << (32 - PAGE_SHIFT);
- }
+
+ boundary_size = dev ? dma_get_seg_boundary(dev) : U32_MAX;
+ /* Overflow-free shortcut for: ALIGN(b + 1, 1 << s) >> s */
+ boundary_size = (boundary_size >> PAGE_SHIFT) + 1;
/* Search forward for the first mask-aligned sequence of N free ptes */
ptes = arena->ptes;
--
2.17.1
^ permalink raw reply related
* [Bug 209029] kernel 5.9-rc2 fails to boot on a PowerMac G5 11,2 - BUG: Kernel NULL pointer dereference on read at 0x00000020
From: bugzilla-daemon @ 2020-08-31 20:09 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-209029-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=209029
--- Comment #2 from Erhard F. (erhard_f@mailbox.org) ---
No change with 5.9-rc3.
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH net-next] net/wan/fsl_ucc_hdlc: Add MODULE_DESCRIPTION
From: David Miller @ 2020-08-31 19:42 UTC (permalink / raw)
To: yuehaibing; +Cc: kuba, netdev, linuxppc-dev, linux-kernel, qiang.zhao
In-Reply-To: <20200829115823.10140-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Sat, 29 Aug 2020 19:58:23 +0800
> Add missing MODULE_DESCRIPTION.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* [RFC PATCH 2/2] powerpc/kernel/iommu: Introduce IOMMU DMA pagecache
From: Leonardo Bras @ 2020-08-31 18:48 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Joel Stanley, Christophe Leroy, Nayna Jain,
Eric Richter, Nicholas Piggin, Hari Bathini, Masahiro Yamada,
Brian King, Murilo Fossa Vicentini, David Dai, Matthew Wilcox
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200831184859.110660-1-leobras.c@gmail.com>
In pseries, mapping a DMA page for a cpu memory range requires a
H_PUT_TCE* hypercall, and unmapping requires a H_STUFF_TCE hypercall.
When doing a lot of I/O, a thread can spend a lot of time doing such
hcalls, specially when a DMA mapping don't get reused.
The purpose of this change is to introduce a mechanism similar to
a pagecache, but for reusing DMA mappings, improving performance and
avoiding multiple DMA mappings of the same cpupage.
This works based in a few current behaviors:
- Page reuse:
It's common for userspace process to reuse the same page for several
allocations This is probably caused by page buffering behavior that
come from libc, so getting pages from the kernel is less expensive.
- A lot of small allocations are used:
Some workloads do a lot of allocations that do not fill a whole page
causing multiple DMA mappings for a single page.
How it work:
- When a DMA mapping is required for given allocation, it first searches
the DMA pagecache for a matching mapping. When found, increment refcount,
when not found, a new mapping is created.
- Every time a new mapping is created, it's added to the DMA pagecache,
with refcount = 1;
- When the mapping is not needed anymore (iommu_free()), it looks in the
DMA pagecache for the entry, and the decrements it's refcount.
- When there are more than IOMMU_MAP_LIST_MAX entries in the
DMA pagecache, it removes the older ones.
What is inside:
- 1 XArray which indexes the DMA page addresses, used for removing
mappings and decreasing refcounts.
- 1 XArray which indexes CPU page addresses, for finding matching mappings.
- As there can be multiple mappings (directions) for the same cpupage,
this one keeps a llist for looking into the entries.
- Every entry points (page) in the XArray points to the mapping struct.
- The mapping struct have:
- DMA & CPU page addresses, size (pages) , direction and refcount.
- 1 llist used for multiple mappings at the same cpupage
- 1 llist used for the FIFO (removing old unused entries)
- The cache struct, added to iommu_table, have:
- Both XArrays,
- 2 llist for entry/removal point on FIFO,
- 1 Atomic Cachesize, to manage the FIFO.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
---
arch/powerpc/include/asm/iommu-cache.h | 31 ++++
arch/powerpc/include/asm/iommu.h | 4 +
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/iommu-cache.c | 247 +++++++++++++++++++++++++
arch/powerpc/kernel/iommu.c | 15 +-
5 files changed, 293 insertions(+), 6 deletions(-)
create mode 100644 arch/powerpc/include/asm/iommu-cache.h
create mode 100644 arch/powerpc/kernel/iommu-cache.c
diff --git a/arch/powerpc/include/asm/iommu-cache.h b/arch/powerpc/include/asm/iommu-cache.h
new file mode 100644
index 000000000000..ad298a4cd9c9
--- /dev/null
+++ b/arch/powerpc/include/asm/iommu-cache.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _IOMMU_CACHE_H
+#define _IOMMU_CACHE_H
+#ifdef __KERNEL__
+
+#include <linux/llist.h>
+#include <linux/xarray.h>
+#include <linux/atomic.h>
+
+struct dmacache {
+ struct llist_head fifo_add;
+ struct llist_head fifo_del;
+ struct xarray cpupages;
+ struct xarray dmapages;
+ atomic64_t cachesize;
+};
+
+#include <asm/iommu.h>
+
+void iommu_cache_init(struct iommu_table *tbl);
+void iommu_dmacache_add(struct iommu_table *tbl, void *page, unsigned int npages, dma_addr_t addr,
+ enum dma_data_direction direction);
+dma_addr_t iommu_dmacache_use(struct iommu_table *tbl, void *page, unsigned int npages,
+ enum dma_data_direction direction);
+void iommu_dmacache_free(struct iommu_table *tbl, dma_addr_t dma_handle, unsigned int npages);
+
+#define IOMMU_MAP_LIST_MAX 8192
+#define IOMMU_MAP_LIST_THRES 128
+
+#endif /* __KERNEL__ */
+#endif /* _IOMMU_CACHE_H */
diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index 2913e5c8b1f8..51a2f5503f8e 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -18,6 +18,7 @@
#include <asm/types.h>
#include <asm/pci-bridge.h>
#include <asm/asm-const.h>
+#include <asm/iommu-cache.h>
#define IOMMU_PAGE_SHIFT_4K 12
#define IOMMU_PAGE_SIZE_4K (ASM_CONST(1) << IOMMU_PAGE_SHIFT_4K)
@@ -114,6 +115,7 @@ struct iommu_table {
int it_nid;
unsigned long it_reserved_start; /* Start of not-DMA-able (MMIO) area */
unsigned long it_reserved_end;
+ struct dmacache cache;
};
#define IOMMU_TABLE_USERSPACE_ENTRY_RO(tbl, entry) \
@@ -317,6 +319,8 @@ extern void iommu_release_ownership(struct iommu_table *tbl);
extern enum dma_data_direction iommu_tce_direction(unsigned long tce);
extern unsigned long iommu_direction_to_tce_perm(enum dma_data_direction dir);
+void __iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr, unsigned int npages);
+
#ifdef CONFIG_PPC_CELL_NATIVE
extern bool iommu_fixed_is_weak;
#else
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index cbf41fb4ee89..62f6c90076d9 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -104,7 +104,7 @@ extra-y += vmlinux.lds
obj-$(CONFIG_RELOCATABLE) += reloc_$(BITS).o
obj-$(CONFIG_PPC32) += entry_32.o setup_32.o early_32.o
-obj-$(CONFIG_PPC64) += dma-iommu.o iommu.o
+obj-$(CONFIG_PPC64) += dma-iommu.o iommu.o iommu-cache.o
obj-$(CONFIG_KGDB) += kgdb.o
obj-$(CONFIG_BOOTX_TEXT) += btext.o
obj-$(CONFIG_SMP) += smp.o
diff --git a/arch/powerpc/kernel/iommu-cache.c b/arch/powerpc/kernel/iommu-cache.c
new file mode 100644
index 000000000000..3a5b2725ab51
--- /dev/null
+++ b/arch/powerpc/kernel/iommu-cache.c
@@ -0,0 +1,247 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <asm/iommu-cache.h>
+
+struct dma_mapping {
+ struct llist_node mapping;
+ struct llist_node fifo;
+ unsigned long dmapage;
+ unsigned long cpupage;
+ unsigned long size;
+ refcount_t count;
+ enum dma_data_direction direction;
+
+};
+
+struct cpupage_entry {
+ struct llist_node node;
+ struct dma_mapping *data;
+};
+
+/**
+ * iommu_dmacache_use() - Looks for a DMA mapping in cache
+ * @tbl: Device's iommu_table.
+ * @page: Address for which a DMA mapping is desired.
+ * @npages: Page count needed from that address
+ * @direction: DMA direction needed for the mapping
+ *
+ * Looks into the DMA cache for a page/range that is already mapped with given direction.
+ *
+ * Return: DMA mapping for range/direction present in cache
+ * DMA_MAPPING_ERROR if not found.
+ */
+dma_addr_t iommu_dmacache_use(struct iommu_table *tbl, void *page, unsigned int npages,
+ enum dma_data_direction direction)
+{
+ struct cpupage_entry *e;
+ struct dma_mapping *d;
+ const unsigned long start = (unsigned long)page >> tbl->it_page_shift;
+ const unsigned long end = start + npages;
+
+ e = xa_load(&tbl->cache.cpupages, start);
+ if (!e)
+ return DMA_MAPPING_ERROR;
+
+ llist_for_each_entry(e, &e->node, node) {
+ d = e->data;
+ if (start < d->cpupage || end > (d->cpupage + d->size) ||
+ !DMA_DIR_COMPAT(d->direction, direction))
+ continue;
+
+ refcount_inc(&d->count);
+ return (d->dmapage + start - d->cpupage) << tbl->it_page_shift;
+ }
+
+ return DMA_MAPPING_ERROR;
+}
+
+/**
+ * iommu_dmacache_entry_remove() - Remove a dma mapping from cpupage & dmapage XArrays
+ * @cache: Device's dmacache.
+ * @d: dma_mapping to be removed
+ */
+static void iommu_dmacache_entry_remove(struct dmacache *cache, struct dma_mapping *d)
+{
+ struct cpupage_entry *e, *tmp;
+ dma_addr_t dp = d->dmapage;
+ dma_addr_t end = dp + d->size;
+ unsigned long cp = d->cpupage;
+
+ for (; dp < end; dp++, cp++) {
+ e = xa_erase(&cache->cpupages, cp);
+ if (e && e->node.next) {
+ tmp = llist_entry(e->node.next, struct cpupage_entry, node);
+ xa_store(&cache->cpupages, cp, tmp, GFP_KERNEL);
+ }
+ xa_erase(&cache->dmapages, dp);
+ kfree(e);
+ }
+}
+
+/**
+ * iommu_dmacache_clean() - Clean count mappings from dmacache fifo
+ * @tbl: Device's iommu_table.
+ * @count: number of entries to be removed.
+ */
+static void iommu_dmacache_clean(struct iommu_table *tbl, const long count)
+{
+ struct dma_mapping *d, *tmp;
+ struct llist_node *n;
+ struct dmacache *cache = &tbl->cache;
+ unsigned long removed = 0;
+
+ n = llist_del_all(&cache->fifo_del);
+
+ if (!n)
+ return;
+
+ llist_for_each_entry_safe(d, tmp, n, fifo) {
+ switch (refcount_read(&d->count)) {
+ case 0:
+ /* Fully remove entry */
+ iommu_dmacache_entry_remove(cache, d);
+ __iommu_free(tbl, d->dmapage << tbl->it_page_shift, d->size);
+ kfree(d);
+ removed++;
+ break;
+ case 1:
+ /* Remove entry but don't undo mapping */
+ iommu_dmacache_entry_remove(cache, d);
+ kfree(d);
+ removed++;
+ break;
+ default:
+ /* In use. Re-add it to list. */
+ n = xchg(&tbl->cache.fifo_add.first, &d->fifo);
+ if (!n)
+ n->next = &d->fifo;
+
+ break;
+ }
+
+ if (removed >= count)
+ break;
+ }
+
+ atomic64_sub(removed, &tbl->cache.cachesize);
+
+ xchg(&tbl->cache.fifo_del.first, &tmp->fifo);
+}
+
+/**
+ * iommu_dmacache_free() - Decrement a mapping usage from dmacache and clean when full
+ * @tbl: Device's iommu_table.
+ * @dma_handle: DMA address from the mapping.
+ * @npages: Page count from that address
+ *
+ * Decrements a refcount for a mapping in this dma_handle + npages, and remove
+ * some unused dma mappings from dmacache fifo.
+ */
+void iommu_dmacache_free(struct iommu_table *tbl, dma_addr_t dma_handle, unsigned int npages)
+{
+ struct dma_mapping *d;
+ long exceeding;
+
+ d = xa_load(&tbl->cache.dmapages, dma_handle >> tbl->it_page_shift);
+ if (!d) {
+ /* Not in list, just free */
+ __iommu_free(tbl, dma_handle, npages);
+ return;
+ }
+
+ refcount_dec(&d->count);
+
+ exceeding = atomic64_read(&tbl->cache.cachesize) - IOMMU_MAP_LIST_MAX;
+
+ if (exceeding > 0)
+ iommu_dmacache_clean(tbl, exceeding + IOMMU_MAP_LIST_THRES);
+}
+
+/**
+ * iommu_dmacache_add() - Create and add a new dma mapping into cache.
+ * @tbl: Device's iommu_table.
+ * @page: Address for which a DMA mapping was created.
+ * @npages: Page count mapped from that address
+ * @addr: DMA address created for that mapping
+ * @direction: DMA direction for the mapping created
+ *
+ * Create a dma_mapping and add it to dmapages and cpupages XArray, then add it to fifo.
+ * On both cpupages and dmapages, an entry will be created for each page in the mapping.
+ * On cpupages, as there may exist many mappings for a single cpupage, each entry has a llist
+ * that starts at the last mapped entry.
+ *
+ */
+void iommu_dmacache_add(struct iommu_table *tbl, void *page, unsigned int npages, dma_addr_t addr,
+ enum dma_data_direction direction)
+{
+ struct dma_mapping *d, *tmp;
+ struct cpupage_entry *e, *old;
+ struct llist_node *n;
+ unsigned long p = (unsigned long)page;
+ unsigned int i;
+
+ d = kmalloc(sizeof(*d), GFP_KERNEL);
+ if (!d)
+ return;
+
+ d->cpupage = (unsigned long)p >> tbl->it_page_shift;
+ d->dmapage = (unsigned long)addr >> tbl->it_page_shift;
+ d->size = npages;
+ d->direction = direction;
+ d->fifo.next = NULL;
+ refcount_set(&d->count, 1);
+
+ p = d->cpupage;
+ addr = d->dmapage;
+
+ for (i = 0; i < npages ; i++) {
+ /* Only one mapping may exist for a DMA address*/
+ tmp = xa_store(&tbl->cache.dmapages, addr++, d, GFP_KERNEL);
+ if (xa_is_err(tmp))
+ break;
+
+ /* Multiple mappings may exist for a page, get them in a list*/
+ e = kmalloc(sizeof(*e), GFP_KERNEL);
+ if (!d)
+ break;
+
+ e->data = d;
+ old = xa_store(&tbl->cache.cpupages, p++, e, GFP_KERNEL);
+ e->node.next = &old->node;
+
+ if (xa_is_err(old)) {
+ kfree(e);
+ break;
+ }
+ }
+
+ n = xchg(&tbl->cache.fifo_add.first, &d->fifo);
+ if (!n)
+ n->next = &d->fifo;
+
+ atomic64_inc(&tbl->cache.cachesize);
+}
+
+void iommu_cache_init(struct iommu_table *tbl)
+{
+ struct dma_mapping *d;
+
+ init_llist_head(&tbl->cache.fifo_add);
+ init_llist_head(&tbl->cache.fifo_del);
+
+ /* First entry for linking both llist_heads */
+ d = kmalloc(sizeof(*d), GFP_KERNEL);
+ if (!d)
+ panic("%s: Can't allocate %ld bytes\n", __func__, sizeof(*d));
+
+ d->cpupage = -1UL;
+ d->dmapage = -1UL;
+ refcount_set(&d->count, 1);
+ llist_add(&d->fifo, &tbl->cache.fifo_add);
+ llist_add(&d->fifo, &tbl->cache.fifo_del);
+
+ xa_init(&tbl->cache.cpupages);
+ xa_init(&tbl->cache.dmapages);
+
+ atomic64_set(&tbl->cache.cachesize, 0);
+}
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index c5d5d36ab65e..60703af172ad 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -40,8 +40,6 @@
static int novmerge;
-static void __iommu_free(struct iommu_table *, dma_addr_t, unsigned int);
-
static int __init setup_iommu(char *str)
{
if (!strcmp(str, "novmerge"))
@@ -308,6 +306,10 @@ static dma_addr_t iommu_alloc(struct device *dev, struct iommu_table *tbl,
dma_addr_t ret = DMA_MAPPING_ERROR;
int build_fail;
+ ret = iommu_dmacache_use(tbl, page, npages, direction);
+ if (ret != DMA_MAPPING_ERROR)
+ return ret;
+
entry = iommu_range_alloc(dev, tbl, npages, NULL, mask, align_order);
if (unlikely(entry == DMA_MAPPING_ERROR))
@@ -338,6 +340,8 @@ static dma_addr_t iommu_alloc(struct device *dev, struct iommu_table *tbl,
/* Make sure updates are seen by hardware */
mb();
+ iommu_dmacache_add(tbl, page, npages, ret, direction);
+
return ret;
}
@@ -388,8 +392,7 @@ static struct iommu_pool *get_pool(struct iommu_table *tbl,
return p;
}
-static void __iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr,
- unsigned int npages)
+void __iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr, unsigned int npages)
{
unsigned long entry, free_entry;
unsigned long flags;
@@ -413,7 +416,7 @@ static void __iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr,
static void iommu_free(struct iommu_table *tbl, dma_addr_t dma_addr,
unsigned int npages)
{
- __iommu_free(tbl, dma_addr, npages);
+ iommu_dmacache_free(tbl, dma_addr, npages);
/* Make sure TLB cache is flushed if the HW needs it. We do
* not do an mb() here on purpose, it is not needed on any of
@@ -719,6 +722,8 @@ struct iommu_table *iommu_init_table(struct iommu_table *tbl, int nid,
iommu_table_clear(tbl);
+ iommu_cache_init(tbl);
+
if (!welcomed) {
printk(KERN_INFO "IOMMU table initialized, virtual merging %s\n",
novmerge ? "disabled" : "enabled");
--
2.25.4
^ permalink raw reply related
* [RFC PATCH 1/2] dma-direction: Add DMA_DIR_COMPAT() macro to test direction compability
From: Leonardo Bras @ 2020-08-31 18:48 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Joel Stanley, Christophe Leroy, Nayna Jain,
Eric Richter, Nicholas Piggin, Hari Bathini, Masahiro Yamada,
Brian King, Murilo Fossa Vicentini, David Dai, Matthew Wilcox
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200831184859.110660-1-leobras.c@gmail.com>
Given a existing mapping with 'current' direction, and a 'wanted'
direction for using that mapping, check if 'wanted' is satisfied by
'current'.
current accepts
DMA_BIDIRECTIONAL DMA_BIDIRECTIONAL, DMA_TO_DEVICE,
DMA_FROM_DEVICE, DMA_NONE
DMA_TO_DEVICE DMA_TO_DEVICE, DMA_NONE
DMA_FROM_DEVICE DMA_FROM_DEVICE, DMA_NONE
DMA_NONE DMA_NONE
This macro is useful for checking if a DMA mapping can be reused.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
---
include/linux/dma-direction.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/dma-direction.h b/include/linux/dma-direction.h
index 9c96e30e6a0b..caf3943a21f4 100644
--- a/include/linux/dma-direction.h
+++ b/include/linux/dma-direction.h
@@ -9,4 +9,7 @@ enum dma_data_direction {
DMA_NONE = 3,
};
+/* Checks if wanted direction is satisfied by current mapping direction*/
+#define DMA_DIR_COMPAT(current, wanted) (((current) & ~(wanted)) == 0)
+
#endif
--
2.25.4
^ permalink raw reply related
* [RFC PATCH 0/2] DMA pagecache
From: Leonardo Bras @ 2020-08-31 18:48 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Leonardo Bras, Joel Stanley, Christophe Leroy, Nayna Jain,
Eric Richter, Nicholas Piggin, Hari Bathini, Masahiro Yamada,
Brian King, Murilo Fossa Vicentini, David Dai, Matthew Wilcox
Cc: linuxppc-dev, linux-kernel
This RFC improves the performance of indirect mapping on all tested DMA
usages, based on a mlx5 device, ranging from 64k packages to 1-byte
packages, from 1 thread to 64 threads.
In all workloads tested, the performance of indirect mapping gets very
near to direct mapping case.
The whole thing is designed to have as much perfomance as possible, so
the impact of the pagecache is not too big.
As I am not very experienced in XArrays usage, nor in lockless
algorithms, I would specially appreaciate feedback on possible
failures on it's usage, missing barriers, and so on.
Also, this size for the FIFO is just for testing purposes.
It's also very possible that it will not be a good idea in platforms
other than pseries, (i have not tested them).
I can plan I bypass for those cases without much work.
Thank you!
Leonardo Bras (2):
dma-direction: Add DMA_DIR_COMPAT() macro to test direction
compability
powerpc/kernel/iommu: Introduce IOMMU DMA pagecache
arch/powerpc/include/asm/iommu-cache.h | 31 ++++
arch/powerpc/include/asm/iommu.h | 4 +
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/iommu-cache.c | 247 +++++++++++++++++++++++++
arch/powerpc/kernel/iommu.c | 15 +-
include/linux/dma-direction.h | 3 +
6 files changed, 296 insertions(+), 6 deletions(-)
create mode 100644 arch/powerpc/include/asm/iommu-cache.h
create mode 100644 arch/powerpc/kernel/iommu-cache.c
--
2.25.4
^ permalink raw reply
* [PATCH] arch: vdso: add vdso linker script to 'targets' instead of extra-y
From: Masahiro Yamada @ 2020-08-31 18:22 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-s390, Vasily Gorbik, Nick Hu, Heiko Carstens, linuxppc-dev,
linux-kernel, Christian Borntraeger, Paul Mackerras, Greentime Hu,
Catalin Marinas, Vincent Chen, Will Deacon, Masahiro Yamada,
linux-arm-kernel
The vdso linker script is preprocessed on demand.
Adding it to 'targets' is enough to include the .cmd file.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
arch/arm64/kernel/vdso/Makefile | 2 +-
arch/arm64/kernel/vdso32/Makefile | 2 +-
arch/nds32/kernel/vdso/Makefile | 2 +-
arch/powerpc/kernel/vdso32/Makefile | 2 +-
arch/powerpc/kernel/vdso64/Makefile | 2 +-
arch/s390/kernel/vdso64/Makefile | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
index 45d5cfe46429..7cd8aafbe96e 100644
--- a/arch/arm64/kernel/vdso/Makefile
+++ b/arch/arm64/kernel/vdso/Makefile
@@ -54,7 +54,7 @@ endif
GCOV_PROFILE := n
obj-y += vdso.o
-extra-y += vdso.lds
+targets += vdso.lds
CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
# Force dependency (incbin is bad)
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index d6adb4677c25..572475b7b7ed 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -155,7 +155,7 @@ asm-obj-vdso := $(addprefix $(obj)/, $(asm-obj-vdso))
obj-vdso := $(c-obj-vdso) $(c-obj-vdso-gettimeofday) $(asm-obj-vdso)
obj-y += vdso.o
-extra-y += vdso.lds
+targets += vdso.lds
CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
# Force dependency (vdso.s includes vdso.so through incbin)
diff --git a/arch/nds32/kernel/vdso/Makefile b/arch/nds32/kernel/vdso/Makefile
index 7c3c1ccb196e..55df25ef0057 100644
--- a/arch/nds32/kernel/vdso/Makefile
+++ b/arch/nds32/kernel/vdso/Makefile
@@ -20,7 +20,7 @@ GCOV_PROFILE := n
obj-y += vdso.o
-extra-y += vdso.lds
+targets += vdso.lds
CPPFLAGS_vdso.lds += -P -C -U$(ARCH)
# Force dependency
diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
index 87ab1152d5ce..fd5072a4c73c 100644
--- a/arch/powerpc/kernel/vdso32/Makefile
+++ b/arch/powerpc/kernel/vdso32/Makefile
@@ -29,7 +29,7 @@ ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
asflags-y := -D__VDSO32__ -s
obj-y += vdso32_wrapper.o
-extra-y += vdso32.lds
+targets += vdso32.lds
CPPFLAGS_vdso32.lds += -P -C -Upowerpc
# Force dependency (incbin is bad)
diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
index 38c317f25141..c737b3ea3207 100644
--- a/arch/powerpc/kernel/vdso64/Makefile
+++ b/arch/powerpc/kernel/vdso64/Makefile
@@ -17,7 +17,7 @@ ccflags-y := -shared -fno-common -fno-builtin -nostdlib \
asflags-y := -D__VDSO64__ -s
obj-y += vdso64_wrapper.o
-extra-y += vdso64.lds
+targets += vdso64.lds
CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)
# Force dependency (incbin is bad)
diff --git a/arch/s390/kernel/vdso64/Makefile b/arch/s390/kernel/vdso64/Makefile
index 4a66a1cb919b..d0d406cfffa9 100644
--- a/arch/s390/kernel/vdso64/Makefile
+++ b/arch/s390/kernel/vdso64/Makefile
@@ -25,7 +25,7 @@ $(targets:%=$(obj)/%.dbg): KBUILD_CFLAGS = $(KBUILD_CFLAGS_64)
$(targets:%=$(obj)/%.dbg): KBUILD_AFLAGS = $(KBUILD_AFLAGS_64)
obj-y += vdso64_wrapper.o
-extra-y += vdso64.lds
+targets += vdso64.lds
CPPFLAGS_vdso64.lds += -P -C -U$(ARCH)
# Disable gcov profiling, ubsan and kasan for VDSO code
--
2.25.1
^ permalink raw reply related
* [PATCH] scsi: ibmvfc: interface updates for future FPIN and MQ support
From: Tyrel Datwyler @ 2020-08-31 17:18 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
VIOS partitions with SLI-4 enabled Emulex adapters will be capable of
driving IO in parallel through mulitple work queues or channels, and
with new hyperviosr firmware that supports multiple interrupt sources
an ibmvfc NPIV single initiator can be modified to exploit end to end
channelization in a PowerVM environment.
VIOS hosts will also be able to expose fabric perfromance impact
notifications (FPIN) via a new asynchronous event to ibmvfc clients that
advertise support via IBMVFC_CAN_HANDLE_FPIN in their capabilities flag
during NPIV_LOGIN.
This patch introduces three new Managment Datagrams (MADs) for
channelization support negotiation as well as the FPIN asynchronous
event and FPIN status flags. Follow up work is required to plumb the
ibmvfc client driver to use these new interfaces.
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.h | 66 +++++++++++++++++++++++++++++++++-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 907889f1fa9d..801681b63daa 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -124,6 +124,9 @@ enum ibmvfc_mad_types {
IBMVFC_PASSTHRU = 0x0200,
IBMVFC_TMF_MAD = 0x0100,
IBMVFC_NPIV_LOGOUT = 0x0800,
+ IBMVFC_CHANNEL_ENQUIRY = 0x1000,
+ IBMVFC_CHANNEL_SETUP = 0x2000,
+ IBMVFC_CONNECTION_INFO = 0x4000,
};
struct ibmvfc_mad_common {
@@ -162,6 +165,8 @@ struct ibmvfc_npiv_login {
__be32 max_cmds;
__be64 capabilities;
#define IBMVFC_CAN_MIGRATE 0x01
+#define IBMVFC_CAN_USE_CHANNELS 0x02
+#define IBMVFC_CAN_HANDLE_FPIN 0x04
__be64 node_name;
struct srp_direct_buf async;
u8 partition_name[IBMVFC_MAX_NAME];
@@ -204,6 +209,7 @@ struct ibmvfc_npiv_login_resp {
__be64 capabilities;
#define IBMVFC_CAN_FLUSH_ON_HALT 0x08
#define IBMVFC_CAN_SUPPRESS_ABTS 0x10
+#define IBMVFC_CAN_SUPPORT_CHANNELS 0x20
__be32 max_cmds;
__be32 scsi_id_sz;
__be64 max_dma_len;
@@ -482,6 +488,52 @@ struct ibmvfc_passthru_mad {
struct ibmvfc_passthru_fc_iu fc_iu;
}__attribute__((packed, aligned (8)));
+struct ibmvfc_channel_enquiry {
+ struct ibmvfc_mad_common common;
+ __be32 flags;
+#define IBMVFC_NO_CHANNELS_TO_CRQ_SUPPORT 0x01
+#define IBMVFC_SUPPORT_VARIABLE_SUBQ_MSG 0x02
+#define IBMVFC_NO_N_TO_M_CHANNELS_SUPPORT 0x04
+ __be32 num_scsi_subq_channels;
+ __be32 num_nvmeof_subq_channels;
+ __be32 num_scsi_vas_channels;
+ __be32 num nvmeof_vas_channels;
+}__attribute__((packed, aligned (8)));
+
+struct ibmvfc_channel_setup_mad {
+ struct ibmvfc_mad_common common;
+ struct srp_direct_buf buffer;
+}__attrribute__((packed, aligned (8)));
+
+#define IBMVFC_MAX_CHANNELS 502
+
+struct ibmvfc_channel_setup {
+ __be32 flags;
+#define IBMVFC_CANCEL_CHANNELS 0x01
+#define IBMVFC_USE_BUFFER 0x02
+#define IBMVFC_CHANNELS_CANCELED 0x04
+ __be32 reserved;
+ __be32 num_scsi_subq_channels;
+ __be32 num_nvmeof_subq_channels;
+ __be32 num_scsi_vas_channels;
+ __be32 num_nvmeof_vas_channels;
+ struct srp_direct_buf buffer;
+ __be64 reserved2[5];
+ __be64 channel_handles[IBMVFC_MAX_CHANNELS];
+}__attribute__((packed, aligned (8)));
+
+struct ibmvfc_connection_info {
+ struct ibmvfc_madd_common common;
+ __be64 information_bits;
+#define IBMVFC_NO_FC_IO_CHANNEL 0x01
+#define IBMVFC_NO_PHYP_VAS 0x02
+#define IBMVFC_NO_PHYP_SUBQ 0x04
+#define IBMVFC_PHYP_DEPRECATED_SUBQ 0x08
+#define IBMVFC_PHYP_PRESERVED_SUBQ 0x10
+#define IBMVFC_PHYP_FULL_SUBQ 0x20
+ __be64 reserved[16];
+}__attribute__((packed, aligned (8)));
+
struct ibmvfc_trace_start_entry {
u32 xfer_len;
}__attribute__((packed));
@@ -532,6 +584,7 @@ enum ibmvfc_async_event {
IBMVFC_AE_HALT = 0x0400,
IBMVFC_AE_RESUME = 0x0800,
IBMVFC_AE_ADAPTER_FAILED = 0x1000,
+ IBMVFC_AE_FPIN = 0x2000,
};
struct ibmvfc_async_desc {
@@ -560,10 +613,18 @@ enum ibmvfc_ae_link_state {
IBMVFC_AE_LS_LINK_DEAD = 0x08,
};
+enum ibmvfc_ae_fpin_status {
+ IBMVFC_AE_FPIN_LINK_CONGESTED = 0x1,
+ IBMVFC_AE_FPIN_PORT_CONGESTED = 0x2,
+ IBMVFC_AE_FPIN_PORT_CLEARED = 0x3,
+ IBMVFC_AE_FPIN_PORT_DEGRADED = 0x4,
+};
+
struct ibmvfc_async_crq {
volatile u8 valid;
u8 link_state;
- u8 pad[2];
+ u8 fpin_status
+ u8 pad;
__be32 pad2;
volatile __be64 event;
volatile __be64 scsi_id;
@@ -590,6 +651,9 @@ union ibmvfc_iu {
struct ibmvfc_tmf tmf;
struct ibmvfc_cmd cmd;
struct ibmvfc_passthru_mad passthru;
+ struct ibmvfc_channel_enquiry channel_enquiry;
+ struct ibmvfc_channel_setup_mad channel_setup;
+ struct ibmvfc_connection_info connection_info;
}__attribute__((packed, aligned (8)));
enum ibmvfc_target_action {
--
2.27.0
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 23/23] fsldma: fix very broken 32-bit ppc ioread64 functionality
From: Sasha Levin @ 2020-08-31 15:30 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, linuxppc-dev, Linus Torvalds, dmaengine,
Guenter Roeck
In-Reply-To: <20200831153039.1024302-1-sashal@kernel.org>
From: Linus Torvalds <torvalds@linux-foundation.org>
[ Upstream commit 0a4c56c80f90797e9b9f8426c6aae4c0cf1c9785 ]
Commit ef91bb196b0d ("kernel.h: Silence sparse warning in
lower_32_bits") caused new warnings to show in the fsldma driver, but
that commit was not to blame: it only exposed some very incorrect code
that tried to take the low 32 bits of an address.
That made no sense for multiple reasons, the most notable one being that
that code was intentionally limited to only 32-bit ppc builds, so "only
low 32 bits of an address" was completely nonsensical. There were no
high bits to mask off to begin with.
But even more importantly fropm a correctness standpoint, turning the
address into an integer then caused the subsequent address arithmetic to
be completely wrong too, and the "+1" actually incremented the address
by one, rather than by four.
Which again was incorrect, since the code was reading two 32-bit values
and trying to make a 64-bit end result of it all. Surprisingly, the
iowrite64() did not suffer from the same odd and incorrect model.
This code has never worked, but it's questionable whether anybody cared:
of the two users that actually read the 64-bit value (by way of some C
preprocessor hackery and eventually the 'get_cdar()' inline function),
one of them explicitly ignored the value, and the other one might just
happen to work despite the incorrect value being read.
This patch at least makes it not fail the build any more, and makes the
logic superficially sane. Whether it makes any difference to the code
_working_ or not shall remain a mystery.
Compile-tested-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/dma/fsldma.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index 56f18ae992332..308bed0a560ac 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -205,10 +205,10 @@ struct fsldma_chan {
#else
static u64 fsl_ioread64(const u64 __iomem *addr)
{
- u32 fsl_addr = lower_32_bits(addr);
- u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32;
+ u32 val_lo = in_le32((u32 __iomem *)addr);
+ u32 val_hi = in_le32((u32 __iomem *)addr + 1);
- return fsl_addr_hi | in_le32((u32 *)fsl_addr);
+ return ((u64)val_hi << 32) + val_lo;
}
static void fsl_iowrite64(u64 val, u64 __iomem *addr)
@@ -219,10 +219,10 @@ static void fsl_iowrite64(u64 val, u64 __iomem *addr)
static u64 fsl_ioread64be(const u64 __iomem *addr)
{
- u32 fsl_addr = lower_32_bits(addr);
- u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32;
+ u32 val_hi = in_be32((u32 __iomem *)addr);
+ u32 val_lo = in_be32((u32 __iomem *)addr + 1);
- return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1));
+ return ((u64)val_hi << 32) + val_lo;
}
static void fsl_iowrite64be(u64 val, u64 __iomem *addr)
--
2.25.1
^ permalink raw reply related
* [PATCH AUTOSEL 5.8 42/42] fsldma: fix very broken 32-bit ppc ioread64 functionality
From: Sasha Levin @ 2020-08-31 15:29 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sasha Levin, linuxppc-dev, Linus Torvalds, dmaengine,
Guenter Roeck
In-Reply-To: <20200831152934.1023912-1-sashal@kernel.org>
From: Linus Torvalds <torvalds@linux-foundation.org>
[ Upstream commit 0a4c56c80f90797e9b9f8426c6aae4c0cf1c9785 ]
Commit ef91bb196b0d ("kernel.h: Silence sparse warning in
lower_32_bits") caused new warnings to show in the fsldma driver, but
that commit was not to blame: it only exposed some very incorrect code
that tried to take the low 32 bits of an address.
That made no sense for multiple reasons, the most notable one being that
that code was intentionally limited to only 32-bit ppc builds, so "only
low 32 bits of an address" was completely nonsensical. There were no
high bits to mask off to begin with.
But even more importantly fropm a correctness standpoint, turning the
address into an integer then caused the subsequent address arithmetic to
be completely wrong too, and the "+1" actually incremented the address
by one, rather than by four.
Which again was incorrect, since the code was reading two 32-bit values
and trying to make a 64-bit end result of it all. Surprisingly, the
iowrite64() did not suffer from the same odd and incorrect model.
This code has never worked, but it's questionable whether anybody cared:
of the two users that actually read the 64-bit value (by way of some C
preprocessor hackery and eventually the 'get_cdar()' inline function),
one of them explicitly ignored the value, and the other one might just
happen to work despite the incorrect value being read.
This patch at least makes it not fail the build any more, and makes the
logic superficially sane. Whether it makes any difference to the code
_working_ or not shall remain a mystery.
Compile-tested-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/dma/fsldma.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index 56f18ae992332..308bed0a560ac 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -205,10 +205,10 @@ struct fsldma_chan {
#else
static u64 fsl_ioread64(const u64 __iomem *addr)
{
- u32 fsl_addr = lower_32_bits(addr);
- u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32;
+ u32 val_lo = in_le32((u32 __iomem *)addr);
+ u32 val_hi = in_le32((u32 __iomem *)addr + 1);
- return fsl_addr_hi | in_le32((u32 *)fsl_addr);
+ return ((u64)val_hi << 32) + val_lo;
}
static void fsl_iowrite64(u64 val, u64 __iomem *addr)
@@ -219,10 +219,10 @@ static void fsl_iowrite64(u64 val, u64 __iomem *addr)
static u64 fsl_ioread64be(const u64 __iomem *addr)
{
- u32 fsl_addr = lower_32_bits(addr);
- u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32;
+ u32 val_hi = in_be32((u32 __iomem *)addr);
+ u32 val_lo = in_be32((u32 __iomem *)addr + 1);
- return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1));
+ return ((u64)val_hi << 32) + val_lo;
}
static void fsl_iowrite64be(u64 val, u64 __iomem *addr)
--
2.25.1
^ permalink raw reply related
* RE: [PATCH] fsldma: fsl_ioread64*() do not need lower_32_bits()
From: Leo Li @ 2020-08-31 14:25 UTC (permalink / raw)
To: Linus Torvalds, Guenter Roeck
Cc: Herbert Xu, Joerg Roedel, Linux Kernel Mailing List, Zhang Wei,
Vinod Koul, dma, Andrew Morton, linuxppc-dev, Dan Williams,
Luc Van Oostenryck
In-Reply-To: <CAHk-=wjDEiWF_DsCVFPFqNa+JCS5SkOygbqeq8_=ZNOrFt7-rg@mail.gmail.com>
> -----Original Message-----
> From: Linus Torvalds <torvalds@linux-foundation.org>
> Sent: Saturday, August 29, 2020 4:20 PM
> To: Guenter Roeck <linux@roeck-us.net>
> Cc: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>; Herbert Xu
> <herbert@gondor.apana.org.au>; Andrew Morton <akpm@linux-
> foundation.org>; Joerg Roedel <joerg.roedel@amd.com>; Leo Li
> <leoyang.li@nxp.com>; Zhang Wei <zw@zh-kernel.org>; Dan Williams
> <dan.j.williams@intel.com>; Vinod Koul <vkoul@kernel.org>; linuxppc-dev
> <linuxppc-dev@lists.ozlabs.org>; dma <dmaengine@vger.kernel.org>; Linux
> Kernel Mailing List <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH] fsldma: fsl_ioread64*() do not need lower_32_bits()
>
> On Sat, Aug 29, 2020 at 1:40 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > Except for
> >
> > CHECK: spaces preferred around that '+' (ctx:VxV)
> > #29: FILE: drivers/dma/fsldma.h:223:
> > + u32 val_lo = in_be32((u32 __iomem *)addr+1);
>
> Added spaces.
>
> > I don't see anything wrong with it either, so
> >
> > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> >
> > Since I didn't see the real problem with the original code, I'd take
> > that with a grain of salt, though.
>
> Well, honestly, the old code was so confused that just making it build is
> clearly already an improvement even if everything else were to be wrong.
>
> So I committed my "fix". If it turns out there's more wrong in there and
> somebody tests it, we can fix it again. But now it hopefully compiles, at least.
>
> My bet is that if that driver ever worked on ppc32, it will continue to work
> whatever we do to that function.
>
> I _think_ the old code happened to - completely by mistake - get the value
> right for the case of "little endian access, with dma_addr_t being 32-bit".
> Because then it would still read the upper bits wrong, but the cast to
> dma_addr_t would then throw those bits away. And the lower bits would be
> right.
>
> But for big-endian accesses or for ARCH_DMA_ADDR_T_64BIT it really looks
> like it always returned a completely incorrect value.
>
> And again - the driver may have worked even with that completely incorrect
> value, since the use of it seems to be very incidental.
>
> In either case ("it didn't work before" or "it worked because the value
> doesn't really matter"), I don't think I could possibly have made things worse.
>
> Famous last words.
Thanks for the patch.
Acked-by: Li Yang <leoyang.li@nxp.com>
We are having periodical auto regression tests covering ppc32 platforms. But looks like it missed this issue. I will ask the test team to investigate on why the test cases are not sufficient.
Regards,
Leo
^ permalink raw reply
* [Bug 205183] PPC64: Signal delivery fails with SIGSEGV if between about 1KB and 4KB bytes of stack remain
From: bugzilla-daemon @ 2020-08-31 13:16 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-205183-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=205183
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |CLOSED
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: KVM on POWER8 host lock up since 10d91611f426 ("powerpc/64s: Reimplement book3s idle code in C")
From: Ruediger Oertel @ 2020-08-31 13:13 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, linuxppc-dev,
Michal Suchánek
In-Reply-To: <87v9gz0y5y.fsf@mpe.ellerman.id.au>
[-- Attachment #1.1: Type: text/plain, Size: 1312 bytes --]
Am 31.08.20 um 14:58 schrieb Michael Ellerman:
[...]
>> The machines are running in OPAL/PowerNV mode, with "ppc64_cpu --smt=off".
>> The number of VMs varies across the machines:
>> obs-power8-01: 18 instances, "-smp 16,threads=8"
>> obs-power8-02: 20 instances, "-smp 8,threads=8"
>> obs-power8-03: 30 instances, "-smp 8,threads=8"
>> obs-power8-04: 20 instances, "-smp 8,threads=8"
>
> Can you send us the output of:
>
> # grep . /sys/module/kvm_hv/parameters/*
of course, the current values are:
/sys/module/kvm_hv/parameters/dynamic_mt_modes:6
/sys/module/kvm_hv/parameters/h_ipi_redirect:1
/sys/module/kvm_hv/parameters/indep_threads_mode:Y
/sys/module/kvm_hv/parameters/kvm_irq_bypass:1
/sys/module/kvm_hv/parameters/nested:Y
/sys/module/kvm_hv/parameters/one_vm_per_core:N
/sys/module/kvm_hv/parameters/target_smt_mode:0
(actually identical on all 5 above)
--
with kind regards (mit freundlichem Grinsen),
Ruediger Oertel (ro@suse.com,ro@suse.de,bugfinder@t-online.de)
--------Do-Not-Accept-Binary-Blobs.----Ever.----From-Anyone.------------
Key fingerprint = 17DC 6553 86A7 384B 53C5 CA5C 3CE4 F2E7 23F2 B417
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg,
Germany, (HRB 36809, AG Nürnberg), Geschäftsführer: Felix Imendörffer
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: KVM on POWER8 host lock up since 10d91611f426 ("powerpc/64s: Reimplement book3s idle code in C")
From: Michael Ellerman @ 2020-08-31 12:58 UTC (permalink / raw)
To: Ruediger Oertel, Nicholas Piggin, linuxppc-dev,
Michal Suchánek
In-Reply-To: <50693d7e-6c8b-4908-d003-25b99a867850@suse.de>
Ruediger Oertel <ro@suse.de> writes:
> Am 31.08.20 um 03:14 schrieb Nicholas Piggin:
>> Excerpts from Michal Suchánek's message of August 31, 2020 6:11 am:
>>> Hello,
>>>
>>> on POWER8 KVM hosts lock up since commit 10d91611f426 ("powerpc/64s:
>>> Reimplement book3s idle code in C").
>>>
>>> The symptom is host locking up completely after some hours of KVM
>>> workload with messages like
>>>
>>> 2020-08-30T10:51:31+00:00 obs-power8-01 kernel: KVM: couldn't grab cpu 47
>>> 2020-08-30T10:51:31+00:00 obs-power8-01 kernel: KVM: couldn't grab cpu 71
>>> 2020-08-30T10:51:31+00:00 obs-power8-01 kernel: KVM: couldn't grab cpu 47
>>> 2020-08-30T10:51:31+00:00 obs-power8-01 kernel: KVM: couldn't grab cpu 71
>>> 2020-08-30T10:51:31+00:00 obs-power8-01 kernel: KVM: couldn't grab cpu 47
>>>
>>> printed before the host locks up.
>>>
>>> The machines run sandboxed builds which is a mixed workload resulting in
>>> IO/single core/mutiple core load over time and there are periods of no
>>> activity and no VMS runnig as well. The VMs are shortlived so VM
>>> setup/terdown is somewhat excercised as well.
>>>
>>> POWER9 with the new guest entry fast path does not seem to be affected.
>>>
>>> Reverted the patch and the followup idle fixes on top of 5.2.14 and
>>> re-applied commit a3f3072db6ca ("powerpc/powernv/idle: Restore IAMR
>>> after idle") which gives same idle code as 5.1.16 and the kernel seems
>>> stable.
>>>
>>> Config is attached.
>>>
>>> I cannot easily revert this commit, especially if I want to use the same
>>> kernel on POWER8 and POWER9 - many of the POWER9 fixes are applicable
>>> only to the new idle code.
>>>
>>> Any idea what can be the problem?
>>
>> So hwthread_state is never getting back to to HWTHREAD_IN_IDLE on
>> those threads. I wonder what they are doing. POWER8 doesn't have a good
>> NMI IPI and I don't know if it supports pdbg dumping registers from the
>> BMC unfortunately. Do the messages always come in pairs of CPUs?
>>
>> I'm not sure where to start with reproducing, I'll have to try. How many
>> vCPUs in the guests? Do you have several guests running at once?
>
> Hello all,
>
> some details on the setup...
> these machines are buildservice workers, (build.opensuse.org) and all they
> do is spawn new VMs, run a package building job inside (rpmbuild, debbuild,...)
>
> The machines are running in OPAL/PowerNV mode, with "ppc64_cpu --smt=off".
> The number of VMs varies across the machines:
> obs-power8-01: 18 instances, "-smp 16,threads=8"
> obs-power8-02: 20 instances, "-smp 8,threads=8"
> obs-power8-03: 30 instances, "-smp 8,threads=8"
> obs-power8-04: 20 instances, "-smp 8,threads=8"
Can you send us the output of:
# grep . /sys/module/kvm_hv/parameters/*
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