block-request stream: split Operation from Request

This patch splits the 'Request' definition into smaller types that are
suitable for the client-side API too.

The new 'Operation' type comprises the block operation's type (opcode)
and the operation's arguments (block number, block count).
The former 'Request::operation_defined' is now 'Operation::valid'.
The 'Request' aggregates an 'Operation', which changes its object
layout.

Note that this commit relaxes the bit-precise definition of 'Request' to
facilitate the use of 'unsigned long' where appropriate, in particular
for the request tag (which should correspond to an 'Id_space::Id'). The
originally bit-precise definition was pursued to allow the sharing of
the 'Request' type between SPARK and C++ code. However, it turns out
that defining a native type in each language and a (set of) converting
constructors is a more natural approach.

Issue #3283
This commit is contained in:
Norman Feske 2019-04-09 16:24:12 +02:00 committed by Christian Helmuth
parent ed246d152b
commit a39474a245
3 changed files with 75 additions and 49 deletions

View File

@ -17,28 +17,51 @@
/* Genode includes */
#include <base/stdint.h>
namespace Block { struct Request; }
namespace Block {
typedef Genode::uint64_t block_number_t;
typedef Genode::size_t block_count_t;
typedef Genode::off_t off_t;
struct Operation;
struct Request;
}
struct Block::Operation
{
enum class Type { INVALID = 0, READ = 1, WRITE = 2, SYNC = 3 };
Type type;
block_number_t block_number;
block_count_t count;
bool valid() const
{
return type == Type::READ || type == Type::WRITE || type == Type::SYNC;
}
};
struct Block::Request
{
enum class Operation : Genode::uint32_t { INVALID = 0, READ = 1, WRITE = 2, SYNC = 3 };
enum class Success : Genode::uint32_t { FALSE = 0, TRUE = 1 };
struct Tag { unsigned long value; };
Operation operation;
Success success;
Genode::uint64_t block_number;
Genode::uint64_t offset;
Genode::uint32_t count;
Genode::uint32_t tag;
Operation operation;
bool operation_defined() const
{
return operation == Operation::READ
|| operation == Operation::WRITE
|| operation == Operation::SYNC;
}
bool success;
} __attribute__((packed));
/**
* Location of payload within the packet stream
*/
off_t offset;
/**
* Client-defined identifier to associate acknowledgements with requests
*
* The underlying type corresponds to 'Id_space::Id'.
*/
Tag tag;
};
#endif /* _INCLUDE__BLOCK__REQUEST_H_ */

View File

@ -26,8 +26,8 @@ class Block::Request_stream : Genode::Noncopyable
{
public:
struct Block_size { Genode::uint32_t value; };
struct Align_log2 { Genode::size_t value; };
struct Block_size { Genode::size_t value; };
struct Align_log2 { Genode::size_t value; };
/**
* Interface for accessing the content of a 'Request'
@ -59,7 +59,7 @@ class Block::Request_stream : Genode::Noncopyable
*/
Genode::size_t _request_size(Block::Request const &request) const
{
return request.count * _info.block_size;
return request.operation.count * _info.block_size;
}
bool _valid_range_and_alignment(Block::Request const &request) const
@ -190,26 +190,27 @@ class Block::Request_stream : Genode::Noncopyable
Packet_descriptor const packet = tx_sink.peek_packet();
auto operation = [] (Packet_descriptor::Opcode op)
auto type = [] (Packet_descriptor::Opcode op)
{
switch (op) {
case Packet_descriptor::READ: return Request::Operation::READ;
case Packet_descriptor::WRITE: return Request::Operation::WRITE;
case Packet_descriptor::END: return Request::Operation::INVALID;
case Packet_descriptor::READ: return Operation::Type::READ;
case Packet_descriptor::WRITE: return Operation::Type::WRITE;
case Packet_descriptor::END: return Operation::Type::INVALID;
};
return Request::Operation::INVALID;
return Operation::Type::INVALID;
};
bool const packet_valid = (tx_sink.packet_valid(packet)
&& (packet.offset() >= 0)
&& (packet.size() <= (size_t)((uint32_t)~0UL)));
bool const packet_valid = tx_sink.packet_valid(packet)
&& (packet.offset() >= 0);
Request request { .operation = operation(packet.operation()),
.success = Request::Success::FALSE,
.block_number = (uint64_t)packet.block_number(),
.offset = (uint64_t)packet.offset(),
.count = (uint32_t)packet.block_count(),
.tag = 0};
Operation operation { .type = type(packet.operation()),
.block_number = packet.block_number(),
.count = packet.block_count() };
Request request { .operation = operation,
.success = false,
.offset = packet.offset(),
.tag = { 0 } };
Response const response = packet_valid
? fn(request)
@ -260,9 +261,9 @@ class Block::Request_stream : Genode::Noncopyable
bool _submitted = false;
Genode::uint32_t const _block_size;
Genode::size_t const _block_size;
Ack(Tx_sink &tx_sink, Genode::uint32_t block_size)
Ack(Tx_sink &tx_sink, Genode::size_t block_size)
: _tx_sink(tx_sink), _block_size(block_size) { }
public:
@ -275,24 +276,27 @@ class Block::Request_stream : Genode::Noncopyable
}
typedef Block::Packet_descriptor Packet_descriptor;
Packet_descriptor packet { (Genode::off_t)request.offset,
request.count * _block_size };
Packet_descriptor
packet { (Genode::off_t)request.offset,
request.operation.count * _block_size };
auto opcode = [] (Request::Operation operation)
auto opcode = [] (Operation::Type type)
{
switch (operation) {
case Request::Operation::READ: return Packet_descriptor::READ;
case Request::Operation::WRITE: return Packet_descriptor::WRITE;
case Request::Operation::SYNC: return Packet_descriptor::END;
case Request::Operation::INVALID: return Packet_descriptor::END;
switch (type) {
case Operation::Type::READ: return Packet_descriptor::READ;
case Operation::Type::WRITE: return Packet_descriptor::WRITE;
case Operation::Type::SYNC: return Packet_descriptor::END;
case Operation::Type::INVALID: return Packet_descriptor::END;
};
return Packet_descriptor::END;
};
packet = Packet_descriptor(packet, opcode(request.operation),
request.block_number, request.count);
packet = Packet_descriptor(packet,
opcode(request.operation.type),
request.operation.block_number,
request.operation.count);
packet.succeeded(request.success == Request::Success::TRUE);
packet.succeeded(request.success);
_tx_sink.try_ack_packet(packet);
_submitted = true;
@ -326,5 +330,4 @@ class Block::Request_stream : Genode::Noncopyable
void wakeup_client_if_needed() { _tx.sink()->wakeup(); }
};
#endif /* _INCLUDE__BLOCK__REQUEST_STREAM_H_ */

View File

@ -106,7 +106,7 @@ struct Test::Jobs : Noncopyable
for (unsigned i = 0; i < N; i++) {
if (_entries[i].state == Entry::IN_PROGRESS) {
_entries[i].state = Entry::COMPLETE;
_entries[i].request.success = Block::Request::Success::TRUE;
_entries[i].request.success = true;
progress = true;
}
}
@ -137,7 +137,7 @@ struct Test::Jobs : Noncopyable
completed_job(request);
if (request.operation_defined())
if (request.operation.valid())
fn(request);
}
};