[Qemu-devel] Any other cross ISA and system mode binary translator like QEMU?
Hi all, Most binary translators I am aware of are user mode, i.e., they are only used to run guest application not OS. The system mode binary translators which do cross ISA translation I know so far are Transmeta CMS, IBM DAISY and QEMU. Any others out there? Thanks. Regards, chenwj [1] -- 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 V4 2/5] xen: Introduce xen_modified_memory.
On Thu, 27 Sep 2012, Anthony PERARD wrote: > This function is to be used during live migration. Every write access to the > guest memory should call this funcion so the Xen tools knows which pages are > dirty. > > Signed-off-by: Anthony PERARD Acked-by: Stefano Stabellini > hw/xen.h | 1 + > xen-all.c | 21 + > xen-stub.c | 4 > 3 files changed, 26 insertions(+) > > diff --git a/hw/xen.h b/hw/xen.h > index e5926b7..d14e92d 100644 > --- a/hw/xen.h > +++ b/hw/xen.h > @@ -48,6 +48,7 @@ void xenstore_store_pv_console_info(int i, struct > CharDriverState *chr); > struct MemoryRegion; > void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, > struct MemoryRegion *mr); > +void xen_modified_memory(ram_addr_t start, ram_addr_t length); > #endif > > struct MemoryRegion; > diff --git a/xen-all.c b/xen-all.c > index f75ae9f..b11542c 100644 > --- a/xen-all.c > +++ b/xen-all.c > @@ -1228,3 +1228,24 @@ void xen_shutdown_fatal_error(const char *fmt, ...) > /* destroy the domain */ > qemu_system_shutdown_request(); > } > + > +void xen_modified_memory(ram_addr_t start, ram_addr_t length) > +{ > +if (unlikely(xen_in_migration)) { > +int rc; > +ram_addr_t start_pfn, nb_pages; > + > +if (length == 0) { > +length = TARGET_PAGE_SIZE; > +} > +start_pfn = start >> TARGET_PAGE_BITS; > +nb_pages = ((start + length + TARGET_PAGE_SIZE - 1) >> > TARGET_PAGE_BITS) > +- start_pfn; > +rc = xc_hvm_modified_memory(xen_xc, xen_domid, start_pfn, nb_pages); > +if (rc) { > +fprintf(stderr, > +"%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i, > %s\n", > +__func__, start, nb_pages, rc, strerror(-rc)); > +} > +} > +} > diff --git a/xen-stub.c b/xen-stub.c > index 5e66ba8..9214392 100644 > --- a/xen-stub.c > +++ b/xen-stub.c > @@ -59,3 +59,7 @@ void xen_register_framebuffer(MemoryRegion *mr) > void qmp_xen_set_global_dirty_log(bool enable, Error **errp) > { > } > + > +void xen_modified_memory(ram_addr_t start, ram_addr_t length) > +{ > +} > -- > Anthony PERARD >
Re: [Qemu-devel] [PATCH V4 5/5] xen: Set the vram dirty when an error occur.
On Thu, 27 Sep 2012, Anthony PERARD wrote: > If the call to xc_hvm_track_dirty_vram() fails, then we set dirtybit on all > the > video ram. This case happens during migration. > > Signed-off-by: Anthony PERARD Acked-by: Stefano Stabellini > xen-all.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/xen-all.c b/xen-all.c > index b11542c..e6308be 100644 > --- a/xen-all.c > +++ b/xen-all.c > @@ -507,7 +507,8 @@ static void xen_sync_dirty_bitmap(XenIOState *state, > bitmap); > if (rc < 0) { > if (rc != -ENODATA) { > -fprintf(stderr, "xen: track_dirty_vram failed (0x" TARGET_FMT_plx > +memory_region_set_dirty(framebuffer, 0, size); > +DPRINTF("xen: track_dirty_vram failed (0x" TARGET_FMT_plx > ", 0x" TARGET_FMT_plx "): %s\n", > start_addr, start_addr + size, strerror(-rc)); > } > -- > Anthony PERARD >
Re: [Qemu-devel] [PATCH V4 4/5] exec, memory: Call to xen_modified_memory.
On Thu, 27 Sep 2012, Anthony PERARD wrote: > This patch add some calls to xen_modified_memory to notify Xen about dirtybits > during migration. > > Signed-off-by: Anthony PERARD If I am not mistaken, this is the last patch that needs reviewing. Avi, are you OK with it? > exec.c | 1 + > memory.c | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/exec.c b/exec.c > index 366684c..1114a09 100644 > --- a/exec.c > +++ b/exec.c > @@ -3427,6 +3427,7 @@ static void invalidate_and_set_dirty(target_phys_addr_t > addr, > /* set dirty bit */ > cpu_physical_memory_set_dirty_flags(addr, (0xff & ~CODE_DIRTY_FLAG)); > } > +xen_modified_memory(addr, length); > } > > void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf, > diff --git a/memory.c b/memory.c > index 4f3ade0..015c544 100644 > --- a/memory.c > +++ b/memory.c > @@ -19,6 +19,7 @@ > #include "bitops.h" > #include "kvm.h" > #include > +#include "hw/xen.h" > > #define WANT_EXEC_OBSOLETE > #include "exec-obsolete.h" > @@ -1077,6 +1078,7 @@ void memory_region_set_dirty(MemoryRegion *mr, > target_phys_addr_t addr, > target_phys_addr_t size) > { > assert(mr->terminates); > +xen_modified_memory(mr->ram_addr + addr, size); > return cpu_physical_memory_set_dirty_range(mr->ram_addr + addr, size, > -1); > } > > -- > Anthony PERARD >
Re: [Qemu-devel] a user here - pci-assign
hmm, still cannot get readon 5450 to work on win7-64, have changed -cpu to host but no fix no vfio in qemu-kvm-0.12.1.2-2.295.el6_3.2.x86_64 yes, I do blacklist modules at grub level and later in modprobe also is primary/secondary VGA setup somehow helped by qemu/components? in guest I can do anything since device is disabled. pci-assign.multifunction=on/off - that would be the case with VGAs like radeon and nvidia - audio part - is it optional or always has to be ON for such devices? where one gets hold of information like: addr= ? I understand these are needed only! if pci-assign.host is not enough and qemu has no way of knowing/finding it, when may this happen? many thanks On 28/09/12 20:48, Alex Williamson wrote: On Fri, 2012-09-28 at 19:46 +0100, lejeczek wrote: thanks Alex for your patience, appreciate it I do what would be the droids I need? I'm experiencing guests' "puzzling" behavior and was suggested that command line arguments were wrong/incomplete. same box/hardware and radeon 5450 and ... - winXP-32bit -> OK - I assumed getting the guest on a external computer monitor was an ultimate test - win7-64bit -> OS reports device as disabled cause the device reported an error I've had this same card working with both of these guests. I believe one trick on win7 was to use -cpu host. Also, don't try to disable the emulated vga device, just set it up like a dual-head system. Once you load the catalyst driver windows will switch to use the assigned device exclusively. I was using the new vfio assignment driver, but someone else recently report it working with the existing driver as well. The 5450 should be a secondary graphics card on the host system, trying to assign the primary graphics is going to cause more problems. Also blacklist the radeon driver on the host, we don't need any leftover state from the Linux driver causing problems since most of these graphics cards don't support a reset mechanism. same box as above and geforce gt640 and .. for both XP and 7 report device with exclamation marks (thus did not even bother to connect any screens like in working case with XP & radeon) I don't think we've seen any reports yet of Nvidia cards working, there's another thread on the kvm list speculating at some of the problems. I'll try to get hold of ROMs of the cards, meanwhile, how can I troubleshoot it? how to get more verbose feedback and what to specifically look for? ROMs are only going to help if you're getting errors trying to read the ROM. Nvidia seems to have this problem, but I don't think radeons typical do. There's a #define in the code that can be enabled to get more logging, but it's not terribly useful unless you know what you're looking at. VGA has plenty of issues with legacy PC address ranges that are known problems, but there are also plenty of unknown problems that make it a pretty difficult black box debugging project. Thanks, Alex
Re: [Qemu-devel] pwrite64 error because of argument position
On 30.09.2012, at 20:50, Alex Barcelo wrote: > This error may be a PPC specific problem, but I don't have another > environment where I can test it (i386 doesn't seem to use pwrite64), > so I ask for a bit of help/check. > > I am in a 32bit linux testing the qemu-ppc. > > My test program: > > // --- > #include > #include > #include > #include > #include > > int main (void) { >int fd, len; > >char* asd = "This is a random test"; >char asd2[20]; > >fd = open ("./test.pwrite", O_RDWR | O_CREAT); > >printf ( "fd: %d\n", fd); >pwrite ( fd, asd, 15, 0 ); >pwrite ( fd, asd+5, 6, 10); >pwrite ( fd, asd, 4, 30); > >len = pread ( fd, asd2, 5, 5); >asd2[len]='\0'; >printf ( "Read %d bytes: %s", len, asd2); > >// This is to force two int arguments for 64bit >//len = pread ( fd, asd2, -1, -1); >close(fd); >return 0; > } > // --- > > Then I do > $ powerpc-linux-gnu-gcc -g --static -o pwrite-alien pwrite-test.c > > and when I launch a qemu-ppc to test, it should be (on the file) > This is a is a rThis > > instead I get: > This rs a randorThis > > and if I print some debug information inside pwrite64 and pread64 I see: > syscall: pwrite arg_i: 3 268909324 15 0 0 0 0 0 > syscall: pwrite arg_i: 3 268909329 6 0 0 10 0 0 > syscall: pwrite arg_i: 3 268909324 4 0 0 30 0 0 > syscall: preadarg_i: 3 1082133156 5 0 0 5 0 0 > (those are arg1, arg2, arg3, arg4, arg5, arg6 and the unused arg7 and arg8) > > As can be seen, arg4 is not used, and the line > ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5))); > should be, in my case > ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg5, arg6))); > > With this changes, the qemu "Works For Me (TM)". > > So, anybody can confirm it or (if it is indeed my problem) can give me > some pointers? I will not post this as a patch until I understand the > problem... and first step is making sure that it really is a qemu > problem. And not my toolchain or something random. Running the above code on a real ppc machine (compiled with -m32, running a ppc64 kernel) I get: 54 68 69 73 20 69 73 20 61 20 69 73 20 61 20 72 |This is a is a r| 0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 54 68 |..Th| 0020 69 73 |is| as output. Is this the expected result? Running it in qemu-ppc, I get: 54 68 69 73 20 72 73 20 61 20 72 61 6e 64 6f |This rs a rando| 000f So yes, something looks broken here. According to arch/powerpc/kernel/sys_ppc32.c, the ppc32 ABI carries 64bit parameters on odd/even pairs: /* * long long munging: * The 32 bit ABI passes long longs in an odd even register pair. */ which is almost the same as what ARM or MIPS do and which explains why you see arg5/arg6 (r7/r8) used instead of arg4/arg5 (r6/r7). Could you please try the below patch? Alex diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 1a38169..e03b3a8 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -587,12 +587,16 @@ extern int setfsgid(int); extern int setgroups(int, gid_t *); /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */ -#ifdef TARGET_ARM +#ifdef TARGET_ARM static inline int regpairs_aligned(void *cpu_env) { return CPUARMState *)cpu_env)->eabi) == 1) ; } #elif defined(TARGET_MIPS) static inline int regpairs_aligned(void *cpu_env) { return 1; } +#elif defined(TARGET_PPC) && !defined(TARGET_PPC64) +/* PPC32 expects 64bit parameters to be passed on odd/even pairs of registers + which is the same as ARM/MIPS, because we start with r3 as arg1. */ +static inline int regpairs_aligned(void *cpu_env) { return 1; } #else static inline int regpairs_aligned(void *cpu_env) { return 0; } #endif @@ -7419,12 +7423,20 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, #endif #ifdef TARGET_NR_pread64 case TARGET_NR_pread64: +if (regpairs_aligned(cpu_env)) { +arg4 = arg5; +arg5 = arg6; +} if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0))) goto efault; ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5))); unlock_user(p, arg2, ret); break; case TARGET_NR_pwrite64: +if (regpairs_aligned(cpu_env)) { +arg4 = arg5; +arg5 = arg6; +} if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1))) goto efault; ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5)));
Re: [Qemu-devel] [Bug 1058225] [NEW] When setting hardware clock on linux guest, hwclock shows crazy date (in the year 2043)
Il 28/09/2012 19:21, Lucas Meneghel Rodrigues ha scritto: > Public bug reported: > > Very easy to reproduce: > > 1) Build the latest qemu.git (we've captured this on internal automated > testing, verified manually), the commit for reference is: > > 14:07:02 INFO | git commit ID is > 6f8fd2530e9a530f237240daf1c981fa5df7f978 (tag v1.2.0-461-g6f8fd25) > > 2) Install a linux guest in it (caught with RHEL 6.2, verified with > Fedora 17) > > 3) In the linux guest, set the hardware clock with hwclock: > > /sbin/hwclock --set --date "2/2/80 03:04:00" > > 4) Verify if hardware clock was set back to the eighties: > > LC_ALL=C /sbin/hwclock > > 5) Observe amazed that hwclock reports a date in the year 2043: > > 14:09:34 INFO | ('hwclock', 'FAIL', 2, "Failed to set hwclock > back to the eighties. Output of hwclock is 'Sun Dec 27 20:35:46 2043 > -0.489664 seconds'") I can reproduce this with qtest. The test is bogus. Linux (drivers/rtc/rtc-cmos.c) doesn't set the century byte, so it may fail if you set a date that is before 2000. Setting a date in 2011 works. However, there is of course a bug, which was latent so far and made visible by the recent algorithmic changes for the RTC. The strange date arises because you actually set the date to 2080. mktimegm (in cutils.c) has an year-2038 overflow. I'll send a patchset to fix this and implement the century byte correctly. Paolo
[Qemu-devel] [PATCH 1/2] linux-user: ppc: mark as long long aligned
The PPC32 ABI dictates that long long (64bit) parameters are pass in odd/even register pairs. Because unlike ARM and MIPS we start at an odd register number, we can reuse the same aligning code that ARM and MIPS use. Signed-off-by: Alexander Graf --- linux-user/syscall.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 1a38169..8cd56f2 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -587,12 +587,16 @@ extern int setfsgid(int); extern int setgroups(int, gid_t *); /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */ -#ifdef TARGET_ARM +#ifdef TARGET_ARM static inline int regpairs_aligned(void *cpu_env) { return CPUARMState *)cpu_env)->eabi) == 1) ; } #elif defined(TARGET_MIPS) static inline int regpairs_aligned(void *cpu_env) { return 1; } +#elif defined(TARGET_PPC) && !defined(TARGET_PPC64) +/* PPC32 expects 64bit parameters to be passed on odd/even pairs of registers + which translates to the same as ARM/MIPS, because we start with r3 as arg1 */ +static inline int regpairs_aligned(void *cpu_env) { return 1; } #else static inline int regpairs_aligned(void *cpu_env) { return 0; } #endif -- 1.6.0.2
[Qemu-devel] [PATCH 2/2] linux-user: register align p{read, write}64
pread64 and pwrite64 pass 64bit parameters which for some architectures need to be aligned to special argument pairs, creating a gap argument. Handle this special case the same way we handle it in other places of the code. Reported-by: Alex Barcelo Signed-off-by: Alexander Graf --- linux-user/syscall.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 8cd56f2..7992b1b 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -7423,12 +7423,20 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, #endif #ifdef TARGET_NR_pread64 case TARGET_NR_pread64: +if (regpairs_aligned(cpu_env)) { +arg4 = arg5; +arg5 = arg6; +} if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0))) goto efault; ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5))); unlock_user(p, arg2, ret); break; case TARGET_NR_pwrite64: +if (regpairs_aligned(cpu_env)) { +arg4 = arg5; +arg5 = arg6; +} if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1))) goto efault; ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5))); -- 1.6.0.2
[Qemu-devel] [PATCH] hw/arm_gic.c: Fix improper DPRINTF output.
s->cpu_enabled is a massive, so s->cpu_enabled ? "En" : "Dis" returns "En" always. We should use s->cpu_enabled[cpu] here. Signed-off-by: Evgeny Voevodin --- hw/arm_gic.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm_gic.c b/hw/arm_gic.c index 55871fa..4024dae 100644 --- a/hw/arm_gic.c +++ b/hw/arm_gic.c @@ -566,7 +566,7 @@ static void gic_cpu_write(gic_state *s, int cpu, int offset, uint32_t value) switch (offset) { case 0x00: /* Control */ s->cpu_enabled[cpu] = (value & 1); -DPRINTF("CPU %d %sabled\n", cpu, s->cpu_enabled ? "En" : "Dis"); +DPRINTF("CPU %d %sabled\n", cpu, s->cpu_enabled[cpu] ? "En" : "Dis"); break; case 0x04: /* Priority mask */ s->priority_mask[cpu] = (value & 0xff); -- 1.7.9.5
[Qemu-devel] [PATCH] qtest: implement QTEST_STOP
It is quite difficult to debug qtest test cases without extra wrapper scripts for QEMU or similar. This patch adds a simple environment variable-based trigger that sends a STOP signal to the QEMU instance under test, before attempting to connect to its QMP session. This will block execution of the testcase and give time to attach a debugger to the stopped QEMU process. Signed-off-by: Paolo Bonzini --- tests/libqtest.c | 38 +- 1 file modificato, 25 inserzioni(+), 13 rimozioni(-) diff --git a/tests/libqtest.c b/tests/libqtest.c index 02d0392..71b84c1 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -85,6 +85,22 @@ static int socket_accept(int sock) return ret; } +static pid_t qtest_qemu_pid(QTestState *s) +{ +FILE *f; +char buffer[1024]; +pid_t pid = -1; + +f = fopen(s->pid_file, "r"); +if (f) { +if (fgets(buffer, sizeof(buffer), f)) { +pid = atoi(buffer); +} +} +fclose(f); +return pid; +} + QTestState *qtest_init(const char *extra_args) { QTestState *s; @@ -136,25 +152,21 @@ QTestState *qtest_init(const char *extra_args) qtest_qmp(s, ""); qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"); +if (getenv("QTEST_STOP")) { +kill(qtest_qemu_pid(s), SIGSTOP); +} + return s; } void qtest_quit(QTestState *s) { -FILE *f; -char buffer[1024]; - -f = fopen(s->pid_file, "r"); -if (f) { -if (fgets(buffer, sizeof(buffer), f)) { -pid_t pid = atoi(buffer); -int status = 0; - -kill(pid, SIGTERM); -waitpid(pid, &status, 0); -} +int status; -fclose(f); +pid_t pid = qtest_qemu_pid(s); +if (pid != -1) { +kill(pid, SIGTERM); +waitpid(pid, &status, 0); } unlink(s->pid_file); -- 1.7.12
[Qemu-devel] [PATCH 0/3] Fix autotest-exposed RTC bug 1058225
> Very easy to reproduce: > > 1) Build the latest qemu.git (we've captured this on internal automated > testing, verified manually), the commit for reference is: > > 14:07:02 INFO | git commit ID is > 6f8fd2530e9a530f237240daf1c981fa5df7f978 (tag v1.2.0-461-g6f8fd25) > > 2) Install a linux guest in it (caught with RHEL 6.2, verified with > Fedora 17) > > 3) In the linux guest, set the hardware clock with hwclock: > > /sbin/hwclock --set --date "2/2/80 03:04:00" > > 4) Verify if hardware clock was set back to the eighties: > > LC_ALL=C /sbin/hwclock > > 5) Observe amazed that hwclock reports a date in the year 2043: > > 14:09:34 INFO | ('hwclock', 'FAIL', 2, "Failed to set hwclock > back to the eighties. Output of hwclock is 'Sun Dec 27 20:35:46 2043 > -0.489664 seconds'") The testcase of patch 1 exposes the bug on an unpatched QEMU. Paolo Paolo Bonzini (3): rtc: fix overflow in mktimegm rtc: map CMOS index 0x37 to 0x32 on read and writes rtc: implement century byte cutils.c | 2 +- hw/mc146818rtc.c | 40 ++-- hw/mc146818rtc_regs.h | 4 +++ tests/rtc-test.c | 73 +++ 4 file modificati, 104 inserzioni(+), 15 rimozioni(-) -- 1.7.12
[Qemu-devel] [PATCH 1/3] rtc: fix overflow in mktimegm
When setting a date in 1980, Linux is actually disregarding the century byte and setting the year to 2080. This causes a year-2038 overflow in mktimegm. Fix this by doing the days-to-seconds computation in 64-bit math. Reported-by: Lucas Meneghel Rodrigues Signed-off-by: Paolo Bonzini --- cutils.c | 2 +- tests/rtc-test.c | 45 + 2 file modificati, 46 inserzioni(+). 1 rimozione(-) diff --git a/cutils.c b/cutils.c index 8ef648f..8edd8fa 100644 --- a/cutils.c +++ b/cutils.c @@ -115,7 +115,7 @@ time_t mktimegm(struct tm *tm) m += 12; y--; } -t = 86400 * (d + (153 * m - 457) / 5 + 365 * y + y / 4 - y / 100 + +t = 86400ULL * (d + (153 * m - 457) / 5 + 365 * y + y / 4 - y / 100 + y / 400 - 719469); t += 3600 * tm->tm_hour + 60 * tm->tm_min + tm->tm_sec; return t; diff --git a/tests/rtc-test.c b/tests/rtc-test.c index f23ac3a..2b9aa63 100644 --- a/tests/rtc-test.c +++ b/tests/rtc-test.c @@ -179,6 +179,50 @@ static void check_time(int wiggle) static int wiggle = 2; +static void set_year(void) +{ +/* Set BCD mode */ +cmos_write(RTC_REG_B, cmos_read(RTC_REG_B) & ~REG_B_DM); +cmos_write(RTC_REG_A, 0x76); +cmos_write(RTC_YEAR, 0x11); +cmos_write(RTC_MONTH, 0x02); +cmos_write(RTC_DAY_OF_MONTH, 0x02); +cmos_write(RTC_HOURS, 0x02); +cmos_write(RTC_MINUTES, 0x04); +cmos_write(RTC_SECONDS, 0x58); +cmos_write(RTC_REG_A, 0x26); + +g_assert_cmpint(cmos_read(RTC_HOURS), ==, 0x02); +g_assert_cmpint(cmos_read(RTC_MINUTES), ==, 0x04); +g_assert_cmpint(cmos_read(RTC_SECONDS), >=, 0x58); +g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02); +g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02); +g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x11); + +/* Set a date in 2080 to ensure there is no year-2038 overflow. */ +cmos_write(RTC_REG_A, 0x76); +cmos_write(RTC_YEAR, 0x80); +cmos_write(RTC_REG_A, 0x26); + +g_assert_cmpint(cmos_read(RTC_HOURS), ==, 0x02); +g_assert_cmpint(cmos_read(RTC_MINUTES), ==, 0x04); +g_assert_cmpint(cmos_read(RTC_SECONDS), >=, 0x58); +g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02); +g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02); +g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x80); + +cmos_write(RTC_REG_A, 0x76); +cmos_write(RTC_YEAR, 0x11); +cmos_write(RTC_REG_A, 0x26); + +g_assert_cmpint(cmos_read(RTC_HOURS), ==, 0x02); +g_assert_cmpint(cmos_read(RTC_MINUTES), ==, 0x04); +g_assert_cmpint(cmos_read(RTC_SECONDS), >=, 0x58); +g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02); +g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02); +g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x11); +} + static void bcd_check_time(void) { /* Set BCD mode */ @@ -269,6 +313,7 @@ int main(int argc, char **argv) qtest_add_func("/rtc/bcd/check-time", bcd_check_time); qtest_add_func("/rtc/dec/check-time", dec_check_time); qtest_add_func("/rtc/alarm-time", alarm_time); +qtest_add_func("/rtc/set-year", set_year); qtest_add_func("/rtc/fuzz-registers", fuzz_registers); ret = g_test_run(); -- 1.7.12
[Qemu-devel] [PATCH 2/3] rtc: map CMOS index 0x37 to 0x32 on read and writes
QEMU's attempt to implement the century byte cover two possible places for the byte. A common one on modern chipsets is 0x32, but QEMU also stores the value in 0x37 (apparently for IBM PS/2 compatibility---it's only been 25 years). To simplify the implementation of the century byte, store it only at 0x32 but remap transparently 0x37 to 0x32 when reading and writing from CMOS. Signed-off-by: Paolo Bonzini --- hw/mc146818rtc.c | 15 +-- hw/mc146818rtc_regs.h | 4 2 file modificati, 13 inserzioni(+), 6 rimozioni(-) diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c index d63554f..a7d20d5 100644 --- a/hw/mc146818rtc.c +++ b/hw/mc146818rtc.c @@ -399,6 +399,10 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data) s->cmos_data[s->cmos_index] = data; check_update_timer(s); break; + case RTC_IBM_PS2_CENTURY_BYTE: +s->cmos_index = RTC_CENTURY; +/* fall through */ +case RTC_CENTURY: case RTC_SECONDS: case RTC_MINUTES: case RTC_HOURS: @@ -598,6 +602,10 @@ static uint32_t cmos_ioport_read(void *opaque, uint32_t addr) return 0xff; } else { switch(s->cmos_index) { + case RTC_IBM_PS2_CENTURY_BYTE: +s->cmos_index = RTC_CENTURY; +/* fall through */ +case RTC_CENTURY: case RTC_SECONDS: case RTC_MINUTES: case RTC_HOURS: @@ -661,10 +669,6 @@ void rtc_set_memory(ISADevice *dev, int addr, int val) s->cmos_data[addr] = val; } -/* PC cmos mappings */ -#define REG_IBM_CENTURY_BYTE0x32 -#define REG_IBM_PS2_CENTURY_BYTE0x37 - static void rtc_set_date_from_host(ISADevice *dev) { RTCState *s = DO_UPCAST(RTCState, dev, dev); @@ -681,8 +685,7 @@ static void rtc_set_date_from_host(ISADevice *dev) rtc_set_cmos(s, &tm); val = rtc_to_bcd(s, (tm.tm_year / 100) + 19); -rtc_set_memory(dev, REG_IBM_CENTURY_BYTE, val); -rtc_set_memory(dev, REG_IBM_PS2_CENTURY_BYTE, val); +rtc_set_memory(dev, RTC_CENTURY, val); } static int rtc_post_load(void *opaque, int version_id) diff --git a/hw/mc146818rtc_regs.h b/hw/mc146818rtc_regs.h index fc10076..ccdee42 100644 --- a/hw/mc146818rtc_regs.h +++ b/hw/mc146818rtc_regs.h @@ -44,6 +44,10 @@ #define RTC_REG_C 12 #define RTC_REG_D 13 +/* PC cmos mappings */ +#define RTC_CENTURY 0x32 +#define RTC_IBM_PS2_CENTURY_BYTE 0x37 + #define REG_A_UIP 0x80 #define REG_B_SET 0x80 -- 1.7.12
[Qemu-devel] [PATCH 3/3] rtc: implement century byte
Implement the century byte in the RTC emulation, and test that it works. This leads to some annoying compatibility code because we need to treat a value of 2000 for the base_year property as "use the century byte properly" (which would be a value of 0). The century byte will now be always-zero, rather than always-20, for the MIPS Magnum machine whose base_year is 1980. Commit 42fc73a (Support epoch of 1980 in RTC emulation for MIPS Magnum, 2009-01-24) correctly said: With an epoch of 1980 and a year of 2009, one could argue that [the century byte] should hold either 0, 1, 19 or 20. NT 3.50 on MIPS does not read the century byte. so I picked the simplest and most sensible implementation which is to return 0 for 1980-2079, 1 for 2080-2179 and so on. Signed-off-by: Paolo Bonzini --- hw/mc146818rtc.c | 27 ++- tests/rtc-test.c | 32 ++-- 2 file modificati, 48 inserzioni(+), 11 rimozioni(-) diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c index a7d20d5..332a77d 100644 --- a/hw/mc146818rtc.c +++ b/hw/mc146818rtc.c @@ -519,7 +519,9 @@ static void rtc_get_time(RTCState *s, struct tm *tm) tm->tm_wday = rtc_from_bcd(s, s->cmos_data[RTC_DAY_OF_WEEK]) - 1; tm->tm_mday = rtc_from_bcd(s, s->cmos_data[RTC_DAY_OF_MONTH]); tm->tm_mon = rtc_from_bcd(s, s->cmos_data[RTC_MONTH]) - 1; -tm->tm_year = rtc_from_bcd(s, s->cmos_data[RTC_YEAR]) + s->base_year - 1900; +tm->tm_year = +rtc_from_bcd(s, s->cmos_data[RTC_YEAR]) + s->base_year + +rtc_from_bcd(s, s->cmos_data[RTC_CENTURY]) * 100 - 1900; } static void rtc_set_time(RTCState *s) @@ -552,10 +554,9 @@ static void rtc_set_cmos(RTCState *s, const struct tm *tm) s->cmos_data[RTC_DAY_OF_WEEK] = rtc_to_bcd(s, tm->tm_wday + 1); s->cmos_data[RTC_DAY_OF_MONTH] = rtc_to_bcd(s, tm->tm_mday); s->cmos_data[RTC_MONTH] = rtc_to_bcd(s, tm->tm_mon + 1); -year = (tm->tm_year - s->base_year) % 100; -if (year < 0) -year += 100; -s->cmos_data[RTC_YEAR] = rtc_to_bcd(s, year); +year = tm->tm_year + 1900 - s->base_year; +s->cmos_data[RTC_YEAR] = rtc_to_bcd(s, year % 100); +s->cmos_data[RTC_CENTURY] = rtc_to_bcd(s, year / 100); } static void rtc_update_time(RTCState *s) @@ -673,7 +674,6 @@ static void rtc_set_date_from_host(ISADevice *dev) { RTCState *s = DO_UPCAST(RTCState, dev, dev); struct tm tm; -int val; qemu_get_timedate(&tm, 0); @@ -683,9 +683,6 @@ static void rtc_set_date_from_host(ISADevice *dev) /* set the CMOS date */ rtc_set_cmos(s, &tm); - -val = rtc_to_bcd(s, (tm.tm_year / 100) + 19); -rtc_set_memory(dev, RTC_CENTURY, val); } static int rtc_post_load(void *opaque, int version_id) @@ -810,6 +807,18 @@ static int rtc_initfn(ISADevice *dev) s->cmos_data[RTC_REG_C] = 0x00; s->cmos_data[RTC_REG_D] = 0x80; +/* This is for historical reasons. The default base year qdev property + * was set to 2000 for most machine types before the century byte was + * implemented. + * + * This if statement means that the century byte will be always 0 + * (at least until 2079...) for base_year = 1980, but will be set + * correctly for base_year = 2000. + */ +if (s->base_year == 2000) { +s->base_year = 0; +} + rtc_set_date_from_host(dev); #ifdef TARGET_I386 diff --git a/tests/rtc-test.c b/tests/rtc-test.c index 2b9aa63..7fdc94a 100644 --- a/tests/rtc-test.c +++ b/tests/rtc-test.c @@ -179,12 +179,13 @@ static void check_time(int wiggle) static int wiggle = 2; -static void set_year(void) +static void set_year_20xx(void) { /* Set BCD mode */ cmos_write(RTC_REG_B, cmos_read(RTC_REG_B) & ~REG_B_DM); cmos_write(RTC_REG_A, 0x76); cmos_write(RTC_YEAR, 0x11); +cmos_write(RTC_CENTURY, 0x20); cmos_write(RTC_MONTH, 0x02); cmos_write(RTC_DAY_OF_MONTH, 0x02); cmos_write(RTC_HOURS, 0x02); @@ -198,6 +199,7 @@ static void set_year(void) g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02); g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02); g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x11); +g_assert_cmpint(cmos_read(RTC_CENTURY), ==, 0x20); /* Set a date in 2080 to ensure there is no year-2038 overflow. */ cmos_write(RTC_REG_A, 0x76); @@ -210,6 +212,7 @@ static void set_year(void) g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02); g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02); g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x80); +g_assert_cmpint(cmos_read(RTC_CENTURY), ==, 0x20); cmos_write(RTC_REG_A, 0x76); cmos_write(RTC_YEAR, 0x11); @@ -221,6 +224,30 @@ static void set_year(void) g_assert_cmpint(cmos_read(RTC_DAY_OF_MONTH), ==, 0x02); g_assert_cmpint(cmos_read(RTC_MONTH), ==, 0x02); g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x11); +g_assert_cmpint(cmos_read(RTC_CENTURY), ==, 0x20); +} + +static void set_year_1980(void) +{ +
Re: [Qemu-devel] [PATCH] hw/arm_gic.c: Fix improper DPRINTF output.
On 1 October 2012 13:11, Evgeny Voevodin wrote: > s->cpu_enabled is a massive, so s->cpu_enabled ? "En" : "Dis" returns "is an array" > "En" always. We should use s->cpu_enabled[cpu] here. > > Signed-off-by: Evgeny Voevodin ...but nice catch. Reviewed-by: Peter Maydell -- PMM
Re: [Qemu-devel] [PATCH 18/22] target-i386: parse cpu_model string into set of stringified properties
On Sun, 30 Sep 2012 12:16:27 + Blue Swirl wrote: > This would break build since strtok_r() isn't available on Mingw, it's > not Posix either. How about g_strsplit()? Thanks! Updated series at: https://github.com/imammedo/qemu/tree/x86-cpu-properties.WIP I'll post fixed [7/22] and [18/22] patches as followup to this thread.
[Qemu-devel] [PATCH 07/22 v4] target-i386: convert cpuid features into properties
add property accessors for cpuid feature bits defined by *_feature_name arrays. Signed-off-by: Igor Mammedov --- v2: * replaced mask/ffs tricks by plain 'for (bit = 0; bit < 32; bit++)' as suggested by Eduardo Habkost v3: * check if property exists before adding it * rebased on top of "i386: cpu: remove duplicate feature names" http://www.mail-archive.com/qemu-devel@nongnu.org/msg129458.html place ext2_feature_name for AMD case into setter, so that not to clutter x86_cpu_realize() with property specific code. * fix for convert cpuid features v4: * Mingw doesn't have strtok_r(), use g_strsplit() instead. --- target-i386/cpu.c | 111 + 1 files changed, 111 insertions(+), 0 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index a713960..f990be2 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -833,6 +833,111 @@ static int check_features_against_host(x86_def_t *guest_def) return rv; } +static bool is_feature_set(const char *name, const uint32_t featbitmap, + const char **featureset) +{ +uint32_t bit; + +for (bit = 0; bit < 32; ++bit) { +if (featureset[bit] && !altcmp(name, NULL, featureset[bit])) { +if (featbitmap & (1 << bit)) { +return true; +} +} +} +return false; +} + +static void x86_cpuid_get_feature(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +X86CPU *cpu = X86_CPU(obj); +CPUX86State *env = &cpu->env; +bool value = true; + +if (!is_feature_set(name, env->cpuid_features, feature_name) && + !is_feature_set(name, env->cpuid_ext_features, ext_feature_name) && + !is_feature_set(name, env->cpuid_ext2_features, ext2_feature_name) && + !is_feature_set(name, env->cpuid_ext3_features, ext3_feature_name) && + !is_feature_set(name, env->cpuid_kvm_features, kvm_feature_name) && + !is_feature_set(name, env->cpuid_svm_features, svm_feature_name)) { +value = false; +} + +visit_type_bool(v, &value, name, errp); +} + +static void x86_cpuid_set_feature(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +X86CPU *cpu = X86_CPU(obj); +CPUX86State *env = &cpu->env; +uint32_t mask = 0; +uint32_t *dst_features; +bool value; + +visit_type_bool(v, &value, name, errp); +if (error_is_set(errp)) { +return; +} + +if (lookup_feature(&mask, name, NULL, feature_name)) { +dst_features = &env->cpuid_features; +} else if (lookup_feature(&mask, name, NULL, ext_feature_name)) { +dst_features = &env->cpuid_ext_features; +} else if (lookup_feature(&mask, name, NULL, ext2_feature_name)) { +dst_features = &env->cpuid_ext2_features; +} else if (lookup_feature(&mask, name, NULL, ext3_feature_name)) { +dst_features = &env->cpuid_ext3_features; +} else if (lookup_feature(&mask, name, NULL, kvm_feature_name)) { +dst_features = &env->cpuid_kvm_features; +} else if (lookup_feature(&mask, name, NULL, svm_feature_name)) { +dst_features = &env->cpuid_svm_features; +} else { +error_set(errp, QERR_PROPERTY_NOT_FOUND, "", name); +return; +} + +if (value) { +*dst_features |= mask; +} else { +*dst_features &= ~mask; +} + +/* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on + * CPUID[1].EDX. + */ +if (dst_features == &env->cpuid_features && +env->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && +env->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && +env->cpuid_vendor3 == CPUID_VENDOR_AMD_3) { +env->cpuid_ext2_features &= ~CPUID_EXT2_AMD_ALIASES; +env->cpuid_ext2_features |= *dst_features & CPUID_EXT2_AMD_ALIASES; +} +} + +static void x86_register_cpuid_properties(Object *obj, const char **featureset) +{ +uint32_t bit; + +for (bit = 0; bit < 32; ++bit) { +if (featureset[bit]) { +char **feature_name; +int i; + +feature_name = g_strsplit(featureset[bit], "|", 0); +for (i = 0; feature_name[i]; ++i) { +if (!object_property_find(obj, feature_name[i], NULL)) { +object_property_add(obj, feature_name[i], "bool", +x86_cpuid_get_feature, +x86_cpuid_set_feature, NULL, NULL, NULL); +} +} +g_strfreev(feature_name); +} +} +} + static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { @@ -1920,6 +2025,12 @@ static void x86_cpu_initfn(Object *obj) object_property_add(obj, "tsc-frequency", "int",
[Qemu-devel] [PATCH 18/22 v3] target-i386: parse cpu_model string into set of stringified properties
cpu_model string does represent features in following format: ([+-]feat)|(feat=foo)|(feat) which makes it impossible directly use property infrastructure to set features on CPU. This patch introduces parser that splits CPU name from cpu_model and converts legacy features string into canonized set of strings that is compatible with property manipulation infrastructure. PS: * later it could be used as a hook to convert legacy command line features to global properties. Then marked as deprecated and removed with -cpu option in the future. Signed-off-by: Igor Mammedov --- v2: * compiler complains that it's unused function but I guess it is easier for review this way, for pull req I'll squash it into next patch * fix spelling error * initialize sptr, due to a CentOS6 compiler warning, that breakes build when -Werror is set. Suggested-by: Don Slutz v3: * Mingw doesn't have strtok_r(), use g_strsplit() instead of it. Suggested-by: Blue Swirl --- target-i386/cpu.c | 51 +++ 1 files changed, 51 insertions(+), 0 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index ad1f088..9cf25f6 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1339,6 +1339,57 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp) } } +/* convert legacy cpumodel string to string cpu_name and + * a uniform set of custom features that will be applied to CPU + * using object_property_parse() + */ +static void compat_normalize_cpu_model(const char *cpu_model, char **cpu_name, +QDict **features, Error **errp) +{ +int i; +gchar **feat_array = g_strsplit(cpu_model, ",", 0); +*features = qdict_new(); + +g_assert(feat_array[0] != NULL); +*cpu_name = g_strdup(feat_array[0]); + +for (i = 1; feat_array[i]; ++i) { +gchar *featurestr = feat_array[i]; +char *val; +if (featurestr[0] == '+') { +/* + * preseve legacy behaviour, if feature was disabled once + * do not allow to enable it again + */ +if (!qdict_haskey(*features, featurestr + 1)) { +qdict_put(*features, featurestr + 1, qstring_from_str("on")); +} +} else if (featurestr[0] == '-') { +qdict_put(*features, featurestr + 1, qstring_from_str("off")); +} else { +val = strchr(featurestr, '='); +if (val) { +*val = 0; val++; +if (!strcmp(featurestr, "vendor")) { +qdict_put(*features, "vendor-override", + qstring_from_str("on")); +qdict_put(*features, featurestr, qstring_from_str(val)); +} else if (!strcmp(featurestr, "tsc_freq")) { +qdict_put(*features, "tsc-frequency", + qstring_from_str(val)); +} else { +qdict_put(*features, featurestr, qstring_from_str(val)); +} +} else { +qdict_put(*features, featurestr, qstring_from_str("on")); +} +} +} + +g_strfreev(feat_array); +return; +} + static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def, const char *cpu_model, Error **errp) { -- 1.7.1
Re: [Qemu-devel] [PATCH 1/2] linux-user: ppc: mark as long long aligned
Am 30.09.2012 03:32, schrieb Alexander Graf: > The PPC32 ABI dictates that long long (64bit) parameters are pass in odd/even > register pairs. Because unlike ARM and MIPS we start at an odd register > number, > we can reuse the same aligning code that ARM and MIPS use. > > Signed-off-by: Alexander Graf > --- > linux-user/syscall.c |6 +- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 1a38169..8cd56f2 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -587,12 +587,16 @@ extern int setfsgid(int); > extern int setgroups(int, gid_t *); > > /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */ > -#ifdef TARGET_ARM > +#ifdef TARGET_ARM For anyone wondering, this is dropping a whitespace at end of line. ;) > static inline int regpairs_aligned(void *cpu_env) { > return CPUARMState *)cpu_env)->eabi) == 1) ; > } > #elif defined(TARGET_MIPS) > static inline int regpairs_aligned(void *cpu_env) { return 1; } > +#elif defined(TARGET_PPC) && !defined(TARGET_PPC64) > +/* PPC32 expects 64bit parameters to be passed on odd/even pairs of registers > + which translates to the same as ARM/MIPS, because we start with r3 as > arg1 */ > +static inline int regpairs_aligned(void *cpu_env) { return 1; } > #else > static inline int regpairs_aligned(void *cpu_env) { return 0; } > #endif It is obvious that this function has been copied unmodified from mips, but shouldn't new code use bool and true, assuming that there is no magic performed on the return value? :) 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 1/2] linux-user: ppc: mark as long long aligned
On 01.10.2012, at 15:04, Andreas Färber wrote: > Am 30.09.2012 03:32, schrieb Alexander Graf: >> The PPC32 ABI dictates that long long (64bit) parameters are pass in odd/even >> register pairs. Because unlike ARM and MIPS we start at an odd register >> number, >> we can reuse the same aligning code that ARM and MIPS use. >> >> Signed-off-by: Alexander Graf >> --- >> linux-user/syscall.c |6 +- >> 1 files changed, 5 insertions(+), 1 deletions(-) >> >> diff --git a/linux-user/syscall.c b/linux-user/syscall.c >> index 1a38169..8cd56f2 100644 >> --- a/linux-user/syscall.c >> +++ b/linux-user/syscall.c >> @@ -587,12 +587,16 @@ extern int setfsgid(int); >> extern int setgroups(int, gid_t *); >> >> /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */ >> -#ifdef TARGET_ARM >> +#ifdef TARGET_ARM > > For anyone wondering, this is dropping a whitespace at end of line. ;) Yeah, my vi scripts marked it as red, which always makes me nervous :). > >> static inline int regpairs_aligned(void *cpu_env) { >> return CPUARMState *)cpu_env)->eabi) == 1) ; >> } >> #elif defined(TARGET_MIPS) >> static inline int regpairs_aligned(void *cpu_env) { return 1; } >> +#elif defined(TARGET_PPC) && !defined(TARGET_PPC64) >> +/* PPC32 expects 64bit parameters to be passed on odd/even pairs of >> registers >> + which translates to the same as ARM/MIPS, because we start with r3 as >> arg1 */ >> +static inline int regpairs_aligned(void *cpu_env) { return 1; } >> #else >> static inline int regpairs_aligned(void *cpu_env) { return 0; } >> #endif > > It is obvious that this function has been copied unmodified from mips, > but shouldn't new code use bool and true, assuming that there is no > magic performed on the return value? :) The asm result should be the same, since they're all getting inlined, no? And it wouldn't tremendously increase readability either. So for now, I'd say either a) send a separate cleanup patch or b) leave it as is :) Alex
Re: [Qemu-devel] qemu-kvm: remove "boot=on|off" drive parameter compatibility
Jan Kiszka writes: > On 2012-10-01 11:31, Marcelo Tosatti wrote: > > It's not just about default configs. We need to validate if the > migration formats are truly compatible (qemu-kvm -> QEMU, the other way > around definitely not). For the command line switches, we could provide > a wrapper script that translates them into upstream format or simply > ignores them. That should be harmless to carry upstream. qemu-kvm has: -no-kvm -no-kvm-irqchip -no-kvm-pit -no-kvm-pit-reinjection -tdf <- does nothing There are replacements for all of the above. If we need to add them to qemu.git, it's not big deal to add them. -drive ...,boot= <- this is ignored cpu_set command for CPU hotplug which is known broken in qemu-kvm. testdev which is nice but only used for development Default nic is rtl8139 vs. e1000. Some logic to move change the default VGA ram size to 16mb for pc-1.2 (QEMU uses 16mb by default now too). I think at this point, none of this matters but I added the various distro maintainers to the thread. I think it's time for the distros to drop qemu-kvm and just ship qemu.git. Is there anything else that needs to happen to make that switch? Regards, Anthony Liguori > > But I would really stop worrying about the qemu-kvm code base. > > Jan > > -- > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE > Corporate Competence Center Embedded Linux > -- > 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] qemu-kvm: remove "boot=on|off" drive parameter compatibility
On 2012-10-01 15:19, Anthony Liguori wrote: > Jan Kiszka writes: > >> On 2012-10-01 11:31, Marcelo Tosatti wrote: >> >> It's not just about default configs. We need to validate if the >> migration formats are truly compatible (qemu-kvm -> QEMU, the other way >> around definitely not). For the command line switches, we could provide >> a wrapper script that translates them into upstream format or simply >> ignores them. That should be harmless to carry upstream. > > qemu-kvm has: > > -no-kvm > -no-kvm-irqchip > -no-kvm-pit > -no-kvm-pit-reinjection > -tdf <- does nothing > > There are replacements for all of the above. If we need to add them to > qemu.git, it's not big deal to add them. But I don't think we should add them to the source code. This can perfectly be handled my a (disposable) script layer on top of qemu-system-x86_64 - the namespace (qemu-kvm in most cases) is also free. > > -drive ...,boot= <- this is ignored > > cpu_set command for CPU hotplug which is known broken in qemu-kvm. Right, so nothing is lost when migrating to QEMU. > > testdev which is nice but only used for development > > Default nic is rtl8139 vs. e1000. > > Some logic to move change the default VGA ram size to 16mb for pc-1.2 > (QEMU uses 16mb by default now too). Also nicely manageable in a wrapper. > > I think at this point, none of this matters but I added the various > distro maintainers to the thread. > > I think it's time for the distros to drop qemu-kvm and just ship > qemu.git. +1 Jan > Is there anything else that needs to happen to make that > switch? > > Regards, > > Anthony Liguori -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] qemu-kvm: remove "boot=on|off" drive parameter compatibility
On Mon, Oct 01, 2012 at 08:19:29AM -0500, Anthony Liguori wrote: > Jan Kiszka writes: > > > On 2012-10-01 11:31, Marcelo Tosatti wrote: > > > > It's not just about default configs. We need to validate if the > > migration formats are truly compatible (qemu-kvm -> QEMU, the other way > > around definitely not). For the command line switches, we could provide > > a wrapper script that translates them into upstream format or simply > > ignores them. That should be harmless to carry upstream. > > qemu-kvm has: > > -no-kvm > -no-kvm-irqchip > -no-kvm-pit > -no-kvm-pit-reinjection > -tdf <- does nothing > > There are replacements for all of the above. If we need to add them to > qemu.git, it's not big deal to add them. At the moment the only purpose of this command line options is for compability with scripts. My view is that scripts are easily fixed, so we can just drop them. No need to carry this to QEMU. > -drive ...,boot= <- this is ignored > > cpu_set command for CPU hotplug which is known broken in qemu-kvm. > > testdev which is nice but only used for development > > Default nic is rtl8139 vs. e1000. Config file (as suggested earlier on this thread). > Some logic to move change the default VGA ram size to 16mb for pc-1.2 > (QEMU uses 16mb by default now too). Config file. > I think at this point, none of this matters but I added the various > distro maintainers to the thread. > > I think it's time for the distros to drop qemu-kvm and just ship > qemu.git. Is there anything else that needs to happen to make that > switch? > > Regards, > > Anthony Liguori > > > > > But I would really stop worrying about the qemu-kvm code base. > > > > Jan > > > > -- > > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE > > Corporate Competence Center Embedded Linux > > -- > > 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] qemu-kvm: remove "boot=on|off" drive parameter compatibility
On 2012-10-01 15:31, Marcelo Tosatti wrote: > On Mon, Oct 01, 2012 at 08:19:29AM -0500, Anthony Liguori wrote: >> Jan Kiszka writes: >> >>> On 2012-10-01 11:31, Marcelo Tosatti wrote: >>> >>> It's not just about default configs. We need to validate if the >>> migration formats are truly compatible (qemu-kvm -> QEMU, the other way >>> around definitely not). For the command line switches, we could provide >>> a wrapper script that translates them into upstream format or simply >>> ignores them. That should be harmless to carry upstream. >> >> qemu-kvm has: >> >> -no-kvm >> -no-kvm-irqchip >> -no-kvm-pit >> -no-kvm-pit-reinjection >> -tdf <- does nothing >> >> There are replacements for all of the above. If we need to add them to >> qemu.git, it's not big deal to add them. > > At the moment the only purpose of this command line options is for > compability with scripts. My view is that scripts are easily fixed, > so we can just drop them. No need to carry this to QEMU. > >> -drive ...,boot= <- this is ignored >> >> cpu_set command for CPU hotplug which is known broken in qemu-kvm. >> >> testdev which is nice but only used for development >> >> Default nic is rtl8139 vs. e1000. > > Config file (as suggested earlier on this thread). If you need to append -config bla, you can also specify the desired NIC explicitly - I see no value in the former. If we decide to mangle a qemu-kvm command line before calling QEMU binaries, we can adjust this variation there. Otherwise it's the same as with all those -kvm*: Scripts/management tools will need adjustment. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PULL for Kevin 00/19] Block job improvements part 1
Il 28/09/2012 20:23, Kevin Wolf ha scritto: > Am 28.09.2012 17:22, schrieb Paolo Bonzini: >> Kevin, >> >> the following changes since commit ac05f3492421caeb05809ffa02c6198ede179e43: >> >> add a boot parameter to set reboot timeout (2012-09-25 20:05:04 -0500) >> >> are available in the git repository at: >> >> git://github.com/bonzini/qemu.git blkmirror-job-1.3-part1 >> >> for you to fetch changes up to ed306a929f16fda8a85561430b1ac370bde65097: >> >> qemu-iotests: add tests for streaming error handling (2012-09-27 15:11:22 >> +0200) >> >> This message has the diff from my post of Sep 25th. You reviewed 18 patches, >> these are 19 because of the new "qmp: add 'busy' member to BlockJobInfo" >> patch. >> >> >> Jeff Cody (1): >> blockdev: rename block_stream_cb to a generic block_job_cb >> >> Paolo Bonzini (18): >> qerror/block: introduce QERR_BLOCK_JOB_NOT_ACTIVE >> block: fix documentation of block_job_cancel_sync >> block: move job APIs to separate files >> block: add block_job_query >> qmp: add 'busy' member to BlockJobInfo >> block: add support for job pause/resume >> qmp: add block-job-pause and block-job-resume >> qemu-iotests: add test for pausing a streaming operation >> block: rename block_job_complete to block_job_completed >> iostatus: rename BlockErrorAction, BlockQMPEventAction >> iostatus: move BlockdevOnError declaration to QAPI >> iostatus: change is_read to a bool >> iostatus: reorganize io error code >> block: introduce block job error >> stream: add on-error argument >> blkdebug: process all set_state rules in the old state >> qemu-iotests: map underscore to dash in QMP argument names >> qemu-iotests: add tests for streaming error handling >> >> Makefile.objs | 5 +- >> QMP/qmp-events.txt| 22 >> block.c | 187 -- >> block.h | 20 ++-- >> block/Makefile.objs | 3 +- >> block/blkdebug.c | 12 +- >> block/stream.c| 33 +- >> block_int.h | 162 ++ >> blockdev.c| 86 +- >> blockjob.c| 249 >> blockjob.h| 243 +++ >> hmp-commands.hx | 35 +- >> hmp.c | 26 - >> hmp.h | 2 + >> hw/fdc.c | 4 +- >> hw/ide/core.c | 22 +--- >> hw/ide/pci.c | 4 +- >> hw/scsi-disk.c| 25 ++-- >> hw/scsi-generic.c | 4 +- >> hw/virtio-blk.c | 23 ++-- >> monitor.c | 1 + >> monitor.h | 1 + >> qapi-schema.json | 91 ++- >> qemu-tool.c | 6 + >> qerror.h | 6 + >> qmp-commands.hx | 14 ++- >> tests/qemu-iotests/030| 260 >> +- >> tests/qemu-iotests/030.out| 4 +- >> tests/qemu-iotests/group | 2 +- >> tests/qemu-iotests/iotests.py | 15 ++- >> trace-events | 4 +- >> 31 file modificati, 1152 inserzioni(+), 419 rimozioni(-) >> create mode 100644 blockjob.c >> create mode 100644 blockjob.h > > Thanks, applied all to the block branch. Great, I rebased and only got mostly trivial conflicts. I pushed the outcome to the branch blkmirror-job-20120925-rebase of my github repository. Do I need to repost, or are you going to continue your review on the previous patchset? The next obvious points to step are: - part 2: "qemu-iotests: add mirroring test case" or "qemu-iotests: add testcases for mirroring on-source-error/on-target-error" - part 3: "mirror: perform COW if the cluster size is bigger than the granularity" or "mirror: allow customizing the granularity" - part 4: "mirror: support arbitrarily-sized iterations" Paolo
Re: [Qemu-devel] qemu-kvm: remove "boot=on|off" drive parameter compatibility
Il 01/10/2012 15:19, Anthony Liguori ha scritto: > I think it's time for the distros to drop qemu-kvm and just ship > qemu.git. Is there anything else that needs to happen to make that > switch? Perhaps change the default to -machine accel=kvm:tcg? Paolo
Re: [Qemu-devel] [PATCH 2/2] cpu_physical_memory_write_rom() needs to do TB invalidates
On 09/12/2012 07:57 AM, David Gibson wrote: On Mon, Sep 10, 2012 at 03:27:45PM +0200, Andreas Färber wrote: Am 10.09.2012 04:30, schrieb David Gibson: cpu_physical_memory_write_rom(), despite the name, can also be used to write images into RAM - and will often be used that way if the machine uses load_image_targphys() into RAM addresses. However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_rw() does invalidate any cached TBs which might be affected by the region "doesn't"? Otherwise doesn't look wrong. Oops, comment updated. From 6b913afaf83f52ee787271827c84b492e8ac5895 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 22 Aug 2012 14:58:04 +1000 Subject: [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates cpu_physical_memory_write_rom(), despite the name, can also be used to write images into RAM - and will often be used that way if the machine uses load_image_targphys() into RAM addresses. However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_rw() doesn't invalidate any cached TBs which might be affected by the region written. This was breaking reset (under full emu) on the pseries machine - we loaded our firmware image into RAM, and while executing it rewrite the code at the entry point (correctly causing a TB invalidate/refresh). When we reset the firmware image was reloaded, but the TB from the rewrite was still active and caused us to get an illegal instruction trap. This patch fixes the bug by duplicating the tb invalidate code from cpu_physical_memory_rw() in cpu_physical_memory_write_rom(). Signed-off-by: David Gibson --- exec.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/exec.c b/exec.c index 5834766..eff40d7 100644 --- a/exec.c +++ b/exec.c @@ -3523,6 +3523,13 @@ void cpu_physical_memory_write_rom(target_phys_addr_t addr, /* ROM/RAM case */ ptr = qemu_get_ram_ptr(addr1); memcpy(ptr, buf, l); +if (!cpu_physical_memory_is_dirty(addr1)) { +/* invalidate code */ +tb_invalidate_phys_page_range(addr1, addr1 + l, 0); +/* set dirty bit */ +cpu_physical_memory_set_dirty_flags( +addr1, (0xff & ~CODE_DIRTY_FLAG)); +} qemu_put_ram_ptr(ptr); } len -= l; Hi, this patch breaks Windows XP guest at all. Windows XP boot ends in loob by restarting itself after time-out expires in windows advanced boot options. I started the guest using this command-line: ./x86_64-softmmu/qemu-system-x86_64 -m 2G -drive file=/data/data-shared/images/winxp-test.img -vnc 0.0.0.0:0 Pavel
Re: [Qemu-devel] qemu-kvm: remove "boot=on|off" drive parameter compatibility
01.10.2012 17:36, Jan Kiszka wrote: > On 2012-10-01 15:31, Marcelo Tosatti wrote: >>> Default nic is rtl8139 vs. e1000. >> >> Config file (as suggested earlier on this thread). > > If you need to append -config bla, you can also specify the desired NIC > explicitly - I see no value in the former. If we decide to mangle a > qemu-kvm command line before calling QEMU binaries, we can adjust this > variation there. Otherwise it's the same as with all those -kvm*: > Scripts/management tools will need adjustment. I don't think there's a need to "manage" this rtl8139 at all at this level. Let's declare that "qemu-kvm 1.3+" will switch from rtl8139 to e1000 by default as a more suitable in modern world, -- the same way as qemu did the switch earlier, and be done with that. I think. Note that this is JUST for "-net nic" users, which should be the minority (proper usage is -device rtl8139 or -device e1000, with explicit model). Thanks, /mjt
Re: [Qemu-devel] qemu-kvm: remove "boot=on|off" drive parameter compatibility
On 2012-10-01 15:38, Paolo Bonzini wrote: > Il 01/10/2012 15:19, Anthony Liguori ha scritto: >> I think it's time for the distros to drop qemu-kvm and just ship >> qemu.git. Is there anything else that needs to happen to make that >> switch? > > Perhaps change the default to -machine accel=kvm:tcg? That's the old discussion again: This way we generate that "silent failure" (unless you monitor the console output), where users will complain that "QEMU is so slow" because something is blocking KVM. Maybe the risk for the latter is lower these days as modules are auto-loaded now, don't know. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] qemu-kvm: remove "boot=on|off" drive parameter compatibility
On 2012-10-01 15:44, Michael Tokarev wrote: > 01.10.2012 17:36, Jan Kiszka wrote: >> On 2012-10-01 15:31, Marcelo Tosatti wrote: > Default nic is rtl8139 vs. e1000. >>> >>> Config file (as suggested earlier on this thread). >> >> If you need to append -config bla, you can also specify the desired NIC >> explicitly - I see no value in the former. If we decide to mangle a >> qemu-kvm command line before calling QEMU binaries, we can adjust this >> variation there. Otherwise it's the same as with all those -kvm*: >> Scripts/management tools will need adjustment. > > I don't think there's a need to "manage" this rtl8139 at > all at this level. Let's declare that "qemu-kvm 1.3+" will > switch from rtl8139 to e1000 by default as a more suitable > in modern world, -- the same way as qemu did the switch > earlier, and be done with that. I think. > > Note that this is JUST for "-net nic" users, which should > be the minority (proper usage is -device rtl8139 or -device > e1000, with explicit model). Well, the alternative to config files or wrapper scripts is just a detailed checklist "how to migrate from qemu-kvm to QEMU". Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] a user here - pci-assign
On Mon, 2012-10-01 at 12:10 +0100, lejeczek wrote: > hmm, still cannot get readon 5450 to work on win7-64, have > changed -cpu to host but no fix > no vfio in qemu-kvm-0.12.1.2-2.295.el6_3.2.x86_64 > yes, I do blacklist modules at grub level and later in > modprobe also You probably want to try newer upstream code. RHEL does not currently support assignment of VGA. > is primary/secondary VGA setup somehow helped by > qemu/components? in guest I can do anything since device is > disabled. > > pci-assign.multifunction=on/off - that would be the case > with VGAs like radeon and nvidia - audio part - is it > optional or always has to be ON for such devices? Optional > where one gets hold of information like: addr= ? > I understand these are needed only! if pci-assign.host is > not enough and qemu has no way of knowing/finding it, when > may this happen? This is not going to help you. It's a matter of finding a free slot, which qemu can do fine on it's own if you don't want to specify. Thanks, Alex > On 28/09/12 20:48, Alex Williamson wrote: > > On Fri, 2012-09-28 at 19:46 +0100, lejeczek wrote: > >> thanks Alex for your patience, appreciate it I do > >> > >> what would be the droids I need? > >> I'm experiencing guests' "puzzling" behavior and was > >> suggested that command line arguments were wrong/incomplete. > >> > >> same box/hardware and radeon 5450 and ... > >> - winXP-32bit -> OK - I assumed getting the guest on a > >> external computer monitor was an ultimate test > >> - win7-64bit -> OS reports device as disabled cause the > >> device reported an error > > I've had this same card working with both of these guests. I believe > > one trick on win7 was to use -cpu host. Also, don't try to disable the > > emulated vga device, just set it up like a dual-head system. Once you > > load the catalyst driver windows will switch to use the assigned device > > exclusively. I was using the new vfio assignment driver, but someone > > else recently report it working with the existing driver as well. The > > 5450 should be a secondary graphics card on the host system, trying to > > assign the primary graphics is going to cause more problems. Also > > blacklist the radeon driver on the host, we don't need any leftover > > state from the Linux driver causing problems since most of these > > graphics cards don't support a reset mechanism. > > > >> same box as above and geforce gt640 and .. > >> for both XP and 7 report device with exclamation marks (thus > >> did not even bother to connect any screens like in working > >> case with XP & radeon) > > I don't think we've seen any reports yet of Nvidia cards working, > > there's another thread on the kvm list speculating at some of the > > problems. > > > >> I'll try to get hold of ROMs of the cards, meanwhile, how > >> can I troubleshoot it? how to get more verbose feedback and > >> what to specifically look for? > > ROMs are only going to help if you're getting errors trying to read the > > ROM. Nvidia seems to have this problem, but I don't think radeons > > typical do. There's a #define in the code that can be enabled to get > > more logging, but it's not terribly useful unless you know what you're > > looking at. VGA has plenty of issues with legacy PC address ranges that > > are known problems, but there are also plenty of unknown problems that > > make it a pretty difficult black box debugging project. Thanks, > > > > Alex > > > > > > > >
Re: [Qemu-devel] qemu-kvm: remove "boot=on|off" drive parameter compatibility
Paolo Bonzini writes: > Il 01/10/2012 15:19, Anthony Liguori ha scritto: >> I think it's time for the distros to drop qemu-kvm and just ship >> qemu.git. Is there anything else that needs to happen to make that >> switch? > > Perhaps change the default to -machine accel=kvm:tcg? > > Paolo I would be in favor of: #if defined(CONFIG_KVM) accel=kvm #else accel=tcg #endif If KVM is available for your target, you almost certainly want to use it. I'd be very happy with that change for 1.3. Regards, Anthony Liguori
Re: [Qemu-devel] a user here - pci-assign
all these attempts and tests I've been doing in 2.6.39-200.32.1.el6uek.x86_64(oracle) now when I try rhel 2.6.32-279.5.1.el6.x86_64(everything else, hw&soft stay the same) I get: ,for win7-64 guest: pci-stub :26:00.0: irq 104 for MSI/MSI-X pci-stub :26:00.0: restoring config space at offset 0x1 (was 0x100400, writing 0x100407) pci-stub :26:00.0: irq 104 for MSI/MSI-X pci-stub :26:00.0: irq 104 for MSI/MSI-X pci-stub :26:00.0: irq 104 for MSI/MSI-X pci-stub :26:00.0: irq 104 for MSI/MSI-X do_IRQ: 12.146 No irq handler for vector (irq -1) and the guest(win7-64) hans/freezes on boot, no devices passed/assigned - guest boots up fine whereas XP-32-bit stays OK for both with device passed and without can this be helped? On 01/10/12 12:10, lejeczek wrote: hmm, still cannot get readon 5450 to work on win7-64, have changed -cpu to host but no fix no vfio in qemu-kvm-0.12.1.2-2.295.el6_3.2.x86_64 yes, I do blacklist modules at grub level and later in modprobe also is primary/secondary VGA setup somehow helped by qemu/components? in guest I can do anything since device is disabled. pci-assign.multifunction=on/off - that would be the case with VGAs like radeon and nvidia - audio part - is it optional or always has to be ON for such devices? where one gets hold of information like: addr= ? I understand these are needed only! if pci-assign.host is not enough and qemu has no way of knowing/finding it, when may this happen? many thanks On 28/09/12 20:48, Alex Williamson wrote: On Fri, 2012-09-28 at 19:46 +0100, lejeczek wrote: thanks Alex for your patience, appreciate it I do what would be the droids I need? I'm experiencing guests' "puzzling" behavior and was suggested that command line arguments were wrong/incomplete. same box/hardware and radeon 5450 and ... - winXP-32bit -> OK - I assumed getting the guest on a external computer monitor was an ultimate test - win7-64bit -> OS reports device as disabled cause the device reported an error I've had this same card working with both of these guests. I believe one trick on win7 was to use -cpu host. Also, don't try to disable the emulated vga device, just set it up like a dual-head system. Once you load the catalyst driver windows will switch to use the assigned device exclusively. I was using the new vfio assignment driver, but someone else recently report it working with the existing driver as well. The 5450 should be a secondary graphics card on the host system, trying to assign the primary graphics is going to cause more problems. Also blacklist the radeon driver on the host, we don't need any leftover state from the Linux driver causing problems since most of these graphics cards don't support a reset mechanism. same box as above and geforce gt640 and .. for both XP and 7 report device with exclamation marks (thus did not even bother to connect any screens like in working case with XP & radeon) I don't think we've seen any reports yet of Nvidia cards working, there's another thread on the kvm list speculating at some of the problems. I'll try to get hold of ROMs of the cards, meanwhile, how can I troubleshoot it? how to get more verbose feedback and what to specifically look for? ROMs are only going to help if you're getting errors trying to read the ROM. Nvidia seems to have this problem, but I don't think radeons typical do. There's a #define in the code that can be enabled to get more logging, but it's not terribly useful unless you know what you're looking at. VGA has plenty of issues with legacy PC address ranges that are known problems, but there are also plenty of unknown problems that make it a pretty difficult black box debugging project. Thanks, Alex
Re: [Qemu-devel] qemu-kvm: remove "boot=on|off" drive parameter compatibility
On 01.10.2012, at 15:19, Anthony Liguori wrote: > Jan Kiszka writes: > >> On 2012-10-01 11:31, Marcelo Tosatti wrote: >> >> It's not just about default configs. We need to validate if the >> migration formats are truly compatible (qemu-kvm -> QEMU, the other way >> around definitely not). For the command line switches, we could provide >> a wrapper script that translates them into upstream format or simply >> ignores them. That should be harmless to carry upstream. > > qemu-kvm has: > > -no-kvm Should be easy to have around as backwards compat hack. All it needs to do is set accel=tcg. > -no-kvm-irqchip > -no-kvm-pit > -no-kvm-pit-reinjection Those are quite important, as we need cmdline backwards compatibility. > -tdf <- does nothing > > There are replacements for all of the above. If we need to add them to > qemu.git, it's not big deal to add them. > > -drive ...,boot= <- this is ignored It's ignored, but useful for certain things. I don't know how many of our users use boot= today, but it's certainly still in the code, and supported. I honestly wouldn't mind to carry a SUSE specific patch that implements boot= for now until we can deem it deprecated enough that we can drop it. > cpu_set command for CPU hotplug which is known broken in qemu-kvm. > > testdev which is nice but only used for development > > Default nic is rtl8139 vs. e1000. Couldn't we have a machine option that tells us that -M pc-1.2 is really -M pc-kvm-1.2? That way we could implement the device difference, right? Alex > > Some logic to move change the default VGA ram size to 16mb for pc-1.2 > (QEMU uses 16mb by default now too). > > I think at this point, none of this matters but I added the various > distro maintainers to the thread. > > I think it's time for the distros to drop qemu-kvm and just ship > qemu.git. Is there anything else that needs to happen to make that > switch? > > Regards, > > Anthony Liguori > >> >> But I would really stop worrying about the qemu-kvm code base. >> >> Jan >> >> -- >> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE >> Corporate Competence Center Embedded Linux >> -- >> 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] qemu-kvm: remove "boot=on|off" drive parameter compatibility (fwd)
We (Ubuntu) plan to switch to qemu in the next release which opens in November. I suppose there's likely to be a hiccough or two, but I can't think of any offhand. -serge Quoting Scott Moser (smo...@ubuntu.com): > you should have been added here. > > > -- Forwarded message -- > Date: Mon, 1 Oct 2012 09:19:29 > From: Anthony Liguori > To: Jan Kiszka , Marcelo Tosatti > Cc: kvm , qemu-devel , > Cole Robinson , Scott Moser , > Andreas Färber , Michael Tokarev > Subject: Re: qemu-kvm: remove "boot=on|off" drive parameter compatibility > > Jan Kiszka writes: > > > On 2012-10-01 11:31, Marcelo Tosatti wrote: > > > > It's not just about default configs. We need to validate if the > > migration formats are truly compatible (qemu-kvm -> QEMU, the other way > > around definitely not). For the command line switches, we could provide > > a wrapper script that translates them into upstream format or simply > > ignores them. That should be harmless to carry upstream. > > qemu-kvm has: > > -no-kvm > -no-kvm-irqchip > -no-kvm-pit > -no-kvm-pit-reinjection > -tdf <- does nothing > > There are replacements for all of the above. If we need to add them to > qemu.git, it's not big deal to add them. > > -drive ...,boot= <- this is ignored > > cpu_set command for CPU hotplug which is known broken in qemu-kvm. > > testdev which is nice but only used for development > > Default nic is rtl8139 vs. e1000. > > Some logic to move change the default VGA ram size to 16mb for pc-1.2 > (QEMU uses 16mb by default now too). > > I think at this point, none of this matters but I added the various > distro maintainers to the thread. > > I think it's time for the distros to drop qemu-kvm and just ship > qemu.git. Is there anything else that needs to happen to make that > switch? > > Regards, > > Anthony Liguori > > > > > But I would really stop worrying about the qemu-kvm code base. > > > > Jan > > > > -- > > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE > > Corporate Competence Center Embedded Linux > > -- > > 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] qemu-kvm: remove "boot=on|off" drive parameter compatibility
Il 01/10/2012 16:07, Alexander Graf ha scritto: >>> -drive ...,boot= <- this is ignored > > It's ignored, but useful for certain things. I don't know how many of > our users use boot= today, but it's certainly still in the code, and > supported. I honestly wouldn't mind to carry a SUSE specific patch > that implements boot= for now until we can deem it deprecated enough > that we can drop it. Extboot is not going to be backported to QEMU, but SeaBIOS can now boot from all emulated device models except MegaSAS. So we may at most have just the option, for example making it an alias for bootindex=1 on the corresponding device. Paolo
Re: [Qemu-devel] [PATCH] hw: Add support for new LSI Logic devices.
Il 29/09/2012 16:35, Don Slutz ha scritto: >> > How important is the big endian support? > > lsi53c895a.c says: > > /* ??? Need to check if the {read,write}[wl] routines work properly on >big-endian targets. */ > > I could do the same, I.E. code it up and submit it un-tested on big > endian soon. Or since I currently do not have access to any real big > endian hardware; do all testing on QEMU on QEMU. Yes, that's a start. The best example of big endian hardware that QEMU supports is pSeries. Usually the pSeries people are quite helpful with endianness bugs. Paolo
Re: [Qemu-devel] qemu-kvm: remove "boot=on|off" drive parameter compatibility (fwd)
On 1 October 2012 15:15, Serge Hallyn wrote: > We (Ubuntu) plan to switch to qemu in the next release which opens in > November. I suppose there's likely to be a hiccough or two, but I can't > think of any offhand. Are you planning to do that for all CPU target architectures, or to maintain the current split between x86 and everything-else ? -- PMM
Re: [Qemu-devel] [PATCH 2/2] cpu_physical_memory_write_rom() needs to do TB invalidates
Pavel Hrdina writes: > On 09/12/2012 07:57 AM, David Gibson wrote: >> On Mon, Sep 10, 2012 at 03:27:45PM +0200, Andreas Färber wrote: >>> Am 10.09.2012 04:30, schrieb David Gibson: cpu_physical_memory_write_rom(), despite the name, can also be used to write images into RAM - and will often be used that way if the machine uses load_image_targphys() into RAM addresses. However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_rw() does invalidate any cached TBs which might be affected by the region >>> "doesn't"? >>> >>> Otherwise doesn't look wrong. >> Oops, comment updated. >> >> From 6b913afaf83f52ee787271827c84b492e8ac5895 Mon Sep 17 00:00:00 2001 >> From: David Gibson >> Date: Wed, 22 Aug 2012 14:58:04 +1000 >> Subject: [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates >> >> cpu_physical_memory_write_rom(), despite the name, can also be used to >> write images into RAM - and will often be used that way if the machine >> uses load_image_targphys() into RAM addresses. >> >> However, cpu_physical_memory_write_rom(), unlike >> cpu_physical_memory_rw() doesn't invalidate any cached TBs which might >> be affected by the region written. >> >> This was breaking reset (under full emu) on the pseries machine - we loaded >> our firmware image into RAM, and while executing it rewrite the code at >> the entry point (correctly causing a TB invalidate/refresh). When we >> reset the firmware image was reloaded, but the TB from the rewrite was >> still active and caused us to get an illegal instruction trap. >> >> This patch fixes the bug by duplicating the tb invalidate code from >> cpu_physical_memory_rw() in cpu_physical_memory_write_rom(). >> >> Signed-off-by: David Gibson >> --- >> exec.c |7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/exec.c b/exec.c >> index 5834766..eff40d7 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -3523,6 +3523,13 @@ void cpu_physical_memory_write_rom(target_phys_addr_t >> addr, >> /* ROM/RAM case */ >> ptr = qemu_get_ram_ptr(addr1); >> memcpy(ptr, buf, l); >> +if (!cpu_physical_memory_is_dirty(addr1)) { >> +/* invalidate code */ >> +tb_invalidate_phys_page_range(addr1, addr1 + l, 0); >> +/* set dirty bit */ >> +cpu_physical_memory_set_dirty_flags( >> +addr1, (0xff & ~CODE_DIRTY_FLAG)); >> +} >> qemu_put_ram_ptr(ptr); >> } >> len -= l; > Hi, > > this patch breaks Windows XP guest at all. Windows XP boot ends in loob > by restarting itself after time-out expires in windows advanced boot > options. > > I started the guest using this command-line: > > ./x86_64-softmmu/qemu-system-x86_64 -m 2G -drive > file=/data/data-shared/images/winxp-test.img -vnc 0.0.0.0:0 Does changing the tb_invalidate_phys_page_range() call to: tb_invalidate_phys_page_range(addr1, addr1 + MAX(l, TARGET_PAGE_SIZE), 0); The dirty flag is being reset for the full page but we're potentially only invalidating a subset of TBs that occur on the page. Regards, Anthony Liguori > > Pavel
[Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it
If we built a target for a host that supports KVM in principle, set the default accelerator to KVM as well. This also means the start of QEMU will fail to start if KVM support turns out to be unavailable at runtime. Signed-off-by: Jan Kiszka --- kvm-all.c |1 + kvm-stub.c |1 + kvm.h |1 + vl.c |4 ++-- 4 files changed, 5 insertions(+), 2 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 92a7137..4d5f86c 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -103,6 +103,7 @@ struct KVMState #endif }; +bool kvm_configured = true; KVMState *kvm_state; bool kvm_kernel_irqchip; bool kvm_async_interrupts_allowed; diff --git a/kvm-stub.c b/kvm-stub.c index 3c52eb5..86a6451 100644 --- a/kvm-stub.c +++ b/kvm-stub.c @@ -17,6 +17,7 @@ #include "gdbstub.h" #include "kvm.h" +bool kvm_configured; KVMState *kvm_state; bool kvm_kernel_irqchip; bool kvm_async_interrupts_allowed; diff --git a/kvm.h b/kvm.h index dea2998..9936e5f 100644 --- a/kvm.h +++ b/kvm.h @@ -22,6 +22,7 @@ #include #endif +extern bool kvm_configured; extern int kvm_allowed; extern bool kvm_kernel_irqchip; extern bool kvm_async_interrupts_allowed; diff --git a/vl.c b/vl.c index 8d305ca..f557bd1 100644 --- a/vl.c +++ b/vl.c @@ -2215,8 +2215,8 @@ static int configure_accelerator(void) } if (p == NULL) { -/* Use the default "accelerator", tcg */ -p = "tcg"; +/* The default accelerator depends on the availability of KVM. */ +p = kvm_configured ? "kvm" : "tcg"; } while (!accel_initialised && *p != '\0') { -- 1.7.3.4
[Qemu-devel] [PATCH v2 0/9] Embedded NBD server
This series rebases the previous qemu-sockets patches for error propagation and uses the new QAPI-friendly socket functions in the embedded NBD server. The changes are due to Orit's patches being now in, some early parts being in Luiz's queue, and glusterfs patches not having touched qemu-sockets.c in the end. Patches 1 to 4 start moving qemu-sockets functions away from error_report (or printf) and away from QemuOpts. Patch 5 makes it easier to reuse the address parser of inet_parse in the new socket_parse function. Patch 6 introduces QAPI-friendly socket parsing and creation functions. Patches 7 and 8 introduces the QMP commands, and patch 9 introduces the HMP version. Paolo Paolo Bonzini (9): build: add QAPI files to the tools qapi: add socket address types qemu-sockets: add error propagation to inet_parse qemu-sockets: add error propagation to Unix socket functions qemu-sockets: return IPSocketAddress from inet_parse qemu-sockets: add socket_listen, socket_connect, socket_parse block: add close notifiers qmp: add NBD server commands hmp: add NBD server commands Makefile.objs | 8 +- block.c | 19 +++- block.h | 1 + block_int.h | 2 + blockdev-nbd.c | 119 hmp-commands.hx | 29 ++ hmp.c | 55 +++ hmp.h | 2 + nbd.c | 4 +- qapi-schema.json| 96 +++ qemu-char.c | 4 +- qemu-sockets.c | 261 +++- qemu-tool.c | 6 ++ qemu_socket.h | 12 ++- qga/channel-posix.c | 2 +- qmp-commands.hx | 16 ui/vnc.c| 4 +- 17 file modificati, 557 inserzioni(+), 83 rimozioni(-) create mode 100644 blockdev-nbd.c -- 1.7.12
[Qemu-devel] [PATCH v2 1/9] build: add QAPI files to the tools
We need them because qemu-sockets will soon be using SocketAddress. Signed-off-by: Paolo Bonzini --- Makefile.objs | 3 ++- 1 file modificato, 2 inserzioni(+). 1 rimozione(-) diff --git a/Makefile.objs b/Makefile.objs index 4412757..03da150 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -47,6 +47,7 @@ block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y) block-obj-$(CONFIG_POSIX) += posix-aio-compat.o block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o block-obj-y += block/ +block-obj-y += $(qapi-obj-y) qapi-types.o qapi-visit.o ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy) # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add. @@ -241,9 +242,9 @@ QEMU_CFLAGS+=$(GLIB_CFLAGS) nested-vars += \ hw-obj-y \ qga-obj-y \ - block-obj-y \ qom-obj-y \ qapi-obj-y \ + block-obj-y \ user-obj-y \ common-obj-y \ extra-obj-y -- 1.7.12
[Qemu-devel] [PATCH v2 3/9] qemu-sockets: add error propagation to inet_parse
Signed-off-by: Paolo Bonzini --- qemu-sockets.c | 41 + 1 file modificato, 21 inserzioni(+), 20 rimozioni(-) diff --git a/qemu-sockets.c b/qemu-sockets.c index 1f14e8b..bf1f794 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -511,7 +511,7 @@ err: } /* compatibility wrapper */ -static int inet_parse(QemuOpts *opts, const char *str) +static void inet_parse(QemuOpts *opts, const char *str, Error **errp) { const char *optstr, *h; char addr[64]; @@ -523,32 +523,28 @@ static int inet_parse(QemuOpts *opts, const char *str) /* no host given */ addr[0] = '\0'; if (1 != sscanf(str,":%32[^,]%n",port,&pos)) { -fprintf(stderr, "%s: portonly parse error (%s)\n", -__FUNCTION__, str); -return -1; +error_setg(errp, "error parsing port in address '%s'", str); +return; } } else if (str[0] == '[') { /* IPv6 addr */ if (2 != sscanf(str,"[%64[^]]]:%32[^,]%n",addr,port,&pos)) { -fprintf(stderr, "%s: ipv6 parse error (%s)\n", -__FUNCTION__, str); -return -1; +error_setg(errp, "error parsing IPv6 address '%s'", str); +return; } qemu_opt_set(opts, "ipv6", "on"); } else if (qemu_isdigit(str[0])) { /* IPv4 addr */ if (2 != sscanf(str,"%64[0-9.]:%32[^,]%n",addr,port,&pos)) { -fprintf(stderr, "%s: ipv4 parse error (%s)\n", -__FUNCTION__, str); -return -1; +error_setg(errp, "error parsing IPv4 address '%s'", str); +return; } qemu_opt_set(opts, "ipv4", "on"); } else { /* hostname */ if (2 != sscanf(str,"%64[^:]:%32[^,]%n",addr,port,&pos)) { -fprintf(stderr, "%s: hostname parse error (%s)\n", -__FUNCTION__, str); -return -1; +error_setg(errp, "error parsing address '%s'", str); +return; } } qemu_opt_set(opts, "host", addr); @@ -563,7 +559,6 @@ static int inet_parse(QemuOpts *opts, const char *str) qemu_opt_set(opts, "ipv4", "on"); if (strstr(optstr, ",ipv6")) qemu_opt_set(opts, "ipv6", "on"); -return 0; } int inet_listen(const char *str, char *ostr, int olen, @@ -572,9 +567,11 @@ int inet_listen(const char *str, char *ostr, int olen, QemuOpts *opts; char *optstr; int sock = -1; +Error *local_err = NULL; opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); -if (inet_parse(opts, str) == 0) { +inet_parse(opts, str, &local_err); +if (local_err == NULL) { sock = inet_listen_opts(opts, port_offset, errp); if (sock != -1 && ostr) { optstr = strchr(str, ','); @@ -591,7 +588,7 @@ int inet_listen(const char *str, char *ostr, int olen, } } } else { -error_set(errp, QERR_SOCKET_CREATE_FAILED); +error_propagate(errp, local_err); } qemu_opts_del(opts); return sock; @@ -609,12 +606,14 @@ int inet_connect(const char *str, Error **errp) { QemuOpts *opts; int sock = -1; +Error *local_err = NULL; opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); -if (inet_parse(opts, str) == 0) { +inet_parse(opts, str, &local_err); +if (local_err == NULL) { sock = inet_connect_opts(opts, errp, NULL, NULL); } else { -error_set(errp, QERR_SOCKET_CREATE_FAILED); +error_propagate(errp, local_err); } qemu_opts_del(opts); return sock; @@ -639,14 +638,16 @@ int inet_nonblocking_connect(const char *str, { QemuOpts *opts; int sock = -1; +Error *local_err = NULL; g_assert(callback != NULL); opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); -if (inet_parse(opts, str) == 0) { +inet_parse(opts, str, &local_err); +if (local_err == NULL) { sock = inet_connect_opts(opts, errp, callback, opaque); } else { -error_set(errp, QERR_SOCKET_CREATE_FAILED); +error_propagate(errp, local_err); } qemu_opts_del(opts); return sock; -- 1.7.12
[Qemu-devel] [PATCH v2 4/9] qemu-sockets: add error propagation to Unix socket functions
This patch starts harmonizing unix_* and inet_* functions. Signed-off-by: Paolo Bonzini --- nbd.c | 4 ++-- qemu-char.c | 4 ++-- qemu-sockets.c | 40 qemu_socket.h | 8 qga/channel-posix.c | 2 +- ui/vnc.c| 4 ++-- 6 file modificati, 31 inserzioni(+), 31 rimozioni(-) diff --git a/nbd.c b/nbd.c index 6f0db62..f61a288 100644 --- a/nbd.c +++ b/nbd.c @@ -230,12 +230,12 @@ int unix_socket_incoming(const char *path) char *ostr = NULL; int olen = 0; -return unix_listen(path, ostr, olen); +return unix_listen(path, ostr, olen, NULL); } int unix_socket_outgoing(const char *path) { -return unix_connect(path); +return unix_connect(path, NULL); } /* Basic flow for negotiation diff --git a/qemu-char.c b/qemu-char.c index b082bae..fa294c0 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2448,9 +2448,9 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts) if (is_unix) { if (is_listen) { -fd = unix_listen_opts(opts); +fd = unix_listen_opts(opts, NULL); } else { -fd = unix_connect_opts(opts); +fd = unix_connect_opts(opts, NULL); } } else { if (is_listen) { diff --git a/qemu-sockets.c b/qemu-sockets.c index bf1f794..1e69274 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -655,7 +655,7 @@ int inet_nonblocking_connect(const char *str, #ifndef _WIN32 -int unix_listen_opts(QemuOpts *opts) +int unix_listen_opts(QemuOpts *opts, Error **errp) { struct sockaddr_un un; const char *path = qemu_opt_get(opts, "path"); @@ -663,7 +663,7 @@ int unix_listen_opts(QemuOpts *opts) sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0); if (sock < 0) { -perror("socket(unix)"); +error_set(errp, QERR_SOCKET_CREATE_FAILED); return -1; } @@ -688,11 +688,11 @@ int unix_listen_opts(QemuOpts *opts) unlink(un.sun_path); if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) { -fprintf(stderr, "bind(unix:%s): %s\n", un.sun_path, strerror(errno)); +error_set(errp, QERR_SOCKET_BIND_FAILED); goto err; } if (listen(sock, 1) < 0) { -fprintf(stderr, "listen(unix:%s): %s\n", un.sun_path, strerror(errno)); +error_set(errp, QERR_SOCKET_LISTEN_FAILED); goto err; } @@ -703,20 +703,20 @@ err: return -1; } -int unix_connect_opts(QemuOpts *opts) +int unix_connect_opts(QemuOpts *opts, Error **errp) { struct sockaddr_un un; const char *path = qemu_opt_get(opts, "path"); int sock; if (NULL == path) { -fprintf(stderr, "unix connect: no path specified\n"); +error_setg(errp, "unix connect: no path specified\n"); return -1; } sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0); if (sock < 0) { -perror("socket(unix)"); +error_set(errp, QERR_SOCKET_CREATE_FAILED); return -1; } @@ -724,7 +724,7 @@ int unix_connect_opts(QemuOpts *opts) un.sun_family = AF_UNIX; snprintf(un.sun_path, sizeof(un.sun_path), "%s", path); if (connect(sock, (struct sockaddr*) &un, sizeof(un)) < 0) { -fprintf(stderr, "connect(unix:%s): %s\n", path, strerror(errno)); +error_set(errp, QERR_SOCKET_CONNECT_FAILED); close(sock); return -1; } @@ -733,7 +733,7 @@ int unix_connect_opts(QemuOpts *opts) } /* compatibility wrapper */ -int unix_listen(const char *str, char *ostr, int olen) +int unix_listen(const char *str, char *ostr, int olen, Error **errp) { QemuOpts *opts; char *path, *optstr; @@ -754,7 +754,7 @@ int unix_listen(const char *str, char *ostr, int olen) qemu_opt_set(opts, "path", str); } -sock = unix_listen_opts(opts); +sock = unix_listen_opts(opts, errp); if (sock != -1 && ostr) snprintf(ostr, olen, "%s%s", qemu_opt_get(opts, "path"), optstr ? optstr : ""); @@ -762,44 +762,44 @@ int unix_listen(const char *str, char *ostr, int olen) return sock; } -int unix_connect(const char *path) +int unix_connect(const char *path, Error **errp) { QemuOpts *opts; int sock; opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); qemu_opt_set(opts, "path", path); -sock = unix_connect_opts(opts); +sock = unix_connect_opts(opts, errp); qemu_opts_del(opts); return sock; } #else -int unix_listen_opts(QemuOpts *opts) +int unix_listen_opts(QemuOpts *opts, Error **errp) { -fprintf(stderr, "unix sockets are not available on windows\n"); +error_setg(errp, "unix sockets are not available on windows\n"); errno = ENOTSUP; return -1; } -int unix_connect_opts(QemuOpts *opts) +int unix_connect_opts(QemuOpts *opts, Error **errp) { -fprintf(stderr, "unix sockets are not available on windows\n"); +error_setg(errp, "unix sockets are not available on windows\n"); errno =
[Qemu-devel] [PATCH v2 7/9] block: add close notifiers
The first user of close notifiers will be the embedded NBD server. It is possible to use them to do some of the ad hoc processing (e.g. for block jobs and I/O limits) that is currently done by bdrv_close. Acked-by: Kevin Wolf Signed-off-by: Paolo Bonzini --- Makefile.objs | 3 ++- block.c | 19 ++- block.h | 1 + block_int.h | 2 ++ 4 file modificati, 19 inserzioni(+), 6 rimozioni(-) diff --git a/Makefile.objs b/Makefile.objs index 03da150..124f783 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -43,6 +43,7 @@ coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o block-obj-y = cutils.o iov.o cache-utils.o qemu-option.o module.o async.o block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o qemu-progress.o qemu-sockets.o +block-obj-y += notify.o block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y) block-obj-$(CONFIG_POSIX) += posix-aio-compat.o block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o @@ -92,7 +93,7 @@ common-obj-y += bt-host.o bt-vhci.o common-obj-y += iov.o acl.o common-obj-$(CONFIG_POSIX) += compatfd.o -common-obj-y += notify.o event_notifier.o +common-obj-y += event_notifier.o common-obj-y += qemu-timer.o qemu-timer-common.o common-obj-$(CONFIG_SLIRP) += slirp/ diff --git a/block.c b/block.c index 751ebdc..795d359 100644 --- a/block.c +++ b/block.c @@ -28,6 +28,7 @@ #include "block_int.h" #include "module.h" #include "qjson.h" +#include "notify.h" #include "qemu-coroutine.h" #include "qmp-commands.h" #include "qemu-timer.h" @@ -310,9 +311,16 @@ BlockDriverState *bdrv_new(const char *device_name) QTAILQ_INSERT_TAIL(&bdrv_states, bs, list); } bdrv_iostatus_disable(bs); +notifier_list_init(&bs->close_notifiers); + return bs; } +void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify) +{ +notifier_list_add(&bs->close_notifiers, notify); +} + BlockDriver *bdrv_find_format(const char *format_name) { BlockDriver *drv1; @@ -1096,12 +1104,13 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state) void bdrv_close(BlockDriverState *bs) { bdrv_flush(bs); -if (bs->drv) { -if (bs->job) { -block_job_cancel_sync(bs->job); -} -bdrv_drain_all(); +if (bs->job) { +block_job_cancel_sync(bs->job); +} +bdrv_drain_all(); +notifier_list_notify(&bs->close_notifiers, bs); +if (bs->drv) { if (bs == bs_snapshots) { bs_snapshots = NULL; } diff --git a/block.h b/block.h index b1095d8..17d8b00 100644 --- a/block.h +++ b/block.h @@ -149,6 +149,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, void bdrv_reopen_commit(BDRVReopenState *reopen_state); void bdrv_reopen_abort(BDRVReopenState *reopen_state); void bdrv_close(BlockDriverState *bs); +void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify); int bdrv_attach_dev(BlockDriverState *bs, void *dev); void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev); void bdrv_detach_dev(BlockDriverState *bs, void *dev); diff --git a/block_int.h b/block_int.h index ac4245c..41754ce 100644 --- a/block_int.h +++ b/block_int.h @@ -299,6 +299,8 @@ struct BlockDriverState { BlockDriverState *backing_hd; BlockDriverState *file; +NotifierList close_notifiers; + /* number of in-flight copy-on-read requests */ unsigned int copy_on_read_in_flight; -- 1.7.12
[Qemu-devel] [PATCH v2 5/9] qemu-sockets: return IPSocketAddress from inet_parse
Prepare inet_parse so that it can be easily reused in socket_parse. socket_parse will be public and will help converting addresses accepted by HMP to SocketAddress structures accepted by the implementation of QMP commands. Signed-off-by: Paolo Bonzini --- qemu-sockets.c | 119 + 1 file modificato, 78 inserzioni(+), 41 rimozioni(-) diff --git a/qemu-sockets.c b/qemu-sockets.c index 1e69274..f67a113 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -511,54 +511,91 @@ err: } /* compatibility wrapper */ -static void inet_parse(QemuOpts *opts, const char *str, Error **errp) +static IPSocketAddress *inet_parse(const char *str, Error **errp) { +IPSocketAddress *addr; const char *optstr, *h; -char addr[64]; +char host[64]; char port[33]; +int to; int pos; +addr = g_new(IPSocketAddress, 1); + /* parse address */ if (str[0] == ':') { /* no host given */ -addr[0] = '\0'; +host[0] = '\0'; if (1 != sscanf(str,":%32[^,]%n",port,&pos)) { error_setg(errp, "error parsing port in address '%s'", str); -return; +goto fail; } } else if (str[0] == '[') { /* IPv6 addr */ -if (2 != sscanf(str,"[%64[^]]]:%32[^,]%n",addr,port,&pos)) { +if (2 != sscanf(str,"[%64[^]]]:%32[^,]%n",host,port,&pos)) { error_setg(errp, "error parsing IPv6 address '%s'", str); -return; +goto fail; } -qemu_opt_set(opts, "ipv6", "on"); +addr->ipv6 = addr->has_ipv6 = true; } else if (qemu_isdigit(str[0])) { /* IPv4 addr */ -if (2 != sscanf(str,"%64[0-9.]:%32[^,]%n",addr,port,&pos)) { +if (2 != sscanf(str,"%64[0-9.]:%32[^,]%n",host,port,&pos)) { error_setg(errp, "error parsing IPv4 address '%s'", str); -return; +goto fail; } -qemu_opt_set(opts, "ipv4", "on"); +addr->ipv4 = addr->has_ipv4 = true; } else { /* hostname */ -if (2 != sscanf(str,"%64[^:]:%32[^,]%n",addr,port,&pos)) { +if (2 != sscanf(str,"%64[^:]:%32[^,]%n",host,port,&pos)) { error_setg(errp, "error parsing address '%s'", str); -return; +goto fail; } } -qemu_opt_set(opts, "host", addr); -qemu_opt_set(opts, "port", port); + +addr->host = g_strdup(host); +addr->port = g_strdup(port); /* parse options */ optstr = str + pos; h = strstr(optstr, ",to="); -if (h) -qemu_opt_set(opts, "to", h+4); -if (strstr(optstr, ",ipv4")) -qemu_opt_set(opts, "ipv4", "on"); -if (strstr(optstr, ",ipv6")) -qemu_opt_set(opts, "ipv6", "on"); +if (h) { +if (1 != sscanf(str, "%d%n", &to, &pos) || +(str[pos] != '\0' && str[pos] != ',')) { +error_setg(errp, "error parsing to= argument"); +goto fail; +} +addr->has_to = true; +addr->to = to; +} +if (strstr(optstr, ",ipv4")) { +addr->ipv4 = addr->has_ipv4 = true; +} +if (strstr(optstr, ",ipv6")) { +addr->ipv6 = addr->has_ipv6 = true; +} +return addr; + +fail: +qapi_free_IPSocketAddress(addr); +return NULL; +} + +static void inet_addr_to_opts(QemuOpts *opts, IPSocketAddress *addr) +{ +bool ipv4 = addr->ipv4 || !addr->has_ipv4; +bool ipv6 = addr->ipv6 || !addr->has_ipv6; + +if (!ipv4 || !ipv6) { +qemu_opt_set_bool(opts, "ipv4", ipv4); +qemu_opt_set_bool(opts, "ipv6", ipv6); +} +if (addr->has_to) { +char to[20]; +snprintf(to, sizeof(to), "%d", addr->to); +qemu_opt_set(opts, "to", to); +} +qemu_opt_set(opts, "host", addr->host); +qemu_opt_set(opts, "port", addr->port); } int inet_listen(const char *str, char *ostr, int olen, @@ -567,11 +604,13 @@ int inet_listen(const char *str, char *ostr, int olen, QemuOpts *opts; char *optstr; int sock = -1; -Error *local_err = NULL; +IPSocketAddress *addr; -opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); -inet_parse(opts, str, &local_err); -if (local_err == NULL) { +addr = inet_parse(str, errp); +if (addr != NULL) { +opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); +inet_addr_to_opts(opts, addr); +qapi_free_IPSocketAddress(addr); sock = inet_listen_opts(opts, port_offset, errp); if (sock != -1 && ostr) { optstr = strchr(str, ','); @@ -587,10 +626,8 @@ int inet_listen(const char *str, char *ostr, int olen, optstr ? optstr : ""); } } -} else { -error_propagate(errp, local_err); +qemu_opts_del(opts); } -qemu_opts_del(opts); return sock; } @@ -606,16 +643,16 @@ int inet_connect(const char *str, Error **errp) { QemuOpts *opts; int sock = -1; -
[Qemu-devel] [PATCH v2 6/9] qemu-sockets: add socket_listen, socket_connect, socket_parse
These are QAPI-friendly versions of the qemu-sockets functions. They support IP sockets, Unix sockets, and named file descriptors, using a QAPI union to dispatch to the correct function. Signed-off-by: Paolo Bonzini --- qemu-sockets.c | 95 ++ qemu-tool.c| 6 qemu_socket.h | 4 +++ 3 file modificati, 105 inserzioni(+) diff --git a/qemu-sockets.c b/qemu-sockets.c index f67a113..ed2d2be 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -22,6 +22,7 @@ #include #include +#include "monitor.h" #include "qemu_socket.h" #include "qemu-common.h" /* for qemu_isdigit */ #include "main-loop.h" @@ -843,6 +844,100 @@ int unix_connect(const char *path, Error **errp) #endif +SocketAddress *socket_parse(const char *str, Error **errp) +{ +SocketAddress *addr = NULL; + +addr = g_new(SocketAddress, 1); +if (strstart(str, "unix:", NULL)) { +if (str[5] == '\0') { +error_setg(errp, "invalid Unix socket address\n"); +goto fail; +} else { +addr->kind = SOCKET_ADDRESS_KIND_UNIX; +addr->q_unix = g_new(UnixSocketAddress, 1); +addr->q_unix->path = g_strdup(str + 5); +} +} else if (strstart(str, "fd:", NULL)) { +if (str[3] == '\0') { +error_setg(errp, "invalid file descriptor address\n"); +goto fail; +} else { +addr->kind = SOCKET_ADDRESS_KIND_FD; +addr->fd = g_new(String, 1); +addr->fd->str = g_strdup(str + 3); +} +} else { +addr->kind = SOCKET_ADDRESS_KIND_INET; +addr->inet = g_new(IPSocketAddress, 1); +addr->inet = inet_parse(str, errp); +if (addr->inet == NULL) { +goto fail; +} +} +return addr; + +fail: +qapi_free_SocketAddress(addr); +return NULL; +} + +int socket_connect(SocketAddress *addr, Error **errp) +{ +QemuOpts *opts; +int fd; + +opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); +switch (addr->kind) { +case SOCKET_ADDRESS_KIND_INET: +inet_addr_to_opts(opts, addr->inet); +fd = inet_connect_opts(opts, true, NULL, errp, NULL); +break; + +case SOCKET_ADDRESS_KIND_UNIX: +qemu_opt_set(opts, "path", addr->q_unix->path); +fd = unix_connect_opts(opts, errp); +break; + +case SOCKET_ADDRESS_KIND_FD: +fd = monitor_get_fd(cur_mon, addr->fd->str, errp); +break; + +default: +abort(); +} +qemu_opts_del(opts); +return fd; +} + +int socket_listen(SocketAddress *addr, Error **errp) +{ +QemuOpts *opts; +int fd; + +opts = qemu_opts_create(&dummy_opts, NULL, 0, NULL); +switch (addr->kind) { +case SOCKET_ADDRESS_KIND_INET: +inet_addr_to_opts(opts, addr->inet); +fd = inet_listen_opts(opts, 0, errp); +break; + +case SOCKET_ADDRESS_KIND_UNIX: +qemu_opt_set(opts, "path", addr->q_unix->path); +fd = unix_listen_opts(opts, errp); +break; + +case SOCKET_ADDRESS_KIND_FD: +fd = monitor_get_fd(cur_mon, addr->fd->str, errp); +break; + +default: +abort(); +} +qemu_opts_del(opts); +return fd; +} + #ifdef _WIN32 static void socket_cleanup(void) { diff --git a/qemu-tool.c b/qemu-tool.c index 18205ba..7658fde 100644 --- a/qemu-tool.c +++ b/qemu-tool.c @@ -42,6 +42,12 @@ int monitor_cur_is_qmp(void) return 0; } +int monitor_get_fd(Monitor *mon, const char *name, Error **errp) +{ +error_setg(errp, "only QEMU supports file descriptor passing"); +return -1; +} + void monitor_set_error(Monitor *mon, QError *qerror) { } diff --git a/qemu_socket.h b/qemu_socket.h index afe8689..cd083d0 100644 --- a/qemu_socket.h +++ b/qemu_socket.h @@ -61,6 +61,10 @@ int unix_listen(const char *path, char *ostr, int olen, Error **errp); int unix_connect_opts(QemuOpts *opts, Error **errp); int unix_connect(const char *path, Error **errp); +SocketAddress *socket_parse(const char *str, Error **errp); +int socket_connect(SocketAddress *addr, Error **errp); +int socket_listen(SocketAddress *addr, Error **errp); + /* Old, ipv4 only bits. Don't use for new code. */ int parse_host_port(struct sockaddr_in *saddr, const char *str); int socket_init(void); -- 1.7.12
[Qemu-devel] [PATCH v2 2/9] qapi: add socket address types
Signed-off-by: Paolo Bonzini --- qapi-schema.json | 53 + 1 file modificato, 53 inserzioni(+) diff --git a/qapi-schema.json b/qapi-schema.json index 191d921..b443a99 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2367,6 +2367,59 @@ 'opts': 'NetClientOptions' } } ## +# @IPSocketAddress +# +# Captures a range of possible destination addresses for an IP socket +# +# @host: host part of the address +# +# @port: port part of the address, or lowest port if @to is present +# +# @to: highest port to try +# +# @ipv4: whether to accept IPv4 addresses, default try both IPv4 and IPv6 +##optional +# +# @ipv6: whether to accept IPv6 addresses, default try both IPv4 and IPv6 +##optional +# +# Since 1.3 +## +{ 'type': 'IPSocketAddress', + 'data': { +'host': 'str', +'port': 'str', +'*to': 'uint16', +'*ipv4': 'bool', +'*ipv6': 'bool' } } + +## +# @UnixSocketAddress +# +# Captures the destination address of a Unix socket +# +# @path: filesystem path to use +# +# Since 1.3 +## +{ 'type': 'UnixSocketAddress', + 'data': { +'path': 'str' } } + +## +# @SocketAddress +# +# Captures the address of a socket, which could also be a named file descriptor +# +# Since 1.3 +## +{ 'union': 'SocketAddress', + 'data': { +'inet': 'IPSocketAddress', +'unix': 'UnixSocketAddress', +'fd': 'String' } } + +## # @getfd: # # Receive a file descriptor via SCM rights and assign it a name -- 1.7.12
[Qemu-devel] [PATCH v2 9/9] hmp: add NBD server commands
Signed-off-by: Paolo Bonzini --- hmp-commands.hx | 29 + hmp.c | 55 +++ hmp.h | 2 ++ 3 file modificati, 86 inserzioni(+) diff --git a/hmp-commands.hx b/hmp-commands.hx index ed67e99..b0af484 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1247,6 +1247,35 @@ Remove all matches from the access control list, and set the default policy back to @code{deny}. ETEXI +{ +.name = "nbd_server_start", +.args_type = "writable:-w,uri:s", +.params = "nbd_server_start [-w] host:port", +.help = "serve block devices on the given host and port", +.mhandler.cmd = hmp_nbd_server_start, +}, +STEXI +@item nbd_server_start @var{host}:@var{port} +@findex nbd_server_start +Start an NBD server on the given host and/or port, and serve all of the +virtual machine's block devices that have an inserted media on it. +The @option{-w} option makes the devices writable. +ETEXI + +{ +.name = "nbd_server_stop", +.args_type = "", +.params = "nbd_server_stop", +.help = "stop serving block devices using the NBD protocol", +.mhandler.cmd = hmp_nbd_server_stop, +}, +STEXI +@item nbd_server_stop +@findex nbd_server_stop +Stop the QEMU embedded NBD server. +ETEXI + + #if defined(TARGET_I386) { diff --git a/hmp.c b/hmp.c index ba6fbd3..3d27628 100644 --- a/hmp.c +++ b/hmp.c @@ -18,6 +18,7 @@ #include "qemu-option.h" #include "qemu-timer.h" #include "qmp-commands.h" +#include "qemu_socket.h" #include "monitor.h" #include "console.h" @@ -1168,3 +1169,57 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict) qmp_screendump(filename, &err); hmp_handle_error(mon, &err); } + +void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) +{ +const char *uri = qdict_get_str(qdict, "uri"); +int writable = qdict_get_try_bool(qdict, "writable", 0); +Error *local_err = NULL; +BlockInfoList *block_list, *info; +SocketAddress *addr; + +/* First check if the address is valid and start the server. */ +addr = socket_parse(uri, &local_err); +if (local_err != NULL) { +goto exit; +} + +qmp_nbd_server_start(addr, &local_err); +qapi_free_SocketAddress(addr); +if (local_err != NULL) { +goto exit; +} + +/* Then try adding all block devices. If one fails, close all and + * exit. + */ +block_list = qmp_query_block(NULL); + +for (info = block_list; info; info = info->next) { +if (!info->value->has_inserted) { +continue; +} + +qmp_nbd_server_add(info->value->device, + true, !info->value->inserted->ro && writable, + &local_err); + +if (local_err != NULL) { +qmp_nbd_server_stop(NULL); +break; +} +} + +qapi_free_BlockInfoList(block_list); + +exit: +hmp_handle_error(mon, &local_err); +} + +void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict) +{ +Error *errp = NULL; + +qmp_nbd_server_stop(&errp); +hmp_handle_error(mon, &errp); +} diff --git a/hmp.h b/hmp.h index 48b9c59..0d02479 100644 --- a/hmp.h +++ b/hmp.h @@ -73,5 +73,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict); void hmp_closefd(Monitor *mon, const QDict *qdict); void hmp_send_key(Monitor *mon, const QDict *qdict); void hmp_screen_dump(Monitor *mon, const QDict *qdict); +void hmp_nbd_server_start(Monitor *mon, const QDict *qdict); +void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict); #endif -- 1.7.12
[Qemu-devel] [PATCH v2 8/9] qmp: add NBD server commands
Adding an NBD server inside QEMU is trivial, since all the logic is in nbd.c and can be shared easily between qemu-nbd and QEMU itself. The main difference is that qemu-nbd serves a single unnamed export, while QEMU serves named exports. Signed-off-by: Paolo Bonzini --- Makefile.objs| 2 +- blockdev-nbd.c | 119 +++ qapi-schema.json | 43 qmp-commands.hx | 16 4 file modificati, 179 inserzioni(+). 1 rimozione(-) create mode 100644 blockdev-nbd.c diff --git a/Makefile.objs b/Makefile.objs index 124f783..40384ff 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -61,7 +61,7 @@ endif # suppress *all* target specific code in case of system emulation, i.e. a # single QEMU executable should support all CPUs and machines. -common-obj-y = $(block-obj-y) blockdev.o +common-obj-y = $(block-obj-y) blockdev.o blockdev-nbd.o common-obj-y += net.o net/ common-obj-y += qom/ common-obj-y += readline.o console.o cursor.o diff --git a/blockdev-nbd.c b/blockdev-nbd.c new file mode 100644 index 000..8031813 --- /dev/null +++ b/blockdev-nbd.c @@ -0,0 +1,119 @@ +/* + * Serving QEMU block devices via NBD + * + * Copyright (c) 2012 Red Hat, Inc. + * + * Author: Paolo Bonzini + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * later. See the COPYING file in the top-level directory. + */ + +#include "blockdev.h" +#include "hw/block-common.h" +#include "monitor.h" +#include "qerror.h" +#include "sysemu.h" +#include "qmp-commands.h" +#include "trace.h" +#include "nbd.h" +#include "qemu_socket.h" + +static int server_fd = -1; + +static void nbd_accept(void *opaque) +{ +struct sockaddr_in addr; +socklen_t addr_len = sizeof(addr); + +int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len); +if (fd >= 0) { +nbd_client_new(NULL, fd, nbd_client_put); +} +} + +void qmp_nbd_server_start(SocketAddress *addr, Error **errp) +{ +if (server_fd != -1) { +error_setg(errp, "NBD server already running"); +return; +} + +server_fd = socket_listen(addr, errp); +if (server_fd != -1) { +qemu_set_fd_handler2(server_fd, NULL, nbd_accept, NULL, NULL); +} +} + +/* Hook into the BlockDriverState notifiers to close the export when + * the file is closed. + */ +typedef struct NBDCloseNotifier { +Notifier n; +NBDExport *exp; +QTAILQ_ENTRY(NBDCloseNotifier) next; +} NBDCloseNotifier; + +static QTAILQ_HEAD(, NBDCloseNotifier) close_notifiers = +QTAILQ_HEAD_INITIALIZER(close_notifiers); + +static void nbd_close_notifier(Notifier *n, void *data) +{ +NBDCloseNotifier *cn = DO_UPCAST(NBDCloseNotifier, n, n); + +notifier_remove(&cn->n); +QTAILQ_REMOVE(&close_notifiers, cn, next); + +nbd_export_close(cn->exp); +nbd_export_put(cn->exp); +g_free(cn); +} + +static void nbd_server_put_ref(NBDExport *exp) +{ +BlockDriverState *bs = nbd_export_get_blockdev(exp); +drive_put_ref(drive_get_by_blockdev(bs)); +} + +void qmp_nbd_server_add(const char *device, bool has_writable, bool writable, +Error **errp) +{ +BlockDriverState *bs; +NBDExport *exp; +NBDCloseNotifier *n; + +if (nbd_export_find(device)) { +error_setg(errp, "NBD server already exporting device '%s'", device); +return; +} + +bs = bdrv_find(device); +if (!bs) { +error_set(errp, QERR_DEVICE_NOT_FOUND, device); +return; +} + +exp = nbd_export_new(bs, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY, + nbd_server_put_ref); + +nbd_export_set_name(exp, device); +drive_get_ref(drive_get_by_blockdev(bs)); + +n = g_malloc0(sizeof(NBDCloseNotifier)); +n->n.notify = nbd_close_notifier; +n->exp = exp; +bdrv_add_close_notifier(bs, &n->n); +QTAILQ_INSERT_TAIL(&close_notifiers, n, next); +} + +void qmp_nbd_server_stop(Error **errp) +{ +while (!QTAILQ_EMPTY(&close_notifiers)) { +NBDCloseNotifier *cn = QTAILQ_FIRST(&close_notifiers); +nbd_close_notifier(&cn->n, nbd_export_get_blockdev(cn->exp)); +} + +qemu_set_fd_handler2(server_fd, NULL, NULL, NULL, NULL); +close(server_fd); +server_fd = -1; +} diff --git a/qapi-schema.json b/qapi-schema.json index b443a99..229421d 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2697,3 +2697,46 @@ # Since: 0.14.0 ## { 'command': 'screendump', 'data': {'filename': 'str'} } + +## +# @nbd-server-start: +# +# Start an NBD server listening on the given host and port. Block +# devices can then be exported using @nbd-server-add. The NBD +# server will present them as named exports; for example, another +# QEMU instance could refer to them as "nbd:HOST:PORT:exportname=NAME". +# +# @addr: Address on which to listen. +# +# Returns: error if the server is already running. +# +# Since: 1.3.0 +## +{ 'command': 'nbd-server-start', + 'data': { 'addr': 'SocketAddress' } } +
Re: [Qemu-devel] [PATCH 2/2] cpu_physical_memory_write_rom() needs to do TB invalidates
On 10/01/2012 04:28 PM, Anthony Liguori wrote: Pavel Hrdina writes: On 09/12/2012 07:57 AM, David Gibson wrote: On Mon, Sep 10, 2012 at 03:27:45PM +0200, Andreas Färber wrote: Am 10.09.2012 04:30, schrieb David Gibson: cpu_physical_memory_write_rom(), despite the name, can also be used to write images into RAM - and will often be used that way if the machine uses load_image_targphys() into RAM addresses. However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_rw() does invalidate any cached TBs which might be affected by the region "doesn't"? Otherwise doesn't look wrong. Oops, comment updated. From 6b913afaf83f52ee787271827c84b492e8ac5895 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 22 Aug 2012 14:58:04 +1000 Subject: [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates cpu_physical_memory_write_rom(), despite the name, can also be used to write images into RAM - and will often be used that way if the machine uses load_image_targphys() into RAM addresses. However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_rw() doesn't invalidate any cached TBs which might be affected by the region written. This was breaking reset (under full emu) on the pseries machine - we loaded our firmware image into RAM, and while executing it rewrite the code at the entry point (correctly causing a TB invalidate/refresh). When we reset the firmware image was reloaded, but the TB from the rewrite was still active and caused us to get an illegal instruction trap. This patch fixes the bug by duplicating the tb invalidate code from cpu_physical_memory_rw() in cpu_physical_memory_write_rom(). Signed-off-by: David Gibson --- exec.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/exec.c b/exec.c index 5834766..eff40d7 100644 --- a/exec.c +++ b/exec.c @@ -3523,6 +3523,13 @@ void cpu_physical_memory_write_rom(target_phys_addr_t addr, /* ROM/RAM case */ ptr = qemu_get_ram_ptr(addr1); memcpy(ptr, buf, l); +if (!cpu_physical_memory_is_dirty(addr1)) { +/* invalidate code */ +tb_invalidate_phys_page_range(addr1, addr1 + l, 0); +/* set dirty bit */ +cpu_physical_memory_set_dirty_flags( +addr1, (0xff & ~CODE_DIRTY_FLAG)); +} qemu_put_ram_ptr(ptr); } len -= l; Hi, this patch breaks Windows XP guest at all. Windows XP boot ends in loob by restarting itself after time-out expires in windows advanced boot options. I started the guest using this command-line: ./x86_64-softmmu/qemu-system-x86_64 -m 2G -drive file=/data/data-shared/images/winxp-test.img -vnc 0.0.0.0:0 Does changing the tb_invalidate_phys_page_range() call to: tb_invalidate_phys_page_range(addr1, addr1 + MAX(l, TARGET_PAGE_SIZE), 0); The dirty flag is being reset for the full page but we're potentially only invalidating a subset of TBs that occur on the page. Regards, Anthony Liguori No, it doesn't fix this bug. Pavel Pavel
Re: [Qemu-devel] qemu-kvm: remove "boot=on|off" drive parameter compatibility
>>> On 10/1/2012 at 07:19 AM, Anthony Liguori wrote: > Jan Kiszka writes: > >> On 2012-10-01 11:31, Marcelo Tosatti wrote: >> >> It's not just about default configs. We need to validate if the >> migration formats are truly compatible (qemu-kvm -> QEMU, the other way >> around definitely not). For the command line switches, we could provide >> a wrapper script that translates them into upstream format or simply >> ignores them. That should be harmless to carry upstream. > > qemu-kvm has: > > -no-kvm > -no-kvm-irqchip > -no-kvm-pit > -no-kvm-pit-reinjection > -tdf <- does nothing > > There are replacements for all of the above. If we need to add them to > qemu.git, it's not big deal to add them. > > -drive ...,boot= <- this is ignored > > cpu_set command for CPU hotplug which is known broken in qemu-kvm. > > testdev which is nice but only used for development > > Default nic is rtl8139 vs. e1000. > > Some logic to move change the default VGA ram size to 16mb for pc-1.2 > (QEMU uses 16mb by default now too). > > I think at this point, none of this matters but I added the various > distro maintainers to the thread. > > I think it's time for the distros to drop qemu-kvm and just ship > qemu.git. Is there anything else that needs to happen to make that > switch? We are seriously considering moving to qemu.git for our SP3 release of SUSE SLES 11. There are just a handful of patches that provide the backwards compatibility we need to maintain (default to kvm, default nic model, vga ram size), so assuming there is a 100% commitment to fully supporting kvm in qemu going forward (which I don't doubt) I think this is a good time for us to make that switch. Bruce
Re: [Qemu-devel] failed to fail over between two bonded interface under user mode network stack
anybody could help? On Sun, Sep 30, 2012 at 3:12 PM, Mulyadi Santosa wrote: > Hi all... > > Recently, I did experiments to see what bonding can do. Therefore I > run Qemu using following command: > qemu-system-i386 -m 512 -hda ./centos.qcow2 \ > -net nic,model=e1000,vlan=0 -net user,vlan=0 \ > -net nic,model=e1000,vlan=0 -net user,vlan=0 \ > > The guest is CentOS 6.3. Host is Linux Mint 13 kernel 3.2.0-31-generic, using: > $ qemu-system-i386 --version > QEMU emulator version 1.0 (qemu-kvm-1.0), Copyright (c) 2003-2008 > Fabrice Bellard > > Inside the guest, I activate the bond interface using following command: > modprobe bonding mode=1 miimon=100 fail_over_mac=1 num_grat_arp=10 > primary=eth0 > > Then I enslave both eth0 and eth1 inside guest: > ifenslave bond0 eth0 eth1 > > I give static IP to the bond0: > ifconfig bond0 10.0.2.10 netmask 255.255.255.0 > > In this case, eth0 is 10.0.2.15 and eth1 is 10.0.2.16 > > I check that bond0 is correctly configured, then I ping 10.0.2.2. So > far so good. But when I take eth0 down: > ifconfig eth0 down > Eth1 takes over as active slave, but pinging to 10.0.2.2. > > What am I missing here? > > Just for note, I do relatively similar thing inside VirtualBox, only > that there I use host only networking. It works fine: failover happens > and ping continues when I disable eth0. > > Thanks in advance for your help. > > -- > regards, > > Mulyadi Santosa > Freelance Linux trainer and consultant > > blog: the-hydra.blogspot.com > training: mulyaditraining.blogspot.com -- regards, Mulyadi Santosa Freelance Linux trainer and consultant blog: the-hydra.blogspot.com training: mulyaditraining.blogspot.com
Re: [Qemu-devel] [PATCH v2] x86: Implement SMEP and SMAP
"H. Peter Anvin" writes: > From: "H. Peter Anvin" > > This patch implements Supervisor Mode Execution Prevention (SMEP) and > Supervisor Mode Access Prevention (SMAP) for x86. The purpose of the > patch, obviously, is to help kernel developers debug the support for > those features. > > A fair bit of the code relates to the handling of CPUID features. The > CPUID code probably would get greatly simplified if all the feature > bit words were unified into a single vector object, but in the > interest of producing a minimal patch for SMEP/SMAP, and because I had > very limited time for this project, I followed the existing style. > > [ v2: don't change the definition of the qemu64 CPU shorthand, since > that breaks loading old snapshots. Per Anthony Liguori this can be > fixed once the CPU feature set is snapshot. > > Change the coding style slightly to conform to checkpatch.pl. ] > > Signed-off-by: H. Peter Anvin Applied. Thanks. Regards, Anthony Liguori > --- > target-i386/cc_helper.c | 10 +++ > target-i386/cpu.c | 34 --- > target-i386/cpu.h | 33 -- > target-i386/helper.c| 150 > ++- > target-i386/helper.h|2 + > target-i386/translate.c | 27 +++-- > 6 files changed, 207 insertions(+), 49 deletions(-) > > diff --git a/target-i386/cc_helper.c b/target-i386/cc_helper.c > index 07892f9..9422003 100644 > --- a/target-i386/cc_helper.c > +++ b/target-i386/cc_helper.c > @@ -353,6 +353,16 @@ void helper_sti(CPUX86State *env) > env->eflags |= IF_MASK; > } > > +void helper_clac(CPUX86State *env) > +{ > +env->eflags &= ~AC_MASK; > +} > + > +void helper_stac(CPUX86State *env) > +{ > +env->eflags |= AC_MASK; > +} > + > #if 0 > /* vm86plus instructions */ > void helper_cli_vm(CPUX86State *env) > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index fd4fe28..f186439 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -100,6 +100,13 @@ static const char *svm_feature_name[] = { > NULL, NULL, NULL, NULL, > }; > > +static const char *cpuid_7_0_ebx_feature_name[] = { > +NULL, NULL, NULL, NULL, NULL, NULL, NULL, "smep", > +NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > +NULL, NULL, NULL, NULL, "smap", NULL, NULL, NULL, > +NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, > +}; > + > /* collects per-function cpuid data > */ > typedef struct model_features_t { > @@ -215,14 +222,17 @@ static void add_flagname_to_bitmaps(const char > *flagname, uint32_t *features, > uint32_t *ext2_features, > uint32_t *ext3_features, > uint32_t *kvm_features, > -uint32_t *svm_features) > +uint32_t *svm_features, > +uint32_t *cpuid_7_0_ebx_features) > { > if (!lookup_feature(features, flagname, NULL, feature_name) && > !lookup_feature(ext_features, flagname, NULL, ext_feature_name) && > !lookup_feature(ext2_features, flagname, NULL, ext2_feature_name) && > !lookup_feature(ext3_features, flagname, NULL, ext3_feature_name) && > !lookup_feature(kvm_features, flagname, NULL, kvm_feature_name) && > -!lookup_feature(svm_features, flagname, NULL, svm_feature_name)) > +!lookup_feature(svm_features, flagname, NULL, svm_feature_name) && > +!lookup_feature(cpuid_7_0_ebx_features, flagname, NULL, > +cpuid_7_0_ebx_feature_name)) > fprintf(stderr, "CPU feature %s not found\n", flagname); > } > > @@ -284,6 +294,7 @@ typedef struct x86_def_t { > #define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \ >CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A) > #define TCG_SVM_FEATURES 0 > +#define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP) > > /* maintains list of cpu model definitions > */ > @@ -1091,10 +1102,12 @@ static int cpu_x86_find_by_name(x86_def_t > *x86_cpu_def, const char *cpu_model) > uint32_t plus_features = 0, plus_ext_features = 0; > uint32_t plus_ext2_features = 0, plus_ext3_features = 0; > uint32_t plus_kvm_features = 0, plus_svm_features = 0; > +uint32_t plus_7_0_ebx_features = 0; > /* Features to be removed */ > uint32_t minus_features = 0, minus_ext_features = 0; > uint32_t minus_ext2_features = 0, minus_ext3_features = 0; > uint32_t minus_kvm_features = 0, minus_svm_features = 0; > +uint32_t minus_7_0_ebx_features = 0; > uint32_t numvalue; > > for (def = x86_defs; def; def = def->next) > @@ -1121,8 +1134,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, > const char *cpu_model) > #endif > > add_flagname_to_bitmaps("hypervisor", &plus_features, > -&plus_ext_features, &plus_ext2_features, &plus_ext3_features, > -
Re: [Qemu-devel] [PATCH v6 0/4] VFIO-based PCI device assignment
Alex Williamson writes: > v6: > Update patch 4/4 so Makefile just uses CONFIG_LINUX and > avoids all the noise in configure. > > Also available in git here: > > git://github.com/awilliam/qemu-vfio.git > branch: vfio-for-qemu > tag: vfio-pci-for-qemu-v6 Applied. Thanks. Regards, Anthony Liguori > > --- > > Alex Williamson (4): > vfio: Enable vfio-pci and mark supported > vfio: vfio-pci device assignment driver > Update Linux kernel headers > Update kernel header script to include vfio > > > MAINTAINERS |5 > hw/Makefile.objs|3 > hw/vfio_pci.c | 1864 > +++ > hw/vfio_pci_int.h | 114 ++ > linux-headers/linux/vfio.h | 368 > scripts/update-linux-headers.sh |2 > 6 files changed, 2354 insertions(+), 2 deletions(-) > create mode 100644 hw/vfio_pci.c > create mode 100644 hw/vfio_pci_int.h > create mode 100644 linux-headers/linux/vfio.h > -- > 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] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it
Jan Kiszka writes: > If we built a target for a host that supports KVM in principle, set the > default accelerator to KVM as well. This also means the start of QEMU > will fail to start if KVM support turns out to be unavailable at > runtime. > > Signed-off-by: Jan Kiszka > --- > kvm-all.c |1 + > kvm-stub.c |1 + > kvm.h |1 + > vl.c |4 ++-- > 4 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index 92a7137..4d5f86c 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -103,6 +103,7 @@ struct KVMState > #endif > }; > > +bool kvm_configured = true; > KVMState *kvm_state; > bool kvm_kernel_irqchip; > bool kvm_async_interrupts_allowed; > diff --git a/kvm-stub.c b/kvm-stub.c > index 3c52eb5..86a6451 100644 > --- a/kvm-stub.c > +++ b/kvm-stub.c > @@ -17,6 +17,7 @@ > #include "gdbstub.h" > #include "kvm.h" > > +bool kvm_configured; > KVMState *kvm_state; > bool kvm_kernel_irqchip; > bool kvm_async_interrupts_allowed; > diff --git a/kvm.h b/kvm.h > index dea2998..9936e5f 100644 > --- a/kvm.h > +++ b/kvm.h > @@ -22,6 +22,7 @@ > #include > #endif > > +extern bool kvm_configured; > extern int kvm_allowed; > extern bool kvm_kernel_irqchip; > extern bool kvm_async_interrupts_allowed; > diff --git a/vl.c b/vl.c > index 8d305ca..f557bd1 100644 > --- a/vl.c > +++ b/vl.c > @@ -2215,8 +2215,8 @@ static int configure_accelerator(void) > } > > if (p == NULL) { > -/* Use the default "accelerator", tcg */ > -p = "tcg"; > +/* The default accelerator depends on the availability of KVM. */ > +p = kvm_configured ? "kvm" : "tcg"; > } How about making this an arch_init() function call and then using a #if defined(KVM_CONFIG) in arch_init.c? I hate to introduce another global variable if we can avoid it... Otherwise: Acked-by: Anthony Liguori Blue/Aurelien, any objections? Regards, Anthony Liguori > > while (!accel_initialised && *p != '\0') { > -- > 1.7.3.4
Re: [Qemu-devel] [PATCH 2/2] cpu_physical_memory_write_rom() needs to do TB invalidates
Pavel Hrdina writes: > On 10/01/2012 04:28 PM, Anthony Liguori wrote: >> Pavel Hrdina writes: >> >>> On 09/12/2012 07:57 AM, David Gibson wrote: On Mon, Sep 10, 2012 at 03:27:45PM +0200, Andreas Färber wrote: > Am 10.09.2012 04:30, schrieb David Gibson: >> cpu_physical_memory_write_rom(), despite the name, can also be used to >> write images into RAM - and will often be used that way if the machine >> uses load_image_targphys() into RAM addresses. >> >> However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_rw() >> does invalidate any cached TBs which might be affected by the region > "doesn't"? > > Otherwise doesn't look wrong. Oops, comment updated. From 6b913afaf83f52ee787271827c84b492e8ac5895 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 22 Aug 2012 14:58:04 +1000 Subject: [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates cpu_physical_memory_write_rom(), despite the name, can also be used to write images into RAM - and will often be used that way if the machine uses load_image_targphys() into RAM addresses. However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_rw() doesn't invalidate any cached TBs which might be affected by the region written. This was breaking reset (under full emu) on the pseries machine - we loaded our firmware image into RAM, and while executing it rewrite the code at the entry point (correctly causing a TB invalidate/refresh). When we reset the firmware image was reloaded, but the TB from the rewrite was still active and caused us to get an illegal instruction trap. This patch fixes the bug by duplicating the tb invalidate code from cpu_physical_memory_rw() in cpu_physical_memory_write_rom(). Signed-off-by: David Gibson --- exec.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/exec.c b/exec.c index 5834766..eff40d7 100644 --- a/exec.c +++ b/exec.c @@ -3523,6 +3523,13 @@ void cpu_physical_memory_write_rom(target_phys_addr_t addr, /* ROM/RAM case */ ptr = qemu_get_ram_ptr(addr1); memcpy(ptr, buf, l); +if (!cpu_physical_memory_is_dirty(addr1)) { +/* invalidate code */ +tb_invalidate_phys_page_range(addr1, addr1 + l, 0); +/* set dirty bit */ +cpu_physical_memory_set_dirty_flags( +addr1, (0xff & ~CODE_DIRTY_FLAG)); +} qemu_put_ram_ptr(ptr); } len -= l; >>> Hi, >>> >>> this patch breaks Windows XP guest at all. Windows XP boot ends in loob >>> by restarting itself after time-out expires in windows advanced boot >>> options. >>> >>> I started the guest using this command-line: >>> >>> ./x86_64-softmmu/qemu-system-x86_64 -m 2G -drive >>> file=/data/data-shared/images/winxp-test.img -vnc 0.0.0.0:0 >> Does changing the tb_invalidate_phys_page_range() call to: >> >> tb_invalidate_phys_page_range(addr1, addr1 + MAX(l, TARGET_PAGE_SIZE), 0); >> >> The dirty flag is being reset for the full page but we're potentially >> only invalidating a subset of TBs that occur on the page. >> >> Regards, >> >> Anthony Liguori >> > > No, it doesn't fix this bug. Then I'm confused... invalidating TBs should never have a functional impact IIUC. Are you confident in your bisection? Reverting this patch fixes the problem? Regards, Anthony Liguori > > Pavel > >>> Pavel
Re: [Qemu-devel] [PATCH 2/2] cpu_physical_memory_write_rom() needs to do TB invalidates
On 10/01/2012 06:21 PM, Anthony Liguori wrote: Pavel Hrdina writes: On 10/01/2012 04:28 PM, Anthony Liguori wrote: Pavel Hrdina writes: On 09/12/2012 07:57 AM, David Gibson wrote: On Mon, Sep 10, 2012 at 03:27:45PM +0200, Andreas Färber wrote: Am 10.09.2012 04:30, schrieb David Gibson: cpu_physical_memory_write_rom(), despite the name, can also be used to write images into RAM - and will often be used that way if the machine uses load_image_targphys() into RAM addresses. However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_rw() does invalidate any cached TBs which might be affected by the region "doesn't"? Otherwise doesn't look wrong. Oops, comment updated. From 6b913afaf83f52ee787271827c84b492e8ac5895 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 22 Aug 2012 14:58:04 +1000 Subject: [PATCH] cpu_physical_memory_write_rom() needs to do TB invalidates cpu_physical_memory_write_rom(), despite the name, can also be used to write images into RAM - and will often be used that way if the machine uses load_image_targphys() into RAM addresses. However, cpu_physical_memory_write_rom(), unlike cpu_physical_memory_rw() doesn't invalidate any cached TBs which might be affected by the region written. This was breaking reset (under full emu) on the pseries machine - we loaded our firmware image into RAM, and while executing it rewrite the code at the entry point (correctly causing a TB invalidate/refresh). When we reset the firmware image was reloaded, but the TB from the rewrite was still active and caused us to get an illegal instruction trap. This patch fixes the bug by duplicating the tb invalidate code from cpu_physical_memory_rw() in cpu_physical_memory_write_rom(). Signed-off-by: David Gibson --- exec.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/exec.c b/exec.c index 5834766..eff40d7 100644 --- a/exec.c +++ b/exec.c @@ -3523,6 +3523,13 @@ void cpu_physical_memory_write_rom(target_phys_addr_t addr, /* ROM/RAM case */ ptr = qemu_get_ram_ptr(addr1); memcpy(ptr, buf, l); +if (!cpu_physical_memory_is_dirty(addr1)) { +/* invalidate code */ +tb_invalidate_phys_page_range(addr1, addr1 + l, 0); +/* set dirty bit */ +cpu_physical_memory_set_dirty_flags( +addr1, (0xff & ~CODE_DIRTY_FLAG)); +} qemu_put_ram_ptr(ptr); } len -= l; Hi, this patch breaks Windows XP guest at all. Windows XP boot ends in loob by restarting itself after time-out expires in windows advanced boot options. I started the guest using this command-line: ./x86_64-softmmu/qemu-system-x86_64 -m 2G -drive file=/data/data-shared/images/winxp-test.img -vnc 0.0.0.0:0 Does changing the tb_invalidate_phys_page_range() call to: tb_invalidate_phys_page_range(addr1, addr1 + MAX(l, TARGET_PAGE_SIZE), 0); The dirty flag is being reset for the full page but we're potentially only invalidating a subset of TBs that occur on the page. Regards, Anthony Liguori No, it doesn't fix this bug. Then I'm confused... invalidating TBs should never have a functional impact IIUC. Are you confident in your bisection? Reverting this patch fixes the problem? Regards, Anthony Liguori Yes I'm 100% sure that this commit cause this bug. I've tried to revert this patch and it works OK. Pavel Pavel Pavel
Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it
Hello Jan, Am 01.10.2012 16:34, schrieb Jan Kiszka: > If we built a target for a host that supports KVM in principle, set the > default accelerator to KVM as well. This also means the start of QEMU > will fail to start if KVM support turns out to be unavailable at > runtime. >From a distro point of view this of course means that we will build against KVM and that the new KVM default will start to fail for users on very old hardware. Can't we do a runtime check to select the default? Would be nice to at least amend the commit message with how they are expected to remedy that via command line. -machine accel=tcg? Regards, Andreas > > Signed-off-by: Jan Kiszka > --- > kvm-all.c |1 + > kvm-stub.c |1 + > kvm.h |1 + > vl.c |4 ++-- > 4 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index 92a7137..4d5f86c 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -103,6 +103,7 @@ struct KVMState > #endif > }; > > +bool kvm_configured = true; > KVMState *kvm_state; > bool kvm_kernel_irqchip; > bool kvm_async_interrupts_allowed; > diff --git a/kvm-stub.c b/kvm-stub.c > index 3c52eb5..86a6451 100644 > --- a/kvm-stub.c > +++ b/kvm-stub.c > @@ -17,6 +17,7 @@ > #include "gdbstub.h" > #include "kvm.h" > > +bool kvm_configured; > KVMState *kvm_state; > bool kvm_kernel_irqchip; > bool kvm_async_interrupts_allowed; > diff --git a/kvm.h b/kvm.h > index dea2998..9936e5f 100644 > --- a/kvm.h > +++ b/kvm.h > @@ -22,6 +22,7 @@ > #include > #endif > > +extern bool kvm_configured; > extern int kvm_allowed; > extern bool kvm_kernel_irqchip; > extern bool kvm_async_interrupts_allowed; > diff --git a/vl.c b/vl.c > index 8d305ca..f557bd1 100644 > --- a/vl.c > +++ b/vl.c > @@ -2215,8 +2215,8 @@ static int configure_accelerator(void) > } > > if (p == NULL) { > -/* Use the default "accelerator", tcg */ > -p = "tcg"; > +/* The default accelerator depends on the availability of KVM. */ > +p = kvm_configured ? "kvm" : "tcg"; > } > > while (!accel_initialised && *p != '\0') { > -- 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] kvm: Set default accelerator to "kvm" if the host supports it
On Mon, Oct 01, 2012 at 06:43:00PM +0200, Andreas Färber wrote: > Hello Jan, > > Am 01.10.2012 16:34, schrieb Jan Kiszka: > > If we built a target for a host that supports KVM in principle, set the > > default accelerator to KVM as well. This also means the start of QEMU > > will fail to start if KVM support turns out to be unavailable at > > runtime. > > From a distro point of view this of course means that we will build > against KVM and that the new KVM default will start to fail for users on > very old hardware. Can't we do a runtime check to select the default? NB, this is *not* only about old hardware. There are plenty of users who use QEMU inside VMs. One very common usage I know of is image building tools which are run inside Amazon VMs, using libguestfs & QEMU. IMHO, default to KVM, fallback to TCG is the most friendly default behaviour. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] qemu-kvm: remove "boot=on|off" drive parameter compatibility (fwd)
Quoting Peter Maydell (peter.mayd...@linaro.org): > On 1 October 2012 15:15, Serge Hallyn wrote: > > We (Ubuntu) plan to switch to qemu in the next release which opens in > > November. I suppose there's likely to be a hiccough or two, but I can't > > think of any offhand. > > Are you planning to do that for all CPU target architectures, or > to maintain the current split between x86 and everything-else ? > > -- PMM I'll have to talk to qemu-linaro folks about that. I don't mind carrying patchsets (that are headed upstream) to enable some chipsets if that's what's needed to consolidate the source trees. The bigger question relates to main vs universe. For instance qemu-system is built against vde2, and qemu-kvm-spice against spice, both of which are in universe. I understand the distinction may change or disappear soon, so this may not be an issue. I intend to schedule a session on this at UDS. -serge
Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it
On Mon, Oct 01, 2012 at 11:20:41AM -0500, Anthony Liguori wrote: > Jan Kiszka writes: > > > If we built a target for a host that supports KVM in principle, set the > > default accelerator to KVM as well. This also means the start of QEMU > > will fail to start if KVM support turns out to be unavailable at > > runtime. > > > > Signed-off-by: Jan Kiszka > > --- > > kvm-all.c |1 + > > kvm-stub.c |1 + > > kvm.h |1 + > > vl.c |4 ++-- > > 4 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/kvm-all.c b/kvm-all.c > > index 92a7137..4d5f86c 100644 > > --- a/kvm-all.c > > +++ b/kvm-all.c > > @@ -103,6 +103,7 @@ struct KVMState > > #endif > > }; > > > > +bool kvm_configured = true; > > KVMState *kvm_state; > > bool kvm_kernel_irqchip; > > bool kvm_async_interrupts_allowed; > > diff --git a/kvm-stub.c b/kvm-stub.c > > index 3c52eb5..86a6451 100644 > > --- a/kvm-stub.c > > +++ b/kvm-stub.c > > @@ -17,6 +17,7 @@ > > #include "gdbstub.h" > > #include "kvm.h" > > > > +bool kvm_configured; > > KVMState *kvm_state; > > bool kvm_kernel_irqchip; > > bool kvm_async_interrupts_allowed; > > diff --git a/kvm.h b/kvm.h > > index dea2998..9936e5f 100644 > > --- a/kvm.h > > +++ b/kvm.h > > @@ -22,6 +22,7 @@ > > #include > > #endif > > > > +extern bool kvm_configured; > > extern int kvm_allowed; > > extern bool kvm_kernel_irqchip; > > extern bool kvm_async_interrupts_allowed; > > diff --git a/vl.c b/vl.c > > index 8d305ca..f557bd1 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -2215,8 +2215,8 @@ static int configure_accelerator(void) > > } > > > > if (p == NULL) { > > -/* Use the default "accelerator", tcg */ > > -p = "tcg"; > > +/* The default accelerator depends on the availability of KVM. */ > > +p = kvm_configured ? "kvm" : "tcg"; > > } > > How about making this an arch_init() function call and then using a #if > defined(KVM_CONFIG) in arch_init.c? > > I hate to introduce another global variable if we can avoid it... > > Otherwise: > > Acked-by: Anthony Liguori > > Blue/Aurelien, any objections? > I am not sure this way of doing is really distribution friendly, where the QEMU package is built for a large variety of hosts, some of them maybe without KVM support. I am all for enabling KVM by default, but I think it should fall back to TCG if it is not enabled (probably with a warning so that the user is aware of that). On the other hand, if the user explicitly enables KVM support with -enable-kvm or with accel=kvm, it should fail to start. In other words, a bit like the configure script options, by default we use auto-detection, but if an option is explicitly enabled, it fails if it can't be enabled. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] fpu/softfloat.c: Return correctly signed values from uint64_to_float32
On Fri, Sep 28, 2012 at 04:17:03PM +0100, Peter Maydell wrote: > The uint64_to_float32() conversion function was incorrectly always > returning numbers with the sign bit set (ie negative numbers). Correct > this so we return positive numbers instead. > > Signed-off-by: Peter Maydell > --- > As far as I can see we use this function only in the three PPC SPE > insns efscfuf, efsctsf, efsctuf. It is therefore untested(!); if > anybody with PPC hw to test against could check the results of > those functions that would be cool. > > fpu/softfloat.c |4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index b29256a..91497e8 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -1238,7 +1238,7 @@ float32 uint64_to_float32( uint64 a STATUS_PARAM ) > if ( a == 0 ) return float32_zero; > shiftCount = countLeadingZeros64( a ) - 40; > if ( 0 <= shiftCount ) { > -return packFloat32( 1 > 0, 0x95 - shiftCount, a< +return packFloat32(0, 0x95 - shiftCount, a< } > else { > shiftCount += 7; > @@ -1248,7 +1248,7 @@ float32 uint64_to_float32( uint64 a STATUS_PARAM ) > else { > a <<= shiftCount; > } > -return roundAndPackFloat32( 1 > 0, 0x9C - shiftCount, a STATUS_VAR ); > +return roundAndPackFloat32(0, 0x9C - shiftCount, a STATUS_VAR); > } > } > Reviewed-by: Aurelien Jarno -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] pwrite64 error because of argument position
ok, thank you very much. Now I understand the problem... and I see the correct way of mending it. Your patch works for me. >On Mon, Oct 1, 2012 at 1:52 PM, Alexander Graf wrote: > > On 30.09.2012, at 20:50, Alex Barcelo wrote: > >> This error may be a PPC specific problem, but I don't have another >> environment where I can test it (i386 doesn't seem to use pwrite64), >> so I ask for a bit of help/check. >> >> I am in a 32bit linux testing the qemu-ppc. >> >> My test program: >> >> // --- >> #include >> #include >> #include >> #include >> #include >> >> int main (void) { >>int fd, len; >> >>char* asd = "This is a random test"; >>char asd2[20]; >> >>fd = open ("./test.pwrite", O_RDWR | O_CREAT); >> >>printf ( "fd: %d\n", fd); >>pwrite ( fd, asd, 15, 0 ); >>pwrite ( fd, asd+5, 6, 10); >>pwrite ( fd, asd, 4, 30); >> >>len = pread ( fd, asd2, 5, 5); >>asd2[len]='\0'; >>printf ( "Read %d bytes: %s", len, asd2); >> >>// This is to force two int arguments for 64bit >>//len = pread ( fd, asd2, -1, -1); >>close(fd); >>return 0; >> } >> // --- >> >> Then I do >> $ powerpc-linux-gnu-gcc -g --static -o pwrite-alien pwrite-test.c >> >> and when I launch a qemu-ppc to test, it should be (on the file) >> This is a is a rThis >> >> instead I get: >> This rs a randorThis >> >> and if I print some debug information inside pwrite64 and pread64 I see: >> syscall: pwrite arg_i: 3 268909324 15 0 0 0 0 0 >> syscall: pwrite arg_i: 3 268909329 6 0 0 10 0 0 >> syscall: pwrite arg_i: 3 268909324 4 0 0 30 0 0 >> syscall: preadarg_i: 3 1082133156 5 0 0 5 0 0 >> (those are arg1, arg2, arg3, arg4, arg5, arg6 and the unused arg7 and arg8) >> >> As can be seen, arg4 is not used, and the line >> ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, arg5))); >> should be, in my case >> ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg5, arg6))); >> >> With this changes, the qemu "Works For Me (TM)". >> >> So, anybody can confirm it or (if it is indeed my problem) can give me >> some pointers? I will not post this as a patch until I understand the >> problem... and first step is making sure that it really is a qemu >> problem. And not my toolchain or something random. > > Running the above code on a real ppc machine (compiled with -m32, running a > ppc64 kernel) I get: > > 54 68 69 73 20 69 73 20 61 20 69 73 20 61 20 72 |This is a is a r| > 0010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 54 68 |..Th| > 0020 69 73 |is| > > as output. Is this the expected result? Running it in qemu-ppc, I get: > > 54 68 69 73 20 72 73 20 61 20 72 61 6e 64 6f |This rs a rando| > 000f > > So yes, something looks broken here. > > According to arch/powerpc/kernel/sys_ppc32.c, the ppc32 ABI carries 64bit > parameters on odd/even pairs: > > /* > * long long munging: > * The 32 bit ABI passes long longs in an odd even register pair. > */ > > which is almost the same as what ARM or MIPS do and which explains why you > see arg5/arg6 (r7/r8) used instead of arg4/arg5 (r6/r7). > > Could you please try the below patch? > > > Alex > > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 1a38169..e03b3a8 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -587,12 +587,16 @@ extern int setfsgid(int); > extern int setgroups(int, gid_t *); > > /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */ > -#ifdef TARGET_ARM > +#ifdef TARGET_ARM > static inline int regpairs_aligned(void *cpu_env) { > return CPUARMState *)cpu_env)->eabi) == 1) ; > } > #elif defined(TARGET_MIPS) > static inline int regpairs_aligned(void *cpu_env) { return 1; } > +#elif defined(TARGET_PPC) && !defined(TARGET_PPC64) > +/* PPC32 expects 64bit parameters to be passed on odd/even pairs of registers > + which is the same as ARM/MIPS, because we start with r3 as arg1. */ > +static inline int regpairs_aligned(void *cpu_env) { return 1; } > #else > static inline int regpairs_aligned(void *cpu_env) { return 0; } > #endif > @@ -7419,12 +7423,20 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > #endif > #ifdef TARGET_NR_pread64 > case TARGET_NR_pread64: > +if (regpairs_aligned(cpu_env)) { > +arg4 = arg5; > +arg5 = arg6; > +} > if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0))) > goto efault; > ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5))); > unlock_user(p, arg2, ret); > break; > case TARGET_NR_pwrite64: > +if (regpairs_aligned(cpu_env)) { > +arg4 = arg5; > +arg5 = arg6; > +} > if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1))) > goto efault; > ret = get_errno(pwrite6
Re: [Qemu-devel] pwrite64 error because of argument position
On 01.10.2012, at 18:57, Alex Barcelo wrote: > ok, thank you very much. Now I understand the problem... and I see the > correct way of mending it. > > Your patch works for me. Awesome. Could you please put your Tested-by tag below the patches I sent to the ML earlier to day then? :) Alex
Re: [Qemu-devel] [PATCH] fpu/softfloat.c: Return correctly signed values from uint64_to_float32
On Fri, Sep 28, 2012 at 04:42:16PM +0100, Peter Maydell wrote: > On 28 September 2012 16:17, Peter Maydell wrote: > > The uint64_to_float32() conversion function was incorrectly always > > returning numbers with the sign bit set (ie negative numbers). Correct > > this so we return positive numbers instead. > > > > Signed-off-by: Peter Maydell > > --- > > As far as I can see we use this function only in the three PPC SPE > > insns efscfuf, efsctsf, efsctuf. It is therefore untested(!); if > > anybody with PPC hw to test against could check the results of > > those functions that would be cool. SPE instructions are not common on the non-embedded world, as far as I know it should be tested using the e500 CPU. That said I don't have a kernel nor an image for such a machine. > ...incidentally in two of those uses we're operating on a constant: > tmp = uint64_to_float64(1ULL << 32, &env->vec_status); > and it would probably be better to use make_float64() instead. > Agreed. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH v2 4/9] qemu-sockets: add error propagation to Unix socket functions
On Mon, 1 Oct 2012 16:52:19 +0200 Paolo Bonzini wrote: > This patch starts harmonizing unix_* and inet_* functions. > > Signed-off-by: Paolo Bonzini > --- > nbd.c | 4 ++-- > qemu-char.c | 4 ++-- > qemu-sockets.c | 40 > qemu_socket.h | 8 > qga/channel-posix.c | 2 +- > ui/vnc.c| 4 ++-- > 6 file modificati, 31 inserzioni(+), 31 rimozioni(-) > > diff --git a/nbd.c b/nbd.c > index 6f0db62..f61a288 100644 > --- a/nbd.c > +++ b/nbd.c > @@ -230,12 +230,12 @@ int unix_socket_incoming(const char *path) > char *ostr = NULL; > int olen = 0; > > -return unix_listen(path, ostr, olen); > +return unix_listen(path, ostr, olen, NULL); > } > > int unix_socket_outgoing(const char *path) > { > -return unix_connect(path); > +return unix_connect(path, NULL); > } > > /* Basic flow for negotiation > diff --git a/qemu-char.c b/qemu-char.c > index b082bae..fa294c0 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -2448,9 +2448,9 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts > *opts) > > if (is_unix) { > if (is_listen) { > -fd = unix_listen_opts(opts); > +fd = unix_listen_opts(opts, NULL); > } else { > -fd = unix_connect_opts(opts); > +fd = unix_connect_opts(opts, NULL); > } > } else { > if (is_listen) { > diff --git a/qemu-sockets.c b/qemu-sockets.c > index bf1f794..1e69274 100644 > --- a/qemu-sockets.c > +++ b/qemu-sockets.c > @@ -655,7 +655,7 @@ int inet_nonblocking_connect(const char *str, > > #ifndef _WIN32 > > -int unix_listen_opts(QemuOpts *opts) > +int unix_listen_opts(QemuOpts *opts, Error **errp) > { > struct sockaddr_un un; > const char *path = qemu_opt_get(opts, "path"); > @@ -663,7 +663,7 @@ int unix_listen_opts(QemuOpts *opts) > > sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0); > if (sock < 0) { > -perror("socket(unix)"); > +error_set(errp, QERR_SOCKET_CREATE_FAILED); > return -1; > } > > @@ -688,11 +688,11 @@ int unix_listen_opts(QemuOpts *opts) > > unlink(un.sun_path); > if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) { > -fprintf(stderr, "bind(unix:%s): %s\n", un.sun_path, strerror(errno)); > +error_set(errp, QERR_SOCKET_BIND_FAILED); This drops error information, making the error message worse. I believe you have a reason to not use error_setg()? Also, I see that in some hunks you do something like: -fd = unix_listen_opts(opts); +fd = unix_listen_opts(opts, NULL); This will break printing the error message to the user. It's fine by me if you do this only temporarily (ie. this is fixed by the next or a later patch), but want to double check that you're aware of it. Btw, I'm making these comments in this hunk but they apply to similar hunks as well. > goto err; > } > if (listen(sock, 1) < 0) { > -fprintf(stderr, "listen(unix:%s): %s\n", un.sun_path, > strerror(errno)); > +error_set(errp, QERR_SOCKET_LISTEN_FAILED); > goto err; > } > > @@ -703,20 +703,20 @@ err: > return -1; > } > > -int unix_connect_opts(QemuOpts *opts) > +int unix_connect_opts(QemuOpts *opts, Error **errp) > { > struct sockaddr_un un; > const char *path = qemu_opt_get(opts, "path"); > int sock; > > if (NULL == path) { > -fprintf(stderr, "unix connect: no path specified\n"); > +error_setg(errp, "unix connect: no path specified\n"); > return -1; > } > > sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0); > if (sock < 0) { > -perror("socket(unix)"); > +error_set(errp, QERR_SOCKET_CREATE_FAILED); > return -1; > } > > @@ -724,7 +724,7 @@ int unix_connect_opts(QemuOpts *opts) > un.sun_family = AF_UNIX; > snprintf(un.sun_path, sizeof(un.sun_path), "%s", path); > if (connect(sock, (struct sockaddr*) &un, sizeof(un)) < 0) { > -fprintf(stderr, "connect(unix:%s): %s\n", path, strerror(errno)); > +error_set(errp, QERR_SOCKET_CONNECT_FAILED); > close(sock); > return -1; > } > @@ -733,7 +733,7 @@ int unix_connect_opts(QemuOpts *opts) > } > > /* compatibility wrapper */ > -int unix_listen(const char *str, char *ostr, int olen) > +int unix_listen(const char *str, char *ostr, int olen, Error **errp) > { > QemuOpts *opts; > char *path, *optstr; > @@ -754,7 +754,7 @@ int unix_listen(const char *str, char *ostr, int olen) > qemu_opt_set(opts, "path", str); > } > > -sock = unix_listen_opts(opts); > +sock = unix_listen_opts(opts, errp); > > if (sock != -1 && ostr) > snprintf(ostr, olen, "%s%s", qemu_opt_get(opts, "path"), optstr ? > optstr : ""); > @@ -762,44 +762,44 @@ int unix_listen(const char *str, char *ostr, int olen) > return so
Re: [Qemu-devel] [PATCH v6 3/4] vfio: vfio-pci device assignment driver
This: struct vfio_iommu_type1_dma_map map = { .argsz = sizeof(map), .flags = VFIO_DMA_MAP_FLAG_READ, .vaddr = (__u64)vaddr, .iova = iova, .size = size, }; (around line 771) breaks in my environment. I am in a crosschain environment on a i386 buildin PPC (32bit) binaries. I am using emdebian utilities, and have been cross building qemu for months now. The configure: PKG_CONFIG_PATH=/usr/powerpc-linux-gnu/lib/pkgconfig ./configure --prefix=/mnt/DATA/DARCO/alien --enable-debug-tcg --enable-debug --enable-trace-backend=simple --disable-strip --disable-kvm --enable-profiler --target-list=i386-softmmu --static --disable-curl --cross-prefix=powerpc-linux-gnu- --with-coroutine=sigaltstack The build: make clean && make -j && make install The error: /mnt/DATA/DARCO/qemu-git/hw/vfio_pci.c: In function 'vfio_dma_map': /mnt/DATA/DARCO/qemu-git/hw/vfio_pci.c:771: error: cast from pointer to integer of different size
Re: [Qemu-devel] [PATCH 1/2] linux-user: ppc: mark as long long aligned
On Sun, 30 Sep 2012, Alexander Graf wrote: > The PPC32 ABI dictates that long long (64bit) parameters are pass in odd/even > register pairs. Because unlike ARM and MIPS we start at an odd register > number, > we can reuse the same aligning code that ARM and MIPS use. > > Signed-off-by: Alexander Graf > --- > linux-user/syscall.c |6 +- > 1 files changed, 5 insertions(+), 1 deletions(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 1a38169..8cd56f2 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -587,12 +587,16 @@ extern int setfsgid(int); > extern int setgroups(int, gid_t *); > > /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */ > -#ifdef TARGET_ARM > +#ifdef TARGET_ARM > static inline int regpairs_aligned(void *cpu_env) { > return CPUARMState *)cpu_env)->eabi) == 1) ; > } > #elif defined(TARGET_MIPS) > static inline int regpairs_aligned(void *cpu_env) { return 1; } > +#elif defined(TARGET_PPC) && !defined(TARGET_PPC64) > +/* PPC32 expects 64bit parameters to be passed on odd/even pairs of registers > + which translates to the same as ARM/MIPS, because we start with r3 as > arg1 */ This is inaccurate, PPC32 doesn't expect anything, SysV ABI for PPC32, which linux uses, does. This is linux-user so SysV ABI is a given but still i'd reword it. > +static inline int regpairs_aligned(void *cpu_env) { return 1; } > #else > static inline int regpairs_aligned(void *cpu_env) { return 0; } > #endif > -- mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH 2/2] linux-user: register align p{read, write}64
On Sun, Sep 30, 2012 at 3:32 AM, Alexander Graf wrote: > pread64 and pwrite64 pass 64bit parameters which for some architectures need > to be aligned to special argument pairs, creating a gap argument. > > Handle this special case the same way we handle it in other places of the > code. > > Reported-by: Alex Barcelo > Signed-off-by: Alexander Graf > --- > linux-user/syscall.c |8 > 1 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 8cd56f2..7992b1b 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -7423,12 +7423,20 @@ abi_long do_syscall(void *cpu_env, int num, abi_long > arg1, > #endif > #ifdef TARGET_NR_pread64 > case TARGET_NR_pread64: > +if (regpairs_aligned(cpu_env)) { > +arg4 = arg5; > +arg5 = arg6; > +} > if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0))) > goto efault; > ret = get_errno(pread64(arg1, p, arg3, target_offset64(arg4, arg5))); > unlock_user(p, arg2, ret); > break; > case TARGET_NR_pwrite64: > +if (regpairs_aligned(cpu_env)) { > +arg4 = arg5; > +arg5 = arg6; > +} > if (!(p = lock_user(VERIFY_READ, arg2, arg3, 1))) > goto efault; > ret = get_errno(pwrite64(arg1, p, arg3, target_offset64(arg4, > arg5))); > -- > 1.6.0.2 > > Tested-by: Alex Barcelo
Re: [Qemu-devel] [PATCH 2/7] tcg: Optimize add2 + sub2
On Thu, Sep 27, 2012 at 04:28:47PM -0700, Richard Henderson wrote: > On 09/27/2012 04:20 PM, Aurelien Jarno wrote: > > I understand that we can't easily insert an instruction, so the > > limitation comes from here, but is it really something happening often? > > It will certainly appear sometimes. E.g. s390x has an add immediate > instruction that does exactly: r1 += imm16 << 32. > > Or did you mean specifically the full constant being folded? That > would happen quite a bit more often. That you can see with most any > 64-bit RISC guest when they attempt to generate a constant from > addition primitives instead of logical primitives. > > For a 32-bit host, we've already decomposed logical primitives to 32-bit > operations. And we can constant-fold through all of those. But when > addition comes into play, we can't constant-fold through add2. > I tried this patch on an i386 host running an x86_64 target, but it even fails to start seabios, there is probably a wrong logic somewhere in the patch. For the first add2 that seemed to have work correctly, this patch optimized 0.2% of them. I am not sure it worth it as is. I think optimizing add2, and in general all *2 ops is a good idea, but we should be able to do more agressive optimization. Maybe, a bit like Blue was suggesting, add2 should always be followed by a nop, so we can do more optimizations? -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH v2 0/9] Embedded NBD server
On Mon, 1 Oct 2012 16:52:15 +0200 Paolo Bonzini wrote: > This series rebases the previous qemu-sockets patches for error > propagation and uses the new QAPI-friendly socket functions in the > embedded NBD server. The changes are due to Orit's patches being > now in, some early parts being in Luiz's queue, and glusterfs > patches not having touched qemu-sockets.c in the end. > > Patches 1 to 4 start moving qemu-sockets functions away from error_report > (or printf) and away from QemuOpts. Patch 5 makes it easier to reuse > the address parser of inet_parse in the new socket_parse function. > Patch 6 introduces QAPI-friendly socket parsing and creation functions. > > Patches 7 and 8 introduces the QMP commands, and patch 9 introduces the > HMP version. Some comments against 4/9, plus this: CCqemu-sockets.o /home/lcapitulino/work/src/qmp-unstable/qemu-sockets.c: In function ‘socket_connect’: /home/lcapitulino/work/src/qmp-unstable/qemu-sockets.c:894:9: error: passing argument 2 of ‘inet_connect_opts’ makes pointer from integer without a cast [-Werror] /home/lcapitulino/work/src/qmp-unstable/qemu-sockets.c:365:5: note: expected ‘struct Error **’ but argument is of type ‘int’ /home/lcapitulino/work/src/qmp-unstable/qemu-sockets.c:894:9: error: too many arguments to function ‘inet_connect_opts’ /home/lcapitulino/work/src/qmp-unstable/qemu-sockets.c:365:5: note: declared here cc1: all warnings being treated as errors make: *** [qemu-sockets.o] Error 1 make: *** Waiting for unfinished jobs It's also worth it to mention that this series actually depends on my last pull request, but it's ok if this is going through my tree. Otherwise, looks good. > > Paolo > > Paolo Bonzini (9): > build: add QAPI files to the tools > qapi: add socket address types > qemu-sockets: add error propagation to inet_parse > qemu-sockets: add error propagation to Unix socket functions > qemu-sockets: return IPSocketAddress from inet_parse > qemu-sockets: add socket_listen, socket_connect, socket_parse > block: add close notifiers > qmp: add NBD server commands > hmp: add NBD server commands > > Makefile.objs | 8 +- > block.c | 19 +++- > block.h | 1 + > block_int.h | 2 + > blockdev-nbd.c | 119 > hmp-commands.hx | 29 ++ > hmp.c | 55 +++ > hmp.h | 2 + > nbd.c | 4 +- > qapi-schema.json| 96 +++ > qemu-char.c | 4 +- > qemu-sockets.c | 261 > +++- > qemu-tool.c | 6 ++ > qemu_socket.h | 12 ++- > qga/channel-posix.c | 2 +- > qmp-commands.hx | 16 > ui/vnc.c| 4 +- > 17 file modificati, 557 inserzioni(+), 83 rimozioni(-) > create mode 100644 blockdev-nbd.c >
Re: [Qemu-devel] [PATCH 2/7] tcg: Optimize add2 + sub2
On 2012-09-30 00:04, Blue Swirl wrote: >> We can't do complete constant folding because we lack "mov2", >> > or the ability to insert opcodes in the stream. But we can >> > at least canonicalize add2 operand ordering and simplify >> > add2 to add when the lowpart adds a constant 0. > Couldn't we introduce add2_part1 and add2_part2, the latter being nop > for architectures that don't need it? Possibly. It certainly would be easy to model these as addcc + addx on targets like sparc where CC never gets clobbered during moves. I'm a bit worried about i386 though, since loading 0 wants to use xor and clobber the flags. We could possibly work around this by taking care of the constant loading for add2_part2 manually. E.g. { INDEX_op_add2_part2, { "r", "ri", "ri" } } if (args[2] == args[0] && !const_args[2]) { // swap arg1 arg2 } if (const_args[1]) { mov $args[1], args[0] } else { mov args[1], args[0] } adcl args[2], args[0] which means that tcg_out_movi will not have to be called in between. It's all a bit fragile though. That said, I do wonder if having a synthetic mov2{rr,ri,ii} opcodes isn't just easier. That could be broken up into two moves by tcg.c without the backends having to care about it. r~
Re: [Qemu-devel] [PATCH 2/7] tcg: Optimize add2 + sub2
On 2012-10-01 10:46, Aurelien Jarno wrote: > For the first add2 that seemed to have work correctly, this patch > optimized 0.2% of them. I am not sure it worth it as is. You're probably right. > I think optimizing add2, and in general all *2 ops is a good idea, but > we should be able to do more agressive optimization. Maybe, a bit like > Blue was suggesting, add2 should always be followed by a nop, so we can > do more optimizations? Adding an extra nop sounds like a better idea than add2_part[12]. And it's probably easier than adding mov2 opcodes -- one little assert inside the optimizer and we're golden. r~
[Qemu-devel] [PATCH] vfio_pci: fix build on 32-bit systems
We cannot cast directly from pointer to uint64. Cc: Alex Williamson Cc: Alex Barcelo Reported-by: Alex Barcelo Signed-off-by: Anthony Liguori --- hw/vfio_pci.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c index a24558a..a1eeced 100644 --- a/hw/vfio_pci.c +++ b/hw/vfio_pci.c @@ -768,7 +768,7 @@ static int vfio_dma_map(VFIOContainer *container, target_phys_addr_t iova, struct vfio_iommu_type1_dma_map map = { .argsz = sizeof(map), .flags = VFIO_DMA_MAP_FLAG_READ, -.vaddr = (__u64)vaddr, +.vaddr = (__u64)(intptr_t)vaddr, .iova = iova, .size = size, }; -- 1.7.5.4
Re: [Qemu-devel] [PATCH 4/7] tcg: Optimize double-word comparisons against zero
On Thu, Sep 27, 2012 at 10:19:54AM -0700, Richard Henderson wrote: > Signed-off-by: Richard Henderson > --- > tcg/optimize.c | 51 +++ > 1 file changed, 51 insertions(+) > > diff --git a/tcg/optimize.c b/tcg/optimize.c > index d39926e..c972e4f 100644 > --- a/tcg/optimize.c > +++ b/tcg/optimize.c > @@ -799,6 +799,57 @@ static TCGArg *tcg_constant_folding(TCGContext *s, > uint16_t *tcg_opc_ptr, > } > args += 6; > break; > +case INDEX_op_brcond2_i32: > +/* Simplify LT/GE comparisons vs zero to a single compare > + vs the high word of the input. */ > +if ((args[4] == TCG_COND_LT || args[4] == TCG_COND_GE) > +&& temps[args[2]].state == TCG_TEMP_CONST > +&& temps[args[3]].state == TCG_TEMP_CONST > +&& temps[args[2]].val == 0 > +&& temps[args[2]].val == 0) { The value comparison there is wrong, probably copy & paste issue. I wonder how it could work. > +gen_opc_buf[op_index] = INDEX_op_brcond_i32; > +args[0] = args[1]; > +args[1] = args[3]; > +args[2] = args[4]; > +args[3] = args[5]; > +gen_args += 4; > +} else { > +gen_args[0] = args[0]; > +gen_args[1] = args[1]; > +gen_args[2] = args[2]; > +gen_args[3] = args[3]; > +gen_args[4] = args[4]; > +gen_args[5] = args[5]; > +gen_args += 6; > +} > +memset(temps, 0, nb_temps * sizeof(struct tcg_temp_info)); > +args += 6; > +break; > +case INDEX_op_setcond2_i32: > +/* Simplify LT/GE comparisons vs zero to a single compare > + vs the high word of the input. */ > +if ((args[5] == TCG_COND_LT || args[5] == TCG_COND_GE) > +&& temps[args[3]].state == TCG_TEMP_CONST > +&& temps[args[4]].state == TCG_TEMP_CONST > +&& temps[args[3]].val == 0 > +&& temps[args[4]].val == 0) { Here it is fine. > +gen_opc_buf[op_index] = INDEX_op_setcond_i32; > +args[1] = args[2]; > +args[2] = args[4]; > +args[3] = args[5]; > +gen_args += 4; > +} else { > +reset_temp(args[0]); > +gen_args[0] = args[0]; > +gen_args[1] = args[1]; > +gen_args[2] = args[2]; > +gen_args[3] = args[3]; > +gen_args[4] = args[4]; > +gen_args[5] = args[5]; > +gen_args += 6; > +} > +args += 6; > +break; > case INDEX_op_call: > nb_call_args = (args[0] >> 16) + (args[0] & 0x); > if (!(args[nb_call_args + 1] & (TCG_CALL_CONST | > TCG_CALL_PURE))) { While it's a nice optimization to have, one that seems to happen a lot more often is the two high parts being equal. It happens when the guest is working on (u)int32_t. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH 5/7] tcg: Split out subroutines from do_constant_folding_cond
On Thu, Sep 27, 2012 at 10:19:55AM -0700, Richard Henderson wrote: > We can re-use these for implementing double-word folding. > > Signed-off-by: Richard Henderson > --- > tcg/optimize.c | 146 > - > 1 file changed, 81 insertions(+), 65 deletions(-) > > diff --git a/tcg/optimize.c b/tcg/optimize.c > index c972e4f..c1881fa 100644 > --- a/tcg/optimize.c > +++ b/tcg/optimize.c > @@ -292,6 +292,82 @@ static TCGArg do_constant_folding(TCGOpcode op, TCGArg > x, TCGArg y) > return res; > } > > +static bool do_constant_folding_cond_32(uint32_t x, uint32_t y, TCGCond c) > +{ > +switch (c) { > +case TCG_COND_EQ: > +return x == y; > +case TCG_COND_NE: > +return x != y; > +case TCG_COND_LT: > +return (int32_t)x < (int32_t)y; > +case TCG_COND_GE: > +return (int32_t)x >= (int32_t)y; > +case TCG_COND_LE: > +return (int32_t)x <= (int32_t)y; > +case TCG_COND_GT: > +return (int32_t)x > (int32_t)y; > +case TCG_COND_LTU: > +return x < y; > +case TCG_COND_GEU: > +return x >= y; > +case TCG_COND_LEU: > +return x <= y; > +case TCG_COND_GTU: > +return x > y; > +default: > +tcg_abort(); > +} > +} > + > +static bool do_constant_folding_cond_64(uint64_t x, uint64_t y, TCGCond c) > +{ > +switch (c) { > +case TCG_COND_EQ: > +return x == y; > +case TCG_COND_NE: > +return x != y; > +case TCG_COND_LT: > +return (int64_t)x < (int64_t)y; > +case TCG_COND_GE: > +return (int64_t)x >= (int64_t)y; > +case TCG_COND_LE: > +return (int64_t)x <= (int64_t)y; > +case TCG_COND_GT: > +return (int64_t)x > (int64_t)y; > +case TCG_COND_LTU: > +return x < y; > +case TCG_COND_GEU: > +return x >= y; > +case TCG_COND_LEU: > +return x <= y; > +case TCG_COND_GTU: > +return x > y; > +default: > +tcg_abort(); > +} > +} > + > +static bool do_constant_folding_cond_eq(TCGCond c) > +{ > +switch (c) { > +case TCG_COND_GT: > +case TCG_COND_LTU: > +case TCG_COND_LT: > +case TCG_COND_GTU: > +case TCG_COND_NE: > +return 0; > +case TCG_COND_GE: > +case TCG_COND_GEU: > +case TCG_COND_LE: > +case TCG_COND_LEU: > +case TCG_COND_EQ: > +return 1; > +default: > +tcg_abort(); > +} > +} > + > /* Return 2 if the condition can't be simplified, and the result > of the condition (0 or 1) if it can */ > static TCGArg do_constant_folding_cond(TCGOpcode op, TCGArg x, > @@ -300,69 +376,14 @@ static TCGArg do_constant_folding_cond(TCGOpcode op, > TCGArg x, > if (temps[x].state == TCG_TEMP_CONST && temps[y].state == > TCG_TEMP_CONST) { > switch (op_bits(op)) { > case 32: > -switch (c) { > -case TCG_COND_EQ: > -return (uint32_t)temps[x].val == (uint32_t)temps[y].val; > -case TCG_COND_NE: > -return (uint32_t)temps[x].val != (uint32_t)temps[y].val; > -case TCG_COND_LT: > -return (int32_t)temps[x].val < (int32_t)temps[y].val; > -case TCG_COND_GE: > -return (int32_t)temps[x].val >= (int32_t)temps[y].val; > -case TCG_COND_LE: > -return (int32_t)temps[x].val <= (int32_t)temps[y].val; > -case TCG_COND_GT: > -return (int32_t)temps[x].val > (int32_t)temps[y].val; > -case TCG_COND_LTU: > -return (uint32_t)temps[x].val < (uint32_t)temps[y].val; > -case TCG_COND_GEU: > -return (uint32_t)temps[x].val >= (uint32_t)temps[y].val; > -case TCG_COND_LEU: > -return (uint32_t)temps[x].val <= (uint32_t)temps[y].val; > -case TCG_COND_GTU: > -return (uint32_t)temps[x].val > (uint32_t)temps[y].val; > -} > -break; > +return do_constant_folding_cond_32(temps[x].val, temps[y].val, > c); > case 64: > -switch (c) { > -case TCG_COND_EQ: > -return (uint64_t)temps[x].val == (uint64_t)temps[y].val; > -case TCG_COND_NE: > -return (uint64_t)temps[x].val != (uint64_t)temps[y].val; > -case TCG_COND_LT: > -return (int64_t)temps[x].val < (int64_t)temps[y].val; > -case TCG_COND_GE: > -return (int64_t)temps[x].val >= (int64_t)temps[y].val; > -case TCG_COND_LE: > -return (int64_t)temps[x].val <= (int64_t)temps[y].val; > -case TCG_COND_GT: > -return (int64_t)temps[x].val > (int64_t)temps[y].val; > -case TCG_COND_LTU: > -return (uint64_t)temps[x].val < (uint64_t)temps[y].val; > -case TCG_COND_GEU: > -return (uint64_t)temps[
Re: [Qemu-devel] [PATCH 4/7] tcg: Optimize double-word comparisons against zero
On 2012-10-01 11:43, Aurelien Jarno wrote: > While it's a nice optimization to have, one that seems to happen a lot > more often is the two high parts being equal. It happens when the guest > is working on (u)int32_t. It depends on what target you're looking at. For alpha guest, all branches are comparisons vs zero, so LT/GE happens with some regularity. r~
Re: [Qemu-devel] [PATCH 6/7] tcg: Tidy brcond optimization
On Thu, Sep 27, 2012 at 10:19:56AM -0700, Richard Henderson wrote: > Do the memset once. Don't reset_temp before doing so. > > Signed-off-by: Richard Henderson > --- > tcg/optimize.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tcg/optimize.c b/tcg/optimize.c > index c1881fa..dfac877 100644 > --- a/tcg/optimize.c > +++ b/tcg/optimize.c > @@ -771,22 +771,22 @@ static TCGArg *tcg_constant_folding(TCGContext *s, > uint16_t *tcg_opc_ptr, > tmp = do_constant_folding_cond(op, args[0], args[1], args[2]); > if (tmp != 2) { > if (tmp) { > -memset(temps, 0, nb_temps * sizeof(struct > tcg_temp_info)); > gen_opc_buf[op_index] = INDEX_op_br; > gen_args[0] = args[3]; > gen_args += 1; > } else { > gen_opc_buf[op_index] = INDEX_op_nop; > +args += 4; > +break; > } > } else { > -memset(temps, 0, nb_temps * sizeof(struct tcg_temp_info)); > -reset_temp(args[0]); > gen_args[0] = args[0]; > gen_args[1] = args[1]; > gen_args[2] = args[2]; > gen_args[3] = args[3]; > gen_args += 4; > } > +memset(temps, 0, nb_temps * sizeof(struct tcg_temp_info)); > args += 4; > break; > CASE_OP_32_64(movcond): > -- > 1.7.11.4 > Removing the useless reset_temp() is indeed something to do. I am not so sure that factorizing the memset() and putting a break in the nop case is easier to read. Nevertheless: Reviewed-by: Aurelien Jarno -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] vfio_pci: fix build on 32-bit systems
On Mon, 2012-10-01 at 13:41 -0500, Anthony Liguori wrote: > We cannot cast directly from pointer to uint64. > > Cc: Alex Williamson > Cc: Alex Barcelo > Reported-by: Alex Barcelo > Signed-off-by: Anthony Liguori > --- > hw/vfio_pci.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c > index a24558a..a1eeced 100644 > --- a/hw/vfio_pci.c > +++ b/hw/vfio_pci.c > @@ -768,7 +768,7 @@ static int vfio_dma_map(VFIOContainer *container, > target_phys_addr_t iova, > struct vfio_iommu_type1_dma_map map = { > .argsz = sizeof(map), > .flags = VFIO_DMA_MAP_FLAG_READ, > -.vaddr = (__u64)vaddr, > +.vaddr = (__u64)(intptr_t)vaddr, > .iova = iova, > .size = size, > }; Thanks Anthony Acked-by: Alex Williamson
Re: [Qemu-devel] [PATCH 7/7] tcg: Do constant folding on double-word comparisons
On Thu, Sep 27, 2012 at 10:19:57AM -0700, Richard Henderson wrote: > Signed-off-by: Richard Henderson > --- > tcg/optimize.c | 134 > - > 1 file changed, 94 insertions(+), 40 deletions(-) > > diff --git a/tcg/optimize.c b/tcg/optimize.c > index dfac877..f6a16fd 100644 > --- a/tcg/optimize.c > +++ b/tcg/optimize.c > @@ -398,6 +398,40 @@ static TCGArg do_constant_folding_cond(TCGOpcode op, > TCGArg x, > } > } > > +/* Return 2 if the condition can't be simplified, and the result > + of the condition (0 or 1) if it can */ > +static TCGArg do_constant_folding_cond2(TCGArg *p1, TCGArg *p2, TCGCond c) > +{ > +TCGArg al = p1[0], ah = p1[1]; > +TCGArg bl = p2[0], bh = p2[1]; > + > +if (temps[bl].state == TCG_TEMP_CONST > +&& temps[bh].state == TCG_TEMP_CONST) { > +uint64_t b = ((uint64_t)temps[bh].val << 32) | > (uint32_t)temps[bl].val; > + > +if (temps[al].state == TCG_TEMP_CONST > +&& temps[ah].state == TCG_TEMP_CONST) { > +uint64_t a; > +a = ((uint64_t)temps[ah].val << 32) | (uint32_t)temps[al].val; > +return do_constant_folding_cond_64(a, b, c); > +} > +if (b == 0) { > +switch (c) { > +case TCG_COND_LTU: > +return 0; > +case TCG_COND_GEU: > +return 1; > +default: > +break; > +} > +} > +} > +if (temps_are_copies(al, bl) && temps_are_copies(ah, bh)) { > +return do_constant_folding_cond_eq(c); > +} > +return 2; > +} > + > static bool swap_commutative(TCGArg dest, TCGArg *p1, TCGArg *p2) > { > TCGArg a1 = *p1, a2 = *p2; > @@ -816,53 +850,73 @@ static TCGArg *tcg_constant_folding(TCGContext *s, > uint16_t *tcg_opc_ptr, > args += 6; > break; > case INDEX_op_brcond2_i32: > -/* Simplify LT/GE comparisons vs zero to a single compare > - vs the high word of the input. */ > -if ((args[4] == TCG_COND_LT || args[4] == TCG_COND_GE) > -&& temps[args[2]].state == TCG_TEMP_CONST > -&& temps[args[3]].state == TCG_TEMP_CONST > -&& temps[args[2]].val == 0 > -&& temps[args[2]].val == 0) { > -gen_opc_buf[op_index] = INDEX_op_brcond_i32; > -args[0] = args[1]; > -args[1] = args[3]; > -args[2] = args[4]; > -args[3] = args[5]; > -gen_args += 4; > +tmp = do_constant_folding_cond2(&args[0], &args[2], args[4]); > +if (tmp != 2) { > +if (tmp) { > +gen_opc_buf[op_index] = INDEX_op_br; > +gen_args[0] = args[5]; > +gen_args += 1; > +} else { > +gen_opc_buf[op_index] = INDEX_op_nop; > +args += 6; > +break; > +} > } else { > -gen_args[0] = args[0]; > -gen_args[1] = args[1]; > -gen_args[2] = args[2]; > -gen_args[3] = args[3]; > -gen_args[4] = args[4]; > -gen_args[5] = args[5]; > -gen_args += 6; > +/* Simplify LT/GE comparisons vs zero to a single compare > + vs the high word of the input. */ > +if ((args[4] == TCG_COND_LT || args[4] == TCG_COND_GE) > +&& temps[args[2]].state == TCG_TEMP_CONST > +&& temps[args[3]].state == TCG_TEMP_CONST > +&& temps[args[2]].val == 0 > +&& temps[args[2]].val == 0) { > +gen_opc_buf[op_index] = INDEX_op_brcond_i32; > +args[0] = args[1]; > +args[1] = args[3]; > +args[2] = args[4]; > +args[3] = args[5]; > +gen_args += 4; > +} else { > +gen_args[0] = args[0]; > +gen_args[1] = args[1]; > +gen_args[2] = args[2]; > +gen_args[3] = args[3]; > +gen_args[4] = args[4]; > +gen_args[5] = args[5]; > +gen_args += 6; > +} > } > memset(temps, 0, nb_temps * sizeof(struct tcg_temp_info)); > args += 6; > break; > case INDEX_op_setcond2_i32: > -/* Simplify LT/GE comparisons vs zero to a single compare > - vs the high word of the input. */ > -if ((args[5] == TCG_COND_LT || args[5] == TCG_COND_GE) > -&& temps[args[3]].state == TCG_TEMP_CONST > -&& temps[args[4]].state == TCG_TEMP_CONST > -&& temps[args[3]].val == 0 > -
[Qemu-devel] [PATCH v2] target-xtensa: de-optimize EXTUI
Now that "and" with 0xff, 0x and 0x and "shr" with 0 shift are optimized in tcg/tcg-op.h there is no need to do it in target-xtensa/translate.c. Cc: Max Filippov Signed-off-by: Aurelien Jarno --- target-xtensa/translate.c | 22 ++ 1 file changed, 2 insertions(+), 20 deletions(-) v1 -> v2: also remove the test on shiftimm to select either a shift or a move. diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c index b9acd70..82e8ccc 100644 --- a/target-xtensa/translate.c +++ b/target-xtensa/translate.c @@ -1829,26 +1829,8 @@ static void disas_xtensa_insn(DisasContext *dc) int maskimm = (1 << (OP2 + 1)) - 1; TCGv_i32 tmp = tcg_temp_new_i32(); - -if (shiftimm) { -tcg_gen_shri_i32(tmp, cpu_R[RRR_T], shiftimm); -} else { -tcg_gen_mov_i32(tmp, cpu_R[RRR_T]); -} - -switch (maskimm) { -case 0xff: -tcg_gen_ext8u_i32(cpu_R[RRR_R], tmp); -break; - -case 0x: -tcg_gen_ext16u_i32(cpu_R[RRR_R], tmp); -break; - -default: -tcg_gen_andi_i32(cpu_R[RRR_R], tmp, maskimm); -break; -} +tcg_gen_shri_i32(tmp, cpu_R[RRR_T], shiftimm); +tcg_gen_andi_i32(cpu_R[RRR_R], tmp, maskimm); tcg_temp_free(tmp); } break; -- 1.7.10.4
Re: [Qemu-devel] [PATCH v10] kvm: notify host when the guest is panicked
On Wed, 29 Aug 2012 13:18:54 +0800 Wen Congyang wrote: > We can know the guest is panicked when the guest runs on xen. > But we do not have such feature on kvm. What's the status of this series? It got lost in my queue and I ended up not reviewing it, but it seems to be stuck. > > Another purpose of this feature is: management app(for example: > libvirt) can do auto dump when the guest is panicked. If management > app does not do auto dump, the guest's user can do dump by hand if > he sees the guest is panicked. > > We have three solutions to implement this feature: > 1. use vmcall > 2. use I/O port > 3. use virtio-serial. > > We have decided to avoid touching hypervisor. The reason why I choose > choose the I/O port is: > 1. it is easier to implememt > 2. it does not depend any virtual device > 3. it can work when starting the kernel > > Signed-off-by: Wen Congyang > --- > Documentation/virtual/kvm/pv_event.txt | 32 > > arch/ia64/include/asm/kvm_para.h | 14 ++ > arch/powerpc/include/asm/kvm_para.h| 14 ++ > arch/s390/include/asm/kvm_para.h | 14 ++ > arch/x86/include/asm/kvm_para.h| 27 +++ > arch/x86/kernel/kvm.c | 25 + > include/linux/kvm_para.h | 23 +++ > 7 files changed, 149 insertions(+), 0 deletions(-) > create mode 100644 Documentation/virtual/kvm/pv_event.txt > > diff --git a/Documentation/virtual/kvm/pv_event.txt > b/Documentation/virtual/kvm/pv_event.txt > new file mode 100644 > index 000..bb04de0 > --- /dev/null > +++ b/Documentation/virtual/kvm/pv_event.txt > @@ -0,0 +1,32 @@ > +The KVM paravirtual event interface > += > + > +Initializing the paravirtual event interface > +== > +kvm_pv_event_init() > +Argiments: > + None > + > +Return Value: > + 0: The guest kernel can use paravirtual event interface. > + 1: The guest kernel can't use paravirtual event interface. > + > +Querying whether the event can be ejected > +== > +kvm_pv_has_feature() > +Arguments: > + feature: The bit value of this paravirtual event to query > + > +Return Value: > + 0 : The guest kernel can't eject this paravirtual event. > + -1: The guest kernel can eject this paravirtual event. > + > + > +Ejecting paravirtual event > +== > +kvm_pv_eject_event() > +Arguments: > + event: The event to be ejected. > + > +Return Value: > + None > diff --git a/arch/ia64/include/asm/kvm_para.h > b/arch/ia64/include/asm/kvm_para.h > index 2019cb9..b5ec658 100644 > --- a/arch/ia64/include/asm/kvm_para.h > +++ b/arch/ia64/include/asm/kvm_para.h > @@ -31,6 +31,20 @@ static inline bool kvm_check_and_clear_guest_paused(void) > return false; > } > > +static inline int kvm_arch_pv_event_init(void) > +{ > + return 0; > +} > + > +static inline unsigned int kvm_arch_pv_features(void) > +{ > + return 0; > +} > + > +static inline void kvm_arch_pv_eject_event(unsigned int event) > +{ > +} > + > #endif > > #endif > diff --git a/arch/powerpc/include/asm/kvm_para.h > b/arch/powerpc/include/asm/kvm_para.h > index c18916b..01b98c7 100644 > --- a/arch/powerpc/include/asm/kvm_para.h > +++ b/arch/powerpc/include/asm/kvm_para.h > @@ -211,6 +211,20 @@ static inline bool kvm_check_and_clear_guest_paused(void) > return false; > } > > +static inline int kvm_arch_pv_event_init(void) > +{ > + return 0; > +} > + > +static inline unsigned int kvm_arch_pv_features(void) > +{ > + return 0; > +} > + > +static inline void kvm_arch_pv_eject_event(unsigned int event) > +{ > +} > + > #endif /* __KERNEL__ */ > > #endif /* __POWERPC_KVM_PARA_H__ */ > diff --git a/arch/s390/include/asm/kvm_para.h > b/arch/s390/include/asm/kvm_para.h > index da44867..00ce058 100644 > --- a/arch/s390/include/asm/kvm_para.h > +++ b/arch/s390/include/asm/kvm_para.h > @@ -154,6 +154,20 @@ static inline bool kvm_check_and_clear_guest_paused(void) > return false; > } > > +static inline int kvm_arch_pv_event_init(void) > +{ > + return 0; > +} > + > +static inline unsigned int kvm_arch_pv_features(void) > +{ > + return 0; > +} > + > +static inline void kvm_arch_pv_eject_event(unsigned int event) > +{ > +} > + > #endif > > #endif /* __S390_KVM_PARA_H */ > diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h > index 2f7712e..7d297f0 100644 > --- a/arch/x86/include/asm/kvm_para.h > +++ b/arch/x86/include/asm/kvm_para.h > @@ -96,8 +96,11 @@ struct kvm_vcpu_pv_apf_data { > #define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK > #define KVM_PV_EOI_DISABLED 0x0 > > +#define KVM_PV_EVENT_PORT(0x505UL) > + > #ifdef __KERNEL__ > #include > +#include > > extern void kvmclock_init(void); > extern int kvm_register_clock(char *txt); > @@ -228,6 +231,30 @@ static inline void kvm_d
[Qemu-devel] [PATCH] tcg: remove obsolete jmp op
The TCG jmp operation doesn't really make sense in the QEMU context, it is unused, it is not implemented by some targets, and it is wrongly implemented by some others. This patch simply removes it. Cc: Andrzej Zaborowski Cc: Richard Henderson Cc: Vassili Karpov (malc) Cc: Alexander Graf Cc: Blue Swirl Cc: Stefan Weil Signed-off-by: Aurelien Jarno --- tcg/README |7 +-- tcg/arm/tcg-target.c |7 --- tcg/hppa/tcg-target.c |6 -- tcg/i386/tcg-target.c |9 - tcg/ia64/tcg-target.c |4 tcg/mips/tcg-target.c |5 - tcg/ppc/tcg-target.c | 10 -- tcg/ppc64/tcg-target.c | 10 -- tcg/s390/tcg-target.c |6 -- tcg/sparc/tcg-target.c |2 -- tcg/tcg-opc.h |1 - tcg/tci/tcg-target.c |4 tci.c |1 - 13 files changed, 1 insertion(+), 71 deletions(-) diff --git a/tcg/README b/tcg/README index 27846f1..aa86992 100644 --- a/tcg/README +++ b/tcg/README @@ -88,8 +88,7 @@ supported. * Branches: -Use the instruction 'br' to jump to a label. Use 'jmp' to jump to an -explicit address. Conditional branches can only jump to labels. +Use the instruction 'br' to jump to a label. 3.3) Code Optimizations @@ -129,10 +128,6 @@ call function 'ptr' (pointer type) * Jumps/Labels -* jmp t0 - -Absolute jump to address t0 (pointer type). - * set_label $label Define label 'label' at the current program point. diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c index 2bad0a2..1e61864 100644 --- a/tcg/arm/tcg-target.c +++ b/tcg/arm/tcg-target.c @@ -1530,12 +1530,6 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, else tcg_out_callr(s, COND_AL, args[0]); break; -case INDEX_op_jmp: -if (const_args[0]) -tcg_out_goto(s, COND_AL, args[0]); -else -tcg_out_bx(s, COND_AL, args[0]); -break; case INDEX_op_br: tcg_out_goto_label(s, COND_AL, args[0]); break; @@ -1769,7 +1763,6 @@ static const TCGTargetOpDef arm_op_defs[] = { { INDEX_op_exit_tb, { } }, { INDEX_op_goto_tb, { } }, { INDEX_op_call, { "ri" } }, -{ INDEX_op_jmp, { "ri" } }, { INDEX_op_br, { } }, { INDEX_op_mov_i32, { "r", "r" } }, diff --git a/tcg/hppa/tcg-target.c b/tcg/hppa/tcg-target.c index 2c79c10..44974c4 100644 --- a/tcg/hppa/tcg-target.c +++ b/tcg/hppa/tcg-target.c @@ -1353,11 +1353,6 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args, } break; -case INDEX_op_jmp: -fprintf(stderr, "unimplemented jmp\n"); -tcg_abort(); -break; - case INDEX_op_br: tcg_out_branch(s, args[0], 1); break; @@ -1592,7 +1587,6 @@ static const TCGTargetOpDef hppa_op_defs[] = { { INDEX_op_goto_tb, { } }, { INDEX_op_call, { "ri" } }, -{ INDEX_op_jmp, { "r" } }, { INDEX_op_br, { } }, { INDEX_op_mov_i32, { "r", "r" } }, diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c index 0e218c8..bb2306d 100644 --- a/tcg/i386/tcg-target.c +++ b/tcg/i386/tcg-target.c @@ -1513,14 +1513,6 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, tcg_out_modrm(s, OPC_GRP5, EXT5_CALLN_Ev, args[0]); } break; -case INDEX_op_jmp: -if (const_args[0]) { -tcg_out_jmp(s, args[0]); -} else { -/* jmp *reg */ -tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, args[0]); -} -break; case INDEX_op_br: tcg_out_jxx(s, JCC_JMP, args[0], 0); break; @@ -1848,7 +1840,6 @@ static const TCGTargetOpDef x86_op_defs[] = { { INDEX_op_exit_tb, { } }, { INDEX_op_goto_tb, { } }, { INDEX_op_call, { "ri" } }, -{ INDEX_op_jmp, { "ri" } }, { INDEX_op_br, { } }, { INDEX_op_mov_i32, { "r", "r" } }, { INDEX_op_movi_i32, { "r" } }, diff --git a/tcg/ia64/tcg-target.c b/tcg/ia64/tcg-target.c index dc9c12c..705712f 100644 --- a/tcg/ia64/tcg-target.c +++ b/tcg/ia64/tcg-target.c @@ -1916,9 +1916,6 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, case INDEX_op_goto_tb: tcg_out_goto_tb(s, args[0]); break; -case INDEX_op_jmp: -tcg_out_jmp(s, args[0]); -break; case INDEX_op_movi_i32: tcg_out_movi(s, TCG_TYPE_I32, args[0], args[1]); @@ -2156,7 +2153,6 @@ static const TCGTargetOpDef ia64_op_defs[] = { { INDEX_op_call, { "r" } }, { INDEX_op_exit_tb, { } }, { INDEX_op_goto_tb, { } }, -{ INDEX_op_jmp, { "r" } }, { INDEX_op_mov_i32, { "r", "r" } }, { INDEX_op_movi_i32, { "r" } }, diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c index 34e3e7f..7e4013e 100644 --- a/tcg/mips/tcg-target.c +++ b/tcg/mips/tcg-target.c @@ -1322,10 +1322,6 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, tcg_out_opc_reg(s, OPC_JALR, TCG_REG_RA, args[0]
Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it
"Daniel P. Berrange" writes: > On Mon, Oct 01, 2012 at 06:43:00PM +0200, Andreas Färber wrote: >> Hello Jan, >> >> Am 01.10.2012 16:34, schrieb Jan Kiszka: >> > If we built a target for a host that supports KVM in principle, set the >> > default accelerator to KVM as well. This also means the start of QEMU >> > will fail to start if KVM support turns out to be unavailable at >> > runtime. >> >> From a distro point of view this of course means that we will build >> against KVM and that the new KVM default will start to fail for users on >> very old hardware. Can't we do a runtime check to select the default? > > NB, this is *not* only about old hardware. There are plenty of users who > use QEMU inside VMs. One very common usage I know of is image building > tools which are run inside Amazon VMs, using libguestfs & QEMU. But libguest can set it's accelerator option to whatever it wants. If your running QEMU under a VM, it's pretty reasonable to have to use a special option IMHO. > IMHO, default to KVM, fallback to TCG is the most friendly default > behaviour. Except if a user expects good network performance and can't understand why they're getting 100kb/s. Regards, Anthony Liguori > > Daniel > -- > |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v2 4/9] qemu-sockets: add error propagation to Unix socket functions
Il 01/10/2012 19:17, Luiz Capitulino ha scritto: >> > if (bind(sock, (struct sockaddr*) &un, sizeof(un)) < 0) { >> > -fprintf(stderr, "bind(unix:%s): %s\n", un.sun_path, >> > strerror(errno)); >> > +error_set(errp, QERR_SOCKET_BIND_FAILED); > This drops error information, making the error message worse. I believe > you have a reason to not use error_setg()? I was waiting for the end of the discussion on errno to add error_setg_errno. > Also, I see that in some hunks you do something like: > > -fd = unix_listen_opts(opts); > +fd = unix_listen_opts(opts, NULL); > > This will break printing the error message to the user. It's fine by me if > you do this only temporarily (ie. this is fixed by the next or a later patch), > but want to double check that you're aware of it. I want to avoid super-large patch series, so I would prefer to fix it later in the 1.3 development. Paolo > Btw, I'm making these comments in this hunk but they apply to similar hunks > as well. >
Re: [Qemu-devel] [PATCH] device tree: simplify dumpdtb code
On Sun, Sep 23, 2012 at 11:29:32PM +0200, Alexander Graf wrote: > As per Peter's suggestion, we can use glib to write out a buffer in whole to > a file, simplifying the code dramatically. > > Signed-off-by: Alexander Graf > > --- > > This patch applies on top of the generic dumpdtb remodeling patch. > > --- > device_tree.c |9 + > 1 files changed, 1 insertions(+), 8 deletions(-) > > diff --git a/device_tree.c b/device_tree.c > index 69ca953..a923613 100644 > --- a/device_tree.c > +++ b/device_tree.c > @@ -314,14 +314,7 @@ void qemu_devtree_dumpdtb(void *fdt, int size) > const char *dumpdtb = qemu_opt_get(machine_opts, "dumpdtb"); > if (dumpdtb) { > /* Dump the dtb to a file and quit */ > -FILE *f = fopen(dumpdtb, "wb"); > -size_t len; > -len = fwrite(fdt, size, 1, f); > -fclose(f); > -if (len != size) { > -exit(1); > -} > -exit(0); > +exit(g_file_set_contents(dumpdtb, fdt, size, NULL) ? 0 : 1); > } > } > Reviewed-by: Aurelien Jarno -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it
> But libguest can set it's accelerator option to whatever it wants. > > If your running QEMU under a VM, it's pretty reasonable to have to > use a special option IMHO. It's also reasonable to have consecutive releases change defaults in a more "friendly" way (i.e. from tcg to kvm:tcg), especially since we'll get users that formerly used qemu-kvm and never had to specify neither -machine accel nor --enable-kvm. > > IMHO, default to KVM, fallback to TCG is the most friendly default > > behaviour. > > Except if a user expects good network performance and can't > understand why they're getting 100kb/s. libguestfs doesn't need network at all (though I wonder if they could use lxc instead...). Paolo
Re: [Qemu-devel] [PATCH] vfio_pci: fix build on 32-bit systems
Anthony Liguori writes: > We cannot cast directly from pointer to uint64. > > Cc: Alex Williamson > Cc: Alex Barcelo > Reported-by: Alex Barcelo > Signed-off-by: Anthony Liguori Applied. Regards, Anthony Liguori > --- > hw/vfio_pci.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c > index a24558a..a1eeced 100644 > --- a/hw/vfio_pci.c > +++ b/hw/vfio_pci.c > @@ -768,7 +768,7 @@ static int vfio_dma_map(VFIOContainer *container, > target_phys_addr_t iova, > struct vfio_iommu_type1_dma_map map = { > .argsz = sizeof(map), > .flags = VFIO_DMA_MAP_FLAG_READ, > -.vaddr = (__u64)vaddr, > +.vaddr = (__u64)(intptr_t)vaddr, > .iova = iova, > .size = size, > }; > -- > 1.7.5.4
Re: [Qemu-devel] [PATCH v10] kvm: notify host when the guest is panicked
On Mon, Oct 01, 2012 at 03:57:40PM -0300, Luiz Capitulino wrote: > On Wed, 29 Aug 2012 13:18:54 +0800 > Wen Congyang wrote: > > > We can know the guest is panicked when the guest runs on xen. > > But we do not have such feature on kvm. > > What's the status of this series? > > It got lost in my queue and I ended up not reviewing it, but it seems > to be stuck. > > > > > Another purpose of this feature is: management app(for example: > > libvirt) can do auto dump when the guest is panicked. If management > > app does not do auto dump, the guest's user can do dump by hand if > > he sees the guest is panicked. > > > > We have three solutions to implement this feature: > > 1. use vmcall > > 2. use I/O port > > 3. use virtio-serial. > > > > We have decided to avoid touching hypervisor. The reason why I choose > > choose the I/O port is: > > 1. it is easier to implememt > > 2. it does not depend any virtual device > > 3. it can work when starting the kernel Have you tried to move it early enough to minimize the number of calls to panic()? Say, how many panics can occur between first Linux instruction and initialization of netconsole? Because this interface (all the way to QEMU) is going to be reimplemented with different code later, in case other architectures want similar interface, which makes maintenance more difficult. Using PCI would basically replacing I/O port write with PCI write (or virtio write). Avi? > > Signed-off-by: Wen Congyang > > --- > > Documentation/virtual/kvm/pv_event.txt | 32 > > > > arch/ia64/include/asm/kvm_para.h | 14 ++ > > arch/powerpc/include/asm/kvm_para.h| 14 ++ > > arch/s390/include/asm/kvm_para.h | 14 ++ > > arch/x86/include/asm/kvm_para.h| 27 +++ > > arch/x86/kernel/kvm.c | 25 + > > include/linux/kvm_para.h | 23 +++ > > 7 files changed, 149 insertions(+), 0 deletions(-) > > create mode 100644 Documentation/virtual/kvm/pv_event.txt > > > > diff --git a/Documentation/virtual/kvm/pv_event.txt > > b/Documentation/virtual/kvm/pv_event.txt > > new file mode 100644 > > index 000..bb04de0 > > --- /dev/null > > +++ b/Documentation/virtual/kvm/pv_event.txt > > @@ -0,0 +1,32 @@ > > +The KVM paravirtual event interface > > += > > + > > +Initializing the paravirtual event interface > > +== > > +kvm_pv_event_init() > > +Argiments: > > + None > > + > > +Return Value: > > + 0: The guest kernel can use paravirtual event interface. > > + 1: The guest kernel can't use paravirtual event interface. > > + > > +Querying whether the event can be ejected > > +== > > +kvm_pv_has_feature() > > +Arguments: > > + feature: The bit value of this paravirtual event to query > > + > > +Return Value: > > + 0 : The guest kernel can't eject this paravirtual event. > > + -1: The guest kernel can eject this paravirtual event. > > + > > + > > +Ejecting paravirtual event > > +== > > +kvm_pv_eject_event() > > +Arguments: > > + event: The event to be ejected. > > + > > +Return Value: > > + None > > diff --git a/arch/ia64/include/asm/kvm_para.h > > b/arch/ia64/include/asm/kvm_para.h > > index 2019cb9..b5ec658 100644 > > --- a/arch/ia64/include/asm/kvm_para.h > > +++ b/arch/ia64/include/asm/kvm_para.h > > @@ -31,6 +31,20 @@ static inline bool kvm_check_and_clear_guest_paused(void) > > return false; > > } > > > > +static inline int kvm_arch_pv_event_init(void) > > +{ > > + return 0; > > +} > > + > > +static inline unsigned int kvm_arch_pv_features(void) > > +{ > > + return 0; > > +} > > + > > +static inline void kvm_arch_pv_eject_event(unsigned int event) > > +{ > > +} > > + > > #endif > > > > #endif > > diff --git a/arch/powerpc/include/asm/kvm_para.h > > b/arch/powerpc/include/asm/kvm_para.h > > index c18916b..01b98c7 100644 > > --- a/arch/powerpc/include/asm/kvm_para.h > > +++ b/arch/powerpc/include/asm/kvm_para.h > > @@ -211,6 +211,20 @@ static inline bool > > kvm_check_and_clear_guest_paused(void) > > return false; > > } > > > > +static inline int kvm_arch_pv_event_init(void) > > +{ > > + return 0; > > +} > > + > > +static inline unsigned int kvm_arch_pv_features(void) > > +{ > > + return 0; > > +} > > + > > +static inline void kvm_arch_pv_eject_event(unsigned int event) > > +{ > > +} > > + > > #endif /* __KERNEL__ */ > > > > #endif /* __POWERPC_KVM_PARA_H__ */ > > diff --git a/arch/s390/include/asm/kvm_para.h > > b/arch/s390/include/asm/kvm_para.h > > index da44867..00ce058 100644 > > --- a/arch/s390/include/asm/kvm_para.h > > +++ b/arch/s390/include/asm/kvm_para.h > > @@ -154,6 +154,20 @@ static inline bool > > kvm_check_and_clear_guest_paused(void) > > return false; > > } > > > > +static inline int kvm_arch_pv_event_init(void) > > +{ >
[Qemu-devel] KVM call agenda for 2012-10-01
hi Please send in any agenda topics you are interested in. Later, Juan.
Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it
Paolo Bonzini writes: >> But libguest can set it's accelerator option to whatever it wants. >> >> If your running QEMU under a VM, it's pretty reasonable to have to >> use a special option IMHO. > > It's also reasonable to have consecutive releases change defaults in > a more "friendly" way (i.e. from tcg to kvm:tcg), especially since > we'll get users that formerly used qemu-kvm and never had to specify > neither -machine accel nor --enable-kvm. I agree with you except for the 'kvm:tcg' part. > >> > IMHO, default to KVM, fallback to TCG is the most friendly default >> > behaviour. >> >> Except if a user expects good network performance and can't >> understand why they're getting 100kb/s. > > libguestfs doesn't need network at all (though I wonder if they could > use lxc instead...). FWIW, libguestfs already uses -machine accel=kvm:tcg so we can completely the libguestfs use-case for this discussion. Regards, Anthony Liguori > > Paolo
Re: [Qemu-devel] [PATCH] fpu/softfloat.c: Return correctly signed values from uint64_to_float32
On Fri, Sep 28, 2012 at 04:17:03PM +0100, Peter Maydell wrote: > The uint64_to_float32() conversion function was incorrectly always > returning numbers with the sign bit set (ie negative numbers). Correct > this so we return positive numbers instead. > > Signed-off-by: Peter Maydell > --- > As far as I can see we use this function only in the three PPC SPE > insns efscfuf, efsctsf, efsctuf. It is therefore untested(!); if > anybody with PPC hw to test against could check the results of > those functions that would be cool. > > fpu/softfloat.c |4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index b29256a..91497e8 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -1238,7 +1238,7 @@ float32 uint64_to_float32( uint64 a STATUS_PARAM ) > if ( a == 0 ) return float32_zero; > shiftCount = countLeadingZeros64( a ) - 40; > if ( 0 <= shiftCount ) { > -return packFloat32( 1 > 0, 0x95 - shiftCount, a< +return packFloat32(0, 0x95 - shiftCount, a< } > else { > shiftCount += 7; > @@ -1248,7 +1248,7 @@ float32 uint64_to_float32( uint64 a STATUS_PARAM ) > else { > a <<= shiftCount; > } > -return roundAndPackFloat32( 1 > 0, 0x9C - shiftCount, a STATUS_VAR ); > +return roundAndPackFloat32(0, 0x9C - shiftCount, a STATUS_VAR); > } > } > Thanks, applied. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] fpu/softfloat.c: Remove pointless shift of always-zero value
On Mon, Sep 24, 2012 at 05:28:35PM +0100, Peter Maydell wrote: > In float16_to_float32, when returning an infinity, just pass zero > as the mantissa argument to packFloat32(), rather than shifting > a value which we know must be zero. > > Signed-off-by: Peter Maydell > --- > Spotted by the clang static analyzer. This brings this code into line with > the other float-to-float conversion functions and was probably a harmless > cut-n-paste error from the normal-return codepath. > > fpu/softfloat.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index b29256a..01a28ca 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -3007,7 +3007,7 @@ float32 float16_to_float32(float16 a, flag ieee > STATUS_PARAM) > if (aSig) { > return commonNaNToFloat32(float16ToCommonNaN(a STATUS_VAR) > STATUS_VAR); > } > -return packFloat32(aSign, 0xff, aSig << 13); > +return packFloat32(aSign, 0xff, 0); > } > if (aExp == 0) { > int8 shiftCount; Thanks, applied. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH v2] target-xtensa: de-optimize EXTUI
On Mon, Oct 1, 2012 at 10:54 PM, Aurelien Jarno wrote: > Now that "and" with 0xff, 0x and 0x and "shr" with 0 shift > are optimized in tcg/tcg-op.h there is no need to do it in > target-xtensa/translate.c. > > Cc: Max Filippov > Signed-off-by: Aurelien Jarno > --- > target-xtensa/translate.c | 22 ++ > 1 file changed, 2 insertions(+), 20 deletions(-) > > v1 -> v2: also remove the test on shiftimm to select either a shift or > a move. Acked-by: Max Filippov -- Thanks. -- Max
Re: [Qemu-devel] KVM call agenda for 2012-10-01
Juan Quintela writes: > hi > > Please send in any agenda topics you are interested in. 1) TODO to finish off qemu-kvm.git/master... Regards, Anthony Liguori > > Later, Juan.
Re: [Qemu-devel] [PATCH] tcg: remove obsolete jmp op
On 2012-10-01 12:00, Aurelien Jarno wrote: > The TCG jmp operation doesn't really make sense in the QEMU context, it > is unused, it is not implemented by some targets, and it is wrongly > implemented by some others. > > This patch simply removes it. > > Cc: Andrzej Zaborowski > Cc: Richard Henderson > Cc: Vassili Karpov (malc) > Cc: Alexander Graf > Cc: Blue Swirl > Cc: Stefan Weil > Signed-off-by: Aurelien Jarno Yaye. I posted a patch to do this 2 years ago and it got missed. Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH] qtest: implement QTEST_STOP
Paolo Bonzini writes: > It is quite difficult to debug qtest test cases without extra wrapper > scripts for QEMU or similar. This patch adds a simple environment > variable-based trigger that sends a STOP signal to the QEMU instance > under test, before attempting to connect to its QMP session. > > This will block execution of the testcase and give time to attach a > debugger to the stopped QEMU process. > > Signed-off-by: Paolo Bonzini > --- > tests/libqtest.c | 38 +- > 1 file modificato, 25 inserzioni(+), 13 rimozioni(-) > > diff --git a/tests/libqtest.c b/tests/libqtest.c > index 02d0392..71b84c1 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -85,6 +85,22 @@ static int socket_accept(int sock) > return ret; > } > > +static pid_t qtest_qemu_pid(QTestState *s) > +{ > +FILE *f; > +char buffer[1024]; > +pid_t pid = -1; > + > +f = fopen(s->pid_file, "r"); > +if (f) { > +if (fgets(buffer, sizeof(buffer), f)) { > +pid = atoi(buffer); > +} > +} > +fclose(f); > +return pid; > +} > + > QTestState *qtest_init(const char *extra_args) > { > QTestState *s; > @@ -136,25 +152,21 @@ QTestState *qtest_init(const char *extra_args) > qtest_qmp(s, ""); > qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"); > > +if (getenv("QTEST_STOP")) { > +kill(qtest_qemu_pid(s), SIGSTOP); > +} > + What about launching the guest with "-S" if that variable is set? That's a bit nicer, right? Regards, Anthony Liguori > return s; > } > > void qtest_quit(QTestState *s) > { > -FILE *f; > -char buffer[1024]; > - > -f = fopen(s->pid_file, "r"); > -if (f) { > -if (fgets(buffer, sizeof(buffer), f)) { > -pid_t pid = atoi(buffer); > -int status = 0; > - > -kill(pid, SIGTERM); > -waitpid(pid, &status, 0); > -} > +int status; > > -fclose(f); > +pid_t pid = qtest_qemu_pid(s); > +if (pid != -1) { > +kill(pid, SIGTERM); > +waitpid(pid, &status, 0); > } > > unlink(s->pid_file); > -- > 1.7.12
Re: [Qemu-devel] [PATCH] Add option to mlock guest and qemu memory
Hi Jan, Thank you for reviewing. On 09/28/2012 04:05 AM, Jan Kiszka wrote: > On 2012-09-28 01:21, Satoru Moriya wrote: >> We have some plans to migrate old enterprise systems which require >> low latency (msec order) to kvm virtualized environment. Usually, >> we uses mlock to preallocate and pin down process memory in order >> to avoid page allocation in latency critical path. On the other >> hand, in kvm environment, mlocking in guests is not effective >> because it can't avoid page reclaim in host. Actually, to avoid >> guest memory reclaim, qemu has "mem-path" option that is actually >> for using hugepage. But a memory region of qemu is not allocated >> on hugepage, so it may be reclaimed. That may cause a latency >> problem. >> >> To avoid guest and qemu memory reclaim, this patch introduces >> a new "mlock" option. With this option, we can preallocate and >> pin down guest and qemu memory before booting guest OS. > > I guess this reduces the likeliness of multi-millisecond latencies for > you but not eliminate them. Of course, mlockall is part of our local > changes for real-time QEMU/KVM, but it is just one of the many pieces > required. I'm wondering how the situation is on your side. You're right. I think this is a first step toward solving latency issue on qemu/kvm. > I think mlockall should once be enabled automatically as soon as you ask > for real-time support for QEMU guests. How that should be controlled is > another question. I'm currently carrying a top-level switch "-rt > maxprio=x[,policy=y]" here, likely not the final solution. I'm not Could you please tell me what that option actually do? Do you have any public repositories or something for me to look at your real-time qemu/kvm changes? > really convinced we need to control memory locking separately. And as we > are very reluctant to add new top-level switches, this is even more > important. Regards, Satoru