* [PATCH] hw/loongarch/virt: Align VIRT_GED_CPUHP_ADDR to 4 bytes
@ 2025-09-22 14:15 Huacai Chen
2025-09-23 2:09 ` Bibo Mao
2025-09-23 3:38 ` Bibo Mao
0 siblings, 2 replies; 5+ messages in thread
From: Huacai Chen @ 2025-09-22 14:15 UTC (permalink / raw)
To: Bibo Mao, Song Gao
Cc: Jiaxun Yang, WANG Xuerui, qemu-devel, Huacai Chen,
Nathan Chancellor, WANG Rui
From: Huacai Chen <chenhuacai@loongson.cn>
Now VIRT_GED_CPUHP_ADDR is not aligned to 4 bytes, but if Linux kernel
is built with ACPI_MISALIGNMENT_NOT_SUPPORTED, it assumes the alignment,
otherwise we get ACPI errors at boot phase:
ACPI Error: AE_AML_ALIGNMENT, Returned by Handler for [SystemMemory] (20250404/evregion-301)
ACPI Error: Aborting method \_SB.CPUS.CSTA due to previous error (AE_AML_ALIGNMENT) (20250404/psparse-529)
ACPI Error: Aborting method \_SB.CPUS.C000._STA due to previous error (AE_AML_ALIGNMENT) (20250404/psparse-529)
ACPI Error: Method execution failed \_SB.CPUS.C000._STA due to previous error (AE_AML_ALIGNMENT) (20250404/uteval-68)
VIRT_GED_MEM_ADDR and VIRT_GED_REG_ADDR are already aligned now, but use
QEMU_ALIGN_UP() to explicitly align them can make code more robust.
Reported-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: WANG Rui <wangrui@loongson.cn>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
include/hw/loongarch/virt.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 602feab0f0..be4f5d603f 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -28,9 +28,9 @@
#define VIRT_LOWMEM_SIZE 0x10000000
#define VIRT_HIGHMEM_BASE 0x80000000
#define VIRT_GED_EVT_ADDR 0x100e0000
-#define VIRT_GED_MEM_ADDR (VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN)
-#define VIRT_GED_REG_ADDR (VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN)
-#define VIRT_GED_CPUHP_ADDR (VIRT_GED_REG_ADDR + ACPI_GED_REG_COUNT)
+#define VIRT_GED_MEM_ADDR QEMU_ALIGN_UP(VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN, 4)
+#define VIRT_GED_REG_ADDR QEMU_ALIGN_UP(VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN, 4)
+#define VIRT_GED_CPUHP_ADDR QEMU_ALIGN_UP(VIRT_GED_REG_ADDR + ACPI_GED_REG_COUNT, 4)
#define COMMAND_LINE_SIZE 512
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/loongarch/virt: Align VIRT_GED_CPUHP_ADDR to 4 bytes
2025-09-22 14:15 [PATCH] hw/loongarch/virt: Align VIRT_GED_CPUHP_ADDR to 4 bytes Huacai Chen
@ 2025-09-23 2:09 ` Bibo Mao
2025-09-23 3:38 ` Bibo Mao
1 sibling, 0 replies; 5+ messages in thread
From: Bibo Mao @ 2025-09-23 2:09 UTC (permalink / raw)
To: Huacai Chen, Song Gao
Cc: Jiaxun Yang, WANG Xuerui, qemu-devel, Huacai Chen,
Nathan Chancellor, WANG Rui
yes, it is actually one problem. And thanks for doing this.
Reviewed-by: Bibo Mao <maobibo@loongson.cn>
On 2025/9/22 下午10:15, Huacai Chen wrote:
> From: Huacai Chen <chenhuacai@loongson.cn>
>
> Now VIRT_GED_CPUHP_ADDR is not aligned to 4 bytes, but if Linux kernel
> is built with ACPI_MISALIGNMENT_NOT_SUPPORTED, it assumes the alignment,
> otherwise we get ACPI errors at boot phase:
>
> ACPI Error: AE_AML_ALIGNMENT, Returned by Handler for [SystemMemory] (20250404/evregion-301)
> ACPI Error: Aborting method \_SB.CPUS.CSTA due to previous error (AE_AML_ALIGNMENT) (20250404/psparse-529)
> ACPI Error: Aborting method \_SB.CPUS.C000._STA due to previous error (AE_AML_ALIGNMENT) (20250404/psparse-529)
> ACPI Error: Method execution failed \_SB.CPUS.C000._STA due to previous error (AE_AML_ALIGNMENT) (20250404/uteval-68)
>
> VIRT_GED_MEM_ADDR and VIRT_GED_REG_ADDR are already aligned now, but use
> QEMU_ALIGN_UP() to explicitly align them can make code more robust.
>
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: WANG Rui <wangrui@loongson.cn>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> include/hw/loongarch/virt.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> index 602feab0f0..be4f5d603f 100644
> --- a/include/hw/loongarch/virt.h
> +++ b/include/hw/loongarch/virt.h
> @@ -28,9 +28,9 @@
> #define VIRT_LOWMEM_SIZE 0x10000000
> #define VIRT_HIGHMEM_BASE 0x80000000
> #define VIRT_GED_EVT_ADDR 0x100e0000
> -#define VIRT_GED_MEM_ADDR (VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN)
> -#define VIRT_GED_REG_ADDR (VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN)
> -#define VIRT_GED_CPUHP_ADDR (VIRT_GED_REG_ADDR + ACPI_GED_REG_COUNT)
> +#define VIRT_GED_MEM_ADDR QEMU_ALIGN_UP(VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN, 4)
> +#define VIRT_GED_REG_ADDR QEMU_ALIGN_UP(VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN, 4)
> +#define VIRT_GED_CPUHP_ADDR QEMU_ALIGN_UP(VIRT_GED_REG_ADDR + ACPI_GED_REG_COUNT, 4)
>
> #define COMMAND_LINE_SIZE 512
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/loongarch/virt: Align VIRT_GED_CPUHP_ADDR to 4 bytes
2025-09-22 14:15 [PATCH] hw/loongarch/virt: Align VIRT_GED_CPUHP_ADDR to 4 bytes Huacai Chen
2025-09-23 2:09 ` Bibo Mao
@ 2025-09-23 3:38 ` Bibo Mao
2025-09-23 4:22 ` Huacai Chen
1 sibling, 1 reply; 5+ messages in thread
From: Bibo Mao @ 2025-09-23 3:38 UTC (permalink / raw)
To: Huacai Chen, Song Gao
Cc: Jiaxun Yang, WANG Xuerui, qemu-devel, Huacai Chen,
Nathan Chancellor, WANG Rui
Hi huacai,
It breaks with compatible issue since acpi table is changed, and test
case qtest-loongarch64/bios-tables-test fails to run.
LoongArch VM compatibility is not perfect now, one method is to modify
test case at the same time, another method is to add extra option in
order to support aligned GED ACPI address.
Does this issue must be fixed now? With ACPI spec, 5.2.12.20 Core
Programmable Interrupt Controller (CORE PIC) Structure, ACPI Processor
ID is not aligned also, its size is 4 byte and offset is 3 bytes.
If it must be fixed, the test case should be modified also together with
the patch. If not, it can be record as pending bug, will solve it if VM
compatibility method is decided.
Regards
Bibo Mao
On 2025/9/22 下午10:15, Huacai Chen wrote:
> From: Huacai Chen <chenhuacai@loongson.cn>
>
> Now VIRT_GED_CPUHP_ADDR is not aligned to 4 bytes, but if Linux kernel
> is built with ACPI_MISALIGNMENT_NOT_SUPPORTED, it assumes the alignment,
> otherwise we get ACPI errors at boot phase:
>
> ACPI Error: AE_AML_ALIGNMENT, Returned by Handler for [SystemMemory] (20250404/evregion-301)
> ACPI Error: Aborting method \_SB.CPUS.CSTA due to previous error (AE_AML_ALIGNMENT) (20250404/psparse-529)
> ACPI Error: Aborting method \_SB.CPUS.C000._STA due to previous error (AE_AML_ALIGNMENT) (20250404/psparse-529)
> ACPI Error: Method execution failed \_SB.CPUS.C000._STA due to previous error (AE_AML_ALIGNMENT) (20250404/uteval-68)
>
> VIRT_GED_MEM_ADDR and VIRT_GED_REG_ADDR are already aligned now, but use
> QEMU_ALIGN_UP() to explicitly align them can make code more robust.
>
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: WANG Rui <wangrui@loongson.cn>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> include/hw/loongarch/virt.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> index 602feab0f0..be4f5d603f 100644
> --- a/include/hw/loongarch/virt.h
> +++ b/include/hw/loongarch/virt.h
> @@ -28,9 +28,9 @@
> #define VIRT_LOWMEM_SIZE 0x10000000
> #define VIRT_HIGHMEM_BASE 0x80000000
> #define VIRT_GED_EVT_ADDR 0x100e0000
> -#define VIRT_GED_MEM_ADDR (VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN)
> -#define VIRT_GED_REG_ADDR (VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN)
> -#define VIRT_GED_CPUHP_ADDR (VIRT_GED_REG_ADDR + ACPI_GED_REG_COUNT)
> +#define VIRT_GED_MEM_ADDR QEMU_ALIGN_UP(VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN, 4)
> +#define VIRT_GED_REG_ADDR QEMU_ALIGN_UP(VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN, 4)
> +#define VIRT_GED_CPUHP_ADDR QEMU_ALIGN_UP(VIRT_GED_REG_ADDR + ACPI_GED_REG_COUNT, 4)
>
> #define COMMAND_LINE_SIZE 512
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/loongarch/virt: Align VIRT_GED_CPUHP_ADDR to 4 bytes
2025-09-23 3:38 ` Bibo Mao
@ 2025-09-23 4:22 ` Huacai Chen
2025-09-23 6:15 ` Bibo Mao
0 siblings, 1 reply; 5+ messages in thread
From: Huacai Chen @ 2025-09-23 4:22 UTC (permalink / raw)
To: Bibo Mao
Cc: Song Gao, Jiaxun Yang, WANG Xuerui, qemu-devel, Huacai Chen,
Nathan Chancellor, WANG Rui
Hi, Bibo,
On Tue, Sep 23, 2025 at 11:40 AM Bibo Mao <maobibo@loongson.cn> wrote:
>
> Hi huacai,
>
> It breaks with compatible issue since acpi table is changed, and test
> case qtest-loongarch64/bios-tables-test fails to run.
>
> LoongArch VM compatibility is not perfect now, one method is to modify
> test case at the same time, another method is to add extra option in
> order to support aligned GED ACPI address.
>
> Does this issue must be fixed now? With ACPI spec, 5.2.12.20 Core
> Programmable Interrupt Controller (CORE PIC) Structure, ACPI Processor
> ID is not aligned also, its size is 4 byte and offset is 3 bytes.
They are different.
ACPI tables are probably packed (so the members may not be aligned),
such as CORE PIC you mentioned. Linux kernel defines two kinds of
accessors to parse members. You can see the code in
drivers/acpi/acpica/acmacros.h from the kernel as an example.
The problem mentioned in this patch is the alignment of a struct as a
whole. If the struct itself isn't aligned, Linux kernel cannot handle
it on hardware without UAL, even with two kinds of accessors.
>
> If it must be fixed, the test case should be modified also together with
> the patch. If not, it can be record as pending bug, will solve it if VM
> compatibility method is decided.
Generally speaking, I think this should be fixed, because it is
allowed for qemu to emulate a machine without UAL. I will take a look
at qtest-loongarch64/bios-tables-test (maybe you means
tests/qtest/bios-tables-test.c?), but it seeams a separate patch is
better?
Huacai
>
> Regards
> Bibo Mao
>
> On 2025/9/22 下午10:15, Huacai Chen wrote:
> > From: Huacai Chen <chenhuacai@loongson.cn>
> >
> > Now VIRT_GED_CPUHP_ADDR is not aligned to 4 bytes, but if Linux kernel
> > is built with ACPI_MISALIGNMENT_NOT_SUPPORTED, it assumes the alignment,
> > otherwise we get ACPI errors at boot phase:
> >
> > ACPI Error: AE_AML_ALIGNMENT, Returned by Handler for [SystemMemory] (20250404/evregion-301)
> > ACPI Error: Aborting method \_SB.CPUS.CSTA due to previous error (AE_AML_ALIGNMENT) (20250404/psparse-529)
> > ACPI Error: Aborting method \_SB.CPUS.C000._STA due to previous error (AE_AML_ALIGNMENT) (20250404/psparse-529)
> > ACPI Error: Method execution failed \_SB.CPUS.C000._STA due to previous error (AE_AML_ALIGNMENT) (20250404/uteval-68)
> >
> > VIRT_GED_MEM_ADDR and VIRT_GED_REG_ADDR are already aligned now, but use
> > QEMU_ALIGN_UP() to explicitly align them can make code more robust.
> >
> > Reported-by: Nathan Chancellor <nathan@kernel.org>
> > Suggested-by: WANG Rui <wangrui@loongson.cn>
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > include/hw/loongarch/virt.h | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> > index 602feab0f0..be4f5d603f 100644
> > --- a/include/hw/loongarch/virt.h
> > +++ b/include/hw/loongarch/virt.h
> > @@ -28,9 +28,9 @@
> > #define VIRT_LOWMEM_SIZE 0x10000000
> > #define VIRT_HIGHMEM_BASE 0x80000000
> > #define VIRT_GED_EVT_ADDR 0x100e0000
> > -#define VIRT_GED_MEM_ADDR (VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN)
> > -#define VIRT_GED_REG_ADDR (VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN)
> > -#define VIRT_GED_CPUHP_ADDR (VIRT_GED_REG_ADDR + ACPI_GED_REG_COUNT)
> > +#define VIRT_GED_MEM_ADDR QEMU_ALIGN_UP(VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN, 4)
> > +#define VIRT_GED_REG_ADDR QEMU_ALIGN_UP(VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN, 4)
> > +#define VIRT_GED_CPUHP_ADDR QEMU_ALIGN_UP(VIRT_GED_REG_ADDR + ACPI_GED_REG_COUNT, 4)
> >
> > #define COMMAND_LINE_SIZE 512
> >
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/loongarch/virt: Align VIRT_GED_CPUHP_ADDR to 4 bytes
2025-09-23 4:22 ` Huacai Chen
@ 2025-09-23 6:15 ` Bibo Mao
0 siblings, 0 replies; 5+ messages in thread
From: Bibo Mao @ 2025-09-23 6:15 UTC (permalink / raw)
To: Huacai Chen
Cc: Song Gao, Jiaxun Yang, WANG Xuerui, qemu-devel, Huacai Chen,
Nathan Chancellor, WANG Rui
On 2025/9/23 下午12:22, Huacai Chen wrote:
> Hi, Bibo,
>
> On Tue, Sep 23, 2025 at 11:40 AM Bibo Mao <maobibo@loongson.cn> wrote:
>>
>> Hi huacai,
>>
>> It breaks with compatible issue since acpi table is changed, and test
>> case qtest-loongarch64/bios-tables-test fails to run.
>>
>> LoongArch VM compatibility is not perfect now, one method is to modify
>> test case at the same time, another method is to add extra option in
>> order to support aligned GED ACPI address.
>>
>> Does this issue must be fixed now? With ACPI spec, 5.2.12.20 Core
>> Programmable Interrupt Controller (CORE PIC) Structure, ACPI Processor
>> ID is not aligned also, its size is 4 byte and offset is 3 bytes.
> They are different.
>
> ACPI tables are probably packed (so the members may not be aligned),
> such as CORE PIC you mentioned. Linux kernel defines two kinds of
> accessors to parse members. You can see the code in
> drivers/acpi/acpica/acmacros.h from the kernel as an example.
>
> The problem mentioned in this patch is the alignment of a struct as a
> whole. If the struct itself isn't aligned, Linux kernel cannot handle
> it on hardware without UAL, even with two kinds of accessors.
>
>>
>> If it must be fixed, the test case should be modified also together with
>> the patch. If not, it can be record as pending bug, will solve it if VM
>> compatibility method is decided.
> Generally speaking, I think this should be fixed, because it is
> allowed for qemu to emulate a machine without UAL. I will take a look
ok, loongarch VM compatibility is not good now, maybe we can skip it
now. There will be compatibility problem every time new feature and
hardware is added.
> at qtest-loongarch64/bios-tables-test (maybe you means
> tests/qtest/bios-tables-test.c?), but it seeams a separate patch is
> better?
yeap, it is this test case, it fails with command "make check" or in
qemu CI. In general it should be in one patch set such as:
https://lists.nongnu.org/archive/html/qemu-riscv/2025-07/msg00155.html
Regards
Bibo Mao
>
> Huacai
>
>>
>> Regards
>> Bibo Mao
>>
>> On 2025/9/22 下午10:15, Huacai Chen wrote:
>>> From: Huacai Chen <chenhuacai@loongson.cn>
>>>
>>> Now VIRT_GED_CPUHP_ADDR is not aligned to 4 bytes, but if Linux kernel
>>> is built with ACPI_MISALIGNMENT_NOT_SUPPORTED, it assumes the alignment,
>>> otherwise we get ACPI errors at boot phase:
>>>
>>> ACPI Error: AE_AML_ALIGNMENT, Returned by Handler for [SystemMemory] (20250404/evregion-301)
>>> ACPI Error: Aborting method \_SB.CPUS.CSTA due to previous error (AE_AML_ALIGNMENT) (20250404/psparse-529)
>>> ACPI Error: Aborting method \_SB.CPUS.C000._STA due to previous error (AE_AML_ALIGNMENT) (20250404/psparse-529)
>>> ACPI Error: Method execution failed \_SB.CPUS.C000._STA due to previous error (AE_AML_ALIGNMENT) (20250404/uteval-68)
>>>
>>> VIRT_GED_MEM_ADDR and VIRT_GED_REG_ADDR are already aligned now, but use
>>> QEMU_ALIGN_UP() to explicitly align them can make code more robust.
>>>
>>> Reported-by: Nathan Chancellor <nathan@kernel.org>
>>> Suggested-by: WANG Rui <wangrui@loongson.cn>
>>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
>>> ---
>>> include/hw/loongarch/virt.h | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
>>> index 602feab0f0..be4f5d603f 100644
>>> --- a/include/hw/loongarch/virt.h
>>> +++ b/include/hw/loongarch/virt.h
>>> @@ -28,9 +28,9 @@
>>> #define VIRT_LOWMEM_SIZE 0x10000000
>>> #define VIRT_HIGHMEM_BASE 0x80000000
>>> #define VIRT_GED_EVT_ADDR 0x100e0000
>>> -#define VIRT_GED_MEM_ADDR (VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN)
>>> -#define VIRT_GED_REG_ADDR (VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN)
>>> -#define VIRT_GED_CPUHP_ADDR (VIRT_GED_REG_ADDR + ACPI_GED_REG_COUNT)
>>> +#define VIRT_GED_MEM_ADDR QEMU_ALIGN_UP(VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN, 4)
>>> +#define VIRT_GED_REG_ADDR QEMU_ALIGN_UP(VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN, 4)
>>> +#define VIRT_GED_CPUHP_ADDR QEMU_ALIGN_UP(VIRT_GED_REG_ADDR + ACPI_GED_REG_COUNT, 4)
>>>
>>> #define COMMAND_LINE_SIZE 512
>>>
>>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-09-23 6:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-22 14:15 [PATCH] hw/loongarch/virt: Align VIRT_GED_CPUHP_ADDR to 4 bytes Huacai Chen
2025-09-23 2:09 ` Bibo Mao
2025-09-23 3:38 ` Bibo Mao
2025-09-23 4:22 ` Huacai Chen
2025-09-23 6:15 ` Bibo Mao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).