From: Waldemar Kozaczuk <jwkozac...@gmail.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master

Refactor virtio layer to support modern PCI and mmio devices

This patch refactors virtio layer in order to support
modern PCI and mmio devices. It is also a step 0
on the list of tasks to make OSv boot on AWS firecracker.

This patch introduces virtio_device class that abstracts
virtio transport layer. This way virtio_driver class and
its specializations can interact with host device without being
tied to a specific transport (PCI, mmio, etc).

The virtio_pci_device and virtio_legacy_pci_device classes
define specializations of virtio_device for PCI transport.
This patch moves all PCI-related logic from virtio_driver
to virtio_legacy_pci_device class and effectively makes virtio driver
classes transport agnostic. Follow-up patches will provide implementations
of virtio_modern_pci_device and future virtio_mmio_device.

The interrupt_factory struct defines a mechanism used
by virtio drivers to specify particulars of creating/registering
proper interrupt logic for each transport.

Additionally this patch slightly changes a sequence of initialization
steps so that it conforms to the list specified by virtio specification
(please see section 3.1 of http://docs.oasis-open.org/virtio/virtio/v1.0/virtio-v1.0.html).

Finally this patch also removes some unused code.

Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
Message-Id: <20190205123005.8053-1-jwkozac...@gmail.com>

---
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -819,6 +819,7 @@ drivers += $(libtsm)
 drivers += drivers/vga.o drivers/kbd.o drivers/isa-serial.o
 drivers += arch/$(arch)/pvclock-abi.o
 drivers += drivers/virtio.o
+drivers += drivers/virtio-pci-device.o
 drivers += drivers/virtio-vring.o
 drivers += drivers/virtio-net.o
 drivers += drivers/vmxnet3.o
diff --git a/drivers/pci-generic.cc b/drivers/pci-generic.cc
--- a/drivers/pci-generic.cc
+++ b/drivers/pci-generic.cc
@@ -16,6 +16,8 @@
 #include "drivers/pci-function.hh"
 #include "drivers/pci-bridge.hh"
 #include "drivers/pci-device.hh"
+#include "drivers/virtio.hh"
+#include "drivers/virtio-pci-device.hh"

 namespace pci {

@@ -93,11 +95,28 @@ bool check_bus(u16 bus)
                 break;
             }

-            if (!device_manager::instance()->register_device(dev)) {
+            hw_device *dev_to_register = dev;
+            //
+            // Create virtio_device if vendor is VIRTIO_VENDOR_ID
+            if (dev->get_vendor_id() == virtio::VIRTIO_VENDOR_ID) {
+                if (auto pci_dev = dynamic_cast<device*>(dev)) {
+ dev_to_register = virtio::create_virtio_pci_device(pci_dev);
+                    if (!dev_to_register) {
+ pci_e("Error: couldn't create virtio pci device %02x:%02x.%x",
+                              bus, slot, func);
+                        delete dev;
+                    }
+                }
+                else
+ pci_e("Error: expected regular pci device %02x:%02x.%x",
+                          bus, slot, func);
+            }
+
+ if (dev_to_register && !device_manager::instance()->register_device(dev_to_register)) {
                 pci_e("Error: couldn't register device %02x:%02x.%x",
-                        bus, slot, func);
+                      bus, slot, func);
//TODO: Need to beautify it as multiple instances of the device may exist
-                delete dev;
+                delete dev_to_register;
             }

             // test for multiple functions
diff --git a/drivers/virtio-blk.cc b/drivers/virtio-blk.cc
--- a/drivers/virtio-blk.cc
+++ b/drivers/virtio-blk.cc
@@ -10,7 +10,6 @@

 #include "drivers/virtio.hh"
 #include "drivers/virtio-blk.hh"
-#include "drivers/pci-device.hh"
 #include <osv/interrupt.hh>

 #include <osv/mempool.hh>
@@ -24,6 +23,7 @@

 #include <osv/sched.hh>
 #include "osv/trace.hh"
+#include "osv/aligned_new.hh"

 #include <osv/device.h>
 #include <osv/bio.h>
@@ -100,7 +100,7 @@ struct driver blk_driver = {

 bool blk::ack_irq()
 {
-    auto isr = virtio_conf_readb(VIRTIO_PCI_ISR);
+    auto isr = _dev.read_and_ack_isr();
     auto queue = get_virt_queue(0);

     if (isr) {
@@ -112,33 +112,44 @@ bool blk::ack_irq()

 }

-blk::blk(pci::device& pci_dev)
-    : virtio_driver(pci_dev), _ro(false)
+blk::blk(virtio_device& virtio_dev)
+    : virtio_driver(virtio_dev), _ro(false)
 {

     _driver_name = "virtio-blk";
     _id = _instance++;
     virtio_i("VIRTIO BLK INSTANCE %d", _id);

+    // Steps 4 & 5 - negotiate and confirm features
     setup_features();
     read_config();

+    // Step 7 - generic init of virtqueues
+    probe_virt_queues();
+
     //register the single irq callback for the block
     sched::thread* t = sched::thread::make([this] { this->req_done(); },
             sched::thread::attr().name("virtio-blk"));
     t->start();
     auto queue = get_virt_queue(0);
-    if (pci_dev.is_msix()) {
- _msi.easy_register({ { 0, [=] { queue->disable_interrupts(); }, t } });
-    } else {
-        _irq.reset(new pci_interrupt(pci_dev,
-                                     [=] { return ack_irq(); },
-                                     [=] { t->wake(); }));
-    }
+
+    interrupt_factory int_factory;
+ int_factory.register_msi_bindings = [queue, t](interrupt_manager &msi) { + msi.easy_register( {{ 0, [=] { queue->disable_interrupts(); }, t }});
+    };
+
+    int_factory.create_pci_interrupt = [this,t](pci::device &pci_dev) {
+        return new pci_interrupt(
+            pci_dev,
+            [=] { return this->ack_irq(); },
+            [=] { t->wake(); });
+    };
+    _dev.register_interrupt(int_factory);

     // Enable indirect descriptor
     queue->set_use_indirect(true);

+    // Step 8
     add_dev_status(VIRTIO_CONFIG_S_DRIVER_OK);

     struct blk_priv* prv;
@@ -164,7 +175,7 @@ blk::~blk()
 void blk::read_config()
 {
//read all of the block config (including size, mce, topology,..) in one shot - virtio_conf_read(virtio_pci_config_offset(), &_config, sizeof(_config));
+    virtio_conf_read(0, &_config, sizeof(_config));

     trace_virtio_blk_read_config_capacity(_config.capacity);

@@ -308,7 +319,7 @@ u32 blk::get_driver_features()

 hw_driver* blk::probe(hw_device* dev)
 {
-    return virtio::probe<blk, VIRTIO_BLK_DEVICE_ID>(dev);
+    return virtio::probe<blk, VIRTIO_ID_BLOCK>(dev);
 }

 }
diff --git a/drivers/virtio-blk.hh b/drivers/virtio-blk.hh
--- a/drivers/virtio-blk.hh
+++ b/drivers/virtio-blk.hh
@@ -8,7 +8,7 @@
 #ifndef VIRTIO_BLK_DRIVER_H
 #define VIRTIO_BLK_DRIVER_H
 #include "drivers/virtio.hh"
-#include "drivers/pci-device.hh"
+#include "drivers/virtio-device.hh"
 #include <osv/bio.h>

 namespace virtio {
@@ -31,7 +31,6 @@ public:
     };

     enum {
-        VIRTIO_BLK_DEVICE_ID = 0x1001,
         VIRTIO_BLK_ID_BYTES = 20, /* ID string length */

         /*
@@ -118,7 +117,7 @@ public:
         u8 status;
     };

-    explicit blk(pci::device& dev);
+    explicit blk(virtio_device& dev);
     virtual ~blk();

     virtual std::string get_name() const { return _driver_name; }
@@ -157,7 +156,6 @@ private:
     bool _ro;
     // This mutex protects parallel make_request invocations
     mutex _lock;
-    std::unique_ptr<pci_interrupt> _irq;
 };

 }
diff --git a/drivers/virtio-device.hh b/drivers/virtio-device.hh
--- a/drivers/virtio-device.hh
+++ b/drivers/virtio-device.hh
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2019 Waldemar Kozaczuk.
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+#ifndef VIRTIO_DEVICE_HH
+#define VIRTIO_DEVICE_HH
+
+#include <osv/types.h>
+#include <osv/pci.hh>
+#include <osv/interrupt.hh>
+#include <osv/msi.hh>
+#include "virtio-vring.hh"
+#include "device.hh"
+
+using namespace hw;
+
+#define VIRTIO_ALIGN(x,alignment) ((x + (alignment-1)) & ~(alignment-1))
+
+namespace virtio {
+
+// Allows virtio drivers specify how to instantiate interrupts or
+// register msi bindings. The associated virtio-device will
+// use adequate functor to create correct interrupt in order
+// to register it.
+struct interrupt_factory {
+ std::function<void(interrupt_manager &)> register_msi_bindings = nullptr; + std::function<pci_interrupt *(pci::device &)> create_pci_interrupt = nullptr; + std::function<gsi_edge_interrupt *()> create_gsi_edge_interrupt = nullptr;
+};
+
+// Defines virtio transport abstraction used by virtio-driver
+// to communicate with virtio device. The specializations of this
+// include virtio pci device (legacy and modern) as well
+// as virtio mmio device. This abstraction allows virtio driver
+// not be tied to any specific transport (pci, mmio).
+//
+// From the specification:
+// "Each device consists of the following parts:
+//   - Device status field - see section 2.1 of the spec
+//   - Feature bits - see section 2.2 of the spec
+//   - Device Configuration space - see section 2.3 of the spec
+//   - One or more virtqueues" - see section 2.4 of the spec
+//
+// For details please VIRTIO 1.0 specification here -
+// http://docs.oasis-open.org/virtio/virtio/v1.0/virtio-v1.0.html.
+class virtio_device : public hw_device {
+public:
+    virtual ~virtio_device() {};
+
+    virtual void init() = 0;
+
+    virtual unsigned get_irq() = 0;
+    virtual u8 read_and_ack_isr() = 0;
+    virtual void register_interrupt(interrupt_factory irq_factory) = 0;
+
+    virtual void select_queue(int queue) = 0;
+    virtual u16 get_queue_size() = 0;
+    virtual void setup_queue(vring *queue) = 0;
+    virtual void kick_queue(int queue) = 0;
+
+    virtual u64 get_available_features() = 0;
+    virtual bool get_available_feature_bit(int bit) = 0;
+
+    virtual void set_enabled_features(u64 features) = 0;
+    virtual u64 get_enabled_features() = 0;
+    virtual bool get_enabled_feature_bit(int bit) = 0;
+
+    // From the spec:
+    // "The device status field provides a simple low-level indication of
+    //  the completed steps of the device initialization sequence".
+    virtual u8 get_status() = 0;
+    virtual void set_status(u8 status) = 0;
+
+    virtual u8 read_config(u32 offset) = 0;
+    virtual void dump_config() = 0;
+
+    virtual bool is_modern() = 0;
+    virtual size_t get_vring_alignment() = 0;
+};
+
+}
+
+#endif //VIRTIO_DEVICE_HH
diff --git a/drivers/virtio-net.cc b/drivers/virtio-net.cc
--- a/drivers/virtio-net.cc
+++ b/drivers/virtio-net.cc
@@ -10,7 +10,6 @@

 #include "drivers/virtio.hh"
 #include "drivers/virtio-net.hh"
-#include "drivers/pci-device.hh"
 #include <osv/interrupt.hh>

 #include <osv/mempool.hh>
@@ -217,32 +216,39 @@ void net::fill_qstats(const struct txq& txq, struct if_data* out_data) const

 bool net::ack_irq()
 {
-    auto isr = virtio_conf_readb(VIRTIO_PCI_ISR);
+    auto isr = _dev.read_and_ack_isr();

     if (isr) {
         _rxq.vqueue->disable_interrupts();
         return true;
     } else {
         return false;
     }
-
 }

-net::net(pci::device& dev)
-    : virtio_driver(dev),
-      _rxq(get_virt_queue(0), [this] { this->receiver(); }),
-      _txq(this, get_virt_queue(1))
+void net::init()
 {
-    sched::thread* poll_task = _rxq.poll_task.get();
+    // Steps 4 & 5 - negotiate and confirm features
+    setup_features();
+    read_config();

-    poll_task->set_priority(sched::thread::priority_infinity);
+    // Step 7 - generic init of virtqueues
+    probe_virt_queues();
+}

+net::net(virtio_device& dev)
+    : virtio_driver(dev),
+    _pre_init(this),
+    _rxq(get_virt_queue(0), [this] { this->receiver(); }),
+    _txq(this, get_virt_queue(1))
+{
     _driver_name = "virtio-net";
     virtio_i("VIRTIO NET INSTANCE");
     _id = _instance++;

-    setup_features();
-    read_config();
+    sched::thread* poll_task = _rxq.poll_task.get();
+
+    poll_task->set_priority(sched::thread::priority_infinity);

_hdr_size = _mergeable_bufs ? sizeof(net_hdr_mrg_rxbuf) : sizeof(net_hdr);

@@ -290,19 +296,25 @@ net::net(pci::device& dev)

     ether_ifattach(_ifn, _config.mac);

-    if (dev.is_msix()) {
-        _msi.easy_register({
-            { 0, [&] { _rxq.vqueue->disable_interrupts(); }, poll_task },
-            { 1, [&] { _txq.vqueue->disable_interrupts(); }, nullptr }
-        });
-    } else {
-        _irq.reset(new pci_interrupt(dev,
-                                     [=] { return this->ack_irq(); },
-                                     [=] { poll_task->wake(); }));
-    }
+    interrupt_factory int_factory;
+ int_factory.register_msi_bindings = [this,poll_task](interrupt_manager &msi) {
+       msi.easy_register({
+ { 0, [&] { this->_rxq.vqueue->disable_interrupts(); }, poll_task },
+           { 1, [&] { this->_txq.vqueue->disable_interrupts(); }, nullptr }
+       });
+    };
+
+ int_factory.create_pci_interrupt = [this,poll_task](pci::device &pci_dev) {
+        return new pci_interrupt(
+            pci_dev,
+            [=] { return this->ack_irq(); },
+            [=] { poll_task->wake(); });
+    };
+    _dev.register_interrupt(int_factory);

     fill_rx_ring();

+    // Step 8
     add_dev_status(VIRTIO_CONFIG_S_DRIVER_OK);
 }

@@ -324,7 +336,7 @@ net::~net()
 void net::read_config()
 {
     //read all of the net config  in one shot
- virtio_conf_read(virtio_pci_config_offset(), &_config, sizeof(_config));
+    virtio_conf_read(0, &_config, sizeof(_config));

     if (get_guest_feature_bit(VIRTIO_NET_F_MAC))
         net_i("The mac addr of the device is %x:%x:%x:%x:%x:%x",
@@ -856,12 +868,12 @@ u32 net::get_driver_features()

 hw_driver* net::probe(hw_device* dev)
 {
-    if (auto pci_dev = dynamic_cast<pci::device*>(dev)) {
- if (pci_dev->get_id() == hw_device_id(VIRTIO_VENDOR_ID, VIRTIO_NET_DEVICE_ID)) {
+    if (auto virtio_dev = dynamic_cast<virtio_device*>(dev)) {
+ if (virtio_dev->get_id() == hw_device_id(VIRTIO_VENDOR_ID, VIRTIO_ID_NET)) {
             if (opt_maxnic && maxnic-- <= 0) {
                 return nullptr;
             } else {
-                return aligned_new<net>(*pci_dev);
+                return aligned_new<net>(*virtio_dev);
             }
         }
     }
diff --git a/drivers/virtio-net.hh b/drivers/virtio-net.hh
--- a/drivers/virtio-net.hh
+++ b/drivers/virtio-net.hh
@@ -53,7 +53,6 @@ public:
     };

     enum {
-        VIRTIO_NET_DEVICE_ID = 0x1000,
         VIRTIO_NET_S_LINK_UP = 1,       /* Link is up */
         VIRTIO_NET_S_ANNOUNCE = 2,       /* Announcement is needed */
         VIRTIO_NET_OK = 0,
@@ -204,8 +203,9 @@ public:
             u16 virtqueue_pairs;
     };

-    explicit net(pci::device& dev);
+    explicit net(virtio_device& dev);
     virtual ~net();
+    void init();

     virtual std::string get_name() const { return _driver_name; }
     void read_config();
@@ -269,8 +269,6 @@ private:

     u32 _hdr_size;

-    std::unique_ptr<pci_interrupt> _irq;
-
     struct rxq_stats {
         u64 rx_packets; /* if_ipackets */
         u64 rx_bytes;   /* if_ibytes */
@@ -299,6 +297,12 @@ private:
         wakeup_stats tx_wakeup_stats;
     };

+    struct pre_init {
+        explicit pre_init(virtio::net *_net) {
+           _net->init();
+        }
+    } _pre_init;
+
     /* Single Rx queue object */
     struct rxq {
         rxq(vring* vq, std::function<void ()> poll_func)
diff --git a/drivers/virtio-pci-device.cc b/drivers/virtio-pci-device.cc
--- a/drivers/virtio-pci-device.cc
+++ b/drivers/virtio-pci-device.cc
@@ -0,0 +1,170 @@
+/*
+ * Copyright (C) 2019 Waldemar Kozaczuk.
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+#include "drivers/virtio-pci-device.hh"
+
+namespace virtio {
+
+virtio_pci_device::virtio_pci_device(pci::device *dev)
+    : virtio_device()
+    , _dev(dev)
+    , _msi(dev)
+{
+}
+
+virtio_pci_device::~virtio_pci_device() {
+    delete _dev;
+}
+
+void virtio_pci_device::dump_config()
+{
+    u8 B, D, F;
+    _dev->get_bdf(B, D, F);
+
+    virtio_d("version:%s, type id:%d, pci device [%x:%x.%x] vid:id=%x:%x",
+             get_version(), get_type_id(),
+             (u16)B, (u16)D, (u16)F,
+             _dev->get_vendor_id(),
+             _dev->get_device_id());
+}
+
+void virtio_pci_device::init()
+{
+    bool status = parse_pci_config();
+    assert(status);
+
+    _dev->set_bus_master(true);
+
+    _dev->msix_enable();
+}
+
+void virtio_pci_device::register_interrupt(interrupt_factory irq_factory)
+{
+    if (irq_factory.register_msi_bindings && _dev->is_msix()) {
+        irq_factory.register_msi_bindings(_msi);
+    } else {
+        _irq.reset(irq_factory.create_pci_interrupt(*_dev));
+    }
+}
+
+virtio_legacy_pci_device::virtio_legacy_pci_device(pci::device *dev)
+    : virtio_pci_device(dev)
+{
+}
+
+void virtio_legacy_pci_device::kick_queue(int queue)
+{
+    virtio_conf_writew(VIRTIO_PCI_QUEUE_NOTIFY, queue);
+}
+
+void virtio_legacy_pci_device::setup_queue(vring *queue)
+{
+    if (_dev->is_msix()) {
+        // Setup queue_id:entry_id 1:1 correlation...
+        virtio_conf_writew(VIRTIO_MSI_QUEUE_VECTOR, queue->index());
+        if (virtio_conf_readw(VIRTIO_MSI_QUEUE_VECTOR) != queue->index()) {
+ virtio_e("Setting MSIx entry for queue %d failed.", queue->index());
+            return;
+        }
+    }
+    // Tell host about pfn
+ // TODO: Yak, this is a bug in the design, on large memory we'll have PFNs > 32 bit
+    // Dor to notify Rusty
+ virtio_conf_writel(VIRTIO_PCI_QUEUE_PFN, (u32)(queue->get_paddr() >> VIRTIO_PCI_QUEUE_ADDR_SHIFT));
+}
+
+void virtio_legacy_pci_device::select_queue(int queue)
+{
+    virtio_conf_writew(VIRTIO_PCI_QUEUE_SEL, queue);
+}
+
+u16 virtio_legacy_pci_device::get_queue_size()
+{
+    return virtio_conf_readw(VIRTIO_PCI_QUEUE_NUM);
+}
+
+bool virtio_legacy_pci_device::get_virtio_config_bit(u32 offset, int bit)
+{
+    return (virtio_conf_readl(offset) & (1 << bit)) != 0;
+}
+
+u64 virtio_legacy_pci_device::get_available_features()
+{
+    return virtio_conf_readl(VIRTIO_PCI_HOST_FEATURES);
+}
+
+bool virtio_legacy_pci_device::get_available_feature_bit(int bit)
+{
+    return get_virtio_config_bit(VIRTIO_PCI_HOST_FEATURES, bit);
+}
+
+void virtio_legacy_pci_device::set_enabled_features(u64 features)
+{
+    virtio_conf_writel(VIRTIO_PCI_GUEST_FEATURES, (u32)features);
+}
+
+u64 virtio_legacy_pci_device::get_enabled_features()
+{
+    return virtio_conf_readl(VIRTIO_PCI_GUEST_FEATURES);
+}
+
+bool virtio_legacy_pci_device::get_enabled_feature_bit(int bit)
+{
+    return get_virtio_config_bit(VIRTIO_PCI_GUEST_FEATURES, bit);
+}
+
+u8 virtio_legacy_pci_device::get_status()
+{
+    return virtio_conf_readb(VIRTIO_PCI_STATUS);
+}
+
+void virtio_legacy_pci_device::set_status(u8 status)
+{
+    virtio_conf_writeb(VIRTIO_PCI_STATUS, status);
+}
+
+u8 virtio_legacy_pci_device::read_config(u32 offset)
+{
+    auto conf_start = _dev->is_msix_enabled()? 24 : 20;
+    return _bar1->readb(conf_start + offset);
+}
+
+u8 virtio_legacy_pci_device::read_and_ack_isr()
+{
+    return virtio_conf_readb(VIRTIO_PCI_ISR);
+}
+
+bool virtio_legacy_pci_device::parse_pci_config()
+{
+    // Test whether bar1 is present
+    _bar1 = _dev->get_bar(1);
+    if (_bar1 == nullptr) {
+        return false;
+    }
+
+    // Check ABI version
+    u8 rev = _dev->get_revision_id();
+    if (rev != VIRTIO_PCI_LEGACY_ABI_VERSION) {
+        virtio_e("Wrong virtio revision=%x", rev);
+        return false;
+    }
+
+    // Check device ID
+    u16 dev_id = _dev->get_device_id();
+ if ((dev_id < VIRTIO_PCI_LEGACY_ID_MIN) || (dev_id > VIRTIO_PCI_LEGACY_ID_MAX)) {
+        virtio_e("Wrong virtio dev id %x", dev_id);
+        return false;
+    }
+
+    return true;
+}
+
+virtio_device* create_virtio_pci_device(pci::device *dev) {
+    return new virtio_legacy_pci_device(dev);
+}
+
+}
diff --git a/drivers/virtio-pci-device.hh b/drivers/virtio-pci-device.hh
--- a/drivers/virtio-pci-device.hh
+++ b/drivers/virtio-pci-device.hh
@@ -0,0 +1,143 @@
+/*
+ * Copyright (C) 2019 Waldemar Kozaczuk.
+ *
+ * This work is open source software, licensed under the terms of the
+ * BSD license as described in the LICENSE file in the top-level directory.
+ */
+
+#ifndef VIRTIO_PCI_DEVICE_HH
+#define VIRTIO_PCI_DEVICE_HH
+
+#include <osv/pci.hh>
+#include <osv/interrupt.hh>
+#include <osv/msi.hh>
+
+#include "virtio-device.hh"
+#include "virtio.hh"
+
+namespace virtio {
+
+enum VIRTIO_PCI_CONFIG {
+    /* A 32-bit r/o bitmask of the features supported by the host */
+    VIRTIO_PCI_HOST_FEATURES = 0,
+    /* A 32-bit r/o bitmask of the features supported by the host */
+    VIRTIO_PCI_HOST_FEATURES_HIGH = 1,
+    /* A 32-bit r/w bitmask of features activated by the guest */
+    VIRTIO_PCI_GUEST_FEATURES = 4,
+    /* A 32-bit r/w PFN for the currently selected queue */
+    VIRTIO_PCI_QUEUE_PFN = 8,
+    /* A 16-bit r/o queue size for the currently selected queue */
+    VIRTIO_PCI_QUEUE_NUM = 12,
+    /* A 16-bit r/w queue selector */
+    VIRTIO_PCI_QUEUE_SEL = 14,
+    /* A 16-bit r/w queue notifier */
+    VIRTIO_PCI_QUEUE_NOTIFY = 16,
+    /* An 8-bit device status register.  */
+    VIRTIO_PCI_STATUS = 18,
+ /* An 8-bit r/o interrupt status register. Reading the value will return the + * current contents of the ISR and will also clear it. This is effectively
+     * a read-and-acknowledge. */
+    VIRTIO_PCI_ISR = 19,
+    /* The bit of the ISR which indicates a device configuration change. */
+    VIRTIO_PCI_ISR_CONFIG  = 0x2,
+    /* MSI-X registers: only enabled if MSI-X is enabled. */
+    /* A 16-bit vector for configuration changes. */
+    VIRTIO_MSI_CONFIG_VECTOR = 20,
+    /* A 16-bit vector for selected queue notifications. */
+    VIRTIO_MSI_QUEUE_VECTOR = 22,
+    /* Vector value used to disable MSI for queue */
+    VIRTIO_MSI_NO_VECTOR = 0xffff,
+    /* Virtio ABI version, this must match exactly */
+    VIRTIO_PCI_LEGACY_ABI_VERSION = 0,
+    /* How many bits to shift physical queue address written to QUEUE_PFN.
+     * 12 is historical, and due to x86 page size. */
+    VIRTIO_PCI_QUEUE_ADDR_SHIFT = 12,
+    /* The alignment to use between consumer and producer parts of vring.
+     * x86 pagesize again. */
+    VIRTIO_PCI_VRING_ALIGN = 4096,
+    VIRTIO_PCI_LEGACY_ID_MIN = 0x1000,
+    VIRTIO_PCI_LEGACY_ID_MAX = 0x103f,
+};
+
+class virtio_pci_device : public virtio_device {
+public:
+    explicit virtio_pci_device(pci::device *dev);
+    ~virtio_pci_device();
+
+    virtual const char *get_version() = 0;
+    virtual u16 get_type_id() = 0;
+
+ virtual hw_device_id get_id() { return hw_device_id(VIRTIO_VENDOR_ID,get_type_id()); }
+    virtual void print() { _dev->print(); }
+    virtual void reset() { _dev->reset(); }
+
+    bool is_attached()  { return _dev->is_attached(); }
+    void set_attached() { _dev->set_attached(); }
+
+    virtual void dump_config();
+    virtual void init();
+    virtual void register_interrupt(interrupt_factory irq_factory);
+
+    virtual unsigned get_irq() { return 0; }
+    size_t get_vring_alignment() { return VIRTIO_PCI_VRING_ALIGN; }
+
+protected:
+    virtual bool parse_pci_config() = 0;
+
+    pci::device *_dev;
+    interrupt_manager _msi;
+    std::unique_ptr<pci_interrupt> _irq;
+};
+
+class virtio_legacy_pci_device : public virtio_pci_device {
+public:
+    explicit virtio_legacy_pci_device(pci::device *dev);
+    ~virtio_legacy_pci_device() {}
+
+    virtual const char *get_version() { return "legacy"; }
+    virtual u16 get_type_id() { return _dev->get_subsystem_id(); };
+
+    virtual void select_queue(int queue);
+    virtual u16 get_queue_size();
+    virtual void setup_queue(vring *queue);
+    virtual void kick_queue(int queue);
+
+    virtual u64 get_available_features();
+    virtual bool get_available_feature_bit(int bit);
+
+    virtual void set_enabled_features(u64 features);
+    virtual u64 get_enabled_features();
+    virtual bool get_enabled_feature_bit(int bit);
+
+    virtual u8 get_status();
+    virtual void set_status(u8 status);
+
+    virtual u8 read_config(u32 offset);
+    virtual u8 read_and_ack_isr();
+
+    virtual bool is_modern() { return false; };
+protected:
+    virtual bool parse_pci_config();
+
+private:
+    // Access the virtio conf address space set by pci bar 1
+    bool get_virtio_config_bit(u32 offset, int bit);
+
+    // Access virtio config space
+    u8 virtio_conf_readb(u32 offset) { return _bar1->readb(offset);};
+    u16 virtio_conf_readw(u32 offset) { return _bar1->readw(offset);};
+    u32 virtio_conf_readl(u32 offset) { return _bar1->readl(offset);};
+ void virtio_conf_writeb(u32 offset, u8 val) { _bar1->writeb(offset, val);}; + void virtio_conf_writew(u32 offset, u16 val) { _bar1->writew(offset, val);}; + void virtio_conf_writel(u32 offset, u32 val) { _bar1->writel(offset, val);};
+
+    pci::bar* _bar1;
+};
+
+// Creates appropriate instance of virtio_pci_device
+// by reading configuration from PCI device
+virtio_device* create_virtio_pci_device(pci::device *dev);
+
+}
+
+#endif //VIRTIO_PCI_DEVICE_HH
\ No newline at end of file
diff --git a/drivers/virtio-rng.cc b/drivers/virtio-rng.cc
--- a/drivers/virtio-rng.cc
+++ b/drivers/virtio-rng.cc
@@ -36,13 +36,28 @@ static int virtio_rng_read(void *buf, int size)
 }

 namespace virtio {
-rng::rng(pci::device& pci_dev)
-    : virtio_driver(pci_dev)
-    , _irq(pci_dev, [&] { return ack_irq(); }, [&] { handle_irq(); })
+rng::rng(virtio_device& dev)
+    : virtio_driver(dev)
, _thread(sched::thread::make([&] { worker(); }, sched::thread::attr().name("virtio-rng")))
 {
+    // Steps 4 & 5 - negotiate and confirm features
+    setup_features();
+
+    // Step 7 - generic init of virtqueues
+    probe_virt_queues();
+
+    interrupt_factory int_factory;
+    int_factory.create_pci_interrupt = [this](pci::device &pci_dev) {
+        return new pci_interrupt(
+            pci_dev,
+            [=] { return this->ack_irq(); },
+            [=] { this->handle_irq(); });
+    };
+    _dev.register_interrupt(int_factory);
+
     _queue = get_virt_queue(0);

+    // Step 8
     add_dev_status(VIRTIO_CONFIG_S_DRIVER_OK);

     _thread->start();
@@ -79,7 +94,7 @@ void rng::handle_irq()

 bool rng::ack_irq()
 {
-    return virtio_conf_readb(VIRTIO_PCI_ISR);
+    return _dev.read_and_ack_isr();
 }

 void rng::worker()
@@ -124,7 +139,7 @@ void rng::refill()

 hw_driver* rng::probe(hw_device* dev)
 {
-    return virtio::probe<rng, VIRTIO_RNG_DEVICE_ID>(dev);
+    return virtio::probe<rng, VIRTIO_ID_RNG>(dev);
 }

 }
diff --git a/drivers/virtio-rng.hh b/drivers/virtio-rng.hh
--- a/drivers/virtio-rng.hh
+++ b/drivers/virtio-rng.hh
@@ -22,11 +22,7 @@ namespace virtio {

 class rng : public virtio_driver, randomdev::hw_rng {
 public:
-    enum {
-        VIRTIO_RNG_DEVICE_ID = 0x1005,
-    };
-
-    explicit rng(pci::device& dev);
+    explicit rng(virtio_device& dev);
     virtual ~rng();

     virtual std::string get_name() const { return "virtio-rng"; }
@@ -44,7 +40,6 @@ private:

     static const size_t _pool_size = 64;
     std::vector<char> _entropy;
-    pci_interrupt _irq;
     std::unique_ptr<sched::thread> _thread;
     condvar _producer;
     condvar _consumer;
diff --git a/drivers/virtio-scsi.cc b/drivers/virtio-scsi.cc
--- a/drivers/virtio-scsi.cc
+++ b/drivers/virtio-scsi.cc
@@ -12,7 +12,6 @@
 #include <osv/mempool.hh>
 #include <osv/sched.hh>
 #include <osv/interrupt.hh>
-#include "drivers/pci-device.hh"
 #include "drivers/virtio.hh"
 #include "drivers/virtio-scsi.hh"
 #include "drivers/scsi-common.hh"
@@ -129,7 +128,7 @@ void scsi::add_lun(u16 target, u16 lun)

 bool scsi::ack_irq()
 {
-    auto isr = virtio_conf_readb(VIRTIO_PCI_ISR);
+    auto isr = _dev.read_and_ack_isr();
     auto queue = get_virt_queue(VIRTIO_SCSI_QUEUE_REQ);

     if (isr) {
@@ -141,37 +140,47 @@ bool scsi::ack_irq()

 }

-scsi::scsi(pci::device& dev)
+scsi::scsi(virtio_device& dev)
     : virtio_driver(dev)
 {

     _driver_name = "virtio-scsi";
     _id = _instance++;

+    // Steps 4 & 5 - negotiate and confirm features
     setup_features();
     read_config();

+    // Step 7 - generic init of virtqueues
+    probe_virt_queues();
+
     //register the single irq callback for the block
     sched::thread* t = sched::thread::make([this] { this->req_done(); },
             sched::thread::attr().name("virtio-scsi"));
     t->start();
     auto queue = get_virt_queue(VIRTIO_SCSI_QUEUE_REQ);

-    if (dev.is_msix()) {
-        _msi.easy_register({
-                { VIRTIO_SCSI_QUEUE_CTRL, nullptr, nullptr },
-                { VIRTIO_SCSI_QUEUE_EVT, nullptr, nullptr },
- { VIRTIO_SCSI_QUEUE_REQ, [=] { queue->disable_interrupts(); }, t },
+    interrupt_factory int_factory;
+    int_factory.register_msi_bindings = [queue,t](interrupt_manager &msi) {
+        msi.easy_register({
+          { VIRTIO_SCSI_QUEUE_CTRL, nullptr, nullptr },
+          { VIRTIO_SCSI_QUEUE_EVT, nullptr, nullptr },
+ { VIRTIO_SCSI_QUEUE_REQ, [=] { queue->disable_interrupts(); }, t },
         });
-    } else {
-        _irq.reset(new pci_interrupt(dev,
-                                     [=] { return this->ack_irq(); },
-                                     [=] { t->wake(); }));
-    }
+    };
+
+    int_factory.create_pci_interrupt = [this,t](pci::device &pci_dev) {
+        return new pci_interrupt(
+                pci_dev,
+                [=] { return this->ack_irq(); },
+                [=] { t->wake(); });
+    };
+    _dev.register_interrupt(int_factory);

     // Enable indirect descriptor
     queue->set_use_indirect(true);

+    // Step 8
     add_dev_status(VIRTIO_CONFIG_S_DRIVER_OK);

     scan();
@@ -184,7 +193,7 @@ scsi::~scsi()

 void scsi::read_config()
 {
- virtio_conf_read(virtio_pci_config_offset(), &_config, sizeof(_config));
+    virtio_conf_read(0, &_config, sizeof(_config));
     config.max_lun = _config.max_lun;
     config.max_target = _config.max_target;
 }
@@ -245,7 +254,6 @@ u32 scsi::get_driver_features()

 hw_driver* scsi::probe(hw_device* dev)
 {
-    return virtio::probe<scsi, VIRTIO_SCSI_DEVICE_ID>(dev);
+    return virtio::probe<scsi, VIRTIO_ID_SCSI>(dev);
 }
-
 }
diff --git a/drivers/virtio-scsi.hh b/drivers/virtio-scsi.hh
--- a/drivers/virtio-scsi.hh
+++ b/drivers/virtio-scsi.hh
@@ -8,7 +8,6 @@
 #ifndef VIRTIO_SCSI_DRIVER_H
 #define VIRTIO_SCSI_DRIVER_H
 #include "drivers/virtio.hh"
-#include "drivers/pci-device.hh"
 #include "drivers/scsi-common.hh"
 #include <osv/bio.h>
 #include <osv/types.h>
@@ -34,7 +33,6 @@ public:
         VIRTIO_SCSI_CDB_SIZE = 32,
         VIRTIO_SCSI_SENSE_SIZE = 96,
         VIRTIO_SCSI_SECTOR_SIZE = 512,
-        VIRTIO_SCSI_DEVICE_ID = 0x1004,
     };

     enum scsi_res_code {
@@ -145,7 +143,7 @@ public:
     };


-    scsi(pci::device& dev);
+    scsi(virtio_device& dev);
     ~scsi();

     virtual std::string get_name() const { return _driver_name; }
@@ -174,8 +172,6 @@ private:
     std::string _driver_name;
     scsi_config _config;

-    std::unique_ptr<pci_interrupt> _irq;
-
     //maintains the virtio instance number for multiple drives
     static int _instance;
     int _id;
diff --git a/drivers/virtio-vring.cc b/drivers/virtio-vring.cc
--- a/drivers/virtio-vring.cc
+++ b/drivers/virtio-vring.cc
@@ -38,12 +38,13 @@ TRACEPOINT(trace_vring_get_buf_ret, "vring=%p _avail_count %d", void*, int);

 namespace virtio {

-    vring::vring(virtio_driver* const dev, u16 num, u16 q_index)
+    vring::vring(virtio_driver* const driver, u16 num, u16 q_index)
     {
-        _dev = dev;
+        _driver = driver;
         _q_index = q_index;
         // Alloc enough pages for the vring...
- unsigned sz = VIRTIO_ALIGN(vring::get_size(num, VIRTIO_PCI_VRING_ALIGN));
+        size_t alignment = driver->get_vring_alignment();
+ size_t sz = VIRTIO_ALIGN(vring::get_size(num, alignment), alignment);
         _vring_ptr = memory::alloc_phys_contiguous_aligned(sz, 4096);
         memset(_vring_ptr, 0, sz);

@@ -53,7 +54,7 @@ namespace virtio {
         _desc = (vring_desc*)_vring_ptr;
         _avail = (vring_avail*)(_vring_ptr + num * sizeof(vring_desc));
         _used = (vring_used*)(((unsigned long)&_avail->_ring[num] +
- sizeof(u16) + VIRTIO_PCI_VRING_ALIGN - 1) & ~(VIRTIO_PCI_VRING_ALIGN - 1));
+                sizeof(u16) + alignment - 1) & ~(alignment - 1));

         // initialize the next pointer within the available ring
         for (int i = 0; i < num; i++) _desc[i]._next = i + 1;
@@ -102,7 +103,7 @@ namespace virtio {
     inline bool vring::use_indirect(int desc_needed)
     {
         return _use_indirect &&
-               _dev->get_indirect_buf_cap() &&
+               _driver->get_indirect_buf_cap() &&
// don't let the posting fail due to low available buffers number
                (desc_needed > _avail_count ||
                // no need to use indirect for a single descriptor
@@ -280,7 +281,7 @@ namespace virtio {
     vring::kick() {
         bool kicked = true;

-        if (_dev->get_event_idx_cap()) {
+        if (_driver->get_event_idx_cap()) {

             std::atomic_thread_fence(std::memory_order_seq_cst);

@@ -310,7 +311,7 @@ namespace virtio {
// and _avail_added_since_kick might wrap around due to this bulking.
         //
         if (kicked || (_avail_added_since_kick >= (u16)(~0) / 2)) {
-            _dev->kick(_q_index);
+            _driver->kick(_q_index);
             _avail_added_since_kick = 0;
             return true;
         }
diff --git a/drivers/virtio-vring.hh b/drivers/virtio-vring.hh
--- a/drivers/virtio-vring.hh
+++ b/drivers/virtio-vring.hh
@@ -122,7 +122,7 @@ class virtio_driver;
     class vring {
     public:

-        vring(virtio_driver* const dev, u16 num, u16 q_index);
+        vring(virtio_driver* const driver, u16 num, u16 q_index);
         virtual ~vring();

         u64 get_paddr();
@@ -183,6 +183,8 @@ class virtio_driver;
         // Total number of descriptors in ring
         int size() {return _num;}

+        u16 index() {return _q_index; }
+
// Use memory order acquire when there are prior updates to local variables that must
         // be seen by the reading threads
void set_used_event(u16 event, std::memory_order order) {_used_event->store(event, order);};
@@ -240,7 +242,7 @@ class virtio_driver;
     private:

         // Up pointer
-        virtio_driver* _dev;
+        virtio_driver* _driver;
         u16 _q_index;
         // The physical of the physical address handed to the virtio device
         void* _vring_ptr;
diff --git a/drivers/virtio.cc b/drivers/virtio.cc
--- a/drivers/virtio.cc
+++ b/drivers/virtio.cc
@@ -12,58 +12,50 @@
 #include <osv/debug.h>
 #include "osv/trace.hh"

-using namespace pci;
-
TRACEPOINT(trace_virtio_wait_for_queue, "queue(%p) have_elements=%d", void*, int);

 namespace virtio {

 int virtio_driver::_disk_idx = 0;

-virtio_driver::virtio_driver(pci::device& dev)
+virtio_driver::virtio_driver(virtio_device& dev)
     : hw_driver()
     , _dev(dev)
-    , _msi(&dev)
     , _num_queues(0)
-    , _bar1(nullptr)
     , _cap_indirect_buf(false)
 {
     for (unsigned i = 0; i < max_virtqueues_nr; i++) {
         _queues[i] = nullptr;
     }
-    bool status = parse_pci_config();
-    assert(status == true);
-
-    _dev.set_bus_master(true);
-
-    _dev.msix_enable();

-    //make sure the queue is reset
-    reset_host_side();
+    // Initialize device
+    _dev.init();

-    // Acknowledge device
-    add_dev_status(VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER);
+    // Step 1 - make sure the device is reset
+    reset_device();

-    // Generic init of virtqueues
-    probe_virt_queues();
+    // Steps 2 & 3 - acknowledge device
+    add_dev_status(VIRTIO_CONFIG_S_ACKNOWLEDGE);
+    add_dev_status(VIRTIO_CONFIG_S_DRIVER);
 }

 virtio_driver::~virtio_driver()
 {
-    reset_host_side();
+    reset_device();
     free_queues();
 }

 void virtio_driver::setup_features()
 {
-    u32 dev_features = get_device_features();
-    u32 drv_features = this->get_driver_features();
+    // Step 4 - negotiate features
+    u64 dev_features = get_device_features();
+    u64 drv_features = this->get_driver_features();

-    u32 subset = dev_features & drv_features;
+    u64 subset = dev_features & drv_features;

     //notify the host about the features in used according
     //to the virtio spec
-    for (int i = 0; i < 32; i++)
+    for (int i = 0; i < 64; i++)
         if (subset & (1 << i))
virtio_d("%s: found feature intersec of bit %d", __FUNCTION__, i);

@@ -78,46 +70,14 @@ void virtio_driver::setup_features()

 void virtio_driver::dump_config()
 {
-    u8 B, D, F;
-    _dev.get_bdf(B, D, F);
-
     _dev.dump_config();
-    virtio_d("%s [%x:%x.%x] vid:id=%x:%x", get_name().c_str(),
-        (u16)B, (u16)D, (u16)F,
-        _dev.get_vendor_id(),
-        _dev.get_device_id());

     virtio_d("    virtio features: ");
-    for (int i = 0; i < 32; i++)
+    for (int i = 0; i < 64; i++)
         virtio_d(" %d ", get_device_feature_bit(i));
 }

-bool virtio_driver::parse_pci_config()
-{
-    // Test whether bar1 is present
-    _bar1 = _dev.get_bar(1);
-    if (_bar1 == nullptr) {
-        return false;
-    }
-
-    // Check ABI version
-    u8 rev = _dev.get_revision_id();
-    if (rev != VIRTIO_PCI_ABI_VERSION) {
-        virtio_e("Wrong virtio revision=%x", rev);
-        return false;
-    }
-
-    // Check device ID
-    u16 dev_id = _dev.get_device_id();
-    if ((dev_id < VIRTIO_PCI_ID_MIN) || (dev_id > VIRTIO_PCI_ID_MAX)) {
-        virtio_e("Wrong virtio dev id %x", dev_id);
-        return false;
-    }
-
-    return true;
-}
-
-void virtio_driver::reset_host_side()
+void virtio_driver::reset_device()
 {
     set_dev_status(0);
 }
@@ -134,7 +94,7 @@ void virtio_driver::free_queues()

 bool virtio_driver::kick(int queue)
 {
-    virtio_conf_writew(VIRTIO_PCI_QUEUE_NOTIFY, queue);
+    _dev.kick_queue(queue);
     return true;
 }

@@ -149,8 +109,8 @@ void virtio_driver::probe_virt_queues()
         }

         // Read queue size
-        virtio_conf_writew(VIRTIO_PCI_QUEUE_SEL, _num_queues);
-        qsize = virtio_conf_readw(VIRTIO_PCI_QUEUE_NUM);
+        _dev.select_queue(_num_queues);
+        qsize = _dev.get_queue_size();
         if (0 == qsize) {
             break;
         }
@@ -159,22 +119,10 @@ void virtio_driver::probe_virt_queues()
         vring* queue = new vring(this, qsize, _num_queues);
         _queues[_num_queues] = queue;

-        if (_dev.is_msix()) {
-            // Setup queue_id:entry_id 1:1 correlation...
-            virtio_conf_writew(VIRTIO_MSI_QUEUE_VECTOR, _num_queues);
- if (virtio_conf_readw(VIRTIO_MSI_QUEUE_VECTOR) != _num_queues) { - virtio_e("Setting MSIx entry for queue %d failed.", _num_queues);
-                return;
-            }
-        }
-
+        // Activate queue
+        _dev.setup_queue(queue);
         _num_queues++;

-        // Tell host about pfn
- // TODO: Yak, this is a bug in the design, on large memory we'll have PFNs > 32 bit
-        // Dor to notify Rusty
- virtio_conf_writel(VIRTIO_PCI_QUEUE_PFN, (u32)(queue->get_paddr()
VIRTIO_PCI_QUEUE_ADDR_SHIFT));
-
         // Debug print
virtio_d("Queue[%d] -> size %d, paddr %x", (_num_queues-1), qsize, queue->get_paddr());

@@ -212,82 +160,46 @@ void virtio_driver::wait_for_queue(vring* queue, bool (vring::*pred)() const)
     });
 }

-u32 virtio_driver::get_device_features()
+u64 virtio_driver::get_device_features()
 {
-    return virtio_conf_readl(VIRTIO_PCI_HOST_FEATURES);
+    return _dev.get_available_features();
 }

 bool virtio_driver::get_device_feature_bit(int bit)
 {
-    return get_virtio_config_bit(VIRTIO_PCI_HOST_FEATURES, bit);
+    return _dev.get_available_feature_bit(bit);
 }

-void virtio_driver::set_guest_features(u32 features)
+void virtio_driver::set_guest_features(u64 features)
 {
-    virtio_conf_writel(VIRTIO_PCI_GUEST_FEATURES, features);
-}
-
-void virtio_driver::set_guest_feature_bit(int bit, bool on)
-{
-    set_virtio_config_bit(VIRTIO_PCI_GUEST_FEATURES, bit, on);
-}
-
-u32 virtio_driver::get_guest_features()
-{
-    return virtio_conf_readl(VIRTIO_PCI_GUEST_FEATURES);
+    _dev.set_enabled_features(features);
 }

 bool virtio_driver::get_guest_feature_bit(int bit)
 {
-    return get_virtio_config_bit(VIRTIO_PCI_GUEST_FEATURES, bit);
+    return _dev.get_enabled_feature_bit(bit);
 }

-
 u8 virtio_driver::get_dev_status()
 {
-    return virtio_conf_readb(VIRTIO_PCI_STATUS);
+    return _dev.get_status();
 }

 void virtio_driver::set_dev_status(u8 status)
 {
-    virtio_conf_writeb(VIRTIO_PCI_STATUS, status);
+    _dev.set_status(status);
 }

 void virtio_driver::add_dev_status(u8 status)
 {
-    set_dev_status(get_dev_status() | status);
-}
-
-void virtio_driver::del_dev_status(u8 status)
-{
-    set_dev_status(get_dev_status() & ~status);
-}
-
-bool virtio_driver::get_virtio_config_bit(u32 offset, int bit)
-{
-    return virtio_conf_readl(offset) & (1 << bit);
-}
-
-void virtio_driver::set_virtio_config_bit(u32 offset, int bit, bool on)
-{
-    u32 val = virtio_conf_readl(offset);
-    u32 newval = ( val & ~(1 << bit) ) | ((int)(on)<<bit);
-    virtio_conf_writel(offset, newval);
-}
-
-void virtio_driver::virtio_conf_write(u32 offset, void* buf, int length)
-{
-    u8* ptr = reinterpret_cast<u8*>(buf);
-    for (int i = 0; i < length; i++)
-        _bar1->writeb(offset + i, ptr[i]);
+    _dev.set_status(get_dev_status() | status);
 }

 void virtio_driver::virtio_conf_read(u32 offset, void* buf, int length)
 {
     unsigned char* ptr = reinterpret_cast<unsigned char*>(buf);
     for (int i = 0; i < length; i++)
-        ptr[i] = _bar1->readb(offset + i);
+        ptr[i] = _dev.read_config(offset + i);
 }

 }
-
diff --git a/drivers/virtio.hh b/drivers/virtio.hh
--- a/drivers/virtio.hh
+++ b/drivers/virtio.hh
@@ -9,14 +9,9 @@
 #define VIRTIO_DRIVER_H

 #include "driver.hh"
-#include <osv/pci.hh>
 #include "drivers/driver.hh"
 #include "drivers/virtio-vring.hh"
-#include "drivers/pci-function.hh"
-#include "drivers/pci-device.hh"
-#include "drivers/virtio-vring.hh"
-#include <osv/interrupt.hh>
-#include <osv/msi.hh>
+#include "drivers/virtio-device.hh"

 namespace virtio {

@@ -42,52 +37,15 @@ enum VIRTIO_CONFIG {
     /* The Host publishes the avail index for which it expects a kick
* at the end of the used ring. Guest should ignore the used->flags field. */
     VIRTIO_RING_F_EVENT_IDX = 29,
-
+    /* Version bit that can be used to detect legacy vs modern devices */
+    VIRTIO_F_VERSION_1 = 32,
     /* Do we get callbacks when the ring is completely used, even if we've
      * suppressed them? */
     VIRTIO_F_NOTIFY_ON_EMPTY = 24,
-    /* A 32-bit r/o bitmask of the features supported by the host */
-    VIRTIO_PCI_HOST_FEATURES = 0,
-    /* A 32-bit r/w bitmask of features activated by the guest */
-    VIRTIO_PCI_GUEST_FEATURES = 4,
-    /* A 32-bit r/w PFN for the currently selected queue */
-    VIRTIO_PCI_QUEUE_PFN = 8,
-    /* A 16-bit r/o queue size for the currently selected queue */
-    VIRTIO_PCI_QUEUE_NUM = 12,
-    /* A 16-bit r/w queue selector */
-    VIRTIO_PCI_QUEUE_SEL = 14,
-    /* A 16-bit r/w queue notifier */
-    VIRTIO_PCI_QUEUE_NOTIFY = 16,
-    /* An 8-bit device status register.  */
-    VIRTIO_PCI_STATUS = 18,
- /* An 8-bit r/o interrupt status register. Reading the value will return the - * current contents of the ISR and will also clear it. This is effectively
-     * a read-and-acknowledge. */
-    VIRTIO_PCI_ISR = 19,
-    /* The bit of the ISR which indicates a device configuration change. */
-    VIRTIO_PCI_ISR_CONFIG  = 0x2,
-    /* MSI-X registers: only enabled if MSI-X is enabled. */
-    /* A 16-bit vector for configuration changes. */
-    VIRTIO_MSI_CONFIG_VECTOR = 20,
-    /* A 16-bit vector for selected queue notifications. */
-    VIRTIO_MSI_QUEUE_VECTOR = 22,
-    /* Vector value used to disable MSI for queue */
-    VIRTIO_MSI_NO_VECTOR = 0xffff,
-    /* Virtio ABI version, this must match exactly */
-    VIRTIO_PCI_ABI_VERSION = 0,
-    /* How many bits to shift physical queue address written to QUEUE_PFN.
-     * 12 is historical, and due to x86 page size. */
-    VIRTIO_PCI_QUEUE_ADDR_SHIFT = 12,
-    /* The alignment to use between consumer and producer parts of vring.
-     * x86 pagesize again. */
-    VIRTIO_PCI_VRING_ALIGN = 4096,
-
 };

 enum {
     VIRTIO_VENDOR_ID = 0x1af4,
-    VIRTIO_PCI_ID_MIN = 0x1000,
-    VIRTIO_PCI_ID_MAX = 0x103f,

     VIRTIO_ID_NET     = 1,
     VIRTIO_ID_BLOCK   = 2,
@@ -100,79 +58,56 @@ enum {
     VIRTIO_ID_RPROC_SERIAL = 11,
 };

-#define VIRTIO_ALIGN(x) ((x + (VIRTIO_PCI_VRING_ALIGN-1)) & ~(VIRTIO_PCI_VRING_ALIGN-1))
-
 const unsigned max_virtqueues_nr = 64;

 class virtio_driver : public hw_driver {
 public:
-    explicit virtio_driver(pci::device& dev);
+    explicit virtio_driver(virtio_device& dev);
     virtual ~virtio_driver();

     virtual std::string get_name() const = 0;

     virtual void dump_config();

-    // The remaining space is defined by each driver as the per-driver
-    // configuration space
- int virtio_pci_config_offset() {return (_dev.is_msix_enabled())? 24 : 20;}
-
-    bool parse_pci_config();
-
     void probe_virt_queues();
     vring* get_virt_queue(unsigned idx);

// block the calling thread until the queue has some used elements in it.
     void wait_for_queue(vring* queue, bool (vring::*pred)() const);

     // guest/host features physical access
-    u32 get_device_features();
+    u64 get_device_features();
     bool get_device_feature_bit(int bit);
-    void set_guest_features(u32 features);
-    void set_guest_feature_bit(int bit, bool on);
-    u32 get_guest_features();
+    void set_guest_features(u64 features);
     bool get_guest_feature_bit(int bit);

     // device status
     u8 get_dev_status();
     void set_dev_status(u8 status);
     void add_dev_status(u8 status);
-    void del_dev_status(u8 status);
-
-    // Access the virtio conf address space set by pci bar 1
-    bool get_virtio_config_bit(u32 offset, int bit);
-    void set_virtio_config_bit(u32 offset, int bit, bool on);

     // Access virtio config space
     void virtio_conf_read(u32 offset, void* buf, int length);
-    void virtio_conf_write(u32 offset, void* buf, int length);
-    u8 virtio_conf_readb(u32 offset) { return _bar1->readb(offset);};
-    u16 virtio_conf_readw(u32 offset) { return _bar1->readw(offset);};
-    u32 virtio_conf_readl(u32 offset) { return _bar1->readl(offset);};
- void virtio_conf_writeb(u32 offset, u8 val) { _bar1->writeb(offset, val);}; - void virtio_conf_writew(u32 offset, u16 val) { _bar1->writew(offset, val);}; - void virtio_conf_writel(u32 offset, u32 val) { _bar1->writel(offset, val);};

     bool kick(int queue);
-    void reset_host_side();
+    void reset_device();
     void free_queues();

     bool get_indirect_buf_cap() {return _cap_indirect_buf;}
     void set_indirect_buf_cap(bool on) {_cap_indirect_buf = on;}
     bool get_event_idx_cap() {return _cap_event_idx;}
     void set_event_idx_cap(bool on) {_cap_event_idx = on;}

-    pci::device& pci_device() { return _dev; }
+    size_t get_vring_alignment() { return _dev.get_vring_alignment();}
+
 protected:
// Actual drivers should implement this on top of the basic ring features virtual u32 get_driver_features() { return 1 << VIRTIO_RING_F_INDIRECT_DESC | 1 << VIRTIO_RING_F_EVENT_IDX; }
     void setup_features();
 protected:
-    pci::device& _dev;
-    interrupt_manager _msi;
+    virtio_device& _dev;
     vring* _queues[max_virtqueues_nr];
     u32 _num_queues;
-    pci::bar* _bar1;
     bool _cap_indirect_buf;
     bool _cap_event_idx = false;
     static int _disk_idx;
@@ -181,9 +116,9 @@ protected:
 template <typename T, u16 ID>
 hw_driver* probe(hw_device* dev)
 {
-    if (auto pci_dev = dynamic_cast<pci::device*>(dev)) {
-        if (pci_dev->get_id() == hw_device_id(VIRTIO_VENDOR_ID, ID)) {
-            return new T(*pci_dev);
+    if (auto virtio_dev = dynamic_cast<virtio_device*>(dev)) {
+        if (virtio_dev->get_id() == hw_device_id(VIRTIO_VENDOR_ID, ID)) {
+            return new T(*virtio_dev);
         }
     }
     return nullptr;
diff --git a/drivers/vmw-pvscsi.hh b/drivers/vmw-pvscsi.hh
--- a/drivers/vmw-pvscsi.hh
+++ b/drivers/vmw-pvscsi.hh
@@ -8,11 +8,13 @@
 #ifndef VMW_PVSCSI_DRIVER_H
 #define VMW_PVSCSI_DRIVER_H

-#include "drivers/virtio.hh"
+#include "drivers/driver.hh"
 #include "drivers/pci-device.hh"
 #include "drivers/scsi-common.hh"
 #include <osv/bio.h>
 #include <osv/types.h>
+#include <osv/interrupt.hh>
+#include <osv/msi.hh>

 namespace vmw {

--
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to