[Qemu-devel] [PATCH v3 0/3] hw: edu: some fixes

[Qemu-devel] [PATCH v3 0/3] hw: edu: some fixes

[Li Qiang]

Recently I am considering write a driver for edu device.
After reading the spec, I found these three small issue.
Two first two related the MMIO access and the third is
related the DMA operation.

Change since v2:
Fix an error in patch 2
Fix some commit message and title.

Change since v1:
Fix format compile error

Li Qiang (3):
  edu: mmio: allow 64-bit access
  edu: mmio: allow 64-bit access in read dispatch
  edu: uses uint64_t in dma operation

 hw/misc/edu.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v3 0/3] hw: edu: some fixes

[Li Qiang]

Recently I am considering write a driver for edu device.
After reading the spec, I found these three small issue.
Two first two related the MMIO access and the third is
related the DMA operation.

Change since v2:
Fix an error in patch 2
Fix some commit message and title.

Change since v1:
Fix format compile error

Li Qiang (3):
  edu: mmio: allow 64-bit access
  edu: mmio: allow 64-bit access in read dispatch
  edu: uses uint64_t in dma operation

 hw/misc/edu.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v3 1/3] edu: mmio: allow 64-bit access

[Li Qiang]

The edu spec says the MMIO area can be accessed by 64-bit.
However currently the 'max_access_size' is not so the MMIO
access dispatch can only access 32-bit one time. This patch fixes
this to respect the spec.

