[v3] pc-bios: s390x: Cleanup part 1

[PATCH v3 2/9] pc-bios: s390x: Consolidate timing functions into time.h

Posted by Janosch Frank 5 years, 8 months ago

Let's consolidate timing related functions into one header.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 pc-bios/s390-ccw/menu.c        |  1 +
 pc-bios/s390-ccw/netmain.c     | 15 +++------------
 pc-bios/s390-ccw/s390-ccw.h    |  2 --
 pc-bios/s390-ccw/time.h        | 23 +++++++++++++++++++++++
 pc-bios/s390-ccw/virtio-net.c  |  1 +
 pc-bios/s390-ccw/virtio-scsi.c |  1 +
 pc-bios/s390-ccw/virtio.c      | 18 +++---------------
 7 files changed, 32 insertions(+), 29 deletions(-)
 create mode 100644 pc-bios/s390-ccw/time.h

diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index ce3815b201..7925c33248 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -12,6 +12,7 @@
 #include "libc.h"
 #include "s390-ccw.h"
 #include "sclp.h"
+#include "time.h"
 
 #define KEYCODE_NO_INP '\0'
 #define KEYCODE_ESCAPE '\033'
diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
index 309ffa30d9..73def8de4f 100644
--- a/pc-bios/s390-ccw/netmain.c
+++ b/pc-bios/s390-ccw/netmain.c
@@ -35,6 +35,7 @@
 #include "s390-ccw.h"
 #include "cio.h"
 #include "virtio.h"
+#include "time.h"
 
 #define DEFAULT_BOOT_RETRIES 10
 #define DEFAULT_TFTP_RETRIES 20
@@ -57,24 +58,14 @@ static SubChannelId net_schid = { .one = 1 };
 static uint8_t mac[6];
 static uint64_t dest_timer;
 
-static uint64_t get_timer_ms(void)
-{
-    uint64_t clk;
-
-    asm volatile(" stck %0 " : : "Q"(clk) : "memory");
-
-    /* Bit 51 is incremented each microsecond */
-    return (clk >> (63 - 51)) / 1000;
-}
-
 void set_timer(int val)
 {
-    dest_timer = get_timer_ms() + val;
+    dest_timer = get_time_ms() + val;
 }
 
 int get_timer(void)
 {
-    return dest_timer - get_timer_ms();
+    return dest_timer - get_time_ms();
 }
 
 int get_sec_ticks(void)
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 21f27e7990..9514b76596 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -74,8 +74,6 @@ unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
 bool virtio_is_supported(SubChannelId schid);
 void virtio_blk_setup_device(SubChannelId schid);
 int virtio_read(ulong sector, void *load_addr);
-u64 get_clock(void);
-ulong get_second(void);
 
 /* bootmap.c */
 void zipl_load(void);
diff --git a/pc-bios/s390-ccw/time.h b/pc-bios/s390-ccw/time.h
new file mode 100644
index 0000000000..ed6d982371
--- /dev/null
+++ b/pc-bios/s390-ccw/time.h
@@ -0,0 +1,23 @@
+#ifndef TIME_H
+#define TIME_H
+
+static inline u64 get_clock(void)
+{
+    u64 r;
+
+    asm volatile("stck %0" : "=Q" (r) : : "cc");
+    return r;
+}
+
+static inline u64 get_time_ms(void)
+{
+    /* Bit 51 is incremented each microsecond */
+    return (get_clock() >> 12) / 1000;
+}
+
+static inline u64 get_time_seconds(void)
+{
+    return get_time_ms() / 1000;
+}
+
+#endif
diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
index ff7f4dad25..4de03728bb 100644
--- a/pc-bios/s390-ccw/virtio-net.c
+++ b/pc-bios/s390-ccw/virtio-net.c
@@ -19,6 +19,7 @@
 #include <ethernet.h>
 #include "s390-ccw.h"
 #include "virtio.h"
+#include "time.h"
 
 #ifndef DEBUG_VIRTIO_NET
 #define DEBUG_VIRTIO_NET 0
diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
index 4fe4b9d261..0620651da8 100644
--- a/pc-bios/s390-ccw/virtio-scsi.c
+++ b/pc-bios/s390-ccw/virtio-scsi.c
@@ -14,6 +14,7 @@
 #include "virtio.h"
 #include "scsi.h"
 #include "virtio-scsi.h"
+#include "time.h"
 
 static ScsiDevice default_scsi_device;
 static VirtioScsiCmdReq req;
diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
index fb40ca9828..43717b83d7 100644
--- a/pc-bios/s390-ccw/virtio.c
+++ b/pc-bios/s390-ccw/virtio.c
@@ -15,6 +15,7 @@
 #include "virtio-scsi.h"
 #include "bswap.h"
 #include "helper.h"
+#include "time.h"
 
 #define VRING_WAIT_REPLY_TIMEOUT 30
 
@@ -157,19 +158,6 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
     }
 }
 
-u64 get_clock(void)
-{
-    u64 r;
-
-    asm volatile("stck %0" : "=Q" (r) : : "cc");
-    return r;
-}
-
-ulong get_second(void)
-{
-    return (get_clock() >> 12) / 1000000;
-}
-
 int vr_poll(VRing *vr)
 {
     if (vr->used->idx == vr->used_idx) {
@@ -194,7 +182,7 @@ int vr_poll(VRing *vr)
  */
 int vring_wait_reply(void)
 {
-    ulong target_second = get_second() + vdev.wait_reply_timeout;
+    ulong target_second = get_time_seconds() + vdev.wait_reply_timeout;
 
     /* Wait for any queue to be updated by the host */
     do {
@@ -207,7 +195,7 @@ int vring_wait_reply(void)
         if (r) {
             return 0;
         }
-    } while (!vdev.wait_reply_timeout || (get_second() < target_second));
+    } while (!vdev.wait_reply_timeout || (get_time_seconds() < target_second));
 
     return 1;
 }
-- 
2.25.1

Re: [PATCH v3 2/9] pc-bios: s390x: Consolidate timing functions into time.h

Posted by Christian Borntraeger 5 years, 8 months ago


On 27.05.20 09:49, Janosch Frank wrote:
> Let's consolidate timing related functions into one header.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  pc-bios/s390-ccw/menu.c        |  1 +
>  pc-bios/s390-ccw/netmain.c     | 15 +++------------
>  pc-bios/s390-ccw/s390-ccw.h    |  2 --
>  pc-bios/s390-ccw/time.h        | 23 +++++++++++++++++++++++

what happened to Thomas comment about a header with the same name from SLOF.
I think it is safer to renamme this one to s390-time.h?

>  pc-bios/s390-ccw/virtio-net.c  |  1 +
>  pc-bios/s390-ccw/virtio-scsi.c |  1 +
>  pc-bios/s390-ccw/virtio.c      | 18 +++---------------
>  7 files changed, 32 insertions(+), 29 deletions(-)
>  create mode 100644 pc-bios/s390-ccw/time.h
> 
> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
> index ce3815b201..7925c33248 100644
> --- a/pc-bios/s390-ccw/menu.c
> +++ b/pc-bios/s390-ccw/menu.c
> @@ -12,6 +12,7 @@
>  #include "libc.h"
>  #include "s390-ccw.h"
>  #include "sclp.h"
> +#include "time.h"
>  
>  #define KEYCODE_NO_INP '\0'
>  #define KEYCODE_ESCAPE '\033'
> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
> index 309ffa30d9..73def8de4f 100644
> --- a/pc-bios/s390-ccw/netmain.c
> +++ b/pc-bios/s390-ccw/netmain.c
> @@ -35,6 +35,7 @@
>  #include "s390-ccw.h"
>  #include "cio.h"
>  #include "virtio.h"
> +#include "time.h"
>  
>  #define DEFAULT_BOOT_RETRIES 10
>  #define DEFAULT_TFTP_RETRIES 20
> @@ -57,24 +58,14 @@ static SubChannelId net_schid = { .one = 1 };
>  static uint8_t mac[6];
>  static uint64_t dest_timer;
>  
> -static uint64_t get_timer_ms(void)
> -{
> -    uint64_t clk;
> -
> -    asm volatile(" stck %0 " : : "Q"(clk) : "memory");
> -
> -    /* Bit 51 is incremented each microsecond */
> -    return (clk >> (63 - 51)) / 1000;
> -}
> -
>  void set_timer(int val)
>  {
> -    dest_timer = get_timer_ms() + val;
> +    dest_timer = get_time_ms() + val;
>  }
>  
>  int get_timer(void)
>  {
> -    return dest_timer - get_timer_ms();
> +    return dest_timer - get_time_ms();
>  }
>  
>  int get_sec_ticks(void)
> diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
> index 21f27e7990..9514b76596 100644
> --- a/pc-bios/s390-ccw/s390-ccw.h
> +++ b/pc-bios/s390-ccw/s390-ccw.h
> @@ -74,8 +74,6 @@ unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
>  bool virtio_is_supported(SubChannelId schid);
>  void virtio_blk_setup_device(SubChannelId schid);
>  int virtio_read(ulong sector, void *load_addr);
> -u64 get_clock(void);
> -ulong get_second(void);
>  
>  /* bootmap.c */
>  void zipl_load(void);
> diff --git a/pc-bios/s390-ccw/time.h b/pc-bios/s390-ccw/time.h
> new file mode 100644
> index 0000000000..ed6d982371
> --- /dev/null
> +++ b/pc-bios/s390-ccw/time.h
> @@ -0,0 +1,23 @@
> +#ifndef TIME_H
> +#define TIME_H
> +
> +static inline u64 get_clock(void)
> +{
> +    u64 r;
> +
> +    asm volatile("stck %0" : "=Q" (r) : : "cc");
> +    return r;
> +}
> +
> +static inline u64 get_time_ms(void)
> +{
> +    /* Bit 51 is incremented each microsecond */
> +    return (get_clock() >> 12) / 1000;
> +}
> +
> +static inline u64 get_time_seconds(void)
> +{
> +    return get_time_ms() / 1000;
> +}
> +
> +#endif
> diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
> index ff7f4dad25..4de03728bb 100644
> --- a/pc-bios/s390-ccw/virtio-net.c
> +++ b/pc-bios/s390-ccw/virtio-net.c
> @@ -19,6 +19,7 @@
>  #include <ethernet.h>
>  #include "s390-ccw.h"
>  #include "virtio.h"
> +#include "time.h"
>  
>  #ifndef DEBUG_VIRTIO_NET
>  #define DEBUG_VIRTIO_NET 0
> diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
> index 4fe4b9d261..0620651da8 100644
> --- a/pc-bios/s390-ccw/virtio-scsi.c
> +++ b/pc-bios/s390-ccw/virtio-scsi.c
> @@ -14,6 +14,7 @@
>  #include "virtio.h"
>  #include "scsi.h"
>  #include "virtio-scsi.h"
> +#include "time.h"
>  
>  static ScsiDevice default_scsi_device;
>  static VirtioScsiCmdReq req;
> diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
> index fb40ca9828..43717b83d7 100644
> --- a/pc-bios/s390-ccw/virtio.c
> +++ b/pc-bios/s390-ccw/virtio.c
> @@ -15,6 +15,7 @@
>  #include "virtio-scsi.h"
>  #include "bswap.h"
>  #include "helper.h"
> +#include "time.h"
>  
>  #define VRING_WAIT_REPLY_TIMEOUT 30
>  
> @@ -157,19 +158,6 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
>      }
>  }
>  
> -u64 get_clock(void)
> -{
> -    u64 r;
> -
> -    asm volatile("stck %0" : "=Q" (r) : : "cc");
> -    return r;
> -}
> -
> -ulong get_second(void)
> -{
> -    return (get_clock() >> 12) / 1000000;
> -}
> -
>  int vr_poll(VRing *vr)
>  {
>      if (vr->used->idx == vr->used_idx) {
> @@ -194,7 +182,7 @@ int vr_poll(VRing *vr)
>   */
>  int vring_wait_reply(void)
>  {
> -    ulong target_second = get_second() + vdev.wait_reply_timeout;
> +    ulong target_second = get_time_seconds() + vdev.wait_reply_timeout;
>  
>      /* Wait for any queue to be updated by the host */
>      do {
> @@ -207,7 +195,7 @@ int vring_wait_reply(void)
>          if (r) {
>              return 0;
>          }
> -    } while (!vdev.wait_reply_timeout || (get_second() < target_second));
> +    } while (!vdev.wait_reply_timeout || (get_time_seconds() < target_second));
>  
>      return 1;
>  }
>

Re: [PATCH v3 2/9] pc-bios: s390x: Consolidate timing functions into time.h

Posted by Janosch Frank 5 years, 8 months ago

On 5/27/20 10:36 AM, Christian Borntraeger wrote:
> 
> 
> On 27.05.20 09:49, Janosch Frank wrote:
>> Let's consolidate timing related functions into one header.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  pc-bios/s390-ccw/menu.c        |  1 +
>>  pc-bios/s390-ccw/netmain.c     | 15 +++------------
>>  pc-bios/s390-ccw/s390-ccw.h    |  2 --
>>  pc-bios/s390-ccw/time.h        | 23 +++++++++++++++++++++++
> 
> what happened to Thomas comment about a header with the same name from SLOF.
> I think it is safer to renamme this one to s390-time.h?

I extensively talked with Thomas about that on IRC and it doesn't seem
to be a problem right now. Since I got no answer to my mail asking if
s390-time.h would be better and he said I don't need to change it when
it works, I just left it like it was and waited for more comments.

> 
>>  pc-bios/s390-ccw/virtio-net.c  |  1 +
>>  pc-bios/s390-ccw/virtio-scsi.c |  1 +
>>  pc-bios/s390-ccw/virtio.c      | 18 +++---------------
>>  7 files changed, 32 insertions(+), 29 deletions(-)
>>  create mode 100644 pc-bios/s390-ccw/time.h
>>
>> diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
>> index ce3815b201..7925c33248 100644
>> --- a/pc-bios/s390-ccw/menu.c
>> +++ b/pc-bios/s390-ccw/menu.c
>> @@ -12,6 +12,7 @@
>>  #include "libc.h"
>>  #include "s390-ccw.h"
>>  #include "sclp.h"
>> +#include "time.h"
>>  
>>  #define KEYCODE_NO_INP '\0'
>>  #define KEYCODE_ESCAPE '\033'
>> diff --git a/pc-bios/s390-ccw/netmain.c b/pc-bios/s390-ccw/netmain.c
>> index 309ffa30d9..73def8de4f 100644
>> --- a/pc-bios/s390-ccw/netmain.c
>> +++ b/pc-bios/s390-ccw/netmain.c
>> @@ -35,6 +35,7 @@
>>  #include "s390-ccw.h"
>>  #include "cio.h"
>>  #include "virtio.h"
>> +#include "time.h"
>>  
>>  #define DEFAULT_BOOT_RETRIES 10
>>  #define DEFAULT_TFTP_RETRIES 20
>> @@ -57,24 +58,14 @@ static SubChannelId net_schid = { .one = 1 };
>>  static uint8_t mac[6];
>>  static uint64_t dest_timer;
>>  
>> -static uint64_t get_timer_ms(void)
>> -{
>> -    uint64_t clk;
>> -
>> -    asm volatile(" stck %0 " : : "Q"(clk) : "memory");
>> -
>> -    /* Bit 51 is incremented each microsecond */
>> -    return (clk >> (63 - 51)) / 1000;
>> -}
>> -
>>  void set_timer(int val)
>>  {
>> -    dest_timer = get_timer_ms() + val;
>> +    dest_timer = get_time_ms() + val;
>>  }
>>  
>>  int get_timer(void)
>>  {
>> -    return dest_timer - get_timer_ms();
>> +    return dest_timer - get_time_ms();
>>  }
>>  
>>  int get_sec_ticks(void)
>> diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
>> index 21f27e7990..9514b76596 100644
>> --- a/pc-bios/s390-ccw/s390-ccw.h
>> +++ b/pc-bios/s390-ccw/s390-ccw.h
>> @@ -74,8 +74,6 @@ unsigned long virtio_load_direct(ulong rec_list1, ulong rec_list2,
>>  bool virtio_is_supported(SubChannelId schid);
>>  void virtio_blk_setup_device(SubChannelId schid);
>>  int virtio_read(ulong sector, void *load_addr);
>> -u64 get_clock(void);
>> -ulong get_second(void);
>>  
>>  /* bootmap.c */
>>  void zipl_load(void);
>> diff --git a/pc-bios/s390-ccw/time.h b/pc-bios/s390-ccw/time.h
>> new file mode 100644
>> index 0000000000..ed6d982371
>> --- /dev/null
>> +++ b/pc-bios/s390-ccw/time.h
>> @@ -0,0 +1,23 @@
>> +#ifndef TIME_H
>> +#define TIME_H
>> +
>> +static inline u64 get_clock(void)
>> +{
>> +    u64 r;
>> +
>> +    asm volatile("stck %0" : "=Q" (r) : : "cc");
>> +    return r;
>> +}
>> +
>> +static inline u64 get_time_ms(void)
>> +{
>> +    /* Bit 51 is incremented each microsecond */
>> +    return (get_clock() >> 12) / 1000;
>> +}
>> +
>> +static inline u64 get_time_seconds(void)
>> +{
>> +    return get_time_ms() / 1000;
>> +}
>> +
>> +#endif
>> diff --git a/pc-bios/s390-ccw/virtio-net.c b/pc-bios/s390-ccw/virtio-net.c
>> index ff7f4dad25..4de03728bb 100644
>> --- a/pc-bios/s390-ccw/virtio-net.c
>> +++ b/pc-bios/s390-ccw/virtio-net.c
>> @@ -19,6 +19,7 @@
>>  #include <ethernet.h>
>>  #include "s390-ccw.h"
>>  #include "virtio.h"
>> +#include "time.h"
>>  
>>  #ifndef DEBUG_VIRTIO_NET
>>  #define DEBUG_VIRTIO_NET 0
>> diff --git a/pc-bios/s390-ccw/virtio-scsi.c b/pc-bios/s390-ccw/virtio-scsi.c
>> index 4fe4b9d261..0620651da8 100644
>> --- a/pc-bios/s390-ccw/virtio-scsi.c
>> +++ b/pc-bios/s390-ccw/virtio-scsi.c
>> @@ -14,6 +14,7 @@
>>  #include "virtio.h"
>>  #include "scsi.h"
>>  #include "virtio-scsi.h"
>> +#include "time.h"
>>  
>>  static ScsiDevice default_scsi_device;
>>  static VirtioScsiCmdReq req;
>> diff --git a/pc-bios/s390-ccw/virtio.c b/pc-bios/s390-ccw/virtio.c
>> index fb40ca9828..43717b83d7 100644
>> --- a/pc-bios/s390-ccw/virtio.c
>> +++ b/pc-bios/s390-ccw/virtio.c
>> @@ -15,6 +15,7 @@
>>  #include "virtio-scsi.h"
>>  #include "bswap.h"
>>  #include "helper.h"
>> +#include "time.h"
>>  
>>  #define VRING_WAIT_REPLY_TIMEOUT 30
>>  
>> @@ -157,19 +158,6 @@ void vring_send_buf(VRing *vr, void *p, int len, int flags)
>>      }
>>  }
>>  
>> -u64 get_clock(void)
>> -{
>> -    u64 r;
>> -
>> -    asm volatile("stck %0" : "=Q" (r) : : "cc");
>> -    return r;
>> -}
>> -
>> -ulong get_second(void)
>> -{
>> -    return (get_clock() >> 12) / 1000000;
>> -}
>> -
>>  int vr_poll(VRing *vr)
>>  {
>>      if (vr->used->idx == vr->used_idx) {
>> @@ -194,7 +182,7 @@ int vr_poll(VRing *vr)
>>   */
>>  int vring_wait_reply(void)
>>  {
>> -    ulong target_second = get_second() + vdev.wait_reply_timeout;
>> +    ulong target_second = get_time_seconds() + vdev.wait_reply_timeout;
>>  
>>      /* Wait for any queue to be updated by the host */
>>      do {
>> @@ -207,7 +195,7 @@ int vring_wait_reply(void)
>>          if (r) {
>>              return 0;
>>          }
>> -    } while (!vdev.wait_reply_timeout || (get_second() < target_second));
>> +    } while (!vdev.wait_reply_timeout || (get_time_seconds() < target_second));
>>  
>>      return 1;
>>  }
>>