Currently, PL011 is used by ARM virt board by default. It's possible to block the system from booting. With below parameters in command line, the backend could run into endless attempts of transmitting packets, which can't succeed because of running out of sending buffer. The socket might be not accepted n server side. It's not correct because disconnected serial port shouldn't stop the system from booting.
-machine virt,gic-version=3 -cpu max -m 4096 -monitor none -serial tcp:127.0.0.1:50900 The issue can be reproduced by starting a program which listens on TCP port 50900 and then sleep without accepting any incoming connections. On the other hand, a VM is started with above parameters and modified qemu where the PL011 is flooded with 5000K data after it's created. Eventually, the flooding won't proceed and stops after transmitting 2574K data. It's basically to simulate tons of output from EDK-II and demonstrates how the tons of output can block the system from booting. This fixes the issue by using newly added API qemu_chr_fe_try_write_all(), which provides another type of service (best-effort). It's different from qemu_chr_fe_write_all() as the data will be dropped if the backend has been running into so-called broken state or 50 attempts of transmissions. The broken state is cleared if the data is transmitted at once. Signed-off-by: Gavin Shan <gs...@redhat.com> --- chardev/char-fe.c | 15 +++++++++++++-- chardev/char.c | 20 ++++++++++++++------ hw/char/pl011.c | 5 +---- include/chardev/char-fe.h | 14 ++++++++++++++ include/chardev/char.h | 6 ++++-- 5 files changed, 46 insertions(+), 14 deletions(-) diff --git a/chardev/char-fe.c b/chardev/char-fe.c index f3530a90e6..6558fcfb94 100644 --- a/chardev/char-fe.c +++ b/chardev/char-fe.c @@ -39,7 +39,7 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len) return 0; } - return qemu_chr_write(s, buf, len, false); + return qemu_chr_write(s, buf, len, false, false); } int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len) @@ -50,7 +50,18 @@ int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len) return 0; } - return qemu_chr_write(s, buf, len, true); + return qemu_chr_write(s, buf, len, true, false); +} + +int qemu_chr_fe_try_write_all(CharBackend *be, const uint8_t *buf, int len) +{ + Chardev *s = be->chr; + + if (!s) { + return 0; + } + + return qemu_chr_write(s, buf, len, true, true); } int qemu_chr_fe_read_all(CharBackend *be, uint8_t *buf, int len) diff --git a/chardev/char.c b/chardev/char.c index 87237568df..cd17fac123 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -106,9 +106,8 @@ static void qemu_chr_write_log(Chardev *s, const uint8_t *buf, size_t len) } } -static int qemu_chr_write_buffer(Chardev *s, - const uint8_t *buf, int len, - int *offset, bool write_all) +static int qemu_chr_write_buffer(Chardev *s, const uint8_t *buf, int len, + int *offset, bool write_all, bool best_effort) { ChardevClass *cc = CHARDEV_GET_CLASS(s); int res = 0; @@ -119,7 +118,14 @@ static int qemu_chr_write_buffer(Chardev *s, retry: res = cc->chr_write(s, buf + *offset, len - *offset); if (res < 0 && errno == EAGAIN && write_all) { + if (best_effort && s->retries > 50) { + break; + } + g_usleep(100); + if (best_effort) { + s->retries++; + } goto retry; } @@ -127,6 +133,7 @@ static int qemu_chr_write_buffer(Chardev *s, break; } + s->retries = 0; *offset += res; if (!write_all) { break; @@ -140,7 +147,8 @@ static int qemu_chr_write_buffer(Chardev *s, return res; } -int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all) +int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, + bool write_all, bool best_effort) { int offset = 0; int res; @@ -148,11 +156,11 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all) if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_PLAY) { replay_char_write_event_load(&res, &offset); assert(offset <= len); - qemu_chr_write_buffer(s, buf, offset, &offset, true); + qemu_chr_write_buffer(s, buf, offset, &offset, true, false); return res; } - res = qemu_chr_write_buffer(s, buf, len, &offset, write_all); + res = qemu_chr_write_buffer(s, buf, len, &offset, write_all, best_effort); if (qemu_chr_replay(s) && replay_mode == REPLAY_MODE_RECORD) { replay_char_write_event_save(res, offset); diff --git a/hw/char/pl011.c b/hw/char/pl011.c index 13e784f9d9..348188f49e 100644 --- a/hw/char/pl011.c +++ b/hw/char/pl011.c @@ -179,11 +179,8 @@ static void pl011_write(void *opaque, hwaddr offset, switch (offset >> 2) { case 0: /* UARTDR */ - /* ??? Check if transmitter is enabled. */ ch = value; - /* XXX this blocks entire thread. Rewrite to use - * qemu_chr_fe_write and background I/O callbacks */ - qemu_chr_fe_write_all(&s->chr, &ch, 1); + qemu_chr_fe_try_write_all(&s->chr, &ch, 1); s->int_level |= PL011_INT_TX; pl011_update(s); break; diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h index a553843364..18281ccfca 100644 --- a/include/chardev/char-fe.h +++ b/include/chardev/char-fe.h @@ -220,6 +220,20 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len); */ int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len); +/** + * qemu_chr_fe_try_write_all: + * @buf: the data + * @len: the number of bytes to send + * + * Write data to a character backend from the front end. This function will + * send data from the front end to the back end. It provides function as to + * @qemu_chr_fe_write_all, except the data will be dropped after 50 attempts + * of transmissions are done. + * + * Returns: the number of bytes consumed (0 if no associated Chardev) + */ +int qemu_chr_fe_try_write_all(CharBackend *be, const uint8_t *buf, int len); + /** * qemu_chr_fe_read_all: * @buf: the data buffer diff --git a/include/chardev/char.h b/include/chardev/char.h index 00589a6025..425a007a0a 100644 --- a/include/chardev/char.h +++ b/include/chardev/char.h @@ -65,6 +65,7 @@ struct Chardev { char *filename; int logfd; int be_open; + int retries; GSource *gsource; GMainContext *gcontext; DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST); @@ -221,8 +222,9 @@ void qemu_chr_set_feature(Chardev *chr, ChardevFeature feature); QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename, bool permit_mux_mon); -int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool write_all); -#define qemu_chr_write_all(s, buf, len) qemu_chr_write(s, buf, len, true) +int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, + bool write_all, bool best_effort); +#define qemu_chr_write_all(s, buf, len) qemu_chr_write(s, buf, len, true, false) int qemu_chr_wait_connected(Chardev *chr, Error **errp); #define TYPE_CHARDEV "chardev" -- 2.23.0