linux-kernel - Re: [External] Re: [PATCH] RISC-V: ACPI: fix early_ioremap to early

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c05e09d0-d996-4712-a6e5-ef77f1f2ff0a@ghiti.fr>
Date: Mon, 14 Oct 2024 15:01:39 +0200
From: Alexandre Ghiti <alex@...ti.fr>
To: yunhui cui <cuiyunhui@...edance.com>
Cc: punit.agrawal@...edance.com, paul.walmsley@...ive.com,
 palmer@...belt.com, aou@...s.berkeley.edu, sunilvl@...tanamicro.com,
 ajones@...tanamicro.com, alexghiti@...osinc.com,
 jeeheng.sia@...rfivetech.com, haibo1.xu@...el.com,
 linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [External] Re: [PATCH] RISC-V: ACPI: fix early_ioremap to
 early_memremap

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