From e64f3dee49fac728bc39e2e39e5fbe9e07e4efcb Mon Sep 17 00:00:00 2001 From: Warner Losh Date: Mon, 8 Jul 2019 19:38:49 +0000 Subject: [PATCH] Work around devices which return all zeros for reads of existing MSI-X table VCTRL registers. Unconditionally program the MSI-X vector control Mask field for MSI-X table entries without regarud for Mask's previous value. Some devices return all zeros on reads of the VCTRL registers, which would cause us to skip disabling interrupts. This fixes the Samsung SM961/PM961 SSDs which are return zero starting from offset 0x3084 within the memory region specified by BAR0, even when they are active MSI-X vectors. The Illumos kernel writes these unconditionally to 0 or 1. However, section 6.8.2.9 of the PCI Local Bus 3.0 spec (dated Feb 3, 2004) states for bits 31::01: After reset, the state of these bits must be 0. However, for potential future use, software must preserve the value of these reserved bits when modifying the value of other Vector Control bits. If software modifies the value of these reserved bits, the result is undefined." so we always set or clear the Mask bit, but otherwise preserves the old value. PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=211713 Reviewed By: imp, jhb Submitted by: Ka Ho Ng MFC After: 1 week Differential Revision: https://reviews.freebsd.org/D20873 --- sys/dev/pci/pci.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c index 6b54b3db2d1..0297808e4ac 100644 --- a/sys/dev/pci/pci.c +++ b/sys/dev/pci/pci.c @@ -1671,10 +1671,13 @@ pci_mask_msix(device_t dev, u_int index) KASSERT(msix->msix_msgnum > index, ("bogus index")); offset = msix->msix_table_offset + index * 16 + 12; val = bus_read_4(msix->msix_table_res, offset); - if (!(val & PCIM_MSIX_VCTRL_MASK)) { - val |= PCIM_MSIX_VCTRL_MASK; - bus_write_4(msix->msix_table_res, offset, val); - } + val |= PCIM_MSIX_VCTRL_MASK; + + /* + * Some devices (e.g. Samsung PM961) do not support reads of this + * register, so always write the new value. + */ + bus_write_4(msix->msix_table_res, offset, val); } void @@ -1687,10 +1690,13 @@ pci_unmask_msix(device_t dev, u_int index) KASSERT(msix->msix_table_len > index, ("bogus index")); offset = msix->msix_table_offset + index * 16 + 12; val = bus_read_4(msix->msix_table_res, offset); - if (val & PCIM_MSIX_VCTRL_MASK) { - val &= ~PCIM_MSIX_VCTRL_MASK; - bus_write_4(msix->msix_table_res, offset, val); - } + val &= ~PCIM_MSIX_VCTRL_MASK; + + /* + * Some devices (e.g. Samsung PM961) do not support reads of this + * register, so always write the new value. + */ + bus_write_4(msix->msix_table_res, offset, val); } int