All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hw/loongarch/virt: Permit bytes/half access to IOCSR
@ 2025-12-21 12:22 Yao Zi
  2025-12-22  3:24 ` Bibo Mao
  0 siblings, 1 reply; 4+ messages in thread
From: Yao Zi @ 2025-12-21 12:22 UTC (permalink / raw)
  To: Song Gao, Bibo Mao, Jiaxun Yang; +Cc: qemu-devel, Yao Zi

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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] hw/loongarch/virt: Permit bytes/half access to IOCSR
  2025-12-21 12:22 [PATCH] hw/loongarch/virt: Permit bytes/half access to IOCSR Yao Zi
@ 2025-12-22  3:24 ` Bibo Mao
  2025-12-22  7:12   ` Yao Zi
  0 siblings, 1 reply; 4+ messages in thread
From: Bibo Mao @ 2025-12-22  3:24 UTC (permalink / raw)
  To: Yao Zi, Song Gao, Jiaxun Yang; +Cc: qemu-devel



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 = {
> 



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] hw/loongarch/virt: Permit bytes/half access to IOCSR
  2025-12-22  3:24 ` Bibo Mao
@ 2025-12-22  7:12   ` Yao Zi
  2025-12-22  7:31     ` Bibo Mao
  0 siblings, 1 reply; 4+ messages in thread
From: Yao Zi @ 2025-12-22  7:12 UTC (permalink / raw)
  To: Bibo Mao, Song Gao, Jiaxun Yang; +Cc: qemu-devel

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] hw/loongarch/virt: Permit bytes/half access to IOCSR
  2025-12-22  7:12   ` Yao Zi
@ 2025-12-22  7:31     ` Bibo Mao
  0 siblings, 0 replies; 4+ messages in thread
From: Bibo Mao @ 2025-12-22  7:31 UTC (permalink / raw)
  To: Yao Zi, Song Gao, Jiaxun Yang; +Cc: qemu-devel



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



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-12-22  7:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-21 12:22 [PATCH] hw/loongarch/virt: Permit bytes/half access to IOCSR Yao Zi
2025-12-22  3:24 ` Bibo Mao
2025-12-22  7:12   ` Yao Zi
2025-12-22  7:31     ` Bibo Mao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.