linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* PCI/kernel msi code vs GIC ITS driver conflict?
@ 2019-09-03 14:09 John Garry
  2019-09-03 16:16 ` Marc Zyngier
  0 siblings, 1 reply; 13+ messages in thread
From: John Garry @ 2019-09-03 14:09 UTC (permalink / raw)
  To: Marc Zyngier, Thomas Gleixner, Bjorn Helgaas
  Cc: Linux PCI, Linuxarm, luojiaxing, linux-kernel

Hi Marc, Bjorn, Thomas,

We've come across a conflict with the kernel/pci msi code and GIC ITS 
driver on our arm64 system, whereby we can't unbind and re-bind a PCI 
device driver under special conditions. I'll explain...

Our PCI device support 32 MSIs. The driver attempts to allocate msi 
vectors with min msi=17, max msi = 32, and affd.pre vectors = 16. For 
our test we make nr_cpus = 1 (just anything less than 16).

We find that the pci/kernel msi code gives us 17 vectors, but the GIC 
ITS code reserves 32 lpi maps in its_irq_domain_alloc(). The problem 
then occurs when unbinding the driver in its_irq_domain_free() call, 
where we only clear bits for 17 vectors. So if we unbind the driver and 
then attempt to bind again, it fails.

Where the fault lies, I can't say. Maybe the kernel msi code should 
always give power of 2 vectors - as I understand, the PCI spec mandates 
this. Or maybe the GIC ITS driver has a problem in the free path, as 
above. Or maybe the PCI driver should not be allowed to request !power 
of 2 min/max vectors.

Opinion?

Thanks in advance,
John


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
  2019-09-03 14:09 PCI/kernel msi code vs GIC ITS driver conflict? John Garry
@ 2019-09-03 16:16 ` Marc Zyngier
  2019-09-04  8:56   ` John Garry
  0 siblings, 1 reply; 13+ messages in thread
From: Marc Zyngier @ 2019-09-03 16:16 UTC (permalink / raw)
  To: John Garry, Thomas Gleixner, Bjorn Helgaas
  Cc: Linux PCI, Linuxarm, luojiaxing, linux-kernel

Hi John,

On 03/09/2019 15:09, John Garry wrote:
> Hi Marc, Bjorn, Thomas,
> 
> We've come across a conflict with the kernel/pci msi code and GIC ITS 
> driver on our arm64 system, whereby we can't unbind and re-bind a PCI 
> device driver under special conditions. I'll explain...
> 
> Our PCI device support 32 MSIs. The driver attempts to allocate msi 
> vectors with min msi=17, max msi = 32, and affd.pre vectors = 16. For 
> our test we make nr_cpus = 1 (just anything less than 16).

Just to confirm: this PCI device is requiring Multi-MSI, right? As
opposed to MSI-X?

> We find that the pci/kernel msi code gives us 17 vectors, but the GIC 
> ITS code reserves 32 lpi maps in its_irq_domain_alloc(). The problem 
> then occurs when unbinding the driver in its_irq_domain_free() call, 
> where we only clear bits for 17 vectors. So if we unbind the driver and 
> then attempt to bind again, it fails.

Is this device, by any chance, sharing its requested-id with another
device? By being behind a bridge of some sort? There is some code to
deal with it, but I'm not sure it has ever been verified in anger...

> Where the fault lies, I can't say. Maybe the kernel msi code should 
> always give power of 2 vectors - as I understand, the PCI spec mandates 
> this. Or maybe the GIC ITS driver has a problem in the free path, as 
> above. Or maybe the PCI driver should not be allowed to request !power 
> of 2 min/max vectors.
> 
> Opinion?

My hunch is that it is an ITS driver bug: the PCI layer is allowed to
give any number of MSIs to an endpoint driver, as long as they match the
requirements of the allocation for Multi-MSI. That's the responsibility
of the ITS driver. If unbind/bind fails, it means that somehow we've
missed the freeing of the LPIs, which isn't good.

Is the device common enough that I can try and reproduce the issue? If
there's a Linux driver somewhere, I can always hack something in
emulation and find out...

Thanks,

	M.
-- 
Jazz is not dead, it just smells funny...

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
  2019-09-03 16:16 ` Marc Zyngier
@ 2019-09-04  8:56   ` John Garry
  2019-09-04 10:25     ` Andrew Murray
  0 siblings, 1 reply; 13+ messages in thread
From: John Garry @ 2019-09-04  8:56 UTC (permalink / raw)
  To: Marc Zyngier, Thomas Gleixner, Bjorn Helgaas
  Cc: Linux PCI, Linuxarm, luojiaxing, linux-kernel

On 03/09/2019 17:16, Marc Zyngier wrote:
> Hi John,
>
> On 03/09/2019 15:09, John Garry wrote:
>> Hi Marc, Bjorn, Thomas,

Hi Marc,

>>
>> We've come across a conflict with the kernel/pci msi code and GIC ITS
>> driver on our arm64 system, whereby we can't unbind and re-bind a PCI
>> device driver under special conditions. I'll explain...
>>
>> Our PCI device support 32 MSIs. The driver attempts to allocate msi
>> vectors with min msi=17, max msi = 32, and affd.pre vectors = 16. For
>> our test we make nr_cpus = 1 (just anything less than 16).
>
> Just to confirm: this PCI device is requiring Multi-MSI, right? As
> opposed to MSI-X?

Right, Multi-MSI.

>
>> We find that the pci/kernel msi code gives us 17 vectors, but the GIC
>> ITS code reserves 32 lpi maps in its_irq_domain_alloc(). The problem
>> then occurs when unbinding the driver in its_irq_domain_free() call,
>> where we only clear bits for 17 vectors. So if we unbind the driver and
>> then attempt to bind again, it fails.
>
> Is this device, by any chance, sharing its requested-id with another
> device? By being behind a bridge of some sort?There is some code to
> deal with it, but I'm not sure it has ever been verified in anger...

It's a RC iEP and there should be no requested-id sharing:

root@ubuntu:/home/john#  lspci -s 74:02.0 -v
74:02.0 Serial Attached SCSI controller: Huawei Technologies Co., Ltd. 
HiSilicon SAS 3.0 HBA (rev 20)
Flags: bus master, fast devsel, latency 0, IRQ 23, NUMA node 0
Memory at a2000000 (32-bit, non-prefetchable) [size=32K]
Capabilities: [40] Express Root Complex Integrated Endpoint, MSI 00
Capabilities: [80] MSI: Enable+ Count=32/32 Maskable+ 64bit+
Capabilities: [b0] Power Management version 3
Kernel driver in use: hisi_sas_v3_hw

>
>> Where the fault lies, I can't say. Maybe the kernel msi code should
>> always give power of 2 vectors - as I understand, the PCI spec mandates
>> this. Or maybe the GIC ITS driver has a problem in the free path, as
>> above. Or maybe the PCI driver should not be allowed to request !power
>> of 2 min/max vectors.
>>
>> Opinion?
>
> My hunch is that it is an ITS driver bug: the PCI layer is allowed to
> give any number of MSIs to an endpoint driver, as long as they match the
> requirements of the allocation for Multi-MSI.

