Re: [Qemu-devel] [PATCH][RFC] Fix bugs in the ATAPI cdrom driver
On 17:42 Fri 17 Aug 2007, Matthew Kent wrote: > On Fri, 2007-17-08 at 16:43 -0700, Brandon Philips wrote: > > The new libata-eh in the Linux kernel is throwing a fit over the QEMU > > cdrom device for two reasons: > > Nice thanks, was suffering from this issue as well. > > Tested with some linux 2.6.23-rc3 guests, looks great now. Great. I would like to see someone from the QEMU project sign off on the correctness of the code too before we carry the patch in our package. Thanks for testing! Brandon
[Qemu-devel] qemu/hw pcnet.c
CVSROOT:/sources/qemu Module name:qemu Changes by: Thiemo Seufer 07/08/18 13:08:30 Modified files: hw : pcnet.c Log message: Remove obsolete comment. CVSWeb URLs: http://cvs.savannah.gnu.org/viewcvs/qemu/hw/pcnet.c?cvsroot=qemu&r1=1.16&r2=1.17
[Qemu-devel] Re: [kvm-devel] [PATCH 0/4] Rework alarm timer infrastrucure - take 2
On Sat, 2007-08-18 at 01:11 +0200, Luca Tettamanti wrote: > Hello, > in reply to this mail I will send a serie of 4 patches that cleans up and > expands the alarm timer handling in QEMU. Patches have been rebased on QEMU > CVS. > > Patch 1 is mostly a cleanup of the existing code; instead of having multiple > #ifdefs to handle different timers scattered all over the code I've created a > modular infrastructure where each timer type is self-contained and generic > code > is more readable. The resulting code is functionally equivalent to the old > one. > > Patch 2 implements the "-clock" command line option proposed by Daniel > Berrange > and Avi Kivity. By default QEMU tries RTC and then falls back to unix timer; > user can override the order of the timer through this options. Syntax is > pretty > simple: -clock timer1,timer2,etc. (QEMU will pick the first one that works). > > Patch 3 adds support for HPET under Linux (which is basically my old patch). > As > suggested HPET takes precedence over other timers, but of course this can be > overridden. > > Patch 4 introduces "dynticks" timer source; patch is mostly based on the work > Dan Kenigsberg. dynticks is now the default alarm timer. Why do you guard dynticks with #ifdef? Is there any reason why you wouldn't want to use dynticks? Regards, Anthony Liguori > Luca
[Qemu-devel] RE: [kvm-devel] [PATCH 0/4] Rework alarm timer infrastrucure - take2
>> Hello, >> in reply to this mail I will send a serie of 4 patches that cleans up >and >> expands the alarm timer handling in QEMU. Patches have been rebased on >QEMU >> CVS. >> >> Patch 1 is mostly a cleanup of the existing code; instead of having >multiple >> #ifdefs to handle different timers scattered all over the code I've >created a >> modular infrastructure where each timer type is self-contained and >generic code >> is more readable. The resulting code is functionally equivalent to the >old one. >> >> Patch 2 implements the "-clock" command line option proposed by Daniel >Berrange >> and Avi Kivity. By default QEMU tries RTC and then falls back to unix >timer; >> user can override the order of the timer through this options. Syntax >is pretty >> simple: -clock timer1,timer2,etc. (QEMU will pick the first one that >works). >> >> Patch 3 adds support for HPET under Linux (which is basically my old >patch). As >> suggested HPET takes precedence over other timers, but of course this >can be >> overridden. >> >> Patch 4 introduces "dynticks" timer source; patch is mostly based on >the work >> Dan Kenigsberg. dynticks is now the default alarm timer. > >Why do you guard dynticks with #ifdef? Is there any reason why you >wouldn't want to use dynticks? I think too that it's should be the default. There is no regression in performance nor behaviour with this option. We didn't test qemu dyn-tick with kernels that don't have high-res-timer+dyn-tick. In this case the dyn-tick minimum res will be 1msec. I believe it should work ok since this is the case without any dyn-tick. So, I join Anthony's vote. > >Regards, > >Anthony Liguori > >> Luca > > >--- - >- >This SF.net email is sponsored by: Splunk Inc. >Still grepping through log files to find problems? Stop. >Now Search log events and configuration files using AJAX and a browser. >Download your FREE copy of Splunk now >> http://get.splunk.com/ >___ >kvm-devel mailing list >[EMAIL PROTECTED] >https://lists.sourceforge.net/lists/listinfo/kvm-devel
[Qemu-devel] [PATCH] usb devices with multiple configurations/interfaces
Hi,
as the patch provided in [1] did not work for me when patching against
qemu-0.9.0 I created a new diff which should work with the official
qemu-0.9.0 sources.
So if you struggle with an error like "usb_host: only one interface
supported" this might help you out.
You can find some more information and also (S)RPM packages at [2].
Bye,
Andreas Winkelbauer
[1] http://lists.gnu.org/archive/html/qemu-devel/2007-06/msg00331.html
[2] http://stud4.tuwien.ac.at/~e0425650/html/linux-qemu.html
--- usb-linux.c 2007-08-16 14:55:02.0 +0200
+++ usb-linux.c 2007-08-16 14:54:37.0 +0200
@@ -46,8 +46,7 @@ typedef int USBScanFunc(void *opaque, in
static int usb_host_find_device(int *pbus_num, int *paddr,
char *product_name, int product_name_size,
const char *devname);
-
-//#define DEBUG
+#define DEBUG
#define USBDEVFS_PATH "/proc/bus/usb"
#define PRODUCT_NAME_SZ 32
@@ -55,8 +54,88 @@ static int usb_host_find_device(int *pbu
typedef struct USBHostDevice {
USBDevice dev;
int fd;
+int configuration;
+uint8_t descr[1024];
+int descr_len;
} USBHostDevice;
+static int usb_host_update_interfaces(USBHostDevice *dev, int configuration)
+{
+int dev_descr_len, config_descr_len;
+int interface, nb_interfaces, nb_configurations;
+int ret, i;
+
+if (configuration == 0) // address state - ignore
+return 1;
+
+i = 0;
+dev_descr_len = dev->descr[0];
+if (dev_descr_len > dev->descr_len)
+goto fail;
+nb_configurations = dev->descr[17];
+
+i += dev_descr_len;
+ while (i < dev->descr_len) {
+#ifdef DEBUG
+printf("i is %d, descr_len is %d, dl %d, dt %d\n", i, dev->descr_len,
+ dev->descr[i], dev->descr[i+1]);
+#endif
+if (dev->descr[i+1] != USB_DT_CONFIG) {
+i += dev->descr[i];
+continue;
+}
+config_descr_len = dev->descr[i];
+
+if (configuration == dev->descr[i + 5])
+break;
+
+i += config_descr_len;
+}
+
+if (i >= dev->descr_len) {
+printf("usb_host: error - device has no matching configuration\n");
+goto fail;
+}
+nb_interfaces = dev->descr[i + 4];
+
+#ifdef USBDEVFS_DISCONNECT
+/* earlier Linux 2.4 do not support that */
+{
+struct usbdevfs_ioctl ctrl;
+for (interface = 0; interface < nb_interfaces; interface++) {
+ctrl.ioctl_code = USBDEVFS_DISCONNECT;
+ctrl.ifno = interface;
+ret = ioctl(dev->fd, USBDEVFS_IOCTL, &ctrl);
+if (ret < 0 && errno != ENODATA) {
+perror("USBDEVFS_DISCONNECT");
+goto fail;
+}
+}
+}
+#endif
+
+/* XXX: only grab if all interfaces are free */
+for (interface = 0; interface < nb_interfaces; interface++) {
+ret = ioctl(dev->fd, USBDEVFS_CLAIMINTERFACE, &interface);
+if (ret < 0) {
+if (errno == EBUSY) {
+fprintf(stderr, "usb_host: warning - device already grabbed\n");
+} else {
+perror("USBDEVFS_CLAIMINTERFACE");
+}
+fail:
+return 0;
+}
+}
+
+#ifdef DEBUG
+printf("usb_host: %d interfaces claimed for configuration %d\n", nb_interfaces,
+ configuration);
+#endif
+
+return 1;
+}
+
static void usb_host_handle_reset(USBDevice *dev)
{
#if 0
@@ -85,13 +164,25 @@ static int usb_host_handle_control(USBDe
{
USBHostDevice *s = (USBHostDevice *)dev;
struct usb_ctrltransfer ct;
+int intf_update_required = 0;
int ret;
if (request == (DeviceOutRequest | USB_REQ_SET_ADDRESS)) {
/* specific SET_ADDRESS support */
dev->addr = value;
return 0;
+} else if (request == (DeviceOutRequest | USB_REQ_SET_CONFIGURATION)) {
+#ifdef DEBUG
+printf("usb_host_handle_control: SET_CONFIGURATION request - config %d\n",
+ value & 0xff);
+#endif
+if (s->configuration != (value & 0xff)) {
+s->configuration = (value & 0xff);
+intf_update_required = 1;
+}
+goto do_request;
} else {
+do_request:
ct.bRequestType = request >> 8;
ct.bRequest = request;
ct.wValue = value;
@@ -108,6 +199,12 @@ static int usb_host_handle_control(USBDe
return USB_RET_STALL;
}
} else {
+if (intf_update_required) {
+#ifdef DEBUG
+printf("usb_host_handle_control: updating interfaces\n");
+#endif
+usb_host_update_interfaces(s, value & 0xff);
+}
return ret;
}
}
@@ -148,15 +245,17 @@ static int usb_host_handle_data(USBDevic
/* XXX: exclude high speed devices or implement EHCI */
USBDevice *usb_host_device_open(const char *devname)
{
-int fd, interface, ret, i;
-USBHostDevice *dev;
+int fd = -1, ret;
+U
[Qemu-devel] Re: [kvm-devel] [PATCH 0/4] Rework alarm timer infrastrucure - take2
Dor Laor ha scritto:
> >> Hello,
> >> in reply to this mail I will send a serie of 4 patches that cleans up
> >and
> >> expands the alarm timer handling in QEMU. Patches have been rebased
> on
> >QEMU
> >> CVS.
> >>
> >> Patch 1 is mostly a cleanup of the existing code; instead of having
> >multiple
> >> #ifdefs to handle different timers scattered all over the code I've
> >created a
> >> modular infrastructure where each timer type is self-contained and
> >generic code
> >> is more readable. The resulting code is functionally equivalent to
> the
> >old one.
> >>
> >> Patch 2 implements the "-clock" command line option proposed by
> Daniel
> >Berrange
> >> and Avi Kivity. By default QEMU tries RTC and then falls back to unix
> >timer;
> >> user can override the order of the timer through this options. Syntax
> >is pretty
> >> simple: -clock timer1,timer2,etc. (QEMU will pick the first one that
> >works).
> >>
> >> Patch 3 adds support for HPET under Linux (which is basically my old
> >patch). As
> >> suggested HPET takes precedence over other timers, but of course this
> >can be
> >> overridden.
> >>
> >> Patch 4 introduces "dynticks" timer source; patch is mostly based on
> >the work
> >> Dan Kenigsberg. dynticks is now the default alarm timer.
> >
> >Why do you guard dynticks with #ifdef? Is there any reason why you
> >wouldn't want to use dynticks?
>
> I think too that it's should be the default.
> There is no regression in performance nor behaviour with this option.
Ok, I've updated the patch. It was pretty easy to implement the same
feature for win32 (slightly tested inside a winxp VM).
> We didn't test qemu dyn-tick with kernels that don't have
> high-res-timer+dyn-tick.
I did ;)
> In this case the dyn-tick minimum res will be 1msec. I believe it should
> work ok since this is the case without any dyn-tick.
Actually minimum resolution depends on host HZ setting, but - yes -
essentially you have the same behaviour of the "unix" timer, plus the
overhead of reprogramming the timer.
Add support for dynamic ticks.
If the the dynticks alarm timer is used qemu does not attempt to generate
SIGALRM at a constant rate. Rather, the system timer is set to generate SIGALRM
only when it is needed. Dynticks timer reduces the number of SIGALRMs sent to
idle dynamic-ticked guests.
Original patch from Dan Kenigsberg <[EMAIL PROTECTED]>
Signed-off-by: Luca Tettamanti <[EMAIL PROTECTED]>
---
vl.c | 178 +++
1 file changed, 170 insertions(+), 8 deletions(-)
Index: qemu/vl.c
===
--- qemu.orig/vl.c 2007-08-18 23:23:47.0 +0200
+++ qemu/vl.c 2007-08-18 23:23:53.0 +0200
@@ -784,12 +784,31 @@
struct qemu_alarm_timer {
char const *name;
+unsigned int flags;
int (*start)(struct qemu_alarm_timer *t);
void (*stop)(struct qemu_alarm_timer *t);
+void (*rearm)(struct qemu_alarm_timer *t);
void *priv;
};
+#define ALARM_FLAG_DYNTICKS 0x1
+
+static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
+{
+return t->flags & ALARM_FLAG_DYNTICKS;
+}
+
+static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t) {
+if (!alarm_has_dynticks(t))
+return;
+
+t->rearm(t);
+}
+
+/* TODO: MIN_TIMER_REARM_US should be optimized */
+#define MIN_TIMER_REARM_US 250
+
static struct qemu_alarm_timer *alarm_timer;
#ifdef _WIN32
@@ -802,12 +821,17 @@
static int win32_start_timer(struct qemu_alarm_timer *t);
static void win32_stop_timer(struct qemu_alarm_timer *t);
+static void win32_rearm_timer(struct qemu_alarm_timer *t);
#else
static int unix_start_timer(struct qemu_alarm_timer *t);
static void unix_stop_timer(struct qemu_alarm_timer *t);
+static int dynticks_start_timer(struct qemu_alarm_timer *t);
+static void dynticks_stop_timer(struct qemu_alarm_timer *t);
+static void dynticks_rearm_timer(struct qemu_alarm_timer *t);
+
#ifdef __linux__
static int hpet_start_timer(struct qemu_alarm_timer *t);
@@ -816,21 +840,23 @@
static int rtc_start_timer(struct qemu_alarm_timer *t);
static void rtc_stop_timer(struct qemu_alarm_timer *t);
-#endif
+#endif /* __linux__ */
#endif /* _WIN32 */
static struct qemu_alarm_timer alarm_timers[] = {
+#ifndef _WIN32
+{"dynticks", ALARM_FLAG_DYNTICKS, dynticks_start_timer,
dynticks_stop_timer, dynticks_rearm_timer, NULL},
#ifdef __linux__
/* HPET - if available - is preferred */
-{"hpet", hpet_start_timer, hpet_stop_timer, NULL},
+{"hpet", 0, hpet_start_timer, hpet_stop_timer, NULL, NULL},
/* ...otherwise try RTC */
-{"rtc", rtc_start_timer, rtc_stop_timer, NULL},
+{"rtc", 0, rtc_start_timer, rtc_stop_timer, NULL, NULL},
#endif
-#ifndef _WIN32
-{"unix", unix_start_timer, unix_stop_timer, NULL},
+{"unix", 0, unix_start_timer, unix_stop_timer, NULL, NULL},
#else
-{"win32", win32_start_timer, win32_stop_timer, &alarm_win32_data
[Qemu-devel] Re: [kvm-devel] [PATCH 0/4] Rework alarm timer infrastrucure - take2
I think this is a really nice and important patch set. Just a couple
things:
On Sun, 2007-08-19 at 00:02 +0200, Luca Tettamanti wrote:
> > In this case the dyn-tick minimum res will be 1msec. I believe it should
> > work ok since this is the case without any dyn-tick.
>
> Actually minimum resolution depends on host HZ setting, but - yes -
> essentially you have the same behaviour of the "unix" timer, plus the
> overhead of reprogramming the timer.
Is this significant? At a high guest HZ, this is could be quite a lot
of additional syscalls right?
> Add support for dynamic ticks.
>
> If the the dynticks alarm timer is used qemu does not attempt to generate
> SIGALRM at a constant rate. Rather, the system timer is set to generate
> SIGALRM
> only when it is needed. Dynticks timer reduces the number of SIGALRMs sent to
> idle dynamic-ticked guests.
> Original patch from Dan Kenigsberg <[EMAIL PROTECTED]>
>
> Signed-off-by: Luca Tettamanti <[EMAIL PROTECTED]>
>
> ---
> vl.c | 178
> +++
> 1 file changed, 170 insertions(+), 8 deletions(-)
>
> Index: qemu/vl.c
> ===
> --- qemu.orig/vl.c2007-08-18 23:23:47.0 +0200
> +++ qemu/vl.c 2007-08-18 23:23:53.0 +0200
> @@ -784,12 +784,31 @@
>
> struct qemu_alarm_timer {
> char const *name;
> +unsigned int flags;
>
> int (*start)(struct qemu_alarm_timer *t);
> void (*stop)(struct qemu_alarm_timer *t);
> +void (*rearm)(struct qemu_alarm_timer *t);
> void *priv;
> };
>
> +#define ALARM_FLAG_DYNTICKS 0x1
> +
> +static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
> +{
> +return t->flags & ALARM_FLAG_DYNTICKS;
> +}
> +
> +static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t) {
The '{' should be on the next line.
The rest looks fine.
Regards,
Anthony Liguori
> +if (!alarm_has_dynticks(t))
> +return;
> +
> +t->rearm(t);
> +}
> +
> +/* TODO: MIN_TIMER_REARM_US should be optimized */
> +#define MIN_TIMER_REARM_US 250
> +
> static struct qemu_alarm_timer *alarm_timer;
>
> #ifdef _WIN32
> @@ -802,12 +821,17 @@
>
> static int win32_start_timer(struct qemu_alarm_timer *t);
> static void win32_stop_timer(struct qemu_alarm_timer *t);
> +static void win32_rearm_timer(struct qemu_alarm_timer *t);
>
> #else
>
> static int unix_start_timer(struct qemu_alarm_timer *t);
> static void unix_stop_timer(struct qemu_alarm_timer *t);
>
> +static int dynticks_start_timer(struct qemu_alarm_timer *t);
> +static void dynticks_stop_timer(struct qemu_alarm_timer *t);
> +static void dynticks_rearm_timer(struct qemu_alarm_timer *t);
> +
> #ifdef __linux__
>
> static int hpet_start_timer(struct qemu_alarm_timer *t);
> @@ -816,21 +840,23 @@
> static int rtc_start_timer(struct qemu_alarm_timer *t);
> static void rtc_stop_timer(struct qemu_alarm_timer *t);
>
> -#endif
> +#endif /* __linux__ */
>
> #endif /* _WIN32 */
>
> static struct qemu_alarm_timer alarm_timers[] = {
> +#ifndef _WIN32
> +{"dynticks", ALARM_FLAG_DYNTICKS, dynticks_start_timer,
> dynticks_stop_timer, dynticks_rearm_timer, NULL},
> #ifdef __linux__
> /* HPET - if available - is preferred */
> -{"hpet", hpet_start_timer, hpet_stop_timer, NULL},
> +{"hpet", 0, hpet_start_timer, hpet_stop_timer, NULL, NULL},
> /* ...otherwise try RTC */
> -{"rtc", rtc_start_timer, rtc_stop_timer, NULL},
> +{"rtc", 0, rtc_start_timer, rtc_stop_timer, NULL, NULL},
> #endif
> -#ifndef _WIN32
> -{"unix", unix_start_timer, unix_stop_timer, NULL},
> +{"unix", 0, unix_start_timer, unix_stop_timer, NULL, NULL},
> #else
> -{"win32", win32_start_timer, win32_stop_timer, &alarm_win32_data},
> +{"dynticks", ALARM_FLAG_DYNTICKS, win32_start_timer, win32_stop_timer,
> win32_rearm_timer, &alarm_win32_data},
> +{"win32", 0, win32_start_timer, win32_stop_timer, NULL,
> &alarm_win32_data},
> #endif
> {NULL, }
> };
> @@ -949,6 +975,8 @@
> }
> pt = &t->next;
> }
> +
> +qemu_rearm_alarm_timer(alarm_timer);
> }
>
> /* modify the current timer so that it will be fired when current_time
> @@ -1008,6 +1036,7 @@
> /* run the callback (the timer list can be modified) */
> ts->cb(ts->opaque);
> }
> +qemu_rearm_alarm_timer(alarm_timer);
> }
>
> int64_t qemu_get_clock(QEMUClock *clock)
> @@ -1115,7 +1144,8 @@
> last_clock = ti;
> }
> #endif
> -if (qemu_timer_expired(active_timers[QEMU_TIMER_VIRTUAL],
> +if (alarm_has_dynticks(alarm_timer) ||
> +qemu_timer_expired(active_timers[QEMU_TIMER_VIRTUAL],
> qemu_get_clock(vm_clock)) ||
> qemu_timer_expired(active_timers[QEMU_TIMER_REALTIME],
> qemu_get_clock(rt_clock))) {
> @@ -1136,6 +1166,27 @@
> }
> }
>
> +static uint64_t qemu_next_deadline(void) {
> +
