Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Am 05.09.2012 22:46, schrieb Anthony Liguori: What do we do when the FSF comes out with the GPLv4 and relicenses again in an incompatible fashion? Do we do this exercise every couple of years? That's exactly why I suggested GPLv2+ because it was supposed to be a preparation for the future. I'm pretty sure I mentioned the potential GPLv4 back then! 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 4/4] kvm: i386: Add classic PCI device assignment
On Thu, Sep 6, 2012 at 8:44 AM, Avi Kivity a...@redhat.com wrote: On 09/05/2012 10:04 PM, Blue Swirl wrote: Reinventing a disassembler for ever growing x86 assembly is no fun. We can try linking to a disassembler library. I use udis86 to disassemble instructions in kvm tracepoints (http://udis86.git.sourceforge.net/git/gitweb.cgi?p=udis86/udis86;a=shortlog), it's maintained but not heavily so. I think commonality with KVM would be preferred. The library looks neat and based on changelog, more actively developed than BSD DDB. Of course for non-x86 we'd need to continue using binutils; this is about copying code vs. libraries, not about licensing. For most architectures, pre-GPLv3 binutils is good enough since the instruction set does not change anymore. Maybe only PPC and Sparc64 still change besides x86. New CPUs types more recent than 2007 will have problems. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 09/05/2012 10:04 PM, Blue Swirl wrote: Reinventing a disassembler for ever growing x86 assembly is no fun. We can try linking to a disassembler library. I use udis86 to disassemble instructions in kvm tracepoints (http://udis86.git.sourceforge.net/git/gitweb.cgi?p=udis86/udis86;a=shortlog), it's maintained but not heavily so. Of course for non-x86 we'd need to continue using binutils; this is about copying code vs. libraries, not about licensing. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Wed, Sep 05, 2012 at 06:26:54PM +0300, Avi Kivity wrote: On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). Would be nice to add a clarification in the header: people tend to copy boilerplate around. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Avi Kivity a...@redhat.com writes: On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). I don't want QEMU to be GPLv3. I don't like the terms of the GPLv3. I don't mind GPLv2+, if people want to share code from QEMU in GPLv3 projects, GPLv2+ enables that. But if new code is coming in and happens to be under GPLv2, that just means that the contribution cannot be used outside of QEMU in a GPLv3 project. That's fine and that's a decision for the submitter to make. Regards, Anthony Liguori -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 09/05/2012 06:41 PM, Anthony Liguori wrote: Avi Kivity a...@redhat.com writes: On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). I don't want QEMU to be GPLv3. I don't like the terms of the GPLv3. I don't mind GPLv2+, if people want to share code from QEMU in GPLv3 projects, GPLv2+ enables that. But if new code is coming in and happens to be under GPLv2, that just means that the contribution cannot be used outside of QEMU in a GPLv3 project. That's fine and that's a decision for the submitter to make. Makes sense. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Wed, Sep 5, 2012 at 3:41 PM, Anthony Liguori anth...@codemonkey.ws wrote: Avi Kivity a...@redhat.com writes: On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). I don't want QEMU to be GPLv3. I don't like the terms of the GPLv3. I don't mind GPLv2+, if people want to share code from QEMU in GPLv3 projects, GPLv2+ enables that. The advantage of 100% GPLv2+ (or other GPLv3 compatible) would be that QEMU could share code from GPLv3 projects, specifically latest binutils. Reinventing a disassembler for ever growing x86 assembly is no fun. But if new code is coming in and happens to be under GPLv2, that just means that the contribution cannot be used outside of QEMU in a GPLv3 project. That's fine and that's a decision for the submitter to make. This policy means that we are locked in with GPLv2. Regards, Anthony Liguori -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Tue, Sep 4, 2012 at 9:28 PM, Michael S. Tsirkin m...@redhat.com wrote: On Tue, Sep 04, 2012 at 07:27:32PM +, Blue Swirl wrote: On Tue, Sep 4, 2012 at 8:32 AM, Avi Kivity a...@redhat.com wrote: On 09/03/2012 10:32 PM, Blue Swirl wrote: On Mon, Sep 3, 2012 at 4:14 PM, Avi Kivity a...@redhat.com wrote: On 08/29/2012 11:27 AM, Markus Armbruster wrote: I don't see a point in making contributors avoid non-problems that might conceivably become trivial problems some day. Especially when there's no automated help with the avoiding. -Wpointer-arith +1 FWIW, I'm not in favour of enabling it, just pointing out that it exists. In general I prefer avoiding unnecessary use of extensions, but in this case the extension is trivial and improves readability. Void pointers are not so type safe as uint8_t pointers. casts are even worse. There's also little difference in readability between those in my opinion. here too, casts are worse for readability. I agree they are bad in both accounts, but in most cases it is possible to use different types consistently (like uint8_t * or char * instead of void *) without adding casts. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Blue Swirl blauwir...@gmail.com writes: On Wed, Sep 5, 2012 at 3:41 PM, Anthony Liguori anth...@codemonkey.ws wrote: Avi Kivity a...@redhat.com writes: On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). I don't want QEMU to be GPLv3. I don't like the terms of the GPLv3. I don't mind GPLv2+, if people want to share code from QEMU in GPLv3 projects, GPLv2+ enables that. The advantage of 100% GPLv2+ (or other GPLv3 compatible) would be that QEMU could share code from GPLv3 projects, specifically latest binutils. Reinventing a disassembler for ever growing x86 assembly is no fun. But we can't share code with Linux (like for virtio). Yes, the GPLv3 sucks and FSF screwed up massively not making it v2 compatible. Regards, Anthony Liguori But if new code is coming in and happens to be under GPLv2, that just means that the contribution cannot be used outside of QEMU in a GPLv3 project. That's fine and that's a decision for the submitter to make. This policy means that we are locked in with GPLv2. Regards, Anthony Liguori -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 09/05/2012 01:04 PM, Blue Swirl wrote: I don't mind GPLv2+, if people want to share code from QEMU in GPLv3 projects, GPLv2+ enables that. The advantage of 100% GPLv2+ (or other GPLv3 compatible) would be that QEMU could share code from GPLv3 projects, specifically latest binutils. Reinventing a disassembler for ever growing x86 assembly is no fun. Not quite right. If qemu is 100% GPLv2+ and binutils is GPLv3+, then binutils can borrow code from qemu and the result is that binutils is still GPLv3+; but in the converse direction, if qemu borrows code from binutils then qemu is no longer 100% GPLv2+ but becomes GPLv3+ by tainting. That is, requesting GPLv2+ allows qemu code to be reused elsewhere, but does not help qemu import external code that is not already GPLv2+. But if new code is coming in and happens to be under GPLv2, that just means that the contribution cannot be used outside of QEMU in a GPLv3 project. That's fine and that's a decision for the submitter to make. This policy means that we are locked in with GPLv2. I'm afraid we're already locked at GPLv2 (and not GPLv2+), for good or for bad. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Wed, Sep 5, 2012 at 7:22 PM, Anthony Liguori anth...@codemonkey.ws wrote: Blue Swirl blauwir...@gmail.com writes: On Wed, Sep 5, 2012 at 3:41 PM, Anthony Liguori anth...@codemonkey.ws wrote: Avi Kivity a...@redhat.com writes: On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). I don't want QEMU to be GPLv3. I don't like the terms of the GPLv3. I don't mind GPLv2+, if people want to share code from QEMU in GPLv3 projects, GPLv2+ enables that. The advantage of 100% GPLv2+ (or other GPLv3 compatible) would be that QEMU could share code from GPLv3 projects, specifically latest binutils. Reinventing a disassembler for ever growing x86 assembly is no fun. But we can't share code with Linux (like for virtio). It's a tradeoff between reimplementing disassembler without using binutils vs. reimplementing virtio without using Linux. Both have their problems and both are growing areas. Disassembler is a bit smaller and the basic function does not ever change. Yes, the GPLv3 sucks and FSF screwed up massively not making it v2 compatible. I sort of agree. They had their reasons, of course. Too bad binutils licensing is fully controlled by FSF, for us it would be enough if they had some sort of dual licensing scheme (GPLv3 + BSD for example) in place. Regards, Anthony Liguori But if new code is coming in and happens to be under GPLv2, that just means that the contribution cannot be used outside of QEMU in a GPLv3 project. That's fine and that's a decision for the submitter to make. This policy means that we are locked in with GPLv2. Regards, Anthony Liguori -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Wed, Sep 5, 2012 at 7:24 PM, Eric Blake ebl...@redhat.com wrote: On 09/05/2012 01:04 PM, Blue Swirl wrote: I don't mind GPLv2+, if people want to share code from QEMU in GPLv3 projects, GPLv2+ enables that. The advantage of 100% GPLv2+ (or other GPLv3 compatible) would be that QEMU could share code from GPLv3 projects, specifically latest binutils. Reinventing a disassembler for ever growing x86 assembly is no fun. Not quite right. If qemu is 100% GPLv2+ and binutils is GPLv3+, then binutils can borrow code from qemu and the result is that binutils is still GPLv3+; but in the converse direction, if qemu borrows code from binutils then qemu is no longer 100% GPLv2+ but becomes GPLv3+ by tainting. I don't see how this disagrees with what I wrote. GPLv2+ QEMU sharing code from GPLv3 would of course become GPLv3. That is, requesting GPLv2+ allows qemu code to be reused elsewhere, but does not help qemu import external code that is not already GPLv2+. Unless we demanded relicensing to GPLv2+ for all GPLv2 QEMU code and forbid new GPLv2 entries. But if new code is coming in and happens to be under GPLv2, that just means that the contribution cannot be used outside of QEMU in a GPLv3 project. That's fine and that's a decision for the submitter to make. This policy means that we are locked in with GPLv2. I'm afraid we're already locked at GPLv2 (and not GPLv2+), for good or for bad. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Blue Swirl blauwir...@gmail.com writes: On Wed, Sep 5, 2012 at 7:22 PM, Anthony Liguori anth...@codemonkey.ws wrote: Blue Swirl blauwir...@gmail.com writes: On Wed, Sep 5, 2012 at 3:41 PM, Anthony Liguori anth...@codemonkey.ws wrote: Avi Kivity a...@redhat.com writes: On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). I don't want QEMU to be GPLv3. I don't like the terms of the GPLv3. I don't mind GPLv2+, if people want to share code from QEMU in GPLv3 projects, GPLv2+ enables that. The advantage of 100% GPLv2+ (or other GPLv3 compatible) would be that QEMU could share code from GPLv3 projects, specifically latest binutils. Reinventing a disassembler for ever growing x86 assembly is no fun. But we can't share code with Linux (like for virtio). It's a tradeoff between reimplementing disassembler without using binutils vs. reimplementing virtio without using Linux. Both have their problems and both are growing areas. Disassembler is a bit smaller and the basic function does not ever change. Since binutils is the project with a copyright controlled by a single entity that decided to change their license such that it was no longer compatible with QEMU, as far as I'm concerned, it's a no brainer. We should care more about working with projects that are willing to cooperate with us. What do we do when the FSF comes out with the GPLv4 and relicenses again in an incompatible fashion? Do we do this exercise every couple of years? Yes, the GPLv3 sucks and FSF screwed up massively not making it v2 compatible. I sort of agree. They had their reasons, of course. Too bad binutils licensing is fully controlled by FSF, for us it would be enough if they had some sort of dual licensing scheme (GPLv3 + BSD for example) in place. I think it's clear that FSF controlled projects are not good places to try to share code with... Regards, Anthony Liguori Regards, Anthony Liguori But if new code is coming in and happens to be under GPLv2, that just means that the contribution cannot be used outside of QEMU in a GPLv3 project. That's fine and that's a decision for the submitter to make. This policy means that we are locked in with GPLv2. Regards, Anthony Liguori -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 09/03/2012 10:32 PM, Blue Swirl wrote: On Mon, Sep 3, 2012 at 4:14 PM, Avi Kivity a...@redhat.com wrote: On 08/29/2012 11:27 AM, Markus Armbruster wrote: I don't see a point in making contributors avoid non-problems that might conceivably become trivial problems some day. Especially when there's no automated help with the avoiding. -Wpointer-arith +1 FWIW, I'm not in favour of enabling it, just pointing out that it exists. In general I prefer avoiding unnecessary use of extensions, but in this case the extension is trivial and improves readability. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Tue, Sep 4, 2012 at 8:32 AM, Avi Kivity a...@redhat.com wrote: On 09/03/2012 10:32 PM, Blue Swirl wrote: On Mon, Sep 3, 2012 at 4:14 PM, Avi Kivity a...@redhat.com wrote: On 08/29/2012 11:27 AM, Markus Armbruster wrote: I don't see a point in making contributors avoid non-problems that might conceivably become trivial problems some day. Especially when there's no automated help with the avoiding. -Wpointer-arith +1 FWIW, I'm not in favour of enabling it, just pointing out that it exists. In general I prefer avoiding unnecessary use of extensions, but in this case the extension is trivial and improves readability. Void pointers are not so type safe as uint8_t pointers. There's also little difference in readability between those in my opinion. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Andreas Färber afaer...@suse.de writes: Am 28.08.2012 14:57, schrieb Anthony Liguori: Andreas Färber afaer...@suse.de writes: Hi, Am 27.08.2012 08:28, schrieb Jan Kiszka: From: Jan Kiszka jan.kis...@siemens.com This adds PCI device assignment for i386 targets using the classic KVM interfaces. This version is 100% identical to what is being maintained in qemu-kvm for several years and is supported by libvirt as well. It is expected to remain relevant for another couple of years until kernels without full-features and performance-wise equivalent VFIO support are obsolete. A refactoring to-do that should be done in-tree is to model MSI and MSI-X support via the generic PCI layer, similar to what VFIO is already doing for MSI-X. This should improve the correctness and clean up the code from duplicate logic. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/kvm/Makefile.objs |2 +- hw/kvm/pci-assign.c | 1929 ++ 2 files changed, 1930 insertions(+), 1 deletions(-) create mode 100644 hw/kvm/pci-assign.c [...] diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c new file mode 100644 index 000..9cce02c --- /dev/null +++ b/hw/kvm/pci-assign.c @@ -0,0 +1,1929 @@ +/* + * Copyright (c) 2007, Neocleus Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. The downside of accepting this into qemu.git is that it gets us a huge blob of GPLv2-only code without history of contributors for GPLv2+ relicensing... That is 100% okay. Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Tue, Sep 04, 2012 at 07:27:32PM +, Blue Swirl wrote: On Tue, Sep 4, 2012 at 8:32 AM, Avi Kivity a...@redhat.com wrote: On 09/03/2012 10:32 PM, Blue Swirl wrote: On Mon, Sep 3, 2012 at 4:14 PM, Avi Kivity a...@redhat.com wrote: On 08/29/2012 11:27 AM, Markus Armbruster wrote: I don't see a point in making contributors avoid non-problems that might conceivably become trivial problems some day. Especially when there's no automated help with the avoiding. -Wpointer-arith +1 FWIW, I'm not in favour of enabling it, just pointing out that it exists. In general I prefer avoiding unnecessary use of extensions, but in this case the extension is trivial and improves readability. Void pointers are not so type safe as uint8_t pointers. casts are even worse. There's also little difference in readability between those in my opinion. here too, casts are worse for readability. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 08/29/2012 11:49 AM, Peter Maydell wrote: On 29 August 2012 09:47, Jan Kiszka jan.kis...@siemens.com wrote: On 2012-08-28 23:26, Peter Maydell wrote: Since this is arch-specific we should probably give the resulting device a more specific name than pci-assign, which implies that it is (a) ok for any PCI system and (b) not something that will be obsolete in the foreseen future. The name has to be like this for libvirt compatibility. I can provide it as alias, though, calling the device kvm-pci-assign by default. Better? ...is it likely to ever work for non-x86 PCI devices? It used to work for ia64 before it died. So it's not architecture specific by design. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 08/28/2012 03:30 AM, Jan Kiszka wrote: Maybe add case 8: and default: with abort(), also below. PIO is never 8 bytes long, the generic layer protects us. Note: eventually the pio space will be mapped directly to mmio (instead of being bounced via cpu_inb() in the bridge's mmio handler), and so we will have to add this restriction to the memory core. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 08/29/2012 11:27 AM, Markus Armbruster wrote: I don't see a point in making contributors avoid non-problems that might conceivably become trivial problems some day. Especially when there's no automated help with the avoiding. -Wpointer-arith -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Mon, Sep 3, 2012 at 4:14 PM, Avi Kivity a...@redhat.com wrote: On 08/29/2012 11:27 AM, Markus Armbruster wrote: I don't see a point in making contributors avoid non-problems that might conceivably become trivial problems some day. Especially when there's no automated help with the avoiding. -Wpointer-arith +1 -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Mon, 2012-09-03 at 18:59 +0300, Avi Kivity wrote: On 08/29/2012 11:49 AM, Peter Maydell wrote: On 29 August 2012 09:47, Jan Kiszka jan.kis...@siemens.com wrote: On 2012-08-28 23:26, Peter Maydell wrote: Since this is arch-specific we should probably give the resulting device a more specific name than pci-assign, which implies that it is (a) ok for any PCI system and (b) not something that will be obsolete in the foreseen future. The name has to be like this for libvirt compatibility. I can provide it as alias, though, calling the device kvm-pci-assign by default. Better? ...is it likely to ever work for non-x86 PCI devices? It used to work for ia64 before it died. So it's not architecture specific by design. Caveat; ia64 is a different architecture, but it was on Intel VT-d chipsets, which therefore supported the IOMMU API and ia64 maps guests the same way. This is the same problem I ran into trying to name the vfio iommu driver for x86. It's not really x86-specific, but it's got to look and smell pretty similar. Thanks, Alex
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Tue, Aug 28, 2012 at 9:51 PM, Anthony Liguori anth...@codemonkey.ws wrote:
Blue Swirl blauwir...@gmail.com writes:
On Tue, Aug 28, 2012 at 7:31 PM, Anthony Liguori anth...@codemonkey.ws
wrote:
Blue Swirl blauwir...@gmail.com writes:
On Tue, Aug 28, 2012 at 5:28 PM, Michael S. Tsirkin m...@redhat.com
wrote:
On Tue, Aug 28, 2012 at 05:01:55PM +, Blue Swirl wrote:
On Tue, Aug 28, 2012 at 7:35 AM, Michael Tokarev m...@tls.msk.ru wrote:
On 27.08.2012 22:56, Blue Swirl wrote:
[]
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t
addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
There are a few places in common qemu code which does this for a very
long time. So I guess it is safe now.
It's a non-standard GCC extension.
So? We use many other GCC extensions. grep for typeof.
Dependencies should not be introduced trivially. In this case, it's
pretty easy to avoid void pointer arithmetic as Jan's next version
shows.
The standard is vague with respect void arithmetic. Most compilers
allow it. A very good analysis of the standard can be found below.
http://stackoverflow.com/questions/3523145/pointer-arithmetic-for-void-pointer-in-c
The analysis would seem to show that arithmetic may be acceptable, but
it doesn't say that void pointers must be treated like char pointers.
In my view, this would make sense:
char *cptr;
void *vptr;
Since
cptr++;
is equivalent to
cptr = (char *)((uintptr_t)cptr + sizeof(*cptr));
therefore
vptr++;
should be equivalent to
vptr = (void *)((uintptr_t)vptr + sizeof(*vptr));
That is, vptr++ should be equivalent to vptr += 0 because sizeof(void)
should be 0 if allowed.
sizeof(void) == 1
With GCC at least.
It's not valid C (0 is just how I think it should be if allowed). Also
GCC can reject it even with std=gnu89 (default, C89 with GNU
extensions):
$ cat void.c
unsigned long x = sizeof(void);
$ gcc -pedantic void.c -c
void.c:1: warning: invalid application of 'sizeof' to a void type
Regards,
Anthony Liguori
Regards,
Anthony Liguori
Is there a work in progress to build GCC with visual studio?
If yes what are the chances KVM device assignment
will work on windows?
IIRC there was really a project to use KVM on Windows and another
project to build QEMU with MSVC.
Look QEMU codebase is what it is. Unless you rework all existing
code to confirm to your taste, I do not see why you NACK valid new code
unless it confirms to same.
Yes, I'd be happy to fix the style with huge patches at once. But our
fearless leader does not agree, so we are stuck with the codebase
being what it is until it is fixed one step at a time.
/mjt
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Anthony Liguori anth...@codemonkey.ws writes:
Blue Swirl blauwir...@gmail.com writes:
On Tue, Aug 28, 2012 at 5:28 PM, Michael S. Tsirkin m...@redhat.com wrote:
On Tue, Aug 28, 2012 at 05:01:55PM +, Blue Swirl wrote:
On Tue, Aug 28, 2012 at 7:35 AM, Michael Tokarev m...@tls.msk.ru wrote:
On 27.08.2012 22:56, Blue Swirl wrote:
[]
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
There are a few places in common qemu code which does this for a very
long time. So I guess it is safe now.
It's a non-standard GCC extension.
So? We use many other GCC extensions. grep for typeof.
Dependencies should not be introduced trivially. In this case, it's
pretty easy to avoid void pointer arithmetic as Jan's next version
shows.
The standard is vague with respect void arithmetic. Most compilers
allow it. A very good analysis of the standard can be found below.
http://stackoverflow.com/questions/3523145/pointer-arithmetic-for-void-pointer-in-c
BTW: can we please stop arguing about C standards. If we currently are
using something in QEMU that's supported by clang and GCC, it's fine and
we ought to continue using it.
Seconded.
I don't see a point in making contributors avoid non-problems that might
conceivably become trivial problems some day. Especially when there's
no automated help with the avoiding.
[...]
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 2012-08-28 23:26, Peter Maydell wrote: On 27 August 2012 13:15, Jan Kiszka jan.kis...@siemens.com wrote: On 2012-08-27 14:07, Andreas Färber wrote: Am I correct to understand we compile this only for i386 / x86_64? This is correct. Did we ever make a decision about whether architecture specific KVM devices should all live in hw/kvm/ or in some directory with the arch name in it? I guess they're all in hw/kvm/ at the moment so they should stay there unless there's a pressing reason to move them around... Yep, that would be the plan. As long as there are x86 platform devices under hw/kvm, I see no point putting this one elsewhere. It's trivially moved when time has come. Since this is arch-specific we should probably give the resulting device a more specific name than pci-assign, which implies that it is (a) ok for any PCI system and (b) not something that will be obsolete in the foreseen future. The name has to be like this for libvirt compatibility. I can provide it as alias, though, calling the device kvm-pci-assign by default. Better? Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 29 August 2012 09:47, Jan Kiszka jan.kis...@siemens.com wrote: On 2012-08-28 23:26, Peter Maydell wrote: Since this is arch-specific we should probably give the resulting device a more specific name than pci-assign, which implies that it is (a) ok for any PCI system and (b) not something that will be obsolete in the foreseen future. The name has to be like this for libvirt compatibility. I can provide it as alias, though, calling the device kvm-pci-assign by default. Better? ...is it likely to ever work for non-x86 PCI devices? -- PMM
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 2012-08-29 10:49, Peter Maydell wrote: On 29 August 2012 09:47, Jan Kiszka jan.kis...@siemens.com wrote: On 2012-08-28 23:26, Peter Maydell wrote: Since this is arch-specific we should probably give the resulting device a more specific name than pci-assign, which implies that it is (a) ok for any PCI system and (b) not something that will be obsolete in the foreseen future. The name has to be like this for libvirt compatibility. I can provide it as alias, though, calling the device kvm-pci-assign by default. Better? ...is it likely to ever work for non-x86 PCI devices? Nope, but it also won't be exposed on them. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Am 28.08.2012 14:57, schrieb Anthony Liguori: Andreas Färber afaer...@suse.de writes: Hi, Am 27.08.2012 08:28, schrieb Jan Kiszka: From: Jan Kiszka jan.kis...@siemens.com This adds PCI device assignment for i386 targets using the classic KVM interfaces. This version is 100% identical to what is being maintained in qemu-kvm for several years and is supported by libvirt as well. It is expected to remain relevant for another couple of years until kernels without full-features and performance-wise equivalent VFIO support are obsolete. A refactoring to-do that should be done in-tree is to model MSI and MSI-X support via the generic PCI layer, similar to what VFIO is already doing for MSI-X. This should improve the correctness and clean up the code from duplicate logic. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/kvm/Makefile.objs |2 +- hw/kvm/pci-assign.c | 1929 ++ 2 files changed, 1930 insertions(+), 1 deletions(-) create mode 100644 hw/kvm/pci-assign.c [...] diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c new file mode 100644 index 000..9cce02c --- /dev/null +++ b/hw/kvm/pci-assign.c @@ -0,0 +1,1929 @@ +/* + * Copyright (c) 2007, Neocleus Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. The downside of accepting this into qemu.git is that it gets us a huge blob of GPLv2-only code without history of contributors for GPLv2+ relicensing... That is 100% okay. Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). If we want to get rid of qemu-kvm by pulling this in nontheless (for which I do have sympathies from a packaging perspective) why not instead do a git-merge from qemu-kvm so that we have the full file history at hands inside qemu.git? Regards, 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 4/4] kvm: i386: Add classic PCI device assignment
Andreas Färber afaer...@suse.de writes: Am 28.08.2012 14:57, schrieb Anthony Liguori: Andreas Färber afaer...@suse.de writes: Hi, Am 27.08.2012 08:28, schrieb Jan Kiszka: From: Jan Kiszka jan.kis...@siemens.com This adds PCI device assignment for i386 targets using the classic KVM interfaces. This version is 100% identical to what is being maintained in qemu-kvm for several years and is supported by libvirt as well. It is expected to remain relevant for another couple of years until kernels without full-features and performance-wise equivalent VFIO support are obsolete. A refactoring to-do that should be done in-tree is to model MSI and MSI-X support via the generic PCI layer, similar to what VFIO is already doing for MSI-X. This should improve the correctness and clean up the code from duplicate logic. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/kvm/Makefile.objs |2 +- hw/kvm/pci-assign.c | 1929 ++ 2 files changed, 1930 insertions(+), 1 deletions(-) create mode 100644 hw/kvm/pci-assign.c [...] diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c new file mode 100644 index 000..9cce02c --- /dev/null +++ b/hw/kvm/pci-assign.c @@ -0,0 +1,1929 @@ +/* + * Copyright (c) 2007, Neocleus Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. The downside of accepting this into qemu.git is that it gets us a huge blob of GPLv2-only code without history of contributors for GPLv2+ relicensing... That is 100% okay. Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). We are not rejecting patches just because aren't GPLv3 compatible. I'm okay with people trying to make QEMU GPLv2+ but we're not going to reject code that cannot be practically made GPLv2+. Making as much code as possible GPLv2+ means more projects can share code from QEMU. I have no desire to make QEMU GPLv3. I said this many times in the past. GPLv2 only is fine for contributions. Regards, Anthony Liguori If we want to get rid of qemu-kvm by pulling this in nontheless (for which I do have sympathies from a packaging perspective) why not instead do a git-merge from qemu-kvm so that we have the full file history at hands inside qemu.git? Regards, 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 4/4] kvm: i386: Add classic PCI device assignment
On 27.08.2012 22:56, Blue Swirl wrote:
[]
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
There are a few places in common qemu code which does this for a very
long time. So I guess it is safe now.
/mjt
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Andreas Färber afaer...@suse.de writes: Hi, Am 27.08.2012 08:28, schrieb Jan Kiszka: From: Jan Kiszka jan.kis...@siemens.com This adds PCI device assignment for i386 targets using the classic KVM interfaces. This version is 100% identical to what is being maintained in qemu-kvm for several years and is supported by libvirt as well. It is expected to remain relevant for another couple of years until kernels without full-features and performance-wise equivalent VFIO support are obsolete. A refactoring to-do that should be done in-tree is to model MSI and MSI-X support via the generic PCI layer, similar to what VFIO is already doing for MSI-X. This should improve the correctness and clean up the code from duplicate logic. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/kvm/Makefile.objs |2 +- hw/kvm/pci-assign.c | 1929 ++ 2 files changed, 1930 insertions(+), 1 deletions(-) create mode 100644 hw/kvm/pci-assign.c [...] diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c new file mode 100644 index 000..9cce02c --- /dev/null +++ b/hw/kvm/pci-assign.c @@ -0,0 +1,1929 @@ +/* + * Copyright (c) 2007, Neocleus Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. The downside of accepting this into qemu.git is that it gets us a huge blob of GPLv2-only code without history of contributors for GPLv2+ relicensing... That is 100% okay. Regards, Anthony Liguori + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple + * Place - Suite 330, Boston, MA 02111-1307 USA. (Expect the usual GNU address reminder here.) + * + * + * Assign a PCI device from the host to a guest VM. + * + * Adapted for KVM by Qumranet. + * + * Copyright (c) 2007, Neocleus, Alex Novik (a...@neocleus.com) + * Copyright (c) 2007, Neocleus, Guy Zana (g...@neocleus.com) + * Copyright (C) 2008, Qumranet, Amit Shah (amit.s...@qumranet.com) + * Copyright (C) 2008, Red Hat, Amit Shah (amit.s...@redhat.com) + * Copyright (C) 2008, IBM, Muli Ben-Yehuda (m...@il.ibm.com) + */ +#include stdio.h +#include unistd.h +#include sys/io.h +#include sys/mman.h +#include sys/types.h +#include sys/stat.h +#include hw/hw.h +#include hw/pc.h +#include qemu-error.h +#include console.h +#include hw/loader.h +#include monitor.h +#include range.h +#include sysemu.h +#include hw/pci.h +#include hw/msi.h +#include kvm_i386.h Am I correct to understand we compile this only for i386 / x86_64? (apic.o in kvm/Makefile.objs hints in that direction) You may want to update the description in the comment above accordingly, also mentioning that this is some deprecated backwards-compatibility thing. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Tue, Aug 28, 2012 at 7:35 AM, Michael Tokarev m...@tls.msk.ru wrote:
On 27.08.2012 22:56, Blue Swirl wrote:
[]
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
There are a few places in common qemu code which does this for a very
long time. So I guess it is safe now.
It's a non-standard GCC extension.
/mjt
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Tue, Aug 28, 2012 at 05:01:55PM +, Blue Swirl wrote:
On Tue, Aug 28, 2012 at 7:35 AM, Michael Tokarev m...@tls.msk.ru wrote:
On 27.08.2012 22:56, Blue Swirl wrote:
[]
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
There are a few places in common qemu code which does this for a very
long time. So I guess it is safe now.
It's a non-standard GCC extension.
So? We use many other GCC extensions. grep for typeof.
Is there a work in progress to build GCC with visual studio?
If yes what are the chances KVM device assignment
will work on windows?
Look QEMU codebase is what it is. Unless you rework all existing
code to confirm to your taste, I do not see why you NACK valid new code
unless it confirms to same.
/mjt
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Tue, Aug 28, 2012 at 5:28 PM, Michael S. Tsirkin m...@redhat.com wrote:
On Tue, Aug 28, 2012 at 05:01:55PM +, Blue Swirl wrote:
On Tue, Aug 28, 2012 at 7:35 AM, Michael Tokarev m...@tls.msk.ru wrote:
On 27.08.2012 22:56, Blue Swirl wrote:
[]
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
There are a few places in common qemu code which does this for a very
long time. So I guess it is safe now.
It's a non-standard GCC extension.
So? We use many other GCC extensions. grep for typeof.
Dependencies should not be introduced trivially. In this case, it's
pretty easy to avoid void pointer arithmetic as Jan's next version shows.
Is there a work in progress to build GCC with visual studio?
If yes what are the chances KVM device assignment
will work on windows?
IIRC there was really a project to use KVM on Windows and another
project to build QEMU with MSVC.
Look QEMU codebase is what it is. Unless you rework all existing
code to confirm to your taste, I do not see why you NACK valid new code
unless it confirms to same.
Yes, I'd be happy to fix the style with huge patches at once. But our
fearless leader does not agree, so we are stuck with the codebase
being what it is until it is fixed one step at a time.
/mjt
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Blue Swirl blauwir...@gmail.com writes:
On Tue, Aug 28, 2012 at 5:28 PM, Michael S. Tsirkin m...@redhat.com wrote:
On Tue, Aug 28, 2012 at 05:01:55PM +, Blue Swirl wrote:
On Tue, Aug 28, 2012 at 7:35 AM, Michael Tokarev m...@tls.msk.ru wrote:
On 27.08.2012 22:56, Blue Swirl wrote:
[]
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
There are a few places in common qemu code which does this for a very
long time. So I guess it is safe now.
It's a non-standard GCC extension.
So? We use many other GCC extensions. grep for typeof.
Dependencies should not be introduced trivially. In this case, it's
pretty easy to avoid void pointer arithmetic as Jan's next version
shows.
The standard is vague with respect void arithmetic. Most compilers
allow it. A very good analysis of the standard can be found below.
http://stackoverflow.com/questions/3523145/pointer-arithmetic-for-void-pointer-in-c
BTW: can we please stop arguing about C standards. If we currently are
using something in QEMU that's supported by clang and GCC, it's fine and
we ought to continue using it.
The reserved names actually did bite us when porting to a new platform.
But the only requirement for C extensions ought to be reasonable support
in GCC and clang.
I don't care at all about supporting proprietary compilers.
Regards,
Anthony Liguori
Is there a work in progress to build GCC with visual studio?
If yes what are the chances KVM device assignment
will work on windows?
IIRC there was really a project to use KVM on Windows and another
project to build QEMU with MSVC.
Look QEMU codebase is what it is. Unless you rework all existing
code to confirm to your taste, I do not see why you NACK valid new code
unless it confirms to same.
Yes, I'd be happy to fix the style with huge patches at once. But our
fearless leader does not agree, so we are stuck with the codebase
being what it is until it is fixed one step at a time.
/mjt
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Tue, 28 Aug 2012, Anthony Liguori wrote:
Blue Swirl blauwir...@gmail.com writes:
On Tue, Aug 28, 2012 at 5:28 PM, Michael S. Tsirkin m...@redhat.com wrote:
On Tue, Aug 28, 2012 at 05:01:55PM +, Blue Swirl wrote:
On Tue, Aug 28, 2012 at 7:35 AM, Michael Tokarev m...@tls.msk.ru wrote:
On 27.08.2012 22:56, Blue Swirl wrote:
[]
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
There are a few places in common qemu code which does this for a very
long time. So I guess it is safe now.
It's a non-standard GCC extension.
So? We use many other GCC extensions. grep for typeof.
Dependencies should not be introduced trivially. In this case, it's
pretty easy to avoid void pointer arithmetic as Jan's next version
shows.
The standard is vague with respect void arithmetic. Most compilers
allow it. A very good analysis of the standard can be found below.
http://stackoverflow.com/questions/3523145/pointer-arithmetic-for-void-pointer-in-c
BTW: can we please stop arguing about C standards. If we currently are
using something in QEMU that's supported by clang and GCC, it's fine and
we ought to continue using it.
No we can not stop arguing. Besides you are wrong.
[..snip..]
--
mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Tue, Aug 28, 2012 at 7:31 PM, Anthony Liguori anth...@codemonkey.ws wrote:
Blue Swirl blauwir...@gmail.com writes:
On Tue, Aug 28, 2012 at 5:28 PM, Michael S. Tsirkin m...@redhat.com wrote:
On Tue, Aug 28, 2012 at 05:01:55PM +, Blue Swirl wrote:
On Tue, Aug 28, 2012 at 7:35 AM, Michael Tokarev m...@tls.msk.ru wrote:
On 27.08.2012 22:56, Blue Swirl wrote:
[]
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
There are a few places in common qemu code which does this for a very
long time. So I guess it is safe now.
It's a non-standard GCC extension.
So? We use many other GCC extensions. grep for typeof.
Dependencies should not be introduced trivially. In this case, it's
pretty easy to avoid void pointer arithmetic as Jan's next version
shows.
The standard is vague with respect void arithmetic. Most compilers
allow it. A very good analysis of the standard can be found below.
http://stackoverflow.com/questions/3523145/pointer-arithmetic-for-void-pointer-in-c
The analysis would seem to show that arithmetic may be acceptable, but
it doesn't say that void pointers must be treated like char pointers.
In my view, this would make sense:
char *cptr;
void *vptr;
Since
cptr++;
is equivalent to
cptr = (char *)((uintptr_t)cptr + sizeof(*cptr));
therefore
vptr++;
should be equivalent to
vptr = (void *)((uintptr_t)vptr + sizeof(*vptr));
That is, vptr++ should be equivalent to vptr += 0 because sizeof(void)
should be 0 if allowed.
BTW: can we please stop arguing about C standards. If we currently are
using something in QEMU that's supported by clang and GCC, it's fine and
we ought to continue using it.
The reserved names actually did bite us when porting to a new platform.
But the only requirement for C extensions ought to be reasonable support
in GCC and clang.
I don't care at all about supporting proprietary compilers.
We also don't have crowds banging doors with their money bags with a
need for such support.
Regards,
Anthony Liguori
Is there a work in progress to build GCC with visual studio?
If yes what are the chances KVM device assignment
will work on windows?
IIRC there was really a project to use KVM on Windows and another
project to build QEMU with MSVC.
Look QEMU codebase is what it is. Unless you rework all existing
code to confirm to your taste, I do not see why you NACK valid new code
unless it confirms to same.
Yes, I'd be happy to fix the style with huge patches at once. But our
fearless leader does not agree, so we are stuck with the codebase
being what it is until it is fixed one step at a time.
/mjt
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 27 August 2012 13:15, Jan Kiszka jan.kis...@siemens.com wrote: On 2012-08-27 14:07, Andreas Färber wrote: Am I correct to understand we compile this only for i386 / x86_64? This is correct. Did we ever make a decision about whether architecture specific KVM devices should all live in hw/kvm/ or in some directory with the arch name in it? I guess they're all in hw/kvm/ at the moment so they should stay there unless there's a pressing reason to move them around... Since this is arch-specific we should probably give the resulting device a more specific name than pci-assign, which implies that it is (a) ok for any PCI system and (b) not something that will be obsolete in the foreseen future. -- PMM
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Blue Swirl blauwir...@gmail.com writes:
On Tue, Aug 28, 2012 at 7:31 PM, Anthony Liguori anth...@codemonkey.ws
wrote:
Blue Swirl blauwir...@gmail.com writes:
On Tue, Aug 28, 2012 at 5:28 PM, Michael S. Tsirkin m...@redhat.com wrote:
On Tue, Aug 28, 2012 at 05:01:55PM +, Blue Swirl wrote:
On Tue, Aug 28, 2012 at 7:35 AM, Michael Tokarev m...@tls.msk.ru wrote:
On 27.08.2012 22:56, Blue Swirl wrote:
[]
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
There are a few places in common qemu code which does this for a very
long time. So I guess it is safe now.
It's a non-standard GCC extension.
So? We use many other GCC extensions. grep for typeof.
Dependencies should not be introduced trivially. In this case, it's
pretty easy to avoid void pointer arithmetic as Jan's next version
shows.
The standard is vague with respect void arithmetic. Most compilers
allow it. A very good analysis of the standard can be found below.
http://stackoverflow.com/questions/3523145/pointer-arithmetic-for-void-pointer-in-c
The analysis would seem to show that arithmetic may be acceptable, but
it doesn't say that void pointers must be treated like char pointers.
In my view, this would make sense:
char *cptr;
void *vptr;
Since
cptr++;
is equivalent to
cptr = (char *)((uintptr_t)cptr + sizeof(*cptr));
therefore
vptr++;
should be equivalent to
vptr = (void *)((uintptr_t)vptr + sizeof(*vptr));
That is, vptr++ should be equivalent to vptr += 0 because sizeof(void)
should be 0 if allowed.
sizeof(void) == 1
With GCC at least.
Regards,
Anthony Liguori
Regards,
Anthony Liguori
Is there a work in progress to build GCC with visual studio?
If yes what are the chances KVM device assignment
will work on windows?
IIRC there was really a project to use KVM on Windows and another
project to build QEMU with MSVC.
Look QEMU codebase is what it is. Unless you rework all existing
code to confirm to your taste, I do not see why you NACK valid new code
unless it confirms to same.
Yes, I'd be happy to fix the style with huge patches at once. But our
fearless leader does not agree, so we are stuck with the codebase
being what it is until it is fixed one step at a time.
/mjt
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Hi, Am 27.08.2012 08:28, schrieb Jan Kiszka: From: Jan Kiszka jan.kis...@siemens.com This adds PCI device assignment for i386 targets using the classic KVM interfaces. This version is 100% identical to what is being maintained in qemu-kvm for several years and is supported by libvirt as well. It is expected to remain relevant for another couple of years until kernels without full-features and performance-wise equivalent VFIO support are obsolete. A refactoring to-do that should be done in-tree is to model MSI and MSI-X support via the generic PCI layer, similar to what VFIO is already doing for MSI-X. This should improve the correctness and clean up the code from duplicate logic. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/kvm/Makefile.objs |2 +- hw/kvm/pci-assign.c | 1929 ++ 2 files changed, 1930 insertions(+), 1 deletions(-) create mode 100644 hw/kvm/pci-assign.c [...] diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c new file mode 100644 index 000..9cce02c --- /dev/null +++ b/hw/kvm/pci-assign.c @@ -0,0 +1,1929 @@ +/* + * Copyright (c) 2007, Neocleus Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. The downside of accepting this into qemu.git is that it gets us a huge blob of GPLv2-only code without history of contributors for GPLv2+ relicensing... + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple + * Place - Suite 330, Boston, MA 02111-1307 USA. (Expect the usual GNU address reminder here.) + * + * + * Assign a PCI device from the host to a guest VM. + * + * Adapted for KVM by Qumranet. + * + * Copyright (c) 2007, Neocleus, Alex Novik (a...@neocleus.com) + * Copyright (c) 2007, Neocleus, Guy Zana (g...@neocleus.com) + * Copyright (C) 2008, Qumranet, Amit Shah (amit.s...@qumranet.com) + * Copyright (C) 2008, Red Hat, Amit Shah (amit.s...@redhat.com) + * Copyright (C) 2008, IBM, Muli Ben-Yehuda (m...@il.ibm.com) + */ +#include stdio.h +#include unistd.h +#include sys/io.h +#include sys/mman.h +#include sys/types.h +#include sys/stat.h +#include hw/hw.h +#include hw/pc.h +#include qemu-error.h +#include console.h +#include hw/loader.h +#include monitor.h +#include range.h +#include sysemu.h +#include hw/pci.h +#include hw/msi.h +#include kvm_i386.h Am I correct to understand we compile this only for i386 / x86_64? (apic.o in kvm/Makefile.objs hints in that direction) You may want to update the description in the comment above accordingly, also mentioning that this is some deprecated backwards-compatibility thing. Regards, 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 4/4] kvm: i386: Add classic PCI device assignment
On 2012-08-27 14:07, Andreas Färber wrote: Hi, Am 27.08.2012 08:28, schrieb Jan Kiszka: From: Jan Kiszka jan.kis...@siemens.com This adds PCI device assignment for i386 targets using the classic KVM interfaces. This version is 100% identical to what is being maintained in qemu-kvm for several years and is supported by libvirt as well. It is expected to remain relevant for another couple of years until kernels without full-features and performance-wise equivalent VFIO support are obsolete. A refactoring to-do that should be done in-tree is to model MSI and MSI-X support via the generic PCI layer, similar to what VFIO is already doing for MSI-X. This should improve the correctness and clean up the code from duplicate logic. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/kvm/Makefile.objs |2 +- hw/kvm/pci-assign.c | 1929 ++ 2 files changed, 1930 insertions(+), 1 deletions(-) create mode 100644 hw/kvm/pci-assign.c [...] diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c new file mode 100644 index 000..9cce02c --- /dev/null +++ b/hw/kvm/pci-assign.c @@ -0,0 +1,1929 @@ +/* + * Copyright (c) 2007, Neocleus Corporation. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. The downside of accepting this into qemu.git is that it gets us a huge blob of GPLv2-only code without history of contributors for GPLv2+ relicensing... The history is documented in qemu-kvm. I personally don't see it will pay off going through this, but someone else may, and nothing will prevent trying this at least. I can leave a comment. BTW, VFIO will be GPLv2 only as well. If I understood Alex correctly, it is too much derived from this code. IOW: There is probably no PCI assignment without this restriction in the foreseeable future. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple + * Place - Suite 330, Boston, MA 02111-1307 USA. (Expect the usual GNU address reminder here.) Will fix. + * + * + * Assign a PCI device from the host to a guest VM. + * + * Adapted for KVM by Qumranet. + * + * Copyright (c) 2007, Neocleus, Alex Novik (a...@neocleus.com) + * Copyright (c) 2007, Neocleus, Guy Zana (g...@neocleus.com) + * Copyright (C) 2008, Qumranet, Amit Shah (amit.s...@qumranet.com) + * Copyright (C) 2008, Red Hat, Amit Shah (amit.s...@redhat.com) + * Copyright (C) 2008, IBM, Muli Ben-Yehuda (m...@il.ibm.com) + */ +#include stdio.h +#include unistd.h +#include sys/io.h +#include sys/mman.h +#include sys/types.h +#include sys/stat.h +#include hw/hw.h +#include hw/pc.h +#include qemu-error.h +#include console.h +#include hw/loader.h +#include monitor.h +#include range.h +#include sysemu.h +#include hw/pci.h +#include hw/msi.h +#include kvm_i386.h Am I correct to understand we compile this only for i386 / x86_64? This is correct. (apic.o in kvm/Makefile.objs hints in that direction) You may want to update the description in the comment above accordingly, also mentioning that this is some deprecated backwards-compatibility thing. You mean in the header of pci-assign.c? Can do. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Mon, Aug 27, 2012 at 06:56:38PM +, Blue Swirl wrote:
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
Why not?
We require gcc and it's a documented extension there.
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Mon, Aug 27, 2012 at 7:01 PM, Michael S. Tsirkin m...@redhat.com wrote:
On Mon, Aug 27, 2012 at 06:56:38PM +, Blue Swirl wrote:
+static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
+{
+AssignedDevRegion *d = opaque;
+uint8_t *in = d-u.r_virtbase + addr;
Don't perform arithmetic with void pointers.
Why not?
We require gcc and it's a documented extension there.
We don't require GCC, Clang can be used for some targets already.
Though it supports this non-standard extension too.
It's a bad idea to introduce dependencies where it's not necessary.
In this case it's not much effort to add the identifier for the struct
and in fact the only benefit ever is that the lazy coder saves a few
key presses.
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Hi Blue,
thanks for the review. I addressed most of them, the others a commented
below.
On 2012-08-27 20:56, Blue Swirl wrote:
+typedef struct AssignedDevice {
+PCIDevice dev;
+PCIHostDeviceAddress host;
+uint32_t dev_id;
+uint32_t features;
+int intpin;
+AssignedDevRegion v_addrs[PCI_NUM_REGIONS - 1];
+PCIDevRegions real_device;
+PCIINTxRoute intx_route;
+AssignedIRQType assigned_irq_type;
+struct {
+#define ASSIGNED_DEVICE_CAP_MSI (1 0)
+#define ASSIGNED_DEVICE_CAP_MSIX (1 1)
+uint32_t available;
+#define ASSIGNED_DEVICE_MSI_ENABLED (1 0)
+#define ASSIGNED_DEVICE_MSIX_ENABLED (1 1)
+#define ASSIGNED_DEVICE_MSIX_MASKED (1 2)
+uint32_t state;
+} cap;
+uint8_t emulate_config_read[PCI_CONFIG_SPACE_SIZE];
+uint8_t emulate_config_write[PCI_CONFIG_SPACE_SIZE];
+int msi_virq_nr;
+int *msi_virq;
+MSIXTableEntry *msix_table;
+target_phys_addr_t msix_table_addr;
+uint16_t msix_max;
+MemoryRegion mmio;
+char *configfd_name;
const? Not if this would mean more casts.
DEFINE_PROP_STRING, where this is used, doesn't allow this.
...
+} else {
+uint32_t port = addr + dev_region-u.r_baseport;
+
+if (data) {
+DEBUG(out data=%lx, size=%d, e_phys=%lx, host=%x\n,
+ *data, size, addr, port);
+switch (size) {
+case 1:
+outb(*data, port);
+break;
+case 2:
+outw(*data, port);
+break;
+case 4:
+outl(*data, port);
+break;
Maybe add case 8: and default: with abort(), also below.
PIO is never 8 bytes long, the generic layer protects us.
...
+
+fclose(f);
+
+/* read and fill vendor ID */
+v = get_real_vendor_id(dir, id);
+if (v) {
+return 1;
+}
+pci_dev-dev.config[0] = id 0xff;
+pci_dev-dev.config[1] = (id 0xff00) 8;
+
+/* read and fill device ID */
+v = get_real_device_id(dir, id);
+if (v) {
+return 1;
+}
+pci_dev-dev.config[2] = id 0xff;
+pci_dev-dev.config[3] = (id 0xff00) 8;
+
+pci_word_test_and_clear_mask(pci_dev-emulate_config_write +
PCI_COMMAND,
+ PCI_COMMAND_MASTER |
PCI_COMMAND_INTX_DISABLE);
+
+dev-region_number = r;
+return 0;
+}
Pretty long function, how about refactoring?
Possibly, but I'd prefer to do such changes in-tree, after the more
important refactoring on MSI[-X] is done.
...
+if (ctrl_byte PCI_MSI_FLAGS_ENABLE) {
+uint8_t *pos = pci_dev-config + pci_dev-msi_cap;
+MSIMessage msg;
+int virq;
+
+msg.address = pci_get_long(pos + PCI_MSI_ADDRESS_LO);
+msg.data = pci_get_word(pos + PCI_MSI_DATA_32);
+virq = kvm_irqchip_add_msi_route(kvm_state, msg);
+if (virq 0) {
+perror(assigned_dev_update_msi: kvm_irqchip_add_msi_route);
+return;
+}
+
+assigned_dev-msi_virq = g_malloc(sizeof(*assigned_dev-msi_virq));
Is this ever freed?
Yep, in free_msi_virqs. If you think you spotted a path where this is
not the case, let me know.
...
+
+static Property da_properties[] = {
const?
Nope, properties must remain writable.
+DEFINE_PROP_PCI_HOST_DEVADDR(host, AssignedDevice, host),
+DEFINE_PROP_BIT(prefer_msi, AssignedDevice, features,
+ASSIGNED_DEVICE_PREFER_MSI_BIT, false),
+DEFINE_PROP_BIT(share_intx, AssignedDevice, features,
+ASSIGNED_DEVICE_SHARE_INTX_BIT, true),
+DEFINE_PROP_INT32(bootindex, AssignedDevice, bootindex, -1),
+DEFINE_PROP_STRING(configfd, AssignedDevice, configfd_name),
+DEFINE_PROP_END_OF_LIST(),
+};
+
Jan
signature.asc
Description: OpenPGP digital signature
