Re: [PATCH 0/1] arm64/tools/sysreg: Add Sysreg128/SysregFields128

[Anshuman Khandual <anshuman.khandual@arm.com>]

On 8/3/24 16:37, Mark Rutland wrote:
> On Thu, Aug 01, 2024 at 11:14:35AM +0530, Anshuman Khandual wrote:
>> FEAT_SYSREG128 enables 128 bit wide system registers which also need to be
>> defined in (arch/arm64/toos/sysreg) for auto mask generation. This adds two
>> new field types i.e Sysreg128 and SysregFields128 for that same purpose. It
>> utilizes recently added macro GENMASK_U128() while also adding some helpers
>> such as define_field_128() and parse_bitdef_128().
>>
>> This patch applies after the following series which adds GENMASK_U128()
>>
>> https://lore.kernel.org/all/20240725054808.286708-1-anshuman.khandual@arm.com/
> 
> Is that patch merged or not? It wouod make a lot more sense to send them
> together as one series.

The latest series [1] here has been reviewed and merged in bitmap-for-next
tree for testing purpose. Also there has been an additional patch [2] just
to keep the GENMASK_U128() helpers inside !__ASSEMBLY__ guard.

[1] https://lore.kernel.org/all/20240801071646.682731-1-anshuman.khandual@arm.com/
[2] https://lore.kernel.org/all/20240803133753.1598137-1-yury.norov@gmail.com/

GENMASK_U128() series could have been part of this series, although 128 bit
mask creation seems generic enough to stand on its own.

> 
>>
>> A. Example for SysregFields128
>>
>> ------------------------------
>> SysregFields128 TTBRx_D128_EL1
>> Res0   127:88
>> Field  87:80   BADDR_HIGH
>> Res0   79:64
>> Field  63:48   ASID
>> Field  47:5    BADDR_LOW
>> Res0   4:3
>> Field  2:1     SKL
>> Field  0       CnP
>> EndSysregFields128
>> ------------------------------
> 
> Ok, so we get the definitions, but do we have all the other helpers we'd
> need to make that useable, i.e.

The first objective was to get the definitions right, so that they could
be stored in the new gcc __unit128 data type.

> 
> * read_sysreg() and write_sysreg() variants that can use MRRS/MSRR
> 
> * Macros for assembly to use these?
> 
> * Other bitfield manipulation helpers that can operate on U128, e.g.
>   FIELD_GET() and FIELD_PREP() ?

These are still work in progress, but will share when available.

> 
> Without end-to-end usage this is a bit academic. If the U128 definitions
> are oainful to use from asm we might want separate hi64/lo64
> definitions.

Right, U128 definitions are difficult to use in asm code because they way
gcc compiler deals with 128 bit data types. Should the separate hi64/lo64
definitions be generic or platform specific without adding them to kernel
bitops ?

