[Qemu-devel] [PATCH] hw/s390x: Fix bad mask in time2tod()

Thomas Huth posted 1 patch 6 years, 10 months ago
Test checkpatch passed
Test asan passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1544792887-14575-1-git-send-email-thuth@redhat.com
include/hw/s390x/tod.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[Qemu-devel] [PATCH] hw/s390x: Fix bad mask in time2tod()
Posted by Thomas Huth 6 years, 10 months ago
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
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/hw/s390x/tod.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/s390x/tod.h b/include/hw/s390x/tod.h
index cbd7552..47ef9de 100644
--- a/include/hw/s390x/tod.h
+++ b/include/hw/s390x/tod.h
@@ -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 */
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH] hw/s390x: Fix bad mask in time2tod()
Posted by David Hildenbrand 6 years, 10 months ago
On 14.12.18 14:08, Thomas Huth 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.
> 
> Fixes: 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  include/hw/s390x/tod.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/hw/s390x/tod.h b/include/hw/s390x/tod.h
> index cbd7552..47ef9de 100644
> --- a/include/hw/s390x/tod.h
> +++ b/include/hw/s390x/tod.h
> @@ -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 */
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH] hw/s390x: Fix bad mask in time2tod()
Posted by Christian Borntraeger 6 years, 10 months ago

On 14.12.2018 14:08, Thomas Huth 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.
> 
> Fixes: 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd

Can you alsways have commit id and subject

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

> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  include/hw/s390x/tod.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/hw/s390x/tod.h b/include/hw/s390x/tod.h
> index cbd7552..47ef9de 100644
> --- a/include/hw/s390x/tod.h
> +++ b/include/hw/s390x/tod.h
> @@ -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 */
> 


Re: [Qemu-devel] [PATCH] hw/s390x: Fix bad mask in time2tod()
Posted by Thomas Huth 6 years, 10 months ago
On 2018-12-14 14:15, Christian Borntraeger wrote:
> 
> 
> On 14.12.2018 14:08, Thomas Huth 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.
>>
>> 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

Re: [Qemu-devel] [PATCH] hw/s390x: Fix bad mask in time2tod()
Posted by Christian Borntraeger 6 years, 10 months ago

On 14.12.2018 14:23, Thomas Huth wrote:
> On 2018-12-14 14:15, Christian Borntraeger wrote:
>>
>>
>> On 14.12.2018 14:08, Thomas Huth 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.
>>>
>>> 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>


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.

> 
> ... 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
> 


Re: [Qemu-devel] [PATCH] hw/s390x: Fix bad mask in time2tod()
Posted by Thomas Huth 6 years, 10 months ago
On 2018-12-14 14:26, Christian Borntraeger wrote:
> 
> 
> On 14.12.2018 14:23, Thomas Huth wrote:
>> On 2018-12-14 14:15, Christian Borntraeger wrote:
>>>
>>>
>>> On 14.12.2018 14:08, Thomas Huth 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.
>>>>
>>>> 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>
> 
> 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

Re: [Qemu-devel] [PATCH] hw/s390x: Fix bad mask in time2tod()
Posted by Philippe Mathieu-Daudé 6 years, 10 months ago
On 12/14/18 2:30 PM, Thomas Huth wrote:
> On 2018-12-14 14:26, Christian Borntraeger wrote:
>>
>>
>> On 14.12.2018 14:23, Thomas Huth wrote:
>>> On 2018-12-14 14:15, Christian Borntraeger wrote:
>>>>
>>>>
>>>> On 14.12.2018 14:08, Thomas Huth 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.
>>>>>
>>>>> 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>
>>
>> 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 :)

Re: [Qemu-devel] [PATCH] hw/s390x: Fix bad mask in time2tod()
Posted by Cornelia Huck 6 years, 10 months ago
On Fri, 14 Dec 2018 14:08:07 +0100
Thomas Huth <thuth@redhat.com> 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.

> 
> Fixes: 14055ce53c2d901d826ffad7fb7d6bb8ab46bdfd

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

> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  include/hw/s390x/tod.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/hw/s390x/tod.h b/include/hw/s390x/tod.h
> index cbd7552..47ef9de 100644
> --- a/include/hw/s390x/tod.h
> +++ b/include/hw/s390x/tod.h
> @@ -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.