[Qemu-devel] [PATCH] fix spelling error
Signed-off-by: Robert Wang wdon...@linux.vnet.ibm.com --- block/sheepdog.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 57b6e1a..ab04932 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -66,7 +66,7 @@ * 20 - 31 (12 bits): reserved data object space * 32 - 55 (24 bits): vdi object space * 56 - 59 ( 4 bits): reserved vdi object space - * 60 - 63 ( 4 bits): object type indentifier space + * 60 - 63 ( 4 bits): object type identifier space */ #define VDI_SPACE_SHIFT 32 -- 1.7.4.1
[Qemu-devel] [PATCH] qemu-coroutine: Add simple work queue support
Add a function co_queue_yield_to_next() which will immediately transfer control to the coroutine at the head of a co queue. This can be used for implementing simple work queues where the manager of a co-queue only needs to restart queued routines one at a time. Signed-off-by: Peter A. G. Crosthwaite peter.crosthwa...@petalogix.com --- qemu-coroutine-lock.c | 13 + qemu-coroutine.h |9 + 2 files changed, 22 insertions(+), 0 deletions(-) diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c index a80f437..de2fc21 100644 --- a/qemu-coroutine-lock.c +++ b/qemu-coroutine-lock.c @@ -75,6 +75,19 @@ bool qemu_co_queue_next(CoQueue *queue) return (next != NULL); } +bool qemu_co_queue_yield_to_next(CoQueue *queue) +{ +Coroutine *next; + +next = QTAILQ_FIRST(queue-entries); +if (next) { +QTAILQ_REMOVE(queue-entries, next, co_queue_next); +qemu_coroutine_enter(next, NULL); +} + +return (next != NULL); +} + bool qemu_co_queue_empty(CoQueue *queue) { return (QTAILQ_FIRST(queue-entries) == NULL); diff --git a/qemu-coroutine.h b/qemu-coroutine.h index 2f2fd95..dbc3610 100644 --- a/qemu-coroutine.h +++ b/qemu-coroutine.h @@ -125,6 +125,15 @@ void coroutine_fn qemu_co_queue_wait(CoQueue *queue); bool qemu_co_queue_next(CoQueue *queue); /** + * Transfers control to the next coroutine in the CoQueue and removes it from + * the queue. + * + * Returns true once after control transfers back to caller, or false + * immediately if the queue is empty. + */ +bool qemu_co_queue_yield_to_next(CoQueue *queue) + +/** * Checks if the CoQueue is empty. */ bool qemu_co_queue_empty(CoQueue *queue); -- 1.7.3.2
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around Qemu to test kernels
On 08/24/2011 01:16 AM, Alexander Graf wrote: On LinuxCon I had a nice chat with Linus on what he thinks kvm-tool would be doing and what he expects from it. Basically he wants a small and simple tool he and other developers can run to try out and see if the kernel they just built actually works. Fortunately, Qemu can do that today already! The only piece that was missing was the simple piece of the equation, so here is a script that wraps around Qemu and executes a kernel you just built. If you do have KVM around and are not cross-compiling, it will use KVM. But if you don't, you can still fall back to emulation mode and at least check if your kernel still does what you expect. I only implemented support for s390x and ppc there, but it's easily extensible to more platforms, as Qemu can emulate (and virtualize) pretty much any platform out there. If you don't have qemu installed, please do so before using this script. Your distro should provide a package for it (might even call it kvm). If not, just compile it from source - it's not hard! To quickly get going, just execute the following as user: $ ./Documentation/run-qemu.sh -r / -a init=/bin/bash This will drop you into a shell on your rootfs. Happy hacking! + +function has_config() { + grep CONFIG_$1=y .config +} grep -q ? + case $1 in + -a|--append) + KERNEL_APPEND2=$2 Might want to append to KERNEL_APPEND2, so you could have multiple -a args. +echo + # Linux Qemu launcher # + +This script executes your currently built Linux kernel using Qemu. If KVM is +available, it will also use KVM for fast virtualization of your guest. + +The intent is to make it very easy to run your kernel. If you need to do more +advanced things, such as passing through real devices, please take the command +line shown below and modify it to your needs. This tool is for simplicity, not +world dominating functionality coverage. Device assignment could be useful for driver developers, yes. + +echo \ +Your guest is bound to the current foreground shell. To quit the guest, +please use Ctrl-A x +echo Executing: $QEMU_BIN $QEMU_OPTIONS -append \$KERNEL_APPEND\ -smp $SMP +echo + +exec $QEMU_BIN $QEMU_OPTIONS -append $KERNEL_APPEND -smp $SMP Would be nice to support launching gdb in a separate terminal with vmlinux already loaded, and already attached to qemu. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] kvm PCI assignment VFIO ramblings
On Tue, Aug 23, 2011 at 03:30:06PM -0400, Alex Williamson wrote: On Tue, 2011-08-23 at 07:01 +1000, Benjamin Herrenschmidt wrote: Could be tho in what form ? returning sysfs pathes ? I'm at a loss there, please suggest. I think we need an ioctl that returns some kind of array of devices within the group and another that maybe takes an index from that array and returns an fd for that device. A sysfs path string might be a reasonable array element, but it sounds like a pain to work with. Limiting to PCI we can just pass the BDF as the argument to optain the device-fd. For a more generic solution we need a unique identifier in some way which is unique across all 'struct device' instances in the system. As far as I know we don't have that yet (besides the sysfs-path) so we either add that or stick with bus-specific solutions. 1:1 process has the advantage of linking to an -mm which makes the whole mmu notifier business doable. How do you want to track down mappings and do the second level translation in the case of explicit map/unmap (like on power) if you are not tied to an mm_struct ? Right, I threw away the mmu notifier code that was originally part of vfio because we can't do anything useful with it yet on x86. I definitely don't want to prevent it where it makes sense though. Maybe we just record current-mm on open and restrict subsequent opens to the same. Hmm, I think we need io-page-fault support in the iommu-api then. Another aspect I don't see discussed is how we represent these things to the guest. On Power for example, I have a requirement that a given iommu domain is represented by a single dma window property in the device-tree. What that means is that that property needs to be either in the node of the device itself if there's only one device in the group or in a parent node (ie a bridge or host bridge) if there are multiple devices. Now I do -not- want to go down the path of simulating P2P bridges, besides we'll quickly run out of bus numbers if we go there. For us the most simple and logical approach (which is also what pHyp uses and what Linux handles well) is really to expose a given PCI host bridge per group to the guest. Believe it or not, it makes things easier :-) I'm all for easier. Why does exposing the bridge use less bus numbers than emulating a bridge? On x86, I want to maintain that our default assignment is at the device level. A user should be able to pick single or multiple devices from across several groups and have them all show up as individual, hotpluggable devices on bus 0 in the guest. Not surprisingly, we've also seen cases where users try to attach a bridge to the guest, assuming they'll get all the devices below the bridge, so I'd be in favor of making this just work if possible too, though we may have to prevent hotplug of those. A side-note: Might it be better to expose assigned devices in a guest on a seperate bus? This will make it easier to emulate an IOMMU for the guest inside qemu. Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Re: [Qemu-devel] kvm PCI assignment VFIO ramblings
On Tue, Aug 23, 2011 at 01:08:29PM -0400, Alex Williamson wrote: On Tue, 2011-08-23 at 15:14 +0200, Roedel, Joerg wrote: Handling it through fds is a good idea. This makes sure that everything belongs to one process. I am not really sure yet if we go the way to just bind plain groups together or if we create meta-groups. The meta-groups thing seems somewhat cleaner, though. I'm leaning towards binding because we need to make it dynamic, but I don't really have a good picture of the lifecycle of a meta-group. In my view the life-cycle of the meta-group is a subrange of the qemu-instance's life-cycle. Putting the process to sleep (which would be uninterruptible) seems bad. The process would sleep until the guest releases the device-group, which can take days or months. The best thing (and the most intrusive :-) ) is to change PCI core to allow unbindings to fail, I think. But this probably further complicates the way to upstream VFIO... Yes, it's not ideal but I think it's sufficient for now and if we later get support for returning an error from release, we can set a timeout after notifying the user to make use of that. Thanks, Ben had the idea of just forcing to hard-unplug this device from the guest. Thats probably the best way to deal with that, I think. VFIO sends a notification to qemu that the device is gone and qemu informs the guest in some way about it. Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Re: [Qemu-devel] kvm PCI assignment VFIO ramblings
On Tue, Aug 23, 2011 at 07:35:37PM -0400, Benjamin Herrenschmidt wrote: On Tue, 2011-08-23 at 15:18 +0200, Roedel, Joerg wrote: Hmm, good idea. But as far as I know the hotplug-event needs to be in the guest _before_ the device is actually unplugged (so that the guest can unbind its driver first). That somehow brings back the sleep-idea and the timeout in the .release function. That's for normal assisted hotplug, but don't we support hard hotplug ? I mean, things like cardbus, thunderbolt (if we ever support that) etc... will need it and some platforms do support hard hotplug of PCIe devices. (That's why drivers should never spin on MMIO waiting for a 1 bit to clear without a timeout :-) Right, thats probably the best semantic for this issue then. The worst thing that happens is that the admin crashed the guest. Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Re: [Qemu-devel] kvm PCI assignment VFIO ramblings
On Tue, Aug 23, 2011 at 01:33:14PM -0400, Aaron Fabbri wrote: On 8/23/11 10:01 AM, Alex Williamson alex.william...@redhat.com wrote: The iommu domain would probably be allocated when the first device is bound to vfio. As each device is bound, it gets attached to the group. DMAs are done via an ioctl on the group. I think group + uiommu leads to effectively reliving most of the problems with the current code. The only benefit is the group assignment to enforce hardware restrictions. We still have the problem that uiommu open() = iommu_domain_alloc(), whose properties are meaningless without attached devices (groups). Which I think leads to the same awkward model of attaching groups to define the domain, then we end up doing mappings via the group to enforce ordering. Is there a better way to allow groups to share an IOMMU domain? Maybe, instead of having an ioctl to allow a group A to inherit the same iommu domain as group B, we could have an ioctl to fully merge two groups (could be what Ben was thinking): A.ioctl(MERGE_TO_GROUP, B) The group A now goes away and its devices join group B. If A ever had an iommu domain assigned (and buffers mapped?) we fail. Groups cannot get smaller (they are defined as minimum granularity of an IOMMU, initially). They can get bigger if you want to share IOMMU resources, though. Any downsides to this approach? As long as this is a 2-way road its fine. There must be a way to split the groups again after the guest exits. But then we are again at the super-groups (aka meta-groups, aka uiommu) point. Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Re: [Qemu-devel] [PULL] slirp: Fix issues with -mms-bitfields
On 2011-08-23 12:49, TeLeMan wrote: On Sun, Aug 21, 2011 at 04:00, Stefan Weil w...@mail.berlios.de wrote: Am 15.08.2011 08:39, schrieb Jan Kiszka: The following changes since commit 3b6ffe50300f13240e1b46420ad05da1116df410: hw/scsi-bus.c: Fix use of uninitialised variable (2011-08-14 19:34:25 +) are available in the git repository at: git://git.kiszka.org/qemu.git queues/slirp Jan Kiszka (1): slirp: Fix bit field types in IP header structs slirp/ip.h | 8 slirp/tcp.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) --- slirp: Fix bit field types in IP header structs -mms-bitfields prevents that the bitfields in current IP header structs are packed into a single byte as it is required. Fix this by using uint8_t as backing type. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- slirp/ip.h | 8 slirp/tcp.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/slirp/ip.h b/slirp/ip.h index 48ea38e..72dbe9a 100644 --- a/slirp/ip.h +++ b/slirp/ip.h @@ -74,10 +74,10 @@ typedef uint32_t n_long; /* long as received from the net */ */ struct ip { #ifdef HOST_WORDS_BIGENDIAN - u_int ip_v:4, /* version */ + uint8_t ip_v:4, /* version */ ip_hl:4; /* header length */ #else - u_int ip_hl:4, /* header length */ + uint8_t ip_hl:4, /* header length */ ip_v:4; /* version */ #endif uint8_t ip_tos; /* type of service */ @@ -140,10 +140,10 @@ struct ip_timestamp { uint8_t ipt_len; /* size of structure (variable) */ uint8_t ipt_ptr; /* index of current entry */ #ifdef HOST_WORDS_BIGENDIAN - u_int ipt_oflw:4, /* overflow counter */ + uint8_t ipt_oflw:4, /* overflow counter */ ipt_flg:4; /* flags, see below */ #else - u_int ipt_flg:4, /* flags, see below */ + uint8_t ipt_flg:4, /* flags, see below */ ipt_oflw:4; /* overflow counter */ #endif union ipt_timestamp { diff --git a/slirp/tcp.h b/slirp/tcp.h index 9d06836..b3817cb 100644 --- a/slirp/tcp.h +++ b/slirp/tcp.h @@ -51,10 +51,10 @@ struct tcphdr { tcp_seq th_seq; /* sequence number */ tcp_seq th_ack; /* acknowledgement number */ #ifdef HOST_WORDS_BIGENDIAN - u_int th_off:4, /* data offset */ + uint8_t th_off:4, /* data offset */ th_x2:4; /* (unused) */ #else - u_int th_x2:4, /* (unused) */ + uint8_t th_x2:4, /* (unused) */ th_off:4; /* data offset */ #endif uint8_t th_flags; Tested-by: Stefan Weil w...@mail.berlios.de slirp is still broken on my mingw32. I used #progma pack(push,1)/#progma pack(pop) to resolve this issue. Can you drill down to the bottom of this problem? What fields in what struct are not properly packed? Maybe this is now a compiler bug, so comparing versions may make sense as well. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] kvm PCI assignment VFIO ramblings
On Tue, Aug 23, 2011 at 12:54:27PM -0400, aafabbri wrote: On 8/23/11 4:04 AM, Joerg Roedel joerg.roe...@amd.com wrote: That is makes uiommu basically the same as the meta-groups, right? Yes, functionality seems the same, thus my suggestion to keep uiommu explicit. Is there some need for group-groups besides defining sets of groups which share IOMMU resources? I do all this stuff (bringing up sets of devices which may share IOMMU domain) dynamically from C applications. I don't really want some static (boot-time or sysfs fiddling) supergroup config unless there is a good reason KVM/power needs it. As you say in your next email, doing it all from ioctls is very easy, programmatically. I don't see a reason to make this meta-grouping static. It would harm flexibility on x86. I think it makes things easier on power but there are options on that platform to get the dynamic solution too. Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
Re: [Qemu-devel] [PATCH] KVM: Add wrapper script around Qemu to test kernels
On 2011-08-24 10:25, Avi Kivity wrote: On 08/24/2011 01:16 AM, Alexander Graf wrote: + +echo \ +Your guest is bound to the current foreground shell. To quit the guest, +please use Ctrl-A x +echo Executing: $QEMU_BIN $QEMU_OPTIONS -append \$KERNEL_APPEND\ -smp $SMP +echo + +exec $QEMU_BIN $QEMU_OPTIONS -append $KERNEL_APPEND -smp $SMP Would be nice to support launching gdb in a separate terminal with vmlinux already loaded, and already attached to qemu. + loading a python script into gdb to pull in module symbols. There are a few implementations floating around (including my own one). It would also be nice if one could append QEMU (note the capitalization BTW) options to the script, maybe everything after a '--' separator. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 00/16] isa_register_portio_list, v2
On 08/24/2011 03:13 AM, Richard Henderson wrote: The problem that malc saw with sb16 was a major think-o on my part with the whole interface. We can't re-use the const sub-arrays of the original MemoryRegionPortio array because they have the wrong offset for the MemoryRegion to which it is attached -- the lookup in find_portio fails. We must adjust the offsets of the old_portio array to be based against the MemoryRegion. Which means we can easily eliminate the major complaint that came with the previous round of comments -- the double PORTIO_END_OF_LIST and the explicit marking of the ranges. All we require of users is that the array be sorted by offset. The entire patch set is at git://repo.or.cz/qemu/rth.git mem-api-isa and is of course based on git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/master Avi, the first two patches are fixes for compile errors in your tree. They probably ought to be squashed when next you rebase. Sloppy of me... The patchset looks good, I'll leave it on the list for a few days before pulling to allow further review. Once we're done with the conversion, we should look at extending the idea to mmio registers - decode the registers in the core instead of a per-region switch statement. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] [PATCH] pcnet-pci: fix wrong opaque given to I/O accessors
On 08/22/2011 05:16 PM, Gerhard Wiesinger wrote: Hello Avi, Thnx, fixed: OK, maybe some credits :-) Acked-by: Gerhard Wiesinger li...@wiesinger.com This pattern is still present at (maybe some further problems!!!) and I guess it has to be fixed, too: grep -ir 'ops, s, ' . ./hw/rtl8139.c:memory_region_init_io(s-bar_io, rtl8139_io_ops, s, rtl8139, 0x100); ./hw/rtl8139.c:memory_region_init_io(s-bar_mem, rtl8139_mmio_ops, s, rtl8139, 0x100); Usually, when you have memory_region_init_io(s-something, ..., s, ...) it means everything is fine. Lance/pcnet is special in this regard. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] kvm PCI assignment VFIO ramblings
On Wed, Aug 24, 2011 at 11:14:26AM +0200, Roedel, Joerg wrote: On Tue, Aug 23, 2011 at 12:54:27PM -0400, aafabbri wrote: On 8/23/11 4:04 AM, Joerg Roedel joerg.roe...@amd.com wrote: That is makes uiommu basically the same as the meta-groups, right? Yes, functionality seems the same, thus my suggestion to keep uiommu explicit. Is there some need for group-groups besides defining sets of groups which share IOMMU resources? I do all this stuff (bringing up sets of devices which may share IOMMU domain) dynamically from C applications. I don't really want some static (boot-time or sysfs fiddling) supergroup config unless there is a good reason KVM/power needs it. As you say in your next email, doing it all from ioctls is very easy, programmatically. I don't see a reason to make this meta-grouping static. It would harm flexibility on x86. I think it makes things easier on power but there are options on that platform to get the dynamic solution too. I think several people are misreading what Ben means by static. I would prefer to say 'persistent', in that the meta-groups lifetime is not tied to an fd, but they can be freely created, altered and removed during runtime. -- 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 00/16] isa_register_portio_list, v2
On Tue, 23 Aug 2011, Richard Henderson wrote: The problem that malc saw with sb16 was a major think-o on my part with the whole interface. We can't re-use the const sub-arrays of the original MemoryRegionPortio array because they have the wrong offset for the MemoryRegion to which it is attached -- the lookup in find_portio fails. We must adjust the offsets of the old_portio array to be based against the MemoryRegion. Which means we can easily eliminate the major complaint that came with the previous round of comments -- the double PORTIO_END_OF_LIST and the explicit marking of the ranges. All we require of users is that the array be sorted by offset. The entire patch set is at git://repo.or.cz/qemu/rth.git mem-api-isa and is of course based on git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/master Avi, the first two patches are fixes for compile errors in your tree. They probably ought to be squashed when next you rebase. Malc, I tested sb16 vs the xtc-play you provided. However I failed to test the gus changes; the xtc-play program could not recognize the device with --device gus before or after the patch set. Should I be using another set of command-line options for that? I guess xtc can't live without environment variable which tells it where gus is located `set ULTRASND=240,3,3,7,7'. Anyhow i've tested things and both sb16 and gus work, thanks. -- mailto:av1...@comtv.ru
Re: [Qemu-devel] PPC* and Sparc32 crash
On 08/23/2011 08:55 PM, Blue Swirl wrote: qemu-system-ppc: /src/qemu/memory.c:1183: memory_region_add_subregion_common: Assertion `!subregion-parent' failed. Aborted qemu-system-ppc64: /src/qemu/memory.c:1183: memory_region_add_subregion_common: Assertion `!subregion-parent' failed. Aborted qemu-system-sparc: /src/qemu/hw/sysbus.c:156: sysbus_register_withprop: Assertion `info-qdev.size= sizeof(SysBusDevice)' failed. Aborted This is with b861b7419c49ad53e786062b4fbf6da53468f130. Other targets seem to work. Please provide disk images and command line options, and I will investigate. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] [PATCH 1/2] pc: make vgabios exit port more useful
On 08/08/2011 10:31 PM, Anthony Liguori wrote: We've always listened on port 501 for vgabios panic messages. In the entire time I've worked on QEMU, I've never actually seen a vgabios panic message :-) If we change the semantics of this port a little bit, it makes it possible to use it for more interesting use-cases. I chose this approach instead of adding a new I/O port because it avoids having a guest visible change. This change allows single-byte access to port 501 and also uses the value written to construct an exit code. A little late to review but... diff --git a/hw/pc.c b/hw/pc.c index 1c9d89a..4b07b35 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -549,8 +549,7 @@ static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val) /* LGPL'ed VGA BIOS messages */ case 0x501: case 0x502: -fprintf(stderr, VGA BIOS panic, line %d\n, val); -exit(1); +exit((val 1) | 1); This code (before the patch) circumvents -no-shutdown. Shifting val left is surprising. What's wrong with even exit codes? -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions
On Tue, Aug 23, 2011 at 04:59:16PM -0400, Don Dutile wrote: So we want to pollute the dev assignment code with knowledge of this array for bounds checking, which you're threatening to remove? OKay, I'll stop being a mule, and apply that patch. -- MST
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions
On Tue, Aug 23, 2011 at 07:28:08PM +0200, Jan Kiszka wrote: From: Alex Williamson alex.william...@redhat.com Nothing good can happen when we overlap capabilities [ Jan: rebased over qemu, minor formatting ] Signed-off-by: Jan Kiszka jan.kis...@siemens.com This doesn't build for me: /scm/qemu/hw/pci.c: In function ‘pci_add_capability’: /scm/qemu/hw/pci.c:1970:45: error: ‘PCIDevice’ has no member named ‘config_map’ I think that what that includes is the capability including each given offset, right? It would be easy to write some code scanning the capability list to figure this value out. Something along the lines of (untested): static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) { uint8_t next, prev, found = 0; if (!(pdev-config[PCI_STATUS] PCI_STATUS_CAP_LIST)) return 0; for (prev = PCI_CAPABILITY_LIST; (next = pdev-config[prev]); prev = next + PCI_CAP_LIST_NEXT) if (next = offset next found) found = next; return found; } --- hw/pci.c | 14 ++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 6124790..ff20631 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1952,11 +1952,25 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t offset, uint8_t size) { uint8_t *config; +int i; + if (!offset) { offset = pci_find_space(pdev, size); if (!offset) { return -ENOSPC; } +} else { +for (i = offset; i offset + size; i++) { +if (pdev-used[i]) { +fprintf(stderr, ERROR: %04x:%02x:%02x.%x +Attempt to add PCI capability %x at offset +%x overlaps existing capability %x at offset %x\n, +pci_find_domain(pdev-bus), pci_bus_num(pdev-bus), +PCI_SLOT(pdev-devfn), PCI_FUNC(pdev-devfn), +cap_id, offset, pdev-config_map[i], i); +return -EFAULT; +} +} } config = pdev-config + offset; -- 1.7.3.4
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions
On 2011-08-24 12:04, Michael S. Tsirkin wrote: On Tue, Aug 23, 2011 at 07:28:08PM +0200, Jan Kiszka wrote: From: Alex Williamson alex.william...@redhat.com Nothing good can happen when we overlap capabilities [ Jan: rebased over qemu, minor formatting ] Signed-off-by: Jan Kiszka jan.kis...@siemens.com This doesn't build for me: /scm/qemu/hw/pci.c: In function ‘pci_add_capability’: /scm/qemu/hw/pci.c:1970:45: error: ‘PCIDevice’ has no member named ‘config_map’ Yeah, sorry, forgot to refresh the commit before posting. I think that what that includes is the capability including each given offset, right? It would be easy to write some code scanning the capability list to figure this value out. Something along the lines of (untested): static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) { uint8_t next, prev, found = 0; if (!(pdev-config[PCI_STATUS] PCI_STATUS_CAP_LIST)) return 0; for (prev = PCI_CAPABILITY_LIST; (next = pdev-config[prev]); prev = next + PCI_CAP_LIST_NEXT) if (next = offset next found) found = next; return found; } Sounds useful, will enhance the patch. (Originally, I just wanted to reduce the qemu-kvm delta... :) ) Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH 13/24] cirrus: wrap memory update in a transaction
This prevents spurious unmapping and remapping of the vga windows, which reduces performance. Signed-off-by: Avi Kivity a...@redhat.com --- hw/cirrus_vga.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index 4d0ef0d..ec7ea82 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -2424,6 +2424,7 @@ static void cirrus_update_memory_access(CirrusVGAState *s) { unsigned mode; +memory_region_transaction_begin(); if ((s-vga.sr[0x17] 0x44) == 0x44) { goto generic_io; } else if (s-cirrus_srcptr != s-cirrus_srcptr_end) { @@ -2443,6 +2444,7 @@ static void cirrus_update_memory_access(CirrusVGAState *s) unmap_linear_vram(s); } } +memory_region_transaction_commit(); } -- 1.7.5.3
[Qemu-devel] [PATCH 16/24] pflash_cfi01/pflash_cfi02: convert to memory API
cfi02 is annoying in that is ignores some address bits; we probably want explicit support in the memory API for that. Signed-off-by: Avi Kivity a...@redhat.com --- hw/collie.c | 17 --- hw/flash.h| 16 +-- hw/gumstix.c | 29 +++- hw/lm32_boards.c | 15 --- hw/mainstone.c| 21 + hw/milkymist.c|8 ++- hw/mips_malta.c | 44 ++- hw/mips_r4k.c | 19 +--- hw/musicpal.c | 16 --- hw/omap_sx1.c | 22 ++ hw/petalogix_ml605_mmu.c |8 ++- hw/petalogix_s3adsp1800_mmu.c |9 +++- hw/pflash_cfi01.c | 67 ++--- hw/pflash_cfi02.c | 95 - hw/ppc405_boards.c| 61 ++ hw/r2d.c |8 +++- hw/virtex_ml507.c |8 ++- hw/z2.c | 16 --- 18 files changed, 256 insertions(+), 223 deletions(-) diff --git a/hw/collie.c b/hw/collie.c index 9b724aa..6f30d24 100644 --- a/hw/collie.c +++ b/hw/collie.c @@ -5,6 +5,7 @@ * * This code is licensed under GNU GPL v2. */ +#include glib.h #include hw.h #include sysbus.h #include boards.h @@ -28,7 +29,7 @@ static void collie_init(MemoryRegion *address_space_mem, { StrongARMState *s; DriveInfo *dinfo; -ram_addr_t phys_flash; +MemoryRegion *phys_flash = g_new(MemoryRegion, 2); if (!cpu_model) { cpu_model = sa1110; @@ -36,17 +37,19 @@ static void collie_init(MemoryRegion *address_space_mem, s = sa1110_init(collie_binfo.ram_size, cpu_model); -phys_flash = qemu_ram_alloc(NULL, collie.fl1, 0x0200); +memory_region_init_rom_device(phys_flash[0], pflash_cfi01_ops_le, + NULL, collie.fl1, 0x0200); dinfo = drive_get(IF_PFLASH, 0, 0); -pflash_cfi01_register(SA_CS0, phys_flash, +pflash_cfi01_register(SA_CS0, phys_flash[0], dinfo ? dinfo-bdrv : NULL, (64 * 1024), -512, 4, 0x00, 0x00, 0x00, 0x00, 0); +512, 4, 0x00, 0x00, 0x00, 0x00); -phys_flash = qemu_ram_alloc(NULL, collie.fl2, 0x0200); +memory_region_init_rom_device(phys_flash[1], pflash_cfi01_ops_le, + NULL, collie.fl2, 0x0200); dinfo = drive_get(IF_PFLASH, 0, 1); -pflash_cfi01_register(SA_CS1, phys_flash, +pflash_cfi01_register(SA_CS1, phys_flash[1], dinfo ? dinfo-bdrv : NULL, (64 * 1024), -512, 4, 0x00, 0x00, 0x00, 0x00, 0); +512, 4, 0x00, 0x00, 0x00, 0x00); sysbus_create_simple(scoop, 0x4080, NULL); diff --git a/hw/flash.h b/hw/flash.h index 140ae39..7fb012b 100644 --- a/hw/flash.h +++ b/hw/flash.h @@ -1,21 +1,27 @@ +#include memory.h + /* NOR flash devices */ typedef struct pflash_t pflash_t; /* pflash_cfi01.c */ -pflash_t *pflash_cfi01_register(target_phys_addr_t base, ram_addr_t off, +extern const MemoryRegionOps pflash_cfi01_ops_be; +extern const MemoryRegionOps pflash_cfi01_ops_le; +extern const MemoryRegionOps pflash_cfi02_ops_be; +extern const MemoryRegionOps pflash_cfi02_ops_le; + +pflash_t *pflash_cfi01_register(target_phys_addr_t base, MemoryRegion *mem, BlockDriverState *bs, uint32_t sector_len, int nb_blocs, int width, uint16_t id0, uint16_t id1, -uint16_t id2, uint16_t id3, int be); +uint16_t id2, uint16_t id3); /* pflash_cfi02.c */ -pflash_t *pflash_cfi02_register(target_phys_addr_t base, ram_addr_t off, +pflash_t *pflash_cfi02_register(target_phys_addr_t base, MemoryRegion *mem, BlockDriverState *bs, uint32_t sector_len, int nb_blocs, int nb_mappings, int width, uint16_t id0, uint16_t id1, uint16_t id2, uint16_t id3, -uint16_t unlock_addr0, uint16_t unlock_addr1, -int be); +uint16_t unlock_addr0, uint16_t unlock_addr1); /* nand.c */ DeviceState *nand_init(BlockDriverState *bdrv, int manf_id, int chip_id); diff --git a/hw/gumstix.c b/hw/gumstix.c index ccdd834..620bdd7 100644 --- a/hw/gumstix.c +++ b/hw/gumstix.c @@ -31,6 +31,7 @@ * # qemu-system-arm -M verdex -pflash flash -monitor null -nographic -m 289 */ +#include glib.h #include hw.h #include pxa.h #include net.h @@ -50,7 +51,8 @@ static void connex_init(MemoryRegion *address_space_mem, { PXA2xxState *cpu; DriveInfo *dinfo; -int be; +const MemoryRegionOps *flash_ops; +MemoryRegion *flash =
Re: [Qemu-devel] [PATCH 01/24] arm_sysctl: convert to memory API
On 24 August 2011 11:11, Avi Kivity a...@redhat.com wrote: Signed-off-by: Avi Kivity a...@redhat.com --- hw/arm_sysctl.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/arm_sysctl.c b/hw/arm_sysctl.c index 17cf6f7..5b845a2 100644 --- a/hw/arm_sysctl.c +++ b/hw/arm_sysctl.c @@ -19,6 +19,7 @@ typedef struct { SysBusDevice busdev; MemoryRegion iomem; qemu_irq pl110_mux_ctrl; + MemoryRegion iomem; uint32_t sys_id; uint32_t leds; Er, looks like you forgot to squish Richard's compile fix patch? -- PMM
Re: [Qemu-devel] [PATCH 06/24] QEMUMachine: pass address space to machine init function
On 24 August 2011 11:11, Avi Kivity a...@redhat.com wrote: Avoids get_system_memory() everywhere. - machine-init(ram_size, boot_devices, + machine-init(get_system_memory(), get_system_io(), ram_size, boot_devices, kernel_filename, kernel_cmdline, initrd_filename, cpu_model); I think this is conceptually wrong. The system memory/IO address spaces are not configurable properties of the machine model (in the way that the other arguments to machine-init() are), they're purely an internal implementation detail of it. (In fact for many of the machines we support there isn't even any such thing as the system IO space...) -- PMM
[Qemu-devel] [PATCH 18/24] g364fb: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/g364fb.c| 61 +++ hw/mips.h |4 ++- hw/mips_jazz.c |3 +- 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/hw/g364fb.c b/hw/g364fb.c index b3020c5..d081bb4 100644 --- a/hw/g364fb.c +++ b/hw/g364fb.c @@ -17,6 +17,7 @@ * with this program; if not, see http://www.gnu.org/licenses/. */ +#include glib.h #include hw.h #include mips.h #include console.h @@ -36,7 +37,7 @@ do { fprintf(stderr, g364 ERROR: fmt , ## __VA_ARGS__);} while (0) typedef struct G364State { /* hardware */ uint8_t *vram; -ram_addr_t vram_offset; +MemoryRegion vram_region; int vram_size; qemu_irq irq; /* registers */ @@ -68,16 +69,17 @@ typedef struct G364State { #define CTLA_FORCE_BLANK 0x0400 #define CTLA_NO_CURSOR 0x0080 -static inline int check_dirty(ram_addr_t page) +static inline int check_dirty(G364State *s, ram_addr_t page) { -return cpu_physical_memory_get_dirty(page, VGA_DIRTY_FLAG); +return memory_region_get_dirty(s-vram_region, page, DIRTY_MEMORY_VGA); } static inline void reset_dirty(G364State *s, ram_addr_t page_min, ram_addr_t page_max) { -cpu_physical_memory_reset_dirty(page_min, page_max + TARGET_PAGE_SIZE - 1, -VGA_DIRTY_FLAG); +memory_region_reset_dirty(s-vram_region, page_min, + page_max + TARGET_PAGE_SIZE - 1, + DIRTY_MEMORY_VGA); } static void g364fb_draw_graphic8(G364State *s) @@ -114,7 +116,7 @@ static void g364fb_draw_graphic8(G364State *s) return; } -page = s-vram_offset; +page = 0; page_min = (ram_addr_t)-1; page_max = 0; @@ -135,7 +137,7 @@ static void g364fb_draw_graphic8(G364State *s) /* XXX: out of range in vram? */ data_display = dd = ds_get_data(s-ds); while (y s-height) { -if (check_dirty(page)) { +if (check_dirty(s, page)) { if (y ymin) ymin = ymax = y; if (page_min == (ram_addr_t)-1) @@ -275,7 +277,7 @@ static inline void g364fb_invalidate_display(void *opaque) s-blanked = 0; for (i = 0; i s-vram_size; i += TARGET_PAGE_SIZE) { -cpu_physical_memory_set_dirty(s-vram_offset + i); +memory_region_set_dirty(s-vram_region, i); } } @@ -411,7 +413,7 @@ static void g364_invalidate_cursor_position(G364State *s) end = (ymax + 1) * ds_get_linesize(s-ds); for (i = start; i end; i += TARGET_PAGE_SIZE) { -cpu_physical_memory_set_dirty(s-vram_offset + i); +memory_region_set_dirty(s-vram_region, i); } } @@ -522,16 +524,20 @@ static void g364fb_ctrl_writeb(void *opaque, target_phys_addr_t addr, uint32_t v g364fb_ctrl_writel(opaque, addr ~0x3, val); } -static CPUReadMemoryFunc * const g364fb_ctrl_read[3] = { -g364fb_ctrl_readb, -g364fb_ctrl_readw, -g364fb_ctrl_readl, -}; - -static CPUWriteMemoryFunc * const g364fb_ctrl_write[3] = { -g364fb_ctrl_writeb, -g364fb_ctrl_writew, -g364fb_ctrl_writel, +static const MemoryRegionOps g364fb_ctrl_ops = { +.old_mmio = { +.read = { +g364fb_ctrl_readb, +g364fb_ctrl_readw, +g364fb_ctrl_readl, +}, +.write = { +g364fb_ctrl_writeb, +g364fb_ctrl_writew, +g364fb_ctrl_writel, +}, +}, +.endianness = DEVICE_NATIVE_ENDIAN, }; static int g364fb_load(QEMUFile *f, void *opaque, int version_id) @@ -583,18 +589,19 @@ static void g364fb_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s-height); } -int g364fb_mm_init(target_phys_addr_t vram_base, +int g364fb_mm_init(MemoryRegion *system_memory, + target_phys_addr_t vram_base, target_phys_addr_t ctrl_base, int it_shift, qemu_irq irq) { G364State *s; -int io_ctrl; +MemoryRegion *io_ctrl = g_new(MemoryRegion, 1); s = g_malloc0(sizeof(G364State)); s-vram_size = 8 * 1024 * 1024; -s-vram_offset = qemu_ram_alloc(NULL, g364fb.vram, s-vram_size); -s-vram = qemu_get_ram_ptr(s-vram_offset); +memory_region_init_ram(s-vram_region, NULL, g364fb.vram, s-vram_size); +s-vram = memory_region_get_ram_ptr(s-vram_region); s-irq = irq; qemu_register_reset(g364fb_reset, s); @@ -605,11 +612,11 @@ int g364fb_mm_init(target_phys_addr_t vram_base, g364fb_invalidate_display, g364fb_screen_dump, NULL, s); -cpu_register_physical_memory(vram_base, s-vram_size, s-vram_offset); +memory_region_add_subregion(system_memory, vram_base, s-vram_region); -io_ctrl = cpu_register_io_memory(g364fb_ctrl_read, g364fb_ctrl_write, s, - DEVICE_NATIVE_ENDIAN); -
[Qemu-devel] [PATCH 21/24] mcf5208: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/mcf5208.c | 71 + 1 files changed, 31 insertions(+), 40 deletions(-) diff --git a/hw/mcf5208.c b/hw/mcf5208.c index f4d4fbd..73c6961 100644 --- a/hw/mcf5208.c +++ b/hw/mcf5208.c @@ -5,6 +5,7 @@ * * This code is licensed under the GPL */ +#include glib.h #include hw.h #include mcf.h #include qemu-timer.h @@ -27,6 +28,7 @@ #define PCSR_PRE_MASK 0x0f00 typedef struct { +MemoryRegion iomem; qemu_irq irq; ptimer_state *timer; uint16_t pcsr; @@ -43,7 +45,7 @@ static void m5208_timer_update(m5208_timer_state *s) } static void m5208_timer_write(void *opaque, target_phys_addr_t offset, - uint32_t value) + uint64_t value, unsigned size) { m5208_timer_state *s = (m5208_timer_state *)opaque; int prescale; @@ -104,7 +106,8 @@ static void m5208_timer_trigger(void *opaque) m5208_timer_update(s); } -static uint32_t m5208_timer_read(void *opaque, target_phys_addr_t addr) +static uint64_t m5208_timer_read(void *opaque, target_phys_addr_t addr, + unsigned size) { m5208_timer_state *s = (m5208_timer_state *)opaque; switch (addr) { @@ -120,19 +123,14 @@ static uint32_t m5208_timer_read(void *opaque, target_phys_addr_t addr) } } -static CPUReadMemoryFunc * const m5208_timer_readfn[] = { - m5208_timer_read, - m5208_timer_read, - m5208_timer_read +static const MemoryRegionOps m5208_timer_ops = { +.read = m5208_timer_read, +.write = m5208_timer_write, +.endianness = DEVICE_NATIVE_ENDIAN, }; -static CPUWriteMemoryFunc * const m5208_timer_writefn[] = { - m5208_timer_write, - m5208_timer_write, - m5208_timer_write -}; - -static uint32_t m5208_sys_read(void *opaque, target_phys_addr_t addr) +static uint64_t m5208_sys_read(void *opaque, target_phys_addr_t addr, + unsigned size) { switch (addr) { case 0x110: /* SDCS0 */ @@ -154,45 +152,36 @@ static uint32_t m5208_sys_read(void *opaque, target_phys_addr_t addr) } static void m5208_sys_write(void *opaque, target_phys_addr_t addr, -uint32_t value) +uint64_t value, unsigned size) { hw_error(m5208_sys_write: Bad offset 0x%x\n, (int)addr); } -static CPUReadMemoryFunc * const m5208_sys_readfn[] = { - m5208_sys_read, - m5208_sys_read, - m5208_sys_read -}; - -static CPUWriteMemoryFunc * const m5208_sys_writefn[] = { - m5208_sys_write, - m5208_sys_write, - m5208_sys_write +static const MemoryRegionOps m5208_sys_ops = { +.read = m5208_sys_read, +.write = m5208_sys_write, +.endianness = DEVICE_NATIVE_ENDIAN, }; -static void mcf5208_sys_init(qemu_irq *pic) +static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic) { -int iomemtype; +MemoryRegion *iomem = g_new(MemoryRegion, 1); m5208_timer_state *s; QEMUBH *bh; int i; -iomemtype = cpu_register_io_memory(m5208_sys_readfn, - m5208_sys_writefn, NULL, - DEVICE_NATIVE_ENDIAN); /* SDRAMC. */ -cpu_register_physical_memory(0xfc0a8000, 0x4000, iomemtype); +memory_region_init_io(iomem, m5208_sys_ops, NULL, m5208-sys, 0x4000); +memory_region_add_subregion(address_space, 0xfc0a8000, iomem); /* Timers. */ for (i = 0; i 2; i++) { s = (m5208_timer_state *)g_malloc0(sizeof(m5208_timer_state)); bh = qemu_bh_new(m5208_timer_trigger, s); s-timer = ptimer_init(bh); -iomemtype = cpu_register_io_memory(m5208_timer_readfn, - m5208_timer_writefn, s, - DEVICE_NATIVE_ENDIAN); -cpu_register_physical_memory(0xfc08 + 0x4000 * i, 0x4000, - iomemtype); +memory_region_init_io(s-iomem, m5208_timer_ops, s, + m5208-timer, 0x4000); +memory_region_add_subregion(address_space, 0xfc08 + 0x4000 * i, +s-iomem); s-irq = pic[4 + i]; } } @@ -209,6 +198,8 @@ static void mcf5208evb_init(MemoryRegion *address_space_mem, uint64_t elf_entry; target_phys_addr_t entry; qemu_irq *pic; +MemoryRegion *ram = g_new(MemoryRegion, 1); +MemoryRegion *sram = g_new(MemoryRegion, 1); if (!cpu_model) cpu_model = m5208; @@ -223,12 +214,12 @@ static void mcf5208evb_init(MemoryRegion *address_space_mem, /* TODO: Configure BARs. */ /* DRAM at 0x4000 */ -cpu_register_physical_memory(0x4000, ram_size, -qemu_ram_alloc(NULL, mcf5208.ram, ram_size) | IO_MEM_RAM); +memory_region_init_ram(ram, NULL, mcf5208.ram, ram_size); +memory_region_add_subregion(address_space_mem, 0x4000,
Re: [Qemu-devel] [PATCH 21/24] mcf5208: convert to memory API
On 24 August 2011 11:11, Avi Kivity a...@redhat.com wrote: diff --git a/hw/mcf5208.c b/hw/mcf5208.c index f4d4fbd..73c6961 100644 --- a/hw/mcf5208.c +++ b/hw/mcf5208.c @@ -5,6 +5,7 @@ * * This code is licensed under the GPL */ +#include glib.h #include hw.h #include mcf.h #include qemu-timer.h You shouldn't need to include glib.h here, I think? hw.h includes qemu-common.h which includes glib.h. -- PMM
[Qemu-devel] [PATCH 20/24] mainstone: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/mainstone.c | 12 +++- 1 files changed, 7 insertions(+), 5 deletions(-) diff --git a/hw/mainstone.c b/hw/mainstone.c index cb62abb..ae7a542 100644 --- a/hw/mainstone.c +++ b/hw/mainstone.c @@ -91,7 +91,8 @@ static struct arm_boot_info mainstone_binfo = { .ram_size = 0x0400, }; -static void mainstone_common_init(ram_addr_t ram_size, +static void mainstone_common_init(MemoryRegion *address_space_mem, +ram_addr_t ram_size, const char *kernel_filename, const char *kernel_cmdline, const char *initrd_filename, const char *cpu_model, enum mainstone_model_e model, int arm_id) @@ -102,6 +103,7 @@ static void mainstone_common_init(ram_addr_t ram_size, DeviceState *mst_irq; DriveInfo *dinfo; int i; +MemoryRegion *rom = g_new(MemoryRegion, 1); MemoryRegion *flashes = g_new(MemoryRegion, 2); const MemoryRegionOps *flash_ops; @@ -110,9 +112,9 @@ static void mainstone_common_init(ram_addr_t ram_size, /* Setup CPU memory */ cpu = pxa270_init(mainstone_binfo.ram_size, cpu_model); -cpu_register_physical_memory(0, MAINSTONE_ROM, -qemu_ram_alloc(NULL, mainstone.rom, - MAINSTONE_ROM) | IO_MEM_ROM); +memory_region_init_ram(rom, NULL, mainstone.rom, MAINSTONE_ROM); +memory_region_set_readonly(rom, true); +memory_region_add_subregion(address_space_mem, 0, rom); #ifdef TARGET_WORDS_BIGENDIAN flash_ops = pflash_cfi01_ops_be; @@ -175,7 +177,7 @@ static void mainstone_init(MemoryRegion *address_space_mem, const char *kernel_filename, const char *kernel_cmdline, const char *initrd_filename, const char *cpu_model) { -mainstone_common_init(ram_size, kernel_filename, +mainstone_common_init(address_space_mem, ram_size, kernel_filename, kernel_cmdline, initrd_filename, cpu_model, mainstone, 0x196); } -- 1.7.5.3
Re: [Qemu-devel] [PATCH 01/24] arm_sysctl: convert to memory API
On 08/24/2011 01:23 PM, Peter Maydell wrote: On 24 August 2011 11:11, Avi Kivitya...@redhat.com wrote: Signed-off-by: Avi Kivitya...@redhat.com --- hw/arm_sysctl.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/arm_sysctl.c b/hw/arm_sysctl.c index 17cf6f7..5b845a2 100644 --- a/hw/arm_sysctl.c +++ b/hw/arm_sysctl.c @@ -19,6 +19,7 @@ typedef struct { SysBusDevice busdev; MemoryRegion iomem; qemu_irq pl110_mux_ctrl; +MemoryRegion iomem; uint32_t sys_id; uint32_t leds; Er, looks like you forgot to squish Richard's compile fix patch? Not applied yet; will fix. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
[Qemu-devel] [PATCH 02/24] stellaris_enet: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/stellaris_enet.c | 29 - 1 files changed, 12 insertions(+), 17 deletions(-) diff --git a/hw/stellaris_enet.c b/hw/stellaris_enet.c index f9bd3da..d5613ff 100644 --- a/hw/stellaris_enet.c +++ b/hw/stellaris_enet.c @@ -69,7 +69,7 @@ typedef struct { NICState *nic; NICConf conf; qemu_irq irq; -int mmio_index; +MemoryRegion mmio; } stellaris_enet_state; static void stellaris_enet_update(stellaris_enet_state *s) @@ -130,7 +130,8 @@ static int stellaris_enet_can_receive(VLANClientState *nc) return (s-np 31); } -static uint32_t stellaris_enet_read(void *opaque, target_phys_addr_t offset) +static uint64_t stellaris_enet_read(void *opaque, target_phys_addr_t offset, +unsigned size) { stellaris_enet_state *s = (stellaris_enet_state *)opaque; uint32_t val; @@ -198,7 +199,7 @@ static uint32_t stellaris_enet_read(void *opaque, target_phys_addr_t offset) } static void stellaris_enet_write(void *opaque, target_phys_addr_t offset, -uint32_t value) + uint64_t value, unsigned size) { stellaris_enet_state *s = (stellaris_enet_state *)opaque; @@ -303,17 +304,12 @@ static void stellaris_enet_write(void *opaque, target_phys_addr_t offset, } } -static CPUReadMemoryFunc * const stellaris_enet_readfn[] = { - stellaris_enet_read, - stellaris_enet_read, - stellaris_enet_read +static const MemoryRegionOps stellaris_enet_ops = { +.read = stellaris_enet_read, +.write = stellaris_enet_write, +.endianness = DEVICE_NATIVE_ENDIAN, }; -static CPUWriteMemoryFunc * const stellaris_enet_writefn[] = { - stellaris_enet_write, - stellaris_enet_write, - stellaris_enet_write -}; static void stellaris_enet_reset(stellaris_enet_state *s) { s-mdv = 0x80; @@ -391,7 +387,7 @@ static void stellaris_enet_cleanup(VLANClientState *nc) unregister_savevm(s-busdev.qdev, stellaris_enet, s); -cpu_unregister_io_memory(s-mmio_index); +memory_region_destroy(s-mmio); g_free(s); } @@ -408,10 +404,9 @@ static int stellaris_enet_init(SysBusDevice *dev) { stellaris_enet_state *s = FROM_SYSBUS(stellaris_enet_state, dev); -s-mmio_index = cpu_register_io_memory(stellaris_enet_readfn, - stellaris_enet_writefn, s, - DEVICE_NATIVE_ENDIAN); -sysbus_init_mmio(dev, 0x1000, s-mmio_index); +memory_region_init_io(s-mmio, stellaris_enet_ops, s, stellaris_enet, + 0x1000); +sysbus_init_mmio_region(dev, s-mmio); sysbus_init_irq(dev, s-irq); qemu_macaddr_default_if_unset(s-conf.macaddr); -- 1.7.5.3
[Qemu-devel] [PATCH 15/24] Makefile.hw: allow hw/ files to include glib headers
Signed-off-by: Avi Kivity a...@redhat.com --- Makefile.hw |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/Makefile.hw b/Makefile.hw index 659e441..63eb7e4 100644 --- a/Makefile.hw +++ b/Makefile.hw @@ -10,6 +10,7 @@ include $(SRC_PATH)/rules.mak $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw) QEMU_CFLAGS+=-I.. +QEMU_CFLAGS += $(GLIB_CFLAGS) include $(SRC_PATH)/Makefile.objs -- 1.7.5.3
Re: [Qemu-devel] [PATCH 06/24] QEMUMachine: pass address space to machine init function
On 08/24/2011 01:32 PM, Peter Maydell wrote: On 24 August 2011 11:11, Avi Kivitya...@redhat.com wrote: Avoids get_system_memory() everywhere. -machine-init(ram_size, boot_devices, +machine-init(get_system_memory(), get_system_io(), ram_size, boot_devices, kernel_filename, kernel_cmdline, initrd_filename, cpu_model); I think this is conceptually wrong. The system memory/IO address spaces are not configurable properties of the machine model (in the way that the other arguments to machine-init() are), they're purely an internal implementation detail of it. (In fact for many of the machines we support there isn't even any such thing as the system IO space...) I agree with all you say, but does it make the patch incorrect? The purpose here is to allow removal of get_system_memory() from the general code base. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] [PATCH 21/24] mcf5208: convert to memory API
On 08/24/2011 01:35 PM, Peter Maydell wrote: On 24 August 2011 11:11, Avi Kivitya...@redhat.com wrote: diff --git a/hw/mcf5208.c b/hw/mcf5208.c index f4d4fbd..73c6961 100644 --- a/hw/mcf5208.c +++ b/hw/mcf5208.c @@ -5,6 +5,7 @@ * * This code is licensed under the GPL */ +#includeglib.h #include hw.h #include mcf.h #include qemu-timer.h You shouldn't need to include glib.h here, I think? hw.h includes qemu-common.h which includes glib.h. My preference is not to depend on indirect includes; but I can remove this include if it's disliked by many. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
[Qemu-devel] [PATCH 01/24] arm_sysctl: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/arm_sysctl.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/arm_sysctl.c b/hw/arm_sysctl.c index 17cf6f7..5b845a2 100644 --- a/hw/arm_sysctl.c +++ b/hw/arm_sysctl.c @@ -19,6 +19,7 @@ typedef struct { SysBusDevice busdev; MemoryRegion iomem; qemu_irq pl110_mux_ctrl; +MemoryRegion iomem; uint32_t sys_id; uint32_t leds; -- 1.7.5.3
Re: [Qemu-devel] [PATCH 06/24] QEMUMachine: pass address space to machine init function
On 24 August 2011 11:46, Avi Kivity a...@redhat.com wrote: On 08/24/2011 01:32 PM, Peter Maydell wrote: On 24 August 2011 11:11, Avi Kivitya...@redhat.com wrote: Avoids get_system_memory() everywhere. - machine-init(ram_size, boot_devices, + machine-init(get_system_memory(), get_system_io(), ram_size, boot_devices, kernel_filename, kernel_cmdline, initrd_filename, cpu_model); I think this is conceptually wrong. The system memory/IO address spaces are not configurable properties of the machine model (in the way that the other arguments to machine-init() are), they're purely an internal implementation detail of it. (In fact for many of the machines we support there isn't even any such thing as the system IO space...) I agree with all you say, but does it make the patch incorrect? My assertion is that patches which are conceptually wrong are also incorrect :-) The purpose here is to allow removal of get_system_memory() from the general code base. The right way to remove get_system_memory() from the general code base is to actually model things correctly, for instance by having machine models create a container memory region into which they insert the memory regions for all the devices which currently use sysbus_mmio_map to map themselves, and then pass the container memory region to the master end of the bus, ie the CPU. -- PMM
[Qemu-devel] [PATCH 08/24] armv7m: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/arm-misc.h |5 - hw/armv7m.c| 24 ++-- hw/stellaris.c |3 ++- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/hw/arm-misc.h b/hw/arm-misc.h index f8a7472..af403a1 100644 --- a/hw/arm-misc.h +++ b/hw/arm-misc.h @@ -11,13 +11,16 @@ #ifndef ARM_MISC_H #define ARM_MISC_H 1 +#include memory.h + /* The CPU is also modeled as an interrupt controller. */ #define ARM_PIC_CPU_IRQ 0 #define ARM_PIC_CPU_FIQ 1 qemu_irq *arm_pic_init_cpu(CPUState *env); /* armv7m.c */ -qemu_irq *armv7m_init(int flash_size, int sram_size, +qemu_irq *armv7m_init(MemoryRegion *address_space_mem, + int flash_size, int sram_size, const char *kernel_filename, const char *cpu_model); /* arm_boot.c */ diff --git a/hw/armv7m.c b/hw/armv7m.c index a932f16..db535fd 100644 --- a/hw/armv7m.c +++ b/hw/armv7m.c @@ -7,6 +7,8 @@ * This code is licensed under the GPL. */ +#include glib.h + #include sysbus.h #include arm-misc.h #include loader.h @@ -156,7 +158,8 @@ static void armv7m_reset(void *opaque) flash_size and sram_size are in kb. Returns the NVIC array. */ -qemu_irq *armv7m_init(int flash_size, int sram_size, +qemu_irq *armv7m_init(MemoryRegion *address_space_mem, + int flash_size, int sram_size, const char *kernel_filename, const char *cpu_model) { CPUState *env; @@ -169,6 +172,9 @@ qemu_irq *armv7m_init(int flash_size, int sram_size, uint64_t lowaddr; int i; int big_endian; +MemoryRegion *sram = g_new(MemoryRegion, 1); +MemoryRegion *flash = g_new(MemoryRegion, 1); +MemoryRegion *hack = g_new(MemoryRegion, 1); flash_size *= 1024; sram_size *= 1024; @@ -194,12 +200,11 @@ qemu_irq *armv7m_init(int flash_size, int sram_size, #endif /* Flash programming is done via the SCU, so pretend it is ROM. */ -cpu_register_physical_memory(0, flash_size, - qemu_ram_alloc(NULL, armv7m.flash, -flash_size) | IO_MEM_ROM); -cpu_register_physical_memory(0x2000, sram_size, - qemu_ram_alloc(NULL, armv7m.sram, -sram_size) | IO_MEM_RAM); +memory_region_init_ram(flash, NULL, armv7m.flash, flash_size); +memory_region_set_readonly(flash, true); +memory_region_add_subregion(address_space_mem, 0, flash); +memory_region_init_ram(sram, NULL, armv7m.sram, sram_size); +memory_region_add_subregion(address_space_mem, 0x2000, sram); armv7m_bitband_init(); nvic = qdev_create(NULL, armv7m_nvic); @@ -232,9 +237,8 @@ qemu_irq *armv7m_init(int flash_size, int sram_size, /* Hack to map an additional page of ram at the top of the address space. This stops qemu complaining about executing code outside RAM when returning from an exception. */ -cpu_register_physical_memory(0xf000, 0x1000, - qemu_ram_alloc(NULL, armv7m.hack, -0x1000) | IO_MEM_RAM); +memory_region_init_ram(hack, NULL, armv7m.hack, 0x1000); +memory_region_add_subregion(address_space_mem, 0xf000, hack); qemu_register_reset(armv7m_reset, env); return pic; diff --git a/hw/stellaris.c b/hw/stellaris.c index f47b06e..d354008 100644 --- a/hw/stellaris.c +++ b/hw/stellaris.c @@ -1276,7 +1276,8 @@ static void stellaris_init(MemoryRegion *address_space_mem, flash_size = ((board-dc0 0x) + 1) 1; sram_size = (board-dc0 18) + 1; -pic = armv7m_init(flash_size, sram_size, kernel_filename, cpu_model); +pic = armv7m_init(address_space_mem, + flash_size, sram_size, kernel_filename, cpu_model); if (board-dc1 (1 16)) { dev = sysbus_create_varargs(stellaris-adc, 0x40038000, -- 1.7.5.3
[Qemu-devel] [PATCH STABLE-0.14/0.15/master] CVE-2011-0011: fix VNC password change to not touch authentication settings
From: Daniel P. Berrange berra...@redhat.com In CVE-2011-0011 it was noted that setting an empty password would disable all authentication for the VNC password. Commit 1cd20f8bf0ecb9d1d1bd5e2ffab3b88835380c9b attempted to fix this but it just broke it in a different way, because now instead of blindly disabling all authentication, it blindly resets all authentication to 'VNC'. This disables any TLS auth that might have been enabled, which is pratically as bad as the original problem. eg, consider launching QEMU with TLS + password as per the docs section 3.11.5 $ qemu [...OPTIONS...] -vnc :1,password,tls -monitor stdio (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vencrypt+tls+vnc Client: none (qemu) change vnc password 123 (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vnc Client: none After setting the password, the TLS auth has been disabled meaning all communications are back in cleartext. The 'change vnc password' command must *never* touch the 'vs-auth' field under any circumstances. Similarly setting the password to (which causes all auth attempts to fail) must *not* touch vs-auth, otherwise it breaks the following sequence $ qemu [...OPTIONS...] -vnc :1,password,tls -monitor stdio (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vencrypt+tls+vnc Client: none (qemu) change vnc password 123 (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vencrypt+tls+vnc Client: none (qemu) change vnc password (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vnc Client: none (qemu) change vnc password 456 (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vnc Client: none This patch puts the behaviour back to what it was before the original mistaken commit 52c18be9e99dabe295321153fda7fce9f76647ac * ui/vnc.c: Do not touch 'vs-auth' when changing password and remove unneccessary 'vnc_disable_login' method * monitor.c: Remove call to 'vnc_disable_login' Signed-off-by: Daniel P. Berrange berra...@redhat.com --- console.h |1 - monitor.c |8 ui/vnc.c | 30 +++--- 3 files changed, 3 insertions(+), 36 deletions(-) diff --git a/console.h b/console.h index 67d1373..2eb03a1 100644 --- a/console.h +++ b/console.h @@ -373,7 +373,6 @@ void vnc_display_init(DisplayState *ds); void vnc_display_close(DisplayState *ds); int vnc_display_open(DisplayState *ds, const char *display); void vnc_display_add_client(DisplayState *ds, int csock, int skipauth); -int vnc_display_disable_login(DisplayState *ds); char *vnc_display_local_addr(DisplayState *ds); #ifdef CONFIG_VNC int vnc_display_password(DisplayState *ds, const char *password); diff --git a/monitor.c b/monitor.c index 1b8ba2c..59af05a 100644 --- a/monitor.c +++ b/monitor.c @@ -1023,14 +1023,6 @@ static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data) #ifdef CONFIG_VNC static int change_vnc_password(const char *password) { -if (!password || !password[0]) { -if (vnc_display_disable_login(NULL)) { -qerror_report(QERR_SET_PASSWD_FAILED); -return -1; -} -return 0; -} - if (vnc_display_password(NULL, password) 0) { qerror_report(QERR_SET_PASSWD_FAILED); return -1; diff --git a/ui/vnc.c b/ui/vnc.c index f1e27d9..f7fc7d2 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -2635,24 +2635,6 @@ void vnc_display_close(DisplayState *ds) #endif } -int vnc_display_disable_login(DisplayState *ds) -{ -VncDisplay *vs = ds ? (VncDisplay *)ds-opaque : vnc_display; - -if (!vs) { -return -1; -} - -if (vs-password) { -qemu_free(vs-password); -} - -vs-password = NULL; -vs-auth = VNC_AUTH_VNC; - -return 0; -} - int vnc_display_password(DisplayState *ds, const char *password) { int ret = 0; @@ -2663,19 +2645,13 @@ int vnc_display_password(DisplayState *ds, const char *password) goto out; } -if (!password) { -/* This is not the intention of this interface but err on the side - of being safe */ -ret = vnc_display_disable_login(ds); -goto out; -} - if (vs-password) { qemu_free(vs-password); vs-password = NULL; } -vs-password = qemu_strdup(password); -vs-auth = VNC_AUTH_VNC; +if (password) + vs-password = qemu_strdup(password); + out: if (ret != 0) { qerror_report(QERR_SET_PASSWD_FAILED); -- 1.7.6
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions
On Wed, Aug 24, 2011 at 12:10:32PM +0200, Jan Kiszka wrote: On 2011-08-24 12:04, Michael S. Tsirkin wrote: On Tue, Aug 23, 2011 at 07:28:08PM +0200, Jan Kiszka wrote: From: Alex Williamson alex.william...@redhat.com Nothing good can happen when we overlap capabilities [ Jan: rebased over qemu, minor formatting ] Signed-off-by: Jan Kiszka jan.kis...@siemens.com This doesn't build for me: /scm/qemu/hw/pci.c: In function ‘pci_add_capability’: /scm/qemu/hw/pci.c:1970:45: error: ‘PCIDevice’ has no member named ‘config_map’ Yeah, sorry, forgot to refresh the commit before posting. Happens to me too, sometimes. I think that what that includes is the capability including each given offset, right? It would be easy to write some code scanning the capability list to figure this value out. Something along the lines of (untested): static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) { uint8_t next, prev, found = 0; if (!(pdev-config[PCI_STATUS] PCI_STATUS_CAP_LIST)) return 0; for (prev = PCI_CAPABILITY_LIST; (next = pdev-config[prev]); prev = next + PCI_CAP_LIST_NEXT) if (next = offset next found) found = next; return found; } Sounds useful, will enhance the patch. (Originally, I just wanted to reduce the qemu-kvm delta... :) ) Jan Oh, if you want to just drop that bit, that's also fine. Up to you. -- MST
[Qemu-devel] [PATCH 12/24] leon3: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/leon3.c | 16 ++-- 1 files changed, 10 insertions(+), 6 deletions(-) diff --git a/hw/leon3.c b/hw/leon3.c index efe6c7c..5c3780d 100644 --- a/hw/leon3.c +++ b/hw/leon3.c @@ -21,6 +21,9 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN * THE SOFTWARE. */ + +#include glib.h + #include hw.h #include qemu-timer.h #include qemu-char.h @@ -102,7 +105,8 @@ static void leon3_generic_hw_init(MemoryRegion *address_space_mem, const char *cpu_model) { CPUState *env; -ram_addr_t ram_offset, prom_offset; +MemoryRegion *ram = g_new(MemoryRegion, 1); +MemoryRegion *prom = g_new(MemoryRegion, 1); int ret; char *filename; qemu_irq *cpu_irqs = NULL; @@ -141,14 +145,14 @@ static void leon3_generic_hw_init(MemoryRegion *address_space_mem, exit(1); } -ram_offset = qemu_ram_alloc(NULL, leon3.ram, ram_size); -cpu_register_physical_memory(0x4000, ram_size, ram_offset | IO_MEM_RAM); +memory_region_init_ram(ram, NULL, leon3.ram, ram_size); +memory_region_add_subregion(address_space_mem, 0x4000, ram); /* Allocate BIOS */ prom_size = 8 * 1024 * 1024; /* 8Mb */ -prom_offset = qemu_ram_alloc(NULL, Leon3.bios, prom_size); -cpu_register_physical_memory(0x, prom_size, - prom_offset | IO_MEM_ROM); +memory_region_init_ram(prom, NULL, Leon3.bios, prom_size); +memory_region_set_readonly(prom, true); +memory_region_add_subregion(address_space_mem, 0x, prom); /* Load boot prom */ if (bios_name == NULL) { -- 1.7.5.3
[Qemu-devel] [PATCH 00/24] Memory API conversions, batch 5
This is a collection of random conversions to devices to the new memory API. Some qemu interfaces are also update, for example machine creation and sysbus. Please review. If all is fine, I will post a [PULL] request for the batch (in git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/batch). Avi Kivity (24): arm_sysctl: convert to memory API stellaris_enet: convert to memory API sysbus: add helpers to add and delete memory regions to the system bus pci_host: convert conf index and data ports to memory API ReadWriteHandler: remove QEMUMachine: pass address space to machine init function an5206: convert to memory API armv7m: convert to memory API axis_dev88: convert to memory API (RAM only) sysbus: add sysbus_add_memory_overlap() integratorcp: convert to memory API (RAM/flash only) leon3: convert to memory API cirrus: wrap memory update in a transaction piix_pci: wrap memory update in a transaction Makefile.hw: allow hw/ files to include glib headers pflash_cfi01/pflash_cfi02: convert to memory API dummy_m68k: convert to memory API g364fb: convert to memory API lm32_boards: convert to memory API mainstone: convert to memory API mcf5208: convert to memory API milkymist-minimac2: convert to memory API milkymist-softusb: convert to memory API milkymist: convert to memory API Makefile.hw |1 + Makefile.target |1 - hw/an5206.c | 16 +-- hw/arm-misc.h |5 ++- hw/arm_sysctl.c |1 + hw/armv7m.c | 24 ++ hw/axis_dev88.c | 20 + hw/boards.h |5 ++- hw/cirrus_vga.c |2 + hw/collie.c | 21 ++--- hw/dec_pci.c | 13 +++--- hw/dummy_m68k.c | 10 +++- hw/flash.h| 16 +-- hw/g364fb.c | 61 +++ hw/grackle_pci.c | 13 +++--- hw/gumstix.c | 37 ++-- hw/integratorcp.c | 32 ++ hw/leon3.c| 20 ++--- hw/lm32_boards.c | 35 +-- hw/mainstone.c| 37 +-- hw/mcf5208.c | 75 +++-- hw/milkymist-minimac2.c | 43 +- hw/milkymist-softusb.c| 48 ++-- hw/milkymist.c| 20 +--- hw/mips.h |4 +- hw/mips_fulong2e.c|4 +- hw/mips_jazz.c| 11 +++- hw/mips_malta.c | 48 - hw/mips_mipssim.c |4 +- hw/mips_r4k.c | 23 ++ hw/musicpal.c | 20 ++--- hw/nseries.c | 18 ++-- hw/omap_sx1.c | 30 - hw/palm.c |4 +- hw/pc_piix.c | 31 +- hw/pci_host.c | 86 - hw/pci_host.h | 16 +++ hw/petalogix_ml605_mmu.c | 12 +++-- hw/petalogix_s3adsp1800_mmu.c | 13 -- hw/pflash_cfi01.c | 67 ++--- hw/pflash_cfi02.c | 95 - hw/piix_pci.c | 13 +- hw/ppc405_boards.c| 69 +- hw/ppc440_bamboo.c|4 +- hw/ppc4xx_pci.c | 10 +++-- hw/ppc_newworld.c |4 +- hw/ppc_oldworld.c |4 +- hw/ppc_prep.c |4 +- hw/ppce500_mpc8544ds.c|4 +- hw/ppce500_pci.c | 21 - hw/prep_pci.c | 12 - hw/r2d.c | 12 - hw/realview.c | 33 ++ hw/s390-virtio.c |4 +- hw/shix.c |4 +- hw/spitz.c| 32 ++ hw/stellaris.c| 21 ++--- hw/stellaris_enet.c | 29 +--- hw/sun4m.c| 48 +++- hw/sun4u.c| 12 - hw/syborg.c |4 +- hw/sysbus.c | 28 hw/sysbus.h |8 +++ hw/tosa.c |4 +- hw/unin_pci.c | 82 ++-- hw/versatilepb.c | 19 ++-- hw/vexpress.c |4 +- hw/virtex_ml507.c | 12 +++-- hw/xen_machine_pv.c |4 +- hw/z2.c | 20 +--- rwhandler.c | 87 - rwhandler.h | 27 vl.c |3 +- 73 files changed, 924 insertions(+), 760 deletions(-) delete
Re: [Qemu-devel] kvm PCI assignment VFIO ramblings
On Wed, Aug 24, 2011 at 05:33:00AM -0400, David Gibson wrote: On Wed, Aug 24, 2011 at 11:14:26AM +0200, Roedel, Joerg wrote: I don't see a reason to make this meta-grouping static. It would harm flexibility on x86. I think it makes things easier on power but there are options on that platform to get the dynamic solution too. I think several people are misreading what Ben means by static. I would prefer to say 'persistent', in that the meta-groups lifetime is not tied to an fd, but they can be freely created, altered and removed during runtime. Even if it can be altered at runtime, from a usability perspective it is certainly the best to handle these groups directly in qemu. Or are there strong reasons to do it somewhere else? Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo, Andrew Bowd Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
[Qemu-devel] [PATCH 17/24] dummy_m68k: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/dummy_m68k.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/dummy_m68k.c b/hw/dummy_m68k.c index 2de69ad..609d04c 100644 --- a/hw/dummy_m68k.c +++ b/hw/dummy_m68k.c @@ -6,6 +6,7 @@ * This code is licensed under the GPL */ +#include glib.h #include hw.h #include boards.h #include loader.h @@ -23,6 +24,7 @@ static void dummy_m68k_init(MemoryRegion *address_space_mem, const char *initrd_filename, const char *cpu_model) { CPUState *env; +MemoryRegion *ram = g_new(MemoryRegion, 1); int kernel_size; uint64_t elf_entry; target_phys_addr_t entry; @@ -39,8 +41,8 @@ static void dummy_m68k_init(MemoryRegion *address_space_mem, env-vbr = 0; /* RAM at address zero */ -cpu_register_physical_memory(0, ram_size, -qemu_ram_alloc(NULL, dummy_m68k.ram, ram_size) | IO_MEM_RAM); +memory_region_init_ram(ram, NULL, dummy_m68k.ram, ram_size); +memory_region_add_subregion(address_space_mem, 0, ram); /* Load kernel. */ if (kernel_filename) { -- 1.7.5.3
Re: [Qemu-devel] [PATCH 06/24] QEMUMachine: pass address space to machine init function
On 08/24/2011 01:53 PM, Peter Maydell wrote: The purpose here is to allow removal of get_system_memory() from the general code base. The right way to remove get_system_memory() from the general code base is to actually model things correctly, for instance by having machine models create a container memory region into which they insert the memory regions for all the devices which currently use sysbus_mmio_map to map themselves, and then pass the container memory region to the master end of the bus, ie the CPU. I think you're right. This also allows eventual removal of system_io on anything non-x86. So a replacement would look like: (before) -static void pc_init_isa(ram_addr_t ram_size, +static void pc_init_isa(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, @@ -259,15 +265,17 @@ static void pc_init_isa(ram_addr_t ram_size, { if (cpu_model == NULL) cpu_model = 486; -pc_init1(get_system_memory(), - get_system_io(), +pc_init1(address_space_mem, + address_space_io, ram_size, boot_device, kernel_filename, kernel_cmdline, initrd_filename, cpu_model, 0, 1); } (after) @@ -259,15 +265,17 @@ static void pc_init_isa(ram_addr_t ram_size, { +MemoryRegion *address_space_mem, *address_space_io; + +setup_system_memory(address_space_mem,address_space_io); if (cpu_model == NULL) cpu_model = 486; -pc_init1(get_system_memory(), - get_system_io(), +pc_init1(address_space_mem, + address_space_io, ram_size, boot_device, kernel_filename, kernel_cmdline, initrd_filename, cpu_model, 0, 1); } Later on, we'd refine the setup_system_memory() calls, for example not to create the io space on non-x86. A possible complication is whether anything currently uses system_memory before -init is called. Anyone know? -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] [PATCH 11/24] integratorcp: convert to memory API (RAM/flash only)
On 24 August 2011 11:11, Avi Kivity a...@redhat.com wrote: @@ -108,9 +111,15 @@ static uint32_t integratorcm_read(void *opaque, target_phys_addr_t offset) static void integratorcm_do_remap(integratorcm_state *s, int flash) { if (flash) { - cpu_register_physical_memory(0, 0x10, IO_MEM_RAM); + if (s-flash_mapped) { + sysbus_del_memory(s-busdev, s-flash); + s-flash_mapped = false; + } } else { - cpu_register_physical_memory(0, 0x10, s-flash_offset | IO_MEM_RAM); + if (!s-flash_mapped) { + sysbus_add_memory_overlap(s-busdev, 0, s-flash, 1); + s-flash_mapped = true; + } } This is introducing a new field in the device state which is actually just tracking s-cm_init bit 2. At least it would be, except that in integratorcm_set_ctrl this line: s-cm_init = (s-cm_init ~ 5) | (value ^ 5); appears to be using ^ when it meant ... This isn't a nak -- I'm happy for this to get committed and I'll fix things up later, since the device doesn't have a VMStateDescription that would be broken by the extra state field. (Or if I get round to it before the series gets committed I'll send you a fix to squash into this patch.) -- PMM
[Qemu-devel] [PATCH 04/24] pci_host: convert conf index and data ports to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/dec_pci.c | 13 hw/grackle_pci.c | 13 hw/pci_host.c| 86 -- hw/pci_host.h| 16 -- hw/piix_pci.c| 11 +- hw/ppc4xx_pci.c | 10 -- hw/ppce500_pci.c | 21 ++--- hw/prep_pci.c| 12 +-- hw/unin_pci.c| 82 +- 9 files changed, 131 insertions(+), 133 deletions(-) diff --git a/hw/dec_pci.c b/hw/dec_pci.c index a35f382..1aec066 100644 --- a/hw/dec_pci.c +++ b/hw/dec_pci.c @@ -80,16 +80,15 @@ PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn) static int pci_dec_21154_init_device(SysBusDevice *dev) { DECState *s; -int pci_mem_config, pci_mem_data; s = FROM_SYSBUS(DECState, dev); -pci_mem_config = pci_host_conf_register_mmio(s-host_state, - DEVICE_LITTLE_ENDIAN); -pci_mem_data = pci_host_data_register_mmio(s-host_state, - DEVICE_LITTLE_ENDIAN); -sysbus_init_mmio(dev, 0x1000, pci_mem_config); -sysbus_init_mmio(dev, 0x1000, pci_mem_data); +memory_region_init_io(s-host_state.conf_mem, pci_host_conf_le_ops, + s-host_state, pci-conf-idx, 0x1000); +memory_region_init_io(s-host_state.data_mem, pci_host_data_le_ops, + s-host_state, pci-data-idx, 0x1000); +sysbus_init_mmio_region(dev, s-host_state.conf_mem); +sysbus_init_mmio_region(dev, s-host_state.data_mem); return 0; } diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c index 9a823e1..9d3ff7d 100644 --- a/hw/grackle_pci.c +++ b/hw/grackle_pci.c @@ -92,16 +92,15 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic, static int pci_grackle_init_device(SysBusDevice *dev) { GrackleState *s; -int pci_mem_config, pci_mem_data; s = FROM_SYSBUS(GrackleState, dev); -pci_mem_config = pci_host_conf_register_mmio(s-host_state, - DEVICE_LITTLE_ENDIAN); -pci_mem_data = pci_host_data_register_mmio(s-host_state, - DEVICE_LITTLE_ENDIAN); -sysbus_init_mmio(dev, 0x1000, pci_mem_config); -sysbus_init_mmio(dev, 0x1000, pci_mem_data); +memory_region_init_io(s-host_state.conf_mem, pci_host_conf_le_ops, + s-host_state, pci-conf-idx, 0x1000); +memory_region_init_io(s-host_state.data_mem, pci_host_data_le_ops, + s-host_state, pci-data-idx, 0x1000); +sysbus_init_mmio_region(dev, s-host_state.conf_mem); +sysbus_init_mmio_region(dev, s-host_state.data_mem); qemu_register_reset(pci_grackle_reset, s-host_state); return 0; diff --git a/hw/pci_host.c b/hw/pci_host.c index 2e8a29f..44c6c20 100644 --- a/hw/pci_host.c +++ b/hw/pci_host.c @@ -94,82 +94,72 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len) return val; } -static void pci_host_config_write(ReadWriteHandler *handler, - pcibus_t addr, uint32_t val, int len) +static void pci_host_config_write(void *opaque, target_phys_addr_t addr, + uint64_t val, unsigned len) { -PCIHostState *s = container_of(handler, PCIHostState, conf_handler); +PCIHostState *s = opaque; -PCI_DPRINTF(%s addr % FMT_PCIBUS %d val %PRIx32\n, +PCI_DPRINTF(%s addr TARGET_FMT_plx len %d val %PRIx64\n, __func__, addr, len, val); s-config_reg = val; } -static uint32_t pci_host_config_read(ReadWriteHandler *handler, - pcibus_t addr, int len) +static uint64_t pci_host_config_read(void *opaque, target_phys_addr_t addr, + unsigned len) { -PCIHostState *s = container_of(handler, PCIHostState, conf_handler); +PCIHostState *s = opaque; uint32_t val = s-config_reg; -PCI_DPRINTF(%s addr % FMT_PCIBUS len %d val %PRIx32\n, +PCI_DPRINTF(%s addr TARGET_FMT_plx len %d val %PRIx32\n, __func__, addr, len, val); return val; } -static void pci_host_data_write(ReadWriteHandler *handler, -pcibus_t addr, uint32_t val, int len) +static void pci_host_data_write(void *opaque, target_phys_addr_t addr, +uint64_t val, unsigned len) { -PCIHostState *s = container_of(handler, PCIHostState, data_handler); -PCI_DPRINTF(write addr % FMT_PCIBUS len %d val %x\n, -addr, len, val); +PCIHostState *s = opaque; +PCI_DPRINTF(write addr TARGET_FMT_plx len %d val %x\n, +addr, len, (unsigned)val); if (s-config_reg (1u 31)) pci_data_write(s-bus, s-config_reg | (addr 3), val, len); } -static uint32_t pci_host_data_read(ReadWriteHandler *handler, - pcibus_t addr, int
[Qemu-devel] [PATCH 10/24] sysbus: add sysbus_add_memory_overlap()
Signed-off-by: Avi Kivity a...@redhat.com --- hw/sysbus.c |6 ++ hw/sysbus.h |2 ++ 2 files changed, 8 insertions(+), 0 deletions(-) diff --git a/hw/sysbus.c b/hw/sysbus.c index f5f0ed2..4efb91a 100644 --- a/hw/sysbus.c +++ b/hw/sysbus.c @@ -263,6 +263,12 @@ void sysbus_add_memory(SysBusDevice *dev, target_phys_addr_t addr, memory_region_add_subregion(get_system_memory(), addr, mem); } +void sysbus_add_memory_overlap(SysBusDevice *dev, target_phys_addr_t addr, + MemoryRegion *mem, unsigned priority) +{ +memory_region_add_subregion(get_system_memory(), addr, mem); +} + void sysbus_del_memory(SysBusDevice *dev, MemoryRegion *mem) { memory_region_del_subregion(get_system_memory(), mem); diff --git a/hw/sysbus.h b/hw/sysbus.h index e4d56cf..b3e1f99 100644 --- a/hw/sysbus.h +++ b/hw/sysbus.h @@ -59,6 +59,8 @@ void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq); void sysbus_mmio_map(SysBusDevice *dev, int n, target_phys_addr_t addr); void sysbus_add_memory(SysBusDevice *dev, target_phys_addr_t addr, MemoryRegion *mem); +void sysbus_add_memory_overlap(SysBusDevice *dev, target_phys_addr_t addr, + MemoryRegion *mem, unsigned priority); void sysbus_del_memory(SysBusDevice *dev, MemoryRegion *mem); void sysbus_add_io(SysBusDevice *dev, target_phys_addr_t addr, MemoryRegion *mem); -- 1.7.5.3
[Qemu-devel] [PATCH 22/24] milkymist-minimac2: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/milkymist-minimac2.c | 43 +-- 1 files changed, 21 insertions(+), 22 deletions(-) diff --git a/hw/milkymist-minimac2.c b/hw/milkymist-minimac2.c index cd36026..fb48e37 100644 --- a/hw/milkymist-minimac2.c +++ b/hw/milkymist-minimac2.c @@ -97,6 +97,8 @@ struct MilkymistMinimac2State { NICConf conf; char *phy_model; target_phys_addr_t buffers_base; +MemoryRegion buffers; +MemoryRegion regs_region; qemu_irq rx_irq; qemu_irq tx_irq; @@ -320,8 +322,8 @@ static ssize_t minimac2_rx(VLANClientState *nc, const uint8_t *buf, size_t size) return size; } -static uint32_t -minimac2_read(void *opaque, target_phys_addr_t addr) +static uint64_t +minimac2_read(void *opaque, target_phys_addr_t addr, unsigned size) { MilkymistMinimac2State *s = opaque; uint32_t r = 0; @@ -350,7 +352,8 @@ minimac2_read(void *opaque, target_phys_addr_t addr) } static void -minimac2_write(void *opaque, target_phys_addr_t addr, uint32_t value) +minimac2_write(void *opaque, target_phys_addr_t addr, uint64_t value, + unsigned size) { MilkymistMinimac2State *s = opaque; @@ -395,16 +398,14 @@ minimac2_write(void *opaque, target_phys_addr_t addr, uint32_t value) } } -static CPUReadMemoryFunc * const minimac2_read_fn[] = { -NULL, -NULL, -minimac2_read, -}; - -static CPUWriteMemoryFunc * const minimac2_write_fn[] = { -NULL, -NULL, -minimac2_write, +static const MemoryRegionOps minimac2_ops = { +.read = minimac2_read, +.write = minimac2_write, +.valid = { +.min_access_size = 4, +.max_access_size = 4, +}, +.endianness = DEVICE_NATIVE_ENDIAN, }; static int minimac2_can_rx(VLANClientState *nc) @@ -457,25 +458,23 @@ static NetClientInfo net_milkymist_minimac2_info = { static int milkymist_minimac2_init(SysBusDevice *dev) { MilkymistMinimac2State *s = FROM_SYSBUS(typeof(*s), dev); -int regs; -ram_addr_t buffers; size_t buffers_size = TARGET_PAGE_ALIGN(3 * MINIMAC2_BUFFER_SIZE); sysbus_init_irq(dev, s-rx_irq); sysbus_init_irq(dev, s-tx_irq); -regs = cpu_register_io_memory(minimac2_read_fn, minimac2_write_fn, s, -DEVICE_NATIVE_ENDIAN); -sysbus_init_mmio(dev, R_MAX * 4, regs); +memory_region_init_io(s-regs_region, minimac2_ops, s, + minimac2-mmio, R_MAX * 4); +sysbus_init_mmio_region(dev, s-regs_region); /* register buffers memory */ -buffers = qemu_ram_alloc(NULL, milkymist_minimac2.buffers, buffers_size); -s-rx0_buf = qemu_get_ram_ptr(buffers); +memory_region_init_ram(s-buffers, NULL, milkymist_minimac2.buffers, + buffers_size); +s-rx0_buf = memory_region_get_ram_ptr(s-buffers); s-rx1_buf = s-rx0_buf + MINIMAC2_BUFFER_SIZE; s-tx_buf = s-rx1_buf + MINIMAC2_BUFFER_SIZE; -cpu_register_physical_memory(s-buffers_base, buffers_size, -buffers | IO_MEM_RAM); +sysbus_add_memory(dev, s-buffers_base, s-buffers); qemu_macaddr_default_if_unset(s-conf.macaddr); s-nic = qemu_new_nic(net_milkymist_minimac2_info, s-conf, -- 1.7.5.3
Re: [Qemu-devel] [PATCH 11/24] integratorcp: convert to memory API (RAM/flash only)
On 08/24/2011 02:22 PM, Peter Maydell wrote: On 24 August 2011 11:11, Avi Kivitya...@redhat.com wrote: @@ -108,9 +111,15 @@ static uint32_t integratorcm_read(void *opaque, target_phys_addr_t offset) static void integratorcm_do_remap(integratorcm_state *s, int flash) { if (flash) { -cpu_register_physical_memory(0, 0x10, IO_MEM_RAM); +if (s-flash_mapped) { +sysbus_del_memory(s-busdev,s-flash); +s-flash_mapped = false; +} } else { -cpu_register_physical_memory(0, 0x10, s-flash_offset | IO_MEM_RAM); +if (!s-flash_mapped) { +sysbus_add_memory_overlap(s-busdev, 0,s-flash, 1); +s-flash_mapped = true; +} } This is introducing a new field in the device state which is actually just tracking s-cm_init bit 2. At least it would be, except that in integratorcm_set_ctrl this line: s-cm_init = (s-cm_init ~ 5) | (value ^ 5); appears to be using ^ when it meant ... This isn't a nak -- I'm happy for this to get committed and I'll fix things up later, since the device doesn't have a VMStateDescription that would be broken by the extra state field. Even with vmstate, nothing would break since the field would be recovered on restore. (Or if I get round to it before the series gets committed I'll send you a fix to squash into this patch.) That would be best. Please post the /^ fixup separately (if needed) since it isn't strictly related to the conversion. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
[Qemu-devel] [PATCH 23/24] milkymist-softusb: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/milkymist-softusb.c | 48 1 files changed, 24 insertions(+), 24 deletions(-) diff --git a/hw/milkymist-softusb.c b/hw/milkymist-softusb.c index fe4eedb..ef4d9ee 100644 --- a/hw/milkymist-softusb.c +++ b/hw/milkymist-softusb.c @@ -49,6 +49,9 @@ struct MilkymistSoftUsbState { HIDState hid_kbd; HIDState hid_mouse; +MemoryRegion regs_region; +MemoryRegion pmem; +MemoryRegion dmem; qemu_irq irq; /* device properties */ @@ -68,7 +71,8 @@ struct MilkymistSoftUsbState { }; typedef struct MilkymistSoftUsbState MilkymistSoftUsbState; -static uint32_t softusb_read(void *opaque, target_phys_addr_t addr) +static uint64_t softusb_read(void *opaque, target_phys_addr_t addr, + unsigned size) { MilkymistSoftUsbState *s = opaque; uint32_t r = 0; @@ -91,7 +95,8 @@ static uint32_t softusb_read(void *opaque, target_phys_addr_t addr) } static void -softusb_write(void *opaque, target_phys_addr_t addr, uint32_t value) +softusb_write(void *opaque, target_phys_addr_t addr, uint64_t value, + unsigned size) { MilkymistSoftUsbState *s = opaque; @@ -110,16 +115,14 @@ softusb_write(void *opaque, target_phys_addr_t addr, uint32_t value) } } -static CPUReadMemoryFunc * const softusb_read_fn[] = { -NULL, -NULL, -softusb_read, -}; - -static CPUWriteMemoryFunc * const softusb_write_fn[] = { -NULL, -NULL, -softusb_write, +static const MemoryRegionOps softusb_mmio_ops = { +.read = softusb_read, +.write = softusb_write, +.endianness = DEVICE_NATIVE_ENDIAN, +.valid = { +.min_access_size = 4, +.max_access_size = 4, +}, }; static inline void softusb_read_dmem(MilkymistSoftUsbState *s, @@ -256,23 +259,20 @@ static void milkymist_softusb_reset(DeviceState *d) static int milkymist_softusb_init(SysBusDevice *dev) { MilkymistSoftUsbState *s = FROM_SYSBUS(typeof(*s), dev); -int softusb_regs; -ram_addr_t pmem_ram; -ram_addr_t dmem_ram; sysbus_init_irq(dev, s-irq); -softusb_regs = cpu_register_io_memory(softusb_read_fn, softusb_write_fn, s, -DEVICE_NATIVE_ENDIAN); -sysbus_init_mmio(dev, R_MAX * 4, softusb_regs); +memory_region_init_io(s-regs_region, softusb_mmio_ops, s, + milkymist-softusb, R_MAX * 4); +sysbus_init_mmio_region(dev, s-regs_region); /* register pmem and dmem */ -pmem_ram = qemu_ram_alloc(NULL, milkymist_softusb.pmem, s-pmem_size); -cpu_register_physical_memory(s-pmem_base, s-pmem_size, -pmem_ram | IO_MEM_RAM); -dmem_ram = qemu_ram_alloc(NULL, milkymist_softusb.dmem, s-dmem_size); -cpu_register_physical_memory(s-dmem_base, s-dmem_size, -dmem_ram | IO_MEM_RAM); +memory_region_init_ram(s-pmem, NULL, milkymist_softusb.pmem, + s-pmem_size); +sysbus_add_memory(dev, s-pmem_base, s-pmem); +memory_region_init_ram(s-dmem, NULL, milkymist_softusb.dmem, + s-dmem_size); +sysbus_add_memory(dev, s-dmem_base, s-dmem); hid_init(s-hid_kbd, HID_KEYBOARD, softusb_kbd_hid_datain); hid_init(s-hid_mouse, HID_MOUSE, softusb_mouse_hid_datain); -- 1.7.5.3
[Qemu-devel] [PATCH 14/24] piix_pci: wrap memory update in a transaction
The code will remap all PAMs, even if just one is updated, resulting in reduced performance. Wrap in a transaction to detect that those other PAMs have not changed. Signed-off-by: Avi Kivity a...@redhat.com --- hw/piix_pci.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/piix_pci.c b/hw/piix_pci.c index f892994..8f6ea42 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -142,6 +142,7 @@ static void i440fx_update_memory_mappings(PCII440FXState *d) int i, r; uint32_t smram; +memory_region_transaction_begin(); update_pam(d, 0xf, 0x10, (d-dev.config[I440FX_PAM] 4) 3, d-pam_regions[0]); for(i = 0; i 12; i++) { @@ -162,6 +163,7 @@ static void i440fx_update_memory_mappings(PCII440FXState *d) d-smram_enabled = false; } } +memory_region_transaction_commit(); } static void i440fx_set_smm(int val, void *arg) -- 1.7.5.3
Re: [Qemu-devel] [PATCH 2/2] Added target to build libvdisk
On Tue 23 Aug 2011 07:21:56 PM IDT, Anthony Liguori wrote: On 08/23/2011 11:18 AM, Daniel P. Berrange wrote: On Tue, Aug 23, 2011 at 11:14:20AM -0500, Anthony Liguori wrote: On 08/23/2011 11:12 AM, Daniel P. Berrange wrote: $(block-obj-y) pulls in 'aio.o' which is built from aio.c which is licensed GPLv2 only. So even those many files are BSD licenses, the combined work will be GPLv2-only. Unfortunately ending up with a libqemublock.so which is GPLv2-only is as good as useless for libs/apps since it is incompatible with both LGPLv2(+) and GPLv3. Now in this case aio.c is labelled as Copyright IBM / Anthony, so IBM could likely resolve this licensing to be more widely compatible. This could^H^Hwould become a non-trivial task if we need to look at many files then also any patches accepted to those files from 3rd parties over the years :-( If there was a block driver library, I would expect it to be GPL, not LGPL. This would prevent us from using it in libvirt, unless we wrote a helper program which we spawned anytime we wanted to use some functionality library :-( libvirtd is GPL, no? But QEMU is GPL. Libraries derived from QEMU will also be GPL. Regards, Anthony Liguori Regards, Daniel OK, I admit it was a pretty naive solution. But I always try to do the simplest solution first. The license issues can be solved later. (Having it as GPL later changing to LGPL if we can). As for the API I can create start a specialized API for the lib that uses the internal API. I can hide BlockDriverState and export it as an fd. int vdisk_open(path, format, flags) size_t vdisk_pread(fd, buf, size, offset) size_t vdisk_pwrite(fd, buff, size, offset) int vdisk_close(fd) int vdisk_get_size(fd) That way no internal structures are exported and we use a minimal set of functions that are very unlikely to change. There is no support for snapshots, metadata etc. But these APIs can be added later. And of-course we can always define the lib as experimental until the API stabilizes.
[Qemu-devel] [PATCH 19/24] lm32_boards: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/lm32_boards.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/lm32_boards.c b/hw/lm32_boards.c index 3c6b5af..0a9ad59 100644 --- a/hw/lm32_boards.c +++ b/hw/lm32_boards.c @@ -79,7 +79,7 @@ static void lm32_evr_init(MemoryRegion *address_space_mem, { CPUState *env; DriveInfo *dinfo; -ram_addr_t phys_ram; +MemoryRegion *phys_ram = g_new(MemoryRegion, 1); MemoryRegion *phys_flash = g_new(MemoryRegion, 1); qemu_irq *cpu_irq, irq[32]; ResetInfo *reset_info; @@ -108,8 +108,8 @@ static void lm32_evr_init(MemoryRegion *address_space_mem, reset_info-flash_base = flash_base; -phys_ram = qemu_ram_alloc(NULL, lm32_evr.sdram, ram_size); -cpu_register_physical_memory(ram_base, ram_size, phys_ram | IO_MEM_RAM); +memory_region_init_ram(phys_ram, NULL, lm32_evr.sdram, ram_size); +memory_region_add_subregion(address_space_mem, ram_base, phys_ram); memory_region_init_rom_device(phys_flash, pflash_cfi02_ops_be, NULL, lm32_evr.flash, flash_size); @@ -170,7 +170,7 @@ static void lm32_uclinux_init(MemoryRegion *address_space_mem, { CPUState *env; DriveInfo *dinfo; -ram_addr_t phys_ram; +MemoryRegion *phys_ram = g_new(MemoryRegion, 1); MemoryRegion *phys_flash = g_new(MemoryRegion, 1); qemu_irq *cpu_irq, irq[32]; HWSetup *hw; @@ -206,8 +206,8 @@ static void lm32_uclinux_init(MemoryRegion *address_space_mem, reset_info-flash_base = flash_base; -phys_ram = qemu_ram_alloc(NULL, lm32_uclinux.sdram, ram_size); -cpu_register_physical_memory(ram_base, ram_size, phys_ram | IO_MEM_RAM); +memory_region_init_ram(phys_ram, NULL, lm32_uclinux.sdram, ram_size); +memory_region_add_subregion(address_space_mem, ram_base, phys_ram); memory_region_init_rom_device(phys_flash, pflash_cfi01_ops_be, NULL, lm32_uclinux.flash, flash_size); -- 1.7.5.3
[Qemu-devel] [PATCH 03/24] sysbus: add helpers to add and delete memory regions to the system bus
Signed-off-by: Avi Kivity a...@redhat.com --- hw/sysbus.c | 22 ++ hw/sysbus.h |6 ++ 2 files changed, 28 insertions(+), 0 deletions(-) diff --git a/hw/sysbus.c b/hw/sysbus.c index f39768b..f5f0ed2 100644 --- a/hw/sysbus.c +++ b/hw/sysbus.c @@ -256,3 +256,25 @@ static char *sysbus_get_fw_dev_path(DeviceState *dev) return strdup(path); } + +void sysbus_add_memory(SysBusDevice *dev, target_phys_addr_t addr, + MemoryRegion *mem) +{ +memory_region_add_subregion(get_system_memory(), addr, mem); +} + +void sysbus_del_memory(SysBusDevice *dev, MemoryRegion *mem) +{ +memory_region_del_subregion(get_system_memory(), mem); +} + +void sysbus_add_io(SysBusDevice *dev, target_phys_addr_t addr, + MemoryRegion *mem) +{ +memory_region_add_subregion(get_system_io(), addr, mem); +} + +void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem) +{ +memory_region_del_subregion(get_system_io(), mem); +} diff --git a/hw/sysbus.h b/hw/sysbus.h index b87c6c5..e4d56cf 100644 --- a/hw/sysbus.h +++ b/hw/sysbus.h @@ -57,6 +57,12 @@ void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size); void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq); void sysbus_mmio_map(SysBusDevice *dev, int n, target_phys_addr_t addr); +void sysbus_add_memory(SysBusDevice *dev, target_phys_addr_t addr, + MemoryRegion *mem); +void sysbus_del_memory(SysBusDevice *dev, MemoryRegion *mem); +void sysbus_add_io(SysBusDevice *dev, target_phys_addr_t addr, + MemoryRegion *mem); +void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem); /* Legacy helper function for creating devices. */ DeviceState *sysbus_create_varargs(const char *name, -- 1.7.5.3
[Qemu-devel] [PATCH 06/24] QEMUMachine: pass address space to machine init function
Avoids get_system_memory() everywhere. Signed-off-by: Avi Kivity a...@redhat.com --- hw/an5206.c |4 ++- hw/axis_dev88.c |4 ++- hw/boards.h |5 +++- hw/collie.c |4 ++- hw/dummy_m68k.c |4 ++- hw/gumstix.c |8 +- hw/integratorcp.c |4 ++- hw/leon3.c|4 ++- hw/lm32_boards.c |8 +- hw/mainstone.c|4 ++- hw/mcf5208.c |4 ++- hw/milkymist.c|5 +++- hw/mips_fulong2e.c|4 ++- hw/mips_jazz.c|8 +- hw/mips_malta.c |4 ++- hw/mips_mipssim.c |4 ++- hw/mips_r4k.c |4 ++- hw/musicpal.c |4 ++- hw/nseries.c | 18 +++ hw/omap_sx1.c |8 +- hw/palm.c |4 ++- hw/pc_piix.c | 31 +- hw/petalogix_ml605_mmu.c |4 ++- hw/petalogix_s3adsp1800_mmu.c |4 ++- hw/ppc405_boards.c|8 +- hw/ppc440_bamboo.c|4 ++- hw/ppc_newworld.c |4 ++- hw/ppc_oldworld.c |4 ++- hw/ppc_prep.c |4 ++- hw/ppce500_mpc8544ds.c|4 ++- hw/r2d.c |4 ++- hw/realview.c | 33 --- hw/s390-virtio.c |4 ++- hw/shix.c |4 ++- hw/spitz.c| 32 +++--- hw/stellaris.c| 18 +++ hw/sun4m.c| 48 ++-- hw/sun4u.c| 12 +++-- hw/syborg.c |4 ++- hw/tosa.c |4 ++- hw/versatilepb.c | 19 hw/vexpress.c |4 ++- hw/virtex_ml507.c |4 ++- hw/xen_machine_pv.c |4 ++- hw/z2.c |4 ++- vl.c |3 +- 46 files changed, 282 insertions(+), 102 deletions(-) diff --git a/hw/an5206.c b/hw/an5206.c index 04ca420..e34de39 100644 --- a/hw/an5206.c +++ b/hw/an5206.c @@ -28,7 +28,9 @@ void irq_info(Monitor *mon) /* Board init. */ -static void an5206_init(ram_addr_t ram_size, +static void an5206_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) diff --git a/hw/axis_dev88.c b/hw/axis_dev88.c index 06200e2..cfc89c2 100644 --- a/hw/axis_dev88.c +++ b/hw/axis_dev88.c @@ -244,7 +244,9 @@ static CPUWriteMemoryFunc * const gpio_write[] = { static struct cris_load_info li; static -void axisdev88_init (ram_addr_t ram_size, +void axisdev88_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) diff --git a/hw/boards.h b/hw/boards.h index 716fd7b..5c12ce7 100644 --- a/hw/boards.h +++ b/hw/boards.h @@ -4,8 +4,11 @@ #define HW_BOARDS_H #include qdev.h +#include memory.h -typedef void QEMUMachineInitFunc(ram_addr_t ram_size, +typedef void QEMUMachineInitFunc(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, diff --git a/hw/collie.c b/hw/collie.c index 156404d..9b724aa 100644 --- a/hw/collie.c +++ b/hw/collie.c @@ -19,7 +19,9 @@ static struct arm_boot_info collie_binfo = { .ram_size = 0x2000, }; -static void collie_init(ram_addr_t ram_size, +static void collie_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) diff --git a/hw/dummy_m68k.c b/hw/dummy_m68k.c index eed9e38..2de69ad 100644 --- a/hw/dummy_m68k.c +++ b/hw/dummy_m68k.c @@ -15,7 +15,9 @@ /* Board init. */ -static void dummy_m68k_init(ram_addr_t ram_size, +static void dummy_m68k_init(MemoryRegion *address_space_mem, + MemoryRegion *address_space_io, + ram_addr_t
[Qemu-devel] [PATCH 05/24] ReadWriteHandler: remove
No longer used. Signed-off-by: Avi Kivity a...@redhat.com --- Makefile.target |1 - rwhandler.c | 87 --- rwhandler.h | 27 - 3 files changed, 0 insertions(+), 115 deletions(-) delete mode 100644 rwhandler.c delete mode 100644 rwhandler.h diff --git a/Makefile.target b/Makefile.target index e280bf6..c9957ae 100644 --- a/Makefile.target +++ b/Makefile.target @@ -195,7 +195,6 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o virt obj-y += vhost_net.o obj-$(CONFIG_VHOST_NET) += vhost.o obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p-device.o -obj-y += rwhandler.o obj-$(CONFIG_KVM) += kvm.o kvm-all.o obj-$(CONFIG_NO_KVM) += kvm-stub.o obj-y += memory.o diff --git a/rwhandler.c b/rwhandler.c deleted file mode 100644 index bb2238f..000 --- a/rwhandler.c +++ /dev/null @@ -1,87 +0,0 @@ -#include rwhandler.h -#include ioport.h -#include cpu-all.h - -#define RWHANDLER_WRITE(name, len, type) \ -static void name(void *opaque, type addr, uint32_t value) \ -{\ -struct ReadWriteHandler *handler = opaque;\ -handler-write(handler, addr, value, len);\ -} - -#define RWHANDLER_READ(name, len, type) \ -static uint32_t name(void *opaque, type addr) \ -{ \ -struct ReadWriteHandler *handler = opaque; \ -return handler-read(handler, addr, len); \ -} - -RWHANDLER_WRITE(cpu_io_memory_simple_writeb, 1, target_phys_addr_t); -RWHANDLER_READ(cpu_io_memory_simple_readb, 1, target_phys_addr_t); -RWHANDLER_WRITE(cpu_io_memory_simple_writew, 2, target_phys_addr_t); -RWHANDLER_READ(cpu_io_memory_simple_readw, 2, target_phys_addr_t); -RWHANDLER_WRITE(cpu_io_memory_simple_writel, 4, target_phys_addr_t); -RWHANDLER_READ(cpu_io_memory_simple_readl, 4, target_phys_addr_t); - -static CPUWriteMemoryFunc * const cpu_io_memory_simple_write[] = { -cpu_io_memory_simple_writeb, -cpu_io_memory_simple_writew, -cpu_io_memory_simple_writel, -}; - -static CPUReadMemoryFunc * const cpu_io_memory_simple_read[] = { -cpu_io_memory_simple_readb, -cpu_io_memory_simple_readw, -cpu_io_memory_simple_readl, -}; - -int cpu_register_io_memory_simple(struct ReadWriteHandler *handler, int endian) -{ -if (!handler-read || !handler-write) { -return -1; -} -return cpu_register_io_memory(cpu_io_memory_simple_read, - cpu_io_memory_simple_write, - handler, endian); -} - -RWHANDLER_WRITE(ioport_simple_writeb, 1, uint32_t); -RWHANDLER_READ(ioport_simple_readb, 1, uint32_t); -RWHANDLER_WRITE(ioport_simple_writew, 2, uint32_t); -RWHANDLER_READ(ioport_simple_readw, 2, uint32_t); -RWHANDLER_WRITE(ioport_simple_writel, 4, uint32_t); -RWHANDLER_READ(ioport_simple_readl, 4, uint32_t); - -int register_ioport_simple(ReadWriteHandler* handler, - pio_addr_t start, int length, int size) -{ -IOPortWriteFunc *write; -IOPortReadFunc *read; -int r; -switch (size) { -case 1: -write = ioport_simple_writeb; -read = ioport_simple_readb; -break; -case 2: -write = ioport_simple_writew; -read = ioport_simple_readw; -break; -default: -write = ioport_simple_writel; -read = ioport_simple_readl; -} -if (handler-write) { -r = register_ioport_write(start, length, size, write, handler); -if (r 0) { -return r; -} -} -if (handler-read) { -r = register_ioport_read(start, length, size, read, handler); -if (r 0) { -return r; -} -} -return 0; -} diff --git a/rwhandler.h b/rwhandler.h deleted file mode 100644 index b2a5790..000 --- a/rwhandler.h +++ /dev/null @@ -1,27 +0,0 @@ -#ifndef READ_WRITE_HANDLER_H -#define READ_WRITE_HANDLER_H - -#include qemu-common.h -#include ioport.h - -typedef struct ReadWriteHandler ReadWriteHandler; - -/* len is guaranteed to be one of 1, 2 or 4, addr is guaranteed to fit in an - * appropriate type (io/memory/etc). They do not need to be range checked. */ -typedef void WriteHandlerFunc(ReadWriteHandler *, pcibus_t addr, - uint32_t value, int len); -typedef uint32_t ReadHandlerFunc(ReadWriteHandler *, pcibus_t addr, int len); - -struct ReadWriteHandler { -WriteHandlerFunc *write; -ReadHandlerFunc *read; -}; - -/* Helpers for when we want to use a single routine with length. */ -/* CPU memory handler: both read and write must be present. */ -int cpu_register_io_memory_simple(ReadWriteHandler *, int endian); -/* io port handler: can supply only read or write handlers. */ -int register_ioport_simple(ReadWriteHandler *, - pio_addr_t start, int length, int size); - -#endif -- 1.7.5.3
Re: [Qemu-devel] [PATCH 21/24] mcf5208: convert to memory API
On 24 August 2011 11:47, Avi Kivity a...@redhat.com wrote: On 08/24/2011 01:35 PM, Peter Maydell wrote: On 24 August 2011 11:11, Avi Kivitya...@redhat.com wrote: diff --git a/hw/mcf5208.c b/hw/mcf5208.c index f4d4fbd..73c6961 100644 --- a/hw/mcf5208.c +++ b/hw/mcf5208.c @@ -5,6 +5,7 @@ * * This code is licensed under the GPL */ +#includeglib.h #include hw.h #include mcf.h #include qemu-timer.h You shouldn't need to include glib.h here, I think? hw.h includes qemu-common.h which includes glib.h. My preference is not to depend on indirect includes; but I can remove this include if it's disliked by many. qemu-common.h says /* we put basic includes here to avoid repeating them in device drivers */ so direct includes of system headers in hw/ files is going against current practice. I don't feel strongly either way on the question of direct vs indirect, but we should be consistent, and the choice we've made is rely on qemu-common.h. -- PMM
[Qemu-devel] [PATCH 24/24] milkymist: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/milkymist.c |7 +++ 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/hw/milkymist.c b/hw/milkymist.c index 74994a7..09de681 100644 --- a/hw/milkymist.c +++ b/hw/milkymist.c @@ -85,7 +85,7 @@ milkymist_init(MemoryRegion *address_space_mem, CPUState *env; int kernel_size; DriveInfo *dinfo; -ram_addr_t phys_sdram; +MemoryRegion *phys_sdram = g_new(MemoryRegion, 1); MemoryRegion *phys_flash = g_new(MemoryRegion, 1); qemu_irq irq[32], *cpu_irq; int i; @@ -113,9 +113,8 @@ milkymist_init(MemoryRegion *address_space_mem, cpu_lm32_set_phys_msb_ignore(env, 1); -phys_sdram = qemu_ram_alloc(NULL, milkymist.sdram, sdram_size); -cpu_register_physical_memory(sdram_base, sdram_size, -phys_sdram | IO_MEM_RAM); +memory_region_init_ram(phys_sdram, NULL, milkymist.sdram, sdram_size); +memory_region_add_subregion(address_space_mem, sdram_base, phys_sdram); memory_region_init_rom_device(phys_flash, pflash_cfi01_ops_be, NULL, milkymist.flash, flash_size); -- 1.7.5.3
[Qemu-devel] [PATCH 11/24] integratorcp: convert to memory API (RAM/flash only)
Signed-off-by: Avi Kivity a...@redhat.com --- hw/integratorcp.c | 28 1 files changed, 20 insertions(+), 8 deletions(-) diff --git a/hw/integratorcp.c b/hw/integratorcp.c index 3d4b339..720cc02 100644 --- a/hw/integratorcp.c +++ b/hw/integratorcp.c @@ -7,6 +7,8 @@ * This code is licensed under the GPL */ +#include glib.h + #include sysbus.h #include primecell.h #include devices.h @@ -17,7 +19,8 @@ typedef struct { SysBusDevice busdev; uint32_t memsz; -uint32_t flash_offset; +MemoryRegion flash; +bool flash_mapped; uint32_t cm_osc; uint32_t cm_ctrl; uint32_t cm_lock; @@ -108,9 +111,15 @@ static uint32_t integratorcm_read(void *opaque, target_phys_addr_t offset) static void integratorcm_do_remap(integratorcm_state *s, int flash) { if (flash) { -cpu_register_physical_memory(0, 0x10, IO_MEM_RAM); +if (s-flash_mapped) { +sysbus_del_memory(s-busdev, s-flash); +s-flash_mapped = false; +} } else { -cpu_register_physical_memory(0, 0x10, s-flash_offset | IO_MEM_RAM); +if (!s-flash_mapped) { +sysbus_add_memory_overlap(s-busdev, 0, s-flash, 1); +s-flash_mapped = true; +} } //??? tlb_flush (cpu_single_env, 1); } @@ -252,7 +261,8 @@ static int integratorcm_init(SysBusDevice *dev) } memcpy(integrator_spd + 73, QEMU-MEMORY, 11); s-cm_init = 0x0112; -s-flash_offset = qemu_ram_alloc(NULL, integrator.flash, 0x10); +memory_region_init_ram(s-flash, NULL, integrator.flash, 0x10); +s-flash_mapped = false; iomemtype = cpu_register_io_memory(integratorcm_readfn, integratorcm_writefn, s, @@ -458,7 +468,8 @@ static void integratorcp_init(MemoryRegion *address_space_mem, const char *initrd_filename, const char *cpu_model) { CPUState *env; -ram_addr_t ram_offset; +MemoryRegion *ram = g_new(MemoryRegion, 1); +MemoryRegion *ram_alias = g_new(MemoryRegion, 1); qemu_irq pic[32]; qemu_irq *cpu_pic; DeviceState *dev; @@ -471,13 +482,14 @@ static void integratorcp_init(MemoryRegion *address_space_mem, fprintf(stderr, Unable to find CPU definition\n); exit(1); } -ram_offset = qemu_ram_alloc(NULL, integrator.ram, ram_size); +memory_region_init_ram(ram, NULL, integrator.ram, ram_size); /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash. */ /* ??? RAM should repeat to fill physical memory space. */ /* SDRAM at address zero*/ -cpu_register_physical_memory(0, ram_size, ram_offset | IO_MEM_RAM); +memory_region_add_subregion(address_space_mem, 0, ram); /* And again at address 0x8000 */ -cpu_register_physical_memory(0x8000, ram_size, ram_offset | IO_MEM_RAM); +memory_region_init_alias(ram_alias, ram.alias, ram, 0, ram_size); +memory_region_add_subregion(address_space_mem, 0x8000, ram_alias); dev = qdev_create(NULL, integrator_core); qdev_prop_set_uint32(dev, memsz, ram_size 20); -- 1.7.5.3
[Qemu-devel] [PATCH 07/24] an5206: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/an5206.c | 12 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/an5206.c b/hw/an5206.c index e34de39..9a91984 100644 --- a/hw/an5206.c +++ b/hw/an5206.c @@ -6,6 +6,8 @@ * This code is licensed under the GPL */ +#include glib.h + #include hw.h #include pc.h #include mcf.h @@ -39,6 +41,8 @@ static void an5206_init(MemoryRegion *address_space_mem, int kernel_size; uint64_t elf_entry; target_phys_addr_t entry; +MemoryRegion *ram = g_new(MemoryRegion, 1); +MemoryRegion *sram = g_new(MemoryRegion, 1); if (!cpu_model) cpu_model = m5206; @@ -54,12 +58,12 @@ static void an5206_init(MemoryRegion *address_space_mem, env-rambar0 = AN5206_RAMBAR_ADDR | 1; /* DRAM at address zero */ -cpu_register_physical_memory(0, ram_size, -qemu_ram_alloc(NULL, an5206.ram, ram_size) | IO_MEM_RAM); +memory_region_init_ram(ram, NULL, an5206.ram, ram_size); +memory_region_add_subregion(address_space_mem, 0, ram); /* Internal SRAM. */ -cpu_register_physical_memory(AN5206_RAMBAR_ADDR, 512, -qemu_ram_alloc(NULL, an5206.sram, 512) | IO_MEM_RAM); +memory_region_init_ram(sram, NULL, an5206.sram, 512); +memory_region_add_subregion(address_space_mem, AN5206_RAMBAR_ADDR, sram); mcf5206_init(AN5206_MBAR_ADDR, env); -- 1.7.5.3
Re: [Qemu-devel] [PATCH 21/24] mcf5208: convert to memory API
On 08/24/2011 02:38 PM, Peter Maydell wrote: On 24 August 2011 11:47, Avi Kivitya...@redhat.com wrote: On 08/24/2011 01:35 PM, Peter Maydell wrote: On 24 August 2011 11:11, Avi Kivitya...@redhat.comwrote: diff --git a/hw/mcf5208.c b/hw/mcf5208.c index f4d4fbd..73c6961 100644 --- a/hw/mcf5208.c +++ b/hw/mcf5208.c @@ -5,6 +5,7 @@ * * This code is licensed under the GPL */ +#includeglib.h #include hw.h #include mcf.h #include qemu-timer.h You shouldn't need to include glib.h here, I think? hw.h includes qemu-common.h which includes glib.h. My preference is not to depend on indirect includes; but I can remove this include if it's disliked by many. qemu-common.h says /* we put basic includes here to avoid repeating them in device drivers */ so direct includes of system headers in hw/ files is going against current practice. I don't feel strongly either way on the question of direct vs indirect, but we should be consistent, and the choice we've made is rely on qemu-common.h. Okay, I'll drop the include (here and elsewhere). -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
[Qemu-devel] [PATCH 09/24] axis_dev88: convert to memory API (RAM only)
Signed-off-by: Avi Kivity a...@redhat.com --- hw/axis_dev88.c | 16 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/axis_dev88.c b/hw/axis_dev88.c index cfc89c2..04a3d5b 100644 --- a/hw/axis_dev88.c +++ b/hw/axis_dev88.c @@ -22,6 +22,8 @@ * THE SOFTWARE. */ +#include glib.h + #include sysbus.h #include net.h #include flash.h @@ -261,8 +263,8 @@ void axisdev88_init (MemoryRegion *address_space_mem, int i; int nand_regs; int gpio_regs; -ram_addr_t phys_ram; -ram_addr_t phys_intmem; +MemoryRegion *phys_ram = g_new(MemoryRegion, 1); +MemoryRegion *phys_intmem = g_new(MemoryRegion, 1); /* init CPUs */ if (cpu_model == NULL) { @@ -271,15 +273,13 @@ void axisdev88_init (MemoryRegion *address_space_mem, env = cpu_init(cpu_model); /* allocate RAM */ -phys_ram = qemu_ram_alloc(NULL, axisdev88.ram, ram_size); -cpu_register_physical_memory(0x4000, ram_size, phys_ram | IO_MEM_RAM); +memory_region_init_ram(phys_ram, NULL, axisdev88.ram, ram_size); +memory_region_add_subregion(address_space_mem, 0x4000, phys_ram); /* The ETRAX-FS has 128Kb on chip ram, the docs refer to it as the internal memory. */ -phys_intmem = qemu_ram_alloc(NULL, axisdev88.chipram, INTMEM_SIZE); -cpu_register_physical_memory(0x3800, INTMEM_SIZE, - phys_intmem | IO_MEM_RAM); - +memory_region_init_ram(phys_intmem, NULL, axisdev88.chipram, INTMEM_SIZE); +memory_region_add_subregion(address_space_mem, 0x3800, phys_intmem); /* Attach a NAND flash to CS1. */ nand = drive_get(IF_MTD, 0, 0); -- 1.7.5.3
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions
On Wed, Aug 24, 2011 at 12:10:32PM +0200, Jan Kiszka wrote: On 2011-08-24 12:04, Michael S. Tsirkin wrote: On Tue, Aug 23, 2011 at 07:28:08PM +0200, Jan Kiszka wrote: From: Alex Williamson alex.william...@redhat.com Nothing good can happen when we overlap capabilities [ Jan: rebased over qemu, minor formatting ] Signed-off-by: Jan Kiszka jan.kis...@siemens.com This doesn't build for me: /scm/qemu/hw/pci.c: In function ‘pci_add_capability’: /scm/qemu/hw/pci.c:1970:45: error: ‘PCIDevice’ has no member named ‘config_map’ Yeah, sorry, forgot to refresh the commit before posting. I think that what that includes is the capability including each given offset, right? It would be easy to write some code scanning the capability list to figure this value out. Something along the lines of (untested): static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) { uint8_t next, prev, found = 0; if (!(pdev-config[PCI_STATUS] PCI_STATUS_CAP_LIST)) return 0; for (prev = PCI_CAPABILITY_LIST; (next = pdev-config[prev]); prev = next + PCI_CAP_LIST_NEXT) if (next = offset next found) found = next; return found; } Sounds useful, will enhance the patch. (Originally, I just wanted to reduce the qemu-kvm delta... :) ) Jan Also, let's add a comment documenting the reason for this check: device assignment depends on this check to verify that the device is not broken. -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH] vga: squash build error in vga_update_memory_access()
Newer gcc complains that base and size may be used uninitialized, even though it is clearly a false warning. Silence the warning by indicating to gcc that the code path triggering the warning cannot happen. Signed-off-by: Avi Kivity a...@redhat.com --- hw/vga.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/vga.c b/hw/vga.c index 851fd68..b74e6e8 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -179,6 +179,8 @@ static void vga_update_memory_access(VGACommonState *s) base = 0xb8000; size = 0x8000; break; +default: +abort(); } region = g_malloc(sizeof(*region)); memory_region_init_alias(region, vga.chain4, s-vram, offset, size); -- 1.7.5.3
Re: [Qemu-devel] [PATCH] qemu-coroutine: Add simple work queue support
On Wed, Aug 24, 2011 at 05:57:51PM +1000, Peter A. G. Crosthwaite wrote: Add a function co_queue_yield_to_next() which will immediately transfer control to the coroutine at the head of a co queue. This can be used for implementing simple work queues where the manager of a co-queue only needs to restart queued routines one at a time. Signed-off-by: Peter A. G. Crosthwaite peter.crosthwa...@petalogix.com --- qemu-coroutine-lock.c | 13 + qemu-coroutine.h |9 + 2 files changed, 22 insertions(+), 0 deletions(-) Please share code that uses this function, even if it's not suitable for merging. I'd like to understand what the pattern for using this function is. diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c index a80f437..de2fc21 100644 --- a/qemu-coroutine-lock.c +++ b/qemu-coroutine-lock.c @@ -75,6 +75,19 @@ bool qemu_co_queue_next(CoQueue *queue) return (next != NULL); } +bool qemu_co_queue_yield_to_next(CoQueue *queue) This function can only be executed in coroutine context (i.e. it cannot be executed outside a coroutine). Therefore please use the coroutine_fn annotation. Perhaps we'll hook the annotation up with a checker in the future to ensure that non-coroutine_fn functions never call coroutine_fn functions statically at build time. In the meantime it serves as documentation. Stefan
Re: [Qemu-devel] [PATCH 06/24] QEMUMachine: pass address space to machine init function
On 08/24/2011 02:18 PM, Avi Kivity wrote: On 08/24/2011 01:53 PM, Peter Maydell wrote: The purpose here is to allow removal of get_system_memory() from the general code base. The right way to remove get_system_memory() from the general code base is to actually model things correctly, for instance by having machine models create a container memory region into which they insert the memory regions for all the devices which currently use sysbus_mmio_map to map themselves, and then pass the container memory region to the master end of the bus, ie the CPU. I think you're right. This also allows eventual removal of system_io on anything non-x86. So a replacement would look like: (before) -static void pc_init_isa(ram_addr_t ram_size, +static void pc_init_isa(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, @@ -259,15 +265,17 @@ static void pc_init_isa(ram_addr_t ram_size, { if (cpu_model == NULL) cpu_model = 486; -pc_init1(get_system_memory(), - get_system_io(), +pc_init1(address_space_mem, + address_space_io, ram_size, boot_device, kernel_filename, kernel_cmdline, initrd_filename, cpu_model, 0, 1); } (after) @@ -259,15 +265,17 @@ static void pc_init_isa(ram_addr_t ram_size, { +MemoryRegion *address_space_mem, *address_space_io; + +setup_system_memory(address_space_mem,address_space_io); if (cpu_model == NULL) cpu_model = 486; -pc_init1(get_system_memory(), - get_system_io(), +pc_init1(address_space_mem, + address_space_io, ram_size, boot_device, kernel_filename, kernel_cmdline, initrd_filename, cpu_model, 0, 1); } Later on, we'd refine the setup_system_memory() calls, for example not to create the io space on non-x86. A possible complication is whether anything currently uses system_memory before -init is called. Anyone know? Okay, it looks like I can just drop the patch and call get_system_memory() in the various _init functions. This avoids the complication. We can tidy up get_system_memory() later. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
[Qemu-devel] [PATCH v2] pci: Error on PCI capability collisions
Nothing good can happen when we overlap capabilities. This may happen when plugging in assigned devices or when devices models contain bugs. Detect the overlap and report it. Based on qemu-kvm commit by Alex Williamson. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/pci.c | 34 ++ 1 files changed, 34 insertions(+), 0 deletions(-) Changes in v2: - properly find the conflicting CAP - adjust error code diff --git a/hw/pci.c b/hw/pci.c index 6124790..634e789 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1811,6 +1811,25 @@ static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id, return next; } +static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) +{ +uint8_t next, prev, found = 0; + +if (!(pdev-used[offset])) { +return 0; +} + +assert(pdev-config[PCI_STATUS] PCI_STATUS_CAP_LIST); + +for (prev = PCI_CAPABILITY_LIST; (next = pdev-config[prev]); + prev = next + PCI_CAP_LIST_NEXT) { +if (next = offset next found) { +found = next; +} +} +return found; +} + /* Patch the PCI vendor and device ids in a PCI rom image if necessary. This is needed for an option rom which is used for more than one device. */ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size) @@ -1952,11 +1971,26 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t offset, uint8_t size) { uint8_t *config; +int i, overlapping_cap; + if (!offset) { offset = pci_find_space(pdev, size); if (!offset) { return -ENOSPC; } +} else { +for (i = offset; i offset + size; i++) { +overlapping_cap = pci_find_capability_at_offset(pdev, i); +if (overlapping_cap) { +fprintf(stderr, ERROR: %04x:%02x:%02x.%x +Attempt to add PCI capability %x at offset +%x overlaps existing capability %x at offset %x\n, +pci_find_domain(pdev-bus), pci_bus_num(pdev-bus), +PCI_SLOT(pdev-devfn), PCI_FUNC(pdev-devfn), +cap_id, offset, overlapping_cap, i); +return -EINVAL; +} +} } config = pdev-config + offset; -- 1.7.3.4
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions
On 2011-08-24 13:58, Michael S. Tsirkin wrote: On Wed, Aug 24, 2011 at 12:10:32PM +0200, Jan Kiszka wrote: On 2011-08-24 12:04, Michael S. Tsirkin wrote: On Tue, Aug 23, 2011 at 07:28:08PM +0200, Jan Kiszka wrote: From: Alex Williamson alex.william...@redhat.com Nothing good can happen when we overlap capabilities [ Jan: rebased over qemu, minor formatting ] Signed-off-by: Jan Kiszka jan.kis...@siemens.com This doesn't build for me: /scm/qemu/hw/pci.c: In function ‘pci_add_capability’: /scm/qemu/hw/pci.c:1970:45: error: ‘PCIDevice’ has no member named ‘config_map’ Yeah, sorry, forgot to refresh the commit before posting. I think that what that includes is the capability including each given offset, right? It would be easy to write some code scanning the capability list to figure this value out. Something along the lines of (untested): static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) { uint8_t next, prev, found = 0; if (!(pdev-config[PCI_STATUS] PCI_STATUS_CAP_LIST)) return 0; for (prev = PCI_CAPABILITY_LIST; (next = pdev-config[prev]); prev = next + PCI_CAP_LIST_NEXT) if (next = offset next found) found = next; return found; } Sounds useful, will enhance the patch. (Originally, I just wanted to reduce the qemu-kvm delta... :) ) Jan Also, let's add a comment documenting the reason for this check: device assignment depends on this check to verify that the device is not broken. Based on the previous discussion, I don't think this is accurate as it will also validate emulated devices. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions
On Wed, Aug 24, 2011 at 02:29:36PM +0200, Jan Kiszka wrote: On 2011-08-24 13:58, Michael S. Tsirkin wrote: On Wed, Aug 24, 2011 at 12:10:32PM +0200, Jan Kiszka wrote: On 2011-08-24 12:04, Michael S. Tsirkin wrote: On Tue, Aug 23, 2011 at 07:28:08PM +0200, Jan Kiszka wrote: From: Alex Williamson alex.william...@redhat.com Nothing good can happen when we overlap capabilities [ Jan: rebased over qemu, minor formatting ] Signed-off-by: Jan Kiszka jan.kis...@siemens.com This doesn't build for me: /scm/qemu/hw/pci.c: In function ‘pci_add_capability’: /scm/qemu/hw/pci.c:1970:45: error: ‘PCIDevice’ has no member named ‘config_map’ Yeah, sorry, forgot to refresh the commit before posting. I think that what that includes is the capability including each given offset, right? It would be easy to write some code scanning the capability list to figure this value out. Something along the lines of (untested): static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) { uint8_t next, prev, found = 0; if (!(pdev-config[PCI_STATUS] PCI_STATUS_CAP_LIST)) return 0; for (prev = PCI_CAPABILITY_LIST; (next = pdev-config[prev]); prev = next + PCI_CAP_LIST_NEXT) if (next = offset next found) found = next; return found; } Sounds useful, will enhance the patch. (Originally, I just wanted to reduce the qemu-kvm delta... :) ) Jan Also, let's add a comment documenting the reason for this check: device assignment depends on this check to verify that the device is not broken. Based on the previous discussion, I don't think this is accurate as it will also validate emulated devices. Jan Something like the below is accurate, right? /* Device assignment depends on this check to verify that the device is not broken. Should never trigger for emulated devices, but it's helpful for debugging these. */ -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions
On 2011-08-24 14:34, Michael S. Tsirkin wrote: On Wed, Aug 24, 2011 at 02:29:36PM +0200, Jan Kiszka wrote: On 2011-08-24 13:58, Michael S. Tsirkin wrote: On Wed, Aug 24, 2011 at 12:10:32PM +0200, Jan Kiszka wrote: On 2011-08-24 12:04, Michael S. Tsirkin wrote: On Tue, Aug 23, 2011 at 07:28:08PM +0200, Jan Kiszka wrote: From: Alex Williamson alex.william...@redhat.com Nothing good can happen when we overlap capabilities [ Jan: rebased over qemu, minor formatting ] Signed-off-by: Jan Kiszka jan.kis...@siemens.com This doesn't build for me: /scm/qemu/hw/pci.c: In function ‘pci_add_capability’: /scm/qemu/hw/pci.c:1970:45: error: ‘PCIDevice’ has no member named ‘config_map’ Yeah, sorry, forgot to refresh the commit before posting. I think that what that includes is the capability including each given offset, right? It would be easy to write some code scanning the capability list to figure this value out. Something along the lines of (untested): static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) { uint8_t next, prev, found = 0; if (!(pdev-config[PCI_STATUS] PCI_STATUS_CAP_LIST)) return 0; for (prev = PCI_CAPABILITY_LIST; (next = pdev-config[prev]); prev = next + PCI_CAP_LIST_NEXT) if (next = offset next found) found = next; return found; } Sounds useful, will enhance the patch. (Originally, I just wanted to reduce the qemu-kvm delta... :) ) Jan Also, let's add a comment documenting the reason for this check: device assignment depends on this check to verify that the device is not broken. Based on the previous discussion, I don't think this is accurate as it will also validate emulated devices. Jan Something like the below is accurate, right? /* Device assignment depends on this check to verify that the device is not broken. Should never trigger for emulated devices, but it's helpful for debugging these. */ I've expressed this in the commit message. Unless there is another reason to do v3, maybe you can merge the comment on commit. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 1/2] pc: make vgabios exit port more useful
On 08/24/2011 04:47 AM, Avi Kivity wrote: On 08/08/2011 10:31 PM, Anthony Liguori wrote: We've always listened on port 501 for vgabios panic messages. In the entire time I've worked on QEMU, I've never actually seen a vgabios panic message :-) If we change the semantics of this port a little bit, it makes it possible to use it for more interesting use-cases. I chose this approach instead of adding a new I/O port because it avoids having a guest visible change. This change allows single-byte access to port 501 and also uses the value written to construct an exit code. A little late to review but... diff --git a/hw/pc.c b/hw/pc.c index 1c9d89a..4b07b35 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -549,8 +549,7 @@ static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val) /* LGPL'ed VGA BIOS messages */ case 0x501: case 0x502: - fprintf(stderr, VGA BIOS panic, line %d\n, val); - exit(1); + exit((val 1) | 1); This code (before the patch) circumvents -no-shutdown. Indeed. I believe that's a feature though? Shifting val left is surprising. What's wrong with even exit codes? Hrm, the '| 1' is wrong. My intention was to make the bottom bit of the exit code mean if set to 0, this is an exit coming from a guest'. I'll spin a patch. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions
On Wed, Aug 24, 2011 at 02:36:31PM +0200, Jan Kiszka wrote: On 2011-08-24 14:34, Michael S. Tsirkin wrote: On Wed, Aug 24, 2011 at 02:29:36PM +0200, Jan Kiszka wrote: On 2011-08-24 13:58, Michael S. Tsirkin wrote: On Wed, Aug 24, 2011 at 12:10:32PM +0200, Jan Kiszka wrote: On 2011-08-24 12:04, Michael S. Tsirkin wrote: On Tue, Aug 23, 2011 at 07:28:08PM +0200, Jan Kiszka wrote: From: Alex Williamson alex.william...@redhat.com Nothing good can happen when we overlap capabilities [ Jan: rebased over qemu, minor formatting ] Signed-off-by: Jan Kiszka jan.kis...@siemens.com This doesn't build for me: /scm/qemu/hw/pci.c: In function ‘pci_add_capability’: /scm/qemu/hw/pci.c:1970:45: error: ‘PCIDevice’ has no member named ‘config_map’ Yeah, sorry, forgot to refresh the commit before posting. I think that what that includes is the capability including each given offset, right? It would be easy to write some code scanning the capability list to figure this value out. Something along the lines of (untested): static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) { uint8_t next, prev, found = 0; if (!(pdev-config[PCI_STATUS] PCI_STATUS_CAP_LIST)) return 0; for (prev = PCI_CAPABILITY_LIST; (next = pdev-config[prev]); prev = next + PCI_CAP_LIST_NEXT) if (next = offset next found) found = next; return found; } Sounds useful, will enhance the patch. (Originally, I just wanted to reduce the qemu-kvm delta... :) ) Jan Also, let's add a comment documenting the reason for this check: device assignment depends on this check to verify that the device is not broken. Based on the previous discussion, I don't think this is accurate as it will also validate emulated devices. Jan Something like the below is accurate, right? /* Device assignment depends on this check to verify that the device is not broken. Should never trigger for emulated devices, but it's helpful for debugging these. */ I've expressed this in the commit message. Unless there is another reason to do v3, maybe you can merge the comment on commit. Jan Sure, I can do that, no need with v3. You are fine with the way it's formulated? -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions
On 2011-08-24 14:39, Michael S. Tsirkin wrote: On Wed, Aug 24, 2011 at 02:36:31PM +0200, Jan Kiszka wrote: On 2011-08-24 14:34, Michael S. Tsirkin wrote: On Wed, Aug 24, 2011 at 02:29:36PM +0200, Jan Kiszka wrote: On 2011-08-24 13:58, Michael S. Tsirkin wrote: On Wed, Aug 24, 2011 at 12:10:32PM +0200, Jan Kiszka wrote: On 2011-08-24 12:04, Michael S. Tsirkin wrote: On Tue, Aug 23, 2011 at 07:28:08PM +0200, Jan Kiszka wrote: From: Alex Williamson alex.william...@redhat.com Nothing good can happen when we overlap capabilities [ Jan: rebased over qemu, minor formatting ] Signed-off-by: Jan Kiszka jan.kis...@siemens.com This doesn't build for me: /scm/qemu/hw/pci.c: In function ‘pci_add_capability’: /scm/qemu/hw/pci.c:1970:45: error: ‘PCIDevice’ has no member named ‘config_map’ Yeah, sorry, forgot to refresh the commit before posting. I think that what that includes is the capability including each given offset, right? It would be easy to write some code scanning the capability list to figure this value out. Something along the lines of (untested): static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) { uint8_t next, prev, found = 0; if (!(pdev-config[PCI_STATUS] PCI_STATUS_CAP_LIST)) return 0; for (prev = PCI_CAPABILITY_LIST; (next = pdev-config[prev]); prev = next + PCI_CAP_LIST_NEXT) if (next = offset next found) found = next; return found; } Sounds useful, will enhance the patch. (Originally, I just wanted to reduce the qemu-kvm delta... :) ) Jan Also, let's add a comment documenting the reason for this check: device assignment depends on this check to verify that the device is not broken. Based on the previous discussion, I don't think this is accurate as it will also validate emulated devices. Jan Something like the below is accurate, right? /* Device assignment depends on this check to verify that the device is not broken. Should never trigger for emulated devices, but it's helpful for debugging these. */ I've expressed this in the commit message. Unless there is another reason to do v3, maybe you can merge the comment on commit. Jan Sure, I can do that, no need with v3. You are fine with the way it's formulated? Yep. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH STABLE-0.14/0.15/master] CVE-2011-0011: fix VNC password change to not touch authentication settings
On 08/24/2011 06:01 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com In CVE-2011-0011 it was noted that setting an empty password would disable all authentication for the VNC password. Commit 1cd20f8bf0ecb9d1d1bd5e2ffab3b88835380c9b attempted to fix this but it just broke it in a different way, because now instead of blindly disabling all authentication, it blindly resets all authentication to 'VNC'. But this is *not* a security problem. Login becomes disabled as expected. We should really not overload the semantics of the change command like this and instead introduce a new QMP operation to disable login. This disables any TLS auth that might have been enabled, which is pratically as bad as the original problem. eg, consider launching QEMU with TLS + password as per the docs section 3.11.5 $ qemu [...OPTIONS...] -vnc :1,password,tls -monitor stdio (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vencrypt+tls+vnc Client: none (qemu) change vnc password 123 (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vnc Client: none After setting the password, the TLS auth has been disabled meaning all communications are back in cleartext. The 'change vnc password' command must *never* touch the 'vs-auth' field under any circumstances. Similarly setting the password to (which causes all auth attempts to fail) must *not* touch vs-auth, otherwise it breaks the following sequence $ qemu [...OPTIONS...] -vnc :1,password,tls -monitor stdio (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vencrypt+tls+vnc Client: none (qemu) change vnc password 123 (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vencrypt+tls+vnc Client: none (qemu) change vnc password (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vnc Client: none (qemu) change vnc password 456 (qemu) info vnc Server: address: 0.0.0.0:5901 auth: vnc Client: none This patch puts the behaviour back to what it was before the original mistaken commit 52c18be9e99dabe295321153fda7fce9f76647ac * ui/vnc.c: Do not touch 'vs-auth' when changing password and remove unneccessary 'vnc_disable_login' method * monitor.c: Remove call to 'vnc_disable_login' Signed-off-by: Daniel P. Berrangeberra...@redhat.com --- console.h |1 - monitor.c |8 ui/vnc.c | 30 +++--- 3 files changed, 3 insertions(+), 36 deletions(-) diff --git a/console.h b/console.h index 67d1373..2eb03a1 100644 --- a/console.h +++ b/console.h @@ -373,7 +373,6 @@ void vnc_display_init(DisplayState *ds); void vnc_display_close(DisplayState *ds); int vnc_display_open(DisplayState *ds, const char *display); void vnc_display_add_client(DisplayState *ds, int csock, int skipauth); -int vnc_display_disable_login(DisplayState *ds); char *vnc_display_local_addr(DisplayState *ds); #ifdef CONFIG_VNC int vnc_display_password(DisplayState *ds, const char *password); diff --git a/monitor.c b/monitor.c index 1b8ba2c..59af05a 100644 --- a/monitor.c +++ b/monitor.c @@ -1023,14 +1023,6 @@ static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data) #ifdef CONFIG_VNC static int change_vnc_password(const char *password) { -if (!password || !password[0]) { -if (vnc_display_disable_login(NULL)) { -qerror_report(QERR_SET_PASSWD_FAILED); -return -1; -} -return 0; -} - if (vnc_display_password(NULL, password) 0) { qerror_report(QERR_SET_PASSWD_FAILED); return -1; diff --git a/ui/vnc.c b/ui/vnc.c index f1e27d9..f7fc7d2 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -2635,24 +2635,6 @@ void vnc_display_close(DisplayState *ds) #endif } -int vnc_display_disable_login(DisplayState *ds) -{ -VncDisplay *vs = ds ? (VncDisplay *)ds-opaque : vnc_display; - -if (!vs) { -return -1; -} - -if (vs-password) { -qemu_free(vs-password); -} - -vs-password = NULL; -vs-auth = VNC_AUTH_VNC; - -return 0; -} - int vnc_display_password(DisplayState *ds, const char *password) { int ret = 0; @@ -2663,19 +2645,13 @@ int vnc_display_password(DisplayState *ds, const char *password) goto out; } -if (!password) { -/* This is not the intention of this interface but err on the side - of being safe */ -ret = vnc_display_disable_login(ds); -goto out; -} - if (vs-password) { qemu_free(vs-password); vs-password = NULL; } -vs-password = qemu_strdup(password); -vs-auth = VNC_AUTH_VNC; +if (password) + vs-password = qemu_strdup(password); + This breaks checkpatch.pl Regards, Anthony Liguori out: if (ret != 0) { qerror_report(QERR_SET_PASSWD_FAILED);
Re: [Qemu-devel] [PATCH] vga: squash build error in vga_update_memory_access()
On 2011-08-24 14:07, Avi Kivity wrote: Newer gcc complains that base and size may be used uninitialized, even though it is clearly a false warning. Mmh, gcc is getting dumber again. Silence the warning by indicating to gcc that the code path triggering the warning cannot happen. Signed-off-by: Avi Kivity a...@redhat.com --- hw/vga.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/vga.c b/hw/vga.c index 851fd68..b74e6e8 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -179,6 +179,8 @@ static void vga_update_memory_access(VGACommonState *s) base = 0xb8000; size = 0x8000; break; +default: +abort(); } region = g_malloc(sizeof(*region)); memory_region_init_alias(region, vga.chain4, s-vram, offset, size); ...or just make the last case default? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 2/2] Added target to build libvdisk
On 08/24/2011 06:32 AM, Saggi Mizrahi wrote: On Tue 23 Aug 2011 07:21:56 PM IDT, Anthony Liguori wrote: But QEMU is GPL. Libraries derived from QEMU will also be GPL. Regards, Anthony Liguori Regards, Daniel OK, I admit it was a pretty naive solution. But I always try to do the simplest solution first. The license issues can be solved later. (Having it as GPL later changing to LGPL if we can). As for the API I can create start a specialized API for the lib that uses the internal API. I can hide BlockDriverState and export it as an fd. int vdisk_open(path, format, flags) size_t vdisk_pread(fd, buf, size, offset) size_t vdisk_pwrite(fd, buff, size, offset) int vdisk_close(fd) int vdisk_get_size(fd) That's not really much better. You'll only end up reinventing the block layer API. The best route is to focus on cleaning up the block layer interface. This means converting existing users to use a single interface, adding documentation to each interface, writing test cases against the block layer API. Practically speaking, I think we really need to move the block layer to use glib primitives too before you can realistically consume the code outside of QEMU. Regards, Anthony Liguori That way no internal structures are exported and we use a minimal set of functions that are very unlikely to change. There is no support for snapshots, metadata etc. But these APIs can be added later. And of-course we can always define the lib as experimental until the API stabilizes.
Re: [Qemu-devel] [PATCH STABLE-0.14/0.15/master] CVE-2011-0011: fix VNC password change to not touch authentication settings
On Wed, Aug 24, 2011 at 07:45:06AM -0500, Anthony Liguori wrote: On 08/24/2011 06:01 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com In CVE-2011-0011 it was noted that setting an empty password would disable all authentication for the VNC password. Commit 1cd20f8bf0ecb9d1d1bd5e2ffab3b88835380c9b attempted to fix this but it just broke it in a different way, because now instead of blindly disabling all authentication, it blindly resets all authentication to 'VNC'. But this is *not* a security problem. Login becomes disabled as expected. It *is* a security problem, because if you do change vnc password 123 change vnc password change vnc password 456 you have lost the authentication settings you requested. With this patch, changing the password to *still* disables the login, without side effects on the auth scheme. We should really not overload the semantics of the change command like this and instead introduce a new QMP operation to disable login. This change I mention below is the one that overloaded the semantics by making a password change, also change the auth scheme, breaking the original behaviour. If we want apps to be able to change the auth scheme that needs a separate monitor command. The current behaviour is not usable and introduces a security problem by changing auth scheme without being asked to. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v2] pci: Error on PCI capability collisions
On Wed, Aug 24, 2011 at 02:29:30PM +0200, Jan Kiszka wrote: Nothing good can happen when we overlap capabilities. This may happen when plugging in assigned devices or when devices models contain bugs. Detect the overlap and report it. Based on qemu-kvm commit by Alex Williamson. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Applied, thanks everyone. --- hw/pci.c | 34 ++ 1 files changed, 34 insertions(+), 0 deletions(-) Changes in v2: - properly find the conflicting CAP - adjust error code diff --git a/hw/pci.c b/hw/pci.c index 6124790..634e789 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -1811,6 +1811,25 @@ static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id, return next; } +static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset) +{ +uint8_t next, prev, found = 0; + +if (!(pdev-used[offset])) { +return 0; +} + +assert(pdev-config[PCI_STATUS] PCI_STATUS_CAP_LIST); + +for (prev = PCI_CAPABILITY_LIST; (next = pdev-config[prev]); + prev = next + PCI_CAP_LIST_NEXT) { +if (next = offset next found) { +found = next; +} +} +return found; +} + /* Patch the PCI vendor and device ids in a PCI rom image if necessary. This is needed for an option rom which is used for more than one device. */ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size) @@ -1952,11 +1971,26 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t offset, uint8_t size) { uint8_t *config; +int i, overlapping_cap; + if (!offset) { offset = pci_find_space(pdev, size); if (!offset) { return -ENOSPC; } +} else { +for (i = offset; i offset + size; i++) { +overlapping_cap = pci_find_capability_at_offset(pdev, i); +if (overlapping_cap) { +fprintf(stderr, ERROR: %04x:%02x:%02x.%x +Attempt to add PCI capability %x at offset +%x overlaps existing capability %x at offset %x\n, +pci_find_domain(pdev-bus), pci_bus_num(pdev-bus), +PCI_SLOT(pdev-devfn), PCI_FUNC(pdev-devfn), +cap_id, offset, overlapping_cap, i); +return -EINVAL; +} +} } config = pdev-config + offset; -- 1.7.3.4
Re: [Qemu-devel] [PATCH] sheepdog: use coroutines
Am 23.08.2011 19:14, schrieb MORITA Kazutaka: At Tue, 23 Aug 2011 14:29:50 +0200, Kevin Wolf wrote: Am 12.08.2011 14:33, schrieb MORITA Kazutaka: This makes the sheepdog block driver support bdrv_co_readv/writev instead of bdrv_aio_readv/writev. With this patch, Sheepdog network I/O becomes fully asynchronous. The block driver yields back when send/recv returns EAGAIN, and is resumed when the sheepdog network connection is ready for the operation. Signed-off-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp --- block/sheepdog.c | 150 + 1 files changed, 93 insertions(+), 57 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index e150ac0..e283c34 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -274,7 +274,7 @@ struct SheepdogAIOCB { int ret; enum AIOCBState aiocb_type; -QEMUBH *bh; +Coroutine *coroutine; void (*aio_done_func)(SheepdogAIOCB *); int canceled; @@ -295,6 +295,10 @@ typedef struct BDRVSheepdogState { char *port; int fd; +CoMutex lock; +Coroutine *co_send; +Coroutine *co_recv; + uint32_t aioreq_seq_num; QLIST_HEAD(outstanding_aio_head, AIOReq) outstanding_aio_head; } BDRVSheepdogState; @@ -346,19 +350,16 @@ static const char * sd_strerror(int err) /* * Sheepdog I/O handling: * - * 1. In the sd_aio_readv/writev, read/write requests are added to the - *QEMU Bottom Halves. - * - * 2. In sd_readv_writev_bh_cb, the callbacks of BHs, we send the I/O - *requests to the server and link the requests to the - *outstanding_list in the BDRVSheepdogState. we exits the - *function without waiting for receiving the response. + * 1. In sd_co_rw_vector, we send the I/O requests to the server and + *link the requests to the outstanding_list in the + *BDRVSheepdogState. The function exits without waiting for + *receiving the response. * - * 3. We receive the response in aio_read_response, the fd handler to + * 2. We receive the response in aio_read_response, the fd handler to *the sheepdog connection. If metadata update is needed, we send *the write request to the vdi object in sd_write_done, the write - *completion function. The AIOCB callback is not called until all - *the requests belonging to the AIOCB are finished. + *completion function. We switch back to sd_co_readv/writev after + *all the requests belonging to the AIOCB are finished. */ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb, @@ -398,7 +399,7 @@ static inline int free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req) static void sd_finish_aiocb(SheepdogAIOCB *acb) { if (!acb-canceled) { -acb-common.cb(acb-common.opaque, acb-ret); +qemu_coroutine_enter(acb-coroutine, NULL); } qemu_aio_release(acb); } @@ -411,7 +412,8 @@ static void sd_aio_cancel(BlockDriverAIOCB *blockacb) * Sheepdog cannot cancel the requests which are already sent to * the servers, so we just complete the request with -EIO here. */ -acb-common.cb(acb-common.opaque, -EIO); +acb-ret = -EIO; +qemu_coroutine_enter(acb-coroutine, NULL); acb-canceled = 1; } @@ -435,24 +437,12 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov, acb-aio_done_func = NULL; acb-canceled = 0; -acb-bh = NULL; +acb-coroutine = qemu_coroutine_self(); acb-ret = 0; QLIST_INIT(acb-aioreq_head); return acb; } -static int sd_schedule_bh(QEMUBHFunc *cb, SheepdogAIOCB *acb) -{ -if (acb-bh) { -error_report(bug: %d %d, acb-aiocb_type, acb-aiocb_type); -return -EIO; -} - -acb-bh = qemu_bh_new(cb, acb); -qemu_bh_schedule(acb-bh); -return 0; -} - #ifdef _WIN32 struct msghdr { @@ -635,7 +625,13 @@ static int do_readv_writev(int sockfd, struct iovec *iov, int len, again: ret = do_send_recv(sockfd, iov, len, iov_offset, write); if (ret 0) { -if (errno == EINTR || errno == EAGAIN) { +if (errno == EINTR) { +goto again; +} +if (errno == EAGAIN) { +if (qemu_in_coroutine()) { +qemu_coroutine_yield(); +} Who reenters the coroutine if we yield here? The fd handlers, co_write_request() and co_read_response(), will call qemu_coroutine_enter() for the coroutine. It will be restarted after the sheepdog network connection becomes ready. Makes sense. Applied to the block branch, And of course for a coroutine based block driver I would like to see much less code in callback functions. But it's a good start anyway. Yes, actually they can be much simpler. This patch adds asynchrous I/O support with a minimal change based on a coroutine, and I think code simplification would be the next
Re: [Qemu-devel] [PATCH 2/2] Added target to build libvdisk
On Wed, Aug 24, 2011 at 07:50:37AM -0500, Anthony Liguori wrote: On 08/24/2011 06:32 AM, Saggi Mizrahi wrote: On Tue 23 Aug 2011 07:21:56 PM IDT, Anthony Liguori wrote: But QEMU is GPL. Libraries derived from QEMU will also be GPL. Regards, Anthony Liguori Regards, Daniel OK, I admit it was a pretty naive solution. But I always try to do the simplest solution first. The license issues can be solved later. (Having it as GPL later changing to LGPL if we can). As for the API I can create start a specialized API for the lib that uses the internal API. I can hide BlockDriverState and export it as an fd. int vdisk_open(path, format, flags) size_t vdisk_pread(fd, buf, size, offset) size_t vdisk_pwrite(fd, buff, size, offset) int vdisk_close(fd) int vdisk_get_size(fd) That's not really much better. You'll only end up reinventing the block layer API. The best route is to focus on cleaning up the block layer interface. This means converting existing users to use a single interface, adding documentation to each interface, writing test cases against the block layer API. Practically speaking, I think we really need to move the block layer to use glib primitives too before you can realistically consume the code outside of QEMU. I think it'd be interesting to ask what the intended use cases for accessing the block layer API outside of QEMU are ? Personally, for libvirt, I'm only interested in a libblkid like library that lets us detect what format a disk is, and query some basic metadata about the file (logical size, vs physical size, backing file paths, backing file format, whether encryption is present, etc - qemu-img info equivalent basically). Apps that actually want to read/write the contents of a virtual disk, may well be better off just using libguestfs as the API, since they may well want to actually interpret the data inside the disk, eg to read specific files out of the filesystem, etc There's possibly a need for a API to create/clone/convert virtual disks formats, but that could likely be done without directly exposing apps to any of the low level BlockDrv APis like pread/pwrite, instead providing an API at the level of 'qemu-img create' command args. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH STABLE-0.14/0.15/master] CVE-2011-0011: fix VNC password change to not touch authentication settings
On 08/24/2011 07:50 AM, Daniel P. Berrange wrote: On Wed, Aug 24, 2011 at 07:45:06AM -0500, Anthony Liguori wrote: On 08/24/2011 06:01 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com In CVE-2011-0011 it was noted that setting an empty password would disable all authentication for the VNC password. Commit 1cd20f8bf0ecb9d1d1bd5e2ffab3b88835380c9b attempted to fix this but it just broke it in a different way, because now instead of blindly disabling all authentication, it blindly resets all authentication to 'VNC'. But this is *not* a security problem. Login becomes disabled as expected. It *is* a security problem, because if you do change vnc password 123 change vnc password change vnc password 456 you have lost the authentication settings you requested. With this patch, changing the password to *still* disables the login, without side effects on the auth scheme. Just because it isn't doing what you expect it to do doesn't make it a security problem. This is the current behavior and you simply cannot write a management tool without being aware of this behavior for better or worse. The password change interface should not be overloaded to deal disable login. There should be a higher level QMP command to do this. We should really not overload the semantics of the change command like this and instead introduce a new QMP operation to disable login. This change I mention below is the one that overloaded the semantics by making a password change, also change the auth scheme, breaking the original behaviour. If we want apps to be able to change the auth scheme that needs a separate monitor command. The current behaviour is not usable and introduces a security problem by changing auth scheme without being asked to. I'll buy an argument about usability but not about security. We need a higher level command to disable login and a higher level command to set the vnc password. This interface should be considered deprecated. Regards, Anthony Liguori Daniel
Re: [Qemu-devel] [PATCH 1/2] pc: make vgabios exit port more useful
On 08/24/2011 03:38 PM, Anthony Liguori wrote: diff --git a/hw/pc.c b/hw/pc.c index 1c9d89a..4b07b35 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -549,8 +549,7 @@ static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val) /* LGPL'ed VGA BIOS messages */ case 0x501: case 0x502: - fprintf(stderr, VGA BIOS panic, line %d\n, val); - exit(1); + exit((val 1) | 1); This code (before the patch) circumvents -no-shutdown. Indeed. I believe that's a feature though? Depends on what the user of -no-shutdown expects. Shifting val left is surprising. What's wrong with even exit codes? Hrm, the '| 1' is wrong. My intention was to make the bottom bit of the exit code mean if set to 0, this is an exit coming from a guest'. Too subtle, IMO. I understand that we want to avoid a full qmp parser for one-off unit tests, but using bit fields in the exit code? Perhaps a python script that launches qemu with qmp and -no-shutdown, listens for the guest shutdown event, and prints out the result? That can be easily reused in test scripts. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] [PATCH] vga: squash build error in vga_update_memory_access()
On 08/24/2011 03:46 PM, Jan Kiszka wrote: +++ b/hw/vga.c @@ -179,6 +179,8 @@ static void vga_update_memory_access(VGACommonState *s) base = 0xb8000; size = 0x8000; break; +default: +abort(); } region = g_malloc(sizeof(*region)); memory_region_init_alias(region, vga.chain4,s-vram, offset, size); ...or just make the last case default? No reason to make the code unobvious in this path, IMO. Eventually gcc will be able to drop the 4/5 bytes this patch adds to the object code. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] [PATCH] vga: squash build error in vga_update_memory_access()
On 2011-08-24 14:58, Avi Kivity wrote: On 08/24/2011 03:46 PM, Jan Kiszka wrote: +++ b/hw/vga.c @@ -179,6 +179,8 @@ static void vga_update_memory_access(VGACommonState *s) base = 0xb8000; size = 0x8000; break; +default: +abort(); } region = g_malloc(sizeof(*region)); memory_region_init_alias(region, vga.chain4,s-vram, offset, size); ...or just make the last case default? No reason to make the code unobvious in this path, IMO. Eventually gcc will be able to drop the 4/5 bytes this patch adds to the object code. diff --git a/hw/vga.c b/hw/vga.c index 851fd68..125fb29 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -176,6 +176,7 @@ static void vga_update_memory_access(VGACommonState *s) size = 0x8000; break; case 3: +default: base = 0xb8000; size = 0x8000; break; ...is fairly common and well readable IMHO. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH STABLE-0.14/0.15/master] CVE-2011-0011: fix VNC password change to not touch authentication settings
On Wed, Aug 24, 2011 at 07:55:38AM -0500, Anthony Liguori wrote: On 08/24/2011 07:50 AM, Daniel P. Berrange wrote: On Wed, Aug 24, 2011 at 07:45:06AM -0500, Anthony Liguori wrote: On 08/24/2011 06:01 AM, Daniel P. Berrange wrote: From: Daniel P. Berrangeberra...@redhat.com In CVE-2011-0011 it was noted that setting an empty password would disable all authentication for the VNC password. Commit 1cd20f8bf0ecb9d1d1bd5e2ffab3b88835380c9b attempted to fix this but it just broke it in a different way, because now instead of blindly disabling all authentication, it blindly resets all authentication to 'VNC'. But this is *not* a security problem. Login becomes disabled as expected. It *is* a security problem, because if you do change vnc password 123 change vnc password change vnc password 456 you have lost the authentication settings you requested. With this patch, changing the password to *still* disables the login, without side effects on the auth scheme. Just because it isn't doing what you expect it to do doesn't make it a security problem. This is the current behavior and you simply cannot write a management tool without being aware of this behavior for better or worse. This was *not* the behaviour for many releases. It is a regression against the original behaviour of the change vnc password in QEMU which we had succesfully worked with in libvirt since password+TLS support was written for QEMU. The current behaviour is unusably broken. It cannot be used without creating a security problem, where as the original QEMU behaviour was succesfully usable. Simply saying that we must create a new command, instead of fixing the QEMU regression does nothing to help existing apps which are expecting current QEMU releases to work as previous releases did as the command is *documented* : http://qemu.weilnetz.de/qemu-doc.html#vnc_005fsec_005fcertificate_005fpw [quote] 3.11.5 With x509 certificates, client verification and passwords Finally, the previous method can be combined with VNC password authentication to provide two layers of authentication for clients. qemu [...OPTIONS...] -vnc :1,password,tls,x509verify=/etc/pki/qemu -monitor stdio (qemu) change vnc password Password: (qemu) [/quote] This documented example no longer works because authentication is being silently reset. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 1/2] pc: make vgabios exit port more useful
On 08/24/2011 07:55 AM, Avi Kivity wrote: On 08/24/2011 03:38 PM, Anthony Liguori wrote: diff --git a/hw/pc.c b/hw/pc.c index 1c9d89a..4b07b35 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -549,8 +549,7 @@ static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val) /* LGPL'ed VGA BIOS messages */ case 0x501: case 0x502: - fprintf(stderr, VGA BIOS panic, line %d\n, val); - exit(1); + exit((val 1) | 1); This code (before the patch) circumvents -no-shutdown. Indeed. I believe that's a feature though? Depends on what the user of -no-shutdown expects. Shifting val left is surprising. What's wrong with even exit codes? Hrm, the '| 1' is wrong. My intention was to make the bottom bit of the exit code mean if set to 0, this is an exit coming from a guest'. Too subtle, IMO. I understand that we want to avoid a full qmp parser for one-off unit tests, but using bit fields in the exit code? Perhaps a python script that launches qemu with qmp and -no-shutdown, listens for the guest shutdown event, and prints out the result? That can be easily reused in test scripts. How can the test pass data via shutdown? You would need a scratch register of some form I think... Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 1/2] pc: make vgabios exit port more useful
On 08/24/2011 04:02 PM, Anthony Liguori wrote: Too subtle, IMO. I understand that we want to avoid a full qmp parser for one-off unit tests, but using bit fields in the exit code? Perhaps a python script that launches qemu with qmp and -no-shutdown, listens for the guest shutdown event, and prints out the result? That can be easily reused in test scripts. How can the test pass data via shutdown? You would need a scratch register of some form I think... I meant using the port in the patch. Just replace exit() with the ordinary shutdown path - notifications, and not exiting if -no-shutdown is requested. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] [PATCH] vga: squash build error in vga_update_memory_access()
On 08/24/2011 04:00 PM, Jan Kiszka wrote: On 2011-08-24 14:58, Avi Kivity wrote: On 08/24/2011 03:46 PM, Jan Kiszka wrote: +++ b/hw/vga.c @@ -179,6 +179,8 @@ static void vga_update_memory_access(VGACommonState *s) base = 0xb8000; size = 0x8000; break; +default: +abort(); } region = g_malloc(sizeof(*region)); memory_region_init_alias(region, vga.chain4,s-vram, offset, size); ...or just make the last case default? No reason to make the code unobvious in this path, IMO. Eventually gcc will be able to drop the 4/5 bytes this patch adds to the object code. diff --git a/hw/vga.c b/hw/vga.c index 851fd68..125fb29 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -176,6 +176,7 @@ static void vga_update_memory_access(VGACommonState *s) size = 0x8000; break; case 3: +default: base = 0xb8000; size = 0x8000; break; ...is fairly common and well readable IMHO. Let's let the maintainers pick the one they like. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
[Qemu-devel] [PATCH v2 01/22] stellaris_enet: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/stellaris_enet.c | 29 - 1 files changed, 12 insertions(+), 17 deletions(-) diff --git a/hw/stellaris_enet.c b/hw/stellaris_enet.c index f9bd3da..d5613ff 100644 --- a/hw/stellaris_enet.c +++ b/hw/stellaris_enet.c @@ -69,7 +69,7 @@ typedef struct { NICState *nic; NICConf conf; qemu_irq irq; -int mmio_index; +MemoryRegion mmio; } stellaris_enet_state; static void stellaris_enet_update(stellaris_enet_state *s) @@ -130,7 +130,8 @@ static int stellaris_enet_can_receive(VLANClientState *nc) return (s-np 31); } -static uint32_t stellaris_enet_read(void *opaque, target_phys_addr_t offset) +static uint64_t stellaris_enet_read(void *opaque, target_phys_addr_t offset, +unsigned size) { stellaris_enet_state *s = (stellaris_enet_state *)opaque; uint32_t val; @@ -198,7 +199,7 @@ static uint32_t stellaris_enet_read(void *opaque, target_phys_addr_t offset) } static void stellaris_enet_write(void *opaque, target_phys_addr_t offset, -uint32_t value) + uint64_t value, unsigned size) { stellaris_enet_state *s = (stellaris_enet_state *)opaque; @@ -303,17 +304,12 @@ static void stellaris_enet_write(void *opaque, target_phys_addr_t offset, } } -static CPUReadMemoryFunc * const stellaris_enet_readfn[] = { - stellaris_enet_read, - stellaris_enet_read, - stellaris_enet_read +static const MemoryRegionOps stellaris_enet_ops = { +.read = stellaris_enet_read, +.write = stellaris_enet_write, +.endianness = DEVICE_NATIVE_ENDIAN, }; -static CPUWriteMemoryFunc * const stellaris_enet_writefn[] = { - stellaris_enet_write, - stellaris_enet_write, - stellaris_enet_write -}; static void stellaris_enet_reset(stellaris_enet_state *s) { s-mdv = 0x80; @@ -391,7 +387,7 @@ static void stellaris_enet_cleanup(VLANClientState *nc) unregister_savevm(s-busdev.qdev, stellaris_enet, s); -cpu_unregister_io_memory(s-mmio_index); +memory_region_destroy(s-mmio); g_free(s); } @@ -408,10 +404,9 @@ static int stellaris_enet_init(SysBusDevice *dev) { stellaris_enet_state *s = FROM_SYSBUS(stellaris_enet_state, dev); -s-mmio_index = cpu_register_io_memory(stellaris_enet_readfn, - stellaris_enet_writefn, s, - DEVICE_NATIVE_ENDIAN); -sysbus_init_mmio(dev, 0x1000, s-mmio_index); +memory_region_init_io(s-mmio, stellaris_enet_ops, s, stellaris_enet, + 0x1000); +sysbus_init_mmio_region(dev, s-mmio); sysbus_init_irq(dev, s-irq); qemu_macaddr_default_if_unset(s-conf.macaddr); -- 1.7.5.3
[Qemu-devel] [PATCH v2 02/22] sysbus: add helpers to add and delete memory regions to the system bus
Signed-off-by: Avi Kivity a...@redhat.com --- hw/sysbus.c | 22 ++ hw/sysbus.h |6 ++ 2 files changed, 28 insertions(+), 0 deletions(-) diff --git a/hw/sysbus.c b/hw/sysbus.c index f39768b..f5f0ed2 100644 --- a/hw/sysbus.c +++ b/hw/sysbus.c @@ -256,3 +256,25 @@ static char *sysbus_get_fw_dev_path(DeviceState *dev) return strdup(path); } + +void sysbus_add_memory(SysBusDevice *dev, target_phys_addr_t addr, + MemoryRegion *mem) +{ +memory_region_add_subregion(get_system_memory(), addr, mem); +} + +void sysbus_del_memory(SysBusDevice *dev, MemoryRegion *mem) +{ +memory_region_del_subregion(get_system_memory(), mem); +} + +void sysbus_add_io(SysBusDevice *dev, target_phys_addr_t addr, + MemoryRegion *mem) +{ +memory_region_add_subregion(get_system_io(), addr, mem); +} + +void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem) +{ +memory_region_del_subregion(get_system_io(), mem); +} diff --git a/hw/sysbus.h b/hw/sysbus.h index b87c6c5..e4d56cf 100644 --- a/hw/sysbus.h +++ b/hw/sysbus.h @@ -57,6 +57,12 @@ void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size); void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq); void sysbus_mmio_map(SysBusDevice *dev, int n, target_phys_addr_t addr); +void sysbus_add_memory(SysBusDevice *dev, target_phys_addr_t addr, + MemoryRegion *mem); +void sysbus_del_memory(SysBusDevice *dev, MemoryRegion *mem); +void sysbus_add_io(SysBusDevice *dev, target_phys_addr_t addr, + MemoryRegion *mem); +void sysbus_del_io(SysBusDevice *dev, MemoryRegion *mem); /* Legacy helper function for creating devices. */ DeviceState *sysbus_create_varargs(const char *name, -- 1.7.5.3
[Qemu-devel] [PATCH v2 15/22] dummy_m68k: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/dummy_m68k.c |7 +-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/dummy_m68k.c b/hw/dummy_m68k.c index eed9e38..30146b9 100644 --- a/hw/dummy_m68k.c +++ b/hw/dummy_m68k.c @@ -10,6 +10,7 @@ #include boards.h #include loader.h #include elf.h +#include exec-memory.h #define KERNEL_LOAD_ADDR 0x1 @@ -21,6 +22,8 @@ static void dummy_m68k_init(ram_addr_t ram_size, const char *initrd_filename, const char *cpu_model) { CPUState *env; +MemoryRegion *address_space_mem = get_system_memory(); +MemoryRegion *ram = g_new(MemoryRegion, 1); int kernel_size; uint64_t elf_entry; target_phys_addr_t entry; @@ -37,8 +40,8 @@ static void dummy_m68k_init(ram_addr_t ram_size, env-vbr = 0; /* RAM at address zero */ -cpu_register_physical_memory(0, ram_size, -qemu_ram_alloc(NULL, dummy_m68k.ram, ram_size) | IO_MEM_RAM); +memory_region_init_ram(ram, NULL, dummy_m68k.ram, ram_size); +memory_region_add_subregion(address_space_mem, 0, ram); /* Load kernel. */ if (kernel_filename) { -- 1.7.5.3
[Qemu-devel] [PATCH v2 14/22] pflash_cfi01/pflash_cfi02: convert to memory API
cfi02 is annoying in that is ignores some address bits; we probably want explicit support in the memory API for that. Signed-off-by: Avi Kivity a...@redhat.com --- hw/collie.c | 16 --- hw/flash.h| 16 +-- hw/gumstix.c | 28 +++- hw/lm32_boards.c | 14 --- hw/mainstone.c| 20 + hw/milkymist.c|7 ++- hw/mips_malta.c | 45 +++- hw/mips_r4k.c | 20 + hw/musicpal.c | 15 --- hw/omap_sx1.c | 21 + hw/petalogix_ml605_mmu.c |7 ++- hw/petalogix_s3adsp1800_mmu.c |8 ++- hw/pflash_cfi01.c | 67 ++--- hw/pflash_cfi02.c | 93 - hw/ppc405_boards.c| 63 +++ hw/r2d.c |7 ++- hw/virtex_ml507.c |7 ++- hw/z2.c | 15 --- 18 files changed, 246 insertions(+), 223 deletions(-) diff --git a/hw/collie.c b/hw/collie.c index 156404d..0b955e0 100644 --- a/hw/collie.c +++ b/hw/collie.c @@ -26,7 +26,7 @@ static void collie_init(ram_addr_t ram_size, { StrongARMState *s; DriveInfo *dinfo; -ram_addr_t phys_flash; +MemoryRegion *phys_flash = g_new(MemoryRegion, 2); if (!cpu_model) { cpu_model = sa1110; @@ -34,17 +34,19 @@ static void collie_init(ram_addr_t ram_size, s = sa1110_init(collie_binfo.ram_size, cpu_model); -phys_flash = qemu_ram_alloc(NULL, collie.fl1, 0x0200); +memory_region_init_rom_device(phys_flash[0], pflash_cfi01_ops_le, + NULL, collie.fl1, 0x0200); dinfo = drive_get(IF_PFLASH, 0, 0); -pflash_cfi01_register(SA_CS0, phys_flash, +pflash_cfi01_register(SA_CS0, phys_flash[0], dinfo ? dinfo-bdrv : NULL, (64 * 1024), -512, 4, 0x00, 0x00, 0x00, 0x00, 0); +512, 4, 0x00, 0x00, 0x00, 0x00); -phys_flash = qemu_ram_alloc(NULL, collie.fl2, 0x0200); +memory_region_init_rom_device(phys_flash[1], pflash_cfi01_ops_le, + NULL, collie.fl2, 0x0200); dinfo = drive_get(IF_PFLASH, 0, 1); -pflash_cfi01_register(SA_CS1, phys_flash, +pflash_cfi01_register(SA_CS1, phys_flash[1], dinfo ? dinfo-bdrv : NULL, (64 * 1024), -512, 4, 0x00, 0x00, 0x00, 0x00, 0); +512, 4, 0x00, 0x00, 0x00, 0x00); sysbus_create_simple(scoop, 0x4080, NULL); diff --git a/hw/flash.h b/hw/flash.h index 140ae39..7fb012b 100644 --- a/hw/flash.h +++ b/hw/flash.h @@ -1,21 +1,27 @@ +#include memory.h + /* NOR flash devices */ typedef struct pflash_t pflash_t; /* pflash_cfi01.c */ -pflash_t *pflash_cfi01_register(target_phys_addr_t base, ram_addr_t off, +extern const MemoryRegionOps pflash_cfi01_ops_be; +extern const MemoryRegionOps pflash_cfi01_ops_le; +extern const MemoryRegionOps pflash_cfi02_ops_be; +extern const MemoryRegionOps pflash_cfi02_ops_le; + +pflash_t *pflash_cfi01_register(target_phys_addr_t base, MemoryRegion *mem, BlockDriverState *bs, uint32_t sector_len, int nb_blocs, int width, uint16_t id0, uint16_t id1, -uint16_t id2, uint16_t id3, int be); +uint16_t id2, uint16_t id3); /* pflash_cfi02.c */ -pflash_t *pflash_cfi02_register(target_phys_addr_t base, ram_addr_t off, +pflash_t *pflash_cfi02_register(target_phys_addr_t base, MemoryRegion *mem, BlockDriverState *bs, uint32_t sector_len, int nb_blocs, int nb_mappings, int width, uint16_t id0, uint16_t id1, uint16_t id2, uint16_t id3, -uint16_t unlock_addr0, uint16_t unlock_addr1, -int be); +uint16_t unlock_addr0, uint16_t unlock_addr1); /* nand.c */ DeviceState *nand_init(BlockDriverState *bdrv, int manf_id, int chip_id); diff --git a/hw/gumstix.c b/hw/gumstix.c index 853f7e1..d3a4bb4 100644 --- a/hw/gumstix.c +++ b/hw/gumstix.c @@ -48,7 +48,8 @@ static void connex_init(ram_addr_t ram_size, { PXA2xxState *cpu; DriveInfo *dinfo; -int be; +const MemoryRegionOps *flash_ops; +MemoryRegion *flash = g_new(MemoryRegion, 1); uint32_t connex_rom = 0x0100; uint32_t connex_ram = 0x0400; @@ -63,14 +64,15 @@ static void connex_init(ram_addr_t ram_size, } #ifdef TARGET_WORDS_BIGENDIAN -be = 1; +flash_ops = pflash_cfi01_ops_be; #else -be = 0; +flash_ops = pflash_cfi01_ops_le; #endif -if
[Qemu-devel] [PATCH v2 19/22] mcf5208: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/mcf5208.c | 72 + 1 files changed, 32 insertions(+), 40 deletions(-) diff --git a/hw/mcf5208.c b/hw/mcf5208.c index 8fe507f..1c2c0c4 100644 --- a/hw/mcf5208.c +++ b/hw/mcf5208.c @@ -13,6 +13,7 @@ #include boards.h #include loader.h #include elf.h +#include exec-memory.h #define SYS_FREQ 6600 @@ -27,6 +28,7 @@ #define PCSR_PRE_MASK 0x0f00 typedef struct { +MemoryRegion iomem; qemu_irq irq; ptimer_state *timer; uint16_t pcsr; @@ -43,7 +45,7 @@ static void m5208_timer_update(m5208_timer_state *s) } static void m5208_timer_write(void *opaque, target_phys_addr_t offset, - uint32_t value) + uint64_t value, unsigned size) { m5208_timer_state *s = (m5208_timer_state *)opaque; int prescale; @@ -104,7 +106,8 @@ static void m5208_timer_trigger(void *opaque) m5208_timer_update(s); } -static uint32_t m5208_timer_read(void *opaque, target_phys_addr_t addr) +static uint64_t m5208_timer_read(void *opaque, target_phys_addr_t addr, + unsigned size) { m5208_timer_state *s = (m5208_timer_state *)opaque; switch (addr) { @@ -120,19 +123,14 @@ static uint32_t m5208_timer_read(void *opaque, target_phys_addr_t addr) } } -static CPUReadMemoryFunc * const m5208_timer_readfn[] = { - m5208_timer_read, - m5208_timer_read, - m5208_timer_read +static const MemoryRegionOps m5208_timer_ops = { +.read = m5208_timer_read, +.write = m5208_timer_write, +.endianness = DEVICE_NATIVE_ENDIAN, }; -static CPUWriteMemoryFunc * const m5208_timer_writefn[] = { - m5208_timer_write, - m5208_timer_write, - m5208_timer_write -}; - -static uint32_t m5208_sys_read(void *opaque, target_phys_addr_t addr) +static uint64_t m5208_sys_read(void *opaque, target_phys_addr_t addr, + unsigned size) { switch (addr) { case 0x110: /* SDCS0 */ @@ -154,45 +152,36 @@ static uint32_t m5208_sys_read(void *opaque, target_phys_addr_t addr) } static void m5208_sys_write(void *opaque, target_phys_addr_t addr, -uint32_t value) +uint64_t value, unsigned size) { hw_error(m5208_sys_write: Bad offset 0x%x\n, (int)addr); } -static CPUReadMemoryFunc * const m5208_sys_readfn[] = { - m5208_sys_read, - m5208_sys_read, - m5208_sys_read -}; - -static CPUWriteMemoryFunc * const m5208_sys_writefn[] = { - m5208_sys_write, - m5208_sys_write, - m5208_sys_write +static const MemoryRegionOps m5208_sys_ops = { +.read = m5208_sys_read, +.write = m5208_sys_write, +.endianness = DEVICE_NATIVE_ENDIAN, }; -static void mcf5208_sys_init(qemu_irq *pic) +static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic) { -int iomemtype; +MemoryRegion *iomem = g_new(MemoryRegion, 1); m5208_timer_state *s; QEMUBH *bh; int i; -iomemtype = cpu_register_io_memory(m5208_sys_readfn, - m5208_sys_writefn, NULL, - DEVICE_NATIVE_ENDIAN); /* SDRAMC. */ -cpu_register_physical_memory(0xfc0a8000, 0x4000, iomemtype); +memory_region_init_io(iomem, m5208_sys_ops, NULL, m5208-sys, 0x4000); +memory_region_add_subregion(address_space, 0xfc0a8000, iomem); /* Timers. */ for (i = 0; i 2; i++) { s = (m5208_timer_state *)g_malloc0(sizeof(m5208_timer_state)); bh = qemu_bh_new(m5208_timer_trigger, s); s-timer = ptimer_init(bh); -iomemtype = cpu_register_io_memory(m5208_timer_readfn, - m5208_timer_writefn, s, - DEVICE_NATIVE_ENDIAN); -cpu_register_physical_memory(0xfc08 + 0x4000 * i, 0x4000, - iomemtype); +memory_region_init_io(s-iomem, m5208_timer_ops, s, + m5208-timer, 0x4000); +memory_region_add_subregion(address_space, 0xfc08 + 0x4000 * i, +s-iomem); s-irq = pic[4 + i]; } } @@ -207,6 +196,9 @@ static void mcf5208evb_init(ram_addr_t ram_size, uint64_t elf_entry; target_phys_addr_t entry; qemu_irq *pic; +MemoryRegion *address_space_mem = get_system_memory(); +MemoryRegion *ram = g_new(MemoryRegion, 1); +MemoryRegion *sram = g_new(MemoryRegion, 1); if (!cpu_model) cpu_model = m5208; @@ -221,12 +213,12 @@ static void mcf5208evb_init(ram_addr_t ram_size, /* TODO: Configure BARs. */ /* DRAM at 0x4000 */ -cpu_register_physical_memory(0x4000, ram_size, -qemu_ram_alloc(NULL, mcf5208.ram, ram_size) | IO_MEM_RAM); +memory_region_init_ram(ram, NULL, mcf5208.ram, ram_size); +
[Qemu-devel] [PATCH v2 04/22] ReadWriteHandler: remove
No longer used. Signed-off-by: Avi Kivity a...@redhat.com --- Makefile.target |1 - rwhandler.c | 87 --- rwhandler.h | 27 - 3 files changed, 0 insertions(+), 115 deletions(-) delete mode 100644 rwhandler.c delete mode 100644 rwhandler.h diff --git a/Makefile.target b/Makefile.target index e280bf6..c9957ae 100644 --- a/Makefile.target +++ b/Makefile.target @@ -195,7 +195,6 @@ obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o virt obj-y += vhost_net.o obj-$(CONFIG_VHOST_NET) += vhost.o obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p-device.o -obj-y += rwhandler.o obj-$(CONFIG_KVM) += kvm.o kvm-all.o obj-$(CONFIG_NO_KVM) += kvm-stub.o obj-y += memory.o diff --git a/rwhandler.c b/rwhandler.c deleted file mode 100644 index bb2238f..000 --- a/rwhandler.c +++ /dev/null @@ -1,87 +0,0 @@ -#include rwhandler.h -#include ioport.h -#include cpu-all.h - -#define RWHANDLER_WRITE(name, len, type) \ -static void name(void *opaque, type addr, uint32_t value) \ -{\ -struct ReadWriteHandler *handler = opaque;\ -handler-write(handler, addr, value, len);\ -} - -#define RWHANDLER_READ(name, len, type) \ -static uint32_t name(void *opaque, type addr) \ -{ \ -struct ReadWriteHandler *handler = opaque; \ -return handler-read(handler, addr, len); \ -} - -RWHANDLER_WRITE(cpu_io_memory_simple_writeb, 1, target_phys_addr_t); -RWHANDLER_READ(cpu_io_memory_simple_readb, 1, target_phys_addr_t); -RWHANDLER_WRITE(cpu_io_memory_simple_writew, 2, target_phys_addr_t); -RWHANDLER_READ(cpu_io_memory_simple_readw, 2, target_phys_addr_t); -RWHANDLER_WRITE(cpu_io_memory_simple_writel, 4, target_phys_addr_t); -RWHANDLER_READ(cpu_io_memory_simple_readl, 4, target_phys_addr_t); - -static CPUWriteMemoryFunc * const cpu_io_memory_simple_write[] = { -cpu_io_memory_simple_writeb, -cpu_io_memory_simple_writew, -cpu_io_memory_simple_writel, -}; - -static CPUReadMemoryFunc * const cpu_io_memory_simple_read[] = { -cpu_io_memory_simple_readb, -cpu_io_memory_simple_readw, -cpu_io_memory_simple_readl, -}; - -int cpu_register_io_memory_simple(struct ReadWriteHandler *handler, int endian) -{ -if (!handler-read || !handler-write) { -return -1; -} -return cpu_register_io_memory(cpu_io_memory_simple_read, - cpu_io_memory_simple_write, - handler, endian); -} - -RWHANDLER_WRITE(ioport_simple_writeb, 1, uint32_t); -RWHANDLER_READ(ioport_simple_readb, 1, uint32_t); -RWHANDLER_WRITE(ioport_simple_writew, 2, uint32_t); -RWHANDLER_READ(ioport_simple_readw, 2, uint32_t); -RWHANDLER_WRITE(ioport_simple_writel, 4, uint32_t); -RWHANDLER_READ(ioport_simple_readl, 4, uint32_t); - -int register_ioport_simple(ReadWriteHandler* handler, - pio_addr_t start, int length, int size) -{ -IOPortWriteFunc *write; -IOPortReadFunc *read; -int r; -switch (size) { -case 1: -write = ioport_simple_writeb; -read = ioport_simple_readb; -break; -case 2: -write = ioport_simple_writew; -read = ioport_simple_readw; -break; -default: -write = ioport_simple_writel; -read = ioport_simple_readl; -} -if (handler-write) { -r = register_ioport_write(start, length, size, write, handler); -if (r 0) { -return r; -} -} -if (handler-read) { -r = register_ioport_read(start, length, size, read, handler); -if (r 0) { -return r; -} -} -return 0; -} diff --git a/rwhandler.h b/rwhandler.h deleted file mode 100644 index b2a5790..000 --- a/rwhandler.h +++ /dev/null @@ -1,27 +0,0 @@ -#ifndef READ_WRITE_HANDLER_H -#define READ_WRITE_HANDLER_H - -#include qemu-common.h -#include ioport.h - -typedef struct ReadWriteHandler ReadWriteHandler; - -/* len is guaranteed to be one of 1, 2 or 4, addr is guaranteed to fit in an - * appropriate type (io/memory/etc). They do not need to be range checked. */ -typedef void WriteHandlerFunc(ReadWriteHandler *, pcibus_t addr, - uint32_t value, int len); -typedef uint32_t ReadHandlerFunc(ReadWriteHandler *, pcibus_t addr, int len); - -struct ReadWriteHandler { -WriteHandlerFunc *write; -ReadHandlerFunc *read; -}; - -/* Helpers for when we want to use a single routine with length. */ -/* CPU memory handler: both read and write must be present. */ -int cpu_register_io_memory_simple(ReadWriteHandler *, int endian); -/* io port handler: can supply only read or write handlers. */ -int register_ioport_simple(ReadWriteHandler *, - pio_addr_t start, int length, int size); - -#endif -- 1.7.5.3
[Qemu-devel] [PATCH v2 07/22] axis_dev88: convert to memory API (RAM only)
Signed-off-by: Avi Kivity a...@redhat.com --- hw/axis_dev88.c | 16 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/axis_dev88.c b/hw/axis_dev88.c index 06200e2..73eb39d 100644 --- a/hw/axis_dev88.c +++ b/hw/axis_dev88.c @@ -31,6 +31,7 @@ #include elf.h #include cris-boot.h #include blockdev.h +#include exec-memory.h #define D(x) #define DNAND(x) @@ -259,8 +260,9 @@ void axisdev88_init (ram_addr_t ram_size, int i; int nand_regs; int gpio_regs; -ram_addr_t phys_ram; -ram_addr_t phys_intmem; +MemoryRegion *address_space_mem = get_system_memory(); +MemoryRegion *phys_ram = g_new(MemoryRegion, 1); +MemoryRegion *phys_intmem = g_new(MemoryRegion, 1); /* init CPUs */ if (cpu_model == NULL) { @@ -269,15 +271,13 @@ void axisdev88_init (ram_addr_t ram_size, env = cpu_init(cpu_model); /* allocate RAM */ -phys_ram = qemu_ram_alloc(NULL, axisdev88.ram, ram_size); -cpu_register_physical_memory(0x4000, ram_size, phys_ram | IO_MEM_RAM); +memory_region_init_ram(phys_ram, NULL, axisdev88.ram, ram_size); +memory_region_add_subregion(address_space_mem, 0x4000, phys_ram); /* The ETRAX-FS has 128Kb on chip ram, the docs refer to it as the internal memory. */ -phys_intmem = qemu_ram_alloc(NULL, axisdev88.chipram, INTMEM_SIZE); -cpu_register_physical_memory(0x3800, INTMEM_SIZE, - phys_intmem | IO_MEM_RAM); - +memory_region_init_ram(phys_intmem, NULL, axisdev88.chipram, INTMEM_SIZE); +memory_region_add_subregion(address_space_mem, 0x3800, phys_intmem); /* Attach a NAND flash to CS1. */ nand = drive_get(IF_MTD, 0, 0); -- 1.7.5.3
[Qemu-devel] [PATCH v2 21/22] milkymist-softusb: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/milkymist-softusb.c | 48 1 files changed, 24 insertions(+), 24 deletions(-) diff --git a/hw/milkymist-softusb.c b/hw/milkymist-softusb.c index fe4eedb..ef4d9ee 100644 --- a/hw/milkymist-softusb.c +++ b/hw/milkymist-softusb.c @@ -49,6 +49,9 @@ struct MilkymistSoftUsbState { HIDState hid_kbd; HIDState hid_mouse; +MemoryRegion regs_region; +MemoryRegion pmem; +MemoryRegion dmem; qemu_irq irq; /* device properties */ @@ -68,7 +71,8 @@ struct MilkymistSoftUsbState { }; typedef struct MilkymistSoftUsbState MilkymistSoftUsbState; -static uint32_t softusb_read(void *opaque, target_phys_addr_t addr) +static uint64_t softusb_read(void *opaque, target_phys_addr_t addr, + unsigned size) { MilkymistSoftUsbState *s = opaque; uint32_t r = 0; @@ -91,7 +95,8 @@ static uint32_t softusb_read(void *opaque, target_phys_addr_t addr) } static void -softusb_write(void *opaque, target_phys_addr_t addr, uint32_t value) +softusb_write(void *opaque, target_phys_addr_t addr, uint64_t value, + unsigned size) { MilkymistSoftUsbState *s = opaque; @@ -110,16 +115,14 @@ softusb_write(void *opaque, target_phys_addr_t addr, uint32_t value) } } -static CPUReadMemoryFunc * const softusb_read_fn[] = { -NULL, -NULL, -softusb_read, -}; - -static CPUWriteMemoryFunc * const softusb_write_fn[] = { -NULL, -NULL, -softusb_write, +static const MemoryRegionOps softusb_mmio_ops = { +.read = softusb_read, +.write = softusb_write, +.endianness = DEVICE_NATIVE_ENDIAN, +.valid = { +.min_access_size = 4, +.max_access_size = 4, +}, }; static inline void softusb_read_dmem(MilkymistSoftUsbState *s, @@ -256,23 +259,20 @@ static void milkymist_softusb_reset(DeviceState *d) static int milkymist_softusb_init(SysBusDevice *dev) { MilkymistSoftUsbState *s = FROM_SYSBUS(typeof(*s), dev); -int softusb_regs; -ram_addr_t pmem_ram; -ram_addr_t dmem_ram; sysbus_init_irq(dev, s-irq); -softusb_regs = cpu_register_io_memory(softusb_read_fn, softusb_write_fn, s, -DEVICE_NATIVE_ENDIAN); -sysbus_init_mmio(dev, R_MAX * 4, softusb_regs); +memory_region_init_io(s-regs_region, softusb_mmio_ops, s, + milkymist-softusb, R_MAX * 4); +sysbus_init_mmio_region(dev, s-regs_region); /* register pmem and dmem */ -pmem_ram = qemu_ram_alloc(NULL, milkymist_softusb.pmem, s-pmem_size); -cpu_register_physical_memory(s-pmem_base, s-pmem_size, -pmem_ram | IO_MEM_RAM); -dmem_ram = qemu_ram_alloc(NULL, milkymist_softusb.dmem, s-dmem_size); -cpu_register_physical_memory(s-dmem_base, s-dmem_size, -dmem_ram | IO_MEM_RAM); +memory_region_init_ram(s-pmem, NULL, milkymist_softusb.pmem, + s-pmem_size); +sysbus_add_memory(dev, s-pmem_base, s-pmem); +memory_region_init_ram(s-dmem, NULL, milkymist_softusb.dmem, + s-dmem_size); +sysbus_add_memory(dev, s-dmem_base, s-dmem); hid_init(s-hid_kbd, HID_KEYBOARD, softusb_kbd_hid_datain); hid_init(s-hid_mouse, HID_MOUSE, softusb_mouse_hid_datain); -- 1.7.5.3
[Qemu-devel] [PATCH v2 03/22] pci_host: convert conf index and data ports to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/dec_pci.c | 13 hw/grackle_pci.c | 13 hw/pci_host.c| 86 -- hw/pci_host.h| 16 -- hw/piix_pci.c| 11 +- hw/ppc4xx_pci.c | 10 -- hw/ppce500_pci.c | 21 ++--- hw/prep_pci.c| 12 +-- hw/unin_pci.c| 82 +- 9 files changed, 131 insertions(+), 133 deletions(-) diff --git a/hw/dec_pci.c b/hw/dec_pci.c index a35f382..1aec066 100644 --- a/hw/dec_pci.c +++ b/hw/dec_pci.c @@ -80,16 +80,15 @@ PCIBus *pci_dec_21154_init(PCIBus *parent_bus, int devfn) static int pci_dec_21154_init_device(SysBusDevice *dev) { DECState *s; -int pci_mem_config, pci_mem_data; s = FROM_SYSBUS(DECState, dev); -pci_mem_config = pci_host_conf_register_mmio(s-host_state, - DEVICE_LITTLE_ENDIAN); -pci_mem_data = pci_host_data_register_mmio(s-host_state, - DEVICE_LITTLE_ENDIAN); -sysbus_init_mmio(dev, 0x1000, pci_mem_config); -sysbus_init_mmio(dev, 0x1000, pci_mem_data); +memory_region_init_io(s-host_state.conf_mem, pci_host_conf_le_ops, + s-host_state, pci-conf-idx, 0x1000); +memory_region_init_io(s-host_state.data_mem, pci_host_data_le_ops, + s-host_state, pci-data-idx, 0x1000); +sysbus_init_mmio_region(dev, s-host_state.conf_mem); +sysbus_init_mmio_region(dev, s-host_state.data_mem); return 0; } diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c index 9a823e1..9d3ff7d 100644 --- a/hw/grackle_pci.c +++ b/hw/grackle_pci.c @@ -92,16 +92,15 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic, static int pci_grackle_init_device(SysBusDevice *dev) { GrackleState *s; -int pci_mem_config, pci_mem_data; s = FROM_SYSBUS(GrackleState, dev); -pci_mem_config = pci_host_conf_register_mmio(s-host_state, - DEVICE_LITTLE_ENDIAN); -pci_mem_data = pci_host_data_register_mmio(s-host_state, - DEVICE_LITTLE_ENDIAN); -sysbus_init_mmio(dev, 0x1000, pci_mem_config); -sysbus_init_mmio(dev, 0x1000, pci_mem_data); +memory_region_init_io(s-host_state.conf_mem, pci_host_conf_le_ops, + s-host_state, pci-conf-idx, 0x1000); +memory_region_init_io(s-host_state.data_mem, pci_host_data_le_ops, + s-host_state, pci-data-idx, 0x1000); +sysbus_init_mmio_region(dev, s-host_state.conf_mem); +sysbus_init_mmio_region(dev, s-host_state.data_mem); qemu_register_reset(pci_grackle_reset, s-host_state); return 0; diff --git a/hw/pci_host.c b/hw/pci_host.c index 2e8a29f..44c6c20 100644 --- a/hw/pci_host.c +++ b/hw/pci_host.c @@ -94,82 +94,72 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len) return val; } -static void pci_host_config_write(ReadWriteHandler *handler, - pcibus_t addr, uint32_t val, int len) +static void pci_host_config_write(void *opaque, target_phys_addr_t addr, + uint64_t val, unsigned len) { -PCIHostState *s = container_of(handler, PCIHostState, conf_handler); +PCIHostState *s = opaque; -PCI_DPRINTF(%s addr % FMT_PCIBUS %d val %PRIx32\n, +PCI_DPRINTF(%s addr TARGET_FMT_plx len %d val %PRIx64\n, __func__, addr, len, val); s-config_reg = val; } -static uint32_t pci_host_config_read(ReadWriteHandler *handler, - pcibus_t addr, int len) +static uint64_t pci_host_config_read(void *opaque, target_phys_addr_t addr, + unsigned len) { -PCIHostState *s = container_of(handler, PCIHostState, conf_handler); +PCIHostState *s = opaque; uint32_t val = s-config_reg; -PCI_DPRINTF(%s addr % FMT_PCIBUS len %d val %PRIx32\n, +PCI_DPRINTF(%s addr TARGET_FMT_plx len %d val %PRIx32\n, __func__, addr, len, val); return val; } -static void pci_host_data_write(ReadWriteHandler *handler, -pcibus_t addr, uint32_t val, int len) +static void pci_host_data_write(void *opaque, target_phys_addr_t addr, +uint64_t val, unsigned len) { -PCIHostState *s = container_of(handler, PCIHostState, data_handler); -PCI_DPRINTF(write addr % FMT_PCIBUS len %d val %x\n, -addr, len, val); +PCIHostState *s = opaque; +PCI_DPRINTF(write addr TARGET_FMT_plx len %d val %x\n, +addr, len, (unsigned)val); if (s-config_reg (1u 31)) pci_data_write(s-bus, s-config_reg | (addr 3), val, len); } -static uint32_t pci_host_data_read(ReadWriteHandler *handler, - pcibus_t addr, int
[Qemu-devel] [PATCH v2 09/22] integratorcp: convert to memory API (RAM/flash only)
Signed-off-by: Avi Kivity a...@redhat.com --- hw/integratorcp.c | 28 1 files changed, 20 insertions(+), 8 deletions(-) diff --git a/hw/integratorcp.c b/hw/integratorcp.c index 2814108..3c8982e 100644 --- a/hw/integratorcp.c +++ b/hw/integratorcp.c @@ -13,11 +13,13 @@ #include boards.h #include arm-misc.h #include net.h +#include exec-memory.h typedef struct { SysBusDevice busdev; uint32_t memsz; -uint32_t flash_offset; +MemoryRegion flash; +bool flash_mapped; uint32_t cm_osc; uint32_t cm_ctrl; uint32_t cm_lock; @@ -108,9 +110,15 @@ static uint32_t integratorcm_read(void *opaque, target_phys_addr_t offset) static void integratorcm_do_remap(integratorcm_state *s, int flash) { if (flash) { -cpu_register_physical_memory(0, 0x10, IO_MEM_RAM); +if (s-flash_mapped) { +sysbus_del_memory(s-busdev, s-flash); +s-flash_mapped = false; +} } else { -cpu_register_physical_memory(0, 0x10, s-flash_offset | IO_MEM_RAM); +if (!s-flash_mapped) { +sysbus_add_memory_overlap(s-busdev, 0, s-flash, 1); +s-flash_mapped = true; +} } //??? tlb_flush (cpu_single_env, 1); } @@ -252,7 +260,8 @@ static int integratorcm_init(SysBusDevice *dev) } memcpy(integrator_spd + 73, QEMU-MEMORY, 11); s-cm_init = 0x0112; -s-flash_offset = qemu_ram_alloc(NULL, integrator.flash, 0x10); +memory_region_init_ram(s-flash, NULL, integrator.flash, 0x10); +s-flash_mapped = false; iomemtype = cpu_register_io_memory(integratorcm_readfn, integratorcm_writefn, s, @@ -456,7 +465,9 @@ static void integratorcp_init(ram_addr_t ram_size, const char *initrd_filename, const char *cpu_model) { CPUState *env; -ram_addr_t ram_offset; +MemoryRegion *address_space_mem = get_system_memory(); +MemoryRegion *ram = g_new(MemoryRegion, 1); +MemoryRegion *ram_alias = g_new(MemoryRegion, 1); qemu_irq pic[32]; qemu_irq *cpu_pic; DeviceState *dev; @@ -469,13 +480,14 @@ static void integratorcp_init(ram_addr_t ram_size, fprintf(stderr, Unable to find CPU definition\n); exit(1); } -ram_offset = qemu_ram_alloc(NULL, integrator.ram, ram_size); +memory_region_init_ram(ram, NULL, integrator.ram, ram_size); /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash. */ /* ??? RAM should repeat to fill physical memory space. */ /* SDRAM at address zero*/ -cpu_register_physical_memory(0, ram_size, ram_offset | IO_MEM_RAM); +memory_region_add_subregion(address_space_mem, 0, ram); /* And again at address 0x8000 */ -cpu_register_physical_memory(0x8000, ram_size, ram_offset | IO_MEM_RAM); +memory_region_init_alias(ram_alias, ram.alias, ram, 0, ram_size); +memory_region_add_subregion(address_space_mem, 0x8000, ram_alias); dev = qdev_create(NULL, integrator_core); qdev_prop_set_uint32(dev, memsz, ram_size 20); -- 1.7.5.3
[Qemu-devel] [PATCH v2 20/22] milkymist-minimac2: convert to memory API
Signed-off-by: Avi Kivity a...@redhat.com --- hw/milkymist-minimac2.c | 43 +-- 1 files changed, 21 insertions(+), 22 deletions(-) diff --git a/hw/milkymist-minimac2.c b/hw/milkymist-minimac2.c index cd36026..fb48e37 100644 --- a/hw/milkymist-minimac2.c +++ b/hw/milkymist-minimac2.c @@ -97,6 +97,8 @@ struct MilkymistMinimac2State { NICConf conf; char *phy_model; target_phys_addr_t buffers_base; +MemoryRegion buffers; +MemoryRegion regs_region; qemu_irq rx_irq; qemu_irq tx_irq; @@ -320,8 +322,8 @@ static ssize_t minimac2_rx(VLANClientState *nc, const uint8_t *buf, size_t size) return size; } -static uint32_t -minimac2_read(void *opaque, target_phys_addr_t addr) +static uint64_t +minimac2_read(void *opaque, target_phys_addr_t addr, unsigned size) { MilkymistMinimac2State *s = opaque; uint32_t r = 0; @@ -350,7 +352,8 @@ minimac2_read(void *opaque, target_phys_addr_t addr) } static void -minimac2_write(void *opaque, target_phys_addr_t addr, uint32_t value) +minimac2_write(void *opaque, target_phys_addr_t addr, uint64_t value, + unsigned size) { MilkymistMinimac2State *s = opaque; @@ -395,16 +398,14 @@ minimac2_write(void *opaque, target_phys_addr_t addr, uint32_t value) } } -static CPUReadMemoryFunc * const minimac2_read_fn[] = { -NULL, -NULL, -minimac2_read, -}; - -static CPUWriteMemoryFunc * const minimac2_write_fn[] = { -NULL, -NULL, -minimac2_write, +static const MemoryRegionOps minimac2_ops = { +.read = minimac2_read, +.write = minimac2_write, +.valid = { +.min_access_size = 4, +.max_access_size = 4, +}, +.endianness = DEVICE_NATIVE_ENDIAN, }; static int minimac2_can_rx(VLANClientState *nc) @@ -457,25 +458,23 @@ static NetClientInfo net_milkymist_minimac2_info = { static int milkymist_minimac2_init(SysBusDevice *dev) { MilkymistMinimac2State *s = FROM_SYSBUS(typeof(*s), dev); -int regs; -ram_addr_t buffers; size_t buffers_size = TARGET_PAGE_ALIGN(3 * MINIMAC2_BUFFER_SIZE); sysbus_init_irq(dev, s-rx_irq); sysbus_init_irq(dev, s-tx_irq); -regs = cpu_register_io_memory(minimac2_read_fn, minimac2_write_fn, s, -DEVICE_NATIVE_ENDIAN); -sysbus_init_mmio(dev, R_MAX * 4, regs); +memory_region_init_io(s-regs_region, minimac2_ops, s, + minimac2-mmio, R_MAX * 4); +sysbus_init_mmio_region(dev, s-regs_region); /* register buffers memory */ -buffers = qemu_ram_alloc(NULL, milkymist_minimac2.buffers, buffers_size); -s-rx0_buf = qemu_get_ram_ptr(buffers); +memory_region_init_ram(s-buffers, NULL, milkymist_minimac2.buffers, + buffers_size); +s-rx0_buf = memory_region_get_ram_ptr(s-buffers); s-rx1_buf = s-rx0_buf + MINIMAC2_BUFFER_SIZE; s-tx_buf = s-rx1_buf + MINIMAC2_BUFFER_SIZE; -cpu_register_physical_memory(s-buffers_base, buffers_size, -buffers | IO_MEM_RAM); +sysbus_add_memory(dev, s-buffers_base, s-buffers); qemu_macaddr_default_if_unset(s-conf.macaddr); s-nic = qemu_new_nic(net_milkymist_minimac2_info, s-conf, -- 1.7.5.3
[Qemu-devel] [PATCH v2 13/22] Makefile.hw: allow hw/ files to include glib headers
Signed-off-by: Avi Kivity a...@redhat.com --- Makefile.hw |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/Makefile.hw b/Makefile.hw index 659e441..63eb7e4 100644 --- a/Makefile.hw +++ b/Makefile.hw @@ -10,6 +10,7 @@ include $(SRC_PATH)/rules.mak $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw) QEMU_CFLAGS+=-I.. +QEMU_CFLAGS += $(GLIB_CFLAGS) include $(SRC_PATH)/Makefile.objs -- 1.7.5.3
[Qemu-devel] [PATCH v2 00/22] Memory API conversions, batch 5
This is a collection of random conversions to devices to the new memory API. Some qemu interfaces are also update, for example machine creation and sysbus. Please review. If all is fine, I will post a [PULL] request for the batch (in git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/batch). v2: - removed glib.h includes - folded in a build fix - dropped patch 6, QEMUMachine: pass address space to machine init. unfortunately this scatters a lot of get_system_memory() which will have to be replaced by something better. Avi Kivity (22): stellaris_enet: convert to memory API sysbus: add helpers to add and delete memory regions to the system bus pci_host: convert conf index and data ports to memory API ReadWriteHandler: remove an5206: convert to memory API armv7m: convert to memory API axis_dev88: convert to memory API (RAM only) sysbus: add sysbus_add_memory_overlap() integratorcp: convert to memory API (RAM/flash only) leon3: convert to memory API cirrus: wrap memory update in a transaction piix_pci: wrap memory update in a transaction Makefile.hw: allow hw/ files to include glib headers pflash_cfi01/pflash_cfi02: convert to memory API dummy_m68k: convert to memory API g364fb: convert to memory API lm32_boards: convert to memory API mainstone: convert to memory API mcf5208: convert to memory API milkymist-minimac2: convert to memory API milkymist-softusb: convert to memory API milkymist: convert to memory API Makefile.hw |1 + Makefile.target |1 - hw/an5206.c | 12 -- hw/arm-misc.h |5 ++- hw/armv7m.c | 22 + hw/axis_dev88.c | 16 hw/cirrus_vga.c |2 + hw/collie.c | 16 --- hw/dec_pci.c | 13 +++--- hw/dummy_m68k.c |7 ++- hw/flash.h| 16 +-- hw/g364fb.c | 60 ++ hw/grackle_pci.c | 13 +++--- hw/gumstix.c | 28 +++- hw/integratorcp.c | 28 + hw/leon3.c| 15 --- hw/lm32_boards.c | 29 +++- hw/mainstone.c| 33 -- hw/mcf5208.c | 72 ++- hw/milkymist-minimac2.c | 43 +-- hw/milkymist-softusb.c| 48 +++--- hw/milkymist.c| 16 --- hw/mips.h |4 +- hw/mips_jazz.c|3 +- hw/mips_malta.c | 45 +++- hw/mips_r4k.c | 20 + hw/musicpal.c | 15 --- hw/omap_sx1.c | 21 + hw/pci_host.c | 86 +- hw/pci_host.h | 16 +++ hw/petalogix_ml605_mmu.c |7 ++- hw/petalogix_s3adsp1800_mmu.c |8 ++- hw/pflash_cfi01.c | 67 ++--- hw/pflash_cfi02.c | 93 - hw/piix_pci.c | 13 +- hw/ppc405_boards.c| 63 +++ hw/ppc4xx_pci.c | 10 +++-- hw/ppce500_pci.c | 21 - hw/prep_pci.c | 12 - hw/r2d.c |7 ++- hw/stellaris.c|5 ++- hw/stellaris_enet.c | 29 +--- hw/sysbus.c | 28 hw/sysbus.h |8 hw/unin_pci.c | 82 ++-- hw/virtex_ml507.c |7 ++- hw/z2.c | 15 --- rwhandler.c | 87 -- rwhandler.h | 27 49 files changed, 637 insertions(+), 658 deletions(-) delete mode 100644 rwhandler.c delete mode 100644 rwhandler.h -- 1.7.5.3
Re: [Qemu-devel] kvm PCI assignment VFIO ramblings
On Wed, 2011-08-24 at 09:51 +1000, Benjamin Herrenschmidt wrote: For us the most simple and logical approach (which is also what pHyp uses and what Linux handles well) is really to expose a given PCI host bridge per group to the guest. Believe it or not, it makes things easier :-) I'm all for easier. Why does exposing the bridge use less bus numbers than emulating a bridge? Because a host bridge doesn't look like a PCI to PCI bridge at all for us. It's an entire separate domain with it's own bus number space (unlike most x86 setups). Ok, I missed the host bridge. In fact we have some problems afaik in qemu today with the concept of PCI domains, for example, I think qemu has assumptions about a single shared IO space domain which isn't true for us (each PCI host bridge provides a distinct IO space domain starting at 0). We'll have to fix that, but it's not a huge deal. Yep, I've seen similar on ia64 systems. So for each group we'd expose in the guest an entire separate PCI domain space with its own IO, MMIO etc... spaces, handed off from a single device-tree host bridge which doesn't itself appear in the config space, doesn't need any emulation of any config space etc... On x86, I want to maintain that our default assignment is at the device level. A user should be able to pick single or multiple devices from across several groups and have them all show up as individual, hotpluggable devices on bus 0 in the guest. Not surprisingly, we've also seen cases where users try to attach a bridge to the guest, assuming they'll get all the devices below the bridge, so I'd be in favor of making this just work if possible too, though we may have to prevent hotplug of those. Given the device requirement on x86 and since everything is a PCI device on x86, I'd like to keep a qemu command line something like -device vfio,host=00:19.0. I assume that some of the iommu properties, such as dma window size/address, will be query-able through an architecture specific (or general if possible) ioctl on the vfio group fd. I hope that will help the specification, but I don't fully understand what all remains. Thanks, Well, for iommu there's a couple of different issues here but yes, basically on one side we'll have some kind of ioctl to know what segment of the device(s) DMA address space is assigned to the group and we'll need to represent that to the guest via a device-tree property in some kind of parent node of all the devices in that group. We -might- be able to implement some kind of hotplug of individual devices of a group under such a PHB (PCI Host Bridge), I don't know for sure yet, some of that PAPR stuff is pretty arcane, but basically, for all intend and purpose, we really want a group to be represented as a PHB in the guest. We cannot arbitrary have individual devices of separate groups be represented in the guest as siblings on a single simulated PCI bus. I think the vfio kernel layer we're describing easily supports both. This is just a matter of adding qemu-vfio code to expose different topologies based on group iommu capabilities and mapping mode. Thanks, Alex