Skip to content

Commit 570aed2

Browse files
Cuda-ChenCuda-Chen
committed
Set BAD_MSG if device receives invalid stream_id
Get re-sending frames effect with the following errors: [ 35.875955] virtio_snd virtio3: virtsnd-tx:id 0 is not a head! [ 36.892059] virtio_snd virtio3: SID 0: failed to flush I/O queue
1 parent 88ce9a1 commit 570aed2

File tree

1 file changed

+110
-103
lines changed

1 file changed

+110
-103
lines changed

‎virtio-snd.c

+110-103
Original file line numberDiff line numberDiff line change
@@ -307,11 +307,12 @@ typedef struct {
307307
static virtio_snd_config_t vsnd_configs[VSND_DEV_CNT_MAX];
308308
static virtio_snd_prop_t vsnd_props[VSND_DEV_CNT_MAX] = {
309309
[0 ... VSND_DEV_CNT_MAX - 1].pp.hdr.hdr.code = VIRTIO_SND_R_PCM_SET_PARAMS,
310-
[0 ... VSND_DEV_CNT_MAX - 1].lock = {
311-
.lock = PTHREAD_MUTEX_INITIALIZER,
312-
.readable = PTHREAD_COND_INITIALIZER,
313-
.writable = PTHREAD_COND_INITIALIZER,
314-
},
310+
[0 ... VSND_DEV_CNT_MAX - 1].lock =
311+
{
312+
.lock = PTHREAD_MUTEX_INITIALIZER,
313+
.readable = PTHREAD_COND_INITIALIZER,
314+
.writable = PTHREAD_COND_INITIALIZER,
315+
},
315316
};
316317
static int vsnd_dev_cnt = 0;
317318

@@ -343,102 +344,109 @@ typedef struct {
343344
struct list_head q;
344345
} virtq_desc_queue_node_t;
345346

346-
#define VSND_GEN_TX_QUEUE_HANDLER(NAME_SUFFIX, WRITE) \
347-
static int virtio_snd_tx_desc_##NAME_SUFFIX##_handler( \
348-
virtio_snd_state_t *vsnd, const virtio_snd_queue_t *queue, \
349-
uint32_t desc_idx, uint32_t *plen) \
350-
{ \
351-
/* A PCM I/O message uses at least 3 virtqueue descriptors to \
352-
* represent a PCM data of a period size. \
353-
* The first part contains one descriptor as follows: \
354-
* struct virtio_snd_pcm_xfer \
355-
* The second part contains one or more descriptors \
356-
* representing PCM frames. \
357-
* the last part contains one descriptor as follows: \
358-
* struct virtio_snd_pcm_status \
359-
*/ \
360-
virtq_desc_queue_node_t *node; \
361-
struct list_head q; \
362-
INIT_LIST_HEAD(&q); \
363-
\
364-
/* Collect the descriptors */ \
365-
int cnt = 0; \
366-
for (;;) { \
367-
/* The size of the `struct virtq_desc` is 4 words */ \
368-
const uint32_t *desc = \
369-
&vsnd->ram[queue->QueueDesc + desc_idx * 4]; \
370-
\
371-
/* Retrieve the fields of current descriptor */ \
372-
node = (virtq_desc_queue_node_t *) malloc(sizeof(*node)); \
373-
node->vq_desc.addr = desc[0]; \
374-
node->vq_desc.len = desc[2]; \
375-
node->vq_desc.flags = desc[3]; \
376-
list_push(&node->q, &q); \
377-
desc_idx = desc[3] >> 16; /* vq_desc[desc_cnt].next */ \
378-
\
379-
cnt++; \
380-
\
381-
/* Leave the loop if next-flag is not set */ \
382-
if (!(desc[3] & VIRTIO_DESC_F_NEXT)) \
383-
break; \
384-
} \
385-
\
386-
int idx = 0; \
387-
uint32_t stream_id = 0; /* Explicitly set the stream_id */ \
388-
uintptr_t base = (uintptr_t) vsnd->ram; \
389-
uint32_t ret_len = 0; \
390-
list_for_each_entry (node, &q, q) { \
391-
uint32_t addr = node->vq_desc.addr; \
392-
uint32_t len = node->vq_desc.len; \
393-
if (idx == 0) { /* the first descriptor */ \
394-
const virtio_snd_pcm_xfer_t *request = \
395-
(virtio_snd_pcm_xfer_t *) (base + addr); \
396-
stream_id = request->stream_id; \
397-
if(stream_id >= VSND_DEV_CNT_MAX) { \
398-
fprintf(stderr, "invalide stream_id %" PRIu32 "\n", stream_id);\
399-
goto err; \
400-
} \
401-
goto early_continue; \
402-
} else if (idx == cnt - 1) { /* the last descriptor */ \
403-
virtio_snd_pcm_status_t *response = \
404-
(virtio_snd_pcm_status_t *) (base + addr); \
405-
response->status = VIRTIO_SND_S_OK; \
406-
response->latency_bytes = ret_len; \
407-
*plen = sizeof(*response); \
408-
goto early_continue; \
409-
} \
410-
\
411-
IIF(WRITE) \
412-
(/* enqueue frames */ \
413-
void *payload = (void *) (base + addr); \
414-
fprintf(stderr, "=== hit " #NAME_SUFFIX "%" PRIu32 "---\n", stream_id); \
415-
__virtio_snd_frame_enqueue(payload, len, stream_id); \
416-
, /* flush queue */ \
417-
(void) stream_id; \
418-
/* Suppress unused variable warning. */) ret_len += len; \
419-
\
420-
early_continue: \
421-
idx++; \
422-
} \
423-
\
424-
IIF(WRITE) \
425-
(/* enque frames */ \
426-
virtio_snd_prop_t *props = &vsnd_props[stream_id]; \
427-
props->lock.buf_ev_notity++; \
428-
pthread_cond_signal(&props->lock.readable);, /* flush queue */ \
429-
) \
430-
\
431-
/* Tear down the descriptor list and free space. */ \
432-
virtq_desc_queue_node_t *tmp = NULL; \
433-
list_for_each_entry_safe (node, tmp, &q, q) { \
434-
list_del(&node->q); \
435-
free(node); \
436-
} \
437-
\
438-
return 0; \
439-
\
440-
err: \
441-
return -1;\
347+
#define VSND_GEN_TX_QUEUE_HANDLER(NAME_SUFFIX, WRITE) \
348+
static int virtio_snd_tx_desc_##NAME_SUFFIX##_handler( \
349+
virtio_snd_state_t *vsnd, const virtio_snd_queue_t *queue, \
350+
uint32_t desc_idx, uint32_t *plen) \
351+
{ \
352+
/* A PCM I/O message uses at least 3 virtqueue descriptors to \
353+
* represent a PCM data of a period size. \
354+
* The first part contains one descriptor as follows: \
355+
* struct virtio_snd_pcm_xfer \
356+
* The second part contains one or more descriptors \
357+
* representing PCM frames. \
358+
* the last part contains one descriptor as follows: \
359+
* struct virtio_snd_pcm_status \
360+
*/ \
361+
virtq_desc_queue_node_t *node; \
362+
struct list_head q; \
363+
INIT_LIST_HEAD(&q); \
364+
\
365+
/* Collect the descriptors */ \
366+
int cnt = 0; \
367+
for (;;) { \
368+
/* The size of the `struct virtq_desc` is 4 words */ \
369+
const uint32_t *desc = \
370+
&vsnd->ram[queue->QueueDesc + desc_idx * 4]; \
371+
\
372+
/* Retrieve the fields of current descriptor */ \
373+
node = (virtq_desc_queue_node_t *) malloc(sizeof(*node)); \
374+
node->vq_desc.addr = desc[0]; \
375+
node->vq_desc.len = desc[2]; \
376+
node->vq_desc.flags = desc[3]; \
377+
list_push(&node->q, &q); \
378+
desc_idx = desc[3] >> 16; /* vq_desc[desc_cnt].next */ \
379+
\
380+
cnt++; \
381+
\
382+
/* Leave the loop if next-flag is not set */ \
383+
if (!(desc[3] & VIRTIO_DESC_F_NEXT)) \
384+
break; \
385+
} \
386+
\
387+
int idx = 0; \
388+
uint32_t stream_id = 0; /* Explicitly set the stream_id */ \
389+
uintptr_t base = (uintptr_t) vsnd->ram; \
390+
uint32_t ret_len = 0; \
391+
uint8_t bad_msg_err = 0; \
392+
list_for_each_entry (node, &q, q) { \
393+
uint32_t addr = node->vq_desc.addr; \
394+
uint32_t len = node->vq_desc.len; \
395+
if (idx == 0) { /* the first descriptor */ \
396+
const virtio_snd_pcm_xfer_t *request = \
397+
(virtio_snd_pcm_xfer_t *) (base + addr); \
398+
stream_id = request->stream_id; \
399+
if (stream_id >= VSND_DEV_CNT_MAX) { \
400+
fprintf(stderr, "invalid stream_id %" PRIu32 "\n", \
401+
stream_id); \
402+
bad_msg_err = 1; \
403+
} \
404+
goto early_continue; \
405+
} else if (idx == cnt - 1) { /* the last descriptor */ \
406+
virtio_snd_pcm_status_t *response = \
407+
(virtio_snd_pcm_status_t *) (base + addr); \
408+
response->status = \
409+
bad_msg_err ? VIRTIO_SND_S_BAD_MSG : VIRTIO_SND_S_OK; \
410+
response->latency_bytes = ret_len; \
411+
*plen = sizeof(*response); \
412+
goto early_continue; \
413+
} \
414+
\
415+
IIF(WRITE) \
416+
(/* enqueue frames */ \
417+
void *payload = (void *) (base + addr); \
418+
fprintf(stderr, \
419+
"=== hit " #NAME_SUFFIX " %" PRIu32 \
420+
" bas_msg_err %" PRIu8 "---\n", \
421+
stream_id, bad_msg_err); \
422+
if (bad_msg_err == 0) \
423+
__virtio_snd_frame_enqueue(payload, len, stream_id); \
424+
, /* flush queue */ \
425+
(void) stream_id; \
426+
/* Suppress unused variable warning. */) ret_len += len; \
427+
\
428+
early_continue: \
429+
idx++; \
430+
} \
431+
\
432+
if (bad_msg_err != 0) \
433+
goto finally; \
434+
IIF(WRITE) \
435+
(/* enque frames */ \
436+
virtio_snd_prop_t *props = &vsnd_props[stream_id]; \
437+
props->lock.buf_ev_notity++; \
438+
pthread_cond_signal(&props->lock.readable);, /* flush queue */ \
439+
) \
440+
\
441+
/* Tear down the descriptor list and free space. */ \
442+
virtq_desc_queue_node_t *tmp = NULL; \
443+
list_for_each_entry_safe (node, tmp, &q, q) { \
444+
list_del(&node->q); \
445+
free(node); \
446+
} \
447+
\
448+
finally: \
449+
return 0; \
442450
}
443451

444452
VSND_GEN_TX_QUEUE_HANDLER(normal, 1);
@@ -651,7 +659,6 @@ static void virtio_snd_read_pcm_prepare(const virtio_snd_pcm_hdr_t *query,
651659
return;
652660
}
653661

654-
finally:
655662
*plen = 0;
656663
}
657664

@@ -743,7 +750,7 @@ static void virtio_snd_read_pcm_release(const virtio_snd_pcm_hdr_t *query,
743750
free(node);
744751
}
745752
}
746-
753+
747754
/* avoid dangling pointers */
748755
props->intermediate = NULL;
749756

0 commit comments

Comments
 (0)