Re: [Qemu-devel] Re: [PATCH v5] virtio-9p: fix build on !CONFIG_UTIMENSAT
Ping. Maintainers, please tell me if still something is required for this patch before applying it. Thanks, H.Seto (2010/11/24 17:01), Jes Sorensen wrote: On 11/24/10 03:38, Hidetoshi Seto wrote: This patch introduce a fallback mechanism for old systems that do not support utimensat(). This fix build failure with following warnings: hw/virtio-9p-local.c: In function 'local_utimensat': hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat' hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat' and: hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod': hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this function) hw/virtio-9p.c:1410: error: (Each undeclared identifier is reported only once hw/virtio-9p.c:1410: error: for each function it appears in.) hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this function) hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod': hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this function) [NOTE: At this time virtio-9p is only user of utimensat(), and is available only when host is linux and CONFIG_VIRTFS is defined. So there are no similar warning for win32. Please provide a wrapper for win32 in oslib-win32.c if new user really requires it.] v5: - Allow fallback on runtime - Move qemu_utimensat() to oslib-posix.c - Rebased on latest qemu.git v4: - Use tv_now.tv_usec v3: - Use better alternative handling for UTIME_NOW/OMIT - Move qemu_utimensat() to cutils.c V2: - Introduce qemu_utimensat() Acked-by: Chris Wright chr...@sous-sol.org Acked-by: M. Mohan Kumar mo...@in.ibm.com Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com --- hw/virtio-9p-local.c |4 ++-- oslib-posix.c| 48 qemu-os-posix.h | 12 3 files changed, 62 insertions(+), 2 deletions(-) Hi Hidetoshi, This looks good to me! Acked-by: Jes Sorensen jes.soren...@redhat.com Cheers, Jes -- 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 v4] virtio-9p: fix build on !CONFIG_UTIMENSAT
(2010/11/18 17:02), Jes Sorensen wrote: On 11/18/10 01:41, Hidetoshi Seto wrote: This patch introduce a fallback mechanism for old systems that do not support utimensat(). This fix build failure with following warnings: hw/virtio-9p-local.c: In function 'local_utimensat': hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat' hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat' and: hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod': hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this function) hw/virtio-9p.c:1410: error: (Each undeclared identifier is reported only once hw/virtio-9p.c:1410: error: for each function it appears in.) hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this function) hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod': hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this function) v4: - Use tv_now.tv_usec - Rebased on latest qemu.git v3: - Use better alternative handling for UTIME_NOW/OMIT - Move qemu_utimensat() to cutils.c V2: - Introduce qemu_utimensat() Acked-by: Chris Wright chr...@sous-sol.org Acked-by: M. Mohan Kumar mo...@in.ibm.com Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com Hi Hidetoshi, I think the idea of the patch is good, but please move qemu_utimensat() to oslib-posix.c and provide a wrapper for oslib-win32.c. It is emulation for a system library function, so it doesn't belong in cutils.c, but rather in the oslib group. Unfortunately one fact is that I'm not familiar with win32 codes so I don't have any idea how the wrapper for win32 will be... If someone could kindly tell me about the win32 part, I could update this patch to v5, but even though I have no test environment for the new part :- Could we wait an incremental patch on this v4? Can somebody help me? Volunteers? Thanks, H.Seto -- 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 v4] virtio-9p: fix build on !CONFIG_UTIMENSAT
(2010/11/18 17:28), Philipp Hahn wrote:
Hello,
Am Donnerstag 18 November 2010 01:41:39 schrieb Hidetoshi Seto:
This patch introduce a fallback mechanism for old systems that do not
support utimensat(). This fix build failure with following warnings:
+#ifdef CONFIG_UTIMENSAT
+return utimensat(dirfd, path, times, flags);
+#else
+/* Fallback: use utimes() instead of utimensat() */
Since we also had a problem with utimestat() some time ago with Samba
http://lists.samba.org/archive/samba-technical/2010-November/074613.html
I'd like to comment on that:
Your have to be careful about compile-time-detection and runtime-detection:
If
you later run your utimestat()-enabled binary on an older kernel not
supporting that syscall, you get -1 as the return-value and errno=ENOSYS. So
even if you detected utimesatat() during compile-time, please always provide
a fallback for run-time.
This is less important for people compiling there own version of kvm, but is
essential for Linux distributions, since people often run newer kvm versions
on older kernels.
Hum, you have a good point.
Well, then I'll change it like:
-#ifdef CONFIG_UTIMENSAT
-return utimensat(dirfd, path, times, flags);
-#else
+{
+int ret = utimensat(dirfd, path, times, flags);
+if (ret != -1 || errno != ENOSYS) {
+return ret;
+}
+}
Thanks,
H.Seto
--
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 v4] virtio-9p: fix build on !CONFIG_UTIMENSAT
This patch introduce a fallback mechanism for old systems that do not
support utimensat(). This fix build failure with following warnings:
hw/virtio-9p-local.c: In function 'local_utimensat':
hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat'
hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat'
and:
hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod':
hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this function)
hw/virtio-9p.c:1410: error: (Each undeclared identifier is reported only once
hw/virtio-9p.c:1410: error: for each function it appears in.)
hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this function)
hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod':
hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this function)
v4:
- Use tv_now.tv_usec
- Rebased on latest qemu.git
v3:
- Use better alternative handling for UTIME_NOW/OMIT
- Move qemu_utimensat() to cutils.c
V2:
- Introduce qemu_utimensat()
Acked-by: Chris Wright chr...@sous-sol.org
Acked-by: M. Mohan Kumar mo...@in.ibm.com
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
cutils.c | 44
hw/virtio-9p-local.c |4 ++--
qemu-common.h| 10 ++
3 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/cutils.c b/cutils.c
index 28089aa..dbeb3f2 100644
--- a/cutils.c
+++ b/cutils.c
@@ -371,3 +371,47 @@ fail:
return retval;
}
+
+int qemu_utimensat(int dirfd, const char *path, const struct timespec *times,
+ int flags)
+{
+#ifdef CONFIG_UTIMENSAT
+return utimensat(dirfd, path, times, flags);
+#else
+/* Fallback: use utimes() instead of utimensat() */
+struct timeval tv[2], tv_now;
+struct stat st;
+int i;
+
+/* happy if special cases */
+if (times[0].tv_nsec == UTIME_OMIT times[1].tv_nsec == UTIME_OMIT) {
+return 0;
+}
+if (times[0].tv_nsec == UTIME_NOW times[1].tv_nsec == UTIME_NOW) {
+return utimes(path, NULL);
+}
+
+/* prepare for hard cases */
+if (times[0].tv_nsec == UTIME_NOW || times[1].tv_nsec == UTIME_NOW) {
+gettimeofday(tv_now, NULL);
+}
+if (times[0].tv_nsec == UTIME_OMIT || times[1].tv_nsec == UTIME_OMIT) {
+stat(path, st);
+}
+
+for (i = 0; i 2; i++) {
+if (times[i].tv_nsec == UTIME_NOW) {
+tv[i].tv_sec = tv_now.tv_sec;
+tv[i].tv_usec = tv_now.tv_usec;
+} else if (times[i].tv_nsec == UTIME_OMIT) {
+tv[i].tv_sec = (i == 0) ? st.st_atime : st.st_mtime;
+tv[i].tv_usec = 0;
+} else {
+tv[i].tv_sec = times[i].tv_sec;
+tv[i].tv_usec = times[i].tv_nsec / 1000;
+}
+}
+
+return utimes(path, tv[0]);
+#endif
+}
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index 0d52020..41603ea 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -480,9 +480,9 @@ static int local_chown(FsContext *fs_ctx, const char *path,
FsCred *credp)
}
static int local_utimensat(FsContext *s, const char *path,
- const struct timespec *buf)
+ const struct timespec *buf)
{
-return utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+return qemu_utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
}
static int local_remove(FsContext *ctx, const char *path)
diff --git a/qemu-common.h b/qemu-common.h
index b3957f1..f0b2c9d 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -150,6 +150,16 @@ int qemu_fls(int i);
int qemu_fdatasync(int fd);
int fcntl_setfl(int fd, int flag);
ssize_t strtosz(const char *nptr, char **end);
+#ifndef CONFIG_UTIMENSAT
+#ifndef UTIME_NOW
+# define UTIME_NOW ((1l 30) - 1l)
+#endif
+#ifndef UTIME_OMIT
+# define UTIME_OMIT((1l 30) - 2l)
+#endif
+#endif
+int qemu_utimensat(int dirfd, const char *path, const struct timespec *times,
+int flags);
/* path.c */
void init_paths(const char *prefix);
--
1.7.3.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: [Qemu-devel] Re: [PATCH] virtio-9p: fix build on !CONFIG_UTIMENSAT v2
(2010/11/14 14:58), Chris Wright wrote:
* Hidetoshi Seto (seto.hideto...@jp.fujitsu.com) wrote:
+/*
+ * Fallback: use utimes() instead of utimensat().
+ * See commit 74bc02b2d2272dc88fb98d43e631eb154717f517 for known
problem.
+ */
+struct timeval tv[2];
+int i;
+
+for (i = 0; i 2; i++) {
+if (times[i].tv_nsec == UTIME_OMIT || times[i].tv_nsec ==
UTIME_NOW) {
+tv[i].tv_sec = 0;
+tv[i].tv_usec = 0;
I don't think this is accurate in either case. It will set the
atime, mtime, or both to 0.
For UTIME_NOW (in both) we'd simply pass NULL to utimes(2). For
UTIME_OMIT (in both) we'd simply skip the call to utimes(2) altogether.
The harder part is a mixed mode (i.e. the truncate fix mentioned in the
above commit). I think the only way to handle UTIME_NOW in one is to
call gettimeofday (or clock_gettime for better resolution) to find out
what current time is. And for UTIME_OMIT call stat to find out what the
current setting is and reset to that value. Both of those cases can
possibly zero out the extra precision (providing only seconds
resolution).
Thank you for comments!
I'll post an updated one soon.
Thanks,
H.Seto
--
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 v3] virtio-9p: fix build on !CONFIG_UTIMENSAT
This patch introduce a fallback mechanism for old systems that do not
support utimensat(). This fix build failure with following warnings:
hw/virtio-9p-local.c: In function 'local_utimensat':
hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat'
hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat'
and:
hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod':
hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this function)
hw/virtio-9p.c:1410: error: (Each undeclared identifier is reported only once
hw/virtio-9p.c:1410: error: for each function it appears in.)
hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this function)
hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod':
hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this function)
v3:
- Use better alternative handling for UTIME_NOW/OMIT
- Move qemu_utimensat() to cutils.c
V2:
- Introduce qemu_utimensat()
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
cutils.c | 43 +++
hw/virtio-9p-local.c |4 ++--
qemu-common.h| 10 ++
3 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/cutils.c b/cutils.c
index 536ee93..3c18941 100644
--- a/cutils.c
+++ b/cutils.c
@@ -288,3 +288,46 @@ int fcntl_setfl(int fd, int flag)
}
#endif
+int qemu_utimensat(int dirfd, const char *path, const struct timespec *times,
+ int flags)
+{
+#ifdef CONFIG_UTIMENSAT
+return utimensat(dirfd, path, times, flags);
+#else
+/* Fallback: use utimes() instead of utimensat() */
+struct timeval tv[2], tv_now;
+struct stat st;
+int i;
+
+/* happy if special cases */
+if (times[0].tv_nsec == UTIME_OMIT times[1].tv_nsec == UTIME_OMIT) {
+return 0;
+}
+if (times[0].tv_nsec == UTIME_NOW times[1].tv_nsec == UTIME_NOW) {
+return utimes(path, NULL);
+}
+
+/* prepare for hard cases */
+if (times[0].tv_nsec == UTIME_NOW || times[1].tv_nsec == UTIME_NOW) {
+gettimeofday(tv_now, NULL);
+}
+if (times[0].tv_nsec == UTIME_OMIT || times[1].tv_nsec == UTIME_OMIT) {
+stat(path, st);
+}
+
+for (i = 0; i 2; i++) {
+if (times[i].tv_nsec == UTIME_NOW) {
+tv[i].tv_sec = tv_now.tv_sec;
+tv[i].tv_usec = 0;
+} else if (times[i].tv_nsec == UTIME_OMIT) {
+tv[i].tv_sec = (i == 0) ? st.st_atime : st.st_mtime;
+tv[i].tv_usec = 0;
+} else {
+tv[i].tv_sec = times[i].tv_sec;
+tv[i].tv_usec = times[i].tv_nsec / 1000;
+}
+}
+
+return utimes(path, tv[0]);
+#endif
+}
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index 0d52020..41603ea 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -480,9 +480,9 @@ static int local_chown(FsContext *fs_ctx, const char *path,
FsCred *credp)
}
static int local_utimensat(FsContext *s, const char *path,
- const struct timespec *buf)
+ const struct timespec *buf)
{
-return utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+return qemu_utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
}
static int local_remove(FsContext *ctx, const char *path)
diff --git a/qemu-common.h b/qemu-common.h
index 2fbc27f..7fe4c16 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -146,6 +146,16 @@ time_t mktimegm(struct tm *tm);
int qemu_fls(int i);
int qemu_fdatasync(int fd);
int fcntl_setfl(int fd, int flag);
+#ifndef CONFIG_UTIMENSAT
+#ifndef UTIME_NOW
+# define UTIME_NOW ((1l 30) - 1l)
+#endif
+#ifndef UTIME_OMIT
+# define UTIME_OMIT((1l 30) - 2l)
+#endif
+#endif
+int qemu_utimensat(int dirfd, const char *path, const struct timespec *times,
+int flags);
/* path.c */
void init_paths(const char *prefix);
--
1.7.3.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: [Qemu-devel] qemu-kvm build issue on RHEL5.1
(2010/11/05 2:03), Chris Wright wrote: * Hidetoshi Seto (seto.hideto...@jp.fujitsu.com) wrote: (2010/10/14 4:11), Blue Swirl wrote: On Wed, Oct 13, 2010 at 8:00 AM, Hidetoshi Seto seto.hideto...@jp.fujitsu.com wrote: (Add CC to k...@vger) (2010/10/12 10:52), Hao, Xudong wrote: Hi, Currently qemu-kvm build fail on RHEL5 with gcc 4.1.2, build can pass on Fedora11 with gcc 4.4.1, can anybody look on RHEL5 system? Gcc: 4.1.2 system: RHEL5.1 qemu-kvm: 85566812a4f8cae721fea0224e05a7e75c08c5dd ... LINK qemu-img LINK qemu-io CClibhw64/virtio-9p-local.o cc1: warnings being treated as errors /home/source/qemu-kvm/hw/virtio-9p-local.c: In function 'local_utimensat': /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat' /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat' make[1]: *** [virtio-9p-local.o] Error 1 make: *** [subdir-libhw64] Error 2 Best Regards, Xudong Hao It seems that this issue is caused by the old glibc. Though I don't know well about virtio-9p and suppose there should be better fix, I confirmed that following change removed the warnings. But then the system call will be made blindly without checking if the kernel supports utimensat(). At the minimum, there should be a sane response to ENOSYS error. Yes. But I'm not sure how this virtio-9p should behave if there is no utimensat. I think it will be better to fix this warning first to allow fellows using RHEL5 to restart contribute on qemu-kvm, and change this issue to virtio-9p specific problem to allow specialists of virtio-9p to have discussion for fix without bothering other developers. One way to workaround this is to simply not install libattr-devel (effecitvely disabling virtio-9p). But I agree with Blue Swirl, need a better fallback plan. A qemu local implementation of something like qemu_utimensat() that simply uses glibc/kernel interface when available and falls back to using utimes() makes sense to me. Then the worst case is loss of resolution from ns to us. According to the commit 74bc02b2d2272dc88fb98d43e631eb154717f517, the title Do not reset atime can tell us that the original motivation to use utimensat() is not for the resolution. Anyway, I agree to have something like qemu_utimensat(). I made a patch for the first step, and will post it next to this reply. Thanks, H.Seto -- 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] virtio-9p: fix build on !CONFIG_UTIMENSAT v2
This patch introduce a fallback mechanism for old systems that do not
support utimensat. This will fix build failure with following warnings:
hw/virtio-9p-local.c: In function 'local_utimensat':
hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat'
hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat'
and
hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod':
hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this function)
hw/virtio-9p.c:1410: error: (Each undeclared identifier is reported only once
hw/virtio-9p.c:1410: error: for each function it appears in.)
hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this function)
hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod':
hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this function)
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
hw/virtio-9p-local.c | 32 ++--
hw/virtio-9p.h | 10 ++
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index 0d52020..7811d2c 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -479,10 +479,38 @@ static int local_chown(FsContext *fs_ctx, const char
*path, FsCred *credp)
return -1;
}
+/* TODO: relocate this to proper file, and make it more generic */
+static int qemu_utimensat(int dirfd, const char *path,
+ const struct timespec *times, int flags)
+{
+#ifdef CONFIG_UTIMENSAT
+return utimensat(dirfd, path, times, flags);
+#else
+/*
+ * Fallback: use utimes() instead of utimensat().
+ * See commit 74bc02b2d2272dc88fb98d43e631eb154717f517 for known problem.
+ */
+struct timeval tv[2];
+int i;
+
+for (i = 0; i 2; i++) {
+if (times[i].tv_nsec == UTIME_OMIT || times[i].tv_nsec == UTIME_NOW) {
+tv[i].tv_sec = 0;
+tv[i].tv_usec = 0;
+} else {
+tv[i].tv_sec = times[i].tv_sec;
+tv[i].tv_usec = times[i].tv_nsec / 1000;
+}
+}
+
+return utimes(path, tv[0]);
+#endif
+}
+
static int local_utimensat(FsContext *s, const char *path,
- const struct timespec *buf)
+ const struct timespec *buf)
{
-return utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+return qemu_utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
}
static int local_remove(FsContext *ctx, const char *path)
diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h
index 6c23319..d448d8a 100644
--- a/hw/virtio-9p.h
+++ b/hw/virtio-9p.h
@@ -8,6 +8,16 @@
#include file-op-9p.h
+/* TODO: relocate this to proper file */
+#ifndef CONFIG_UTIMENSAT
+#ifndef UTIME_NOW
+# define UTIME_NOW ((1l 30) - 1l)
+#endif
+#ifndef UTIME_OMIT
+# define UTIME_OMIT((1l 30) - 2l)
+#endif
+#endif
+
/* The feature bitmap for virtio 9P */
/* The mount point is specified in a config variable */
#define VIRTIO_9P_MOUNT_TAG 0
--
1.7.3.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 1/5] PCI: MSI: Move MSI-X entry definition to pci_regs.h
Yeah, I think there are many virtualization stuff awaiting this change in these days. Reviewed-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com (2010/11/04 15:15), Sheng Yang wrote: Then it can be used by others. Reviewed-by: Matthew Wilcox wi...@linux.intel.com Cc: Jesse Barnes jbar...@virtuousgeek.org Cc: linux-...@vger.kernel.org Signed-off-by: Sheng Yang sh...@linux.intel.com --- drivers/pci/msi.h|6 -- include/linux/pci_regs.h |7 +++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h index de27c1c..28a3c52 100644 --- a/drivers/pci/msi.h +++ b/drivers/pci/msi.h @@ -6,12 +6,6 @@ #ifndef MSI_H #define MSI_H -#define PCI_MSIX_ENTRY_SIZE 16 -#define PCI_MSIX_ENTRY_LOWER_ADDR 0 -#define PCI_MSIX_ENTRY_UPPER_ADDR 4 -#define PCI_MSIX_ENTRY_DATA 8 -#define PCI_MSIX_ENTRY_VECTOR_CTRL 12 - #define msi_control_reg(base)(base + PCI_MSI_FLAGS) #define msi_lower_address_reg(base) (base + PCI_MSI_ADDRESS_LO) #define msi_upper_address_reg(base) (base + PCI_MSI_ADDRESS_HI) diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h index 455b9cc..acfc224 100644 --- a/include/linux/pci_regs.h +++ b/include/linux/pci_regs.h @@ -307,6 +307,13 @@ #define PCI_MSIX_FLAGS_MASKALL (1 14) #define PCI_MSIX_FLAGS_BIRMASK (7 0) +/* MSI-X entry's format */ +#define PCI_MSIX_ENTRY_SIZE 16 +#define PCI_MSIX_ENTRY_LOWER_ADDR 0 +#define PCI_MSIX_ENTRY_UPPER_ADDR 4 +#define PCI_MSIX_ENTRY_DATA 8 +#define PCI_MSIX_ENTRY_VECTOR_CTRL 12 + /* CompactPCI Hotswap Register */ #define PCI_CHSWP_CSR2 /* Control and Status Register */ -- 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] PCI: Add mask bit definition for MSI-X table
(2010/11/04 15:15), Sheng Yang wrote:
Then we can use it instead of magic number 1.
Cc: Matthew Wilcox wi...@linux.intel.com
Cc: Jesse Barnes jbar...@virtuousgeek.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Sheng Yang sh...@linux.intel.com
---
drivers/pci/msi.c|4 ++--
include/linux/pci_regs.h |1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 69b7be3..673e7dc 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -158,7 +158,7 @@ static u32 __msix_mask_irq(struct msi_desc *desc, u32
flag)
u32 mask_bits = desc-masked;
unsigned offset = desc-msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
PCI_MSIX_ENTRY_VECTOR_CTRL;
- mask_bits = ~1;
+ mask_bits = ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
mask_bits |= flag;
writel(mask_bits, desc-mask_base + offset);
@@ -185,7 +185,7 @@ static void msi_set_mask_bit(unsigned irq, u32 flag)
void mask_msi_irq(unsigned int irq)
{
- msi_set_mask_bit(irq, 1);
+ msi_set_mask_bit(irq, PCI_MSIX_ENTRY_CTRL_MASKBIT);
}
void unmask_msi_irq(unsigned int irq)
This hunk is not good, because the function msi_set_mask_bit() is
not only for MSI-X but also for MSI, so the number 0/1 here works
as like as false/true:
static void msi_set_mask_bit(struct irq_data *data, u32 flag)
{
struct msi_desc *desc = irq_data_get_msi(data);
if (desc-msi_attrib.is_msix) {
msix_mask_irq(desc, flag);
readl(desc-mask_base); /* Flush write to device */
} else {
unsigned offset = data-irq - desc-dev-irq;
msi_mask_irq(desc, 1 offset, flag offset);
}
}
diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h
index acfc224..ff51632 100644
--- a/include/linux/pci_regs.h
+++ b/include/linux/pci_regs.h
@@ -313,6 +313,7 @@
#define PCI_MSIX_ENTRY_UPPER_ADDR 4
#define PCI_MSIX_ENTRY_DATA 8
#define PCI_MSIX_ENTRY_VECTOR_CTRL 12
+#define PCI_MSIX_ENTRY_CTRL_MASKBIT1
/* CompactPCI Hotswap Register */
So I recommend you to have a patch with the above hunk for header
plus a change like:
- mask_bits = ~1;
- mask_bits |= flag;
+ mask_bits = ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
+ mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT !!flag;
Anyway thank you very much for doing this work.
Reviewed-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
Thanks,
H.Seto
--
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] Fix build on !KVM_CAP_MCE
This patch removes following warnings:
target-i386/kvm.c: In function 'kvm_put_msrs':
target-i386/kvm.c:782: error: unused variable 'i'
target-i386/kvm.c: In function 'kvm_get_msrs':
target-i386/kvm.c:1083: error: label at end of compound statement
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
target-i386/kvm.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 512d533..786eeeb 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -779,7 +779,7 @@ static int kvm_put_msrs(CPUState *env, int level)
struct kvm_msr_entry entries[100];
} msr_data;
struct kvm_msr_entry *msrs = msr_data.entries;
-int i, n = 0;
+int n = 0;
kvm_msr_entry_set(msrs[n++], MSR_IA32_SYSENTER_CS, env-sysenter_cs);
kvm_msr_entry_set(msrs[n++], MSR_IA32_SYSENTER_ESP, env-sysenter_esp);
@@ -801,6 +801,7 @@ static int kvm_put_msrs(CPUState *env, int level)
}
#ifdef KVM_CAP_MCE
if (env-mcg_cap) {
+int i;
if (level == KVM_PUT_RESET_STATE)
kvm_msr_entry_set(msrs[n++], MSR_MCG_STATUS, env-mcg_status);
else if (level == KVM_PUT_FULL_STATE) {
@@ -1085,9 +1086,9 @@ static int kvm_get_msrs(CPUState *env)
if (msrs[i].index = MSR_MC0_CTL
msrs[i].index MSR_MC0_CTL + (env-mcg_cap 0xff) * 4) {
env-mce_banks[msrs[i].index - MSR_MC0_CTL] = msrs[i].data;
-break;
}
#endif
+break;
}
}
--
1.6.5.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] x86, mce: ignore SRAO only when MCG_SER_P is available
And restruct this block to call kvm_mce_in_exception() only when it is
required.
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
target-i386/kvm.c | 16 ++--
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 786eeeb..a0d0603 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -239,12 +239,16 @@ static void kvm_do_inject_x86_mce(void *_data)
struct kvm_x86_mce_data *data = _data;
int r;
-/* If there is an MCE excpetion being processed, ignore this SRAO MCE */
-r = kvm_mce_in_exception(data-env);
-if (r == -1)
-fprintf(stderr, Failed to get MCE status\n);
-else if (r !(data-mce-status MCI_STATUS_AR))
-return;
+/* If there is an MCE exception being processed, ignore this SRAO MCE */
+if ((data-env-mcg_cap MCG_SER_P)
+!(data-mce-status MCI_STATUS_AR)) {
+r = kvm_mce_in_exception(data-env);
+if (r == -1) {
+fprintf(stderr, Failed to get MCE status\n);
+} else if (r) {
+return;
+}
+}
r = kvm_set_mce(data-env, data-mce);
if (r 0) {
--
1.6.5.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] x86, mce: broadcast mce depending on the cpu version
There is no reason why SRAO event received by the main thread
is the only one that being broadcasted.
According to the x86 ASDM vol.3A 15.10.4.1,
MCE signal is broadcast on processor version 06H_EH or later.
This change is required to handle SRAR in smp guests.
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
target-i386/kvm.c | 29 -
1 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index a0d0603..00bb083 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1637,6 +1637,28 @@ static void hardware_memory_error(void)
exit(1);
}
+#ifdef KVM_CAP_MCE
+static void kvm_mce_broadcast_rest(CPUState *env)
+{
+CPUState *cenv;
+int family, model, cpuver = env-cpuid_version;
+
+family = (cpuver 8) 0xf;
+model = ((cpuver 12) 0xf0) + ((cpuver 4) 0xf);
+
+/* Broadcast MCA signal for processor version 06H_EH and above */
+if ((family == 6 model = 14) || family 6) {
+if (cenv == env) {
+continue;
+}
+for (cenv = first_cpu; cenv != NULL; cenv = cenv-next_cpu) {
+kvm_inject_x86_mce(cenv, 1, MCI_STATUS_VAL | MCI_STATUS_UC,
+ MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0, 1);
+}
+}
+}
+#endif
+
int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
{
#if defined(KVM_CAP_MCE)
@@ -1694,6 +1716,7 @@ int kvm_on_sigbus_vcpu(CPUState *env, int code, void
*addr)
fprintf(stderr, kvm_set_mce: %s\n, strerror(errno));
abort();
}
+kvm_mce_broadcast_rest(env);
} else
#endif
{
@@ -1716,7 +1739,6 @@ int kvm_on_sigbus(int code, void *addr)
void *vaddr;
ram_addr_t ram_addr;
target_phys_addr_t paddr;
-CPUState *cenv;
/* Hope we are lucky for AO MCE */
vaddr = addr;
@@ -1732,10 +1754,7 @@ int kvm_on_sigbus(int code, void *addr)
kvm_inject_x86_mce(first_cpu, 9, status,
MCG_STATUS_MCIP | MCG_STATUS_RIPV, paddr,
(MCM_ADDR_PHYS 6) | 0xc, 1);
-for (cenv = first_cpu-next_cpu; cenv != NULL; cenv = cenv-next_cpu) {
-kvm_inject_x86_mce(cenv, 1, MCI_STATUS_VAL | MCI_STATUS_UC,
- MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0, 1);
-}
+kvm_mce_broadcast_rest(first_cpu);
} else
#endif
{
--
1.6.5.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
Re: [Qemu-devel] Re: [PATCH 11/11] kvm, x86: broadcast mce depending on the cpu version
(2010/10/15 22:30), Marcelo Tosatti wrote: On Fri, Oct 15, 2010 at 10:52:05AM +0900, Hidetoshi Seto wrote: (2010/10/15 10:06), Marcelo Tosatti wrote: On Thu, Oct 14, 2010 at 05:55:28PM +0900, Jin Dongming wrote: There is no reason why SRAO event received by the main thread is the only one that being broadcasted. According to the x86 ASDM vol.3A 15.10.4.1, MCE signal is broadcast on processor version 06H_EH or later. This change is required to handle SRAR in the guest. Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com Tested-by: Jin Dongming jin.dongm...@np.css.fujitsu.com --- qemu-kvm.c | 63 +-- 1 files changed, 31 insertions(+), 32 deletions(-) Why is this necessary? _AO SIGBUS should be sent to all vcpu threads and main thread. Humm? If you are right, vcpu threads will receive same SRAO event twice, one is that received by itself and another is that received by main thread and forwarded by the broadcast. My understanding is (Jin, please correct me if something wrong): - _AO SIGBUS is sent to main thread only, and then SRAO event is broadcasted to all vcpu threads. - _AR SIGBUS is sent to a vcpu thread that tried to touch the unmapped poisoned page, and SRAR event is posted to the vcpu. One problem here is that SRAR is not broadcasted. The guest might observe the event differently, like some cpus don't enter machine check. Right. Please separate bug fixes from cleanups. Very nice, thanks. Maybe this set is considered as 10 cleanups + 1 fix. I think this fix will be complicated one without preceding cleanups. Why? All you need is to broadcast from vcpu context. No, it is not correct. What I really need is reliable QEMU and maintainable source codes with open community. Anyway, since I found it could be simpler than what I expected, I rebased 2 functional change pieces in this set to today's uq/master. But these are not tested on the tree yet since I could not build the uq/master due to many warnings on it (even without my fixes). Please do a minimal fix separately so it can be backported, and the cleanups can be done later once its merged upstream. When it will be merged? Thanks, H.Seto -- 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 uq/master 1/2] kvm, x86: ignore SRAO only when MCG_SER_P is available
And restruct this block to call kvm_mce_in_exception() only when it is
required.
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
target-i386/kvm.c | 16 ++--
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index d940175..98a0505 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -239,12 +239,16 @@ static void kvm_do_inject_x86_mce(void *_data)
struct kvm_x86_mce_data *data = _data;
int r;
-/* If there is an MCE excpetion being processed, ignore this SRAO MCE */
-r = kvm_mce_in_exception(data-env);
-if (r == -1)
-fprintf(stderr, Failed to get MCE status\n);
-else if (r !(data-mce-status MCI_STATUS_AR))
-return;
+/* If there is an MCE exception being processed, ignore this SRAO MCE */
+if ((data-env-mcg_cap MCG_SER_P)
+!(data-mce-status MCI_STATUS_AR)) {
+r = kvm_mce_in_exception(data-env);
+if (r == -1) {
+fprintf(stderr, Failed to get MCE status\n);
+} else if (r) {
+return;
+}
+}
r = kvm_set_mce(data-env, data-mce);
if (r 0) {
--
1.7.3.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
[PATCH uq/master 2/2] kvm, x86: broadcast mce depending on the cpu version
There is no reason why SRAO event received by the main thread
is the only one that being broadcasted.
According to the x86 ASDM vol.3A 15.10.4.1,
MCE signal is broadcast on processor version 06H_EH or later.
This change is required to handle SRAR in smp guests.
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
target-i386/kvm.c | 28
1 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 98a0505..e97fbd3 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1631,6 +1631,28 @@ static void hardware_memory_error(void)
exit(1);
}
+#ifdef KVM_CAP_MCE
+static void kvm_mce_broadcast_rest(CPUState *env)
+{
+CPUState *cenv;
+int family, model, cpuver = env-cpuid_version;
+
+family = (cpuver 8) 0xf;
+model = ((cpuver 12) 0xf0) + ((cpuver 4) 0xf);
+
+/* Broadcast MCA signal for processor version 06H_EH and above */
+if ((family == 6 model = 14) || family 6) {
+if (cenv == env) {
+continue;
+}
+for (cenv = first_cpu; cenv != NULL; cenv = cenv-next_cpu) {
+kvm_inject_x86_mce(cenv, 1, MCI_STATUS_VAL | MCI_STATUS_UC,
+ MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0, 1);
+}
+}
+}
+#endif
+
int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
{
#if defined(KVM_CAP_MCE)
@@ -1688,6 +1710,7 @@ int kvm_on_sigbus_vcpu(CPUState *env, int code, void
*addr)
fprintf(stderr, kvm_set_mce: %s\n, strerror(errno));
abort();
}
+kvm_mce_broadcast_rest(env);
} else
#endif
{
@@ -1726,10 +1749,7 @@ int kvm_on_sigbus(int code, void *addr)
kvm_inject_x86_mce(first_cpu, 9, status,
MCG_STATUS_MCIP | MCG_STATUS_RIPV, paddr,
(MCM_ADDR_PHYS 6) | 0xc, 1);
-for (cenv = first_cpu-next_cpu; cenv != NULL; cenv = cenv-next_cpu) {
-kvm_inject_x86_mce(cenv, 1, MCI_STATUS_VAL | MCI_STATUS_UC,
- MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0, 1);
-}
+kvm_mce_broadcast_rest(first_cpu);
} else
#endif
{
--
1.7.3.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: [Qemu-devel] Re: [PATCH 07/11] kvm, x86: unify sigbus handling, prep
(2010/10/15 9:36), Marcelo Tosatti wrote: On Thu, Oct 14, 2010 at 05:49:43PM +0900, Jin Dongming wrote: There are 2 similar functions to handle SIGBUS: sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo, void *ctx) kvm_on_sigbus(CPUState *env, siginfo_t *siginfo) The former is used when main thread receives SIGBUS via signalfd, while latter is used when vcpu thread receives SIGBUS. These 2 take different siginfo, but in both case required parameters are common, the code and the addr in the info. Restruct functions to take the code and the addr explicitly. Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com Tested-by: Jin Dongming jin.dongm...@np.css.fujitsu.com --- qemu-kvm.c | 41 - 1 files changed, 20 insertions(+), 21 deletions(-) Don't see the benefit, separate functions are cleaner. I think this is good for maintainability. If you want to fix a bug in this area, you might have to change 2 separate functions in completely same way. See 6c85786 and a05684e for examples. Thanks, H.Seto -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Re: [PATCH 11/11] kvm, x86: broadcast mce depending on the cpu version
(2010/10/15 10:06), Marcelo Tosatti wrote: On Thu, Oct 14, 2010 at 05:55:28PM +0900, Jin Dongming wrote: There is no reason why SRAO event received by the main thread is the only one that being broadcasted. According to the x86 ASDM vol.3A 15.10.4.1, MCE signal is broadcast on processor version 06H_EH or later. This change is required to handle SRAR in the guest. Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com Tested-by: Jin Dongming jin.dongm...@np.css.fujitsu.com --- qemu-kvm.c | 63 +-- 1 files changed, 31 insertions(+), 32 deletions(-) Why is this necessary? _AO SIGBUS should be sent to all vcpu threads and main thread. Humm? If you are right, vcpu threads will receive same SRAO event twice, one is that received by itself and another is that received by main thread and forwarded by the broadcast. My understanding is (Jin, please correct me if something wrong): - _AO SIGBUS is sent to main thread only, and then SRAO event is broadcasted to all vcpu threads. - _AR SIGBUS is sent to a vcpu thread that tried to touch the unmapped poisoned page, and SRAR event is posted to the vcpu. One problem here is that SRAR is not broadcasted. The guest might observe the event differently, like some cpus don't enter machine check. Please separate bug fixes from cleanups. Very nice, thanks. Maybe this set is considered as 10 cleanups + 1 fix. I think this fix will be complicated one without preceding cleanups. Thanks, H.Seto -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] qemu-kvm build issue on RHEL5.1
(Add CC to k...@vger)
(2010/10/12 10:52), Hao, Xudong wrote:
Hi,
Currently qemu-kvm build fail on RHEL5 with gcc 4.1.2, build can pass on
Fedora11 with gcc 4.4.1, can anybody look on RHEL5 system?
Gcc: 4.1.2
system: RHEL5.1
qemu-kvm: 85566812a4f8cae721fea0224e05a7e75c08c5dd
...
LINK qemu-img
LINK qemu-io
CClibhw64/virtio-9p-local.o
cc1: warnings being treated as errors
/home/source/qemu-kvm/hw/virtio-9p-local.c: In function 'local_utimensat':
/home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: implicit declaration
of function 'utimensat'
/home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: nested extern
declaration of 'utimensat'
make[1]: *** [virtio-9p-local.o] Error 1
make: *** [subdir-libhw64] Error 2
Best Regards,
Xudong Hao
It seems that this issue is caused by the old glibc.
Though I don't know well about virtio-9p and suppose there
should be better fix, I confirmed that following change
removed the warnings.
Thanks,
H.Seto
=
[PATCH] virtio-9p: fix build on !CONFIG_UTIMENSAT
This removes following warnings on RHEL5, which has utimensat syscall
but has old glibc that doesn't have support for it:
hw/virtio-9p-local.c: In function 'local_utimensat':
hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat'
hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat'
and
hw/virtio-9p.c: In function 'v9fs_setattr_post_chmod':
hw/virtio-9p.c:1410: error: 'UTIME_NOW' undeclared (first use in this function)
hw/virtio-9p.c:1410: error: (Each undeclared identifier is reported only once
hw/virtio-9p.c:1410: error: for each function it appears in.)
hw/virtio-9p.c:1413: error: 'UTIME_OMIT' undeclared (first use in this function)
hw/virtio-9p.c: In function 'v9fs_wstat_post_chmod':
hw/virtio-9p.c:2905: error: 'UTIME_OMIT' undeclared (first use in this function)
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
hw/virtio-9p-local.c |8
hw/virtio-9p.c |9 +
2 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index 57f9243..e075c27 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -18,6 +18,9 @@
#include sys/socket.h
#include sys/un.h
#include attr/xattr.h
+#ifndef CONFIG_UTIMENSAT
+#include syscall.h
+#endif
static const char *rpath(FsContext *ctx, const char *path)
{
@@ -476,7 +479,12 @@ static int local_chown(FsContext *fs_ctx, const char
*path, FsCred *credp)
static int local_utimensat(FsContext *s, const char *path,
const struct timespec *buf)
{
+#ifndef CONFIG_UTIMENSAT
+return syscall(SYS_utimensat, AT_FDCWD, rpath(s, path), buf,
+ AT_SYMLINK_NOFOLLOW);
+#else
return utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+#endif
}
static int local_remove(FsContext *ctx, const char *path)
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 32fa3bc..efe5c51 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1393,6 +1393,15 @@ out:
qemu_free(vs);
}
+#ifndef CONFIG_UTIMENSAT
+#ifndef UTIME_NOW
+# define UTIME_NOW ((1l 30) - 1l)
+#endif
+#ifndef UTIME_OMIT
+# define UTIME_OMIT((1l 30) - 2l)
+#endif
+#endif
+
static void v9fs_setattr_post_chmod(V9fsState *s, V9fsSetattrState *vs, int
err)
{
if (err == -1) {
--
1.7.3.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
[PATCH] device-assignment: cleanup assigned_dev_ioport_map
Here we already have: AssignedDevRegion *region = r_dev-v_addrs[region_num]; Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com --- hw/device-assignment.c | 18 ++ 1 files changed, 6 insertions(+), 12 deletions(-) diff --git a/hw/device-assignment.c b/hw/device-assignment.c index 26cb797..975bf29 100644 --- a/hw/device-assignment.c +++ b/hw/device-assignment.c @@ -324,18 +324,12 @@ static void assigned_dev_ioport_map(PCIDevice *pci_dev, int region_num, kvm_ioperm(env, data); } -register_ioport_read(addr, size, 1, assigned_dev_ioport_readb, - (r_dev-v_addrs + region_num)); -register_ioport_read(addr, size, 2, assigned_dev_ioport_readw, - (r_dev-v_addrs + region_num)); -register_ioport_read(addr, size, 4, assigned_dev_ioport_readl, - (r_dev-v_addrs + region_num)); -register_ioport_write(addr, size, 1, assigned_dev_ioport_writeb, - (r_dev-v_addrs + region_num)); -register_ioport_write(addr, size, 2, assigned_dev_ioport_writew, - (r_dev-v_addrs + region_num)); -register_ioport_write(addr, size, 4, assigned_dev_ioport_writel, - (r_dev-v_addrs + region_num)); +register_ioport_read(addr, size, 1, assigned_dev_ioport_readb, region); +register_ioport_read(addr, size, 2, assigned_dev_ioport_readw, region); +register_ioport_read(addr, size, 4, assigned_dev_ioport_readl, region); +register_ioport_write(addr, size, 1, assigned_dev_ioport_writeb, region); +register_ioport_write(addr, size, 2, assigned_dev_ioport_writew, region); +register_ioport_write(addr, size, 4, assigned_dev_ioport_writel, region); } static uint32_t assigned_dev_pci_read(PCIDevice *d, int pos, int len) -- 1.7.3.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
[PATCH] device-assignment: introduce get_assigned_dev_id
Stop repeating same code.
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
hw/device-assignment.c | 25 +++--
1 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 975bf29..a2fa902 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -823,6 +823,11 @@ static uint32_t calc_assigned_dev_id(uint16_t seg, uint8_t
bus, uint8_t devfn)
return (uint32_t)seg 16 | (uint32_t)bus 8 | (uint32_t)devfn;
}
+static uint32_t get_assigned_dev_id(AssignedDevice *dev)
+{
+return calc_assigned_dev_id(dev-h_segnr, dev-h_busnr, dev-h_devfn);
+}
+
static void assign_failed_examine(AssignedDevice *dev)
{
char name[PATH_MAX], dir[PATH_MAX], driver[PATH_MAX] = {}, *ns;
@@ -886,8 +891,7 @@ static int assign_device(AssignedDevice *dev)
#endif
memset(assigned_dev_data, 0, sizeof(assigned_dev_data));
-assigned_dev_data.assigned_dev_id =
- calc_assigned_dev_id(dev-h_segnr, dev-h_busnr, dev-h_devfn);
+assigned_dev_data.assigned_dev_id = get_assigned_dev_id(dev);
#ifdef KVM_CAP_PCI_SEGMENT
assigned_dev_data.segnr = dev-h_segnr;
#endif
@@ -944,8 +948,7 @@ static int assign_irq(AssignedDevice *dev)
return r;
memset(assigned_irq_data, 0, sizeof(assigned_irq_data));
-assigned_irq_data.assigned_dev_id =
-calc_assigned_dev_id(dev-h_segnr, dev-h_busnr, dev-h_devfn);
+assigned_irq_data.assigned_dev_id = get_assigned_dev_id(dev);
assigned_irq_data.guest_irq = irq;
assigned_irq_data.host_irq = dev-real_device.irq;
#ifdef KVM_CAP_ASSIGN_DEV_IRQ
@@ -985,8 +988,7 @@ static void deassign_device(AssignedDevice *dev)
int r;
memset(assigned_dev_data, 0, sizeof(assigned_dev_data));
-assigned_dev_data.assigned_dev_id =
- calc_assigned_dev_id(dev-h_segnr, dev-h_busnr, dev-h_devfn);
+assigned_dev_data.assigned_dev_id = get_assigned_dev_id(dev);
r = kvm_deassign_pci_device(kvm_context, assigned_dev_data);
if (r 0)
@@ -1041,9 +1043,7 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev,
unsigned int ctrl_pos)
int r;
memset(assigned_irq_data, 0, sizeof assigned_irq_data);
-assigned_irq_data.assigned_dev_id =
-calc_assigned_dev_id(assigned_dev-h_segnr, assigned_dev-h_busnr,
-(uint8_t)assigned_dev-h_devfn);
+assigned_irq_data.assigned_dev_id = get_assigned_dev_id(assigned_dev);
/* Some guests gratuitously disable MSI even if they're not using it,
* try to catch this by only deassigning irqs if the guest is using
@@ -1133,8 +1133,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice
*pci_dev)
fprintf(stderr, MSI-X entry number is zero!\n);
return -EINVAL;
}
-msix_nr.assigned_dev_id = calc_assigned_dev_id(adev-h_segnr,
adev-h_busnr,
- (uint8_t)adev-h_devfn);
+msix_nr.assigned_dev_id = get_assigned_dev_id(adev);
msix_nr.entry_nr = entries_nr;
r = kvm_assign_set_msix_nr(kvm_context, msix_nr);
if (r != 0) {
@@ -1205,9 +1204,7 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev,
unsigned int ctrl_pos)
int r;
memset(assigned_irq_data, 0, sizeof assigned_irq_data);
-assigned_irq_data.assigned_dev_id =
-calc_assigned_dev_id(assigned_dev-h_segnr, assigned_dev-h_busnr,
-(uint8_t)assigned_dev-h_devfn);
+assigned_irq_data.assigned_dev_id = get_assigned_dev_id(assigned_dev);
/* Some guests gratuitously disable MSIX even if they're not using it,
* try to catch this by only deassigning irqs if the guest is using
--
1.7.3.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: [Qemu-devel] qemu-kvm build issue on RHEL5.1
(2010/10/14 4:11), Blue Swirl wrote: On Wed, Oct 13, 2010 at 8:00 AM, Hidetoshi Seto seto.hideto...@jp.fujitsu.com wrote: (Add CC to k...@vger) (2010/10/12 10:52), Hao, Xudong wrote: Hi, Currently qemu-kvm build fail on RHEL5 with gcc 4.1.2, build can pass on Fedora11 with gcc 4.4.1, can anybody look on RHEL5 system? Gcc: 4.1.2 system: RHEL5.1 qemu-kvm: 85566812a4f8cae721fea0224e05a7e75c08c5dd ... LINK qemu-img LINK qemu-io CClibhw64/virtio-9p-local.o cc1: warnings being treated as errors /home/source/qemu-kvm/hw/virtio-9p-local.c: In function 'local_utimensat': /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: implicit declaration of function 'utimensat' /home/source/qemu-kvm/hw/virtio-9p-local.c:479: warning: nested extern declaration of 'utimensat' make[1]: *** [virtio-9p-local.o] Error 1 make: *** [subdir-libhw64] Error 2 Best Regards, Xudong Hao It seems that this issue is caused by the old glibc. Though I don't know well about virtio-9p and suppose there should be better fix, I confirmed that following change removed the warnings. But then the system call will be made blindly without checking if the kernel supports utimensat(). At the minimum, there should be a sane response to ENOSYS error. Yes. But I'm not sure how this virtio-9p should behave if there is no utimensat. I think it will be better to fix this warning first to allow fellows using RHEL5 to restart contribute on qemu-kvm, and change this issue to virtio-9p specific problem to allow specialists of virtio-9p to have discussion for fix without bothering other developers. ... Or is it better to put abort() here instead of syscall? What happens if the system headers do not define SYS_utimensat? I suppose build will fail, say, we might need incremental patch named fix build on RHEL4 or so. But I have no idea, whether there could be a workaround if there is no utimensat, whether we could provide something like wrapper named compat_utimensat or qemu_utimensat, and/or whether it makes sense if virtio-9p have dependency with presence of utimensat. Thanks, H.Seto -- 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] Fix SRAO/SRAR MCE injecting on guest without MCG_SER_P
(2010/10/08 17:25), Huang Ying wrote:
On real machine, if MCG_SER_P in MSR_MCG_CAP is not set, SRAO/SRAR MCE
should not be raised by hardware. This patch makes QEMU-KVM to follow
the same rule.
Reported-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
Signed-off-by: Huang Ying ying.hu...@intel.com
---
qemu-kvm.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1134,7 +1134,7 @@ static void sigbus_handler(int n, struct
void *ctx)
{
#if defined(KVM_CAP_MCE) defined(TARGET_I386)
-if (first_cpu-mcg_cap siginfo-ssi_addr
+if ((first_cpu-mcg_cap MCG_SER_P) siginfo-ssi_addr
siginfo-ssi_code == BUS_MCEERR_AO) {
uint64_t status;
void *vaddr;
@@ -1324,7 +1324,7 @@ static void kvm_on_sigbus(CPUState *env,
unsigned long paddr;
int r;
-if (env-mcg_cap siginfo-si_addr
+if ((env-mcg_cap MCG_SER_P) siginfo-si_addr
(siginfo-si_code == BUS_MCEERR_AR
|| siginfo-si_code == BUS_MCEERR_AO)) {
if (siginfo-si_code == BUS_MCEERR_AR) {
@@ -1356,7 +1356,7 @@ static void kvm_on_sigbus(CPUState *env,
if (do_qemu_ram_addr_from_host(vaddr, ram_addr) ||
!kvm_physical_memory_addr_from_ram(kvm_state, ram_addr,
(target_phys_addr_t *)paddr)) {
fprintf(stderr, Hardware memory error for memory used by
-QEMU itself instaed of guest system!\n);
+QEMU itself instead of guest system!\n);
/* Hope we are lucky for AO MCE */
if (siginfo-si_code == BUS_MCEERR_AO) {
return;
Reviewed-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
Thanks,
H.Seto
--
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 uq/master 7/8] MCE: Relay UCR MCE to guest
Hi, Huang-san, (2010/10/08 12:15), Huang Ying wrote: Hi, Seto, On Thu, 2010-10-07 at 11:41 +0800, Hidetoshi Seto wrote: (2010/10/07 3:10), Dean Nelson wrote: On 10/06/2010 11:05 AM, Marcelo Tosatti wrote: On Wed, Oct 06, 2010 at 10:58:36AM +0900, Hidetoshi Seto wrote: I got some more question: (2010/10/05 3:54), Marcelo Tosatti wrote: Index: qemu/target-i386/cpu.h === --- qemu.orig/target-i386/cpu.h +++ qemu/target-i386/cpu.h @@ -250,16 +250,32 @@ #define PG_ERROR_RSVD_MASK 0x08 #define PG_ERROR_I_D_MASK 0x10 -#define MCG_CTL_P(1UL8) /* MCG_CAP register available */ +#define MCG_CTL_P(1ULL8) /* MCG_CAP register available */ +#define MCG_SER_P(1ULL24) /* MCA recovery/new status bits */ -#define MCE_CAP_DEFMCG_CTL_P +#define MCE_CAP_DEF(MCG_CTL_P|MCG_SER_P) #define MCE_BANKS_DEF10 It seems that current kvm doesn't support SER_P, so injecting SRAO to guest will mean that guest receives VAL|UC|!PCC and RIPV event from virtual processor that doesn't have SER_P. Dean also noted this. I don't think it was deliberate choice to not expose SER_P. Huang? In my testing, I found that MCG_SER_P was not being set (and I was running on a Nehalem-EX system). Injecting a MCE resulted in the guest entering into panic() from mce_panic(). If crash_kexec() finds a kexec_crash_image the system ends up rebooting, otherwise, what happens next requires operator intervention. Good to know. What I'm concerning is that if memory scrubbing SRAO event is injected when !SER_P, linux guest with certain mce tolerant level might grade it as UC severity and continue running with none of panicking, killing and poisoning because of !PCC and RIPV. Could you provide the panic message of the guest in your test? I think it can tell me why the mce handler decided to go panic. That is a bug that the SER_P is not in KVM_MCE_CAP_SUPPORTED in kernel. I will fix it as soon as possible. And SRAO MCE should not be sent when !SER_P, we should add that condition in qemu-kvm. That makes sense. I think it is qemu's responsibility for what follows the AO-SIGBUS, what action should be taken depends on the KVM's capability. When I applied a patch to the guest's kernel which forces mce_ser to be set, as if MCG_SER_P was set (see __mcheck_cpu_cap_init()), I found that when the memory page was 'owned' by a guest process, the process would be killed (if the page was dirty), and the guest would stay running. The HWPoisoned page would be sidelined and not cause any more issues. Excellent. So while guest kernel knows which page is poisoned, guest processes are controlled not to touch the page. ... Therefore rebooting the vm and renewing kernel will lost the information where is poisoned. Yes. That is an issue. Dean suggests that make qemu-kvm to refuse reboot the guest if there is poisoned page and ask for user to intervention. I have another idea to replace the poison pages with good pages when reboot, that is, recover without user intervention. Sounds good. I think it may be worth something to reserve pages for the replacement before reboot is requested; at least we really don't want to fail rebooting with 'no memory'. I think most OSes don't expect that it can receives MCE with !PCC on traditional x86 processor without SER_P. Q1: Is it safe to expect that guests can handle such !PCC event? This might be best answered by Huang, but as I mentioned above, without MCG_SER_P being set, the result was an orderly system panic on the guest. Though I'll wait Huang (I think he is on holiday), I believe that system panic is just a possible option for AO (Action Optional) event, no matter how the SER_P is. We should fix this as I said above. Q2: What is the expected behavior on the guest? I think I answered this above. Yeah, thanks. Q3: What happen if guest reboots itself in response to the MCE? That depends... And the following issue also holds for a guest that is rebooted at some point having successfully sidelined the bad page. After the guest has panic'd, a system_reset of the guest or a restart initiated by crash_kexec() (called by panic() on the guest), usually results in the guest hanging because the bad page still belongs to qemu-kvm and is now being referenced by the new guest in some way. Yes. In other words my concern about reboot is that new guest kernel including kdump kernel might try to read the bad page. If there is no AR-SIGBUS etc., we need some tricks to inhibit such accesses. (It actually may not hang, but successfully reboot and be runnable, with the bad page lurking in the background. It all seems to depend on where the bad page ends up, and whether it's ever referenced.) I know some tough guys using their PC with buggy DIMMs :-) I believe there was an attempt to deal with this in kvm on the host. See kvm_handle_bad_page
Re: [patch uq/master 7/8] MCE: Relay UCR MCE to guest
(2010/10/07 3:10), Dean Nelson wrote: On 10/06/2010 11:05 AM, Marcelo Tosatti wrote: On Wed, Oct 06, 2010 at 10:58:36AM +0900, Hidetoshi Seto wrote: I got some more question: (2010/10/05 3:54), Marcelo Tosatti wrote: Index: qemu/target-i386/cpu.h === --- qemu.orig/target-i386/cpu.h +++ qemu/target-i386/cpu.h @@ -250,16 +250,32 @@ #define PG_ERROR_RSVD_MASK 0x08 #define PG_ERROR_I_D_MASK 0x10 -#define MCG_CTL_P(1UL8) /* MCG_CAP register available */ +#define MCG_CTL_P(1ULL8) /* MCG_CAP register available */ +#define MCG_SER_P(1ULL24) /* MCA recovery/new status bits */ -#define MCE_CAP_DEFMCG_CTL_P +#define MCE_CAP_DEF(MCG_CTL_P|MCG_SER_P) #define MCE_BANKS_DEF10 It seems that current kvm doesn't support SER_P, so injecting SRAO to guest will mean that guest receives VAL|UC|!PCC and RIPV event from virtual processor that doesn't have SER_P. Dean also noted this. I don't think it was deliberate choice to not expose SER_P. Huang? In my testing, I found that MCG_SER_P was not being set (and I was running on a Nehalem-EX system). Injecting a MCE resulted in the guest entering into panic() from mce_panic(). If crash_kexec() finds a kexec_crash_image the system ends up rebooting, otherwise, what happens next requires operator intervention. Good to know. What I'm concerning is that if memory scrubbing SRAO event is injected when !SER_P, linux guest with certain mce tolerant level might grade it as UC severity and continue running with none of panicking, killing and poisoning because of !PCC and RIPV. Could you provide the panic message of the guest in your test? I think it can tell me why the mce handler decided to go panic. When I applied a patch to the guest's kernel which forces mce_ser to be set, as if MCG_SER_P was set (see __mcheck_cpu_cap_init()), I found that when the memory page was 'owned' by a guest process, the process would be killed (if the page was dirty), and the guest would stay running. The HWPoisoned page would be sidelined and not cause any more issues. Excellent. So while guest kernel knows which page is poisoned, guest processes are controlled not to touch the page. ... Therefore rebooting the vm and renewing kernel will lost the information where is poisoned. I think most OSes don't expect that it can receives MCE with !PCC on traditional x86 processor without SER_P. Q1: Is it safe to expect that guests can handle such !PCC event? This might be best answered by Huang, but as I mentioned above, without MCG_SER_P being set, the result was an orderly system panic on the guest. Though I'll wait Huang (I think he is on holiday), I believe that system panic is just a possible option for AO (Action Optional) event, no matter how the SER_P is. Q2: What is the expected behavior on the guest? I think I answered this above. Yeah, thanks. Q3: What happen if guest reboots itself in response to the MCE? That depends... And the following issue also holds for a guest that is rebooted at some point having successfully sidelined the bad page. After the guest has panic'd, a system_reset of the guest or a restart initiated by crash_kexec() (called by panic() on the guest), usually results in the guest hanging because the bad page still belongs to qemu-kvm and is now being referenced by the new guest in some way. Yes. In other words my concern about reboot is that new guest kernel including kdump kernel might try to read the bad page. If there is no AR-SIGBUS etc., we need some tricks to inhibit such accesses. (It actually may not hang, but successfully reboot and be runnable, with the bad page lurking in the background. It all seems to depend on where the bad page ends up, and whether it's ever referenced.) I know some tough guys using their PC with buggy DIMMs :-) I believe there was an attempt to deal with this in kvm on the host. See kvm_handle_bad_page(). This function was suppose to result in the sending of a BUS_MCEERR_AR flavored SIGBUS by do_sigbus() to qemu-kvm which in theory would result in the right thing happening. But commit 96054569190bdec375fe824e48ca1f4e3b53dd36 prevents the signal from being sent. So this mechanism needs to be re-worked, and the issue remains. Definitely. I guess Huang has some plan or hint for rework this point. I would think that if the the bad page can't be sidelined, such that the newly booting guest can't use it, then the new guest shouldn't be allowed to boot. But perhaps there is some merit in letting it try to boot and see if one gets 'lucky'. In case of booting a real machine in real world, hardware and firmware usually (or often) do self-test before passing control to OS. Some platform can boot OS with degraded configuration (for example, fewer memory) if it has trouble on its component. Some BIOS may stop booting and show messages like please reseat [component
Re: [patch uq/master 7/8] MCE: Relay UCR MCE to guest
(2010/10/05 3:54), Marcelo Tosatti wrote:
Port qemu-kvm's
commit 4b62fff1101a7ad77553147717a8bd3bf79df7ef
Author: Huang Ying ying.hu...@intel.com
Date: Mon Sep 21 10:43:25 2009 +0800
MCE: Relay UCR MCE to guest
UCR (uncorrected recovery) MCE is supported in recent Intel CPUs,
where some hardware error such as some memory error can be reported
without PCC (processor context corrupted). To recover from such MCE,
the corresponding memory will be unmapped, and all processes accessing
the memory will be killed via SIGBUS.
For KVM, if QEMU/KVM is killed, all guest processes will be killed
too. So we relay SIGBUS from host OS to guest system via a UCR MCE
injection. Then guest OS can isolate corresponding memory and kill
necessary guest processes only. SIGBUS sent to main thread (not VCPU
threads) will be broadcast to all VCPU threads as UCR MCE.
Signed-off-by: Marcelo Tosatti mtosa...@redhat.com
(snip)
+static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
+ void *ctx)
+{
+#if defined(TARGET_I386)
+if (kvm_on_sigbus_vcpu(siginfo-ssi_code, (void
*)(intptr_t)siginfo-ssi_addr))
+#endif
+sigbus_reraise();
+}
+
static void qemu_kvm_eat_signal(CPUState *env, int timeout)
{
struct timespec ts;
int r, e;
siginfo_t siginfo;
sigset_t waitset;
+sigset_t chkset;
ts.tv_sec = timeout / 1000;
ts.tv_nsec = (timeout % 1000) * 100;
sigemptyset(waitset);
sigaddset(waitset, SIG_IPI);
+sigaddset(waitset, SIGBUS);
-qemu_mutex_unlock(qemu_global_mutex);
-r = sigtimedwait(waitset, siginfo, ts);
-e = errno;
-qemu_mutex_lock(qemu_global_mutex);
+do {
+qemu_mutex_unlock(qemu_global_mutex);
-if (r == -1 !(e == EAGAIN || e == EINTR)) {
-fprintf(stderr, sigtimedwait: %s\n, strerror(e));
-exit(1);
-}
+r = sigtimedwait(waitset, siginfo, ts);
+e = errno;
+
+qemu_mutex_lock(qemu_global_mutex);
+
+if (r == -1 !(e == EAGAIN || e == EINTR)) {
+fprintf(stderr, sigtimedwait: %s\n, strerror(e));
+exit(1);
+}
+
+switch (r) {
+case SIGBUS:
+#ifdef TARGET_I386
+if (kvm_on_sigbus(env, siginfo.si_code, siginfo.si_addr))
+#endif
+sigbus_reraise();
+break;
+default:
+break;
+}
+
+r = sigpending(chkset);
+if (r == -1) {
+fprintf(stderr, sigpending: %s\n, strerror(e));
+exit(1);
+}
+} while (sigismember(chkset, SIG_IPI) || sigismember(chkset, SIGBUS));
}
static void qemu_kvm_wait_io_event(CPUState *env)
(snip)
Index: qemu/kvm.h
===
--- qemu.orig/kvm.h
+++ qemu/kvm.h
@@ -110,6 +110,9 @@ int kvm_arch_init_vcpu(CPUState *env);
void kvm_arch_reset_vcpu(CPUState *env);
+int kvm_on_sigbus(CPUState *env, int code, void *addr);
+int kvm_on_sigbus_vcpu(int code, void *addr);
+
struct kvm_guest_debug;
struct kvm_debug_exit_arch;
So kvm_on_sigbus() is called from qemu_kvm_eat_signal() that is
called on vcpu thread, while kvm_on_sigbus_vcpu() is called via
sigbus_handler that invoked on iothread using signalfd.
... Inverse naming?
Thanks,
H.Seto
--
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 uq/master 7/8] MCE: Relay UCR MCE to guest
I got some more question: (2010/10/05 3:54), Marcelo Tosatti wrote: Index: qemu/target-i386/cpu.h === --- qemu.orig/target-i386/cpu.h +++ qemu/target-i386/cpu.h @@ -250,16 +250,32 @@ #define PG_ERROR_RSVD_MASK 0x08 #define PG_ERROR_I_D_MASK 0x10 -#define MCG_CTL_P(1UL8) /* MCG_CAP register available */ +#define MCG_CTL_P(1ULL8) /* MCG_CAP register available */ +#define MCG_SER_P(1ULL24) /* MCA recovery/new status bits */ -#define MCE_CAP_DEF MCG_CTL_P +#define MCE_CAP_DEF (MCG_CTL_P|MCG_SER_P) #define MCE_BANKS_DEF10 It seems that current kvm doesn't support SER_P, so injecting SRAO to guest will mean that guest receives VAL|UC|!PCC and RIPV event from virtual processor that doesn't have SER_P. I think most OSes don't expect that it can receives MCE with !PCC on traditional x86 processor without SER_P. Q1: Is it safe to expect that guests can handle such !PCC event? Q2: What is the expected behavior on the guest? Q3: What happen if guest reboots itself in response to the MCE? Thanks, H.Seto -- 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: PCI passthru issue
(2010/09/17 8:01), Anirban Chakraborty wrote: Hi All, I noticed that the PCI registers are not all mapped to the guest for a PCI pass through device. For example, I was trying to pass through a PCI function (NIC) to a guest and in the guest the MSI-X table offset register is not mapped properly. It was showing all 0s. This is in RHEL 6.0 beta and the guest was a SLES11 64 bit. In the RHEL 6.0 host I see following for the PCI function: - 05:00.0 Ethernet controller: QLogic Corp. cLOM8214 1/10GbE Controller (rev 54) Subsystem: QLogic Corp. 8200 Series Dual Port 10GbE Converged Network Adapter (TCP/IP Networking) Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Interrupt: pin A routed to IRQ 74 Region 0: Memory at df20 (64-bit, non-prefetchable) [size=2M] Region 4: Memory at df1b (64-bit, non-prefetchable) [size=64K] Expansion ROM at df1c [disabled] [size=256K] Capabilities: [40] MSI-X: Enable- Count=32 Masked- Vector table: BAR=0 offset=0009 PBA: BAR=0 offset=00090800 ... Kernel driver in use: pci-stub 00: 77 10 20 80 42 05 10 00 54 00 00 02 40 00 80 00 10: 04 00 20 df 00 00 00 00 00 00 00 00 00 00 00 00 20: 04 00 1b df 00 00 00 00 00 00 00 00 77 10 07 02 30: 00 00 1c df 40 00 00 00 00 00 00 00 0a 01 00 00 40: 11 80 1f 00 00 00 09 00 00 08 09 00 00 00 00 00 -- And in the SLES11 guest, I see following for the same function: 00:07.0 Ethernet controller: QLogic Corp. Device 8020 (rev 54) Subsystem: QLogic Corp. Device 0207 Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Interrupt: pin A routed to IRQ 11 Region 0: Memory at f220 (32-bit, non-prefetchable) [size=2M] Region 4: Memory at f240 (32-bit, non-prefetchable) [size=64K] Expansion ROM at f244 [disabled] [size=256K] Capabilities: [40] Message Signalled Interrupts: Mask- 64bit- Count=1/1 Enable- Address: Data: Capabilities: [50] MSI-X: Enable- Mask- TabSize=32 Vector table: BAR=0 offset=0009 PBA: BAR=0 offset= 00: 77 10 20 80 42 05 10 00 54 00 00 02 40 00 80 00 10: 00 00 20 f2 00 00 00 00 00 00 00 00 00 00 00 00 20: 00 00 40 f2 00 00 00 00 00 00 00 00 77 10 07 02 30: 08 00 44 f2 40 00 00 00 00 00 00 00 0b 01 00 00 40: 05 50 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Note that the data at byte offset 40h is different for the host and the guest. Is this expected or is it a bug in KVM? This is expected, since MSIs of PCI pass through device need some emulation to route them to the guest instead of the host. So what we can see on the guest's PCI pass through device is modified capability list that contains emulated capabilities. Is there any spec out there that specifies what PCI registers are accessible from guest and how much of the PCI config space is mapped to the guest? This is helpful to know, especially in the context of SR-IOV. If a VF is pass throughed to a guest, are all the VF registers visible in host system will be mapped (and accessible) to the guest OS? AFAIK the QEMU source code, hw/device-assignment.c, will help you to know what are going on there. I'm not sure what interesting registers can the VF have. But I think you should be careful because the register in the PCI config space might not work like that on the host even the register is mapped and visible from the guest. Thanks, H.Seto -- 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] Use signed 16-bit values for ivshmem register writes
(2010/08/31 1:58), Cam Macdonell wrote: fixes gcc 4.1 warning Signed-off-by: Cam Macdonell c...@cs.ualberta.ca --- hw/ivshmem.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/ivshmem.c b/hw/ivshmem.c index bbb5cba..fa9c684 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -181,8 +181,8 @@ static void ivshmem_io_writel(void *opaque, target_phys_addr_t addr, IVShmemState *s = opaque; uint64_t write_one = 1; -uint16_t dest = val 16; -uint16_t vector = val 0xff; +int16_t dest = val 16; +int16_t vector = val 0xff; addr = 0xfc; Since val is uint32_t, I think this change doesn't make sense. Thanks, H.Seto -- 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] ivshmem: remove unnecessary checks for unsigned
fixes gcc 4.1 warning:
In function 'ivshmem_io_writel':
202: warning: comparison is always false due to limited range of data type
208: warning: comparison is always true due to limited range of data type
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
hw/ivshmem.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index bbb5cba..afebbc3 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -199,13 +199,13 @@ static void ivshmem_io_writel(void *opaque,
target_phys_addr_t addr,
case DOORBELL:
/* check that dest VM ID is reasonable */
-if ((dest 0) || (dest s-max_peer)) {
+if (dest s-max_peer) {
IVSHMEM_DPRINTF(Invalid destination VM ID (%d)\n, dest);
break;
}
/* check doorbell range */
-if ((vector = 0) (vector s-peers[dest].nb_eventfds)) {
+if (vector s-peers[dest].nb_eventfds) {
IVSHMEM_DPRINTF(Writing % PRId64 to VM %d on vector %d\n,
write_one, dest, vector);
if (write(s-peers[dest].eventfds[vector],
--
1.7.2.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
Re: [PATCH] ivshmem: remove unnecessary checks for unsigned
(2010/09/03 10:54), Hidetoshi Seto wrote:
fixes gcc 4.1 warning:
In function 'ivshmem_io_writel':
202: warning: comparison is always false due to limited range of data type
208: warning: comparison is always true due to limited range of data type
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
hw/ivshmem.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index bbb5cba..afebbc3 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -199,13 +199,13 @@ static void ivshmem_io_writel(void *opaque,
target_phys_addr_t addr,
case DOORBELL:
/* check that dest VM ID is reasonable */
-if ((dest 0) || (dest s-max_peer)) {
+if (dest s-max_peer) {
IVSHMEM_DPRINTF(Invalid destination VM ID (%d)\n, dest);
break;
}
/* check doorbell range */
-if ((vector = 0) (vector s-peers[dest].nb_eventfds)) {
+if (vector s-peers[dest].nb_eventfds) {
IVSHMEM_DPRINTF(Writing % PRId64 to VM %d on vector
%d\n,
write_one, dest, vector);
if (write(s-peers[dest].eventfds[vector],
Oops, since I've registered qemu-kvm ML but not qemu-devel ML, I could not
noticed that Jes have already posted same patch to qemu-devel.
Now build of ivshmem is enabled only on KVM systems, please apply this patch
to qemu-kvm.git asap.
Thanks,
H.Seto
--
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] device-assignment: Rework name of assigned pci device
(2010/06/30 15:53), Markus Armbruster wrote:
Summary: upstream qemu commit b560a9ab broke -pcidevice and pci_add host
in two ways:
* Use without options id and name is broken when option host contains
':'. That's because id defaults to host. I recommend to fix it
incompatibly: don't default id to host. The alternative is to get
upstream qemu to accept ':' in qdev IDs again.
* Funny characters in option name no longer work. Same as funny
characters in id options all over the place. Intentional change. I
recommend to do nothing.
Thanks a lot.
I'm not a person in really need, so now I'm going to follow your
recommendation.
Details inline.
Hidetoshi Seto seto.hideto...@jp.fujitsu.com writes:
Thanks Markus,
(2010/06/29 14:28), Markus Armbruster wrote:
Hidetoshi Seto seto.hideto...@jp.fujitsu.com writes:
Hao, Xudong xudong@intel.com writes:
When assign one PCI device, qemu fail to parse the command line:
qemu-system_x86 -smp 2 -m 1024 -hda /path/to/img -pcidevice host=00:19.0
Error:
qemu-system-x86_64: Parameter 'id' expects an identifier
Identifiers consist of letters, digits, '-', '.', '_', starting with a
letter.
pcidevice argument parse error; please check the help text for usage
Could not add assigned device host=00:19.0
https://bugs.launchpad.net/qemu/+bug/597932
This issue caused by qemu-kvm commit
b560a9ab9be06afcbb78b3791ab836dad208a239.
This patch is a response to the above report.
Thanks,
H.Seto
=
Because use of some characters in id is restricted recently, assigned
device start to fail having implicit id that uses address string of
host device, like 00:19.0 which includes restricted character ':'.
It seems that this implicit id is intended to be run as name that
could be passed with option -pcidevice ... ,name=... to specify a
string to be used in log outputs. In other words it seems that
dev-dev.qdev.id of assigned device had been used to have such
name, that is user-defined string or address string of host.
As far as I can tell, option name is just a leftover from pre-qdev
days, kept for compatibility.
Yea, I see.
I don't know well about the history of such pre-qdev days...
It's often useful to examine history to figure out what a piece of code
attempts to accomplish. git-blame and git-log are your friends :)
I often play with git-log, however I have a little trouble here since
qemu tree is too young.
The problem is that name for specific use is not equal to id for
universal use. So it is better to remove this tricky mix up here.
This patch introduces new function assigned_dev_name() that returns
proper name string for the device.
Now property name is explicitly defined in struct AssignedDevice.
When if the device have neither name nor id, address string like
:00:19.0 will be created and passed instead. Once created, new
field r_name holds the string to be reused and to be released later.
[...]
@@ -1520,6 +1545,7 @@ static PCIDeviceInfo assign_info = {
DEFINE_PROP(host, AssignedDevice, host, qdev_prop_hostaddr,
PCIHostDevice),
DEFINE_PROP_UINT32(iommu, AssignedDevice, use_iommu, 1),
DEFINE_PROP_STRING(configfd, AssignedDevice, configfd_name),
+DEFINE_PROP_STRING(name, AssignedDevice, u_name),
DEFINE_PROP_END_OF_LIST(),
},
};
@@ -1545,24 +1571,25 @@ device_init(assign_register_devices)
QemuOpts *add_assigned_device(const char *arg)
{
QemuOpts *opts = NULL;
-char host[64], id[64], dma[8];
+char host[64], buf[64], dma[8];
int r;
+/* host must be with -pcidevice */
r = get_param_value(host, sizeof(host), host, arg);
if (!r)
goto bad;
-r = get_param_value(id, sizeof(id), id, arg);
-if (!r)
-r = get_param_value(id, sizeof(id), name, arg);
-if (!r)
-r = get_param_value(id, sizeof(id), host, arg);
-opts = qemu_opts_create(qemu_device_opts, id, 0);
+opts = qemu_opts_create(qemu_device_opts, NULL, 0);
I think you break option id here. Its value must become the qdev ID,
visible in info qtree and usable as argument to device_del. And if
option id is missing, option name must become the qdev ID, for backwards
compatibility.
Ah, I missed to check hot-add path - I had wonder why id could be here
since I could not find documents that mentions use of id with -pcidevice.
So, I should use id here if specified. That's right,
But in case if id is missing and name is specified, I think there is no
reason that the characters in name should be restricted in same manner
with that in id.
In case that there is neither id nor name but host (in fact it is original
case) now ID BB:DD.F (like 00:19.0) will be rejected (because it starts
with digit, plus it uses restricted ':').
In other words, your change already breaks backwards compatibility because it
prevents -pcidevice host=BB:DD.F from creating ID BB:DD.F that might be
used
[PATCH] Add QMP/qmp-commands.txt to .gitignore
QMP/qmp-commands.txt is a generated file. Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com --- .gitignore |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index ce66ed5..a32b7c4 100644 --- a/.gitignore +++ b/.gitignore @@ -28,6 +28,7 @@ qemu-img-cmds.texi qemu-img-cmds.h qemu-io qemu-monitor.texi +QMP/qmp-commands.txt .gdbinit *.a *.aux -- 1.7.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] Add vapic.bin to .gitignore
# This patch is for qemu-kvm.git The vapic.bin is a generated binary file. Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com --- .gitignore |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index ddc248b..26eba20 100644 --- a/.gitignore +++ b/.gitignore @@ -53,4 +53,5 @@ pc-bios/optionrom/linuxboot.bin pc-bios/optionrom/multiboot.bin pc-bios/optionrom/multiboot.raw pc-bios/optionrom/extboot.bin +pc-bios/optionrom/vapic.bin .stgit-* -- 1.7.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: [Qemu-devel] [PATCH] QEMU: Update .gitignore
(2010/07/01 6:33), Aurelien Jarno wrote: On Mon, Jun 21, 2010 at 06:14:17PM +0900, Hidetoshi Seto wrote: (2010/06/21 17:19), Avi Kivity wrote: On 06/21/2010 08:24 AM, Hidetoshi Seto wrote: I think some people have noticed that: $ ./configure $ make $ git status # On branch master # Untracked files: # (use git add file... to include in what will be committed) # # QMP/qmp-commands.txt # libdis-user/ # libdis/ # pc-bios/optionrom/vapic.bin nothing added to commit but untracked files present (use git add to track) Please consider applying this patch to qemu-kvm.git. This is equally applicable to qemu.git, so please sent it to the qemu mailing list, qemu-de...@nongnu.org. Thanks for your advice, Avi. Now this mail is sent to qemu ML, w/ above quotes as short history. Could someone pick this up? Thanks, H.Seto = This patch looks good, but doesn't apply anymore, as the libdis/ and libdis-user/ part has already been applied from another patch. Care to resend it? I've just posted. Please check it. Thanks. H.Seto -- 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] device-assignment: Rework name of assigned pci device
Thanks Markus,
(2010/06/29 14:28), Markus Armbruster wrote:
Hidetoshi Seto seto.hideto...@jp.fujitsu.com writes:
Hao, Xudong xudong@intel.com writes:
When assign one PCI device, qemu fail to parse the command line:
qemu-system_x86 -smp 2 -m 1024 -hda /path/to/img -pcidevice host=00:19.0
Error:
qemu-system-x86_64: Parameter 'id' expects an identifier
Identifiers consist of letters, digits, '-', '.', '_', starting with a
letter.
pcidevice argument parse error; please check the help text for usage
Could not add assigned device host=00:19.0
https://bugs.launchpad.net/qemu/+bug/597932
This issue caused by qemu-kvm commit
b560a9ab9be06afcbb78b3791ab836dad208a239.
This patch is a response to the above report.
Thanks,
H.Seto
=
Because use of some characters in id is restricted recently, assigned
device start to fail having implicit id that uses address string of
host device, like 00:19.0 which includes restricted character ':'.
It seems that this implicit id is intended to be run as name that
could be passed with option -pcidevice ... ,name=... to specify a
string to be used in log outputs. In other words it seems that
dev-dev.qdev.id of assigned device had been used to have such
name, that is user-defined string or address string of host.
As far as I can tell, option name is just a leftover from pre-qdev
days, kept for compatibility.
Yea, I see.
I don't know well about the history of such pre-qdev days...
The problem is that name for specific use is not equal to id for
universal use. So it is better to remove this tricky mix up here.
This patch introduces new function assigned_dev_name() that returns
proper name string for the device.
Now property name is explicitly defined in struct AssignedDevice.
When if the device have neither name nor id, address string like
:00:19.0 will be created and passed instead. Once created, new
field r_name holds the string to be reused and to be released later.
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
Comments inline.
---
hw/device-assignment.c | 59
++-
hw/device-assignment.h |2 +
2 files changed, 44 insertions(+), 17 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 585162b..d73516f 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -62,6 +62,25 @@ static void assigned_dev_load_option_rom(AssignedDevice
*dev);
static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev);
+static const char *assigned_dev_name(AssignedDevice *dev)
+{
+/* use user-defined name if specified */
+if (dev-u_name)
+return dev-u_name;
+/* else use id if available */
+if (dev-dev.qdev.id)
+return dev-dev.qdev.id;
+/* otherwise use address of host device instead */
+if (!dev-r_name) {
+char buf[32];
+
+snprintf(buf, sizeof(buf), %04x:%02x:%02x.%01x,
+ dev-host.seg, dev-host.bus, dev-host.dev,
dev-host.func);
+dev-r_name = qemu_strdup(buf);
+}
+return dev-r_name;
+}
+
static uint32_t guest_to_host_ioport(AssignedDevRegion *region, uint32_t
addr)
{
return region-u.r_baseport + (addr - region-e_physbase);
@@ -798,6 +817,10 @@ static void free_assigned_device(AssignedDevice *dev)
dev-real_device.config_fd = 0;
}
+if (dev-r_name) {
+qemu_free(dev-r_name);
+}
+
#ifdef KVM_CAP_IRQ_ROUTING
free_dev_irq_entries(dev);
#endif
@@ -885,7 +908,7 @@ static int assign_device(AssignedDevice *dev)
if (dev-use_iommu) {
if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) {
fprintf(stderr, No IOMMU found. Unable to assign device
\%s\\n,
-dev-dev.qdev.id);
+assigned_dev_name(dev));
return -ENODEV;
}
assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
@@ -897,7 +920,7 @@ static int assign_device(AssignedDevice *dev)
r = kvm_assign_pci_device(kvm_context, assigned_dev_data);
if (r 0) {
fprintf(stderr, Failed to assign device \%s\ : %s\n,
-dev-dev.qdev.id, strerror(-r));
+assigned_dev_name(dev), strerror(-r));
switch (r) {
case -EBUSY:
@@ -953,7 +976,7 @@ static int assign_irq(AssignedDevice *dev)
r = kvm_assign_irq(kvm_context, assigned_irq_data);
if (r 0) {
fprintf(stderr, Failed to assign irq for \%s\: %s\n,
-dev-dev.qdev.id, strerror(-r));
+assigned_dev_name(dev), strerror(-r));
fprintf(stderr, Perhaps you are assigning a device
that shares an IRQ with another device?\n);
return r;
@@ -977,7 +1000,7 @@ static void deassign_device(AssignedDevice *dev)
r = kvm_deassign_pci_device(kvm_context, assigned_dev_data);
if (r 0
[PATCH] Update .gitignore
Add some files/directories to .gitignore - vapic.bin A generated binary file - libdis/ and libdis-user/ Directories generated in ./configure - QMP/qmp-commands.txt A generated text - qemu-options.def A generated file used as a source of qemu-options.h Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com --- .gitignore |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index 2d7f439..81b1ba4 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,8 @@ config-target.* libhw32 libhw64 libuser +libdis +libdis-user qemu-doc.html qemu-tech.html qemu-doc.info @@ -21,11 +23,13 @@ qemu-img qemu-nbd qemu-nbd.8 qemu-nbd.pod +qemu-options.def qemu-options.texi qemu-img-cmds.texi qemu-img-cmds.h qemu-io qemu-monitor.texi +QMP/qmp-commands.txt .gdbinit *.a *.aux @@ -50,4 +54,5 @@ pc-bios/optionrom/linuxboot.bin pc-bios/optionrom/multiboot.bin pc-bios/optionrom/multiboot.raw pc-bios/optionrom/extboot.bin +pc-bios/optionrom/vapic.bin .stgit-* -- 1.7.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] device-assignment: Rework name of assigned pci device
Hao, Xudong xudong@intel.com writes:
When assign one PCI device, qemu fail to parse the command line:
qemu-system_x86 -smp 2 -m 1024 -hda /path/to/img -pcidevice host=00:19.0
Error:
qemu-system-x86_64: Parameter 'id' expects an identifier
Identifiers consist of letters, digits, '-', '.', '_', starting with a
letter.
pcidevice argument parse error; please check the help text for usage
Could not add assigned device host=00:19.0
https://bugs.launchpad.net/qemu/+bug/597932
This issue caused by qemu-kvm commit
b560a9ab9be06afcbb78b3791ab836dad208a239.
This patch is a response to the above report.
Thanks,
H.Seto
=
Because use of some characters in id is restricted recently, assigned
device start to fail having implicit id that uses address string of
host device, like 00:19.0 which includes restricted character ':'.
It seems that this implicit id is intended to be run as name that
could be passed with option -pcidevice ... ,name=... to specify a
string to be used in log outputs. In other words it seems that
dev-dev.qdev.id of assigned device had been used to have such
name, that is user-defined string or address string of host.
The problem is that name for specific use is not equal to id for
universal use. So it is better to remove this tricky mix up here.
This patch introduces new function assigned_dev_name() that returns
proper name string for the device.
Now property name is explicitly defined in struct AssignedDevice.
When if the device have neither name nor id, address string like
:00:19.0 will be created and passed instead. Once created, new
field r_name holds the string to be reused and to be released later.
Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
---
hw/device-assignment.c | 59 ++-
hw/device-assignment.h |2 +
2 files changed, 44 insertions(+), 17 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 585162b..d73516f 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -62,6 +62,25 @@ static void assigned_dev_load_option_rom(AssignedDevice
*dev);
static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev);
+static const char *assigned_dev_name(AssignedDevice *dev)
+{
+/* use user-defined name if specified */
+if (dev-u_name)
+return dev-u_name;
+/* else use id if available */
+if (dev-dev.qdev.id)
+return dev-dev.qdev.id;
+/* otherwise use address of host device instead */
+if (!dev-r_name) {
+char buf[32];
+
+snprintf(buf, sizeof(buf), %04x:%02x:%02x.%01x,
+ dev-host.seg, dev-host.bus, dev-host.dev, dev-host.func);
+dev-r_name = qemu_strdup(buf);
+}
+return dev-r_name;
+}
+
static uint32_t guest_to_host_ioport(AssignedDevRegion *region, uint32_t addr)
{
return region-u.r_baseport + (addr - region-e_physbase);
@@ -798,6 +817,10 @@ static void free_assigned_device(AssignedDevice *dev)
dev-real_device.config_fd = 0;
}
+if (dev-r_name) {
+qemu_free(dev-r_name);
+}
+
#ifdef KVM_CAP_IRQ_ROUTING
free_dev_irq_entries(dev);
#endif
@@ -885,7 +908,7 @@ static int assign_device(AssignedDevice *dev)
if (dev-use_iommu) {
if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) {
fprintf(stderr, No IOMMU found. Unable to assign device
\%s\\n,
-dev-dev.qdev.id);
+assigned_dev_name(dev));
return -ENODEV;
}
assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
@@ -897,7 +920,7 @@ static int assign_device(AssignedDevice *dev)
r = kvm_assign_pci_device(kvm_context, assigned_dev_data);
if (r 0) {
fprintf(stderr, Failed to assign device \%s\ : %s\n,
-dev-dev.qdev.id, strerror(-r));
+assigned_dev_name(dev), strerror(-r));
switch (r) {
case -EBUSY:
@@ -953,7 +976,7 @@ static int assign_irq(AssignedDevice *dev)
r = kvm_assign_irq(kvm_context, assigned_irq_data);
if (r 0) {
fprintf(stderr, Failed to assign irq for \%s\: %s\n,
-dev-dev.qdev.id, strerror(-r));
+assigned_dev_name(dev), strerror(-r));
fprintf(stderr, Perhaps you are assigning a device
that shares an IRQ with another device?\n);
return r;
@@ -977,7 +1000,7 @@ static void deassign_device(AssignedDevice *dev)
r = kvm_deassign_pci_device(kvm_context, assigned_dev_data);
if (r 0)
fprintf(stderr, Failed to deassign device \%s\ : %s\n,
-dev-dev.qdev.id, strerror(-r));
+assigned_dev_name(dev), strerror(-r));
#endif
}
@@ -1421,7 +1444,7 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
if (get_real_device(dev, dev-host.seg, dev-host.bus,
dev-host.dev, dev-host.func)) {
error_report(pci-assign: Error
Re: qemu fail to parse command line with -pcidevice 00:19.0
(2010/06/24 15:08), Markus Armbruster wrote: Note to qemu-devel: this issue is qemu-kvm only. Hao, Xudong xudong@intel.com writes: When assign one PCI device, qemu fail to parse the command line: qemu-system_x86 -smp 2 -m 1024 -hda /path/to/img -pcidevice host=00:19.0 Error: qemu-system-x86_64: Parameter 'id' expects an identifier Identifiers consist of letters, digits, '-', '.', '_', starting with a letter. pcidevice argument parse error; please check the help text for usage Could not add assigned device host=00:19.0 https://bugs.launchpad.net/qemu/+bug/597932 This issue caused by qemu-kvm commit b560a9ab9be06afcbb78b3791ab836dad208a239. The bug is in add_assigned_device(): r = get_param_value(id, sizeof(id), id, arg); if (!r) r = get_param_value(id, sizeof(id), name, arg); if (!r) r = get_param_value(id, sizeof(id), host, arg); We end up with invalid ID 00:19.0. ... Are there any strong reason why we cannot use ':' in the identifier? Thanks, H.Seto -- 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] QEMU: Update .gitignore
(2010/06/21 17:19), Avi Kivity wrote: On 06/21/2010 08:24 AM, Hidetoshi Seto wrote: I think some people have noticed that: $ ./configure $ make $ git status # On branch master # Untracked files: # (use git add file... to include in what will be committed) # # QMP/qmp-commands.txt # libdis-user/ # libdis/ # pc-bios/optionrom/vapic.bin nothing added to commit but untracked files present (use git add to track) Please consider applying this patch to qemu-kvm.git. This is equally applicable to qemu.git, so please sent it to the qemu mailing list, qemu-de...@nongnu.org. Thanks for your advice, Avi. Now this mail is sent to qemu ML, w/ above quotes as short history. Could someone pick this up? Thanks, H.Seto = Subject: [PATCH] QEMU: Update .gitignore Add some files/directories to .gitignore - vapic.bin A generated binary file. - libdis/ and libdis-user/ These are directories generated by ./configure. - QMP/qmp-commands.txt A generated text. Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com --- .gitignore |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index 2d7f439..fa4f241 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,8 @@ config-target.* libhw32 libhw64 libuser +libdis +libdis-user qemu-doc.html qemu-tech.html qemu-doc.info @@ -26,6 +28,7 @@ qemu-img-cmds.texi qemu-img-cmds.h qemu-io qemu-monitor.texi +QMP/qmp-commands.txt .gdbinit *.a *.aux @@ -50,4 +53,5 @@ pc-bios/optionrom/linuxboot.bin pc-bios/optionrom/multiboot.bin pc-bios/optionrom/multiboot.raw pc-bios/optionrom/extboot.bin +pc-bios/optionrom/vapic.bin .stgit-* -- 1.7.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] Update .gitignore
I think some people have noticed that: $ ./configure $ make $ git status # On branch master # Untracked files: # (use git add file... to include in what will be committed) # # QMP/qmp-commands.txt # libdis-user/ # libdis/ # pc-bios/optionrom/vapic.bin nothing added to commit but untracked files present (use git add to track) Please consider applying this patch to qemu-kvm.git. Thanks, H.Seto = Subject: [PATCH] Update .gitignore Add some files/directories to .gitignore - vapic.bin A generated binary file. - libdis/ and libdis-user/ These are directories generated by ./configure. - QMP/qmp-commands.txt A generated text. Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com --- .gitignore |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/.gitignore b/.gitignore index 2d7f439..fa4f241 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,8 @@ config-target.* libhw32 libhw64 libuser +libdis +libdis-user qemu-doc.html qemu-tech.html qemu-doc.info @@ -26,6 +28,7 @@ qemu-img-cmds.texi qemu-img-cmds.h qemu-io qemu-monitor.texi +QMP/qmp-commands.txt .gdbinit *.a *.aux @@ -50,4 +53,5 @@ pc-bios/optionrom/linuxboot.bin pc-bios/optionrom/multiboot.bin pc-bios/optionrom/multiboot.raw pc-bios/optionrom/extboot.bin +pc-bios/optionrom/vapic.bin .stgit-* -- 1.7.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] device-assignment, msi: PBA is long
Accidentally a pci_read_long() was replaced with assigned_dev_pci_read_byte() by the commit: commit a81a1f0a7410976be7dbc9a81524a8640f446ab5 Author: Alex Williamson alex.william...@redhat.com device-assignment: Don't use libpci Signed-off-by: Hidetoshi Seto seto.hideto...@jp.fujitsu.com --- hw/device-assignment.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/device-assignment.c b/hw/device-assignment.c index e254203..e237bd3 100644 --- a/hw/device-assignment.c +++ b/hw/device-assignment.c @@ -1263,7 +1263,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) pci_dev-cap.length + PCI_MSIX_TABLE) = msix_table_entry; *(uint32_t *)(pci_dev-config + pci_dev-cap.start + pci_dev-cap.length + PCI_MSIX_PBA) = -assigned_dev_pci_read_byte(pci_dev, pos + PCI_MSIX_PBA); +assigned_dev_pci_read_long(pci_dev, pos + PCI_MSIX_PBA); bar_nr = msix_table_entry PCI_MSIX_BIR; msix_table_entry = ~PCI_MSIX_BIR; dev-msix_table_addr = pci_region[bar_nr].base_addr + msix_table_entry; -- 1.7.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
