@samitouri / QOSamiQemu / commits / 468fa450a7

hw/sd: Switch read/write primitive to buf+len

Currently, read/writes are broken down into individual bytes which result in many function calls. This is quite bad for performance and since both the layer below and above work with larger buffers, it should be corrected. This patch is the first that switches the corresponding interface over to use a buf+len instead of a single byte. However, for most cases the implementation still only reads one byte and is then called again with the remaining buffer. Optimizations taking advantage of this new interface are to follow in the next commits. Signed-off-by: Christian Speich <c.speich@avm.de> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-ID: <20260417-sdcard-performance-b4-v4-1-119e66be10c2@avm.de> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Christian Speich committed Apr 17, 2026 at 11:51 UTC 468fa450a7e0ccc3dacc308e771b09a726cc7f37
3 files changed +71 -39
hw/sd/core.c
+16 -10
@@ -113,21 +113,24 @@ void sdbus_write_byte(SDBus *sdbus, uint8_t value)
113 if (card) {
114 SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
115
116 - sc->write_byte(card, value);
116 + sc->write_data(card, &value, 1);
117 }
118 }
119
120 void sdbus_write_data(SDBus *sdbus, const void *buf, size_t length)
121 {
122 SDState *card = get_card(sdbus);
123 - const uint8_t *data = buf;
123
124 if (card) {
125 SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
126
128 - for (size_t i = 0; i < length; i++) {
129 - trace_sdbus_write(sdbus_name(sdbus), data[i]);
130 - sc->write_byte(card, data[i]);
127 + while (length > 0) {
128 + size_t written = sc->write_data(card, buf, length);
129 +
130 + g_assert(written >= 1);
131 +
132 + buf += written;
133 + length -= written;
134 }
135 }
136 }
@@ -140,7 +143,7 @@ uint8_t sdbus_read_byte(SDBus *sdbus)
143 if (card) {
144 SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
145
143 - value = sc->read_byte(card);
146 + sc->read_data(card, &value, 1);
147 }
148 trace_sdbus_read(sdbus_name(sdbus), value);
149
@@ -150,14 +153,17 @@ uint8_t sdbus_read_byte(SDBus *sdbus)
153 void sdbus_read_data(SDBus *sdbus, void *buf, size_t length)
154 {
155 SDState *card = get_card(sdbus);
153 - uint8_t *data = buf;
156
157 if (card) {
158 SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
159
158 - for (size_t i = 0; i < length; i++) {
159 - data[i] = sc->read_byte(card);
160 - trace_sdbus_read(sdbus_name(sdbus), data[i]);
160 + while (length > 0) {
161 + size_t read = sc->read_data(card, buf, length);
162 +
163 + g_assert(read >= 1);
164 +
165 + buf += read;
166 + length -= read;
167 }
168 }
169 }
hw/sd/sd.c
+40 -22
@@ -2639,30 +2639,37 @@ static bool sd_generic_read_byte(SDState *sd, uint8_t *value)
2639 return false;
2640 }
2641
2642 -static void sd_write_byte(SDState *sd, uint8_t value)
2642 +static size_t sd_write_data(SDState *sd, const void *buf, size_t length)
2643 {
2644 unsigned int partition_access;
2645 int i;
2646 + const uint8_t *value = buf;
2647
2648 if (!sd->blk || !blk_is_inserted(sd->blk)) {
2648 - return;
2649 + return length;
2650 }
2651
2652 if (sd->state != sd_receivingdata_state) {
2653 qemu_log_mask(LOG_GUEST_ERROR,
2654 "%s: not in Receiving-Data state\n", __func__);
2654 - return;
2655 + return length;
2656 }
2657
2658 if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION))
2658 - return;
2659 + return length;
2660 +
2661 + /*
2662 + * Only read one byte at a time. We will be called again with the
2663 + * remaining.
2664 + */
2665 + length = 1;
2666
2667 trace_sdcard_write_data(sd->proto->name,
2668 sd->last_cmd_name,
2662 - sd->current_cmd, sd->data_offset, value);
2669 + sd->current_cmd, sd->data_offset, value[0]);
2670 switch (sd->current_cmd) {
2671 case 24: /* CMD24: WRITE_SINGLE_BLOCK */
2665 - if (sd_generic_write_byte(sd, value)) {
2672 + if (sd_generic_write_byte(sd, value[0])) {
2673 /* TODO: Check CRC before committing */
2674 sd->state = sd_programming_state;
2675 sd_blk_write(sd, sd->data_start, sd->data_offset);
@@ -2687,7 +2694,7 @@ static void sd_write_byte(SDState *sd, uint8_t value)
2694 }
2695 }
2696 }
2690 - sd->data[sd->data_offset++] = value;
2697 + sd->data[sd->data_offset++] = value[0];
2698 if (sd->data_offset >= sd->blk_len) {
2699 /* TODO: Check CRC before committing */
2700 sd->state = sd_programming_state;
@@ -2717,7 +2724,7 @@ static void sd_write_byte(SDState *sd, uint8_t value)
2724 break;
2725
2726 case 26: /* CMD26: PROGRAM_CID */
2720 - if (sd_generic_write_byte(sd, value)) {
2727 + if (sd_generic_write_byte(sd, value[0])) {
2728 /* TODO: Check CRC before committing */
2729 sd->state = sd_programming_state;
2730 for (i = 0; i < sizeof(sd->cid); i ++)
@@ -2735,7 +2742,7 @@ static void sd_write_byte(SDState *sd, uint8_t value)
2742 break;
2743
2744 case 27: /* CMD27: PROGRAM_CSD */
2738 - if (sd_generic_write_byte(sd, value)) {
2745 + if (sd_generic_write_byte(sd, value[0])) {
2746 /* TODO: Check CRC before committing */
2747 sd->state = sd_programming_state;
2748 for (i = 0; i < sizeof(sd->csd); i ++)
@@ -2758,7 +2765,7 @@ static void sd_write_byte(SDState *sd, uint8_t value)
2765 break;
2766
2767 case 42: /* CMD42: LOCK_UNLOCK */
2761 - if (sd_generic_write_byte(sd, value)) {
2768 + if (sd_generic_write_byte(sd, value[0])) {
2769 /* TODO: Check CRC before committing */
2770 sd->state = sd_programming_state;
2771 sd_lock_command(sd);
@@ -2768,36 +2775,47 @@ static void sd_write_byte(SDState *sd, uint8_t value)
2775 break;
2776
2777 case 56: /* CMD56: GEN_CMD */
2771 - sd_generic_write_byte(sd, value);
2778 + sd_generic_write_byte(sd, value[0]);
2779 break;
2780
2781 default:
2782 g_assert_not_reached();
2783 }
2784 +
2785 + return length;
2786 }
2787
2779 -static uint8_t sd_read_byte(SDState *sd)
2788 +static size_t sd_read_data(SDState *sd, void *buf, size_t length)
2789 {
2790 /* TODO: Append CRCs */
2791 const uint8_t dummy_byte = 0x00;
2792 unsigned int partition_access;
2784 - uint8_t ret;
2793 uint32_t io_len;
2794 + uint8_t *value = buf;
2795
2796 if (!sd->blk || !blk_is_inserted(sd->blk)) {
2788 - return dummy_byte;
2797 + memset(buf, dummy_byte, length);
2798 + return length;
2799 }
2800
2801 if (sd->state != sd_sendingdata_state) {
2802 qemu_log_mask(LOG_GUEST_ERROR,
2803 "%s: not in Sending-Data state\n", __func__);
2794 - return dummy_byte;
2804 + memset(buf, dummy_byte, length);
2805 + return length;
2806 }
2807
2808 if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION)) {
2798 - return dummy_byte;
2809 + memset(buf, dummy_byte, length);
2810 + return length;
2811 }
2812
2813 + /*
2814 + * We will only read one byte at a time. We will be called again with the
2815 + * remaining buffer.
2816 + */
2817 + length = 1;
2818 +
2819 io_len = sd_blk_len(sd);
2820
2821 trace_sdcard_read_data(sd->proto->name,
@@ -2815,7 +2833,7 @@ static uint8_t sd_read_byte(SDState *sd)
2833 case 30: /* CMD30: SEND_WRITE_PROT */
2834 case 51: /* ACMD51: SEND_SCR */
2835 case 56: /* CMD56: GEN_CMD */
2818 - sd_generic_read_byte(sd, &ret);
2836 + sd_generic_read_byte(sd, value);
2837 break;
2838
2839 case 18: /* CMD18: READ_MULTIPLE_BLOCK */
@@ -2832,7 +2850,7 @@ static uint8_t sd_read_byte(SDState *sd)
2850 sd_blk_read(sd, sd->data_start, io_len);
2851 }
2852 }
2835 - ret = sd->data[sd->data_offset ++];
2853 + *value = sd->data[sd->data_offset++];
2854
2855 if (sd->data_offset >= io_len) {
2856 sd->data_start += io_len;
@@ -2851,10 +2869,10 @@ static uint8_t sd_read_byte(SDState *sd)
2869 default:
2870 qemu_log_mask(LOG_GUEST_ERROR, "%s: DAT read illegal for command %s\n",
2871 __func__, sd->last_cmd_name);
2854 - return dummy_byte;
2872 + *value = dummy_byte;
2873 }
2874
2857 - return ret;
2875 + return length;
2876 }
2877
2878 static bool sd_receive_ready(SDState *sd)
@@ -3196,8 +3214,8 @@ static void sdmmc_common_class_init(ObjectClass *klass, const void *data)
3214 sc->get_dat_lines = sd_get_dat_lines;
3215 sc->get_cmd_line = sd_get_cmd_line;
3216 sc->do_command = sd_do_command;
3199 - sc->write_byte = sd_write_byte;
3200 - sc->read_byte = sd_read_byte;
3217 + sc->write_data = sd_write_data;
3218 + sc->read_data = sd_read_data;
3219 sc->receive_ready = sd_receive_ready;
3220 sc->data_ready = sd_data_ready;
3221 sc->get_inserted = sd_get_inserted;
include/hw/sd/sd.h
+15 -7
@@ -107,22 +107,30 @@ struct SDCardClass {
107 size_t (*do_command)(SDState *sd, SDRequest *req,
108 uint8_t *resp, size_t respsz);
109 /**
110 - * Write a byte to a SD card.
110 + * Write data to a SD card.
111 * @sd: card
112 - * @value: byte to write
112 + * @value: data to write
113 + * @len: length of data
114 *
114 - * Write a byte on the data lines of a SD card.
115 + * Write data on the data lines of a SD card. May write not all data, in
116 + * which case it should be called again. At least one byte must be consumed.
117 + *
118 + * Return: number of bytes actually written. >= 1
119 */
116 - void (*write_byte)(SDState *sd, uint8_t value);
120 + size_t (*write_data)(SDState *sd, const void *buf, size_t len);
121 /**
122 * Read a byte from a SD card.
123 * @sd: card
124 + * @buf: buffer to receive the data
125 + * @len: size of data to read
126 *
121 - * Read a byte from the data lines of a SD card.
127 + * Read data from the data lines of a SD card. This may not read all
128 + * requested data, in this case it should be called again with the remaining
129 + * buffer. At least one byte must be read.
130 *
123 - * Return: byte value read
131 + * Return: number of bytes actually read. >= 1
132 */
125 - uint8_t (*read_byte)(SDState *sd);
133 + size_t (*read_data)(SDState *sd, void* buf, size_t len);
134 bool (*receive_ready)(SDState *sd);
135 bool (*data_ready)(SDState *sd);
136 void (*set_voltage)(SDState *sd, uint16_t millivolts);