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?
