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

[Qemu-devel] [PATCH v4 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 | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v4 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 v4 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>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
 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 v4 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>
---
Change since v3:
Change 'size2' para of 'edu_check_range' to 64-bits.

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

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index 33de05141f..19e5545e2c 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,
-                uint32_t size2)
+static void edu_check_range(uint64_t addr, uint64_t size1, uint64_t start,
+                uint64_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 v4 0/3] hw: edu: some fixes

[Peter Xu]

On Fri, May 10, 2019 at 09:43:46AM -0700, Li Qiang wrote:
> Recently I am considering write a driver for edu device.

I don't know why you wanted to write it, but there's one (though I
don't even remember whether it's working or not)...

https://github.com/xzpeter/clibs/blob/master/gpl/linux_kernel/edu_device_driver/edu.c

Regards,

-- 
Peter Xu

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

[Li Qiang]

Peter Xu <peterx@redhat.com> 于2019年5月11日周六 上午7:53写道：

> On Fri, May 10, 2019 at 09:43:46AM -0700, Li Qiang wrote:
> > Recently I am considering write a driver for edu device.
>
> I don't know why you wanted to write it, but there's one (though I
> don't even remember whether it's working or not)...
>
>
This is a simple device, once I used it to debug and  make clear some
hardware-related stuff of PCI device,
such as PCI interrupt routing, interrupt linking device, interrupt raise
and the process of injecting.
I think if there is a driver, I can do this inspection more easily.



>
> https://github.com/xzpeter/clibs/blob/master/gpl/linux_kernel/edu_device_driver/edu.c
>
>

Great!

Thanks,
Li Qiang



> Regards,
>
> --
> Peter Xu
>

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

[Li Qiang]

Ping....

Paolo, could you merge these to your misc tree?

Thanks,
Li Qiang

Li Qiang <liq3ea@163.com> 于2019年5月11日周六 上午12:44写道：

> 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 | 32 +++++++++++++++++++++++---------
>  1 file changed, 23 insertions(+), 9 deletions(-)
>
> --
> 2.17.1
>
>
>

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

[Paolo Bonzini]

On 21/05/19 15:30, Li Qiang wrote:
> Ping....
> 
> Paolo, could you merge these to your misc tree?

Queued now, thanks.

Paolo

> Thanks,
> Li Qiang
> 
> Li Qiang <liq3ea@163.com <mailto:liq3ea@163.com>> 于2019年5月11日周六 上
> 午12:44写道：
> 
>     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 | 32 +++++++++++++++++++++++---------
>      1 file changed, 23 insertions(+), 9 deletions(-)
> 
>     -- 
>     2.17.1
> 
> 

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

[Philippe Mathieu-Daudé]

On 5/10/19 6:43 PM, 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-Daudé <philmd@redhat.com>

> ---
> Change since v3:
> Change 'size2' para of 'edu_check_range' to 64-bits.
> 
>  hw/misc/edu.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/misc/edu.c b/hw/misc/edu.c
> index 33de05141f..19e5545e2c 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,
> -                uint32_t size2)
> +static void edu_check_range(uint64_t addr, uint64_t size1, uint64_t start,
> +                uint64_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),
>