1. Patchew
  2. linux
  3. View series

[PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap

Yunhui Cui posted 1 patch 1 month, 1 week ago
There is a newer version of this series
arch/riscv/kernel/acpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap
Posted by Yunhui Cui 1 month, 1 week ago 
When SVPBMT is enabled, __acpi_map_table() will directly access the
data in DDR through the IO attribute, rather than through hardware
cache consistency, resulting in incorrect data in the obtained ACPI
table.

The log: ACPI: [ACPI:0x18] Invalid zero length.

We do not assume whether the bootloader flushes or not. We should
access in a cacheable way instead of maintaining cache consistency
by software.

Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
---
 arch/riscv/kernel/acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
index 6e0d333f57e5..3177c9af8764 100644
--- a/arch/riscv/kernel/acpi.c
+++ b/arch/riscv/kernel/acpi.c
@@ -210,7 +210,7 @@ void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
 	if (!size)
 		return NULL;
 
-	return early_ioremap(phys, size);
+	return early_memremap(phys, size);
 }
 
 void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
-- 
2.39.2
Re: [PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap
Posted by Alexandre Ghiti 1 month, 1 week ago 
Hi Yunhui,

On 14/10/2024 11:47, Yunhui Cui wrote:
> When SVPBMT is enabled, __acpi_map_table() will directly access the
> data in DDR through the IO attribute, rather than through hardware
> cache consistency, resulting in incorrect data in the obtained ACPI
> table.
>
> The log: ACPI: [ACPI:0x18] Invalid zero length.
>
> We do not assume whether the bootloader flushes or not. We should
> access in a cacheable way instead of maintaining cache consistency
> by software.
>
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>   arch/riscv/kernel/acpi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
> index 6e0d333f57e5..3177c9af8764 100644
> --- a/arch/riscv/kernel/acpi.c
> +++ b/arch/riscv/kernel/acpi.c
> @@ -210,7 +210,7 @@ void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
>   	if (!size)
>   		return NULL;
>   
> -	return early_ioremap(phys, size);
> +	return early_memremap(phys, size);
>   }
>   
>   void __init __acpi_unmap_table(void __iomem *map, unsigned long size)


It makes sense to me since with this, we don't have to care about how 
the firmware mapped the table. And it mimics all other architectures 
(arm64, loongarch and x86).

Here is the corresponding fixes tag:

