Re: [PATCH] riscv: hwprobe: export VA_BITS

["Clément Léger" <cleger@rivosinc.com>]

On 04/02/2024 01:22, Jessica Clarke wrote:
> On 3 Feb 2024, at 10:40, Stefan O'Rear <sorear@fastmail.com> wrote:
>>
>> On Fri, Feb 2, 2024, at 3:22 AM, Clément Léger wrote:
>>> On 02/02/2024 03:32, Charlie Jenkins wrote:
>>>> On Thu, Feb 01, 2024 at 03:02:45PM +0100, Clément Léger wrote:
>>>>> Some userspace applications (OpenJDK for instance) uses the free bits
>>>>> in pointers to insert additional information for their own logic.
>>>>> Currently they rely on parsing /proc/cpuinfo to obtain the current value
>>>>> of virtual address used bits [1]. Exporting VA_BITS through hwprobe will
>>>>> allow for a more stable interface to be used.
>>>>
>>>> mmap already supports this without a need for applications to know the
>>>> underlying hardware. If a hint address is passed into mmap, it will never
>>>> return an address that uses more bits than the hint address. I designed
>>>> it that way so that something like this wasn't necessary.
>>>
>>> Ok even though probing this kind of thing is probably not what mmap is
>>> meant to do. IMHO, probing this through the regular hwprobe interface is
>>> probably more coherent but maybe Robin (which needs this information)
>>> can rely on that.
>>
>> Both of these are useful, separately and in conjunction.
>>
>> hwprobe allows applications which can adapt to different VA sizes to learn
>> which is in use prior to allocating memory.
>>
>> mmap hints allow applications which require a fixed limit on the VA size to
>> express that limit at the point of requirement, the hint can be set based on
>> the hwprobe result to explicitly indicate its use.
> 
> Neither is an architecture-specific concept though. If you want to
> expose a notion of VA bits to userspace then it should probably be done
> in a more generic manner than the RISC-V-specific hwprobe syscall,
> probably as two numbers, low and high bits (ia64’s regions may be gone,
> but sparc64 still has both halves of the address space presented to
> userspace, with the stack in high/negative memory).

Agreed, this is not architecture specific. The initial goal was to
expose SVxx extensions which are hardware specific but it seemed wrong
since this is some S-mode extension. But maybe we can go back with
exposing these extensions rather than VA_BITS.

If exposing VA_BITS is still  useful for other architectures, maybe
exposing a mask of "used" bits could be more flexible. Which mechanism
to use to expose that is another problem. sysconf() might be a good fit
since it already exposes page size.

Clément

> 
> Jess
> 
>> -s
>>
>>> Clément
>>>
>>>>
>>>> - Charlie
>>>>
>>>>>
>>>>> Link: https://github.com/openjdk/jdk/blob/master/src/hotspot/os_cpu/linux_riscv/vm_version_linux_riscv.cpp#L171 [1]
>>>>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>>>>>
>>>>> ---
>>>>> Documentation/arch/riscv/hwprobe.rst  | 3 +++
>>>>> arch/riscv/include/asm/hwprobe.h      | 2 +-
>>>>> arch/riscv/include/uapi/asm/hwprobe.h | 1 +
>>>>> arch/riscv/kernel/sys_hwprobe.c       | 3 +++
>>>>> 4 files changed, 8 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
>>>>> index b2bcc9eed9aa..6f198c6ed4f0 100644
>>>>> --- a/Documentation/arch/riscv/hwprobe.rst
>>>>> +++ b/Documentation/arch/riscv/hwprobe.rst
>>>>> @@ -210,3 +210,6 @@ The following keys are defined:
>>>>>
>>>>> * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
>>>>>   represents the size of the Zicboz block in bytes.
>>>>> +
>>>>> +* :c:macro:`RISCV_HWPROBE_KEY_VA_BITS`: An unsigned long which
>>>>> +  represent the number of bits used to store virtual addresses.
>>>>> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
>>>>> index 630507dff5ea..150a9877b0af 100644
>>>>> --- a/arch/riscv/include/asm/hwprobe.h
>>>>> +++ b/arch/riscv/include/asm/hwprobe.h
>>>>> @@ -8,7 +8,7 @@
>>>>>
>>>>> #include <uapi/asm/hwprobe.h>
>>>>>
>>>>> -#define RISCV_HWPROBE_MAX_KEY 6
>>>>> +#define RISCV_HWPROBE_MAX_KEY 7
>>>>>
>>>>> static inline bool riscv_hwprobe_key_is_valid(__s64 key)
>>>>> {
>>>>> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
>>>>> index 9f2a8e3ff204..2a5006cddb7b 100644
>>>>> --- a/arch/riscv/include/uapi/asm/hwprobe.h
>>>>> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
>>>>> @@ -67,6 +67,7 @@ struct riscv_hwprobe {
>>>>> #define RISCV_HWPROBE_MISALIGNED_UNSUPPORTED (4 << 0)
>>>>> #define RISCV_HWPROBE_MISALIGNED_MASK (7 << 0)
>>>>> #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE 6
>>>>> +#define RISCV_HWPROBE_KEY_VA_BITS 7
>>>>> /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
>>>>>
>>>>> /* Flags */
>>>>> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
>>>>> index a7c56b41efd2..328435836e36 100644
>>>>> --- a/arch/riscv/kernel/sys_hwprobe.c
>>>>> +++ b/arch/riscv/kernel/sys_hwprobe.c
>>>>> @@ -202,6 +202,9 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
>>>>> if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
>>>>> pair->value = riscv_cboz_block_size;
>>>>> break;
>>>>> + case RISCV_HWPROBE_KEY_VA_BITS:
>>>>> + pair->value = VA_BITS;
>>>>> + break;
>>>>>
>>>>> /*
>>>>>  * For forward compatibility, unknown keys don't fail the whole
>>>>> -- 
>>>>> 2.43.0
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> linux-riscv mailing list
>>>>> linux-riscv@lists.infradead.org
>>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>>
>>> _______________________________________________
>>> linux-riscv mailing list
>>> linux-riscv@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
> 
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv