Subject: Re: access to external proms for PCI
To: Johan Danielsson <joda@pdc.kth.se>
From: Garrett D'Amore <garrett_damore@tadpole.com>
List: tech-kern
Date: 12/16/2005 21:13:53
I've been looking at this a lot more closely, and it looks the problem
is a lot simpler.

NetBSD already has a pci_mapreg_map() that *should* be able to map up
expansion ROMs.

The problem is that the PCI configuration code (which not all platforms
use) by default does not assign any mapping, and does not even reserve
address space for expansion ROMs.

Based upon my read, the authors of the pci_configure_bus() code assumed
we would want one of two scenarios:

    * all ROMs mapped and enabled
    * no access to ROMs at all, ever.

The first choice is a really bad idea, and will probably break some
devices.  (Apparently some device drivers are "aware" of this problem
and explicitly disable the ROM address decoder.)  Some broken BIOS may
act this way by default.

The 2nd choice is the default.

There is an override mechanism, but it is done per platform, rather than
per device (pci_conf_mode_hook).  The only example I could find that
actually did anything other disable all ROM mappings was
src/sys/arch/evbppc/ev64260/gt_mainbus.c, and I suspect this was an error.

There is a 3rd behavior, which I think is the *correct* behavior, and
what I would like to do is change the PCI configuration code to do this
by default:

    * ROMs disabled by default, but left with address space assigned
(i.e. the addresses are assigned, but the decoder is left disabled).

I cannot think of any reason not to assign address space to expansion
ROMs.  These things are usually quite small (usually absent, actually),
and the virtual address space they would consume would be miniscule, I'd
think.

So, to get this "correct" behavior, I'd change the code so that

PCI_CONF_MAP_ROM means to actually *map* the ROMs.   (I.e. no behavior
change.)

Having this bit clear would still assign address space, but would not
enable the decoder.

If someone feels there is a case for leaving the ROM mapped and enabled,
then I can create a new bit, say PCI_CONF_ENABLE_ROM, that actually
enables the ROM.

I'd probably also want to create a PCI_CONF_DEFAULT to replace
PCI_CONF_ALL, that is the normal, reasonable default values (i.e.
leaving off those bits that are likely to be problematic, such as
PCI_CONF_ENABLE_ROM and PCI_CONF_MAP_ROM.)

Then for the cases like my Radeon driver, which need to access expansion
ROM, I can just use pci_mapreg_map() with a type of PCI_MAPREG_TYPE_ROM.

Does ths change seem sane?  I think it is a lot better.  I'll file a PR
and diffs if folks generally agree with it.

-- 
Garrett D'Amore                          http://www.tadpolecomputer.com/
Sr. Staff Engineer          Extending the Power of 64-bit UNIX Computing
Tadpole Computer, Inc.                             Phone: (951) 325-2134