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

[Janosch Frank <frankja@linux.ibm.com>]

[-- Attachment #1.1: Type: text/plain, Size: 6022 bytes --]

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;
>>  }
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]