Subject: Re: PCI_MAPREG_TYPE_ROM... or the lack of it
To: Garrett D'Amore <garrett_damore@tadpole.com>
From: Garrett D'Amore <garrett_damore@tadpole.com>
List: tech-kern
Date: 02/26/2006 21:59:22
This is a multi-part message in MIME format.
--------------020205010900080109040308
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Forgot to attach the diffs. Sorry about that.
Garrett D'Amore wrote:
> Garrett D'Amore wrote:
>
>> Allen Briggs wrote:
>>
>>
>>> On Fri, Feb 24, 2006 at 07:43:01PM -0800, Garrett D'Amore wrote:
>>>
>>>
>>>
>>>> Actually, it occurs to me that there is an easier way:
>>>>
>>>> PCI_MAPREG_TYPE_ROM could be defined the same as PCI_MAPREG_TYPE_MEM,
>>>> and the PCI register mapping code could look at the offset of the register.
>>>>
>>>> PCI_MAPREG_ROM is *always* 0x30. That's probably a better way to do it.
>>>>
>>>>
>>>>
>>> I agree. I think it should be noted in the man page that
>>> PCI_MAPREG_TYPE_ROM is only valid for PCI_MAPREG_ROM.
>>>
>>> -allen
>>>
>>>
>>>
>>>
>> Okay, I'm going to take that as the "official" word then. Expect diffs
>> early next week. :-)
>>
>> - Garrett
>>
>>
>>
> My diffs are attached, please review. These diffs add support for
> PCI_MAPREG_TYPE_ROM as an argument to pci_mapreg_map().
>
> A consequence of these changes is that now pci_configure_bus() properly
> setups up the PCI expansion ROM BAR at 0x30, whereas before it still
> wasn't getting it right (due to an incorrect check against
> PCI_CONF_ENABLE_ROM that should only have been PCI_CONF_MAP_ROM.
>
> This means that on *most* systems that use PCI_NETBSD_CONFIGURE, you
> will now see Expansion ROMs get allocated address space *and* have that
> address written to the ROM BAR, but it will *not* be enabled.
>
> pci_mapreg_map() will *enable* the expansion rom address decoder if not
> already done. As this can have detrimental effects for some devices, it
> should only be done in target device drivers that really need this
> access to ROMs. Note that there is *no* standard API for *disabling* an
> enabled ROM address decoder. If you need to do that, just turn off the
> enable bit in the ROM address decoder. (This will not normally be a
> problem, since those devices with drivers that need an expansion rom
> usually will have a separate address decoder for the expansion rom.)
>
> If none of this makes any sense, rest assured that your devices/drivers
> will continue to operate as before.
>
> These diffs do not include the necessary changes to the man pages
> (primarily to indicate that PCI_MAPREG_TYPE_ROM is only allowed when reg
> is PCI_MAPREG_ROM), but I'll do that as well, assuming that the
> technical content of these diffs is acceptable.
>
> -- Garrett
>
>
--
Garrett D'Amore, Principal Software Engineer
Tadpole Computer / Computing Technologies Division,
General Dynamics C4 Systems
http://www.tadpolecomputer.com/
Phone: 951 325-2134 Fax: 951 325-2191
--------------020205010900080109040308
Content-Type: text/plain;
name="diff.exrom"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="diff.exrom"
Index: pci_map.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/pci_map.c,v
retrieving revision 1.14
diff -u -r1.14 pci_map.c
--- pci_map.c 11 Dec 2005 12:22:50 -0000 1.14
+++ pci_map.c 27 Feb 2006 05:05:07 -0000
@@ -111,20 +111,21 @@
{
pcireg_t address, mask, address1 = 0, mask1 = 0xffffffff;
u_int64_t waddress, wmask;
- int s, is64bit;
+ int s, is64bit, isrom;
is64bit = (PCI_MAPREG_MEM_TYPE(type) == PCI_MAPREG_MEM_TYPE_64BIT);
+ isrom = reg == PCI_MAPREG_ROM;
- if (reg < PCI_MAPREG_START ||
+ if ((!isrom) && (reg < PCI_MAPREG_START ||
#if 0
/*
* Can't do this check; some devices have mapping registers
* way out in left field.
*/
- reg >= PCI_MAPREG_END ||
+ reg >= PCI_MAPREG_END ||
#endif
- (reg & 3))
- panic("pci_mem_find: bad request");
+ (reg & 3)))
+ panic("pci_mem_find: bad request");
if (is64bit && (reg + 4) >= PCI_MAPREG_END)
panic("pci_mem_find: bad 64-bit request");
@@ -152,15 +153,24 @@
}
splx(s);
- if (PCI_MAPREG_TYPE(address) != PCI_MAPREG_TYPE_MEM) {
- printf("pci_mem_find: expected type mem, found i/o\n");
- return (1);
- }
- if (PCI_MAPREG_MEM_TYPE(address) != PCI_MAPREG_MEM_TYPE(type)) {
- printf("pci_mem_find: expected mem type %08x, found %08x\n",
- PCI_MAPREG_MEM_TYPE(type),
- PCI_MAPREG_MEM_TYPE(address));
- return (1);
+ if (!isrom) {
+ /*
+ * roms should have an enable bit instead of a memory
+ * type decoder bit. For normal BARs, make sure that
+ * the address decoder type matches what we asked for.
+ */
+ if (PCI_MAPREG_TYPE(address) != PCI_MAPREG_TYPE_MEM) {
+ printf("pci_mem_find: expected type mem, found i/o\n");
+ return (1);
+ }
+ if (PCI_MAPREG_MEM_TYPE(address) !=
+ PCI_MAPREG_MEM_TYPE(type)) {
+ printf("pci_mem_find: "
+ "expected mem type %08x, found %08x\n",
+ PCI_MAPREG_MEM_TYPE(type),
+ PCI_MAPREG_MEM_TYPE(address));
+ return (1);
+ }
}
waddress = (u_int64_t)address1 << 32UL | address;
@@ -208,7 +218,7 @@
*sizep = PCI_MAPREG_MEM64_SIZE(wmask);
}
if (flagsp != 0)
- *flagsp = PCI_MAPREG_MEM_PREFETCHABLE(address) ?
+ *flagsp = (isrom || PCI_MAPREG_MEM_PREFETCHABLE(address)) ?
BUS_SPACE_MAP_PREFETCHABLE : 0;
return (0);
@@ -287,6 +297,17 @@
tag = pa->pa_memt;
}
+ if (reg == PCI_MAPREG_ROM) {
+ pcireg_t mask;
+ int s;
+ /* we have to enable the ROM address decoder... */
+ s = splhigh();
+ mask = pci_conf_read(pa->pa_pc, pa->pa_tag, reg);
+ mask |= PCI_MAPREG_ROM_ENABLE;
+ pci_conf_write(pa->pa_pc, pa->pa_tag, reg, mask);
+ splx(s);
+ }
+
if (bus_space_map(tag, base, size, busflags | flags, &handle))
return (1);
Index: pciconf.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/pciconf.c,v
retrieving revision 1.28
diff -u -r1.28 pciconf.c
--- pciconf.c 10 Feb 2006 20:52:57 -0000 1.28
+++ pciconf.c 27 Feb 2006 05:05:07 -0000
@@ -808,7 +808,7 @@
for (pm=pb->pcimemwin; pm < &pb->pcimemwin[pb->nmemwin] ; pm++) {
if (pm->reg == PCI_MAPREG_ROM && pm->address != -1) {
pd = pm->dev;
- if (!(pd->enable & PCI_CONF_ENABLE_ROM))
+ if (!(pd->enable & PCI_CONF_MAP_ROM))
continue;
if (pci_conf_debug) {
print_tag(pd->pc, pd->tag);
@@ -817,7 +817,10 @@
PRIx64 " (reg %x)\n", pm->size,
pm->address, pm->reg);
}
- base = (pcireg_t) (pm->address | PCI_MAPREG_ROM_ENABLE);
+ base = (pcireg_t) pm->address;
+ if (pd->enable & PCI_CONF_ENABLE_ROM)
+ base |= PCI_MAPREG_ROM_ENABLE;
+
pci_conf_write(pd->pc, pd->tag, pm->reg, base);
}
}
Index: pcireg.h
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/pcireg.h,v
retrieving revision 1.47
diff -u -r1.47 pcireg.h
--- pcireg.h 11 Dec 2005 12:22:50 -0000 1.47
+++ pcireg.h 27 Feb 2006 05:05:07 -0000
@@ -352,6 +352,7 @@
#define PCI_MAPREG_TYPE_MASK 0x00000001
#define PCI_MAPREG_TYPE_MEM 0x00000000
+#define PCI_MAPREG_TYPE_ROM 0x00000000
#define PCI_MAPREG_TYPE_IO 0x00000001
#define PCI_MAPREG_ROM_ENABLE 0x00000001
--------------020205010900080109040308--