I would tend to say that, but isn't the requirement to allocate power of 
2 msi vectors, which doesn't seem to be enforced in the kernel msi layer?

  That's the responsibility
> of the ITS driver. If unbind/bind fails, it means that somehow we've
> missed the freeing of the LPIs, which isn't good.
>
> Is the device common enough that I can try and reproduce the issue?

No, it's integrated into the hi1620 SoC found in the D06 dev board only, 
but I don't think that there is anything special about this HW.

If
> there's a Linux driver somewhere, I can always hack something in
> emulation and find out...

Ok, the interrupt allocation for this particular driver in this test is 
in 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c#n2393

Cheers,
John

>
> Thanks,
>
> 	M.
>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
  2019-09-04  8:56   ` John Garry
@ 2019-09-04 10:25     ` Andrew Murray
  2019-09-05  8:38       ` Marc Zyngier
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Murray @ 2019-09-04 10:25 UTC (permalink / raw)
  To: John Garry
  Cc: Marc Zyngier, Thomas Gleixner, Bjorn Helgaas, Linux PCI,
	Linuxarm, luojiaxing, linux-kernel

On Wed, Sep 04, 2019 at 09:56:51AM +0100, John Garry wrote:
> On 03/09/2019 17:16, Marc Zyngier wrote:
> > Hi John,
> > 
> > On 03/09/2019 15:09, John Garry wrote:
> > > Hi Marc, Bjorn, Thomas,
> 
> Hi Marc,
> 
> > > 
> > > We've come across a conflict with the kernel/pci msi code and GIC ITS
> > > driver on our arm64 system, whereby we can't unbind and re-bind a PCI
> > > device driver under special conditions. I'll explain...
> > > 
> > > Our PCI device support 32 MSIs. The driver attempts to allocate msi
> > > vectors with min msi=17, max msi = 32, and affd.pre vectors = 16. For
> > > our test we make nr_cpus = 1 (just anything less than 16).
> > 
> > Just to confirm: this PCI device is requiring Multi-MSI, right? As
> > opposed to MSI-X?
> 
> Right, Multi-MSI.
> 
> > 
> > > We find that the pci/kernel msi code gives us 17 vectors, but the GIC
> > > ITS code reserves 32 lpi maps in its_irq_domain_alloc(). The problem
> > > then occurs when unbinding the driver in its_irq_domain_free() call,
> > > where we only clear bits for 17 vectors. So if we unbind the driver and
> > > then attempt to bind again, it fails.
> > 
> > Is this device, by any chance, sharing its requested-id with another
> > device? By being behind a bridge of some sort?There is some code to
> > deal with it, but I'm not sure it has ever been verified in anger...
> 
> It's a RC iEP and there should be no requested-id sharing:
> 
> root@ubuntu:/home/john#  lspci -s 74:02.0 -v
> 74:02.0 Serial Attached SCSI controller: Huawei Technologies Co., Ltd.
> HiSilicon SAS 3.0 HBA (rev 20)
> Flags: bus master, fast devsel, latency 0, IRQ 23, NUMA node 0
> Memory at a2000000 (32-bit, non-prefetchable) [size=32K]
> Capabilities: [40] Express Root Complex Integrated Endpoint, MSI 00
> Capabilities: [80] MSI: Enable+ Count=32/32 Maskable+ 64bit+
> Capabilities: [b0] Power Management version 3
> Kernel driver in use: hisi_sas_v3_hw
> 
> > 
> > > Where the fault lies, I can't say. Maybe the kernel msi code should
> > > always give power of 2 vectors - as I understand, the PCI spec mandates
> > > this. Or maybe the GIC ITS driver has a problem in the free path, as
> > > above. Or maybe the PCI driver should not be allowed to request !power
> > > of 2 min/max vectors.
> > > 
> > > Opinion?
> > 
> > My hunch is that it is an ITS driver bug: the PCI layer is allowed to
> > give any number of MSIs to an endpoint driver, as long as they match the
> > requirements of the allocation for Multi-MSI.
> 
> I would tend to say that, but isn't the requirement to allocate power of 2
> msi vectors, which doesn't seem to be enforced in the kernel msi layer?

For a PCI device that supports MSI but not MSI-X - my understanding is that
pci_alloc_irq_vectors_affinity and pci_alloc_irq_vectors will request *from
the device* a power of 2 msi vectors between the min and max given by the
driver - msi_setup_entry rounds up to nearest power of 2.

However this doesn't guarantee that pci_alloc_irq_vectors will return a
power of 2. For example if you set maxvec to 17, then it will request
32 from the device and pci_alloc_irq_vectors will return 17 (i.e. it satisfies
your request by over allocating, but still gives you what you asked for).

I'm not yet familiar with ITS, however if it is reserving 32 yet you only
clear 17, then there is mismatch between the number actually reserved from
the hardware, and the value returned from pci_alloc_irq_vectors.

(It looks like its_alloc_device_irq rounds up to the nearest power of 2).

Thanks,

Andrew Murray

> 
>  That's the responsibility
> > of the ITS driver. If unbind/bind fails, it means that somehow we've
> > missed the freeing of the LPIs, which isn't good.
> > 
> > Is the device common enough that I can try and reproduce the issue?
> 
> No, it's integrated into the hi1620 SoC found in the D06 dev board only, but
> I don't think that there is anything special about this HW.
> 
> If
> > there's a Linux driver somewhere, I can always hack something in
> > emulation and find out...
> 
> Ok, the interrupt allocation for this particular driver in this test is in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c#n2393
> 
> Cheers,
> John
> 
> > 
> > Thanks,
> > 
> > 	M.
> > 
> 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
  2019-09-04 10:25     ` Andrew Murray
@ 2019-09-05  8:38       ` Marc Zyngier
       [not found]         ` <84f6756f-79f2-2e46-fe44-9a46be69f99d@huawei.com>
  0 siblings, 1 reply; 13+ messages in thread
From: Marc Zyngier @ 2019-09-05  8:38 UTC (permalink / raw)
  To: Andrew Murray, John Garry
  Cc: Thomas Gleixner, Bjorn Helgaas, Linux PCI, Linuxarm, luojiaxing,
	linux-kernel

