[Qemu-devel] [PATCH 00/26] SDCard housekeeping

[Qemu-devel] [PATCH 00/26] SDCard housekeeping

[Philippe Mathieu-Daudé]

Now that we have a way to write qtest in Python, lets start with a simple test
to perform basic card identification, covering many functions of the sd/sd.c
file.

[patch 1]
When a device is not MMIO-connected but rather plugged into a Bus, it is easier
to write tests using QMP.
So we add a 'sdbus-command' entry to directly operate on a SD Bus.
(Note this method should also work with all other QBus).

[patch 2]
We write a simple pyqtest, which
  - resolve the SDBus QOM path in the test setup()
  - verify default values for SD Spec v2.

Since have a test, lets run after each patch, so we are sure we don't break things.

[patch 4-14,20-23]
Add a bunch of trace events
[patch 3,11,15-18]
Aesthetic changes mostly
[other patches]
Trivial changes

Once this series applied, booting some Linux on a vexpress-a9 with few
sd*command traces enabled result in this outputs (which result useful to
understand/fix the different SD implementations):

48.687227:sdcard_normal_command         IO_RW_DIRECT/ CMD52 arg 0x00000c00 (state idle)
48.687233:sdcard_command_response (no response) (state idle)
48.689237:sdcard_normal_command         IO_RW_DIRECT/ CMD52 arg 0x80000c08 (state idle)
48.689244:sdcard_command_response (no response) (state idle)
48.690877:sdcard_normal_command        GO_IDLE_STATE/ CMD00 arg 0x00000000 (state idle)
48.690890:sdcard_command_response (no response) (state idle)
48.723486:sdcard_normal_command         SEND_IF_COND/ CMD08 arg 0x000001aa (state idle)
48.723507:sdcard_command_response 00 00 01 aa  (state idle)
48.723886:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg 0x00000000 (state idle)
48.723892:sdcard_command_response (no response) (state idle)
48.724019:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg 0x00000000 (state idle)
48.724023:sdcard_command_response (no response) (state idle)
48.724110:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg 0x00000000 (state idle)
48.724115:sdcard_command_response (no response) (state idle)
48.724178:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg 0x00000000 (state idle)
48.724181:sdcard_command_response (no response) (state idle)
48.724999:sdcard_command_response 00 40 01 20  (state idle)
48.725214:sdcard_app_command         SD_SEND_OP_COND/ACMD41 arg 0x00000000 (state idle)
48.725225:sdcard_command_response 40 ff ff 00  (state idle)
48.727113:sdcard_normal_command        GO_IDLE_STATE/ CMD00 arg 0x00000000 (state idle)
48.727125:sdcard_command_response (no response) (state idle)
48.729206:sdcard_normal_command         SEND_IF_COND/ CMD08 arg 0x000001aa (state idle)
48.729213:sdcard_command_response 00 00 01 aa  (state idle)
48.729339:sdcard_command_response 00 00 01 20  (state idle)
48.729412:sdcard_app_command         SD_SEND_OP_COND/ACMD41 arg 0x40300000 (state idle)
48.729419:sdcard_command_response c0 ff ff 00  (state ready)
48.729786:sdcard_normal_command         ALL_SEND_CID/ CMD02 arg 0x00000000 (state ready)
48.729797:sdcard_command_response aa 58 59 51 45 4d 55 21 01 de ad be ef 00 62 19  (state identification)
48.730825:sdcard_normal_command   SEND_RELATIVE_ADDR/ CMD03 arg 0x00000000 (state identification)
48.730835:sdcard_command_response 45 67 05 00  (state standby)
48.731221:sdcard_normal_command             SEND_CSD/ CMD09 arg 0x45670000 (state standby)
48.731228:sdcard_command_response 40 0e 00 32 5b 59 00 00 0d 47 7f 80 0a 40 00 00  (state standby)
48.732012:sdcard_normal_command SELECT/DESELECT_CARD/ CMD07 arg 0x45670000 (state standby)
48.732021:sdcard_command_response 00 00 07 00  (state transfer)
48.732419:sdcard_command_response 00 00 09 20  (state transfer)
48.764983:sdcard_app_command                SEND_SCR/ACMD51 arg 0x00000000 (state transfer)
48.765000:sdcard_command_response 00 00 09 20  (state sendingdata)
48.766951:sdcard_command_response 00 00 09 20  (state transfer)
48.767159:sdcard_app_command               SD_STATUS/ACMD13 arg 0x00000000 (state transfer)
48.767168:sdcard_command_response 00 00 09 20  (state sendingdata)
48.966731:sdcard_normal_command  READ_MULTIPLE_BLOCK/ CMD18 arg 0x00000000 (state transfer)
48.966763:sdcard_command_response 00 00 09 00  (state sendingdata)
48.974869:sdcard_normal_command    STOP_TRANSMISSION/ CMD12 arg 0x00000000 (state sendingdata)
48.974877:sdcard_command_response 00 00 0b 00  (state transfer)
50.044308:sdcard_normal_command          SEND_STATUS/ CMD13 arg 0x45670000 (state transfer)
50.044330:sdcard_command_response 00 00 09 00  (state transfer)
[...]

Regards,

Phil.

Based-on: 20171213213557.26561-7-f4bug@amsat.org
          (QTests: use Python to run complex tests)

Philippe Mathieu-Daudé (26):
  sdbus: add a QMP command to access a SDBus
  sdcard: add Python qtests
  sdcard: use ldst API
  sdcard: replace fprintf() -> qemu_log_mask()
  sdcard: rename sd_set_mode() -> sd_update_mode()
  sdcard: add sd_set_mode()
  sdcard: add sdcard_set_mode() trace event
  sdcard: add sd_set_state()
  sdcard: add a sdcard_set_state() trace event
  sdcard: use more detailled state/mode trace events
  sdcard: use warn_report() instead of fprintf()
  sdcard: replace DPRINTF() by trace events
  sdcard: add more trace events
  sdcard: use qemu_hexbuf_strdup() to trace command response
  sdcard: use PW_LEN define instead of '16' magic
  sdcard: let cmd_valid_while_locked() returns a bool
  sdcard: rename sd_set_REG() functions called by sd_reset() as sd_reset_REG()
  sdcard: move Memory Card registers together
  sdcard: add DSR register
  sdcard: add/use SD_CMD_MAX to check valid SD commands
  sdcard: add sd_cmd_abbreviation() to resolve the SD command id
  sdcard: reduce sd_cmd traces
  sdcard: add ACMD trace events
  sdcard: use a 16-bit type for the 16-bit RCA register
  sdcard: add/use a SDCardCommandClass enum instead of magic numbers
  sdcard: add/use a ccc_spi enum for the commands supported in SPI mode

 qapi-schema.json       |  41 ++++
 hw/sd/core.c           |  43 ++++
 hw/sd/sd.c             | 519 +++++++++++++++++++++++++++++++------------------
 stubs/qmp_sdbus.c      |  11 ++
 hw/sd/trace-events     |  17 ++
 stubs/Makefile.objs    |   1 +
 tests/Makefile.include |   2 +
 tests/sdcard_tests.py  | 172 ++++++++++++++++
 8 files changed, 617 insertions(+), 189 deletions(-)
 create mode 100644 stubs/qmp_sdbus.c
 create mode 100755 tests/sdcard_tests.py

-- 
2.15.1

[Qemu-devel] [PATCH 01/26] sdbus: add a QMP command to access a SDBus

[Philippe Mathieu-Daudé]

Use Base64 to serialize the binary blobs in JSON.
So far at most 512 bytes will be transfered, which result
in a 684 bytes payload.
Since this command is intented for qtesting, this is acceptable.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 qapi-schema.json    | 41 +++++++++++++++++++++++++++++++++++++++++
 hw/sd/core.c        | 43 +++++++++++++++++++++++++++++++++++++++++++
 stubs/qmp_sdbus.c   | 11 +++++++++++
 stubs/Makefile.objs |  1 +
 4 files changed, 96 insertions(+)
 create mode 100644 stubs/qmp_sdbus.c

diff --git a/qapi-schema.json b/qapi-schema.json
index 18457954a8..9b0fd90fed 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3200,3 +3200,44 @@
 # Since: 2.11
 ##
 { 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogAction'} }
+
+##
+# @SDBusCommandResponse:
+#
+# SD Bus command response.
+#
+# @base64: the command response encoded as a Base64 string, if any (optional)
+#
+# Since: 2.11
+##
+{ 'struct': 'SDBusCommandResponse', 'data': {'*base64': 'str'} }
+
+##
+# @sdbus-command:
+#
+# Execute a command on a SD Bus return the response (if any).
+#
+# @qom-path: the SD Bus path
+# @command: the SD protocol command to execute in the bus
+# @arg: a 64-bit command argument (optional)
+# @crc: the command/argument CRC (optional)
+#
+# Returns: the response of the command encoded as a Base64 string
+#
+# Since: 2.11
+#
+# -> { "execute": "sdbus-command",
+#      "arguments": { "qom-path": "/machine/unattached/device[32]/sd.0",
+#                     "command": 0x01
+#      }
+#    }
+# <- { "return": {'base64': 'A='} }
+#
+##
+{ 'command': 'sdbus-command',
+  'data': { 'qom-path': 'str',
+            'command': 'uint8',
+            '*arg': 'uint64',
+            '*crc': 'uint16' },
+  'returns': 'SDBusCommandResponse'
+}
diff --git a/hw/sd/core.c b/hw/sd/core.c
index da3a7e0efa..c546db2b22 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -22,6 +22,7 @@
 #include "qemu/osdep.h"
 #include "hw/sd/sd.h"
 #include "qemu/cutils.h"
+#include "qmp-commands.h"
 #include "sd-internal.h"
 #include "trace.h"
 
@@ -220,3 +221,45 @@ SDBus *sdbus_create_bus(DeviceState *parent, const char *name)
 {
     return SD_BUS(qbus_create(TYPE_SD_BUS, parent, name));
 }
+
+SDBusCommandResponse *qmp_sdbus_command(const char *qom_path,
+                                        uint8_t command,
+                                        bool has_arg, uint64_t arg,
+                                        bool has_crc, uint16_t crc,
+                                        Error **errp)
+{
+    uint8_t response[16 + 1];
+    SDBusCommandResponse *res;
+    bool ambiguous = false;
+    Object *obj;
+    SDBus *sdbus;
+    int sz;
+
+    obj = object_resolve_path(qom_path, &ambiguous);
+    if (obj == NULL) {
+        if (ambiguous) {
+            error_setg(errp, "Path '%s' is ambiguous", qom_path);
+        } else {
+            error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+                      "Device '%s' not found", qom_path);
+        }
+        return NULL;
+    }
+    sdbus = (SDBus *)object_dynamic_cast(obj, TYPE_SD_BUS);
+    if (sdbus == NULL) {
+        error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+                  "Device '%s' not a sd-bus", qom_path);
+        return NULL;
+    }
+
+    res = g_new0(SDBusCommandResponse, 1);
+    sz = sdbus_do_command(sdbus,
+                          &(SDRequest){ command, arg, has_crc ? crc : -1 },
+                          response);
+    if (sz > 0) {
+        res->has_base64 = true;
+        res->base64 = g_base64_encode(response, sz);
+    }
+
+    return res;
+}
diff --git a/stubs/qmp_sdbus.c b/stubs/qmp_sdbus.c
new file mode 100644
index 0000000000..50f9f1410d
--- /dev/null
+++ b/stubs/qmp_sdbus.c
@@ -0,0 +1,11 @@
+#include "qemu/osdep.h"
+#include "qmp-commands.h"
+#include "hw/sd/sd.h"
+
+SDBusCommandResponse *qmp_sdbus_command(const char *qom_path, uint8_t command,
+                                        bool has_arg, uint64_t arg,
+                                        bool has_crc, uint16_t crc,
+                                        Error **errp)
+{
+    return NULL;
+}
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 8cfe34328a..a46cb3b992 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -35,6 +35,7 @@ stub-obj-y += vm-stop.o
 stub-obj-y += vmstate.o
 stub-obj-$(CONFIG_WIN32) += fd-register.o
 stub-obj-y += qmp_pc_dimm.o
+stub-obj-y += qmp_sdbus.o
 stub-obj-y += target-monitor-defs.o
 stub-obj-y += target-get-monitor-def.o
 stub-obj-y += pc_madt_cpu_entry.o
-- 
2.15.1

[Qemu-devel] [RFC PATCH 02/26] sdcard: add a Python qtest

[Philippe Mathieu-Daudé]

Use Python to write high-level SD commands.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/Makefile.include |   2 +
 tests/sdcard_tests.py  | 172 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 174 insertions(+)
 create mode 100755 tests/sdcard_tests.py

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 13673f6d1d..4a1afcb499 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -355,8 +355,10 @@ check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
 gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
 check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
 gcov-files-arm-y += hw/timer/arm_mptimer.c
+check-qtest-arm-y += tests/sdcard_tests.py
 
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
+check-qtest-aarch64-y += tests/sdcard_tests.py
 
 check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
 
diff --git a/tests/sdcard_tests.py b/tests/sdcard_tests.py
new file mode 100755
index 0000000000..033b756cf1
--- /dev/null
+++ b/tests/sdcard_tests.py
@@ -0,0 +1,172 @@
+#!/usr/bin/env python
+#
+# Tests for the SD-Bus protocol
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import os
+import base64
+import struct
+import binascii
+import logging
+
+import qtest
+
+
+# CMD
+(GO_IDLE_STATE, SEND_OP_CMD, ALL_SEND_CID, SEND_RELATIVE_ADDR, # 0 ...
+SEND_DSR, CMD5, SWITCH_FUNCTION, CMD7,
+SEND_IF_COND, SEND_CSD, SEND_CID, READ_DAT_UNTIL_STOP, # 8 ...
+STOP_TRANSMISSION, SEND_STATUS, CMD14, GO_INACTIVE_STATE,
+SET_BLOCKLEN, READ_SINGLE_BLOCK, READ_MULTIPLE_BLOCK, CMD19, # 16 ...
+CMD20, CMD21, CMD22, SET_BLOCK_COUNT,
+WRITE_SINGLE_BLOCK, WRITE_MULTIPLE_BLOCK, PROGRAM_CID, PROGRAM_CSD,  # 24 ...
+SET_WRITE_PROT, CLR_WRITE_PROT, SEND_WRITE_PROT, CMD31,
+ERASE_WR_BLK_START, ERASE_WR_BLK_END, CMD34, CMD35, # 32 ...
+CMD36, CMD37, ERASE, CMD39,
+CMD40, CMD41, LOCK_UNLOCK, CMD43,
+CMD44, CMD45, CMD46, CMD47,
+CMD48, CMD49, CMD50, CMD51,
+CMD52, CMD53, CMD54, APP_CMD,
+GEN_CMD,) = range(57)
+
+# ACMD
+SET_BUS_WIDTH = 6
+SD_STATUS = 13
+SEND_NUM_WR_BLOCKS = 22
+SET_WR_BLK_ERASE_COUNT = 23
+SD_APP_OP_COND = 41
+SET_CLR_CARD_DETECT = 42
+SEND_SCR = 51
+
+
+class SDBus(object):
+    def __init__(self, vm, qom_path, verbose=False):
+        self.vm = vm
+        self.path = qom_path
+        self.verbose = verbose
+
+    def do_cmd(self, command, arg=0, verbose=None):
+        assert command < 64
+        if verbose is None:
+            verbose = self.verbose
+
+        data = self.vm.qmp('sdbus-command', qom_path=self.path, command=command,
+                           arg=arg)['return']
+        if 'base64' in data:
+            data = base64.b64decode(data['base64'])
+            logging.info("CMD#%02d -> %s" % (command, binascii.hexlify(data)))
+        else:
+            logging.info("CMD#%02d -> (none)" % (command))
+            data = None
+        return data
+
+    def do_acmd(self, command, arg=0, verbose=None):
+        self.do_cmd(APP_CMD, verbose=False)
+        return self.do_cmd(command, arg, verbose if verbose else self.verbose)
+
+
+def sdbus_get_qom_path(vm, bus=0):
+        qom_paths = []
+
+        result = vm.qmp('query-block')
+        for block in result['return']:
+            if not 'qdev' in block:
+                continue
+            d = vm.qmp('qom-get', path=block['qdev'], property="parent_bus")
+            qom_paths += [d['return']]
+        assert len(qom_paths) > 0
+
+        return qom_paths[bus]
+
+
+# dumb helper
+def l(buf):
+    return struct.unpack("<L", buf)[0]
+
+
+class TestSdCardSpecV2(qtest.QMPTestCase):
+    def setUp(self):
+        self.vm = qtest.createQtestMachine()
+
+        self.vm._args.append('-drive')
+        self.vm._args.append("id=testcard,driver=null-co,if=sd")
+        self.vm._args.append("-machine")
+        self.vm._args.append("xilinx-zynq-a9")
+        self.vm.launch()
+
+        bus_path = sdbus_get_qom_path(self.vm)
+        self.bus = SDBus(self.vm, bus_path, True)
+
+    def tearDown(self):
+        self.vm.shutdown()
+
+    def test_power_on_v2(self):
+
+        self.bus.do_cmd(GO_IDLE_STATE)
+
+        # get voltages
+        for i in range(6):
+            vhs = 1 << (8 + i)
+            data = self.bus.do_cmd(SEND_IF_COND, vhs)
+            v = l(data)
+            self.assertNotEqual(v, 0)
+            self.assertEqual(vhs, v >> 8)
+
+        # get OCR
+        data = self.bus.do_acmd(SD_APP_OP_COND)
+        v = l(data)
+        ocr = (v >> 8) & 0xffff
+        s1_8 = (v >> 24) & 1
+        uhs_ii = (v >> 29) & 1
+        ccs = (v >> 30) & 1
+        init = (v >> 31) & 1
+        # all those are null for v2.00
+        self.assertEqual(s1_8, 0)
+        self.assertEqual(uhs_ii, 0)
+        self.assertEqual(ccs, 0)
+        self.assertEqual(init, 0)
+
+        # ocr << 8
+        # 0 << 24 # use current voltage
+        # 0 << 28 # powersave
+        # 0 << 30 # SDSC
+        data = self.bus.do_acmd(SD_APP_OP_COND, ocr << 8)
+        v = l(data)
+        # check OCR accepted
+        self.assertEqual(ocr, v >> 8)
+
+        # check CID
+        data = self.bus.do_cmd(ALL_SEND_CID)
+        oid, pnm, psn = struct.unpack(">x2s5sxLxxx", data)
+        self.assertEqual(oid, "XY") # QEMU default
+        self.assertEqual(pnm, "QEMU!") # QEMU default
+        self.assertEqual(psn, 0xdeadbeef) # QEMU default
+
+        # check non null RCA
+        data = self.bus.do_cmd(SEND_RELATIVE_ADDR)
+        rca, = struct.unpack(">H", data[:2])
+        self.assertNotEqual(rca, 0)
+
+        self.assertEqual(rca, 0x4567) # QEMU default
+
+        # check for new RCA
+        data = self.bus.do_cmd(SEND_RELATIVE_ADDR)
+        new_rca, = struct.unpack(">H", data[:2])
+        self.assertNotEqual(new_rca, rca)
+
+
+if __name__ == '__main__':
+    qtest.main(supported_machines=['xilinx-zynq-a9'])
-- 
2.15.1

