[PATCH] hw/char/pl011: support backend hotswap

[PATCH] hw/char/pl011: support backend hotswap

[Alexander Mikhalitsyn]

From: Aleksanr Mikhalitsyn <alexander@mihalicyn.com>

Currently, when Incus issues "chardev-change" QMP command to change
chardev backend from ringbuf to socket it receives an error (with aarch64 VM):
"Chardev user does not support chardev hotswap" [1], [2]

Let's fix this by properly implementing BackendChangeHandler for pl011.

Link: https://discuss.linuxcontainers.org/t/unable-to-connect-to-vm-console-on-arm-architecture/23096/3 [1]
Link: https://github.com/lxc/distrobuilder/issues/892 [2]
Reported-by: Stéphane Graber <stgraber@stgraber.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
---
 hw/char/pl011.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index cb12c3e224f..3622248ec0c 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -660,12 +660,26 @@ static void pl011_init(Object *obj)
     s->id = pl011_id_arm;
 }
 
+static int pl011_be_change(void *opaque)
+{
+    PL011State *s = opaque;
+    int break_enable = s->lcr & LCR_BRK;
+
+    qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
+                             pl011_event, pl011_be_change, s, NULL, true);
+
+    qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
+                      &break_enable);
+
+    return 0;
+}
+
 static void pl011_realize(DeviceState *dev, Error **errp)
 {
     PL011State *s = PL011(dev);
 
     qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
-                             pl011_event, NULL, s, NULL, true);
+                             pl011_event, pl011_be_change, s, NULL, true);
 }
 
 static void pl011_reset(DeviceState *dev)
-- 
2.47.3

Re: [PATCH] hw/char/pl011: support backend hotswap

[Alex Bennée]

Alexander Mikhalitsyn <alexander@mihalicyn.com> writes:

> From: Aleksanr Mikhalitsyn <alexander@mihalicyn.com>
>
> Currently, when Incus issues "chardev-change" QMP command to change
> chardev backend from ringbuf to socket it receives an error (with aarch64 VM):
> "Chardev user does not support chardev hotswap" [1], [2]
>
> Let's fix this by properly implementing BackendChangeHandler for pl011.
>
> Link: https://discuss.linuxcontainers.org/t/unable-to-connect-to-vm-console-on-arm-architecture/23096/3 [1]
> Link: https://github.com/lxc/distrobuilder/issues/892 [2]
> Reported-by: Stéphane Graber <stgraber@stgraber.org>
> Signed-off-by: Alexander Mikhalitsyn
> <aleksandr.mikhalitsyn@futurfusion.io>

At first I wondered why the FE needs to care about where the BE is
routed to but 7bb86085e61 (char: chardevice hotswap) explains:

  However, backends are not stateless and are set up by the frontends
  via qemu_chr_fe_<> functions, and it's not (generally) possible to
  replay that setup entirely in a backend code, as different chardevs
  respond to the setup calls differently, so do frontends work
  differently basing on those setup responses. Moreover, some frontend
  can generally get and save the backend pointer
  (qemu_chr_fe_get_driver()), and it will become invalid after backend
  change.

> ---
>  hw/char/pl011.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> index cb12c3e224f..3622248ec0c 100644
> --- a/hw/char/pl011.c
> +++ b/hw/char/pl011.c
> @@ -660,12 +660,26 @@ static void pl011_init(Object *obj)
>      s->id = pl011_id_arm;
>  }
>  
> +static int pl011_be_change(void *opaque)
> +{
> +    PL011State *s = opaque;
> +    int break_enable = s->lcr & LCR_BRK;
> +
> +    qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
> +                             pl011_event, pl011_be_change, s, NULL, true);
> +
> +    qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
> +                      &break_enable);

So this is just ensuring that if we switch to a serial backend we
properly register the break behaviour on the backend? Is it idempotent?

> +
> +    return 0;
> +}
> +
>  static void pl011_realize(DeviceState *dev, Error **errp)
>  {
>      PL011State *s = PL011(dev);
>  
>      qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
> -                             pl011_event, NULL, s, NULL, true);
> +                             pl011_event, pl011_be_change, s, NULL, true);
>  }

This does make me wonder if having a static pl011_set_handlers helper
would keep things tidy and in one place? Either way:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

>  
>  static void pl011_reset(DeviceState *dev)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH] hw/char/pl011: support backend hotswap

[Alexander Mikhalitsyn]

