diff mbox series

[v3,6/7] irqchip/gic-v3-mbi: Don't map the MSI page in mbi_compose_m{b, s}i_msg()

Message ID 20190501135824.25586-7-julien.grall@arm.com
State New
Headers show
Series iommu/dma-iommu: Split iommu_dma_map_msi_msg in two parts | expand

Commit Message

Julien Grall May 1, 2019, 1:58 p.m. UTC 
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@arm.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(-)

-- 
2.11.0

Comments

Marc Zyngier May 3, 2019, 2:27 p.m. UTC | #1 
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@arm.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.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
Robin Murphy May 3, 2019, 2:52 p.m. UTC | #2 
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@arm.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.
diff mbox series

Patch

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;
+
 	for (i = 0; i < nr_irqs; i++) {
 		err = mbi_irq_gic_domain_alloc(domain, virq + i, hwirq + i);
 		if (err)
@@ -142,7 +153,7 @@  static void mbi_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 	msg[0].address_lo = lower_32_bits(mbi_phys_base + GICD_SETSPI_NSR);
 	msg[0].data = data->parent_data->hwirq;
 
-	iommu_dma_map_msi_msg(data->irq, msg);
+	iommu_dma_compose_msi_msg(irq_data_get_msi_desc(data), msg);
 }
 
 #ifdef CONFIG_PCI_MSI
@@ -202,7 +213,7 @@  static void mbi_compose_mbi_msg(struct irq_data *data, struct msi_msg *msg)
 	msg[1].address_lo = lower_32_bits(mbi_phys_base + GICD_CLRSPI_NSR);
 	msg[1].data = data->parent_data->hwirq;
 
-	iommu_dma_map_msi_msg(data->irq, &msg[1]);
+	iommu_dma_compose_msi_msg(irq_data_get_msi_desc(data), &msg[1]);
 }
 
 /* Platform-MSI specific irqchip */