Re: [Qemu-devel] [RFC 1/5] memory: Define API for MemoryRegionOps to take attrs and return status

[Peter Maydell]

On 16 March 2015 at 17:20, Peter Maydell <address@hidden> wrote:
> Define an API so that devices can register MemoryRegionOps whose read
> and write callback functions are passed an arbitrary pointer to some
> transaction attributes and can return a success-or-failure status code.
> This will allow us to model devices which:
>  * behave differently for ARM Secure/NonSecure memory accesses
>  * behave differently for privileged/unprivileged accesses
>  * may return a transaction failure (causing a guest exception)
>    for erroneous accesses

> The success/failure response indication is currently ignored; it is
> provided so we can implement it later without having to change the
> callback function API yet again in future.

> +/* New-style MMIO accessors can indicate that the transaction failed.
> + * This is currently ignored, but provided in the API to allow us to add
> + * support later without changing the MemoryRegionOps functions yet again.
> + */
> +typedef enum {
> +    MemTx_OK = 0,
> +    MemTx_DecodeError = 1, /* "nothing at that address" */
> +    MemTx_SlaveError = 2,  /* "device unhappy with request" (eg 
> misalignment) */
> +} MemTxResult;

So I was looking at how this would actually get plumbed through
the memory subsystem code, and there are some awkwardnesses
with this simple enum approach. In particular, functions like
address_space_rw want to combine the error returns from
several io_mem_read/write calls into a single response to
return to the caller. With an enum we'd need some pretty
ugly code to prioritise particular failure types, or to
do something arbitrary like "return first failure code".
Alternatively we could:
(a) make MemTxResult a uint32_t, where all-bits zero indicates
"OK" and any bit set indicates some kind of error, eg
bit 0 set for "device returned an error", and bit 1 for
"decode error", and higher bits available for other kinds
of extra info about errors in future. Then address_space_rw
just ORs together all the bits in all the return codes it
receives.
(b) give up and say "just use a bool"

Opinions?

-- PMM