Does this PCI API look good enough for actual use?

Question about which tools to use, bugs, the best way to implement a function, etc should go here. Don't forget to see if your question is answered in the wiki first! When in doubt post here.
User avatar
AaronMiller
Member
Member
Posts: 81
Joined: Thu Mar 06, 2008 1:26 pm
Location: Roseville, California (USA)
Contact:

Does this PCI API look good enough for actual use?

Post by AaronMiller »

I recently read the PCI specification released by the PCI Special Interest Group and decided to write how the API in my OS should look according to that. Below is the code to the header file (not tested). I'm wondering if I might be missing anything or if there are any improvements I could make. If there aren't, I'll proceed with writing the corresponding code to it and releasing it--GPL'd--to the community. :) Thanks in advance for any advice, criticisms, etc.

These will serve as functions I use with the Extensible Driver Interface (EDI) to help support other hobbyist OS'.

Code: Select all

#ifndef DRAGON_OS_PCI_H
#define DRAGON_OS_PCI_H

/* Includes */
#include <kernel/preprocessor.h> /* Contains several macros, such as API, which is __attribute__((stdcall)) */
#include <kernel/types.h> /* Specifies u8, u16, u32, u64, s8, s16, s32, s64, ptr_t, result_t, and bitfield_t */

/* PCI Types */
typedef u8 pci_bus_t; /* 0..255 (8 bits) */
typedef u8 pci_device_t; /* 0..32 (5 bits) */
typedef u8 pci_function_t; /* 0..7 (3 bits) */
typedef u16 pci_pack_t; /* 0..2 = pci_function_t, 3..7 = pci_device_t, 8..15 = pci_bus_t */
/* NOTE: Maybe rename pci_pack_t to pci_path_t (in essence, it is a "path" of sorts) */
typedef u8 pci_register_t;
typedef u8 pci_result_t;

/* PCI Results (pci_result_t) */
#define PCI_SUCCESS 0x00
#define PCI_BIOS_PRESENT 0x00
#define PCI_BIOS_NOT_PRESENT 0x80
#define PCI_DEVICE_NOT_FOUND 0x86
#define PCI_BAD_VENDOR_ID 0x83
#define PCI_FUNCTION_NOT_SUPPORTED 0x81
#define PCI_BAD_REGISTER_NUMBER 0x87

/* Functions */
EXTERN pci_result_t API pciBiosPresent(PCI_BIOS_DESC *pPciBios); /* TODO: Create PCI_BIOS_DESC structure */
EXTERN pci_result_t API pciFindDevice(u16 deviceId, u16 vendorId, u16 index, pci_pack_t *pPack /* out */);
EXTERN pci_result_t API pciFindClassCode(u32 classCode, u16 index, pci_pack_t *pPack /* out */);
EXTERN pci_result_t API pciGenerateSpecialCycle(pci_bus_t bus, u32 data);
EXTERN pci_result_t API pciReadByte(pci_pack_t pack, pci_register_t reg, u8 *pData /* out */);
EXTERN pci_result_t API pciReadWord(pci_pack_t pack, pci_register_t reg, u16 *pData /* out */);
EXTERN pci_result_t API pciReadDword(pci_pack_t pack, pci_register_t reg, u32 *pData /* out */);
EXTERN pci_result_t API pciWriteByte(pci_pack_t pack, pci_register_t reg, u8 data);
EXTERN pci_result_t API pciWriteWord(pci_pack_t pack, pci_register_t reg, u16 data);
EXTERN pci_result_t API pciWriteDword(pci_pack_t pack, pci_register_t reg, u32 data);

#endif
User avatar
NickJohnson
Member
Member
Posts: 1249
Joined: Tue Mar 24, 2009 8:11 pm
Location: Sunnyvale, California

Re: Does this PCI API look good enough for actual use?

Post by NickJohnson »

Why not have a function that reads an entire PCI device configuration space into a structure? I mean, all you have there is the ability to read the configuration space dword by dword (or do a search) - not much more abstract than the port accesses themselves. That would at least allow a driver to cache its PCI config information and access it through named elements, instead of pulling information out piece by piece.
User avatar
AaronMiller
Member
Member
Posts: 81
Joined: Thu Mar 06, 2008 1:26 pm
Location: Roseville, California (USA)
Contact:

