From 7adc90a2970dfdc54560305ddca46c90ce519016 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Mon, 28 Oct 2019 21:07:16 -0700 Subject: [PATCH] dev: Make the virtio devices track endianness explicitly. These classes now track what endianness they're supposed to use explicitly, initially set by the getGuestByteOrder accessor on the system object. In the future, if the endianness depends on the version of the VirtIO spec as the comment suggest, it will be easier to dynamically set the endianness in the various structures based on the version being used, Since there isn't anything special about the virt IO versions of these converters other than their types, and since the endianness conversion infrastructure can be taught how to convert new types, the code was switched over to using the standard htog and gtoh but with the explicit byte order provided. This also gets rid of the final use of TheISA in the dev directory. Change-Id: I9345e3295eb27fc5eb87e8ce0d8d424ad1e75d2d Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/22273 Reviewed-by: Andreas Sandberg Maintainer: Andreas Sandberg Tested-by: kokoro --- src/dev/virtio/base.cc | 19 ++++---- src/dev/virtio/base.hh | 95 +++++++++++++++++++-------------------- src/dev/virtio/block.cc | 9 ++-- src/dev/virtio/block.hh | 5 ++- src/dev/virtio/console.cc | 8 ++-- src/dev/virtio/console.hh | 10 +++-- src/dev/virtio/fs9p.cc | 4 +- src/dev/virtio/fs9p.hh | 5 ++- 8 files changed, 79 insertions(+), 76 deletions(-) diff --git a/src/dev/virtio/base.cc b/src/dev/virtio/base.cc index 347f5089f..e607769c2 100644 --- a/src/dev/virtio/base.cc +++ b/src/dev/virtio/base.cc @@ -42,10 +42,11 @@ #include "debug/VIO.hh" #include "params/VirtIODeviceBase.hh" #include "params/VirtIODummyDevice.hh" +#include "sim/system.hh" -VirtDescriptor::VirtDescriptor(PortProxy &_memProxy, VirtQueue &_queue, - Index descIndex) - : memProxy(&_memProxy), queue(&_queue), _index(descIndex), +VirtDescriptor::VirtDescriptor(PortProxy &_memProxy, ByteOrder bo, + VirtQueue &_queue, Index descIndex) + : memProxy(&_memProxy), queue(&_queue), byteOrder(bo), _index(descIndex), desc{0, 0, 0, 0} { } @@ -64,6 +65,7 @@ VirtDescriptor::operator=(VirtDescriptor &&rhs) noexcept { memProxy = std::move(rhs.memProxy); queue = std::move(rhs.queue); + byteOrder = std::move(rhs.byteOrder); _index = std::move(rhs._index); desc = std::move(rhs.desc); @@ -82,7 +84,7 @@ VirtDescriptor::update() const Addr desc_addr(vq_addr + sizeof(desc) * _index); vring_desc guest_desc; memProxy->readBlob(desc_addr, &guest_desc, sizeof(guest_desc)); - desc = vtoh_legacy(guest_desc); + desc = gtoh(guest_desc, byteOrder); DPRINTF(VIO, "VirtDescriptor(%i): Addr: 0x%x, Len: %i, Flags: 0x%x, " "Next: 0x%x\n", @@ -224,14 +226,14 @@ VirtDescriptor::chainSize() const -VirtQueue::VirtQueue(PortProxy &proxy, uint16_t size) - : _size(size), _address(0), memProxy(proxy), - avail(proxy, size), used(proxy, size), +VirtQueue::VirtQueue(PortProxy &proxy, ByteOrder bo, uint16_t size) + : byteOrder(bo), _size(size), _address(0), memProxy(proxy), + avail(proxy, bo, size), used(proxy, bo, size), _last_avail(0) { descriptors.reserve(_size); for (int i = 0; i < _size; ++i) - descriptors.emplace_back(proxy, *this, i); + descriptors.emplace_back(proxy, bo, *this, i); } void @@ -326,6 +328,7 @@ VirtIODeviceBase::VirtIODeviceBase(Params *params, DeviceId id, size_t config_size, FeatureBits features) : SimObject(params), guestFeatures(0), + byteOrder(params->system->getGuestByteOrder()), deviceId(id), configSize(config_size), deviceFeatures(features), _deviceStatus(0), _queueSelect(0), transKick(NULL) diff --git a/src/dev/virtio/base.hh b/src/dev/virtio/base.hh index f4c85afec..a0acb9259 100644 --- a/src/dev/virtio/base.hh +++ b/src/dev/virtio/base.hh @@ -65,48 +65,20 @@ class VirtQueue; * of byte swapping. */ -/** Convert legacy VirtIO endianness to host endianness. */ -template inline T -vtoh_legacy(T v) { - return TheISA::gtoh(v); -} - -/** Convert host endianness to legacy VirtIO endianness. */ -template inline T -htov_legacy(T v) { - return TheISA::htog(v); -} - template <> inline vring_used_elem -vtoh_legacy(vring_used_elem v) { - v.id = vtoh_legacy(v.id); - v.len = vtoh_legacy(v.len); - return v; -} - -template <> inline vring_used_elem -htov_legacy(vring_used_elem v) { - v.id = htov_legacy(v.id); - v.len = htov_legacy(v.len); +swap_byte(vring_used_elem v) { + v.id = swap_byte(v.id); + v.len = swap_byte(v.len); return v; } template <> inline vring_desc -vtoh_legacy(vring_desc v) { - v.addr = vtoh_legacy(v.addr); - v.len = vtoh_legacy(v.len); - v.flags = vtoh_legacy(v.flags); - v.next = vtoh_legacy(v.next); - return v; -} - -template <> inline vring_desc -htov_legacy(vring_desc v) { - v.addr = htov_legacy(v.addr); - v.len = htov_legacy(v.len); - v.flags = htov_legacy(v.flags); - v.next = htov_legacy(v.next); +swap_byte(vring_desc v) { + v.addr = swap_byte(v.addr); + v.len = swap_byte(v.len); + v.flags = swap_byte(v.flags); + v.next = swap_byte(v.next); return v; } @@ -149,7 +121,8 @@ class VirtDescriptor * @param queue Queue owning this descriptor. * @param index Index within the queue. */ - VirtDescriptor(PortProxy &memProxy, VirtQueue &queue, Index index); + VirtDescriptor(PortProxy &memProxy, ByteOrder bo, + VirtQueue &queue, Index index); // WORKAROUND: The noexcept declaration works around a bug where // gcc 4.7 tries to call the wrong constructor when emplacing // something into a vector. @@ -298,6 +271,9 @@ class VirtDescriptor /** Pointer to virtqueue owning this descriptor */ VirtQueue *queue; + /** The byte order the descriptor is stored in. */ + ByteOrder byteOrder; + /** Index in virtqueue */ Index _index; @@ -447,7 +423,10 @@ public: * @param proxy Proxy to the guest physical memory. * @param size Size in descriptors/pages. */ - VirtQueue(PortProxy &proxy, uint16_t size); + VirtQueue(PortProxy &proxy, ByteOrder bo, uint16_t size); + + /** Byte order in this queue */ + ByteOrder byteOrder; private: VirtQueue(); @@ -479,8 +458,9 @@ public: Index index; } M5_ATTR_PACKED; - VirtRing(PortProxy &proxy, uint16_t size) - : header{0, 0}, ring(size), _proxy(proxy), _base(0) {} + VirtRing(PortProxy &proxy, ByteOrder bo, uint16_t size) : + header{0, 0}, ring(size), _proxy(proxy), _base(0), byteOrder(bo) + {} /** * Set the base address of the VirtIO ring buffer. @@ -490,22 +470,28 @@ public: void setAddress(Addr addr) { _base = addr; } /** Update the ring buffer header with data from the guest. */ - void readHeader() { + void + readHeader() + { assert(_base != 0); _proxy.readBlob(_base, &header, sizeof(header)); - header.flags = vtoh_legacy(header.flags); - header.index = vtoh_legacy(header.index); + header.flags = gtoh(header.flags, byteOrder); + header.index = gtoh(header.index, byteOrder); } - void writeHeader() { + void + writeHeader() + { Header out; assert(_base != 0); - out.flags = htov_legacy(header.flags); - out.index = htov_legacy(header.index); + out.flags = htog(header.flags, byteOrder); + out.index = htog(header.index, byteOrder); _proxy.writeBlob(_base, &out, sizeof(out)); } - void read() { + void + read() + { readHeader(); /* Read and byte-swap the elements in the ring */ @@ -513,16 +499,18 @@ public: _proxy.readBlob(_base + sizeof(header), temp, sizeof(T) * ring.size()); for (int i = 0; i < ring.size(); ++i) - ring[i] = vtoh_legacy(temp[i]); + ring[i] = gtoh(temp[i], byteOrder); } - void write() { + void + write() + { assert(_base != 0); /* Create a byte-swapped copy of the ring and write it to * guest memory. */ T temp[ring.size()]; for (int i = 0; i < ring.size(); ++i) - temp[i] = htov_legacy(ring[i]); + temp[i] = htog(ring[i], byteOrder); _proxy.writeBlob(_base + sizeof(header), temp, sizeof(T) * ring.size()); writeHeader(); @@ -541,6 +529,8 @@ public: PortProxy &_proxy; /** Guest physical base address of the ring buffer */ Addr _base; + /** Byte order in the ring */ + ByteOrder byteOrder; }; /** Ring of available (incoming) descriptors */ @@ -722,6 +712,11 @@ class VirtIODeviceBase : public SimObject */ void writeConfigBlob(PacketPtr pkt, Addr cfgOffset, uint8_t *cfg); + /** + * The byte order of the queues, descriptors, etc. + */ + ByteOrder byteOrder; + /** @} */ public: diff --git a/src/dev/virtio/block.cc b/src/dev/virtio/block.cc index cf7f01bf6..6e1998aaa 100644 --- a/src/dev/virtio/block.cc +++ b/src/dev/virtio/block.cc @@ -45,7 +45,8 @@ VirtIOBlock::VirtIOBlock(Params *params) : VirtIODeviceBase(params, ID_BLOCK, sizeof(Config), 0), - qRequests(params->system->physProxy, params->queueSize, *this), + qRequests(params->system->physProxy, byteOrder, + params->queueSize, *this), image(*params->image) { registerQueue(qRequests); @@ -62,7 +63,7 @@ void VirtIOBlock::readConfig(PacketPtr pkt, Addr cfgOffset) { Config cfg_out; - cfg_out.capacity = htov_legacy(config.capacity); + cfg_out.capacity = htog(config.capacity, byteOrder); readConfigBlob(pkt, cfgOffset, (uint8_t *)&cfg_out); } @@ -132,8 +133,8 @@ VirtIOBlock::RequestQueue::onNotifyDescriptor(VirtDescriptor *desc) */ BlkRequest req; desc->chainRead(0, (uint8_t *)&req, sizeof(req)); - req.type = htov_legacy(req.type); - req.sector = htov_legacy(req.sector); + req.type = htog(req.type, byteOrder); + req.sector = htog(req.sector, byteOrder); Status status; const size_t data_size(desc->chainSize() diff --git a/src/dev/virtio/block.hh b/src/dev/virtio/block.hh index be88bd035..419f24bf3 100644 --- a/src/dev/virtio/block.hh +++ b/src/dev/virtio/block.hh @@ -163,8 +163,9 @@ class VirtIOBlock : public VirtIODeviceBase : public VirtQueue { public: - RequestQueue(PortProxy &proxy, uint16_t size, VirtIOBlock &_parent) - : VirtQueue(proxy, size), parent(_parent) {} + RequestQueue(PortProxy &proxy, ByteOrder bo, + uint16_t size, VirtIOBlock &_parent) + : VirtQueue(proxy, bo, size), parent(_parent) {} virtual ~RequestQueue() {} void onNotifyDescriptor(VirtDescriptor *desc); diff --git a/src/dev/virtio/console.cc b/src/dev/virtio/console.cc index 1027925bb..896318853 100644 --- a/src/dev/virtio/console.cc +++ b/src/dev/virtio/console.cc @@ -45,8 +45,8 @@ VirtIOConsole::VirtIOConsole(Params *params) : VirtIODeviceBase(params, ID_CONSOLE, sizeof(Config), F_SIZE), - qRecv(params->system->physProxy, params->qRecvSize, *this), - qTrans(params->system->physProxy, params->qTransSize, *this), + qRecv(params->system->physProxy, byteOrder, params->qRecvSize, *this), + qTrans(params->system->physProxy, byteOrder, params->qTransSize, *this), device(*params->device), callbackDataAvail(qRecv) { registerQueue(qRecv); @@ -66,8 +66,8 @@ void VirtIOConsole::readConfig(PacketPtr pkt, Addr cfgOffset) { Config cfg_out; - cfg_out.rows = htov_legacy(config.rows); - cfg_out.cols = htov_legacy(config.cols); + cfg_out.rows = htog(config.rows, byteOrder); + cfg_out.cols = htog(config.cols, byteOrder); readConfigBlob(pkt, cfgOffset, (uint8_t *)&cfg_out); } diff --git a/src/dev/virtio/console.hh b/src/dev/virtio/console.hh index cd044b12e..ebc83bff5 100644 --- a/src/dev/virtio/console.hh +++ b/src/dev/virtio/console.hh @@ -108,8 +108,9 @@ class VirtIOConsole : public VirtIODeviceBase : public VirtQueue { public: - TermRecvQueue(PortProxy &proxy, uint16_t size, VirtIOConsole &_parent) - : VirtQueue(proxy, size), parent(_parent) {} + TermRecvQueue(PortProxy &proxy, ByteOrder bo, + uint16_t size, VirtIOConsole &_parent) + : VirtQueue(proxy, bo, size), parent(_parent) {} virtual ~TermRecvQueue() {} void onNotify() { trySend(); } @@ -132,8 +133,9 @@ class VirtIOConsole : public VirtIODeviceBase : public VirtQueue { public: - TermTransQueue(PortProxy &proxy, uint16_t size, VirtIOConsole &_parent) - : VirtQueue(proxy, size), parent(_parent) {} + TermTransQueue(PortProxy &proxy, ByteOrder bo, + uint16_t size, VirtIOConsole &_parent) + : VirtQueue(proxy, bo, size), parent(_parent) {} virtual ~TermTransQueue() {} void onNotifyDescriptor(VirtDescriptor *desc); diff --git a/src/dev/virtio/fs9p.cc b/src/dev/virtio/fs9p.cc index 7857feafc..87333b6dc 100644 --- a/src/dev/virtio/fs9p.cc +++ b/src/dev/virtio/fs9p.cc @@ -118,11 +118,11 @@ VirtIO9PBase::VirtIO9PBase(Params *params) : VirtIODeviceBase(params, ID_9P, sizeof(Config) + params->tag.size(), F_MOUNT_TAG), - queue(params->system->physProxy, params->queueSize, *this) + queue(params->system->physProxy, byteOrder, params->queueSize, *this) { config.reset((Config *) operator new(configSize)); - config->len = htov_legacy(params->tag.size()); + config->len = htog(params->tag.size(), byteOrder); memcpy(config->tag, params->tag.c_str(), params->tag.size()); registerQueue(queue); diff --git a/src/dev/virtio/fs9p.hh b/src/dev/virtio/fs9p.hh index 46525603d..1d9fbb9c0 100644 --- a/src/dev/virtio/fs9p.hh +++ b/src/dev/virtio/fs9p.hh @@ -147,8 +147,9 @@ class VirtIO9PBase : public VirtIODeviceBase class FSQueue : public VirtQueue { public: - FSQueue(PortProxy &proxy, uint16_t size, VirtIO9PBase &_parent) - : VirtQueue(proxy, size), parent(_parent) {} + FSQueue(PortProxy &proxy, ByteOrder bo, + uint16_t size, VirtIO9PBase &_parent) + : VirtQueue(proxy, bo, size), parent(_parent) {} virtual ~FSQueue() {} void onNotifyDescriptor(VirtDescriptor *desc); -- 2.30.2