hw/char/stm32l4x5_usart.c | 93 +++++++++++++++++++++++------- tests/qtest/stm32l4x5_usart-test.c | 50 +++++++++++++++- 2 files changed, 120 insertions(+), 23 deletions(-)
Official STM32CubeL4 drivers use and require support for 16-bit writes to
UART registers for proper function.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2540
Signed-off-by: Jacob Whitaker Abrams <satur9nine@gmail.com>
---
hw/char/stm32l4x5_usart.c | 93 +++++++++++++++++++++++-------
tests/qtest/stm32l4x5_usart-test.c | 50 +++++++++++++++-
2 files changed, 120 insertions(+), 23 deletions(-)
diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
index dd1b099195..88c4a3b5a7 100644
--- a/hw/char/stm32l4x5_usart.c
+++ b/hw/char/stm32l4x5_usart.c
@@ -154,8 +154,15 @@ REG32(RDR, 0x24)
REG32(TDR, 0x28)
FIELD(TDR, TDR, 0, 9)
+#define ISR_RESET_VALUE (0x020000C0)
+
static void stm32l4x5_update_isr(Stm32l4x5UsartBaseState *s)
{
+ if (!(s->cr1 & R_CR1_UE_MASK)) {
+ s->isr = ISR_RESET_VALUE;
+ return;
+ }
+
if (s->cr1 & R_CR1_TE_MASK) {
s->isr |= R_ISR_TEACK_MASK;
} else {
@@ -404,9 +411,11 @@ static uint64_t stm32l4x5_usart_base_read(void *opaque, hwaddr addr,
unsigned int size)
{
Stm32l4x5UsartBaseState *s = opaque;
+ hwaddr base = addr & ~0x3ULL;
+ unsigned int offset = addr & 0x3;
uint64_t retvalue = 0;
- switch (addr) {
+ switch (base) {
case A_CR1:
retvalue = s->cr1;
break;
@@ -451,6 +460,13 @@ static uint64_t stm32l4x5_usart_base_read(void *opaque, hwaddr addr,
break;
}
+ /* Adjust for partial access */
+ if (size == 1) {
+ retvalue = (retvalue >> (offset * 8)) & 0xFF;
+ } else if (size == 2) {
+ retvalue = (retvalue >> (offset * 8)) & 0xFFFF;
+ }
+
trace_stm32l4x5_usart_read(addr, retvalue);
return retvalue;
@@ -460,55 +476,88 @@ static void stm32l4x5_usart_base_write(void *opaque, hwaddr addr,
uint64_t val64, unsigned int size)
{
Stm32l4x5UsartBaseState *s = opaque;
- const uint32_t value = val64;
+ hwaddr base = addr & ~0x3ULL;
+ unsigned int offset = addr & 0x3;
+ uint32_t value = (uint32_t)val64;
+
+ /* Build mask for partial access */
+ uint32_t mask;
+ if (size == 4) {
+ mask = 0xFFFFFFFF;
+ } else if (size == 2) {
+ mask = 0xFFFF << (offset * 8);
+ } else if (size == 1) {
+ mask = 0xFF << (offset * 8);
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported access size %u\n", __func__, size);
+ return;
+ }
+ value = (value << (offset * 8)) & mask;
trace_stm32l4x5_usart_write(addr, value);
- switch (addr) {
- case A_CR1:
- s->cr1 = value;
+ switch (base) {
+ case A_CR1: {
+ uint32_t old = s->cr1;
+ s->cr1 = (old & ~mask) | value;
stm32l4x5_update_params(s);
stm32l4x5_update_isr(s);
stm32l4x5_update_irq(s);
return;
- case A_CR2:
- s->cr2 = value;
+ }
+ case A_CR2: {
+ uint32_t old = s->cr2;
+ s->cr2 = (old & ~mask) | value;
stm32l4x5_update_params(s);
return;
- case A_CR3:
- s->cr3 = value;
+ }
+ case A_CR3: {
+ uint32_t old = s->cr3;
+ s->cr3 = (old & ~mask) | value;
return;
- case A_BRR:
- s->brr = value;
+ }
+ case A_BRR: {
+ uint32_t old = s->brr;
+ s->brr = (old & ~mask) | value;
stm32l4x5_update_params(s);
return;
- case A_GTPR:
- s->gtpr = value;
+ }
+ case A_GTPR: {
+ uint32_t old = s->gtpr;
+ s->gtpr = (old & ~mask) | value;
return;
- case A_RTOR:
- s->rtor = value;
+ }
+ case A_RTOR: {
+ uint32_t old = s->rtor;
+ s->rtor = (old & ~mask) | value;
return;
- case A_RQR:
+ }
+ case A_RQR: {
+ /* RQR is write-only, assume full 32-bit access */
usart_update_rqr(s, value);
return;
+ }
case A_ISR:
qemu_log_mask(LOG_GUEST_ERROR,
"%s: ISR is read only !\n", __func__);
return;
- case A_ICR:
- /* Clear the status flags */
+ case A_ICR: {
+ /* Clear flags: value is masked to written bytes */
s->isr &= ~value;
stm32l4x5_update_irq(s);
return;
+ }
case A_RDR:
qemu_log_mask(LOG_GUEST_ERROR,
"%s: RDR is read only !\n", __func__);
return;
- case A_TDR:
- s->tdr = value;
+ case A_TDR: {
+ uint32_t old = s->tdr;
+ s->tdr = (old & ~mask) | value;
s->isr &= ~R_ISR_TXE_MASK;
usart_transmit(NULL, G_IO_OUT, s);
return;
+ }
default:
qemu_log_mask(LOG_GUEST_ERROR,
"%s: Bad offset 0x%"HWADDR_PRIx"\n", __func__, addr);
@@ -521,12 +570,12 @@ static const MemoryRegionOps stm32l4x5_usart_base_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
.valid = {
.max_access_size = 4,
- .min_access_size = 4,
+ .min_access_size = 2,
.unaligned = false
},
.impl = {
.max_access_size = 4,
- .min_access_size = 4,
+ .min_access_size = 2,
.unaligned = false
},
};
diff --git a/tests/qtest/stm32l4x5_usart-test.c b/tests/qtest/stm32l4x5_usart-test.c
index a72c5a685d..86b36915ac 100644
--- a/tests/qtest/stm32l4x5_usart-test.c
+++ b/tests/qtest/stm32l4x5_usart-test.c
@@ -332,6 +332,53 @@ static void test_ack(void)
qtest_quit(qts);
}
+static void test_isr_reset(void)
+{
+ QTestState *qts = qtest_init("-M b-l475e-iot01a");
+ init_uart(qts);
+ /* ISR should show TEACK/REACK after UART init */
+ uint32_t isr = qtest_readl(qts, USART1_BASE_ADDR + A_ISR);
+ g_assert_true(isr & R_ISR_TEACK_MASK);
+ g_assert_true(isr & R_ISR_REACK_MASK);
+
+ /* Clear UE bit in CR1 -> USART disabled */
+ uint32_t cr1 = qtest_readl(qts, USART1_BASE_ADDR + A_CR1);
+ qtest_writel(qts, USART1_BASE_ADDR + A_CR1, cr1 & ~R_CR1_UE_MASK);
+ /* ISR must reset to reset value 0x020000C0 when UE is cleared */
+ isr = qtest_readl(qts, USART1_BASE_ADDR + A_ISR);
+ g_assert_cmpuint(isr, ==, 0x020000C0);
+
+ qtest_quit(qts);
+}
+
+static void test_16bit_access(void)
+{
+ QTestState *qts = qtest_init("-M b-l475e-iot01a");
+
+ /* Test 16-bit write/read on RTOR */
+ qtest_writel(qts, USART1_BASE_ADDR + A_RTOR, 0x00000000);
+ qtest_writew(qts, USART1_BASE_ADDR + A_RTOR, 0xABCD);
+ uint16_t rtor16 = qtest_readw(qts, USART1_BASE_ADDR + A_RTOR);
+ g_assert_cmpuint(rtor16, ==, 0xABCD);
+ uint32_t rtor32 = qtest_readl(qts, USART1_BASE_ADDR + A_RTOR);
+ g_assert_cmpuint(rtor32, ==, 0x0000ABCD);
+
+ /* Test upper half 16-bit write */
+ qtest_writew(qts, USART1_BASE_ADDR + A_RTOR + 2, 0x1234);
+ rtor32 = qtest_readl(qts, USART1_BASE_ADDR + A_RTOR);
+ g_assert_cmpuint(rtor32, ==, 0x1234ABCD);
+
+ /* Test 16-bit write/read on CR1 */
+ qtest_writel(qts, USART1_BASE_ADDR + A_CR1, 0x00000000);
+ qtest_writew(qts, USART1_BASE_ADDR + A_CR1, 0x00FF);
+ uint16_t cr116 = qtest_readw(qts, USART1_BASE_ADDR + A_CR1);
+ g_assert_cmpuint(cr116, ==, 0x00FF);
+ uint32_t cr132 = qtest_readl(qts, USART1_BASE_ADDR + A_CR1);
+ g_assert_cmpuint(cr132, ==, 0x000000FF);
+
+ qtest_quit(qts);
+}
+
static void check_clock(QTestState *qts, const char *path, uint32_t rcc_reg,
uint32_t reg_offset)
{
@@ -369,7 +416,8 @@ int main(int argc, char **argv)
qtest_add_func("stm32l4x5/usart/receive_str", test_receive_str);
qtest_add_func("stm32l4x5/usart/send_str", test_send_str);
qtest_add_func("stm32l4x5/usart/ack", test_ack);
+ qtest_add_func("stm32l4x5/usart/isr_reset", test_isr_reset);
+ qtest_add_func("stm32l4x5/usart/16bit_access", test_16bit_access);
qtest_add_func("stm32l4x5/usart/clock_enable", test_clock_enable);
return g_test_run();
}
-
--
2.43.0On Mon, 17 Aug 2026 at 06:03, Jacob Whitaker Abrams
<jwhitakera@gmail.com> wrote:
>
> Official STM32CubeL4 drivers use and require support for 16-bit writes to
> UART registers for proper function.
This is a breach of the datasheet (assuming I have the right one:
RM0351 "STM32L47xxx, STM32L48xxx, STM32L49xxx and STM32L4Axxx
advanced Arm®-based 32-bit MCUs"), which is pretty clear:
"The peripheral registers have to be accessed by words (32 bits)".
Still, if the official drivers are doing it then presumably the
hardware actually does allow smaller accesses.
Do the drivers do small accesses to the top parts of registers,
or do they always use 4-aligned addresses but just sometimes
do smaller width accesses ? If only the latter is needed, we
can make the code simpler (reads return the full register value
and the QEMU core code chops off the unneeded high parts, writes
are "write as if zero-extended").
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2540
> Signed-off-by: Jacob Whitaker Abrams <satur9nine@gmail.com>
> ---
> hw/char/stm32l4x5_usart.c | 93 +++++++++++++++++++++++-------
> tests/qtest/stm32l4x5_usart-test.c | 50 +++++++++++++++-
> 2 files changed, 120 insertions(+), 23 deletions(-)
>
> diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
> index dd1b099195..88c4a3b5a7 100644
> --- a/hw/char/stm32l4x5_usart.c
> +++ b/hw/char/stm32l4x5_usart.c
> @@ -154,8 +154,15 @@ REG32(RDR, 0x24)
> REG32(TDR, 0x28)
> FIELD(TDR, TDR, 0, 9)
>
> +#define ISR_RESET_VALUE (0x020000C0)
> +
> static void stm32l4x5_update_isr(Stm32l4x5UsartBaseState *s)
> {
> + if (!(s->cr1 & R_CR1_UE_MASK)) {
> + s->isr = ISR_RESET_VALUE;
> + return;
> + }
This seems to be an unrelated change to adding 16-bit access handling ?
> +
> if (s->cr1 & R_CR1_TE_MASK) {
> s->isr |= R_ISR_TEACK_MASK;
> } else {
> @@ -404,9 +411,11 @@ static uint64_t stm32l4x5_usart_base_read(void *opaque, hwaddr addr,
> unsigned int size)
> {
> Stm32l4x5UsartBaseState *s = opaque;
> + hwaddr base = addr & ~0x3ULL;
> + unsigned int offset = addr & 0x3;
> uint64_t retvalue = 0;
>
> - switch (addr) {
> + switch (base) {
> case A_CR1:
> retvalue = s->cr1;
> break;
> @@ -451,6 +460,13 @@ static uint64_t stm32l4x5_usart_base_read(void *opaque, hwaddr addr,
> break;
> }
>
> + /* Adjust for partial access */
> + if (size == 1) {
> + retvalue = (retvalue >> (offset * 8)) & 0xFF;
> + } else if (size == 2) {
> + retvalue = (retvalue >> (offset * 8)) & 0xFFFF;
> + }
You can avoid the if():
retvalue = extract32(retvalue, offset * 8, size * 8);
> +
> trace_stm32l4x5_usart_read(addr, retvalue);
>
> return retvalue;
> @@ -460,55 +476,88 @@ static void stm32l4x5_usart_base_write(void *opaque, hwaddr addr,
> uint64_t val64, unsigned int size)
> {
> Stm32l4x5UsartBaseState *s = opaque;
> - const uint32_t value = val64;
> + hwaddr base = addr & ~0x3ULL;
> + unsigned int offset = addr & 0x3;
> + uint32_t value = (uint32_t)val64;
> +
> + /* Build mask for partial access */
> + uint32_t mask;
> + if (size == 4) {
> + mask = 0xFFFFFFFF;
> + } else if (size == 2) {
> + mask = 0xFFFF << (offset * 8);
> + } else if (size == 1) {
> + mask = 0xFF << (offset * 8);
> + } else {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported access size %u\n", __func__, size);
> + return;
> + }
> + value = (value << (offset * 8)) & mask;
>
> trace_stm32l4x5_usart_write(addr, value);
>
> - switch (addr) {
> - case A_CR1:
> - s->cr1 = value;
> + switch (base) {
> + case A_CR1: {
> + uint32_t old = s->cr1;
> + s->cr1 = (old & ~mask) | value;
If you write these as
s->cr1 = deposit32(s->cr1, value, offset * 8, size * 8);
then you don't need to calculate a mask or shift-and-mask
the value by hand.
thanks
-- PMM
On 9/15/26 06:58, Peter Maydell wrote:
> On Mon, 17 Aug 2026 at 06:03, Jacob Whitaker Abrams
> <jwhitakera@gmail.com> wrote:
>> Official STM32CubeL4 drivers use and require support for 16-bit writes to
>> UART registers for proper function.
> This is a breach of the datasheet (assuming I have the right one:
> RM0351 "STM32L47xxx, STM32L48xxx, STM32L49xxx and STM32L4Axxx
> advanced Arm®-based 32-bit MCUs"), which is pretty clear:
> "The peripheral registers have to be accessed by words (32 bits)".
>
> Still, if the official drivers are doing it then presumably the
> hardware actually does allow smaller accesses.
>
> Do the drivers do small accesses to the top parts of registers,
> or do they always use 4-aligned addresses but just sometimes
> do smaller width accesses ? If only the latter is needed, we
> can make the code simpler (reads return the full register value
> and the QEMU core code chops off the unneeded high parts, writes
> are "write as if zero-extended").
Yes it is a documentation error, we discussed this in 2024, Nicolas Fillon at STM wrote back to me "I see we are making 16 bit access read and write to these 16 bit registers in our library for both HAL and LL so this should be a documentation issue."
It appears specifically to affect the USART peripheral inside the chip, here is the official C struct they use in their source code, it contains uint16_t sized registers resulting in 16-bit memory assembly operations:
typedef struct
{
__IO uint32_t CR1; /*!< USART Control register 1, Address offset: 0x00 */
__IO uint32_t CR2; /*!< USART Control register 2, Address offset: 0x04 */
__IO uint32_t CR3; /*!< USART Control register 3, Address offset: 0x08 */
__IO uint32_t BRR; /*!< USART Baud rate register, Address offset: 0x0C */
__IO uint16_t GTPR; /*!< USART Guard time and prescaler register, Address offset: 0x10 */
uint16_t RESERVED2; /*!< Reserved, 0x12 */
__IO uint32_t RTOR; /*!< USART Receiver Time Out register, Address offset: 0x14 */
__IO uint16_t RQR; /*!< USART Request register, Address offset: 0x18 */
uint16_t RESERVED3; /*!< Reserved, 0x1A */
__IO uint32_t ISR; /*!< USART Interrupt and status register, Address offset: 0x1C */
__IO uint32_t ICR; /*!< USART Interrupt flag Clear register, Address offset: 0x20 */
__IO uint16_t RDR; /*!< USART Receive Data register, Address offset: 0x24 */
uint16_t RESERVED4; /*!< Reserved, 0x26 */
__IO uint16_t TDR; /*!< USART Transmit Data register, Address offset: 0x28 */
uint16_t RESERVED5; /*!< Reserved, 0x2A */
__IO uint32_t PRESC; /*!< USART Prescaler register, Address offset: 0x2C */
} USART_TypeDef;
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2540
>> Signed-off-by: Jacob Whitaker Abrams <satur9nine@gmail.com>
>> ---
>> hw/char/stm32l4x5_usart.c | 93 +++++++++++++++++++++++-------
>> tests/qtest/stm32l4x5_usart-test.c | 50 +++++++++++++++-
>> 2 files changed, 120 insertions(+), 23 deletions(-)
>>
>> diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
>> index dd1b099195..88c4a3b5a7 100644
>> --- a/hw/char/stm32l4x5_usart.c
>> +++ b/hw/char/stm32l4x5_usart.c
>> @@ -154,8 +154,15 @@ REG32(RDR, 0x24)
>> REG32(TDR, 0x28)
>> FIELD(TDR, TDR, 0, 9)
>>
>> +#define ISR_RESET_VALUE (0x020000C0)
>> +
>> static void stm32l4x5_update_isr(Stm32l4x5UsartBaseState *s)
>> {
>> + if (!(s->cr1 & R_CR1_UE_MASK)) {
>> + s->isr = ISR_RESET_VALUE;
>> + return;
>> + }
> This seems to be an unrelated change to adding 16-bit access handling ?
Fair point, I will remove that from the upcoming v2 patch.
>> +
>> if (s->cr1 & R_CR1_TE_MASK) {
>> s->isr |= R_ISR_TEACK_MASK;
>> } else {
>> @@ -404,9 +411,11 @@ static uint64_t stm32l4x5_usart_base_read(void *opaque, hwaddr addr,
>> unsigned int size)
>> {
>> Stm32l4x5UsartBaseState *s = opaque;
>> + hwaddr base = addr & ~0x3ULL;
>> + unsigned int offset = addr & 0x3;
>> uint64_t retvalue = 0;
>>
>> - switch (addr) {
>> + switch (base) {
>> case A_CR1:
>> retvalue = s->cr1;
>> break;
>> @@ -451,6 +460,13 @@ static uint64_t stm32l4x5_usart_base_read(void *opaque, hwaddr addr,
>> break;
>> }
>>
>> + /* Adjust for partial access */
>> + if (size == 1) {
>> + retvalue = (retvalue >> (offset * 8)) & 0xFF;
>> + } else if (size == 2) {
>> + retvalue = (retvalue >> (offset * 8)) & 0xFFFF;
>> + }
> You can avoid the if():
>
> retvalue = extract32(retvalue, offset * 8, size * 8);
OK I will do so.
>> +
>> trace_stm32l4x5_usart_read(addr, retvalue);
>>
>> return retvalue;
>> @@ -460,55 +476,88 @@ static void stm32l4x5_usart_base_write(void *opaque, hwaddr addr,
>> uint64_t val64, unsigned int size)
>> {
>> Stm32l4x5UsartBaseState *s = opaque;
>> - const uint32_t value = val64;
>> + hwaddr base = addr & ~0x3ULL;
>> + unsigned int offset = addr & 0x3;
>> + uint32_t value = (uint32_t)val64;
>> +
>> + /* Build mask for partial access */
>> + uint32_t mask;
>> + if (size == 4) {
>> + mask = 0xFFFFFFFF;
>> + } else if (size == 2) {
>> + mask = 0xFFFF << (offset * 8);
>> + } else if (size == 1) {
>> + mask = 0xFF << (offset * 8);
>> + } else {
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported access size %u\n", __func__, size);
>> + return;
>> + }
>> + value = (value << (offset * 8)) & mask;
>>
>> trace_stm32l4x5_usart_write(addr, value);
>>
>> - switch (addr) {
>> - case A_CR1:
>> - s->cr1 = value;
>> + switch (base) {
>> + case A_CR1: {
>> + uint32_t old = s->cr1;
>> + s->cr1 = (old & ~mask) | value;
> If you write these as
> s->cr1 = deposit32(s->cr1, value, offset * 8, size * 8);
>
> then you don't need to calculate a mask or shift-and-mask
> the value by hand.
OK I will.
Regards,
Jacob Abrams
On Mon, 17 Aug 2026 at 06:03, Jacob Whitaker Abrams <jwhitakera@gmail.com> wrote: > > Official STM32CubeL4 drivers use and require support for 16-bit writes to > UART registers for proper function. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2540 This is a bug that was closed in 2024... -- PMM
I started the effort back then but I never fully finished. This is to complete that work. Regards, Jacob On Mon, Sep 14, 2026, 02:23 Peter Maydell <peter.maydell@linaro.org> wrote: > On Mon, 17 Aug 2026 at 06:03, Jacob Whitaker Abrams > <jwhitakera@gmail.com> wrote: > > > > Official STM32CubeL4 drivers use and require support for 16-bit writes to > > UART registers for proper function. > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2540 > > This is a bug that was closed in 2024... > > -- PMM > Jacob
© 2016 - 2026 Red Hat, Inc.