Re: [Qemu-riscv] [Qemu-devel] [PATCH v4 02/15] memory: Access MemoryRegion with MemOp

[<tony.nguyen@bt.com>]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

On 7/25/19 9:45 PM, Philippe Mathieu-Daudé wrote:
>On 7/25/19 11:52 AM, tony.nguyen@bt.com wrote:
>> Replacing size with size+sign+endianness (MemOp) will enable us to
>> collapse the two byte swaps, adjust_endianness and handle_bswap, along
>> the I/O path.
>>
>> While interfaces are converted, callers will have existing unsigned
>> size coerced into a MemOp, and the callee will use this MemOp as an
>> unsigned size.
>>
>> Signed-off-by: Tony Nguyen <tony.nguyen@bt.com>
>> ---
>>  include/exec/memop.h  | 4 ++++
>>  include/exec/memory.h | 9 +++++----
>>  memory.c              | 7 +++++--
>>  3 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/exec/memop.h b/include/exec/memop.h
>> index ac58066..09c8d20 100644
>> --- a/include/exec/memop.h
>> +++ b/include/exec/memop.h
>> @@ -106,4 +106,8 @@ typedef enum MemOp {
>>      MO_SSIZE = MO_SIZE | MO_SIGN,
>>  } MemOp;
>>
>> +/* No-op while memory_region_dispatch_[read|write] is converted to MemOp */
>> +#define MEMOP_SIZE(op)  (op)    /* MemOp to size.  */
>> +#define SIZE_MEMOP(ul)  (ul)    /* Size to MemOp.  */
>
>SIZE_MEMOP() is never used until patch #10 "memory: Access MemoryRegion
>with MemOp semantics", it would be clearer to only introduce the
>MEMOP_SIZE() no-op here, and directly introduce the correct SIZE_MEMOP()
>macro in patch #10.

SIZE_MEMOP() is used, and is the main change, in patches #3 to #10. Perhaps you
meant MEMOP_SIZE()?

Either way, you have raised an issue :)

There is a lack of clarity in how the two macros are used to update the
interfaces.?


[-- Attachment #2: Type: text/html, Size: 3033 bytes --]