[RFC] Simple PCI config physical implementation

[alexisvl]

This RFC proposes an simple API for accessing physical PCI configuration space, to be added to extended_apis and to provide a foundation for more advanced PCI functionality.

The class suggested below provides register access by offset, as well as a set of methods to access named registers. This API will be simple to extend in the future to support the extended configuration space for PCIe, once we have the necessary tools to locate and map the physical memory region used for ECAM.

API is architecture-independent as PCI is not specific to x64; ARM should eventually have an implementation too.

phys_pci class definition

namespace eapis
{

class EXPORT_EAPIS_PCI phys_pci
{
public:
    pci_device(bus_type bus, device_type device, func_type func);
    virtual ~pci_device() = default;

    ///
    /// Register reads
    ///
    uint16_t get_device_id() const;
    uint16_t get_vendor_id() const;
    uint16_t get_status() const;
    uint16_t read_command() const;
    uint8_t get_class() const;
    uint8_t get_subclass() const;
    uint8_t get_prog_if() const;
    uint8_t get_revision() const;
    uint8_t read_bist() const;
    uint8_t get_header_type() const;
    uint8_t get_latency() const;
    uint8_t get_cl_size() const;
    uint32_t get_bar(uint8_t n) const;
    uint32_t get_cisp() const;
    uint16_t get_subsystem_id() const;
    uint16_t get_subsystem_vid() const;
    uint32_t get_exprom_bar() const;
    uint8_t get_capabilities_ptr() const;
    uint8_t get_max_latency() const;
    uint8_t get_min_grant() const;
    uint8_t get_int_pin() const;
    uint8_t get_int_line() const;

    ///
    /// Register writes
    ///
    void send_command(uint16_t command);
    void send_bist(uint8_t value);
    void set_latency(uint8_t cycles);
    void set_cl_size(uint8_t n_dwords);
    void set_bar(uint8_t n, uint32_t value);
    void set_exprom_bar(uint32_t value);
    void set_int_line(uint8_t line);

    ///
    /// Direct register accesses to this device
    ///
    uint8_t read_register_u8(register_type reg);
    uint16_t read_register_u16(register_type reg);
    uint32_t read_register_u32(register_type reg);
    void write_register(register_type reg, uint8_t val);
    void write_register(register_type reg, uint16_t val);
    void write_register(register_type reg, uint32_t val);

protected:

    ///
    /// Direct register accesses to any device
    ///

    static uint8_t read_register_u8(bus_type bus, device_type device,
        func_type func, register_type reg);

    static uint16_t read_register_u16(bus_type bus, device_type device,
        func_type func, register_type reg);

    static uint32_t read_register_u32(bus_type bus, device_type device,
        func_type func, register_type reg);

    static void write_register(bus_type bus, device_type device,
        func_type func, register_type reg, uint8_t value);

    static void write_register(bus_type bus, device_type device,
        func_type func, register_type reg, uint16_t value);

    static void write_register(bus_type bus, device_type device,
        func_type func, register_type reg, uint32_t value);

public:

    /// @cond

    phys_pci(phys_pci &&) = default;
    phys_pci &operator=(phys_pci &&) = default;

    phys_pci(const phys_pci &) = delete;
    phys_pci &operator=(const phys_pci &) = delete;

    /// @endcond
};

}

[cjams]

What is the get_w function supposed to do? Is that supposed to be get_f for 'get function' or something else?

What are the set(*) functions setting? The one with uint32_t seems like it would set the b/d/f/, but what about the others?

[alexisvl]

_b, _w, and _d are byte/word/dword matching the naming used in portio, since different width accesses to config space are possible - I couldn't find anywhere else that had to make that distinction so I kept that naming. set writes to config space just like get_* reads from it.

(Of couse the naming in portio comes from the x86 portio instructions so maybe it's not the best example to follow, but I wanted to match what was already there as well as I could. Let me know if I missed a more obvious example to follow.)

[cjams]

After chatting with Rian, the plan for PCI is to try to use a "virt half" "phys half" design. I thought the phys part was going to be like the other intrinsics, but for now we have settled on using classes.

The role of the physical half will still be to talk to the real PCI devices, but with a class interface similar to the ones on my interrupt-manager branch instead. If you look at my interrupt-manager branch of the extended APIs, under vic you'll see 'phys_lapic', 'phys_x2apic', 'virt_lapic', and 'virt_x2apic'.

The lapic classes are abstract base classes; phys_lapic is designed for reading and writing to a physical lapic, whereas the virt_lapic will (soon) provide an interface for registering handlers for a given interrupt vector, as well as reading and writing to a virtual apic.

If this design seems reasonable to you for PCI (Rian and I think it should be ok), please rewrite the RFC for a 'physpci' (name it whatever, but use phys for the physical prefix, virt_ for virtual prefix). Feel free to look at my interrupt-manager branch, as that will give you an idea of the design we're looking for.

Currently, the interrupt_manager has a phys_x2apic and a virt_x2apic, which are concrete implementations for talking to a phys,virt x2apic. the manager also registers the relevant exit handlers for guest apic accesses.

The PCI doesn't have to follow the interrupt design exactly, but there will have to be something that manages virtual pci devices and guest port I/O accesses to them, and those accesses need to be coordinated with physical PCI accesses.

We had this discussion late this afternoon and I know there is a lot, so if you have questions or concerns let me know. For now I would still focus on configuration of physical PCI devices using a class rather than namespaces

[alexisvl]

I've updated the RFC. I don't think what I had was all that far from what you're describing, and from what I see in the interrupt manager code. The original lower-level API was just to provide a wrapper around the architecture-specific ways of accessing PCI, so the part that provides clean register access can be architecture-independent - I still think that layer is needed, but I pulled it into the class as a set of static methods.

[cjams]

Looks good! This is exactly what I was hoping for. For now lets just put it in hve - EXPORT_EAPIS_HVE.

[alexisvl]

Got it, wasn't sure how that was structured. Thanks :+1:

[rianquinn]

+1, LGTM

[alexisvl]

While I'm still making occasional minor tweaks, this was effectively closed by Bareflank/extended_apis#66