linux-kernel - Re: [PATCH v3 6/7] irqchip/gic-v3-mbi: Don't map the MSI page in mbi_compose_m{b, s}i

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] [day] [month] [year] [list]

Message-ID: <b339523b-4bab-02c8-7aea-56ac4c4f3023@arm.com>
Date:   Fri, 3 May 2019 15:52:08 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Marc Zyngier <marc.zyngier@....com>,
        Julien Grall <julien.grall@....com>,
        linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org
Cc:     logang@...tatee.com, douliyangs@...il.com,
        miquel.raynal@...tlin.com, jason@...edaemon.net,
        tglx@...utronix.de, joro@...tes.org, bigeasy@...utronix.de,
        linux-rt-users@...r.kernel.org
Subject: Re: [PATCH v3 6/7] irqchip/gic-v3-mbi: Don't map the MSI page in
 mbi_compose_m{b, s}i_msg()

On 03/05/2019 15:27, Marc Zyngier wrote:
> On 01/05/2019 14:58, Julien Grall wrote:
>> The functions mbi_compose_m{b, s}i_msg may be called from non-preemptible
>> context. However, on RT, iommu_dma_map_msi_msg() requires to be called
>> from a preemptible context.
>>
>> A recent patch split iommu_dma_map_msi_msg in two new functions:
>> one that should be called in preemptible context, the other does
>> not have any requirement.
>>
>> The GICv3 MSI driver is reworked to avoid executing preemptible code in
>> non-preemptible context. This can be achieved by preparing the two MSI
>> mappings when allocating the MSI interrupt.
>>
>> Signed-off-by: Julien Grall <julien.grall@....com>
>>
>> ---
>>      Changes in v2:
>>          - Rework the commit message to use imperative mood
>> ---
>>   drivers/irqchip/irq-gic-v3-mbi.c | 15 +++++++++++++--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic-v3-mbi.c b/drivers/irqchip/irq-gic-v3-mbi.c
>> index fbfa7ff6deb1..d50f6cdf043c 100644
>> --- a/drivers/irqchip/irq-gic-v3-mbi.c
>> +++ b/drivers/irqchip/irq-gic-v3-mbi.c
>> @@ -84,6 +84,7 @@ static void mbi_free_msi(struct mbi_range *mbi, unsigned int hwirq,
>>   static int mbi_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
>>   				   unsigned int nr_irqs, void *args)
>>   {
>> +	msi_alloc_info_t *info = args;
>>   	struct mbi_range *mbi = NULL;
>>   	int hwirq, offset, i, err = 0;
>>   
>> @@ -104,6 +105,16 @@ static int mbi_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
>>   
>>   	hwirq = mbi->spi_start + offset;
>>   
>> +	err = iommu_dma_prepare_msi(info->desc,
>> +				    mbi_phys_base + GICD_CLRSPI_NSR);
>> +	if (err)
>> +		return err;
>> +
>> +	err = iommu_dma_prepare_msi(info->desc,
>> +				    mbi_phys_base + GICD_SETSPI_NSR);
>> +	if (err)
>> +		return err;
> 
> Nit here: The two registers are guaranteed to be in the same 4k page
> (respectively offsets 0x48 and 0x40), so the second call is at best
> useless. I'll fix it when applying it, no need to resend.

Ack; the second call will simply find the existing page and overwrite 
desc->iommu_cookie with the same value. And if the two calls *did* ever 
come back with different pages, that logic would be quietly broken anyway.

Robin.