--k3qmt+ucFURmlhDS
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Our handling of the PCI Memory Write and Invalidate command is
suboptimal.

We currently have flags that are passed down to the device from
machdep PCI code which says that MWI is okay to use.  However,
in order to use it, it must be enabled in the PCI Command register
for that device, and on some devices, in another device-specific
register.

I'm wondering if we're missing out on the opportunity to use MWI on
a lot of devices that support it.  The PCI spec says that if a device
does not support MWI, it must hard-wire the MWI bit in the PCI CSR
to 0, which means it's probably safe to unconditionally set that bit
in generic PCI code.  Another option is to make sure it is always clear
and place responsibility on individual device drivers for setting it.

Attached is a sample patch for the latter approach, but I think I'd
prefer the former, since it means we'd get MWI on more devices with
fewer driver modifications.  But I'd like a sanity check on whether
or not doing that would be safe.

Thoughts?

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

--k3qmt+ucFURmlhDS
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=pci-patch

Index: if_fxp_pci.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/pci/if_fxp_pci.c,v
retrieving revision 1.23
diff -c -r1.23 if_fxp_pci.c
*** if_fxp_pci.c	2002/06/22 08:09:07	1.23
--- if_fxp_pci.c	2002/06/28 21:05:14
***************
*** 319,326 ****
  			/*
  			 * Enable the MWI command for memory writes.
  			 */
! 			if (pa->pa_flags & PCI_FLAGS_MWI_OKAY)
  				sc->sc_flags |= FXPF_MWI;
  		}
  		if (sc->sc_rev >= FXP_REV_82559_A0)
  			chipname = "i82559 Ethernet";
--- 319,335 ----
  			/*
  			 * Enable the MWI command for memory writes.
  			 */
! 			if (pa->pa_flags & PCI_FLAGS_MWI_OKAY) {
! 				pcireg_t reg;
! 
  				sc->sc_flags |= FXPF_MWI;
+ 
+ 				reg = pci_conf_read(pa->pa_pc, pa->pa_tag,
+ 				    PCI_COMMAND_STATUS_REG);
+ 				pci_conf_write(pa->pa_pc, pa->pa_tag,
+ 				    PCI_COMMAND_STATUS_REG,
+ 				    reg | PCI_COMMAND_INVALIDATE_ENABLE);
+ 			}
  		}
  		if (sc->sc_rev >= FXP_REV_82559_A0)
  			chipname = "i82559 Ethernet";
Index: pci.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/pci/pci.c,v
retrieving revision 1.67
diff -c -r1.67 pci.c
*** pci.c	2002/06/20 23:02:06	1.67
--- pci.c	2002/06/28 21:05:19
***************
*** 321,326 ****
--- 321,334 ----
  		if (ret != 0 && pap != NULL)
  			*pap = pa;
  	} else {
+ 		/*
+ 		 * Make sure the MWI-Enable bit is clear in the
+ 		 * PCI Command register.  Devices that wish to
+ 		 * use MWI are responsible for setting the bit
+ 		 * themselves.
+ 		 */
+ 		pci_conf_write(pc, tag, PCI_COMMAND_STATUS_REG,
+ 		    csr & ~PCI_COMMAND_INVALIDATE_ENABLE);
  		ret = config_found_sm(&sc->sc_dev, &pa, pciprint,
  		    pcisubmatch) != NULL;
  	}

--k3qmt+ucFURmlhDS--