From: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>

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.

Please, note that we have to "replay" CHR_IOCTL_SERIAL_SET_BREAK, because
if BRK bit was set before backend change (i.e. (s->lcr & LCR_BRK) is true),
then after change we need to send break to a new backend too.

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>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
---
v3:
	- introduced pl011_set_handlers()
	  [ as suggested by Alex Bennée ]
	- introduced pl011_set_break()
	  [ as suggested by Philippe Mathieu-Daudé ]
v2:
	- fixed a typo in commit author name
	  [ 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. ]
	- added RWB tag from Alex Bennée
	- adjusted a commit message
---
 hw/char/pl011.c | 34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index cb12c3e224f..031afa5b246 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -425,6 +425,15 @@ static void pl011_loopback_break(PL011State *s, int brk_enable)
     }
 }
 
+static inline int pl011_set_break(PL011State *s, uint64_t lcr)
+{
+    int break_enable = lcr & LCR_BRK;
+
+    qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK, &break_enable);
+
+    return break_enable;
+}
+
 static void pl011_write(void *opaque, hwaddr offset,
                         uint64_t value, unsigned size)
 {
@@ -462,9 +471,7 @@ static void pl011_write(void *opaque, hwaddr offset,
             pl011_reset_tx_fifo(s);
         }
         if ((s->lcr ^ value) & LCR_BRK) {
-            int break_enable = value & LCR_BRK;
-            qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
-                              &break_enable);
+            int break_enable = pl011_set_break(s, value);
             pl011_loopback_break(s, break_enable);
         }
         s->lcr = value;
@@ -660,12 +667,29 @@ static void pl011_init(Object *obj)
     s->id = pl011_id_arm;
 }
 
+static int pl011_be_change(void *opaque);
+
+static inline void pl011_set_handlers(PL011State *s)
+{
+    qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
+                             pl011_event, pl011_be_change, s, NULL, true);
+}
+
+static int pl011_be_change(void *opaque)
+{
+    PL011State *s = opaque;
+
+    pl011_set_handlers(s);
+    pl011_set_break(s, s->lcr);
+
+    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_set_handlers(s);
 }
 
 static void pl011_reset(DeviceState *dev)
-- 
2.47.3

On Mon, 17 Aug 2026 at 12:33, Alexander Mikhalitsyn
<alexander@mihalicyn.com> wrote:
>
> From: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
>
> 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.
>
> Please, note that we have to "replay" CHR_IOCTL_SERIAL_SET_BREAK, because
> if BRK bit was set before backend change (i.e. (s->lcr & LCR_BRK) is true),
> then after change we need to send break to a new backend too.
>
> 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>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
> ---
> v3:
>         - introduced pl011_set_handlers()
>           [ as suggested by Alex Bennée ]
>         - introduced pl011_set_break()
>           [ as suggested by Philippe Mathieu-Daudé ]
> v2:
>         - fixed a typo in commit author name
>           [ 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. ]
>         - added RWB tag from Alex Bennée
>         - adjusted a commit message

I have one review suggestion here:

> +static inline int pl011_set_break(PL011State *s, uint64_t lcr)
> +{
> +    int break_enable = lcr & LCR_BRK;
> +
> +    qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK, &break_enable);
> +
> +    return break_enable;
> +}

I think this would be better with a similar API to
pl011_loopback_break(): make it take a bool brk_enable,
and return void. (pl011_loopback_break() takes an int,
but we shouldn't copy that: it uses it as a bool, so it
ought to take a bool.)

>  static void pl011_write(void *opaque, hwaddr offset,
>                          uint64_t value, unsigned size)
>  {
> @@ -462,9 +471,7 @@ static void pl011_write(void *opaque, hwaddr offset,
>              pl011_reset_tx_fifo(s);
>          }
>          if ((s->lcr ^ value) & LCR_BRK) {
> -            int break_enable = value & LCR_BRK;
> -            qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
> -                              &break_enable);
> +            int break_enable = pl011_set_break(s, value);
>              pl011_loopback_break(s, break_enable);

Then this code becomes something like
    bool break_enable = value & LCR_BRK;
    pl011_set_break(s, break_enable);
    pl011_loopback_break(s, break_enable);

which is more straightforward than passing the raw LCR value
to pl011_set_break and relying on it returning the "is BRK set?"
information that we then pass to pl011_loopback_break().

> +static int pl011_be_change(void *opaque);

> +static inline void pl011_set_handlers(PL011State *s)
> +{
> +    qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
> +                             pl011_event, pl011_be_change, s, NULL, true);
> +}
> +
> +static int pl011_be_change(void *opaque)
> +{
> +    PL011State *s = opaque;
> +
> +    pl011_set_handlers(s);
> +    pl011_set_break(s, s->lcr);

and here we would then pass in s->lcr & LCR_BRK.

> +
> +    return 0;
> +}

