Skip to content

Commit

Permalink
PCI MSI: allow alignment restrictions on vector allocation
Browse files Browse the repository at this point in the history
ath9k hardware claims to support up to 4 MSI vectors, and when run in
that configuration, it would be allowed to modify the lower bits of the
MSI Message Data when generating interrupts in order to signal which
of the 4 vectors the interrupt is being raised on.

Linux's PCI-MSI irqchip only supports a single MSI vector for each
device, and it tells the device this, but the device appears to assume
it is working with 4, as it will unset the lower 2 bits of Message Data
presumably to indicate that it is an IRQ for the first of 4 possible
vectors.

Linux will then receive an interrupt on the wrong vector, so the
ath9k interrupt handler will not be invoked.

To work around this, introduce a mechanism where the vector assignment
algorithm can be restricted to only a subset of available vector numbers
based on a bitmap.

As a user of this bitmap, introduce a pci_dev.align_msi_vector flag which
can be used to state that MSI vector numbers must be aligned to a specific
amount. If we 4-align the ath9k MSI vector then the lower bits will
already be 0 and hence the device will not modify the Message Data away
from its original value.

Signed-off-by: Daniel Drake <[email protected]>

https://phabricator.endlessm.com/T16988
  • Loading branch information
dsd committed Jul 9, 2019
1 parent e2a7a7e commit 35d8c7c
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 15 deletions.
1 change: 1 addition & 0 deletions arch/x86/include/asm/hw_irq.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ struct irq_alloc_info {
struct {
struct pci_dev *msi_dev;
irq_hw_number_t msi_hwirq;
unsigned int vector_align;
};
#endif
#ifdef CONFIG_X86_IO_APIC
Expand Down
2 changes: 2 additions & 0 deletions arch/x86/kernel/apic/msi.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ int pci_msi_prepare(struct irq_domain *domain, struct device *dev, int nvec,
arg->flags |= X86_IRQ_ALLOC_CONTIGUOUS_VECTORS;
}

arg->vector_align = pdev->align_msi_vector;

return 0;
}
EXPORT_SYMBOL_GPL(pci_msi_prepare);
Expand Down
13 changes: 10 additions & 3 deletions arch/x86/kernel/apic/vector.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ struct apic_chip_data {
unsigned int cpu;
unsigned int prev_cpu;
unsigned int irq;
unsigned int vector_align;
struct hlist_node clist;
unsigned int move_in_progress : 1,
is_managed : 1,
Expand Down Expand Up @@ -189,7 +190,8 @@ static int reserve_managed_vector(struct irq_data *irqd)

raw_spin_lock_irqsave(&vector_lock, flags);
apicd->is_managed = true;
ret = irq_matrix_reserve_managed(vector_matrix, affmsk);
ret = irq_matrix_reserve_managed(vector_matrix, affmsk,
apicd->vector_align);
raw_spin_unlock_irqrestore(&vector_lock, flags);
trace_vector_reserve_managed(irqd->irq, ret);
return ret;
Expand Down Expand Up @@ -244,7 +246,8 @@ assign_vector_locked(struct irq_data *irqd, const struct cpumask *dest)
if (apicd->move_in_progress || !hlist_unhashed(&apicd->clist))
return -EBUSY;

vector = irq_matrix_alloc(vector_matrix, dest, resvd, &cpu);
vector = irq_matrix_alloc(vector_matrix, dest, resvd, &cpu,
apicd->vector_align);
trace_vector_alloc(irqd->irq, vector, resvd, vector);
if (vector < 0)
return vector;
Expand Down Expand Up @@ -317,7 +320,7 @@ assign_managed_vector(struct irq_data *irqd, const struct cpumask *dest)
if (apicd->vector && cpumask_test_cpu(apicd->cpu, vector_searchmask))
return 0;
vector = irq_matrix_alloc_managed(vector_matrix, vector_searchmask,
&cpu);
&cpu, apicd->vector_align);
trace_vector_alloc_managed(irqd->irq, vector, vector);
if (vector < 0)
return vector;
Expand Down Expand Up @@ -540,6 +543,10 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
goto error;
}

if (info->type == X86_IRQ_ALLOC_TYPE_MSI
|| info->type == X86_IRQ_ALLOC_TYPE_MSIX)
apicd->vector_align = info->vector_align;

