[Qemu-devel] [PATCH -V3] coroutine: Add CoRwlock support
Signed-off-by: Aneesh Kumar K.V
---
qemu-coroutine-lock.c | 44
qemu-coroutine.h | 32
2 files changed, 76 insertions(+), 0 deletions(-)
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index a80f437..2a385a3 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -115,3 +115,47 @@ void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex)
trace_qemu_co_mutex_unlock_return(mutex, self);
}
+
+void qemu_co_rwlock_init(CoRwlock *lock)
+{
+memset(lock, 0, sizeof(*lock));
+qemu_co_queue_init(&lock->queue);
+}
+
+void qemu_co_rwlock_rdlock(CoRwlock *lock)
+{
+while (lock->writer) {
+qemu_co_queue_wait(&lock->queue);
+}
+lock->reader++;
+}
+
+void qemu_co_rwlock_unlock(CoRwlock *lock)
+{
+assert(qemu_in_coroutine());
+if (lock->writer) {
+lock->writer = false;
+while (!qemu_co_queue_empty(&lock->queue)) {
+/*
+ * Wakeup every body. This will include some
+ * writers too.
+ */
+qemu_co_queue_next(&lock->queue);
+}
+} else {
+lock->reader--;
+assert(lock->reader >= 0);
+/* Wakeup only one waiting writer */
+if (!lock->reader) {
+qemu_co_queue_next(&lock->queue);
+}
+}
+}
+
+void qemu_co_rwlock_wrlock(CoRwlock *lock)
+{
+while (lock->writer || lock->reader) {
+qemu_co_queue_wait(&lock->queue);
+}
+lock->writer = true;
+}
diff --git a/qemu-coroutine.h b/qemu-coroutine.h
index 2f2fd95..b8fc4f4 100644
--- a/qemu-coroutine.h
+++ b/qemu-coroutine.h
@@ -156,4 +156,36 @@ void coroutine_fn qemu_co_mutex_lock(CoMutex *mutex);
*/
void coroutine_fn qemu_co_mutex_unlock(CoMutex *mutex);
+typedef struct CoRwlock {
+bool writer;
+int reader;
+CoQueue queue;
+} CoRwlock;
+
+/**
+ * Initialises a CoRwlock. This must be called before any other operation
+ * is used on the CoRwlock
+ */
+void qemu_co_rwlock_init(CoRwlock *lock);
+
+/**
+ * Read locks the CoRwlock. If the lock cannot be taken immediately because
+ * of a parallel writer, control is transferred to the caller of the current
+ * coroutine.
+ */
+void qemu_co_rwlock_rdlock(CoRwlock *lock);
+
+/**
+ * Write Locks the mutex. If the lock cannot be taken immediately because
+ * of a parallel reader, control is transferred to the caller of the current
+ * coroutine.
+ */
+void qemu_co_rwlock_wrlock(CoRwlock *lock);
+
+/**
+ * Unlocks the read/write lock and schedules the next coroutine that was
+ * waiting for this lock to be run.
+ */
+void qemu_co_rwlock_unlock(CoRwlock *lock);
+
#endif /* QEMU_COROUTINE_H */
--
1.7.4.1
Re: [Qemu-devel] [PATCH] Sparc: fix non-faulting unassigned memory accesses
Blue Swirl wrote: > Commit b14ef7c9ab41ea824c3ccadb070ad95567cca84e > introduced cpu_unassigned_access() function. On Sparc, > the function does not restore AREG0 used for global CPUState > on function exit, causing bugs with non-faulting unassigned > memory accesses. Alpha, Microblaze and MIPS are not affected. > > Fix by restoring AREG0 on exit. Remove excess saving by > do_unassigned_access() functions. > > Also ignore unassigned accesses outside of CPU context. > > Reported-by: Bob Breuer > Signed-off-by: Blue Swirl > --- > target-sparc/op_helper.c | 25 - > 1 files changed, 8 insertions(+), 17 deletions(-) > [snip] Works for my testcases. Tested-by: Bob Breuer
Re: [Qemu-devel] [RFC 01/24] block: add block conversion api
On Mon, Aug 1, 2011 at 9:34 AM, Kevin Wolf wrote: >> + /** >> + * Gets a mapping in the image file. >> + * >> + * The function starts searching for a mapping at >> + * starting_guest_offset = guest_offset + contiguous_bytes >> + * >> + * @param bs[in] The image in which to find mapping. >> + * @param guest_offset[in,out] On function entry used to calculate >> + * starting search address. >> + * On function exit contains the >> starting >> + * guest offset of the mapping. >> + * @param host_offset[out] The starting image file offset for >> the >> + * mapping. >> + * @param contiguous_bytes[in,out] On function entry used to calculate >> + * starting search address. >> + * On function exit contains the number >> of >> + * bytes for which this mapping is >> valid. >> + * A value of 0 means there are no more >> + * mappings in the image. >> + * @return Returns non-zero on error. >> + */ >> + int (*bdrv_get_mapping)(BlockDriverState *bs, uint64_t *guest_offset, >> + uint64_t *host_offset, uint64_t *contiguous_bytes); > > Last time I suggested to change the semantics of this function as follows: > >> I think it would be less surprising if the function worked like >> bdrv_is_allocated(). That is, it doesn't search for mapped clusters, but >> always returns information for exactly the given offset. It would return >> if the range starting at the given offset is used or free. >> >> If the caller wants to find the next existing mapping, he can just take >> contiguous_bytes of the free region and add it to his current offset. > > Do you believe that this change would be a bad idea or did you just > forget it? I was just kept putting it off because for some reason I had the idea that it would be a lot of work to change it. I've done it now and it turns out I only had to change about 3 line of code. >> + >> + /** >> + * Copies out the header of a conversion target >> + * >> + * Saves the current header for the image in a temporary file and >> overwrites >> + * it with the header for the new format (at the moment the header is >> + * assumed to be 1 sector) >> + * >> + * @param bs Usualy opened with bdrv_open_conversion_target(). >> + * @return Returns non-zero on failure >> + */ >> + int (*bdrv_copy_header) (BlockDriverState *bs); > > Is it true with the current implementation that the old header is saved? > I think it's supposed to be used if updating the header goes wrong. Who > will read the temporary file in this case? Does the user need to know > where it is or even have control over it? Yes, the header is in fact save in the current implementation. Its always saved to headerbackup.temp, or something to that effect. Soon I plan on replacing that with a randomly generated temp file and informing the user of the name. Also, I plan on implementing the ability to override it with a command line option >> + uint64_t image_size; >> + uint64_t cluster_size; >> + uint64_t allocation_size; >> +}; > > Comments explaining the difference between cluster_size and allocation_size? > At the moment there is none, and I don't think allocation_size has a future since the allocation size of the source image isn't really important.
Re: [Qemu-devel] [RFC 21/24] qcow2: add qcow2_open_conversion_target()
On Mon, Aug 1, 2011 at 11:26 AM, Kevin Wolf wrote: > Am 29.07.2011 06:49, schrieb Devin Nakamura: >> Still in very raw form. Not likely to work yet >> >> Signed-off-by: Devin Nakamura > > I don't think it's quite as easy. > > The problem is that qcow2 will access the image header in some places, > for example when growing the L1 or refcount table. You need to make sure > that it doesn't destroy the source format header, but writes to the > target format header at a different offset. This is why I've made sure to size the L1 and refcount tables so they wont need to be resized. But I suppose that's really a balancing act that's likely to break if someone makes changes to the current code, or when snapshots are implemented > I think much of qcow2_open and qcow2_open_conversion_target could be the > same. That is, both could call a common function that takes a parameter > for the header offset. I'm almost certain I could do that (considering I lifted a lot of the code for qcow2_open_conversion_target from qcow2_open) > The other part of qcow2_open_conversion_target (creating a new header > and a basic L1/refcount table) could possibly be shared with > qcow2_create2, though I'm not quite as sure about this one. >> + //TODO: double check this, it seems a bit fishy > > That's my impression of the whole operation, too. :-) > > This is why I thought it would be nice to share this code with normal > image creation. Then we have this fishy code at least only once. > > Kevin >
Re: [Qemu-devel] [RFC 22/24] qemu-io: make map command use new block mapping function
On Mon, Aug 1, 2011 at 11:38 AM, Kevin Wolf wrote: > Am 29.07.2011 06:49, schrieb Devin Nakamura: >> bdrv_get_mapping will be used when it is defined, >> otherwise default to old behaviour. >> >> Signed-off-by: Devin Nakamura > > Hm, I think I would use a new command for this, like 'get_mapping'. The > old 'map' command can still be useful even for formats supporting > bdrv_get_mapping. You would use it whenever you are only interested > which offsets are allocated, but don't care about the offsets in the > image file to which they are mapped. This makes the output much shorter. > > Kevin > Ok, no problem. Its on the todo list.
[Qemu-devel] [Bug 739785] Re: qemu-i386 user mode on ARMv5 host fails (bash: fork: Invalid argument)
YES! That did the trick! Though, with this patch I was unable to compile qemu 0.15rc1. I suppose the patch wasn't meant for it, but with 0.14.1 it works flawlessly. Also, I had to do echo 2 >/proc/cpu/alignment as was mentioned by Peter. I hope the patch gets applied to the main tree. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/739785 Title: qemu-i386 user mode on ARMv5 host fails (bash: fork: Invalid argument) Status in QEMU: New Bug description: Good time of day everybody, I have been trying to make usermode qemu on ARM with plugapps (archlinux) with archlinux i386 chroot to work. 1. I installed arch linux in a virtuabox and created a chroot for it with mkarchroot. Transferred it to my pogo plug into /i386/ 2. I comiled qemu-i386 static and put it into /i386/usr/bin/ ./configure --static --disable-blobs --disable-system --target-list=i386-linux-user make 3. I also compiled linux kernel 2.6.38 with CONFIG_BINFMT_MISC=y and installed it. uname -a Linux Plugbox 2.6.38 #4 PREEMPT Fri Mar 18 22:19:10 CDT 2011 armv5tel Feroceon 88FR131 rev 1 (v5l) Marvell SheevaPlug Reference Board GNU/Linux 4. Added the following options into /etc/rc.local /sbin/modprobe binfmt_misc /bin/mount binfmt_misc -t binfmt_misc /proc/sys/fs/binfmt_misc echo ':qemu-i386:M::\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x03\x00:\xff\xff\xff\xff\xff\xfe\xfe\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfb\xff\xff\xff:/usr/bin/qemu-i386:' >/proc/sys/fs/binfmt_misc/register 5. Also copied ld-linux.so.3 (actually ld-2.13.so because ld- linux.so.3 is a link to that file) from /lib/ to /i386/lib/ 6.Now i chroot into /i386 and I get this: [root@Plugbox i386]# chroot . [II aI hnve ao n@P /]# pacman -Suy bash: fork: Invalid argument 7.I also downloaded linux-user-test-0.3 from qemu website and ran the test: [root@Plugbox linux-user-test-0.3]# make ./qemu-linux-user.sh [qemu-i386] ../qemu-0.14.0/i386-linux-user/qemu-i386 -L ./gnemul/qemu-i386 i386/ls -l dummyfile BUG IN DYNAMIC LINKER ld.so: dl-version.c: 210: _dl_check_map_versions: Assertion `needed != ((void *)0)' failed! make: *** [test] Error 127 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/739785/+subscriptions
[Qemu-devel] buildbot failure in qemu on trivial-patches_i386_debian_5_0
The Buildbot has detected a new failure on builder trivial-patches_i386_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/trivial-patches_i386_debian_5_0/builds/52 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_trivial-patches' triggered this build Build Source Stamp: [branch trivial-patches] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
Re: [Qemu-devel] [PATCH] [FYI] Scripts to generate project stats
On 08/01/2011 09:36 PM, Anthony Liguori wrote:
As part of my talk for KVM Forum, I am collecting some stats on the project
since last year. I thought I'd share the scripts in case anyone is interested
in how they work.
+function gen-stats() {
+until="$1"
+since="$2"
+
+echo 'Total Commits'
+echo '-'
+gen-commits "$until" "$since"
+echo
+
+echo 'Committers'
+echo '--'
+gen-committers "$until" "$since"
+echo
+
+echo 'Authors'
+echo '---'
+gen-authors "$until" "$since"
+echo
+
+echo 'Companies'
+echo '-'
+gen-companies "$util" "$since"
Should be:
gen-companies "$until" "$since"
Regards,
Anthony Liguori
+}
+
+gen-stats "$1" "$2"
+
[Qemu-devel] buildbot failure in qemu on qmp_x86_64_debian_5_0
The Buildbot has detected a new failure on builder qmp_x86_64_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/qmp_x86_64_debian_5_0/builds/52 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_qmp' triggered this build Build Source Stamp: [branch queue/qmp] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on qmp_i386_debian_5_0
The Buildbot has detected a new failure on builder qmp_i386_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/qmp_i386_debian_5_0/builds/52 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_qmp' triggered this build Build Source Stamp: [branch queue/qmp] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] [PATCH] [FYI] Scripts to generate project stats
As part of my talk for KVM Forum, I am collecting some stats on the project
since last year. I thought I'd share the scripts in case anyone is interested
in how they work.
I think this is just about all of the data I need, but patches are certainly
welcome.
Of course, you'll have to come to KVM Forum to see the pretty version of these
stats (there should be videos too for those that can't make it :-))
And thanks to Alex for poking me to collect these too.
---
scripts/aliases.txt | 18 +
scripts/companies.txt | 20 ++
scripts/genstats.sh | 96 +
3 files changed, 134 insertions(+), 0 deletions(-)
create mode 100644 scripts/aliases.txt
create mode 100644 scripts/companies.txt
create mode 100755 scripts/genstats.sh
diff --git a/scripts/aliases.txt b/scripts/aliases.txt
new file mode 100644
index 000..aadea25
--- /dev/null
+++ b/scripts/aliases.txt
@@ -0,0 +1,18 @@
+andrew.zaborow...@intel.com: bal...@zabor.org
+ed...@axis.com: edgar.igles...@gmail.com
+edgar.igles...@petalogix.com: edgar.igles...@gmail.com
+lcapitul...@gmail.com: lcapitul...@redhat.com
+riku.voi...@nokia.com: riku.voi...@linaro.org
+riku.voi...@iki.fi: riku.voi...@linaro.org
+andreas.faerber: andreas.faer...@web.de
+anth...@codemonkey.ws: aligu...@us.ibm.com
+atar4q...@googlemail.com: atar4q...@gmail.com
+bernhard.k...@gmx.net: bernhard.k...@nsn.com
+jan.kis...@web.de: jan.kis...@siemens.com
+m...@kevin-wolf.de: kw...@redhat.com
+marcandre.lur...@gmail.com: marcandre.lur...@redhat.com
+r...@twiddle.net: r...@redhat.com
+sripa...@sripathi.in.ibm.com: sripat...@in.ibm.com
+
+
+
diff --git a/scripts/companies.txt b/scripts/companies.txt
new file mode 100644
index 000..436e3b3
--- /dev/null
+++ b/scripts/companies.txt
@@ -0,0 +1,20 @@
+Red Hat: redhat.com h...@lst.de glommer@mothafucka.localdomain
+SuSE: suse.de novell.com
+IBM: ibm.com kernel.crashing.org gibson.dropbear.id.au
+AMD: amd.com
+Citrix: citrix.com
+Canonical: canonical.com
+Intel: intel.com
+VIA: viatech.com.cn
+Linaro: linaro
+Google: google.com
+Code Sourcery: codesourcery.com
+Siemens: siemens.com siemens-enterprise.com
+Fujitsu: fujitsu.com
+Dream Host: dreamhost.com
+Nokia: nokia.com
+Samsung: samsung.com
+NTT: lab.ntt.co.jp
+FreeScale: freescale.com
+XenSource: xensource.com
+VA Linux: valinux.co.jp
diff --git a/scripts/genstats.sh b/scripts/genstats.sh
new file mode 100755
index 000..6d1228f
--- /dev/null
+++ b/scripts/genstats.sh
@@ -0,0 +1,96 @@
+#!/bin/bash
+
+# Usage: scripts/genstats.sh "today" "1 year ago"
+
+aliases="scripts/aliases.txt"
+companies="scripts/companies.txt"
+
+function dedup() {
+while read addr; do
+ f=`grep "^$addr: " "$aliases" | cut -f2- -d' '`
+ if test "$f"; then
+ echo "$f"
+ else
+ echo "$addr"
+ fi
+done
+}
+
+function gen-committers() {
+until="$1"
+since="$2"
+
+git log --until="$until" --since="$since" --pretty=format:%ce | \
+ sort -u | dedup | sort -u | while read committer; do
+ addresses=`grep " $committer\$" "$aliases" | cut -f1 -d: | while read
a; do echo -n "--committer=$a "; done`
+
+ echo -n "$committer, "
+ git log --until="$until" --since="$since" \
+ --pretty=oneline --committer="$committer" $addresses | wc -l
+done
+}
+
+function gen-authors() {
+until="$1"
+since="$2"
+
+git log --until="$until" --since="$since" --pretty=format:%ae | \
+ sort -u | dedup | sort -u | while read author; do
+ addresses=`grep " $author\$" "$aliases" | cut -f1 -d: | while read a;
do echo -n "--author=$a "; done`
+
+ echo -n "$author, "
+ git log --until="$until" --since="$since" \
+ --pretty=oneline --author="$author" | wc -l
+done
+}
+
+function gen-commits() {
+until="$1"
+since="$2"
+
+git log --until="$until" --since="$since" --pretty=oneline | wc -l
+}
+
+function gen-companies() {
+until="$1"
+since="$2"
+
+cat "$companies" | while read LINE; do
+ company=`echo $LINE | cut -f1 -d:`
+ addrs=`echo $LINE | cut -f2- -d:`
+
+ authors=`echo "$addrs" | sed -e 's: : --author=:g'`
+ echo "$company," \
+ `git log --until="$until" --since="$since" --pretty=oneline \
+$authors | wc -l`, \
+`git log --until="$until" --since="$since" --pretty="format:%ae\n"
\
+$authors | sort -u | dedup | sort -u | wc -l`
+done
+}
+
+function gen-stats() {
+until="$1"
+since="$2"
+
+echo 'Total Commits'
+echo '-'
+gen-commits "$until" "$since"
+echo
+
+echo 'Committers'
+echo '--'
+gen-committers "$until" "$since"
+echo
+
+echo 'Authors'
+echo '---'
+gen-authors "$until" "$since"
+echo
+
+echo 'Companies'
+echo '-'
+gen-companies "$util" "$since"
+}
+
+gen-stats "$1" "$2"
+
--
1.7.4.1
Re: [Qemu-devel] [PATCH 04/25] Add hard build dependency on glib
This patch introduces "-mms-bitfields" cflag on MinGW but this cflag
breaks gcc packed structures("__attribute__((packed))"). For example,
slirp does not work on Win32.
Re: [Qemu-devel] kvm PCI assignment & VFIO ramblings
On Mon, 2011-08-01 at 12:59 -0600, Alex Williamson wrote: > > > > .../... > > I'll try to consolidate my reply to all the above here because there are > too many places above to interject and make this thread even more > difficult to respond to. True, I should try to do the same :-) > Much of what you're discussion above comes > down to policy. Do we trust DisINTx? Do we trust multi-function > devices? I have no doubt there are devices we can use as examples for > each behaving badly. On x86 this is one of the reasons we have SR-IOV. Right, that and having the ability to provide way more functions that you would normally have. > Besides splitting a single device into multiple, it makes sure each > devices is actually virtualization friendly. POWER seems to add > multiple layers of hardware so that you don't actually have to trust the > device, which is a great value add for enterprise systems, but in doing > so it mostly defeats the purpose and functionality of SR-IOV. Well not entirely. A lot of what POWER does is also about isolation on errors. This is going to be useful with and without SR-IOV. Also not all devices are SR-IOV capable and there are plenty of situations where one would want to pass-through devices that aren't, I don't see that as disappearing tomorrow. > How we present this in a GUI is largely irrelevant because something has > to create a superset of what the hardware dictates (can I uniquely > identify transactions from this device, can I protect other devices from > it, etc.), the system policy (do I trust DisINTx, do I trust function > isolation, do I require ACS) and mold that with what the user actually > wants to assign. For the VFIO kernel interface, we should only be > concerned with the first problem. Userspace is free to make the rest as > simple or complete as it cares to. I argue for x86, we want device > level granularity of assignment, but that also tends to be the typical > case (when only factoring in hardware restrictions) due to our advanced > iommus. Well, POWER iommu's are advanced too ... just in a different way :-) x86 seems to be a lot less interested in robustness and reliability for example :-) I tend to agree that the policy decisions in general should be done by the user, tho with appropriate information :-) But some of them on our side are hard requirements imposed by how our firmware or early kernel code assigned the PE's and we need to expose that. It directly derives the sharing of iommu's too but then we -could- have those different iommu's point to the same table in memory and essentially mimmic the x86 domains. We chose not to. The segments are too small in our current HW design for one and it means we lose the isolation between devices which is paramount to getting the kind of reliability and error handling we want to achieve. > > > > Maybe something like /sys/devgroups ? This probably warrants involving > > > > more kernel people into the discussion. > > > > > > I don't yet buy into passing groups to qemu since I don't buy into the > > > idea of always exposing all of those devices to qemu. Would it be > > > sufficient to expose iommu nodes in sysfs that link to the devices > > > behind them and describe properties and capabilities of the iommu > > > itself? More on this at the end. > > > > Well, iommu aren't the only factor. I mentioned shared interrupts (and > > my unwillingness to always trust DisINTx), > > *userspace policy* Maybe ... some of it yes. I suppose. You can always hand out to userspace bigger guns to shoot itself in the foot. Not always very wise but heh. Some of these are hard requirements tho. And we have to make that decision when we assign PE's at boot time. > > there's also the MMIO > > grouping I mentioned above (in which case it's an x86 -limitation- with > > small BARs that I don't want to inherit, especially since it's based on > > PAGE_SIZE and we commonly have 64K page size on POWER), etc... > > But isn't MMIO grouping effectively *at* the iommu? No exactly. It's a different set of tables & registers in the host bridge and essentially a different set of logic, tho it does hook into the whole "shared PE# state" thingy to enforce isolation of all layers on error. > > So I'm not too fan of making it entirely look like the iommu is the > > primary factor, but we -can-, that would be workable. I still prefer > > calling a cat a cat and exposing the grouping for what it is, as I think > > I've explained already above, tho. > > The trouble is the "group" analogy is more fitting to a partitionable > system, whereas on x86 we can really mix-n-match devices across iommus > fairly easily. The iommu seems to be the common point to describe these > differences. No. You can do that by throwing away isolation between those devices and thus throwing away error isolation capabilities as well. I suppose if you don't care about RAS... :-) > > > > Now some of this can be fixed with tweaks, and we've sta
[Qemu-devel] [PATCH] qdev: Eliminate duplicate reset
qbus_reset_all_fn was registered twice, so a lot of device reset
functions were also called twice when QEMU started.
Which was introduced by 80376c3fc2c38fdd45354e4b0eb45031f35587ed
This patch fixes it by making the main_system_bus creation not register
reset handler.
Cc: Stefan Weil
Signed-off-by: Isaku Yamahata
Tested-by: Stefan Weil
---
hw/qdev.c | 14 --
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index b4ea8e1..6819537 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -36,6 +36,7 @@ static bool qdev_hot_removed = false;
/* This is a nasty hack to allow passing a NULL bus to qdev_create. */
static BusState *main_system_bus;
+static void main_system_bus_create(void);
DeviceInfo *device_info_list;
@@ -328,8 +329,7 @@ static int qdev_reset_one(DeviceState *dev, void *opaque)
BusState *sysbus_get_default(void)
{
if (!main_system_bus) {
-main_system_bus = qbus_create(&system_bus_info, NULL,
- "main-system-bus");
+main_system_bus_create();
}
return main_system_bus;
}
@@ -784,6 +784,16 @@ BusState *qbus_create(BusInfo *info, DeviceState *parent,
const char *name)
return bus;
}
+static void main_system_bus_create(void)
+{
+/* assign main_system_bus before qbus_create_inplace()
+ * in order to make "if (bus != main_system_bus)" work */
+main_system_bus = qemu_mallocz(system_bus_info.size);
+main_system_bus->qdev_allocated = 1;
+qbus_create_inplace(main_system_bus, &system_bus_info, NULL,
+"main-system-bus");
+}
+
void qbus_free(BusState *bus)
{
DeviceState *dev;
--
1.7.1.1
Re: [Qemu-devel] [PATCH] Correctly assign PCI domain numbers
On Mon, Aug 01, 2011 at 05:03:18PM +0300, Michael S. Tsirkin wrote: > On Mon, Aug 01, 2011 at 11:33:37PM +1000, David Gibson wrote: > > On Mon, Aug 01, 2011 at 01:10:38PM +0300, Michael S. Tsirkin wrote: > > > On Mon, Aug 01, 2011 at 04:51:02PM +1000, David Gibson wrote: > > > > qemu already almost supports PCI domains; that is, several entirely > > > > independent PCI host bridges on the same machine. However, a bug in > > > > pci_bus_new_inplace() means that every host bridge gets assigned domain > > > > number zero and so can't be properly distinguished. This patch fixes > > > > the > > > > bug, giving each new host bridge a new domain number. > > > > > > > > Signed-off-by: David Gibson > > > > > > OK, but I'd like to see the whole picture. > > > How does the guest detect multiple domains, > > > and how does it access them? > > > > For the pseries machine, which is what I'm concerned with, each host > > bridge is advertised through the device tree passed to the guest. > > Could you explain please? > What generates the device tree and passes it to the guest? In the case of the pseries machine, it is generated from hw/spapr.c and loaded into memory for use by the firmware and/or the kernel. > > That gives the necessary handles and addresses for accesing config > > space and memory and IO windows for each host bridge. > > I see. I think maybe a global counter in the common code > is not exactly the best solution in the general case. Well, which general case do you have in mind. Since by definition, PCI domains are entirely independent from each other, domain numbers are essentially arbitrary as long as they're unique - simply a convention which makes it easier to describe which host bridge devices belong on. I don't see an obvious approach which is better than a global counter, or least not one that doesn't involve a significant rewrite of the PCI subsystem. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [Xen-devel] [PATCH] xen-mapcache: Fix rlimit set size.
>>> Anthony PERARD 08/01/11 9:27 PM >>> >Previously, the address space soft limit was set mcache_max_size. So, >before the mcache_max_size was reached by the mapcache, QEMU was killed >for overuse of the virtual address space. > >This patch fix that by setting the soft limit to mcache_max_size + 80MB. >I observed that QEMU use 75MB more than max_mcache_size after several >empirical tests. Rather fragile going forward I would say. I there any reason not to set the limit to infinity? Jan
[Qemu-devel] buildbot failure in qemu on block_i386_debian_5_0
The Buildbot has detected a new failure on builder block_i386_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/block_i386_debian_5_0/builds/52 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_block' triggered this build Build Source Stamp: [branch block] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on block_x86_64_debian_5_0
The Buildbot has detected a new failure on builder block_x86_64_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/block_x86_64_debian_5_0/builds/52 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_block' triggered this build Build Source Stamp: [branch block] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
Re: [Qemu-devel] [SeaBIOS] SeaBIOS error with Juniper FreeBSD kernel
On Mon, Aug 01, 2011 at 03:49:11PM +0200, Bjørn Mork wrote: > I just confirmed the issue running > > "JUNOS 11.1R3.5 built 2011-06-25 00:17:21 UTC" > > which is as new as it officially gets at the moment. [...] > Also confirmed that 11.1R3.5 is working with SeaBIOS modified as > follows: [...] > -void *finaltable = malloc_high(structure_table_length); > +void *finaltable = malloc_fseg(structure_table_length); I'm not really sure how best to handle this. The smbios table can be larger than the current space reserved for the f-segment (when there are a large number of CPUs). Some ideas: There is actually space in the f-segment that is unused, but not given to the malloc_fseg pool. That space could be given to the pool - though the available space will still vary depending on the code size. It's also possible to relocate the 32bit "run-time" code to high memory which would then free up more space in the f-segment (at the cost of some high memory being reserved from the OS). As above, though, the f-segment is still fundamentally limited by the 16bit code size. Also, it's possible the code could try to use the f-segment if there are less than say 16 cpus and use high memory when more cpus are present. -Kevin
[Qemu-devel] [Bug 739785] Re: qemu-i386 user mode on ARMv5 host fails (bash: fork: Invalid argument)
Awesome Artyom! I'm compiling now. I will report back if this patch works for arm as well. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/739785 Title: qemu-i386 user mode on ARMv5 host fails (bash: fork: Invalid argument) Status in QEMU: New Bug description: Good time of day everybody, I have been trying to make usermode qemu on ARM with plugapps (archlinux) with archlinux i386 chroot to work. 1. I installed arch linux in a virtuabox and created a chroot for it with mkarchroot. Transferred it to my pogo plug into /i386/ 2. I comiled qemu-i386 static and put it into /i386/usr/bin/ ./configure --static --disable-blobs --disable-system --target-list=i386-linux-user make 3. I also compiled linux kernel 2.6.38 with CONFIG_BINFMT_MISC=y and installed it. uname -a Linux Plugbox 2.6.38 #4 PREEMPT Fri Mar 18 22:19:10 CDT 2011 armv5tel Feroceon 88FR131 rev 1 (v5l) Marvell SheevaPlug Reference Board GNU/Linux 4. Added the following options into /etc/rc.local /sbin/modprobe binfmt_misc /bin/mount binfmt_misc -t binfmt_misc /proc/sys/fs/binfmt_misc echo ':qemu-i386:M::\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x03\x00:\xff\xff\xff\xff\xff\xfe\xfe\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfb\xff\xff\xff:/usr/bin/qemu-i386:' >/proc/sys/fs/binfmt_misc/register 5. Also copied ld-linux.so.3 (actually ld-2.13.so because ld- linux.so.3 is a link to that file) from /lib/ to /i386/lib/ 6.Now i chroot into /i386 and I get this: [root@Plugbox i386]# chroot . [II aI hnve ao n@P /]# pacman -Suy bash: fork: Invalid argument 7.I also downloaded linux-user-test-0.3 from qemu website and ran the test: [root@Plugbox linux-user-test-0.3]# make ./qemu-linux-user.sh [qemu-i386] ../qemu-0.14.0/i386-linux-user/qemu-i386 -L ./gnemul/qemu-i386 i386/ls -l dummyfile BUG IN DYNAMIC LINKER ld.so: dl-version.c: 210: _dl_check_map_versions: Assertion `needed != ((void *)0)' failed! make: *** [test] Error 127 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/739785/+subscriptions
Re: [Qemu-devel] [PATCH 00/12][RFC] char: add flow control and fix guest_[open|close]
On 08/01/2011 04:54 PM, Blue Swirl wrote: On Mon, Aug 1, 2011 at 2:22 PM, Anthony Liguori wrote: The char layer has been growing some nasty warts for some time now as we ask it to do things it was never intended on doing. It's been long over due for an overhaul and its become evident to me that we need to address this first before adding any more features to the char layer. This series is the start at sanitizing the char layer. It effectively turns the char layer into an internal pipe. It supports flow control using an intermediate ring queue for each direction. This series is an RFC because I don't think we should merge the series until we completely convert the old style flow control users to the new style. The terms 'back-end' and 'front-end' could be improved. How about just 'device' or 'hw' and 'chrdev'? It's all temporary as there currently is very little asymmetry. Historically, the biggest source of asymmetry was the fact that the back-end -> front-end path had flow control and the opposite direction didn't. This series fixes that. The only remaining asymmetry is the ioctl() call. I think if we change the semantics of qemu_chr_event() though to have a return value and a data parameter, I think we can fold ioctl into event and ultimately fix that asymmetry. Once that's done, there's no longer a distinction between front-end and back-end. That makes CharDriverState act as basically a socketpair(). The architecture could be described in for example qemu-tech.texi. I'll include something for docs/ for the next submission. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 00/12][RFC] char: add flow control and fix guest_[open|close]
On Mon, Aug 1, 2011 at 2:22 PM, Anthony Liguori wrote: > The char layer has been growing some nasty warts for some time now as we ask > it > to do things it was never intended on doing. It's been long over due for an > overhaul and its become evident to me that we need to address this first > before > adding any more features to the char layer. > > This series is the start at sanitizing the char layer. It effectively turns > the char layer into an internal pipe. It supports flow control using an > intermediate ring queue for each direction. > > This series is an RFC because I don't think we should merge the series until > we > completely convert the old style flow control users to the new style. The terms 'back-end' and 'front-end' could be improved. How about just 'device' or 'hw' and 'chrdev'? The architecture could be described in for example qemu-tech.texi. > One particularly nasty area is the mux device. I'm not entirely sure yet how > to preceed there. > > >
[Qemu-devel] [0.15][PATCH] lm32: softusb: claim to support full speed
Am Donnerstag 21 Juli 2011, 20:52:24 schrieb Michael Walle: > The QEMU keyboard and mouse reports themselves as full speed devices, > though they are actually low speed devices. Until this is fixed, claim that > we are supporting full speed devices. > > Signed-off-by: Michael Walle > --- > hw/milkymist-softusb.c |6 -- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/hw/milkymist-softusb.c b/hw/milkymist-softusb.c > index ce2bfc6..abf7b59 100644 > --- a/hw/milkymist-softusb.c > +++ b/hw/milkymist-softusb.c > @@ -310,10 +310,12 @@ static int milkymist_softusb_init(SysBusDevice *dev) > usb_bus_new(&s->usbbus, &softusb_bus_ops, NULL); > > /* our two ports */ > +/* FIXME: claim to support full speed devices. qemu mouse and keyboard > + * report themselves as full speed devices. */ > usb_register_port(&s->usbbus, &s->usbport[0], NULL, 0, &softusb_ops, > -USB_SPEED_MASK_LOW); > +USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL); > usb_register_port(&s->usbbus, &s->usbport[1], NULL, 1, &softusb_ops, > -USB_SPEED_MASK_LOW); > +USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL); > > /* and finally create an usb keyboard */ > s->usbdev = usb_create_simple(&s->usbbus, "usb-kbd"); Ping. Without this patch the whole milkymist target is non-functional in the 0.15 release. -- Michael
Re: [Qemu-devel] [PATCH] guest agent: add --enable-guest-agent config option
On 08/01/2011 02:52 PM, Michael Roth wrote: QAPI will require glib/python, but for now the guest agent is the only user. For now, make these dependencies an explicit guest agent one, and give users the option to disable it if need be. Once QAPI is adopted in core QEMU code, we would basically revert this patch. Signed-off-by: Michael Roth Hi Stuart, Can you confirm this patch fixes your problem? Thanks for sending this Mike. Regards, Anthony Liguori --- configure | 40 +++- 1 files changed, 27 insertions(+), 13 deletions(-) diff --git a/configure b/configure index 77194cf..58a37d9 100755 --- a/configure +++ b/configure @@ -181,6 +181,7 @@ smartcard_nss="" usb_redir="" opengl="" zlib="yes" +guest_agent="yes" # parse CC options first for opt do @@ -757,6 +758,10 @@ for opt do ;; --disable-zlib-test) zlib="no" ;; + --enable-guest-agent) guest_agent="yes" + ;; + --disable-guest-agent) guest_agent="no" + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac @@ -1035,6 +1040,8 @@ echo " --disable-smartcard-nss disable smartcard nss support" echo " --enable-smartcard-nss enable smartcard nss support" echo " --disable-usb-redir disable usb network redirection support" echo " --enable-usb-redir enable usb network redirection support" +echo " --disable-guest-agentdisable building of the QEMU Guest Agent" +echo " --enable-guest-agent enable building of the QEMU Guest Agent" echo "" echo "NOTE: The object files are built at the place where configure is launched" exit 1 @@ -1094,11 +1101,13 @@ if test "$solaris" = "yes" ; then fi fi -if has $python; then - : -else - echo "Python not found. Use --python=/path/to/python" - exit 1 +if test "$guest_agent" != "no" ; then + if has $python; then +: + else +echo "Python not found. Use --python=/path/to/python" +exit 1 + fi fi if test -z "$target_list" ; then @@ -1830,14 +1839,16 @@ fi ## # glib support probe -if $pkg_config --modversion glib-2.0> /dev/null 2>&1 ; then -glib_cflags=`$pkg_config --cflags glib-2.0 2>/dev/null` -glib_libs=`$pkg_config --libs glib-2.0 2>/dev/null` -libs_softmmu="$glib_libs $libs_softmmu" -libs_tools="$glib_libs $libs_tools" -else -echo "glib-2.0 required to compile QEMU" -exit 1 +if test "$guest_agent" != "no" ; then +if $pkg_config --modversion glib-2.0> /dev/null 2>&1 ; then +glib_cflags=`$pkg_config --cflags glib-2.0 2>/dev/null` +glib_libs=`$pkg_config --libs glib-2.0 2>/dev/null` +libs_softmmu="$glib_libs $libs_softmmu" +libs_tools="$glib_libs $libs_tools" +else +echo "glib-2.0 required to compile QEMU" +exit 1 +fi fi ## @@ -2597,7 +2608,9 @@ if test "$softmmu" = yes ; then tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools" if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then tools="qemu-nbd\$(EXESUF) $tools" +if [ "$guest_agent" = "yes" ]; then tools="qemu-ga\$(EXESUF) $tools" +fi if [ "$check_utests" = "yes" ]; then tools="check-qint check-qstring check-qdict check-qlist $tools" tools="check-qfloat check-qjson $tools" @@ -2699,6 +2712,7 @@ echo "xfsctl support$xfs" echo "nss used $smartcard_nss" echo "usb net redir $usb_redir" echo "OpenGL support$opengl" +echo "build guest agent $guest_agent" if test $sdl_too_old = "yes"; then echo "-> Your SDL version is too old - please upgrade to have SDL support"
Re: [Qemu-devel] OpenBSD/macppc and sparc64 failing to boot with similar error.
On Mon, Aug 1, 2011 at 12:33 AM, Brad wrote: > I know sparc64 had little chance of actually working but > I thought I'd take it for a spin with 0.15.0-rc1 and > see how it fared in addition to macppc which has a good > chance of working nowdays with modern QEMU. Lets see > what QEMU and related bugs are left.. > > I noticed the bootblocks for each respective arch are > failing in a very similar manner. I would guess that > this is most likely a bug with the OpenBIOS Open Firmware > implementation? > > Any assistance with this? Blue? > > > $ qemu-system-ppc -nographic -cdrom cd50.iso -boot d > >>> = >>> OpenBIOS 1.0 [Jun 16 2011 08:02] >>> Configuration device id QEMU version 1 machine id 2 >>> CPUs: 1 >>> Memory: 128M >>> UUID: ---- >>> CPU type PowerPC,750 > Welcome to OpenBIOS v1.0 built on Jun 16 2011 08:02 > Trying cd:,\\:tbxi... >>> OpenBSD/macppc BOOT 1.1 > open(cd:/etc/boot.conf): Unknown error: code 24 > boot> ? > > > $ qemu-system-sparc64 -nographic -cdrom cd50.iso -boot d > OpenBIOS for Sparc64 > Configuration device id QEMU version 1 machine id 0 > kernel cmdline > CPUs: 1 x SUNW,UltraSPARC-IIi > UUID: ---- > Welcome to OpenBIOS v1.0 built on Jul 20 2011 21:17 > Type 'help' for detailed information > Trying cdrom:f... > Not a bootable ELF image > Not a bootable a.out image > > Loading FCode image... > Loaded 4829 bytes > entry point is 0x4000 > OpenBSD IEEE 1275 Bootblock 1.3 > .. > Jumping to entry point 0010 for type 0001... > switching to new context: entry point 0x10 stack 0xffe86b49 >>> OpenBSD BOOT 1.4 > Trying bsd... > open > : > Unknown error: code 24 > > Boot: I get this with OpenBSD 4.3: OpenBIOS for Sparc64 Configuration device id QEMU version 1 machine id 0 kernel cmdline CPUs: 1 x SUNW,UltraSPARC-IIi UUID: ---- Welcome to OpenBIOS v1.0 built on Jul 20 2011 21:17 Type 'help' for detailed information Trying cdrom:f... Not a bootable ELF image Not a bootable a.out image Loading FCode image... Loaded 4893 bytes entry point is 0x4000 OpenBSD IEEE 1275 Bootblock 1.1 .. Jumping to entry point 0080 for type 0001... switching to new context: entry point 0x80 stack 0xffe86b49 >> OpenBSD BOOT 1.2 Trying bsd... Booting cdrom:f/bsd 2590536@0x100+3236576@0x180+957728@0x1b162e0 symbols @ 0xffcda1c0 36 start=0x100 CHAINprom_get_msgbuf: test failed prom_get_msgbuf: allocated new buf at prom_get_msgbuf: claiming new buf at
[Qemu-devel] [PATCH] Sparc: fix non-faulting unassigned memory accesses
Commit b14ef7c9ab41ea824c3ccadb070ad95567cca84e
introduced cpu_unassigned_access() function. On Sparc,
the function does not restore AREG0 used for global CPUState
on function exit, causing bugs with non-faulting unassigned
memory accesses. Alpha, Microblaze and MIPS are not affected.
Fix by restoring AREG0 on exit. Remove excess saving by
do_unassigned_access() functions.
Also ignore unassigned accesses outside of CPU context.
Reported-by: Bob Breuer
Signed-off-by: Blue Swirl
---
target-sparc/op_helper.c | 25 -
1 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index c1c4d4b..5aeca2b 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -4252,13 +4252,8 @@ void tlb_fill(target_ulong addr, int is_write,
int mmu_idx, void *retaddr)
static void do_unassigned_access(target_phys_addr_t addr, int is_write,
int is_exec, int is_asi, int size)
{
-CPUState *saved_env;
int fault_type;
-/* XXX: hack to restore env in all cases, even if not called from
- generated code */
-saved_env = env;
-env = cpu_single_env;
#ifdef DEBUG_UNASSIGNED
if (is_asi)
printf("Unassigned mem %s access of %d byte%s to " TARGET_FMT_plx
@@ -4306,8 +4301,6 @@ static void
do_unassigned_access(target_phys_addr_t addr, int is_write,
if (env->mmuregs[0] & MMU_NF) {
tlb_flush(env, 1);
}
-
-env = saved_env;
}
#endif
#else
@@ -4319,13 +4312,6 @@ static void
do_unassigned_access(target_phys_addr_t addr, int is_write,
int is_exec, int is_asi, int size)
#endif
{
-CPUState *saved_env;
-
-/* XXX: hack to restore env in all cases, even if not called from
- generated code */
-saved_env = env;
-env = cpu_single_env;
-
#ifdef DEBUG_UNASSIGNED
printf("Unassigned mem access to " TARGET_FMT_plx " from " TARGET_FMT_lx
"\n", addr, env->pc);
@@ -4335,8 +4321,6 @@ static void
do_unassigned_access(target_phys_addr_t addr, int is_write,
raise_exception(TT_CODE_ACCESS);
else
raise_exception(TT_DATA_ACCESS);
-
-env = saved_env;
}
#endif
@@ -4370,7 +4354,14 @@ void helper_tick_set_limit(void *opaque, uint64_t limit)
void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
int is_write, int is_exec, int is_asi, int size)
{
+CPUState *saved_env;
+
+saved_env = env;
env = env1;
-do_unassigned_access(addr, is_write, is_exec, is_asi, size);
+/* Ignore unassigned accesses outside of CPU context */
+if (env1) {
+do_unassigned_access(addr, is_write, is_exec, is_asi, size);
+}
+env = saved_env;
}
#endif
--
1.6.2.4
From 3e3dbb411f07fecff173eea4f6f6d52182d9945c Mon Sep 17 00:00:00 2001
Message-Id: <3e3dbb411f07fecff173eea4f6f6d52182d9945c.1312234275.git.blauwir...@gmail.com>
From: Blue Swirl
Date: Mon, 1 Aug 2011 21:26:03 +
Subject: [PATCH] Sparc: fix non-faulting unassigned memory accesses
Commit b14ef7c9ab41ea824c3ccadb070ad95567cca84e
introduced cpu_unassigned_access() function. On Sparc,
the function does not restore AREG0 used for global CPUState
on function exit, causing bugs with non-faulting unassigned
memory accesses. Alpha, Microblaze and MIPS are not affected.
Fix by restoring AREG0 on exit. Remove excess saving by
do_unassigned_access() functions.
Also ignore unassigned accesses outside of CPU context.
Reported-by: Bob Breuer
Signed-off-by: Blue Swirl
---
target-sparc/op_helper.c | 25 -
1 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index c1c4d4b..5aeca2b 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -4252,13 +4252,8 @@ void tlb_fill(target_ulong addr, int is_write, int mmu_idx, void *retaddr)
static void do_unassigned_access(target_phys_addr_t addr, int is_write,
int is_exec, int is_asi, int size)
{
-CPUState *saved_env;
int fault_type;
-/* XXX: hack to restore env in all cases, even if not called from
- generated code */
-saved_env = env;
-env = cpu_single_env;
#ifdef DEBUG_UNASSIGNED
if (is_asi)
printf("Unassigned mem %s access of %d byte%s to " TARGET_FMT_plx
@@ -4306,8 +4301,6 @@ static void do_unassigned_access(target_phys_addr_t addr, int is_write,
if (env->mmuregs[0] & MMU_NF) {
tlb_flush(env, 1);
}
-
-env = saved_env;
}
#endif
#else
@@ -4319,13 +4312,6 @@ static void do_unassigned_access(target_phys_addr_t addr, int is_write,
int is_exec, int is_asi, int size)
#endif
{
-CPUState *saved_env;
-
-/* XXX: hack to restore env in all cases, even if not called from
- generated code */
-saved_env = env;
-env = cpu_single_env;
-
#ifdef DEBUG_UNASSIGNED
printf("Unassig
[Qemu-devel] [RFC] Alpha system patchset updated for Memory API, batch 2: PCI devices
On 07/31/2011 10:57 AM, Avi Kivity wrote:
> Also available from:
>
> git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory-region-b2
I've updated my alpha port based off of this. It seems to work
just about as well as it did beforehand.
git://repo.or.cz/qmu/rth.git axp-system-6
The interesting patch for the memory region stuff is appended,
if you'd like to look it over.
There's a hack or two that will be removable when ISA devices
are also updated for the memory api.
The major thing that's missing is 8-byte accesses. But I guess
that has to wait until all devices are updated and we can have
softmmu_template.h call into memory.c directly. For the moment
I'm continuing to latch two 4-byte accesses in a gross way.
r~
---
commit 862094979877efaf2b1cebd9853cdfac7572e8f1
Author: Richard Henderson
Date: Mon Apr 18 16:14:11 2011 -0700
target-alpha: Add CLIPPER emulation.
This is a DP264 variant, SMP capable, no unusual hardware present.
The emulation does not currently include any PCI IOMMU code.
Hopefully the generic support for that can be merged to HEAD soon.
Signed-off-by: Richard Henderson
diff --git a/Makefile.target b/Makefile.target
index 0f42eaa..c73727a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -372,6 +372,7 @@ obj-s390x-y = s390-virtio-bus.o s390-virtio.o
obj-alpha-y = i8259.o mc146818rtc.o
obj-alpha-y += vga.o cirrus_vga.o
+obj-alpha-y += alpha_pci.o alpha_dp264.o alpha_typhoon.o
main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
diff --git a/default-configs/alpha-softmmu.mak
b/default-configs/alpha-softmmu.mak
index abadcff..be86d0c 100644
--- a/default-configs/alpha-softmmu.mak
+++ b/default-configs/alpha-softmmu.mak
@@ -3,7 +3,9 @@
include pci.mak
CONFIG_SERIAL=y
CONFIG_I8254=y
+CONFIG_PCKBD=y
CONFIG_VGA_PCI=y
CONFIG_IDE_CORE=y
CONFIG_IDE_QDEV=y
CONFIG_VMWARE_VGA=y
+CONFIG_IDE_CMD646=y
diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c
new file mode 100644
index 000..18560db
--- /dev/null
+++ b/hw/alpha_dp264.c
@@ -0,0 +1,189 @@
+/*
+ * QEMU Alpha DP264/CLIPPER hardware system emulator.
+ *
+ * Choose CLIPPER IRQ mappings over, say, DP264, MONET, or WEBBRICK
+ * variants because CLIPPER doesn't have an SMC669 SuperIO controler
+ * that we need to emulate as well.
+ */
+
+#include "hw.h"
+#include "elf.h"
+#include "loader.h"
+#include "boards.h"
+#include "alpha_sys.h"
+#include "sysemu.h"
+#include "mc146818rtc.h"
+#include "ide.h"
+
+#define MAX_IDE_BUS 2
+
+static uint64_t cpu_alpha_superpage_to_phys(void *opaque, uint64_t addr)
+{
+if (((addr >> 41) & 3) == 2) {
+addr &= 0xffull;
+}
+return addr;
+}
+
+/* Note that there are at least 3 viewpoints of IRQ numbers on Alpha systems.
+(0) The dev_irq_n lines into the cpu, which we totally ignore,
+(1) The DRIR lines in the typhoon chipset,
+(2) The "vector" aka mangled interrupt number reported by SRM PALcode,
+(3) The interrupt number assigned by the kernel.
+ The following function is concerned with (1) only. */
+
+static int clipper_pci_map_irq(PCIDevice *d, int irq_num)
+{
+int slot = d->devfn >> 3;
+
+assert(irq_num >= 0 && irq_num <= 3);
+
+return (slot + 1) * 4 + irq_num;
+}
+
+static void clipper_init(MemoryRegion *address_space_mem,
+ MemoryRegion *address_space_io,
+ ram_addr_t ram_size,
+ const char *boot_device,
+ const char *kernel_filename,
+ const char *kernel_cmdline,
+ const char *initrd_filename,
+ const char *cpu_model)
+{
+CPUState *cpus[4];
+ram_addr_t ram_offset;
+PCIBus *pci_bus;
+qemu_irq isa_pci_irq, rtc_irq, *isa_irqs;
+long size, i;
+const char *palcode_filename;
+uint64_t palcode_entry, palcode_low, palcode_high;
+uint64_t kernel_entry, kernel_low, kernel_high;
+
+/* Create up to 4 cpus. */
+memset(cpus, 0, sizeof(cpus));
+for (i = 0; i < smp_cpus; ++i) {
+cpus[i] = cpu_init(cpu_model ? cpu_model : "ev67");
+}
+
+cpus[0]->trap_arg0 = ram_size;
+cpus[0]->trap_arg1 = 0;
+cpus[0]->trap_arg2 = smp_cpus;
+
+ram_offset = qemu_ram_alloc(NULL, "ram", ram_size);
+cpu_register_physical_memory(0, ram_size, ram_offset);
+
+/* Init the chipset. */
+pci_bus = typhoon_init(address_space_mem, address_space_io,
+ &isa_pci_irq, &rtc_irq, cpus, clipper_pci_map_irq);
+
+/* Init the ISA bus. */
+isa_bus_new(NULL);
+isa_irqs = i8259_init(isa_pci_irq);
+isa_bus_irqs(isa_irqs);
+
+rtc_init(1980, rtc_irq);
+pit_init(0x40, 0);
+isa_create_simple("i8042");
+
+/* VGA setup. Don't bother loading the bios. */
+alpha_pci_vga_setup(pci_bus);
+
+/* Serial code setup. */
+for (i = 0; i < MAX_SERIAL_PORTS; ++i) {
+if (serial_hds[i]) {
+serial_isa_init(i, serial_hds[i]);
+}
+
Re: [Qemu-devel] [RFC PATCH v2 1/3] separate thread for VM migration
On 08/01/2011 05:37 AM, Paolo Bonzini wrote:
On 07/29/2011 10:57 PM, Umesh Deshpande wrote:
This patch creates a separate thread for the guest migration on the
source side.
Signed-off-by: Umesh Deshpande
Looks pretty good!
One thing that shows, is that the interface separation between
buffered_file.c is migration.c is pretty weird. Your patch makes it
somewhat worse, but it was like this before so it's not your fault.
The good thing is that if buffered_file.c uses threads, you can fix a
large part of this and get even simpler code:
1) there is really just one way to implement migrate_fd_put_notify,
and with your simplifications it does not belong anymore in migration.c.
2) s->callback is actually not NULL exactly if s->file->frozen_output
is true, you can remove it as well;
3) buffered_close is messy because it can be called from both the
iothread (monitor->migrate_fd_cancel->migrate_fd_cleanup->qemu_fclose)
or the migration thread (after qemu_savevm_state_complete). But
buffered_close is actually very similar to your thread function (it
does flush+wait_for_unfreeze, basically)! So buffered_close can be
simply:
s->closed = 1;
ret = qemu_thread_join(s->thread); /* doesn't exist yet :) */
qemu_free(...);
return ret;
Another nit is that here:
+if (migrate_fd_check_expire()) {
+buffered_rate_tick(s->file);
+}
+
+if (s->state != MIG_STATE_ACTIVE) {
+break;
+}
+
+if (s->callback) {
+migrate_fd_wait_for_unfreeze(s);
+s->callback(s);
+}
you can still have a busy wait.
Putting it all together, you can move the thread function back to
buffered_file.c like:
while (!s->closed || (!s->has_error && s->buffer_size)) {
if (s->freeze_output) {
qemu_mutex_unlock_iothread();
s->wait_for_unfreeze(s);
qemu_mutex_lock_iothread();
/* This comes from qemu_file_put_notify (via
buffered_put_buffer---can be simplified a lot too?).
s->freeze_output = 0;
/* Test again for cancellation. */
continue;
}
int64_t current_time = qemu_get_clock_ms(rt_clock);
if (s->expire_time > current_time) {
struct timeval tv = { .tv_sec = 0, .tv_usec = ... };
qemu_mutex_unlock_iothread();
select (0, NULL, NULL, NULL, &tv);
qemu_mutex_lock_iothread();
s->expire_time = qemu_get_clock_ms(rt_clock) + 100;
continue;
}
/* This comes from buffered_rate_tick. */
s->bytes_xfer = 0;
buffered_flush(s);
if (!s->closed) {
s->put_ready(s->opaque);
}
}
ret = s->close(s->opaque);
...
Does it look sane?
I kept this in migration.c to call qemu_savevm_state_begin. (The way it
is done currently. i.e. to keep access to FdMigrationState in migration.c)
Calling it from buffered_file.c would be inconsistent in that sense. or
we will have to call it from the iothread before spawning the migration
thread.
Also why is the separation between FdMigrationState and QEMUFileBuffered
is required. Is QEMUFileBuffered designed to use also for things other
than migration?
Thanks
Umesh
Paolo
Re: [Qemu-devel] [PATCH v2] Fix unassigned memory access handling
On Mon, Aug 1, 2011 at 4:26 PM, Bob Breuer wrote:
> Blue Swirl wrote:
>> cea5f9a28faa528b6b1b117c9ab2d8828f473fef exposed bugs in unassigned memory
>> access handling. Fix them by always passing CPUState to the handlers.
>>
>> Reported-by: Hervé Poussineau
>> Signed-off-by: Blue Swirl
>> ---
>> v2: don't try to restore env since all targets eventually always call
>> cpu_loop_exit() which will not return.
>>
>> exec-all.h | 2 +-
>> exec.c | 12 ++--
>> target-alpha/cpu.h | 5 +++--
>> target-alpha/op_helper.c | 6 --
>> target-microblaze/cpu.h | 4 ++--
>> target-microblaze/op_helper.c | 14 --
>> target-mips/cpu.h | 4 ++--
>> target-mips/op_helper.c | 6 --
>> target-sparc/cpu.h | 4 ++--
>> target-sparc/op_helper.c | 26 --
>> 10 files changed, 48 insertions(+), 35 deletions(-)
>>
>> diff --git a/exec-all.h b/exec-all.h
>> index 69acf3b..9b8d62c 100644
>> --- a/exec-all.h
>> +++ b/exec-all.h
>> @@ -323,7 +323,7 @@ static inline tb_page_addr_t
>> get_page_addr_code(CPUState *env1, target_ulong add
>> pd = env1->tlb_table[mmu_idx][page_index].addr_code & ~TARGET_PAGE_MASK;
>> if (pd > IO_MEM_ROM && !(pd & IO_MEM_ROMD)) {
>> #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SPARC)
>> - do_unassigned_access(addr, 0, 1, 0, 4);
>> + cpu_unassigned_access(env1, addr, 0, 1, 0, 4);
>> #else
>> cpu_abort(env1, "Trying to execute code outside RAM or ROM at
>> 0x" TARGET_FMT_lx "\n", addr);
>> #endif
>> diff --git a/exec.c b/exec.c
>> index f1777e6..16e16f3 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3236,7 +3236,7 @@ static uint32_t unassigned_mem_readb(void
>> *opaque, target_phys_addr_t addr)
>> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 0, 0, 0, 1);
>> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 1);
>> #endif
>> return 0;
>> }
>> @@ -3247,7 +3247,7 @@ static uint32_t unassigned_mem_readw(void
>> *opaque, target_phys_addr_t addr)
>> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 0, 0, 0, 2);
>> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 2);
>> #endif
>> return 0;
>> }
>> @@ -3258,7 +3258,7 @@ static uint32_t unassigned_mem_readl(void
>> *opaque, target_phys_addr_t addr)
>> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 0, 0, 0, 4);
>> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 4);
>> #endif
>> return 0;
>> }
>> @@ -3269,7 +3269,7 @@ static void unassigned_mem_writeb(void *opaque,
>> target_phys_addr_t addr, uint32_
>> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 1, 0, 0, 1);
>> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 1);
>> #endif
>> }
>>
>> @@ -3279,7 +3279,7 @@ static void unassigned_mem_writew(void *opaque,
>> target_phys_addr_t addr, uint32_
>> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 1, 0, 0, 2);
>> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 2);
>> #endif
>> }
>>
>> @@ -3289,7 +3289,7 @@ static void unassigned_mem_writel(void *opaque,
>> target_phys_addr_t addr, uint32_
>> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 1, 0, 0, 4);
>> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 4);
>> #endif
>> }
>>
>
> [..snip..]
>
>> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
>> index 4edae78..f6cb300 100644
>> --- a/target-sparc/cpu.h
>> +++ b/target-sparc/cpu.h
>> @@ -510,8 +510,8 @@ static inline int tlb_compare_context(const
>> SparcTLBEntry *tlb,
>>
>> /* cpu-exec.c */
>> #if !defined(CONFIG_USER_ONLY)
>> -void do_unassigned_access(target_phys_addr_t addr, int is_write, int
>> is_exec,
>> - int is_asi, int size);
>> +void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
>> + int is_write, int is_exec, int is_asi, int size);
>> target_phys_addr_t cpu_get_phys_page_nofault(CPUState *env, target_ulong
>> addr,
>> int
Re: [Qemu-devel] [PATCH] Fix duplicate device reset
Am 01.08.2011 13:00, schrieb Isaku Yamahata: Hi, here is the patch. Can you please give it a try? From 41039df3174fa46477c4faf93d13eab360dccc22 Mon Sep 17 00:00:00 2001 Message-Id: <41039df3174fa46477c4faf93d13eab360dccc22.1312196365.git.yamah...@valinux.co.jp> From: Isaku Yamahata Date: Mon, 1 Aug 2011 19:56:42 +0900 Subject: [PATCH] qdev: Fix duplicate reset qbus_reset_all_fn was registered twice, so a lot of device reset functions were also called twice when QEMU started. Which was introduced by 80376c3fc2c38fdd45354e4b0eb45031f35587ed This patch fixes it by making the main_sytem_bus creation not register main_system_bus reset handler. Cc: Stefan Weil Signed-off-by: Isaku Yamahata --- hw/qdev.c | 14 -- 1 files changed, 12 insertions(+), 2 deletions(-) Thanks. I tested your patch with i386-softmmu (bios only) and with mipsel-softmmu (debian boot / malta). All registered reset functions were called only once, and qbus_reset_all_fn was the last one called. I noticed that there are two functions named piix3_reset. One might be renamed to piix3_ide_reset, but this is not related to your patch. There are also two functions piix4_reset. Tested-by: Stefan Weil
Re: [Qemu-devel] [PATCH v4 3/3] The support for queue timer and throttling algorithm
* Zhi Yong Wu [2011-08-01 01:32]:
> Note:
> 1.) When bps/iops limits are specified to a small value such as 511
> bytes/s, this VM will hang up. We are considering how to handle this senario.
> 2.) When "dd" command is issued in guest, if its option bs is set to a
> large value such as "bs=1024K", the result speed will slightly bigger than
> the limits.
>
> For these problems, if you have nice thought, pls let us know.:)
>
> Signed-off-by: Zhi Yong Wu
> ---
> block.c | 302
> +--
> block.h |1 -
> block_int.h | 29 ++
> 3 files changed, 323 insertions(+), 9 deletions(-)
>
> diff --git a/block.c b/block.c
> index 24a25d5..42763a3 100644
> --- a/block.c
> +++ b/block.c
> @@ -29,6 +29,9 @@
> #include "module.h"
> #include "qemu-objects.h"
>
> +#include "qemu-timer.h"
> +#include "block/blk-queue.h"
> +
> #ifdef CONFIG_BSD
> #include
> #include
> @@ -58,6 +61,13 @@ static int bdrv_read_em(BlockDriverState *bs, int64_t
> sector_num,
> static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num,
> const uint8_t *buf, int nb_sectors);
>
> +static bool bdrv_exceed_bps_limits(BlockDriverState *bs, int nb_sectors,
> +bool is_write, double elapsed_time, uint64_t *wait);
> +static bool bdrv_exceed_iops_limits(BlockDriverState *bs, bool is_write,
> +double elapsed_time, uint64_t *wait);
> +static bool bdrv_exceed_io_limits(BlockDriverState *bs, int nb_sectors,
> +bool is_write, uint64_t *wait);
> +
> static QTAILQ_HEAD(, BlockDriverState) bdrv_states =
> QTAILQ_HEAD_INITIALIZER(bdrv_states);
>
> @@ -90,6 +100,20 @@ int is_windows_drive(const char *filename)
> }
> #endif
>
> +static int bdrv_io_limits_enable(BlockIOLimit *io_limits)
> +{
> +if ((io_limits->bps[0] == 0)
> + && (io_limits->bps[1] == 0)
> + && (io_limits->bps[2] == 0)
> + && (io_limits->iops[0] == 0)
> + && (io_limits->iops[1] == 0)
> + && (io_limits->iops[2] == 0)) {
> +return 0;
I'd add a field to BlockIOLimit structure, and just do:
static int bdrv_io_limits_enabled(BlockIOLimit *io_limits)
{
return io_limist->enabled;
}
Update bdrv_set_io_limits() to do the original check you have, and if
one of the fields is set, update the enabled flag.
We incur that logic on *every* request, so let's make it as cheap as
possible.
> +}
> +
> +return 1;
> +}
> +
> /* check if the path starts with ":" */
> static int path_has_protocol(const char *path)
> {
> @@ -167,6 +191,28 @@ void path_combine(char *dest, int dest_size,
> }
> }
>
> +static void bdrv_block_timer(void *opaque)
> +{
> +BlockDriverState *bs = opaque;
> +BlockQueue *queue = bs->block_queue;
> +
> +while (!QTAILQ_EMPTY(&queue->requests)) {
> +BlockIORequest *request = NULL;
> +int ret = 0;
> +
> +request = QTAILQ_FIRST(&queue->requests);
> +QTAILQ_REMOVE(&queue->requests, request, entry);
> +
> +ret = qemu_block_queue_handler(request);
> +if (ret == 0) {
> +QTAILQ_INSERT_HEAD(&queue->requests, request, entry);
> +break;
btw, I did some tracing and I never saw a request get re-added here.
Now, ideally, I think you were try to do the following:
The request is coming from the queue, if we exceed our limits, then we'd
get back NULL from the handler and we'd requeue the request at the
head... but that doesn't actually happen.
Rather, if we exceed our limits, we invoke qemu_block_queue_enqueue()
again, which allocates a whole new request and puts it at the tail.
I think we want to update the throttling logic in readv/writev to return
NULL if bs->req_from_queue == true and we exceed the limits. Then this
logic will do the right thing by inserting the request back to the head.
> +}
> +
> +qemu_free(request);
See my email to blk-queue.c on how we can eliminate free'ing the request
here.
> +}
> +}
> +
> void bdrv_register(BlockDriver *bdrv)
> {
> if (!bdrv->bdrv_aio_readv) {
> @@ -642,6 +688,19 @@ int bdrv_open(BlockDriverState *bs, const char
> *filename, int flags,
> bs->change_cb(bs->change_opaque, CHANGE_MEDIA);
> }
>
> +/* throttling disk I/O limits */
> +if (bdrv_io_limits_enable(&bs->io_limits)) {
> +bs->req_from_queue = false;
> +bs->block_queue= qemu_new_block_queue();
> +bs->block_timer= qemu_new_timer_ns(vm_clock, bdrv_block_timer,
> bs);
> +
> +bs->slice_start[0] = qemu_get_clock_ns(vm_clock);
> +bs->slice_start[1] = qemu_get_clock_ns(vm_clock);
> +
> +bs->slice_end[0] = qemu_get_clock_ns(vm_clock) +
> BLOCK_IO_SLICE_TIME;
> +bs->slice_end[1] = qemu_get_clock_ns(vm_clock) +
> BLOCK_IO_SLICE_TIME;
> +}
> +
> return 0;
>
> unlink_and_fail:
> @@ -680,6 +739,16 @@ void bdrv_close(BlockDriverState *bs)
> if (bs->change
Re: [Qemu-devel] [PATCH 20/39] virtio-pci: convert to memory API
On 08/01/2011 05:23 AM, Michael S. Tsirkin wrote:
On Mon, Aug 01, 2011 at 12:35:44PM +0300, Avi Kivity wrote:
On 08/01/2011 11:26 AM, Michael S. Tsirkin wrote:
static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
uint32_t val, int len)
{
VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+VirtIODevice *vdev = proxy->vdev;
if (PCI_COMMAND == address) {
if (!(val& PCI_COMMAND_MASTER)) {
@@ -525,6 +503,9 @@ static void virtio_write_config(PCIDevice *pci_dev,
uint32_t address,
}
}
}
+if (address == PCI_BASE_ADDRESS_0&& vdev->config_len) {
+vdev->get_config(vdev, vdev->config);
+}
pci_default_write_config(pci_dev, address, val, len);
msix_write_config(pci_dev, address, val, len);
I'm not really sure why did we get the config on map,
specifically - Anthony, do you know?
But if we want to do that, memory space enable might
be a better place. Or maybe we just want a callback on
map.
Just because a memory region becomes visible to the cpu is no reason
to have a callback. From the device perspective, it can't tell that
it happened.
Well, the reason we have this logic here, I think, is
to make sure it runs before the guest accesses
the configuration with a write access.
I'm not sure why we don't do this in the init
callback - Anthony?
It could be done in init I think. I can't recall why it didn't start
that way initially.
Regards,
Anthony Liguori
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v2] Fix unassigned memory access handling
On Mon, Aug 1, 2011 at 4:26 PM, Bob Breuer wrote:
> Blue Swirl wrote:
>> cea5f9a28faa528b6b1b117c9ab2d8828f473fef exposed bugs in unassigned memory
>> access handling. Fix them by always passing CPUState to the handlers.
>>
>> Reported-by: Hervé Poussineau
>> Signed-off-by: Blue Swirl
>> ---
>> v2: don't try to restore env since all targets eventually always call
>> cpu_loop_exit() which will not return.
>>
>> exec-all.h | 2 +-
>> exec.c | 12 ++--
>> target-alpha/cpu.h | 5 +++--
>> target-alpha/op_helper.c | 6 --
>> target-microblaze/cpu.h | 4 ++--
>> target-microblaze/op_helper.c | 14 --
>> target-mips/cpu.h | 4 ++--
>> target-mips/op_helper.c | 6 --
>> target-sparc/cpu.h | 4 ++--
>> target-sparc/op_helper.c | 26 --
>> 10 files changed, 48 insertions(+), 35 deletions(-)
>>
>> diff --git a/exec-all.h b/exec-all.h
>> index 69acf3b..9b8d62c 100644
>> --- a/exec-all.h
>> +++ b/exec-all.h
>> @@ -323,7 +323,7 @@ static inline tb_page_addr_t
>> get_page_addr_code(CPUState *env1, target_ulong add
>> pd = env1->tlb_table[mmu_idx][page_index].addr_code & ~TARGET_PAGE_MASK;
>> if (pd > IO_MEM_ROM && !(pd & IO_MEM_ROMD)) {
>> #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SPARC)
>> - do_unassigned_access(addr, 0, 1, 0, 4);
>> + cpu_unassigned_access(env1, addr, 0, 1, 0, 4);
>> #else
>> cpu_abort(env1, "Trying to execute code outside RAM or ROM at
>> 0x" TARGET_FMT_lx "\n", addr);
>> #endif
>> diff --git a/exec.c b/exec.c
>> index f1777e6..16e16f3 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3236,7 +3236,7 @@ static uint32_t unassigned_mem_readb(void
>> *opaque, target_phys_addr_t addr)
>> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 0, 0, 0, 1);
>> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 1);
>> #endif
>> return 0;
>> }
>> @@ -3247,7 +3247,7 @@ static uint32_t unassigned_mem_readw(void
>> *opaque, target_phys_addr_t addr)
>> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 0, 0, 0, 2);
>> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 2);
>> #endif
>> return 0;
>> }
>> @@ -3258,7 +3258,7 @@ static uint32_t unassigned_mem_readl(void
>> *opaque, target_phys_addr_t addr)
>> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 0, 0, 0, 4);
>> + cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 4);
>> #endif
>> return 0;
>> }
>> @@ -3269,7 +3269,7 @@ static void unassigned_mem_writeb(void *opaque,
>> target_phys_addr_t addr, uint32_
>> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 1, 0, 0, 1);
>> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 1);
>> #endif
>> }
>>
>> @@ -3279,7 +3279,7 @@ static void unassigned_mem_writew(void *opaque,
>> target_phys_addr_t addr, uint32_
>> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 1, 0, 0, 2);
>> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 2);
>> #endif
>> }
>>
>> @@ -3289,7 +3289,7 @@ static void unassigned_mem_writel(void *opaque,
>> target_phys_addr_t addr, uint32_
>> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
>> #endif
>> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
>> defined(TARGET_MICROBLAZE)
>> - do_unassigned_access(addr, 1, 0, 0, 4);
>> + cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 4);
>> #endif
>> }
>>
>
> [..snip..]
>
>> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
>> index 4edae78..f6cb300 100644
>> --- a/target-sparc/cpu.h
>> +++ b/target-sparc/cpu.h
>> @@ -510,8 +510,8 @@ static inline int tlb_compare_context(const
>> SparcTLBEntry *tlb,
>>
>> /* cpu-exec.c */
>> #if !defined(CONFIG_USER_ONLY)
>> -void do_unassigned_access(target_phys_addr_t addr, int is_write, int
>> is_exec,
>> - int is_asi, int size);
>> +void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
>> + int is_write, int is_exec, int is_asi, int size);
>> target_phys_addr_t cpu_get_phys_page_nofault(CPUState *env, target_ulong
>> addr,
>> int
Re: [Qemu-devel] [PATCH 20/39] virtio-pci: convert to memory API
On 08/01/2011 03:26 AM, Michael S. Tsirkin wrote:
On Sun, Jul 31, 2011 at 08:57:43PM +0300, Avi Kivity wrote:
@@ -491,30 +473,26 @@ static void virtio_pci_config_writel(void *opaque,
uint32_t addr, uint32_t val)
virtio_config_writel(proxy->vdev, addr, val);
}
-static void virtio_map(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
-{
-VirtIOPCIProxy *proxy = container_of(pci_dev, VirtIOPCIProxy, pci_dev);
-VirtIODevice *vdev = proxy->vdev;
-unsigned config_len = VIRTIO_PCI_REGION_SIZE(pci_dev) + vdev->config_len;
-
-proxy->addr = addr;
-
-register_ioport_write(addr, config_len, 1, virtio_pci_config_writeb,
proxy);
-register_ioport_write(addr, config_len, 2, virtio_pci_config_writew,
proxy);
-register_ioport_write(addr, config_len, 4, virtio_pci_config_writel,
proxy);
-register_ioport_read(addr, config_len, 1, virtio_pci_config_readb, proxy);
-register_ioport_read(addr, config_len, 2, virtio_pci_config_readw, proxy);
-register_ioport_read(addr, config_len, 4, virtio_pci_config_readl, proxy);
+const MemoryRegionPortio virtio_portio[] = {
+{ 0, 0x1, 1, .write = virtio_pci_config_writeb, },
+{ 0, 0x1, 2, .write = virtio_pci_config_writew, },
+{ 0, 0x1, 4, .write = virtio_pci_config_writel, },
+{ 0, 0x1, 1, .read = virtio_pci_config_readb, },
+{ 0, 0x1, 2, .read = virtio_pci_config_readw, },
+{ 0, 0x1, 4, .read = virtio_pci_config_readl, },
+PORTIO_END
+};
-if (vdev->config_len)
-vdev->get_config(vdev, vdev->config);
-}
+static const MemoryRegionOps virtio_pci_config_ops = {
+.old_portio = virtio_portio,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
uint32_t val, int len)
{
VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+VirtIODevice *vdev = proxy->vdev;
if (PCI_COMMAND == address) {
if (!(val& PCI_COMMAND_MASTER)) {
@@ -525,6 +503,9 @@ static void virtio_write_config(PCIDevice *pci_dev,
uint32_t address,
}
}
}
+if (address == PCI_BASE_ADDRESS_0&& vdev->config_len) {
+vdev->get_config(vdev, vdev->config);
+}
pci_default_write_config(pci_dev, address, val, len);
msix_write_config(pci_dev, address, val, len);
I'm not really sure why did we get the config on map,
specifically - Anthony, do you know?
It's just a mechanism to lazily load the config. We could just as
easily read the config whenever the (virtio) config space was accessed.
Regards,
Anthony Liguori
But if we want to do that, memory space enable might
be a better place. Or maybe we just want a callback on
map.
Re: [Qemu-devel] [PATCH v4 2/3] The support for block queue
* Zhi Yong Wu [2011-08-01 01:30]:
> +static AIOPool block_queue_pool = {
> +.aiocb_size = sizeof(struct BlockDriverAIOCB),
> +.cancel = qemu_block_queue_cancel,
> +};
> +
> +static void qemu_block_queue_callback(void *opaque, int ret)
> +{
> +BlockDriverAIOCB *acb = opaque;
> +
> +qemu_aio_release(acb);
> +}
> +
So, here we really want to invoke the original commands callback, and
then free the request here (via qemu_aio_release()). see below.
> +BlockDriverAIOCB *qemu_block_queue_enqueue(BlockQueue *queue,
> +BlockDriverState *bs,
> +BlockRequestHandler *handler,
> +int64_t sector_num,
> +QEMUIOVector *qiov,
> +int nb_sectors,
> +BlockDriverCompletionFunc *cb,
> +void *opaque)
> +{
> +BlockIORequest *request;
> +BlockDriverAIOCB *acb;
> +
> +request = qemu_malloc(sizeof(BlockIORequest));
> +request->bs = bs;
> +request->handler = handler;
> +request->sector_num = sector_num;
> +request->qiov = qiov;
> +request->nb_sectors = nb_sectors;
> +request->cb = cb;
> +request->opaque = opaque;
> +
> +QTAILQ_INSERT_TAIL(&queue->requests, request, entry);
> +
> +acb = qemu_aio_get(&block_queue_pool, bs,
> + qemu_block_queue_callback, opaque);
> +request->acb = acb;
> +
> +return acb;
> +}
> +
> +int qemu_block_queue_handler(BlockIORequest *request)
> +{
> +int ret;
> +BlockDriverAIOCB *res;
> +
> +/* indicate this req is from block queue */
> +request->bs->req_from_queue = true;
> +
> +res = request->handler(request->bs, request->sector_num,
> + request->qiov, request->nb_sectors,
> + request->cb, request->opaque);
> +
> +if (request->acb) {
> +qemu_block_queue_callback(request->acb, 0);
> +}
> +
> +ret = (res == NULL) ? 0 : 1;
> +
> +return ret;
> +}
You don't want to malloc the BlockIORequest directly in _enqueue(), rather
you want to allocate that from your AIOPool via qemu_aio_get(). As it
is now, we're allocating two BlockIORequests (malloc and then a
aio_get()). You'll need a BlockDriverAIOCB in the BlockIORequest
structure. Then in your queue_handler, instead of passing the original
read or write callback (request->cb), you want to hook the block_queue callback
(qemu_block_queue_callback()), in that callback you can then invoke the
request callback and then release the request.
The request should be, only one malloc (via the pool which will re-use
the memory instead of incuring a malloc on every request), and then you
release the memory back to the pool once your request is complete, which
you'll know after wiring up the block_queue callback to the completion
of the request's handler. And then since we don't double allocate, you
won't need to do the qemu_free(request) in block.c in block_timer...
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com
Re: [Qemu-devel] [PATCH V3 1/2] [SLIRP] Simple ARP table
On 2011-08-01 18:18, Fabien Chouteau wrote:
> This patch adds a simple ARP table in Slirp and also adds handling of
> gratuitous ARP requests.
>
> Signed-off-by: Fabien Chouteau
> ---
> Makefile.objs |2 +-
> slirp/arp_table.c | 72
> +
> slirp/bootp.c | 21 +--
> slirp/slirp.c | 63 +++--
> slirp/slirp.h | 47 --
> 5 files changed, 146 insertions(+), 59 deletions(-)
> create mode 100644 slirp/arp_table.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 6991a9f..0c10557 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -151,7 +151,7 @@ common-obj-y += qemu-timer.o qemu-timer-common.o
>
> slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o
> slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp_output.o
> -slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o
> +slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o
> common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
>
> # xen backend driver support
> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
> new file mode 100644
> index 000..c7034ee
> --- /dev/null
> +++ b/slirp/arp_table.c
> @@ -0,0 +1,72 @@
> +#include "slirp.h"
> +
> +void arp_table_add(Slirp *slirp, int ip_addr, uint8_t ethaddr[ETH_ALEN])
> +{
> +const in_addr_t broadcast_addr =
> +~slirp->vnetwork_mask.s_addr | slirp->vnetwork_addr.s_addr;
That's only part of the picture. 255.255.255.255 is a valid broadcast
address as well.
> +ArpTable *arptbl = &slirp->arp_table;
> +int i;
> +
> +DEBUG_CALL("arp_table_add");
> +DEBUG_ARG("ip = 0x%x", ip_addr);
> +DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
> +ethaddr[0], ethaddr[1], ethaddr[2],
> +ethaddr[3], ethaddr[4], ethaddr[5]));
> +
> +if ((ip_addr & ~(0xf << 28)) == 0 ||
> +ip_addr == broadcast_addr) {
> +/* Do not register 0.0.0.0/8 or broadcast addresses */
> +return;
> +}
> +
> +/* Search for an entry */
> +for (i = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0; i++) {
Forgot to remove the test for ar_sip != 0.
> +if (arptbl->table[i].ar_sip == ip_addr) {
> +/* Update the entry */
> +memcpy(arptbl->table[i].ar_sha, ethaddr, ETH_ALEN);
> +return;
> +}
> +}
> +
> +/* No entry found, create a new one */
> +arptbl->table[arptbl->next_victim].ar_sip = ip_addr;
> +memcpy(arptbl->table[arptbl->next_victim].ar_sha, ethaddr, ETH_ALEN);
> +arptbl->next_victim = (arptbl->next_victim + 1) % ARP_TABLE_SIZE;
> +}
> +
> +bool arp_table_search(Slirp *slirp, int in_ip_addr,
> + uint8_t out_ethaddr[ETH_ALEN])
> +{
> +const in_addr_t broadcast_addr =
> +~slirp->vnetwork_mask.s_addr | slirp->vnetwork_addr.s_addr;
Same as above. That means DCHP is still broken. Please include that
scenario in your tests before sending the next round.
> +ArpTable *arptbl = &slirp->arp_table;
> +int i;
> +
> +DEBUG_CALL("arp_table_search");
> +DEBUG_ARG("ip = 0x%x", in_ip_addr);
> +
> +/* If address in 0.0.0.0/8 */
> +if ((in_ip_addr & ~(0xf << 28)) == 0) {
Should rather be an assert() as it means the caller is about to send a
frame to that source-only address.
Jan
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 0/3] The intro for QEMU disk I/O limits
* Zhi Yong Wu [2011-08-01 01:30]: > The main goal of the patch is to effectively cap the disk I/O speed or counts > of one single VM.It is only one draft, so it unavoidably has some drawbacks, > if you catch them, please let me know. > > The patch will mainly introduce one block I/O throttling algorithm, one timer > and one block queue for each I/O limits enabled drive. > > When a block request is coming in, the throttling algorithm will check if its > I/O rate or counts exceed the limits; if yes, then it will enqueue to the > block queue; The timer will handle the I/O requests in it. > > Some available features follow as below: > (1) global bps limit. > -drive bps=xxxin bytes/s > (2) only read bps limit > -drive bps_rd=xxx in bytes/s > (3) only write bps limit > -drive bps_wr=xxx in bytes/s > (4) global iops limit > -drive iops=xxx in ios/s > (5) only read iops limit > -drive iops_rd=xxxin ios/s > (6) only write iops limit > -drive iops_wr=xxxin ios/s > (7) the combination of some limits. > -drive bps=xxx,iops=xxx > > Known Limitations: > (1) #1 can not coexist with #2, #3 > (2) #4 can not coexist with #5, #6 > (3) When bps/iops limits are specified to a small value such as 511 bytes/s, > this VM will hang up. We are considering how to handle this senario. > > > Zhi Yong Wu (3): > v4: fix memory leaking based on ryan's feedback. It looks like the leak has been fixed, but I think we need to rework how the blk-queue is using the AIOPool. I'll reply to that patch. -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx ry...@us.ibm.com
Re: [Qemu-devel] [PATCH] xen: Avoid useless allocation in Xen case.
On 2011-08-01 21:26, Anthony PERARD wrote:
> The code_gen_buffer is not use by Xen and can be really big (several
> GB). Even if the host RAM is not used, this buffer just burn the address
> space of the QEMU process.
>
> So to "avoid" this allocation, the asked tb_size is set to the minimum.
>
> The other way to do that would be to not call code_gen_alloc when Xen is
> enabled.
>
> Signed-off-by: Anthony PERARD
> ---
> vl.c |8
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index d8c7c01..bd60a89 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3106,6 +3106,14 @@ int main(int argc, char **argv, char **envp)
> }
> }
>
> +if (xen_enabled()) {
> +/* Allocate only the minimum amount of memory for the
> code_gen_buffer.
> + * Xen does not use it and we need the virtual address space for the
> + * MapCache.
> + */
> +tb_size = 1;
> +}
> +
The same applies to kvm, please generalize.
Jan
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH] guest agent: add --enable-guest-agent config option
QAPI will require glib/python, but for now the guest agent is the only user. For now, make these dependencies an explicit guest agent one, and give users the option to disable it if need be. Once QAPI is adopted in core QEMU code, we would basically revert this patch. Signed-off-by: Michael Roth --- configure | 40 +++- 1 files changed, 27 insertions(+), 13 deletions(-) diff --git a/configure b/configure index 77194cf..58a37d9 100755 --- a/configure +++ b/configure @@ -181,6 +181,7 @@ smartcard_nss="" usb_redir="" opengl="" zlib="yes" +guest_agent="yes" # parse CC options first for opt do @@ -757,6 +758,10 @@ for opt do ;; --disable-zlib-test) zlib="no" ;; + --enable-guest-agent) guest_agent="yes" + ;; + --disable-guest-agent) guest_agent="no" + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac @@ -1035,6 +1040,8 @@ echo " --disable-smartcard-nss disable smartcard nss support" echo " --enable-smartcard-nss enable smartcard nss support" echo " --disable-usb-redir disable usb network redirection support" echo " --enable-usb-redir enable usb network redirection support" +echo " --disable-guest-agentdisable building of the QEMU Guest Agent" +echo " --enable-guest-agent enable building of the QEMU Guest Agent" echo "" echo "NOTE: The object files are built at the place where configure is launched" exit 1 @@ -1094,11 +1101,13 @@ if test "$solaris" = "yes" ; then fi fi -if has $python; then - : -else - echo "Python not found. Use --python=/path/to/python" - exit 1 +if test "$guest_agent" != "no" ; then + if has $python; then +: + else +echo "Python not found. Use --python=/path/to/python" +exit 1 + fi fi if test -z "$target_list" ; then @@ -1830,14 +1839,16 @@ fi ## # glib support probe -if $pkg_config --modversion glib-2.0 > /dev/null 2>&1 ; then -glib_cflags=`$pkg_config --cflags glib-2.0 2>/dev/null` -glib_libs=`$pkg_config --libs glib-2.0 2>/dev/null` -libs_softmmu="$glib_libs $libs_softmmu" -libs_tools="$glib_libs $libs_tools" -else -echo "glib-2.0 required to compile QEMU" -exit 1 +if test "$guest_agent" != "no" ; then +if $pkg_config --modversion glib-2.0 > /dev/null 2>&1 ; then +glib_cflags=`$pkg_config --cflags glib-2.0 2>/dev/null` +glib_libs=`$pkg_config --libs glib-2.0 2>/dev/null` +libs_softmmu="$glib_libs $libs_softmmu" +libs_tools="$glib_libs $libs_tools" +else +echo "glib-2.0 required to compile QEMU" +exit 1 +fi fi ## @@ -2597,7 +2608,9 @@ if test "$softmmu" = yes ; then tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools" if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then tools="qemu-nbd\$(EXESUF) $tools" +if [ "$guest_agent" = "yes" ]; then tools="qemu-ga\$(EXESUF) $tools" +fi if [ "$check_utests" = "yes" ]; then tools="check-qint check-qstring check-qdict check-qlist $tools" tools="check-qfloat check-qjson $tools" @@ -2699,6 +2712,7 @@ echo "xfsctl support$xfs" echo "nss used $smartcard_nss" echo "usb net redir $usb_redir" echo "OpenGL support$opengl" +echo "build guest agent $guest_agent" if test $sdl_too_old = "yes"; then echo "-> Your SDL version is too old - please upgrade to have SDL support" -- 1.7.0.4
Re: [Qemu-devel] [PATCH 20/39] virtio-pci: convert to memory API
On Mon, Aug 01, 2011 at 12:35:44PM +0300, Avi Kivity wrote:
> On 08/01/2011 11:26 AM, Michael S. Tsirkin wrote:
> >>
> >> static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
> >> uint32_t val, int len)
> >> {
> >> VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
> >> +VirtIODevice *vdev = proxy->vdev;
> >>
> >> if (PCI_COMMAND == address) {
> >> if (!(val& PCI_COMMAND_MASTER)) {
> >> @@ -525,6 +503,9 @@ static void virtio_write_config(PCIDevice *pci_dev,
> >> uint32_t address,
> >> }
> >> }
> >> }
> >> +if (address == PCI_BASE_ADDRESS_0&& vdev->config_len) {
> >> +vdev->get_config(vdev, vdev->config);
> >> +}
> >>
> >> pci_default_write_config(pci_dev, address, val, len);
> >> msix_write_config(pci_dev, address, val, len);
> >
> >I'm not really sure why did we get the config on map,
> >specifically - Anthony, do you know?
> >But if we want to do that, memory space enable might
> >be a better place. Or maybe we just want a callback on
> >map.
>
>
> Just because a memory region becomes visible to the cpu is no reason
> to have a callback. From the device perspective, it can't tell that
> it happened.
BTW this is what qxl does, too, conceptually: on config writes, it peeks
at the bar to detect whether that got unmapped.
> --
> error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH] xen: Avoid useless allocation in Xen case.
The code_gen_buffer is not use by Xen and can be really big (several
GB). Even if the host RAM is not used, this buffer just burn the address
space of the QEMU process.
So to "avoid" this allocation, the asked tb_size is set to the minimum.
The other way to do that would be to not call code_gen_alloc when Xen is
enabled.
Signed-off-by: Anthony PERARD
---
vl.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/vl.c b/vl.c
index d8c7c01..bd60a89 100644
--- a/vl.c
+++ b/vl.c
@@ -3106,6 +3106,14 @@ int main(int argc, char **argv, char **envp)
}
}
+if (xen_enabled()) {
+/* Allocate only the minimum amount of memory for the code_gen_buffer.
+ * Xen does not use it and we need the virtual address space for the
+ * MapCache.
+ */
+tb_size = 1;
+}
+
/* init the dynamic translator */
cpu_exec_init_all(tb_size * 1024 * 1024);
--
Anthony PERARD
[Qemu-devel] [PATCH] xen-mapcache: Fix rlimit set size.
Previously, the address space soft limit was set mcache_max_size. So,
before the mcache_max_size was reached by the mapcache, QEMU was killed
for overuse of the virtual address space.
This patch fix that by setting the soft limit to mcache_max_size + 80MB.
I observed that QEMU use 75MB more than max_mcache_size after several
empirical tests.
Signed-off-by: Anthony PERARD
---
xen-mapcache.c | 13 ++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/xen-mapcache.c b/xen-mapcache.c
index 007136a..40212f7 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -40,6 +40,13 @@
#endif
#define MCACHE_BUCKET_SIZE (1UL << MCACHE_BUCKET_SHIFT)
+/* This is the size of the virtual address space reserve to QEMU that will not
+ * be use by MapCache.
+ * From empirical tests I observed that qemu use 75MB more than the
+ * max_mcache_size.
+ */
+#define NON_MCACHE_MEMORY_SIZE (80 * 1024 * 1024)
+
#define mapcache_lock() ((void)0)
#define mapcache_unlock() ((void)0)
@@ -93,14 +100,14 @@ void xen_map_cache_init(void)
mapcache->last_address_index = -1;
getrlimit(RLIMIT_AS, &rlimit_as);
-if (rlimit_as.rlim_max < MCACHE_MAX_SIZE) {
+if (rlimit_as.rlim_max < MCACHE_MAX_SIZE + NON_MCACHE_MEMORY_SIZE) {
rlimit_as.rlim_cur = rlimit_as.rlim_max;
} else {
-rlimit_as.rlim_cur = MCACHE_MAX_SIZE;
+rlimit_as.rlim_cur = MCACHE_MAX_SIZE + NON_MCACHE_MEMORY_SIZE;
}
setrlimit(RLIMIT_AS, &rlimit_as);
-mapcache->max_mcache_size = rlimit_as.rlim_cur;
+mapcache->max_mcache_size = rlimit_as.rlim_cur - NON_MCACHE_MEMORY_SIZE;
mapcache->nr_buckets =
(((mapcache->max_mcache_size >> XC_PAGE_SHIFT) +
--
Anthony PERARD
[Qemu-devel] glib dependency
Anthony, So in QEMU 0.15 rc2 it looks like the dependency on gio and gthread has been removed, but glib is still required correct? ## # glib support probe -if $pkg_config --modversion gthread-2.0 gio-2.0 > /dev/null 2>&1 ; then -glib_cflags=`$pkg_config --cflags gthread-2.0 gio-2.0 2>/dev/null` -glib_libs=`$pkg_config --libs gthread-2.0 gio-2.0 2>/dev/null` +if $pkg_config --modversion glib-2.0 > /dev/null 2>&1 ; then +glib_cflags=`$pkg_config --cflags glib-2.0 2>/dev/null` +glib_libs=`$pkg_config --libs glib-2.0 2>/dev/null` ...I had the impression that the glib dependency itself was going away. I'm trying to get qemu integrated into our internal SDK system for building user space and am trying to figure out what is required. Stuart
Re: [Qemu-devel] [PATCH 18/39] ide: convert to memory API
On 07/31/2011 10:57 AM, Avi Kivity wrote: > +pci_register_bar_region(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, > +&d->cmd646_bar[2].cmd); Typo: cmd646_bar[1]. r~
Re: [Qemu-devel] kvm PCI assignment & VFIO ramblings
On Sun, 2011-07-31 at 09:54 +1000, Benjamin Herrenschmidt wrote: > On Sat, 2011-07-30 at 12:20 -0600, Alex Williamson wrote: > > > On x86, the USB controllers don't typically live behind a PCIe-to-PCI > > bridge, so don't suffer the source identifier problem, but they do often > > share an interrupt. But even then, we can count on most modern devices > > supporting PCI2.3, and thus the DisINTx feature, which allows us to > > share interrupts. In any case, yes, it's more rare but we need to know > > how to handle devices behind PCI bridges. However I disagree that we > > need to assign all the devices behind such a bridge to the guest. > > Well, ok so let's dig a bit more here :-) First, yes I agree they don't > all need to appear to the guest. My point is really that we must prevent > them to be "used" by somebody else, either host or another guest. > > Now once you get there, I personally prefer having a clear "group" > ownership rather than having devices stay in some "limbo" under vfio > control but it's an implementation detail. > > Regarding DisINTx, well, it's a bit like putting separate PCIe functions > into separate guests, it looks good ... but you are taking a chance. > Note that I do intend to do some of that for power ... well I think, I > haven't completely made my mind. > > pHyp for has a stricter requirement, PEs essentially are everything > behind a bridge. If you have a slot, you have some kind of bridge above > this slot and everything on it will be a PE. > > The problem I see is that with your filtering of config space, BAR > emulation, DisINTx etc... you essentially assume that you can reasonably > reliably isolate devices. But in practice, it's chancy. Some devices for > example have "backdoors" into their own config space via MMIO. If I have > such a device in a guest, I can completely override your DisINTx and > thus DOS your host or another guest with a shared interrupt. I can move > my MMIO around and DOS another function by overlapping the addresses. > > You can really only be protect yourself against a device if you have it > behind a bridge (in addition to having a filtering iommu), which limits > the MMIO span (and thus letting the guest whack the BARs randomly will > only allow that guest to shoot itself in the foot). > > Some bridges also provide a way to block INTx below them which comes in > handy but it's bridge specific. Some devices can be coerced to send the > INTx "assert" message and never de-assert it (for example by doing a > soft-reset while it's asserted, which can be done with some devices with > an MMIO). > > Anything below a PCIe -> PCI/PCI-X needs to also be "grouped" due to > simple lack of proper filtering by the iommu (PCI-X in theory has RIDs > and fowards them up, but this isn't very reliable, for example it fails > over with split transactions). > > Fortunately in PCIe land, we most have bridges above everything. The > problem somewhat remains with functions of a device, how can you be sure > that there isn't a way via some MMIO to create side effects on the other > functions of the device ? (For example by checkstopping the whole > thing). You can't really :-) > > So it boils down of the "level" of safety/isolation you want to provide, > and I suppose to some extent it's a user decision but the user needs to > be informed to some extent. A hard problem :-) > > > There's a difference between removing the device from the host and > > exposing the device to the guest. If I have a NIC and HBA behind a > > bridge, it's perfectly reasonable that I might only assign the NIC to > > the guest, but as you describe, we then need to prevent the host, or any > > other guest from making use of the HBA. > > Yes. However the other device is in "limbo" and it may be not clear to > the user why it can't be used anymore :-) > > The question is more, the user needs to "know" (or libvirt does, or > somebody ... ) that in order to pass-through device A, it must also > "remove" device B from the host. How can you even provide a meaningful > error message to the user if all VFIO does is give you something like > -EBUSY ? > > So the information about the grouping constraint must trickle down > somewhat. > > Look at it from a GUI perspective for example. Imagine a front-end > showing you devices in your system and allowing you to "Drag & drop" > them to your guest. How do you represent that need for grouping ? First > how do you expose it from kernel/libvirt to the GUI tool and how do you > represent it to the user ? > > By grouping the devices in logical groups which end up being the > "objects" you can drag around, at least you provide some amount of > clarity. Now if you follow that path down to how the GUI app, libvirt > and possibly qemu need to know / resolve the dependency, being given the > "groups" as the primary information of what can be used for pass-through > makes everything a lot simpler. > > > > - The -minimum- granularity of pass-through
Re: [Qemu-devel] qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.
On 08/01/2011 11:44 AM, David Ahern wrote: > qemu-kvm.git as of: > > commit dacdc4b10bafbb21120e1c24a9665444768ef999 > Merge: 7b69d4f 0af4922 > Author: Avi Kivity > Date: Sun Jul 31 11:42:26 2011 +0300 > > Merge branch 'upstream-merge' into next > > is aborting with the error: > > qemu-kvm: qemu-kvm.git/hw/vhost.c:123: vhost_dev_unassign_memory: > Assertion `to >= 0' failed. > Aborted BTW, happens when I pass a kernel and initrd on the command line. qemu-kvm -kernel vmlinuz -initrd initrd.img ... David > > $ git bisect bad > 00cb2a99f5e7f73c4fff54ae16c7b6acf463ab5c is the first bad commit > commit 00cb2a99f5e7f73c4fff54ae16c7b6acf463ab5c > Author: Avi Kivity > Date: Tue Jul 26 14:26:17 2011 +0300 > > pc: convert pc_memory_init() to memory API > > Reviewed-by: Anthony Liguori > Signed-off-by: Avi Kivity > Signed-off-by: Anthony Liguori > > :04 04 3d709c2cab75b934030fb9fbdfa99024c855b2a6 > 720d362f9702f15d16519093555182169d0b09bd Mhw >
Re: [Qemu-devel] [PATCH 00/12][RFC] char: add flow control and fix guest_[open|close]
Hi, On 08/01/2011 04:22 PM, Anthony Liguori wrote: The char layer has been growing some nasty warts for some time now as we ask it to do things it was never intended on doing. It's been long over due for an overhaul and its become evident to me that we need to address this first before adding any more features to the char layer. This series is the start at sanitizing the char layer. It effectively turns the char layer into an internal pipe. It supports flow control using an intermediate ring queue for each direction. This series is an RFC because I don't think we should merge the series until we completely convert the old style flow control users to the new style. One particularly nasty area is the mux device. I'm not entirely sure yet how to preceed there. Anthony, thanks for looking into this / cooking up a patchset for this. Unfortunately I don't have the time to look into right now. Likely I won't have any time for this until after kvm forum. But if no-one has given this a serious look by then I'll try to convert the various spice related chardev frontends / backends to this and run some tests. Regards, Hans
[Qemu-devel] qemu-kvm aborts - vhost_dev_unassign_memory: Assertion `to >= 0' failed.
qemu-kvm.git as of: commit dacdc4b10bafbb21120e1c24a9665444768ef999 Merge: 7b69d4f 0af4922 Author: Avi Kivity Date: Sun Jul 31 11:42:26 2011 +0300 Merge branch 'upstream-merge' into next is aborting with the error: qemu-kvm: qemu-kvm.git/hw/vhost.c:123: vhost_dev_unassign_memory: Assertion `to >= 0' failed. Aborted $ git bisect bad 00cb2a99f5e7f73c4fff54ae16c7b6acf463ab5c is the first bad commit commit 00cb2a99f5e7f73c4fff54ae16c7b6acf463ab5c Author: Avi Kivity Date: Tue Jul 26 14:26:17 2011 +0300 pc: convert pc_memory_init() to memory API Reviewed-by: Anthony Liguori Signed-off-by: Avi Kivity Signed-off-by: Anthony Liguori :04 04 3d709c2cab75b934030fb9fbdfa99024c855b2a6 720d362f9702f15d16519093555182169d0b09bd M hw
Re: [Qemu-devel] [PATCH 00/39] Memory API, batch 2: PCI devices
On 07/31/2011 10:57 AM, Avi Kivity wrote: > This is a mostly mindless conversion of all QEMU PCI devices to the memory > API. > After this patchset is applied, it is no longer possible to create a PCI > device > using the old API. > > An immediate benefit is that PCI BARs that overlap each other are now handled > correctly: currently, the sequence > > map BAR 0 > map BAR 1 at an overlapping address > unmap either BAR 0 or BAR 1 > > will leave a hole where the overlap exists. With the patchset, the memory map > is restored correctly. > > Note that overlaps of PCI BARs with memory or non-PCI resources are still not > resolved correctly; this will be fixed later on. > > The vga patches have ugly intermediate states; however the result is fairly > clean. > > Also available from: > > git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory-region-b2 1-39 Reviewed-by: Richard Henderson Nice cleanups. r~
[Qemu-devel] [PATCH V3 2/2] [SLIRP] Delayed IP packets
In the current implementation, if Slirp tries to send an IP packet to a client
with an unknown hardware address, the packet is simply dropped and an ARP
request is sent (if_encap in slirp/slirp.c).
With this patch, Slirp will send the ARP request, re-queue the packet and try
to send it later. The packet is dropped after one second if the ARP reply is
not received.
Signed-off-by: Fabien Chouteau
---
slirp/if.c| 28 +++---
slirp/main.h |2 +-
slirp/mbuf.c |2 +
slirp/mbuf.h |2 +
slirp/slirp.c | 72 +++-
5 files changed, 69 insertions(+), 37 deletions(-)
diff --git a/slirp/if.c b/slirp/if.c
index 0f04e13..2d79e45 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -6,6 +6,7 @@
*/
#include
+#include "qemu-timer.h"
#define ifs_init(ifm) ((ifm)->ifs_next = (ifm)->ifs_prev = (ifm))
@@ -105,6 +106,9 @@ if_output(struct socket *so, struct mbuf *ifm)
ifs_init(ifm);
insque(ifm, ifq);
+/* Expiration date = Now + 1 second */
+ifm->expiration_date = qemu_get_clock_ns(rt_clock) + 10ULL;
+
diddit:
slirp->if_queued++;
@@ -153,6 +157,9 @@ diddit:
void
if_start(Slirp *slirp)
{
+int requeued = 0;
+uint64_t now;
+
struct mbuf *ifm, *ifqt;
DEBUG_CALL("if_start");
@@ -165,6 +172,8 @@ if_start(Slirp *slirp)
if (!slirp_can_output(slirp->opaque))
return;
+now = qemu_get_clock_ns(rt_clock);
+
/*
* See which queue to get next packet from
* If there's something in the fastq, select it immediately
@@ -199,11 +208,22 @@ if_start(Slirp *slirp)
ifm->ifq_so->so_nqueued = 0;
}
- /* Encapsulate the packet for sending */
-if_encap(slirp, (uint8_t *)ifm->m_data, ifm->m_len);
-
-m_free(ifm);
+if (ifm->expiration_date < now) {
+/* Expired */
+m_free(ifm);
+} else {
+/* Encapsulate the packet for sending */
+if (if_encap(slirp, ifm)) {
+m_free(ifm);
+} else {
+/* re-queue */
+insque(ifm, ifqt);
+requeued++;
+}
+}
if (slirp->if_queued)
goto again;
+
+slirp->if_queued = requeued;
}
diff --git a/slirp/main.h b/slirp/main.h
index 0dd8d81..028df4b 100644
--- a/slirp/main.h
+++ b/slirp/main.h
@@ -42,5 +42,5 @@ extern int tcp_keepintvl;
#define PROTO_PPP 0x2
#endif
-void if_encap(Slirp *slirp, const uint8_t *ip_data, int ip_data_len);
+int if_encap(Slirp *slirp, struct mbuf *ifm);
ssize_t slirp_send(struct socket *so, const void *buf, size_t len, int flags);
diff --git a/slirp/mbuf.c b/slirp/mbuf.c
index ce2eb84..c699c75 100644
--- a/slirp/mbuf.c
+++ b/slirp/mbuf.c
@@ -70,6 +70,8 @@ m_get(Slirp *slirp)
m->m_len = 0;
m->m_nextpkt = NULL;
m->m_prevpkt = NULL;
+m->arp_requested = false;
+m->expiration_date = (uint64_t)-1;
end_error:
DEBUG_ARG("m = %lx", (long )m);
return m;
diff --git a/slirp/mbuf.h b/slirp/mbuf.h
index b74544b..55170e5 100644
--- a/slirp/mbuf.h
+++ b/slirp/mbuf.h
@@ -86,6 +86,8 @@ struct mbuf {
charm_dat_[1]; /* ANSI don't like 0 sized arrays */
char*m_ext_;
} M_dat;
+bool arp_requested;
+uint64_t expiration_date;
};
#define m_next m_hdr.mh_next
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 4a9a4d5..a86cc6e 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -692,55 +692,63 @@ void slirp_input(Slirp *slirp, const uint8_t *pkt, int
pkt_len)
}
}
-/* output the IP packet to the ethernet device */
-void if_encap(Slirp *slirp, const uint8_t *ip_data, int ip_data_len)
+/* Output the IP packet to the ethernet device. Returns 0 if the packet must be
+ * re-queued.
+ */
+int if_encap(Slirp *slirp, struct mbuf *ifm)
{
uint8_t buf[1600];
struct ethhdr *eh = (struct ethhdr *)buf;
uint8_t ethaddr[ETH_ALEN];
-const struct ip *iph = (const struct ip *)ip_data;
+const struct ip *iph = (const struct ip *)ifm->m_data;
-if (ip_data_len + ETH_HLEN > sizeof(buf))
-return;
+if (ifm->m_len + ETH_HLEN > sizeof(buf)) {
+return 1;
+}
if (!arp_table_search(slirp, iph->ip_dst.s_addr, ethaddr)) {
uint8_t arp_req[ETH_HLEN + sizeof(struct arphdr)];
struct ethhdr *reh = (struct ethhdr *)arp_req;
struct arphdr *rah = (struct arphdr *)(arp_req + ETH_HLEN);
-/* If the client addr is not known, there is no point in
- sending the packet to it. Normally the sender should have
- done an ARP request to get its MAC address. Here we do it
- in place of sending the packet and we hope that the sender
- will retry sending its packet. */
-memset(reh->h_dest, 0xff, ETH_ALEN);
-memcpy(reh->h_source, special_ethaddr, ETH
[Qemu-devel] [PATCH V3 1/2] [SLIRP] Simple ARP table
This patch adds a simple ARP table in Slirp and also adds handling of
gratuitous ARP requests.
Signed-off-by: Fabien Chouteau
---
Makefile.objs |2 +-
slirp/arp_table.c | 72 +
slirp/bootp.c | 21 +--
slirp/slirp.c | 63 +++--
slirp/slirp.h | 47 --
5 files changed, 146 insertions(+), 59 deletions(-)
create mode 100644 slirp/arp_table.c
diff --git a/Makefile.objs b/Makefile.objs
index 6991a9f..0c10557 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -151,7 +151,7 @@ common-obj-y += qemu-timer.o qemu-timer-common.o
slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o
slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o tcp_output.o
-slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o
+slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o
common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
# xen backend driver support
diff --git a/slirp/arp_table.c b/slirp/arp_table.c
new file mode 100644
index 000..c7034ee
--- /dev/null
+++ b/slirp/arp_table.c
@@ -0,0 +1,72 @@
+#include "slirp.h"
+
+void arp_table_add(Slirp *slirp, int ip_addr, uint8_t ethaddr[ETH_ALEN])
+{
+const in_addr_t broadcast_addr =
+~slirp->vnetwork_mask.s_addr | slirp->vnetwork_addr.s_addr;
+ArpTable *arptbl = &slirp->arp_table;
+int i;
+
+DEBUG_CALL("arp_table_add");
+DEBUG_ARG("ip = 0x%x", ip_addr);
+DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
+ethaddr[0], ethaddr[1], ethaddr[2],
+ethaddr[3], ethaddr[4], ethaddr[5]));
+
+if ((ip_addr & ~(0xf << 28)) == 0 ||
+ip_addr == broadcast_addr) {
+/* Do not register 0.0.0.0/8 or broadcast addresses */
+return;
+}
+
+/* Search for an entry */
+for (i = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0; i++) {
+if (arptbl->table[i].ar_sip == ip_addr) {
+/* Update the entry */
+memcpy(arptbl->table[i].ar_sha, ethaddr, ETH_ALEN);
+return;
+}
+}
+
+/* No entry found, create a new one */
+arptbl->table[arptbl->next_victim].ar_sip = ip_addr;
+memcpy(arptbl->table[arptbl->next_victim].ar_sha, ethaddr, ETH_ALEN);
+arptbl->next_victim = (arptbl->next_victim + 1) % ARP_TABLE_SIZE;
+}
+
+bool arp_table_search(Slirp *slirp, int in_ip_addr,
+ uint8_t out_ethaddr[ETH_ALEN])
+{
+const in_addr_t broadcast_addr =
+~slirp->vnetwork_mask.s_addr | slirp->vnetwork_addr.s_addr;
+ArpTable *arptbl = &slirp->arp_table;
+int i;
+
+DEBUG_CALL("arp_table_search");
+DEBUG_ARG("ip = 0x%x", in_ip_addr);
+
+/* If address in 0.0.0.0/8 */
+if ((in_ip_addr & ~(0xf << 28)) == 0) {
+/* Invalid address */
+return 0;
+}
+
+/* If broadcast address */
+if (in_ip_addr == broadcast_addr) {
+/* return Ethernet broadcast address */
+memset(out_ethaddr, 0xff, ETH_ALEN);
+return 1;
+}
+
+for (i = 0; i < ARP_TABLE_SIZE; i++) {
+if (arptbl->table[i].ar_sip == in_ip_addr) {
+memcpy(out_ethaddr, arptbl->table[i].ar_sha, ETH_ALEN);
+DEBUG_ARGS((dfd, " found hw addr =
%02x:%02x:%02x:%02x:%02x:%02x\n",
+out_ethaddr[0], out_ethaddr[1], out_ethaddr[2],
+out_ethaddr[3], out_ethaddr[4], out_ethaddr[5]));
+return 1;
+}
+}
+
+return 0;
+}
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 1eb2ed1..efd1fe7 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -149,6 +149,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t
*bp)
struct in_addr preq_addr;
int dhcp_msg_type, val;
uint8_t *q;
+uint8_t client_ethaddr[ETH_ALEN];
/* extract exact DHCP msg type */
dhcp_decode(bp, &dhcp_msg_type, &preq_addr);
@@ -164,8 +165,9 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t
*bp)
if (dhcp_msg_type != DHCPDISCOVER &&
dhcp_msg_type != DHCPREQUEST)
return;
-/* XXX: this is a hack to get the client mac address */
-memcpy(slirp->client_ethaddr, bp->bp_hwaddr, 6);
+
+/* Get client's hardware address from bootp request */
+memcpy(client_ethaddr, bp->bp_hwaddr, ETH_ALEN);
m = m_get(slirp);
if (!m) {
@@ -178,25 +180,25 @@ static void bootp_reply(Slirp *slirp, const struct
bootp_t *bp)
if (dhcp_msg_type == DHCPDISCOVER) {
if (preq_addr.s_addr != htonl(0L)) {
-bc = request_addr(slirp, &preq_addr, slirp->client_ethaddr);
+bc = request_addr(slirp, &preq_addr, client_ethaddr);
if (bc) {
daddr.sin_addr = preq_addr;
}
}
if (!bc) {
new_addr:
-bc = get_new_addr(slirp, &daddr.sin_addr, slirp->client_ethaddr);
+
Re: [Qemu-devel] [PATCH v2] Fix unassigned memory access handling
Blue Swirl wrote:
> cea5f9a28faa528b6b1b117c9ab2d8828f473fef exposed bugs in unassigned memory
> access handling. Fix them by always passing CPUState to the handlers.
>
> Reported-by: Hervé Poussineau
> Signed-off-by: Blue Swirl
> ---
> v2: don't try to restore env since all targets eventually always call
> cpu_loop_exit() which will not return.
>
> exec-all.h|2 +-
> exec.c| 12 ++--
> target-alpha/cpu.h|5 +++--
> target-alpha/op_helper.c |6 --
> target-microblaze/cpu.h |4 ++--
> target-microblaze/op_helper.c | 14 --
> target-mips/cpu.h |4 ++--
> target-mips/op_helper.c |6 --
> target-sparc/cpu.h|4 ++--
> target-sparc/op_helper.c | 26 --
> 10 files changed, 48 insertions(+), 35 deletions(-)
>
> diff --git a/exec-all.h b/exec-all.h
> index 69acf3b..9b8d62c 100644
> --- a/exec-all.h
> +++ b/exec-all.h
> @@ -323,7 +323,7 @@ static inline tb_page_addr_t
> get_page_addr_code(CPUState *env1, target_ulong add
> pd = env1->tlb_table[mmu_idx][page_index].addr_code & ~TARGET_PAGE_MASK;
> if (pd > IO_MEM_ROM && !(pd & IO_MEM_ROMD)) {
> #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SPARC)
> -do_unassigned_access(addr, 0, 1, 0, 4);
> +cpu_unassigned_access(env1, addr, 0, 1, 0, 4);
> #else
> cpu_abort(env1, "Trying to execute code outside RAM or ROM at
> 0x" TARGET_FMT_lx "\n", addr);
> #endif
> diff --git a/exec.c b/exec.c
> index f1777e6..16e16f3 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3236,7 +3236,7 @@ static uint32_t unassigned_mem_readb(void
> *opaque, target_phys_addr_t addr)
> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
> #endif
> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
> defined(TARGET_MICROBLAZE)
> -do_unassigned_access(addr, 0, 0, 0, 1);
> +cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 1);
> #endif
> return 0;
> }
> @@ -3247,7 +3247,7 @@ static uint32_t unassigned_mem_readw(void
> *opaque, target_phys_addr_t addr)
> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
> #endif
> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
> defined(TARGET_MICROBLAZE)
> -do_unassigned_access(addr, 0, 0, 0, 2);
> +cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 2);
> #endif
> return 0;
> }
> @@ -3258,7 +3258,7 @@ static uint32_t unassigned_mem_readl(void
> *opaque, target_phys_addr_t addr)
> printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
> #endif
> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
> defined(TARGET_MICROBLAZE)
> -do_unassigned_access(addr, 0, 0, 0, 4);
> +cpu_unassigned_access(cpu_single_env, addr, 0, 0, 0, 4);
> #endif
> return 0;
> }
> @@ -3269,7 +3269,7 @@ static void unassigned_mem_writeb(void *opaque,
> target_phys_addr_t addr, uint32_
> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
> #endif
> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
> defined(TARGET_MICROBLAZE)
> -do_unassigned_access(addr, 1, 0, 0, 1);
> +cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 1);
> #endif
> }
>
> @@ -3279,7 +3279,7 @@ static void unassigned_mem_writew(void *opaque,
> target_phys_addr_t addr, uint32_
> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
> #endif
> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
> defined(TARGET_MICROBLAZE)
> -do_unassigned_access(addr, 1, 0, 0, 2);
> +cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 2);
> #endif
> }
>
> @@ -3289,7 +3289,7 @@ static void unassigned_mem_writel(void *opaque,
> target_phys_addr_t addr, uint32_
> printf("Unassigned mem write " TARGET_FMT_plx " = 0x%x\n", addr, val);
> #endif
> #if defined(TARGET_ALPHA) || defined(TARGET_SPARC) ||
> defined(TARGET_MICROBLAZE)
> -do_unassigned_access(addr, 1, 0, 0, 4);
> +cpu_unassigned_access(cpu_single_env, addr, 1, 0, 0, 4);
> #endif
> }
>
[..snip..]
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 4edae78..f6cb300 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -510,8 +510,8 @@ static inline int tlb_compare_context(const
> SparcTLBEntry *tlb,
>
> /* cpu-exec.c */
> #if !defined(CONFIG_USER_ONLY)
> -void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
> - int is_asi, int size);
> +void cpu_unassigned_access(CPUState *env1, target_phys_addr_t addr,
> + int is_write, int is_exec, int is_asi, int size);
> target_phys_addr_t cpu_get_phys_page_nofault(CPUState *env, target_ulong
> addr,
> int mmu_idx);
>
> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
> index fd0cfbd..101edfb 100644
> --- a/target-sparc/op_helper.c
> +++ b/target-sparc/op_helper.
Re: [Qemu-devel] [PATCH 00/12][RFC] char: add flow control and fix guest_[open|close]
On 08/01/2011 11:04 AM, Alon Levy wrote: On Mon, Aug 01, 2011 at 09:22:58AM -0500, Anthony Liguori wrote: The char layer has been growing some nasty warts for some time now as we ask it to do things it was never intended on doing. It's been long over due for an overhaul and its become evident to me that we need to address this first before adding any more features to the char layer. This series is the start at sanitizing the char layer. It effectively turns the char layer into an internal pipe. It supports flow control using an intermediate ring queue for each direction. This series is an RFC because I don't think we should merge the series until we completely convert the old style flow control users to the new style. One particularly nasty area is the mux device. I'm not entirely sure yet how to preceed there. So, adding a copy - is that really a good idea? Yes, otherwise I wouldn't have proposed it ;-) I don't have any alternative code, so I'm already starting bad, I know, and I understand the want to have a "middle ground" to ease the logic. You need an intermediate buffer if you want to preserve a simple read/write interface. Maybe keeping an iovec? add a function on each side for freeing, i.e. release_be_buffer, release_fe_buffer. At least it could make this as fast as the current code. I'm thinking of copy/paste for vdagent, usbredir, guest agent doing dmesg or anything larger. I think you're prematurely optimizing. I think you'll find that there are many other bottlenecks well before this copy becomes an issue. Regardless, correctness needs to trump performance here. We ought to focus on making the layer lossless and sane first, and then we can work on improving performance. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 00/12][RFC] char: add flow control and fix guest_[open|close]
On Mon, Aug 01, 2011 at 09:22:58AM -0500, Anthony Liguori wrote: > The char layer has been growing some nasty warts for some time now as we ask > it > to do things it was never intended on doing. It's been long over due for an > overhaul and its become evident to me that we need to address this first > before > adding any more features to the char layer. > > This series is the start at sanitizing the char layer. It effectively turns > the char layer into an internal pipe. It supports flow control using an > intermediate ring queue for each direction. > > This series is an RFC because I don't think we should merge the series until > we > completely convert the old style flow control users to the new style. > > One particularly nasty area is the mux device. I'm not entirely sure yet how > to preceed there. > > So, adding a copy - is that really a good idea? I don't have any alternative code, so I'm already starting bad, I know, and I understand the want to have a "middle ground" to ease the logic. Maybe keeping an iovec? add a function on each side for freeing, i.e. release_be_buffer, release_fe_buffer. At least it could make this as fast as the current code. I'm thinking of copy/paste for vdagent, usbredir, guest agent doing dmesg or anything larger.
Re: [Qemu-devel] [PATCH V2 1/2] [SLIRP] Simple ARP table
On 01/08/2011 17:11, Jan Kiszka wrote:
> On 2011-08-01 17:03, Fabien Chouteau wrote:
>> On 30/07/2011 11:19, Jan Kiszka wrote:
>>> On 2011-07-30 11:09, Jan Kiszka wrote:
On 2011-07-29 19:34, Jan Kiszka wrote:
> On 2011-07-29 18:25, Fabien Chouteau wrote:
>> This patch adds a simple ARP table in Slirp and also adds handling of
>> gratuitous ARP requests.
>>
>> Signed-off-by: Fabien Chouteau
>> ---
>> Makefile.objs |2 +-
>> slirp/arp_table.c | 50 ++
>> slirp/bootp.c | 23 --
>> slirp/slirp.c | 63
>> +---
>> slirp/slirp.h | 50 +++--
>> 5 files changed, 129 insertions(+), 59 deletions(-)
>> create mode 100644 slirp/arp_table.c
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 6991a9f..0c10557 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -151,7 +151,7 @@ common-obj-y += qemu-timer.o qemu-timer-common.o
>>
>> slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o
>> slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o
>> tcp_output.o
>> -slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o
>> +slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o
>> common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
>>
>> # xen backend driver support
>> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
>> new file mode 100644
>> index 000..5d7404b
>> --- /dev/null
>> +++ b/slirp/arp_table.c
>> @@ -0,0 +1,50 @@
>> +#include "slirp.h"
>> +
>> +void arp_table_add(ArpTable *arptbl,
>> + int ip_addr,
>> + uint8_t ethaddr[ETH_ALEN])
>
> I still prefer the condensed formatting, but that's a minor nit.
>>
>> OK I'll change it to keep consistent style.
>>
>> Unfortunately, there's nothing on this subject in the CODING_STYLE...
>
> We should add a section on consistency - but I guess that will always
> remain a subjective matter. :)
Indeed, I bet we can find in Qemu an example of every C coding style...
>
>>
>
>> +{
>> +int i;
>> +
>> +DEBUG_CALL("arp_table_add");
>> +DEBUG_ARG("ip = 0x%x", ip_addr);
>> +DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
>> +ethaddr[0], ethaddr[1], ethaddr[2],
>> +ethaddr[3], ethaddr[4], ethaddr[5]));
>> +
>> +/* Search for an entry */
>> +for (i = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0;
>> i++) {
>
> Missed that on round #1: Why treating 0.0.0.0 special? If ip_addr can be
> zero, the current logic will append every "update" of that address as a
> new entry.
>>
>> Isn't 0.0.0.0 a reserved address? I think it's safe to use it here.
>
> Actually, the whole 0.0.0.0/8 is source-only, ie. it should never show
> up in the ARP table.
>
Great, so I will just prevent insertion and search of 0.0.0.0/8 addresses.
Regards,
--
Fabien Chouteau
Re: [Qemu-devel] 0.15 testing, spice, wxp
On Monday 01 August 2011 09:34:32 Gerd Hoffmann wrote: > On 07/29/11 04:48, Rick Vernam wrote: > > I'd like to do some very high-level testing of qemu-stable-0.15 > > (qemu-system- x86_64, specifically) running Linux host, Windows XP SP3 > > guest. > > I'm afraid that I don't have a ready environment for building the windows > > guest bits, and the added time requirement simply puts it over the top > > for me. Are there updated spice windows guest binaries available > > anywhere? I suppose this would just be the QXL video driver and the > > guest agent? From looking at spice-space.org/download.html I concluded > > that I'm looking for verion 0.8.x, but my searches have left me empty > > handed. > > Just use the most recent versions from spice-space.org/download.html, > they should work just fine with 0.15 > > HTH, >Gerd Most recent windows binaries are 0.6.x. They've been working well for the past few months, thank you. I take it, then, that there are no 0.8.x compiled binaries for 0.8.x? Thanks, -Rick
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
On 08/01/2011 10:44 AM, Stefan Hajnoczi wrote: On Mon, Aug 1, 2011 at 4:44 PM, Kevin Wolf wrote: Am 01.08.2011 17:28, schrieb Anthony Liguori: On 08/01/2011 10:22 AM, Stefan Hajnoczi wrote: I like the block_set approach. Anthony, Kevin, Supriya: Any thoughts? For the sake of overall QMP sanity, I think block_set_hostcache is really our only option. Ideally we should have blockdev_add, and blockdev_set would just take the same arguments and update the given driver. But we don't have blockdev_add today, so whatever works for your as a temporary solution... Anthony's point is that blockdev_set does not fit with QMP command discoverability. It doesn't, but if we had a 'plug_set' that worked for devices and any type of backends, we could have a single introspection mechanism. But we should really try to avoid having every backend implement their own ways of doing things. Regards, Anthony Liguori Stefan
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
On Mon, Aug 1, 2011 at 4:44 PM, Kevin Wolf wrote:
> Am 01.08.2011 17:28, schrieb Anthony Liguori:
>> On 08/01/2011 10:22 AM, Stefan Hajnoczi wrote:
>>> On Thu, Jul 28, 2011 at 10:29 AM, Stefan Hajnoczi
>>> wrote:
On Wed, Jul 27, 2011 at 5:02 PM, Anthony Liguori
wrote:
> On 07/27/2011 09:31 AM, Stefan Hajnoczi wrote:
>>
>> On Wed, Jul 27, 2011 at 1:58 PM, Anthony Liguori
>> wrote:
Index: qemu/hmp-commands.hx
===
--- qemu.orig/hmp-commands.hx
+++ qemu/hmp-commands.hx
@@ -70,6 +70,20 @@ but should be used with extreme caution.
resizes image files, it can not resize block devices like LVM
volumes.
ETEXI
+ {
+ .name = "block_set",
+ .args_type = "device:B,device:O",
+ .params = "device [prop=value][,...]",
+ .help = "Change block device parameters
[hostcache=on/off]",
+ .user_print = monitor_user_noop,
+ .mhandler.cmd_new = do_block_set,
+ },
+STEXI
+@item block_set @var{config}
+@findex block_set
+Change block device parameters (eg: hostcache=on/off) while guest is
running.
+ETEXI
+
>>>
>>> block_set_hostcache() please.
>>>
>>> Multiplexing commands is generally a bad idea. It weakens typing. In
>>> the
>>> absence of a generic way to set block device properties, implementing
>>> properties as generic in the QMP layer seems like a bad idea to me.
>>
>> The idea behind block_set was to have a unified interface for changing
>> block device parameters at runtime. This prevents us from reinventing
>> new commands from scratch. For example, block I/O throttling is
>> already queued up to add run-time parameters.
>>
>> Without a unified command we have a bulkier QMP/HMP interface,
>> duplicated code, and possibly inconsistencies in syntax between the
>> commands. Isn't the best way to avoid these problems a unified
>> interface?
>>
>> I understand the lack of type safety concern but in this case we
>> already have to manually pull parsed arguments (i.e. cast to specific
>> types and deal with invalid input). To me this is a reason *for*
>> using a unified interface like block_set.
>
> Think about it from a client perspective. How do I determine which
> properties are supported by this version of QEMU? I have no way to
> identify
> programmatically what arguments are valid for block_set.
>
> OTOH, if you have strong types like block_set_hostcache, query-commands
> tells me exactly what's supported.
Use query-block and see if 'hostcache' is there. If yes, then the
hostcache parameter is available. If we allow BlockDrivers to have
their own runtime parameters then query-commands does not tell you
anything because the specific BlockDriver may or may not support that
runtime parameter - you need to use query-block.
>>>
>>> Let's reach agreement here. The choices are:
>>>
>>> 1. Top-level block_set command. Supported parameters are discovered
>>> by looking query-block output.
>>
>> I'm strongly opposed to this. There needs to be a single consistent way
>> to determine supported operations with QMP.
>>
>> And that single mechanism already exists--query_commands.
>>
>>> 2. Top-level command for each parameter (e.g. block_set_hostcache).
>>> Supported parameters are easily discoverable via query-commands. If
>>> individual block devices support different sets of parameters then
>>> they may have to return -ENOTSUPP.
>>>
>>> I like the block_set approach.
>>>
>>> Anthony, Kevin, Supriya: Any thoughts?
>>
>> For the sake of overall QMP sanity, I think block_set_hostcache is
>> really our only option.
>
> Ideally we should have blockdev_add, and blockdev_set would just take
> the same arguments and update the given driver.
>
> But we don't have blockdev_add today, so whatever works for your as a
> temporary solution...
Anthony's point is that blockdev_set does not fit with QMP command
discoverability.
Stefan
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
On 08/01/2011 10:44 AM, Kevin Wolf wrote: Am 01.08.2011 17:28, schrieb Anthony Liguori: 2. Top-level command for each parameter (e.g. block_set_hostcache). Supported parameters are easily discoverable via query-commands. If individual block devices support different sets of parameters then they may have to return -ENOTSUPP. I like the block_set approach. Anthony, Kevin, Supriya: Any thoughts? For the sake of overall QMP sanity, I think block_set_hostcache is really our only option. Ideally we should have blockdev_add, and blockdev_set would just take the same arguments and update the given driver. Ideally we'd have a backend_add, backend_set, etc. But in the absence of that, we should provide the best interface we can with the current tools we have. For now, using high level commands is the best we can do. Regards, Anthony Liguori But we don't have blockdev_add today, so whatever works for your as a temporary solution... Kevin
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
Am 01.08.2011 17:28, schrieb Anthony Liguori:
> On 08/01/2011 10:22 AM, Stefan Hajnoczi wrote:
>> On Thu, Jul 28, 2011 at 10:29 AM, Stefan Hajnoczi wrote:
>>> On Wed, Jul 27, 2011 at 5:02 PM, Anthony Liguori
>>> wrote:
On 07/27/2011 09:31 AM, Stefan Hajnoczi wrote:
>
> On Wed, Jul 27, 2011 at 1:58 PM, Anthony Liguori
> wrote:
>>>
>>> Index: qemu/hmp-commands.hx
>>> ===
>>> --- qemu.orig/hmp-commands.hx
>>> +++ qemu/hmp-commands.hx
>>> @@ -70,6 +70,20 @@ but should be used with extreme caution.
>>> resizes image files, it can not resize block devices like LVM volumes.
>>> ETEXI
>>>
>>> +{
>>> +.name = "block_set",
>>> +.args_type = "device:B,device:O",
>>> +.params = "device [prop=value][,...]",
>>> +.help = "Change block device parameters
>>> [hostcache=on/off]",
>>> +.user_print = monitor_user_noop,
>>> +.mhandler.cmd_new = do_block_set,
>>> +},
>>> +STEXI
>>> +@item block_set @var{config}
>>> +@findex block_set
>>> +Change block device parameters (eg: hostcache=on/off) while guest is
>>> running.
>>> +ETEXI
>>> +
>>
>> block_set_hostcache() please.
>>
>> Multiplexing commands is generally a bad idea. It weakens typing. In
>> the
>> absence of a generic way to set block device properties, implementing
>> properties as generic in the QMP layer seems like a bad idea to me.
>
> The idea behind block_set was to have a unified interface for changing
> block device parameters at runtime. This prevents us from reinventing
> new commands from scratch. For example, block I/O throttling is
> already queued up to add run-time parameters.
>
> Without a unified command we have a bulkier QMP/HMP interface,
> duplicated code, and possibly inconsistencies in syntax between the
> commands. Isn't the best way to avoid these problems a unified
> interface?
>
> I understand the lack of type safety concern but in this case we
> already have to manually pull parsed arguments (i.e. cast to specific
> types and deal with invalid input). To me this is a reason *for*
> using a unified interface like block_set.
Think about it from a client perspective. How do I determine which
properties are supported by this version of QEMU? I have no way to
identify
programmatically what arguments are valid for block_set.
OTOH, if you have strong types like block_set_hostcache, query-commands
tells me exactly what's supported.
>>>
>>> Use query-block and see if 'hostcache' is there. If yes, then the
>>> hostcache parameter is available. If we allow BlockDrivers to have
>>> their own runtime parameters then query-commands does not tell you
>>> anything because the specific BlockDriver may or may not support that
>>> runtime parameter - you need to use query-block.
>>
>> Let's reach agreement here. The choices are:
>>
>> 1. Top-level block_set command. Supported parameters are discovered
>> by looking query-block output.
>
> I'm strongly opposed to this. There needs to be a single consistent way
> to determine supported operations with QMP.
>
> And that single mechanism already exists--query_commands.
>
>> 2. Top-level command for each parameter (e.g. block_set_hostcache).
>> Supported parameters are easily discoverable via query-commands. If
>> individual block devices support different sets of parameters then
>> they may have to return -ENOTSUPP.
>>
>> I like the block_set approach.
>>
>> Anthony, Kevin, Supriya: Any thoughts?
>
> For the sake of overall QMP sanity, I think block_set_hostcache is
> really our only option.
Ideally we should have blockdev_add, and blockdev_set would just take
the same arguments and update the given driver.
But we don't have blockdev_add today, so whatever works for your as a
temporary solution...
Kevin
Re: [Qemu-devel] [PATCH 06/12] char: add an edge event API for the front ends
On 08/01/2011 10:39 AM, Stefan Hajnoczi wrote:
On Mon, Aug 1, 2011 at 3:23 PM, Anthony Liguori wrote:
Signed-off-by: Anthony Liguori
---
qemu-char.c | 91 +--
qemu-char.h |9 ++
2 files changed, 85 insertions(+), 15 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 0e4a30c..9e40e04 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -139,13 +139,23 @@ void qemu_chr_generic_open(CharDriverState *s)
}
}
+static uint32_t char_queue_get_avail(CharQueue *q)
+{
+return sizeof(q->ring) - (q->prod - q->cons);
+}
+
+static bool char_queue_get_empty(CharQueue *q)
+{
+return (q->cons == q->prod);
+}
bool function naming nitpick: char_queue_is_empty() is clearer than
char_queue_get_empty(). "is" and "has" are always bool, "get" could
return anything.
Ack.
I thought the same thing to myself, meant to rename it, and then
promptly forgot to :-)
Regards,
Anthony Liguori
Stefan
Re: [Qemu-devel] [PATCH 12/12] char: enforce the use of qemu_chr_guest_open()
On 08/01/2011 10:38 AM, Alon Levy wrote:
On Mon, Aug 01, 2011 at 09:23:10AM -0500, Anthony Liguori wrote:
Signed-off-by: Anthony Liguori
Title should be "fe_open", not guest_open.
Indeed, thanks.
Regards,
Anthony Liguori
---
qemu-char.c | 10 ++
qemu-char.h |2 ++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index fb7c937..5e62795 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -206,6 +206,8 @@ int qemu_chr_fe_write(CharDriverState *s, const uint8_t
*buf, int len)
int ret;
bool is_empty;
+assert(s->fe_opened> 0);
+
is_empty = char_queue_get_empty(&s->fe_tx);
ret = char_queue_write(&s->fe_tx, buf, len);
@@ -228,6 +230,8 @@ int qemu_chr_fe_read(CharDriverState *s, uint8_t *buf, int
len)
bool is_full;
int ret;
+assert(s->fe_opened> 0);
+
is_full = (char_queue_get_avail(&s->be_tx) == 0);
ret = char_queue_read(&s->be_tx, buf, len);
@@ -249,6 +253,8 @@ void qemu_chr_fe_set_handlers(CharDriverState *s,
IOEventHandler *chr_event,
void *opaque)
{
+assert(s->fe_opened> 0);
+
if (!opaque&& !chr_read&& !chr_write&& !chr_event) {
/* chr driver being released. */
++s->avail_connections;
@@ -2822,6 +2828,8 @@ void qemu_chr_set_echo(struct CharDriverState *chr, bool
echo)
void qemu_chr_fe_open(struct CharDriverState *chr)
{
+chr->fe_opened++;
+
if (chr->chr_guest_open) {
chr->chr_guest_open(chr);
}
@@ -2832,6 +2840,8 @@ void qemu_chr_fe_close(struct CharDriverState *chr)
if (chr->chr_guest_close) {
chr->chr_guest_close(chr);
}
+
+chr->fe_opened--;
}
void qemu_chr_close(CharDriverState *chr)
diff --git a/qemu-char.h b/qemu-char.h
index 8b37fcf..b910c5e 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -88,6 +88,8 @@ struct CharDriverState {
int opened;
int avail_connections;
+int fe_opened;
+
CharQueue fe_tx;
CharQueue be_tx;
--
1.7.4.1
Re: [Qemu-devel] [PATCH 06/12] char: add an edge event API for the front ends
On Mon, Aug 1, 2011 at 3:23 PM, Anthony Liguori wrote:
> Signed-off-by: Anthony Liguori
> ---
> qemu-char.c | 91
> +--
> qemu-char.h | 9 ++
> 2 files changed, 85 insertions(+), 15 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 0e4a30c..9e40e04 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -139,13 +139,23 @@ void qemu_chr_generic_open(CharDriverState *s)
> }
> }
>
> +static uint32_t char_queue_get_avail(CharQueue *q)
> +{
> + return sizeof(q->ring) - (q->prod - q->cons);
> +}
> +
> +static bool char_queue_get_empty(CharQueue *q)
> +{
> + return (q->cons == q->prod);
> +}
bool function naming nitpick: char_queue_is_empty() is clearer than
char_queue_get_empty(). "is" and "has" are always bool, "get" could
return anything.
Stefan
Re: [Qemu-devel] [PATCH 12/12] char: enforce the use of qemu_chr_guest_open()
On Mon, Aug 01, 2011 at 09:23:10AM -0500, Anthony Liguori wrote:
> Signed-off-by: Anthony Liguori
Title should be "fe_open", not guest_open.
> ---
> qemu-char.c | 10 ++
> qemu-char.h |2 ++
> 2 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index fb7c937..5e62795 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -206,6 +206,8 @@ int qemu_chr_fe_write(CharDriverState *s, const uint8_t
> *buf, int len)
> int ret;
> bool is_empty;
>
> +assert(s->fe_opened > 0);
> +
> is_empty = char_queue_get_empty(&s->fe_tx);
>
> ret = char_queue_write(&s->fe_tx, buf, len);
> @@ -228,6 +230,8 @@ int qemu_chr_fe_read(CharDriverState *s, uint8_t *buf,
> int len)
> bool is_full;
> int ret;
>
> +assert(s->fe_opened > 0);
> +
> is_full = (char_queue_get_avail(&s->be_tx) == 0);
>
> ret = char_queue_read(&s->be_tx, buf, len);
> @@ -249,6 +253,8 @@ void qemu_chr_fe_set_handlers(CharDriverState *s,
>IOEventHandler *chr_event,
>void *opaque)
> {
> +assert(s->fe_opened > 0);
> +
> if (!opaque && !chr_read && !chr_write && !chr_event) {
> /* chr driver being released. */
> ++s->avail_connections;
> @@ -2822,6 +2828,8 @@ void qemu_chr_set_echo(struct CharDriverState *chr,
> bool echo)
>
> void qemu_chr_fe_open(struct CharDriverState *chr)
> {
> +chr->fe_opened++;
> +
> if (chr->chr_guest_open) {
> chr->chr_guest_open(chr);
> }
> @@ -2832,6 +2840,8 @@ void qemu_chr_fe_close(struct CharDriverState *chr)
> if (chr->chr_guest_close) {
> chr->chr_guest_close(chr);
> }
> +
> +chr->fe_opened--;
> }
>
> void qemu_chr_close(CharDriverState *chr)
> diff --git a/qemu-char.h b/qemu-char.h
> index 8b37fcf..b910c5e 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -88,6 +88,8 @@ struct CharDriverState {
> int opened;
> int avail_connections;
>
> +int fe_opened;
> +
> CharQueue fe_tx;
> CharQueue be_tx;
>
> --
> 1.7.4.1
>
>
Re: [Qemu-devel] [PATCH 04/12] char: introduce backend tx queue
On 08/01/2011 10:33 AM, Stefan Hajnoczi wrote:
On Mon, Aug 1, 2011 at 3:23 PM, Anthony Liguori wrote:
While the front tx queue has no flow control, the backend tx queue uses a
polling function to determine when the front end can receive data.
To convert this to the new queue model, we simply try to flush the backend tx
queue whenever we poll. We then return the remaining space in the queue as
the value of the polling function.
Signed-off-by: Anthony Liguori
---
qemu-char.c | 49 -
qemu-char.h |3 ++-
2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 3f9b32c..2746652 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -173,6 +173,11 @@ static size_t char_queue_read(CharQueue *q, void *data,
size_t size)
return i;
}
+static uint32_t char_queue_get_avail(CharQueue *q)
+{
+return sizeof(q->ring) - (q->prod - q->cons);
+}
"avail" confuses me. How many bytes are available for the consumer?
How many bytes are available for the producer?
How about char_queue_get_space() or char_queue_get_nfree()?
Sure.
Regards,
Anthony Liguori
Stefan
Re: [Qemu-devel] [RFC 22/24] qemu-io: make map command use new block mapping function
Am 29.07.2011 06:49, schrieb Devin Nakamura: > bdrv_get_mapping will be used when it is defined, > otherwise default to old behaviour. > > Signed-off-by: Devin Nakamura Hm, I think I would use a new command for this, like 'get_mapping'. The old 'map' command can still be useful even for formats supporting bdrv_get_mapping. You would use it whenever you are only interested which offsets are allocated, but don't care about the offsets in the image file to which they are mapped. This makes the output much shorter. Kevin
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
On Mon, Aug 1, 2011 at 4:28 PM, Anthony Liguori wrote:
> On 08/01/2011 10:22 AM, Stefan Hajnoczi wrote:
>>
>> On Thu, Jul 28, 2011 at 10:29 AM, Stefan Hajnoczi
>> wrote:
>>>
>>> On Wed, Jul 27, 2011 at 5:02 PM, Anthony Liguori
>>> wrote:
On 07/27/2011 09:31 AM, Stefan Hajnoczi wrote:
>
> On Wed, Jul 27, 2011 at 1:58 PM, Anthony Liguori
> wrote:
>>>
>>> Index: qemu/hmp-commands.hx
>>> ===
>>> --- qemu.orig/hmp-commands.hx
>>> +++ qemu/hmp-commands.hx
>>> @@ -70,6 +70,20 @@ but should be used with extreme caution.
>>> resizes image files, it can not resize block devices like LVM
>>> volumes.
>>> ETEXI
>>>
>>> + {
>>> + .name = "block_set",
>>> + .args_type = "device:B,device:O",
>>> + .params = "device [prop=value][,...]",
>>> + .help = "Change block device parameters
>>> [hostcache=on/off]",
>>> + .user_print = monitor_user_noop,
>>> + .mhandler.cmd_new = do_block_set,
>>> + },
>>> +STEXI
>>> +@item block_set @var{config}
>>> +@findex block_set
>>> +Change block device parameters (eg: hostcache=on/off) while guest is
>>> running.
>>> +ETEXI
>>> +
>>
>> block_set_hostcache() please.
>>
>> Multiplexing commands is generally a bad idea. It weakens typing. In
>> the
>> absence of a generic way to set block device properties, implementing
>> properties as generic in the QMP layer seems like a bad idea to me.
>
> The idea behind block_set was to have a unified interface for changing
> block device parameters at runtime. This prevents us from reinventing
> new commands from scratch. For example, block I/O throttling is
> already queued up to add run-time parameters.
>
> Without a unified command we have a bulkier QMP/HMP interface,
> duplicated code, and possibly inconsistencies in syntax between the
> commands. Isn't the best way to avoid these problems a unified
> interface?
>
> I understand the lack of type safety concern but in this case we
> already have to manually pull parsed arguments (i.e. cast to specific
> types and deal with invalid input). To me this is a reason *for*
> using a unified interface like block_set.
Think about it from a client perspective. How do I determine which
properties are supported by this version of QEMU? I have no way to
identify
programmatically what arguments are valid for block_set.
OTOH, if you have strong types like block_set_hostcache, query-commands
tells me exactly what's supported.
>>>
>>> Use query-block and see if 'hostcache' is there. If yes, then the
>>> hostcache parameter is available. If we allow BlockDrivers to have
>>> their own runtime parameters then query-commands does not tell you
>>> anything because the specific BlockDriver may or may not support that
>>> runtime parameter - you need to use query-block.
>>
>> Let's reach agreement here. The choices are:
>>
>> 1. Top-level block_set command. Supported parameters are discovered
>> by looking query-block output.
>
> I'm strongly opposed to this. There needs to be a single consistent way to
> determine supported operations with QMP.
>
> And that single mechanism already exists--query_commands.
Okay, works for me.
Supriya: this means that there should be a block_set_hostcache command
and you don't need to worry about a generic block_set command.
Stefan
Re: [Qemu-devel] [PATCH 04/12] char: introduce backend tx queue
On Mon, Aug 1, 2011 at 3:23 PM, Anthony Liguori wrote:
> While the front tx queue has no flow control, the backend tx queue uses a
> polling function to determine when the front end can receive data.
>
> To convert this to the new queue model, we simply try to flush the backend tx
> queue whenever we poll. We then return the remaining space in the queue as
> the value of the polling function.
>
> Signed-off-by: Anthony Liguori
> ---
> qemu-char.c | 49 -
> qemu-char.h | 3 ++-
> 2 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 3f9b32c..2746652 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -173,6 +173,11 @@ static size_t char_queue_read(CharQueue *q, void *data,
> size_t size)
> return i;
> }
>
> +static uint32_t char_queue_get_avail(CharQueue *q)
> +{
> + return sizeof(q->ring) - (q->prod - q->cons);
> +}
"avail" confuses me. How many bytes are available for the consumer?
How many bytes are available for the producer?
How about char_queue_get_space() or char_queue_get_nfree()?
Stefan
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
On 08/01/2011 10:22 AM, Stefan Hajnoczi wrote:
On Thu, Jul 28, 2011 at 10:29 AM, Stefan Hajnoczi wrote:
On Wed, Jul 27, 2011 at 5:02 PM, Anthony Liguori wrote:
On 07/27/2011 09:31 AM, Stefan Hajnoczi wrote:
On Wed, Jul 27, 2011 at 1:58 PM, Anthony Liguori
wrote:
Index: qemu/hmp-commands.hx
===
--- qemu.orig/hmp-commands.hx
+++ qemu/hmp-commands.hx
@@ -70,6 +70,20 @@ but should be used with extreme caution.
resizes image files, it can not resize block devices like LVM volumes.
ETEXI
+{
+.name = "block_set",
+.args_type = "device:B,device:O",
+.params = "device [prop=value][,...]",
+.help = "Change block device parameters
[hostcache=on/off]",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_block_set,
+},
+STEXI
+@item block_set @var{config}
+@findex block_set
+Change block device parameters (eg: hostcache=on/off) while guest is
running.
+ETEXI
+
block_set_hostcache() please.
Multiplexing commands is generally a bad idea. It weakens typing. In
the
absence of a generic way to set block device properties, implementing
properties as generic in the QMP layer seems like a bad idea to me.
The idea behind block_set was to have a unified interface for changing
block device parameters at runtime. This prevents us from reinventing
new commands from scratch. For example, block I/O throttling is
already queued up to add run-time parameters.
Without a unified command we have a bulkier QMP/HMP interface,
duplicated code, and possibly inconsistencies in syntax between the
commands. Isn't the best way to avoid these problems a unified
interface?
I understand the lack of type safety concern but in this case we
already have to manually pull parsed arguments (i.e. cast to specific
types and deal with invalid input). To me this is a reason *for*
using a unified interface like block_set.
Think about it from a client perspective. How do I determine which
properties are supported by this version of QEMU? I have no way to identify
programmatically what arguments are valid for block_set.
OTOH, if you have strong types like block_set_hostcache, query-commands
tells me exactly what's supported.
Use query-block and see if 'hostcache' is there. If yes, then the
hostcache parameter is available. If we allow BlockDrivers to have
their own runtime parameters then query-commands does not tell you
anything because the specific BlockDriver may or may not support that
runtime parameter - you need to use query-block.
Let's reach agreement here. The choices are:
1. Top-level block_set command. Supported parameters are discovered
by looking query-block output.
I'm strongly opposed to this. There needs to be a single consistent way
to determine supported operations with QMP.
And that single mechanism already exists--query_commands.
2. Top-level command for each parameter (e.g. block_set_hostcache).
Supported parameters are easily discoverable via query-commands. If
individual block devices support different sets of parameters then
they may have to return -ENOTSUPP.
I like the block_set approach.
Anthony, Kevin, Supriya: Any thoughts?
For the sake of overall QMP sanity, I think block_set_hostcache is
really our only option.
Regards,
Anthony Liguori
Stefan
Re: [Qemu-devel] [RFC 21/24] qcow2: add qcow2_open_conversion_target()
Am 29.07.2011 06:49, schrieb Devin Nakamura:
> Still in very raw form. Not likely to work yet
>
> Signed-off-by: Devin Nakamura
I don't think it's quite as easy.
The problem is that qcow2 will access the image header in some places,
for example when growing the L1 or refcount table. You need to make sure
that it doesn't destroy the source format header, but writes to the
target format header at a different offset.
I think much of qcow2_open and qcow2_open_conversion_target could be the
same. That is, both could call a common function that takes a parameter
for the header offset.
The other part of qcow2_open_conversion_target (creating a new header
and a basic L1/refcount table) could possibly be shared with
qcow2_create2, though I'm not quite as sure about this one.
> ---
> block/qcow2.c | 124
> +
> 1 files changed, 124 insertions(+), 0 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 86df65d..f1e1e12 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1444,6 +1444,129 @@ static int
> qcow2_get_conversion_options(BlockDriverState *bs,
> }
> return 0;
> }
> +
> +
> +static int qcow2_open_conversion_target(BlockDriverState *bs,
> +BlockConversionOptions *drv_options,
> +QEMUOptionParameter *usr_options)
> +{
> +uint64_t imgsize, sectorsize, clusterbits;
> +uint64_t num_refcount_blocks;
> +uint16_t *refcount_block;
> +uint64_t table_index, table_limit, i;
> +
> +sectorsize = drv_options->cluster_size;
Sectors aren't clusters.
> +clusterbits = 0;
> +while (~sectorsize & 1) {
> +sectorsize >>=1;
> +clusterbits++;
> +}
> +if (sectorsize != 1) {
> +return -EINVAL;
> +}
Have a look at qcow2_create2, it uses ffs() to do this.
> +
> +
> +imgsize = drv_options->image_size;
> +
> +BDRVQcowState *s = bs->opaque;
> +s->crypt_method_header = QCOW_CRYPT_NONE;
Shouldn't this be taken from drv_options?
> +s->cluster_bits = clusterbits;
> +s->cluster_size = 1 << s->cluster_bits;
> +s->cluster_sectors = 1 << (s->cluster_bits - 9);
> +s->l2_bits = s->cluster_bits - 3; /* L2 is always one cluster */
> +s->l2_size = 1 << s->l2_bits;
> +bs->total_sectors = imgsize / 512;
> +s->csize_shift = (62 - (s->cluster_bits - 8));
> +s->csize_mask = (1 << (s->cluster_bits - 8)) - 1;
> +s->cluster_offset_mask = (1LL << s->csize_shift) - 1;
> +//s->refcount_table_offset = 0;
> +//s->refcount_table_size = 0;
> +s->snapshots_offset = 0;
> +s->nb_snapshots = 0;
> +/* alloc L2 table/refcount block cache */
> +s->l2_table_cache = qcow2_cache_create(bs, L2_CACHE_SIZE, true); //todo:
> double check writethrough
writethrough mode will hurt performance and isn't required here.
> +s->refcount_block_cache = qcow2_cache_create(bs, REFCOUNT_CACHE_SIZE,
> +true);
> +
> +s->cluster_cache = qemu_malloc(s->cluster_size);
> +/* one more sector for decompressed data alignment */
> +s->cluster_data = qemu_malloc(QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size
> + + 512);
> +s->cluster_cache_offset = -1;
> +
> +//Make up a refcount table
> +const uint64_t num_clusters = bs->file->total_sectors >>
> +(clusterbits - BDRV_SECTOR_BITS);
This is the number of cluster in the image file.
> +const uint64_t cluster_size = 1 << s->cluster_bits;
> +const uint64_t uint64_per_cluster = cluster_size / sizeof(uint64_t);
> +const uint64_t uint16_per_cluster = cluster_size / sizeof(uint16_t);
> +const uint64_t num_l2_tables = (num_clusters + uint64_per_cluster - 1) /
> +uint64_per_cluster;
> +const uint64_t num_l1_clusters = (num_l2_tables + uint64_per_cluster - 1)
> + / uint64_per_cluster);
L1/L2 tables are for clusters on the virtual disk. This can be something
completely different, and with some bad luck even larger than what you
calculate here (consider an image where in each L2 table only one
cluster is allocated - this will make the image files very small, but
requires still many L2 tables).
> +s->l1_size = num_l2_tables;
> +s->l1_table = qemu_mallocz(s->l1_size * s->cluster_size);
> +num_refcount_blocks = num_clusters + num_l2_tables + num_l1_clusters;
> +num_refcount_blocks = (num_refcount_blocks + uint16_per_cluster - 1) /
> + uint16_per_cluster;
> +
> +/* Account for refcount blocks used to track refcount blocks */
> +num_refcount_blocks *= uint16_per_cluster;
> +num_refcount_blocks /= uint16_per_cluster -1;
> +num_refcount_blocks += 1; //todo: fix rounding
Not sure what you're trying to do here, but align_offset() from qcow2.h
could help.
> +
> +/* Account for refcount blocks used to track refcount table */
> +num_
Re: [Qemu-devel] [V5 Patch 3/4]Qemu: Command "block_set" for dynamic block params change
On Thu, Jul 28, 2011 at 10:29 AM, Stefan Hajnoczi wrote:
> On Wed, Jul 27, 2011 at 5:02 PM, Anthony Liguori
> wrote:
>> On 07/27/2011 09:31 AM, Stefan Hajnoczi wrote:
>>>
>>> On Wed, Jul 27, 2011 at 1:58 PM, Anthony Liguori
>>> wrote:
>
> Index: qemu/hmp-commands.hx
> ===
> --- qemu.orig/hmp-commands.hx
> +++ qemu/hmp-commands.hx
> @@ -70,6 +70,20 @@ but should be used with extreme caution.
> resizes image files, it can not resize block devices like LVM volumes.
> ETEXI
>
> + {
> + .name = "block_set",
> + .args_type = "device:B,device:O",
> + .params = "device [prop=value][,...]",
> + .help = "Change block device parameters
> [hostcache=on/off]",
> + .user_print = monitor_user_noop,
> + .mhandler.cmd_new = do_block_set,
> + },
> +STEXI
> +@item block_set @var{config}
> +@findex block_set
> +Change block device parameters (eg: hostcache=on/off) while guest is
> running.
> +ETEXI
> +
block_set_hostcache() please.
Multiplexing commands is generally a bad idea. It weakens typing. In
the
absence of a generic way to set block device properties, implementing
properties as generic in the QMP layer seems like a bad idea to me.
>>>
>>> The idea behind block_set was to have a unified interface for changing
>>> block device parameters at runtime. This prevents us from reinventing
>>> new commands from scratch. For example, block I/O throttling is
>>> already queued up to add run-time parameters.
>>>
>>> Without a unified command we have a bulkier QMP/HMP interface,
>>> duplicated code, and possibly inconsistencies in syntax between the
>>> commands. Isn't the best way to avoid these problems a unified
>>> interface?
>>>
>>> I understand the lack of type safety concern but in this case we
>>> already have to manually pull parsed arguments (i.e. cast to specific
>>> types and deal with invalid input). To me this is a reason *for*
>>> using a unified interface like block_set.
>>
>> Think about it from a client perspective. How do I determine which
>> properties are supported by this version of QEMU? I have no way to identify
>> programmatically what arguments are valid for block_set.
>>
>> OTOH, if you have strong types like block_set_hostcache, query-commands
>> tells me exactly what's supported.
>
> Use query-block and see if 'hostcache' is there. If yes, then the
> hostcache parameter is available. If we allow BlockDrivers to have
> their own runtime parameters then query-commands does not tell you
> anything because the specific BlockDriver may or may not support that
> runtime parameter - you need to use query-block.
Let's reach agreement here. The choices are:
1. Top-level block_set command. Supported parameters are discovered
by looking query-block output.
2. Top-level command for each parameter (e.g. block_set_hostcache).
Supported parameters are easily discoverable via query-commands. If
individual block devices support different sets of parameters then
they may have to return -ENOTSUPP.
I like the block_set approach.
Anthony, Kevin, Supriya: Any thoughts?
Stefan
[Qemu-devel] [PATCH 02/12] char: rename qemu_chr_[can_]read() to qemu_chr_be_[can_]write().
To be clear about who calls this function and for what purpose.
Signed-off-by: Anthony Liguori
---
console.c |4 ++--
gdbstub.c |2 +-
hw/baum.c | 12 ++--
hw/msmouse.c |2 +-
qemu-char.c | 38 +++---
qemu-char.h |4 ++--
spice-qemu-char.c |4 ++--
7 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/console.c b/console.c
index 242086c..489c74c 100644
--- a/console.c
+++ b/console.c
@@ -1123,14 +1123,14 @@ static void kbd_send_chars(void *opaque)
int len;
uint8_t buf[16];
-len = qemu_chr_can_read(s->chr);
+len = qemu_chr_be_can_write(s->chr);
if (len > s->out_fifo.count)
len = s->out_fifo.count;
if (len > 0) {
if (len > sizeof(buf))
len = sizeof(buf);
qemu_fifo_read(&s->out_fifo, buf, len);
-qemu_chr_read(s->chr, buf, len);
+qemu_chr_be_write(s->chr, buf, len);
}
/* characters are pending: we send them a bit later (XXX:
horrible, should change char device API) */
diff --git a/gdbstub.c b/gdbstub.c
index d6c362e..058c626 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2194,7 +2194,7 @@ static int gdb_handle_packet(GDBState *s, const char
*line_buf)
hextomem(mem_buf, p + 5, len);
len = len / 2;
mem_buf[len++] = 0;
-qemu_chr_read(s->mon_chr, mem_buf, len);
+qemu_chr_be_write(s->mon_chr, mem_buf, len);
put_packet(s, "OK");
break;
}
diff --git a/hw/baum.c b/hw/baum.c
index 33a22a7..244eee5 100644
--- a/hw/baum.c
+++ b/hw/baum.c
@@ -223,7 +223,7 @@ static void baum_accept_input(struct CharDriverState *chr)
if (!baum->out_buf_used)
return;
-room = qemu_chr_can_read(chr);
+room = qemu_chr_be_can_write(chr);
if (!room)
return;
if (room > baum->out_buf_used)
@@ -231,12 +231,12 @@ static void baum_accept_input(struct CharDriverState *chr)
first = BUF_SIZE - baum->out_buf_ptr;
if (room > first) {
-qemu_chr_read(chr, baum->out_buf + baum->out_buf_ptr, first);
+qemu_chr_be_write(chr, baum->out_buf + baum->out_buf_ptr, first);
baum->out_buf_ptr = 0;
baum->out_buf_used -= first;
room -= first;
}
-qemu_chr_read(chr, baum->out_buf + baum->out_buf_ptr, room);
+qemu_chr_be_write(chr, baum->out_buf + baum->out_buf_ptr, room);
baum->out_buf_ptr += room;
baum->out_buf_used -= room;
}
@@ -250,16 +250,16 @@ static void baum_write_packet(BaumDriverState *baum,
const uint8_t *buf, int len
while (len--)
if ((*cur++ = *buf++) == ESC)
*cur++ = ESC;
-room = qemu_chr_can_read(baum->chr);
+room = qemu_chr_be_can_write(baum->chr);
len = cur - io_buf;
if (len <= room) {
/* Fits */
-qemu_chr_read(baum->chr, io_buf, len);
+qemu_chr_be_write(baum->chr, io_buf, len);
} else {
int first;
uint8_t out;
/* Can't fit all, send what can be, and store the rest. */
-qemu_chr_read(baum->chr, io_buf, room);
+qemu_chr_be_write(baum->chr, io_buf, room);
len -= room;
cur = io_buf + room;
if (len > BUF_SIZE - baum->out_buf_used) {
diff --git a/hw/msmouse.c b/hw/msmouse.c
index 67c6cd4..fbed80b 100644
--- a/hw/msmouse.c
+++ b/hw/msmouse.c
@@ -50,7 +50,7 @@ static void msmouse_event(void *opaque,
/* We always send the packet of, so that we do not have to keep track
of previous state of the middle button. This can potentially confuse
some very old drivers for two button mice though. */
-qemu_chr_read(chr, bytes, 4);
+qemu_chr_be_write(chr, bytes, 4);
}
static int msmouse_chr_write (struct CharDriverState *s, const uint8_t *buf,
int len)
diff --git a/qemu-char.c b/qemu-char.c
index fe5b28e..795a3cc 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -151,14 +151,14 @@ int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg)
return s->chr_ioctl(s, cmd, arg);
}
-int qemu_chr_can_read(CharDriverState *s)
+int qemu_chr_be_can_write(CharDriverState *s)
{
if (!s->chr_can_read)
return 0;
return s->chr_can_read(s->handler_opaque);
}
-void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len)
+void qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len)
{
s->chr_read(s->handler_opaque, buf, len);
}
@@ -565,7 +565,7 @@ static int fd_chr_read_poll(void *opaque)
CharDriverState *chr = opaque;
FDCharDriver *s = chr->opaque;
-s->max_size = qemu_chr_can_read(chr);
+s->max_size = qemu_chr_be_can_write(chr);
return s->max_size;
}
@@ -589,7 +589,7 @@ static void fd_chr_read(void *opaque)
return;
}
if (size > 0) {
-qemu_chr_read(chr, buf, size);
+qemu_chr_be_write(chr, buf, size);
}
}
@@ -699,8 +699,8 @@ static int stdio_read_poll(void *opaque)
CharDriverSt
Re: [Qemu-devel] [PATCH V2 1/2] [SLIRP] Simple ARP table
On 2011-08-01 17:03, Fabien Chouteau wrote:
> On 30/07/2011 11:19, Jan Kiszka wrote:
>> On 2011-07-30 11:09, Jan Kiszka wrote:
>>> On 2011-07-29 19:34, Jan Kiszka wrote:
On 2011-07-29 18:25, Fabien Chouteau wrote:
> This patch adds a simple ARP table in Slirp and also adds handling of
> gratuitous ARP requests.
>
> Signed-off-by: Fabien Chouteau
> ---
> Makefile.objs |2 +-
> slirp/arp_table.c | 50 ++
> slirp/bootp.c | 23 --
> slirp/slirp.c | 63
> +---
> slirp/slirp.h | 50 +++--
> 5 files changed, 129 insertions(+), 59 deletions(-)
> create mode 100644 slirp/arp_table.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 6991a9f..0c10557 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -151,7 +151,7 @@ common-obj-y += qemu-timer.o qemu-timer-common.o
>
> slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o
> slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o
> tcp_output.o
> -slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o
> +slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o
> common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
>
> # xen backend driver support
> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
> new file mode 100644
> index 000..5d7404b
> --- /dev/null
> +++ b/slirp/arp_table.c
> @@ -0,0 +1,50 @@
> +#include "slirp.h"
> +
> +void arp_table_add(ArpTable *arptbl,
> + int ip_addr,
> + uint8_t ethaddr[ETH_ALEN])
I still prefer the condensed formatting, but that's a minor nit.
>
> OK I'll change it to keep consistent style.
>
> Unfortunately, there's nothing on this subject in the CODING_STYLE...
We should add a section on consistency - but I guess that will always
remain a subjective matter. :)
>
> +{
> +int i;
> +
> +DEBUG_CALL("arp_table_add");
> +DEBUG_ARG("ip = 0x%x", ip_addr);
> +DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
> +ethaddr[0], ethaddr[1], ethaddr[2],
> +ethaddr[3], ethaddr[4], ethaddr[5]));
> +
> +/* Search for an entry */
> +for (i = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0; i++)
> {
Missed that on round #1: Why treating 0.0.0.0 special? If ip_addr can be
zero, the current logic will append every "update" of that address as a
new entry.
>
> Isn't 0.0.0.0 a reserved address? I think it's safe to use it here.
Actually, the whole 0.0.0.0/8 is source-only, ie. it should never show
up in the ARP table.
Jan
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH 12/12] char: enforce the use of qemu_chr_guest_open()
Signed-off-by: Anthony Liguori
---
qemu-char.c | 10 ++
qemu-char.h |2 ++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index fb7c937..5e62795 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -206,6 +206,8 @@ int qemu_chr_fe_write(CharDriverState *s, const uint8_t
*buf, int len)
int ret;
bool is_empty;
+assert(s->fe_opened > 0);
+
is_empty = char_queue_get_empty(&s->fe_tx);
ret = char_queue_write(&s->fe_tx, buf, len);
@@ -228,6 +230,8 @@ int qemu_chr_fe_read(CharDriverState *s, uint8_t *buf, int
len)
bool is_full;
int ret;
+assert(s->fe_opened > 0);
+
is_full = (char_queue_get_avail(&s->be_tx) == 0);
ret = char_queue_read(&s->be_tx, buf, len);
@@ -249,6 +253,8 @@ void qemu_chr_fe_set_handlers(CharDriverState *s,
IOEventHandler *chr_event,
void *opaque)
{
+assert(s->fe_opened > 0);
+
if (!opaque && !chr_read && !chr_write && !chr_event) {
/* chr driver being released. */
++s->avail_connections;
@@ -2822,6 +2828,8 @@ void qemu_chr_set_echo(struct CharDriverState *chr, bool
echo)
void qemu_chr_fe_open(struct CharDriverState *chr)
{
+chr->fe_opened++;
+
if (chr->chr_guest_open) {
chr->chr_guest_open(chr);
}
@@ -2832,6 +2840,8 @@ void qemu_chr_fe_close(struct CharDriverState *chr)
if (chr->chr_guest_close) {
chr->chr_guest_close(chr);
}
+
+chr->fe_opened--;
}
void qemu_chr_close(CharDriverState *chr)
diff --git a/qemu-char.h b/qemu-char.h
index 8b37fcf..b910c5e 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -88,6 +88,8 @@ struct CharDriverState {
int opened;
int avail_connections;
+int fe_opened;
+
CharQueue fe_tx;
CharQueue be_tx;
--
1.7.4.1
[Qemu-devel] [PATCH 03/12] char: introduce tx queue to enable Unix style flow control
The char layer tries very hard to avoid using an intermediate buffer. The
implication of this is that when the backend does a write(), the data for that
write must be immediately passed to the front end.
Flow control is needed to handle the likely event that the front end is not
able to handle the data at this point in time. We implement flow control
today by allowing the front ends to register a polling function. The polling
function returns non-zero when it is able to receive data.
This works okay because most backends are tied to some sort of file descriptor
and our main loop allows polling to be included with file descriptor
registration.
This falls completely apart when dealing with the front end writing to the
back end though because the front end (devices) don't have an obvious place to
integrate polling.
Short summary: we're broken by design. A way to fix this is to eliminate
polling entirely and use a Unix style flow control mechanism. This involves
using an intermediate buffer and allowing registration of notifications when
the buffer either has data in it (readability) or is not full (writability).
This patch introduces a queue and uses it for front end -> back end writes.
Signed-off-by: Anthony Liguori
---
qemu-char.c | 58 +-
qemu-char.h | 11 +++
2 files changed, 68 insertions(+), 1 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 795a3cc..3f9b32c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -139,9 +139,65 @@ void qemu_chr_generic_open(CharDriverState *s)
}
}
+static size_t char_queue_write(CharQueue *q, const void *data, size_t size)
+{
+const uint8_t *ptr = data;
+size_t i;
+
+for (i = 0; i < size; i++) {
+if ((q->prod - q->cons) == sizeof(q->ring)) {
+break;
+}
+
+q->ring[q->prod % sizeof(q->ring)] = ptr[i];
+q->prod++;
+}
+
+return i;
+}
+
+static size_t char_queue_read(CharQueue *q, void *data, size_t size)
+{
+uint8_t *ptr = data;
+size_t i;
+
+for (i = 0; i < size; i++) {
+if (q->cons == q->prod) {
+break;
+}
+
+ptr[i] = q->ring[q->cons % sizeof(q->ring)];
+q->cons++;
+}
+
+return i;
+}
+
+static void qemu_chr_flush_fe_tx(CharDriverState *s)
+{
+uint8_t buf[MAX_CHAR_QUEUE_RING];
+int len, written_len;
+
+/* Drain the queue into a flat buffer */
+len = char_queue_read(&s->fe_tx, buf, sizeof(buf));
+
+written_len = s->chr_write(s, buf, len);
+if (written_len < len) {
+/* If the backend didn't accept the full write, queue the unwritten
+ * data back in the queue. */
+char_queue_write(&s->fe_tx, &buf[written_len], len - written_len);
+}
+}
+
int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len)
{
-return s->chr_write(s, buf, len);
+int ret;
+
+ret = char_queue_write(&s->fe_tx, buf, len);
+
+qemu_chr_flush_fe_tx(s);
+
+return ret;
}
int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg)
diff --git a/qemu-char.h b/qemu-char.h
index bcd413c..bb9c1a7 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -51,6 +51,15 @@ typedef struct {
typedef void IOEventHandler(void *opaque, int event);
+#define MAX_CHAR_QUEUE_RING 1024
+
+typedef struct CharQueue
+{
+uint32_t prod;
+uint32_t cons;
+uint8_t ring[MAX_CHAR_QUEUE_RING];
+} CharQueue;
+
struct CharDriverState {
void (*init)(struct CharDriverState *s);
int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len);
@@ -75,6 +84,8 @@ struct CharDriverState {
int opened;
int avail_connections;
+CharQueue fe_tx;
+
QTAILQ_ENTRY(CharDriverState) next;
};
--
1.7.4.1
Re: [Qemu-devel] [PATCH V2 1/2] [SLIRP] Simple ARP table
On 30/07/2011 11:19, Jan Kiszka wrote:
> On 2011-07-30 11:09, Jan Kiszka wrote:
>> On 2011-07-29 19:34, Jan Kiszka wrote:
>>> On 2011-07-29 18:25, Fabien Chouteau wrote:
This patch adds a simple ARP table in Slirp and also adds handling of
gratuitous ARP requests.
Signed-off-by: Fabien Chouteau
---
Makefile.objs |2 +-
slirp/arp_table.c | 50 ++
slirp/bootp.c | 23 --
slirp/slirp.c | 63
+---
slirp/slirp.h | 50 +++--
5 files changed, 129 insertions(+), 59 deletions(-)
create mode 100644 slirp/arp_table.c
diff --git a/Makefile.objs b/Makefile.objs
index 6991a9f..0c10557 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -151,7 +151,7 @@ common-obj-y += qemu-timer.o qemu-timer-common.o
slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o
slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o
tcp_output.o
-slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o
+slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o
common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
# xen backend driver support
diff --git a/slirp/arp_table.c b/slirp/arp_table.c
new file mode 100644
index 000..5d7404b
--- /dev/null
+++ b/slirp/arp_table.c
@@ -0,0 +1,50 @@
+#include "slirp.h"
+
+void arp_table_add(ArpTable *arptbl,
+ int ip_addr,
+ uint8_t ethaddr[ETH_ALEN])
>>>
>>> I still prefer the condensed formatting, but that's a minor nit.
OK I'll change it to keep consistent style.
Unfortunately, there's nothing on this subject in the CODING_STYLE...
>>>
+{
+int i;
+
+DEBUG_CALL("arp_table_add");
+DEBUG_ARG("ip = 0x%x", ip_addr);
+DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
+ethaddr[0], ethaddr[1], ethaddr[2],
+ethaddr[3], ethaddr[4], ethaddr[5]));
+
+/* Search for an entry */
+for (i = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0; i++) {
>>>
>>> Missed that on round #1: Why treating 0.0.0.0 special? If ip_addr can be
>>> zero, the current logic will append every "update" of that address as a
>>> new entry.
Isn't 0.0.0.0 a reserved address? I think it's safe to use it here.
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 1eb2ed1..07cbb80 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -209,6 +211,11 @@ static void bootp_reply(Slirp *slirp, const struct
bootp_t *bp)
}
}
+if (daddr.sin_addr.s_addr != 0) {
+/* Update ARP table for this IP address */
+arp_table_add(&slirp->arp_table, daddr.sin_addr.s_addr,
client_ethaddr);
>>>
>>> Here you prevent that arp_table_add is called with a zero IP, but not in
>>> arp_input below. Likely harmless, but at least inconsistent.
I will handle this case directly in arp_table_add to get a consistent behavior.
>>
>> Unfortunately, this patch also has a more sever issues: it breaks DHCP e.g.
>>
>> Slirp's bootp sends out all replies, also acks and offers, as
>> broadcasts. Normal servers already use the clients IP address as
>> destination here.
>
> Obviously, using a broadcast address on return is valid as well. So this
> is no slirp bug, it's purely an ARP table lookup issue introduced by
> this patch.
>
>>
>> Even if bootp is fixed, you still lack logic to deal with special
>> addresses in your arp table lookup. At least broadcasts need to be
>> handled, I think other multicasts aren't supported by slirp anyway.
>>
>
That's right, I will add broadcast handling in the next version.
Thanks for your review.
--
Fabien Chouteau
[Qemu-devel] [PATCH 11/12] char: make all devices do qemu_chr_fe_open()
The semantics of guest_open are unreliable today because they there's no way
of knowing whether a front end will ever call open.
Let's make them reliable which means making everything call open.
Signed-off-by: Anthony Liguori
---
gdbstub.c |1 +
hw/ccid-card-passthru.c |1 +
hw/debugcon.c |1 +
hw/escc.c |1 +
hw/etraxfs_ser.c|4 +++-
hw/grlib_apbuart.c |1 +
hw/ivshmem.c|2 ++
hw/lm32_juart.c |1 +
hw/lm32_uart.c |1 +
hw/mcf_uart.c |1 +
hw/milkymist-uart.c |1 +
hw/pl011.c |1 +
hw/pxa2xx.c |4 +++-
hw/serial.c |2 ++
hw/sh_serial.c |4 +++-
hw/spapr_vty.c |1 +
hw/strongarm.c |1 +
hw/syborg_serial.c |1 +
hw/usb-serial.c |1 +
hw/xen_console.c|8 ++--
hw/xilinx_uartlite.c|4 +++-
monitor.c |2 ++
usb-redir.c |1 +
23 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 058c626..8e0459e 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2768,6 +2768,7 @@ int gdbserver_start(const char *device)
if (!chr)
return -1;
+qemu_chr_fe_open(chr);
qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
gdb_chr_event, NULL);
}
diff --git a/hw/ccid-card-passthru.c b/hw/ccid-card-passthru.c
index 082fd82..00468c9 100644
--- a/hw/ccid-card-passthru.c
+++ b/hw/ccid-card-passthru.c
@@ -281,6 +281,7 @@ static int passthru_initfn(CCIDCardState *base)
card->vscard_in_hdr = 0;
if (card->cs) {
DPRINTF(card, D_INFO, "initing chardev\n");
+qemu_chr_fe_open(card->cs);
qemu_chr_add_handlers(card->cs,
ccid_card_vscard_can_read,
ccid_card_vscard_read,
diff --git a/hw/debugcon.c b/hw/debugcon.c
index c9ee6d9..1faa918 100644
--- a/hw/debugcon.c
+++ b/hw/debugcon.c
@@ -73,6 +73,7 @@ static void debugcon_init_core(DebugconState *s)
exit(1);
}
+qemu_chr_fe_open(s->chr);
qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, s);
}
diff --git a/hw/escc.c b/hw/escc.c
index c1460b7..9f98bba 100644
--- a/hw/escc.c
+++ b/hw/escc.c
@@ -911,6 +911,7 @@ static int escc_init1(SysBusDevice *dev)
s->chn[i].chn = 1 - i;
s->chn[i].clock = s->frequency / 2;
if (s->chn[i].chr) {
+qemu_chr_fe_open(s->chn[i].chr);
qemu_chr_add_handlers(s->chn[i].chr, serial_can_receive,
serial_receive1, serial_event, &s->chn[i]);
}
diff --git a/hw/etraxfs_ser.c b/hw/etraxfs_ser.c
index 35f8325..ccffa3e 100644
--- a/hw/etraxfs_ser.c
+++ b/hw/etraxfs_ser.c
@@ -212,10 +212,12 @@ static int etraxfs_ser_init(SysBusDevice *dev)
DEVICE_NATIVE_ENDIAN);
sysbus_init_mmio(dev, R_MAX * 4, ser_regs);
s->chr = qdev_init_chardev(&dev->qdev);
-if (s->chr)
+if (s->chr) {
+qemu_chr_fe_open(s->chr);
qemu_chr_add_handlers(s->chr,
serial_can_receive, serial_receive,
serial_event, s);
+}
return 0;
}
diff --git a/hw/grlib_apbuart.c b/hw/grlib_apbuart.c
index c90b810..184a0c8 100644
--- a/hw/grlib_apbuart.c
+++ b/hw/grlib_apbuart.c
@@ -149,6 +149,7 @@ static int grlib_apbuart_init(SysBusDevice *dev)
UART *uart = FROM_SYSBUS(typeof(*uart), dev);
int uart_regs = 0;
+qemu_chr_fe_open(uart->chr);
qemu_chr_add_handlers(uart->chr,
grlib_apbuart_can_receive,
grlib_apbuart_receive,
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index 3055dd2..6c126eb 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -326,6 +326,7 @@ static CharDriverState* create_eventfd_chr_device(void *
opaque, int eventfd,
exit(-1);
}
+qemu_chr_fe_open(chr);
/* if MSI is supported we need multiple interrupts */
if (ivshmem_has_feature(s, IVSHMEM_MSI)) {
s->eventfd_table[vector].pdev = &s->dev;
@@ -749,6 +750,7 @@ static int pci_ivshmem_init(PCIDevice *dev)
s->eventfd_chr = qemu_mallocz(s->vectors * sizeof(CharDriverState *));
+qemu_chr_fe_open(s->server_chr);
qemu_chr_add_handlers(s->server_chr, ivshmem_can_receive, ivshmem_read,
ivshmem_event, s);
} else {
diff --git a/hw/lm32_juart.c b/hw/lm32_juart.c
index 5454aa4..c3bf70c 100644
--- a/hw/lm32_juart.c
+++ b/hw/lm32_juart.c
@@ -116,6 +116,7 @@ static int lm32_juart_init(SysBusDevice *dev)
s->chr = qdev_init_chardev(&dev->qdev);
if (s->chr) {
+qemu_chr_fe_open(s->chr);
qemu_chr_add_handlers(s->chr, juart_can_rx, juart_rx, juart_event, s);
}
diff --git a/hw/lm32_uart.c b/hw/lm32_uart.c
index 3678545..3dbe735 100644
--- a/hw/lm32_uart.c
+++ b/hw/lm32_uart.c
[Qemu-devel] [PATCH 07/12] char: add backend edge notification interface
Signed-off-by: Anthony Liguori
---
qemu-char.c | 38 ++
qemu-char.h |2 ++
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 9e40e04..4ba18e2 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -188,6 +188,11 @@ static void qemu_chr_flush_fe_tx(CharDriverState *s)
uint8_t buf[MAX_CHAR_QUEUE_RING];
int len;
+/* Don't use polling queue if new style handlers are registered */
+if (s->be_read || s->be_write) {
+return;
+}
+
/* Drain the queue into a flat buffer */
len = char_queue_read(&s->fe_tx, buf, sizeof(buf));
@@ -199,9 +204,20 @@ static void qemu_chr_flush_fe_tx(CharDriverState *s)
int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len)
{
int ret;
+bool is_empty;
+
+is_empty = char_queue_get_empty(&s->fe_tx);
ret = char_queue_write(&s->fe_tx, buf, len);
+/* If the queue was empty, and now it's not, generate a read edge
+ * event for the backend. */
+if (is_empty && !char_queue_get_empty(&s->fe_tx)) {
+if (s->be_read) {
+s->be_read(s->opaque);
+}
+}
+
qemu_chr_flush_fe_tx(s);
return ret;
@@ -209,7 +225,22 @@ int qemu_chr_fe_write(CharDriverState *s, const uint8_t
*buf, int len)
int qemu_chr_fe_read(CharDriverState *s, uint8_t *buf, int len)
{
-return char_queue_read(&s->be_tx, buf, len);
+bool is_full;
+int ret;
+
+is_full = (char_queue_get_avail(&s->be_tx) == 0);
+
+ret = char_queue_read(&s->be_tx, buf, len);
+
+/* If the queue was empty, and now its not, generate a write edge
+ * event for the backend. */
+if (is_full && char_queue_get_avail(&s->be_tx)) {
+if (s->be_write) {
+s->be_write(s->opaque);
+}
+}
+
+return ret;
}
void qemu_chr_fe_set_handlers(CharDriverState *s,
@@ -259,7 +290,7 @@ static void qemu_chr_flush_be_tx(CharDriverState *s)
/* We only drained what we knew the be could handle so we don't need to
* requeue any data. */
-s->chr_read(s, buf, len);
+s->chr_read(s->handler_opaque, buf, len);
}
int qemu_chr_be_can_write(CharDriverState *s)
@@ -282,8 +313,7 @@ int qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int
len)
/* If the queue was empty, and now it's not, trigger a read edge
* event. */
-if ((s->fe_read || s->fe_write) &&
-(is_empty && !char_queue_get_empty(&s->be_tx))) {
+if (is_empty && !char_queue_get_empty(&s->be_tx)) {
if (s->fe_read) {
s->fe_read(s->handler_opaque);
}
diff --git a/qemu-char.h b/qemu-char.h
index 84fd628..c5bbaf6 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -63,6 +63,8 @@ typedef struct CharQueue
struct CharDriverState {
void (*init)(struct CharDriverState *s);
int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len);
+IOHandler *be_read;
+IOHandler *be_write;
void (*chr_update_read_handler)(struct CharDriverState *s);
int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg);
int (*get_msgfd)(struct CharDriverState *s);
--
1.7.4.1
[Qemu-devel] [PATCH 10/12] char: rename qemu_chr_guest_close() -> qemu_chr_fe_close()
To be more explicit about what the function is for.
Signed-off-by: Anthony Liguori
---
hw/virtio-console.c |2 +-
qemu-char.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 21f752b..d3351c8 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -60,7 +60,7 @@ static void guest_close(VirtIOSerialPort *port)
{
VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
-qemu_chr_guest_close(vcon->chr);
+qemu_chr_fe_close(vcon->chr);
}
/* Readiness of the guest to accept data on a port */
diff --git a/qemu-char.c b/qemu-char.c
index 7e2cff0..fb7c937 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2827,7 +2827,7 @@ void qemu_chr_fe_open(struct CharDriverState *chr)
}
}
-void qemu_chr_guest_close(struct CharDriverState *chr)
+void qemu_chr_fe_close(struct CharDriverState *chr)
{
if (chr->chr_guest_close) {
chr->chr_guest_close(chr);
--
1.7.4.1
[Qemu-devel] [PATCH 09/12] char: rename qemu_chr_guest_open() -> qemu_chr_fe_open()
---
hw/virtio-console.c |2 +-
qemu-char.c |2 +-
qemu-char.h |4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 6c386fa..21f752b 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -52,7 +52,7 @@ static void guest_open(VirtIOSerialPort *port)
{
VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
-qemu_chr_guest_open(vcon->chr);
+qemu_chr_fe_open(vcon->chr);
}
/* Callback function that's called when the guest closes the port */
diff --git a/qemu-char.c b/qemu-char.c
index 4ba18e2..7e2cff0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2820,7 +2820,7 @@ void qemu_chr_set_echo(struct CharDriverState *chr, bool
echo)
}
}
-void qemu_chr_guest_open(struct CharDriverState *chr)
+void qemu_chr_fe_open(struct CharDriverState *chr)
{
if (chr->chr_guest_open) {
chr->chr_guest_open(chr);
diff --git a/qemu-char.h b/qemu-char.h
index c5bbaf6..8b37fcf 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -99,8 +99,8 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
void (*init)(struct CharDriverState *s));
CharDriverState *qemu_chr_open(const char *label, const char *filename, void
(*init)(struct CharDriverState *s));
void qemu_chr_set_echo(struct CharDriverState *chr, bool echo);
-void qemu_chr_guest_open(struct CharDriverState *chr);
-void qemu_chr_guest_close(struct CharDriverState *chr);
+void qemu_chr_fe_open(struct CharDriverState *chr);
+void qemu_chr_fe_close(struct CharDriverState *chr);
void qemu_chr_close(CharDriverState *chr);
void qemu_chr_printf(CharDriverState *s, const char *fmt, ...)
GCC_FMT_ATTR(2, 3);
--
1.7.4.1
Re: [Qemu-devel] [RFC 19/24] qcow2: add qcow2_copy_header()
Am 29.07.2011 06:49, schrieb Devin Nakamura: > Signed-off-by: Devin Nakamura > --- > block/qcow2.c | 54 ++ > 1 files changed, 54 insertions(+), 0 deletions(-) Do we really need to build a completely new header? We should already have one at a different offset that we only need to copy, no? Kevin
Re: [Qemu-devel] 0.15 testing, spice, wxp
On 07/29/11 04:48, Rick Vernam wrote: I'd like to do some very high-level testing of qemu-stable-0.15 (qemu-system- x86_64, specifically) running Linux host, Windows XP SP3 guest. I'm afraid that I don't have a ready environment for building the windows guest bits, and the added time requirement simply puts it over the top for me. Are there updated spice windows guest binaries available anywhere? I suppose this would just be the QXL video driver and the guest agent? From looking at spice-space.org/download.html I concluded that I'm looking for verion 0.8.x, but my searches have left me empty handed. Just use the most recent versions from spice-space.org/download.html, they should work just fine with 0.15 HTH, Gerd
Re: [Qemu-devel] [RFC 18/24] qcow2: add qcow2_map
Am 29.07.2011 06:49, schrieb Devin Nakamura:
> Signed-off-by: Devin Nakamura
> ---
> block/qcow2-cluster.c | 49
> +
> block/qcow2.c |1 +
> block/qcow2.h |3 +++
> 3 files changed, 53 insertions(+), 0 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index ca56918..848f2ee 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -977,3 +977,52 @@ int qcow2_discard_clusters(BlockDriverState *bs,
> uint64_t offset,
>
> return 0;
> }
> +
> +int qcow2_map(BlockDriverState *bs, uint64_t guest_offset,
> +uint64_t host_offset, uint64_t contiguous_bytes)
> +{
> +BDRVQcowState *s = bs->opaque;
> +unsigned int nelms;
> +
> +
> +if ((s->cluster_size - 1) & guest_offset) {
Usually you would have guest_offset first.
> +return -EINVAL;
> +}
> +
> +if (contiguous_bytes % s->cluster_size) {
> +return -EINVAL;
> +}
Any reason why the two checks are different? I don't really care if they
use & or %, but they should use the same thing.
host_offset isn't checked at all?
> +
> +nelms = s->l2_size / sizeof(uint64_t);
s->l2_size is already in elements, not in bytes.
> +
> +while (contiguous_bytes > 0) {
> +unsigned int l1_index, l2_index;
> +uint64_t *l2_table;
> +int ret;
> +l1_index = guest_offset >> (s->l2_bits + s->cluster_bits);
> +l2_index = (guest_offset >> s->cluster_bits) & (s->l2_size - 1);
> +
> +if (!s->l1_table[l1_index]) {
> +ret = l2_allocate(bs, l1_index, &l2_table);
> +if (ret) {
> +return ret;
> +}
> +} else {
> +ret = l2_load(bs, s->l1_table[l1_index] & ~QCOW_OFLAG_COPIED,
> &l2_table);
> +if (ret) {
> +return ret;
> +}
> +}
Can't you use get_cluster_table() for this part?
> +qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
> +
> +for (; l2_index < nelms && contiguous_bytes > 0; l2_index++) {
> +l2_table[l2_index] = cpu_to_be64(host_offset |
> QCOW_OFLAG_COPIED);
You should increase the refcount for host_offset and set
QCOW_OFLAG_COPIED only if it becomes 1. Or maybe we should just fail
bdrv_map if the old refcount is != 0.
> +host_offset += s->cluster_size;
> +contiguous_bytes -= s->cluster_size;
> +}
> +
> +qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table);
> +
> +}
> +return 0;
> +}
Kevin
[Qemu-devel] [PULL] pci, virtio, net
The following changes since commit d1afc48b7cfdb4490f322d5d82a2aae6d545ec06: SPARC64: implement addtional MMU faults related to nonfaulting load (2011-07-21 20:02:22 +) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony Amit Shah (3): virtio-blk: Fix memleak on exit virtio-net: don't use vdev after virtio_cleanup virtio: Plug memleak by freeing vdev Isaku Yamahata (1): pcie_host: verify mmcfg address range Jan Kiszka (1): pci: Common overflow prevention Markus Armbruster (2): Fix automatically assigned network names for netdev Fix netdev name lookup in -device, device_add, netdev_del Michael S. Tsirkin (1): virtio-pci: use generic logic for command access hw/pci.c|6 ++ hw/pci_host.c | 24 hw/pci_host.h |6 ++ hw/pcie_host.c | 32 hw/virtio-blk.c |1 + hw/virtio-net.c |2 +- hw/virtio-pci.c | 18 +- hw/virtio.c |1 + net.c | 19 +++ 9 files changed, 79 insertions(+), 30 deletions(-)
[Qemu-devel] [PATCH 08/12] char: make monitor use new style interface
---
monitor.c | 35 ---
1 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/monitor.c b/monitor.c
index 38d4544..3ca211b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4747,13 +4747,6 @@ cleanup:
}
}
-static int monitor_can_read(void *opaque)
-{
-Monitor *mon = opaque;
-
-return (mon->suspend_cnt == 0) ? 1 : 0;
-}
-
static int invalid_qmp_mode(const Monitor *mon, const char *cmd_name)
{
int is_cap = compare_cmd(cmd_name, "qmp_capabilities");
@@ -5118,24 +5111,35 @@ out:
/**
* monitor_control_read(): Read and handle QMP input
*/
-static void monitor_control_read(void *opaque, const uint8_t *buf, int size)
+static void monitor_control_read(void *opaque)
{
Monitor *old_mon = cur_mon;
+uint8_t buf[1024];
+int size;
cur_mon = opaque;
-json_message_parser_feed(&cur_mon->mc->parser, (const char *) buf, size);
+size = qemu_chr_fe_read(cur_mon->chr, buf, sizeof(buf));
+json_message_parser_feed(&cur_mon->mc->parser, (const char *)buf, size);
cur_mon = old_mon;
}
-static void monitor_read(void *opaque, const uint8_t *buf, int size)
+static void monitor_read(void *opaque)
{
Monitor *old_mon = cur_mon;
+uint8_t buf[1024];
+int size;
int i;
cur_mon = opaque;
+if (cur_mon->suspend_cnt > 0) {
+return;
+}
+
+size = qemu_chr_fe_read(cur_mon->chr, buf, sizeof(buf));
+
if (cur_mon->rs) {
for (i = 0; i < size; i++)
readline_handle_byte(cur_mon->rs, buf[i]);
@@ -5168,8 +5172,10 @@ void monitor_resume(Monitor *mon)
{
if (!mon->rs)
return;
-if (--mon->suspend_cnt == 0)
+if (--mon->suspend_cnt == 0) {
readline_show_prompt(mon->rs);
+monitor_read(mon);
+}
}
static QObject *get_qmp_greeting(void)
@@ -5273,12 +5279,11 @@ void monitor_init(CharDriverState *chr, int flags)
if (monitor_ctrl_mode(mon)) {
mon->mc = qemu_mallocz(sizeof(MonitorControl));
/* Control mode requires special handlers */
-qemu_chr_add_handlers(chr, monitor_can_read, monitor_control_read,
- monitor_control_event, mon);
+qemu_chr_fe_set_handlers(chr, monitor_control_read, NULL,
+ monitor_control_event, mon);
qemu_chr_set_echo(chr, true);
} else {
-qemu_chr_add_handlers(chr, monitor_can_read, monitor_read,
- monitor_event, mon);
+qemu_chr_fe_set_handlers(chr, monitor_read, NULL, monitor_event, mon);
}
QLIST_INSERT_HEAD(&mon_list, mon, entry);
--
1.7.4.1
[Qemu-devel] [PATCH 04/12] char: introduce backend tx queue
While the front tx queue has no flow control, the backend tx queue uses a
polling function to determine when the front end can receive data.
To convert this to the new queue model, we simply try to flush the backend tx
queue whenever we poll. We then return the remaining space in the queue as
the value of the polling function.
Signed-off-by: Anthony Liguori
---
qemu-char.c | 49 -
qemu-char.h |3 ++-
2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 3f9b32c..2746652 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -173,6 +173,11 @@ static size_t char_queue_read(CharQueue *q, void *data,
size_t size)
return i;
}
+static uint32_t char_queue_get_avail(CharQueue *q)
+{
+return sizeof(q->ring) - (q->prod - q->cons);
+}
+
static void qemu_chr_flush_fe_tx(CharDriverState *s)
{
uint8_t buf[MAX_CHAR_QUEUE_RING];
@@ -200,23 +205,49 @@ int qemu_chr_fe_write(CharDriverState *s, const uint8_t
*buf, int len)
return ret;
}
-int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg)
+static void qemu_chr_flush_be_tx(CharDriverState *s)
{
-if (!s->chr_ioctl)
-return -ENOTSUP;
-return s->chr_ioctl(s, cmd, arg);
+uint8_t buf[MAX_CHAR_QUEUE_RING];
+int len;
+
+/* Only drain what the be can handle */
+len = s->chr_can_read(s->handler_opaque);
+if (len == 0) {
+return;
+}
+
+len = char_queue_read(&s->be_tx, buf, len);
+
+/* We only drained what we knew the be could handle so we don't need to
+ * requeue any data. */
+s->chr_read(s, buf, len);
}
int qemu_chr_be_can_write(CharDriverState *s)
{
-if (!s->chr_can_read)
-return 0;
-return s->chr_can_read(s->handler_opaque);
+/* Try to flush any queued data before returning how much data we can
+ * accept. */
+qemu_chr_flush_be_tx(s);
+
+return char_queue_get_avail(&s->be_tx);
}
-void qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len)
+int qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len)
{
-s->chr_read(s->handler_opaque, buf, len);
+int ret;
+
+ret = char_queue_write(&s->be_tx, buf, len);
+
+qemu_chr_flush_be_tx(s);
+
+return ret;
+}
+
+int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg)
+{
+if (!s->chr_ioctl)
+return -ENOTSUP;
+return s->chr_ioctl(s, cmd, arg);
}
int qemu_chr_get_msgfd(CharDriverState *s)
diff --git a/qemu-char.h b/qemu-char.h
index bb9c1a7..85735b5 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -85,6 +85,7 @@ struct CharDriverState {
int avail_connections;
CharQueue fe_tx;
+CharQueue be_tx;
QTAILQ_ENTRY(CharDriverState) next;
};
@@ -109,7 +110,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg);
void qemu_chr_generic_open(CharDriverState *s);
int qemu_chr_be_can_write(CharDriverState *s);
-void qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len);
+int qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len);
int qemu_chr_get_msgfd(CharDriverState *s);
void qemu_chr_accept_input(CharDriverState *s);
int qemu_chr_add_client(CharDriverState *s, int fd);
--
1.7.4.1
[Qemu-devel] [PATCH 06/12] char: add an edge event API for the front ends
Signed-off-by: Anthony Liguori
---
qemu-char.c | 91 +--
qemu-char.h |9 ++
2 files changed, 85 insertions(+), 15 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 0e4a30c..9e40e04 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -139,13 +139,23 @@ void qemu_chr_generic_open(CharDriverState *s)
}
}
+static uint32_t char_queue_get_avail(CharQueue *q)
+{
+return sizeof(q->ring) - (q->prod - q->cons);
+}
+
+static bool char_queue_get_empty(CharQueue *q)
+{
+return (q->cons == q->prod);
+}
+
static size_t char_queue_write(CharQueue *q, const void *data, size_t size)
{
const uint8_t *ptr = data;
size_t i;
for (i = 0; i < size; i++) {
-if ((q->prod - q->cons) == sizeof(q->ring)) {
+if (char_queue_get_avail(q) == 0) {
break;
}
@@ -162,7 +172,7 @@ static size_t char_queue_read(CharQueue *q, void *data,
size_t size)
size_t i;
for (i = 0; i < size; i++) {
-if (q->cons == q->prod) {
+if (char_queue_get_empty(q)) {
break;
}
@@ -173,25 +183,17 @@ static size_t char_queue_read(CharQueue *q, void *data,
size_t size)
return i;
}
-static uint32_t char_queue_get_avail(CharQueue *q)
-{
-return sizeof(q->ring) - (q->prod - q->cons);
-}
-
static void qemu_chr_flush_fe_tx(CharDriverState *s)
{
uint8_t buf[MAX_CHAR_QUEUE_RING];
-int len, written_len;
+int len;
/* Drain the queue into a flat buffer */
len = char_queue_read(&s->fe_tx, buf, sizeof(buf));
-written_len = s->chr_write(s, buf, len);
-if (written_len < len) {
-/* If the backend didn't accept the full write, queue the unwritten
- * data back in the queue. */
-char_queue_write(&s->fe_tx, &buf[written_len], len - written_len);
-}
+s->chr_write(s, buf, len);
+
+/* We drop unwritten data until we have backend flow control */
}
int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len)
@@ -210,11 +212,43 @@ int qemu_chr_fe_read(CharDriverState *s, uint8_t *buf,
int len)
return char_queue_read(&s->be_tx, buf, len);
}
+void qemu_chr_fe_set_handlers(CharDriverState *s,
+ IOHandler *chr_read,
+ IOHandler *chr_write,
+ IOEventHandler *chr_event,
+ void *opaque)
+{
+if (!opaque && !chr_read && !chr_write && !chr_event) {
+/* chr driver being released. */
+++s->avail_connections;
+}
+s->fe_read = chr_read;
+s->fe_write = chr_write;
+s->chr_event = chr_event;
+s->handler_opaque = opaque;
+
+/* FIXME broken for mux */
+if (s->chr_update_read_handler) {
+s->chr_update_read_handler(s);
+}
+
+/* We're connecting to an already opened device, so let's make sure we
+ also get the open event */
+if (s->opened) {
+qemu_chr_generic_open(s);
+}
+}
+
static void qemu_chr_flush_be_tx(CharDriverState *s)
{
uint8_t buf[MAX_CHAR_QUEUE_RING];
int len;
+/* Don't invoke the polling handlers if we have edge handlers installed. */
+if (s->fe_read || s->fe_write) {
+return;
+}
+
/* Only drain what the be can handle */
len = s->chr_can_read(s->handler_opaque);
if (len == 0) {
@@ -240,9 +274,21 @@ int qemu_chr_be_can_write(CharDriverState *s)
int qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len)
{
int ret;
+bool is_empty;
+
+is_empty = char_queue_get_empty(&s->be_tx);
ret = char_queue_write(&s->be_tx, buf, len);
+/* If the queue was empty, and now it's not, trigger a read edge
+ * event. */
+if ((s->fe_read || s->fe_write) &&
+(is_empty && !char_queue_get_empty(&s->be_tx))) {
+if (s->fe_read) {
+s->fe_read(s->handler_opaque);
+}
+}
+
qemu_chr_flush_be_tx(s);
return ret;
@@ -250,7 +296,22 @@ int qemu_chr_be_write(CharDriverState *s, uint8_t *buf,
int len)
int qemu_chr_be_read(CharDriverState *s, uint8_t *buf, int len)
{
-return char_queue_read(&s->fe_tx, buf, len);
+bool is_full;
+int ret;
+
+is_full = (char_queue_get_avail(&s->fe_tx) == 0);
+
+ret = char_queue_read(&s->fe_tx, buf, len);
+
+/* If the queue was full, and now it's not, trigger an write edge
+ * event. */
+if (is_full && char_queue_get_avail(&s->fe_tx)) {
+if (s->fe_write) {
+s->fe_write(s->handler_opaque);
+}
+}
+
+return ret;
}
int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg)
diff --git a/qemu-char.h b/qemu-char.h
index a75bd1c..84fd628 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -70,6 +70,8 @@ struct CharDriverState {
IOEventHandler *chr_event;
IOCanReadHandler *chr_can_read;
IOReadHandler *chr_read;
+IOHandler *fe_read;
+IOHandler *fe_write;
void *handler_opaque;
void (*
[Qemu-devel] [PATCH 05/12] char: add read functions for backend and frontend
Instead of waiting for a callback to send data, allow for reading the backend
and frontend tx queues directly.
Signed-off-by: Anthony Liguori
---
qemu-char.c | 10 ++
qemu-char.h |2 ++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 2746652..0e4a30c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -205,6 +205,11 @@ int qemu_chr_fe_write(CharDriverState *s, const uint8_t
*buf, int len)
return ret;
}
+int qemu_chr_fe_read(CharDriverState *s, uint8_t *buf, int len)
+{
+return char_queue_read(&s->be_tx, buf, len);
+}
+
static void qemu_chr_flush_be_tx(CharDriverState *s)
{
uint8_t buf[MAX_CHAR_QUEUE_RING];
@@ -243,6 +248,11 @@ int qemu_chr_be_write(CharDriverState *s, uint8_t *buf,
int len)
return ret;
}
+int qemu_chr_be_read(CharDriverState *s, uint8_t *buf, int len)
+{
+return char_queue_read(&s->fe_tx, buf, len);
+}
+
int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg)
{
if (!s->chr_ioctl)
diff --git a/qemu-char.h b/qemu-char.h
index 85735b5..a75bd1c 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -101,6 +101,7 @@ void qemu_chr_close(CharDriverState *chr);
void qemu_chr_printf(CharDriverState *s, const char *fmt, ...)
GCC_FMT_ATTR(2, 3);
int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len);
+int qemu_chr_fe_read(CharDriverState *s, uint8_t *buf, int len);
void qemu_chr_send_event(CharDriverState *s, int event);
void qemu_chr_add_handlers(CharDriverState *s,
IOCanReadHandler *fd_can_read,
@@ -111,6 +112,7 @@ int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg);
void qemu_chr_generic_open(CharDriverState *s);
int qemu_chr_be_can_write(CharDriverState *s);
int qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len);
+int qemu_chr_be_read(CharDriverState *s, uint8_t *buf, int len);
int qemu_chr_get_msgfd(CharDriverState *s);
void qemu_chr_accept_input(CharDriverState *s);
int qemu_chr_add_client(CharDriverState *s, int fd);
--
1.7.4.1
[Qemu-devel] [PATCH 01/12] char: rename qemu_chr_write() to qemu_chr_fe_write()
The char layer is confusing. There is a front-end, typically a device, that
can send and receive data. The front-end sends data by calling
qemu_chr_write().
The back-end, typically created via -chardev, can also send and receive data.
Oddly, it sends data by calling qemu_chr_read().
Let's be explicit about which function is for which party.
Signed-off-by: Anthony Liguori
---
gdbstub.c |2 +-
hw/ccid-card-passthru.c |4 ++--
hw/debugcon.c |2 +-
hw/escc.c |2 +-
hw/etraxfs_ser.c|2 +-
hw/grlib_apbuart.c |2 +-
hw/lm32_juart.c |2 +-
hw/lm32_uart.c |2 +-
hw/mcf_uart.c |2 +-
hw/milkymist-uart.c |2 +-
hw/omap2.c |6 +++---
hw/parallel.c |2 +-
hw/pl011.c |2 +-
hw/pxa2xx.c |2 +-
hw/serial.c |2 +-
hw/sh_serial.c |2 +-
hw/spapr_vty.c |4 ++--
hw/strongarm.c |2 +-
hw/syborg_serial.c |4 ++--
hw/usb-serial.c |2 +-
hw/virtio-console.c |2 +-
hw/xen_console.c|2 +-
hw/xilinx_uartlite.c|2 +-
monitor.c |2 +-
qemu-char.c |4 ++--
qemu-char.h |3 ++-
slirp/slirp.c |2 +-
usb-redir.c |2 +-
28 files changed, 35 insertions(+), 34 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 27b0cfa..d6c362e 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -382,7 +382,7 @@ static void put_buffer(GDBState *s, const uint8_t *buf, int
len)
}
}
#else
-qemu_chr_write(s->chr, buf, len);
+qemu_chr_fe_write(s->chr, buf, len);
#endif
}
diff --git a/hw/ccid-card-passthru.c b/hw/ccid-card-passthru.c
index 28eb9d1..082fd82 100644
--- a/hw/ccid-card-passthru.c
+++ b/hw/ccid-card-passthru.c
@@ -72,8 +72,8 @@ static void ccid_card_vscard_send_msg(PassthruState *s,
scr_msg_header.type = htonl(type);
scr_msg_header.reader_id = htonl(reader_id);
scr_msg_header.length = htonl(length);
-qemu_chr_write(s->cs, (uint8_t *)&scr_msg_header, sizeof(VSCMsgHeader));
-qemu_chr_write(s->cs, payload, length);
+qemu_chr_fe_write(s->cs, (uint8_t *)&scr_msg_header, sizeof(VSCMsgHeader));
+qemu_chr_fe_write(s->cs, payload, length);
}
static void ccid_card_vscard_send_apdu(PassthruState *s,
diff --git a/hw/debugcon.c b/hw/debugcon.c
index 5ee6821..c9ee6d9 100644
--- a/hw/debugcon.c
+++ b/hw/debugcon.c
@@ -51,7 +51,7 @@ static void debugcon_ioport_write(void *opaque, uint32_t
addr, uint32_t val)
printf("debugcon: write addr=0x%04x val=0x%02x\n", addr, val);
#endif
-qemu_chr_write(s->chr, &ch, 1);
+qemu_chr_fe_write(s->chr, &ch, 1);
}
diff --git a/hw/escc.c b/hw/escc.c
index f6fd919..c1460b7 100644
--- a/hw/escc.c
+++ b/hw/escc.c
@@ -578,7 +578,7 @@ static void escc_mem_writeb(void *opaque,
target_phys_addr_t addr, uint32_t val)
s->tx = val;
if (s->wregs[W_TXCTRL2] & TXCTRL2_TXEN) { // tx enabled
if (s->chr)
-qemu_chr_write(s->chr, &s->tx, 1);
+qemu_chr_fe_write(s->chr, &s->tx, 1);
else if (s->type == kbd && !s->disabled) {
handle_kbd_command(s, val);
}
diff --git a/hw/etraxfs_ser.c b/hw/etraxfs_ser.c
index b917d4d..35f8325 100644
--- a/hw/etraxfs_ser.c
+++ b/hw/etraxfs_ser.c
@@ -118,7 +118,7 @@ ser_writel (void *opaque, target_phys_addr_t addr, uint32_t
value)
switch (addr)
{
case RW_DOUT:
-qemu_chr_write(s->chr, &ch, 1);
+qemu_chr_fe_write(s->chr, &ch, 1);
s->regs[R_INTR] |= 3;
s->pending_tx = 1;
s->regs[addr] = value;
diff --git a/hw/grlib_apbuart.c b/hw/grlib_apbuart.c
index 169a56e..c90b810 100644
--- a/hw/grlib_apbuart.c
+++ b/hw/grlib_apbuart.c
@@ -114,7 +114,7 @@ grlib_apbuart_writel(void *opaque, target_phys_addr_t addr,
uint32_t value)
switch (addr) {
case DATA_OFFSET:
c = value & 0xFF;
-qemu_chr_write(uart->chr, &c, 1);
+qemu_chr_fe_write(uart->chr, &c, 1);
return;
case STATUS_OFFSET:
diff --git a/hw/lm32_juart.c b/hw/lm32_juart.c
index fddcf7e..5454aa4 100644
--- a/hw/lm32_juart.c
+++ b/hw/lm32_juart.c
@@ -72,7 +72,7 @@ void lm32_juart_set_jtx(DeviceState *d, uint32_t jtx)
s->jtx = jtx;
if (s->chr) {
-qemu_chr_write(s->chr, &ch, 1);
+qemu_chr_fe_write(s->chr, &ch, 1);
}
}
diff --git a/hw/lm32_uart.c b/hw/lm32_uart.c
index 09090e9..3678545 100644
--- a/hw/lm32_uart.c
+++ b/hw/lm32_uart.c
@@ -169,7 +169,7 @@ static void uart_write(void *opaque, target_phys_addr_t
addr, uint32_t value)
switch (addr) {
case R_RXTX:
if (s->chr) {
-qemu_chr_write(s->chr, &ch, 1);
+qemu_chr_fe_write(s->chr, &ch, 1);
}
break;
case R_IER:
diff --git a/hw/mcf_uart.c b/hw/mcf_uart.c
index
[Qemu-devel] [PATCH 00/12][RFC] char: add flow control and fix guest_[open|close]
The char layer has been growing some nasty warts for some time now as we ask it to do things it was never intended on doing. It's been long over due for an overhaul and its become evident to me that we need to address this first before adding any more features to the char layer. This series is the start at sanitizing the char layer. It effectively turns the char layer into an internal pipe. It supports flow control using an intermediate ring queue for each direction. This series is an RFC because I don't think we should merge the series until we completely convert the old style flow control users to the new style. One particularly nasty area is the mux device. I'm not entirely sure yet how to preceed there.
Re: [Qemu-devel] [RFC 16/24] qcow2: add qcow2_drop_leaked_clusters()
Am 29.07.2011 06:49, schrieb Devin Nakamura: > Signed-off-by: Devin Nakamura > --- > block/qcow2-refcount.c | 34 ++ > block/qcow2.h |2 ++ > 2 files changed, 36 insertions(+), 0 deletions(-) Just for the record, discussed on IRC: Maybe it would be better to leave this functionality inside the normal qcow2_check_refcounts() implementation which would get a new flag that says "fix leaked clusters". This would allow a trivial implementation of a qemu-img check --fix, which is something that we've been wanting for a while. Kevin
[Qemu-devel] KVM call agenda for August 2
Hi Please send in any agenda items you are interested in covering. Later, Juan.
Re: [Qemu-devel] [PATCH] Correctly assign PCI domain numbers
On Mon, Aug 01, 2011 at 11:33:37PM +1000, David Gibson wrote: > On Mon, Aug 01, 2011 at 01:10:38PM +0300, Michael S. Tsirkin wrote: > > On Mon, Aug 01, 2011 at 04:51:02PM +1000, David Gibson wrote: > > > qemu already almost supports PCI domains; that is, several entirely > > > independent PCI host bridges on the same machine. However, a bug in > > > pci_bus_new_inplace() means that every host bridge gets assigned domain > > > number zero and so can't be properly distinguished. This patch fixes the > > > bug, giving each new host bridge a new domain number. > > > > > > Signed-off-by: David Gibson > > > > OK, but I'd like to see the whole picture. > > How does the guest detect multiple domains, > > and how does it access them? > > For the pseries machine, which is what I'm concerned with, each host > bridge is advertised through the device tree passed to the guest. Could you explain please? What generates the device tree and passes it to the guest? > That gives the necessary handles and addresses for accesing config > space and memory and IO windows for each host bridge. I see. I think maybe a global counter in the common code is not exactly the best solution in the general case. > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au| minimalist, thank you. NOT _the_ > _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [RFC PATCH 0/4] Fix subsection ambiguity in the migration format
On 08/01/2011 02:54 AM, Christoph Hellwig wrote: On Sun, Jul 31, 2011 at 07:15:21PM -0500, Anthony Liguori wrote: I think we've set the bar too low historically for introducing new interfaces. I think Avi's new memory API is a good example of how we should approach these things--do the vast majority of the thankless work up front before initial merge. Yes, that seems to work a bit better. So how will we sort out and finalized the vmstate bits, http://wiki.qemu.org/Features/Migration/Next Is what I think we need to do next for migration. In terms of VMState, I think we should can leave it in the current state its in for now. If there is a desire to keep converting devices, that would be fine. Because I think the next thing to do in terms of changing device serialization is to make serialization a proper virtual method of the base object class. I think devices that use composition should also serialize their children as part of their serialization. I think that falls under the banner of updating the object model. QMP, and making sure we have one sort of error reporting? I've updated the QMP merge plan on the wiki: http://wiki.qemu.org/Features/QAPI#Merge_Plan We've merged phase one, and phase two shouldn't be that hard to merge as the code is already written. It's just a matter of rebasing and incorporating in an incremental fashion. Phase two eliminates qerror_report() in favor of passing Error **s. It's very invasive which is why we decided to merge in two phases. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] Correctly assign PCI domain numbers
On Mon, Aug 01, 2011 at 05:31:06PM +0900, Isaku Yamahata wrote: > [Added mst to Cc.] > > In order to use multi PCI domain, several areas need to be addressed > in addition to this patch. For example, bios, acpi dsdt. For x86, yes. For powerpc, which is what I'm working on, no. > Do you have any plan for addressing those area? No. AFAICT this won't make anything less working than it is now, and is sufficient to be useful for the pseries machine. > What's your motivation for multi pci domain? Multiple PCI host bridges is typical on IBM pSeries (powerpc) machines. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH] Correctly assign PCI domain numbers
On Mon, Aug 01, 2011 at 01:10:38PM +0300, Michael S. Tsirkin wrote: > On Mon, Aug 01, 2011 at 04:51:02PM +1000, David Gibson wrote: > > qemu already almost supports PCI domains; that is, several entirely > > independent PCI host bridges on the same machine. However, a bug in > > pci_bus_new_inplace() means that every host bridge gets assigned domain > > number zero and so can't be properly distinguished. This patch fixes the > > bug, giving each new host bridge a new domain number. > > > > Signed-off-by: David Gibson > > OK, but I'd like to see the whole picture. > How does the guest detect multiple domains, > and how does it access them? For the pseries machine, which is what I'm concerned with, each host bridge is advertised through the device tree passed to the guest. That gives the necessary handles and addresses for accesing config space and memory and IO windows for each host bridge. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [SeaBIOS] SeaBIOS error with Juniper FreeBSD kernel
Brandon Bennett writes:
>> Are all versions based on FreeBSD 4.11?
>> Are newer versions still affected?
>
> Newer versions should be based on 6.1 but there are a lot of changes.
> I haven't had a chance to test with something newer yet.
Sorry for the delay due to vacation...
I just confirmed the issue running
"JUNOS 11.1R3.5 built 2011-06-25 00:17:21 UTC"
which is as new as it officially gets at the moment. I don't think the
FreeBSD version really matters. I believe the issue is related to the
"platform_early_bootinit", whatever that is, which is Juniper specific
code. I assume they are checking the SMBIOS for some data which is
present on the real hardware, and this check triggers the crash.
>From the 11.1R3.5 boot:
Hit [Enter] to boot immediately, or space bar for command prompt.
Booting [/kernel]...
platform_early_bootinit: M/T Series Early Boot Initialization
kernel trap 12 with interrupts disabled
Fatal trap 12: page fault while in kernel mode
fault virtual address = 0xbff3fffc
fault code = supervisor write, page not present
instruction pointer = 0x20:0xc0a22377
stac
Fatal trap 30: reserved (unknown) fault while in kernel mode
instruction pointer = 0x20:0xc09f4e66
stack pointer = 0x28:0xc1021a00
frame pointer = 0x28:0xc1021a10
code segment= base 0x0, limit 0xf, type 0x1b
= DPL 0, pres 1, def32 1, gran 1
processor eflags= interrupt enabled, IOPL = 0
current process = 0 ()
trap number = 30
dog: ERROR - reset of uninitialized watchdog
panic: reserved (unknown) fault
(null)(c0c4f4a0,c0c4f4a0,c0be3672,c1021948,a) at 0xc0a14867
(null)(c0be3672,1e,c10219c0,1,1) at 0xc05a723e
(null)(a,0,a,7fff,) at 0xc0a289dd
(null)(c10219c0) at 0xc0a29921
(null)(0,c021b4b,303,c0c22a00,c0c54400) at 0xc0a15e7b
(null)(c0c22a00,63,5,c1021bac,63) at 0xc09f50c2
(null)(63,10,100,1,c0bbe530) at 0xc05e66f3
(null)(63,c1021bac,c0be340e,3f8,1002580) at 0xc05c7ca5
(null)(c0bbe52c,c05c7c45,c1021bac,a,c1021bcc) at 0xc05c7df5
(null)(c0bbe52c,28,c1021cc4,c0be33fe,a) at 0xc05c8f73
(null)(c0c4d0a0,c1021c84,c1021c38,c05c8f73,c0bbe604) at 0xc0a287da
(null)(c0bbe604,c,0,0,0) at 0xc0a28c2a
(null)(c1021c84) at 0xc0a296e8
(null)(cef0,ef0,4,c00e,c00e) at 0xc0a15e7b
(null)(0,c0bc71c6,4,10,0) at 0xc0a0f7df
(null)(c0be5837,c1021d34,c1021d30,a,c1021d54) at 0xc0a0f8ed
(null)(c0bc1e40,c0b6f924,c1021d84,c0a1e5a9,80) at 0xc0b3a694
(null)(80,c0a15ec0,f,3,20) at 0xc0b3ad7b
(null)(1026000) at 0xc0a1e5a9
(null)() at 0xc04aa67d
dog: ERROR - reset of uninitialized watchdog
dog: ERROR - reset of uninitialized watchdog
Uptime: 1s
Also confirmed that 11.1R3.5 is working with SeaBIOS modified as
follows:
diff --git a/src/smbios.c b/src/smbios.c
index 8df0f2d..c96deb5 100644
--- a/src/smbios.c
+++ b/src/smbios.c
@@ -17,7 +17,7 @@ smbios_entry_point_init(u16 max_structure_size,
u16 number_of_structures)
{
struct smbios_entry_point *ep = malloc_fseg(sizeof(*ep));
-void *finaltable = malloc_high(structure_table_length);
+void *finaltable = malloc_fseg(structure_table_length);
if (!ep || !finaltable) {
warn_noalloc();
free(ep);
Bjørn
Re: [Qemu-devel] [RFC 07/24] qed: make qed_alloc_clusters round up offset to nearest cluster
Am 29.07.2011 06:49, schrieb Devin Nakamura:
> Signed-off-by: Devin Nakamura
> ---
> block/qed.c |3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/block/qed.c b/block/qed.c
> index 3970379..00cf895 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -263,6 +263,9 @@ static int qed_read_string(BlockDriverState *file,
> uint64_t offset, size_t n,
> */
> static uint64_t qed_alloc_clusters(BDRVQEDState *s, unsigned int n)
> {
> +s->file_size = (s->file_size + s->header.cluster_size -1)
> +/ s->header.cluster_size;
> +s->file_size *= s->header.cluster_size;
This looks a bit simpler:
s->file_size = qed_start_of_cluster(s->file_size +
s->header.cluster_size - 1);
Kevin
Re: [Qemu-devel] SeaBIOS image is lacking CONFIG_AHCI
On 07/25/11 09:11, Jan Kiszka wrote: On 2011-05-09 08:03, Jan Kiszka wrote: Hi Anthony, please rebuild SeaBIOS after enabling AHCI. QEMU's current version is still not able to boot from such controllers. This unfortunately still applies to 0.15-rc0. Please fix. Updating to latest seabios would be great too as the seabios ahci code got bootorder support recently. cheers, Gerd
Re: [Qemu-devel] [RFC 03/24] block: add bdrv_open_conversion_target()
Am 29.07.2011 06:49, schrieb Devin Nakamura:
> Conflicts:
>
> block.h
You can probably remove this note. ;-)
>
> Signed-off-by: Devin Nakamura
> ---
> block.c | 32
> block.h |4
> 2 files changed, 36 insertions(+), 0 deletions(-)
>
> diff --git a/block.c b/block.c
> index 4503b7b..9530577 100644
> --- a/block.c
> +++ b/block.c
> @@ -3038,6 +3038,38 @@ out:
> return ret;
> }
>
> +int bdrv_open_conversion_target(BlockDriverState **bs, BlockDriverState
> *file,
> +BlockConversionOptions *drv_options,
> +QEMUOptionParameter *usr_options,
> +const char *target_fmt)
> +{
> +BlockDriver *drv;
> +BlockDriverState *bss;
> +
> +drv = bdrv_find_format(target_fmt);
> +if (!drv) {
> +return -ENOENT;
> +}
> +
> +if (!drv->bdrv_open_conversion_target) {
> +return -ENOTSUP;
> +}
> +
> +*bs = bdrv_new("");
> +bss = *bs;
> +bss->file = file;
> +bss->total_sectors = drv_options->image_size >> BDRV_SECTOR_BITS;
> +bss->encrypted = 0;
> +bss->valid_key = 0;
> +bss->open_flags = 0;
> +/* buffer_alignment defaulted to 512, drivers can change this value */
> +bss->buffer_alignment = 512;
> +bss->opaque = qemu_mallocz(drv->instance_size);
> +bss->drv = drv;
> +return drv->bdrv_open_conversion_target(bss, drv_options, usr_options);
> +
> +}
How big are the differences really to bdrv_open_common? Have you checked
if you could reuse it? It looks to me as if you're just handling fewer
cases and could achieve the same by passing the right flags to
bdrv_open_common. The only real difference is drv->bdrv_open vs.
drv->bdrv_open_conversion_target, which could probably be handled by
another flag.
The problem with keeping it separate is that it makes it easy to change
bdrv_open without changing bdrv_open_conversion_target. Most people
touching the code won't use in-place conversion very often, so they
won't notice any breakage in their testing.
Kevin
