[PATCH 0/2] virtio-fs: negotiate as modern virtio driver

[Fotis Xenakis]

This is a minor fix to ensure virtio-fs operation, which broke due to a
recent change in QEMU. See the second commit for more details.

Fotis Xenakis (2):
virtio: fix return type of get_driver_features()
virtio-fs: set the VIRTIO_F_VERSION_1 feature bit

drivers/virtio-blk.cc | 2 +-
drivers/virtio-blk.hh | 2 +-
drivers/virtio-fs.cc | 7 +++++++
drivers/virtio-fs.hh | 2 ++
drivers/virtio-net.cc | 4 ++--
drivers/virtio-net.hh | 2 +-
drivers/virtio-scsi.cc | 2 +-
drivers/virtio-scsi.hh | 2 +-
drivers/virtio.hh | 2 +-
9 files changed, 17 insertions(+), 8 deletions(-)

--
2.29.2

[Fotis Xenakis]

The virtio spec defines the features field to be 64 bits. The lower
parts of the virtio drivers (virtio::virtio_driver) treated it as such,
but the interface for each specific driver to override the feature bits
(virtio::virtio_driver::get_driver_features()) limited that to 32 bits
only. This patch rectifies it, changing the function's signature and all
overrides in the various drivers.

Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
---

drivers/virtio-blk.cc | 2 +-
drivers/virtio-blk.hh | 2 +-

drivers/virtio-net.cc | 4 ++--
drivers/virtio-net.hh | 2 +-
drivers/virtio-scsi.cc | 2 +-
drivers/virtio-scsi.hh | 2 +-
drivers/virtio.hh | 2 +-

7 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/virtio-blk.cc b/drivers/virtio-blk.cc
index 8845a093..4740c8ee 100644
--- a/drivers/virtio-blk.cc
+++ b/drivers/virtio-blk.cc
@@ -333,7 +333,7 @@ int blk::make_request(struct bio* bio)
}
}

