| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <038659b4-a148-4663-870e-898f33406a08@amd.com>
Date: Wed, 20 Dec 2023 09:42:44 -0800
From: Ben Levinsky <ben.levinsky@....com>
To: Michal Simek <michal.simek@....com>, jassisinghbrar@...il.com,
linux-kernel@...r.kernel.org
Cc: shubhrajyoti.datta@....com, tanmay.shah@....com,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 3/3] mailbox: zynqmp: Enable Bufferless IPI usage on
Versal-based SOC's
On 12/20/23 5:29 AM, Michal Simek wrote:
>
>
> On 12/14/23 22:13, Ben Levinsky wrote:
>> On Xilinx-AMD Versal and Versal-NET, there exist both
>> inter-processor-interrupts with corresponding message buffers and without
>> such buffers.
>>
>> Add a routine that, if the corresponding DT compatible
>> string "xlnx,versal-ipi-mailbox" is used then a Versal-based SOC
>> can use a mailbox Device Tree entry where both host and remote
>> can use either of the buffered or bufferless interrupts.
>>
>> Signed-off-by: Ben Levinsky <ben.levinsky@....com>
>> ---
>> Note that the linked patch provides corresponding bindings.
>> Depends on: https://lore.kernel.org/all/20231214054224.957336-3-tanmay.shah@amd.com/T/
>> ---
>> drivers/mailbox/zynqmp-ipi-mailbox.c | 146 +++++++++++++++++++++++++--
>> 1 file changed, 139 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c
>> index edefb80a6e47..316d9406064e 100644
>> --- a/drivers/mailbox/zynqmp-ipi-mailbox.c
>> +++ b/drivers/mailbox/zynqmp-ipi-mailbox.c
>> @@ -52,6 +52,13 @@
>> #define IPI_MB_CHNL_TX 0 /* IPI mailbox TX channel */
>> #define IPI_MB_CHNL_RX 1 /* IPI mailbox RX channel */
>> +/* IPI Message Buffer Information */
>> +#define RESP_OFFSET 0x20U
>> +#define DEST_OFFSET 0x40U
>> +#define IPI_BUF_SIZE 0x20U
>> +#define DST_BIT_POS 9U
>> +#define SRC_BITMASK GENMASK(11, 8)
>> +
>> /**
>> * struct zynqmp_ipi_mchan - Description of a Xilinx ZynqMP IPI mailbox channel
>> * @is_opened: indicate if the IPI channel is opened
>> @@ -170,9 +177,11 @@ static irqreturn_t zynqmp_ipi_interrupt(int irq, void *data)
>> if (ret > 0 && ret & IPI_MB_STATUS_RECV_PENDING) {
>> if (mchan->is_opened) {
>> msg = mchan->rx_buf;
>> - msg->len = mchan->req_buf_size;
>> - memcpy_fromio(msg->data, mchan->req_buf,
>> - msg->len);
>> + if (msg) {
>> + msg->len = mchan->req_buf_size;
>> + memcpy_fromio(msg->data, mchan->req_buf,
>> + msg->len);
>> + }
>> mbox_chan_received_data(chan, (void *)msg);
>> status = IRQ_HANDLED;
>> }
>> @@ -282,26 +291,26 @@ static int zynqmp_ipi_send_data(struct mbox_chan *chan, void *data)
>> if (mchan->chan_type == IPI_MB_CHNL_TX) {
>> /* Send request message */
>> - if (msg && msg->len > mchan->req_buf_size) {
>> + if (msg && msg->len > mchan->req_buf_size && mchan->req_buf) {
>> dev_err(dev, "channel %d message length %u > max %lu\n",
>> mchan->chan_type, (unsigned int)msg->len,
>> mchan->req_buf_size);
>> return -EINVAL;
>> }
>> - if (msg && msg->len)
>> + if (msg && msg->len && mchan->req_buf)
>> memcpy_toio(mchan->req_buf, msg->data, msg->len);
>> /* Kick IPI mailbox to send message */
>> arg0 = SMC_IPI_MAILBOX_NOTIFY;
>> zynqmp_ipi_fw_call(ipi_mbox, arg0, 0, &res);
>> } else {
>> /* Send response message */
>> - if (msg && msg->len > mchan->resp_buf_size) {
>> + if (msg && msg->len > mchan->resp_buf_size && mchan->resp_buf) {
>> dev_err(dev, "channel %d message length %u > max %lu\n",
>> mchan->chan_type, (unsigned int)msg->len,
>> mchan->resp_buf_size);
>> return -EINVAL;
>> }
>> - if (msg && msg->len)
>> + if (msg && msg->len && mchan->resp_buf)
>> memcpy_toio(mchan->resp_buf, msg->data, msg->len);
>> arg0 = SMC_IPI_MAILBOX_ACK;
>> zynqmp_ipi_fw_call(ipi_mbox, arg0, IPI_SMC_ACK_EIRQ_MASK,
>> @@ -640,6 +649,126 @@ static int zynqmp_ipi_setup(struct zynqmp_ipi_mbox *ipi_mbox,
>> return 0;
>> }
>> +/**
>> + * versal_ipi_setup - Set up IPIs to support mixed usage of
>> + * Buffered and Bufferless IPIs.
>> + *
>> + * @ipi_mbox: pointer to IPI mailbox private data structure
>> + * @node: IPI mailbox device node
>> + *
>> + * Return: 0 for success, negative value for failure
>> + */
>> +static int versal_ipi_setup(struct zynqmp_ipi_mbox *ipi_mbox,
>> + struct device_node *node)
>> +{
>> + struct zynqmp_ipi_mchan *tx_mchan, *rx_mchan;
>> + struct resource host_res, remote_res;
>> + struct device_node *parent_node;
>> + int host_idx, remote_idx;
>> + struct device *mdev, *dev;
>> +
>> + tx_mchan = &ipi_mbox->mchans[IPI_MB_CHNL_TX];
>> + rx_mchan = &ipi_mbox->mchans[IPI_MB_CHNL_RX];
>> + parent_node = of_get_parent(node);
>> + dev = ipi_mbox->pdata->dev;
>> + mdev = &ipi_mbox->dev;
>> +
>> + host_idx = zynqmp_ipi_mbox_get_buf_res(parent_node, "msg", &host_res);
>> + remote_idx = zynqmp_ipi_mbox_get_buf_res(node, "msg", &remote_res);
>> +
>> + /*
>> + * Only set up buffers if both sides claim to have msg buffers.
>> + * This is because each buffered IPI's corresponding msg buffers
>> + * are reserved for use by other buffered IPI's.
>> + */
>> + if (!host_idx && !remote_idx) {
>> + u32 host_src, host_dst, remote_src, remote_dst;
>> + u32 buff_sz;
>> +
>> + buff_sz = resource_size(&host_res);
>> +
>> + host_src = host_res.start & SRC_BITMASK;
>> + remote_src = remote_res.start & SRC_BITMASK;
>> +
>> + host_dst = (host_src >> DST_BIT_POS) * DEST_OFFSET;
>> + remote_dst = (remote_src >> DST_BIT_POS) * DEST_OFFSET;
>> +
>> + /* Validate that IPI IDs is within IPI Message buffer space. */
>> + if (host_dst >= buff_sz || remote_dst >= buff_sz) {
>> + dev_err(mdev,
>> + "Invalid IPI Message buffer values: %x %x\n",
>> + host_dst, remote_dst);
>> + return -EINVAL;
>> + }
>> +
>> + tx_mchan->req_buf = devm_ioremap(mdev,
>> + host_res.start | remote_dst,
>> + IPI_BUF_SIZE);
>> + if (!tx_mchan->req_buf) {
>> + dev_err(mdev, "Unable to map IPI buffer I/O memory\n");
>> + return -ENOMEM;
>> + }
>> +
>> + tx_mchan->resp_buf = devm_ioremap(mdev,
>> + (remote_res.start | host_dst) +
>> + RESP_OFFSET, IPI_BUF_SIZE);
>> + if (!tx_mchan->resp_buf) {
>> + dev_err(mdev, "Unable to map IPI buffer I/O memory\n");
>> + return -ENOMEM;
>> + }
>> +
>> + rx_mchan->req_buf = devm_ioremap(mdev,
>> + remote_res.start | host_dst,
>> + IPI_BUF_SIZE);
>> + if (!rx_mchan->req_buf) {
>> + dev_err(mdev, "Unable to map IPI buffer I/O memory\n");
>> + return -ENOMEM;
>> + }
>> +
>> + rx_mchan->resp_buf = devm_ioremap(mdev,
>> + (host_res.start | remote_dst) +
>> + RESP_OFFSET, IPI_BUF_SIZE);
>> + if (!rx_mchan->resp_buf) {
>> + dev_err(mdev, "Unable to map IPI buffer I/O memory\n");
>> + return -ENOMEM;
>> + }
>> +
>> + tx_mchan->resp_buf_size = IPI_BUF_SIZE;
>> + tx_mchan->req_buf_size = IPI_BUF_SIZE;
>> + tx_mchan->rx_buf = devm_kzalloc(mdev, IPI_BUF_SIZE +
>> + sizeof(struct zynqmp_ipi_message),
>> + GFP_KERNEL);
>> + if (!tx_mchan->rx_buf)
>> + return -ENOMEM;
>> +
>> + rx_mchan->resp_buf_size = IPI_BUF_SIZE;
>> + rx_mchan->req_buf_size = IPI_BUF_SIZE;
>> + rx_mchan->rx_buf = devm_kzalloc(mdev, IPI_BUF_SIZE +
>> + sizeof(struct zynqmp_ipi_message),
>> + GFP_KERNEL);
>> + if (!rx_mchan->rx_buf)
>> + return -ENOMEM;
>> + } else {
>> + /*
>> + * If here, then set up Bufferless IPI Channel because
>> + * one or both of the IPI's is bufferless.
>> + */
>> + tx_mchan->req_buf = NULL;
>> + tx_mchan->resp_buf = NULL;
>> + tx_mchan->rx_buf = NULL;
>> + tx_mchan->resp_buf_size = 0;
>> + tx_mchan->req_buf_size = 0;
>> +
>> + rx_mchan->req_buf = NULL;
>> + rx_mchan->resp_buf = NULL;
>> + rx_mchan->rx_buf = NULL;
>> + rx_mchan->resp_buf_size = 0;
>> + rx_mchan->req_buf_size = 0;
>
> Just curious if this is really needed. If none fills that values aren't they actually already 0/NULL because that location is cleared by kzalloc.
>
> Thanks,
> Michal
Confirmed. I removed the whole else condition and it still worked.
Will update in next rev after I receive any pending feedback from Jassi
Powered by blists - more mailing lists