Re: Does this PCI API look good enough for actual use?

Post by AaronMiller »

You do have a point there. Perhaps the creation of several structures and the use of a linked list may help.

Perhaps something like this?

Code: Select all

/* TODO: Create PCI_DEVICE structure. */

/* Additional Functions */
EXTERN PCI_DEVICE *API pciGetFirstDevice(void);
EXTERN PCI_DEVICE *API pciGetLastDevice(void);
EXTERN PCI_DEVICE *API pciGetNextDevice(PCI_DEVICE *pDevice);
EXTERN PCI_DEVICE *API pciGetPreviousDevice(PCI_DEVICE *pDevice);
/* Perhaps make the above two a set of macros. */
EXTERN PCI_DEVICE *API pciGetDevice(const char *pszName);

/* Configuration Address Functions */
EXTERN pci_result_t API pciReadAddress(pci_pack_t pack, ptr_t offset, const void *pBuffer /* out */, size_t length);
EXTERN pci_result_t API pciWriteAddress(pci_pack_t pack, ptr_t offset, const void *pBuffer, size_t length);
User avatar
Combuster
Member
Member
Posts: 9301
Joined: Wed Oct 18, 2006 3:45 am
Libera.chat IRC: [com]buster
Location: On the balcony, where I can actually keep 1½m distance
Contact:

Re: Does this PCI API look good enough for actual use?

Post by Combuster »

Caching PCI information is a Bad Idea (tm). There are several side effects to reading and writing, and the configuration can change without modifying any PCI registers - Ever tried writing a chipset driver?
"Certainly avoid yourself. He is a newbie and might not realize it. You'll hate his code deeply a few years down the road." - Sortie
[ My OS ] [ VDisk/SFS ]
pcmattman
Member
Member
Posts: 2566
Joined: Sun Jan 14, 2007 9:15 pm
Libera.chat IRC: miselin
Location: Sydney, Australia (I come from a land down under!)
Contact:

Re: Does this PCI API look good enough for actual use?

Post by pcmattman »

Combuster wrote:Caching PCI information is a Bad Idea (tm). There are several side effects to reading and writing...
Perhaps a write-through cache, and the ability to request that a function call bypasses the cache altogether if it is important that cache isn't used? It may overcomplicate the API, however, and also may end up being pointless (with everyone bypassing the cache for every little thing).
User avatar
Love4Boobies
Member
Member
Posts: 2111
Joined: Fri Mar 07, 2008 5:36 pm
Location: Bucharest, Romania

Re: Does this PCI API look good enough for actual use?

Post by Love4Boobies »

This may be a shameless plug but I just wanted to point out that UDI (Uniform Driver Interface) already has a PCI bus binding. :D
"Computers in the future may weigh no more than 1.5 tons.", Popular Mechanics (1949)
[ Project UDI ]
User avatar
Creature
Member
Member
Posts: 548
Joined: Sat Dec 27, 2008 2:34 pm
Location: Belgium

Re: Does this PCI API look good enough for actual use?

Post by Creature »

Or you could just cache the ports, functions, etc. that are available (or where you find a card or something). Then you'd just have a list of 'numbers' where reading and writing still needs to go directly.
When the chance of succeeding is 99%, there is still a 50% chance of that success happening.
User avatar
Combuster
Member
Member
Posts: 9301
Joined: Wed Oct 18, 2006 3:45 am
Libera.chat IRC: [com]buster
Location: On the balcony, where I can actually keep 1½m distance
Contact:

Re: Does this PCI API look good enough for actual use?

Post by Combuster »

Notes from my intel driver (if you want proof, read the docs for the 82915 GMCH and related)
Device:vendor can change outside of the PCI driver's control
class codes can change outside of the PCI driver's control
BARs can change outside of the PCI driver's control.

And in general, several hardware devices have an alias for PCI configuration registers in their MMIO space.

Now, what can you cache? The only one that knows the details is the driver for the device. If it can and wants to use caching, it'll do that itself. That even saves on IPC calls.
"Certainly avoid yourself. He is a newbie and might not realize it. You'll hate his code deeply a few years down the road." - Sortie
[ My OS ] [ VDisk/SFS ]
User avatar
Brendan
Member
Member
Posts: 8561
Joined: Sat Jan 15, 2005 12:00 am
Location: At his keyboard!
Contact:

Re: Does this PCI API look good enough for actual use?

Post by Brendan »

Hi,
Combuster wrote:Notes from my intel driver (if you want proof, read the docs for the 82915 GMCH and related)
Device:vendor can change outside of the PCI driver's control
class codes can change outside of the PCI driver's control
BARs can change outside of the PCI driver's control.
Which docs? I checked Intel's "815EM Chipset: 82815EM Graphics and Memory Controller Hub (GMCH2-M)" datasheet and found nothing unusual. The only thing close is the ability to enable/disable the entire graphics controller, but that's not something that the BIOS will change while the OS is running, and not something an OS/kernel would change while the driver is running. Of course in theory it might be possible for the BIOS or OS to emulate "hot-plug PCI" for the onboard video card, but that'd be too insane to seriously contemplate (it lacks any practical advantage/motive).

Of course I'm not suggesting that caching PCI configuration space is a good idea - I have an entirely different perspective: the OS shouldn't let the driver access the device's configuration space to begin with (and the driver shouldn't need to access the device's configuration space for any reason). I guess what I'm saying is that (IMHO) the perfect PCI API for device drivers is "no PCI API needed".


Cheers,

Brendan
For all things; perfection is, and will always remain, impossible to achieve in practice. However; by striving for perfection we create things that are as perfect as practically possible. Let the pursuit of perfection be our guide.
User avatar
Combuster
Member
Member
Posts: 9301
Joined: Wed Oct 18, 2006 3:45 am
Libera.chat IRC: [com]buster
Location: On the balcony, where I can actually keep 1½m distance
Contact:

Re: Does this PCI API look good enough for actual use?

Post by Combuster »

I was indeed referring to the fact that you can enable/disable the integrated graphics controller. Especially useful when you want to multihead or add extra computational resources (i.e.: GPU). If you'd cache the device:vendor, then how ever well you try to enable a bios-disabled controller, you won't see it since it was previously off. Of course, with hotplug PCI you'll have even less certainty about what whole configuration spaces appear or disappear.

Similarly, you can change whether the controller emulates VGA and listens to VGA accesses. Doing so will change the class code from vga compatible to incompatible. If you can't find that, try the 915/945 chipset docs rather than the 815 :wink:

Also, some real hardware has configuration options only available from configuration space (f.x. the 3dfx voodoo 1), without mmio equivalents. That means you can't in general deny drivers access to their configuration space.
"Certainly avoid yourself. He is a newbie and might not realize it. You'll hate his code deeply a few years down the road." - Sortie
[ My OS ] [ VDisk/SFS ]
User avatar
Brendan
Member
Member
Posts: 8561
Joined: Sat Jan 15, 2005 12:00 am
Location: At his keyboard!
Contact:

Re: Does this PCI API look good enough for actual use?

Post by Brendan »

Hi,
Combuster wrote:I was indeed referring to the fact that you can enable/disable the integrated graphics controller. Especially useful when you want to multihead or add extra computational resources (i.e.: GPU). If you'd cache the device:vendor, then how ever well you try to enable a bios-disabled controller, you won't see it since it was previously off. Of course, with hotplug PCI you'll have even less certainty about what whole configuration spaces appear or disappear.
In this case (for enabling/disabling devices and for "hot-plug PCI"), caching wouldn't be a problem. When the device is disabled/removed the OS knows and the OS can terminate the driver and flush the cache for that device, and when the device is enabled/inserted the OS knows and the OS can begin caching that device's PCI configuration space again and start a driver. It really isn't much different to caching removeable media (e.g. USB storage device "blocks"), and something like caching floppy sectors might be harder as there's no (reliable) "disk changed" notification.
Combuster wrote:Similarly, you can change whether the controller emulates VGA and listens to VGA accesses. Doing so will change the class code from vga compatible to incompatible. If you can't find that, try the 915/945 chipset docs rather than the 815 :wink:
I think that applies to all modern video controllers (e.g. PCI or later video controllers that support systems with multiple video controllers). When it's the primary video controller the BIOS sets it up so that it listens to VGA I/O ports, etc (for backward compatibility) and when the video controller is the second (or third, fourth, etc) video controller then it doesn't listen to VGA I/O ports, etc (to avoid conflicts with the primary video controller). This also applies to disk controllers, where there's a "legacy mode" (controller uses "ISA ATA/IDE controller" I/O ports and IRQs) and a "native mode". I'd probably put all devices into "native, non-legacy" mode as soon as possible, including the primary video controller (e.g. during boot, while you're assigning resources to devices and before any device drivers are started), so that none of the device drivers need to worry about what mode the device is in (and so the device driver doesn't need one set of routines for "legacy mode" plus another set of routines for "native mode").
Combuster wrote:Also, some real hardware has configuration options only available from configuration space (f.x. the 3dfx voodoo 1), without mmio equivalents. That means you can't in general deny drivers access to their configuration space.
In general you can still deny drivers access to configuration space, but this would mean you'd need to provide work-arounds for "less sane" devices.