[Qemu-devel] [PATCH 03/26] sdcard: use ldst API

[Philippe Mathieu-Daudé]

To keep the code way easier to review!

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 51 ++++++++++++++-------------------------------------
 1 file changed, 14 insertions(+), 37 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8e12b07ee4..9b3745a019 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -352,10 +352,7 @@ static int sd_req_crc_validate(SDRequest *req)
 {
     uint8_t buffer[5];
     buffer[0] = 0x40 | req->cmd;
-    buffer[1] = (req->arg >> 24) & 0xff;
-    buffer[2] = (req->arg >> 16) & 0xff;
-    buffer[3] = (req->arg >> 8) & 0xff;
-    buffer[4] = (req->arg >> 0) & 0xff;
+    stl_be_p(buffer + 1, req->arg);
     return 0;
     return sd_crc7(buffer, 5) != req->crc;	/* TODO */
 }
@@ -365,19 +362,12 @@ static void sd_response_r1_make(SDState *sd, uint8_t *response)
     uint32_t status = sd->card_status;
     /* Clear the "clear on read" status bits */
     sd->card_status &= ~CARD_STATUS_C;
-
-    response[0] = (status >> 24) & 0xff;
-    response[1] = (status >> 16) & 0xff;
-    response[2] = (status >> 8) & 0xff;
-    response[3] = (status >> 0) & 0xff;
+    stl_be_p(response, status);
 }
 
 static void sd_response_r3_make(SDState *sd, uint8_t *response)
 {
-    response[0] = (sd->ocr >> 24) & 0xff;
-    response[1] = (sd->ocr >> 16) & 0xff;
-    response[2] = (sd->ocr >> 8) & 0xff;
-    response[3] = (sd->ocr >> 0) & 0xff;
+    stl_be_p(response, sd->ocr);
 }
 
 static void sd_response_r6_make(SDState *sd, uint8_t *response)
@@ -390,19 +380,14 @@ static void sd_response_r6_make(SDState *sd, uint8_t *response)
              ((sd->card_status >> 6) & 0x2000) |
               (sd->card_status & 0x1fff);
     sd->card_status &= ~(CARD_STATUS_C & 0xc81fff);
+    stw_be_p(response + 0, arg);
+    stw_be_p(response + 2, status);
 
-    response[0] = (arg >> 8) & 0xff;
-    response[1] = arg & 0xff;
-    response[2] = (status >> 8) & 0xff;
-    response[3] = status & 0xff;
 }
 
 static void sd_response_r7_make(SDState *sd, uint8_t *response)
 {
-    response[0] = (sd->vhs >> 24) & 0xff;
-    response[1] = (sd->vhs >> 16) & 0xff;
-    response[2] = (sd->vhs >>  8) & 0xff;
-    response[3] = (sd->vhs >>  0) & 0xff;
+    stl_be_p(response, sd->vhs);
 }
 
 static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
@@ -644,20 +629,13 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
     int i, mode, new_func, crc;
     mode = !!(arg & 0x80000000);
 
-    sd->data[0] = 0x00;		/* Maximum current consumption */
-    sd->data[1] = 0x01;
-    sd->data[2] = 0x80;		/* Supported group 6 functions */
-    sd->data[3] = 0x01;
-    sd->data[4] = 0x80;		/* Supported group 5 functions */
-    sd->data[5] = 0x01;
-    sd->data[6] = 0x80;		/* Supported group 4 functions */
-    sd->data[7] = 0x01;
-    sd->data[8] = 0x80;		/* Supported group 3 functions */
-    sd->data[9] = 0x01;
-    sd->data[10] = 0x80;	/* Supported group 2 functions */
-    sd->data[11] = 0x43;
-    sd->data[12] = 0x80;	/* Supported group 1 functions */
-    sd->data[13] = 0x03;
+    stw_be_p(sd->data +  0, 1);         /* Maximum current consumption */
+    stw_be_p(sd->data +  2, 0x8001);    /* Supported group 6 functions */
+    stw_be_p(sd->data +  4, 0x8001);    /* Supported group 5 functions */
+    stw_be_p(sd->data +  6, 0x8001);    /* Supported group 4 functions */
+    stw_be_p(sd->data +  8, 0x8001);    /* Supported group 3 functions */
+    stw_be_p(sd->data + 10, 0x8043);    /* Supported group 2 functions */
+    stw_be_p(sd->data + 12, 0x8003);    /* Supported group 1 functions */
     for (i = 0; i < 6; i ++) {
         new_func = (arg >> (i * 4)) & 0x0f;
         if (mode && new_func != 0x0f)
@@ -666,8 +644,7 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
     }
     memset(&sd->data[17], 0, 47);
     crc = sd_crc16(sd->data, 64);
-    sd->data[65] = crc >> 8;
-    sd->data[66] = crc & 0xff;
+    stw_be_p(sd->data + 65, crc);
 }
 
 static inline bool sd_wp_addr(SDState *sd, uint64_t addr)
-- 
2.15.1

[Qemu-devel] [PATCH 04/26] sdcard: replace fprintf() -> qemu_log_mask()

[Philippe Mathieu-Daudé]

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 9b3745a019..6227a3518b 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1557,14 +1557,16 @@ static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
     DPRINTF("sd_blk_read: addr = 0x%08llx, len = %d\n",
             (unsigned long long) addr, len);
     if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) {
-        fprintf(stderr, "sd_blk_read: read error on host side\n");
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "sd_blk_read: read error on host side\n");
     }
 }
 
 static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
 {
     if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) {
-        fprintf(stderr, "sd_blk_write: write error on host side\n");
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "sd_blk_write: write error on host side\n");
     }
 }
 
-- 
2.15.1

[Qemu-devel] [PATCH 05/26] sdcard: rename sd_set_mode() -> sd_update_mode()

[Philippe Mathieu-Daudé]

This will ease to trace mode changes (in the following patchs).

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 6227a3518b..f63459d2c0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -128,7 +128,7 @@ struct SDState {
     bool enable;
 };
 
-static void sd_set_mode(SDState *sd)
+static void sd_update_mode(SDState *sd)
 {
     switch (sd->state) {
     case sd_inactive_state:
@@ -1470,7 +1470,7 @@ int sd_do_command(SDState *sd, SDRequest *req, uint8_t *response)
     }
 
     last_state = sd->state;
-    sd_set_mode(sd);
+    sd_update_mode(sd);
 
     if (sd->expecting_acmd) {
         sd->expecting_acmd = false;
-- 
2.15.1

[Qemu-devel] [PATCH 06/26] sdcard: add sd_set_mode()

[Philippe Mathieu-Daudé]

This will ease to trace mode changes (in the following patch).

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index f63459d2c0..2c0f8a7dbd 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -128,17 +128,22 @@ struct SDState {
     bool enable;
 };
 
+static void sd_set_mode(SDState *sd, enum SDCardModes mode)
+{
+    sd->mode = mode;
+}
+
 static void sd_update_mode(SDState *sd)
 {
     switch (sd->state) {
     case sd_inactive_state:
-        sd->mode = sd_inactive;
+        sd_set_mode(sd, sd_inactive);
         break;
 
     case sd_idle_state:
     case sd_ready_state:
     case sd_identification_state:
-        sd->mode = sd_card_identification_mode;
+        sd_set_mode(sd, sd_card_identification_mode);
         break;
 
     case sd_standby_state:
@@ -147,7 +152,7 @@ static void sd_update_mode(SDState *sd)
     case sd_receivingdata_state:
     case sd_programming_state:
     case sd_disconnect_state:
-        sd->mode = sd_data_transfer_mode;
+        sd_set_mode(sd, sd_data_transfer_mode);
         break;
     }
 }
-- 
2.15.1

[Qemu-devel] [PATCH 07/26] sdcard: add sdcard_set_mode() trace event

[Philippe Mathieu-Daudé]

Use sd_mode_name() to pretty-print the SDCardMode.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c         | 16 +++++++++++++++-
 hw/sd/trace-events |  3 +++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2c0f8a7dbd..465d254f2e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -40,6 +40,7 @@
 #include "qemu/timer.h"
 #include "qemu/log.h"
 #include "sd-internal.h"
+#include "trace.h"
 
 //#define DEBUG_SD 1
 
@@ -128,9 +129,22 @@ struct SDState {
     bool enable;
 };
 
+static const char *sd_mode_name(enum SDCardModes mode)
+{
+    static const char *mode_name[] = {
+        [sd_inactive]                   = "inactive",
+        [sd_card_identification_mode]   = "card_identification",
+        [sd_data_transfer_mode]         = "data_transfer",
+    };
+    return mode_name[mode];
+}
+
 static void sd_set_mode(SDState *sd, enum SDCardModes mode)
 {
-    sd->mode = mode;
+    if (sd->mode != mode) {
+        trace_sdcard_set_mode(sd_mode_name(sd->mode), sd_mode_name(mode));
+        sd->mode = mode;
+    }
 }
 
 static void sd_update_mode(SDState *sd)
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 6b1dc7380f..369d258d10 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -21,6 +21,9 @@ sdhci_read_dataport(uint16_t data_count) "all %u bytes of data have been read fr
 sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes of data"
 sdhci_led(bool state) "LED: %u"
 
+# hw/sd/sd.c
+sdcard_set_mode(const char *current_mode, const char *new_mode) "%s -> %s"
+
 # 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"
-- 
2.15.1

[Qemu-devel] [PATCH 08/26] sdcard: add sd_set_state()

[Philippe Mathieu-Daudé]

This will ease to trace state changes (in the following patchs).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 128 ++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 67 insertions(+), 61 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 465d254f2e..f67c9ff49c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -171,6 +171,11 @@ static void sd_update_mode(SDState *sd)
     }
 }
 
+static void sd_set_state(SDState *sd, enum SDCardStates state)
+{
+    sd->state = state;
+}
+
 static const sd_cmd_type_t sd_cmd_type[64] = {
     sd_bc,   sd_none, sd_bcr,  sd_bcr,  sd_none, sd_none, sd_none, sd_ac,
     sd_bcr,  sd_ac,   sd_ac,   sd_adtc, sd_ac,   sd_ac,   sd_none, sd_ac,
@@ -429,7 +434,7 @@ static void sd_reset(DeviceState *dev)
 
     sect = sd_addr_to_wpnum(size) + 1;
 
-    sd->state = sd_idle_state;
+    sd_set_state(sd, sd_idle_state);
     sd->rca = 0x0000;
     sd_set_ocr(sd);
     sd_set_scr(sd);
@@ -767,7 +772,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             return sd->spi ? sd_r1 : sd_r0;
 
         default:
-            sd->state = sd_idle_state;
+            sd_set_state(sd, sd_idle_state);
             sd_reset(DEVICE(sd));
             return sd->spi ? sd_r1 : sd_r0;
         }
@@ -777,7 +782,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         if (!sd->spi)
             goto bad_cmd;
 
-        sd->state = sd_transfer_state;
+        sd_set_state(sd, sd_transfer_state);
         return sd_r1;
 
     case 2:	/* CMD2:   ALL_SEND_CID */
