On 5/24/24 10:56, Dan Carpenter wrote: > On Thu, May 23, 2024 at 11:21:52AM -0400, Sean Anderson wrote: >> On 5/22/24 18:28, Bjorn Helgaas wrote: >> > On Mon, May 20, 2024 at 10:53:57AM -0400, Sean Anderson wrote: >> >> MSGF_LEG_MASK is laid out with INTA in bit 0, INTB in bit 1, INTC in bit >> >> 2, and INTD in bit 3. Hardware IRQ numbers start at 0, and we register >> >> PCI_NUM_INTX irqs. So to enable INTA (aka hwirq 0) we should set bit 0. >> >> Remove the subtraction of one. This fixes the following UBSAN error: >> > >> > Thanks for these details! >> > >> > I guess UBSAN == "undefined behavior sanitizer", right? That sounds >> > like an easy way to find this but not the way users are likely to find >> > it. >> >> It's pretty likely they will find it this way, since I found it this way >> and no one else had ;) >> >> > I assume users would notice spurious and missing interrupts, e.g., >> > a driver that tried to enable INTB would have actually enabled INTA, >> > so we'd see spurious INTA interrupts and the driver would never see >> > the INTB it expected. >> > >> > And a driver that tried to enable INTA would never see that interrupt, >> > and we might not set any bit in MSGF_LEG_MASK? >> >> And yes, this would manifest as INTx interrupts being broken. >> > > It's so weird that it's been broken for seven years and no one reported > it. :/ If I had to guess it's because most PCIe hardware uses MSIs. Unless you plugged in a PCI bridge there's almost no reason to use INTx at all. --Sean