Am Mo., 10. Aug. 2026 um 16:58 Uhr schrieb Alex Bennée <alex.bennee@linaro.org>:
>
> Alexander Mikhalitsyn <alexander@mihalicyn.com> writes:
>
> > From: Aleksanr Mikhalitsyn <alexander@mihalicyn.com>

Ugh, I did git format-patch and copied this patch from my Raspberry PI
dev/test machine
and it turns out that I have a stupid typo in my `git config get
user.name` on that machine.
I'll drop -v2 a bit later.

> >
> > Currently, when Incus issues "chardev-change" QMP command to change
> > chardev backend from ringbuf to socket it receives an error (with aarch64 VM):
> > "Chardev user does not support chardev hotswap" [1], [2]
> >
> > Let's fix this by properly implementing BackendChangeHandler for pl011.
> >
> > Link: https://discuss.linuxcontainers.org/t/unable-to-connect-to-vm-console-on-arm-architecture/23096/3 [1]
> > Link: https://github.com/lxc/distrobuilder/issues/892 [2]
> > Reported-by: Stéphane Graber <stgraber@stgraber.org>
> > Signed-off-by: Alexander Mikhalitsyn
> > <aleksandr.mikhalitsyn@futurfusion.io>

Dear Alex,

first of all thank you very much for such a fast reaction and review ;-)

>
> At first I wondered why the FE needs to care about where the BE is
> routed to but 7bb86085e61 (char: chardevice hotswap) explains:
>
>   However, backends are not stateless and are set up by the frontends
>   via qemu_chr_fe_<> functions, and it's not (generally) possible to
>   replay that setup entirely in a backend code, as different chardevs
>   respond to the setup calls differently, so do frontends work
>   differently basing on those setup responses. Moreover, some frontend
>   can generally get and save the backend pointer
>   (qemu_chr_fe_get_driver()), and it will become invalid after backend
>   change.

yes, exactly

>
> > ---
> >  hw/char/pl011.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> > index cb12c3e224f..3622248ec0c 100644
> > --- a/hw/char/pl011.c
> > +++ b/hw/char/pl011.c
> > @@ -660,12 +660,26 @@ static void pl011_init(Object *obj)
> >      s->id = pl011_id_arm;
> >  }
> >
> > +static int pl011_be_change(void *opaque)
> > +{
> > +    PL011State *s = opaque;
> > +    int break_enable = s->lcr & LCR_BRK;
> > +
> > +    qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
> > +                             pl011_event, pl011_be_change, s, NULL, true);
> > +
> > +    qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
> > +                      &break_enable);
>
> So this is just ensuring that if we switch to a serial backend we
> properly register the break behaviour on the backend? Is it idempotent?

Right. And AFAIU, `CHR_IOCTL_SERIAL_SET_BREAK` is specific to the
char-serial device only.

So when we switch, let's say, from a serial to a socket backend, this
call is basically a no-op.
But when we switch back from the socket to the serial, this will
trigger `tcsendbreak(fioc->fd, 1)`, which is not idempotent (and is a
single-shot action).

But I believe it's not a problem here, because we do it once when we
*switch* between two different backends, and `fioc->fd` is a fresh fd
every time.
My point is that we can't really trigger this thing twice for the same fd.

Please, correct me if I'm wrong. I took inspiration from 16550A's
serial_be_change() when working on this.

==== BEGIN offtopic :-)

Btw, thanks to your question I did some more research on tcsendbreak()
and BRK behavior [1] and it feels to me that what we currently have in
pl011 UARTLCR_H's register handling code is not really correct. See:

static void pl011_write(void *opaque, hwaddr offset,
                        uint64_t value, unsigned size)
{
<..>
    case 11: /* UARTLCR_H */
        /* Reset the FIFO state on FIFO enable or disable */
        if ((s->lcr ^ value) & LCR_FEN) {
            pl011_reset_rx_fifo(s);
            pl011_reset_tx_fifo(s);
        }
        if ((s->lcr ^ value) & LCR_BRK) {

           // << spec [1] says that setting BRK means "If this bit is
set to 1, a low-level is continually output on the UARTTXD output,
after completing transmission of the current character."
           // what we do is CHR_IOCTL_SERIAL_SET_BREAK which is just
tcsendbreak(fioc->fd, 1) under the hood which can't emulate continuous
low-level signal on the UARTTXD output
           // for the time of BRK bit is set. It only does this for a
short period of time.

            int break_enable = value & LCR_BRK;
            qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
                              &break_enable);
            pl011_loopback_break(s, break_enable);
        }