-u32 blk::get_driver_features()
+u64 blk::get_driver_features()
{
auto base = virtio_driver::get_driver_features();
return (base | ( 1 << VIRTIO_BLK_F_SIZE_MAX)
diff --git a/drivers/virtio-blk.hh b/drivers/virtio-blk.hh
index 00e679d3..072df16d 100644
--- a/drivers/virtio-blk.hh
+++ b/drivers/virtio-blk.hh
@@ -125,7 +125,7 @@ public:
virtual std::string get_name() const { return _driver_name; }
void read_config();

- virtual u32 get_driver_features();
+ virtual u64 get_driver_features();

int make_request(struct bio*);

diff --git a/drivers/virtio-net.cc b/drivers/virtio-net.cc
index 65265ca7..c31ae64b 100644
--- a/drivers/virtio-net.cc
+++ b/drivers/virtio-net.cc
@@ -900,9 +900,9 @@ void net::txq::gc()
vqueue->get_buf_gc();
}

-u32 net::get_driver_features()
+u64 net::get_driver_features()
{
- u32 base = virtio_driver::get_driver_features();
+ auto base = virtio_driver::get_driver_features();
return (base | (1 << VIRTIO_NET_F_MAC) \
| (1 << VIRTIO_NET_F_MRG_RXBUF) \
| (1 << VIRTIO_NET_F_STATUS) \
diff --git a/drivers/virtio-net.hh b/drivers/virtio-net.hh
index be801ce0..ccd421f1 100644
--- a/drivers/virtio-net.hh
+++ b/drivers/virtio-net.hh
@@ -216,7 +216,7 @@ public:
virtual std::string get_name() const { return _driver_name; }
void read_config();

- virtual u32 get_driver_features();
+ virtual u64 get_driver_features();

void wait_for_queue(vring* queue);
bool bad_rx_csum(struct mbuf* m, struct net_hdr* hdr);
diff --git a/drivers/virtio-scsi.cc b/drivers/virtio-scsi.cc
index 4fd89c24..a9617f07 100644
--- a/drivers/virtio-scsi.cc
+++ b/drivers/virtio-scsi.cc
@@ -246,7 +246,7 @@ int scsi::make_request(struct bio* bio)
}
}

-u32 scsi::get_driver_features()
+u64 scsi::get_driver_features()
{
auto base = virtio_driver::get_driver_features();
return base | ( 1 << VIRTIO_SCSI_F_INOUT);
diff --git a/drivers/virtio-scsi.hh b/drivers/virtio-scsi.hh
index de93129b..d9d6737f 100644
--- a/drivers/virtio-scsi.hh
+++ b/drivers/virtio-scsi.hh
@@ -149,7 +149,7 @@ public:
virtual std::string get_name() const { return _driver_name; }
void read_config();

- virtual u32 get_driver_features();
+ virtual u64 get_driver_features();

static struct scsi_priv *get_priv(struct bio *bio) {
return reinterpret_cast<struct scsi_priv*>(bio->bio_dev->private_data);
diff --git a/drivers/virtio.hh b/drivers/virtio.hh
index aa832c51..c9103b84 100644
--- a/drivers/virtio.hh
+++ b/drivers/virtio.hh
@@ -105,7 +105,7 @@ public:

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; }
+ virtual u64 get_driver_features() { return 1 << VIRTIO_RING_F_INDIRECT_DESC | 1 << VIRTIO_RING_F_EVENT_IDX; }
void setup_features();
protected:
virtio_device& _dev;
--
2.29.2

[Fotis Xenakis]

The virtio-fs is a modern virtio-only device in its current QEMU
implementation. Until now, we failed to set the corresponding
VIRTIO_F_VERSION_1 feauture bit in the driver, but the device did not
complain. But as of QEMU commit 2ffc54708087c6e524297957be2fc5d543abb767
this changed and virtiofsd fails if this feature bit is not offered by
the driver. This patches the driver to offer said feature bit.

Signed-off-by: Fotis Xenakis <fo...@windowslive.com>
---

drivers/virtio-fs.cc | 7 +++++++
drivers/virtio-fs.hh | 2 ++

2 files changed, 9 insertions(+)

diff --git a/drivers/virtio-fs.cc b/drivers/virtio-fs.cc
index a8a81265..673bb3c7 100644
--- a/drivers/virtio-fs.cc
+++ b/drivers/virtio-fs.cc
@@ -130,6 +130,7 @@ fs::fs(virtio_device& virtio_dev)

// Step 8
add_dev_status(VIRTIO_CONFIG_S_DRIVER_OK);
+ // sleep(10);

// TODO: Don't ignore the virtio-fs tag and use that instead of _id for
// identifying the device (e.g. something like /dev/virtiofs/<tag> or at
@@ -207,6 +208,12 @@ int fs::make_request(fuse_request& req)
return 0;
}

+u64 fs::get_driver_features()
+{

+ auto base = virtio_driver::get_driver_features();

+ return base | ((u64)1 << VIRTIO_F_VERSION_1);
+}
+
hw_driver* fs::probe(hw_device* dev)
{
return virtio::probe<fs, VIRTIO_ID_FS>(dev);
diff --git a/drivers/virtio-fs.hh b/drivers/virtio-fs.hh
index 0505a9b0..644827ac 100644
--- a/drivers/virtio-fs.hh
+++ b/drivers/virtio-fs.hh
@@ -80,6 +80,8 @@ public:

virtual std::string get_name() const { return _driver_name; }
void read_config();

+ virtual u64 get_driver_features();
+
int make_request(fuse_request&);
dax_window* get_dax() {
return (_dax.addr != mmio_nullptr) ? &_dax : nullptr;
--
2.29.2

[Commit Bot]

From: Fotis Xenakis <fo...@windowslive.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

virtio: fix return type of get_driver_features()

The virtio spec defines the features field to be 64 bits. The lower
parts of the virtio drivers (virtio::virtio_driver) treated it as such,
but the interface for each specific driver to override the feature bits
(virtio::virtio_driver::get_driver_features()) limited that to 32 bits
only. This patch rectifies it, changing the function's signature and all
overrides in the various drivers.

Signed-off-by: Fotis Xenakis <fo...@windowslive.com>

Message-Id: <AM0PR03MB62923C5820...@AM0PR03MB6292.eurprd03.prod.outlook.com>

---
diff --git a/drivers/virtio-blk.cc b/drivers/virtio-blk.cc

--- a/drivers/virtio-blk.cc
+++ b/drivers/virtio-blk.cc
@@ -333,7 +333,7 @@ int blk::make_request(struct bio* bio)
}
}

-u32 blk::get_driver_features()
+u64 blk::get_driver_features()
{
auto base = virtio_driver::get_driver_features();
return (base | ( 1 << VIRTIO_BLK_F_SIZE_MAX)
diff --git a/drivers/virtio-blk.hh b/drivers/virtio-blk.hh

--- a/drivers/virtio-blk.hh
+++ b/drivers/virtio-blk.hh
@@ -125,7 +125,7 @@ public:
virtual std::string get_name() const { return _driver_name; }
void read_config();

- virtual u32 get_driver_features();
+ virtual u64 get_driver_features();

int make_request(struct bio*);

diff --git a/drivers/virtio-net.cc b/drivers/virtio-net.cc

--- a/drivers/virtio-net.cc
+++ b/drivers/virtio-net.cc
@@ -900,9 +900,9 @@ void net::txq::gc()
vqueue->get_buf_gc();
}

-u32 net::get_driver_features()
+u64 net::get_driver_features()
{
- u32 base = virtio_driver::get_driver_features();
+ auto base = virtio_driver::get_driver_features();
return (base | (1 << VIRTIO_NET_F_MAC) \
| (1 << VIRTIO_NET_F_MRG_RXBUF) \
| (1 << VIRTIO_NET_F_STATUS) \
diff --git a/drivers/virtio-net.hh b/drivers/virtio-net.hh

--- a/drivers/virtio-net.hh
+++ b/drivers/virtio-net.hh
@@ -216,7 +216,7 @@ public:
virtual std::string get_name() const { return _driver_name; }
void read_config();

- virtual u32 get_driver_features();
+ virtual u64 get_driver_features();

void wait_for_queue(vring* queue);
bool bad_rx_csum(struct mbuf* m, struct net_hdr* hdr);
diff --git a/drivers/virtio-scsi.cc b/drivers/virtio-scsi.cc

--- a/drivers/virtio-scsi.cc
+++ b/drivers/virtio-scsi.cc
@@ -246,7 +246,7 @@ int scsi::make_request(struct bio* bio)
}
}

-u32 scsi::get_driver_features()
+u64 scsi::get_driver_features()
{
auto base = virtio_driver::get_driver_features();
return base | ( 1 << VIRTIO_SCSI_F_INOUT);
diff --git a/drivers/virtio-scsi.hh b/drivers/virtio-scsi.hh

--- a/drivers/virtio-scsi.hh
+++ b/drivers/virtio-scsi.hh
@@ -149,7 +149,7 @@ public:
virtual std::string get_name() const { return _driver_name; }
void read_config();

- virtual u32 get_driver_features();
+ virtual u64 get_driver_features();

static struct scsi_priv *get_priv(struct bio *bio) {
return reinterpret_cast<struct scsi_priv*>(bio->bio_dev->private_data);
diff --git a/drivers/virtio.hh b/drivers/virtio.hh

[Commit Bot]

From: Fotis Xenakis <fo...@windowslive.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

virtio-fs: set the VIRTIO_F_VERSION_1 feature bit

The virtio-fs is a modern virtio-only device in its current QEMU
implementation. Until now, we failed to set the corresponding
VIRTIO_F_VERSION_1 feauture bit in the driver, but the device did not
complain. But as of QEMU commit 2ffc54708087c6e524297957be2fc5d543abb767
this changed and virtiofsd fails if this feature bit is not offered by
the driver. This patches the driver to offer said feature bit.

Signed-off-by: Fotis Xenakis <fo...@windowslive.com>

Message-Id: <AM0PR03MB6292D10F4C...@AM0PR03MB6292.eurprd03.prod.outlook.com>

---
diff --git a/drivers/virtio-fs.cc b/drivers/virtio-fs.cc

--- a/drivers/virtio-fs.cc
+++ b/drivers/virtio-fs.cc
@@ -130,6 +130,7 @@ fs::fs(virtio_device& virtio_dev)

// Step 8
add_dev_status(VIRTIO_CONFIG_S_DRIVER_OK);
+ // sleep(10);

// TODO: Don't ignore the virtio-fs tag and use that instead of _id for
// identifying the device (e.g. something like /dev/virtiofs/<tag> or at
@@ -207,6 +208,12 @@ int fs::make_request(fuse_request& req)
return 0;
}

+u64 fs::get_driver_features()
+{
+ auto base = virtio_driver::get_driver_features();
+ return base | ((u64)1 << VIRTIO_F_VERSION_1);
+}
+
hw_driver* fs::probe(hw_device* dev)
{
return virtio::probe<fs, VIRTIO_ID_FS>(dev);
diff --git a/drivers/virtio-fs.hh b/drivers/virtio-fs.hh