hw/ide/atapi: read the whole elementary transfer asynchronously
An ATAPI PIO read whose byte-count limit spans more than one CD sector must fetch the later sectors of a DRQ burst from inside the completion of the first, asynchronous read. cd_read_sector_sync() did this with a synchronous blk_pread(), which runs blk_wait_while_drained() before issuing the request. If a drain is in progress when that completion runs -- as happens when a guest reset reaches virtio_blk_stop_ioeventfd() -> bdrv_drain_all_begin() while an ATAPI read is in flight on the same QEMU -- the nested read is queued until the drained section ends while the outer completion still holds blk->in_flight. bdrv_drain_all_begin() then waits forever for that in_flight count to drop: the main loop is wedged in the drain with the BQL held, and every other QMP/monitor operation blocks behind it. Read the whole elementary transfer in a single asynchronous request up front instead, so no read is ever issued in the middle of a burst. cd_read_sector() now reads all the sectors a burst spans (the raw 2352-byte case is unpacked in place on completion) and cd_read_sector_sync() is removed. The DMA path already batched its reads and is unchanged. Signed-off-by: Denis V. Lunev <den@openvz.org>
Denis V. Lunev committed
Jun 19, 2026 at 02:29 UTC
12289dc700d910bca4b79bb8de71acff06097489
1 file changed
+84
-96
hw/ide/atapi.c
+84
-96
@@ -88,46 +88,14 @@ static void cd_data_to_raw(uint8_t *buf, int lba)
88
memset(buf, 0, 288);
89
}
90
91
-static int
92
-cd_read_sector_sync(IDEState *s)
93
-{
94
- int ret;
95
- block_acct_start(blk_get_stats(s->blk), &s->acct,
96
- ATAPI_SECTOR_SIZE, BLOCK_ACCT_READ);
97
-
98
- trace_cd_read_sector_sync(s->lba);
99
-
100
- switch (s->cd_sector_size) {
101
- case 2048:
102
- ret = blk_pread(s->blk, (int64_t)s->lba << ATAPI_SECTOR_BITS,
103
- ATAPI_SECTOR_SIZE, s->io_buffer, 0);
104
- break;
105
- case 2352:
106
- ret = blk_pread(s->blk, (int64_t)s->lba << ATAPI_SECTOR_BITS,
107
- ATAPI_SECTOR_SIZE, s->io_buffer + 16, 0);
108
- if (ret >= 0) {
109
- cd_data_to_raw(s->io_buffer, s->lba);
110
- }
111
- break;
112
- default:
113
- block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_READ);
114
- return -EIO;
115
- }
116
-
117
- if (ret < 0) {
118
- block_acct_failed(blk_get_stats(s->blk), &s->acct);
119
- } else {
120
- block_acct_done(blk_get_stats(s->blk), &s->acct);
121
- s->lba++;
122
- s->io_buffer_index = 0;
123
- }
124
-
125
- return ret;
126
-}
127
-
91
static void cd_read_sector_cb(void *opaque, int ret)
92
{
93
IDEState *s = opaque;
94
+ int et = s->elementary_transfer_size;
95
+ int skip = s->io_buffer_index;
96
+ int nsec = DIV_ROUND_UP(skip + et, s->cd_sector_size);
97
+ uint8_t *buf;
98
+ int i;
99
100
trace_cd_read_sector_cb(s->lba, ret);
101
@@ -140,34 +108,64 @@ static void cd_read_sector_cb(void *opaque, int ret)
108
block_acct_done(blk_get_stats(s->blk), &s->acct);
109
110
if (s->cd_sector_size == 2352) {
143
- cd_data_to_raw(s->io_buffer, s->lba);
111
+ /* unpack back-to-front so a sector never clobbers an unmoved one */
112
+ for (i = nsec - 1; i >= 0; i--) {
113
+ memmove(s->io_buffer + i * 2352 + 16, s->io_buffer + i * 2048,
114
+ ATAPI_SECTOR_SIZE);
115
+ cd_data_to_raw(s->io_buffer + i * 2352, s->lba + i);
116
+ }
117
}
118
146
- s->lba++;
147
- s->io_buffer_index = 0;
119
s->status &= ~BUSY_STAT;
120
150
- ide_atapi_cmd_reply_end(s);
121
+ s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO;
122
+ s->lcyl = et & 0xff;
123
+ s->hcyl = (et >> 8) & 0xff;
124
+ ide_bus_set_irq(s->bus);
125
+
126
+ /* a boundary sector shared with the next burst is re-read there */
127
+ buf = s->io_buffer + skip;
128
+ s->packet_transfer_size -= et;
129
+ s->lba += (skip + et) / s->cd_sector_size;
130
+ s->io_buffer_index = (skip + et) % s->cd_sector_size;
131
+ s->elementary_transfer_size = 0;
132
+
133
+ if (ide_transfer_start_norecurse(s, buf, et, ide_atapi_cmd_reply_end)) {
134
+ ide_atapi_cmd_reply_end(s);
135
+ }
136
}
137
138
+/*
139
+ * Read the whole elementary transfer (one DRQ burst) in a single async
140
+ * request. No read is issued mid-burst, so unlike the old synchronous
141
+ * rebuffer it cannot deadlock against a concurrent drain.
142
+ */
143
static int cd_read_sector(IDEState *s)
144
{
155
- void *buf;
145
+ int et = s->elementary_transfer_size;
146
+ int skip = s->io_buffer_index;
147
+ int nsec = DIV_ROUND_UP(skip + et, s->cd_sector_size);
148
149
if (s->cd_sector_size != 2048 && s->cd_sector_size != 2352) {
150
block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_READ);
151
return -EINVAL;
152
}
153
162
- buf = (s->cd_sector_size == 2352) ? s->io_buffer + 16 : s->io_buffer;
163
- qemu_iovec_init_buf(&s->qiov, buf, ATAPI_SECTOR_SIZE);
154
+ /* a burst is bounded by the byte count limit, so it fits io_buffer */
155
+ assert(nsec * s->cd_sector_size <= s->io_buffer_total_len);
156
+
157
+ /*
158
+ * Read the payload packed at the front of io_buffer; the 2352 raw case is
159
+ * unpacked into place on completion.
160
+ */
161
+ qemu_iovec_init_buf(&s->qiov, s->io_buffer, nsec * ATAPI_SECTOR_SIZE);
162
163
trace_cd_read_sector(s->lba);
164
165
block_acct_start(blk_get_stats(s->blk), &s->acct,
168
- ATAPI_SECTOR_SIZE, BLOCK_ACCT_READ);
166
+ nsec * ATAPI_SECTOR_SIZE, BLOCK_ACCT_READ);
167
170
- ide_buffered_readv(s, (int64_t)s->lba << 2, &s->qiov, 4,
168
+ ide_buffered_readv(s, (int64_t)s->lba << 2, &s->qiov, nsec * 4,
169
cd_read_sector_cb, s);
170
171
s->status |= BUSY_STAT;
@@ -222,59 +220,49 @@ static uint16_t atapi_byte_count_limit(IDEState *s)
220
void ide_atapi_cmd_reply_end(IDEState *s)
221
{
222
int byte_count_limit, size, ret;
225
- while (s->packet_transfer_size > 0) {
226
- trace_ide_atapi_cmd_reply_end(s, s->packet_transfer_size,
227
- s->elementary_transfer_size,
228
- s->io_buffer_index);
229
-
230
- /* see if a new sector must be read */
231
- if (s->lba != -1 && s->io_buffer_index >= s->cd_sector_size) {
232
- if (!s->elementary_transfer_size) {
233
- ret = cd_read_sector(s);
234
- if (ret < 0) {
235
- ide_atapi_io_error(s, ret);
236
- }
237
- return;
238
- } else {
239
- /* rebuffering within an elementary transfer is
240
- * only possible with a sync request because we
241
- * end up with a race condition otherwise */
242
- ret = cd_read_sector_sync(s);
243
- if (ret < 0) {
244
- ide_atapi_io_error(s, ret);
245
- return;
246
- }
223
+
224
+ trace_ide_atapi_cmd_reply_end(s, s->packet_transfer_size,
225
+ s->elementary_transfer_size,
226
+ s->io_buffer_index);
227
+
228
+ if (s->lba != -1 && s->packet_transfer_size > 0) {
229
+ byte_count_limit = atapi_byte_count_limit(s);
230
+ trace_ide_atapi_cmd_reply_end_bcl(s, byte_count_limit);
231
+ size = s->packet_transfer_size;
232
+ if (size > byte_count_limit) {
233
+ /* byte count limit must be even if this case */
234
+ if (byte_count_limit & 1) {
235
+ byte_count_limit--;
236
}
237
+ size = byte_count_limit;
238
}
249
- if (s->elementary_transfer_size > 0) {
250
- /* there are some data left to transmit in this elementary
251
- transfer */
252
- size = s->cd_sector_size - s->io_buffer_index;
253
- if (size > s->elementary_transfer_size)
254
- size = s->elementary_transfer_size;
255
- } else {
256
- /* a new transfer is needed */
257
- s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO;
258
- ide_bus_set_irq(s->bus);
259
- byte_count_limit = atapi_byte_count_limit(s);
260
- trace_ide_atapi_cmd_reply_end_bcl(s, byte_count_limit);
261
- size = s->packet_transfer_size;
262
- if (size > byte_count_limit) {
263
- /* byte count limit must be even if this case */
264
- if (byte_count_limit & 1)
265
- byte_count_limit--;
266
- size = byte_count_limit;
267
- }
268
- s->lcyl = size & 0xff;
269
- s->hcyl = size >> 8;
270
- s->elementary_transfer_size = size;
271
- /* we cannot transmit more than one sector at a time */
272
- if (s->lba != -1) {
273
- if (size > (s->cd_sector_size - s->io_buffer_index))
274
- size = (s->cd_sector_size - s->io_buffer_index);
239
+ s->elementary_transfer_size = size;
240
+ ret = cd_read_sector(s);
241
+ if (ret < 0) {
242
+ ide_atapi_io_error(s, ret);
243
+ }
244
+ return;
245
+ }
246
+
247
+ while (s->packet_transfer_size > 0) {
248
+ /* a new transfer is needed */
249
+ s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO;
250
+ ide_bus_set_irq(s->bus);
251
+ byte_count_limit = atapi_byte_count_limit(s);
252
+ trace_ide_atapi_cmd_reply_end_bcl(s, byte_count_limit);
253
+ size = s->packet_transfer_size;
254
+ if (size > byte_count_limit) {
255
+ /* byte count limit must be even if this case */
256
+ if (byte_count_limit & 1) {
257
+ byte_count_limit--;
258
}
276
- trace_ide_atapi_cmd_reply_end_new(s, s->status);
259
+ size = byte_count_limit;
260
}
261
+ s->lcyl = size & 0xff;
262
+ s->hcyl = size >> 8;
263
+ s->elementary_transfer_size = size;
264
+ trace_ide_atapi_cmd_reply_end_new(s, s->status);
265
+
266
s->packet_transfer_size -= size;
267
s->elementary_transfer_size -= size;
268
s->io_buffer_index += size;
@@ -329,7 +317,7 @@ static void ide_atapi_cmd_read_pio(IDEState *s, int lba, int nb_sectors,
317
s->lba = lba;
318
s->packet_transfer_size = nb_sectors * sector_size;
319
s->elementary_transfer_size = 0;
332
- s->io_buffer_index = sector_size;
320
+ s->io_buffer_index = 0;
321
s->cd_sector_size = sector_size;
322
323
ide_atapi_cmd_reply_end(s);