Re: [PATCH v2 3/3] misc: pci_endpoint_test: Fix irq_type to convey the correct type

Niklas Cassel
2025-01-31T13:13:10Z

[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]

On Fri, Jan 31, 2025 at 01:20:38PM +0100, Niklas Cassel wrote:
> On Fri, Jan 31, 2025 at 01:10:54PM +0100, Niklas Cassel wrote:
> > >
> > > If SET_IRQTYPE is AUTO, how will test->irq_type be set?
> >
> > I was thinking something like this:
> >
> > pci_endpoint_test_set_irq()
> > {
> > u32 caps = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_CAPS);
> >
> > ...
> >
> > if (req_irq_type == IRQ_TYPE_AUTO) {
> > if (caps & MSI_CAPABLE)
> > test->irq_type = IRQ_TYPE_MSI;
> > else if (caps & MSIX_CAPABLE)
> > test->irq_type = IRQ_TYPE_MSIX;
> > else
> > test->irq_type = IRQ_TYPE_INTX;
> >
> > }
> >
> > ...
> > }
>
>
> Or even simpler (since it requires less changes to
> pci_endpoint_test_set_irq()):
>
> if (req_irq_type == IRQ_TYPE_AUTO) {
> if (caps & MSI_CAPABLE)
> req_irq_type = IRQ_TYPE_MSI;
> else if (caps & MSIX_CAPABLE)
> req_irq_type = IRQ_TYPE_MSIX;
> else
> req_irq_type = IRQ_TYPE_INTX;
>
> }
>
> ...
>
> /* Sets test->irq_type = req_irq_type; on success */
> pci_endpoint_test_alloc_irq_vectors();

See attached patch.

Mani, removing the global irq_type would go on top of this.

Kind regards,
Niklas

