Re: [Qemu-devel] [PATCH v7 00/11] target-arm: Parts of the AArch64 EL2/3 exception model
On 2014-09-29 12:41, Peter Maydell wrote: On 29 September 2014 11:31, Jan Kiszka jan.kis...@siemens.com wrote: Sorry for hijacking the thread, but it seems related: These bits address AArch64, but what is the status of AArch32 /wrt hyp mode emulation? After playing with the fast model, I would be glad to find such support in QEMU rather sooner than later. I don't think anybody is currently working on that, though it is an obvious gap to fill in at some point. I would caution against testing hypervisors on QEMU rather than against hardware or the Fast Model, except for anything other than basic smoke tests, though -- there are a lot of complications and corner cases in the architecture here and I expect we will spend some time flushing out bugs as OSes expose them. No difference with nested virt for KVM: I fixed several corner cases along the way of getting Jailhouse running on x86. This time we already have a working baseline, so shaking out bugs might be even easier. What matters more mid- and long-term is the round-trip time, in fact. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v2] slirp: udp: fix NULL pointer dereference because of uninitialized socket
On 2014-09-18 08:35, Petr Matousek wrote: When guest sends udp packet with source port and source addr 0, uninitialized socket is picked up when looking for matching and already created udp sockets, and later passed to sosendto() where NULL pointer dereference is hit during so-slirp-vnetwork_mask.s_addr access. Fix this by checking that the socket is not just a socket stub. This is CVE-2014-3640. Signed-off-by: Petr Matousek pmato...@redhat.com Reported-by: Xavier Mehrenberger xavier.mehrenber...@airbus.com Reported-by: Stephane Duverger stephane.duver...@eads.net --- v1 - v2 * change the check so that it's consistent with the rest of the code slirp/udp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slirp/udp.c b/slirp/udp.c index 8cc6cb6..f77e00f 100644 --- a/slirp/udp.c +++ b/slirp/udp.c @@ -152,7 +152,7 @@ udp_input(register struct mbuf *m, int iphlen) * Locate pcb for datagram. */ so = slirp-udp_last_so; - if (so-so_lport != uh-uh_sport || + if (so == slirp-udb || so-so_lport != uh-uh_sport || so-so_laddr.s_addr != ip-ip_src.s_addr) { struct socket *tmp; Reviewed-by: Jan Kiszka jan.kis...@siemens.com Thanks, Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] kvm: ioapic: conditionally delay irq delivery during eoi broadcast
On 2014-09-11 07:06, Zhang Haoyu wrote: Currently, we call ioapic_service() immediately when we find the irq is still active during eoi broadcast. But for real hardware, there's some dealy between the EOI writing and irq delivery (system bus latency?). So we need to emulate this behavior. Otherwise, for a guest who haven't register a proper irq handler , it would stay in the interrupt routine as this irq would be re-injected immediately after guest enables interrupt. This would lead guest can't move forward and may miss the possibility to get proper irq handler registered (one example is windows guest resuming from hibernation). As there's no way to differ the unhandled irq from new raised ones, this patch solve this problems by scheduling a delayed work when the count of irq injected during eoi broadcast exceeds a threshold value. After this patch, the guest can move a little forward when there's no suitable irq handler in case it may register one very soon and for guest who has a bad irq detection routine ( such as note_interrupt() in linux ), this bad irq would be recognized soon as in the past. Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Jason Wang jasow...@redhat.com Signed-off-by: Zhang Haoyu zhan...@sangfor.com --- include/trace/events/kvm.h | 20 ++ virt/kvm/ioapic.c | 51 -- virt/kvm/ioapic.h | 6 ++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h index 908925a..b05f688 100644 --- a/include/trace/events/kvm.h +++ b/include/trace/events/kvm.h @@ -95,6 +95,26 @@ TRACE_EVENT(kvm_ioapic_set_irq, __entry-coalesced ? (coalesced) : ) ); +TRACE_EVENT(kvm_ioapic_delayed_eoi_inj, + TP_PROTO(__u64 e), + TP_ARGS(e), + + TP_STRUCT__entry( + __field(__u64, e ) + ), + + TP_fast_assign( + __entry-e = e; + ), + + TP_printk(dst %x vec=%u (%s|%s|%s%s), + (u8)(__entry-e 56), (u8)__entry-e, + __print_symbolic((__entry-e 8 0x7), kvm_deliver_mode), + (__entry-e (111)) ? logical : physical, + (__entry-e (115)) ? level : edge, + (__entry-e (116)) ? |masked : ) +); + TRACE_EVENT(kvm_msi_set_irq, TP_PROTO(__u64 address, __u64 data), TP_ARGS(address, data), diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c index e8ce34c..a36c4c4 100644 --- a/virt/kvm/ioapic.c +++ b/virt/kvm/ioapic.c @@ -405,6 +405,24 @@ void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id) spin_unlock(ioapic-lock); } +static void kvm_ioapic_eoi_inject_work(struct work_struct *work) +{ + int i; + struct kvm_ioapic *ioapic = container_of(work, struct kvm_ioapic, + eoi_inject.work); + spin_lock(ioapic-lock); + for (i = 0; i IOAPIC_NUM_PINS; i++) { + union kvm_ioapic_redirect_entry *ent = ioapic-redirtbl[i]; + + if (ent-fields.trig_mode != IOAPIC_LEVEL_TRIG) + continue; + + if (ioapic-irr (1 i) !ent-fields.remote_irr) + ioapic_service(ioapic, i, false); + } + spin_unlock(ioapic-lock); +} + static void __kvm_ioapic_update_eoi(struct kvm_vcpu *vcpu, struct kvm_ioapic *ioapic, int vector, int trigger_mode) { @@ -435,8 +453,33 @@ static void __kvm_ioapic_update_eoi(struct kvm_vcpu *vcpu, ASSERT(ent-fields.trig_mode == IOAPIC_LEVEL_TRIG); ent-fields.remote_irr = 0; - if (ioapic-irr (1 i)) - ioapic_service(ioapic, i, false); + if (!ent-fields.mask (ioapic-irr (1 i))) { The mask check is new - why now? You don't check it in the work handler as well. + ++ioapic-irq_eoi[i]; + if (ioapic-irq_eoi[i] == IOAPIC_SUCCESSIVE_IRQ_MAX_COUNT) { + /* + * Real hardware does not deliver the irq so + * immediately during eoi broadcast, so we need + * to emulate this behavior. Otherwise, for + * guests who has not registered handler of a + * level irq, this irq would be injected + * immediately after guest enables interrupt + * (which happens usually at the end of the + * common interrupt routine). This would lead + * guest can't move forward and may miss the + * possibility to get proper irq handler + * registered. So we
Re: [Qemu-devel] [PATCH] virtio-pci: enable bus master for old guests
On 2014-09-08 18:05, Michael S. Tsirkin wrote: commit cc943c36faa192cd4b32af8fe5edb31894017d35 pci: Use bus master address space for delivering MSI/MSI-X messages breaks virtio-net for rhel6.[56] x86 guests because they don't enable bus mastering for virtio PCI devices Old guests forgot to enable bus mastering, enable it automatically on DRIVER_OK. Note: we should either back out the original patch from stable or apply this one on top. Cc: qemu-sta...@nongnu.org Reported-by: Greg Kurz gk...@linux.vnet.ibm.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com I didn't signed off as I didn't write this patch ;). But you can add my reviewed-by if you like. Jan Signed-off-by: Michael S. Tsirkin m...@redhat.com --- hw/virtio/virtio-pci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index ddb5da1..af937d2 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -320,6 +320,8 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) if ((val VIRTIO_CONFIG_S_DRIVER_OK) !(proxy-pci_dev.config[PCI_COMMAND] PCI_COMMAND_MASTER)) { proxy-flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG; + memory_region_set_enabled(proxy-pci_dev.bus_master_enable_region, + true); } break; case VIRTIO_MSI_CONFIG_VECTOR: -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] slirp: Honour vlan/stack in hostfwd_remove commands
On 2014-09-02 12:33, Peter Maydell wrote: On 8 July 2014 12:40, Peter Maydell peter.mayd...@linaro.org wrote: On 26 June 2014 13:35, Peter Maydell peter.mayd...@linaro.org wrote: On 16 June 2014 16:47, Peter Maydell peter.mayd...@linaro.org wrote: The hostfwd_add and hostfwd_remove monitor commands allow the user to optionally specify a vlan/stack tuple. hostfwd_add honours this, but hostfwd_remove does not (it looks up the tuple but then ignores the SlirpState it has looked up and always uses the first stack in the list anyway). Correct this to honour what the user requested. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- net/slirp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/slirp.c b/net/slirp.c index 647039e..c171119 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -345,8 +345,7 @@ void net_slirp_hostfwd_remove(Monitor *mon, const QDict *qdict) host_port = atoi(p); -err = slirp_remove_hostfwd(QTAILQ_FIRST(slirp_stacks)-slirp, is_udp, - host_addr, host_port); +err = slirp_remove_hostfwd(s-slirp, is_udp, host_addr, host_port); monitor_printf(mon, host forwarding rule for %s %s\n, src_str, err ? not found : removed); -- 1.9.2 Ping! (and cc trivial). Ping^2. Ping^3 :-( Sorry, you will probably need someone else for these topics. I've difficulties scheduling these into my todo list. :( Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v3 0/5] intel-iommu: introduce Intel IOMMU (VT-d) emulation to q35 chipset
On 2014-08-18 18:34, Knut Omang wrote: On Sat, 2014-08-16 at 10:47 +0200, Jan Kiszka wrote: On 2014-08-16 10:45, Jan Kiszka wrote: On 2014-08-16 09:54, Knut Omang wrote: On Fri, 2014-08-15 at 19:37 +0800, Le Tan wrote: Hi Knut, 2014-08-15 19:15 GMT+08:00 Knut Omang knut.om...@oracle.com: On Fri, 2014-08-15 at 06:42 +0200, Knut Omang wrote: On Thu, 2014-08-14 at 14:10 +0200, Jan Kiszka wrote: On 2014-08-14 13:15, Michael S. Tsirkin wrote: On Mon, Aug 11, 2014 at 03:04:57PM +0800, Le Tan wrote: Hi, These patches are intended to introduce Intel IOMMU (VT-d) emulation to q35 chipset. The major job in these patches is to add support for emulating Intel IOMMU according to the VT-d specification, including basic responses to CSRs accesses, the logics of DMAR (DMA remapping) and DMA memory address translations. Thanks! Looks very good overall, I noted some coding style issues - I didn't bother reporting each issue in every place where it appears - reported each issue once only, so please find and fix all instances of each issue. BTW, because I was in urgent need for virtual test environment for Jailhouse, I hacked interrupt remapping on top of Le's patches: http://git.kiszka.org/?p=qemu.git;a=shortlog;h=refs/heads/queues/vtd-intremap The approach likely needs further discussions and refinements but it already allows me to work on top with our hypervisor, and also Linux. You can see from the last commit that Le's work made it pretty easy to build this on top. Le, I have tried Jan's branch with my device setup which consists of a minimal q35 setup, an ioh3420 root port (specified as -device ioh3420,slot=0 ) and a pcie device plugged into that root port, which gives the following lscpi -t: -[:00]-+-00.0 +-01.0 +-02.0 +-03.0-[01]00.0 +-04.0 +-1f.0 +-1f.2 \-1f.3 All seems to work beautifully (I see the ISA bridge happily receive translations) until the first DMA from my device model (at 1:00.0) arrives, at which point I get: [ 1663.732413] dmar: DMAR:[DMA Write] Request device [00:03.0] fault addr fffa [ 1663.732413] DMAR:[fault reason 02] Present bit in context entry is clear I would have expected request device 01:00.0 for this. It is not clear to me yet if this is a weakness of the implementation of ioh3420 or the iommu. Just wanted to let you know right away in case you can shed some light to it or it is an easy fix, The device uses pci_dma_rw with itself as device pointer. To verify my hypothesis: with this rude hack my device now works much better: @@ -774,6 +780,8 @@ static void iommu_translate(VTDAddressSpace *vtd_as, int bus_num, int devfn, is_fpd_set = ce.lo VTD_CONTEXT_ENTRY_FPD; } else { ret_fr = dev_to_context_entry(s, bus_num, devfn, ce); +if (ret_fr) +ret_fr = dev_to_context_entry(s, 1, 0, ce); is_fpd_set = ce.lo VTD_CONTEXT_ENTRY_FPD; if (ret_fr) { ret_fr = -ret_fr; Looking at how things look on hardware, multiple devices often receive overlapping DMA address ranges for different physical addresses. So if I understand the way this works, every requester ID would also need to have it's own unique VTDAddressSpace, as each pci device/function sees a unique DMA address space.. ioh3420 is a pcie-to-pcie bridge, right? Yes. In my opinion, each pci-e device behind the pcie-to-pcie bridge can be assigned individually. For now I added the VT-d to q35 by just adding it to the root pci bus. You can see here in q35.c: pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu); So if we add a pcie-to-pcie bridge, we may have to call the pci_setup_iommu() for that new bus. I don't know where to hook into this now. :) If you know the mechanism behind that, you can try to add that for the new bus. (I will dive into this after the clean up.) What do you think? Thanks for the quick answer, that helped a lot! Looking into the details here I realize it is slightly more complicated: secondary buses are enumerated after device instantiation, as part of the host PCI enumeration, so if I add a similar setup call in the bridge setup, it will be called for a new device long before it has received it's bus number from the OS (via config[PCI_SECONDARY_BUS] ) I agree that the lookup function for contexts needs to be as efficient as possible so the simple busno,defvn lookup key may be the best solution but then the address_spaces table cannot be populated with the secondary bus entries before it receives a nonzero != 255 bus number, eg. along the lines of this: diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index 4becdc1..d9a8c23 100644 --- a/hw/pci/pci_bridge.c +++ b/hw/pci/pci_bridge.c @@ -265,6 +265,12 @@ void pci_bridge_write_config(PCIDevice *d, pci_bridge_update_mappings(s); } +if (ranges_overlap(address
Re: [Qemu-devel] [PATCH v3 0/5] intel-iommu: introduce Intel IOMMU (VT-d) emulation to q35 chipset
On 2014-08-19 06:08, Knut Omang wrote: Are you depending on interrupt remapping? If not, my patches are a bit hacky and may cause their own issues if you are unlucky. It does not depend directly but interprets a NULL PciDevice pointer as the special bus number (0xff) for non-pci devices, so I have tried to Yeah, this won't map nicely on future chipsets - unless we redefine Q35_PSEUDO_XXX as VTD_PSEUDO_XXX, or so, and use the same values for all of them. Or create an unconnected pseudo PCI bus for the chipset devices that carry the pseudo bus number. take heights for that - I can rebase if so desired, but I would like to see the interrupt remapping emerge as well ;-) Good, then we are already two ;). Let's get the baseline ready, then we can discuss extensions like interrupt remapping. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3 0/5] intel-iommu: introduce Intel IOMMU (VT-d) emulation to q35 chipset
On 2014-08-16 09:54, Knut Omang wrote: On Fri, 2014-08-15 at 19:37 +0800, Le Tan wrote: Hi Knut, 2014-08-15 19:15 GMT+08:00 Knut Omang knut.om...@oracle.com: On Fri, 2014-08-15 at 06:42 +0200, Knut Omang wrote: On Thu, 2014-08-14 at 14:10 +0200, Jan Kiszka wrote: On 2014-08-14 13:15, Michael S. Tsirkin wrote: On Mon, Aug 11, 2014 at 03:04:57PM +0800, Le Tan wrote: Hi, These patches are intended to introduce Intel IOMMU (VT-d) emulation to q35 chipset. The major job in these patches is to add support for emulating Intel IOMMU according to the VT-d specification, including basic responses to CSRs accesses, the logics of DMAR (DMA remapping) and DMA memory address translations. Thanks! Looks very good overall, I noted some coding style issues - I didn't bother reporting each issue in every place where it appears - reported each issue once only, so please find and fix all instances of each issue. BTW, because I was in urgent need for virtual test environment for Jailhouse, I hacked interrupt remapping on top of Le's patches: http://git.kiszka.org/?p=qemu.git;a=shortlog;h=refs/heads/queues/vtd-intremap The approach likely needs further discussions and refinements but it already allows me to work on top with our hypervisor, and also Linux. You can see from the last commit that Le's work made it pretty easy to build this on top. Le, I have tried Jan's branch with my device setup which consists of a minimal q35 setup, an ioh3420 root port (specified as -device ioh3420,slot=0 ) and a pcie device plugged into that root port, which gives the following lscpi -t: -[:00]-+-00.0 +-01.0 +-02.0 +-03.0-[01]00.0 +-04.0 +-1f.0 +-1f.2 \-1f.3 All seems to work beautifully (I see the ISA bridge happily receive translations) until the first DMA from my device model (at 1:00.0) arrives, at which point I get: [ 1663.732413] dmar: DMAR:[DMA Write] Request device [00:03.0] fault addr fffa [ 1663.732413] DMAR:[fault reason 02] Present bit in context entry is clear I would have expected request device 01:00.0 for this. It is not clear to me yet if this is a weakness of the implementation of ioh3420 or the iommu. Just wanted to let you know right away in case you can shed some light to it or it is an easy fix, The device uses pci_dma_rw with itself as device pointer. To verify my hypothesis: with this rude hack my device now works much better: @@ -774,6 +780,8 @@ static void iommu_translate(VTDAddressSpace *vtd_as, int bus_num, int devfn, is_fpd_set = ce.lo VTD_CONTEXT_ENTRY_FPD; } else { ret_fr = dev_to_context_entry(s, bus_num, devfn, ce); +if (ret_fr) +ret_fr = dev_to_context_entry(s, 1, 0, ce); is_fpd_set = ce.lo VTD_CONTEXT_ENTRY_FPD; if (ret_fr) { ret_fr = -ret_fr; Looking at how things look on hardware, multiple devices often receive overlapping DMA address ranges for different physical addresses. So if I understand the way this works, every requester ID would also need to have it's own unique VTDAddressSpace, as each pci device/function sees a unique DMA address space.. ioh3420 is a pcie-to-pcie bridge, right? Yes. In my opinion, each pci-e device behind the pcie-to-pcie bridge can be assigned individually. For now I added the VT-d to q35 by just adding it to the root pci bus. You can see here in q35.c: pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu); So if we add a pcie-to-pcie bridge, we may have to call the pci_setup_iommu() for that new bus. I don't know where to hook into this now. :) If you know the mechanism behind that, you can try to add that for the new bus. (I will dive into this after the clean up.) What do you think? Thanks for the quick answer, that helped a lot! Looking into the details here I realize it is slightly more complicated: secondary buses are enumerated after device instantiation, as part of the host PCI enumeration, so if I add a similar setup call in the bridge setup, it will be called for a new device long before it has received it's bus number from the OS (via config[PCI_SECONDARY_BUS] ) I agree that the lookup function for contexts needs to be as efficient as possible so the simple busno,defvn lookup key may be the best solution but then the address_spaces table cannot be populated with the secondary bus entries before it receives a nonzero != 255 bus number, eg. along the lines of this: diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index 4becdc1..d9a8c23 100644 --- a/hw/pci/pci_bridge.c +++ b/hw/pci/pci_bridge.c @@ -265,6 +265,12 @@ void pci_bridge_write_config(PCIDevice *d, pci_bridge_update_mappings(s); } +if (ranges_overlap(address, len, PCI_SECONDARY_BUS, 1)) { +int bus_num = pci_bus_num(s-sec_bus); +if (bus_num != 0xff bus_num != 0x00
Re: [Qemu-devel] [PATCH v3 0/5] intel-iommu: introduce Intel IOMMU (VT-d) emulation to q35 chipset
On 2014-08-16 10:45, Jan Kiszka wrote: On 2014-08-16 09:54, Knut Omang wrote: On Fri, 2014-08-15 at 19:37 +0800, Le Tan wrote: Hi Knut, 2014-08-15 19:15 GMT+08:00 Knut Omang knut.om...@oracle.com: On Fri, 2014-08-15 at 06:42 +0200, Knut Omang wrote: On Thu, 2014-08-14 at 14:10 +0200, Jan Kiszka wrote: On 2014-08-14 13:15, Michael S. Tsirkin wrote: On Mon, Aug 11, 2014 at 03:04:57PM +0800, Le Tan wrote: Hi, These patches are intended to introduce Intel IOMMU (VT-d) emulation to q35 chipset. The major job in these patches is to add support for emulating Intel IOMMU according to the VT-d specification, including basic responses to CSRs accesses, the logics of DMAR (DMA remapping) and DMA memory address translations. Thanks! Looks very good overall, I noted some coding style issues - I didn't bother reporting each issue in every place where it appears - reported each issue once only, so please find and fix all instances of each issue. BTW, because I was in urgent need for virtual test environment for Jailhouse, I hacked interrupt remapping on top of Le's patches: http://git.kiszka.org/?p=qemu.git;a=shortlog;h=refs/heads/queues/vtd-intremap The approach likely needs further discussions and refinements but it already allows me to work on top with our hypervisor, and also Linux. You can see from the last commit that Le's work made it pretty easy to build this on top. Le, I have tried Jan's branch with my device setup which consists of a minimal q35 setup, an ioh3420 root port (specified as -device ioh3420,slot=0 ) and a pcie device plugged into that root port, which gives the following lscpi -t: -[:00]-+-00.0 +-01.0 +-02.0 +-03.0-[01]00.0 +-04.0 +-1f.0 +-1f.2 \-1f.3 All seems to work beautifully (I see the ISA bridge happily receive translations) until the first DMA from my device model (at 1:00.0) arrives, at which point I get: [ 1663.732413] dmar: DMAR:[DMA Write] Request device [00:03.0] fault addr fffa [ 1663.732413] DMAR:[fault reason 02] Present bit in context entry is clear I would have expected request device 01:00.0 for this. It is not clear to me yet if this is a weakness of the implementation of ioh3420 or the iommu. Just wanted to let you know right away in case you can shed some light to it or it is an easy fix, The device uses pci_dma_rw with itself as device pointer. To verify my hypothesis: with this rude hack my device now works much better: @@ -774,6 +780,8 @@ static void iommu_translate(VTDAddressSpace *vtd_as, int bus_num, int devfn, is_fpd_set = ce.lo VTD_CONTEXT_ENTRY_FPD; } else { ret_fr = dev_to_context_entry(s, bus_num, devfn, ce); +if (ret_fr) +ret_fr = dev_to_context_entry(s, 1, 0, ce); is_fpd_set = ce.lo VTD_CONTEXT_ENTRY_FPD; if (ret_fr) { ret_fr = -ret_fr; Looking at how things look on hardware, multiple devices often receive overlapping DMA address ranges for different physical addresses. So if I understand the way this works, every requester ID would also need to have it's own unique VTDAddressSpace, as each pci device/function sees a unique DMA address space.. ioh3420 is a pcie-to-pcie bridge, right? Yes. In my opinion, each pci-e device behind the pcie-to-pcie bridge can be assigned individually. For now I added the VT-d to q35 by just adding it to the root pci bus. You can see here in q35.c: pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu); So if we add a pcie-to-pcie bridge, we may have to call the pci_setup_iommu() for that new bus. I don't know where to hook into this now. :) If you know the mechanism behind that, you can try to add that for the new bus. (I will dive into this after the clean up.) What do you think? Thanks for the quick answer, that helped a lot! Looking into the details here I realize it is slightly more complicated: secondary buses are enumerated after device instantiation, as part of the host PCI enumeration, so if I add a similar setup call in the bridge setup, it will be called for a new device long before it has received it's bus number from the OS (via config[PCI_SECONDARY_BUS] ) I agree that the lookup function for contexts needs to be as efficient as possible so the simple busno,defvn lookup key may be the best solution but then the address_spaces table cannot be populated with the secondary bus entries before it receives a nonzero != 255 bus number, eg. along the lines of this: diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index 4becdc1..d9a8c23 100644 --- a/hw/pci/pci_bridge.c +++ b/hw/pci/pci_bridge.c @@ -265,6 +265,12 @@ void pci_bridge_write_config(PCIDevice *d, pci_bridge_update_mappings(s); } +if (ranges_overlap(address, len, PCI_SECONDARY_BUS, 1)) { +int bus_num = pci_bus_num(s-sec_bus
Re: [Qemu-devel] [PATCH v3 3/5] intel-iommu: add DMAR table to ACPI tables
On 2014-08-14 13:06, Michael S. Tsirkin wrote: On Mon, Aug 11, 2014 at 03:05:00PM +0800, Le Tan wrote: Expose Intel IOMMU to the BIOS. If object of TYPE_INTEL_IOMMU_DEVICE exists, add DMAR table to ACPI RSDT table. For now the DMAR table indicates that there is only one hardware unit without INTR_REMAP capability on the platform. Signed-off-by: Le Tan tamlokv...@gmail.com Could you add a unit test please? While unit tests would really be helpful, I'm afraid that's not in reach (GSoC is almost over). The good news is that we have pretty broad test coverage with both Linux and also Jailhouse already. Do you see unit tests as precondition for merging the series? Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3 3/5] intel-iommu: add DMAR table to ACPI tables
On 2014-08-14 13:43, Michael S. Tsirkin wrote: On Thu, Aug 14, 2014 at 01:36:49PM +0200, Jan Kiszka wrote: On 2014-08-14 13:06, Michael S. Tsirkin wrote: On Mon, Aug 11, 2014 at 03:05:00PM +0800, Le Tan wrote: Expose Intel IOMMU to the BIOS. If object of TYPE_INTEL_IOMMU_DEVICE exists, add DMAR table to ACPI RSDT table. For now the DMAR table indicates that there is only one hardware unit without INTR_REMAP capability on the platform. Signed-off-by: Le Tan tamlokv...@gmail.com Could you add a unit test please? While unit tests would really be helpful, I'm afraid that's not in reach (GSoC is almost over). The good news is that we have pretty broad test coverage with both Linux and also Jailhouse already. Do you see unit tests as precondition for merging the series? Jan Not a pre-requisite - it's very easy to add a unit test, just add a case in test_acpi_tcg. So I can do it myself afterwards. Ah, ok, that's tests/bios-tables-test.c. Maybe Le can have a look if time is left. But hard pecils-down is already on the 18th. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3 0/5] intel-iommu: introduce Intel IOMMU (VT-d) emulation to q35 chipset
On 2014-08-14 13:15, Michael S. Tsirkin wrote: On Mon, Aug 11, 2014 at 03:04:57PM +0800, Le Tan wrote: Hi, These patches are intended to introduce Intel IOMMU (VT-d) emulation to q35 chipset. The major job in these patches is to add support for emulating Intel IOMMU according to the VT-d specification, including basic responses to CSRs accesses, the logics of DMAR (DMA remapping) and DMA memory address translations. Thanks! Looks very good overall, I noted some coding style issues - I didn't bother reporting each issue in every place where it appears - reported each issue once only, so please find and fix all instances of each issue. BTW, because I was in urgent need for virtual test environment for Jailhouse, I hacked interrupt remapping on top of Le's patches: http://git.kiszka.org/?p=qemu.git;a=shortlog;h=refs/heads/queues/vtd-intremap The approach likely needs further discussions and refinements but it already allows me to work on top with our hypervisor, and also Linux. You can see from the last commit that Le's work made it pretty easy to build this on top. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] MSI-X interrupt emulation
On 2014-08-01 08:22, Danzer, Uwe wrote: Hi there, I'm implementing an emulated PCIe Memory class device, but can't get MSI-X interrupt emulation working. So far, the card appears in the guest system and the driver for the card recognises it and the emulation of 1MB of accessible r/w registers works as desired. As the real card is connected to the outside world, it can signal events from there to the software via 5 MSI-X interrupts. Though I do not manage do get MSI-X emulation working in my implementation. The guest OS is QNX and works just fine on the real hardware. Running QNX as guest inside QEMU, the command pci -vvv (QNX equivalent of lspci on Linux) shows my card and that it says it's able to do the desired 5 MSI-X interrupts, but the QNX driver doesn't activate MSI-X for the card. I suppose the config space layouts are identical between emulated and real card? Just in case the QNX driver has hard-coded capability offsets (it shouldn't, but who knows). Do you have the source code of the driver? In my init function of the PCIe card, I try to make MSI-X available with this code: ret = msix_init_exclusive_bar (dev, 5, 1); if (ret) { printf(msix_init() failed\n); } else { int i; for (i = 0; i 5; i++) { msix_vector_use (dev, i); } msix = 1; } Can someone tell me, what is wrong or missing in my code or does somebody have a minimal example for a (pseudo)device with MSI-X? If the problem still persists and you can't share your code here, drop me an email directly, and I can have a look or suggest further instrumentations. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH][RESEND] pci: Use bus master address space for delivering MSI/MSI-X, messages
The spec says (and real HW confirms this) that, if the bus master bit is 0, the device will not generate any PCI accesses. MSI and MSI-X messages fall among these, so we should use the corresponding address space to deliver them. This will prevent delivery if bus master support is disabled. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Reviewed-by: Paolo Bonzini pbonz...@redhat.com Reviewed-by: Michael S. Tsirkin m...@redhat.com --- hw/pci/msi.c | 2 +- hw/pci/msix.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/pci/msi.c b/hw/pci/msi.c index a4a3040..52d2313 100644 --- a/hw/pci/msi.c +++ b/hw/pci/msi.c @@ -291,7 +291,7 @@ void msi_notify(PCIDevice *dev, unsigned int vector) notify vector 0x%x address: 0x%PRIx64 data: 0x%PRIx32\n, vector, msg.address, msg.data); -stl_le_phys(address_space_memory, msg.address, msg.data); +stl_le_phys(dev-bus_master_as, msg.address, msg.data); } /* Normally called by pci_default_write_config(). */ diff --git a/hw/pci/msix.c b/hw/pci/msix.c index 5c49bfc..20ae476 100644 --- a/hw/pci/msix.c +++ b/hw/pci/msix.c @@ -439,7 +439,7 @@ void msix_notify(PCIDevice *dev, unsigned vector) msg = msix_get_message(dev, vector); -stl_le_phys(address_space_memory, msg.address, msg.data); +stl_le_phys(dev-bus_master_as, msg.address, msg.data); } void msix_reset(PCIDevice *dev) -- 1.8.1.1.298.ge7eed54
Re: [Qemu-devel] [Qemu-trivial] [PATCH] apic: Fix reported DFR content
On 2014-08-13 08:14, Michael Tokarev wrote: 09.08.2014 18:05, Jan Kiszka wrote: From: Jan Kiszka jan.kis...@siemens.com IA-32 SDM, Figure 10-14: Bits 27:0 are reserved as 1. Fixes Jailhouse hypervisor start with in-kernel irqchips off. Applied to -trivial, thank you! Are the other similar cases in there okay? Say, 0x0d or 0x02 which also uses shifts like this one? Nope, didn't find any. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] apic: Fix reported DFR content
From: Jan Kiszka jan.kis...@siemens.com IA-32 SDM, Figure 10-14: Bits 27:0 are reserved as 1. Fixes Jailhouse hypervisor start with in-kernel irqchips off. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/intc/apic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/intc/apic.c b/hw/intc/apic.c index ef19e55..03ff9e9 100644 --- a/hw/intc/apic.c +++ b/hw/intc/apic.c @@ -698,7 +698,7 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr) val = s-log_dest 24; break; case 0x0e: -val = s-dest_mode 28; +val = (s-dest_mode 28) | 0xfff; break; case 0x0f: val = s-spurious_vec; -- 1.8.1.1.298.ge7eed54 signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] About the VT-d features that q35 chipset supports
On 2014-08-05 14:55, Le Tan wrote: Hi, I am now adding features to the VT-d emulation for q35 chipset. I think it is good to know what features q35 chipset supports exactly. However I can't find materials about this. Does anyone know this? Or it will be fine if someone can tell me the value of the Capability Register and Extended Capability Register of the VT-d on q35. The kernel reports those registers during boot-up, so we would just need dmesg | grep dmar from a real 82Q35 box (Intel 3 Series). Jan Any help is appreciated! Thanks very much! Regards, Le signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] vfio in the guest: no available reset mechanism
On 2014-08-01 18:35, Alex Williamson wrote: On Fri, 2014-08-01 at 09:25 -0600, Alex Williamson wrote: On Fri, 2014-08-01 at 09:35 +0800, Le Tan wrote: Hi Alex, 2014-07-30 22:46 GMT+08:00 Alex Williamson alex.william...@redhat.com: On Wed, 2014-07-30 at 22:16 +0800, Le Tan wrote: Hi Michael, 2014-07-30 21:16 GMT+08:00 Michael S. Tsirkin m...@redhat.com: On Wed, Jul 30, 2014 at 08:24:04PM +0800, Le Tan wrote: Hi, I am testing vfio in L1 with my VT-d emulation project. I assigned one of the two AHCI controllers in L1 to L2 via vfio. After I ran the QEMU in L1, it complains that: qemu-system-x86_64: vfio: Cannot reset device :00:03.0, no available reset mechanism. qemu-system-x86_64: vfio: Cannot reset device :00:03.0, no available reset mechanism. Then L2 paused when the SeaBIOS executed in ahci_controller_setup(). I look into this and found that: val = ahci_ctrl_readl(ctrl, HOST_CTL); ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_AHCI_EN); When the BIOS tried to read the HOST_CTL, it returns 0x8002, which bit 2 (Interrupt Enable) is 1. The AHCI manual says that this bit should be cleared by default. So maybe L1 didn't reset the device before assigning it to L2? Then the BIOS tried to write back to HOST_CTL and it was stuck here. :( So can anyone give me some advice? About the state of PCI device or bus-level reset? Here is the detail of the environment and the way I did the vfio. 1. lspci in L1 said: 00:03.0 SATA controller [0106]: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port SATA Controller [AHCI mode] [8086:2922] (rev 02) 00:1f.0 ISA bridge [0601]: Intel Corporation 82801IB (ICH9) LPC Interface Controller [8086:2918] (rev 02) 00:1f.2 SATA controller [0106]: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6 port SATA Controller [AHCI mode] [8086:2922] (rev 02) 00:1f.3 SMBus [0c05]: Intel Corporation 82801I (ICH9 Family) SMBus Controller [8086:2930] (rev 02) 2. Unbind 00:03.0 and do vfio: modprobe -r vfio_iommu_type1 modprobe vfio_iommu_type1 allow_unsafe_interrupts=1 modprobe vfio-pci echo :00:03.0 /sys/bus/pci/devices/\:00\:03.0/driver/unbind echo 8086 2922 /sys/bus/pci/drivers/vfio-pci/new_id 3. run L2 with -device vfio-pci,host=00:03.0 Any help is appreciated! Thanks very much! Regards, Le Clearly, bus level reset can't work for the root bus :) Thanks very much! I test the vfio with a second-bus ahci controller and it didn't complain about the lack of reset mechanism. :) And the return val of HOST_CTL is normal now (the same as emulated ahci controller). However, it still paused when the BIOS tried to write to the HOST_CTL. Do you have any idea? And we should just test vfio and legacy pci-assignment with second bus devices, not considering the root-bus devices? AHCI seems like a poor choice of device for this work, they typically don't support any kind of reset and they can be troublesome even for the L1 assignment. You really want something with FLR support so that both the host and L1 guest can potentially reset the device. That said, you may still run into bugs with a L1 guest directed FLR. Thanks, Alex So what device do you think is suitable for the pci-assign test? e1000? I just tested it with sound card ac97 and USB controller. But I don't know how to attach them to a pcie-to-pci bridge, so maybe they weren't reset before being assigned to L2. But it seems that they can work. 1. With the sound card, I assigned it to L2 via both vfio and legacy pci-assign and I could hear the music played in L2 from host's speakers. Of course, the vfio also complained about the lack of reset mechanism. First off, maybe I'm a little confused, are you trying to assigned emulated devices for the L1 guest to the L2 guest or are these assigned devices to the L1 guest that you want to re-assign to L2? AFAIK, we don't have any emulated devices that support reset, but it wouldn't be that hard to add a PM capability to one that advertises a soft reset on D3hot-D0 transition and calls the QEMU driver reset function when that occurs. This would provide the most flexibility. Or even better might be to add an Advanced Features capability to each device so it can do an explicit FLR. We don't tend to trust PM reset as being reliably on all devices. But FLR is PCIe-only, isn't it? Also, it may let some of our device models deviate from their real versions (I suppose, e.g., none of the e1000 devices we currently emulate exposed FLR). In any case, I suggested Le to focus on his GSoC task again (VT-d emulation) and postpone this topic to later point (likely outside GSoC). Only few weeks left... Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] vfio in the guest: no available reset mechanism
On 2014-08-01 19:16, Alex Williamson wrote: Also, it may let some of our device models deviate from their real versions (I suppose, e.g., none of the e1000 devices we currently emulate exposed FLR). Of course, but what are the chances that the driver will care? No drivers of GPOSes, but special or legacy OSes may do so. Keep in mind that Intel e.g. is documenting their PCI devices with fixed config space addresses for their capability. Also, we completely lack PM caps so far. Adding them would already have the value of increasing emulation accuracy. And here I think we are free to always implement reset behavior behind D3-D0 transitions. So my believe is that this will be the better path. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH 2/3] pc: Create 2.2 machine type
Yet identical to 2.1. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/i386/pc_piix.c| 19 --- hw/i386/pc_q35.c | 17 +++-- include/hw/i386/pc.h | 3 +++ 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 73ba77d..5edf42a 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -450,16 +450,28 @@ static void pc_xen_hvm_init(MachineState *machine) .desc = Standard PC (i440FX + PIIX, 1996), \ .hot_add_cpu = pc_hot_add_cpu -#define PC_I440FX_2_1_MACHINE_OPTIONS \ +#define PC_I440FX_2_2_MACHINE_OPTIONS \ PC_I440FX_MACHINE_OPTIONS, \ .default_machine_opts = firmware=bios-256k.bin +static QEMUMachine pc_i440fx_machine_v2_2 = { +PC_I440FX_2_2_MACHINE_OPTIONS, +.name = pc-i440fx-2.2, +.alias = pc, +.init = pc_init_pci, +.is_default = 1, +}; + +#define PC_I440FX_2_1_MACHINE_OPTIONS PC_I440FX_2_2_MACHINE_OPTIONS + static QEMUMachine pc_i440fx_machine_v2_1 = { PC_I440FX_2_1_MACHINE_OPTIONS, .name = pc-i440fx-2.1, -.alias = pc, .init = pc_init_pci, -.is_default = 1, +.compat_props = (GlobalProperty[]) { +PC_COMPAT_2_1, +{ /* end of list */ } +}, }; #define PC_I440FX_2_0_MACHINE_OPTIONS PC_I440FX_2_1_MACHINE_OPTIONS @@ -896,6 +908,7 @@ static QEMUMachine xenfv_machine = { static void pc_machine_init(void) { +qemu_register_pc_machine(pc_i440fx_machine_v2_2); qemu_register_pc_machine(pc_i440fx_machine_v2_1); qemu_register_pc_machine(pc_i440fx_machine_v2_0); qemu_register_pc_machine(pc_i440fx_machine_v1_7); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index c39ee98..fcc3ad7 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -348,15 +348,27 @@ static void pc_q35_init_1_4(MachineState *machine) .desc = Standard PC (Q35 + ICH9, 2009), \ .hot_add_cpu = pc_hot_add_cpu -#define PC_Q35_2_1_MACHINE_OPTIONS \ +#define PC_Q35_2_2_MACHINE_OPTIONS \ PC_Q35_MACHINE_OPTIONS, \ .default_machine_opts = firmware=bios-256k.bin +static QEMUMachine pc_q35_machine_v2_2 = { +PC_Q35_2_2_MACHINE_OPTIONS, +.name = pc-q35-2.2, +.alias = q35, +.init = pc_q35_init, +}; + +#define PC_Q35_2_1_MACHINE_OPTIONS PC_Q35_2_2_MACHINE_OPTIONS + static QEMUMachine pc_q35_machine_v2_1 = { PC_Q35_2_1_MACHINE_OPTIONS, .name = pc-q35-2.1, -.alias = q35, .init = pc_q35_init, +.compat_props = (GlobalProperty[]) { +PC_COMPAT_2_1, +{ /* end of list */ } +}, }; #define PC_Q35_2_0_MACHINE_OPTIONS PC_Q35_2_1_MACHINE_OPTIONS @@ -421,6 +433,7 @@ static QEMUMachine pc_q35_machine_v1_4 = { static void pc_q35_machine_init(void) { +qemu_register_pc_machine(pc_q35_machine_v2_2); qemu_register_pc_machine(pc_q35_machine_v2_1); qemu_register_pc_machine(pc_q35_machine_v2_0); qemu_register_pc_machine(pc_q35_machine_v1_7); diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index f4b9b2b..0fb4200 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -295,7 +295,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); int e820_get_num_entries(void); bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); +#define PC_COMPAT_2_1 + #define PC_COMPAT_2_0 \ +PC_COMPAT_2_1, \ {\ .driver = virtio-scsi-pci,\ .property = any_layout,\ -- 1.8.1.1.298.ge7eed54
[Qemu-devel] [PATCH 3/3] hw/audio/intel-hda: Fix MSI capability address
According to ICH9 spec, the MSI capability is located at 0x60. This is important for guest drivers that do not parse the capability chain and use absolute addresses instead. CC: Gerd Hoffmann kra...@redhat.com Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/audio/intel-hda.c | 4 +++- include/hw/i386/pc.h | 7 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c index aa49b47..0ac911e 100644 --- a/hw/audio/intel-hda.c +++ b/hw/audio/intel-hda.c @@ -187,6 +187,7 @@ struct IntelHDAState { /* properties */ uint32_t debug; uint32_t msi; +bool old_msi_addr; }; #define TYPE_INTEL_HDA_GENERIC intel-hda-generic @@ -1141,7 +1142,7 @@ static int intel_hda_init(PCIDevice *pci) intel-hda, 0x4000); pci_register_bar(d-pci, 0, 0, d-mmio); if (d-msi) { -msi_init(d-pci, 0x50, 1, true, false); +msi_init(d-pci, d-old_msi_addr ? 0x50 : 0x60, 1, true, false); } hda_codec_bus_init(DEVICE(pci), d-codecs, sizeof(d-codecs), @@ -1236,6 +1237,7 @@ static const VMStateDescription vmstate_intel_hda = { static Property intel_hda_properties[] = { DEFINE_PROP_UINT32(debug, IntelHDAState, debug, 0), DEFINE_PROP_UINT32(msi, IntelHDAState, msi, 1), +DEFINE_PROP_BOOL(old_msi_addr, IntelHDAState, old_msi_addr, false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 0fb4200..be8fdfe 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -295,7 +295,12 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); int e820_get_num_entries(void); bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); -#define PC_COMPAT_2_1 +#define PC_COMPAT_2_1 \ +{\ +.driver = intel-hda,\ +.property = old_msi_addr,\ +.value= on,\ +} #define PC_COMPAT_2_0 \ PC_COMPAT_2_1, \ -- 1.8.1.1.298.ge7eed54
[Qemu-devel] [PATCH 0/3] pc: Small fixes for Intel HDA config space and PC_COMPAT_1_0
Patch 1 is a repost, likely a harmless issue as no one should have used QEMU 1.0 for such purposes that require compat migration (rather qemu-kvm at that time). Patch 2 prepares compat support for 2.1 machines and the last one fixes the MSI capability address of intel-hda in backward-compatible way. CC: Gerd Hoffmann kra...@redhat.com Jan Kiszka (3): pc: Fix disabling of vapic for compat PC models pc: Create 2.2 machine type hw/audio/intel-hda: Fix MSI capability address hw/audio/intel-hda.c | 4 +++- hw/i386/pc_piix.c| 21 + hw/i386/pc_q35.c | 17 +++-- include/hw/i386/pc.h | 8 4 files changed, 43 insertions(+), 7 deletions(-) -- 1.8.1.1.298.ge7eed54
[Qemu-devel] [PATCH 1/3] pc: Fix disabling of vapic for compat PC models
We used to be able to address both the QEMU and the KVM APIC via apic. This doesn't work anymore. So we need to use their parent class to turn off the vapic on machines that should not expose them. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/i386/pc_piix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 9694f88..73ba77d 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -645,7 +645,7 @@ static QEMUMachine pc_machine_v1_1 = { .property = class,\ .value= stringify(PCI_CLASS_MEMORY_RAM),\ },{\ -.driver = apic,\ +.driver = apic-common,\ .property = vapic,\ .value= off,\ },{\ -- 1.8.1.1.298.ge7eed54
Re: [Qemu-devel] [PATCH 2/3] pc: Create 2.2 machine type
On 2014-07-30 10:41, Alex Bligh wrote: On 30/07/2014 08:02, Jan Kiszka wrote: Yet identical to 2.1. If it's *really* identical to 2.1, are we going to add the 2.2 machine type to 2.1 as well, so that things that have difficulty using different source and destination machine types can migrate from 2.2 to 2.1? Or am I missing the point and is the issue that 2.2 is actually different to 2.1, not in the machine type definition, but in what is sent within the migrated data? If so, a commit message to this effect might be helpful. See patch 3 for the (current) motivation of this patch. Maybe more will come as 2.2 evolves. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] hw/audio/intel-hda: Fix MSI capability address
On 2014-07-28 10:11, Paolo Bonzini wrote: Il 27/07/2014 08:57, Jan Kiszka ha scritto: From: Jan Kiszka jan.kis...@siemens.com According to ICH9 spec, the MSI capability is located at 0x60. This is important for guest drivers that do not parse the capability chain and use absolute addresses instead. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/audio/intel-hda.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c index aa49b47..09c4118 100644 --- a/hw/audio/intel-hda.c +++ b/hw/audio/intel-hda.c @@ -1141,7 +1141,7 @@ static int intel_hda_init(PCIDevice *pci) intel-hda, 0x4000); pci_register_bar(d-pci, 0, 0, d-mmio); if (d-msi) { -msi_init(d-pci, 0x50, 1, true, false); +msi_init(d-pci, 0x60, 1, true, false); } hda_codec_bus_init(DEVICE(pci), d-codecs, sizeof(d-codecs), Does this need a compat property? Sigh, right, it's guest visible. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] hw/audio/intel-hda: Fix MSI capability address
From: Jan Kiszka jan.kis...@siemens.com According to ICH9 spec, the MSI capability is located at 0x60. This is important for guest drivers that do not parse the capability chain and use absolute addresses instead. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/audio/intel-hda.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c index aa49b47..09c4118 100644 --- a/hw/audio/intel-hda.c +++ b/hw/audio/intel-hda.c @@ -1141,7 +1141,7 @@ static int intel_hda_init(PCIDevice *pci) intel-hda, 0x4000); pci_register_bar(d-pci, 0, 0, d-mmio); if (d-msi) { -msi_init(d-pci, 0x50, 1, true, false); +msi_init(d-pci, 0x60, 1, true, false); } hda_codec_bus_init(DEVICE(pci), d-codecs, sizeof(d-codecs), -- 1.8.1.1.298.ge7eed54 signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] pci: Use bus master address space for delivering MSI/MSI-X messages
From: Jan Kiszka jan.kis...@siemens.com The spec says (and real HW confirms this) that, if the bus master bit is 0, the device will not generate any PCI accesses. MSI and MSI-X messages fall among these, so we should use the corresponding address space to deliver them. This will prevent delivery if bus master support is disabled. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/pci/msi.c | 2 +- hw/pci/msix.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/pci/msi.c b/hw/pci/msi.c index a4a3040..52d2313 100644 --- a/hw/pci/msi.c +++ b/hw/pci/msi.c @@ -291,7 +291,7 @@ void msi_notify(PCIDevice *dev, unsigned int vector) notify vector 0x%x address: 0x%PRIx64 data: 0x%PRIx32\n, vector, msg.address, msg.data); -stl_le_phys(address_space_memory, msg.address, msg.data); +stl_le_phys(dev-bus_master_as, msg.address, msg.data); } /* Normally called by pci_default_write_config(). */ diff --git a/hw/pci/msix.c b/hw/pci/msix.c index 5c49bfc..20ae476 100644 --- a/hw/pci/msix.c +++ b/hw/pci/msix.c @@ -439,7 +439,7 @@ void msix_notify(PCIDevice *dev, unsigned vector) msg = msix_get_message(dev, vector); -stl_le_phys(address_space_memory, msg.address, msg.data); +stl_le_phys(dev-bus_master_as, msg.address, msg.data); } void msix_reset(PCIDevice *dev) -- 1.8.1.1.298.ge7eed54 signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 3/3] intel-iommu: add Intel IOMMU emulation to q35 and add a machine option vtd as a switch
On 2014-07-22 17:47, Le Tan wrote: Add Intel IOMMU emulation to q35 chipset and expose it to the guest. 1. Add a machine option. Users can use -machine vtd=on|off in the command line to enable/disable Intel IOMMU. The default is off. Better call it iommu. We could once reuse the switch when someone adds an AMD IOMMU emulation (and a corresponding chipset as well, I suppose). And maybe other target archs will find it useful as well. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] pci: Don't deliver MSI/MSI-X messages if bus master support is off
On 2014-07-22 21:06, Michael S. Tsirkin wrote: On Mon, Jul 21, 2014 at 12:04:22AM +0200, Jan Kiszka wrote: On 2014-07-20 23:03, Michael S. Tsirkin wrote: On Sun, Jul 20, 2014 at 11:45:10PM +0200, Jan Kiszka wrote: On 2014-07-20 21:48, Michael S. Tsirkin wrote: On Sat, Jul 19, 2014 at 06:55:48PM +0200, Jan Kiszka wrote: From: Jan Kiszka jan.kis...@siemens.com The spec says (and real HW confirms this) that, if the bus master bit is 0, the device will not generate any PCI accesses. MSI and MSI-X messages fall among these. Signed-off-by: Jan Kiszka jan.kis...@siemens.com I guess an alternative is for callers to check before invoking msi_notify. Please note is this is only option when using e.g. irqfd, so this has some advantages. Is there a specific device that is affected by this? I would expect drivers to disable msi before clearing bus master bit ... This is about emulating conforming behaviour without touching each and every device. I stumbled over this while playing with emulated vs. real Intel HDA. Right so that's my question. How did you hit it? With a custom driver? So to say: with a hand full lines of code to tickle some MSI event out of that device for testing purposes. Doesn't regulat driver disable MSI? Sure. This is not fixing a regular's driver problem. It's a behavioral correction for faulty corner cases. Jan OK based on this I think this is not 2.1 material. Agree? Agree. I'll look into Paolo's suggestion how to model this asap. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] kvm-all: Use 'tmpcpu' instead of 'cpu' in sub-looping to avoid 'cpu' be NULL
On 2014-07-19 03:21, Chen Gang wrote: If kvm_arch_remove_sw_breakpoint() in CPU_FOREACH() always be fail, it will let 'cpu' NULL. And the next kvm_arch_remove_sw_breakpoint() in QTAILQ_FOREACH_SAFE() will get NULL parameter for 'cpu'. And kvm_arch_remove_sw_breakpoint() can assumes 'cpu' must never be NULL, so need define additional temporary variable for 'cpu' to avoid the case. Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- kvm-all.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 3ae30ee..1402f4f 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -2077,12 +2077,13 @@ void kvm_remove_all_breakpoints(CPUState *cpu) { struct kvm_sw_breakpoint *bp, *next; KVMState *s = cpu-kvm_state; +CPUState *tmpcpu; QTAILQ_FOREACH_SAFE(bp, s-kvm_sw_breakpoints, entry, next) { if (kvm_arch_remove_sw_breakpoint(cpu, bp) != 0) { /* Try harder to find a CPU that currently sees the breakpoint. */ -CPU_FOREACH(cpu) { -if (kvm_arch_remove_sw_breakpoint(cpu, bp) == 0) { +CPU_FOREACH(tmpcpu) { +if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) { break; } } Good catch. To make it clear in the changelog: The actual issue is that we misuse cpu as an iteration variable while its original value is still in use. That cpu can eventually become NULL this way is one result. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] pci: Don't deliver MSI/MSI-X messages if bus master support is off
On 2014-07-20 21:48, Michael S. Tsirkin wrote: On Sat, Jul 19, 2014 at 06:55:48PM +0200, Jan Kiszka wrote: From: Jan Kiszka jan.kis...@siemens.com The spec says (and real HW confirms this) that, if the bus master bit is 0, the device will not generate any PCI accesses. MSI and MSI-X messages fall among these. Signed-off-by: Jan Kiszka jan.kis...@siemens.com I guess an alternative is for callers to check before invoking msi_notify. Please note is this is only option when using e.g. irqfd, so this has some advantages. Is there a specific device that is affected by this? I would expect drivers to disable msi before clearing bus master bit ... This is about emulating conforming behaviour without touching each and every device. I stumbled over this while playing with emulated vs. real Intel HDA. It may not be complete, but I think it's a step forward. Irqfd users apparently have to do this themselves then, I didn't look into this. But all the rest should not open-code this logic. Jan --- hw/pci/msi.c | 4 hw/pci/msix.c | 4 2 files changed, 8 insertions(+) diff --git a/hw/pci/msi.c b/hw/pci/msi.c index a4a3040..36b651b 100644 --- a/hw/pci/msi.c +++ b/hw/pci/msi.c @@ -285,6 +285,10 @@ void msi_notify(PCIDevice *dev, unsigned int vector) return; } +if (!(pci_get_word(dev-config + PCI_COMMAND) PCI_COMMAND_MASTER)) { +return; +} + msg = msi_get_message(dev, vector); MSI_DEV_PRINTF(dev, diff --git a/hw/pci/msix.c b/hw/pci/msix.c index 5c49bfc..c77ae7d 100644 --- a/hw/pci/msix.c +++ b/hw/pci/msix.c @@ -437,6 +437,10 @@ void msix_notify(PCIDevice *dev, unsigned vector) return; } +if (!(pci_get_word(dev-config + PCI_COMMAND) PCI_COMMAND_MASTER)) { +return; +} + msg = msix_get_message(dev, vector); stl_le_phys(address_space_memory, msg.address, msg.data); -- 1.8.1.1.298.ge7eed54 signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] pci: Don't deliver MSI/MSI-X messages if bus master support is off
On 2014-07-20 23:03, Michael S. Tsirkin wrote: On Sun, Jul 20, 2014 at 11:45:10PM +0200, Jan Kiszka wrote: On 2014-07-20 21:48, Michael S. Tsirkin wrote: On Sat, Jul 19, 2014 at 06:55:48PM +0200, Jan Kiszka wrote: From: Jan Kiszka jan.kis...@siemens.com The spec says (and real HW confirms this) that, if the bus master bit is 0, the device will not generate any PCI accesses. MSI and MSI-X messages fall among these. Signed-off-by: Jan Kiszka jan.kis...@siemens.com I guess an alternative is for callers to check before invoking msi_notify. Please note is this is only option when using e.g. irqfd, so this has some advantages. Is there a specific device that is affected by this? I would expect drivers to disable msi before clearing bus master bit ... This is about emulating conforming behaviour without touching each and every device. I stumbled over this while playing with emulated vs. real Intel HDA. Right so that's my question. How did you hit it? With a custom driver? So to say: with a hand full lines of code to tickle some MSI event out of that device for testing purposes. Doesn't regulat driver disable MSI? Sure. This is not fixing a regular's driver problem. It's a behavioral correction for faulty corner cases. Jan It may not be complete, but I think it's a step forward. Irqfd users apparently have to do this themselves then, I didn't look into this. But all the rest should not open-code this logic. Jan --- hw/pci/msi.c | 4 hw/pci/msix.c | 4 2 files changed, 8 insertions(+) diff --git a/hw/pci/msi.c b/hw/pci/msi.c index a4a3040..36b651b 100644 --- a/hw/pci/msi.c +++ b/hw/pci/msi.c @@ -285,6 +285,10 @@ void msi_notify(PCIDevice *dev, unsigned int vector) return; } +if (!(pci_get_word(dev-config + PCI_COMMAND) PCI_COMMAND_MASTER)) { +return; +} + msg = msi_get_message(dev, vector); MSI_DEV_PRINTF(dev, diff --git a/hw/pci/msix.c b/hw/pci/msix.c index 5c49bfc..c77ae7d 100644 --- a/hw/pci/msix.c +++ b/hw/pci/msix.c @@ -437,6 +437,10 @@ void msix_notify(PCIDevice *dev, unsigned vector) return; } +if (!(pci_get_word(dev-config + PCI_COMMAND) PCI_COMMAND_MASTER)) { +return; +} + msg = msix_get_message(dev, vector); stl_le_phys(address_space_memory, msg.address, msg.data); -- 1.8.1.1.298.ge7eed54 signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] pci: Don't deliver MSI/MSI-X messages if bus master support is off
From: Jan Kiszka jan.kis...@siemens.com The spec says (and real HW confirms this) that, if the bus master bit is 0, the device will not generate any PCI accesses. MSI and MSI-X messages fall among these. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/pci/msi.c | 4 hw/pci/msix.c | 4 2 files changed, 8 insertions(+) diff --git a/hw/pci/msi.c b/hw/pci/msi.c index a4a3040..36b651b 100644 --- a/hw/pci/msi.c +++ b/hw/pci/msi.c @@ -285,6 +285,10 @@ void msi_notify(PCIDevice *dev, unsigned int vector) return; } +if (!(pci_get_word(dev-config + PCI_COMMAND) PCI_COMMAND_MASTER)) { +return; +} + msg = msi_get_message(dev, vector); MSI_DEV_PRINTF(dev, diff --git a/hw/pci/msix.c b/hw/pci/msix.c index 5c49bfc..c77ae7d 100644 --- a/hw/pci/msix.c +++ b/hw/pci/msix.c @@ -437,6 +437,10 @@ void msix_notify(PCIDevice *dev, unsigned vector) return; } +if (!(pci_get_word(dev-config + PCI_COMMAND) PCI_COMMAND_MASTER)) { +return; +} + msg = msix_get_message(dev, vector); stl_le_phys(address_space_memory, msg.address, msg.data); -- 1.8.1.1.298.ge7eed54 signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] ahci: map memory via device's address space instead of address_space_memory
On 2014-07-03 10:26, Le Tan wrote: In map_page() in hw/ide/ahci.c, replace cpu_physical_memory_map() and cpu_physical_memory_unmap() with dma_memory_map() and dma_memory_unmap(), because ahci devices should not access memory directly but via their address space. Add an AddressSpace parameter to map_page(). In order to call map_page(), we should pass the AHCIState.as as the AddressSpace argument. BTW, when doing git grep cpu_physical_memory_map hw, there are some more cases that should be checked (for x86). I suppose vhost is incompatible with an IOMMU, but plain virtio should work, same for vmxnet. Jan Signed-off-by: Le Tan tamlokv...@gmail.com --- hw/ide/ahci.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 9bae22e..7bb0a03 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -175,17 +175,18 @@ static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d, ahci_check_irq(s); } -static void map_page(uint8_t **ptr, uint64_t addr, uint32_t wanted) +static void map_page(AddressSpace *as, uint8_t **ptr, uint64_t addr, + uint32_t wanted) { hwaddr len = wanted; if (*ptr) { -cpu_physical_memory_unmap(*ptr, len, 1, len); +dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len); } -*ptr = cpu_physical_memory_map(addr, len, 1); +*ptr = dma_memory_map(as, addr, len, DMA_DIRECTION_FROM_DEVICE); if (len wanted) { -cpu_physical_memory_unmap(*ptr, len, 1, len); +dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len); *ptr = NULL; } } @@ -198,24 +199,24 @@ static void ahci_port_write(AHCIState *s, int port, int offset, uint32_t val) switch (offset) { case PORT_LST_ADDR: pr-lst_addr = val; -map_page(s-dev[port].lst, +map_page(s-as, s-dev[port].lst, ((uint64_t)pr-lst_addr_hi 32) | pr-lst_addr, 1024); s-dev[port].cur_cmd = NULL; break; case PORT_LST_ADDR_HI: pr-lst_addr_hi = val; -map_page(s-dev[port].lst, +map_page(s-as, s-dev[port].lst, ((uint64_t)pr-lst_addr_hi 32) | pr-lst_addr, 1024); s-dev[port].cur_cmd = NULL; break; case PORT_FIS_ADDR: pr-fis_addr = val; -map_page(s-dev[port].res_fis, +map_page(s-as, s-dev[port].res_fis, ((uint64_t)pr-fis_addr_hi 32) | pr-fis_addr, 256); break; case PORT_FIS_ADDR_HI: pr-fis_addr_hi = val; -map_page(s-dev[port].res_fis, +map_page(s-as, s-dev[port].res_fis, ((uint64_t)pr-fis_addr_hi 32) | pr-fis_addr, 256); break; case PORT_IRQ_STAT: @@ -1260,9 +1261,9 @@ static int ahci_state_post_load(void *opaque, int version_id) ad = s-dev[i]; AHCIPortRegs *pr = ad-port_regs; -map_page(ad-lst, +map_page(s-as, ad-lst, ((uint64_t)pr-lst_addr_hi 32) | pr-lst_addr, 1024); -map_page(ad-res_fis, +map_page(s-as, ad-res_fis, ((uint64_t)pr-fis_addr_hi 32) | pr-fis_addr, 256); /* * All pending i/o should be flushed out on a migrate. However, signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] ahci: map memory via device's address space instead of address_space_memory
On 2014-07-03 12:02, Michael S. Tsirkin wrote: On Thu, Jul 03, 2014 at 10:43:57AM +0200, Jan Kiszka wrote: On 2014-07-03 10:26, Le Tan wrote: In map_page() in hw/ide/ahci.c, replace cpu_physical_memory_map() and cpu_physical_memory_unmap() with dma_memory_map() and dma_memory_unmap(), because ahci devices should not access memory directly but via their address space. Add an AddressSpace parameter to map_page(). In order to call map_page(), we should pass the AHCIState.as as the AddressSpace argument. BTW, when doing git grep cpu_physical_memory_map hw, there are some more cases that should be checked (for x86). I suppose vhost is incompatible with an IOMMU, vhost can be made to work: you just need to update its memory tables as appropriate. But see below but plain virtio should work, It doesn't: all guests pass in physical addresses at the moment. You mean they do not put virtio devices into IOMMU domains, or they do put them but ignore any translation rules that are not 1:1? We discussed requiring this for virtio 1.0, but in the end, most people thought that passing through virtio devices isn't worthwhile. It should be consistent at least. If virtio is not translated, we have to exclude such devices via ACPI tables from the scope of our IOMMUs. We can certainly add that as an option, with a feature bit. If you feel otherwise, you can comment on the latest spec draft. Does the spec at least state that virtio devices are not subject to any guest configured IOMMU translation? Is is this left undefined? Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] ahci: map memory via device's address space instead of address_space_memory
On 2014-07-03 22:30, Michael S. Tsirkin wrote: On Thu, Jul 03, 2014 at 06:45:03PM +0200, Jan Kiszka wrote: On 2014-07-03 12:02, Michael S. Tsirkin wrote: On Thu, Jul 03, 2014 at 10:43:57AM +0200, Jan Kiszka wrote: On 2014-07-03 10:26, Le Tan wrote: In map_page() in hw/ide/ahci.c, replace cpu_physical_memory_map() and cpu_physical_memory_unmap() with dma_memory_map() and dma_memory_unmap(), because ahci devices should not access memory directly but via their address space. Add an AddressSpace parameter to map_page(). In order to call map_page(), we should pass the AHCIState.as as the AddressSpace argument. BTW, when doing git grep cpu_physical_memory_map hw, there are some more cases that should be checked (for x86). I suppose vhost is incompatible with an IOMMU, vhost can be made to work: you just need to update its memory tables as appropriate. But see below but plain virtio should work, It doesn't: all guests pass in physical addresses at the moment. You mean they do not put virtio devices into IOMMU domains, or they do put them but ignore any translation rules that are not 1:1? Look at the code. We just pass in physical addresses ignoring which iommu domain device ended up with. Should probably be fixed, at least in Linux. I suppose there is always a 1:1 domain where virtio devices can be put in so that they are not involved in any translation if this is not desired (PPC?). We discussed requiring this for virtio 1.0, but in the end, most people thought that passing through virtio devices isn't worthwhile. It should be consistent at least. If virtio is not translated, we have to exclude such devices via ACPI tables from the scope of our IOMMUs. I didn't know this is possible. How does one do this? Both the DMAR (Intel) and the IVRS (AMD) ACPI tables allow to report the scope of an IOMMU unit. There you list sets of devices on specific buses or a complete segment that the unit virtualizes. Le currently reports that there is a single DMAR unit for q35, and that one controls the complete segment 0. The structure could be adapted if a virtio device is present so that this device is not included. That should prevent that the guest tries to control virtio devices via IOMMUs or even assign them to some L2 guests. We could then provide a machine property that controls virtualization of virtio devices, default off, but you can enable it when running properly adjusted guest drivers. We can certainly add that as an option, with a feature bit. If you feel otherwise, you can comment on the latest spec draft. Does the spec at least state that virtio devices are not subject to any guest configured IOMMU translation? Is is this left undefined? Jan I don't think we have anything like this. And I'm not even sure it necessarily belongs to the spec. I think it's just a guest driver deficit to ignore IOMMU settings that the guest OS may apply. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Why devfn will be -1
Hi Le, On 2014-07-01 04:34, Le Tan wrote: Hi Jan, I use pci_setup_iommu() to setup a PCIIOMMUFunc for the q35 pci bus. In the iommu_fn, I print out the devfn parameter and find out that it sometimes will be -1. So what does it mean? The detail code is here: In mch_init() function, I write like this: PCIBus *pci_bus = PCI_BUS(qdev_get_parent_bus(DEVICE(mch))); pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu); And in q35_host_dma_iommu(PCIBus *bus, void *opaque, int devfn), I print out the devfn parameter, sometimes it will be -1. Hmm, I have no idea about the reason and would suggest to set a conditional breakpoint on this function, then print the backtrace to see where this comes from and analyze the device structure from where that -1 was most probably taken. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PULL 2/4] net: L2TPv3 transport
On 2014-06-27 16:24, Stefan Hajnoczi wrote: From: Anton Ivanov antiv...@cisco.com This transport allows to connect a QEMU nic to a static Ethernet over L2TPv3 tunnel. The transport supports all options present in the Linux kernel implementation. It allows QEMU to connect to any Linux host running kernel 3.3+, most routers and network devices as well as other QEMU instances. [Fixed up net_client_init1() switch statement to support -netdev --Stefan] Signed-off-by: Anton Ivanov antiv...@cisco.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- net/Makefile.objs | 1 + net/clients.h | 2 + net/l2tpv3.c | 757 ++ net/net.c | 6 + qapi-schema.json | 60 + qemu-options.hx | 82 ++ 6 files changed, 908 insertions(+) create mode 100644 net/l2tpv3.c diff --git a/net/Makefile.objs b/net/Makefile.objs index 301f6b6..a06ba59 100644 --- a/net/Makefile.objs +++ b/net/Makefile.objs @@ -2,6 +2,7 @@ common-obj-y = net.o queue.o checksum.o util.o hub.o common-obj-y += socket.o common-obj-y += dump.o common-obj-y += eth.o +common-obj-$(CONFIG_LINUX) += l2tpv3.o common-obj-$(CONFIG_POSIX) += tap.o vhost-user.o common-obj-$(CONFIG_LINUX) += tap-linux.o common-obj-$(CONFIG_WIN32) += tap-win32.o diff --git a/net/clients.h b/net/clients.h index 7f3d4ae..2e8feda 100644 --- a/net/clients.h +++ b/net/clients.h @@ -47,6 +47,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name, int net_init_bridge(const NetClientOptions *opts, const char *name, NetClientState *peer); +int net_init_l2tpv3(const NetClientOptions *opts, const char *name, +NetClientState *peer); #ifdef CONFIG_VDE int net_init_vde(const NetClientOptions *opts, const char *name, NetClientState *peer); diff --git a/net/l2tpv3.c b/net/l2tpv3.c new file mode 100644 index 000..528d95b --- /dev/null +++ b/net/l2tpv3.c @@ -0,0 +1,757 @@ +/* + * QEMU System Emulator + * + * Copyright (c) 2003-2008 Fabrice Bellard + * Copyright (c) 2012-2014 Cisco Systems + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include linux/ip.h +#include netdb.h +#include config-host.h +#include net/net.h +#include clients.h +#include monitor/monitor.h +#include qemu-common.h +#include qemu/error-report.h +#include qemu/option.h +#include qemu/sockets.h +#include qemu/iov.h +#include qemu/main-loop.h + + +/* The buffer size needs to be investigated for optimum numbers and + * optimum means of paging in on different systems. This size is + * chosen to be sufficient to accommodate one packet with some headers + */ + +#define BUFFER_ALIGN sysconf(_SC_PAGESIZE) +#define BUFFER_SIZE 2048 +#define IOVSIZE 2 +#define MAX_L2TPV3_MSGCNT 64 +#define MAX_L2TPV3_IOVCNT (MAX_L2TPV3_MSGCNT * IOVSIZE) + +/* Header set to 0x3 signifies a data packet */ + +#define L2TPV3_DATA_PACKET 0x3 + +/* IANA-assigned IP protocol ID for L2TPv3 */ + +#ifndef IPPROTO_L2TP +#define IPPROTO_L2TP 0x73 +#endif + +typedef struct NetL2TPV3State { +NetClientState nc; +int fd; + +/* + * these are used for xmit - that happens packet a time + * and for first sign of life packet (easier to parse that once) + */ + +uint8_t *header_buf; +struct iovec *vec; + +/* + * these are used for receive - try to eat up to 32 packets at a time + */ + +struct mmsghdr *msgvec; struct mmsghdr is only available with recent distro kernel headers. Please check if it's there and otherwise disable L2TPv3 (or provide the struct yourself). Thanks, Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] Why devfn will be -1
On 2014-07-01 14:55, Le Tan wrote: 2014-07-01 20:52 GMT+08:00 Le Tan tamlokv...@gmail.com: Hi Jan, 2014-07-01 15:34 GMT+08:00 Jan Kiszka jan.kis...@web.de: Hi Le, On 2014-07-01 04:34, Le Tan wrote: Hi Jan, I use pci_setup_iommu() to setup a PCIIOMMUFunc for the q35 pci bus. In the iommu_fn, I print out the devfn parameter and find out that it sometimes will be -1. So what does it mean? The detail code is here: In mch_init() function, I write like this: PCIBus *pci_bus = PCI_BUS(qdev_get_parent_bus(DEVICE(mch))); pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu); And in q35_host_dma_iommu(PCIBus *bus, void *opaque, int devfn), I print out the devfn parameter, sometimes it will be -1. Hmm, I have no idea about the reason and would suggest to set a conditional breakpoint on this function, then print the backtrace to see where this comes from and analyze the device structure from where that -1 was most probably taken. I think maybe this is a bug? In the function do_pci_register_device(), maybe these two sentence should be reorder? dma_as = pci_device_iommu_address_space(pci_dev); pci_dev-devfn = devfn; Looks like. Give it a try, then possibly send a patch :) Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Why devfn will be -1
On 2014-07-01 15:02, Le Tan wrote: 2014-07-01 20:56 GMT+08:00 Jan Kiszka jan.kis...@web.de: On 2014-07-01 14:55, Le Tan wrote: 2014-07-01 20:52 GMT+08:00 Le Tan tamlokv...@gmail.com: Hi Jan, 2014-07-01 15:34 GMT+08:00 Jan Kiszka jan.kis...@web.de: Hi Le, On 2014-07-01 04:34, Le Tan wrote: Hi Jan, I use pci_setup_iommu() to setup a PCIIOMMUFunc for the q35 pci bus. In the iommu_fn, I print out the devfn parameter and find out that it sometimes will be -1. So what does it mean? The detail code is here: In mch_init() function, I write like this: PCIBus *pci_bus = PCI_BUS(qdev_get_parent_bus(DEVICE(mch))); pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu); And in q35_host_dma_iommu(PCIBus *bus, void *opaque, int devfn), I print out the devfn parameter, sometimes it will be -1. Hmm, I have no idea about the reason and would suggest to set a conditional breakpoint on this function, then print the backtrace to see where this comes from and analyze the device structure from where that -1 was most probably taken. I think maybe this is a bug? In the function do_pci_register_device(), maybe these two sentence should be reorder? dma_as = pci_device_iommu_address_space(pci_dev); pci_dev-devfn = devfn; Looks like. Give it a try, then possibly send a patch :) I reorder these two sentences and get the print log like this: vtd bus 0 slot 31 func 0 devfn 248 vtd bus 0 slot 31 func 2 devfn 250 vtd bus 0 slot 31 func 3 devfn 251 vtd bus 0 slot 1 func 0 devfn 8 vtd bus 0 slot 2 func 0 devfn 16 The info pci output is here: (qemu) info pci Bus 0, device 0, function 0: Host bridge: PCI device 8086:29c0 id Bus 0, device 1, function 0: VGA controller: PCI device 1013:00b8 BAR0: 32 bit prefetchable memory at 0xfc00 [0xfdff]. BAR1: 32 bit memory at 0xfebf [0xfebf0fff]. BAR6: 32 bit memory at 0x [0xfffe]. id Bus 0, device 2, function 0: Ethernet controller: PCI device 8086:100e IRQ 11. BAR0: 32 bit memory at 0xfebc [0xfebd]. BAR1: I/O at 0xc000 [0xc03f]. BAR6: 32 bit memory at 0x [0x0003fffe]. id Bus 0, device 31, function 0: ISA bridge: PCI device 8086:2918 id Bus 0, device 31, function 2: SATA controller: PCI device 8086:2922 IRQ 10. BAR4: I/O at 0xc080 [0xc09f]. BAR5: 32 bit memory at 0xfebf1000 [0xfebf1fff]. id Bus 0, device 31, function 3: SMBus: PCI device 8086:2930 IRQ 10. BAR4: I/O at 0x0700 [0x073f]. id So maybe it is all right now? And I will go on the vtd emulation.:) Thanks very much! Looks better :). Don't forget to send the patch against pci.c! Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] About AddressSpace in intel-iommu emulation
On 2014-06-27 07:46, Le Tan wrote: 2014-06-27 12:55 GMT+08:00 Paolo Bonzini pbonz...@redhat.com: Il 27/06/2014 04:08, Le Tan ha scritto: 1. In struct IOMMUTLBEntry, I think the addr_mask field should be the mask of the page offset, right? But I see different usages of this field. In spapr_tce_translate_iommu(), the addr_mask field is assigned with the mask of the page offset. However, in pbm_translate_iommu(), in the passthrough case, the addr_mask field seems to be assigned the mask of the page number. Is there any problem here? The intended usage is the one of spapr_tce_translate_iommu(). In practice it doesn't matter, both work. 2. For q35, how to identify origination of DMA requests? The VT-d manual says we should use source-id(for PCI-Express devices, it is requester identifier) to map devices to domains. What is the related part in QEMU? Where can I get the source-id of a DMA request? You need to create a different AddressSpace for each PCI bus or device. How to create a different AddressSpace for each device? I thought a AddressSpace just belongs to a PCI bus before. The paging structures for different functions of the same device can also be different, too. So maybe we should create a different AddressSpace for each function? How to achieve it? Could you give me some more hints or is there any existing example in QEMU? I would suggest to study the apb IOMMU implementation Paolo referenced and the PCI layer functions used by that code. Specifically, pci_setup_iommu takes a callback that is supposed to return an address space to be used for a particular device. For apb, it's the same for all devices on a bus, but that's not required... Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Using virtio for inter-VM communication
On 2014-06-17 07:24, Paolo Bonzini wrote: Il 15/06/2014 08:20, Jan Kiszka ha scritto: I think implementing Xen hypercalls in jailhouse for grant table and event channels would actually make a lot of sense. The Xen implementation is 2.5kLOC and I think it should be possible to compact it noticeably, especially if you limit yourself to 64-bit guests. At least the grant table model seems unsuited for Jailhouse. It allows a guest to influence the mapping of another guest during runtime. This we want (or even have) to avoid in Jailhouse. IIRC implementing the grant table hypercalls with copies is inefficient but valid. Back to #1: This is what Rusty is suggesting for virtio. Nothing to win with grant tables then. And if we really have to copy, I would prefer to use a standard. I guess we need to play with prototypes to assess feasibility and impact on existing code. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Using virtio for inter-VM communication
On 2014-06-13 10:45, Paolo Bonzini wrote: Il 13/06/2014 08:23, Jan Kiszka ha scritto: That would preserve zero-copy capabilities (as long as you can work against the shared mem directly, e.g. doing DMA from a physical NIC or storage device into it) and keep the hypervisor out of the loop. This seems ill thought out. How will you program a NIC via the virtio protocol without a hypervisor? And how will you make it safe? You'll need an IOMMU. But if you have an IOMMU you don't need shared memory. Scenarios behind this are things like driver VMs: You pass through the physical hardware to a driver guest that talks to the hardware and relays data via one or more virtual channels to other VMs. This confines a certain set of security and stability risks to the driver VM. I think implementing Xen hypercalls in jailhouse for grant table and event channels would actually make a lot of sense. The Xen implementation is 2.5kLOC and I think it should be possible to compact it noticeably, especially if you limit yourself to 64-bit guests. At least the grant table model seems unsuited for Jailhouse. It allows a guest to influence the mapping of another guest during runtime. This we want (or even have) to avoid in Jailhouse. I'm therefore more in favor of a model where the shared memory region is defined on cell (guest) creation by adding a virtual device that comes with such a region. Jan It should also be almost enough to run Xen PVH guests as jailhouse partitions. If later Xen starts to support virtio, you will get that for free. Paolo signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Using virtio for inter-VM communication
On 2014-06-13 02:47, Rusty Russell wrote: Jan Kiszka jan.kis...@siemens.com writes: On 2014-06-12 04:27, Rusty Russell wrote: Henning Schild henning.sch...@siemens.com writes: It was also never implemented, and remains a thought experiment. However, implementing it in lguest should be fairly easy. The reason why a trusted helper, i.e. additional logic in the hypervisor, is not our favorite solution is that we'd like to keep the hypervisor as small as possible. I wouldn't exclude such an approach categorically, but we have to weigh the costs (lines of code, additional hypervisor interface) carefully against the gain (existing specifications and guest driver infrastructure). Reasonable, but I think you'll find it is about the minimal implementation in practice. Unfortunately, I don't have time during the next 6 months to implement it myself :( Back to VIRTIO_F_RING_SHMEM_ADDR (which you once brought up in an MCA working group discussion): What speaks against introducing an alternative encoding of addresses inside virtio data structures? The idea of this flag was to replace guest-physical addresses with offsets into a shared memory region associated with or part of a virtio device. We would also need a way of defining the shared memory region. But that's not the problem. If such a feature is not accepted by the guest? How to you fall back? Depends on the hypervisor and its scope, but it should be quite straightforward: full-featured ones like KVM could fall back to slow copying, specialized ones like Jailhouse would clear FEATURES_OK if the guest driver does not accept it (because there would be no ring walking or copying code in Jailhouse), thus refuse the activate the device. That would be absolutely fine for application domains of specialized hypervisors (often embedded, customized guests etc.). The shared memory regions could be exposed as a BARs (PCI) or additional address ranges (device tree) and addressed in the redefined guest address fields via some region index and offset. We don't add features which unmake the standard. That would preserve zero-copy capabilities (as long as you can work against the shared mem directly, e.g. doing DMA from a physical NIC or storage device into it) and keep the hypervisor out of the loop. This seems ill thought out. How will you program a NIC via the virtio protocol without a hypervisor? And how will you make it safe? You'll need an IOMMU. But if you have an IOMMU you don't need shared memory. Scenarios behind this are things like driver VMs: You pass through the physical hardware to a driver guest that talks to the hardware and relays data via one or more virtual channels to other VMs. This confines a certain set of security and stability risks to the driver VM. Is it too invasive to existing infrastructure or does it have some other pitfalls? You'll have to convince every vendor to implement your addition to the standard. Which is easier than inventing a completely new system, but it's not quite virtio. It would be an optional addition, a feature all three sides (host and the communicating guests) would have to agree on. I think we would only have to agree on extending the spec to enable this - after demonstrating it via an implementation, of course. Thanks, Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] Using virtio for inter-VM communication
On 2014-06-12 04:27, Rusty Russell wrote: Henning Schild henning.sch...@siemens.com writes: Hi, i am working on the jailhouse[1] project and am currently looking at inter-VM communication. We want to connect guests directly with virtual consoles based on shared memory. The code complexity in the hypervisor should be minimal, it should just make the shared memory discoverable and provide a signaling mechanism. Hi Henning, The virtio assumption was that the host can see all of guest memory. This simplifies things significantly, and makes it efficient. If you don't have this, *someone* needs to do a copy. Usually the guest OS does a bounce buffer into your shared region. Goodbye performance. Or you can play remapping tricks. Goodbye performance again. My preferred model is to have a trusted helper (ie. host) which understands how to copy between virtio rings. The backend guest (to steal Xen vocab) R/O maps the descriptor, avail ring and used rings in the guest. It then asks the trusted helper to do various operation (copy into writable descriptor, copy out of readable descriptor, mark used). The virtio ring itself acts as a grant table. Note: that helper mechanism is completely protocol agnostic. It was also explicitly designed into the virtio mechanism (with its 4k boundaries for data structures and its 'len' field to indicate how much was written into the descriptor). It was also never implemented, and remains a thought experiment. However, implementing it in lguest should be fairly easy. The reason why a trusted helper, i.e. additional logic in the hypervisor, is not our favorite solution is that we'd like to keep the hypervisor as small as possible. I wouldn't exclude such an approach categorically, but we have to weigh the costs (lines of code, additional hypervisor interface) carefully against the gain (existing specifications and guest driver infrastructure). Back to VIRTIO_F_RING_SHMEM_ADDR (which you once brought up in an MCA working group discussion): What speaks against introducing an alternative encoding of addresses inside virtio data structures? The idea of this flag was to replace guest-physical addresses with offsets into a shared memory region associated with or part of a virtio device. That would preserve zero-copy capabilities (as long as you can work against the shared mem directly, e.g. doing DMA from a physical NIC or storage device into it) and keep the hypervisor out of the loop. Is it too invasive to existing infrastructure or does it have some other pitfalls? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] slirp: goto bad in udp_input if sosendto fails
On 2014-06-11 10:55, Samuel Thibault wrote: Before this patch, if sosendto fails, udp_input is executed as if the packet was sent. This could cause memory leak. Cannot follow yet how this could leak (not saying I fully got what it should NOT leak - nasty code). Can you elaborate on the before/after? Jan This patch adds a goto bad to cut the execution of this function. Signed-off-by: Guillaume Subiron maet...@subiron.org --- slirp/udp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/slirp/udp.c b/slirp/udp.c index 8cc6cb6..fd2446a 100644 --- a/slirp/udp.c +++ b/slirp/udp.c @@ -218,6 +218,7 @@ udp_input(register struct mbuf *m, int iphlen) *ip=save_ip; DEBUG_MISC((dfd,udp tx errno = %d-%s\n,errno,strerror(errno))); icmp_error(m, ICMP_UNREACH,ICMP_UNREACH_NET, 0,strerror(errno)); + goto bad; } m_free(so-so_m); /* used for ICMP if error on sorecvfrom */ -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCHv2, DoS] slirp (arp): do not special-case bogus IP addresses
On 2014-05-28 01:10, Edgar E. Iglesias wrote: On Wed, May 14, 2014 at 01:22:25AM +, Edgar E. Iglesias wrote: On Wed, May 14, 2014 at 03:13:09AM +0200, Samuel Thibault wrote: Do not special-case addresses with zero host part, as we do not necessarily know how big it is, and the guest can fake them anyway. Silently avoid having 0.0.0.0 as a destination, however. Signed-off-by: Samuel Thibault samuel.thiba...@ens-lyon.org Reviewed-by: Edgar E. Iglesias edgar.igles...@xilinx.com Ping Thanks for merging! Jan --- This is particularly bad actually, one can for instance simply do this inside a Linux guest ip addr add 192.0.0.0/1 dev eth0 and crash qemu (thus a DoS) by just emitting a packet (thus from 192.0.0.0), getting: qemu-system-x86_64: /usr/src/qemu/slirp/arp_table.c:77: arp_table_search: Assertion `(ip_addr __bswap_32 (~(0xfU 28))) != 0' failed. so it should probably go to all stable maintained versions. diff --git a/slirp/arp_table.c b/slirp/arp_table.c index ecdb0ba..bcaeb44 100644 --- a/slirp/arp_table.c +++ b/slirp/arp_table.c @@ -37,12 +37,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN]) ethaddr[0], ethaddr[1], ethaddr[2], ethaddr[3], ethaddr[4], ethaddr[5])); -/* Check 0.0.0.0/8 invalid source-only addresses */ -if ((ip_addr htonl(~(0xfU 28))) == 0) { -return; -} - -if (ip_addr == 0x || ip_addr == broadcast_addr) { +if (ip_addr == 0 || ip_addr == 0x || ip_addr == broadcast_addr) { /* Do not register broadcast addresses */ return; } @@ -73,9 +68,6 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr, DEBUG_CALL(arp_table_search); DEBUG_ARG(ip = 0x%x, ip_addr); -/* Check 0.0.0.0/8 invalid source-only addresses */ -assert((ip_addr htonl(~(0xfU 28))) != 0); - /* If broadcast address */ if (ip_addr == 0x || ip_addr == broadcast_addr) { /* return Ethernet broadcast address */ diff --git a/slirp/slirp.c b/slirp/slirp.c index 3fb48a4..00f4eb5 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -778,6 +778,11 @@ int if_encap(Slirp *slirp, struct mbuf *ifm) return 1; } +if (iph-ip_dst.s_addr == 0) { +/* 0.0.0.0 can not be a destination address, something went wrong, + * avoid making it worse */ +return 1; +} if (!arp_table_search(slirp, iph-ip_dst.s_addr, ethaddr)) { uint8_t arp_req[ETH_HLEN + sizeof(struct arphdr)]; struct ethhdr *reh = (struct ethhdr *)arp_req; -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v3] block: replace fprintf(stderr, ...) with error_report()
On 2014-05-25 10:44, Le Tan wrote: Replace fprintf(stderr,...) with error_report() in files block/*, block.c, block-migration.c and blockdev.c. The trailing \ns of the @fmt argument have been removed because @fmt of error_report() should not contain newline. Also fix some coding style issues. Let's do the also part in a separate patch. It's more than one or two trivial pass-by style fixes, and some people may like the fprintf conversion while having different views on the other changes. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/4] Support more than 255 cpus
On 2014-05-15 09:16, Li, ZhenHua wrote: Kernel's kvm support is not here. x2APIC is needed, I will try to do that later. x2APIC emulation can wait if KVM support is there. But we need at least one of them before starting to think about raising the limit. Jan PS: Please don't top-post. On 05/13/2014 06:53 PM, Jan Kiszka wrote: On 2014-05-13 09:09, Li, Zhen-Hua wrote: From: Li, ZhenHua zhen-h...@hp.com These series patches are trying to make Qemu support more than 255 CPUs. The max cpu number changed to 4096. Support more than 255 cpus: ACPI and APIC defines Support more than 255 cpus: max_cpus to 4096 Support more than 255 cpus: max cpumask bit to 4096 Support more than 255 cpus: runtime chec include/hw/acpi/cpu_hotplug_defs.h | 4 ++-- include/hw/i386/apic_internal.h| 2 +- include/hw/i386/pc.h | 2 +- include/sysemu/sysemu.h | 2 +- hw/i386/acpi-build.c | 8 Don't we need x2APIC support to provide 255 CPUs? Where so you enforce this, i.e. keep the restriction to 255 CPUs when we are not in KVM mode with in-kernel APIC (the emulate APIC lacks x2APIC mode, unfortunately)? But, wait, KVM only supports up to 255 VCPUs. So what are you targeting at? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 0/4] Support more than 255 cpus
On 2014-05-13 09:09, Li, Zhen-Hua wrote: From: Li, ZhenHua zhen-h...@hp.com These series patches are trying to make Qemu support more than 255 CPUs. The max cpu number changed to 4096. Support more than 255 cpus: ACPI and APIC defines Support more than 255 cpus: max_cpus to 4096 Support more than 255 cpus: max cpumask bit to 4096 Support more than 255 cpus: runtime chec include/hw/acpi/cpu_hotplug_defs.h | 4 ++-- include/hw/i386/apic_internal.h| 2 +- include/hw/i386/pc.h | 2 +- include/sysemu/sysemu.h | 2 +- hw/i386/acpi-build.c | 8 Don't we need x2APIC support to provide 255 CPUs? Where so you enforce this, i.e. keep the restriction to 255 CPUs when we are not in KVM mode with in-kernel APIC (the emulate APIC lacks x2APIC mode, unfortunately)? But, wait, KVM only supports up to 255 VCPUs. So what are you targeting at? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PULL 7/8] gtk: Fix -serial vc
On 2014-05-05 16:46, Gerd Hoffmann wrote: On Mo, 2014-05-05 at 13:55 +0200, Gerd Hoffmann wrote: Hi, Looks better thanks to gtk: zap scrolled_window (monitor is properly formatted again). But the whole queue spits this out on the terminal: (unknown:13169): Gtk-CRITICAL **: IA__gtk_window_resize: assertion `width 0' failed (unknown:13169): Gdk-CRITICAL **: IA__gdk_window_set_cursor: assertion `GDK_IS_WINDOW (window)' failed Yes, there is some work-in-progress stuff in that branch causing this. Feel free to check out the branch again. Warnings are fixed, resizing is fixed, and you can move tabs to windows (so you can see vga + monitor at the same time). Still issues remaining. Scenario: Fired up a Linux guest up to graphical mode, issued hard reset from monitor console, switched to guest display - window is larger than boot loader screen. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
On 2014-05-06 12:19, Kevin Wolf wrote: The immediately visible effect of this patch is that it fixes committing a temporary snapshot to its backing file. Previously, it would fail with a permission denied error because bdrv_inherited_flags() forced the backing file to be read-only, ignoring the r/w reopen of bdrv_commit(). The bigger problem this releaved is that the original open flags must actually only be applied to the temporary snapshot, and the original image file must be treated as a backing file of the temporary snapshot and get the right flags for that. Reported-by: Jan Kiszka jan.kis...@web.de Signed-off-by: Kevin Wolf kw...@redhat.com --- block.c| 34 +++--- include/block/block.h | 2 +- tests/qemu-iotests/051 | 4 tests/qemu-iotests/051.out | 10 ++ 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/block.c b/block.c index b749d31..c90c71a 100644 --- a/block.c +++ b/block.c @@ -775,6 +775,16 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs) } /* + * Returns the flags that a temporary snapshot should get, based on the + * originally requested flags (the originally requested image will have flags + * like a backing file) + */ +static int bdrv_temp_snapshot_flags(int flags) +{ +return (flags ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY; +} + +/* * Returns the flags that bs-file should get, based on the given flags for * the parent BDS */ @@ -787,11 +797,6 @@ static int bdrv_inherited_flags(int flags) * so we can enable both unconditionally on lower layers. */ flags |= BDRV_O_CACHE_WB | BDRV_O_UNMAP; -/* The backing file of a temporary snapshot is read-only */ -if (flags BDRV_O_SNAPSHOT) { -flags = ~BDRV_O_RDWR; -} - /* Clear flags that only apply to the top layer */ flags = ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ); @@ -817,11 +822,6 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags) { int open_flags = flags | BDRV_O_CACHE_WB; -/* The backing file of a temporary snapshot is read-only */ -if (flags BDRV_O_SNAPSHOT) { -open_flags = ~BDRV_O_RDWR; -} - /* * Clear flags that are internal to the block layer before opening the * image. @@ -1206,7 +1206,7 @@ done: return ret; } -void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp) +void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp) { /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */ char *tmp_filename = g_malloc0(PATH_MAX + 1); @@ -1262,8 +1262,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp) bs_snapshot = bdrv_new(, error_abort); ret = bdrv_open(bs_snapshot, NULL, NULL, snapshot_options, -(bs-open_flags ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY, -bdrv_qcow2, local_err); +flags, bdrv_qcow2, local_err); if (ret 0) { error_propagate(errp, local_err); goto out; @@ -1298,6 +1297,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, BlockDriverState *file = NULL, *bs; const char *drvname; Error *local_err = NULL; +int snapshot_flags = 0; assert(pbs); @@ -1358,6 +1358,10 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, if (flags BDRV_O_RDWR) { flags |= BDRV_O_ALLOW_RDWR; } +if (flags BDRV_O_SNAPSHOT) { +snapshot_flags = bdrv_temp_snapshot_flags(flags); +flags = bdrv_backing_flags(flags); +} assert(file == NULL); ret = bdrv_open_image(file, filename, options, file, @@ -1417,8 +1421,8 @@ int bdrv_open(BlockDriverState **pbs, const char *filename, /* For snapshot=on, create a temporary qcow2 overlay. bs points to the * temporary snapshot afterwards. */ -if (flags BDRV_O_SNAPSHOT) { -bdrv_append_temp_snapshot(bs, local_err); +if (snapshot_flags) { +bdrv_append_temp_snapshot(bs, snapshot_flags, local_err); if (local_err) { error_propagate(errp, local_err); goto close_and_fail; diff --git a/include/block/block.h b/include/block/block.h index 467fb2b..2fda81c 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -191,7 +191,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename, QDict *options, const char *bdref_key, int flags, bool allow_none, Error **errp); int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp); -void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp); +void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp); int bdrv_open(BlockDriverState **pbs, const char *filename, const char
Re: [Qemu-devel] [PULL 7/8] gtk: Fix -serial vc
On 2014-04-29 11:46, Gerd Hoffmann wrote: From: Cole Robinson crobi...@redhat.com Try kicking off a rhel5 text install over serial, the text menu navigation is all messed up, and some of the kernel boot messages are randomly corrupted. Drop use of a pty and just use vte infrastructure for reading and writing. This fixes the above corruption, and is simpler to boot. (I don't know what was wrong with the original code though. FWIW this is what virt-manager has done for years). Signed-off-by: Cole Robinson crobi...@redhat.com Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/gtk.c | 41 + 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index c85aea3..1465a38 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -115,7 +115,6 @@ typedef struct VirtualConsole GtkWidget *scrolled_window; CharDriverState *chr; #endif -int fd; } VirtualConsole; typedef struct GtkDisplayState @@ -1162,9 +1161,12 @@ static gboolean gd_focus_out_event(GtkWidget *widget, static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len) { +#if defined(CONFIG_VTE) VirtualConsole *vc = chr-opaque; -return vc ? write(vc-fd, buf, len) : len; +vte_terminal_feed(VTE_TERMINAL(vc-terminal), (const char *)buf, len); +#endif +return len; } static int nb_vcs; @@ -1190,19 +1192,12 @@ void early_gtk_display_init(void) } #if defined(CONFIG_VTE) -static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque) +static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size, + gpointer user_data) { -VirtualConsole *vc = opaque; -uint8_t buffer[1024]; -ssize_t len; - -len = read(vc-fd, buffer, sizeof(buffer)); -if (len = 0) { -return FALSE; -} - -qemu_chr_be_write(vc-chr, buffer, len); +VirtualConsole *vc = user_data; +qemu_chr_be_write(vc-chr, (uint8_t *)text, (unsigned int)size); return TRUE; } #endif @@ -1214,13 +1209,8 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL const char *label; char buffer[32]; char path[32]; -#if VTE_CHECK_VERSION(0, 26, 0) -VtePty *pty; -#endif -GIOChannel *chan; GtkWidget *scrolled_window; GtkAdjustment *vadjustment; -int master_fd, slave_fd; snprintf(buffer, sizeof(buffer), vc%d, index); snprintf(path, sizeof(path), QEMU/View/VC%d, index); @@ -1239,16 +1229,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS); vc-terminal = vte_terminal_new(); - -master_fd = qemu_openpty_raw(slave_fd, NULL); -g_assert(master_fd != -1); - -#if VTE_CHECK_VERSION(0, 26, 0) -pty = vte_pty_new_foreign(master_fd, NULL); -vte_terminal_set_pty_object(VTE_TERMINAL(vc-terminal), pty); -#else -vte_terminal_set_pty(VTE_TERMINAL(vc-terminal), master_fd); -#endif +g_signal_connect(vc-terminal, commit, G_CALLBACK(gd_vc_in), vc); vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc-terminal), -1); @@ -1263,7 +1244,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL vte_terminal_set_size(VTE_TERMINAL(vc-terminal), 80, 25); -vc-fd = slave_fd; vc-chr-opaque = vc; vc-scrolled_window = scrolled_window; @@ -1281,9 +1261,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL vc-chr-init(vc-chr); } -chan = g_io_channel_unix_new(vc-fd); -g_io_add_watch(chan, G_IO_IN, gd_vc_in, vc); - #endif /* CONFIG_VTE */ return group; } This commit somehow messes up the monitor vc: Fire up qemu-system-x86_64 and switch to console 2 (monitor). You'll find it formatted as if the console was only ~10 chars wide during printout of the monitor greetings. When typing, everything is fine again. Maybe an ordering issue that was only revealed by this commit, dunno yet. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PULL 16/31] block: Fix open_flags in bdrv_reopen()
On 2014-04-30 20:23, Kevin Wolf wrote: Use the same function as bdrv_open() for determining what the right flags for bs-file are. Without doing this, a reopen means that bs-file loses BDRV_O_CACHE_WB or BDRV_O_UNMAP if bs doesn't have it as well. Signed-off-by: Kevin Wolf kw...@redhat.com Reviewed-by: Max Reitz mre...@redhat.com --- block.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index 9f6f07e..b749d31 100644 --- a/block.c +++ b/block.c @@ -1525,8 +1525,11 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue, QSIMPLEQ_INIT(bs_queue); } +/* bdrv_open() masks this flag out */ +flags = ~BDRV_O_PROTOCOL; + if (bs-file) { -bdrv_reopen_queue(bs_queue, bs-file, flags); +bdrv_reopen_queue(bs_queue, bs-file, bdrv_inherited_flags(flags)); } bs_entry = g_new0(BlockReopenQueueEntry, 1); This breaks the commit monitor command for disks in snapshot mode. Returned error is no permission (I assume -EPERM). Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PULL 7/8] gtk: Fix -serial vc
On 2014-05-04 19:07, Cole Robinson wrote: On 05/04/2014 04:35 AM, Jan Kiszka wrote: On 2014-04-29 11:46, Gerd Hoffmann wrote: From: Cole Robinson crobi...@redhat.com Try kicking off a rhel5 text install over serial, the text menu navigation is all messed up, and some of the kernel boot messages are randomly corrupted. Drop use of a pty and just use vte infrastructure for reading and writing. This fixes the above corruption, and is simpler to boot. (I don't know what was wrong with the original code though. FWIW this is what virt-manager has done for years). Signed-off-by: Cole Robinson crobi...@redhat.com Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/gtk.c | 41 + 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index c85aea3..1465a38 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -115,7 +115,6 @@ typedef struct VirtualConsole GtkWidget *scrolled_window; CharDriverState *chr; #endif -int fd; } VirtualConsole; typedef struct GtkDisplayState @@ -1162,9 +1161,12 @@ static gboolean gd_focus_out_event(GtkWidget *widget, static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len) { +#if defined(CONFIG_VTE) VirtualConsole *vc = chr-opaque; -return vc ? write(vc-fd, buf, len) : len; +vte_terminal_feed(VTE_TERMINAL(vc-terminal), (const char *)buf, len); +#endif +return len; } static int nb_vcs; @@ -1190,19 +1192,12 @@ void early_gtk_display_init(void) } #if defined(CONFIG_VTE) -static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque) +static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size, + gpointer user_data) { -VirtualConsole *vc = opaque; -uint8_t buffer[1024]; -ssize_t len; - -len = read(vc-fd, buffer, sizeof(buffer)); -if (len = 0) { -return FALSE; -} - -qemu_chr_be_write(vc-chr, buffer, len); +VirtualConsole *vc = user_data; +qemu_chr_be_write(vc-chr, (uint8_t *)text, (unsigned int)size); return TRUE; } #endif @@ -1214,13 +1209,8 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL const char *label; char buffer[32]; char path[32]; -#if VTE_CHECK_VERSION(0, 26, 0) -VtePty *pty; -#endif -GIOChannel *chan; GtkWidget *scrolled_window; GtkAdjustment *vadjustment; -int master_fd, slave_fd; snprintf(buffer, sizeof(buffer), vc%d, index); snprintf(path, sizeof(path), QEMU/View/VC%d, index); @@ -1239,16 +1229,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS); vc-terminal = vte_terminal_new(); - -master_fd = qemu_openpty_raw(slave_fd, NULL); -g_assert(master_fd != -1); - -#if VTE_CHECK_VERSION(0, 26, 0) -pty = vte_pty_new_foreign(master_fd, NULL); -vte_terminal_set_pty_object(VTE_TERMINAL(vc-terminal), pty); -#else -vte_terminal_set_pty(VTE_TERMINAL(vc-terminal), master_fd); -#endif +g_signal_connect(vc-terminal, commit, G_CALLBACK(gd_vc_in), vc); vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc-terminal), -1); @@ -1263,7 +1244,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL vte_terminal_set_size(VTE_TERMINAL(vc-terminal), 80, 25); -vc-fd = slave_fd; vc-chr-opaque = vc; vc-scrolled_window = scrolled_window; @@ -1281,9 +1261,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL vc-chr-init(vc-chr); } -chan = g_io_channel_unix_new(vc-fd); -g_io_add_watch(chan, G_IO_IN, gd_vc_in, vc); - #endif /* CONFIG_VTE */ return group; } This commit somehow messes up the monitor vc: Fire up qemu-system-x86_64 and switch to console 2 (monitor). You'll find it formatted as if the console was only ~10 chars wide during printout of the monitor greetings. When typing, everything is fine again. Maybe an ordering issue that was only revealed by this commit, dunno yet. Check out gerd's ui-gtk-next branch, there's a few extra patches related to vte sizing that might fix it. Looks better thanks to gtk: zap scrolled_window (monitor is properly formatted again). But the whole queue spits this out on the terminal: (unknown:13169): Gtk-CRITICAL **: IA__gtk_window_resize: assertion `width 0' failed (unknown:13169): Gdk-CRITICAL **: IA__gdk_window_set_cursor: assertion `GDK_IS_WINDOW (window)' failed The last two patches seem to be responsible for this. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PULL 7/8] gtk: Fix -serial vc
On 2014-05-04 20:25, Jan Kiszka wrote: On 2014-05-04 19:07, Cole Robinson wrote: On 05/04/2014 04:35 AM, Jan Kiszka wrote: On 2014-04-29 11:46, Gerd Hoffmann wrote: From: Cole Robinson crobi...@redhat.com Try kicking off a rhel5 text install over serial, the text menu navigation is all messed up, and some of the kernel boot messages are randomly corrupted. Drop use of a pty and just use vte infrastructure for reading and writing. This fixes the above corruption, and is simpler to boot. (I don't know what was wrong with the original code though. FWIW this is what virt-manager has done for years). Signed-off-by: Cole Robinson crobi...@redhat.com Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/gtk.c | 41 + 1 file changed, 9 insertions(+), 32 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index c85aea3..1465a38 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -115,7 +115,6 @@ typedef struct VirtualConsole GtkWidget *scrolled_window; CharDriverState *chr; #endif -int fd; } VirtualConsole; typedef struct GtkDisplayState @@ -1162,9 +1161,12 @@ static gboolean gd_focus_out_event(GtkWidget *widget, static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len) { +#if defined(CONFIG_VTE) VirtualConsole *vc = chr-opaque; -return vc ? write(vc-fd, buf, len) : len; +vte_terminal_feed(VTE_TERMINAL(vc-terminal), (const char *)buf, len); +#endif +return len; } static int nb_vcs; @@ -1190,19 +1192,12 @@ void early_gtk_display_init(void) } #if defined(CONFIG_VTE) -static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque) +static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size, + gpointer user_data) { -VirtualConsole *vc = opaque; -uint8_t buffer[1024]; -ssize_t len; - -len = read(vc-fd, buffer, sizeof(buffer)); -if (len = 0) { -return FALSE; -} - -qemu_chr_be_write(vc-chr, buffer, len); +VirtualConsole *vc = user_data; +qemu_chr_be_write(vc-chr, (uint8_t *)text, (unsigned int)size); return TRUE; } #endif @@ -1214,13 +1209,8 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL const char *label; char buffer[32]; char path[32]; -#if VTE_CHECK_VERSION(0, 26, 0) -VtePty *pty; -#endif -GIOChannel *chan; GtkWidget *scrolled_window; GtkAdjustment *vadjustment; -int master_fd, slave_fd; snprintf(buffer, sizeof(buffer), vc%d, index); snprintf(path, sizeof(path), QEMU/View/VC%d, index); @@ -1239,16 +1229,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS); vc-terminal = vte_terminal_new(); - -master_fd = qemu_openpty_raw(slave_fd, NULL); -g_assert(master_fd != -1); - -#if VTE_CHECK_VERSION(0, 26, 0) -pty = vte_pty_new_foreign(master_fd, NULL); -vte_terminal_set_pty_object(VTE_TERMINAL(vc-terminal), pty); -#else -vte_terminal_set_pty(VTE_TERMINAL(vc-terminal), master_fd); -#endif +g_signal_connect(vc-terminal, commit, G_CALLBACK(gd_vc_in), vc); vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc-terminal), -1); @@ -1263,7 +1244,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL vte_terminal_set_size(VTE_TERMINAL(vc-terminal), 80, 25); -vc-fd = slave_fd; vc-chr-opaque = vc; vc-scrolled_window = scrolled_window; @@ -1281,9 +1261,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, VirtualConsole *vc, int index, GSL vc-chr-init(vc-chr); } -chan = g_io_channel_unix_new(vc-fd); -g_io_add_watch(chan, G_IO_IN, gd_vc_in, vc); - #endif /* CONFIG_VTE */ return group; } This commit somehow messes up the monitor vc: Fire up qemu-system-x86_64 and switch to console 2 (monitor). You'll find it formatted as if the console was only ~10 chars wide during printout of the monitor greetings. When typing, everything is fine again. Maybe an ordering issue that was only revealed by this commit, dunno yet. Check out gerd's ui-gtk-next branch, there's a few extra patches related to vte sizing that might fix it. Looks better thanks to gtk: zap scrolled_window (monitor is properly formatted again). ...err - no. The price of this patch is that the window is no longer properly resized on all guest mode changes. There is still more broken. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [GSoC] Wanted: small warmup tasks
On 2014-04-24 08:19, Jan Kiszka wrote: On 2014-04-23 11:25, Stefan Hajnoczi wrote: Dear QEMU, Libvirt, and KVM communities, We are participating in Google Summer of Code 2014 (http://google-melange.com/) and Outreach Program for Women (http://opw.gnome.org/). Both programs fund candidates to work on our open source projects for 12 weeks this summer. To follow up on this: I'm currently looking for optional tiny warmup tasks for our QEMU students during the bonding period (till May 18). If you have any trivial issues or extensions in mind that someone could address within a few days or even hours, that would be perfect. It could even be something like reformat the printing of these messages or so. We used this mechanism last year with the KVM student quite successfully. The idea is to give the student very early a chance to get in contact with the community and with the patch submission review procedure. So the focus is more on dealing with patches than on solving a technical problem in QEMU. If all works fine, this should encourage her/him to work with the community right from the beginning, ask question, post things early etc. Thanks for all these suggestion so far! I personally wasn't able to look into them in details yet. A wish to the mentors and students: if someone picks up a task, please drop a note here so that we can avoid duplicate work! Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [GSoC] Wanted: small warmup tasks
On 2014-04-23 11:25, Stefan Hajnoczi wrote: Dear QEMU, Libvirt, and KVM communities, We are participating in Google Summer of Code 2014 (http://google-melange.com/) and Outreach Program for Women (http://opw.gnome.org/). Both programs fund candidates to work on our open source projects for 12 weeks this summer. To follow up on this: I'm currently looking for optional tiny warmup tasks for our QEMU students during the bonding period (till May 18). If you have any trivial issues or extensions in mind that someone could address within a few days or even hours, that would be perfect. It could even be something like reformat the printing of these messages or so. We used this mechanism last year with the KVM student quite successfully. The idea is to give the student very early a chance to get in contact with the community and with the patch submission review procedure. So the focus is more on dealing with patches than on solving a technical problem in QEMU. If all works fine, this should encourage her/him to work with the community right from the beginning, ask question, post things early etc. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] pc: Fix disabling of vapic for compat PC models
From: Jan Kiszka jan.kis...@siemens.com We used to be able to address both the QEMU and the KVM APIC via apic. This doesn't work anymore. So we need to use their parent class to turn off the vapic on machines that should not expose them. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- I'm pretty sure it was like this, that -global apic.vapic=off also worked for -enable-kvm, ie. the KVM APIC. If someone could explain what changes, probably around QOM - maybe we can restore the original behavior and skip this patch. hw/i386/pc_piix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 7930a26..12ee716 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -566,7 +566,7 @@ static QEMUMachine pc_machine_v1_1 = { .property = class,\ .value= stringify(PCI_CLASS_MEMORY_RAM),\ },{\ -.driver = apic,\ +.driver = apic-common,\ .property = vapic,\ .value= off,\ },{\ -- 1.8.1.1.298.ge7eed54
Re: [Qemu-devel] About SIG_IPI handler
On 2014-04-17 07:46, Shiru Ren wrote: Hi, all I’m trying to figure out how do_savevm works in QEMU. But there is one thing has bothered me quite a lot. I found that vm_stop invoke qemu_cpu_kick_thread to send SIG_IPI to a vcpu thread, and I have understand that in TCG mode, the cpu_signal() function will be invoked as the SIG_IPI handler. But I don’t know what happens in KVM mode. Actually I can’t find the signal handler function. I only find a function named dummy_signal, and it doesn't do anything. This signal is handled synchronously in KVM mode, see qemu_kvm_eat_signals in cpus.c. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2 for-2.0] target-i386: x86_cpu_get_phys_page_debug(): support 1GB page translation
On 2014-03-19 22:03, Luiz Capitulino wrote: Linux guests, when using more than 4GB of RAM, may end up using 1GB pages to store (kernel) data. When this happens, we're unable to debug a running Linux kernel with GDB: (gdb) p node_data[0]-node_id Cannot access memory at address 0x88013fffd3a0 (gdb) GDB returns this error because x86_cpu_get_phys_page_debug() doesn't support translating 1GB pages in IA-32e paging mode and returns an error to GDB. This commit adds support for 1GB page translation for IA32e paging. Signed-off-by: Luiz capitulino lcapitul...@redhat.com --- - I'm proposing this patch for 2.0 because GDB debugging of large Linux guests is kind of broken - Changelog v2: - Move PS bit handling to if (env-hflags HF_LMA_MASK) block - Update changelog target-i386/helper.c | 9 + 1 file changed, 9 insertions(+) diff --git a/target-i386/helper.c b/target-i386/helper.c index 4f447b8..7cee501 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -941,6 +941,14 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr) pdpe = ldq_phys(cs-as, pdpe_addr); if (!(pdpe PG_PRESENT_MASK)) return -1; + +if (pdpe PG_PSE_MASK) { +page_size = 1024 * 1024 * 1024; +pte = pdpe ~( (page_size - 1) ~0xfff); +pte = ~(PG_NX_MASK | PG_HI_USER_MASK); +goto out; +} + } else #endif { @@ -993,6 +1001,7 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr) pte = pte env-a20_mask; } +out: page_offset = (addr TARGET_PAGE_MASK) (page_size - 1); paddr = (pte TARGET_PAGE_MASK) + page_offset; return paddr; Reviewed-by: Jan Kiszka jan.kis...@siemens.com -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH for-2.0?] target-i386: fix gdb debugging with large memory guests
On 2014-03-18 02:54, Luiz Capitulino wrote: If you start a Linux guest with more than 4GB of memory and try to look at a memory address, you will get an error from gdb: (gdb) p node_data[0]-node_id Cannot access memory at address 0x88013fffd3a0 (gdb) I suppose this is x86-64, not 32-bit with PTE, right? I debugged this down to x86_cpu_get_phys_page_debug(), it doesn't handle the case where the PDPTE has the PS bit set (although I didn't check where Linux sets that bit). This commit adds the PS bit handling, which fixes the problem for me. Signed-off-by: Luiz capitulino lcapitul...@redhat.com --- Two observations: 1. This bug has always existed, so it's not a regression, so I'm not sure it's worth it to fix for 2.0 2. I'm not familiar with every detail of x86_cpu_get_phys_page_debug(), so I'm not completely sure this is the right thing to do target-i386/helper.c | 8 1 file changed, 8 insertions(+) diff --git a/target-i386/helper.c b/target-i386/helper.c index 4f447b8..9b7803f 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -951,6 +951,13 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr) return -1; } +if (pdpe PG_PSE_MASK) { +page_size = 1024 * 1024 * 1024; +pte = pdpe ~( (page_size - 1) ~0xfff); +pte = ~(PG_NX_MASK | PG_HI_USER_MASK); +goto out; +} Does this also apply if we are not in long mode? I'll check this more carefully later. Jan + pde_addr = ((pdpe ~0xfff ~(PG_NX_MASK | PG_HI_USER_MASK)) + (((addr 21) 0x1ff) 3)) env-a20_mask; pde = ldq_phys(cs-as, pde_addr); @@ -993,6 +1000,7 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr) pte = pte env-a20_mask; } +out: page_offset = (addr TARGET_PAGE_MASK) (page_size - 1); paddr = (pte TARGET_PAGE_MASK) + page_offset; return paddr; -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH for-2.0?] target-i386: fix gdb debugging with large memory guests
On 2014-03-18 08:36, Paolo Bonzini wrote: Il 18/03/2014 08:19, Jan Kiszka ha scritto: On 2014-03-18 02:54, Luiz Capitulino wrote: If you start a Linux guest with more than 4GB of memory and try to look at a memory address, you will get an error from gdb: (gdb) p node_data[0]-node_id Cannot access memory at address 0x88013fffd3a0 (gdb) I suppose this is x86-64, not 32-bit with PTE, right? I debugged this down to x86_cpu_get_phys_page_debug(), it doesn't handle the case where the PDPTE has the PS bit set (although I didn't check where Linux sets that bit). This commit adds the PS bit handling, which fixes the problem for me. Signed-off-by: Luiz capitulino lcapitul...@redhat.com --- Two observations: 1. This bug has always existed, so it's not a regression, so I'm not sure it's worth it to fix for 2.0 Sure, why not? 2. I'm not familiar with every detail of x86_cpu_get_phys_page_debug(), so I'm not completely sure this is the right thing to do target-i386/helper.c | 8 1 file changed, 8 insertions(+) diff --git a/target-i386/helper.c b/target-i386/helper.c index 4f447b8..9b7803f 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -951,6 +951,13 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr) return -1; } +if (pdpe PG_PSE_MASK) { +page_size = 1024 * 1024 * 1024; +pte = pdpe ~( (page_size - 1) ~0xfff); +pte = ~(PG_NX_MASK | PG_HI_USER_MASK); +goto out; +} Does this also apply if we are not in long mode? No, it doesn't. The only valid bits in a PAE PDPTE are P, PWT and PCD. Bit 7 (PS) is reserved. Right, this belongs in the if (env-hflags HF_LMA_MASK) block. And the subject or description should mention that x86_cpu_get_phys_page_debug was lacking support for 1G hugepages. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH for-2.0?] target-i386: fix gdb debugging with large memory guests
On 2014-03-18 14:00, Luiz Capitulino wrote: On Tue, 18 Mar 2014 11:30:42 +0100 Jan Kiszka jan.kis...@siemens.com wrote: On 2014-03-18 08:36, Paolo Bonzini wrote: Il 18/03/2014 08:19, Jan Kiszka ha scritto: On 2014-03-18 02:54, Luiz Capitulino wrote: If you start a Linux guest with more than 4GB of memory and try to look at a memory address, you will get an error from gdb: (gdb) p node_data[0]-node_id Cannot access memory at address 0x88013fffd3a0 (gdb) I suppose this is x86-64, not 32-bit with PTE, right? I debugged this down to x86_cpu_get_phys_page_debug(), it doesn't handle the case where the PDPTE has the PS bit set (although I didn't check where Linux sets that bit). This commit adds the PS bit handling, which fixes the problem for me. Signed-off-by: Luiz capitulino lcapitul...@redhat.com --- Two observations: 1. This bug has always existed, so it's not a regression, so I'm not sure it's worth it to fix for 2.0 Sure, why not? 2. I'm not familiar with every detail of x86_cpu_get_phys_page_debug(), so I'm not completely sure this is the right thing to do target-i386/helper.c | 8 1 file changed, 8 insertions(+) diff --git a/target-i386/helper.c b/target-i386/helper.c index 4f447b8..9b7803f 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -951,6 +951,13 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr) return -1; } +if (pdpe PG_PSE_MASK) { +page_size = 1024 * 1024 * 1024; +pte = pdpe ~( (page_size - 1) ~0xfff); +pte = ~(PG_NX_MASK | PG_HI_USER_MASK); +goto out; +} Does this also apply if we are not in long mode? No, it doesn't. The only valid bits in a PAE PDPTE are P, PWT and PCD. Bit 7 (PS) is reserved. Right, this belongs in the if (env-hflags HF_LMA_MASK) block. And the subject or description should mention that x86_cpu_get_phys_page_debug was lacking support for 1G hugepages. To be honest, although the PS bit is set and that indicates a 1GB page, I didn't know Linux does that. I thought Linux would use 4KB pages for everything unless it's explicitly asked to use bigger pages. Also, note that I was using gdb to debug really early kernel boot code (start_kernel()). I could imagine that Linux initially creates a giant identity mapping page table for the startup process and only later on switches to fine-grained tables of 4K and 2M pages. Giant pages still require hughtlbfs, IIRC. I'd feel more confident to have such a changelog after I find out where exactly Linux sets that bit, but I won't have time in the next days. On the other hand, the patch does fix the problem to me. Don't worry about Linux (the code should work with any OS anyway), just believe your reviewers. ;) Alternatively, check Intel IA32 SDM on page table structures. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH for-2.0?] target-i386: fix gdb debugging with large memory guests
On 2014-03-18 17:37, Paolo Bonzini wrote: Il 18/03/2014 17:23, Luiz Capitulino ha scritto: On Tue, 18 Mar 2014 15:36:45 +0100 Jan Kiszka jan.kis...@siemens.com wrote: Right, this belongs in the if (env-hflags HF_LMA_MASK) block. And the subject or description should mention that x86_cpu_get_phys_page_debug was lacking support for 1G hugepages. To be honest, although the PS bit is set and that indicates a 1GB page, I didn't know Linux does that. I thought Linux would use 4KB pages for everything unless it's explicitly asked to use bigger pages. Also, note that I was using gdb to debug really early kernel boot code (start_kernel()). I could imagine that Linux initially creates a giant identity mapping page table for the startup process and only later on switches to fine-grained tables of 4K and 2M pages. Giant pages still require hughtlbfs, IIRC. I'd feel more confident to have such a changelog after I find out where exactly Linux sets that bit, but I won't have time in the next days. On the other hand, the patch does fix the problem to me. Don't worry about Linux (the code should work with any OS anyway), just believe your reviewers. ;) Alternatively, check Intel IA32 SDM on page table structures. OK, so you want me to change the subject? Anything else for v2? You only need to move the new code into the if (env-hflags HF_LMA_MASK), I think. The subject is ok. Yes. Subject is fine, a reference to GB pages in the description would be nice-to-have. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 1/6] slirp: Remove default_mon usage
On 2014-03-12 00:15, Cole Robinson wrote: These errors don't seem user initiated, so forcibly printing to the monitor doesn't seem right. Just print to stderr. Drop lprint since it's now unused. Cc: Jan Kiszka jan.kis...@siemens.com Signed-off-by: Cole Robinson crobi...@redhat.com --- checkpatch flags some pre-existing tab issues, but I didn't retab. Should I? slirp/misc.c | 13 ++--- slirp/slirp.c | 8 slirp/slirp.h | 2 -- 3 files changed, 6 insertions(+), 17 deletions(-) diff --git a/slirp/misc.c b/slirp/misc.c index 6c1636f..662fb1d 100644 --- a/slirp/misc.c +++ b/slirp/misc.c @@ -136,7 +136,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty) if ((s = qemu_socket(AF_INET, SOCK_STREAM, 0)) 0 || bind(s, (struct sockaddr *)addr, addrlen) 0 || listen(s, 1) 0) { - lprint(Error: inet socket: %s\n, strerror(errno)); + fprintf(stderr, Error: inet socket: %s\n, strerror(errno)); closesocket(s); return 0; @@ -146,7 +146,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty) pid = fork(); switch(pid) { case -1: - lprint(Error: fork failed: %s\n, strerror(errno)); + fprintf(stderr, Error: fork failed: %s\n, strerror(errno)); close(s); return 0; @@ -242,15 +242,6 @@ strdup(str) } #endif -void lprint(const char *format, ...) -{ -va_list args; - -va_start(args, format); -monitor_vprintf(default_mon, format, args); -va_end(args); -} - void slirp_connection_info(Slirp *slirp, Monitor *mon) { const char * const tcpstates[] = { diff --git a/slirp/slirp.c b/slirp/slirp.c index bad8dad..3fb48a4 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -139,7 +139,7 @@ int get_dns_addr(struct in_addr *pdns_addr) return -1; #ifdef DEBUG -lprint(IP address of your DNS(s): ); +fprintf(stderr, IP address of your DNS(s): ); #endif while (fgets(buff, 512, f) != NULL) { if (sscanf(buff, nameserver%*[ \t]%256s, buff2) == 1) { @@ -153,17 +153,17 @@ int get_dns_addr(struct in_addr *pdns_addr) } #ifdef DEBUG else -lprint(, ); +fprintf(stderr, , ); #endif if (++found 3) { #ifdef DEBUG -lprint((more)); +fprintf(stderr, (more)); #endif break; } #ifdef DEBUG else -lprint(%s, inet_ntoa(tmp_addr)); +fprintf(stderr, %s, inet_ntoa(tmp_addr)); #endif } } diff --git a/slirp/slirp.h b/slirp/slirp.h index e4a1bd4..6589d7e 100644 --- a/slirp/slirp.h +++ b/slirp/slirp.h @@ -287,8 +287,6 @@ void if_start(struct ttys *); long gethostid(void); #endif -void lprint(const char *, ...) GCC_FMT_ATTR(1, 2); - #ifndef _WIN32 #include netdb.h #endif Reviewed-by: Jan Kiszka jan.kis...@siemens.com I suppose this goes through Luiz' queue? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] slirp smb with modern win guests when samba is also running on host
On 2013-11-28 20:32, Michael Tokarev wrote: After numerous reports that -smb (or -netdev user,smb=foo) not working with modern windows (win7 and vista are reported as non-working), I started digging myself. And found that indeed it doesn't work, and why. The thing is that modern win tries to connect to port 445 (microsoft-ds) first, and if that fails, it falls back to old port 139 (netbios-ssn). slirp code in qemu only redirects port 139, it does not touch port 445. So the prob is that if samba is also running on the host, guest will try to communicate using port 445, and that will succed, but ofcourse guest will not talk with our samba but with samba running on the host. If samba is not running on the host, guest will fall back to port 139, and will reach the redirecting rule and qemu will spawn smbd correctly. The solution is to redirect both ports (139 and 445), and the fix is a one-liner, adding second call to slirp_add_exec() at the end of net/slirp.c:slirp_smb() function (provided below). But it looks like that is not a proper fix really, since in theory we should redirect both ports to the SAME, single samba instance, but I'm not sure this is possible with slirp. Well, even if two smbd processes will be run on the same config dir, it should not be a problem. I don't see either that this is expressible with the current exec feature of slirp. The one-liner (not exactly 1 since it touches previous line too) is like this: Signed-off-By: Michael Tokarev m...@tls.msk.ru diff --git a/net/slirp.c b/net/slirp.c index 124e953..a22e976 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -549,7 +549,8 @@ static int slirp_smb(SlirpState* s, const char *exported_dir snprintf(smb_cmdline, sizeof(smb_cmdline), %s -s %s, CONFIG_SMBD_COMMAND, smb_conf); -if (slirp_add_exec(s-slirp, 0, smb_cmdline, vserver_addr, 139) 0) { +if (slirp_add_exec(s-slirp, 0, smb_cmdline, vserver_addr, 139) 0 || +slirp_add_exec(s-slirp, 0, smb_cmdline, vserver_addr, 445) 0) { slirp_smb_cleanup(s); error_report(conflicting/invalid smbserver address); return -1; Thanks, merged to my slirp queue. Will send this out ASAP, sorry for the excessive delay. JAn -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCHv3 00/16] slirp: Adding IPv6 support to Qemu -net user mode
On 2014-02-11 14:08, Samuel Thibault wrote: Hello, This is a respin of IPv6 in Qemu -net user mode. These patches add ICMPv6, NDP, and make UDP and TCP compatible with IPv6. We have made some refactoring to make current code compatible with IPv6. Some patches, like 2 and 13, can be reviewed using interdiff -w /dev/null patchfile to get rid of the indentation. The alternative is to split reindention from refactoring patches. Difference with version 2 is: - The default IP range has been made fec0::/64 instead of fc00::/64 This is very valuable work. Unfortunately, I'm lacking time to review or even test this ATM. If someone else could, that would be great. Thanks, Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [pull] slirp queue
The following changes since commit 239618707637ec87eba8c452d2b2f75dc5ca20c7: Merge remote-tracking branch 'remotes/kvm/uq/master' into staging (2014-03-11 19:39:17 +) are available in the git repository at: git://git.kiszka.org/qemu.git queues/slirp for you to fetch changes up to 5c1e1890bfa1f6b4bc3f51e368bfd47af1b60db0: slirp smb with modern win guests when samba is also running on host (2014-03-12 08:13:24 +0100) Michael Buesch (1): qemu/slirp: Fix SMB security configuration on newer samba versions Michael Tokarev (1): slirp smb with modern win guests when samba is also running on host net/slirp.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH][RESEND] gtk: Allow to activate grab-on-hover from the command line
As long as we have no persistent GTK configuration, this allows to enable the useful grab-on-hover feature already when starting the VM. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/ui/console.h | 2 +- qemu-options.hx | 5 + ui/gtk.c | 5 - vl.c | 22 +- 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index 08a38ea..8a86617 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -345,6 +345,6 @@ int index_from_key(const char *key); /* gtk.c */ void early_gtk_display_init(void); -void gtk_display_init(DisplayState *ds, bool full_screen); +void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover); #endif diff --git a/qemu-options.hx b/qemu-options.hx index 068da2d..ee5437b 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -810,6 +810,7 @@ ETEXI DEF(display, HAS_ARG, QEMU_OPTION_display, -display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]\n [,window_close=on|off]|curses|none|\n +gtk[,grab_on_hover=on|off]|\n vnc=display[,optargs]\n select display type\n, QEMU_ARCH_ALL) STEXI @@ -833,6 +834,10 @@ graphics card, but its output will not be displayed to the QEMU user. This option differs from the -nographic option in that it only affects what is done with video output; -nographic also changes the destination of the serial and parallel port data. +@item gtk +Display video output in a GTK window. This interface provides drop-down +menus and other UI elements to configure and control the VM during +runtime. @item vnc Start a VNC server on display arg @end table diff --git a/ui/gtk.c b/ui/gtk.c index 1851495..992f7d7 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -1415,7 +1415,7 @@ static const DisplayChangeListenerOps dcl_ops = { .dpy_cursor_define = gd_cursor_define, }; -void gtk_display_init(DisplayState *ds, bool full_screen) +void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover) { GtkDisplayState *s = g_malloc0(sizeof(*s)); char *filename; @@ -1494,6 +1494,9 @@ void gtk_display_init(DisplayState *ds, bool full_screen) if (full_screen) { gtk_menu_item_activate(GTK_MENU_ITEM(s-full_screen_item)); } +if (grab_on_hover) { +gtk_menu_item_activate(GTK_MENU_ITEM(s-grab_on_hover_item)); +} register_displaychangelistener(s-dcl); diff --git a/vl.c b/vl.c index bca5c95..b73744d 100644 --- a/vl.c +++ b/vl.c @@ -143,6 +143,7 @@ int vga_interface_type = VGA_NONE; static int full_screen = 0; static int no_frame = 0; int no_quit = 0; +static bool grab_on_hover; CharDriverState *serial_hds[MAX_SERIAL_PORTS]; CharDriverState *parallel_hds[MAX_PARALLEL_PORTS]; CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES]; @@ -2241,6 +2242,25 @@ static DisplayType select_display(const char *p) } else if (strstart(p, gtk, opts)) { #ifdef CONFIG_GTK display = DT_GTK; +while (*opts) { +const char *nextopt; + +if (strstart(opts, ,grab_on_hover=, nextopt)) { +opts = nextopt; +if (strstart(opts, on, nextopt)) { +grab_on_hover = true; +} else if (strstart(opts, off, nextopt)) { +grab_on_hover = false; +} else { +goto invalid_gtk_args; +} +} else { +invalid_gtk_args: +fprintf(stderr, Invalid GTK option string: %s\n, p); +exit(1); +} +opts = nextopt; +} #else fprintf(stderr, GTK support is disabled\n); exit(1); @@ -4351,7 +4371,7 @@ int main(int argc, char **argv, char **envp) #endif #if defined(CONFIG_GTK) case DT_GTK: -gtk_display_init(ds, full_screen); +gtk_display_init(ds, full_screen, grab_on_hover); break; #endif default: -- 1.8.1.1.298.ge7eed54
Re: [Qemu-devel] [PATCH 1/2][RESENT] Add GDB qAttached support
On 2013-07-17 10:10, Jan Kiszka wrote: With this patch QEMU handles qAttached request from gdb. When QEMU replies 1, GDB sends a detach command at the end of a debugging session otherwise GDB sends kill. The default value for qAttached is 1 on system emulation and 0 on user emulation. Based on original version by Fabien Chouteau. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- gdbstub.c | 10 ++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index 0ee82a9..bc626f5 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -42,6 +42,12 @@ #include sysemu/kvm.h #include qemu/bitops.h +#ifdef CONFIG_USER_ONLY +#define GDB_ATTACHED 0 +#else +#define GDB_ATTACHED 1 +#endif + #ifndef TARGET_CPU_MEMORY_RW_DEBUG static inline int target_memory_rw_debug(CPUArchState *env, target_ulong addr, uint8_t *buf, int len, int is_write) @@ -2504,6 +2510,10 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) break; } #endif +if (strncmp(p, Attached, 8) == 0) { +put_packet(s, GDB_ATTACHED); +break; +} /* Unrecognised 'q' command. */ goto unknown_command; Peter, could you pick up these two almost trivial long-pending patches? They still apply and are still useful. If you prefer that I repost them, just let me know. Thanks, Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v2 1/2] Add GDB qAttached support
On 2014-03-12 17:58, Andreas Färber wrote: Am 26.07.2013 20:26, schrieb Jan Kiszka: With this patch QEMU handles qAttached request from gdb. When QEMU With this patch is always weird to read in Git history, also a gdbstub: prefix would've been nice for consistency. Yes, will rephrase this. replies 1, GDB sends a detach command at the end of a debugging session otherwise GDB sends kill. The default value for qAttached is 1 on system emulation and 0 on user emulation. Based on original version by Fabien Chouteau. If this is based on code by Fabien, shouldn't it carry his Signed-off-by before yours? Need to check how similar our versions actually are, if I can reuse his signed-off or if I changed it (in that case it's not appropriate to keep the original signed-off - according to my understanding). Since GDB stub is in Odd Fixes state, maybe just step up as maintainer and send a pull like for SLIRP? :) Oh, I can surely send a pull, but I can't handle another maintainership properly. Slirp already became a stepchild... Thanks for looking into this. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PULL v3 08/14] Add a 'name' parameter to qemu_thread_create
On 2014-03-09 20:20, Michael S. Tsirkin wrote: From: Dr. David Alan Gilbert dgilb...@redhat.com If enabled, set the thread name at creation (on GNU systems with pthread_set_np) Fix up all the callers with a thread name Seems like not all older Linux systems come with support for pthread_setname_np. Just ran into a linker bug on a box with glib 2.11. Can we discover the availability, if nothing helps during configure? Thanks, Jan Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com Acked-by: Michael S. Tsirkin m...@redhat.com Reviewed-by: Laszlo Ersek ler...@redhat.com --- include/qemu/thread.h | 2 +- cpus.c | 25 - hw/block/dataplane/virtio-blk.c | 2 +- hw/usb/ccid-card-emulated.c | 8 libcacard/vscclient.c | 2 +- migration.c | 2 +- thread-pool.c | 2 +- ui/vnc-jobs.c | 3 ++- util/compatfd.c | 3 ++- util/qemu-thread-posix.c| 9 +++-- util/qemu-thread-win32.c| 2 +- 11 files changed, 41 insertions(+), 19 deletions(-) diff --git a/include/qemu/thread.h b/include/qemu/thread.h index bf1e110..f7e3b9b 100644 --- a/include/qemu/thread.h +++ b/include/qemu/thread.h @@ -52,7 +52,7 @@ void qemu_event_reset(QemuEvent *ev); void qemu_event_wait(QemuEvent *ev); void qemu_event_destroy(QemuEvent *ev); -void qemu_thread_create(QemuThread *thread, +void qemu_thread_create(QemuThread *thread, const char *name, void *(*start_routine)(void *), void *arg, int mode); void *qemu_thread_join(QemuThread *thread); diff --git a/cpus.c b/cpus.c index 945d85b..b6421fd 100644 --- a/cpus.c +++ b/cpus.c @@ -1117,8 +1117,13 @@ void resume_all_vcpus(void) } } +/* For temporary buffers for forming a name */ +#define VCPU_THREAD_NAME_SIZE 16 + static void qemu_tcg_init_vcpu(CPUState *cpu) { +char thread_name[VCPU_THREAD_NAME_SIZE]; + tcg_cpu_address_space_init(cpu, cpu-as); /* share a single thread for all cpus with TCG */ @@ -1127,8 +1132,10 @@ static void qemu_tcg_init_vcpu(CPUState *cpu) cpu-halt_cond = g_malloc0(sizeof(QemuCond)); qemu_cond_init(cpu-halt_cond); tcg_halt_cond = cpu-halt_cond; -qemu_thread_create(cpu-thread, qemu_tcg_cpu_thread_fn, cpu, - QEMU_THREAD_JOINABLE); +snprintf(thread_name, VCPU_THREAD_NAME_SIZE, CPU %d/TCG, + cpu-cpu_index); +qemu_thread_create(cpu-thread, thread_name, qemu_tcg_cpu_thread_fn, + cpu, QEMU_THREAD_JOINABLE); #ifdef _WIN32 cpu-hThread = qemu_thread_get_handle(cpu-thread); #endif @@ -1144,11 +1151,15 @@ static void qemu_tcg_init_vcpu(CPUState *cpu) static void qemu_kvm_start_vcpu(CPUState *cpu) { +char thread_name[VCPU_THREAD_NAME_SIZE]; + cpu-thread = g_malloc0(sizeof(QemuThread)); cpu-halt_cond = g_malloc0(sizeof(QemuCond)); qemu_cond_init(cpu-halt_cond); -qemu_thread_create(cpu-thread, qemu_kvm_cpu_thread_fn, cpu, - QEMU_THREAD_JOINABLE); +snprintf(thread_name, VCPU_THREAD_NAME_SIZE, CPU %d/KVM, + cpu-cpu_index); +qemu_thread_create(cpu-thread, thread_name, qemu_kvm_cpu_thread_fn, + cpu, QEMU_THREAD_JOINABLE); while (!cpu-created) { qemu_cond_wait(qemu_cpu_cond, qemu_global_mutex); } @@ -1156,10 +1167,14 @@ static void qemu_kvm_start_vcpu(CPUState *cpu) static void qemu_dummy_start_vcpu(CPUState *cpu) { +char thread_name[VCPU_THREAD_NAME_SIZE]; + cpu-thread = g_malloc0(sizeof(QemuThread)); cpu-halt_cond = g_malloc0(sizeof(QemuCond)); qemu_cond_init(cpu-halt_cond); -qemu_thread_create(cpu-thread, qemu_dummy_cpu_thread_fn, cpu, +snprintf(thread_name, VCPU_THREAD_NAME_SIZE, CPU %d/DUMMY, + cpu-cpu_index); +qemu_thread_create(cpu-thread, thread_name, qemu_dummy_cpu_thread_fn, cpu, QEMU_THREAD_JOINABLE); while (!cpu-created) { qemu_cond_wait(qemu_cpu_cond, qemu_global_mutex); diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 2237edb..d1c7ad4 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -358,7 +358,7 @@ static void start_data_plane_bh(void *opaque) qemu_bh_delete(s-start_bh); s-start_bh = NULL; -qemu_thread_create(s-thread, data_plane_thread, +qemu_thread_create(s-thread, data_plane, data_plane_thread, s, QEMU_THREAD_JOINABLE); } diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c index aa913df..7213c89 100644 --- a/hw/usb/ccid-card-emulated.c +++ b/hw/usb/ccid-card-emulated.c @@ -546,10 +546,10 @@ static int emulated_initfn(CCIDCardState
[Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version
pthread_setname_np was introduced with 2.12. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- util/qemu-thread-posix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 45113b4..960d7f5 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -420,7 +420,7 @@ void qemu_thread_create(QemuThread *thread, const char *name, if (err) error_exit(err, __func__); -#ifdef _GNU_SOURCE +#if defined(__GLIBC__) (__GLIBC__ 2 || (__GLIBC__ == 2 __GLIBC_MINOR__ = 12)) if (name_threads) { pthread_setname_np(thread-thread, name); } -- 1.8.1.1.298.ge7eed54
[Qemu-devel] [PATCH] gtk: Add mouse wheel support
Hook into scroll-event to properly forward mouse wheel movements to the guest, just like we already do in SDL. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- ui/gtk.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/ui/gtk.c b/ui/gtk.c index 992f7d7..016804d 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -683,6 +683,27 @@ static gboolean gd_button_event(GtkWidget *widget, GdkEventButton *button, return TRUE; } +static gboolean gd_scroll_event(GtkWidget *widget, GdkEventScroll *scroll, +void *opaque) +{ +GtkDisplayState *s = opaque; +InputButton btn; + +if (scroll-direction == GDK_SCROLL_UP) { +btn = INPUT_BUTTON_WHEEL_UP; +} else if (scroll-direction == GDK_SCROLL_DOWN) { +btn = INPUT_BUTTON_WHEEL_DOWN; +} else { +return TRUE; +} + +qemu_input_queue_btn(s-dcl.con, btn, true); +qemu_input_event_sync(); +qemu_input_queue_btn(s-dcl.con, btn, false); +qemu_input_event_sync(); +return TRUE; +} + static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque) { GtkDisplayState *s = opaque; @@ -1229,6 +1250,8 @@ static void gd_connect_signals(GtkDisplayState *s) G_CALLBACK(gd_button_event), s); g_signal_connect(s-drawing_area, button-release-event, G_CALLBACK(gd_button_event), s); +g_signal_connect(s-drawing_area, scroll-event, + G_CALLBACK(gd_scroll_event), s); g_signal_connect(s-drawing_area, key-press-event, G_CALLBACK(gd_key_event), s); g_signal_connect(s-drawing_area, key-release-event, -- 1.8.1.1.298.ge7eed54
Re: [Qemu-devel] [PATCH] hw/arm/musicpal: Remove nonexistent CDTP2, CDTP3 registers
On 2014-02-18 16:28, Peter Maydell wrote: The ethernet device in the musicpal only has two tx queues, but we modelled it with four CTDP registers, presumably a cut and paste from the rx queue registers. Since the tx_queue[] array is only 2 entries long this allowed a guest to overrun this buffer. Remove the nonexistent registers. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Acked-by: Jan Kiszka jan.kis...@web.de --- There's no readily available documentation for this SoC, but I'm told the BSP for it indicates that there are indeed only two tx queues. hw/arm/musicpal.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index 023e875..a8d0086 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -92,8 +92,6 @@ #define MP_ETH_CRDP30x4AC #define MP_ETH_CTDP00x4E0 #define MP_ETH_CTDP10x4E4 -#define MP_ETH_CTDP20x4E8 -#define MP_ETH_CTDP30x4EC /* MII PHY access */ #define MP_ETH_SMIR_DATA0x @@ -308,7 +306,7 @@ static uint64_t mv88w8618_eth_read(void *opaque, hwaddr offset, case MP_ETH_CRDP0 ... MP_ETH_CRDP3: return s-rx_queue[(offset - MP_ETH_CRDP0)/4]; -case MP_ETH_CTDP0 ... MP_ETH_CTDP3: +case MP_ETH_CTDP0 ... MP_ETH_CTDP1: return s-tx_queue[(offset - MP_ETH_CTDP0)/4]; default: @@ -362,7 +360,7 @@ static void mv88w8618_eth_write(void *opaque, hwaddr offset, s-cur_rx[(offset - MP_ETH_CRDP0)/4] = value; break; -case MP_ETH_CTDP0 ... MP_ETH_CTDP3: +case MP_ETH_CTDP0 ... MP_ETH_CTDP1: s-tx_queue[(offset - MP_ETH_CTDP0)/4] = value; break; } signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Looking for project ideas and mentors for Google Summer of Code 2014
On 2014-02-11 11:17, Stefan Hajnoczi wrote: On Mon, Feb 3, 2014 at 8:45 AM, Stefan Hajnoczi stefa...@gmail.com wrote: Project ideas Please post project ideas on the wiki page below. Project ideas should be suitable as a 12-week project that a student fluent in C/Python/etc can complete. No prior knowledge of QEMU/KVM/libvirt internals can be assumed. http://qemu-project.org/Google_Summer_of_Code_2014 Please post your project ideas before Friday. I need to submit our organization application (including our project ideas) on Friday. Hope it's not too late: just added the VT-d emulation proposal. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 7/8] roms: update vgabios binaries
On 2013-12-06 10:35, Gerd Hoffmann wrote: This also switches from lgplvgabios to seavgabios. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- pc-bios/vgabios-cirrus.bin | Bin 35840 - 36864 bytes pc-bios/vgabios-qxl.bin| Bin 40448 - 37376 bytes pc-bios/vgabios-stdvga.bin | Bin 40448 - 37376 bytes pc-bios/vgabios-vmware.bin | Bin 40448 - 37376 bytes pc-bios/vgabios.bin| Bin 40448 - 36864 bytes 5 files changed, 0 insertions(+), 0 deletions(-) This update breaks a 64-bit win7 guest for me. It now spins infinitely in the Starting Windows screen. I'm using stdvga. Is this a known issue? Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 7/8] roms: update vgabios binaries
On 2014-01-05 12:26, Paolo Bonzini wrote: Il 05/01/2014 10:35, Jan Kiszka ha scritto: On 2013-12-06 10:35, Gerd Hoffmann wrote: This also switches from lgplvgabios to seavgabios. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- pc-bios/vgabios-cirrus.bin | Bin 35840 - 36864 bytes pc-bios/vgabios-qxl.bin| Bin 40448 - 37376 bytes pc-bios/vgabios-stdvga.bin | Bin 40448 - 37376 bytes pc-bios/vgabios-vmware.bin | Bin 40448 - 37376 bytes pc-bios/vgabios.bin| Bin 40448 - 36864 bytes 5 files changed, 0 insertions(+), 0 deletions(-) This update breaks a 64-bit win7 guest for me. It now spins infinitely in the Starting Windows screen. I'm using stdvga. Is this a known issue? Yes, and IIRC it's fixed in seabios.git. (Hmm, cd roms; make seavgabios doesn't work with out-of-tree builds) Confirmed, win7 boots fine again with current seabios.git master. So please update qemu. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH uq/master] kvm: x86: Separately write feature control MSR on reset
If the guest is running in nested mode on system reset, clearing the feature MSR signals the kernel to leave this mode. Recent kernels processes this properly, but leave the VCPU state undefined behind. It is the job of userspace to bring it to a proper shape. Therefore, write this specific MSR first so that no state transfer gets lost. This allows to cleanly reset a guest with VMX in use. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- target-i386/kvm.c | 32 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 1188482..ec51447 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -1104,6 +1104,25 @@ static int kvm_put_tscdeadline_msr(X86CPU *cpu) return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, msr_data); } +/* + * Provide a separate write service for the feature control MSR in order to + * kick the VCPU out of VMXON or even guest mode on reset. This has to be done + * before writing any other state because forcibly leaving nested mode + * invalidates the VCPU state. + */ +static int kvm_put_msr_feature_control(X86CPU *cpu) +{ +struct { +struct kvm_msrs info; +struct kvm_msr_entry entry; +} msr_data; + +kvm_msr_entry_set(msr_data.entry, MSR_IA32_FEATURE_CONTROL, + cpu-env.msr_ia32_feature_control); +msr_data.info.nmsrs = 1; +return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, msr_data); +} + static int kvm_put_msrs(X86CPU *cpu, int level) { CPUX86State *env = cpu-env; @@ -1204,10 +1223,8 @@ static int kvm_put_msrs(X86CPU *cpu, int level) if (cpu-hyperv_vapic) { kvm_msr_entry_set(msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0); } -if (has_msr_feature_control) { -kvm_msr_entry_set(msrs[n++], MSR_IA32_FEATURE_CONTROL, - env-msr_ia32_feature_control); -} +/* Note: MSR_IA32_FEATURE_CONTROL is written separately, see + * kvm_put_msr_feature_control. */ } if (env-mcg_cap) { int i; @@ -1801,6 +1818,13 @@ int kvm_arch_put_registers(CPUState *cpu, int level) assert(cpu_is_stopped(cpu) || qemu_cpu_is_self(cpu)); +if (level = KVM_PUT_RESET_STATE has_msr_feature_control) { +ret = kvm_put_msr_feature_control(x86_cpu); +if (ret 0) { +return ret; +} +} + ret = kvm_getput_regs(x86_cpu, 1); if (ret 0) { return ret; -- 1.8.1.1.298.ge7eed54
Re: [Qemu-devel] [PATCH v3 4/6] qemu-option: support +foo/-foo command line agruments
On 2013-11-11 13:41, Andreas Färber wrote: Am 11.11.2013 08:44, schrieb Alexey Kardashevskiy: This converts +foo/-foo to foo=on/foo=off respectively when QEMU parser is used for the command line options. -cpu parsers in x86 and other architectures should be unaffected by this change. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- util/qemu-option.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/util/qemu-option.c b/util/qemu-option.c index efcb5dc..6c8667c 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -890,6 +890,12 @@ static int opts_do_parse(QemuOpts *opts, const char *params, if (strncmp(option, no, 2) == 0) { memmove(option, option+2, strlen(option+2)+1); pstrcpy(value, sizeof(value), off); +} else if (strncmp(option, -, 1) == 0) { +memmove(option, option+1, strlen(option+1)+1); +pstrcpy(value, sizeof(value), off); +} else if (strncmp(option, +, 1) == 0) { +memmove(option, option+1, strlen(option+1)+1); +pstrcpy(value, sizeof(value), on); } else { pstrcpy(value, sizeof(value), on); } This looks like an interesting idea! However this is much too big a change to just CC ppc folks on... Jan, I wonder if this might break slirp's hostfwd option? hostfwd starts with : in the simplest case - or what pattern do you have in mind? Jan Not sure what other options potentially starting with '-' might be affected. Test cases would be a helpful way of demonstrating that this change does not have undesired side effects. Regards, Andreas -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH] gtk: Allow to activate grab-on-hover from the command line
From: Jan Kiszka jan.kis...@siemens.com As long as we have no persistent GTK configuration, this allows to enable the useful grab-on-hover feature already when starting the VM. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- include/ui/console.h | 2 +- qemu-options.hx | 5 + ui/gtk.c | 5 - vl.c | 22 +- 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index 98edf41..13420c1 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -339,6 +339,6 @@ int index_from_keycode(int code); /* gtk.c */ void early_gtk_display_init(void); -void gtk_display_init(DisplayState *ds, bool full_screen); +void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover); #endif diff --git a/qemu-options.hx b/qemu-options.hx index 5dc8b75..2cb11b5 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -809,6 +809,7 @@ ETEXI DEF(display, HAS_ARG, QEMU_OPTION_display, -display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]\n [,window_close=on|off]|curses|none|\n +gtk[,grab_on_hover=on|off]|\n vnc=display[,optargs]\n select display type\n, QEMU_ARCH_ALL) STEXI @@ -832,6 +833,10 @@ graphics card, but its output will not be displayed to the QEMU user. This option differs from the -nographic option in that it only affects what is done with video output; -nographic also changes the destination of the serial and parallel port data. +@item gtk +Display video output in a GTK window. This interface provides drop-down +menus and other UI elements to configure and control the VM during +runtime. @item vnc Start a VNC server on display arg @end table diff --git a/ui/gtk.c b/ui/gtk.c index b5f4f0b..43883b7 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -1443,7 +1443,7 @@ static const DisplayChangeListenerOps dcl_ops = { .dpy_cursor_define = gd_cursor_define, }; -void gtk_display_init(DisplayState *ds, bool full_screen) +void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover) { GtkDisplayState *s = g_malloc0(sizeof(*s)); char *filename; @@ -1522,6 +1522,9 @@ void gtk_display_init(DisplayState *ds, bool full_screen) if (full_screen) { gtk_menu_item_activate(GTK_MENU_ITEM(s-full_screen_item)); } +if (grab_on_hover) { +gtk_menu_item_activate(GTK_MENU_ITEM(s-grab_on_hover_item)); +} register_displaychangelistener(s-dcl); diff --git a/vl.c b/vl.c index 4ad15b8..7b1b02b 100644 --- a/vl.c +++ b/vl.c @@ -201,6 +201,7 @@ int vga_interface_type = VGA_NONE; static int full_screen = 0; static int no_frame = 0; int no_quit = 0; +static bool grab_on_hover; CharDriverState *serial_hds[MAX_SERIAL_PORTS]; CharDriverState *parallel_hds[MAX_PARALLEL_PORTS]; CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES]; @@ -,6 +2223,25 @@ static DisplayType select_display(const char *p) } else if (strstart(p, gtk, opts)) { #ifdef CONFIG_GTK display = DT_GTK; +while (*opts) { +const char *nextopt; + +if (strstart(opts, ,grab_on_hover=, nextopt)) { +opts = nextopt; +if (strstart(opts, on, nextopt)) { +grab_on_hover = true; +} else if (strstart(opts, off, nextopt)) { +grab_on_hover = false; +} else { +goto invalid_gtk_args; +} +} else { +invalid_gtk_args: +fprintf(stderr, Invalid GTK option string: %s\n, p); +exit(1); +} +opts = nextopt; +} #else fprintf(stderr, GTK support is disabled\n); exit(1); @@ -4284,7 +4304,7 @@ int main(int argc, char **argv, char **envp) #endif #if defined(CONFIG_GTK) case DT_GTK: -gtk_display_init(ds, full_screen); +gtk_display_init(ds, full_screen, grab_on_hover); break; #endif default: -- 1.8.1.1.298.ge7eed54 signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/7] usb: remove old usb-host code
On 2013-10-09 13:42, Gerd Hoffmann wrote: Hi, Only very recent distros fulfill the need of = 1.0.13, so you naturally fall back to this code. I just realized that even the factory build of OpenSUSE is still on libusb-1.0.9. Current Ubuntu versions are on 1.0.12 at best. Didn't check others so far. Ouch. The 1.0.13 release is one year old by now. Fedora 19 is at 1.0.16 btw. So isn't this step a bit too early? There is always the 'git revert' option in case it turns out there are too many issues ... So what to do? Do you expect all the other distros to catch up regarding libusb until QEMU 1.7 is released? They will update once they figure this is needed for qemu 1.7 :) Updates will probably not yet be available at release time though. If we revert, thereby continue fallback to the old code, chances are high that nothing happens and we'll face the same issue for qemu 1.8. Given that there are some known issues in the host-linux code which are fixed in host-libusb I really want get rid of it. OK, then here is the first issue I ran into while trying libusbx (git head, i.e. 1.0.17+: The new stack causes significant latency issues that makes it almost unusable for pass-through of USB audio devices (some headset in my case). Reverting usb-linux and disabling libusb over QEMU git head makes things work again. I'll have to stick with this for now as it is affecting my work environment. Any spontaneous ideas how to analyse or even resolve this? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 1/7] usb: remove old usb-host code
On 2013-11-08 16:39, Gerd Hoffmann wrote: Hi, OK, then here is the first issue I ran into while trying libusbx (git head, i.e. 1.0.17+: The new stack causes significant latency issues that makes it almost unusable for pass-through of USB audio devices (some headset in my case). Reverting usb-linux and disabling libusb over QEMU git head makes things work again. I'll have to stick with this for now as it is affecting my work environment. Any spontaneous ideas how to analyse or even resolve this? Try setting isobsize property to something smaller than 32 (which is the default). OK, isobsize=2 and isobufs=32 helped, possibly other combinations as well - but not just reducing isobsize or increasing isobufs. Any theory about this? How can we find better defaults? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
On 2013-11-06 16:08, Aneesh Kumar K.V wrote: Jan Kiszka jan.kis...@siemens.com writes: On 2013-10-07 18:53, Aneesh Kumar K.V wrote: From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com Targets like ppc64 support different typed of KVM, one which use hypervisor mode and the other which doesn't. Add a new machine property kvm_type that helps in selecting the respective ones We also add a new QEMUMachine callback get_vm_type that helps in mapping the string representation of kvm type specified. Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com ... diff --git a/vl.c b/vl.c index 4e709d5..7ecc581 100644 --- a/vl.c +++ b/vl.c @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = { .name = usb, .type = QEMU_OPT_BOOL, .help = Set on/off to enable/disable usb, +},{ +.name = kvm_type, +.type = QEMU_OPT_STRING, +.help = Set to kvm type to be used in create vm ioctl, This does not sound like an appropriate documentation for an enduser. Any other suggestion for that ? It is actually a string which will be parsed by the machine opt callback, and the return value is used as the argument to create vm ioctl. Maybe something like this: Specifies the KVM virtualization mode (xxx, yyy, whatever) No user is interesting in how this information gets transfered to the KVM core. That's an irrelavant implementation detail. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] USB Passthrough not working for Windows 7 guest
On 2013-11-05 17:01, Frederich, Jens wrote: Hi all, we're currently evaluating different RTOS systems (Windows CE, Intime, RTX, etc.). One system is Linux RT + KVM/QEMU with a Windows 7 guest. Up to now all works fine, Linux RT has good latency and KVM/Qemu setup was easy. But one QEMU bug breaks my measurement setup and evaluation. I've some usb devices for the Windows 7 guest. I configure them as USB passthrough. The devices appears in the device manager of Windows 7, but with Error code 10: device cannot start. The Windows driver fails on USB set configuration. The driver creates a IRP and send it via IOCTRL to lower layer. The IOCTRL fails with invalid parameter. driver log: 0009 0.65470564 vnCDrvUsbControlRequestSetConfiguration, WdfUsbTargetDeviceSelectConfig single interface failed 0xc00d 0010 0.65472370 vnCDrvUsbIFPrepareHardwareState, vnCDrvUsbControlRequestSetConfiguration failed: 0xc00d 0011 0.65473646 vnCDrvDevConPrepareHardware, vnCDrvUsbIFPrepareHardwareState failed 0xc00d 0012 0.65474838 vnCDrvEvtDevicePrepareHardware, vnCDrvDevConPrepareHardware failed 0xc001 0013 0.6547 This bug breaks my latency measurement setup and Linux RT is out of the evaluationg race. Windows CE should not win :-), it there anyway workaround or hack to fix the issue? Workaround: Pass-through one of the (typically) many USB host controllers to the Windows guest (vfio or classic pci-assign). I did this back then when *HCI emulation was still pretty immature. But USB device pass-through should also work. Do you happen to pass a USB 2.0 device via an emulated UHCI? Or are you already using the EHCI emulation? In the latter case, activating USB tracing (see also qemu/docs/tracing.txt) and posting the results here may help analysing the issue. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] USB Passthrough not working for Windows 7 guest
On 2013-11-05 21:20, Jens Frederich wrote: On 2013-11-05 17:01, Frederich, Jens wrote: Hi all, we're currently evaluating different RTOS systems (Windows CE, Intime, RTX, etc.). One system is Linux RT + KVM/QEMU with a Windows 7 guest. Up to now all works fine, Linux RT has good latency and KVM/Qemu setup was easy. But one QEMU bug breaks my measurement setup and evaluation. I've some usb devices for the Windows 7 guest. I configure them as USB passthrough. The devices appears in the device manager of Windows 7, but with Error code 10: device cannot start. The Windows driver fails on USB set configuration. The driver creates a IRP and send it via IOCTRL to lower layer. The IOCTRL fails with invalid parameter. driver log: 0009 0.65470564 vnCDrvUsbControlRequestSetConfiguration, WdfUsbTargetDeviceSelectConfig single interface failed 0xc00d 0010 0.65472370 vnCDrvUsbIFPrepareHardwareState, vnCDrvUsbControlRequestSetConfiguration failed: 0xc00d 0011 0.65473646 vnCDrvDevConPrepareHardware, vnCDrvUsbIFPrepareHardwareState failed 0xc00d 0012 0.65474838 vnCDrvEvtDevicePrepareHardware, vnCDrvDevConPrepareHardware failed 0xc001 0013 0.6547 This bug breaks my latency measurement setup and Linux RT is out of the evaluationg race. Windows CE should not win :-), it there anyway workaround or hack to fix the issue? Workaround: Pass-through one of the (typically) many USB host controllers to the Windows guest (vfio or classic pci-assign). I did this back then when *HCI emulation was still pretty immature. But USB device pass-through should also work. Do you happen to pass a USB 2.0 device via an emulated UHCI? Or are you already using the EHCI emulation? I'm not sure which mode it has been. I've used the virt-manager to configure the device. A usb controller is already configured in mode 'default'. My steps on virt-manager: 1. add hardware 2. select usb host device 3. I can see my usb device, I select it 4. start guest and open Windows device manager Unfortunately, I do not know what virt-manager is configuring by default. It likely also depends on its version, so you should share this information as well. Maybe other folks here can comment on this. I don't know is this UHCI or EHCI? On the usb host device list are some controller listed e.q. xhci, ehci and so on. Should I map these controller to Windows 7 as well? Pick the host controller to which the USB device you want to give to the guest is attached to (lsusb and the bus number reported via /sys/bus/pci/devices/id/usbX can tell this - or trial and error). When doing this, you no longer need to pass through the USB device itself, it is implicitely passed. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
On 2013-10-07 18:53, Aneesh Kumar K.V wrote: From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com Targets like ppc64 support different typed of KVM, one which use hypervisor mode and the other which doesn't. Add a new machine property kvm_type that helps in selecting the respective ones We also add a new QEMUMachine callback get_vm_type that helps in mapping the string representation of kvm type specified. Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com ... diff --git a/vl.c b/vl.c index 4e709d5..7ecc581 100644 --- a/vl.c +++ b/vl.c @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = { .name = usb, .type = QEMU_OPT_BOOL, .help = Set on/off to enable/disable usb, +},{ +.name = kvm_type, +.type = QEMU_OPT_STRING, +.help = Set to kvm type to be used in create vm ioctl, This does not sound like an appropriate documentation for an enduser. OTOH, I do not recall right now how these help strings can be obtained at all. One could intuitively try -machine sometype,?, but that's not implemented. Anyone? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] rtc: remove dead SQW IRQ code
On 2013-08-14 13:29, Jan Kiszka wrote: This was once introduced by commit 100d9891d6 but was never used in-tree and then got broken by commit 32e0c8260d. Time to clean up. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/timer/mc146818rtc.c |9 + 1 files changed, 1 insertions(+), 8 deletions(-) diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c index 3c3baac..b3f1baa 100644 --- a/hw/timer/mc146818rtc.c +++ b/hw/timer/mc146818rtc.c @@ -70,7 +70,6 @@ typedef struct RTCState { uint64_t last_update; int64_t offset; qemu_irq irq; -qemu_irq sqw_irq; int it_shift; /* periodic timer */ QEMUTimer *periodic_timer; @@ -151,8 +150,7 @@ static void periodic_timer_update(RTCState *s, int64_t current_time) period_code = s-cmos_data[RTC_REG_A] 0x0f; if (period_code != 0 - ((s-cmos_data[RTC_REG_B] REG_B_PIE) -|| ((s-cmos_data[RTC_REG_B] REG_B_SQWE) s-sqw_irq))) { + (s-cmos_data[RTC_REG_B] REG_B_PIE)) { if (period_code = 2) period_code += 7; /* period in 32 Khz cycles */ @@ -202,11 +200,6 @@ static void rtc_periodic_timer(void *opaque) #endif qemu_irq_raise(s-irq); } -if (s-cmos_data[RTC_REG_B] REG_B_SQWE) { -/* Not square wave at all but we don't want 2048Hz interrupts! - Must be seen as a pulse. */ -qemu_irq_raise(s-sqw_irq); -} } /* handle update-ended timer */ ping Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type
On 2013-11-04 14:30, Alexander Graf wrote: On 04.11.2013, at 14:28, Jan Kiszka jan.kis...@siemens.com wrote: On 2013-10-07 18:53, Aneesh Kumar K.V wrote: From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com Targets like ppc64 support different typed of KVM, one which use hypervisor mode and the other which doesn't. Add a new machine property kvm_type that helps in selecting the respective ones We also add a new QEMUMachine callback get_vm_type that helps in mapping the string representation of kvm type specified. Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com ... diff --git a/vl.c b/vl.c index 4e709d5..7ecc581 100644 --- a/vl.c +++ b/vl.c @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = { .name = usb, .type = QEMU_OPT_BOOL, .help = Set on/off to enable/disable usb, +},{ +.name = kvm_type, +.type = QEMU_OPT_STRING, +.help = Set to kvm type to be used in create vm ioctl, This does not sound like an appropriate documentation for an enduser. OTOH, I do not recall right now how these help strings can be obtained at all. One could intuitively try -machine sometype,?, but that's not implemented. Anyone? They should definitely show up in the man page. But yes, -machine kvm_type=? would be helpful as well. The man page is not generate from this C code, is it? As for -machine ? we are have a problem orthogonal to this patch. I agree that we need some way to programmatically list all machine options. I'm not saying that this is an issue of this patch, just that I wonder how that .help message can be made visisble at at besides with the help of an editor. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH uq/master] pci-assign: Remove dead code for direct I/O region access from userspace
This feature was already deprecated back then in qemu-kvm, ie. before pci-assign went upstream. assigned_dev_ioport_rw will never be invoked with resource_fd 0. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/i386/kvm/pci-assign.c | 56 +--- 1 file changed, 10 insertions(+), 46 deletions(-) diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index 011764f..4e65110 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -154,55 +154,19 @@ static uint64_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region, uint64_t val = 0; int fd = dev_region-region-resource_fd; -if (fd = 0) { -if (data) { -DEBUG(pwrite data=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx - , addr=TARGET_FMT_plx\n, *data, size, addr, addr); -if (pwrite(fd, data, size, addr) != size) { -error_report(%s - pwrite failed %s, - __func__, strerror(errno)); -} -} else { -if (pread(fd, val, size, addr) != size) { -error_report(%s - pread failed %s, - __func__, strerror(errno)); -val = (1UL (size * 8)) - 1; -} -DEBUG(pread val=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx - , addr= TARGET_FMT_plx \n, val, size, addr, addr); +if (data) { +DEBUG(pwrite data=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx + , addr=TARGET_FMT_plx\n, *data, size, addr, addr); +if (pwrite(fd, data, size, addr) != size) { +error_report(%s - pwrite failed %s, __func__, strerror(errno)); } } else { -uint32_t port = addr + dev_region-u.r_baseport; - -if (data) { -DEBUG(out data=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx - , host=%x\n, *data, size, addr, port); -switch (size) { -case 1: -outb(*data, port); -break; -case 2: -outw(*data, port); -break; -case 4: -outl(*data, port); -break; -} -} else { -switch (size) { -case 1: -val = inb(port); -break; -case 2: -val = inw(port); -break; -case 4: -val = inl(port); -break; -} -DEBUG(in data=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx - , host=%x\n, val, size, addr, port); +if (pread(fd, val, size, addr) != size) { +error_report(%s - pread failed %s, __func__, strerror(errno)); +val = (1UL (size * 8)) - 1; } +DEBUG(pread val=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx + , addr= TARGET_FMT_plx \n, val, size, addr, addr); } return val; } -- 1.8.1.1.298.ge7eed54
Re: [Qemu-devel] [PATCH] qemu: Broken -smb with latest SAMBA package. (Unsupported security=share option)
On 2013-11-04 14:55, Michael Tokarev wrote: 04.11.2013 00:06, Jan Kiszka wrote: On 2013-11-01 11:10, Michael Tokarev wrote: [] If Jan picks it up, that's fine. If not, I think it can go to the trivial patches queue. Works fine, applied to queues/slirp. Okay, thank you Jan. But this is not a trivial patch as the fix is not obvious for a reader (unless you know smb.conf semantics by heart). It's trivial for my understanding. If we require that every change going to -trivial should be obvious to everyone, we should just close it right away. Then we may need -less-trivial, because - to my understanding - -trivial was once set up according to the rule that (most) QEMU hackers should be able to understand that a trivial change is at least mostly harmless. And this area does not have an active maintainer anyway, at least according to MAINTAINERS and ./scripts/get_maintainer.pl. Yeah, I think we have some holes there. That smb configuration conceptually belongs to slirp is right, just not clear documented in our script. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] qemu: Broken -smb with latest SAMBA package. (Unsupported security=share option)
On 2013-11-01 11:10, Michael Tokarev wrote: 01.11.2013 13:54, Michael Büsch wrote: On Fri, 01 Nov 2013 13:32:49 +0400 Michael Tokarev m...@tls.msk.ru wrote: That looks right. Are you okay adding your Signed-off-by to the patch you initially submitted? If yes, I'll make a formal patch submission upstream. Here you go. Thank you! Adding Jan as slirp maintainer, and my Reviewed-by: Michael Tokarev m...@tls.msk.ru If Jan picks it up, that's fine. If not, I think it can go to the trivial patches queue. Works fine, applied to queues/slirp. But this is not a trivial patch as the fix is not obvious for a reader (unless you know smb.conf semantics by heart). Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [Bug 1246890] [NEW] AC97 sound card crashes QEMU
On 2013-10-31 21:48, John Arbuckle wrote: Public bug reported: The AC97 sound card does not work. It stops QEMU on startup. The cause appears to be some kind of deadlock. Steps to reproduce: Just add -soundhw ac97 to QEMU's arguments. Example: qemu-system-ppc -soundhw ac97 The example above is all it takes to reproduce the problem. This problem has been observed on Mac OS X and Debian Linux. I question whether the ac97 support ever worked. It is a file that was taken from VirtualBox and added to QEMU. I do know that VirtualBox's support for the ac97 sound card works perfectly. The exact line of code that stops QEMU in its tracks is located in the file main-loop.c, in the function os_host_main_loop_wait(), the call made to qemu_mutex_lock_iothread(). The is where QEMU stops under Mac OS X. ** Affects: qemu Importance: Undecided Status: New Maybe this is just a regression: I'm using ac97 for a win7 guest for a while, and since recently (don't recall when precisely, some weeks maybe) I'm getting main-loop: WARNING: I/O thread spun for 1000 iterations, a temporarily stuck guest and broken sound output. This used to work fine. Someone has to bisect, I didn't find the time yet. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2] cpu-exec: Fix compiler warning (-Werror=clobbered)
On 2013-10-31 20:31, Stefan Weil wrote: Reloading of local variables after sigsetjmp is only needed for some buggy compilers. The code which should reload these variables causes compiler warnings with gcc 4.7 when compiler optimizations are enabled: cpu-exec.c:204:15: error: variable ‘cpu’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] cpu-exec.c:207:15: error: variable ‘cc’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] cpu-exec.c:202:28: error: argument ‘env’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered] Now this code is only used for compilers which need it (and gcc 4.5.x, x 0 which does not need it but won't give warnings). There were bug reports for clang and gcc 4.5.0, while gcc 4.5.1 was reported to work fine without the reload code. Signed-off-by: Stefan Weil s...@weilnetz.de --- v2: Don't remove the code which causes the warnings, but use it only with clang or gcc 4.6. cpu-exec.c |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cpu-exec.c b/cpu-exec.c index 30cfa2a..fec20c3 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -677,14 +677,18 @@ int cpu_exec(CPUArchState *env) only be set by a memory fault) */ } /* for(;;) */ } else { -/* Reload env after longjmp - the compiler may have smashed all - * local variables as longjmp is marked 'noreturn'. */ +#if defined(__clang__) || !QEMU_GNUC_PREREQ(4, 6) +/* Some compilers wrongly smash all local variables after + * siglongjmp. There were bug reports for gcc 4.5.0 and clang. + * Reload essential local variables here for those compilers. + * gcc 4.7 would complain about this code (-Wclobbered). */ cpu = current_cpu; env = cpu-env_ptr; #if !(defined(CONFIG_USER_ONLY) \ (defined(TARGET_M68K) || defined(TARGET_PPC) || defined(TARGET_S390X))) cc = CPU_GET_CLASS(cpu); #endif +#endif /* __clang__ or old gcc */ } } /* for(;;) */ Are all clang versions affected? Then this looks reasonable. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] cpu-exec: Fix compiler warning (-Werror=clobbered)
On 2013-10-28 20:18, Stefan Weil wrote: Am 18.09.2013 09:48, schrieb Jan Kiszka: On 2013-09-18 09:26, Peter Maydell wrote: [...] And gcc's documentation of the 'noreturn' attribute specifically says it does not affect the exceptional path where the function returns via longjmp. OK, that is the clarifying bit of information. Now the question is if want to drop support for faulty compilers again, work around the false-positive warning, or avoid the issue differently than via reloading. Jan Recently commit 6c78f29a2424622bfc9c30dfbbc13404481eacb6 added a third variable which is reloaded now. Obviously the clang compiler needs this workaround. Jan, can you remember whether the initial problems were also caused by clang? If yes, we might restrict the code to that compiler. This would avoid the -Wclobbered warnings with newer gcc while still fixing the code generated by clang. Look up this thread: gcc 4.5.0 Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 06/12] fix qemu_alloc/qemu_free for slirp subsystem
On 2009-06-18 22:50, Jean-Christophe DUBOIS wrote: From: Jean-Christophe Dubois jcd@jcd-laptop.(none) Signed-off-by: Jean-Christophe DUBOIS j...@tribudubois.net --- slirp/socket.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/slirp/socket.c b/slirp/socket.c index 82d026c..e4d84d7 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -53,7 +53,7 @@ socreate(void) { struct socket *so; - so = (struct socket *)malloc(sizeof(struct socket)); + so = (struct socket *)qemu_malloc(sizeof(struct socket)); if(so) { qemu_malloc doesn't return NULL. So you should clean up more here, and possibly elsewhere. Jan memset(so, 0, sizeof(struct socket)); so-so_state = SS_NOFDREF; @@ -82,7 +82,7 @@ sofree(struct socket *so) if(so-so_next so-so_prev) remque(so); /* crashes if so is not in a queue */ - free(so); + qemu_free(so); } size_t sopreprbuf(struct socket *so, struct iovec *iov, int *np) @@ -606,13 +606,13 @@ solisten(u_int port, u_int32_t laddr, u_int lport, int flags) DEBUG_ARG(flags = %x, flags); if ((so = socreate()) == NULL) { - /* free(so); Not sofree() ??? free(NULL) == NOP */ + /* qemu_free(so); Not sofree() ??? qemu_free(NULL) == NOP */ return NULL; } /* Don't tcp_attach... we don't need so_snd nor so_rcv */ if ((so-so_tcpcb = tcp_newtcpcb(so)) == NULL) { - free(so); + qemu_free(so); return NULL; } insque(so,tcb); signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/7] usb: remove old usb-host code
On 2013-10-07 09:16, Gerd Hoffmann wrote: On Mi, 2013-10-02 at 15:14 +0200, Jan Kiszka wrote: On 2013-09-19 11:34, Gerd Hoffmann wrote: The usb-host code has been rewritten for qemu 1.5 to use libusb, the old code has been left in as temporary fallback. Now we are two releases further out, targeting the 1.7 release. No major issues with the new code poped up until now. Time to remove it from tre tree. Should we ever need it again for some reason -- git has a copy for us in the history. Well, maybe not many users were actually exploiting the new libusb code, thus were able to produce feedback. Only very recent distros fulfill the need of = 1.0.13, so you naturally fall back to this code. I just realized that even the factory build of OpenSUSE is still on libusb-1.0.9. Current Ubuntu versions are on 1.0.12 at best. Didn't check others so far. Ouch. The 1.0.13 release is one year old by now. Fedora 19 is at 1.0.16 btw. So isn't this step a bit too early? There is always the 'git revert' option in case it turns out there are too many issues ... So what to do? Do you expect all the other distros to catch up regarding libusb until QEMU 1.7 is released? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] cpu-exec(): also reload CPUClass *cc after longjmp return
On 2013-10-03 18:05, Peter Maydell wrote: On 3 October 2013 23:09, Juergen Lock qem...@jelal.kn-bremen.de wrote: Local variable CPUClass *cc needs to be reloaded after return from longjmp too. (This fixes the mips-softmmu crash observed on FreeBSD when qemu is built with clang.) Signed-off-by: Juergen Lock n...@jelal.kn-bremen.de Found-by: Dimitry Andric d...@freebsd.org --- a/cpu-exec.c +++ b/cpu-exec.c @@ -681,6 +681,10 @@ int cpu_exec(CPUArchState *env) * local variables as longjmp is marked 'noreturn'. */ cpu = current_cpu; env = cpu-env_ptr; +#if !(defined(CONFIG_USER_ONLY) \ + (defined(TARGET_M68K) || defined(TARGET_PPC) || defined(TARGET_S390X))) +cc = CPU_GET_CLASS(cpu); +#endif This is a c compiler or libc bug -- the C standard says that this local variable should not be trashed by the longjmp. We were actually discussing removing the current workarounds there... But we didn't decide if we should stop supporting the affected compiler versions. Does this issue also exist with the latest clang version available for your platform? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 1/7] usb: remove old usb-host code
On 2013-09-19 11:34, Gerd Hoffmann wrote: The usb-host code has been rewritten for qemu 1.5 to use libusb, the old code has been left in as temporary fallback. Now we are two releases further out, targeting the 1.7 release. No major issues with the new code poped up until now. Time to remove it from tre tree. Should we ever need it again for some reason -- git has a copy for us in the history. Well, maybe not many users were actually exploiting the new libusb code, thus were able to produce feedback. Only very recent distros fulfill the need of = 1.0.13, so you naturally fall back to this code. I just realized that even the factory build of OpenSUSE is still on libusb-1.0.9. Current Ubuntu versions are on 1.0.12 at best. Didn't check others so far. So isn't this step a bit too early? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux