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.
static uint32_t pci_config_read(uint32_t bus, uint32_t device, uint32_t func, uint32_t reg)
{
outportl(PCI_CONFIGURATION_ADDRESS,
0x80000000
| (bus << 16)
| (device << 11)
| (func << 8)
| (reg )); // Bit 0 and Bit 1 is reserved and is automatically set to 0.
// Therefore, we do not need & 0xFC, but this mask should be used for simulations.
return inportl(PCI_CONFIGURATION_DATA);
}
We do not like this reg & 0xFC. Some people tell us that this procedure might be not secure.
Does a crystal clear instruction exist that you need or do not need this "reg & 0xFC"?
The rule is, you can only read doublewords from configuration space. If you don't I have enough pieces of real hardware that make your code fail.
It is your choice whether you need the 0xfc or not.
"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 ]
Combuster wrote:The rule is, you can only read doublewords from configuration space. If you don't I have enough pieces of real hardware that make your code fail.
Which would be the fault of buggy hardware rather than the code, right?
"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 ]
I am going to use reg & 0xFC due to the fact that there might be buggy hardware and that qemu does no perfect simulation, but accepts unaligned register numbers.
Kevin wrote:I prefer reading the specification to reading code of a random hobby OS: "Bits 1 and 0 are read-only and must return 0's when read."
1) You don't know whether all hardware adheres 100% to the specification (in fact, in general, there's always hardware that doesn't completely comply to one spec or another)
2) Reserved bits may always change in a future specfication.
It is therefore always best to mask out any bits you are not interested in, whether they are supposed to be zero or not.
jal wrote:I really don't understand why you wouldn't just ignore the lower two bits.
Same here. I don't know a lot about the PCI spec, but it looks to me like the use of assert would just crash the program (albeit in debug mode...) for any hardware that doesn't zero those bits. But there's not a lot of point, because that's a hardware issue - just mask (and ignore) the bits in the first place.
In the context of the OP's original function, it is the programmer that supplies the value of reg, not hardware.
"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 ]
assert(reg & 0x03 && "Bits 0 & 1 are reserved by the PCI specification and must be zero");
Since it provides more information to me (I'm less likely to have to investigate the code of the asserting function - which is normally irrelevant - to see whats wrong)