apicd->irq = virq + i;
irqd->chip = &lapic_controller;
irqd->chip_data = apicd;
Expand Down
6 changes: 3 additions & 3 deletions include/linux/irq.h
Original file line number Diff line number Diff line change
Expand Up @@ -1164,14 +1164,14 @@ struct irq_matrix *irq_alloc_matrix(unsigned int matrix_bits,
void irq_matrix_online(struct irq_matrix *m);
void irq_matrix_offline(struct irq_matrix *m);
void irq_matrix_assign_system(struct irq_matrix *m, unsigned int bit, bool replace);
int irq_matrix_reserve_managed(struct irq_matrix *m, const struct cpumask *msk);
int irq_matrix_reserve_managed(struct irq_matrix *m, const struct cpumask *msk, unsigned int align);
void irq_matrix_remove_managed(struct irq_matrix *m, const struct cpumask *msk);
int irq_matrix_alloc_managed(struct irq_matrix *m, const struct cpumask *msk,
unsigned int *mapped_cpu);
unsigned int *mapped_cpu, unsigned int align);
void irq_matrix_reserve(struct irq_matrix *m);
void irq_matrix_remove_reserved(struct irq_matrix *m);
int irq_matrix_alloc(struct irq_matrix *m, const struct cpumask *msk,
bool reserved, unsigned int *mapped_cpu);
bool reserved, unsigned int *mapped_cpu, unsigned int align);
void irq_matrix_free(struct irq_matrix *m, unsigned int cpu,
unsigned int bit, bool managed);
void irq_matrix_assign(struct irq_matrix *m, unsigned int bit);
Expand Down
1 change: 1 addition & 0 deletions include/linux/pci.h
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ struct pci_dev {
#endif
#ifdef CONFIG_PCI_MSI
const struct attribute_group **msi_irq_groups;
unsigned int align_msi_vector;
#endif
struct pci_vpd *vpd;
#ifdef CONFIG_PCI_ATS
Expand Down
25 changes: 16 additions & 9 deletions kernel/irq/matrix.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,17 @@ void irq_matrix_offline(struct irq_matrix *m)
}

static unsigned int matrix_alloc_area(struct irq_matrix *m, struct cpumap *cm,
unsigned int num, bool managed)
unsigned int num, bool managed, unsigned int align)
{
unsigned int area, start = m->alloc_start;
unsigned int end = m->alloc_end;

if (align > 0)
align--;

bitmap_or(m->scratch_map, cm->managed_map, m->system_map, end);
bitmap_or(m->scratch_map, m->scratch_map, cm->alloc_map, end);
area = bitmap_find_next_zero_area(m->scratch_map, end, start, num, 0);
area = bitmap_find_next_zero_area(m->scratch_map, end, start, num, align);
if (area >= end)
return area;
if (managed)
Expand Down Expand Up @@ -207,15 +210,15 @@ void irq_matrix_assign_system(struct irq_matrix *m, unsigned int bit,
* on all CPUs in @msk, but it's not guaranteed that the bits are at the
* same offset on all CPUs
*/
int irq_matrix_reserve_managed(struct irq_matrix *m, const struct cpumask *msk)
int irq_matrix_reserve_managed(struct irq_matrix *m, const struct cpumask *msk, unsigned int align)
{
unsigned int cpu, failed_cpu;

for_each_cpu(cpu, msk) {
struct cpumap *cm = per_cpu_ptr(m->maps, cpu);
unsigned int bit;

bit = matrix_alloc_area(m, cm, 1, true);
bit = matrix_alloc_area(m, cm, 1, true, align);
if (bit >= m->alloc_end)
goto cleanup;
cm->managed++;
Expand Down Expand Up @@ -283,7 +286,7 @@ void irq_matrix_remove_managed(struct irq_matrix *m, const struct cpumask *msk)
* @cpu: On which CPU the interrupt should be allocated
*/
int irq_matrix_alloc_managed(struct irq_matrix *m, const struct cpumask *msk,
unsigned int *mapped_cpu)
unsigned int *mapped_cpu, unsigned int align)
{
unsigned int bit, cpu, end = m->alloc_end;
struct cpumap *cm;
Expand All @@ -297,9 +300,14 @@ int irq_matrix_alloc_managed(struct irq_matrix *m, const struct cpumask *msk,

cm = per_cpu_ptr(m->maps, cpu);
end = m->alloc_end;

if (align > 0)
align--;

/* Get managed bit which are not allocated */
bitmap_andnot(m->scratch_map, cm->managed_map, cm->alloc_map, end);
bit = find_first_bit(m->scratch_map, end);
bitmap_complement(m->scratch_map, m->scratch_map, end);
bit = bitmap_find_next_zero_area(m->scratch_map, end, 0, 1, align);
if (bit >= end)
return -ENOSPC;
set_bit(bit, cm->alloc_map);
Expand Down Expand Up @@ -375,7 +383,7 @@ void irq_matrix_remove_reserved(struct irq_matrix *m)
* @mapped_cpu: Pointer to store the CPU for which the irq was allocated
*/
int irq_matrix_alloc(struct irq_matrix *m, const struct cpumask *msk,
bool reserved, unsigned int *mapped_cpu)
bool reserved, unsigned int *mapped_cpu, unsigned int align)
{
unsigned int cpu, bit;
struct cpumap *cm;
Expand All @@ -385,7 +393,7 @@ int irq_matrix_alloc(struct irq_matrix *m, const struct cpumask *msk,
return -ENOSPC;

cm = per_cpu_ptr(m->maps, cpu);
bit = matrix_alloc_area(m, cm, 1, false);
bit = matrix_alloc_area(m, cm, 1, false, align);
if (bit >= m->alloc_end)
return -ENOSPC;
cm->allocated++;
Expand All @@ -397,7 +405,6 @@ int irq_matrix_alloc(struct irq_matrix *m, const struct cpumask *msk,
*mapped_cpu = cpu;
trace_irq_matrix_alloc(bit, cpu, m, cm);
return bit;

}

/**
Expand Down

0 comments on commit 35d8c7c

Please sign in to comment.