[-- Attachment #2: auto_irq.patch --]
[-- Type: text/plain, Size: 5622 bytes --]

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index d5ac71a49386..5e42930124e2 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -32,6 +32,7 @@
#define IRQ_TYPE_INTX 0
#define IRQ_TYPE_MSI 1
#define IRQ_TYPE_MSIX 2
+#define IRQ_TYPE_AUTO 3

#define PCI_ENDPOINT_TEST_MAGIC 0x0

@@ -71,6 +72,8 @@

#define PCI_ENDPOINT_TEST_CAPS 0x30
#define CAP_UNALIGNED_ACCESS BIT(0)
+#define CAP_MSI BIT(1)
+#define CAP_MSIX BIT(2)

#define PCI_DEVICE_ID_TI_AM654 0xb00c
#define PCI_DEVICE_ID_TI_J7200 0xb00f
@@ -126,6 +129,7 @@ struct pci_endpoint_test {
struct miscdevice miscdev;
enum pci_barno test_reg_bar;
size_t alignment;
+ u32 ep_caps;
const char *name;
};

@@ -805,11 +809,20 @@ static int pci_endpoint_test_set_irq(struct pci_endpoint_test *test,
struct device *dev = &pdev->dev;
int ret;

- if (req_irq_type < IRQ_TYPE_INTX || req_irq_type > IRQ_TYPE_MSIX) {
+ if (req_irq_type < IRQ_TYPE_INTX || req_irq_type > IRQ_TYPE_AUTO) {
dev_err(dev, "Invalid IRQ type option\n");
return -EINVAL;
}

+ if (req_irq_type == IRQ_TYPE_AUTO) {
+ if (test->ep_caps & CAP_MSI)
+ req_irq_type = IRQ_TYPE_MSI;
+ else if (test->ep_caps & CAP_MSIX)
+ req_irq_type = IRQ_TYPE_MSIX;
+ else
+ req_irq_type = IRQ_TYPE_INTX;
+ }
+
if (test->irq_type == req_irq_type)
return 0;

@@ -895,13 +908,12 @@ static void pci_endpoint_test_get_capabilities(struct pci_endpoint_test *test)
{
struct pci_dev *pdev = test->pdev;
struct device *dev = &pdev->dev;
- u32 caps;

- caps = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_CAPS);
- dev_dbg(dev, "PCI_ENDPOINT_TEST_CAPS: %#x\n", caps);
+ test->ep_caps = pci_endpoint_test_readl(test, PCI_ENDPOINT_TEST_CAPS);
+ dev_dbg(dev, "PCI_ENDPOINT_TEST_CAPS: %#x\n", test->ep_caps);

/* CAP_UNALIGNED_ACCESS is set if the EP can do unaligned access */
- if (caps & CAP_UNALIGNED_ACCESS)
+ if (test->ep_caps & CAP_UNALIGNED_ACCESS)
test->alignment = 0;
}

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index b94e205ae10b..8917f7c6c741 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -45,6 +45,8 @@
#define TIMER_RESOLUTION 1

#define CAP_UNALIGNED_ACCESS BIT(0)
+#define CAP_MSI BIT(1)
+#define CAP_MSIX BIT(2)

static struct workqueue_struct *kpcitest_workqueue;

@@ -753,6 +755,12 @@ static void pci_epf_test_set_capabilities(struct pci_epf *epf)
if (epc->ops->align_addr)
caps |= CAP_UNALIGNED_ACCESS;

+ if (epf_test->epc_features->msi_capable)
+ caps |= CAP_MSI;
+
+ if (epf_test->epc_features->msix_capable)
+ caps |= CAP_MSIX;
+
reg->caps = cpu_to_le32(caps);
}

diff --git a/include/uapi/linux/pcitest.h b/include/uapi/linux/pcitest.h
index acd261f49866..1edbd4357470 100644
--- a/include/uapi/linux/pcitest.h
+++ b/include/uapi/linux/pcitest.h
@@ -23,6 +23,11 @@
#define PCITEST_BARS _IO('P', 0xa)
#define PCITEST_CLEAR_IRQ _IO('P', 0x10)

+#define PCITEST_IRQ_TYPE_INTX 0
+#define PCITEST_IRQ_TYPE_MSI 1
+#define PCITEST_IRQ_TYPE_MSIX 2
+#define PCITEST_IRQ_TYPE_AUTO 3
+
#define PCITEST_FLAGS_USE_DMA 0x00000001

struct pci_endpoint_test_xfer_param {
diff --git a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
index c267b822c108..c820a67e6437 100644
--- a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
+++ b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
@@ -97,7 +97,7 @@ TEST_F(pci_ep_basic, LEGACY_IRQ_TEST)
{
int ret;

- pci_ep_ioctl(PCITEST_SET_IRQTYPE, 0);
+ pci_ep_ioctl(PCITEST_SET_IRQTYPE, PCITEST_IRQ_TYPE_INTX);
ASSERT_EQ(0, ret) TH_LOG("Can't set Legacy IRQ type");

pci_ep_ioctl(PCITEST_LEGACY_IRQ, 0);
@@ -108,7 +108,7 @@ TEST_F(pci_ep_basic, MSI_TEST)
{
int ret, i;

- pci_ep_ioctl(PCITEST_SET_IRQTYPE, 1);
+ pci_ep_ioctl(PCITEST_SET_IRQTYPE, PCITEST_IRQ_TYPE_MSI);
ASSERT_EQ(0, ret) TH_LOG("Can't set MSI IRQ type");

for (i = 1; i <= 32; i++) {
@@ -121,7 +121,7 @@ TEST_F(pci_ep_basic, MSIX_TEST)
{
int ret, i;

- pci_ep_ioctl(PCITEST_SET_IRQTYPE, 2);
+ pci_ep_ioctl(PCITEST_SET_IRQTYPE, PCITEST_IRQ_TYPE_MSIX);
ASSERT_EQ(0, ret) TH_LOG("Can't set MSI-X IRQ type");

for (i = 1; i <= 2048; i++) {
@@ -170,8 +170,8 @@ TEST_F(pci_ep_data_transfer, READ_TEST)
if (variant->use_dma)
param.flags = PCITEST_FLAGS_USE_DMA;

- pci_ep_ioctl(PCITEST_SET_IRQTYPE, 1);
- ASSERT_EQ(0, ret) TH_LOG("Can't set MSI IRQ type");
+ pci_ep_ioctl(PCITEST_SET_IRQTYPE, PCITEST_IRQ_TYPE_AUTO);
+ ASSERT_EQ(0, ret) TH_LOG("Can't set AUTO IRQ type");

for (i = 0; i < ARRAY_SIZE(test_size); i++) {
param.size = test_size[i];
@@ -189,8 +189,8 @@ TEST_F(pci_ep_data_transfer, WRITE_TEST)
if (variant->use_dma)
param.flags = PCITEST_FLAGS_USE_DMA;

- pci_ep_ioctl(PCITEST_SET_IRQTYPE, 1);
- ASSERT_EQ(0, ret) TH_LOG("Can't set MSI IRQ type");
+ pci_ep_ioctl(PCITEST_SET_IRQTYPE, PCITEST_IRQ_TYPE_AUTO);
+ ASSERT_EQ(0, ret) TH_LOG("Can't set AUTO IRQ type");

for (i = 0; i < ARRAY_SIZE(test_size); i++) {
param.size = test_size[i];
@@ -208,8 +208,8 @@ TEST_F(pci_ep_data_transfer, COPY_TEST)
if (variant->use_dma)
param.flags = PCITEST_FLAGS_USE_DMA;

- pci_ep_ioctl(PCITEST_SET_IRQTYPE, 1);
- ASSERT_EQ(0, ret) TH_LOG("Can't set MSI IRQ type");
+ pci_ep_ioctl(PCITEST_SET_IRQTYPE, PCITEST_IRQ_TYPE_AUTO);
+ ASSERT_EQ(0, ret) TH_LOG("Can't set AUTO IRQ type");

for (i = 0; i < ARRAY_SIZE(test_size); i++) {
param.size = test_size[i];