Re: [Qemu-devel] [PATCH] qmp: add BLOCK_MEDIUM_EJECT event
Am 30.01.2012 16:55, schrieb Paolo Bonzini: On 01/30/2012 04:43 PM, Kevin Wolf wrote: If we had force=true in the initial eject command, bdrv_close is called, which in turn goes through bdrv_dev_change_media_cb where an event is emitted. Can't this race with the guest eject? Can't see how, there's nothing asynchronous in the path. The button press is sent down to the guest even with -f. Right, but there's no race. The guest will always be later. Kevin
Re: [Qemu-devel] [PATCH v11 4/9] ARM: exynos4210: PWM support.
On 01/30/2012 11:38 AM, Evgeny Voevodin wrote: Signed-off-by: Evgeny Voevodine.voevo...@samsung.com Reviewed-by: Peter Maydellpeter.mayd...@linaro.org --- This patch should not contain Reviewed-by: since QOM usage was added. Apologise for it. Peter, could you, please, rereview it? -- Kind regards, Evgeny Voevodin, Leading Software Engineer, ASWG, Moscow RD center, Samsung Electronics e-mail: e.voevo...@samsung.com
Re: [Qemu-devel] git bisect results
On January 30, 2012 at 3:48 PM Jan Kiszka jan.kis...@siemens.com wrote: On 2012-01-30 15:17, Erik Rull wrote: On January 30, 2012 at 2:48 PM Jan Kiszka jan.kis...@siemens.com wrote: On 2012-01-30 14:17, Erik Rull wrote: On January 30, 2012 at 12:52 PM Jan Kiszka jan.kis...@siemens.com wrote: On 2012-01-30 12:34, Erik Rull wrote: Hi Jan, I'm sorry, but this does not solve my issue. I applied the patch and crosschecked that the resulting file looks fine. The final function looks like: static void sdl_grab_start(void) { /* * If the application is not active, do not try to enter grab state. This * prevents 'SDL_WM_GrabInput(SDL_GRAB_ON)' from blocking all the * application (SDL bug). */ if (!(SDL_GetAppState() SDL_APPINPUTFOCUS)) { return; } if (guest_cursor) { SDL_SetCursor(guest_sprite); if (!kbd_mouse_is_absolute() !absolute_enabled) SDL_WarpMouse(guest_x, guest_y); } else sdl_hide_cursor(); SDL_WM_GrabInput(SDL_GRAB_ON); gui_grab = 1; sdl_update_caption(); } That makes no sense as gui_grab must be 1 now. Please retry your previous instrumentation. Thanks, Jan You're right. So I added the instrumentation again. Still looks strange. So I added into the sdl_grab_start() a printf. Wow - a lot of output! This pointed me to all other sdl_grab_start() calls (and in additon to that all sdl_grab_end() calls). And here are the results of the qemu voting :-) I already assigned a usable name to the printf output that is directly one line above the corresponding sdl_grab_*() call, so you should be able to find this easily in your code as well. The huge number of recurring printf's are: sdl_grab_start() called from absolute_mouse_grab() sdl_grab_end() called from handle_activation() sdl_grab_start() called from absolute_mouse_grab() sdl_grab_end() called from handle_activation() sdl_grab_start() called from absolute_mouse_grab() sdl_grab_end() called from handle_activation() sdl_grab_start() called from absolute_mouse_grab() sdl_grab_end() called from handle_activation() sdl_grab_start() called from absolute_mouse_grab() sdl_grab_end() called from handle_activation() sdl_grab_start() called from absolute_mouse_grab() sdl_grab_end() called from handle_activation() sdl_grab_start() called from absolute_mouse_grab() sdl_grab_end() called from handle_activation() Any idea how to proceed? Maybe the first two if-statements in handle_activation() cause the problem? Because there the two given functions are called in sequence if both if-clauses are valid one after the other. Maybe the first one sets the state so that the second if is valid, too. Maybe a simple else if solves the issue? ev-active.gain makes both clauses mutually exclusive - unless someone messes with the memory of the event object. I'm not familiar with the variables that are checked here, so it's just a guess. So handle_activation() is called directly after absolute_mouse_grab(), and the reported event contains state = SDL_APPINPUTFOCUS gain = 0 (please validate!) That would mean we are constantly losing the input focus again after trying to gain it via SDL_WM_GrabInput. Weird. What's the call chain for absolute_mouse_grab()? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux Here the results: Handle Activation 0: at function start (before the first if) Handle Activation 1: between the first and the second if Handle Activation 2: after the second if The output is formatted like: printf(Handle Activation 0: %d %d %d %d\n,gui_grab,(ev-active.state == SDL_APPINPUTFOCUS),ev-active.gain,gui_fullscreen); Handle Activation 0: 0 1 1 0 Handle Activation 1: 0 1 1 0 absolute_mouse_grab() called from handle_activation() sdl_grab_start() called from absolute_mouse_grab() Handle Activation 2: 1 1 1 0 Handle Activation 0: 1 1 0 0 sdl_grab_end() called from handle_activation() Handle Activation 1: 0 1 0 0 Handle Activation 2: 0 1 0 0 Handle Activation 0: 0 1 1 0 Handle Activation 1: 0 1 1 0 absolute_mouse_grab() called from handle_activation() sdl_grab_start() called from absolute_mouse_grab() Handle Activation 2: 1 1 1 0 Handle Activation 0: 1 1 0 0 sdl_grab_end() called from handle_activation() Handle Activation 1: 0 1 0 0 Handle Activation 2: 0 1 0 0 Handle Activation 0: 0 1 1 0 Handle Activation 1: 0 1 1 0 absolute_mouse_grab() called from handle_activation() sdl_grab_start() called from absolute_mouse_grab() Handle Activation 2: 1 1 1 0 The gain seems to toggle between the successive calls of handle_activation... Next steps? Try this: diff --git a/ui/sdl.c b/ui/sdl.c index 73e5839..c3fe80d 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -828,10 +828,6 @@ static void handle_mousebutton(DisplayState *ds, SDL_Event
Re: [Qemu-devel] [PATCH v11 6/9] ARM: exynos4210: MCT support.
On 01/30/2012 11:38 AM, Evgeny Voevodin wrote: Signed-off-by: Evgeny Voevodine.voevo...@samsung.com Reviewed-by: Peter Maydellpeter.mayd...@linaro.org This patch should not contain Reviewed-by: since QOM usage was added. Apologise for it. Peter, could you, please, rereview it? -- Kind regards, Evgeny Voevodin, Leading Software Engineer, ASWG, Moscow RD center, Samsung Electronics e-mail: e.voevo...@samsung.com
Re: [Qemu-devel] [PATCH v11 9/9] Exynos4210: added display controller implementation
On 01/30/2012 11:38 AM, Evgeny Voevodin wrote: From: Mitsyanko Igori.mitsya...@samsung.com Exynos4210 display controller (FIMD) has 5 hardware windows with alpha and chroma key blending functions. Signed-off-by: Mitsyanko Igori.mitsya...@samsung.com Reviewed-by: Peter Maydellpeter.mayd...@linaro.org Signed-off-by: Evgeny Voevodine.voevo...@samsung.com This patch should not contain Reviewed-by: since QOM usage was added. Apologise for it. Peter, could you, please, rereview it? -- Kind regards, Evgeny Voevodin, Leading Software Engineer, ASWG, Moscow RD center, Samsung Electronics e-mail: e.voevo...@samsung.com
Re: [Qemu-devel] nested page table translation for non-x86 operating system
On Fri, Jan 20, 2012 at 08:54:12AM -0500, Xin Tong wrote: On Fri, Jan 20, 2012 at 3:23 AM, 陳韋任 che...@iis.sinica.edu.tw wrote: 1. The control of gCR3 and hCR3 needs kernel access. While they can be set with a device module as what is done in kvm. Trapping into the kernel every time gCR3 is reseted might be too expensive. Why the control of gCR3 needs kernel access? Isn't gCR3 just a field of the CPUX86State? QEMU should have the control of it. Or you mean the trapping thing? I do not think gCR3 is a field in the CPUx86State. I think inorder to change the guest CR3, we need to trap into the kernel as kvm does. If your scenario is pure QEMU (without kvm), I think gCR3 is a field in the CPUx86State. See below, typedef struct CPUX86State { ... target_ulong cr[5]; /* NOTE: cr1 is unused */ ... }; Or I misunderstand what you're trying to do? Regards, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] [PATCH 22/23] container: make a decendent of Object
Am 31.01.2012 08:50, schrieb Paolo Bonzini: On 01/30/2012 10:09 PM, Anthony Liguori wrote: diff --git a/qom/container.c b/qom/container.c new file mode 100644 index 000..39d7b1e --- /dev/null +++ b/qom/container.c @@ -0,0 +1,15 @@ +#include qemu/object.h +#include module.h + +static TypeInfo container_info = { +.name = container, +.instance_size = sizeof(Object), +.parent= TYPE_OBJECT, +}; + License header, please. (GPLv2+ or LGPLv2+?) Given that object.c is GPLv2+, I've sticked with that license myself; but was that choice of license intentional? Effectively means we can't really have any liberally-licensed device emulation. (But same holds for MemoryAPI.) Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] qmp: add BLOCK_MEDIUM_EJECT event
Kevin Wolf kw...@redhat.com writes: Am 30.01.2012 16:18, schrieb Luiz Capitulino: On Fri, 27 Jan 2012 10:52:15 +0100 Kevin Wolf kw...@redhat.com wrote: Am 26.01.2012 18:57, schrieb Luiz Capitulino: On Wed, 25 Jan 2012 10:42:04 -0200 Luiz Capitulino lcapitul...@redhat.com wrote: On Wed, 25 Jan 2012 09:41:20 +0100 Kevin Wolf kw...@redhat.com wrote: Am 24.01.2012 21:03, schrieb Eric Blake: On 01/24/2012 11:16 AM, Luiz Capitulino wrote: Libvirt wants to be notified when the guest ejects a medium, so that it can update its view of the guest. This code has been originally written by Daniel Berrange. It adds the event to IDE and SCSI emulation. Please, note that this only covers guest initiated ejects, that's, the QMP/HMP commands 'eject' and 'change' are not covered. What's the reason for this behaviour? It feels inconsistent. I don't think it's inconsistent because we're limiting it to guest initiated actions. Also, the mngt app knows when it sends a 'eject' or 'change' command. The exception is if it allows HMP to run in parallel with QMP, but I don't think this is recommended (at least not for commands that change any VM state). Let me elaborate more. We have two options: 1. Emit the event for guest initiated ejects (this patch, although I think the event should be renamed to GUEST_MEDIUM_EJECT) 2. Emit the event for guest QMP initiated ejects, that's, also emit the event for the eject and change commands The first thing to note is that, they're not mutually exclusive. If we do item 1 now, we can add 2 later (as a different event). But my point is that doing 2 is problematic and we should avoid it. The problem is that the semantics of eject and change are complex and/or buggy. And something I've learned about confusing semantics in QMP is that, they will give you headaches soon or later. But I'm not really interested in the semantics of QMP commands, because they are irrelevant for the events. I do think they are relevant, because the event will have to match what the eject/change commands do with the tray. If what they do is messy, the event will turn out to be messy too. Now, I don't doubt this can be all fixed and made clean. I'm just not sure if it's worth it. If a mess best describes to the outside what we're doing to the device, then having a messy event is the best you can expect. Or in other words, if you're doing messy things with the device and you straighten things out in the event generation, then your events are lying to the management tools. Yup. The event is merely a passive sensor. If reality is messy, sensor data better reflect that. Maybe it's easier to understand from a distance, so let's examine a more distant example: filesystem event monitoring with inotify(), specifically file permissions change. The event is simple enough: you get it when file permissions change. Now imagine chmod(2) was improved to refuse to set write bits unless read bits are also set, but only on Tuedays. That's a messy chmod() indeed. But the event remains as simple and clean as ever: you still get it when file permissions change. Back to QMP. In my opinion, the simple and clean event is tray moved. Emit it when the tray moves, regardless of what made it move. Yes, the management app gets the event even when it itself directly triggered the move by commanding a media eject. That's a feature. It also gets the event when its media eject command actually becomes a polite request to the guest OS, because the tray happens to be locked, and the guest OS complies. That's a feature, too. [...]
Re: [Qemu-devel] [PATCH v3 1/9] fdc: take side count into account
Hervé Poussineau hpous...@reactos.org writes: Markus Armbruster a écrit : Hervé Poussineau hpous...@reactos.org writes: Floppies can be simple or double-sided. However, current code was only taking the common case into account (ie 2 sides). Impact? This repairs single-sided floppies Are single sided floppies broken before the patch? How? Yes. For head 0, wrong sector number is calculated, so data is read/written at wrong place on underlying block device. Fortunately, mostly nobody is still using single-sided floppies (only some 360 kB floppies are single-sided), so bug has been neglected since a long time. Nice info, thanks. Best put it in the commit message.
[Qemu-devel] [PATCH] sheepdog: fix co_recv coroutine context
The co_recv coroutine has two things that will try to enter it: 1. The select(2) read callback on the sheepdog socket. 2. The aio_add_request() blocking operations, including a coroutine mutex. This patch fixes it by setting NULL to co_recv before sending data. In future, we should make the sheepdog driver fully coroutine-based and simplify request handling. Signed-off-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp --- block/sheepdog.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 9416400..00276f6f 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -629,6 +629,9 @@ static void coroutine_fn aio_read_response(void *opaque) switch (acb-aiocb_type) { case AIOCB_WRITE_UDATA: +/* this coroutine context is no longer suitable for co_recv + * because we may send data to update vdi objects */ +s-co_recv = NULL; if (!is_data_obj(aio_req-oid)) { break; } -- 1.7.2.5
Re: [Qemu-devel] longjmp in qemu
On Sat, Jan 28, 2012 at 05:17:56PM -0500, Xin Tong wrote: I am investigating what longjmp is used for in qemu. longjmp is used in a couple of places. 1. void cpu_loop_exit(void) { env-current_tb = NULL; longjmp(env-jmp_env, 1); } cpu_loop_exit is called when there is an interrupt_request or exit_request pending Yes. 2. void cpu_resume_from_signal(CPUState *env1, void *puc) { ... longjmp(env-jmp_env, 1); } cpu_resume_from_signal is called in a couple of places, each of which suggests something faulty has happened. my guess is that it will get call when exceptions have occurred in the code cache. Am I right ? Not exactly. `grep -r cpu_resume_from_signal` shows places using cpu_resume_from_signal. Not all of them means something faulty has happened, I think. Regards, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] TCG register allocator
On Wed, Jan 25, 2012 at 01:58:10PM -0500, Xin Tong wrote: I am working on extending coremu (parallel version of qemu). Currently, the code cache in coremu is private, I am working towards to make it shared by all cores. I think the add_tb_jump may not be atomic. If you're talking about [1], maybe you can seek for help on their mailing list too. [1] http://sourceforge.net/p/coremu/home/Home/ Regards, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
[Qemu-devel] [PATCH 08/19] linux-user: add SO_PEERCRED support for getsockopt
From: Akos PASZTORY akos.paszt...@gmail.com Signed-off-by: Akos PASZTORY akos.paszt...@gmail.com Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/syscall.c | 34 +- linux-user/syscall_defs.h |6 ++ 2 files changed, 39 insertions(+), 1 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index c6bfcd8..15b8b22 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -1530,9 +1530,41 @@ static abi_long do_getsockopt(int sockfd, int level, int optname, case TARGET_SO_LINGER: case TARGET_SO_RCVTIMEO: case TARGET_SO_SNDTIMEO: -case TARGET_SO_PEERCRED: case TARGET_SO_PEERNAME: goto unimplemented; +case TARGET_SO_PEERCRED: { +struct ucred cr; +socklen_t crlen; +struct target_ucred *tcr; + +if (get_user_u32(len, optlen)) { +return -TARGET_EFAULT; +} +if (len 0) { +return -TARGET_EINVAL; +} + +crlen = sizeof(cr); +ret = get_errno(getsockopt(sockfd, level, SO_PEERCRED, + cr, crlen)); +if (ret 0) { +return ret; +} +if (len crlen) { +len = crlen; +} +if (!lock_user_struct(VERIFY_WRITE, tcr, optval_addr, 0)) { +return -TARGET_EFAULT; +} +__put_user(cr.pid, tcr-pid); +__put_user(cr.uid, tcr-uid); +__put_user(cr.gid, tcr-gid); +unlock_user_struct(tcr, optval_addr, 1); +if (put_user_u32(len, optlen)) { +return -TARGET_EFAULT; +} +break; +} /* Options with 'int' argument. */ case TARGET_SO_DEBUG: optname = SO_DEBUG; diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h index 2857805..41f0ff8 100644 --- a/linux-user/syscall_defs.h +++ b/linux-user/syscall_defs.h @@ -2336,3 +2336,9 @@ struct target_rlimit64 { uint64_t rlim_cur; uint64_t rlim_max; }; + +struct target_ucred { +uint32_t pid; +uint32_t uid; +uint32_t gid; +}; -- 1.7.5.4
Re: [Qemu-devel] [PATCH] rtl8139: honor RxOverflow flag in can_receive method
2012/1/30 Fernando Luis Vázquez Cao ferna...@oss.ntt.co.jp: Some drivers (Linux' 8139too among them) rely on the NIC injecting an interrupt in the event of a receive buffer overflow and, accordingly, set the RxOverflow bit in the interrupt mask. Unfortunately rtl8139's can_receive method ignores the RxOverflow flag, which may lead to a situation where rtl8139 stops receiving packets (can_receive returns 0) when the receive buffer becomes full. If the driver eventually read from the receive buffer or reset the card the emulator could recover from this situation. However some implementations only do this upon receiving an interrupt with either RxOK or RxOverflow set in the ISR; interrupt that will never come because QEMU's flow control mechanisms would prevent rtl8139 from receiving any packet. Letting packets go through when the overflow interrupt is enabled makes the QEMU emulator compliant to the spec and solves the problem. This patch should fix a relatively common (in our experience) network stall observed when running enterprise distros with rtl8139 as the NIC; in some cases the 8139too device driver gets loaded and when under heavy load the network eventually stops working. It would be great to see specific example to verify the issue. Otherwise the change looks great. -- Kind regards, Igor V. Kovalenko
Re: [Qemu-devel] [PATCH v3 5/9] fdc: add CCR (Configuration Control Register) write register
Hervé Poussineau hpous...@reactos.org writes: Markus Armbruster a écrit : Hervé Poussineau hpous...@reactos.org writes: DIR and CCR registers share the same address ; DIR is read-only while CCR is write-only Looks like guest writes to CCR are silently ignored before this patch. Is that correct? Yes. Impact? CCR is only used to program media rate, which is not checked without this patchset. So, none for this patch. Aha, it's just a prerequisite for the rest of the series. Makes sense. Explain in commit message?
[Qemu-devel] [PATCH 10/19] linux-user/strace.c: Correct errno printing for mmap etc
From: Peter Maydell peter.mayd...@linaro.org Correct the printing of errnos for syscalls which are handled via print_syscall_ret_addr (mmap, mmap2, brk, shmat): errnos are returned as negative returned values at this level, not via the host 'errno' variable. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/strace.c |9 - 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/linux-user/strace.c b/linux-user/strace.c index 269481e..05a0d3e 100644 --- a/linux-user/strace.c +++ b/linux-user/strace.c @@ -1,5 +1,4 @@ #include stdio.h -#include errno.h #include sys/ipc.h #include sys/msg.h #include sys/sem.h @@ -286,11 +285,11 @@ print_syscall_ret_addr(const struct syscallname *name, abi_long ret) { char *errstr = NULL; -if (ret == -1) { -errstr = target_strerror(errno); +if (ret 0) { +errstr = target_strerror(-ret); } -if ((ret == -1) errstr) { -gemu_log( = -1 errno=%d (%s)\n, errno, errstr); +if (errstr) { +gemu_log( = -1 errno=%d (%s)\n, (int)-ret, errstr); } else { gemu_log( = 0x TARGET_ABI_FMT_lx \n, ret); } -- 1.7.5.4
[Qemu-devel] [PATCH 07/19] linux-user/main.c: Add option to user-mode emulation so that user can specify log file name
From: 陳韋任 che...@iis.sinica.edu.tw QEMU linux user-mode's default log file name is /tmp/qemu.log. In order to change the log file name, user need to modify the source code then recompile QEMU. This patch allow user use -D logfile option to specify the log file name. Signed-off-by: Chen Wen-Ren che...@iis.sinica.edu.tw Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/main.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/linux-user/main.c b/linux-user/main.c index 64d2208..14bf5f0 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -2945,6 +2945,11 @@ static void handle_arg_log(const char *arg) cpu_set_log(mask); } +static void handle_arg_log_filename(const char *arg) +{ +cpu_set_log_filename(arg); +} + static void handle_arg_set_env(const char *arg) { char *r, *p, *token; @@ -3125,6 +3130,8 @@ struct qemu_argument arg_table[] = { #endif {d, QEMU_LOG, true, handle_arg_log, options,activate log}, +{D, QEMU_LOG_FILENAME, true, handle_arg_log_filename, + logfile, override default logfile location}, {p, QEMU_PAGESIZE,true, handle_arg_pagesize, pagesize, set the host page size to 'pagesize'}, {singlestep, QEMU_SINGLESTEP, false, handle_arg_singlestep, -- 1.7.5.4
[Qemu-devel] qemu(-dm): aborting on wrong mmio size?
Hi, in the qemu-xen-unstable tree (git://xenbits.xen.org/qemu-xen-unstable.git), the do_inp() function [i386-dm/helper2.c] makes the process exit if the operand size is wrong. Blame: 6040eea5 (More files imported from xen-unstable 17192:59b8768d0d0d). In the qemu tree (git://git.qemu.org/qemu.git), the do_inp() function [xen-all.c] does the same (via hw_error() / abort()). Blame: 9ce94e7c (xen: Initialize event channels and io rings). Is it justified to kill the emulator when this happens (eg. memory mapped IO with 64-bit operand)? What would happen on real hardware? If it's undefined, wouldn't it be better (for some definition of better) to return a constant? Thank you, Laszlo
Re: [Qemu-devel] [PATCH 22/23] container: make a decendent of Object
On 01/31/2012 10:21 AM, Andreas Färber wrote: License header, please. (GPLv2+ or LGPLv2+?) Given that object.c is GPLv2+, I've sticked with that license myself; but was that choice of license intentional? Effectively means we can't really have any liberally-licensed device emulation. (But same holds for MemoryAPI.) Actually you can. It's the combination that cannot be released liberally, but that's already the case without QOM or MemoryRegion. If somebody wishes to port such a module to a proprietary platform (which doesn't have QOM or other GPL bits), they can. Paolo
Re: [Qemu-devel] [PATCH v3 0/9] Misc fixes for floppy emulation
Looks sane, except for the migration of media rate. See my reply to 7/9.
[Qemu-devel] [PATCH 01/19] linux-user: stack_base is now mandatory on all targets
From: Riku Voipio riku.voi...@linaro.org --- linux-user/qemu.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/linux-user/qemu.h b/linux-user/qemu.h index 55ad9d8..30e2abd 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -123,10 +123,10 @@ typedef struct TaskState { #endif #if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32) /* Extra fields for semihosted binaries. */ -uint32_t stack_base; uint32_t heap_base; uint32_t heap_limit; #endif +uint32_t stack_base; int used; /* non zero if used */ struct image_info *info; struct linux_binprm *bprm; -- 1.7.5.4
[Qemu-devel] [PATCH 15/19] linux-user: Add default-configs for mipsn32[el]
From: Andreas Färber afaer...@suse.de Prepares for mipsn32[el]-linux-user targets. Signed-off-by: Ulricht Hecht u...@suse.de Signed-off-by: Andreas Färber afaer...@suse.de Signed-off-by: Riku Voipio riku.voi...@linaro.org --- default-configs/mipsn32-linux-user.mak |1 + default-configs/mipsn32el-linux-user.mak |1 + 2 files changed, 2 insertions(+), 0 deletions(-) create mode 100644 default-configs/mipsn32-linux-user.mak create mode 100644 default-configs/mipsn32el-linux-user.mak diff --git a/default-configs/mipsn32-linux-user.mak b/default-configs/mipsn32-linux-user.mak new file mode 100644 index 000..5b97919 --- /dev/null +++ b/default-configs/mipsn32-linux-user.mak @@ -0,0 +1 @@ +# Default configuration for mipsn32-linux-user diff --git a/default-configs/mipsn32el-linux-user.mak b/default-configs/mipsn32el-linux-user.mak new file mode 100644 index 000..d6367ff --- /dev/null +++ b/default-configs/mipsn32el-linux-user.mak @@ -0,0 +1 @@ +# Default configuration for mipsn32el-linux-user -- 1.7.5.4
[Qemu-devel] [PATCH 11/19] linux-user: fix wait* syscall status returns
From: Alexander Graf ag...@suse.de When calling wait4 or waitpid with a status pointer and WNOHANG, the syscall can potentially not modify the status pointer input. Now if we have guest code like: int status = 0; waitpid(pid, status, WNOHANG); if (status) breakage then we have to make sure that in case status did not change we actually return the guest's initialized status variable instead of our own uninitialized. We fail to do so today, as we proxy everything through an uninitialized status variable which for me ended up always containing the last error code. This patch fixes some test cases when building yast2-core in OBS for ARM. Signed-off-by: Alexander Graf ag...@suse.de Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/syscall.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 29d92c4..06b19e0 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -4867,7 +4867,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { int status; ret = get_errno(waitpid(arg1, status, arg3)); -if (!is_error(ret) arg2 +if (!is_error(ret) arg2 ret put_user_s32(host_to_target_waitstatus(status), arg2)) goto efault; } @@ -6423,7 +6423,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, rusage_ptr = NULL; ret = get_errno(wait4(arg1, status, arg3, rusage_ptr)); if (!is_error(ret)) { -if (status_ptr) { +if (status_ptr ret) { status = host_to_target_waitstatus(status); if (put_user_s32(status, status_ptr)) goto efault; -- 1.7.5.4
Re: [Qemu-devel] [RFC/PATCH] Fix guest OS panic when 64bit BAR is present
On 01/27/2012 06:42 AM, Alexey Korolev wrote: On 27/01/12 04:12, Avi Kivity wrote: On 01/26/2012 04:36 PM, Michael S. Tsirkin wrote: On Thu, Jan 26, 2012 at 03:52:27PM +0200, Avi Kivity wrote: On 01/26/2012 11:14 AM, Michael S. Tsirkin wrote: On Wed, Jan 25, 2012 at 06:46:03PM +1300, Alexey Korolev wrote: Hi, In this post http://lists.gnu.org/archive/html/qemu-devel/2011-12/msg03171.html I've mentioned about the issues when 64Bit PCI BAR is present and 32bit address range is selected for it. The issue affects all recent qemu releases and all old and recent guest Linux kernel versions. We've done some investigations. Let me explain what happens. Assume we have 64bit BAR with size 32MB mapped at [0xF000 - 0xF200] When Linux guest starts it does PCI bus enumeration. The OS enumerates 64BIT bars using the following procedure. 1. Write all FF's to lower half of 64bit BAR 2. Write address back to lower half of 64bit BAR 3. Write all FF's to higher half of 64bit BAR 4. Write address back to higher half of 64bit BAR Linux code is here: http://lxr.linux.no/#linux+v3.2.1/drivers/pci/probe.c#L149 What does it mean for qemu? At step 1. qemu pci_default_write_config() recevies all FFs for lower part of the 64bit BAR. Then it applies the mask and converts the value to All FF's - size + 1 (FE00 if size is 32MB). Then pci_bar_address() checks if BAR address is valid. Since it is a 64bit bar it reads 0xFE00 - this address is valid. So qemu updates topology and sends request to update mappings in KVM with new range for the 64bit BAR FE00 - 0x. This usually means kernel panic on boot, if there is another mapping in the FE00 - 0x range, which is quite common. Do you know why does it panic? As far as I can see from code at http://lxr.linux.no/#linux+v2.6.35.9/drivers/pci/probe.c#L162 171pci_read_config_dword(dev, pos, l); 172pci_write_config_dword(dev, pos, l | mask); 173pci_read_config_dword(dev, pos, sz); 174pci_write_config_dword(dev, pos, l); BAR is restored: what triggers an access between lines 172 and 174? Random interrupt reading the time, likely. Weird, what the backtrace shows is init, unrelated to interrupts. It's a bug then. qemu doesn't undo the mapping correctly. If you have clear instructions, I'll try to reproduce it. Well the easiest way to reproduce this is: 1. Get kernel bzImage (version 2.6.36) 2. Apply patch to ivshmem.c --- diff --git a/hw/ivshmem.c b/hw/ivshmem.c index 1aa9e3b..71f8c21 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -341,7 +341,7 @@ static void create_shared_memory_BAR(IVShmemState *s, int fd) { memory_region_add_subregion(s-bar, 0, s-ivshmem); /* region for shared memory */ -pci_register_bar(s-dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, s-bar); +pci_register_bar(s-dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY|PCI_BASE_ADDRESS_MEM_TYPE_64, s-bar) } static void close_guest_eventfds(IVShmemState *s, int posn) --- 3. Launch qemu with a command like that /usr/bin/qemu-system-x86_64 -M pc-0.14 -enable-kvm -m 2048 -smp 1,socket=1,cores=1,threads=1 -name centos54 -uuid d37daefd-75bd-4387-cee1-7f0b153ee2af -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/centos54.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -rtc base=utc -drive file=/dev/dock200-1/centos54,if=none,id=drive-ide0-0-0,format=raw -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 -drive file=/data/CentOS-5.4-x86_64-bin-DVD.iso,if=none,media=cdrom,id=drive-ide0-1-0,readonly=on,format=raw -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -chardev file,id=charserial0,path=/home/alexey/cent54.log -device isa-serial,chardev=charserial0,id=serial0 -usb -vnc 127.0.0.1:0 -k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x4.0x0 --device ivshmem,size=32,shm=shm -kernel bzImage -append root=/dev/hda1 console=ttyS0,115200n8 console=tty0 in other words add: --device ivshmem,size=32,shm=shm That is all. Note: it won't necessary cause panic message on some kernels it just hangs or reboots. In fact qemu segfaults for me, since registering a ram region not on a page boundary is broken. This happens when the ivshmem bar is split by the hpet region, which is less than page long. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH 14/19] linux-user: Implement *listxattr syscalls
From: Peter Maydell peter.mayd...@linaro.org Implement listxattr, flistxattr and llistxattr syscalls. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/syscall.c | 36 +++- 1 files changed, 35 insertions(+), 1 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 762115b..ee8899e 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -7798,9 +7798,43 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, #ifdef TARGET_NR_setxattr case TARGET_NR_listxattr: case TARGET_NR_llistxattr: +{ +void *p, *b = 0; +if (arg2) { +b = lock_user(VERIFY_WRITE, arg2, arg3, 0); +if (!b) { +ret = -TARGET_EFAULT; +break; +} +} +p = lock_user_string(arg1); +if (p) { +if (num == TARGET_NR_listxattr) { +ret = get_errno(listxattr(p, b, arg3)); +} else { +ret = get_errno(llistxattr(p, b, arg3)); +} +} else { +ret = -TARGET_EFAULT; +} +unlock_user(p, arg1, 0); +unlock_user(b, arg2, arg3); +break; +} case TARGET_NR_flistxattr: -ret = -TARGET_EOPNOTSUPP; +{ +void *b = 0; +if (arg2) { +b = lock_user(VERIFY_WRITE, arg2, arg3, 0); +if (!b) { +ret = -TARGET_EFAULT; +break; +} +} +ret = get_errno(flistxattr(arg1, b, arg3)); +unlock_user(b, arg2, arg3); break; +} case TARGET_NR_setxattr: case TARGET_NR_lsetxattr: { -- 1.7.5.4
[Qemu-devel] [PATCH 17/19] linux-user: Define TARGET_QEMU_ESIGRETURN for mipsn32
From: Andreas Färber afaer...@suse.de Copied from mips/syscall.h. Signed-off-by: Ulrich Hecht u...@suse.de Signed-off-by: Andreas Färber afaer...@suse.de Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/mipsn32/syscall.h |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/linux-user/mipsn32/syscall.h b/linux-user/mipsn32/syscall.h index 4ec506c..ebe98f2 100644 --- a/linux-user/mipsn32/syscall.h +++ b/linux-user/mipsn32/syscall.h @@ -218,4 +218,7 @@ struct target_pt_regs { +/* Nasty hack: define a fake errno value for use by sigreturn. */ +#define TARGET_QEMU_ESIGRETURN 255 + #define UNAME_MACHINE mips64 -- 1.7.5.4
Re: [Qemu-devel] [RFC/PATCH] Fix guest OS panic when 64bit BAR is present
On 01/31/2012 11:40 AM, Avi Kivity wrote: On 01/27/2012 06:42 AM, Alexey Korolev wrote: On 27/01/12 04:12, Avi Kivity wrote: On 01/26/2012 04:36 PM, Michael S. Tsirkin wrote: On Thu, Jan 26, 2012 at 03:52:27PM +0200, Avi Kivity wrote: On 01/26/2012 11:14 AM, Michael S. Tsirkin wrote: On Wed, Jan 25, 2012 at 06:46:03PM +1300, Alexey Korolev wrote: Hi, In this post http://lists.gnu.org/archive/html/qemu-devel/2011-12/msg03171.html I've mentioned about the issues when 64Bit PCI BAR is present and 32bit address range is selected for it. The issue affects all recent qemu releases and all old and recent guest Linux kernel versions. We've done some investigations. Let me explain what happens. Assume we have 64bit BAR with size 32MB mapped at [0xF000 - 0xF200] When Linux guest starts it does PCI bus enumeration. The OS enumerates 64BIT bars using the following procedure. 1. Write all FF's to lower half of 64bit BAR 2. Write address back to lower half of 64bit BAR 3. Write all FF's to higher half of 64bit BAR 4. Write address back to higher half of 64bit BAR Linux code is here: http://lxr.linux.no/#linux+v3.2.1/drivers/pci/probe.c#L149 What does it mean for qemu? At step 1. qemu pci_default_write_config() recevies all FFs for lower part of the 64bit BAR. Then it applies the mask and converts the value to All FF's - size + 1 (FE00 if size is 32MB). Then pci_bar_address() checks if BAR address is valid. Since it is a 64bit bar it reads 0xFE00 - this address is valid. So qemu updates topology and sends request to update mappings in KVM with new range for the 64bit BAR FE00 - 0x. This usually means kernel panic on boot, if there is another mapping in the FE00 - 0x range, which is quite common. Do you know why does it panic? As far as I can see from code at http://lxr.linux.no/#linux+v2.6.35.9/drivers/pci/probe.c#L162 171pci_read_config_dword(dev, pos, l); 172pci_write_config_dword(dev, pos, l | mask); 173pci_read_config_dword(dev, pos, sz); 174pci_write_config_dword(dev, pos, l); BAR is restored: what triggers an access between lines 172 and 174? Random interrupt reading the time, likely. Weird, what the backtrace shows is init, unrelated to interrupts. It's a bug then. qemu doesn't undo the mapping correctly. If you have clear instructions, I'll try to reproduce it. Well the easiest way to reproduce this is: 1. Get kernel bzImage (version 2.6.36) 2. Apply patch to ivshmem.c --- diff --git a/hw/ivshmem.c b/hw/ivshmem.c index 1aa9e3b..71f8c21 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -341,7 +341,7 @@ static void create_shared_memory_BAR(IVShmemState *s, int fd) { memory_region_add_subregion(s-bar, 0, s-ivshmem); /* region for shared memory */ -pci_register_bar(s-dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, s-bar); +pci_register_bar(s-dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY|PCI_BASE_ADDRESS_MEM_TYPE_64, s-bar) } static void close_guest_eventfds(IVShmemState *s, int posn) --- 3. Launch qemu with a command like that /usr/bin/qemu-system-x86_64 -M pc-0.14 -enable-kvm -m 2048 -smp 1,socket=1,cores=1,threads=1 -name centos54 -uuid d37daefd-75bd-4387-cee1-7f0b153ee2af -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/centos54.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -rtc base=utc -drive file=/dev/dock200-1/centos54,if=none,id=drive-ide0-0-0,format=raw -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 -drive file=/data/CentOS-5.4-x86_64-bin-DVD.iso,if=none,media=cdrom,id=drive-ide0-1-0,readonly=on,format=raw -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -chardev file,id=charserial0,path=/home/alexey/cent54.log -device isa-serial,chardev=charserial0,id=serial0 -usb -vnc 127.0.0.1:0 -k en-us -vga cirrus -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x4.0x0 --device ivshmem,size=32,shm=shm -kernel bzImage -append root=/dev/hda1 console=ttyS0,115200n8 console=tty0 in other words add: --device ivshmem,size=32,shm=shm That is all. Note: it won't necessary cause panic message on some kernels it just hangs or reboots. In fact qemu segfaults for me, since registering a ram region not on a page boundary is broken. This happens when the ivshmem bar is split by the hpet region, which is less than page long. Happens only with qemu-kvm for some reason. Two separate bugs. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH 12/19] linux-user: Allow NULL value pointer in setxattr and getxattr
From: Peter Maydell peter.mayd...@linaro.org It's valid to pass a NULL value pointer to setxattr, so don't fail this case EFAULT. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/syscall.c | 24 ++-- 1 files changed, 18 insertions(+), 6 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 06b19e0..0a78a18 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -7809,11 +7809,17 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, break; case TARGET_NR_setxattr: { -void *p, *n, *v; +void *p, *n, *v = 0; +if (arg3) { +v = lock_user(VERIFY_READ, arg3, arg4, 1); +if (!v) { +ret = -TARGET_EFAULT; +break; +} +} p = lock_user_string(arg1); n = lock_user_string(arg2); -v = lock_user(VERIFY_READ, arg3, arg4, 1); -if (p n v) { +if (p n) { ret = get_errno(setxattr(p, n, v, arg4, arg5)); } else { ret = -TARGET_EFAULT; @@ -7825,11 +7831,17 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, break; case TARGET_NR_getxattr: { -void *p, *n, *v; +void *p, *n, *v = 0; +if (arg3) { +v = lock_user(VERIFY_WRITE, arg3, arg4, 0); +if (!v) { +ret = -TARGET_EFAULT; +break; +} +} p = lock_user_string(arg1); n = lock_user_string(arg2); -v = lock_user(VERIFY_WRITE, arg3, arg4, 0); -if (p n v) { +if (p n) { ret = get_errno(getxattr(p, n, v, arg4)); } else { ret = -TARGET_EFAULT; -- 1.7.5.4
[Qemu-devel] [PATCH 13/19] linux-user/syscall.c: Implement f and l versions of set/get/removexattr
From: Peter Maydell peter.mayd...@linaro.org Implement the f and l versions (operate on fd, don't follow links) of the setxattr, getxattr and removexattr syscalls. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/syscall.c | 79 -- 1 files changed, 70 insertions(+), 9 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 0a78a18..762115b 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -7796,18 +7796,13 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, #endif #ifdef CONFIG_ATTR #ifdef TARGET_NR_setxattr -case TARGET_NR_lsetxattr: -case TARGET_NR_fsetxattr: -case TARGET_NR_lgetxattr: -case TARGET_NR_fgetxattr: case TARGET_NR_listxattr: case TARGET_NR_llistxattr: case TARGET_NR_flistxattr: -case TARGET_NR_lremovexattr: -case TARGET_NR_fremovexattr: ret = -TARGET_EOPNOTSUPP; break; case TARGET_NR_setxattr: +case TARGET_NR_lsetxattr: { void *p, *n, *v = 0; if (arg3) { @@ -7820,7 +7815,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, p = lock_user_string(arg1); n = lock_user_string(arg2); if (p n) { -ret = get_errno(setxattr(p, n, v, arg4, arg5)); +if (num == TARGET_NR_setxattr) { +ret = get_errno(setxattr(p, n, v, arg4, arg5)); +} else { +ret = get_errno(lsetxattr(p, n, v, arg4, arg5)); +} } else { ret = -TARGET_EFAULT; } @@ -7829,7 +7828,28 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, unlock_user(v, arg3, 0); } break; +case TARGET_NR_fsetxattr: +{ +void *n, *v = 0; +if (arg3) { +v = lock_user(VERIFY_READ, arg3, arg4, 1); +if (!v) { +ret = -TARGET_EFAULT; +break; +} +} +n = lock_user_string(arg2); +if (n) { +ret = get_errno(fsetxattr(arg1, n, v, arg4, arg5)); +} else { +ret = -TARGET_EFAULT; +} +unlock_user(n, arg2, 0); +unlock_user(v, arg3, 0); +} +break; case TARGET_NR_getxattr: +case TARGET_NR_lgetxattr: { void *p, *n, *v = 0; if (arg3) { @@ -7842,7 +7862,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, p = lock_user_string(arg1); n = lock_user_string(arg2); if (p n) { -ret = get_errno(getxattr(p, n, v, arg4)); +if (num == TARGET_NR_getxattr) { +ret = get_errno(getxattr(p, n, v, arg4)); +} else { +ret = get_errno(lgetxattr(p, n, v, arg4)); +} } else { ret = -TARGET_EFAULT; } @@ -7851,13 +7875,38 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, unlock_user(v, arg3, arg4); } break; +case TARGET_NR_fgetxattr: +{ +void *n, *v = 0; +if (arg3) { +v = lock_user(VERIFY_WRITE, arg3, arg4, 0); +if (!v) { +ret = -TARGET_EFAULT; +break; +} +} +n = lock_user_string(arg2); +if (n) { +ret = get_errno(fgetxattr(arg1, n, v, arg4)); +} else { +ret = -TARGET_EFAULT; +} +unlock_user(n, arg2, 0); +unlock_user(v, arg3, arg4); +} +break; case TARGET_NR_removexattr: +case TARGET_NR_lremovexattr: { void *p, *n; p = lock_user_string(arg1); n = lock_user_string(arg2); if (p n) { -ret = get_errno(removexattr(p, n)); +if (num == TARGET_NR_removexattr) { +ret = get_errno(removexattr(p, n)); +} else { +ret = get_errno(lremovexattr(p, n)); +} } else { ret = -TARGET_EFAULT; } @@ -7865,6 +7914,18 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, unlock_user(n, arg2, 0); } break; +case TARGET_NR_fremovexattr: +{ +void *n; +n = lock_user_string(arg2); +if (n) { +ret = get_errno(fremovexattr(arg1, n)); +} else { +ret = -TARGET_EFAULT; +} +unlock_user(n, arg2, 0); +} +break; #endif #endif /* CONFIG_ATTR */ #ifdef TARGET_NR_set_thread_area --
Re: [Qemu-devel] [PATCH 01/19] linux-user: stack_base is now mandatory on all targets
Am 31.01.2012 10:29, schrieb riku.voi...@linaro.org: From: Riku Voipio riku.voi...@linaro.org SoB missing. Andreas --- linux-user/qemu.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/linux-user/qemu.h b/linux-user/qemu.h index 55ad9d8..30e2abd 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -123,10 +123,10 @@ typedef struct TaskState { #endif #if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32) /* Extra fields for semihosted binaries. */ -uint32_t stack_base; uint32_t heap_base; uint32_t heap_limit; #endif +uint32_t stack_base; int used; /* non zero if used */ struct image_info *info; struct linux_binprm *bprm; -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH 06/19] linux-user: fake /proc/self/auxv
From: Alexander Graf ag...@suse.de Gtk tries to read /proc/self/auxv to find its auxv table instead of taking it from its own program memory space. However, when running with linux-user, we see the host's auxv which clearly exposes wrong information. so let's instead expose the guest memory backed auxv tables via /proc/self/auxv as well. Signed-off-by: Alexander Graf ag...@suse.de Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/syscall.c | 30 ++ 1 files changed, 30 insertions(+), 0 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 5a5fdac..c6bfcd8 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -4639,6 +4639,35 @@ static int open_self_stat(void *cpu_env, int fd) return 0; } +static int open_self_auxv(void *cpu_env, int fd) +{ +TaskState *ts = ((CPUState *)cpu_env)-opaque; +abi_ulong auxv = ts-info-saved_auxv; +abi_ulong len = ts-info-auxv_len; +char *ptr; + +/* + * Auxiliary vector is stored in target process stack. + * read in whole auxv vector and copy it to file + */ +ptr = lock_user(VERIFY_READ, auxv, len, 0); +if (ptr != NULL) { +while (len 0) { +ssize_t r; +r = write(fd, ptr, len); +if (r = 0) { +break; +} +len -= r; +ptr += r; +} +lseek(fd, 0, SEEK_SET); +unlock_user(ptr, auxv, len); +} + +return 0; +} + static int do_open(void *cpu_env, const char *pathname, int flags, mode_t mode) { struct fake_open { @@ -4649,6 +4678,7 @@ static int do_open(void *cpu_env, const char *pathname, int flags, mode_t mode) static const struct fake_open fakes[] = { { /proc/self/maps, open_self_maps }, { /proc/self/stat, open_self_stat }, +{ /proc/self/auxv, open_self_auxv }, { NULL, NULL } }; -- 1.7.5.4
[Qemu-devel] [PATCH 05/19] linux-user: fake /proc/self/stat
From: Alexander Graf ag...@suse.de The boehm gc finds the program's stack starting pointer by checking /proc/self/stat. Unfortunately, so far it reads qemu's stack pointer which clearly is wrong. So let's instead fake the file so the guest program sees the right address. Signed-off-by: Alexander Graf ag...@suse.de Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/syscall.c | 26 ++ 1 files changed, 26 insertions(+), 0 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 1864d7f..5a5fdac 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -4614,6 +4614,31 @@ static int open_self_maps(void *cpu_env, int fd) return 0; } +static int open_self_stat(void *cpu_env, int fd) +{ +TaskState *ts = ((CPUState *)cpu_env)-opaque; +abi_ulong start_stack = ts-info-start_stack; +int i; + +for (i = 0; i 44; i++) { + char buf[128]; + int len; + uint64_t val = 0; + + if (i == 27) { + /* stack bottom */ + val = start_stack; + } + snprintf(buf, sizeof(buf), %PRId64 %c, val, i == 43 ? '\n' : ' '); + len = strlen(buf); + if (write(fd, buf, len) != len) { + return -1; + } +} + +return 0; +} + static int do_open(void *cpu_env, const char *pathname, int flags, mode_t mode) { struct fake_open { @@ -4623,6 +4648,7 @@ static int do_open(void *cpu_env, const char *pathname, int flags, mode_t mode) const struct fake_open *fake_open; static const struct fake_open fakes[] = { { /proc/self/maps, open_self_maps }, +{ /proc/self/stat, open_self_stat }, { NULL, NULL } }; -- 1.7.5.4
[Qemu-devel] [PATCH 00/19] Pending linux-user patches
From: Riku Voipio riku.voi...@linaro.org The collection of simpler linux-user patches submitted since release of 1.0. To be sent as pull request later this week unless bugs found. Patches are also available in the git repository at: git://git.linaro.org/people/rikuvoipio/qemu.git linux-user-for-upstream Akos PASZTORY (1): linux-user: add SO_PEERCRED support for getsockopt Alexander Graf (7): linux-user: save auxv length linux-user: add open() hijack infrastructure linux-user: fake /proc/self/maps linux-user: fake /proc/self/stat linux-user: fake /proc/self/auxv linux-user: fix QEMU_STRACE=1 segfault linux-user: fix wait* syscall status returns Andreas Färber (5): linux-user: Add default-configs for mipsn32[el] linux-user: Add default configs for mips64[el] linux-user: Define TARGET_QEMU_ESIGRETURN for mipsn32 linux-user: Define TARGET_QEMU_ESIGRETURN for mips64 linux-user: Fix sa_flags byte swaps for mips Peter Maydell (4): linux-user/strace.c: Correct errno printing for mmap etc linux-user: Allow NULL value pointer in setxattr and getxattr linux-user/syscall.c: Implement f and l versions of set/get/removexattr linux-user: Implement *listxattr syscalls Riku Voipio (1): linux-user: stack_base is now mandatory on all targets 陳韋任 (1): linux-user/main.c: Add option to user-mode emulation so that user can specify log file name default-configs/mips64-linux-user.mak|1 + default-configs/mips64el-linux-user.mak |1 + default-configs/mipsn32-linux-user.mak |1 + default-configs/mipsn32el-linux-user.mak |1 + linux-user/elfload.c | 15 +- linux-user/main.c|7 + linux-user/mips64/syscall.h |3 + linux-user/mipsn32/syscall.h |3 + linux-user/qemu.h|3 +- linux-user/signal.c |8 + linux-user/strace.c | 19 ++- linux-user/syscall.c | 303 +++--- linux-user/syscall_defs.h|6 + 13 files changed, 332 insertions(+), 39 deletions(-) create mode 100644 default-configs/mips64-linux-user.mak create mode 100644 default-configs/mips64el-linux-user.mak create mode 100644 default-configs/mipsn32-linux-user.mak create mode 100644 default-configs/mipsn32el-linux-user.mak -- 1.7.5.4
[Qemu-devel] [PATCH 09/19] linux-user: fix QEMU_STRACE=1 segfault
From: Alexander Graf ag...@suse.de While debugging some issues with QEMU_STRACE I stumbled over segmentation faults that were pretty reproducible. Turns out we tried to treat a normal return value as errno, resulting in an access over array boundaries for the resolution. Fix this by allowing failure to resolve invalid errnos into strings. Signed-off-by: Alexander Graf ag...@suse.de Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/strace.c | 18 ++ linux-user/syscall.c |3 +++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/linux-user/strace.c b/linux-user/strace.c index 90027a1..269481e 100644 --- a/linux-user/strace.c +++ b/linux-user/strace.c @@ -284,8 +284,13 @@ print_ipc(const struct syscallname *name, static void print_syscall_ret_addr(const struct syscallname *name, abi_long ret) { -if( ret == -1 ) { -gemu_log( = -1 errno=%d (%s)\n, errno, target_strerror(errno)); +char *errstr = NULL; + +if (ret == -1) { +errstr = target_strerror(errno); +} +if ((ret == -1) errstr) { +gemu_log( = -1 errno=%d (%s)\n, errno, errstr); } else { gemu_log( = 0x TARGET_ABI_FMT_lx \n, ret); } @@ -1515,14 +1520,19 @@ void print_syscall_ret(int num, abi_long ret) { int i; +char *errstr = NULL; for(i=0;insyscalls;i++) if( scnames[i].nr == num ) { if( scnames[i].result != NULL ) { scnames[i].result(scnames[i],ret); } else { -if( ret 0 ) { -gemu_log( = -1 errno= TARGET_ABI_FMT_ld (%s)\n, -ret, target_strerror(-ret)); +if (ret 0) { +errstr = target_strerror(-ret); +} +if (errstr) { +gemu_log( = -1 errno= TARGET_ABI_FMT_ld (%s)\n, + -ret, errstr); } else { gemu_log( = TARGET_ABI_FMT_ld \n, ret); } diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 15b8b22..29d92c4 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -731,6 +731,9 @@ static inline int is_error(abi_long ret) char *target_strerror(int err) { +if ((err = ERRNO_TABLE_SIZE) || (err 0)) { +return NULL; +} return strerror(target_to_host_errno(err)); } -- 1.7.5.4
Re: [Qemu-devel] Proper way to walk through all vpcus
On Sat, Jan 28, 2012 at 05:06:54PM -0500, Xin Tong wrote: What is the proper way to iterate over all vcpus in qemu ? below is what i use in my code. Not sure whether it is the best way, also is a a macro is qemu to do this ? CPUState *curr_cpu = first_cpu; for(; curr_cpu != NULL; curr_cpu = curr_cpu-next_cpu) { ... } Seems this is a proper way to iterate all virtual cpus. Regards, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] setjmp outside cpu loop
On Sun, Jan 22, 2012 at 11:18:43AM -0500, Xin Tong wrote: There is a setjmp outside the cpu loop in qemu cpu-exec.c. it is used by longjmp later when a cpu exit request is given. I am wondering that can a cpu_loop_exit() be called when the tcg is doing the translation ? Do you mean if there is another thread calls cpu_loop_exit when TCG is doing the translation? Regards, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] Why QEMUClock is defined in qemu-timer.c?
This is used in many places. Maybe to simulate 'private' variables and access methods like in C++. If you define structure in .c file then every function that works with this structure must reside in same file. And any other code to use structure must use access-functions (api) to alter it and not directly (compiler will issue error about incomplete type). This way you could change structure or internal data handling without affecting external code that relies on stable api functions. On Tue, Jan 31, 2012 at 8:35 AM, Richard Yang weiy...@linux.vnet.ibm.com wrote: On Tue, Jan 31, 2012 at 01:12:24PM +0700, Mulyadi Santosa wrote: Hi :) On Tue, Jan 31, 2012 at 12:55, Richard Yang weiy...@linux.vnet.ibm.com wrote: Hi, experts I am just coming to the qemu world. While reading the code, I am wondering why put a struct definition in the c file? Seems there is no error in compile, while I felt it is not a proper way to do it. maybe someone just forgot to refactor it :) I suggest to send a patch to do just that and let's see what everybody thinks about. I, myself thinks that the more readable the code is, the better Ok, let me do it. -- regards, Mulyadi Santosa Freelance Linux trainer and consultant blog: the-hydra.blogspot.com training: mulyaditraining.blogspot.com -- Richard Yang Help you, Help me
[Qemu-devel] [PATCH 18/19] linux-user: Define TARGET_QEMU_ESIGRETURN for mips64
From: Andreas Färber afaer...@suse.de Copied from mips/syscall.h. Signed-off-by: Khansa Butt kha...@kics.edu.pk Signed-off-by: Andreas Färber afaer...@suse.de Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/mips64/syscall.h |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/linux-user/mips64/syscall.h b/linux-user/mips64/syscall.h index 668a2b9..e436ea5 100644 --- a/linux-user/mips64/syscall.h +++ b/linux-user/mips64/syscall.h @@ -218,4 +218,7 @@ struct target_pt_regs { +/* Nasty hack: define a fake errno value for use by sigreturn. */ +#define TARGET_QEMU_ESIGRETURN 255 + #define UNAME_MACHINE mips64 -- 1.7.5.4
[Qemu-devel] [PATCH 04/19] linux-user: fake /proc/self/maps
From: Alexander Graf ag...@suse.de glibc's pthread_attr_getstack tries to find the stack range from /proc/self/maps. Unfortunately, /proc is usually the host's /proc which means linux-user guests see qemu's stack there. Fake the file with a constructed maps entry that exposes the guest's stack range. Signed-off-by: Alexander Graf ag...@suse.de Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/syscall.c | 15 +++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index e100025..1864d7f 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -4600,6 +4600,20 @@ int get_osversion(void) return osversion; } + +static int open_self_maps(void *cpu_env, int fd) +{ +TaskState *ts = ((CPUState *)cpu_env)-opaque; + +dprintf(fd, %08llx-%08llx rw-p %08llx 00:00 0 [stack]\n, +(unsigned long long)ts-info-stack_limit, +(unsigned long long)(ts-stack_base + (TARGET_PAGE_SIZE - 1)) + TARGET_PAGE_MASK, +(unsigned long long)ts-stack_base); + +return 0; +} + static int do_open(void *cpu_env, const char *pathname, int flags, mode_t mode) { struct fake_open { @@ -4608,6 +4622,7 @@ static int do_open(void *cpu_env, const char *pathname, int flags, mode_t mode) }; const struct fake_open *fake_open; static const struct fake_open fakes[] = { +{ /proc/self/maps, open_self_maps }, { NULL, NULL } }; -- 1.7.5.4
[Qemu-devel] [PATCH 19/19] linux-user: Fix sa_flags byte swaps for mips
From: Andreas Färber afaer...@suse.de sa_flags is uint32_t for mips{,n32,64}, so don't use tswapal(). edited by Riku Voipio: likewise on alpha Reported-by: Khansa Butt kha...@kics.edu.pk Suggested-by: Richard Henderson r...@twiddle.net Signed-off-by: Andreas Färber afaer...@suse.de Cc: Ehsan Ul Haq ehsan.ul...@kics.edu.pk Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/signal.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index ded12ca..79a39dc 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -587,7 +587,11 @@ int do_sigaction(int sig, const struct target_sigaction *act, #endif if (oact) { oact-_sa_handler = tswapal(k-_sa_handler); +#if defined(TARGET_MIPS) || defined (TARGET_ALPHA) +oact-sa_flags = bswap32(k-sa_flags); +#else oact-sa_flags = tswapal(k-sa_flags); +#endif #if !defined(TARGET_MIPS) oact-sa_restorer = tswapal(k-sa_restorer); #endif @@ -596,7 +600,11 @@ int do_sigaction(int sig, const struct target_sigaction *act, if (act) { /* FIXME: This is not threadsafe. */ k-_sa_handler = tswapal(act-_sa_handler); +#if defined(TARGET_MIPS) || defined (TARGET_ALPHA) +k-sa_flags = bswap32(act-sa_flags); +#else k-sa_flags = tswapal(act-sa_flags); +#endif #if !defined(TARGET_MIPS) k-sa_restorer = tswapal(act-sa_restorer); #endif -- 1.7.5.4
[Qemu-devel] [PATCH 16/19] linux-user: Add default configs for mips64[el]
From: Andreas Färber afaer...@suse.de Prepares for mips64[el]-linux-user targets. Signed-off-by: Khansa Butt kha...@kics.edu.pk Signed-off-by: Andreas Färber afaer...@suse.de Signed-off-by: Riku Voipio riku.voi...@linaro.org --- default-configs/mips64-linux-user.mak |1 + default-configs/mips64el-linux-user.mak |1 + 2 files changed, 2 insertions(+), 0 deletions(-) create mode 100644 default-configs/mips64-linux-user.mak create mode 100644 default-configs/mips64el-linux-user.mak diff --git a/default-configs/mips64-linux-user.mak b/default-configs/mips64-linux-user.mak new file mode 100644 index 000..1598bfc --- /dev/null +++ b/default-configs/mips64-linux-user.mak @@ -0,0 +1 @@ +# Default configuration for mips64-linux-user diff --git a/default-configs/mips64el-linux-user.mak b/default-configs/mips64el-linux-user.mak new file mode 100644 index 000..629f084 --- /dev/null +++ b/default-configs/mips64el-linux-user.mak @@ -0,0 +1 @@ +# Default configuration for mips64el-linux-user -- 1.7.5.4
[Qemu-devel] [PATCH 03/19] linux-user: add open() hijack infrastructure
From: Alexander Graf ag...@suse.de There are a number of files in /proc that expose host information to the guest program. This patch adds infrastructure to override the open() syscall for guest programs to enable us to on the fly generate guest sensible files. Signed-off-by: Alexander Graf ag...@suse.de Signed-off-by: Riku Voipio riku.voi...@linaro.org --- linux-user/syscall.c | 52 +++-- 1 files changed, 49 insertions(+), 3 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 2bf9e7e..e100025 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -4600,6 +4600,52 @@ int get_osversion(void) return osversion; } +static int do_open(void *cpu_env, const char *pathname, int flags, mode_t mode) +{ +struct fake_open { +const char *filename; +int (*fill)(void *cpu_env, int fd); +}; +const struct fake_open *fake_open; +static const struct fake_open fakes[] = { +{ NULL, NULL } +}; + +for (fake_open = fakes; fake_open-filename; fake_open++) { +if (!strncmp(pathname, fake_open-filename, + strlen(fake_open-filename))) { +break; +} +} + +if (fake_open-filename) { +const char *tmpdir; +char filename[PATH_MAX]; +int fd, r; + +/* create temporary file to map stat to */ +tmpdir = getenv(TMPDIR); +if (!tmpdir) +tmpdir = /tmp; +snprintf(filename, sizeof(filename), %s/qemu-open.XX, tmpdir); +fd = mkstemp(filename); +if (fd 0) { +return fd; +} +unlink(filename); + +if ((r = fake_open-fill(cpu_env, fd))) { +close(fd); +return r; +} +lseek(fd, 0, SEEK_SET); + +return fd; +} + +return get_errno(open(path(pathname), flags, mode)); +} + /* do_syscall() should always have a single exit point at the end so that actions, such as logging of syscall results, can be performed. All errnos that do_syscall() returns must be -TARGET_errcode. */ @@ -4685,9 +4731,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, case TARGET_NR_open: if (!(p = lock_user_string(arg1))) goto efault; -ret = get_errno(open(path(p), - target_to_host_bitmask(arg2, fcntl_flags_tbl), - arg3)); +ret = get_errno(do_open(cpu_env, p, +target_to_host_bitmask(arg2, fcntl_flags_tbl), +arg3)); unlock_user(p, arg1, 0); break; #if defined(TARGET_NR_openat) defined(__NR_openat) -- 1.7.5.4
[Qemu-devel] [PATCH 02/19] linux-user: save auxv length
From: Alexander Graf ag...@suse.de We create our own AUXV segment on stack and save a pointer to it. However we don't save the length of it, so any code that wants to do anything useful with it later on has to walk it again. Instead, let's remember the length of our AUXV segment. This simplifies later uses by a lot. (edited by Riku to apply to qemu HEAD) Signed-off-by: Alexander Graf ag...@suse.de Signed-off-by: Riku Voipio riku.voi...@iki.fi --- linux-user/elfload.c | 15 --- linux-user/qemu.h|1 + 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 845be8b..2fd4a93 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -1245,6 +1245,7 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, struct image_info *interp_info) { abi_ulong sp; +abi_ulong sp_auxv; int size; int i; abi_ulong u_rand_bytes; @@ -1316,6 +1317,7 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, sp -= n; put_user_ual(id, sp); \ } while(0) +sp_auxv = sp; NEW_AUX_ENT (AT_NULL, 0); /* There must be exactly DLINFO_ITEMS entries here. */ @@ -1346,6 +1348,7 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, #undef NEW_AUX_ENT info-saved_auxv = sp; +info-auxv_len = sp_auxv - sp; sp = loader_build_argptr(envc, argc, sp, p, 0); return sp; @@ -2326,9 +2329,8 @@ static void fill_auxv_note(struct memelfnote *note, const TaskState *ts) { elf_addr_t auxv = (elf_addr_t)ts-info-saved_auxv; elf_addr_t orig_auxv = auxv; -abi_ulong val; void *ptr; -int i, len; +int len = ts-info-auxv_len; /* * Auxiliary vector is stored in target process stack. It contains @@ -2336,15 +2338,6 @@ static void fill_auxv_note(struct memelfnote *note, const TaskState *ts) * strictly necessary but we do it here for sake of completeness. */ -/* find out length of the vector, AT_NULL is terminator */ -i = len = 0; -do { -get_user_ual(val, auxv); -i += 2; -auxv += 2 * sizeof (elf_addr_t); -} while (val != AT_NULL); -len = i * sizeof (elf_addr_t); - /* read in whole auxv vector and copy it to memelfnote */ ptr = lock_user(VERIFY_READ, orig_auxv, len, 0); if (ptr != NULL) { diff --git a/linux-user/qemu.h b/linux-user/qemu.h index 30e2abd..308dbc0 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -48,6 +48,7 @@ struct image_info { abi_ulong code_offset; abi_ulong data_offset; abi_ulong saved_auxv; +abi_ulong auxv_len; abi_ulong arg_start; abi_ulong arg_end; int personality; -- 1.7.5.4
Re: [Qemu-devel] [Xen-devel] qemu(-dm): aborting on wrong mmio size?
On 31.01.12 at 10:34, Laszlo Ersek ler...@redhat.com wrote: in the qemu-xen-unstable tree (git://xenbits.xen.org/qemu-xen-unstable.git), the do_inp() function [i386-dm/helper2.c] makes the process exit if the operand size is wrong. Blame: 6040eea5 (More files imported from xen-unstable 17192:59b8768d0d0d). In the qemu tree (git://git.qemu.org/qemu.git), the do_inp() function [xen-all.c] does the same (via hw_error() / abort()). Blame: 9ce94e7c (xen: Initialize event channels and io rings). Is it justified to kill the emulator when this happens (eg. memory mapped IO with 64-bit operand)? Afaict, this is not about MMIO, but PIO. What would happen on real hardware? If it's undefined, wouldn't it be better (for some definition of better) to return a constant? The AMD manual specifies that REX.W is ignored; the Intel manual doesn't mention REX at all here. However, if a decoder incorrectly decodes the guest instruction, that's a bug there. So imo qemu validly treats this condition as fatal. Jan
Re: [Qemu-devel] [PATCH RFC 4/7] qom: Introduce CPU class
Am 30.01.2012 03:14, schrieb Anthony Liguori: On 01/29/2012 07:25 AM, Andreas Färber wrote: +static TypeInfo cpu_type_info = { +.name = TYPE_CPU, +.parent = TYPE_OBJECT, +.instance_size = sizeof(CPU), Probably want to do CPUState or something of that nature so that you can use CPU() as a dynamic_cast macro. My testing seems to indicate CPU and CPU(obj) can co-exist (GCC 4.6.2). Considered name alternatives: CPUState - already taken as #define for legacy CPU${arch}State CPUCore - fits ARM but there's socket vs. core vs. thread on x86 CPUCommon - puts the emphasis on common, which is not a noun CommonCPU - no other base class uses such a naming scheme CPUModel - it's not just a model but the state of one instance CPUFamily - same as for CPUModel CPUState would be nicest IMO, but refactoring CPUState into something else just to refactor it back to QOM CPUState feels like churn. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] KVM call agenda for tuesday 31
Anthony Liguori writes: On 01/30/2012 05:41 PM, Andreas Färber wrote: Mascot contest: Any update? Were there too few entries? Has it been forgotten? If the contest is still open then that should be stated on the list. Yes, I'm falling victim to perfectionism here attempting to find a clever online voting mechanism. I'm just going to hold a vote over the mailing list. I'll post an email tomorrow with details. http://www.cs.cornell.edu/andru/civs.html 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] [RFC/PATCH] Fix guest OS panic when 64bit BAR is present
On 01/27/2012 06:42 AM, Alexey Korolev wrote: On 27/01/12 04:12, Avi Kivity wrote: On 01/26/2012 04:36 PM, Michael S. Tsirkin wrote: On Thu, Jan 26, 2012 at 03:52:27PM +0200, Avi Kivity wrote: On 01/26/2012 11:14 AM, Michael S. Tsirkin wrote: On Wed, Jan 25, 2012 at 06:46:03PM +1300, Alexey Korolev wrote: Hi, In this post http://lists.gnu.org/archive/html/qemu-devel/2011-12/msg03171.html I've mentioned about the issues when 64Bit PCI BAR is present and 32bit address range is selected for it. The issue affects all recent qemu releases and all old and recent guest Linux kernel versions. We've done some investigations. Let me explain what happens. Assume we have 64bit BAR with size 32MB mapped at [0xF000 - 0xF200] When Linux guest starts it does PCI bus enumeration. The OS enumerates 64BIT bars using the following procedure. 1. Write all FF's to lower half of 64bit BAR 2. Write address back to lower half of 64bit BAR 3. Write all FF's to higher half of 64bit BAR 4. Write address back to higher half of 64bit BAR Linux code is here: http://lxr.linux.no/#linux+v3.2.1/drivers/pci/probe.c#L149 What does it mean for qemu? At step 1. qemu pci_default_write_config() recevies all FFs for lower part of the 64bit BAR. Then it applies the mask and converts the value to All FF's - size + 1 (FE00 if size is 32MB). Then pci_bar_address() checks if BAR address is valid. Since it is a 64bit bar it reads 0xFE00 - this address is valid. So qemu updates topology and sends request to update mappings in KVM with new range for the 64bit BAR FE00 - 0x. This usually means kernel panic on boot, if there is another mapping in the FE00 - 0x range, which is quite common. Do you know why does it panic? As far as I can see from code at http://lxr.linux.no/#linux+v2.6.35.9/drivers/pci/probe.c#L162 171pci_read_config_dword(dev, pos, l); 172pci_write_config_dword(dev, pos, l | mask); 173pci_read_config_dword(dev, pos, sz); 174pci_write_config_dword(dev, pos, l); BAR is restored: what triggers an access between lines 172 and 174? Random interrupt reading the time, likely. Weird, what the backtrace shows is init, unrelated to interrupts. It's a bug then. qemu doesn't undo the mapping correctly. If you have clear instructions, I'll try to reproduce it. Well the easiest way to reproduce this is: 1. Get kernel bzImage (version 2.6.36) 2. Apply patch to ivshmem.c I have some patches that fix this, but they're very hacky since they're dealing with the old and rotten core. I much prefer to let this resolve itself in my continuing rewrite. Is this an urgent problem for you or can you live with this for a while? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [Xen-devel] qemu(-dm): aborting on wrong mmio size?
On 01/31/12 11:36, Jan Beulich wrote: On 31.01.12 at 10:34, Laszlo Ersekler...@redhat.com wrote: in the qemu-xen-unstable tree (git://xenbits.xen.org/qemu-xen-unstable.git), the do_inp() function [i386-dm/helper2.c] makes the process exit if the operand size is wrong. Blame: 6040eea5 (More files imported from xen-unstable 17192:59b8768d0d0d). In the qemu tree (git://git.qemu.org/qemu.git), the do_inp() function [xen-all.c] does the same (via hw_error() / abort()). Blame: 9ce94e7c (xen: Initialize event channels and io rings). Is it justified to kill the emulator when this happens (eg. memory mapped IO with 64-bit operand)? Afaict, this is not about MMIO, but PIO. One possible way seems to be (see http://xenbits.xensource.com/hg/linux-2.6.18-xen.hg/rev/1141): vmx_hpw_miss() [xen/arch/ia64/vmx/vmx_fault.c] - emulate_io_inst() [xen/arch/ia64/vmx/mmio.c] - mmio_access() - legacy_io_access() - vmx_send_assist_req() [xen/arch/ia64/vmx/vmx_support.c] - notify_via_xen_event_channel() [xen/common/event_channel.c] and in qemu-xen-unstable, cpu_handle_ioreq() [i386-dm/helper2.c], set up in main_loop() - __handle_ioreq() - cpu_ioreq_pio() - do_inp() Thanks, Laszlo
Re: [Qemu-devel] [Xen-devel] qemu(-dm): aborting on wrong mmio size?
On 01/31/12 11:36, Jan Beulich wrote: On 31.01.12 at 10:34, Laszlo Ersekler...@redhat.com wrote: Is it justified to kill the emulator when this happens (eg. memory mapped IO with 64-bit operand)? The AMD manual specifies that REX.W is ignored; the Intel manual doesn't mention REX at all here. However, if a decoder incorrectly decodes the guest instruction, that's a bug there. So imo qemu validly treats this condition as fatal. From the Itanium(R) SDM rev 2.3, 10.7.2.1 I/O Port Addressing Restrictions For the 64MB physical I/O port block the following operations are undefined and may result in unpredictable processor operation; references larger than 4-bytes, [...] It seems that not only a decoding failure can trigger this. Laszlo
Re: [Qemu-devel] [help]how to make qemu-img utility to support writting file or directory to the image file
Am 29.01.2012 10:02, schrieb 马磊: Hi, qemu-img is only to support create/info and so on whithout writting operation to the image file. I have ported the reading operation for a image file form grub2 to qemu-img. But NTFS document online is not detailed enough, how to do inorder to achieve the goal of writting to a image file? If anyone encountered the same problem, please reply to me. Thanks ahead! Embedding file system drivers in qemu-img is not the right approach. You need to get the image mapped to a block device on your host (e.g. using qemu-nbd), so you can use the host's file system drivers. Kevin
Re: [Qemu-devel] [PATCH v2 00/15] SCSI s/g + SCSI migration + virtio-scsi
On 01/30/2012 10:33 AM, Hu Tao wrote: I cannot reproduce this with a 100G qcow2 image (created with qemu-img create scsi.qcow2 100G just before launching the host), with a partition starting at sector 2048 and extending to the end of the disk. mkfs (ext4) takes less than 1 minute and extends the qcow2 file to a little less than 2 gigs. I've re-tested today, with host kernel 2.6.35.6-45.fc14.x86_64, 2.6.32-71.el6.x86_64, 3.1.0 and 3.3.0-rc1+, qemu version and guest configuration remain the same, including guest kernel. It apears that the problem appears only when the host kernel version is 3.1.0. There are two parts in the bug: 1) I/O cancellation doesn't actually cancel anything on qcow2 (and maybe others) since the coroutine transition. This makes exception handling basically unusable in the virtio-scsi guest driver. I have been working on this but I don't have patches ready for testing yet. 2) Something unknown in the kernel is causing writes to take a longer time than usual. This appears to be a problem only with 3.1.0 guest kernels. I think we can wait until (1) causes problems elsewhere. Thanks for testing! Paolo
[Qemu-devel] [PATCH 0/5] sdl: various fixes
See individual patches for details. CC: Erik Rull erik.r...@rdsoftware.de Jan Kiszka (5): sdl: Do not grab mouse on mode switch while in background sdl: Fix block prevention of SDL_WM_GrabInput Revert Handle SDL grabs failing (Mark McLoughlin) sdl: Grab input on end of non-absolute mouse click sdl: Limit sdl_grab_end in handle_activation to Windows hosts ui/sdl.c | 60 1 files changed, 32 insertions(+), 28 deletions(-) -- 1.7.3.4
[Qemu-devel] [PATCH 4/5] sdl: Grab input on end of non-absolute mouse click
By grabbing the input already on button down, we leave the button in that state for the host GUI. Thus it takes another click after releasing the input again to synchronize the mouse button state. Avoid this by grabbing on button up. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- ui/sdl.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/ui/sdl.c b/ui/sdl.c index 0d3a889..73e5839 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -802,8 +802,7 @@ static void handle_mousebutton(DisplayState *ds, SDL_Event *ev) bev = ev-button; if (!gui_grab !kbd_mouse_is_absolute()) { -if (ev-type == SDL_MOUSEBUTTONDOWN -(bev-button == SDL_BUTTON_LEFT)) { +if (ev-type == SDL_MOUSEBUTTONUP bev-button == SDL_BUTTON_LEFT) { /* start grabbing all events */ sdl_grab_start(); } -- 1.7.3.4
[Qemu-devel] [PATCH 1/5] sdl: Do not grab mouse on mode switch while in background
When the mouse mode changes to absolute while the SDL windows is not in focus, refrain from grabbing the input. It would steal from some other window. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- ui/sdl.c | 30 -- 1 files changed, 16 insertions(+), 14 deletions(-) diff --git a/ui/sdl.c b/ui/sdl.c index 8cafc44..384276d 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -483,12 +483,27 @@ static void sdl_grab_end(void) sdl_update_caption(); } +static void absolute_mouse_grab(void) +{ +int mouse_x, mouse_y; + +if (SDL_GetAppState() SDL_APPINPUTFOCUS) { +SDL_GetMouseState(mouse_x, mouse_y); +if (mouse_x 0 mouse_x real_screen-w - 1 +mouse_y 0 mouse_y real_screen-h - 1) { +sdl_grab_start(); +} +} +} + static void sdl_mouse_mode_change(Notifier *notify, void *data) { if (kbd_mouse_is_absolute()) { if (!absolute_enabled) { -sdl_grab_start(); absolute_enabled = 1; +if (is_graphic_console()) { +absolute_mouse_grab(); +} } } else if (absolute_enabled) { if (!gui_fullscreen) { @@ -571,19 +586,6 @@ static void toggle_full_screen(DisplayState *ds) vga_hw_update(); } -static void absolute_mouse_grab(void) -{ -int mouse_x, mouse_y; - -if (SDL_GetAppState() SDL_APPINPUTFOCUS) { -SDL_GetMouseState(mouse_x, mouse_y); -if (mouse_x 0 mouse_x real_screen-w - 1 -mouse_y 0 mouse_y real_screen-h - 1) { -sdl_grab_start(); -} -} -} - static void handle_keydown(DisplayState *ds, SDL_Event *ev) { int mod_state; -- 1.7.3.4
[Qemu-devel] [PATCH 5/5] sdl: Limit sdl_grab_end in handle_activation to Windows hosts
There are scenarios on Linux with some SDL versions where handle_activation is continuous invoked with state = SDL_APPINPUTFOCUS and gain = 0 while we grabbed the input. This causes a ping-pong when we grab the input after an absolute mouse entered the window. As this sdl_grab_end was once introduced to work around a Windows-only issue (0294ffb9c8), limit it to that platform. CC: Erik Rull erik.r...@rdsoftware.de Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- ui/sdl.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/ui/sdl.c b/ui/sdl.c index 73e5839..6f8091c 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -828,10 +828,14 @@ static void handle_mousebutton(DisplayState *ds, SDL_Event *ev) static void handle_activation(DisplayState *ds, SDL_Event *ev) { +#ifdef _WIN32 +/* Disable grab if the window no longer has the focus + * (Windows-only workaround) */ if (gui_grab ev-active.state == SDL_APPINPUTFOCUS !ev-active.gain !gui_fullscreen) { sdl_grab_end(); } +#endif if (!gui_grab ev-active.gain is_graphic_console() (kbd_mouse_is_absolute() || absolute_enabled)) { absolute_mouse_grab(); -- 1.7.3.4
[Qemu-devel] [PATCH 3/5] Revert Handle SDL grabs failing (Mark McLoughlin)
This reverts commit 6bb816031f8bc0aafc3476e6dfa4293ee3a5f106. SDL_WM_GrabInput does not reliably bail out if grabbing is impossible. So if we get here, we already lost and will block. But this can no longer happen due to the check in sdl_grab_start. So this patch became obsolete. Conflicts: sdl.c Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- ui/sdl.c |9 +++-- 1 files changed, 3 insertions(+), 6 deletions(-) diff --git a/ui/sdl.c b/ui/sdl.c index d845efb..0d3a889 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -475,12 +475,9 @@ static void sdl_grab_start(void) SDL_WarpMouse(guest_x, guest_y); } else sdl_hide_cursor(); - -if (SDL_WM_GrabInput(SDL_GRAB_ON) == SDL_GRAB_ON) { -gui_grab = 1; -sdl_update_caption(); -} else -sdl_show_cursor(); +SDL_WM_GrabInput(SDL_GRAB_ON); +gui_grab = 1; +sdl_update_caption(); } static void sdl_grab_end(void) -- 1.7.3.4
[Qemu-devel] [PATCH 2/5] sdl: Fix block prevention of SDL_WM_GrabInput
Consistently check for SDL_APPINPUTFOCUS before trying to grab the input focus. Just checking for SDL_APPACTIVE doesn't work. Moving the check to sdl_grab_start allows for some consolidation. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- ui/sdl.c | 26 ++ 1 files changed, 14 insertions(+), 12 deletions(-) diff --git a/ui/sdl.c b/ui/sdl.c index 384276d..d845efb 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -461,6 +461,14 @@ static void sdl_show_cursor(void) static void sdl_grab_start(void) { +/* + * If the application is not active, do not try to enter grab state. This + * prevents 'SDL_WM_GrabInput(SDL_GRAB_ON)' from blocking all the + * application (SDL bug). + */ +if (!(SDL_GetAppState() SDL_APPINPUTFOCUS)) { +return; +} if (guest_cursor) { SDL_SetCursor(guest_sprite); if (!kbd_mouse_is_absolute() !absolute_enabled) @@ -487,12 +495,10 @@ static void absolute_mouse_grab(void) { int mouse_x, mouse_y; -if (SDL_GetAppState() SDL_APPINPUTFOCUS) { -SDL_GetMouseState(mouse_x, mouse_y); -if (mouse_x 0 mouse_x real_screen-w - 1 -mouse_y 0 mouse_y real_screen-h - 1) { -sdl_grab_start(); -} +SDL_GetMouseState(mouse_x, mouse_y); +if (mouse_x 0 mouse_x real_screen-w - 1 +mouse_y 0 mouse_y real_screen-h - 1) { +sdl_grab_start(); } } @@ -745,11 +751,7 @@ static void handle_keyup(DisplayState *ds, SDL_Event *ev) if (gui_keysym == 0) { /* exit/enter grab if pressing Ctrl-Alt */ if (!gui_grab) { -/* If the application is not active, do not try to enter grab - * state. It prevents 'SDL_WM_GrabInput(SDL_GRAB_ON)' from - * blocking all the application (SDL bug). */ -if (is_graphic_console() -SDL_GetAppState() SDL_APPACTIVE) { +if (is_graphic_console()) { sdl_grab_start(); } } else if (!gui_fullscreen) { @@ -779,7 +781,7 @@ static void handle_mousemotion(DisplayState *ds, SDL_Event *ev) ev-motion.x == max_x || ev-motion.y == max_y)) { sdl_grab_end(); } -if (!gui_grab SDL_GetAppState() SDL_APPINPUTFOCUS +if (!gui_grab (ev-motion.x 0 ev-motion.x max_x ev-motion.y 0 ev-motion.y max_y)) { sdl_grab_start(); -- 1.7.3.4
Re: [Qemu-devel] [Xen-devel] qemu(-dm): aborting on wrong mmio size?
On 31.01.12 at 12:04, Laszlo Ersek ler...@redhat.com wrote: On 01/31/12 11:36, Jan Beulich wrote: On 31.01.12 at 10:34, Laszlo Ersekler...@redhat.com wrote: Is it justified to kill the emulator when this happens (eg. memory mapped IO with 64-bit operand)? The AMD manual specifies that REX.W is ignored; the Intel manual doesn't mention REX at all here. However, if a decoder incorrectly decodes the guest instruction, that's a bug there. So imo qemu validly treats this condition as fatal. From the Itanium(R) SDM rev 2.3, 10.7.2.1 I/O Port Addressing Restrictions For the 64MB physical I/O port block the following operations are undefined and may result in unpredictable processor operation; references larger than 4-bytes, [...] It seems that not only a decoding failure can trigger this. Oh, yes, I forgot that port I/O goes via MMIO on ia64. So yes, for that case your observation is correct, though killing qemu-dm in that case still seems to fall well under undefined behavior. But improving the situation wouldn't be bad then. Jan
Re: [Qemu-devel] KVM call agenda for tuesday 31
Am 31.01.2012 00:53, schrieb Anthony Liguori: On 01/30/2012 05:41 PM, Andreas Färber wrote: Am 30.01.2012 19:55, schrieb Juan Quintela: Please send in any agenda items you are interested in covering. VMState: Anthony specifically said that VMState were not affected by QOM and that patches should not be deferred until the merge. Yet there's no review and/or decision-making for a month now. Ping^2 for AHCI+SDHC. Do you have pointers (to pending VMState patches)? http://patchwork.ozlabs.org/patch/137732/ (PATCH v4) It's basically about how to deal with variable-sized arrays. (Alex mentioned it on one call around November.) I found ways to deal with subsets of arrays embedded within the struct and variable-sized list of pointers to structs but no solution for a malloc()'ed array of structs. Maybe I'm just too stupid to see. Anyway, no one commented since Xmas. Igor posted (and refined for v2) a patch with a callback-based approach that I find promising. From my view, unofficially Juan is the VMState guy, he's been cc'ed. Are we lacking an official maintainer that cares? Or is Juan the official, undocumented maintainer but simply busy? SUSE's interest is making AHCI migratable, and my VMState workaround for that is simply ugly: http://patchwork.ozlabs.org/patch/133066/ (RFC) Therefore I'm waiting for some resolution. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] MAINTAINERS: Add a section for the host OS and a W32 maintainer
Am 27.01.2012 18:53, schrieb Stefan Weil: Up to now, there was no special section for the different host operating systems used with QEMU. scripts/get_maintainer.pl did not show a maintainer for OS specific files and patches. Therefore I added three hosts systems: * POSIX for the majority of host systems which are supported. This includes BSD and Linux host systems. * LINUX is a special case of POSIX needed for some Linux specific files and directories. * W32, W64 for a well known family of closed source operating systems. I also added myself as a maintainer for W32, W64. Signed-off-by: Stefan Weil s...@weilnetz.de --- MAINTAINERS | 20 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 73c0a10..882958e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -161,6 +161,26 @@ S: Supported F: xen-* F: */xen* +Hosts: +-- + +LINUX +L: qemu-devel@nongnu.org +S: Maintained +F: linux-* +F: linux-headers/ + +POSIX +L: qemu-devel@nongnu.org +S: Maintained +F: *posix* How can these be Maintained without a maintainer? Kevin
Re: [Qemu-devel] [PATCH] qmp: add BLOCK_MEDIUM_EJECT event
On Tue, 31 Jan 2012 10:23:59 +0100 Markus Armbruster arm...@redhat.com wrote: Kevin Wolf kw...@redhat.com writes: Am 30.01.2012 16:18, schrieb Luiz Capitulino: On Fri, 27 Jan 2012 10:52:15 +0100 Kevin Wolf kw...@redhat.com wrote: Am 26.01.2012 18:57, schrieb Luiz Capitulino: On Wed, 25 Jan 2012 10:42:04 -0200 Luiz Capitulino lcapitul...@redhat.com wrote: On Wed, 25 Jan 2012 09:41:20 +0100 Kevin Wolf kw...@redhat.com wrote: Am 24.01.2012 21:03, schrieb Eric Blake: On 01/24/2012 11:16 AM, Luiz Capitulino wrote: Libvirt wants to be notified when the guest ejects a medium, so that it can update its view of the guest. This code has been originally written by Daniel Berrange. It adds the event to IDE and SCSI emulation. Please, note that this only covers guest initiated ejects, that's, the QMP/HMP commands 'eject' and 'change' are not covered. What's the reason for this behaviour? It feels inconsistent. I don't think it's inconsistent because we're limiting it to guest initiated actions. Also, the mngt app knows when it sends a 'eject' or 'change' command. The exception is if it allows HMP to run in parallel with QMP, but I don't think this is recommended (at least not for commands that change any VM state). Let me elaborate more. We have two options: 1. Emit the event for guest initiated ejects (this patch, although I think the event should be renamed to GUEST_MEDIUM_EJECT) 2. Emit the event for guest QMP initiated ejects, that's, also emit the event for the eject and change commands The first thing to note is that, they're not mutually exclusive. If we do item 1 now, we can add 2 later (as a different event). But my point is that doing 2 is problematic and we should avoid it. The problem is that the semantics of eject and change are complex and/or buggy. And something I've learned about confusing semantics in QMP is that, they will give you headaches soon or later. But I'm not really interested in the semantics of QMP commands, because they are irrelevant for the events. I do think they are relevant, because the event will have to match what the eject/change commands do with the tray. If what they do is messy, the event will turn out to be messy too. Now, I don't doubt this can be all fixed and made clean. I'm just not sure if it's worth it. If a mess best describes to the outside what we're doing to the device, then having a messy event is the best you can expect. Or in other words, if you're doing messy things with the device and you straighten things out in the event generation, then your events are lying to the management tools. Yup. The event is merely a passive sensor. If reality is messy, sensor data better reflect that. Maybe it's easier to understand from a distance, so let's examine a more distant example: filesystem event monitoring with inotify(), specifically file permissions change. The event is simple enough: you get it when file permissions change. Now imagine chmod(2) was improved to refuse to set write bits unless read bits are also set, but only on Tuedays. That's a messy chmod() indeed. But the event remains as simple and clean as ever: you still get it when file permissions change. Back to QMP. In my opinion, the simple and clean event is tray moved. Emit it when the tray moves, regardless of what made it move. I'll give it a tray. Oh, I meant a try :) Yes, the management app gets the event even when it itself directly triggered the move by commanding a media eject. That's a feature. It also gets the event when its media eject command actually becomes a polite request to the guest OS, because the tray happens to be locked, and the guest OS complies. That's a feature, too. [...]
Re: [Qemu-devel] [PATCH 11/23] qom: allow object_class_foreach to take additional parameters to refine search
$subject is a bit long. Am 30.01.2012 22:08, schrieb Anthony Liguori: Signed-off-by: Anthony Liguori aligu...@us.ibm.com Reviewed-by: Andreas Färber afaer...@suse.de However... diff --git a/hw/qdev.c b/hw/qdev.c index 636b6b4..a7980c5 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -255,7 +255,7 @@ int qdev_device_help(QemuOpts *opts) driver = qemu_opt_get(opts, driver); if (driver !strcmp(driver, ?)) { bool show_no_user = false; -object_class_foreach(qdev_print_devinfo, show_no_user); +object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, show_no_user); return 1; } @@ -1077,7 +1077,7 @@ void do_info_qtree(Monitor *mon) void do_info_qdm(Monitor *mon) { -object_class_foreach(qdev_print_devinfo, NULL); +object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, NULL); } int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data) If you reordered this patch before 07/23 then you could apply it right away, unbreaking object_class_foreach(). Andreas diff --git a/include/qemu/object.h b/include/qemu/object.h index ba37850..adbcfb1 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -431,6 +431,7 @@ const char *object_class_get_name(ObjectClass *klass); ObjectClass *object_class_by_name(const char *typename); void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque), + const char *implements_type, bool include_abstract, void *opaque); #endif diff --git a/qom/object.c b/qom/object.c index a12895f..3dabb1a 100644 --- a/qom/object.c +++ b/qom/object.c @@ -467,6 +467,8 @@ ObjectClass *object_class_by_name(const char *typename) typedef struct OCFData { void (*fn)(ObjectClass *klass, void *opaque); +const char *implements_type; +bool include_abstract; void *opaque; } OCFData; @@ -475,16 +477,28 @@ static void object_class_foreach_tramp(gpointer key, gpointer value, { OCFData *data = opaque; TypeImpl *type = value; +ObjectClass *k; type_class_init(type); +k = type-class; -data-fn(value, type-class); +if (!data-include_abstract type-abstract) { +return; +} + +if (data-implements_type +!object_class_dynamic_cast(k, data-implements_type)) { +return; +} + +data-fn(k, data-opaque); } void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque), + const char *implements_type, bool include_abstract, void *opaque) { -OCFData data = { fn, opaque }; +OCFData data = { fn, implements_type, include_abstract, opaque }; g_hash_table_foreach(type_table_get(), object_class_foreach_tramp, data); } -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 07/23] qdev: kill off DeviceInfo list
Am 30.01.2012 22:08, schrieb Anthony Liguori: Teach the various bits of code that need to walk through available devices to do so via QOM. Signed-off-by: Anthony Liguori (Email missing.) NACK. This introduces broken code, see below. bool qdev_exists(const char *name) { -return !!qdev_find_info(NULL, name); +return !!object_class_by_name(name); } Minor nit: I'd find object_class_by_name(name) != NULL easier to read than double negation. @@ -245,17 +213,28 @@ DeviceState *qdev_try_create(BusState *bus, const char *name) return qdev_create_from_info(bus, name); } -static void qdev_print_devinfo(DeviceInfo *info) +static void qdev_print_devinfo(ObjectClass *klass, void *opaque) { -error_printf(name \%s\, bus %s, - info-name, info-bus_info-name); -if (info-alias) { -error_printf(, alias \%s\, info-alias); +DeviceClass *dc; +bool *show_no_user = opaque; Due to this... + +dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE); ...and this, ... + +if (!dc || (show_no_user !*show_no_user dc-no_user)) { +return; } -if (info-desc) { -error_printf(, desc \%s\, info-desc); + +error_printf(name \%s\, object_class_get_name(klass)); +if (dc-bus_info) { +error_printf(, bus %s, dc-bus_info-name); } -if (info-no_user) { +if (dc-alias) { +error_printf(, alias \%s\, dc-alias); +} +if (dc-desc) { +error_printf(, desc \%s\, dc-desc); +} +if (dc-no_user) { error_printf(, no-user); } error_printf(\n); @@ -279,17 +258,14 @@ static int set_property(const char *name, const char *value, void *opaque) int qdev_device_help(QemuOpts *opts) { const char *driver; -DeviceInfo *info; Property *prop; +ObjectClass *klass; +DeviceClass *info; driver = qemu_opt_get(opts, driver); if (driver !strcmp(driver, ?)) { -for (info = device_info_list; info != NULL; info = info-next) { -if (info-no_user) { -continue; /* not available, don't show */ -} -qdev_print_devinfo(info); -} +bool show_no_user = false; +object_class_foreach(qdev_print_devinfo, show_no_user); This... return 1; } @@ -297,10 +273,11 @@ int qdev_device_help(QemuOpts *opts) return 0; } -info = qdev_find_info(NULL, driver); -if (!info) { +klass = object_class_by_name(driver); +if (!klass) { return 0; } +info = DEVICE_CLASS(klass); for (prop = info-props; prop prop-name; prop++) { /* @@ -312,14 +289,14 @@ int qdev_device_help(QemuOpts *opts) if (!prop-info-parse) { continue; /* no way to set it, don't show */ } -error_printf(%s.%s=%s\n, info-name, prop-name, +error_printf(%s.%s=%s\n, driver, prop-name, prop-info-legacy_name ?: prop-info-name); } for (prop = info-bus_info-props; prop prop-name; prop++) { if (!prop-info-parse) { continue; /* no way to set it, don't show */ } -error_printf(%s.%s=%s\n, info-name, prop-name, +error_printf(%s.%s=%s\n, driver, prop-name, prop-info-legacy_name ?: prop-info-name); } return 1; @@ -1085,11 +1062,7 @@ void do_info_qtree(Monitor *mon) void do_info_qdm(Monitor *mon) { -DeviceInfo *info; - -for (info = device_info_list; info != NULL; info = info-next) { -qdev_print_devinfo(info); -} +object_class_foreach(qdev_print_devinfo, NULL); ...and this is broken, as pointed out before: http://patchwork.ozlabs.org/patch/138396/ Easiest would be to reorder 11/23 before this one so that we don't introduce a broken, bloated user of foreach in the first place. Otherwise looking good. Andreas } int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data) -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] KVM call agenda for tuesday 31
On 01/31/2012 05:15 PM, Andreas Färber wrote: Am 31.01.2012 00:53, schrieb Anthony Liguori: On 01/30/2012 05:41 PM, Andreas Färber wrote: Am 30.01.2012 19:55, schrieb Juan Quintela: Please send in any agenda items you are interested in covering. VMState: Anthony specifically said that VMState were not affected by QOM and that patches should not be deferred until the merge. Yet there's no review and/or decision-making for a month now. Ping^2 for AHCI+SDHC. Do you have pointers (to pending VMState patches)? http://patchwork.ozlabs.org/patch/137732/ (PATCH v4) It's basically about how to deal with variable-sized arrays. (Alex mentioned it on one call around November.) I found ways to deal with subsets of arrays embedded within the struct and variable-sized list of pointers to structs but no solution for a malloc()'ed array of structs. Maybe I'm just too stupid to see. Anyway, no one commented since Xmas. Igor posted (and refined for v2) a patch with a callback-based approach that I find promising. From my view, unofficially Juan is the VMState guy, he's been cc'ed. Are we lacking an official maintainer that cares? Or is Juan the official, undocumented maintainer but simply busy? SUSE's interest is making AHCI migratable, and my VMState workaround for that is simply ugly: http://patchwork.ozlabs.org/patch/133066/ (RFC) If I'm not mistaken, if you change AHCIState's .ports type to uint32_t you can use existing VMSTATE_BUFFER_MULTIPLY macro like this: VMSTATE_BUFFER_MULTIPLY(dev, AHCIState, 0, NULL, 0, ports, sizeof(AHCIDevice)) VMSTATE_BUFFER_MULTIPLY currently lacks VMS_POINTER flag and therefore doesn't make any use of _start field (you don't need it anyway) Nevertheless, VMSTATE_BUFFER_MULTIPLY is just a partial solution to a bigger set of possible problems. SD card's vmstate implementation requires shift operation, SDHC gets size from switch {} statement, something else later may require division or addition e.t.c., get_bufsize callback will cover all possible cases. -- Mitsyanko Igor ASWG, Moscow RD center, Samsung Electronics email: i.mitsya...@samsung.com
Re: [Qemu-devel] [PATCH] multiboot: mh_load_end_addr and mh_bss_end_addr may be zero
Am 23.01.2012 13:49, schrieb Göran Weinholt: There are two special cases in the address fields of the multiboot format. If mh_load_end_addr is zero then the whole image file should be loaded and if mh_bss_end_addr is zero then there is no bss segment. With this change it is again possible to boot kernels where these fields are zero. Signed-off-by: Göran Weinholt go...@weinholt.se Tested-by: Alexander Graf ag...@suse.de --- hw/multiboot.c | 15 ++- 1 files changed, 14 insertions(+), 1 deletions(-) diff --git a/hw/multiboot.c b/hw/multiboot.c index b4484a3..db28328 100644 --- a/hw/multiboot.c +++ b/hw/multiboot.c @@ -202,10 +202,23 @@ int load_multiboot(void *fw_cfg, uint32_t mh_bss_end_addr = ldl_p(header+i+24); mh_load_addr = ldl_p(header+i+16); uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr); -uint32_t mb_load_size = mh_load_end_addr - mh_load_addr; +uint32_t mb_load_size; + +/* A load end address of zero indicates that the whole file + * should be loaded. */ +if (!mh_load_end_addr) { +mh_load_end_addr = kernel_file_size + mh_load_addr; This is only right if the OS image starts at offset 0 in the image file. IIUC, in the general case it starts at byte i - (mh_header_addr - mh_load_addr), so you need to subtract this from kernel_file_size. Kevin
Re: [Qemu-devel] KVM call agenda for tuesday 31
On 01/31/2012 03:59 PM, Anthony Liguori wrote: On 01/30/2012 05:41 PM, Andreas Färber wrote: Am 30.01.2012 19:55, schrieb Juan Quintela: Please send in any agenda items you are interested in covering. QOM roadmap update: * Series 3/4 is on the list. - Please officially designate a merge date (Friday?). - To make review sensible, I ask for a hard device freeze until merged. I.e., no new devices and no conflicting changes to DeviceInfo. I put together a few slides to help this discussion: http://www.codemonkey.ws/files/qom-overview.pdf That was helpful, thanks. Can you clarify - Types and their properties will be ABI compatible - Types and properties will not be backwards compatible – We can re-examine this as the device model matures and stabilizes the first two seem very similar, except for the not. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] KVM call agenda for tuesday 31
On 01/31/2012 07:15 AM, Andreas Färber wrote: Am 31.01.2012 00:53, schrieb Anthony Liguori: On 01/30/2012 05:41 PM, Andreas Färber wrote: Am 30.01.2012 19:55, schrieb Juan Quintela: Please send in any agenda items you are interested in covering. VMState: Anthony specifically said that VMState were not affected by QOM and that patches should not be deferred until the merge. Yet there's no review and/or decision-making for a month now. Ping^2 for AHCI+SDHC. Do you have pointers (to pending VMState patches)? http://patchwork.ozlabs.org/patch/137732/ (PATCH v4) It's basically about how to deal with variable-sized arrays. (Alex mentioned it on one call around November.) I found ways to deal with subsets of arrays embedded within the struct and variable-sized list of pointers to structs but no solution for a malloc()'ed array of structs. Maybe I'm just too stupid to see. Anyway, no one commented since Xmas. /me puts on his flame proof suit Don't use VMState. Just open code a save/restore function. VMState is too limited in how it handles complex data structures. I really believe the only long term solution we're going to get to here is something that uses a builder interface (like Visitors). Regards, Anthony Liguori Igor posted (and refined for v2) a patch with a callback-based approach that I find promising. From my view, unofficially Juan is the VMState guy, he's been cc'ed. Are we lacking an official maintainer that cares? Or is Juan the official, undocumented maintainer but simply busy? SUSE's interest is making AHCI migratable, and my VMState workaround for that is simply ugly: http://patchwork.ozlabs.org/patch/133066/ (RFC) Therefore I'm waiting for some resolution. Regards, Andreas
Re: [Qemu-devel] KVM call agenda for tuesday 31
On 01/31/2012 08:09 AM, Avi Kivity wrote: On 01/31/2012 03:59 PM, Anthony Liguori wrote: On 01/30/2012 05:41 PM, Andreas Färber wrote: Am 30.01.2012 19:55, schrieb Juan Quintela: Please send in any agenda items you are interested in covering. QOM roadmap update: * Series 3/4 is on the list. - Please officially designate a merge date (Friday?). - To make review sensible, I ask for a hard device freeze until merged. I.e., no new devices and no conflicting changes to DeviceInfo. I put together a few slides to help this discussion: http://www.codemonkey.ws/files/qom-overview.pdf That was helpful, thanks. Can you clarify - Types and their properties will be ABI compatible - Types and properties will not be backwards compatible – We can re-examine this as the device model matures and stabilizes the first two seem very similar, except for the not. I guess I really mean QMP compatible. The expectation is that well written management code does: if (check_for_command(qom-list)) { run_command(qom-list, /); } ABI compatible means that if qom-list is there, it's semantics never change. Backwards compatibility would mean that once qom-list is introduced, it never goes away. In terms of QOM types, we won't guarantee that a Type will stick around forever, but we will guarantee if it's there, it'll behave in a certain way. When the device model stabilizes over time, we can revisit this, but in the 1.x series, I'm sure we're going to go through a few significant refactorings that change types significantly. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 09/23] qdev: kill of DeviceInfo
Am 30.01.2012 22:08, schrieb Anthony Liguori: It is no longer used in the tree since everything is done natively through QEMU Object Model. Signed-off-by: Anthony Liguori aligu...@us.ibm.com Reviewed-by: Andreas Färber afaer...@suse.de Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 05/15] piix: create the HPET and RTC through composition
On 2012-01-26 20:00, Anthony Liguori wrote: Signed-off-by: Anthony Liguori aligu...@us.ibm.com --- hw/hpet.c| 38 +- hw/hpet_emul.h | 40 hw/mc146818rtc.c | 30 ++--- hw/mc146818rtc.h | 27 +++ hw/pc.c | 38 +-- hw/piix_pci.c| 76 ++--- 6 files changed, 145 insertions(+), 104 deletions(-) diff --git a/hw/hpet.c b/hw/hpet.c index b6ace4e..c5b8b9e 100644 --- a/hw/hpet.c +++ b/hw/hpet.c @@ -41,40 +41,6 @@ #define HPET_MSI_SUPPORT0 -struct HPETState; -typedef struct HPETTimer { /* timers */ -uint8_t tn; /*timer number*/ -QEMUTimer *qemu_timer; -struct HPETState *state; -/* Memory-mapped, software visible timer registers */ -uint64_t config;/* configuration/cap */ -uint64_t cmp; /* comparator */ -uint64_t fsb; /* FSB route */ -/* Hidden register state */ -uint64_t period;/* Last value written to comparator */ -uint8_t wrap_flag; /* timer pop will indicate wrap for one-shot 32-bit - * mode. Next pop will be actual timer expiration. - */ -} HPETTimer; - -typedef struct HPETState { -SysBusDevice busdev; -MemoryRegion iomem; -uint64_t hpet_offset; -qemu_irq irqs[HPET_NUM_IRQ_ROUTES]; -uint32_t flags; -uint8_t rtc_irq_level; -uint8_t num_timers; -HPETTimer timer[HPET_MAX_TIMERS]; - -/* Memory-mapped, software visible registers */ -uint64_t capability;/* capabilities */ -uint64_t config;/* configuration */ -uint64_t isr; /* interrupt status reg */ -uint64_t hpet_counter; /* main counter */ -uint8_t hpet_id; /* instance id */ -} HPETState; - Both structs are private and should remain so, same for similar patches in this series. Does your composition concept requires publicizing them? If yes, can't it be fixed. Would be a step backward if not. Also note that the HPET is not a part of the PIIX, so composition is wrong here. The RTC is again. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 06/15] piix: create i8254 through composition
On 2012-01-26 20:00, Anthony Liguori wrote: @@ -548,6 +550,13 @@ static int piix3_realize(PCIDevice *dev) /* Setup the RTC IRQ */ s-rtc.irq = rtc_irq; +/* Realize the PIT */ +qdev_set_parent_bus(DEVICE(s-pit), BUS(s-bus)); +qdev_init_nofail(DEVICE(s-pit)); + +/* FIXME this should be refactored */ +pcspk_init(ISA_DEVICE(s-pit)); Fixing ATM, ie. converting to qdev/QOM. Q: How do I use qdev_property_add_link Co. to establish the relation from the speaker port device to the PIT? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 14/15] i440fx: move bios loading to i440fx
On 2012-01-26 20:00, Anthony Liguori wrote: --- hw/pc.c | 70 -- hw/pc.h |3 +- hw/piix_pci.c | 74 +++- sysemu.h |2 - 4 files changed, 79 insertions(+), 70 deletions(-) How does the ISA PC get its BIOS after this change? Or did that change in a step I miss right now? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] MAINTAINERS: Add a section for the host OS and a W32 maintainer
On 31 January 2012 13:35, Kevin Wolf kw...@redhat.com wrote: Am 27.01.2012 18:53, schrieb Stefan Weil: +Hosts: +-- + +LINUX +L: qemu-devel@nongnu.org +S: Maintained +F: linux-* +F: linux-headers/ + +POSIX +L: qemu-devel@nongnu.org +S: Maintained +F: *posix* How can these be Maintained without a maintainer? They're maintained in the sense that if you break it somebody *will* fix it, probably with a faster turnaround than any other host system. Certainly 'Odd Fixes' doesn't sound right. -- PMM
Re: [Qemu-devel] [PATCH 05/15] piix: create the HPET and RTC through composition
On 01/31/2012 08:26 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: Signed-off-by: Anthony Liguorialigu...@us.ibm.com --- hw/hpet.c| 38 +- hw/hpet_emul.h | 40 hw/mc146818rtc.c | 30 ++--- hw/mc146818rtc.h | 27 +++ hw/pc.c | 38 +-- hw/piix_pci.c| 76 ++--- 6 files changed, 145 insertions(+), 104 deletions(-) diff --git a/hw/hpet.c b/hw/hpet.c index b6ace4e..c5b8b9e 100644 --- a/hw/hpet.c +++ b/hw/hpet.c @@ -41,40 +41,6 @@ #define HPET_MSI_SUPPORT0 -struct HPETState; -typedef struct HPETTimer { /* timers */ -uint8_t tn; /*timer number*/ -QEMUTimer *qemu_timer; -struct HPETState *state; -/* Memory-mapped, software visible timer registers */ -uint64_t config;/* configuration/cap */ -uint64_t cmp; /* comparator */ -uint64_t fsb; /* FSB route */ -/* Hidden register state */ -uint64_t period;/* Last value written to comparator */ -uint8_t wrap_flag; /* timer pop will indicate wrap for one-shot 32-bit - * mode. Next pop will be actual timer expiration. - */ -} HPETTimer; - -typedef struct HPETState { -SysBusDevice busdev; -MemoryRegion iomem; -uint64_t hpet_offset; -qemu_irq irqs[HPET_NUM_IRQ_ROUTES]; -uint32_t flags; -uint8_t rtc_irq_level; -uint8_t num_timers; -HPETTimer timer[HPET_MAX_TIMERS]; - -/* Memory-mapped, software visible registers */ -uint64_t capability;/* capabilities */ -uint64_t config;/* configuration */ -uint64_t isr; /* interrupt status reg */ -uint64_t hpet_counter; /* main counter */ -uint8_t hpet_id; /* instance id */ -} HPETState; - Both structs are private and should remain so, same for similar patches in this series. Does your composition concept requires publicizing them? If yes, can't it be fixed. Would be a step backward if not. It doesn't strictly require it, no, but I like it. It encourages using proper interfaces like: void rtc_set_memory(RTCState *rtc, int addr, int val); Instead of: void rtc_set_memory(ISADevice *dev, int addr, int val); Yes, we can achieve the same thing with forward declarations. The second thing I like about this style is that it makes it easier to use a code generator to generate serialization functions. Finally, I think embedded a devices memory within its parent device provides a certain level of elegance. Also note that the HPET is not a part of the PIIX, so composition is wrong here. There is no HPET in an i440fx system. The HPET usually sits on the LPC bus (which replaces ISA in modern systems). It's sometimes a dedicated chip but can certain co-exist in a Super IO chip. I think in terms of where it would live in this hypothetical device model, putting it in the PIIX is rational. The RTC is again. -ENOPARSE Regards, Anthony Liguori Jan
Re: [Qemu-devel] [PATCH] MAINTAINERS: Add a section for the host OS and a W32 maintainer
Am 31.01.2012 15:40, schrieb Peter Maydell: On 31 January 2012 13:35, Kevin Wolf kw...@redhat.com wrote: Am 27.01.2012 18:53, schrieb Stefan Weil: +Hosts: +-- + +LINUX +L: qemu-devel@nongnu.org +S: Maintained +F: linux-* +F: linux-headers/ + +POSIX +L: qemu-devel@nongnu.org +S: Maintained +F: *posix* How can these be Maintained without a maintainer? They're maintained in the sense that if you break it somebody *will* fix it, probably with a faster turnaround than any other host system. Certainly 'Odd Fixes' doesn't sound right. I agree, adding a maintainer name (Anthony?) would be better. Kevin
Re: [Qemu-devel] KVM call agenda for tuesday 31
On 01/31/2012 04:17 PM, Anthony Liguori wrote: On 01/31/2012 08:09 AM, Avi Kivity wrote: On 01/31/2012 03:59 PM, Anthony Liguori wrote: On 01/30/2012 05:41 PM, Andreas Färber wrote: Am 30.01.2012 19:55, schrieb Juan Quintela: Please send in any agenda items you are interested in covering. QOM roadmap update: * Series 3/4 is on the list. - Please officially designate a merge date (Friday?). - To make review sensible, I ask for a hard device freeze until merged. I.e., no new devices and no conflicting changes to DeviceInfo. I put together a few slides to help this discussion: http://www.codemonkey.ws/files/qom-overview.pdf That was helpful, thanks. Can you clarify - Types and their properties will be ABI compatible - Types and properties will not be backwards compatible – We can re-examine this as the device model matures and stabilizes the first two seem very similar, except for the not. I guess I really mean QMP compatible. The expectation is that well written management code does: if (check_for_command(qom-list)) { run_command(qom-list, /); } ABI compatible means that if qom-list is there, it's semantics never change. Backwards compatibility would mean that once qom-list is introduced, it never goes away. In terms of QOM types, we won't guarantee that a Type will stick around forever, but we will guarantee if it's there, it'll behave in a certain way. When the device model stabilizes over time, we can revisit this, but in the 1.x series, I'm sure we're going to go through a few significant refactorings that change types significantly. Ok. We have to communicate this very carefully. If a management tool uses something that is ABI compatible, but not backwards compatible, and lacks a known alternative at the time of writing the management tool, then the ABI-compatible-but-not-backwards-compatible property becomes not backwards compatible to the management tool's users. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 06/15] piix: create i8254 through composition
On 01/31/2012 08:34 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: @@ -548,6 +550,13 @@ static int piix3_realize(PCIDevice *dev) /* Setup the RTC IRQ */ s-rtc.irq = rtc_irq; +/* Realize the PIT */ +qdev_set_parent_bus(DEVICE(s-pit), BUS(s-bus)); +qdev_init_nofail(DEVICE(s-pit)); + +/* FIXME this should be refactored */ +pcspk_init(ISA_DEVICE(s-pit)); Fixing ATM, ie. converting to qdev/QOM. Q: How do I use qdev_property_add_link Co. to establish the relation from the speaker port device to the PIT? In the state structure, have: struct PCSpkState { ... PITState *pit; }; In the pcspk instance_init, do: object_property_add_link(obj, pit, TYPE_PIT, (Object **)s-pit, NULL); In the pcspk realize function (DeviceClass::init), do: assert(s-pit != NULL); // make sure the pit link is set And that's it. You can set the s-pit field directly. You are not required to use any special QOM function to interact with link properties. BTW, this is yet another benefit of making structures public. You can take the address of a child and set link fields directly without accessors. Regards, Anthony LIguori Jan
Re: [Qemu-devel] [PATCH 05/15] piix: create the HPET and RTC through composition
On 2012-01-31 15:43, Anthony Liguori wrote: On 01/31/2012 08:26 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: Signed-off-by: Anthony Liguorialigu...@us.ibm.com --- hw/hpet.c| 38 +- hw/hpet_emul.h | 40 hw/mc146818rtc.c | 30 ++--- hw/mc146818rtc.h | 27 +++ hw/pc.c | 38 +-- hw/piix_pci.c| 76 ++--- 6 files changed, 145 insertions(+), 104 deletions(-) diff --git a/hw/hpet.c b/hw/hpet.c index b6ace4e..c5b8b9e 100644 --- a/hw/hpet.c +++ b/hw/hpet.c @@ -41,40 +41,6 @@ #define HPET_MSI_SUPPORT0 -struct HPETState; -typedef struct HPETTimer { /* timers */ -uint8_t tn; /*timer number*/ -QEMUTimer *qemu_timer; -struct HPETState *state; -/* Memory-mapped, software visible timer registers */ -uint64_t config;/* configuration/cap */ -uint64_t cmp; /* comparator */ -uint64_t fsb; /* FSB route */ -/* Hidden register state */ -uint64_t period;/* Last value written to comparator */ -uint8_t wrap_flag; /* timer pop will indicate wrap for one-shot 32-bit - * mode. Next pop will be actual timer expiration. - */ -} HPETTimer; - -typedef struct HPETState { -SysBusDevice busdev; -MemoryRegion iomem; -uint64_t hpet_offset; -qemu_irq irqs[HPET_NUM_IRQ_ROUTES]; -uint32_t flags; -uint8_t rtc_irq_level; -uint8_t num_timers; -HPETTimer timer[HPET_MAX_TIMERS]; - -/* Memory-mapped, software visible registers */ -uint64_t capability;/* capabilities */ -uint64_t config;/* configuration */ -uint64_t isr; /* interrupt status reg */ -uint64_t hpet_counter; /* main counter */ -uint8_t hpet_id; /* instance id */ -} HPETState; - Both structs are private and should remain so, same for similar patches in this series. Does your composition concept requires publicizing them? If yes, can't it be fixed. Would be a step backward if not. It doesn't strictly require it, no, but I like it. It encourages using proper interfaces like: void rtc_set_memory(RTCState *rtc, int addr, int val); Instead of: void rtc_set_memory(ISADevice *dev, int addr, int val); Yes, we can achieve the same thing with forward declarations. The second thing I like about this style is that it makes it easier to use a code generator to generate serialization functions. Finally, I think embedded a devices memory within its parent device provides a certain level of elegance. It reopens the door for poking inside the device states. That was closed (widely) by privatizing the states (I think mostly driven by Blue). I'm not convinced yet that being able to embed the struct into a containing device is worth giving up on this. Also note that the HPET is not a part of the PIIX, so composition is wrong here. There is no HPET in an i440fx system. The HPET usually sits on the LPC bus (which replaces ISA in modern systems). It's sometimes a dedicated chip but can certain co-exist in a Super IO chip. I think in terms of where it would live in this hypothetical device model, putting it in the PIIX is rational. Does it buy us anything? I don't see the advantage of this imprecision. If the model works well, it should be able to cover the real architecture elegantly, too. The RTC is again. -ENOPARSE I meant that the RTC was correctly moved into the PIIX. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 14/15] i440fx: move bios loading to i440fx
On 01/31/2012 08:38 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: --- hw/pc.c | 70 -- hw/pc.h |3 +- hw/piix_pci.c | 74 +++- sysemu.h |2 - 4 files changed, 79 insertions(+), 70 deletions(-) How does the ISA PC get its BIOS after this change? Or did that change in a step I miss right now? Oh, I broke it. I made no attempt to keep ISA PC working. The way I'd like to handle this is to introduce a ROM device so that this code would be trivialized. Regards, Anthony Liguori Jan
Re: [Qemu-devel] [PATCH 06/15] piix: create i8254 through composition
On 2012-01-31 15:47, Anthony Liguori wrote: On 01/31/2012 08:34 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: @@ -548,6 +550,13 @@ static int piix3_realize(PCIDevice *dev) /* Setup the RTC IRQ */ s-rtc.irq = rtc_irq; +/* Realize the PIT */ +qdev_set_parent_bus(DEVICE(s-pit), BUS(s-bus)); +qdev_init_nofail(DEVICE(s-pit)); + +/* FIXME this should be refactored */ +pcspk_init(ISA_DEVICE(s-pit)); Fixing ATM, ie. converting to qdev/QOM. Q: How do I use qdev_property_add_link Co. to establish the relation from the speaker port device to the PIT? In the state structure, have: struct PCSpkState { ... PITState *pit; }; In the pcspk instance_init, do: object_property_add_link(obj, pit, TYPE_PIT, (Object **)s-pit, NULL); In the pcspk realize function (DeviceClass::init), do: assert(s-pit != NULL); // make sure the pit link is set And that's it. You can set the s-pit field directly. You are not required to use any special QOM function to interact with link properties. BTW, this is yet another benefit of making structures public. You can take the address of a child and set link fields directly without accessors. Well, that has two sides. We introduced properties to avoid this direct messing. Does linking also work without exposing internals? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 14/15] i440fx: move bios loading to i440fx
On 2012-01-31 15:50, Anthony Liguori wrote: On 01/31/2012 08:38 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: --- hw/pc.c | 70 -- hw/pc.h |3 +- hw/piix_pci.c | 74 +++- sysemu.h |2 - 4 files changed, 79 insertions(+), 70 deletions(-) How does the ISA PC get its BIOS after this change? Or did that change in a step I miss right now? Oh, I broke it. I made no attempt to keep ISA PC working. The way I'd like to handle this is to introduce a ROM device so that this code would be trivialized. Or just keep the common pc.c library as I voted for. It has its purpose, obviously. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 05/15] piix: create the HPET and RTC through composition
On 01/31/2012 08:49 AM, Jan Kiszka wrote: On 2012-01-31 15:43, Anthony Liguori wrote: On 01/31/2012 08:26 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: Signed-off-by: Anthony Liguorialigu...@us.ibm.com --- hw/hpet.c| 38 +- hw/hpet_emul.h | 40 hw/mc146818rtc.c | 30 ++--- hw/mc146818rtc.h | 27 +++ hw/pc.c | 38 +-- hw/piix_pci.c| 76 ++--- 6 files changed, 145 insertions(+), 104 deletions(-) diff --git a/hw/hpet.c b/hw/hpet.c index b6ace4e..c5b8b9e 100644 --- a/hw/hpet.c +++ b/hw/hpet.c @@ -41,40 +41,6 @@ #define HPET_MSI_SUPPORT0 -struct HPETState; -typedef struct HPETTimer { /* timers */ -uint8_t tn; /*timer number*/ -QEMUTimer *qemu_timer; -struct HPETState *state; -/* Memory-mapped, software visible timer registers */ -uint64_t config;/* configuration/cap */ -uint64_t cmp; /* comparator */ -uint64_t fsb; /* FSB route */ -/* Hidden register state */ -uint64_t period;/* Last value written to comparator */ -uint8_t wrap_flag; /* timer pop will indicate wrap for one-shot 32-bit - * mode. Next pop will be actual timer expiration. - */ -} HPETTimer; - -typedef struct HPETState { -SysBusDevice busdev; -MemoryRegion iomem; -uint64_t hpet_offset; -qemu_irq irqs[HPET_NUM_IRQ_ROUTES]; -uint32_t flags; -uint8_t rtc_irq_level; -uint8_t num_timers; -HPETTimer timer[HPET_MAX_TIMERS]; - -/* Memory-mapped, software visible registers */ -uint64_t capability;/* capabilities */ -uint64_t config;/* configuration */ -uint64_t isr; /* interrupt status reg */ -uint64_t hpet_counter; /* main counter */ -uint8_t hpet_id; /* instance id */ -} HPETState; - Both structs are private and should remain so, same for similar patches in this series. Does your composition concept requires publicizing them? If yes, can't it be fixed. Would be a step backward if not. It doesn't strictly require it, no, but I like it. It encourages using proper interfaces like: void rtc_set_memory(RTCState *rtc, int addr, int val); Instead of: void rtc_set_memory(ISADevice *dev, int addr, int val); Yes, we can achieve the same thing with forward declarations. The second thing I like about this style is that it makes it easier to use a code generator to generate serialization functions. Finally, I think embedded a devices memory within its parent device provides a certain level of elegance. It reopens the door for poking inside the device states. That was closed (widely) by privatizing the states (I think mostly driven by Blue). I'm not convinced yet that being able to embed the struct into a containing device is worth giving up on this. Also note that the HPET is not a part of the PIIX, so composition is wrong here. There is no HPET in an i440fx system. The HPET usually sits on the LPC bus (which replaces ISA in modern systems). It's sometimes a dedicated chip but can certain co-exist in a Super IO chip. I think in terms of where it would live in this hypothetical device model, putting it in the PIIX is rational. Does it buy us anything? I don't see the advantage of this imprecision. If the model works well, it should be able to cover the real architecture elegantly, too. We could move the HPET to a child of the 440fx-pmc. That's probably more correct. I don't think it's worth modeling an LPC bus. LPC is just a spec for allowing third party chips, it's not mandated that all HPET chips have a pin-out that's LPC compatible. I don't think there's any guest-visible state that comes from a device being on the LPC verses being hard wired in the north bridge. Regards, Anthony Liguori The RTC is again. -ENOPARSE I meant that the RTC was correctly moved into the PIIX. Jan
Re: [Qemu-devel] [PATCH 06/15] piix: create i8254 through composition
On 01/31/2012 08:51 AM, Jan Kiszka wrote: On 2012-01-31 15:47, Anthony Liguori wrote: On 01/31/2012 08:34 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: @@ -548,6 +550,13 @@ static int piix3_realize(PCIDevice *dev) /* Setup the RTC IRQ */ s-rtc.irq = rtc_irq; +/* Realize the PIT */ +qdev_set_parent_bus(DEVICE(s-pit), BUS(s-bus)); +qdev_init_nofail(DEVICE(s-pit)); + +/* FIXME this should be refactored */ +pcspk_init(ISA_DEVICE(s-pit)); Fixing ATM, ie. converting to qdev/QOM. Q: How do I use qdev_property_add_link Co. to establish the relation from the speaker port device to the PIT? In the state structure, have: struct PCSpkState { ... PITState *pit; }; In the pcspk instance_init, do: object_property_add_link(obj, pit, TYPE_PIT, (Object **)s-pit, NULL); In the pcspk realize function (DeviceClass::init), do: assert(s-pit != NULL); // make sure the pit link is set And that's it. You can set the s-pit field directly. You are not required to use any special QOM function to interact with link properties. BTW, this is yet another benefit of making structures public. You can take the address of a child and set link fields directly without accessors. Well, that has two sides. We introduced properties to avoid this direct messing. Does linking also work without exposing internals? Yes, you can set links through properties (although I haven't added those accessors yet). But... you lose type safety because now you're dealing with strings. If we want to avoid exposing internals, we should provide accessors to get/set links. So: typedef struct PCSpkState PCSpkState; void pcspk_set_pit(PCSpkState *s, PITState *pit); Regards, Anthony Liguori Jan
Re: [Qemu-devel] [PATCH 05/15] piix: create the HPET and RTC through composition
On 2012-01-31 15:54, Anthony Liguori wrote: On 01/31/2012 08:49 AM, Jan Kiszka wrote: On 2012-01-31 15:43, Anthony Liguori wrote: On 01/31/2012 08:26 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: Signed-off-by: Anthony Liguorialigu...@us.ibm.com --- hw/hpet.c| 38 +- hw/hpet_emul.h | 40 hw/mc146818rtc.c | 30 ++--- hw/mc146818rtc.h | 27 +++ hw/pc.c | 38 +-- hw/piix_pci.c| 76 ++--- 6 files changed, 145 insertions(+), 104 deletions(-) diff --git a/hw/hpet.c b/hw/hpet.c index b6ace4e..c5b8b9e 100644 --- a/hw/hpet.c +++ b/hw/hpet.c @@ -41,40 +41,6 @@ #define HPET_MSI_SUPPORT0 -struct HPETState; -typedef struct HPETTimer { /* timers */ -uint8_t tn; /*timer number*/ -QEMUTimer *qemu_timer; -struct HPETState *state; -/* Memory-mapped, software visible timer registers */ -uint64_t config;/* configuration/cap */ -uint64_t cmp; /* comparator */ -uint64_t fsb; /* FSB route */ -/* Hidden register state */ -uint64_t period;/* Last value written to comparator */ -uint8_t wrap_flag; /* timer pop will indicate wrap for one-shot 32-bit - * mode. Next pop will be actual timer expiration. - */ -} HPETTimer; - -typedef struct HPETState { -SysBusDevice busdev; -MemoryRegion iomem; -uint64_t hpet_offset; -qemu_irq irqs[HPET_NUM_IRQ_ROUTES]; -uint32_t flags; -uint8_t rtc_irq_level; -uint8_t num_timers; -HPETTimer timer[HPET_MAX_TIMERS]; - -/* Memory-mapped, software visible registers */ -uint64_t capability;/* capabilities */ -uint64_t config;/* configuration */ -uint64_t isr; /* interrupt status reg */ -uint64_t hpet_counter; /* main counter */ -uint8_t hpet_id; /* instance id */ -} HPETState; - Both structs are private and should remain so, same for similar patches in this series. Does your composition concept requires publicizing them? If yes, can't it be fixed. Would be a step backward if not. It doesn't strictly require it, no, but I like it. It encourages using proper interfaces like: void rtc_set_memory(RTCState *rtc, int addr, int val); Instead of: void rtc_set_memory(ISADevice *dev, int addr, int val); Yes, we can achieve the same thing with forward declarations. The second thing I like about this style is that it makes it easier to use a code generator to generate serialization functions. Finally, I think embedded a devices memory within its parent device provides a certain level of elegance. It reopens the door for poking inside the device states. That was closed (widely) by privatizing the states (I think mostly driven by Blue). I'm not convinced yet that being able to embed the struct into a containing device is worth giving up on this. Also note that the HPET is not a part of the PIIX, so composition is wrong here. There is no HPET in an i440fx system. The HPET usually sits on the LPC bus (which replaces ISA in modern systems). It's sometimes a dedicated chip but can certain co-exist in a Super IO chip. I think in terms of where it would live in this hypothetical device model, putting it in the PIIX is rational. Does it buy us anything? I don't see the advantage of this imprecision. If the model works well, it should be able to cover the real architecture elegantly, too. We could move the HPET to a child of the 440fx-pmc. That's probably more correct. Nope, it was a separate chip in such systems. It sits on the board (today our sysbus), nakedly and alone. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 14/15] i440fx: move bios loading to i440fx
On 01/31/2012 08:53 AM, Jan Kiszka wrote: On 2012-01-31 15:50, Anthony Liguori wrote: On 01/31/2012 08:38 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: --- hw/pc.c | 70 -- hw/pc.h |3 +- hw/piix_pci.c | 74 +++- sysemu.h |2 - 4 files changed, 79 insertions(+), 70 deletions(-) How does the ISA PC get its BIOS after this change? Or did that change in a step I miss right now? Oh, I broke it. I made no attempt to keep ISA PC working. The way I'd like to handle this is to introduce a ROM device so that this code would be trivialized. Or just keep the common pc.c library as I voted for. It has its purpose, obviously. Coding sharing needs to happen through device sharing. Otherwise we'll be stuck in the magic device creation through arbitrary functions rut that we currently find ourselves in. Regards, Anthony Liguori Jan
Re: [Qemu-devel] [PATCH 06/15] piix: create i8254 through composition
On 01/31/2012 03:51 PM, Jan Kiszka wrote: BTW, this is yet another benefit of making structures public. You can take the address of a child and set link fields directly without accessors. Well, that has two sides. We introduced properties to avoid this direct messing. Does linking also work without exposing internals? Perhaps it doesn't need to expose internals. Just like we create interfaces automatically when creating a parent object, perhaps we can create children as well, like TypeInfo type_piix3 = { ... .children = { { pic[0], TYPE_I8259, offsetof(PIIX3, pic[0]) }, { pic[1], TYPE_I8259, offsetof(PIIX3, pic[1]) }, { pit, TYPE_I8254, offsetof(PIIX3, pit) }, { rtc, TYPE_RTC, offsetof(PIIX3, rtc) }, { } } } QOM's object_init would allocate a single malloced block for all of them, carve out space for the parent and all children, and add the properties. Paolo
Re: [Qemu-devel] [PATCH 14/15] i440fx: move bios loading to i440fx
On 2012-01-31 15:57, Anthony Liguori wrote: On 01/31/2012 08:53 AM, Jan Kiszka wrote: On 2012-01-31 15:50, Anthony Liguori wrote: On 01/31/2012 08:38 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: --- hw/pc.c | 70 -- hw/pc.h |3 +- hw/piix_pci.c | 74 +++- sysemu.h |2 - 4 files changed, 79 insertions(+), 70 deletions(-) How does the ISA PC get its BIOS after this change? Or did that change in a step I miss right now? Oh, I broke it. I made no attempt to keep ISA PC working. The way I'd like to handle this is to introduce a ROM device so that this code would be trivialized. Or just keep the common pc.c library as I voted for. It has its purpose, obviously. Coding sharing needs to happen through device sharing. Otherwise we'll be stuck in the magic device creation through arbitrary functions rut that we currently find ourselves in. Well, let's see what this will mean in practice. I'm sure that that there are steps in a PC construction that cannot be modeled reasonable even as pseudo devices but at still shared among boards. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 05/15] piix: create the HPET and RTC through composition
On 01/31/2012 08:56 AM, Jan Kiszka wrote: On 2012-01-31 15:54, Anthony Liguori wrote: On 01/31/2012 08:49 AM, Jan Kiszka wrote: On 2012-01-31 15:43, Anthony Liguori wrote: On 01/31/2012 08:26 AM, Jan Kiszka wrote: On 2012-01-26 20:00, Anthony Liguori wrote: Signed-off-by: Anthony Liguorialigu...@us.ibm.com --- hw/hpet.c| 38 +- hw/hpet_emul.h | 40 hw/mc146818rtc.c | 30 ++--- hw/mc146818rtc.h | 27 +++ hw/pc.c | 38 +-- hw/piix_pci.c| 76 ++--- 6 files changed, 145 insertions(+), 104 deletions(-) diff --git a/hw/hpet.c b/hw/hpet.c index b6ace4e..c5b8b9e 100644 --- a/hw/hpet.c +++ b/hw/hpet.c @@ -41,40 +41,6 @@ #define HPET_MSI_SUPPORT0 -struct HPETState; -typedef struct HPETTimer { /* timers */ -uint8_t tn; /*timer number*/ -QEMUTimer *qemu_timer; -struct HPETState *state; -/* Memory-mapped, software visible timer registers */ -uint64_t config;/* configuration/cap */ -uint64_t cmp; /* comparator */ -uint64_t fsb; /* FSB route */ -/* Hidden register state */ -uint64_t period;/* Last value written to comparator */ -uint8_t wrap_flag; /* timer pop will indicate wrap for one-shot 32-bit - * mode. Next pop will be actual timer expiration. - */ -} HPETTimer; - -typedef struct HPETState { -SysBusDevice busdev; -MemoryRegion iomem; -uint64_t hpet_offset; -qemu_irq irqs[HPET_NUM_IRQ_ROUTES]; -uint32_t flags; -uint8_t rtc_irq_level; -uint8_t num_timers; -HPETTimer timer[HPET_MAX_TIMERS]; - -/* Memory-mapped, software visible registers */ -uint64_t capability;/* capabilities */ -uint64_t config;/* configuration */ -uint64_t isr; /* interrupt status reg */ -uint64_t hpet_counter; /* main counter */ -uint8_t hpet_id; /* instance id */ -} HPETState; - Both structs are private and should remain so, same for similar patches in this series. Does your composition concept requires publicizing them? If yes, can't it be fixed. Would be a step backward if not. It doesn't strictly require it, no, but I like it. It encourages using proper interfaces like: void rtc_set_memory(RTCState *rtc, int addr, int val); Instead of: void rtc_set_memory(ISADevice *dev, int addr, int val); Yes, we can achieve the same thing with forward declarations. The second thing I like about this style is that it makes it easier to use a code generator to generate serialization functions. Finally, I think embedded a devices memory within its parent device provides a certain level of elegance. It reopens the door for poking inside the device states. That was closed (widely) by privatizing the states (I think mostly driven by Blue). I'm not convinced yet that being able to embed the struct into a containing device is worth giving up on this. Also note that the HPET is not a part of the PIIX, so composition is wrong here. There is no HPET in an i440fx system. The HPET usually sits on the LPC bus (which replaces ISA in modern systems). It's sometimes a dedicated chip but can certain co-exist in a Super IO chip. I think in terms of where it would live in this hypothetical device model, putting it in the PIIX is rational. Does it buy us anything? I don't see the advantage of this imprecision. If the model works well, it should be able to cover the real architecture elegantly, too. We could move the HPET to a child of the 440fx-pmc. That's probably more correct. Nope, it was a separate chip in such systems. It sits on the board (today our sysbus), nakedly and alone. So the northbridge would need to implement an LPC bus. This can be as simple as having an LPC interface (which just consists of a few MemoryRegions and Pins) and then a few linkLPCDevice in the i440fx-pmc for expansion. Regards, Anthony Liguori Jan
Re: [Qemu-devel] KVM call agenda for tuesday 31
On Tue, Jan 31, 2012 at 08:12:29AM -0600, Anthony Liguori wrote: On 01/31/2012 07:15 AM, Andreas Färber wrote: Am 31.01.2012 00:53, schrieb Anthony Liguori: On 01/30/2012 05:41 PM, Andreas Färber wrote: Am 30.01.2012 19:55, schrieb Juan Quintela: Please send in any agenda items you are interested in covering. VMState: Anthony specifically said that VMState were not affected by QOM and that patches should not be deferred until the merge. Yet there's no review and/or decision-making for a month now. Ping^2 for AHCI+SDHC. Do you have pointers (to pending VMState patches)? http://patchwork.ozlabs.org/patch/137732/ (PATCH v4) It's basically about how to deal with variable-sized arrays. (Alex mentioned it on one call around November.) I found ways to deal with subsets of arrays embedded within the struct and variable-sized list of pointers to structs but no solution for a malloc()'ed array of structs. Maybe I'm just too stupid to see. Anyway, no one commented since Xmas. /me puts on his flame proof suit Don't use VMState. Just open code a save/restore function. VMState is too limited in how it handles complex data structures. I really believe the only long term solution we're going to get to here is something that uses a builder interface (like Visitors). Regards, Anthony Liguori So the TPM patches started implementing a visitor interface for BER, they are still blocked, right? Igor posted (and refined for v2) a patch with a callback-based approach that I find promising. From my view, unofficially Juan is the VMState guy, he's been cc'ed. Are we lacking an official maintainer that cares? Or is Juan the official, undocumented maintainer but simply busy? SUSE's interest is making AHCI migratable, and my VMState workaround for that is simply ugly: http://patchwork.ozlabs.org/patch/133066/ (RFC) Therefore I'm waiting for some resolution. Regards, Andreas -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for tuesday 31
On Tue, Jan 31, 2012 at 05:04:48PM +0200, Michael S. Tsirkin wrote: On Tue, Jan 31, 2012 at 08:12:29AM -0600, Anthony Liguori wrote: On 01/31/2012 07:15 AM, Andreas Färber wrote: Am 31.01.2012 00:53, schrieb Anthony Liguori: On 01/30/2012 05:41 PM, Andreas Färber wrote: Am 30.01.2012 19:55, schrieb Juan Quintela: Please send in any agenda items you are interested in covering. VMState: Anthony specifically said that VMState were not affected by QOM and that patches should not be deferred until the merge. Yet there's no review and/or decision-making for a month now. Ping^2 for AHCI+SDHC. Do you have pointers (to pending VMState patches)? http://patchwork.ozlabs.org/patch/137732/ (PATCH v4) It's basically about how to deal with variable-sized arrays. (Alex mentioned it on one call around November.) I found ways to deal with subsets of arrays embedded within the struct and variable-sized list of pointers to structs but no solution for a malloc()'ed array of structs. Maybe I'm just too stupid to see. Anyway, no one commented since Xmas. /me puts on his flame proof suit Don't use VMState. Just open code a save/restore function. VMState is too limited in how it handles complex data structures. I really believe the only long term solution we're going to get to here is something that uses a builder interface (like Visitors). Regards, Anthony Liguori So the TPM patches started implementing a visitor interface for BER, they are still blocked, right? And by the way, I believe we really should switch to something like BER and just drop the legacy migration format - being non self delimiting makes it just too painful for words to abstract, and attempts to hide that mess behind a visitor interface will just cause more cruft to accumulate, and cause inefficiencies such as extra data copies. Igor posted (and refined for v2) a patch with a callback-based approach that I find promising. From my view, unofficially Juan is the VMState guy, he's been cc'ed. Are we lacking an official maintainer that cares? Or is Juan the official, undocumented maintainer but simply busy? SUSE's interest is making AHCI migratable, and my VMState workaround for that is simply ugly: http://patchwork.ozlabs.org/patch/133066/ (RFC) Therefore I'm waiting for some resolution. Regards, Andreas -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for tuesday 31
On 01/31/2012 03:12 PM, Anthony Liguori wrote: Don't use VMState. Just open code a save/restore function. VMState is too limited in how it handles complex data structures. I really believe the only long term solution we're going to get to here is something that uses a builder interface (like Visitors). Visitors for complex data structures still require you separate get/set functions. VMState was created to avoid having to write things twice and avoid getting the two out of sync. Paolo
Re: [Qemu-devel] [PATCH v4 0/6] save/restore on Xen
On Wed, 25 Jan 2012, Stefano Stabellini wrote: Hi all, this is the fourth version of the Xen save/restore patch series. We have been discussing this issue for quite a while on #qemu and qemu-devel: http://marc.info/?l=qemu-develm=132346828427314w=2 http://marc.info/?l=qemu-develm=132377734605464w=2 The principal changes in the this version are: - Following Anthony's suggestion I have introduced a new monitor command to save the non-ram device state to file. - I have also removed the hack not to reset the cirrus videoram on restore, because it turns out that the videoram doesn't need to be reset in the reset handler at all (tested on Win2K, where the problem was found in the first place). Is everybody happy enough with this series? Do you have any additional comments?
Re: [Qemu-devel] [PATCH 05/15] piix: create the HPET and RTC through composition
On 2012-01-31 15:26, Jan Kiszka wrote: Also note that the HPET is not a part of the PIIX, so composition is wrong here. The RTC is again. Err, forgot my nonsense. The HPET is part of the PIIX. Dunno, I was somehow thinking of the IOAPIC while reading HPET. Too few sleep, I guess... Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 06/15] piix: create i8254 through composition
On 2012-01-31 15:58, Paolo Bonzini wrote: On 01/31/2012 03:51 PM, Jan Kiszka wrote: BTW, this is yet another benefit of making structures public. You can take the address of a child and set link fields directly without accessors. Well, that has two sides. We introduced properties to avoid this direct messing. Does linking also work without exposing internals? Perhaps it doesn't need to expose internals. Just like we create interfaces automatically when creating a parent object, perhaps we can create children as well, like TypeInfo type_piix3 = { ... .children = { { pic[0], TYPE_I8259, offsetof(PIIX3, pic[0]) }, { pic[1], TYPE_I8259, offsetof(PIIX3, pic[1]) }, { pit, TYPE_I8254, offsetof(PIIX3, pit) }, { rtc, TYPE_RTC, offsetof(PIIX3, rtc) }, { } } and .links = ... Yep, such a thing looks better. I suppose that also opens the door for (runtime) type checking, right? } QOM's object_init would allocate a single malloced block for all of them, carve out space for the parent and all children, and add the properties. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH v4 0/6] prevent QEMU from waking up needlessly
Hi all, this small patch series prevents QEMU from waking up needlessly on Xen several times a second in order to check some timers. The first patch stops QEMU from emulating the PIT on Xen, the second patch disables the rtc_clock entirely. The third patch makes use of a new mechanism to receive buffered io event notifications from Xen, so that QEMU doesn't need to check the buffered io page for data 10 times a sec for the entire life of the VM. The fourth patch fixes win32_rearm_timer and mm_rearm_timer that risk an overflow if INT64_MAX is passed as delta. The fifth patch changes qemu_next_alarm_deadline only to check the expire time of a clock if it is enabled. Finally the last patch increases the default select timeout to 1h: nothing should rely on the select timeout to be 1sec, so we might as well increase it to 1h. Changes in v4: - do not initialize pcspk on xen; - disable rtc_clock only when it points to the host_clock (the default); - make sure it compiles on older xen versions. Changes in v3: - added a new patch to fix win32_rearm_timer and mm_rearm_timer, that risk an overflow if INT64_MAX is passed as delta. Shortlog and diffstat follow: Stefano Stabellini (6): xen: do not initialize the interval timer and PCSPK emulator xen: disable rtc_clock xen: introduce an event channel for buffered io event notifications timers: the rearm function should be able to handle delta = INT64_MAX qemu_next_alarm_deadline: check the expire time of a clock only if it is enabled qemu_calculate_timeout: increase minimum timeout to 1h hw/pc.c |9 ++--- qemu-timer.c | 30 ++ xen-all.c| 49 +++-- 3 files changed, 67 insertions(+), 21 deletions(-) A git tree, based on fd39941ac78fbe969e292eeb91415ec548bd97a6, is available here: git://xenbits.xen.org/people/sstabellini/qemu-dm.git timers-4 Cheers, Stefano
[Qemu-devel] [PATCH v4 4/6] timers: the rearm function should be able to handle delta = INT64_MAX
Fix win32_rearm_timer and mm_rearm_timer: they should be able to handle INT64_MAX as a delta parameter without overflowing. Also, the next deadline in ms should be calculated rounding down rather than up (see unix_rearm_timer and dynticks_rearm_timer). Finally ChangeTimerQueueTimer takes an unsigned long and timeSetEvent takes an unsigned int as delta, so cast the ms delta to the appropriate unsigned integer. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com --- qemu-timer.c | 18 +- 1 files changed, 13 insertions(+), 5 deletions(-) diff --git a/qemu-timer.c b/qemu-timer.c index a22f27e..29410f1 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -696,13 +696,17 @@ static void mm_stop_timer(struct qemu_alarm_timer *t) static void mm_rearm_timer(struct qemu_alarm_timer *t, int64_t delta) { -int nearest_delta_ms = (delta + 99) / 100; +int64_t nearest_delta_ms = delta / 100; if (nearest_delta_ms 1) { nearest_delta_ms = 1; } +/* UINT_MAX can be 32 bit */ +if (nearest_delta_ms UINT_MAX) { +nearest_delta_ms = UINT_MAX; +} timeKillEvent(mm_timer); -mm_timer = timeSetEvent(nearest_delta_ms, +mm_timer = timeSetEvent((unsigned int) nearest_delta_ms, mm_period, mm_alarm_handler, (DWORD_PTR)t, @@ -757,16 +761,20 @@ static void win32_rearm_timer(struct qemu_alarm_timer *t, int64_t nearest_delta_ns) { HANDLE hTimer = t-timer; -int nearest_delta_ms; +int64_t nearest_delta_ms; BOOLEAN success; -nearest_delta_ms = (nearest_delta_ns + 99) / 100; +nearest_delta_ms = nearest_delta_ns / 100; if (nearest_delta_ms 1) { nearest_delta_ms = 1; } +/* ULONG_MAX can be 32 bit */ +if (nearest_delta_ms ULONG_MAX) { +nearest_delta_ms = ULONG_MAX; +} success = ChangeTimerQueueTimer(NULL, hTimer, -nearest_delta_ms, +(unsigned long) nearest_delta_ms, 360); if (!success) { -- 1.7.2.5
[Qemu-devel] [PATCH v4 2/6] xen: disable rtc_clock
rtc_clock is only used by the RTC emulator (mc146818rtc.c), however Xen has its own RTC emulator in the hypervisor so we can disable it. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com --- xen-all.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/xen-all.c b/xen-all.c index fd39168..101c962 100644 --- a/xen-all.c +++ b/xen-all.c @@ -514,6 +514,10 @@ void xen_vcpu_init(void) qemu_register_reset(xen_reset_vcpu, first_cpu); xen_reset_vcpu(first_cpu); } +/* if rtc_clock is left to default (host_clock), disable it */ +if (rtc_clock == host_clock) { +qemu_clock_enable(rtc_clock, false); +} } /* get the ioreq packets from share mem */ -- 1.7.2.5
[Qemu-devel] [PATCH v4 5/6] qemu_next_alarm_deadline: check the expire time of a clock only if it is enabled
Also delta in qemu_next_alarm_deadline is a 64 bit value so set the default to INT64_MAX instead of INT32_MAX. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com --- qemu-timer.c | 10 -- 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/qemu-timer.c b/qemu-timer.c index 29410f1..de20852 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -106,23 +106,21 @@ static inline int alarm_has_dynticks(struct qemu_alarm_timer *t) static int64_t qemu_next_alarm_deadline(void) { -int64_t delta; +int64_t delta = INT64_MAX; int64_t rtdelta; -if (!use_icount vm_clock-active_timers) { +if (!use_icount vm_clock-enabled vm_clock-active_timers) { delta = vm_clock-active_timers-expire_time - qemu_get_clock_ns(vm_clock); -} else { -delta = INT32_MAX; } -if (host_clock-active_timers) { +if (host_clock-enabled host_clock-active_timers) { int64_t hdelta = host_clock-active_timers-expire_time - qemu_get_clock_ns(host_clock); if (hdelta delta) { delta = hdelta; } } -if (rt_clock-active_timers) { +if (rt_clock-enabled rt_clock-active_timers) { rtdelta = (rt_clock-active_timers-expire_time - qemu_get_clock_ns(rt_clock)); if (rtdelta delta) { -- 1.7.2.5
[Qemu-devel] [PATCH v4 6/6] qemu_calculate_timeout: increase minimum timeout to 1h
There is no reason why the minimum timeout should be 1sec, it could easily be 1h and we would save lots of cpu cycles. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com --- qemu-timer.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/qemu-timer.c b/qemu-timer.c index de20852..84b970e 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -823,6 +823,6 @@ fail: int qemu_calculate_timeout(void) { -return 1000; +return 1000*60*60; } -- 1.7.2.5
Re: [Qemu-devel] [PATCH 06/15] piix: create i8254 through composition
On 01/31/2012 08:58 AM, Paolo Bonzini wrote: On 01/31/2012 03:51 PM, Jan Kiszka wrote: BTW, this is yet another benefit of making structures public. You can take the address of a child and set link fields directly without accessors. Well, that has two sides. We introduced properties to avoid this direct messing. Does linking also work without exposing internals? Perhaps it doesn't need to expose internals. Just like we create interfaces automatically when creating a parent object, perhaps we can create children as well, like TypeInfo type_piix3 = { ... .children = { { pic[0], TYPE_I8259, offsetof(PIIX3, pic[0]) }, { pic[1], TYPE_I8259, offsetof(PIIX3, pic[1]) }, { pit, TYPE_I8254, offsetof(PIIX3, pit) }, { rtc, TYPE_RTC, offsetof(PIIX3, rtc) }, Eeek. I absolutely want to avoid any offset based interfaces. You can just as well do: void object_property_add_child(Object *obj, const char *name, const char *type, Object **child); It could then do: *child = object_new(type); Regards, Anthony Liguori { } } } QOM's object_init would allocate a single malloced block for all of them, carve out space for the parent and all children, and add the properties. Paolo