Re: [PATCH] target/riscv: rvzicbo: Fixup CBO extension register calculation

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 16/5/24 07:09, Alistair Francis wrote:
> On Tue, May 14, 2024 at 7:11 PM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>>
>>
>>
>> On 5/13/24 23:39, Alistair Francis wrote:
>>> When running the instruction
>>>
>>> ```
>>>       cbo.flush 0(x0)
>>> ```
>>>
>>> QEMU would segfault.
>>>
>>> The issue was in cpu_gpr[a->rs1] as QEMU does not have cpu_gpr[0]
>>> allocated.
>>>
>>> In order to fix this let's use the existing get_address()
>>> helper. This also has the benefit of performing pointer mask
>>> calculations on the address specified in rs1.
>>>
>>> The pointer masking specificiation specifically states:
>>>
>>> """
>>> Cache Management Operations: All instructions in Zicbom, Zicbop and Zicboz
>>> """
>>>
>>> So this is the correct behaviour and we previously have been incorrectly
>>> not masking the address.
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>>> Reported-by: Fabian Thomas <fabian.thomas@cispa.de>
>>> Fixes: e05da09b7cfd ("target/riscv: implement Zicbom extension")

Reported-by: Zhiwei Jiang (姜智伟) <jiangzw@tecorigin.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>>> ---
>>
>> LGTM but I wonder if this is the same fix as this one sent by Phil a month
>> ago or so:
>>
>> https://lore.kernel.org/qemu-riscv/20240419110514.69697-1-philmd@linaro.org/
>> ("[PATCH] target/riscv: Use get_address() to get address with Zicbom extensions")
> 
> It is the same fix!
> 
> I somehow missed that patch at the time. Sorry Philippe!
> 
> I'm going to merge this one as it includes the details about pointer
> masking, which I think is useful as that's why we are using
> get_address() instead of get_gpr()

Fine by me :)

> Alistair
> 
>>
>>
>> Thanks,
>>
>> Daniel
>>
>>>    target/riscv/insn_trans/trans_rvzicbo.c.inc | 16 ++++++++++++----
>>>    1 file changed, 12 insertions(+), 4 deletions(-)