| Message ID | 20221004145152.3020464-1-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [boot-wrapper] sme: Fix sign-extension bug in SMCR_EL3 write | expand |
On Tue, Oct 04, 2022 at 03:51:52PM +0100, Andre Przywara wrote: > To enable the full AArch64 ISA support in streaming SVE mode, we need to > set bit 31 in the SMCR_EL3 system register. However ORing (1 << 31) into > an unsigned long variable will lead to all upper 32 bits becoming 1, > which is not what we want. We are just saved by those bits being RES0, > at least for now. > > Explicitly use an unsigned base for the shift, to avoid the sign > extension. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > arch/aarch64/include/asm/cpu.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h > index d063948..0c400cc 100644 > --- a/arch/aarch64/include/asm/cpu.h > +++ b/arch/aarch64/include/asm/cpu.h > @@ -117,7 +117,7 @@ > #define ZCR_EL3_LEN_MAX 0xf > > #define SMCR_EL3 s3_6_c1_c2_6 > -#define SMCR_EL3_FA64 (1 << 31) > +#define SMCR_EL3_FA64 (1U << 31) This looks good, but I'd prefer if we used BIT(31) here; that'll do the right thing internally. Are you happy if I fix that up when applying? Thanks, Mark. > #define SMCR_EL3_LEN_MAX 0xf > > #define ID_AA64ISAR2_EL1 s3_0_c0_c6_2 > -- > 2.25.1 >
On Tue, 4 Oct 2022 16:01:23 +0100 Mark Rutland <mark.rutland@arm.com> wrote: > On Tue, Oct 04, 2022 at 03:51:52PM +0100, Andre Przywara wrote: > > To enable the full AArch64 ISA support in streaming SVE mode, we need to > > set bit 31 in the SMCR_EL3 system register. However ORing (1 << 31) into > > an unsigned long variable will lead to all upper 32 bits becoming 1, > > which is not what we want. We are just saved by those bits being RES0, > > at least for now. > > > > Explicitly use an unsigned base for the shift, to avoid the sign > > extension. > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > arch/aarch64/include/asm/cpu.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h > > index d063948..0c400cc 100644 > > --- a/arch/aarch64/include/asm/cpu.h > > +++ b/arch/aarch64/include/asm/cpu.h > > @@ -117,7 +117,7 @@ > > #define ZCR_EL3_LEN_MAX 0xf > > > > #define SMCR_EL3 s3_6_c1_c2_6 > > -#define SMCR_EL3_FA64 (1 << 31) > > +#define SMCR_EL3_FA64 (1U << 31) > > This looks good, but I'd prefer if we used BIT(31) here; that'll do the right > thing internally. > > Are you happy if I fix that up when applying? Yes, sure. Thanks for taking care! Cheers, Andre > > > #define SMCR_EL3_LEN_MAX 0xf > > > > #define ID_AA64ISAR2_EL1 s3_0_c0_c6_2 > > -- > > 2.25.1 > >
On Tue, Oct 04, 2022 at 04:06:18PM +0100, Andre Przywara wrote: > On Tue, 4 Oct 2022 16:01:23 +0100 > Mark Rutland <mark.rutland@arm.com> wrote: > > > On Tue, Oct 04, 2022 at 03:51:52PM +0100, Andre Przywara wrote: > > > To enable the full AArch64 ISA support in streaming SVE mode, we need to > > > set bit 31 in the SMCR_EL3 system register. However ORing (1 << 31) into > > > an unsigned long variable will lead to all upper 32 bits becoming 1, > > > which is not what we want. We are just saved by those bits being RES0, > > > at least for now. > > > > > > Explicitly use an unsigned base for the shift, to avoid the sign > > > extension. > > > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > > --- > > > arch/aarch64/include/asm/cpu.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h > > > index d063948..0c400cc 100644 > > > --- a/arch/aarch64/include/asm/cpu.h > > > +++ b/arch/aarch64/include/asm/cpu.h > > > @@ -117,7 +117,7 @@ > > > #define ZCR_EL3_LEN_MAX 0xf > > > > > > #define SMCR_EL3 s3_6_c1_c2_6 > > > -#define SMCR_EL3_FA64 (1 << 31) > > > +#define SMCR_EL3_FA64 (1U << 31) > > > > This looks good, but I'd prefer if we used BIT(31) here; that'll do the right > > thing internally. > > > > Are you happy if I fix that up when applying? > > Yes, sure. > > Thanks for taking care! Thanks; applied. Mark.
diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h index d063948..0c400cc 100644 --- a/arch/aarch64/include/asm/cpu.h +++ b/arch/aarch64/include/asm/cpu.h @@ -117,7 +117,7 @@ #define ZCR_EL3_LEN_MAX 0xf #define SMCR_EL3 s3_6_c1_c2_6 -#define SMCR_EL3_FA64 (1 << 31) +#define SMCR_EL3_FA64 (1U << 31) #define SMCR_EL3_LEN_MAX 0xf #define ID_AA64ISAR2_EL1 s3_0_c0_c6_2
To enable the full AArch64 ISA support in streaming SVE mode, we need to set bit 31 in the SMCR_EL3 system register. However ORing (1 << 31) into an unsigned long variable will lead to all upper 32 bits becoming 1, which is not what we want. We are just saved by those bits being RES0, at least for now. Explicitly use an unsigned base for the shift, to avoid the sign extension. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- arch/aarch64/include/asm/cpu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)