Created attachment 245210 [details] Fix to align the msi vector properly MSI interrupt vectors needs to be aligned to the size of the request and have to be in powers of 2. The code in native_apic_alloc_vectors() does an alignment check in the loop: if ((vector & (align - 1)) != 0) continue; first = vector; But when it adds APIC_IO_INTS to the value it returns: return (first + APIC_IO_INTS); The problem is that APIC_IO_INTS is not a multiple of 32. It is 48: #define NRSVIDT 32 #define IDT_IO_INTS NRSVIDT #define APIC_IO_INTS (IDT_IO_INTS + 16) My fix is to include it during the alignment check: --- /sys/x86/x86/local_apic.c.org 2023-04-07 02:34:45.000000000 +0200 +++ /sys/x86/x86/local_apic.c 2023-09-24 21:10:04.785720000 +0200 @@ -1657,7 +1657,7 @@ /* Start a new run if run == 0 and vector is aligned. */ if (run == 0) { - if ((vector & (align - 1)) != 0) + if (((vector + APIC_IO_INTS) & (align - 1)) != 0) continue; first = vector; } The patch is against 13.2, but the code in current looks the same. Without the patch, pci_alloc_msi() with a count of 32 would return without an error and allocate 32 vectors, but in my case with a base vector of 0x50 and that was configured on the card. But the card would then ignore the bottom 5 bits and generate interrupt vectors from 0x40 to 0x5f.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=d33a4ae8ba5343f555842e6e32321f9cd64dfd09 commit d33a4ae8ba5343f555842e6e32321f9cd64dfd09 Author: John Hay <jhay@FreeBSD.org> AuthorDate: 2023-09-28 21:08:08 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2023-09-28 21:08:08 +0000 x86: Properly align interrupt vectors for MSI MSI (not MSI-X) interrupt vectors must be allocated in groups that are powers of 2, and the block of IDT vectors must be aligned to the size of the request. The code in native_apic_alloc_vectors() does an alignment check in the loop: if ((vector & (align - 1)) != 0) continue; first = vector; But it adds APIC_IO_INTS to the value it returns: return (first + APIC_IO_INTS); The problem is that APIC_IO_INTS is not a multiple of 32. It is 48: As a result, a request for 32 vectors (the max supported by MSI), was not always aligned. To fix, check the alignment of 'vector + APIC_IO_INTS' in the loop. PR: 274074 Reviewed by: jhb sys/x86/x86/local_apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Thank you, John. Any chance of it making its way to 14 and maybe 13?
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=d73b4f06403af6c1a384c8c59ba9eb138c861ee1 commit d73b4f06403af6c1a384c8c59ba9eb138c861ee1 Author: John Hay <jhay@FreeBSD.org> AuthorDate: 2023-09-28 21:08:08 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2023-10-11 15:10:32 +0000 x86: Properly align interrupt vectors for MSI MSI (not MSI-X) interrupt vectors must be allocated in groups that are powers of 2, and the block of IDT vectors must be aligned to the size of the request. The code in native_apic_alloc_vectors() does an alignment check in the loop: if ((vector & (align - 1)) != 0) continue; first = vector; But it adds APIC_IO_INTS to the value it returns: return (first + APIC_IO_INTS); The problem is that APIC_IO_INTS is not a multiple of 32. It is 48: As a result, a request for 32 vectors (the max supported by MSI), was not always aligned. To fix, check the alignment of 'vector + APIC_IO_INTS' in the loop. PR: 274074 Reviewed by: jhb (cherry picked from commit d33a4ae8ba5343f555842e6e32321f9cd64dfd09) sys/x86/x86/local_apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=358db6d8825b38296a205206ac3d25b2084aa5f4 commit 358db6d8825b38296a205206ac3d25b2084aa5f4 Author: John Hay <jhay@FreeBSD.org> AuthorDate: 2023-09-28 21:08:08 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2023-10-11 15:09:52 +0000 x86: Properly align interrupt vectors for MSI MSI (not MSI-X) interrupt vectors must be allocated in groups that are powers of 2, and the block of IDT vectors must be aligned to the size of the request. The code in native_apic_alloc_vectors() does an alignment check in the loop: if ((vector & (align - 1)) != 0) continue; first = vector; But it adds APIC_IO_INTS to the value it returns: return (first + APIC_IO_INTS); The problem is that APIC_IO_INTS is not a multiple of 32. It is 48: As a result, a request for 32 vectors (the max supported by MSI), was not always aligned. To fix, check the alignment of 'vector + APIC_IO_INTS' in the loop. PR: 274074 Reviewed by: jhb (cherry picked from commit d33a4ae8ba5343f555842e6e32321f9cd64dfd09) sys/x86/x86/local_apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
A commit in branch releng/14.0 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=9851b24ed4fa571ae3b931f834d21b9f800db9e9 commit 9851b24ed4fa571ae3b931f834d21b9f800db9e9 Author: John Hay <jhay@FreeBSD.org> AuthorDate: 2023-09-28 21:08:08 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2023-10-16 23:04:33 +0000 x86: Properly align interrupt vectors for MSI MSI (not MSI-X) interrupt vectors must be allocated in groups that are powers of 2, and the block of IDT vectors must be aligned to the size of the request. The code in native_apic_alloc_vectors() does an alignment check in the loop: if ((vector & (align - 1)) != 0) continue; first = vector; But it adds APIC_IO_INTS to the value it returns: return (first + APIC_IO_INTS); The problem is that APIC_IO_INTS is not a multiple of 32. It is 48: As a result, a request for 32 vectors (the max supported by MSI), was not always aligned. To fix, check the alignment of 'vector + APIC_IO_INTS' in the loop. PR: 274074 Reviewed by: jhb (cherry picked from commit d33a4ae8ba5343f555842e6e32321f9cd64dfd09) (cherry picked from commit d73b4f06403af6c1a384c8c59ba9eb138c861ee1) Approved by: re (karels) sys/x86/x86/local_apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)