Re: [RFC/PATCH 3/3] x86/signal/64: Add explicit controls for sigcontext SS handling
On Fri, Aug 14, 2015 at 01:57:42PM -0700, Andy Lutomirski wrote: > > Don't bother testing yet. I'm waffling between trying something like > this and adding SA_SAVE_SS. I have partially written patches for the > latter. ok, ping me if anything -- 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: [RFC/PATCH 3/3] x86/signal/64: Add explicit controls for sigcontext SS handling
On Thu, Aug 13, 2015 at 01:18:50PM -0700, Andy Lutomirski wrote: > This adds two new uc_flags flags. UC_SAVED_SS will be set for all > 64-bit signals (including x32). It indicates that the saved SS field > is valid and that the kernel understands UC_RESTORE_SS. > > The kernel will *not* set UC_RESTORE_SS. User signal handlers can > set UC_RESTORE_SS themselves to indicate that sigreturn should > restore SS from the sigcontext. > > 64-bit programs that use segmentation are encouraged to check > UC_SAVED_SS and set UC_RESTORE_SS in their signal handlers. This is > the only straightforward way to cause sigreturn to restore SS. (The > only non-test program that I know of that uses segmentation in a > 64-bit binary is DOSEMU, and DOSEMU currently uses a nasty > trampoline to work around the lack of this mechanism in old kernels. > It could detect UC_RESTORE_SS and use it to avoid needing a > trampoline. > > Cc: Stas Sergeev > Cc: Linus Torvalds > Cc: Cyrill Gorcunov > Cc: Pavel Emelyanov > Signed-off-by: Andy Lutomirski Looks reasonable to me. Andy, Linus, what the final conclusion -- are we about to introduce this flag or simply continue with revert? Should I test this one? (from the code I don't excpect it break criu anyhow but still). -- 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: [patch 2/2] tools: lkvm - Filter out cpu vendor string
On Thu, Jun 06, 2013 at 03:03:03PM +0300, Pekka Enberg wrote: > > /* Set X86_FEATURE_HYPERVISOR */ > > if (entry->index == 0) > > Ping! Is there someone out there who has a AMD box they could test this on? I don't have it, sorry :-( -- 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
[patch 2/2] tools: lkvm - Filter out cpu vendor string
If cpuvendor string is not filetered in case of host
amd machine we get unhandled msr reads
| [1709265.368464] kvm: 25706: cpu6 unhandled rdmsr: 0xc0010048
| [1709265.397161] kvm: 25706: cpu7 unhandled rdmsr: 0xc0010048
| [1709265.425774] kvm: 25706: cpu8 unhandled rdmsr: 0xc0010048
thus provide own string and kernel will use generic cpu init.
Reported-by: Ingo Molnar
CC: Pekka Enberg
CC: Sasha Levin
CC: Asias He
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/x86/cpuid.c |8
1 file changed, 8 insertions(+)
Index: linux-2.6.git/tools/kvm/x86/cpuid.c
===
--- linux-2.6.git.orig/tools/kvm/x86/cpuid.c
+++ linux-2.6.git/tools/kvm/x86/cpuid.c
@@ -12,6 +12,7 @@
static void filter_cpuid(struct kvm_cpuid2 *kvm_cpuid)
{
+ unsigned int signature[3];
unsigned int i;
/*
@@ -21,6 +22,13 @@ static void filter_cpuid(struct kvm_cpui
struct kvm_cpuid_entry2 *entry = &kvm_cpuid->entries[i];
switch (entry->function) {
+ case 0:
+ /* Vendor name */
+ memcpy(signature, "LKVMLKVMLKVM", 12);
+ entry->ebx = signature[0];
+ entry->ecx = signature[1];
+ entry->edx = signature[2];
+ break;
case 1:
/* Set X86_FEATURE_HYPERVISOR */
if (entry->index == 0)
--
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
[patch 1/2] tools: lkvm - Dont generate deps and verion file on clean target
This is redundant since we're to remove them right after being generated. CC: Ingo Molnar CC: Pekka Enberg CC: Sasha Levin CC: Asias He Signed-off-by: Cyrill Gorcunov --- tools/kvm/Makefile | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) Index: linux-2.6.git/tools/kvm/Makefile === --- linux-2.6.git.orig/tools/kvm/Makefile +++ linux-2.6.git/tools/kvm/Makefile @@ -324,10 +324,6 @@ all: arch_support_check $(PROGRAM) $(PRO arch_support_check: $(UNSUPP_ERR) -KVMTOOLS-VERSION-FILE: - @$(SHELL_PATH) util/KVMTOOLS-VERSION-GEN $(OUTPUT) --include $(OUTPUT)KVMTOOLS-VERSION-FILE - # When building -static all objects are built with appropriate flags, which # may differ between static & dynamic .o. The objects are separated into # .o and .static.o. See the %.o: %.c rules below. @@ -504,5 +500,12 @@ cscope: $(Q) $(CSCOPE) -bkqu .PHONY: cscope -# Deps +# +# Escape redundant work on cleaning up +ifneq ($(MAKECMDGOALS),clean) -include $(DEPS) + +KVMTOOLS-VERSION-FILE: + @$(SHELL_PATH) util/KVMTOOLS-VERSION-GEN $(OUTPUT) +-include $(OUTPUT)KVMTOOLS-VERSION-FILE +endif -- 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
[patch 0/2] A few fixes for make and cpuid
Hi guys, here are a couple of fixes for "make clean" and cpuid filtering. Please give them a shot. -- 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: kvm tools: why SDL window does not exit on "poweroff"?
On Fri, Apr 12, 2013 at 12:17:58PM +0300, Pekka Enberg wrote: > On Fri, Apr 12, 2013 at 10:42 AM, Lin Ming wrote: > > I run "poweroff" or "halt" in SDL window, but the window does not exit > > although guest is already halted. But qemu can exit properly. > > > > Is it because "hlt" instruction is not emulated? > > Or other reason? > > > > Any hint to fix it? > > IIRC, poweroff is supposed to happen via some dedicated ioport on x86. > Cyrill, didn't you look into this at some point? Hi guys! Not yet :-) Will report once I manage to. -- 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: [BUG] lkvm crash on crashkernel boot
On Fri, Oct 26, 2012 at 06:31:00PM +0300, Pekka Enberg wrote: > On Thu, 25 Oct 2012, Sasha Levin wrote: > > I think we're seeing that because we don't handle VIRTIO_MSI_NO_VECTOR > > properly. > > > > We need to deal with the ability to remove GSI & friends as well. I've > > added it to my workqueue (unless someone deals with it first). > > Any reason I shouldn't apply Kirill's patch before someone find the time > to do that? I think it's worth to apply until proper fix appear. -- 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: [PATCH 2/2] kvm tools: Bring SMP back
On Wed, Sep 12, 2012 at 11:03:36PM +0800, Asias He wrote: > We have this currently: > >kvm__init() > kvm__arch_setup_firmware() > mptable__init() > using kvm->nrcpus > >kvm_cpu__init() > kvm->nrcpus = kvm->cfg.nrcpus > > kvm->nrcpus is used in mptable__init() before it is initialized, so > mptable__init() will setup a wrong mp table. > > Before: > $ ./lkvm -c 4 > $ cat /proc/cpuinfo |grep ^processor|wc -l > 1 > > After: > $ ./lkvm -c 4 > $ cat /proc/cpuinfo |grep ^processor|wc -l > 4 > > Signed-off-by: Asias He Good catch! -- 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: [PATCH 5/7] kvm tools: enable LTO
On Thu, Aug 30, 2012 at 10:33:21AM +0200, Sasha Levin wrote: > >>> > >>> Ingo, any objections to this? > >> > >> No objections if you can live with a 2x-4x increase in build > >> time - at worst it might cause funnies with the BIOS linker > >> script and such. > > > > Maybe we could enable it via some make option? > > Say make LTO=1 or something? > > Build time went from 6 sec to 14, I don't think it's that significant... At moment. But I bet lkvm will grow with time and build time increase as well. Still if you think we better stick with LTO, no problem, lets do it then ;) Cyrill -- 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: [PATCH 5/7] kvm tools: enable LTO
On Thu, Aug 30, 2012 at 10:16:54AM +0200, Ingo Molnar wrote: > > * Pekka Enberg wrote: > > > On Thu, Aug 30, 2012 at 10:36 AM, Sasha Levin > > wrote: > > > Build with -flto set, which should enable link-time-optimizations. > > > > > > I'm not sure if it provides a significant performance increase, but > > > it's probably just worth it for catching issues which it may cause. > > > > > > Signed-off-by: Sasha Levin > > > > Ingo, any objections to this? > > No objections if you can live with a 2x-4x increase in build > time - at worst it might cause funnies with the BIOS linker > script and such. Maybe we could enable it via some make option? Say make LTO=1 or something? Cyrill -- 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: [PATCH 1/7] kvm tools: add HOME env var to hostfs
On Thu, Aug 30, 2012 at 09:36:37AM +0200, Sasha Levin wrote:
> + char *new_env[] = { "TERM=linux", "DISPLAY=192.168.33.1:0",
> + "HOME=/virt/home", NULL };
> +
> + mkdir("/virt/home", 0755);
Please add check for mkdir error code. Frankly, this is a bad habbit
to assume that mkdir never fails (this could be done on top of this
series I think but should not be leaved without attention).
Cyrill
--
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: perf uncore & lkvm woes
On Thu, Aug 16, 2012 at 11:45:52AM +0300, Avi Kivity wrote: > >> On Thu, Aug 16, 2012 at 10:38 AM, Yan, Zheng > >> wrote: > >> > The Intel uncore doc does not specify how to check if uncore exist. > >> > How about disabling uncore on virtualized CPU? > >> > >> (CC'ing Avi.) > > > > Why not simply add bootline option for that? Would it be acceptible? > > Most users just install a distro, they don't mess with kernel command lines. The command line option might be added implicitly in qemu/lkvm. -- 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: perf uncore & lkvm woes
On Thu, Aug 16, 2012 at 10:41:53AM +0300, Pekka Enberg wrote: > On 08/16/2012 03:19 PM, Peter Zijlstra wrote: > >> On Thu, 2012-08-16 at 10:01 +0300, Pekka Enberg wrote: > >>> Has anyone seen this? It's kvmtool/next with 3.6.0-rc1. Looks like we > >>> are doing uncore_init() on virtualized CPU which breaks boot. > >> > >> I think you're the first.. I don't normally use kvm if I can at all > >> avoid it. > >> > >> But I think its a 'simple' matter of kvm not emulating the entire > >> hardware. Afaik the uncore isn't enumerated and we simply assume MSR > >> presence based on cpu model. > > On Thu, Aug 16, 2012 at 10:38 AM, Yan, Zheng > wrote: > > The Intel uncore doc does not specify how to check if uncore exist. > > How about disabling uncore on virtualized CPU? > > (CC'ing Avi.) Why not simply add bootline option for that? Would it be acceptible? Cyrill -- 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: perf uncore & lkvm woes
On Thu, Aug 16, 2012 at 11:07:43AM +0400, Cyrill Gorcunov wrote: > On Thu, Aug 16, 2012 at 10:01:58AM +0300, Pekka Enberg wrote: > > Hello, > > [0.248962] Pid: 0, comm: swapper/0 Not tainted 3.6.0-rc1+ #24 > > [penberg@tux ~]$ cat perf-kvmtool-issue > > Hello, > > > > Has anyone seen this? It's kvmtool/next with 3.6.0-rc1. Looks like we > > are doing uncore_init() on virtualized CPU which breaks boot. > > Hi, I guess some cpuid/msr bit is not cleared again ;) I'll take a look > once time permit. If only I'm not missing something we've two options 1) either tune up cpu model via cpuid interception in lkvm (which is bad I think) 2) provide some new kernel boot line option to not use unboxed pmu. Cyrill -- 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: perf uncore & lkvm woes
On Thu, Aug 16, 2012 at 10:01:58AM +0300, Pekka Enberg wrote: > Hello, > [0.248962] Pid: 0, comm: swapper/0 Not tainted 3.6.0-rc1+ #24 > [penberg@tux ~]$ cat perf-kvmtool-issue > Hello, > > Has anyone seen this? It's kvmtool/next with 3.6.0-rc1. Looks like we > are doing uncore_init() on virtualized CPU which breaks boot. Hi, I guess some cpuid/msr bit is not cleared again ;) I'll take a look once time permit. Cyrill -- 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: [PATCH 1/2] kvm tools: set the HYPERVISOR flag in cpuid
On Wed, Jun 20, 2012 at 10:23:06AM +0300, Pekka Enberg wrote:
> On Fri, 15 Jun 2012, Cyrill Gorcunov wrote:
>
> > On Fri, Jun 15, 2012 at 01:34:16PM +0200, Sasha Levin wrote:
> > > We need to set the HYPERVISOR flag to let the kernel know we're running
> > > under a hypervisor.
> > >
> > > This makes the kernel enable all sorts of para-virtualization options
> > > such as kvm-clock.
> > >
> > > Signed-off-by: Sasha Levin
> >
> > OK, looks good, but please Sasha, add a comment into the code
> > itself about the bitflag enabled (or maybe Pekka would add at
> > merge time).
>
> Sasha?
I think it should be something like below
Cyrill
---
From: Sasha Levin
Subject: [PATCH] kvm tools: set the HYPERVISOR flag in cpuid
We need to set the HYPERVISOR flag to let the kernel know we're running
under a hypervisor.
This makes the kernel enable all sorts of para-virtualization options
such as kvm-clock.
Signed-off-by: Sasha Levin
[gorcunov@: Add comments on bits]
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/x86/cpuid.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
Index: linux-2.6.git/tools/kvm/x86/cpuid.c
===
--- linux-2.6.git.orig/tools/kvm/x86/cpuid.c
+++ linux-2.6.git/tools/kvm/x86/cpuid.c
@@ -21,8 +21,13 @@ static void filter_cpuid(struct kvm_cpui
struct kvm_cpuid_entry2 *entry = &kvm_cpuid->entries[i];
switch (entry->function) {
+ case 1:
+ /* Set X86_FEATURE_HYPERVISOR */
+ if (entry->index == 0)
+ entry->ecx |= (1 << 31);
+ break;
case 6:
- /* Clear presence of IA32_ENERGY_PERF_BIAS */
+ /* Clear X86_FEATURE_EPB */
entry->ecx = entry->ecx & ~(1 << 3);
break;
case CPUID_FUNC_PERFMON:
--
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: [PATCH 1/2] kvm tools: set the HYPERVISOR flag in cpuid
On Fri, Jun 15, 2012 at 01:34:16PM +0200, Sasha Levin wrote: > We need to set the HYPERVISOR flag to let the kernel know we're running > under a hypervisor. > > This makes the kernel enable all sorts of para-virtualization options > such as kvm-clock. > > Signed-off-by: Sasha Levin OK, looks good, but please Sasha, add a comment into the code itself about the bitflag enabled (or maybe Pekka would add at merge time). Cyrill -- 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
kvm tool: Use safe string hanlding functions
From: Cyrill Gorcunov
Use str[n|l] functions to make sure destination is
not overflowed.
Seems socket path generation should be moved into
a separate helper, but it's for another patch.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/include/kvm/strbuf.h |1 +
tools/kvm/kvm.c| 18 --
tools/kvm/util/strbuf.c| 23 +++
3 files changed, 36 insertions(+), 6 deletions(-)
Index: linux-2.6.git/tools/kvm/include/kvm/strbuf.h
===
--- linux-2.6.git.orig/tools/kvm/include/kvm/strbuf.h
+++ linux-2.6.git/tools/kvm/include/kvm/strbuf.h
@@ -7,6 +7,7 @@
int prefixcmp(const char *str, const char *prefix);
extern size_t strlcat(char *dest, const char *src, size_t count);
+extern size_t strlcpy(char *dest, const char *src, size_t size);
/* some inline functions */
Index: linux-2.6.git/tools/kvm/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/kvm.c
+++ linux-2.6.git/tools/kvm/kvm.c
@@ -1,6 +1,7 @@
#include "kvm/kvm.h"
#include "kvm/read-write.h"
#include "kvm/util.h"
+#include "kvm/strbuf.h"
#include "kvm/mutex.h"
#include "kvm/kvm-cpu.h"
#include "kvm/kvm-ipc.h"
@@ -142,11 +143,14 @@ static int kvm__create_socket(struct kvm
struct sockaddr_un local;
int len, r;
+ /* This usually 108 bytes long */
+ BUILD_BUG_ON(sizeof(local.sun_path) < 32);
+
if (!kvm->name)
return -EINVAL;
- sprintf(full_name, "%s/%s%s", kvm__get_dir(), kvm->name,
- KVM_SOCK_SUFFIX);
+ snprintf(full_name, sizeof(full_name), "%s/%s%s",
+kvm__get_dir(), kvm->name, KVM_SOCK_SUFFIX);
if (access(full_name, F_OK) == 0) {
pr_err("Socket file %s already exist", full_name);
return -EEXIST;
@@ -156,7 +160,7 @@ static int kvm__create_socket(struct kvm
if (s < 0)
return s;
local.sun_family = AF_UNIX;
- strcpy(local.sun_path, full_name);
+ strlcpy(local.sun_path, full_name, sizeof(local.sun_path));
len = strlen(local.sun_path) + sizeof(local.sun_family);
r = bind(s, (struct sockaddr *)&local, len);
if (r < 0)
@@ -177,7 +181,8 @@ void kvm__remove_socket(const char *name
{
char full_name[PATH_MAX];
- sprintf(full_name, "%s/%s%s", kvm__get_dir(), name, KVM_SOCK_SUFFIX);
+ snprintf(full_name, sizeof(full_name), "%s/%s%s",
+kvm__get_dir(), name, KVM_SOCK_SUFFIX);
unlink(full_name);
}
@@ -187,11 +192,12 @@ int kvm__get_sock_by_instance(const char
char sock_file[PATH_MAX];
struct sockaddr_un local;
- sprintf(sock_file, "%s/%s%s", kvm__get_dir(), name, KVM_SOCK_SUFFIX);
+ snprintf(sock_file, sizeof(sock_file), "%s/%s%s",
+kvm__get_dir(), name, KVM_SOCK_SUFFIX);
s = socket(AF_UNIX, SOCK_STREAM, 0);
local.sun_family = AF_UNIX;
- strcpy(local.sun_path, sock_file);
+ strlcpy(local.sun_path, sock_file, sizeof(local.sun_path));
len = strlen(local.sun_path) + sizeof(local.sun_family);
r = connect(s, &local, len);
Index: linux-2.6.git/tools/kvm/util/strbuf.c
===
--- linux-2.6.git.orig/tools/kvm/util/strbuf.c
+++ linux-2.6.git/tools/kvm/util/strbuf.c
@@ -37,3 +37,26 @@ size_t strlcat(char *dest, const char *s
return res;
}
+
+/**
+ * strlcpy - Copy a %NUL terminated string into a sized buffer
+ * @dest: Where to copy the string to
+ * @src: Where to copy the string from
+ * @size: size of destination buffer
+ *
+ * Compatible with *BSD: the result is always a valid
+ * NUL-terminated string that fits in the buffer (unless,
+ * of course, the buffer size is zero). It does not pad
+ * out the result like strncpy() does.
+ */
+size_t strlcpy(char *dest, const char *src, size_t size)
+{
+ size_t ret = strlen(src);
+
+ if (size) {
+ size_t len = (ret >= size) ? size - 1 : ret;
+ memcpy(dest, src, len);
+ dest[len] = '\0';
+ }
+ return ret;
+}
--
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: [PATCH] kvm tools: Process virito blk requests in separate thread
On Tue, Jun 05, 2012 at 08:47:17AM +0800, Asias He wrote: > > I must admit I don't understand this code ;) The data get read into > > stack variable forever? > > The data we read itself is not interesting at all. virtio_blk_thread() > sleeps on the eventfd bdev->io_efd until notify_vq() writes something > to wake it up. Ah, thanks for explanation, Asias. Cyrill -- 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: [PATCH] kvm tools: Process virito blk requests in separate thread
On Mon, Jun 04, 2012 at 11:40:53PM +0800, Asias He wrote:
>
> +static void *virtio_blk_thread(void *dev)
> +{
> + struct blk_dev *bdev = dev;
> + u64 data;
> +
> + while (1) {
> + read(bdev->io_efd, &data, sizeof(u64));
> + virtio_blk_do_io(bdev->kvm, &bdev->vqs[0], bdev);
> + }
> +
> + pthread_exit(NULL);
> + return NULL;
> +}
I must admit I don't understand this code ;) The data get read into
stack variable forever?
Cyrill
--
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
[PATCH] kvm tool: Add own barrier() definition
Otherwise I'm getting the following compile
problem on my Fedora machine. The helper is
rather taken from linux kernel.
| [cyrill@moon kvm]$ make tags
| x86/include/kvm/barrier.h:11:25: fatal error: asm/barrier.h: No such file or
directory compilation terminated.
Signed-off-by: Cyrill Gorcunov
Acked-by: Ingo Molnar
---
tools/kvm/x86/include/kvm/barrier.h | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
Index: linux-2.6.git/tools/kvm/x86/include/kvm/barrier.h
===
--- linux-2.6.git.orig/tools/kvm/x86/include/kvm/barrier.h
+++ linux-2.6.git/tools/kvm/x86/include/kvm/barrier.h
@@ -1,13 +1,20 @@
#ifndef _KVM_BARRIER_H_
#define _KVM_BARRIER_H_
-/*
- * asm/system.h cannot be #included standalone on 32-bit x86 yet.
- *
- * Provide the dependencies here - we can drop these wrappers once
- * the header is fixed upstream:
- */
+#define barrier() asm volatile("": : :"memory")
-#include
+#define mb() asm volatile ("mfence": : :"memory")
+#define rmb() asm volatile ("lfence": : :"memory")
+#define wmb() asm volatile ("sfence": : :"memory")
+
+#ifdef CONFIG_SMP
+#define smp_mb() mb()
+#define smp_rmb() rmb()
+#define smp_wmb() wmb()
+#else
+#define smp_mb() barrier()
+#define smp_rmb() barrier()
+#define smp_wmb() barrier()
+#endif
#endif /* _KVM_BARRIER_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: [PATCH v2] kvm tools: Make raw block device work
On Thu, Apr 12, 2012 at 09:39:59PM +0800, 'Asias He wrote:
> +static bool is_mounted(struct stat *st)
> +{
> + struct stat st_buf;
> + struct mntent *mnt;
> + FILE *f;
> +
> + f = setmntent("/proc/mounts", "r");
> + if (!f)
> + return false;
> +
> + while ((mnt = getmntent(f)) != NULL) {
> + if (stat(mnt->mnt_fsname, &st_buf) == 0 &&
> + S_ISBLK(st_buf.st_mode) && st->st_rdev == st_buf.st_rdev)
> + return true;
> + }
> +
> + return false;
> +}
fclose missed?
Cyrill
--
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: native kvm tool hrtimer problem
On Thu, Mar 08, 2012 at 04:31:17PM +0100, Daniele Carollo wrote: > > > I don't really use the tap interface so lets CC Asias. Which guest > > > kernel are you using, btw? > > > > > > > Yup, Asias was using it, if my memory doesn't betray me. Also both -- host > > and guest kernel versions might be useful to know. iirc we were emulating > > rtc only while anything else passes through to kvm kernel driver. > > > > Cyrill > > As guest I'm using debian 6.0 squeeze found here > http://people.debian.org/~aurel32/qemu/i386/debian_squeeze_i386_standard.qcow2 > while on the host I'm using Opensuse 11.2 with kernel 3.3.0-rc1KVM > gitted from here http://git.kernel.org/pub/scm/virt/kvm/kvm.git > OK, lets see what Asias say. Cyrill -- 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: native kvm tool hrtimer problem
On Thu, Mar 08, 2012 at 05:11:16PM +0200, Pekka Enberg wrote: > On Thu, Mar 8, 2012 at 4:32 PM, Daniele Carollo > wrote: > > for an university study, I'm doing some network test between two vm > > using native linux kvm tool and connected via tap/virtio/vhost. > > When I run my script (several consecutive iperf tcp and udp > > execution), the first vm freeze with a message like: "hrtimer: > > interrupt took * ns". > > Is this a bug? (In order to complete the test i had to set the number > > of guest cpu to 1) > > > > Do you know what is the expected throughput between two vm using > > virtio/vhost? > > I don't really use the tap interface so lets CC Asias. Which guest > kernel are you using, btw? > Yup, Asias was using it, if my memory doesn't betray me. Also both -- host and guest kernel versions might be useful to know. iirc we were emulating rtc only while anything else passes through to kvm kernel driver. Cyrill -- 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: [PATCH] kvm tools: Make 'vm sandbox' more user-friendly
On Wed, Mar 07, 2012 at 08:53:59PM +0200, Pekka Enberg wrote:
> This patch changes 'vm sandbox' to automatically prefix a program path with
> "/host" in the guest side making this, for example, work as expected:
>
> $ ./vm sandbox -- ~/trinity/trinity --mode=random --dangerous
>
> Cc: Asias He
> Cc: Cyrill Gorcunov
> Cc: Ingo Molnar
> Cc: Sasha Levin
> Signed-off-by: Pekka Enberg
> ---
> tools/kvm/builtin-run.c | 29 ++---
> 1 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> index 6acc490..ce76b69 100644
> --- a/tools/kvm/builtin-run.c
> +++ b/tools/kvm/builtin-run.c
> @@ -847,9 +847,26 @@ static void kvm_write_sandbox_cmd_exactly(int fd, const
> char *arg)
> }
> }
>
> +static void resolve_program(const char *src, char *dst, size_t len)
> +{
> + struct stat st;
> +
> + stat(src, &st);
Hi Pekka, looks cool! I suspect we might need to add
a check if stat call has not been failed, on top of this
patch course ;)
Cyrill
--
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: [RFC/PATCH 1/2] kvm tools, seabios: Add "--bios" option to "vm run"
On Fri, Feb 24, 2012 at 05:05:29PM +0200, Pekka Enberg wrote: > This patch adds a "--bios" command line option to "vm run". You can use this > to > try to boot with SeaBIOS, for example: > > ./vm run --bios=/usr/share/seabios/bios.bin \ >--disk $HOME/images/debian_lenny_amd64_standard.qcow2 > > This doesn't boot yet for obvious reasons but at least people can now start to > play with external BIOS images easily. Hi Pekka, I believe it worth merging to start working on this feature. Cyrill -- 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: The way of mapping BIOS into the guest's address space
On Tue, Feb 14, 2012 at 11:07:08PM -0500, Kevin O'Connor wrote: ... > > hardware. Maybe we could poke someone from KVM camp for a hint? > > SeaBIOS has two ways to be deployed - first is to copy the image to > the top of the first 1MB (eg, 0xe-0xf) and jump to > 0xf000:0xfff0 in 16bit mode. The second way is to use the SeaBIOS elf > and deploy into memory (according to the elf memory map) and jump to > SeaBIOS in 32bit mode (according to the elf entry point). > > SeaBIOS doesn't really need to be in the top 4G of ram. SeaBIOS does > expect to have normal PC hardware devices (eg, a PIC), though many > hardware devices can be compiled out via its kconfig interface. The > more interesting challenge will likely be in communicating critical > pieces of information (eg, total memory size) into SeaBIOS. > > The SeaBIOS mailing list (seab...@seabios.org) is probably a better > location for technical seabios questions. > Hi Kevin, thanks for pointing. Yes, providing info back to seabios to setup mttr and such (so seabios would recognize them) is most challeging I think. Cyrill -- 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: The way of mapping BIOS into the guest's address space
On Tue, Feb 14, 2012 at 05:35:47PM +0200, Pekka Enberg wrote: > On Tue, Feb 14, 2012 at 09:20:18PM +0800, Yang Bai wrote: > >> And will seabios replace the present bios implement or co-exsit? > > On Tue, Feb 14, 2012 at 3:32 PM, Cyrill Gorcunov wrote: > > Ideally we should get rid of our minibios completely and only have > > seabios here instead. > > No, no, they should co-exist. There's absolutely no reason to force > people to use a BIOS to boot Linux. > I meant run-time (ie in memory). I didn't mean substitude our minibios, but rather have an ability to either run with compiled-in bios or with seabios instead. Cyrill -- 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: The way of mapping BIOS into the guest's address space
On Tue, Feb 14, 2012 at 09:20:18PM +0800, Yang Bai wrote: > And will seabios replace the present bios implement or co-exsit? Ideally we should get rid of our minibios completely and only have seabios here instead. Cyrill -- 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: The way of mapping BIOS into the guest's address space
On Tue, Feb 14, 2012 at 01:10:59PM +0200, Pekka Enberg wrote: > On Tue, Feb 14, 2012 at 1:03 PM, Yang Bai wrote: > > Since on X86, bios is always at the end of the address space, so I > > have some thought about how to implement the seabios support for kvm > > tool. > > > > 1. using kvm__register_mem to map the end of address space to the > > guest then copy the code of seabios to this mem region. Just emulating > > the bios chip. I think this is what should be done. > > > > 2. leave the bios code alone and don't touch the guest's address > > space. If the guest accesses the address belonging to the bios, it > > will be an IO request and we can emulate the IO access to the bios > > chip. > > > > Any ideas about this? > > The latter solution doesn't make any sense to me. Cyrill, do we really > need to put the BIOS at the end of the address space? Don't we have > unused space below 1 MB? I don't remember for sure how SeaBIOS works actually. What I rememer is that it aquires all hw environment might have. So without real look into seabios code I fear I can't answer. But reserving end of 4G address space for bios copy sounds reasonable if we going to behave as real hardware. Maybe we could poke someone from KVM camp for a hint? Cyrill -- 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: Q: Does linux kvm native tool support loading BIOS as the default loader now?
On Mon, Feb 13, 2012 at 08:14:22PM +0800, Yang Bai wrote: > Hi all, > > As I know, native tool does not support loading BIOS so it does not > support Windows. Is this supporting now? > If not, I may try to implement it. > Nope yet. There was a plan to implement seabios support, but nothing is done that far. Feel free to implement such support. Cyrill -- 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: [PATCH] kvm tool: rewrite kvm__init
On Thu, Feb 09, 2012 at 03:01:26PM +0200, Pekka Enberg wrote: > On Thu, Feb 9, 2012 at 7:40 AM, Yang Bai wrote: > > Since the different issues have been handled in the > > internal of kvm__init, it can only return NULL if error > > happened. > > > > Signed-off-by: Yang Bai > > Sorry, I don't understand what this patch is attempting to fix? Why do > you think it's better to drop the explicit error codes and always > return NULL upon error? > Yeah, I somehow don't get it as well. Cyrill -- 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: [PATCH 1/2] kvm tool: Report error and don't segfault if kvm__init() fails
On Mon, Feb 06, 2012 at 12:22:04PM +0200, Pekka Enberg wrote:
> Hi Michael,
>
> On Mon, 6 Feb 2012, Michael Ellerman wrote:
> >Signed-off-by: Michael Ellerman
> >---
> >tools/kvm/builtin-run.c |5 +
> >1 files changed, 5 insertions(+), 0 deletions(-)
> >
> >diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> >index 95d35a5..569246e 100644
> >--- a/tools/kvm/builtin-run.c
> >+++ b/tools/kvm/builtin-run.c
> >@@ -997,6 +997,11 @@ static int kvm_cmd_run_init(int argc, const char **argv)
> > }
> >
> > kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
> >+if (IS_ERR(kvm)) {
> >+r = PTR_ERR(kvm);
> >+pr_err("kvm__init() failed with error %d\n", r);
> >+goto fail;
> >+}
> >
>
> I just pushed commit 3dfcb6ec85d5430622c8b99ca05451c1afd08bf5 ("kvm
> tool: Don't close not yet opened files and SIGSEV fix") from
> Cyrillos which was which seems to fix both issues. I was flying back
> from FOSDEM so I couldn't merge it earlier, sorry.
>
Ouch, I somehow missed these patches from Michael. If I saw them earlier
I would not provide my patch (since Michael's changelog is a way more
descriptive and better than mine). Anyway, thanks!
Cyrill
--
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: [PATCH] kvm tool: Don't close not yet opened files and SIGSEV fix
On Sat, Feb 04, 2012 at 10:02:19PM +0400, Cyrill Gorcunov wrote:
>
> Strictly speaking, kvm__init need more serious rewrite together with
> kvm__arch_init/kvm_ipc__start/kvm_ipc__register_handler ret. vals tests,
> i'll do this a bit late.
>
Sorry for delay, was busy. Anyway, here is a quickfix for sigsev and close()s.
Cyrill
---
Subject: kvm tool: Don't close not yet opened files and SIGSEV fix
In case if there error happened in kvm__init and we have
no files opened -- we should not try to close them.
Also once kvm failed to init the caller should not try
to dereference a pointer obtained, otherwise we might get
SIGSEV
| [cyrill@moon kvm]$ ./lkvm run ...
| Error: '/dev/kvm' not found. Please make sure your kernel has CONFIG_KVM
| enabled and that the KVM modules are loaded.
| Segmentation fault (core dumped)
| [cyrill@moon kvm]$
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/builtin-run.c |4
tools/kvm/kvm.c | 41 +
2 files changed, 25 insertions(+), 20 deletions(-)
Index: linux-2.6.git/tools/kvm/builtin-run.c
===
--- linux-2.6.git.orig/tools/kvm/builtin-run.c
+++ linux-2.6.git/tools/kvm/builtin-run.c
@@ -997,6 +997,10 @@ static int kvm_cmd_run_init(int argc, co
}
kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
+ if (IS_ERR(kvm)) {
+ r = PTR_ERR(kvm);
+ goto fail;
+ }
kvm->single_step = single_step;
Index: linux-2.6.git/tools/kvm/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/kvm.c
+++ linux-2.6.git/tools/kvm/kvm.c
@@ -123,10 +123,12 @@ static int kvm__check_extensions(struct
static struct kvm *kvm__new(void)
{
struct kvm *kvm = calloc(1, sizeof(*kvm));
-
if (!kvm)
return ERR_PTR(-ENOMEM);
+ kvm->sys_fd = -1;
+ kvm->vm_fd = -1;
+
return kvm;
}
@@ -341,47 +343,42 @@ struct kvm *kvm__init(const char *kvm_de
}
kvm = kvm__new();
- if (IS_ERR_OR_NULL(kvm))
+ if (IS_ERR(kvm))
return kvm;
kvm->sys_fd = open(kvm_dev, O_RDWR);
if (kvm->sys_fd < 0) {
- if (errno == ENOENT) {
+ if (errno == ENOENT)
pr_err("'%s' not found. Please make sure your kernel
has CONFIG_KVM "
- "enabled and that the KVM modules are loaded.",
kvm_dev);
- ret = -errno;
- goto cleanup;
- }
- if (errno == ENODEV) {
- die("'%s' KVM driver not available.\n # (If the KVM "
- "module is loaded then 'dmesg' may offer
further clues "
- "about the failure.)", kvm_dev);
- ret = -errno;
- goto cleanup;
- }
+ "enabled and that the KVM modules are loaded.",
kvm_dev);
+ else if (errno == ENODEV)
+ pr_err("'%s' KVM driver not available.\n # (If the KVM
"
+ "module is loaded then 'dmesg' may offer further
clues "
+ "about the failure.)", kvm_dev);
+ else
+ pr_err("Could not open %s: ", kvm_dev);
- pr_err("Could not open %s: ", kvm_dev);
ret = -errno;
- goto cleanup;
+ goto err_free;
}
ret = ioctl(kvm->sys_fd, KVM_GET_API_VERSION, 0);
if (ret != KVM_API_VERSION) {
pr_err("KVM_API_VERSION ioctl");
ret = -errno;
- goto cleanup;
+ goto err_sys_fd;
}
kvm->vm_fd = ioctl(kvm->sys_fd, KVM_CREATE_VM, 0);
if (kvm->vm_fd < 0) {
ret = kvm->vm_fd;
- goto cleanup;
+ goto err_sys_fd;
}
kvm->name = strdup(name);
if (!kvm->name) {
ret = -ENOMEM;
- goto cleanup;
+ goto err;
}
if (kvm__check_extensions(kvm)) {
@@ -393,10 +390,14 @@ struct kvm *kvm__init(const char *kvm_de
kvm_ipc__start(kvm__create_socket(kvm));
kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid);
+
return kvm;
-cleanup:
+
+err:
close(kvm->vm_fd);
+err_sys_fd:
close(kvm->sys_fd);
+err_free:
free(kvm);
return ERR_PTR(ret);
--
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: [PATCH] kvm tool: Don't close not yet opened files and SIGSEV fix
On Sat, Feb 04, 2012 at 07:54:38PM +0200, Pekka Enberg wrote:
> On Sat, 4 Feb 2012, Cyrill Gorcunov wrote:
> >Index: linux-2.6.git/tools/kvm/kvm.c
> >===
> >--- linux-2.6.git.orig/tools/kvm/kvm.c
> >+++ linux-2.6.git/tools/kvm/kvm.c
> >@@ -123,10 +123,12 @@ static int kvm__check_extensions(struct
> >static struct kvm *kvm__new(void)
> >{
> > struct kvm *kvm = calloc(1, sizeof(*kvm));
> >-
> > if (!kvm)
> > return ERR_PTR(-ENOMEM);
> >
> >+kvm->sys_fd = -1;
> >+kvm->vm_fd = -1;
> >+
> > return kvm;
> >}
> >
> >@@ -394,9 +396,12 @@ struct kvm *kvm__init(const char *kvm_de
> > kvm_ipc__start(kvm__create_socket(kvm));
> > kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid);
> > return kvm;
> >+
> >cleanup:
> >-close(kvm->vm_fd);
> >-close(kvm->sys_fd);
> >+if (kvm->vm_fd >= 0)
> >+close(kvm->vm_fd);
> >+if (kvm->sys_fd >= 0)
> >+close(kvm->sys_fd);
>
> No, please don't do this! Use specific error handling labels instead.
>
Strictly speaking, kvm__init need more serious rewrite together with
kvm__arch_init/kvm_ipc__start/kvm_ipc__register_handler ret. vals tests,
i'll do this a bit late.
Cyrill
--
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: [PATCH] kvm tool: Don't close not yet opened files and SIGSEV fix
On Sat, Feb 04, 2012 at 07:38:41PM +0200, Pekka Enberg wrote:
> On Sat, 4 Feb 2012, Cyrill Gorcunov wrote:
> >In case if there error happened in kvm__init and we have
> >no files opened -- we should not try to close them.
> >
> >Also once kvm failed to init the caller should not try
> >to dereference a pointer obtained, otherwise we might get
> >SIGSEV
> >
> >| [cyrill@moon kvm]$ ./lkvm run ...
> >| Error: '/dev/kvm' not found. Please make sure your kernel has CONFIG_KVM
> >enabled and that the KVM modules are loaded.
> >| Segmentation fault (core dumped)
> >| [cyrill@moon kvm]$
> >
> >Signed-off-by: Cyrill Gorcunov
> >---
> >tools/kvm/builtin-run.c |4
> >tools/kvm/kvm.c | 11 ---
> >2 files changed, 12 insertions(+), 3 deletions(-)
> >
> >Index: linux-2.6.git/tools/kvm/builtin-run.c
> >===
> >--- linux-2.6.git.orig/tools/kvm/builtin-run.c
> >+++ linux-2.6.git/tools/kvm/builtin-run.c
> >@@ -997,6 +997,10 @@ static int kvm_cmd_run_init(int argc, co
> > }
> >
> > kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
> >+if (IS_ERR_OR_NULL(kvm)) {
>
> PTR_ERR is still not going to work if kvm is NULL.
>
It should be IS_ERR rather (kvm__init never returns with NULL),
but it's a candidate for another patch where both kvm__init
and kvm__new should test for IS_ERR only.
Pekka, could you please s/IS_ERR_OR_NULL/IS_ERR/ in this patch,
and that's all. Hm?
Cyrill
--
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
[PATCH] kvm tool: Don't close not yet opened files and SIGSEV fix
In case if there error happened in kvm__init and we have
no files opened -- we should not try to close them.
Also once kvm failed to init the caller should not try
to dereference a pointer obtained, otherwise we might get
SIGSEV
| [cyrill@moon kvm]$ ./lkvm run ...
| Error: '/dev/kvm' not found. Please make sure your kernel has CONFIG_KVM
enabled and that the KVM modules are loaded.
| Segmentation fault (core dumped)
| [cyrill@moon kvm]$
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/builtin-run.c |4
tools/kvm/kvm.c | 11 ---
2 files changed, 12 insertions(+), 3 deletions(-)
Index: linux-2.6.git/tools/kvm/builtin-run.c
===
--- linux-2.6.git.orig/tools/kvm/builtin-run.c
+++ linux-2.6.git/tools/kvm/builtin-run.c
@@ -997,6 +997,10 @@ static int kvm_cmd_run_init(int argc, co
}
kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
+ if (IS_ERR_OR_NULL(kvm)) {
+ r = PTR_ERR(kvm);
+ goto fail;
+ }
kvm->single_step = single_step;
Index: linux-2.6.git/tools/kvm/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/kvm.c
+++ linux-2.6.git/tools/kvm/kvm.c
@@ -123,10 +123,12 @@ static int kvm__check_extensions(struct
static struct kvm *kvm__new(void)
{
struct kvm *kvm = calloc(1, sizeof(*kvm));
-
if (!kvm)
return ERR_PTR(-ENOMEM);
+ kvm->sys_fd = -1;
+ kvm->vm_fd = -1;
+
return kvm;
}
@@ -394,9 +396,12 @@ struct kvm *kvm__init(const char *kvm_de
kvm_ipc__start(kvm__create_socket(kvm));
kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid);
return kvm;
+
cleanup:
- close(kvm->vm_fd);
- close(kvm->sys_fd);
+ if (kvm->vm_fd >= 0)
+ close(kvm->vm_fd);
+ if (kvm->sys_fd >= 0)
+ close(kvm->sys_fd);
free(kvm);
return ERR_PTR(ret);
--
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: [PATCH] kvm tool: Make kvm structure to carry name copy
On Sat, Feb 04, 2012 at 02:57:23PM +0200, Pekka Enberg wrote:
> On Sat, 4 Feb 2012, Cyrill Gorcunov wrote:
> >Index: linux-2.6.git/tools/kvm/kvm.c
> >===
> >--- linux-2.6.git.orig/tools/kvm/kvm.c
> >+++ linux-2.6.git/tools/kvm/kvm.c
> >@@ -254,6 +254,7 @@ int kvm__exit(struct kvm *kvm)
> > kvm__arch_delete_ram(kvm);
> > kvm_ipc__stop();
> > kvm__remove_socket(kvm->name);
> >+free((void *)kvm->name);
>
> Please fix the struct definition and drop the cast.
>
I believe having it as const char * might save us some
potential troubles, but sure, here we go ;)
Cyrill
---
kvm tool: Make kvm structure to carry name copy
If default guest name is used (which is the default
case) the kvm might end up carrying the pointer to
a name which is allocated on stack.
kvm_cmd_run_init
(on stack) default_name
kvm__init(..., default_name)
kvm->name = default_name
So make it to carry a copy of name.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/kvm.c |9 +++--
tools/kvm/powerpc/include/kvm/kvm-arch.h |2 +-
tools/kvm/x86/include/kvm/kvm-arch.h |2 +-
3 files changed, 9 insertions(+), 4 deletions(-)
Index: linux-2.6.git/tools/kvm/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/kvm.c
+++ linux-2.6.git/tools/kvm/kvm.c
@@ -254,6 +254,7 @@ int kvm__exit(struct kvm *kvm)
kvm__arch_delete_ram(kvm);
kvm_ipc__stop();
kvm__remove_socket(kvm->name);
+ free(kvm->name);
free(kvm);
return 0;
@@ -377,6 +378,12 @@ struct kvm *kvm__init(const char *kvm_de
goto cleanup;
}
+ kvm->name = strdup(name);
+ if (!kvm->name) {
+ ret = -ENOMEM;
+ goto cleanup;
+ }
+
if (kvm__check_extensions(kvm)) {
pr_err("A required KVM extention is not supported by OS");
ret = -ENOSYS;
@@ -384,8 +391,6 @@ struct kvm *kvm__init(const char *kvm_de
kvm__arch_init(kvm, hugetlbfs_path, ram_size);
- kvm->name = name;
-
kvm_ipc__start(kvm__create_socket(kvm));
kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid);
return kvm;
Index: linux-2.6.git/tools/kvm/powerpc/include/kvm/kvm-arch.h
===
--- linux-2.6.git.orig/tools/kvm/powerpc/include/kvm/kvm-arch.h
+++ linux-2.6.git/tools/kvm/powerpc/include/kvm/kvm-arch.h
@@ -64,7 +64,7 @@ struct kvm {
unsigned long fdt_gra;
unsigned long initrd_gra;
unsigned long initrd_size;
- const char *name;
+ char*name;
int vm_state;
};
Index: linux-2.6.git/tools/kvm/x86/include/kvm/kvm-arch.h
===
--- linux-2.6.git.orig/tools/kvm/x86/include/kvm/kvm-arch.h
+++ linux-2.6.git/tools/kvm/x86/include/kvm/kvm-arch.h
@@ -48,7 +48,7 @@ struct kvm {
struct disk_image **disks;
int nr_disks;
- const char *name;
+ char*name;
int vm_state;
};
--
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: [PATCH] kvm tool: Make kvm structure to carry name copy
On Sat, Feb 04, 2012 at 04:20:05PM +0400, Cyrill Gorcunov wrote:
> On Sat, Feb 04, 2012 at 02:15:36PM +0200, Pekka Enberg wrote:
> > On Fri, 3 Feb 2012, Cyrill Gorcunov wrote:
> > >If guest name is used (which is default case) the kvm might end
> > >up carrying the pointer to name which is allocated on stack.
> > >
> > >kvm_cmd_run_init
> > > (on stack) default_name
> > > kvm__init(..., default_name)
> > > kvm->name = default_name
> > >
> > >So I think better to allow kvm to carry own copy
> > >of guest name. 64 symbols should be more than enough.
> > >
> > >Signed-off-by: Cyrill Gorcunov
> > >---
> > >
> > >I hope I didn't miss anything?
> >
> > Can't we just use strdup()?
> >
>
> Yeah, I think this will be even better, I'll update.
>
Something like below I think.
Cyrill
---
Subject: [PATCH] kvm tool: Make kvm structure to carry name copy
If default guest name is used (which is the default
case) the kvm might end up carrying the pointer to
a name which is allocated on stack.
kvm_cmd_run_init
(on stack) default_name
kvm__init(..., default_name)
kvm->name = default_name
So make it to carry a copy of name.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/kvm.c |9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
Index: linux-2.6.git/tools/kvm/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/kvm.c
+++ linux-2.6.git/tools/kvm/kvm.c
@@ -254,6 +254,7 @@ int kvm__exit(struct kvm *kvm)
kvm__arch_delete_ram(kvm);
kvm_ipc__stop();
kvm__remove_socket(kvm->name);
+ free((void *)kvm->name);
free(kvm);
return 0;
@@ -377,6 +378,12 @@ struct kvm *kvm__init(const char *kvm_de
goto cleanup;
}
+ kvm->name = strdup(name);
+ if (!kvm->name) {
+ ret = -ENOMEM;
+ goto cleanup;
+ }
+
if (kvm__check_extensions(kvm)) {
pr_err("A required KVM extention is not supported by OS");
ret = -ENOSYS;
@@ -384,8 +391,6 @@ struct kvm *kvm__init(const char *kvm_de
kvm__arch_init(kvm, hugetlbfs_path, ram_size);
- kvm->name = name;
-
kvm_ipc__start(kvm__create_socket(kvm));
kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid);
return kvm;
--
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: [PATCH] kvm tool: Make kvm structure to carry name copy
On Sat, Feb 04, 2012 at 02:15:36PM +0200, Pekka Enberg wrote: > On Fri, 3 Feb 2012, Cyrill Gorcunov wrote: > >If guest name is used (which is default case) the kvm might end > >up carrying the pointer to name which is allocated on stack. > > > >kvm_cmd_run_init > > (on stack) default_name > > kvm__init(..., default_name) > > kvm->name = default_name > > > >So I think better to allow kvm to carry own copy > >of guest name. 64 symbols should be more than enough. > > > >Signed-off-by: Cyrill Gorcunov > >--- > > > >I hope I didn't miss anything? > > Can't we just use strdup()? > Yeah, I think this will be even better, I'll update. Cyrill -- 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
[PATCH] kvm tool: Make kvm structure to carry name copy
If guest name is used (which is default case) the kvm might end
up carrying the pointer to name which is allocated on stack.
kvm_cmd_run_init
(on stack) default_name
kvm__init(..., default_name)
kvm->name = default_name
So I think better to allow kvm to carry own copy
of guest name. 64 symbols should be more than enough.
Signed-off-by: Cyrill Gorcunov
---
I hope I didn't miss anything?
tools/kvm/kvm.c |2 +-
tools/kvm/powerpc/include/kvm/kvm-arch.h |2 +-
tools/kvm/x86/include/kvm/kvm-arch.h |2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
Index: linux-2.6.git/tools/kvm/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/kvm.c
+++ linux-2.6.git/tools/kvm/kvm.c
@@ -384,7 +384,7 @@ struct kvm *kvm__init(const char *kvm_de
kvm__arch_init(kvm, hugetlbfs_path, ram_size);
- kvm->name = name;
+ strncpy(kvm->name, name, sizeof(kvm->name));
kvm_ipc__start(kvm__create_socket(kvm));
kvm_ipc__register_handler(KVM_IPC_PID, kvm__pid);
Index: linux-2.6.git/tools/kvm/powerpc/include/kvm/kvm-arch.h
===
--- linux-2.6.git.orig/tools/kvm/powerpc/include/kvm/kvm-arch.h
+++ linux-2.6.git/tools/kvm/powerpc/include/kvm/kvm-arch.h
@@ -64,7 +64,7 @@ struct kvm {
unsigned long fdt_gra;
unsigned long initrd_gra;
unsigned long initrd_size;
- const char *name;
+ charname[64];
int vm_state;
};
Index: linux-2.6.git/tools/kvm/x86/include/kvm/kvm-arch.h
===
--- linux-2.6.git.orig/tools/kvm/x86/include/kvm/kvm-arch.h
+++ linux-2.6.git/tools/kvm/x86/include/kvm/kvm-arch.h
@@ -48,7 +48,7 @@ struct kvm {
struct disk_image **disks;
int nr_disks;
- const char *name;
+ charname[64];
int vm_state;
};
--
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: [PATCH] kvm tools: Fix test for mmap failure
On Fri, Feb 03, 2012 at 11:15:41PM +0400, Cyrill Gorcunov wrote:
> On error mmap returns MAP_FAILED so we
> need a proper test here.
>
Pekka, pick this one instead -- a caller is expecting null/not-null
only.
Cyrill
---
kvm tools: Fix test for mmap failure
On error mmap returns MAP_FAILED so we
need a proper test here.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/hw/pci-shmem.c |7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
Index: linux-2.6.git/tools/kvm/hw/pci-shmem.c
===
--- linux-2.6.git.orig/tools/kvm/hw/pci-shmem.c
+++ linux-2.6.git/tools/kvm/hw/pci-shmem.c
@@ -207,10 +207,11 @@ static void *setup_shmem(const char *key
}
mem = mmap(NULL, len,
PROT_READ | PROT_WRITE, MAP_SHARED | MAP_NORESERVE, fd, 0);
- close(fd);
-
- if (mem == NULL)
+ if (mem == MAP_FAILED) {
pr_warning("Failed to mmap shared memory file");
+ mem = NULL;
+ }
+ close(fd);
return mem;
}
--
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
[PATCH] kvm tools: Fix test for mmap failure
On error mmap returns MAP_FAILED so we
need a proper test here.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/hw/pci-shmem.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Index: linux-2.6.git/tools/kvm/hw/pci-shmem.c
===
--- linux-2.6.git.orig/tools/kvm/hw/pci-shmem.c
+++ linux-2.6.git/tools/kvm/hw/pci-shmem.c
@@ -209,7 +209,7 @@ static void *setup_shmem(const char *key
PROT_READ | PROT_WRITE, MAP_SHARED | MAP_NORESERVE, fd, 0);
close(fd);
- if (mem == NULL)
+ if (mem == MAP_FAILED)
pr_warning("Failed to mmap shared memory file");
return mem;
@@ -259,8 +259,9 @@ int pci_shmem__init(struct kvm *kvm)
/* Open shared memory and plug it into the guest */
mem = setup_shmem(shmem_region->handle, shmem_region->size,
shmem_region->create);
- if (mem == NULL)
+ if (mem == MAP_FAILED)
return 0;
+
kvm__register_mem(kvm, shmem_region->phys_addr, shmem_region->size,
mem);
return 1;
--
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: [PATCH] kvm tools: Fix segfault when failing to initialize KVM
On Wed, Feb 01, 2012 at 09:26:00AM +0200, Pekka Enberg wrote:
> On Wed, 1 Feb 2012, Cyrill Gorcunov wrote:
> >I suspect we need something like
> >---
> >tools/kvm/builtin-run.c |5 +
> >tools/kvm/kvm.c |2 +-
> >2 files changed, 6 insertions(+), 1 deletion(-)
> >
> >Index: linux-2.6.git/tools/kvm/builtin-run.c
> >===
> >--- linux-2.6.git.orig/tools/kvm/builtin-run.c
> >+++ linux-2.6.git/tools/kvm/builtin-run.c
> >@@ -997,6 +997,11 @@ static int kvm_cmd_run_init(int argc, co
> > }
> >
> > kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
> >+if (IS_ERR(kvm)) {
> >+r = (int)PTR_ERR(kvm);
>
> The cast is not needed.
>
Yeah, it's leftover from draft patch ;) Sasha, would you make
some new version or I should create new patch?
Cyrill
--
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: [PATCH] kvm tools: Fix segfault when failing to initialize KVM
On Wed, Feb 01, 2012 at 09:05:34AM +0200, Pekka Enberg wrote:
> On Tue, 31 Jan 2012, Sasha Levin wrote:
> >Might happen when hardware virtualization is not supported.
> >
> >Reported-by: Ingo Molnar
> >Signed-off-by: Sasha Levin
> >---
> >tools/kvm/builtin-run.c |4
> >1 files changed, 4 insertions(+), 0 deletions(-)
> >
> >diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
> >index 6ded1d2..a67faf8 100644
> >--- a/tools/kvm/builtin-run.c
> >+++ b/tools/kvm/builtin-run.c
> >@@ -997,6 +997,10 @@ static int kvm_cmd_run_init(int argc, const char **argv)
> > }
> >
> > kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
> >+if (IS_ERR_OR_NULL(kvm)) {
> >+r = PTR_ERR(kvm);
>
> How is this going to work when 'kvm' is NULL? It'd be best if
> kvm_init() never returned NULL on error.
>
> >+goto fail;
> >+}
> >
> > kvm->single_step = single_step;
> >
I suspect we need something like
---
tools/kvm/builtin-run.c |5 +
tools/kvm/kvm.c |2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
Index: linux-2.6.git/tools/kvm/builtin-run.c
===
--- linux-2.6.git.orig/tools/kvm/builtin-run.c
+++ linux-2.6.git/tools/kvm/builtin-run.c
@@ -997,6 +997,11 @@ static int kvm_cmd_run_init(int argc, co
}
kvm = kvm__init(dev, hugetlbfs_path, ram_size, guest_name);
+ if (IS_ERR(kvm)) {
+ r = (int)PTR_ERR(kvm);
+ pr_err("Can't initialize KVM, failed with error %d\n", r);
+ goto fail;
+ }
kvm->single_step = single_step;
Index: linux-2.6.git/tools/kvm/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/kvm.c
+++ linux-2.6.git/tools/kvm/kvm.c
@@ -340,7 +340,7 @@ struct kvm *kvm__init(const char *kvm_de
}
kvm = kvm__new();
- if (IS_ERR_OR_NULL(kvm))
+ if (IS_ERR(kvm))
return kvm;
kvm->sys_fd = open(kvm_dev, O_RDWR);
--
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
[PATCH] kvm tools: Remove tags/TAGS on "make clean"
Signed-off-by: Cyrill Gorcunov --- tools/kvm/Makefile |2 ++ 1 file changed, 2 insertions(+) Index: linux-2.6.git/tools/kvm/Makefile === --- linux-2.6.git.orig/tools/kvm/Makefile +++ linux-2.6.git/tools/kvm/Makefile @@ -327,6 +327,8 @@ clean: $(Q) rm -rf tests/boot/rootfs/ $(Q) rm -f $(DEPS) $(OBJS) $(PROGRAM) $(PROGRAM_ALIAS) $(GUEST_INIT) $(GUEST_INIT_S2) $(Q) rm -f cscope.* + $(Q) rm -f tags + $(Q) rm -f TAGS $(Q) rm -f $(KVM_INCLUDE)/common-cmds.h $(Q) rm -f KVMTOOLS-VERSION-FILE .PHONY: clean -- 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
[PATCH] kvm tool: Introduce own BUG_ON handler
Raise SIGABRT in case if run-time crtitical
problem found.
Proposed-by: Ingo Molnar
Signed-off-by: Cyrill Gorcunov
---
Ingo, you meant something like below?
tools/kvm/include/kvm/util.h | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
Index: linux-2.6.git/tools/kvm/include/kvm/util.h
===
--- linux-2.6.git.orig/tools/kvm/include/kvm/util.h
+++ linux-2.6.git/tools/kvm/include/kvm/util.h
@@ -9,7 +9,6 @@
* Some bits are stolen from perf tool :)
*/
-#include
#include
#include
#include
@@ -17,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -51,9 +51,20 @@ extern void set_die_routine(void (*routi
__func__, __LINE__, ##__VA_ARGS__); \
} while (0)
-#
+
#define BUILD_BUG_ON(condition)((void)sizeof(char[1 -
2*!!(condition)]))
-#define BUG_ON(condition) assert(!(condition))
+
+#ifndef BUG_ON_HANDLER
+# define BUG_ON_HANDLER(condition) \
+ do {\
+ if ((condition)) { \
+ pr_err("BUG at %s:%d", __FILE__, __LINE__); \
+ raise(SIGABRT); \
+ } \
+ } while (0)
+#endif
+
+#define BUG_ON(condition) BUG_ON_HANDLER((condition))
#define DIE_IF(cnd)\
do { \
--
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: [PATCH] kvm tools: Use assert() helper to check a variable value
On Mon, Dec 19, 2011 at 11:50:31AM +0100, Ingo Molnar wrote: ... > > The tool-specific BUG() implementation can be added as a delta > on top of that. It's in fact better to keep those two steps > separate. > OK, will do on top. Cyrill -- 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: [PATCH] kvm tools: Use assert() helper to check a variable value
On Mon, Dec 19, 2011 at 11:40:09AM +0100, Ingo Molnar wrote: > > GDB will catch that signal. > Yeah, good point! Pekka, drop this patch please, I'll make new one at evening. Cyrill -- 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: [PATCH] kvm tools: Use assert() helper to check a variable value
On Mon, Dec 19, 2011 at 10:19:43AM +0200, Pekka Enberg wrote:
> On Mon, Dec 19, 2011 at 09:13:28AM +0200, Pekka Enberg wrote:
> >> >
> >> >- BUILD_BUG_ON(i > E820_X_MAX);
> >> >+ assert(i <= E820_X_MAX);
> >>
> >> We should use BUG_ON() like tools/perf does.
>
> On Mon, Dec 19, 2011 at 9:57 AM, Cyrill Gorcunov wrote:
> > We dont have it yet. So I'll introduce this helper later,
> > but note that we will have to cover _all_ assert() calls then,
> > so it's better to make in a separate patch. Meanwhile such fix
> > it better than bug ;)
>
> You don't need to convert all of them at the same time but we're not
> adding new assert() calls.
>
You both (Pekka and Ingo) know the way how to convince people ;)
Something like below?
Cyrill
---
kvm tools: Add BUG_ON() helper to make a run-time critical tests
Also drop useless assert.h inclusions.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/include/kvm/util.h |5 -
tools/kvm/ioport.c |1 -
tools/kvm/kvm-cmd.c |6 ++
tools/kvm/kvm.c |1 -
tools/kvm/pci.c |6 ++
tools/kvm/powerpc/kvm.c |1 -
tools/kvm/virtio/console.c |3 +--
tools/kvm/virtio/net.c |1 -
tools/kvm/x86/bios.c |2 +-
tools/kvm/x86/cpuid.c|1 -
tools/kvm/x86/kvm.c |1 -
11 files changed, 10 insertions(+), 18 deletions(-)
Index: linux-2.6.git/tools/kvm/include/kvm/util.h
===
--- linux-2.6.git.orig/tools/kvm/include/kvm/util.h
+++ linux-2.6.git/tools/kvm/include/kvm/util.h
@@ -9,6 +9,7 @@
* Some bits are stolen from perf tool :)
*/
+#include
#include
#include
#include
@@ -50,7 +51,9 @@ extern void set_die_routine(void (*routi
__func__, __LINE__, ##__VA_ARGS__); \
} while (0)
-#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#
+#define BUILD_BUG_ON(condition)((void)sizeof(char[1 -
2*!!(condition)]))
+#define BUG_ON(condition) assert(!(condition))
#define DIE_IF(cnd)\
do { \
Index: linux-2.6.git/tools/kvm/ioport.c
===
--- linux-2.6.git.orig/tools/kvm/ioport.c
+++ linux-2.6.git/tools/kvm/ioport.c
@@ -10,7 +10,6 @@
#include
#include
-#include
#include
#include
#include
Index: linux-2.6.git/tools/kvm/kvm-cmd.c
===
--- linux-2.6.git.orig/tools/kvm/kvm-cmd.c
+++ linux-2.6.git/tools/kvm/kvm-cmd.c
@@ -2,8 +2,6 @@
#include
#include
-#include
-
/* user defined header files */
#include "kvm/builtin-debug.h"
#include "kvm/builtin-pause.h"
@@ -71,14 +69,14 @@ int handle_command(struct cmd_struct *co
if (!argv || !*argv) {
p = kvm_get_command(command, "help");
- assert(p);
+ BUG_ON(!p);
return p->fn(argc, argv, prefix);
}
p = kvm_get_command(command, argv[0]);
if (!p) {
p = kvm_get_command(command, "help");
- assert(p);
+ BUG_ON(!p);
p->fn(0, NULL, prefix);
return EINVAL;
}
Index: linux-2.6.git/tools/kvm/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/kvm.c
+++ linux-2.6.git/tools/kvm/kvm.c
@@ -14,7 +14,6 @@
#include
#include
#include
-#include
#include
#include
#include
Index: linux-2.6.git/tools/kvm/pci.c
===
--- linux-2.6.git.orig/tools/kvm/pci.c
+++ linux-2.6.git/tools/kvm/pci.c
@@ -3,8 +3,6 @@
#include "kvm/util.h"
#include "kvm/kvm.h"
-#include
-
#define PCI_BAR_OFFSET(b) (offsetof(struct pci_device_header,
bar[b]))
static struct pci_device_header*pci_devices[PCI_MAX_DEVICES];
@@ -170,13 +168,13 @@ void pci__config_rd(struct kvm *kvm, uni
void pci__register(struct pci_device_header *dev, u8 dev_num)
{
- assert(dev_num < PCI_MAX_DEVICES);
+ BUG_ON(dev_num >= PCI_MAX_DEVICES);
pci_devices[dev_num]= dev;
}
struct pci_device_header *pci__find_dev(u8 dev_num)
{
- assert(dev_num < PCI_MAX_DEVICES);
+ BUG_ON(dev_num >= PCI_MAX_DEVICES);
return pci_devices[dev_num];
}
Index: linux-2.6.git/tools/kvm/powerpc/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/powerpc/kvm.c
+++ linux-2.6.git/tools/kvm/powerpc/kvm.c
@@ -17,7 +17,6 @@
#include
#include
#include
-#include
#include
#includ
Re: [PATCH] kvm tools: Use assert() helper to check a variable value
On Mon, Dec 19, 2011 at 09:13:28AM +0200, Pekka Enberg wrote: > > > >-BUILD_BUG_ON(i > E820_X_MAX); > >+assert(i <= E820_X_MAX); > > We should use BUG_ON() like tools/perf does. > We dont have it yet. So I'll introduce this helper later, but note that we will have to cover _all_ assert() calls then, so it's better to make in a separate patch. Meanwhile such fix it better than bug ;) Cyrill -- 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
[PATCH] kvm tools: Use assert() helper to check a variable value
BUILD_BUG_ON is unable to catch errors on expression which can't be evaluated at compile time. Signed-off-by: Cyrill Gorcunov --- tools/kvm/x86/bios.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: linux-2.6.git/tools/kvm/x86/bios.c === --- linux-2.6.git.orig/tools/kvm/x86/bios.c +++ linux-2.6.git/tools/kvm/x86/bios.c @@ -5,6 +5,7 @@ #include "kvm/util.h" #include +#include #include #include "bios/bios-rom.h" @@ -98,7 +99,7 @@ static void e820_setup(struct kvm *kvm) }; } - BUILD_BUG_ON(i > E820_X_MAX); + assert(i <= E820_X_MAX); e820->nr_map = i; } -- 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
[RFC] kvm tools: Make kvm__arch_setup_firmware to return error code
If some of subsequent calls fails we better to return error
code instead of dying with a message. This is a first step
in getting rid of number of die() calls we have in code.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/builtin-run.c |5 -
tools/kvm/include/kvm/kvm.h |2 +-
tools/kvm/powerpc/kvm.c |4 +++-
tools/kvm/x86/include/kvm/mptable.h |2 +-
tools/kvm/x86/kvm.c |4 ++--
tools/kvm/x86/mptable.c | 10 +++---
6 files changed, 18 insertions(+), 9 deletions(-)
Index: linux-2.6.git/tools/kvm/builtin-run.c
===
--- linux-2.6.git.orig/tools/kvm/builtin-run.c
+++ linux-2.6.git/tools/kvm/builtin-run.c
@@ -1121,7 +1121,9 @@ int kvm_cmd_run(int argc, const char **a
kvm__start_timer(kvm);
- kvm__arch_setup_firmware(kvm);
+ exit_code = kvm__arch_setup_firmware(kvm);
+ if (exit_code)
+ goto err;
for (i = 0; i < nrcpus; i++) {
kvm_cpus[i] = kvm_cpu__init(kvm, i);
@@ -1151,6 +1153,7 @@ int kvm_cmd_run(int argc, const char **a
exit_code = 1;
}
+err:
compat__print_all_messages();
fb__stop();
Index: linux-2.6.git/tools/kvm/include/kvm/kvm.h
===
--- linux-2.6.git.orig/tools/kvm/include/kvm/kvm.h
+++ linux-2.6.git/tools/kvm/include/kvm/kvm.h
@@ -55,7 +55,7 @@ void kvm__remove_socket(const char *name
void kvm__arch_set_cmdline(char *cmdline, bool video);
void kvm__arch_init(struct kvm *kvm, const char *kvm_dev, const char
*hugetlbfs_path, u64 ram_size, const char *name);
-void kvm__arch_setup_firmware(struct kvm *kvm);
+int kvm__arch_setup_firmware(struct kvm *kvm);
bool kvm__arch_cpu_supports_vm(void);
void kvm__arch_periodic_poll(struct kvm *kvm);
Index: linux-2.6.git/tools/kvm/powerpc/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/powerpc/kvm.c
+++ linux-2.6.git/tools/kvm/powerpc/kvm.c
@@ -176,7 +176,7 @@ static void setup_fdt(struct kvm *kvm)
/**
* kvm__arch_setup_firmware
*/
-void kvm__arch_setup_firmware(struct kvm *kvm)
+int kvm__arch_setup_firmware(struct kvm *kvm)
{
/* Load RTAS */
@@ -184,4 +184,6 @@ void kvm__arch_setup_firmware(struct kvm
/* Init FDT */
setup_fdt(kvm);
+
+ return 0;
}
Index: linux-2.6.git/tools/kvm/x86/include/kvm/mptable.h
===
--- linux-2.6.git.orig/tools/kvm/x86/include/kvm/mptable.h
+++ linux-2.6.git/tools/kvm/x86/include/kvm/mptable.h
@@ -3,6 +3,6 @@
struct kvm;
-void mptable_setup(struct kvm *kvm, unsigned int ncpus);
+int mptable_setup(struct kvm *kvm, unsigned int ncpus);
#endif /* KVM_MPTABLE_H_ */
Index: linux-2.6.git/tools/kvm/x86/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/x86/kvm.c
+++ linux-2.6.git/tools/kvm/x86/kvm.c
@@ -349,7 +349,7 @@ bool load_bzimage(struct kvm *kvm, int f
* This function is a main routine where we poke guest memory
* and install BIOS there.
*/
-void kvm__arch_setup_firmware(struct kvm *kvm)
+int kvm__arch_setup_firmware(struct kvm *kvm)
{
/* standart minimal configuration */
setup_bios(kvm);
@@ -357,7 +357,7 @@ void kvm__arch_setup_firmware(struct kvm
/* FIXME: SMP, ACPI and friends here */
/* MP table */
- mptable_setup(kvm, kvm->nrcpus);
+ return mptable_setup(kvm, kvm->nrcpus);
}
void kvm__arch_periodic_poll(struct kvm *kvm)
Index: linux-2.6.git/tools/kvm/x86/mptable.c
===
--- linux-2.6.git.orig/tools/kvm/x86/mptable.c
+++ linux-2.6.git/tools/kvm/x86/mptable.c
@@ -71,7 +71,7 @@ static void mptable_add_irq_src(struct m
/**
* mptable_setup - create mptable and fill guest memory with it
*/
-void mptable_setup(struct kvm *kvm, unsigned int ncpus)
+int mptable_setup(struct kvm *kvm, unsigned int ncpus)
{
unsigned long real_mpc_table, real_mpf_intel, size;
struct mpf_intel *mpf_intel;
@@ -264,8 +264,11 @@ void mptable_setup(struct kvm *kvm, unsi
*/
if (size > (unsigned long)(MB_BIOS_END - bios_rom_size) ||
- size > MPTABLE_MAX_SIZE)
- die("MP table is too big");
+ size > MPTABLE_MAX_SIZE) {
+ free(mpc_table);
+ pr_err("MP table is too big");
+ return -1;
+ }
/*
* OK, it is time to move it to guest memory.
@@ -273,4 +276,5 @@ void mptable_setup(struct kvm *kvm, unsi
memcpy(guest_flat_to_host(kvm, real_mpc_table), mpc_table, size);
free(mpc_table);
+ return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to
[PATCH] kvm tools: sdl -- Fix array size for keymap
Index is u8 value so array size should be 256.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/ui/sdl.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.git/tools/kvm/ui/sdl.c
===
--- linux-2.6.git.orig/tools/kvm/ui/sdl.c
+++ linux-2.6.git/tools/kvm/ui/sdl.c
@@ -34,7 +34,7 @@ struct set2_scancode {
.type = SCANCODE_ESCAPED,\
}
-static const struct set2_scancode const keymap[255] = {
+static const struct set2_scancode const keymap[256] = {
[9] = DEFINE_SC(0x76), /* */
[10]= DEFINE_SC(0x16), /* 1 */
[11]= DEFINE_SC(0x1e), /* 2 */
--
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
[PATCH] kvm tools: Rename pr_error to pr_err to follow kernel convention
The kernel already has pr_err helper lets do the same.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/builtin-stat.c |2 +-
tools/kvm/disk/core.c |2 +-
tools/kvm/include/kvm/util.h |2 +-
tools/kvm/kvm.c|2 +-
tools/kvm/util/parse-options.c | 16
tools/kvm/util/util.c |2 +-
6 files changed, 13 insertions(+), 13 deletions(-)
Index: linux-2.6.git/tools/kvm/builtin-stat.c
===
--- linux-2.6.git.orig/tools/kvm/builtin-stat.c
+++ linux-2.6.git/tools/kvm/builtin-stat.c
@@ -68,7 +68,7 @@ static int do_memstat(const char *name,
r = select(1, &fdset, NULL, NULL, &t);
if (r < 0) {
- pr_error("Could not retrieve mem stats from %s", name);
+ pr_err("Could not retrieve mem stats from %s", name);
return r;
}
r = read(sock, &stats, sizeof(stats));
Index: linux-2.6.git/tools/kvm/disk/core.c
===
--- linux-2.6.git.orig/tools/kvm/disk/core.c
+++ linux-2.6.git/tools/kvm/disk/core.c
@@ -118,7 +118,7 @@ struct disk_image **disk_image__open_all
disks[i] = disk_image__open(filenames[i], readonly[i]);
if (!disks[i]) {
- pr_error("Loading disk image '%s' failed",
filenames[i]);
+ pr_err("Loading disk image '%s' failed", filenames[i]);
goto error;
}
}
Index: linux-2.6.git/tools/kvm/include/kvm/util.h
===
--- linux-2.6.git.orig/tools/kvm/include/kvm/util.h
+++ linux-2.6.git/tools/kvm/include/kvm/util.h
@@ -38,7 +38,7 @@ extern bool do_debug_print;
extern void die(const char *err, ...) NORETURN __attribute__((format (printf,
1, 2)));
extern void die_perror(const char *s) NORETURN;
-extern int pr_error(const char *err, ...) __attribute__((format (printf, 1,
2)));
+extern int pr_err(const char *err, ...) __attribute__((format (printf, 1, 2)));
extern void pr_warning(const char *err, ...) __attribute__((format (printf, 1,
2)));
extern void pr_info(const char *err, ...) __attribute__((format (printf, 1,
2)));
extern void set_die_routine(void (*routine)(const char *err, va_list params)
NORETURN);
Index: linux-2.6.git/tools/kvm/kvm.c
===
--- linux-2.6.git.orig/tools/kvm/kvm.c
+++ linux-2.6.git/tools/kvm/kvm.c
@@ -109,7 +109,7 @@ static int kvm__check_extensions(struct
if (!kvm_req_ext[i].name)
break;
if (!kvm__supports_extension(kvm, kvm_req_ext[i].code)) {
- pr_error("Unsuppored KVM extension detected: %s",
+ pr_err("Unsuppored KVM extension detected: %s",
kvm_req_ext[i].name);
return (int)-i;
}
Index: linux-2.6.git/tools/kvm/util/parse-options.c
===
--- linux-2.6.git.orig/tools/kvm/util/parse-options.c
+++ linux-2.6.git/tools/kvm/util/parse-options.c
@@ -17,10 +17,10 @@
static int opterror(const struct option *opt, const char *reason, int flags)
{
if (flags & OPT_SHORT)
- return pr_error("switch `%c' %s", opt->short_name, reason);
+ return pr_err("switch `%c' %s", opt->short_name, reason);
if (flags & OPT_UNSET)
- return pr_error("option `no-%s' %s", opt->long_name, reason);
- return pr_error("option `%s' %s", opt->long_name, reason);
+ return pr_err("option `no-%s' %s", opt->long_name, reason);
+ return pr_err("option `%s' %s", opt->long_name, reason);
}
static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
@@ -324,7 +324,7 @@ static void check_typos(const char *arg,
return;
if (!prefixcmp(arg, "no-")) {
- pr_error ("did you mean `--%s` (with two dashes ?)", arg);
+ pr_err("did you mean `--%s` (with two dashes ?)", arg);
exit(129);
}
@@ -332,7 +332,7 @@ static void check_typos(const char *arg,
if (!options->long_name)
continue;
if (!prefixcmp(options->long_name, arg)) {
- pr_error ("did you mean `--%s` (with two dashes ?)",
arg);
+ pr_err("did you mean `--%s` (with two dashes ?)", arg);
exit(129);
}
}
@@ -430,7 +430,7 @@
[PATCH] kvm tools: Define __compiletime_error helper
To eliminate compile errors like | CC builtin-run.o | In file included from ../../arch/x86/include/asm/system.h:7:0, | from include/kvm/barrier.h:13, | from builtin-run.c:16: | ../../arch/x86/include/asm/cmpxchg.h:11:13: error: no previous prototype for ‘__xchg_wrong_size’ [-Werror=missing-prototypes] | ../../arch/x86/include/asm/cmpxchg.h: In function ‘__xchg_wrong_size’: | ../../arch/x86/include/asm/cmpxchg.h:12:2: error: expected declaration specifiers before ‘__compiletime_error’ Signed-off-by: Cyrill Gorcunov --- Not sure if it's me only or not. I've had no such problems before. tools/kvm/include/kvm/compiler.h |4 1 file changed, 4 insertions(+) Index: linux-2.6.git/tools/kvm/include/kvm/compiler.h === --- linux-2.6.git.orig/tools/kvm/include/kvm/compiler.h +++ linux-2.6.git/tools/kvm/include/kvm/compiler.h @@ -1,6 +1,10 @@ #ifndef KVM_COMPILER_H_ #define KVM_COMPILER_H_ +#ifndef __compiletime_error +# define __compiletime_error(message) +#endif + #define notrace __attribute__((no_instrument_function)) #endif /* KVM_COMPILER_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: [PATCH] kvm tools: Trivial cleanup
On Fri, Dec 16, 2011 at 10:40:06AM +0200, Sasha Levin wrote: > Signed-off-by: Sasha Levin > --- Thanks, Sasha! Cyrill -- 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: [PATCH] kvm tools: Add 'kvm nmi' command
On Wed, Dec 07, 2011 at 12:41:50PM +0200, Gleb Natapov wrote: > > > > Yup, but while we support linux kernels only it should be fine. Still > > of course on long term we need a check. > > > Tomorrow someone will send a patch to change how Linux behaves and > slightly older kvmtool will not be able to run newer kernels :) No need > to wait for long term, a couple of lines of code will fix the issue in the > patch. Look here for reference: > > http://article.gmane.org/gmane.comp.emulators.kvm.devel/80339 > Sure. Thanks Gleb! Cyrill -- 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: [PATCH] kvm tools: Add 'kvm nmi' command
On Wed, Dec 07, 2011 at 12:33:05PM +0200, Gleb Natapov wrote:
> On Wed, Dec 07, 2011 at 02:31:11PM +0400, Cyrill Gorcunov wrote:
> > On Wed, Dec 07, 2011 at 12:21:52PM +0200, Gleb Natapov wrote:
> > > On Tue, Dec 06, 2011 at 10:42:55PM +0200, Sasha Levin wrote:
> > > > +static void handle_nmi(int fd, u32 type, u32 len, u8 *msg)
> > > > +{
> > > > + u32 vcpu = *(u32 *)msg;
> > > > +
> > > > + ioctl(kvm_cpus[vcpu]->vcpu_fd, KVM_NMI);
> > >
> > > You need to check that vcpu apic's LINT1 is configured to receive
> > > NMI (and not masked obviously) before injecting NMI.
> > >
> >
> > I've been configuring mptable to have lint1 as nmi receiver,
> > so it should remain so I suppose (if only we've not masked it
> > somewhere else ;)
> >
>
> That's up to the guest. mptable is just a hint to an OS on how things is
> wired in HW.
>
Yup, but while we support linux kernels only it should be fine. Still
of course on long term we need a check.
Cyrill
--
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: [PATCH] kvm tools: Add 'kvm nmi' command
On Wed, Dec 07, 2011 at 12:21:52PM +0200, Gleb Natapov wrote:
> On Tue, Dec 06, 2011 at 10:42:55PM +0200, Sasha Levin wrote:
> > +static void handle_nmi(int fd, u32 type, u32 len, u8 *msg)
> > +{
> > + u32 vcpu = *(u32 *)msg;
> > +
> > + ioctl(kvm_cpus[vcpu]->vcpu_fd, KVM_NMI);
>
> You need to check that vcpu apic's LINT1 is configured to receive
> NMI (and not masked obviously) before injecting NMI.
>
I've been configuring mptable to have lint1 as nmi receiver,
so it should remain so I suppose (if only we've not masked it
somewhere else ;)
Cyrill
--
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: [PATCH 16/28] kvm tools: Allow load_flat_binary() to load an initrd alongside
On Wed, Dec 07, 2011 at 11:42:27AM +1100, Matt Evans wrote: > Hi Cyrill, > > On 06/12/11 23:04, Cyrill Gorcunov wrote: > > On Tue, Dec 06, 2011 at 12:29:48PM +0200, Pekka Enberg wrote: > > ... > >> > >> Otherwise looks OK to me. Cyrill? > >> > > > > It might be not seen from patch (or my local kvm repo > > is not yet updated well) but I somehow miss who will be > > reading initrd in case of loading flat image? If noone, > > then what's the point to pass fd there at all? > > I pass in the initrd fd in generic code in preparation for the PPC support in > "[PATCH 1/8] kvm tools: Add initial SPAPR PPC64 architecture support" which > uses > it. As you saw, I haven't made x86 load the initrd in the case of a flat > kernel > binary being loaded; I didn't think a flat kernel can have a non-embedded > initrd > on x86? (My x86 is rusty, this may not be true ;) bootparams are [b]zImage > only?) > Ah, I see. Sorry for confusion. Thanks. Cyrill -- 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: [PATCH 23/28] kvm tools: Endian-sanitise pci.h and PCI device setup
On Tue, Dec 06, 2011 at 03:29:00PM +0200, Pekka Enberg wrote:
> >
> >Hehe, this is because it should be rtaher defined as
> >
> >union pci_config_address {
> >struct {
> > #if __BYTE_ORDER == __LITTLE_ENDIAN
> > unsignedzeros : 2;
> > unsignedregister_number : 6;
> > #else
> > ...
> > #endif
> >}
> >u32 w;
> >};
>
> Yup, that fixes it for me.
>
Good. Matt, mind to update?
Cyrill
--
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: [PATCH 23/28] kvm tools: Endian-sanitise pci.h and PCI device setup
On Tue, Dec 06, 2011 at 01:58:24PM +0200, Pekka Enberg wrote:
> On Tue, 2011-12-06 at 15:47 +0400, Cyrill Gorcunov wrote:
> > On Tue, Dec 06, 2011 at 01:41:56PM +0200, Pekka Enberg wrote:
> > > On Tue, Dec 6, 2011 at 12:28 PM, Cyrill Gorcunov
> > > wrote:
> > > > On Tue, Dec 06, 2011 at 12:25:29PM +0200, Pekka Enberg wrote:
> > > >> On Tue, Dec 6, 2011 at 5:42 AM, Matt Evans wrote:
> > > >> > vesa, pci-shmem and virtio-pci devices need to set up config space
> > > >> > with
> > > >> > little-endian conversions (as config space is LE). The
> > > >> > pci_config_address
> > > >> > bitfield also needs to be reversed when building on BE systems.
> > > >> >
> > > >> > Signed-off-by: Matt Evans
> > > >>
> > > >> Looks OK to me. Sasha, Cyrill?
> > > >>
> > > >
> > > > BIOS part looks pretty good to me. LE/BE part as well. Thanks Matt!
> > >
> > > Hmm. This seems to break "make check" for me:
> > >
> >
> > If you change back to
> >
> > static struct pci_device_header pci_shmem_pci_device = {
> > ...
> > .class = 0xFF, /* misc pci device */
> > ...
> > };
> >
> > does it help?
>
> No but dropping these hunks fixes it for me:
>
> @@ -17,7 +18,8 @@
> #define PCI_CONFIG_BUS_FORWARD 0xcfa
> #define PCI_IO_SIZE0x100
>
> -struct pci_config_address {
> +union pci_config_address {
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
>unsignedzeros : 2;/* 1 .. 0 */
>unsignedregister_number : 6;/* 7 .. 2 */
>unsignedfunction_number : 3;/* 10 .. 8 */
> @@ -25,6 +27,16 @@ struct pci_config_address {
>unsignedbus_number : 8;/* 23 .. 16 */
>unsignedreserved: 7;/* 30 .. 24 */
>unsignedenable_bit : 1;/* 31 */
> +#else
> + unsignedenable_bit : 1;/* 31 */
> + unsignedreserved: 7;/* 30 .. 24 */
> + unsignedbus_number : 8;/* 23 .. 16 */
> + unsigneddevice_number : 5;/* 15 .. 11 */
> + unsignedfunction_number : 3;/* 10 .. 8 */
> + unsignedregister_number : 6;/* 7 .. 2 */
> + unsignedzeros : 2;/* 1 .. 0 */
> +#endif
> + u32 w;
> };
>
> Pekka
>
Hehe, this is because it should be rtaher defined as
union pci_config_address {
struct {
#if __BYTE_ORDER == __LITTLE_ENDIAN
unsignedzeros : 2;
unsignedregister_number : 6;
#else
...
#endif
}
u32 w;
};
Cyrill
--
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: [PATCH 16/28] kvm tools: Allow load_flat_binary() to load an initrd alongside
On Tue, Dec 06, 2011 at 12:29:48PM +0200, Pekka Enberg wrote: ... > > Otherwise looks OK to me. Cyrill? > It might be not seen from patch (or my local kvm repo is not yet updated well) but I somehow miss who will be reading initrd in case of loading flat image? If noone, then what's the point to pass fd there at all? Cyrill -- 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: [PATCH 23/28] kvm tools: Endian-sanitise pci.h and PCI device setup
On Tue, Dec 06, 2011 at 01:41:56PM +0200, Pekka Enberg wrote:
> On Tue, Dec 6, 2011 at 12:28 PM, Cyrill Gorcunov wrote:
> > On Tue, Dec 06, 2011 at 12:25:29PM +0200, Pekka Enberg wrote:
> >> On Tue, Dec 6, 2011 at 5:42 AM, Matt Evans wrote:
> >> > vesa, pci-shmem and virtio-pci devices need to set up config space with
> >> > little-endian conversions (as config space is LE). The
> >> > pci_config_address
> >> > bitfield also needs to be reversed when building on BE systems.
> >> >
> >> > Signed-off-by: Matt Evans
> >>
> >> Looks OK to me. Sasha, Cyrill?
> >>
> >
> > BIOS part looks pretty good to me. LE/BE part as well. Thanks Matt!
>
> Hmm. This seems to break "make check" for me:
>
If you change back to
static struct pci_device_header pci_shmem_pci_device = {
...
.class = 0xFF, /* misc pci device */
...
};
does it help?
Cyrill
--
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: [PATCH 23/28] kvm tools: Endian-sanitise pci.h and PCI device setup
On Tue, Dec 06, 2011 at 12:25:29PM +0200, Pekka Enberg wrote: > On Tue, Dec 6, 2011 at 5:42 AM, Matt Evans wrote: > > vesa, pci-shmem and virtio-pci devices need to set up config space with > > little-endian conversions (as config space is LE). The pci_config_address > > bitfield also needs to be reversed when building on BE systems. > > > > Signed-off-by: Matt Evans > > Looks OK to me. Sasha, Cyrill? > BIOS part looks pretty good to me. LE/BE part as well. Thanks Matt! Cyrill -- 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: [PATCH] kvm tools: Split custom rootfs init into two stages
On Mon, Dec 05, 2011 at 11:22:11AM +0200, Sasha Levin wrote:
>
> +static int kvm_custom_stage2(void)
> +{
> + char tmp[PATH_MAX], dst[PATH_MAX], *src;
> + const char *rootfs;
> + int r;
> +
> + src = realpath("guest/init_stage2", NULL);
> + if (src == NULL)
> + return -ENOMEM;
> +
> + if (image_filename[0] == NULL)
> + rootfs = "default";
> + else
> + rootfs = image_filename[0];
> +
> + sprintf(tmp, "%s%s/virt/init_stage2", kvm__get_dir(), rootfs);
> + remove(tmp);
> +
> + sprintf(dst, "/host/%s", src);
> + r = symlink(dst, tmp);
> + free(src);
> +
> + return r;
> +}
> +
I might be paranoid -- but could you please use snprintf here? :)
--
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: [PATCH] kvm tools: Support virtio indirect buffers
On Tue, Nov 29, 2011 at 03:01:59PM +0200, Pekka Enberg wrote: > >> > >>Hi Sasha, where the rmb() then? Or maybe you wanted plain barrier() here? > > > >On the kernel side. > >Theres a mb there which happens there during the kick. > > I guess we need to improve the comment in next_desc()? > Kernel's code has pretty good aliases for virtio barriers I think #ifdef CONFIG_SMP /* Where possible, use SMP barriers which are more lightweight than mandatory * barriers, because mandatory barriers control MMIO effects on accesses * through relaxed memory I/O windows (which virtio does not use). */ #define virtio_mb() smp_mb() #define virtio_rmb() smp_rmb() #define virtio_wmb() smp_wmb() #else /* We must force memory ordering even if guest is UP since host could be * running on another CPU, but SMP barriers are defined to barrier() in that * configuration. So fall back to mandatory barriers instead. */ #define virtio_mb() mb() #define virtio_rmb() rmb() #define virtio_wmb() wmb() #endif Maybe we could use somethig similar? -- 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: [PATCH] kvm tools: Support virtio indirect buffers
On Mon, Nov 28, 2011 at 07:54:27PM +0200, Sasha Levin wrote:
>
> +/*
> + * Each buffer in the virtqueues is actually a chain of descriptors. This
> + * function returns the next descriptor in the chain, or vq->vring.num if
> we're
> + * at the end.
> + */
> +static unsigned next_desc(struct vring_desc *desc,
> + unsigned int i, unsigned int max)
> +{
> + unsigned int next;
> +
> + /* If this descriptor says it doesn't chain, we're done. */
> + if (!(desc[i].flags & VRING_DESC_F_NEXT))
> + return max;
> +
> + /* Check they're not leading us off end of descriptors. */
> + next = desc[i].next;
> + /* Make sure compiler knows to grab that: we don't want it changing! */
> + wmb();
> +
> + return next;
> +}
> +
Hi Sasha, where the rmb() then? Or maybe you wanted plain barrier() here?
--
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: kvm tools: clock sources for hrtimer
On Wed, Nov 09, 2011 at 09:13:54PM +0200, Pekka Enberg wrote:
...
> Yup, "notsc" was used in the early days to avoid APIC emulation.
>
> Anyone care to send a patch to drop it from master?
>
> Pekka
>
Here we go
---
Subject: [PATCH] tools, kvm: Drop "notsc" no longer needed kernel option
"notsc" option has been used to avoid APIC calibration
problem at early days when we didn't support APIC at all.
Now with KVM APIC emulation used there is no longer need
for this option. Drop it.
Reported-by: Richard Weinberger
Tested-by: Richard Weinberger
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/builtin-run.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.git/tools/kvm/builtin-run.c
===
--- linux-2.6.git.orig/tools/kvm/builtin-run.c
+++ linux-2.6.git/tools/kvm/builtin-run.c
@@ -830,7 +830,7 @@ int kvm_cmd_run(int argc, const char **a
vidmode = 0;
memset(real_cmdline, 0, sizeof(real_cmdline));
- strcpy(real_cmdline, "notsc noapic noacpi pci=conf1 reboot=k panic=1
i8042.direct=1 "
+ strcpy(real_cmdline, "noapic noacpi pci=conf1 reboot=k panic=1
i8042.direct=1 "
"i8042.dumbkbd=1 i8042.nopnp=1");
if (vnc || sdl) {
strcat(real_cmdline, " video=vesafb console=tty0");
--
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: kvm tools: clock sources for hrtimer
On Wed, Nov 09, 2011 at 08:00:06PM +0400, Cyrill Gorcunov wrote: ... > > > > You'll need CONFIG_KVM_CLOCK. > > > > I'm not actually sure how close our implementation is to having tsc > > working so far, Cyrill knows more about that than me. > > > > We dropped tsc while were debuggin timer interrupts and apic routing > setup, it might be not needed already. (Still to be fair I'm not sure > does kvm hypervisor has a control bit set for tsc and cause or not vm-exit). > > In short -- you could drop this from command line and tell us how it goes ;) > The history shows the following commit | commit 513fa5b4ccba8f9a2270a4f5262433071456540b | Author: Pekka Enberg | Date: Sun Apr 11 20:55:56 2010 +0300 | |kvm: Force 'notsc' and 'earlyprintk' kernel parameters | |We don't support TSC calibration properly and we want early printk so force |them as kernel parameters. | |Signed-off-by: Pekka Enberg and as far as I remember it's because we were stuck at apic calibration, but now we have an apic from kvm hypervisor so I think it's safe to drop it now (but still should be tested more widely). Cyrill -- 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: kvm tools: clock sources for hrtimer
On Wed, Nov 09, 2011 at 05:49:53PM +0200, Sasha Levin wrote: > On Wed, 2011-11-09 at 17:42 +0200, Richard Weinberger wrote: > > On Wed, 09 Nov 2011 16:49:51 +0200, Sasha Levin > > wrote: > > > We'll do kvm_clock as well if you compile it in the kernel. > > > > CONFIG_HIGH_RES_TIMERS is on both host and guest kernels enabled. > > BTW: Why adds the kvm tool "notsc" to the guest's kernel command line? > > You'll need CONFIG_KVM_CLOCK. > > I'm not actually sure how close our implementation is to having tsc > working so far, Cyrill knows more about that than me. > We dropped tsc while were debuggin timer interrupts and apic routing setup, it might be not needed already. (Still to be fair I'm not sure does kvm hypervisor has a control bit set for tsc and cause or not vm-exit). In short -- you could drop this from command line and tell us how it goes ;) Cyrill -- 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: [PATCH 4/5] kvm tools: Teach 'run' to handle guestfs
On Tue, Sep 06, 2011 at 02:23:54AM +0300, Sasha Levin wrote:
> This patch allows to run previously created guestfs by simply specifying it
> with the '-d' parameter.
>
> This allows running guestfs which were created before using:
>
> kvm setup -n [name]
>
> Signed-off-by: Sasha Levin
> ---
...
> @@ -103,6 +104,7 @@ static int img_name_parser(const struct option *opt,
> const char *arg, int unset)
> {
> char *sep;
> struct stat st;
> + char path[PATH_MAX];
>
Hi Sasha, the whole series looks good to me, thanks a lot!
The only thing which was always bothering me -- is PATH_MAX on the stack.
As far as I remember it might be up to 4K which is not that good ;) Probably
we might move it somewhere into .bss? (in some patches on top)
Cyrill
--
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: [PATCH 1/4] kvm tools: Use GSI routing
On Thu, Jul 28, 2011 at 12:01:52PM +0300, Sasha Levin wrote: > Map GSIs manually when starting the guest. > This will allow us mapping new GSIs for MSIX in the future. > > Signed-off-by: Sasha Levin > --- Other than a few nits the series looks good to me, thanks Sasha! -- 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: [PATCH 3/4] kvm tools: Add a void ptr to be passed to mmio callback
On Thu, Jul 28, 2011 at 12:01:54PM +0300, Sasha Levin wrote:
...
>
> struct mmio_mapping {
> struct rb_int_node node;
> - void(*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32
> len, u8 is_write);
> + void(*kvm_mmio_callback_fn)(u64 addr, u8 *data, u32
> len, u8 is_write, void *ptr);
> + void*ptr;
> };
I guess no need to name it *that* long, probably simple
struct mmio_mapping {
struct rb_int_node node;
void(*mmio_fn)(u64 addr, u8 *data, u32 len, u8
is_write, void *ptr);
void*ptr;
};
...
>
> if (mmio)
> - mmio->kvm_mmio_callback_fn(phys_addr, data, len, is_write);
> + mmio->kvm_mmio_callback_fn(phys_addr, data, len, is_write,
> mmio->ptr);
So this would be
if (mmio)
mmio->mmio_fn(phys_addr, data, len, is_write, mmio->ptr);
no?
Cyrill
--
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: [PATCH 2/4] kvm tools: Fix PCI probing
On Thu, Jul 28, 2011 at 12:31:51PM +0300, Pekka Enberg wrote:
> On Thu, Jul 28, 2011 at 12:01 PM, Sasha Levin wrote:
> > PCI BAR probing is done in four steps:
> >
> > 1. Read address (and flags).
> > 2. Mask BAR.
> > 3. Read BAR again - Now the expected result is the size of the BAR.
> > 4. Mask BAR with address.
> >
> > So far, we have only took care of the first step. This means that the kernel
> > was using address as the size, causing a PCI allocation blunder.
> >
> > This patch fixes the issue by passing a proper size after masking.
> >
> > Signed-off-by: Sasha Levin
> > ---
> > tools/kvm/include/kvm/pci.h | 1 +
> > tools/kvm/pci.c | 57
> > +++
> > 2 files changed, 53 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/kvm/include/kvm/pci.h b/tools/kvm/include/kvm/pci.h
> > index 6ad4426..a7532e3 100644
> > --- a/tools/kvm/include/kvm/pci.h
> > +++ b/tools/kvm/include/kvm/pci.h
> > @@ -51,5 +51,6 @@ struct pci_device_header {
> >
> > void pci__init(void);
> > void pci__register(struct pci_device_header *dev, u8 dev_num);
> > +u32 pci_get_io_space_block(void);
>
> s/pci_get_io_space_block/pci__get_io_space_block/
>
Pekka, can we drop this idea with double underscopes? iirc perf is about
to drop them too.
--
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: [patch 2/5] kvm tools: Introduce vidmode parmeter
On Wed, Jun 08, 2011 at 12:10:30AM +0400, Cyrill Gorcunov wrote:
> On Tue, Jun 07, 2011 at 10:53:28PM +0300, Pekka Enberg wrote:
> > On Tue, 7 Jun 2011, Cyrill Gorcunov wrote:
> > >Usually this might be set by loader but since
> > >we're the loader lets allow to specify vesa
> > >mode as well.
> > >
> > >Signed-off-by: Cyrill Gorcunov
> >
> > This patch causes 'make check' to go crazy and print out bunch of these:
> >
>
> Pekka, are you sure it's because of _this_ particular patch?
>
> Cyrill
This one should do the trick, cant say I like it, we probably need some
default values from options parser, ie to extend it.
Cyrill
---
kvm tools: Introduce vidmode parmeter v2
Usually this might be set by loader but since
we're the loader lets allow to specify vesa
mode as well.
v2: Pekka spotted the default value was being compromised,
so revert it back and set only if specified.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/kvm-run.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
Index: linux-2.6.git/tools/kvm/kvm-run.c
===
--- linux-2.6.git.orig/tools/kvm/kvm-run.c
+++ linux-2.6.git/tools/kvm/kvm-run.c
@@ -80,6 +80,7 @@ extern int active_console;
bool do_debug_print = false;
static int nrcpus;
+static int vidmode = -1;
static const char * const run_usage[] = {
"kvm run [] []",
@@ -139,6 +140,10 @@ static const struct option options[] = {
OPT_STRING('\0', "tapscript", &script, "Script path",
"Assign a script to process created tap device"),
+ OPT_GROUP("BIOS options:"),
+ OPT_INTEGER('\0', "vidmode", &vidmode,
+ "Video mode"),
+
OPT_GROUP("Debug options:"),
OPT_BOOLEAN('\0', "debug", &do_debug_print,
"Enable debug messages"),
@@ -434,7 +439,6 @@ int kvm_cmd_run(int argc, const char **a
struct framebuffer *fb = NULL;
unsigned int nr_online_cpus;
int exit_code = 0;
- u16 vidmode = 0;
int max_cpus;
char *hi;
int i;
@@ -539,14 +543,22 @@ int kvm_cmd_run(int argc, const char **a
kvm->nrcpus = nrcpus;
+ /*
+* vidmode should be either specified
+* either set by default
+*/
+ if (vnc || sdl) {
+ if (vidmode == -1)
+ vidmode = 0x312;
+ } else
+ vidmode = 0;
+
memset(real_cmdline, 0, sizeof(real_cmdline));
strcpy(real_cmdline, "notsc noapic noacpi pci=conf1");
if (vnc || sdl) {
strcat(real_cmdline, " video=vesafb console=tty0");
- vidmode = 0x312;
- } else {
+ } else
strcat(real_cmdline, " console=ttyS0 earlyprintk=serial");
- }
strcat(real_cmdline, " ");
if (kernel_cmdline)
strlcat(real_cmdline, kernel_cmdline, sizeof(real_cmdline));
--
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: [patch 2/5] kvm tools: Introduce vidmode parmeter
On Tue, Jun 07, 2011 at 10:53:28PM +0300, Pekka Enberg wrote: > On Tue, 7 Jun 2011, Cyrill Gorcunov wrote: > >Usually this might be set by loader but since > >we're the loader lets allow to specify vesa > >mode as well. > > > >Signed-off-by: Cyrill Gorcunov > > This patch causes 'make check' to go crazy and print out bunch of these: > Pekka, are you sure it's because of _this_ particular patch? Cyrill -- 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: [patch 2/5] kvm tools: Introduce vidmode parmeter
On Tue, Jun 07, 2011 at 10:53:28PM +0300, Pekka Enberg wrote: > On Tue, 7 Jun 2011, Cyrill Gorcunov wrote: > >Usually this might be set by loader but since > >we're the loader lets allow to specify vesa > >mode as well. > > > >Signed-off-by: Cyrill Gorcunov > > This patch causes 'make check' to go crazy and print out bunch of these: > > Warning: Ignoring MMIO write at d0031f40 (length 4) > Hmm, weird... Cyrill -- 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
[patch 3/5] kvm tools: Delete dangling cursor from int10
Noone use it anymore. Also cleanup comment on int10 as well, int10_handler routine do all the hard work. Signed-off-by: Cyrill Gorcunov --- tools/kvm/bios/bios-rom.S | 14 +- 1 file changed, 1 insertion(+), 13 deletions(-) Index: linux-2.6.git/tools/kvm/bios/bios-rom.S === --- linux-2.6.git.orig/tools/kvm/bios/bios-rom.S +++ linux-2.6.git/tools/kvm/bios/bios-rom.S @@ -18,13 +18,7 @@ ENTRY(bios_intfake) ENTRY_END(bios_intfake) /* - * int 10 - video - write character and advance cursor (tty write) - * ah = 0eh - * al = character - * bh = display page (alpha modes) - * bl = foreground color (graphics modes) - * - * We ignore bx settings + * int 10 - video - service */ ENTRY(bios_int10) pushw %fs @@ -55,12 +49,6 @@ ENTRY(bios_int10) popw%fs IRET - - -/* - * private IRQ data - */ -cursor:.long 0 ENTRY_END(bios_int10) #define EFLAGS_CF (1 << 0) -- 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
[patch 2/5] kvm tools: Introduce vidmode parmeter
Usually this might be set by loader but since
we're the loader lets allow to specify vesa
mode as well.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/kvm-run.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
Index: linux-2.6.git/tools/kvm/kvm-run.c
===
--- linux-2.6.git.orig/tools/kvm/kvm-run.c
+++ linux-2.6.git/tools/kvm/kvm-run.c
@@ -80,6 +80,7 @@ extern int active_console;
bool do_debug_print = false;
static int nrcpus;
+static int vidmode = 0x312;
static const char * const run_usage[] = {
"kvm run [] []",
@@ -139,6 +140,10 @@ static const struct option options[] = {
OPT_STRING('\0', "tapscript", &script, "Script path",
"Assign a script to process created tap device"),
+ OPT_GROUP("BIOS options:"),
+ OPT_INTEGER('\0', "vidmode", &vidmode,
+ "Video mode"),
+
OPT_GROUP("Debug options:"),
OPT_BOOLEAN('\0', "debug", &do_debug_print,
"Enable debug messages"),
@@ -434,7 +439,6 @@ int kvm_cmd_run(int argc, const char **a
struct framebuffer *fb = NULL;
unsigned int nr_online_cpus;
int exit_code = 0;
- u16 vidmode = 0;
int max_cpus;
char *hi;
int i;
@@ -541,12 +545,10 @@ int kvm_cmd_run(int argc, const char **a
memset(real_cmdline, 0, sizeof(real_cmdline));
strcpy(real_cmdline, "notsc noapic noacpi pci=conf1");
- if (vnc || sdl) {
+ if (vnc || sdl)
strcat(real_cmdline, " video=vesafb console=tty0");
- vidmode = 0x312;
- } else {
+ else
strcat(real_cmdline, " console=ttyS0 earlyprintk=serial");
- }
strcat(real_cmdline, " ");
if (kernel_cmdline)
strlcat(real_cmdline, kernel_cmdline, sizeof(real_cmdline));
--
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
[patch 1/5] kvm tools: Options parser to handle hex numbers
Some kernel parameters are convenient if passed in
hex form so our options parser should handle even
such form of input.
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/util/parse-options.c | 102 -
1 file changed, 82 insertions(+), 20 deletions(-)
Index: linux-2.6.git/tools/kvm/util/parse-options.c
===
--- linux-2.6.git.orig/tools/kvm/util/parse-options.c
+++ linux-2.6.git/tools/kvm/util/parse-options.c
@@ -39,6 +39,84 @@ static int get_arg(struct parse_opt_ctx_
return 0;
}
+#define numvalue(c)\
+ ((c) >= 'a' ? (c) - 'a' + 10 : \
+(c) >= 'A' ? (c) - 'A' + 10 : (c) - '0')
+
+static u64 readhex(const char *str, bool *error)
+{
+ char *pos = strchr(str, 'x') + 1;
+ u64 res = 0;
+
+ while (*pos) {
+ unsigned int v = numvalue(*pos);
+ if (v > 16) {
+ *error = true;
+ return 0;
+ }
+
+ res = (res * 16) + v;
+ pos++;
+ }
+
+ *error = false;
+ return res;
+}
+
+static int readnum(const struct option *opt, int flags,
+ const char *str, char **end)
+{
+ if (strchr(str, 'x')) {
+ bool error;
+ u64 value;
+
+ value = readhex(str, &error);
+ if (error)
+ goto enotnum;
+
+ switch (opt->type) {
+ case OPTION_INTEGER:
+ *(int *)opt->value = value;
+ break;
+ case OPTION_UINTEGER:
+ *(unsigned int *)opt->value = value;
+ break;
+ case OPTION_LONG:
+ *(long *)opt->value = value;
+ break;
+ case OPTION_U64:
+ *(u64 *)opt->value = value;
+ break;
+ default:
+ goto invcall;
+ }
+ } else {
+ switch (opt->type) {
+ case OPTION_INTEGER:
+ *(int *)opt->value = strtol(str, end, 10);
+ break;
+ case OPTION_UINTEGER:
+ *(unsigned int *)opt->value = strtol(str, end, 10);
+ break;
+ case OPTION_LONG:
+ *(long *)opt->value = strtol(str, end, 10);
+ break;
+ case OPTION_U64:
+ *(u64 *)opt->value = strtoull(str, end, 10);
+ break;
+ default:
+ goto invcall;
+ }
+ }
+
+ return 0;
+
+enotnum:
+ return opterror(opt, "expects a numerical value", flags);
+invcall:
+ return opterror(opt, "invalid numeric conversion", flags);
+}
+
static int get_value(struct parse_opt_ctx_t *p,
const struct option *opt, int flags)
{
@@ -131,11 +209,7 @@ static int get_value(struct parse_opt_ct
}
if (get_arg(p, opt, flags, &arg))
return -1;
- *(int *)opt->value = strtol(arg, (char **)&s, 10);
- if (*s)
- return opterror(opt, "expects a numerical value",
- flags);
- return 0;
+ return readnum(opt, flags, arg, (char **)&s);
case OPTION_UINTEGER:
if (unset) {
@@ -148,11 +222,7 @@ static int get_value(struct parse_opt_ct
}
if (get_arg(p, opt, flags, &arg))
return -1;
- *(unsigned int *)opt->value = strtol(arg, (char **)&s, 10);
- if (*s)
- return opterror(opt,
- "expects a numerical value", flags);
- return 0;
+ return readnum(opt, flags, arg, (char **)&s);
case OPTION_LONG:
if (unset) {
@@ -165,11 +235,7 @@ static int get_value(struct parse_opt_ct
}
if (get_arg(p, opt, flags, &arg))
return -1;
- *(long *)opt->value = strtol(arg, (char **)&s, 10);
- if (*s)
- return opterror(opt,
- "expects a numerical value", flags);
- return 0;
+ return readnum(opt, flags, arg, (char **)&s);
case OPTION_U64:
if (unset) {
@@ -182,11 +248,7 @@ static int get_value(struct parse_opt_ct
}
[patch 5/5] kvm tools: Reform bios make fules
Put bios code into bios.s and adjust makefile rules accordingly. It's more natural than bios-rom.S (which is now simply a container over real bios code). Also improve bios deps in Makefile. Signed-off-by: Cyrill Gorcunov --- tools/kvm/Makefile| 29 +++- tools/kvm/bios/bios-rom.S | 95 +++--- tools/kvm/bios/bios.S | 95 ++ tools/kvm/bios/gen-offsets.sh |3 - 4 files changed, 115 insertions(+), 107 deletions(-) Index: linux-2.6.git/tools/kvm/Makefile === --- linux-2.6.git.orig/tools/kvm/Makefile +++ linux-2.6.git/tools/kvm/Makefile @@ -82,7 +82,7 @@ DEPS := $(patsubst %.o,%.d,$(OBJS)) # Exclude BIOS object files from header dependencies. OBJS += bios.o -OBJS += bios/bios.o +OBJS += bios/bios-rom.o LIBS += -lrt LIBS += -lpthread @@ -165,20 +165,27 @@ BIOS_CFLAGS += -m32 BIOS_CFLAGS += -march=i386 BIOS_CFLAGS += -mregparm=3 -bios.o: bios/bios-rom.bin -bios/bios.o: bios/bios.S bios/bios-rom.bin - $(E) " CC " $@ - $(Q) $(CC) -c $(CFLAGS) bios/bios.S -o bios/bios.o - -bios/bios-rom.bin: bios/bios-rom.S bios/e820.c - $(E) " CC " $@ +bios.o: bios/bios.bin bios/bios-rom.h + +bios/bios.bin.elf: bios/bios.S bios/e820.c bios/int10.c bios/rom.ld.S + $(E) " CC bios/e820.o" $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c -s bios/e820.c -o bios/e820.o + $(E) " CC bios/int10.o" $(Q) $(CC) -include code16gcc.h $(CFLAGS) $(BIOS_CFLAGS) -c -s bios/int10.c -o bios/int10.o - $(Q) $(CC) $(CFLAGS) $(BIOS_CFLAGS) -c -s bios/bios-rom.S -o bios/bios-rom.o + $(E) " CC bios/bios.o" + $(Q) $(CC) $(CFLAGS) $(BIOS_CFLAGS) -c -s bios/bios.S -o bios/bios.o $(E) " LD " $@ - $(Q) ld -T bios/rom.ld.S -o bios/bios-rom.bin.elf bios/bios-rom.o bios/e820.o bios/int10.o + $(Q) ld -T bios/rom.ld.S -o bios/bios.bin.elf bios/bios.o bios/e820.o bios/int10.o + +bios/bios.bin: bios/bios.bin.elf $(E) " OBJCOPY " $@ - $(Q) objcopy -O binary -j .text bios/bios-rom.bin.elf bios/bios-rom.bin + $(Q) objcopy -O binary -j .text bios/bios.bin.elf bios/bios.bin + +bios/bios-rom.o: bios/bios-rom.S bios/bios.bin bios/bios-rom.h + $(E) " CC " $@ + $(Q) $(CC) -c $(CFLAGS) bios/bios-rom.S -o bios/bios-rom.o + +bios/bios-rom.h: bios/bios.bin.elf $(E) " NM " $@ $(Q) cd bios && sh gen-offsets.sh > bios-rom.h && cd .. Index: linux-2.6.git/tools/kvm/bios/bios-rom.S === --- linux-2.6.git.orig/tools/kvm/bios/bios-rom.S +++ linux-2.6.git/tools/kvm/bios/bios-rom.S @@ -1,89 +1,12 @@ -/* - * Our pretty trivial BIOS emulation - */ - -#include #include .org 0 - .code16gcc - -#include "macro.S" - -/* - * fake interrupt handler, nothing can be faster ever - */ -ENTRY(bios_intfake) - IRET -ENTRY_END(bios_intfake) - -/* - * int 10 - video - service - */ -ENTRY(bios_int10) - pushw %fs - pushl %es - pushl %edi - pushl %esi - pushl %ebp - pushl %esp - pushl %edx - pushl %ecx - pushl %ebx - pushl %eax - - movl%esp, %eax - /* this is way easier than doing it in assembly */ - /* just push all the regs and jump to a C handler */ - callint10_handler - - popl%eax - popl%ebx - popl%ecx - popl%edx - popl%esp - popl%ebp - popl%esi - popl%edi - popl%es - popw%fs - - IRET -ENTRY_END(bios_int10) - -#define EFLAGS_CF (1 << 0) - -ENTRY(bios_int15) - cmp $0xE820, %eax - jne 1f - - pushw %fs - - pushl %edx - pushl %ecx - pushl %edi - pushl %ebx - pushl %eax - - movl%esp, %eax # it's bioscall case - calle820_query_map - - popl%eax - popl%ebx - popl%edi - popl%ecx - popl%edx - - popw%fs - - /* Clear CF */ - andl$~EFLAGS_CF, 0x4(%esp) -1: - IRET -ENTRY_END(bios_int15) - -GLOBAL(__locals) - -#include "local.S" - -END(__locals) +#ifdef CONFIG_X86_64 + .code64 +#else + .code32 +#endif + +GLOBAL(bios_rom) + .incbin "bios/bios.bin" +END(bios_rom) Index: linux-2.6.git/tools/kvm/bios/bios.S === --- linux-2.6.git.orig/tools/kvm/bios/bios.S +++ linux-2.6.git/tools/kvm/bios/bios.S @@ -1,12 +1,89 @@ +/* + * Our pretty trivial BIOS emulation + */ + +#include #include .org 0 -#i
[patch 4/5] kvm tools: Get rid of spaces in ld script
Signed-off-by: Cyrill Gorcunov
---
tools/kvm/bios/rom.ld.S |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-2.6.git/tools/kvm/bios/rom.ld.S
===
--- linux-2.6.git.orig/tools/kvm/bios/rom.ld.S
+++ linux-2.6.git/tools/kvm/bios/rom.ld.S
@@ -11,7 +11,7 @@ PHDRS {
}
SECTIONS {
- . = 0;
- .text : { *(.text) } :text = 0x9090
+ . = 0;
+ .text : { *(.text) } :text = 0x9090
}
--
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
[patch 0/5] kvm tools: A few fixes
Nothing serious, please review. Thanks. Cyrill -- 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: [PATCH 2/2] kvm tools, vesa: Fix 'ah' access in int10_vesa()
On Mon, Jun 06, 2011 at 12:09:25PM +0300, Pekka Enberg wrote: ... > > Type conversion will do the work but having explicit masking is > > a way better I believe, at least it makes this code snippet notable. > > True but the patch description is bogus as it really doesn't _fix_ anything. > > Pekka Yup, I somehow missed 'fix' word in log, looked only in patch body, sorry. Cyrill -- 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: [PATCH 2/2] kvm tools, vesa: Fix 'ah' access in int10_vesa()
On Mon, Jun 06, 2011 at 11:28:56AM +0300, Avi Kivity wrote:
> On 06/03/2011 10:37 PM, Pekka Enberg wrote:
> >This patch fixes access to 'ah' in int10_vesa() by masking the high bits.
> >
> >@@ -131,7 +131,7 @@ static void int10_vesa(struct int10_args *args)
> > {
> > u8 al;
> >
> >-al = args->eax;
> >+al = args->eax& 0xff;
>
> Isn't this reading %al? And the high bits are masked by the type of
> the variable?
>
Type conversion will do the work but having explicit masking is
a way better I believe, at least it makes this code snippet notable.
Cyrill
--
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: [PATCH 1/2] kvm tools, vesa: Cleanup code in bios/int10.c
On Fri, Jun 03, 2011 at 10:37:03PM +0300, Pekka Enberg wrote: > This patch cleans up the code in bios/int10.c without changing functionality. > > Cc: Ingo Molnar > Cc: Cyrill Gorcunov > Cc: John Floren > Cc: Sasha Levin > Signed-off-by: Pekka Enberg > --- > tools/kvm/bios/int10.c | 88 +-- > 1 files changed, 47 insertions(+), 41 deletions(-) > Looks good to me, thanks Pekka. Verifying vesa data is still in my todo list, sorry for taking it too long :/ Cyrill -- 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: [PATCH 2/2] kvm tools, vesa: Fix 'ah' access in int10_vesa()
On Fri, Jun 03, 2011 at 10:37:04PM +0300, Pekka Enberg wrote: > This patch fixes access to 'ah' in int10_vesa() by masking the high bits. > > Cc: Ingo Molnar > Cc: Cyrill Gorcunov > Cc: John Floren > Cc: Sasha Levin > Signed-off-by: Pekka Enberg > --- ... Ack, thanks Pekka! Cyrill -- 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: [PATCH 5/5 V2] kvm tools: Initialize and use VESA and VNC
On 05/24/2011 12:37 PM, Paolo Bonzini wrote: > On 05/23/2011 01:38 PM, Ingo Molnar wrote: >> Later on even this could be removed: using section >> tricks we can put init functions into a section > > This is not kernel space, the C library provides a way to do that with > __attribute__((constructor))... > > Paolo Ingo meant to use the same trick as we do in kernel space for late initcalls. -- Cyrill -- 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
[PATCH] kvm tools: Drop unused vars from int10.c code
There is a couple of functions which defines 'ah' variable but
never use it in real so that gcc 4.6.x series does complain on
me as
CC bios/bios-rom.bin
bios/int10.c: In function ‘int10_putchar’:
bios/int10.c:86:9: error: variable ‘ah’ set but not used
[-Werror=unused-but-set-variable]
bios/int10.c: In function ‘int10_vesa’:
bios/int10.c:96:9: error: variable ‘ah’ set but not used
[-Werror=unused-but-set-variable]
cc1: all warnings being treated as errors
so get rid of them.
Signed-off-by: Cyrill Gorcunov
CC: Sasha Levin
---
tools/kvm/bios/int10.c |8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
Index: linux-2.6.git/tools/kvm/bios/int10.c
===
--- linux-2.6.git.orig/tools/kvm/bios/int10.c
+++ linux-2.6.git/tools/kvm/bios/int10.c
@@ -83,22 +83,18 @@ static inline void outb(unsigned short p
*/
static inline void int10_putchar(struct int10_args *args)
{
- u8 al, ah;
-
- al = args->eax & 0xFF;
- ah = (args->eax & 0xFF00) >> 8;
+ u8 al = args->eax & 0xFF;
outb(0x3f8, al);
}
static void int10_vesa(struct int10_args *args)
{
- u8 al, ah;
+ u8 al;
struct vesa_general_info *destination;
struct vminfo *vi;
al = args->eax;
- ah = args->eax >> 8;
switch (al) {
case 0:
--
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: [tip:tools/kvm] kvm tools: Add conditional compilation of symbol resolving
On 05/22/2011 03:00 PM, Ingo Molnar wrote: > > * Ingo Molnar wrote: > >> >> * tip-bot for Cyrill Gorcunov wrote: >> >>> diff --git a/tools/perf/feature-tests.mak >>> b/tools/kvm/config/feature-tests.mak >>> similarity index 83% >>> copy from tools/perf/feature-tests.mak >>> copy to tools/kvm/config/feature-tests.mak >> >> Btw, now that we have feature-tests.mak it would be nice to populate the >> checks >> for the various assumptions. >> >> One i recently ran into on a new box where i tried to install tools/kvm was: >> >> In file included from /usr/include/features.h:387:0, >> from /usr/include/unistd.h:26, >> from include/kvm/util.h:12, >> from bios/e820.c:5: >> /usr/include/gnu/stubs.h:7:27: fatal error: gnu/stubs-32.h: No such file or >> directory >> compilation terminated. >> make: *** [bios/bios-rom.bin] Error 1 >> >> that's a dependency on glibc-dev[el]. > > Another detail: on 64-bit hosts the dependency is on gibc-dev[el].i686, i.e. > the 32-bit package. > > Would it be simple to remove this dependency? It's not typically installed by > default on distros and it would be nice to make most of the kvm code build by > default almost everywhere. > > Thanks, > > Ingo I'll take a look if we really need it (note we need to compile 16bit code for bios blob so it might eventually be a problem ;) -- Cyrill -- 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: [tip:tools/kvm] kvm tools: Add conditional compilation of symbol resolving
On 05/22/2011 02:58 PM, Ingo Molnar wrote: > > * tip-bot for Cyrill Gorcunov wrote: > >> diff --git a/tools/perf/feature-tests.mak >> b/tools/kvm/config/feature-tests.mak >> similarity index 83% >> copy from tools/perf/feature-tests.mak >> copy to tools/kvm/config/feature-tests.mak > > Btw, now that we have feature-tests.mak it would be nice to populate the > checks > for the various assumptions. > > One i recently ran into on a new box where i tried to install tools/kvm was: > > In file included from /usr/include/features.h:387:0, > from /usr/include/unistd.h:26, > from include/kvm/util.h:12, > from bios/e820.c:5: > /usr/include/gnu/stubs.h:7:27: fatal error: gnu/stubs-32.h: No such file or > directory > compilation terminated. > make: *** [bios/bios-rom.bin] Error 1 > > that's a dependency on glibc-dev[el]. > > Thanks, > > Ingo Ouch. I've been hitting this lack of gnu/stubs-32.h on Fedora 15 too and eventually I had to install i686 packages for devel. I'll try to resolve this issue tonight, I suppose it's not absolutely urgent, right? -- Cyrill -- 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: [PATCH 2/2] kvm tools: Modify ioport to use interval rbtree
On 05/21/2011 04:08 PM, Cyrill Gorcunov wrote:
> On 05/21/2011 02:55 PM, Sasha Levin wrote:
> ...
>>>> void ioport__register(u16 port, struct ioport_operations *ops, int count)
>>>> {
>>>> - int i;
>>>> + struct ioport_entry *entry;
>>>>
>>>> - for (i = 0; i < count; i++)
>>>> - ioport_ops[port + i]= ops;
>>>> + entry = ioport_search(&ioport_tree, port);
>>>> + if (entry)
>>>> + rb_int_erase(&ioport_tree, &entry->node);
>>>> +
>>>
>>> Hi Sasha, if I understand this correct we're simply drop old
>>> registartion, right? I think
>>> it should not be like that, if one port get used for several
>>> drivers/purposes we need a
>>> ref-counting, but at moment I think we simply should not allow to
>>> re-register port without
>>> previously unregister it. Or I miss something?
>>
>> Currently we register some ports as dummy ports in the ioport
>> initialization, and re-register them once they get someone who can use
>> them (for example, serial device).
>>
>> Not allowing ports to re-register would mean we can't reassign ports to
>> serial console when the serial console module gets loaded.
>>
>
> Yup, my bad, drop my complain, thanks ;)
>
What about this one on top? Pekka?
--
kvm tools: Print out a warning on io-port re-registration
We only support re-registartion of dummy io-port operations
so anything other should yield a warning and been considered
harmful until inspected and confirmed.
Signed-off-by: Cyrill Gorcunov
---
Pekka, for some reason dropping the dummy io-ports for
serial console is not what I like. To be fair -- I can't
explain why, some gut feeling ;)
tools/kvm/ioport.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
Index: linux-2.6.git/tools/kvm/ioport.c
===
--- linux-2.6.git.orig/tools/kvm/ioport.c
+++ linux-2.6.git/tools/kvm/ioport.c
@@ -72,8 +72,17 @@ void ioport__register(u16 port, struct i
struct ioport_entry *entry;
entry = ioport_search(&ioport_tree, port);
- if (entry)
+ if (entry) {
+ /*
+* Only dummy io-port operations are supposed to be
+* re-registered, anything other should be considered
+* harmfull and issue warning until inspected and confirmed.
+*/
+ if (entry->ops != &dummy_read_write_ioport_ops &&
+ entry->ops != &dummy_write_only_ioport_ops)
+ pr_warning("Non-dummy ioport re-registered: %x", port);
rb_int_erase(&ioport_tree, &entry->node);
+ }
entry = malloc(sizeof(*entry));
if (entry == NULL)
--
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
[PATCH] kvm tools, 9p: Test for tuncation result
Without 'ret' usage I get | cyrill@sun kvm $ make | CC virtio/9p.o | virtio/9p.c: In function ‘virtio_p9_wstat’: | virtio/9p.c:448:6: error: variable ‘res’ set but not used [-Werror=unused-but-set-variable] | cc1: all warnings being treated as errors | make: *** [virtio/9p.o] Error 1 so add a basic check for ftruncate result, this eliminate warning and we might need to use 'res' status later in caller code. Signed-off-by: Cyrill Gorcunov CC: Sasha Levin --- Pekka, are you fine with 'kvm-tools,9p' prefix? tools/kvm/virtio/9p.c |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) Index: linux-2.6.git/tools/kvm/virtio/9p.c === --- linux-2.6.git.orig/tools/kvm/virtio/9p.c +++ linux-2.6.git/tools/kvm/virtio/9p.c @@ -445,7 +445,7 @@ static bool virtio_p9_wstat(struct p9_ms struct p9_twstat *twstat = (struct p9_twstat *)msg->msg; struct p9_str *str; struct p9_fid *fid = &p9dev.fids[twstat->fid]; - int res; + int res = 0; if (twstat->stat.length != -1UL) res = ftruncate(fid->fd, twstat->stat.length); @@ -472,7 +472,8 @@ static bool virtio_p9_wstat(struct p9_ms *outlen = VIRTIO_P9_HDR_LEN; set_p9msg_hdr(outmsg, *outlen, P9_RWSTAT, msg->tag); - return true; + + return res == 0; } static bool virtio_p9_remove(struct p9_msg *msg, u32 len, struct iovec *iov, u32 *outlen) -- 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: [PATCH 2/2] kvm tools: Modify ioport to use interval rbtree
On 05/21/2011 02:55 PM, Sasha Levin wrote:
...
>>> void ioport__register(u16 port, struct ioport_operations *ops, int count)
>>> {
>>> - int i;
>>> + struct ioport_entry *entry;
>>>
>>> - for (i = 0; i < count; i++)
>>> - ioport_ops[port + i]= ops;
>>> + entry = ioport_search(&ioport_tree, port);
>>> + if (entry)
>>> + rb_int_erase(&ioport_tree, &entry->node);
>>> +
>>
>> Hi Sasha, if I understand this correct we're simply drop old registartion,
>> right? I think
>> it should not be like that, if one port get used for several
>> drivers/purposes we need a
>> ref-counting, but at moment I think we simply should not allow to
>> re-register port without
>> previously unregister it. Or I miss something?
>
> Currently we register some ports as dummy ports in the ioport
> initialization, and re-register them once they get someone who can use
> them (for example, serial device).
>
> Not allowing ports to re-register would mean we can't reassign ports to
> serial console when the serial console module gets loaded.
>
Yup, my bad, drop my complain, thanks ;)
--
Cyrill
--
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: [PATCH 2/2] kvm tools: Modify ioport to use interval rbtree
On 05/21/2011 12:51 PM, Sasha Levin wrote:
> Currently the ioport implementation is based on a USHRT_MAX length
> array of ptrs to ioport_operations.
>
> Instead, use an interval rbtree to map the ioports to
> ioport_operations.
>
> Signed-off-by: Sasha Levin
> ---
...
> -static struct ioport_operations *ioport_ops[USHRT_MAX];
> -
> void ioport__register(u16 port, struct ioport_operations *ops, int count)
> {
> - int i;
> + struct ioport_entry *entry;
>
> - for (i = 0; i < count; i++)
> - ioport_ops[port + i]= ops;
> + entry = ioport_search(&ioport_tree, port);
> + if (entry)
> + rb_int_erase(&ioport_tree, &entry->node);
> +
Hi Sasha, if I understand this correct we're simply drop old registartion,
right? I think
it should not be like that, if one port get used for several drivers/purposes
we need a
ref-counting, but at moment I think we simply should not allow to re-register
port without
previously unregister it. Or I miss something?
--
Cyrill
--
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: [PATCH] kvm tools: Cleanup e820 code
On 05/20/2011 06:23 PM, Sasha Levin wrote: > Several cleanups in the patch: > - Use kernel headers for e820 types and definitions. > - A byte sized entry count for e820 enteries was used, > this should be dword sized. Update in-memory layout and > bios code to fix it. > - Use struct e820map to calculate offsets used by bios code. > > Signed-off-by: Sasha Levin > --- Thanks Sasha! Reviewed-by: Cyrill Gorcunov -- Cyrill -- 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: [PATCH] kvm tools: Default guest cpu count to host cpu count
On 05/19/2011 11:08 PM, Cyrill Gorcunov wrote:
...
>>
>> What prevents nr_online_cpus from being greater than KVM_NR_CPUS? Since
>> that latter is a #define, might want to change 'else if' to if there.
>>
>> David
>>
>
> Good catch! We should add a constraint here and limit it to KVM_NR_CPUS.
>
Heh, actually it get catched futher in code by
max_cpus = kvm__max_cpus(kvm);
if (nrcpus > max_cpus) {
printf(" # Limit the number of CPUs to %d\n", max_cpus);
kvm->nrcpus = max_cpus;
}
so no issue here (except that MP table can support a limited number of
cpus and for 32bit apic addressing we need ACPI support implemented I think).
--
Cyrill
--
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
