[Qemu-devel] [PATCH 0/2] pxa2xx_timer: ignore incorrect registers access to use U-Boot

[Qemu-devel] [PATCH 0/2] pxa2xx_timer: ignore incorrect registers access to use U-Boot

[Philippe Mathieu-Daudé]

Hi,

tiny patches that allow to boot a Gumstix Connex board and use U-Boot.

Using https://wiki.gumstix.com/index.php/Making_qemu_images#Connex
Linux kernel also booting but crashes entering userland:

    $ arm-softmmu/qemu-system-arm -M connex -nographic -pflash cflash.img
    pxa2xx_clkcfg_write: CPU frequency change attempt
    pxa2xx_timer_write: incorrect reg 0xd8 (value 0x000000c9)
    pxa2xx_timer_write: incorrect reg 0x98 (value 0x00000001)
    pxa2xx_timer_write: incorrect reg 0x58 (value 0x00000001)


    U-Boot 1.2.0 (May 10 2008 - 21:17:19) - PXA270@400 MHz - 1604

    *** Welcome to Gumstix ***

    DRAM:  256 MB
    Flash: 16 MB
    Using default environment

    Hit any key to stop autoboot:  0 
    Instruction Cache is ON
    Copying kernel to 0xa2000000 from 0x00f00000 (length 0x00100000)...done
    ## Booting image at a2000000 ...
       Image Name:   Angstrom/2.6.21/gumstix-custom-c
       Image Type:   ARM Linux Kernel Image (uncompressed)
       Data Size:    1041252 Bytes = 1016.8 kB
       Load Address: a0008000
       Entry Point:  a0008000
    OK

    Starting kernel ...

    Linux version 2.6.21 (otto@otto) (gcc version 4.1.2) #1 PREEMPT Mon May 12 14:33:32 PDT 2008
    CPU: XScale-PXA255 [69052d00] revision 0 (ARMv5TE), cr=00007977
    Machine: The Gumstix Platform
    Memory policy: ECC disabled, Data cache writeback
    Memory clock: 0.00MHz (*0)
    Run Mode clock: 0.00MHz (*0)
    Turbo Mode clock: 0.00MHz (*2.0, active)
    CPU0: D VIVT write-back cache
    CPU0: I cache: 16384 bytes, associativity 64, 32 byte lines, 8 sets
    CPU0: D cache: 16384 bytes, associativity 64, 32 byte lines, 8 sets
    Built 1 zonelists.  Total pages: 65024
    Kernel command line: console=ttyS0,115200n8 root=1f01 rootfstype=jffs2 reboot=cold,hard
    PID hash table entries: 1024 (order: 10, 4096 bytes)
    Console: colour dummy device 80x30
    Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
    Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
    Memory: 256MB = 256MB total
    Memory: 257536KB available (1884K code, 191K data, 144K init)
    Mount-cache hash table entries: 512
    CPU: Testing write buffer coherency: ok
    NET: Registered protocol family 16
    Time: pxa_timer clocksource has been installed.
    NET: Registered protocol family 2
    IP route cache hash table entries: 2048 (order: 1, 8192 bytes)
    TCP established hash table entries: 8192 (order: 4, 65536 bytes)
    TCP bind hash table entries: 8192 (order: 3, 32768 bytes)
    TCP: Hash tables configured (established 8192 bind 8192)
    TCP reno registered
    JFFS2 version 2.2. (NAND) (SUMMARY)  (C) 2001-2006 Red Hat, Inc.
    io scheduler noop registered
    io scheduler cfq registered (default)
    Console: switching to colour frame buffer device 80x24
    pxa2xx-uart.0: ttyS0 at MMIO 0x40100000 (irq = 15) is a FFUART
    pxa2xx-uart.1: ttyS1 at MMIO 0x40200000 (irq = 14) is a BTUART
    pxa2xx-uart.2: ttyS2 at MMIO 0x40700000 (irq = 13) is a STUART
    pxa2xx-uart.3: ttyS3 at MMIO 0x41600000 (irq = 0) is a HWUART
    Probing Gumstix Flash ROM at physical address 0x00000000 (16-bit bankwidth)
    Gumstix Flash ROM: Found 1 x16 devices at 0x0 in 16-bit bank
     Intel/Sharp Extended Query Table at 0x0031
    Using buffer write method
    Using static partitions on Gumstix Flash ROM
    Creating 3 MTD partitions on "Gumstix Flash ROM":
    0x00000000-0x00040000 : "Bootloader"
    0x00040000-0x00f00000 : "RootFS"
    0x00f00000-0x01000000 : "Kernel"
    TCP cubic registered
    NET: Registered protocol family 1
    NET: Registered protocol family 17
    XScale DSP coprocessor detected.
    VFS: Mounted root (jffs2 filesystem).
    Freeing init memory: 144K
    INIT: version 2.86 booting
    qemu-system-arm: Trying to execute code outside RAM or ROM at 0x000618e8
    This usually means one of the following happened:

    (1) You told QEMU to execute a kernel for the wrong machine type, and it crashed on startup (eg trying to run a raspberry pi kernel on a versatilepb QEMU machine)
    (2) You didn't give QEMU a kernel or BIOS filename at all, and QEMU executed a ROM full of no-op instructions until it fell off the end
    (3) Your guest kernel has a bug and crashed by jumping off into nowhere

    This is almost always one of the first two, so check your command line and that you are using the right type of kernel for this machine.
    If you think option (3) is likely then you can try debugging your guest with the -d debug options; in particular -d guest_errors will cause the log to include a dump of the guest register state at this point.

    Execution cannot continue; stopping here.

    qemu: fatal: Trying to execute code outside RAM or ROM at 0x000618e8
    R00=00000000 R01=be9acd04 R02=000bd818 R03=000b1d78
    R04=000bd838 R05=000bd80c R06=00000001 R07=000bda88
    R08=00000000 R09=00000000 R10=401d7000 R11=00000000
    R12=00000000 R13=be9aca38 R14=000876b8 R15=000618e8
    PSR=60000010 -ZC- A usr32
    FPSCR: 00000000

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  hw/timer/pxa2xx_timer: replace hw_error() -> qemu_log_mask()
  hw/sd/pxa2xx_mmci: add read/write() trace events

 hw/sd/pxa2xx_mmci.c     | 63 +++++++++++++++++++++++++++++++------------------
 hw/timer/pxa2xx_timer.c | 13 ++++++++--
 hw/sd/trace-events      |  4 ++++
 3 files changed, 55 insertions(+), 25 deletions(-)

