On Fri, Jul 16, 2021 at 10:03:47PM -0400, Adam Stouffer wrote:
> On Fri, Jul 16, 2021 at 6:47 PM Jonathan Matthew <jonat...@d14n.org> wrote:
> >
> >
> > I think the problem here is that we don't check if msi is enabled
> > before deciding we can use msix. Can you try this diff out?
> > I wrote this after seeing a similar report somewhere, but I can't find
> > it now.
> >
> > Index: pci.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/pci/pci.c,v
> > retrieving revision 1.119
> > diff -u -p -r1.119 pci.c
> > --- pci.c 8 Sep 2020 20:13:52 -0000 1.119
> > +++ pci.c 22 Jun 2021 02:55:50 -0000
> > @@ -410,16 +410,48 @@ pcisubmatch(struct device *parent, void
> > }
> >
> > int
> > +pci_device_msi_enabled(pci_chipset_tag_t pc, pcitag_t tag)
> > +{
> > + int off;
> > + pcireg_t cap;
> > + uint64_t addr;
> > +
> > + if (pci_get_ht_capability(pc, tag, PCI_HT_CAP_MSI, &off, &cap)) {
> > + /*
> > + * XXX Should we enable MSI mapping ourselves on
> > + * systems that have it disabled?
> > + */
> > + if (cap & PCI_HT_MSI_ENABLED) {
> > + if ((cap & PCI_HT_MSI_FIXED) == 0) {
> > + addr = pci_conf_read(pc, tag,
> > + off + PCI_HT_MSI_ADDR);
> > + addr |= (uint64_t)pci_conf_read(pc, tag,
> > + off + PCI_HT_MSI_ADDR_HI32) << 32;
> > + } else
> > + addr = PCI_HT_MSI_FIXED_ADDR;
> > +
> > + /*
> > + * XXX This will fail to enable MSI on systems
> > + * that don't use the canonical address.
> > + */
> > + if (addr == PCI_HT_MSI_FIXED_ADDR)
> > + return (1);
> > + }
> > + }
> > +
> > + return (0);
> > +}
> > +
> > +int
> > pci_probe_device(struct pci_softc *sc, pcitag_t tag,
> > int (*match)(struct pci_attach_args *), struct pci_attach_args *pap)
> > {
> > pci_chipset_tag_t pc = sc->sc_pc;
> > struct pci_attach_args pa;
> > struct pci_dev *pd;
> > - pcireg_t id, class, intr, bhlcr, cap;
> > + pcireg_t id, class, intr, bhlcr;
> > int pin, bus, device, function;
> > - int off, ret = 0;
> > - uint64_t addr;
> > + int ret = 0;
> >
> > pci_decompose_tag(pc, tag, &bus, &device, &function);
> >
> > @@ -486,28 +518,8 @@ pci_probe_device(struct pci_softc *sc, p
> > }
> > pa.pa_intrline = PCI_INTERRUPT_LINE(intr);
> >
> > - if (pci_get_ht_capability(pc, tag, PCI_HT_CAP_MSI, &off, &cap)) {
> > - /*
> > - * XXX Should we enable MSI mapping ourselves on
> > - * systems that have it disabled?
> > - */
> > - if (cap & PCI_HT_MSI_ENABLED) {
> > - if ((cap & PCI_HT_MSI_FIXED) == 0) {
> > - addr = pci_conf_read(pc, tag,
> > - off + PCI_HT_MSI_ADDR);
> > - addr |= (uint64_t)pci_conf_read(pc, tag,
> > - off + PCI_HT_MSI_ADDR_HI32) << 32;
> > - } else
> > - addr = PCI_HT_MSI_FIXED_ADDR;
> > -
> > - /*
> > - * XXX This will fail to enable MSI on systems
> > - * that don't use the canonical address.
> > - */
> > - if (addr == PCI_HT_MSI_FIXED_ADDR)
> > - pa.pa_flags |= PCI_FLAGS_MSI_ENABLED;
> > - }
> > - }
> > + if (pci_device_msi_enabled(pc, tag))
> > + pa.pa_flags |= PCI_FLAGS_MSI_ENABLED;
> >
> > /*
> > * Give the MD code a chance to alter pci_attach_args and/or
> > @@ -1697,6 +1709,9 @@ int
> > pci_intr_msix_count(pci_chipset_tag_t pc, pcitag_t tag)
> > {
> > pcireg_t reg;
> > +
> > + if (pci_device_msi_enabled(pc, tag) == 0)
> > + return (0);
> >
> > if (pci_get_capability(pc, tag, PCI_CAP_MSIX, NULL, ®) == 0)
> > return (0);
>
> Jonathan, thanks for the quick reply. I've never applied a patch
> before but don't mind giving it a shot if you'll bear with me. This
> patch would be for the -current tree, correct? So the process would be
> to get the -current source, apply the patch, then follow the steps to
> compile a new kernel?
It will also apply to 6.9, since the file it touches hasn't changed
since 6.9 was tagged.