thanks
-- PMM

Am Fr., 21. Aug. 2026 um 11:57 Uhr schrieb Peter Maydell
<peter.maydell@linaro.org>:
>
> On Mon, 17 Aug 2026 at 12:33, Alexander Mikhalitsyn
> <alexander@mihalicyn.com> wrote:
> >
> > From: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
> >
> > 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.
> >
> > Please, note that we have to "replay" CHR_IOCTL_SERIAL_SET_BREAK, because
> > if BRK bit was set before backend change (i.e. (s->lcr & LCR_BRK) is true),
> > then after change we need to send break to a new backend too.
> >
> > 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>
> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
> > ---
> > v3:
> >         - introduced pl011_set_handlers()
> >           [ as suggested by Alex Bennée ]
> >         - introduced pl011_set_break()
> >           [ as suggested by Philippe Mathieu-Daudé ]
> > v2:
> >         - fixed a typo in commit author name
> >           [ 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. ]
> >         - added RWB tag from Alex Bennée
> >         - adjusted a commit message
>
> I have one review suggestion here:
>
> > +static inline int pl011_set_break(PL011State *s, uint64_t lcr)
> > +{
> > +    int break_enable = lcr & LCR_BRK;
> > +
> > +    qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK, &break_enable);
> > +
> > +    return break_enable;
> > +}
>
> I think this would be better with a similar API to
> pl011_loopback_break(): make it take a bool brk_enable,
> and return void. (pl011_loopback_break() takes an int,
> but we shouldn't copy that: it uses it as a bool, so it
> ought to take a bool.)

Dear Peter,

Sure, I've changed this ;-) The only thing is that I kept brk_enable
as int, because in serial_chr_ioctl we have:
static int serial_chr_ioctl(Chardev *chr, int cmd, void *arg)
{
<...>
    case CHR_IOCTL_SERIAL_SET_BREAK:
        {
            int enable = *(int *)arg; // << int is assumed
            if (enable) {
                tcsendbreak(fioc->fd, 1);
            }
        }

I'll send -v4 in a moment.

Kind regards,
Alex

>
> >  static void pl011_write(void *opaque, hwaddr offset,
> >                          uint64_t value, unsigned size)
> >  {
> > @@ -462,9 +471,7 @@ static void pl011_write(void *opaque, hwaddr offset,
> >              pl011_reset_tx_fifo(s);
> >          }
> >          if ((s->lcr ^ value) & LCR_BRK) {
> > -            int break_enable = value & LCR_BRK;
> > -            qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_BREAK,
> > -                              &break_enable);
> > +            int break_enable = pl011_set_break(s, value);
> >              pl011_loopback_break(s, break_enable);
>
> Then this code becomes something like
>     bool break_enable = value & LCR_BRK;
>     pl011_set_break(s, break_enable);
>     pl011_loopback_break(s, break_enable);
>
> which is more straightforward than passing the raw LCR value
> to pl011_set_break and relying on it returning the "is BRK set?"
> information that we then pass to pl011_loopback_break().

+

>
> > +static int pl011_be_change(void *opaque);
>
> > +static inline void pl011_set_handlers(PL011State *s)
> > +{
> > +    qemu_chr_fe_set_handlers(&s->chr, pl011_can_receive, pl011_receive,
> > +                             pl011_event, pl011_be_change, s, NULL, true);
> > +}
> > +
> > +static int pl011_be_change(void *opaque)
> > +{
> > +    PL011State *s = opaque;
> > +
> > +    pl011_set_handlers(s);
> > +    pl011_set_break(s, s->lcr);
>
> and here we would then pass in s->lcr & LCR_BRK.

+

>
> > +
> > +    return 0;
> > +}
>
> thanks
> -- PMM