hw: use enum class for PCI config registers

[uadhran]

Replace PCI config register #defines with typed enum classes (config_reg, command, cap_id) for type safety. Updates in pci_device.cpp and pci_msi.cpp.

Tested: nix-build unittests.nix — 85/85 passed (Nix 2.34.7, Linux).

Split from #2367 per review feedback.

Related: #2333

[mazunki]

All the integration checks seem to be working too :)

I wonder if it's reasonable to keep using shrtnd forms for the enum fields, or if we should use full words.

Are these acronyms standard in other places, or is it just to save a few keystrokes here? Personally I like when names are expressive on their own right, even if someone is unfamiliar with the context.

For instance, I have no idea what cap_id::MSIX means. At the very least, I would have hoped for its meaning to be expressed in the enum class definition, if not clear. Otherwise, if these are their standard names, I would include a reference url for where to find the information.

[mazunki]

You can add Closes: #2333 to this PR, and it will automatically close the related issue upon merge :)

[mazunki]

Casting things back to raw numbers for all operations defeats the purpose of using enum classes for type safety, and just adds noise with every cast.

Perhaps it'd be necessary to remove a generic write16 and read16, and make these methods for the enum-class instead for proper type safety?

If these functions are used by other parts of IncludeOS, I would approach it by first making one commit to refactor it internally, and exposing a temporary stub function, and a secondary commit to clean up legacy interface users (easy to find by just deleting the stub).

If these interfaces are required by an external protocol, I don't think there's too much we can do except hope that people prefer using the type-safe versions.

See also inline comments.

[mazunki]

Can we change the signature here? Is there a reason to keep this uint8_t?

  void PCI_Device::write16(const PCI_Device::cap_id reg, const uint16_t value) noexcept {
    PCI::msg req;
    req.data = 0x80000000;
    req.addr = pci_addr_;
    req.reg  = reg;

    outpd(PCI::CONFIG_ADDR, 0x80000000 | req.data);
    outpw(PCI::CONFIG_DATA + (reg & 2), value);
  }

  // idem for PCI_Device::config_reg etc

  // fallback if this needs to be exposed, otherwise for type safety better not
  void PCI_Device::write16(const uint8_t reg, const uint16_t value) noexcept {
    write16(static_cast<PCI_Device::cap_id>(reg), value);
  }

[mazunki]

See api/mem/alloc/buddy.hpp for an example of api/util/bitops.hpp::enable_bitmask_ops. Casting this to uint32_t isn't necessary.

That said, can all combinations of enum fields be combined? I'd like for proper bitmask support to check for valid combinations.