Fixes: 3b426d4b5b14 ("RISC-V: ACPI : Fix for usage of pointers in 
different address space")

With the corresponding fix in __acpi_unmap_table() as pointed by Sunil, 
you can add:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

And regarding the sparse error, I don't see any other architecture 
casting to __iomem, so maybe that's not necessary anymore?

Thanks,

Alex
Re: [External] Re: [PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap
Posted by yunhui cui 1 month, 1 week ago 
Hi Alex,

On Mon, Oct 14, 2024 at 8:12 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> Hi Yunhui,
>
> On 14/10/2024 11:47, Yunhui Cui wrote:
> > When SVPBMT is enabled, __acpi_map_table() will directly access the
> > data in DDR through the IO attribute, rather than through hardware
> > cache consistency, resulting in incorrect data in the obtained ACPI
> > table.
> >
> > The log: ACPI: [ACPI:0x18] Invalid zero length.
> >
> > We do not assume whether the bootloader flushes or not. We should
> > access in a cacheable way instead of maintaining cache consistency
> > by software.
> >
> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > ---
> >   arch/riscv/kernel/acpi.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
> > index 6e0d333f57e5..3177c9af8764 100644
> > --- a/arch/riscv/kernel/acpi.c
> > +++ b/arch/riscv/kernel/acpi.c
> > @@ -210,7 +210,7 @@ void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
> >       if (!size)
> >               return NULL;
> >
> > -     return early_ioremap(phys, size);
> > +     return early_memremap(phys, size);
> >   }
> >
> >   void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
>
>
> It makes sense to me since with this, we don't have to care about how
> the firmware mapped the table. And it mimics all other architectures
> (arm64, loongarch and x86).
>
> Here is the corresponding fixes tag:
>
> Fixes: 3b426d4b5b14 ("RISC-V: ACPI : Fix for usage of pointers in
> different address space")
>
> With the corresponding fix in __acpi_unmap_table() as pointed by Sunil,
> you can add:
>
> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>
> And regarding the sparse error, I don't see any other architecture
> casting to __iomem, so maybe that's not necessary anymore?

OK. I will make the changes in v2. Regarding the sparse error, I will
use another patch specifically to solve it. Is that okay?

>
> Thanks,
>
> Alex
>
>

Thanks,
Yunhui
Re: [External] Re: [PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap
Posted by Alexandre Ghiti 1 month, 1 week ago 
On 14/10/2024 14:30, yunhui cui wrote:
> Hi Alex,
>
> On Mon, Oct 14, 2024 at 8:12 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
>> Hi Yunhui,
>>
>> On 14/10/2024 11:47, Yunhui Cui wrote:
>>> When SVPBMT is enabled, __acpi_map_table() will directly access the
>>> data in DDR through the IO attribute, rather than through hardware
>>> cache consistency, resulting in incorrect data in the obtained ACPI
>>> table.
>>>
>>> The log: ACPI: [ACPI:0x18] Invalid zero length.
>>>
>>> We do not assume whether the bootloader flushes or not. We should
>>> access in a cacheable way instead of maintaining cache consistency
>>> by software.
>>>
>>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
>>> ---
>>>    arch/riscv/kernel/acpi.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
>>> index 6e0d333f57e5..3177c9af8764 100644
>>> --- a/arch/riscv/kernel/acpi.c
>>> +++ b/arch/riscv/kernel/acpi.c
>>> @@ -210,7 +210,7 @@ void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
>>>        if (!size)
>>>                return NULL;
>>>
>>> -     return early_ioremap(phys, size);
>>> +     return early_memremap(phys, size);
>>>    }
>>>
>>>    void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
>>
>> It makes sense to me since with this, we don't have to care about how
>> the firmware mapped the table. And it mimics all other architectures
>> (arm64, loongarch and x86).
>>
>> Here is the corresponding fixes tag:
>>
>> Fixes: 3b426d4b5b14 ("RISC-V: ACPI : Fix for usage of pointers in
>> different address space")
>>
>> With the corresponding fix in __acpi_unmap_table() as pointed by Sunil,
>> you can add:
>>
>> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>>
>> And regarding the sparse error, I don't see any other architecture
>> casting to __iomem, so maybe that's not necessary anymore?
> OK. I will make the changes in v2. Regarding the sparse error, I will
> use another patch specifically to solve it. Is that okay?


If the second patch only consists in casting, I would not use another 
patch since the patch 2 would fix something introduced in patch 1.

But if patch 2 is more complicated,  it may make sense to do as you 
suggest, the goal is to merge patch 1 asap.

Thanks!

Alex


>
>> Thanks,
>>
>> Alex
>>
>>
> Thanks,
> Yunhui
Re: [External] Re: [PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap
Posted by yunhui cui 1 month ago 
Hi Alex,

On Mon, Oct 14, 2024 at 9:01 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> On 14/10/2024 14:30, yunhui cui wrote:
> > Hi Alex,
> >
> > On Mon, Oct 14, 2024 at 8:12 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> >> Hi Yunhui,
> >>
> >> On 14/10/2024 11:47, Yunhui Cui wrote:
> >>> When SVPBMT is enabled, __acpi_map_table() will directly access the
> >>> data in DDR through the IO attribute, rather than through hardware
> >>> cache consistency, resulting in incorrect data in the obtained ACPI
> >>> table.
> >>>
> >>> The log: ACPI: [ACPI:0x18] Invalid zero length.
> >>>
> >>> We do not assume whether the bootloader flushes or not. We should
> >>> access in a cacheable way instead of maintaining cache consistency
> >>> by software.
> >>>
> >>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> >>> ---
> >>>    arch/riscv/kernel/acpi.c | 2 +-
> >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
> >>> index 6e0d333f57e5..3177c9af8764 100644
> >>> --- a/arch/riscv/kernel/acpi.c
> >>> +++ b/arch/riscv/kernel/acpi.c
> >>> @@ -210,7 +210,7 @@ void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
> >>>        if (!size)
> >>>                return NULL;
> >>>
> >>> -     return early_ioremap(phys, size);
> >>> +     return early_memremap(phys, size);
> >>>    }
> >>>
> >>>    void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
> >>
> >> It makes sense to me since with this, we don't have to care about how
> >> the firmware mapped the table. And it mimics all other architectures
> >> (arm64, loongarch and x86).
> >>
> >> Here is the corresponding fixes tag:
> >>
> >> Fixes: 3b426d4b5b14 ("RISC-V: ACPI : Fix for usage of pointers in
> >> different address space")
> >>
> >> With the corresponding fix in __acpi_unmap_table() as pointed by Sunil,
> >> you can add:
> >>
> >> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> >>
> >> And regarding the sparse error, I don't see any other architecture
> >> casting to __iomem, so maybe that's not necessary anymore?
> > OK. I will make the changes in v2. Regarding the sparse error, I will
> > use another patch specifically to solve it. Is that okay?
>
>
> If the second patch only consists in casting, I would not use another
> patch since the patch 2 would fix something introduced in patch 1.
>
> But if patch 2 is more complicated,  it may make sense to do as you
> suggest, the goal is to merge patch 1 asap.

Regarding patch 2, I executed "make & make install" for sparse of the
riscv arch. I "git cloned" from
https://github.com/ConchuOD/sparse.git.
Then when compiling the kernel, the following error was reported:

cd /data00/cyh/linux-next/
make defconfig
cp .config build-riscv64/

make O=build-riscv64/ C=1 W=1 arch/riscv/kernel/ drivers/acpi/
drivers/mailbox/ -j
make[1]: Entering directory '/data00/cyh/linux-next/build-riscv64'
  GEN     Makefile
  CC      scripts/mod/empty.o
  CC      scripts/mod/devicetable-offsets.s
  CHECK   ../scripts/mod/empty.c
invalid argument to '-march': '_zicsr_zifencei'

make[3]: *** [../scripts/Makefile.build:229: scripts/mod/empty.o] Error 1
make[3]: *** Deleting file 'scripts/mod/empty.o'
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [/data00/cyh/linux-next/Makefile:1215: prepare0] Error 2
make[1]: *** [/data00/cyh/linux-next/Makefile:224: __sub-make] Error 2

Do you know the solution?

>
> Thanks!
>
> Alex
>
>
> >
> >> Thanks,
> >>
> >> Alex
> >>
> >>
> > Thanks,
> > Yunhui

Thanks,
Yunhui
Re: [External] Re: [PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap
Posted by Alexandre Ghiti 4 weeks, 1 day ago 
Hi Yunhui,

Sorry for the late reply, I was on vacation last week.

On Mon, Oct 21, 2024 at 8:45 AM yunhui cui <cuiyunhui@bytedance.com> wrote:
>
> Hi Alex,
>
> On Mon, Oct 14, 2024 at 9:01 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> >
> > On 14/10/2024 14:30, yunhui cui wrote:
> > > Hi Alex,
> > >
> > > On Mon, Oct 14, 2024 at 8:12 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> > >> Hi Yunhui,
> > >>
> > >> On 14/10/2024 11:47, Yunhui Cui wrote:
> > >>> When SVPBMT is enabled, __acpi_map_table() will directly access the
> > >>> data in DDR through the IO attribute, rather than through hardware
> > >>> cache consistency, resulting in incorrect data in the obtained ACPI
> > >>> table.
> > >>>
> > >>> The log: ACPI: [ACPI:0x18] Invalid zero length.
> > >>>
> > >>> We do not assume whether the bootloader flushes or not. We should
> > >>> access in a cacheable way instead of maintaining cache consistency
> > >>> by software.
> > >>>
> > >>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > >>> ---
> > >>>    arch/riscv/kernel/acpi.c | 2 +-
> > >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
> > >>> index 6e0d333f57e5..3177c9af8764 100644
> > >>> --- a/arch/riscv/kernel/acpi.c
> > >>> +++ b/arch/riscv/kernel/acpi.c
> > >>> @@ -210,7 +210,7 @@ void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
> > >>>        if (!size)
> > >>>                return NULL;
> > >>>
> > >>> -     return early_ioremap(phys, size);
> > >>> +     return early_memremap(phys, size);
> > >>>    }
> > >>>
> > >>>    void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
> > >>
> > >> It makes sense to me since with this, we don't have to care about how
> > >> the firmware mapped the table. And it mimics all other architectures
> > >> (arm64, loongarch and x86).
> > >>
> > >> Here is the corresponding fixes tag:
> > >>
> > >> Fixes: 3b426d4b5b14 ("RISC-V: ACPI : Fix for usage of pointers in
> > >> different address space")
> > >>
> > >> With the corresponding fix in __acpi_unmap_table() as pointed by Sunil,
> > >> you can add:
> > >>
> > >> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > >>
> > >> And regarding the sparse error, I don't see any other architecture
> > >> casting to __iomem, so maybe that's not necessary anymore?
> > > OK. I will make the changes in v2. Regarding the sparse error, I will
> > > use another patch specifically to solve it. Is that okay?
> >
> >
> > If the second patch only consists in casting, I would not use another
> > patch since the patch 2 would fix something introduced in patch 1.
> >
> > But if patch 2 is more complicated,  it may make sense to do as you
> > suggest, the goal is to merge patch 1 asap.
>
> Regarding patch 2, I executed "make & make install" for sparse of the
> riscv arch. I "git cloned" from
> https://github.com/ConchuOD/sparse.git.
> Then when compiling the kernel, the following error was reported:
>
> cd /data00/cyh/linux-next/
> make defconfig
> cp .config build-riscv64/
>
> make O=build-riscv64/ C=1 W=1 arch/riscv/kernel/ drivers/acpi/
> drivers/mailbox/ -j
> make[1]: Entering directory '/data00/cyh/linux-next/build-riscv64'
>   GEN     Makefile
>   CC      scripts/mod/empty.o
>   CC      scripts/mod/devicetable-offsets.s
>   CHECK   ../scripts/mod/empty.c
> invalid argument to '-march': '_zicsr_zifencei'
>
> make[3]: *** [../scripts/Makefile.build:229: scripts/mod/empty.o] Error 1
> make[3]: *** Deleting file 'scripts/mod/empty.o'
> make[3]: *** Waiting for unfinished jobs....
> make[2]: *** [/data00/cyh/linux-next/Makefile:1215: prepare0] Error 2
> make[1]: *** [/data00/cyh/linux-next/Makefile:224: __sub-make] Error 2
>
> Do you know the solution?

In case you did not find the solution yet, I would say that your
toolchain is too old.

Let me know if I can help,

Thanks,

Alex

>
> >
> > Thanks!
> >
> > Alex
> >
> >
> > >
> > >> Thanks,
> > >>
> > >> Alex
> > >>
> > >>
> > > Thanks,
> > > Yunhui
>
> Thanks,
> Yunhui
Re: [External] Re: [PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap
Posted by yunhui cui 4 weeks ago 
Hi Alex,


On Mon, Oct 28, 2024 at 9:37 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> Hi Yunhui,
>
> Sorry for the late reply, I was on vacation last week.
>
> On Mon, Oct 21, 2024 at 8:45 AM yunhui cui <cuiyunhui@bytedance.com> wrote:
> >
> > Hi Alex,
> >
> > On Mon, Oct 14, 2024 at 9:01 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> > >
> > > On 14/10/2024 14:30, yunhui cui wrote:
> > > > Hi Alex,
> > > >
> > > > On Mon, Oct 14, 2024 at 8:12 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> > > >> Hi Yunhui,
> > > >>
> > > >> On 14/10/2024 11:47, Yunhui Cui wrote:
> > > >>> When SVPBMT is enabled, __acpi_map_table() will directly access the
> > > >>> data in DDR through the IO attribute, rather than through hardware
> > > >>> cache consistency, resulting in incorrect data in the obtained ACPI
> > > >>> table.
> > > >>>
> > > >>> The log: ACPI: [ACPI:0x18] Invalid zero length.
> > > >>>
> > > >>> We do not assume whether the bootloader flushes or not. We should
> > > >>> access in a cacheable way instead of maintaining cache consistency
> > > >>> by software.
> > > >>>
> > > >>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > > >>> ---
> > > >>>    arch/riscv/kernel/acpi.c | 2 +-
> > > >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> > > >>>
> > > >>> diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
> > > >>> index 6e0d333f57e5..3177c9af8764 100644
> > > >>> --- a/arch/riscv/kernel/acpi.c
> > > >>> +++ b/arch/riscv/kernel/acpi.c
> > > >>> @@ -210,7 +210,7 @@ void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
> > > >>>        if (!size)
> > > >>>                return NULL;
> > > >>>
> > > >>> -     return early_ioremap(phys, size);
> > > >>> +     return early_memremap(phys, size);
> > > >>>    }
> > > >>>
> > > >>>    void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
> > > >>
> > > >> It makes sense to me since with this, we don't have to care about how
> > > >> the firmware mapped the table. And it mimics all other architectures
> > > >> (arm64, loongarch and x86).
> > > >>
> > > >> Here is the corresponding fixes tag:
> > > >>
> > > >> Fixes: 3b426d4b5b14 ("RISC-V: ACPI : Fix for usage of pointers in
> > > >> different address space")
> > > >>
> > > >> With the corresponding fix in __acpi_unmap_table() as pointed by Sunil,
> > > >> you can add:
> > > >>
> > > >> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > > >>
> > > >> And regarding the sparse error, I don't see any other architecture
> > > >> casting to __iomem, so maybe that's not necessary anymore?
> > > > OK. I will make the changes in v2. Regarding the sparse error, I will
> > > > use another patch specifically to solve it. Is that okay?
> > >
> > >
> > > If the second patch only consists in casting, I would not use another
> > > patch since the patch 2 would fix something introduced in patch 1.
> > >
> > > But if patch 2 is more complicated,  it may make sense to do as you
> > > suggest, the goal is to merge patch 1 asap.
> >
> > Regarding patch 2, I executed "make & make install" for sparse of the
> > riscv arch. I "git cloned" from
> > https://github.com/ConchuOD/sparse.git.
> > Then when compiling the kernel, the following error was reported:
> >
> > cd /data00/cyh/linux-next/
> > make defconfig
> > cp .config build-riscv64/
> >
> > make O=build-riscv64/ C=1 W=1 arch/riscv/kernel/ drivers/acpi/
> > drivers/mailbox/ -j
> > make[1]: Entering directory '/data00/cyh/linux-next/build-riscv64'
> >   GEN     Makefile
> >   CC      scripts/mod/empty.o
> >   CC      scripts/mod/devicetable-offsets.s
> >   CHECK   ../scripts/mod/empty.c
> > invalid argument to '-march': '_zicsr_zifencei'
> >
> > make[3]: *** [../scripts/Makefile.build:229: scripts/mod/empty.o] Error 1
> > make[3]: *** Deleting file 'scripts/mod/empty.o'
> > make[3]: *** Waiting for unfinished jobs....
> > make[2]: *** [/data00/cyh/linux-next/Makefile:1215: prepare0] Error 2
> > make[1]: *** [/data00/cyh/linux-next/Makefile:224: __sub-make] Error 2
> >
> > Do you know the solution?
>
> In case you did not find the solution yet, I would say that your
> toolchain is too old.
>
> Let me know if I can help,
It may not be a problem of an old compiler. The version of the
compiler I am using is 13.2.
It is a problem with sparse. When running "sparse --arch=riscv
-march=rv64imac_zicsr_zifencei scripts/mod/empty.c", there is an
invalid argument for '-march': '_zicsr_zifencei'.

>
> Thanks,
>
> Alex
>
> >
> > >
> > > Thanks!
> > >
> > > Alex
> > >
> > >
> > > >
> > > >> Thanks,
> > > >>
> > > >> Alex
> > > >>
> > > >>
> > > > Thanks,
> > > > Yunhui
> >
> > Thanks,
> > Yunhui

Thanks,
Yunhui
Re: [External] Re: [PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap
Posted by Sunil V L 4 weeks ago 
On Tue, Oct 29, 2024 at 11:04:49AM +0800, yunhui cui wrote:
> Hi Alex,
> 
> 
> On Mon, Oct 28, 2024 at 9:37 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> >
> > Hi Yunhui,
> >
> > Sorry for the late reply, I was on vacation last week.
> >
> > On Mon, Oct 21, 2024 at 8:45 AM yunhui cui <cuiyunhui@bytedance.com> wrote:
> > >
> > > Hi Alex,
> > >
> > > On Mon, Oct 14, 2024 at 9:01 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> > > >
> > > > On 14/10/2024 14:30, yunhui cui wrote:
> > > > > Hi Alex,
> > > > >
> > > > > On Mon, Oct 14, 2024 at 8:12 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> > > > >> Hi Yunhui,
> > > > >>
> > > > >> On 14/10/2024 11:47, Yunhui Cui wrote:
> > > > >>> When SVPBMT is enabled, __acpi_map_table() will directly access the
> > > > >>> data in DDR through the IO attribute, rather than through hardware
> > > > >>> cache consistency, resulting in incorrect data in the obtained ACPI
> > > > >>> table.
> > > > >>>
> > > > >>> The log: ACPI: [ACPI:0x18] Invalid zero length.
> > > > >>>
> > > > >>> We do not assume whether the bootloader flushes or not. We should
> > > > >>> access in a cacheable way instead of maintaining cache consistency
> > > > >>> by software.
> > > > >>>
> > > > >>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > > > >>> ---
> > > > >>>    arch/riscv/kernel/acpi.c | 2 +-
> > > > >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >>>
> > > > >>> diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
> > > > >>> index 6e0d333f57e5..3177c9af8764 100644
> > > > >>> --- a/arch/riscv/kernel/acpi.c
> > > > >>> +++ b/arch/riscv/kernel/acpi.c
> > > > >>> @@ -210,7 +210,7 @@ void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
> > > > >>>        if (!size)
> > > > >>>                return NULL;
> > > > >>>
> > > > >>> -     return early_ioremap(phys, size);
> > > > >>> +     return early_memremap(phys, size);
> > > > >>>    }
> > > > >>>
> > > > >>>    void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
> > > > >>
> > > > >> It makes sense to me since with this, we don't have to care about how
> > > > >> the firmware mapped the table. And it mimics all other architectures
> > > > >> (arm64, loongarch and x86).
> > > > >>
> > > > >> Here is the corresponding fixes tag:
> > > > >>
> > > > >> Fixes: 3b426d4b5b14 ("RISC-V: ACPI : Fix for usage of pointers in
> > > > >> different address space")
> > > > >>
> > > > >> With the corresponding fix in __acpi_unmap_table() as pointed by Sunil,
> > > > >> you can add:
> > > > >>
> > > > >> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > > > >>
> > > > >> And regarding the sparse error, I don't see any other architecture
> > > > >> casting to __iomem, so maybe that's not necessary anymore?
> > > > > OK. I will make the changes in v2. Regarding the sparse error, I will
> > > > > use another patch specifically to solve it. Is that okay?
> > > >
> > > >
> > > > If the second patch only consists in casting, I would not use another
> > > > patch since the patch 2 would fix something introduced in patch 1.
> > > >
> > > > But if patch 2 is more complicated,  it may make sense to do as you
> > > > suggest, the goal is to merge patch 1 asap.
> > >
> > > Regarding patch 2, I executed "make & make install" for sparse of the
> > > riscv arch. I "git cloned" from
> > > https://github.com/ConchuOD/sparse.git.
> > > Then when compiling the kernel, the following error was reported:
> > >
> > > cd /data00/cyh/linux-next/
> > > make defconfig
> > > cp .config build-riscv64/
> > >
> > > make O=build-riscv64/ C=1 W=1 arch/riscv/kernel/ drivers/acpi/
> > > drivers/mailbox/ -j
> > > make[1]: Entering directory '/data00/cyh/linux-next/build-riscv64'
> > >   GEN     Makefile
> > >   CC      scripts/mod/empty.o
> > >   CC      scripts/mod/devicetable-offsets.s
> > >   CHECK   ../scripts/mod/empty.c
> > > invalid argument to '-march': '_zicsr_zifencei'
> > >
> > > make[3]: *** [../scripts/Makefile.build:229: scripts/mod/empty.o] Error 1
> > > make[3]: *** Deleting file 'scripts/mod/empty.o'
> > > make[3]: *** Waiting for unfinished jobs....
> > > make[2]: *** [/data00/cyh/linux-next/Makefile:1215: prepare0] Error 2
> > > make[1]: *** [/data00/cyh/linux-next/Makefile:224: __sub-make] Error 2
> > >
> > > Do you know the solution?
> >
> > In case you did not find the solution yet, I would say that your
> > toolchain is too old.
> >
> > Let me know if I can help,
> It may not be a problem of an old compiler. The version of the
> compiler I am using is 13.2.
> It is a problem with sparse. When running "sparse --arch=riscv
> -march=rv64imac_zicsr_zifencei scripts/mod/empty.c", there is an
> invalid argument for '-march': '_zicsr_zifencei'.
> 
Hi Yunhui,

I think you need latest sparse. I remember building sparse from latest
sources to avoid this error.

https://www.kernel.org/doc/html/v4.11/dev-tools/sparse.html

Thanks,
Sunil
Re: [External] Re: [PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap
Posted by yunhui cui 3 weeks, 1 day ago 
Hi Sunil, Alex

On Tue, Oct 29, 2024 at 12:08 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> On Tue, Oct 29, 2024 at 11:04:49AM +0800, yunhui cui wrote:
> > Hi Alex,
> >
> >
> > On Mon, Oct 28, 2024 at 9:37 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> > >
> > > Hi Yunhui,
> > >
> > > Sorry for the late reply, I was on vacation last week.
> > >
> > > On Mon, Oct 21, 2024 at 8:45 AM yunhui cui <cuiyunhui@bytedance.com> wrote:
> > > >
> > > > Hi Alex,
> > > >
> > > > On Mon, Oct 14, 2024 at 9:01 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> > > > >
> > > > > On 14/10/2024 14:30, yunhui cui wrote:
> > > > > > Hi Alex,
> > > > > >
> > > > > > On Mon, Oct 14, 2024 at 8:12 PM Alexandre Ghiti <alex@ghiti.fr> wrote:
> > > > > >> Hi Yunhui,
> > > > > >>
> > > > > >> On 14/10/2024 11:47, Yunhui Cui wrote:
> > > > > >>> When SVPBMT is enabled, __acpi_map_table() will directly access the
> > > > > >>> data in DDR through the IO attribute, rather than through hardware
> > > > > >>> cache consistency, resulting in incorrect data in the obtained ACPI
> > > > > >>> table.
> > > > > >>>
> > > > > >>> The log: ACPI: [ACPI:0x18] Invalid zero length.
> > > > > >>>
> > > > > >>> We do not assume whether the bootloader flushes or not. We should
> > > > > >>> access in a cacheable way instead of maintaining cache consistency
> > > > > >>> by software.
> > > > > >>>
> > > > > >>> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > > > > >>> ---
> > > > > >>>    arch/riscv/kernel/acpi.c | 2 +-
> > > > > >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >>>
> > > > > >>> diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
> > > > > >>> index 6e0d333f57e5..3177c9af8764 100644
> > > > > >>> --- a/arch/riscv/kernel/acpi.c
> > > > > >>> +++ b/arch/riscv/kernel/acpi.c
> > > > > >>> @@ -210,7 +210,7 @@ void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
> > > > > >>>        if (!size)
> > > > > >>>                return NULL;
> > > > > >>>
> > > > > >>> -     return early_ioremap(phys, size);
> > > > > >>> +     return early_memremap(phys, size);
> > > > > >>>    }
> > > > > >>>
> > > > > >>>    void __init __acpi_unmap_table(void __iomem *map, unsigned long size)
> > > > > >>
> > > > > >> It makes sense to me since with this, we don't have to care about how
> > > > > >> the firmware mapped the table. And it mimics all other architectures
> > > > > >> (arm64, loongarch and x86).
> > > > > >>
> > > > > >> Here is the corresponding fixes tag:
> > > > > >>
> > > > > >> Fixes: 3b426d4b5b14 ("RISC-V: ACPI : Fix for usage of pointers in
> > > > > >> different address space")
> > > > > >>
> > > > > >> With the corresponding fix in __acpi_unmap_table() as pointed by Sunil,
> > > > > >> you can add:
> > > > > >>
> > > > > >> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > > > > >>
> > > > > >> And regarding the sparse error, I don't see any other architecture
> > > > > >> casting to __iomem, so maybe that's not necessary anymore?
> > > > > > OK. I will make the changes in v2. Regarding the sparse error, I will
> > > > > > use another patch specifically to solve it. Is that okay?
> > > > >
> > > > >
> > > > > If the second patch only consists in casting, I would not use another
> > > > > patch since the patch 2 would fix something introduced in patch 1.
> > > > >
> > > > > But if patch 2 is more complicated,  it may make sense to do as you
> > > > > suggest, the goal is to merge patch 1 asap.
> > > >
> > > > Regarding patch 2, I executed "make & make install" for sparse of the
> > > > riscv arch. I "git cloned" from
> > > > https://github.com/ConchuOD/sparse.git.
> > > > Then when compiling the kernel, the following error was reported:
> > > >
> > > > cd /data00/cyh/linux-next/
> > > > make defconfig
> > > > cp .config build-riscv64/
> > > >
> > > > make O=build-riscv64/ C=1 W=1 arch/riscv/kernel/ drivers/acpi/
> > > > drivers/mailbox/ -j
> > > > make[1]: Entering directory '/data00/cyh/linux-next/build-riscv64'
> > > >   GEN     Makefile
> > > >   CC      scripts/mod/empty.o
> > > >   CC      scripts/mod/devicetable-offsets.s
> > > >   CHECK   ../scripts/mod/empty.c
> > > > invalid argument to '-march': '_zicsr_zifencei'
> > > >
> > > > make[3]: *** [../scripts/Makefile.build:229: scripts/mod/empty.o] Error 1
> > > > make[3]: *** Deleting file 'scripts/mod/empty.o'
> > > > make[3]: *** Waiting for unfinished jobs....
> > > > make[2]: *** [/data00/cyh/linux-next/Makefile:1215: prepare0] Error 2
> > > > make[1]: *** [/data00/cyh/linux-next/Makefile:224: __sub-make] Error 2
> > > >
> > > > Do you know the solution?
> > >
> > > In case you did not find the solution yet, I would say that your
> > > toolchain is too old.
> > >
> > > Let me know if I can help,
> > It may not be a problem of an old compiler. The version of the
> > compiler I am using is 13.2.
> > It is a problem with sparse. When running "sparse --arch=riscv
> > -march=rv64imac_zicsr_zifencei scripts/mod/empty.c", there is an
> > invalid argument for '-march': '_zicsr_zifencei'.
> >
> Hi Yunhui,
>
> I think you need latest sparse. I remember building sparse from latest
> sources to avoid this error.
>
> https://www.kernel.org/doc/html/v4.11/dev-tools/sparse.html
>
> Thanks,
> Sunil

Thank you for your suggestions on sparse. The patch has been sent to
the community. Please refer to:
https://lore.kernel.org/linux-arm-kernel/20241031112030.72647-1-cuiyunhui@bytedance.com/T/.


Thanks,
Yunhui
Re: [PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap
Posted by Sunil V L 1 month, 1 week ago 
Hi Yunhui,
On Mon, Oct 14, 2024 at 05:47:05PM +0800, Yunhui Cui wrote:
> When SVPBMT is enabled, __acpi_map_table() will directly access the
> data in DDR through the IO attribute, rather than through hardware
> cache consistency, resulting in incorrect data in the obtained ACPI
> table.
> 
> The log: ACPI: [ACPI:0x18] Invalid zero length.
> 
> We do not assume whether the bootloader flushes or not. We should
> access in a cacheable way instead of maintaining cache consistency
> by software.
> 
> Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> ---
>  arch/riscv/kernel/acpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
> index 6e0d333f57e5..3177c9af8764 100644
> --- a/arch/riscv/kernel/acpi.c
> +++ b/arch/riscv/kernel/acpi.c
> @@ -210,7 +210,7 @@ void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
>  	if (!size)
>  		return NULL;
>  
> -	return early_ioremap(phys, size);
> +	return early_memremap(phys, size);
>  }
> 
I think __acpi_unmap_table() also needs similar change. You might need
to typecast to suppress the sparse error [1] then.

[1] - https://lore.kernel.org/oe-kbuild-all/202305201427.I7QhPjNW-lkp@intel.com/#r

Thanks,
Sunil
Re: [External] Re: [PATCH] RISC-V: ACPI: fix early_ioremap to early_memremap
Posted by yunhui cui 1 month, 1 week ago 
Hi Sunil,

On Mon, Oct 14, 2024 at 7:35 PM Sunil V L <sunilvl@ventanamicro.com> wrote:
>
> Hi Yunhui,
> On Mon, Oct 14, 2024 at 05:47:05PM +0800, Yunhui Cui wrote:
> > When SVPBMT is enabled, __acpi_map_table() will directly access the
> > data in DDR through the IO attribute, rather than through hardware
> > cache consistency, resulting in incorrect data in the obtained ACPI
> > table.
> >
> > The log: ACPI: [ACPI:0x18] Invalid zero length.
> >
> > We do not assume whether the bootloader flushes or not. We should
> > access in a cacheable way instead of maintaining cache consistency
> > by software.
> >
> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com>
> > ---
> >  arch/riscv/kernel/acpi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/kernel/acpi.c b/arch/riscv/kernel/acpi.c
> > index 6e0d333f57e5..3177c9af8764 100644
> > --- a/arch/riscv/kernel/acpi.c
> > +++ b/arch/riscv/kernel/acpi.c
> > @@ -210,7 +210,7 @@ void __init __iomem *__acpi_map_table(unsigned long phys, unsigned long size)
> >       if (!size)
> >               return NULL;
> >
> > -     return early_ioremap(phys, size);
> > +     return early_memremap(phys, size);
> >  }
> >
> I think __acpi_unmap_table() also needs similar change. You might need
> to typecast to suppress the sparse error [1] then.

OK. I will make the changes in v2. Regarding the sparse error, I will
use another patch specifically to solve it. Is that okay?

>
> [1] - https://lore.kernel.org/oe-kbuild-all/202305201427.I7QhPjNW-lkp@intel.com/#r
>
> Thanks,
> Sunil

Thanks,
Yunhui

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.