[PATCH 1/2] lib: utils: Implement "64bit-mmio" property parsing

[Jessica Clarke]

On 17 Apr 2021, at 14:32, Anup Patel <Anup.Patel at wdc.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Jessica Clarke <jrtc27 at jrtc27.com>
>> Sent: 17 April 2021 18:58
>> To: Anup Patel <Anup.Patel at wdc.com>
>> Cc: guoren at kernel.org; Alistair Francis <Alistair.Francis at wdc.com>; Anup
>> Patel <anup at brainfault.org>; OpenSBI <opensbi at lists.infradead.org>; Guo
>> Ren <guoren at linux.alibaba.com>; Xiang W <wxjstz at 126.com>
>> Subject: Re: [PATCH 1/2] lib: utils: Implement "64bit-mmio" property parsing
>> 
>> On 17 Apr 2021, at 14:26, Anup Patel <Anup.Patel at wdc.com> wrote:
>>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: Jessica Clarke <jrtc27 at jrtc27.com>
>>>> Sent: 17 April 2021 18:47
>>>> To: Anup Patel <Anup.Patel at wdc.com>
>>>> Cc: guoren at kernel.org; Alistair Francis <Alistair.Francis at wdc.com>;
>>>> Anup Patel <anup at brainfault.org>; OpenSBI
>>>> <opensbi at lists.infradead.org>; Guo Ren <guoren at linux.alibaba.com>;
>>>> Xiang W <wxjstz at 126.com>
>>>> Subject: Re: [PATCH 1/2] lib: utils: Implement "64bit-mmio" property
>>>> parsing
>>>> 
>>>> On 17 Apr 2021, at 14:16, Anup Patel <Anup.Patel at wdc.com> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>>> -----Original Message-----
>>>>>> From: Jessica Clarke <jrtc27 at jrtc27.com>
>>>>>> Sent: 17 April 2021 17:16
>>>>>> To: guoren at kernel.org
>>>>>> Cc: Alistair Francis <Alistair.Francis at wdc.com>; Anup Patel
>>>>>> <anup at brainfault.org>; OpenSBI <opensbi at lists.infradead.org>; Guo
>>>> Ren
>>>>>> <guoren at linux.alibaba.com>; Anup Patel <Anup.Patel at wdc.com>;
>> Xiang
>>>> W
>>>>>> <wxjstz at 126.com>
>>>>>> Subject: Re: [PATCH 1/2] lib: utils: Implement "64bit-mmio"
>>>>>> property parsing
>>>>>> 
>>>>>> On 17 Apr 2021, at 12:41, Jessica Clarke <jrtc27 at jrtc27.com> wrote:
>>>>>>> 
>>>>>>> On 13 Apr 2021, at 04:44, guoren at kernel.org wrote:
>>>>>>>> 
>>>>>>>> From: Guo Ren <guoren at linux.alibaba.com>
>>>>>>>> 
>>>>>>>> Figure out CLINT has_64bit_mmio from DT node and using antonym
>>>>>>>> for compatibility.
>>>>>>>> 
>>>>>>>> Signed-off-by: Guo Ren <guoren at linux.alibaba.com>
>>>>>>>> Cc: Anup Patel <anup.patel at wdc.com>
>>>>>>>> Cc: Xiang W <wxjstz at 126.com>
>>>>>>>> ---
>>>>>>>> lib/utils/fdt/fdt_helper.c | 3 ++-
>>>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>>> 
>>>>>>>> diff --git a/lib/utils/fdt/fdt_helper.c
>>>>>>>> b/lib/utils/fdt/fdt_helper.c index bf19ff9..909de01 100644
>>>>>>>> --- a/lib/utils/fdt/fdt_helper.c
>>>>>>>> +++ b/lib/utils/fdt/fdt_helper.c
>>>>>>>> @@ -442,8 +442,9 @@ int fdt_parse_clint_node(void *fdt, int
>>>>>> nodeoffset, bool for_timer,
>>>>>>>> 	if (clint->hart_count < count)
>>>>>>>> 		clint->hart_count = count;
>>>>>>>> 
>>>>>>>> -	/* TODO: We should figure-out CLINT has_64bit_mmio from
>> DT node
>>>>>> */
>>>>>>>> 	clint->has_64bit_mmio = TRUE;
>>>>>>>> +	if (fdt_getprop(fdt, nodeoffset, "clint,has-no-64bit-mmio",
>>>>>>>> +&count))
>>>>>>> 
>>>>>>> I do not see this specified in clint.yaml.
>>>>>> 
>>>>>> Also, the RISC-V privileged is very clear:
>>>>>> 
>>>>>>> For RV64, naturally aligned 64-bit memory accesses to the mtime
>>>>>>> and mtimecmp registers are atomic.
>>>>>> 
>>>>>> So I would say your CLINT violates the spec and thus should not
>>>>>> claim to be a RISC-V or SiFive-compatible CLINT.
>>>>> 
>>>>> There is no CLINT spec maintained by RISC-V international.
>>>>> 
>>>>> All implementation have SiFive compatible CLINT or some variant of it.
>>>> 
>>>> That was a quote from the ratified privileged spec. The memory
>>>> mapping may not be specified by RISC-V Intl, but the fact that
>>>> mtime(cmp) are 64-bit registers is very much specified.
>>> 
>>> Yes, mtime(cmp) has to be a 64bit wide as-per ratified spec but the
>>> spec does not require 64bit MMIO to be supported for these MMIO
>> registers.
>>> 
>>> In fact, for RV32 64bit MMIO is not possible at all because machine
>>> word is 32bits wide.
>>> 
>>> The T-Head implementation does not violate the mtime(cmp)
>> requirements
>>> of ratified spec but it is certainly bit strange because they have a
>>> RV64 system but their custom CLINT only supports 32bit MMIO accesses.
>> 
>> The quoted part of the spec says that, on RV64, aligned 64-bit accesses are
>> atomic. There is the clear implication there that 64-bit accesses are allowed.
>> This is entirely a hardware bug that violates the spec.
> 
> It does not say that aligned 64bit accesses have to be supported. There is
> no mandate in the statement.
> 
> It only says if 64bit accesses are supported then they have to be atomic.

There is no “if” in the spec, just that they *are*.

Jess