[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 at 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 at 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 at 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