Skip to contentSkip to conversation

replacing gettimeofday with clock_gettime in hw/serial

PATCH In review Stefano Stabellini 3 participants 4 messages Jul 30, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 replacing gettimeofday with clock_gettime in hw/serial

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.

---

hw/serial.c +20 -20
index d49b5de..6ae3b20 100644
@@ -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;
#1 lore raw
Quoted 4 lines · 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.

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

Paul

#2 lore raw
Quoted 10 lines · Paul Brook
Quoted 5 lines · 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.

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

Quoted 4 lines

Paul

#3 lore raw
Quoted 14 lines · Anthony Liguori
Quoted 4 lines · Paul Brook
Quoted 2 lines · Stefano Stabellini

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

Quoted 8 lines
Quoted 4 lines

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.

Quoted 6 lines

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.