@@ -785,7 +790,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             goto bad_cmd;
         switch (sd->state) {
         case sd_ready_state:
-            sd->state = sd_identification_state;
+            sd_set_state(sd, sd_identification_state);
             return sd_r2_i;
 
         default:
@@ -799,7 +804,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         switch (sd->state) {
         case sd_identification_state:
         case sd_standby_state:
-            sd->state = sd_standby_state;
+            sd_set_state(sd, sd_standby_state);
             sd_set_rca(sd);
             return sd_r6;
 
@@ -829,7 +834,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         switch (sd->mode) {
         case sd_data_transfer_mode:
             sd_function_switch(sd, req.arg);
-            sd->state = sd_sendingdata_state;
+            sd_set_state(sd, sd_sendingdata_state);
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
@@ -847,7 +852,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             if (sd->rca != rca)
                 return sd_r0;
 
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
             return sd_r1b;
 
         case sd_transfer_state:
@@ -855,21 +860,21 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             if (sd->rca == rca)
                 break;
 
-            sd->state = sd_standby_state;
+            sd_set_state(sd, sd_standby_state);
             return sd_r1b;
 
         case sd_disconnect_state:
             if (sd->rca != rca)
                 return sd_r0;
 
-            sd->state = sd_programming_state;
+            sd_set_state(sd, sd_programming_state);
             return sd_r1b;
 
         case sd_programming_state:
             if (sd->rca == rca)
                 break;
 
-            sd->state = sd_disconnect_state;
+            sd_set_state(sd, sd_disconnect_state);
             return sd_r1b;
 
         default:
@@ -908,7 +913,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         case sd_transfer_state:
             if (!sd->spi)
                 break;
-            sd->state = sd_sendingdata_state;
+            sd_set_state(sd, sd_sendingdata_state);
             memcpy(sd->data, sd->csd, 16);
             sd->data_start = addr;
             sd->data_offset = 0;
@@ -930,7 +935,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         case sd_transfer_state:
             if (!sd->spi)
                 break;
-            sd->state = sd_sendingdata_state;
+            sd_set_state(sd, sd_sendingdata_state);
             memcpy(sd->data, sd->cid, 16);
             sd->data_start = addr;
             sd->data_offset = 0;
@@ -946,7 +951,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             goto bad_cmd;
         switch (sd->state) {
         case sd_transfer_state:
-            sd->state = sd_sendingdata_state;
+            sd_set_state(sd, sd_sendingdata_state);
             sd->data_start = req.arg;
             sd->data_offset = 0;
 
@@ -962,13 +967,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
     case 12:	/* CMD12:  STOP_TRANSMISSION */
         switch (sd->state) {
         case sd_sendingdata_state:
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
             return sd_r1b;
 
         case sd_receivingdata_state:
-            sd->state = sd_programming_state;
+            sd_set_state(sd, sd_programming_state);
             /* Bzzzzzzztt .... Operation complete.  */
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
             return sd_r1b;
 
         default:
@@ -997,7 +1002,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             if (sd->rca != rca)
                 return sd_r0;
 
-            sd->state = sd_inactive_state;
+            sd_set_state(sd, sd_inactive_state);
             return sd_r0;
 
         default:
@@ -1024,7 +1029,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
     case 17:	/* CMD17:  READ_SINGLE_BLOCK */
         switch (sd->state) {
         case sd_transfer_state:
-            sd->state = sd_sendingdata_state;
+            sd_set_state(sd, sd_sendingdata_state);
             sd->data_start = addr;
             sd->data_offset = 0;
 
@@ -1040,7 +1045,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
     case 18:	/* CMD18:  READ_MULTIPLE_BLOCK */
         switch (sd->state) {
         case sd_transfer_state:
-            sd->state = sd_sendingdata_state;
+            sd_set_state(sd, sd_sendingdata_state);
             sd->data_start = addr;
             sd->data_offset = 0;
 
@@ -1073,7 +1078,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             /* Writing in SPI mode not implemented.  */
             if (sd->spi)
                 break;
-            sd->state = sd_receivingdata_state;
+            sd_set_state(sd, sd_receivingdata_state);
             sd->data_start = addr;
             sd->data_offset = 0;
             sd->blk_written = 0;
@@ -1099,7 +1104,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             /* Writing in SPI mode not implemented.  */
             if (sd->spi)
                 break;
-            sd->state = sd_receivingdata_state;
+            sd_set_state(sd, sd_receivingdata_state);
             sd->data_start = addr;
             sd->data_offset = 0;
             sd->blk_written = 0;
@@ -1122,7 +1127,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             goto bad_cmd;
         switch (sd->state) {
         case sd_transfer_state:
-            sd->state = sd_receivingdata_state;
+            sd_set_state(sd, sd_receivingdata_state);
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
@@ -1137,7 +1142,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             goto unimplemented_cmd;
         switch (sd->state) {
         case sd_transfer_state:
-            sd->state = sd_receivingdata_state;
+            sd_set_state(sd, sd_receivingdata_state);
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
@@ -1156,10 +1161,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
                 return sd_r1b;
             }
 
-            sd->state = sd_programming_state;
+            sd_set_state(sd, sd_programming_state);
             set_bit(sd_addr_to_wpnum(addr), sd->wp_groups);
             /* Bzzzzzzztt .... Operation complete.  */
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
             return sd_r1b;
 
         default:
@@ -1175,10 +1180,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
                 return sd_r1b;
             }
 
-            sd->state = sd_programming_state;
+            sd_set_state(sd, sd_programming_state);
             clear_bit(sd_addr_to_wpnum(addr), sd->wp_groups);
             /* Bzzzzzzztt .... Operation complete.  */
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
             return sd_r1b;
 
         default:
@@ -1189,7 +1194,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
     case 30:	/* CMD30:  SEND_WRITE_PROT */
         switch (sd->state) {
         case sd_transfer_state:
-            sd->state = sd_sendingdata_state;
+            sd_set_state(sd, sd_sendingdata_state);
             *(uint32_t *) sd->data = sd_wpbits(sd, req.arg);
             sd->data_start = addr;
             sd->data_offset = 0;
@@ -1231,10 +1236,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
                 return sd_r1b;
             }
 
-            sd->state = sd_programming_state;
+            sd_set_state(sd, sd_programming_state);
             sd_erase(sd);
             /* Bzzzzzzztt .... Operation complete.  */
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
             return sd_r1b;
 
         default:
@@ -1248,7 +1253,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             goto unimplemented_cmd;
         switch (sd->state) {
         case sd_transfer_state:
-            sd->state = sd_receivingdata_state;
+            sd_set_state(sd, sd_receivingdata_state);
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
@@ -1283,10 +1288,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         switch (sd->state) {
         case sd_transfer_state:
             sd->data_offset = 0;
-            if (req.arg & 1)
-                sd->state = sd_sendingdata_state;
-            else
-                sd->state = sd_receivingdata_state;
+            if (req.arg & 1) {
+                sd_set_state(sd, sd_sendingdata_state);
+            } else {
+                sd_set_state(sd, sd_receivingdata_state);
+            }
             return sd_r1;
 
         default:
@@ -1331,7 +1337,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     case 13:	/* ACMD13: SD_STATUS */
         switch (sd->state) {
         case sd_transfer_state:
-            sd->state = sd_sendingdata_state;
+            sd_set_state(sd, sd_sendingdata_state);
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
@@ -1346,7 +1352,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         case sd_transfer_state:
             *(uint32_t *) sd->data = sd->blk_written;
 
-            sd->state = sd_sendingdata_state;
+            sd_set_state(sd, sd_sendingdata_state);
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
@@ -1369,7 +1375,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     case 41:	/* ACMD41: SD_APP_OP_COND */
         if (sd->spi) {
             /* SEND_OP_CMD */
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
             return sd_r1;
         }
         switch (sd->state) {
@@ -1399,7 +1405,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
              * unless it's an enquiry ACMD41 (bits 23:0 == 0).
              */
             if (req.arg & ACMD41_ENQUIRY_MASK) {
-                sd->state = sd_ready_state;
+                sd_set_state(sd, sd_ready_state);
             }
 
             return sd_r3;
@@ -1423,7 +1429,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     case 51:	/* ACMD51: SEND_SCR */
         switch (sd->state) {
         case sd_transfer_state:
-            sd->state = sd_sendingdata_state;
+            sd_set_state(sd, sd_sendingdata_state);
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
@@ -1615,12 +1621,12 @@ void sd_write_data(SDState *sd, uint8_t value)
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sd->blk_len) {
             /* TODO: Check CRC before committing */
-            sd->state = sd_programming_state;
+            sd_set_state(sd, sd_programming_state);
             BLK_WRITE_BLOCK(sd->data_start, sd->data_offset);
             sd->blk_written ++;
             sd->csd[14] |= 0x40;
             /* Bzzzzzzztt .... Operation complete.  */
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         }
         break;
 
@@ -1639,7 +1645,7 @@ void sd_write_data(SDState *sd, uint8_t value)
         sd->data[sd->data_offset++] = value;
         if (sd->data_offset >= sd->blk_len) {
             /* TODO: Check CRC before committing */
-            sd->state = sd_programming_state;
+            sd_set_state(sd, sd_programming_state);
             BLK_WRITE_BLOCK(sd->data_start, sd->data_offset);
             sd->blk_written++;
             sd->data_start += sd->blk_len;
@@ -1650,12 +1656,12 @@ void sd_write_data(SDState *sd, uint8_t value)
             if (sd->multi_blk_cnt != 0) {
                 if (--sd->multi_blk_cnt == 0) {
                     /* Stop! */
-                    sd->state = sd_transfer_state;
+                    sd_set_state(sd, sd_transfer_state);
                     break;
                 }
             }
 
-            sd->state = sd_receivingdata_state;
+            sd_set_state(sd, sd_receivingdata_state);
         }
         break;
 
@@ -1663,7 +1669,7 @@ void sd_write_data(SDState *sd, uint8_t value)
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sizeof(sd->cid)) {
             /* TODO: Check CRC before committing */
-            sd->state = sd_programming_state;
+            sd_set_state(sd, sd_programming_state);
             for (i = 0; i < sizeof(sd->cid); i ++)
                 if ((sd->cid[i] | 0x00) != sd->data[i])
                     sd->card_status |= CID_CSD_OVERWRITE;
@@ -1674,7 +1680,7 @@ void sd_write_data(SDState *sd, uint8_t value)
                     sd->cid[i] &= sd->data[i];
                 }
             /* Bzzzzzzztt .... Operation complete.  */
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         }
         break;
 
@@ -1682,7 +1688,7 @@ void sd_write_data(SDState *sd, uint8_t value)
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sizeof(sd->csd)) {
             /* TODO: Check CRC before committing */
-            sd->state = sd_programming_state;
+            sd_set_state(sd, sd_programming_state);
             for (i = 0; i < sizeof(sd->csd); i ++)
                 if ((sd->csd[i] | sd_csd_rw_mask[i]) !=
                     (sd->data[i] | sd_csd_rw_mask[i]))
@@ -1698,7 +1704,7 @@ void sd_write_data(SDState *sd, uint8_t value)
                     sd->csd[i] &= sd->data[i];
                 }
             /* Bzzzzzzztt .... Operation complete.  */
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         }
         break;
 
@@ -1706,10 +1712,10 @@ void sd_write_data(SDState *sd, uint8_t value)
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sd->blk_len) {
             /* TODO: Check CRC before committing */
-            sd->state = sd_programming_state;
+            sd_set_state(sd, sd_programming_state);
             sd_lock_command(sd);
             /* Bzzzzzzztt .... Operation complete.  */
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         }
         break;
 
@@ -1717,7 +1723,7 @@ void sd_write_data(SDState *sd, uint8_t value)
         sd->data[sd->data_offset ++] = value;
         if (sd->data_offset >= sd->blk_len) {
             APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         }
         break;
 
@@ -1752,7 +1758,7 @@ uint8_t sd_read_data(SDState *sd)
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= 64)
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         break;
 
     case 9:	/* CMD9:   SEND_CSD */
@@ -1760,7 +1766,7 @@ uint8_t sd_read_data(SDState *sd)
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= 16)
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         break;
 
     case 11:	/* CMD11:  READ_DAT_UNTIL_STOP */
@@ -1782,7 +1788,7 @@ uint8_t sd_read_data(SDState *sd)
         ret = sd->sd_status[sd->data_offset ++];
 
         if (sd->data_offset >= sizeof(sd->sd_status))
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         break;
 
     case 17:	/* CMD17:  READ_SINGLE_BLOCK */
@@ -1791,7 +1797,7 @@ uint8_t sd_read_data(SDState *sd)
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= io_len)
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         break;
 
     case 18:	/* CMD18:  READ_MULTIPLE_BLOCK */
@@ -1811,7 +1817,7 @@ uint8_t sd_read_data(SDState *sd)
             if (sd->multi_blk_cnt != 0) {
                 if (--sd->multi_blk_cnt == 0) {
                     /* Stop! */
-                    sd->state = sd_transfer_state;
+                    sd_set_state(sd, sd_transfer_state);
                     break;
                 }
             }
@@ -1822,21 +1828,21 @@ uint8_t sd_read_data(SDState *sd)
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= 4)
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         break;
 
     case 30:	/* CMD30:  SEND_WRITE_PROT */
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= 4)
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         break;
 
     case 51:	/* ACMD51: SEND_SCR */
         ret = sd->scr[sd->data_offset ++];
 
         if (sd->data_offset >= sizeof(sd->scr))
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         break;
 
     case 56:	/* CMD56:  GEN_CMD */
@@ -1845,7 +1851,7 @@ uint8_t sd_read_data(SDState *sd)
         ret = sd->data[sd->data_offset ++];
 
         if (sd->data_offset >= sd->blk_len)
-            sd->state = sd_transfer_state;
+            sd_set_state(sd, sd_transfer_state);
         break;
 
     default:
-- 
2.15.1

[Qemu-devel] [PATCH 09/26] sdcard: add a sdcard_set_state() trace event

[Philippe Mathieu-Daudé]

and his sd_state_name() companion to pretty-print the SDCardState.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c         | 24 +++++++++++++++++++++++-
 hw/sd/trace-events |  1 +
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index f67c9ff49c..bd4a896cba 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -139,6 +139,25 @@ static const char *sd_mode_name(enum SDCardModes mode)
     return mode_name[mode];
 }
 
