Re: [PATCH v2] RISC-V: Clean up the Zicbom block size probing

Conor.Dooley
2022-09-02T09:55:54Z

On 01/09/2022 16:57, Heiko Stübner wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Am Freitag, 12. August 2022, 17:40:10 CEST schrieb Palmer Dabbelt:
>> This fixes two issues: I truncated the warning's hart ID when porting to
>> the 64-bit hart ID code, and the original code's warning handling could
>> fire on an uninitialized hart ID.
>>
>> The biggest change here is that riscv_cbom_block_size is no longer
>> initialized, as IMO the default isn't sane: there's nothing in the ISA
>> that mandates any specific cache block size, so falling back to one will
>> just silently produce the wrong answer on some systems. This also
>> changes the probing order so the cache block size is known before
>> enabling Zicbom support.
>>
>> Fixes: 3aefb2ee5bdd ("riscv: implement Zicbom-based CMO instructions + the t-head variant")
>> Fixes: 1631ba1259d6 ("riscv: Add support for non-coherent devices using zicbom extension")
>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
>
> With Conor's fixes folded in, this compiles and breaks the T-Head CMOs
> as they rely on that default value :-) .
>
> Can we do the following:
>
> (1) pick Anup's patch moving the block-size init over to cacheflush [0]
> (2) apply this patch (with Conor's fixes and adapted to the changed
> location) and add this one additional line:
>
> diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> index d4b1526538ad..67fa078f303f 100644
> --- a/arch/riscv/errata/thead/errata.c
> +++ b/arch/riscv/errata/thead/errata.c
> @@ -42,6 +42,7 @@ static bool errata_probe_cmo(unsigned int stage,
> if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> return false;
>
> + riscv_cbom_block_size = L1_CACHE_BYTES;
> riscv_noncoherent_supported();
> return true;
> }
>
>
> With that done everything works (again) and looks great, so would be
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Tested-by: Heiko Stuebner <heiko@sntech.de>

With all of the above, it'd also be:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

>
>
> Heiko
>
> [0] https://lore.kernel.org/r/20220830044642.566769-3-apatel@ventanamicro.com