Re: [PATCH 14/15] KVM: Add support for enabling capabilities per-vcpu
On 03/08/2010 04:18 PM, Alexander Graf wrote:
Avi Kivity wrote:
On 03/08/2010 04:10 PM, Alexander Graf wrote:
When we have reserved fields which are later used for something new,
the kernel needs a way to know if the reserved fields are known or not
by userspace. One way to do this is to assume a value of zero means
the field is unknown to usespace so ignore it. Another is to require
userspace to set a bit in an already-known flags field, and only act
on the new field if its bit was set. This has the advantage that the
old kernel checks for unknown flags and errors out, improving forwards
and backwards compatibility.
I thought ->cap was already a bit field, so this isn't necessary, but
if it isn't, then a flags field is helpful.
-> cap is the capability number. So you want something like:
struct kvm_enable_cap {
__u32 cap;
__u32 flags;
__u64 args[4];
__u8 pad[64];
};
And then check for flags == 0 in the ioctl handler? Flags could later on
define if the padding changed to a different position, adding new fields
in between args and pad?
Exactly, we do so in several places. Can be useful if, for example,
some new capability comes with a resource count value.
What's this thing anyway? like cpuid bits for x86?
What thing? This ioctl or the OSI call?
The ioctl is a way to enable a feature on a per-vcpu basis. MOL overlays
the syscall interface with a hypercall interface, so a normal OS syscall
magically becomes a hypercall when magic constants get passed in r3 and r4.
Because for obvious reasons we don't want to enable that when not using
MOL, I figured I'd go in and have userspace decide if it wants to get a
hypercall exit or not. Qemu couldn't really do anything with it after
all. And while at it, I figured let's better make the interface generic.
That's reasonable. Thanks.
--
error compiling committee.c: too many arguments to function
--
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 14/15] KVM: Add support for enabling capabilities per-vcpu
Avi Kivity wrote:
> On 03/08/2010 04:10 PM, Alexander Graf wrote:
>>
>>> When we have reserved fields which are later used for something new,
>>> the kernel needs a way to know if the reserved fields are known or not
>>> by userspace. One way to do this is to assume a value of zero means
>>> the field is unknown to usespace so ignore it. Another is to require
>>> userspace to set a bit in an already-known flags field, and only act
>>> on the new field if its bit was set. This has the advantage that the
>>> old kernel checks for unknown flags and errors out, improving forwards
>>> and backwards compatibility.
>>>
>>> I thought ->cap was already a bit field, so this isn't necessary, but
>>> if it isn't, then a flags field is helpful.
>>>
>> -> cap is the capability number. So you want something like:
>>
>> struct kvm_enable_cap {
>>__u32 cap;
>>__u32 flags;
>>__u64 args[4];
>>__u8 pad[64];
>> };
>>
>> And then check for flags == 0 in the ioctl handler? Flags could later on
>> define if the padding changed to a different position, adding new fields
>> in between args and pad?
>>
>
> Exactly, we do so in several places. Can be useful if, for example,
> some new capability comes with a resource count value.
>
> What's this thing anyway? like cpuid bits for x86?
What thing? This ioctl or the OSI call?
The ioctl is a way to enable a feature on a per-vcpu basis. MOL overlays
the syscall interface with a hypercall interface, so a normal OS syscall
magically becomes a hypercall when magic constants get passed in r3 and r4.
Because for obvious reasons we don't want to enable that when not using
MOL, I figured I'd go in and have userspace decide if it wants to get a
hypercall exit or not. Qemu couldn't really do anything with it after
all. And while at it, I figured let's better make the interface generic.
Alex
--
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 14/15] KVM: Add support for enabling capabilities per-vcpu
On 03/08/2010 04:10 PM, Alexander Graf wrote:
When we have reserved fields which are later used for something new,
the kernel needs a way to know if the reserved fields are known or not
by userspace. One way to do this is to assume a value of zero means
the field is unknown to usespace so ignore it. Another is to require
userspace to set a bit in an already-known flags field, and only act
on the new field if its bit was set. This has the advantage that the
old kernel checks for unknown flags and errors out, improving forwards
and backwards compatibility.
I thought ->cap was already a bit field, so this isn't necessary, but
if it isn't, then a flags field is helpful.
-> cap is the capability number. So you want something like:
struct kvm_enable_cap {
__u32 cap;
__u32 flags;
__u64 args[4];
__u8 pad[64];
};
And then check for flags == 0 in the ioctl handler? Flags could later on
define if the padding changed to a different position, adding new fields
in between args and pad?
Exactly, we do so in several places. Can be useful if, for example,
some new capability comes with a resource count value.
What's this thing anyway? like cpuid bits for x86?
--
error compiling committee.c: too many arguments to function
--
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 14/15] KVM: Add support for enabling capabilities per-vcpu
Avi Kivity wrote:
> On 03/08/2010 03:56 PM, Alexander Graf wrote:
>> Avi Kivity wrote:
>>
>>> On 03/08/2010 03:51 PM, Alexander Graf wrote:
>>>
Avi Kivity wrote:
> On 03/05/2010 06:50 PM, Alexander Graf wrote:
>
>
>> }
>> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
>> index ce28767..c7ed3cb 100644
>> --- a/include/linux/kvm.h
>> +++ b/include/linux/kvm.h
>> @@ -400,6 +400,12 @@ struct kvm_ioeventfd {
>> __u8 pad[36];
>> };
>>
>> +/* for KVM_ENABLE_CAP */
>> +struct kvm_enable_cap {
>> +/* in */
>> +__u32 cap;
>>
>>
>>
> Reserve space here. Add a flags field and check it for zeros.
>
>
Flags? How about something like
u64 args[4]
That way the capability enabling code could decide what to do with the
arguments. We don't always only need flags I suppose?.
>>> If you interpret these as bit flags anyway, that would be redundant.
>>>
>>>
>> I think I just don't understand what you're trying to say with "flags".
>> For the OSI enabling we don't need any flags. For later additions we
>> don't know what we'll need.
>>
>
> When we have reserved fields which are later used for something new,
> the kernel needs a way to know if the reserved fields are known or not
> by userspace. One way to do this is to assume a value of zero means
> the field is unknown to usespace so ignore it. Another is to require
> userspace to set a bit in an already-known flags field, and only act
> on the new field if its bit was set. This has the advantage that the
> old kernel checks for unknown flags and errors out, improving forwards
> and backwards compatibility.
>
> I thought ->cap was already a bit field, so this isn't necessary, but
> if it isn't, then a flags field is helpful.
-> cap is the capability number. So you want something like:
struct kvm_enable_cap {
__u32 cap;
__u32 flags;
__u64 args[4];
__u8 pad[64];
};
And then check for flags == 0 in the ioctl handler? Flags could later on
define if the padding changed to a different position, adding new fields
in between args and pad?
Alex
--
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 14/15] KVM: Add support for enabling capabilities per-vcpu
On 03/08/2010 03:56 PM, Alexander Graf wrote:
Avi Kivity wrote:
On 03/08/2010 03:51 PM, Alexander Graf wrote:
Avi Kivity wrote:
On 03/05/2010 06:50 PM, Alexander Graf wrote:
}
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index ce28767..c7ed3cb 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -400,6 +400,12 @@ struct kvm_ioeventfd {
__u8 pad[36];
};
+/* for KVM_ENABLE_CAP */
+struct kvm_enable_cap {
+/* in */
+__u32 cap;
Reserve space here. Add a flags field and check it for zeros.
Flags? How about something like
u64 args[4]
That way the capability enabling code could decide what to do with the
arguments. We don't always only need flags I suppose?.
If you interpret these as bit flags anyway, that would be redundant.
I think I just don't understand what you're trying to say with "flags".
For the OSI enabling we don't need any flags. For later additions we
don't know what we'll need.
When we have reserved fields which are later used for something new, the
kernel needs a way to know if the reserved fields are known or not by
userspace. One way to do this is to assume a value of zero means the
field is unknown to usespace so ignore it. Another is to require
userspace to set a bit in an already-known flags field, and only act on
the new field if its bit was set. This has the advantage that the old
kernel checks for unknown flags and errors out, improving forwards and
backwards compatibility.
I thought ->cap was already a bit field, so this isn't necessary, but if
it isn't, then a flags field is helpful.
--
error compiling committee.c: too many arguments to function
--
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 14/15] KVM: Add support for enabling capabilities per-vcpu
Avi Kivity wrote:
> On 03/08/2010 03:51 PM, Alexander Graf wrote:
>> Avi Kivity wrote:
>>
>>> On 03/05/2010 06:50 PM, Alexander Graf wrote:
>>>
}
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index ce28767..c7ed3cb 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -400,6 +400,12 @@ struct kvm_ioeventfd {
__u8 pad[36];
};
+/* for KVM_ENABLE_CAP */
+struct kvm_enable_cap {
+/* in */
+__u32 cap;
>>> Reserve space here. Add a flags field and check it for zeros.
>>>
>> Flags? How about something like
>>
>> u64 args[4]
>>
>> That way the capability enabling code could decide what to do with the
>> arguments. We don't always only need flags I suppose?.
>>
>
> If you interpret these as bit flags anyway, that would be redundant.
>
I think I just don't understand what you're trying to say with "flags".
For the OSI enabling we don't need any flags. For later additions we
don't know what we'll need.
Alex
--
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 14/15] KVM: Add support for enabling capabilities per-vcpu
On 03/08/2010 03:51 PM, Alexander Graf wrote:
Avi Kivity wrote:
On 03/05/2010 06:50 PM, Alexander Graf wrote:
}
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index ce28767..c7ed3cb 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -400,6 +400,12 @@ struct kvm_ioeventfd {
__u8 pad[36];
};
+/* for KVM_ENABLE_CAP */
+struct kvm_enable_cap {
+/* in */
+__u32 cap;
Reserve space here. Add a flags field and check it for zeros.
Flags? How about something like
u64 args[4]
That way the capability enabling code could decide what to do with the
arguments. We don't always only need flags I suppose?.
If you interpret these as bit flags anyway, that would be redundant.
--
error compiling committee.c: too many arguments to function
--
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 14/15] KVM: Add support for enabling capabilities per-vcpu
Avi Kivity wrote:
> On 03/05/2010 06:50 PM, Alexander Graf wrote:
>> }
>> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
>> index ce28767..c7ed3cb 100644
>> --- a/include/linux/kvm.h
>> +++ b/include/linux/kvm.h
>> @@ -400,6 +400,12 @@ struct kvm_ioeventfd {
>> __u8 pad[36];
>> };
>>
>> +/* for KVM_ENABLE_CAP */
>> +struct kvm_enable_cap {
>> +/* in */
>> +__u32 cap;
>>
>
> Reserve space here. Add a flags field and check it for zeros.
Flags? How about something like
u64 args[4]
That way the capability enabling code could decide what to do with the
arguments. We don't always only need flags I suppose?.
Alex
--
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 14/15] KVM: Add support for enabling capabilities per-vcpu
On 03/05/2010 06:50 PM, Alexander Graf wrote:
}
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index ce28767..c7ed3cb 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -400,6 +400,12 @@ struct kvm_ioeventfd {
__u8 pad[36];
};
+/* for KVM_ENABLE_CAP */
+struct kvm_enable_cap {
+ /* in */
+ __u32 cap;
Reserve space here. Add a flags field and check it for zeros.
Patch Documentation/kvm/api.txt please.
--
error compiling committee.c: too many arguments to function
--
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 14/15] KVM: Add support for enabling capabilities per-vcpu
Some times we don't want all capabilities to be available to all
our vcpus. One example for that is the OSI interface, implemented
in the next patch.
In order to have a generic mechanism in how to enable capabilities
individually, this patch introduces a new ioctl that can be used
for this purpose. That way features we don't want in all guests or
userspace configurations can just not be enabled and we're good.
Signed-off-by: Alexander Graf
---
arch/powerpc/kvm/powerpc.c | 23 +++
include/linux/kvm.h|8
2 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index a28a512..f52752c 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -462,6 +462,20 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct
kvm_interrupt *irq)
return 0;
}
+static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
+struct kvm_enable_cap *cap)
+{
+ int r;
+
+ switch (cap->cap) {
+ default:
+ r = -EINVAL;
+ break;
+ }
+
+ return r;
+}
+
int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu,
struct kvm_mp_state *mp_state)
{
@@ -490,6 +504,15 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
r = kvm_vcpu_ioctl_interrupt(vcpu, &irq);
break;
}
+ case KVM_ENABLE_CAP:
+ {
+ struct kvm_enable_cap cap;
+ r = -EFAULT;
+ if (copy_from_user(&cap, argp, sizeof(cap)))
+ goto out;
+ r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
+ break;
+ }
default:
r = -EINVAL;
}
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index ce28767..c7ed3cb 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -400,6 +400,12 @@ struct kvm_ioeventfd {
__u8 pad[36];
};
+/* for KVM_ENABLE_CAP */
+struct kvm_enable_cap {
+ /* in */
+ __u32 cap;
+};
+
#define KVMIO 0xAE
/*
@@ -696,6 +702,8 @@ struct kvm_clock_data {
/* Available with KVM_CAP_DEBUGREGS */
#define KVM_GET_DEBUGREGS _IOR(KVMIO, 0xa1, struct kvm_debugregs)
#define KVM_SET_DEBUGREGS _IOW(KVMIO, 0xa2, struct kvm_debugregs)
+/* No need for CAP, because then it just always fails */
+#define KVM_ENABLE_CAP_IOW(KVMIO, 0xa3, struct kvm_enable_cap)
#define KVM_DEV_ASSIGN_ENABLE_IOMMU(1 << 0)
--
1.6.0.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
