On Wed, 12 Mar 2025 at 13:36, Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Tue, 11 Mar 2025 at 15:37, Paolo Bonzini <pbonz...@redhat.com> wrote:
> >
> > The guest does not control whether characters are sent on the UART.
> > Sending them before the guest happens to boot will now result in a
> > "guest error" log entry that is only because of timing, even if the
> > guest _would_ later setup the receiver correctly.
> >
> > This reverts commit abf2b6a028670bd2890bb3aee7e103fe53e4b0df, apart
> > from adding the comment.
> >
> > Cc: Philippe Mathieu-Daudé <phi...@linaro.org>
> > Cc: Peter Maydell <peter.mayd...@linaro.org>
> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> > ---
> > hw/char/pl011.c | 12 ++----------
> > 1 file changed, 2 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> > index 23a9db8c57c..efca8baecd7 100644
> > --- a/hw/char/pl011.c
> > +++ b/hw/char/pl011.c
> > @@ -85,7 +85,6 @@ DeviceState *pl011_create(hwaddr addr, qemu_irq irq,
> > Chardev *chr)
> > #define CR_OUT1 (1 << 12)
> > #define CR_RTS (1 << 11)
> > #define CR_DTR (1 << 10)
> > -#define CR_RXE (1 << 9)
> > #define CR_TXE (1 << 8)
> > #define CR_LBE (1 << 7)
> > #define CR_UARTEN (1 << 0)
> > @@ -490,16 +489,9 @@ static int pl011_can_receive(void *opaque)
> > unsigned fifo_depth = pl011_get_fifo_depth(s);
> > unsigned fifo_available = fifo_depth - s->read_count;
> >
> > - if (!(s->cr & CR_UARTEN)) {
> > - qemu_log_mask(LOG_GUEST_ERROR,
> > - "PL011 receiving data on disabled UART\n");
> > - }
> > - if (!(s->cr & CR_RXE)) {
> > - qemu_log_mask(LOG_GUEST_ERROR,
> > - "PL011 receiving data on disabled RX UART\n");
> > - }
> > + /* Should check enable and return 0? */
>
> We decided deliberately not to check the enable and return 0
> here, as described in the commit message of abf2b6a028670bd:
> we think there's too likely to be existing works-on-QEMU code
> out there that doesn't ever set the enable bits.
>
> Otherwise, yes, agreed with the revert.
I've applied this to target-arm.next with the comment expanded
(and I left the define of CR_RXE in too):
+ /*
+ * In theory we should check the UART and RX enable bits here and
+ * return 0 if they are not set (so the guest can't receive data
+ * until you have enabled the UART). In practice we suspect there
+ * is at least some guest code out there which has been tested only
+ * on QEMU and which never bothers to enable the UART because we
+ * historically never enforced that. So we effectively keep the
+ * UART continuously enabled regardless of the enable bits.
+ */
thanks
-- PMM
