Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+QEMUIOVector *iov)
+{
+nfsclient *client = bs-opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client-context, client-fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs-total_sectors = MAX(bs-total_sectors, sector_num +
nb_sectors);
+client-allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write. -ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs-opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(runtime_opts);
+qemu_opts_absorb_qdict(opts, options, local_err);
+if (error_is_set(local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, filename);
+
+client-context = nfs_init_context();
+
+if (client-context == NULL) {
+error_setg(errp, Failed to init NFS context);
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive, especially
with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, Invalid server in URL);
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client-context, server, path) != 0) {
+error_setg(errp, Failed to mount nfs share: %s,
+nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags O_CREAT) {
+if (nfs_creat(client-context, file, 0600, client-fh) != 0) {
+error_setg(errp, Failed to create file: %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client-context, file, open_flags, client-fh)
!= 0) {
+error_setg(errp, Failed to open file : %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client-context, client-fh, st) != 0) {
+error_setg(errp, Failed to fstat file: %s,
+ nfs_get_error(client-context));
+ret = -EIO;
+goto fail;
+}
+
+bs-total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential for
VMDK description file. But you are right, please add check code and make
sure that we don't read beyond EOF as well.
Thanks for reviewing!
One wish as I think you are the VMDK format maintainer. Can you rework
vmdk_create and possible
other
Re: [Qemu-devel] [PATCH] spapr-pci: remove io ports workaround
On 12/17/2013 06:52 PM, Greg Kurz wrote: On Wed, 11 Dec 2013 18:07:58 +1100 Alexey Kardashevskiy a...@ozlabs.ru wrote: Hm. Nack. This fails: ./qemu-system-ppc64 \ -trace events=qemu_trace_events \ -L qemu-ppc64-bios/ \ -nographic \ -vga none \ -device \ virtio-blk-pci,id=virtioiblk0,drive=drive0,bootindex=20,ioeventfd=on \ -drive \ file=virtimg/fc19_16GB.qcow2,if=none,id=drive0,readonly=off,format=qcow2,media=disk,werror=stop,rerror=stop,discard=on \ -S \ -m 2048 \ -machine pseries \ -enable-kvm command line: BOOT_IMAGE=/vmlinux-3.10.0-rc7-aik-guest+ root=UUID=27cde746-128e-4528-b4de-44a00d807ea0 ro rd.md=0 rd.lvm=0 rd.dm=0 vconsole.font=latarcyrheb-sun16 vconsole.keymap=us rd.luks=0 console=hvc0 debug memory layout at init: memory_limit : (16 MB aligned) alloc_bottom : 0340 alloc_top: 3000 alloc_top_hi : 8000 rmo_top : 3000 ram_top : 8000 instantiating rtas at 0x2fff... done boot cpu hw idx 0 copying OF device tree... Building dt strings... Building dt structure... Device tree strings 0x0341 - 0x03410817 Device tree struct 0x0342 - 0x0343 [Switching to Thread 0x3fff8ca4eee0 (LWP 10370)] Breakpoint 8, unassigned_mem_accepts (opaque=0x0, addr=0x1008052, size=0x1, is_write=0x1) at /home/alexey/p/qemu/memory.c:892 892 return false; Missing separate debuginfos, use: debuginfo-install glusterfs-api-3.4.0-8.fc19.ppc64 glusterfs-libs-3.4.0-8.fc19.ppc64 gnutls-3.1.16-1.fc19.ppc64 keyutils-libs-1.5.6-1.fc19.ppc64 libgcc-4.8.2-1.fc19.ppc64 libgcrypt-1.5.3-2.fc19.ppc64 libibverbs-1.1.7-3.fc19.ppc64 libiscsi-1.7.0-5.fc19.ppc64 libpng-1.5.13-2.fc19.ppc64 librdmacm-1.0.17-1.fc19.ppc64 libusbx-1.0.16-3.fc19.ppc64 systemd-libs-204-17.fc19.ppc64 usbredir-0.6-2.fc19.ppc64 (gdb) bt #0 unassigned_mem_accepts (opaque=0x0, addr=0x1008052, size=0x1, is_write=0x1) at /home/alexey/p/qemu/memory.c:892 #1 0x103cb238 in memory_region_access_valid (mr=0x10b76ec8 io_mem_unassigned, addr=0x1008052, size=0x1, is_write=0x1) at /home/alexey/p/qemu/memory.c:928 #2 0x103cb4bc in memory_region_dispatch_write (mr=0x10b76ec8 io_mem_unassigned, addr=0x1008052, data=0x80, size=0x1) at /home/alexey/p/qemu/memory.c:976 #3 0x103cebec in io_mem_write (mr=0x10b76ec8 io_mem_unassigned, addr=0x1008052, val=0x80, size=0x1) at /home/alexey/p/qemu/memory.c:1748 #4 0x10329b54 in address_space_rw (as=0x10b80cf0 address_space_memory, addr=0x1008052, buf=0x3fff8ad9e190 \200, len=0x1, is_write=0x1) at /home/alexey/p/qemu/exec.c:1941 #5 0x1032a000 in cpu_physical_memory_rw (addr=0x1008052, buf=0x3fff8ad9e190 \200, len=0x1, is_write=0x1) at /home/alexey/p/qemu/exec.c:2010 #6 0x103231c4 in cpu_physical_memory_write (addr=0x1008052, buf=0x3fff8ad9e190, len=0x1) at /home/alexey/p/qemu/include/exec/cpu-common.h:68 #7 0x1032b5b0 in stb_phys (addr=0x1008052, val=0x80) at /home/alexey/p/qemu/exec.c:2506 #8 0x103a0c10 in h_logical_store (cpu=0x3fff8ada0010, spapr=0x100118ca410, opcode=0x40, args=0x3fff8bfc0030) at /home/alexey/p/qemu/hw/ppc/spapr_hcall.c:564 #9 0x103a140c in spapr_hypercall (cpu=0x3fff8ada0010, opcode=0x40, args=0x3fff8bfc0030) at /home/alexey/p/qemu/hw/ppc/spapr_hcall.c:737 #10 0x1041b080 in kvm_arch_handle_exit (cs=0x3fff8ada0010, run=0x3fff8bfc) at /home/alexey/p/qemu/target-ppc/kvm.c:1223 #11 0x103c5cbc in kvm_cpu_exec (cpu=0x3fff8ada0010) at /home/alexey/p/qemu/kvm-all.c:1726 #12 0x1031902c in qemu_kvm_cpu_thread_fn (arg=0x3fff8ada0010) at /home/alexey/p/qemu/cpus.c:874 #13 0x0080bcd0c29c in start_thread (arg=0x3fff8ad9eee0) at pthread_create.c:310 #14 0x0080bcb1ddb0 in .__clone () at ../sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S:111 Without your patch, unassigned_mem_accepts() is not called so it tells us that IO stopped working after your patch. Alex, Can you elaborate ? Does the kernel fail to boot ? Unfortunately I do not have good 3D imagination, do not understand this memory region well enough to give advice and cannot tell quickly what exactly is wrong here :) Heh no problem. Let me share my findings then ! :) First, if I pass kernel/initrd/cmdline directly to the qemu command line, I don't get this weird access to 0x1008052 at all (with or without my patch). Second, if I run the very same test WITHOUT my patch and set a breakpoint in unassigned_io_write(), it pops with the same 0x1008052 address. It does not stop there when I try it WITHOUT my patch, that's my entire point. I retried right now, with the upstream QEMU + KVM breakpoint stubs patch. With your patch - it still 100% reproducible - gdb stops at
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+QEMUIOVector *iov)
+{
+nfsclient *client = bs-opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client-context, client-fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs-total_sectors = MAX(bs-total_sectors, sector_num +
nb_sectors);
+client-allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write. -ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs-opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(runtime_opts);
+qemu_opts_absorb_qdict(opts, options, local_err);
+if (error_is_set(local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, filename);
+
+client-context = nfs_init_context();
+
+if (client-context == NULL) {
+error_setg(errp, Failed to init NFS context);
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive, especially with
strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, Invalid server in URL);
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client-context, server, path) != 0) {
+error_setg(errp, Failed to mount nfs share: %s,
+nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags O_CREAT) {
+if (nfs_creat(client-context, file, 0600, client-fh) != 0) {
+error_setg(errp, Failed to create file: %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client-context, file, open_flags, client-fh)
!= 0) {
+error_setg(errp, Failed to open file : %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client-context, client-fh, st) != 0) {
+error_setg(errp, Failed to fstat file: %s,
+ nfs_get_error(client-context));
+ret = -EIO;
+goto fail;
+}
+
+bs-total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential for VMDK
description file. But you are right, please add check code and make sure that
we don't read beyond EOF as well.
Actually it would like to keep bs-total_sectors = st.st_size /
BDRV_SECTOR_SIZE; for
Re: [Qemu-devel] [PATCH] block: add native support for NFS
于2013年12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+QEMUIOVector *iov)
+{
+nfsclient *client = bs-opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client-context, client-fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs-total_sectors = MAX(bs-total_sectors, sector_num +
nb_sectors);
+client-allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs-opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(runtime_opts);
+qemu_opts_absorb_qdict(opts, options, local_err);
+if (error_is_set(local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, filename);
+
+client-context = nfs_init_context();
+
+if (client-context == NULL) {
+error_setg(errp, Failed to init NFS context);
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, Invalid server in URL);
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client-context, server, path) != 0) {
+error_setg(errp, Failed to mount nfs share: %s,
+nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags O_CREAT) {
+if (nfs_creat(client-context, file, 0600, client-fh)
!= 0) {
+error_setg(errp, Failed to create file: %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client-context, file, open_flags, client-fh)
!= 0) {
+error_setg(errp, Failed to open file : %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client-context, client-fh, st) != 0) {
+error_setg(errp, Failed to fstat file: %s,
+ nfs_get_error(client-context));
+ret = -EIO;
+goto fail;
+}
+
+bs-total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK description file. But you are right, please add check code
and make sure that we don't read beyond EOF as well.
Actually it would like to keep bs-total_sectors
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 17.12.2013 09:51, Fam Zheng wrote:
于2013年 12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+QEMUIOVector *iov)
+{
+nfsclient *client = bs-opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client-context, client-fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs-total_sectors = MAX(bs-total_sectors, sector_num +
nb_sectors);
+client-allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs-opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(runtime_opts);
+qemu_opts_absorb_qdict(opts, options, local_err);
+if (error_is_set(local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, filename);
+
+client-context = nfs_init_context();
+
+if (client-context == NULL) {
+error_setg(errp, Failed to init NFS context);
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, Invalid server in URL);
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client-context, server, path) != 0) {
+error_setg(errp, Failed to mount nfs share: %s,
+nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags O_CREAT) {
+if (nfs_creat(client-context, file, 0600, client-fh)
!= 0) {
+error_setg(errp, Failed to create file: %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client-context, file, open_flags, client-fh)
!= 0) {
+error_setg(errp, Failed to open file : %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client-context, client-fh, st) != 0) {
+error_setg(errp, Failed to fstat file: %s,
+ nfs_get_error(client-context));
+ret = -EIO;
+goto fail;
+}
+
+bs-total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK description file. But you are right, please add check code
and make sure that we don't read beyond EOF as well.
Actually it would like to
[Qemu-devel] qemu 1.7.0 does not build on NetBSD (patch)
On NetBSD int8_t and friends are preprocessor macros expanding to __int8_t (which is a typedef provided by machine dependent headers). include/exec/softmmu_template.h tries to safe 3 lines of code by using preprosseor concatenation, which only will work if int8_t is not a define, or defined to int8_t. The attached patch fixes this. Martin Do not rely on int8_t (and friends) not being preprocessor symbols (or symbols expanding to themselves). On NetBSD (for example) the glue(u, SDATA_TYPE) results in u__int8_t, which is undefined. There is no way to stop cpp expanding inner macros, so just add the few lines explicitly and get rid of the magic. --- include/exec/softmmu_template.h.orig2013-11-27 23:15:55.0 +0100 +++ include/exec/softmmu_template.h 2013-12-16 20:57:50.0 +0100 @@ -30,23 +30,26 @@ #define SUFFIX q #define LSUFFIX q #define SDATA_TYPE int64_t +#define DATA_TYPE uint64_t #elif DATA_SIZE == 4 #define SUFFIX l #define LSUFFIX l #define SDATA_TYPE int32_t +#define DATA_TYPE uint32_t #elif DATA_SIZE == 2 #define SUFFIX w #define LSUFFIX uw #define SDATA_TYPE int16_t +#define DATA_TYPE uint16_t #elif DATA_SIZE == 1 #define SUFFIX b #define LSUFFIX ub #define SDATA_TYPE int8_t +#define DATA_TYPE uint8_t #else #error unsupported data size #endif -#define DATA_TYPE glue(u, SDATA_TYPE) /* For the benefit of TCG generated code, we want to avoid the complication of ABI-specific return type promotion and always return a value extended
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 2013年12月17日 16:55, Peter Lieven wrote:
On 17.12.2013 09:51, Fam Zheng wrote:
于2013年 12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+QEMUIOVector *iov)
+{
+nfsclient *client = bs-opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client-context, client-fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs-total_sectors = MAX(bs-total_sectors, sector_num +
nb_sectors);
+client-allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs-opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(runtime_opts);
+qemu_opts_absorb_qdict(opts, options, local_err);
+if (error_is_set(local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, filename);
+
+client-context = nfs_init_context();
+
+if (client-context == NULL) {
+error_setg(errp, Failed to init NFS context);
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, Invalid server in URL);
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client-context, server, path) != 0) {
+error_setg(errp, Failed to mount nfs share: %s,
+nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags O_CREAT) {
+if (nfs_creat(client-context, file, 0600, client-fh)
!= 0) {
+error_setg(errp, Failed to create file: %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client-context, file, open_flags,
client-fh)
!= 0) {
+error_setg(errp, Failed to open file : %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client-context, client-fh, st) != 0) {
+error_setg(errp, Failed to fstat file: %s,
+ nfs_get_error(client-context));
+ret = -EIO;
+goto fail;
+}
+
+bs-total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK description file. But you are right, please add check code
and make sure that we don't read beyond
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 17.12.2013 10:01, Fam Zheng wrote:
On 2013年12月17日 16:55, Peter Lieven wrote:
On 17.12.2013 09:51, Fam Zheng wrote:
于2013年 12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+ QEMUIOVector *iov)
+{
+nfsclient *client = bs-opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client-context, client-fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs-total_sectors = MAX(bs-total_sectors, sector_num +
nb_sectors);
+client-allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async call.
On open I fstat anyway and store that value. For qemu-img info this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs-opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(runtime_opts);
+qemu_opts_absorb_qdict(opts, options, local_err);
+if (error_is_set(local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, filename);
+
+client-context = nfs_init_context();
+
+if (client-context == NULL) {
+error_setg(errp, Failed to init NFS context);
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before accessing
filename[6].
Good point. I will check for this, but in fact I think it can't happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, Invalid server in URL);
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client-context, server, path) != 0) {
+error_setg(errp, Failed to mount nfs share: %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags O_CREAT) {
+if (nfs_creat(client-context, file, 0600, client-fh)
!= 0) {
+error_setg(errp, Failed to create file: %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client-context, file, open_flags,
client-fh)
!= 0) {
+error_setg(errp, Failed to open file : %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client-context, client-fh, st) != 0) {
+error_setg(errp, Failed to fstat file: %s,
+ nfs_get_error(client-context));
+ret = -EIO;
+goto fail;
+}
+
+bs-total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK description file. But you are right, please add check code
and make sure that we don't read beyond EOF as well.
Actually it would like
Re: [Qemu-devel] [PATCH V4] char: restore read callback on a reattached (hotplug) chardev
On 16/12/2013 22:32, Amit Shah wrote:
On (Sun) 15 Dec 2013 [12:26:37], Gal Hammer wrote:
Fix a bug that was introduced in commit 386a5a1e. A removal of a device
set the chr handlers to NULL. However when the device is plugged back,
its read callback is not restored so data can't be transferred from the
host to the guest (e.g. via the virtio-serial port).
https://bugzilla.redhat.com/show_bug.cgi?id=1027181
Signed-off-by: Gal Hammer gham...@redhat.com
---
qemu-char.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
V4: - Same as V3, but this time done right.
V3: - fix a typo in comment.
- move the revision history after the signed-off-by tag.
V2: - do not call chr_update_read_handler on device removal.
- add asserts to verify chr_update_read_handler is not called
with an assigned fd_in_tag to prevent fd leaks.
- update fd and udp backends' chr_update_read_handler function
so it won't remove fd_in to prevent a double release.
Looks like you missed the pty backend. Can't blame you -- this
chardev stuff is really messy. pty is doing is own handling + polling
+ HUP detection, which really is equally applicable to tcp and udp as
well.
As far as I could tell the pty backend doesn't suffer from this issue.
That's why I didn't change anything there.
More below.
diff --git a/qemu-char.c b/qemu-char.c
index e00f84c..69649f0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -213,7 +213,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
s-chr_read = fd_read;
s-chr_event = fd_event;
s-handler_opaque = opaque;
-if (s-chr_update_read_handler)
+if (fe_open s-chr_update_read_handler)
s-chr_update_read_handler(s);
if (!s-explicit_fe_open) {
@@ -870,6 +870,7 @@ static void fd_chr_update_read_handler(CharDriverState *chr)
{
FDCharDriver *s = chr-opaque;
+assert(!chr-fd_in_tag);
remove_fd_in_watch(chr);
if (s-fd_in) {
chr-fd_in_tag = io_add_watch_poll(s-fd_in, fd_chr_read_poll,
@@ -2228,7 +2229,7 @@ static void udp_chr_update_read_handler(CharDriverState
*chr)
{
NetCharDriver *s = chr-opaque;
-remove_fd_in_watch(chr);
+assert(!chr-fd_in_tag);
You're removing remove_fd_in_watch() here but not in
fd_chr_update_read_handler() above. Any particular reason?
I missed it while commit -p the changes. Fixed.
Also, this assert would be a risky thing in pty, since it has its own
HUP detection method. A common way to deal with all this would be
nice, but chardevs are hacks upon hacks already, and this fixes a
regression, so I'm fine with applying the patch in this form now.
Please update for pty.
Gerd, can you please look at this and pick it up? I'm going to be
away till Jan.
Thanks,
Amit
Gal.
[Qemu-devel] [Bug 1261268] Re: save guest running time is more than 450s with AVX running.
after re-check , the first bad commit is: commit 3e469dbfe413c25d48321c3a19ddfae0727dc6e5 Author: Andrea Arcangeli aarca...@redhat.com Date: Thu Jul 25 12:11:15 2013 +0200 exec: always use MADV_DONTFORK MADV_DONTFORK prevents fork to fail with -ENOMEM if the default overcommit heuristics decides there's too much anonymous virtual memory allocated. If the KVM secondary MMU is synchronized with MMU notifiers or not, doesn't make a difference in that regard. Secondly it's always more efficient to avoid copying the guest physical address space in the fork child (so we avoid to mark all the guest memory readonly in the parent and so we skip the establishment and teardown of lots of pagetables in the child). In the common case we can ignore the error if MADV_DONTFORK is not available. Leave a second invocation that errors out in the KVM path if MMU notifiers are missing and KVM is enabled, to abort in such case. Signed-off-by: Andrea Arcangeli aarca...@redhat.com Tested-By: Benoit Canet ben...@irqsave.net Acked-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Gleb Natapov g...@redhat.com -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1261268 Title: save guest running time is more than 450s with AVX running. Status in QEMU: New Bug description: Environment: Host OS (ia32/ia32e/IA64):ia32e Guest OS (ia32/ia32e/IA64):ia32e Guest OS Type (Linux/Windows):linux kvm.git Commit:d6d63b51fe3bfea0cf596993afa480b0b3b02c32 qemu.git Commit:8f84271da83c0e9f92aa7c1c2d0d3875bf0a5cb8 Host Kernel Version:3.13.0-rc1 Hardware:Romley_EP, Ivytown_EP Bug detailed description: -- when guest running avx , then do save /restore, save guest running time is too lomg Note: 1.when save guest (migrate exec:dd of=test.img)sometimes , the file of test.img is 29G, running time of save guest is about 900s 2. this should be a qemu bug: kvm + qemu = result d6d63b51 + 8f84271d = bad d6d63b51 + b5d54bd4 = good Reproduce steps: 1.qemu-system-x86_64 -enable-kvm -m 1024 -smp 6 -net nic,macaddr=00:12:34:43:14:78 -net tap,script=/etc/kvm/qemu-ifup rhel6u4.qcow 2. scp /usr/tet/XVS/tsets/control_panel/tools/bin/avx.tar.gz $guest_IP:/root 3. tar -zxf avx.tar.gz 4. cd /avx 5. sh chk_avx.sh /dev/null 6. ctrl-alt-2 7. migrate exec:dd of=test.img Current result: running time of save guest is more than 450s Expected result: running time of save guest is less than 450s Basic root-causing log: -- To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1261268/+subscriptions
[Qemu-devel] [Bug 1261268] Re: save guest running time is more than 450s with AVX running.
after re-check , the first bad commit is: commit 3e469dbfe413c25d48321c3a19ddfae0727dc6e5 Author: Andrea Arcangeli aarca...@redhat.com Date: Thu Jul 25 12:11:15 2013 +0200 exec: always use MADV_DONTFORK MADV_DONTFORK prevents fork to fail with -ENOMEM if the default overcommit heuristics decides there's too much anonymous virtual memory allocated. If the KVM secondary MMU is synchronized with MMU notifiers or not, doesn't make a difference in that regard. Secondly it's always more efficient to avoid copying the guest physical address space in the fork child (so we avoid to mark all the guest memory readonly in the parent and so we skip the establishment and teardown of lots of pagetables in the child). In the common case we can ignore the error if MADV_DONTFORK is not available. Leave a second invocation that errors out in the KVM path if MMU notifiers are missing and KVM is enabled, to abort in such case. Signed-off-by: Andrea Arcangeli aarca...@redhat.com Tested-By: Benoit Canet ben...@irqsave.net Acked-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Gleb Natapov g...@redhat.com -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1261268 Title: save guest running time is more than 450s with AVX running. Status in QEMU: New Bug description: Environment: Host OS (ia32/ia32e/IA64):ia32e Guest OS (ia32/ia32e/IA64):ia32e Guest OS Type (Linux/Windows):linux kvm.git Commit:d6d63b51fe3bfea0cf596993afa480b0b3b02c32 qemu.git Commit:8f84271da83c0e9f92aa7c1c2d0d3875bf0a5cb8 Host Kernel Version:3.13.0-rc1 Hardware:Romley_EP, Ivytown_EP Bug detailed description: -- when guest running avx , then do save /restore, save guest running time is too lomg Note: 1.when save guest (migrate exec:dd of=test.img)sometimes , the file of test.img is 29G, running time of save guest is about 900s 2. this should be a qemu bug: kvm + qemu = result d6d63b51 + 8f84271d = bad d6d63b51 + b5d54bd4 = good Reproduce steps: 1.qemu-system-x86_64 -enable-kvm -m 1024 -smp 6 -net nic,macaddr=00:12:34:43:14:78 -net tap,script=/etc/kvm/qemu-ifup rhel6u4.qcow 2. scp /usr/tet/XVS/tsets/control_panel/tools/bin/avx.tar.gz $guest_IP:/root 3. tar -zxf avx.tar.gz 4. cd /avx 5. sh chk_avx.sh /dev/null 6. ctrl-alt-2 7. migrate exec:dd of=test.img Current result: running time of save guest is more than 450s Expected result: running time of save guest is less than 450s Basic root-causing log: -- To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1261268/+subscriptions
[Qemu-devel] [Bug 1261268] Re: save guest running time is more than 450s with AVX running.
after re-check , the first bad commit is: commit 3e469dbfe413c25d48321c3a19ddfae0727dc6e5 Author: Andrea Arcangeli aarca...@redhat.com Date: Thu Jul 25 12:11:15 2013 +0200 exec: always use MADV_DONTFORK MADV_DONTFORK prevents fork to fail with -ENOMEM if the default overcommit heuristics decides there's too much anonymous virtual memory allocated. If the KVM secondary MMU is synchronized with MMU notifiers or not, doesn't make a difference in that regard. Secondly it's always more efficient to avoid copying the guest physical address space in the fork child (so we avoid to mark all the guest memory readonly in the parent and so we skip the establishment and teardown of lots of pagetables in the child). In the common case we can ignore the error if MADV_DONTFORK is not available. Leave a second invocation that errors out in the KVM path if MMU notifiers are missing and KVM is enabled, to abort in such case. Signed-off-by: Andrea Arcangeli aarca...@redhat.com Tested-By: Benoit Canet ben...@irqsave.net Acked-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Gleb Natapov g...@redhat.com -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1261268 Title: save guest running time is more than 450s with AVX running. Status in QEMU: New Bug description: Environment: Host OS (ia32/ia32e/IA64):ia32e Guest OS (ia32/ia32e/IA64):ia32e Guest OS Type (Linux/Windows):linux kvm.git Commit:d6d63b51fe3bfea0cf596993afa480b0b3b02c32 qemu.git Commit:8f84271da83c0e9f92aa7c1c2d0d3875bf0a5cb8 Host Kernel Version:3.13.0-rc1 Hardware:Romley_EP, Ivytown_EP Bug detailed description: -- when guest running avx , then do save /restore, save guest running time is too lomg Note: 1.when save guest (migrate exec:dd of=test.img)sometimes , the file of test.img is 29G, running time of save guest is about 900s 2. this should be a qemu bug: kvm + qemu = result d6d63b51 + 8f84271d = bad d6d63b51 + b5d54bd4 = good Reproduce steps: 1.qemu-system-x86_64 -enable-kvm -m 1024 -smp 6 -net nic,macaddr=00:12:34:43:14:78 -net tap,script=/etc/kvm/qemu-ifup rhel6u4.qcow 2. scp /usr/tet/XVS/tsets/control_panel/tools/bin/avx.tar.gz $guest_IP:/root 3. tar -zxf avx.tar.gz 4. cd /avx 5. sh chk_avx.sh /dev/null 6. ctrl-alt-2 7. migrate exec:dd of=test.img Current result: running time of save guest is more than 450s Expected result: running time of save guest is less than 450s Basic root-causing log: -- To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1261268/+subscriptions
[Qemu-devel] [Bug 1261268] Re: save guest running time is more than 450s with AVX running.
after re-check , the first bad commit is: commit 3e469dbfe413c25d48321c3a19ddfae0727dc6e5 Author: Andrea Arcangeli aarca...@redhat.com Date: Thu Jul 25 12:11:15 2013 +0200 exec: always use MADV_DONTFORK MADV_DONTFORK prevents fork to fail with -ENOMEM if the default overcommit heuristics decides there's too much anonymous virtual memory allocated. If the KVM secondary MMU is synchronized with MMU notifiers or not, doesn't make a difference in that regard. Secondly it's always more efficient to avoid copying the guest physical address space in the fork child (so we avoid to mark all the guest memory readonly in the parent and so we skip the establishment and teardown of lots of pagetables in the child). In the common case we can ignore the error if MADV_DONTFORK is not available. Leave a second invocation that errors out in the KVM path if MMU notifiers are missing and KVM is enabled, to abort in such case. Signed-off-by: Andrea Arcangeli aarca...@redhat.com Tested-By: Benoit Canet ben...@irqsave.net Acked-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Gleb Natapov g...@redhat.com -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1261268 Title: save guest running time is more than 450s with AVX running. Status in QEMU: New Bug description: Environment: Host OS (ia32/ia32e/IA64):ia32e Guest OS (ia32/ia32e/IA64):ia32e Guest OS Type (Linux/Windows):linux kvm.git Commit:d6d63b51fe3bfea0cf596993afa480b0b3b02c32 qemu.git Commit:8f84271da83c0e9f92aa7c1c2d0d3875bf0a5cb8 Host Kernel Version:3.13.0-rc1 Hardware:Romley_EP, Ivytown_EP Bug detailed description: -- when guest running avx , then do save /restore, save guest running time is too lomg Note: 1.when save guest (migrate exec:dd of=test.img)sometimes , the file of test.img is 29G, running time of save guest is about 900s 2. this should be a qemu bug: kvm + qemu = result d6d63b51 + 8f84271d = bad d6d63b51 + b5d54bd4 = good Reproduce steps: 1.qemu-system-x86_64 -enable-kvm -m 1024 -smp 6 -net nic,macaddr=00:12:34:43:14:78 -net tap,script=/etc/kvm/qemu-ifup rhel6u4.qcow 2. scp /usr/tet/XVS/tsets/control_panel/tools/bin/avx.tar.gz $guest_IP:/root 3. tar -zxf avx.tar.gz 4. cd /avx 5. sh chk_avx.sh /dev/null 6. ctrl-alt-2 7. migrate exec:dd of=test.img Current result: running time of save guest is more than 450s Expected result: running time of save guest is less than 450s Basic root-causing log: -- To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1261268/+subscriptions
[Qemu-devel] [Bug 1261268] Re: save guest running time is more than 450s with AVX running.
after re-check , the first bad commit is: commit 3e469dbfe413c25d48321c3a19ddfae0727dc6e5 Author: Andrea Arcangeli aarca...@redhat.com Date: Thu Jul 25 12:11:15 2013 +0200 exec: always use MADV_DONTFORK MADV_DONTFORK prevents fork to fail with -ENOMEM if the default overcommit heuristics decides there's too much anonymous virtual memory allocated. If the KVM secondary MMU is synchronized with MMU notifiers or not, doesn't make a difference in that regard. Secondly it's always more efficient to avoid copying the guest physical address space in the fork child (so we avoid to mark all the guest memory readonly in the parent and so we skip the establishment and teardown of lots of pagetables in the child). In the common case we can ignore the error if MADV_DONTFORK is not available. Leave a second invocation that errors out in the KVM path if MMU notifiers are missing and KVM is enabled, to abort in such case. Signed-off-by: Andrea Arcangeli aarca...@redhat.com Tested-By: Benoit Canet ben...@irqsave.net Acked-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Gleb Natapov g...@redhat.com -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1261268 Title: save guest running time is more than 450s with AVX running. Status in QEMU: New Bug description: Environment: Host OS (ia32/ia32e/IA64):ia32e Guest OS (ia32/ia32e/IA64):ia32e Guest OS Type (Linux/Windows):linux kvm.git Commit:d6d63b51fe3bfea0cf596993afa480b0b3b02c32 qemu.git Commit:8f84271da83c0e9f92aa7c1c2d0d3875bf0a5cb8 Host Kernel Version:3.13.0-rc1 Hardware:Romley_EP, Ivytown_EP Bug detailed description: -- when guest running avx , then do save /restore, save guest running time is too lomg Note: 1.when save guest (migrate exec:dd of=test.img)sometimes , the file of test.img is 29G, running time of save guest is about 900s 2. this should be a qemu bug: kvm + qemu = result d6d63b51 + 8f84271d = bad d6d63b51 + b5d54bd4 = good Reproduce steps: 1.qemu-system-x86_64 -enable-kvm -m 1024 -smp 6 -net nic,macaddr=00:12:34:43:14:78 -net tap,script=/etc/kvm/qemu-ifup rhel6u4.qcow 2. scp /usr/tet/XVS/tsets/control_panel/tools/bin/avx.tar.gz $guest_IP:/root 3. tar -zxf avx.tar.gz 4. cd /avx 5. sh chk_avx.sh /dev/null 6. ctrl-alt-2 7. migrate exec:dd of=test.img Current result: running time of save guest is more than 450s Expected result: running time of save guest is less than 450s Basic root-causing log: -- To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1261268/+subscriptions
[Qemu-devel] [PATCHv2] block: add native support for NFS
This patch adds native support for accessing images on NFS shares without
the requirement to actually mount the entire NFS share on the host.
NFS Images can simply be specified by an url of the form:
nfs://host/export/filename
For example:
qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2
You need libnfs from Ronnie Sahlberg available at:
git://github.com/sahlberg/libnfs.git
for this to work.
During configure it is automatically probed for libnfs and support
is enabled on-the-fly. You can forbid or enforce libnfs support
with --disable-libnfs or --enable-libnfs respectively.
Due to NFS restrictions you might need to execute your binaries
as root, allow them to open priviledged ports (1024) or specify
insecure option on the NFS server.
Signed-off-by: Peter Lieven p...@kamp.de
---
v1-v2:
- fixed block/Makefile.objs [Ronnie]
- do not always register a read handler [Ronnie]
- add support for reading beyond EOF [Fam]
- fixed struct and paramter naming [Fam]
- fixed overlong lines and whitespace errors [Fam]
- return return status from libnfs whereever possible [Fam]
- added comment why we set allocated_file_size to -ENOTSUP after write [Fam]
- avoid segfault when parsing filname [Fam]
- remove unused close_bh from NFSClient [Fam]
- avoid dividing and mutliplying total_size by BDRV_SECTOR_SIZE in
nfs_file_create [Fam]
MAINTAINERS |5 +
block/Makefile.objs |1 +
block/nfs.c | 419 +++
configure | 38 +
4 files changed, 463 insertions(+)
create mode 100644 block/nfs.c
diff --git a/MAINTAINERS b/MAINTAINERS
index c19133f..f53d184 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -899,6 +899,11 @@ M: Peter Lieven p...@kamp.de
S: Supported
F: block/iscsi.c
+NFS
+M: Peter Lieven p...@kamp.de
+S: Maintained
+F: block/nfs.c
+
SSH
M: Richard W.M. Jones rjo...@redhat.com
S: Supported
diff --git a/block/Makefile.objs b/block/Makefile.objs
index f43ecbc..aa8eaf9 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -12,6 +12,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
ifeq ($(CONFIG_POSIX),y)
block-obj-y += nbd.o sheepdog.o
block-obj-$(CONFIG_LIBISCSI) += iscsi.o
+block-obj-$(CONFIG_LIBNFS) += nfs.o
block-obj-$(CONFIG_CURL) += curl.o
block-obj-$(CONFIG_RBD) += rbd.o
block-obj-$(CONFIG_GLUSTERFS) += gluster.o
diff --git a/block/nfs.c b/block/nfs.c
new file mode 100644
index 000..006b8cc
--- /dev/null
+++ b/block/nfs.c
@@ -0,0 +1,419 @@
+/*
+ * QEMU Block driver for native access to files on NFS shares
+ *
+ * Copyright (c) 2013 Peter Lieven p...@kamp.de
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include config-host.h
+
+#include poll.h
+#include qemu-common.h
+#include qemu/config-file.h
+#include qemu/error-report.h
+#include block/block_int.h
+#include trace.h
+#include qemu/iov.h
+#include sysemu/sysemu.h
+
+#include nfsc/libnfs-zdr.h
+#include nfsc/libnfs.h
+#include nfsc/libnfs-raw.h
+#include nfsc/libnfs-raw-mount.h
+
+typedef struct nfsclient {
+struct nfs_context *context;
+struct nfsfh *fh;
+int events;
+bool has_zero_init;
+int64_t allocated_file_size;
+} NFSClient;
+
+typedef struct NFSTask {
+int status;
+int complete;
+QEMUIOVector *iov;
+Coroutine *co;
+QEMUBH *bh;
+} NFSTask;
+
+static void nfs_process_read(void *arg);
+static void nfs_process_write(void *arg);
+
+static void nfs_set_events(NFSClient *client)
+{
+int ev = nfs_which_events(client-context);
+if (ev != client-events) {
+qemu_aio_set_fd_handler(nfs_get_fd(client-context),
+ (ev POLLIN) ? nfs_process_read : NULL,
+ (ev POLLOUT) ? nfs_process_write : NULL,
+ client);
+
+}
+client-events = ev;
+}
+
+static void nfs_process_read(void *arg)
+{
+NFSClient *client = arg;
+nfs_service(client-context, POLLIN);
+
[Qemu-devel] [PATCH] qom: fix cast results caching
A single cast cache is used for both an object casting and a class
casting. In case of interface presence a class cast result may be not
the same pointer as opposite to an object casting. So do not cache cast
results for an object casting in a presence of interfaces.
Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
---
qom/object.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qom/object.c b/qom/object.c
index fc19cf6..f7384de 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -473,7 +473,7 @@ Object *object_dynamic_cast_assert(Object *obj, const char
*typename,
assert(obj == inst);
-if (obj obj == inst) {
+if (obj obj == inst !obj-class-interfaces) {
for (i = 1; i OBJECT_CLASS_CAST_CACHE; i++) {
obj-class-cast_cache[i - 1] = obj-class-cast_cache[i];
}
--
1.7.9.5
[Qemu-devel] [PATCH] qom: fix cast results caching
A single cast cache is used for both an object casting and a class
casting. In case of interface presence a class cast result may be not
the same pointer as opposite to an object casting. So do not cache cast
results for an object casting in a presence of interfaces.
Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
---
qom/object.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qom/object.c b/qom/object.c
index fc19cf6..f7384de 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -473,7 +473,7 @@ Object *object_dynamic_cast_assert(Object *obj, const char
*typename,
assert(obj == inst);
-if (obj obj == inst) {
+if (obj obj == inst !obj-class-interfaces) {
for (i = 1; i OBJECT_CLASS_CAST_CACHE; i++) {
obj-class-cast_cache[i - 1] = obj-class-cast_cache[i];
}
--
1.7.9.5
Re: [Qemu-devel] [PATCH] qom: fix cast results caching
On Tue, Dec 17, 2013 at 7:20 PM, Sergey Fedorov s.fedo...@samsung.com wrote:
A single cast cache is used for both an object casting and a class
casting. In case of interface presence a class cast result may be not
the same pointer as opposite to an object casting. So do not cache cast
results for an object casting in a presence of interfaces.
I think this is fixed by my cast cache splitter patch which is
currently enqueued.
http://patchwork.ozlabs.org/patch/294766/
Regards,
Peter
Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
---
qom/object.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qom/object.c b/qom/object.c
index fc19cf6..f7384de 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -473,7 +473,7 @@ Object *object_dynamic_cast_assert(Object *obj, const
char *typename,
assert(obj == inst);
-if (obj obj == inst) {
+if (obj obj == inst !obj-class-interfaces) {
for (i = 1; i OBJECT_CLASS_CAST_CACHE; i++) {
obj-class-cast_cache[i - 1] = obj-class-cast_cache[i];
}
--
1.7.9.5
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 2013年12月17日 17:07, Peter Lieven wrote:
On 17.12.2013 10:01, Fam Zheng wrote:
On 2013年12月17日 16:55, Peter Lieven wrote:
On 17.12.2013 09:51, Fam Zheng wrote:
于2013年 12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+ QEMUIOVector *iov)
+{
+nfsclient *client = bs-opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors *
BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client-context, client-fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs-total_sectors = MAX(bs-total_sectors, sector_num +
nb_sectors);
+client-allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async
call.
On open I fstat anyway and store that value. For qemu-img info
this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs-opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(runtime_opts);
+qemu_opts_absorb_qdict(opts, options, local_err);
+if (error_is_set(local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, filename);
+
+client-context = nfs_init_context();
+
+if (client-context == NULL) {
+error_setg(errp, Failed to init NFS context);
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before
accessing
filename[6].
Good point. I will check for this, but in fact I think it can't
happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, Invalid server in URL);
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client-context, server, path) != 0) {
+error_setg(errp, Failed to mount nfs share: %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags O_CREAT) {
+if (nfs_creat(client-context, file, 0600, client-fh)
!= 0) {
+error_setg(errp, Failed to create file: %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client-context, file, open_flags,
client-fh)
!= 0) {
+error_setg(errp, Failed to open file : %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client-context, client-fh, st) != 0) {
+error_setg(errp, Failed to fstat file: %s,
+ nfs_get_error(client-context));
+ret = -EIO;
+goto fail;
+}
+
+bs-total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK description file. But you are right, please add check code
and make sure that we don't read
Re: [Qemu-devel] [PATCH] qom: fix cast results caching
On 12/17/2013 01:40 PM, Peter Crosthwaite wrote:
On Tue, Dec 17, 2013 at 7:20 PM, Sergey Fedorov s.fedo...@samsung.com wrote:
A single cast cache is used for both an object casting and a class
casting. In case of interface presence a class cast result may be not
the same pointer as opposite to an object casting. So do not cache cast
results for an object casting in a presence of interfaces.
I think this is fixed by my cast cache splitter patch which is
currently enqueued.
http://patchwork.ozlabs.org/patch/294766/
Regards,
Peter
Yes, your cache splitter looks much better. Thank you.
Best regards,
Sergey Fedorov
Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
---
qom/object.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qom/object.c b/qom/object.c
index fc19cf6..f7384de 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -473,7 +473,7 @@ Object *object_dynamic_cast_assert(Object *obj, const char
*typename,
assert(obj == inst);
-if (obj obj == inst) {
+if (obj obj == inst !obj-class-interfaces) {
for (i = 1; i OBJECT_CLASS_CAST_CACHE; i++) {
obj-class-cast_cache[i - 1] = obj-class-cast_cache[i];
}
--
1.7.9.5
Re: [Qemu-devel] [PATCH 1/5] Add the standby-mem machine option
Il 16/12/2013 21:51, Matthew Rosato ha scritto:
Add the machine=...,standby-mem={size} option and associated
documentation.
See how Igor Mammedov's x86 memory hotplug instead added -m NN,maxmem=NN.
You could use these two patches:
http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg03438.html
http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg03439.html
Paolo
Signed-off-by: Matthew Rosato mjros...@linux.vnet.ibm.com
---
qemu-options.hx |6 +-
vl.c|6 ++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index af34483..def4493 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -35,7 +35,8 @@ DEF(machine, HAS_ARG, QEMU_OPTION_machine, \
kernel_irqchip=on|off controls accelerated irqchip
support\n
kvm_shadow_mem=size of KVM shadow MMU\n
dump-guest-core=on|off include guest memory in a core
dump (default=on)\n
-mem-merge=on|off controls memory merge support
(default: on)\n,
+mem-merge=on|off controls memory merge support
(default: on)\n
+standby-mem=size sets size of additional offline
memory\n,
QEMU_ARCH_ALL)
STEXI
@item -machine [type=]@var{name}[,prop=@var{value}[,...]]
@@ -58,6 +59,9 @@ Include guest memory in a core dump. The default is on.
Enables or disables memory merge support. This feature, when supported by
the host, de-duplicates identical memory pages among VMs instances
(enabled by default).
+@item standby-mem=size
+Defines the size, in megabytes, of additional memory to be left offline for
+future hotplug by the guest.
@end table
ETEXI
diff --git a/vl.c b/vl.c
index b0399de..7d58d24 100644
--- a/vl.c
+++ b/vl.c
@@ -187,6 +187,7 @@ DisplayType display_type = DT_DEFAULT;
static int display_remote;
const char* keyboard_layout = NULL;
ram_addr_t ram_size;
+ram_addr_t standby_mem_size;
const char *mem_path = NULL;
int mem_prealloc = 0; /* force preallocation of physical target memory */
int nb_nics;
@@ -430,6 +431,10 @@ static QemuOptsList qemu_machine_opts = {
.name = firmware,
.type = QEMU_OPT_STRING,
.help = firmware image,
+},{
+.name = standby-mem,
+.type = QEMU_OPT_SIZE,
+.help = standby memory size,
},
{ /* End of list */ }
},
@@ -2906,6 +2911,7 @@ int main(int argc, char **argv, char **envp)
machine = find_default_machine();
cpu_model = NULL;
ram_size = 0;
+standby_mem_size = 0;
snapshot = 0;
cyls = heads = secs = 0;
translation = BIOS_ATA_TRANSLATION_AUTO;
Re: [Qemu-devel] strange failures with SATA drive but not with IDE drive
Il 17/12/2013 05:12, Michael Tokarev ha scritto: When you switch from bus=ahci.0 to bus=ide.0 on the qemu command line above, it all works fine and does not spew any warnings like that about emulation failure. It looks like we have some uninitialized data on sata emulation (in addition to the altmbr issue which I'm investigating further). Next year, I plan to update my patches to improve SATA emulation.
Re: [Qemu-devel] qemu 1.7.0 does not build on NetBSD (patch)
Il 17/12/2013 09:55, Martin Husemann ha scritto: On NetBSD int8_t and friends are preprocessor macros expanding to __int8_t (which is a typedef provided by machine dependent headers). include/exec/softmmu_template.h tries to safe 3 lines of code by using preprosseor concatenation, which only will work if int8_t is not a define, or defined to int8_t. The attached patch fixes this. Hi Martin, the patch looks good, but please resend it according to the rules in http://wiki.qemu.org/Contribute/SubmitAPatch - thanks! Paolo
Re: [Qemu-devel] [PATCH] block: add native support for NFS
On 17.12.2013 10:46, Fam Zheng wrote:
On 2013年12月17日 17:07, Peter Lieven wrote:
On 17.12.2013 10:01, Fam Zheng wrote:
On 2013年12月17日 16:55, Peter Lieven wrote:
On 17.12.2013 09:51, Fam Zheng wrote:
于2013年 12月17日 星期二 16时46分50秒,Peter Lieven写到:
On 17.12.2013 09:29, Fam Zheng wrote:
On 2013年12月17日 15:53, Peter Lieven wrote:
Hi Fam,
On 17.12.2013 05:07, Fam Zheng wrote:
On 2013年12月16日 23:34, Peter Lieven wrote:
+static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
+int64_t sector_num, int
nb_sectors,
+ QEMUIOVector *iov)
+{
+nfsclient *client = bs-opaque;
+struct NFSTask Task;
+char *buf = NULL;
+
+nfs_co_init_task(client, Task);
+
+buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_to_buf(iov, 0, buf, nb_sectors *
BDRV_SECTOR_SIZE);
+
+if (nfs_pwrite_async(client-context, client-fh,
+ sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE,
+ buf, nfs_co_generic_cb, Task) != 0) {
+g_free(buf);
+return -EIO;
+}
+
+while (!Task.complete) {
+nfs_set_events(client);
+qemu_coroutine_yield();
+}
+
+g_free(buf);
+
+if (Task.status != nb_sectors * BDRV_SECTOR_SIZE) {
+return -EIO;
+}
+
+bs-total_sectors = MAX(bs-total_sectors, sector_num +
nb_sectors);
+client-allocated_file_size = -ENOTSUP;
Why does allocated_file_size become not supported after a write?
I thought that someone would ask this ;-) bdrv_allocated_file_size
is only
used in image info. I saved some code here implementing an async
call.
On open I fstat anyway and store that value. For qemu-img info
this is
sufficient, but the allocated size likely changes after a write.
-ENOTSUP
is the default if bdrv_allocated_file_size is not implemented.
OK. Please add some comment here.
+static int nfs_file_open_common(BlockDriverState *bs, QDict
*options, int flags,
+ int open_flags, Error **errp)
+{
+nfsclient *client = bs-opaque;
+const char *filename;
+int ret = 0;
+QemuOpts *opts;
+Error *local_err = NULL;
+char *server = NULL, *path = NULL, *file = NULL, *strp;
+struct stat st;
+
+opts = qemu_opts_create_nofail(runtime_opts);
+qemu_opts_absorb_qdict(opts, options, local_err);
+if (error_is_set(local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+ret = -EINVAL;
+goto fail;
+}
+
+filename = qemu_opt_get(opts, filename);
+
+client-context = nfs_init_context();
+
+if (client-context == NULL) {
+error_setg(errp, Failed to init NFS context);
+ret = -EINVAL;
+goto fail;
+}
+
+server = g_strdup(filename + 6);
Please check the length of filename is longer than 6 before
accessing
filename[6].
Good point. I will check for this, but in fact I think it can't
happen
because we will
never end up there if filename does not start with nfs://
True, at least for now, but it doesn't hurt to be defensive,
especially with strings.
+if (server[0] == '/' || server[0] == '\0') {
+error_setg(errp, Invalid server in URL);
+ret = -EINVAL;
+goto fail;
+}
+strp = strchr(server, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+path = g_strdup(strp);
+*strp = 0;
+strp = strrchr(path, '/');
+if (strp == NULL) {
+error_setg(errp, Invalid URL specified.\n);
+ret = -EINVAL;
+goto fail;
+}
+file = g_strdup(strp);
+*strp = 0;
+
+if (nfs_mount(client-context, server, path) != 0) {
+error_setg(errp, Failed to mount nfs share: %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+
+if (open_flags O_CREAT) {
+if (nfs_creat(client-context, file, 0600, client-fh)
!= 0) {
+error_setg(errp, Failed to create file: %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+} else {
+open_flags = (flags BDRV_O_RDWR) ? O_RDWR : O_RDONLY;
+if (nfs_open(client-context, file, open_flags,
client-fh)
!= 0) {
+error_setg(errp, Failed to open file : %s,
+ nfs_get_error(client-context));
+ret = -EINVAL;
+goto fail;
+}
+}
+
+if (nfs_fstat(client-context, client-fh, st) != 0) {
+error_setg(errp, Failed to fstat file: %s,
+ nfs_get_error(client-context));
+ret = -EIO;
+goto fail;
+}
+
+bs-total_sectors = st.st_size / BDRV_SECTOR_SIZE;
Please use DIV_ROUND_UP(). Otherwise the remainder in last sector
couldn't be read.
Will do. Can't it happen that we end up reading unallocated sectors?
Hmm, maybe. Not missing last bytes of unaligned sector is essential
for VMDK description file. But you are right, please add check
Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
On 17 December 2013 01:24, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
On Tue, Dec 17, 2013 at 10:58 AM, Peter Maydell
peter.mayd...@linaro.org wrote:
On 17 December 2013 00:52, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
On Fri, Dec 13, 2013 at 8:05 PM, Peter Maydell peter.mayd...@linaro.org
wrote:
On 13 December 2013 03:19, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
Why do we need blobs at all? Cant we just fix arm/boot to directly
setup the CPU state to the desired? Rather than complex blobs that
execute ARM instructions just manipulate the regs directly.
We could in theory do this for the primary bootloader, but
the secondary CPU blob has to have a loop in it so we
can sit around waiting for the guest code running in the
primary to tell us it's time to go:
+static const ARMInsnFixup smpboot[] = {
+{ 0xe59f2028 }, /* ldr r2, gic_cpu_if */
+{ 0xe59f0028 }, /* ldr r0, startaddr */
+{ 0xe3a01001 }, /* mov r1, #1 */
+{ 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
+{ 0xe3a010ff }, /* mov r1, #0xff */
+{ 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
+{ 0, FIXUP_DSB }, /* dsb */
+{ 0xe320f003 }, /* wfi */
+{ 0xe5901000 }, /* ldr r1, [r0] */
+{ 0xe1110001 }, /* tst r1, r1 */
+{ 0x0afb }, /* beq wfi */
+{ 0xe12fff11 }, /* bx r1 */
+{ 0, FIXUP_GIC_CPU_IF },
Reading up on Christopher's Kernel documentation link:
Documentation/arm64/booting.txt
Documentation/arm/Booting
I can't see any reference to GIC, where are these GICisms coming from?
v7 secondary CPU boot protocol is platform specific,
though most boards seem to do something vaguely
vexpress like.
So Zynq is very different. It just rewrites the vector table and
resets the secondarys from peripherals rst controller.
Yeah. This is why boot.c supports letting the board provide its
own secondary boot code string (used by exynos and highbank).
The kernel expects that we've set the
GIC up so that the primary CPU can do an IPI to get
the secondary out of the holding pen loop (that's the
wfi / ldr /tst / beq sequence, which loops waiting
for a CPU interrupt).
It seems a bit board-specific and an obstacle to ultimately sanitizing
the embedded bootloaders across the architectures (I hope to one day
boot MB and ARM with one BL once I get the arch-isms out of the boot
flow).
I have another problem while we are on the bootstrap discussion - In
Zynq we have some Linux specific bootstrap issues out in device land.
Our clock driver expects the bootloader to setup some state:
https://github.com/Xilinx/meta-xilinx/blob/master/recipes-devtools/qemu/files/HACK_zynq_slcr_Bring_SLCR_out_of_reset_in_kernel_state.patch
This seems similar to the Vexpress GIC requirement - peripherals
needing linux specific setup. Can we solve both problems here with a
bit or framework allowing devs to have an alternate Linux-specific
reset state?
Then we can ditch on the machine code too :)
I'm not convinced about this at all -- this would be letting the
I know about booting Linux code spread out from boot.c
where it currently is into every device. That sounds like a bad idea.
-- PMM
Re: [Qemu-devel] [PATCH RFC 1/2] rng-egd: improve egd backend performance
On (Tue) 17 Dec 2013 [08:47:34], Markus Armbruster wrote: Amos Kong ak...@redhat.com writes: Bugzilla: https://bugs.launchpad.net/qemu/+bug/1253563 We have a requests queue to cache the random data, but the second will come in when the first request is returned, so we always only have one items in the queue. It effects the performance. This patch changes the IOthread to fill a fixed buffer with random data from egd socket, request_entropy() will return data to virtio queue if buffer has available data. (test with a fast source, disguised egd socket) # cat /dev/urandom | nc -l localhost 8003 # qemu .. -chardev socket,host=localhost,port=8003,id=chr0 \ -object rng-egd,chardev=chr0,id=rng0,buf_size=1024 \ -device virtio-rng-pci,rng=rng0 bytes kb/s -- 131072 - 835 65536 - 652 32768 - 356 16384 - 182 8192 - 99 4096 - 52 2048 - 30 1024 - 15 512 -8 256 -4 128 -3 64 -2 I'm not familiar with the rng-egd code, but perhaps my question has value anyway: could agressive reading ahead on a source of randomness cause trouble by depleting the source? Consider a server restarting a few dozen guests after reboot, where each guest's QEMU then tries to slurp in a couple of KiB of randomness. How does this behave? It is a problem and we surely don't want such a problem. So far, it looks like we need to test this differently. I'm not at all sure these numbers will be meaningful once we get a proper test going. Amit
[Qemu-devel] [qemu-kvm PATCH] block: vhdx - improve error message, and .bdrv_check implementation
If there is a dirty log file to be replayed in a VHDX image, it is
replayed in .vhdx_open(). However, if the file is opened read-only,
then a somewhat cryptic error message results.
This adds a more helpful error message for the user. If an image file
contains a log to be replayed, and is opened read-only, the user is
instructed to run 'qemu-img check -r all' on the image file.
Running qemu-img check -r all will cause the image file to be opened
r/w, which will replay the log file. If a log file replay is detected,
this is flagged, and bdrv_check will increase the corruptions_fixed
count for the image.
Signed-off-by: Jeff Cody jc...@redhat.com
---
block/vhdx-log.c | 12 +++-
block/vhdx.c | 22 --
block/vhdx.h | 5 -
3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index ee5583c..34c563e 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -706,7 +706,8 @@ exit:
*
* If read-only, we must replay the log in RAM (or refuse to open
* a dirty VHDX file read-only) */
-int vhdx_parse_log(BlockDriverState *bs, BDRVVHDXState *s, bool *flushed)
+int vhdx_parse_log(BlockDriverState *bs, BDRVVHDXState *s, bool *flushed,
+ Error **errp)
{
int ret = 0;
VHDXHeader *hdr;
@@ -761,6 +762,15 @@ int vhdx_parse_log(BlockDriverState *bs, BDRVVHDXState *s,
bool *flushed)
}
if (logs.valid) {
+if (bs-read_only) {
+ret = -EPERM;
+error_setg_errno(errp, EPERM,
+ VHDX image file '%s' opened read-only, but
+ contains a log that needs replayed. To replay
+ the log, execute:\n qemu-img check -r all '%s',
+ bs-filename, bs-filename);
+goto exit;
+}
/* now flush the log */
ret = vhdx_log_flush(bs, s, logs);
if (ret 0) {
diff --git a/block/vhdx.c b/block/vhdx.c
index 67bbe10..1995778 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -878,7 +878,6 @@ static int vhdx_open(BlockDriverState *bs, QDict *options,
int flags,
int ret = 0;
uint32_t i;
uint64_t signature;
-bool log_flushed = false;
s-bat = NULL;
@@ -907,7 +906,7 @@ static int vhdx_open(BlockDriverState *bs, QDict *options,
int flags,
goto fail;
}
-ret = vhdx_parse_log(bs, s, log_flushed);
+ret = vhdx_parse_log(bs, s, s-log_replayed_on_open, errp);
if (ret 0) {
goto fail;
}
@@ -1854,6 +1853,24 @@ exit:
return ret;
}
+/* If opened r/w, the VHDX driver will automatically replay the log,
+ * if one is present, inside the vhdx_open() call.
+ *
+ * If qemu-img check -r all is called, the image is automatically opened
+ * r/w and any log has already been replayed, so there is nothing (currently)
+ * for us to do here
+ */
+static int vhdx_check(BlockDriverState *bs, BdrvCheckResult *result,
+ BdrvCheckMode fix)
+{
+BDRVVHDXState *s = bs-opaque;
+
+if (s-log_replayed_on_open) {
+result-corruptions_fixed++;
+}
+return 0;
+}
+
static QEMUOptionParameter vhdx_create_options[] = {
{
.name = BLOCK_OPT_SIZE,
@@ -1898,6 +1915,7 @@ static BlockDriver bdrv_vhdx = {
.bdrv_co_writev = vhdx_co_writev,
.bdrv_create= vhdx_create,
.bdrv_get_info = vhdx_get_info,
+.bdrv_check = vhdx_check,
.create_options = vhdx_create_options,
};
diff --git a/block/vhdx.h b/block/vhdx.h
index 51183b2..2acd7c2 100644
--- a/block/vhdx.h
+++ b/block/vhdx.h
@@ -394,6 +394,8 @@ typedef struct BDRVVHDXState {
Error *migration_blocker;
+bool log_replayed_on_open;
+
QLIST_HEAD(VHDXRegionHead, VHDXRegionEntry) regions;
} BDRVVHDXState;
@@ -408,7 +410,8 @@ uint32_t vhdx_checksum_calc(uint32_t crc, uint8_t *buf,
size_t size,
bool vhdx_checksum_is_valid(uint8_t *buf, size_t size, int crc_offset);
-int vhdx_parse_log(BlockDriverState *bs, BDRVVHDXState *s, bool *flushed);
+int vhdx_parse_log(BlockDriverState *bs, BDRVVHDXState *s, bool *flushed,
+ Error **errp);
int vhdx_log_write_and_flush(BlockDriverState *bs, BDRVVHDXState *s,
void *data, uint32_t length, uint64_t offset);
--
1.8.3.1
Re: [Qemu-devel] [PATCH 09/11] ACPI: move PRST OperationRegion into SSDT
On Mon, Dec 16, 2013 at 10:53:24PM +0100, Igor Mammedov wrote: On Mon, 16 Dec 2013 22:13:30 +0100 Laszlo Ersek ler...@redhat.com wrote: On 12/16/13 21:38, Igor Mammedov wrote: On Mon, 16 Dec 2013 21:30:14 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Dec 13, 2013 at 05:22:14PM +0100, Igor Mammedov wrote: .. and report range used by it to OSPM via _CRS. PRST is needed in SSDT since its base will depend on chipset and will be dynamically set by QEMU. Also move PRSC() method along with PRST since cross table reference to PRST doesn't work. Could you clarify this last sentence? I don't mind where it is but I'd like to know where does the limitation come from. It's empiric deduction so far I haven't found such limitation in spec yet. iasl builds tables just fine but neither linux nor windows were able to find Operation region from SSDT when loading DSDT, failing whole table loading process. Decompiling DSDT/SSDT tables in guest shows that region is in expected scope but OSPM refuses to see it when referenced outside SSDT. There seem to be four things here: - the OperationRegion definition, - its external declaration, - the Field() declaration, - use of fields. I think referencing an OperationRegion defined in another table should work (by way of External). I suspect the tricky part is with Field(): ^^^ it looks like it should work and decompiled tables look fine as well but it unfortunately doesn't. The fields are parts of the object named by RegionName, but their names appear in the same scope as the Field term. So, - maybe moving PRST only, and leaving the definition of PRS (as part of Field()) together with PRSC would suffice, That was the first thing I've tried. - or, after moving the definition of PRS (as part of Field()) together with PRST to another table, all references to PRS (in the PRSC method) would have to be qualified. (But I guess this is what you tried.) yep, that didn't work too. I'm not fun of moving a bunch of code around, but looks like there is no other way. I'd be happy to try if there are any other suggestions. I'm not sure we need to spend too much time on it. Just clarifying that 'doesn't work' means 'builds but makes OSPM fail to resolve the OperationRegion, even though the spec makes it look like this should work' in the commit log should be enough. Laszlo -- Regards, Igor
Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram
Hi,
Problem is that the firmware places the xbar @ 0xb00.
Hardcoded, assuming qemu will not map ram above 0xb000.
Can't bios figure out the size of memory below 4G from fwcfg?
I refer to 7db16f2480db5e246d34d0c453cff4f58549df0e specifically.
It can, but it doesn't.
Additional issue for coreboot is that mmconfig base is a compile-time
constant, because it is setup _very_ early in the boot process.
Coreboot then does the whole pci initialization using mmconfig.
On the other hand coreboot has a much more sophisticated ressource
management than seabios, so moving the mmconf xbar up to to
0xe000-0xefff, then managing two regions (below 0xe000 and
above 0xf000) for pci bars probably isn't a big issue for coreboot.
So, we must (a) fix firmware first and (b) get a ugly dependency
that older firmware will not run on latest qemu.
That's only important for old machine types though, right?
Correct. That makes it a bit less problematic, but it is still not very
nice.
We also need to figure how we wanna fixup things. So, current memory
layout looks like this:
0x - 0xafff -- RAM / unused
0xb000 - 0xbfff -- mmconfig xbar [256 pci busses]
0xc000 - 0xfec0 -- space for pci bars, almost 1g
Largest pci bar we can map below 4g is 512m, @ 0xc000.
If we wanna map 3G RAM we need to move the xbar somewhere else. Big
question is where?
We can move it to 0xc000. Then we can't map 512m pci bars any more.
I would go for this when we have 3G of RAM.
I think that ability to support a single 512m BAR is not all that important.
Use case: pci passthrough of graphics cards.
We can move it to 0xe000. Then we have to split the pci bar space,
mapping large bars below 0xe000 and small ones above 0xf000.
SeaBIOS pci init code isn't really up to it.
Could also become tricky
to declare it correctly in acpi / e820 due to the split.
My laptop's ACPI has this space all fragmented up, seems to boot fine ...
We need to change the way we reserve the mmconfig space though.
Currently it is marked reserved in the e820 table. Having that overlap
with the _CRS region makes windows quite unhappy, we tried that
recently.
My laptop has the mmconfig space declared as LPC ressource:
Device (LPC)
{
Name (_ADR, 0x001F) // _ADR: Address
Name (_S3D, 0x03) // _S3D: S3 Device State
Name (RID, 0x00)
Device (SIO)
{
Name (_HID, EisaId (PNP0C02))
Name (_UID, 0x00) // _UID: Unique ID
Name (SCRS, ResourceTemplate ()
[ ... ]
Memory32Fixed (ReadWrite,
0xF800, // Address Base
0x0400, // Address Length
)
[ ... ]
Method (_CRS, 0, NotSerialized)
[ ... return SCRS, with updates applied in some cases ... ]
When doing it this way we can simply make the PCI0._CRS cover the whole
end-of-ram - ioapic-base range, simliar to piix, and we are pretty free
to place the mmconfig xbar anywhere in that area.
Doing the transition is non-trivial though because we (a) move the job
of reserving the mmconfig area from firmware to qemu and (b) the testing
needed for that.
Maybe we should set the gbyte alignment on q35 aside for a while and
tackle the mmconfig reservation handling first.
cheers,
Gerd
Re: [Qemu-devel] [Bug 1261450] [NEW] libvirtd reload and hooks problem routed-net
On Mon, Dec 16, 2013 at 05:00:51PM -, Chris Weltzien wrote: if we do a reload of libvirt, some iptables rules, which are created through /etc/libvirt/hooks/qemu are not working anymore. Every time a other (one or two,thee) vm is affected. Please report this to the libvirt project: http://libvirt.org/bugs.html Stefan
Re: [Qemu-devel] [PATCH v3 1/7] Add -mem-share option
Il 16/12/2013 16:47, Igor Mammedov ha scritto: memdev is introduced here: http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02532.html as for mem-path mem-prealloc, I was thinking about adding HugePageMem backend to handle hugepage specifics. mem-share could be a part of ShareMem backend or something like this. We need three backends, something like the following: - anonymous mmap (-object memory-ram,size=128M) - file mmap (-object memory-file,shared=on/off,size=128M,file=FILENAME), creates file if size property provided. - mkstemp + file mmap (-object memory-hugepage,size=128M,path=PATH) I don't think shared=on/off is useful in the third case, but it would be trivial to add. Paolo
Re: [Qemu-devel] [PATCH 01/42] ui/sdl2 : initial port to SDL 2.0 (v2.0)
Hi, +if test $sdlabi == 2.0; then Please replace '==' by a single '=' here. dash (and maybe other less sophisticated shells) don't like '=='. I'll fix it up. I know that sdl2.c is based on sdl.c which was coded before the introduction of the current coding rules, but would you mind if I send a follow-up patch which fixes the warnings from checkpatch.pl for sdl2.c? It's in the series already, pretty close to the end, should be easy to find as it carries checkpatch in the $subject. Some of the other patches don't include a Signed-off-by, so those patches are only ready for local testing. I'll have a look before sending out the next version (I expect I need at least one more iteration with cocoa fixes anyway ...). I was very curious to get test results here in my environment because of a strange effect which I had noticed recently: booting a Linux system (Tiny Core Linux) with SDL 1.2 (1:06) takes twice as long as with GTK (0:33) or curses (0:29). The test was run on a remote Linux x86_64 server (so X display output is slower than normal). The huge difference is not caused by more activity but simply by delays were the QEMU process is waiting. The SDL API is synchronous. For remote X11 perform reasonable well you must operate asynchronously. So that is a really bad fit. We might be able to workaround that by running SDL in a thread, so SDL doing a (blocking) wait on the remote X-Server doesn't disturb the qemu iothread. But at the end of the day you are much better off using vnc or spice for a remote display, even in case SDL runs threaded some day. For starters vnc only sends over the parts of the screen which did actually change. vnc also knows when the network link is saturated and skips frames then. cheers, Gerd
[Qemu-devel] [PATCH v2 0/5] Monitor commands for object-add/del
These allow hotplugging (and hot-unplugging without leaking an object) virtio-rng devices. They can also be used for memory hotplug. v1-v2: fix mistyped underscores in qapi-schema.json Paolo Bonzini (5): rng: initialize file descriptor to -1 qom: fix leak for objects created with -object qom: catch errors in object_property_add_child monitor: add object-add (QMP) and object_add (HMP) command monitor: add object-del (QMP) and object_del (HMP) command backends/rng-random.c | 4 +-- hmp-commands.hx | 28 ++ hmp.c | 67 ++ hmp.h | 2 ++ include/monitor/monitor.h | 3 ++ include/qapi/visitor.h| 3 +- include/qemu/typedefs.h | 2 ++ qapi-schema.json | 34 ++ qmp-commands.hx | 51 qmp.c | 74 +++ qom/object.c | 9 -- vl.c | 3 +- 12 files changed, 273 insertions(+), 7 deletions(-) -- 1.8.4.2
[Qemu-devel] [PATCH v2 5/5] monitor: add object-del (QMP) and object_del (HMP) command
These two commands invoke the unparent method of Object.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hmp-commands.hx | 14 ++
hmp.c| 9 +
hmp.h| 1 +
qapi-schema.json | 14 ++
qmp-commands.hx | 24
qmp.c| 12
6 files changed, 74 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 0563b33..cb5aa87 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1257,6 +1257,20 @@ STEXI
Create QOM object.
ETEXI
+{
+.name = object_del,
+.args_type = id:s,
+.params = id,
+.help = destroy QOM object,
+.mhandler.cmd = hmp_object_del,
+},
+
+STEXI
+@item object_del
+@findex object_del
+Destroy QOM object.
+ETEXI
+
#ifdef CONFIG_SLIRP
{
.name = hostfwd_add,
diff --git a/hmp.c b/hmp.c
index a1669ab..9f4c9b3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1622,3 +1622,12 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, err);
}
+
+void hmp_object_del(Monitor *mon, const QDict *qdict)
+{
+const char *id = qdict_get_str(qdict, id);
+Error *err = NULL;
+
+qmp_object_del(id, err);
+hmp_handle_error(mon, err);
+}
diff --git a/hmp.h b/hmp.h
index 521449b..94264d6 100644
--- a/hmp.h
+++ b/hmp.h
@@ -90,5 +90,6 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
void hmp_object_add(Monitor *mon, const QDict *qdict);
+void hmp_object_del(Monitor *mon, const QDict *qdict);
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 111463b..8ca4d0c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2779,6 +2779,20 @@
'gen': 'no' }
##
+# @object-del:
+#
+# Remove a QOM object.
+#
+# @id: the name of the QOM object to remove
+#
+# Returns: Nothing on success
+# Error if @id is not a valid id for a QOM object
+#
+# Since: 2.0
+##
+{ 'command': 'object-del', 'data': {'id': 'str'} }
+
+##
# @NetdevNoneOptions
#
# Use it alone to have zero network devices.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d09ea53..02cc815 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -904,6 +904,30 @@ Example:
EQMP
+{
+.name = object-del,
+.args_type = id:s,
+.mhandler.cmd_new = qmp_marshal_input_object_del,
+},
+
+SQMP
+object-del
+--
+
+Remove QOM object.
+
+Arguments:
+
+- id: the object's ID (json-string)
+
+Example:
+
+- { execute: object-del, arguments: { id: rng1 } }
+- { return: {} }
+
+
+EQMP
+
{
.name = block_resize,
diff --git a/qmp.c b/qmp.c
index 255c55f..0d51e44 100644
--- a/qmp.c
+++ b/qmp.c
@@ -591,3 +591,16 @@ out:
return 0;
}
+
+void qmp_object_del(const char *id, Error **errp)
+{
+Object *obj;
+
+obj = object_resolve_path_component(container_get(object_get_root(),
/objects),
+id);
+if (!obj) {
+error_setg(errp, object id not found);
+return;
+}
+object_unparent(obj);
+}
--
1.8.4.2
[Qemu-devel] [PATCH v2 1/5] rng: initialize file descriptor to -1
The file descriptor is never initialized to -1, which makes rng-random
close stdin if an object is created and immediately destroyed. If we
change it to -1, we also need to protect qemu_set_fd_handler from
receiving a bogus file descriptor.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
backends/rng-random.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/backends/rng-random.c b/backends/rng-random.c
index 68dfc8a..136499d 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -123,15 +123,15 @@ static void rng_random_init(Object *obj)
NULL);
s-filename = g_strdup(/dev/random);
+s-fd = -1;
}
static void rng_random_finalize(Object *obj)
{
RndRandom *s = RNG_RANDOM(obj);
-qemu_set_fd_handler(s-fd, NULL, NULL, NULL);
-
if (s-fd != -1) {
+qemu_set_fd_handler(s-fd, NULL, NULL, NULL);
qemu_close(s-fd);
}
--
1.8.4.2
[Qemu-devel] [PATCH v2 3/5] qom: catch errors in object_property_add_child
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
qom/object.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index fc19cf6..68fe07a 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -988,17 +988,22 @@ static void object_finalize_child_property(Object *obj,
const char *name,
void object_property_add_child(Object *obj, const char *name,
Object *child, Error **errp)
{
+Error *local_err = NULL;
gchar *type;
type = g_strdup_printf(child%s, object_get_typename(OBJECT(child)));
object_property_add(obj, name, type, object_get_child_property,
-NULL, object_finalize_child_property, child, errp);
-
+NULL, object_finalize_child_property, child,
local_err);
+if (error_is_set(local_err)) {
+error_propagate(errp, local_err);
+goto out;
+}
object_ref(child);
g_assert(child-parent == NULL);
child-parent = obj;
+out:
g_free(type);
}
--
1.8.4.2
[Qemu-devel] [PATCH v2 2/5] qom: fix leak for objects created with -object
The object must be unref-ed when its variable goes out of scope.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
vl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 8d5d874..6917fd1 100644
--- a/vl.c
+++ b/vl.c
@@ -2807,12 +2807,13 @@ static int object_create(QemuOpts *opts, void *opaque)
obj = object_new(type);
if (qemu_opt_foreach(opts, object_set_property, obj, 1) 0) {
+object_unref(obj);
return -1;
}
object_property_add_child(container_get(object_get_root(), /objects),
id, obj, NULL);
-
+object_unref(obj);
return 0;
}
--
1.8.4.2
[Qemu-devel] [PATCH v2 4/5] monitor: add object-add (QMP) and object_add (HMP) command
Add two commands that are the monitor counterparts of -object. The commands
have the same Visitor-based implementation, but use different kinds of
visitors so that the HMP command has a DWIM string-based syntax, while
the QMP variant accepts a stricter JSON-based properties dictionary.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hmp-commands.hx | 14 +++
hmp.c | 58
hmp.h | 1 +
include/monitor/monitor.h | 3 +++
include/qapi/visitor.h| 3 +--
include/qemu/typedefs.h | 2 ++
qapi-schema.json | 20 +++
qmp-commands.hx | 27 +
qmp.c | 62 +++
9 files changed, 188 insertions(+), 2 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index caae5ad..0563b33 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1243,6 +1243,20 @@ STEXI
Remove host network device.
ETEXI
+{
+.name = object_add,
+.args_type = object:O,
+.params = [qom-type=]type,id=str[,prop=value][,...],
+.help = create QOM object,
+.mhandler.cmd = hmp_object_add,
+},
+
+STEXI
+@item object_add
+@findex object_add
+Create QOM object.
+ETEXI
+
#ifdef CONFIG_SLIRP
{
.name = hostfwd_add,
diff --git a/hmp.c b/hmp.c
index 32ee285..a1669ab 100644
--- a/hmp.c
+++ b/hmp.c
@@ -21,6 +21,7 @@
#include qmp-commands.h
#include qemu/sockets.h
#include monitor/monitor.h
+#include qapi/opts-visitor.h
#include ui/console.h
#include block/qapi.h
#include qemu-io.h
@@ -1354,6 +1355,63 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, err);
}
+void hmp_object_add(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+QemuOpts *opts;
+char *type = NULL;
+char *id = NULL;
+void *dummy = NULL;
+OptsVisitor *ov;
+QDict *pdict;
+
+opts = qemu_opts_from_qdict(qemu_find_opts(object), qdict, err);
+if (error_is_set(err)) {
+goto out;
+}
+
+ov = opts_visitor_new(opts);
+pdict = qdict_clone_shallow(qdict);
+
+visit_start_struct(opts_get_visitor(ov), dummy, NULL, NULL, 0, err);
+if (error_is_set(err)) {
+goto out_clean;
+}
+
+qdict_del(pdict, qom-type);
+visit_type_str(opts_get_visitor(ov), type, qom-type, err);
+if (error_is_set(err)) {
+goto out_clean;
+}
+
+qdict_del(pdict, id);
+visit_type_str(opts_get_visitor(ov), id, id, err);
+if (error_is_set(err)) {
+goto out_clean;
+}
+
+object_add(type, id, pdict, opts_get_visitor(ov), err);
+if (error_is_set(err)) {
+goto out_clean;
+}
+visit_end_struct(opts_get_visitor(ov), err);
+if (error_is_set(err)) {
+qmp_object_del(id, NULL);
+}
+
+out_clean:
+opts_visitor_cleanup(ov);
+
+QDECREF(pdict);
+qemu_opts_del(opts);
+g_free(id);
+g_free(type);
+g_free(dummy);
+
+out:
+hmp_handle_error(mon, err);
+}
+
void hmp_getfd(Monitor *mon, const QDict *qdict)
{
const char *fdname = qdict_get_str(qdict, fdname);
diff --git a/hmp.h b/hmp.h
index 54cf71f..521449b 100644
--- a/hmp.h
+++ b/hmp.h
@@ -89,5 +89,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
+void hmp_object_add(Monitor *mon, const QDict *qdict);
#endif
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 10fa0e3..22d8b8f 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -93,6 +93,9 @@ int monitor_read_password(Monitor *mon, ReadLineFunc
*readline_func,
int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
+int qmp_object_add(Monitor *mon, const QDict *qdict, QObject **ret);
+void object_add(const char *type, const char *id, const QDict *qdict,
+Visitor *v, Error **errp);
AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
bool has_opaque, const char *opaque,
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 48a2a2e..29da211 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -13,6 +13,7 @@
#ifndef QAPI_VISITOR_CORE_H
#define QAPI_VISITOR_CORE_H
+#include qemu/typedefs.h
#include qapi/qmp/qobject.h
#include qapi/error.h
#include stdlib.h
@@ -26,8 +27,6 @@ typedef struct GenericList
struct GenericList *next;
} GenericList;
-typedef struct Visitor Visitor;
-
void visit_start_handle(Visitor *v, void **obj, const char *kind,
const char *name, Error **errp);
void visit_end_handle(Visitor *v, Error **errp);
diff --git a/include/qemu/typedefs.h
Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
On Tue, Dec 17, 2013 at 8:31 PM, Peter Maydell peter.mayd...@linaro.org wrote:
On 17 December 2013 01:24, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
On Tue, Dec 17, 2013 at 10:58 AM, Peter Maydell
peter.mayd...@linaro.org wrote:
On 17 December 2013 00:52, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
On Fri, Dec 13, 2013 at 8:05 PM, Peter Maydell peter.mayd...@linaro.org
wrote:
On 13 December 2013 03:19, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
Why do we need blobs at all? Cant we just fix arm/boot to directly
setup the CPU state to the desired? Rather than complex blobs that
execute ARM instructions just manipulate the regs directly.
We could in theory do this for the primary bootloader, but
the secondary CPU blob has to have a loop in it so we
can sit around waiting for the guest code running in the
primary to tell us it's time to go:
+static const ARMInsnFixup smpboot[] = {
+{ 0xe59f2028 }, /* ldr r2, gic_cpu_if */
+{ 0xe59f0028 }, /* ldr r0, startaddr */
+{ 0xe3a01001 }, /* mov r1, #1 */
+{ 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
+{ 0xe3a010ff }, /* mov r1, #0xff */
+{ 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff
*/
+{ 0, FIXUP_DSB }, /* dsb */
+{ 0xe320f003 }, /* wfi */
+{ 0xe5901000 }, /* ldr r1, [r0] */
+{ 0xe1110001 }, /* tst r1, r1 */
+{ 0x0afb }, /* beq wfi */
+{ 0xe12fff11 }, /* bx r1 */
+{ 0, FIXUP_GIC_CPU_IF },
Reading up on Christopher's Kernel documentation link:
Documentation/arm64/booting.txt
Documentation/arm/Booting
I can't see any reference to GIC, where are these GICisms coming from?
v7 secondary CPU boot protocol is platform specific,
though most boards seem to do something vaguely
vexpress like.
So Zynq is very different. It just rewrites the vector table and
resets the secondarys from peripherals rst controller.
Yeah. This is why boot.c supports letting the board provide its
own secondary boot code string (used by exynos and highbank).
The kernel expects that we've set the
GIC up so that the primary CPU can do an IPI to get
the secondary out of the holding pen loop (that's the
wfi / ldr /tst / beq sequence, which loops waiting
for a CPU interrupt).
It seems a bit board-specific and an obstacle to ultimately sanitizing
the embedded bootloaders across the architectures (I hope to one day
boot MB and ARM with one BL once I get the arch-isms out of the boot
flow).
I have another problem while we are on the bootstrap discussion - In
Zynq we have some Linux specific bootstrap issues out in device land.
Our clock driver expects the bootloader to setup some state:
https://github.com/Xilinx/meta-xilinx/blob/master/recipes-devtools/qemu/files/HACK_zynq_slcr_Bring_SLCR_out_of_reset_in_kernel_state.patch
This seems similar to the Vexpress GIC requirement - peripherals
needing linux specific setup. Can we solve both problems here with a
bit or framework allowing devs to have an alternate Linux-specific
reset state?
Then we can ditch on the machine code too :)
I'm not convinced about this at all -- this would be letting the
I know about booting Linux code spread out from boot.c
where it currently is into every device. That sounds like a bad idea. -
So the I know about boot linux code already has to span multiple
architectures it's already reasonably 'spread out' just looking at the
zoo of Linux bootloaders. Its also a contributor to the much disliked
inconsistencies in boot flow between archs.
If we can outsource arch/board specific bringup to device land it
would allow consolidation of all the different Linux bootloaders. Or
at least across embedded - E.g. Microblaze, ARM, some of the board
level PPC bootloaders O.R. and the newly posted Moxie machine.
ARM_CPU / MICROBLAZE_CPU would be responsible for setting dtb reg ptr
/ kernel args, program entry point.
ARM_GIC would set itself up.
ZYNQ_SLCR would set itself up.
QOM interfaces can make this purely additive and unobtrusive. E.G.
interface TYPE_LINUX_RESET implements a reset hook that runs iff the
system is booting Linux.
Anyways,
we are not going to solve this today and at the end of the day, the
patch does make this much more self documenting and flexible for ARM,
so:
Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Regards,
Peter
-- PMM
Re: [Qemu-devel] [PATCH 2/7] target-arm: Clean up handling of AArch64 PSTATE
On 17 December 2013 04:45, Christoffer Dall christoffer.d...@linaro.org wrote: I think this could be written slightly more clearly for the uninitiated, but maybe I'm just not qemu-savy enough. It was a bit compressed; I've reworded it to: /* PSTATE isn't an architectural register for ARMv8. However, it is * convenient for us to assemble the underlying state into a 32 bit format * identical to the architectural format used for the SPSR. (This is also * what the Linux kernel's 'pstate' field in signal handlers and KVM's * 'pstate' register are.) Of the PSTATE bits: * NZCV are kept in the split out env-CF/VF/NF/ZF, (which have the same *semantics as for AArch32, as described in the comments on each field) * nRW (also known as M[4]) is kept, inverted, in env-aarch64 * all other bits are stored in their correct places in env-pstate */ thanks -- PMM
Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
On 17 December 2013 11:36, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: On Tue, Dec 17, 2013 at 8:31 PM, Peter Maydell peter.mayd...@linaro.org wrote: I'm not convinced about this at all -- this would be letting the I know about booting Linux code spread out from boot.c where it currently is into every device. That sounds like a bad idea. - So the I know about boot linux code already has to span multiple architectures it's already reasonably 'spread out' just looking at the zoo of Linux bootloaders. Its also a contributor to the much disliked inconsistencies in boot flow between archs. If we can outsource arch/board specific bringup to device land it would allow consolidation of all the different Linux bootloaders. Or at least across embedded - E.g. Microblaze, ARM, some of the board level PPC bootloaders O.R. and the newly posted Moxie machine. ARM_CPU / MICROBLAZE_CPU would be responsible for setting dtb reg ptr / kernel args, program entry point. ARM_GIC would set itself up. ...but how? What the GIC state needs to be depends on the board. It's times like this I see the appeal of pushing the whole thing off onto a custom firmware blob which does the work :-) we are not going to solve this today and at the end of the day, the patch does make this much more self documenting and flexible for ARM, so: Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Thanks. -- PMM
Re: [Qemu-devel] [PATCH v2 4/5] monitor: add object-add (QMP) and object_add (HMP) command
On Tue, Dec 17, 2013 at 9:26 PM, Paolo Bonzini pbonz...@redhat.com wrote:
Add two commands that are the monitor counterparts of -object. The commands
have the same Visitor-based implementation, but use different kinds of
visitors so that the HMP command has a DWIM string-based syntax, while
the QMP variant accepts a stricter JSON-based properties dictionary.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hmp-commands.hx | 14 +++
hmp.c | 58
hmp.h | 1 +
include/monitor/monitor.h | 3 +++
include/qapi/visitor.h| 3 +--
include/qemu/typedefs.h | 2 ++
qapi-schema.json | 20 +++
qmp-commands.hx | 27 +
qmp.c | 62
+++
9 files changed, 188 insertions(+), 2 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index caae5ad..0563b33 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1243,6 +1243,20 @@ STEXI
Remove host network device.
ETEXI
+{
+.name = object_add,
+.args_type = object:O,
+.params = [qom-type=]type,id=str[,prop=value][,...],
+.help = create QOM object,
+.mhandler.cmd = hmp_object_add,
+},
+
+STEXI
+@item object_add
+@findex object_add
+Create QOM object.
+ETEXI
+
#ifdef CONFIG_SLIRP
{
.name = hostfwd_add,
diff --git a/hmp.c b/hmp.c
index 32ee285..a1669ab 100644
--- a/hmp.c
+++ b/hmp.c
@@ -21,6 +21,7 @@
#include qmp-commands.h
#include qemu/sockets.h
#include monitor/monitor.h
+#include qapi/opts-visitor.h
#include ui/console.h
#include block/qapi.h
#include qemu-io.h
@@ -1354,6 +1355,63 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, err);
}
+void hmp_object_add(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+QemuOpts *opts;
+char *type = NULL;
+char *id = NULL;
+void *dummy = NULL;
+OptsVisitor *ov;
+QDict *pdict;
+
+opts = qemu_opts_from_qdict(qemu_find_opts(object), qdict, err);
+if (error_is_set(err)) {
+goto out;
+}
+
+ov = opts_visitor_new(opts);
+pdict = qdict_clone_shallow(qdict);
+
+visit_start_struct(opts_get_visitor(ov), dummy, NULL, NULL, 0, err);
+if (error_is_set(err)) {
+goto out_clean;
+}
So I have been thinking about repeated if(error_is_set(err)) { goto
foo; } and how to reduce its verbosity in situations like this. Can it
be solved with a simple semantic:
Error ** accepting APIs will perform no action if the Error **
argument is already set.
In this case we would patch visit_foo to just return if
error_is_set(errp). Then we can delete these three LOC...
+
+qdict_del(pdict, qom-type);
+visit_type_str(opts_get_visitor(ov), type, qom-type, err);
+if (error_is_set(err)) {
+goto out_clean;
+}
+
and these
+qdict_del(pdict, id);
+visit_type_str(opts_get_visitor(ov), id, id, err);
+if (error_is_set(err)) {
+goto out_clean;
+}
and these
+
+object_add(type, id, pdict, opts_get_visitor(ov), err);
+if (error_is_set(err)) {
+goto out_clean;
+}
And leave just the last one which will catch and report the
first-occured error as desired.
Out of scope of the series of course.
Regards,
Peter
+visit_end_struct(opts_get_visitor(ov), err);
+if (error_is_set(err)) {
+qmp_object_del(id, NULL);
+}
+
+out_clean:
+opts_visitor_cleanup(ov);
+
+QDECREF(pdict);
+qemu_opts_del(opts);
+g_free(id);
+g_free(type);
+g_free(dummy);
+
+out:
+hmp_handle_error(mon, err);
+}
+
void hmp_getfd(Monitor *mon, const QDict *qdict)
{
const char *fdname = qdict_get_str(qdict, fdname);
diff --git a/hmp.h b/hmp.h
index 54cf71f..521449b 100644
--- a/hmp.h
+++ b/hmp.h
@@ -89,5 +89,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
+void hmp_object_add(Monitor *mon, const QDict *qdict);
#endif
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 10fa0e3..22d8b8f 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -93,6 +93,9 @@ int monitor_read_password(Monitor *mon, ReadLineFunc
*readline_func,
int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
+int qmp_object_add(Monitor *mon, const QDict *qdict, QObject **ret);
+void object_add(const char *type, const char *id, const QDict *qdict,
+Visitor *v, Error **errp);
AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id,
Re: [Qemu-devel] [PATCH v2] x86: gigabyte alignment for ram
On Tue, Dec 17, 2013 at 11:54:46AM +0100, Gerd Hoffmann wrote:
Hi,
Problem is that the firmware places the xbar @ 0xb00.
Hardcoded, assuming qemu will not map ram above 0xb000.
Can't bios figure out the size of memory below 4G from fwcfg?
I refer to 7db16f2480db5e246d34d0c453cff4f58549df0e specifically.
It can, but it doesn't.
Additional issue for coreboot is that mmconfig base is a compile-time
constant, because it is setup _very_ early in the boot process.
Coreboot then does the whole pci initialization using mmconfig.
On the other hand coreboot has a much more sophisticated ressource
management than seabios, so moving the mmconf xbar up to to
0xe000-0xefff, then managing two regions (below 0xe000 and
above 0xf000) for pci bars probably isn't a big issue for coreboot.
So, we must (a) fix firmware first and (b) get a ugly dependency
that older firmware will not run on latest qemu.
That's only important for old machine types though, right?
Correct. That makes it a bit less problematic, but it is still not very
nice.
We also need to figure how we wanna fixup things. So, current memory
layout looks like this:
0x - 0xafff -- RAM / unused
0xb000 - 0xbfff -- mmconfig xbar [256 pci busses]
0xc000 - 0xfec0 -- space for pci bars, almost 1g
Largest pci bar we can map below 4g is 512m, @ 0xc000.
If we wanna map 3G RAM we need to move the xbar somewhere else. Big
question is where?
We can move it to 0xc000. Then we can't map 512m pci bars any more.
I would go for this when we have 3G of RAM.
I think that ability to support a single 512m BAR is not all that important.
Use case: pci passthrough of graphics cards.
We can move it to 0xe000. Then we have to split the pci bar space,
mapping large bars below 0xe000 and small ones above 0xf000.
SeaBIOS pci init code isn't really up to it.
Could also become tricky
to declare it correctly in acpi / e820 due to the split.
My laptop's ACPI has this space all fragmented up, seems to boot fine ...
We need to change the way we reserve the mmconfig space though.
Currently it is marked reserved in the e820 table. Having that overlap
with the _CRS region makes windows quite unhappy, we tried that
recently.
Yes this also contradicts the spec, see below.
My laptop has the mmconfig space declared as LPC ressource:
Device (LPC)
{
Name (_ADR, 0x001F) // _ADR: Address
Name (_S3D, 0x03) // _S3D: S3 Device State
Name (RID, 0x00)
Device (SIO)
{
Name (_HID, EisaId (PNP0C02))
Name (_UID, 0x00) // _UID: Unique ID
Name (SCRS, ResourceTemplate ()
[ ... ]
Memory32Fixed (ReadWrite,
0xF800, // Address Base
0x0400, // Address Length
)
[ ... ]
Method (_CRS, 0, NotSerialized)
[ ... return SCRS, with updates applied in some cases ... ]
When doing it this way we can simply make the PCI0._CRS cover the whole
end-of-ram - ioapic-base range, simliar to piix, and we are pretty free
to place the mmconfig xbar anywhere in that area.
The spec says:
2.If the operating system does not natively comprehend reserving the
MMCFG region, the
MMCFG region must be reserved by firmware. The address range reported in
the MCFG table
or by _CBA method (see Section 4.1.3) must be reserved by declaring a
motherboard resource.
For most systems, the motherboard resource would appear at the root of
the ACPI namespace
(under \_SB) in a node with a _HID of EISAID (PNP0C02), and the
resources in this case
should not be claimed in the root PCI bus’s _CRS. The resources can
optionally be returned in
Int15 E820 or EFIGetMemoryMap as reserved memory but must always be
reported through
ACPI as a motherboard resource.
My reading of the above is that this can be an LPC resource but
claiming this as the root's _CRS isn't ok then.
Doing the transition is non-trivial though because we (a) move the job
of reserving the mmconfig area from firmware to qemu and (b) the testing
needed for that.
Maybe we should set the gbyte alignment on q35 aside for a while and
tackle the mmconfig reservation handling first.
cheers,
Gerd
I merged your patch but split it: q35 is separate and piix
is separate. Would you like me to drop the q35 part then?
Re: [Qemu-devel] [PATCH 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API
On Mon, 16 Dec 2013 15:26:37 -0800 Anthony Liguori anth...@codemonkey.ws wrote: Igor Mammedov imamm...@redhat.com writes: changes since v2: * s/hotplugable/hotpluggable/ * move hotplug check to an earlier patch: qdev: add hotpluggable property to Device -- Refactor PCI specific hotplug API to a more generic/reusable one. Model it after SCSI-BUS like hotplug API replacing single hotplug callback with hotplug/hot_unplug pair of callbacks as suggested by Paolo. Difference between SCSI-BUS and this approach is that the former is BUS centric while the latter is device centred. Which is evolved from the fact that hotplug callbacks used by ACPI/SHPC/PCIE are implemented by devices rather than by bus and bus serves only as a proxy to forward event to hotplug device. Memory hotplug also exposes tha same usage pattern hence an attempt to generalize hotplug API. Refactoring also simplifies wiring of a hotplug device with a bus, all it needs is to set hotplug-device link on bus, which would potentially allow to do it from configuration file, there is not need to setup hotplug device callbacks on bus synce it can get them via HOTPLUG_DEVICE API of hotplug-device target. In addition device centred hotplug API may be used by bus-less hotplug implementations as well if it's decided to use linkfoo... instead of bus. I'm having a hard time parsing this description. Sharing hot plug code is a good thing. Making hotplug a qdev-level concept seems like a bad thing to me. we could add TYPE_BUS_DEVICE as Peter suggests but let me explain why I've chosen DEVICE for it. All current hotplug code depends on hotpluggable bus but periodically there were suggestions to use links for hotplug instead of bus. Making hotplug the bus device only concept will not allow to move in direction of hotplug with links which are/were supposed to replace buses in future. As side effect series lays ground work for link based hotplug, it's not complete but a step towards it. It still doesn't allow link based hotplug, since purpose was to have unified hotplug interface across PCI/SCSI/virtio devices/buses. Maybe we need TYPE_HOTPLUGGABLE_DEVICE so it could be abstracted from bus as well and eventually it could be used with bussless devices? The series is a net add of code so I don't think we're winning anything by generalizing here. it will allow to remove different implementations of hotplug mechanism in SCSI/virtio and maybe USB buses in addition to converted PCI hotplug. Is there a use-case this enables that isn't possible today? It was prompted by memory hotplug, alternative was that it can re-implement hotplug mechanism in its own way or duplicating code from scsi or pci buses/devices. Neither alternatives look nice when there is a common usage pattern. Regards, Anthony Liguori Patches 8-11 are should be merged as one and are split only for simplifying review (they compile fine but PCI hotplug is broken until the last patch is applyed). git tree for testing: https://github.com/imammedo/qemu/commits/hotplug_dev_inf_v3 tested only ACPI and PCIE hotplug. Hervé Poussineau (1): qom: detect bad reentrance during object_class_foreach Igor Mammedov (9): define hotplug interface qdev: add to BusState hotplug-handler link qdev: add hotpluggable property to Device hw/acpi: move typeinfo to the file end qdev:pci: refactor PCIDevice to use generic hotpluggable property acpi/piix4pm: convert ACPI PCI hotplug to use hotplug-handler API pci/shpc: convert SHPC hotplug to use hotplug-handler API pci/pcie: convert PCIE hotplug to use hotplug-handler API hw/pci: switch to a generic hotplug handling for PCIDevice Paolo Bonzini (1): qom: do not register interface types in the type table hw/acpi/piix4.c| 151 ++--- hw/core/Makefile.objs | 1 + hw/core/hotplug.c | 48 + hw/core/qdev.c | 50 -- hw/display/cirrus_vga.c| 2 +- hw/display/qxl.c | 2 +- hw/display/vga-pci.c | 2 +- hw/display/vmware_vga.c| 2 +- hw/i386/acpi-build.c | 6 +- hw/ide/piix.c | 4 +- hw/isa/piix4.c | 2 +- hw/pci-bridge/pci_bridge_dev.c | 9 +++ hw/pci-host/piix.c | 6 +- hw/pci/pci.c | 40 +-- hw/pci/pcie.c | 73 +--- hw/pci/pcie_port.c | 8 +++ hw/pci/shpc.c | 133 +++- hw/usb/hcd-ehci-pci.c | 2 +- hw/usb/hcd-ohci.c | 2 +- hw/usb/hcd-uhci.c | 2 +- hw/usb/hcd-xhci.c | 2 +- include/hw/hotplug.h | 75 include/hw/pci/pci.h | 13
[Qemu-devel] [PATCH] vmdk: Allow vmdk_create to work with protocol
This changes vmdk_create to use bdrv_* functions to replace qemu_open
and other fd functions. The error handling are improved as well. One
difference is that bdrv_pwrite will round up buffer to sectors, so for
description file, an extra bdrv_truncate is used in the end to drop
ending zeros.
I tested that new code produces exactly the same file as previously.
Signed-off-by: Fam Zheng f...@redhat.com
---
block/vmdk.c | 137 ++-
1 file changed, 79 insertions(+), 58 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 0734bc2..76efa38 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1447,23 +1447,33 @@ static int coroutine_fn
vmdk_co_write_zeroes(BlockDriverState *bs,
}
static int vmdk_create_extent(const char *filename, int64_t filesize,
- bool flat, bool compress, bool zeroed_grain)
+ bool flat, bool compress, bool zeroed_grain,
+ Error **errp)
{
int ret, i;
-int fd = 0;
+BlockDriverState *bs = NULL;
VMDK4Header header;
-uint32_t tmp, magic, grains, gd_size, gt_size, gt_count;
+Error *local_err;
+uint32_t tmp, magic, grains, gd_sectors, gt_size, gt_count;
+uint32_t *gd_buf = NULL;
+int gd_buf_size;
-fd = qemu_open(filename,
- O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
- 0644);
-if (fd 0) {
-return -errno;
+ret = bdrv_create_file(filename, NULL, local_err);
+if (ret 0) {
+error_propagate(errp, local_err);
+goto exit;
}
+
+ret = bdrv_file_open(bs, filename, NULL, BDRV_O_RDWR, local_err);
+if (ret 0) {
+error_propagate(errp, local_err);
+goto exit;
+}
+
if (flat) {
-ret = ftruncate(fd, filesize);
+ret = bdrv_truncate(bs, filesize);
if (ret 0) {
-ret = -errno;
+error_setg(errp, Could not truncate file);
}
goto exit;
}
@@ -1482,14 +1492,14 @@ static int vmdk_create_extent(const char *filename,
int64_t filesize,
gt_size = ((header.num_gtes_per_gt * sizeof(uint32_t)) + 511) 9;
gt_count =
(grains + header.num_gtes_per_gt - 1) / header.num_gtes_per_gt;
-gd_size = (gt_count * sizeof(uint32_t) + 511) 9;
+gd_sectors = (gt_count * sizeof(uint32_t) + 511) 9;
header.desc_offset = 1;
header.desc_size = 20;
header.rgd_offset = header.desc_offset + header.desc_size;
-header.gd_offset = header.rgd_offset + gd_size + (gt_size * gt_count);
+header.gd_offset = header.rgd_offset + gd_sectors + (gt_size * gt_count);
header.grain_offset =
- ((header.gd_offset + gd_size + (gt_size * gt_count) +
+ ((header.gd_offset + gd_sectors + (gt_size * gt_count) +
header.granularity - 1) / header.granularity) *
header.granularity;
/* swap endianness for all header fields */
@@ -1511,48 +1521,55 @@ static int vmdk_create_extent(const char *filename,
int64_t filesize,
header.check_bytes[3] = 0xa;
/* write all the data */
-ret = qemu_write_full(fd, magic, sizeof(magic));
-if (ret != sizeof(magic)) {
-ret = -errno;
+ret = bdrv_pwrite(bs, 0, magic, sizeof(magic));
+if (ret 0) {
+error_set(errp, QERR_IO_ERROR);
goto exit;
}
-ret = qemu_write_full(fd, header, sizeof(header));
+ret = bdrv_pwrite(bs, sizeof(magic), header, sizeof(header));
if (ret != sizeof(header)) {
+error_set(errp, QERR_IO_ERROR);
ret = -errno;
goto exit;
}
-ret = ftruncate(fd, le64_to_cpu(header.grain_offset) 9);
+ret = bdrv_truncate(bs, (le64_to_cpu(header.grain_offset)) 9);
if (ret 0) {
-ret = -errno;
-goto exit;
+error_setg(errp, Could not truncate file);
}
/* write grain directory */
-lseek(fd, le64_to_cpu(header.rgd_offset) 9, SEEK_SET);
-for (i = 0, tmp = le64_to_cpu(header.rgd_offset) + gd_size;
+gd_buf_size = gd_sectors * BDRV_SECTOR_SIZE * sizeof(*gd_buf);
+gd_buf = g_malloc0(gd_buf_size);
+for (i = 0, tmp = le64_to_cpu(header.rgd_offset) + gd_sectors;
i gt_count; i++, tmp += gt_size) {
-ret = qemu_write_full(fd, tmp, sizeof(tmp));
-if (ret != sizeof(tmp)) {
-ret = -errno;
-goto exit;
-}
+gd_buf[i] = cpu_to_le32(tmp);
+}
+ret = bdrv_pwrite(bs, le64_to_cpu(header.rgd_offset) * BDRV_SECTOR_SIZE,
+ gd_buf, gd_buf_size);
+if (ret 0) {
+error_set(errp, QERR_IO_ERROR);
+goto exit;
}
/* write backup grain directory */
-lseek(fd, le64_to_cpu(header.gd_offset) 9, SEEK_SET);
-for (i = 0, tmp = le64_to_cpu(header.gd_offset) + gd_size;
+for (i = 0, tmp = le64_to_cpu(header.gd_offset) + gd_sectors;
i gt_count; i++, tmp += gt_size) {
-ret = qemu_write_full(fd, tmp,
Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code
On Tue, Dec 17, 2013 at 9:47 PM, Peter Maydell peter.mayd...@linaro.org wrote:
On 17 December 2013 11:36, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
On Tue, Dec 17, 2013 at 8:31 PM, Peter Maydell peter.mayd...@linaro.org
wrote:
I'm not convinced about this at all -- this would be letting the
I know about booting Linux code spread out from boot.c
where it currently is into every device. That sounds like a bad idea. -
So the I know about boot linux code already has to span multiple
architectures it's already reasonably 'spread out' just looking at the
zoo of Linux bootloaders. Its also a contributor to the much disliked
inconsistencies in boot flow between archs.
If we can outsource arch/board specific bringup to device land it
would allow consolidation of all the different Linux bootloaders. Or
at least across embedded - E.g. Microblaze, ARM, some of the board
level PPC bootloaders O.R. and the newly posted Moxie machine.
ARM_CPU / MICROBLAZE_CPU would be responsible for setting dtb reg ptr
/ kernel args, program entry point.
ARM_GIC would set itself up.
...but how? What the GIC state needs to be depends on the board.
In such cases, boards could trivially define a lightweight QOM child class
(of the device in question) that implements the linux_reset() hook their
specific way:
TypeInfo vexpress_gic_info {
.parent = TYPE_ARM_GIC,
.interfaces = {
{ TYPE_LINUX_RESET },
{}
},
.class_init = vexpress_gic_class_init,
}
Regards,
Peter
It's times like this I see the appeal of pushing the whole thing off onto
a custom firmware blob which does the work :-)
we are not going to solve this today and at the end of the day, the
patch does make this much more self documenting and flexible for ARM,
so:
Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Thanks.
-- PMM
[Qemu-devel] [PATCH v2 6/7] hw/arm/boot: Add boot support for AArch64 processor
From: Mian M. Hamayun m.hama...@virtualopensystems.com
This commit adds support for booting a single AArch64 CPU by setting
appropriate registers. The bootloader includes placehoders for Board-ID
that are used to implement uniform indexing across different bootloaders.
Signed-off-by: Mian M. Hamayun m.hama...@virtualopensystems.com
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Message-id: 1385645602-18662-7-git-send-email-peter.mayd...@linaro.org
[PMM:
* updated to use ARMInsnFixup style bootloader fragments
* dropped virt.c additions
* use runtime checks for is this an AArch64 core rather than ifdefs
* drop some unnecessary setting of registers in reset hook
]
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Christoffer Dall christoffer.d...@linaro.org
---
hw/arm/boot.c | 43 ++-
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 0c05a64..90e9534 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -17,8 +17,13 @@
#include sysemu/device_tree.h
#include qemu/config-file.h
+/* Kernel boot protocol is specified in the kernel docs
+ * Documentation/arm/Booting and Documentation/arm64/booting.txt
+ * They have different preferred image load offsets from system RAM base.
+ */
#define KERNEL_ARGS_ADDR 0x100
#define KERNEL_LOAD_ADDR 0x0001
+#define KERNEL64_LOAD_ADDR 0x0008
typedef enum {
FIXUP_NONE = 0, /* do nothing */
@@ -37,6 +42,20 @@ typedef struct ARMInsnFixup {
FixupType fixup;
} ARMInsnFixup;
+static const ARMInsnFixup bootloader_aarch64[] = {
+{ 0x58c0 }, /* ldr x0, arg ; Load the lower 32-bits of DTB */
+{ 0xaa1f03e1 }, /* mov x1, xzr */
+{ 0xaa1f03e2 }, /* mov x2, xzr */
+{ 0xaa1f03e3 }, /* mov x3, xzr */
+{ 0x5884 }, /* ldr x4, entry ; Load the lower 32-bits of kernel entry
*/
+{ 0xd61f0080 }, /* br x4 ; Jump to the kernel entry point */
+{ 0, FIXUP_ARGPTR }, /* arg: .word @DTB Lower 32-bits */
+{ 0 }, /* .word @DTB Higher 32-bits */
+{ 0, FIXUP_ENTRYPOINT }, /* entry: .word @Kernel Entry Lower 32-bits */
+{ 0 }, /* .word @Kernel Entry Higher 32-bits */
+{ 0, FIXUP_TERMINATOR }
+};
+
/* The worlds second smallest bootloader. Set r0-r2, then jump to kernel. */
static const ARMInsnFixup bootloader[] = {
{ 0xe3a0 }, /* mov r0, #0 */
@@ -396,7 +415,12 @@ static void do_cpu_reset(void *opaque)
env-thumb = info-entry 1;
} else {
if (CPU(cpu) == first_cpu) {
-env-regs[15] = info-loader_start;
+if (env-aarch64) {
+env-pc = info-loader_start;
+} else {
+env-regs[15] = info-loader_start;
+}
+
if (!info-dtb_filename) {
if (old_param) {
set_kernel_args_old(info);
@@ -418,8 +442,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
int initrd_size;
int is_linux = 0;
uint64_t elf_entry;
-hwaddr entry;
+hwaddr entry, kernel_load_offset;
int big_endian;
+static const ARMInsnFixup *primary_loader;
/* Load the kernel. */
if (!info-kernel_filename) {
@@ -429,6 +454,14 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
return;
}
+if (arm_feature(cpu-env, ARM_FEATURE_AARCH64)) {
+primary_loader = bootloader_aarch64;
+kernel_load_offset = KERNEL64_LOAD_ADDR;
+} else {
+primary_loader = bootloader;
+kernel_load_offset = KERNEL_LOAD_ADDR;
+}
+
info-dtb_filename = qemu_opt_get(qemu_get_machine_opts(), dtb);
if (!info-secondary_cpu_reset_hook) {
@@ -469,9 +502,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
is_linux);
}
if (kernel_size 0) {
-entry = info-loader_start + KERNEL_LOAD_ADDR;
+entry = info-loader_start + kernel_load_offset;
kernel_size = load_image_targphys(info-kernel_filename, entry,
- info-ram_size - KERNEL_LOAD_ADDR);
+ info-ram_size - kernel_load_offset);
is_linux = 1;
}
if (kernel_size 0) {
@@ -532,7 +565,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
fixupcontext[FIXUP_ENTRYPOINT] = entry;
write_bootloader(bootloader, info-loader_start,
- bootloader, fixupcontext);
+ primary_loader, fixupcontext);
if (info-nb_cpus 1) {
info-write_secondary_boot(cpu, info);
--
1.8.5
[Qemu-devel] [PATCH v2 7/7] default-configs: Add config for aarch64-softmmu
Add a config for aarch64-softmmu; this enables building of this target. The resulting executable doesn't know about any 64 bit CPUs, but all the 32 bit CPUs and board models work. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Message-id: 1385645602-18662-8-git-send-email-peter.mayd...@linaro.org --- default-configs/aarch64-softmmu.mak | 6 ++ 1 file changed, 6 insertions(+) create mode 100644 default-configs/aarch64-softmmu.mak diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak new file mode 100644 index 000..6d3b5c7 --- /dev/null +++ b/default-configs/aarch64-softmmu.mak @@ -0,0 +1,6 @@ +# Default configuration for aarch64-softmmu + +# We support all the 32 bit boards so need all their config +include arm-softmmu.mak + +# Currently no 64-bit specific config requirements -- 1.8.5
[Qemu-devel] [PATCH v2 5/7] hw/arm/boot: Allow easier swapping in of different loader code
For AArch64 we will obviously require a different set of
primary and secondary boot loader code fragments. However currently
we hardcode the offsets into the loader code where we must write
the entrypoint and other data into arm_load_kernel(). This makes it
hard to substitute a different loader fragment, so switch to a more
flexible scheme where instead of a raw array of instructions we use
an array of (instruction, fixup-type) pairs that indicate which
words need special action or data written into them.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Message-id: 1385645602-18662-6-git-send-email-peter.mayd...@linaro.org
Reviewed-by: Christoffer Dall christoffer.d...@linaro.org
Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
hw/arm/boot.c | 152 +-
1 file changed, 107 insertions(+), 45 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 55d552f..0c05a64 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -20,15 +20,33 @@
#define KERNEL_ARGS_ADDR 0x100
#define KERNEL_LOAD_ADDR 0x0001
+typedef enum {
+FIXUP_NONE = 0, /* do nothing */
+FIXUP_TERMINATOR, /* end of insns */
+FIXUP_BOARDID,/* overwrite with board ID number */
+FIXUP_ARGPTR, /* overwrite with pointer to kernel args */
+FIXUP_ENTRYPOINT, /* overwrite with kernel entry point */
+FIXUP_GIC_CPU_IF, /* overwrite with GIC CPU interface address */
+FIXUP_BOOTREG,/* overwrite with boot register address */
+FIXUP_DSB,/* overwrite with correct DSB insn for cpu */
+FIXUP_MAX,
+} FixupType;
+
+typedef struct ARMInsnFixup {
+uint32_t insn;
+FixupType fixup;
+} ARMInsnFixup;
+
/* The worlds second smallest bootloader. Set r0-r2, then jump to kernel. */
-static uint32_t bootloader[] = {
- 0xe3a0, /* mov r0, #0 */
- 0xe59f1004, /* ldr r1, [pc, #4] */
- 0xe59f2004, /* ldr r2, [pc, #4] */
- 0xe59ff004, /* ldr pc, [pc, #4] */
- 0, /* Board ID */
- 0, /* Address of kernel args. Set by integratorcp_init. */
- 0 /* Kernel entry point. Set by integratorcp_init. */
+static const ARMInsnFixup bootloader[] = {
+{ 0xe3a0 }, /* mov r0, #0 */
+{ 0xe59f1004 }, /* ldr r1, [pc, #4] */
+{ 0xe59f2004 }, /* ldr r2, [pc, #4] */
+{ 0xe59ff004 }, /* ldr pc, [pc, #4] */
+{ 0, FIXUP_BOARDID },
+{ 0, FIXUP_ARGPTR },
+{ 0, FIXUP_ENTRYPOINT },
+{ 0, FIXUP_TERMINATOR }
};
/* Handling for secondary CPU boot in a multicore system.
@@ -48,39 +66,83 @@ static uint32_t bootloader[] = {
#define DSB_INSN 0xf57ff04f
#define CP15_DSB_INSN 0xee070f9a /* mcr cp15, 0, r0, c7, c10, 4 */
-static uint32_t smpboot[] = {
- 0xe59f2028, /* ldr r2, gic_cpu_if */
- 0xe59f0028, /* ldr r0, startaddr */
- 0xe3a01001, /* mov r1, #1 */
- 0xe5821000, /* str r1, [r2] - set GICC_CTLR.Enable */
- 0xe3a010ff, /* mov r1, #0xff */
- 0xe5821004, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
- DSB_INSN, /* dsb */
- 0xe320f003, /* wfi */
- 0xe5901000, /* ldr r1, [r0] */
- 0xe1110001, /* tst r1, r1 */
- 0x0afb, /* beq wfi */
- 0xe12fff11, /* bx r1 */
- 0, /* gic_cpu_if: base address of GIC CPU interface */
- 0 /* bootreg: Boot register address is held here */
+static const ARMInsnFixup smpboot[] = {
+{ 0xe59f2028 }, /* ldr r2, gic_cpu_if */
+{ 0xe59f0028 }, /* ldr r0, bootreg_addr */
+{ 0xe3a01001 }, /* mov r1, #1 */
+{ 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
+{ 0xe3a010ff }, /* mov r1, #0xff */
+{ 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
+{ 0, FIXUP_DSB }, /* dsb */
+{ 0xe320f003 }, /* wfi */
+{ 0xe5901000 }, /* ldr r1, [r0] */
+{ 0xe1110001 }, /* tst r1, r1 */
+{ 0x0afb }, /* beq wfi */
+{ 0xe12fff11 }, /* bx r1 */
+{ 0, FIXUP_GIC_CPU_IF }, /* gic_cpu_if: .word 0x */
+{ 0, FIXUP_BOOTREG }, /* bootreg_addr: .word 0x */
+{ 0, FIXUP_TERMINATOR }
};
+static void write_bootloader(const char *name, hwaddr addr,
+ const ARMInsnFixup *insns, uint32_t *fixupcontext)
+{
+/* Fix up the specified bootloader fragment and write it into
+ * guest memory using rom_add_blob_fixed(). fixupcontext is
+ * an array giving the values to write in for the fixup types
+ * which write a value into the code array.
+ */
+int i, len;
+uint32_t *code;
+
+len = 0;
+while (insns[len].fixup != FIXUP_TERMINATOR) {
+len++;
+}
+
+code = g_new0(uint32_t, len);
+
+for (i = 0; i len; i++) {
+uint32_t insn = insns[i].insn;
+FixupType fixup = insns[i].fixup;
+
+switch (fixup) {
+case FIXUP_NONE:
+break;
+case FIXUP_BOARDID:
+case FIXUP_ARGPTR:
+case FIXUP_ENTRYPOINT:
+case FIXUP_GIC_CPU_IF:
+case FIXUP_BOOTREG:
+case FIXUP_DSB:
+
Re: [Qemu-devel] [PATCH v2 4/5] monitor: add object-add (QMP) and object_add (HMP) command
Il 17/12/2013 12:54, Peter Crosthwaite ha scritto:
+visit_start_struct(opts_get_visitor(ov), dummy, NULL, NULL, 0, err);
+if (error_is_set(err)) {
+goto out_clean;
+}
So I have been thinking about repeated if(error_is_set(err)) { goto
foo; } and how to reduce its verbosity in situations like this. Can it
be solved with a simple semantic:
Error ** accepting APIs will perform no action if the Error **
argument is already set.
I think this is a case where verbosity ease of use.
In this case, the caller code is particularly simple, but what if I
needed to dereference the return value of the first called function, to
get the argument to the second? You would still need an if.
Paolo
In this case we would patch visit_foo to just return if
error_is_set(errp). Then we can delete these three LOC...
+
+qdict_del(pdict, qom-type);
+visit_type_str(opts_get_visitor(ov), type, qom-type, err);
+if (error_is_set(err)) {
+goto out_clean;
+}
+
and these
+qdict_del(pdict, id);
+visit_type_str(opts_get_visitor(ov), id, id, err);
+if (error_is_set(err)) {
+goto out_clean;
+}
and these
+
+object_add(type, id, pdict, opts_get_visitor(ov), err);
+if (error_is_set(err)) {
+goto out_clean;
+}
And leave just the last one which will catch and report the
first-occured error as desired.
Out of scope of the series of course.
Regards,
Peter
+visit_end_struct(opts_get_visitor(ov), err);
+if (error_is_set(err)) {
+qmp_object_del(id, NULL);
+}
+
+out_clean:
+opts_visitor_cleanup(ov);
+
+QDECREF(pdict);
+qemu_opts_del(opts);
+g_free(id);
+g_free(type);
+g_free(dummy);
+
+out:
+hmp_handle_error(mon, err);
+}
+
void hmp_getfd(Monitor *mon, const QDict *qdict)
{
const char *fdname = qdict_get_str(qdict, fdname);
diff --git a/hmp.h b/hmp.h
index 54cf71f..521449b 100644
--- a/hmp.h
+++ b/hmp.h
@@ -89,5 +89,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
+void hmp_object_add(Monitor *mon, const QDict *qdict);
#endif
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 10fa0e3..22d8b8f 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -93,6 +93,9 @@ int monitor_read_password(Monitor *mon, ReadLineFunc
*readline_func,
int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
+int qmp_object_add(Monitor *mon, const QDict *qdict, QObject **ret);
+void object_add(const char *type, const char *id, const QDict *qdict,
+Visitor *v, Error **errp);
AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
bool has_opaque, const char *opaque,
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 48a2a2e..29da211 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -13,6 +13,7 @@
#ifndef QAPI_VISITOR_CORE_H
#define QAPI_VISITOR_CORE_H
+#include qemu/typedefs.h
#include qapi/qmp/qobject.h
#include qapi/error.h
#include stdlib.h
@@ -26,8 +27,6 @@ typedef struct GenericList
struct GenericList *next;
} GenericList;
-typedef struct Visitor Visitor;
-
void visit_start_handle(Visitor *v, void **obj, const char *kind,
const char *name, Error **errp);
void visit_end_handle(Visitor *v, Error **errp);
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index a4c1b84..4524496 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -10,6 +10,8 @@ typedef struct QEMUBH QEMUBH;
typedef struct AioContext AioContext;
+typedef struct Visitor Visitor;
+
struct Monitor;
typedef struct Monitor Monitor;
typedef struct MigrationParams MigrationParams;
diff --git a/qapi-schema.json b/qapi-schema.json
index 83fa485..111463b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2759,6 +2759,26 @@
{ 'command': 'netdev_del', 'data': {'id': 'str'} }
##
+# @object-add:
+#
+# Create a QOM object.
+#
+# @qom-type: the class name for the object to be created
+#
+# @id: the name of the new object
+#
+# @props: #optional a dictionary of properties to be passed to the backend
+#
+# Returns: Nothing on success
+# Error if @qom-type is not a valid class name
+#
+# Since: 2.0
+##
+{ 'command': 'object-add',
+ 'data': {'qom-type': 'str', 'id': 'str', '*props': 'dict'},
+ 'gen': 'no' }
+
+##
# @NetdevNoneOptions
#
# Use it alone to have zero network devices.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index fba15cd..d09ea53 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -879,6 +879,33 @@
[Qemu-devel] [PATCH v2 3/7] target-arm: Add minimal KVM AArch64 support
From: Mian M. Hamayun m.hama...@virtualopensystems.com
Add the bare minimum set of functions needed for control of an
AArch64 KVM vcpu:
* CPU initialization
* minimal get/put register functions which only handle the
basic state of the CPU
Signed-off-by: Mian M. Hamayun m.hama...@virtualopensystems.com
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Message-id: 1385645602-18662-4-git-send-email-peter.mayd...@linaro.org
[PMM: significantly overhauled; most notably:
* code lives in kvm64.c rather than using #ifdefs
* support '-cpu host' rather than implicitly using whatever the
host's CPU is regardless of what the user requests
* fix bug attempting to get/set nonexistent X[31]
* fix bug writing 64 bit kernel pstate into uint32_t env field
]
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Christoffer Dall christoffer.d...@linaro.org
---
target-arm/Makefile.objs | 1 +
target-arm/kvm.c | 4 +
target-arm/kvm64.c | 204 +++
3 files changed, 209 insertions(+)
create mode 100644 target-arm/kvm64.c
diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index d1db77c..5493a4c 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -7,3 +7,4 @@ obj-y += neon_helper.o iwmmxt_helper.o
obj-y += gdbstub.o
obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o gdbstub64.o
obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
+obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 5cdb3b9..1d2688d 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -128,7 +128,11 @@ static void kvm_arm_host_cpu_initfn(Object *obj)
static const TypeInfo host_arm_cpu_type_info = {
.name = TYPE_ARM_HOST_CPU,
+#ifdef TARGET_AARCH64
+.parent = TYPE_AARCH64_CPU,
+#else
.parent = TYPE_ARM_CPU,
+#endif
.instance_init = kvm_arm_host_cpu_initfn,
.class_init = kvm_arm_host_cpu_class_init,
.class_size = sizeof(ARMHostCPUClass),
diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c
new file mode 100644
index 000..1b7ca90
--- /dev/null
+++ b/target-arm/kvm64.c
@@ -0,0 +1,204 @@
+/*
+ * ARM implementation of KVM hooks, 64 bit specific code
+ *
+ * Copyright Mian-M. Hamayun 2013, Virtual Open Systems
+ *
+ * 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 stdio.h
+#include sys/types.h
+#include sys/ioctl.h
+#include sys/mman.h
+
+#include linux/kvm.h
+
+#include qemu-common.h
+#include qemu/timer.h
+#include sysemu/sysemu.h
+#include sysemu/kvm.h
+#include kvm_arm.h
+#include cpu.h
+#include hw/arm/arm.h
+
+static inline void set_feature(uint64_t *features, int feature)
+{
+*features |= 1ULL feature;
+}
+
+bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
+{
+/* Identify the feature bits corresponding to the host CPU, and
+ * fill out the ARMHostCPUClass fields accordingly. To do this
+ * we have to create a scratch VM, create a single CPU inside it,
+ * and then query that CPU for the relevant ID registers.
+ * For AArch64 we currently don't care about ID registers at
+ * all; we just want to know the CPU type.
+ */
+int fdarray[3];
+uint64_t features = 0;
+/* Old kernels may not know about the PREFERRED_TARGET ioctl: however
+ * we know these will only support creating one kind of guest CPU,
+ * which is its preferred CPU type. Fortunately these old kernels
+ * support only a very limited number of CPUs.
+ */
+static const uint32_t cpus_to_try[] = {
+KVM_ARM_TARGET_AEM_V8,
+KVM_ARM_TARGET_FOUNDATION_V8,
+KVM_ARM_TARGET_CORTEX_A57,
+QEMU_KVM_ARM_TARGET_NONE
+};
+struct kvm_vcpu_init init;
+
+if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, init)) {
+return false;
+}
+
+ahcc-target = init.target;
+ahcc-dtb_compatible = arm,arm-v8;
+
+kvm_arm_destroy_scratch_host_vcpu(fdarray);
+
+ /* We can assume any KVM supporting CPU is at least a v8
+ * with VFPv4+Neon; this in turn implies most of the other
+ * feature bits.
+ */
+set_feature(features, ARM_FEATURE_V8);
+set_feature(features, ARM_FEATURE_VFP4);
+set_feature(features, ARM_FEATURE_NEON);
+set_feature(features, ARM_FEATURE_AARCH64);
+
+ahcc-features = features;
+
+return true;
+}
+
+int kvm_arch_init_vcpu(CPUState *cs)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+struct kvm_vcpu_init init;
+int ret;
+
+if (cpu-kvm_target == QEMU_KVM_ARM_TARGET_NONE ||
+!arm_feature(cpu-env, ARM_FEATURE_AARCH64)) {
+fprintf(stderr, KVM is not supported for this guest CPU type\n);
+return -EINVAL;
+}
+
+init.target = cpu-kvm_target;
+memset(init.features, 0, sizeof(init.features));
+if (cpu-start_powered_off) {
+init.features[0] = 1
[Qemu-devel] [PATCH v2 0/7] target-arm: Support AArch64 KVM
This patchset adds support for basic AArch64 KVM VM control; it's based on current master. This is a quick resend with the very minor nits Christoffer pointed out fixed; I'm planning to put it into a target-arm pullreq in the next day or two. This patch series supports: * 64 bit KVM VM control * SMP and UP * PSCI boot of secondary CPUs It doesn't support: * migration * reset (partly because there's no way to reset a mach-virt system yet) * anything except -cpu host * debugging the VM via qemu gdbstub * running 32 bit VMs on a 64 bit system [Mian's patchset includes support for that but I have left it out for the moment because it needs more thought about UI and so on] Changes v1-v2: * improved a couple of comments * compat string for 64 bit is arm,arm-v8, not -v7 * removed superfluous include of usb.mak and pci.mak from config Mian M. Hamayun (2): target-arm: Add minimal KVM AArch64 support hw/arm/boot: Add boot support for AArch64 processor Peter Maydell (5): target-arm/kvm: Split 32 bit only code into its own file target-arm: Clean up handling of AArch64 PSTATE configure: Enable KVM for aarch64 host/target combination hw/arm/boot: Allow easier swapping in of different loader code default-configs: Add config for aarch64-softmmu configure | 2 +- default-configs/aarch64-softmmu.mak | 6 + hw/arm/boot.c | 193 ++ linux-user/signal.c | 6 +- target-arm/Makefile.objs| 2 + target-arm/cpu.c| 6 + target-arm/cpu.h| 70 - target-arm/gdbstub64.c | 4 +- target-arm/kvm.c| 495 +- target-arm/kvm32.c | 515 target-arm/kvm64.c | 204 ++ target-arm/translate-a64.c | 12 +- 12 files changed, 954 insertions(+), 561 deletions(-) create mode 100644 default-configs/aarch64-softmmu.mak create mode 100644 target-arm/kvm32.c create mode 100644 target-arm/kvm64.c -- 1.8.5
[Qemu-devel] [PATCH v2 4/7] configure: Enable KVM for aarch64 host/target combination
Enable KVM if the host and target CPU are both aarch64. Note that host aarch64 + target arm is not valid for KVM acceleration: the 64 bit kernel does not support the ioctl interface for 32 bit CPUs. 32 bit VMs on 64 bit hosts need to be created using the 64 bit ioctl interface; when QEMU supports this it will be on the arch64-softmmu target with a -cpu parameter for a 32 bit CPU, which is still an aarch64/aarch64 combination as far as configure is concerned. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Message-id: 1385645602-18662-5-git-send-email-peter.mayd...@linaro.org Reviewed-by: Christoffer Dall christoffer.d...@linaro.org --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index edfea95..02c94e2 100755 --- a/configure +++ b/configure @@ -4550,7 +4550,7 @@ case $target_name in *) esac case $target_name in - arm|i386|x86_64|ppcemb|ppc|ppc64|s390x) + aarch64|arm|i386|x86_64|ppcemb|ppc|ppc64|s390x) # Make sure the target and host cpus are compatible if test $kvm = yes -a $target_softmmu = yes -a \ \( $target_name = $cpu -o \ -- 1.8.5
[Qemu-devel] [PATCH v2 1/7] target-arm/kvm: Split 32 bit only code into its own file
Split ARM KVM support code which is 32 bit specific out into its
own file, which we only compile on 32 bit hosts. This will give
us a place to add the 64 bit support code without adding lots of
ifdefs to kvm.c.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Message-id: 1385645602-18662-2-git-send-email-peter.mayd...@linaro.org
Reviewed-by: Christoffer Dall christoffer.d...@linaro.org
---
target-arm/Makefile.objs | 1 +
target-arm/kvm.c | 491
target-arm/kvm32.c | 515 +++
3 files changed, 516 insertions(+), 491 deletions(-)
create mode 100644 target-arm/kvm32.c
diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index 356fbfc..d1db77c 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -6,3 +6,4 @@ obj-y += translate.o op_helper.o helper.o cpu.o
obj-y += neon_helper.o iwmmxt_helper.o
obj-y += gdbstub.o
obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o gdbstub64.o
+obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index f865dac..5cdb3b9 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -100,120 +100,6 @@ void kvm_arm_destroy_scratch_host_vcpu(int *fdarray)
}
}
-static inline void set_feature(uint64_t *features, int feature)
-{
-*features |= 1ULL feature;
-}
-
-bool kvm_arm_get_host_cpu_features(ARMHostCPUClass *ahcc)
-{
-/* Identify the feature bits corresponding to the host CPU, and
- * fill out the ARMHostCPUClass fields accordingly. To do this
- * we have to create a scratch VM, create a single CPU inside it,
- * and then query that CPU for the relevant ID registers.
- */
-int i, ret, fdarray[3];
-uint32_t midr, id_pfr0, id_isar0, mvfr1;
-uint64_t features = 0;
-/* Old kernels may not know about the PREFERRED_TARGET ioctl: however
- * we know these will only support creating one kind of guest CPU,
- * which is its preferred CPU type.
- */
-static const uint32_t cpus_to_try[] = {
-QEMU_KVM_ARM_TARGET_CORTEX_A15,
-QEMU_KVM_ARM_TARGET_NONE
-};
-struct kvm_vcpu_init init;
-struct kvm_one_reg idregs[] = {
-{
-.id = KVM_REG_ARM | KVM_REG_SIZE_U32
-| ENCODE_CP_REG(15, 0, 0, 0, 0, 0),
-.addr = (uintptr_t)midr,
-},
-{
-.id = KVM_REG_ARM | KVM_REG_SIZE_U32
-| ENCODE_CP_REG(15, 0, 0, 1, 0, 0),
-.addr = (uintptr_t)id_pfr0,
-},
-{
-.id = KVM_REG_ARM | KVM_REG_SIZE_U32
-| ENCODE_CP_REG(15, 0, 0, 2, 0, 0),
-.addr = (uintptr_t)id_isar0,
-},
-{
-.id = KVM_REG_ARM | KVM_REG_SIZE_U32
-| KVM_REG_ARM_VFP | KVM_REG_ARM_VFP_MVFR1,
-.addr = (uintptr_t)mvfr1,
-},
-};
-
-if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, init)) {
-return false;
-}
-
-ahcc-target = init.target;
-
-/* This is not strictly blessed by the device tree binding docs yet,
- * but in practice the kernel does not care about this string so
- * there is no point maintaining an KVM_ARM_TARGET_* - string table.
- */
-ahcc-dtb_compatible = arm,arm-v7;
-
-for (i = 0; i ARRAY_SIZE(idregs); i++) {
-ret = ioctl(fdarray[2], KVM_GET_ONE_REG, idregs[i]);
-if (ret) {
-break;
-}
-}
-
-kvm_arm_destroy_scratch_host_vcpu(fdarray);
-
-if (ret) {
-return false;
-}
-
-/* Now we've retrieved all the register information we can
- * set the feature bits based on the ID register fields.
- * We can assume any KVM supporting CPU is at least a v7
- * with VFPv3, LPAE and the generic timers; this in turn implies
- * most of the other feature bits, but a few must be tested.
- */
-set_feature(features, ARM_FEATURE_V7);
-set_feature(features, ARM_FEATURE_VFP3);
-set_feature(features, ARM_FEATURE_LPAE);
-set_feature(features, ARM_FEATURE_GENERIC_TIMER);
-
-switch (extract32(id_isar0, 24, 4)) {
-case 1:
-set_feature(features, ARM_FEATURE_THUMB_DIV);
-break;
-case 2:
-set_feature(features, ARM_FEATURE_ARM_DIV);
-set_feature(features, ARM_FEATURE_THUMB_DIV);
-break;
-default:
-break;
-}
-
-if (extract32(id_pfr0, 12, 4) == 1) {
-set_feature(features, ARM_FEATURE_THUMB2EE);
-}
-if (extract32(mvfr1, 20, 4) == 1) {
-set_feature(features, ARM_FEATURE_VFP_FP16);
-}
-if (extract32(mvfr1, 12, 4) == 1) {
-set_feature(features, ARM_FEATURE_NEON);
-}
-if (extract32(mvfr1, 28, 4) == 1) {
-/* FMAC support implies VFPv4 */
-set_feature(features, ARM_FEATURE_VFP4);
-}
-
-ahcc-features = features;
-
-return true;
-}
-
static void kvm_arm_host_cpu_class_init(ObjectClass *oc, void
[Qemu-devel] [PATCH v2 2/7] target-arm: Clean up handling of AArch64 PSTATE
The env-pstate field is a little odd since it doesn't strictly
speaking represent an architectural register. However it's convenient
for QEMU to use it to hold the various PSTATE architectural bits
in the same format the architecture specifies for SPSR registers
(since this is the same format the kernel uses for signal handlers
and the KVM register). Add some structure to how we deal with it:
* document what env-pstate is
* add some #defines for various bits in it
* add helpers for reading/writing it taking account of caching
of NZCV, and use them where appropriate
* reset it on startup
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Message-id: 1385645602-18662-3-git-send-email-peter.mayd...@linaro.org
Reviewed-by: Christoffer Dall christoffer.d...@linaro.org
---
linux-user/signal.c| 6 ++--
target-arm/cpu.c | 6
target-arm/cpu.h | 70 ++
target-arm/gdbstub64.c | 4 +--
target-arm/translate-a64.c | 12
5 files changed, 78 insertions(+), 20 deletions(-)
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 7751c47..4e7148a 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1171,7 +1171,7 @@ static int target_setup_sigframe(struct
target_rt_sigframe *sf,
}
__put_user(env-xregs[31], sf-uc.tuc_mcontext.sp);
__put_user(env-pc, sf-uc.tuc_mcontext.pc);
-__put_user(env-pstate, sf-uc.tuc_mcontext.pstate);
+__put_user(pstate_read(env), sf-uc.tuc_mcontext.pstate);
__put_user(/*current-thread.fault_address*/ 0,
sf-uc.tuc_mcontext.fault_address);
@@ -1210,6 +1210,7 @@ static int target_restore_sigframe(CPUARMState *env,
struct target_aux_context *aux =
(struct target_aux_context *)sf-uc.tuc_mcontext.__reserved;
uint32_t magic, size;
+uint64_t pstate;
target_to_host_sigset(set, sf-uc.tuc_sigmask);
sigprocmask(SIG_SETMASK, set, NULL);
@@ -1220,7 +1221,8 @@ static int target_restore_sigframe(CPUARMState *env,
__get_user(env-xregs[31], sf-uc.tuc_mcontext.sp);
__get_user(env-pc, sf-uc.tuc_mcontext.pc);
-__get_user(env-pstate, sf-uc.tuc_mcontext.pstate);
+__get_user(pstate, sf-uc.tuc_mcontext.pstate);
+pstate_write(env, pstate);
__get_user(magic, aux-fpsimd.head.magic);
__get_user(size, aux-fpsimd.head.size);
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 0635e78..42057ad 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -88,6 +88,12 @@ static void arm_cpu_reset(CPUState *s)
if (arm_feature(env, ARM_FEATURE_AARCH64)) {
/* 64 bit CPUs always start in 64 bit mode */
env-aarch64 = 1;
+#if defined(CONFIG_USER_ONLY)
+env-pstate = PSTATE_MODE_EL0t;
+#else
+env-pstate = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F
+| PSTATE_MODE_EL1h;
+#endif
}
#if defined(CONFIG_USER_ONLY)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c3f007f..d15fdcd 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -113,8 +113,15 @@ typedef struct CPUARMState {
/* Regs for A64 mode. */
uint64_t xregs[32];
uint64_t pc;
-/* TODO: pstate doesn't correspond to an architectural register;
- * it would be better modelled as the underlying fields.
+/* PSTATE isn't an architectural register for ARMv8. However, it is
+ * convenient for us to assemble the underlying state into a 32 bit format
+ * identical to the architectural format used for the SPSR. (This is also
+ * what the Linux kernel's 'pstate' field in signal handlers and KVM's
+ * 'pstate' register are.) Of the PSTATE bits:
+ * NZCV are kept in the split out env-CF/VF/NF/ZF, (which have the same
+ *semantics as for AArch32, as described in the comments on each field)
+ * nRW (also known as M[4]) is kept, inverted, in env-aarch64
+ * all other bits are stored in their correct places in env-pstate
*/
uint32_t pstate;
uint32_t aarch64; /* 1 if CPU is in aarch64 state; inverse of PSTATE.nRW */
@@ -309,15 +316,6 @@ static inline bool is_a64(CPUARMState *env)
return env-aarch64;
}
-#define PSTATE_N_SHIFT 3
-#define PSTATE_N (1 PSTATE_N_SHIFT)
-#define PSTATE_Z_SHIFT 2
-#define PSTATE_Z (1 PSTATE_Z_SHIFT)
-#define PSTATE_C_SHIFT 1
-#define PSTATE_C (1 PSTATE_C_SHIFT)
-#define PSTATE_V_SHIFT 0
-#define PSTATE_V (1 PSTATE_V_SHIFT)
-
/* you can call this signal handler from your SIGBUS and SIGSEGV
signal handlers to inform the virtual CPU of exceptions. non zero
is returned if the signal was handled by the virtual CPU. */
@@ -352,6 +350,56 @@ int cpu_arm_handle_mmu_fault (CPUARMState *env,
target_ulong address, int rw,
/* Execution state bits. MRS read as zero, MSR writes ignored. */
#define CPSR_EXEC (CPSR_T | CPSR_IT | CPSR_J)
+/* Bit definitions for ARMv8 SPSR (PSTATE) format.
+ * Only these are valid when in AArch64 mode; in
+ * AArch32 mode SPSRs are basically CPSR-format.
+ */
Re: [Qemu-devel] [PATCH 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API
Il 17/12/2013 00:26, Anthony Liguori ha scritto: Sharing hot plug code is a good thing. Making hotplug a qdev-level concept seems like a bad thing to me. Can you explain what you mean? The series is a net add of code so I don't think we're winning anything by generalizing here. Any generalization that's used just once will be a net add of code (and this code will be reused by SCSI and x86 memory hotplug at least; perhaps x86 CPU hotplug too). Any generalization that requires some boilerplate code will be a net add of code, too. QEMU being written in C, we unfortunately cannot avoid that. So I don't think that lines of code are a good metric. Paolo Is there a use-case this enables that isn't possible today? Regards, Anthony Liguori Patches 8-11 are should be merged as one and are split only for simplifying review (they compile fine but PCI hotplug is broken until the last patch is applyed). git tree for testing: https://github.com/imammedo/qemu/commits/hotplug_dev_inf_v3 tested only ACPI and PCIE hotplug. Hervé Poussineau (1): qom: detect bad reentrance during object_class_foreach Igor Mammedov (9): define hotplug interface qdev: add to BusState hotplug-handler link qdev: add hotpluggable property to Device hw/acpi: move typeinfo to the file end qdev:pci: refactor PCIDevice to use generic hotpluggable property acpi/piix4pm: convert ACPI PCI hotplug to use hotplug-handler API pci/shpc: convert SHPC hotplug to use hotplug-handler API pci/pcie: convert PCIE hotplug to use hotplug-handler API hw/pci: switch to a generic hotplug handling for PCIDevice Paolo Bonzini (1): qom: do not register interface types in the type table hw/acpi/piix4.c| 151 ++--- hw/core/Makefile.objs | 1 + hw/core/hotplug.c | 48 + hw/core/qdev.c | 50 -- hw/display/cirrus_vga.c| 2 +- hw/display/qxl.c | 2 +- hw/display/vga-pci.c | 2 +- hw/display/vmware_vga.c| 2 +- hw/i386/acpi-build.c | 6 +- hw/ide/piix.c | 4 +- hw/isa/piix4.c | 2 +- hw/pci-bridge/pci_bridge_dev.c | 9 +++ hw/pci-host/piix.c | 6 +- hw/pci/pci.c | 40 +-- hw/pci/pcie.c | 73 +--- hw/pci/pcie_port.c | 8 +++ hw/pci/shpc.c | 133 +++- hw/usb/hcd-ehci-pci.c | 2 +- hw/usb/hcd-ohci.c | 2 +- hw/usb/hcd-uhci.c | 2 +- hw/usb/hcd-xhci.c | 2 +- include/hw/hotplug.h | 75 include/hw/pci/pci.h | 13 include/hw/pci/pci_bus.h | 2 - include/hw/pci/pcie.h | 5 ++ include/hw/pci/shpc.h | 8 +++ include/hw/qdev-core.h | 8 +++ qom/object.c | 17 - 28 files changed, 455 insertions(+), 220 deletions(-) create mode 100644 hw/core/hotplug.c create mode 100644 include/hw/hotplug.h -- 1.8.3.1
Re: [Qemu-devel] [PATCH v2 4/5] monitor: add object-add (QMP) and object_add (HMP) command
On Tue, Dec 17, 2013 at 10:24 PM, Paolo Bonzini pbonz...@redhat.com wrote:
Il 17/12/2013 12:54, Peter Crosthwaite ha scritto:
+visit_start_struct(opts_get_visitor(ov), dummy, NULL, NULL, 0, err);
+if (error_is_set(err)) {
+goto out_clean;
+}
So I have been thinking about repeated if(error_is_set(err)) { goto
foo; } and how to reduce its verbosity in situations like this. Can it
be solved with a simple semantic:
Error ** accepting APIs will perform no action if the Error **
argument is already set.
I think this is a case where verbosity ease of use.
In this case, the caller code is particularly simple, but what if I
needed to dereference the return value of the first called function, to
get the argument to the second? You would still need an if.
Yes thats right. This isn't going to work universally and callers will
always have the responsibility of knowing whether they can continue or
not. But it will help a lot for repetitive collections of similar
independent functions calls. The ultimate example is probably the
device tree API calls in hw/ppc/e500.c.
I want to patch the device tree API to be nice and Error**ified (for
my own future reasons) but I sure don't want to have to patch e500 to
check every qemu_devtree_foo API call with these 3 LOC. TBH i'll
probably just preserve current behavior using error_abort in next rev
of that series, but it should be possible to do less verbose
caller-customized collective error handling in some way.
Regards,
Peter
Re: [Qemu-devel] [PATCH v2 7/7] default-configs: Add config for aarch64-softmmu
On Tue, Dec 17, 2013 at 10:15 PM, Peter Maydell peter.mayd...@linaro.org wrote: Add a config for aarch64-softmmu; this enables building of this target. The resulting executable doesn't know about any 64 bit CPUs, but all the 32 bit CPUs and board models work. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Message-id: 1385645602-18662-8-git-send-email-peter.mayd...@linaro.org --- default-configs/aarch64-softmmu.mak | 6 ++ 1 file changed, 6 insertions(+) create mode 100644 default-configs/aarch64-softmmu.mak diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak new file mode 100644 index 000..6d3b5c7 --- /dev/null +++ b/default-configs/aarch64-softmmu.mak @@ -0,0 +1,6 @@ +# Default configuration for aarch64-softmmu + +# We support all the 32 bit boards so need all their config +include arm-softmmu.mak + +# Currently no 64-bit specific config requirements 1.8.5
Re: [Qemu-devel] [PATCH qom-cpu 00/16 v10] target-i386: convert CPU features into properties
On Mon, 16 Dec 2013 16:26:55 -0200 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Dec 16, 2013 at 04:01:05PM +0100, Igor Mammedov wrote: On Sun, 15 Dec 2013 23:50:47 +0100 Andreas Färber afaer...@suse.de wrote: Am 27.11.2013 23:28, schrieb Igor Mammedov: Igor Mammedov (16): target-i386: cleanup 'foo' feature handling' target-i386: cleanup 'foo=val' feature handling Thanks, I've queued these on qom-cpu-next: https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next target-i386: cpu: convert 'level' to static property target-i386: cpu: convert 'xlevel' to static property target-i386: cpu: convert 'family' to static property target-i386: cpu: convert 'model' to static property target-i386: cpu: convert 'stepping' to static property target-i386: cpu: convert 'vendor' to static property target-i386: cpu: convert 'model-id' to static property target-i386: cpu: convert 'tsc-frequency' to static property But I still don't see the utility of this conversion after all the discussions we've had... :( The below patches seem to only operate on CPUID bits, which get added as properties in the following patch. Above patches are there to simplify/unify current codebase. For example, level xlevel replace custom setters/getters with static property onliners. The rest are making initfn more readable, not to mention that they become visible in HMP along with the rest features wich is nice for consistent behavior even is we do not care about HMP. Otherwise there is not much difference between dynamic vs static anymore, so this patches could be dropped, however with them ,I think, code is a bit cleaner. I agree with Igor, here. We don't strictly need to make those properties static anymore, but it is still useful to do it, because it makes the instrospection information visible to other code (inside and eventually outside QEMU) before a CPU object gets created, and to me it really looks simpler than registering the properties at instance_init(). target-i386: set [+-]feature using static properties qdev: introduce qdev_prop_find_bit() target-i386: use static properties in check_features_against_host() to print CPUID feature names target-i386: use static properties to list CPUID features I am reading too many occurrences of static properties above that should IMO just be properties. You got permission to use a name-based in current code base static properties are almost the same as dynamic ones, it's just a more abbreviated version of dynamic ones with static defaults, range checking, bit handling ... So I don't see why more verbose dynamic properties SHOULD be used, where the same code could be written more compact with static properties. In the specific case of the feature-bit properties, I am more inclined to agree with Andreas: is making the properties static worth the extra code complexity? qdev_prop_find_bit() is the only complexity with static bit properties, otherwise feature bits as static props are much more maintainable and self descriptive then feature name arrays. We still don't have a QMP command to list all the properties supported by a DeviceClass, do we? If we had it, having static properties would be much more useful. Today they are not much better than simple dynamic properties registered at instance_init(). with static properties there won't be need to rewrite this twice, once command becomes available. scheme to iterate over feat-* properties, so why are you still iterating over static properties with a helper searching for offsets rather than QOM properties with feat- prefix? Either we need that scheme for automated processing as I understood you, then we should be consequent in using it, or we don't. And I would prefer to keep these mappings in x86 code rather than messing in generic device infrastructure and iterating over *all* properties in your qdev_prop_find_bit() and making generally available new QDEV_* macros QDEV_PROP_FOREACH() and QDEV_CLASS_FOREACH(). this patch was, was on list for more than half a year without any complaints/reviews. I don't have ICR for that time already, but if I recall correctly something like this was suggested by Anthony to resolve problem of mapping bit fields to names. If there is a more simple elegant way to do it I'd like to hear more concrete suggestion(s) how it should be vs just messing verdict with which I don't agree btw. The need to iterate over feature properties was something that I considered okay only because we were forced to use static properties. Now we moved global property handling to instance_post_init and we don't really need it anymore. One more simple way to do it is to simply keep the existing feature_name[] arrays and use them to register/lookup feature property names. I would prefer that to
Re: [Qemu-devel] [PATCH v2 6/7] hw/arm/boot: Add boot support for AArch64 processor
On Tue, Dec 17, 2013 at 10:15 PM, Peter Maydell
peter.mayd...@linaro.org wrote:
From: Mian M. Hamayun m.hama...@virtualopensystems.com
This commit adds support for booting a single AArch64 CPU by setting
appropriate registers. The bootloader includes placehoders for Board-ID
placeholders
that are used to implement uniform indexing across different bootloaders.
Signed-off-by: Mian M. Hamayun m.hama...@virtualopensystems.com
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Message-id: 1385645602-18662-7-git-send-email-peter.mayd...@linaro.org
[PMM:
* updated to use ARMInsnFixup style bootloader fragments
* dropped virt.c additions
* use runtime checks for is this an AArch64 core rather than ifdefs
* drop some unnecessary setting of registers in reset hook
]
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Christoffer Dall christoffer.d...@linaro.org
---
hw/arm/boot.c | 43 ++-
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 0c05a64..90e9534 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -17,8 +17,13 @@
#include sysemu/device_tree.h
#include qemu/config-file.h
+/* Kernel boot protocol is specified in the kernel docs
+ * Documentation/arm/Booting and Documentation/arm64/booting.txt
+ * They have different preferred image load offsets from system RAM base.
+ */
#define KERNEL_ARGS_ADDR 0x100
#define KERNEL_LOAD_ADDR 0x0001
So out of context, but I have been booting an ARMv7 multi defconfig on
a few qemu platforms recently (spec. allwinner, highbank and zynq) and
I found I had to patch this to 0x8000 due to some image alignment
expectations of head.S. Could we patch this to 0x8000 - is there this
same sense of preferred image offset in KERNEL32_LOAD_ADDR?
+#define KERNEL64_LOAD_ADDR 0x0008
typedef enum {
FIXUP_NONE = 0, /* do nothing */
@@ -37,6 +42,20 @@ typedef struct ARMInsnFixup {
FixupType fixup;
} ARMInsnFixup;
+static const ARMInsnFixup bootloader_aarch64[] = {
+{ 0x58c0 }, /* ldr x0, arg ; Load the lower 32-bits of DTB */
+{ 0xaa1f03e1 }, /* mov x1, xzr */
+{ 0xaa1f03e2 }, /* mov x2, xzr */
+{ 0xaa1f03e3 }, /* mov x3, xzr */
+{ 0x5884 }, /* ldr x4, entry ; Load the lower 32-bits of kernel
entry */
+{ 0xd61f0080 }, /* br x4 ; Jump to the kernel entry point */
+{ 0, FIXUP_ARGPTR }, /* arg: .word @DTB Lower 32-bits */
+{ 0 }, /* .word @DTB Higher 32-bits */
+{ 0, FIXUP_ENTRYPOINT }, /* entry: .word @Kernel Entry Lower 32-bits */
+{ 0 }, /* .word @Kernel Entry Higher 32-bits */
+{ 0, FIXUP_TERMINATOR }
+};
+
/* The worlds second smallest bootloader. Set r0-r2, then jump to kernel.
*/
static const ARMInsnFixup bootloader[] = {
{ 0xe3a0 }, /* mov r0, #0 */
@@ -396,7 +415,12 @@ static void do_cpu_reset(void *opaque)
env-thumb = info-entry 1;
} else {
if (CPU(cpu) == first_cpu) {
-env-regs[15] = info-loader_start;
+if (env-aarch64) {
Curious, why does this 'if' directly deref env, while the one below
(for primary_loader selection) uses ARM_FEATURE?
Regards,
Peter
+env-pc = info-loader_start;
+} else {
+env-regs[15] = info-loader_start;
+}
+
if (!info-dtb_filename) {
if (old_param) {
set_kernel_args_old(info);
@@ -418,8 +442,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
int initrd_size;
int is_linux = 0;
uint64_t elf_entry;
-hwaddr entry;
+hwaddr entry, kernel_load_offset;
int big_endian;
+static const ARMInsnFixup *primary_loader;
/* Load the kernel. */
if (!info-kernel_filename) {
@@ -429,6 +454,14 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
return;
}
+if (arm_feature(cpu-env, ARM_FEATURE_AARCH64)) {
+primary_loader = bootloader_aarch64;
+kernel_load_offset = KERNEL64_LOAD_ADDR;
+} else {
+primary_loader = bootloader;
+kernel_load_offset = KERNEL_LOAD_ADDR;
+}
+
info-dtb_filename = qemu_opt_get(qemu_get_machine_opts(), dtb);
if (!info-secondary_cpu_reset_hook) {
@@ -469,9 +502,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
is_linux);
}
if (kernel_size 0) {
-entry = info-loader_start + KERNEL_LOAD_ADDR;
+entry = info-loader_start + kernel_load_offset;
kernel_size = load_image_targphys(info-kernel_filename, entry,
- info-ram_size - KERNEL_LOAD_ADDR);
+ info-ram_size -
kernel_load_offset);
is_linux = 1;
}
if (kernel_size 0) {
Re: [Qemu-devel] [PATCH v2 4/5] monitor: add object-add (QMP) and object_add (HMP) command
Peter Crosthwaite peter.crosthwa...@xilinx.com writes:
On Tue, Dec 17, 2013 at 10:24 PM, Paolo Bonzini pbonz...@redhat.com wrote:
Il 17/12/2013 12:54, Peter Crosthwaite ha scritto:
+visit_start_struct(opts_get_visitor(ov), dummy, NULL, NULL, 0, err);
+if (error_is_set(err)) {
+goto out_clean;
+}
So I have been thinking about repeated if(error_is_set(err)) { goto
foo; } and how to reduce its verbosity in situations like this. Can it
be solved with a simple semantic:
Error ** accepting APIs will perform no action if the Error **
argument is already set.
I think this is a case where verbosity ease of use.
In this case, the caller code is particularly simple, but what if I
needed to dereference the return value of the first called function, to
get the argument to the second? You would still need an if.
Yes thats right. This isn't going to work universally and callers will
always have the responsibility of knowing whether they can continue or
not. But it will help a lot for repetitive collections of similar
independent functions calls. The ultimate example is probably the
device tree API calls in hw/ppc/e500.c.
I want to patch the device tree API to be nice and Error**ified (for
my own future reasons) but I sure don't want to have to patch e500 to
check every qemu_devtree_foo API call with these 3 LOC. TBH i'll
probably just preserve current behavior using error_abort in next rev
of that series, but it should be possible to do less verbose
caller-customized collective error handling in some way.
error_set() friends currently have a errp doesn't contain an error
already precondition:
if (errp == NULL) {
return;
}
assert(*errp == NULL);
You could change the function contracts to ignore additional errors.
Theoretical drawback: in situations where that isn't intended,
programming errors no longer get caught. If people actually care for
that enough to veto your change, you can try adding new functions for
use when it is intended.
Re: [Qemu-devel] [PATCH v2 6/7] hw/arm/boot: Add boot support for AArch64 processor
On 17 December 2013 13:04, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
On Tue, Dec 17, 2013 at 10:15 PM, Peter Maydell
peter.mayd...@linaro.org wrote:
From: Mian M. Hamayun m.hama...@virtualopensystems.com
This commit adds support for booting a single AArch64 CPU by setting
appropriate registers. The bootloader includes placehoders for Board-ID
placeholders
Fixed (not going to respin just for that though :-))
that are used to implement uniform indexing across different bootloaders.
Signed-off-by: Mian M. Hamayun m.hama...@virtualopensystems.com
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Message-id: 1385645602-18662-7-git-send-email-peter.mayd...@linaro.org
[PMM:
* updated to use ARMInsnFixup style bootloader fragments
* dropped virt.c additions
* use runtime checks for is this an AArch64 core rather than ifdefs
* drop some unnecessary setting of registers in reset hook
]
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Christoffer Dall christoffer.d...@linaro.org
---
hw/arm/boot.c | 43 ++-
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 0c05a64..90e9534 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -17,8 +17,13 @@
#include sysemu/device_tree.h
#include qemu/config-file.h
+/* Kernel boot protocol is specified in the kernel docs
+ * Documentation/arm/Booting and Documentation/arm64/booting.txt
+ * They have different preferred image load offsets from system RAM base.
+ */
#define KERNEL_ARGS_ADDR 0x100
#define KERNEL_LOAD_ADDR 0x0001
So out of context, but I have been booting an ARMv7 multi defconfig on
a few qemu platforms recently (spec. allwinner, highbank and zynq) and
I found I had to patch this to 0x8000 due to some image alignment
expectations of head.S. Could we patch this to 0x8000 - is there this
same sense of preferred image offset in KERNEL32_LOAD_ADDR?
Hmm. Are you booting non-zImage kernels? The booting doc says
# When booting a raw (non-zImage) kernel the constraints are tighter.
# In this case the kernel must be loaded at an offset into system equal
# to TEXT_OFFSET - PAGE_OFFSET.
so maybe that's what we're not getting right there? I practically
always use a zImage so I wouldn't ever see that.
In any case if you can nail down whether that's the problem and
submit a patch that would be good, though we'll have to test
on a bunch of boards that we haven't broken something else.
(I don't particularly want to change to 0x8000 because it happens
to work, but the same change with a rationale attached is fine :-))
+#define KERNEL64_LOAD_ADDR 0x0008
typedef enum {
FIXUP_NONE = 0, /* do nothing */
@@ -37,6 +42,20 @@ typedef struct ARMInsnFixup {
FixupType fixup;
} ARMInsnFixup;
+static const ARMInsnFixup bootloader_aarch64[] = {
+{ 0x58c0 }, /* ldr x0, arg ; Load the lower 32-bits of DTB */
+{ 0xaa1f03e1 }, /* mov x1, xzr */
+{ 0xaa1f03e2 }, /* mov x2, xzr */
+{ 0xaa1f03e3 }, /* mov x3, xzr */
+{ 0x5884 }, /* ldr x4, entry ; Load the lower 32-bits of kernel
entry */
+{ 0xd61f0080 }, /* br x4 ; Jump to the kernel entry point */
+{ 0, FIXUP_ARGPTR }, /* arg: .word @DTB Lower 32-bits */
+{ 0 }, /* .word @DTB Higher 32-bits */
+{ 0, FIXUP_ENTRYPOINT }, /* entry: .word @Kernel Entry Lower 32-bits */
+{ 0 }, /* .word @Kernel Entry Higher 32-bits */
+{ 0, FIXUP_TERMINATOR }
+};
+
/* The worlds second smallest bootloader. Set r0-r2, then jump to kernel.
*/
static const ARMInsnFixup bootloader[] = {
{ 0xe3a0 }, /* mov r0, #0 */
@@ -396,7 +415,12 @@ static void do_cpu_reset(void *opaque)
env-thumb = info-entry 1;
} else {
if (CPU(cpu) == first_cpu) {
-env-regs[15] = info-loader_start;
+if (env-aarch64) {
Curious, why does this 'if' directly deref env, while the one below
(for primary_loader selection) uses ARM_FEATURE?
When we're picking the bootloader the CPU isn't started, so
its internal state (including the aarch64 flag, which is a reflection
of PSTATE.nRW) isn't valid. We want to pick a bootloader on
the basis of if this CPU can do AArch64 then use the AArch64
loader. This code however runs after reset and so we can say
if you're currently in AArch64 mode then this is how the PC is
set, otherwise like this.
I'm mulling a cleanup at some point that makes the AArch32
also use env-pc, at which point this if() will just evaporate anyway.
thanks
-- PMM
[Qemu-devel] [PATCH 0/8] s390 sigp, chsc and diag bugfixes/cleanups
Alex, this is mostly bugfixes and cleanup patches, except for Patch 4/8 which implements a SIGP order code for cpu start. regards Jens Cornelia Huck (1): s390x/kvm: Fix diagnose handling. Thomas Huth (7): s390x/kvm: Removed duplicated SIGP defines s390x/kvm: Removed s390_store_status stub s390x/kvm: Fix coding style in handle_sigp() s390x/kvm: Implemented SIGP START s390x/kvm: Simplified the calculation of the SIGP order code s390x/kvm: Fixed condition code for unknown SIGP orders s390x/ioinst: CHSC has to set a condition code target-s390x/cpu.h| 3 ++ target-s390x/ioinst.c | 1 + target-s390x/kvm.c| 98 --- 3 files changed, 50 insertions(+), 52 deletions(-) -- 1.8.3.4
[Qemu-devel] [PATCH 1/8] s390x/kvm: Fix diagnose handling.
From: Cornelia Huck cornelia.h...@de.ibm.com
The instruction intercept handler for diagnose used only the displacement
when trying to calculate the function code. This is only correct for base
0, however; we need to perform a complete base/displacement address
calculation and use bits 48-63 as the function code.
Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com
Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
---
target-s390x/cpu.h | 3 +++
target-s390x/kvm.c | 19 +--
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index a2c077b..68b5ab7 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -352,6 +352,9 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env,
uint32_t ipb)
return addr;
}
+/* Base/displacement are at the same locations. */
+#define decode_basedisp_rs decode_basedisp_s
+
void s390x_tod_timer(void *opaque);
void s390x_cpu_timer(void *opaque);
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 02ac4ba..b00a661 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -562,11 +562,19 @@ static void kvm_handle_diag_308(S390CPU *cpu, struct
kvm_run *run)
handle_diag_308(cpu-env, r1, r3);
}
-static int handle_diag(S390CPU *cpu, struct kvm_run *run, int ipb_code)
+#define DIAG_KVM_CODE_MASK 0x
+
+static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
{
int r = 0;
-
-switch (ipb_code) {
+uint16_t func_code;
+
+/*
+ * For any diagnose call we support, bits 48-63 of the resulting
+ * address specify the function code; the remainder is ignored.
+ */
+func_code = decode_basedisp_rs(cpu-env, ipb) DIAG_KVM_CODE_MASK;
+switch (func_code) {
case DIAG_IPL:
kvm_handle_diag_308(cpu, run);
break;
@@ -577,7 +585,7 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run,
int ipb_code)
sleep(10);
break;
default:
-DPRINTF(KVM: unknown DIAG: 0x%x\n, ipb_code);
+DPRINTF(KVM: unknown DIAG: 0x%x\n, func_code);
r = -1;
break;
}
@@ -684,7 +692,6 @@ static void handle_instruction(S390CPU *cpu, struct kvm_run
*run)
{
unsigned int ipa0 = (run-s390_sieic.ipa 0xff00);
uint8_t ipa1 = run-s390_sieic.ipa 0x00ff;
-int ipb_code = (run-s390_sieic.ipb 0x0fff) 16;
int r = -1;
DPRINTF(handle_instruction 0x%x 0x%x\n,
@@ -696,7 +703,7 @@ static void handle_instruction(S390CPU *cpu, struct kvm_run
*run)
r = handle_priv(cpu, run, ipa0 8, ipa1);
break;
case IPA0_DIAG:
-r = handle_diag(cpu, run, ipb_code);
+r = handle_diag(cpu, run, run-s390_sieic.ipb);
break;
case IPA0_SIGP:
r = handle_sigp(cpu, run, ipa1);
--
1.8.3.4
[Qemu-devel] [PATCH 4/8] s390x/kvm: Fix coding style in handle_sigp()
From: Thomas Huth th...@linux.vnet.ibm.com
To make scripts/checkpatch.pl happy for the following patches,
the coding style in handle_sigp() has to be fixed first.
Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com
Acked-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
---
target-s390x/kvm.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 5b243b4..8c54134 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -642,18 +642,18 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run,
uint8_t ipa1)
}
switch (order_code) {
-case SIGP_RESTART:
-r = kvm_s390_cpu_restart(target_cpu);
-break;
-case SIGP_SET_ARCH:
-/* make the caller panic */
-return -1;
-case SIGP_INITIAL_CPU_RESET:
-r = s390_cpu_initial_reset(target_cpu);
-break;
-default:
-fprintf(stderr, KVM: unknown SIGP: 0x%x\n, order_code);
-break;
+case SIGP_RESTART:
+r = kvm_s390_cpu_restart(target_cpu);
+break;
+case SIGP_SET_ARCH:
+/* make the caller panic */
+return -1;
+case SIGP_INITIAL_CPU_RESET:
+r = s390_cpu_initial_reset(target_cpu);
+break;
+default:
+fprintf(stderr, KVM: unknown SIGP: 0x%x\n, order_code);
+break;
}
out:
--
1.8.3.4
[Qemu-devel] [PATCH 8/8] s390x/ioinst: CHSC has to set a condition code
From: Thomas Huth th...@linux.vnet.ibm.com I missed to set the CC in the CHSC instruction when I refactored the CC setting in the IO instructions with the following commit: 5d9bf1c07c1369ab3506fc82cc65a10f4415d867 s390/ioinst: Moved the CC setting to the IO instruction handlers This patch now restores the correct behaviour of CHSC by setting the condition code 0 at the end of the instruction. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com --- target-s390x/ioinst.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c index 8d6363d..b8a6486 100644 --- a/target-s390x/ioinst.c +++ b/target-s390x/ioinst.c @@ -622,6 +622,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb) break; } +setcc(cpu, 0);/* Command execution complete */ out: s390_cpu_physical_memory_unmap(env, req, map_size, 1); } -- 1.8.3.4
[Qemu-devel] [PATCH 2/8] s390x/kvm: Removed duplicated SIGP defines
From: Thomas Huth th...@linux.vnet.ibm.com
The SIGP order defines are also available in cpu.h,
so there is no need to re-define them in kvm.c.
Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
---
target-s390x/kvm.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b00a661..52d93a7 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -82,11 +82,6 @@
#define ICPT_CPU_STOP 0x28
#define ICPT_IO 0x40
-#define SIGP_RESTART0x06
-#define SIGP_INITIAL_CPU_RESET 0x0b
-#define SIGP_STORE_STATUS_ADDR 0x0e
-#define SIGP_SET_ARCH 0x12
-
const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
KVM_CAP_LAST_INFO
};
--
1.8.3.4
[Qemu-devel] [PATCH 3/8] s390x/kvm: Removed s390_store_status stub
From: Thomas Huth th...@linux.vnet.ibm.com
The SIGP order STORE STATUS AT ADDRESS will be handled in
kernel space, so we do not need the stub in QEMU anymore.
Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
---
target-s390x/kvm.c | 22 --
1 file changed, 22 deletions(-)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 52d93a7..5b243b4 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -597,13 +597,6 @@ int kvm_s390_cpu_restart(S390CPU *cpu)
return 0;
}
-static int s390_store_status(CPUS390XState *env, uint32_t parameter)
-{
-/* XXX */
-fprintf(stderr, XXX SIGP store status\n);
-return -1;
-}
-
static int s390_cpu_initial_reset(S390CPU *cpu)
{
CPUState *cs = CPU(cpu);
@@ -629,12 +622,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run,
uint8_t ipa1)
{
CPUS390XState *env = cpu-env;
uint8_t order_code;
-uint32_t parameter;
uint16_t cpu_addr;
-uint8_t t;
int r = -1;
S390CPU *target_cpu;
-CPUS390XState *target_env;
cpu_synchronize_state(CPU(cpu));
@@ -645,28 +635,16 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run,
uint8_t ipa1)
}
order_code += (run-s390_sieic.ipb 0x0fff) 16;
-/* get parameters */
-t = (ipa1 0xf0) 4;
-if (!(t % 2)) {
-t++;
-}
-
-parameter = env-regs[t] 0x7e00;
cpu_addr = env-regs[ipa1 0x0f];
-
target_cpu = s390_cpu_addr2state(cpu_addr);
if (target_cpu == NULL) {
goto out;
}
-target_env = target_cpu-env;
switch (order_code) {
case SIGP_RESTART:
r = kvm_s390_cpu_restart(target_cpu);
break;
-case SIGP_STORE_STATUS_ADDR:
-r = s390_store_status(target_env, parameter);
-break;
case SIGP_SET_ARCH:
/* make the caller panic */
return -1;
--
1.8.3.4
[Qemu-devel] [PATCH 5/8] s390x/kvm: Implemented SIGP START
From: Thomas Huth th...@linux.vnet.ibm.com
This patch adds the missing START order to the SIGP instruction handler.
Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
---
target-s390x/kvm.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 8c54134..fcc159f 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -588,6 +588,14 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run,
uint32_t ipb)
return r;
}
+static int kvm_s390_cpu_start(S390CPU *cpu)
+{
+s390_add_running_cpu(cpu);
+qemu_cpu_kick(CPU(cpu));
+DPRINTF(DONE: KVM cpu start: %p\n, cpu-env);
+return 0;
+}
+
int kvm_s390_cpu_restart(S390CPU *cpu)
{
kvm_s390_interrupt(cpu, KVM_S390_RESTART, 0);
@@ -642,6 +650,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run,
uint8_t ipa1)
}
switch (order_code) {
+case SIGP_START:
+r = kvm_s390_cpu_start(target_cpu);
+break;
case SIGP_RESTART:
r = kvm_s390_cpu_restart(target_cpu);
break;
--
1.8.3.4
[Qemu-devel] [PATCH 6/8] s390x/kvm: Simplified the calculation of the SIGP order code
From: Thomas Huth th...@linux.vnet.ibm.com
We've already got a helper function for calculating the
base/displacement of RS formatted instructions, so we can
get rid of the manual calculation of the SIGP order code.
Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
---
target-s390x/kvm.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index fcc159f..0bf3d1f 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -626,6 +626,8 @@ static int s390_cpu_initial_reset(S390CPU *cpu)
return 0;
}
+#define SIGP_ORDER_MASK 0x00ff
+
static int handle_sigp(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
{
CPUS390XState *env = cpu-env;
@@ -637,11 +639,7 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run,
uint8_t ipa1)
cpu_synchronize_state(CPU(cpu));
/* get order code */
-order_code = run-s390_sieic.ipb 28;
-if (order_code 0) {
-order_code = env-regs[order_code];
-}
-order_code += (run-s390_sieic.ipb 0x0fff) 16;
+order_code = decode_basedisp_rs(env, run-s390_sieic.ipb)
SIGP_ORDER_MASK;
cpu_addr = env-regs[ipa1 0x0f];
target_cpu = s390_cpu_addr2state(cpu_addr);
--
1.8.3.4
[Qemu-devel] [PATCH 7/8] s390x/kvm: Fixed condition code for unknown SIGP orders
From: Thomas Huth th...@linux.vnet.ibm.com
If SIGP is called with an unknown order code, it has to return CC1
instead of CC3 and set the invalid order bit in the return status.
Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
---
target-s390x/kvm.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 0bf3d1f..f7b7726 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -633,8 +633,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run,
uint8_t ipa1)
CPUS390XState *env = cpu-env;
uint8_t order_code;
uint16_t cpu_addr;
-int r = -1;
S390CPU *target_cpu;
+uint64_t *statusreg = env-regs[ipa1 4];
+int cc;
cpu_synchronize_state(CPU(cpu));
@@ -644,29 +645,33 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run,
uint8_t ipa1)
cpu_addr = env-regs[ipa1 0x0f];
target_cpu = s390_cpu_addr2state(cpu_addr);
if (target_cpu == NULL) {
+cc = 3;/* not operational */
goto out;
}
switch (order_code) {
case SIGP_START:
-r = kvm_s390_cpu_start(target_cpu);
+cc = kvm_s390_cpu_start(target_cpu);
break;
case SIGP_RESTART:
-r = kvm_s390_cpu_restart(target_cpu);
+cc = kvm_s390_cpu_restart(target_cpu);
break;
case SIGP_SET_ARCH:
/* make the caller panic */
return -1;
case SIGP_INITIAL_CPU_RESET:
-r = s390_cpu_initial_reset(target_cpu);
+cc = s390_cpu_initial_reset(target_cpu);
break;
default:
-fprintf(stderr, KVM: unknown SIGP: 0x%x\n, order_code);
+DPRINTF(KVM: unknown SIGP: 0x%x\n, order_code);
+*statusreg = 0xUL;
+*statusreg |= SIGP_STAT_INVALID_ORDER;
+cc = 1; /* status stored */
break;
}
out:
-setcc(cpu, r ? 3 : 0);
+setcc(cpu, cc);
return 0;
}
--
1.8.3.4
Re: [Qemu-devel] [PATCH V3 00/13] Stage 2 VSX Support
On 03.12.2013, at 17:11, Tom Musta tommu...@gmail.com wrote: On 11/1/2013 8:21 AM, Tom Musta wrote: NOTE: this is a resubmission of this patch series. Alex discovered some corruption in the patches from my previous submission. This patch series continues adding support for the PowerPC Vector Scalar Extension (VSX). Patches are relative to the Stage 1 delivery (see http://lists.nongnu.org/archive/html/qemu-ppc/2013-09/msg00231.html). This series adds the following: a) all remaining load and store instructions defined by the V2.06 Power ISA (aka Power7). b) The vector and scalar move instructions. c) The logical instructions defined by V2.06. d) Assorted permute and select instructions. V2: reworked patches 4, 10, 11 and 12 per comments from Richard Henderson (thanks, Richard!) V3: reworked patches 7 8 per comments from Paolo Bonzini (thanks, Paulo) Tom Musta (13): Abandon GEN_VSX_* macros Add lxsdx Add lxvdsx Add lxvw4x Add stxsdx Add stxvw4x Add VSX Scalar Move Instructions Add VSX Vector Move Instructions Add Power7 VSX Logical Instructions Add xxmrgh/xxmrgl Add xxsel Add xxspltw Add xxsldwi target-ppc/translate.c | 491 +++- 1 files changed, 483 insertions(+), 8 deletions(-) Ping. Thanks, applied to ppc-next. Alex
Re: [Qemu-devel] [PATCH 7/7] default-configs: Add config for aarch64-softmmu
On 12/16/2013 06:40 PM, Christoffer Dall wrote: On Thu, Nov 28, 2013 at 01:33:22PM +, Peter Maydell wrote: Add a config for aarch64-softmmu; this enables building of this target. The resulting executable doesn't know about any 64 bit CPUs, but all the 32 bit CPUs and board models work. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- default-configs/aarch64-softmmu.mak |9 + 1 file changed, 9 insertions(+) create mode 100644 default-configs/aarch64-softmmu.mak diff --git a/default-configs/aarch64-softmmu.mak b/default-configs/aarch64-softmmu.mak new file mode 100644 index 000..76a45cc --- /dev/null +++ b/default-configs/aarch64-softmmu.mak @@ -0,0 +1,9 @@ +# Default configuration for aarch64-softmmu + +include pci.mak +include usb.mak pci and usb? pci is required for virtio I presume, and USB? for 32-bit boards? PCI is not required for VirtIO since the introduction of VirtIO-MMIO. 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] [Qemu-ppc] [PATCH V3 01/19] Fix float64_to_uint64
On 01.11.2013, at 14:35, Tom Musta tommu...@gmail.com wrote:
The comment preceding the float64_to_uint64 routine suggests that
the implementation is broken. And this is, indeed, the case.
This patch properly implements the conversion of a 64-bit floating
point number to an unsigned, 64 bit integer.
This contribution can be licensed under either the softfloat-2a or -2b
license.
V2: Added softfloat license statement.
V3: Modified to meet QEMU coding conventions.
Signed-off-by: Tom Musta tommu...@gmail.com
Softfloat really isn't my area of expertise and I'd prefer to see these patches
go in through a different tree :). Peter, do you want to take care of this
patch and Add float32_to_uint64()? Or at least give me your reviewed-by tag
and I'll send it through my tree since the second one is a build dependency for
VSX patches.
I've also seen comments on folding in bugfixes for patches that occurred
earlier. I presume this is one of these cases. Mind to shed some light on this?
Alex
---
fpu/softfloat.c | 95 ++-
1 files changed, 87 insertions(+), 8 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 7ba51b6..3070eaa 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -204,6 +204,47 @@ static int64 roundAndPackInt64( flag zSign, uint64_t
absZ0, uint64_t absZ1 STATU
}
/*
+| Takes the 128-bit fixed-point value formed by concatenating `absZ0' and
+| `absZ1', with binary point between bits 63 and 64 (between the input
words),
+| and returns the properly rounded 64-bit unsigned integer corresponding to
the
+| input. Ordinarily, the fixed-point input is simply rounded to an integer,
+| with the inexact exception raised if the input cannot be represented
exactly
+| as an integer. However, if the fixed-point input is too large, the invalid
+| exception is raised and the largest unsigned integer is returned.
+**/
+
+static int64 roundAndPackUint64(uint64_t absZ0, uint64_t absZ1 STATUS_PARAM)
+{
+int8 roundingMode;
+flag roundNearestEven, increment;
+int64_t z;
+
+roundingMode = STATUS(float_rounding_mode);
+roundNearestEven = (roundingMode == float_round_nearest_even);
+increment = ((int64_t) absZ1 0);
+if (!roundNearestEven) {
+if (roundingMode == float_round_to_zero) {
+increment = 0;
+} else {
+increment = (roundingMode == float_round_up) absZ1;
+}
+}
+if (increment) {
+++absZ0;
+if (absZ0 == 0) {
+float_raise(float_flag_invalid STATUS_VAR);
+return LIT64(0x);
+}
+absZ0 = ~(((uint64_t)(absZ11) == 0) roundNearestEven);
+}
+z = absZ0;
+if (absZ1) {
+STATUS(float_exception_flags) |= float_flag_inexact;
+}
+return z;
+}
+
+/*
| Returns the fraction bits of the single-precision floating-point value `a'.
**/
@@ -6536,18 +6577,56 @@ uint_fast16_t float64_to_uint16_round_to_zero(float64
a STATUS_PARAM)
return res;
}
-/* FIXME: This looks broken. */
-uint64_t float64_to_uint64 (float64 a STATUS_PARAM)
-{
-int64_t v;
+/*
+| Returns the result of converting the double-precision floating-point value
+| `a' to the 64-bit unsigned integer format. The conversion is
+| performed according to the IEC/IEEE Standard for Binary Floating-Point
+| Arithmetic---which means in particular that the conversion is rounded
+| according to the current rounding mode. If `a' is a NaN, the largest
+| positive integer is returned. If the conversion overflows, the
+| largest unsigned integer is returned. If 'a' is negative, zero is
+| returned.
+**/
-v = float64_val(int64_to_float64(INT64_MIN STATUS_VAR));
-v += float64_val(a);
-v = float64_to_int64(make_float64(v) STATUS_VAR);
+uint64_t float64_to_uint64(float64 a STATUS_PARAM)
+{
+flag aSign;
+int_fast16_t aExp, shiftCount;
+uint64_t aSig, aSigExtra;
+a = float64_squash_input_denormal(a STATUS_VAR);
-return v - INT64_MIN;
+aSig = extractFloat64Frac(a);
+aExp = extractFloat64Exp(a);
+aSign = extractFloat64Sign(a);
+if (aSign) {
+if (aExp) {
+float_raise(float_flag_invalid STATUS_VAR);
+} else if (aSig) { /* negative denormalized */
+float_raise(float_flag_inexact STATUS_VAR);
+}
+return 0;
+}
+if (aExp) {
+aSig |=
Re: [Qemu-devel] [PATCH v2 6/7] hw/arm/boot: Add boot support for AArch64 processor
On Tue, Dec 17, 2013 at 11:14 PM, Peter Maydell
peter.mayd...@linaro.org wrote:
On 17 December 2013 13:04, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
On Tue, Dec 17, 2013 at 10:15 PM, Peter Maydell
peter.mayd...@linaro.org wrote:
From: Mian M. Hamayun m.hama...@virtualopensystems.com
This commit adds support for booting a single AArch64 CPU by setting
appropriate registers. The bootloader includes placehoders for Board-ID
placeholders
Fixed (not going to respin just for that though :-))
that are used to implement uniform indexing across different bootloaders.
Signed-off-by: Mian M. Hamayun m.hama...@virtualopensystems.com
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Message-id: 1385645602-18662-7-git-send-email-peter.mayd...@linaro.org
[PMM:
* updated to use ARMInsnFixup style bootloader fragments
* dropped virt.c additions
* use runtime checks for is this an AArch64 core rather than ifdefs
* drop some unnecessary setting of registers in reset hook
]
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Christoffer Dall christoffer.d...@linaro.org
---
hw/arm/boot.c | 43 ++-
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 0c05a64..90e9534 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -17,8 +17,13 @@
#include sysemu/device_tree.h
#include qemu/config-file.h
+/* Kernel boot protocol is specified in the kernel docs
+ * Documentation/arm/Booting and Documentation/arm64/booting.txt
+ * They have different preferred image load offsets from system RAM base.
+ */
#define KERNEL_ARGS_ADDR 0x100
#define KERNEL_LOAD_ADDR 0x0001
So out of context, but I have been booting an ARMv7 multi defconfig on
a few qemu platforms recently (spec. allwinner, highbank and zynq) and
I found I had to patch this to 0x8000 due to some image alignment
expectations of head.S. Could we patch this to 0x8000 - is there this
same sense of preferred image offset in KERNEL32_LOAD_ADDR?
Hmm. Are you booting non-zImage kernels? The booting doc says
Yep.
# When booting a raw (non-zImage) kernel the constraints are tighter.
# In this case the kernel must be loaded at an offset into system equal
# to TEXT_OFFSET - PAGE_OFFSET.
Yes it'll probably that, although my trace through head.S had
significant variation with different Kconfigs and normal Image has
booted in the past. Would have to get back with specifics.
so maybe that's what we're not getting right there? I practically
always use a zImage so I wouldn't ever see that.
In any case if you can nail down whether that's the problem and
submit a patch that would be good, though we'll have to test
on a bunch of boards that we haven't broken something else.
(I don't particularly want to change to 0x8000 because it happens
to work, but the same change with a rationale attached is fine :-))
+#define KERNEL64_LOAD_ADDR 0x0008
typedef enum {
FIXUP_NONE = 0, /* do nothing */
@@ -37,6 +42,20 @@ typedef struct ARMInsnFixup {
FixupType fixup;
} ARMInsnFixup;
+static const ARMInsnFixup bootloader_aarch64[] = {
+{ 0x58c0 }, /* ldr x0, arg ; Load the lower 32-bits of DTB */
+{ 0xaa1f03e1 }, /* mov x1, xzr */
+{ 0xaa1f03e2 }, /* mov x2, xzr */
+{ 0xaa1f03e3 }, /* mov x3, xzr */
+{ 0x5884 }, /* ldr x4, entry ; Load the lower 32-bits of kernel
entry */
+{ 0xd61f0080 }, /* br x4 ; Jump to the kernel entry point */
+{ 0, FIXUP_ARGPTR }, /* arg: .word @DTB Lower 32-bits */
+{ 0 }, /* .word @DTB Higher 32-bits */
+{ 0, FIXUP_ENTRYPOINT }, /* entry: .word @Kernel Entry Lower 32-bits */
+{ 0 }, /* .word @Kernel Entry Higher 32-bits */
+{ 0, FIXUP_TERMINATOR }
+};
+
/* The worlds second smallest bootloader. Set r0-r2, then jump to kernel.
*/
static const ARMInsnFixup bootloader[] = {
{ 0xe3a0 }, /* mov r0, #0 */
@@ -396,7 +415,12 @@ static void do_cpu_reset(void *opaque)
env-thumb = info-entry 1;
} else {
if (CPU(cpu) == first_cpu) {
-env-regs[15] = info-loader_start;
+if (env-aarch64) {
Curious, why does this 'if' directly deref env, while the one below
(for primary_loader selection) uses ARM_FEATURE?
When we're picking the bootloader the CPU isn't started, so
its internal state (including the aarch64 flag, which is a reflection
of PSTATE.nRW) isn't valid. We want to pick a bootloader on
the basis of if this CPU can do AArch64 then use the AArch64
loader. This code however runs after reset and so we can say
if you're currently in AArch64 mode then this is how the PC is
set, otherwise like this.
But isn't the selection of which primary bootloader pre-decided by
arm_feature? This would lead to an error if (somehow) the bootloader
blobbed in the 64 bit version and the CPU reset into 32 (via this
aarch64 flag).
Re: [Qemu-devel] [PATCH v10 0/6] hw/arm: add initial support for Canon DIGIC SoC
On 16 December 2013 10:15, Antony Pavlov antonynpav...@gmail.com wrote: Changes since v9: 1. rebase over Peter Crosthwaite's Fix Support for ARM CBAR and reset-hivecs v5 patch series 2. qom-test: add canon-a1100 to arm machines list 3. include a diffstat in the cover letter (use --cover-letter option) 4. fix patches timestamp Applied all to target-arm.next; thanks for your patience in working through our code review process. -- PMM
Re: [Qemu-devel] outlined TLB lookup on x86
On Sun, Dec 8, 2013 at 2:54 AM, Xin Tong trent.t...@gmail.com wrote: On Thu, Nov 28, 2013 at 8:12 AM, Lluís Vilanova vilan...@ac.upc.edu wrote: Xin Tong writes: Hi LIuis we can probably generate vector intrinsics using the tcg, e.g. add support to tcg to emit vector instructions directly in code cache There was some discussion long ago about adding vector instructions to TCG, but I don't remember what was the conclusion. Also remember that using vector instructions will emulate a low-associativity TLB; don't know how much better than a 1-way TLB will that be, though. why would a larger TLB make some operations slower, the TLB is a direct-mapped hash and lookup should be O(1) there. In the cputlb, the CPU_TLB_SIZE is always used to index into the TLB, i.e. (X (CPU_TLB_SIZE -1)). It would make TLB invalidations slower (e.g., see 'tlb_flush' in cputlb.c). And right now QEMU performs full TLB invalidations more frequently than the equivalent HW needs to, although I suppose that should be quantified too. I see QEMU executed ~1M instructions per context switch for qemu-system-x86_64. Is this because of the fact that the periodical time interval interrupt is delivered in real time while QEMU is significantly slower than real hw ? Xin you are right LIuis. QEMU does context switch quite more often that real hw, this is probably primarily due to the fact that QEMU is magnitude slower than real hw. I am wondering where timer is emulated in QEMU system-x86_64. I imagine the guest OS must program the timers to do interrupt for context switches. Another question, what happens when a vcpu is stuck in an infinite loop ? QEMU must need an timer interrupt somewhere as well ? Is my understanding correct ? Xin Lluis -- And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer. -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
Re: [Qemu-devel] [Qemu-ppc] [PATCH V3 01/19] Fix float64_to_uint64
On 17 December 2013 13:42, Alexander Graf ag...@suse.de wrote: Softfloat really isn't my area of expertise and I'd prefer to see these patches go in through a different tree :). Peter, do you want to take care of this patch and Add float32_to_uint64()? Or at least give me your reviewed-by tag and I'll send it through my tree since the second one is a build dependency for VSX patches. I've also seen comments on folding in bugfixes for patches that occurred earlier. I presume this is one of these cases. Mind to shed some light on this? Yeah, these are on my to-review list (we need the fixes for aarch64 too :-)) but I'm waiting for Tom to post an updated patchset which has the complete set of softfloat fixes rather than having one patchset with a first version and a different patchset with a patch which fixes bugs in the first one... thanks -- PMM
Re: [Qemu-devel] [PATCH 5/8] s390x/kvm: Implemented SIGP START
On 17.12.2013, at 14:22, Jens Freimann jf...@linux.vnet.ibm.com wrote:
From: Thomas Huth th...@linux.vnet.ibm.com
This patch adds the missing START order to the SIGP instruction handler.
Does the spec define what happens on START when the CPU is already running?
Does START modify any register state?
Alex
Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
---
target-s390x/kvm.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 8c54134..fcc159f 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -588,6 +588,14 @@ static int handle_diag(S390CPU *cpu, struct kvm_run
*run, uint32_t ipb)
return r;
}
+static int kvm_s390_cpu_start(S390CPU *cpu)
+{
+s390_add_running_cpu(cpu);
+qemu_cpu_kick(CPU(cpu));
+DPRINTF(DONE: KVM cpu start: %p\n, cpu-env);
+return 0;
+}
+
int kvm_s390_cpu_restart(S390CPU *cpu)
{
kvm_s390_interrupt(cpu, KVM_S390_RESTART, 0);
@@ -642,6 +650,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run *run,
uint8_t ipa1)
}
switch (order_code) {
+case SIGP_START:
+r = kvm_s390_cpu_start(target_cpu);
+break;
case SIGP_RESTART:
r = kvm_s390_cpu_restart(target_cpu);
break;
--
1.8.3.4
Re: [Qemu-devel] [qemu-kvm PATCH] block: vhdx - improve error message, and .bdrv_check implementation
On 12/17/2013 03:33 AM, Jeff Cody wrote: If there is a dirty log file to be replayed in a VHDX image, it is replayed in .vhdx_open(). However, if the file is opened read-only, then a somewhat cryptic error message results. This adds a more helpful error message for the user. If an image file contains a log to be replayed, and is opened read-only, the user is instructed to run 'qemu-img check -r all' on the image file. Running qemu-img check -r all will cause the image file to be opened r/w, which will replay the log file. If a log file replay is detected, this is flagged, and bdrv_check will increase the corruptions_fixed count for the image. +error_setg_errno(errp, EPERM, + VHDX image file '%s' opened read-only, but + contains a log that needs replayed. To replay s/replayed/replaying/ + the log, execute:\n qemu-img check -r all '%s', + bs-filename, bs-filename); +goto exit; -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] hindsight on qcow2v3 format
On Mon, Dec 16, 2013 at 05:24:53PM -0700, Eric Blake wrote: Is it worth tweaking docs/specs/qcow2.txt to permanently change the incompatible_features field to be 4 bytes (76-79) and mandate that bytes 72-75 always be 0, as a conservative way to prevent other misbehavior of programs like libvirt 0.10.2 that have code paths that don't correctly validate for version 2 files? And if we ever really did hit a situation of having 31 or more incompatible features, I could envision using bit 31 as an incompatible_feature witness that tells new enough qemu to look at some other offset for remaining feature bits (thankfully, the semantics of the incompatible_feature field mean that older qemu that honors version 3 formats will reject a file with incompatible_feature bit 31 set), so this is no real loss in the number of potential incompatible features being added. I don't think qcow2 changes are necessary, just make a libvirt 0.10.x stable release that verifies the qcow2 version. Stefan
Re: [Qemu-devel] [PATCH 8/8] s390x/ioinst: CHSC has to set a condition code
On 17.12.2013, at 14:22, Jens Freimann jf...@linux.vnet.ibm.com wrote: From: Thomas Huth th...@linux.vnet.ibm.com I missed to set the CC in the CHSC instruction when I refactored the CC setting in the IO instructions with the following commit: 5d9bf1c07c1369ab3506fc82cc65a10f4415d867 s390/ioinst: Moved the CC setting to the IO instruction handlers This patch now restores the correct behaviour of CHSC by setting the condition code 0 at the end of the instruction. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com I suppose this patch should be CC'ed to stable? Alex --- target-s390x/ioinst.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c index 8d6363d..b8a6486 100644 --- a/target-s390x/ioinst.c +++ b/target-s390x/ioinst.c @@ -622,6 +622,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb) break; } +setcc(cpu, 0);/* Command execution complete */ out: s390_cpu_physical_memory_unmap(env, req, map_size, 1); } -- 1.8.3.4
Re: [Qemu-devel] [Qemu-ppc] [PATCH V3 01/19] Fix float64_to_uint64
On 17.12.2013, at 14:52, Peter Maydell peter.mayd...@linaro.org wrote: On 17 December 2013 13:42, Alexander Graf ag...@suse.de wrote: Softfloat really isn't my area of expertise and I'd prefer to see these patches go in through a different tree :). Peter, do you want to take care of this patch and Add float32_to_uint64()? Or at least give me your reviewed-by tag and I'll send it through my tree since the second one is a build dependency for VSX patches. I've also seen comments on folding in bugfixes for patches that occurred earlier. I presume this is one of these cases. Mind to shed some light on this? Yeah, these are on my to-review list (we need the fixes for aarch64 too :-)) but I'm waiting for Tom to post an updated patchset which has the complete set of softfloat fixes rather than having one patchset with a first version and a different patchset with a patch which fixes bugs in the first one... Ok, I'll wait until you have applied them then until I apply stage 3 :). Alex
Re: [Qemu-devel] [PATCH] specs: mandate qcow2 v2 end-of-extensions in v3 header
On Mon, Dec 16, 2013 at 05:52:08PM -0700, Eric Blake wrote: At least libvirt 0.10.2 has a bug where it can be provoked into attempting to search for extension headers (in particular, the backing file format header) without first validating that a particular file is qcow2 version 2. Fortunately, to date, this probe always ends early when feeding a version 3 file to this version of libvirt: the version 2 file treats bytes 72-75 as the magic number for an extension header, while version 3 treats it as the upper half of the 8-byte incompatible_features field. As long as we don't have that many incompatible features, these bytes are always 0, which aligns with the end-of-extension magic number, and forces the broken libvirt to quit looking for other headers in its v2 view of the world. But as future extensions may conceivably contain contents that would cause broken libvirt to misbehave if it kept looking for headers, it is better to codify this into the qcow2 v3 specification. No qemu code changes are required by this doc patch - anyone setting bits within bytes 72-75 already causes the file to be rejected as an invalid qcow2 v3 file by all current v3 implementations. And cutting the field from 8 bytes to 4 has no real serious consequence - if we ever have that many incompatible bits in the future, we can always declare bit 31 as an incompatible feature bit that says where else to look for any further needed incompatible feature bits. [If we had a time machine, I would propose that we make bytes 72-79 of the version 3 header resemble a well-formed extension header of version 2, and move incompatible_features to offset 80 or later - but that's what we get for having hindsight.] * docs/specs/qcow2.txt: Formally define what happened to be an accidental feature protecting us from buggy v2 clients. Signed-off-by: Eric Blake ebl...@redhat.com --- docs/specs/qcow2.txt | 20 ++-- 1 file changed, 18 insertions(+), 2 deletions(-) I see this as a libvirt bug that should be fixed in a libvirt stable release. The libvirt bug needs to be fixed anyway, let's not add quirks to the qcow2 spec. Kevin is on holiday, he may want to discuss more when he returns. Stefan
Re: [Qemu-devel] [PATCH v13 0/6] add allwinner A10 SoC support
On 16 December 2013 02:01, liguang lig.f...@cn.fujitsu.com wrote: lay a foundation for allwinner A10 SoC with a cortex-a8 processor, and will add more devices later. Thanks; applied all to target-arm.next. -- PMM
Re: [Qemu-devel] [PATCH v10 5/6] hw/arm/digic: add NOR ROM support
On 16 December 2013 10:15, Antony Pavlov antonynpav...@gmail.com wrote:
+static void digic_load_rom(DigicBoardState *s, hwaddr addr,
+ hwaddr max_size, const char *def_filename)
+{
+target_long rom_size;
+const char *filename;
+
+if (bios_name) {
+filename = bios_name;
+} else {
+filename = def_filename;
+}
+
+if (filename) {
+char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, filename);
+
+if (!fn) {
+error_report(Couldn't find rom image '%s'.\n, filename);
+exit(1);
+}
+
+rom_size = load_image_targphys(fn, addr, max_size);
+if (rom_size 0 || rom_size max_size) {
+error_report(Couldn't load rom image '%s'.\n, filename);
+exit(1);
+}
+}
+}
This results in 'make check' failing if you don't have the rom blob
around. Since I've already applied this to my tree, I've just fixed this
nit there, by adding this change:
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -30,6 +30,7 @@
#include hw/block/flash.h
#include hw/loader.h
#include sysemu/sysemu.h
+#include sysemu/qtest.h
#define DIGIC4_ROM0_BASE 0xf000
#define DIGIC4_ROM1_BASE 0xf800
@@ -86,6 +87,13 @@ static void digic_load_rom(DigicBoardState *s, hwaddr addr,
target_long rom_size;
const char *filename;
+if (qtest_enabled()) {
+/* qtest runs no code so don't attempt a ROM load which
+ * could fail and result in a spurious test failure.
+ */
+return;
+}
+
if (bios_name) {
filename = bios_name;
} else {
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2] block/iscsi: Fix compilation for libiscsi 1.4.0 (API change)
On Tue, Dec 17, 2013 at 08:57:10AM +0100, Stefan Weil wrote: Function iscsi_read10_task got additional parameters starting with version libiscsi 1.5.0. libiscsi 1.4.0 is still widely used (Debian wheezy, jessie and other Linux distributions currently provide packages for QEMU which use it), so we still need support for this older API. Reviewed-by: Peter Lieven p...@kamp.de Signed-off-by: Stefan Weil s...@weilnetz.de --- v2: Fixed libscsi - libiscsi block/iscsi.c |5 - configure | 22 ++ 2 files changed, 26 insertions(+), 1 deletion(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH 5/8] s390x/kvm: Implemented SIGP START
Hi Alex,
On Tue, 17 Dec 2013 14:56:33 +0100
Alexander Graf ag...@suse.de wrote:
On 17.12.2013, at 14:22, Jens Freimann jf...@linux.vnet.ibm.com wrote:
From: Thomas Huth th...@linux.vnet.ibm.com
This patch adds the missing START order to the SIGP instruction handler.
Does the spec define what happens on START when the CPU is already running?
As far as I can see, the spec does not explicitely defines the behavior
in that case, it says only: The order is effective only when the
addressed CPU is in the stopped state. But according to my experiments
(I wrote a test program that I ran without additional hypervisor), the
START order is simply ignored when the CPU is already running and CC0 is
returned.
This is also what happens with the code below, since
s390_add_running_cpu() only does something if the halted flag is set,
and kicking a CPU that is already running does not hurt either, as far
as I can see.
Does START modify any register state?
No, the CPU simply continues running with the state where it has been
stopped before.
Thomas
Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com
Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
---
target-s390x/kvm.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 8c54134..fcc159f 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -588,6 +588,14 @@ static int handle_diag(S390CPU *cpu, struct kvm_run
*run, uint32_t ipb)
return r;
}
+static int kvm_s390_cpu_start(S390CPU *cpu)
+{
+s390_add_running_cpu(cpu);
+qemu_cpu_kick(CPU(cpu));
+DPRINTF(DONE: KVM cpu start: %p\n, cpu-env);
+return 0;
+}
+
int kvm_s390_cpu_restart(S390CPU *cpu)
{
kvm_s390_interrupt(cpu, KVM_S390_RESTART, 0);
@@ -642,6 +650,9 @@ static int handle_sigp(S390CPU *cpu, struct kvm_run
*run, uint8_t ipa1)
}
switch (order_code) {
+case SIGP_START:
+r = kvm_s390_cpu_start(target_cpu);
+break;
case SIGP_RESTART:
r = kvm_s390_cpu_restart(target_cpu);
break;
--
1.8.3.4
Re: [Qemu-devel] [PATCH 5/8] s390x/kvm: Implemented SIGP START
On 17.12.2013, at 15:26, Thomas Huth th...@linux.vnet.ibm.com wrote: Hi Alex, On Tue, 17 Dec 2013 14:56:33 +0100 Alexander Graf ag...@suse.de wrote: On 17.12.2013, at 14:22, Jens Freimann jf...@linux.vnet.ibm.com wrote: From: Thomas Huth th...@linux.vnet.ibm.com This patch adds the missing START order to the SIGP instruction handler. Does the spec define what happens on START when the CPU is already running? As far as I can see, the spec does not explicitely defines the behavior in that case, it says only: The order is effective only when the addressed CPU is in the stopped state. But according to my experiments (I wrote a test program that I ran without additional hypervisor), the START order is simply ignored when the CPU is already running and CC0 is returned. This is also what happens with the code below, since s390_add_running_cpu() only does something if the halted flag is set, and kicking a CPU that is already running does not hurt either, as far as I can see. Yup :) Does START modify any register state? No, the CPU simply continues running with the state where it has been stopped before. Ok, great. Then it really does the same thing :) Alex
Re: [Qemu-devel] [PATCH v3 00/13] target-arm: A64 decoder set 2: misc logic and bit ops
On 9 December 2013 12:37, Peter Maydell peter.mayd...@linaro.org wrote: Third revision of the second chunk of A64 decoder patches: a grabbag of miscellaneous logic and bit-twiddling operations, plus some other minor stuff like ADR and conditional-select. Applied to target-arm.next since all patches in this set have got review. thanks -- PMM
[Qemu-devel] [Bug 1261743] [NEW] trace-backend simple doesn't support disable property of event
Public bug reported: trace-backend simple generates wrong eventid in trace/generated- tracers.c after disable property occured in trace-events record. Result: missing or mixing logs in trace file. ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1261743 Title: trace-backend simple doesn't support disable property of event Status in QEMU: New Bug description: trace-backend simple generates wrong eventid in trace/generated- tracers.c after disable property occured in trace-events record. Result: missing or mixing logs in trace file. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1261743/+subscriptions
Re: [Qemu-devel] [PATCH v3 00/12] target-arm: A64 decoder, foundation plus branches
On 5 December 2013 12:39, Peter Maydell peter.mayd...@linaro.org wrote: Round three of the first-chunk of A64 decoder work, updated following code review. Applied to target-arm.next since all patches in this group have now got review. thanks -- PMM
[Qemu-devel] [PATCH 07/21] target-arm: A64: add support for 3 src data proc insns
From: Alexander Graf ag...@suse.de
This patch adds emulation for the Data-processing (3 source)
family of instructions, namely MADD, MSUB, SMADDL, SMSUBL, SMULH,
UMADDL, UMSUBL, UMULH.
Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Alex Bennée alex.ben...@linaro.org
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/translate-a64.c | 92 +-
1 file changed, 90 insertions(+), 2 deletions(-)
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 96ae4e1..97c9f0b 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -2126,10 +2126,98 @@ static void disas_add_sub_reg(DisasContext *s, uint32_t
insn)
tcg_temp_free_i64(tcg_result);
}
-/* Data-processing (3 source) */
+/* C3.5.9 Data-processing (3 source)
+
+ 31 30 29 28 24 23 21 20 16 15 14 10 95 40
+ +--+--+---+--+--++--+--+--+
+ |sf| op54 | 1 1 0 1 1 | op31 | Rm | o0 | Ra | Rn | Rd |
+ +--+--+---+--+--++--+--+--+
+
+ */
static void disas_data_proc_3src(DisasContext *s, uint32_t insn)
{
-unsupported_encoding(s, insn);
+int rd = extract32(insn, 0, 5);
+int rn = extract32(insn, 5, 5);
+int ra = extract32(insn, 10, 5);
+int rm = extract32(insn, 16, 5);
+int op_id = (extract32(insn, 29, 3) 4) |
+(extract32(insn, 21, 3) 1) |
+extract32(insn, 15, 1);
+bool sf = extract32(insn, 31, 1);
+bool is_sub = extract32(op_id, 0, 1);
+bool is_high = extract32(op_id, 2, 1);
+bool is_signed = false;
+TCGv_i64 tcg_op1;
+TCGv_i64 tcg_op2;
+TCGv_i64 tcg_tmp;
+
+/* Note that op_id is sf:op54:op31:o0 so it includes the 32/64 size flag */
+switch (op_id) {
+case 0x42: /* SMADDL */
+case 0x43: /* SMSUBL */
+case 0x44: /* SMULH */
+is_signed = true;
+break;
+case 0x0: /* MADD (32bit) */
+case 0x1: /* MSUB (32bit) */
+case 0x40: /* MADD (64bit) */
+case 0x41: /* MSUB (64bit) */
+case 0x4a: /* UMADDL */
+case 0x4b: /* UMSUBL */
+case 0x4c: /* UMULH */
+break;
+default:
+unallocated_encoding(s);
+return;
+}
+
+if (is_high) {
+TCGv_i64 low_bits = tcg_temp_new_i64(); /* low bits discarded */
+TCGv_i64 tcg_rd = cpu_reg(s, rd);
+TCGv_i64 tcg_rn = cpu_reg(s, rn);
+TCGv_i64 tcg_rm = cpu_reg(s, rm);
+
+if (is_signed) {
+tcg_gen_muls2_i64(low_bits, tcg_rd, tcg_rn, tcg_rm);
+} else {
+tcg_gen_mulu2_i64(low_bits, tcg_rd, tcg_rn, tcg_rm);
+}
+
+tcg_temp_free_i64(low_bits);
+return;
+}
+
+tcg_op1 = tcg_temp_new_i64();
+tcg_op2 = tcg_temp_new_i64();
+tcg_tmp = tcg_temp_new_i64();
+
+if (op_id 0x42) {
+tcg_gen_mov_i64(tcg_op1, cpu_reg(s, rn));
+tcg_gen_mov_i64(tcg_op2, cpu_reg(s, rm));
+} else {
+if (is_signed) {
+tcg_gen_ext32s_i64(tcg_op1, cpu_reg(s, rn));
+tcg_gen_ext32s_i64(tcg_op2, cpu_reg(s, rm));
+} else {
+tcg_gen_ext32u_i64(tcg_op1, cpu_reg(s, rn));
+tcg_gen_ext32u_i64(tcg_op2, cpu_reg(s, rm));
+}
+}
+
+tcg_gen_mul_i64(tcg_tmp, tcg_op1, tcg_op2);
+if (is_sub) {
+tcg_gen_sub_i64(cpu_reg(s, rd), cpu_reg(s, ra), tcg_tmp);
+} else {
+tcg_gen_add_i64(cpu_reg(s, rd), cpu_reg(s, ra), tcg_tmp);
+}
+
+if (!sf) {
+tcg_gen_ext32u_i64(cpu_reg(s, rd), cpu_reg(s, rd));
+}
+
+tcg_temp_free_i64(tcg_op1);
+tcg_temp_free_i64(tcg_op2);
+tcg_temp_free_i64(tcg_tmp);
}
/* Add/subtract (with carry) */
--
1.8.5
[Qemu-devel] [PATCH 02/21] target-arm: A64: add support for ld/st unsigned imm
From: Alex Bennée a...@bennee.com
This adds support for the forms of ld/st with a 12 bit
unsigned immediate offset.
Signed-off-by: Alex Bennée alex.ben...@linaro.org
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/translate-a64.c | 89 +-
1 file changed, 88 insertions(+), 1 deletion(-)
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index e4ce038..3712a6d 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -899,10 +899,97 @@ static void disas_ldst_pair(DisasContext *s, uint32_t
insn)
}
}
+/*
+ * C3.3.13 Load/store (unsigned immediate)
+ *
+ * 31 30 29 27 26 25 24 23 22 2110 9 5
+ * ++---+---+-+-++---+--+
+ * |size| 1 1 1 | V | 0 1 | opc | imm12| Rn | Rt |
+ * ++---+---+-+-++---+--+
+ *
+ * For non-vector:
+ * size: 00- byte, 01 - 16 bit, 10 - 32bit, 11 - 64bit
+ * opc: 00 - store, 01 - loadu, 10 - loads 64, 11 - loads 32
+ * For vector:
+ * size is opc1:size1:0 so 100 - 128 bit; 110 and 111 unallocated
+ * opc0: 0 - store, 1 - load
+ * Rn: base address register (inc SP)
+ * Rt: target register
+ */
+static void disas_ldst_reg_unsigned_imm(DisasContext *s, uint32_t insn)
+{
+int rt = extract32(insn, 0, 5);
+int rn = extract32(insn, 5, 5);
+unsigned int imm12 = extract32(insn, 10, 12);
+bool is_vector = extract32(insn, 26, 1);
+int size = extract32(insn, 30, 2);
+int opc = extract32(insn, 22, 2);
+unsigned int offset;
+
+TCGv_i64 tcg_addr;
+
+bool is_store;
+bool is_signed = false;
+bool is_extended = false;
+
+if (is_vector) {
+size |= (opc 2) 1;
+if (size 4) {
+unallocated_encoding(s);
+return;
+}
+is_store = ((opc 1) == 0);
+} else {
+if (size == 3 opc == 2) {
+/* PRFM - prefetch */
+return;
+}
+if (opc == 3 size 1) {
+unallocated_encoding(s);
+return;
+}
+is_store = (opc == 0);
+is_signed = opc (11);
+is_extended = (size 3) (opc 1);
+}
+
+if (rn == 31) {
+gen_check_sp_alignment(s);
+}
+tcg_addr = read_cpu_reg_sp(s, rn, 1);
+offset = imm12 size;
+tcg_gen_addi_i64(tcg_addr, tcg_addr, offset);
+
+if (is_vector) {
+if (is_store) {
+do_fp_st(s, rt, tcg_addr, size);
+} else {
+do_fp_ld(s, rt, tcg_addr, size);
+}
+} else {
+TCGv_i64 tcg_rt = cpu_reg(s, rt);
+if (is_store) {
+do_gpr_st(s, tcg_rt, tcg_addr, size);
+} else {
+do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, is_extended);
+}
+}
+}
+
/* Load/store register (all forms) */
static void disas_ldst_reg(DisasContext *s, uint32_t insn)
{
-unsupported_encoding(s, insn);
+switch (extract32(insn, 24, 2)) {
+case 0:
+unsupported_encoding(s, insn);
+break;
+case 1:
+disas_ldst_reg_unsigned_imm(s, insn);
+break;
+default:
+unallocated_encoding(s);
+break;
+}
}
/* AdvSIMD load/store multiple structures */
--
1.8.5
[Qemu-devel] [PATCH 18/21] target-arm: aarch64: add support for ld lit
From: Alexander Graf ag...@suse.de
Adds support for Load Register (literal), both normal
and SIMD/FP forms.
Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Alex Bennée alex.ben...@linaro.org
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/translate-a64.c | 47 --
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 3c18d6f..e4bca31 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1117,10 +1117,53 @@ static void disas_ldst_excl(DisasContext *s, uint32_t
insn)
unsupported_encoding(s, insn);
}
-/* Load register (literal) */
+/*
+ * C3.3.5 Load register (literal)
+ *
+ * 31 30 29 27 26 25 24 235 4 0
+ * +-+---+---+-+---+---+
+ * | opc | 0 1 1 | V | 0 0 | imm19 | Rt |
+ * +-+---+---+-+---+---+
+ *
+ * V: 1 - vector (simd/fp)
+ * opc (non-vector): 00 - 32 bit, 01 - 64 bit,
+ * 10- 32 bit signed, 11 - prefetch
+ * opc (vector): 00 - 32 bit, 01 - 64 bit, 10 - 128 bit (11 unallocated)
+ */
static void disas_ld_lit(DisasContext *s, uint32_t insn)
{
-unsupported_encoding(s, insn);
+int rt = extract32(insn, 0, 5);
+int64_t imm = sextract32(insn, 5, 19) 2;
+bool is_vector = extract32(insn, 26, 1);
+int opc = extract32(insn, 30, 2);
+bool is_signed = false;
+int size = 2;
+TCGv_i64 tcg_rt, tcg_addr;
+
+if (is_vector) {
+if (opc == 3) {
+unallocated_encoding(s);
+return;
+}
+size = 2 + opc;
+} else {
+if (opc == 3) {
+/* PRFM (literal) : prefetch */
+return;
+}
+size = 2 + extract32(opc, 0, 1);
+is_signed = extract32(opc, 1, 1);
+}
+
+tcg_rt = cpu_reg(s, rt);
+
+tcg_addr = tcg_const_i64((s-pc - 4) + imm);
+if (is_vector) {
+do_fp_ld(s, rt, tcg_addr, size);
+} else {
+do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, false);
+}
+tcg_temp_free_i64(tcg_addr);
}
/*
--
1.8.5
[Qemu-devel] [PATCH 14/21] target-arm: A64: Implement minimal set of EL0-visible sysregs
Implement an initial minimal set of EL0-visible system registers:
* NZCV
* FPCR
* FPSR
* CTR_EL0
* DCZID_EL0
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/cpu.h | 3 ++-
target-arm/helper.c| 58 ++
target-arm/translate-a64.c | 54 +-
3 files changed, 113 insertions(+), 2 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 5ec74b1..32387b0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -661,7 +661,8 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
#define ARM_CP_AA64 128
#define ARM_CP_NOP (ARM_CP_SPECIAL | (1 8))
#define ARM_CP_WFI (ARM_CP_SPECIAL | (2 8))
-#define ARM_LAST_SPECIAL ARM_CP_WFI
+#define ARM_CP_NZCV (ARM_CP_SPECIAL | (3 8))
+#define ARM_LAST_SPECIAL ARM_CP_NZCV
/* Used only as a terminator for ARMCPRegInfo lists */
#define ARM_CP_SENTINEL 0x
/* Mask of only the flag bits in a type field */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 72f0ee8..c64f618 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1560,6 +1560,61 @@ static const ARMCPRegInfo lpae_cp_reginfo[] = {
REGINFO_SENTINEL
};
+static int aa64_fpcr_read(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t *value)
+{
+*value = vfp_get_fpcr(env);
+return 0;
+}
+
+static int aa64_fpcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+vfp_set_fpcr(env, value);
+return 0;
+}
+
+static int aa64_fpsr_read(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t *value)
+{
+*value = vfp_get_fpsr(env);
+return 0;
+}
+
+static int aa64_fpsr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+vfp_set_fpsr(env, value);
+return 0;
+}
+
+static const ARMCPRegInfo aarch64_cp_reginfo[] = {
+/* Minimal set of EL0-visible registers. This will need to be expanded
+ * significantly for system emulation.
+ */
+{ .name = NZCV, .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 2,
+ .access = PL0_RW, .type = ARM_CP_AA64 | ARM_CP_NZCV },
+{. name = FPCR, .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 4,
+ .access = PL0_RW, .type = ARM_CP_AA64,
+ .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write },
+{. name = FPSR, .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 4, .crm = 4,
+ .access = PL0_RW, .type = ARM_CP_AA64,
+ .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write },
+/* This claims a 32 byte cacheline size for icache and dcache, VIPT icache.
+ * It will eventually need to have a CPU-specified reset value.
+ */
+{ .name = CTR_EL0, .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0,
+ .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_AA64,
+ .resetvalue = 0x80030003 },
+/* Prohibit use of DC ZVA. OPTME: implement DC ZVA and allow its use.
+ * For system mode the DZP bit here will need to be computed, not constant.
+ */
+{ .name = DCZID_EL0, .opc0 = 3, .opc1 = 3, .opc2 = 7, .crn = 0, .crm = 0,
+ .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_AA64,
+ .resetvalue = 0x10 },
+REGINFO_SENTINEL
+};
+
static int sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t
value)
{
env-cp15.c1_sys = value;
@@ -1662,6 +1717,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
} else {
define_arm_cp_regs(cpu, not_v7_cp_reginfo);
}
+if (arm_feature(env, ARM_FEATURE_AARCH64)) {
+define_arm_cp_regs(cpu, aarch64_cp_reginfo);
+}
if (arm_feature(env, ARM_FEATURE_MPU)) {
/* These are the MPU registers prior to PMSAv6. Any new
* PMSA core later than the ARM946 will require that we
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index dd37962..a7d2b60 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -738,6 +738,50 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
unsupported_encoding(s, insn);
}
+static void gen_get_nzcv(TCGv_i64 tcg_rt)
+{
+TCGv_i32 tmp = tcg_temp_new_i32();
+TCGv_i32 nzcv = tcg_temp_new_i32();
+
+/* build bit 31, N */
+tcg_gen_andi_i32(nzcv, cpu_NF, (1 31));
+/* build bit 30, Z */
+tcg_gen_setcondi_i32(TCG_COND_EQ, tmp, cpu_ZF, 0);
+tcg_gen_deposit_i32(nzcv, nzcv, tmp, 30, 1);
+/* build bit 29, C */
+tcg_gen_deposit_i32(nzcv, nzcv, cpu_CF, 29, 1);
+/* build bit 28, V */
+tcg_gen_shri_i32(tmp, cpu_VF, 31);
+tcg_gen_deposit_i32(nzcv, nzcv, tmp, 28, 1);
+/* generate result */
+tcg_gen_extu_i32_i64(tcg_rt, nzcv);
+
+tcg_temp_free_i32(nzcv);
+tcg_temp_free_i32(tmp);
+}
+
+static void gen_set_nzcv(TCGv_i64 tcg_rt)
+
+{
+TCGv_i32 nzcv = tcg_temp_new_i32();
+
+/* take NZCV from R[t] */
+tcg_gen_trunc_i64_i32(nzcv, tcg_rt);
+
+/* bit 31, N */
+tcg_gen_andi_i32(cpu_NF, nzcv,
Re: [Qemu-devel] [PATCH] qemu will core dump with -smp 254, , sockets=2, cores=3, threads=2
As Eric and Eduardo's suggestions, use is_power_of_2 to check whether
nr_cores
and nr_threads is the power of 2 in function x86_apicid_from_cpu_idx in file
target-i386/topology.h. This check is very simple, I prefer add it in a
function to write a new function. Thanks for Eric and Eduardo.
Best Regards,
Jun Li
Signed-off-by: Jun Li junm...@gmail.com
---
target-i386/topology.h | 8
1 file changed, 8 insertions(+)
diff --git a/target-i386/topology.h b/target-i386/topology.h
index 07a6c5f..ff21a98 100644
--- a/target-i386/topology.h
+++ b/target-i386/topology.h
@@ -126,6 +126,14 @@ static inline apic_id_t
x86_apicid_from_cpu_idx(unsigned nr_cores,
unsigned cpu_index)
{
unsigned pkg_id, core_id, smt_id;
+
+/* Check whether nr_cores and nr_threads is a power of 2.
+ * If not, 1 is assigned to them.
+ */
+nr_cores = is_power_of_2(nr_cores) 0 ? nr_cores : 1;
+nr_threads = is_power_of_2(nr_threads) 0 ? nr_threads : 1;
+
+
x86_topo_ids_from_idx(nr_cores, nr_threads, cpu_index,
pkg_id, core_id, smt_id);
return apicid_from_topo_ids(nr_cores, nr_threads, pkg_id, core_id,
smt_id);
--
1.8.3.1
[Qemu-devel] [PATCH 02/38] memory: cpu_physical_memory_set_dirty_flags() result is never used
So return void.
Signed-off-by: Juan Quintela quint...@redhat.com
Reviewed-by: Orit Wasserman owass...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
include/exec/memory-internal.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index d0e0633..c71a5e6 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -70,10 +70,10 @@ static inline int cpu_physical_memory_get_dirty(ram_addr_t
start,
return ret;
}
-static inline int cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
+static inline void cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
int dirty_flags)
{
-return ram_list.phys_dirty[addr TARGET_PAGE_BITS] |= dirty_flags;
+ram_list.phys_dirty[addr TARGET_PAGE_BITS] |= dirty_flags;
}
static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
--
1.8.3.1
[Qemu-devel] [PATCH v3 00/38] bitmap queue
Hi This is the last version of the bitmap patches, changes from last submission: - cleanups: now it passes checkpatch - all coments from Eric Paolo addressed - fixed problem with DIRTY_MEMORY_NUM - tested by Vinod and numbers are very good - move bitmap operations to take long as index - round ram_addr's of blocks as suggesetd by Paolo. If there is no negative comments, will sent for inclusion. Thanks, Juan. [v2] In this version: - fixed all the comments from last versions (thanks Eric) - kvm migration bitmap is synchronized using bitmap operations - qemu bitmap - migration bitmap is synchronized using bitmap operations If bitmaps are not properly aligned, we fall back to old code. Code survives virt-tests, so should be in quite good shape. ToDo list: - vga ram by default is not aligned in a page number multiple of 64, it could be optimized. Kraxel? It syncs the kvm bitmap at least 1 a second or so? bitmap is only 2048 pages (16MB by default). We need to change the ram_addr only - vga: still more, after we finish migration, vga code continues synchronizing the kvm bitmap on source machine. Notice that there is no graphics client connected to the VGA. Worth investigating? - I haven't yet meassure speed differences on big hosts. Vinod? - Depending of performance, more optimizations to do. - debugging printf's still on the code, just to see if we are taking (or not) the optimized paths. And that is all. Please test comment. Thanks, Juan. The following changes since commit f46e720a82ccdf1a521cf459448f3f96ed895d43: qemu_opts_parse(): always check return value (2013-12-16 15:33:48 -0800) are available in the git repository at: git://github.com/juanquintela/qemu.git bitmap.next for you to fetch changes up to 197a77684c35f75e43d937220ffac72101783384: ram: align ram_addr_t's regions in multiples of 64 (2013-12-17 15:56:04 +0100) Juan Quintela (38): bitmap: use long as index memory: cpu_physical_memory_set_dirty_flags() result is never used memory: cpu_physical_memory_set_dirty_range() return void exec: use accessor function to know if memory is dirty memory: create function to set a single dirty bit exec: create function to get a single dirty bit memory: make cpu_physical_memory_is_dirty return bool memory: all users of cpu_physical_memory_get_dirty used only one flag memory: set single dirty flags when possible memory: cpu_physical_memory_set_dirty_range() always dirty all flags memory: cpu_physical_memory_mask_dirty_range() always clears a single flag memory: use bit 2 for migration memory: make sure that client is always inside range memory: only resize dirty bitmap when memory size increases memory: cpu_physical_memory_clear_dirty_flag() result is never used bitmap: Add bitmap_zero_extend operation memory: split dirty bitmap into three memory: unfold cpu_physical_memory_clear_dirty_flag() in its only user memory: unfold cpu_physical_memory_set_dirty() in its only user memory: unfold cpu_physical_memory_set_dirty_flag() memory: make cpu_physical_memory_get_dirty() the main function memory: cpu_physical_memory_get_dirty() is used as returning a bool memory: s/mask/clear/ cpu_physical_memory_mask_dirty_range memory: use find_next_bit() to find dirty bits memory: cpu_physical_memory_set_dirty_range() now uses bitmap operations memory: cpu_physical_memory_clear_dirty_range() now uses bitmap operations memory: s/dirty/clean/ in cpu_physical_memory_is_dirty() memory: make cpu_physical_memory_reset_dirty() take a length parameter memory: cpu_physical_memory_set_dirty_tracking() should return void memory: split cpu_physical_memory_* functions to its own include memory: unfold memory_region_test_and_clear() kvm: use directly cpu_physical_memory_* api for tracking dirty pages kvm: refactor start address calculation memory: move bitmap synchronization to its own function memory: syncronize kvm bitmap using bitmaps operations ram: split function that synchronizes a range migration: synchronize memory bitmap 64bits at a time ram: align ram_addr_t's regions in multiples of 64 arch_init.c| 52 --- cputlb.c | 11 +-- exec.c | 78 +++--- include/exec/cpu-all.h | 3 +- include/exec/memory-internal.h | 90 - include/exec/memory.h | 12 ++-- include/exec/ram_addr.h| 147 + include/qemu/bitmap.h | 86 +--- include/qemu/bitops.h | 14 ++-- kvm-all.c | 28 ++-- memory.c | 17 ++--- util/bitmap.c | 60
[Qemu-devel] [PATCH 05/38] memory: create function to set a single dirty bit
Signed-off-by: Juan Quintela quint...@redhat.com
Reviewed-by: Orit Wasserman owass...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
cputlb.c | 2 +-
include/exec/memory-internal.h | 6 ++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/cputlb.c b/cputlb.c
index fff0afb..72452e5 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -137,7 +137,7 @@ void tlb_protect_code(ram_addr_t ram_addr)
void tlb_unprotect_code_phys(CPUArchState *env, ram_addr_t ram_addr,
target_ulong vaddr)
{
-cpu_physical_memory_set_dirty_flags(ram_addr, CODE_DIRTY_FLAG);
+cpu_physical_memory_set_dirty_flag(ram_addr, CODE_DIRTY_FLAG);
}
static bool tlb_is_dirty_ram(CPUTLBEntry *tlbe)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index c71a5e6..4ebab80 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -76,6 +76,12 @@ static inline void
cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
ram_list.phys_dirty[addr TARGET_PAGE_BITS] |= dirty_flags;
}
+static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
+ int dirty_flag)
+{
+ram_list.phys_dirty[addr TARGET_PAGE_BITS] |= dirty_flag;
+}
+
static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
{
cpu_physical_memory_set_dirty_flags(addr, 0xff);
--
1.8.3.1
