[PATCH 0/2] arm64: Drop ID_AA64PFR0_EL1_ELx_[64BIT_ONLY|32BIT_64BIT]

[PATCH 0/2] arm64: Drop ID_AA64PFR0_EL1_ELx_[64BIT_ONLY|32BIT_64BIT]

[Anshuman Khandual]

This series replaces custom macros usage for ID_AA64PFR0_EL1_ELx_64BIT_ONLY
and ID_AA64PFR0_EL1_ELx_32BIT_64BIT fields, thus finally just dropping them
off completely. This series applies on v6.9-rc4.

Cc: Marc Zyngier <maz@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: kvmarm@lists.linux.dev
Cc: linux-kernel@vger.kernel.org

Anshuman Khandual (2):
  KVM: arm64: Replace custom macros with fields from ID_AA64PFR0_EL1
  arm64/cpufeature: Replace custom macros with fields from ID_AA64PFR0_EL1

 arch/arm64/include/asm/cpufeature.h            | 4 ++--
 arch/arm64/include/asm/sysreg.h                | 4 ----
 arch/arm64/kernel/cpufeature.c                 | 4 ++--
 arch/arm64/kvm/hyp/include/nvhe/fixed_config.h | 8 ++++----
 arch/arm64/kvm/hyp/nvhe/pkvm.c                 | 4 ++--
 arch/arm64/kvm/hyp/nvhe/sys_regs.c             | 2 +-
 6 files changed, 11 insertions(+), 15 deletions(-)

-- 
2.25.1

[PATCH 1/2] KVM: arm64: Replace custom macros with fields from ID_AA64PFR0_EL1

[Anshuman Khandual]

This replaces custom macros usage (i.e ID_AA64PFR0_EL1_ELx_64BIT_ONLY and
ID_AA64PFR0_EL1_ELx_32BIT_64BIT) and instead directly uses register fields
from ID_AA64PFR0_EL1 sysreg definition.

Cc: Marc Zyngier <maz@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: kvmarm@lists.linux.dev
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/arm64/kvm/hyp/include/nvhe/fixed_config.h | 8 ++++----
 arch/arm64/kvm/hyp/nvhe/pkvm.c                 | 4 ++--
 arch/arm64/kvm/hyp/nvhe/sys_regs.c             | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
index 51f043649146..0034bfffced6 100644
--- a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
+++ b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
@@ -52,10 +52,10 @@
  *	Supported by KVM
  */
 #define PVM_ID_AA64PFR0_RESTRICT_UNSIGNED (\
-	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
-	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
-	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL2), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
-	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL3), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
+	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), ID_AA64PFR0_EL1_EL0_IMP) | \
+	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), ID_AA64PFR0_EL1_EL1_IMP) | \
+	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL2), ID_AA64PFR0_EL1_EL2_IMP) | \
+	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL3), ID_AA64PFR0_EL1_EL3_IMP) | \
 	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_RAS), ID_AA64PFR0_EL1_RAS_IMP) \
 	)
 
diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 26dd9a20ad6e..58da2fec89b2 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -31,9 +31,9 @@ static void pvm_init_traps_aa64pfr0(struct kvm_vcpu *vcpu)
 
 	/* Protected KVM does not support AArch32 guests. */
 	BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0),
-		PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) != ID_AA64PFR0_EL1_ELx_64BIT_ONLY);
+		PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) != ID_AA64PFR0_EL1_EL0_IMP);
 	BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1),
-		PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) != ID_AA64PFR0_EL1_ELx_64BIT_ONLY);
+		PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) != ID_AA64PFR0_EL1_EL1_IMP);
 
 	/*
 	 * Linux guests assume support for floating-point and Advanced SIMD. Do
diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
index edd969a1f36b..2860548d4250 100644
--- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c
+++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
@@ -276,7 +276,7 @@ static bool pvm_access_id_aarch32(struct kvm_vcpu *vcpu,
 	 * of AArch32 feature id registers.
 	 */
 	BUILD_BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1),
-		     PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) > ID_AA64PFR0_EL1_ELx_64BIT_ONLY);
+		     PVM_ID_AA64PFR0_RESTRICT_UNSIGNED) > ID_AA64PFR0_EL1_EL1_IMP);
 
 	return pvm_access_raz_wi(vcpu, p, r);
 }
-- 
2.25.1

Re: [PATCH 1/2] KVM: arm64: Replace custom macros with fields from ID_AA64PFR0_EL1

[Marc Zyngier]

+ Fuad