+static const char *sd_state_name(enum SDCardStates state)
+{
+    static const char *state_name[] = {
+        [sd_idle_state]             = "idle",
+        [sd_ready_state]            = "ready",
+        [sd_identification_state]   = "identification",
+        [sd_standby_state]          = "standby",
+        [sd_transfer_state]         = "transfer",
+        [sd_sendingdata_state]      = "sendingdata",
+        [sd_receivingdata_state]    = "receivingdata",
+        [sd_programming_state]      = "programming",
+        [sd_disconnect_state]       = "disconnect",
+    };
+    if (state == sd_inactive_state) {
+        return "inactive";
+    }
+    return state_name[state];
+}
+
 static void sd_set_mode(SDState *sd, enum SDCardModes mode)
 {
     if (sd->mode != mode) {
@@ -173,7 +192,10 @@ static void sd_update_mode(SDState *sd)
 
 static void sd_set_state(SDState *sd, enum SDCardStates state)
 {
-    sd->state = state;
+    if (sd->state != state) {
+        trace_sdcard_set_state(sd_state_name(sd->state), sd_state_name(state));
+        sd->state = state;
+    }
 }
 
 static const sd_cmd_type_t sd_cmd_type[64] = {
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 369d258d10..2677c2195d 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -23,6 +23,7 @@ sdhci_led(bool state) "LED: %u"
 
 # hw/sd/sd.c
 sdcard_set_mode(const char *current_mode, const char *new_mode) "%s -> %s"
+sdcard_set_state(const char *current_state, const char *new_state) "%s -> %s"
 
 # hw/sd/milkymist-memcard.c
 milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
-- 
2.15.1

[Qemu-devel] [PATCH 10/26] sdcard: use more detailled state/mode trace events

[Philippe Mathieu-Daudé]

sd_set_mode() can now use sd_state_name(),
and sd_set_state() use sd_mode_name().

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index bd4a896cba..c1ed7b59f1 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -161,7 +161,8 @@ static const char *sd_state_name(enum SDCardStates state)
 static void sd_set_mode(SDState *sd, enum SDCardModes mode)
 {
     if (sd->mode != mode) {
-        trace_sdcard_set_mode(sd_mode_name(sd->mode), sd_mode_name(mode));
+        trace_sdcard_set_mode(sd_mode_name(sd->mode), sd_mode_name(mode),
+                              sd_state_name(sd->state));
         sd->mode = mode;
     }
 }
@@ -193,7 +194,8 @@ static void sd_update_mode(SDState *sd)
 static void sd_set_state(SDState *sd, enum SDCardStates state)
 {
     if (sd->state != state) {
-        trace_sdcard_set_state(sd_state_name(sd->state), sd_state_name(state));
+        trace_sdcard_set_state(sd_state_name(sd->state), sd_state_name(state),
+                               sd_mode_name(sd->mode));
         sd->state = state;
     }
 }
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 2677c2195d..db5a5e62f2 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -22,8 +22,8 @@ sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes of
 sdhci_led(bool state) "LED: %u"
 
 # hw/sd/sd.c
-sdcard_set_mode(const char *current_mode, const char *new_mode) "%s -> %s"
-sdcard_set_state(const char *current_state, const char *new_state) "%s -> %s"
+sdcard_set_mode(const char *current_mode, const char *new_mode, const char *state) "%s -> %s (state: %s)"
+sdcard_set_state(const char *current_state, const char *new_state, const char *mode) "%s -> %s (mode: %s)"
 
 # hw/sd/milkymist-memcard.c
 milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
-- 
2.15.1

[Qemu-devel] [PATCH 11/26] sdcard: use warn_report() instead of fprintf()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index c1ed7b59f1..2fa05f42b7 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -725,7 +725,7 @@ static void sd_lock_command(SDState *sd)
         sd->card_status &= ~CARD_IS_LOCKED;
         sd->pwd_len = 0;
         /* Erasing the entire card here! */
-        fprintf(stderr, "SD: Card force-erased by CMD42\n");
+        warn_report("SD: Card force-erased by CMD42");
         return;
     }
 
-- 
2.15.1

[Qemu-devel] [PATCH 12/26] sdcard: replace DPRINTF() by trace events

[Philippe Mathieu-Daudé]

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2fa05f42b7..bd583be835 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -773,6 +773,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
     uint32_t rca = 0x0000;
     uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
 
+    trace_sdcard_normal_command(req.cmd, req.arg, sd_state_name(sd->state));
+
     /* Not interpreting this as an app command */
     sd->card_status &= ~APP_CMD;
 
@@ -787,7 +789,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         sd->multi_blk_cnt = 0;
     }
 
-    DPRINTF("CMD%d 0x%08x state %d\n", req.cmd, req.arg, sd->state);
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
     case 0:	/* CMD0:   GO_IDLE_STATE */
@@ -1307,8 +1308,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         return sd_r1;
 
     case 56:	/* CMD56:  GEN_CMD */
-        fprintf(stderr, "SD: GEN_CMD 0x%08x\n", req.arg);
-
         switch (sd->state) {
         case sd_transfer_state:
             sd->data_offset = 0;
@@ -1343,7 +1342,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 static sd_rsp_type_t sd_app_command(SDState *sd,
                                     SDRequest req)
 {
-    DPRINTF("ACMD%d 0x%08x\n", req.cmd, req.arg);
+    trace_sdcard_app_command(req.cmd, req.arg);
     sd->card_status |= APP_CMD;
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
@@ -1603,8 +1602,7 @@ send_response:
 
 static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 {
-    DPRINTF("sd_blk_read: addr = 0x%08llx, len = %d\n",
-            (unsigned long long) addr, len);
+    trace_sdcard_read_block(addr, len);
     if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "sd_blk_read: read error on host side\n");
@@ -1613,6 +1611,7 @@ static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 
 static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
 {
+    trace_sdcard_write_block(addr, len);
     if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "sd_blk_write: write error on host side\n");
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index db5a5e62f2..117da83014 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -24,6 +24,10 @@ sdhci_led(bool state) "LED: %u"
 # hw/sd/sd.c
 sdcard_set_mode(const char *current_mode, const char *new_mode, const char *state) "%s -> %s (state: %s)"
 sdcard_set_state(const char *current_state, const char *new_state, const char *mode) "%s -> %s (mode: %s)"
+sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state) "CMD%d arg 0x%08x (state %s)"
+sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
+sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%08lx size 0x%x"
+sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%08lx size 0x%x"
 
 # hw/sd/milkymist-memcard.c
 milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
-- 
2.15.1

[Qemu-devel] [PATCH 13/26] sdcard: add more trace events

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c         | 12 ++++++++++++
 hw/sd/trace-events |  8 ++++++++
 2 files changed, 20 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index bd583be835..84e02745f1 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -449,6 +449,7 @@ static void sd_reset(DeviceState *dev)
     uint64_t size;
     uint64_t sect;
 
+    trace_sdcard_reset();
     if (sd->blk) {
         blk_get_geometry(sd->blk, &sect);
     } else {
@@ -500,7 +501,10 @@ static void sd_cardchange(void *opaque, bool load, Error **errp)
     bool readonly = sd_get_readonly(sd);
 
     if (inserted) {
+        trace_sdcard_inserted(readonly);
         sd_reset(dev);
+    } else {
+        trace_sdcard_ejected();
     }
 
     /* The IRQ notification is for legacy non-QOM SD controller devices;
@@ -632,6 +636,7 @@ static void sd_erase(SDState *sd)
     uint64_t erase_start = sd->erase_start;
     uint64_t erase_end = sd->erase_end;
 
+    trace_sdcard_erase();
     if (!sd->erase_start || !sd->erase_end) {
         sd->card_status |= ERASE_SEQ_ERROR;
         return;
@@ -713,6 +718,11 @@ static void sd_lock_command(SDState *sd)
     else
         pwd_len = 0;
 
+    if (lock) {
+        trace_sdcard_lock();
+    } else {
+        trace_sdcard_unlock();
+    }
     if (erase) {
         if (!(sd->card_status & CARD_IS_LOCKED) || sd->blk_len > 1 ||
                         set_pwd || clr_pwd || lock || sd->wp_switch ||
@@ -1639,6 +1649,7 @@ void sd_write_data(SDState *sd, uint8_t value)
     if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION))
         return;
 
+    trace_sdcard_write_data(value);
     switch (sd->current_cmd) {
     case 24:	/* CMD24:  WRITE_SINGLE_BLOCK */
         sd->data[sd->data_offset ++] = value;
@@ -1776,6 +1787,7 @@ uint8_t sd_read_data(SDState *sd)
 
     io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
 
+    trace_sdcard_read_data(io_len);
     switch (sd->current_cmd) {
     case 6:	/* CMD6:   SWITCH_FUNCTION */
         ret = sd->data[sd->data_offset ++];
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 117da83014..0e78f980de 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -26,8 +26,16 @@ sdcard_set_mode(const char *current_mode, const char *new_mode, const char *stat
 sdcard_set_state(const char *current_state, const char *new_state, const char *mode) "%s -> %s (mode: %s)"
 sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state) "CMD%d arg 0x%08x (state %s)"
 sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
+sdcard_reset(void) ""
+sdcard_inserted(bool readonly) "read_only: %d"
+sdcard_ejected(void) ""
+sdcard_erase(void) ""
+sdcard_lock(void) ""
+sdcard_unlock(void) ""
 sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%08lx size 0x%x"
 sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%08lx size 0x%x"
+sdcard_write_data(uint8_t value) "value 0x%02x"
+sdcard_read_data(int length) "len %d"
 
 # hw/sd/milkymist-memcard.c
 milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
-- 
2.15.1

[Qemu-devel] [RFC PATCH 14/26] sdcard: use qemu_hexbuf_strdup() to trace command response

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c         | 26 +++++++-------------------
 hw/sd/trace-events |  1 +
 2 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 84e02745f1..2a249b1612 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -35,6 +35,7 @@
 #include "hw/sd/sd.h"
 #include "qapi/error.h"
 #include "qemu/bitmap.h"
+#include "qemu/cutils.h"
 #include "hw/qdev-properties.h"
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
@@ -42,15 +43,6 @@
 #include "sd-internal.h"
 #include "trace.h"
 
-//#define DEBUG_SD 1
-
-#ifdef DEBUG_SD
-#define DPRINTF(fmt, ...) \
-do { fprintf(stderr, "SD: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) do {} while(0)
-#endif
-
 #define ACMD41_ENQUIRY_MASK     0x00ffffff
 #define OCR_POWER_UP            0x80000000
 #define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
@@ -1595,17 +1587,13 @@ send_response:
         sd->card_status &= ~CARD_STATUS_B;
     }
 
-#ifdef DEBUG_SD
-    if (rsplen) {
-        int i;
-        DPRINTF("Response:");
-        for (i = 0; i < rsplen; i++)
-            fprintf(stderr, " %02x", response[i]);
-        fprintf(stderr, " state %d\n", sd->state);
-    } else {
-        DPRINTF("No response %d\n", sd->state);
+    if (trace_event_get_state_backends(TRACE_SDCARD_COMMAND_RESPONSE)) {
+        char *hexbuf;
+
+        hexbuf = qemu_hexbuf_strdup(response, rsplen, NULL, "(no response)");
+        trace_sdcard_command_response(hexbuf, sd_state_name(sd->state));
+        g_free(hexbuf);
     }
-#endif
 
     return rsplen;
 }
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 0e78f980de..01f94c4b54 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -26,6 +26,7 @@ sdcard_set_mode(const char *current_mode, const char *new_mode, const char *stat
 sdcard_set_state(const char *current_state, const char *new_state, const char *mode) "%s -> %s (mode: %s)"
 sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state) "CMD%d arg 0x%08x (state %s)"
 sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
+sdcard_command_response(const char *hexdump, const char *state) "%s (state %s)"
 sdcard_reset(void) ""
 sdcard_inserted(bool readonly) "read_only: %d"
 sdcard_ejected(void) ""
-- 
2.15.1

[Qemu-devel] [PATCH 15/26] sdcard: use PW_LEN define instead of '16' magic

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2a249b1612..542170c3ec 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -47,6 +47,8 @@
 #define OCR_POWER_UP            0x80000000
 #define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
 
+#define PW_LEN 16
+
 typedef enum {
     sd_r0 = 0,    /* no response */
     sd_r1,        /* normal response command */
@@ -100,7 +102,7 @@ struct SDState {
     uint32_t multi_blk_cnt;
     uint32_t erase_start;
     uint32_t erase_end;
-    uint8_t pwd[16];
+    uint8_t pwd[PW_LEN];
     uint32_t pwd_len;
     uint8_t function_group[6];
 
@@ -571,7 +573,7 @@ static const VMStateDescription sd_vmstate = {
         VMSTATE_UINT32(multi_blk_cnt, SDState),
         VMSTATE_UINT32(erase_start, SDState),
         VMSTATE_UINT32(erase_end, SDState),
-        VMSTATE_UINT8_ARRAY(pwd, SDState, 16),
+        VMSTATE_UINT8_ARRAY(pwd, SDState, PW_LEN),
         VMSTATE_UINT32(pwd_len, SDState),
         VMSTATE_UINT8_ARRAY(function_group, SDState, 6),
         VMSTATE_UINT8(current_cmd, SDState),
@@ -733,7 +735,7 @@ static void sd_lock_command(SDState *sd)
 
     if (sd->blk_len < 2 + pwd_len ||
                     pwd_len <= sd->pwd_len ||
-                    pwd_len > sd->pwd_len + 16) {
+                    pwd_len > sd->pwd_len + sizeof(sd->pwd)) {
         sd->card_status |= LOCK_UNLOCK_FAILED;
         return;
     }
-- 
2.15.1

[Qemu-devel] [PATCH 16/26] sdcard: let cmd_valid_while_locked() returns a bool

[Philippe Mathieu-Daudé]

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 542170c3ec..b231411108 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1475,7 +1475,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     return sd_illegal;
 }
 
-static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
+static bool cmd_valid_while_locked(SDState *sd, SDRequest *req)
 {
     /* Valid commands in locked state:
      * basic class (0)
@@ -1489,7 +1489,7 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
         return req->cmd == 41 || req->cmd == 42;
     }
     if (req->cmd == 16 || req->cmd == 55) {
-        return 1;
+        return true;
     }
     return sd_cmd_class[req->cmd & 0x3F] == 0
             || sd_cmd_class[req->cmd & 0x3F] == 7;
-- 
2.15.1

[Qemu-devel] [PATCH 17/26] sdcard: rename sd_set_REG() functions called by sd_reset() as sd_reset_REG()

[Philippe Mathieu-Daudé]

They are only called once, during reset :)

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b231411108..d8e3f60536 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -245,7 +245,7 @@ static uint16_t sd_crc16(void *message, size_t width)
     return shift_reg;
 }
 
-static void sd_set_ocr(SDState *sd)
+static void sd_reset_ocr(SDState *sd)
 {
     /* All voltages OK, Standard Capacity SD Memory Card, not yet powered up */
     sd->ocr = 0x00ffff00;
@@ -260,7 +260,7 @@ static void sd_ocr_powerup(void *opaque)
     sd->ocr |= OCR_POWER_UP;
 }
 
-static void sd_set_scr(SDState *sd)
+static void sd_reset_scr(SDState *sd)
 {
     sd->scr[0] = 0x00;		/* SCR Structure */
     sd->scr[1] = 0x2f;		/* SD Security Support */
@@ -279,7 +279,7 @@ static void sd_set_scr(SDState *sd)
 #define MDT_YR	2006
 #define MDT_MON	2
 
-static void sd_set_cid(SDState *sd)
+static void sd_reset_cid(SDState *sd)
 {
     sd->cid[0] = MID;		/* Fake card manufacturer ID (MID) */
     sd->cid[1] = OID[0];	/* OEM/Application ID (OID) */
@@ -311,7 +311,7 @@ static const uint8_t sd_csd_rw_mask[16] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xfc, 0xfe,
 };
 
-static void sd_set_csd(SDState *sd, uint64_t size)
+static void sd_reset_csd(SDState *sd, uint64_t size)
 {
     uint32_t csize = (size >> (CMULT_SHIFT + HWBLOCK_SHIFT)) - 1;
     uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
@@ -380,12 +380,12 @@ static void sd_set_rca(SDState *sd)
 #define CARD_STATUS_B	0x00c01e00
 #define CARD_STATUS_C	0xfd39a028
 
-static void sd_set_cardstatus(SDState *sd)
+static void sd_reset_cardstatus(SDState *sd)
 {
     sd->card_status = 0x00000100;
 }
 
-static void sd_set_sdstatus(SDState *sd)
+static void sd_reset_sdstatus(SDState *sd)
 {
     memset(sd->sd_status, 0, 64);
 }
@@ -455,12 +455,12 @@ static void sd_reset(DeviceState *dev)
 
     sd_set_state(sd, sd_idle_state);
     sd->rca = 0x0000;
-    sd_set_ocr(sd);
-    sd_set_scr(sd);
-    sd_set_cid(sd);
-    sd_set_csd(sd, size);
-    sd_set_cardstatus(sd);
-    sd_set_sdstatus(sd);
+    sd_reset_ocr(sd);
+    sd_reset_scr(sd);
+    sd_reset_cid(sd);
+    sd_reset_csd(sd, size);
+    sd_reset_cardstatus(sd);
+    sd_reset_sdstatus(sd);
 
     g_free(sd->wp_groups);
     sd->wp_switch = sd->blk ? blk_is_read_only(sd->blk) : false;
-- 
2.15.1

[Qemu-devel] [PATCH 18/26] sdcard: move Memory Card registers together

[Philippe Mathieu-Daudé]

Aesthetic, but ease review.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d8e3f60536..992236df75 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -85,14 +85,17 @@ struct SDState {
 
     uint32_t mode;    /* current card mode, one of SDCardModes */
     int32_t state;    /* current card state, one of SDCardStates */
-    uint32_t ocr;
     QEMUTimer *ocr_power_timer;
-    uint8_t scr[8];
-    uint8_t cid[16];
-    uint8_t csd[16];
-    uint16_t rca;
-    uint32_t card_status;
-    uint8_t sd_status[64];
+
+    /* SD Memory Card Registers */
+    uint8_t cid[128 / BITS_PER_BYTE];       /* Card Identification Number */
+    uint16_t rca;                           /* Relative Card Address */
+    uint8_t csd[128 / BITS_PER_BYTE];       /* Card Specific Data */
+    uint8_t scr[64  / BITS_PER_BYTE];       /* SD Configuration */
+    uint32_t ocr;                           /* Operation Conditions */
+    uint8_t sd_status[512 / BITS_PER_BYTE]; /* SD Status */
+    uint32_t card_status;                   /* Card Status */
+
     uint32_t vhs;
     bool wp_switch;
     unsigned long *wp_groups;
-- 
2.15.1

[Qemu-devel] [PATCH 19/26] sdcard: add DSR register

[Philippe Mathieu-Daudé]

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 992236df75..8a10e28080 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -90,6 +90,7 @@ struct SDState {
     /* SD Memory Card Registers */
     uint8_t cid[128 / BITS_PER_BYTE];       /* Card Identification Number */
     uint16_t rca;                           /* Relative Card Address */
+    uint16_t dsr;                           /* Driver Stage */
     uint8_t csd[128 / BITS_PER_BYTE];       /* Card Specific Data */
     uint8_t scr[64  / BITS_PER_BYTE];       /* SD Configuration */
     uint32_t ocr;                           /* Operation Conditions */
@@ -458,6 +459,7 @@ static void sd_reset(DeviceState *dev)
 
     sd_set_state(sd, sd_idle_state);
     sd->rca = 0x0000;
+    sd->dsr = 0x0404;
     sd_reset_ocr(sd);
     sd_reset_scr(sd);
     sd_reset_cid(sd);
-- 
2.15.1

[Qemu-devel] [PATCH 20/26] sdcard: add/use SD_CMD_MAX to check valid SD commands

[Philippe Mathieu-Daudé]

We check once in sd_do_command() if the command is valid
(and remove duplicate checks in sd_normal_command() and
cmd_valid_while_locked()).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8a10e28080..99678c89d5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -198,18 +198,23 @@ static void sd_set_state(SDState *sd, enum SDCardStates state)
     }
 }
 
-static const sd_cmd_type_t sd_cmd_type[64] = {
+#define SD_CMD_MAX 64
+
+static const sd_cmd_type_t sd_cmd_type[SD_CMD_MAX] = {
     sd_bc,   sd_none, sd_bcr,  sd_bcr,  sd_none, sd_none, sd_none, sd_ac,
     sd_bcr,  sd_ac,   sd_ac,   sd_adtc, sd_ac,   sd_ac,   sd_none, sd_ac,
+    /* 16 */
     sd_ac,   sd_adtc, sd_adtc, sd_none, sd_none, sd_none, sd_none, sd_none,
     sd_adtc, sd_adtc, sd_adtc, sd_adtc, sd_ac,   sd_ac,   sd_adtc, sd_none,
+    /* 32 */
     sd_ac,   sd_ac,   sd_none, sd_none, sd_none, sd_none, sd_ac,   sd_none,
     sd_none, sd_none, sd_bc,   sd_none, sd_none, sd_none, sd_none, sd_none,
+    /* 48 */
     sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_ac,
     sd_adtc, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none,
 };
 
-static const int sd_cmd_class[64] = {
+static const int sd_cmd_class[SD_CMD_MAX] = {
     0,  0,  0,  0,  0,  9, 10,  0,  0,  0,  0,  1,  0,  0,  0,  0,
     2,  2,  2,  2,  3,  3,  3,  3,  4,  4,  4,  4,  6,  6,  6,  6,
     5,  5, 10, 10, 10, 10,  5,  9,  9,  9,  7,  7,  7,  7,  7,  7,
@@ -787,8 +792,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
     /* Not interpreting this as an app command */
     sd->card_status &= ~APP_CMD;
 
-    if (sd_cmd_type[req.cmd & 0x3F] == sd_ac
-        || sd_cmd_type[req.cmd & 0x3F] == sd_adtc) {
+    if (sd_cmd_type[req.cmd] & sd_ac || sd_cmd_type[req.cmd] & sd_adtc) {
         rca = req.arg >> 16;
     }
 
@@ -1496,8 +1500,8 @@ static bool cmd_valid_while_locked(SDState *sd, SDRequest *req)
     if (req->cmd == 16 || req->cmd == 55) {
         return true;
     }
-    return sd_cmd_class[req->cmd & 0x3F] == 0
-            || sd_cmd_class[req->cmd & 0x3F] == 7;
+    return sd_cmd_class[req->cmd] == 0
+            || sd_cmd_class[req->cmd] == 7;
 }
 
 int sd_do_command(SDState *sd, SDRequest *req, uint8_t *response)
@@ -1509,6 +1513,11 @@ int sd_do_command(SDState *sd, SDRequest *req, uint8_t *response)
     if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable) {
         return 0;
     }
+    if (req->cmd > SD_CMD_MAX) {
+        qemu_log_mask(LOG_GUEST_ERROR, "SD: incorrect command 0x%02x\n",
+                      req->cmd);
+        req->cmd &= 0x3f;
+    }
 
     if (sd_req_crc_validate(req)) {
         sd->card_status |= COM_CRC_ERROR;
-- 
2.15.1

[Qemu-devel] [PATCH 21/26] sdcard: add sd_cmd_abbreviation() to resolve the SD command id

[Philippe Mathieu-Daudé]

This simplify reading trace output :)

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 99678c89d5..c50ac752d4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -200,6 +200,45 @@ static void sd_set_state(SDState *sd, enum SDCardStates state)
 
 #define SD_CMD_MAX 64
 
+static const char *sd_cmd_abbreviation(uint8_t cmd)
+{
+    static const char *cmd_abbrev[SD_CMD_MAX] = {
+         [0]    = "GO_IDLE_STATE",
+         [2]    = "ALL_SEND_CID",            [3]    = "SEND_RELATIVE_ADDR",
+         [4]    = "SET_DSR",                 [5]    = "IO_SEND_OP_COND",
+         [6]    = "SWITCH_FUNC",             [7]    = "SELECT/DESELECT_CARD",
+         [8]    = "SEND_IF_COND",            [9]    = "SEND_CSD",
+        [10]    = "SEND_CID",               [11]    = "VOLTAGE_SWITCH",
+        [12]    = "STOP_TRANSMISSION",      [13]    = "SEND_STATUS",
+                                            [15]    = "GO_INACTIVE_STATE",
+        [16]    = "SET_BLOCKLEN",           [17]    = "READ_SINGLE_BLOCK",
+        [18]    = "READ_MULTIPLE_BLOCK",    [19]    = "SEND_TUNING_BLOCK",
+        [20]    = "SPEED_CLASS_CONTROL",    [21]    = "DPS_spec",
+                                            [23]    = "SET_BLOCK_COUNT",
+        [24]    = "WRITE_BLOCK",            [25]    = "WRITE_MULTIPLE_BLOCK",
+        [26]    = "MANUF_RSVD",             [27]    = "PROGRAM_CSD",
+        [28]    = "SET_WRITE_PROT",         [29]    = "CLR_WRITE_PROT",
+        [30]    = "SEND_WRITE_PROT",
+        [32]    = "ERASE_WR_BLK_START",     [33]    = "ERASE_WR_BLK_END",
+        [34]    = "SW_FUNC_RSVD",           [35]    = "SW_FUNC_RSVD",
+        [36]    = "SW_FUNC_RSVD",           [37]    = "SW_FUNC_RSVD",
+        [38]    = "ERASE",
+        [40]    = "DPS_spec",
+        [42]    = "LOCK_UNLOCK",            [43]    = "Q_MANAGEMENT",
+        [44]    = "Q_TASK_INFO_A",          [45]    = "Q_TASK_INFO_B",
+        [46]    = "Q_RD_TASK",              [47]    = "Q_WR_TASK",
+        [48]    = "READ_EXTR_SINGLE",       [49]    = "WRITE_EXTR_SINGLE",
+        [50]    = "SW_FUNC_RSVD", /* FIXME */
+        [52]    = "IO_RW_DIRECT",           [53]    = "IO_RW_EXTENDED",
+        [54]    = "SDIO_RSVD",              [55]    = "APP_CMD",
+        [56]    = "GEN_CMD",                [57]    = "SW_FUNC_RSVD",
+        [58]    = "READ_EXTR_MULTI",        [59]    = "WRITE_EXTR_MULTI",
+        [60]    = "MANUF_RSVD",             [61]    = "MANUF_RSVD",
+        [62]    = "MANUF_RSVD",             [63]    = "MANUF_RSVD",
+    };
+    return cmd_abbrev[cmd] ? cmd_abbrev[cmd] : "UNKNOWN_CMD";
+}
+
 static const sd_cmd_type_t sd_cmd_type[SD_CMD_MAX] = {
     sd_bc,   sd_none, sd_bcr,  sd_bcr,  sd_none, sd_none, sd_none, sd_ac,
     sd_bcr,  sd_ac,   sd_ac,   sd_adtc, sd_ac,   sd_ac,   sd_none, sd_ac,
@@ -787,7 +826,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
     uint32_t rca = 0x0000;
     uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
 
-    trace_sdcard_normal_command(req.cmd, req.arg, sd_state_name(sd->state));
+    trace_sdcard_normal_command(req.cmd, req.arg, sd_state_name(sd->state),
+                                sd_cmd_abbreviation(req.cmd));
 
     /* Not interpreting this as an app command */
     sd->card_status &= ~APP_CMD;
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 01f94c4b54..f3714a6dc5 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -24,7 +24,7 @@ sdhci_led(bool state) "LED: %u"
 # hw/sd/sd.c
 sdcard_set_mode(const char *current_mode, const char *new_mode, const char *state) "%s -> %s (state: %s)"
 sdcard_set_state(const char *current_state, const char *new_state, const char *mode) "%s -> %s (mode: %s)"
-sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state) "CMD%d arg 0x%08x (state %s)"
+sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state, const char *cmd_desc) "CMD%d arg 0x%08x (state %s) %s"
 sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
 sdcard_command_response(const char *hexdump, const char *state) "%s (state %s)"
 sdcard_reset(void) ""
-- 
2.15.1

[Qemu-devel] [PATCH 22/26] sdcard: reduce sd_cmd traces

[Philippe Mathieu-Daudé]

Do not trace CMD55 used to start an ACMD.

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index c50ac752d4..8104d6c055 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -826,8 +826,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
     uint32_t rca = 0x0000;
     uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
 
-    trace_sdcard_normal_command(req.cmd, req.arg, sd_state_name(sd->state),
-                                sd_cmd_abbreviation(req.cmd));
+    if (req.cmd != 55 || sd->expecting_acmd) {
+        trace_sdcard_normal_command(sd_cmd_abbreviation(req.cmd), req.cmd,
+                                    req.arg, sd_state_name(sd->state));
+    }
 
     /* Not interpreting this as an app command */
     sd->card_status &= ~APP_CMD;
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index f3714a6dc5..412f01a832 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -24,7 +24,7 @@ sdhci_led(bool state) "LED: %u"
 # hw/sd/sd.c
 sdcard_set_mode(const char *current_mode, const char *new_mode, const char *state) "%s -> %s (state: %s)"
 sdcard_set_state(const char *current_state, const char *new_state, const char *mode) "%s -> %s (mode: %s)"
-sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state, const char *cmd_desc) "CMD%d arg 0x%08x (state %s) %s"
+sdcard_normal_command(const char *cmd_desc, uint8_t cmd, uint32_t arg, const char *state) "%20s/ CMD%02d arg 0x%08x (state %s)"
 sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
 sdcard_command_response(const char *hexdump, const char *state) "%s (state %s)"
 sdcard_reset(void) ""
-- 
2.15.1

[Qemu-devel] [PATCH 23/26] sdcard: add ACMD trace events

[Philippe Mathieu-Daudé]

and his sd_acmd_abbreviation() companion to pretty-print the ACMD id.

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

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8104d6c055..b8a5a344f5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -239,6 +239,27 @@ static const char *sd_cmd_abbreviation(uint8_t cmd)
     return cmd_abbrev[cmd] ? cmd_abbrev[cmd] : "UNKNOWN_CMD";
 }
 
+static const char *sd_acmd_abbreviation(uint8_t cmd)
+{
+    static const char *acmd_abbrev[SD_CMD_MAX] = {
+         [6] = "SET_BUS_WIDTH",
+        [13] = "SD_STATUS",
+        [14] = "DPS_spec",                  [15] = "DPS_spec",
+        [16] = "DPS_spec",
+        [18] = "SECU_spec",
+        [22] = "SEND_NUM_WR_BLOCKS",        [23] = "SET_WR_BLK_ERASE_COUNT",
+        [41] = "SD_SEND_OP_COND",
+        [42] = "SET_CLR_CARD_DETECT",
+        [51] = "SEND_SCR",
+        [52] = "SECU_spec",                 [53] = "SECU_spec",
+        [54] = "SECU_spec",
+        [56] = "SECU_spec",                 [57] = "SECU_spec",
+        [58] = "SECU_spec",                 [59] = "SECU_spec",
+    };
+
+    return acmd_abbrev[cmd] ? acmd_abbrev[cmd] : "UNKNOWN_ACMD";
+}
+
 static const sd_cmd_type_t sd_cmd_type[SD_CMD_MAX] = {
     sd_bc,   sd_none, sd_bcr,  sd_bcr,  sd_none, sd_none, sd_none, sd_ac,
     sd_bcr,  sd_ac,   sd_ac,   sd_adtc, sd_ac,   sd_ac,   sd_none, sd_ac,
@@ -1397,7 +1418,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 static sd_rsp_type_t sd_app_command(SDState *sd,
                                     SDRequest req)
 {
-    trace_sdcard_app_command(req.cmd, req.arg);
+    trace_sdcard_app_command(sd_acmd_abbreviation(req.cmd), req.cmd,
+                             req.arg, sd_state_name(sd->state));
+
     sd->card_status |= APP_CMD;
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 412f01a832..651e3fb303 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -25,7 +25,7 @@ sdhci_led(bool state) "LED: %u"
 sdcard_set_mode(const char *current_mode, const char *new_mode, const char *state) "%s -> %s (state: %s)"
 sdcard_set_state(const char *current_state, const char *new_state, const char *mode) "%s -> %s (mode: %s)"
 sdcard_normal_command(const char *cmd_desc, uint8_t cmd, uint32_t arg, const char *state) "%20s/ CMD%02d arg 0x%08x (state %s)"
-sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
+sdcard_app_command(const char *acmd_desc, uint8_t acmd, uint32_t arg, const char *state) "%23s/ACMD%02d arg 0x%08x (state %s)"
 sdcard_command_response(const char *hexdump, const char *state) "%s (state %s)"
 sdcard_reset(void) ""
 sdcard_inserted(bool readonly) "read_only: %d"
-- 
2.15.1

[Qemu-devel] [PATCH 24/26] sdcard: use a 16-bit type for the 16-bit RCA register

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b8a5a344f5..9b90987d7f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -844,7 +844,7 @@ static void sd_lock_command(SDState *sd)
 static sd_rsp_type_t sd_normal_command(SDState *sd,
                                        SDRequest req)
 {
-    uint32_t rca = 0x0000;
+    uint16_t rca = 0x0000;
     uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
 
     if (req.cmd != 55 || sd->expecting_acmd) {
-- 
2.15.1

[Qemu-devel] [PATCH 25/26] sdcard: add/use a SDCardCommandClass enum instead of magic numbers

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 9b90987d7f..688bb40bd1 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -274,11 +274,45 @@ static const sd_cmd_type_t sd_cmd_type[SD_CMD_MAX] = {
     sd_adtc, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none,
 };
 
-static const int sd_cmd_class[SD_CMD_MAX] = {
-    0,  0,  0,  0,  0,  9, 10,  0,  0,  0,  0,  1,  0,  0,  0,  0,
-    2,  2,  2,  2,  3,  3,  3,  3,  4,  4,  4,  4,  6,  6,  6,  6,
-    5,  5, 10, 10, 10, 10,  5,  9,  9,  9,  7,  7,  7,  7,  7,  7,
-    7,  7, 10,  7,  9,  9,  9,  8,  8, 10,  8,  8,  8,  8,  8,  8,
+enum SDCardCommandClass {
+    ccc_basic       = 1 << 0,
+    ccc_blk_read    = 1 << 2,
+    ccc_blk_write   = 1 << 4,
+    ccc_erase       = 1 << 5,
+    ccc_wp          = 1 << 6,
+    ccc_lock        = 1 << 7,
+    ccc_appspec     = 1 << 8,
+    ccc_io          = 1 << 9,
+    ccc_switch      = 1 << 10,
+    ccc_ext         = 1 << 11,
+};
+
+static const uint32_t sd_cmd_class[SD_CMD_MAX] = {
+    ccc_basic, 0, ccc_basic, ccc_basic,
+    ccc_basic, ccc_io, ccc_switch, ccc_basic,
+    ccc_basic, ccc_basic, ccc_basic, ccc_basic,
+    ccc_basic, ccc_basic, 0, ccc_basic,
+    /* 16 */
+    ccc_blk_read | ccc_blk_write | ccc_lock, ccc_blk_read,
+    ccc_blk_read, ccc_blk_read,
+    ccc_blk_read | ccc_blk_write, ccc_ext, 0,
+    ccc_blk_read | ccc_blk_write,
+    ccc_blk_write, ccc_blk_write,
+    0, ccc_blk_write,
+    ccc_wp, ccc_wp,
+    ccc_wp, 0,
+    /* 32 */
+    ccc_erase, ccc_erase,
+    ccc_switch, ccc_switch,
+    ccc_switch, ccc_switch,
+    ccc_erase, 0,
+    ccc_lock, 0, ccc_lock, 0, 0, 0, 0, 0,
+    /* 48 */
+    ccc_ext, ccc_ext, ccc_switch, 0,
+    ccc_io, ccc_io, 0, ccc_appspec,
+    ccc_appspec, ccc_switch,
+    ccc_ext, ccc_ext,
+    0, 0, 0, 0
 };
 
 static uint8_t sd_crc7(void *message, size_t width)
@@ -1565,8 +1599,8 @@ static bool cmd_valid_while_locked(SDState *sd, SDRequest *req)
     if (req->cmd == 16 || req->cmd == 55) {
         return true;
     }
-    return sd_cmd_class[req->cmd] == 0
-            || sd_cmd_class[req->cmd] == 7;
+    return sd_cmd_class[req->cmd] == ccc_basic
+        || sd_cmd_class[req->cmd] == ccc_lock;
 }
 
 int sd_do_command(SDState *sd, SDRequest *req, uint8_t *response)
-- 
2.15.1

[Qemu-devel] [PATCH 26/26] sdcard: add/use a ccc_spi enum for the commands supported in SPI mode

[Philippe Mathieu-Daudé]

We can now simplify the 'if (sd->spi) ...' checks, reporting an UNIMP error
and returning a sd_illegal value.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 83 +++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 41 insertions(+), 42 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 688bb40bd1..a32da6af9f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -285,34 +285,45 @@ enum SDCardCommandClass {
     ccc_io          = 1 << 9,
     ccc_switch      = 1 << 10,
     ccc_ext         = 1 << 11,
+
+    /* extra */
+    ccc_spi         = 1 << 31,
 };
 
 static const uint32_t sd_cmd_class[SD_CMD_MAX] = {
-    ccc_basic, 0, ccc_basic, ccc_basic,
-    ccc_basic, ccc_io, ccc_switch, ccc_basic,
-    ccc_basic, ccc_basic, ccc_basic, ccc_basic,
-    ccc_basic, ccc_basic, 0, ccc_basic,
+    ccc_basic | ccc_spi, ccc_spi, ccc_basic, ccc_basic,
+    ccc_basic, ccc_io, ccc_switch | ccc_spi, ccc_basic,
+    ccc_basic, ccc_basic | ccc_spi, ccc_basic | ccc_spi, ccc_basic,
+    ccc_basic | ccc_spi, ccc_basic | ccc_spi, 0, ccc_basic,
     /* 16 */
-    ccc_blk_read | ccc_blk_write | ccc_lock, ccc_blk_read,
-    ccc_blk_read, ccc_blk_read,
+    ccc_blk_read | ccc_blk_write | ccc_lock | ccc_spi, ccc_blk_read | ccc_spi,
+    ccc_blk_read | ccc_spi, ccc_blk_read,
     ccc_blk_read | ccc_blk_write, ccc_ext, 0,
     ccc_blk_read | ccc_blk_write,
-    ccc_blk_write, ccc_blk_write,
-    0, ccc_blk_write,
-    ccc_wp, ccc_wp,
-    ccc_wp, 0,
+    ccc_blk_write | ccc_spi, ccc_blk_write | ccc_spi,
+    0, ccc_blk_write | ccc_spi,
+    ccc_wp | ccc_spi, ccc_wp | ccc_spi,
+    ccc_wp | ccc_spi, 0,
     /* 32 */
-    ccc_erase, ccc_erase,
-    ccc_switch, ccc_switch,
-    ccc_switch, ccc_switch,
-    ccc_erase, 0,
-    ccc_lock, 0, ccc_lock, 0, 0, 0, 0, 0,
+    ccc_erase | ccc_spi, ccc_erase | ccc_spi,
+    ccc_switch | ccc_spi, ccc_switch | ccc_spi,
+    ccc_switch | ccc_spi, ccc_switch | ccc_spi,
+    ccc_erase | ccc_spi, 0,
+    ccc_lock, 0, ccc_lock | ccc_spi, 0, 0, 0, 0, 0,
     /* 48 */
-    ccc_ext, ccc_ext, ccc_switch, 0,
-    ccc_io, ccc_io, 0, ccc_appspec,
-    ccc_appspec, ccc_switch,
-    ccc_ext, ccc_ext,
+    ccc_ext, ccc_ext, ccc_switch | ccc_spi, 0,
+    ccc_io | ccc_spi, ccc_io | ccc_spi, 0, ccc_appspec | ccc_spi,
+    ccc_appspec | ccc_spi, ccc_switch | ccc_spi,
+    ccc_ext | ccc_spi, ccc_ext | ccc_spi,
     0, 0, 0, 0
+}, sd_acmd_class[SD_CMD_MAX] = {
+    [6] = ccc_appspec, [13] = ccc_appspec | ccc_spi,
+    [14] = ccc_appspec, [15] = ccc_appspec,
+    [16] = ccc_appspec, [18] = ccc_spi,
+    [22] = ccc_appspec | ccc_spi, [23] = ccc_appspec | ccc_spi,
+    [25] = ccc_spi, [26] = ccc_spi, [28] = ccc_appspec, [38] = ccc_spi,
+    [41] = ccc_appspec | ccc_spi, [42] = ccc_appspec | ccc_spi, [43] = ccc_spi,
+    [49] = ccc_spi, [51] = ccc_appspec | ccc_spi,
 };
 
 static uint8_t sd_crc7(void *message, size_t width)
@@ -899,6 +910,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         sd->multi_blk_cnt = 0;
     }
 
+    if (sd->spi && !(sd_cmd_class[req.cmd] & ccc_spi)) {
+        qemu_log_mask(LOG_UNIMP, "SD: CMD%02u not implemented in SPI mode\n",
+                      req.cmd);
+        return sd_illegal;
+    }
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
     case 0:	/* CMD0:   GO_IDLE_STATE */
@@ -921,8 +937,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         return sd_r1;
 
     case 2:	/* CMD2:   ALL_SEND_CID */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_ready_state:
             sd_set_state(sd, sd_identification_state);
@@ -934,8 +948,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         break;
 
     case 3:	/* CMD3:   SEND_RELATIVE_ADDR */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_identification_state:
         case sd_standby_state:
@@ -949,8 +961,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         break;
 
     case 4:	/* CMD4:   SEND_DSR */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_standby_state:
             break;
@@ -964,8 +974,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         return sd_illegal;
 
     case 6:	/* CMD6:   SWITCH_FUNCTION */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->mode) {
         case sd_data_transfer_mode:
             sd_function_switch(sd, req.arg);
@@ -980,8 +988,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         break;
 
     case 7:	/* CMD7:   SELECT/DESELECT_CARD */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_standby_state:
             if (sd->rca != rca)
@@ -1082,8 +1088,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         break;
 
     case 11:	/* CMD11:  READ_DAT_UNTIL_STOP */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_transfer_state:
             sd_set_state(sd, sd_sendingdata_state);
@@ -1130,8 +1134,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         break;
 
     case 15:	/* CMD15:  GO_INACTIVE_STATE */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->mode) {
         case sd_data_transfer_mode:
             if (sd->rca != rca)
@@ -1206,8 +1208,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 
     /* Block write commands (Class 4) */
     case 24:	/* CMD24:  WRITE_SINGLE_BLOCK */
-        if (sd->spi)
-            goto unimplemented_cmd;
         switch (sd->state) {
         case sd_transfer_state:
             /* Writing in SPI mode not implemented.  */
@@ -1232,8 +1232,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         break;
 
     case 25:	/* CMD25:  WRITE_MULTIPLE_BLOCK */
-        if (sd->spi)
-            goto unimplemented_cmd;
         switch (sd->state) {
         case sd_transfer_state:
             /* Writing in SPI mode not implemented.  */
@@ -1258,8 +1256,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         break;
 
     case 26:	/* CMD26:  PROGRAM_CID */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_transfer_state:
             sd_set_state(sd, sd_receivingdata_state);
@@ -1274,7 +1270,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 
     case 27:	/* CMD27:  PROGRAM_CSD */
         if (sd->spi)
-            goto unimplemented_cmd;
+            goto unimplemented_cmd; /* XXX */
         switch (sd->state) {
         case sd_transfer_state:
             sd_set_state(sd, sd_receivingdata_state);
@@ -1384,8 +1380,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 
     /* Lock card commands (Class 7) */
     case 42:	/* CMD42:  LOCK_UNLOCK */
-        if (sd->spi)
-            goto unimplemented_cmd;
         switch (sd->state) {
         case sd_transfer_state:
             sd_set_state(sd, sd_receivingdata_state);
@@ -1456,6 +1450,11 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
                              req.arg, sd_state_name(sd->state));
 
     sd->card_status |= APP_CMD;
+    if (sd->spi && !(sd_acmd_class[req.cmd] & ccc_spi)) {
+        qemu_log_mask(LOG_UNIMP, "SD: ACMD%02u not implemented in SPI mode\n",
+                      req.cmd);
+        return sd_illegal;
+    }
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
         switch (sd->state) {
-- 
2.15.1

Re: [Qemu-devel] [PATCH 00/26] SDCard housekeeping

[Philippe Mathieu-Daudé]

attaching a more complete boot log.

> Once this series applied, booting some Linux on a vexpress-a9 with few
> sd*command traces enabled result in this outputs (which result useful to
> understand/fix the different SD implementations):

$ cat /tmp/events
sd*command*
sdcard_set_mode
sdcard_read_block

$ qemu-system-arm \
  -M vexpress-a9 \
  -kernel vmlinuz-3.2.0-4-vexpress -initrd initrd.img-3.2.0-4-vexpress \
  -drive if=sd,cache=unsafe,file=a10-debian-server-2gb.2014-02-17.img \
  -append "root=/dev/mmcblk0p2 console=ttyAMA0 console=tty" -nographic \
  -trace events=/tmp/events
Uncompressing Linux... done, booting the kernel.
[    0.000000] Booting Linux on physical CPU 0
[    0.000000] Linux version 3.2.0-4-vexpress
(debian-kernel@lists.debian.org) (gcc version 4.6.3 (Debian 4.6.3-12)
) #1 SMP Debian 3.2.35-2
[    0.000000] CPU: ARMv7 Processor [410fc090] revision 0 (ARMv7), cr=10c5387d
[    0.000000] Machine: ARM-Versatile Express
[    0.000000] Kernel command line: root=/dev/mmcblk0p2
console=ttyAMA0 console=tty
[    0.000000] Memory: 128MB = 128MB total
[    0.000000] Console: colour dummy device 80x30
[    0.000000] console [tty0] enabled
[    0.319496] Brought up 1 CPUs
[    0.319905] SMP: Total of 1 processors activated (617.67 BogoMIPS).
[    0.444966] Serial: AMBA PL011 UART driver
[    0.450544] mb:uart0: ttyAMA0 at MMIO 0x10009000 (irq = 37) is a PL011 rev1
[    0.481078] console [ttyAMA0] enabled
[    3.081225] mmci-pl18x mb:mmci: mmc0: PL181 manf 41 rev0 at
0x10005000 irq 41,42 (pio)
86.660802:sdcard_set_mode inactive -> card_identification (state: idle)
86.660815:sdcard_normal_command         IO_RW_DIRECT/ CMD52 arg
0x00000c00 (state idle)
86.660829:sdcard_command_response (no response) (state idle)
86.689998:sdcard_normal_command         IO_RW_DIRECT/ CMD52 arg
0x80000c08 (state idle)
86.690014:sdcard_command_response (no response) (state idle)
86.692380:sdcard_normal_command        GO_IDLE_STATE/ CMD00 arg
0x00000000 (state idle)
86.692392:sdcard_command_response (no response) (state idle)
86.695229:sdcard_normal_command         SEND_IF_COND/ CMD08 arg
0x000001aa (state idle)
86.695247:sdcard_command_response 00 00 01 aa  (state idle)
86.695887:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg
0x00000000 (state idle)
86.695898:sdcard_command_response (no response) (state idle)
86.696065:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg
0x00000000 (state idle)
86.696072:sdcard_command_response (no response) (state idle)
86.696198:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg
0x00000000 (state idle)
86.696207:sdcard_command_response (no response) (state idle)
86.696311:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg
0x00000000 (state idle)
86.696320:sdcard_command_response (no response) (state idle)
[    3.298428] mmc0: host does not support reading read-only switch.
assuming write-enable.
86.697384:sdcard_command_response 00 40 01 20  (state idle)
86.697639:sdcard_app_command         SD_SEND_OP_COND/ACMD41 arg
0x00000000 (state idle)
86.697656:sdcard_command_response 40 ff ff 00  (state idle)
86.699975:sdcard_normal_command        GO_IDLE_STATE/ CMD00 arg
0x00000000 (state idle)
86.700000:sdcard_command_response (no response) (state idle)
86.732420:sdcard_normal_command         SEND_IF_COND/ CMD08 arg
0x000001aa (state idle)
86.732460:sdcard_command_response 00 00 01 aa  (state idle)
86.732700:sdcard_command_response 00 00 01 20  (state idle)
86.732812:sdcard_app_command         SD_SEND_OP_COND/ACMD41 arg
0x40300000 (state idle)
86.732825:sdcard_command_response c0 ff ff 00  (state ready)
86.733272:sdcard_normal_command         ALL_SEND_CID/ CMD02 arg
0x00000000 (state ready)
86.733307:sdcard_command_response aa 58 59 51 45 4d 55 21 01 de ad be
ef 00 62 19  (state identification)
86.734086:sdcard_normal_command   SEND_RELATIVE_ADDR/ CMD03 arg
0x00000000 (state identification)
86.734107:sdcard_command_response 45 67 05 00  (state standby)
86.734573:sdcard_set_mode card_identification -> data_transfer (state: standby)
86.734580:sdcard_normal_command             SEND_CSD/ CMD09 arg
0x45670000 (state standby)
86.734593:sdcard_command_response 40 0e 00 32 5b 59 00 00 0d 47 7f 80
0a 40 00 00  (state standby)
86.735447:sdcard_normal_command SELECT/DESELECT_CARD/ CMD07 arg
0x45670000 (state standby)
86.735462:sdcard_command_response 00 00 07 00  (state transfer)
86.735917:sdcard_command_response 00 00 09 20  (state transfer)
86.737808:sdcard_app_command                SEND_SCR/ACMD51 arg
0x00000000 (state transfer)
86.737818:sdcard_command_response 00 00 09 20  (state sendingdata)
86.739778:sdcard_command_response 00 00 09 20  (state transfer)
86.740004:sdcard_app_command               SD_STATUS/ACMD13 arg
0x00000000 (state transfer)
86.740011:sdcard_command_response 00 00 09 20  (state sendingdata)
[    3.319526] mmc0: new SDHC card at address 4567
[    3.401452] mmcblk0: mmc0:4567 QEMU! 1.66 GiB
86.939168:sdcard_normal_command  READ_MULTIPLE_BLOCK/ CMD18 arg
0x00000000 (state transfer)
86.939198:sdcard_command_response 00 00 09 00  (state sendingdata)
86.939201:sdcard_read_block addr 0x00000000 size 0x200
86.939768:sdcard_read_block addr 0x00000200 size 0x200
86.940137:sdcard_read_block addr 0x00000400 size 0x200
86.940549:sdcard_read_block addr 0x00000600 size 0x200
86.940918:sdcard_read_block addr 0x00000800 size 0x200
86.941506:sdcard_read_block addr 0x00000a00 size 0x200
86.941796:sdcard_read_block addr 0x00000c00 size 0x200
86.942080:sdcard_read_block addr 0x00000e00 size 0x200
86.942451:sdcard_normal_command    STOP_TRANSMISSION/ CMD12 arg
0x00000000 (state sendingdata)
86.942463:sdcard_command_response 00 00 0b 00  (state transfer)
[    3.522382]  mmcblk0: p1 p2
88.022437:sdcard_normal_command          SEND_STATUS/ CMD13 arg
0x45670000 (state transfer)
88.022459:sdcard_command_response 00 00 09 00  (state transfer)
89.025296:sdcard_normal_command          SEND_STATUS/ CMD13 arg
0x45670000 (state transfer)
89.025315:sdcard_command_response 00 00 09 00  (state transfer)
[    5.777261] udevd[45]: starting version 175
90.032049:sdcard_normal_command          SEND_STATUS/ CMD13 arg
0x45670000 (state transfer)
90.032069:sdcard_command_response 00 00 09 00  (state transfer)
91.032126:sdcard_normal_command          SEND_STATUS/ CMD13 arg
0x45670000 (state transfer)
91.032148:sdcard_command_response 00 00 09 00  (state transfer)
91.854517:sdcard_normal_command  READ_MULTIPLE_BLOCK/ CMD18 arg
0x00351f80 (state transfer)
91.854547:sdcard_command_response 00 00 09 00  (state sendingdata)
91.854550:sdcard_read_block addr 0x6a3f0000 size 0x200
91.856100:sdcard_read_block addr 0x6a3f0200 size 0x200
91.856393:sdcard_read_block addr 0x6a3f0400 size 0x200
91.856692:sdcard_read_block addr 0x6a3f0600 size 0x200
91.857342:sdcard_read_block addr 0x6a3f0800 size 0x200
91.857652:sdcard_read_block addr 0x6a3f0a00 size 0x200
91.857955:sdcard_read_block addr 0x6a3f0c00 size 0x200
91.858256:sdcard_read_block addr 0x6a3f0e00 size 0x200
91.858979:sdcard_normal_command    STOP_TRANSMISSION/ CMD12 arg
0x00000000 (state sendingdata)
91.858990:sdcard_command_response 00 00 0b 00  (state transfer)
92.003670:sdcard_normal_command  READ_MULTIPLE_BLOCK/ CMD18 arg
0x00351ff0 (state transfer)
92.003692:sdcard_command_response 00 00 09 00  (state sendingdata)
92.003696:sdcard_read_block addr 0x6a3fe000 size 0x200
92.004126:sdcard_read_block addr 0x6a3fe200 size 0x200
92.004418:sdcard_read_block addr 0x6a3fe400 size 0x200
92.004704:sdcard_read_block addr 0x6a3fe600 size 0x200
92.005011:sdcard_read_block addr 0x6a3fe800 size 0x200
92.005313:sdcard_read_block addr 0x6a3fea00 size 0x200
92.005665:sdcard_read_block addr 0x6a3fec00 size 0x200
92.006023:sdcard_read_block addr 0x6a3fee00 size 0x200
92.006466:sdcard_normal_command    STOP_TRANSMISSION/ CMD12 arg
0x00000000 (state sendingdata)
92.006481:sdcard_command_response 00 00 0b 00  (state transfer)
92.022454:sdcard_normal_command  READ_MULTIPLE_BLOCK/ CMD18 arg
0x00000000 (state transfer)
92.022488:sdcard_command_response 00 00 09 00  (state sendingdata)
92.022497:sdcard_read_block addr 0x00000000 size 0x200
92.023021:sdcard_read_block addr 0x00000200 size 0x200
92.023436:sdcard_read_block addr 0x00000400 size 0x200
92.023841:sdcard_read_block addr 0x00000600 size 0x200
92.024179:sdcard_read_block addr 0x00000800 size 0x200
92.024555:sdcard_read_block addr 0x00000a00 size 0x200
92.024849:sdcard_read_block addr 0x00000c00 size 0x200
92.025151:sdcard_read_block addr 0x00000e00 size 0x200
92.025507:sdcard_normal_command    STOP_TRANSMISSION/ CMD12 arg
0x00000000 (state sendingdata)
92.025518:sdcard_command_response 00 00 0b 00  (state transfer)
92.043359:sdcard_normal_command          SEND_STATUS/ CMD13 arg
0x45670000 (state transfer)
92.043403:sdcard_command_response 00 00 09 00  (state transfer)
92.071967:sdcard_normal_command  READ_MULTIPLE_BLOCK/ CMD18 arg
0x00000008 (state transfer)
92.072003:sdcard_command_response 00 00 09 00  (state sendingdata)
92.072008:sdcard_read_block addr 0x00001000 size 0x200
92.072429:sdcard_read_block addr 0x00001200 size 0x200
92.072763:sdcard_read_block addr 0x00001400 size 0x200
92.073064:sdcard_read_block addr 0x00001600 size 0x200
92.073383:sdcard_read_block addr 0x00001800 size 0x200
92.073670:sdcard_read_block addr 0x00001a00 size 0x200
92.073949:sdcard_read_block addr 0x00001c00 size 0x200
92.074224:sdcard_read_block addr 0x00001e00 size 0x200
92.074552:sdcard_normal_command    STOP_TRANSMISSION/ CMD12 arg
0x00000000 (state sendingdata)
92.074562:sdcard_command_response 00 00 0b 00  (state transfer)
92.093569:sdcard_normal_command  READ_MULTIPLE_BLOCK/ CMD18 arg
0x00351ff8 (state transfer)
92.093590:sdcard_command_response 00 00 09 00  (state sendingdata)
92.093596:sdcard_read_block addr 0x6a3ff000 size 0x200
92.093994:sdcard_read_block addr 0x6a3ff200 size 0x200
92.094281:sdcard_read_block addr 0x6a3ff400 size 0x200
92.094568:sdcard_read_block addr 0x6a3ff600 size 0x200
92.094843:sdcard_read_block addr 0x6a3ff800 size 0x200
92.095118:sdcard_read_block addr 0x6a3ffa00 size 0x200
92.095389:sdcard_read_block addr 0x6a3ffc00 size 0x200
92.095654:sdcard_read_block addr 0x6a3ffe00 size 0x200
qemu-system-arm: terminating on signal 15 from pid 12630 (killall)

Re: [Qemu-devel] [PATCH 01/26] sdbus: add a QMP command to access a SDBus

[Kevin Wolf]

Am 14.12.2017 um 00:20 hat Philippe Mathieu-Daudé geschrieben:
> Use Base64 to serialize the binary blobs in JSON.
> So far at most 512 bytes will be transfered, which result
> in a 684 bytes payload.
> Since this command is intented for qtesting, this is acceptable.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Doing this kind of thing over QMP doesn't look right to me. qtests
should access hardware the same way as real guests access the hardware
(i.e. MMIO and I/O ports).

But if for some reason the QMP maintainers were to think that this is
acceptable in QMP, I'd argue it should at least get an x-debug- prefix
to avoid making it a stable API that management tools may rely on.

Kevin

Re: [Qemu-devel] [PATCH 01/26] sdbus: add a QMP command to access a SDBus

[Paolo Bonzini]

On 14/12/2017 10:06, Kevin Wolf wrote:
> Am 14.12.2017 um 00:20 hat Philippe Mathieu-Daudé geschrieben:
>> Use Base64 to serialize the binary blobs in JSON.
>> So far at most 512 bytes will be transfered, which result
>> in a 684 bytes payload.
>> Since this command is intented for qtesting, this is acceptable.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Doing this kind of thing over QMP doesn't look right to me. qtests
> should access hardware the same way as real guests access the hardware
> (i.e. MMIO and I/O ports).
> 
> But if for some reason the QMP maintainers were to think that this is
> acceptable in QMP, I'd argue it should at least get an x-debug- prefix
> to avoid making it a stable API that management tools may rely on.

Yeah, what we usually do is not test the device (e.g. SCSI) directly,
but only through the HBA (e.g. virtio-scsi or AHCI, it would be SDHCI in
this case).

Thanks,

Paolo

Re: [Qemu-devel] [RFC PATCH 02/26] sdcard: add a Python qtest

[Paolo Bonzini]

On 14/12/2017 00:20, Philippe Mathieu-Daudé wrote:
> Use Python to write high-level SD commands.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/Makefile.include |   2 +
>  tests/sdcard_tests.py  | 172 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 174 insertions(+)
>  create mode 100755 tests/sdcard_tests.py

How hard would it be to rewrite this test as an SDHCI test?

Thanks,

Paolo

> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 13673f6d1d..4a1afcb499 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -355,8 +355,10 @@ check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
>  gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
>  check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
>  gcov-files-arm-y += hw/timer/arm_mptimer.c
> +check-qtest-arm-y += tests/sdcard_tests.py
>  
>  check-qtest-aarch64-y = tests/numa-test$(EXESUF)
> +check-qtest-aarch64-y += tests/sdcard_tests.py
>  
>  check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
>  
> diff --git a/tests/sdcard_tests.py b/tests/sdcard_tests.py
> new file mode 100755
> index 0000000000..033b756cf1
> --- /dev/null
> +++ b/tests/sdcard_tests.py
> @@ -0,0 +1,172 @@
> +#!/usr/bin/env python
> +#
> +# Tests for the SD-Bus protocol
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import os
> +import base64
> +import struct
> +import binascii
> +import logging
> +
> +import qtest
> +
> +
> +# CMD
> +(GO_IDLE_STATE, SEND_OP_CMD, ALL_SEND_CID, SEND_RELATIVE_ADDR, # 0 ...
> +SEND_DSR, CMD5, SWITCH_FUNCTION, CMD7,
> +SEND_IF_COND, SEND_CSD, SEND_CID, READ_DAT_UNTIL_STOP, # 8 ...
> +STOP_TRANSMISSION, SEND_STATUS, CMD14, GO_INACTIVE_STATE,
> +SET_BLOCKLEN, READ_SINGLE_BLOCK, READ_MULTIPLE_BLOCK, CMD19, # 16 ...
> +CMD20, CMD21, CMD22, SET_BLOCK_COUNT,
> +WRITE_SINGLE_BLOCK, WRITE_MULTIPLE_BLOCK, PROGRAM_CID, PROGRAM_CSD,  # 24 ...
> +SET_WRITE_PROT, CLR_WRITE_PROT, SEND_WRITE_PROT, CMD31,
> +ERASE_WR_BLK_START, ERASE_WR_BLK_END, CMD34, CMD35, # 32 ...
> +CMD36, CMD37, ERASE, CMD39,
> +CMD40, CMD41, LOCK_UNLOCK, CMD43,
> +CMD44, CMD45, CMD46, CMD47,
> +CMD48, CMD49, CMD50, CMD51,
> +CMD52, CMD53, CMD54, APP_CMD,
> +GEN_CMD,) = range(57)
> +
> +# ACMD
> +SET_BUS_WIDTH = 6
> +SD_STATUS = 13
> +SEND_NUM_WR_BLOCKS = 22
> +SET_WR_BLK_ERASE_COUNT = 23
> +SD_APP_OP_COND = 41
> +SET_CLR_CARD_DETECT = 42
> +SEND_SCR = 51
> +
> +
> +class SDBus(object):
> +    def __init__(self, vm, qom_path, verbose=False):
> +        self.vm = vm
> +        self.path = qom_path
> +        self.verbose = verbose
> +
> +    def do_cmd(self, command, arg=0, verbose=None):
> +        assert command < 64
> +        if verbose is None:
> +            verbose = self.verbose
> +
> +        data = self.vm.qmp('sdbus-command', qom_path=self.path, command=command,
> +                           arg=arg)['return']
> +        if 'base64' in data:
> +            data = base64.b64decode(data['base64'])
> +            logging.info("CMD#%02d -> %s" % (command, binascii.hexlify(data)))
> +        else:
> +            logging.info("CMD#%02d -> (none)" % (command))
> +            data = None
> +        return data
> +
> +    def do_acmd(self, command, arg=0, verbose=None):
> +        self.do_cmd(APP_CMD, verbose=False)
> +        return self.do_cmd(command, arg, verbose if verbose else self.verbose)
> +
> +
> +def sdbus_get_qom_path(vm, bus=0):
> +        qom_paths = []
> +
> +        result = vm.qmp('query-block')
> +        for block in result['return']:
> +            if not 'qdev' in block:
> +                continue
> +            d = vm.qmp('qom-get', path=block['qdev'], property="parent_bus")
> +            qom_paths += [d['return']]
> +        assert len(qom_paths) > 0
> +
> +        return qom_paths[bus]
> +
> +
> +# dumb helper
> +def l(buf):
> +    return struct.unpack("<L", buf)[0]
> +
> +
> +class TestSdCardSpecV2(qtest.QMPTestCase):
> +    def setUp(self):
> +        self.vm = qtest.createQtestMachine()
> +
> +        self.vm._args.append('-drive')
> +        self.vm._args.append("id=testcard,driver=null-co,if=sd")
> +        self.vm._args.append("-machine")
> +        self.vm._args.append("xilinx-zynq-a9")
> +        self.vm.launch()
> +
> +        bus_path = sdbus_get_qom_path(self.vm)
> +        self.bus = SDBus(self.vm, bus_path, True)
> +
> +    def tearDown(self):
> +        self.vm.shutdown()
> +
> +    def test_power_on_v2(self):
> +
> +        self.bus.do_cmd(GO_IDLE_STATE)
> +
> +        # get voltages
> +        for i in range(6):
> +            vhs = 1 << (8 + i)
> +            data = self.bus.do_cmd(SEND_IF_COND, vhs)
> +            v = l(data)
> +            self.assertNotEqual(v, 0)
> +            self.assertEqual(vhs, v >> 8)
> +
> +        # get OCR
> +        data = self.bus.do_acmd(SD_APP_OP_COND)
> +        v = l(data)
> +        ocr = (v >> 8) & 0xffff
> +        s1_8 = (v >> 24) & 1
> +        uhs_ii = (v >> 29) & 1
> +        ccs = (v >> 30) & 1
> +        init = (v >> 31) & 1
> +        # all those are null for v2.00
> +        self.assertEqual(s1_8, 0)
> +        self.assertEqual(uhs_ii, 0)
> +        self.assertEqual(ccs, 0)
> +        self.assertEqual(init, 0)
> +
> +        # ocr << 8
> +        # 0 << 24 # use current voltage
> +        # 0 << 28 # powersave
> +        # 0 << 30 # SDSC
> +        data = self.bus.do_acmd(SD_APP_OP_COND, ocr << 8)
> +        v = l(data)
> +        # check OCR accepted
> +        self.assertEqual(ocr, v >> 8)
> +
> +        # check CID
> +        data = self.bus.do_cmd(ALL_SEND_CID)
> +        oid, pnm, psn = struct.unpack(">x2s5sxLxxx", data)
> +        self.assertEqual(oid, "XY") # QEMU default
> +        self.assertEqual(pnm, "QEMU!") # QEMU default
> +        self.assertEqual(psn, 0xdeadbeef) # QEMU default
> +
> +        # check non null RCA
> +        data = self.bus.do_cmd(SEND_RELATIVE_ADDR)
> +        rca, = struct.unpack(">H", data[:2])
> +        self.assertNotEqual(rca, 0)
> +
> +        self.assertEqual(rca, 0x4567) # QEMU default
> +
> +        # check for new RCA
> +        data = self.bus.do_cmd(SEND_RELATIVE_ADDR)
> +        new_rca, = struct.unpack(">H", data[:2])
> +        self.assertNotEqual(new_rca, rca)
> +
> +
> +if __name__ == '__main__':
> +    qtest.main(supported_machines=['xilinx-zynq-a9'])
> 

Re: [Qemu-devel] [PATCH 01/26] sdbus: add a QMP command to access a SDBus

[Philippe Mathieu-Daudé]

Hi Kevin,

On 12/14/2017 06:06 AM, Kevin Wolf wrote:
> Am 14.12.2017 um 00:20 hat Philippe Mathieu-Daudé geschrieben:
>> Use Base64 to serialize the binary blobs in JSON.
>> So far at most 512 bytes will be transfered, which result
>> in a 684 bytes payload.
>> Since this command is intented for qtesting, this is acceptable.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Doing this kind of thing over QMP doesn't look right to me. qtests
> should access hardware the same way as real guests access the hardware
> (i.e. MMIO and I/O ports).

Yes, I agree with you, however this command does not implement a guest
access behavior (MMIO and I/O ports) but a _bus_ access.

Guests access buses via MMIO/IOP hardware frontend (bus master), bus
slave devices might be considered as backend, like the BlockBackend.

As the current iotests are meant for backend testing, this command is
meant for SDBus backend testing.

Actually with SDHCI I started to qtest the hardware frontend then
realized the backend was incorrect, so I had to go this way to fix it.
Later series do test the HCI using C qtests.

This approach should works for any buses, and start to be quite
interesting with:
- hot-plug buses to unplug/plug slaves
- multi-master buses like I2C to inject noise on the bus and see if the
host can recover/continue
- testing slave failures like a bricked SPI slave keeping some bus lines
held and checking if the HCI expose this failure to the guest (or the
guest checking the HCI for failures)

> But if for some reason the QMP maintainers were to think that this is
> acceptable in QMP, I'd argue it should at least get an x-debug- prefix
> to avoid making it a stable API that management tools may rely on.

I'd rather have the qtests using this command always run (if they take
too long they might be tagged as 'slow' tests), so I'd keep this stable.

Maybe we can prefix the qtests related QMP commands as "x-qtest-"? Else
your suggestion is fine.

Regards,

Phil.

Re: [Qemu-devel] [PATCH 01/26] sdbus: add a QMP command to access a SDBus

[Philippe Mathieu-Daudé]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

Hi Paolo,

On 12/14/2017 06:34 AM, Paolo Bonzini wrote:
> On 14/12/2017 10:06, Kevin Wolf wrote:
[...]
>> Doing this kind of thing over QMP doesn't look right to me. qtests
>> should access hardware the same way as real guests access the hardware
>> (i.e. MMIO and I/O ports).
[...]
> 
> Yeah, what we usually do is not test the device (e.g. SCSI) directly,
> but only through the HBA (e.g. virtio-scsi or AHCI, it would be SDHCI in
> this case).

Yes, a SDHCI qtest is added in a later series (in C) using MMIO access:
http://lists.nongnu.org/archive/html/qemu-devel/2017-12/msg02391.html

But to be sure the SDHCI is correct I needed a SD slave to behave
correctly ;) Hence this "bus test".

Regards,

Phil.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH 01/26] sdbus: add a QMP command to access a SDBus

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 388 bytes --]

On 14/12/2017 14:25, Philippe Mathieu-Daudé wrote:
> Yes, a SDHCI qtest is added in a later series (in C) using MMIO access:
> http://lists.nongnu.org/archive/html/qemu-devel/2017-12/msg02391.html
> 
> But to be sure the SDHCI is correct I needed a SD slave to behave
> correctly ;) Hence this "bus test".

You could also use the SPI interface for a lower-level test.

Paolo


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 01/26] sdbus: add a QMP command to access a SDBus

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

On 12/14/2017 03:06 AM, Kevin Wolf wrote:
> Am 14.12.2017 um 00:20 hat Philippe Mathieu-Daudé geschrieben:
>> Use Base64 to serialize the binary blobs in JSON.
>> So far at most 512 bytes will be transfered, which result

s/transfered/transferred/

>> in a 684 bytes payload.
>> Since this command is intented for qtesting, this is acceptable.

s/intented/intended/

>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Doing this kind of thing over QMP doesn't look right to me. qtests
> should access hardware the same way as real guests access the hardware
> (i.e. MMIO and I/O ports).
> 
> But if for some reason the QMP maintainers were to think that this is
> acceptable in QMP, I'd argue it should at least get an x-debug- prefix
> to avoid making it a stable API that management tools may rely on.

I'm not convinced the command is needed, but agree that if we want it,
it should have an 'x-debug-' prefix.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH 00/26] SDCard housekeeping

[Alistair Francis]

On Wed, Dec 13, 2017 at 3:19 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Now that we have a way to write qtest in Python, lets start with a simple test
> to perform basic card identification, covering many functions of the sd/sd.c
> file.
>
> [patch 1]
> When a device is not MMIO-connected but rather plugged into a Bus, it is easier
> to write tests using QMP.
> So we add a 'sdbus-command' entry to directly operate on a SD Bus.
> (Note this method should also work with all other QBus).
>
> [patch 2]
> We write a simple pyqtest, which
>   - resolve the SDBus QOM path in the test setup()
>   - verify default values for SD Spec v2.
>
> Since have a test, lets run after each patch, so we are sure we don't break things.
>
> [patch 4-14,20-23]
> Add a bunch of trace events
> [patch 3,11,15-18]
> Aesthetic changes mostly
> [other patches]
> Trivial changes
>
> Once this series applied, booting some Linux on a vexpress-a9 with few
> sd*command traces enabled result in this outputs (which result useful to
> understand/fix the different SD implementations):
>
> 48.687227:sdcard_normal_command         IO_RW_DIRECT/ CMD52 arg 0x00000c00 (state idle)
> 48.687233:sdcard_command_response (no response) (state idle)
> 48.689237:sdcard_normal_command         IO_RW_DIRECT/ CMD52 arg 0x80000c08 (state idle)
> 48.689244:sdcard_command_response (no response) (state idle)
> 48.690877:sdcard_normal_command        GO_IDLE_STATE/ CMD00 arg 0x00000000 (state idle)
> 48.690890:sdcard_command_response (no response) (state idle)
> 48.723486:sdcard_normal_command         SEND_IF_COND/ CMD08 arg 0x000001aa (state idle)
> 48.723507:sdcard_command_response 00 00 01 aa  (state idle)
> 48.723886:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg 0x00000000 (state idle)
> 48.723892:sdcard_command_response (no response) (state idle)
> 48.724019:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg 0x00000000 (state idle)
> 48.724023:sdcard_command_response (no response) (state idle)
> 48.724110:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg 0x00000000 (state idle)
> 48.724115:sdcard_command_response (no response) (state idle)
> 48.724178:sdcard_normal_command      IO_SEND_OP_COND/ CMD05 arg 0x00000000 (state idle)
> 48.724181:sdcard_command_response (no response) (state idle)
> 48.724999:sdcard_command_response 00 40 01 20  (state idle)
> 48.725214:sdcard_app_command         SD_SEND_OP_COND/ACMD41 arg 0x00000000 (state idle)
> 48.725225:sdcard_command_response 40 ff ff 00  (state idle)
> 48.727113:sdcard_normal_command        GO_IDLE_STATE/ CMD00 arg 0x00000000 (state idle)
> 48.727125:sdcard_command_response (no response) (state idle)
> 48.729206:sdcard_normal_command         SEND_IF_COND/ CMD08 arg 0x000001aa (state idle)
> 48.729213:sdcard_command_response 00 00 01 aa  (state idle)
> 48.729339:sdcard_command_response 00 00 01 20  (state idle)
> 48.729412:sdcard_app_command         SD_SEND_OP_COND/ACMD41 arg 0x40300000 (state idle)
> 48.729419:sdcard_command_response c0 ff ff 00  (state ready)
> 48.729786:sdcard_normal_command         ALL_SEND_CID/ CMD02 arg 0x00000000 (state ready)
> 48.729797:sdcard_command_response aa 58 59 51 45 4d 55 21 01 de ad be ef 00 62 19  (state identification)
> 48.730825:sdcard_normal_command   SEND_RELATIVE_ADDR/ CMD03 arg 0x00000000 (state identification)
> 48.730835:sdcard_command_response 45 67 05 00  (state standby)
> 48.731221:sdcard_normal_command             SEND_CSD/ CMD09 arg 0x45670000 (state standby)
> 48.731228:sdcard_command_response 40 0e 00 32 5b 59 00 00 0d 47 7f 80 0a 40 00 00  (state standby)
> 48.732012:sdcard_normal_command SELECT/DESELECT_CARD/ CMD07 arg 0x45670000 (state standby)
> 48.732021:sdcard_command_response 00 00 07 00  (state transfer)
> 48.732419:sdcard_command_response 00 00 09 20  (state transfer)
> 48.764983:sdcard_app_command                SEND_SCR/ACMD51 arg 0x00000000 (state transfer)
> 48.765000:sdcard_command_response 00 00 09 20  (state sendingdata)
> 48.766951:sdcard_command_response 00 00 09 20  (state transfer)
> 48.767159:sdcard_app_command               SD_STATUS/ACMD13 arg 0x00000000 (state transfer)
> 48.767168:sdcard_command_response 00 00 09 20  (state sendingdata)
> 48.966731:sdcard_normal_command  READ_MULTIPLE_BLOCK/ CMD18 arg 0x00000000 (state transfer)
> 48.966763:sdcard_command_response 00 00 09 00  (state sendingdata)
> 48.974869:sdcard_normal_command    STOP_TRANSMISSION/ CMD12 arg 0x00000000 (state sendingdata)
> 48.974877:sdcard_command_response 00 00 0b 00  (state transfer)
> 50.044308:sdcard_normal_command          SEND_STATUS/ CMD13 arg 0x45670000 (state transfer)
> 50.044330:sdcard_command_response 00 00 09 00  (state transfer)
> [...]
>
> Regards,
>
> Phil.
>
> Based-on: 20171213213557.26561-7-f4bug@amsat.org
>           (QTests: use Python to run complex tests)

As this series can't be merged until the Python for Qtest series is
merged I'm not going to go over this now. Let's wait and see how that
discussion goes first.

Alistair

>
> Philippe Mathieu-Daudé (26):
>   sdbus: add a QMP command to access a SDBus
>   sdcard: add Python qtests
>   sdcard: use ldst API
>   sdcard: replace fprintf() -> qemu_log_mask()
>   sdcard: rename sd_set_mode() -> sd_update_mode()
>   sdcard: add sd_set_mode()
>   sdcard: add sdcard_set_mode() trace event
>   sdcard: add sd_set_state()
>   sdcard: add a sdcard_set_state() trace event
>   sdcard: use more detailled state/mode trace events
>   sdcard: use warn_report() instead of fprintf()
>   sdcard: replace DPRINTF() by trace events
>   sdcard: add more trace events
>   sdcard: use qemu_hexbuf_strdup() to trace command response
>   sdcard: use PW_LEN define instead of '16' magic
>   sdcard: let cmd_valid_while_locked() returns a bool
>   sdcard: rename sd_set_REG() functions called by sd_reset() as sd_reset_REG()
>   sdcard: move Memory Card registers together
>   sdcard: add DSR register
>   sdcard: add/use SD_CMD_MAX to check valid SD commands
>   sdcard: add sd_cmd_abbreviation() to resolve the SD command id
>   sdcard: reduce sd_cmd traces
>   sdcard: add ACMD trace events
>   sdcard: use a 16-bit type for the 16-bit RCA register
>   sdcard: add/use a SDCardCommandClass enum instead of magic numbers
>   sdcard: add/use a ccc_spi enum for the commands supported in SPI mode
>
>  qapi-schema.json       |  41 ++++
>  hw/sd/core.c           |  43 ++++
>  hw/sd/sd.c             | 519 +++++++++++++++++++++++++++++++------------------
>  stubs/qmp_sdbus.c      |  11 ++
>  hw/sd/trace-events     |  17 ++
>  stubs/Makefile.objs    |   1 +
>  tests/Makefile.include |   2 +
>  tests/sdcard_tests.py  | 172 ++++++++++++++++
>  8 files changed, 617 insertions(+), 189 deletions(-)
>  create mode 100644 stubs/qmp_sdbus.c
>  create mode 100755 tests/sdcard_tests.py
>
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 00/26] SDCard housekeeping

[Philippe Mathieu-Daudé]

[-- Attachment #1: Type: text/plain, Size: 10225 bytes --]

I'll try to give a status on this series.

First, let me collect few subthreads here to recapitulate:

>> On 14/12/2017 10:06, Kevin Wolf wrote:
> [...]
>> Doing this kind of thing over QMP doesn't look right to me. qtests
>> should access hardware the same way as real guests access the hardware
>> (i.e. MMIO and I/O ports).
>
> Yes, I agree with you, however this command does not implement a guest
> access behavior (MMIO and I/O ports) but a _bus_ access.
>
> Guests access buses via MMIO/IOP hardware frontend (bus master), bus
> slave devices might be considered as backend, like the BlockBackend.
>
> As the current iotests are meant for backend testing, this command is
> meant for SDBus backend testing.
>
> Actually with SDHCI I started to qtest the hardware frontend then
> realized the backend was incorrect, so I had to go this way to fix it.
> Later series do test the HCI using C qtests.
>
> This approach should works for any buses, and start to be quite
> interesting with:
> - hot-plug buses to unplug/plug slaves
> - multi-master buses like I2C to inject noise on the bus and see if the
> host can recover/continue
> - testing slave failures like a bricked SPI slave keeping some bus lines
> held and checking if the HCI expose this failure to the guest (or the
> guest checking the HCI for failures)
> [...]
>> On 12/14/2017 06:34 AM, Paolo Bonzini wrote:
>> Yeah, what we usually do is not test the device (e.g. SCSI) directly,
>> but only through the HBA (e.g. virtio-scsi or AHCI, it would be SDHCI in
>> this case).
>
> Yes, a SDHCI qtest is added in a later series (in C) using MMIO access:
> http://lists.nongnu.org/archive/html/qemu-devel/2017-12/msg02391.html
>
> But to be sure the SDHCI is correct I needed a SD slave to behave
> correctly ;) Hence this "bus test".
On 12/15/2017 09:13 PM, Alistair Francis wrote:
>> Based-on: 20171213213557.26561-7-f4bug@amsat.org
>>           (QTests: use Python to run complex tests)
>
> As this series can't be merged until the Python for Qtest series is
> merged I'm not going to go over this now. Let's wait and see how that
> discussion goes first.

On 12/14/2017 06:34 AM, Paolo Bonzini wrote:
>>  create mode 100755 tests/sdcard_tests.py
>
> How hard would it be to rewrite this test as an SDHCI test?

Sadly, more than two weeks later, I can tell you it was somehow hard.

I previously spent 1 day to write this quick python test, 1 more to
write more tests (different sdcard size and handle SD/MMC/SPI protocols).

Remember this series' goal was to have a correct sdcard model so we
could improve the various SDHCI safely (fix bugs, add support for Spec
v3.01).

Back to C.

Apparently the easiest HCI was the PXA2xx one.
First let find an image and boot it to see how it behaves.
After spending time digging on the Gumstix web, I could find a correct
set of u-boot + uImage + rootfs, and generate the test image.

Let's boot it:

$ qemu-system-arm -M connex -nographic -pflash connex.img \
  -drive if=sd,cache=unsafe,file=sdcard.img
qemu: hardware error: pxa2xx_timer_write: Bad offset 0x00000000000000d8
CPU #0:
R00=40f00030 R01=40a000d8 R02=000000c9 R03=48000004
R04=00000300 R05=00000000 R06=00000000 R07=00000000
R08=00000000 R09=00000000 R10=00000000 R11=00000000
R12=00000868 R13=00000000 R14=0000090c R15=00000f0c
PSR=000001d3 ---- A svc32
FPSCR: 00000000
Aborted

Ok, let's find the PXA255 datasheet, find the timer section, look
registers, implement what's missing, try again later...

$ qemu-system-arm -M connex -nographic -pflash connex.img \
  -drive if=sd,cache=unsafe,file=sdcard.img

U-Boot 1.2.0 (May 10 2008 - 21:17:19) - PXA270@400 MHz - 1604
Hit any key to stop autoboot:  0
## Booting image at a2000000 ...
   Image Name:   Angstrom/2.6.21/gumstix-custom-c
   Image Type:   ARM Linux Kernel Image (uncompressed)
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
Probing Gumstix Flash ROM at physical address 0x00000000 (16-bit bankwidth)
Gumstix Flash ROM: Found 1 x16 devices at 0x0 in 16-bit bank
Creating 3 MTD partitions on "Gumstix Flash ROM":
0x00000000-0x00040000 : "Bootloader"
0x00040000-0x00f00000 : "RootFS"
0x00f00000-0x01000000 : "Kernel"
VFS: Mounted root (jffs2 filesystem).
INIT: version 2.86 booting
qemu-system-arm: Trying to execute code outside RAM or ROM at 0x000618e8
qemu: fatal: Trying to execute code outside RAM or ROM at 0x000618e8
Your guest kernel has a bug and crashed by jumping off into nowhere
Execution cannot continue; stopping here.
R00=00000000 R01=beda1d04 R02=000bd818 R03=000b1d78
R04=000bd838 R05=000bd80c R06=00000001 R07=000bda88
R08=00000000 R09=00000000 R10=401d7000 R11=00000000
R12=00000000 R13=beda1a38 R14=000876b8 R15=000618e8
PSR=60000010 -ZC- A usr32
FPSCR: 00000000

No time to figure out this, but at least U-Boot worked and have shell
accessible:

U-Boot 1.2.0 (May 10 2008 - 21:17:19) - PXA270@400 MHz - 1604
Hit any key to stop autoboot:  0
GUM> mmcinit
5366@1514908231.461749:sdcard_normal_command SD        GO_IDLE_STATE/
CMD00 arg 0x00000000 (state idle)
5366@1514908231.461760:sdcard_reset
5366@1514908231.662147:sdcard_app_command SD
SD_SEND_OP_COND/ACMD41 arg 0x00200000 (state idle)
5366@1514908231.662278:sdcard_normal_command SD         ALL_SEND_CID/
CMD02 arg 0x00000000 (state ready)
5366@1514908231.662419:sdcard_normal_command SD   SEND_RELATIVE_ADDR/
CMD03 arg 0x00000000 (state identification)
5366@1514908231.662515:sdcard_normal_command SD             SEND_CSD/
CMD09 arg 0x45670000 (state standby)
Detected: 32768 blocks of 512 bytes (16MB) SD card.
Vendor: Man aa OEM XY "QEMU!" Date 02/2006
Product: 3735928559
Revision: 0.1
5366@1514908231.664080:sdcard_normal_command SD SELECT/DESELECT_CARD/
CMD07 arg 0x45670000 (state standby)
5366@1514908231.664168:sdcard_app_command SD
SET_BUS_WIDTH/ACMD06 arg 0x00000002 (state transfer)
5366@1514908231.664568:sdcard_normal_command SD         SET_BLOCKLEN/
CMD16 arg 0x00000200 (state transfer)
5366@1514908231.664642:sdcard_normal_command SD    READ_SINGLE_BLOCK/
CMD17 arg 0x00000000 (state transfer)
5366@1514908231.664648:sdcard_read_block addr 0x0 size 0x200
GUM>

Ok this might be useful, however this board is supposed to have the card
plugged in MMC mode which is not yet modelled. We'll can still use it to
test the SD protocol! Good.

Let's write a MMC qtest adapter using this MMIO device.
We have to follow the datashit state machines change, [........]

3 days later: I still didn't succeed to reproduce U-Boot SD commands.
Bad choice, this wasn't probably the easiest HCI.

Let's try with another one.


What about this simple SPI mode modelled in hw/sd/ssi-sd.c?

$ git grep ssi-sd
hw/arm/stellaris.c:1377:  sddev = ssi_create_slave(bus, "ssi-sd");

Cool, let's use the lm3s6965evb machine.
[...] write SPI test [...]

[...] implement the qtest SD SPI adapter [...]
test it!
not working, the bus is never accessed... :(

After a few debugging, it seems we correctly MMIO access the PL022 but
then our sdcard isn't responding, why?

Looking at the monitor:

$ arm-softmmu/qemu-system-arm -M lm3s6965evb -monitor stdio \
  -drive if=sd,cache=unsafe,file=/tmp/ok -accel qtest
(qemu) info qtree
bus: main-system-bus
  type System
  dev: pl022, id ""
    gpio-out "sysbus-irq" 1
    mmio 0000000040008000/0000000000001000
    bus: ssi
      type SSI
      dev: ssd0323, id ""
        gpio-in "" 1
        gpio-in "ssi-gpio-cs" 1
      dev: ssi-sd, id ""
        gpio-in "ssi-gpio-cs" 1

Where is dangling our sdcard?...

Looking at the code, stellaris_init() never use a SD card.

I suppose the code using ssi-sd.c was lost during some refactor, we'll
look later in git history, for now let's add the missing code to
plug/use our card.
[...]

(qemu) info qtree
bus: main-system-bus
  type System
  dev: pl022, id ""
    gpio-out "sysbus-irq" 1
    mmio 0000000040008000/0000000000001000
    bus: ssi
      type SSI
      dev: ssd0323, id ""
        gpio-in "" 1
        gpio-in "ssi-gpio-cs" 1
      dev: ssi-sd, id ""
        gpio-in "ssi-gpio-cs" 1
        bus: sd-bus
          type sd-bus
          dev: sd-card, id ""
            drive = "sd0"
            spi = true

Cool, our card is here! Now run qtests!

Oops, many many failures...
Again, probably nobody used the sdcard code in SPI mode since age.

Remember, I try to have a working card to test the generic SDHCI to
implement the v3...
This SPI mode could have been the simpler way to test the card if it was
working but it isn't, no time for that.

Let's drop our MMC and SPI tests and find some working board using the
SD mode.


Ok we have the nuri board (exynos4210), with a SDHCI v2.
Hmm but wait ... Am I going to test the card with the HCI I want to
test? ...

Ok, clock is ticking, let's forget about using HCI via MMIO and try
again our first idea, access the _bus_ via QMP!
[... lot of fun trying with qobjects, qobject_to_qdict, qlist =
qdict_get(), qobject_to_qstring, qstring_get_str, ...]
4 days. lots of leaks, not enough QDECREF()?
2 days. What am I doing with valgrind? I'm supposed to spend time on the
SDHCI model.
Let's ignore those leaks, and run the test.

Plug a card. We need a Spec v3 working to plug a SDXC card, but smaller
cards are supposed to work.
Let's test the easiest, the smaller card!
Plug a SDSC card: tests fail.... Oh, SDSC support broken for cards
between 1G-2G. Now I remember and understand the warning I read when
preparing the first Connex card:
"The current qemu version seems to be limited to 1GB card images." [1]

Luckily the SDHC cards work better, cool! let's start working!
Oh but wait which day is it...?! The deadline is over, I'm out of
schedule, too late.
No time to write more qtests, I'll just run the first python sample test
and send the series as it :(

Regards,

Phil.

https://wiki.gumstix.com/index.php/Making_qemu_images#mmc.2FCF_images



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]