Re: [Qemu-devel] [PATCH uq/master] kvm: Drop unused kvm_pit_in_kernel
On Wed, Mar 21, 2012 at 02:49:09PM +0100, Jan Kiszka wrote: > On 2012-03-21 14:41, Gleb Natapov wrote: > > On Wed, Mar 21, 2012 at 02:39:47PM +0100, Jan Kiszka wrote: > >> On 2012-03-21 14:36, Avi Kivity wrote: > >>> On 03/21/2012 02:36 PM, Jan Kiszka wrote: > This is now implied by kvm_irqchip_in_kernel. > > > >>> > >>> So we can't have -no-kvm-pit? > >>> > >>> No huge loss, but unexpected. > >> > >> See e81dda195556e72f8cd294998296c1051aab30a8. > >> > > I am curious what is the reason for upstream to not supporting disabling the > > in-kernel PIT separately? > > It was considered no longer relevant: > http://thread.gmane.org/gmane.comp.emulators.kvm.devel/85393 > Hmm, may be we should think about this some more. If in the (not so far) future we want to drop pit emulation from the kernel we may want to support -no-kvm-pit to allow migration from old kernels to new one. -- Gleb.
Re: [Qemu-devel] [PATCH 00/11 v10] introducing a new, dedicated guest memory dump mechanism
Does anyone have time to review this patchset? Thanks Wen Congyang At 03/20/2012 11:28 AM, Wen Congyang Wrote: > Hi, all > > 'virsh dump' can not work when host pci device is used by guest. We have > discussed this issue here: > http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.html > > The last version is here: > http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg02536.html > > We have determined to introduce a new command dump-guest-memory to dump > guest's memory. The core file's format is elf32 or elf64. > > Note: > 1. The guest should be x86 or x86_64. The other arch is not supported now. > 2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not crash. > 3. If the OS is in the second kernel, gdb may not work well, and crash can >work by specifying '--machdep phys_addr=xxx' in the command line. The >reason is that the second kernel will update the page table, and we can >not get the page table for the first kernel. > 4. The cpu's state is stored in QEMU note. You neet to modify crash to use >it to calculate phys_base. > 5. If the guest OS is 32 bit and the memory size is larger than 4G, the vmcore >is elf64 format. You should use the gdb which is built with > --enable-64-bit-bfd. > 6. This patchset is based on the upstream tree, and apply one patch that is > still >in Luiz Capitulino's tree, because I use the API qemu_get_fd() in this > patchset. > > Changes from v9 to v10: > 1. fix some bug > 2. addressed Luiz's and Hatayam's comment > 3. remove cancel and query command > > Changes from v8 to v9: > 1. remove async support(it will be reimplemented after QAPI async commands > support >is finished) > 2. fix some typo error > > Changes from v7 to v8: > 1. addressed Hatayama's comments > > Changes from v6 to v7: > 1. addressed Jan's comments > 2. fix some bugs > 3. store cpu's state into the vmcore > > Changes from v5 to v6: > 1. allow user to dump a fraction of the memory > 2. fix some bugs > > Changes from v4 to v5: > 1. convert the new command dump to QAPI > > Changes from v3 to v4: > 1. support it to run asynchronously > 2. add API to cancel dumping and query dumping progress > 3. add API to control dumping speed > 4. auto cancel dumping when the user resumes vm, and the status is failed. > > Changes from v2 to v3: > 1. address Jan Kiszka's comment > > Changes from v1 to v2: > 1. fix virt addr in the vmcore. > > Wen Congyang (11): > Add API to create memory mapping list > Add API to check whether a physical address is I/O address > implement cpu_get_memory_mapping() > Add API to check whether paging mode is enabled > Add API to get memory mapping > Add API to get memory mapping without do paging > target-i386: Add API to write elf notes to core file > target-i386: Add API to write cpu status to core file > target-i386: add API to get dump info > make gdb_id() generally avialable > introduce a new monitor command 'dump-guest-memory' to dump guest's > memory > > Makefile.target |3 + > configure |8 + > cpu-all.h | 67 +++ > cpu-common.h |2 + > dump.c| 841 > + > dump.h| 23 + > elf.h |5 + > exec.c|9 + > gdbstub.c |9 - > gdbstub.h |9 + > hmp-commands.hx | 28 ++ > hmp.c | 22 + > hmp.h |1 + > memory_mapping.c | 236 +++ > memory_mapping.h | 68 +++ > qapi-schema.json | 18 + > qmp-commands.hx | 38 ++ > target-i386/arch_dump.c | 425 +++ > target-i386/arch_memory_mapping.c | 271 > 19 files changed, 2074 insertions(+), 9 deletions(-) > create mode 100644 dump.c > create mode 100644 dump.h > create mode 100644 memory_mapping.c > create mode 100644 memory_mapping.h > create mode 100644 target-i386/arch_dump.c > create mode 100644 target-i386/arch_memory_mapping.c > > >
Re: [Qemu-devel] [PATCH 0/2 v3] kvm: notify host when guest panicked
On Wed, Mar 21, 2012 at 02:19:34PM -0500, Anthony Liguori wrote: > On 03/21/2012 11:25 AM, Avi Kivity wrote: > >On 03/21/2012 06:18 PM, Corey Minyard wrote: > >> > >>>Look at drivers/char/ipmi/ipmi_msghandler.c. It has code to send panic > >>>event over IMPI. The code is pretty complex. Of course if we a going to > >>>implement something more complex than simple hypercall for panic > >>>notification we better do something more interesting with it than just > >>>saying "panic happened", like sending stack traces on all cpus for > >>>instance. > >> > >>I doubt that's the best example, unfortunately. The IPMI event log > >>has limited space and it has to be send a little piece at a time since > >>each log entry is 14 bytes. It just prints the panic string, nothing > >>else. Not that it isn't useful, it has saved my butt before. > >> > >>You have lots of interesting options with paravirtualization. You > >>could, for instance, create a console driver that delivered all > >>console output efficiently through a hypercall. That would be really > >>easy. Or, as you mention, a custom way to deliver panic information. > >>Collecting information like stack traces would be harder to > >>accomplish, as I don't think there is currently a way to get it except > >>by sending it to printk. > > > >That already exists; virtio-console (or serial console emulation) can do > >the job. > > I think the use case here is pretty straight forward: if the guest > finds itself in bad place, it wants to indicate that to the host. > > We shouldn't rely on any device drivers or complex code. It should > be as close to a single instruction as possible that can run even if > interrupts are disabled. > > An out instruction fits this very well. I think a simple protocol like: > > inl PORT -> returns a magic number indicating the presence of qemucalls > inl PORT+1 -> returns a bitmap of supported features > Sigh, one more PV isa device. > outl PORT+1 -> data reg1 > outl PORT+2 -> data reg2 > outl PORT+N -> data regN > > outl PORT -> qemucall of index value with arguments 1..N And you think you can trust panicked SMP guest to not call this on multiple cpus simultaneously? > > Regards, > > Anthony Liguori > > > > >In fact the feature can be implemented 100% host side by searching for a > >panic string signature in the console logs. > > -- Gleb.
Re: [Qemu-devel] [PATCH 0/2 v3] kvm: notify host when guest panicked
On Thu, Mar 22, 2012 at 09:05:12AM +0800, Wen Congyang wrote: > At 03/22/2012 03:19 AM, Anthony Liguori Wrote: > > On 03/21/2012 11:25 AM, Avi Kivity wrote: > >> On 03/21/2012 06:18 PM, Corey Minyard wrote: > >>> > Look at drivers/char/ipmi/ipmi_msghandler.c. It has code to send panic > event over IMPI. The code is pretty complex. Of course if we a going to > implement something more complex than simple hypercall for panic > notification we better do something more interesting with it than just > saying "panic happened", like sending stack traces on all cpus for > instance. > >>> > >>> I doubt that's the best example, unfortunately. The IPMI event log > >>> has limited space and it has to be send a little piece at a time since > >>> each log entry is 14 bytes. It just prints the panic string, nothing > >>> else. Not that it isn't useful, it has saved my butt before. > >>> > >>> You have lots of interesting options with paravirtualization. You > >>> could, for instance, create a console driver that delivered all > >>> console output efficiently through a hypercall. That would be really > >>> easy. Or, as you mention, a custom way to deliver panic information. > >>> Collecting information like stack traces would be harder to > >>> accomplish, as I don't think there is currently a way to get it except > >>> by sending it to printk. > >> > >> That already exists; virtio-console (or serial console emulation) can do > >> the job. > > > > I think the use case here is pretty straight forward: if the guest finds > > itself in bad place, it wants to indicate that to the host. > > > > We shouldn't rely on any device drivers or complex code. It should be > > as close to a single instruction as possible that can run even if > > interrupts are disabled. > > > > An out instruction fits this very well. I think a simple protocol like: > > This solution is more simple than using virtio-serial. > > > > > inl PORT -> returns a magic number indicating the presence of qemucalls > > I donot understantd this instruction's purpose. > > > inl PORT+1 -> returns a bitmap of supported features > > Hmm, we can execute this instruction when guest starts. If the userspace > does not process panicked event, there is no need to notify it. > > > > > outl PORT+1 -> data reg1 > > outl PORT+2 -> data reg2 > > outl PORT+N -> data regN > > We can get the register value from vmcs. So there is no need to tell > the register value to the host. > No device should examine register value. Ideally QEMU would read registers only during migration. > If we decide to avoid touching hypervisor, I agree with this solution. > > Thanks > Wen Congyang > > > > outl PORT -> qemucall of index value with arguments 1..N > > > > Regards, > > > > Anthony Liguori > > > >> > >> In fact the feature can be implemented 100% host side by searching for a > >> panic string signature in the console logs. > >> > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > -- Gleb.
Re: [Qemu-devel] [PATCH 0/2 v3] kvm: notify host when guest panicked
On Wed, Mar 21, 2012 at 02:04:34PM -0500, Anthony Liguori wrote: > On 03/13/2012 05:47 AM, Avi Kivity wrote: > >On 03/13/2012 11:18 AM, Daniel P. Berrange wrote: > >>On Mon, Mar 12, 2012 at 12:33:33PM +0200, Avi Kivity wrote: > >>>On 03/12/2012 11:04 AM, Wen Congyang wrote: > Do you have any other comments about this patch? > > >>> > >>>Not really, but I'm not 100% convinced the patch is worthwhile. It's > >>>likely to only be used by Linux, which has kexec facilities, and you can > >>>put talk to management via virtio-serial and describe the crash in more > >>>details than a simple hypercall. > >> > >>As mentioned before, I don't think virtio-serial is a good fit for this. > >>We want something that is simple& guaranteed always available. Using > >>virtio-serial requires significant setup work on both the host and guest. > > > >So what? It needs to be done anyway for the guest agent. > > > >>Many management application won't know to make a vioserial device available > >>to all guests they create. > > > >Then they won't know to deal with the panic event either. > > > >>Most administrators won't even configure kexec, > >>let alone virtio serial on top of it. > > > >It should be done by the OS vendor, not the individual admin. > > > >>The hypercall requires zero host > >>side config, and zero guest side config, which IMHO is what we need for > >>this feature. > > > >If it was this one feature, yes. But we keep getting more and more > >features like that and we bloat the hypervisor. There's a reason we > >have a host-to-guest channel, we should use it. > > The problem is that virtio-serial sucks for something like this. > How do we know if we haven't tried :) > We have two options I think: > > 1) We could reserve a portion of the hypercall space to be deferred > to userspace for stuff like this. > > 2) We could invent a new hypercall like facility that was less > bloated than virtio-serial for stuff like this using MMIO/PIO. > > Regards, > > Anthony Liguori > > > -- Gleb.
Re: [Qemu-devel] [PATCH 0/2 v3] kvm: notify host when guest panicked
At 03/22/2012 03:28 PM, Gleb Natapov Wrote: > On Wed, Mar 21, 2012 at 02:19:34PM -0500, Anthony Liguori wrote: >> On 03/21/2012 11:25 AM, Avi Kivity wrote: >>> On 03/21/2012 06:18 PM, Corey Minyard wrote: > Look at drivers/char/ipmi/ipmi_msghandler.c. It has code to send panic > event over IMPI. The code is pretty complex. Of course if we a going to > implement something more complex than simple hypercall for panic > notification we better do something more interesting with it than just > saying "panic happened", like sending stack traces on all cpus for > instance. I doubt that's the best example, unfortunately. The IPMI event log has limited space and it has to be send a little piece at a time since each log entry is 14 bytes. It just prints the panic string, nothing else. Not that it isn't useful, it has saved my butt before. You have lots of interesting options with paravirtualization. You could, for instance, create a console driver that delivered all console output efficiently through a hypercall. That would be really easy. Or, as you mention, a custom way to deliver panic information. Collecting information like stack traces would be harder to accomplish, as I don't think there is currently a way to get it except by sending it to printk. >>> >>> That already exists; virtio-console (or serial console emulation) can do >>> the job. >> >> I think the use case here is pretty straight forward: if the guest >> finds itself in bad place, it wants to indicate that to the host. >> >> We shouldn't rely on any device drivers or complex code. It should >> be as close to a single instruction as possible that can run even if >> interrupts are disabled. >> >> An out instruction fits this very well. I think a simple protocol like: >> >> inl PORT -> returns a magic number indicating the presence of qemucalls >> inl PORT+1 -> returns a bitmap of supported features >> > Sigh, one more PV isa device. > >> outl PORT+1 -> data reg1 >> outl PORT+2 -> data reg2 >> outl PORT+N -> data regN >> >> outl PORT -> qemucall of index value with arguments 1..N > And you think you can trust panicked SMP guest to not call this on > multiple cpus simultaneously? We can register panic notifier in the guest kernel, and do it in the panic notifier callback. Thanks Wen Congyang > >> >> Regards, >> >> Anthony Liguori >> >>> >>> In fact the feature can be implemented 100% host side by searching for a >>> panic string signature in the console logs. >>> > > -- > Gleb. > >
Re: [Qemu-devel] [PATCH] uhci: stop queue filling when we find a in-flight td
On March 21, 2012 at 6:36 PM Gerd Hoffmann wrote: > Not only QHs can form rings, but TDs too. With the new > queuing/pipelining support we are following TD chains and > can actually walk in circles. An assert() prevents us from > entering an endless loop then. > > Fix is easy: Just stop queuing when we figure the TD we are > about to queue up is in flight already. > > Signed-off-by: Gerd Hoffmann > --- > hw/usb/hcd-uhci.c |3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c > index e55dad9..2be564b 100644 > --- a/hw/usb/hcd-uhci.c > +++ b/hw/usb/hcd-uhci.c > @@ -965,6 +965,9 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td) > } > trace_usb_uhci_td_queue(plink & ~0xf, ptd.ctrl, ptd.token); > ret = uhci_handle_td(s, plink, &ptd, &int_mask); > +if (ret == TD_RESULT_ASYNC_CONT) { > +break; > +} > assert(ret == TD_RESULT_ASYNC_START); > assert(int_mask == 0); > plink = ptd.link; > -- > 1.7.1 > > Issue fixed - my Windows boots again :-) Thanks a lot! Best regards, Erik
Re: [Qemu-devel] [PATCH 0/2 v3] kvm: notify host when guest panicked
At 03/22/2012 03:12 AM, Anthony Liguori Wrote: > On 03/15/2012 06:46 AM, Avi Kivity wrote: >> On 03/15/2012 01:25 PM, Jan Kiszka wrote: > There was such vm exit (KVM_EXIT_HYPERCALL), but it was deemed to be a bad idea. >>> >>> BTW, this would help a lot in emulating hypercalls of other hypervisors >>> (or of KVM's VAPIC in the absence of in-kernel irqchip - I had to jump >>> through hoops therefore) in user space. Not all those hypercall handlers >>> actually have to reside in the KVM module. >>> >> >> That is true. On the other hand the hypercall ABI might go to pieces if >> there was no central implementation. > > Just declare that outl 0x505 is a megaultracall and s/vmcall/outb/g and > call it a day. Why use 0x505? Is it a reserved port, and it will not be used by any device? How can I get all reserved port? Thanks Wen Congyang > > The performance difference between vmcall and outl is so tiny compared > to the cost of dropping to userspace that it really doesn't matter which > instruction is used. > > Regards, > > Anthony Liguori > >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
Re: [Qemu-devel] [PATCH 0/2 v3] kvm: notify host when guest panicked
At 03/22/2012 03:31 PM, Gleb Natapov Wrote: > On Thu, Mar 22, 2012 at 09:05:12AM +0800, Wen Congyang wrote: >> At 03/22/2012 03:19 AM, Anthony Liguori Wrote: >>> On 03/21/2012 11:25 AM, Avi Kivity wrote: On 03/21/2012 06:18 PM, Corey Minyard wrote: > >> Look at drivers/char/ipmi/ipmi_msghandler.c. It has code to send panic >> event over IMPI. The code is pretty complex. Of course if we a going to >> implement something more complex than simple hypercall for panic >> notification we better do something more interesting with it than just >> saying "panic happened", like sending stack traces on all cpus for >> instance. > > I doubt that's the best example, unfortunately. The IPMI event log > has limited space and it has to be send a little piece at a time since > each log entry is 14 bytes. It just prints the panic string, nothing > else. Not that it isn't useful, it has saved my butt before. > > You have lots of interesting options with paravirtualization. You > could, for instance, create a console driver that delivered all > console output efficiently through a hypercall. That would be really > easy. Or, as you mention, a custom way to deliver panic information. > Collecting information like stack traces would be harder to > accomplish, as I don't think there is currently a way to get it except > by sending it to printk. That already exists; virtio-console (or serial console emulation) can do the job. >>> >>> I think the use case here is pretty straight forward: if the guest finds >>> itself in bad place, it wants to indicate that to the host. >>> >>> We shouldn't rely on any device drivers or complex code. It should be >>> as close to a single instruction as possible that can run even if >>> interrupts are disabled. >>> >>> An out instruction fits this very well. I think a simple protocol like: >> >> This solution is more simple than using virtio-serial. >> >>> >>> inl PORT -> returns a magic number indicating the presence of qemucalls >> >> I donot understantd this instruction's purpose. >> >>> inl PORT+1 -> returns a bitmap of supported features >> >> Hmm, we can execute this instruction when guest starts. If the userspace >> does not process panicked event, there is no need to notify it. >> >>> >>> outl PORT+1 -> data reg1 >>> outl PORT+2 -> data reg2 >>> outl PORT+N -> data regN >> >> We can get the register value from vmcs. So there is no need to tell >> the register value to the host. >> > No device should examine register value. Ideally QEMU would read > registers only during migration. I mean: if the qemu(or other app) want to know the register value, it can get it from vmcs. So there is no need to pass register value from guest to host. Another question: each outl will cause vmexit? Thanks Wen Congyang > >> If we decide to avoid touching hypervisor, I agree with this solution. >> >> Thanks >> Wen Congyang >>> >>> outl PORT -> qemucall of index value with arguments 1..N >>> >>> Regards, >>> >>> Anthony Liguori >>> In fact the feature can be implemented 100% host side by searching for a panic string signature in the console logs. >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >>> the body of a message to majord...@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> Please read the FAQ at http://www.tux.org/lkml/ >>> > > -- > Gleb. > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
Re: [Qemu-devel] [PATCH 0/2 v3] kvm: notify host when guest panicked
On Thu, Mar 22, 2012 at 03:44:45PM +0800, Wen Congyang wrote: > At 03/22/2012 03:31 PM, Gleb Natapov Wrote: > > On Thu, Mar 22, 2012 at 09:05:12AM +0800, Wen Congyang wrote: > >> At 03/22/2012 03:19 AM, Anthony Liguori Wrote: > >>> On 03/21/2012 11:25 AM, Avi Kivity wrote: > On 03/21/2012 06:18 PM, Corey Minyard wrote: > > > >> Look at drivers/char/ipmi/ipmi_msghandler.c. It has code to send panic > >> event over IMPI. The code is pretty complex. Of course if we a going to > >> implement something more complex than simple hypercall for panic > >> notification we better do something more interesting with it than just > >> saying "panic happened", like sending stack traces on all cpus for > >> instance. > > > > I doubt that's the best example, unfortunately. The IPMI event log > > has limited space and it has to be send a little piece at a time since > > each log entry is 14 bytes. It just prints the panic string, nothing > > else. Not that it isn't useful, it has saved my butt before. > > > > You have lots of interesting options with paravirtualization. You > > could, for instance, create a console driver that delivered all > > console output efficiently through a hypercall. That would be really > > easy. Or, as you mention, a custom way to deliver panic information. > > Collecting information like stack traces would be harder to > > accomplish, as I don't think there is currently a way to get it except > > by sending it to printk. > > That already exists; virtio-console (or serial console emulation) can do > the job. > >>> > >>> I think the use case here is pretty straight forward: if the guest finds > >>> itself in bad place, it wants to indicate that to the host. > >>> > >>> We shouldn't rely on any device drivers or complex code. It should be > >>> as close to a single instruction as possible that can run even if > >>> interrupts are disabled. > >>> > >>> An out instruction fits this very well. I think a simple protocol like: > >> > >> This solution is more simple than using virtio-serial. > >> > >>> > >>> inl PORT -> returns a magic number indicating the presence of qemucalls > >> > >> I donot understantd this instruction's purpose. > >> > >>> inl PORT+1 -> returns a bitmap of supported features > >> > >> Hmm, we can execute this instruction when guest starts. If the userspace > >> does not process panicked event, there is no need to notify it. > >> > >>> > >>> outl PORT+1 -> data reg1 > >>> outl PORT+2 -> data reg2 > >>> outl PORT+N -> data regN > >> > >> We can get the register value from vmcs. So there is no need to tell > >> the register value to the host. > >> > > No device should examine register value. Ideally QEMU would read > > registers only during migration. > > I mean: if the qemu(or other app) want to know the register value, it can > get it from vmcs. So there is no need to pass register value from guest > to host. > I understand what you mean and I am saying that reading register values shouldn't be part of the protocol. Examining cpu state postmortem is OK of course. > Another question: each outl will cause vmexit? > Yes, IIRC you can pass through io port to a guest but KVM doesn't do it. > Thanks > Wen Congyang > > > > >> If we decide to avoid touching hypervisor, I agree with this solution. > >> > >> Thanks > >> Wen Congyang > >>> > >>> outl PORT -> qemucall of index value with arguments 1..N > >>> > >>> Regards, > >>> > >>> Anthony Liguori > >>> > > In fact the feature can be implemented 100% host side by searching for a > panic string signature in the console logs. > > >>> > >>> -- > >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > >>> the body of a message to majord...@vger.kernel.org > >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > >>> Please read the FAQ at http://www.tux.org/lkml/ > >>> > > > > -- > > Gleb. > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > -- Gleb.
[Qemu-devel] [Bug 877155] Re: VT-D NIC doesn't work in windows guest
>From my test, this bug is fixed in the latest kvm.git. I tried 82572 NIC for >device assignment in WinXP, Win7 and Win2k8. The VT-d NIC works fine in >Windows guest. kvm.git+qemu-kvm.git = result f4d1f417 +df44afac = bad eb9ede96 +df44afac = good But I don't know which exact patch fixed this bug. ** Changed in: qemu Status: New => Fix Committed ** Changed in: qemu Status: Fix Committed => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/877155 Title: VT-D NIC doesn't work in windows guest Status in QEMU: Fix Released Bug description: [This should be a kvm kernel bug. I met this issue with quemu and kvm for virtualization. For https://bugzilla.kernel.org is not available yet, I just file this bug here just for tracking. ] Environment: Host OS (ia32/ia32e/IA64):All Guest OS (ia32/ia32e/IA64):ia32e Guest OS Type (Linux/Windows):Linux kvm.git Commit:211978fb6c666f824ae8221baea7ee28efc1 qemu.git Commit:edbb7c0de56692868e6126c7ff7e8bf917f8d0e7 Host Kernel Version:3.1.0-rc9+ Hardware: Westmere-EP && SandyBridge platform Bug detailed description: -- Using qemu and kvm upstream, VT-D NIC and SR-IOV VF don't work in Windows guest. I tried Windows XP, Vista, Win7, Win2k8 as a guest. And I used 82572 NIC and 82576 Virtual Function to assign to windows guest, and got the same result. Reproduce steps: 1.start a Windows guest with a NIC assigned: qemu-system-x86_64 -m 1024 -smp 2 -device pci-assign,host=08:00.0 -net none -hda Windows.img 2.check network in the Windows guest. (You'll find the assigned NIC doesn't work in guest.) Current result: NIC doesn't work in Windows guest Expected result: NIC will work. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/877155/+subscriptions
[Qemu-devel] [Bug 739785] Re: qemu-i386 user mode on ARMv5 host fails (bash: fork: Invalid argument)
@Ricardo I did not have such a problem and it ran all simple programs just fine (top, ls, cd, cp and so on) on my armv5t. Though I did not cross-compile, but natively compiled qemu on the box itself. It would be cool to have it working, but I don't think it will happen any time soon. I just bought eeepc 700 for $50 and run all x86 binary-only stuff on it as full-qemu is too heavy :) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/739785 Title: qemu-i386 user mode on ARMv5 host fails (bash: fork: Invalid argument) Status in QEMU: New Bug description: Good time of day everybody, I have been trying to make usermode qemu on ARM with plugapps (archlinux) with archlinux i386 chroot to work. 1. I installed arch linux in a virtuabox and created a chroot for it with mkarchroot. Transferred it to my pogo plug into /i386/ 2. I comiled qemu-i386 static and put it into /i386/usr/bin/ ./configure --static --disable-blobs --disable-system --target-list=i386-linux-user make 3. I also compiled linux kernel 2.6.38 with CONFIG_BINFMT_MISC=y and installed it. uname -a Linux Plugbox 2.6.38 #4 PREEMPT Fri Mar 18 22:19:10 CDT 2011 armv5tel Feroceon 88FR131 rev 1 (v5l) Marvell SheevaPlug Reference Board GNU/Linux 4. Added the following options into /etc/rc.local /sbin/modprobe binfmt_misc /bin/mount binfmt_misc -t binfmt_misc /proc/sys/fs/binfmt_misc echo ':qemu-i386:M::\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x03\x00:\xff\xff\xff\xff\xff\xfe\xfe\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfb\xff\xff\xff:/usr/bin/qemu-i386:' >/proc/sys/fs/binfmt_misc/register 5. Also copied ld-linux.so.3 (actually ld-2.13.so because ld- linux.so.3 is a link to that file) from /lib/ to /i386/lib/ 6.Now i chroot into /i386 and I get this: [root@Plugbox i386]# chroot . [II aI hnve ao n@P /]# pacman -Suy bash: fork: Invalid argument 7.I also downloaded linux-user-test-0.3 from qemu website and ran the test: [root@Plugbox linux-user-test-0.3]# make ./qemu-linux-user.sh [qemu-i386] ../qemu-0.14.0/i386-linux-user/qemu-i386 -L ./gnemul/qemu-i386 i386/ls -l dummyfile BUG IN DYNAMIC LINKER ld.so: dl-version.c: 210: _dl_check_map_versions: Assertion `needed != ((void *)0)' failed! make: *** [test] Error 127 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/739785/+subscriptions
Re: [Qemu-devel] [PATCH 10/12] trace: [tracetool] Automatically establish available backends and formats
On Wed, Mar 21, 2012 at 03:10:49PM +0100, Lluís Vilanova wrote: > Stefan Hajnoczi writes: > [...] > >> Maybe this would work nice for everybody: > >> > >> tracetool.py # main program (just parse cmdline opts and > >> call tracetool module) > >> tracetool/__init__.py # common boilerplate code (e.g., event > >> parsing and call dispatching) > >> tracetool/format/__init__.py # format auto-registration/lookup code > >> tracetool/format/h.py # code for the 'h' format > >> # __doc__ [mandatory, format > >> description] > >> # def begin(events) [optional] > >> # def end(events) [optional] > >> tracetool/backend/__init__.py # backend auto-registration/lookup code > >> tracetool/backend/simple.py # code for the 'simple' backend > >> # __doc__ [mandatory, backend > >> description] > >> # PUBLIC = [True|False] [optional, > >> # defaults to False, > >> # indicates it's > >> listed on --list-backends] > >> # def format(events) [optional, > >> # backend-specific code > >> for given format, > >> # implicitly indicates > >> format compatibility] > > > Let's call this function backend.generate(events) instead of "format" > > since we already use that term and it's a Python builtin too. This is > > a code generation function. > > Maybe I wasn't clear. I meant that for tracetool/backend/simple.py to work > with > the format 'h', it must have the following routine: > > def h (events): > ... > > That is, there must exist a routine with the same name of the format (with '-' > replaced with '_', when necessary) for the backend to be compatible with that > format. Ah, yes. That makes sense, I misunderstood. Stefan
Re: [Qemu-devel] tracetool: cast const types to avoid compile time warnings
On Wed, Mar 21, 2012 at 05:08:15PM +, Lee Essen wrote: > On Solaris/Illumos dtrace will remove the "const" qualifier from any > arguments when producing the header file, this results in hundreds > of compile-time warnings. I wonder why DTrace does this, it seems wrong. If anything it should be the other way around with all DTrace arguments becoming const :). Will you submit a patch to preserve const in upstream DTrace? > I have put together a patch to tracetool to cast any const argument > appropriately to remove the warnings, but I don't know how it > behaves on any other dtrace platform. If the "const" isn't removed > then this patch will actually create warnings. Why would it introduce warnings, maybe I'm misunderstanding? The following generated .h code should work without warnings whether the DTRACE_FOO() is const char* or char*: void trace_foo(const char *s) { DTRACE_FOO((char *)s); } > Do you see these warnings on any other dtrace platform? If not, > then I'll need to make sure the patch is solaris-specific. I haven't seen this issue on SystemTap. Stefan
Re: [Qemu-devel] [PATCH v2] test: remove qemu-ga reference
On Wed, Mar 21, 2012 at 10:10:46AM -0500, Michael Roth wrote: > This was added by mistake a while back. > > Signed-off-by: Michael Roth > --- > Makefile |1 + > tests/Makefile |2 +- > 2 files changed, 2 insertions(+), 1 deletions(-) Thanks, applied all 3 patches to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
[Qemu-devel] [PATCH] Remove PCI class code from virtio balloon device
Currently the virtio balloon device, when using the virtio-pci interface advertises itself with PCI class code MEMORY_RAM. This is wrong; the balloon is vaguely related to memory, but is nothing like a PCI memory device in the meaning of the class code, and this code is not required or suggested by the virtio PCI specification. Worse, this patch causes problems on the pseries machine, because the firmware, seeing this class code, advertises the device as memory in the device tree, and then a guest kernel bug causes it to see this "memory" before the real system memory, leading to a crash in early boot. This patch fixes the problem by removing the bogus PCI class code on the balloon device. The backwards compatibility PC machines get new compat properties so that they don't change. Cc: Michael S. Tsirkin Cc: Rusty Russell Signed-off-by: David Gibson --- hw/pc_piix.c| 24 hw/virtio-pci.c |7 ++- 2 files changed, 30 insertions(+), 1 deletions(-) diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 3f99f9a..72a4250 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_0 = { .driver = "isa-fdc", .property = "check_media_rate", .value= "off", +}, { +.driver = "virtio-balloon-pci", +.property = "class", +.value= stringify(PCI_CLASS_MEMORY_RAM), }, { /* end of list */ } }, @@ -449,6 +453,10 @@ static QEMUMachine pc_machine_v0_14 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +}, { +.driver = "virtio-balloon-pci", +.property = "class", +.value= stringify(PCI_CLASS_MEMORY_RAM), }, { /* end of list */ } }, @@ -505,6 +513,10 @@ static QEMUMachine pc_machine_v0_13 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +}, { +.driver = "virtio-balloon-pci", +.property = "class", +.value= stringify(PCI_CLASS_MEMORY_RAM), }, { /* end of list */ } }, @@ -565,6 +577,10 @@ static QEMUMachine pc_machine_v0_12 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +}, { +.driver = "virtio-balloon-pci", +.property = "class", +.value= stringify(PCI_CLASS_MEMORY_RAM), }, { /* end of list */ } } @@ -633,6 +649,10 @@ static QEMUMachine pc_machine_v0_11 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +}, { +.driver = "virtio-balloon-pci", +.property = "class", +.value= stringify(PCI_CLASS_MEMORY_RAM), }, { /* end of list */ } } @@ -713,6 +733,10 @@ static QEMUMachine pc_machine_v0_10 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +}, { +.driver = "virtio-balloon-pci", +.property = "class", +.value= stringify(PCI_CLASS_MEMORY_RAM), }, { /* end of list */ } }, diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index a0fb7c1..1fd5768 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -790,6 +790,10 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev) VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); VirtIODevice *vdev; +if (proxy->class_code != PCI_CLASS_OTHERS && +proxy->class_code != PCI_CLASS_MEMORY_RAM) /* qemu < 1.1 */ +proxy->class_code = PCI_CLASS_OTHERS; + vdev = virtio_balloon_init(&pci_dev->qdev); if (!vdev) { return -1; @@ -905,6 +909,7 @@ static TypeInfo virtio_serial_info = { }; static Property virtio_balloon_properties[] = { +DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, PCI_CLASS_OTHERS), DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), DEFINE_PROP_END_OF_LIST(), }; @@ -919,7 +924,7 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data) k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; k->device_id = PCI_DEVICE_ID_VIRTIO_BALLOON; k->revision = VIRTIO_PCI_ABI_VERSION; -k->class_id = PCI_CLASS_MEMORY_RAM; +k->class_id = PCI_CLASS_OTHERS; dc->reset = virtio_pci_reset; dc->props = virtio_balloon_properties; } -- 1.7.9.1
Re: [Qemu-devel] tracetool: cast const types to avoid compile time warnings
On 22/03/2012 08:28, Stefan Hajnoczi wrote: On Wed, Mar 21, 2012 at 05:08:15PM +, Lee Essen wrote: On Solaris/Illumos dtrace will remove the "const" qualifier from any arguments when producing the header file, this results in hundreds of compile-time warnings. I wonder why DTrace does this, it seems wrong. If anything it should be the other way around with all DTrace arguments becoming const :). Will you submit a patch to preserve const in upstream DTrace? I have asked the Illumos dev-list what the reasoning is. If it turns out to be a bug then I'll see what I can do about a fix, although it will be an illumos fix rather than a Solaris fix - so we'll still need a solution for generic Solaris. I have put together a patch to tracetool to cast any const argument appropriately to remove the warnings, but I don't know how it behaves on any other dtrace platform. If the "const" isn't removed then this patch will actually create warnings. Why would it introduce warnings, maybe I'm misunderstanding? The following generated .h code should work without warnings whether the DTRACE_FOO() is const char* or char*: void trace_foo(const char *s) { DTRACE_FOO((char *)s); } Hmmm ... yes, you're right, I wasn't thinking straight. This is good since it means we can use the same approach without breaking anything. I'll let you know what the illumos-dev response is. Cheers, Lee.
[Qemu-devel] Internal Snapshot disappear after write operation
Hi all, i am using *qcow2 internal snapshot *, but it seems they are not working fine Now to explain my scenario, I created qcow2 internal snapshot *(qcow2 VM is on). * Now i list the snapshot *-bash-4.1# qemu-img snapshot -l guestqcow2* Snapshot list: IDTAG VM SIZEDATE VM CLOCK 0 1970-01-01 09:00:00 00:00:00.000 0 1970-01-01 09:00:00 00:00:00.000 0 1970-01-01 09:00:00 00:00:00.000 0 1970-01-01 09:00:00 00:00:00.000 1 on3 0 2012-03-22 13:45:26 00:00:00.000 2 on4 0 2012-03-22 13:51:28 00:00:00.000 upto this point it is fine. But now when i write something on the vm using dd command Then * -bash-4.1# qemu-img snapshot -l guestqcow2* Snapshot list: IDTAG VM SIZEDATE VM CLOCK 0 1970-01-01 09:00:00 00:00:00.000 0 1970-01-01 09:00:00 00:00:00.000 0 1970-01-01 09:00:00 00:00:00.000 0 1970-01-01 09:00:00 00:00:00.000 0 1970-01-01 09:00:00 00:00:00.000 0 1970-01-01 09:00:00 00:00:00.000 all my snapshots are gone. This is not a correct behavior can any one explain why it is happening ? and how can i fix it ? -- *Pankaj Rawat*
[Qemu-devel] How To Boot Up Linux Kernel/Android on QEMU
Dear All, I have downloaded the QEMU sources from linaro and built it successfully. My PC architecture is X86, I downloaded the test image tarball from linaro, the image includes two images (zImage.integrator and arm_root.img). Then I use the following commands to boot them on QEMU successfully. # ./qemu-system-arm -kernel arm-test/zImage.integrator -initrd arm-test/arm_root.img But When I use the uImage I built instead of the test image (zImage.integrator), the kernel can not boot up, I don't know why. Please help me to create the right uImage with my linux kernel source. Another question, how can I boot up Android on QEMU? Best Regards, Stefan
Re: [Qemu-devel] [PATCH] use bdrv_aio functions in fdc.c
On Tue, Mar 20, 2012 at 9:56 PM, Stefan Hajnoczi wrote: > On Mon, Mar 19, 2012 at 05:17:15PM +0800, Li Zhi Hui wrote: >> @@ -1196,107 +1322,108 @@ static int fdctrl_transfer_handler (void *opaque, >> int nchan, >> return 0; >> } >> cur_drv = get_cur_drv(fdctrl); >> - if (fdctrl->data_dir == FD_DIR_SCANE || fdctrl->data_dir == >> FD_DIR_SCANL || >> - fdctrl->data_dir == FD_DIR_SCANH) >> + if ((fdctrl->data_dir == FD_DIR_SCANE) || >> + (fdctrl->data_dir == FD_DIR_SCANL) || >> + (fdctrl->data_dir == FD_DIR_SCANH)) { >> status2 = FD_SR2_SNS; >> - if (dma_len > fdctrl->data_len) >> + } >> + if (dma_len > fdctrl->data_len) { >> dma_len = fdctrl->data_len; >> + } >> if (cur_drv->bs == NULL) { >> - if (fdctrl->data_dir == FD_DIR_WRITE) >> - fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, >> 0x00, 0x00); >> - else >> + if (fdctrl->data_dir == FD_DIR_WRITE) { >> + fdctrl_stop_transfer(fdctrl, >> + FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00); >> + } else { >> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00); >> + } >> len = 0; >> goto transfer_error; >> } >> + >> + if ((fdctrl->data_dir != FD_DIR_WRITE) && (fdctrl->data_pos < dma_len)) >> { >> + fdc_sector_num = (dma_len + FD_SECTOR_LEN - 1) / FD_SECTOR_LEN; >> + opaque_cb = g_malloc0(sizeof(FDC_opaque)); > > I think we can only have 1 I/O pending at a time. Therefore it's > probably not necessary to create a separate struct, we can just pass the > FDrive/FDCtrl. > >> + qiov = g_malloc0(sizeof(QEMUIOVector)); > > This is leaked. I think it can be a field in opaque_cb, there's no need > to allocate this separately on the heap. > >> + pfdc_string = g_malloc0(fdc_sector_num * FD_SECTOR_LEN); > > Looks like this buffer is leaked. A block I/O buffer should be > allocated with qemu_blockalign() instead of g_malloc0() so that memory > alignment requirements for O_DIRECT image files are met. > > Would it be possible to use the fifo[] buffer instead of allocating a > new buffer? > >> + >> + qemu_iovec_init(qiov, 1); >> + qiov->iov->iov_base = pfdc_string; >> + qiov->iov->iov_len = fdc_sector_num * FD_SECTOR_LEN; >> + qiov->niov = 1; >> + qiov->size = fdc_sector_num * FD_SECTOR_LEN; > > The easiest way to do this is: > > iov.iov_base = fifo; > iov.iov_len = fdc_sector_num * FD_SECTOR_LEN; > qemu_iovec_init_external(qiov, iov, 1); > > We shouldn't duplicate the qiov->size calculation - that's already > provided by qemu_iovec_init_external() or qemu_iovec_add(). > >> + opaque_cb->fdctrl = fdctrl; >> + opaque_cb->qiov = qiov; >> + opaque_cb->nchan = nchan; >> + opaque_cb->dma_len = dma_len; >> + bdrv_aio_readv(cur_drv->bs, fd_sector(cur_drv), >> + qiov, fdc_sector_num, fdctrl_read_DMA_cb, opaque_cb); >> + return dma_len; > > We are returning dma_len but the I/O has not yet happened. The guest > could see that the DMA controller register has updated before we've > actually transferred data. This seems risky. > I think that fdctrl_stop_transfer() update the FDCtrl, so until then, the guest driver will see the update. > Have you checked what hw/dma.c does after we return? The DMA operation > has not completed yet so I wonder if it will call > fdctrl_transfer_handler() again from DMA_run()? > > Stefan > >
Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt
On Tue, Mar 13, 2012 at 11:53:19AM -0300, Eduardo Habkost wrote: > So, trying to summarize what was discussed in the call: > > On Mon, Mar 12, 2012 at 10:08:10AM -0300, Eduardo Habkost wrote: > > > Let's say we moved CPU definitions to /usr/share/qemu/cpu-models.xml. > > > > > > Obviously, we'd want a command line option to be able to change that > > > location so we'd introduce -cpu-models PATH. > > > > > > But we want all of our command line options to be settable by the > > > global configuration file so we would have a cpu-model=PATH to the > > > configuration file. > > > > > > But why hard code a path when we can just set the default path in the > > > configuration file so let's avoid hard coding and just put > > > cpu-models=/usr/share/qemu/cpu-models.xml in the default > > > configuration file. > > > > We wouldn't do the above. > > > > -nodefconfig should disable the loading of files on /etc, but it > > shouldn't disable loading internal non-configurable data that we just > > happened to choose to store outside the qemu binary because it makes > > development easier. > > The statement above is the one not fulfilled by the compromise solution: > -nodefconfig would really disable the loading of files on /usr/share. > What does this mean? Will -nodefconfig disable loading of bios.bin, option roms, keymaps? > > > > Really, the requirement of a "default configuration file" is a problem > > by itself. Qemu should not require a default configuration file to work, > > and it shouldn't require users to copy the default configuration file to > > change options from the default. > > The statement above is only partly true. The default configuration file > would be still needed, but if defaults are stored on /usr/share, I will > be happy with it. > > My main problem was with the need to _copy_ or edit a non-trivial > default config file. If the not-often-edited defaults/templates are > easily found on /usr/share to be used with -readconfig, I will be happy > with this solution, even if -nodefconfig disable the files on > /usr/share. > > > > > Doing this would make it impossible to deploy fixes to users if we evern > > find out that the default configuration file had a serious bug. What if > > a bug in our default configuration file has a serious security > > implication? > > The answer to this is: if the broken templates/defaults are on > /usr/share, it would be easy to deploy the fix. > > So, the compromise solution is: > > - We can move some configuration data (especially defaults/templates) > to /usr/share (machine-types and CPU models could go there). This > way we can easily deploy fixes to the defaults, if necessary. > - To reuse Qemu models, or machine-types, and not define everything from > scratch, libvirt will have to use something like: > "-nodefconfig -readconfig /usr/share/qemu/cpu-models-x86.conf" > cpu-models-x86.conf is not a configuration file. It is hardware description file. QEMU should not lose capability just because you run it with -nodefconfig. -nodefconfig means that QEMU does not create machine for you, but all parts needed to create a machine that would have been created without -nodefconfig are still present. Not been able to create Nehalem CPU after specifying -nodefconfig is the same as not been able to create virtio-net i.e the bug. > > (the item below is not something discussed on the call, just something I > want to add) > > To make this work better, we can allow users (humans or machines) to > "extend" CPU models on the config file, instead of having to define > everything from scratch. So, on /etc (or on a libvirt-generated config) > we could have something like: > > = > [cpu] > base_cpudef = Nehalem > add_features = "vmx" > = > > Then, as long as /usr/share/cpu-models-x86.conf is loaded, the user will > be able to reuse the Nehalem CPU model provided by Qemu. > And if it will not be loaded? > > > > > > > > But now when libvirt uses -nodefconfig, those models go away. > > > -nodefconfig means start QEMU in the most minimal state possible. > > > You get what you pay for if you use it. > > > > > > We'll have the same problem with machine configuration files. At > > > some point in time, -nodefconfig will make machine models disappear. > > > > It shouldn't. Machine-types are defaults to be used as base, they are > > not user-provided configuration. And the fact that we decided to store > > some data outside of the Qemu binary is orthogonal the design decisions > > in the Qemu command-line and configuration interface. > > So, this problem is solved if the defaults are easily found on > /usr/share. > What problem is solved and why are we mixing machine configuration files and cpu configuration files? They are different and should be treated differently. -nodefconfig exists only because there is not machine configuration files currently. With machine configuration files libvirt does not need -nodefconfig because it can c
Re: [Qemu-devel] How To Boot Up Linux Kernel/Android on QEMU
On 22 March 2012 06:24, stefan weids wrote: > I have downloaded the QEMU sources from linaro and built it successfully. > > My PC architecture is X86, I downloaded the test image tarball from linaro, > the image includes two images (zImage.integrator and arm_root.img). Then I > use the following commands to boot them on QEMU successfully. Incidentally I don't think you got this test image from Linaro: we don't distribute Integrator images or root tarballs... > # ./qemu-system-arm -kernel arm-test/zImage.integrator -initrd > arm-test/arm_root.img > > But When I use the uImage I built instead of the test image > (zImage.integrator), the kernel can not boot up, I don't know why. Please > help me to create the right uImage with my linux kernel source. The most common problem is building an image for one type of board (eg "versatile express") and trying to run it on a QEMU model of a different board (eg "integrator"). ARM is not like x86: every board is different and typically you must compile the kernel specifically for the board you want to run on. The default for qemu-system-arm is the integrator board but this is rarely what you want because it is incredibly ancient. My other piece of advice is to get an image which runs on the real hardware and work from there. A lot of problems that ARM qemu newbies have seem to be simple kernel configuration errors which would be just as much of a problem on the hardware. -- PMM
[Qemu-devel] [PATCH] Add bootindex support to usb-host and usb-redir
When passing through a usb pendrive seabios will present it in the F12 boot menu and will happily boot from it. This patch adds bootorder support so you can even make it the default boot device. Signed-off-by: Gerd Hoffmann --- hw/usb/host-linux.c |3 +++ hw/usb/redirect.c |3 +++ 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c index 72f0306..f3d5a62 100644 --- a/hw/usb/host-linux.c +++ b/hw/usb/host-linux.c @@ -117,6 +117,7 @@ typedef struct USBHostDevice { int addr; char port[MAX_PORTLEN]; struct USBAutoFilter match; +int32_t bootindex; int seen, errcount; QTAILQ_ENTRY(USBHostDevice) next; @@ -1420,6 +1421,7 @@ static int usb_host_initfn(USBDevice *dev) if (s->match.bus_num != 0 && s->match.port != NULL) { usb_host_claim_port(s); } +add_boot_device_path(s->bootindex, &dev->qdev, NULL); return 0; } @@ -1435,6 +1437,7 @@ static Property usb_host_dev_properties[] = { DEFINE_PROP_HEX32("vendorid", USBHostDevice, match.vendor_id, 0), DEFINE_PROP_HEX32("productid", USBHostDevice, match.product_id, 0), DEFINE_PROP_UINT32("isobufs", USBHostDevice, iso_urb_count,4), +DEFINE_PROP_INT32("bootindex", USBHostDevice, bootindex,-1), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index 8e9f175..4288324 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -74,6 +74,7 @@ struct USBRedirDevice { CharDriverState *cs; uint8_t debug; char *filter_str; +int32_t bootindex; /* Data passed from chardev the fd_read cb to the usbredirparser read cb */ const uint8_t *read_buf; int read_buf_size; @@ -923,6 +924,7 @@ static int usbredir_initfn(USBDevice *udev) qemu_chr_add_handlers(dev->cs, usbredir_chardev_can_read, usbredir_chardev_read, usbredir_chardev_event, dev); +add_boot_device_path(dev->bootindex, &udev->qdev, NULL); return 0; } @@ -1452,6 +1454,7 @@ static Property usbredir_properties[] = { DEFINE_PROP_CHR("chardev", USBRedirDevice, cs), DEFINE_PROP_UINT8("debug", USBRedirDevice, debug, 0), DEFINE_PROP_STRING("filter", USBRedirDevice, filter_str), +DEFINE_PROP_INT32("bootindex", USBRedirDevice, bootindex, -1), DEFINE_PROP_END_OF_LIST(), }; -- 1.7.1
Re: [Qemu-devel] [PATCH 03/13] usb-xhci: Use PCI DMA helper functions
Hi, > -#include "hw/qdev-addr.h" > +//#include "hw/qdev-addr.h" Minor nit: Just zap it instead of leaving it in commented out. Looks fine otherwise. Acked-by: Gerd Hoffmann cheers, Gerd
Re: [Qemu-devel] [PATCH] Remove PCI class code from virtio balloon device
On Thu, Mar 22, 2012 at 9:09 AM, David Gibson wrote: > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 3f99f9a..72a4250 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c > @@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_0 = { > .driver = "isa-fdc", > .property = "check_media_rate", > .value = "off", > + }, { > + .driver = "virtio-balloon-pci", > + .property = "class", > + .value = stringify(PCI_CLASS_MEMORY_RAM), > }, > { /* end of list */ } > }, > @@ -449,6 +453,10 @@ static QEMUMachine pc_machine_v0_14 = { pc_machine_v0_15 should also use PCI_CLASS_MEMORY_RAM? Did you skip it on purpose? > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index a0fb7c1..1fd5768 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -790,6 +790,10 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev) > VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); > VirtIODevice *vdev; > > + if (proxy->class_code != PCI_CLASS_OTHERS && > + proxy->class_code != PCI_CLASS_MEMORY_RAM) /* qemu < 1.1 */ > + proxy->class_code = PCI_CLASS_OTHERS; > + Why is this hunk is needed?
Re: [Qemu-devel] [PATCH 06/13] usb-ohci: Use universal DMA helper functions
On 03/22/12 03:14, David Gibson wrote: > This patch converts it to use the new universal DMA helper functions. > In the PCI case, it obtains its DMAContext from pci_dma_context(), in > the SysBus case, it uses NULL - i.e. assumes for now that there will > be no IOMMU translation for a SysBus OHCI. Acked-by: Gerd Hoffmann cheers, Gerd
Re: [Qemu-devel] [Spice-devel] [PATCH] ui/spice-display: use uintptr_t when casting qxl physical addresses
Good one, ACK. Acked-by: Hans de Goede On 03/21/2012 05:17 PM, Alon Levy wrote: The current intptr_t casts are a problem when the address's highest bit is 1, and it is cast to a intptr_t and then to uint64_t, such as at: surface.mem= (intptr_t)ssd->buf; This causes the sign bit to be extended which causes a wrong address to be passed on to spice, which then complains when it gets the wrong slot_id number, since the slot_id is taken from the higher bits. The assertion happens early - during the first primary surface creation. This fixes running "-vga qxl -spice" with 32 bit compiled qemu-system-i386. Signed-off-by: Alon Levy --- ui/spice-display.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ui/spice-display.c b/ui/spice-display.c index 35499e2..f5764e9 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -158,7 +158,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) drawable->bbox= ssd->dirty; drawable->clip.type = SPICE_CLIP_TYPE_NONE; drawable->effect = QXL_EFFECT_OPAQUE; -drawable->release_info.id = (intptr_t)update; +drawable->release_info.id = (uintptr_t)update; drawable->type= QXL_DRAW_COPY; drawable->surfaces_dest[0] = -1; drawable->surfaces_dest[1] = -1; @@ -169,7 +169,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) + time_space.tv_nsec / 1000 / 1000; drawable->u.copy.rop_descriptor = SPICE_ROPD_OP_PUT; -drawable->u.copy.src_bitmap = (intptr_t)image; +drawable->u.copy.src_bitmap = (uintptr_t)image; drawable->u.copy.src_area.right = bw; drawable->u.copy.src_area.bottom = bh; @@ -179,7 +179,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) image->bitmap.stride = bw * 4; image->descriptor.width = image->bitmap.x = bw; image->descriptor.height = image->bitmap.y = bh; -image->bitmap.data = (intptr_t)(update->bitmap); +image->bitmap.data = (uintptr_t)(update->bitmap); image->bitmap.palette = 0; image->bitmap.format = SPICE_BITMAP_FMT_32BIT; @@ -200,7 +200,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) } cmd->type = QXL_CMD_DRAW; -cmd->data = (intptr_t)drawable; +cmd->data = (uintptr_t)drawable; memset(&ssd->dirty, 0, sizeof(ssd->dirty)); return update; @@ -244,7 +244,7 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd) surface.mouse_mode = true; surface.flags = 0; surface.type = 0; -surface.mem= (intptr_t)ssd->buf; +surface.mem= (uintptr_t)ssd->buf; surface.group_id = MEMSLOT_GROUP_HOST; qemu_spice_create_primary_surface(ssd, 0,&surface, QXL_SYNC);
[Qemu-devel] [PATCH 1/7] e1000: introduce bits of PHY control register
This would be used be following patches. Signed-off-by: Jason Wang --- hw/e1000_hw.h | 12 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/hw/e1000_hw.h b/hw/e1000_hw.h index 9e29af8..c9cb79e 100644 --- a/hw/e1000_hw.h +++ b/hw/e1000_hw.h @@ -349,6 +349,18 @@ #define M88E1000_PHY_VCO_REG_BIT8 0x100 /* Bits 8 & 11 are adjusted for */ #define M88E1000_PHY_VCO_REG_BIT11 0x800/* improved BER performance */ +/* PHY Control Register */ +#define MII_CR_SPEED_SELECT_MSB 0x0040 /* bits 6,13: 10=1000, 01=100, 00=10 */ +#define MII_CR_COLL_TEST_ENABLE 0x0080 /* Collision test enable */ +#define MII_CR_FULL_DUPLEX 0x0100 /* FDX =1, half duplex =0 */ +#define MII_CR_RESTART_AUTO_NEG 0x0200 /* Restart auto negotiation */ +#define MII_CR_ISOLATE 0x0400 /* Isolate PHY from MII */ +#define MII_CR_POWER_DOWN 0x0800 /* Power down */ +#define MII_CR_AUTO_NEG_EN 0x1000 /* Auto Neg Enable */ +#define MII_CR_SPEED_SELECT_LSB 0x2000 /* bits 6,13: 10=1000, 01=100, 00=10 */ +#define MII_CR_LOOPBACK 0x4000 /* 0 = normal, 1 = loopback */ +#define MII_CR_RESET0x8000 /* 0 = normal, 1 = PHY reset */ + /* PHY Status Register */ #define MII_SR_EXTENDED_CAPS 0x0001/* Extended register capabilities */ #define MII_SR_JABBER_DETECT 0x0002/* Jabber Detected */
[Qemu-devel] [PATCH 2/7] e1000: conditionally raise irq at the end of MDI cycle
According to the spec: "When set to 1b by software, it causes an Interrupt to be asserted to indicate the end of an MDI cycle." We need check the Interrupt Enable bit and raise irq only when it is set. Signed-off-by: Jason Wang --- hw/e1000.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/hw/e1000.c b/hw/e1000.c index 7babc0b..dd6a97d 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -234,7 +234,10 @@ set_mdic(E1000State *s, int index, uint32_t val) s->phy_reg[addr] = data; } s->mac_reg[MDIC] = val | E1000_MDIC_READY; -set_ics(s, 0, E1000_ICR_MDAC); + +if (val & E1000_MDIC_INT_EN) { +set_ics(s, 0, E1000_ICR_MDAC); +} } static uint32_t
[Qemu-devel] [PATCH 3/7] e1000: PHY loopback mode support
The missing of loopback mode prevent the running of self diagnosis program in guest. This patch adds this support. After this patch, loopback test of ethtool were passed in guest. Signed-off-by: Jason Wang --- hw/e1000.c | 14 -- 1 files changed, 12 insertions(+), 2 deletions(-) diff --git a/hw/e1000.c b/hw/e1000.c index dd6a97d..bc26a0c 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -352,6 +352,16 @@ fcs_len(E1000State *s) } static void +e1000_send_packet(E1000State *s, const uint8_t *buf, int size) +{ +if (s->phy_reg[PHY_CTRL] & MII_CR_LOOPBACK) { +s->nic->nc.info->receive(&s->nic->nc, buf, size); +} else { +qemu_send_packet(&s->nic->nc, buf, size); +} +} + +static void xmit_seg(E1000State *s) { uint16_t len, *sp; @@ -400,9 +410,9 @@ xmit_seg(E1000State *s) memmove(tp->vlan, tp->data, 4); memmove(tp->data, tp->data + 4, 8); memcpy(tp->data + 8, tp->vlan_header, 4); -qemu_send_packet(&s->nic->nc, tp->vlan, tp->size + 4); +e1000_send_packet(s, tp->vlan, tp->size + 4); } else -qemu_send_packet(&s->nic->nc, tp->data, tp->size); +e1000_send_packet(s, tp->data, tp->size); s->mac_reg[TPT]++; s->mac_reg[GPTC]++; n = s->mac_reg[TOTL];
Re: [Qemu-devel] [PATCH] use bdrv_aio functions in fdc.c
On Thu, Mar 22, 2012 at 7:06 AM, liu ping fan wrote: > On Tue, Mar 20, 2012 at 9:56 PM, Stefan Hajnoczi > wrote: >> On Mon, Mar 19, 2012 at 05:17:15PM +0800, Li Zhi Hui wrote: >>> @@ -1196,107 +1322,108 @@ static int fdctrl_transfer_handler (void *opaque, >>> int nchan, >>> return 0; >>> } >>> cur_drv = get_cur_drv(fdctrl); >>> - if (fdctrl->data_dir == FD_DIR_SCANE || fdctrl->data_dir == >>> FD_DIR_SCANL || >>> - fdctrl->data_dir == FD_DIR_SCANH) >>> + if ((fdctrl->data_dir == FD_DIR_SCANE) || >>> + (fdctrl->data_dir == FD_DIR_SCANL) || >>> + (fdctrl->data_dir == FD_DIR_SCANH)) { >>> status2 = FD_SR2_SNS; >>> - if (dma_len > fdctrl->data_len) >>> + } >>> + if (dma_len > fdctrl->data_len) { >>> dma_len = fdctrl->data_len; >>> + } >>> if (cur_drv->bs == NULL) { >>> - if (fdctrl->data_dir == FD_DIR_WRITE) >>> - fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM | FD_SR0_SEEK, >>> 0x00, 0x00); >>> - else >>> + if (fdctrl->data_dir == FD_DIR_WRITE) { >>> + fdctrl_stop_transfer(fdctrl, >>> + FD_SR0_ABNTERM | FD_SR0_SEEK, 0x00, 0x00); >>> + } else { >>> fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00); >>> + } >>> len = 0; >>> goto transfer_error; >>> } >>> + >>> + if ((fdctrl->data_dir != FD_DIR_WRITE) && (fdctrl->data_pos < >>> dma_len)) { >>> + fdc_sector_num = (dma_len + FD_SECTOR_LEN - 1) / FD_SECTOR_LEN; >>> + opaque_cb = g_malloc0(sizeof(FDC_opaque)); >> >> I think we can only have 1 I/O pending at a time. Therefore it's >> probably not necessary to create a separate struct, we can just pass the >> FDrive/FDCtrl. >> >>> + qiov = g_malloc0(sizeof(QEMUIOVector)); >> >> This is leaked. I think it can be a field in opaque_cb, there's no need >> to allocate this separately on the heap. >> >>> + pfdc_string = g_malloc0(fdc_sector_num * FD_SECTOR_LEN); >> >> Looks like this buffer is leaked. A block I/O buffer should be >> allocated with qemu_blockalign() instead of g_malloc0() so that memory >> alignment requirements for O_DIRECT image files are met. >> >> Would it be possible to use the fifo[] buffer instead of allocating a >> new buffer? >> >>> + >>> + qemu_iovec_init(qiov, 1); >>> + qiov->iov->iov_base = pfdc_string; >>> + qiov->iov->iov_len = fdc_sector_num * FD_SECTOR_LEN; >>> + qiov->niov = 1; >>> + qiov->size = fdc_sector_num * FD_SECTOR_LEN; >> >> The easiest way to do this is: >> >> iov.iov_base = fifo; >> iov.iov_len = fdc_sector_num * FD_SECTOR_LEN; >> qemu_iovec_init_external(qiov, iov, 1); >> >> We shouldn't duplicate the qiov->size calculation - that's already >> provided by qemu_iovec_init_external() or qemu_iovec_add(). >> >>> + opaque_cb->fdctrl = fdctrl; >>> + opaque_cb->qiov = qiov; >>> + opaque_cb->nchan = nchan; >>> + opaque_cb->dma_len = dma_len; >>> + bdrv_aio_readv(cur_drv->bs, fd_sector(cur_drv), >>> + qiov, fdc_sector_num, fdctrl_read_DMA_cb, opaque_cb); >>> + return dma_len; >> >> We are returning dma_len but the I/O has not yet happened. The guest >> could see that the DMA controller register has updated before we've >> actually transferred data. This seems risky. >> > I think that fdctrl_stop_transfer() update the FDCtrl, so until then, > the guest driver will see the update. You're referring to the fdc device state but I meant the ISA DMA controller registers. hw/dma.c:read_chan() will expose the incremented value to the guest before we've transferred any data. I think we shouldn't increment the DMA transfer byte count before it has completed. Stefan
[Qemu-devel] [PATCH 5/7] e1000: introduce bit for debugging PHY emulation
Signed-off-by: Jason Wang --- hw/e1000.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/e1000.c b/hw/e1000.c index fc30361..4eb95be 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -42,7 +42,7 @@ enum { DEBUG_GENERAL, DEBUG_IO, DEBUG_MMIO, DEBUG_INTERRUPT, DEBUG_RX, DEBUG_TX, DEBUG_MDIC, DEBUG_EEPROM, DEBUG_UNKNOWN, DEBUG_TXSUM,DEBUG_TXERR,DEBUG_RXERR, -DEBUG_RXFILTER,DEBUG_NOTYET, +DEBUG_RXFILTER, DEBUG_PHY, DEBUG_NOTYET, }; #define DBGBIT(x) (1<
[Qemu-devel] [PATCH 4/7] e1000: introduce helpers to manipulate link status
This patch introduces helpers to change link status bit for phy/mac register. This would help to reduce code duplication and would be used by following patches. Signed-off-by: Jason Wang --- hw/e1000.c | 23 +-- 1 files changed, 17 insertions(+), 6 deletions(-) diff --git a/hw/e1000.c b/hw/e1000.c index bc26a0c..fc30361 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -142,6 +142,20 @@ enum { defreg(VET), }; +static void +e1000_link_down(E1000State *s) +{ +s->mac_reg[STATUS] &= ~E1000_STATUS_LU; +s->phy_reg[PHY_STATUS] &= ~MII_SR_LINK_STATUS; +} + +static void +e1000_link_up(E1000State *s) +{ +s->mac_reg[STATUS] |= E1000_STATUS_LU; +s->phy_reg[PHY_STATUS] |= MII_SR_LINK_STATUS; +} + enum { PHY_R = 1, PHY_W = 2, PHY_RW = PHY_R | PHY_W }; static const char phy_regcap[0x20] = { [PHY_STATUS] = PHY_R, [M88E1000_EXT_PHY_SPEC_CTRL] = PHY_RW, @@ -635,11 +649,9 @@ e1000_set_link_status(VLANClientState *nc) uint32_t old_status = s->mac_reg[STATUS]; if (nc->link_down) { -s->mac_reg[STATUS] &= ~E1000_STATUS_LU; -s->phy_reg[PHY_STATUS] &= ~MII_SR_LINK_STATUS; +e1000_link_down(s); } else { -s->mac_reg[STATUS] |= E1000_STATUS_LU; -s->phy_reg[PHY_STATUS] |= MII_SR_LINK_STATUS; +e1000_link_up(s); } if (s->mac_reg[STATUS] != old_status) @@ -1148,8 +1160,7 @@ static void e1000_reset(void *opaque) memset(&d->tx, 0, sizeof d->tx); if (d->nic->nc.link_down) { -d->mac_reg[STATUS] &= ~E1000_STATUS_LU; -d->phy_reg[PHY_STATUS] &= ~MII_SR_LINK_STATUS; +e1000_link_down(d); } }
[Qemu-devel] [PATCH 6/7] e1000: link auto-negotiation emulation
Indeed, there's nothing else except for the time spent on the negotiation needs to be emulated. This is needed for resuming windows guest from hibernation, as without a proper delay, qemu would send the packet too early ( guest even does not have a proper intr handler), which could lead windows guest hang. This patch first introduces an array of function pointers to make it possible to emulate per-register write behavior. Then traps the PHY_CTRL register write and when guest want to restart the link auto negotiation, we would down the link and mark the auto negotiation in progress in PHY_STATUS register. After time, a timer with 500 ms ( which is the minimum timeout of auto-negotation specified in 802.3 spec). The link would be up when timer expired. Test with resuming windows guest plus flood ping and linux ethtool linkstatus test. Signed-off-by: Jason Wang --- hw/e1000.c | 45 +++-- 1 files changed, 43 insertions(+), 2 deletions(-) diff --git a/hw/e1000.c b/hw/e1000.c index 4eb95be..921f0cc 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -125,6 +125,8 @@ typedef struct E1000State_st { uint16_t reading; uint32_t old_eecd; } eecd_state; + +QEMUTimer *autoneg_timer; } E1000State; #definedefreg(x) x = (E1000_##x>>2) @@ -156,6 +158,34 @@ e1000_link_up(E1000State *s) s->phy_reg[PHY_STATUS] |= MII_SR_LINK_STATUS; } +static void +set_phy_ctrl(E1000State *s, int index, uint16_t val) +{ +if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) { +s->nic->nc.link_down = true; +e1000_link_down(s); +s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE; +DBGOUT(PHY, "Start link auto negotiation\n"); +qemu_mod_timer(s->autoneg_timer, qemu_get_clock_ms(vm_clock) + 500); +} +} + +static void +e1000_autoneg_timer(void *opaque) +{ +E1000State *s = opaque; +s->nic->nc.link_down = false; +e1000_link_up(s); +s->phy_reg[PHY_STATUS] |= MII_SR_AUTONEG_COMPLETE; +DBGOUT(PHY, "Auto negotiation is completed\n"); +} + +static void (*phyreg_writeops[])(E1000State *, int, uint16_t) = { +[PHY_CTRL] = set_phy_ctrl, +}; + +enum { NPHYWRITEOPS = ARRAY_SIZE(phyreg_writeops) }; + enum { PHY_R = 1, PHY_W = 2, PHY_RW = PHY_R | PHY_W }; static const char phy_regcap[0x20] = { [PHY_STATUS] = PHY_R, [M88E1000_EXT_PHY_SPEC_CTRL] = PHY_RW, @@ -244,8 +274,12 @@ set_mdic(E1000State *s, int index, uint32_t val) if (!(phy_regcap[addr] & PHY_W)) { DBGOUT(MDIC, "MDIC write reg %x unhandled\n", addr); val |= E1000_MDIC_ERROR; -} else +} else { +if (addr < NPHYWRITEOPS && phyreg_writeops[addr]) { +phyreg_writeops[addr](s, index, data); +} s->phy_reg[addr] = data; +} } s->mac_reg[MDIC] = val | E1000_MDIC_READY; @@ -926,6 +960,7 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = { [MTA ... MTA+127] = &mac_writereg, [VFTA ... VFTA+127] = &mac_writereg, }; + enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) }; static void @@ -1087,7 +1122,8 @@ static const uint16_t e1000_eeprom_template[64] = { }; static const uint16_t phy_reg_init[] = { -[PHY_CTRL] = 0x1140, [PHY_STATUS] = 0x796d, // link initially up +[PHY_CTRL] = 0x1140, +[PHY_STATUS] = 0x794d, /* link initially up with not completed autoneg */ [PHY_ID1] = 0x141, [PHY_ID2] = PHY_ID2_INIT, [PHY_1000T_CTRL] = 0x0e00, [M88E1000_PHY_SPEC_CTRL] = 0x360, [M88E1000_EXT_PHY_SPEC_CTRL] = 0x0d60, [PHY_AUTONEG_ADV] = 0xde1, @@ -1142,6 +1178,8 @@ pci_e1000_uninit(PCIDevice *dev) { E1000State *d = DO_UPCAST(E1000State, dev, dev); +qemu_del_timer(d->autoneg_timer); +qemu_free_timer(d->autoneg_timer); memory_region_destroy(&d->mmio); memory_region_destroy(&d->io); qemu_del_vlan_client(&d->nic->nc); @@ -1152,6 +1190,7 @@ static void e1000_reset(void *opaque) { E1000State *d = opaque; +qemu_del_timer(d->autoneg_timer); memset(d->phy_reg, 0, sizeof d->phy_reg); memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init); memset(d->mac_reg, 0, sizeof d->mac_reg); @@ -1212,6 +1251,8 @@ static int pci_e1000_init(PCIDevice *pci_dev) add_boot_device_path(d->conf.bootindex, &pci_dev->qdev, "/ethernet-phy@0"); +d->autoneg_timer = qemu_new_timer_ms(vm_clock, e1000_autoneg_timer, d); + return 0; }
[Qemu-devel] [PATCH 7/7] e1000: set E1000_ICR_INT_ASSERTED only for 8257x
E1000_ICR_INT_ASSERTED were introduced only for 8257x, so we need to check the E1000_DEVID before setting this bit in ICS. Signed-off-by: Jason Wang --- hw/e1000.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/hw/e1000.c b/hw/e1000.c index 921f0cc..5584cc6 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -199,8 +199,10 @@ static const char phy_regcap[0x20] = { static void set_interrupt_cause(E1000State *s, int index, uint32_t val) { -if (val) +if (val && (E1000_DEVID >= E1000_DEV_ID_82547EI_MOBILE)) { +/* Only for 8257x */ val |= E1000_ICR_INT_ASSERTED; +} s->mac_reg[ICR] = val; s->mac_reg[ICS] = val; qemu_set_irq(s->dev.irq[0], (s->mac_reg[IMS] & s->mac_reg[ICR]) != 0);
Re: [Qemu-devel] [Qemu-trivial] [PATCH 0/4] More whitespace and coding style clean ups ahead of future changes
On Thu, Mar 22, 2012 at 12:05 AM, Paolo Bonzini wrote: > Il 21/03/2012 22:02, Peter Portante ha scritto: >> Hi Folks, >> >> Please forgive me if you find these changes are annoying, as I am trying to >> learn the ropes of patch submission with git ahead of making a real patch. >> >> While working on the code, I found that scripts/checkpatch.pl will flag lines >> that I am changing as not adhereing to the codeing standard due to >> pre-existing coding violations. So I figured I could learn a bit about how to >> submit patches by fixing these files I will be touching before submitting the >> code changes. > > I think slirp is a mess and it's not worth doing this kind of sweeping > change. Just ignore checkpatch output for slirp. Please resend without slirp changes. I suggest including the individual block/raw-posix.c patch you sent in the new series too. > For the others, the changes are pretty small so they could be doable. > If you want to change them then please also add braces around if statements. If you're doing a whitespace patch please include the 3-space indentation issues that checkpatch.pl reports in block/raw-posix.c. > In any case, your patch submission workflow seems to be fine. :P Please use a prefix in your commit messages, for example "raw-posix: whitespace cleanup". This gives each commit a unique, useful message which communicates more than "Yet another whitespace cleanup"-style messages. Stefan
Re: [Qemu-devel] [PATCH] Remove PCI class code from virtio balloon device
>> +if (proxy->class_code != PCI_CLASS_OTHERS && >> +proxy->class_code != PCI_CLASS_MEMORY_RAM) /* qemu < 1.1 */ >> +proxy->class_code = PCI_CLASS_OTHERS; >> + > > Why is this hunk is needed? Catch users doing -device virtio-balloon,class=42 cheers, Gerd
Re: [Qemu-devel] [PATCH v4 5/7] RTC:Add RTC update-ended interrupt support
On Thu, 22 Mar 2012, Zhang, Yang Z wrote: > > -Original Message- > > From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com] > > Sent: Wednesday, March 21, 2012 2:04 AM > > > > On Mon, 19 Mar 2012, Zhang, Yang Z wrote: > > > Use a timer to emulate update cycle. When update cycle ended and UIE is > > setting, then raise an interrupt. The timer runs only when UF or AF is > > cleared. > > > > The idea is that if the user requests the update-ended interrupt (UIE) > > we setup a timer to inject it at the right time into the guest and > > another timer to update the UIP bit, correct? > No, the timer runs whenever the UF and AF is cleared, not only UIE or AIE is > set. > > > But do we actually need the second timer? Why can't we just update the > > UIP bit whenever the user tries to read reg A, as we do when UIE is not set? > The purpose of using two timer is trying to keep the UF, AF and UIP > synchronous. User can poll UIP to check UF and AF bit. If we use timer for > UF/AF bit track and check UIP by another way, since the timer will be fired > with delay, then the problem is encountered: the UIP is cleared, but due to > the delay of timer, the UF/AF bit is not set. So we need to check them on a > same level. Although we can update UF/AF when reading it, the logic is too > complicated, especially for AF bit. fair enough
Re: [Qemu-devel] [PATCH v4 5/7] RTC:Add RTC update-ended interrupt support
On Wed, 21 Mar 2012, Paolo Bonzini wrote: > Il 21/03/2012 17:54, Stefano Stabellini ha scritto: > >> > > >> > No, you need to set UF in case the code observes it without actually > >> > enabling interrupt delivery on the ISA bus. > > > > Well, if it is just about updating UF, can we do it only when the user > > reads REG_C? > > Perhaps, but for AF it is much harder. Note that if the guest does not > care, this will run just once. Right, probably not worth the effort.
Re: [Qemu-devel] [PATCH v4 5/7] RTC:Add RTC update-ended interrupt support
Il 22/03/2012 11:29, Stefano Stabellini ha scritto: >> The purpose of using two timer is trying to keep the UF, AF and UIP >> synchronous. User can poll UIP to check UF and AF bit. If we use >> timer for UF/AF bit track and check UIP by another way, since the >> timer will be fired with delay, then the problem is encountered: >> the UIP is cleared, but due to the delay of timer, the UF/AF bit is >> not set. So we need to check them on a same level. Although we can >> update UF/AF when reading it, the logic is too complicated, >> especially for AF bit. FWIW, the solution I used when I removed the second timer from the current code (not yet posted, I do hope your patches can be fixed!) was to set UIP when reading A, and clear it in the timer. You have something like that, but keep the two timers. Paolo
Re: [Qemu-devel] [PATCH] Remove PCI class code from virtio balloon device
On Thu, Mar 22, 2012 at 10:27 AM, Gerd Hoffmann wrote: >>> + if (proxy->class_code != PCI_CLASS_OTHERS && >>> + proxy->class_code != PCI_CLASS_MEMORY_RAM) /* qemu < 1.1 */ >>> + proxy->class_code = PCI_CLASS_OTHERS; >>> + >> >> Why is this hunk is needed? > > Catch users doing -device virtio-balloon,class=42 I see that the other virtio devices that have a class property also do this. Sorry, my bad. Stefan
Re: [Qemu-devel] [PATCH] Add bootindex support to usb-host and usb-redir
Il 22/03/2012 10:52, Gerd Hoffmann ha scritto: > When passing through a usb pendrive seabios will present it in the F12 > boot menu and will happily boot from it. > > This patch adds bootorder support so you can even make it the default > boot device. > > Signed-off-by: Gerd Hoffmann > --- > hw/usb/host-linux.c |3 +++ > hw/usb/redirect.c |3 +++ > 2 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c > index 72f0306..f3d5a62 100644 > --- a/hw/usb/host-linux.c > +++ b/hw/usb/host-linux.c > @@ -117,6 +117,7 @@ typedef struct USBHostDevice { > int addr; > char port[MAX_PORTLEN]; > struct USBAutoFilter match; > +int32_t bootindex; > int seen, errcount; > > QTAILQ_ENTRY(USBHostDevice) next; > @@ -1420,6 +1421,7 @@ static int usb_host_initfn(USBDevice *dev) > if (s->match.bus_num != 0 && s->match.port != NULL) { > usb_host_claim_port(s); > } > +add_boot_device_path(s->bootindex, &dev->qdev, NULL); > return 0; > } > > @@ -1435,6 +1437,7 @@ static Property usb_host_dev_properties[] = { > DEFINE_PROP_HEX32("vendorid", USBHostDevice, match.vendor_id, 0), > DEFINE_PROP_HEX32("productid", USBHostDevice, match.product_id, 0), > DEFINE_PROP_UINT32("isobufs", USBHostDevice, iso_urb_count,4), > +DEFINE_PROP_INT32("bootindex", USBHostDevice, bootindex,-1), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c > index 8e9f175..4288324 100644 > --- a/hw/usb/redirect.c > +++ b/hw/usb/redirect.c > @@ -74,6 +74,7 @@ struct USBRedirDevice { > CharDriverState *cs; > uint8_t debug; > char *filter_str; > +int32_t bootindex; > /* Data passed from chardev the fd_read cb to the usbredirparser read cb > */ > const uint8_t *read_buf; > int read_buf_size; > @@ -923,6 +924,7 @@ static int usbredir_initfn(USBDevice *udev) > qemu_chr_add_handlers(dev->cs, usbredir_chardev_can_read, >usbredir_chardev_read, usbredir_chardev_event, > dev); > > +add_boot_device_path(dev->bootindex, &udev->qdev, NULL); > return 0; > } > > @@ -1452,6 +1454,7 @@ static Property usbredir_properties[] = { > DEFINE_PROP_CHR("chardev", USBRedirDevice, cs), > DEFINE_PROP_UINT8("debug", USBRedirDevice, debug, 0), > DEFINE_PROP_STRING("filter", USBRedirDevice, filter_str), > +DEFINE_PROP_INT32("bootindex", USBRedirDevice, bootindex, -1), > DEFINE_PROP_END_OF_LIST(), > }; > That was fast. :) Looks good, thanks! Paolo
Re: [Qemu-devel] [PATCH v3 1/2] Force timedrift=none on previous machines
On Wed, 21 Mar 2012, Paolo Bonzini wrote: > Il 21/03/2012 17:06, Crístian Viana ha scritto: > > The current value for the -rtc timedrift option is none. This patch > > makes sure that the old machines configuration will work the same way > > even after that option changes its default value. > > > > Signed-off-by: Crístian Viana > > --- > > hw/pc_piix.c | 39 +++ > > 1 files changed, 39 insertions(+), 0 deletions(-) > > > > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > > index 3f99f9a..08255b5 100644 > > --- a/hw/pc_piix.c > > +++ b/hw/pc_piix.c > > @@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_0 = { > > .driver = "isa-fdc", > > .property = "check_media_rate", > > .value= "off", > > +}, { > > +.driver = "mc146818rtc", > > +.property = "lost_tick_policy", > > +.value= "none", > > }, > > { /* end of list */ } > > }, > > @@ -405,6 +409,10 @@ static QEMUMachine pc_machine_v0_15 = { > > .driver = "isa-fdc", > > .property = "check_media_rate", > > .value= "off", > > +}, { > > +.driver = "mc146818rtc", > > +.property = "lost_tick_policy", > > +.value= "none", > > }, > > { /* end of list */ } > > }, > > @@ -449,6 +457,10 @@ static QEMUMachine pc_machine_v0_14 = { > > .driver = "pc-sysfw", > > .property = "rom_only", > > .value= stringify(1), > > +}, { > > +.driver = "mc146818rtc", > > +.property = "lost_tick_policy", > > +.value= "none", > > }, > > { /* end of list */ } > > }, > > @@ -505,6 +517,10 @@ static QEMUMachine pc_machine_v0_13 = { > > .driver = "pc-sysfw", > > .property = "rom_only", > > .value= stringify(1), > > +}, { > > +.driver = "mc146818rtc", > > +.property = "lost_tick_policy", > > +.value= "none", > > }, > > { /* end of list */ } > > }, > > @@ -565,6 +581,10 @@ static QEMUMachine pc_machine_v0_12 = { > > .driver = "pc-sysfw", > > .property = "rom_only", > > .value= stringify(1), > > +}, { > > +.driver = "mc146818rtc", > > +.property = "lost_tick_policy", > > +.value= "none", > > }, > > { /* end of list */ } > > } > > @@ -633,6 +653,10 @@ static QEMUMachine pc_machine_v0_11 = { > > .driver = "pc-sysfw", > > .property = "rom_only", > > .value= stringify(1), > > +}, { > > +.driver = "mc146818rtc", > > +.property = "lost_tick_policy", > > +.value= "none", > > }, > > { /* end of list */ } > > } > > @@ -713,6 +737,10 @@ static QEMUMachine pc_machine_v0_10 = { > > .driver = "pc-sysfw", > > .property = "rom_only", > > .value= stringify(1), > > +}, { > > +.driver = "mc146818rtc", > > +.property = "lost_tick_policy", > > +.value= "none", > > }, > > { /* end of list */ } > > }, > > @@ -728,6 +756,10 @@ static QEMUMachine isapc_machine = { > > .driver = "pc-sysfw", > > .property = "rom_only", > > .value= stringify(1), > > +}, { > > +.driver = "mc146818rtc", > > +.property = "lost_tick_policy", > > +.value= "none", > > }, > > { /* end of list */ } > > }, > > @@ -740,6 +772,13 @@ static QEMUMachine xenfv_machine = { > > .init = pc_xen_hvm_init, > > .max_cpus = HVM_MAX_VCPUS, > > .default_machine_opts = "accel=xen", > > +.compat_props = (GlobalProperty[]) { > > +{ > > +.driver = "mc146818rtc", > > +.property = "lost_tick_policy", > > +.value= "none", > > +}, > > +{ /* end of list */ } > > }; > > #endif > > > > Stefano, what do you want for Xen? Anyhow, I would like to keep the old policy for Xen, so the patch is fine by me. Thanks for pinging me!
Re: [Qemu-devel] [PATCH V9 1/8] pci_ids: Add INTEL_82599_SFP_VF id.
On Wed, 21 Mar 2012, Anthony PERARD wrote: > Signed-off-by: Anthony PERARD Acked-by: Stefano Stabellini
Re: [Qemu-devel] [PATCH V9 4/8] pci.c: Add opaque argument to pci_for_each_device.
On Wed, 21 Mar 2012, Anthony PERARD wrote: > Signed-off-by: Anthony PERARD Acked-by: Stefano Stabellini
Re: [Qemu-devel] [PATCH 0/4] More whitespace and coding style clean ups ahead of future changes
On 21 March 2012 21:02, Peter Portante wrote: > Please forgive me if you find these changes are annoying, as I am trying to > learn the ropes of patch submission with git ahead of making a real patch. I'd recommend using git format-patch's --cover-letter option to create your 'cover letter' patch, as this will include a diffstat and summary of all the patches in the patchset. > While working on the code, I found that scripts/checkpatch.pl will flag lines > that I am changing as not adhereing to the codeing standard due to > pre-existing coding violations. So I figured I could learn a bit about how to > submit patches by fixing these files I will be touching before submitting the > code changes. Whitespace-fixes and other coding-style-only changes are a bit of a touchy issue here; they've caused long mailing list arguments in the past. Mostly the line we seem to take is "fix issues in new code submitted; when patching old code you can fix problems complained about in the immediately surrounding few lines". The idea is that gradually the codebase moves into line with the style guide without having massive style-only changes that break tools like 'git blame'. Small changes (like the qemu-timers one) are more likely to be accepted than massive ones (like the slirp patches). Also, it would be helpful if you could split your patches more logically into smaller ones which touch the different subsystems: any patch which touches all of block_int.h, qemu-timer.c and slirp is going to struggle to be reviewed and committed because it is trying to modify three different bits of the codebase and so it needs agreement from more people. Sometimes a patch really does have to touch several different areas, but in this case you're only putting roadblocks in your own path... -- PMM
[Qemu-devel] [PATCH 00/10] QAPI minor fixes and strict mode
This patch includes a few minor changes to QAPI found during a more careful code review (patches 1-5) and the implementation of a strict mode for the QObject input visitor (patches 6-10). While QMP in general is designed so that it is possible to ignore unknown arguments, in the case of the QMP server it is better to reject them to detect bad clients. In fact, we're already doing this at the top level in the argument checker. Strict modes checks for unvisited keys and raises an error if it finds one, so that this checking extends to complex structures. Please review. Paolo Bonzini (10): qapi: add a test case for type errors qapi: fail hard on stack imbalance qapi: fix memory leak on error qapi: shortcut visits on errors qapi: allow freeing partially-allocated objects qapi: simplify qmp_input_next_list qapi: place outermost object on qiv stack qapi: add strict mode to input visitor qmp: add and use q type specifier qmp: parse commands in strict mode monitor.c|3 + qapi/qmp-input-visitor.c | 113 ++ qapi/qmp-input-visitor.h |2 + qmp-commands.hx |4 +- scripts/qapi-commands.py |2 +- scripts/qapi-visit.py| 16 +++ test-qmp-input-strict.c | 234 ++ test-qmp-input-visitor.c | 19 tests/Makefile |5 +- 9 files changed, 354 insertions(+), 44 deletions(-) create mode 100644 test-qmp-input-strict.c -- 1.7.9.1
[Qemu-devel] [PATCH 09/10] qmp: add and use q type specifier
"O" is being used by the transaction and qom-set commands to mean "any QObject", but it really means "do not validate the argument list". Add a new specifier with the correct meaning. Signed-off-by: Paolo Bonzini --- monitor.c |3 +++ qmp-commands.hx |4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/monitor.c b/monitor.c index 2ff1e0b..8946a10 100644 --- a/monitor.c +++ b/monitor.c @@ -4157,6 +4157,9 @@ static int check_client_args_type(const QDict *client_args, case 'O': assert(flags & QMP_ACCEPT_UNKNOWNS); break; +case 'q': +/* Any QObject can be passed. */ +break; case '/': case '.': /* diff --git a/qmp-commands.hx b/qmp-commands.hx index c626ba8..9447871 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -708,7 +708,7 @@ EQMP }, { .name = "transaction", -.args_type = "actions:O", +.args_type = "actions:q", .mhandler.cmd_new = qmp_marshal_input_transaction, }, @@ -2125,7 +2125,7 @@ EQMP { .name = "qom-set", - .args_type = "path:s,property:s,opts:O", + .args_type = "path:s,property:s,value:q", .mhandler.cmd_new = qmp_qom_set, }, -- 1.7.9.1
Re: [Qemu-devel] [PATCH] Remove PCI class code from virtio balloon device
On Thu, Mar 22, 2012 at 08:09:27PM +1100, David Gibson wrote: > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index a0fb7c1..1fd5768 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -790,6 +790,10 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev) > VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); > VirtIODevice *vdev; > > +if (proxy->class_code != PCI_CLASS_OTHERS && > +proxy->class_code != PCI_CLASS_MEMORY_RAM) /* qemu < 1.1 */ > +proxy->class_code = PCI_CLASS_OTHERS; > + {} around if. > vdev = virtio_balloon_init(&pci_dev->qdev); > if (!vdev) { > return -1; > @@ -905,6 +909,7 @@ static TypeInfo virtio_serial_info = { > }; > > static Property virtio_balloon_properties[] = { > +DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, PCI_CLASS_OTHERS), > DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > DEFINE_PROP_END_OF_LIST(), > }; Sorry to bug you - why not set + DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), and then +if (!proxy->class_code) { +proxy->class_code = PCI_CLASS_OTHERS; +} This is what other devices do. > @@ -919,7 +924,7 @@ static void virtio_balloon_class_init(ObjectClass *klass, > void *data) > k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET; > k->device_id = PCI_DEVICE_ID_VIRTIO_BALLOON; > k->revision = VIRTIO_PCI_ABI_VERSION; > -k->class_id = PCI_CLASS_MEMORY_RAM; > +k->class_id = PCI_CLASS_OTHERS; > dc->reset = virtio_pci_reset; > dc->props = virtio_balloon_properties; > } > -- > 1.7.9.1
Re: [Qemu-devel] [PATCH] Remove PCI class code from virtio balloon device
On Thu, Mar 22, 2012 at 10:01:46AM +, Stefan Hajnoczi wrote: > On Thu, Mar 22, 2012 at 9:09 AM, David Gibson > wrote: > > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > > index 3f99f9a..72a4250 100644 > > --- a/hw/pc_piix.c > > +++ b/hw/pc_piix.c > > @@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_0 = { > > .driver = "isa-fdc", > > .property = "check_media_rate", > > .value = "off", > > + }, { > > + .driver = "virtio-balloon-pci", > > + .property = "class", > > + .value = stringify(PCI_CLASS_MEMORY_RAM), > > }, > > { /* end of list */ } > > }, > > @@ -449,6 +453,10 @@ static QEMUMachine pc_machine_v0_14 = { > > pc_machine_v0_15 should also use PCI_CLASS_MEMORY_RAM? Did you skip > it on purpose? Nope, just missed it. I'll respin. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
[Qemu-devel] [PATCH 06/10] qapi: simplify qmp_input_next_list
Tweak a bit the code so that there is cleaner separation between operation on the list and operation on the output. The next patch requires this change. Signed-off-by: Paolo Bonzini --- qapi/qmp-input-visitor.c | 15 +++ 1 files changed, 7 insertions(+), 8 deletions(-) diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index ef9288f..dfc859a 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -64,9 +64,7 @@ static const QObject *qmp_input_get_object(QmpInputVisitor *qiv, static void qmp_input_push(QmpInputVisitor *qiv, const QObject *obj, Error **errp) { qiv->stack[qiv->nb_stack].obj = obj; -if (qobject_type(obj) == QTYPE_QLIST) { -qiv->stack[qiv->nb_stack].entry = qlist_first(qobject_to_qlist(obj)); -} +qiv->stack[qiv->nb_stack].entry = NULL; qiv->nb_stack++; if (qiv->nb_stack >= QIV_STACK_SIZE) { @@ -134,16 +132,17 @@ static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, StackObject *so = &qiv->stack[qiv->nb_stack - 1]; if (so->entry == NULL) { +so->entry = qlist_first(qobject_to_qlist(so->obj)); +} else { +so->entry = qlist_next(so->entry); +} + +if (so->entry == NULL) { return NULL; } entry = g_malloc0(sizeof(*entry)); if (*list) { -so->entry = qlist_next(so->entry); -if (so->entry == NULL) { -g_free(entry); -return NULL; -} (*list)->next = entry; } -- 1.7.9.1
[Qemu-devel] [PATCH] usb/vmstate: add parent dev path
... to make vmstate id string truely unique with multiple host controllers, i.e. move from "1/usb-ptr" to ":00:01.3/1/usb-ptr" (usb tabled connected to piix3 uhci). This obviously breaks migration. To handle this the usb bus property "full-path" is added. When setting this to false old behavior is maintained. This way current qemu will be compatible with old versions when started using '-M pc-$oldversion'. Signed-off-by: Gerd Hoffmann --- hw/pc_piix.c | 28 hw/usb.h |5 + hw/usb/bus.c | 18 +- 3 files changed, 50 insertions(+), 1 deletions(-) diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 3f99f9a..8cb7d5f 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_0 = { .driver = "isa-fdc", .property = "check_media_rate", .value= "off", +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } }, @@ -405,6 +409,10 @@ static QEMUMachine pc_machine_v0_15 = { .driver = "isa-fdc", .property = "check_media_rate", .value= "off", +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } }, @@ -449,6 +457,10 @@ static QEMUMachine pc_machine_v0_14 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } }, @@ -505,6 +517,10 @@ static QEMUMachine pc_machine_v0_13 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } }, @@ -565,6 +581,10 @@ static QEMUMachine pc_machine_v0_12 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } } @@ -633,6 +653,10 @@ static QEMUMachine pc_machine_v0_11 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } } @@ -713,6 +737,10 @@ static QEMUMachine pc_machine_v0_10 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } }, diff --git a/hw/usb.h b/hw/usb.h index e95085f..ae7ccda 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -182,12 +182,17 @@ struct USBEndpoint { QTAILQ_HEAD(, USBPacket) queue; }; +enum USBDeviceFlags { +USB_DEV_FLAG_FULL_PATH, +}; + /* definition of a USB device */ struct USBDevice { DeviceState qdev; USBPort *port; char *port_path; void *opaque; +uint32_t flags; /* Actual connected speed */ int speed; diff --git a/hw/usb/bus.c b/hw/usb/bus.c index d3f8358..f1e567b 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -19,6 +19,8 @@ static struct BusInfo usb_bus_info = { .get_fw_dev_path = usb_get_fw_dev_path, .props = (Property[]) { DEFINE_PROP_STRING("port", USBDevice, port_path), +DEFINE_PROP_BIT("full-path", USBDevice, flags, +USB_DEV_FLAG_FULL_PATH, true), DEFINE_PROP_END_OF_LIST() }, }; @@ -460,7 +462,21 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent) static char *usb_get_dev_path(DeviceState *qdev) { USBDevice *dev = USB_DEVICE(qdev); -return g_strdup(dev->port->path); +DeviceState *hcd = qdev->parent_bus->parent; +char *id = NULL; + +if ((dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) && +hcd && hcd->parent_bus && hcd->parent_bus->info->get_dev_path) { +id = hcd->parent_bus->info->get_dev_path(hcd); +} +if (id) { +int len = strlen(id)+strlen(dev->port->path)+2; +char *ret = g_malloc(len); +snprintf(ret, len, "%s/%s", id, dev->port->path); +return ret; +} else { +return g_strdup(dev->port->path); +} } static char *usb_get_fw_dev_path(DeviceState *qdev) -- 1.7.1
Re: [Qemu-devel] [PATCH] usb/vmstate: add parent dev path
Il 22/03/2012 13:07, Gerd Hoffmann ha scritto: > @@ -460,7 +462,21 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState > *qdev, int indent) > static char *usb_get_dev_path(DeviceState *qdev) > { > USBDevice *dev = USB_DEVICE(qdev); > -return g_strdup(dev->port->path); > +DeviceState *hcd = qdev->parent_bus->parent; > +char *id = NULL; > + > +if ((dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) && > +hcd && hcd->parent_bus && hcd->parent_bus->info->get_dev_path) { > +id = hcd->parent_bus->info->get_dev_path(hcd); > +} > +if (id) { > +int len = strlen(id)+strlen(dev->port->path)+2; > +char *ret = g_malloc(len); > +snprintf(ret, len, "%s/%s", id, dev->port->path); You can use g_strdup_printf here, also this is leaking id (I have to fix it for SCSI too). > +return ret; > +} else { > +return g_strdup(dev->port->path); > +} > } > > static char *usb_get_fw_dev_path(DeviceState *qdev) Paolo
[Qemu-devel] [PATCH 03/10] qapi: fix memory leak on error
QmpInputVisitor would leak the malloced struct if the stack was overflowed. This can be easily fixed using error_propagate. Signed-off-by: Paolo Bonzini --- qapi/qmp-input-visitor.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index b4013cc..ef9288f 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -86,6 +86,7 @@ static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind, { QmpInputVisitor *qiv = to_qiv(v); const QObject *qobj = qmp_input_get_object(qiv, name); +Error *err = NULL; if (!qobj || qobject_type(qobj) != QTYPE_QDICT) { error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -93,8 +94,9 @@ static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind, return; } -qmp_input_push(qiv, qobj, errp); -if (error_is_set(errp)) { +qmp_input_push(qiv, qobj, &err); +if (err) { +error_propagate(errp, err); return; } -- 1.7.9.1
[Qemu-devel] [PATCH 07/10] qapi: place outermost object on qiv stack
This is a slight change in the implementation of QMPInputVisitor that helps when adding strict mode. Const QObjects cannot be inc/decref-ed, and that's why QMPInputVisitor relies heavily on weak references to inner objects. I'm not removing the weak references now, but since refcount+const is a lost battle in C (C++ has "mutable") I think removing const is fine in this case. Signed-off-by: Paolo Bonzini --- qapi/qmp-input-visitor.c | 41 + 1 files changed, 17 insertions(+), 24 deletions(-) diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index dfc859a..97a0186 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -22,14 +22,13 @@ typedef struct StackObject { -const QObject *obj; -const QListEntry *entry; +QObject *obj; +const QListEntry *entry; } StackObject; struct QmpInputVisitor { Visitor visitor; -QObject *obj; StackObject stack[QIV_STACK_SIZE]; int nb_stack; }; @@ -39,21 +38,15 @@ static QmpInputVisitor *to_qiv(Visitor *v) return container_of(v, QmpInputVisitor, visitor); } -static const QObject *qmp_input_get_object(QmpInputVisitor *qiv, - const char *name) +static QObject *qmp_input_get_object(QmpInputVisitor *qiv, + const char *name) { -const QObject *qobj; - -if (qiv->nb_stack == 0) { -qobj = qiv->obj; -} else { -qobj = qiv->stack[qiv->nb_stack - 1].obj; -} +QObject *qobj = qiv->stack[qiv->nb_stack - 1].obj; if (qobj) { if (name && qobject_type(qobj) == QTYPE_QDICT) { return qdict_get(qobject_to_qdict(qobj), name); -} else if (qiv->nb_stack > 0 && qobject_type(qobj) == QTYPE_QLIST) { +} else if (qiv->stack[qiv->nb_stack - 1].entry) { return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry); } } @@ -61,7 +54,7 @@ static const QObject *qmp_input_get_object(QmpInputVisitor *qiv, return qobj; } -static void qmp_input_push(QmpInputVisitor *qiv, const QObject *obj, Error **errp) +static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp) { qiv->stack[qiv->nb_stack].obj = obj; qiv->stack[qiv->nb_stack].entry = NULL; @@ -83,7 +76,7 @@ static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind, const char *name, size_t size, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); -const QObject *qobj = qmp_input_get_object(qiv, name); +QObject *qobj = qmp_input_get_object(qiv, name); Error *err = NULL; if (!qobj || qobject_type(qobj) != QTYPE_QDICT) { @@ -113,7 +106,7 @@ static void qmp_input_end_struct(Visitor *v, Error **errp) static void qmp_input_start_list(Visitor *v, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); -const QObject *qobj = qmp_input_get_object(qiv, name); +QObject *qobj = qmp_input_get_object(qiv, name); if (!qobj || qobject_type(qobj) != QTYPE_QLIST) { error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -160,7 +153,7 @@ static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); -const QObject *qobj = qmp_input_get_object(qiv, name); +QObject *qobj = qmp_input_get_object(qiv, name); if (!qobj || qobject_type(qobj) != QTYPE_QINT) { error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -175,7 +168,7 @@ static void qmp_input_type_bool(Visitor *v, bool *obj, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); -const QObject *qobj = qmp_input_get_object(qiv, name); +QObject *qobj = qmp_input_get_object(qiv, name); if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) { error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -190,7 +183,7 @@ static void qmp_input_type_str(Visitor *v, char **obj, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); -const QObject *qobj = qmp_input_get_object(qiv, name); +QObject *qobj = qmp_input_get_object(qiv, name); if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) { error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -205,7 +198,7 @@ static void qmp_input_type_number(Visitor *v, double *obj, const char *name, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); -const QObject *qobj = qmp_input_get_object(qiv, name); +QObject *qobj = qmp_input_get_object(qiv, name); if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) { error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -220,7 +213,7 @@ static void qmp_input_start_optional(Visitor *v, bool *present,
Re: [Qemu-devel] [PATCH V9 5/8] Introduce Xen PCI Passthrough, qdevice (1/3)
On Wed, Mar 21, 2012 at 20:46, Michael S. Tsirkin wrote: > On Wed, Mar 21, 2012 at 06:29:02PM +, Anthony PERARD wrote: >> diff --git a/hw/xen_pci_passthrough.h b/hw/xen_pci_passthrough.h >> new file mode 100644 >> index 000..2c5e83d >> --- /dev/null >> +++ b/hw/xen_pci_passthrough.h >> @@ -0,0 +1,250 @@ >> +#ifndef QEMU_HW_XEN_PCI_PASSTHROUGH_H >> +# define QEMU_HW_XEN_PCI_PASSTHROUGH_H > > Way overboard. QEMU_HW_ is not needed. > >> + >> +#include "qemu-common.h" >> +#include "xen_common.h" >> +#include "pci.h" >> +#include "host-pci-device.h" >> + >> +/* #define PT_LOGGING_ENABLED */ >> +/* #define PT_DEBUG_PCI_CONFIG_ACCESS */ > > Don't keep dead code around esp not in headers. > >> + >> +void pt_log(const PCIDevice *d, const char *f, ...) GCC_FMT_ATTR(2, 3); >> + >> +#define PT_ERR(d, _f, _a...) pt_log(d, "%s: Error: " _f, __func__, ##_a) >> + >> +#ifdef PT_LOGGING_ENABLED >> +# define PT_LOG(d, _f, _a...) pt_log(d, "%s: " _f, __func__, ##_a) >> +# define PT_WARN(d, _f, _a...) pt_log(d, "%s: Warning: " _f, __func__, >> ##_a) >> +#else >> +# define PT_LOG(d, _f, _a...) >> +# define PT_WARN(d, _f, _a...) >> +#endif >> + >> +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS >> +# define PT_LOG_CONFIG(d, addr, val, len) \ >> + pt_log(d, "%s: address=0x%04x val=0x%08x len=%d\n", \ >> + __func__, addr, val, len) >> +#else >> +# define PT_LOG_CONFIG(d, addr, val, len) >> +#endif > > There's no reason not to prefix xen things with XEN_PT_ > xen_pt_ etc. > And if you make names consistens with file names > naming files xen_pt_.x, people will know where to look for them. OK, I'll change that. Thanks,+ -- Anthony PERARD
[Qemu-devel] [PATCH 05/10] qapi: allow freeing partially-allocated objects
Objects going through the dealloc visitor can be only partially allocated. Detect the situation and avoid a segfault. This also helps with the input visitor, when there are errors. Signed-off-by: Paolo Bonzini --- scripts/qapi-visit.py |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index b242315..a85fb76 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -65,6 +65,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** return; } visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), errp); +if (obj && !*obj) { +goto end; +} ''', name=name) push_indent() @@ -72,6 +75,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** pop_indent() ret += mcgen(''' +end: visit_end_struct(m, errp); } ''') @@ -122,6 +126,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** return; } visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err); +if (obj && !*obj) { +goto end; +} visit_type_%(name)sKind(m, &(*obj)->kind, "type", &err); if (err) { error_propagate(errp, err); -- 1.7.9.1
[Qemu-devel] [PATCH 04/10] qapi: shortcut visits on errors
We can exit very soon if we enter a visitor with a preexisting error. This simplifies some cases because we will not have to deal with obj being non-NULL while *obj is NULL. Signed-off-by: Paolo Bonzini --- scripts/qapi-visit.py |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 54117d4..b242315 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -61,6 +61,9 @@ def generate_visit_struct(name, members): void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp) { +if (error_is_set(errp)) { +return; +} visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), errp); ''', name=name) @@ -81,6 +84,9 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, { GenericList *i, **head = (GenericList **)obj; +if (error_is_set(errp)) { +return; +} visit_start_list(m, name, errp); for (*head = i = visit_next_list(m, head, errp); i; i = visit_next_list(m, &i, errp)) { @@ -112,6 +118,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error ** { Error *err = NULL; +if (error_is_set(errp)) { +return; +} visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err); visit_type_%(name)sKind(m, &(*obj)->kind, "type", &err); if (err) { -- 1.7.9.1
[Qemu-devel] [PATCH 10/10] qmp: parse commands in strict mode
Signed-off-by: Paolo Bonzini --- scripts/qapi-commands.py |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 3aabf61..4774b61 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -140,7 +140,7 @@ v = qapi_dealloc_get_visitor(md); ''') else: ret += mcgen(''' -mi = qmp_input_visitor_new(%(obj)s); +mi = qmp_input_visitor_new_strict(%(obj)s); v = qmp_input_get_visitor(mi); ''', obj=obj) -- 1.7.9.1
[Qemu-devel] [PATCH 02/10] qapi: fail hard on stack imbalance
QmpOutputVisitor will segfault if an imbalanced end function is called. So we can abort in QmpInputVisitor too. Signed-off-by: Paolo Bonzini --- qapi/qmp-input-visitor.c |5 + 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index e6b6152..b4013cc 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -77,11 +77,8 @@ static void qmp_input_push(QmpInputVisitor *qiv, const QObject *obj, Error **err static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp) { +assert(qiv->nb_stack > 0); qiv->nb_stack--; -if (qiv->nb_stack < 0) { -error_set(errp, QERR_BUFFER_OVERRUN); -return; -} } static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind, -- 1.7.9.1
[Qemu-devel] [PATCH 01/10] qapi: add a test case for type errors
There is no test case for parse errors, add one. Signed-off-by: Paolo Bonzini --- test-qmp-input-visitor.c | 19 +++ 1 files changed, 19 insertions(+), 0 deletions(-) diff --git a/test-qmp-input-visitor.c b/test-qmp-input-visitor.c index 1996e49..c30fdc4 100644 --- a/test-qmp-input-visitor.c +++ b/test-qmp-input-visitor.c @@ -258,6 +258,23 @@ static void input_visitor_test_add(const char *testpath, visitor_input_teardown); } +static void test_visitor_in_errors(TestInputVisitorData *data, + const void *unused) +{ +TestStruct *p = NULL; +Error *errp = NULL; +Visitor *v; + +v = visitor_input_test_init(data, "{ 'integer': false, 'boolean': 'foo', 'string': -42 }"); + +visit_type_TestStruct(v, &p, NULL, &errp); +g_assert(error_is_set(&errp)); +g_assert(p->string == NULL); + +g_free(p->string); +g_free(p); +} + int main(int argc, char **argv) { TestInputVisitorData in_visitor_data; @@ -282,6 +299,8 @@ int main(int argc, char **argv) &in_visitor_data, test_visitor_in_list); input_visitor_test_add("/visitor/input/union", &in_visitor_data, test_visitor_in_union); +input_visitor_test_add("/visitor/input/errors", +&in_visitor_data, test_visitor_in_errors); g_test_run(); -- 1.7.9.1
Re: [Qemu-devel] [PATCH uq/master] kvm: Drop unused kvm_pit_in_kernel
On 2012-03-22 08:18, Gleb Natapov wrote: > On Wed, Mar 21, 2012 at 02:49:09PM +0100, Jan Kiszka wrote: >> On 2012-03-21 14:41, Gleb Natapov wrote: >>> On Wed, Mar 21, 2012 at 02:39:47PM +0100, Jan Kiszka wrote: On 2012-03-21 14:36, Avi Kivity wrote: > On 03/21/2012 02:36 PM, Jan Kiszka wrote: >> This is now implied by kvm_irqchip_in_kernel. >> >> > > So we can't have -no-kvm-pit? > > No huge loss, but unexpected. See e81dda195556e72f8cd294998296c1051aab30a8. >>> I am curious what is the reason for upstream to not supporting disabling the >>> in-kernel PIT separately? >> >> It was considered no longer relevant: >> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/85393 >> > Hmm, may be we should think about this some more. If in the (not so far) > future we want to drop pit emulation from the kernel we may want to support > -no-kvm-pit to allow migration from old kernels to new one. That's not an issue. Both device models are compatible, and you can migrate between kernel_irqchip=on/off theses days with QEMU. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 28/36] vmstate: rename machine.c to vmstate-cpu.c
Am 19.03.2012 23:57, schrieb Juan Quintela: > They only contain vmstate cpu sections nowadays. Change name to reflect the > case. In light of me creating a file cpu.c for every target except ppc, might it make more sense to call it cpu-vmstate.c? Or do you envision other vmstate-*.c files? Andreas > > Signed-off-by: Juan Quintela > --- > Makefile.target|3 ++- > target-alpha/{machine.c => vmstate-cpu.c} |0 > target-arm/{machine.c => vmstate-cpu.c}|0 > target-cris/{machine.c => vmstate-cpu.c} |0 > target-i386/{machine.c => vmstate-cpu.c} |0 > target-lm32/{machine.c => vmstate-cpu.c} |0 > target-m68k/{machine.c => vmstate-cpu.c} |0 > target-microblaze/{machine.c => vmstate-cpu.c} |0 > target-mips/{machine.c => vmstate-cpu.c} |0 > target-ppc/{machine.c => vmstate-cpu.c}|0 > target-s390x/{machine.c => vmstate-cpu.c} |0 > target-sh4/{machine.c => vmstate-cpu.c}|0 > target-sparc/{machine.c => vmstate-cpu.c} |0 > target-xtensa/{machine.c => vmstate-cpu.c} |0 > 14 files changed, 2 insertions(+), 1 deletions(-) -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH 08/10] qapi: add strict mode to input visitor
While QMP in general is designed so that it is possible to ignore unknown arguments, in the case of the QMP server it is better to reject them to detect bad clients. In fact, we're already doing this at the top level in the argument checker. To extend this to complex structures, add a mode to the input visitor where it checks for unvisited keys and raises an error if it finds one. Signed-off-by: Paolo Bonzini --- qapi/qmp-input-visitor.c | 48 +- qapi/qmp-input-visitor.h |2 + test-qmp-input-strict.c | 234 ++ tests/Makefile |5 +- 4 files changed, 285 insertions(+), 4 deletions(-) create mode 100644 test-qmp-input-strict.c diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 97a0186..eb09874 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -24,6 +24,7 @@ typedef struct StackObject { QObject *obj; const QListEntry *entry; +GHashTable *h; } StackObject; struct QmpInputVisitor @@ -31,6 +32,7 @@ struct QmpInputVisitor Visitor visitor; StackObject stack[QIV_STACK_SIZE]; int nb_stack; +bool strict; }; static QmpInputVisitor *to_qiv(Visitor *v) @@ -45,6 +47,9 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv, if (qobj) { if (name && qobject_type(qobj) == QTYPE_QDICT) { +if (qiv->stack[qiv->nb_stack - 1].h) { +g_hash_table_remove(qiv->stack[qiv->nb_stack - 1].h, name); +} return qdict_get(qobject_to_qdict(qobj), name); } else if (qiv->stack[qiv->nb_stack - 1].entry) { return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry); @@ -54,20 +59,47 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv, return qobj; } +static void qdict_add_key(const char *key, QObject *obj, void *opaque) +{ +GHashTable *h = opaque; +g_hash_table_insert(h, (gpointer) key, NULL); +} + static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp) { -qiv->stack[qiv->nb_stack].obj = obj; -qiv->stack[qiv->nb_stack].entry = NULL; -qiv->nb_stack++; +GHashTable *h; if (qiv->nb_stack >= QIV_STACK_SIZE) { error_set(errp, QERR_BUFFER_OVERRUN); return; } + +qiv->stack[qiv->nb_stack].obj = obj; +qiv->stack[qiv->nb_stack].entry = NULL; +qiv->stack[qiv->nb_stack].h = NULL; + +if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) { +h = g_hash_table_new(g_str_hash, g_str_equal); +qdict_iter(qobject_to_qdict(obj), qdict_add_key, h); +qiv->stack[qiv->nb_stack].h = h; +} + +qiv->nb_stack++; } static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp) { +GHashTableIter iter; +gpointer key; + +if (qiv->strict && qiv->stack[qiv->nb_stack - 1].h) { +g_hash_table_iter_init(&iter, qiv->stack[qiv->nb_stack - 1].h); +if (g_hash_table_iter_next(&iter, &key, NULL)) { +error_set(errp, QERR_QMP_EXTRA_MEMBER, (char *) key); +} +g_hash_table_unref(qiv->stack[qiv->nb_stack - 1].h); +} + assert(qiv->nb_stack > 0); qiv->nb_stack--; } @@ -257,3 +289,13 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj) return v; } + +QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj) +{ +QmpInputVisitor *v; + +v = qmp_input_visitor_new(obj); +v->strict = true; + +return v; +} diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h index 3f798f0..e0a48a5 100644 --- a/qapi/qmp-input-visitor.h +++ b/qapi/qmp-input-visitor.h @@ -20,6 +20,8 @@ typedef struct QmpInputVisitor QmpInputVisitor; QmpInputVisitor *qmp_input_visitor_new(QObject *obj); +QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj); + void qmp_input_visitor_cleanup(QmpInputVisitor *v); Visitor *qmp_input_get_visitor(QmpInputVisitor *v); diff --git a/test-qmp-input-strict.c b/test-qmp-input-strict.c new file mode 100644 index 000..f6df8cb --- /dev/null +++ b/test-qmp-input-strict.c @@ -0,0 +1,234 @@ +/* + * QMP Input Visitor unit-tests (strict mode). + * + * Copyright (C) 2011-2012 Red Hat Inc. + * + * Authors: + * Luiz Capitulino + * Paolo Bonzini + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include +#include + +#include "qapi/qmp-input-visitor.h" +#include "test-qapi-types.h" +#include "test-qapi-visit.h" +#include "qemu-objects.h" + +typedef struct TestInputVisitorData { +QObject *obj; +QmpInputVisitor *qiv; +} TestInputVisitorData; + +static void validate_teardown(TestInputVisitorData *data, + const void *unused) +{ +qobject_decref(data->obj); +data->obj = NULL; + +if (data->qiv) { +qmp_input_visitor_cleanup(data->qiv); +data->qiv = NULL; +} +} + +/* This is provided instead of a test setup function so that the JSON +
Re: [Qemu-devel] [PATCH uq/master] kvm: Drop unused kvm_pit_in_kernel
On Thu, Mar 22, 2012 at 01:41:18PM +0100, Jan Kiszka wrote: > On 2012-03-22 08:18, Gleb Natapov wrote: > > On Wed, Mar 21, 2012 at 02:49:09PM +0100, Jan Kiszka wrote: > >> On 2012-03-21 14:41, Gleb Natapov wrote: > >>> On Wed, Mar 21, 2012 at 02:39:47PM +0100, Jan Kiszka wrote: > On 2012-03-21 14:36, Avi Kivity wrote: > > On 03/21/2012 02:36 PM, Jan Kiszka wrote: > >> This is now implied by kvm_irqchip_in_kernel. > >> > >> > > > > So we can't have -no-kvm-pit? > > > > No huge loss, but unexpected. > > See e81dda195556e72f8cd294998296c1051aab30a8. > > >>> I am curious what is the reason for upstream to not supporting disabling > >>> the > >>> in-kernel PIT separately? > >> > >> It was considered no longer relevant: > >> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/85393 > >> > > Hmm, may be we should think about this some more. If in the (not so far) > > future we want to drop pit emulation from the kernel we may want to support > > -no-kvm-pit to allow migration from old kernels to new one. > > That's not an issue. Both device models are compatible, and you can > migrate between kernel_irqchip=on/off theses days with QEMU. > Cool. Including PIT lost tick compensation? -- Gleb.
Re: [Qemu-devel] block stream: close unused files and update ->backing_hd
On Thu, Mar 22, 2012 at 1:09 AM, Marcelo Tosatti wrote: > > Close the now unused images that were part of the previous backing file > chain and adjust ->backing_hd properly. > > Fixes https://bugzilla.redhat.com/show_bug.cgi?id=801449 > > Signed-off-by: Marcelo Tosatti Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [PATCH 3/3] configure: add --confsuffix option
On Wed, Mar 21, 2012 at 11:23:00PM +0100, Paolo Bonzini wrote: > Il 21/03/2012 15:42, Eduardo Habkost ha scritto: > > For this one, we would have compatibility issues to take care of: > > > > ./configure --datadir=FOO --sysconfdir=SYS > > qemu data dir: FOO > > (it would be better if it was FOO/qemu, but needed for compatibility) > > qemu conf dir: SYS/qemu > > Hmm, perhaps we can break it... I checked Fedora, Debian > (http://cdn.debian.net/debian/pool/main/q/qemu/qemu_1.0.1+dfsg-1.debian.tar.gz), > Arch Linux > (http://projects.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/qemu), > Gentoo > (http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/app-emulation/qemu-kvm/qemu-kvm-0.15.1-r1.ebuild?view=markup), > FreeBSD > (http://www.freebsd.org/cgi/cvsweb.cgi/ports/emulators/qemu/Makefile?rev=1.122;content-type=text%2Fplain) > and none of them use it. > > And since we are at it, let's call the option --with-confsuffix so it is > a bit more autoconfy. Excellent. I'll do it. -- Eduardo
Re: [Qemu-devel] [PATCH 28/36] vmstate: rename machine.c to vmstate-cpu.c
Andreas Färber wrote: > Am 19.03.2012 23:57, schrieb Juan Quintela: >> They only contain vmstate cpu sections nowadays. Change name to >> reflect the case. > > In light of me creating a file cpu.c for every target except ppc, might > it make more sense to call it cpu-vmstate.c? Or do you envision other > vmstate-*.c files? I renamed them pecause they don't contain the boards definitions anymore. Will move next round to cpu-vmstate.c, I don't really care about the name. Later, Juan.
Re: [Qemu-devel] [PATCH V2 4/7] qed: add bdrv_invalidate_cache to be called after incoming live migration
On Wed, Mar 21, 2012 at 3:52 PM, Benoît Canet wrote: > The QED image is reopened to flush metadata and check consistency. > > Signed-off-by: Benoit Canet > --- > block/qed.c | 15 +++ > block/qed.h | 1 + > 2 files changed, 16 insertions(+), 0 deletions(-) > > diff --git a/block/qed.c b/block/qed.c > index a041d31..c47272c 100644 > --- a/block/qed.c > +++ b/block/qed.c > @@ -375,6 +375,10 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags) > int ret; > > s->bs = bs; > + > + /* backup flags for bdrv_qed_invalidate_cache */ > + s->flags = flags; It's not clear to me why we need to introduce this field to stash flags values. bs->open_flags already has this information. Originally this was introduced in 06d9260ffa9 ("qcow2: implement bdrv_invalidate_cache (v2)") for qcow2. I wonder if that field is necessary when we already have bs->open_flags. What I don't like about s->flags is that it duplicates state *and* it's done in each block driver that supports .bdrv_invalidate_cache(). So I wonder if we can drop it? Stefan
Re: [Qemu-devel] [PATCH uq/master] kvm: Drop unused kvm_pit_in_kernel
On 2012-03-22 13:52, Gleb Natapov wrote: > On Thu, Mar 22, 2012 at 01:41:18PM +0100, Jan Kiszka wrote: >> On 2012-03-22 08:18, Gleb Natapov wrote: >>> On Wed, Mar 21, 2012 at 02:49:09PM +0100, Jan Kiszka wrote: On 2012-03-21 14:41, Gleb Natapov wrote: > On Wed, Mar 21, 2012 at 02:39:47PM +0100, Jan Kiszka wrote: >> On 2012-03-21 14:36, Avi Kivity wrote: >>> On 03/21/2012 02:36 PM, Jan Kiszka wrote: This is now implied by kvm_irqchip_in_kernel. >>> >>> So we can't have -no-kvm-pit? >>> >>> No huge loss, but unexpected. >> >> See e81dda195556e72f8cd294998296c1051aab30a8. >> > I am curious what is the reason for upstream to not supporting disabling > the > in-kernel PIT separately? It was considered no longer relevant: http://thread.gmane.org/gmane.comp.emulators.kvm.devel/85393 >>> Hmm, may be we should think about this some more. If in the (not so far) >>> future we want to drop pit emulation from the kernel we may want to support >>> -no-kvm-pit to allow migration from old kernels to new one. >> >> That's not an issue. Both device models are compatible, and you can >> migrate between kernel_irqchip=on/off theses days with QEMU. >> > Cool. Including PIT lost tick compensation? The feature is not yet available for the userspace PIT (we only support tick compensation for the userspace RTC - IIRC). Requires better IRQ injection feedback, you likely remember. ;) Also, I wonder if the kernel exports all tick-compensation related states for save/restore. Need to check again... Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH v2] usb/vmstate: add parent dev path
... to make vmstate id string truely unique with multiple host controllers, i.e. move from "1/usb-ptr" to ":00:01.3/1/usb-ptr" (usb tabled connected to piix3 uhci). This obviously breaks migration. To handle this the usb bus property "full-path" is added. When setting this to false old behavior is maintained. This way current qemu will be compatible with old versions when started using '-M pc-$oldversion'. Signed-off-by: Gerd Hoffmann --- hw/pc_piix.c | 28 hw/usb.h |5 + hw/usb/bus.c | 17 - 3 files changed, 49 insertions(+), 1 deletions(-) diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 3f99f9a..8cb7d5f 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_0 = { .driver = "isa-fdc", .property = "check_media_rate", .value= "off", +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } }, @@ -405,6 +409,10 @@ static QEMUMachine pc_machine_v0_15 = { .driver = "isa-fdc", .property = "check_media_rate", .value= "off", +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } }, @@ -449,6 +457,10 @@ static QEMUMachine pc_machine_v0_14 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } }, @@ -505,6 +517,10 @@ static QEMUMachine pc_machine_v0_13 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } }, @@ -565,6 +581,10 @@ static QEMUMachine pc_machine_v0_12 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } } @@ -633,6 +653,10 @@ static QEMUMachine pc_machine_v0_11 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } } @@ -713,6 +737,10 @@ static QEMUMachine pc_machine_v0_10 = { .driver = "pc-sysfw", .property = "rom_only", .value= stringify(1), +},{ +.driver = "USB", +.property = "full-path", +.value= "no", }, { /* end of list */ } }, diff --git a/hw/usb.h b/hw/usb.h index e95085f..ae7ccda 100644 --- a/hw/usb.h +++ b/hw/usb.h @@ -182,12 +182,17 @@ struct USBEndpoint { QTAILQ_HEAD(, USBPacket) queue; }; +enum USBDeviceFlags { +USB_DEV_FLAG_FULL_PATH, +}; + /* definition of a USB device */ struct USBDevice { DeviceState qdev; USBPort *port; char *port_path; void *opaque; +uint32_t flags; /* Actual connected speed */ int speed; diff --git a/hw/usb/bus.c b/hw/usb/bus.c index d3f8358..2068640 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -19,6 +19,8 @@ static struct BusInfo usb_bus_info = { .get_fw_dev_path = usb_get_fw_dev_path, .props = (Property[]) { DEFINE_PROP_STRING("port", USBDevice, port_path), +DEFINE_PROP_BIT("full-path", USBDevice, flags, +USB_DEV_FLAG_FULL_PATH, true), DEFINE_PROP_END_OF_LIST() }, }; @@ -460,7 +462,20 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent) static char *usb_get_dev_path(DeviceState *qdev) { USBDevice *dev = USB_DEVICE(qdev); -return g_strdup(dev->port->path); +DeviceState *hcd = qdev->parent_bus->parent; +char *id = NULL; + +if ((dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) && +hcd && hcd->parent_bus && hcd->parent_bus->info->get_dev_path) { +id = hcd->parent_bus->info->get_dev_path(hcd); +} +if (id) { +char *ret = g_strdup_printf("%s/%s", id, dev->port->path); +g_free(id); +return ret; +} else { +return g_strdup(dev->port->path); +} } static char *usb_get_fw_dev_path(DeviceState *qdev) -- 1.7.1
Re: [Qemu-devel] [PATCH] kvm: allow arbitrarily sized mmio ioeventfd
On 2012-03-20 13:31, Michael S. Tsirkin wrote: > We use a 2 byte ioeventfd for virtio memory, > add support for this. > > Signed-off-by: Michael S. Tsirkin > --- > hw/ivshmem.c |8 > kvm-all.c| 15 --- > kvm-stub.c |2 +- > kvm.h|3 ++- > 4 files changed, 15 insertions(+), 13 deletions(-) > > diff --git a/hw/ivshmem.c b/hw/ivshmem.c > index 5ebf840..f02530c 100644 > --- a/hw/ivshmem.c > +++ b/hw/ivshmem.c > @@ -354,8 +354,8 @@ static void close_guest_eventfds(IVShmemState *s, int > posn) > guest_curr_max = s->peers[posn].nb_eventfds; > > for (i = 0; i < guest_curr_max; i++) { > -kvm_set_ioeventfd_mmio_long(s->peers[posn].eventfds[i], > -s->mmio_addr + DOORBELL, (posn << 16) | i, 0); > +kvm_set_ioeventfd_mmio(s->peers[posn].eventfds[i], > +s->mmio_addr + DOORBELL, (posn << 16) | i, 0, 4); > close(s->peers[posn].eventfds[i]); > } > > @@ -500,8 +500,8 @@ static void ivshmem_read(void *opaque, const uint8_t * > buf, int flags) > } > > if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD)) { > -if (kvm_set_ioeventfd_mmio_long(incoming_fd, s->mmio_addr + DOORBELL, > -(incoming_posn << 16) | guest_max_eventfd, 1) < 0) { > +if (kvm_set_ioeventfd_mmio(incoming_fd, s->mmio_addr + DOORBELL, > +(incoming_posn << 16) | guest_max_eventfd, 1, 4) < > 0) { > fprintf(stderr, "ivshmem: ioeventfd not available\n"); > } > } > diff --git a/kvm-all.c b/kvm-all.c > index 42e5e23..bcf0dbe 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -744,10 +744,10 @@ static void kvm_mem_ioeventfd_add(MemoryRegionSection > *section, > { > int r; > > -assert(match_data && section->size == 4); > +assert(match_data && section->size <= 8); Probably nitpicking, but does it also work with non-power-of-two sizes? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] kvm: allow arbitrarily sized mmio ioeventfd
On 2012-03-20 13:31, Michael S. Tsirkin wrote: > We use a 2 byte ioeventfd for virtio memory, > add support for this. > > Signed-off-by: Michael S. Tsirkin > --- > hw/ivshmem.c |8 > kvm-all.c| 15 --- > kvm-stub.c |2 +- > kvm.h|3 ++- > 4 files changed, 15 insertions(+), 13 deletions(-) > > diff --git a/hw/ivshmem.c b/hw/ivshmem.c > index 5ebf840..f02530c 100644 > --- a/hw/ivshmem.c > +++ b/hw/ivshmem.c > @@ -354,8 +354,8 @@ static void close_guest_eventfds(IVShmemState *s, int > posn) > guest_curr_max = s->peers[posn].nb_eventfds; > > for (i = 0; i < guest_curr_max; i++) { > -kvm_set_ioeventfd_mmio_long(s->peers[posn].eventfds[i], > -s->mmio_addr + DOORBELL, (posn << 16) | i, 0); > +kvm_set_ioeventfd_mmio(s->peers[posn].eventfds[i], > +s->mmio_addr + DOORBELL, (posn << 16) | i, 0, 4); > close(s->peers[posn].eventfds[i]); > } > > @@ -500,8 +500,8 @@ static void ivshmem_read(void *opaque, const uint8_t * > buf, int flags) > } > > if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD)) { > -if (kvm_set_ioeventfd_mmio_long(incoming_fd, s->mmio_addr + DOORBELL, > -(incoming_posn << 16) | guest_max_eventfd, 1) < 0) { > +if (kvm_set_ioeventfd_mmio(incoming_fd, s->mmio_addr + DOORBELL, > +(incoming_posn << 16) | guest_max_eventfd, 1, 4) < > 0) { > fprintf(stderr, "ivshmem: ioeventfd not available\n"); > } > } > diff --git a/kvm-all.c b/kvm-all.c > index 42e5e23..bcf0dbe 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -744,10 +744,10 @@ static void kvm_mem_ioeventfd_add(MemoryRegionSection > *section, > { > int r; > > -assert(match_data && section->size == 4); > +assert(match_data && section->size <= 8); Probably nitpicking, but does it also work with non-power-of-two sizes? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 0/4] More whitespace and coding style clean ups ahead of future changes
On 2012-03-22 01:05, Paolo Bonzini wrote: > Il 21/03/2012 22:02, Peter Portante ha scritto: >> Hi Folks, >> >> Please forgive me if you find these changes are annoying, as I am trying to >> learn the ropes of patch submission with git ahead of making a real patch. >> >> While working on the code, I found that scripts/checkpatch.pl will flag lines >> that I am changing as not adhereing to the codeing standard due to >> pre-existing coding violations. So I figured I could learn a bit about how to >> submit patches by fixing these files I will be touching before submitting the >> code changes. > > I think slirp is a mess and it's not worth doing this kind of sweeping > change. Just ignore checkpatch output for slirp. In fact, I could imagine accepting slirp cleanups provided they come in palatable pieces and validate that the binary output is unchanged. It's really a pain changing something in slirp having to use a totally inconsistent style just to keep the code slightly readable. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] fix '-cpu ?' Segfault
On 03/21/2012 08:33 AM, Eduardo Habkost wrote: Fix stupid copy&paste mistake at commit ecf40beae7dcbb057d4f115207f9d8276832a774: I moved code around but kept "optarg" on the cpu_list() call. Reported-by: Jiri Denemark Signed-off-by: Eduardo Habkost --- vl.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/vl.c b/vl.c index 112b0e0..0fccf50 100644 --- a/vl.c +++ b/vl.c @@ -3196,7 +3196,7 @@ int main(int argc, char **argv, char **envp) cpudef_init(); if (cpu_model&& *cpu_model == '?') { -list_cpus(stdout,&fprintf, optarg); +list_cpus(stdout,&fprintf, cpu_model); exit(0); } Does -cpu ? actually work then? I only see the list of cpus when using -cpu ?_ for example. Stefan
Re: [Qemu-devel] [PATCH] kvm: allow arbitrarily sized mmio ioeventfd
On 03/22/2012 03:28 PM, Jan Kiszka wrote: > On 2012-03-20 13:31, Michael S. Tsirkin wrote: > > We use a 2 byte ioeventfd for virtio memory, > > add support for this. > > > > Signed-off-by: Michael S. Tsirkin > > --- > > hw/ivshmem.c |8 > > kvm-all.c| 15 --- > > kvm-stub.c |2 +- > > kvm.h|3 ++- > > 4 files changed, 15 insertions(+), 13 deletions(-) > > > > diff --git a/hw/ivshmem.c b/hw/ivshmem.c > > index 5ebf840..f02530c 100644 > > --- a/hw/ivshmem.c > > +++ b/hw/ivshmem.c > > @@ -354,8 +354,8 @@ static void close_guest_eventfds(IVShmemState *s, int > > posn) > > guest_curr_max = s->peers[posn].nb_eventfds; > > > > for (i = 0; i < guest_curr_max; i++) { > > -kvm_set_ioeventfd_mmio_long(s->peers[posn].eventfds[i], > > -s->mmio_addr + DOORBELL, (posn << 16) | i, 0); > > +kvm_set_ioeventfd_mmio(s->peers[posn].eventfds[i], > > +s->mmio_addr + DOORBELL, (posn << 16) | i, 0, 4); > > close(s->peers[posn].eventfds[i]); > > } > > > > @@ -500,8 +500,8 @@ static void ivshmem_read(void *opaque, const uint8_t * > > buf, int flags) > > } > > > > if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD)) { > > -if (kvm_set_ioeventfd_mmio_long(incoming_fd, s->mmio_addr + > > DOORBELL, > > -(incoming_posn << 16) | guest_max_eventfd, 1) < 0) > > { > > +if (kvm_set_ioeventfd_mmio(incoming_fd, s->mmio_addr + DOORBELL, > > +(incoming_posn << 16) | guest_max_eventfd, 1, 4) < > > 0) { > > fprintf(stderr, "ivshmem: ioeventfd not available\n"); > > } > > } > > diff --git a/kvm-all.c b/kvm-all.c > > index 42e5e23..bcf0dbe 100644 > > --- a/kvm-all.c > > +++ b/kvm-all.c > > @@ -744,10 +744,10 @@ static void kvm_mem_ioeventfd_add(MemoryRegionSection > > *section, > > { > > int r; > > > > -assert(match_data && section->size == 4); > > +assert(match_data && section->size <= 8); > > Probably nitpicking, but does it also work with non-power-of-two sizes? > It won't, but it's hard to generate 3-byte writes (not impossible though). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt
On Thu, Mar 22, 2012 at 11:32:44AM +0200, Gleb Natapov wrote: > On Tue, Mar 13, 2012 at 11:53:19AM -0300, Eduardo Habkost wrote: > > So, trying to summarize what was discussed in the call: > > > > On Mon, Mar 12, 2012 at 10:08:10AM -0300, Eduardo Habkost wrote: > > > > Let's say we moved CPU definitions to /usr/share/qemu/cpu-models.xml. > > > > > > > > Obviously, we'd want a command line option to be able to change that > > > > location so we'd introduce -cpu-models PATH. > > > > > > > > But we want all of our command line options to be settable by the > > > > global configuration file so we would have a cpu-model=PATH to the > > > > configuration file. > > > > > > > > But why hard code a path when we can just set the default path in the > > > > configuration file so let's avoid hard coding and just put > > > > cpu-models=/usr/share/qemu/cpu-models.xml in the default > > > > configuration file. > > > > > > We wouldn't do the above. > > > > > > -nodefconfig should disable the loading of files on /etc, but it > > > shouldn't disable loading internal non-configurable data that we just > > > happened to choose to store outside the qemu binary because it makes > > > development easier. > > > > The statement above is the one not fulfilled by the compromise solution: > > -nodefconfig would really disable the loading of files on /usr/share. > > > What does this mean? Will -nodefconfig disable loading of bios.bin, > option roms, keymaps? Correcting myself: loading of _config_ files on /usr/share. ROM images are opaque data to be presented to the guest somehow, just like a disk image or kernel binary. But maybe keymaps will become "configuration" someday, I really don't know. > > > > > > Doing this would make it impossible to deploy fixes to users if we evern > > > find out that the default configuration file had a serious bug. What if > > > a bug in our default configuration file has a serious security > > > implication? > > > > The answer to this is: if the broken templates/defaults are on > > /usr/share, it would be easy to deploy the fix. > > > > So, the compromise solution is: > > > > - We can move some configuration data (especially defaults/templates) > > to /usr/share (machine-types and CPU models could go there). This > > way we can easily deploy fixes to the defaults, if necessary. > > - To reuse Qemu models, or machine-types, and not define everything from > > scratch, libvirt will have to use something like: > > "-nodefconfig -readconfig /usr/share/qemu/cpu-models-x86.conf" > > > cpu-models-x86.conf is not a configuration file. It is hardware > description file. QEMU should not lose capability just because you run > it with -nodefconfig. -nodefconfig means that QEMU does not create > machine for you, but all parts needed to create a machine that would have > been created without -nodefconfig are still present. Not been able to > create Nehalem CPU after specifying -nodefconfig is the same as not been > able to create virtio-net i.e the bug. The current design direction Qemu seems to be following is different from that: hardware description is also considered "configuration" just like actual machine configuration. Anthony, please correct me if I am wrong. What you propose is to have two levels of "configuration" (or descriptions, or whatever we call it): 1) Hardware descriptions (or templates, or models, whatever we call it), that are not editable by the user (and not disabled by -nodefconfig). This may include CPU models, hardware emulation implemented in another language, machine-types, and other stuff that is part of "what Qemu always provides". 2) Actual machine configuration file, that is configurable and editable by the user, and normally loaded from /etc on from the command-line. The only problem is: the Qemu design simply doesn't have this distinction today (well, it _has_, the only difference is that today item (1) is almost completely coded inside tables in .c files). So if we want to go in that direction we have to agree this will be part of the Qemu design. I am not strongly inclined either way. Both approaches look good to me, we just have to decide where we are going, because we're are in this weird position todady because we never decided it explicitly, libvirt expected one thing, and we implemented something else. On the one hand I think the "two-layer" design gives us more freedom to move stuff outside .c files and change implementation details, and fits how we have been doing until today with machine types and built-in CPU models, keymaps, etc. On the other hand, I think not having this distinction between "machine configuration" and "hardware description" may be a good thing. For example: today there are two different ways of enabling a feature on a CPU: defining a new model, and adding a flag to "-cpu". And I think this asymmetry shouldn't be there: you just need a good system to define a CPU, a good set of defaults/templates, and a goo
Re: [Qemu-devel] [PATCH] kvm: allow arbitrarily sized mmio ioeventfd
On 03/20/2012 02:31 PM, Michael S. Tsirkin wrote: > We use a 2 byte ioeventfd for virtio memory, > add support for this. > > Applied, thanks. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH V2 0/4] MIPS ASE DSP Support for Qemu
This is the MIPS ASE DSP Support for Qemu. Signed-off-by: Jia Liu --- Version History: V2: Addressed Stefan's review comments: - fixed coding style. - changed acc into unsigned int form int and no initialization in translation. - added return value in testcases. V1: - add MIPS ASE DSP Support. Jia Liu (4): add MIPS DSP helpers define add MIPS DSP helpers implement add MIPS DSP translation add MIPS DSP testcase target-mips/helper.h | 152 + target-mips/op_helper.c| 3936 target-mips/translate.c| 1110 +++- tests/tcg/mips/mips32-dsp/Makefile | 133 + tests/tcg/mips/mips32-dsp/absq_s_ph.c | 30 + tests/tcg/mips/mips32-dsp/absq_s_w.c | 37 + tests/tcg/mips/mips32-dsp/addq_ph.c| 30 + tests/tcg/mips/mips32-dsp/addq_s_ph.c | 30 + tests/tcg/mips/mips32-dsp/addsc.c | 30 + tests/tcg/mips/mips32-dsp/addu_qb.c| 30 + tests/tcg/mips/mips32-dsp/addu_s_qb.c | 30 + tests/tcg/mips/mips32-dsp/addwc.c | 30 + tests/tcg/mips/mips32-dsp/bitrev.c | 20 + tests/tcg/mips/mips32-dsp/bposge32.c | 44 + tests/tcg/mips/mips32-dsp/cmp_eq_ph.c | 35 + tests/tcg/mips/mips32-dsp/cmp_le_ph.c | 35 + tests/tcg/mips/mips32-dsp/cmp_lt_ph.c | 35 + tests/tcg/mips/mips32-dsp/cmpgu_eq_qb.c| 31 + tests/tcg/mips/mips32-dsp/cmpgu_le_qb.c| 31 + tests/tcg/mips/mips32-dsp/cmpgu_lt_qb.c| 31 + tests/tcg/mips/mips32-dsp/cmpu_eq_qb.c | 35 + tests/tcg/mips/mips32-dsp/cmpu_le_qb.c | 35 + tests/tcg/mips/mips32-dsp/cmpu_lt_qb.c | 35 + tests/tcg/mips/mips32-dsp/dpaq_s_w_ph.c| 31 + tests/tcg/mips/mips32-dsp/dpaq_sa_l_w.c| 31 + tests/tcg/mips/mips32-dsp/dpau_h_qbl.c | 27 + tests/tcg/mips/mips32-dsp/dpau_h_qbr.c | 27 + tests/tcg/mips/mips32-dsp/dpsq_s_w_ph.c| 27 + tests/tcg/mips/mips32-dsp/dpsq_sa_l_w.c| 31 + tests/tcg/mips/mips32-dsp/dpsu_h_qbl.c | 27 + tests/tcg/mips/mips32-dsp/dpsu_h_qbr.c | 27 + tests/tcg/mips/mips32-dsp/extp.c | 44 + tests/tcg/mips/mips32-dsp/extpdp.c | 46 + tests/tcg/mips/mips32-dsp/extpdpv.c| 47 + tests/tcg/mips/mips32-dsp/extpv.c | 45 + tests/tcg/mips/mips32-dsp/extr_r_w.c | 25 + tests/tcg/mips/mips32-dsp/extr_rs_w.c | 25 + tests/tcg/mips/mips32-dsp/extr_s_h.c | 25 + tests/tcg/mips/mips32-dsp/extr_w.c | 25 + tests/tcg/mips/mips32-dsp/extrv_r_w.c | 29 + tests/tcg/mips/mips32-dsp/extrv_rs_w.c | 29 + tests/tcg/mips/mips32-dsp/extrv_s_h.c | 29 + tests/tcg/mips/mips32-dsp/extrv_w.c| 29 + tests/tcg/mips/mips32-dsp/insv.c | 23 + tests/tcg/mips/mips32-dsp/lbux.c | 23 + tests/tcg/mips/mips32-dsp/lhx.c| 23 + tests/tcg/mips/mips32-dsp/lwx.c| 23 + tests/tcg/mips/mips32-dsp/madd.c | 31 + tests/tcg/mips/mips32-dsp/maddu.c | 31 + tests/tcg/mips/mips32-dsp/maq_s_w_phl.c| 31 + tests/tcg/mips/mips32-dsp/maq_s_w_phr.c| 31 + tests/tcg/mips/mips32-dsp/maq_sa_w_phl.c | 31 + tests/tcg/mips/mips32-dsp/maq_sa_w_phr.c | 31 + tests/tcg/mips/mips32-dsp/mfhi.c | 21 + tests/tcg/mips/mips32-dsp/mflo.c | 21 + tests/tcg/mips/mips32-dsp/modsub.c | 30 + tests/tcg/mips/mips32-dsp/msub.c | 30 + tests/tcg/mips/mips32-dsp/msubu.c | 30 + tests/tcg/mips/mips32-dsp/mthi.c | 21 + tests/tcg/mips/mips32-dsp/mthlip.c | 34 + tests/tcg/mips/mips32-dsp/mtlo.c | 21 + tests/tcg/mips/mips32-dsp/muleq_s_w_phr.c | 40 + tests/tcg/mips/mips32-dsp/muleu_s_ph_qbl.c | 25 + tests/tcg/mips/mips32-dsp/muleu_s_ph_qbr.c | 25 + tests/tcg/mips/mips32-dsp/mulq_rs_ph.c | 25 + tests/tcg/mips/mips32-dsp/mult.c | 24 + tests/tcg/mips/mips32-dsp/multu.c | 24 + tests/tcg/mips/mips32-dsp/packrl_ph.c | 21 + tests/tcg/mips/mips32-dsp/pick_ph.c| 23 + tests/tcg/mips/mips32-dsp/pick_qb.c| 23 + tests/tcg/mips/mips32-dsp/preceq_w_phl.c | 20 + tests/tcg/mips/mips32-dsp/preceq_w_phr.c | 20 + tests/tcg/mips/mips32-dsp/precequ_ph_qbl.c | 20 + tests/tcg/mips/mips32-dsp/precequ_ph_qbla.c| 20 + tests/tcg/mips/mips32-dsp/precequ_ph_qbr.c | 20 + tests/tcg/mips/mips32-dsp/precequ_ph_qbra.c| 20 + tests/tcg/mips/mips32-dsp/preceu_ph_qbl.c | 20 + tests/tcg/mips/mips32-dsp/preceu_ph_qbla.c | 20 + tests/tcg/mips/mips32-dsp/preceu_ph_qbr.c | 20 + tests/tcg/mips/mips32-dsp/preceu_ph_
[Qemu-devel] [PATCH V2 1/4] add MIPS DSP helpers define
This is the helper define of MIPS ASE DSP. Signed-off-by: Jia Liu --- Version History: V2: - Addressed Stefan's review comments, use "one space after comma" codeing style. V1: - add MIPS ASE DSP helper define. target-mips/helper.h | 152 ++ 1 files changed, 152 insertions(+), 0 deletions(-) diff --git a/target-mips/helper.h b/target-mips/helper.h index 442f684..f814a89 100644 --- a/target-mips/helper.h +++ b/target-mips/helper.h @@ -297,4 +297,156 @@ DEF_HELPER_0(rdhwr_ccres, tl) DEF_HELPER_1(pmon, void, int) DEF_HELPER_0(wait, void) +/* MIPS32 DSP */ +DEF_HELPER_1(absqsph, i32, i32) +DEF_HELPER_1(absqsw, i32, i32) +DEF_HELPER_2(addqph, i32, i32, i32) +DEF_HELPER_2(addqsph, i32, i32, i32) +DEF_HELPER_2(addqsw, i32, i32, i32) +DEF_HELPER_2(addsc, i32, i32, i32) +DEF_HELPER_2(addwc, i32, i32, i32) +DEF_HELPER_1(bitrev, i32, i32) +DEF_HELPER_2(cmpeqph, void, i32, i32) +DEF_HELPER_2(cmpltph, void, i32, i32) +DEF_HELPER_2(cmpleph, void, i32, i32) +DEF_HELPER_2(cmpgueqqb, i32, i32, i32) +DEF_HELPER_2(cmpgultqb, i32, i32, i32) +DEF_HELPER_2(cmpguleqb, i32, i32, i32) +DEF_HELPER_2(cmpueqqb, void, i32, i32) +DEF_HELPER_2(cmpultqb, void, i32, i32) +DEF_HELPER_2(cmpuleqb, void, i32, i32) +DEF_HELPER_3(dpaqswph, void, int, i32, i32) +DEF_HELPER_3(dpaqsalw, void, int, i32, i32) +DEF_HELPER_3(dpauhqbl, void, int, i32, i32) +DEF_HELPER_3(dpauhqbr, void, int, i32, i32) +DEF_HELPER_3(dpsqswph, void, int, i32, i32) +DEF_HELPER_3(dpsqsalw, void, int, i32, i32) +DEF_HELPER_3(dpsuhqbl, void, int, i32, i32) +DEF_HELPER_3(dpsuhqbr, void, int, i32, i32) +DEF_HELPER_3(extp, void, int, int, int) +DEF_HELPER_3(extpdp, void, int, int, int) +DEF_HELPER_3(extpdpv, void, int, i32, int) +DEF_HELPER_3(extpv, void, int, i32, int) +DEF_HELPER_3(extrsh, void, int, int, int) +DEF_HELPER_3(extrw, void, int, int, int) +DEF_HELPER_3(extrrw, void, int, int, int) +DEF_HELPER_3(extrrsw, void, int, int, int) +DEF_HELPER_2(extrvsh, i32, int, i32); +DEF_HELPER_3(extrvw, void, int, i32, int) +DEF_HELPER_3(extrvrw, void, int, i32, int) +DEF_HELPER_3(extrvrsw, void, int, i32, int) +DEF_HELPER_3(insv, void, int, i32, i32) +DEF_HELPER_2(lbux, tl, tl, int) +DEF_HELPER_2(lhx, i32, i32, int) +DEF_HELPER_2(lwx, i32, i32, int) +DEF_HELPER_3(maqswphl, void, int, i32, i32) +DEF_HELPER_3(maqsawphl, void, int, i32, i32) +DEF_HELPER_3(maqswphr, void, int, i32, i32) +DEF_HELPER_3(maqsawphr, void, int, i32, i32) +DEF_HELPER_2(modsub, i32, i32, i32) +DEF_HELPER_2(mthlip, void, int, i32) +DEF_HELPER_2(muleqswphl, i32, i32, i32) +DEF_HELPER_2(muleqswphr, i32, i32, i32) +DEF_HELPER_2(muleusphqbl, i32, i32, i32) +DEF_HELPER_2(muleusphqbr, i32, i32, i32) +DEF_HELPER_2(mulqrsph, i32, i32, i32) +DEF_HELPER_3(mulsaqswph, void, int, i32, i32) +DEF_HELPER_2(packrlph, i32, i32, i32) +DEF_HELPER_2(pickqb, i32, i32, i32) +DEF_HELPER_1(preceqwphl, i32, i32) +DEF_HELPER_2(pickph, i32, i32, i32) +DEF_HELPER_1(preceqwphr, i32, i32) +DEF_HELPER_1(precequphqbl, i32, i32) +DEF_HELPER_1(precequphqbla, i32, i32) +DEF_HELPER_1(precequphqbr, i32, i32) +DEF_HELPER_1(precequphqbra, i32, i32) +DEF_HELPER_1(preceuphqbl, i32, i32) +DEF_HELPER_1(preceuphqbla, i32, i32) +DEF_HELPER_1(preceuphqbr, i32, i32) +DEF_HELPER_1(preceuphqbra, i32, i32) +DEF_HELPER_2(precrqqbph, i32, i32, i32) +DEF_HELPER_2(precrqphw, i32, i32, i32) +DEF_HELPER_2(precrqrsphw, i32, i32, i32) +DEF_HELPER_2(precrqusqbph, i32, i32, i32) +DEF_HELPER_1(radduwqb, i32, i32) +DEF_HELPER_1(rddsp, i32, i32) +DEF_HELPER_1(replph, i32, i32) +DEF_HELPER_1(replqb, i32, i32) +DEF_HELPER_1(replvph, i32, i32) +DEF_HELPER_1(replvqb, i32, i32) +DEF_HELPER_2(shilo, void, int, int) +DEF_HELPER_2(shilov, void, int, i32) +DEF_HELPER_2(shllph, i32, int, i32) +DEF_HELPER_2(shllsph, i32, int, i32) +DEF_HELPER_2(shllqb, i32, int, i32) +DEF_HELPER_2(shllsw, i32, int, i32) +DEF_HELPER_2(shllvph, i32, i32, i32) +DEF_HELPER_2(shllvsph, i32, i32, i32) +DEF_HELPER_2(shllvqb, i32, i32, i32) +DEF_HELPER_2(shllvsw, i32, i32, i32) +DEF_HELPER_2(shraph, i32, int, i32) +DEF_HELPER_2(shrarph, i32, int, i32) +DEF_HELPER_2(shrarw, i32, int, i32) +DEF_HELPER_2(shravph, i32, i32, i32) +DEF_HELPER_2(shravrph, i32, i32, i32) +DEF_HELPER_2(shravrw, i32, i32, i32) +DEF_HELPER_2(shrlqb, i32, int, i32) +DEF_HELPER_2(shrlvqb, i32, i32, i32) +DEF_HELPER_2(subqph, i32, i32, i32) +DEF_HELPER_2(subqsph, i32, i32, i32) +DEF_HELPER_2(subqsw, i32, i32, i32) +DEF_HELPER_2(subuqb, i32, i32, i32) +DEF_HELPER_2(subusqb, i32, i32, i32) +DEF_HELPER_2(wrdsp, void, i32, int) + +/* MIPS32 DSPR2 */ +DEF_HELPER_1(absqsqb, i32, i32) +DEF_HELPER_2(addqhph, i32, i32, i32) +DEF_HELPER_2(addqhrph, i32, i32, i32) +DEF_HELPER_2(addqhw, i32, i32, i32) +DEF_HELPER_2(addqhrw, i32, i32, i32) +DEF_HELPER_2(adduhqb, i32, i32, i32) +DEF_HELPER_2(adduhrqb, i32, i32, i32) +DEF_HELPER_2(adduph, i32, i32, i32) +DEF_HELPER_2(addusph, i32, i32, i32) +DEF_HELPER_2(adduqb, i32, i32, i32) +DEF_HELPER_2(addusqb, i32, i32, i32) +DEF_HELPER_3(append, i32, i32,
[Qemu-devel] [PATCH V2 3/4] add MIPS DSP translation
This is the translation of MIPS ASE DSP. Signed-off-by: Jia Liu --- Version History: V2: - Addressed Stefan's review comments, changed acc into unsigned int form int, and no initialization. - Coding style fixed. V1: - add MIPS ASE DSP translation. target-mips/translate.c | 1110 ++- 1 files changed, 1086 insertions(+), 24 deletions(-) diff --git a/target-mips/translate.c b/target-mips/translate.c index a663b74..0c6c7b0 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -249,6 +249,11 @@ enum { OPC_SYNCI= (0x1F << 16) | OPC_REGIMM, }; +/* REGIMM mipsdsp opcodes */ +enum { +OPC_BPOSGE32 = (0x1C << 16) | OPC_REGIMM, +}; + /* Special2 opcodes */ #define MASK_SPECIAL2(op) MASK_OP_MAJOR(op) | (op & 0x3F) @@ -312,6 +317,21 @@ enum { OPC_MODU_G_2E = 0x23 | OPC_SPECIAL3, OPC_DMOD_G_2E = 0x26 | OPC_SPECIAL3, OPC_DMODU_G_2E = 0x27 | OPC_SPECIAL3, + +/* MIPS DSP */ +OPC_ABSQ_S_PH_DSP = 0x12 | OPC_SPECIAL3, +OPC_ADDU_QB_DSP= 0x10 | OPC_SPECIAL3, +/* OPC_ADDUH_QB_DSP is same as OPC_MULT_G_2E. */ +/* OPC_ADDUH_QB_DSP = 0x18 | OPC_SPECIAL3, */ +OPC_APPEND_DSP = 0x31 | OPC_SPECIAL3, +OPC_CMPU_EQ_QB_DSP = 0x11 | OPC_SPECIAL3, +OPC_DPA_W_PH_DSP = 0x30 | OPC_SPECIAL3, +OPC_EXTR_W_DSP = 0x38 | OPC_SPECIAL3, +OPC_INSV_DSP = 0x0C | OPC_SPECIAL3, +OPC_LX_DSP = 0x0A | OPC_SPECIAL3, +/* OPC_MUL_PH_DSP is same as OPC_ADDUH_QB_DSP. */ +/* OPC_MUL_PH_DSP = 0x18 | OPC_SPECIAL3, */ +OPC_SHLL_QB_DSP= 0x13 | OPC_SPECIAL3, }; /* BSHFL opcodes */ @@ -331,6 +351,231 @@ enum { OPC_DSHD = (0x05 << 6) | OPC_DBSHFL, }; +#define MASK_ABSQ_S_PH(op) MASK_SPECIAL3(op) | (op & (0x1F << 6)) +/* MIPS DSP */ +enum { +OPC_ABSQ_S_PH = (0x09 << 6) | OPC_ABSQ_S_PH_DSP, +OPC_ABSQ_S_W= (0x11 << 6) | OPC_ABSQ_S_PH_DSP, +OPC_BITREV = (0x1B << 6) | OPC_ABSQ_S_PH_DSP, +OPC_PRECEQ_W_PHL= (0x0C << 6) | OPC_ABSQ_S_PH_DSP, +OPC_PRECEQ_W_PHR= (0x0D << 6) | OPC_ABSQ_S_PH_DSP, +OPC_PRECEQU_PH_QBL = (0x04 << 6) | OPC_ABSQ_S_PH_DSP, +OPC_PRECEQU_PH_QBLA = (0x06 << 6) | OPC_ABSQ_S_PH_DSP, +OPC_PRECEQU_PH_QBR = (0x05 << 6) | OPC_ABSQ_S_PH_DSP, +OPC_PRECEQU_PH_QBRA = (0x07 << 6) | OPC_ABSQ_S_PH_DSP, +OPC_PRECEU_PH_QBL = (0x1C << 6) | OPC_ABSQ_S_PH_DSP, +OPC_PRECEU_PH_QBLA = (0x1E << 6) | OPC_ABSQ_S_PH_DSP, +OPC_PRECEU_PH_QBR = (0x1D << 6) | OPC_ABSQ_S_PH_DSP, +OPC_PRECEU_PH_QBRA = (0x1F << 6) | OPC_ABSQ_S_PH_DSP, +OPC_REPL_PH = (0x0A << 6) | OPC_ABSQ_S_PH_DSP, +OPC_REPL_QB = (0x02 << 6) | OPC_ABSQ_S_PH_DSP, +OPC_REPLV_PH= (0x0B << 6) | OPC_ABSQ_S_PH_DSP, +OPC_REPLV_QB= (0x03 << 6) | OPC_ABSQ_S_PH_DSP, +}; + +/* MIPS DSPR2 */ +enum { +OPC_ABSQ_S_QB = (0x01 << 6) | OPC_ABSQ_S_PH_DSP, +}; + +#define MASK_ADDU_QB(op) MASK_SPECIAL3(op) | (op & (0x1F << 6)) +/* MIPS DSP */ +enum { +OPC_ADDQ_PH= (0x0A << 6) | OPC_ADDU_QB_DSP, +OPC_ADDQ_S_PH = (0x0E << 6) | OPC_ADDU_QB_DSP, +OPC_ADDQ_S_W = (0x16 << 6) | OPC_ADDU_QB_DSP, +OPC_ADDSC = (0x10 << 6) | OPC_ADDU_QB_DSP, +OPC_ADDU_QB= (0x00 << 6) | OPC_ADDU_QB_DSP, +OPC_ADDU_S_QB = (0x04 << 6) | OPC_ADDU_QB_DSP, +OPC_ADDWC = (0x11 << 6) | OPC_ADDU_QB_DSP, +OPC_MODSUB = (0x12 << 6) | OPC_ADDU_QB_DSP, +OPC_MULEQ_S_W_PHL = (0x1C << 6) | OPC_ADDU_QB_DSP, +OPC_MULEQ_S_W_PHR = (0x1D << 6) | OPC_ADDU_QB_DSP, +OPC_MULEU_S_PH_QBL = (0x06 << 6) | OPC_ADDU_QB_DSP, +OPC_MULEU_S_PH_QBR = (0x07 << 6) | OPC_ADDU_QB_DSP, +OPC_MULQ_RS_PH = (0x1F << 6) | OPC_ADDU_QB_DSP, +OPC_RADDU_W_QB = (0x14 << 6) | OPC_ADDU_QB_DSP, +OPC_SUBQ_PH= (0x0B << 6) | OPC_ADDU_QB_DSP, +OPC_SUBQ_S_PH = (0x0F << 6) | OPC_ADDU_QB_DSP, +OPC_SUBQ_S_W = (0x17 << 6) | OPC_ADDU_QB_DSP, +OPC_SUBU_QB= (0x01 << 6) | OPC_ADDU_QB_DSP, +OPC_SUBU_S_QB = (0x05 << 6) | OPC_ADDU_QB_DSP, +}; + +/* MIPS DSPR2 */ +enum { +OPC_ADDU_PH = (0x08 << 6) | OPC_ADDU_QB_DSP, +OPC_ADDU_S_PH = (0x0C << 6) | OPC_ADDU_QB_DSP, +OPC_MULQ_S_PH = (0x1E << 6) | OPC_ADDU_QB_DSP, +OPC_SUBU_PH = (0x09 << 6) | OPC_ADDU_QB_DSP, +OPC_SUBU_S_PH = (0x0D << 6) | OPC_ADDU_QB_DSP, +}; + +#define OPC_ADDUH_QB_DSP OPC_MULT_G_2E +#define MASK_ADDUH_QB(op) MASK_SPECIAL3(op) | (op & (0x1F << 6)) +/* MIPS DSPR2 */ +enum { +OPC_ADDQH_PH = (0x08 << 6) | OPC_ADDUH_QB_DSP, +OPC_ADDQH_R_PH = (0x0A << 6) | OPC_ADDUH_QB_DSP, +OPC_ADDQH_W= (0x10 << 6) | OPC_ADDUH_QB_DSP, +OPC_ADDQH_R_W = (0x12 << 6) | OPC_ADDUH_QB_DSP, +OPC_ADDUH_QB = (0x00 << 6) | OPC_ADDUH_QB_DSP, +OPC_ADDUH_R_QB = (0x02 << 6) | OPC_ADDUH_QB_DSP, +OPC_MUL_PH = (0x0C << 6) | OPC_ADDUH_QB_DSP, +OPC_MUL_S_PH = (0x0E << 6) | OPC_ADDUH_QB_DSP, +OPC_SUBQH_PH = (0x09 <<
[Qemu-devel] [PATCH 2/2] ui/spice-display: use uintptr_t when casting qxl physical addresses
From: Alon Levy The current intptr_t casts are a problem when the address's highest bit is 1, and it is cast to a intptr_t and then to uint64_t, such as at: surface.mem= (intptr_t)ssd->buf; This causes the sign bit to be extended which causes a wrong address to be passed on to spice, which then complains when it gets the wrong slot_id number, since the slot_id is taken from the higher bits. The assertion happens early - during the first primary surface creation. This fixes running "-vga qxl -spice" with 32 bit compiled qemu-system-i386. Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann --- ui/spice-display.c | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ui/spice-display.c b/ui/spice-display.c index 6d7563f..cb8a7ad 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -168,7 +168,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) drawable->bbox= ssd->dirty; drawable->clip.type = SPICE_CLIP_TYPE_NONE; drawable->effect = QXL_EFFECT_OPAQUE; -drawable->release_info.id = (intptr_t)update; +drawable->release_info.id = (uintptr_t)update; drawable->type= QXL_DRAW_COPY; drawable->surfaces_dest[0] = -1; drawable->surfaces_dest[1] = -1; @@ -179,7 +179,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) + time_space.tv_nsec / 1000 / 1000; drawable->u.copy.rop_descriptor = SPICE_ROPD_OP_PUT; -drawable->u.copy.src_bitmap = (intptr_t)image; +drawable->u.copy.src_bitmap = (uintptr_t)image; drawable->u.copy.src_area.right = bw; drawable->u.copy.src_area.bottom = bh; @@ -189,7 +189,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) image->bitmap.stride = bw * 4; image->descriptor.width = image->bitmap.x = bw; image->descriptor.height = image->bitmap.y = bh; -image->bitmap.data = (intptr_t)(update->bitmap); +image->bitmap.data = (uintptr_t)(update->bitmap); image->bitmap.palette = 0; image->bitmap.format = SPICE_BITMAP_FMT_32BIT; @@ -210,7 +210,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) } cmd->type = QXL_CMD_DRAW; -cmd->data = (intptr_t)drawable; +cmd->data = (uintptr_t)drawable; memset(&ssd->dirty, 0, sizeof(ssd->dirty)); return update; @@ -254,7 +254,7 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd) surface.mouse_mode = true; surface.flags = 0; surface.type = 0; -surface.mem= (intptr_t)ssd->buf; +surface.mem= (uintptr_t)ssd->buf; surface.group_id = MEMSLOT_GROUP_HOST; qemu_spice_create_primary_surface(ssd, 0, &surface, QXL_SYNC); -- 1.7.1
[Qemu-devel] [PULL 0/2] spice: 32bit support
Hi, This patch series fixes the remaining 32bit bugs in spice. With this series merged and the latest spice-server bits (from git, no release yet) spice works on 32bit hosts too. please pull, Gerd The following changes since commit 33cf629a3754b58a1e2dbbe01d91d97e712b7c06: Merge remote-tracking branch 'sstabellini/saverestore-8' into staging (2012-03-19 13:39:42 -0500) are available in the git repository at: git://anongit.freedesktop.org/spice/qemu spice.v51 Alon Levy (1): ui/spice-display: use uintptr_t when casting qxl physical addresses Peter Maydell (1): ui/spice-display.c: Fix compilation warnings on 32 bit hosts ui/spice-display.c | 22 +++--- 1 files changed, 11 insertions(+), 11 deletions(-)
[Qemu-devel] [PATCH 1/2] ui/spice-display.c: Fix compilation warnings on 32 bit hosts
From: Peter Maydell Fix compilation failures ("cast from pointer to integer of different size [-Werror=pointer-to-int-cast]") by using uintptr_t instead. Signed-off-by: Peter Maydell Signed-off-by: Gerd Hoffmann --- ui/spice-display.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ui/spice-display.c b/ui/spice-display.c index 28d6d4a..6d7563f 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -80,8 +80,8 @@ void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot, if (async != QXL_SYNC) { spice_qxl_add_memslot_async(&ssd->qxl, memslot, -(uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, - QXL_IO_MEMSLOT_ADD_ASYNC)); +(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_MEMSLOT_ADD_ASYNC)); } else { ssd->worker->add_memslot(ssd->worker, memslot); } @@ -100,8 +100,8 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id, trace_qemu_spice_create_primary_surface(ssd->qxl.id, id, surface, async); if (async != QXL_SYNC) { spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, -(uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, - QXL_IO_CREATE_PRIMARY_ASYNC)); +(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_CREATE_PRIMARY_ASYNC)); } else { ssd->worker->create_primary_surface(ssd->worker, id, surface); } @@ -113,8 +113,8 @@ void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, trace_qemu_spice_destroy_primary_surface(ssd->qxl.id, id, async); if (async != QXL_SYNC) { spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, -(uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, - QXL_IO_DESTROY_PRIMARY_ASYNC)); +(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_DESTROY_PRIMARY_ASYNC)); } else { ssd->worker->destroy_primary_surface(ssd->worker, id); } -- 1.7.1
Re: [Qemu-devel] [PATCH 1/4] add MIPS DSP helpers define
On Wed, Mar 21, 2012 at 4:17 PM, Stefan Weil wrote: > Am 12.03.2012 09:32, schrieb Jia Liu: > >> >> This patch is the helper define of MIPS ASE DSP. >> >> Signed-off-by: Jia Liu >> --- >> target-mips/helper.h | 152 >> ++ >> 1 files changed, 152 insertions(+), 0 deletions(-) >> >> diff --git a/target-mips/helper.h b/target-mips/helper.h >> index 442f684..1abf582 100644 >> --- a/target-mips/helper.h >> +++ b/target-mips/helper.h >> @@ -297,4 +297,156 @@ DEF_HELPER_0(rdhwr_ccres, tl) >> DEF_HELPER_1(pmon, void, int) >> DEF_HELPER_0(wait, void) >> >> +/* MIPS32 DSP */ >> +DEF_HELPER_1(absqsph, i32, i32) >> +DEF_HELPER_1(absqsw, i32, i32) >> +DEF_HELPER_2(addqph, i32, i32, i32) >> +DEF_HELPER_2(addqsph, i32, i32, i32) > > [snip] > > Hi, > > I know that a lot of people love such tabulated code, but I personally > would prefer to see those DEF_HELPER lines using the style which > is used in the existing code (one space after comma). > Hi Stefan, Thank you for review. I've fixed it, addressed your comments. Regards, Jia. > Regards, > Stefan Weil >
Re: [Qemu-devel] [PATCH] fix '-cpu ?' Segfault
Am 22.03.2012 14:28, schrieb Stefan Berger: > On 03/21/2012 08:33 AM, Eduardo Habkost wrote: >> Fix stupid copy&paste mistake at commit >> ecf40beae7dcbb057d4f115207f9d8276832a774: I moved code around but kept >> "optarg" on the cpu_list() call. >> >> Reported-by: Jiri Denemark >> Signed-off-by: Eduardo Habkost >> --- >> vl.c |2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index 112b0e0..0fccf50 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -3196,7 +3196,7 @@ int main(int argc, char **argv, char **envp) >> cpudef_init(); >> >> if (cpu_model&& *cpu_model == '?') { >> -list_cpus(stdout,&fprintf, optarg); >> +list_cpus(stdout,&fprintf, cpu_model); >> exit(0); >> } >> > Does -cpu ? actually work then? I only see the list of cpus when using > -cpu ?_ for example. You're right, it should be cpu_model + 1 from what I recall, i.e. just the optional argument, not the whole model string. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH V2 2/4] add MIPS DSP helpers implement
On 22 March 2012 13:36, Jia Liu wrote: > This is the helper implementation of MIPS ASE DSP. > > Signed-off-by: Jia Liu git am complains about this patch: Applying: add MIPS DSP helpers implement /home/petmay01/linaro/qemu-from-laptop/qemu/.git/rebase-apply/patch:17: new blank line at EOF. + warning: 1 line adds whitespace errors. -- PMM
Re: [Qemu-devel] [PATCH] usb/vmstate: add parent dev path
Am 22.03.2012 13:07, schrieb Gerd Hoffmann: > ... to make vmstate id string truely unique with multiple host > controllers, i.e. move from "1/usb-ptr" to ":00:01.3/1/usb-ptr" > (usb tabled connected to piix3 uhci). > > This obviously breaks migration. To handle this the usb bus > property "full-path" is added. When setting this to false old > behavior is maintained. This way current qemu will be compatible > with old versions when started using '-M pc-$oldversion'. > > Signed-off-by: Gerd Hoffmann > --- > hw/pc_piix.c | 28 > hw/usb.h |5 + > hw/usb/bus.c | 18 +- > 3 files changed, 50 insertions(+), 1 deletions(-) > > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 3f99f9a..8cb7d5f 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c > @@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_0 = { > .driver = "isa-fdc", > .property = "check_media_rate", > .value= "off", > +},{ > +.driver = "USB", > +.property = "full-path", > +.value= "no", This touches on our "favorite" bit/bool topic again. While I agree that "no" makes sense for a property of that name, the current code still expects "on" and "off". In particular, "yes" would not work as expected. We should either merge support for yes/no or use "off" here. I had a patch for the former but didn't resubmit after the QOM merge since I thought we would want to move this from qdev to Object first. Andreas > }, > { /* end of list */ } > }, > @@ -405,6 +409,10 @@ static QEMUMachine pc_machine_v0_15 = { > .driver = "isa-fdc", > .property = "check_media_rate", > .value= "off", > +},{ > +.driver = "USB", > +.property = "full-path", > +.value= "no", > }, > { /* end of list */ } > }, > @@ -449,6 +457,10 @@ static QEMUMachine pc_machine_v0_14 = { > .driver = "pc-sysfw", > .property = "rom_only", > .value= stringify(1), > +},{ > +.driver = "USB", > +.property = "full-path", > +.value= "no", > }, > { /* end of list */ } > }, > @@ -505,6 +517,10 @@ static QEMUMachine pc_machine_v0_13 = { > .driver = "pc-sysfw", > .property = "rom_only", > .value= stringify(1), > +},{ > +.driver = "USB", > +.property = "full-path", > +.value= "no", > }, > { /* end of list */ } > }, > @@ -565,6 +581,10 @@ static QEMUMachine pc_machine_v0_12 = { > .driver = "pc-sysfw", > .property = "rom_only", > .value= stringify(1), > +},{ > +.driver = "USB", > +.property = "full-path", > +.value= "no", > }, > { /* end of list */ } > } > @@ -633,6 +653,10 @@ static QEMUMachine pc_machine_v0_11 = { > .driver = "pc-sysfw", > .property = "rom_only", > .value= stringify(1), > +},{ > +.driver = "USB", > +.property = "full-path", > +.value= "no", > }, > { /* end of list */ } > } > @@ -713,6 +737,10 @@ static QEMUMachine pc_machine_v0_10 = { > .driver = "pc-sysfw", > .property = "rom_only", > .value= stringify(1), > +},{ > +.driver = "USB", > +.property = "full-path", > +.value= "no", > }, > { /* end of list */ } > }, > diff --git a/hw/usb/bus.c b/hw/usb/bus.c > index d3f8358..f1e567b 100644 > --- a/hw/usb/bus.c > +++ b/hw/usb/bus.c > @@ -19,6 +19,8 @@ static struct BusInfo usb_bus_info = { > .get_fw_dev_path = usb_get_fw_dev_path, > .props = (Property[]) { > DEFINE_PROP_STRING("port", USBDevice, port_path), > +DEFINE_PROP_BIT("full-path", USBDevice, flags, > +USB_DEV_FLAG_FULL_PATH, true), > DEFINE_PROP_END_OF_LIST() > }, > }; -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH V2 0/4] MIPS ASE DSP Support for Qemu
On 22 March 2012 13:36, Jia Liu wrote: > Jia Liu (4): > add MIPS DSP helpers define > add MIPS DSP helpers implement > add MIPS DSP translation > add MIPS DSP testcase You can't split these changes into patches like this. Every patch needs to be complete of itself, and in particular it has to compile cleanly at each intermediate point, not just after all patches are applied. If you apply just patch 1/4 then compilation fails: LINK mips-softmmu/qemu-system-mips translate.o: In function `mips_tcg_init': /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/helper.h:301: undefined reference to `helper_absqsph' /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/helper.h:302: undefined reference to `helper_absqsw' /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/helper.h:303: undefined reference to `helper_addqph' [snipped long list of similar errors] It would be better to break it up as patches each of which adds support for a coherent bite-sized subset of these instructions (so each individual patch includes the helper function declaration, implementation and translate.c changes for a smaller number of instructions). You also need to test compilation on a 32 bit host, as that is currently broken. For example: /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/op_helper.c: In function ‘mipsdsp_sat_add_i32’: /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/op_helper.c:3565: error: integer constant is too large for ‘long’ type /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/op_helper.c: In function ‘mipsdsp_mul_q31_q31’: /home/petmay01/linaro/qemu-from-laptop/qemu/target-mips/op_helper.c:3874: error: integer constant is too large for ‘long’ type [and on for another long list of errors] Most of these seem to be that you need the "ULL" suffix for constants which are more than 32 bits wide. -- PMM
Re: [Qemu-devel] [PATCH V2 0/4] MIPS ASE DSP Support for Qemu
On 22 March 2012 13:36, Jia Liu wrote: > This is the MIPS ASE DSP Support for Qemu. > Jia Liu (4): > add MIPS DSP helpers define > add MIPS DSP helpers implement > add MIPS DSP translation > add MIPS DSP testcase Most of these patches generate lots of errors when run through scripts/checkpatch.pl, which you should fix. -- PMM
Re: [Qemu-devel] [PATCH] fix '-cpu ?' Segfault
On Thu, Mar 22, 2012 at 02:46:48PM +0100, Andreas Färber wrote: > Am 22.03.2012 14:28, schrieb Stefan Berger: > > On 03/21/2012 08:33 AM, Eduardo Habkost wrote: > >> Fix stupid copy&paste mistake at commit > >> ecf40beae7dcbb057d4f115207f9d8276832a774: I moved code around but kept > >> "optarg" on the cpu_list() call. > >> > >> Reported-by: Jiri Denemark > >> Signed-off-by: Eduardo Habkost > >> --- > >> vl.c |2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >> diff --git a/vl.c b/vl.c > >> index 112b0e0..0fccf50 100644 > >> --- a/vl.c > >> +++ b/vl.c > >> @@ -3196,7 +3196,7 @@ int main(int argc, char **argv, char **envp) > >> cpudef_init(); > >> > >> if (cpu_model&& *cpu_model == '?') { > >> -list_cpus(stdout,&fprintf, optarg); > >> +list_cpus(stdout,&fprintf, cpu_model); > >> exit(0); > >> } > >> > > Does -cpu ? actually work then? I only see the list of cpus when using > > -cpu ?_ for example. > > You're right, it should be cpu_model + 1 from what I recall, i.e. just > the optional argument, not the whole model string. No, list_cpus() expects the whole argument string, including the leading "?": void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg) { /* XXX: implement xxx_cpu_list for targets that still miss it */ #if defined(cpu_list_id) cpu_list_id(f, cpu_fprintf, optarg); #elif defined(cpu_list) cpu_list(f, cpu_fprintf); /* deprecated */ #endif } [...] #define cpu_list_id x86_cpu_list [...] void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg) { unsigned char model = !strcmp("?model", optarg); unsigned char dump = !strcmp("?dump", optarg); unsigned char cpuid = !strcmp("?cpuid", optarg); [...] -- Eduardo
Re: [Qemu-devel] [PATCH V2 4/7] qed: add bdrv_invalidate_cache to be called after incoming live migration
>It's not clear to me why we need to introduce this field to stash >flags values. bs->open_flags already has this information. >Originally this was introduced in 06d9260ffa9 ("qcow2: implement >bdrv_invalidate_cache (v2)") for qcow2. I wonder if that field is >necessary when we already have bs->open_flags. >What I don't like about s->flags is that it duplicates state *and* >it's done in each block driver that supports .bdrv_invalidate_cache(). > So I wonder if we can drop it? I added this flag after seeing the following code in in bdrv_open_common. /* * Clear flags that are internal to the block layer before opening the * image. */ open_flags = flags & ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING); This lead me to believe that bs->open_flags != open_flags.
Re: [Qemu-devel] [PATCH V2 4/7] qed: add bdrv_invalidate_cache to be called after incoming live migration
Il 22/03/2012 15:15, Benoît Canet ha scritto: > > I added this flag after seeing the following code in in bdrv_open_common. > > /* > * Clear flags that are internal to the block layer before opening the > * image. > */ > open_flags = flags & ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING); > > This lead me to believe that bs->open_flags != open_flags. Correct, see the first two patches in the series at http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg03467.html Paolo
Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt
On Thu, Mar 22, 2012 at 10:31:21AM -0300, Eduardo Habkost wrote: > On Thu, Mar 22, 2012 at 11:32:44AM +0200, Gleb Natapov wrote: > > On Tue, Mar 13, 2012 at 11:53:19AM -0300, Eduardo Habkost wrote: > > > So, trying to summarize what was discussed in the call: > > > > > > On Mon, Mar 12, 2012 at 10:08:10AM -0300, Eduardo Habkost wrote: > > > > > Let's say we moved CPU definitions to /usr/share/qemu/cpu-models.xml. > > > > > > > > > > Obviously, we'd want a command line option to be able to change that > > > > > location so we'd introduce -cpu-models PATH. > > > > > > > > > > But we want all of our command line options to be settable by the > > > > > global configuration file so we would have a cpu-model=PATH to the > > > > > configuration file. > > > > > > > > > > But why hard code a path when we can just set the default path in the > > > > > configuration file so let's avoid hard coding and just put > > > > > cpu-models=/usr/share/qemu/cpu-models.xml in the default > > > > > configuration file. > > > > > > > > We wouldn't do the above. > > > > > > > > -nodefconfig should disable the loading of files on /etc, but it > > > > shouldn't disable loading internal non-configurable data that we just > > > > happened to choose to store outside the qemu binary because it makes > > > > development easier. > > > > > > The statement above is the one not fulfilled by the compromise solution: > > > -nodefconfig would really disable the loading of files on /usr/share. > > > > > What does this mean? Will -nodefconfig disable loading of bios.bin, > > option roms, keymaps? > > Correcting myself: loading of _config_ files on /usr/share. ROM images > are opaque data to be presented to the guest somehow, just like a disk > image or kernel binary. But maybe keymaps will become "configuration" > someday, I really don't know. > Where do you draw the line between "opaque data" and configuration. CPU models are also something that is present to a guest somehow. Are you consider ROMs to be "opaque data" because they are binary and CPU models to be config just because it is ascii file? What if we pre-process CPU models into binary for QEMU to read will it magically stop being configuration? > > > > > > > > > Doing this would make it impossible to deploy fixes to users if we evern > > > > find out that the default configuration file had a serious bug. What if > > > > a bug in our default configuration file has a serious security > > > > implication? > > > > > > The answer to this is: if the broken templates/defaults are on > > > /usr/share, it would be easy to deploy the fix. > > > > > > So, the compromise solution is: > > > > > > - We can move some configuration data (especially defaults/templates) > > > to /usr/share (machine-types and CPU models could go there). This > > > way we can easily deploy fixes to the defaults, if necessary. > > > - To reuse Qemu models, or machine-types, and not define everything from > > > scratch, libvirt will have to use something like: > > > "-nodefconfig -readconfig /usr/share/qemu/cpu-models-x86.conf" > > > > > cpu-models-x86.conf is not a configuration file. It is hardware > > description file. QEMU should not lose capability just because you run > > it with -nodefconfig. -nodefconfig means that QEMU does not create > > machine for you, but all parts needed to create a machine that would have > > been created without -nodefconfig are still present. Not been able to > > create Nehalem CPU after specifying -nodefconfig is the same as not been > > able to create virtio-net i.e the bug. > > > The current design direction Qemu seems to be following is different > from that: hardware description is also considered "configuration" just > like actual machine configuration. Anthony, please correct me if I am > wrong. That's a bug. Why trying to rationalize it now instead of fixing it. It was fixed in RHEL by the same person who introduced it in upstream in the first place. He just forgot to send the fix upstream. Does bug that is present for a long time is promoted to a feature? > > > What you propose is to have two levels of "configuration" (or > descriptions, or whatever we call it): > > 1) Hardware descriptions (or templates, or models, whatever we call it), >that are not editable by the user (and not disabled by -nodefconfig). >This may include CPU models, hardware emulation implemented in >another language, machine-types, and other stuff that is part of >"what Qemu always provides". > 2) Actual machine configuration file, that is configurable and editable >by the user, and normally loaded from /etc on from the command-line. > > The only problem is: the Qemu design simply doesn't have this > distinction today (well, it _has_, the only difference is that today > item (1) is almost completely coded inside tables in .c files). So if we > want to go in that direction we have to agree this will be part of the > Qemu design. > > I am not strongly inclined ei
Re: [Qemu-devel] [PATCH 1/2] slirp: clean up conflicts with system headers
On 2012-03-22 01:02, Paolo Bonzini wrote: > Right now, slirp/slirp.h cannot include some system headers and, > indirectly, qemu_socket.h. Clean this up, and remove a duplicate > prototype that was introduced because of that. > > > Signed-off-by: Paolo Bonzini > --- > slirp/slirp.h |8 +--- > slirp/tcp.h | 21 +++-- > 2 files changed, 16 insertions(+), 13 deletions(-) > > diff --git a/slirp/slirp.h b/slirp/slirp.h > index 5033ee3..2098b20 100644 > --- a/slirp/slirp.h > +++ b/slirp/slirp.h > @@ -88,10 +88,6 @@ void *malloc(size_t arg); > void free(void *ptr); > #endif > > -#ifndef HAVE_INET_ATON > -int inet_aton(const char *cp, struct in_addr *ia); > -#endif > - > #include > #ifndef NO_UNIX_SOCKETS > #include > @@ -144,6 +140,7 @@ int inet_aton(const char *cp, struct in_addr *ia); > #include "debug.h" > > #include "qemu-queue.h" > +#include "qemu_socket.h" > > #include "libslirp.h" > #include "ip.h" > @@ -167,9 +164,6 @@ int inet_aton(const char *cp, struct in_addr *ia); > #include "bootp.h" > #include "tftp.h" > > -/* osdep.c */ > -int qemu_socket(int domain, int type, int protocol); > - > #define ETH_ALEN 6 > #define ETH_HLEN 14 > > diff --git a/slirp/tcp.h b/slirp/tcp.h > index b3817cb..8299603 100644 > --- a/slirp/tcp.h > +++ b/slirp/tcp.h > @@ -45,6 +45,7 @@ typedef uint32_t tcp_seq; > * TCP header. > * Per RFC 793, September, 1981. > */ > +#define tcphdr slirp_tcphdr Nice :). What about s/tcphdr/bsd_tcphdr/ or so for all slirp files? Even better would be enabling slirp to use an existing declaration. But that looks trickier in first sight. > struct tcphdr { > uint16_t th_sport; /* source port */ > uint16_t th_dport; /* destination port */ > @@ -58,12 +59,6 @@ struct tcphdr { > th_off:4; /* data offset */ > #endif > uint8_t th_flags; > -#define TH_FIN 0x01 > -#define TH_SYN 0x02 > -#define TH_RST 0x04 > -#define TH_PUSH 0x08 > -#define TH_ACK 0x10 > -#define TH_URG 0x20 > uint16_t th_win;/* window */ > uint16_t th_sum;/* checksum */ > uint16_t th_urp;/* urgent pointer */ > @@ -71,6 +66,16 @@ struct tcphdr { > > #include "tcp_var.h" > > +#ifndef TH_FIN > +#define TH_FIN 0x01 > +#define TH_SYN 0x02 > +#define TH_RST 0x04 > +#define TH_PUSH 0x08 > +#define TH_ACK 0x10 > +#define TH_URG 0x20 > +#endif > + > +#ifndef TCPOPT_EOL > #define TCPOPT_EOL 0 > #define TCPOPT_NOP 1 > #define TCPOPT_MAXSEG 2 > @@ -86,6 +91,7 @@ struct tcphdr { > > #define TCPOPT_TSTAMP_HDR\ > (TCPOPT_NOP<<24|TCPOPT_NOP<<16|TCPOPT_TIMESTAMP<<8|TCPOLEN_TIMESTAMP) > +#endif Is there no portable header that offers those defines for us? > > /* > * Default maximum segment size for TCP. > @@ -95,10 +101,13 @@ struct tcphdr { > * > * We make this 1460 because we only care about Ethernet in the qemu context. > */ > +#undef TCP_MSS > #define TCP_MSS 1460 > > +#undef TCP_MAXWIN > #define TCP_MAXWIN 65535 /* largest value for (unscaled) window > */ > > +#undef TCP_MAX_WINSHIFT > #define TCP_MAX_WINSHIFT 14 /* maximum window shift */ > > /* Same here. The direction is appreciated a lot, but I'm measuring only a moderate overall hack-level reduction. ;) Patch 2 looks ok, btw. Thanks, Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] usb/vmstate: add parent dev path
Hi, >> +.driver = "USB", >> +.property = "full-path", >> +.value= "no", > > This touches on our "favorite" bit/bool topic again. While I agree that > "no" makes sense for a property of that name, the current code still > expects "on" and "off". In particular, "yes" would not work as expected. "no" *does* work as expected though, at least it survived my tests, thats why I didn't notice. I can s/no/off/ for consistency, no problem. Merging your yes/no patch is fine with me too. cheers, Gerd
Re: [Qemu-devel] [PATCH] Whitespace clean up.
For one, while non-functional, this patch should probably be reviewed by the block maintainer to not interfere with already queued patches. Then the subject should indicate which part of code it touches, like "block: " or "block/raw-posix.c: ". If Kevin agrees to merge this patch I would suggest to amend the commit message saying that this is converting tabs to spaces. We don't usually place a period after the subject line (which happens to not be a sentence) but that's an irrelevant detail. :) Our policy has been to not apply formatting-only fixes without an obvious reason. Is there a patch following that depends on this one? That being said, the code movement looks correct. Andreas Am 21.03.2012 19:23, schrieb Peter Portante: > Signed-off-by: Peter Portante > --- > block/raw-posix.c | 28 ++-- > 1 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/block/raw-posix.c b/block/raw-posix.c > index 2d1bc13..719a590 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -112,7 +112,7 @@ > reopen it to see if the disk has been changed */ > #define FD_OPEN_TIMEOUT (10) > > -#define MAX_BLOCKSIZE4096 > +#define MAX_BLOCKSIZE 4096 > > typedef struct BDRVRawState { > int fd; > @@ -494,7 +494,7 @@ again: > #endif > if (!fstat(fd, &sb) && (S_IFCHR & sb.st_mode)) { > #ifdef DIOCGMEDIASIZE > - if (ioctl(fd, DIOCGMEDIASIZE, (off_t *)&size)) > +if (ioctl(fd, DIOCGMEDIASIZE, (off_t *)&size)) > #elif defined(DIOCGPART) > { > struct partinfo pi; > @@ -894,7 +894,7 @@ static int hdev_has_zero_init(BlockDriverState *bs) > > static BlockDriver bdrv_host_device = { > .format_name= "host_device", > -.protocol_name= "host_device", > +.protocol_name = "host_device", > .instance_size = sizeof(BDRVRawState), > .bdrv_probe_device = hdev_probe_device, > .bdrv_file_open = hdev_open, > @@ -903,12 +903,12 @@ static BlockDriver bdrv_host_device = { > .create_options = raw_create_options, > .bdrv_has_zero_init = hdev_has_zero_init, > > -.bdrv_aio_readv = raw_aio_readv, > -.bdrv_aio_writev = raw_aio_writev, > -.bdrv_aio_flush = raw_aio_flush, > +.bdrv_aio_readv = raw_aio_readv, > +.bdrv_aio_writev= raw_aio_writev, > +.bdrv_aio_flush = raw_aio_flush, > > .bdrv_truncate = raw_truncate, > -.bdrv_getlength = raw_getlength, > +.bdrv_getlength = raw_getlength, > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > > @@ -1015,7 +1015,7 @@ static BlockDriver bdrv_host_floppy = { > .format_name= "host_floppy", > .protocol_name = "host_floppy", > .instance_size = sizeof(BDRVRawState), > -.bdrv_probe_device = floppy_probe_device, > +.bdrv_probe_device = floppy_probe_device, > .bdrv_file_open = floppy_open, > .bdrv_close = raw_close, > .bdrv_create= hdev_create, > @@ -1024,10 +1024,10 @@ static BlockDriver bdrv_host_floppy = { > > .bdrv_aio_readv = raw_aio_readv, > .bdrv_aio_writev= raw_aio_writev, > -.bdrv_aio_flush = raw_aio_flush, > +.bdrv_aio_flush = raw_aio_flush, > > .bdrv_truncate = raw_truncate, > -.bdrv_getlength = raw_getlength, > +.bdrv_getlength = raw_getlength, > .bdrv_get_allocated_file_size > = raw_get_allocated_file_size, > > @@ -1114,7 +1114,7 @@ static BlockDriver bdrv_host_cdrom = { > .format_name= "host_cdrom", > .protocol_name = "host_cdrom", > .instance_size = sizeof(BDRVRawState), > -.bdrv_probe_device = cdrom_probe_device, > +.bdrv_probe_device = cdrom_probe_device, > .bdrv_file_open = cdrom_open, > .bdrv_close = raw_close, > .bdrv_create= hdev_create, > @@ -1123,7 +1123,7 @@ static BlockDriver bdrv_host_cdrom = { > > .bdrv_aio_readv = raw_aio_readv, > .bdrv_aio_writev= raw_aio_writev, > -.bdrv_aio_flush = raw_aio_flush, > +.bdrv_aio_flush = raw_aio_flush, > > .bdrv_truncate = raw_truncate, > .bdrv_getlength = raw_getlength, > @@ -1233,7 +1233,7 @@ static BlockDriver bdrv_host_cdrom = { > .format_name= "host_cdrom", > .protocol_name = "host_cdrom", > .instance_size = sizeof(BDRVRawState), > -.bdrv_probe_device = cdrom_probe_device, > +.bdrv_probe_device = cdrom_probe_device, > .bdrv_file_open = cdrom_open, > .bdrv_close = raw_close, > .bdrv_create= hdev_create, > @@ -1242,7 +1242,7 @@ static BlockDriver bdrv_host_cdrom = { > > .bdrv_aio_readv = raw_aio_readv, > .bdrv_aio_writev= raw_aio_writev, > -.bdrv_aio_flush = raw_aio_flush, > +.bdrv_aio_flush = raw_aio_flush, > > .bdrv_truncate = ra
[Qemu-devel] [PATCH] Small change to remove short 250us timeouts every other time through the wait loop.
Basically, the main wait loop calls qemu_run_all_timers() unconditionally. The first thing this routine used to do is to see if a timer had been serviced, and then reset the loop timeout to the next deadline. However, the new deadlines had not been calculated at that point, as qemu_run_timers() had not been called yet for each of the clocks. So qemu_rearm_alarm_timer() would end up with a negative or zero deadline, and default to setting a 250us timeout for the loop. As qemu_run_timers() is called for each clock, the real deadlines would be put in place, but because a loop timeout was already set, the loop timeout would not be changed. Once that 250us timeout fired, the real deadline would be used for the subsequent timeout. For idle VMs, this effectively doubles the number of times through the loop, doubling the number of select() system calls, timer calls, etc. putting added scheduling pressure on the kernel. And under cgroups, this really causes a big problem because the cgroup code does not scale well. By simply running the timers before trying to rearm the timer, we always rearm with a non-zero deadline, effectively halving the number of system calls. Signed-off-by: Peter Portante --- qemu-timer.c | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/qemu-timer.c b/qemu-timer.c index d7f56e5..b0845f1 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -472,16 +472,16 @@ void qemu_run_all_timers(void) { alarm_timer->pending = 0; +/* vm time timers */ +qemu_run_timers(vm_clock); +qemu_run_timers(rt_clock); +qemu_run_timers(host_clock); + /* rearm timer, if not periodic */ if (alarm_timer->expired) { alarm_timer->expired = 0; qemu_rearm_alarm_timer(alarm_timer); } - -/* vm time timers */ -qemu_run_timers(vm_clock); -qemu_run_timers(rt_clock); -qemu_run_timers(host_clock); } #ifdef _WIN32 -- 1.7.7.6
Re: [Qemu-devel] [PATCH] usb/vmstate: add parent dev path
Am 22.03.2012 15:37, schrieb Gerd Hoffmann: >>> +.driver = "USB", >>> +.property = "full-path", >>> +.value= "no", >> >> This touches on our "favorite" bit/bool topic again. While I agree that >> "no" makes sense for a property of that name, the current code still >> expects "on" and "off". In particular, "yes" would not work as expected. > > "no" *does* work as expected though, at least it survived my tests, > thats why I didn't notice. > > I can s/no/off/ for consistency, no problem. Merging your yes/no patch > is fine with me too. I stand corrected: Michael Roth's conversion of qdev properties to use visitors implicitly changed the parsing logic to cover yes/no and true/false as well. That's great news! So no change of .value is needed here. The only thing to note is that print_bit() will output the value as on/off. Sorry for the confusion, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH 00/10] apply volume on client side v5
Hi, This patch series implements client-side audio volume support. This reduces confusion of guest users when volume control is not effective (because mixemu is disabled or because client-side is muted and can't be unmuted by the guest..) Instead, the backend is responsible for applying volume giving the guest control over the full range of the client, and avoiding multiple level of volume/mute effects. Although I was mainly interested in having the Spice audio backend support, I also added PulseAudio backend support (which unfortunately requires full-blown API, even after quick discussion with upstream). v5: - code style fixes (spaces) - left out from patch series the RFC "remove PLIVE and PERIOD options" Marc-André Lureau (10): audio: add VOICE_VOLUME ctl audio: don't apply volume effect if backend has VOICE_VOLUME_CAP hw/ac97: remove USE_MIXER code hw/ac97: the volume mask is not only 0x1f hw/ac97: add support for volume control audio/spice: add support for volume control Do not use pa_simple PulseAudio API configure: pa_simple is not needed anymore Allow controlling volume with PulseAudio backend Enable mixemu by default, add runtime option audio/audio.c | 29 +++- audio/audio_int.h |6 + audio/audio_template.h |2 + audio/mixeng.c |6 - audio/paaudio.c| 476 +++- audio/spiceaudio.c | 41 configure | 14 +- hw/ac97.c | 139 +- hw/hda-audio.c |4 - 9 files changed, 561 insertions(+), 156 deletions(-) -- 1.7.7.6