On 04/09/2019 11:25, Andrew Murray wrote:
> On Wed, Sep 04, 2019 at 09:56:51AM +0100, John Garry wrote:
>> On 03/09/2019 17:16, Marc Zyngier wrote:
>>> Hi John,
>>>
>>> On 03/09/2019 15:09, John Garry wrote:
>>>> Hi Marc, Bjorn, Thomas,
>>
>> Hi Marc,
>>
>>>>
>>>> We've come across a conflict with the kernel/pci msi code and GIC ITS
>>>> driver on our arm64 system, whereby we can't unbind and re-bind a PCI
>>>> device driver under special conditions. I'll explain...
>>>>
>>>> Our PCI device support 32 MSIs. The driver attempts to allocate msi
>>>> vectors with min msi=17, max msi = 32, and affd.pre vectors = 16. For
>>>> our test we make nr_cpus = 1 (just anything less than 16).
>>>
>>> Just to confirm: this PCI device is requiring Multi-MSI, right? As
>>> opposed to MSI-X?
>>
>> Right, Multi-MSI.
>>
>>>
>>>> We find that the pci/kernel msi code gives us 17 vectors, but the GIC
>>>> ITS code reserves 32 lpi maps in its_irq_domain_alloc(). The problem
>>>> then occurs when unbinding the driver in its_irq_domain_free() call,
>>>> where we only clear bits for 17 vectors. So if we unbind the driver and
>>>> then attempt to bind again, it fails.
>>>
>>> Is this device, by any chance, sharing its requested-id with another
>>> device? By being behind a bridge of some sort?There is some code to
>>> deal with it, but I'm not sure it has ever been verified in anger...
>>
>> It's a RC iEP and there should be no requested-id sharing:
>>
>> root@ubuntu:/home/john#  lspci -s 74:02.0 -v
>> 74:02.0 Serial Attached SCSI controller: Huawei Technologies Co., Ltd.
>> HiSilicon SAS 3.0 HBA (rev 20)
>> Flags: bus master, fast devsel, latency 0, IRQ 23, NUMA node 0
>> Memory at a2000000 (32-bit, non-prefetchable) [size=32K]
>> Capabilities: [40] Express Root Complex Integrated Endpoint, MSI 00
>> Capabilities: [80] MSI: Enable+ Count=32/32 Maskable+ 64bit+
>> Capabilities: [b0] Power Management version 3
>> Kernel driver in use: hisi_sas_v3_hw
>>
>>>
>>>> Where the fault lies, I can't say. Maybe the kernel msi code should
>>>> always give power of 2 vectors - as I understand, the PCI spec mandates
>>>> this. Or maybe the GIC ITS driver has a problem in the free path, as
>>>> above. Or maybe the PCI driver should not be allowed to request !power
>>>> of 2 min/max vectors.
>>>>
>>>> Opinion?
>>>
>>> My hunch is that it is an ITS driver bug: the PCI layer is allowed to
>>> give any number of MSIs to an endpoint driver, as long as they match the
>>> requirements of the allocation for Multi-MSI.
>>
>> I would tend to say that, but isn't the requirement to allocate power of 2
>> msi vectors, which doesn't seem to be enforced in the kernel msi layer?
> 
> For a PCI device that supports MSI but not MSI-X - my understanding is that
> pci_alloc_irq_vectors_affinity and pci_alloc_irq_vectors will request *from
> the device* a power of 2 msi vectors between the min and max given by the
> driver - msi_setup_entry rounds up to nearest power of 2.
> 
> However this doesn't guarantee that pci_alloc_irq_vectors will return a
> power of 2. For example if you set maxvec to 17, then it will request
> 32 from the device and pci_alloc_irq_vectors will return 17 (i.e. it satisfies
> your request by over allocating, but still gives you what you asked for).
> 
> I'm not yet familiar with ITS, however if it is reserving 32 yet you only
> clear 17, then there is mismatch between the number actually reserved from
> the hardware, and the value returned from pci_alloc_irq_vectors.
> 
> (It looks like its_alloc_device_irq rounds up to the nearest power of 2).

That's a "feature" of the architecture. The ITT is sized by the number
of bits used to index the table, meaning that you can only describe a
power of two >= 2.

John, could you stick a "#define DEBUG 1" at the top of irq-gic-v3-its.c
and report the LPI allocations for this device?

Thanks,

	M.
-- 
Jazz is not dead, it just smells funny...

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
       [not found]         ` <84f6756f-79f2-2e46-fe44-9a46be69f99d@huawei.com>
@ 2019-09-05 10:02           ` Marc Zyngier
       [not found]             ` <8ac8e372-15a0-2f95-089c-c189b619ea62@huawei.com>
  0 siblings, 1 reply; 13+ messages in thread
From: Marc Zyngier @ 2019-09-05 10:02 UTC (permalink / raw)
  To: John Garry, Andrew Murray
  Cc: Thomas Gleixner, Bjorn Helgaas, Linux PCI, Linuxarm, luojiaxing,
	linux-kernel

On 05/09/2019 10:39, John Garry wrote:
>>
>> That's a "feature" of the architecture. The ITT is sized by the number
>> of bits used to index the table, meaning that you can only describe a
>> power of two >= 2.
>>
>> John, could you stick a "#define DEBUG 1" at the top of irq-gic-v3-its.c
>> and report the LPI allocations for this device?
>>
> 
> Hi Marc,
> 
> As requested, I enabled debug for that driver and here are some kernel 
> log snippets:
> 
> [    8.435707] hisi_sas_v3_hw 0000:74:02.0: Adding to iommu group 0
> [    8.461467] scsi host0: hisi_sas_v3_hw
> [    9.683463] ITS: alloc 9920:32
> [    9.686509] ITT 32 entries, 5 bits
> [    9.690044] ID:0 pID:9920 vID:23
> [    9.693263] ID:1 pID:9921 vID:24
> [    9.696480] ID:2 pID:9922 vID:25
> [    9.699696] ID:3 pID:9923 vID:26
> [    9.702911] ID:4 pID:9924 vID:27
> [    9.706128] ID:5 pID:9925 vID:28
> [    9.709344] ID:6 pID:9926 vID:29
> [    9.712560] ID:7 pID:9927 vID:30
> [    9.715776] ID:8 pID:9928 vID:31
> [    9.718990] ID:9 pID:9929 vID:32
> [    9.722207] ID:10 pID:9930 vID:33
> [    9.725510] ID:11 pID:9931 vID:34
> [    9.728813] ID:12 pID:9932 vID:35
> [    9.732116] ID:13 pID:9933 vID:36
> [    9.735419] ID:14 pID:9934 vID:37
> [    9.738721] ID:15 pID:9935 vID:38
> [    9.742024] ID:16 pID:9936 vID:39
> 
> <snip>
> 
> (none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/unbind
> 
> <snip>
> 
> root@(none)$
> $ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/bind
> [   41.110557] scsi host0: hisi_sas_v3_hw
> [   42.335455] Reusing ITT for devID 7410
> [   42.359151] hisi_sas_v3_hw: probe of 0000:74:02.0 failed with error -2
> sh: echo: write error: No such device
> root@(none)$

Very interesting. Somehow, we think that this is a *new* device that 
aliases with itself. Needless to say, that's unexpected. My hunch is 
that something goes wrong when freeing the device. Can you try adding 
the patch below and report what is happening on unbind?

Thanks,

	M.

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 1b5c3672aea2..3fed87e551d9 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2651,6 +2651,8 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
 
 		/* Nuke the entry in the domain */
 		irq_domain_reset_irq_data(data);
+
+		pr_debug("Freed devid %x LPI %ld\n", its_dev->device_id, data->hwirq);
 	}
 
 	mutex_lock(&its->dev_alloc_lock);
@@ -2667,6 +2669,7 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
 			     its_dev->event_map.nr_lpis);
 		kfree(its_dev->event_map.col_map);
 
+		pr_debug("Unmap devid %x\n", its_dev->device_id);
 		/* Unmap device/itt */
 		its_send_mapd(its_dev, 0);
 		its_free_device(its_dev);

-- 
Jazz is not dead, it just smells funny...

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
       [not found]             ` <8ac8e372-15a0-2f95-089c-c189b619ea62@huawei.com>
