Re: [PATCH] vfio: Split virqfd into a separate module for vfio bus drivers
On Thu, 2015-03-19 at 14:12 -0600, Alex Williamson wrote:
> Well, at some point when I was doing vfio it seemed like a good idea and
> I copied it from another driver.
For some time now I have this idea that there's a Linux kernel module
template somewhere that uses defines like the ones you're using. That
all started when I noticed drivers defining DRIVER_LICENSE. (I really
like the name of that define.) Because every driver defining it ends up
using it only once.
> Is it more valuable to remove a few lines of source code with no net
> effect on the resulting output?
We should discuss this from the opposite direction: why is this patch
adding a few lines with no obvious benefit?
> Besides, look at how much more aesthetically pleasing the above is
> versus this:
>
> MODULE_VERSION("0.1");
> MODULE_LICENSE("GPL v2");
> MODULE_AUTHOR("Alex Williamson ");
> MODULE_DESCRIPTION("IRQFD support for VFIO bus drivers");
>
> ;)
I don't do smileys. Perhaps that's why I never know what to think when
someone uses them. Anyhow, sure, my comment is extremely trivial, but I
do think I should raise this point just once. Because, well, ...
because!
Paul Bolle
--
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: [PATCH] vfio: Split virqfd into a separate module for vfio bus drivers
On Wed, 2015-03-18 at 11:27 -0600, Alex Williamson wrote:
> --- a/drivers/vfio/virqfd.c
> +++ b/drivers/vfio/virqfd.c
> +#define DRIVER_VERSION "0.1"
> +#define DRIVER_AUTHOR "Alex Williamson "
> +#define DRIVER_DESC "IRQFD support for VFIO bus drivers"
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
Why bother with those three defines? They're all used just once, aren't
they?
Paul Bolle
--
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: [PATCH v3 0/2] x86/arm64: add xenconfig
On Tue, 2015-02-10 at 14:21 -0800, David Rientjes wrote: > We need an update to the MAINTAINERS file if "Yann E. MORIN" > isn't the active Kconfig maintainer anymore. Yes, we do. Michal, what update would you suggest? Thanks, Paul Bolle -- 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: [PATCH v3 0/2] x86/arm64: add xenconfig
[Added Michal. Removed Yann.] On Thu, 2015-01-29 at 12:38 -0800, Luis R. Rodriguez wrote: > On Tue, Jan 27, 2015 at 12:00 PM, Luis R. Rodriguez wrote: > > On Fri, Jan 23, 2015 at 03:19:25PM +, Stefano Stabellini wrote: > >> On Fri, 23 Jan 2015, Luis R. Rodriguez wrote: > >> > On Wed, Jan 14, 2015 at 11:33:45AM -0800, Luis R. Rodriguez wrote: > >> > > From: "Luis R. Rodriguez" > >> > > > >> > > This v3 addresses Stefano's feedback from the v2 series, namely > >> > > moving PCI stuff to x86 as its all x86 specific and also just > >> > > removing the CONFIG_TCG_XEN=m from the general config. To be > >> > > clear the changes from the v2 series are below. > >> > > > >> > > Luis R. Rodriguez (2): > >> > > x86, platform, xen, kconfig: clarify kvmconfig is for kvm > >> > > x86, arm, platform, xen, kconfig: add xen defconfig helper > >> > > > >> > > arch/x86/configs/xen.config | 10 ++ > >> > > kernel/configs/xen.config | 26 ++ > >> > > scripts/kconfig/Makefile| 7 ++- > >> > > 3 files changed, 42 insertions(+), 1 deletion(-) > >> > > create mode 100644 arch/x86/configs/xen.config > >> > > create mode 100644 kernel/configs/xen.config > >> > > >> > Who could these changes go through? > >> > >> I would be OK with taking it in the Xen tree, but I would feel more > >> comfortable doing that if you had an ack from Yann Morin (CC'ed). > > > > *Poke* > > Hey Yann, wondering if you had any feedback. Thanks. Yann has disappeared a year ago. Michal now, informally, keeps an eye on kconfig related patches. Paul Bolle -- 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: KVM: HAVE_KVM_ARCH_DIRTY_LOG_PROTECT?
On Thu, 2015-01-22 at 14:57 +0100, Paolo Bonzini wrote:
> On 22/01/2015 10:19, Paul Bolle wrote:
> > Your commit ba0513b5b8ff ("KVM: Add generic support for dirty page
> > logging") is included in today's linux-next (ie, next-20150122). I
> > noticed because a script I use to check linux-next spotted a problem
> > with it.
> >
> > That commit added a Kconfig symbol HAVE_KVM_ARCH_DIRTY_LOG_PROTECT. But
> > nothing in linux-next uses that symbol. Why was it added?
>
> Ah, there are two Kconfig symbols added by mistake.
>
> +config HAVE_KVM_ARCH_DIRTY_LOG_PROTECT
> + bool
> +
> +config KVM_GENERIC_DIRTYLOG_READ_PROTECT
> + bool
This one is actually used (so my 800 line perl monster didn't bark):
$ git grep -n KVM_GENERIC_DIRTYLOG_READ_PROTECT next-20150122
next-20150122:arch/arm/kvm/Kconfig:27: select
KVM_GENERIC_DIRTYLOG_READ_PROTECT
next-20150122:arch/arm64/kvm/Kconfig:30:select
KVM_GENERIC_DIRTYLOG_READ_PROTECT
next-20150122:arch/x86/kvm/Kconfig:42: select
KVM_GENERIC_DIRTYLOG_READ_PROTECT
next-20150122:virt/kvm/Kconfig:47:config KVM_GENERIC_DIRTYLOG_READ_PROTECT
next-20150122:virt/kvm/kvm_main.c:998:#ifdef
CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT
> Christoffer, can you fix that in kvm-arm and also the other mistake that
> Paul reported?
Thanks,
Paul Bolle
--
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
KVM: CONFIG_LOCK_DEP and CONFIG_LOCK_DETECTOR?
Mario,
Your commit c64735554c0a ("KVM: arm: Add initial dirty page locking
support") is included in today's linux-next (ie, next-20150122). I
noticed because a script I use to check linux-next spotted a minor
problem with it.
This commit added a comment that mentions CONFIG_LOCK_DEP and
CONFIG_LOCK_DETECTOR. It seems CONFIG_LOCKDEP and CONFIG_LOCKUP_DETECTOR
should be used instead. Is a trivial patch to fix these typos queued
somewhere?
Thanks,
Paul Bolle
--
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
KVM: HAVE_KVM_ARCH_DIRTY_LOG_PROTECT?
Mario,
Your commit ba0513b5b8ff ("KVM: Add generic support for dirty page
logging") is included in today's linux-next (ie, next-20150122). I
noticed because a script I use to check linux-next spotted a problem
with it.
That commit added a Kconfig symbol HAVE_KVM_ARCH_DIRTY_LOG_PROTECT. But
nothing in linux-next uses that symbol. Why was it added?
Thanks,
Paul Bolle
--
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: [PATCH v4 03/25] virtio-pci: move freeze/restore to virtio core
On Mon, 2014-10-13 at 10:48 +0300, Michael S. Tsirkin wrote: > This is in preparation to extending config changed event handling > in core. > Wrapping these in an API also seems to make for a cleaner code. > > Signed-off-by: Michael S. Tsirkin > Reviewed-by: Cornelia Huck This patch landed in today's linux-next (next-20141015). > include/linux/virtio.h | 6 + > drivers/virtio/virtio.c | 54 > + > drivers/virtio/virtio_pci.c | 54 > ++--- > 3 files changed, 62 insertions(+), 52 deletions(-) > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h > index 3c19bd3..8df7ba8 100644 > --- a/include/linux/virtio.h > +++ b/include/linux/virtio.h > @@ -78,6 +78,7 @@ bool virtqueue_is_broken(struct virtqueue *vq); > /** > * virtio_device - representation of a device using virtio > * @index: unique position on the virtio bus > + * @failed: saved value for CONFIG_S_FAILED bit (for restore) s/CONFIG_S_FAILED/VIRTIO_CONFIG_S_FAILED/? > * @dev: underlying device. > * @id: the device type identification (used to match it with a driver). > * @config: the configuration ops for this device. Paul Bolle -- 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
[PATCH] [next-20140923] KVM: Remove KVM_VFIO
There are no checks for CONFIG_KVM_VFIO and nothing depends on KVM_VFIO.
Setting KVM_VFIO has no effect. Remove that Kconfig symbol.
Signed-off-by: Paul Bolle
---
I choose not to mention commit 80ce1639727e ("KVM: VFIO: register
kvm_device_ops dynamically") as references to linux-next commits might
go stale and only confuse future readers.
Done on top of next-21040923. Tested with "git grep" only.
arch/x86/kvm/Kconfig | 1 -
virt/kvm/Kconfig | 3 ---
2 files changed, 4 deletions(-)
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index f9d16ff56c6b..178d872734b1 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -39,7 +39,6 @@ config KVM
select PERF_EVENTS
select HAVE_KVM_MSI
select HAVE_KVM_CPU_RELAX_INTERCEPT
- select KVM_VFIO
---help---
Support hosting fully virtualized guest machines using hardware
virtualization extensions. You will need a fairly recent
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index fc0c5e603eb4..1f5ebc4693c3 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -34,6 +34,3 @@ config HAVE_KVM_MSI
config HAVE_KVM_CPU_RELAX_INTERCEPT
bool
-
-config KVM_VFIO
- bool
--
1.9.3
--
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
KVM: remove Kconfig symbol KVM_VFIO?
Will,
Your commit 80ce1639727e ("KVM: VFIO: register kvm_device_ops
dynamically") is included in linux-next since next-20140918. It removes
the last usage of CONFIG_KVM_VFIO. After that commit setting KVM_VFIO is
pointless.
Is the patch to remove the Kconfig symbol KVM_VFIO from the tree queued
somewhere? If not, should I submit a trivial patch that does that?
Paul Bolle
--
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
[PATCH] kvm: vfio: silence GCC warning
Building vfio.o triggers a GCC warning (when building for 32 bits x86):
arch/x86/kvm/../../../virt/kvm/vfio.c: In function 'kvm_vfio_set_group':
arch/x86/kvm/../../../virt/kvm/vfio.c:104:22: warning: cast to pointer from
integer of different size [-Wint-to-pointer-cast]
void __user *argp = (void __user *)arg;
^
Silence this warning by casting arg to unsigned long.
argp's current type, "void __user *", is always casted to "int32_t
__user *". So its type might as well be changed to "int32_t __user *".
Signed-off-by: Paul Bolle
---
Compile tested on both 32 and 64 bits x86. I've no KVM capable hardware
that is already running v3.13-rc* at hand. Besides, I wouldn't know how
to test the codepaths I'm changing here.
virt/kvm/vfio.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
index ca4260e..b4f9507 100644
--- a/virt/kvm/vfio.c
+++ b/virt/kvm/vfio.c
@@ -101,14 +101,14 @@ static int kvm_vfio_set_group(struct kvm_device *dev,
long attr, u64 arg)
struct kvm_vfio *kv = dev->private;
struct vfio_group *vfio_group;
struct kvm_vfio_group *kvg;
- void __user *argp = (void __user *)arg;
+ int32_t __user *argp = (int32_t __user *)(unsigned long)arg;
struct fd f;
int32_t fd;
int ret;
switch (attr) {
case KVM_DEV_VFIO_GROUP_ADD:
- if (get_user(fd, (int32_t __user *)argp))
+ if (get_user(fd, argp))
return -EFAULT;
f = fdget(fd);
@@ -148,7 +148,7 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long
attr, u64 arg)
return 0;
case KVM_DEV_VFIO_GROUP_DEL:
- if (get_user(fd, (int32_t __user *)argp))
+ if (get_user(fd, argp))
return -EFAULT;
f = fdget(fd);
--
1.8.4.2
--
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
