[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-riscv] [Qemu-devel] [PATCH v5 02/15] memory: Access MemoryRegi
From: |
Richard Henderson |
Subject: |
Re: [Qemu-riscv] [Qemu-devel] [PATCH v5 02/15] memory: Access MemoryRegion with MemOp |
Date: |
Fri, 26 Jul 2019 07:04:37 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 7/26/19 6:36 AM, Richard Henderson wrote:
> On 7/25/19 11:43 PM, address@hidden wrote:
>> } 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. */
>> +
>
> This doesn't thrill me, because for 9 patches these macros don't do what they
> say on the tin, but I'll accept it.
>
> I would prefer lower-case and that the real implementation in patch 10 be
> inline functions with proper types instead of typeless macros. In particular,
> "unsigned" not "unsigned long" as you imply from "ul" here, since that's what
> was used ...
>
>> MemTxResult memory_region_dispatch_read(MemoryRegion *mr,
>> hwaddr addr,
>> uint64_t *pval,
>> - unsigned size,
>> + MemOp op,
>> MemTxAttrs attrs);
Actually, I thought of something that would make me happier:
Do not make any change to memory_region_dispatch_{read,write} now. Let the
operand continue to be "unsigned size", because it still is, because of the
no-op macros.
Make the change to to the proper type at the same time that you flip the switch
to use the proper conversion function. This will make patch 10 about 5 lines
longer, but we'll have proper typing at all points in between.
r~
>
> ... here.
>
> With the name case change,
> Reviewed-by: Richard Henderson <address@hidden>
>
>
> r~
>
- [Qemu-riscv] [Qemu-devel] [PATCH v5 00/15] Invert Endian bit in SPARCv9 MMU TTE, tony.nguyen, 2019/07/26
- [Qemu-riscv] [Qemu-devel] [PATCH v5 02/15] memory: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/26
- [Qemu-riscv] [Qemu-devel] [PATCH v5 01/15] tcg: TCGMemOp is now accelerator independent MemOp, tony.nguyen, 2019/07/26
- [Qemu-riscv] [Qemu-devel] [PATCH v5 03/15] target/mips: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/26
- [Qemu-riscv] [Qemu-devel] [PATCH v5 04/15] hw/s390x: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/26
- [Qemu-riscv] [Qemu-devel] [PATCH v5 05/15] hw/intc/armv7m_nic: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/26
- [Qemu-riscv] [Qemu-devel] [PATCH v5 06/15] hw/virtio: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/26