I guess we need some pl011 expert in here :-)

[1] https://support.arm.com/documentation/ddi0183/g/programmers-model/register-descriptions/line-control-register--uartlcr-h

>
> > +
> > +    return 0;
> > +}
> > +
> >  static void pl011_realize(DeviceState *dev, Error **errp)
> >  {
> >      PL011State *s = PL011(dev);
> >
> >      qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
> > -                             pl011_event, NULL, s, NULL, true);
> > +                             pl011_event, pl011_be_change, s, NULL, true);
> >  }
>
> This does make me wonder if having a static pl011_set_handlers helper
> would keep things tidy and in one place? Either way:

I'm happy to do this too if you want me to. And probably for
hw/char/serial.c too, then?

>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Thanks ;-)

Giving all the suspicion around UARTLCR_H and BRK behavior I still
believe that I'm not making things
worse by this change, but better, cause now we can do backend hotswap.
But it would be great to fully
understand that part too.

Kind regards,
Alex

>
> >
> >  static void pl011_reset(DeviceState *dev)
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro

Re: [PATCH] hw/char/pl011: support backend hotswap

[Alexander Mikhalitsyn]

Am Mo., 10. Aug. 2026 um 19:31 Uhr schrieb Alexander Mikhalitsyn
<alexander@mihalicyn.com>:
>
> Am Mo., 10. Aug. 2026 um 16:58 Uhr schrieb Alex Bennée <alex.bennee@linaro.org>:
> >
> > Alexander Mikhalitsyn <alexander@mihalicyn.com> writes:
> >
> > > From: Aleksanr Mikhalitsyn <alexander@mihalicyn.com>
>
> Ugh, I did git format-patch and copied this patch from my Raspberry PI
> dev/test machine
> and it turns out that I have a stupid typo in my `git config get
> user.name` on that machine.
> I'll drop -v2 a bit later.
>
> > >
> > > Currently, when Incus issues "chardev-change" QMP command to change
> > > chardev backend from ringbuf to socket it receives an error (with aarch64 VM):
> > > "Chardev user does not support chardev hotswap" [1], [2]
> > >
> > > Let's fix this by properly implementing BackendChangeHandler for pl011.
> > >
> > > Link: https://discuss.linuxcontainers.org/t/unable-to-connect-to-vm-console-on-arm-architecture/23096/3 [1]
> > > Link: https://github.com/lxc/distrobuilder/issues/892 [2]
> > > Reported-by: Stéphane Graber <stgraber@stgraber.org>
> > > Signed-off-by: Alexander Mikhalitsyn
> > > <aleksandr.mikhalitsyn@futurfusion.io>
>
> Dear Alex,
>
> first of all thank you very much for such a fast reaction and review ;-)
>
> >
> > At first I wondered why the FE needs to care about where the BE is
> > routed to but 7bb86085e61 (char: chardevice hotswap) explains:
> >
> >   However, backends are not stateless and are set up by the frontends
> >   via qemu_chr_fe_<> functions, and it's not (generally) possible to
> >   replay that setup entirely in a backend code, as different chardevs
> >   respond to the setup calls differently, so do frontends work
> >   differently basing on those setup responses. Moreover, some frontend
> >   can generally get and save the backend pointer
> >   (qemu_chr_fe_get_driver()), and it will become invalid after backend
> >   change.
>
> yes, exactly
>
> >
> > > ---
> > >  hw/char/pl011.c | 16 +++++++++++++++-
> > >  1 file changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> > > index cb12c3e224f..3622248ec0c 100644
> > > --- a/hw/char/pl011.c
> > > +++ b/hw/char/pl011.c
> > > @@ -660,12 +660,26 @@ static void pl011_init(Object *obj)
> > >      s->id = pl011_id_arm;
> > >  }
> > >
> > > +static int pl011_be_change(void *opaque)
> > > +{
> > > +    PL011State *s = opaque;
> > > +    int break_enable = s->lcr & LCR_BRK;
> > > +
> > > +    qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
> > > +                             pl011_event, pl011_be_change, s, NULL, true);
> > > +
> > > +    qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
> > > +                      &break_enable);
> >
> > So this is just ensuring that if we switch to a serial backend we
> > properly register the break behaviour on the backend? Is it idempotent?
>
> Right. And AFAIU, `CHR_IOCTL_SERIAL_SET_BREAK` is specific to the
> char-serial device only.
>
> So when we switch, let's say, from a serial to a socket backend, this
> call is basically a no-op.
> But when we switch back from the socket to the serial, this will
> trigger `tcsendbreak(fioc->fd, 1)`, which is not idempotent (and is a
> single-shot action).
>
> But I believe it's not a problem here, because we do it once when we
> *switch* between two different backends, and `fioc->fd` is a fresh fd
> every time.
> My point is that we can't really trigger this thing twice for the same fd.
>
> Please, correct me if I'm wrong. I took inspiration from 16550A's
> serial_be_change() when working on this.
>
> ==== BEGIN offtopic :-)
>
> Btw, thanks to your question I did some more research on tcsendbreak()
> and BRK behavior [1] and it feels to me that what we currently have in
> pl011 UARTLCR_H's register handling code is not really correct. See:
>
> static void pl011_write(void *opaque, hwaddr offset,
>                         uint64_t value, unsigned size)
> {
> <..>
>     case 11: /* UARTLCR_H */
>         /* Reset the FIFO state on FIFO enable or disable */
>         if ((s->lcr ^ value) & LCR_FEN) {
>             pl011_reset_rx_fifo(s);
>             pl011_reset_tx_fifo(s);
>         }
>         if ((s->lcr ^ value) & LCR_BRK) {
>
>            // << spec [1] says that setting BRK means "If this bit is
> set to 1, a low-level is continually output on the UARTTXD output,
> after completing transmission of the current character."
>            // what we do is CHR_IOCTL_SERIAL_SET_BREAK which is just
> tcsendbreak(fioc->fd, 1) under the hood which can't emulate continuous
> low-level signal on the UARTTXD output
>            // for the time of BRK bit is set. It only does this for a
> short period of time.
>
>             int break_enable = value & LCR_BRK;
>             qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
>                               &break_enable);
>             pl011_loopback_break(s, break_enable);
>         }
>
> I guess we need some pl011 expert in here :-)
>
> [1] https://support.arm.com/documentation/ddi0183/g/programmers-model/register-descriptions/line-control-register--uartlcr-h