On Thu, 18 Apr 2024 06:38:03 +0100,
Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> 
> This replaces custom macros usage (i.e ID_AA64PFR0_EL1_ELx_64BIT_ONLY and
> ID_AA64PFR0_EL1_ELx_32BIT_64BIT) and instead directly uses register fields
> from ID_AA64PFR0_EL1 sysreg definition.
> 
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: kvmarm@lists.linux.dev
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/arm64/kvm/hyp/include/nvhe/fixed_config.h | 8 ++++----
>  arch/arm64/kvm/hyp/nvhe/pkvm.c                 | 4 ++--
>  arch/arm64/kvm/hyp/nvhe/sys_regs.c             | 2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
> index 51f043649146..0034bfffced6 100644
> --- a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
> +++ b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
> @@ -52,10 +52,10 @@
>   *	Supported by KVM
>   */
>  #define PVM_ID_AA64PFR0_RESTRICT_UNSIGNED (\
> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL2), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL3), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), ID_AA64PFR0_EL1_EL0_IMP) | \
> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), ID_AA64PFR0_EL1_EL1_IMP) | \
> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL2), ID_AA64PFR0_EL1_EL2_IMP) | \
> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL3), ID_AA64PFR0_EL1_EL3_IMP) | \

If you are going to rework this, can we instead use something less
verbose such as SYS_FIELD_GET()?

There is also a series from Fuad moving things around, and maybe
that's the opportunity to rework this while limiting the amount of
cosmetic churn. Not to that this fixed config stuff needs to be
reworked in order to match the runtime feature enforcement that the
rest of KVM has adopted.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 1/2] KVM: arm64: Replace custom macros with fields from ID_AA64PFR0_EL1

[Anshuman Khandual]

On 4/18/24 13:09, Marc Zyngier wrote:
> + Fuad
> 
> On Thu, 18 Apr 2024 06:38:03 +0100,
> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>>
>> This replaces custom macros usage (i.e ID_AA64PFR0_EL1_ELx_64BIT_ONLY and
>> ID_AA64PFR0_EL1_ELx_32BIT_64BIT) and instead directly uses register fields
>> from ID_AA64PFR0_EL1 sysreg definition.
>>
>> Cc: Marc Zyngier <maz@kernel.org>
>> Cc: Oliver Upton <oliver.upton@linux.dev>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: kvmarm@lists.linux.dev
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/arm64/kvm/hyp/include/nvhe/fixed_config.h | 8 ++++----
>>  arch/arm64/kvm/hyp/nvhe/pkvm.c                 | 4 ++--
>>  arch/arm64/kvm/hyp/nvhe/sys_regs.c             | 2 +-
>>  3 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
>> index 51f043649146..0034bfffced6 100644
>> --- a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
>> +++ b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
>> @@ -52,10 +52,10 @@
>>   *	Supported by KVM
>>   */
>>  #define PVM_ID_AA64PFR0_RESTRICT_UNSIGNED (\
>> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
>> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
>> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL2), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
>> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL3), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
>> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), ID_AA64PFR0_EL1_EL0_IMP) | \
>> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), ID_AA64PFR0_EL1_EL1_IMP) | \
>> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL2), ID_AA64PFR0_EL1_EL2_IMP) | \
>> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL3), ID_AA64PFR0_EL1_EL3_IMP) | \
> 
> If you are going to rework this, can we instead use something less
> verbose such as SYS_FIELD_GET()?

Just wondering, is not FIELD_PREP() and SYS_FIELD_GET() does the exact opposite thing.
The earlier builds the entire register value from various constituents, where as the
later extracts a single register field from a complete register value instead. Or did
I just misunderstood something here.

> 
> There is also a series from Fuad moving things around, and maybe
> that's the opportunity to rework this while limiting the amount of
> cosmetic churn. Not to that this fixed config stuff needs to be

I guess that might be a better place to change the code instead. Because this series
just replaces the derived register field from tools syreg, but will be happy to have
those changes here as well in a separate pre/post patch.

> reworked in order to match the runtime feature enforcement that the
> rest of KVM has adopted.

I am afraid, did not get the above. Could you please give some more details.

Re: [PATCH 1/2] KVM: arm64: Replace custom macros with fields from ID_AA64PFR0_EL1

[Mark Rutland]

On Mon, Apr 29, 2024 at 07:53:14AM +0530, Anshuman Khandual wrote:
> On 4/18/24 13:09, Marc Zyngier wrote:
> > On Thu, 18 Apr 2024 06:38:03 +0100,
> > Anshuman Khandual <anshuman.khandual@arm.com> wrote:
> >>  #define PVM_ID_AA64PFR0_RESTRICT_UNSIGNED (\
> >> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
> >> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
> >> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL2), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
> >> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL3), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
> >> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), ID_AA64PFR0_EL1_EL0_IMP) | \
> >> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), ID_AA64PFR0_EL1_EL1_IMP) | \
> >> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL2), ID_AA64PFR0_EL1_EL2_IMP) | \
> >> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL3), ID_AA64PFR0_EL1_EL3_IMP) | \
> > 
> > If you are going to rework this, can we instead use something less
> > verbose such as SYS_FIELD_GET()?
> 
> Just wondering, is not FIELD_PREP() and SYS_FIELD_GET() does the exact opposite thing.
> The earlier builds the entire register value from various constituents, where as the
> later extracts a single register field from a complete register value instead. Or did
> I just misunderstood something here.
 
He means use one of the SYS_FIELD_*() helpers, e.g. SYS_FIELD_PREP_ENUM(), with
which this can be:

#define PVM_ID_AA64PFR0_RESTRICT_UNSIGNED (\
        SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, EL0, IMP) | \
        SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, EL1, IMP) | \
        SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, EL2, IMP) | \
        SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, EL3, IMP) | \
        SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, RAS, IMP) \
        )

... which is far less verbose, and much easier to read.

Mark.

Re: [PATCH 1/2] KVM: arm64: Replace custom macros with fields from ID_AA64PFR0_EL1

[Anshuman Khandual]

On 5/10/24 23:39, Mark Rutland wrote:
> On Mon, Apr 29, 2024 at 07:53:14AM +0530, Anshuman Khandual wrote:
>> On 4/18/24 13:09, Marc Zyngier wrote:
>>> On Thu, 18 Apr 2024 06:38:03 +0100,
>>> Anshuman Khandual <anshuman.khandual@arm.com> wrote:
>>>>  #define PVM_ID_AA64PFR0_RESTRICT_UNSIGNED (\
>>>> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
>>>> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
>>>> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL2), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
>>>> -	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL3), ID_AA64PFR0_EL1_ELx_64BIT_ONLY) | \
>>>> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL0), ID_AA64PFR0_EL1_EL0_IMP) | \
>>>> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL1), ID_AA64PFR0_EL1_EL1_IMP) | \
>>>> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL2), ID_AA64PFR0_EL1_EL2_IMP) | \
>>>> +	FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_EL3), ID_AA64PFR0_EL1_EL3_IMP) | \
>>>
>>> If you are going to rework this, can we instead use something less
>>> verbose such as SYS_FIELD_GET()?
>>
>> Just wondering, is not FIELD_PREP() and SYS_FIELD_GET() does the exact opposite thing.
>> The earlier builds the entire register value from various constituents, where as the
>> later extracts a single register field from a complete register value instead. Or did
>> I just misunderstood something here.
>  
> He means use one of the SYS_FIELD_*() helpers, e.g. SYS_FIELD_PREP_ENUM(), with
> which this can be:
> 
> #define PVM_ID_AA64PFR0_RESTRICT_UNSIGNED (\
>         SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, EL0, IMP) | \
>         SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, EL1, IMP) | \
>         SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, EL2, IMP) | \
>         SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, EL3, IMP) | \
>         SYS_FIELD_PREP_ENUM(ID_AA64PFR0_EL1, RAS, IMP) \
>         )
> 
> ... which is far less verbose, and much easier to read.

Got it, this makes sense, will fold in the above changes and respin
after the merge window. Thanks for the clarification.

Re: [PATCH 0/2] arm64: Drop ID_AA64PFR0_EL1_ELx_[64BIT_ONLY|32BIT_64BIT]

[Anshuman Khandual]

On 4/18/24 11:08, Anshuman Khandual wrote:
> This series replaces custom macros usage for ID_AA64PFR0_EL1_ELx_64BIT_ONLY
> and ID_AA64PFR0_EL1_ELx_32BIT_64BIT fields, thus finally just dropping them
> off completely. This series applies on v6.9-rc4.
> 
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: kvmarm@lists.linux.dev
> Cc: linux-kernel@vger.kernel.org
> 
> Anshuman Khandual (2):
>   KVM: arm64: Replace custom macros with fields from ID_AA64PFR0_EL1
>   arm64/cpufeature: Replace custom macros with fields from ID_AA64PFR0_EL1

Hello All,

I am bit confused here, is there something more that needs to be done. It
seems like the previous discussion thread remained some what inconclusive.
Please do let me know. Thank you.

- Anshuman