Re: CVS commit: src/sys/dev/pci

["Jonathan A. Kollasch" <jakllsch%kollasch.net@localhost>]

On Wed, May 16, 2018 at 01:45:36PM -0700, Jason Thorpe wrote:
> 
> 
> > On May 16, 2018, at 1:07 PM, Jonathan A. Kollasch <jakllsch%kollasch.net@localhost> wrote:
> > 
> > I'm a bit uneasy about it myself for that same reason.  However, we
> > do not to my knowledge have the infrastructure available to do a
> > complete validation of the resource assignment.  If we did, we'd be
> > able to do hot attach of PCIe ExpressCard with just a little more work.
> 
> 
> We used to have something like this to support CardBus way back in the day, but I will admit I wasn’t entirely happy with it at the time.

rbus?  That's still around, and it's still ugly and doesn't always work.

> I guess I’m frustrated that UEFI isn’t taking care of this.  On the other hand, perhaps UEFI isn’t doing it because they assume that the BARs have to be reprogrammed if power is removed from the device?

Likewise.  I have no idea what their reasoning is.

> What does FreeBSD do?

I'm not sure I'm reading it correctly but..

FreeBSD appears to have a enabled-by-default and kernel-load-time-adjustable
knob for enabling the decodes in the CSR.  (hw.pci.enable_io_modes, AKA
static int pci_enable_io_modes in dev/pci/pci.c)

They also have pci_activate_resource().  But I'm not sure what calls it
and what exactly it does for which devices.


Does the attached patch look to be an improvement?  Admittedly this
added knob would be of the `boot -d`/`gdb -write` sort, but it'd be
there.

Perhaps another way to do this would be to leave this knob disabled,
but turn it on when boot on a UEFI/x86 machine is detected.

	Jonathan Kollasch

Index: sys/dev/pci/pci_map.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/pci_map.c,v
retrieving revision 1.34
diff -d -u -a -p -r1.34 pci_map.c
--- sys/dev/pci/pci_map.c	16 May 2018 19:02:00 -0000	1.34
+++ sys/dev/pci/pci_map.c	17 May 2018 16:50:39 -0000
@@ -43,6 +43,8 @@ __KERNEL_RCSID(0, "$NetBSD: pci_map.c,v 
 #include <dev/pci/pcireg.h>
 #include <dev/pci/pcivar.h>
 
+int pci_enable_decode = 1;
+
 static int
 pci_io_find(pci_chipset_tag_t pc, pcitag_t tag, int reg, pcireg_t type,
     bus_addr_t *basep, bus_size_t *sizep, int *flagsp)
@@ -293,11 +295,6 @@ pci_mapreg_submap(const struct pci_attac
 	if (PCI_MAPREG_TYPE(type) == PCI_MAPREG_TYPE_IO) {
 		if ((pa->pa_flags & PCI_FLAGS_IO_OKAY) == 0)
 			return 1;
-		s = splhigh();
-		csr = pci_conf_read(pa->pa_pc, pa->pa_tag, PCI_COMMAND_STATUS_REG);
-		csr |= PCI_COMMAND_IO_ENABLE;
-		pci_conf_write(pa->pa_pc, pa->pa_tag, PCI_COMMAND_STATUS_REG, csr);
-		splx(s);
 		if (pci_io_find(pa->pa_pc, pa->pa_tag, reg, type, &base,
 		    &realmaxsize, &flags))
 			return 1;
@@ -305,11 +302,6 @@ pci_mapreg_submap(const struct pci_attac
 	} else {
 		if ((pa->pa_flags & PCI_FLAGS_MEM_OKAY) == 0)
 			return 1;
-		s = splhigh();
-		csr = pci_conf_read(pa->pa_pc, pa->pa_tag, PCI_COMMAND_STATUS_REG);
-		csr |= PCI_COMMAND_MEM_ENABLE;
-		pci_conf_write(pa->pa_pc, pa->pa_tag, PCI_COMMAND_STATUS_REG, csr);
-		splx(s);
 		if (pci_mem_find(pa->pa_pc, pa->pa_tag, reg, type, &base,
 		    &realmaxsize, &flags))
 			return 1;
@@ -339,6 +331,15 @@ pci_mapreg_submap(const struct pci_attac
 	if (bus_space_map(tag, base, reqsize, busflags | flags, &handle))
 		return 1;
 
+	if (pci_enable_decode) {
+		s = splhigh();
+		csr = pci_conf_read(pa->pa_pc, pa->pa_tag, PCI_COMMAND_STATUS_REG);
+		csr |= (PCI_MAPREG_TYPE(type) == PCI_MAPREG_TYPE_IO) ?
+		    PCI_COMMAND_IO_ENABLE : PCI_COMMAND_MEM_ENABLE;
+		pci_conf_write(pa->pa_pc, pa->pa_tag, PCI_COMMAND_STATUS_REG, csr);
+		splx(s);
+	}
+
 	if (tagp != NULL)
 		*tagp = tag;
 	if (handlep != NULL)