From adc594a7e6ac097249b8133f25b949fb14c23700 Mon Sep 17 00:00:00 2001 From: Stefan Kalkowski Date: Mon, 20 Feb 2023 15:58:24 +0100 Subject: [PATCH] os: remove conversion warnings/errors from virtio The read_config and write_config functions in the generic virtio headers used by all drivers lead to compiler warnings resp. errors if effective-c++ switch is enabled. Moreover, the functions require to define the access width as parameter. We can better turn them into template functions using the value type to read resp. write to derive the access width. Ref genodelabs/genode#4344 --- repos/os/include/virtio/mmio_device.h | 27 ++++++------- repos/os/include/virtio/pci_device.h | 39 +++++++------------ repos/os/include/virtio/queue.h | 2 +- .../drivers/framebuffer/virtio/component.h | 9 ++--- repos/os/src/drivers/input/virtio/component.h | 24 ++++++------ repos/os/src/drivers/nic/virtio/component.h | 11 +++--- .../src/drivers/nic/virtio/spec/x86/target.mk | 2 - .../os/src/drivers/nic/virtio/target_mmio.inc | 2 - 8 files changed, 45 insertions(+), 71 deletions(-) diff --git a/repos/os/include/virtio/mmio_device.h b/repos/os/include/virtio/mmio_device.h index 0a630fe47d..25c1ce78cf 100644 --- a/repos/os/include/virtio/mmio_device.h +++ b/repos/os/include/virtio/mmio_device.h @@ -86,9 +86,9 @@ class Virtio::Device : Platform::Device::Mmio * VIRTIO 1.0 spec 64 bit wide registers are supposed to be read as * two 32 bit values. */ - struct Config_8 : Register_array<0x100, 8, 256, 8> { }; - struct Config_16 : Register_array<0x100, 16, 128, 16> { }; - struct Config_32 : Register_array<0x100, 32, 64, 32> { }; + template class Config : + public Register_array<0x100, sizeof(T)*8, + 256/sizeof(T), sizeof(T)*8> {}; /* * Noncopyable @@ -141,23 +141,18 @@ class Virtio::Device : Platform::Device::Mmio return (uint16_t)read(); } - uint32_t read_config(uint8_t offset, Access_size size) + template + T read_config(const uint8_t offset) { - switch (size) { - case ACCESS_8BIT: return read(offset); - case ACCESS_16BIT: return read(offset >> 1); - case ACCESS_32BIT: return read(offset >> 2); - } - return 0; + static_assert(sizeof(T) <= 4); + return read>(offset >> log2(sizeof(T))); } - void write_config(uint8_t offset, Access_size size, uint32_t value) + template + void write_config(const uint8_t offset, const T value) { - switch (size) { - case ACCESS_8BIT: write ((uint8_t) value, offset); break; - case ACCESS_16BIT: write((uint16_t)value, (offset >> 1)); break; - case ACCESS_32BIT: write(value, (offset >> 2)); break; - } + static_assert(sizeof(T) <= 4); + write>(value, (offset >> log2(sizeof(T)))); } bool configure_queue(uint16_t queue_index, diff --git a/repos/os/include/virtio/pci_device.h b/repos/os/include/virtio/pci_device.h index 088e494ace..bfa54e2273 100644 --- a/repos/os/include/virtio/pci_device.h +++ b/repos/os/include/virtio/pci_device.h @@ -45,11 +45,11 @@ struct Virtio::Device_mmio : public Genode::Mmio struct QueueUsedLow : Register<0x30, 32> { }; struct QueueUsedHigh : Register<0x34, 32> { }; - struct Config_8 : Register_array<0x0, 8, 256, 8> { }; - struct Config_16 : Register_array<0x0, 16, 128, 16> { }; - struct Config_32 : Register_array<0x0, 32, 64, 32> { }; + template class Config : + public Register_array<0x0, sizeof(T)*8, + 256/sizeof(T), sizeof(T)*8> {}; - struct IrqReason : Register<0x0, 32> { }; + struct IrqReason : Register<0x0, 32> { }; using Mmio::Mmio; }; @@ -184,32 +184,19 @@ class Virtio::Device return _cfg_common.read(); } - uint32_t read_config(uint8_t offset, Access_size size) + template + T read_config(const uint8_t offset) { - switch (size) { - case Device::ACCESS_8BIT: - return _dev_config.read(offset); - case Device::ACCESS_16BIT: - return _dev_config.read(offset >> 1); - case Device::ACCESS_32BIT: - return _dev_config.read(offset >> 2); - } - return 0; + static_assert(sizeof(T) <= 4); + return _dev_config.read>(offset >> log2(sizeof(T))); } - void write_config(uint8_t offset, Access_size size, uint32_t value) + template + void write_config(const uint8_t offset, const T value) { - switch (size) { - case Device::ACCESS_8BIT: - _dev_config.write(value, offset); - break; - case Device::ACCESS_16BIT: - _dev_config.write(value, offset >> 1); - break; - case Device::ACCESS_32BIT: - _dev_config.write(value, offset >> 2); - break; - } + static_assert(sizeof(T) <= 4); + _dev_config.write>(value, + (offset >> log2(sizeof(T)))); } bool configure_queue(uint16_t queue_index, Virtio::Queue_description desc) diff --git a/repos/os/include/virtio/queue.h b/repos/os/include/virtio/queue.h index 529dee2ddf..d7253e205e 100644 --- a/repos/os/include/virtio/queue.h +++ b/repos/os/include/virtio/queue.h @@ -413,7 +413,7 @@ class Virtio::Queue static_assert(!TRAITS::device_write_only); static_assert(TRAITS::has_data_payload); - int const req_desc_count = 1 + (sizeof(header) + data_size) / _buffers.buffer_size(); + size_t const req_desc_count = 1UL + (sizeof(header) + data_size) / _buffers.buffer_size(); if (req_desc_count > _descriptors.available_capacity()) return false; diff --git a/repos/os/src/drivers/framebuffer/virtio/component.h b/repos/os/src/drivers/framebuffer/virtio/component.h index 93194bf005..ddf88ee843 100644 --- a/repos/os/src/drivers/framebuffer/virtio/component.h +++ b/repos/os/src/drivers/framebuffer/virtio/component.h @@ -226,8 +226,7 @@ class Virtio_fb::Driver uint32_t num_scanouts; uint32_t before = 0, after = 0; do { - num_scanouts = device.read_config( - Config::NUM_SCANOUTS, Virtio::Device::ACCESS_32BIT); + num_scanouts = device.read_config(Config::NUM_SCANOUTS); } while (after != before); return num_scanouts; } @@ -237,12 +236,10 @@ class Virtio_fb::Driver uint32_t events; uint32_t before = 0, after = 0; do { - events = _device.read_config( - Config::EVENTS_READ, Virtio::Device::ACCESS_32BIT); + events = _device.read_config(Config::EVENTS_READ); } while (after != before); do { - _device.write_config( - Config::EVENTS_CLEAR, Virtio::Device::ACCESS_32BIT, events); + _device.write_config(Config::EVENTS_CLEAR, events); } while (after != before); return events; } diff --git a/repos/os/src/drivers/input/virtio/component.h b/repos/os/src/drivers/input/virtio/component.h index d799733d29..9c90fd4f00 100644 --- a/repos/os/src/drivers/input/virtio/component.h +++ b/repos/os/src/drivers/input/virtio/component.h @@ -252,9 +252,9 @@ class Virtio_input::Driver static size_t _cfg_select(Virtio::Device &device, Config_id sel, uint8_t subsel) { - device.write_config(Config::SelectID, Virtio::Device::ACCESS_8BIT, sel); - device.write_config(Config::SelectSubID, Virtio::Device::ACCESS_8BIT, subsel); - return device.read_config(Config::Data_size, Virtio::Device::ACCESS_8BIT); + device.write_config(Config::SelectID, sel); + device.write_config(Config::SelectSubID, subsel); + return device.read_config(Config::Data_size); } @@ -265,14 +265,14 @@ class Virtio_input::Driver auto size = _cfg_select(device, Config_id::Abs_info, Event::Code::Abs_x); if (size >= sizeof(cfg.x)) { - cfg.x.min = device.read_config(Config::Data, Virtio::Device::ACCESS_32BIT); - cfg.x.max = device.read_config(Config::Data + 4, Virtio::Device::ACCESS_32BIT); + cfg.x.min = device.read_config(Config::Data); + cfg.x.max = device.read_config(Config::Data + 4); } size = _cfg_select(device, Config_id::Abs_info, Event::Code::Abs_y); if (size >= sizeof(cfg.y)) { - cfg.y.min = device.read_config(Config::Data, Virtio::Device::ACCESS_32BIT); - cfg.y.max = device.read_config(Config::Data + 4, Virtio::Device::ACCESS_32BIT); + cfg.y.min = device.read_config(Config::Data); + cfg.y.max = device.read_config(Config::Data + 4); } cfg.width = config.attribute_value("width", cfg.x.max); @@ -292,10 +292,10 @@ class Virtio_input::Driver throw Device_init_failed(); } - device_id.bus_type = (uint16_t)device.read_config(Config::Data + 0, Virtio::Device::ACCESS_16BIT); - device_id.vendor = (uint16_t)device.read_config(Config::Data + 2, Virtio::Device::ACCESS_16BIT); - device_id.product = (uint16_t)device.read_config(Config::Data + 4, Virtio::Device::ACCESS_16BIT); - device_id.version = (uint16_t)device.read_config(Config::Data + 6, Virtio::Device::ACCESS_16BIT); + device_id.bus_type = device.read_config(Config::Data + 0); + device_id.vendor = device.read_config(Config::Data + 2); + device_id.product = device.read_config(Config::Data + 4); + device_id.version = device.read_config(Config::Data + 6); return device_id; } @@ -311,7 +311,7 @@ class Virtio_input::Driver memset(buf, 0, sizeof(buf)); for (unsigned i = 0; i < size; ++i) - buf[i] = (uint8_t)device.read_config((uint8_t)(Config::Data + i), Virtio::Device::ACCESS_8BIT); + buf[i] = device.read_config((uint8_t)(Config::Data + i)); return String(buf); } diff --git a/repos/os/src/drivers/nic/virtio/component.h b/repos/os/src/drivers/nic/virtio/component.h index 003ae5dc54..d0c96a8364 100644 --- a/repos/os/src/drivers/nic/virtio/component.h +++ b/repos/os/src/drivers/nic/virtio/component.h @@ -112,7 +112,7 @@ class Virtio_nic::Device : Noncopyable /** * See section 5.1.4 of VirtIO 1.0 specification. */ - enum { CONFIG_MAC_BASE = 0, CONFIG_STATUS = 6 }; + enum Config : uint8_t { CONFIG_MAC_BASE = 0, CONFIG_STATUS = 6 }; enum { STATUS_LINK_UP = 1 << 0 }; /** @@ -181,10 +181,9 @@ class Virtio_nic::Device : Noncopyable uint32_t before = 0, after = 0; do { before = device.get_config_generation(); - for (size_t idx = 0; idx < sizeof(mac.addr); ++idx) { - mac.addr[idx] = device.read_config( - CONFIG_MAC_BASE + idx, Virtio::Device::ACCESS_8BIT); - } + for (uint8_t idx = 0; idx < sizeof(mac.addr); ++idx) + mac.addr[idx] = + device.read_config(CONFIG_MAC_BASE + idx); after = device.get_config_generation(); } while (after != before); @@ -398,7 +397,7 @@ class Virtio_nic::Device : Noncopyable uint8_t status = 0; do { before = _device.get_config_generation(); - status = _device.read_config(CONFIG_STATUS, Virtio::Device::ACCESS_8BIT); + status = _device.read_config(CONFIG_STATUS); after = _device.get_config_generation(); } while (after != before); diff --git a/repos/os/src/drivers/nic/virtio/spec/x86/target.mk b/repos/os/src/drivers/nic/virtio/spec/x86/target.mk index 65e45566ac..da2d995713 100644 --- a/repos/os/src/drivers/nic/virtio/spec/x86/target.mk +++ b/repos/os/src/drivers/nic/virtio/spec/x86/target.mk @@ -6,5 +6,3 @@ INC_DIR = $(REP_DIR)/src/drivers/nic/virtio CONFIG_XSD = ../../config.xsd vpath % $(REP_DIR)/src/drivers/nic/virtio - -CC_CXX_WARN_STRICT_CONVERSION = diff --git a/repos/os/src/drivers/nic/virtio/target_mmio.inc b/repos/os/src/drivers/nic/virtio/target_mmio.inc index 29d9e53f6c..5731bfc7b4 100644 --- a/repos/os/src/drivers/nic/virtio/target_mmio.inc +++ b/repos/os/src/drivers/nic/virtio/target_mmio.inc @@ -5,5 +5,3 @@ INC_DIR = $(REP_DIR)/src/drivers/nic/virtio CONFIG_XSD = ../../config.xsd vpath % $(REP_DIR)/src/drivers/nic/virtio - -CC_CXX_WARN_STRICT_CONVERSION =