Signed-off-by: Li Qiang <liq3ea@163.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
 hw/misc/edu.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index 91af452c9e..65fc32b928 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -289,6 +289,15 @@ static const MemoryRegionOps edu_mmio_ops = {
     .read = edu_mmio_read,
     .write = edu_mmio_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 8,
+    },
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 8,
+    },
+
 };
 
 /*
-- 
2.17.1

[Qemu-devel] [PATCH v3 1/3] edu: mmio: allow 64-bit access

[Li Qiang]

The edu spec says the MMIO area can be accessed by 64-bit.
However currently the 'max_access_size' is not so the MMIO
access dispatch can only access 32-bit one time. This patch fixes
this to respect the spec.

Signed-off-by: Li Qiang <liq3ea@163.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
 hw/misc/edu.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index 91af452c9e..65fc32b928 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -289,6 +289,15 @@ static const MemoryRegionOps edu_mmio_ops = {
     .read = edu_mmio_read,
     .write = edu_mmio_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 8,
+    },
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 8,
+    },
+
 };
 
 /*
-- 
2.17.1

[Qemu-devel] [PATCH v3 2/3] edu: mmio: allow 64-bit access in read dispatch

[Li Qiang]

The edu spec says when address >= 0x80, the MMIO area can
be accessed by 64-bit.

Signed-off-by: Li Qiang <liq3ea@163.com>
---
Change since v2:
Fix an error per Phillippe's advice

 hw/misc/edu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index 65fc32b928..33de05141f 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -185,7 +185,11 @@ static uint64_t edu_mmio_read(void *opaque, hwaddr addr, unsigned size)
     EduState *edu = opaque;
     uint64_t val = ~0ULL;
 
-    if (size != 4) {
+    if (addr < 0x80 && size != 4) {
+        return val;
+    }
+
+    if (addr >= 0x80 && size != 4 && size != 8) {
         return val;
     }
 
-- 
2.17.1

[Qemu-devel] [PATCH v3 2/3] edu: mmio: allow 64-bit access in read dispatch

[Li Qiang]

The edu spec says when address >= 0x80, the MMIO area can
be accessed by 64-bit.

Signed-off-by: Li Qiang <liq3ea@163.com>
---
Change since v2:
Fix an error per Phillippe's advice

 hw/misc/edu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index 65fc32b928..33de05141f 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -185,7 +185,11 @@ static uint64_t edu_mmio_read(void *opaque, hwaddr addr, unsigned size)
     EduState *edu = opaque;
     uint64_t val = ~0ULL;
 
-    if (size != 4) {
+    if (addr < 0x80 && size != 4) {
+        return val;
+    }
+
+    if (addr >= 0x80 && size != 4 && size != 8) {
         return val;
     }
 
-- 
2.17.1

[Qemu-devel] [PATCH v3 3/3] edu: uses uint64_t in dma operation

[Li Qiang]

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.

Signed-off-by: Li Qiang <liq3ea@163.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@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)
 {
-    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);
 }
 
@@ -139,13 +140,13 @@ static void edu_dma_timer(void *opaque)
     }
 
     if (EDU_DMA_DIR(edu->dma.cmd) == EDU_DMA_FROM_PCI) {
-        uint32_t dst = edu->dma.dst;
+        uint64_t dst = edu->dma.dst;
         edu_check_range(dst, edu->dma.cnt, DMA_START, DMA_SIZE);
         dst -= DMA_START;
         pci_dma_read(&edu->pdev, edu_clamp_addr(edu, edu->dma.src),
                 edu->dma_buf + dst, edu->dma.cnt);
     } else {
-        uint32_t src = edu->dma.src;
+        uint64_t src = edu->dma.src;
         edu_check_range(src, edu->dma.cnt, DMA_START, DMA_SIZE);
         src -= DMA_START;
         pci_dma_write(&edu->pdev, edu_clamp_addr(edu, edu->dma.dst),
-- 
2.17.1

[Qemu-devel] [PATCH v3 3/3] edu: uses uint64_t in dma operation

[Li Qiang]

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.

Signed-off-by: Li Qiang <liq3ea@163.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@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)
 {
-    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);
 }
 
@@ -139,13 +140,13 @@ static void edu_dma_timer(void *opaque)
     }
 
     if (EDU_DMA_DIR(edu->dma.cmd) == EDU_DMA_FROM_PCI) {
-        uint32_t dst = edu->dma.dst;
+        uint64_t dst = edu->dma.dst;
         edu_check_range(dst, edu->dma.cnt, DMA_START, DMA_SIZE);
         dst -= DMA_START;
         pci_dma_read(&edu->pdev, edu_clamp_addr(edu, edu->dma.src),
                 edu->dma_buf + dst, edu->dma.cnt);
     } else {
-        uint32_t src = edu->dma.src;
+        uint64_t src = edu->dma.src;
         edu_check_range(src, edu->dma.cnt, DMA_START, DMA_SIZE);
         src -= DMA_START;
         pci_dma_write(&edu->pdev, edu_clamp_addr(edu, edu->dma.dst),
-- 
2.17.1

Re: [Qemu-devel] [PATCH v3 2/3] edu: mmio: allow 64-bit access in read dispatch

[Philippe Mathieu-Daudé]

On 4/22/19 4:11 PM, Li Qiang wrote:
> The edu spec says when address >= 0x80, the MMIO area can
> be accessed by 64-bit.
> 
> Signed-off-by: Li Qiang <liq3ea@163.com>

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> ---
> Change since v2:
> Fix an error per Phillippe's advice
> 
>  hw/misc/edu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> index 65fc32b928..33de05141f 100644
> --- a/hw/misc/edu.c
> +++ b/hw/misc/edu.c
> @@ -185,7 +185,11 @@ static uint64_t edu_mmio_read(void *opaque, hwaddr addr, unsigned size)
>      EduState *edu = opaque;
>      uint64_t val = ~0ULL;
>  
> -    if (size != 4) {
> +    if (addr < 0x80 && size != 4) {
> +        return val;
> +    }
> +
> +    if (addr >= 0x80 && size != 4 && size != 8) {
>          return val;
>      }
>  
> 

Re: [Qemu-devel] [PATCH v3 2/3] edu: mmio: allow 64-bit access in read dispatch

[Philippe Mathieu-Daudé]

On 4/22/19 4:11 PM, Li Qiang wrote:
> The edu spec says when address >= 0x80, the MMIO area can
> be accessed by 64-bit.
> 
> Signed-off-by: Li Qiang <liq3ea@163.com>

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

> ---
> Change since v2:
> Fix an error per Phillippe's advice
> 
>  hw/misc/edu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> index 65fc32b928..33de05141f 100644
> --- a/hw/misc/edu.c
> +++ b/hw/misc/edu.c
> @@ -185,7 +185,11 @@ static uint64_t edu_mmio_read(void *opaque, hwaddr addr, unsigned size)
>      EduState *edu = opaque;
>      uint64_t val = ~0ULL;
>  
> -    if (size != 4) {
> +    if (addr < 0x80 && size != 4) {
> +        return val;
> +    }
> +
> +    if (addr >= 0x80 && size != 4 && size != 8) {
>          return val;
>      }
>  
> 

Re: [Qemu-devel] [PATCH v3 3/3] edu: uses uint64_t in dma operation

[Jiri Slaby]

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.
> 
> Signed-off-by: Li Qiang <liq3ea@163.com>
> Reviewed-by: Philippe Mathieu-Daude <philmd@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 :)?

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);
>  }



-- 
js
suse labs

Re: [Qemu-devel] [PATCH v3 3/3] edu: uses uint64_t in dma operation

[Jiri Slaby]

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.
> 
> Signed-off-by: Li Qiang <liq3ea@163.com>
> Reviewed-by: Philippe Mathieu-Daude <philmd@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 :)?

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);
>  }



-- 
js
suse labs

Re: [Qemu-devel] [PATCH v3 2/3] edu: mmio: allow 64-bit access in read dispatch

[Jiri Slaby]

On 22. 04. 19, 16:11, Li Qiang wrote:
> The edu spec says when address >= 0x80, the MMIO area can
> be accessed by 64-bit.
> 
> Signed-off-by: Li Qiang <liq3ea@163.com>
> ---
> Change since v2:
> Fix an error per Phillippe's advice
> 
>  hw/misc/edu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> index 65fc32b928..33de05141f 100644
> --- a/hw/misc/edu.c
> +++ b/hw/misc/edu.c
> @@ -185,7 +185,11 @@ static uint64_t edu_mmio_read(void *opaque, hwaddr addr, unsigned size)
>      EduState *edu = opaque;
>      uint64_t val = ~0ULL;
>  
> -    if (size != 4) {
> +    if (addr < 0x80 && size != 4) {
> +        return val;
> +    }
> +
> +    if (addr >= 0x80 && size != 4 && size != 8) {
>          return val;
>      }

Good catch, thanks.

-- 
js
suse labs

Re: [Qemu-devel] [PATCH v3 2/3] edu: mmio: allow 64-bit access in read dispatch

[Jiri Slaby]

On 22. 04. 19, 16:11, Li Qiang wrote:
> The edu spec says when address >= 0x80, the MMIO area can
> be accessed by 64-bit.
> 
> Signed-off-by: Li Qiang <liq3ea@163.com>
> ---
> Change since v2:
> Fix an error per Phillippe's advice
> 
>  hw/misc/edu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> index 65fc32b928..33de05141f 100644
> --- a/hw/misc/edu.c
> +++ b/hw/misc/edu.c
> @@ -185,7 +185,11 @@ static uint64_t edu_mmio_read(void *opaque, hwaddr addr, unsigned size)
>      EduState *edu = opaque;
>      uint64_t val = ~0ULL;
>  
> -    if (size != 4) {
> +    if (addr < 0x80 && size != 4) {
> +        return val;
> +    }
> +
> +    if (addr >= 0x80 && size != 4 && size != 8) {
>          return val;
>      }

Good catch, thanks.

-- 
js
suse labs

Re: [Qemu-devel] [PATCH v3 3/3] edu: uses uint64_t in dma operation

[Philippe Mathieu-Daudé]

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 <liq3ea@163.com>
>> Reviewed-by: Philippe Mathieu-Daude <philmd@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);
>>  }
> 
> 
> 

Re: [Qemu-devel] [PATCH v3 3/3] edu: uses uint64_t in dma operation

[Philippe Mathieu-Daudé]

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 <liq3ea@163.com>
>> Reviewed-by: Philippe Mathieu-Daude <philmd@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);
>>  }
> 
> 
>