-- 
2.15.1

[Qemu-devel] [PATCH 1/2] hw/timer/pxa2xx_timer: replace hw_error() -> qemu_log_mask()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/timer/pxa2xx_timer.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/timer/pxa2xx_timer.c b/hw/timer/pxa2xx_timer.c
index 68ba5a70b3..cfea0a5e22 100644
--- a/hw/timer/pxa2xx_timer.c
+++ b/hw/timer/pxa2xx_timer.c
@@ -13,6 +13,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/arm/pxa.h"
 #include "hw/sysbus.h"
+#include "qemu/log.h"
 
 #define OSMR0	0x00
 #define OSMR1	0x04
@@ -252,8 +253,12 @@ static uint64_t pxa2xx_timer_read(void *opaque, hwaddr offset,
     case OSNR:
         return s->snapshot;
     default:
+        qemu_log_mask(LOG_UNIMP, "%s: unknown reg 0x%02" HWADDR_PRIx
+                      "\n", __func__, offset);
+        break;
     badreg:
-        hw_error("pxa2xx_timer_read: Bad offset " REG_FMT "\n", offset);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: incorrect reg 0x%02" HWADDR_PRIx
+                      "\n", __func__, offset);
     }
 
     return 0;
@@ -377,8 +382,12 @@ static void pxa2xx_timer_write(void *opaque, hwaddr offset,
         }
         break;
     default:
+        qemu_log_mask(LOG_UNIMP, "%s: unknown reg 0x%02" HWADDR_PRIx " "
+                      "(value 0x%08" PRIx64 ")\n", __func__, offset, value);
+        break;
     badreg:
-        hw_error("pxa2xx_timer_write: Bad offset " REG_FMT "\n", offset);
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: incorrect reg 0x%02" HWADDR_PRIx " "
+                      "(value 0x%08" PRIx64 ")\n", __func__, offset, value);
     }
 }
 
-- 
2.15.1

[Qemu-devel] [PATCH 2/2] hw/sd/pxa2xx_mmci: add read/write() trace events

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/pxa2xx_mmci.c | 63 ++++++++++++++++++++++++++++++++++-------------------
 hw/sd/trace-events  |  4 ++++
 2 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index 3deccf02c9..0759a0d2eb 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -19,6 +19,7 @@
 #include "hw/qdev.h"
 #include "hw/qdev-properties.h"
 #include "qemu/error-report.h"
+#include "trace.h"
 
 #define TYPE_PXA2XX_MMCI "pxa2xx-mmci"
 #define PXA2XX_MMCI(obj) OBJECT_CHECK(PXA2xxMMCIState, (obj), TYPE_PXA2XX_MMCI)
@@ -278,43 +279,55 @@ static void pxa2xx_mmci_wakequeues(PXA2xxMMCIState *s)
 static uint64_t pxa2xx_mmci_read(void *opaque, hwaddr offset, unsigned size)
 {
     PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
-    uint32_t ret;
+    uint32_t ret = 0;
 
     switch (offset) {
     case MMC_STRPCL:
-        return 0;
+        break;
     case MMC_STAT:
-        return s->status;
+        ret = s->status;
+        break;
     case MMC_CLKRT:
-        return s->clkrt;
+        ret = s->clkrt;
+        break;
     case MMC_SPI:
-        return s->spi;
+        ret = s->spi;
+        break;
     case MMC_CMDAT:
-        return s->cmdat;
+        ret = s->cmdat;
+        break;
     case MMC_RESTO:
-        return s->resp_tout;
+        ret = s->resp_tout;
+        break;
     case MMC_RDTO:
-        return s->read_tout;
+        ret = s->read_tout;
+        break;
     case MMC_BLKLEN:
-        return s->blklen;
+        ret = s->blklen;
+        break;
     case MMC_NUMBLK:
-        return s->numblk;
+        ret = s->numblk;
+        break;
     case MMC_PRTBUF:
-        return 0;
+        break;
     case MMC_I_MASK:
-        return s->intmask;
+        ret = s->intmask;
+        break;
     case MMC_I_REG:
-        return s->intreq;
+        ret = s->intreq;
+        break;
     case MMC_CMD:
-        return s->cmd | 0x40;
+        ret = s->cmd | 0x40;
+        break;
     case MMC_ARGH:
-        return s->arg >> 16;
+        ret = s->arg >> 16;
+        break;
     case MMC_ARGL:
-        return s->arg & 0xffff;
+        ret = s->arg & 0xffff;
+        break;
     case MMC_RES:
-        if (s->resp_len < 9)
-            return s->resp_fifo[s->resp_len ++];
-        return 0;
+        ret = (s->resp_len < 9) ? s->resp_fifo[s->resp_len++] : 0;
+        break;
     case MMC_RXFIFO:
         ret = 0;
         while (size-- && s->rx_len) {
@@ -324,16 +337,19 @@ static uint64_t pxa2xx_mmci_read(void *opaque, hwaddr offset, unsigned size)
         }
         s->intreq &= ~INT_RXFIFO_REQ;
         pxa2xx_mmci_fifo_update(s);
-        return ret;
+        break;
+        ret = ret;
     case MMC_RDWAIT:
-        return 0;
+        break;
     case MMC_BLKS_REM:
-        return s->numblk;
+        ret = s->numblk;
+        break;
     default:
         hw_error("%s: Bad offset " REG_FMT "\n", __FUNCTION__, offset);
     }
+    trace_pxa2xx_mmci_read(size, offset, ret);
 
-    return 0;
+    return ret;
 }
 
 static void pxa2xx_mmci_write(void *opaque,
@@ -341,6 +357,7 @@ static void pxa2xx_mmci_write(void *opaque,
 {
     PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
 
+    trace_pxa2xx_mmci_write(size, offset, value);
     switch (offset) {
     case MMC_STRPCL:
         if (value & STRPCL_STRT_CLK) {
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 1fc0bcf44b..6eca3470e2 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -3,3 +3,7 @@
 # hw/sd/milkymist-memcard.c
 milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
 milkymist_memcard_memory_write(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
+
+# hw/sd/pxa2xx_mmci.c
+pxa2xx_mmci_read(uint8_t size, uint32_t addr, uint32_t value) "size %d addr 0x%02x value 0x%08x"
+pxa2xx_mmci_write(uint8_t size, uint32_t addr, uint32_t value) "size %d addr 0x%02x value 0x%08x"
-- 
2.15.1

Re: [Qemu-devel] [PATCH 1/2] hw/timer/pxa2xx_timer: replace hw_error() -> qemu_log_mask()

[Alistair Francis]

On Wed, Jan 3, 2018 at 8:41 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/timer/pxa2xx_timer.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/hw/timer/pxa2xx_timer.c b/hw/timer/pxa2xx_timer.c
> index 68ba5a70b3..cfea0a5e22 100644
> --- a/hw/timer/pxa2xx_timer.c
> +++ b/hw/timer/pxa2xx_timer.c
> @@ -13,6 +13,7 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/arm/pxa.h"
>  #include "hw/sysbus.h"
> +#include "qemu/log.h"
>
>  #define OSMR0  0x00
>  #define OSMR1  0x04
> @@ -252,8 +253,12 @@ static uint64_t pxa2xx_timer_read(void *opaque, hwaddr offset,
>      case OSNR:
>          return s->snapshot;
>      default:
> +        qemu_log_mask(LOG_UNIMP, "%s: unknown reg 0x%02" HWADDR_PRIx
> +                      "\n", __func__, offset);
> +        break;
>      badreg:
> -        hw_error("pxa2xx_timer_read: Bad offset " REG_FMT "\n", offset);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: incorrect reg 0x%02" HWADDR_PRIx
> +                      "\n", __func__, offset);

It might just be my email display, but if these lines don't line up
can you fix them?

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

>      }
>
>      return 0;
> @@ -377,8 +382,12 @@ static void pxa2xx_timer_write(void *opaque, hwaddr offset,
>          }
>          break;
>      default:
> +        qemu_log_mask(LOG_UNIMP, "%s: unknown reg 0x%02" HWADDR_PRIx " "
> +                      "(value 0x%08" PRIx64 ")\n", __func__, offset, value);
> +        break;
>      badreg:
> -        hw_error("pxa2xx_timer_write: Bad offset " REG_FMT "\n", offset);
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: incorrect reg 0x%02" HWADDR_PRIx " "
> +                      "(value 0x%08" PRIx64 ")\n", __func__, offset, value);
>      }
>  }
>
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 2/2] hw/sd/pxa2xx_mmci: add read/write() trace events

[Alistair Francis]

On Wed, Jan 3, 2018 at 8:41 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/pxa2xx_mmci.c | 63 ++++++++++++++++++++++++++++++++++-------------------
>  hw/sd/trace-events  |  4 ++++
>  2 files changed, 44 insertions(+), 23 deletions(-)
>
> diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
> index 3deccf02c9..0759a0d2eb 100644
> --- a/hw/sd/pxa2xx_mmci.c
> +++ b/hw/sd/pxa2xx_mmci.c
> @@ -19,6 +19,7 @@
>  #include "hw/qdev.h"
>  #include "hw/qdev-properties.h"
>  #include "qemu/error-report.h"
> +#include "trace.h"
>
>  #define TYPE_PXA2XX_MMCI "pxa2xx-mmci"
>  #define PXA2XX_MMCI(obj) OBJECT_CHECK(PXA2xxMMCIState, (obj), TYPE_PXA2XX_MMCI)
> @@ -278,43 +279,55 @@ static void pxa2xx_mmci_wakequeues(PXA2xxMMCIState *s)
>  static uint64_t pxa2xx_mmci_read(void *opaque, hwaddr offset, unsigned size)
>  {
>      PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
> -    uint32_t ret;
> +    uint32_t ret = 0;
>
>      switch (offset) {
>      case MMC_STRPCL:
> -        return 0;
> +        break;
>      case MMC_STAT:
> -        return s->status;
> +        ret = s->status;
> +        break;
>      case MMC_CLKRT:
> -        return s->clkrt;
> +        ret = s->clkrt;
> +        break;
>      case MMC_SPI:
> -        return s->spi;
> +        ret = s->spi;
> +        break;
>      case MMC_CMDAT:
> -        return s->cmdat;
> +        ret = s->cmdat;
> +        break;
>      case MMC_RESTO:
> -        return s->resp_tout;
> +        ret = s->resp_tout;
> +        break;
>      case MMC_RDTO:
> -        return s->read_tout;
> +        ret = s->read_tout;
> +        break;
>      case MMC_BLKLEN:
> -        return s->blklen;
> +        ret = s->blklen;
> +        break;
>      case MMC_NUMBLK:
> -        return s->numblk;
> +        ret = s->numblk;
> +        break;
>      case MMC_PRTBUF:
> -        return 0;
> +        break;
>      case MMC_I_MASK:
> -        return s->intmask;
> +        ret = s->intmask;
> +        break;
>      case MMC_I_REG:
> -        return s->intreq;
> +        ret = s->intreq;
> +        break;
>      case MMC_CMD:
> -        return s->cmd | 0x40;
> +        ret = s->cmd | 0x40;
> +        break;
>      case MMC_ARGH:
> -        return s->arg >> 16;
> +        ret = s->arg >> 16;
> +        break;
>      case MMC_ARGL:
> -        return s->arg & 0xffff;
> +        ret = s->arg & 0xffff;
> +        break;
>      case MMC_RES:
> -        if (s->resp_len < 9)
> -            return s->resp_fifo[s->resp_len ++];
> -        return 0;
> +        ret = (s->resp_len < 9) ? s->resp_fifo[s->resp_len++] : 0;
> +        break;
>      case MMC_RXFIFO:
>          ret = 0;
>          while (size-- && s->rx_len) {
> @@ -324,16 +337,19 @@ static uint64_t pxa2xx_mmci_read(void *opaque, hwaddr offset, unsigned size)
>          }
>          s->intreq &= ~INT_RXFIFO_REQ;
>          pxa2xx_mmci_fifo_update(s);
> -        return ret;
> +        break;
> +        ret = ret;
>      case MMC_RDWAIT:
> -        return 0;
> +        break;
>      case MMC_BLKS_REM:
> -        return s->numblk;
> +        ret = s->numblk;
> +        break;
>      default:
>          hw_error("%s: Bad offset " REG_FMT "\n", __FUNCTION__, offset);

Maybe worth removing this as well?

Either way:

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

>      }
> +    trace_pxa2xx_mmci_read(size, offset, ret);
>
> -    return 0;
> +    return ret;
>  }
>
>  static void pxa2xx_mmci_write(void *opaque,
> @@ -341,6 +357,7 @@ static void pxa2xx_mmci_write(void *opaque,
>  {
>      PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
>
> +    trace_pxa2xx_mmci_write(size, offset, value);
>      switch (offset) {
>      case MMC_STRPCL:
>          if (value & STRPCL_STRT_CLK) {
> diff --git a/hw/sd/trace-events b/hw/sd/trace-events
> index 1fc0bcf44b..6eca3470e2 100644
> --- a/hw/sd/trace-events
> +++ b/hw/sd/trace-events
> @@ -3,3 +3,7 @@
>  # hw/sd/milkymist-memcard.c
>  milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
>  milkymist_memcard_memory_write(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
> +
> +# hw/sd/pxa2xx_mmci.c
> +pxa2xx_mmci_read(uint8_t size, uint32_t addr, uint32_t value) "size %d addr 0x%02x value 0x%08x"
> +pxa2xx_mmci_write(uint8_t size, uint32_t addr, uint32_t value) "size %d addr 0x%02x value 0x%08x"
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 2/2] hw/sd/pxa2xx_mmci: add read/write() trace events

[Philippe Mathieu-Daudé]

On 01/03/2018 06:54 PM, Alistair Francis wrote:
> On Wed, Jan 3, 2018 at 8:41 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/sd/pxa2xx_mmci.c | 63 ++++++++++++++++++++++++++++++++++-------------------
>>  hw/sd/trace-events  |  4 ++++
>>  2 files changed, 44 insertions(+), 23 deletions(-)
>>
>> diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
>> index 3deccf02c9..0759a0d2eb 100644
>> --- a/hw/sd/pxa2xx_mmci.c
>> +++ b/hw/sd/pxa2xx_mmci.c
>> @@ -19,6 +19,7 @@
>>  #include "hw/qdev.h"
>>  #include "hw/qdev-properties.h"
>>  #include "qemu/error-report.h"
>> +#include "trace.h"
>>
>>  #define TYPE_PXA2XX_MMCI "pxa2xx-mmci"
>>  #define PXA2XX_MMCI(obj) OBJECT_CHECK(PXA2xxMMCIState, (obj), TYPE_PXA2XX_MMCI)
>> @@ -278,43 +279,55 @@ static void pxa2xx_mmci_wakequeues(PXA2xxMMCIState *s)
>>  static uint64_t pxa2xx_mmci_read(void *opaque, hwaddr offset, unsigned size)
>>  {
>>      PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
>> -    uint32_t ret;
>> +    uint32_t ret = 0;
>>
>>      switch (offset) {
>>      case MMC_STRPCL:
>> -        return 0;
>> +        break;
>>      case MMC_STAT:
>> -        return s->status;
>> +        ret = s->status;
>> +        break;
>>      case MMC_CLKRT:
>> -        return s->clkrt;
>> +        ret = s->clkrt;
>> +        break;
>>      case MMC_SPI:
>> -        return s->spi;
>> +        ret = s->spi;
>> +        break;
>>      case MMC_CMDAT:
>> -        return s->cmdat;
>> +        ret = s->cmdat;
>> +        break;
>>      case MMC_RESTO:
>> -        return s->resp_tout;
>> +        ret = s->resp_tout;
>> +        break;
>>      case MMC_RDTO:
>> -        return s->read_tout;
>> +        ret = s->read_tout;
>> +        break;
>>      case MMC_BLKLEN:
>> -        return s->blklen;
>> +        ret = s->blklen;
>> +        break;
>>      case MMC_NUMBLK:
>> -        return s->numblk;
>> +        ret = s->numblk;
>> +        break;
>>      case MMC_PRTBUF:
>> -        return 0;
>> +        break;
>>      case MMC_I_MASK:
>> -        return s->intmask;
>> +        ret = s->intmask;
>> +        break;
>>      case MMC_I_REG:
>> -        return s->intreq;
>> +        ret = s->intreq;
>> +        break;
>>      case MMC_CMD:
>> -        return s->cmd | 0x40;
>> +        ret = s->cmd | 0x40;
>> +        break;
>>      case MMC_ARGH:
>> -        return s->arg >> 16;
>> +        ret = s->arg >> 16;
>> +        break;
>>      case MMC_ARGL:
>> -        return s->arg & 0xffff;
>> +        ret = s->arg & 0xffff;
>> +        break;
>>      case MMC_RES:
>> -        if (s->resp_len < 9)
>> -            return s->resp_fifo[s->resp_len ++];
>> -        return 0;
>> +        ret = (s->resp_len < 9) ? s->resp_fifo[s->resp_len++] : 0;
>> +        break;
>>      case MMC_RXFIFO:
>>          ret = 0;
>>          while (size-- && s->rx_len) {
>> @@ -324,16 +337,19 @@ static uint64_t pxa2xx_mmci_read(void *opaque, hwaddr offset, unsigned size)
>>          }
>>          s->intreq &= ~INT_RXFIFO_REQ;
>>          pxa2xx_mmci_fifo_update(s);
>> -        return ret;
>> +        break;
>> +        ret = ret;

Oops...

>>      case MMC_RDWAIT:
>> -        return 0;
>> +        break;
>>      case MMC_BLKS_REM:
>> -        return s->numblk;
>> +        ret = s->numblk;
>> +        break;
>>      default:
>>          hw_error("%s: Bad offset " REG_FMT "\n", __FUNCTION__, offset);
> 
> Maybe worth removing this as well?

Indeed!

> 
> Either way:
> 
> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks :)

> 
> Alistair
> 
>>      }
>> +    trace_pxa2xx_mmci_read(size, offset, ret);
>>
>> -    return 0;
>> +    return ret;
>>  }
>>
>>  static void pxa2xx_mmci_write(void *opaque,
>> @@ -341,6 +357,7 @@ static void pxa2xx_mmci_write(void *opaque,
>>  {
>>      PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
>>
>> +    trace_pxa2xx_mmci_write(size, offset, value);
>>      switch (offset) {
>>      case MMC_STRPCL:
>>          if (value & STRPCL_STRT_CLK) {
>> diff --git a/hw/sd/trace-events b/hw/sd/trace-events
>> index 1fc0bcf44b..6eca3470e2 100644
>> --- a/hw/sd/trace-events
>> +++ b/hw/sd/trace-events
>> @@ -3,3 +3,7 @@
>>  # hw/sd/milkymist-memcard.c
>>  milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
>>  milkymist_memcard_memory_write(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
>> +
>> +# hw/sd/pxa2xx_mmci.c
>> +pxa2xx_mmci_read(uint8_t size, uint32_t addr, uint32_t value) "size %d addr 0x%02x value 0x%08x"
>> +pxa2xx_mmci_write(uint8_t size, uint32_t addr, uint32_t value) "size %d addr 0x%02x value 0x%08x"
>> --
>> 2.15.1
>>
>>

Re: [Qemu-devel] [PATCH 1/2] hw/timer/pxa2xx_timer: replace hw_error() -> qemu_log_mask()

[Philippe Mathieu-Daudé]

On 01/03/2018 06:53 PM, Alistair Francis wrote:
> On Wed, Jan 3, 2018 at 8:41 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/timer/pxa2xx_timer.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/timer/pxa2xx_timer.c b/hw/timer/pxa2xx_timer.c
>> index 68ba5a70b3..cfea0a5e22 100644
>> --- a/hw/timer/pxa2xx_timer.c
>> +++ b/hw/timer/pxa2xx_timer.c
>> @@ -13,6 +13,7 @@
>>  #include "sysemu/sysemu.h"
>>  #include "hw/arm/pxa.h"
>>  #include "hw/sysbus.h"
>> +#include "qemu/log.h"
>>
>>  #define OSMR0  0x00
>>  #define OSMR1  0x04
>> @@ -252,8 +253,12 @@ static uint64_t pxa2xx_timer_read(void *opaque, hwaddr offset,
>>      case OSNR:
>>          return s->snapshot;
>>      default:
>> +        qemu_log_mask(LOG_UNIMP, "%s: unknown reg 0x%02" HWADDR_PRIx
>> +                      "\n", __func__, offset);
>> +        break;
>>      badreg:
>> -        hw_error("pxa2xx_timer_read: Bad offset " REG_FMT "\n", offset);
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: incorrect reg 0x%02" HWADDR_PRIx
>> +                      "\n", __func__, offset);
> 
> It might just be my email display, but if these lines don't line up
> can you fix them?

My guess is your email display is correct but my eyes are tired :S

> 
> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks!

> 
> Alistair
> 
>>      }
>>
>>      return 0;
>> @@ -377,8 +382,12 @@ static void pxa2xx_timer_write(void *opaque, hwaddr offset,
>>          }
>>          break;
>>      default:
>> +        qemu_log_mask(LOG_UNIMP, "%s: unknown reg 0x%02" HWADDR_PRIx " "
>> +                      "(value 0x%08" PRIx64 ")\n", __func__, offset, value);
>> +        break;
>>      badreg:
>> -        hw_error("pxa2xx_timer_write: Bad offset " REG_FMT "\n", offset);
>> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: incorrect reg 0x%02" HWADDR_PRIx " "
>> +                      "(value 0x%08" PRIx64 ")\n", __func__, offset, value);
>>      }
>>  }
>>
>> --
>> 2.15.1
>>
>>