On 5/3/19 11:12 AM, Jiri Slaby wrote:
> On 22. 04. 19, 16:11, Li Qiang wrote:
>> The dma related variable dma.dst/src/cnt is dma_addr_t, it is
>> uint64_t in x64 platform. Change these usage from uint32_to
>> uint64_t to avoid trancation in edu_dma_timer.
Here I suggested fix the typo with "to avoid address truncation".
>>
>> Signed-off-by: Li Qiang <liq...@163.com>
>> Reviewed-by: Philippe Mathieu-Daude <phi...@redhat.com>
>> ---
>> hw/misc/edu.c | 15 ++++++++-------
>> 1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
>> index 33de05141f..401ada74af 100644
>> --- a/hw/misc/edu.c
>> +++ b/hw/misc/edu.c
>> @@ -98,23 +98,24 @@ static void edu_lower_irq(EduState *edu, uint32_t val)
>> }
>> }
>>
>> -static bool within(uint32_t addr, uint32_t start, uint32_t end)
>> +static bool within(uint64_t addr, uint64_t start, uint64_t end)
>> {
>> return start <= addr && addr < end;
>> }
>>
>> -static void edu_check_range(uint32_t addr, uint32_t size1, uint32_t start,
>> +static void edu_check_range(uint64_t addr, uint64_t size1, uint64_t start,
>> uint32_t size2)
>
> So in this version you don't change size2's type, but you still change
> size1's one :)?
On my previous review I suggested Li to keep u32 since the MMIO region
is 1 MB wide:
https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03505.html
I agree to be coherent we should use one or another, but not a mix.
>
> Other than that, looks good to me.
>
>> {
>> - uint32_t end1 = addr + size1;
>> - uint32_t end2 = start + size2;
>> + uint64_t end1 = addr + size1;
>> + uint64_t end2 = start + size2;
>>
>> if (within(addr, start, end2) &&
>> end1 > addr && within(end1, start, end2)) {
>> return;
>> }
>>
>> - hw_error("EDU: DMA range 0x%.8x-0x%.8x out of bounds (0x%.8x-0x%.8x)!",
>> + hw_error("EDU: DMA range 0x%016"PRIx64"-0x%016"PRIx64
>> + " out of bounds (0x%016"PRIx64"-0x%016"PRIx64")!",
>> addr, end1 - 1, start, end2 - 1);
>> }
>
>
>