I did a little bit of checking. Old Voodoo/3DFX video controllers would need workarounds. Old Oak, S3 and Tseng Labs video controllers are fine. Newer VIA, Intel and ATI video controllers are fine too. I couldn't find decent information for other video controllers.

Maybe it'd be better/easier to deny access to standard PCI configuration space registers - e.g. registers 0x000 to 0x03F, plus any "Capabilities" in the linked list starting at the "Capabilities Pointer" at offset 0x034 (if supported), plus any standard PCI-E registers (if it's a PCI-E device). That's "everything" in most cases anyway.


Cheers,

Brendan
For all things; perfection is, and will always remain, impossible to achieve in practice. However; by striving for perfection we create things that are as perfect as practically possible. Let the pursuit of perfection be our guide.
User avatar
Combuster
Member
Member
Posts: 9301
Joined: Wed Oct 18, 2006 3:45 am
Libera.chat IRC: [com]buster
Location: On the balcony, where I can actually keep 1½m distance
Contact:

Re: Does this PCI API look good enough for actual use?

Post by Combuster »

Brendan wrote:I did a little bit of checking. Old Voodoo/3DFX video controllers would need workarounds. Old Oak, S3 and Tseng Labs video controllers are fine. Newer VIA, Intel and ATI video controllers are fine too. I couldn't find decent information for other video controllers.
S3 needs workarounds for using ISA style sparse memory maps. Intel abuses a BAR for something that isn't one (7.2.11 GTTMMADR — Graphics Translation Table Range Address), which may confuse your PCI code by (re)mapping it into non-present memory when it should actually point to valid RAM. ATI's Mach64s & Radeons, and Verites make sense on the PCI level. I haven't poked around the OTI card I have so I'll take your word on it for now; I should dig through VGADoc4b once again to pick out all potential PCI devices that use nonstandard hardcoded addresses, which could screw over some other devices. IIRC the S3 wasn't the only culprit in that regard...

I also don't know any other devices that change class code the moment you disable the "VGA register emulation", as far as documentation goes. I should test that hypothesis on my Verite - I expect it isn't stupid/intelligent enough to change class codes, as it still can do VGA.

Bottom line, I don't try to pretend hardware to be "nice", then add workarounds. I expect them to disbehave and take any good behavior with gratitude. :wink:
"Certainly avoid yourself. He is a newbie and might not realize it. You'll hate his code deeply a few years down the road." - Sortie
[ My OS ] [ VDisk/SFS ]
User avatar
Owen
Member
Member
Posts: 1700
Joined: Fri Jun 13, 2008 3:21 pm
Location: Cambridge, United Kingdom
Contact:

Re: Does this PCI API look good enough for actual use?

Post by Owen »

Whats the reason to prevent the driver from accessing the device's config space?
User avatar
Brynet-Inc
Member
Member
Posts: 2426
Joined: Tue Oct 17, 2006 9:29 pm
Libera.chat IRC: brynet
Location: Canada
Contact:

Re: Does this PCI API look good enough for actual use?

Post by Brynet-Inc »

