| 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: <64388266-1707-ee20-c3ab-edb67ada68dc@amd.com>
Date: Tue, 27 Sep 2022 09:28:39 -0700
From: Lizhi Hou <lizhi.hou@....com>
To: Martin Tůma <tumic@...see.org>,
<vkoul@...nel.org>, <dmaengine@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <trix@...hat.com>
CC: <max.zhen@....com>, <sonal.santan@....com>, <larry.liu@....com>,
<brian.xu@....com>
Subject: Re: [PATCH V4 XDMA 2/2] dmaengine: xilinx: xdma: Add user logic
interrupt support
On 9/27/22 08:31, Martin Tůma wrote:
> On 22. 09. 22 20:38, Lizhi Hou wrote:
>> The Xilinx DMA/Bridge Subsystem for PCIe (XDMA) provides up to 16 user
>> interrupt wires to user logic that generate interrupts to the host.
>> This patch adds APIs to register/unregister interrupt handler for a
>> given
>> interrupt wire index.
>>
>> Signed-off-by: Lizhi Hou <lizhi.hou@....com>
>> Signed-off-by: Sonal Santan <sonal.santan@....com>
>> Signed-off-by: Max Zhen <max.zhen@....com>
>> Signed-off-by: Brian Xu <brian.xu@....com>
>> ---
>> MAINTAINERS | 1 +
>> drivers/dma/xilinx/xdma.c | 80 ++++++++++++++++++++++++++++++++++++
>> include/linux/dma/amd_xdma.h | 17 ++++++++
>> 3 files changed, 98 insertions(+)
>> create mode 100644 include/linux/dma/amd_xdma.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index c1be0b2e378a..019d84b2b086 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -21691,6 +21691,7 @@ L: dmaengine@...r.kernel.org
>> S: Supported
>> F: drivers/dma/xilinx/xdma-regs.h
>> F: drivers/dma/xilinx/xdma.c
>> +F: include/linux/dma/amd_xdma.h
>> F: include/linux/platform_data/amd_xdma.h
>> XILINX ZYNQMP DPDMA DRIVER
>> diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
>> index 58a57e03bef5..884942cd2a37 100644
>> --- a/drivers/dma/xilinx/xdma.c
>> +++ b/drivers/dma/xilinx/xdma.c
>> @@ -25,6 +25,7 @@
>> #include <linux/dmapool.h>
>> #include <linux/regmap.h>
>> #include <linux/dmaengine.h>
>> +#include <linux/dma/amd_xdma.h>
>> #include <linux/platform_device.h>
>> #include <linux/platform_data/amd_xdma.h>
>> #include <linux/dma-mapping.h>
>> @@ -736,6 +737,7 @@ static int xdma_set_vector_reg(struct xdma_device
>> *xdev, u32 vec_tbl_start,
>> static int xdma_irq_init(struct xdma_device *xdev)
>> {
>> u32 irq = xdev->irq_start;
>> + u32 user_irq_start;
>> int i, j, ret;
>> /* return failure if there are not enough IRQs */
>> @@ -786,6 +788,18 @@ static int xdma_irq_init(struct xdma_device *xdev)
>> goto failed;
>> }
>> + /* config user IRQ registers if needed */
>> + user_irq_start = xdev->h2c_chan_num + xdev->c2h_chan_num;
>> + if (xdev->irq_num > user_irq_start) {
>> + ret = xdma_set_vector_reg(xdev, XDMA_IRQ_USER_VEC_NUM,
>> + user_irq_start,
>> + xdev->irq_num - user_irq_start);
>> + if (ret) {
>> + xdma_err(xdev, "failed to set user vectors: %d", ret);
>> + goto failed;
>> + }
>> + }
>> +
>> /* enable interrupt */
>> ret = xdma_enable_intr(xdev);
>> if (ret) {
>> @@ -816,6 +830,72 @@ static bool xdma_filter_fn(struct dma_chan
>> *chan, void *param)
>> return true;
>> }
>> +/**
>> + * xdma_free_user_irq - Free user interrupt
>> + * @pdev: Pointer to the platform_device structure
>> + * @user_irq_index: User IRQ index
>> + * @arg: User interrupt cookie
>> + */
>> +void xdma_free_user_irq(struct platform_device *pdev, u32
>> user_irq_index,
>> + void *arg)
>> +{
>> + struct xdma_device *xdev = platform_get_drvdata(pdev);
>> + u32 user_irq;
>> +
>> + user_irq = xdev->h2c_chan_num + xdev->c2h_chan_num +
>> user_irq_index;
>> + if (user_irq >= xdev->irq_num) {
>> + xdma_err(xdev, "invalid user irq index");
>> + return;
>> + }
>> + user_irq += xdev->irq_start;
>> +
>> + xdma_write_reg(xdev, XDMA_IRQ_BASE, XDMA_IRQ_USER_INT_EN_W1C,
>> + (1 << user_irq_index));
>> +
>> + free_irq(user_irq, arg);
>> +}
>> +EXPORT_SYMBOL(xdma_free_user_irq);
>> +
>> +/**
>> + * xdma_request_user_irq - Register user interrupt
>> + * @pdev: Pointer to the platform_device structure
>> + * @user_irq_index: User IRQ index
>> + * @flags: Handling flags
>> + * @handler: User interrupt handler
>> + * @arg: User interrupt cookie
>> + */
>> +int xdma_request_user_irq(struct platform_device *pdev, u32
>> user_irq_index,
>> + irq_handler_t handler, void *arg, ulong flags)
>> +{
>> + struct xdma_device *xdev = platform_get_drvdata(pdev);
>> + u32 user_irq;
>> + int ret;
>> +
>> + user_irq = xdev->h2c_chan_num + xdev->c2h_chan_num +
>> user_irq_index;
>> + if (user_irq >= xdev->irq_num) {
>> + xdma_err(xdev, "invalid user irq index");
>> + return -EINVAL;
>> + }
>> + user_irq += xdev->irq_start;
>> +
>> + ret = request_irq(user_irq, handler, flags, "xdma-user", arg);
>> + if (ret) {
>> + xdma_err(xdev, "request irq failed, %d", ret);
>> + return ret;
>> + }
>> +
>> + ret = xdma_write_reg(xdev, XDMA_IRQ_BASE, XDMA_IRQ_USER_INT_EN_W1S,
>> + (1 << user_irq_index));
>> + if (ret) {
>> + xdma_err(xdev, "set user irq mask failed, %d", ret);
>> + free_irq(user_irq, arg);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(xdma_request_user_irq);
>> +
>> /**
>> * xdma_remove - Driver remove function
>> * @pdev: Pointer to the platform_device structure
>> diff --git a/include/linux/dma/amd_xdma.h b/include/linux/dma/amd_xdma.h
>> new file mode 100644
>> index 000000000000..b70486121ca6
>> --- /dev/null
>> +++ b/include/linux/dma/amd_xdma.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (C) 2022, Advanced Micro Devices, Inc.
>> + */
>> +
>> +#ifndef _DMAENGINE_AMD_XDMA_H
>> +#define _DMAENGINE_AMD_XDMA_H
>> +
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +
>> +int xdma_request_user_irq(struct platform_device *pdev, u32
>> user_irq_index,
>> + irq_handler_t handler, void *arg, ulong flags);
>> +void xdma_free_user_irq(struct platform_device *pdev, u32
>> user_irq_index,
>> + void *arg);
>> +
>> +#endif /* _DMAENGINE_AMD_XDMA_H */
>
> Hi,
> The user IRQ logic is unusable, if you have other IP cores with linux
> drivers (eg. spi or i2c) connected to that IRQs like we (and many
> other users as well) have on our PCIe card. You can't use the already
> existing
> xiic or xilinx-spi drivers with your xdma driver!
>
> You must split the "IRQ enable" and "IRQ request" operations for your
> driver to be usable with the other platform drivers in linux. Instead of
> "xdma_request_user_irq" there must be something like
> "xdma_enable_user_irq" that only enables the IRQ in the IP block, but
> does not "eat" the IRQ so you can pass it to the initialization of
> the platform driver of xiic or xilinx-spi.
>
> M.
Okay, I got the point. How about changing request/remove APIs to
enable/disable APIs as below
xdma_enable_user_irq(struct platform_device *pdev, u32
user_irq_index, u32 *irq)
user_irq_index: user logic interrupt wire index. (XDMA
driver determines how system IRQs are mapped to DMA channels and user
logic wires)
irq: IRQ number returned for registering interrupt handler
(request_irq()) or passing to existing platform driver.
xdma_disable_user_irq(struct platform_device *pdev, u32 user_irq_index)
Does this make sense to you?
Thanks,
Lizhi
Powered by blists - more mailing lists