[PATCH] hw/loongarch/virt: Permit bytes/half access to IOCSR

Yao Zi posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251221122205.56463-2-me@ziyao.cc
Maintainers: Song Gao <gaosong@loongson.cn>, Bibo Mao <maobibo@loongson.cn>, Jiaxun Yang <jiaxun.yang@flygoat.com>
hw/loongarch/virt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] hw/loongarch/virt: Permit bytes/half access to IOCSR
Posted by Yao Zi 1 month, 2 weeks ago
IOCSRs could be accessed in any sizes from 1 to 8 bytes as long as the
address is aligned, regardless whether through MMIO or iocsr{rd,wr}
instructions. Lower min_access_size to 1 byte for IOCSR memory region to
match real-hardware behavior.

Fixes: f84a2aacf5d1 ("target/loongarch: Add LoongArch IOCSR instruction")
Signed-off-by: Yao Zi <me@ziyao.cc>
---
 hw/loongarch/virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 49434ad1828b..5cc57e9b5aa7 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -692,7 +692,7 @@ static const MemoryRegionOps virt_iocsr_misc_ops = {
     .write_with_attrs = virt_iocsr_misc_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
-        .min_access_size = 4,
+        .min_access_size = 1,
         .max_access_size = 8,
     },
     .impl = {
-- 
2.51.2
Re: [PATCH] hw/loongarch/virt: Permit bytes/half access to IOCSR
Posted by Bibo Mao 1 month, 2 weeks ago

On 2025/12/21 下午8:22, Yao Zi wrote:
> IOCSRs could be accessed in any sizes from 1 to 8 bytes as long as the
> address is aligned, regardless whether through MMIO or iocsr{rd,wr}
> instructions. Lower min_access_size to 1 byte for IOCSR memory region to
> match real-hardware behavior.
Hi Yao,

What is the detailed problem you encountered? Or just look through code 
and think that it should be so.

IOCSR supports 1/2/4/8 byte access like MMIO, however here is IOCSR MISC 
device rather than IOCSR bus emulation. What is the usage and scenery to 
read IOCSR MISC device with one byte?

It is similar with other device emulation with MMIO, such as e1000e with 
4 bytes aligned rather than any byte:
static const MemoryRegionOps mmio_ops = {
     .read = e1000e_mmio_read,
     .write = e1000e_mmio_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .impl = {
         .min_access_size = 4,
         .max_access_size = 4,
     },
};


Regards
Bibo Mao
> 
> Fixes: f84a2aacf5d1 ("target/loongarch: Add LoongArch IOCSR instruction")
> Signed-off-by: Yao Zi <me@ziyao.cc>
> ---
>   hw/loongarch/virt.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 49434ad1828b..5cc57e9b5aa7 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -692,7 +692,7 @@ static const MemoryRegionOps virt_iocsr_misc_ops = {
>       .write_with_attrs = virt_iocsr_misc_write,
>       .endianness = DEVICE_LITTLE_ENDIAN,
>       .valid = {
> -        .min_access_size = 4,
> +        .min_access_size = 1,
>           .max_access_size = 8,
>       },
>       .impl = {
> 


Re: [PATCH] hw/loongarch/virt: Permit bytes/half access to IOCSR
Posted by Yao Zi 1 month, 2 weeks ago
On Mon, Dec 22, 2025 at 11:24:38AM +0800, Bibo Mao wrote:
> 
> 
> On 2025/12/21 下午8:22, Yao Zi wrote:
> > IOCSRs could be accessed in any sizes from 1 to 8 bytes as long as the
> > address is aligned, regardless whether through MMIO or iocsr{rd,wr}
> > instructions. Lower min_access_size to 1 byte for IOCSR memory region to
> > match real-hardware behavior.
> Hi Yao,
> 
> What is the detailed problem you encountered? Or just look through code and
> think that it should be so.

I don't think there's a real use-case for this. However, without the
patch, the behavior of iocsrrd.b differs between real hardware and QEMU,
you could try this diff with Linux kernel for comparing.

diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
index 25a87378e48e..679e311ac654 100644
--- a/arch/loongarch/kernel/setup.c
+++ b/arch/loongarch/kernel/setup.c
@@ -621,3 +621,13 @@ void __init setup_arch(char **cmdline_p)
 	kasan_init();
 #endif
 }
+
+static int __init read_iocsr(void)
+{
+	pr_info("%s: iocsrrd_b(0x10) = 0x%x\n", __func__,
+		__iocsrrd_b(0x10));
+
+	return 0;
+}
+
+late_initcall(read_iocsr);

On QEMU, the error raised by address_space_ldub is silently ignored by
helper_iocsrrd_b(), and thus __iocsrrd_b(0x10) results in zero, while on
real hardware it doesn't.

Ignoring the error returned by address_space_{ld,st}* in helper_iocsr*
could cause more behaviors inconsistent with real LoongArch hardware.
But in the case shown in the diff that a single byte is read from iocsr,
the access shouldn't fail at all, which this patch tries to fix.

Regards,
Yao Zi

> IOCSR supports 1/2/4/8 byte access like MMIO, however here is IOCSR MISC
> device rather than IOCSR bus emulation. What is the usage and scenery to
> read IOCSR MISC device with one byte?
> 
> It is similar with other device emulation with MMIO, such as e1000e with 4
> bytes aligned rather than any byte:
> static const MemoryRegionOps mmio_ops = {
>     .read = e1000e_mmio_read,
>     .write = e1000e_mmio_write,
>     .endianness = DEVICE_LITTLE_ENDIAN,
>     .impl = {
>         .min_access_size = 4,
>         .max_access_size = 4,
>     },
> };
> 
> 
> Regards
> Bibo Mao

Re: [PATCH] hw/loongarch/virt: Permit bytes/half access to IOCSR
Posted by Bibo Mao 1 month, 2 weeks ago

On 2025/12/22 下午3:12, Yao Zi wrote:
> On Mon, Dec 22, 2025 at 11:24:38AM +0800, Bibo Mao wrote:
>>
>>
>> On 2025/12/21 下午8:22, Yao Zi wrote:
>>> IOCSRs could be accessed in any sizes from 1 to 8 bytes as long as the
>>> address is aligned, regardless whether through MMIO or iocsr{rd,wr}
>>> instructions. Lower min_access_size to 1 byte for IOCSR memory region to
>>> match real-hardware behavior.
>> Hi Yao,
>>
>> What is the detailed problem you encountered? Or just look through code and
>> think that it should be so.
> 
> I don't think there's a real use-case for this. However, without the
> patch, the behavior of iocsrrd.b differs between real hardware and QEMU,
> you could try this diff with Linux kernel for comparing.
> 
> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> index 25a87378e48e..679e311ac654 100644
> --- a/arch/loongarch/kernel/setup.c
> +++ b/arch/loongarch/kernel/setup.c
> @@ -621,3 +621,13 @@ void __init setup_arch(char **cmdline_p)
>   	kasan_init();
>   #endif
>   }
> +
> +static int __init read_iocsr(void)
> +{
> +	pr_info("%s: iocsrrd_b(0x10) = 0x%x\n", __func__,
> +		__iocsrrd_b(0x10));
> +
> +	return 0;
> +}
> +
> +late_initcall(read_iocsr);
> 
> On QEMU, the error raised by address_space_ldub is silently ignored by
> helper_iocsrrd_b(), and thus __iocsrrd_b(0x10) results in zero, while on
> real hardware it doesn't.
In general we should write code based on user manual rather than 
experience, is that right?

Also you can try readb/writeb API with generic PCI devices and check 
whether the emulated PCI device is the same with real device.

Regards
Bibo Mao
> 
> Ignoring the error returned by address_space_{ld,st}* in helper_iocsr*
> could cause more behaviors inconsistent with real LoongArch hardware.
> But in the case shown in the diff that a single byte is read from iocsr,
> the access shouldn't fail at all, which this patch tries to fix.
> 
> Regards,
> Yao Zi
> 
>> IOCSR supports 1/2/4/8 byte access like MMIO, however here is IOCSR MISC
>> device rather than IOCSR bus emulation. What is the usage and scenery to
>> read IOCSR MISC device with one byte?
>>
>> It is similar with other device emulation with MMIO, such as e1000e with 4
>> bytes aligned rather than any byte:
>> static const MemoryRegionOps mmio_ops = {
>>      .read = e1000e_mmio_read,
>>      .write = e1000e_mmio_write,
>>      .endianness = DEVICE_LITTLE_ENDIAN,
>>      .impl = {
>>          .min_access_size = 4,
>>          .max_access_size = 4,
>>      },
>> };
>>
>>
>> Regards
>> Bibo Mao