Dear friends,

I looked closely into this stuff and found something interesting I
would like to share just in case
and discuss. (I'm not an expert in UART or tty code, so I can be wrong
in my understanding.)

1. It seems to me that QEMU BRK emulation code for both PL011 and
16550A is not fully correct.

According to the spec for PL011 [1]:
===
Send break. If this bit is set to 1, a low-level is continually output
on the UARTTXD output, after completing transmission of the current
character. For the proper execution of the break command, the software
must set this bit for at least two complete frames.

For normal use, this bit must be cleared to 0.
===

According to the spec for 16550A [2]:
===
bit 6: When this bit is set a break condition is
forced in the transmission line. The serial output
pin (txd) is forced to the spacing state (zero).
When this bit is cleared, the break state is
removed. The break state has no effect on the
transmitter’s logic, so if several characters are
stored in the transmitter’s FIFO they will be
removed from this FIFO and passed
sequentially to the Transmitter Shift Register
which serializes them. This fact can be useful to
establish the break time making use of the THR
Empty and Transmitter Empty flags of the LSR
===

My interpretation of this is that setting a BRK flag for both means
that TX line will be set to spacing state (zero) for an infinite time
(until bit is unset).

Our implementation in QEMU for both just calls qemu_chr_fe_ioctl(..
CHR_IOCTL_SERIAL_SET_BREAK ..) which is just:
===
            int enable = *(int *)arg;
            if (enable) {
                tcsendbreak(fioc->fd, 1);
            }
===
under the hood.

tcsendbreak() in glibc is implemented as ioctl(TCSBRK / TCSBRKP),
which internally sets a BRK bit for a certain
period of time and unsets it back. To be fully sure, I looked into the
Linux kernel, and the call stack is the following:
tty_ioctl(TCSBRK / TCSBRKP) -> send_break():

===
/**
 * send_break - performed time break
 * @tty: device to break on
 * @duration: timeout in mS
 *
 * Perform a timed break on hardware that lacks its own driver level timed
 * break functionality.
 *
 * Locking:
 * @tty->atomic_write_lock serializes
 */
