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 <drake@endlessm.com>

https://phabricator.endlessm.com/T16988
  • Loading branch information
dsd authored and starnight committed Mar 2, 2023
1 parent 791f66a commit 0a2d86a
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
Expand Up @@ -85,6 +85,7 @@ struct irq_alloc_info {
struct ioapic_alloc_info ioapic;
struct uv_alloc_info uv;
};
unsigned int vector_align;
};

struct irq_cfg {
Expand Down
2 changes: 2 additions & 0 deletions arch/x86/kernel/apic/msi.c
Expand Up @@ -168,6 +168,8 @@ int pci_msi_prepare(struct irq_domain *domain, struct device *dev, int nvec,
arg->flags |= X86_IRQ_ALLOC_CONTIGUOUS_VECTORS;
}

arg->vector_align = to_pci_dev(dev)->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
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 @@ -190,7 +191,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 @@ -245,7 +247,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 @@ -322,7 +325,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 @@ -562,6 +565,10 @@ static int x86_vector_alloc_irqs(struct irq_domain *domain, unsigned int virq,
goto error;
}

if (info->type == X86_IRQ_ALLOC_TYPE_PCI_MSI
|| info->type == X86_IRQ_ALLOC_TYPE_PCI_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
Expand Up @@ -1242,14 +1242,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
Expand Up @@ -485,6 +485,7 @@ struct pci_dev {
#ifdef CONFIG_PCI_MSI
void __iomem *msix_base;
raw_spinlock_t msi_lock;
unsigned int align_msi_vector;
#endif
struct pci_vpd vpd;
#ifdef CONFIG_PCIE_DPC
Expand Down
25 changes: 16 additions & 9 deletions kernel/irq/matrix.c
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 @@ -284,7 +287,7 @@ void irq_matrix_remove_managed(struct irq_matrix *m, const struct cpumask *msk)
* @mapped_cpu: Pointer to store the CPU for which the irq was 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;
struct cpumap *cm;
Expand All @@ -298,9 +301,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 @@ -392,7 +400,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 @@ -404,7 +412,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 0a2d86a

Please sign in to comment.