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

[Alexandre Ghiti]

On 14/10/2024 14:30, yunhui cui wrote:
> Hi Alex,
>
> On Mon, Oct 14, 2024 at 8:12 PM Alexandre Ghiti <alex@xxxxxxxx> 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@xxxxxxxxxxxxx>
> > > ---
> > >    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@xxxxxxxxxxxx>
> >
> > 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