static int send_break(struct tty_struct *tty, unsigned int duration)
{
        int retval;

        if (tty->ops->break_ctl == NULL)
                return 0;

        if (tty->driver->flags & TTY_DRIVER_HARDWARE_BREAK) // <<<
*not* our case
                return tty->ops->break_ctl(tty, duration);

        /* Do the work ourselves */
        if (tty_write_lock(tty, false) < 0)
                return -EINTR;

        retval = tty->ops->break_ctl(tty, -1); // << set BRK bit
        if (!retval) {
                msleep_interruptible(duration);
                retval = tty->ops->break_ctl(tty, 0); // << unset BRK bit
        } else if (retval == -EOPNOTSUPP) {
                /* some drivers can tell only dynamically */
                retval = 0;
        }
        tty_write_unlock(tty);

        if (signal_pending(current))
                retval = -EINTR;

        return retval;
}
===

tty->ops->break_ctl is just uart_break_ctl() which in turns is just a
wrapper for (struct uart_ops *)->break_ctl().

For PL011 (struct uart_ops *)->break_ctl is pl011_break_ctl():
===
static void pl011_break_ctl(struct uart_port *port, int break_state)
{
        struct uart_amba_port *uap =
            container_of(port, struct uart_amba_port, port);
        unsigned long flags;
        unsigned int lcr_h;

        uart_port_lock_irqsave(&uap->port, &flags);
        lcr_h = pl011_read(uap, REG_LCRH_TX);
        if (break_state == -1)
                lcr_h |= UART01x_LCRH_BRK;
        else
                lcr_h &= ~UART01x_LCRH_BRK;
        pl011_write(lcr_h, uap, REG_LCRH_TX);
        uart_port_unlock_irqrestore(&uap->port, flags);
}
===

For 16550A (struct uart_ops *)->break_ctl is serial8250_break_ctl():
===
static void serial8250_break_ctl(struct uart_port *port, int break_state)
{
        struct uart_8250_port *up = up_to_u8250p(port);

        guard(serial8250_rpm)(up);
        guard(uart_port_lock_irqsave)(port);

        if (break_state == -1)
                up->lcr |= UART_LCR_SBC;
        else
                up->lcr &= ~UART_LCR_SBC;
        serial_port_out(port, UART_LCR, up->lcr);
}
===

i.e the same logic.

It means that our current code in QEMU is good enough for simple cases
when the guest sets a BRK bit
for a short period of time. The guest can't really see the difference.
I hoped that this pattern when
BRK bit is set and then cleared is the only way how BRK bit is used in
Linux kernel (then our gap in the
implementation would not be noticeable as Linux kernel is our primary
kernel), but it's not.

We actually have:
===
/*
 * Split this up, as gcc can choke on it otherwise..
 */
long tty_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
<...>
        case TIOCSBRK: /* Turn break on, unconditionally */
                if (tty->ops->break_ctl)
                        return tty->ops->break_ctl(tty, -1);
                return 0;
        case TIOCCBRK: /* Turn break off, unconditionally */
                if (tty->ops->break_ctl)
                        return tty->ops->break_ctl(tty, 0);
                return 0;
===

So even *userspace* can set BRK bit for PL011/16550A for as long as it wants to!

The question is why we don't use these ioctls in QEMU to properly
emulate BRK behavior and instead use ioctl(TCSBRK / TCSBRKP)
which sets BRK bit and clears it after a short period of time?.. There
must be a reason, but I'm not familiar with the code
to see it. Ideas?

Link: https://support.arm.com/documentation/ddi0183/f/programmer-s-model/register-descriptions/line-control-register--uartlcr-h
[1]
Link: https://caro.su/msx/ocm_de1/16550.pdf [2]

Kind regards,
Alex

>
> >
> > > +
> > > +    return 0;
> > > +}
> > > +
> > >  static void pl011_realize(DeviceState *dev, Error **errp)
> > >  {
> > >      PL011State *s = PL011(dev);
> > >
> > >      qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
> > > -                             pl011_event, NULL, s, NULL, true);
> > > +                             pl011_event, pl011_be_change, s, NULL, true);
> > >  }
> >
> > This does make me wonder if having a static pl011_set_handlers helper
> > would keep things tidy and in one place? Either way:
>
> I'm happy to do this too if you want me to. And probably for
> hw/char/serial.c too, then?
>
> >
> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
> Thanks ;-)
>
> Giving all the suspicion around UARTLCR_H and BRK behavior I still
> believe that I'm not making things
> worse by this change, but better, cause now we can do backend hotswap.
> But it would be great to fully
> understand that part too.
>
> Kind regards,
> Alex
>
> >
> > >
> > >  static void pl011_reset(DeviceState *dev)
> >
> > --
> > Alex Bennée
> > Virtualisation Tech Lead @ Linaro