| Message ID | 20221205142043.95185-1-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for-8.0] target/s390x/tcg/mem_helper: Test the right bits in psw_key_valid() | expand |
On 05.12.22 15:20, Thomas Huth wrote: > The PSW key mask is a 16 bit field, and the psw_key variable is > in the range from 0 to 15, so it does not make sense to use > "0x80 >> psw_key" for testing the bits here. We should use 0x8000 > instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > Found by code inspection (Linux likely does not use these PSW key masks > yet, otherwise we might have noticed earlier) > > target/s390x/tcg/mem_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c > index 9542fad59b..cb82cd1c1d 100644 > --- a/target/s390x/tcg/mem_helper.c > +++ b/target/s390x/tcg/mem_helper.c > @@ -51,7 +51,7 @@ static inline bool psw_key_valid(CPUS390XState *env, uint8_t psw_key) > > if (env->psw.mask & PSW_MASK_PSTATE) { > /* PSW key has range 0..15, it is valid if the bit is 1 in the PKM */ > - return pkm & (0x80 >> psw_key); > + return pkm & (0x8000 >> psw_key); > } > return true; > } I assume a Fixes tag might be applicable (I remember I added that once). Reviewed-by: David Hildenbrand <david@redhat.com>
On Mon, 2022-12-05 at 15:20 +0100, Thomas Huth wrote: > The PSW key mask is a 16 bit field, and the psw_key variable is > in the range from 0 to 15, so it does not make sense to use > "0x80 >> psw_key" for testing the bits here. We should use 0x8000 > instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com> > --- > Found by code inspection (Linux likely does not use these PSW key masks > yet, otherwise we might have noticed earlier) > > target/s390x/tcg/mem_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c > index 9542fad59b..cb82cd1c1d 100644 > --- a/target/s390x/tcg/mem_helper.c > +++ b/target/s390x/tcg/mem_helper.c > @@ -51,7 +51,7 @@ static inline bool psw_key_valid(CPUS390XState *env, uint8_t psw_key) > > if (env->psw.mask & PSW_MASK_PSTATE) { > /* PSW key has range 0..15, it is valid if the bit is 1 in the PKM */ > - return pkm & (0x80 >> psw_key); > + return pkm & (0x8000 >> psw_key); > } > return true; > }
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c index 9542fad59b..cb82cd1c1d 100644 --- a/target/s390x/tcg/mem_helper.c +++ b/target/s390x/tcg/mem_helper.c @@ -51,7 +51,7 @@ static inline bool psw_key_valid(CPUS390XState *env, uint8_t psw_key) if (env->psw.mask & PSW_MASK_PSTATE) { /* PSW key has range 0..15, it is valid if the bit is 1 in the PKM */ - return pkm & (0x80 >> psw_key); + return pkm & (0x8000 >> psw_key); } return true; }
The PSW key mask is a 16 bit field, and the psw_key variable is in the range from 0 to 15, so it does not make sense to use "0x80 >> psw_key" for testing the bits here. We should use 0x8000 instead. Signed-off-by: Thomas Huth <thuth@redhat.com> --- Found by code inspection (Linux likely does not use these PSW key masks yet, otherwise we might have noticed earlier) target/s390x/tcg/mem_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)