Re: [Qemu-devel] [PATCH 0/5] atapi: Implement 'media' subcommand for GESN
On (Fri) 08 Apr 2011 [11:39:26], Markus Armbruster wrote: Results of quick test run now, patch review to follow. Test uses a simple program to try ioctl CDROM_DRIVE_STATUS (attached). ... Test in guest without your patches: [start with empty drive] # ./drive-status CDS_NO_DISC # eject /dev/sr0 # ./drive-status CDS_NO_DISC [incorrect, should be CDS_TRAY_OPEN] # eject -t /dev/sr0 # ./drive-status CDS_NO_DISC [insert media with monitor command change] # ./drive-status CDS_DISC_OK # eject /dev/sr0 # ./drive-status CDS_NO_DISC [incorrect, should be CDS_TRAY_OPEN] # eject -t /dev/sr0 # ./drive-status CDS_DISC_OK With the patches, it behaves as expected. Except something (guest kernel?) closes the tray right after eject if there's a medium in the open tray. Can you try with the two patches I sent on Saturday: atapi: Drives can be locked without media present atapi: Report correct errors on guest eject request Thanks, Amit
Re: [Qemu-devel] [PATCH 2/2 V7] qemu,qmp: add inject-nmi qmp command
Avi Kivity a...@redhat.com writes: On 04/08/2011 12:41 AM, Anthony Liguori wrote: And it's a good thing to have, but exposing this as the only API to do something as simple as generating a guest crash dump is not the friendliest thing in the world to do to users. nmi is a fine name for something that corresponds to a real-life nmi button (often labeled NMI). Agree. generate-crash-dump is a wrong name for something that doesn't generate a crash dump (the guest may not be configured for it, or it may fail to work). Or the OS uses the NMI button for something else. I'd expect that to be host-side functionality.
[Qemu-devel] Re: [PATCH 4/4] qemu-timer: Fix timers for w32
On 04/10/2011 08:28 PM, Stefan Weil wrote: Commit 68c23e5520e8286d79d96ab47c0ea722ceb75041 removed the multimedia timer, but this timer is needed for certain Linux kernels. Otherwise Linux boot stops with this error: MP-BIOS bug: 8254 timer not connected to IO-APIC So the multimedia timer is added again here. Which distribution and Windows version is that? Also, have they tried the non-dynticks timer (win32)? Paolo
[Qemu-devel] Re: [PATCH] ppc: remove a write-only variable
On 11.04.2011, at 06:30, David Gibson wrote: On Sat, Apr 09, 2011 at 05:28:06PM +0200, Alexander Graf wrote: Am 09.04.2011 um 16:56 schrieb Blue Swirl blauwir...@gmail.com: Remove a write-only variable, spotted by GCC 4.6.0: /src/qemu/hw/ppc.c: In function 'power7_set_irq': /src/qemu/hw/ppc.c:255:9: error: variable 'cur_level' set but not used [-Werror=unused-but-set-variable] Signed-off-by: Blue Swirl blauwir...@gmail.com --- hw/ppc.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/hw/ppc.c b/hw/ppc.c index dabb816..1873328 100644 --- a/hw/ppc.c +++ b/hw/ppc.c @@ -252,11 +252,9 @@ void ppc970_irq_init (CPUState *env) static void power7_set_irq (void *opaque, int pin, int level) { CPUState *env = opaque; -int cur_level; LOG_IRQ(%s: env %p pin %d level %d\n, __func__, env, pin, level); -cur_level = (env-irq_input_state pin) 1; David, Ben? That should be fine. It's a hang over from the ppc970 code which I adapted. Alright. Acked-by: Alexander Graf ag...@suse.de Blue, mind to apply it directly? That's easier than going through my tree. Alex
Re: [Qemu-devel] [PATCH v2 3/5] hpet 'driftfix': add fields to HPETTimer and VMStateDescription
typedef struct HPETState { @@ -248,7 +253,7 @@ static int hpet_post_load(void *opaque, int version_id) static const VMStateDescription vmstate_hpet_timer = { .name = hpet_timer, - .version_id = 1, + .version_id = 3, Why jump from 1 to 3? .minimum_version_id = 1, .minimum_version_id_old = 1, .fields = (VMStateField []) { @@ -258,6 +263,11 @@ static const VMStateDescription vmstate_hpet_timer = { VMSTATE_UINT64(fsb, HPETTimer), VMSTATE_UINT64(period, HPETTimer), VMSTATE_UINT8(wrap_flag, HPETTimer), + VMSTATE_UINT64_V(saved_period, HPETTimer, 3), + VMSTATE_UINT64_V(ticks_not_accounted, HPETTimer, 3), + VMSTATE_UINT32_V(irqs_to_inject, HPETTimer, 3), + VMSTATE_UINT32_V(irq_rate, HPETTimer, 3), + VMSTATE_UINT32_V(divisor, HPETTimer, 3), Anthony, I incremented the version ID of 'vmstate_hpet' from 2 to 3 to make sure that migrations from a QEMU process that is capable of 'driftfix' to a QEMU process that is _not_ capable of 'driftfix' will fail. I assigned version ID 3 to 'vmstate_hpet_timer' and to the new fields in there too to indicate that adding those fields was the reason why the version ID of 'vmstate_hpet' was incremented to 3. As far as the flow of execution in vmstate_load_state() is concerned, I think it does not matter whether the version ID of 'vmstate_hpet_timer' and the new fields in there is 2 or 3 (as long as they are consistent). When the 'while(field-name)' loop in vmstate_load_state() gets to the following field in 'vmstate_hpet' ... VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers, 0, vmstate_hpet_timer, HPETTimer), ... it calls itself recursively ... if (field-flags VMS_STRUCT) { ret = vmstate_load_state(f, field-vmsd, addr, field-vmsd-version_id); 'field-vmsd-version_id' is the version ID of 'vmstate_hpet_timer' [1]. Hence 'vmstate_hpet_timer.version_id' is being checked against itself ... if (version_id vmsd-version_id) { return -EINVAL; } ... and the version IDs of the new fields are also being checked against 'vmstate_hpet_timer.version_id' ... if ((field-field_exists field-field_exists(opaque, version_id)) || (!field-field_exists field-version_id = version_id)) { If you want me to change the version ID of 'vmstate_hpet_timer' and the new fields in there from 3 to 2, I can do that. Regards, Uli [1] Ref.: commit fa3aad24d94a6cf894db52d83f72a399324a17bb
[Qemu-devel] Re: To O_EXCL or not to O_EXCL open host_cdrom
On Mon, Apr 11, 2011 at 10:37:32AM +0530, Amit Shah wrote: On (Fri) 08 Apr 2011 [12:33:27], Stefan Hajnoczi wrote: The other concern I have about using O_EXCL is that we expose ourselves to race conditions if there is ever a need to re-open the device. When QEMU closes its file descriptor another program may be scheduled to run and open the device with O_EXCL. Now QEMU will not be able to open the CD-ROM anymore. The admins should really be the ones worrying about this, not QEMU. Think of a desktop use case. virt-manager lets me pass through the host CD-ROM today. Desktops have hald/udisks and you can't expect users to disable/reenable those services just for QEMU. Stefan
Re: [Qemu-devel] Question about total_sectors in block/vpc.c
On Sun, Apr 10, 2011 at 05:02:20PM +0800, Lyu Mitnick wrote: diff --git a/block.c b/block.c index f731c7a..a80ec49 100644 --- a/block.c +++ b/block.c @@ -239,6 +239,16 @@ int bdrv_create(BlockDriver *drv, const char* filename, if (!drv-bdrv_create) return -ENOTSUP; + while (options options-name) { + if (!strcmp(options-name, size)) { + if (options-value.n % 512 == 0) + break; + else + return -EINVAL; + } + options++; + } Please use BDRV_SECTOR_SIZE instead of hardcoding 512. get_option_parameter() does the search for you, please use it instead of duplicating the loop. Please see the CODING_STYLE and HACKING files, new code should follow it: * Indentation is 4 spaces * Always use {} even for if/else with single-statement bodies Stefan
Re: [Qemu-devel] [PATCH] qemu-iotests: Fixed no scratch directory in qemu-iotests
On Sat, Apr 09, 2011 at 05:21:24AM +0800, Lyu Mitnick wrote: This my first time to submit patch, please tell me if I have something wrong! Please CC Christoph Hellwig h...@lst.de who maintains qemu-iotests. Also, please prepend the email subject with [qemu-iotests] so it's easy to spot that this patch is for qemu-iotests.git and not qemu.git.. diff --git a/common.config b/common.config index bdd0530..09923d9 100644 --- a/common.config +++ b/common.config @@ -102,6 +102,15 @@ export QEMU_IO=$QEMU_IO_PROG $QEMU_IO_OPTIONS [ -f /etc/qemu-iotest.config ]. /etc/qemu-iotest.config +if [ -e scratch -a ! -d scratch ]; then +echo scratch exist and is not a directory +exit 1 +fi + +if [ ! -e scratch ]; then +mkdir scratch +fi + if [ ! -e $TEST_DIR ]; then TEST_DIR=`pwd`/scratch fi TEST_DIR is the temporary directory used to keep files while a test runs. You've hardcoded 'scratch' and will create it in the current directory. The user should be able to set TEST_DIR. Please use TEST_DIR instead of hardcoding 'scratch'. For example, the following should create /tmp/iotests-tmp if it does not exist already: $ TEST_DIR=/tmp/iotests-tmp ./check And the following should create ./scratch if it does not exist already: $ ./check Stefan
[Qemu-devel] Re: [PATCH 3/5] atapi: GESN: Spin off No Event Available handling into own function
Am 09.04.2011 12:36, schrieb Amit Shah: On (Fri) 08 Apr 2011 [15:31:49], Kevin Wolf wrote: Am 08.04.2011 09:15, schrieb Amit Shah: Handle GET_EVENT_STATUS_NOTIFICATION's No Event Available status in its own function. Also ensure the buffer the driver sent us is big enough to fill in all the data we have -- else just fill in as much as the buffer can hold. This is unnecessary and in fact none of the IDE code does this. s-io_buffer isn't guest memory, but an internal buffer that is allocated like this: s-io_buffer = qemu_memalign(2048, IDE_DMA_BUF_SECTORS*512 + 4); OK - so all the code paths will be much easier then. But by my reading of (the kernel) code, it looks as if the kernel allocates the memory and passes it on. What am I missing? That the data is copied. All command work on the internal s-io_buffer. Once the command is completed, the response can be transferred to the guest, either using DMA (see bmdma_rw_buf) or PIO (ide_data_readl). So that's more than enough for storing four bytes. ide_atapi_cmd_reply() takes care of making only max_size bytes visible to the guest. OK - but in some cases we just do a ide_set_irq() instead of ide_atapi_cmd_reply() so what happens in that case? I can't find any ATAPI command that calls ide_set_irq directly. Anyway, the important thing is that s-io_buffer_size is set correctly. Kevin
Re: [Qemu-devel] [PATCH v2 3/5] hpet 'driftfix': add fields to HPETTimer and VMStateDescription
vmstate_hpet_timer = { VMSTATE_UINT64(fsb, HPETTimer), VMSTATE_UINT64(period, HPETTimer), VMSTATE_UINT8(wrap_flag, HPETTimer), + VMSTATE_UINT64_V(saved_period, HPETTimer, 3), + VMSTATE_UINT64_V(ticks_not_accounted, HPETTimer, 3), + VMSTATE_UINT32_V(irqs_to_inject, HPETTimer, 3), + VMSTATE_UINT32_V(irq_rate, HPETTimer, 3), + VMSTATE_UINT32_V(divisor, HPETTimer, 3), We ought to be able to use a subsection keyed off of whether any ticks are currently accumulated, no? Anthony, I'm not sure if I understand your question correctly. Are you suggesting to migrate the driftfix-related state conditionally / only if there are any ticks accumulated in 'ticks_not_accounted' and 'irqs_to_inject' ? The size of the driftfix-related state is 28 bytes per timer and we have 32 timers per HPETState, i.e. 896 additional bytes per HPETState. With a maximum number of 8 HPET blocks (HPETState), this amounts to 7168 bytes. Hence, unconditional migration of the driftfix-related state should not cause significant additional overhead. Maybe I missed something. Could you please explain which benefit you see in using a subsection ? Regards, Uli
Re: [Qemu-devel] [PATCH v2 3/5] hpet 'driftfix': add fields to HPETTimer and VMStateDescription
On 04/11/2011 12:06 PM, Ulrich Obergfell wrote: vmstate_hpet_timer = { VMSTATE_UINT64(fsb, HPETTimer), VMSTATE_UINT64(period, HPETTimer), VMSTATE_UINT8(wrap_flag, HPETTimer), + VMSTATE_UINT64_V(saved_period, HPETTimer, 3), + VMSTATE_UINT64_V(ticks_not_accounted, HPETTimer, 3), + VMSTATE_UINT32_V(irqs_to_inject, HPETTimer, 3), + VMSTATE_UINT32_V(irq_rate, HPETTimer, 3), + VMSTATE_UINT32_V(divisor, HPETTimer, 3), We ought to be able to use a subsection keyed off of whether any ticks are currently accumulated, no? Anthony, I'm not sure if I understand your question correctly. Are you suggesting to migrate the driftfix-related state conditionally / only if there are any ticks accumulated in 'ticks_not_accounted' and 'irqs_to_inject' ? The size of the driftfix-related state is 28 bytes per timer and we have 32 timers per HPETState, i.e. 896 additional bytes per HPETState. With a maximum number of 8 HPET blocks (HPETState), this amounts to 7168 bytes. Hence, unconditional migration of the driftfix-related state should not cause significant additional overhead. It's not about overhead. Maybe I missed something. Could you please explain which benefit you see in using a subsection ? In the common case of there being no drift, you can migrate from a qemu that supports driftfix to a qemu that doesn't. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 1/4] vnc: tight: Fix crash after 2GB of output
Aurelien, could you also take a look at http://patchwork.ozlabs.org/patch/87717/ ? Thanks, -- Corentin Chary http://xf.iksaif.net
Re: [Qemu-devel] [PATCH 1/4] vnc: tight: Fix crash after 2GB of output
On Mon, Apr 11, 2011 at 11:27:23AM +0200, Corentin Chary wrote: Aurelien, could you also take a look at http://patchwork.ozlabs.org/patch/87717/ ? I gave a look, but I don't really know this part of QEMU, so I would prefer if someone else look at it. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 0/3] qed: Add support for zero clusters
Am 17.12.2010 16:58, schrieb Stefan Hajnoczi: This patch series adds zero data clusters to QED. Clusters can be marked as zero clusters to store zeroed regions in a space-efficient way. The patch never actually creates new zero clusters but includes the I/O path support code to handle them if they are used by an image file. Image streaming and copy-on-read take advantage of zero data clusters to avoid expanding out zeroes from the backing file. Those features are separate patches that will come later but I'm presenting this patch now so we can get this core QED image format feature in before doing the first QEMU release containing QED. The first patch fixes up an issue with the QED merge where '^' characters were dropped from the QED specification. The last two patches document and implement zero clusters, which were originally implemented by Anthony Liguori aligu...@us.ibm.com. Thanks, applied all to the block branch. Kevin
[Qemu-devel] Re: Slow PXE boot in qemu.git (fast in qemu-kvm.git)
On Sat, 9 Apr 2011 13:34:43 +0300 Blue Swirl blauwir...@gmail.com wrote: On Sat, Apr 9, 2011 at 2:25 AM, Luiz Capitulino lcapitul...@redhat.com wrote: Hi there, Summary: - PXE boot in qemu.git (HEAD f124a41) is quite slow, more than 5 minutes. Got the problem with e1000, virtio and rtl8139. However, pcnet *works* (it's as fast as qemu-kvm.git) - PXE boot in qemu-kvm.git (HEAD df85c051) is fast, less than a minute. Tried with e1000, virtio and rtl8139 (I don't remember if I tried with pcnet) I tried with qemu.git v0.13.0 in order to check if this was a regression, but I got the same problem... Then I inspected qemu-kvm.git under the assumption that it could have a fix that wasn't commited to qemu.git. Found this: - commit 0836b77f0f65d56d08bdeffbac25cd6d78267dc9 which is merge, works - commit cc015e9a5dde2f03f123357fa060acbdfcd570a4 does not work (it's slow) I tried a bisect, but it brakes due to gcc4 vs. gcc3 changes. Then I inspected commits manually, and found out that commit 64d7e9a4 doesn't work, which makes me think that the fix could be in the conflict resolution of 0836b77f, which makes me remember that I'm late for diner, so my conclusions at this point are not reliable :) Ideas? What is the test case? It's an external PXE server, command-line is: qemu -boot n -enable-kvm -net nic,model=virtio -net tap,ifname=vnet0,script= I tried PXE booting a 10M file with and without KVM and the results are pretty much the same with pcnet and e1000. time qemu -monitor stdio -boot n -net nic,model=e1000 -net user,tftp=.,bootfile=10M -net dump,file=foo -enable-kvm time qemu -monitor stdio -boot n -net nic,model=pcnet -net user,tftp=.,bootfile=10M -net dump,file=foo -enable-kvm time qemu -monitor stdio -boot n -net nic,model=e1000 -net user,tftp=.,bootfile=10M -net dump,file=foo time qemu -monitor stdio -boot n -net nic,model=pcnet -net user,tftp=.,bootfile=10M -net dump,file=foo All times are ~10s. Yeah, you're using the internal tftp server.
Re: [Qemu-devel] [PATCH v2 3/5] hpet 'driftfix': add fields to HPETTimer and VMStateDescription
On 04/11/2011 04:08 AM, Avi Kivity wrote: On 04/11/2011 12:06 PM, Ulrich Obergfell wrote: vmstate_hpet_timer = { VMSTATE_UINT64(fsb, HPETTimer), VMSTATE_UINT64(period, HPETTimer), VMSTATE_UINT8(wrap_flag, HPETTimer), + VMSTATE_UINT64_V(saved_period, HPETTimer, 3), + VMSTATE_UINT64_V(ticks_not_accounted, HPETTimer, 3), + VMSTATE_UINT32_V(irqs_to_inject, HPETTimer, 3), + VMSTATE_UINT32_V(irq_rate, HPETTimer, 3), + VMSTATE_UINT32_V(divisor, HPETTimer, 3), We ought to be able to use a subsection keyed off of whether any ticks are currently accumulated, no? Anthony, I'm not sure if I understand your question correctly. Are you suggesting to migrate the driftfix-related state conditionally / only if there are any ticks accumulated in 'ticks_not_accounted' and 'irqs_to_inject' ? The size of the driftfix-related state is 28 bytes per timer and we have 32 timers per HPETState, i.e. 896 additional bytes per HPETState. With a maximum number of 8 HPET blocks (HPETState), this amounts to 7168 bytes. Hence, unconditional migration of the driftfix-related state should not cause significant additional overhead. It's not about overhead. Maybe I missed something. Could you please explain which benefit you see in using a subsection ? In the common case of there being no drift, you can migrate from a qemu that supports driftfix to a qemu that doesn't. Right, subsections are a trick. The idea is that when you introduce new state for a device model that is not always going to be set, when you do the migration, you detect whether the state is set or not and if it's not set, instead of sending empty versions of that state (i.e. missed_ticks=0) you just don't send the new state at all. This means that you can migrate to an older version of QEMU provided the migration would work correctly. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH v2 3/5] hpet 'driftfix': add fields to HPETTimer and VMStateDescription
On 04/11/2011 03:24 AM, Ulrich Obergfell wrote: typedef struct HPETState { @@ -248,7 +253,7 @@ static int hpet_post_load(void *opaque, int version_id) static const VMStateDescription vmstate_hpet_timer = { .name = hpet_timer, - .version_id = 1, + .version_id = 3, Why jump from 1 to 3? .minimum_version_id = 1, .minimum_version_id_old = 1, .fields = (VMStateField []) { @@ -258,6 +263,11 @@ static const VMStateDescription vmstate_hpet_timer = { VMSTATE_UINT64(fsb, HPETTimer), VMSTATE_UINT64(period, HPETTimer), VMSTATE_UINT8(wrap_flag, HPETTimer), + VMSTATE_UINT64_V(saved_period, HPETTimer, 3), + VMSTATE_UINT64_V(ticks_not_accounted, HPETTimer, 3), + VMSTATE_UINT32_V(irqs_to_inject, HPETTimer, 3), + VMSTATE_UINT32_V(irq_rate, HPETTimer, 3), + VMSTATE_UINT32_V(divisor, HPETTimer, 3), Anthony, I incremented the version ID of 'vmstate_hpet' from 2 to 3 to make sure that migrations from a QEMU process that is capable of 'driftfix' to a QEMU process that is _not_ capable of 'driftfix' will fail. I assigned version ID 3 to 'vmstate_hpet_timer' and to the new fields in there too to indicate that adding those fields was the reason why the version ID of 'vmstate_hpet' was incremented to 3. As far as the flow of execution in vmstate_load_state() is concerned, I think it does not matter whether the version ID of 'vmstate_hpet_timer' and the new fields in there is 2 or 3 (as long as they are consistent). When the 'while(field-name)' loop in vmstate_load_state() gets to the following field in 'vmstate_hpet' ... VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers, 0, vmstate_hpet_timer, HPETTimer), ... it calls itself recursively ... if (field-flags VMS_STRUCT) { ret = vmstate_load_state(f, field-vmsd, addr, field-vmsd-version_id); 'field-vmsd-version_id' is the version ID of 'vmstate_hpet_timer' [1]. Hence 'vmstate_hpet_timer.version_id' is being checked against itself ... if (version_id vmsd-version_id) { return -EINVAL; } ... and the version IDs of the new fields are also being checked against 'vmstate_hpet_timer.version_id' ... if ((field-field_exists field-field_exists(opaque, version_id)) || (!field-field_exists field-version_id= version_id)) { If you want me to change the version ID of 'vmstate_hpet_timer' and the new fields in there from 3 to 2, I can do that. It avoids surprises so I think it's a reasonable thing to do. But yes, your analysis is correct. Regards, Anthony Liguori Regards, Uli [1] Ref.: commit fa3aad24d94a6cf894db52d83f72a399324a17bb
Re: [Qemu-devel] To O_EXCL or not to O_EXCL open host_cdrom
On 04/08/2011 02:33 PM, Stefan Hajnoczi wrote: Amit and I were discussing the pros and cons of using O_EXCL to open host CD-ROM devices on IRC but this discussion could benefit from more input. Linux block devices (like /dev/sr0 CD-ROMs) can be opened with O_EXCL and only one userspace process will succeed at a time. This prevents programs from interfering with each other. The polling daemons, hald and udisks, use O_EXCL and mount does too. Today QEMU does not use O_EXCL and will therefore access host CD-ROMs while they are in use by other programs. This also means that programs can be started on the host while QEMU is already running that may interfere with the virtual machine's ability to access the CD-ROM (for example by ejecting it). Also, performance would likely be ruined if the disk wasn't cached (likely with a DVD). CD-ROMs seek very slowly. It would be even funnier if we supported cd-writers. Therefore, it sounds reasonable to switch to O_EXCL to prevent interfering with other programs and to prevent other programs interfering with QEMU. On the downside, it will no longer be possible to share a host CD-ROM between multiple virtual machines or to mount it on host while passing it through to a guest. These scenarios are not safe because on of the clients could eject the device, spoiling the party for everyone else. However, it is a handy feature for putting installation media into a machine and installing several guests at the same time. You'd probably benefit a lot more by copying the media to disk. The other concern I have about using O_EXCL is that we expose ourselves to race conditions if there is ever a need to re-open the device. When QEMU closes its file descriptor another program may be scheduled to run and open the device with O_EXCL. Now QEMU will not be able to open the CD-ROM anymore. From the guest perspective this could be at an odd time and we'd have to start failing requests. Today we do not re-open host CD-ROMs though so this isn't a pressing problem. We really should avoid re-open whenever possible. The standard response to such questions is let's add another option, but in this case I don't think we should. Like Amit says, this is a niche use case. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: To O_EXCL or not to O_EXCL open host_cdrom
On 04/11/2011 11:31 AM, Stefan Hajnoczi wrote: On Mon, Apr 11, 2011 at 10:37:32AM +0530, Amit Shah wrote: On (Fri) 08 Apr 2011 [12:33:27], Stefan Hajnoczi wrote: The other concern I have about using O_EXCL is that we expose ourselves to race conditions if there is ever a need to re-open the device. When QEMU closes its file descriptor another program may be scheduled to run and open the device with O_EXCL. Now QEMU will not be able to open the CD-ROM anymore. The admins should really be the ones worrying about this, not QEMU. Think of a desktop use case. virt-manager lets me pass through the host CD-ROM today. Desktops have hald/udisks and you can't expect users to disable/reenable those services just for QEMU. It should be solved at that level then. If I insert a disc into an assigned cd-rom drive, I shouldn't get a file manager or autorun window to pop in the host, just the guest. So: libvirt should inform the rest of the system that it is taking over the cd-rom and as far as they're concerned, it no longer exists. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH 0/7] Rework PCMCIA subsystem
Please pull the following changeset that makes PCMCIA subsystem to use QBus and Qdev for managing devices. Currently the only implementation of PCMCIA host is a PXA2xx host and the only possible PCMCIA device is IDE MicroDrive (dscm1). With this patchset I can create a microdrive device from command line: -device dscm1 -device ide-drive,drive=test -drive if=none,id=test,file=/dev/null Dmitry Eremin-Solenikov (7): pxa2xx_pcmcia: qdevify PCMCIA: start qdev'ication microdrive: qdevify pcmcia: move all card callbacks to PCMCIACardInfo pcmcia: move attach and detach socket methods to PCMCIASocket pxa: change order of pcmcia devices instantiation, so that the socket 0 will be default ide-core: allocate metadata storage for CFATA drives Makefile.objs |3 + hw/ide/core.c |4 ++ hw/ide/internal.h |2 + hw/ide/microdrive.c | 88 +++--- hw/mainstone.c | 14 +++-- hw/pcmcia.c | 145 + hw/pcmcia.h | 49 - hw/pxa.h|9 +--- hw/pxa2xx.c |9 ++-- hw/pxa2xx_pcmcia.c | 148 ++- hw/spitz.c | 26 ++ hw/tosa.c | 18 --- vl.c| 43 --- 13 files changed, 372 insertions(+), 186 deletions(-) create mode 100644 hw/pcmcia.c
[Qemu-devel] [PATCH 5/7] pcmcia: move attach and detach socket methods to PCMCIASocket
Make attach and detach calls to be automatically called by PCMCIA card instantiation code, rather than calling them by hand from the board code. Signed-off-by: Dmitry Eremin-Solenikov dbarysh...@gmail.com --- hw/pcmcia.c| 45 +++- hw/pcmcia.h|2 + hw/pxa.h |4 --- hw/pxa2xx_pcmcia.c | 73 +++ hw/spitz.c |1 - hw/tosa.c |1 - 6 files changed, 79 insertions(+), 47 deletions(-) diff --git a/hw/pcmcia.c b/hw/pcmcia.c index 17a49b6..d661663 100644 --- a/hw/pcmcia.c +++ b/hw/pcmcia.c @@ -86,16 +86,59 @@ static int pcmcia_device_init(DeviceState *dev, DeviceInfo *info) { PCMCIACardState *state = DO_UPCAST(PCMCIACardState, dev, dev); PCMCIACardInfo *pinfo = DO_UPCAST(PCMCIACardInfo, qdev, info); +PCMCIASocket *socket = DO_UPCAST(PCMCIASocket, qbus, dev-parent_bus); int rc; +if (socket-attached) { +return -1; +} + state-info = pinfo; rc = pinfo-init(state); -return rc; +if (rc) { +return rc; +} + +socket-attached = 1; +state-slot = socket; + +rc = socket-attach(socket, state); +if (rc) { +return rc; +} + +rc = state-info-attach(state); +if (rc) { +socket-detach(socket); +return rc; +} + +return 0; +} + +static int pcmcia_device_exit(DeviceState *dev) +{ +PCMCIACardState *state = DO_UPCAST(PCMCIACardState, dev, dev); +PCMCIASocket *socket = DO_UPCAST(PCMCIASocket, qbus, dev-parent_bus); + +if (!socket-attached) { +return -ENOENT; +} + +state-info-detach(state); +state-slot = NULL; + +socket-detach(socket); + +socket-attached = 0; + +return 0; } void pcmcia_card_register(PCMCIACardInfo *info) { info-qdev.init = pcmcia_device_init; +info-qdev.exit = pcmcia_device_exit; info-qdev.bus_info = pcmcia_bus_info; assert(info-qdev.size = sizeof(PCMCIACardState)); qdev_register(info-qdev); diff --git a/hw/pcmcia.h b/hw/pcmcia.h index 2c012d9..4f90af7 100644 --- a/hw/pcmcia.h +++ b/hw/pcmcia.h @@ -11,6 +11,8 @@ struct PCMCIASocket { int attached; const char *slot_string; const char *card_string; +int (*attach)(PCMCIASocket *socket, PCMCIACardState *card); +int (*detach)(PCMCIASocket *socket); }; void pcmcia_socket_register(PCMCIASocket *socket, DeviceState *parent); diff --git a/hw/pxa.h b/hw/pxa.h index 25176ef..c145029 100644 --- a/hw/pxa.h +++ b/hw/pxa.h @@ -89,10 +89,6 @@ PXA2xxMMCIState *pxa2xx_mmci_init(target_phys_addr_t base, void pxa2xx_mmci_handlers(PXA2xxMMCIState *s, qemu_irq readonly, qemu_irq coverswitch); -/* pxa2xx_pcmcia.c */ -int pxa2xx_pcmcia_attach(void *opaque, PCMCIACardState *card); -int pxa2xx_pcmcia_dettach(void *opaque); - /* pxa2xx_keypad.c */ struct keymap { int column; diff --git a/hw/pxa2xx_pcmcia.c b/hw/pxa2xx_pcmcia.c index ae7d01a..efd4c09 100644 --- a/hw/pxa2xx_pcmcia.c +++ b/hw/pxa2xx_pcmcia.c @@ -150,6 +150,37 @@ DeviceState *pxa2xx_pcmcia_init(target_phys_addr_t base, uint8_t id) return dev-qdev; } +/* Insert a new card into a slot */ +static int pxa2xx_pcmcia_attach(PCMCIASocket *socket, PCMCIACardState *card) +{ +PXA2xxPCMCIAState *s = container_of(socket, PXA2xxPCMCIAState, slot); + +if (s-cd_irq) { +qemu_irq_raise(s-cd_irq); +} + +s-card = card; + +return 0; +} + +/* Eject card from the slot */ +static int pxa2xx_pcmcia_detach(PCMCIASocket *socket) +{ +PXA2xxPCMCIAState *s = container_of(socket, PXA2xxPCMCIAState, slot); + +s-card = NULL; + +if (s-irq) { +qemu_irq_lower(s-irq); +} +if (s-cd_irq) { +qemu_irq_lower(s-cd_irq); +} + +return 0; +} + static int pxa2xx_pcmcia_initfn(SysBusDevice *dev) { int iomemtype; @@ -186,48 +217,10 @@ static int pxa2xx_pcmcia_initfn(SysBusDevice *dev) snprintf(str, 256, PXA PC Card Socket %d, s-id); s-slot.slot_string = str; +s-slot.attach = pxa2xx_pcmcia_attach; +s-slot.detach = pxa2xx_pcmcia_detach; s-slot.irq = qemu_allocate_irqs(pxa2xx_pcmcia_set_irq, s, 1)[0]; pcmcia_socket_register(s-slot, s-busdev.qdev); -return 0; -} - -/* Insert a new card into a slot */ -int pxa2xx_pcmcia_attach(void *opaque, PCMCIACardState *card) -{ -PXA2xxPCMCIAState *s = (PXA2xxPCMCIAState *) opaque; -if (s-slot.attached) -return -EEXIST; - -if (s-cd_irq) { -qemu_irq_raise(s-cd_irq); -} - -s-card = card; - -s-slot.attached = 1; -s-card-slot = s-slot; -s-card-info-attach(s-card); - -return 0; -} - -/* Eject card from the slot */ -int pxa2xx_pcmcia_dettach(void *opaque) -{ -PXA2xxPCMCIAState *s = (PXA2xxPCMCIAState *) opaque; -if (!s-slot.attached) -return -ENOENT; - -s-card-info-detach(s-card); -s-card-slot = NULL; -s-card = NULL; - -s-slot.attached = 0; - -if (s-irq) -
[Qemu-devel] [PATCH 2/7] PCMCIA: start qdev'ication
Convert PCMCIA bus handling code to use QBus internally. MicroDrive code is still unaffected. Signed-off-by: Dmitry Eremin-Solenikov dbarysh...@gmail.com --- Makefile.objs |3 ++ hw/pcmcia.c| 102 hw/pcmcia.h| 15 +++- hw/pxa2xx_pcmcia.c |2 +- vl.c | 43 -- 5 files changed, 120 insertions(+), 45 deletions(-) create mode 100644 hw/pcmcia.c diff --git a/Makefile.objs b/Makefile.objs index 44ce368..153a148 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -289,6 +289,9 @@ hw-obj-$(CONFIG_REALLY_VIRTFS) += virtio-9p-debug.o hw-obj-$(CONFIG_VIRTFS) += virtio-9p-local.o virtio-9p-xattr.o hw-obj-$(CONFIG_VIRTFS) += virtio-9p-xattr-user.o virtio-9p-posix-acl.o +# PCMCIA +hw-obj-y += pcmcia.o + ## # libdis # NOTE: the disassembler code is only needed for debugging diff --git a/hw/pcmcia.c b/hw/pcmcia.c new file mode 100644 index 000..17a49b6 --- /dev/null +++ b/hw/pcmcia.c @@ -0,0 +1,102 @@ +/* + * QEMU System Emulator + * PCMCIA subsystem + * + * Copyright (c) 2003-2008 Fabrice Bellard + * Copyright (c) 2011 Dmitry Eremin-Solenikov + * + * 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 hw.h +#include pcmcia.h +#include monitor.h + +/***/ +/* PCMCIA/Cardbus */ + +static struct pcmcia_socket_entry_s { +PCMCIASocket *socket; +struct pcmcia_socket_entry_s *next; +} *pcmcia_sockets = 0; + +static BusInfo pcmcia_bus_info = { +.name = PCMCIA, +.size = sizeof(PCMCIASocket), +}; + +void pcmcia_socket_register(PCMCIASocket *socket, DeviceState *parent) +{ +struct pcmcia_socket_entry_s *entry; + +qbus_create_inplace(socket-qbus, pcmcia_bus_info, +parent, pcmcia); + +entry = qemu_malloc(sizeof(struct pcmcia_socket_entry_s)); +entry-socket = socket; +entry-next = pcmcia_sockets; +pcmcia_sockets = entry; +} + +void pcmcia_socket_unregister(PCMCIASocket *socket) +{ +struct pcmcia_socket_entry_s *entry, **ptr; + +ptr = pcmcia_sockets; +for (entry = *ptr; entry; ptr = entry-next, entry = *ptr) +if (entry-socket == socket) { +*ptr = entry-next; +qemu_free(entry); +} + +qbus_free(socket-qbus); +} + +void pcmcia_info(Monitor *mon) +{ +struct pcmcia_socket_entry_s *iter; + +if (!pcmcia_sockets) { +monitor_printf(mon, No PCMCIA sockets\n); +} + +for (iter = pcmcia_sockets; iter; iter = iter-next) { +monitor_printf(mon, %s: %s\n, iter-socket-slot_string, + iter-socket-attached ? iter-socket-card_string : + Empty); +} +} + +static int pcmcia_device_init(DeviceState *dev, DeviceInfo *info) +{ +PCMCIACardState *state = DO_UPCAST(PCMCIACardState, dev, dev); +PCMCIACardInfo *pinfo = DO_UPCAST(PCMCIACardInfo, qdev, info); +int rc; + +state-info = pinfo; +rc = pinfo-init(state); +return rc; +} + +void pcmcia_card_register(PCMCIACardInfo *info) +{ +info-qdev.init = pcmcia_device_init; +info-qdev.bus_info = pcmcia_bus_info; +assert(info-qdev.size = sizeof(PCMCIACardState)); +qdev_register(info-qdev); +} diff --git a/hw/pcmcia.h b/hw/pcmcia.h index f0b16b8..561d86c 100644 --- a/hw/pcmcia.h +++ b/hw/pcmcia.h @@ -1,19 +1,30 @@ /* PCMCIA/Cardbus */ #include qemu-common.h +#include qdev.h typedef struct { +BusState qbus; qemu_irq irq; int attached; const char *slot_string; const char *card_string; } PCMCIASocket; -void pcmcia_socket_register(PCMCIASocket *socket); +void pcmcia_socket_register(PCMCIASocket *socket, DeviceState *parent); void pcmcia_socket_unregister(PCMCIASocket *socket); void pcmcia_info(Monitor *mon); +typedef struct PCMCIACardInfo { +DeviceInfo qdev; + +
[Qemu-devel] [PATCH 4/7] pcmcia: move all card callbacks to PCMCIACardInfo
Signed-off-by: Dmitry Eremin-Solenikov dbarysh...@gmail.com last commit fixup Signed-off-by: Dmitry Eremin-Solenikov dbarysh...@gmail.com Revert microdrive fixup This reverts commit 6a9f969b0626e218ff910d84ed1c9eec285cbcd5. Signed-off-by: Dmitry Eremin-Solenikov dbarysh...@gmail.com --- hw/ide/microdrive.c | 41 - hw/pcmcia.h | 29 +++-- hw/pxa2xx_pcmcia.c | 16 3 files changed, 43 insertions(+), 43 deletions(-) diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c index 167ec09..a251544 100644 --- a/hw/ide/microdrive.c +++ b/hw/ide/microdrive.c @@ -113,9 +113,9 @@ static void md_reset(MicroDriveState *s) ide_bus_reset(s-bus); } -static uint8_t md_attr_read(void *opaque, uint32_t at) +static uint8_t md_attr_read(PCMCIACardState *opaque, uint32_t at) { -MicroDriveState *s = opaque; +MicroDriveState *s = DO_UPCAST(MicroDriveState, card, opaque); if (at s-attr_base) { if (at s-card.cis_len) return s-card.cis[at]; @@ -146,9 +146,9 @@ static uint8_t md_attr_read(void *opaque, uint32_t at) return 0; } -static void md_attr_write(void *opaque, uint32_t at, uint8_t value) +static void md_attr_write(PCMCIACardState *opaque, uint32_t at, uint8_t value) { -MicroDriveState *s = opaque; +MicroDriveState *s = DO_UPCAST(MicroDriveState, card, opaque); at -= s-attr_base; switch (at) { @@ -177,9 +177,9 @@ static void md_attr_write(void *opaque, uint32_t at, uint8_t value) } } -static uint16_t md_common_read(void *opaque, uint32_t at) +static uint16_t md_common_read(PCMCIACardState *opaque, uint32_t at) { -MicroDriveState *s = opaque; +MicroDriveState *s = DO_UPCAST(MicroDriveState, card, opaque); IDEState *ifs; uint16_t ret; at -= s-io_base; @@ -239,9 +239,9 @@ static uint16_t md_common_read(void *opaque, uint32_t at) return 0; } -static void md_common_write(void *opaque, uint32_t at, uint16_t value) +static void md_common_write(PCMCIACardState *opaque, uint32_t at, uint16_t value) { -MicroDriveState *s = opaque; +MicroDriveState *s = DO_UPCAST(MicroDriveState, card, opaque); at -= s-io_base; switch (s-opt OPT_MODE) { @@ -503,15 +503,9 @@ static const uint8_t dscm1_cis[0x14a] = { [0x146] = CISTPL_END, /* Tuple End */ }; -static int dscm1_attach(void *opaque) +static int dscm1_attach(PCMCIACardState *opaque) { -MicroDriveState *md = opaque; -md-card.attr_read = md_attr_read; -md-card.attr_write = md_attr_write; -md-card.common_read = md_common_read; -md-card.common_write = md_common_write; -md-card.io_read = md_common_read; -md-card.io_write = md_common_write; +MicroDriveState *md = DO_UPCAST(MicroDriveState, card, opaque); md-attr_base = md-card.cis[0x74] | (md-card.cis[0x76] 8); md-io_base = 0x0; @@ -523,9 +517,9 @@ static int dscm1_attach(void *opaque) return 0; } -static int dscm1_detach(void *opaque) +static int dscm1_detach(PCMCIACardState *opaque) { -MicroDriveState *md = opaque; +MicroDriveState *md = DO_UPCAST(MicroDriveState, card, opaque); md_reset(md); return 0; } @@ -552,9 +546,6 @@ static int dscm1_initfn(PCMCIACardState *state) MicroDriveState *md; md = DO_UPCAST(MicroDriveState, card, state); -md-card.state = md; -md-card.attach = dscm1_attach; -md-card.detach = dscm1_detach; md-card.cis = dscm1_cis; md-card.cis_len = sizeof(dscm1_cis); @@ -571,6 +562,14 @@ static PCMCIACardInfo dscm1_info = { .init = dscm1_initfn, .qdev.size = sizeof(MicroDriveState), .qdev.vmsd = vmstate_microdrive, +.attach = dscm1_attach, +.detach = dscm1_detach, +.attr_read = md_attr_read, +.attr_write = md_attr_write, +.common_read= md_common_read, +.common_write = md_common_write, +.io_read= md_common_read, +.io_write = md_common_write, }; static void dscm1_register(void) diff --git a/hw/pcmcia.h b/hw/pcmcia.h index c6b8100..2c012d9 100644 --- a/hw/pcmcia.h +++ b/hw/pcmcia.h @@ -3,13 +3,15 @@ #include qemu-common.h #include qdev.h -typedef struct { +typedef struct PCMCIASocket PCMCIASocket; + +struct PCMCIASocket { BusState qbus; qemu_irq irq; int attached; const char *slot_string; const char *card_string; -} PCMCIASocket; +}; void pcmcia_socket_register(PCMCIASocket *socket, DeviceState *parent); void pcmcia_socket_unregister(PCMCIASocket *socket); @@ -19,26 +21,25 @@ typedef struct PCMCIACardInfo { DeviceInfo qdev; int (*init)(PCMCIACardState *state); + +int (*attach)(PCMCIACardState *state); +int (*detach)(PCMCIACardState *state); + +/* Only valid if attached */ +uint8_t (*attr_read)(PCMCIACardState *state, uint32_t address); +void
[Qemu-devel] [PATCH 1/7] pxa2xx_pcmcia: qdevify
Use qdev insfrastructure for managing PXA PCMCIA devices. PCMCIA bus itself isn't converted to QBus (yet). pxa2xx_pcmcia_init() function is moved to pcmcia.h as it will be used by other cores/devices (like StrongARM). Signed-off-by: Dmitry Eremin-Solenikov dbarysh...@gmail.com --- hw/mainstone.c | 14 + hw/pcmcia.h|3 ++ hw/pxa.h |5 +--- hw/pxa2xx.c|9 +++--- hw/pxa2xx_pcmcia.c | 77 +-- hw/spitz.c | 20 - hw/tosa.c | 12 7 files changed, 91 insertions(+), 49 deletions(-) diff --git a/hw/mainstone.c b/hw/mainstone.c index 50691ca..77b224c 100644 --- a/hw/mainstone.c +++ b/hw/mainstone.c @@ -149,12 +149,14 @@ static void mainstone_common_init(ram_addr_t ram_size, /* MMC/SD host */ pxa2xx_mmci_handlers(cpu-mmc, NULL, qdev_get_gpio_in(mst_irq, MMC_IRQ)); -pxa2xx_pcmcia_set_irq_cb(cpu-pcmcia[0], -qdev_get_gpio_in(mst_irq, S0_IRQ), -qdev_get_gpio_in(mst_irq, S0_CD_IRQ)); -pxa2xx_pcmcia_set_irq_cb(cpu-pcmcia[1], -qdev_get_gpio_in(mst_irq, S1_IRQ), -qdev_get_gpio_in(mst_irq, S1_CD_IRQ)); +sysbus_connect_irq(sysbus_from_qdev(cpu-pcmcia[0]), 0, +qdev_get_gpio_in(mst_irq, S0_IRQ)); +sysbus_connect_irq(sysbus_from_qdev(cpu-pcmcia[0]), 1, +qdev_get_gpio_in(mst_irq, S0_CD_IRQ)); +sysbus_connect_irq(sysbus_from_qdev(cpu-pcmcia[1]), 0, +qdev_get_gpio_in(mst_irq, S1_IRQ)); +sysbus_connect_irq(sysbus_from_qdev(cpu-pcmcia[1]), 1, +qdev_get_gpio_in(mst_irq, S1_CD_IRQ)); smc91c111_init(nd_table[0], MST_ETH_PHYS, qdev_get_gpio_in(mst_irq, ETHERNET_IRQ)); diff --git a/hw/pcmcia.h b/hw/pcmcia.h index 50648c9..f0b16b8 100644 --- a/hw/pcmcia.h +++ b/hw/pcmcia.h @@ -47,5 +47,8 @@ struct PCMCIACardState { #define CISTPL_END 0xff/* Tuple End */ #define CISTPL_ENDMARK 0xff +/* pxa2xx_pcmcia.h -- used also for StrongARM */ +DeviceState *pxa2xx_pcmcia_init(target_phys_addr_t base, uint8_t id); + /* dscm1.c */ PCMCIACardState *dscm1_init(DriveInfo *bdrv); diff --git a/hw/pxa.h b/hw/pxa.h index d982f00..25176ef 100644 --- a/hw/pxa.h +++ b/hw/pxa.h @@ -90,11 +90,8 @@ void pxa2xx_mmci_handlers(PXA2xxMMCIState *s, qemu_irq readonly, qemu_irq coverswitch); /* pxa2xx_pcmcia.c */ -typedef struct PXA2xxPCMCIAState PXA2xxPCMCIAState; -PXA2xxPCMCIAState *pxa2xx_pcmcia_init(target_phys_addr_t base); int pxa2xx_pcmcia_attach(void *opaque, PCMCIACardState *card); int pxa2xx_pcmcia_dettach(void *opaque); -void pxa2xx_pcmcia_set_irq_cb(void *opaque, qemu_irq irq, qemu_irq cd_irq); /* pxa2xx_keypad.c */ struct keymap { @@ -126,7 +123,7 @@ typedef struct { SSIBus **ssp; PXA2xxI2CState *i2c[2]; PXA2xxMMCIState *mmc; -PXA2xxPCMCIAState *pcmcia[2]; +DeviceState *pcmcia[2]; PXA2xxI2SState *i2s; PXA2xxFIrState *fir; PXA2xxKeyPadState *kp; diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c index 9b95e2c..ef4c0a2 100644 --- a/hw/pxa2xx.c +++ b/hw/pxa2xx.c @@ -13,6 +13,7 @@ #include pc.h #include i2c.h #include ssi.h +#include pcmcia.h #include qemu-char.h #include blockdev.h @@ -2221,8 +,8 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) qdev_get_gpio_in(s-pic, PXA2XX_PIC_USBH1)); } -s-pcmcia[0] = pxa2xx_pcmcia_init(0x2000); -s-pcmcia[1] = pxa2xx_pcmcia_init(0x3000); +s-pcmcia[0] = pxa2xx_pcmcia_init(0x2000, 0); +s-pcmcia[1] = pxa2xx_pcmcia_init(0x3000, 1); sysbus_create_simple(pxa2xx_rtc, 0x4090, qdev_get_gpio_in(s-pic, PXA2XX_PIC_RTCALARM)); @@ -2357,8 +2358,8 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) qdev_get_gpio_in(s-pic, PXA2XX_PIC_USBH1)); } -s-pcmcia[0] = pxa2xx_pcmcia_init(0x2000); -s-pcmcia[1] = pxa2xx_pcmcia_init(0x3000); +s-pcmcia[0] = pxa2xx_pcmcia_init(0x2000, 0); +s-pcmcia[1] = pxa2xx_pcmcia_init(0x3000, 1); sysbus_create_simple(pxa2xx_rtc, 0x4090, qdev_get_gpio_in(s-pic, PXA2XX_PIC_RTCALARM)); diff --git a/hw/pxa2xx_pcmcia.c b/hw/pxa2xx_pcmcia.c index 50d4649..3d93829 100644 --- a/hw/pxa2xx_pcmcia.c +++ b/hw/pxa2xx_pcmcia.c @@ -9,15 +9,19 @@ #include hw.h #include pcmcia.h +#include sysbus.h #include pxa.h -struct PXA2xxPCMCIAState { +typedef struct PXA2xxPCMCIAState { +SysBusDevice busdev; + +uint8_t id; PCMCIASocket slot; PCMCIACardState *card; qemu_irq irq; qemu_irq cd_irq; -}; +} PXA2xxPCMCIAState; static uint32_t pxa2xx_pcmcia_common_read(void *opaque, target_phys_addr_t offset) @@ -130,39 +134,61 @@ static void pxa2xx_pcmcia_set_irq(void *opaque, int line, int level) qemu_set_irq(s-irq, level); }
Re: [Qemu-devel] [PATCH v2 3/5] hpet 'driftfix': add fields to HPETTimer and VMStateDescription
On Mon, 2011-04-11 at 08:10 -0500, Anthony Liguori wrote: On 04/11/2011 04:08 AM, Avi Kivity wrote: On 04/11/2011 12:06 PM, Ulrich Obergfell wrote: vmstate_hpet_timer = { VMSTATE_UINT64(fsb, HPETTimer), VMSTATE_UINT64(period, HPETTimer), VMSTATE_UINT8(wrap_flag, HPETTimer), + VMSTATE_UINT64_V(saved_period, HPETTimer, 3), + VMSTATE_UINT64_V(ticks_not_accounted, HPETTimer, 3), + VMSTATE_UINT32_V(irqs_to_inject, HPETTimer, 3), + VMSTATE_UINT32_V(irq_rate, HPETTimer, 3), + VMSTATE_UINT32_V(divisor, HPETTimer, 3), We ought to be able to use a subsection keyed off of whether any ticks are currently accumulated, no? Anthony, I'm not sure if I understand your question correctly. Are you suggesting to migrate the driftfix-related state conditionally / only if there are any ticks accumulated in 'ticks_not_accounted' and 'irqs_to_inject' ? The size of the driftfix-related state is 28 bytes per timer and we have 32 timers per HPETState, i.e. 896 additional bytes per HPETState. With a maximum number of 8 HPET blocks (HPETState), this amounts to 7168 bytes. Hence, unconditional migration of the driftfix-related state should not cause significant additional overhead. It's not about overhead. Maybe I missed something. Could you please explain which benefit you see in using a subsection ? In the common case of there being no drift, you can migrate from a qemu that supports driftfix to a qemu that doesn't. Right, subsections are a trick. The idea is that when you introduce new state for a device model that is not always going to be set, when you do the migration, you detect whether the state is set or not and if it's not set, instead of sending empty versions of that state (i.e. missed_ticks=0) you just don't send the new state at all. This means that you can migrate to an older version of QEMU provided the migration would work correctly. Using subsections and testing for hpet option being disabled vs enabled, is fine. But checking for the existence of drift, like you suggested (or at least how I understood you), is very tricky. It is expected to change many times during guest lifetime, and would make our migration predictability something Heisenberg would be proud of.
[Qemu-devel] Breaking out virtfs as a standalone server?
Right now, there's no decent userspace server for the 9p filesystem that I can find. (In part because the 9P2000.L spec is an undocumented work in progress.) The only up-to-date server seems to be virtfs in qemu, which has no TCP transport layer. Are there any plans to: A) Add a TCP transport layer so we can test with something we can intercept/examine/log/redirect with netcat and such? B) Break the 9p server out so it could be built as a standalone userspace program? Rob
Re: [Qemu-devel] [PATCH 0/5] atapi: Implement 'media' subcommand for GESN
Amit Shah amit.s...@redhat.com writes: On (Fri) 08 Apr 2011 [11:39:26], Markus Armbruster wrote: Results of quick test run now, patch review to follow. Test uses a simple program to try ioctl CDROM_DRIVE_STATUS (attached). ... Test in guest without your patches: [start with empty drive] # ./drive-status CDS_NO_DISC # eject /dev/sr0 # ./drive-status CDS_NO_DISC [incorrect, should be CDS_TRAY_OPEN] # eject -t /dev/sr0 # ./drive-status CDS_NO_DISC [insert media with monitor command change] # ./drive-status CDS_DISC_OK # eject /dev/sr0 # ./drive-status CDS_NO_DISC [incorrect, should be CDS_TRAY_OPEN] # eject -t /dev/sr0 # ./drive-status CDS_DISC_OK With the patches, it behaves as expected. Except something (guest kernel?) closes the tray right after eject if there's a medium in the open tray. Can you try with the two patches I sent on Saturday: atapi: Drives can be locked without media present atapi: Report correct errors on guest eject request My test cases show no further improvement.
Re: [Qemu-devel] [PATCH v2 3/5] hpet 'driftfix': add fields to HPETTimer and VMStateDescription
On 04/11/2011 04:39 PM, Glauber Costa wrote: On Mon, 2011-04-11 at 08:10 -0500, Anthony Liguori wrote: On 04/11/2011 04:08 AM, Avi Kivity wrote: On 04/11/2011 12:06 PM, Ulrich Obergfell wrote: vmstate_hpet_timer = { VMSTATE_UINT64(fsb, HPETTimer), VMSTATE_UINT64(period, HPETTimer), VMSTATE_UINT8(wrap_flag, HPETTimer), + VMSTATE_UINT64_V(saved_period, HPETTimer, 3), + VMSTATE_UINT64_V(ticks_not_accounted, HPETTimer, 3), + VMSTATE_UINT32_V(irqs_to_inject, HPETTimer, 3), + VMSTATE_UINT32_V(irq_rate, HPETTimer, 3), + VMSTATE_UINT32_V(divisor, HPETTimer, 3), We ought to be able to use a subsection keyed off of whether any ticks are currently accumulated, no? Anthony, I'm not sure if I understand your question correctly. Are you suggesting to migrate the driftfix-related state conditionally / only if there are any ticks accumulated in 'ticks_not_accounted' and 'irqs_to_inject' ? The size of the driftfix-related state is 28 bytes per timer and we have 32 timers per HPETState, i.e. 896 additional bytes per HPETState. With a maximum number of 8 HPET blocks (HPETState), this amounts to 7168 bytes. Hence, unconditional migration of the driftfix-related state should not cause significant additional overhead. It's not about overhead. Maybe I missed something. Could you please explain which benefit you see in using a subsection ? In the common case of there being no drift, you can migrate from a qemu that supports driftfix to a qemu that doesn't. Right, subsections are a trick. The idea is that when you introduce new state for a device model that is not always going to be set, when you do the migration, you detect whether the state is set or not and if it's not set, instead of sending empty versions of that state (i.e. missed_ticks=0) you just don't send the new state at all. This means that you can migrate to an older version of QEMU provided the migration would work correctly. Using subsections and testing for hpet option being disabled vs enabled, is fine. But checking for the existence of drift, like you suggested (or at least how I understood you), is very tricky. It is expected to change many times during guest lifetime, and would make our migration predictability something Heisenberg would be proud of. First, I'd expect no drift under normal circumstances, at least without overcommit. We may also allow a small amount of drift to pass migration (we lost time during the last phase anyway). Second, the problem only occurs on new-old migrations. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v2 3/5] hpet 'driftfix': add fields to HPETTimer and VMStateDescription
On Mon, 2011-04-11 at 08:47 -0500, Anthony Liguori wrote: On 04/11/2011 08:39 AM, Glauber Costa wrote: On Mon, 2011-04-11 at 08:10 -0500, Anthony Liguori wrote: On 04/11/2011 04:08 AM, Avi Kivity wrote: On 04/11/2011 12:06 PM, Ulrich Obergfell wrote: vmstate_hpet_timer = { VMSTATE_UINT64(fsb, HPETTimer), VMSTATE_UINT64(period, HPETTimer), VMSTATE_UINT8(wrap_flag, HPETTimer), + VMSTATE_UINT64_V(saved_period, HPETTimer, 3), + VMSTATE_UINT64_V(ticks_not_accounted, HPETTimer, 3), + VMSTATE_UINT32_V(irqs_to_inject, HPETTimer, 3), + VMSTATE_UINT32_V(irq_rate, HPETTimer, 3), + VMSTATE_UINT32_V(divisor, HPETTimer, 3), We ought to be able to use a subsection keyed off of whether any ticks are currently accumulated, no? Anthony, I'm not sure if I understand your question correctly. Are you suggesting to migrate the driftfix-related state conditionally / only if there are any ticks accumulated in 'ticks_not_accounted' and 'irqs_to_inject' ? The size of the driftfix-related state is 28 bytes per timer and we have 32 timers per HPETState, i.e. 896 additional bytes per HPETState. With a maximum number of 8 HPET blocks (HPETState), this amounts to 7168 bytes. Hence, unconditional migration of the driftfix-related state should not cause significant additional overhead. It's not about overhead. Maybe I missed something. Could you please explain which benefit you see in using a subsection ? In the common case of there being no drift, you can migrate from a qemu that supports driftfix to a qemu that doesn't. Right, subsections are a trick. The idea is that when you introduce new state for a device model that is not always going to be set, when you do the migration, you detect whether the state is set or not and if it's not set, instead of sending empty versions of that state (i.e. missed_ticks=0) you just don't send the new state at all. This means that you can migrate to an older version of QEMU provided the migration would work correctly. Using subsections and testing for hpet option being disabled vs enabled, is fine. But checking for the existence of drift, like you suggested (or at least how I understood you), is very tricky. It is expected to change many times during guest lifetime, and would make our migration predictability something Heisenberg would be proud of. Is this true? I would expect it to be very tied to workloads. For idle workloads, you should never have accumulated missed ticks whereas with heavy workloads, you always will have accumulated ticks. Is that not correct? Yes, it is , but we lose a lot of reliability by tying migration to the workload. Given that we still have to start qemu the same way both sides, we end up with a situation in which at time t, migration is possible, and at time t+1 migration is not. I'd rather have subsections enabled at all times when the option to allow driftfix is enabled.
[Qemu-devel] [PATCH] vmstate: fix varrays with uint32_t indexes
VARRAY_UINT32 only exists in vmstate_load_state(), but not in vmstate_save_state(). CC: Juan Quintela quint...@redhat.com Signed-off-by: Amos Kong ak...@redhat.com --- savevm.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/savevm.c b/savevm.c index 03fce62..09825e6 100644 --- a/savevm.c +++ b/savevm.c @@ -1393,6 +1393,8 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, n_elems = field-num; } else if (field-flags VMS_VARRAY_INT32) { n_elems = *(int32_t *)(opaque+field-num_offset); +} else if (field-flags VMS_VARRAY_UINT32) { +n_elems = *(uint32_t *)(opaque+field-num_offset); } else if (field-flags VMS_VARRAY_UINT16) { n_elems = *(uint16_t *)(opaque+field-num_offset); } else if (field-flags VMS_VARRAY_UINT8) {
Re: [Qemu-devel] [PATCH v2 3/5] hpet 'driftfix': add fields to HPETTimer and VMStateDescription
On 04/11/2011 08:39 AM, Glauber Costa wrote: On Mon, 2011-04-11 at 08:10 -0500, Anthony Liguori wrote: On 04/11/2011 04:08 AM, Avi Kivity wrote: On 04/11/2011 12:06 PM, Ulrich Obergfell wrote: vmstate_hpet_timer = { VMSTATE_UINT64(fsb, HPETTimer), VMSTATE_UINT64(period, HPETTimer), VMSTATE_UINT8(wrap_flag, HPETTimer), + VMSTATE_UINT64_V(saved_period, HPETTimer, 3), + VMSTATE_UINT64_V(ticks_not_accounted, HPETTimer, 3), + VMSTATE_UINT32_V(irqs_to_inject, HPETTimer, 3), + VMSTATE_UINT32_V(irq_rate, HPETTimer, 3), + VMSTATE_UINT32_V(divisor, HPETTimer, 3), We ought to be able to use a subsection keyed off of whether any ticks are currently accumulated, no? Anthony, I'm not sure if I understand your question correctly. Are you suggesting to migrate the driftfix-related state conditionally / only if there are any ticks accumulated in 'ticks_not_accounted' and 'irqs_to_inject' ? The size of the driftfix-related state is 28 bytes per timer and we have 32 timers per HPETState, i.e. 896 additional bytes per HPETState. With a maximum number of 8 HPET blocks (HPETState), this amounts to 7168 bytes. Hence, unconditional migration of the driftfix-related state should not cause significant additional overhead. It's not about overhead. Maybe I missed something. Could you please explain which benefit you see in using a subsection ? In the common case of there being no drift, you can migrate from a qemu that supports driftfix to a qemu that doesn't. Right, subsections are a trick. The idea is that when you introduce new state for a device model that is not always going to be set, when you do the migration, you detect whether the state is set or not and if it's not set, instead of sending empty versions of that state (i.e. missed_ticks=0) you just don't send the new state at all. This means that you can migrate to an older version of QEMU provided the migration would work correctly. Using subsections and testing for hpet option being disabled vs enabled, is fine. But checking for the existence of drift, like you suggested (or at least how I understood you), is very tricky. It is expected to change many times during guest lifetime, and would make our migration predictability something Heisenberg would be proud of. Is this true? I would expect it to be very tied to workloads. For idle workloads, you should never have accumulated missed ticks whereas with heavy workloads, you always will have accumulated ticks. Is that not correct? Regards, Anthony Liguori
[Qemu-devel] [PATCH 7/7] ide-core: allocate metadata storage for CFATA drives
Currently it's done by hw/ide/microdrive.c To simplify that part, move this initialization to ide core. Signed-off-by: Dmitry Eremin-Solenikov dbarysh...@gmail.com --- hw/ide/core.c |4 hw/ide/internal.h |2 ++ hw/ide/microdrive.c |6 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index c11d457..f6f7c04 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2556,6 +2556,10 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, return -1; } } +if (s-drive_kind == IDE_CFATA) { +s-mdata_size = CFA_METADATA_SIZE; +s-mdata_storage = qemu_mallocz(CFA_METADATA_SIZE); +} if (serial) { strncpy(s-drive_serial_str, serial, sizeof(s-drive_serial_str)); } else { diff --git a/hw/ide/internal.h b/hw/ide/internal.h index d533fb6..d1e0528 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -364,6 +364,8 @@ typedef struct IDEDMAOps IDEDMAOps; #define SMART_DISABLE 0xd9 #define SMART_STATUS 0xda +#define CFA_METADATA_SIZE 0x20 + typedef enum { IDE_HD, IDE_CD, IDE_CFATA } IDEDriveKind; typedef void EndTransferFunc(IDEState *); diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c index a251544..898ad15 100644 --- a/hw/ide/microdrive.c +++ b/hw/ide/microdrive.c @@ -35,8 +35,6 @@ /***/ /* CF-ATA Microdrive */ -#define METADATA_SIZE 0x20 - /* DSCM-1 Microdrive hard disk with CF+ II / PCMCIA interface. */ typedef struct { PCMCIACardState card; @@ -534,9 +532,6 @@ PCMCIACardState *dscm1_init(PCMCIASocket *socket, DriveInfo *bdrv) md = DO_UPCAST(MicroDriveState, card.dev, dev); ide_create_drive(md-bus, 0, bdrv); -md-bus.ifs[0].drive_kind = IDE_CFATA; -md-bus.ifs[0].mdata_size = METADATA_SIZE; -md-bus.ifs[0].mdata_storage = (uint8_t *) qemu_mallocz(METADATA_SIZE); return md-card; } @@ -553,6 +548,7 @@ static int dscm1_initfn(PCMCIACardState *state) qdev_init_gpio_in(state-dev, md_set_irq, 1); ide_init2(md-bus, qdev_get_gpio_in(state-dev, 0)); +md-bus.ifs[0].drive_kind = IDE_CFATA; return 0; } -- 1.7.4.1
[Qemu-devel] [PATCH 6/7] pxa: change order of pcmcia devices instantiation, so that the socket 0 will be default
Signed-off-by: Dmitry Eremin-Solenikov dbarysh...@gmail.com --- hw/pxa2xx.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c index ef4c0a2..bcb42cf 100644 --- a/hw/pxa2xx.c +++ b/hw/pxa2xx.c @@ -,8 +,8 @@ PXA2xxState *pxa270_init(unsigned int sdram_size, const char *revision) qdev_get_gpio_in(s-pic, PXA2XX_PIC_USBH1)); } -s-pcmcia[0] = pxa2xx_pcmcia_init(0x2000, 0); s-pcmcia[1] = pxa2xx_pcmcia_init(0x3000, 1); +s-pcmcia[0] = pxa2xx_pcmcia_init(0x2000, 0); sysbus_create_simple(pxa2xx_rtc, 0x4090, qdev_get_gpio_in(s-pic, PXA2XX_PIC_RTCALARM)); @@ -2358,8 +2358,8 @@ PXA2xxState *pxa255_init(unsigned int sdram_size) qdev_get_gpio_in(s-pic, PXA2XX_PIC_USBH1)); } -s-pcmcia[0] = pxa2xx_pcmcia_init(0x2000, 0); s-pcmcia[1] = pxa2xx_pcmcia_init(0x3000, 1); +s-pcmcia[0] = pxa2xx_pcmcia_init(0x2000, 0); sysbus_create_simple(pxa2xx_rtc, 0x4090, qdev_get_gpio_in(s-pic, PXA2XX_PIC_RTCALARM)); -- 1.7.4.1
[Qemu-devel] Re: [PATCH 1/2] atapi: Drives can be locked without media present
Am 09.04.2011 12:24, schrieb Amit Shah: Drivers are free to lock drives without any media present. Such a condition should not result in an error condition. See Table 341 in MMC-5 spec for details. Signed-off-by: Amit Shah amit.s...@redhat.com Thanks, applied both to the block branch. Kevin
[Qemu-devel] Re: [PATCH] target-sh4: get rid of CPU_{Float, Double}U
On Sun, Apr 10, 2011 at 09:13:05PM +0200, Aurelien Jarno wrote: SH4 is always using softfloat, so it's possible to have helpers directly taking float32 or float64 value. This allow to get rid of conversions through CPU_{Float,Double}U. Eh, I think this punning on i32/f32 and i64/f64 values is not healthy. But Peter's already said that the floats-as-structs bit of softfloat is broken, so maybe it's not worth trying to ensure floats-as-structs works (or even making it the default, to discourage people from bit-twiddling directly). -Nathan
[Qemu-devel] Re: [PATCH] target-sh4: get rid of CPU_{Float, Double}U
On 11 April 2011 15:55, Nathan Froyd froy...@codesourcery.com wrote: On Sun, Apr 10, 2011 at 09:13:05PM +0200, Aurelien Jarno wrote: SH4 is always using softfloat, so it's possible to have helpers directly taking float32 or float64 value. This allow to get rid of conversions through CPU_{Float,Double}U. Eh, I think this punning on i32/f32 and i64/f64 values is not healthy. But Peter's already said that the floats-as-structs bit of softfloat is broken, so maybe it's not worth trying to ensure floats-as-structs works (or even making it the default, to discourage people from bit-twiddling directly). I guess I should clarify that about the floats-as-structs thing. (1) It does compile cleanly for the ARM target. Some other targets don't compile because they're (buggily) not using the boxing/unboxing macros when they do bit-twiddling of floats; that should be fixed. (2) I think most of the value is in whether it compiles OK or not, rather than trying to actually run with it as a config (which I agree with Nathan is likely to go wrong if you have a host which doesn't pass 32/64 bit structs in registers). The compile test catches cases where the C code is doing bit-twiddling on float32s. (3) If we did say you shouldn't be passing 'float32' etc into helper functions, this would make the def-helper.h support for 'f32' and 'f64' a bit pointless because they could never be used (4) I think you should be able to write a helper function for an add as just float32 HELPER(my_float_add)(float32 a, float32 b) { return float32_add(a, b, status); } and having to add boxing/unboxing macros to this reduces clarity for no real gain. Using the macros should be a sign you're doing something wrong, not that you're doing it right :-) -- PMM
Re: [Qemu-devel] [PATCH 1/3] arm: basic support for ARMv4/ARMv4T emulation
Hi, On 4/10/11, Marek Vasut marek.va...@gmail.com wrote: On Monday 04 April 2011 15:38:44 Dmitry Eremin-Solenikov wrote: Currently target-arm/ assumes at least ARMv5 core. Add support for handling also ARMv4/ARMv4T. This changes the following instructions: BX(v4T and later) BKPT, BLX, CDP2, CLZ, LDC2, LDRD, MCRR, MCRR2, MRRC, MCRR, MRC2, MRRC, MRRC2, PLD QADD, QDADD, QDSUB, QSUB, STRD, SMLAxy, SMLALxy, SMLAWxy, SMULxy, SMULWxy, STC2 (v5 and later) All instructions that are v5TE and later are also bound to just v5, as that's how it was before. This patch doesn _not_ include disabling of cp15 access and base-updated data abort model (that will be required to emulate chips based on a ARM7TDMI), because: * no ARM7TDMI chips are currently emulated (or planned) Hi, this will come handy as I plan to add support for certain arm7tdmi chip (forward port at91sam7 patchset) Please be sure to check for the base-updated/base-restored part of the original patch as I'm nearly sure that it will be required for your chips. -- With best wishes Dmitry
[Qemu-devel] [PATCH 00/13] ARM: Handle UNDEF cases in Neon data processing insns
This extremely dull patch set cleans up the UNDEF handling in the Neon data processing instruction space, so that we UNDEF in all the cases where the architecture demands it, and do so early enough that we don't leak a TCG temporary in the process. It also fixes the handling of the one UNPREDICTABLE in the space, and does some minor cleanup that is possible once you know the UNDEF cases have been caught earlier. The patchset is inspired by a number of patches from the qemu-meego tree, although in most cases I ended up writing a more generic check rather than using Juha's patches directly. The order of patches matches the order in which disas_neon_data_insn() handles the various subcases. This has been tested via the usual random-instruction-sequence testing with a set of patterns that cover all of the Neon dp insn space. [This means that in the process I ended up also testing the actual functionality of all the Neon dp insns; I only found one bug, which I'll send a patch for in a moment.] Juha Riihimäki (2): target-arm: Simplify three-register pairwise code target-arm: Handle UNDEF cases for VDUP (scalar) Peter Maydell (11): target-arm: Use lookup table for size check on Neon 3-reg-same insns target-arm: Handle UNDEF cases for Neon 3-regs-same insns target-arm: Handle UNDEF cases for Neon 2 regs and shift insns target-arm: Collapse VSRI case into VSHL,VSLI target-arm: Handle UNDEF cases for Neon invalid modified-immediates target-arm: Handle UNDEF cases for Neon 3-regs-different-widths target-arm: Handle UNDEF cases for Neon 2 regs + scalar forms target-arm: Handle UNDEF cases for VEXT target-arm: Simplify checking of size field in Neon 2reg-misc forms target-arm: Handle UNDEF cases for Neon 2 register misc forms target-arm: Treat UNPREDICTABLE VTBL,VTBX case as UNDEF target-arm/translate.c | 698 +--- 1 files changed, 483 insertions(+), 215 deletions(-)
[Qemu-devel] [PATCH 05/13] target-arm: Collapse VSRI case into VSHL, VSLI
Collapse some switch cases for VSRI into those for VSHL, VSLI, since the bodies are the same. (This is not completely obvious for the size 3 case, but since for VSRI we know U=1 the GEN_NEON_INTEGER_OP() expansion is equivalent to the open-coded VSHL/VSLI case.) Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/translate.c |4 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index c0ffa9f..a86c54c 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -4813,8 +4813,6 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) gen_helper_neon_rshl_s64(cpu_V0, cpu_V0, cpu_V1); break; case 4: /* VSRI */ -gen_helper_neon_shl_u64(cpu_V0, cpu_V0, cpu_V1); -break; case 5: /* VSHL, VSLI */ gen_helper_neon_shl_u64(cpu_V0, cpu_V0, cpu_V1); break; @@ -4867,8 +4865,6 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) GEN_NEON_INTEGER_OP(rshl); break; case 4: /* VSRI */ -GEN_NEON_INTEGER_OP(shl); -break; case 5: /* VSHL, VSLI */ switch (size) { case 0: gen_helper_neon_shl_u8(tmp, tmp, tmp2); break; -- 1.7.1
[Qemu-devel] [PATCH 03/13] target-arm: Simplify three-register pairwise code
From: Juha Riihimäki juha.riihim...@nokia.com Since we know that the case of (pairwise q) has been caught earlier, we can simplify the register setup code for each pass in the three-register-same-size Neon loop. Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com --- target-arm/translate.c | 19 --- 1 files changed, 8 insertions(+), 11 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 5ffbace..0cf933d 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -4328,7 +4328,6 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) int count; int pairwise; int u; -int n; uint32_t imm, mask; TCGv tmp, tmp2, tmp3, tmp4, tmp5; TCGv_i64 tmp64; @@ -4480,16 +4479,12 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) if (pairwise) { /* Pairwise. */ -if (q) -n = (pass 1) * 2; -else -n = 0; -if (pass q + 1) { -tmp = neon_load_reg(rn, n); -tmp2 = neon_load_reg(rn, n + 1); +if (pass 1) { +tmp = neon_load_reg(rn, 0); +tmp2 = neon_load_reg(rn, 1); } else { -tmp = neon_load_reg(rm, n); -tmp2 = neon_load_reg(rm, n + 1); +tmp = neon_load_reg(rm, 0); +tmp2 = neon_load_reg(rm, 1); } } else { /* Elementwise. */ @@ -5147,6 +5142,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) /* VMOV, VMVN. */ tmp = tcg_temp_new_i32(); if (op == 14 invert) { +int n; uint32_t val; val = 0; for (n = 0; n 4; n++) { @@ -5575,6 +5571,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) break; case 33: /* VTRN */ if (size == 2) { +int n; for (n = 0; n (q ? 4 : 2); n += 2) { tmp = neon_load_reg(rm, n); tmp2 = neon_load_reg(rd, n + 1); @@ -5866,7 +5863,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) } } else if ((insn (1 10)) == 0) { /* VTBL, VTBX. */ -n = ((insn 5) 0x18) + 8; +int n = ((insn 5) 0x18) + 8; if (insn (1 6)) { tmp = neon_load_reg(rd, 0); } else { -- 1.7.1
[Qemu-devel] [PATCH 08/13] target-arm: Handle UNDEF cases for Neon 2 regs + scalar forms
Add missing checks for cases which must UNDEF in the Neon 2 registers and a scalar data processing instruction space. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/translate.c | 37 +++-- 1 files changed, 27 insertions(+), 10 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 9ff5af0..15c2015 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -5368,16 +5368,29 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) } } } else { -/* Two registers and a scalar. */ +/* Two registers and a scalar. NB that for ops of this form + * the ARM ARM labels bit 24 as Q, but it is in our variable + * 'u', not 'q'. + */ +if (size == 0) { +return 1; +} switch (op) { -case 0: /* Integer VMLA scalar */ case 1: /* Float VMLA scalar */ -case 4: /* Integer VMLS scalar */ case 5: /* Floating point VMLS scalar */ -case 8: /* Integer VMUL scalar */ case 9: /* Floating point VMUL scalar */ +if (size == 1) { +return 1; +} +/* fall through */ +case 0: /* Integer VMLA scalar */ +case 4: /* Integer VMLS scalar */ +case 8: /* Integer VMUL scalar */ case 12: /* VQDMULH scalar */ case 13: /* VQRDMULH scalar */ +if (u ((rd | rn) 1)) { +return 1; +} tmp = neon_get_scalar(size, rm); neon_store_scratch(0, tmp); for (pass = 0; pass (u ? 4 : 2); pass++) { @@ -5402,7 +5415,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) case 0: gen_helper_neon_mul_u8(tmp, tmp, tmp2); break; case 1: gen_helper_neon_mul_u16(tmp, tmp, tmp2); break; case 2: tcg_gen_mul_i32(tmp, tmp, tmp2); break; -default: return 1; +default: abort(); } } tcg_temp_free_i32(tmp2); @@ -5430,15 +5443,19 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) neon_store_reg(rd, pass, tmp); } break; -case 2: /* VMLAL sclar */ case 3: /* VQDMLAL scalar */ -case 6: /* VMLSL scalar */ case 7: /* VQDMLSL scalar */ -case 10: /* VMULL scalar */ case 11: /* VQDMULL scalar */ -if (size == 0 (op == 3 || op == 7 || op == 11)) +if (u == 1) { return 1; - +} +/* fall through */ +case 2: /* VMLAL sclar */ +case 6: /* VMLSL scalar */ +case 10: /* VMULL scalar */ +if (rd 1) { +return 1; +} tmp2 = neon_get_scalar(size, rm); /* We need a copy of tmp2 because gen_neon_mull * deletes it during pass 0. */ -- 1.7.1
[Qemu-devel] [PATCH 11/13] target-arm: Handle UNDEF cases for Neon 2 register misc forms
Add missing UNDEF checks for Neon two register miscellaneous forms: * all instructions except VMOVN,VQMOVN must UNDEF if Q==1 (Vd0 == 1 || Vm0 == 1) * VMOVN,VQMOVN,VCVT.F16.F32 UNDEF if Q == 1 || Vm0 == 1 * VSHLL,VCVT.F32.F16 UNDEF if Q == 1 || Vd0 == 1 (The only other UNDEF case is VZIP,VUZP if Q == 0 size == 10, which we already handle.) Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/translate.c | 21 - 1 files changed, 16 insertions(+), 5 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 4728248..b647c7b 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -5677,6 +5677,10 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) if ((neon_2rm_sizes[op] (1 size)) == 0) { return 1; } +if ((op != NEON_2RM_VMOVN op != NEON_2RM_VQMOVN) +q ((rm | rd) 1)) { +return 1; +} switch (op) { case NEON_2RM_VREV64: for (pass = 0; pass (q ? 2 : 1); pass++) { @@ -5747,6 +5751,9 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) break; case NEON_2RM_VMOVN: case NEON_2RM_VQMOVN: /* also VQMOVUN; op field and mnemonics don't line up */ +if (rm 1) { +return 1; +} TCGV_UNUSED(tmp2); for (pass = 0; pass 2; pass++) { neon_load_reg64(cpu_V0, rm + pass); @@ -5762,7 +5769,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) } break; case NEON_2RM_VSHLL: -if (q) { +if (q || (rd 1)) { return 1; } tmp = neon_load_reg(rm, 0); @@ -5776,8 +5783,10 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) } break; case NEON_2RM_VCVT_F16_F32: -if (!arm_feature(env, ARM_FEATURE_VFP_FP16)) - return 1; +if (!arm_feature(env, ARM_FEATURE_VFP_FP16) || +q || (rm 1)) { +return 1; +} tmp = tcg_temp_new_i32(); tmp2 = tcg_temp_new_i32(); tcg_gen_ld_f32(cpu_F0s, cpu_env, neon_reg_offset(rm, 0)); @@ -5798,8 +5807,10 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) tcg_temp_free_i32(tmp); break; case NEON_2RM_VCVT_F32_F16: -if (!arm_feature(env, ARM_FEATURE_VFP_FP16)) - return 1; +if (!arm_feature(env, ARM_FEATURE_VFP_FP16) || +q || (rd 1)) { +return 1; +} tmp3 = tcg_temp_new_i32(); tmp = neon_load_reg(rm, 0); tmp2 = neon_load_reg(rm, 1); -- 1.7.1
[Qemu-devel] [PATCH 09/13] target-arm: Handle UNDEF cases for VEXT
VEXT must UNDEF if Q == 1 (Vd0 == 1 || Vr0 == 1 || Vm0 == 1) Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/translate.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 15c2015..f47e5ea 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -5514,6 +5514,10 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) if (imm 7 !q) return 1; +if (q ((rd | rn | rm) 1)) { +return 1; +} + if (imm == 0) { neon_load_reg64(cpu_V0, rn); if (q) { -- 1.7.1
[Qemu-devel] [PATCH 13/13] target-arm: Handle UNDEF cases for VDUP (scalar)
From: Juha Riihimäki juha.riihim...@nokia.com Handle the UNDEF cases for VDUP(scalar): imm4 == x000 Q == 1 Vd0 == 1 Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com --- target-arm/translate.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index be25c8f..6190028 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -6057,6 +6057,9 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) tcg_temp_free_i32(tmp); } else if ((insn 0x380) == 0) { /* VDUP */ +if ((insn (7 16)) == 0 || (q (rd 1))) { +return 1; +} if (insn (1 19)) { tmp = neon_load_reg(rm, 1); } else { -- 1.7.1
[Qemu-devel] [PATCH 10/13] target-arm: Simplify checking of size field in Neon 2reg-misc forms
Many of the Neon 2 register misc instruction forms require invalid size fields to cause the instruction to UNDEF. Pull this information out into an array; this simplifies the code and also means we can do the check early and avoid the problem of leaking TCG temporaries in the illegal_op case. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/translate.c | 255 +-- 1 files changed, 179 insertions(+), 76 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index f47e5ea..4728248 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -3662,7 +3662,7 @@ static inline TCGv neon_get_scalar(int size, int reg) static int gen_neon_unzip(int rd, int rm, int size, int q) { TCGv tmp, tmp2; -if (size == 3 || (!q size == 2)) { +if (!q size == 2) { return 1; } tmp = tcg_const_i32(rd); @@ -3701,7 +3701,7 @@ static int gen_neon_unzip(int rd, int rm, int size, int q) static int gen_neon_zip(int rd, int rm, int size, int q) { TCGv tmp, tmp2; -if (size == 3 || (!q size == 2)) { +if (!q size == 2) { return 1; } tmp = tcg_const_i32(rd); @@ -4312,6 +4312,113 @@ static const uint8_t neon_3r_sizes[] = { [NEON_3R_VRECPS_VRSQRTS] = 0x5, /* size bit 1 encodes op */ }; +/* Symbolic constants for op fields for Neon 2-register miscellaneous. + * The values correspond to bits [17:16,10:7]; see the ARM ARM DDI0406B + * table A7-13. + */ +#define NEON_2RM_VREV64 0 +#define NEON_2RM_VREV32 1 +#define NEON_2RM_VREV16 2 +#define NEON_2RM_VPADDL 4 +#define NEON_2RM_VPADDL_U 5 +#define NEON_2RM_VCLS 8 +#define NEON_2RM_VCLZ 9 +#define NEON_2RM_VCNT 10 +#define NEON_2RM_VMVN 11 +#define NEON_2RM_VPADAL 12 +#define NEON_2RM_VPADAL_U 13 +#define NEON_2RM_VQABS 14 +#define NEON_2RM_VQNEG 15 +#define NEON_2RM_VCGT0 16 +#define NEON_2RM_VCGE0 17 +#define NEON_2RM_VCEQ0 18 +#define NEON_2RM_VCLE0 19 +#define NEON_2RM_VCLT0 20 +#define NEON_2RM_VABS 22 +#define NEON_2RM_VNEG 23 +#define NEON_2RM_VCGT0_F 24 +#define NEON_2RM_VCGE0_F 25 +#define NEON_2RM_VCEQ0_F 26 +#define NEON_2RM_VCLE0_F 27 +#define NEON_2RM_VCLT0_F 28 +#define NEON_2RM_VABS_F 30 +#define NEON_2RM_VNEG_F 31 +#define NEON_2RM_VSWP 32 +#define NEON_2RM_VTRN 33 +#define NEON_2RM_VUZP 34 +#define NEON_2RM_VZIP 35 +#define NEON_2RM_VMOVN 36 /* Includes VQMOVN, VQMOVUN */ +#define NEON_2RM_VQMOVN 37 /* Includes VQMOVUN */ +#define NEON_2RM_VSHLL 38 +#define NEON_2RM_VCVT_F16_F32 44 +#define NEON_2RM_VCVT_F32_F16 46 +#define NEON_2RM_VRECPE 56 +#define NEON_2RM_VRSQRTE 57 +#define NEON_2RM_VRECPE_F 58 +#define NEON_2RM_VRSQRTE_F 59 +#define NEON_2RM_VCVT_FS 60 +#define NEON_2RM_VCVT_FU 61 +#define NEON_2RM_VCVT_SF 62 +#define NEON_2RM_VCVT_UF 63 + +static int neon_2rm_is_float_op(int op) +{ +/* Return true if this neon 2reg-misc op is float-to-float */ +return (op == NEON_2RM_VABS_F || op == NEON_2RM_VNEG_F || +op = NEON_2RM_VRECPE_F); +} + +/* Each entry in this array has bit n set if the insn allows + * size value n (otherwise it will UNDEF). Since unallocated + * op values will have no bits set they always UNDEF. + */ +static const uint8_t neon_2rm_sizes[] = { +[NEON_2RM_VREV64] = 0x7, +[NEON_2RM_VREV32] = 0x3, +[NEON_2RM_VREV16] = 0x1, +[NEON_2RM_VPADDL] = 0x7, +[NEON_2RM_VPADDL_U] = 0x7, +[NEON_2RM_VCLS] = 0x7, +[NEON_2RM_VCLZ] = 0x7, +[NEON_2RM_VCNT] = 0x1, +[NEON_2RM_VMVN] = 0x1, +[NEON_2RM_VPADAL] = 0x7, +[NEON_2RM_VPADAL_U] = 0x7, +[NEON_2RM_VQABS] = 0x7, +[NEON_2RM_VQNEG] = 0x7, +[NEON_2RM_VCGT0] = 0x7, +[NEON_2RM_VCGE0] = 0x7, +[NEON_2RM_VCEQ0] = 0x7, +[NEON_2RM_VCLE0] = 0x7, +[NEON_2RM_VCLT0] = 0x7, +[NEON_2RM_VABS] = 0x7, +[NEON_2RM_VNEG] = 0x7, +[NEON_2RM_VCGT0_F] = 0x4, +[NEON_2RM_VCGE0_F] = 0x4, +[NEON_2RM_VCEQ0_F] = 0x4, +[NEON_2RM_VCLE0_F] = 0x4, +[NEON_2RM_VCLT0_F] = 0x4, +[NEON_2RM_VABS_F] = 0x4, +[NEON_2RM_VNEG_F] = 0x4, +[NEON_2RM_VSWP] = 0x1, +[NEON_2RM_VTRN] = 0x7, +[NEON_2RM_VUZP] = 0x7, +[NEON_2RM_VZIP] = 0x7, +[NEON_2RM_VMOVN] = 0x7, +[NEON_2RM_VQMOVN] = 0x7, +[NEON_2RM_VSHLL] = 0x7, +[NEON_2RM_VCVT_F16_F32] = 0x2, +[NEON_2RM_VCVT_F32_F16] = 0x2, +[NEON_2RM_VRECPE] = 0x4, +[NEON_2RM_VRSQRTE] = 0x4, +[NEON_2RM_VRECPE_F] = 0x4, +[NEON_2RM_VRSQRTE_F] = 0x4, +[NEON_2RM_VCVT_FS] = 0x4, +[NEON_2RM_VCVT_FU] = 0x4, +[NEON_2RM_VCVT_SF] = 0x4, +[NEON_2RM_VCVT_UF] = 0x4, +}; + /* Translate a NEON data processing instruction. Return nonzero if the instruction is invalid. We process data in a mixture of 32-bit and 64-bit chunks. @@ -5566,10 +5673,12 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) /* Two register misc. */ op = ((insn 12) 0x30) | ((insn 7) 0xf); size = (insn 18) 3; +/* UNDEF
[Qemu-devel] [PATCH 12/13] target-arm: Treat UNPREDICTABLE VTBL, VTBX case as UNDEF
Catch the UNPREDICTABLE case for Neon VTBL,VTBX, and UNDEF it rather than allowing the helper function to index off the end of the register file. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/translate.c |9 - 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index b647c7b..be25c8f 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -6023,7 +6023,14 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) } } else if ((insn (1 10)) == 0) { /* VTBL, VTBX. */ -int n = ((insn 5) 0x18) + 8; +int n = ((insn 8) 3) + 1; +if ((rn + n) 32) { +/* This is UNPREDICTABLE; we choose to UNDEF to avoid the + * helper function running off the end of the register file. + */ +return 1; +} +n = 3; if (insn (1 6)) { tmp = neon_load_reg(rd, 0); } else { -- 1.7.1
[Qemu-devel] [PATCH 02/13] target-arm: Handle UNDEF cases for Neon 3-regs-same insns
Correct the handling of UNDEF cases for the NEON 3 registers same size forms, by adding missing checks and rationalising some others so they are done early enough to avoid leaking TCG temporaries. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/translate.c | 54 ++- 1 files changed, 43 insertions(+), 11 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 3fa27e1..5ffbace 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -4348,6 +4348,12 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) if ((neon_3r_sizes[op] (1 size)) == 0) { return 1; } +/* All insns of this form UNDEF for either this condition or the + * superset of cases Q==1; we catch the latter later. + */ +if (q ((rd | rn | rm) 1)) { +return 1; +} if (size == 3 op != NEON_3R_LOGIC) { /* 64-bit element instructions. */ for (pass = 0; pass (q ? 2 : 1); pass++) { @@ -4410,6 +4416,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) } return 0; } +pairwise = 0; switch (op) { case NEON_3R_VSHL: case NEON_3R_VQSHL: @@ -4421,25 +4428,54 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) rtmp = rn; rn = rm; rm = rtmp; -pairwise = 0; } break; +case NEON_3R_VPADD: +if (u) { +return 1; +} +/* Fall through */ case NEON_3R_VPMAX: case NEON_3R_VPMIN: -case NEON_3R_VPADD: pairwise = 1; break; -case NEON_3R_FLOAT_ARITH: /* VADD, VSUB, VPADD, VABD (float) */ -pairwise = (u size 2); +case NEON_3R_FLOAT_ARITH: +pairwise = (u size 2); /* if VPADD (float) */ +break; +case NEON_3R_FLOAT_MINMAX: +pairwise = u; /* if VPMIN/VPMAX (float) */ +break; +case NEON_3R_FLOAT_CMP: +if (!u size) { +/* no encoding for U=0 C=1x */ +return 1; +} +break; +case NEON_3R_FLOAT_ACMP: +if (!u) { +return 1; +} +break; +case NEON_3R_VRECPS_VRSQRTS: +if (u) { +return 1; +} break; -case NEON_3R_FLOAT_MINMAX: /* VPMIN/VPMAX (float) */ -pairwise = u; +case NEON_3R_VMUL: +if (u (size != 0)) { +/* UNDEF on invalid size for polynomial subcase */ +return 1; +} break; default: -pairwise = 0; break; } +if (pairwise q) { +/* All the pairwise insns UNDEF if Q is set */ +return 1; +} + for (pass = 0; pass (q ? 4 : 2); pass++) { if (pairwise) { @@ -4621,8 +4657,6 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) } break; case NEON_3R_VPADD: -if (u) -return 1; switch (size) { case 0: gen_helper_neon_padd_u8(tmp, tmp, tmp2); break; case 1: gen_helper_neon_padd_u16(tmp, tmp, tmp2); break; @@ -4671,8 +4705,6 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) } break; case NEON_3R_FLOAT_ACMP: -if (!u) -return 1; if (size == 0) gen_helper_neon_acge_f32(tmp, tmp, tmp2); else -- 1.7.1
[Qemu-devel] [PATCH 01/13] target-arm: Use lookup table for size check on Neon 3-reg-same insns
Simplify the checks for invalid size values for the Neon three registers of the same size instruction forms (and add them where they were missing) by using a lookup table. This includes adding symbolic constants for the op values in this space, since we now use them in multiple places. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/translate.c | 197 1 files changed, 133 insertions(+), 64 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 998cfd5..3fa27e1 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -3558,15 +3558,14 @@ static void gen_nop_hint(DisasContext *s, int val) #define CPU_V001 cpu_V0, cpu_V0, cpu_V1 -static inline int gen_neon_add(int size, TCGv t0, TCGv t1) +static inline void gen_neon_add(int size, TCGv t0, TCGv t1) { switch (size) { case 0: gen_helper_neon_add_u8(t0, t0, t1); break; case 1: gen_helper_neon_add_u16(t0, t0, t1); break; case 2: tcg_gen_add_i32(t0, t0, t1); break; -default: return 1; +default: abort(); } -return 0; } static inline void gen_neon_rsb(int size, TCGv t0, TCGv t1) @@ -4245,6 +4244,74 @@ static void gen_neon_narrow_op(int op, int u, int size, TCGv dest, TCGv_i64 src) } } +/* Symbolic constants for op fields for Neon 3-register same-length. + * The values correspond to bits [11:8,4]; see the ARM ARM DDI0406B + * table A7-9. + */ +#define NEON_3R_VHADD 0 +#define NEON_3R_VQADD 1 +#define NEON_3R_VRHADD 2 +#define NEON_3R_LOGIC 3 /* VAND,VBIC,VORR,VMOV,VORN,VEOR,VBIF,VBIT,VBSL */ +#define NEON_3R_VHSUB 4 +#define NEON_3R_VQSUB 5 +#define NEON_3R_VCGT 6 +#define NEON_3R_VCGE 7 +#define NEON_3R_VSHL 8 +#define NEON_3R_VQSHL 9 +#define NEON_3R_VRSHL 10 +#define NEON_3R_VQRSHL 11 +#define NEON_3R_VMAX 12 +#define NEON_3R_VMIN 13 +#define NEON_3R_VABD 14 +#define NEON_3R_VABA 15 +#define NEON_3R_VADD_VSUB 16 +#define NEON_3R_VTST_VCEQ 17 +#define NEON_3R_VML 18 /* VMLA, VMLAL, VMLS, VMLSL */ +#define NEON_3R_VMUL 19 +#define NEON_3R_VPMAX 20 +#define NEON_3R_VPMIN 21 +#define NEON_3R_VQDMULH_VQRDMULH 22 +#define NEON_3R_VPADD 23 +#define NEON_3R_FLOAT_ARITH 26 /* float VADD, VSUB, VPADD, VABD */ +#define NEON_3R_FLOAT_MULTIPLY 27 /* float VMLA, VMLS, VMUL */ +#define NEON_3R_FLOAT_CMP 28 /* float VCEQ, VCGE, VCGT */ +#define NEON_3R_FLOAT_ACMP 29 /* float VACGE, VACGT, VACLE, VACLT */ +#define NEON_3R_FLOAT_MINMAX 30 /* float VMIN, VMAX */ +#define NEON_3R_VRECPS_VRSQRTS 31 /* float VRECPS, VRSQRTS */ + +static const uint8_t neon_3r_sizes[] = { +[NEON_3R_VHADD] = 0x7, +[NEON_3R_VQADD] = 0xf, +[NEON_3R_VRHADD] = 0x7, +[NEON_3R_LOGIC] = 0xf, /* size field encodes op type */ +[NEON_3R_VHSUB] = 0x7, +[NEON_3R_VQSUB] = 0xf, +[NEON_3R_VCGT] = 0x7, +[NEON_3R_VCGE] = 0x7, +[NEON_3R_VSHL] = 0xf, +[NEON_3R_VQSHL] = 0xf, +[NEON_3R_VRSHL] = 0xf, +[NEON_3R_VQRSHL] = 0xf, +[NEON_3R_VMAX] = 0x7, +[NEON_3R_VMIN] = 0x7, +[NEON_3R_VABD] = 0x7, +[NEON_3R_VABA] = 0x7, +[NEON_3R_VADD_VSUB] = 0xf, +[NEON_3R_VTST_VCEQ] = 0x7, +[NEON_3R_VML] = 0x7, +[NEON_3R_VMUL] = 0x7, +[NEON_3R_VPMAX] = 0x7, +[NEON_3R_VPMIN] = 0x7, +[NEON_3R_VQDMULH_VQRDMULH] = 0x6, +[NEON_3R_VPADD] = 0x7, +[NEON_3R_FLOAT_ARITH] = 0x5, /* size bit 1 encodes op */ +[NEON_3R_FLOAT_MULTIPLY] = 0x5, /* size bit 1 encodes op */ +[NEON_3R_FLOAT_CMP] = 0x5, /* size bit 1 encodes op */ +[NEON_3R_FLOAT_ACMP] = 0x5, /* size bit 1 encodes op */ +[NEON_3R_FLOAT_MINMAX] = 0x5, /* size bit 1 encodes op */ +[NEON_3R_VRECPS_VRSQRTS] = 0x5, /* size bit 1 encodes op */ +}; + /* Translate a NEON data processing instruction. Return nonzero if the instruction is invalid. We process data in a mixture of 32-bit and 64-bit chunks. @@ -4277,56 +4344,59 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) if ((insn (1 23)) == 0) { /* Three register same length. */ op = ((insn 7) 0x1e) | ((insn 4) 1); -if (size == 3 (op == 1 || op == 5 || op == 8 || op == 9 - || op == 10 || op == 11 || op == 16)) { -/* 64-bit element instructions. */ +/* Catch invalid op and bad size combinations: UNDEF */ +if ((neon_3r_sizes[op] (1 size)) == 0) { +return 1; +} +if (size == 3 op != NEON_3R_LOGIC) { +/* 64-bit element instructions. */ for (pass = 0; pass (q ? 2 : 1); pass++) { neon_load_reg64(cpu_V0, rn + pass); neon_load_reg64(cpu_V1, rm + pass); switch (op) { -case 1: /* VQADD */ +case NEON_3R_VQADD: if (u) { gen_helper_neon_qadd_u64(cpu_V0, cpu_V0, cpu_V1); } else { gen_helper_neon_qadd_s64(cpu_V0, cpu_V0,
[Qemu-devel] [PATCH 04/13] target-arm: Handle UNDEF cases for Neon 2 regs and shift insns
Correctly handle all the UNDEF cases for Neon instructions of the 2 registers and shift form, and make sure that we check for these cases early enough not to leak TCG temporaries. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/translate.c | 41 ++--- 1 files changed, 22 insertions(+), 19 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 0cf933d..c0ffa9f 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -4744,7 +4744,10 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) /* Two registers and shift. */ op = (insn 8) 0xf; if (insn (1 7)) { -/* 64-bit shift. */ +/* 64-bit shift. */ +if (op 7) { +return 1; +} size = 3; } else { size = 2; @@ -4757,6 +4760,12 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) if (op 8) { /* Shift by immediate: VSHR, VSRA, VRSHR, VRSRA, VSRI, VSHL, VQSHL, VQSHLU. */ +if (q ((rd | rm) 1)) { +return 1; +} +if (!u (op == 4 || op == 6)) { +return 1; +} /* Right shifts are encoded as N - shift, where N is the element size in bits. */ if (op = 4) @@ -4804,20 +4813,13 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) gen_helper_neon_rshl_s64(cpu_V0, cpu_V0, cpu_V1); break; case 4: /* VSRI */ -if (!u) -return 1; gen_helper_neon_shl_u64(cpu_V0, cpu_V0, cpu_V1); break; case 5: /* VSHL, VSLI */ gen_helper_neon_shl_u64(cpu_V0, cpu_V0, cpu_V1); break; case 6: /* VQSHLU */ -if (u) { -gen_helper_neon_qshlu_s64(cpu_V0, - cpu_V0, cpu_V1); -} else { -return 1; -} +gen_helper_neon_qshlu_s64(cpu_V0, cpu_V0, cpu_V1); break; case 7: /* VQSHL */ if (u) { @@ -4865,8 +4867,6 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) GEN_NEON_INTEGER_OP(rshl); break; case 4: /* VSRI */ -if (!u) -return 1; GEN_NEON_INTEGER_OP(shl); break; case 5: /* VSHL, VSLI */ @@ -4874,13 +4874,10 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) case 0: gen_helper_neon_shl_u8(tmp, tmp, tmp2); break; case 1: gen_helper_neon_shl_u16(tmp, tmp, tmp2); break; case 2: gen_helper_neon_shl_u32(tmp, tmp, tmp2); break; -default: return 1; +default: abort(); } break; case 6: /* VQSHLU */ -if (!u) { -return 1; -} switch (size) { case 0: gen_helper_neon_qshlu_s8(tmp, tmp, tmp2); @@ -4892,7 +4889,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) gen_helper_neon_qshlu_s32(tmp, tmp, tmp2); break; default: -return 1; +abort(); } break; case 7: /* VQSHL */ @@ -4950,7 +4947,9 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) /* Shift by immediate and narrow: VSHRN, VRSHRN, VQSHRN, VQRSHRN. */ int input_unsigned = (op == 8) ? !u : u; - +if (rm 1) { +return 1; +} shift = shift - (1 (size + 3)); size++; if (size == 3) { @@ -5018,9 +5017,10 @@ static int disas_neon_data_insn(CPUState * env,
[Qemu-devel] [PATCH 07/13] target-arm: Handle UNDEF cases for Neon 3-regs-different-widths
Add missing UNDEF checks for instructions in the Neon 3 registers of different widths data processing space. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/translate.c | 56 ++- 1 files changed, 36 insertions(+), 20 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index 0a9b3cf..9ff5af0 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -5174,31 +5174,47 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) int src1_wide; int src2_wide; int prewiden; -/* prewiden, src1_wide, src2_wide */ -static const int neon_3reg_wide[16][3] = { -{1, 0, 0}, /* VADDL */ -{1, 1, 0}, /* VADDW */ -{1, 0, 0}, /* VSUBL */ -{1, 1, 0}, /* VSUBW */ -{0, 1, 1}, /* VADDHN */ -{0, 0, 0}, /* VABAL */ -{0, 1, 1}, /* VSUBHN */ -{0, 0, 0}, /* VABDL */ -{0, 0, 0}, /* VMLAL */ -{0, 0, 0}, /* VQDMLAL */ -{0, 0, 0}, /* VMLSL */ -{0, 0, 0}, /* VQDMLSL */ -{0, 0, 0}, /* Integer VMULL */ -{0, 0, 0}, /* VQDMULL */ -{0, 0, 0} /* Polynomial VMULL */ +/* undefreq: bit 0 : UNDEF if size != 0 + * bit 1 : UNDEF if size == 0 + * bit 2 : UNDEF if U == 1 + * Note that [1:0] set implies 'always UNDEF' + */ +int undefreq; +/* prewiden, src1_wide, src2_wide, undefreq */ +static const int neon_3reg_wide[16][4] = { +{1, 0, 0, 0}, /* VADDL */ +{1, 1, 0, 0}, /* VADDW */ +{1, 0, 0, 0}, /* VSUBL */ +{1, 1, 0, 0}, /* VSUBW */ +{0, 1, 1, 0}, /* VADDHN */ +{0, 0, 0, 0}, /* VABAL */ +{0, 1, 1, 0}, /* VSUBHN */ +{0, 0, 0, 0}, /* VABDL */ +{0, 0, 0, 0}, /* VMLAL */ +{0, 0, 0, 6}, /* VQDMLAL */ +{0, 0, 0, 0}, /* VMLSL */ +{0, 0, 0, 6}, /* VQDMLSL */ +{0, 0, 0, 0}, /* Integer VMULL */ +{0, 0, 0, 2}, /* VQDMULL */ +{0, 0, 0, 5}, /* Polynomial VMULL */ +{0, 0, 0, 3}, /* Reserved: always UNDEF */ }; prewiden = neon_3reg_wide[op][0]; src1_wide = neon_3reg_wide[op][1]; src2_wide = neon_3reg_wide[op][2]; +undefreq = neon_3reg_wide[op][3]; -if (size == 0 (op == 9 || op == 11 || op == 13)) +if (((undefreq 1) (size != 0)) || +((undefreq 2) (size == 0)) || +((undefreq 4) u)) { +return 1; +} +if ((src1_wide (rn 1)) || +(src2_wide (rm 1)) || +(!src2_wide (rd 1))) { return 1; +} /* Avoid overlapping operands. Wide source operands are always aligned so will never overlap with wide @@ -5279,8 +5295,8 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) tcg_temp_free_i32(tmp2); tcg_temp_free_i32(tmp); break; -default: /* 15 is RESERVED. */ -return 1; +default: /* 15 is RESERVED: caught earlier */ +abort(); } if (op == 13) { /* VQDMULL */ -- 1.7.1
[Qemu-devel] [PATCH 06/13] target-arm: Handle UNDEF cases for Neon invalid modified-immediates
For Neon one register and a modified immediate value forms, the combination op=1 cmode= is unallocated and should UNDEF. All instructions of this form also UNDEF if Q == 1 and Vd0 == 1. We also add a comment on the only UNPREDICTABLE in this space. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/translate.c | 10 ++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index a86c54c..0a9b3cf 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -5084,11 +5084,18 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) } } else { /* (insn 0x00380080) == 0 */ int invert; +if (q (rd 1)) { +return 1; +} op = (insn 8) 0xf; /* One register and immediate. */ imm = (u 7) | ((insn 12) 0x70) | (insn 0xf); invert = (insn (1 5)) != 0; +/* Note that op = 2,3,4,5,6,7,10,11,12,13 imm=0 is UNPREDICTABLE. + * We choose to not special-case this and will behave as if a + * valid constant encoding of 0 had been given. + */ switch (op) { case 0: case 1: /* no-op */ @@ -5120,6 +5127,9 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn) imm = ~imm; break; case 15: +if (invert) { +return 1; +} imm = ((imm 0x80) 24) | ((imm 0x3f) 19) | ((imm 0x40) ? (0x1f 25) : (1 30)); break; -- 1.7.1
Re: [Qemu-devel] Re: [PATCH] target-sh4: get rid of CPU_{Float, Double}U
On 11 April 2011 16:19, Richard Henderson r...@twiddle.net wrote: On 04/11/2011 08:09 AM, Peter Maydell wrote: (4) I think you should be able to write a helper function for an add as just float32 HELPER(my_float_add)(float32 a, float32 b) { return float32_add(a, b, status); } While this is a laudable goal, this will fail for hosts that pass all structures by reference. This is true of, e.g. PPC32. ...but only if float32 is a struct, which is where we came in. In the sane default configuration float32 is just a uint32_t in disguise. In other words, my point is that I'd prefer to give up[*] being able to run with float32-is-a-struct rather than give up having clean and straightforward helper functions. [*] actually I suspect we've never actually had this capability so we're not really giving anything up except philosophically... -- PMM
Re: [Qemu-devel] [PATCH 13/13] target-arm: Handle UNDEF cases for VDUP (scalar)
On 11 April 2011 16:26, Peter Maydell peter.mayd...@linaro.org wrote: From: Juha Riihimäki juha.riihim...@nokia.com Handle the UNDEF cases for VDUP(scalar): imm4 == x000 Q == 1 Vd0 == 1 Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com Reviewed-by: Peter Maydell peter.mayd...@linaro.org (obviously... but I forgot to put that into the commit message.) -- PMM
Re: [Qemu-devel] [PATCH 03/13] target-arm: Simplify three-register pairwise code
On 11 April 2011 16:26, Peter Maydell peter.mayd...@linaro.org wrote: From: Juha Riihimäki juha.riihim...@nokia.com Since we know that the case of (pairwise q) has been caught earlier, we can simplify the register setup code for each pass in the three-register-same-size Neon loop. Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM
[Qemu-devel] [Bug 757654] Re: UHCI fails to signal stall response
** Patch added: report error on stall response https://bugs.launchpad.net/bugs/757654/+attachment/2017520/+files/usb_stall.patch -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/757654 Title: UHCI fails to signal stall response Status in QEMU: New Bug description: When TD execution results in STALL error (STALL handshake, no stall as a result of err count reaching 0), there is no way to know about it except for checking that TD. IMO it is an error condition and it should be reflected in the status register (and issue an interrupt if enabled). Ways to replicate: Send a query that is answered by stall (like set_idle request to a mouse) Expected behavior: UHCI hc sets status bit 1 (usb error interrupt) and issues an interrupt current behavior: Neither status bit is set nor interrupt triggered Version 0.14 attached patch for current master (quick fix, it might be I got something wrong)
[Qemu-devel] [Bug 757654] [NEW] UHCI fails to signal stall response
Public bug reported: When TD execution results in STALL error (STALL handshake, no stall as a result of err count reaching 0), there is no way to know about it except for checking that TD. IMO it is an error condition and it should be reflected in the status register (and issue an interrupt if enabled). Ways to replicate: Send a query that is answered by stall (like set_idle request to a mouse) Expected behavior: UHCI hc sets status bit 1 (usb error interrupt) and issues an interrupt current behavior: Neither status bit is set nor interrupt triggered Version 0.14 attached patch for current master (quick fix, it might be I got something wrong) ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/757654 Title: UHCI fails to signal stall response Status in QEMU: New Bug description: When TD execution results in STALL error (STALL handshake, no stall as a result of err count reaching 0), there is no way to know about it except for checking that TD. IMO it is an error condition and it should be reflected in the status register (and issue an interrupt if enabled). Ways to replicate: Send a query that is answered by stall (like set_idle request to a mouse) Expected behavior: UHCI hc sets status bit 1 (usb error interrupt) and issues an interrupt current behavior: Neither status bit is set nor interrupt triggered Version 0.14 attached patch for current master (quick fix, it might be I got something wrong)
[Qemu-devel] Re: [PATCH] target-sh4: get rid of CPU_{Float, Double}U
On Mon, Apr 11, 2011 at 04:09:53PM +0100, Peter Maydell wrote: On 11 April 2011 15:55, Nathan Froyd froy...@codesourcery.com wrote: On Sun, Apr 10, 2011 at 09:13:05PM +0200, Aurelien Jarno wrote: SH4 is always using softfloat, so it's possible to have helpers directly taking float32 or float64 value. This allow to get rid of conversions through CPU_{Float,Double}U. Eh, I think this punning on i32/f32 and i64/f64 values is not healthy. But Peter's already said that the floats-as-structs bit of softfloat is broken, so maybe it's not worth trying to ensure floats-as-structs works (or even making it the default, to discourage people from bit-twiddling directly). I guess I should clarify that about the floats-as-structs thing. I think I should ask some more details about this floats-as-structs history. It has been added when a lot of targets were still using softfloat-native and it was still possible to think about being able to switch from one to another (at least having this goal). The idea was to avoid doing float computation on the wrong value. For example this code does work with softfloat-native, but will produce wrong results with softfloat: float64 fadd(float64 a, float64 b) { return a + b; } The problem with this kind of code is that it compiles both with softfloat and softfloat-native, but gives wrong results on softfloat. By adding this floats-as-structs, it was possible to catch such errors during compile time. However now that most targets (except i386) default to softfloat, we don't really need to forbid bit twiddling in target code. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] [PATCH] target-arm: Don't overflow when calculating value for signed VABAL
In the VABAL instruction we take the absolute difference of two values of size x and store it in a result of size 2x. This means we have to be careful to calculate the absolute difference using a wide enough type that we don't accidentally overflow. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- target-arm/neon_helper.c | 38 +- 1 files changed, 21 insertions(+), 17 deletions(-) diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c index c3ac96a..7df925a 100644 --- a/target-arm/neon_helper.c +++ b/target-arm/neon_helper.c @@ -1514,9 +1514,13 @@ uint64_t HELPER(neon_addl_saturate_s64)(uint64_t a, uint64_t b) return result; } -#define DO_ABD(dest, x, y, type) do { \ -type tmp_x = x; \ -type tmp_y = y; \ +/* We have to do the arithmetic in a larger type than + * the input type, because for example with a signed 32 bit + * op the absolute difference can overflow a signed 32 bit value. + */ +#define DO_ABD(dest, x, y, intype, arithtype) do {\ +arithtype tmp_x = (intype)(x);\ +arithtype tmp_y = (intype)(y);\ dest = ((tmp_x tmp_y) ? tmp_x - tmp_y : tmp_y - tmp_x); \ } while(0) @@ -1524,12 +1528,12 @@ uint64_t HELPER(neon_abdl_u16)(uint32_t a, uint32_t b) { uint64_t tmp; uint64_t result; -DO_ABD(result, a, b, uint8_t); -DO_ABD(tmp, a 8, b 8, uint8_t); +DO_ABD(result, a, b, uint8_t, uint32_t); +DO_ABD(tmp, a 8, b 8, uint8_t, uint32_t); result |= tmp 16; -DO_ABD(tmp, a 16, b 16, uint8_t); +DO_ABD(tmp, a 16, b 16, uint8_t, uint32_t); result |= tmp 32; -DO_ABD(tmp, a 24, b 24, uint8_t); +DO_ABD(tmp, a 24, b 24, uint8_t, uint32_t); result |= tmp 48; return result; } @@ -1538,12 +1542,12 @@ uint64_t HELPER(neon_abdl_s16)(uint32_t a, uint32_t b) { uint64_t tmp; uint64_t result; -DO_ABD(result, a, b, int8_t); -DO_ABD(tmp, a 8, b 8, int8_t); +DO_ABD(result, a, b, int8_t, int32_t); +DO_ABD(tmp, a 8, b 8, int8_t, int32_t); result |= tmp 16; -DO_ABD(tmp, a 16, b 16, int8_t); +DO_ABD(tmp, a 16, b 16, int8_t, int32_t); result |= tmp 32; -DO_ABD(tmp, a 24, b 24, int8_t); +DO_ABD(tmp, a 24, b 24, int8_t, int32_t); result |= tmp 48; return result; } @@ -1552,8 +1556,8 @@ uint64_t HELPER(neon_abdl_u32)(uint32_t a, uint32_t b) { uint64_t tmp; uint64_t result; -DO_ABD(result, a, b, uint16_t); -DO_ABD(tmp, a 16, b 16, uint16_t); +DO_ABD(result, a, b, uint16_t, uint32_t); +DO_ABD(tmp, a 16, b 16, uint16_t, uint32_t); return result | (tmp 32); } @@ -1561,22 +1565,22 @@ uint64_t HELPER(neon_abdl_s32)(uint32_t a, uint32_t b) { uint64_t tmp; uint64_t result; -DO_ABD(result, a, b, int16_t); -DO_ABD(tmp, a 16, b 16, int16_t); +DO_ABD(result, a, b, int16_t, int32_t); +DO_ABD(tmp, a 16, b 16, int16_t, int32_t); return result | (tmp 32); } uint64_t HELPER(neon_abdl_u64)(uint32_t a, uint32_t b) { uint64_t result; -DO_ABD(result, a, b, uint32_t); +DO_ABD(result, a, b, uint32_t, uint64_t); return result; } uint64_t HELPER(neon_abdl_s64)(uint32_t a, uint32_t b) { uint64_t result; -DO_ABD(result, a, b, int32_t); +DO_ABD(result, a, b, int32_t, int64_t); return result; } #undef DO_ABD -- 1.7.1
Re: [Qemu-devel] Slow PXE boot in qemu.git (fast in qemu-kvm.git)
On 04/11/2011 03:04 PM, Jan Kiszka wrote: On 2011-04-11 21:15, Luiz Capitulino wrote: On Mon, 11 Apr 2011 13:00:32 -0600 Alex Williamsonalex.william...@redhat.com wrote: On Mon, 2011-04-11 at 15:35 -0300, Luiz Capitulino wrote: On Fri, 08 Apr 2011 19:50:57 -0500 Anthony Liguorianth...@codemonkey.ws wrote: On 04/08/2011 06:25 PM, Luiz Capitulino wrote: Hi there, Summary: - PXE boot in qemu.git (HEAD f124a41) is quite slow, more than 5 minutes. Got the problem with e1000, virtio and rtl8139. However, pcnet *works* (it's as fast as qemu-kvm.git) - PXE boot in qemu-kvm.git (HEAD df85c051) is fast, less than a minute. Tried with e1000, virtio and rtl8139 (I don't remember if I tried with pcnet) I was having this problem too, but I think it's because I forgot to build qemu with --enable-io-thread, which is the default for qemu-kvm. Can you re-configure and build with that and see if it's fast? Thanks, Yes, nice catch, it's faster with I/O thread enabled, even seem faster than qemu-kvm.git. What's the performance under qemu-kvm with -no-kvm-irqchip? So, does this have to be fixed w/o I/O thread? If it's most probably an architectural deficit of non-io-thread mode, I would say let it rest in peace. But maybe it points to a generic issues that is just magnified by non-threaded mode. If gpxe is spinning waiting for I/O to complete, that's going to prevent select from running until the next signal (timer event). Regards, Anthony Liguori Jan
Re: [Qemu-devel] [PATCH v2 1/2] rbd: use the higher level librbd instead of just librados
On 04/08/2011 01:43 AM, Stefan Hajnoczi wrote: On Mon, Mar 28, 2011 at 04:15:57PM -0700, Josh Durgin wrote: librbd stacks on top of librados to provide access to rbd images. Using librbd simplifies the qemu code, and allows qemu to use new versions of the rbd format with few (if any) changes. Signed-off-by: Josh Durginjosh.dur...@dreamhost.com Signed-off-by: Yehuda Sadehyeh...@hq.newdream.net --- block/rbd.c | 785 +++-- block/rbd_types.h | 71 - configure | 33 +-- 3 files changed, 221 insertions(+), 668 deletions(-) delete mode 100644 block/rbd_types.h Hi Josh, I have applied your patches onto qemu.git/master and am running ceph.git/master. Unfortunately qemu-iotests fails for me. Test 016 seems to hang in qemu-io -g -c write -P 66 128M 512 rbd:rbd/t.raw. I can reproduce this consistently. Here is the backtrace of the hung process (not consuming CPU, probably deadlocked): This hung because it wasn't checking the return value of rbd_aio_write. I've fixed this in the for-qemu branch of http://ceph.newdream.net/git/qemu-kvm.git. Also, the existing rbd implementation is not 'growable' - writing to a large offset will not expand the rbd image correctly. Should we implement bdrv_truncate to support this (librbd has a resize operation)? Is bdrv_truncate useful outside of qemu-img and qemu-io? Test 008 failed with an assertion but succeeded when run again. I think this is a race condition: This is likely a use-after-free, but I haven't been able to find the race condition yet (or reproduce it). Could you get a backtrace from the core file? Thanks, Josh
[Qemu-devel] [PATCH v2] kvm: ppc: fixes for KVM_SET_SREGS on init
Classic/server ppc has had SREGS for a while now (though I think not always?), but it's still missing for booke. Check the capability before calling KVM_SET_SREGS. Without this, booke kvm fails to boot as of commit 84b4915dd2c0eaa86c970ffc42a68ea8ba9e48b5 (kvm: Handle kvm_init_vcpu errors). Also, don't write random stack state into the non-PVR sregs fields -- have kvm fill it in first. Eventually booke will have sregs and it will have its own capability to be tested here. However, we will want a way for platform code to request to look like the actual CPU we're running on, especially if SoC devices are being directly assigned. Signed-off-by: Scott Wood scottw...@freescale.com --- v2: Use a runtime check for booke, at least for now. target-ppc/kvm.c | 33 ++--- 1 files changed, 30 insertions(+), 3 deletions(-) diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index 2cfb24b..e3cde16 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -77,13 +77,40 @@ int kvm_arch_init(KVMState *s) return 0; } -int kvm_arch_init_vcpu(CPUState *cenv) +static int kvm_arch_sync_sregs(CPUState *cenv) { -int ret = 0; struct kvm_sregs sregs; +int ret; + +if (cenv-excp_model == POWERPC_EXCP_BOOKE) { +return 0; +} else { +#ifdef KVM_CAP_PPC_SEGSTATE +if (!kvm_check_extension(cenv-kvm_state, KVM_CAP_PPC_SEGSTATE)) { +return 0; +} +#else +return 0; +#endif +} + +ret = kvm_vcpu_ioctl(cenv, KVM_GET_SREGS, sregs); +if (ret) { +return ret; +} sregs.pvr = cenv-spr[SPR_PVR]; -ret = kvm_vcpu_ioctl(cenv, KVM_SET_SREGS, sregs); +return kvm_vcpu_ioctl(cenv, KVM_SET_SREGS, sregs); +} + +int kvm_arch_init_vcpu(CPUState *cenv) +{ +int ret; + +ret = kvm_arch_sync_sregs(cenv); +if (ret) { +return ret; +} idle_timer = qemu_new_timer_ns(vm_clock, kvm_kick_env, cenv); -- 1.7.1
Re: [Qemu-devel] [V9fs-developer] Breaking out virtfs as a standalone server?
FWIW I've put a prerelease (1.0-pre20) of a pure 9P2000.L server up here: http://code.google.com/p/diod/ If anyone would like to give it a spin with a 2.6.38-ish kernel, please do and let me know how it goes! To get started with a quick (insecure) test: $ tar xjf diod-1.0-pre20.tar.bz2 $ cd diod-1.0-pre20 $ ./configure --disable-munge $ make $ sudo diod/diod -n -f -l 0.0.0.0:564 -c /dev/null -e /tmp $ sudo utils/diodmount -n -oport=564 localhost:/tmp /mnt I've been testing on a RHEL 6 (glibc-2.12) based system. We're just starting to test on big clusters with NFS. We've been successful with home directories and other things, but there is still much testing to do before a real 1.0 release. Also, as I mentioned before, I have attempted to document 9P2000.L as I've implemented it (referring to the kernel and qemu source) on a wiki page here: http://code.google.com/p/diod/wiki/protocol Maybe that will be a helpful start for someone implementing a new server, until something more official comes along. Jim On Mon, Apr 11, 2011 at 01:28:49PM -0700, Venkateswararao Jujjuri wrote: On 04/11/2011 06:42 AM, Rob Landley wrote: Right now, there's no decent userspace server for the 9p filesystem that I can find. (In part because the 9P2000.L spec is an undocumented work in progress.) This statement is true for 9P2000.L protocol; But for older protocols we have standalone servers like spfs/npfs. http://sourceforge.net/projects/npfs/ http://9p.cat-v.org/implementations The only up-to-date server seems to be virtfs in qemu, which has no TCP transport layer. Are there any plans to: A) Add a TCP transport layer so we can test with something we can intercept/examine/log/redirect with netcat and such? No plans as of now; I know folks in the Latchesar Ionkov attempted char dev transport. Not sure the latest though. http://sourceforge.net/mailarchive/forum.php?thread_name=AANLkTim4eZttAmaNQfOuM1h7cmLvO-osckHNunMvG7o%2B%40mail.gmail.comforum_name=v9fs-developer B) Break the 9p server out so it could be built as a standalone userspace program? No plans yet..and I think this is a bigger discussion. Being part of QEMU brings few implicit advantages like simplicity in sharing, security and performance advantage. I think taking it out can have its own merits. If there is enough interest I am sure these two are something we can look at as a community. - JV Rob -- Forrester Wave Report - Recovery time is now measured in hours and minutes not days. Key insights are discussed in the 2010 Forrester Wave Report as part of an in-depth evaluation of disaster recovery service providers. Forrester found the best-in-class provider in terms of services and vision. Read this report now! http://p.sf.net/sfu/ibm-webcastpromo ___ V9fs-developer mailing list v9fs-develo...@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/v9fs-developer
Re: [Qemu-devel] Question about total_sectors in block/vpc.c
Hello Christoph, Stefan I am wondering whether the problem occurred that value of BDRV_SECTOR_SIZE is a macro constant (defined at block.h). This problem could be fixed if we use variable instead of macro to implement BDRV_SECTOR_SIZE. Each block device may reassign the value if needed. Is it right?? Thanks a lot 2011/4/12 Christoph Hellwig h...@lst.de On Sat, Apr 09, 2011 at 09:05:41PM +0100, Stefan Hajnoczi wrote: On Sat, Apr 9, 2011 at 5:51 PM, Lyu Mitnick mitnick@gmail.com wrote: Hell all, I have take a look of block/vpc.c and meet a question in vpc_create().?At the line 550, the code is: total_sectors = options-value.n / 512; I am wondering whether the size between?total_sectors * 512 and?options-value.n would be discard. Yes, it rounds down. This reflects the assumption that a block device cannot be addressed below 512 byte sectors. Because of this block devices size must be a multiple of 512 bytes. I think a reasonable protection would be to have block.c:bdrv_create() fail if size is not a multiple of BDRV_SECTOR_SIZE. This way other image formats are protected too. There are block devices that aren't alignment to 512 bytes. Audio CDROMs are the most prominent example, or AS/400 disks. I don't think these matter for emulation, but if we'd ever implement DIF/DIX emulation inside qemu we'd have to store the protection information somewhere. It still wouldn't work with existing disk format, so adding the above check into the formats bdrv_create routines sounds fine, but doing it in the core block code might not be an overly smart idea. Mitnick
[Qemu-devel] [Bug 757654] Re: UHCI fails to signal stall response
Hi, Thanks for reporting this issue. Just so you are aware: the qemu USB maintainer is probably going to be away for a while longer, so don't be too worried if this patch doesn't get looked at for 2-4 weeks. Brad -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/757654 Title: UHCI fails to signal stall response Status in QEMU: New Bug description: When TD execution results in STALL error (STALL handshake, no stall as a result of err count reaching 0), there is no way to know about it except for checking that TD. IMO it is an error condition and it should be reflected in the status register (and issue an interrupt if enabled). Ways to replicate: Send a query that is answered by stall (like set_idle request to a mouse) Expected behavior: UHCI hc sets status bit 1 (usb error interrupt) and issues an interrupt current behavior: Neither status bit is set nor interrupt triggered Version 0.14 attached patch for current master (quick fix, it might be I got something wrong)
Re: [Qemu-devel] [PATCH v2 0/3] pc-bios: Use iPXE ROMs
On Mon, 2011-04-11 at 23:13 +0200, Stefan Weil wrote: Am 11.04.2011 21:35, schrieb Alex Williamson: This series replaces our current gPXE based PXE ROMs with iPXE versions from the iPXE project (http://ipxe.org). This version adds ipxe to our submodules so it can be easily included in releases. I'm still including a script for updating these, perhaps someone better with Makefiles can eventually adopt this to a build target. This email series is mainly for reference, there's too much renaming and replacing binary files to send out to the mailing list. I'll strip out the binaries here so the rest can be reviewed. For the real code, please pull: git://github.com/awilliam/qemu.git (ipxe branch) Thanks to Anthony for already setting up an ipxe mirror. Thanks, Alex --- Alex Williamson (3): PXE: Refresh all PXE ROMs from the ipxe submodule PXE: Use consistent naming for PXE ROMs Add ipxe submodule .gitmodules | 3 + Makefile | 16 +++--- hw/e1000.c | 2 - hw/eepro100.c | 2 - hw/ne2000.c | 2 - hw/pcnet-pci.c | 2 - hw/rtl8139.c | 2 - hw/virtio-pci.c | 2 - pc-bios/README | 19 +++ pc-bios/gpxe-eepro100-80861209.rom | Bin pc-bios/pxe-e1000.bin | Bin pc-bios/pxe-e1000.rom | Bin pc-bios/pxe-eepro100.rom | Bin pc-bios/pxe-ne2k_pci.bin | Bin pc-bios/pxe-ne2k_pci.rom | Bin pc-bios/pxe-pcnet.bin | Bin pc-bios/pxe-pcnet.rom | Bin pc-bios/pxe-rtl8139.bin | Bin pc-bios/pxe-rtl8139.rom | Bin pc-bios/pxe-virtio.bin | Bin pc-bios/pxe-virtio.rom | Bin roms/ipxe | 1 scripts/refresh-pxe-roms.sh | 99 23 files changed, 126 insertions(+), 24 deletions(-) delete mode 100644 pc-bios/gpxe-eepro100-80861209.rom delete mode 100644 pc-bios/pxe-e1000.bin create mode 100644 pc-bios/pxe-e1000.rom create mode 100644 pc-bios/pxe-eepro100.rom delete mode 100644 pc-bios/pxe-ne2k_pci.bin create mode 100644 pc-bios/pxe-ne2k_pci.rom delete mode 100644 pc-bios/pxe-pcnet.bin create mode 100644 pc-bios/pxe-pcnet.rom delete mode 100644 pc-bios/pxe-rtl8139.bin create mode 100644 pc-bios/pxe-rtl8139.rom delete mode 100644 pc-bios/pxe-virtio.bin create mode 100644 pc-bios/pxe-virtio.rom create mode 16 roms/ipxe create mode 100755 scripts/refresh-pxe-roms.sh Hi, thanks for your patches. I have two small remarks: $ scripts/refresh-pxe-roms.sh scripts/refresh-pxe-roms.sh: 29: Syntax error: ( unexpected Obviously this script depends on features only supported by more advanced shells like bash. It fails with /bin/sh - dash. Ok, switched to bash, hopefully that works better for you. Thanks for finding that. My second remark is about lists like those in Makefile: they should be sorted alphabetically. This may seem a cosmetic change, but it improves readability, avoids errors like duplicate or missing entries and also can avoid merge conflicts. Simple enough. I fixed both of these in the git tree, please verify. Thanks, Alex
Re: [Qemu-devel] Slow PXE boot in qemu.git (fast in qemu-kvm.git)
On Mon, 11 Apr 2011 22:04:52 +0200 Jan Kiszka jan.kis...@web.de wrote: On 2011-04-11 21:15, Luiz Capitulino wrote: On Mon, 11 Apr 2011 13:00:32 -0600 Alex Williamson alex.william...@redhat.com wrote: On Mon, 2011-04-11 at 15:35 -0300, Luiz Capitulino wrote: On Fri, 08 Apr 2011 19:50:57 -0500 Anthony Liguori anth...@codemonkey.ws wrote: On 04/08/2011 06:25 PM, Luiz Capitulino wrote: Hi there, Summary: - PXE boot in qemu.git (HEAD f124a41) is quite slow, more than 5 minutes. Got the problem with e1000, virtio and rtl8139. However, pcnet *works* (it's as fast as qemu-kvm.git) - PXE boot in qemu-kvm.git (HEAD df85c051) is fast, less than a minute. Tried with e1000, virtio and rtl8139 (I don't remember if I tried with pcnet) I was having this problem too, but I think it's because I forgot to build qemu with --enable-io-thread, which is the default for qemu-kvm. Can you re-configure and build with that and see if it's fast? Thanks, Yes, nice catch, it's faster with I/O thread enabled, even seem faster than qemu-kvm.git. What's the performance under qemu-kvm with -no-kvm-irqchip? Still fast, but just realized that qemu-kvm's configure says that I/O thread is disabled: IO thread no And it's fast.. So, does this have to be fixed w/o I/O thread? If it's most probably an architectural deficit of non-io-thread mode, I would say let it rest in peace. But maybe it points to a generic issues that is just magnified by non-threaded mode. Jan
Re: [Qemu-devel] [PATCH v2 0/3] pc-bios: Use iPXE ROMs
Am 11.04.2011 21:35, schrieb Alex Williamson: This series replaces our current gPXE based PXE ROMs with iPXE versions from the iPXE project (http://ipxe.org). This version adds ipxe to our submodules so it can be easily included in releases. I'm still including a script for updating these, perhaps someone better with Makefiles can eventually adopt this to a build target. This email series is mainly for reference, there's too much renaming and replacing binary files to send out to the mailing list. I'll strip out the binaries here so the rest can be reviewed. For the real code, please pull: git://github.com/awilliam/qemu.git (ipxe branch) Thanks to Anthony for already setting up an ipxe mirror. Thanks, Alex --- Alex Williamson (3): PXE: Refresh all PXE ROMs from the ipxe submodule PXE: Use consistent naming for PXE ROMs Add ipxe submodule .gitmodules | 3 + Makefile | 16 +++--- hw/e1000.c | 2 - hw/eepro100.c | 2 - hw/ne2000.c | 2 - hw/pcnet-pci.c | 2 - hw/rtl8139.c | 2 - hw/virtio-pci.c | 2 - pc-bios/README | 19 +++ pc-bios/gpxe-eepro100-80861209.rom | Bin pc-bios/pxe-e1000.bin | Bin pc-bios/pxe-e1000.rom | Bin pc-bios/pxe-eepro100.rom | Bin pc-bios/pxe-ne2k_pci.bin | Bin pc-bios/pxe-ne2k_pci.rom | Bin pc-bios/pxe-pcnet.bin | Bin pc-bios/pxe-pcnet.rom | Bin pc-bios/pxe-rtl8139.bin | Bin pc-bios/pxe-rtl8139.rom | Bin pc-bios/pxe-virtio.bin | Bin pc-bios/pxe-virtio.rom | Bin roms/ipxe | 1 scripts/refresh-pxe-roms.sh | 99 23 files changed, 126 insertions(+), 24 deletions(-) delete mode 100644 pc-bios/gpxe-eepro100-80861209.rom delete mode 100644 pc-bios/pxe-e1000.bin create mode 100644 pc-bios/pxe-e1000.rom create mode 100644 pc-bios/pxe-eepro100.rom delete mode 100644 pc-bios/pxe-ne2k_pci.bin create mode 100644 pc-bios/pxe-ne2k_pci.rom delete mode 100644 pc-bios/pxe-pcnet.bin create mode 100644 pc-bios/pxe-pcnet.rom delete mode 100644 pc-bios/pxe-rtl8139.bin create mode 100644 pc-bios/pxe-rtl8139.rom delete mode 100644 pc-bios/pxe-virtio.bin create mode 100644 pc-bios/pxe-virtio.rom create mode 16 roms/ipxe create mode 100755 scripts/refresh-pxe-roms.sh Hi, thanks for your patches. I have two small remarks: $ scripts/refresh-pxe-roms.sh scripts/refresh-pxe-roms.sh: 29: Syntax error: ( unexpected Obviously this script depends on features only supported by more advanced shells like bash. It fails with /bin/sh - dash. My second remark is about lists like those in Makefile: they should be sorted alphabetically. This may seem a cosmetic change, but it improves readability, avoids errors like duplicate or missing entries and also can avoid merge conflicts. Regards, Stefan Weil
[Qemu-devel] UNBEATABLE OFFER: ORDER 2 APPLE iPHONES AND GET 1 FREE
TELECOM-SALES SERVICE Lmtd UK is offering BRAND NEW PHONES 3G/4G,TRI-BAND/QUADBAND,Factory Sealed Boxes,complete Accessories and 1 year Manufacturers Warranty,Also Guaranteed to work with all GSM Carrier/Network Worldwide. APPLE iPHONE 4G/32GB = $450 http://www.apple.com/iphone/ APPLE iPADS 64GB/3G with WiFi = $650 http://www.apple.com/ipad/ Ultra Thin APPLE Macbook Air 13 Laptop = $1050 http://www.apple.com/macbookair/ BLACKBERRY Torch = $470 USD http://us.blackberry.com/smartphones/blackberrytorch/ BLACKBERRY Curve = $420 USD http://us.blackberry.com/smartphones/blackberrycurve/ BLACKBERRY Bold = $425 USD http://us.blackberry.com/smartphones/blackberrybold/ BLACKBERRY Storm = $450 USD http://us.blackberry.com/smartphones/blackberrystorm/ Order any 2 Phones AND Get FREE APPLE iPHONE 4G/32GB Order any 3 Phones AND Get FREE APPLE iPADS 64GB Order any 4 Phones AND Get FREE Apple Mac-book Air 13 Laptop TELECOM-SALES SERVICE Lmtd UK (Reg No. 07490474) 42 ROLLESBY GARDENS, ST HELENS, WA9 5WG, UNITED KINGDOM
Re: [Qemu-devel] Slow PXE boot in qemu.git (fast in qemu-kvm.git)
On 2011-04-11 22:18, Jan Kiszka wrote: On 2011-04-11 22:14, Alex Williamson wrote: On Mon, 2011-04-11 at 22:04 +0200, Jan Kiszka wrote: On 2011-04-11 21:15, Luiz Capitulino wrote: On Mon, 11 Apr 2011 13:00:32 -0600 Alex Williamson alex.william...@redhat.com wrote: On Mon, 2011-04-11 at 15:35 -0300, Luiz Capitulino wrote: On Fri, 08 Apr 2011 19:50:57 -0500 Anthony Liguori anth...@codemonkey.ws wrote: On 04/08/2011 06:25 PM, Luiz Capitulino wrote: Hi there, Summary: - PXE boot in qemu.git (HEAD f124a41) is quite slow, more than 5 minutes. Got the problem with e1000, virtio and rtl8139. However, pcnet *works* (it's as fast as qemu-kvm.git) - PXE boot in qemu-kvm.git (HEAD df85c051) is fast, less than a minute. Tried with e1000, virtio and rtl8139 (I don't remember if I tried with pcnet) I was having this problem too, but I think it's because I forgot to build qemu with --enable-io-thread, which is the default for qemu-kvm. Can you re-configure and build with that and see if it's fast? Thanks, Yes, nice catch, it's faster with I/O thread enabled, even seem faster than qemu-kvm.git. What's the performance under qemu-kvm with -no-kvm-irqchip? So, does this have to be fixed w/o I/O thread? If it's most probably an architectural deficit of non-io-thread mode, I would say let it rest in peace. But maybe it points to a generic issues that is just magnified by non-threaded mode. I've probably been told, but forget. Why isn't io-thread enabled by default? Thanks, TCG performance still sucks in io-threaded mode. I've three patches in my queue that reduces the overhead a bit further - for me to a reasonable level (will post them the next days). But, still, YMMV depending on the workload. In fact, they were already prepared. So I've just sent them out. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Slow PXE boot in qemu.git (fast in qemu-kvm.git)
On Mon, 11 Apr 2011 13:00:32 -0600 Alex Williamson alex.william...@redhat.com wrote: On Mon, 2011-04-11 at 15:35 -0300, Luiz Capitulino wrote: On Fri, 08 Apr 2011 19:50:57 -0500 Anthony Liguori anth...@codemonkey.ws wrote: On 04/08/2011 06:25 PM, Luiz Capitulino wrote: Hi there, Summary: - PXE boot in qemu.git (HEAD f124a41) is quite slow, more than 5 minutes. Got the problem with e1000, virtio and rtl8139. However, pcnet *works* (it's as fast as qemu-kvm.git) - PXE boot in qemu-kvm.git (HEAD df85c051) is fast, less than a minute. Tried with e1000, virtio and rtl8139 (I don't remember if I tried with pcnet) I was having this problem too, but I think it's because I forgot to build qemu with --enable-io-thread, which is the default for qemu-kvm. Can you re-configure and build with that and see if it's fast? Thanks, Yes, nice catch, it's faster with I/O thread enabled, even seem faster than qemu-kvm.git. So, does this have to be fixed w/o I/O thread?
[Qemu-devel] [PATCH v2 0/3] pc-bios: Use iPXE ROMs
This series replaces our current gPXE based PXE ROMs with iPXE versions from the iPXE project (http://ipxe.org). This version adds ipxe to our submodules so it can be easily included in releases. I'm still including a script for updating these, perhaps someone better with Makefiles can eventually adopt this to a build target. This email series is mainly for reference, there's too much renaming and replacing binary files to send out to the mailing list. I'll strip out the binaries here so the rest can be reviewed. For the real code, please pull: git://github.com/awilliam/qemu.git (ipxe branch) Thanks to Anthony for already setting up an ipxe mirror. Thanks, Alex --- Alex Williamson (3): PXE: Refresh all PXE ROMs from the ipxe submodule PXE: Use consistent naming for PXE ROMs Add ipxe submodule .gitmodules|3 + Makefile | 16 +++--- hw/e1000.c |2 - hw/eepro100.c |2 - hw/ne2000.c|2 - hw/pcnet-pci.c |2 - hw/rtl8139.c |2 - hw/virtio-pci.c|2 - pc-bios/README | 19 +++ pc-bios/gpxe-eepro100-80861209.rom | Bin pc-bios/pxe-e1000.bin | Bin pc-bios/pxe-e1000.rom | Bin pc-bios/pxe-eepro100.rom | Bin pc-bios/pxe-ne2k_pci.bin | Bin pc-bios/pxe-ne2k_pci.rom | Bin pc-bios/pxe-pcnet.bin | Bin pc-bios/pxe-pcnet.rom | Bin pc-bios/pxe-rtl8139.bin| Bin pc-bios/pxe-rtl8139.rom| Bin pc-bios/pxe-virtio.bin | Bin pc-bios/pxe-virtio.rom | Bin roms/ipxe |1 scripts/refresh-pxe-roms.sh| 99 23 files changed, 126 insertions(+), 24 deletions(-) delete mode 100644 pc-bios/gpxe-eepro100-80861209.rom delete mode 100644 pc-bios/pxe-e1000.bin create mode 100644 pc-bios/pxe-e1000.rom create mode 100644 pc-bios/pxe-eepro100.rom delete mode 100644 pc-bios/pxe-ne2k_pci.bin create mode 100644 pc-bios/pxe-ne2k_pci.rom delete mode 100644 pc-bios/pxe-pcnet.bin create mode 100644 pc-bios/pxe-pcnet.rom delete mode 100644 pc-bios/pxe-rtl8139.bin create mode 100644 pc-bios/pxe-rtl8139.rom delete mode 100644 pc-bios/pxe-virtio.bin create mode 100644 pc-bios/pxe-virtio.rom create mode 16 roms/ipxe create mode 100755 scripts/refresh-pxe-roms.sh
Re: [Qemu-devel] GSoC - QCOW2 - QED image converter
On Thu, Apr 07, 2011 at 07:10:00AM +0800, Lyu Mitnick wrote: Would you mind to share your solution about this feature?? My idea is replace all bdrv_pread(), bdrv_pwrite_sync(), bdrv_pwrite() in block/vpc.c to bdrv_pread_split(), bdrv_pwrite_sync_split(), bdrv_pwrite_split() corresponding. bdrv_pread_split(), bdrv_pwrite_sync_split(), bdrv_pwrite_split() should deal with data splitted at different files via bdrv_pread(), bdrv_pwrite_sync(), bdrv_pwrite(), give an interface of sequential byte accessed. Is it suitable to solve this issue?? For the synchronous bdrv_read/bdrv_write used in vmdk the problem actually is rather trivial, as it already split it's reads/write on a cluster boundary internally, and the higher level split is always alignment to a cluster boundary. So basically no new work there.
Re: [Qemu-devel] No scratch directory in qemu-iotests
On Fri, Apr 08, 2011 at 09:47:47AM +0100, Stefan Hajnoczi wrote: I think it's a good idea to make qemu-iotests work out-of-the-box. Can you send a patch for this? Note that scratch/ is in .gitignore so you might have to tweak .gitignore to get the behavior you want. Alternatively common.config could mkdir scratch/ on first run. Hmm. The original idea was you make it a symlink to a partitions so that you don't have to nessecarily run it inside the source tree. But I suspect just include the directory and allowing an override is fine, too.
Re: [Qemu-devel] [PATCH] ppc: remove a write-only variable
On Mon, Apr 11, 2011 at 10:48 AM, Alexander Graf ag...@suse.de wrote: On 11.04.2011, at 06:30, David Gibson wrote: On Sat, Apr 09, 2011 at 05:28:06PM +0200, Alexander Graf wrote: Am 09.04.2011 um 16:56 schrieb Blue Swirl blauwir...@gmail.com: Remove a write-only variable, spotted by GCC 4.6.0: /src/qemu/hw/ppc.c: In function 'power7_set_irq': /src/qemu/hw/ppc.c:255:9: error: variable 'cur_level' set but not used [-Werror=unused-but-set-variable] Signed-off-by: Blue Swirl blauwir...@gmail.com --- hw/ppc.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/hw/ppc.c b/hw/ppc.c index dabb816..1873328 100644 --- a/hw/ppc.c +++ b/hw/ppc.c @@ -252,11 +252,9 @@ void ppc970_irq_init (CPUState *env) static void power7_set_irq (void *opaque, int pin, int level) { CPUState *env = opaque; - int cur_level; LOG_IRQ(%s: env %p pin %d level %d\n, __func__, env, pin, level); - cur_level = (env-irq_input_state pin) 1; David, Ben? That should be fine. It's a hang over from the ppc970 code which I adapted. Alright. Acked-by: Alexander Graf ag...@suse.de Blue, mind to apply it directly? That's easier than going through my tree. Thanks for the ack, applied.
Re: [Qemu-devel] [PATCH 4/4] qemu-timer: Fix timers for w32
Am 11.04.2011 09:36, schrieb Paolo Bonzini: On 04/10/2011 08:28 PM, Stefan Weil wrote: Commit 68c23e5520e8286d79d96ab47c0ea722ceb75041 removed the multimedia timer, but this timer is needed for certain Linux kernels. Otherwise Linux boot stops with this error: MP-BIOS bug: 8254 timer not connected to IO-APIC So the multimedia timer is added again here. Which distribution and Windows version is that? Also, have they tried the non-dynticks timer (win32)? Paolo The bug was reported for a tinycore 3.5.1 guest (linux kernel 3.6.33-3). The iso image is available from http://distro.ibiblio.org/tinycorelinux/3.x/release/. QEMU was running on XP SP3. Other Linux live CD-ROMs (e.g. FC14) were reported to show the same bug when run as guest. APIC can be disabled (kernel parameter), without APIC there is no problem. I see the same bug here with two XP hosts and also tried both timer variants of current QEMU (without a difference). I don't get the bug when running on a Linux host using wine. Regards, Stefan
Re: [Qemu-devel] [PATCH 2/2 V7] qemu, qmp: add inject-nmi qmp command
On Mon, Apr 11, 2011 at 10:01 AM, Markus Armbruster arm...@redhat.com wrote: Avi Kivity a...@redhat.com writes: On 04/08/2011 12:41 AM, Anthony Liguori wrote: And it's a good thing to have, but exposing this as the only API to do something as simple as generating a guest crash dump is not the friendliest thing in the world to do to users. nmi is a fine name for something that corresponds to a real-life nmi button (often labeled NMI). Agree. We could also introduce an alias mechanism for user friendly names, so nmi could be used in addition of full path. Aliases could be useful for device paths as well.
Re: [Qemu-devel] Question about total_sectors in block/vpc.c
On Sat, Apr 09, 2011 at 09:05:41PM +0100, Stefan Hajnoczi wrote: On Sat, Apr 9, 2011 at 5:51 PM, Lyu Mitnick mitnick@gmail.com wrote: Hell all, I have take a look of block/vpc.c and meet a question in vpc_create().?At the line 550, the code is: total_sectors = options-value.n / 512; I am wondering whether the size between?total_sectors * 512 and?options-value.n would be discard. Yes, it rounds down. This reflects the assumption that a block device cannot be addressed below 512 byte sectors. Because of this block devices size must be a multiple of 512 bytes. I think a reasonable protection would be to have block.c:bdrv_create() fail if size is not a multiple of BDRV_SECTOR_SIZE. This way other image formats are protected too. There are block devices that aren't alignment to 512 bytes. Audio CDROMs are the most prominent example, or AS/400 disks. I don't think these matter for emulation, but if we'd ever implement DIF/DIX emulation inside qemu we'd have to store the protection information somewhere. It still wouldn't work with existing disk format, so adding the above check into the formats bdrv_create routines sounds fine, but doing it in the core block code might not be an overly smart idea.
[Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option
From: Herv� Poussineau hpous...@reactos.org This option is described in RFC 1783. As this is only an optional field, we may ignore it in some situations and handle it in some others. Here, if client requests a block size bigger than the block size we emit (512 bytes), accept the option with a value of 512 Signed-off-by: Herv� Poussineau hpous...@reactos.org --- slirp/tftp.c | 40 1 files changed, 32 insertions(+), 8 deletions(-) diff --git a/slirp/tftp.c b/slirp/tftp.c index 8055ccc..7ef44c9 100644 --- a/slirp/tftp.c +++ b/slirp/tftp.c @@ -116,13 +116,13 @@ static int tftp_read_data(struct tftp_session *spt, uint16_t block_nr, } static int tftp_send_oack(struct tftp_session *spt, - const char *key, uint32_t value, + const char *key[], uint32_t value[], int nb, struct tftp_t *recv_tp) { struct sockaddr_in saddr, daddr; struct mbuf *m; struct tftp_t *tp; -int n = 0; +int i, n = 0; m = m_get(spt-slirp); @@ -136,10 +136,12 @@ static int tftp_send_oack(struct tftp_session *spt, m-m_data += sizeof(struct udpiphdr); tp-tp_op = htons(TFTP_OACK); -n += snprintf(tp-x.tp_buf + n, sizeof(tp-x.tp_buf) - n, %s, - key) + 1; -n += snprintf(tp-x.tp_buf + n, sizeof(tp-x.tp_buf) - n, %u, - value) + 1; +for (i = 0; i nb; i++) { + n += snprintf(tp-x.tp_buf + n, sizeof(tp-x.tp_buf) - n, %s, +key[i]) + 1; + n += snprintf(tp-x.tp_buf + n, sizeof(tp-x.tp_buf) - n, %u, +value[i]) + 1; +} saddr.sin_addr = recv_tp-ip.ip_dst; saddr.sin_port = recv_tp-udp.uh_dport; @@ -260,6 +262,9 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen) int s, k; size_t prefix_len; char *req_fname; + const char *option_name[2]; + uint32_t option_value[2]; + int nb_options = 0; /* check if a session already exists and if so terminate it */ s = tftp_session_find(slirp, tp); @@ -364,9 +369,28 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen) } } - tftp_send_oack(spt, tsize, tsize, tp); - return; + option_name[nb_options] = tsize; + option_value[nb_options] = tsize; + nb_options++; } +if (strcasecmp(key, blksize) == 0) { + int blksize = atoi(value); + + /* If blksize option is bigger than what we will + * emit, accept the option with our packet size. + * Otherwise, simply do as we didn't see the option. + */ + if (blksize = 512) { +option_name[nb_options] = blksize; +option_value[nb_options] = 512; +nb_options++; + } +} + } + + if (nb_options 0) { +tftp_send_oack(spt, option_name, option_value, nb_options, tp); +return; } tftp_send_data(spt, 1, tp); -- 1.6.0.2.GIT
Re: [Qemu-devel] [PATCH v2 0/3] pc-bios: Use iPXE ROMs
On 04/11/2011 02:35 PM, Alex Williamson wrote: This series replaces our current gPXE based PXE ROMs with iPXE versions from the iPXE project (http://ipxe.org). This version adds ipxe to our submodules so it can be easily included in releases. I'm still including a script for updating these, perhaps someone better with Makefiles can eventually adopt this to a build target. This email series is mainly for reference, there's too much renaming and replacing binary files to send out to the mailing list. I'll strip out the binaries here so the rest can be reviewed. For the real code, please pull: git://github.com/awilliam/qemu.git (ipxe branch) Thanks to Anthony for already setting up an ipxe mirror. Thanks, Looks good to me. How different is this from what we've been shipping? Have you tested PXE boot from the builtin TFTP server and from an external one (like dnsmasq)? Regards, Anthony Liguori Alex --- Alex Williamson (3): PXE: Refresh all PXE ROMs from the ipxe submodule PXE: Use consistent naming for PXE ROMs Add ipxe submodule .gitmodules|3 + Makefile | 16 +++--- hw/e1000.c |2 - hw/eepro100.c |2 - hw/ne2000.c|2 - hw/pcnet-pci.c |2 - hw/rtl8139.c |2 - hw/virtio-pci.c|2 - pc-bios/README | 19 +++ pc-bios/gpxe-eepro100-80861209.rom | Bin pc-bios/pxe-e1000.bin | Bin pc-bios/pxe-e1000.rom | Bin pc-bios/pxe-eepro100.rom | Bin pc-bios/pxe-ne2k_pci.bin | Bin pc-bios/pxe-ne2k_pci.rom | Bin pc-bios/pxe-pcnet.bin | Bin pc-bios/pxe-pcnet.rom | Bin pc-bios/pxe-rtl8139.bin| Bin pc-bios/pxe-rtl8139.rom| Bin pc-bios/pxe-virtio.bin | Bin pc-bios/pxe-virtio.rom | Bin roms/ipxe |1 scripts/refresh-pxe-roms.sh| 99 23 files changed, 126 insertions(+), 24 deletions(-) delete mode 100644 pc-bios/gpxe-eepro100-80861209.rom delete mode 100644 pc-bios/pxe-e1000.bin create mode 100644 pc-bios/pxe-e1000.rom create mode 100644 pc-bios/pxe-eepro100.rom delete mode 100644 pc-bios/pxe-ne2k_pci.bin create mode 100644 pc-bios/pxe-ne2k_pci.rom delete mode 100644 pc-bios/pxe-pcnet.bin create mode 100644 pc-bios/pxe-pcnet.rom delete mode 100644 pc-bios/pxe-rtl8139.bin create mode 100644 pc-bios/pxe-rtl8139.rom delete mode 100644 pc-bios/pxe-virtio.bin create mode 100644 pc-bios/pxe-virtio.rom create mode 16 roms/ipxe create mode 100755 scripts/refresh-pxe-roms.sh
[Qemu-devel] [qemu-iotests] [PATCH] common.config: Fix no $TEST_DIR directory
mkdir $TEST_DIR on common.config first run Signed-off-by: Mitnick Lyu mitnick@gmail.com --- common.config |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/common.config b/common.config index bdd0530..d5a72af 100644 --- a/common.config +++ b/common.config @@ -102,8 +102,12 @@ export QEMU_IO=$QEMU_IO_PROG $QEMU_IO_OPTIONS [ -f /etc/qemu-iotest.config ]. /etc/qemu-iotest.config +if [ -z $TEST_DIR ]; then + TEST_DIR=`pwd`/scratch +fi + if [ ! -e $TEST_DIR ]; then -TEST_DIR=`pwd`/scratch + mkdir $TEST_DIR fi if [ ! -d $TEST_DIR ]; then -- 1.7.0.4
Re: [Qemu-devel] [PATCH] target-sh4: get rid of CPU_{Float, Double}U
On 04/11/2011 08:30 AM, Peter Maydell wrote: On 11 April 2011 16:19, Richard Henderson r...@twiddle.net wrote: On 04/11/2011 08:09 AM, Peter Maydell wrote: (4) I think you should be able to write a helper function for an add as just float32 HELPER(my_float_add)(float32 a, float32 b) { return float32_add(a, b, status); } While this is a laudable goal, this will fail for hosts that pass all structures by reference. This is true of, e.g. PPC32. ...but only if float32 is a struct, which is where we came in. In the sane default configuration float32 is just a uint32_t in disguise. Well, that's all right then. So long as we restrict ourselves to passing around (typedefed) integers and pointers only, we'll be ok. r~
Re: [Qemu-devel] [PATCH V12 05/17] xen: Add xenfv machine
On 2011-04-11 20:10, Anthony PERARD wrote: } static CPUState *pc_new_cpu(const char *cpu_model) @@ -952,7 +957,12 @@ void pc_cpus_init(const char *cpu_model) #endif } -for(i = 0; i smp_cpus; i++) { +if (!xen_enabled()) { +for(i = 0; i smp_cpus; i++) { +pc_new_cpu(cpu_model); +} +} else { +/* Xen require only one Qemu VCPU */ pc_new_cpu(cpu_model); This looks a bit fishy. What is the semantic of -smp 2 or more in Xen mode? If that is an invalid/unused configuration option, catch that and reject it instead of installing this workaround. If it has a valid semantic, please elaborate why you need to restrict the number of instantiated cpus. Just to optimize memory usage? I thought in a first place that was needed to avoid errors. But it works also when we initialise other CPUs. But I prefere to keep it that way to save memory and in the case where there is a thread for each cpu that will also avoid to have many useless threads. How much memory does this save? More than a few KB per VCPU? That should be negligible compared to the normal size of VMs. And as long as we do not support multi-threaded TCG VCPUs, Xen will only create on thread for all VCPUs (once that may change, Xen could control the execution model via qemu_init_vcpu). So I would prefer to avoid this additional Xen-specific branch in generic code. Thanks, Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Breaking out virtfs as a standalone server?
On 04/11/2011 06:42 AM, Rob Landley wrote: Right now, there's no decent userspace server for the 9p filesystem that I can find. (In part because the 9P2000.L spec is an undocumented work in progress.) This statement is true for 9P2000.L protocol; But for older protocols we have standalone servers like spfs/npfs. http://sourceforge.net/projects/npfs/ http://9p.cat-v.org/implementations The only up-to-date server seems to be virtfs in qemu, which has no TCP transport layer. Are there any plans to: A) Add a TCP transport layer so we can test with something we can intercept/examine/log/redirect with netcat and such? No plans as of now; I know folks in the Latchesar Ionkov attempted char dev transport. Not sure the latest though. http://sourceforge.net/mailarchive/forum.php?thread_name=AANLkTim4eZttAmaNQfOuM1h7cmLvO-osckHNunMvG7o%2B%40mail.gmail.comforum_name=v9fs-developer B) Break the 9p server out so it could be built as a standalone userspace program? No plans yet..and I think this is a bigger discussion. Being part of QEMU brings few implicit advantages like simplicity in sharing, security and performance advantage. I think taking it out can have its own merits. If there is enough interest I am sure these two are something we can look at as a community. - JV Rob
Re: [Qemu-devel] Slow PXE boot in qemu.git (fast in qemu-kvm.git)
On 2011-04-11 23:05, Luiz Capitulino wrote: On Mon, 11 Apr 2011 22:04:52 +0200 Jan Kiszka jan.kis...@web.de wrote: On 2011-04-11 21:15, Luiz Capitulino wrote: On Mon, 11 Apr 2011 13:00:32 -0600 Alex Williamson alex.william...@redhat.com wrote: On Mon, 2011-04-11 at 15:35 -0300, Luiz Capitulino wrote: On Fri, 08 Apr 2011 19:50:57 -0500 Anthony Liguori anth...@codemonkey.ws wrote: On 04/08/2011 06:25 PM, Luiz Capitulino wrote: Hi there, Summary: - PXE boot in qemu.git (HEAD f124a41) is quite slow, more than 5 minutes. Got the problem with e1000, virtio and rtl8139. However, pcnet *works* (it's as fast as qemu-kvm.git) - PXE boot in qemu-kvm.git (HEAD df85c051) is fast, less than a minute. Tried with e1000, virtio and rtl8139 (I don't remember if I tried with pcnet) I was having this problem too, but I think it's because I forgot to build qemu with --enable-io-thread, which is the default for qemu-kvm. Can you re-configure and build with that and see if it's fast? Thanks, Yes, nice catch, it's faster with I/O thread enabled, even seem faster than qemu-kvm.git. What's the performance under qemu-kvm with -no-kvm-irqchip? Still fast, I meant: is it even faster with unaccelerated userspace irqchip? I've seen such effects with emulated NICs before. but just realized that qemu-kvm's configure says that I/O thread is disabled: IO thread no And it's fast.. That only means (so far) that the upstream io-thread code is disabled. Qemu-kvm's own solution is enabled all the time, and you can't switch to upstream anyway as both are incompatible. That's going to change soon (hopefully) when we migrate qemu-kvm to the upstream version. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] KVM call agenda for April 12
On 04/11/2011 11:35 AM, Juan Quintela wrote: Please, send in any agenda items you are interested in covering. I won't be able to attend due. Regards, Anthony Liguori Later, Juan. -- 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] Slow PXE boot in qemu.git (fast in qemu-kvm.git)
On 2011-04-11 22:14, Alex Williamson wrote: On Mon, 2011-04-11 at 22:04 +0200, Jan Kiszka wrote: On 2011-04-11 21:15, Luiz Capitulino wrote: On Mon, 11 Apr 2011 13:00:32 -0600 Alex Williamson alex.william...@redhat.com wrote: On Mon, 2011-04-11 at 15:35 -0300, Luiz Capitulino wrote: On Fri, 08 Apr 2011 19:50:57 -0500 Anthony Liguori anth...@codemonkey.ws wrote: On 04/08/2011 06:25 PM, Luiz Capitulino wrote: Hi there, Summary: - PXE boot in qemu.git (HEAD f124a41) is quite slow, more than 5 minutes. Got the problem with e1000, virtio and rtl8139. However, pcnet *works* (it's as fast as qemu-kvm.git) - PXE boot in qemu-kvm.git (HEAD df85c051) is fast, less than a minute. Tried with e1000, virtio and rtl8139 (I don't remember if I tried with pcnet) I was having this problem too, but I think it's because I forgot to build qemu with --enable-io-thread, which is the default for qemu-kvm. Can you re-configure and build with that and see if it's fast? Thanks, Yes, nice catch, it's faster with I/O thread enabled, even seem faster than qemu-kvm.git. What's the performance under qemu-kvm with -no-kvm-irqchip? So, does this have to be fixed w/o I/O thread? If it's most probably an architectural deficit of non-io-thread mode, I would say let it rest in peace. But maybe it points to a generic issues that is just magnified by non-threaded mode. I've probably been told, but forget. Why isn't io-thread enabled by default? Thanks, TCG performance still sucks in io-threaded mode. I've three patches in my queue that reduces the overhead a bit further - for me to a reasonable level (will post them the next days). But, still, YMMV depending on the workload. At least Windows should no longer we a functional blocker thanks to Paolo's work. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error
On Mon, Apr 11, 2011 at 9:53 PM, Artyom Tarasenko atar4q...@gmail.com wrote: Can do it, but I'd like to understand first what we are looking for. How does the main works in this case? Is it something like following? translate {brz,pn ; wrpr} - optimize - execute -translate {retl ; ...} -optimize - execute. The subject error is a tcg error, so it is happening in one of the two translate/optimise phases drawn above, right? So, why are we looking at the wrpr helper code? Because you asked about alternate globals :) Do you have public test case? It is possible to code this delay slot write test but real issue may be corruption elsewhere. You assume ts-val_type gets corrupted? But then it must happen before the wrpr helper call, or actually before the translation of {brz,pn ; wrpr} block, no? In theory there could be multiple issues including compiler induced ones. I'd prefer to see some kind of reproducible testcase. -- Kind regards, Igor V. Kovalenko
Re: [Qemu-devel] [PATCH v2 0/3] pc-bios: Use iPXE ROMs
On Mon, 2011-04-11 at 14:48 -0500, Anthony Liguori wrote: On 04/11/2011 02:35 PM, Alex Williamson wrote: This series replaces our current gPXE based PXE ROMs with iPXE versions from the iPXE project (http://ipxe.org). This version adds ipxe to our submodules so it can be easily included in releases. I'm still including a script for updating these, perhaps someone better with Makefiles can eventually adopt this to a build target. This email series is mainly for reference, there's too much renaming and replacing binary files to send out to the mailing list. I'll strip out the binaries here so the rest can be reviewed. For the real code, please pull: git://github.com/awilliam/qemu.git (ipxe branch) Thanks to Anthony for already setting up an ipxe mirror. Thanks, Looks good to me. How different is this from what we've been shipping? Have you tested PXE boot from the builtin TFTP server and from an external one (like dnsmasq)? We were shipping v0.9.9, which was tagged 10/2009. There's been a gpxe v1.0.0 release since then, plus the split between ipxe and gpxe. I think Michael is hoping to have a release soon, but the code feels pretty stable to me as is. I've tested external booting from dhcp/tftp server for all the NICs. I'll make a pass through testing with the builtin server and report back. Thanks, Alex
[Qemu-devel] [PATCH 3/3] slirp: improve TFTP performance
From: Herv� Poussineau hpous...@reactos.org When transfering a file, keep it open during the whole transfer, instead of opening/closing it for each block. Signed-off-by: Herv� Poussineau hpous...@reactos.org --- slirp/tftp.c | 20 slirp/tftp.h |1 + 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/slirp/tftp.c b/slirp/tftp.c index 7e63269..48748f1 100644 --- a/slirp/tftp.c +++ b/slirp/tftp.c @@ -37,6 +37,10 @@ static inline void tftp_session_update(struct tftp_session *spt) static void tftp_session_terminate(struct tftp_session *spt) { +if (spt-fd = 0) { +close(spt-fd); +spt-fd = -1; +} qemu_free(spt-filename); spt-slirp = NULL; } @@ -54,7 +58,7 @@ static int tftp_session_allocate(Slirp *slirp, struct tftp_t *tp) /* sessions time out after 5 inactive seconds */ if ((int)(curtime - spt-timestamp) 5000) { -qemu_free(spt-filename); +tftp_session_terminate(spt); goto found; } } @@ -64,6 +68,7 @@ static int tftp_session_allocate(Slirp *slirp, struct tftp_t *tp) found: memset(spt, 0, sizeof(*spt)); memcpy(spt-client_ip, tp-ip.ip_src, sizeof(spt-client_ip)); + spt-fd = -1; spt-client_port = tp-udp.uh_sport; spt-slirp = slirp; @@ -95,23 +100,22 @@ static int tftp_session_find(Slirp *slirp, struct tftp_t *tp) static int tftp_read_data(struct tftp_session *spt, uint32_t block_nr, uint8_t *buf, int len) { - int fd; int bytes_read = 0; - fd = open(spt-filename, O_RDONLY | O_BINARY); + if (spt-fd 0) { +spt-fd = open(spt-filename, O_RDONLY | O_BINARY); + } - if (fd 0) { + if (spt-fd 0) { return -1; } if (len) { -lseek(fd, block_nr * 512, SEEK_SET); +lseek(spt-fd, block_nr * 512, SEEK_SET); -bytes_read = read(fd, buf, len); +bytes_read = read(spt-fd, buf, len); } - close(fd); - return bytes_read; } diff --git a/slirp/tftp.h b/slirp/tftp.h index 471f22e..51704e4 100644 --- a/slirp/tftp.h +++ b/slirp/tftp.h @@ -33,6 +33,7 @@ struct tftp_t { struct tftp_session { Slirp *slirp; char *filename; +int fd; struct in_addr client_ip; uint16_t client_port; -- 1.6.0.2.GIT
[Qemu-devel] [PATCH v2 0/3] io-thread optimizations
These patches were posted before. They bring down the overhead of the io-thread mode for TCG here, specifically when emulating SMP. The major change in this version, besides rebasing, is the exclusion of KVM from the main loop polling optimization. Jan Kiszka (3): Do not drop global mutex for polled main loop runs Poll main loop after I/O events were received Do not kick vcpus in TCG mode cpus.c |2 +- sysemu.h |2 +- vl.c | 22 -- 3 files changed, 18 insertions(+), 8 deletions(-)
[Qemu-devel] [PATCH v2 3/3] Do not kick vcpus in TCG mode
From: Jan Kiszka jan.kis...@siemens.com In TCG mode, iothread and vcpus run in lock-step. So it's pointless to send a signal from qemu_cpu_kick to the vcpu thread - if we got here, the receiver already left the vcpu loop. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- cpus.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/cpus.c b/cpus.c index 41bec7c..82a7806 100644 --- a/cpus.c +++ b/cpus.c @@ -860,7 +860,7 @@ void qemu_cpu_kick(void *_env) CPUState *env = _env; qemu_cond_broadcast(env-halt_cond); -if (!env-thread_kicked) { +if (kvm_enabled() !env-thread_kicked) { qemu_cpu_kick_thread(env); env-thread_kicked = true; } -- 1.7.1
Re: [Qemu-devel] [PATCH v2 0/3] pc-bios: Use iPXE ROMs
On Mon, 2011-04-11 at 13:57 -0600, Alex Williamson wrote: On Mon, 2011-04-11 at 14:48 -0500, Anthony Liguori wrote: On 04/11/2011 02:35 PM, Alex Williamson wrote: This series replaces our current gPXE based PXE ROMs with iPXE versions from the iPXE project (http://ipxe.org). This version adds ipxe to our submodules so it can be easily included in releases. I'm still including a script for updating these, perhaps someone better with Makefiles can eventually adopt this to a build target. This email series is mainly for reference, there's too much renaming and replacing binary files to send out to the mailing list. I'll strip out the binaries here so the rest can be reviewed. For the real code, please pull: git://github.com/awilliam/qemu.git (ipxe branch) Thanks to Anthony for already setting up an ipxe mirror. Thanks, Looks good to me. How different is this from what we've been shipping? Have you tested PXE boot from the builtin TFTP server and from an external one (like dnsmasq)? We were shipping v0.9.9, which was tagged 10/2009. There's been a gpxe v1.0.0 release since then, plus the split between ipxe and gpxe. I think Michael is hoping to have a release soon, but the code feels pretty stable to me as is. I've tested external booting from dhcp/tftp server for all the NICs. I'll make a pass through testing with the builtin server and report back. Thanks, Check, tested virtio-net-pci, e1000, rtl8139, ne2k_pci, pcnet, and i82550 using builtin server, with bootfile and tftp loaded kernel/initrd. All work. Thanks, Alex
[Qemu-devel] [PATCH v2 2/3] Poll main loop after I/O events were received
From: Jan Kiszka jan.kis...@siemens.com Polling until select returns empty fdsets helps to reduce the switches between iothread and vcpus. The benefit of this patch is best visible when running an SMP guest on an SMP host in emulation mode. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- sysemu.h |2 +- vl.c | 12 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/sysemu.h b/sysemu.h index bbbd0fd..f75a03a 100644 --- a/sysemu.h +++ b/sysemu.h @@ -87,7 +87,7 @@ void cpu_synchronize_all_post_init(void); void qemu_announce_self(void); -void main_loop_wait(int nonblocking); +int main_loop_wait(int nonblocking); bool qemu_savevm_state_blocked(Monitor *mon); int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable, diff --git a/vl.c b/vl.c index b7bbed8..2c46cd9 100644 --- a/vl.c +++ b/vl.c @@ -1286,7 +1286,7 @@ void qemu_system_vmstop_request(int reason) qemu_notify_event(); } -void main_loop_wait(int nonblocking) +int main_loop_wait(int nonblocking) { fd_set rfds, wfds, xfds; int ret, nfds; @@ -1333,6 +1333,7 @@ void main_loop_wait(int nonblocking) them. */ qemu_bh_poll(); +return ret; } #ifndef CONFIG_IOTHREAD @@ -1350,7 +1351,8 @@ qemu_irq qemu_system_powerdown; static void main_loop(void) { -bool nonblocking = false; +bool nonblocking; +int last_io = 0; #ifdef CONFIG_PROFILER int64_t ti; #endif @@ -1359,7 +1361,9 @@ static void main_loop(void) qemu_main_loop_start(); for (;;) { -#ifndef CONFIG_IOTHREAD +#ifdef CONFIG_IOTHREAD +nonblocking = !kvm_enabled() last_io 0; +#else nonblocking = cpu_exec_all(); if (vm_request_pending()) { nonblocking = true; @@ -1368,7 +1372,7 @@ static void main_loop(void) #ifdef CONFIG_PROFILER ti = profile_getclock(); #endif -main_loop_wait(nonblocking); +last_io = main_loop_wait(nonblocking); #ifdef CONFIG_PROFILER dev_time += profile_getclock() - ti; #endif -- 1.7.1
Re: [Qemu-devel] tcg/tcg.c:1892: tcg fatal error
On Mon, Apr 11, 2011 at 5:16 AM, Igor Kovalenko igor.v.kovale...@gmail.com wrote: On Mon, Apr 11, 2011 at 12:00 AM, Artyom Tarasenko atar4q...@gmail.com wrote: On Sun, Apr 10, 2011 at 9:41 PM, Igor Kovalenko igor.v.kovale...@gmail.com wrote: On Sun, Apr 10, 2011 at 11:37 PM, Artyom Tarasenko atar4q...@gmail.com wrote: On Sun, Apr 10, 2011 at 8:52 PM, Igor Kovalenko igor.v.kovale...@gmail.com wrote: On Sun, Apr 10, 2011 at 10:35 PM, Artyom Tarasenko atar4q...@gmail.com wrote: On Sun, Apr 10, 2011 at 7:57 PM, Blue Swirl blauwir...@gmail.com wrote: On Sun, Apr 10, 2011 at 8:48 PM, Artyom Tarasenko atar4q...@gmail.com wrote: On Sun, Apr 10, 2011 at 4:44 PM, Blue Swirl blauwir...@gmail.com wrote: On Sun, Apr 10, 2011 at 5:09 PM, Artyom Tarasenko atar4q...@gmail.com wrote: On Sun, Apr 10, 2011 at 3:24 PM, Aurelien Jarno aurel...@aurel32.net wrote: On Sun, Apr 10, 2011 at 02:29:59PM +0200, Artyom Tarasenko wrote: Trying to boot some proprietary OS I get qemu-system-sparc64 crash with a tcg/tcg.c:1892: tcg fatal error error message. It looks like it can be a platform independent bug though, because when a '-singlestep' option IS present, qemu doesn't crash and seems to translate the code properly. (gdb) bt #0 0x0032c2e327f5 in raise () from /lib64/libc.so.6 #1 0x0032c2e33fd5 in abort () from /lib64/libc.so.6 #2 0x0051933d in tcg_reg_alloc_call (s=value optimized out, def=0x89d340, opc=INDEX_op_call, args=0x10acc98, dead_iargs=3) at qemu/tcg/tcg.c:1892 #3 0x0051a557 in tcg_gen_code_common (s=0x10b8940, gen_code_buf=0x40338b60 I\213n@H\213] 3\355I\211\256\220) at qemu/tcg/tcg.c:2099 #4 tcg_gen_code (s=0x10b8940, gen_code_buf=0x40338b60 I\213n@H\213] 3\355I\211\256\220) at qemu/tcg/tcg.c:2142 #5 0x004d38f1 in cpu_sparc_gen_code (env=0x10cce10, tb=0x7fffe91bc218, gen_code_size_ptr=0x7fffd9b4) at qemu/translate-all.c:93 #6 0x004d1fd7 in tb_gen_code (env=0x10cce10, pc=18868776, cs_base=18868780, flags=15, cflags=0) at qemu/exec.c:989 #7 0x004d4029 in tb_find_slow (env1=value optimized out) at qemu/cpu-exec.c:167 #8 tb_find_fast (env1=value optimized out) at cpu-exec.c:194 #9 cpu_sparc_exec (env1=value optimized out) at qemu/cpu-exec.c:556 #10 0x00408868 in tcg_cpu_exec () at qemu/cpus.c:1066 #11 cpu_exec_all () at qemu/cpus.c:1102 #12 0x0053c756 in main_loop (argc=value optimized out, argv=value optimized out, envp=value optimized out) at qemu/vl.c:1430 I inspected ts-val_type causing the abort() case and it turned out to be 0. The last lines of qemu.log (without -singlestep) IN: 0x011fe9f0: rdpr %pstate, %g1 0x011fe9f4: wrpr %g1, 2, %pstate -- IN: 0x011fe9f8: ldub [ %o0 ], %o1 0x011fe9fc: mov %o1, %o2 0x011fea00: rdpr %tick, %o3 0x011fea04: cmp %o1, %o2 0x011fea08: be %icc, 0x11fea00 0x011fea0c: ldub [ %o0 ], %o2 Search PC... Search PC... Search PC... Search PC... Search PC... Search PC... -- IN: 0x011fe9f8: ldub [ %o0 ], %o1 0x011fe9fc: mov %o1, %o2 0x011fea00: rdpr %tick, %o3 0x011fea04: cmp %o1, %o2 0x011fea08: be %icc, 0x11fea00 0x011fea0c: ldub [ %o0 ], %o2 110521: Data Access MMU Miss (v=0068) pc=011fe9f8 npc=011fe9fc SP=0180ae41 pc: 011fe9f8 npc: 011fe9fc IN: 0x011fea00: rdpr %tick, %o3 0x011fea04: cmp %o1, %o2 0x011fea08: be %icc, 0x11fea00 0x011fea0c: ldub [ %o0 ], %o2 -- IN: 0x011fea10: brz,pn %o2, 0x11fe9f8 0x011fea14: mov %o2, %o4 -- IN: 0x011fea18: rdpr %tick, %o5 0x011fea1c: cmp %o2, %o4 0x011fea20: be %icc, 0x11fea18 0x011fea24: ldub [ %o0 ], %o4 -- IN: 0x011fea28: brz,pn %o4, 0x11fe9f4 0x011fea2c: wrpr %g0, %g1, %pstate EOF The crash is 100% reproducible and happens always on the same place, so it's probably a pure TCG issue, not related on getting the external/timer interrupts. Do you need any additional info? What would be interesting would be to get the corresponding TCG code from qemu.log (-d op,op_opt). OP: 0x11fea28 ld_i64 tmp6,regwptr,$0x20 movi_i64 cond,$0x0 movi_i64 tmp8,$0x0 brcond_i64 tmp6,tmp8,ne,$0x0 movi_i64 cond,$0x1 set_label $0x0 0x11fea2c movi_i64 tmp7,$0x0 xor_i64 tmp0,tmp7,g1 movi_i64 pc,$0x11fea2c movi_i64 tmp8,$compute_psr call tmp8,$0x0,$0 movi_i64 tmp8,$0x0 brcond_i64 cond,tmp8,eq,$0x1 movi_i64 npc,$0x11fe9f4 br $0x2 set_label $0x1 movi_i64 npc,$0x11fea30 set_label $0x2 movi_i64 tmp8,$wrpstate call tmp8,$0x0,$0,tmp0 mov_i64 pc,npc movi_i64 tmp8,$0x4 add_i64 npc,npc,tmp8 exit_tb $0x0 OP after liveness analysis: 0x11fea28 ld_i64
Re: [Qemu-devel] Slow PXE boot in qemu.git (fast in qemu-kvm.git)
On Mon, 2011-04-11 at 22:04 +0200, Jan Kiszka wrote: On 2011-04-11 21:15, Luiz Capitulino wrote: On Mon, 11 Apr 2011 13:00:32 -0600 Alex Williamson alex.william...@redhat.com wrote: On Mon, 2011-04-11 at 15:35 -0300, Luiz Capitulino wrote: On Fri, 08 Apr 2011 19:50:57 -0500 Anthony Liguori anth...@codemonkey.ws wrote: On 04/08/2011 06:25 PM, Luiz Capitulino wrote: Hi there, Summary: - PXE boot in qemu.git (HEAD f124a41) is quite slow, more than 5 minutes. Got the problem with e1000, virtio and rtl8139. However, pcnet *works* (it's as fast as qemu-kvm.git) - PXE boot in qemu-kvm.git (HEAD df85c051) is fast, less than a minute. Tried with e1000, virtio and rtl8139 (I don't remember if I tried with pcnet) I was having this problem too, but I think it's because I forgot to build qemu with --enable-io-thread, which is the default for qemu-kvm. Can you re-configure and build with that and see if it's fast? Thanks, Yes, nice catch, it's faster with I/O thread enabled, even seem faster than qemu-kvm.git. What's the performance under qemu-kvm with -no-kvm-irqchip? So, does this have to be fixed w/o I/O thread? If it's most probably an architectural deficit of non-io-thread mode, I would say let it rest in peace. But maybe it points to a generic issues that is just magnified by non-threaded mode. I've probably been told, but forget. Why isn't io-thread enabled by default? Thanks, Alex
Re: [Qemu-devel] Slow PXE boot in qemu.git (fast in qemu-kvm.git)
On Mon, 2011-04-11 at 15:35 -0300, Luiz Capitulino wrote: On Fri, 08 Apr 2011 19:50:57 -0500 Anthony Liguori anth...@codemonkey.ws wrote: On 04/08/2011 06:25 PM, Luiz Capitulino wrote: Hi there, Summary: - PXE boot in qemu.git (HEAD f124a41) is quite slow, more than 5 minutes. Got the problem with e1000, virtio and rtl8139. However, pcnet *works* (it's as fast as qemu-kvm.git) - PXE boot in qemu-kvm.git (HEAD df85c051) is fast, less than a minute. Tried with e1000, virtio and rtl8139 (I don't remember if I tried with pcnet) I was having this problem too, but I think it's because I forgot to build qemu with --enable-io-thread, which is the default for qemu-kvm. Can you re-configure and build with that and see if it's fast? Thanks, Alex
Re: [Qemu-devel] [PATCH] target-sh4: get rid of CPU_{Float, Double}U
On 04/11/2011 08:09 AM, Peter Maydell wrote: (4) I think you should be able to write a helper function for an add as just float32 HELPER(my_float_add)(float32 a, float32 b) { return float32_add(a, b, status); } While this is a laudable goal, this will fail for hosts that pass all structures by reference. This is true of, e.g. PPC32. r~
[Qemu-devel] [Bug 757702] [NEW] Undefined instruction exception starts at offset 0x8 instead of 0x4
Public bug reported: ARMv7a has lot of undefined instruction from its instruction opcode space. This undefined instructions are very useful for replacing sensitive non-priviledged instructions of guest operating systems (virtualization). The undefined instruction exception executes at exception_base + 0x4, where exception_base can be 0x0 or 0xfff0. Currently, in qemu 0.14.0 undefined instruction fault at 0x8 offset instead of 0x4. This was not a problem with qemu 0.13.0, seems like this is a new bug. As as example, if we try to execute value 0xec019800 in qemu 0.14.0 then it should cause undefined exception at exception_base+0x4 since 0xec019800 is an undefined instruction. ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/757702 Title: Undefined instruction exception starts at offset 0x8 instead of 0x4 Status in QEMU: New Bug description: ARMv7a has lot of undefined instruction from its instruction opcode space. This undefined instructions are very useful for replacing sensitive non-priviledged instructions of guest operating systems (virtualization). The undefined instruction exception executes at exception_base + 0x4, where exception_base can be 0x0 or 0xfff0. Currently, in qemu 0.14.0 undefined instruction fault at 0x8 offset instead of 0x4. This was not a problem with qemu 0.13.0, seems like this is a new bug. As as example, if we try to execute value 0xec019800 in qemu 0.14.0 then it should cause undefined exception at exception_base+0x4 since 0xec019800 is an undefined instruction.
[Qemu-devel] [PATCH] ppc.ld: add rela.iplt and provide __rela_iplt_{start, end}
Signed-off-by: Andreas Schwab sch...@linux-m68k.org --- ppc.ld |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/ppc.ld b/ppc.ld index 5248ef1..18511ce 100644 --- a/ppc.ld +++ b/ppc.ld @@ -49,6 +49,12 @@ SECTIONS .rela.sbss2 : { *(.rela.sbss2 .rela.sbss2.* .rela.gnu.linkonce.sb2.*) } .rel.bss: { *(.rel.bss .rel.bss.* .rel.gnu.linkonce.b.*) } .rela.bss : { *(.rela.bss .rela.bss.* .rela.gnu.linkonce.b.*) } + .rela.iplt : +{ + PROVIDE_HIDDEN (__rela_iplt_start = .); + *(.rela.iplt) + PROVIDE_HIDDEN (__rela_iplt_end = .); +} .rel.plt: { *(.rel.plt) } .rela.plt : { *(.rela.plt) } .init : -- 1.7.4.4 -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different.
[Qemu-devel] [PATCH v2 1/3] Add ipxe submodule
Signed-off-by: Alex Williamson alex.william...@redhat.com --- .gitmodules |3 +++ roms/ipxe |1 + 2 files changed, 4 insertions(+), 0 deletions(-) create mode 16 roms/ipxe diff --git a/.gitmodules b/.gitmodules index 44fdd1a..7884471 100644 --- a/.gitmodules +++ b/.gitmodules @@ -7,3 +7,6 @@ [submodule roms/SLOF] path = roms/SLOF url = git://git.qemu.org/SLOF.git +[submodule roms/ipxe] + path = roms/ipxe + url = git://git.qemu.org/ipxe.git diff --git a/roms/ipxe b/roms/ipxe new file mode 16 index 000..7aee315 --- /dev/null +++ b/roms/ipxe @@ -0,0 +1 @@ +Subproject commit 7aee315f61aaf1be6d2fff26339f28a1137231a5