linux-kernel - Re: PCI/kernel msi code vs GIC ITS driver conflict?

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ecdb638b-d5d3-efdc-becd-478ce6e6ff96@huawei.com>
Date:   Thu, 5 Sep 2019 15:23:30 +0100
From:   John Garry <john.garry@...wei.com>
To:     Marc Zyngier <maz@...nel.org>,
        Andrew Murray <andrew.murray@....com>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Linux PCI <linux-pci@...r.kernel.org>,
        Linuxarm <linuxarm@...wei.com>,
        "luojiaxing@...wei.com" <luojiaxing@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: PCI/kernel msi code vs GIC ITS driver conflict?

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);
>  	}
>
>