@ 2019-09-05 11:22               ` Marc Zyngier
  2019-09-05 13:26                 ` John Garry
  0 siblings, 1 reply; 13+ messages in thread
From: Marc Zyngier @ 2019-09-05 11:22 UTC (permalink / raw)
  To: John Garry, Andrew Murray
  Cc: Thomas Gleixner, Bjorn Helgaas, Linux PCI, Linuxarm, luojiaxing,
	linux-kernel

On 05/09/2019 11:35, John Garry wrote:
>>>
>>> Hi Marc,
>>>
>>> As requested, I enabled debug for that driver and here are some kernel
>>> log snippets:
>>>
>>> [    8.435707] hisi_sas_v3_hw 0000:74:02.0: Adding to iommu group 0
>>> [    8.461467] scsi host0: hisi_sas_v3_hw
>>> [    9.683463] ITS: alloc 9920:32
>>> [    9.686509] ITT 32 entries, 5 bits
>>> [    9.690044] ID:0 pID:9920 vID:23
>>> [    9.693263] ID:1 pID:9921 vID:24
>>> [    9.696480] ID:2 pID:9922 vID:25
>>> [    9.699696] ID:3 pID:9923 vID:26
>>> [    9.702911] ID:4 pID:9924 vID:27
>>> [    9.706128] ID:5 pID:9925 vID:28
>>> [    9.709344] ID:6 pID:9926 vID:29
>>> [    9.712560] ID:7 pID:9927 vID:30
>>> [    9.715776] ID:8 pID:9928 vID:31
>>> [    9.718990] ID:9 pID:9929 vID:32
>>> [    9.722207] ID:10 pID:9930 vID:33
>>> [    9.725510] ID:11 pID:9931 vID:34
>>> [    9.728813] ID:12 pID:9932 vID:35
>>> [    9.732116] ID:13 pID:9933 vID:36
>>> [    9.735419] ID:14 pID:9934 vID:37
>>> [    9.738721] ID:15 pID:9935 vID:38
>>> [    9.742024] ID:16 pID:9936 vID:39
>>>
>>> <snip>
>>>
>>> (none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/unbind
>>>
>>> <snip>
>>>
>>> root@(none)$
>>> $ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/bind
>>> [   41.110557] scsi host0: hisi_sas_v3_hw
>>> [   42.335455] Reusing ITT for devID 7410
>>> [   42.359151] hisi_sas_v3_hw: probe of 0000:74:02.0 failed with error -2
>>> sh: echo: write error: No such device
>>> root@(none)$
>>
>> Very interesting. Somehow, we think that this is a *new* device that
>> aliases with itself. Needless to say, that's unexpected. My hunch is
>> that something goes wrong when freeing the device. Can you try adding
>> the patch below and report what is happening on unbind?
>>
> 
> Hi Marc,
> 
> Here's the new snippet for unbind + bind:
> 
> (none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/unbind
> 
> <snip>
> 
> [  102.189618] Freed devid 7410 LPI 0
> [  102.193019] Freed devid 7410 LPI 0
> [  102.196420] Freed devid 7410 LPI 0
> [  102.199854] Freed devid 7410 LPI 0
> [  102.203242] Freed devid 7410 LPI 0
> [  102.206640] Freed devid 7410 LPI 0
> [  102.210036] Freed devid 7410 LPI 0
> [  102.213426] Freed devid 7410 LPI 0
> [  102.216816] Freed devid 7410 LPI 0
> [  102.220206] Freed devid 7410 LPI 0
> [  102.223596] Freed devid 7410 LPI 0
> [  102.226984] Freed devid 7410 LPI 0
> [  102.230373] Freed devid 7410 LPI 0
> [  102.233763] Freed devid 7410 LPI 0
> [  102.237152] Freed devid 7410 LPI 0
> [  102.240542] Freed devid 7410 LPI 0
> [  102.243931] Freed devid 7410 LPI 0
> root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/bind
> [  111.662451] scsi host0: hisi_sas_v3_hw
> [  112.887353] Reusing ITT for devID 7410
> [  112.911275] hisi_sas_v3_hw: probe of 0000:74:02.0 failed with error -2


OK, debug was slightly off, but it is interesting that the driver didn't 
unmap the device, either because it is flagged as shared (with what?) or 
that additional interrupts are allocated in the lpi_map for this 
instance.

Here's an updated debug patch. Can you please run the same thing again?

Thanks,

	M.

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 1b5c3672aea2..07375171900a 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3,7 +3,7 @@
  * Copyright (C) 2013-2017 ARM Limited, All Rights Reserved.
  * Author: Marc Zyngier <marc.zyngier@arm.com>
  */
-
+#define DEBUG 1
 #include <linux/acpi.h>
 #include <linux/acpi_iort.h>
 #include <linux/bitmap.h>
@@ -2649,6 +2649,8 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
 		/* Mark interrupt index as unused */
 		clear_bit(event, its_dev->event_map.lpi_map);
 
+		pr_debug("Freed devid %x event %d LPI %ld\n", its_dev->device_id, event, data->hwirq);
+
 		/* Nuke the entry in the domain */
 		irq_domain_reset_irq_data(data);
 	}
@@ -2659,6 +2661,9 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
 	 * If all interrupts have been freed, start mopping the
 	 * floor. This is conditionned on the device not being shared.
 	 */
+	pr_debug("Unmap devid %x shared %d lpi_map %*pbl\n",
+		 its_dev->device_id, its_dev->shared,
+		 its_dev->event_map.nr_lpis, its_dev->event_map.lpi_map);
 	if (!its_dev->shared &&
 	    bitmap_empty(its_dev->event_map.lpi_map,
 			 its_dev->event_map.nr_lpis)) {
@@ -2667,6 +2672,7 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
 			     its_dev->event_map.nr_lpis);
 		kfree(its_dev->event_map.col_map);
 
+		pr_debug("Unmap devid %x\n", its_dev->device_id);
 		/* Unmap device/itt */
 		its_send_mapd(its_dev, 0);
 		its_free_device(its_dev);


-- 
Jazz is not dead, it just smells funny...

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
  2019-09-05 11:22               ` Marc Zyngier
@ 2019-09-05 13:26                 ` John Garry
  2019-09-05 13:50                   ` Marc Zyngier
  0 siblings, 1 reply; 13+ messages in thread
From: John Garry @ 2019-09-05 13:26 UTC (permalink / raw)
  To: Marc Zyngier, Andrew Murray
  Cc: Thomas Gleixner, Bjorn Helgaas, Linux PCI, Linuxarm, luojiaxing,
	linux-kernel

On 05/09/2019 12:22, Marc Zyngier wrote:
> OK, debug was slightly off, but it is interesting that the driver didn't
> unmap the device, either because it is flagged as shared (with what?) or
> that additional interrupts are allocated in the lpi_map for this
> instance.
>
> Here's an updated debug patch. Can you please run the same thing again?
>

As requested:

root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/unbind

<snip>

[   78.593897] Freed devid 7410 event 0 LPI 0
[   78.597990] Freed devid 7410 event 1 LPI 0
[   78.602080] Freed devid 7410 event 2 LPI 0
[   78.606169] Freed devid 7410 event 3 LPI 0
[   78.610253] Freed devid 7410 event 4 LPI 0
[   78.614337] Freed devid 7410 event 5 LPI 0
[   78.618422] Freed devid 7410 event 6 LPI 0
[   78.622506] Freed devid 7410 event 7 LPI 0
[   78.626590] Freed devid 7410 event 8 LPI 0
[   78.630674] Freed devid 7410 event 9 LPI 0
[   78.634758] Freed devid 7410 event 10 LPI 0
[   78.638930] Freed devid 7410 event 11 LPI 0
[   78.643101] Freed devid 7410 event 12 LPI 0
[   78.647272] Freed devid 7410 event 13 LPI 0
[   78.651445] Freed devid 7410 event 14 LPI 0
[   78.655616] Freed devid 7410 event 15 LPI 0
[   78.659787] Freed devid 7410 event 16 LPI 0
[   78.663959] Unmap devid 7410 shared 0 lpi_map 17-31
root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/bind
[   86.074545] scsi host0: hisi_sas_v3_hw
[   87.299141] Reusing ITT for devID 7410
[   87.322337] hisi_sas_v3_hw: probe of 0000:74:02.0 failed with error -2
sh: echo: write error: No such device
root@(none)$


And here's a working log (without specifying nr_cpus=1), just for reference:

root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/unbind

<snip>

[   28.412419] Freed devid 7410 event 0 LPI 0
[   28.416505] Freed devid 7410 event 1 LPI 0
[   28.420590] Freed devid 7410 event 2 LPI 0
[   28.424674] Freed devid 7410 event 3 LPI 0
[   28.428759] Freed devid 7410 event 4 LPI 0
[   28.432844] Freed devid 7410 event 5 LPI 0
[   28.436929] Freed devid 7410 event 6 LPI 0
[   28.441013] Freed devid 7410 event 7 LPI 0
[   28.445099] Freed devid 7410 event 8 LPI 0
[   28.449183] Freed devid 7410 event 9 LPI 0
[   28.453268] Freed devid 7410 event 10 LPI 0
[   28.457439] Freed devid 7410 event 11 LPI 0
[   28.461611] Freed devid 7410 event 12 LPI 0
[   28.465782] Freed devid 7410 event 13 LPI 0
[   28.469954] Freed devid 7410 event 14 LPI 0
[   28.474126] Freed devid 7410 event 15 LPI 0
[   28.478298] Freed devid 7410 event 16 LPI 0
[   28.482469] Freed devid 7410 event 17 LPI 0
[   28.486641] Freed devid 7410 event 18 LPI 0
[   28.490812] Freed devid 7410 event 19 LPI 0
[   28.494984] Freed devid 7410 event 20 LPI 0
[   28.499155] Freed devid 7410 event 21 LPI 0
[   28.503327] Freed devid 7410 event 22 LPI 0
[   28.507498] Freed devid 7410 event 23 LPI 0
[   28.511670] Freed devid 7410 event 24 LPI 0
[   28.515842] Freed devid 7410 event 25 LPI 0
[   28.520017] Freed devid 7410 event 26 LPI 0
[   28.524189] Freed devid 7410 event 27 LPI 0
[   28.528360] Freed devid 7410 event 28 LPI 0
[   28.532531] Freed devid 7410 event 29 LPI 0
[   28.536703] Freed devid 7410 event 30 LPI 0
[   28.540874] Freed devid 7410 event 31 LPI 0
[   28.545047] Unmap devid 7410 shared 0 lpi_map
[   28.552084] Unmap devid 7410
root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/bind
[   34.900373] scsi host0: hisi_sas_v3_hw
[   36.112726] ITS: alloc 9920:32
[   36.115771] ITT 32 entries, 5 bits
[   36.119442] ID:0 pID:9920 vID:23
[   36.122661] ID:1 pID:9921 vID:24
[   36.125878] ID:2 pID:9922 vID:25
[   36.129095] ID:3 pID:9923 vID:26
[   36.132309] ID:4 pID:9924 vID:27
[   36.135526] ID:5 pID:9925 vID:28
[   36.138742] ID:6 pID:9926 vID:29
[   36.141959] ID:7 pID:9927 vID:30
[   36.145175] ID:8 pID:9928 vID:31
[   36.148390] ID:9 pID:9929 vID:32
[   36.151606] ID:10 pID:9930 vID:33
[   36.154910] ID:11 pID:9931 vID:34
[   36.158214] ID:12 pID:9932 vID:35
[   36.161517] ID:13 pID:9933 vID:36
[   36.164820] ID:14 pID:9934 vID:37
[   36.168121] ID:15 pID:9935 vID:38
[   36.171424] ID:16 pID:9936 vID:39
[   36.174727] ID:17 pID:9937 vID:40
[   36.178031] ID:18 pID:9938 vID:41
[   36.181334] ID:19 pID:9939 vID:42
[   36.184636] ID:20 pID:9940 vID:43
[   36.187939] ID:21 pID:9941 vID:44
[   36.191242] ID:22 pID:9942 vID:45
[   36.194545] ID:23 pID:9943 vID:46
[   36.197848] ID:24 pID:9944 vID:47
[   36.201152] ID:25 pID:9945 vID:48
[   36.204453] ID:26 pID:9946 vID:49
[   36.207756] ID:27 pID:9947 vID:50
[   36.211060] ID:28 pID:9948 vID:51
[   36.214363] ID:29 pID:9949 vID:52
[   36.217667] ID:30 pID:9950 vID:53
[   36.220970] ID:31 pID:9951 vID:54

<snip>


Cheers,
John

> Thanks,
>
> 	M.
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 1b5c3672aea2..07375171900a 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -3,7 +3,7 @@
>   * Copyright (C) 2013-2017 ARM Limited, All Rights Reserved.
>   * Author: Marc Zyngier <marc.zyngier@arm.com>
>   */
> -
> +#define DEBUG 1
>  #include <linux/acpi.h>
>  #include <linux/acpi_iort.h>
>  #include <linux/bitmap.h>
> @@ -2649,6 +2649,8 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
>  		/* Mark interrupt index as unused */
>  		clear_bit(event, its_dev->event_map.lpi_map);
>
> +		pr_debug("Freed devid %x event %d LPI %ld\n", its_dev->device_id, event, data->hwirq);
> +
>  		/* Nuke the entry in the domain */
>  		irq_domain_reset_irq_data(data);
>  	}
> @@ -2659,6 +2661,9 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
>  	 * If all interrupts have been freed, start mopping the
>  	 * floor. This is conditionned on the device not being shared.
>  	 */
> +	pr_debug("Unmap devid %x shared %d lpi_map %*pbl\n",
> +		 its_dev->device_id, its_dev->shared,
> +		 its_dev->event_map.nr_lpis, its_dev->event_map.lpi_map);
>  	if (!its_dev->shared &&
>  	    bitmap_empty(its_dev->event_map.lpi_map,
>  			 its_dev->event_map.nr_lpis)) {
> @@ -2667,6 +2672,7 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
>  			     its_dev->event_map.nr_lpis);
>  		kfree(its_dev->event_map.col_map);
>
> +		pr_debug("Unmap devid %x\n", its_dev->device_id);
>  		/* Unmap device/itt */
>  		its_send_mapd(its_dev, 0);
>  		its_free_device(its_dev);



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
  2019-09-05 13:26                 ` John Garry
@ 2019-09-05 13:50                   ` Marc Zyngier
  2019-09-05 14:23                     ` John Garry
  0 siblings, 1 reply; 13+ messages in thread
From: Marc Zyngier @ 2019-09-05 13:50 UTC (permalink / raw)
  To: John Garry, Andrew Murray
  Cc: Thomas Gleixner, Bjorn Helgaas, Linux PCI, Linuxarm, luojiaxing,
	linux-kernel

On 05/09/2019 14:26, John Garry wrote:
> On 05/09/2019 12:22, Marc Zyngier wrote:
>> OK, debug was slightly off, but it is interesting that the driver didn't
>> unmap the device, either because it is flagged as shared (with what?) or
>> that additional interrupts are allocated in the lpi_map for this
>> instance.
>>
>> Here's an updated debug patch. Can you please run the same thing again?
>>
> 
> As requested:
> 
> root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/unbind
> 
> <snip>
> 
> [   78.593897] Freed devid 7410 event 0 LPI 0
> [   78.597990] Freed devid 7410 event 1 LPI 0
> [   78.602080] Freed devid 7410 event 2 LPI 0
> [   78.606169] Freed devid 7410 event 3 LPI 0
> [   78.610253] Freed devid 7410 event 4 LPI 0
> [   78.614337] Freed devid 7410 event 5 LPI 0
> [   78.618422] Freed devid 7410 event 6 LPI 0
> [   78.622506] Freed devid 7410 event 7 LPI 0
> [   78.626590] Freed devid 7410 event 8 LPI 0
> [   78.630674] Freed devid 7410 event 9 LPI 0
> [   78.634758] Freed devid 7410 event 10 LPI 0
> [   78.638930] Freed devid 7410 event 11 LPI 0
> [   78.643101] Freed devid 7410 event 12 LPI 0
> [   78.647272] Freed devid 7410 event 13 LPI 0
> [   78.651445] Freed devid 7410 event 14 LPI 0
> [   78.655616] Freed devid 7410 event 15 LPI 0
> [   78.659787] Freed devid 7410 event 16 LPI 0
> [   78.663959] Unmap devid 7410 shared 0 lpi_map 17-31

Bah. Try this for size...

	M.

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 1b5c3672aea2..c3a8d732805f 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2641,14 +2641,13 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
 	struct its_node *its = its_dev->its;
 	int i;
 
+	bitmap_release_region(its_dev->event_map.lpi_map,
+			      its_get_event_id(irq_domain_get_irq_data(domain, virq)),
+			      get_count_order(nr_irqs));
+
 	for (i = 0; i < nr_irqs; i++) {
 		struct irq_data *data = irq_domain_get_irq_data(domain,
 								virq + i);
-		u32 event = its_get_event_id(data);
-
-		/* Mark interrupt index as unused */
-		clear_bit(event, its_dev->event_map.lpi_map);
-
 		/* Nuke the entry in the domain */
 		irq_domain_reset_irq_data(data);
 	}


-- 
Jazz is not dead, it just smells funny...

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
  2019-09-05 13:50                   ` Marc Zyngier
@ 2019-09-05 14:23                     ` John Garry
  2019-09-05 14:32                       ` Marc Zyngier
  0 siblings, 1 reply; 13+ messages in thread
From: John Garry @ 2019-09-05 14:23 UTC (permalink / raw)
  To: Marc Zyngier, Andrew Murray
  Cc: Thomas Gleixner, Bjorn Helgaas, Linux PCI, Linuxarm, luojiaxing,
	linux-kernel

On 05/09/2019 14:50, Marc Zyngier wrote:
> On 05/09/2019 14:26, John Garry wrote:
>> On 05/09/2019 12:22, Marc Zyngier wrote:
>>> OK, debug was slightly off, but it is interesting that the driver didn't
>>> unmap the device, either because it is flagged as shared (with what?) or
>>> that additional interrupts are allocated in the lpi_map for this
>>> instance.
>>>
>>> Here's an updated debug patch. Can you please run the same thing again?
>>>
>>
>> As requested:
>>
>> root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/unbind
>>
>> <snip>
>>
>> [   78.593897] Freed devid 7410 event 0 LPI 0
>> [   78.597990] Freed devid 7410 event 1 LPI 0
>> [   78.602080] Freed devid 7410 event 2 LPI 0
>> [   78.606169] Freed devid 7410 event 3 LPI 0
>> [   78.610253] Freed devid 7410 event 4 LPI 0
>> [   78.614337] Freed devid 7410 event 5 LPI 0
>> [   78.618422] Freed devid 7410 event 6 LPI 0
>> [   78.622506] Freed devid 7410 event 7 LPI 0
>> [   78.626590] Freed devid 7410 event 8 LPI 0
>> [   78.630674] Freed devid 7410 event 9 LPI 0
>> [   78.634758] Freed devid 7410 event 10 LPI 0
>> [   78.638930] Freed devid 7410 event 11 LPI 0
>> [   78.643101] Freed devid 7410 event 12 LPI 0
>> [   78.647272] Freed devid 7410 event 13 LPI 0
>> [   78.651445] Freed devid 7410 event 14 LPI 0
>> [   78.655616] Freed devid 7410 event 15 LPI 0
>> [   78.659787] Freed devid 7410 event 16 LPI 0
>> [   78.663959] Unmap devid 7410 shared 0 lpi_map 17-31
>
> Bah. Try this for size...
>

It fits:

root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/unbind

<snip>

[   34.806156] Freed devid 7410 LPI 0
[   34.809555] Freed devid 7410 LPI 0
[   34.812951] Freed devid 7410 LPI 0
[   34.816344] Freed devid 7410 LPI 0
[   34.819734] Freed devid 7410 LPI 0
[   34.823122] Freed devid 7410 LPI 0
[   34.826512] Freed devid 7410 LPI 0
[   34.829901] Freed devid 7410 LPI 0
[   34.833291] Freed devid 7410 LPI 0
[   34.836680] Freed devid 7410 LPI 0
[   34.840071] Freed devid 7410 LPI 0
[   34.843461] Freed devid 7410 LPI 0
[   34.846848] Freed devid 7410 LPI 0
[   34.850238] Freed devid 7410 LPI 0
[   34.853627] Freed devid 7410 LPI 0
[   34.857017] Freed devid 7410 LPI 0
[   34.860406] Freed devid 7410 LPI 0
[   34.863797] Unmap devid 7410 shared 0 lpi_map
[   34.868229] Unmap devid 7410
root@(none)$
root@(none)$
root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/bind
[   39.158802] scsi host0: hisi_sas_v3_hw
[   40.383384] ITS: alloc 9920:32
[   40.386429] ITT 32 entries, 5 bits
[   40.389970] ID:0 pID:9920 vID:23
[   40.393188] ID:1 pID:9921 vID:24
[   40.396404] ID:2 pID:9922 vID:25
[   40.399621] ID:3 pID:9923 vID:26
[   40.402836] ID:4 pID:9924 vID:27
[   40.406053] ID:5 pID:9925 vID:28
[   40.409269] ID:6 pID:9926 vID:29
[   40.412485] ID:7 pID:9927 vID:30
[   40.415702] ID:8 pID:9928 vID:31
[   40.418916] ID:9 pID:9929 vID:32
[   40.422132] ID:10 pID:9930 vID:33
[   40.425435] ID:11 pID:9931 vID:34
[   40.428739] ID:12 pID:9932 vID:35
[   40.432042] ID:13 pID:9933 vID:36
[   40.435345] ID:14 pID:9934 vID:37
[   40.438648] ID:15 pID:9935 vID:38
[   40.441951] ID:16 pID:9936 vID:39


<snip>

Btw, I hacked the "Freed devid %x event %d LPI %ld\n" print to remove 
the "event" value, as you may have noticed.

Cheers,
John

> 	M.
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 1b5c3672aea2..c3a8d732805f 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -2641,14 +2641,13 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
>  	struct its_node *its = its_dev->its;
>  	int i;
>
> +	bitmap_release_region(its_dev->event_map.lpi_map,
> +			      its_get_event_id(irq_domain_get_irq_data(domain, virq)),
> +			      get_count_order(nr_irqs));
> +
>  	for (i = 0; i < nr_irqs; i++) {
>  		struct irq_data *data = irq_domain_get_irq_data(domain,
>  								virq + i);
> -		u32 event = its_get_event_id(data);
> -
> -		/* Mark interrupt index as unused */
> -		clear_bit(event, its_dev->event_map.lpi_map);
> -
>  		/* Nuke the entry in the domain */
>  		irq_domain_reset_irq_data(data);
>  	}
>
>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
  2019-09-05 14:23                     ` John Garry
@ 2019-09-05 14:32                       ` Marc Zyngier
  2019-09-05 14:53                         ` John Garry
  0 siblings, 1 reply; 13+ messages in thread
From: Marc Zyngier @ 2019-09-05 14:32 UTC (permalink / raw)
  To: John Garry, Andrew Murray
  Cc: Thomas Gleixner, Bjorn Helgaas, Linux PCI, Linuxarm, luojiaxing,
	linux-kernel

On 05/09/2019 15:23, John Garry wrote:
> On 05/09/2019 14:50, Marc Zyngier wrote:
>> On 05/09/2019 14:26, John Garry wrote:
>>> On 05/09/2019 12:22, Marc Zyngier wrote:
>>>> OK, debug was slightly off, but it is interesting that the driver didn't
>>>> unmap the device, either because it is flagged as shared (with what?) or
>>>> that additional interrupts are allocated in the lpi_map for this
>>>> instance.
>>>>
>>>> Here's an updated debug patch. Can you please run the same thing again?
>>>>
>>>
>>> As requested:
>>>
>>> root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/unbind
>>>
>>> <snip>
>>>
>>> [   78.593897] Freed devid 7410 event 0 LPI 0
>>> [   78.597990] Freed devid 7410 event 1 LPI 0
>>> [   78.602080] Freed devid 7410 event 2 LPI 0
>>> [   78.606169] Freed devid 7410 event 3 LPI 0
>>> [   78.610253] Freed devid 7410 event 4 LPI 0
>>> [   78.614337] Freed devid 7410 event 5 LPI 0
>>> [   78.618422] Freed devid 7410 event 6 LPI 0
>>> [   78.622506] Freed devid 7410 event 7 LPI 0
>>> [   78.626590] Freed devid 7410 event 8 LPI 0
>>> [   78.630674] Freed devid 7410 event 9 LPI 0
>>> [   78.634758] Freed devid 7410 event 10 LPI 0
>>> [   78.638930] Freed devid 7410 event 11 LPI 0
>>> [   78.643101] Freed devid 7410 event 12 LPI 0
>>> [   78.647272] Freed devid 7410 event 13 LPI 0
>>> [   78.651445] Freed devid 7410 event 14 LPI 0
>>> [   78.655616] Freed devid 7410 event 15 LPI 0
>>> [   78.659787] Freed devid 7410 event 16 LPI 0
>>> [   78.663959] Unmap devid 7410 shared 0 lpi_map 17-31
>>
>> Bah. Try this for size...
>>
> 
> It fits:
> 
> root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/unbind
> 
> <snip>
> 
> [   34.806156] Freed devid 7410 LPI 0
> [   34.809555] Freed devid 7410 LPI 0
> [   34.812951] Freed devid 7410 LPI 0
> [   34.816344] Freed devid 7410 LPI 0
> [   34.819734] Freed devid 7410 LPI 0
> [   34.823122] Freed devid 7410 LPI 0
> [   34.826512] Freed devid 7410 LPI 0
> [   34.829901] Freed devid 7410 LPI 0
> [   34.833291] Freed devid 7410 LPI 0
> [   34.836680] Freed devid 7410 LPI 0
> [   34.840071] Freed devid 7410 LPI 0
> [   34.843461] Freed devid 7410 LPI 0
> [   34.846848] Freed devid 7410 LPI 0
> [   34.850238] Freed devid 7410 LPI 0
> [   34.853627] Freed devid 7410 LPI 0
> [   34.857017] Freed devid 7410 LPI 0
> [   34.860406] Freed devid 7410 LPI 0
> [   34.863797] Unmap devid 7410 shared 0 lpi_map
> [   34.868229] Unmap devid 7410
> root@(none)$
> root@(none)$
> root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/bind
> [   39.158802] scsi host0: hisi_sas_v3_hw
> [   40.383384] ITS: alloc 9920:32
> [   40.386429] ITT 32 entries, 5 bits
> [   40.389970] ID:0 pID:9920 vID:23
> [   40.393188] ID:1 pID:9921 vID:24
> [   40.396404] ID:2 pID:9922 vID:25
> [   40.399621] ID:3 pID:9923 vID:26
> [   40.402836] ID:4 pID:9924 vID:27
> [   40.406053] ID:5 pID:9925 vID:28
> [   40.409269] ID:6 pID:9926 vID:29
> [   40.412485] ID:7 pID:9927 vID:30
> [   40.415702] ID:8 pID:9928 vID:31
> [   40.418916] ID:9 pID:9929 vID:32
> [   40.422132] ID:10 pID:9930 vID:33
> [   40.425435] ID:11 pID:9931 vID:34
> [   40.428739] ID:12 pID:9932 vID:35
> [   40.432042] ID:13 pID:9933 vID:36
> [   40.435345] ID:14 pID:9934 vID:37
> [   40.438648] ID:15 pID:9935 vID:38
> [   40.441951] ID:16 pID:9936 vID:39
> 
> 
> <snip>

Awesome. Can I take this as a Tested-by?

> Btw, I hacked the "Freed devid %x event %d LPI %ld\n" print to remove 
> the "event" value, as you may have noticed.

Yup, not meaningful for the problem at hand.

Thanks again for your help!

	M.
-- 
Jazz is not dead, it just smells funny...

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
  2019-09-05 14:32                       ` Marc Zyngier
@ 2019-09-05 14:53                         ` John Garry
  2019-09-05 15:09                           ` Marc Zyngier
  0 siblings, 1 reply; 13+ messages in thread
From: John Garry @ 2019-09-05 14:53 UTC (permalink / raw)
  To: Marc Zyngier, Andrew Murray
  Cc: Thomas Gleixner, Bjorn Helgaas, Linux PCI, Linuxarm, luojiaxing,
	linux-kernel


>> >
>> > root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/unbind
>> >
>> > <snip>
>> >
>> > [   34.806156] Freed devid 7410 LPI 0
>> > [   34.809555] Freed devid 7410 LPI 0
>> > [   34.812951] Freed devid 7410 LPI 0
>> > [   34.816344] Freed devid 7410 LPI 0
>> > [   34.819734] Freed devid 7410 LPI 0
>> > [   34.823122] Freed devid 7410 LPI 0
>> > [   34.826512] Freed devid 7410 LPI 0
>> > [   34.829901] Freed devid 7410 LPI 0
>> > [   34.833291] Freed devid 7410 LPI 0
>> > [   34.836680] Freed devid 7410 LPI 0
>> > [   34.840071] Freed devid 7410 LPI 0
>> > [   34.843461] Freed devid 7410 LPI 0
>> > [   34.846848] Freed devid 7410 LPI 0
>> > [   34.850238] Freed devid 7410 LPI 0
>> > [   34.853627] Freed devid 7410 LPI 0
>> > [   34.857017] Freed devid 7410 LPI 0
>> > [   34.860406] Freed devid 7410 LPI 0
>> > [   34.863797] Unmap devid 7410 shared 0 lpi_map
>> > [   34.868229] Unmap devid 7410
>> > root@(none)$
>> > root@(none)$
>> > root@(none)$ echo 0000:74:02.0 > ./sys/bus/pci/drivers/hisi_sas_v3_hw/bind
>> > [   39.158802] scsi host0: hisi_sas_v3_hw
>> > [   40.383384] ITS: alloc 9920:32
>> > [   40.386429] ITT 32 entries, 5 bits
>> > [   40.389970] ID:0 pID:9920 vID:23
>> > [   40.393188] ID:1 pID:9921 vID:24
>> > [   40.396404] ID:2 pID:9922 vID:25
>> > [   40.399621] ID:3 pID:9923 vID:26
>> > [   40.402836] ID:4 pID:9924 vID:27
>> > [   40.406053] ID:5 pID:9925 vID:28
>> > [   40.409269] ID:6 pID:9926 vID:29
>> > [   40.412485] ID:7 pID:9927 vID:30
>> > [   40.415702] ID:8 pID:9928 vID:31
>> > [   40.418916] ID:9 pID:9929 vID:32
>> > [   40.422132] ID:10 pID:9930 vID:33
>> > [   40.425435] ID:11 pID:9931 vID:34
>> > [   40.428739] ID:12 pID:9932 vID:35
>> > [   40.432042] ID:13 pID:9933 vID:36
>> > [   40.435345] ID:14 pID:9934 vID:37
>> > [   40.438648] ID:15 pID:9935 vID:38
>> > [   40.441951] ID:16 pID:9936 vID:39
>> >
>> >
>> > <snip>
> Awesome. Can I take this as a Tested-by?

Sure, btw, could you please also add:

Reported-by: Jiaxing Luo <luojiaxing@huawei.com>

... as he did initial discovery and analysis on the problem.


>
>> > Btw, I hacked the "Freed devid %x event %d LPI %ld\n" print to remove
>> > the "event" value, as you may have noticed.
> Yup, not meaningful for the problem at hand.
>
> Thanks again for your help!
>

Thanks for the help,
John

> 	M.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: PCI/kernel msi code vs GIC ITS driver conflict?
  2019-09-05 14:53                         ` John Garry
@ 2019-09-05 15:09                           ` Marc Zyngier
  0 siblings, 0 replies; 13+ messages in thread
From: Marc Zyngier @ 2019-09-05 15:09 UTC (permalink / raw)
  To: John Garry, Andrew Murray
  Cc: Thomas Gleixner, Bjorn Helgaas, Linux PCI, Linuxarm, luojiaxing,
	linux-kernel

On 05/09/2019 15:53, John Garry wrote:

[...[

>> Awesome. Can I take this as a Tested-by?
> 
> Sure, btw, could you please also add:
> 
> Reported-by: Jiaxing Luo <luojiaxing@huawei.com>
> 
> ... as he did initial discovery and analysis on the problem.

Sure. Now pushed to irqchip-next.

Thanks,
	
	M.
-- 
Jazz is not dead, it just smells funny...

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-09-05 15:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-03 14:09 PCI/kernel msi code vs GIC ITS driver conflict? John Garry
2019-09-03 16:16 ` Marc Zyngier
2019-09-04  8:56   ` John Garry
2019-09-04 10:25     ` Andrew Murray
2019-09-05  8:38       ` Marc Zyngier
     [not found]         ` <84f6756f-79f2-2e46-fe44-9a46be69f99d@huawei.com>
2019-09-05 10:02           ` Marc Zyngier
     [not found]             ` <8ac8e372-15a0-2f95-089c-c189b619ea62@huawei.com>
2019-09-05 11:22               ` Marc Zyngier
2019-09-05 13:26                 ` John Garry
2019-09-05 13:50                   ` Marc Zyngier
2019-09-05 14:23                     ` John Garry
2019-09-05 14:32                       ` Marc Zyngier
2019-09-05 14:53                         ` John Garry
2019-09-05 15:09                           ` Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).