Skip to contentSkip to conversation

hw/s390x: Fix bad mask in time2tod()

PATCH Applied Thomas Huth 5 participants 8 messages Dec 14, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 hw/s390x: Fix bad mask in time2tod()

The time2tod() function tries to deal with the 9 uppermost bits in the time value, but uses the wrong mask for this: 0xff80000000000000 should be used instead of 0xff10000000000000 here.

Fixes 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd
---
 include/hw/s390x/tod.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/hw/s390x/tod.h +1 -1
index cbd7552..47ef9de 100644
@@ -56,7 +56,7 @@ typedef struct S390TODClass {
 /* Converts ns to s390's clock format */
 static inline uint64_t time2tod(uint64_t ns)
 {
-    return (ns << 9) / 125 + (((ns & 0xff10000000000000ull) / 125) << 9);
+    return (ns << 9) / 125 + (((ns & 0xff80000000000000ull) / 125) << 9);
 }
 
 /* Converts s390's clock format to ns */
Signature · 1 line
1.8.3.1
Reviewed-by on v1 1/1
#1 lore raw
Quoted 16 lines · Thomas Huth

The time2tod() function tries to deal with the 9 uppermost bits in the time value, but uses the wrong mask for this: 0xff80000000000000 should be used instead of 0xff10000000000000 here.

Fixes 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd
---
 include/hw/s390x/tod.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/hw/s390x/tod.h +0 -0
index cbd7552..47ef9de 100644

include/hw/s390x/tod.h typedef struct S390TODClass { · line 62

Quoted 8 lines
include/hw/s390x/tod.h +1 -1
@@ -56,7 +56,7 @@ typedef struct S390TODClass {
 /* Converts ns to s390's clock format */
 static inline uint64_t time2tod(uint64_t ns)
 {
-    return (ns << 9) / 125 + (((ns & 0xff10000000000000ull) / 125) << 9);
+    return (ns << 9) / 125 + (((ns & 0xff80000000000000ull) / 125) << 9);
 }
 
 /* Converts s390's clock format to ns */
Signature · 3 lines
Thanks,

David / dhildenb
#2 lore raw
In reply to Thomas Huth
Quoted 5 lines · Thomas Huth

The time2tod() function tries to deal with the 9 uppermost bits in the time value, but uses the wrong mask for this: 0xff80000000000000 should be used instead of 0xff10000000000000 here.

Fixes 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd

Can you alsways have commit id and subject

like

Fixes 14055ce53c2d ("s390x/tcg: avoid overflows in time2tod/tod2time")
Quoted 19 lines
---
 include/hw/s390x/tod.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/hw/s390x/tod.h +1 -1
index cbd7552..47ef9de 100644
@@ -56,7 +56,7 @@ typedef struct S390TODClass {
 /* Converts ns to s390's clock format */
 static inline uint64_t time2tod(uint64_t ns)
 {
-    return (ns << 9) / 125 + (((ns & 0xff10000000000000ull) / 125) << 9);
+    return (ns << 9) / 125 + (((ns & 0xff80000000000000ull) / 125) << 9);
 }
 
 /* Converts s390's clock format to ns */
#3 lore raw
Quoted 7 lines · Christian Borntraeger
Quoted 4 lines · Thomas Huth

The time2tod() function tries to deal with the 9 uppermost bits in the time value, but uses the wrong mask for this: 0xff80000000000000 should be used instead of 0xff10000000000000 here.

Quoted 6 lines
Quoted 1 line
Fixes 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd

Can you alsways have commit id and subject

like

Fixes 14055ce53c2d ("s390x/tcg: avoid overflows in time2tod/tod2time")

In https://wiki.qemu.org/Contribute/SubmitAPatch we currently have:

Fixes <full-SHA-commit-id>

... and the full commit ID is sometimes very useful for downstream, so I'd rather avoid to abbreviate that here. But I can try to remember to put the title of the patch in another spot of the description, too.

Thomas

#4 lore raw
Quoted 10 lines · Thomas Huth
Quoted 9 lines · Christian Borntraeger
Quoted 5 lines · Thomas Huth

The time2tod() function tries to deal with the 9 uppermost bits in the time value, but uses the wrong mask for this: 0xff80000000000000 should be used instead of 0xff10000000000000 here.

Fixes 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd
Quoted 8 lines
Quoted 4 lines

Can you alsways have commit id and subject

like

Fixes 14055ce53c2d ("s390x/tcg: avoid overflows in time2tod/tod2time")

In https://wiki.qemu.org/Contribute/SubmitAPatch we currently have:

Fixes <full-SHA-commit-id>

Interesting. Linus strongly opposed to only have the commit id as people often do cut and paste errors so nobody could actually find out which commit was meant. So the Linux variant is not sha commit of at least 12 digits + subject.

Quoted 7 lines

... and the full commit ID is sometimes very useful for downstream, so I'd rather avoid to abbreviate that here. But I can try to remember to put the title of the patch in another spot of the description, too.

Thomas

#5 lore raw
Quoted 18 lines · Christian Borntraeger
Quoted 15 lines · Thomas Huth
Quoted 13 lines · Christian Borntraeger
Quoted 5 lines · Thomas Huth

The time2tod() function tries to deal with the 9 uppermost bits in the time value, but uses the wrong mask for this: 0xff80000000000000 should be used instead of 0xff10000000000000 here.

Fixes 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd

Can you alsways have commit id and subject

like

Fixes 14055ce53c2d ("s390x/tcg: avoid overflows in time2tod/tod2time")
Quoted 7 lines
Quoted 3 lines

In https://wiki.qemu.org/Contribute/SubmitAPatch we currently have:

Fixes <full-SHA-commit-id>

Interesting. Linus strongly opposed to only have the commit id as people often do cut and paste errors so nobody could actually find out which commit was meant. So the Linux variant is not sha commit of at least 12 digits + subject.

Mentioning the title certainly makes sense, too, so feel free to extend the Wiki page if you like!

Thomas

#6 lore raw
Quoted 21 lines · Thomas Huth
Quoted 20 lines · Christian Borntraeger
Quoted 17 lines · Thomas Huth
Quoted 7 lines · Christian Borntraeger
Quoted 4 lines · Thomas Huth

The time2tod() function tries to deal with the 9 uppermost bits in the time value, but uses the wrong mask for this: 0xff80000000000000 should be used instead of 0xff10000000000000 here.

Quoted 6 lines
Quoted 1 line
Fixes 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd

Can you alsways have commit id and subject

like

Fixes 14055ce53c2d ("s390x/tcg: avoid overflows in time2tod/tod2time")

In https://wiki.qemu.org/Contribute/SubmitAPatch we currently have:

Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Fixes <full-SHA-commit-id>

Interesting. Linus strongly opposed to only have the commit id as people often do cut and paste errors so nobody could actually find out which commit was meant. So the Linux variant is not sha commit of at least 12 digits + subject.

Mentioning the title certainly makes sense, too, so feel free to extend the Wiki page if you like!

Done :)

#7 lore raw
In reply to Thomas Huth
Quoted 3 lines · On Fri, 14 Dec 2018 14:08:07 +0100 Thomas Huth <[email protected]> wrote:

The time2tod() function tries to deal with the 9 uppermost bits in the time value, but uses the wrong mask for this: 0xff80000000000000 should be used instead of 0xff10000000000000 here.

I've tweaked this to

Since "s390x/tcg: avoid overflows in time2tod/tod2time", the time2tod() function tries to deal with the 9 uppermost bits in the time value, but uses the wrong mask for this: 0xff80000000000000 should be used instead of 0xff10000000000000 here.

Quoted 2 lines
Fixes 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd

I'll add cc:stable in the patch description as well.

Quoted 10 lines
---
 include/hw/s390x/tod.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/hw/s390x/tod.h +0 -0
index cbd7552..47ef9de 100644

include/hw/s390x/tod.h typedef struct S390TODClass { · line 62

Quoted 8 lines
include/hw/s390x/tod.h +1 -1
@@ -56,7 +56,7 @@ typedef struct S390TODClass {
 /* Converts ns to s390's clock format */
 static inline uint64_t time2tod(uint64_t ns)
 {
-    return (ns << 9) / 125 + (((ns & 0xff10000000000000ull) / 125) << 9);
+    return (ns << 9) / 125 + (((ns & 0xff80000000000000ull) / 125) << 9);
 }
 
 /* Converts s390's clock format to ns */

Thanks, applied.