Re: [kvm-unit-tests PATCH v1 2/2] arm64: Define name for the bits used in SCTLR_EL1_RES1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Shaoqin,

On 7/19/23 05:19, Shaoqin Huang wrote:
> Currently some fields in SCTLR_EL1 don't define a name and directly used
> in the SCTLR_EL1_RES1, that's not good now since these fields have been
> functional and have a name.
>
> According to the ARM DDI 0487J.a, define the name related to these
> fields.
>
> Suggested-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
> Signed-off-by: Shaoqin Huang <shahuang@xxxxxxxxxx>
> ---
>  lib/arm64/asm/sysreg.h | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/lib/arm64/asm/sysreg.h b/lib/arm64/asm/sysreg.h
> index c7f529d..9c68698 100644
> --- a/lib/arm64/asm/sysreg.h
> +++ b/lib/arm64/asm/sysreg.h
> @@ -80,17 +80,26 @@ asm(
>  #define ICC_GRPEN1_EL1			sys_reg(3, 0, 12, 12, 7)
>  
>  /* System Control Register (SCTLR_EL1) bits */
> +#define SCTLR_EL1_LSMAOE	_BITUL(29)
> +#define SCTLR_EL1_NTLSMD	_BITUL(28)
>  #define SCTLR_EL1_EE		_BITUL(25)
> +#define SCTLR_EL1_SPAN		_BITUL(23)
> +#define SCTLR_EL1_EIS		_BITUL(22)
> +#define SCTLR_EL1_TSCXT		_BITUL(20)
>  #define SCTLR_EL1_WXN		_BITUL(19)
>  #define SCTLR_EL1_I		_BITUL(12)
> +#define SCTLR_EL1_EOS		_BITUL(11)
> +#define SCTLR_EL1_SED		_BITUL(8)
> +#define SCTLR_EL1_ITD		_BITUL(7)
>  #define SCTLR_EL1_SA0		_BITUL(4)
>  #define SCTLR_EL1_SA		_BITUL(3)
>  #define SCTLR_EL1_C		_BITUL(2)
>  #define SCTLR_EL1_A		_BITUL(1)
>  #define SCTLR_EL1_M		_BITUL(0)
>  
> -#define SCTLR_EL1_RES1	(_BITUL(7) | _BITUL(8) | _BITUL(11) | _BITUL(20) | \
> -			 _BITUL(22) | _BITUL(23) | _BITUL(28) | _BITUL(29))
> +#define SCTLR_EL1_RES1	(SCTLR_EL1_ITD | SCTLR_EL1_SED | SCTLR_EL1_EOS | \
> +			 SCTLR_EL1_TSCXT | SCTLR_EL1_EIS | SCTLR_EL1_SPAN | \
> +			 SCTLR_EL1_NTLSMD | SCTLR_EL1_LSMAOE)
>  #define INIT_SCTLR_EL1_MMU_OFF	\
>  			SCTLR_EL1_RES1
>  
The change looks good to me (although _BITULL remark still holds).

Independently on this patch the _RES1 terminology looks odd to me. For
example ESO bit is RES1 only if FEAT_ExS is not implemented. Maybe I
misunderstand why it was named that way but to me RES1 means another
thing. If confirmed we could simply drop SCTLR_EL1_RES1 which is not
used elsewhere and directly define INIT_SCTLR_EL1_MMU_OF.

Thanks

Eric




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux