Determining valid PCI address ranges

[Octocontrabass]

You're detecting 64-bit BARs incorrectly: you're only checking bits 1 and 2, but you also need to check bit 0 since only memory BARs can be 64-bit.

Your code for restoring the upper half of a 64-bit BAR will never actually do that: you've got the condition backwards.

I forgot to ask earlier: why are you using count_ones() to find the number of set bits when you could calculate the size of the BAR directly?

[Ethin]

You're detecting 64-bit BARs incorrectly: you're only checking bits 1 and 2, but you also need to check bit 0 since only memory BARs can be 64-bit.

Your code for restoring the upper half of a 64-bit BAR will never actually do that: you've got the condition backwards.

Oopes! I can't believe I forgot that.

I forgot to ask earlier: why are you using count_ones() to find the number of set bits when you could calculate the size of the BAR directly?

Static typing constraints, or at least that was my reasoning at the time. The count_ones method returns the same type regardless of what actual type you were originally using, and I'm trying to store the (reversed) BAR in the outermost bar variable. I've also noticed I'm not adding 1 to the result -- would I do that when doing the 1 << ones operation or would I do that after NOTting the BAR (so (!bar + 1).count_ones())?
I think I've figured out two of the problems -- I was (1) counting *all* the bits set, including the informational ones, and (2) I was doing count_ones() twice. Which, you know, really doesn't work well. Not my day of coding today.
Should I continue skipping BARs that are zero? Not really sure if its worth it but... Also, what do I do about card bus bridges? Or should I keep ignoring those?

[Octocontrabass]

I've also noticed I'm not adding 1 to the result -- would I do that when doing the 1 << ones operation or would I do that after NOTting the BAR (so (!bar + 1).count_ones())?

Neither. If you add 1 after NOTing the BAR (after clearing the non-address bits), the resulting value is the size. You wouldn't need any shifting or bit counting.

[Ethin]

I've also noticed I'm not adding 1 to the result -- would I do that when doing the 1 << ones operation or would I do that after NOTting the BAR (so (!bar + 1).count_ones())?

Neither. If you add 1 after NOTing the BAR (after clearing the non-address bits), the resulting value is the size. You wouldn't need any shifting or bit counting.

Trying this and I'm getting addition overflows for 32-bit BARs.

Code: Select all

            if bar2 == 0x00 {
                let mut bar = if !oldbar.get_bit(0) {
                    *bar.set_bits(0..4, 0)
                } else {
                    *bar.set_bits(0..2, 0)
                };
                bar = (!bar) + 1;
                info!("BAR {} uses {} bytes", idx, bar);
            } else {
                let mut bar = (bar2 as u64) << 32 | (bar as u64);
                bar.set_bits(0..4, 0);
                bar = (!bar) + 1;
                info!("BAR {} uses {} bytes", idx, bar);
            }

It doesn't happen with 64-bit BARs but the 32-bit one is confusing. Its like its completely saturating the value when it NOTs it, but that doesn't make any sense.

[Octocontrabass]

Its like its completely saturating the value when it NOTs it, but that doesn't make any sense.

It makes sense for unimplemented BARs. All 32 bits are wired to 0 if the BAR is unimplemented, so when you NOT it you get the highest possible 32-bit integer.

Also, what do I do about card bus bridges?

CardBus bridges implement only BAR0, much like how PCI-to-PCI bridges implement BAR0 and BAR1. The other address-related registers decide which address ranges pass through the bridge to address devices behind the bridge. (Except the register at offset 0x44, but your CardBus bridge driver shouldn't use that one.)

[Ethin]

Its like its completely saturating the value when it NOTs it, but that doesn't make any sense.

It makes sense for unimplemented BARs. All 32 bits are wired to 0 if the BAR is unimplemented, so when you NOT it you get the highest possible 32-bit integer.

Also, what do I do about card bus bridges?

CardBus bridges implement only BAR0, much like how PCI-to-PCI bridges implement BAR0 and BAR1. The other address-related registers decide which address ranges pass through the bridge to address devices behind the bridge. (Except the register at offset 0x44, but your CardBus bridge driver shouldn't use that one.)

Oh okay, thanks. This is my new code (I threw in some debug calls because I'm getting unimplemented BARs for devices that (should) have them -- I'll explain that below):

Code: Select all

    let mut idx = 0;
    loop {
        if (dev.header_type == 0x00 && idx > 5)
            || (dev.header_type == 0x01 && idx > 1)
            || (dev.header_type == 0x02 && idx > 0)
        {
            break;
        }
        if dev.bars[&idx] == 0x00 {
            break;
        }
        let real_idx = match idx {
            0 => BAR0,
            1 => BAR1,
            2 => BAR2,
            3 => BAR3,
            4 => BAR4,
            5 => BAR5,
            _ => 0,
        };
        let oldbar = read_dword(addr, real_idx);
        let oldbar2 = if !oldbar.get_bit(0) && oldbar.get_bits(1..=2) == 0x02 {
            read_dword(addr, real_idx + 4)
        } else {
            0
        };
        debug!(
            "Read raw BARs from index {:X} and {:X}: {:X}, {:X}",
            real_idx,
            real_idx + 4,
            oldbar,
            oldbar2
        );
        write_dword(addr, real_idx, u32::MAX);
        debug!("Wrote {:X} to index {:X}", u32::MAX, real_idx);
        if !oldbar.get_bit(0) && oldbar.get_bits(1..=2) == 0x02 {
            write_dword(addr, real_idx + 4, u32::MAX);
            debug!(
                "BAR is 64 bits, so wrote {:X} to index {:X}",
                u32::MAX,
                real_idx + 4
            );
        }
        let mut bar = read_dword(addr, real_idx);
        debug!("Read {:X} from index {:X}", bar, real_idx);
        let bar2 = if !oldbar.get_bit(0) && oldbar.get_bits(1..=2) == 0x02 {
            read_dword(addr, real_idx + 4)
        } else {
            0
        };
        if bar2 != 0x00 {
            debug!(
                "BAR is 64 bits, so read {:X} from index {:X}",
                bar2,
                real_idx + 4
            );
        }
        write_dword(addr, real_idx, oldbar);
        debug!("Wrote old BAR value: {:X}, index {:X}", oldbar, real_idx);
        if bar2 != 0x00 {
            write_dword(addr, real_idx + 4, oldbar2);
            debug!(
                "Wrote old BAR value: {:X}, index {:X}",
                oldbar2,
                real_idx + 4
            );
        }
        if bar2 == 0x00 {
            let mut bar = if !oldbar.get_bit(0) {
                *bar.set_bits(0..4, 0)
            } else {
                *bar.set_bits(0..2, 0)
            };
            bar = (!bar) + 1;
            info!("BAR {} uses {} bytes", idx, bar);
        } else {
            let mut bar = (bar2 as u64) << 32 | (bar as u64);
            bar.set_bits(0..4, 0);
            bar = (!bar) + 1;
            info!("BAR {} uses {} bytes", idx, bar);
        }
        if oldbar.get_bits(1..=2) == 0x02 {
            idx += 2;
        } else {
            idx += 1;
        }
    }

The unimplemented BAR issue is a bit of a weird one. I'm launching qemu and requesting both an NVMe device and an AHCI device each with their own disk images attached to them. This is so I can work on my AHCI and NVMe drivers in sequence and just keep building on the VM as I get more and more of a functional system in future (since I think I've figured out AHCI finally). The device PCI tree looks something like this, and I'm quite confused (this is what my loop finds, not the actual values):

• Device 0:0:0:0 is a host bridge with no BARs
• Device 0:0:1:0 is a VGA-compatible display controller with 1 BAR
• Device 0:0:2:0 is an ethernet NIC with 4 BARs
• Device 0:0:3:0 is a non-volatile memory controller with 1 BAR
• Device 0:0:4:0 is a SATA controller with no BARs
• Device 0:0:5:0 is an audio device with 1 BAR
• Device 0:0:6:0 is a USB controller with 1 BAR
• Devices 0:0:1D:0, 0:0:1D:1, and 0:0:1D:2 are USB controllers with no BARs
• Device 0:0:1D:7 is yet another USB controller with 1 BAR
• Device 0:0:1F:0 is an ISA bridge with no BARs
• Device 0:0:1F:2 is a SATA controller with no BARs
• Device 0:0:1F:3 is an SMBus with no BARs

I've only removed the VirtIO stuff. But it does appear as though all the BARs are being found -- my loop just skips them for some reason. I could iterate through the BARs by key, but the problem is that I'd like to skip upper halves of 32-bit BARs (my initial BAR scan doesn't do this and just pulls in everything, but I'm hoping to refactor that too).

[Octocontrabass]

You're breaking out of the loop when you encounter an unimplemented BAR, when you actually want to continue to the next iteration.

[Ethin]

You're breaking out of the loop when you encounter an unimplemented BAR, when you actually want to continue to the next iteration.

Thanks. Got it working just fine now (it appears that way, anyway). Will definitely consider merging my BAR location algorithm into this one so it does both simultaneously and I don't read BARs that won't actually work (that is, I read all BARs, even if one of them was used in the calculation for the BAR before the one I'm reading).