Re: [patch 21/33] genirq/msi: Provide msi_domain_alloc_irq_at()

From: Reinette Chatre <reinette.chatre@intel.com>
To: Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>
Cc: <x86@kernel.org>, Joerg Roedel <joro@8bytes.org>,
	Will Deacon <will@kernel.org>, <linux-pci@vger.kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	"Marc Zyngier" <maz@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jason Gunthorpe <jgg@mellanox.com>,
	Dave Jiang <dave.jiang@intel.com>,
	"Alex Williamson" <alex.williamson@redhat.com>,
	Kevin Tian <kevin.tian@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Logan Gunthorpe <logang@deltatee.com>,
	Ashok Raj <ashok.raj@intel.com>, Jon Mason <jdmason@kudzu.us>,
	Allen Hubbe <allenbh@gmail.com>,
	"Ahmed S. Darwish" <darwi@linutronix.de>
Subject: Re: [patch 21/33] genirq/msi: Provide msi_domain_alloc_irq_at()
Date: Fri, 18 Nov 2022 10:18:30 -0800	[thread overview]
Message-ID: <d33e63f0-a5ba-38d3-1cfb-dd5ab8d249b8@intel.com> (raw)
In-Reply-To: <87wn7sjzeq.ffs@tglx>

Hi Thomas,

On 11/18/2022 3:05 AM, Thomas Gleixner wrote:
> On Fri, Nov 18 2022 at 10:15, Thomas Gleixner wrote:
>> On Fri, Nov 18 2022 at 01:58, Thomas Gleixner wrote:

...

> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index d4f26649a185..d243ad3e5489 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -141,7 +141,7 @@ static int msi_insert_desc(struct device *dev, struct msi_desc *desc,
>  		if (ret)
>  			goto fail;
>  
> -		desc->msi_index = index;
> +		desc->msi_index = index - baseidx;

Could msi_desc->msi_index be made bigger? The hardware I am testing
on claims to support more IMS entries than what the u16 can
accommodate.

>  		return 0;
>  	} else {
>  		if (index >= hwsize) {
> @@ -1476,9 +1476,10 @@ struct msi_map msi_domain_alloc_irq_at(struct device *dev, unsigned int domid, u
>  				       const struct irq_affinity_desc *affdesc,
>  				       union msi_dev_cookie *cookie)
>  {
> +	struct msi_ctrl ctrl = { .domid	= domid, .nirqs = 1, };
> +	struct msi_domain_info *info;
>  	struct irq_domain *domain;
>  	struct msi_map map = { };
> -	struct msi_desc *desc;

(*desc is still needed)

>  	int ret;
>  
>  	msi_lock_descs(dev);
> @@ -1503,12 +1504,16 @@ struct msi_map msi_domain_alloc_irq_at(struct device *dev, unsigned int domid, u
>  		goto unlock;
>  	}
>  
> -	map.index = desc->msi_index;
> -	ret = msi_domain_alloc_irqs_range_locked(dev, domid, map.index, map.index);
> -	if (ret)
> +	ctrl.first = ctrl.last = desc->msi_index;
> +	info = domain->host_data;
> +
> +	ret = __msi_domain_alloc_irqs(dev, domain, &ctrl);
> +	if (ret) {
>  		map.index = ret;
> -	else
> +		msi_domain_free_locked(dev, &ctrl);
> +	} else {
>  		map.virq = desc->irq;
> +	}
>  unlock:
>  	msi_unlock_descs(dev);
>  	return map;

Thank you very much. With the above snippet it is possible to
allocate an IMS IRQ. I am not yet able to use the IRQ and I am working
on more tracing to figure out why. In the mean time, I did
just try the pci_ims_alloc_irq()/pci_ims_free_irq() flow and
pci_ims_free_irq() triggered the WARN below:

remove_proc_entry: removing non-empty directory 'irq/220', leaking at least 'idxd-portal'
WARNING: CPU: XX PID: 4322 at fs/proc/generic.c:718 remove_proc_entry+0x184/0x190

[SNIP]

RIP: 0010:remove_proc_entry+0x184/0x190
Code: a5 af 48 8d 90 68 ff ff ff 48 85 c0 48 0f 45 c2 48 8b 95 88 00 00 00 4c 8b 80 b0 00 00 00 48 8b 92 b0 00 00 00 e8 2d 67 c6 00 <0f> 0b e9 4d ff ff ff e8 a0 c1 ce 00 0f 1f 44 00 00 41 57 41 56 41
RSP: 0018:ff223b51cf947c80 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ff1b39f680241300 RCX: 0000000000000000
RDX: 0000000000000001 RSI: ffffffffafa37b97 RDI: 00000000ffffffff
RBP: ff1b3a06666b8000 R08: ff1b3a15bdbfffe8 R09: 0000000000000003
R10: ff1b3a15bce00000 R11: ff1b3a15bd900000 R12: ff1b3a06666b8090
R13: 00000000000000dd R14: ff1b3a069237fb80 R15: 0000000000000001
FS:  00007fedd2dff000(0000) GS:ff1b3a15be940000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ff8e5bfc01c CR3: 000000110a138006 CR4: 0000000000771ee0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<TASK>
unregister_irq_proc+0xe3/0x110
free_desc+0x29/0x70
irq_free_descs+0x4b/0x80
msi_domain_free_locked.part.0+0x19b/0x1d0
msi_domain_free_irqs_range+0x67/0xb0
idxd_wq_free_irq+0x89/0x150 [idxd]
drv_disable_wq+0x5f/0x90 [idxd]
idxd_dmaengine_drv_remove+0xa3/0xc0 [idxd]
device_release_driver_internal+0x1aa/0x230
driver_detach+0x44/0x90
bus_remove_driver+0x58/0xe0
idxd_exit_module+0x18/0x3a [idxd]
__do_sys_delete_module.constprop.0+0x186/0x280
? fpregs_assert_state_consistent+0x22/0x50
? exit_to_user_mode_prepare+0x40/0x150
do_syscall_64+0x40/0x90
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7fedd2526c9b
Code: 73 01 c3 48 8b 0d 95 21 0f 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 65 21 0f 00 f7 d8 64 89 01 48
RSP: 002b:00007ffca85a47d8 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
RAX: ffffffffffffffda RBX: 000055af0e1a3700 RCX: 00007fedd2526c9b
RDX: 000000000000000a RSI: 0000000000000800 RDI: 000055af0e1a3768
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 00007fedd25beac0 R11: 0000000000000206 R12: 00007ffca85a4a30
R13: 000055af0e1a32a0 R14: 00007ffca85a58e5 R15: 000055af0e1a3700
</TASK>

Reinette