Brendan wrote:This also applies to disk controllers, where there's a "legacy mode" (controller uses "ISA ATA/IDE controller" I/O ports and IRQs) and a "native mode". I'd probably put all devices into "native, non-legacy" mode as soon as possible, including the primary video controller (e.g. during boot, while you're assigning resources to devices and before any device drivers are started), so that none of the device drivers need to worry about what mode the device is in (and so the device driver doesn't need one set of routines for "legacy mode" plus another set of routines for "native mode").
Perhaps a little off topic, but not all PCI IDE/ATA controllers support "native" mode.. none of the integrated Intel ICH devices do, for instance.
Image
Twitter: @canadianbryan. Award by smcerm, I stole it. Original was larger.
User avatar
Brendan
Member
Member
Posts: 8561
Joined: Sat Jan 15, 2005 12:00 am
Location: At his keyboard!
Contact:

Re: Does this PCI API look good enough for actual use?

Post by Brendan »

Hi,
Combuster wrote:
Brendan wrote:I did a little bit of checking. Old Voodoo/3DFX video controllers would need workarounds. Old Oak, S3 and Tseng Labs video controllers are fine. Newer VIA, Intel and ATI video controllers are fine too. I couldn't find decent information for other video controllers.
S3 needs workarounds for using ISA style sparse memory maps. Intel abuses a BAR for something that isn't one (7.2.11 GTTMMADR — Graphics Translation Table Range Address), which may confuse your PCI code by (re)mapping it into non-present memory when it should actually point to valid RAM. ATI's Mach64s & Radeons, and Verites make sense on the PCI level. I haven't poked around the OTI card I have so I'll take your word on it for now; I should dig through VGADoc4b once again to pick out all potential PCI devices that use nonstandard hardcoded addresses, which could screw over some other devices. IIRC the S3 wasn't the only culprit in that regard...
For onboard video cards that use system RAM instead of dedicated video RAM, the RAM must be "stolen" somehow. Normally the BIOS sets it up, and I assume that the OS and the device driver aren't meant to change the BARs after that (in the same way that the OS and device drivers aren't really meant to mess with the memory controller's configuration).

For video cards, there was a "transition period" where video card manufacturers tried to adapt their existing (ISA/VLB) cards so that they worked on a PCI bus (instead of redesigning the card to suit PCI). In a lot of cases the end results was a hybrid mess. For example, the existing ISA/VLB card may have already had I/O ports that control where the device is mapped into the physical address space, and rather than redesigning the video controller the manufacturer may have left it "as is" (with no BAR) or used the BAR as a mirror of the existing configuration register (so you could use the BAR or the I/O port). As a side-effect, most of these video cards are incapable of being used as the second (or third, fourth, etc) video card and must be the primary video card. For me, these devices will probably become a huge problem (but fortunately they're all obsolete now anyway).
Combuster wrote:Bottom line, I don't try to pretend hardware to be "nice", then add workarounds. I expect them to disbehave and take any good behavior with gratitude. :wink:
Heh - that's where we differ: I expect device drivers to misbehave, and I'm willing to implement any workarounds necessary to create the illusion that hardware is nice (even when hardware isn't).
Owen wrote:Whats the reason to prevent the driver from accessing the device's config space?
The main reason is security (e.g. the principle of least privilege). I don't trust device drivers if I don't have to (as requiring trusted device drivers leads to things like "open source drivers only" and/or ineffective device driver certification programs), and want to make sure that no (third party, source unseen, potentially malicious) device driver is able to complete screw up the physical address space.

Here's a nice little graph (from arstechnica.com) showing how reliable device drivers are in Vista:
Image

That's fairly shocking when you consider how important the Vista/Windows market is to hardware companies (and Linux isn't much different - device drivers are probably the leading cause of instability for all popular OSs). If my OS is 100 times less important than Vista/Windows, then I could probably expect that any third party device drivers will be 100 times more dodgy. I have to assume that third party device drivers for my OS will be crap, and therefore I have no choice but to (attempt to) protect the rest of the system from these device drivers (which includes preventing them from messing with BARs, changing MSI configuration, etc).


Cheers,

Brendan
For all things; perfection is, and will always remain, impossible to achieve in practice. However; by striving for perfection we create things that are as perfect as practically possible. Let the pursuit of perfection be our guide.
Post Reply