[Qemu-devel] [Bug 965327] Re: virtio-pci: can't reserve io 0x0000-0x001f
Scubbing our ppc64 bugs. Thanks for the update Ken, I'll close this. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/965327 Title: virtio-pci: can't reserve io 0x-0x001f Status in QEMU: New Bug description: Before 2012-03-05 I was able to successfully enable a virtio-pci block device from a sPAPR pseries ppc64 Linux guest. With the current git master branch after this date I get the following error: virtio-pci :00:00.0: device not available (can't reserve [io 0x-0x001f]) virtio-pci: probe of :00:00.0 failed with error -22 virtio-pci :00:01.0: device not available (can't reserve [io 0x-0x003f]) virtio-pci: probe of :00:01.0 failed with error -22 Full details: - command line: - ./testing/qemu/ppc64-softmmu/qemu-system-ppc64 \ -L ./testing/qemu/pc-bios \ -M pseries \ -m 1024 \ -rtc base=localtime \ -parallel none \ -netdev type=user,id=mynet0,hostfwd=tcp:127.0.0.1:9011-10.0.2.11:22 \ -device virtio-net-pci,netdev=mynet0 \ -drive file=images/suse-ppc.img,if=virtio,index=0,media=disk,cache=unsafe \ -kernel images/iso/suseboot/vmlinux \ -append "root=/dev/mapper/system-root ro audit=0 selinux=0 apparmor=0 console=tty0 console=ttyPZ0" \ -initrd images/iso/suseboot/initrd.img \ -gdb tcp::1234 -- BEFORE virtio-pci "bug/user error?" introduced: -- sPAPR memory map: RTAS : 0x3fff..3fff0013 FDT : 0x3ffe..3ffe Kernel : 0x0040..01abad7b Ramdisk : 0x01ad..02053df7 Firmware load: 0x..000d6ec0 Firmware runtime : 0x3d7e..3ffe sPAPR reset SLOF ** QEMU Starting Build Date = Mar 3 2012 21:46:40 FW Version = git-440e662879c4fc3c Press "s" to enter Open Firmware. Populating /vdevice methods Populating /vdevice/v-scsi@2000 VSCSI: Initializing VSCSI: Looking for disks SCSI ID 2 CD-ROM : "QEMU QEMU CD-ROM 1.0." Populating /vdevice/vty@3000 Populating /pci@0,0 Adapters on 00 (D) : 1af4 1000virtio [ net ] 00 0800 (D) : 1af4 1001virtio [ block ] No NVRAM common partition, re-initializing... Using default console: /vdevice/vty@3000 Detected RAM kernel at 40 (16bad7c bytes) Welcome to Open Firmware Copyright (c) 2004, 2011 IBM Corporation All rights reserved. This program and the accompanying materials are made available under the terms of the BSD License available at http://www.opensource.org/licenses/bsd-license.php Booting from memory... OF stdout device is: /vdevice/vty@3000 Preparing to boot Linux version 3.2.0-2-ppc64 (geeko@buildhost) (gcc version 4.6.2 20111212 [gcc-4_6-branch revision 18] (SUSE Linux) ) #1 SMP Wed Jan 25 10:51:08 UTC 2012 (2206a5c) Detected machine type: 0101 Max number of cores passed to firmware: 1024 (NR_CPUS = 1024) Calling ibm,client-architecture-support... not implemented couldn't open /packages/elf-loader command line: root=/dev/mapper/system-root ro audit=0 selinux=0 apparmor=0 console=tty0 console=ttyPZ0 memory layout at init: memory_limit : (16 MB aligned) alloc_bottom : 01ad alloc_top: 3000 alloc_top_hi : 4000 rmo_top : 3000 ram_top : 4000 instantiating rtas at 0x2fff... done Querying for OPAL presence... not there. boot cpu hw idx 0 copying OF device tree... Building dt strings... Building dt structure... Device tree strings 0x020e -> 0x020e0635 Device tree struct 0x020f -> 0x0210 Calling quiesce... returning from prom_init Using pSeries machine description Using 1TB segments Found initrd at 0xc1ad:0xc2053df8 bootconsole [udbg0] enabled CPU maps initialized for 1 thread per core Starting Linux PPC64 #1 SMP Wed Jan 25 10:51:08 UTC 2012 (2206a5c) - ppc64_pft_size= 0x18 physicalMemorySize= 0x4000 htab_hash_mask= 0x1 - Initializing cgroup subsys cpuset Initializing cgroup subsys cpu Linux version 3.2.0-2-ppc64 (geeko@buildhost) (gc
Re: [Qemu-devel] [question] one vhost kthread servers mulitiple tx/rx queues which blong to one virtio-net device
On 08/22/2014 10:30 AM, Zhang Haoyu wrote: > Hi, Krishna, Shirley > > How got get the latest patch of M:N Implementation of mulitiqueue, > > I am going to test the the combination of "M:N Implementation of mulitiqueue" > and "vhost: add polling mode". > > Thanks, > Zhang Haoyu > > Just FYI. You may refer Bandan Das 's cwmq patch with vhost (search "Workqueue based vhost workers") and the recent vhost polling mode patches by Razya Ladelsky.
Re: [Qemu-devel] issue: linking 64bit glib when building for cpu=i386
John Snow writes: > I was running a series of tests on 32 and 64 bit hosts to test for > endianness and variable width issues when I noticed that I couldn't > properly perform a build of "make check" against a 32bit target from a > 64bit host: > > ../../configure --cpu=i386 && make -j4 && make check > > This produces some warnings in tests-cutils about overflowing > variables that are of type guint64. It's been mentioned on the mailing > lists before, actually: > http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00452.html > > The problem is that guint64 is being aliased against "unsigned long", > which is only 4 bytes instead of the implied 8. This occurs because we > link against the 64bit headers for glib instead of the 32bit ones when > we're building for i386 from an x86_64 host. >From glibconfig.h: typedef signed char gint8; typedef unsigned char guint8; typedef signed short gint16; typedef unsigned short guint16; typedef signed int gint32; typedef unsigned int guint32; typedef signed long gint64; typedef unsigned long guint64; Anyone redoing in this day and age is a weirdo. Anyone getting it wrong is a weirdo and a fool. But getting it wrong in code expressively generated to configure for a specific platform with a perfectly working , that's in a category of weird foolishness of its own. But wait, it gets even better: typedef signed long gssize; typedef unsigned long gsize; Oh yeah, can't rely on size_t, it's been reliably available only since a decade before GLib was born! [...]
Re: [Qemu-devel] [PATCH V2] net: Fix dealing with packets when runstate changes
On 08/21/2014 08:39 PM, zhanghailiang wrote:
> For all NICs(except virtio-net) emulated by qemu,
> Such as e1000, rtl8139, pcnet and ne2k_pci,
> Qemu can still receive packets when VM is not running.
> If this happened in *migration's* last PAUSE VM stage,
> The new dirty RAM related to the packets will be missed,
> And this will lead serious network fault in VM.
>
> To avoid this, we forbid receiving packets in generic net code when
> VM is not running. Also, when the runstate changes back to running,
> we definitely need to flush queues to get packets flowing again.
Hi:
Can you describe what will happen if you don't flush queues after vm is
started? Btw, the notifier dependency and impact on vhost should be
mentioned here as well.
>
> Here we implement this in the net layer:
> (1) Judge the vm runstate in qemu_can_send_packet
> (2) Add a member 'VMChangeStateEntry *vmstate' to struct NICState,
> Which will listen for VM runstate changes.
> (3) Register a handler function for VMstate change.
> When vm changes back to running, we flush all queues in the callback function.
> (4) Remove checking vm state in virtio_net_can_receive
>
> Signed-off-by: zhanghailiang
> ---
> v2:
> - remove the superfluous check of nc->received_disabled
> ---
> hw/net/virtio-net.c | 4
> include/net/net.h | 2 ++
> net/net.c | 31 +++
> 3 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 268eff9..287d762 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -839,10 +839,6 @@ static int virtio_net_can_receive(NetClientState *nc)
> VirtIODevice *vdev = VIRTIO_DEVICE(n);
> VirtIONetQueue *q = virtio_net_get_subqueue(nc);
>
> -if (!vdev->vm_running) {
> -return 0;
> -}
> -
> if (nc->queue_index >= n->curr_queues) {
> return 0;
> }
> diff --git a/include/net/net.h b/include/net/net.h
> index ed594f9..a294277 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -8,6 +8,7 @@
> #include "net/queue.h"
> #include "migration/vmstate.h"
> #include "qapi-types.h"
> +#include "sysemu/sysemu.h"
>
> #define MAX_QUEUE_NUM 1024
>
> @@ -96,6 +97,7 @@ typedef struct NICState {
> NICConf *conf;
> void *opaque;
> bool peer_deleted;
> +VMChangeStateEntry *vmstate;
> } NICState;
>
> NetClientState *qemu_find_netdev(const char *id);
> diff --git a/net/net.c b/net/net.c
> index 6d930ea..113a37b 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -242,6 +242,28 @@ NetClientState *qemu_new_net_client(NetClientInfo *info,
> return nc;
> }
>
> +static void nic_vmstate_change_handler(void *opaque,
> + int running,
> + RunState state)
> +{
> +NICState *nic = opaque;
> +NetClientState *nc;
> +int i, queues;
> +
> +if (!running) {
> +return;
> +}
> +
> +queues = MAX(1, nic->conf->peers.queues);
> +for (i = 0; i < queues; i++) {
> +nc = &nic->ncs[i];
> +if (nc->info->can_receive && !nc->info->can_receive(nc)) {
> +continue;
> +}
Why not just using qemu_can_send_packet() here?
> +qemu_flush_queued_packets(nc);
> +}
> +}
> +
> NICState *qemu_new_nic(NetClientInfo *info,
> NICConf *conf,
> const char *model,
> @@ -259,6 +281,8 @@ NICState *qemu_new_nic(NetClientInfo *info,
> nic->ncs = (void *)nic + info->size;
> nic->conf = conf;
> nic->opaque = opaque;
> +nic->vmstate =
> qemu_add_vm_change_state_handler(nic_vmstate_change_handler,
> +nic);
>
> for (i = 0; i < queues; i++) {
> qemu_net_client_setup(&nic->ncs[i], info, peers[i], model, name,
> @@ -379,6 +403,7 @@ void qemu_del_nic(NICState *nic)
> qemu_free_net_client(nc);
> }
>
> +qemu_del_vm_change_state_handler(nic->vmstate);
> g_free(nic);
> }
>
> @@ -452,6 +477,12 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len)
>
> int qemu_can_send_packet(NetClientState *sender)
> {
> +int vmstat = runstate_is_running();
> +
> +if (!vmstat) {
> +return 0;
> +}
I think you want "vmstart" here?
> +
> if (!sender->peer) {
> return 1;
> }
Re: [Qemu-devel] [Fwd: [PATCH v4 07/21] iscsi: Handle failure for potentially large allocations]
Am 22.08.2014 um 09:35 schrieb Peter Lieven:
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
>
> This patch addresses the allocations in the iscsi block driver.
>
> Signed-off-by: Kevin Wolf
> Acked-by: Paolo Bonzini
> Reviewed-by: Benoit Canet
> Reviewed-by: Eric Blake
> ---
> block/iscsi.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 84aa22a..06afa78 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -893,7 +893,10 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState
> *bs, int64_t sector_num,
> nb_blocks = sector_qemu2lun(nb_sectors, iscsilun);
>
> if (iscsilun->zeroblock == NULL) {
> -iscsilun->zeroblock = g_malloc0(iscsilun->block_size);
> +iscsilun->zeroblock = g_try_malloc0(iscsilun->block_size);
> +if (iscsilun->zeroblock == NULL) {
> +return -ENOMEM;
> +}
> }
>
> iscsi_co_init_iscsitask(iscsilun, &iTask);
Unfortunately, I missed that one. The zeroblock is typicalls 512 Byte or 4K
depending
on the blocksize. What is significantly larger is the allocationmap. It is
typically created
on iscsi_open, but is also recreated on iscsi_truncate. I don't have the
context why this
patch was introduced, but I would vote for introducing a bitmap_try_new and
issue
a warning if the allocation fails. The allocationmap is optional we can work
without it.
If the pointer is NULL its not used.
Peter
Re: [Qemu-devel] issue: linking 64bit glib when building for cpu=i386
On 22 August 2014 01:10, John Snow wrote: > I was running a series of tests on 32 and 64 bit hosts to test for > endianness and variable width issues when I noticed that I couldn't properly > perform a build of "make check" against a 32bit target from a 64bit host: > > ../../configure --cpu=i386 && make -j4 && make check The 32-bit-on-64-bit x86 stuff is a weird special case and I'm not terribly surprised it's broken. For pretty much every other non-native build case, --cpu is only there to override configure's guesses if it guesses wrong: you just tell configure the correct compiler to use, and then it will automatically figure out it's i386 because of the compiler #defines. If you use --cross-prefix= this will tell configure to use all of the correct compiler and the correct pkgconfig, assuming your distro has set up the compiler and the pkgconfig with the right prefixes. But i386-on-x86_64 seems to be odd in that rather than just having a correct compiler for it there's the same compiler with a bunch of flags, and a pkg-config in a non-standard place. thanks -- PMM
Re: [Qemu-devel] [PATCH] Fix pflash_cfi01 to restore flash command/array state after migration
On 21 August 2014 19:48, Markus Armbruster wrote: > > Doesn't this break migration to/from unfixed versions? > It might. It is not something we've tried here, and I'm certainly not familiar enough with QEMU internals myself to say one way or another. We encountered this issue in what I can best describe as a short-and-furious proof-of-concept project, and as part of the wrapping up of this I'm going over our change logs to see if anything can be contributed back upstream. It seemed like a trivial fix in the tree, but I acknowledge it may have ramifications we didn't consider. I suspect you need a subsection. Initialize ->romd = 1, then let the > subsection override it. > And here I'm well and truly out of my depth! If someone who actually knows what they're doing would like to run with this, please do. Sorry to effectively do a dump-and-run here - I hope its a net positive at least. Regards, /Johny -- Johny Mattsson Senior Software Engineer DiUS Computing Pty. Ltd. *where ideas are engineered *
[Qemu-devel] [PATCH] block/iscsi: fix memory corruption on iscsi resize
bs->total_sectors is not yet updated at this point. resulting
in memory corruption if the volume has grown and data is written
to the newly availble areas.
CC: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven
---
block/iscsi.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index a7bb697..ed883c3 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1509,7 +1509,8 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t
offset)
if (iscsilun->allocationmap != NULL) {
g_free(iscsilun->allocationmap);
iscsilun->allocationmap =
-bitmap_new(DIV_ROUND_UP(bs->total_sectors,
+bitmap_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks,
+iscsilun),
iscsilun->cluster_sectors));
}
--
1.7.9.5
Re: [Qemu-devel] [RFC PATCH 1/9] block: Add bdrv_aio_cancel_async
Il 22/08/2014 03:23, Fam Zheng ha scritto: > What about we save cb and opaque locally, and set acb->cb to a nop. When > cancel > is done we can call the original cb? That might work but needs some auditing. Right now the AIOCB is still valid when the callback is called, and this would be changed. Also, having different semantics for cancellation vs. completion would be painful. Paolo
[Qemu-devel] [PATCH] mirror: improve io performance
From: ChenLiang
Mirror buffer is split into two pieces to improve io performance.
In this way, one piece of buffer can read data from source disk
when another one is writing data to dest disk.
previous:
io bandwidth: 41MB/s
migration time: 8min15s
now:
io bandwidth: 67MB/s
migration time: 5min3s
The size of vm image is 20G.
Signed-off-by: ChenLiang
Signed-off-by: Gonglei
---
block/mirror.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/block/mirror.c b/block/mirror.c
index 5e7a166..6596e92 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -51,6 +51,7 @@ typedef struct MirrorBlockJob {
uint8_t *buf;
QSIMPLEQ_HEAD(, MirrorBuffer) buf_free;
int buf_free_count;
+int buf_total_count;
unsigned long *in_flight_bitmap;
int in_flight;
@@ -238,6 +239,10 @@ static uint64_t coroutine_fn
mirror_iteration(MirrorBlockJob *s)
break;
}
+if (s->buf_total_count / 2 < nb_chunks + added_chunks) {
+break;
+}
+
/* We have enough free space to copy these sectors. */
bitmap_set(s->in_flight_bitmap, next_chunk, added_chunks);
@@ -307,6 +312,7 @@ static void mirror_free_init(MirrorBlockJob *s)
buf_size -= granularity;
buf += granularity;
}
+s->buf_total_count = s->buf_free_count;
}
static void mirror_drain(MirrorBlockJob *s)
--
1.7.12.4
Re: [Qemu-devel] issue: linking 64bit glib when building for cpu=i386
On Thu, Aug 21, 2014 at 08:10:54PM -0400, John Snow wrote: > I was running a series of tests on 32 and 64 bit hosts to test for > endianness and variable width issues when I noticed that I couldn't properly > perform a build of "make check" against a 32bit target from a 64bit host: > > ../../configure --cpu=i386 && make -j4 && make check > > This produces some warnings in tests-cutils about overflowing variables that > are of type guint64. It's been mentioned on the mailing lists before, > actually: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00452.html > > The problem is that guint64 is being aliased against "unsigned long", which > is only 4 bytes instead of the implied 8. This occurs because we link > against the 64bit headers for glib instead of the 32bit ones when we're > building for i386 from an x86_64 host. > > Our include flags wind up looking like: -I/usr/include/glib-2.0 but > -I/usr/lib64/glib-2.0/include > > I was discussing the problem with Stefan: > > On 08/21/2014 05:03 AM, Stefan Hajnoczi wrote: > >The problem is that pkg-config uses libdir=/usr/lib64 by default on > >amd64 hosts. It doesn't know that gcc -m32 is being used. > > > >This results in glib's 64-bit headers being used where guint64 is just > >unsigned long. On 32-bit hosts this is incorrect. > > > >Two workarounds: > > > >1. yum install pkgconfig.i686 and run it instead of pkgconfig.x86_64 > > > >2. Use the pkg-config --define-variable libdir=/usr/lib option > > > >You can set PKG_CONFIG=path/to/pkg-config.i686 on QEMU's ./configure > >command-line. > > > >This is all distro-specific :(. Any other solutions? > > > >Stefan > > > > I am not extremely well versed in configure or pkg-config ninjutsu, but I > must imagine that the ARCH/cpu variables we are setting in configure could > help us know to call the 32bit pkg-config instead of the native 64bit > version and fix this issue. > > Does anyone have any good ideas? Surely other projects must have run into > this elsewhere. Distros will install pkg-config .pc files for non-native architectures in a different location normally. The supported / recommended way to tell pkg-config to look in these alternative dirs is to set the env variable PKG_CONFIG_LIBDIR. This replaces the built-in default search directory that looks for native. So on a Fedora / RHELL system, to make pkg-config use 32-bit libs you want to set PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig which replaces the default location of /usr/lib64/pkgconfig. This is the same thing you'd need to do to build QEMU for say, mingw32 where you must set something like PKG_CONFIG_LIBDIR=/usr/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig/ Note, i say PKG_CONFIG_LIBDIR here, *not* PKG_CONFIG_PATH. The latter variable adds the default search path - you want to stop it looking in the default search path completely because it is the wrong arch, so must use PKG_CONFIG_LIBDIR Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PATCH] linux-user: fix file descriptor leaks
Handle variable "fd_orig" going out of scope leaks the handle.
Signed-off-by: zhanghailiang
---
linux-user/syscall.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a50229d..11a48c2 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5082,6 +5082,7 @@ static int open_self_cmdline(void *cpu_env, int fd)
if (word_skipped) {
if (write(fd, cp_buf, nb_read) != nb_read) {
+close(fd_orig);
return -1;
}
}
--
1.7.12.4
[Qemu-devel] adding a parameter in qemu tap option
Hi,
I'm trying to understand how to add a parameter in tap option.
I currently have a working config for a vm; i.e several interfaces using
multiqueues.
I had a try with:
--- a/qapi-schema.json
+++ b/qapi-schema.json
-2006,6 +2008,7 @@
##
{ 'type': 'NetdevTapOptions',
'data': {
+ '*foo': 'str',
'*ifname': 'str',
'*fd': 'str',
'*fds': 'str',
i.e just adding a foo parameter; nothing change in my config file (I'm
using -readconfig option and not cmdline)
multiqueue is no longer working in some cases.
I don't get how is it possible. I don't even understand why I don't have
the same behaviour using the cmdline.
Maybe I forgot something obvious.
Does someone has some hints?
Thanks,
--
William
signature.asc
Description: Digital signature
Re: [Qemu-devel] issue: linking 64bit glib when building for cpu=i386
On 22 August 2014 09:20, Daniel P. Berrange wrote: > Distros will install pkg-config .pc files for non-native architectures > in a different location normally. The supported / recommended way to > tell pkg-config to look in these alternative dirs is to set the env > variable PKG_CONFIG_LIBDIR. This replaces the built-in default search > directory that looks for native. > > So on a Fedora / RHELL system, to make pkg-config use 32-bit libs you > want to set > >PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig > > which replaces the default location of /usr/lib64/pkgconfig. This is > the same thing you'd need to do to build QEMU for say, mingw32 where > you must set something like > >PKG_CONFIG_LIBDIR=/usr/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig/ Yes, but this should be done by the i686-w64-mingw32-pkg-config wrapper IMHO. (That's how I have my mingw setup configured, anyway.) > Note, i say PKG_CONFIG_LIBDIR here, *not* PKG_CONFIG_PATH. The latter > variable adds the default search path - you want to stop it looking in > the default search path completely because it is the wrong arch, so > must use PKG_CONFIG_LIBDIR Interestingly, Debian's cross-compile pkg-config wrapper (aarch64-linux-gnu-pkg-config etc) sets PKG_CONFIG_PATH, not PKG_CONFIG_LIBDIR. Maybe that's a bug, but it works... thanks -- PMM
Re: [Qemu-devel] issue: linking 64bit glib when building for cpu=i386
On Fri, Aug 22, 2014 at 09:28:05AM +0100, Peter Maydell wrote: > On 22 August 2014 09:20, Daniel P. Berrange wrote: > > Distros will install pkg-config .pc files for non-native architectures > > in a different location normally. The supported / recommended way to > > tell pkg-config to look in these alternative dirs is to set the env > > variable PKG_CONFIG_LIBDIR. This replaces the built-in default search > > directory that looks for native. > > > > So on a Fedora / RHELL system, to make pkg-config use 32-bit libs you > > want to set > > > >PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig > > > > which replaces the default location of /usr/lib64/pkgconfig. This is > > the same thing you'd need to do to build QEMU for say, mingw32 where > > you must set something like > > > >PKG_CONFIG_LIBDIR=/usr/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig/ > > Yes, but this should be done by the i686-w64-mingw32-pkg-config > wrapper IMHO. (That's how I have my mingw setup configured, > anyway.) Sure if your distro wants to rebuild the pkg-config binary for each arch that is a valid approach too, but it is by no means required in order to get non-native builds working. > > Note, i say PKG_CONFIG_LIBDIR here, *not* PKG_CONFIG_PATH. The latter > > variable adds the default search path - you want to stop it looking in > > the default search path completely because it is the wrong arch, so > > must use PKG_CONFIG_LIBDIR > > Interestingly, Debian's cross-compile pkg-config wrapper > (aarch64-linux-gnu-pkg-config etc) sets PKG_CONFIG_PATH, > not PKG_CONFIG_LIBDIR. Maybe that's a bug, but it works... Yes, that would be a bug. Consider if you have mistakenly only installed the 64-bit dev package for libusb and not the 32-bit package. Now the 'libusb-1.0.pc' file will only be present in the directory /usr/lib64/pkgconfig but not in /usr/lib/pkgconfig. If you used PKG_CONFIG_PATH to point to the 32-bit directory, then it won't find the 32-bit libusb-1.0, so will fallback to looking in the 64-bit directory and repo the 64-bit version which is definitely not what you want. That'll lead to pain & suffering with wierd build and/or link errors. If you used PKG_CONFIG_LIBDIR then you'll get an immediate error from pkg-config telling you the 32-bit libusb-1.0 package was not installed. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v3 1/2] hw/misc/arm_sp810: Create SP810 device
On 19 Aug 2014, at 16:03, Peter Maydell wrote:
> On 17 August 2014 15:24, Fabian Aggeler wrote:
>> This adds a device model for the PrimeXsys System Controller (SP810)
>> which is present in the Versatile Express motherboards. It is
>> so far read-only but allows to read the SCCTRL register.
>>
>> Signed-off-by: Fabian Aggeler
>> ---
>> default-configs/arm-softmmu.mak | 1 +
>> hw/misc/Makefile.objs | 1 +
>> hw/misc/arm_sp810.c | 109
>>
>> include/hw/misc/arm_sp810.h | 80 +
>> 4 files changed, 191 insertions(+)
>> create mode 100644 hw/misc/arm_sp810.c
>> create mode 100644 include/hw/misc/arm_sp810.h
>>
>> diff --git a/default-configs/arm-softmmu.mak
>> b/default-configs/arm-softmmu.mak
>> index f3513fa..67ba99a 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -55,6 +55,7 @@ CONFIG_PL181=y
>> CONFIG_PL190=y
>> CONFIG_PL310=y
>> CONFIG_PL330=y
>> +CONFIG_SP810=y
>> CONFIG_CADENCE=y
>> CONFIG_XGMAC=y
>> CONFIG_EXYNOS4=y
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index 979e532..49d023b 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -13,6 +13,7 @@ common-obj-$(CONFIG_PL310) += arm_l2x0.o
>> common-obj-$(CONFIG_INTEGRATOR_DEBUG) += arm_integrator_debug.o
>> common-obj-$(CONFIG_A9SCU) += a9scu.o
>> common-obj-$(CONFIG_ARM11SCU) += arm11scu.o
>> +common-obj-$(CONFIG_SP810) += arm_sp810.o
>
> We mostly don't use the 'arm' prefix (compare pl330, pl011,
> and other examples for other architectures), so you can
> remove it from filenames and variable names here.
Ok, will remove it then.
>
>>
>> # PKUnity SoC devices
>> common-obj-$(CONFIG_PUV3) += puv3_pm.o
>> diff --git a/hw/misc/arm_sp810.c b/hw/misc/arm_sp810.c
>> new file mode 100644
>> index 000..7aff9bd
>> --- /dev/null
>> +++ b/hw/misc/arm_sp810.c
>> @@ -0,0 +1,109 @@
>> +/*
>> + * ARM PrimeXsys System Controller (SP810)
>> + *
>> + * Copyright (C) 2014 Fabian Aggeler
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include "hw/misc/arm_sp810.h"
>> +
>> +static const VMStateDescription vmstate_arm_sysctl = {
>
> ^^^ should be vmstate_sp810.
Thanks for catching this. I will change it.
>
>> +.name = "arm_sp810",
>> +.version_id = 1,
>> +.minimum_version_id = 1,
>> +.fields = (VMStateField[]) {
>> +VMSTATE_UINT8(timeren0_sel, arm_sp810_state),
>> +VMSTATE_UINT8(timeren1_sel, arm_sp810_state),
>> +VMSTATE_UINT8(timeren2_sel, arm_sp810_state),
>> +VMSTATE_UINT8(timeren3_sel, arm_sp810_state),
>
> These are read only properties, so you don't need to save them.
Okay.
>
>> +VMSTATE_END_OF_LIST()
>> +}
>> +};
>> +
>> +static uint64_t arm_sp810_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> +arm_sp810_state *s = opaque;
>> +
>> +switch (offset) {
>> +case SCCTRL:
>> +return (s->timeren3_sel << 21) | (s->timeren2_sel << 19)
>> +| (s->timeren1_sel << 17) | (s->timeren0_sel << 15);
>> +default:
>> +qemu_log_mask(LOG_UNIMP,
>> +"arm_sp810_read: Register not yet implemented: %s\n",
>> +HWADDR_PRIx);
>> +return 0;
>> +}
>> +}
>> +
>> +static void arm_sp810_write(void *opaque, hwaddr offset,
>> +uint64_t val, unsigned size)
>> +{
>> +switch (offset) {
>> +default:
>> +qemu_log_mask(LOG_UNIMP,
>> +"arm_sp810_write: Register not yet implemented: %s\n",
>> +HWADDR_PRIx);
>> +return;
>> +}
>> +}
>> +
>> +static const MemoryRegionOps arm_sp810_ops = {
>> +.read = arm_sp810_read,
>> +.write = arm_sp810_write,
>> +.endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static void arm_sp810_init(Object *obj)
>> +{
>> +arm_sp810_state *s = ARM_SP810(obj);
>> +
>> +memory_region_init_io(&s->iomem, obj, &arm_sp810_ops, s, "arm-sp810",
>> + 0x1000);
>> +sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
>> +}
>> +
>> +static Property arm_sp810_properties[] = {
>> +DEFINE_PROP_UINT8("timeren0-sel", arm_sp810_state, timeren0_sel,
>> + SCCTRL_TIMER_REFCLK),
>> +DEFINE_PROP_UINT8("timeren1-sel", arm_sp810_state, timeren1_sel,
>> + SCCTRL_TIMER_REFCLK),
>> +DE
Re: [Qemu-devel] [Fwd: [PATCH v4 07/21] iscsi: Handle failure for potentially large allocations]
Am 22.08.2014 um 09:40 hat Peter Lieven geschrieben:
> Am 22.08.2014 um 09:35 schrieb Peter Lieven:
> > Some code in the block layer makes potentially huge allocations. Failure
> > is not completely unexpected there, so avoid aborting qemu and handle
> > out-of-memory situations gracefully.
> >
> > This patch addresses the allocations in the iscsi block driver.
> >
> > Signed-off-by: Kevin Wolf
> > Acked-by: Paolo Bonzini
> > Reviewed-by: Benoit Canet
> > Reviewed-by: Eric Blake
> > ---
> > block/iscsi.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/iscsi.c b/block/iscsi.c
> > index 84aa22a..06afa78 100644
> > --- a/block/iscsi.c
> > +++ b/block/iscsi.c
> > @@ -893,7 +893,10 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState
> > *bs, int64_t sector_num,
> > nb_blocks = sector_qemu2lun(nb_sectors, iscsilun);
> >
> > if (iscsilun->zeroblock == NULL) {
> > -iscsilun->zeroblock = g_malloc0(iscsilun->block_size);
> > +iscsilun->zeroblock = g_try_malloc0(iscsilun->block_size);
> > +if (iscsilun->zeroblock == NULL) {
> > +return -ENOMEM;
> > +}
> > }
> >
> > iscsi_co_init_iscsitask(iscsilun, &iTask);
>
> Unfortunately, I missed that one. The zeroblock is typicalls 512 Byte or 4K
> depending
> on the blocksize.
I don't remember the details, but I think when I went through all
drivers, I couldn't convince myself that a reasonable block size is
enforced somewhere. So I just went ahead and converted the call to be on
the safe side. It can never hurt anyway.
> What is significantly larger is the allocationmap. It is typically created
> on iscsi_open, but is also recreated on iscsi_truncate. I don't have the
> context why this
> patch was introduced, but I would vote for introducing a bitmap_try_new and
> issue
> a warning if the allocation fails. The allocationmap is optional we can work
> without it.
> If the pointer is NULL its not used.
Right, that one I missed because it doesn't directly use g_malloc().
Your proposal sounds good to me. Are you going to prepare a patch?
Kevin
Re: [Qemu-devel] [PATCH] linux-user: fix file descriptor leaks
> -Original Message-
> From: qemu-devel-bounces+arei.gonglei=huawei@nongnu.org
> [mailto:qemu-devel-bounces+arei.gonglei=huawei@nongnu.org] On
> Behalf Of zhanghailiang
> Sent: Friday, August 22, 2014 4:24 PM
> To: qemu-devel@nongnu.org
> Cc: qemu-triv...@nongnu.org; riku.voi...@iki.fi; Luonengjun; Huangpeng
> (Peter); Zhanghailiang
> Subject: [Qemu-devel] [PATCH] linux-user: fix file descriptor leaks
>
> Handle variable "fd_orig" going out of scope leaks the handle.
>
> Signed-off-by: zhanghailiang
> ---
> linux-user/syscall.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index a50229d..11a48c2 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5082,6 +5082,7 @@ static int open_self_cmdline(void *cpu_env, int fd)
>
> if (word_skipped) {
> if (write(fd, cp_buf, nb_read) != nb_read) {
> +close(fd_orig);
> return -1;
> }
> }
> --
> 1.7.12.4
>
>
Reviewed-by: Gonglei
Best regards,
-Gonglei
Re: [Qemu-devel] issue: linking 64bit glib when building for cpu=i386
On 22 August 2014 09:34, Daniel P. Berrange wrote: > On Fri, Aug 22, 2014 at 09:28:05AM +0100, Peter Maydell wrote: >> Yes, but this should be done by the i686-w64-mingw32-pkg-config >> wrapper IMHO. (That's how I have my mingw setup configured, >> anyway.) > > Sure if your distro wants to rebuild the pkg-config binary for each > arch that is a valid approach too, but it is by no means required > in order to get non-native builds working. It's just a wrapper script, not a complete fresh binary. The point is that all the compilation tools needed for a build for $TRIPLET are $TRIPLET-toolname. That's what QEMU's configure script expects and it's easy for users. >> Interestingly, Debian's cross-compile pkg-config wrapper >> (aarch64-linux-gnu-pkg-config etc) sets PKG_CONFIG_PATH, >> not PKG_CONFIG_LIBDIR. Maybe that's a bug, but it works... > > Yes, that would be a bug. Consider if you have mistakenly only installed > the 64-bit dev package for libusb and not the 32-bit package. Now the > 'libusb-1.0.pc' file will only be present in the directory > /usr/lib64/pkgconfig but not in /usr/lib/pkgconfig. Mmm, this does seem to be a bug. I'll see if it's been reported to Debian yet... -- PMM
Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
> From: Konrad Rzeszutek Wilk [mailto:konrad.w...@oracle.com]
> Sent: Thursday, August 21, 2014 6:31 AM
> To: Fabio Fantoni
> Cc: Ross Philipson; Ian Campbell; Paul Durrant; ke...@koconnor.net;
> Huangweidong (C); Hanweidong (Randy); m...@redhat.com;
> qemu-devel@nongnu.org; xen-de...@lists.xen.org;
> johannes.kra...@googlemail.com; Gonglei (Arei); Stefano Stabellini; Gaowei
> (UVP); Jan Beulich; Anthony Perard
> Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0
> methods for PCIslots that support hotplug by runtime patching
>
> On Wed, Aug 20, 2014 at 02:11:48PM +0200, Fabio Fantoni wrote:
> > Il 12/05/2014 16:32, Ross Philipson ha scritto:
> > >On 05/12/2014 05:05 AM, Ian Campbell wrote:
> > >>On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:
> > >>>On 05/09/2014 12:34 PM, Paul Durrant wrote:
> > >-Original Message-
> > >From: Ian Campbell
> > >Sent: 09 May 2014 17:12
> > >To: Konrad Rzeszutek Wilk
> > >Cc: Ross Philipson; ke...@koconnor.net; Huangweidong (C);
> Hanweidong
> > >(Randy); m...@redhat.com; qemu-devel@nongnu.org; xen-
> > >de...@lists.xen.org; fabio.fant...@m2r.biz;
> > >johannes.kra...@googlemail.com; Gonglei (Arei); Stefano Stabellini;
> > >Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
> > >Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only
> > >supply
> > >_EJ0 methods for PCIslots that support hotplug by runtime patching
> >
> > Ping...
> > Are there any news about this patch?
>
> I think we are waiting on the patch submitter to do some homework
> and reimplement the patch based on our feedback.
> >
I' m so sorry. It's so long time.
And this work is not a top job for me right now.
Best regards,
-Gonglei
> > Thanks for any reply.
> >
> > >
> > >On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:
> > >
> > >>So we could just then gat the _EJ0 functionality based on values
> > >>that
> > >>are present (or not) in the SSDT ?
> > >
> > >AIUI the very presence of _EJ0 is what marks the device as being
> > >ejectable (e.g. in the Windows device manager).
> > >
> > >It would be possible to make _EJ0 conditionally turn itself into a
> > >NOP
> > >without resorting to an SSDT, but I don't think that solves the issue
> > >they are trying to solve, which is that the user can even try to
> > >eject
> > >an non-hotplug device. (grep for UAR1 in our dsdt.asl and
> > >acpi_info->com1_present in hvmloader/acpi/build.c for an example
> > >of this
> > >sort of conditional thing)
> > >
> > >>>
> > >>>Going back to the SSDT idea. A little poking around and what not and I
> > >>>came up with something like this that I build into an SSDT:
> > >>>
> > >>>DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
> > >>>{
> > >>> /* S00 device is defined in DSDT, this allows me to
> > >>> * refrence it in this SSDT
> > >>> */
> > >>> External (\_SB.PCI0.S00, DeviceObj)
> > >>>
> > >>> ...
> > >>>
> > >>> /* Extend the functionality of S00 */
> > >>> Scope ( \_SB.PCI0.S00 ) {
> > >>> Method(_EJ0, 1, NotSerialized)
> > >>> {
> > >>> /* Do stuffs here */
> > >>> }
> > >>> }
> > >>>}
> > >>
> > >>Thanks, this looks like the sort of thing I was naively imagining would
> > >>be possible.
> > >>
> > >>>So I did find some examples of this after all in my pile of ACPI
> > >>>firmware snapshots from all our supported platforms.
> > >>
> > >>Thanks (none of the machines I looked at had PCI hotplug apparently). I
> > >>was curious to know how Real Firmware Engineers(tm) dealt with this sort
> > >>of issue.
> > >>
> > >>I was worried how real life OSPMs might interpret this method being in
> > >>an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
> > >>that real firmware does this seem to suggest that at least Windows
> > >>treats it that way (which is a relief).
> > >
> > >I did actually find SSDTs that were specifically adding an _EJ0 to a
> > >device scope for a device defined externally. I attached an example from a
> > >Fujitsu system I have. The PRT1 device on SAT0 is external:
> > >
> > >External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)
> > >
> > >And _EJ0 is added to the scope.
> > >
> > >>
> > >>> I think this would
> > >>>work allowing you to just add or not add _EJ0 methods to the PCI
> > >>>devices
> > >>>you want by either using different SSDTs or doing something to generate
> > >>>or munge the SSDT at runtime (which would be simpler than messing with
> > >>>the DSDT I think.
> > >>
> > >>Without filling out the body of _EJ0 (which I tried but failed to do)
> > >>your stub compiles to 60 bytes of AML, I suppose that even having filled
> > >>in _EJ0 in the result would be less than, say, 128 bytes.
> > >>
> > >>Given that there are 32 PCI slots we would be talking about a total of
> > >>4k of space in hvmlo
Re: [Qemu-devel] [PATCH v3 1/2] hw/misc/arm_sp810: Create SP810 device
On 22 August 2014 09:38, Aggeler Fabian wrote: > > On 19 Aug 2014, at 16:03, Peter Maydell wrote: >> Which input signals in the hardware do these properties >> correspond to? I can't figure out what the mapping is. >> As far as I can tell from the documentation the bits >> in SCCTRL are just always reset to 0 and are not >> controlled by external signals (which is what qdev >> properties should typically correspond to). > > Actually I don’t know how it is done in hardware. My goal was > to reflect the default speed of the SP804 timer in QEMU’s vexpress > emulation in the SCCTRL. What do you suggest in this case? Well, fundamentally you have to find out what the hardware does, and do that. We can't model anything else. > I could remove the QOM properties and set the reset value of the > TimerEnXSel bits in SCCTRL to 1 (TIMCLK) to match the SCCTRL > with the speed of the SP804. The h/w docs suggest the reset value is 0. I have a feeling you may be attempting to make QEMU's hardware model include behaviour which is the result of firmware code setting register values before booting the kernel... thanks -- PMM
Re: [Qemu-devel] [PATCH v2] scsi-generic: remove superfluous DPRINTF avoid to break compiling
Il 22/08/2014 04:01, arei.gong...@huawei.com ha scritto:
> From: Gonglei
>
> variables lun and tag had been eliminated, break compiling
> when enable debug switch. Meanwhile traces provide the same
> information with this DPRINTF, so remove it.
>
> Signed-off-by: Gonglei
> ---
> v2:
> - as Paolo's suggestion, remove the superfluous DPRINTF.
> - change patch topic and commit message.
> ---
> hw/scsi/scsi-generic.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
> index 0b2ff90..0b352fc 100644
> --- a/hw/scsi/scsi-generic.c
> +++ b/hw/scsi/scsi-generic.c
> @@ -303,9 +303,6 @@ static int32_t scsi_send_command(SCSIRequest *req,
> uint8_t *cmd)
> SCSIDevice *s = r->req.dev;
> int ret;
>
> -DPRINTF("Command: lun=%d tag=0x%x len %zd data=0x%02x", lun, tag,
> -r->req.cmd.xfer, cmd[0]);
> -
> #ifdef DEBUG_SCSI
> {
> int i;
>
Applied, thanks.
Paolo
Re: [Qemu-devel] [PATCH] block/iscsi: fix memory corruption on iscsi resize
Am 22.08.2014 um 10:08 hat Peter Lieven geschrieben: > bs->total_sectors is not yet updated at this point. resulting > in memory corruption if the volume has grown and data is written > to the newly availble areas. > > CC: qemu-sta...@nongnu.org > Signed-off-by: Peter Lieven Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [Fwd: [PATCH v4 07/21] iscsi: Handle failure for potentially large allocations]
Am 22.08.2014 um 10:42 schrieb Kevin Wolf:
> Am 22.08.2014 um 09:40 hat Peter Lieven geschrieben:
>> Am 22.08.2014 um 09:35 schrieb Peter Lieven:
>>> Some code in the block layer makes potentially huge allocations. Failure
>>> is not completely unexpected there, so avoid aborting qemu and handle
>>> out-of-memory situations gracefully.
>>>
>>> This patch addresses the allocations in the iscsi block driver.
>>>
>>> Signed-off-by: Kevin Wolf
>>> Acked-by: Paolo Bonzini
>>> Reviewed-by: Benoit Canet
>>> Reviewed-by: Eric Blake
>>> ---
>>> block/iscsi.c | 5 -
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>> index 84aa22a..06afa78 100644
>>> --- a/block/iscsi.c
>>> +++ b/block/iscsi.c
>>> @@ -893,7 +893,10 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState
>>> *bs, int64_t sector_num,
>>> nb_blocks = sector_qemu2lun(nb_sectors, iscsilun);
>>>
>>> if (iscsilun->zeroblock == NULL) {
>>> -iscsilun->zeroblock = g_malloc0(iscsilun->block_size);
>>> +iscsilun->zeroblock = g_try_malloc0(iscsilun->block_size);
>>> +if (iscsilun->zeroblock == NULL) {
>>> +return -ENOMEM;
>>> +}
>>> }
>>>
>>> iscsi_co_init_iscsitask(iscsilun, &iTask);
>> Unfortunately, I missed that one. The zeroblock is typicalls 512 Byte or 4K
>> depending
>> on the blocksize.
> I don't remember the details, but I think when I went through all
> drivers, I couldn't convince myself that a reasonable block size is
> enforced somewhere. So I just went ahead and converted the call to be on
> the safe side. It can never hurt anyway.
>
>> What is significantly larger is the allocationmap. It is typically created
>> on iscsi_open, but is also recreated on iscsi_truncate. I don't have the
>> context why this
>> patch was introduced, but I would vote for introducing a bitmap_try_new and
>> issue
>> a warning if the allocation fails. The allocationmap is optional we can work
>> without it.
>> If the pointer is NULL its not used.
> Right, that one I missed because it doesn't directly use g_malloc().
>
> Your proposal sounds good to me. Are you going to prepare a patch?
will do.
Peter
Re: [Qemu-devel] [PATCH 0/2] raw-posix: fix O_DIRECT short reads
Am 21.08.2014 um 14:44 hat Stefan Hajnoczi geschrieben: > This series fixes qemu-iotests ./check -nocache -vmdk 059. It also adds a > dedicated test case for O_DIRECT short reads. > > Stefan Hajnoczi (2): > raw-posix: fix O_DIRECT short reads > qemu-iotests: add test case 101 for short file I/O Cc: qemu-sta...@nongnu.org Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] issue: linking 64bit glib when building for cpu=i386
On Fri, Aug 22, 2014 at 9:20 AM, Daniel P. Berrange wrote: > On Thu, Aug 21, 2014 at 08:10:54PM -0400, John Snow wrote: >> I was running a series of tests on 32 and 64 bit hosts to test for >> endianness and variable width issues when I noticed that I couldn't properly >> perform a build of "make check" against a 32bit target from a 64bit host: >> >> ../../configure --cpu=i386 && make -j4 && make check >> >> This produces some warnings in tests-cutils about overflowing variables that >> are of type guint64. It's been mentioned on the mailing lists before, >> actually: http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00452.html >> >> The problem is that guint64 is being aliased against "unsigned long", which >> is only 4 bytes instead of the implied 8. This occurs because we link >> against the 64bit headers for glib instead of the 32bit ones when we're >> building for i386 from an x86_64 host. >> >> Our include flags wind up looking like: -I/usr/include/glib-2.0 but >> -I/usr/lib64/glib-2.0/include >> >> I was discussing the problem with Stefan: >> >> On 08/21/2014 05:03 AM, Stefan Hajnoczi wrote: >> >The problem is that pkg-config uses libdir=/usr/lib64 by default on >> >amd64 hosts. It doesn't know that gcc -m32 is being used. >> > >> >This results in glib's 64-bit headers being used where guint64 is just >> >unsigned long. On 32-bit hosts this is incorrect. >> > >> >Two workarounds: >> > >> >1. yum install pkgconfig.i686 and run it instead of pkgconfig.x86_64 >> > >> >2. Use the pkg-config --define-variable libdir=/usr/lib option >> > >> >You can set PKG_CONFIG=path/to/pkg-config.i686 on QEMU's ./configure >> >command-line. >> > >> >This is all distro-specific :(. Any other solutions? >> > >> >Stefan >> > >> >> I am not extremely well versed in configure or pkg-config ninjutsu, but I >> must imagine that the ARCH/cpu variables we are setting in configure could >> help us know to call the 32bit pkg-config instead of the native 64bit >> version and fix this issue. >> >> Does anyone have any good ideas? Surely other projects must have run into >> this elsewhere. > > Distros will install pkg-config .pc files for non-native architectures > in a different location normally. The supported / recommended way to > tell pkg-config to look in these alternative dirs is to set the env > variable PKG_CONFIG_LIBDIR. This replaces the built-in default search > directory that looks for native. > > So on a Fedora / RHELL system, to make pkg-config use 32-bit libs you > want to set > >PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig > > which replaces the default location of /usr/lib64/pkgconfig. Nice, and Paolo sent me an automated way of doing that: On Fri, Aug 22, 2014 at 12:14:28AM +0200, Paolo Bonzini wrote: > You need to set PKG_CONFIG_LIBDIR to > > /usr/lib/$MULTILIBDIR/pkgconfig > > where MULTILIBDIR is > > if $CC -print-multiarch >/dev/null 2>&1; then > MULTILIBDIR=`$CC -print-multiarch $CFLAGS $CPPFLAGS` > fi > if test -z "$MULTILIBDIR"; then > MULTILIBDIR=`$CC --print-multi-os-directory $CFLAGS $CPPFLAGS` > fi > > This will point at /usr/lib/pkgconfig/glib-2.0.pc instead of > /usr/lib64/pkgconfig/glib-2.0.pc I tested that it works. Stefan
Re: [Qemu-devel] [PATCH] blkdebug: Delete BH in bdrv_aio_cancel
Am 22.08.2014 um 06:45 hat Fam Zheng geschrieben: > Otherwise error_callback_bh will access the already released acb. > > Signed-off-by: Fam Zheng Cc: qemu-sta...@nongnu.org Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH v2] vmdk: Use bdrv_nb_sectors() where sectors, not bytes are wanted
Am 21.08.2014 um 14:36 hat Markus Armbruster geschrieben: > Instead of bdrv_getlength(). > > Commit 57322b7 did this all over block, but one more bdrv_getlength() > has crept in since. > > Signed-off-by: Markus Armbruster > --- > v2: Actually use bdrv_nb_sectors() as advertized [Fam] > Passes ./check -T -vmdk -g quick Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] Fix pflash_cfi01 to restore flash command/array state after migration
Johny Mattsson writes: > On 21 August 2014 19:48, Markus Armbruster wrote: >> >> Doesn't this break migration to/from unfixed versions? >> > > It might. It is not something we've tried here, and I'm certainly not > familiar enough with QEMU internals myself to say one way or another. We > encountered this issue in what I can best describe as a short-and-furious > proof-of-concept project, and as part of the wrapping up of this I'm going > over our change logs to see if anything can be contributed back upstream. > It seemed like a trivial fix in the tree, but I acknowledge it may have > ramifications we didn't consider. > > >> I suspect you need a subsection. Initialize ->romd = 1, then let the >> subsection override it. >> > > And here I'm well and truly out of my depth! If someone who actually knows > what they're doing would like to run with this, please do. > > Sorry to effectively do a dump-and-run here - I hope its a net positive at > least. Sure it is! Imperfect patches can be a rather nice kind of bug report.
Re: [Qemu-devel] [PATCH V2] net: Fix dealing with packets when runstate changes
On 2014/8/22 15:40, Jason Wang wrote:
On 08/21/2014 08:39 PM, zhanghailiang wrote:
For all NICs(except virtio-net) emulated by qemu,
Such as e1000, rtl8139, pcnet and ne2k_pci,
Qemu can still receive packets when VM is not running.
If this happened in *migration's* last PAUSE VM stage,
The new dirty RAM related to the packets will be missed,
And this will lead serious network fault in VM.
To avoid this, we forbid receiving packets in generic net code when
VM is not running. Also, when the runstate changes back to running,
we definitely need to flush queues to get packets flowing again.
Hi:
Can you describe what will happen if you don't flush queues after vm is
started? Btw, the notifier dependency and impact on vhost should be
mentioned here as well.
Hi Jason,
There is no side-effect for vhost-net because in nic_vmstate_change_handler
callback,
We will check the return value of nc->info->can_receive before flush queues,
If the vhost-net(virtio-net) is not ready, it will return 0, then we will not
do the flush action.
Also i have tested this patch with vhost_net, include when multiqueue is on,
Everything seems to be Ok!
When nc->info->receive return 0, the receive_disabled will be set to 1,
If this happened just before VM pause(is not running), after VM runstate
change back to run, the receive_disabled is still 1, if qemu want to
send packets to VM, it will always fail until something happen to clear it.
So we'd better to clear it after runstate change back to run.
Here we implement this in the net layer:
(1) Judge the vm runstate in qemu_can_send_packet
(2) Add a member 'VMChangeStateEntry *vmstate' to struct NICState,
Which will listen for VM runstate changes.
(3) Register a handler function for VMstate change.
When vm changes back to running, we flush all queues in the callback function.
(4) Remove checking vm state in virtio_net_can_receive
Signed-off-by: zhanghailiang
---
v2:
- remove the superfluous check of nc->received_disabled
---
hw/net/virtio-net.c | 4
include/net/net.h | 2 ++
net/net.c | 31 +++
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 268eff9..287d762 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -839,10 +839,6 @@ static int virtio_net_can_receive(NetClientState *nc)
VirtIODevice *vdev = VIRTIO_DEVICE(n);
VirtIONetQueue *q = virtio_net_get_subqueue(nc);
-if (!vdev->vm_running) {
-return 0;
-}
-
if (nc->queue_index>= n->curr_queues) {
return 0;
}
diff --git a/include/net/net.h b/include/net/net.h
index ed594f9..a294277 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -8,6 +8,7 @@
#include "net/queue.h"
#include "migration/vmstate.h"
#include "qapi-types.h"
+#include "sysemu/sysemu.h"
#define MAX_QUEUE_NUM 1024
@@ -96,6 +97,7 @@ typedef struct NICState {
NICConf *conf;
void *opaque;
bool peer_deleted;
+VMChangeStateEntry *vmstate;
} NICState;
NetClientState *qemu_find_netdev(const char *id);
diff --git a/net/net.c b/net/net.c
index 6d930ea..113a37b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -242,6 +242,28 @@ NetClientState *qemu_new_net_client(NetClientInfo *info,
return nc;
}
+static void nic_vmstate_change_handler(void *opaque,
+ int running,
+ RunState state)
+{
+NICState *nic = opaque;
+NetClientState *nc;
+int i, queues;
+
+if (!running) {
+return;
+}
+
+queues = MAX(1, nic->conf->peers.queues);
+for (i = 0; i< queues; i++) {
+nc =&nic->ncs[i];
+if (nc->info->can_receive&& !nc->info->can_receive(nc)) {
+continue;
+}
Why not just using qemu_can_send_packet() here?
qemu_can_send_packet will check the value of receive_disabled,
Our purpose is to clear it, if its value is 1.
May be we should check it as follow:
if ((!nc->receive_disabled) || (nc->info->can_receive &&
!nc->info->can_receive(nc)) {
continue;
}
qemu_flush_queued_packets(nc);
+qemu_flush_queued_packets(nc);
+}
+}
+
NICState *qemu_new_nic(NetClientInfo *info,
NICConf *conf,
const char *model,
@@ -259,6 +281,8 @@ NICState *qemu_new_nic(NetClientInfo *info,
nic->ncs = (void *)nic + info->size;
nic->conf = conf;
nic->opaque = opaque;
+nic->vmstate = qemu_add_vm_change_state_handler(nic_vmstate_change_handler,
+nic);
for (i = 0; i< queues; i++) {
qemu_net_client_setup(&nic->ncs[i], info, peers[i], model, name,
@@ -379,6 +403,7 @@ void qemu_del_nic(NICState *nic)
qemu_free_net_client(nc);
}
+qemu_del_vm_change_state_handler(nic->vmstate);
g_free(nic);
}
@@ -452,6 +477,12 @@ void qemu_set_vnet_hdr_len(NetClientState
[Qemu-devel] Don't return type from host in readdir on local 9p filesystem
From: Bastian Blank
When using mapped mode in 9pfs, readdir implementation
should not return file type in d_type from the host
readdir, instead, it should use the type stored in
the extended attributes. Since d_type is optional
and reading ext attrs for every readdir is expensive,
it should be sufficient to just set d_type to DT_UNKNOWN,
so guest will know to look it up separately.
This is a -stable material.
Signed-off-by: Michael Tokarev
--- qemu-2.0.0+dfsg.orig/hw/9pfs/virtio-9p-local.c
+++ qemu-2.0.0+dfsg/hw/9pfs/virtio-9p-local.c
@@ -396,12 +396,16 @@ static int local_readdir_r(FsContext *ct
again:
ret = readdir_r(fs->dir, entry, result);
-if (ctx->export_flags & V9FS_SM_MAPPED_FILE) {
+if (ctx->export_flags & V9FS_SM_MAPPED) {
+entry->d_type = DT_UNKNOWN;
+}
+else if (ctx->export_flags & V9FS_SM_MAPPED_FILE) {
if (!ret && *result != NULL &&
!strcmp(entry->d_name, VIRTFS_META_DIR)) {
/* skp the meta data directory */
goto again;
}
+entry->d_type = DT_UNKNOWN;
}
return ret;
}
Re: [Qemu-devel] [PATCH] IDE: MMIO IDE device control should be little endian
You forgot to cc qemu-devel...
On 22 August 2014 09:52, Valentin Manea wrote:
> Set the IDE MMIO memory type to little endian. The ATA specs identify
> words part of the control commands encoded as little endian.
> While this has no impact on little endian systems, it's required for big
> endian systems(eg OpenRisc).
>
> Signed-off-by: Valentin Manea
> ---
> hw/ide/mmio.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ide/mmio.c b/hw/ide/mmio.c
> index 01c1d0e..334c8cc 100644
> --- a/hw/ide/mmio.c
> +++ b/hw/ide/mmio.c
> @@ -82,7 +82,7 @@ static void mmio_ide_write(void *opaque, hwaddr addr,
> static const MemoryRegionOps mmio_ide_ops = {
> .read = mmio_ide_read,
> .write = mmio_ide_write,
> -.endianness = DEVICE_NATIVE_ENDIAN,
> +.endianness = DEVICE_LITTLE_ENDIAN,
> };
>
> static uint64_t mmio_ide_status_read(void *opaque, hwaddr addr,
> @@ -102,7 +102,7 @@ static void mmio_ide_cmd_write(void *opaque, hwaddr
> addr,
> static const MemoryRegionOps mmio_ide_cs_ops = {
> .read = mmio_ide_status_read,
> .write = mmio_ide_cmd_write,
> -.endianness = DEVICE_NATIVE_ENDIAN,
> +.endianness = DEVICE_LITTLE_ENDIAN,
> };
>
> static const VMStateDescription vmstate_ide_mmio = {
> --
> 1.9.1
thanks
-- PMM
[Qemu-devel] [PATCH 2/2] block/iscsi: handle failure on malloc of the allocationmap
Signed-off-by: Peter Lieven
---
block/iscsi.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index ed883c3..131357c 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -325,6 +325,19 @@ static bool is_request_lun_aligned(int64_t sector_num, int
nb_sectors,
return 1;
}
+static unsigned long *iscsi_allocationmap_init(IscsiLun *iscsilun)
+{
+unsigned long *ptr;
+ptr = bitmap_try_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks,
+ iscsilun),
+ iscsilun->cluster_sectors));
+if (ptr == NULL) {
+error_report("iSCSI: could not initialize allocationmap. "
+ "Out of memory.");
+}
+return ptr;
+}
+
static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num,
int nb_sectors)
{
@@ -1413,9 +1426,7 @@ static int iscsi_open(BlockDriverState *bs, QDict
*options, int flags,
iscsilun->cluster_sectors = (iscsilun->bl.opt_unmap_gran *
iscsilun->block_size) >> BDRV_SECTOR_BITS;
if (iscsilun->lbprz && !(bs->open_flags & BDRV_O_NOCACHE)) {
-iscsilun->allocationmap =
-bitmap_new(DIV_ROUND_UP(bs->total_sectors,
-iscsilun->cluster_sectors));
+iscsilun->allocationmap = iscsi_allocationmap_init(iscsilun);
}
}
@@ -1508,10 +1519,7 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t
offset)
if (iscsilun->allocationmap != NULL) {
g_free(iscsilun->allocationmap);
-iscsilun->allocationmap =
-bitmap_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks,
-iscsilun),
-iscsilun->cluster_sectors));
+iscsilun->allocationmap = iscsi_allocationmap_init(iscsilun);
}
return 0;
--
1.7.9.5
[Qemu-devel] [PATCH 1/2] util: introduce bitmap_try_new
regular bitmap_new simply aborts if the memory allocation fails.
bitmap_try_new returns NULL on failure and allows for proper
error handling.
Signed-off-by: Peter Lieven
---
include/qemu/bitmap.h |6 ++
1 file changed, 6 insertions(+)
diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 1babd5d..51b430f 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -94,6 +94,12 @@ static inline unsigned long *bitmap_new(long nbits)
return g_malloc0(len);
}
+static inline unsigned long *bitmap_try_new(long nbits)
+{
+long len = BITS_TO_LONGS(nbits) * sizeof(unsigned long);
+return g_try_malloc0(len);
+}
+
static inline void bitmap_zero(unsigned long *dst, long nbits)
{
if (small_nbits(nbits)) {
--
1.7.9.5
[Qemu-devel] [PATCH 0/2] introduce bitmap_try_new
this adds a new helper to handle errors when allocation a bitmap and also introduces its first user. Peter Lieven (2): util: introduce bitmap_try_new block/iscsi: handle failure on malloc of the allocationmap block/iscsi.c | 22 +++--- include/qemu/bitmap.h |6 ++ 2 files changed, 21 insertions(+), 7 deletions(-) -- 1.7.9.5
Re: [Qemu-devel] [PATCH V2] net: Fix dealing with packets when runstate changes
On 08/22/2014 05:21 PM, zhanghailiang wrote:
> On 2014/8/22 15:40, Jason Wang wrote:
>> On 08/21/2014 08:39 PM, zhanghailiang wrote:
>>> For all NICs(except virtio-net) emulated by qemu,
>>> Such as e1000, rtl8139, pcnet and ne2k_pci,
>>> Qemu can still receive packets when VM is not running.
>>> If this happened in *migration's* last PAUSE VM stage,
>>> The new dirty RAM related to the packets will be missed,
>>> And this will lead serious network fault in VM.
>>>
>>> To avoid this, we forbid receiving packets in generic net code when
>>> VM is not running. Also, when the runstate changes back to running,
>>> we definitely need to flush queues to get packets flowing again.
>> Hi:
>>
>> Can you describe what will happen if you don't flush queues after vm is
>> started? Btw, the notifier dependency and impact on vhost should be
>> mentioned here as well.
>
> Hi Jason,
>
> There is no side-effect for vhost-net because in
> nic_vmstate_change_handler callback,
> We will check the return value of nc->info->can_receive before flush
> queues,
> If the vhost-net(virtio-net) is not ready, it will return 0, then we
> will not
> do the flush action.
Thanks for the clarification. Then the vmstate handler does nothing for
virtio-net :)
>
> Also i have tested this patch with vhost_net, include when multiqueue
> is on,
> Everything seems to be Ok!
>
> When nc->info->receive return 0, the receive_disabled will be set to 1,
> If this happened just before VM pause(is not running), after VM runstate
> change back to run, the receive_disabled is still 1, if qemu want to
> send packets to VM, it will always fail until something happen to
> clear it.
> So we'd better to clear it after runstate change back to run.
The same question as I replied in V1. Let's take virtio_net as an
example, when its receive() returns zero, it means its rx ring needs
refilling. And guest will kick qemu if it finishes the refilling, at
this time virtio_net_handle_rx() will call qemu_flush_queued_packets()
to clear receive_disabled.
So it looks safe to keep its value?
>
>>>
>>> Here we implement this in the net layer:
>>> (1) Judge the vm runstate in qemu_can_send_packet
>>> (2) Add a member 'VMChangeStateEntry *vmstate' to struct NICState,
>>> Which will listen for VM runstate changes.
>>> (3) Register a handler function for VMstate change.
>>> When vm changes back to running, we flush all queues in the callback
>>> function.
>>> (4) Remove checking vm state in virtio_net_can_receive
>>>
>>> Signed-off-by: zhanghailiang
>>> ---
>>> v2:
>>> - remove the superfluous check of nc->received_disabled
>>> ---
>>> hw/net/virtio-net.c | 4
>>> include/net/net.h | 2 ++
>>> net/net.c | 31 +++
>>> 3 files changed, 33 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> index 268eff9..287d762 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -839,10 +839,6 @@ static int
>>> virtio_net_can_receive(NetClientState *nc)
>>> VirtIODevice *vdev = VIRTIO_DEVICE(n);
>>> VirtIONetQueue *q = virtio_net_get_subqueue(nc);
>>>
>>> -if (!vdev->vm_running) {
>>> -return 0;
>>> -}
>>> -
>>> if (nc->queue_index>= n->curr_queues) {
>>> return 0;
>>> }
>>> diff --git a/include/net/net.h b/include/net/net.h
>>> index ed594f9..a294277 100644
>>> --- a/include/net/net.h
>>> +++ b/include/net/net.h
>>> @@ -8,6 +8,7 @@
>>> #include "net/queue.h"
>>> #include "migration/vmstate.h"
>>> #include "qapi-types.h"
>>> +#include "sysemu/sysemu.h"
>>>
>>> #define MAX_QUEUE_NUM 1024
>>>
>>> @@ -96,6 +97,7 @@ typedef struct NICState {
>>> NICConf *conf;
>>> void *opaque;
>>> bool peer_deleted;
>>> +VMChangeStateEntry *vmstate;
>>> } NICState;
>>>
>>> NetClientState *qemu_find_netdev(const char *id);
>>> diff --git a/net/net.c b/net/net.c
>>> index 6d930ea..113a37b 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -242,6 +242,28 @@ NetClientState
>>> *qemu_new_net_client(NetClientInfo *info,
>>> return nc;
>>> }
>>>
>>> +static void nic_vmstate_change_handler(void *opaque,
>>> + int running,
>>> + RunState state)
>>> +{
>>> +NICState *nic = opaque;
>>> +NetClientState *nc;
>>> +int i, queues;
>>> +
>>> +if (!running) {
>>> +return;
>>> +}
>>> +
>>> +queues = MAX(1, nic->conf->peers.queues);
>>> +for (i = 0; i< queues; i++) {
>>> +nc =&nic->ncs[i];
>>> +if (nc->info->can_receive&& !nc->info->can_receive(nc)) {
>>> +continue;
>>> +}
>>
>> Why not just using qemu_can_send_packet() here?
>
> qemu_can_send_packet will check the value of receive_disabled,
> Our purpose is to clear it, if its value is 1.
>
> May be we should check it as follow:
>
> if ((!nc->receive_disabled) || (nc->info->can_receive &&
> !nc->info->can_receive(nc)) {
Re: [Qemu-devel] [RFC PATCH 1/9] block: Add bdrv_aio_cancel_async
On Fri, 08/22 10:14, Paolo Bonzini wrote: > Il 22/08/2014 03:23, Fam Zheng ha scritto: > > What about we save cb and opaque locally, and set acb->cb to a nop. When > > cancel > > is done we can call the original cb? > > That might work but needs some auditing. Right now the AIOCB is still > valid when the callback is called, and this would be changed. > > Also, having different semantics for cancellation vs. completion would > be painful. > Exactly. I'd rather not change the contract then. Alternatively, we may add a refcnt field to BlockDriverAioCB and grab one before calling .cancel, so the qemu_aio_release will not free it. Fam
Re: [Qemu-devel] [PATCH v3] Add ACPI tables for TPM
On Thu, Aug 21, 2014 at 08:14:48AM -0400, Stefan Berger wrote: > On 08/11/2014 04:33 PM, Stefan Berger wrote: > >From: Stefan Berger > > > >Add an SSDT ACPI table for the TPM device. > >Add a TCPA table for BIOS logging area when a TPM is being used. > > > >The latter follows this spec here: > > > >http://www.trustedcomputinggroup.org/files/static_page_files/DCD4188E-1A4B-B294-D050A155FB6F7385/TCG_ACPIGeneralSpecification_PublicReview.pdf > > > >This patch has Michael Tsirkin's patches folded in. > > > >Signed-off-by: Stefan Berger > >Signed-off-by: Michael S. Tsirkin > > Any comments on this patch? > >Stefan I applied this in my tree.
Re: [Qemu-devel] [PATCH] net: Forbid dealing with packets when VM is not running
On Wed, Aug 20, 2014 at 11:17:56AM +0800, Jason Wang wrote:
> On 08/19/2014 08:29 PM, Stefan Hajnoczi wrote:
> > On Mon, Aug 18, 2014 at 04:32:42PM +0800, zhanghailiang wrote:
> >> On 2014/8/18 14:55, Jason Wang wrote:
> >>> On 08/18/2014 12:46 PM, zhanghailiang wrote:
> diff --git a/net/net.c b/net/net.c
> index 6d930ea..21f0d48 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -242,6 +242,29 @@ NetClientState *qemu_new_net_client(NetClientInfo
> *info,
> return nc;
> }
>
> +static void nic_vmstate_change_handler(void *opaque,
> + int running,
> + RunState state)
> +{
> +NICState *nic = opaque;
> +NetClientState *nc;
> +int i, queues;
> +
> +if (!running) {
> +return;
> +}
> +
> +queues = MAX(1, nic->conf->peers.queues);
> +for (i = 0; i< queues; i++) {
> +nc =&nic->ncs[i];
> +if (nc->receive_disabled
> +|| (nc->info->can_receive&& !nc->info->can_receive(nc))) {
> +continue;
> +}
> +qemu_flush_queued_packets(nc);
> >>> How about simply purge the receive queue during stop? If ok, there's no
> >>> need to introduce extra vmstate change handler.
> >>>
> >> I don't know whether it is OK to purge the receive packages, it was
> >> suggested by Stefan Hajnoczi, and i am waiting for his opinion .:)
> >>
> >> I think we still need the extra vmstate change handler, Without the
> >> change handler, we don't know if the VM will go to stop and the time
> >> when to call qemu_purge_queued_packets.
> > qemu_flush_queued_packets() sets nc->received_disabled = 0. This may be
> > needed to get packets flowing again if ->receive() previously returned 0.
> >
> > Purging the queue does not clear nc->received_disabled so it is not
> > enough.
>
> Confused.
>
> virtio_net_receive() only returns 0 when it does not have enough rx
> buffers. In this case, it just wait for the guest to refill and kick
> again. Its rx kick handler will call qemu_flush_queued_packets() to
> clear nc->received_disabled. So does usbnet and others.
>
> If nic_received_disabled is 1, it means the no available rx buffer. We
> need wait guest to do the processing and refilling. Then why need clear
> it after vm was started?
I took a look at other emulated NICs, they don't use return 0 in
->receive().
I think you are right, we don't need to worry about flushing.
Stefan
pgpgbOAjL6YTZ.pgp
Description: PGP signature
Re: [Qemu-devel] [RFC PATCH 1/9] block: Add bdrv_aio_cancel_async
Il 22/08/2014 11:37, Fam Zheng ha scritto: > Exactly. I'd rather not change the contract then. > > Alternatively, we may add a refcnt field to BlockDriverAioCB and grab one > before > calling .cancel, so the qemu_aio_release will not free it. Yes, and I don't exclude that sooner or later we'll have to add reference counts to AIOCB anyway. However, reference counting is not _that_ cheap so for now I'd rather see other solutions explored. The problem is implementing cancel_sync in terms of cancel. The simplest solution, for now, is to make bdrv_aio_cancel_async return false if the callback is not implemented, and fall back to synchronous cancellation. Paolo
Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements
On Tue, Aug 19, 2014 at 01:50:27PM +0400, M.Kustova wrote: > On Tue, Aug 19, 2014 at 1:38 PM, Stefan Hajnoczi wrote: > > On Tue, Aug 19, 2014 at 02:00:24AM +0400, Maria Kustova wrote: > >> diff --git a/tests/image-fuzzer/qcow2/fuzz.py > >> b/tests/image-fuzzer/qcow2/fuzz.py > >> index 6e272c6..c652dc9 100644 > >> --- a/tests/image-fuzzer/qcow2/fuzz.py > >> +++ b/tests/image-fuzzer/qcow2/fuzz.py > >> @@ -123,7 +123,7 @@ def string_validator(current, strings): > >> return validator(current, random.choice, strings) > >> > >> > >> -def selector(current, constraints, validate=int_validator): > >> +def selector(current, constraints, validate=None): > >> """Select one value from all defined by constraints. > >> > >> Each constraint produces one random value satisfying to it. The > >> function > >> @@ -131,6 +131,9 @@ def selector(current, constraints, > >> validate=int_validator): > >> constraints overlaps). > >> """ > >> > >> +if validate is None: > >> +validate = int_validator > >> + > >> def iter_validate(c): > >> """Apply validate() only to constraints represented as lists. > >> > > > > I don't understand the benefit of this change. The default initializer > > syntax did what was intended, now it has been made more complicated with > > a None value that gets converted to int_validator later. > > It's a convention rule that allows keep the function signature > unchanged in case of modified function internals. > In other words, wrapper functions should not change in response of the > internal function change. I don't think that is necessary. This is not a stable API that cannot change. Also, what exactly creates an API commitment here? Hiding int_validator doesn't hide anything. Stefan pgpOHm7AH7WJw.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] vmxnet3: Pad short frames to minimum size (60 bytes)
Interestingly this does not appear to affect Windows guests on the same
host/bridge as the ESXi guest, as
Windows pads the frames before sending them out. However it does affect
Linux guests on the same host/bridge
from communicating with the ESXi guest itself and the guests ESXi hosts.
--
*View My LinkedIn Profile*
http://uk.linkedin.com/in/bendraper00
On 20 August 2014 13:27, Ben Draper wrote:
> When running VMware ESXi under qemu-kvm the guest discards frames
> that are too short. Short ARP Requests will be dropped, this prevents
> guests on the same bridge as VMware ESXi from communicating. This patch
> simply adds the padding on the network device itself.
>
> Signed-off-by: Ben Draper
> ---
> hw/net/vmxnet3.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 791321f..f246fa1 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -34,6 +34,7 @@
>
> #define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
> #define VMXNET3_MSIX_BAR_SIZE 0x2000
> +#define MIN_BUF_SIZE 60
>
> #define VMXNET3_BAR0_IDX (0)
> #define VMXNET3_BAR1_IDX (1)
> @@ -1871,12 +1872,21 @@ vmxnet3_receive(NetClientState *nc, const uint8_t
> *buf, size_t size)
> {
> VMXNET3State *s = qemu_get_nic_opaque(nc);
> size_t bytes_indicated;
> +uint8_t min_buf[MIN_BUF_SIZE];
>
> if (!vmxnet3_can_receive(nc)) {
> VMW_PKPRN("Cannot receive now");
> return -1;
> }
>
> +/* Pad to minimum Ethernet frame length */
> +if (size < sizeof(min_buf)) {
> +memcpy(min_buf, buf, size);
> +memset(&min_buf[size], 0, sizeof(min_buf) - size);
> +buf = min_buf;
> +size = sizeof(min_buf);
> +}
> +
> if (s->peer_has_vhdr) {
> vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
> buf += sizeof(struct virtio_net_hdr);
> --
> 1.7.10.4
>
>
[Qemu-devel] [PATCH 10/15] hw/intc/arm_gic: Handle grouping for GICC_HPPIR
Grouping (GICv2) and Security Extensions change the behaviour of reads
of the highest priority pending interrupt register (ICCHPIR/GICC_HPPIR).
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c | 29 -
hw/intc/gic_internal.h | 1 +
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 473b8f4..78efae1 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -300,6 +300,33 @@ uint8_t gic_get_running_priority(GICState *s, int cpu)
}
}
+uint16_t gic_get_current_pending_irq(GICState *s, int cpu)
+{
+bool isGrp0;
+uint16_t pendingId = s->current_pending[cpu];
+
+if (pendingId < GIC_MAXIRQ && (s->revision >= 2 || s->security_extn)) {
+isGrp0 = GIC_TEST_GROUP0(pendingId, (1 << cpu));
+if ((isGrp0 && !s->enabled_grp[0])
+|| (!isGrp0 && !s->enabled_grp[1])) {
+return 1023;
+}
+if (s->security_extn) {
+if (isGrp0 && ns_access()) {
+/* Group0 interrupts hidden from Non-secure access */
+return 1023;
+}
+if (!isGrp0 && !ns_access()
+&& !(s->cpu_control[cpu][0] & GICC_CTLR_S_ACK_CTL)) {
+/* Group1 interrupts only seen by Secure access if
+ * AckCtl bit set. */
+return 1022;
+}
+}
+}
+return pendingId;
+}
+
void gic_complete_irq(GICState *s, int cpu, int irq)
{
int update = 0;
@@ -818,7 +845,7 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int
offset)
case 0x14: /* Running Priority */
return gic_get_running_priority(s, cpu);
case 0x18: /* Highest Pending Interrupt */
-return s->current_pending[cpu];
+return gic_get_current_pending_irq(s, cpu);
case 0x1c: /* Aliased Binary Point */
if ((s->security_extn && ns_access())) {
/* If Security Extensions are present ABPR is a secure register,
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 433d75e..17632c1 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -79,6 +79,7 @@ void gic_set_priority(GICState *s, int cpu, int irq, uint8_t
val);
uint32_t gic_get_cpu_control(GICState *s, int cpu);
void gic_set_cpu_control(GICState *s, int cpu, uint32_t value);
uint8_t gic_get_running_priority(GICState *s, int cpu);
+uint16_t gic_get_current_pending_irq(GICState *s, int cpu);
static inline bool gic_test_pending(GICState *s, int irq, int cm)
--
1.8.3.2
[Qemu-devel] [PATCH 02/15] hw/arm/vexpress.c: Wire FIQ between CPU <> GIC
Connect FIQ output of the GIC CPU interfaces to the CPUs. Signed-off-by: Fabian Aggeler --- hw/arm/vexpress.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c index a88732c..bafe8d2 100644 --- a/hw/arm/vexpress.c +++ b/hw/arm/vexpress.c @@ -229,6 +229,8 @@ static void init_cpus(const char *cpu_model, const char *privdev, DeviceState *cpudev = DEVICE(qemu_get_cpu(n)); sysbus_connect_irq(busdev, n, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ)); +sysbus_connect_irq(busdev, n+smp_cpus, + qdev_get_gpio_in(cpudev, ARM_CPU_FIQ)); } } -- 1.8.3.2
[Qemu-devel] [PATCH 15/15] hw/intc/arm_gic: add gic_update() for grouping
GICs with grouping (GICv2 or GICv1 with Security Extensions) have a
different exception generation model which is more complicated than
without interrupt grouping. We add a new function to handle this model.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c | 87 +-
hw/intc/gic_internal.h | 1 +
2 files changed, 87 insertions(+), 1 deletion(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 4fe3555..f4492f4 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -52,6 +52,87 @@ static inline bool ns_access(void)
return true;
}
+inline void gic_update_with_grouping(GICState *s)
+{
+int best_irq;
+int best_prio;
+int irq;
+int irq_level;
+int fiq_level;
+int cpu;
+int cm;
+bool next_int;
+bool next_grp0;
+bool gicc_grp0_enabled;
+bool gicc_grp1_enabled;
+
+for (cpu = 0; cpu < NUM_CPU(s); cpu++) {
+cm = 1 << cpu;
+gicc_grp0_enabled = s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP0;
+gicc_grp1_enabled = s->cpu_control[cpu][1] & GICC_CTLR_S_EN_GRP1;
+next_int = 0;
+next_grp0 = 0;
+
+s->current_pending[cpu] = 1023;
+if ((!s->enabled_grp[0] && !s->enabled_grp[1])
+|| (!gicc_grp0_enabled && !gicc_grp1_enabled)) {
+qemu_irq_lower(s->parent_irq[cpu]);
+qemu_irq_lower(s->parent_fiq[cpu]);
+return;
+}
+
+/* Determine highest priority pending interrupt */
+best_prio = 0x100;
+best_irq = 1023;
+for (irq = 0; irq < s->num_irq; irq++) {
+if (GIC_TEST_ENABLED(irq, cm) && gic_test_pending(s, irq, cm)) {
+if (GIC_GET_PRIORITY(irq, cpu) < best_prio) {
+best_prio = GIC_GET_PRIORITY(irq, cpu);
+best_irq = irq;
+}
+}
+}
+
+/* Priority of IRQ higher than priority mask? */
+if (best_prio < s->priority_mask[cpu]) {
+s->current_pending[cpu] = best_irq;
+if (GIC_TEST_GROUP0(best_irq, cm) && s->enabled_grp[0]) {
+/* TODO: Add subpriority handling (binary point register) */
+if (best_prio < s->running_priority[cpu]) {
+next_int = true;
+next_grp0 = true;
+}
+} else if (!GIC_TEST_GROUP0(best_irq, cm) && s->enabled_grp[1]) {
+/* TODO: Add subpriority handling (binary point register) */
+if (best_prio < s->running_priority[cpu]) {
+next_int = true;
+next_grp0 = false;
+}
+}
+}
+
+fiq_level = 0;
+irq_level = 0;
+if (next_int) {
+if (next_grp0 && (s->cpu_control[cpu][0] & GICC_CTLR_S_FIQ_EN)) {
+if (gicc_grp0_enabled) {
+fiq_level = 1;
+DPRINTF("Raised pending FIQ %d (cpu %d)\n", best_irq, cpu);
+}
+} else {
+if ((next_grp0 && gicc_grp0_enabled)
+ || (!next_grp0 && gicc_grp1_enabled)) {
+irq_level = 1;
+DPRINTF("Raised pending IRQ %d (cpu %d)\n", best_irq, cpu);
+}
+}
+}
+/* Set IRQ/FIQ signal */
+qemu_set_irq(s->parent_irq[cpu], irq_level);
+qemu_set_irq(s->parent_fiq[cpu], fiq_level);
+}
+}
+
inline void gic_update_no_grouping(GICState *s)
{
int best_irq;
@@ -94,7 +175,11 @@ inline void gic_update_no_grouping(GICState *s)
/* Update interrupt status after enabled or pending bits have been changed. */
void gic_update(GICState *s)
{
-gic_update_no_grouping(s);
+if (s->revision >= 2 || s->security_extn) {
+gic_update_with_grouping(s);
+} else {
+gic_update_no_grouping(s);
+}
}
void gic_set_pending_private(GICState *s, int cpu, int irq)
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 150f867..6f67a09 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -73,6 +73,7 @@
void gic_set_pending_private(GICState *s, int cpu, int irq);
uint32_t gic_acknowledge_irq(GICState *s, int cpu);
void gic_complete_irq(GICState *s, int cpu, int irq);
+inline void gic_update_with_grouping(GICState *s);
inline void gic_update_no_grouping(GICState *s);
void gic_update(GICState *s);
void gic_init_irqs_and_distributor(GICState *s, int num_irq);
--
1.8.3.2
[Qemu-devel] [PATCH 00/15] target-arm: Add GICv1/SecExt and GICv2/Grouping
Hi, this series adds GICv1 Security Extensions (secure/non-secure interrupts) and interrupt grouping of the GICv2 specification. The patches use the terminology introduced by GICv2 (Group0 instead of secure). The series first adds FIQ lines from the GIC to the CPUs and then adds the Security Extensions. As the Security Extensions for CPUs are not upstream yet a ns_access() stub is used, which can be replaced in a follow-up with the actual implementation to determine the security state of a read/write access (needed for banking). Any feedback is highly appreciated! Thanks, Fabian Fabian Aggeler (15): hw/intc/arm_gic: Request FIQ sources hw/arm/vexpress.c: Wire FIQ between CPU <> GIC hw/intc/arm_gic: Add Security Extensions property hw/intc/arm_gic: Add ns_access() function hw/intc/arm_gic: Add Interrupt Group Registers hw/intc/arm_gic: Make ICDDCR/GICD_CTLR banked hw/intc/arm_gic: Make ICCICR/GICC_CTLR banked hw/intc/arm_gic: Make ICCBPR/GICC_BPR banked hw/intc/arm_gic: Implement Non-secure view of RPR hw/intc/arm_gic: Handle grouping for GICC_HPPIR hw/intc/arm_gic: Change behavior of EOIR writes hw/intc/arm_gic: Change behavior of IAR writes hw/intc/arm_gic: Restrict priority view hw/intc/arm_gic: Break out gic_update() function hw/intc/arm_gic: add gic_update() for grouping hw/arm/vexpress.c| 2 + hw/intc/arm_gic.c| 422 --- hw/intc/arm_gic_common.c | 9 +- hw/intc/arm_gic_kvm.c| 8 +- hw/intc/armv7m_nvic.c| 2 +- hw/intc/gic_internal.h | 25 +++ include/hw/intc/arm_gic_common.h | 20 +- 7 files changed, 447 insertions(+), 41 deletions(-) -- 1.8.3.2
[Qemu-devel] [PATCH 08/15] hw/intc/arm_gic: Make ICCBPR/GICC_BPR banked
This register is banked in GICs with Security Extensions. Storing the
non-secure copy of BPR in the abpr, which is an alias to the non-secure
copy for secure access. ABPR itself is only accessible from secure state
if the GIC implements Security Extensions.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c| 25 +
include/hw/intc/arm_gic_common.h | 8 +---
2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 7f7fac3..57021fd 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -792,7 +792,12 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int
offset)
case 0x04: /* Priority mask */
return s->priority_mask[cpu];
case 0x08: /* Binary Point */
-return s->bpr[cpu];
+if (s->security_extn && ns_access()) {
+/* BPR is banked. Non-secure copy stored in ABPR. */
+return s->abpr[cpu];
+} else {
+return s->bpr[cpu];
+}
case 0x0c: /* Acknowledge */
return gic_acknowledge_irq(s, cpu);
case 0x14: /* Running Priority */
@@ -800,7 +805,14 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int
offset)
case 0x18: /* Highest Pending Interrupt */
return s->current_pending[cpu];
case 0x1c: /* Aliased Binary Point */
-return s->abpr[cpu];
+if ((s->security_extn && ns_access())) {
+/* If Security Extensions are present ABPR is a secure register,
+ * only accessible from secure state.
+ */
+return 0;
+} else {
+return s->abpr[cpu];
+}
case 0xd0: case 0xd4: case 0xd8: case 0xdc:
return s->apr[(offset - 0xd0) / 4][cpu];
default:
@@ -819,12 +831,17 @@ static void gic_cpu_write(GICState *s, int cpu, int
offset, uint32_t value)
s->priority_mask[cpu] = (value & 0xff);
break;
case 0x08: /* Binary Point */
-s->bpr[cpu] = (value & 0x7);
+if (s->security_extn && ns_access()) {
+/* BPR is banked. Non-secure copy stored in ABPR. */
+s->abpr[cpu] = (value & 0x7);
+} else {
+s->bpr[cpu] = (value & 0x7);
+}
break;
case 0x10: /* End Of Interrupt */
return gic_complete_irq(s, cpu, value & 0x3ff);
case 0x1c: /* Aliased Binary Point */
-if (s->revision >= 2) {
+if (s->revision >= 2 && !(s->security_extn && ns_access())) {
s->abpr[cpu] = (value & 0x7);
}
break;
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index a912972..c547418 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -78,9 +78,11 @@ typedef struct GICState {
uint16_t running_priority[GIC_NCPU];
uint16_t current_pending[GIC_NCPU];
-/* We present the GICv2 without security extensions to a guest and
- * therefore the guest can configure the GICC_CTLR to configure group 1
- * binary point in the abpr.
+/* If we present the GICv2 without security extensions to a guest,
+ * the guest can configure the GICC_CTLR to configure group 1 binary point
+ * in the abpr.
+ * For a GIC with Security Extensions we use use bpr for the
+ * secure copy and abpr as storage for the non-secure copy of the register.
*/
uint8_t bpr[GIC_NCPU];
uint8_t abpr[GIC_NCPU];
--
1.8.3.2
[Qemu-devel] [PATCH 09/15] hw/intc/arm_gic: Implement Non-secure view of RPR
For GICs with Security Extensions Non-secure reads have a restricted
view on the current running priority.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c | 17 -
hw/intc/gic_internal.h | 1 +
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 57021fd..473b8f4 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -285,6 +285,21 @@ void gic_set_cpu_control(GICState *s, int cpu, uint32_t
value)
}
}
+uint8_t gic_get_running_priority(GICState *s, int cpu)
+{
+if (s->security_extn && ns_access()) {
+if (s->running_priority[cpu] & 0x80) {
+/* Running priority in upper half, return Non-secure view */
+return s->running_priority[cpu] << 1;
+} else {
+/* Running priority in lower half, RAZ */
+return 0;
+}
+} else {
+return s->running_priority[cpu];
+}
+}
+
void gic_complete_irq(GICState *s, int cpu, int irq)
{
int update = 0;
@@ -801,7 +816,7 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int
offset)
case 0x0c: /* Acknowledge */
return gic_acknowledge_irq(s, cpu);
case 0x14: /* Running Priority */
-return s->running_priority[cpu];
+return gic_get_running_priority(s, cpu);
case 0x18: /* Highest Pending Interrupt */
return s->current_pending[cpu];
case 0x1c: /* Aliased Binary Point */
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index e9814f4..433d75e 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -78,6 +78,7 @@ void gic_init_irqs_and_distributor(GICState *s, int num_irq);
void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val);
uint32_t gic_get_cpu_control(GICState *s, int cpu);
void gic_set_cpu_control(GICState *s, int cpu, uint32_t value);
+uint8_t gic_get_running_priority(GICState *s, int cpu);
static inline bool gic_test_pending(GICState *s, int irq, int cm)
--
1.8.3.2
[Qemu-devel] [PATCH 04/15] hw/intc/arm_gic: Add ns_access() function
Security Extensions for GICv1 and GICv2 use register banking
to provide transparent access to seperate Secure and Non-secure
copies of GIC configuration registers. This function will later
be replaced by code determining the security state of a read/write
access to a register.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 75b5121..9b83af0 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -45,6 +45,13 @@ static inline int gic_get_current_cpu(GICState *s)
return 0;
}
+/* Security state of a read / write access */
+static inline bool ns_access(void)
+{
+/* TODO: use actual security state */
+return true;
+}
+
/* TODO: Many places that call this routine could be optimized. */
/* Update interrupt status after enabled or pending bits have been changed. */
void gic_update(GICState *s)
--
1.8.3.2
[Qemu-devel] [PATCH 05/15] hw/intc/arm_gic: Add Interrupt Group Registers
Interrupt Group Registers (previously called Interrupt Security
Registers) as defined in GICv1 with Security Extensions or GICv2 allow
to configure interrupts as Secure (Group0) or Non-secure (Group1).
In GICv2 these registers are implemented independent of the existence of
Security Extensions.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c| 47 +---
hw/intc/arm_gic_common.c | 1 +
hw/intc/gic_internal.h | 4
include/hw/intc/arm_gic_common.h | 1 +
4 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 9b83af0..a972942 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -311,8 +311,26 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset)
if (offset < 0x08)
return 0;
if (offset >= 0x80) {
-/* Interrupt Security , RAZ/WI */
-return 0;
+/* Interrupt Group Registers
+ *
+ * For GIC with Security Extn and Non-secure access RAZ/WI
+ * For GICv1 without Security Extn RAZ/WI
+ */
+res = 0;
+if (!(s->security_extn && ns_access()) &&
+((s->revision == 1 && s->security_extn)
+|| s->revision == 2)) {
+irq = (offset - 0x080) * 8 + GIC_BASE_IRQ;
+if (irq >= s->num_irq) {
+goto bad_reg;
+}
+for (i = 0; i < 8; i++) {
+if (!GIC_TEST_GROUP0(irq + i, cm)) {
+res |= (1 << i);
+}
+}
+}
+return res;
}
goto bad_reg;
} else if (offset < 0x200) {
@@ -456,7 +474,30 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
} else if (offset < 4) {
/* ignored. */
} else if (offset >= 0x80) {
-/* Interrupt Security Registers, RAZ/WI */
+/* Interrupt Group Registers
+ *
+ * For GIC with Security Extn and Non-secure access RAZ/WI
+ * For GICv1 without Security Extn RAZ/WI
+ */
+if (!(s->security_extn && ns_access()) &&
+((s->revision == 1 && s->security_extn)
+|| s->revision == 2)) {
+irq = (offset - 0x080) * 8 + GIC_BASE_IRQ;
+if (irq >= s->num_irq) {
+goto bad_reg;
+}
+for (i = 0; i < 8; i++) {
+/* Group bits are banked for private interrupts
(internal)*/
+int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
+if (value & (1 << i)) {
+/* Group1 (Non-secure) */
+GIC_SET_GROUP1(irq + i, cm);
+} else {
+/* Group0 (Secure) */
+GIC_SET_GROUP0(irq + i, cm);
+}
+}
+}
} else {
goto bad_reg;
}
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index 302a056..f74175d 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -52,6 +52,7 @@ static const VMStateDescription vmstate_gic_irq_state = {
VMSTATE_UINT8(level, gic_irq_state),
VMSTATE_BOOL(model, gic_irq_state),
VMSTATE_BOOL(edge_trigger, gic_irq_state),
+VMSTATE_UINT8(group, gic_irq_state),
VMSTATE_END_OF_LIST()
}
};
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 48a58d7..b0430ff 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -50,6 +50,10 @@
s->priority1[irq][cpu] :\
s->priority2[(irq) - GIC_INTERNAL])
#define GIC_TARGET(irq) s->irq_target[irq]
+#define GIC_SET_GROUP0(irq, cm) (s->irq_state[irq].group |= (cm))
+#define GIC_SET_GROUP1(irq, cm) (s->irq_state[irq].group &= ~(cm))
+#define GIC_TEST_GROUP0(irq, cm) ((s->irq_state[irq].group & (cm)) == 0)
+
/* The special cases for the revision property: */
#define REV_11MPCORE 0
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index 4e25017..a61e52e 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -42,6 +42,7 @@ typedef struct gic_irq_state {
uint8_t level;
bool model; /* 0 = N:N, 1 = 1:N */
bool edge_trigger; /* true: edge-triggered, false: level-triggered */
+uint8_t group;
} gic_irq_state;
typedef struct GICState {
--
1.8.3.2
[Qemu-devel] [PATCH 06/15] hw/intc/arm_gic: Make ICDDCR/GICD_CTLR banked
ICDDCR/GICD_CTLR is banked in GICv1 implementations with Security
Extensions or in GICv2 in independent from Security Extensions.
This makes it possible to enable forwarding of interrupts from
Distributor to the CPU interfaces for Group0 and Group1.
EnableGroup0 (Bit [1]) in GICv1 is IMPDEF. Since this bit (Enable
Non-secure) is present in the integrated IC of the Cortex-A9 MPCore,
which implements the GICv1 profile, we support this bit in GICv1 too.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c| 34 ++
hw/intc/arm_gic_common.c | 2 +-
include/hw/intc/arm_gic_common.h | 7 ++-
3 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index a972942..c78b301 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -301,8 +301,18 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset)
cpu = gic_get_current_cpu(s);
cm = 1 << cpu;
if (offset < 0x100) {
-if (offset == 0)
-return s->enabled;
+if (offset == 0) {
+res = 0;
+if ((s->revision == 2 && !s->security_extn)
+|| (s->security_extn && !ns_access())) {
+res = (s->enabled_grp[1] << 1) | s->enabled_grp[0];
+} else if (s->security_extn && ns_access()) {
+res = s->enabled_grp[1];
+} else {
+/* Neither GICv2 nor Security Extensions present */
+res = s->enabled;
+}
+}
if (offset == 4)
/* Interrupt Controller Type Register */
return ((s->num_irq / 32) - 1)
@@ -469,8 +479,24 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
cpu = gic_get_current_cpu(s);
if (offset < 0x100) {
if (offset == 0) {
-s->enabled = (value & 1);
-DPRINTF("Distribution %sabled\n", s->enabled ? "En" : "Dis");
+if ((s->revision == 2 && !s->security_extn)
+|| (s->security_extn && !ns_access())) {
+s->enabled_grp[0] = value & (1U << 0); /* EnableGrp0 */
+/* For a GICv1 with Security Extn "EnableGrp1" is IMPDEF. */
+s->enabled_grp[1] = value & (1U << 1); /* EnableGrp1 */
+DPRINTF("Group0 distribution %sabled\n"
+"Group1 distribution %sabled\n",
+s->enabled_grp[0] ? "En" : "Dis",
+s->enabled_grp[1] ? "En" : "Dis");
+} else if (s->security_extn && ns_access()) {
+s->enabled_grp[1] = (value & 1U);
+DPRINTF("Group1 distribution %sabled\n",
+s->enabled_grp[1] ? "En" : "Dis");
+} else {
+/* Neither GICv2 nor Security Extensions present */
+s->enabled = (value & 1U);
+DPRINTF("Distribution %sabled\n", s->enabled ? "En" : "Dis");
+}
} else if (offset < 4) {
/* ignored. */
} else if (offset >= 0x80) {
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index f74175d..7652754 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -64,7 +64,7 @@ static const VMStateDescription vmstate_gic = {
.pre_save = gic_pre_save,
.post_load = gic_post_load,
.fields = (VMStateField[]) {
-VMSTATE_BOOL(enabled, GICState),
+VMSTATE_UINT8_ARRAY(enabled_grp, GICState, GIC_NR_GROUP),
VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, GIC_NCPU),
VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1,
vmstate_gic_irq_state, gic_irq_state),
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index a61e52e..a39b066 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -30,6 +30,8 @@
#define GIC_NR_SGIS 16
/* Maximum number of possible CPU interfaces, determined by GIC architecture */
#define GIC_NCPU 8
+/* Number of Groups (Group0 [Secure], Group1 [Non-secure]) */
+#define GIC_NR_GROUP 2
#define MAX_NR_GROUP_PRIO 128
#define GIC_NR_APRS (MAX_NR_GROUP_PRIO / 32)
@@ -52,7 +54,10 @@ typedef struct GICState {
qemu_irq parent_irq[GIC_NCPU];
qemu_irq parent_fiq[GIC_NCPU];
-bool enabled;
+union {
+uint8_t enabled;
+uint8_t enabled_grp[GIC_NR_GROUP]; /* EnableGrp0 and EnableGrp1 */
+};
bool cpu_enabled[GIC_NCPU];
gic_irq_state irq_state[GIC_MAXIRQ];
--
1.8.3.2
[Qemu-devel] [PATCH 12/15] hw/intc/arm_gic: Change behavior of IAR writes
Grouping (GICv2) and Security Extensions change the behavior of IAR
reads. Acknowledging Group0 interrupts is only allowed from Secure
state and acknowledging Group1 interrupts from Secure state is only
allowed if AckCtl bit is set.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c | 24
1 file changed, 24 insertions(+)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index a96f4a2..cddad45 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -189,11 +189,35 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu)
int ret, irq, src;
int cm = 1 << cpu;
irq = s->current_pending[cpu];
+bool isGrp0;
if (irq == 1023
|| GIC_GET_PRIORITY(irq, cpu) >= s->running_priority[cpu]) {
DPRINTF("ACK no pending IRQ\n");
return 1023;
}
+
+if (s->revision >= 2 || s->security_extn) {
+isGrp0 = GIC_TEST_GROUP0(irq, (1 << cpu));
+if ((isGrp0 && (!s->enabled_grp[0]
+|| !(s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP0)))
+ || (!isGrp0 && (!s->enabled_grp[1]
+|| !(s->cpu_control[cpu][1] & GICC_CTLR_NS_EN_GRP1 {
+return 1023;
+}
+
+if ((s->revision >= 2 && !s->security_extn)
+|| (s->security_extn && !ns_access())) {
+if (!isGrp0 && !(s->cpu_control[cpu][0] & GICC_CTLR_S_ACK_CTL)) {
+DPRINTF("Read of IAR ignored for Group1 interrupt %d "
+"(AckCtl disabled)\n", irq);
+return 1022;
+}
+} else if (s->security_extn && ns_access() && isGrp0) {
+DPRINTF("Non-secure read of IAR ignored for Group0 interrupt %d\n",
+irq);
+return 1023;
+}
+}
s->last_active[irq][cpu] = s->running_irq[cpu];
if (s->revision == REV_11MPCORE || s->revision == REV_NVIC) {
--
1.8.3.2
[Qemu-devel] [PATCH 11/15] hw/intc/arm_gic: Change behavior of EOIR writes
Grouping (GICv2) and Security Extensions change the behavior of EOIR
writes. Completing Group0 interrupts is only allowed from Secure state
and completing Group1 interrupts from Secure state is only allowed if
AckCtl bit is set.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 78efae1..a96f4a2 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -355,6 +355,21 @@ void gic_complete_irq(GICState *s, int cpu, int irq)
GIC_SET_PENDING(irq, cm);
update = 1;
}
+} else if ((s->revision >= 2 && !s->security_extn)
+ || (s->security_extn && !ns_access())) {
+/* Handle GICv2 without Security Extensions or GIC with Security
+ * Extensions and a secure write.
+ */
+if (!GIC_TEST_GROUP0(irq, cm)
+&& !(s->cpu_control[cpu][0] & GICC_CTLR_S_ACK_CTL)) {
+/* Unpredictable. We choose to ignore. */
+DPRINTF("EOI for Group1 interrupt %d ignored "
+"(AckCtl disabled)\n", irq);
+return;
+}
+} else if (s->security_extn && ns_access() && GIC_TEST_GROUP0(irq, cm)) {
+DPRINTF("Non-secure EOI for Group0 interrupt %d ignored\n", irq);
+return;
}
if (irq != s->running_irq[cpu]) {
--
1.8.3.2
[Qemu-devel] [PATCH 13/15] hw/intc/arm_gic: Restrict priority view
GICs with Security Extensions restrict the non-secure view of the
interrupt priority and priority mask registers.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c | 66 +-
hw/intc/gic_internal.h | 3 +++
2 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index cddad45..3fe5f09 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -256,11 +256,66 @@ uint32_t gic_acknowledge_irq(GICState *s, int cpu)
void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val)
{
+uint8_t prio = val;
+
+if (s->security_extn && ns_access()) {
+if (GIC_TEST_GROUP0(irq, (1 << cpu))) {
+return; /* Ignore Non-secure access of Group0 IRQ */
+}
+prio = 0x80 | (prio >> 1); /* Non-secure view */
+}
+
if (irq < GIC_INTERNAL) {
-s->priority1[irq][cpu] = val;
+s->priority1[irq][cpu] = prio;
} else {
-s->priority2[(irq) - GIC_INTERNAL] = val;
+s->priority2[(irq) - GIC_INTERNAL] = prio;
+}
+}
+
+uint32_t gic_get_priority(GICState *s, int cpu, int irq)
+{
+uint32_t prio = GIC_GET_PRIORITY(irq, cpu);
+
+if (s->security_extn && ns_access()) {
+if (GIC_TEST_GROUP0(irq, (1 << cpu))) {
+return 0; /* Non-secure access cannot read priority of Group0 IRQ
*/
+}
+prio = (prio << 1); /* Non-secure view */
}
+return prio;
+}
+
+void gic_set_priority_mask(GICState *s, int cpu, uint8_t val)
+{
+uint8_t pmask = (val & 0xff);
+
+if (s->security_extn && ns_access()) {
+if (s->priority_mask[cpu] & 0x80) {
+/* Priority Mask in upper half */
+pmask = 0x80 | (pmask >> 1);
+} else {
+/* Non-secure write ignored if priority mask is in lower half */
+return;
+}
+}
+s->priority_mask[cpu] = pmask;
+}
+
+uint32_t gic_get_priority_mask(GICState *s, int cpu)
+{
+uint32_t pmask = s->priority_mask[cpu];
+
+if (s->security_extn && ns_access()) {
+if (pmask & 0x80) {
+/* Priority Mask in upper half, return Non-secure view */
+pmask = (pmask << 1);
+} else {
+/* Priority Mask in lower half, RAZ */
+pmask = 0;
+}
+}
+return pmask;
+
}
uint32_t gic_get_cpu_control(GICState *s, int cpu)
@@ -518,7 +573,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset)
irq = (offset - 0x400) + GIC_BASE_IRQ;
if (irq >= s->num_irq)
goto bad_reg;
-res = GIC_GET_PRIORITY(irq, cpu);
+res = gic_get_priority(s, cpu, irq);
} else if (offset < 0xc00) {
/* Interrupt CPU Target. */
if (s->num_cpu == 1 && s->revision != REV_11MPCORE) {
@@ -871,7 +926,7 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int
offset)
case 0x00: /* Control */
return gic_get_cpu_control(s, cpu);
case 0x04: /* Priority mask */
-return s->priority_mask[cpu];
+return gic_get_priority_mask(s, cpu);
case 0x08: /* Binary Point */
if (s->security_extn && ns_access()) {
/* BPR is banked. Non-secure copy stored in ABPR. */
@@ -909,8 +964,7 @@ static void gic_cpu_write(GICState *s, int cpu, int offset,
uint32_t value)
case 0x00: /* Control */
return gic_set_cpu_control(s, cpu, value);
case 0x04: /* Priority mask */
-s->priority_mask[cpu] = (value & 0xff);
-break;
+return gic_set_priority_mask(s, cpu, value);
case 0x08: /* Binary Point */
if (s->security_extn && ns_access()) {
/* BPR is banked. Non-secure copy stored in ABPR. */
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 17632c1..8d951cc 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -76,6 +76,9 @@ void gic_complete_irq(GICState *s, int cpu, int irq);
void gic_update(GICState *s);
void gic_init_irqs_and_distributor(GICState *s, int num_irq);
void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val);
+uint32_t gic_get_priority(GICState *s, int cpu, int irq);
+void gic_set_priority_mask(GICState *s, int cpu, uint8_t val);
+uint32_t gic_get_priority_mask(GICState *s, int cpu);
uint32_t gic_get_cpu_control(GICState *s, int cpu);
void gic_set_cpu_control(GICState *s, int cpu, uint32_t value);
uint8_t gic_get_running_priority(GICState *s, int cpu);
--
1.8.3.2
[Qemu-devel] [PATCH 07/15] hw/intc/arm_gic: Make ICCICR/GICC_CTLR banked
ICCICR/GICC_CTLR is banked in GICv1 implementations with Security
Extensions or in GICv2 in independent from Security Extensions.
This makes it possible to enable forwarding of interrupts from
the CPU interfaces to the connected processors for Group0 and Group1.
We also allow to set additional bits like AckCtl and FIQEn by changing
the type from bool to uint32. Since the field does not only store the
enable bit anymore and since we are touching the vmstate, we use the
opportunity to rename the field to cpu_control.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c| 54
hw/intc/arm_gic_common.c | 5 ++--
hw/intc/arm_gic_kvm.c| 8 +++---
hw/intc/armv7m_nvic.c| 2 +-
hw/intc/gic_internal.h | 14 +++
include/hw/intc/arm_gic_common.h | 2 +-
6 files changed, 72 insertions(+), 13 deletions(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index c78b301..7f7fac3 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -66,7 +66,7 @@ void gic_update(GICState *s)
for (cpu = 0; cpu < NUM_CPU(s); cpu++) {
cm = 1 << cpu;
s->current_pending[cpu] = 1023;
-if (!s->enabled || !s->cpu_enabled[cpu]) {
+if (!s->enabled || !(s->cpu_control[cpu][0] & 1)) {
qemu_irq_lower(s->parent_irq[cpu]);
return;
}
@@ -239,6 +239,52 @@ void gic_set_priority(GICState *s, int cpu, int irq,
uint8_t val)
}
}
+uint32_t gic_get_cpu_control(GICState *s, int cpu)
+{
+if ((s->revision >= 2 && !s->security_extn)
+|| (s->security_extn && !ns_access())) {
+return s->cpu_control[cpu][0];
+} else if (s->security_extn && ns_access()) {
+return s->cpu_control[cpu][1];
+} else {
+return s->cpu_control[cpu][0];
+}
+}
+
+void gic_set_cpu_control(GICState *s, int cpu, uint32_t value)
+{
+/* CPU Interface Control is banked for GICv2 and GICv1 with Security Extn
*/
+if ((s->revision >= 2 && !s->security_extn)
+|| (s->security_extn && !ns_access())) {
+/* Write to Secure instance of the register */
+s->cpu_control[cpu][0] = (value & GICC_CTLR_S_MASK);
+/* Synchronize EnableGrp1 alias of Non-secure copy */
+s->cpu_control[cpu][1] &= ~GICC_CTLR_NS_EN_GRP1;
+s->cpu_control[cpu][1] |= (value & GICC_CTLR_S_EN_GRP1) ?
+GICC_CTLR_NS_EN_GRP1 : 0;
+
+DPRINTF("CPU Interface %d: Group0 Interrupts %sabled, "
+"Group1 Interrupts %sabled\n", cpu,
+(s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP0) ? "En" : "Dis",
+(s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP1) ? "En" : "Dis");
+} else if (s->security_extn && ns_access()) {
+/* Write to Non-secure instance of the register */
+s->cpu_control[cpu][1] = (value & GICC_CTLR_NS_MASK);
+/* Synchronize EnableGrp1 alias of Secure copy */
+s->cpu_control[cpu][0] &= ~GICC_CTLR_S_EN_GRP1;
+s->cpu_control[cpu][0] |= (value & GICC_CTLR_NS_EN_GRP1) ?
+GICC_CTLR_S_EN_GRP1 : 0;
+
+DPRINTF("CPU Interface %d: Group1 Interrupts %sabled\n", cpu,
+(s->cpu_control[cpu][1] & GICC_CTLR_NS_EN_GRP1) ? "En" :
"Dis");
+} else {
+s->cpu_control[cpu][0] = (value & 1);
+
+DPRINTF("CPU Interface %d %sabled\n", cpu,
+s->cpu_control[cpu][0] ? "En" : "Dis");
+}
+}
+
void gic_complete_irq(GICState *s, int cpu, int irq)
{
int update = 0;
@@ -742,7 +788,7 @@ static uint32_t gic_cpu_read(GICState *s, int cpu, int
offset)
{
switch (offset) {
case 0x00: /* Control */
-return s->cpu_enabled[cpu];
+return gic_get_cpu_control(s, cpu);
case 0x04: /* Priority mask */
return s->priority_mask[cpu];
case 0x08: /* Binary Point */
@@ -768,9 +814,7 @@ static void gic_cpu_write(GICState *s, int cpu, int offset,
uint32_t value)
{
switch (offset) {
case 0x00: /* Control */
-s->cpu_enabled[cpu] = (value & 1);
-DPRINTF("CPU %d %sabled\n", cpu, s->cpu_enabled[cpu] ? "En" : "Dis");
-break;
+return gic_set_cpu_control(s, cpu, value);
case 0x04: /* Priority mask */
s->priority_mask[cpu] = (value & 0xff);
break;
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index 7652754..c873f7b 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -65,7 +65,7 @@ static const VMStateDescription vmstate_gic = {
.post_load = gic_post_load,
.fields = (VMStateField[]) {
VMSTATE_UINT8_ARRAY(enabled_grp, GICState, GIC_NR_GROUP),
-VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, GIC_NCPU),
+VMSTATE_UINT32_2DARRAY(cpu_control, GICState, GIC_NCPU, GIC_NR_GROUP),
VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1,
vmstate_gic_irq_state, gic_irq_state),
VMSTATE_UINT8_ARRAY(irq_ta
[Qemu-devel] [PATCH 01/15] hw/intc/arm_gic: Request FIQ sources
Preparing for FIQ lines from GIC to CPUs, which is needed for GIC
Security Extensions.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c| 3 +++
include/hw/intc/arm_gic_common.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 1532ef9..b27bd0e 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -786,6 +786,9 @@ void gic_init_irqs_and_distributor(GICState *s, int num_irq)
for (i = 0; i < NUM_CPU(s); i++) {
sysbus_init_irq(sbd, &s->parent_irq[i]);
}
+for (i = 0; i < NUM_CPU(s); i++) {
+sysbus_init_irq(sbd, &s->parent_fiq[i]);
+}
memory_region_init_io(&s->iomem, OBJECT(s), &gic_dist_ops, s,
"gic_dist", 0x1000);
}
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index f6887ed..01c6f24 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -50,6 +50,7 @@ typedef struct GICState {
/*< public >*/
qemu_irq parent_irq[GIC_NCPU];
+qemu_irq parent_fiq[GIC_NCPU];
bool enabled;
bool cpu_enabled[GIC_NCPU];
--
1.8.3.2
[Qemu-devel] [PATCH 03/15] hw/intc/arm_gic: Add Security Extensions property
The existing implementation does not support Security Extensions mentioned
in the GICv1 and GICv2 architecture specification. Security Extensions are
not available on all GICs. This property makes it possible to enable Security
Extensions.
It also makes GICD_TYPER/ICDICTR.SecurityExtn RAO for GICs which implement
Security Extensions.
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c| 5 -
hw/intc/arm_gic_common.c | 1 +
include/hw/intc/arm_gic_common.h | 1 +
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index b27bd0e..75b5121 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -297,7 +297,10 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset)
if (offset == 0)
return s->enabled;
if (offset == 4)
-return ((s->num_irq / 32) - 1) | ((NUM_CPU(s) - 1) << 5);
+/* Interrupt Controller Type Register */
+return ((s->num_irq / 32) - 1)
+| ((NUM_CPU(s) - 1) << 5)
+| (s->security_extn << 10);
if (offset < 0x08)
return 0;
if (offset >= 0x80) {
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index 6d884ec..302a056 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -149,6 +149,7 @@ static Property arm_gic_common_properties[] = {
* (Internally, 0x also indicates "not a GIC but an NVIC".)
*/
DEFINE_PROP_UINT32("revision", GICState, revision, 1),
+DEFINE_PROP_UINT8("security-extn", GICState, security_extn, 0),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h
index 01c6f24..4e25017 100644
--- a/include/hw/intc/arm_gic_common.h
+++ b/include/hw/intc/arm_gic_common.h
@@ -105,6 +105,7 @@ typedef struct GICState {
MemoryRegion cpuiomem[GIC_NCPU + 1]; /* CPU interfaces */
uint32_t num_irq;
uint32_t revision;
+uint8_t security_extn;
int dev_fd; /* kvm device fd if backed by kvm vgic support */
} GICState;
--
1.8.3.2
[Qemu-devel] [PATCH 14/15] hw/intc/arm_gic: Break out gic_update() function
Prepare to split gic_update() in two functions, one for GICs with
interrupt grouping and one without grouping (existing).
Signed-off-by: Fabian Aggeler
---
hw/intc/arm_gic.c | 11 ---
hw/intc/gic_internal.h | 1 +
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 3fe5f09..4fe3555 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -52,9 +52,7 @@ static inline bool ns_access(void)
return true;
}
-/* TODO: Many places that call this routine could be optimized. */
-/* Update interrupt status after enabled or pending bits have been changed. */
-void gic_update(GICState *s)
+inline void gic_update_no_grouping(GICState *s)
{
int best_irq;
int best_prio;
@@ -92,6 +90,13 @@ void gic_update(GICState *s)
}
}
+/* TODO: Many places that call this routine could be optimized. */
+/* Update interrupt status after enabled or pending bits have been changed. */
+void gic_update(GICState *s)
+{
+gic_update_no_grouping(s);
+}
+
void gic_set_pending_private(GICState *s, int cpu, int irq)
{
int cm = 1 << cpu;
diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 8d951cc..150f867 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -73,6 +73,7 @@
void gic_set_pending_private(GICState *s, int cpu, int irq);
uint32_t gic_acknowledge_irq(GICState *s, int cpu);
void gic_complete_irq(GICState *s, int cpu, int irq);
+inline void gic_update_no_grouping(GICState *s);
void gic_update(GICState *s);
void gic_init_irqs_and_distributor(GICState *s, int num_irq);
void gic_set_priority(GICState *s, int cpu, int irq, uint8_t val);
--
1.8.3.2
Re: [Qemu-devel] [RFC PATCH 1/9] block: Add bdrv_aio_cancel_async
On Fri, 08/22 12:10, Paolo Bonzini wrote:
> Il 22/08/2014 11:37, Fam Zheng ha scritto:
> > Exactly. I'd rather not change the contract then.
> >
> > Alternatively, we may add a refcnt field to BlockDriverAioCB and grab one
> > before
> > calling .cancel, so the qemu_aio_release will not free it.
>
> Yes, and I don't exclude that sooner or later we'll have to add
> reference counts to AIOCB anyway. However, reference counting is not
> _that_ cheap so for now I'd rather see other solutions explored.
Why doesn it have an performance effect? Just because of the would-be
"if (likely(--acb->refcnt == 0))" testing?
>
> The problem is implementing cancel_sync in terms of cancel. The
> simplest solution, for now, is to make bdrv_aio_cancel_async return
> false if the callback is not implemented, and fall back to synchronous
> cancellation.
This does keep the code change local, but not necessarily simple, since there
would be two cancelling code paths in scsi-bus. I already find the scsi req
ref/unref pairs a bit hard to track.
I prefer that we change the implementation to avoid complicating interface:
don't call qemu_aio_release in .cancel, but call it in
bdrv_aio_cancel{,_async} after calling .cancel(). Does that work?
Fam
[Qemu-devel] [PATCH v2 3/7] stubs: Add iohandler.c
Add stub function "qemu_set_fd_handler" which is used by
util/event_notifier-posix.c.
Signed-off-by: Fam Zheng
---
stubs/Makefile.objs | 1 +
stubs/iohandler.c | 20
2 files changed, 21 insertions(+)
create mode 100644 stubs/iohandler.c
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 5e347d0..d9cad1b 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -40,3 +40,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
stub-obj-y += cpus.o
stub-obj-y += kvm.o
stub-obj-y += qmp_pc_dimm_device_list.o
+stub-obj-y += iohandler.o
diff --git a/stubs/iohandler.c b/stubs/iohandler.c
new file mode 100644
index 000..97c0ce5
--- /dev/null
+++ b/stubs/iohandler.c
@@ -0,0 +1,20 @@
+/*
+ * iohandler stub functions
+ *
+ * Copyright Red Hat, Inc., 2014
+ *
+ * Author: Fam Zheng
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/main-loop.h"
+
+int qemu_set_fd_handler(int fd,
+IOHandler *fd_read,
+IOHandler *fd_write,
+void *opaque)
+{
+abort();
+}
--
2.0.3
[Qemu-devel] [PATCH v2 1/7] build-sys: Move fifo8.c to hw/misc
Since it has a dependency on vmstate and is only used by device
emulation, moving out from util will make the archive more independent.
Signed-off-by: Fam Zheng
---
hw/misc/Makefile.objs | 1 +
{util => hw/misc}/fifo8.c | 0
util/Makefile.objs| 1 -
3 files changed, 1 insertion(+), 1 deletion(-)
rename {util => hw/misc}/fifo8.c (100%)
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 979e532..a4a5ad7 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
obj-$(CONFIG_ZYNQ) += zynq_slcr.o
obj-$(CONFIG_PVPANIC) += pvpanic.o
+obj-y += fifo8.o
diff --git a/util/fifo8.c b/hw/misc/fifo8.c
similarity index 100%
rename from util/fifo8.c
rename to hw/misc/fifo8.c
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 6b3c83b..65a36f6 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -3,7 +3,6 @@ util-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
event_notifier-win
util-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
event_notifier-posix.o qemu-openpty.o
util-obj-y += envlist.o path.o host-utils.o module.o
util-obj-y += bitmap.o bitops.o hbitmap.o
-util-obj-y += fifo8.o
util-obj-y += acl.o
util-obj-y += error.o qemu-error.o
util-obj-$(CONFIG_POSIX) += compatfd.o
--
2.0.3
[Qemu-devel] [PATCH v2 2/7] configure: Add -lutil to libs_qga if necessary
Since we will soon need to resolve openpty (3) when linking. Signed-off-by: Fam Zheng --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index 283c71c..5c35ecf 100755 --- a/configure +++ b/configure @@ -3546,6 +3546,7 @@ fi if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \ "$aix" != "yes" -a "$haiku" != "yes" ; then libs_softmmu="-lutil $libs_softmmu" +libs_qga="-lutil $libs_qga" fi ## -- 2.0.3
[Qemu-devel] [PATCH v2 5/7] util: Move throttle.c out to top level
It is only used by block code and has dependency on timers, so move it out to allow util to be linked unconditionally. Signed-off-by: Fam Zheng --- Makefile.objs | 2 +- util/throttle.c => throttle.c | 0 util/Makefile.objs| 1 - 3 files changed, 1 insertion(+), 2 deletions(-) rename util/throttle.c => throttle.c (100%) diff --git a/Makefile.objs b/Makefile.objs index 97db978..6445ce9 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -7,7 +7,7 @@ util-obj-y = util/ qobject/ qapi/ qapi-types.o qapi-visit.o qapi-event.o # block-obj-y is code used by both qemu system emulation and qemu-img block-obj-y = async.o thread-pool.o -block-obj-y += nbd.o block.o blockjob.o +block-obj-y += nbd.o block.o blockjob.o throttle.o block-obj-y += main-loop.o iohandler.o qemu-timer.o block-obj-$(CONFIG_POSIX) += aio-posix.o block-obj-$(CONFIG_WIN32) += aio-win32.o diff --git a/util/throttle.c b/throttle.c similarity index 100% rename from util/throttle.c rename to throttle.c diff --git a/util/Makefile.objs b/util/Makefile.objs index 65a36f6..5940acc 100644 --- a/util/Makefile.objs +++ b/util/Makefile.objs @@ -10,7 +10,6 @@ util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o util-obj-y += qemu-option.o qemu-progress.o util-obj-y += hexdump.o util-obj-y += crc32c.o -util-obj-y += throttle.o util-obj-y += getauxval.o util-obj-y += readline.o util-obj-y += rfifolock.o -- 2.0.3
[Qemu-devel] [PATCH v2 4/7] stubs: Merge set-fd-handler.c into iohandler.c
Signed-off-by: Fam Zheng
---
stubs/Makefile.objs| 1 -
stubs/iohandler.c | 9 +
stubs/set-fd-handler.c | 11 ---
3 files changed, 9 insertions(+), 12 deletions(-)
delete mode 100644 stubs/set-fd-handler.c
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index d9cad1b..2489c17 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -29,7 +29,6 @@ stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
stub-obj-y += qtest.o
stub-obj-y += reset.o
stub-obj-y += runstate-check.o
-stub-obj-y += set-fd-handler.o
stub-obj-y += slirp.o
stub-obj-y += sysbus.o
stub-obj-y += uuid.o
diff --git a/stubs/iohandler.c b/stubs/iohandler.c
index 97c0ce5..31b8b0f 100644
--- a/stubs/iohandler.c
+++ b/stubs/iohandler.c
@@ -18,3 +18,12 @@ int qemu_set_fd_handler(int fd,
{
abort();
}
+
+int qemu_set_fd_handler2(int fd,
+ IOCanReadHandler *fd_read_poll,
+ IOHandler *fd_read,
+ IOHandler *fd_write,
+ void *opaque)
+{
+abort();
+}
diff --git a/stubs/set-fd-handler.c b/stubs/set-fd-handler.c
deleted file mode 100644
index fc874d3..000
--- a/stubs/set-fd-handler.c
+++ /dev/null
@@ -1,11 +0,0 @@
-#include "qemu-common.h"
-#include "qemu/main-loop.h"
-
-int qemu_set_fd_handler2(int fd,
- IOCanReadHandler *fd_read_poll,
- IOHandler *fd_read,
- IOHandler *fd_write,
- void *opaque)
-{
-abort();
-}
--
2.0.3
[Qemu-devel] [PATCH v2 6/7] build-sys: Change libqemuutil.a to qemuutil.o and link whole object
When an executable is being generated, unused functions from libqemuutil.a are not linked. This is the linker's convention on archives (libqemuutil.a). Now that we have dynamically loaded modules, which may reference function from libqemuutil.a but not linked in the executable, because the executable itself didn't reference this symbol. That is a problem for module build. We can't link both executable and the dynamic shared object to libqemuutil.a, because of the risk of inconsistent views of program variables: DSO module sees a copy of some data because it is linked against libqemuutil.a, whereas the executable sees another copy. In other words, they each maintains a copy but with a same name. In this case, it can be very tricky to notice such a duplication, and make a bug hard to reason. So it's good to avoid it from the beginning. This patch solves the above issue by fully linking. Specifically, it fixes block-iscsi.mo: in block/iscsi.c, util/bitmap.c functions are used, but qemu-img doesn't link it. The solution is to link everything in libqemuutil.a. We do this by changing it to qemuutil.o, which includes all the util objects. This is easier and also expected to be more portable than "--whole-archive". Because qemuutil.o is now fully linked and hence make executables references more symbols than before, some test executables now need libqemustub.a, so add them as necessary too. Signed-off-by: Fam Zheng --- Makefile| 17 +--- Makefile.objs | 2 +- Makefile.target | 2 +- tests/Makefile | 60 - 4 files changed, 42 insertions(+), 39 deletions(-) diff --git a/Makefile b/Makefile index b33aaac..773e5eb 100644 --- a/Makefile +++ b/Makefile @@ -187,7 +187,7 @@ subdir-dtc:dtc/libfdt dtc/tests dtc/%: mkdir -p $@ -$(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y) +$(SUBDIR_RULES): qemuutil.o libqemustub.a $(common-obj-y) ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS)) romsubdir-%: @@ -208,7 +208,10 @@ Makefile: $(version-obj-y) $(version-lobj-y) # Build libraries libqemustub.a: $(stub-obj-y) -libqemuutil.a: $(util-obj-y) + +LD_REL := $(CC) -nostdlib -Wl,-r +qemuutil.o: $(util-obj-y) + $(call quiet-command,$(LD_REL) -o $@ $^," LD -r $(TARGET_DIR)$@") block-modules = $(foreach o,$(block-obj-m),"$(basename $(subst /,-,$o))",) NULL util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)' @@ -217,13 +220,13 @@ util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)' qemu-img.o: qemu-img-cmds.h -qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a -qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a -qemu-io$(EXESUF): qemu-io.o $(block-obj-y) libqemuutil.a libqemustub.a +qemu-img$(EXESUF): qemu-img.o $(block-obj-y) qemuutil.o libqemustub.a +qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) qemuutil.o libqemustub.a +qemu-io$(EXESUF): qemu-io.o $(block-obj-y) qemuutil.o libqemustub.a qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o -fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o libqemuutil.a libqemustub.a +fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o qemuutil.o libqemustub.a fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx @@ -280,7 +283,7 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h) $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN) -qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a +qemu-ga$(EXESUF): $(qga-obj-y) qemuutil.o libqemustub.a $(call LINK, $^) clean: diff --git a/Makefile.objs b/Makefile.objs index 6445ce9..bbb2ea4 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -109,6 +109,6 @@ target-obj-y += trace/ # guest agent # FIXME: a few definitions from qapi-types.o/qapi-visit.o are needed -# by libqemuutil.a. These should be moved to a separate .json schema. +# by qemuutil.o. These should be moved to a separate .json schema. qga-obj-y = qga/ qga-vss-dll-obj-y = qga/ diff --git a/Makefile.target b/Makefile.target index 1e8d7ab..48a3089 100644 --- a/Makefile.target +++ b/Makefile.target @@ -176,7 +176,7 @@ all-obj-y += $(target-obj-y) all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y) # build either PROG or PROGW -$(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a +$(QEMU_PROG_BUILD): $(all-obj-y) ../qemuutil.o ../libqemustub.a $(call LINK,$^) gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/scripts/feature_to_c.sh diff --git a/tests/Makefile b/tests/Makefile index 837e9c8..b79a299 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -226,22 +226,22 @@ qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386 -tests/check-qint$(EXESUF): tests/c
Re: [Qemu-devel] [PATCH v12 4/6] qapi: introduce PreallocMode and a new PreallocMode full.
Am 11.07.2014 um 08:10 hat Hu Tao geschrieben: > This patch prepares for the subsequent patches. > > Reviewed-by: Fam Zheng > Reviewed-by: Eric Blake > Reviewed-by: Max Reitz > Signed-off-by: Hu Tao Strictly speaking, qcow2 should error out if it is given full or falloc after this patch. Two patches later it will actually interpret those values correctly, so it's not that big of a problem, but it would definitely be nicer. Kevin
Re: [Qemu-devel] [PATCH v12 0/6] qcow2, raw: add preallocation=full and preallocation=falloc
Am 28.07.2014 um 10:48 hat Hu Tao geschrieben: > ping... > > All the 6 patches have reviewed-by now. Looks mostly good to me, I have only a few minor comments that wouldn't block inclusion but could be addressed in follow-up patches. However, you have a dependency on a patch series from Max (you use minimal_blob_size()), which hasn't been reviewed and merged yet, so your series is blocked on that. If you want to get your series merged quicker, you could replace this with a rough estimate that excludes the clusters used by refcount table and blocks. If full preallocation isn't really full, but only preallocation of 99.9%, that's probably good enough in practice. Kevin
[Qemu-devel] [PATCH v2 7/7] iscsi: Move iqn generation code to util
Function qmp_query_uuid, even with a version in libqemustub.a, is not
linked in qemu-img, unless we use it in somewhere that is linked into
qemu-img. For example util-obj-y - since iqn generation a general
function, not iscsi specific, moving it to util makes some sense, and
more importantly this will allow us to build iscsi as a dynamic module.
Signed-off-by: Fam Zheng
---
block/iscsi.c | 15 +--
include/qemu-common.h | 3 +++
util/Makefile.objs| 1 +
util/iqn.c| 37 +
4 files changed, 42 insertions(+), 14 deletions(-)
create mode 100644 util/iqn.c
diff --git a/block/iscsi.c b/block/iscsi.c
index 2c9cfc1..4436156 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -37,8 +37,6 @@
#include "trace.h"
#include "block/scsi.h"
#include "qemu/iov.h"
-#include "sysemu/sysemu.h"
-#include "qmp-commands.h"
#include
#include
@@ -1034,8 +1032,6 @@ static char *parse_initiator_name(const char *target)
QemuOptsList *list;
QemuOpts *opts;
const char *name;
-char *iscsi_name;
-UuidInfo *uuid_info;
list = qemu_find_opts("iscsi");
if (list) {
@@ -1051,16 +1047,7 @@ static char *parse_initiator_name(const char *target)
}
}
-uuid_info = qmp_query_uuid(NULL);
-if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
-name = qemu_get_vm_name();
-} else {
-name = uuid_info->UUID;
-}
-iscsi_name = g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s",
- name ? ":" : "", name ? name : "");
-qapi_free_UuidInfo(uuid_info);
-return iscsi_name;
+return iqn_generate();
}
static void iscsi_nop_timed_event(void *opaque)
diff --git a/include/qemu-common.h b/include/qemu-common.h
index bcf7a6a..69505b5 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -420,6 +420,9 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n);
/* unicode.c */
int mod_utf8_codepoint(const char *s, size_t n, char **end);
+/* iqn.c */
+char *iqn_generate(void);
+
/*
* Hexdump a buffer to a file. An optional string prefix is added to every line
*/
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 5940acc..30a3c77 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -13,3 +13,4 @@ util-obj-y += crc32c.o
util-obj-y += getauxval.o
util-obj-y += readline.o
util-obj-y += rfifolock.o
+util-obj-y += iqn.o
diff --git a/util/iqn.c b/util/iqn.c
new file mode 100644
index 000..ed30f0f
--- /dev/null
+++ b/util/iqn.c
@@ -0,0 +1,37 @@
+/*
+ * iqn generate function
+ *
+ * Copyright Red Hat, Inc., 2014
+ *
+ * Author: Paolo Bonzini
+ * Fam Zheng
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#include
+#include "qemu/error-report.h"
+#include "qemu-common.h"
+#include "sysemu/sysemu.h"
+#include "qmp-commands.h"
+
+char *iqn_generate(void)
+{
+const char *name;
+char *iqn;
+UuidInfo *uuid_info;
+
+uuid_info = qmp_query_uuid(NULL);
+if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
+name = qemu_get_vm_name();
+} else {
+name = uuid_info->UUID;
+}
+iqn = g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s",
+ name ? ":" : "",
+ name ? : "");
+qapi_free_UuidInfo(uuid_info);
+
+return iqn;
+}
--
2.0.3
Re: [Qemu-devel] [PATCH v12 1/6] block: round up file size to nearest sector
Am 11.07.2014 um 08:09 hat Hu Tao geschrieben: > Reviewed-by: Max Reitz > Reviewed-by: Eric Blake > Signed-off-by: Hu Tao If we make this change, shouldn't we do it consistently across all image formats? With this patch we have some formats that round up, and many others that round down. Kevin
Re: [Qemu-devel] [PATCH v12 5/6] raw-posix: Add falloc and full preallocation option
Am 11.07.2014 um 08:10 hat Hu Tao geschrieben: > This patch adds a new option preallocation for raw format, and implements > falloc and full preallocation. > > Reviewed-by: Max Reitz > Signed-off-by: Hu Tao raw-posix needs to error out if called with preallocation=metadata, it doesn't implement this option. Kevin
Re: [Qemu-devel] [PATCH v12 6/6] qcow2: Add falloc and full preallocation option
Am 11.07.2014 um 08:10 hat Hu Tao geschrieben:
> This adds preallocation=falloc and preallocation=full mode to qcow2
> image creation.
>
> preallocation=full allocates disk space by writing zeros to disk to
> ensure disk space in any cases.
>
> preallocation=falloc likes preallocation=full, but allocates disk space
> by posix_fallocate().
>
> Signed-off-by: Hu Tao
> ---
> block/qcow2.c | 31 --
> tests/qemu-iotests/082.out | 54
> +++---
> 2 files changed, 56 insertions(+), 29 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index cfba93b..f48e915 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1593,6 +1593,9 @@ static int preallocate(BlockDriverState *bs)
> return 0;
> }
>
> +static uint64_t minimal_blob_size(uint64_t ts, int cb, int spcb,
> + uint64_t overhead);
> +
> static int qcow2_create2(const char *filename, int64_t total_size,
> const char *backing_file, const char
> *backing_format,
> int flags, size_t cluster_size, PreallocMode
> prealloc,
> @@ -1628,6 +1631,29 @@ static int qcow2_create2(const char *filename, int64_t
> total_size,
> Error *local_err = NULL;
> int ret;
>
> +if (prealloc == PREALLOC_MODE_FULL || prealloc == PREALLOC_MODE_FALLOC) {
> +int64_t meta_size = 0;
> +uint64_t nl2e;
> +
> +total_size = align_offset(total_size, cluster_size);
I don't think it's a good idea to let the virtual disk size depend on
whether preallocation is enabled or not. You should always get the same
rounding (which is rounding up to the next sector boundary).
Do you need full clusters for your calculations below or what is this
good for? If so, please use a local variable and leave the value used
for the bdrv_truncate() call unmodified.
> +/* total size of L2 tables */
> +nl2e = total_size >> cluster_bits;
> +nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
> +uint64_t l2_clusters = nl2e * sizeof(uint64_t) >> cluster_bits;
> +
> +meta_size =
> +(1 +
> + minimal_blob_size(total_size >> BDRV_SECTOR_BITS,
> + cluster_bits, cluster_bits - BDRV_SECTOR_BITS,
> + 1 + l2_clusters +
> + (total_size >> cluster_bits)) +
> + l2_clusters) << cluster_bits;
> +
> +qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size + meta_size);
> +qemu_opt_set(opts, BLOCK_OPT_PREALLOC,
> PreallocMode_lookup[prealloc]);
> +}
> +
> ret = bdrv_create_file(filename, opts, &local_err);
> if (ret < 0) {
> error_propagate(errp, local_err);
Kevin
[Qemu-devel] [PATCH] linux-user: Convert blkpg to use a special subop handler
The blkpg ioctl can take different payloads depending on the opcode in
its payload structure. Create a new special ioctl handler that can only
deal with partition style ones for now.
This patch fixes running parted for me.
Signed-off-by: Alexander Graf
---
linux-user/ioctls.h| 3 ++-
linux-user/syscall.c | 53 ++
linux-user/syscall_types.h | 2 +-
3 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 609b27c..e672655 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -78,7 +78,8 @@
IOCTL(BLKRAGET, IOC_R, MK_PTR(TYPE_LONG))
IOCTL(BLKSSZGET, IOC_R, MK_PTR(TYPE_LONG))
IOCTL(BLKBSZGET, IOC_R, MK_PTR(TYPE_INT))
- IOCTL(BLKPG, IOC_W, MK_PTR(MK_STRUCT(STRUCT_blkpg_ioctl_arg)))
+ IOCTL_SPECIAL(BLKPG, IOC_W, do_ioctl_blkpg,
+ MK_PTR(MK_STRUCT(STRUCT_blkpg_ioctl_arg)))
#ifdef FIBMAP
IOCTL(FIBMAP, IOC_W | IOC_R, MK_PTR(TYPE_LONG))
#endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a50229d..f6c887f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3652,6 +3652,59 @@ out:
return ret;
}
+static abi_long do_ioctl_blkpg(const IOCTLEntry *ie, uint8_t *buf_temp, int fd,
+ abi_long cmd, abi_long arg)
+{
+void *argptr;
+int target_size;
+const argtype *arg_type = ie->arg_type;
+const argtype part_arg_type[] = { MK_STRUCT(STRUCT_blkpg_partition) };
+abi_long ret;
+
+struct blkpg_ioctl_arg *host_blkpg = (void*)buf_temp;
+struct blkpg_partition host_part;
+
+/* Read and convert blkpg */
+arg_type++;
+target_size = thunk_type_size(arg_type, 0);
+argptr = lock_user(VERIFY_READ, arg, target_size, 1);
+if (!argptr) {
+ret = -TARGET_EFAULT;
+goto out;
+}
+thunk_convert(buf_temp, argptr, arg_type, THUNK_HOST);
+unlock_user(argptr, arg, 0);
+
+switch (host_blkpg->op) {
+case BLKPG_ADD_PARTITION:
+case BLKPG_DEL_PARTITION:
+/* payload is struct blkpg_partition */
+break;
+default:
+/* Unknown opcode */
+ret = -TARGET_EINVAL;
+goto out;
+}
+
+/* Read and convert blkpg->data */
+arg = (abi_long)(uintptr_t)host_blkpg->data;
+target_size = thunk_type_size(part_arg_type, 0);
+argptr = lock_user(VERIFY_READ, arg, target_size, 1);
+if (!argptr) {
+ret = -TARGET_EFAULT;
+goto out;
+}
+thunk_convert(&host_part, argptr, part_arg_type, THUNK_HOST);
+unlock_user(argptr, arg, 0);
+
+/* Swizzle the data pointer to our local copy and call! */
+host_blkpg->data = &host_part;
+ret = get_errno(ioctl(fd, ie->host_cmd, host_blkpg));
+
+out:
+return ret;
+}
+
static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp,
int fd, abi_long cmd, abi_long arg)
{
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index 9d0c92d..1fd4ee0 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -252,4 +252,4 @@ STRUCT(blkpg_ioctl_arg,
TYPE_INT, /* op */
TYPE_INT, /* flags */
TYPE_INT, /* datalen */
- MK_PTR(MK_STRUCT(STRUCT_blkpg_partition))) /* data */
+ TYPE_PTRVOID) /* data */
--
1.7.12.4
[Qemu-devel] [PATCH] linux-user: Simplify boundary checks on g_posix_timers range
We check whether the passed in counter value is negative on all calls
that involve g_posix_timers. However, we AND the value down to 16 bits
right before the check, so they can never be negative.
Simplify all the checks and remove the useless negativity check.
Signed-off-by: Alexander Graf
---
linux-user/syscall.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f6c887f..bb68dd4 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9509,7 +9509,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
/* args: timer_t timerid, int flags, const struct itimerspec
*new_value,
* struct itimerspec * old_value */
arg1 &= 0x;
-if (arg3 == 0 || arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
+if (arg3 == 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
timer_t htimer = g_posix_timers[arg1];
@@ -9531,7 +9531,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
arg1 &= 0x;
if (!arg2) {
return -TARGET_EFAULT;
-} else if (arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
+} else if (arg1 >= ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
timer_t htimer = g_posix_timers[arg1];
@@ -9551,7 +9551,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
{
/* args: timer_t timerid */
arg1 &= 0x;
-if (arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
+if (arg1 >= ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
timer_t htimer = g_posix_timers[arg1];
@@ -9566,7 +9566,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
{
/* args: timer_t timerid */
arg1 &= 0x;
-if (arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
+if (arg1 >= ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
timer_t htimer = g_posix_timers[arg1];
--
1.7.12.4
[Qemu-devel] [Bug 1359930] Re: [ARMv5] Integrator/CP regression when reading FPSID instruction
Hi Jakub,
Thanks for the test case. I've just done a git bisect using your test
image above and it seems as if the offending commit is this:
commit 2c7ffc414d8591018248b5487757e45f7bb6bd3c
Author: Peter Maydell
Date: Tue Apr 15 19:18:40 2014 +0100
target-arm: Fix VFP enables for AArch32 EL0 under AArch64 EL1
The current A32/T32 decoder bases its "is VFP/Neon enabled?" check
on the FPSCR.EN bit. This is correct if EL1 is AArch32, but for
an AArch64 EL1 the logic is different: it must act as if FPSCR.EN
is always set. Instead, trapping must happen according to CPACR
bits for cp10/cp11; these cover all of FP/Neon, including the
FPSCR/FPSID/MVFR register accesses which FPSCR.EN does not affect.
Add support for CPACR checks (which are also required for ARMv7,
but were unimplemented because Linux happens not to use them)
and make sure they generate exceptions with the correct syndrome.
We actually return incorrect syndrome information for cases
where FP is disabled but the specific instruction bit pattern
is unallocated: strictly these should be the Uncategorized
exception, not a "SIMD disabled" exception. This should be
mostly harmless, and the structure of the A32/T32 VFP/Neon
decoder makes it painful to put the 'FP disabled?' checks in
the right places.
Signed-off-by: Peter Maydell
Reviewed-by: Peter Crosthwaite
(Diff is viewable at
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=2c7ffc414d8591018248b5487757e45f7bb6bd3c)
HTH,
Mark.
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1359930
Title:
[ARMv5] Integrator/CP regression when reading FPSID instruction
Status in Home for various HelenOS development branches:
New
Status in QEMU:
New
Bug description:
There seems to be a regression in QEMU 2.1.0 which demonstrates itself
when running the attached HelenOS Integrator/CP (i.e. ARMv5) image. The
offending instruction seems to be:
vmrs r0, fpsid
Upon its execution, HelenOS kernel receives an Undefined instruction
exception (which it does not anticipate at that point) and crashes.
QEMU 2.0.0 was not affected by this issue.
Command line to reproduce with QEMU 2.1.0:
$ qemu-system-arm -M integratorcp -kernel image.boot -s -S &
$ /usr/local/cross/arm32/bin/arm-linux-gnueabi-gdb
...
(gdb) target remote localhost:1234
Remote debugging using localhost:1234
warning: Can not parse XML target description; XML support was disabled at
compile time
0x in ?? ()
(gdb) symbol-file kernel/kernel.raw
Reading symbols from
/home/jermar/software/HelenOS.mainline/kernel/kernel.raw...done.
(gdb) break ras_check
Breakpoint 1 at 0x80a021bc: file arch/arm32/src/ras.c, line 67.
(gdb) c
Continuing.
Breakpoint 1, ras_check (n=1, istate=0x813e7f70) at arch/arm32/src/ras.c:67
67{
(gdb) set radix 16
Input and output radices now set to decimal 16, hex 10, octal 20.
(gdb) print istate->pc
$1 = 0x80a02458
(gdb) disassemble 0x80a02458
Dump of assembler code for function fpsid_read:
0x80a02454 <+0>: vmrsr0, fpsid <===
UNDEFINED EXCEPTION INSTRUCTION
0x80a02458 <+4>: mov pc, lr
End of assembler dump.
The Undefined instruction exception is not expected at this point when
executing the VMRS r0,fpsid instruction.
To manage notifications about this bug go to:
https://bugs.launchpad.net/helenos/+bug/1359930/+subscriptions
Re: [Qemu-devel] [PATCH v3 00/21] target-mips: add MIPS64R6 Instruction Set support
ping Anybody? There hasn't been any feedback on this patchset for almost 2 months now... On 12/08/2014 12:36, Leon Alrae wrote: > ping > > On 05/08/2014 10:26, Leon Alrae wrote: >> ping >> >> http://patchwork.ozlabs.org/patch/365066/ >> http://patchwork.ozlabs.org/patch/365042/ >> http://patchwork.ozlabs.org/patch/365046/ >> http://patchwork.ozlabs.org/patch/365056/ >> http://patchwork.ozlabs.org/patch/365059/ >> >> On 27/06/2014 16:21, Leon Alrae wrote: >>> The following patchset implements MIPS64 Release 6 Instruction Set. >>> New instructions are added and also there is a number of instructions which >>> are deleted or moved (the encodings have changed). >>> >>> The MIPS64 Release 6 documentation is available: >>> http://www.imgtec.com/mips/architectures/mips64.asp >>> >>> The following patch series is focusing on instruction set changes only. >>> There is also a new generic cpu supporting R6. >>> >>> Please note that even though the new Floating Point instructions were added, >>> softfloat for MIPS has not been updated yet (in R6 MIPS FPU is updated to >>> IEEE2008). Also, current patchset does not include MIPS64 Privileged >>> Resource >>> Architecture modifications. All those changes will follow the current >>> patchset >>> soon. >>> >>> v3: >>> * addressed further comments and suggestions (more detailed changelog >>> included >>> in the separate patches). >>> * dropped patch adding function pointers due to its doubtful usefulness >>> * rebased >>> v2: >>> * addressed all comments so far from Richard and Aurelien. More detailed >>> changelog included in the separate patches. >>> * added missing zero register case for LSA, ALIGN and BITSWAP instructions >>> >>> Leon Alrae (17): >>> target-mips: define ISA_MIPS64R6 >>> target-mips: signal RI Exception on instructions removed in R6 >>> target-mips: add SELEQZ and SELNEZ instructions >>> target-mips: move LL and SC instructions >>> target-mips: extract decode_opc_special* from decode_opc >>> target-mips: split decode_opc_special* into *_r6 and *_legacy >>> target-mips: signal RI Exception on DSP and Loongson instructions >>> target-mips: move PREF, CACHE, LLD and SCD instructions >>> target-mips: redefine Integer Multiply and Divide instructions >>> target-mips: move CLO, DCLO, CLZ, DCLZ, SDBBP and free special2 in R6 >>> target-mips: Status.UX/SX/KX enable 32-bit address wrapping >>> target-mips: add AUI, LSA and PCREL instruction families >>> softfloat: add functions corresponding to IEEE-2008 min/maxNumMag >>> target-mips: add new Floating Point instructions >>> target-mips: do not allow Status.FR=0 mode in 64-bit FPU >>> mips_malta: update malta's pseudo-bootloader - replace JR with JALR >>> target-mips: define a new generic CPU supporting MIPS64 Release 6 ISA >>> >>> Yongbok Kim (4): >>> target-mips: add ALIGN, DALIGN, BITSWAP and DBITSWAP instructions >>> target-mips: add compact and CP1 branches >>> target-mips: add new Floating Point Comparison instructions >>> target-mips: remove JR, BLTZAL, BGEZAL and add NAL, BAL instructions >>> >>> disas/mips.c | 211 +++- >>> fpu/softfloat.c | 37 +- >>> hw/mips/mips_malta.c | 10 +- >>> include/fpu/softfloat.h |4 + >>> target-mips/cpu.h| 18 +- >>> target-mips/helper.h | 52 + >>> target-mips/mips-defs.h | 28 +- >>> target-mips/op_helper.c | 238 +++ >>> target-mips/translate.c | 3814 >>> +++--- >>> target-mips/translate_init.c | 30 + >>> 10 files changed, 3430 insertions(+), 1012 deletions(-) >>> >> >> > >
[Qemu-devel] [PATCH] qemu-iotests: stop using /tmp directly
Before this patch you could not run multiple tests concurrently as they
might clobber each other test files. This patch solves that by using
random temporary directory instead of `/tmp` (for writing output in the
individual tests and valgrind logs).
Furthermore, this patch stops removing everything in `/tmp/` matching a
certain pattern (`/tmp/*.{err,out,time}`). These might not be a property
of QEMU.
Running multiple concurrent tests in the same object directory is still
not supported though as the scratch directory and .bad and .notrun files
still interfere with each other. Also not touched is the situation that
/tmp/check.log and /tmp/check.sts are hard-coded (and thus unusable in
concurrent tests).
Signed-off-by: Peter Wu
---
Hi,
This patch introduces a dependency on mktemp of coreutils. I could still not get
concurrent tests to work fully reliably (test 030 failed randomly with QED):
FAIL: test_ignore (__main__.TestEIO)
--
Traceback (most recent call last):
File "030", line 223, in test_ignore
self.assert_qmp(result, 'return[0]/paused', False)
File "/tmp/qemu/tests/qemu-iotests/iotests.py", line 233, in assert_qmp
result = self.dictpath(d, path)
File "/tmp/qemu/tests/qemu-iotests/iotests.py", line 221, in dictpath
self.fail('invalid index "%s" in path "%s" in "%s"' % (idx, path,
str(d)))
AssertionError: invalid index "0" in path "return[0]/paused" in "[]"
I still think that the patches are valuable though, it reduces predictable file
names.
Kind regards,
Peter
---
tests/qemu-iotests/001 | 2 +-
tests/qemu-iotests/002 | 2 +-
tests/qemu-iotests/003 | 2 +-
tests/qemu-iotests/004 | 2 +-
tests/qemu-iotests/005 | 2 +-
tests/qemu-iotests/006 | 2 +-
tests/qemu-iotests/007 | 2 +-
tests/qemu-iotests/008 | 2 +-
tests/qemu-iotests/009 | 2 +-
tests/qemu-iotests/010 | 2 +-
tests/qemu-iotests/011 | 2 +-
tests/qemu-iotests/012 | 2 +-
tests/qemu-iotests/013 | 2 +-
tests/qemu-iotests/014 | 2 +-
tests/qemu-iotests/015 | 2 +-
tests/qemu-iotests/016 | 2 +-
tests/qemu-iotests/017 | 2 +-
tests/qemu-iotests/018 | 2 +-
tests/qemu-iotests/019 | 2 +-
tests/qemu-iotests/020 | 2 +-
tests/qemu-iotests/021 | 2 +-
tests/qemu-iotests/022 | 2 +-
tests/qemu-iotests/023 | 2 +-
tests/qemu-iotests/024 | 2 +-
tests/qemu-iotests/025 | 2 +-
tests/qemu-iotests/026 | 2 +-
tests/qemu-iotests/027 | 2 +-
tests/qemu-iotests/028 | 2 +-
tests/qemu-iotests/029 | 2 +-
tests/qemu-iotests/031 | 2 +-
tests/qemu-iotests/032 | 2 +-
tests/qemu-iotests/033 | 2 +-
tests/qemu-iotests/034 | 2 +-
tests/qemu-iotests/035 | 2 +-
tests/qemu-iotests/036 | 2 +-
tests/qemu-iotests/037 | 2 +-
tests/qemu-iotests/038 | 2 +-
tests/qemu-iotests/039 | 2 +-
tests/qemu-iotests/042 | 2 +-
tests/qemu-iotests/043 | 2 +-
tests/qemu-iotests/046 | 2 +-
tests/qemu-iotests/047 | 2 +-
tests/qemu-iotests/049 | 2 +-
tests/qemu-iotests/050 | 2 +-
tests/qemu-iotests/051 | 2 +-
tests/qemu-iotests/052 | 2 +-
tests/qemu-iotests/053 | 2 +-
tests/qemu-iotests/054 | 2 +-
tests/qemu-iotests/058 | 2 +-
tests/qemu-iotests/059 | 2 +-
tests/qemu-iotests/060 | 2 +-
tests/qemu-iotests/061 | 2 +-
tests/qemu-iotests/062 | 2 +-
tests/qemu-iotests/063 | 2 +-
tests/qemu-iotests/064 | 2 +-
tests/qemu-iotests/066 | 2 +-
tests/qemu-iotests/067 | 2 +-
tests/qemu-iotests/068 | 2 +-
tests/qemu-iotests/069 | 2 +-
tests/qemu-iotests/070 | 2 +-
tests/qemu-iotests/071 | 2 +-
tests/qemu-iotests/072 | 2 +-
tests/qemu-iotests/073 | 2 +-
tests/qemu-iotests/075 | 2 +-
tests/qemu-iotests/076 | 2 +-
tests/qemu-iotests/077 | 2 +-
tests/qemu-iotests/078 | 2 +-
tests/qemu-iotests/079 | 2 +-
tests/qemu-iotests/080 | 2 +-
tests/qemu-iotests/081 | 2 +-
tests/qemu-iotests/082 | 2 +-
tests/qemu-iotests/083 | 2 +-
tests/qemu-iotests/084 | 2 +-
tests/qemu-iotests/086 | 2 +-
tests/qemu-iotests/087 | 2 +-
tests/qemu-iotests/088 | 2 +-
tests/qemu-iotests/089 | 2 +-
tests/qemu-iotests/090 | 2 +-
tests/qemu-iotests/092 | 2 +-
tests/qemu-iotests/check | 9 ++---
tests/qemu-iotests/common.rc | 7 ---
81 files changed, 89 insertions(+), 85 deletions(-)
diff --git a/tests/qemu-iotests/001 b/tests/qemu-iotests/001
index 4e16469..6472c67 100755
--- a/tests/qemu-iotests/001
+++ b/tests/qemu-iotests/001
@@ -25,7 +25,7 @@ seq=`basename $0`
echo "QA output created by $seq"
here=`pwd`
-tmp=/tmp/$$
+tmp=${QEMU_IOTESTS_TMPDIR:-/tmp}/$$
status=1 # failure
Re: [Qemu-devel] [PATCH] linux-user: Simplify boundary checks on g_posix_timers range
On 22 August 2014 12:19, Alexander Graf wrote: > We check whether the passed in counter value is negative on all calls > that involve g_posix_timers. However, we AND the value down to 16 bits > right before the check, so they can never be negative. ...but why exactly are we doing that AND with 0x ?? It seems unlikely that the kernel really allows random garbage in the top half of the timer ID arguments, so maybe we should drop the mask and keep the <0 bounds checks? -- PMM
Re: [Qemu-devel] [PATCH] linux-user: Simplify boundary checks on g_posix_timers range
Am 22.08.2014 13:33, schrieb Peter Maydell: > On 22 August 2014 12:19, Alexander Graf wrote: >> We check whether the passed in counter value is negative on all calls >> that involve g_posix_timers. However, we AND the value down to 16 bits >> right before the check, so they can never be negative. > > ...but why exactly are we doing that AND with 0x ?? It seems > unlikely that the kernel really allows random garbage in the top > half of the timer ID arguments, so maybe we should drop the > mask and keep the <0 bounds checks? Or maybe just use a local int16_t variable? I.e., should 0x match the <0 check there or not? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [Bug 1359930] Re: [ARMv5] Integrator/CP regression when reading FPSID register
** Summary changed:
- [ARMv5] Integrator/CP regression when reading FPSID instruction
+ [ARMv5] Integrator/CP regression when reading FPSID register
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1359930
Title:
[ARMv5] Integrator/CP regression when reading FPSID register
Status in Home for various HelenOS development branches:
New
Status in QEMU:
New
Bug description:
There seems to be a regression in QEMU 2.1.0 which demonstrates itself
when running the attached HelenOS Integrator/CP (i.e. ARMv5) image. The
offending instruction seems to be:
vmrs r0, fpsid
Upon its execution, HelenOS kernel receives an Undefined instruction
exception (which it does not anticipate at that point) and crashes.
QEMU 2.0.0 was not affected by this issue.
Command line to reproduce with QEMU 2.1.0:
$ qemu-system-arm -M integratorcp -kernel image.boot -s -S &
$ /usr/local/cross/arm32/bin/arm-linux-gnueabi-gdb
...
(gdb) target remote localhost:1234
Remote debugging using localhost:1234
warning: Can not parse XML target description; XML support was disabled at
compile time
0x in ?? ()
(gdb) symbol-file kernel/kernel.raw
Reading symbols from
/home/jermar/software/HelenOS.mainline/kernel/kernel.raw...done.
(gdb) break ras_check
Breakpoint 1 at 0x80a021bc: file arch/arm32/src/ras.c, line 67.
(gdb) c
Continuing.
Breakpoint 1, ras_check (n=1, istate=0x813e7f70) at arch/arm32/src/ras.c:67
67{
(gdb) set radix 16
Input and output radices now set to decimal 16, hex 10, octal 20.
(gdb) print istate->pc
$1 = 0x80a02458
(gdb) disassemble 0x80a02458
Dump of assembler code for function fpsid_read:
0x80a02454 <+0>: vmrsr0, fpsid <===
UNDEFINED EXCEPTION INSTRUCTION
0x80a02458 <+4>: mov pc, lr
End of assembler dump.
The Undefined instruction exception is not expected at this point when
executing the VMRS r0,fpsid instruction.
To manage notifications about this bug go to:
https://bugs.launchpad.net/helenos/+bug/1359930/+subscriptions
Re: [Qemu-devel] [PATCH] linux-user: Simplify boundary checks on g_posix_timers range
On 22.08.14 13:33, Peter Maydell wrote: > On 22 August 2014 12:19, Alexander Graf wrote: >> We check whether the passed in counter value is negative on all calls >> that involve g_posix_timers. However, we AND the value down to 16 bits >> right before the check, so they can never be negative. > > ...but why exactly are we doing that AND with 0x ?? It seems > unlikely that the kernel really allows random garbage in the top > half of the timer ID arguments, so maybe we should drop the > mask and keep the <0 bounds checks? Or we drop the AND and and the <0 check and treat arg1 as unsigned ;). Alex
Re: [Qemu-devel] [Bug 1359930] Re: [ARMv5] Integrator/CP regression when reading FPSID instruction
On 22 August 2014 12:17, Mark Cave-Ayland wrote:
> Hi Jakub,
>
> Thanks for the test case. I've just done a git bisect using your test
> image above and it seems as if the offending commit is this:
>
>
> commit 2c7ffc414d8591018248b5487757e45f7bb6bd3c
> Author: Peter Maydell
> Date: Tue Apr 15 19:18:40 2014 +0100
>
> target-arm: Fix VFP enables for AArch32 EL0 under AArch64 EL1
>
> The current A32/T32 decoder bases its "is VFP/Neon enabled?" check
> on the FPSCR.EN bit. This is correct if EL1 is AArch32, but for
> an AArch64 EL1 the logic is different: it must act as if FPSCR.EN
> is always set. Instead, trapping must happen according to CPACR
> bits for cp10/cp11; these cover all of FP/Neon, including the
> FPSCR/FPSID/MVFR register accesses which FPSCR.EN does not affect.
> Add support for CPACR checks (which are also required for ARMv7,
> but were unimplemented because Linux happens not to use them)
> and make sure they generate exceptions with the correct syndrome.
Thanks for the bisect; this was actually my primary suspect
for the offending commit but I hadn't got round to looking at it.
We're trapping based on the CPACR (c1_coproc) bits being zero,
but that register only appears in ARMv6 and later, so we
accidentally disabled VFP in ARMv5 CPUs.
Probably the best fix is to mak cpu_get_tb_cpu_state() do
if (arm_feature(env, ARM_FEATURE_V6)) {
fpen = extract32(env->cp15.c1_coproc, 20, 2);
} else {
/* CPACR doesn't exist before v6 so VFP always accessible */
fpen = 3;
}
-- PMM
Re: [Qemu-devel] [PATCH] linux-user: Simplify boundary checks on g_posix_timers range
On 22 August 2014 12:42, Alexander Graf wrote: > On 22.08.14 13:33, Peter Maydell wrote: >> On 22 August 2014 12:19, Alexander Graf wrote: >>> We check whether the passed in counter value is negative on all calls >>> that involve g_posix_timers. However, we AND the value down to 16 bits >>> right before the check, so they can never be negative. >> >> ...but why exactly are we doing that AND with 0x ?? It seems >> unlikely that the kernel really allows random garbage in the top >> half of the timer ID arguments, so maybe we should drop the >> mask and keep the <0 bounds checks? > > Or we drop the AND and and the <0 check and treat arg1 as unsigned ;). That probably just requires equally many changes to code that is currently correct because the arg* are signed but would need changes if they became unsigned. -- PMM
Re: [Qemu-devel] [PATCH] linux-user: Simplify boundary checks on g_posix_timers range
On 22 August 2014 12:36, Andreas Färber wrote: > Am 22.08.2014 13:33, schrieb Peter Maydell: >> On 22 August 2014 12:19, Alexander Graf wrote: >>> We check whether the passed in counter value is negative on all calls >>> that involve g_posix_timers. However, we AND the value down to 16 bits >>> right before the check, so they can never be negative. >> >> ...but why exactly are we doing that AND with 0x ?? It seems >> unlikely that the kernel really allows random garbage in the top >> half of the timer ID arguments, so maybe we should drop the >> mask and keep the <0 bounds checks? > > Or maybe just use a local int16_t variable? I.e., should 0x match > the <0 check there or not? The kernel seems to use 'int' for the timer id type, which suggests that this mask is just wrong. -- PMM
Re: [Qemu-devel] [PATCH] linux-user: Simplify boundary checks on g_posix_timers range
On 22.08.14 13:44, Peter Maydell wrote: > On 22 August 2014 12:42, Alexander Graf wrote: >> On 22.08.14 13:33, Peter Maydell wrote: >>> On 22 August 2014 12:19, Alexander Graf wrote: We check whether the passed in counter value is negative on all calls that involve g_posix_timers. However, we AND the value down to 16 bits right before the check, so they can never be negative. >>> >>> ...but why exactly are we doing that AND with 0x ?? It seems >>> unlikely that the kernel really allows random garbage in the top >>> half of the timer ID arguments, so maybe we should drop the >>> mask and keep the <0 bounds checks? >> >> Or we drop the AND and and the <0 check and treat arg1 as unsigned ;). > > That probably just requires equally many changes to > code that is currently correct because the arg* are > signed but would need changes if they became unsigned. Well, I do have a downstream patch that makes them unsigned, so I'd rather like to make the code as stable to that as I can ;). Alex
Re: [Qemu-devel] [PATCH] linux-user: Simplify boundary checks on g_posix_timers range
On 22 August 2014 12:45, Alexander Graf wrote: > On 22.08.14 13:44, Peter Maydell wrote: >> On 22 August 2014 12:42, Alexander Graf wrote: >>> Or we drop the AND and and the <0 check and treat arg1 as unsigned ;). >> >> That probably just requires equally many changes to >> code that is currently correct because the arg* are >> signed but would need changes if they became unsigned. > > Well, I do have a downstream patch that makes them unsigned, so I'd > rather like to make the code as stable to that as I can ;). Yeah, I know. When I was looking through your patch tree I saw that one and my reaction was "why on earth did you do that?"... -- PMM
Re: [Qemu-devel] [PATCH] linux-user: Simplify boundary checks on g_posix_timers range
On 22.08.14 13:49, Peter Maydell wrote: > On 22 August 2014 12:45, Alexander Graf wrote: >> On 22.08.14 13:44, Peter Maydell wrote: >>> On 22 August 2014 12:42, Alexander Graf wrote: Or we drop the AND and and the <0 check and treat arg1 as unsigned ;). >>> >>> That probably just requires equally many changes to >>> code that is currently correct because the arg* are >>> signed but would need changes if they became unsigned. >> >> Well, I do have a downstream patch that makes them unsigned, so I'd >> rather like to make the code as stable to that as I can ;). > > Yeah, I know. When I was looking through your patch tree > I saw that one and my reaction was "why on earth did > you do that?"... I don't fully remember all the glorious details either - and there's a good reason I never pushed it upstream :). It seemed to make the code more robust though. Maybe we'll just ditch it again sooner or later. Or push it upstream and make unsigned the default (which IMHO is a lot more sane, you get way less unwanted side effects). Alex
[Qemu-devel] [PATCH v2] linux-user: Simplify timerid checks on g_posix_timers range
We check whether the passed in timer id is negative on all calls
that involve g_posix_timers.
However, these checks are bogus. First off we limit the timer_id to
16 bits which is not what Linux does. Then we check whether it's negative
which it can't be because we masked it.
We can safely remove the masking. For the negativity check we can just
treat the timerid as unsigned and only check for upper boundaries.
Signed-off-by: Alexander Graf
---
v1 -> v2:
- drop 0x mask
- explicitly cast to unsigned because the mask is missing now
---
linux-user/syscall.c | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f6c887f..92b6a38 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9508,11 +9508,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
{
/* args: timer_t timerid, int flags, const struct itimerspec
*new_value,
* struct itimerspec * old_value */
-arg1 &= 0x;
-if (arg3 == 0 || arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
+target_ulong timerid = arg1;
+
+if (arg3 == 0 || timerid >= ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
struct itimerspec hspec_new = {{0},}, hspec_old = {{0},};
target_to_host_itimerspec(&hspec_new, arg3);
@@ -9528,13 +9529,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
case TARGET_NR_timer_gettime:
{
/* args: timer_t timerid, struct itimerspec *curr_value */
-arg1 &= 0x;
+target_ulong timerid = arg1;
+
if (!arg2) {
return -TARGET_EFAULT;
-} else if (arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
+} else if (timerid >= ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
struct itimerspec hspec;
ret = get_errno(timer_gettime(htimer, &hspec));
@@ -9550,11 +9552,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
case TARGET_NR_timer_getoverrun:
{
/* args: timer_t timerid */
-arg1 &= 0x;
-if (arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
+target_ulong timerid = arg1;
+
+if (timerid >= ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
ret = get_errno(timer_getoverrun(htimer));
}
break;
@@ -9565,13 +9568,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
case TARGET_NR_timer_delete:
{
/* args: timer_t timerid */
-arg1 &= 0x;
-if (arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
+target_ulong timerid = arg1;
+
+if (timerid >= ARRAY_SIZE(g_posix_timers)) {
ret = -TARGET_EINVAL;
} else {
-timer_t htimer = g_posix_timers[arg1];
+timer_t htimer = g_posix_timers[timerid];
ret = get_errno(timer_delete(htimer));
-g_posix_timers[arg1] = 0;
+g_posix_timers[timerid] = 0;
}
break;
}
--
1.7.12.4
[Qemu-devel] glusterfs: do not log to stdout if daemonized
Else stdout is not closed correctly.
Signed-off-by: Dietmar Maurer
Index: new/block/gluster.c
===
--- new.orig/block/gluster.c2014-08-22 13:21:39.0 +0200
+++ new/block/gluster.c 2014-08-22 13:25:18.0 +0200
@@ -196,9 +196,11 @@
* TODO: Use GF_LOG_ERROR instead of hard code value of 4 here when
* GlusterFS makes GF_LOG_* macros available to libgfapi users.
*/
-ret = glfs_set_logging(glfs, "-", 4);
-if (ret < 0) {
-goto out;
+if (!is_daemonized()) {
+ret = glfs_set_logging(glfs, "-", 4);
+if (ret < 0) {
+goto out;
+}
}
ret = glfs_init(glfs);
Re: [Qemu-devel] [PATCH v2] linux-user: Simplify timerid checks on g_posix_timers range
On 22 August 2014 12:56, Alexander Graf wrote: > We check whether the passed in timer id is negative on all calls > that involve g_posix_timers. > > However, these checks are bogus. First off we limit the timer_id to > 16 bits which is not what Linux does. Then we check whether it's negative > which it can't be because we masked it. > > We can safely remove the masking. For the negativity check we can just > treat the timerid as unsigned and only check for upper boundaries. Timer IDs aren't unsigned for the kernel; why not just drop the mask and keep the <0 checks? -- PMM
Re: [Qemu-devel] [PATCH v2] linux-user: Simplify timerid checks on g_posix_timers range
Hi,
as in the kernel timer_t is an "int" (as said PMM), you should cast to "int" to
remove garbage on 64bit hosts and check sign ...
Regards,
Laurent
> Le 22 août 2014 à 13:56, Alexander Graf a écrit :
>
>
> We check whether the passed in timer id is negative on all calls
> that involve g_posix_timers.
>
> However, these checks are bogus. First off we limit the timer_id to
> 16 bits which is not what Linux does. Then we check whether it's negative
> which it can't be because we masked it.
>
> We can safely remove the masking. For the negativity check we can just
> treat the timerid as unsigned and only check for upper boundaries.
>
> Signed-off-by: Alexander Graf
>
> ---
>
> v1 -> v2:
>
> - drop 0x mask
> - explicitly cast to unsigned because the mask is missing now
>
> ---
> linux-user/syscall.c | 30 +-
> 1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index f6c887f..92b6a38 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9508,11 +9508,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> {
> /* args: timer_t timerid, int flags, const struct itimerspec *new_value,
> * struct itimerspec * old_value */
> - arg1 &= 0x;
> - if (arg3 == 0 || arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
> + target_ulong timerid = arg1;
> +
> + if (arg3 == 0 || timerid >= ARRAY_SIZE(g_posix_timers)) {
> ret = -TARGET_EINVAL;
> } else {
> - timer_t htimer = g_posix_timers[arg1];
> + timer_t htimer = g_posix_timers[timerid];
> struct itimerspec hspec_new = {{0},}, hspec_old = {{0},};
>
> target_to_host_itimerspec(&hspec_new, arg3);
> @@ -9528,13 +9529,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> case TARGET_NR_timer_gettime:
> {
> /* args: timer_t timerid, struct itimerspec *curr_value */
> - arg1 &= 0x;
> + target_ulong timerid = arg1;
> +
> if (!arg2) {
> return -TARGET_EFAULT;
> - } else if (arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
> + } else if (timerid >= ARRAY_SIZE(g_posix_timers)) {
> ret = -TARGET_EINVAL;
> } else {
> - timer_t htimer = g_posix_timers[arg1];
> + timer_t htimer = g_posix_timers[timerid];
> struct itimerspec hspec;
> ret = get_errno(timer_gettime(htimer, &hspec));
>
> @@ -9550,11 +9552,12 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> case TARGET_NR_timer_getoverrun:
> {
> /* args: timer_t timerid */
> - arg1 &= 0x;
> - if (arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
> + target_ulong timerid = arg1;
> +
> + if (timerid >= ARRAY_SIZE(g_posix_timers)) {
> ret = -TARGET_EINVAL;
> } else {
> - timer_t htimer = g_posix_timers[arg1];
> + timer_t htimer = g_posix_timers[timerid];
> ret = get_errno(timer_getoverrun(htimer));
> }
> break;
> @@ -9565,13 +9568,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> case TARGET_NR_timer_delete:
> {
> /* args: timer_t timerid */
> - arg1 &= 0x;
> - if (arg1 < 0 || arg1 >= ARRAY_SIZE(g_posix_timers)) {
> + target_ulong timerid = arg1;
> +
> + if (timerid >= ARRAY_SIZE(g_posix_timers)) {
> ret = -TARGET_EINVAL;
> } else {
> - timer_t htimer = g_posix_timers[arg1];
> + timer_t htimer = g_posix_timers[timerid];
> ret = get_errno(timer_delete(htimer));
> - g_posix_timers[arg1] = 0;
> + g_posix_timers[timerid] = 0;
> }
> break;
> }
> --
> 1.7.12.4
>
>
Re: [Qemu-devel] [PATCH v2] linux-user: Simplify timerid checks on g_posix_timers range
On 22.08.14 14:07, Peter Maydell wrote: > On 22 August 2014 12:56, Alexander Graf wrote: >> We check whether the passed in timer id is negative on all calls >> that involve g_posix_timers. >> >> However, these checks are bogus. First off we limit the timer_id to >> 16 bits which is not what Linux does. Then we check whether it's negative >> which it can't be because we masked it. >> >> We can safely remove the masking. For the negativity check we can just >> treat the timerid as unsigned and only check for upper boundaries. > > Timer IDs aren't unsigned for the kernel; why not just drop > the mask and keep the <0 checks? Because I'd then have to carry yet another local patch. In Linux, the timer id is a "key" into a hash table that the kernel searches to find its timer. In QEMU it's an offset into an array. In both cases the syscall user receives it as a token from a create function and should treat it as opaque. So in the QEMU case it is unsigned, regardless of what the kernel allows it to be, because it's an array offset. Alex
Re: [Qemu-devel] [PATCH v2 1/7] build-sys: Move fifo8.c to hw/misc
On Fri, Aug 22, 2014 at 8:54 PM, Fam Zheng wrote:
> Since it has a dependency on vmstate and is only used by device
> emulation, moving out from util will make the archive more independent.
>
> Signed-off-by: Fam Zheng
We probably also want to move the corresponding header at some stage too.
but
Reviewed-by: Peter Crosthwaite
> ---
> hw/misc/Makefile.objs | 1 +
> {util => hw/misc}/fifo8.c | 0
> util/Makefile.objs| 1 -
> 3 files changed, 1 insertion(+), 1 deletion(-)
> rename {util => hw/misc}/fifo8.c (100%)
>
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 979e532..a4a5ad7 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -41,3 +41,4 @@ obj-$(CONFIG_SLAVIO) += slavio_misc.o
> obj-$(CONFIG_ZYNQ) += zynq_slcr.o
>
> obj-$(CONFIG_PVPANIC) += pvpanic.o
> +obj-y += fifo8.o
> diff --git a/util/fifo8.c b/hw/misc/fifo8.c
> similarity index 100%
> rename from util/fifo8.c
> rename to hw/misc/fifo8.c
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 6b3c83b..65a36f6 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -3,7 +3,6 @@ util-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
> event_notifier-win
> util-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o
> event_notifier-posix.o qemu-openpty.o
> util-obj-y += envlist.o path.o host-utils.o module.o
> util-obj-y += bitmap.o bitops.o hbitmap.o
> -util-obj-y += fifo8.o
> util-obj-y += acl.o
> util-obj-y += error.o qemu-error.o
> util-obj-$(CONFIG_POSIX) += compatfd.o
> --
> 2.0.3
>
>
Re: [Qemu-devel] [PATCH v2] linux-user: Simplify timerid checks on g_posix_timers range
On 22 August 2014 13:12, Alexander Graf wrote: > In Linux, the timer id is a "key" into a hash table that the kernel > searches to find its timer. In QEMU it's an offset into an array. > > In both cases the syscall user receives it as a token from a create > function and should treat it as opaque. > > So in the QEMU case it is unsigned, regardless of what the kernel allows > it to be, because it's an array offset. It's a number between 0 and 32. That doesn't imply that it has to be an unsigned variable, and we already have it in a signed variable arg1... -- PMM
Re: [Qemu-devel] [PATCH v12 0/6] qcow2, raw: add preallocation=full and preallocation=falloc
On Mon, Jul 28, 2014 at 04:48:46PM +0800, Hu Tao wrote: > ping... > > All the 6 patches have reviewed-by now. > > On Fri, Jul 11, 2014 at 02:09:57PM +0800, Hu Tao wrote: > > This series adds two preallocation mode to qcow2 and raw: > > > > Option preallocation=full preallocates disk space for image by writing > > zeros to disk, this ensures disk space in any cases. > > > > Option preallocation=falloc preallocates disk space by calling > > posix_fallocate(). This is faster than preallocation=full. Sorry if this was discussed before, but why would anyone use preallocation=full if preallocation=falloc was possible? Shouldn't preallocation=full simply use posix_fallocate if it's available, and fall back to writing zeroes if not? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Re: [Qemu-devel] [PATCH v2] linux-user: Simplify timerid checks on g_posix_timers range
On 22.08.14 14:25, Peter Maydell wrote: > On 22 August 2014 13:12, Alexander Graf wrote: >> In Linux, the timer id is a "key" into a hash table that the kernel >> searches to find its timer. In QEMU it's an offset into an array. >> >> In both cases the syscall user receives it as a token from a create >> function and should treat it as opaque. >> >> So in the QEMU case it is unsigned, regardless of what the kernel allows >> it to be, because it's an array offset. > > It's a number between 0 and 32. That doesn't imply that it has > to be an unsigned variable, and we already have it in a > signed variable arg1... Yes, so the end result will be the same. What's the point of this bike shedding? Alex
Re: [Qemu-devel] [PATCH v3 1/2] hw/misc/arm_sp810: Create SP810 device
On Fri, Aug 22, 2014 at 6:53 PM, Peter Maydell wrote: > On 22 August 2014 09:38, Aggeler Fabian wrote: >> >> On 19 Aug 2014, at 16:03, Peter Maydell wrote: >>> Which input signals in the hardware do these properties >>> correspond to? I can't figure out what the mapping is. >>> As far as I can tell from the documentation the bits >>> in SCCTRL are just always reset to 0 and are not >>> controlled by external signals (which is what qdev >>> properties should typically correspond to). >> >> Actually I don’t know how it is done in hardware. My goal was >> to reflect the default speed of the SP804 timer in QEMU’s vexpress >> emulation in the SCCTRL. What do you suggest in this case? > > Well, fundamentally you have to find out what the hardware > does, and do that. We can't model anything else. > With no actual clock muxing support it makes sense to me to pin the register value to a hardwired setting. This as least allows a guest to self-identify the one and only supported clocking setup without the need for dummy firmware. To do what the hardware does and have this value at it's docced reset you would really want to add the clock frequency selection support which means propagating the numbers through to timers etc making this series much more complicated. Regards, Peter >> I could remove the QOM properties and set the reset value of the >> TimerEnXSel bits in SCCTRL to 1 (TIMCLK) to match the SCCTRL >> with the speed of the SP804. > > The h/w docs suggest the reset value is 0. > I have a feeling you may be attempting to make QEMU's > hardware model include behaviour which is the result of > firmware code setting register values before booting the > kernel... > > thanks > -- PMM >
Re: [Qemu-devel] [PATCH trivial] Fix debug print warning
On Fri, Aug 22, 2014 at 1:38 PM, wrote:
> From: Gonglei
>
> Steps:
>
> 1.enable qemu debug print, using simply scprit as below:
> grep "//#define DEBUG" * -rl | xargs sed -i "s/\/\/#define DEBUG/#define
> DEBUG/g"
> 2. make -j
> 3. get some warning:
> hw/i2c/pm_smbus.c: In function 'smb_ioport_writeb':
> hw/i2c/pm_smbus.c:142: warning: format '%04x' expects type 'unsigned int',
> but argument 2 has type 'hwaddr'
> hw/i2c/pm_smbus.c:142: warning: format '%02x' expects type 'unsigned int',
> but argument 3 has type 'uint64_t'
> hw/i2c/pm_smbus.c: In function 'smb_ioport_readb':
> hw/i2c/pm_smbus.c:209: warning: format '%04x' expects type 'unsigned int',
> but argument 2 has type 'hwaddr'
> hw/intc/i8259.c: In function 'pic_ioport_read':
> hw/intc/i8259.c:373: warning: format '%02x' expects type 'unsigned int', but
> argument 2 has type 'hwaddr'
> hw/input/pckbd.c: In function 'kbd_write_command':
> hw/input/pckbd.c:232: warning: format '%02x' expects type 'unsigned int', but
> argument 2 has type 'uint64_t'
> hw/input/pckbd.c: In function 'kbd_write_data':
> hw/input/pckbd.c:333: warning: format '%02x' expects type 'unsigned int', but
> argument 2 has type 'uint64_t'
> hw/isa/apm.c: In function 'apm_ioport_writeb':
> hw/isa/apm.c:44: warning: format '%x' expects type 'unsigned int', but
> argument 2 has type 'hwaddr'
> hw/isa/apm.c:44: warning: format '%02x' expects type 'unsigned int', but
> argument 3 has type 'uint64_t'
> hw/isa/apm.c: In function 'apm_ioport_readb':
> hw/isa/apm.c:67: warning: format '%x' expects type 'unsigned int', but
> argument 2 has type 'hwaddr'
> hw/timer/mc146818rtc.c: In function 'cmos_ioport_write':
> hw/timer/mc146818rtc.c:394: warning: format '%02x' expects type 'unsigned
> int', but argument 3 has type 'uint64_t'
> hw/i386/pc.c: In function 'port92_write':
> hw/i386/pc.c:479: warning: format '%02x' expects type 'unsigned int', but
> argument 2 has type 'uint64_t'
>
> Fix them.
>
> Cc: qemu-triv...@nongnu.org
> Signed-off-by: Gonglei
> ---
> BTW,
> I have posted three patches for some module which broke QEMU compling in the
> same way.
> [PATCH v2] scsi-generic: remove superfluous DPRINTF avoid to break compiling
> [PATCH v2] monitor: fix debug print compiling error
> [PATCH] xhci: fix debug print compiling error
>
> And I think this one and above "monitor.." can be included in trivial branch.
> Thanks.
> ---
> hw/i2c/pm_smbus.c | 5 +++--
> hw/i386/pc.c | 2 +-
> hw/input/pckbd.c | 4 ++--
> hw/intc/i8259.c| 2 +-
> hw/isa/apm.c | 5 +++--
> hw/timer/mc146818rtc.c | 2 +-
> 6 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
> index fedb5fb..98eb71f 100644
> --- a/hw/i2c/pm_smbus.c
> +++ b/hw/i2c/pm_smbus.c
> @@ -139,7 +139,8 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr,
> uint64_t val,
> {
> PMSMBus *s = opaque;
>
> -SMBUS_DPRINTF("SMB writeb port=0x%04x val=0x%02x\n", addr, val);
> +SMBUS_DPRINTF("SMB writeb port=0x%04" PRIx64
> + " val=0x%02" PRIx64 "\n", addr, val);
HWADDR_PRI here and below.
Regards,
Peter
> switch(addr) {
> case SMBHSTSTS:
> s->smb_stat = (~(val & 0xff)) & s->smb_stat;
> @@ -206,7 +207,7 @@ static uint64_t smb_ioport_readb(void *opaque, hwaddr
> addr, unsigned width)
> val = 0;
> break;
> }
> -SMBUS_DPRINTF("SMB readb port=0x%04x val=0x%02x\n", addr, val);
> +SMBUS_DPRINTF("SMB readb port=0x%04" PRIx64 " val=0x%02x\n", addr, val);
> return val;
> }
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 8fa8d2f..0b1ab1f 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -476,7 +476,7 @@ static void port92_write(void *opaque, hwaddr addr,
> uint64_t val,
> Port92State *s = opaque;
> int oldval = s->outport;
>
> -DPRINTF("port92: write 0x%02x\n", val);
> +DPRINTF("port92: write 0x%02" PRIx64 "\n", val);
> s->outport = val;
> qemu_set_irq(*s->a20_out, (val >> 1) & 1);
> if ((val & 1) && !(oldval & 1)) {
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index ca1cffc..2ab8c87 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -229,7 +229,7 @@ static void kbd_write_command(void *opaque, hwaddr addr,
> {
> KBDState *s = opaque;
>
> -DPRINTF("kbd: write cmd=0x%02x\n", val);
> +DPRINTF("kbd: write cmd=0x%02" PRIx64 "\n", val);
>
> /* Bits 3-0 of the output port P2 of the keyboard controller may be
> pulsed
> * low for approximately 6 micro seconds. Bits 3-0 of the KBD_CCMD_PULSE
> @@ -330,7 +330,7 @@ static void kbd_write_data(void *opaque, hwaddr addr,
> {
> KBDState *s = opaque;
>
> -DPRINTF("kbd: write data=0x%02x\n", val);
> +DPRINTF("kbd: write data=0x%02" PRIx64 "\n", val);
>
> switch(s->write_cmd) {
> case 0:
> diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
> index a563b82..3a62e33 100644
> --- a/hw/intc/i8259.c
> +++ b/hw/intc/i8259.c
> @
Re: [Qemu-devel] [PATCH v12 0/6] qcow2, raw: add preallocation=full and preallocation=falloc
On Fri, Aug 22, 2014 at 01:25:56PM +0100, Richard W.M. Jones wrote: > > On Mon, Jul 28, 2014 at 04:48:46PM +0800, Hu Tao wrote: > > ping... > > > > All the 6 patches have reviewed-by now. > > > > On Fri, Jul 11, 2014 at 02:09:57PM +0800, Hu Tao wrote: > > > This series adds two preallocation mode to qcow2 and raw: > > > > > > Option preallocation=full preallocates disk space for image by writing > > > zeros to disk, this ensures disk space in any cases. > > > > > > Option preallocation=falloc preallocates disk space by calling > > > posix_fallocate(). This is faster than preallocation=full. > > Sorry if this was discussed before, but why would anyone use > preallocation=full if preallocation=falloc was possible? > > Shouldn't preallocation=full simply use posix_fallocate if it's > available, and fall back to writing zeroes if not? posix_fallocate() will itself fallback to writing zeros to disk if the filesystem can't do the fast allocation method. So I can't see any reason to have separate options for this. Just have a preallocation=full option which - at compile time - chooses between posix_fallocate and write(). This is what we've done in libvirt for many years now and no one has ever asked for more http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/util/virfile.c;h=b6f5e3f2cbbcc6768d764009b238b2349bee3fee;hb=HEAD#l1037 We actually try to use mmap() before going to write() since that is slightly faster. http://log.amitshah.net/2009/03/comparison-of-file-systems-and-speeding-up-applications/ Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [V9fs-developer] QEMU dies on any attempt to load a Linux kernel module when using a 9P rootfs
Hi Dominique, On 08/22/2014 02:27 AM, Dominique Martinet wrote: > Hi, > > Christopher Covington wrote on Thu, Aug 21, 2014 at 03:50:58PM -0400: >> With my 3.15.0+ kernel, qemu-system-x86_64 substituted for qemu-kvm, and the >> path changed from your arguments I get: >> >> 9pnet_virtio: no channels available >> VFS: Cannot open root device "root" or unknown-block(0,0): error -2 >> Please append a correct "root=" boot option; here are the available >> partitions: >> 0b00 1048575 sr0 driver: sr >> Kernel panic - not syncing: VFS: Unable to mount root fs on >> unknown-block(0,0) >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS >> rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org 04/01/2014 > > I'm pretty sure that's the same problem that's been reported a few times > here, could you try again after cherry-picking this commit: > > commit f15844e0777fec936f87a87f97394f83911dacd3 > Author: Dominique Martinet > Date: Wed May 21 10:02:12 2014 +0200 > > 9P: fix return value in v9fs_fid_xattr_set > > v9fs_fid_xattr_set is supposed to return 0 on success. > > This corrects the behaviour introduced in commit > bdd5c28dcb8330b9074404cc92a0b83aae5606a > "9p: fix return value in case in v9fs_fid_xattr_set()" > > (The function returns a negative error on error, as expected) > > Signed-off-by: Dominique Martinet > Signed-off-by: Eric Van Hensbergen > > > It has gotten in in v3.16-rc1, so your 3.15.0+ proably doesn't have it. Thanks for the pointer to this patch. I think I started this kernel half way through the 3.16 merge window. The last non-cherry-picked patch I have is: commit 6d87c225f5d82d29243dc124f1ffcbb0e14ec358 Merge: 338c09a 22001f6 Author: Linus Torvalds Date: Thu Jun 12 23:06:23 2014 -0700 Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/sage/ceph-client So I do have the change you pointed out. Nevertheless, I'll reconfirm with the latest torvalds/master. Thanks, Christopher -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation.
Re: [Qemu-devel] [PATCH v2 0/2] qemu-img: Allow source cache mode specification
Am 22.07.2014 um 22:58 hat Max Reitz geschrieben: > Currently, qemu-img does not allow setting the cache mode for source > images. However, it reads images generally only once, therefore a full > writeback cache unnecessarily clutters the host cache. In case the user > finds this undesirable, there has to be a way of disabling that cache. > This series first introduces such a way for check, compare, convert and > rebase and then (while at it) adds a cache mode switch to amend (which > was missing so far). > > Peter already tried to add a BDRV_O_SEQUENTIAL mode to qemu-img convert. > If we want to add such a mode (which in my opinion would be the logical > consequence of this series), we may want to make it just another cache > mode which can be selected by the user. > > v2: > - Patch 1: >- Remove superfluous space in qemu-img-cmds.hx [Eric] >- Reorder all getopt() optstrings which are touched by this patch > anyway according to the subsequent switch [Eric (implicitly)] > - Patch 2: >- Reorder img_amend()'s optstring [Eric] Thanks, applied all to the block branch. Kevin