> 
> Mark.
> 
>> The above input generates the following macros
>>
>> #define TTBRx_D128_EL1_BADDR_HIGH                       GENMASK_U128(87, 80)
>> #define TTBRx_D128_EL1_BADDR_HIGH_MASK                  GENMASK_U128(87, 80)
>> #define TTBRx_D128_EL1_BADDR_HIGH_SHIFT                 80
>> #define TTBRx_D128_EL1_BADDR_HIGH_WIDTH                 8
>>
>> #define TTBRx_D128_EL1_ASID                             GENMASK_U128(63, 48)
>> #define TTBRx_D128_EL1_ASID_MASK                        GENMASK_U128(63, 48)
>> #define TTBRx_D128_EL1_ASID_SHIFT                       48
>> #define TTBRx_D128_EL1_ASID_WIDTH                       16
>>
>> #define TTBRx_D128_EL1_BADDR_LOW                        GENMASK_U128(47, 5)
>> #define TTBRx_D128_EL1_BADDR_LOW_MASK                   GENMASK_U128(47, 5)
>> #define TTBRx_D128_EL1_BADDR_LOW_SHIFT                  5
>> #define TTBRx_D128_EL1_BADDR_LOW_WIDTH                  43
>>
>> #define TTBRx_D128_EL1_SKL                              GENMASK_U128(2, 1)
>> #define TTBRx_D128_EL1_SKL_MASK                         GENMASK_U128(2, 1)
>> #define TTBRx_D128_EL1_SKL_SHIFT                        1
>> #define TTBRx_D128_EL1_SKL_WIDTH                        2
>>
>> #define TTBRx_D128_EL1_CnP                              GENMASK_U128(0, 0)
>> #define TTBRx_D128_EL1_CnP_MASK                         GENMASK_U128(0, 0)
>> #define TTBRx_D128_EL1_CnP_SHIFT                        0
>> #define TTBRx_D128_EL1_CnP_WIDTH                        1
>>
>> #define TTBRx_D128_EL1_RES0                             (UL(0) | GENMASK_U128(127, 88) | GENMASK_U128(79, 64) | GENMASK_U128(4, 3))
>> #define TTBRx_D128_EL1_RES1                             (UL(0))
>> #define TTBRx_D128_EL1_UNKN                             (UL(0))
>>
>> B. Example Sysreg128
>>
>> ------------------------------
>> Sysreg128      TTBR1_D128_EL1  3       0       2       0       1
>> Res0   127:88
>> Field  87:80   BADDR_HIGH
>> Res0   79:64
>> Field  63:48   ASID
>> Field  47:5    BADDR_LOW
>> Res0   4:3
>> Field  2:1     SKL
>> Field  0       CnP
>> EndSysreg128
>> ------------------------------
>>
>> The above input generates the following macros
>>
>> #define REG_TTBR1_D128_EL1                              S3_0_C2_C0_1
>> #define SYS_TTBR1_D128_EL1                              sys_reg(3, 0, 2, 0, 1)
>> #define SYS_TTBR1_D128_EL1_Op0                          3
>> #define SYS_TTBR1_D128_EL1_Op1                          0
>> #define SYS_TTBR1_D128_EL1_CRn                          2
>> #define SYS_TTBR1_D128_EL1_CRm                          0
>> #define SYS_TTBR1_D128_EL1_Op2                          1
>>
>> #define TTBR1_D128_EL1_BADDR_HIGH                       GENMASK_U128(87, 80)
>> #define TTBR1_D128_EL1_BADDR_HIGH_MASK                  GENMASK_U128(87, 80)
>> #define TTBR1_D128_EL1_BADDR_HIGH_SHIFT                 80
>> #define TTBR1_D128_EL1_BADDR_HIGH_WIDTH                 8
>>
>> #define TTBR1_D128_EL1_ASID                             GENMASK_U128(63, 48)
>> #define TTBR1_D128_EL1_ASID_MASK                        GENMASK_U128(63, 48)
>> #define TTBR1_D128_EL1_ASID_SHIFT                       48
>> #define TTBR1_D128_EL1_ASID_WIDTH                       16
>>
>> #define TTBR1_D128_EL1_BADDR_LOW                        GENMASK_U128(47, 5)
>> #define TTBR1_D128_EL1_BADDR_LOW_MASK                   GENMASK_U128(47, 5)
>> #define TTBR1_D128_EL1_BADDR_LOW_SHIFT                  5
>> #define TTBR1_D128_EL1_BADDR_LOW_WIDTH                  43
>>
>> #define TTBR1_D128_EL1_SKL                              GENMASK_U128(2, 1)
>> #define TTBR1_D128_EL1_SKL_MASK                         GENMASK_U128(2, 1)
>> #define TTBR1_D128_EL1_SKL_SHIFT                        1
>> #define TTBR1_D128_EL1_SKL_WIDTH                        2
>>
>> #define TTBR1_D128_EL1_CnP                              GENMASK_U128(0, 0)
>> #define TTBR1_D128_EL1_CnP_MASK                         GENMASK_U128(0, 0)
>> #define TTBR1_D128_EL1_CnP_SHIFT                        0
>> #define TTBR1_D128_EL1_CnP_WIDTH                        1
>>
>> #define TTBR1_D128_EL1_RES0                             (UL(0) | GENMASK_U128(127, 88) | GENMASK_U128(79, 64) | GENMASK_U128(4, 3))
>> #define TTBR1_D128_EL1_RES1                             (UL(0))
>> #define TTBR1_D128_EL1_UNKN                             (UL(0))
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mark Brown <broonie@kernel.org>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>>
>> Anshuman Khandual (1):
>>   arm64/tools/sysreg: Add Sysreg128/SysregFields128
>>
>>  arch/arm64/tools/gen-sysreg.awk | 231 ++++++++++++++++++++++++++++++++
>>  1 file changed, 231 insertions(+)
>>
>> -- 
>> 2.30.2
>>
>>