[Qemu-devel] [PATCH] replacing gettimeofday with clock_gettime in hw/serial

[Qemu-devel] [PATCH] replacing gettimeofday with clock_gettime in hw/serial

[Stefano Stabellini]

This patch substitutes gettimeofday with clock_gettime in hw/serial.c.

gettimeofday is unsafe because can lead to incorrect behaviors if the user
changes the system's date.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

diff --git a/hw/serial.c b/hw/serial.c
index d49b5de..6ae3b20 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -155,8 +155,8 @@ struct SerialState {
    doesn't kill dom0.  Simple token bucket.  If we get some actual
    data from the user, instantly refil the bucket. */
 
-/* How long it takes to generate a token, in microseconds. */
-#define TOKEN_PERIOD 1000
+/* How long it takes to generate a token, in nanoseconds. */
+#define TOKEN_PERIOD 1000000
 /* Maximum and initial size of token bucket */
 #define TOKENS_MAX 100000
 
@@ -279,47 +279,47 @@ static void serial_update_parameters(SerialState *s)
 
 static void serial_get_token(void)
 {
-    static struct timeval last_refil_time;
+    static struct timespec last_refil_time;
     static int started;
 
     assert(tokens_avail >= 0);
     if (!tokens_avail) {
-	struct timeval delta, now;
-	int generated;
+	struct timespec delta, now;
+	long generated;
 
 	if (!started) {
-	    gettimeofday(&last_refil_time, NULL);
+	    clock_gettime(CLOCK_MONOTONIC, &last_refil_time);
 	    tokens_avail = TOKENS_MAX;
 	    started = 1;
 	    return;
 	}
     retry:
-	gettimeofday(&now, NULL);
+	clock_gettime(CLOCK_MONOTONIC, &now);
 	delta.tv_sec = now.tv_sec - last_refil_time.tv_sec;
-	delta.tv_usec = now.tv_usec - last_refil_time.tv_usec;
-	if (delta.tv_usec < 0) {
-	    delta.tv_usec += 1000000;
+	delta.tv_nsec = now.tv_nsec - last_refil_time.tv_nsec;
+	if (delta.tv_nsec < 0) {
+	    delta.tv_nsec += 1000000000;
 	    delta.tv_sec--;
 	}
-	assert(delta.tv_usec >= 0 && delta.tv_sec >= 0);
-	if (delta.tv_usec < TOKEN_PERIOD) {
+	assert(delta.tv_nsec >= 0 && delta.tv_sec >= 0);
+	if (delta.tv_nsec < TOKEN_PERIOD) {
 	    struct timespec ts;
 	    /* Wait until at least one token is available. */
-	    ts.tv_sec = TOKEN_PERIOD / 1000000;
-	    ts.tv_nsec = (TOKEN_PERIOD % 1000000) * 1000;
+	    ts.tv_sec = TOKEN_PERIOD / 1000000000;
+	    ts.tv_nsec = TOKEN_PERIOD % 1000000000;
 	    while (nanosleep(&ts, &ts) < 0 && errno == EINTR)
 		;
 	    goto retry;
 	}
-	generated = (delta.tv_sec * 1000000) / TOKEN_PERIOD;
+	generated = (delta.tv_sec * 1000000000) / TOKEN_PERIOD;
 	generated +=
-	    ((delta.tv_sec * 1000000) % TOKEN_PERIOD + delta.tv_usec) / TOKEN_PERIOD;
+	    ((delta.tv_sec * 1000000000) % TOKEN_PERIOD + delta.tv_nsec) / TOKEN_PERIOD;
 	assert(generated > 0);
 
-	last_refil_time.tv_usec += (generated * TOKEN_PERIOD) % 1000000;
-	last_refil_time.tv_sec  += last_refil_time.tv_usec / 1000000;
-	last_refil_time.tv_usec %= 1000000;
-	last_refil_time.tv_sec  += (generated * TOKEN_PERIOD) / 1000000;
+	last_refil_time.tv_nsec += (generated * TOKEN_PERIOD) % 1000000000;
+	last_refil_time.tv_sec  += last_refil_time.tv_nsec / 1000000000;
+	last_refil_time.tv_nsec %= 1000000000;
+        last_refil_time.tv_sec  += (generated * TOKEN_PERIOD) / 1000000000;
 	if (generated > TOKENS_MAX)
 	    generated = TOKENS_MAX;
 	tokens_avail = generated;

Re: [Qemu-devel] [PATCH] replacing gettimeofday with clock_gettime in hw/serial

[Paul Brook]

On Wednesday 30 July 2008, Stefano Stabellini wrote:
> This patch substitutes gettimeofday with clock_gettime in hw/serial.c.
>
> gettimeofday is unsafe because can lead to incorrect behaviors if the user
> changes the system's date.

This code is just plain wrong to start with.  Devices should not depend on 
host time.

Paul

Re: [Qemu-devel] [PATCH] replacing gettimeofday with clock_gettime in hw/serial

[Anthony Liguori]

Paul Brook wrote:
> On Wednesday 30 July 2008, Stefano Stabellini wrote:
>   
>> This patch substitutes gettimeofday with clock_gettime in hw/serial.c.
>>
>> gettimeofday is unsafe because can lead to incorrect behaviors if the user
>> changes the system's date.
>>     
>
> This code is just plain wrong to start with.  Devices should not depend on 
> host time.
>   

Because host time continues even when the guest is stopped.  It should 
instead be based on the vm_clock.

I don't think it works either.  I still have gotten messages in the 
guest about too many interrupts on the serial port.

Regards,

Anthony Liguori

> Paul
>
>
>   

Re: [Qemu-devel] [PATCH] replacing gettimeofday with clock_gettime in hw/serial

[Stefano Stabellini]

Anthony Liguori wrote:

> Paul Brook wrote:
>> On Wednesday 30 July 2008, Stefano Stabellini wrote:
>>  
>>> This patch substitutes gettimeofday with clock_gettime in hw/serial.c.
>>>
>>> gettimeofday is unsafe because can lead to incorrect behaviors if the
>>> user
>>> changes the system's date.
>>>     
>>
>> This code is just plain wrong to start with.  Devices should not
>> depend on host time.
>>   
> 
> Because host time continues even when the guest is stopped.  It should
> instead be based on the vm_clock.
> 
> I don't think it works either.  I still have gotten messages in the
> guest about too many interrupts on the serial port.
> 

I am sorry, I did a mistake: I have just realized that the serial
emulation code in qemu and ioemu under xen are so different that this
patch doesn't even apply to qemu.
I am so used to send the same patch twice that I haven't realize that
until now.
I'll try to merge them, if it makes sense.