Re: [Qemu-devel] [PATCH v2] Release usb devices on shutdown and usb_del command
Hi, What about the existing callbacks? Could handle_destroy do? For hot-unplug it should do. --- a/vl.c +++ b/vl.c @@ -3914,6 +3914,7 @@ int main(int argc, char **argv, char **envp) main_loop(); quit_timers(); net_cleanup(); +usb_cleanup(); return 0; } Figure we'd have to clean up the qdev tree on exit. Gerd? Hmm, yes. Question is how to do that best. There is qdev_free(). Today this is used for hot-unplug only. Using it on exit() too could have unwanted guest-visible side effects as it doesn't just release ressources, but also unplugs the device if possible. Maybe it is better to add a exit notifier ... cheers, Gerd
Re: [Qemu-devel] [PATCH 3/3] Monitor: Drop QMP documentation from code
Luiz Capitulino writes: > Previous commit added the QMP/qmp-commands.txt file, which is a > copy of this information. This is no longer true. > While it's good to keep it near code, maintaining two copies of > the same information is too hard and has little benefit as we > don't expect client writers to consult the code to find how to > use a QMP command. > > Signed-off-by: Luiz Capitulino [...]
Re: [Qemu-devel] [PATCH v2] Release usb devices on shutdown and usb_del command
"David S. Ahern" writes: > On 05/19/2010 12:10 PM, Shahar Havivi wrote: >> When closig Vm or removing usb on guest via usb_del monitor command, >> qemu does not return the control to the host, the user have to >> unplug and plug the device in order to use it on the host. >> >> v2: >> added empty methods to usb-bsd and usb-stub. >> release usb devices when main is out. >> >> Signed-off-by: Shahar Havivi >> --- >> hw/usb-bus.c |4 >> hw/usb.h |2 ++ >> usb-bsd.c| 10 ++ >> usb-linux.c | 21 + >> usb-stub.c | 10 ++ >> vl.c |1 + >> 6 files changed, 48 insertions(+), 0 deletions(-) >> >> diff --git a/hw/usb-bus.c b/hw/usb-bus.c >> index b692503..75dc819 100644 >> --- a/hw/usb-bus.c >> +++ b/hw/usb-bus.c >> @@ -207,6 +207,10 @@ int usb_device_delete_addr(int busnr, int addr) >> return -1; >> dev = port->dev; >> >> +if (!strcmp(dev->info->usbdevice_name, "host")) { >> +usb_host_device_release(dev); >> +} >> + > > Shouldn't this be done through a callback -- say usbdevice_release > similar to usbdevice_init -- instead of embedding host specifics here? > You wouldn't need the bsd and stub stubs then. > > David What about the existing callbacks? Could handle_destroy do? Note: usbdevice_init() is not for general initialization, just for dealing with the legacy -usbdevice command line. >> qdev_free(&dev->qdev); >> return 0; >> } [...] >> diff --git a/usb-linux.c b/usb-linux.c >> index 88273ff..cea5b84 100644 >> --- a/usb-linux.c >> +++ b/usb-linux.c >> @@ -286,6 +286,27 @@ static void async_cancel(USBPacket *unused, void >> *opaque) >> } >> } >> >> +void usb_cleanup(void) >> +{ >> +struct USBHostDevice *s; >> + >> +QTAILQ_FOREACH(s, &hostdevs, next) { >> +if (s->fd != -1) { >> +usb_host_device_release((USBDevice*)s); >> +} >> +} >> +} >> + >> +int usb_host_device_release(USBDevice *dev) >> +{ >> +int ret; >> + >> +USBHostDevice *s = DO_UPCAST(USBHostDevice, dev, dev); >> +ret = ioctl(s->fd, USBDEVFS_RESET); >> + >> +return ret; >> +} >> + >> static int usb_host_claim_interfaces(USBHostDevice *dev, int configuration) >> { >> int dev_descr_len, config_descr_len; [...] >> diff --git a/vl.c b/vl.c >> index d77b47c..e3f4dc9 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -3914,6 +3914,7 @@ int main(int argc, char **argv, char **envp) >> main_loop(); >> quit_timers(); >> net_cleanup(); >> +usb_cleanup(); >> >> return 0; >> } Figure we'd have to clean up the qdev tree on exit. Gerd?
Re: [Qemu-devel] [RFC PATCH 1/1] ceph/rbd block driver for qemu-kvm
At Fri, 21 May 2010 06:28:42 +0100, Stefan Hajnoczi wrote: > > On Thu, May 20, 2010 at 11:16 PM, Christian Brunner wrote: > > 2010/5/20 Anthony Liguori : > >> Both sheepdog and ceph ultimately transmit I/O over a socket to a central > >> daemon, right? So could we not standardize a protocol for this that both > >> sheepdog and ceph could implement? > > > > There is no central daemon. The concept is that they talk to many > > storage nodes at the same time. Data is distributed and replicated > > over many nodes in the network. The mechanism to do this is quite > > complex. I don't know about sheepdog, but in Ceph this is called RADOS > > (reliable autonomic distributed object store). Sheepdog and Ceph may > > look similar, but this is where they act different. I don't think that > > it would be possible to implement a common protocol. > > I believe Sheepdog has a local daemon on each node. The QEMU storage > backend talks to the daemon on the same node, which then does the real > network communication with the rest of the distributed storage system. Yes. It is because Sheepdog doesn't have a configuration about cluster membership as I mentioned in another mail, so the drvier doesn't know which node to access other than localhost. > So I think we're not talking about a network protocol here, we're > talking about a common interface that can be used by QEMU and other > programs to take advantage of Ceph, Sheepdog, etc services available > on the local node. > > Haven't looked into your patch enough yet, but does librados talk > directly over the network or does it connect to a local daemon/driver? > AFAIK, librados access directly over the network, so I think it is difficult to define a common interface. Thanks, Kazutaka
[Qemu-devel] [PATCH] resent: fix CPUID vendor override
the meaning of vendor_override is actually the opposite of how it is currently used :-( Fix it to allow KVM to export the non-native CPUID vendor if explicitly requested by the user. The semantic is now as intended: - With TCG, the guest always sees the configured vendor. - With KVM, the default is to propagate the host's vendor - when explicitly requested via -cpu ,vendor=xxx obey this and use the specified vendor Signed-off-by: Andre Przywara --- target-i386/cpuid.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Hi, this hasn't been picked up the last time I sent it out, are there any objections? Regards, Andre. diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c index 56938e2..99d1f44 100644 --- a/target-i386/cpuid.c +++ b/target-i386/cpuid.c @@ -962,7 +962,7 @@ static void get_cpuid_vendor(CPUX86State *env, uint32_t *ebx, * this if you want to use KVM's sysenter/syscall emulation * in compatibility mode and when doing cross vendor migration */ -if (kvm_enabled() && env->cpuid_vendor_override) { +if (kvm_enabled() && ! env->cpuid_vendor_override) { host_cpuid(0, 0, NULL, ebx, ecx, edx); } } -- 1.6.4
Re: [Qemu-devel] [RFC PATCH 1/1] ceph/rbd block driver for qemu-kvm
At Fri, 21 May 2010 00:16:46 +0200, Christian Brunner wrote: > > 2010/5/20 Anthony Liguori : > >> With new approaches like Sheepdog or Ceph, things are getting a lot > >> cheaper and you can scale your system without disrupting your service. > >> The concepts are quite similar to what Amazon is doing in their EC2 > >> environment, but they certainly won't publish it as OpenSource anytime > >> soon. > >> > >> Both projects have advantages and disadvantages. Ceph is a bit more > >> universal as it implements a whole filesystem. Sheepdog is more > >> feature complete in regards of managing images (e.g. snapshots). Both I think a major difference is that Sheepdog servers act fully autonomously. Any Sheepdog server has no fixed role such as a monitor server, and Sheepdog doesn't require any configuration about a list of nodes in the cluster. > >> projects require some additional work to become stable, but they are > >> on a good way. > >> > >> I would really like to see both drivers in the qemu tree, as they are > >> the key to a design shift in how storage in the datacenter is being > >> built. > >> > > > > I'd be more interested in enabling people to build these types of storage > > systems without touching qemu. > > You could do this by using Yehuda's rbd kernel driver, but I think > that it would be better to avoid this additional layer. > I agree. In addition, if a storage client is a qemu driver, the storage system can support some features specific to qemu such as live snapshot from qemu monitor. Regards, Kazutaka
Re: [Qemu-devel] [RFC PATCH 1/1] ceph/rbd block driver for qemu-kvm
On Thu, May 20, 2010 at 11:16 PM, Christian Brunner wrote: > 2010/5/20 Anthony Liguori : >> Both sheepdog and ceph ultimately transmit I/O over a socket to a central >> daemon, right? So could we not standardize a protocol for this that both >> sheepdog and ceph could implement? > > There is no central daemon. The concept is that they talk to many > storage nodes at the same time. Data is distributed and replicated > over many nodes in the network. The mechanism to do this is quite > complex. I don't know about sheepdog, but in Ceph this is called RADOS > (reliable autonomic distributed object store). Sheepdog and Ceph may > look similar, but this is where they act different. I don't think that > it would be possible to implement a common protocol. I believe Sheepdog has a local daemon on each node. The QEMU storage backend talks to the daemon on the same node, which then does the real network communication with the rest of the distributed storage system. So I think we're not talking about a network protocol here, we're talking about a common interface that can be used by QEMU and other programs to take advantage of Ceph, Sheepdog, etc services available on the local node. Haven't looked into your patch enough yet, but does librados talk directly over the network or does it connect to a local daemon/driver? Stefan
[Qemu-devel] [Bug 540230] Re: Configuration option error for ARM in default-configs
This problem has been fixed in 23f2166d at Feb 23 ** Changed in: qemu Status: New => Fix Committed -- Configuration option error for ARM in default-configs https://bugs.launchpad.net/bugs/540230 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: Fix Committed Bug description: The problem occurs when I try to launch qemu-system-arm for the machine: lm3s6965evb (Stellaris LM3S6965EVB) The error message was: qemu: hardware error: Unknown device 'ssd0323' for bus 'SSI' The error message means that the LED display driver (SSD0323) of the LM3S6965 evaluation board isn't recognised. Searching through the source code of QEMU 0.12.3, I've seen in default-configs/arm-softmmu.mak that there is reference made to: CONFIG_SD0303=y CONFIG_SD0323=y These parameters in turn are evaluated in the Makefile, as obj-$(CONFIG_SSD0303) += ssd0303.o obj-$(CONFIG_SSD0323) += ssd0323.o The problem is that the spelling of the tags doesn't match up: CONFIG_SD0303 in the .mak vs CONFIG_SSD0303 in the Makefile (double SS) Furthermore, in arm-softmmu.mak, reference is made to CONFIG_LAN9118=y and CONFIG_SMC91C111=y, which isn't referenced by the Makefile at all. Please correct these parameters in default-configs/arm-softmmu.mak in order to give full functionality to the ARM Cortex M3 evaluation boards.
Re: [Qemu-devel] [RFC PATCH 1/1] ceph/rbd block driver for qemu-kvm
On Thu, May 20, 2010 at 1:31 PM, Blue Swirl wrote: > On Wed, May 19, 2010 at 7:22 PM, Christian Brunner wrote: >> The attached patch is a block driver for the distributed file system >> Ceph (http://ceph.newdream.net/). This driver uses librados (which >> is part of the Ceph server) for direct access to the Ceph object >> store and is running entirely in userspace. Therefore it is >> called "rbd" - rados block device. ... > > IIRC underscores here may conflict with system header use. Please use > something like QEMU_BLOCK_RADOS_H. This header is shared between the linux kernel client and the ceph userspace servers and client. We can actually get rid of it, as we only need it to define CEPH_OSD_TMAP_SET. We can move this definition to librados.h. >> diff --git a/block/rbd_types.h b/block/rbd_types.h >> new file mode 100644 >> index 000..dfd5aa0 >> --- /dev/null >> +++ b/block/rbd_types.h >> @@ -0,0 +1,48 @@ >> +#ifndef _FS_CEPH_RBD >> +#define _FS_CEPH_RBD > > QEMU_BLOCK_RBD? This header is shared between the ceph kernel client, between the qemu rbd module (and between other ceph utilities). It'd be much easier maintaining it without having to have a different implementation for each. The same goes to the use of __le32/64 and __u32/64 within these headers. > >> + >> +#include > > Can you use standard includes, like or ? Are > Ceph libraries used in other systems than Linux? Not at the moment. I guess that we can take this include out. > >> + >> +/* >> + * rbd image 'foo' consists of objects >> + * foo.rbd - image metadata >> + * foo. >> + * foo.0001 >> + * ... - data >> + */ >> + >> +#define RBD_SUFFIX ".rbd" >> +#define RBD_DIRECTORY "rbd_directory" >> + >> +#define RBD_DEFAULT_OBJ_ORDER 22 /* 4MB */ >> + >> +#define RBD_MAX_OBJ_NAME_SIZE 96 >> +#define RBD_MAX_SEG_NAME_SIZE 128 >> + >> +#define RBD_COMP_NONE 0 >> +#define RBD_CRYPT_NONE 0 >> + >> +static const char rbd_text[] = "<<< Rados Block Device Image >>>\n"; >> +static const char rbd_signature[] = "RBD"; >> +static const char rbd_version[] = "001.001"; >> + >> +struct rbd_obj_snap_ondisk { >> + __le64 id; >> + __le64 image_size; >> +} __attribute__((packed)); >> + >> +struct rbd_obj_header_ondisk { >> + char text[64]; >> + char signature[4]; >> + char version[8]; >> + __le64 image_size; > > Unaligned? Is the disk format fixed? This is a packed structure that represents the on disk format. Operations on it are being done only to read from the disk header or to write to the disk header. Yehuda
Re: [Qemu-devel] Problems changing dvdrom iso during execution
On 05/20/2010 03:48 PM, Adnan Khaleel wrote: > Thanks for your response. > > > Does it work if the guest uses ide based CD's: > rmmod ide-scsi > modprobe ide-cd > > There isn't an ide-scsi but there is a scsi_mod and when I try to remove > that it gives > ERROR: Module scsi_mod is in use by sr_mod,sg,sd_mod,libata > > modprobe ide-cd seems to work. Ok, I pulled those from a RHEL3 VM. Looks like SLES11 is using a newer 2.6 kernel. The idea I was poking at was to get the CD in the VM to go through the ide-cd layer and not the ata/scsi route. I had to do that for my RHEL3 guest to get some consistency with the DVD -- similar to the problem you are seeing. David > > However it doesn't fix the problem. > > Interestingly, before doing modprobe ide-cd, > linux> lsmod | grep ide > ide_pci_generic 46520 > ide_core 115068 2 ide_pci_generic, piix > > After the modprobe ide-cd, I get > ide_cd_mod 339840 > cdrom 362002 ide_cd_mod, sr_mod > ide_pci_generic 46520 > ide_core 115068 3 ide_cd_mod, ide_pci_generic, piix > > > >
Re: [Qemu-devel] [RFC PATCH 1/1] ceph/rbd block driver for qemu-kvm
2010/5/20 Anthony Liguori : >> With new approaches like Sheepdog or Ceph, things are getting a lot >> cheaper and you can scale your system without disrupting your service. >> The concepts are quite similar to what Amazon is doing in their EC2 >> environment, but they certainly won't publish it as OpenSource anytime >> soon. >> >> Both projects have advantages and disadvantages. Ceph is a bit more >> universal as it implements a whole filesystem. Sheepdog is more >> feature complete in regards of managing images (e.g. snapshots). Both >> projects require some additional work to become stable, but they are >> on a good way. >> >> I would really like to see both drivers in the qemu tree, as they are >> the key to a design shift in how storage in the datacenter is being >> built. >> > > I'd be more interested in enabling people to build these types of storage > systems without touching qemu. You could do this by using Yehuda's rbd kernel driver, but I think that it would be better to avoid this additional layer. > Both sheepdog and ceph ultimately transmit I/O over a socket to a central > daemon, right? So could we not standardize a protocol for this that both > sheepdog and ceph could implement? There is no central daemon. The concept is that they talk to many storage nodes at the same time. Data is distributed and replicated over many nodes in the network. The mechanism to do this is quite complex. I don't know about sheepdog, but in Ceph this is called RADOS (reliable autonomic distributed object store). Sheepdog and Ceph may look similar, but this is where they act different. I don't think that it would be possible to implement a common protocol. Regards, Christian
Re: [Qemu-devel] Problems changing dvdrom iso during execution
Thanks for your response. Does it work if the guest uses ide based CD's: rmmod ide-scsi modprobe ide-cd There isn't an ide-scsi but there is a scsi_mod and when I try to remove that it gives ERROR: Module scsi_mod is in use by sr_mod,sg,sd_mod,libata modprobe ide-cd seems to work. However it doesn't fix the problem. Interestingly, before doing modprobe ide-cd, linux> lsmod | grep ide ide_pci_generic 46520 ide_core 115068 2 ide_pci_generic, piix After the modprobe ide-cd, I get ide_cd_mod 339840 cdrom 362002 ide_cd_mod, sr_mod ide_pci_generic 46520 ide_core 115068 3 ide_cd_mod, ide_pci_generic, piix
Re: [Qemu-devel] [RFC] Bug Day - June 1st, 2010
20.05.2010 11:15, Andre Przywara wrote: Michael Tokarev wrote: [] It'd be nice if we had more flexibility in defining custom machine types so you could just do qemu -M win98. This is wrong IMHO. win98 and winNT can run on various different machines, including all modern ones (yes I tried the same winNT on my Athlon X2-64, just had to switch SATA from AHCI to IDE; win95 works too)... just not in kvm :) Well, not really. You were lucky with your Athlon X2-64, actually it is the last machine not triggering the bug. I tried it on a AthlonII-X4 (which has maxleaf=5 as any newer AMD machines) and it showed the same bug. On Intel boxes this bug should trigger on every CPU starting with some Pentium4 models, including all Core chips. Have you tried versions with a newer service pack (SP6)? I replied in the original discussion -- after upgrading to SP6 there's no need in ,level=1 anymore, any -cpu variant works without crashes. The problem is to set it up, at least for me, since I don't have sp6 integrated into setup. Well, I don't use winNT to start with, actually, so for me it's not a problem at all ;) -- the reason why I asked is because I have a debian bugreport about this very issue, see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=575439 (and because I had winNT install handy) But this is really interesting information - that winNT fails on other CPUs too. Thank you for that, now I can close the debian bugreport ;) BTW: Does anyone knows what the problem with Windows95/98 on KVM is? I tried some tracing today, but couldn't find a hint. Um. The bugreport(s) come as a surprize for me: I tried to install win98 in kvm several times in the past but setup always failed - different messages in different versions of kvm, either "unable to emulate" or "real mode trap" or something else, or just lockup, usually on first reboot. So - the bugreports talks about mouse non-working, but this means win98 itself works somehow... I dunno :) I think these bug reports are about plain QEMU. I tried it yesterday, in fact the mouse is non-functional. In KVM Windows95 gives me a black screen after the welcome screen with the moving bottom row. There are just two lines at the top: (translated from the german version) While initializing device NTKERN: Windows protection fault. Restart the computer. Yeah, that's what i've seen too, it's exactly ow it fails here with modern kvm. KVM catched some #UDs due to ARPL from VM86 mode, but TCG got them too and it survived. So if anyone has some more hints, I'd be grateful. Thank you! /mjt
Re: [Qemu-devel] [RFC PATCH 1/1] ceph/rbd block driver for qemu-kvm
On 05/20/2010 04:18 PM, Christian Brunner wrote: Thanks for your comments. I'll send an updated patch in a few days. Having a central storage system is quite essential in larger hosting environments, it enables you to move your guest systems from one node to another easily (live-migration or dynamic restart). Traditionally this has been done using SAN, iSCSI or NFS. However most of these systems don't scale very well and and the costs for high-availability are quite high. With new approaches like Sheepdog or Ceph, things are getting a lot cheaper and you can scale your system without disrupting your service. The concepts are quite similar to what Amazon is doing in their EC2 environment, but they certainly won't publish it as OpenSource anytime soon. Both projects have advantages and disadvantages. Ceph is a bit more universal as it implements a whole filesystem. Sheepdog is more feature complete in regards of managing images (e.g. snapshots). Both projects require some additional work to become stable, but they are on a good way. I would really like to see both drivers in the qemu tree, as they are the key to a design shift in how storage in the datacenter is being built. I'd be more interested in enabling people to build these types of storage systems without touching qemu. Both sheepdog and ceph ultimately transmit I/O over a socket to a central daemon, right? So could we not standardize a protocol for this that both sheepdog and ceph could implement? Regards, Anthony Liguori Christian -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [RFC PATCH 1/1] ceph/rbd block driver for qemu-kvm
2010/5/20 Blue Swirl : > On Wed, May 19, 2010 at 7:22 PM, Christian Brunner wrote: >> The attached patch is a block driver for the distributed file system >> Ceph (http://ceph.newdream.net/). This driver uses librados (which >> is part of the Ceph server) for direct access to the Ceph object >> store and is running entirely in userspace. Therefore it is >> called "rbd" - rados block device. >> >> To compile the driver a recent version of ceph (>= 0.20.1) is needed >> and you have to "--enable-rbd" when running configure. >> >> Additional information is available on the Ceph-Wiki: >> >> http://ceph.newdream.net/wiki/Kvm-rbd > > > I have no idea whether it makes sense to add Ceph (no objection > either). I have some minor comments below. Thanks for your comments. I'll send an updated patch in a few days. Having a central storage system is quite essential in larger hosting environments, it enables you to move your guest systems from one node to another easily (live-migration or dynamic restart). Traditionally this has been done using SAN, iSCSI or NFS. However most of these systems don't scale very well and and the costs for high-availability are quite high. With new approaches like Sheepdog or Ceph, things are getting a lot cheaper and you can scale your system without disrupting your service. The concepts are quite similar to what Amazon is doing in their EC2 environment, but they certainly won't publish it as OpenSource anytime soon. Both projects have advantages and disadvantages. Ceph is a bit more universal as it implements a whole filesystem. Sheepdog is more feature complete in regards of managing images (e.g. snapshots). Both projects require some additional work to become stable, but they are on a good way. I would really like to see both drivers in the qemu tree, as they are the key to a design shift in how storage in the datacenter is being built. Christian
[Qemu-devel] [PATCH] fix curses update - v2
On Mon, May 03, 2010 at 01:06:46PM -0500, Anthony Liguori wrote: > On 04/22/2010 09:08 AM, Bernhard Kauer wrote: > >Hi, > > > >>I believe this issue has come up before with a similar patch but > >well i've submitted such a patch more than two years ago. Unfortunatelly > >it got never applied, so that I have to patch my Qemu on every update... > > > > > >>someone checked their ncurses and they didn't see the same issue. > >>I just checked and here mvwaddchnstr() does not expect a null-terminated > >>string either, but it skips the \0 characters. > >This is not conforming to the Single UNIX Specification, which states > >that the string is shown "until a null chtype is encountered". See for > >example: > > http://www.opengroup.org/onlinepubs/007908775/xcurses/addchstr.html > > > > > >> So probably we should > >>replace them with spaces or something else, I wouldn't like to > >>replace a single library call with 80 calls, it's better to go through > >>the string and replace them, maybe in console_write_ch or somewhere > >>else. > >That would be a one-liner. Should I send such a patch? > > Yes. Replace the \0 character with a space to allow to use mvwaddchnstr for full-screen updates in curses mode. Signed-off-by: Bernhard Kauer diff --git a/console.h b/console.h index 6def115..42ff822 100644 --- a/console.h +++ b/console.h @@ -306,6 +306,7 @@ static inline int ds_get_bytes_per_pixel(DisplayState *ds) typedef unsigned long console_ch_t; static inline void console_write_ch(console_ch_t *dest, uint32_t ch) { +if (!(ch & 0xff)) ch = 0x20; cpu_to_le32wu((uint32_t *) dest, ch); }
Re: [Qemu-devel] Problems changing dvdrom iso during execution
On 05/20/2010 02:29 PM, Adnan Khaleel wrote: > I cannot change DVD roms during execution using the monitor. I can only > mount a cdrom/dvdrom if I specify the iso file in the command line > x86_64-softmmu/qemu-system-x86_64 -hda ../../OSImages/sles11.qcow2 > -cdrom ../../ISOz/mydvd.iso -m 2048 > > In the guest I can mount the iso image as you could normally expect > mount /dev/cdrom /mnt > mount: block device /dev/sr0 is write-protected, mounting read-only > > Info block in the monitor yields > (qemu) info block > ide0-hd0: type=hd removable=0 file../../OSImages/sles11.qcow2 ro=0 > drv=dcow2 encrypted=0 > ide1-cd0: type=cdrom removable=1 locked=1 file=../../ISOz/mydvd.iso ro=0 > drv=raw encrypted=0 > floppy0: type=floppy removable=1 locked=0 [not inserted] > sd0: type=floppy removable=1 locked=0 [not inserted] > > When I try to do a eject ide0-cd0, I get a device busy message so I have > to do a eject -f ide0-cd0 > > After which, an info block yields: > (qemu) info block > ide0-hd0: type=hd removable=0 file../../OSImages/sles11.qcow2 ro=0 > drv=dcow2 encrypted=0 > ide1-cd0: type=cdrom removable=1 locked=1 [not inserted] > floppy0: type=floppy removable=1 locked=0 [not inserted] > sd0: type=floppy removable=1 locked=0 [not inserted] > > I change the iso image with (or so it seems) > > (qemu) change ide1-cd0 ../../ISOz/mydvd2_rom.iso > (qemu) info block > ide0-hd0: type=hd removable=0 file../../OSImages/sles11.qcow2 ro=0 > drv=dcow2 encrypted=0 > ide1-cd0: type=cdrom removable=1 locked=1 file=../../ISOz/mydvd2.iso > ro=0 drv=raw encrypted=0 > floppy0: type=floppy removable=1 locked=0 [not inserted] > sd0: type=floppy removable=1 locked=0 [not inserted] > > I go back to the guest and when I try to mount: > > mount /dev/cdrom /mnt > mount: /dev/sr0 unknown device > > I'm running sles11 as guest and I think it may have something to do with > sles11 as it works fine with ubuntu9. > > Any ideas what might be happening? Does it work if the guest uses ide based CD's: rmmod ide-scsi modprobe ide-cd David > > Thanks > > AK
[Qemu-devel] Re: [PATCH] pc: fix segfault introduced by 3d53f5c36ff6
Good catch. Thanks, applied. On Thu, May 20, 2010 at 6:14 AM, Eduard - Gabriel Munteanu wrote: > Commit 3d53f5c36ff6 introduced a segfault by erroneously making fw_cfg a > 'void **' and passing it around in different ways. > > Signed-off-by: Eduard - Gabriel Munteanu > --- > hw/pc.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/pc.c b/hw/pc.c > index fee08c9..4a4a706 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -822,7 +822,7 @@ void pc_memory_init(ram_addr_t ram_size, > ram_addr_t ram_addr, bios_offset, option_rom_offset; > ram_addr_t below_4g_mem_size, above_4g_mem_size = 0; > int bios_size, isa_bios_size; > - void **fw_cfg; > + void *fw_cfg; > > if (ram_size >= 0xe000 ) { > above_4g_mem_size = ram_size - 0xe000; > @@ -905,7 +905,7 @@ void pc_memory_init(ram_addr_t ram_size, > rom_set_fw(fw_cfg); > > if (linux_boot) { > - load_linux(*fw_cfg, kernel_filename, initrd_filename, > kernel_cmdline, below_4g_mem_size); > + load_linux(fw_cfg, kernel_filename, initrd_filename, kernel_cmdline, > below_4g_mem_size); > } > > for (i = 0; i < nb_option_roms; i++) { > -- > 1.6.4.4 > >
Re: [Qemu-devel] [RFC PATCH 1/1] ceph/rbd block driver for qemu-kvm
On Wed, May 19, 2010 at 7:22 PM, Christian Brunner wrote: > The attached patch is a block driver for the distributed file system > Ceph (http://ceph.newdream.net/). This driver uses librados (which > is part of the Ceph server) for direct access to the Ceph object > store and is running entirely in userspace. Therefore it is > called "rbd" - rados block device. > > To compile the driver a recent version of ceph (>= 0.20.1) is needed > and you have to "--enable-rbd" when running configure. > > Additional information is available on the Ceph-Wiki: > > http://ceph.newdream.net/wiki/Kvm-rbd I have no idea whether it makes sense to add Ceph (no objection either). I have some minor comments below. > > --- > Makefile | 3 + > Makefile.objs | 1 + > block/rados.h | 376 ++ > block/rbd.c | 585 > + > block/rbd_types.h | 48 + > configure | 27 +++ > 6 files changed, 1040 insertions(+), 0 deletions(-) > create mode 100644 block/rados.h > create mode 100644 block/rbd.c > create mode 100644 block/rbd_types.h > > diff --git a/Makefile b/Makefile > index eb9e02b..b1ab3e9 100644 > --- a/Makefile > +++ b/Makefile > @@ -27,6 +27,9 @@ configure: ; > $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw) > > LIBS+=-lz $(LIBS_TOOLS) > +ifdef CONFIG_RBD > +LIBS+=-lrados > +endif > > ifdef BUILD_DOCS > DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 > diff --git a/Makefile.objs b/Makefile.objs > index acbaf22..85791ac 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -18,6 +18,7 @@ block-nested-y += parallels.o nbd.o blkdebug.o > block-nested-$(CONFIG_WIN32) += raw-win32.o > block-nested-$(CONFIG_POSIX) += raw-posix.o > block-nested-$(CONFIG_CURL) += curl.o > +block-nested-$(CONFIG_RBD) += rbd.o > > block-obj-y += $(addprefix block/, $(block-nested-y)) > > diff --git a/block/rados.h b/block/rados.h > new file mode 100644 > index 000..6cde9a1 > --- /dev/null > +++ b/block/rados.h > @@ -0,0 +1,376 @@ > +#ifndef __RADOS_H > +#define __RADOS_H IIRC underscores here may conflict with system header use. Please use something like QEMU_BLOCK_RADOS_H. > + > +/* > + * Data types for the Ceph distributed object storage layer RADOS > + * (Reliable Autonomic Distributed Object Store). > + */ > + > + > + > +/* > + * osdmap encoding versions > + */ > +#define CEPH_OSDMAP_INC_VERSION 5 > +#define CEPH_OSDMAP_INC_VERSION_EXT 5 > +#define CEPH_OSDMAP_VERSION 5 > +#define CEPH_OSDMAP_VERSION_EXT 5 > + > +/* > + * fs id > + */ > +struct ceph_fsid { > + unsigned char fsid[16]; Too large indent, please check also elsewhere. > +}; > + > +static inline int ceph_fsid_compare(const struct ceph_fsid *a, > + const struct ceph_fsid *b) > +{ > + return memcmp(a, b, sizeof(*a)); > +} > + > +/* > + * ino, object, etc. > + */ > +typedef __le64 ceph_snapid_t; Please use uint64_t and le_to_cpu()/cpu_to_le(). > +#define CEPH_SNAPDIR ((__u64)(-1)) /* reserved for hidden .snap dir */ Likewise, uint64_t is the standard type. Also other places. > +#define CEPH_NOSNAP ((__u64)(-2)) /* "head", "live" revision */ > +#define CEPH_MAXSNAP ((__u64)(-3)) /* largest valid snapid */ > + > +struct ceph_timespec { > + __le32 tv_sec; > + __le32 tv_nsec; > +} __attribute__ ((packed)); > + > + > +/* > + * object layout - how objects are mapped into PGs > + */ > +#define CEPH_OBJECT_LAYOUT_HASH 1 > +#define CEPH_OBJECT_LAYOUT_LINEAR 2 > +#define CEPH_OBJECT_LAYOUT_HASHINO 3 > + > +/* > + * pg layout -- how PGs are mapped onto (sets of) OSDs > + */ > +#define CEPH_PG_LAYOUT_CRUSH 0 > +#define CEPH_PG_LAYOUT_HASH 1 > +#define CEPH_PG_LAYOUT_LINEAR 2 > +#define CEPH_PG_LAYOUT_HYBRID 3 > + > + > +/* > + * placement group. > + * we encode this into one __le64. > + */ > +struct ceph_pg { > + __le16 preferred; /* preferred primary osd */ > + __le16 ps; /* placement seed */ > + __le32 pool; /* object pool */ > +} __attribute__ ((packed)); > + > +/* > + * pg_pool is a set of pgs storing a pool of objects > + * > + * pg_num -- base number of pseudorandomly placed pgs > + * > + * pgp_num -- effective number when calculating pg placement. this > + * is used for pg_num increases. new pgs result in data being "split" > + * into new pgs. for this to proceed smoothly, new pgs are intiially > + * colocated with their parents; that is, pgp_num doesn't increase > + * until the new pgs have successfully split. only _then_ are the new > + * pgs placed independently. > + * > + * lpg_num -- localized pg count (per device). replicas are randomly > + * selected. > + * > + * lpgp_num -- as above. > + */ > +#define CEPH_PG_TYPE_REP 1 > +#define CEPH_PG_TYPE_RAID4 2 > +#define CEPH_PG_POOL_VERSION 2 > +struct ceph_pg_pool { > + __u8 type; /* CEPH_PG_TYPE_* */ > + __u8 size;
[Qemu-devel] Problems changing dvdrom iso during execution
I cannot change DVD roms during execution using the monitor. I can only mount a cdrom/dvdrom if I specify the iso file in the command line x86_64-softmmu/qemu-system-x86_64 -hda ../../OSImages/sles11.qcow2 -cdrom ../../ISOz/mydvd.iso -m 2048 In the guest I can mount the iso image as you could normally expect mount /dev/cdrom /mnt mount: block device /dev/sr0 is write-protected, mounting read-only Info block in the monitor yields (qemu) info block ide0-hd0: type=hd removable=0 file../../OSImages/sles11.qcow2 ro=0 drv=dcow2 encrypted=0 ide1-cd0: type=cdrom removable=1 locked=1 file=../../ISOz/mydvd.iso ro=0 drv=raw encrypted=0 floppy0: type=floppy removable=1 locked=0 [not inserted] sd0: type=floppy removable=1 locked=0 [not inserted] When I try to do a eject ide0-cd0, I get a device busy message so I have to do a eject -f ide0-cd0 After which, an info block yields: (qemu) info block ide0-hd0: type=hd removable=0 file../../OSImages/sles11.qcow2 ro=0 drv=dcow2 encrypted=0 ide1-cd0: type=cdrom removable=1 locked=1 [not inserted] floppy0: type=floppy removable=1 locked=0 [not inserted] sd0: type=floppy removable=1 locked=0 [not inserted] I change the iso image with (or so it seems) (qemu) change ide1-cd0 ../../ISOz/mydvd2_rom.iso (qemu) info block ide0-hd0: type=hd removable=0 file../../OSImages/sles11.qcow2 ro=0 drv=dcow2 encrypted=0 ide1-cd0: type=cdrom removable=1 locked=1 file=../../ISOz/mydvd2.iso ro=0 drv=raw encrypted=0 floppy0: type=floppy removable=1 locked=0 [not inserted] sd0: type=floppy removable=1 locked=0 [not inserted] I go back to the guest and when I try to mount: mount /dev/cdrom /mnt mount: /dev/sr0 unknown device I'm running sles11 as guest and I think it may have something to do with sles11 as it works fine with ubuntu9. Any ideas what might be happening? Thanks AK
[Qemu-devel] [[RfC PATCH]] linux fbdev display driver prototype.
Display works with 32 bpp (both host + guest) only. Which surprisingly didn't cause much problems so far in my testing. Host runs with kms and inteldrmfb. Mouse support isn't available yet. I've cheated by passed through the hosts usb mouse for testing. Keyboard works. Guest screen has whatever keymap you load inside the guest. Text windows (monitor, serial, ...) have a simple en-us keymap. Good enougth to type monitor commands. Not goot enougth to work seriously on a serial terminal. But the qemu terminal emulation isn't good enougth for that anyway ;) Hot keys: Ctrl-Alt-F -> host console switching. Ctrl-Alt- -> qemu console switching. Ctrl-Alt-ESC-> exit qemu. Special feature: Sane console switching. Switching away stops screen updates. Switching back redraws the screen. When started from the linux console qemu uses the vt you've started it from (requires just read/write access to /dev/fb0). When starting from somewhere else qemu tries to open a unused virtual terminal and switch to it (usually requires root privileges to open /dev/tty). For some strange reason console switching from X11 to qemu doesn't work. Anything else (including X11 -> text console -> qemu) works fine. To be investigated ... Cc: Julian Pidancet Cc: Stefano Stabellini Signed-off-by: Gerd Hoffmann --- Makefile.objs|1 + console.h|3 + fbdev.c | 770 ++ linux-keynames.h | 386 +++ qemu-options.hx | 10 + sysemu.h |1 + vl.c | 10 + 7 files changed, 1181 insertions(+), 0 deletions(-) create mode 100644 fbdev.c create mode 100644 linux-keynames.h diff --git a/Makefile.objs b/Makefile.objs index ecdd53e..cff1a23 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -102,6 +102,7 @@ common-obj-y += $(addprefix audio/, $(audio-obj-y)) common-obj-y += keymaps.o common-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o common-obj-$(CONFIG_CURSES) += curses.o +common-obj-$(CONFIG_LINUX) += fbdev.o common-obj-y += vnc.o acl.o d3des.o common-obj-y += vnc-encoding-zlib.o vnc-encoding-hextile.o common-obj-y += iov.o diff --git a/console.h b/console.h index 6def115..bba1da8 100644 --- a/console.h +++ b/console.h @@ -338,6 +338,9 @@ void qemu_console_copy(DisplayState *ds, int src_x, int src_y, /* sdl.c */ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame); +/* fbdev.c */ +void fbdev_display_init(DisplayState *ds, const char *device); + /* cocoa.m */ void cocoa_display_init(DisplayState *ds, int full_screen); diff --git a/fbdev.c b/fbdev.c new file mode 100644 index 000..9ad7db6 --- /dev/null +++ b/fbdev.c @@ -0,0 +1,770 @@ +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include +#include +#include +#include + +#include "qemu-common.h" +#include "console.h" +#include "keymaps.h" + +/* */ + +/* file handles */ +static inttty, fb; + +/* saved state, for restore on exit */ +static intorig_vtno = 0; +static intkd_omode; +static struct vt_mode vt_omode; +static struct fb_var_screeninfo fb_ovar; + +/* framebuffer */ +static struct fb_fix_screeninfo fb_fix; +static struct fb_var_screeninfo fb_var; +static uint8_t *fb_mem; +static int fb_mem_offset = 0; + +/* linux console */ +static intvtno; +static struct vt_mode vt_mode; +static struct termios tty_attributes; +static unsigned long tty_mode; +static unsigned int tty_flags; +static bool tty_mediumraw; +static bool key_down[KEY_CNT]; + +/* console switching */ +#define SIG_ACQ (SIGRTMIN+6) +#define SIG_REL (SIGRTMIN+7) +#define FB_ACTIVE0 +#define FB_REL_REQ 1 +#define FB_INACTIVE 2 +#define FB_ACQ_REQ 3 +static int fb_switch_state = FB_ACTIVE; + +/* qdev windup */ +static DisplayChangeListener *dcl; +static intresize_screen; +static intredraw_screen; +static intcx, cy; +static intdebug = 0; + +/* fwd decls */ +static int fbdev_activate_vt(int tty, int vtno, bool wait); + +/* */ +/* keyboard */ + +static const char *keynames[] = { +#include "linux-keynames.h" +}; + +static int scancode_map[KEY_CNT] = { +[ KEY_ESC ] = 0x01, +[ KEY_1] = 0x02, +[ KEY_2] = 0x03, +[ KEY_3] = 0x04, +[ KEY_4] = 0x05, +[ KEY_5] = 0x06, +[ KEY_6] = 0x07, +[ KEY_7] = 0x08, +[ KE
[Qemu-devel] Re: phys_page_find bug?
2010/5/7 Artyom Tarasenko : > phys_page_find (exec.c) returns sometimes a page for addresses where > nothing is connected. > > One example, done with qemu-system-sparc -M SS-20 > > ok f130 2f spacec@ . > > // The address translates correctly, in cpu_physical_memory_rw > // addr== 0xff130 (where nothing is connected) > // but then phys_page_find returns a nonzero and produces > > Unassigned mem read access of 1 byte to 000ff150 from x > > (note the "5" in the line above where "3" is expected) > > I wonder if this is only true for non-wired addresses, or whether > phys_page_find can also > find wrong pages for the addresses where something is connected? > > Or is my assumption is wrong and phys_page_find can return a page for > not-connected > addresses and the bug is actually in cpu_physical_memory_rw ? > > Is the qemu algorithm of working with the physical address space > described somewhere? I'm surprised that no one is interested in discussing this issue. It may affect other targets too. After some debugging I see that page 0xff15ff000 is allocated twice when emulating SS-20. Can this be a problem? >From the phys_page_find logic it looks like the pages are expected to be allocated in the natural order: the loop descends till the page hits a search mask. sun4m_hw_init initializes devices in a more or less random order. Can this be a problem? Also the function cpu_register_physical_memory_offset the following comment: ...Both start_addr and region_offset are rounded down to a page boundary before calculating this offset. This should not be a problem unless the low bits of start_addr and region_offset differ. */ What is meant here by "low bits"? I put a check if((region_offset & TARGET_PAGE_MASK)!=(start_addr & TARGET_PAGE_MASK)) printf... and it gets hit a lot within the address range 0xd0512-ff180 . Does it indicate a problem? -- Regards, Artyom Tarasenko solaris/sparc under qemu blog: http://tyom.blogspot.com/
[Qemu-devel] Re: [PATCH 3/3] target-sparc: Inline some generation of carry for ADDX/SUBX.
Thanks, applied. On Wed, May 12, 2010 at 6:04 PM, Richard Henderson wrote: > Computing carry is trivial for some inputs. By avoiding an > external function call, we generate near-optimal code for > the common cases of add+addx (double-word arithmetic) and > cmp+addx (a setcc pattern). > > Signed-off-by: Richard Henderson > --- > target-sparc/helper.h | 2 +- > target-sparc/op_helper.c | 2 +- > target-sparc/translate.c | 272 > +- > 3 files changed, 200 insertions(+), 76 deletions(-) > > diff --git a/target-sparc/helper.h b/target-sparc/helper.h > index 04c1306..6f103e7 100644 > --- a/target-sparc/helper.h > +++ b/target-sparc/helper.h > @@ -158,6 +158,6 @@ VIS_CMPHELPER(cmpne); > #undef VIS_HELPER > #undef VIS_CMPHELPER > DEF_HELPER_0(compute_psr, void); > -DEF_HELPER_0(compute_C_icc, tl); > +DEF_HELPER_0(compute_C_icc, i32); > > #include "def-helper.h" > diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c > index 3783b02..125cd67 100644 > --- a/target-sparc/op_helper.c > +++ b/target-sparc/op_helper.c > @@ -1342,7 +1342,7 @@ void helper_compute_psr(void) > CC_OP = CC_OP_FLAGS; > } > > -target_ulong helper_compute_C_icc(void) > +uint32_t helper_compute_C_icc(void) > { > uint32_t ret; > > diff --git a/target-sparc/translate.c b/target-sparc/translate.c > index ea7c71b..713d3e1 100644 > --- a/target-sparc/translate.c > +++ b/target-sparc/translate.c > @@ -332,24 +332,132 @@ static inline void gen_op_add_cc(TCGv dst, TCGv src1, > TCGv src2) > tcg_gen_mov_tl(dst, cpu_cc_dst); > } > > -static inline void gen_op_addxi_cc(TCGv dst, TCGv src1, target_long src2) > +static TCGv_i32 gen_add32_carry32(void) > { > - gen_helper_compute_C_icc(cpu_tmp0); > - tcg_gen_mov_tl(cpu_cc_src, src1); > - tcg_gen_movi_tl(cpu_cc_src2, src2); > - tcg_gen_add_tl(cpu_cc_dst, cpu_cc_src, cpu_tmp0); > - tcg_gen_addi_tl(cpu_cc_dst, cpu_cc_dst, src2); > - tcg_gen_mov_tl(dst, cpu_cc_dst); > + TCGv_i32 carry_32, cc_src1_32, cc_src2_32; > + > + /* Carry is computed from a previous add: (dst < src) */ > +#if TARGET_LONG_BITS == 64 > + cc_src1_32 = tcg_temp_new_i32(); > + cc_src2_32 = tcg_temp_new_i32(); > + tcg_gen_trunc_i64_i32(cc_src1_32, cpu_cc_dst); > + tcg_gen_trunc_i64_i32(cc_src2_32, cpu_cc_src); > +#else > + cc_src1_32 = cpu_cc_dst; > + cc_src2_32 = cpu_cc_src; > +#endif > + > + carry_32 = tcg_temp_new_i32(); > + tcg_gen_setcond_i32(TCG_COND_LTU, carry_32, cc_src1_32, cc_src2_32); > + > +#if TARGET_LONG_BITS == 64 > + tcg_temp_free_i32(cc_src1_32); > + tcg_temp_free_i32(cc_src2_32); > +#endif > + > + return carry_32; > } > > -static inline void gen_op_addx_cc(TCGv dst, TCGv src1, TCGv src2) > +static TCGv_i32 gen_sub32_carry32(void) > { > - gen_helper_compute_C_icc(cpu_tmp0); > - tcg_gen_mov_tl(cpu_cc_src, src1); > - tcg_gen_mov_tl(cpu_cc_src2, src2); > - tcg_gen_add_tl(cpu_cc_dst, cpu_cc_src, cpu_tmp0); > - tcg_gen_add_tl(cpu_cc_dst, cpu_cc_dst, cpu_cc_src2); > - tcg_gen_mov_tl(dst, cpu_cc_dst); > + TCGv_i32 carry_32, cc_src1_32, cc_src2_32; > + > + /* Carry is computed from a previous borrow: (src1 < src2) */ > +#if TARGET_LONG_BITS == 64 > + cc_src1_32 = tcg_temp_new_i32(); > + cc_src2_32 = tcg_temp_new_i32(); > + tcg_gen_trunc_i64_i32(cc_src1_32, cpu_cc_src); > + tcg_gen_trunc_i64_i32(cc_src2_32, cpu_cc_src2); > +#else > + cc_src1_32 = cpu_cc_src; > + cc_src2_32 = cpu_cc_src2; > +#endif > + > + carry_32 = tcg_temp_new_i32(); > + tcg_gen_setcond_i32(TCG_COND_LTU, carry_32, cc_src1_32, cc_src2_32); > + > +#if TARGET_LONG_BITS == 64 > + tcg_temp_free_i32(cc_src1_32); > + tcg_temp_free_i32(cc_src2_32); > +#endif > + > + return carry_32; > +} > + > +static void gen_op_addx_int(DisasContext *dc, TCGv dst, TCGv src1, > + TCGv src2, int update_cc) > +{ > + TCGv_i32 carry_32; > + TCGv carry; > + > + switch (dc->cc_op) { > + case CC_OP_DIV: > + case CC_OP_LOGIC: > + /* Carry is known to be zero. Fall back to plain ADD. */ > + if (update_cc) { > + gen_op_add_cc(dst, src1, src2); > + } else { > + tcg_gen_add_tl(dst, src1, src2); > + } > + return; > + > + case CC_OP_ADD: > + case CC_OP_TADD: > + case CC_OP_TADDTV: > +#if TCG_TARGET_REG_BITS == 32 && TARGET_LONG_BITS == 32 > + { > + /* For 32-bit hosts, we can re-use the host's hardware carry > + generation by using an ADD2 opcode. We discard the low > + part of the output. Ideally we'd combine this operation > + with the add that generated the carry in the first place. */ > + TCGv dst_low = tcg_temp_new(); > + tcg_gen_op6_i32(INDEX_op_add2_i32, dst_low, dst, > + cpu_cc_src, src1, cpu_cc_src2, src2); > + tcg_temp_free(dst_low); > + goto add_done;
Re: [Qemu-devel] [PATCH 0/6]: QMP: Fix issues in parser/lexer
On 05/20/2010 12:43 AM, Anthony Liguori wrote: The JSON specification explicitly says: "A JSON parser transforms a JSON text into another representation. A JSON parser MUST accept all texts that conform to the JSON grammar. A JSON parser MAY accept non-JSON forms or extensions." IOW, we're under no obligation to reject extensions and I can't think of a reason why we should. At the very least, we should document them. If the extension doesn't add any value but is merely a side effect of the implementation, we should remove it. Examples where this could hurt us: - we move to a json parsing library, the extension disappears, client breaks - someone writes a qemu simulator to test managment tool scalability (run zillions of fake guests on one machine), client breaks - someone writes a debug tool that interposes between client and qemu, client breaks - the json specification adds a new form that conflicts with one of our extensions [1], we can't use the new form Being strict in what we accept will reduce our support burden later on. [1] allowing infinite extensibility like this is irresponsible -- Do not meddle in the internals of kernels, for they are subtle and quick to panic.
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On Thu, 20 May 2010 13:52:08 -0500 Anthony Liguori wrote: > On 05/20/2010 01:47 PM, Luiz Capitulino wrote: > > On Thu, 20 May 2010 11:55:00 -0500 > > Anthony Liguori wrote: > > > > > >> On 05/20/2010 11:27 AM, Luiz Capitulino wrote: > >> > >>> On Thu, 20 May 2010 10:50:41 -0500 > >>> Anthony Liguori wrote: > >>> > >>> > >>> > On 05/20/2010 10:16 AM, Paolo Bonzini wrote: > > > > On 05/20/2010 03:44 PM, Luiz Capitulino wrote: > > > > > >> I think there's another issue in the handling of strings. > >> > >> The spec says that valid unescaped chars are in the following > >> range: > >> > >>unescaped = %x20-21 / %x23-5B / %x5D-10 > >> > >> > That's a spec bug IMHO. Tab is %x09. Surely you can include tabs in > strings. Any parser that didn't accept that would be broken. > > > >>>Honestly, I had the impression this should be encoded as: %x5C %x74, > >>> but > >>> if you're right, wouldn't this be true for other sequences as well? > >>> > >>> > >> I don't think most reasonable clients are going to quote tabs as '\t'. > >> > > That would be a bug, wouldn't it? > > > > Tabs are valid in JavaScript strings and I don't think it's reasonable > to expect that a valid JavaScript string is not a valid JSON string. IMO, we should do what the spec says and what bug free clients expect, what we consider reasonable or unreasonable is a different matter. I would be with you if the spec was proved wrong, specially if reference implementations out there didn't follow it either, but everything I found so far shows this is not the case. Another example: http://www.json.org/json2.js Search for 'character substitutions'.
Re: [Qemu-devel] [PATCH] QEMU: change default disk cache behavior
On 05/20/2010 08:49 AM, Jes Sorensen wrote: On 05/20/10 15:40, Anthony Liguori wrote: On 05/20/2010 08:36 AM, Jes Sorensen wrote: And I strongly suspect that such a blanket change would be wrong but that a more targeted change like making cache=none default for physical devices would satisfy mostly everyone. Is there any other thing than physical devices attached to the -drive parameter? Image files which are the overwhelming more common use-case. For image files we certainly want it too, at least for proper ones (ie. raw). What makes you say that? It could be that it causes problems for qcow2. It's definitely the wrong thing for qcow2 with backing files. Regards, Anthony Liguori I'll try and look at it when I am back. Cheers, Jes
Re: [Qemu-devel] Re: [PATCH] QEMU: Change default disk caching to nocache
On 05/20/2010 10:24 AM, Paolo Bonzini wrote: On 05/20/2010 11:32 AM, jes.soren...@redhat.com wrote: +if (bdrv_flags & BDRV_O_NOCACHE) { +fprintf(stderr, "qemu: failed to open disk image %s as " +"nocache (O_DIRECT) retrying as write-back\n", file); +bdrv_flags &= BDRV_O_NOCACHE; Missing ~ here. +bdrv_flags |= BDRV_O_CACHE_WB; +if (bdrv_open(dinfo->bdrv, file, bdrv_flags, drv)< 0) +goto error_open; +} else { I think the retry should be done silently if no cache= option is given. That is cache=none will be the default but: - if it is not specified and not supported by the image, fall back to writeback with no warning. However, this is just a QoI issue and can be fixed later. - if it is specified and not supported by the image, either fall back to writeback with a warning, or fail altogether. The former would be a change in behavior, so it has to be documented somewhere if it changes. Or maybe add BDRV_O_CACHE_WT and let the backend decide the default? It used to be that we had a CACHE_DEFAULT which allowed qcow2 to do CACHE_WB by default whereas everything else did CACHE_WT. The same technique could be used to let physical devices do NOCACHE by default. Regards, Anthony Liguori Paolo
Re: [Qemu-devel] [Bug 391879] Re: migrate exec ignores exit status
On 05/20/2010 12:11 PM, Daniel P. Berrange wrote: On Thu, May 20, 2010 at 04:50:59PM -, Dave Walker wrote: This is a bug and has been reported upstream, it is unlikely to be fixed at the distribution level and therefore anyone interested in working on this bug should contribute a patch to the upstream project. This will then filter down to Ubuntu when it is merged mainline. Marking "Won't Fix" against the Ubuntu package. Thanks for reporting this bug. ** Changed in: qemu-kvm (Ubuntu) Status: Confirmed => Won't Fix -- migrate exec ignores exit status https://bugs.launchpad.net/bugs/391879 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. This bug appears to be filed against the Ubuntu qemu component, rather than the upstream qemu component. Are we supposed to be getting notifications for all Ubuntu distro qemu bugs too, rather than just usptream bug reports ? It's an upstream bug that references an Ubuntu bug. Whenever a referenced bug has it's status changed, the upstream bug will be notified. You can also reference bugs in just about any Bugzilla including the Fedora bugzilla which is pretty nice because then when a bug gets fixed in Fedora, you get an update in the Launchpad bug tracker. Regards, Anthony Liguori Daniel.
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On 05/20/2010 01:47 PM, Luiz Capitulino wrote: On Thu, 20 May 2010 11:55:00 -0500 Anthony Liguori wrote: On 05/20/2010 11:27 AM, Luiz Capitulino wrote: On Thu, 20 May 2010 10:50:41 -0500 Anthony Liguori wrote: On 05/20/2010 10:16 AM, Paolo Bonzini wrote: On 05/20/2010 03:44 PM, Luiz Capitulino wrote: I think there's another issue in the handling of strings. The spec says that valid unescaped chars are in the following range: unescaped = %x20-21 / %x23-5B / %x5D-10 That's a spec bug IMHO. Tab is %x09. Surely you can include tabs in strings. Any parser that didn't accept that would be broken. Honestly, I had the impression this should be encoded as: %x5C %x74, but if you're right, wouldn't this be true for other sequences as well? I don't think most reasonable clients are going to quote tabs as '\t'. That would be a bug, wouldn't it? Tabs are valid in JavaScript strings and I don't think it's reasonable to expect that a valid JavaScript string is not a valid JSON string. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 04/22] tcg-i386: Tidy ext8s and ext16s operations.
On Tue, Apr 13, 2010 at 04:13:49PM -0700, Richard Henderson wrote: > Define OPC_MOVSBL and OPC_MOVSWL. Factor opcode emission to > separate functions. Don't restrict the input register to the > low 4 "q" registers; emit shifts instead if needed. > Given this patch is of the same type as the previous one, I have also benchmarked it, here are the results: | instr | size | +++ before | 101258 | 344829 | after | 101258 | 344833 | This time the patch clearly doesn't bring an improvement, so I think it should also be rewritten without the constraints change. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 03/22] tcg-i386: Tidy ext8u and ext16u operations.
On Thu, May 20, 2010 at 07:40:59AM -0700, Richard Henderson wrote: > On 05/20/2010 07:04 AM, Aurelien Jarno wrote: > >> Do you have tried to compare the generated code before and after your > >> patch? I expect a few cases where your patch has some drawbacks, so I > >> don't know if there is a net gain on the size of the translated code. > >> > > > > I have done a quick test on /bin/ls. > >| instr | size | > >+++ > > before | 101305 | 344770 | > > after | 101258 | 344829 | > > > > In short a small gain in the number of instructions, and a small loss in > > the size of the translated code. > > That was pretty much the test I would have done. > > So where are we? Is the patch acceptable as-is, or should I be > re-writing it without the constraints change? > Given the tests do not show a real improvement and given that it complexify the code generation, I don't think we should have such a patch. Could you please rewrite it without the constraints change? -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On Thu, 20 May 2010 11:55:00 -0500 Anthony Liguori wrote: > On 05/20/2010 11:27 AM, Luiz Capitulino wrote: > > On Thu, 20 May 2010 10:50:41 -0500 > > Anthony Liguori wrote: > > > > > >> On 05/20/2010 10:16 AM, Paolo Bonzini wrote: > >> > >>> On 05/20/2010 03:44 PM, Luiz Capitulino wrote: > >>> > I think there's another issue in the handling of strings. > > The spec says that valid unescaped chars are in the following range: > > unescaped = %x20-21 / %x23-5B / %x5D-10 > > >> That's a spec bug IMHO. Tab is %x09. Surely you can include tabs in > >> strings. Any parser that didn't accept that would be broken. > >> > > Honestly, I had the impression this should be encoded as: %x5C %x74, but > > if you're right, wouldn't this be true for other sequences as well? > > > > I don't think most reasonable clients are going to quote tabs as '\t'. That would be a bug, wouldn't it? Python example: >>> json.dumps('\t') '"\\t"' >>> YAJL example (inlined below): /tmp/ ./teste 0x22 0x5c 0x74 0x22 /tmp/ I think we should strictly conform to the spec, quirks should only be added when really needed. #include #include int main(void) { yajl_gen g; unsigned int i, len = 0; const unsigned char *str = NULL; yajl_gen_config conf = { 0, " " }; g = yajl_gen_alloc(&conf, NULL); if (yajl_gen_string(g, (unsigned char *) "\t", 1) != yajl_gen_status_ok) return 1; if (yajl_gen_get_buf(g, &str, &len) != yajl_gen_status_ok) return 1; for (i = 0; i < len; i++) printf("0x%x ", str[i]); printf("\n"); return 0; }
[Qemu-devel] [Bug 583462] Re: qemu disables screensaver
** Attachment added: "Code used to check idle time." http://launchpadlibrarian.net/48825708/idletime -- qemu disables screensaver https://bugs.launchpad.net/bugs/583462 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: lucid, with compiz and fglrx: Screensaver on host will not kick in when qemu is running (kvm or no kvm). It seems to be related to the fact that the idle time reported by libXss.so on the host is being reset every four seconds or so when qemu is running, eventhough there is no activity on either guest or host.
[Qemu-devel] [Bug 583462] [NEW] qemu disables screensaver
Public bug reported: lucid, with compiz and fglrx: Screensaver on host will not kick in when qemu is running (kvm or no kvm). It seems to be related to the fact that the idle time reported by libXss.so on the host is being reset every four seconds or so when qemu is running, eventhough there is no activity on either guest or host. ** Affects: qemu Importance: Undecided Status: New -- qemu disables screensaver https://bugs.launchpad.net/bugs/583462 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: lucid, with compiz and fglrx: Screensaver on host will not kick in when qemu is running (kvm or no kvm). It seems to be related to the fact that the idle time reported by libXss.so on the host is being reset every four seconds or so when qemu is running, eventhough there is no activity on either guest or host.
[Qemu-devel] [Bug 241119] Re: usb_add of a Creative ZEN unrecognized in guest
** Also affects: qemu Importance: Undecided Status: New -- usb_add of a Creative ZEN unrecognized in guest https://bugs.launchpad.net/bugs/241119 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Status in “qemu-kvm” package in Ubuntu: Confirmed Bug description: Binary package hint: kvm This happens when I add my Creative ZEN to a virtual machine running XP. The device is recognised well at first and drivers are installed correctly. But when trying to connect windows crashes with the classic blue screen It complains about something like usbohci.sys, I can't read well because it crashes too fast. I have also tried with another virtual machine running Vista, same results. Any help would be really appreciated! I'm using the module kvm-amd with Ubuntu 8.04 The USB device has the following ID: 041e:4157 Creative Technology, Ltd kvm: Installed: 1:62+dfsg-0ubuntu7 Candidate: 1:62+dfsg-0ubuntu7 Version table: *** 1:62+dfsg-0ubuntu7 0 500 http://archive.ubuntu.com hardy/main Packages 100 /var/lib/dpkg/status
[Qemu-devel] Re: [PATCH 0/6]: QMP: Fix issues in parser/lexer
On Thu, 20 May 2010 10:52:58 -0500 Anthony Liguori wrote: > On 05/20/2010 10:18 AM, Paolo Bonzini wrote: > > On 05/19/2010 11:43 PM, Anthony Liguori wrote: > >> > >>> 4. Lexer expects a 'terminal' char to process a token > >>> > >>> Which means clients must send a sort of end of line char, so > >>> that we > >>> process their input. > >>> > >>> Maybe I'm missing something here, but I thought that the whole > >>> point of writing our own parser was to avoid this. > >> > >> If the lexer gets: > >> > >> "abc" > >> > >> It has no way of knowing if that's a token or if we're going to get: > >> > >> "abcd" > > > > Only } and ] are valid characters at the end of a JSON object, and > > neither requires lookahead. > > Having look ahead operate differently for different states really > complicates the lexer. I don't see this as a big problem in practice. Would be a nice feature, but it's fine for me too and we'll have to note that in the QMP's spec.
[Qemu-devel] Re: [PATCH 1/2] arm_timer: reload timer when enabled
On Sun, May 02, 2010 at 03:20:51PM +0530, Rabin Vincent wrote: > Reload the timer when TimerControl is written, if the timer is to be > enabled. Otherwise, if an earlier write to TimerLoad was done while > periodic mode was not set, s->delta may incorrectly still have the value > of the maximum limit instead of the value written to TimerLoad. > > This problem is evident on versatileap on current linux-next, which > enables TIMER_CTRL_32BIT before writing to TimerLoad and then enabling > periodic mode and starting the timer. This causes the first periodic > tick to be scheduled to occur after 0x periods, leading to a > perceived hang while the kernel waits for the first timer tick. > > Signed-off-by: Rabin Vincent Could these patches please be applied? What was then linux-next is now current Linux mainline, and it doesn't boot without this patch. Rabin
Re: [Qemu-devel] [Bug 391879] Re: migrate exec ignores exit status
On Thu, May 20, 2010 at 12:11 PM, Daniel P. Berrange wrote: > This bug appears to be filed against the Ubuntu qemu component, > rather than the upstream qemu component. Are we supposed to be > getting notifications for all Ubuntu distro qemu bugs too, rather > than just usptream bug reports ? This bug is filed as affecting both the qemu-kvm package in Ubuntu, as well as the QEMU project (upstream). Activity in the bug is sent to subscribed parties of both the affected package, and the affected project. :-Dustin
Re: [Qemu-devel] [Bug 391879] Re: migrate exec ignores exit status
On Thu, May 20, 2010 at 04:50:59PM -, Dave Walker wrote: > This is a bug and has been reported upstream, it is unlikely to be fixed > at the distribution level and therefore anyone interested in working on > this bug should contribute a patch to the upstream project. This will > then filter down to Ubuntu when it is merged mainline. Marking "Won't > Fix" against the Ubuntu package. > > Thanks for reporting this bug. > > ** Changed in: qemu-kvm (Ubuntu) >Status: Confirmed => Won't Fix > > -- > migrate exec ignores exit status > https://bugs.launchpad.net/bugs/391879 > You received this bug notification because you are a member of qemu- > devel-ml, which is subscribed to QEMU. This bug appears to be filed against the Ubuntu qemu component, rather than the upstream qemu component. Are we supposed to be getting notifications for all Ubuntu distro qemu bugs too, rather than just usptream bug reports ? Daniel. -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
[Qemu-devel] [Bug 391879] Re: migrate exec ignores exit status
This is a bug and has been reported upstream, it is unlikely to be fixed at the distribution level and therefore anyone interested in working on this bug should contribute a patch to the upstream project. This will then filter down to Ubuntu when it is merged mainline. Marking "Won't Fix" against the Ubuntu package. Thanks for reporting this bug. ** Changed in: qemu-kvm (Ubuntu) Status: Confirmed => Won't Fix -- migrate exec ignores exit status https://bugs.launchpad.net/bugs/391879 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Status in “qemu-kvm” package in Ubuntu: Won't Fix Bug description: Binary package hint: kvm Using migrate "exec:cat > foo; false" in the monitor results in the state of the VM being written to foo, as expected, and the VM then being stopped. This is surprising, as I think it stands to reason that in case of a failed migrate-exec process, which is what a non-zero exit status implies to me, the VM should continue. == Version information $ lsb_release -rd Description:Ubuntu 9.04 Release:9.04 $ apt-cache policy kvm kvm: Installed: 1:84+dfsg-0ubuntu11 Candidate: 1:84+dfsg-0ubuntu11 Version table: *** 1:84+dfsg-0ubuntu11 0 500 http://gb.archive.ubuntu.com jaunty/main Packages 100 /var/lib/dpkg/status
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On 05/20/2010 11:27 AM, Luiz Capitulino wrote: On Thu, 20 May 2010 10:50:41 -0500 Anthony Liguori wrote: On 05/20/2010 10:16 AM, Paolo Bonzini wrote: On 05/20/2010 03:44 PM, Luiz Capitulino wrote: I think there's another issue in the handling of strings. The spec says that valid unescaped chars are in the following range: unescaped = %x20-21 / %x23-5B / %x5D-10 That's a spec bug IMHO. Tab is %x09. Surely you can include tabs in strings. Any parser that didn't accept that would be broken. Honestly, I had the impression this should be encoded as: %x5C %x74, but if you're right, wouldn't this be true for other sequences as well? I don't think most reasonable clients are going to quote tabs as '\t'. Regards, Anthony Liguori But we do: [IN_DQ_STRING] = { [1 ... 0xFF] = IN_DQ_STRING, ['\\'] = IN_DQ_STRING_ESCAPE, ['"'] = IN_DONE_STRING, }, Shouldn't we cover 0x20 .. 0xFF instead? If it's the lexer, isn't just it being liberal in what it accepts? I believe the parser correctly rejects invalid UTF-8 sequences. Will check.
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On Thu, 20 May 2010 10:54:42 -0500 Anthony Liguori wrote: > On 05/20/2010 10:35 AM, Luiz Capitulino wrote: > >> I meant that we're just accepting some invalid JSON and that's not a big > >> deal. > >> > > It can become a big deal if clients rely on it and for some reason we > > decide we should drop it. Ie. after QMP is declared stable such changes > > won't be allowed. > > > > Clients should only rely on standard JSON. Anything else is a bug in > the client. I feel this is like a trap, why exposing it if don't want clients to use them?
Re: [Qemu-devel] [PATCH 04/10] target-mips: refactor {c, abs}.cond.fmt insns
On Thu, May 20, 2010 at 08:34:16AM -0700, Richard Henderson wrote: > On 05/20/2010 07:52 AM, Nathan Froyd wrote: > > +/* Tests */ > > +#define OP_COND(name, cond) \ > > +#define OP_CONDI(name, cond) > > \ > > +#define OP_CONDZ(name, cond) \ > > What are these doing in this patch? They are zombies, come back from the grave of source control. v2 of the patch will be forthcoming once people have had time to comment on other parts of the patch. -Nathan
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On Thu, 20 May 2010 10:50:41 -0500 Anthony Liguori wrote: > On 05/20/2010 10:16 AM, Paolo Bonzini wrote: > > On 05/20/2010 03:44 PM, Luiz Capitulino wrote: > >> I think there's another issue in the handling of strings. > >> > >> The spec says that valid unescaped chars are in the following range: > >> > >> unescaped = %x20-21 / %x23-5B / %x5D-10 > > That's a spec bug IMHO. Tab is %x09. Surely you can include tabs in > strings. Any parser that didn't accept that would be broken. Honestly, I had the impression this should be encoded as: %x5C %x74, but if you're right, wouldn't this be true for other sequences as well? > >> > >> But we do: > >> > >> [IN_DQ_STRING] = { > >> [1 ... 0xFF] = IN_DQ_STRING, > >> ['\\'] = IN_DQ_STRING_ESCAPE, > >> ['"'] = IN_DONE_STRING, > >> }, > >> > >> Shouldn't we cover 0x20 .. 0xFF instead? > > > > If it's the lexer, isn't just it being liberal in what it accepts? > > I believe the parser correctly rejects invalid UTF-8 sequences. Will check.
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On 05/20/2010 10:35 AM, Luiz Capitulino wrote: I meant that we're just accepting some invalid JSON and that's not a big deal. It can become a big deal if clients rely on it and for some reason we decide we should drop it. Ie. after QMP is declared stable such changes won't be allowed. Clients should only rely on standard JSON. Anything else is a bug in the client. Regards, Anthony Liguori Yes, I know, the chances of someone relying on this kind of thing is probably almost zero. At the same time I think we should be very conservative if there's no good reason to do otherwise.
[Qemu-devel] Re: [PATCH 0/6]: QMP: Fix issues in parser/lexer
On 05/20/2010 10:18 AM, Paolo Bonzini wrote: On 05/19/2010 11:43 PM, Anthony Liguori wrote: 4. Lexer expects a 'terminal' char to process a token Which means clients must send a sort of end of line char, so that we process their input. Maybe I'm missing something here, but I thought that the whole point of writing our own parser was to avoid this. If the lexer gets: "abc" It has no way of knowing if that's a token or if we're going to get: "abcd" Only } and ] are valid characters at the end of a JSON object, and neither requires lookahead. Having look ahead operate differently for different states really complicates the lexer. I don't see this as a big problem in practice. Regards, Anthony Liguori Paolo
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On 05/20/2010 10:16 AM, Paolo Bonzini wrote: On 05/20/2010 03:44 PM, Luiz Capitulino wrote: I think there's another issue in the handling of strings. The spec says that valid unescaped chars are in the following range: unescaped = %x20-21 / %x23-5B / %x5D-10 That's a spec bug IMHO. Tab is %x09. Surely you can include tabs in strings. Any parser that didn't accept that would be broken. But we do: [IN_DQ_STRING] = { [1 ... 0xFF] = IN_DQ_STRING, ['\\'] = IN_DQ_STRING_ESCAPE, ['"'] = IN_DONE_STRING, }, Shouldn't we cover 0x20 .. 0xFF instead? If it's the lexer, isn't just it being liberal in what it accepts? I believe the parser correctly rejects invalid UTF-8 sequences. Regards, Anthony Liguori paolo
Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself
On 05/20/2010 05:34 PM, Rusty Russell wrote: Have just one ring, no indexes. The producer places descriptors into the ring and updates the head, The consumer copies out descriptors to be processed and copies back in completed descriptors. Chaining is always linear. The descriptors contain a tag that allow the producer to identify the completion. This could definitely work. The original reason for the page boundaries was for untrusted inter-guest communication: with appropriate page protections they could see each other's rings and a simply inter-guest copy hypercall could verify that the other guest really exposed that data via virtio ring. But, cute as that is, we never did that. And it's not clear that it wins much over simply having the hypervisor read both rings directly. AFAICS having separate avail_ring/used_ring/desc_pool is orthogonal to this cuteness. Can we do better? The obvious idea is to try to get rid of last_used and used, and use the ring itself. We would use an invalid entry to mark the head of the ring. Interesting! So a peer will read until it hits a wall. But how to update the wall atomically? Maybe we can have a flag in the descriptor indicate headness or tailness. Update looks ugly though: write descriptor with head flag, write next descriptor with head flag, remove flag from previous descriptor. I was thinking a separate magic "invalid" entry. To publish an 3 descriptor chain, you would write descriptors 2 and 3, write an invalid entry at 4, barrier, write entry 1. It is a bit ugly, yes, but not terrible. Worth exploring. This amortizes the indexes into the ring, a good thing. Another thing we can do is place the tail a half ring away from the head (and limit ring utilization to 50%), reducing bounces on short kicks. Or equivalently have an avail ring and used ring, but both containing tagged descriptors instead of pointers to descriptors. I think that a simple simulator for this is worth writing, which tracks cacheline moves under various fullness scenarios... Yup. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic.
Re: [Qemu-devel] [PATCH 04/10] target-mips: refactor {c, abs}.cond.fmt insns
On 05/20/2010 07:52 AM, Nathan Froyd wrote: > +/* Tests */ > +#define OP_COND(name, cond) \ > +static inline void glue(gen_op_, name) (TCGv ret, TCGv t0, TCGv t1) \ > +{ \ > +int l1 = gen_new_label(); \ > +int l2 = gen_new_label(); \ > +\ > +tcg_gen_brcond_tl(cond, t0, t1, l1);\ > +tcg_gen_movi_tl(ret, 0);\ > +tcg_gen_br(l2); \ > +gen_set_label(l1); \ > +tcg_gen_movi_tl(ret, 1);\ > +gen_set_label(l2); \ > +} > +OP_COND(eq, TCG_COND_EQ); > +OP_COND(ne, TCG_COND_NE); > +OP_COND(ge, TCG_COND_GE); > +OP_COND(geu, TCG_COND_GEU); > +OP_COND(lt, TCG_COND_LT); > +OP_COND(ltu, TCG_COND_LTU); > +#undef OP_COND > + > +#define OP_CONDI(name, cond) > \ ... > +#define OP_CONDZ(name, cond) \ What are these doing in this patch? r~
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On Thu, 20 May 2010 17:26:03 +0200 Paolo Bonzini wrote: > On 05/20/2010 05:25 PM, Luiz Capitulino wrote: > > On Thu, 20 May 2010 17:16:01 +0200 > > Paolo Bonzini wrote: > > > >> On 05/20/2010 03:44 PM, Luiz Capitulino wrote: > >>>I think there's another issue in the handling of strings. > >>> > >>>The spec says that valid unescaped chars are in the following range: > >>> > >>> unescaped = %x20-21 / %x23-5B / %x5D-10 > >>> > >>>But we do: > >>> > >>> [IN_DQ_STRING] = { > >>> [1 ... 0xFF] = IN_DQ_STRING, > >>> ['\\'] = IN_DQ_STRING_ESCAPE, > >>> ['"'] = IN_DONE_STRING, > >>> }, > >>> > >>>Shouldn't we cover 0x20 .. 0xFF instead? > >> > >> If it's the lexer, isn't just it being liberal in what it accepts? > > > > Yes, it's the lexer, but you meant that the fix should be in > > somewhere else? > > I meant that we're just accepting some invalid JSON and that's not a big > deal. It can become a big deal if clients rely on it and for some reason we decide we should drop it. Ie. after QMP is declared stable such changes won't be allowed. Yes, I know, the chances of someone relying on this kind of thing is probably almost zero. At the same time I think we should be very conservative if there's no good reason to do otherwise.
[Qemu-devel] Re: [PATCH 0/6]: QMP: Fix issues in parser/lexer
On Thu, 20 May 2010 17:18:23 +0200 Paolo Bonzini wrote: > On 05/19/2010 11:43 PM, Anthony Liguori wrote: > > > >> 4. Lexer expects a 'terminal' char to process a token > >> > >> Which means clients must send a sort of end of line char, so that we > >> process their input. > >> > >> Maybe I'm missing something here, but I thought that the whole > >> point of writing our own parser was to avoid this. > > > > If the lexer gets: > > > > "abc" > > > > It has no way of knowing if that's a token or if we're going to get: > > > > "abcd" > > Only } and ] are valid characters at the end of a JSON object, and > neither requires lookahead. Good point.
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On Thu, 20 May 2010 17:16:01 +0200 Paolo Bonzini wrote: > On 05/20/2010 03:44 PM, Luiz Capitulino wrote: > > I think there's another issue in the handling of strings. > > > > The spec says that valid unescaped chars are in the following range: > > > > unescaped = %x20-21 / %x23-5B / %x5D-10 > > > > But we do: > > > > [IN_DQ_STRING] = { > > [1 ... 0xFF] = IN_DQ_STRING, > > ['\\'] = IN_DQ_STRING_ESCAPE, > > ['"'] = IN_DONE_STRING, > > }, > > > > Shouldn't we cover 0x20 .. 0xFF instead? > > If it's the lexer, isn't just it being liberal in what it accepts? Yes, it's the lexer, but you meant that the fix should be in somewhere else?
[Qemu-devel] Re: [PATCH 0/6]: QMP: Fix issues in parser/lexer
On 05/19/2010 11:43 PM, Anthony Liguori wrote: 4. Lexer expects a 'terminal' char to process a token Which means clients must send a sort of end of line char, so that we process their input. Maybe I'm missing something here, but I thought that the whole point of writing our own parser was to avoid this. If the lexer gets: "abc" It has no way of knowing if that's a token or if we're going to get: "abcd" Only } and ] are valid characters at the end of a JSON object, and neither requires lookahead. Paolo
[Qemu-devel] Re: [PATCH] QEMU: Change default disk caching to nocache
On 05/20/2010 11:32 AM, jes.soren...@redhat.com wrote: +if (bdrv_flags & BDRV_O_NOCACHE) { +fprintf(stderr, "qemu: failed to open disk image %s as " +"nocache (O_DIRECT) retrying as write-back\n", file); +bdrv_flags &= BDRV_O_NOCACHE; Missing ~ here. +bdrv_flags |= BDRV_O_CACHE_WB; +if (bdrv_open(dinfo->bdrv, file, bdrv_flags, drv)< 0) +goto error_open; +} else { I think the retry should be done silently if no cache= option is given. That is cache=none will be the default but: - if it is not specified and not supported by the image, fall back to writeback with no warning. However, this is just a QoI issue and can be fixed later. - if it is specified and not supported by the image, either fall back to writeback with a warning, or fail altogether. The former would be a change in behavior, so it has to be documented somewhere if it changes. Or maybe add BDRV_O_CACHE_WT and let the backend decide the default? Paolo
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On 05/20/2010 05:25 PM, Luiz Capitulino wrote: On Thu, 20 May 2010 17:16:01 +0200 Paolo Bonzini wrote: On 05/20/2010 03:44 PM, Luiz Capitulino wrote: I think there's another issue in the handling of strings. The spec says that valid unescaped chars are in the following range: unescaped = %x20-21 / %x23-5B / %x5D-10 But we do: [IN_DQ_STRING] = { [1 ... 0xFF] = IN_DQ_STRING, ['\\'] = IN_DQ_STRING_ESCAPE, ['"'] = IN_DONE_STRING, }, Shouldn't we cover 0x20 .. 0xFF instead? If it's the lexer, isn't just it being liberal in what it accepts? Yes, it's the lexer, but you meant that the fix should be in somewhere else? I meant that we're just accepting some invalid JSON and that's not a big deal. Paolo
[Qemu-devel] Re: [PATCH 2/6] json-lexer: Handle missing escapes
On 05/20/2010 03:44 PM, Luiz Capitulino wrote: I think there's another issue in the handling of strings. The spec says that valid unescaped chars are in the following range: unescaped = %x20-21 / %x23-5B / %x5D-10 But we do: [IN_DQ_STRING] = { [1 ... 0xFF] = IN_DQ_STRING, ['\\'] = IN_DQ_STRING_ESCAPE, ['"'] = IN_DONE_STRING, }, Shouldn't we cover 0x20 .. 0xFF instead? If it's the lexer, isn't just it being liberal in what it accepts? paolo
[Qemu-devel] [PATCH 03/10] target-mips: add enum constants for various invocations of FOP
Tweak gen_farith and its caller to use them. Signed-off-by: Nathan Froyd --- target-mips/translate.c | 266 --- 1 files changed, 180 insertions(+), 86 deletions(-) diff --git a/target-mips/translate.c b/target-mips/translate.c index 2075d09..2568e16 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -5714,6 +5714,100 @@ static void gen_compute_branch1 (CPUState *env, DisasContext *ctx, uint32_t op, #define FOP(func, fmt) (((fmt) << 21) | (func)) +enum { +OPC_ADD_S = FOP(0, FMT_S), +OPC_SUB_S = FOP(1, FMT_S), +OPC_MUL_S = FOP(2, FMT_S), +OPC_DIV_S = FOP(3, FMT_S), +OPC_SQRT_S = FOP(4, FMT_S), +OPC_ABS_S = FOP(5, FMT_S), +OPC_MOV_S = FOP(6, FMT_S), +OPC_NEG_S = FOP(7, FMT_S), +OPC_ROUND_L_S = FOP(8, FMT_S), +OPC_TRUNC_L_S = FOP(9, FMT_S), +OPC_CEIL_L_S = FOP(10, FMT_S), +OPC_FLOOR_L_S = FOP(11, FMT_S), +OPC_ROUND_W_S = FOP(12, FMT_S), +OPC_TRUNC_W_S = FOP(13, FMT_S), +OPC_CEIL_W_S = FOP(14, FMT_S), +OPC_FLOOR_W_S = FOP(15, FMT_S), +OPC_MOVCF_S = FOP(17, FMT_S), +OPC_MOVZ_S = FOP(18, FMT_S), +OPC_MOVN_S = FOP(19, FMT_S), +OPC_RECIP_S = FOP(21, FMT_S), +OPC_RSQRT_S = FOP(22, FMT_S), +OPC_RECIP2_S = FOP(28, FMT_S), +OPC_RECIP1_S = FOP(29, FMT_S), +OPC_RSQRT1_S = FOP(30, FMT_S), +OPC_RSQRT2_S = FOP(31, FMT_S), +OPC_CVT_D_S = FOP(33, FMT_S), +OPC_CVT_W_S = FOP(36, FMT_S), +OPC_CVT_L_S = FOP(37, FMT_S), +OPC_CVT_PS_S = FOP(38, FMT_S), +/* FOP(48..63, FMT_S) used for comparisons */ +OPC_ADD_D = FOP(0, FMT_D), +OPC_SUB_D = FOP(1, FMT_D), +OPC_MUL_D = FOP(2, FMT_D), +OPC_DIV_D = FOP(3, FMT_D), +OPC_SQRT_D = FOP(4, FMT_D), +OPC_ABS_D = FOP(5, FMT_D), +OPC_MOV_D = FOP(6, FMT_D), +OPC_NEG_D = FOP(7, FMT_D), +OPC_ROUND_L_D = FOP(8, FMT_D), +OPC_TRUNC_L_D = FOP(9, FMT_D), +OPC_CEIL_L_D = FOP(10, FMT_D), +OPC_FLOOR_L_D = FOP(11, FMT_D), +OPC_ROUND_W_D = FOP(12, FMT_D), +OPC_TRUNC_W_D = FOP(13, FMT_D), +OPC_CEIL_W_D = FOP(14, FMT_D), +OPC_FLOOR_W_D = FOP(15, FMT_D), +OPC_MOVCF_D = FOP(17, FMT_D), +OPC_MOVZ_D = FOP(18, FMT_D), +OPC_MOVN_D = FOP(19, FMT_D), +OPC_RECIP_D = FOP(21, FMT_D), +OPC_RSQRT_D = FOP(22, FMT_D), +OPC_RECIP2_D = FOP(28, FMT_D), +OPC_RECIP1_D = FOP(29, FMT_D), +OPC_RSQRT1_D = FOP(30, FMT_D), +OPC_RSQRT2_D = FOP(31, FMT_D), +OPC_CVT_S_D = FOP(32, FMT_D), +OPC_CVT_W_D = FOP(36, FMT_D), +OPC_CVT_L_D = FOP(37, FMT_D), +/* FOP(48..63, FMT_D) used for comparisons */ + +OPC_CVT_S_W = FOP(32, FMT_W), +OPC_CVT_D_W = FOP(33, FMT_W), +OPC_CVT_S_L = FOP(32, FMT_L), +OPC_CVT_D_L = FOP(33, FMT_L), +OPC_CVT_PS_PW = FOP(38, FMT_W), + +OPC_ADD_PS = FOP(0, FMT_PS), +OPC_SUB_PS = FOP(1, FMT_PS), +OPC_MUL_PS = FOP(2, FMT_PS), +OPC_DIV_PS = FOP(3, FMT_PS), +OPC_ABS_PS = FOP(5, FMT_PS), +OPC_MOV_PS = FOP(6, FMT_PS), +OPC_NEG_PS = FOP(7, FMT_PS), +OPC_MOVCF_PS = FOP(17, FMT_PS), +OPC_MOVZ_PS = FOP(18, FMT_PS), +OPC_MOVN_PS = FOP(19, FMT_PS), +OPC_ADDR_PS = FOP(24, FMT_PS), +OPC_MULR_PS = FOP(26, FMT_PS), +OPC_RECIP2_PS = FOP(28, FMT_PS), +OPC_RECIP1_PS = FOP(29, FMT_PS), +OPC_RSQRT1_PS = FOP(30, FMT_PS), +OPC_RSQRT2_PS = FOP(31, FMT_PS), + +OPC_CVT_S_PU = FOP(32, FMT_PS), +OPC_CVT_PW_PS = FOP(36, FMT_PS), +OPC_CVT_S_PL = FOP(40, FMT_PS), +OPC_PLL_PS = FOP(44, FMT_PS), +OPC_PLU_PS = FOP(45, FMT_PS), +OPC_PUL_PS = FOP(46, FMT_PS), +OPC_PUU_PS = FOP(47, FMT_PS), +/* FOP(48..63, FMT_PS) used for comparisons */ +}; + static void gen_cp1 (DisasContext *ctx, uint32_t opc, int rt, int fs) { const char *opn = "cp1 move"; @@ -5937,8 +6031,8 @@ static void gen_farith (DisasContext *ctx, uint32_t op1, enum { BINOP, CMPOP, OTHEROP } optype = OTHEROP; uint32_t func = ctx->opcode & 0x3f; -switch (ctx->opcode & FOP(0x3f, 0x1f)) { -case FOP(0, 16): +switch (opc) { +case OPC_ADD_S: { TCGv_i32 fp0 = tcg_temp_new_i32(); TCGv_i32 fp1 = tcg_temp_new_i32(); @@ -5953,7 +6047,7 @@ static void gen_farith (DisasContext *ctx, uint32_t op1, opn = "add.s"; optype = BINOP; break; -case FOP(1, 16): +case OPC_SUB_S: { TCGv_i32 fp0 = tcg_temp_new_i32(); TCGv_i32 fp1 = tcg_temp_new_i32(); @@ -5968,7 +6062,7 @@ static void gen_farith (DisasContext *ctx, uint32_t op1, opn = "sub.s"; optype = BINOP; break; -case FOP(2, 16): +case OPC_MUL_S: { TCGv_i32 fp0 = tcg_temp_new_i32(); TCGv_i32 fp1 = tcg_temp_new_i32(); @@ -5983,7 +6077,7 @@ static void gen_farith (DisasContext *ctx, uint32_t op1, opn = "mul.s"; optype = BINOP; break; -case FOP(3, 16): +case OPC_DIV_S: { TCGv_i32 fp0 = tcg_temp
[Qemu-devel] [PATCH 08/10] target-mips: add microMIPS exception handler support
Unlike MIPS16, microMIPS lets you choose the ISA mode for your exception handlers. Signed-off-by: Nathan Froyd --- target-mips/helper.c | 21 +++-- 1 files changed, 15 insertions(+), 6 deletions(-) diff --git a/target-mips/helper.c b/target-mips/helper.c index 8102f03..90c3b3a 100644 --- a/target-mips/helper.c +++ b/target-mips/helper.c @@ -385,6 +385,18 @@ static target_ulong exception_resume_pc (CPUState *env) return bad_pc; } +static void set_hflags_for_handler (CPUState *env) +{ +/* Exception handlers are entered in 32-bit mode. */ +env->hflags &= ~(MIPS_HFLAG_M16); +/* ...except that microMIPS lets you choose. */ +if (env->insn_flags & ASE_MICROMIPS) { +env->hflags |= (!!(env->CP0_Config3 + & (1 << CP0C3_ISA_ON_EXC)) +<< MIPS_HFLAG_M16_SHIFT); +} +} + #endif void do_interrupt (CPUState *env) @@ -440,8 +452,7 @@ void do_interrupt (CPUState *env) if (!(env->CP0_Status & (1 << CP0St_EXL))) env->CP0_Cause &= ~(1 << CP0Ca_BD); env->active_tc.PC = (int32_t)0xBFC00480; -/* Exception handlers are entered in 32-bit mode. */ -env->hflags &= ~(MIPS_HFLAG_M16); +set_hflags_for_handler(env); break; case EXCP_RESET: cpu_reset(env); @@ -461,8 +472,7 @@ void do_interrupt (CPUState *env) if (!(env->CP0_Status & (1 << CP0St_EXL))) env->CP0_Cause &= ~(1 << CP0Ca_BD); env->active_tc.PC = (int32_t)0xBFC0; -/* Exception handlers are entered in 32-bit mode. */ -env->hflags &= ~(MIPS_HFLAG_M16); +set_hflags_for_handler(env); break; case EXCP_EXT_INTERRUPT: cause = 0; @@ -581,8 +591,7 @@ void do_interrupt (CPUState *env) env->active_tc.PC = (int32_t)(env->CP0_EBase & ~0x3ff); } env->active_tc.PC += offset; -/* Exception handlers are entered in 32-bit mode. */ -env->hflags &= ~(MIPS_HFLAG_M16); +set_hflags_for_handler(env); env->CP0_Cause = (env->CP0_Cause & ~(0x1f << CP0Ca_EC)) | (cause << CP0Ca_EC); break; default: -- 1.6.3.2
[Qemu-devel] [PATCH 04/10] target-mips: refactor {c, abs}.cond.fmt insns
Move all knowledge about coprocessor-checking and register numbering into the gen_cmp* helper functions. Signed-off-by: Nathan Froyd --- target-mips/translate.c | 232 ++- 1 files changed, 149 insertions(+), 83 deletions(-) diff --git a/target-mips/translate.c b/target-mips/translate.c index 2568e16..8a7f3e9 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -354,6 +354,18 @@ enum { /* Coprocessor 1 (rs field) */ #define MASK_CP1(op) MASK_OP_MAJOR(op) | (op & (0x1F << 21)) +/* Values for the fmt field in FP instructions */ +enum { +/* 0 - 15 are reserved */ +FMT_S = 16, +FMT_D = 17, +/* 18 - 19 are reserved */ +FMT_W = 20, +FMT_L = 21, +FMT_PS = 22, +/* 23 - 31 are reserved */ +}; + enum { OPC_MFC1 = (0x00 << 21) | OPC_CP1, OPC_DMFC1= (0x01 << 21) | OPC_CP1, @@ -663,39 +675,6 @@ static inline int get_fp_bit (int cc) return 23; } -#define FOP_CONDS(type, fmt, bits)\ -static inline void gen_cmp ## type ## _ ## fmt(int n, TCGv_i##bits a, \ - TCGv_i##bits b, int cc)\ -{ \ -switch (n) { \ -case 0: gen_helper_2i(cmp ## type ## _ ## fmt ## _f, a, b, cc);break;\ -case 1: gen_helper_2i(cmp ## type ## _ ## fmt ## _un, a, b, cc); break;\ -case 2: gen_helper_2i(cmp ## type ## _ ## fmt ## _eq, a, b, cc); break;\ -case 3: gen_helper_2i(cmp ## type ## _ ## fmt ## _ueq, a, b, cc); break;\ -case 4: gen_helper_2i(cmp ## type ## _ ## fmt ## _olt, a, b, cc); break;\ -case 5: gen_helper_2i(cmp ## type ## _ ## fmt ## _ult, a, b, cc); break;\ -case 6: gen_helper_2i(cmp ## type ## _ ## fmt ## _ole, a, b, cc); break;\ -case 7: gen_helper_2i(cmp ## type ## _ ## fmt ## _ule, a, b, cc); break;\ -case 8: gen_helper_2i(cmp ## type ## _ ## fmt ## _sf, a, b, cc); break;\ -case 9: gen_helper_2i(cmp ## type ## _ ## fmt ## _ngle, a, b, cc); break;\ -case 10: gen_helper_2i(cmp ## type ## _ ## fmt ## _seq, a, b, cc); break;\ -case 11: gen_helper_2i(cmp ## type ## _ ## fmt ## _ngl, a, b, cc); break;\ -case 12: gen_helper_2i(cmp ## type ## _ ## fmt ## _lt, a, b, cc); break;\ -case 13: gen_helper_2i(cmp ## type ## _ ## fmt ## _nge, a, b, cc); break;\ -case 14: gen_helper_2i(cmp ## type ## _ ## fmt ## _le, a, b, cc); break;\ -case 15: gen_helper_2i(cmp ## type ## _ ## fmt ## _ngt, a, b, cc); break;\ -default: abort(); \ -} \ -} - -FOP_CONDS(, d, 64) -FOP_CONDS(abs, d, 64) -FOP_CONDS(, s, 32) -FOP_CONDS(abs, s, 32) -FOP_CONDS(, ps, 64) -FOP_CONDS(abs, ps, 64) -#undef FOP_CONDS - /* Tests */ static inline void gen_save_pc(target_ulong pc) { @@ -836,6 +815,125 @@ static inline void check_mips_64(DisasContext *ctx) generate_exception(ctx, EXCP_RI); } +/* Define small wrappers for gen_load_fpr* so that we have a uniform + calling interface for 32 and 64-bit FPRs. No sense in changing + all callers for gen_load_fpr32 when we need the CTX parameter for + this one use. */ +#define gen_ldcmp_fpr32(ctx, x, y) gen_load_fpr32(x, y) +#define gen_ldcmp_fpr64(ctx, x, y) gen_load_fpr64(ctx, x, y) +#define FOP_CONDS(type, abs, fmt, ifmt, bits) \ +static inline void gen_cmp ## type ## _ ## fmt(DisasContext *ctx, int n, \ + int ft, int fs, int cc)\ +{ \ +TCGv_i##bits fp0 = tcg_temp_new_i##bits (); \ +TCGv_i##bits fp1 = tcg_temp_new_i##bits (); \ +switch (ifmt) { \ +case FMT_PS: \ +check_cp1_64bitmode(ctx); \ +break;\ +case FMT_D: \ +if (abs) \ +check_cop1x(ctx); \ +check_cp1_registers(ctx, fs | ft);\ +break;\ +case FMT_S: \ +if (abs) \ +check_cop1x(ctx); \ +
[Qemu-devel] [PATCH 09/10] linux-user: honor low bit of entry PC for MIPS
Signed-off-by: Nathan Froyd --- linux-user/main.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/linux-user/main.c b/linux-user/main.c index 18b52c0..76d443b 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -3192,7 +3192,9 @@ int main(int argc, char **argv, char **envp) for(i = 0; i < 32; i++) { env->active_tc.gpr[i] = regs->regs[i]; } -env->active_tc.PC = regs->cp0_epc; +env->active_tc.PC = regs->cp0_epc & ~(target_ulong)1; +if (regs->cp0_epc & 1) +env->hflags |= MIPS_HFLAG_M16; } #elif defined(TARGET_SH4) { -- 1.6.3.2
[Qemu-devel] [PATCH 01/10] target-mips: break out [ls][wd]c1 and rdhwr insn generation
Signed-off-by: Nathan Froyd --- target-mips/translate.c | 106 ++- 1 files changed, 59 insertions(+), 47 deletions(-) diff --git a/target-mips/translate.c b/target-mips/translate.c index c95ecb1..2075d09 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -1220,6 +1220,17 @@ static void gen_flt_ldst (DisasContext *ctx, uint32_t opc, int ft, tcg_temp_free(t0); } +static void gen_cop1_ldst(CPUState *env, DisasContext *ctx, + uint32_t op, int rt, int rs, int16_t imm) +{ +if (env->CP0_Config1 & (1 << CP0C1_FP)) { +check_cp1_enabled(ctx); +gen_flt_ldst(ctx, op, rt, rs, imm); +} else { +generate_exception_err(ctx, EXCP_CpU, 1); +} +} + /* Arithmetic with immediate operand */ static void gen_arith_imm (CPUState *env, DisasContext *ctx, uint32_t opc, int rt, int rs, int16_t imm) @@ -7528,6 +7539,52 @@ static void gen_flt3_arith (DisasContext *ctx, uint32_t opc, fregnames[fs], fregnames[ft]); } +static void +gen_rdhwr (CPUState *env, DisasContext *ctx, int rt, int rd) +{ +TCGv t0; + +check_insn(env, ctx, ISA_MIPS32R2); +t0 = tcg_temp_new(); + +switch (rd) { +case 0: +save_cpu_state(ctx, 1); +gen_helper_rdhwr_cpunum(t0); +gen_store_gpr(t0, rt); +break; +case 1: +save_cpu_state(ctx, 1); +gen_helper_rdhwr_synci_step(t0); +gen_store_gpr(t0, rt); +break; +case 2: +save_cpu_state(ctx, 1); +gen_helper_rdhwr_cc(t0); +gen_store_gpr(t0, rt); +break; +case 3: +save_cpu_state(ctx, 1); +gen_helper_rdhwr_ccres(t0); +gen_store_gpr(t0, rt); +break; +case 29: +#if defined(CONFIG_USER_ONLY) +tcg_gen_ld_tl(t0, cpu_env, offsetof(CPUState, tls_value)); +gen_store_gpr(t0, rt); +break; +#else +/* XXX: Some CPUs implement this in hardware. + Not supported yet. */ +#endif +default:/* Invalid */ +MIPS_INVAL("rdhwr"); +generate_exception(ctx, EXCP_RI); +break; +} +tcg_temp_free(t0); +} + static void handle_delay_slot (CPUState *env, DisasContext *ctx, int insn_bytes) { @@ -8999,47 +9056,7 @@ static void decode_opc (CPUState *env, DisasContext *ctx, int *is_branch) gen_bshfl(ctx, op2, rt, rd); break; case OPC_RDHWR: -check_insn(env, ctx, ISA_MIPS32R2); -{ -TCGv t0 = tcg_temp_new(); - -switch (rd) { -case 0: -save_cpu_state(ctx, 1); -gen_helper_rdhwr_cpunum(t0); -gen_store_gpr(t0, rt); -break; -case 1: -save_cpu_state(ctx, 1); -gen_helper_rdhwr_synci_step(t0); -gen_store_gpr(t0, rt); -break; -case 2: -save_cpu_state(ctx, 1); -gen_helper_rdhwr_cc(t0); -gen_store_gpr(t0, rt); -break; -case 3: -save_cpu_state(ctx, 1); -gen_helper_rdhwr_ccres(t0); -gen_store_gpr(t0, rt); -break; -case 29: -#if defined(CONFIG_USER_ONLY) -tcg_gen_ld_tl(t0, cpu_env, offsetof(CPUState, tls_value)); -gen_store_gpr(t0, rt); -break; -#else -/* XXX: Some CPUs implement this in hardware. - Not supported yet. */ -#endif -default:/* Invalid */ -MIPS_INVAL("rdhwr"); -generate_exception(ctx, EXCP_RI); -break; -} -tcg_temp_free(t0); -} +gen_rdhwr(env, ctx, rt, rd); break; case OPC_FORK: check_insn(env, ctx, ASE_MT); @@ -9242,12 +9259,7 @@ static void decode_opc (CPUState *env, DisasContext *ctx, int *is_branch) case OPC_LDC1: case OPC_SWC1: case OPC_SDC1: -if (env->CP0_Config1 & (1 << CP0C1_FP)) { -check_cp1_enabled(ctx); -gen_flt_ldst(ctx, op, rt, rs, imm); -} else { -generate_exception_err(ctx, EXCP_CpU, 1); -} +gen_cop1_ldst(env, ctx, op, rt, rs, imm); break; case OPC_CP1: -- 1.6.3.2
[Qemu-devel] [PATCH 05/10] target-mips: small changes to use new FMT_ enums
Signed-off-by: Nathan Froyd --- target-mips/translate.c | 17 + 1 files changed, 9 insertions(+), 8 deletions(-) diff --git a/target-mips/translate.c b/target-mips/translate.c index 8a7f3e9..c42d8dd 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -359,7 +359,8 @@ enum { /* 0 - 15 are reserved */ FMT_S = 16, FMT_D = 17, -/* 18 - 19 are reserved */ +FMT_E = 18, +FMT_Q = 19, FMT_W = 20, FMT_L = 21, FMT_PS = 22, @@ -378,13 +379,13 @@ enum { OPC_BC1 = (0x08 << 21) | OPC_CP1, /* bc */ OPC_BC1ANY2 = (0x09 << 21) | OPC_CP1, OPC_BC1ANY4 = (0x0A << 21) | OPC_CP1, -OPC_S_FMT= (0x10 << 21) | OPC_CP1, /* 16: fmt=single fp */ -OPC_D_FMT= (0x11 << 21) | OPC_CP1, /* 17: fmt=double fp */ -OPC_E_FMT= (0x12 << 21) | OPC_CP1, /* 18: fmt=extended fp */ -OPC_Q_FMT= (0x13 << 21) | OPC_CP1, /* 19: fmt=quad fp */ -OPC_W_FMT= (0x14 << 21) | OPC_CP1, /* 20: fmt=32bit fixed */ -OPC_L_FMT= (0x15 << 21) | OPC_CP1, /* 21: fmt=64bit fixed */ -OPC_PS_FMT = (0x16 << 21) | OPC_CP1, /* 22: fmt=paired single fp */ +OPC_S_FMT= (FMT_S << 21) | OPC_CP1, /* 16: fmt=single fp */ +OPC_D_FMT= (FMT_D << 21) | OPC_CP1, /* 17: fmt=double fp */ +OPC_E_FMT= (FMT_E << 21) | OPC_CP1, /* 18: fmt=extended fp */ +OPC_Q_FMT= (FMT_Q << 21) | OPC_CP1, /* 19: fmt=quad fp */ +OPC_W_FMT= (FMT_W << 21) | OPC_CP1, /* 20: fmt=32bit fixed */ +OPC_L_FMT= (FMT_L << 21) | OPC_CP1, /* 21: fmt=64bit fixed */ +OPC_PS_FMT = (FMT_PS << 21) | OPC_CP1, /* 22: fmt=paired single fp */ }; #define MASK_CP1_FUNC(op) MASK_CP1(op) | (op & 0x3F) -- 1.6.3.2
[Qemu-devel] [PATCH 07/10] target-mips: add microMIPS CPUs
Signed-off-by: Nathan Froyd --- target-mips/translate_init.c | 61 ++ 1 files changed, 61 insertions(+), 0 deletions(-) diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c index b79ed56..8e17f4b 100644 --- a/target-mips/translate_init.c +++ b/target-mips/translate_init.c @@ -141,6 +141,25 @@ static const mips_def_t mips_defs[] = .mmu_type = MMU_TYPE_FMT, }, { +.name = "4Km-micromips", +.CP0_PRid = 0x00018300, +/* Config1 implemented, fixed mapping MMU, + no virtual icache, uncached coherency. */ +.CP0_Config0 = MIPS_CONFIG0 | (MMU_TYPE_FMT << CP0C0_MT), +.CP0_Config1 = MIPS_CONFIG1 | + (0 << CP0C1_IS) | (3 << CP0C1_IL) | (1 << CP0C1_IA) | + (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA), +.CP0_Config2 = MIPS_CONFIG2, +.CP0_Config3 = MIPS_CONFIG3, +.SYNCI_Step = 32, +.CCRes = 2, +.CP0_Status_rw_bitmask = 0x1258FF17, +.SEGBITS = 32, +.PABITS = 32, +.insn_flags = CPU_MIPS32R2 | ASE_MICROMIPS, +.mmu_type = MMU_TYPE_FMT, +}, +{ .name = "4KEcR1", .CP0_PRid = 0x00018400, .CP0_Config0 = MIPS_CONFIG0 | (MMU_TYPE_R4000 << CP0C0_MT), @@ -245,6 +264,25 @@ static const mips_def_t mips_defs[] = .mmu_type = MMU_TYPE_R4000, }, { +.name = "24Kc-micromips", +.CP0_PRid = 0x00019300, +.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | +(MMU_TYPE_R4000 << CP0C0_MT), +.CP0_Config1 = MIPS_CONFIG1 | (15 << CP0C1_MMU) | + (0 << CP0C1_IS) | (3 << CP0C1_IL) | (1 << CP0C1_IA) | + (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA), +.CP0_Config2 = MIPS_CONFIG2, +.CP0_Config3 = MIPS_CONFIG3 | (0 << CP0C3_VInt), +.SYNCI_Step = 32, +.CCRes = 2, +/* No DSP implemented. */ +.CP0_Status_rw_bitmask = 0x1278FF1F, +.SEGBITS = 32, +.PABITS = 32, +.insn_flags = CPU_MIPS32R2 | ASE_MICROMIPS, +.mmu_type = MMU_TYPE_R4000, +}, +{ .name = "24Kf", .CP0_PRid = 0x00019300, .CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | @@ -269,6 +307,29 @@ static const mips_def_t mips_defs[] = .mmu_type = MMU_TYPE_R4000, }, { +.name = "24Kf-micromips", +.CP0_PRid = 0x00019300, +.CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | +(MMU_TYPE_R4000 << CP0C0_MT), +.CP0_Config1 = MIPS_CONFIG1 | (1 << CP0C1_FP) | (15 << CP0C1_MMU) | + (0 << CP0C1_IS) | (3 << CP0C1_IL) | (1 << CP0C1_IA) | + (0 << CP0C1_DS) | (3 << CP0C1_DL) | (1 << CP0C1_DA), +.CP0_Config2 = MIPS_CONFIG2, +.CP0_Config3 = MIPS_CONFIG3 | (0 << CP0C3_VInt), +.CP0_LLAddr_rw_bitmask = 0, +.CP0_LLAddr_shift = 4, +.SYNCI_Step = 32, +.CCRes = 2, +/* No DSP implemented. */ +.CP0_Status_rw_bitmask = 0x3678FF1F, +.CP1_fcr0 = (1 << FCR0_F64) | (1 << FCR0_L) | (1 << FCR0_W) | +(1 << FCR0_D) | (1 << FCR0_S) | (0x93 << FCR0_PRID), +.SEGBITS = 32, +.PABITS = 32, +.insn_flags = CPU_MIPS32R2 | ASE_MICROMIPS, +.mmu_type = MMU_TYPE_R4000, +}, +{ .name = "34Kf", .CP0_PRid = 0x00019500, .CP0_Config0 = MIPS_CONFIG0 | (0x1 << CP0C0_AR) | -- 1.6.3.2
[Qemu-devel] [PATCH 00/10] target-mips: add microMIPS ASE support
This patch series adds support for the microMIPS ASE. microMIPS is a new ASE similar to MIPS16, but re-encodes the entire instruction set into 16-bit and 32-bit instructions--in contrast to MIPS16, which re-encodes only integer instructions. The mechanisms for going in and out of microMIPS mode are identical to those for MIPS16; a given chip cannot support both ASEs simultaneously. The first half of the series consists of small refactorings to make it easier to delegate microMIPS instruction decoding to the usual gen_* functions. The second half adds support for microMIPS in all the necessary places. The patch has been tested extensively in our QEMU tree; this patch has been tested against our compilers (GNU/Linux emulation), which include microMIPS support. We have obtained identical test results for MIPS32 and microMIPS testing. (The microMIPS patch for binutils has been posted upstream; the microMIPS patch for GCC is forthcoming.) It is possible to boot kernels compiled for microMIPS, but we have been unsuccessful in consistently being able to do so, and have not yet tracked down the root issue(s). Nathan Froyd (10): target-mips: break out [ls][wd]c1 and rdhwr insn generation target-mips: add microMIPS-specific bits to mips-defs.h target-mips: add enum constants for various invocations of FOP target-mips: refactor {c,abs}.cond.fmt insns target-mips: small changes to use new FMT_ enums target-mips: add microMIPS ASE support target-mips: add microMIPS CPUs target-mips: add microMIPS exception handler support linux-user: honor low bit of entry PC for MIPS hw: honor low bit in mipssim machine hw/mips_mipssim.c|4 +- linux-user/main.c|4 +- target-mips/cpu.h|3 + target-mips/helper.c | 21 +- target-mips/helper.h |9 + target-mips/mips-defs.h |1 + target-mips/op_helper.c | 136 ++ target-mips/translate.c | 3050 ++ target-mips/translate_init.c | 61 + 9 files changed, 3047 insertions(+), 242 deletions(-)
[Qemu-devel] [PATCH 10/10] hw: honor low bit in mipssim machine
Signed-off-by: Nathan Froyd --- hw/mips_mipssim.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/hw/mips_mipssim.c b/hw/mips_mipssim.c index a747de5..cd6c2be 100644 --- a/hw/mips_mipssim.c +++ b/hw/mips_mipssim.c @@ -106,7 +106,9 @@ static void main_cpu_reset(void *opaque) CPUState *env = s->env; cpu_reset(env); -env->active_tc.PC = s->vector; +env->active_tc.PC = s->vector & ~(target_ulong)1; +if (s->vector & 1) +env->hflags |= MIPS_HFLAG_M16; } static void -- 1.6.3.2
[Qemu-devel] [PATCH 02/10] target-mips: add microMIPS-specific bits to mips-defs.h
There's a new ASE_MICROMIPS instruction flag, and some extra CP0_Config3 fields. The ISA and ISA_ON_EXC fields are specific to microMIPS. The DSP2P is for version 2 of the DSP ASE. Signed-off-by: Nathan Froyd --- target-mips/cpu.h |3 +++ target-mips/mips-defs.h |1 + 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/target-mips/cpu.h b/target-mips/cpu.h index 7285636..986d938 100644 --- a/target-mips/cpu.h +++ b/target-mips/cpu.h @@ -363,6 +363,9 @@ struct CPUMIPSState { #define CP0C2_SA 0 int32_t CP0_Config3; #define CP0C3_M31 +#define CP0C3_ISA_ON_EXC 16 +#define CP0C3_ISA 14 +#define CP0C3_DSP2P 11 #define CP0C3_DSPP 10 #define CP0C3_LPA 7 #define CP0C3_VEIC 6 diff --git a/target-mips/mips-defs.h b/target-mips/mips-defs.h index c57de02..a7f4697 100644 --- a/target-mips/mips-defs.h +++ b/target-mips/mips-defs.h @@ -38,6 +38,7 @@ #defineASE_DSPR2 0x0001 #defineASE_MT 0x0002 #defineASE_SMARTMIPS 0x0004 +#defineASE_MICROMIPS 0x0008 /* Chip specific instructions. */ #defineINSN_VR54XX 0x8000 -- 1.6.3.2
[Qemu-devel] [PATCH 1/2] qcow2: Clear L2 table cache after write error
If the L2 table was already updated in cache, but writing it to disk has failed, we must not continue using the changed version in the cache to stay consistent with what's on the disk. Signed-off-by: Kevin Wolf --- block/qcow2-cluster.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index c11680d..ed5c4b2 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -696,6 +696,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m) ret = write_l2_entries(bs, l2_table, l2_offset, l2_index, m->nb_clusters); if (ret < 0) { +qcow2_l2_cache_reset(bs); goto err; } -- 1.6.6.1
[Qemu-devel] [PATCH 2/2] qcow2: Fix error handling in l2_allocate
l2_allocate has some intermediate states in which the image is inconsistent. Change the order to write to the L1 table only after the new L2 table has successfully been initialized. Also reset the L2 cache in failure case, it's very likely wrong. Signed-off-by: Kevin Wolf --- block/qcow2-cluster.c | 23 +-- 1 files changed, 13 insertions(+), 10 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index ed5c4b2..244b4a7 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -239,14 +239,6 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, uint64_t **table) return l2_offset; } -/* update the L1 entry */ - -s->l1_table[l1_index] = l2_offset | QCOW_OFLAG_COPIED; -ret = write_l1_entry(bs, l1_index); -if (ret < 0) { -return ret; -} - /* allocate a new entry in the l2 cache */ min_index = l2_cache_new_entry(bs); @@ -261,7 +253,7 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, uint64_t **table) ret = bdrv_pread(bs->file, old_l2_offset, l2_table, s->l2_size * sizeof(uint64_t)); if (ret < 0) { -return ret; +goto fail; } } /* write the l2 table to the file */ @@ -269,7 +261,14 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, uint64_t **table) ret = bdrv_pwrite(bs->file, l2_offset, l2_table, s->l2_size * sizeof(uint64_t)); if (ret < 0) { -return ret; +goto fail; +} + +/* update the L1 entry */ +s->l1_table[l1_index] = l2_offset | QCOW_OFLAG_COPIED; +ret = write_l1_entry(bs, l1_index); +if (ret < 0) { +goto fail; } /* update the l2 cache entry */ @@ -279,6 +278,10 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, uint64_t **table) *table = l2_table; return 0; + +fail: +qcow2_l2_cache_reset(bs); +return ret; } static int count_contiguous_clusters(uint64_t nb_clusters, int cluster_size, -- 1.6.6.1
Re: [Qemu-devel] [PATCH 03/22] tcg-i386: Tidy ext8u and ext16u operations.
On 05/20/2010 07:04 AM, Aurelien Jarno wrote: >> Do you have tried to compare the generated code before and after your >> patch? I expect a few cases where your patch has some drawbacks, so I >> don't know if there is a net gain on the size of the translated code. >> > > I have done a quick test on /bin/ls. >| instr | size | >+++ > before | 101305 | 344770 | > after | 101258 | 344829 | > > In short a small gain in the number of instructions, and a small loss in > the size of the translated code. That was pretty much the test I would have done. So where are we? Is the patch acceptable as-is, or should I be re-writing it without the constraints change? r~
Re: [Qemu-devel] [PATCH RFC] virtio: put last seen used index into ring itself
On Thu, 20 May 2010 04:30:56 pm Avi Kivity wrote: > On 05/20/2010 08:01 AM, Rusty Russell wrote: > > > >> A device with out of order > >> completion (like virtio-blk) will quickly randomize the unused > >> descriptor indexes, so every descriptor fetch will require a bounce. > >> > >> In contrast, if the rings hold the descriptors themselves instead of > >> pointers, we bounce (sizeof(descriptor)/cache_line_size) cache lines for > >> every descriptor, amortized. > >> > > We already have indirect, this would be a logical next step. So let's > > think about it. The avail ring would contain 64 bit values, the used ring > > would contain indexes into the avail ring. > > Have just one ring, no indexes. The producer places descriptors into > the ring and updates the head, The consumer copies out descriptors to > be processed and copies back in completed descriptors. Chaining is > always linear. The descriptors contain a tag that allow the producer to > identify the completion. This could definitely work. The original reason for the page boundaries was for untrusted inter-guest communication: with appropriate page protections they could see each other's rings and a simply inter-guest copy hypercall could verify that the other guest really exposed that data via virtio ring. But, cute as that is, we never did that. And it's not clear that it wins much over simply having the hypervisor read both rings directly. > > Can we do better? The obvious idea is to try to get rid of last_used and > > used, and use the ring itself. We would use an invalid entry to mark the > > head of the ring. > > Interesting! So a peer will read until it hits a wall. But how to > update the wall atomically? > > Maybe we can have a flag in the descriptor indicate headness or > tailness. Update looks ugly though: write descriptor with head flag, > write next descriptor with head flag, remove flag from previous descriptor. I was thinking a separate magic "invalid" entry. To publish an 3 descriptor chain, you would write descriptors 2 and 3, write an invalid entry at 4, barrier, write entry 1. It is a bit ugly, yes, but not terrible. I think that a simple simulator for this is worth writing, which tracks cacheline moves under various fullness scenarios... Cheers, Rusty.
Re: [Qemu-devel] [PATCH] QEMU: change default disk cache behavior
On Thu, May 20, 2010 at 2:49 PM, Jes Sorensen wrote: > On 05/20/10 15:40, Anthony Liguori wrote: >> On 05/20/2010 08:36 AM, Jes Sorensen wrote: And I strongly suspect that such a blanket change would be wrong but that a more targeted change like making cache=none default for physical devices would satisfy mostly everyone. >>> Is there any other thing than physical devices attached to the -drive >>> parameter? >> >> Image files which are the overwhelming more common use-case. > > For image files we certainly want it too, at least for proper ones (ie. > raw). It could be that it causes problems for qcow2. Qcow2 is safest with cache=writethrough because it doesn't order image updates: https://bugzilla.redhat.com/show_bug.cgi?id=572825 http://wiki.qemu.org/Features/Qcow2DataIntegrity Stefan
Re: [Qemu-devel] [PATCH 03/22] tcg-i386: Tidy ext8u and ext16u operations.
On Thu, May 20, 2010 at 03:39:08PM +0200, Aurelien Jarno wrote: > On Wed, May 19, 2010 at 11:31:27AM -0700, Richard Henderson wrote: > > On 05/18/2010 11:47 PM, Aurelien Jarno wrote: > > > The reg allocator is able to issue move if needed, so the only > > > improvement this patch is for doing a ext8u on both "q" registers. > > > > > > OTOH the reg allocator knows this situation and will try to avoid this > > > situation during the allocation. Cheating on the reg allocator might > > > have some wrong effects, especially after your patch "Allocate > > > call-saved registers first". I am thinking of the scenario where the > > > value is in memory (which is likely to be the case given the limited > > > number of registers), it will be likely loaded in a "r" register (they > > > are now at the top priority), and then ext8u will be called, which will > > > issue "mov" + "and" instructions instead of a "movzbl" instruction. > > > > The case I was concerned with is the fact that if we have a value > > allocated to, say, %esi, and we need to to an ext8u, then the > > register allocator has been told that it must move the value to a > > "q" register in order to perform the movzbl. In this case, the > > new code will simply emit the andl. > > > > I.e. the real problem is that we've told the register allocator > > one way that the extend can be implemented, but not every way. > > > > > All of that is purely theoretical. Do you know how does it behave in > > > practice? > > > > Picking the i386 target since it seems to use more extensions than > > any other target, from linux-user-test -d op_opt,out_asm i386/ls: > > > > There are 176 instances of ext8u. > > Of those, 83 instances are in-place, i.e. "ext8u_i32 tmp0,tmp0" > > > > I examined the first 2 dozen appearances in the output assembly: > > > > There are several instances of the value being in an "r" register: > > > > shr_i32 tmp1,edx,tmp13 > > ext8u_i32 tmp1,tmp1 > > => > > 0x601c5468: shr$0x8,%edi > > 0x601c546b: and$0xff,%edi > > > > All of the instances that I looked at that were not in-place happened > > to already be using a "q" register -- usually %ebx. I assume that's > > because we place %ebx as the first allocation register and that's just > > how things happen to work out once we've flushed the registers before > > the qemu_ld. > > > > qemu_ld8u tmp0,tmp2,$0x > > ext8u_i32 tmp13,tmp0 > > => > > 0x601c82f9: movzbl (%esi),%ebx > > 0x601c82fc: movzbl %bl,%ebx > > > > Do you have tried to compare the generated code before and after your > patch? I expect a few cases where your patch has some drawbacks, so I > don't know if there is a net gain on the size of the translated code. > I have done a quick test on /bin/ls. | instr | size | +++ before | 101305 | 344770 | after | 101258 | 344829 | In short a small gain in the number of instructions, and a small loss in the size of the translated code. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] [RFC PATCH] AMD IOMMU emulation
This is preliminary work for AMD IOMMU emulation support. Signed-off-by: Eduard - Gabriel Munteanu --- Makefile.target |2 + configure |9 + hw/amd_iommu.c | 442 +++ hw/pc.c |2 + hw/pc.h |3 + hw/pci_ids.h|2 + hw/pci_regs.h |1 + 7 files changed, 461 insertions(+), 0 deletions(-) create mode 100644 hw/amd_iommu.c diff --git a/Makefile.target b/Makefile.target index 0bdb184..13f8086 100644 --- a/Makefile.target +++ b/Makefile.target @@ -217,6 +217,8 @@ obj-i386-y += testdev.o obj-i386-$(CONFIG_KVM_PIT) += i8254-kvm.o obj-i386-$(CONFIG_KVM_DEVICE_ASSIGNMENT) += device-assignment.o +obj-i386-$(CONFIG_AMD_IOMMU) += amd_iommu.o + # Hardware support obj-ia64-y += ide.o pckbd.o vga.o $(SOUND_HW) dma.o $(AUDIODRV) obj-ia64-y += fdc.o mc146818rtc.o serial.o i8259.o ipf.o diff --git a/configure b/configure index ed8e17b..34e5194 100755 --- a/configure +++ b/configure @@ -305,6 +305,7 @@ mixemu="no" kvm_trace="no" kvm_cap_pit="" kvm_cap_device_assignment="" +amd_iommu="no" kerneldir="" aix="no" blobs="yes" @@ -603,6 +604,8 @@ for opt do ;; --enable-kvm-device-assignment) kvm_cap_device_assignment="yes" ;; + --enable-amd-iommu-emul) amd_iommu="yes" + ;; --enable-profiler) profiler="yes" ;; --enable-cocoa) @@ -829,6 +832,8 @@ echo " --disable-kvm-pitdisable KVM pit support" echo " --enable-kvm-pit enable KVM pit support" echo " --disable-kvm-device-assignment disable KVM device assignment support" echo " --enable-kvm-device-assignment enable KVM device assignment support" +echo " --disable-amd-iommu-emul disable AMD IOMMU emulation" +echo " --enable-amd-iommu-emul enable AMD IOMMU emulation" echo " --disable-nptl disable usermode NPTL support" echo " --enable-nptlenable usermode NPTL support" echo " --enable-system enable all system emulation targets" @@ -2185,6 +2190,7 @@ echo "KVM support $kvm" echo "KVM PIT support $kvm_cap_pit" echo "KVM device assig. $kvm_cap_device_assignment" echo "KVM trace support $kvm_trace" +echo "AMD IOMMU emul. $amd_iommu" echo "fdt support $fdt" echo "preadv support$preadv" echo "fdatasync $fdatasync" @@ -2599,6 +2605,9 @@ case "$target_arch2" in x86_64) TARGET_BASE_ARCH=i386 target_phys_bits=64 +if test "$amd_iommu" = "yes"; then + echo "CONFIG_AMD_IOMMU=y" >> $config_target_mak +fi ;; ia64) target_phys_bits=64 diff --git a/hw/amd_iommu.c b/hw/amd_iommu.c new file mode 100644 index 000..cde90d0 --- /dev/null +++ b/hw/amd_iommu.c @@ -0,0 +1,442 @@ +/* + * AMD IOMMU emulation + * + * Copyright (c) 2010 Eduard - Gabriel Munteanu + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "pc.h" +#include "hw.h" +#include "pci.h" + +/* Capability registers */ +#define CAPAB_HEADER0x00 +#define CAPAB_REV_TYPE0x02 +#define CAPAB_FLAGS 0x03 +#define CAPAB_BAR_LOW 0x04 +#define CAPAB_BAR_HIGH 0x08 +#define CAPAB_RANGE 0x0C +#define CAPAB_MISC 0x10 + +#define CAPAB_SIZE 0x14 + +/* Capability header data */ +#define CAPAB_FLAG_IOTLBSUP (1 << 0) +#define CAPAB_FLAG_HTTUNNEL (1 << 1) +#define CAPAB_FLAG_NPCACHE (1 << 2) +#define CAPAB_INIT_REV (1 << 3) +#define CAPAB_INIT_TYPE 3 +#define CAPAB_INIT_REV_TYPE (CAPAB_REV | CAPAB_TYPE) +#define CAPAB_INIT_FLAGS(CAPAB_FLAG_NPCACHE | CAPAB_FLAG_HTTUNNEL) +#define CAPAB_INIT_MISC (64 << 15) | (48 << 8) +#define CAPAB_BAR_MASK ~((1UL << 14) - 1) + +/* MMIO registers */ +#define MMIO_DEVICE_TABLE 0x +#define MMIO_COMMAND_BASE 0x0008 +#define MMIO_EVENT_BASE 0x0010 +#define MMIO_CONTROL0x0018 +#define MMIO_EXCL_BASE 0x0020 +#define MMIO_EXCL_LIMIT
Re: [Qemu-devel] [PATCH] QEMU: change default disk cache behavior
On 05/20/10 15:40, Anthony Liguori wrote: > On 05/20/2010 08:36 AM, Jes Sorensen wrote: >>> And I strongly suspect that such a blanket change would be wrong but >>> that a more targeted change like making cache=none default for physical >>> devices would satisfy mostly everyone. >>> >> Is there any other thing than physical devices attached to the -drive >> parameter? > > Image files which are the overwhelming more common use-case. For image files we certainly want it too, at least for proper ones (ie. raw). It could be that it causes problems for qcow2. I'll try and look at it when I am back. Cheers, Jes
Re: [Qemu-devel] [PATCH 2/6] json-lexer: Handle missing escapes
On Wed, 19 May 2010 16:44:47 -0500 Anthony Liguori wrote: > On 05/19/2010 04:15 PM, Luiz Capitulino wrote: > > The JSON escape sequence "\/" and "\\" are valid and should be > > handled. > > > > Signed-off-by: Luiz Capitulino > > > > Good catch. I think there's another issue in the handling of strings. The spec says that valid unescaped chars are in the following range: unescaped = %x20-21 / %x23-5B / %x5D-10 But we do: [IN_DQ_STRING] = { [1 ... 0xFF] = IN_DQ_STRING, ['\\'] = IN_DQ_STRING_ESCAPE, ['"'] = IN_DONE_STRING, }, Shouldn't we cover 0x20 .. 0xFF instead?
Re: [Qemu-devel] [PATCH] QEMU: change default disk cache behavior
On 05/20/2010 08:36 AM, Jes Sorensen wrote: On 05/20/10 14:30, Anthony Liguori wrote: On 05/20/2010 04:32 AM, jes.soren...@redhat.com wrote: Therefore, here is a patch that does two things: - default to "nocache" - in case of failure with nocache, retry with "write-back" This sort of change requires performance data in a variety of circumstances to justify. And I strongly suspect that such a blanket change would be wrong but that a more targeted change like making cache=none default for physical devices would satisfy mostly everyone. Is there any other thing than physical devices attached to the -drive parameter? Image files which are the overwhelming more common use-case. Regards, Anthony Liguori If so, I can take a look at making it more generic when I am back from holiday next week. Jes
Re: [Qemu-devel] [PATCH] QEMU: change default disk cache behavior
On 05/20/10 14:30, Anthony Liguori wrote: > On 05/20/2010 04:32 AM, jes.soren...@redhat.com wrote: >> Therefore, here is a patch that does two things: >> - default to "nocache" >> - in case of failure with nocache, retry with "write-back" >> > > This sort of change requires performance data in a variety of > circumstances to justify. > > And I strongly suspect that such a blanket change would be wrong but > that a more targeted change like making cache=none default for physical > devices would satisfy mostly everyone. Is there any other thing than physical devices attached to the -drive parameter? If so, I can take a look at making it more generic when I am back from holiday next week. Jes
Re: [Qemu-devel] [PATCH 03/22] tcg-i386: Tidy ext8u and ext16u operations.
On Wed, May 19, 2010 at 11:31:27AM -0700, Richard Henderson wrote: > On 05/18/2010 11:47 PM, Aurelien Jarno wrote: > > The reg allocator is able to issue move if needed, so the only > > improvement this patch is for doing a ext8u on both "q" registers. > > > > OTOH the reg allocator knows this situation and will try to avoid this > > situation during the allocation. Cheating on the reg allocator might > > have some wrong effects, especially after your patch "Allocate > > call-saved registers first". I am thinking of the scenario where the > > value is in memory (which is likely to be the case given the limited > > number of registers), it will be likely loaded in a "r" register (they > > are now at the top priority), and then ext8u will be called, which will > > issue "mov" + "and" instructions instead of a "movzbl" instruction. > > The case I was concerned with is the fact that if we have a value > allocated to, say, %esi, and we need to to an ext8u, then the > register allocator has been told that it must move the value to a > "q" register in order to perform the movzbl. In this case, the > new code will simply emit the andl. > > I.e. the real problem is that we've told the register allocator > one way that the extend can be implemented, but not every way. > > > All of that is purely theoretical. Do you know how does it behave in > > practice? > > Picking the i386 target since it seems to use more extensions than > any other target, from linux-user-test -d op_opt,out_asm i386/ls: > > There are 176 instances of ext8u. > Of those, 83 instances are in-place, i.e. "ext8u_i32 tmp0,tmp0" > > I examined the first 2 dozen appearances in the output assembly: > > There are several instances of the value being in an "r" register: > > shr_i32 tmp1,edx,tmp13 > ext8u_i32 tmp1,tmp1 > => > 0x601c5468: shr$0x8,%edi > 0x601c546b: and$0xff,%edi > > All of the instances that I looked at that were not in-place happened > to already be using a "q" register -- usually %ebx. I assume that's > because we place %ebx as the first allocation register and that's just > how things happen to work out once we've flushed the registers before > the qemu_ld. > > qemu_ld8u tmp0,tmp2,$0x > ext8u_i32 tmp13,tmp0 > => > 0x601c82f9: movzbl (%esi),%ebx > 0x601c82fc: movzbl %bl,%ebx > Do you have tried to compare the generated code before and after your patch? I expect a few cases where your patch has some drawbacks, so I don't know if there is a net gain on the size of the translated code. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 0/6]: QMP: Fix issues in parser/lexer
On Wed, 19 May 2010 16:43:08 -0500 Anthony Liguori wrote: > On 05/19/2010 04:15 PM, Luiz Capitulino wrote: > > Hi Anthony, > > > > While investigating a QMP bug reported by a user, I've found a few issues > > in our parser/lexer. > > > > The patches in this series fix the problems I was able to solve, but we > > still have the following issues: > > > > 1. Our 'private extension' is open to the public > > > > Eg. The following input issued by a client is valid: > > > > { 'execute': 'query-pci' } > > > > I don't think it's a good idea to have clients relying on this kind of > > JSON extension. > > > > To fix this we could add a 'extension' flag to JSONLexer and set it to > > nonzero in internal functions (eg. qobject_from_jsonf()), of course that > > the lexer code should handle this too. > > > > The JSON specification explicitly says: > > "A JSON parser transforms a JSON text into another representation. A > JSON parser MUST accept all texts that conform to the JSON grammar. A > JSON parser MAY accept non-JSON forms or extensions." > > IOW, we're under no obligation to reject extensions and I can't think of > a reason why we should. I know we're legal, but what's the point to offer this extension to clients? The main motivation behind this was to write JSON in C strings w/o the need of repetitive escapes. This is internal to QEMU, but it's also available to clients for no reason. And you know, after 0.13 we won't be able to remove it. > > 2. QMP doesn't check the return of json_message_parser_feed() > > > > Which means we don't handle JSON syntax errors. While the fix might seem > > trivial (ie. just return an error!), I'm not sure what's the best way > > to handle this, because the streamer seems to return multiple errors for > > the same input string. > > > > For example, this input: > > > > { "execute": yy_uu } > > > > Seems to return an error for each bad character (yy_uu), shouldn't it > > return only once and stop processing the whole string? > > > > It probably should kill the connection. Ok. > > 3. The lexer enter in ERROR state when processing is done > > > > Not sure whether this is an issue, but I found it while reviewing the > > code > > and maybe this is related with item 2 above. > > > > When json_lexer_feed_char() is finished scanning a string, (ie. ch='\0') > > the JSON_SKIP clause will set lexer->state to ERROR as there's no entry > > for '\0' in the IN_START array. > > > > Shouldn't we have a LEXER_DONE or something like it instead? > > > > No, you must have malformed input if an error occurs. Yes, json_message_parser_feed() returns OK. > [IN_WHITESPACE] -> TERMINAL(JSON_SKIP) > > JSON_SKIP is a terminal so once you're in that state, you go back to > IN_START. Yes, but what I'm trying to say is that when ch='\0' and you do: lexer->state = json_lexer[IN_START][(uint8_t)ch]; Then 'lexer->state' becomes 0, which is what the code recognizes as ERROR. Again, not sure if this is an issue. Just caught my attention. > > 4. Lexer expects a 'terminal' char to process a token > > > > Which means clients must send a sort of end of line char, so that we > > process their input. > > > > Maybe I'm missing something here, but I thought that the whole point of > > writing our own parser was to avoid this. > > > > If the lexer gets: > > "abc" > > It has no way of knowing if that's a token or if we're going to get: > > "abcd" > > As a token. You can fix this in two ways. You can either flush() the > lexer to significant end of input or you can wait until there's some > other valid symbol to cause the previous symbol to be emitted. > > IOW, a client either needs to: 1) send the request and follow it with a > newline or some form of whitespace or 2) close the connection to flush > the request Ok.
[Qemu-devel] [PATCH] check for active_console before using it
Other vga_hw_* functions do the same. Fixes a segmentation fault. Trigger: boot with -nodefaults, then connect via vnc. Signed-off-by: Gerd Hoffmann --- console.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/console.c b/console.c index 7070b1b..4c42b28 100644 --- a/console.c +++ b/console.c @@ -167,7 +167,7 @@ void vga_hw_update(void) void vga_hw_invalidate(void) { -if (active_console->hw_invalidate) +if (active_console && active_console->hw_invalidate) active_console->hw_invalidate(active_console->hw); } -- 1.6.6.1
Re: [Qemu-devel] [PATCH 0/2] Fix scsi-generic breakage in upstream qemu-kvm.git
Am 17.05.2010 18:45, schrieb Nicholas A. Bellinger: > From: Nicholas Bellinger > > Greetings, > > Attached are the updated patches following hch's comments to fix scsi-generic > device breakage with find_image_format() and refresh_total_sectors(). > > These are being resent as the last attachments where in MBOX format from > git-format-patch. > > Signed-off-by: Nicholas A. Bellinger Thanks, applied all to the block branch, even though I forgot to reply here. Kevin
Re: [Qemu-devel] Re: [PATCH 1/3] cursor: add cursor functions.
On 05/20/2010 07:49 AM, Gerd Hoffmann wrote: Hi, Well. You can't have both. We can have a efficiently packed format (i.e. two bitmaps). Or we can do it in a way which doesn't need parsing, but that wouldn't be the most compact format ... You're right, so packing or introducing a small conversion function is not critical. I'd still prefer a standard format if possible. Personally, I'd rather see Gerd's original format but read from a file instead of hard coded in a .c file. IOW, a /usr/share/qemu/default-cursor.qpm that contained the appropriate strings. A couple extra lines that made it an xpm I think would be worth it too. xpms are designed to be easily #include-able, and parsing them that way is easier than loading them at runtime. At least without adding a dependency to libXpm. So how about the following incremental RfC patch? It adds the cursors as separate files which are standard xpm format. Nevertheless they are compiled in, i.e. they can't be changed at runtime. That works for me. Nice job. Regards, Anthony Liguori cheers, Gerd
Re: [Qemu-devel] [RFC] Bug Day - June 1st, 2010
Michael Tokarev wrote: 20.05.2010 02:30, Anthony Liguori wrote: On 05/19/2010 05:29 PM, Andre Przywara wrote: Michael Tokarev wrote: ... Also, thanks to Andre Przywara, whole winNT thing works but it requires -cpu qemu64,level=1 (or level=2 or =3), -- _not_ with default CPU. This [] It'd be nice if we had more flexibility in defining custom machine types so you could just do qemu -M win98. This is wrong IMHO. win98 and winNT can run on various different machines, including all modern ones (yes I tried the same winNT on my Athlon X2-64, just had to switch SATA from AHCI to IDE; win95 works too)... just not in kvm :) Well, not really. You were lucky with your Athlon X2-64, actually it is the last machine not triggering the bug. I tried it on a AthlonII-X4 (which has maxleaf=5 as any newer AMD machines) and it showed the same bug. On Intel boxes this bug should trigger on every CPU starting with some Pentium4 models, including all Core chips. Have you tried versions with a newer service pack (SP6)? BTW: Does anyone knows what the problem with Windows95/98 on KVM is? I tried some tracing today, but couldn't find a hint. Um. The bugreport(s) come as a surprize for me: I tried to install win98 in kvm several times in the past but setup always failed - different messages in different versions of kvm, either "unable to emulate" or "real mode trap" or something else, or just lockup, usually on first reboot. So - the bugreports talks about mouse non-working, but this means win98 itself works somehow... I dunno :) I think these bug reports are about plain QEMU. I tried it yesterday, in fact the mouse is non-functional. In KVM Windows95 gives me a black screen after the welcome screen with the moving bottom row. There are just two lines at the top: (translated from the german version) While initializing device NTKERN: Windows protection fault. Restart the computer. KVM catched some #UDs due to ARPL from VM86 mode, but TCG got them too and it survived. So if anyone has some more hints, I'd be grateful. Regards, Andre. -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany Tel: +49 351 448-3567-12
[Qemu-devel] [PATCH 7/8] vvfat: Fix compilation with DEBUG defined
From: Riccardo Magliocchetti gcc does not like passing a NULL where an int value is expected: block/vvfat.c: In function ‘checkpoint’: block/vvfat.c:2868: error: passing argument 2 of ‘remove_mapping’ makes integer from pointer without a cast Signed-off-by: Riccardo Magliocchetti Signed-off-by: Kevin Wolf --- block/vvfat.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index ce16bbd..13c31fa 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2865,7 +2865,7 @@ static void checkpoint(void) { return; /* avoid compiler warnings: */ hexdump(NULL, 100); -remove_mapping(vvv, NULL); +remove_mapping(vvv, 0); print_mapping(NULL); print_direntry(NULL); } -- 1.6.6.1
[Qemu-devel] [PATCH 8/8] vvfat: More build fixes with DEBUG
Casting a pointer to an int doesn't work on 64 bit platforms. Use the %p printf conversion specifier instead. Signed-off-by: Kevin Wolf --- block/vvfat.c |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index 13c31fa..6d61c2e 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1244,7 +1244,7 @@ static void print_direntry(const direntry_t* direntry) int j = 0; char buffer[1024]; -fprintf(stderr, "direntry 0x%x: ", (int)direntry); +fprintf(stderr, "direntry %p: ", direntry); if(!direntry) return; if(is_long_name(direntry)) { @@ -1273,7 +1273,11 @@ static void print_direntry(const direntry_t* direntry) static void print_mapping(const mapping_t* mapping) { -fprintf(stderr, "mapping (0x%x): begin, end = %d, %d, dir_index = %d, first_mapping_index = %d, name = %s, mode = 0x%x, " , (int)mapping, mapping->begin, mapping->end, mapping->dir_index, mapping->first_mapping_index, mapping->path, mapping->mode); +fprintf(stderr, "mapping (%p): begin, end = %d, %d, dir_index = %d, " +"first_mapping_index = %d, name = %s, mode = 0x%x, " , +mapping, mapping->begin, mapping->end, mapping->dir_index, +mapping->first_mapping_index, mapping->path, mapping->mode); + if (mapping->mode & MODE_DIRECTORY) fprintf(stderr, "parent_mapping_index = %d, first_dir_index = %d\n", mapping->info.dir.parent_mapping_index, mapping->info.dir.first_dir_index); else -- 1.6.6.1
[Qemu-devel] [PATCH 6/8] block: Add SG_IO device check in refresh_total_sectors()
From: Nicholas Bellinger This patch adds a special case check for scsi-generic devices in refresh_total_sectors() to skip the subsequent BlockDriver->bdrv_getlength() that will be returning -ESPIPE from block/raw-posic.c:raw_getlength() for BlockDriverState->sg=1 devices. Signed-off-by: Nicholas A. Bellinger Signed-off-by: Kevin Wolf --- block.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/block.c b/block.c index 6a95768..0b0966c 100644 --- a/block.c +++ b/block.c @@ -361,6 +361,10 @@ static int refresh_total_sectors(BlockDriverState *bs, int64_t hint) { BlockDriver *drv = bs->drv; +/* Do not attempt drv->bdrv_getlength() on scsi-generic devices */ +if (bs->sg) +return 0; + /* query actual device if possible, otherwise just trust the hint */ if (drv->bdrv_getlength) { int64_t length = drv->bdrv_getlength(bs); -- 1.6.6.1
[Qemu-devel] [PATCH 1/8] block: fix aio_flush segfaults for read-only protocols (e.g. curl)
From: Avi Kivity Not all block format drivers expose an io_flush method (reasonable for read-only protocols), so calling io_flush there will immediately segfault. Fix by checking for the method's existence before calling it. Signed-off-by: Avi Kivity Signed-off-by: Kevin Wolf --- aio.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/aio.c b/aio.c index f164a47..2f08655 100644 --- a/aio.c +++ b/aio.c @@ -113,7 +113,9 @@ void qemu_aio_flush(void) qemu_aio_wait(); QLIST_FOREACH(node, &aio_handlers, node) { -ret |= node->io_flush(node->opaque); +if (node->io_flush) { +ret |= node->io_flush(node->opaque); +} } } while (qemu_bh_poll() || ret > 0); } -- 1.6.6.1
[Qemu-devel] [PATCH 2/8] virtio-blk: Avoid zeroing every request structure
From: Stefan Hajnoczi The VirtIOBlockRequest structure is about 40 KB in size. This patch avoids zeroing every request by only initializing fields that are read. The other fields are either written to or may not be used at all. Oprofile shows about 10% of CPU samples in memset called by virtio_blk_alloc_request(). The workload is dd if=/dev/vda of=/dev/null iflag=direct bs=8k running concurrently 4 times. This patch makes memset disappear to the bottom of the profile. Signed-off-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- hw/virtio-blk.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index b05d15e..d270225 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -105,8 +105,10 @@ static void virtio_blk_flush_complete(void *opaque, int ret) static VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s) { -VirtIOBlockReq *req = qemu_mallocz(sizeof(*req)); +VirtIOBlockReq *req = qemu_malloc(sizeof(*req)); req->dev = s; +req->qiov.size = 0; +req->next = NULL; return req; } -- 1.6.6.1
[Qemu-devel] [PATCH 5/8] block: Make find_image_format() return 'raw' BlockDriver for SG_IO devices
From: Nicholas Bellinger This patch adds a special BlockDriverState->sg check in block.c:find_image_format() after bdrv_file_open() -> block/raw-posix.c:hdev_open() has been called to determine if we are dealing with a Linux host scsi-generic device. The patch then returns the BlockDriver * from bdrv_find_format("raw"), skipping the subsequent bdrv_read() and rest of find_image_format(). Signed-off-by: Nicholas A. Bellinger Signed-off-by: Kevin Wolf --- block.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/block.c b/block.c index 89eece7..6a95768 100644 --- a/block.c +++ b/block.c @@ -329,6 +329,11 @@ static BlockDriver *find_image_format(const char *filename) ret = bdrv_file_open(&bs, filename, 0); if (ret < 0) return NULL; + +/* Return the raw BlockDriver * to scsi-generic devices */ +if (bs->sg) +return bdrv_find_format("raw"); + ret = bdrv_pread(bs, 0, buf, sizeof(buf)); bdrv_delete(bs); if (ret < 0) { -- 1.6.6.1
[Qemu-devel] [PATCH 4/8] block: fix sector comparism in multiwrite_req_compare
From: Christoph Hellwig The difference between the start sectors of two requests can be larger than the size of the "int" type, which can lead to a not correctly sorted multiwrite array and thus spurious I/O errors and filesystem corruption due to incorrect request merges. So instead of doing the cute sector arithmetics trick spell out the exact comparisms. Spotted by Kevin Wolf based on a testcase from Michael Tokarev. Signed-off-by: Christoph Hellwig Signed-off-by: Kevin Wolf --- block.c | 14 +- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/block.c b/block.c index bfe46e3..89eece7 100644 --- a/block.c +++ b/block.c @@ -1929,7 +1929,19 @@ static void multiwrite_cb(void *opaque, int ret) static int multiwrite_req_compare(const void *a, const void *b) { -return (((BlockRequest*) a)->sector - ((BlockRequest*) b)->sector); +const BlockRequest *req1 = a, *req2 = b; + +/* + * Note that we can't simply subtract req2->sector from req1->sector + * here as that could overflow the return value. + */ +if (req1->sector > req2->sector) { +return 1; +} else if (req1->sector < req2->sector) { +return -1; +} else { +return 0; +} } /* -- 1.6.6.1
[Qemu-devel] [PATCH 3/8] virtio-blk: fix barrier support
From: Christoph Hellwig Before issuing the barrier to the block driver we need to flush our oustanding queue of write requests, as the flush is supposed to be issued after them. Signed-off-by: Christoph Hellwig Signed-off-by: Kevin Wolf --- hw/virtio-blk.c | 15 +-- 1 files changed, 13 insertions(+), 2 deletions(-) diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index d270225..5d7f1a2 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -240,10 +240,20 @@ static void do_multiwrite(BlockDriverState *bs, BlockRequest *blkreq, } } -static void virtio_blk_handle_flush(VirtIOBlockReq *req) +static void virtio_blk_handle_flush(BlockRequest *blkreq, int *num_writes, +VirtIOBlockReq *req, BlockDriverState **old_bs) { BlockDriverAIOCB *acb; +/* + * Make sure all outstanding writes are posted to the backing device. + */ +if (*old_bs != NULL) { +do_multiwrite(*old_bs, blkreq, *num_writes); +} +*num_writes = 0; +*old_bs = req->dev->bs; + acb = bdrv_aio_flush(req->dev->bs, virtio_blk_flush_complete, req); if (!acb) { virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); @@ -316,7 +326,8 @@ static void virtio_blk_handle_request(VirtIOBlockReq *req, req->in = (void *)req->elem.in_sg[req->elem.in_num - 1].iov_base; if (req->out->type & VIRTIO_BLK_T_FLUSH) { -virtio_blk_handle_flush(req); +virtio_blk_handle_flush(mrb->blkreq, &mrb->num_writes, +req, &mrb->old_bs); } else if (req->out->type & VIRTIO_BLK_T_SCSI_CMD) { virtio_blk_handle_scsi(req); } else if (req->out->type & VIRTIO_BLK_T_OUT) { -- 1.6.6.1
[Qemu-devel] [PULL 0/8] Block patches
Hi Anthony, this bunch of patches contains some fixes that I think qualify for stable, especially the first two. Please cherry-pick them into stable-0.12 when you have pulled: 468f932 block: fix sector comparism in multiwrite_req_compare 49eb7b7 virtio-blk: fix barrier support 5fdf2a6 block: fix aio_flush segfaults for read-only protocols (e.g. curl) Kevin The following changes since commit 5a4bb580cdb10b066f9fd67658b31cac4a4ea5e5: Richard Henderson (1): target-sparc: Simplify ICC generation. are available in the git repository at: git://repo.or.cz/qemu/kevin.git for-anthony Avi Kivity (1): block: fix aio_flush segfaults for read-only protocols (e.g. curl) Christoph Hellwig (2): virtio-blk: fix barrier support block: fix sector comparism in multiwrite_req_compare Kevin Wolf (1): vvfat: More build fixes with DEBUG Nicholas Bellinger (2): block: Make find_image_format() return 'raw' BlockDriver for SG_IO devices block: Add SG_IO device check in refresh_total_sectors() Riccardo Magliocchetti (1): vvfat: Fix compilation with DEBUG defined Stefan Hajnoczi (1): virtio-blk: Avoid zeroing every request structure aio.c |4 +++- block.c | 23 ++- block/vvfat.c | 10 +++--- hw/virtio-blk.c | 19 --- 4 files changed, 48 insertions(+), 8 deletions(-)
[Qemu-devel] Re: vgabios plans ( Re: [PATCH 1/5] Makefile cleanup)
On 05/20/2010 07:57 AM, Gerd Hoffmann wrote: On 05/07/10 12:09, Gerd Hoffmann wrote: Use a single rule for building bios binaries. Use target specific variables to set compile flags. This makes it more obvious what the differences between the versions are. It also makes it easier to add new bios binaries with slightly different settings. Hmm. No response for weeks from vgabios folks on this patch series. How to go forward best with vgabios bits? Just upgrade http://git.qemu.org/vgabios.git/ to 0.6c, then apply patches there? Yeah, I think the long term goal should be to move to SeaBIOS's vgabios fork but for now, I guess we'll have to do it. Regards, Anthony Liguori cheers, Gerd
[Qemu-devel] vgabios plans ( Re: [PATCH 1/5] Makefile cleanup)
On 05/07/10 12:09, Gerd Hoffmann wrote: Use a single rule for building bios binaries. Use target specific variables to set compile flags. This makes it more obvious what the differences between the versions are. It also makes it easier to add new bios binaries with slightly different settings. Hmm. No response for weeks from vgabios folks on this patch series. How to go forward best with vgabios bits? Just upgrade http://git.qemu.org/vgabios.git/ to 0.6c, then apply patches there? cheers, Gerd
Re: [Qemu-devel] Re: [PATCH 1/3] cursor: add cursor functions.
Hi, Well. You can't have both. We can have a efficiently packed format (i.e. two bitmaps). Or we can do it in a way which doesn't need parsing, but that wouldn't be the most compact format ... You're right, so packing or introducing a small conversion function is not critical. I'd still prefer a standard format if possible. Personally, I'd rather see Gerd's original format but read from a file instead of hard coded in a .c file. IOW, a /usr/share/qemu/default-cursor.qpm that contained the appropriate strings. A couple extra lines that made it an xpm I think would be worth it too. xpms are designed to be easily #include-able, and parsing them that way is easier than loading them at runtime. At least without adding a dependency to libXpm. So how about the following incremental RfC patch? It adds the cursors as separate files which are standard xpm format. Nevertheless they are compiled in, i.e. they can't be changed at runtime. cheers, Gerd From 7066e5a759ec887191601e6a96deab59a1dda721 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 20 May 2010 14:39:26 +0200 Subject: [PATCH] cursor: switch to xpm --- cursor.c| 94 ++- cursor_hidden.xpm | 37 cursor_left_ptr.xpm | 39 + 3 files changed, 124 insertions(+), 46 deletions(-) create mode 100644 cursor_hidden.xpm create mode 100644 cursor_left_ptr.xpm diff --git a/cursor.c b/cursor.c index 3995a31..dfb9eef 100644 --- a/cursor.c +++ b/cursor.c @@ -1,51 +1,57 @@ #include "qemu-common.h" #include "console.h" -static const char cursor_hidden_32[32*32]; -static const char cursor_left_ptr_32[32*32] = { -"" -" X " -" XX " -" X.X" -" X..X " -" X...X " -" XX " -" X.X" -" X..X " -" X...X " -" XX " -" X.X" -" X..X..X" -" X.X X..X " -" XX X..X " -" XX..X " -" X..X " -" X..X " -" X..X " -"XX " -"" -}; +#include "cursor_hidden.xpm" +#include "cursor_left_ptr.xpm" /* for creating built-in cursors */ -static void cursor_parse_ascii_art(QEMUCursor *c, const char *ptr) +static QEMUCursor *cursor_parse_xpm(const char *xpm[]) { -int i, pixels; - -pixels = c->width * c->height; -for (i = 0; i < pixels; i++) { -switch (ptr[i]) { -case 'X': /* black */ -c->data[i] = 0xff00; -break; -case '.': /* white */ -c->data[i] = 0x; -break; -case ' ': /* transparent */ -default: -c->data[i] = 0x; -break; +QEMUCursor *c; +uint32_t ctab[128]; +unsigned int width, height, colors, chars; +unsigned int line = 0, i, r, g, b, x, y, pixel; +char name[16]; +uint8_t idx; + +/* parse header line: width, height, #colors, #chars */ +if (sscanf(xpm[line], "%d %d %d %d", &width, &height, &colors, &chars) != 4) { +fprintf(stderr, "%s: header parse error: \"%s\"\n", +__FUNCTION__, xpm[line]); +return NULL; +} +if (chars != 1) { +fprintf(stderr, "%s: chars != 1 not supported\n", __FUNCTION__); +return NULL; +} +line++; + +/* parse color table */ +for (i = 0; i < colors; i++, line++) { +if (sscanf(xpm[line], "%c c %15s", &idx, name) == 2) { +if (sscanf(name, "#%02x%02x%02x", &r, &g, &b) == 3) { +ctab[idx] = (0xff << 24) | (b << 16) | (g << 8) | r; +continue; +} +if (strcmp(name, "None") == 0) { +ctab[idx] = 0x; +continue; +} } +fprintf(stderr, "%s: color parse error: \"%s\"\n", +__FUNCTION__, xpm[line]); +return NULL; } + +/* parse pixel data */ +c = cursor_alloc(width, height); +for (pixel = 0, y = 0; y < height; y++, line++) { +for (x = 0; x < height; x++, pixel++) { +idx = xpm[line][x]; +c->data[pixel] = ctab[idx]; +} +} +return c; } /* nice for debugging */ @@ -75,8 +81,7 @@ QEMUCursor *cursor_builtin_hidden(void) { QEMUCursor *c; -c = cursor_alloc(32, 32); -cursor_parse_ascii_art(c, cursor_hidden_32); +c = cursor_parse_xpm(cursor_hidden_xpm); return c; } @@ -84,10 +89,7 @@ QEMUCursor *cursor_builtin_left_ptr(void) {
[Qemu-devel] [PATCH 1/2] Fix TEXI section mark imbalance in qemu-img-cmd.hx
From: Jan Kiszka Signed-off-by: Jan Kiszka --- qemu-img-cmds.hx |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index c079019..c4cf3e7 100644 --- a/qemu-img-cmds.hx +++ b/qemu-img-cmds.hx @@ -7,7 +7,7 @@ HXCOMM HXCOMM can be used for comments, discarded from both texi and C STEXI @table @option -STEXI +ETEXI DEF("check", img_check, "check [-f fmt] filename") -- 1.6.0.2
Re: [Qemu-devel] [PATCH] QEMU: change default disk cache behavior
On 05/20/2010 04:32 AM, jes.soren...@redhat.com wrote: From: Jes Sorensen We seem to get into the discussion of what is the correct default setting disk images in QEMU. The libvirt team is reluctant to change specified for newly created images without the default setting matching it, and everybody seems to agree that the current setting of WT is the worse possible option. 'nocache' seems to be the preferred option, but it doesn't work for all cases, like images on ramfs, NFS etc. Therefore, here is a patch that does two things: - default to "nocache" - in case of failure with nocache, retry with "write-back" This sort of change requires performance data in a variety of circumstances to justify. And I strongly suspect that such a blanket change would be wrong but that a more targeted change like making cache=none default for physical devices would satisfy mostly everyone. Regards, Anthony Liguori Jes Sorensen (1): QEMU: Change default disk caching to nocache vl.c | 25 +++-- 1 files changed, 19 insertions(+), 6 deletions(-)
[Qemu-devel] [Bug 583296] Re: I/O errors with qemu-nbd/qcow2
I forgot: this is on Ubuntu 10.04, Qemu 0.12.3. -- I/O errors with qemu-nbd/qcow2 https://bugs.launchpad.net/bugs/583296 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: I tried to open a qcow2 file with qemu-nbd and backup the files in it. After some coping I get lot of I/O errors in dmesg and the system hangs. One time I got even a kernel panic (Of course on a productive Server ;-) ) How to reproduce: 1. Connect nbd to a qcow2 file, a virtual machine mustn't use this file: qemu-nbd --connect=/dev/nbd0 /mnt/qcow/andromeda.qcow2 2. Read some data from /dev/nbd0p1 dd if=/dev/nbd0p1 of=/dev/null bs=1M After a few Seconds till Minutes somethings crash's Attached some dmesg logs
[Qemu-devel] [Bug 583296] [NEW] I/O errors with qemu-nbd/qcow2
Public bug reported: I tried to open a qcow2 file with qemu-nbd and backup the files in it. After some coping I get lot of I/O errors in dmesg and the system hangs. One time I got even a kernel panic (Of course on a productive Server ;-) ) How to reproduce: 1. Connect nbd to a qcow2 file, a virtual machine mustn't use this file: qemu-nbd --connect=/dev/nbd0 /mnt/qcow/andromeda.qcow2 2. Read some data from /dev/nbd0p1 dd if=/dev/nbd0p1 of=/dev/null bs=1M After a few Seconds till Minutes somethings crash's Attached some dmesg logs ** Affects: qemu Importance: Undecided Status: New -- I/O errors with qemu-nbd/qcow2 https://bugs.launchpad.net/bugs/583296 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: I tried to open a qcow2 file with qemu-nbd and backup the files in it. After some coping I get lot of I/O errors in dmesg and the system hangs. One time I got even a kernel panic (Of course on a productive Server ;-) ) How to reproduce: 1. Connect nbd to a qcow2 file, a virtual machine mustn't use this file: qemu-nbd --connect=/dev/nbd0 /mnt/qcow/andromeda.qcow2 2. Read some data from /dev/nbd0p1 dd if=/dev/nbd0p1 of=/dev/null bs=1M After a few Seconds till Minutes somethings crash's Attached some dmesg logs
[Qemu-devel] [Bug 583296] Re: I/O errors with qemu-nbd/qcow2
** Attachment added: "dmesg" http://launchpadlibrarian.net/48810728/kern.log -- I/O errors with qemu-nbd/qcow2 https://bugs.launchpad.net/bugs/583296 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: I tried to open a qcow2 file with qemu-nbd and backup the files in it. After some coping I get lot of I/O errors in dmesg and the system hangs. One time I got even a kernel panic (Of course on a productive Server ;-) ) How to reproduce: 1. Connect nbd to a qcow2 file, a virtual machine mustn't use this file: qemu-nbd --connect=/dev/nbd0 /mnt/qcow/andromeda.qcow2 2. Read some data from /dev/nbd0p1 dd if=/dev/nbd0p1 of=/dev/null bs=1M After a few Seconds till Minutes somethings crash's Attached some dmesg logs