| Message ID | 20211128120723.4053-1-olivier.heriveaux@ledger.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix STM32F2XX USART data register readout | expand |
On Sun, 28 Nov 2021 at 12:07, Olivier Hériveaux <olivier.heriveaux@ledger.fr> wrote: > > Fix issue where the data register may be overwritten by next character > reception before being read and returned. > > Signed-off-by: Olivier Hériveaux <olivier.heriveaux@ledger.fr> > --- > hw/char/stm32f2xx_usart.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c > index 8df0832424..fde67f4f03 100644 > --- a/hw/char/stm32f2xx_usart.c > +++ b/hw/char/stm32f2xx_usart.c > @@ -103,10 +103,11 @@ static uint64_t stm32f2xx_usart_read(void *opaque, hwaddr addr, > return retvalue; > case USART_DR: > DB_PRINT("Value: 0x%" PRIx32 ", %c\n", s->usart_dr, (char) s->usart_dr); > + retvalue = s->usart_dr & 0x3FF; > s->usart_sr &= ~USART_SR_RXNE; > qemu_chr_fe_accept_input(&s->chr); > qemu_set_irq(s->irq, 0); > - return s->usart_dr & 0x3FF; > + return retvalue; > case USART_BRR: > return s->usart_brr; > case USART_CR1: > -- > 2.17.1 The bug happens because qemu_chr_fe_accept_input() can cause stm32f2xx_usart_receive() to be called, right ? Reviewed-by: Peter Maydell <peter.maydell@linaro.org> I'll put this in my list of patches to take via target-arm.next for the 7.0 release. thanks -- PMM
On Mon, Nov 29, 2021 at 12:46 AM Olivier Hériveaux <olivier.heriveaux@ledger.fr> wrote: > > Fix issue where the data register may be overwritten by next character > reception before being read and returned. > > Signed-off-by: Olivier Hériveaux <olivier.heriveaux@ledger.fr> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > hw/char/stm32f2xx_usart.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c > index 8df0832424..fde67f4f03 100644 > --- a/hw/char/stm32f2xx_usart.c > +++ b/hw/char/stm32f2xx_usart.c > @@ -103,10 +103,11 @@ static uint64_t stm32f2xx_usart_read(void *opaque, hwaddr addr, > return retvalue; > case USART_DR: > DB_PRINT("Value: 0x%" PRIx32 ", %c\n", s->usart_dr, (char) s->usart_dr); > + retvalue = s->usart_dr & 0x3FF; > s->usart_sr &= ~USART_SR_RXNE; > qemu_chr_fe_accept_input(&s->chr); > qemu_set_irq(s->irq, 0); > - return s->usart_dr & 0x3FF; > + return retvalue; > case USART_BRR: > return s->usart_brr; > case USART_CR1: > -- > 2.17.1 > > > -- > > Les informations contenues dans ce message électronique ainsi que celles > contenues dans les documents attachés sont strictement confidentielles et > sont destinées à l'usage exclusif du (des) destinataire(s) nommé(s). > Toute > divulgation, distribution ou reproduction, même partielle, en est > strictement interdite sauf autorisation écrite et expresse de l’émetteur. > Si vous recevez ce message par erreur, veuillez le notifier immédiatement à > son émetteur par retour, et le détruire ainsi que tous les documents qui y > sont attachés. > > > The information contained in this email and in any > document enclosed is strictly confidential and is intended solely for the > use of the individual or entity to which it is addressed. > Partial or total > disclosure, distribution or reproduction of its contents is strictly > prohibited unless expressly approved in writing by the sender. > If you have > received this communication in error, please notify us immediately by > responding to this email, and then delete the message and its attached > files from your system. > >
If I understand correctly (I'm not a QEmu internals expert), yes this is what happens. Maybe stm32f2xx_usart_can_receive() is also called but since the USART_SR_RXNE flag is reset before the USART_DR is read, it does not prevent reading the next character. Best regards, Olivier Hériveaux Le lun. 29 nov. 2021 à 11:46, Peter Maydell <peter.maydell@linaro.org> a écrit : > On Sun, 28 Nov 2021 at 12:07, Olivier Hériveaux > <olivier.heriveaux@ledger.fr> wrote: > > > > Fix issue where the data register may be overwritten by next character > > reception before being read and returned. > > > > Signed-off-by: Olivier Hériveaux <olivier.heriveaux@ledger.fr> > > --- > > hw/char/stm32f2xx_usart.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c > > index 8df0832424..fde67f4f03 100644 > > --- a/hw/char/stm32f2xx_usart.c > > +++ b/hw/char/stm32f2xx_usart.c > > @@ -103,10 +103,11 @@ static uint64_t stm32f2xx_usart_read(void *opaque, > hwaddr addr, > > return retvalue; > > case USART_DR: > > DB_PRINT("Value: 0x%" PRIx32 ", %c\n", s->usart_dr, (char) > s->usart_dr); > > + retvalue = s->usart_dr & 0x3FF; > > s->usart_sr &= ~USART_SR_RXNE; > > qemu_chr_fe_accept_input(&s->chr); > > qemu_set_irq(s->irq, 0); > > - return s->usart_dr & 0x3FF; > > + return retvalue; > > case USART_BRR: > > return s->usart_brr; > > case USART_CR1: > > -- > > 2.17.1 > > The bug happens because qemu_chr_fe_accept_input() can cause > stm32f2xx_usart_receive() to be called, right ? > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > I'll put this in my list of patches to take via target-arm.next for the > 7.0 release. > > thanks > -- PMM >
diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c index 8df0832424..fde67f4f03 100644 --- a/hw/char/stm32f2xx_usart.c +++ b/hw/char/stm32f2xx_usart.c @@ -103,10 +103,11 @@ static uint64_t stm32f2xx_usart_read(void *opaque, hwaddr addr, return retvalue; case USART_DR: DB_PRINT("Value: 0x%" PRIx32 ", %c\n", s->usart_dr, (char) s->usart_dr); + retvalue = s->usart_dr & 0x3FF; s->usart_sr &= ~USART_SR_RXNE; qemu_chr_fe_accept_input(&s->chr); qemu_set_irq(s->irq, 0); - return s->usart_dr & 0x3FF; + return retvalue; case USART_BRR: return s->usart_brr; case USART_CR1:
Fix issue where the data register may be overwritten by next character reception before being read and returned. Signed-off-by: Olivier Hériveaux <olivier.heriveaux@ledger.fr> --- hw/char/stm32f2xx_usart.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)