diff mbox

[v8] KVM: PPC: reserve a capability and ioctl numbers for realmode VFIO

Message ID 1376552966-8529-1-git-send-email-aik@ozlabs.ru (mailing list archive)
State Not Applicable
Headers show

Commit Message

Alexey Kardashevskiy Aug. 15, 2013, 7:49 a.m. UTC
This is to reserve a capablity number for upcoming support
of VFIO-IOMMU DMA operations in real mode.

The last ioctl in the group which KVM_CREATE_SPAPR_TCE_IOMMU is added to
is 0xac, the next two numbers are taken - 0xad for KVM_KVMCLOCK_CTRL and
0xae for KVM_ARM_VCPU_INIT. So the KVM_CREATE_SPAPR_TCE_IOMMU ioclt gets
0xaf.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

---
Changes:
2013/08/15 v8:
* fixed comment again

2013/08/15:
* fixed mistype in comments
* fixed commit message which says what uses ioctls 0xad and 0xae

2013/07/16:
* changed the number

2013/07/11:
* changed order in a file, added comment about a gap in ioctl number
---
 include/uapi/linux/kvm.h | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Gleb Natapov Aug. 27, 2013, 7:56 a.m. UTC | #1
On Thu, Aug 15, 2013 at 05:49:26PM +1000, Alexey Kardashevskiy wrote:
> This is to reserve a capablity number for upcoming support
> of VFIO-IOMMU DMA operations in real mode.
> 
> The last ioctl in the group which KVM_CREATE_SPAPR_TCE_IOMMU is added to
> is 0xac, the next two numbers are taken - 0xad for KVM_KVMCLOCK_CTRL and
0xac was also taken by KVM_SET_ONE_REG :(

> 0xae for KVM_ARM_VCPU_INIT. So the KVM_CREATE_SPAPR_TCE_IOMMU ioclt gets
> 0xaf.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> ---
> Changes:
> 2013/08/15 v8:
> * fixed comment again
> 
> 2013/08/15:
> * fixed mistype in comments
> * fixed commit message which says what uses ioctls 0xad and 0xae
> 
> 2013/07/16:
> * changed the number
> 
> 2013/07/11:
> * changed order in a file, added comment about a gap in ioctl number
> ---
>  include/uapi/linux/kvm.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 99c2533..bd94127 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -668,6 +668,7 @@ struct kvm_ppc_smmu_info {
>  #define KVM_CAP_IRQ_XICS 92
>  #define KVM_CAP_ARM_EL1_32BIT 93
>  #define KVM_CAP_SPAPR_MULTITCE 94
> +#define KVM_CAP_SPAPR_TCE_IOMMU 95
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> @@ -933,6 +934,11 @@ struct kvm_s390_ucas_mapping {
>  #define KVM_ARM_SET_DEVICE_ADDR	  _IOW(KVMIO,  0xab, struct kvm_arm_device_addr)
>  /* Available with KVM_CAP_PPC_RTAS */
>  #define KVM_PPC_RTAS_DEFINE_TOKEN _IOW(KVMIO,  0xac, struct kvm_rtas_token_args)
> +/* 0xad is taken by KVM_KVMCLOCK_CTRL */
> +/* 0xae is taken by KVM_ARM_VCPU_INIT */
> +/* Available with KVM_CAP_SPAPR_TCE_IOMMU */
> +#define KVM_CREATE_SPAPR_TCE_IOMMU _IOW(KVMIO,  0xaf, \
> +					struct kvm_create_spapr_tce_iommu)
>  
Why not use KVM_CREATE_DEVICE API for that?

>  /* ioctl for vm fd */
>  #define KVM_CREATE_DEVICE	  _IOWR(KVMIO,  0xe0, struct kvm_create_device)
> -- 
> 1.8.3.2

--
			Gleb.
Alexey Kardashevskiy Aug. 27, 2013, 8:42 a.m. UTC | #2
On 08/27/2013 05:56 PM, Gleb Natapov wrote:
> On Thu, Aug 15, 2013 at 05:49:26PM +1000, Alexey Kardashevskiy wrote:
>> This is to reserve a capablity number for upcoming support
>> of VFIO-IOMMU DMA operations in real mode.
>>
>> The last ioctl in the group which KVM_CREATE_SPAPR_TCE_IOMMU is added to
>> is 0xac, the next two numbers are taken - 0xad for KVM_KVMCLOCK_CTRL and
> 0xac was also taken by KVM_SET_ONE_REG :(
> 
>> 0xae for KVM_ARM_VCPU_INIT. So the KVM_CREATE_SPAPR_TCE_IOMMU ioclt gets
>> 0xaf.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>> ---
>> Changes:
>> 2013/08/15 v8:
>> * fixed comment again
>>
>> 2013/08/15:
>> * fixed mistype in comments
>> * fixed commit message which says what uses ioctls 0xad and 0xae
>>
>> 2013/07/16:
>> * changed the number
>>
>> 2013/07/11:
>> * changed order in a file, added comment about a gap in ioctl number
>> ---
>>  include/uapi/linux/kvm.h | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 99c2533..bd94127 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -668,6 +668,7 @@ struct kvm_ppc_smmu_info {
>>  #define KVM_CAP_IRQ_XICS 92
>>  #define KVM_CAP_ARM_EL1_32BIT 93
>>  #define KVM_CAP_SPAPR_MULTITCE 94
>> +#define KVM_CAP_SPAPR_TCE_IOMMU 95
>>  
>>  #ifdef KVM_CAP_IRQ_ROUTING
>>  
>> @@ -933,6 +934,11 @@ struct kvm_s390_ucas_mapping {
>>  #define KVM_ARM_SET_DEVICE_ADDR	  _IOW(KVMIO,  0xab, struct kvm_arm_device_addr)
>>  /* Available with KVM_CAP_PPC_RTAS */
>>  #define KVM_PPC_RTAS_DEFINE_TOKEN _IOW(KVMIO,  0xac, struct kvm_rtas_token_args)
>> +/* 0xad is taken by KVM_KVMCLOCK_CTRL */
>> +/* 0xae is taken by KVM_ARM_VCPU_INIT */
>> +/* Available with KVM_CAP_SPAPR_TCE_IOMMU */
>> +#define KVM_CREATE_SPAPR_TCE_IOMMU _IOW(KVMIO,  0xaf, \
>> +					struct kvm_create_spapr_tce_iommu)
>>  
> Why not use KVM_CREATE_DEVICE API for that?


Because when I came up with my ioctl first time, it was not in upstream and
since then nobody pointed me to this new ioctl :)
So my stuff is not going to upstream again. Heh. Ok. I'll implement it.


> 
>>  /* ioctl for vm fd */
>>  #define KVM_CREATE_DEVICE	  _IOWR(KVMIO,  0xe0, struct kvm_create_device)
Gleb Natapov Aug. 27, 2013, 10:58 a.m. UTC | #3
On Tue, Aug 27, 2013 at 06:42:18PM +1000, Alexey Kardashevskiy wrote:
> On 08/27/2013 05:56 PM, Gleb Natapov wrote:
> > On Thu, Aug 15, 2013 at 05:49:26PM +1000, Alexey Kardashevskiy wrote:
> >> This is to reserve a capablity number for upcoming support
> >> of VFIO-IOMMU DMA operations in real mode.
> >>
> >> The last ioctl in the group which KVM_CREATE_SPAPR_TCE_IOMMU is added to
> >> is 0xac, the next two numbers are taken - 0xad for KVM_KVMCLOCK_CTRL and
> > 0xac was also taken by KVM_SET_ONE_REG :(
> > 
> >> 0xae for KVM_ARM_VCPU_INIT. So the KVM_CREATE_SPAPR_TCE_IOMMU ioclt gets
> >> 0xaf.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >>
> >> ---
> >> Changes:
> >> 2013/08/15 v8:
> >> * fixed comment again
> >>
> >> 2013/08/15:
> >> * fixed mistype in comments
> >> * fixed commit message which says what uses ioctls 0xad and 0xae
> >>
> >> 2013/07/16:
> >> * changed the number
> >>
> >> 2013/07/11:
> >> * changed order in a file, added comment about a gap in ioctl number
> >> ---
> >>  include/uapi/linux/kvm.h | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> >> index 99c2533..bd94127 100644
> >> --- a/include/uapi/linux/kvm.h
> >> +++ b/include/uapi/linux/kvm.h
> >> @@ -668,6 +668,7 @@ struct kvm_ppc_smmu_info {
> >>  #define KVM_CAP_IRQ_XICS 92
> >>  #define KVM_CAP_ARM_EL1_32BIT 93
> >>  #define KVM_CAP_SPAPR_MULTITCE 94
> >> +#define KVM_CAP_SPAPR_TCE_IOMMU 95
> >>  
> >>  #ifdef KVM_CAP_IRQ_ROUTING
> >>  
> >> @@ -933,6 +934,11 @@ struct kvm_s390_ucas_mapping {
> >>  #define KVM_ARM_SET_DEVICE_ADDR	  _IOW(KVMIO,  0xab, struct kvm_arm_device_addr)
> >>  /* Available with KVM_CAP_PPC_RTAS */
> >>  #define KVM_PPC_RTAS_DEFINE_TOKEN _IOW(KVMIO,  0xac, struct kvm_rtas_token_args)
> >> +/* 0xad is taken by KVM_KVMCLOCK_CTRL */
> >> +/* 0xae is taken by KVM_ARM_VCPU_INIT */
> >> +/* Available with KVM_CAP_SPAPR_TCE_IOMMU */
> >> +#define KVM_CREATE_SPAPR_TCE_IOMMU _IOW(KVMIO,  0xaf, \
> >> +					struct kvm_create_spapr_tce_iommu)
> >>  
> > Why not use KVM_CREATE_DEVICE API for that?
> 
> 
> Because when I came up with my ioctl first time, it was not in upstream and
> since then nobody pointed me to this new ioctl :)
Sorry about that :(. The ioctl exists for a while now, but with v8 patch I
imaging your patch series predates it.

> So my stuff is not going to upstream again. Heh. Ok. I'll implement it.
> 
Thanks! Should I keep KVM_CAP_SPAPR_MULTITCE capability patch or can I
drop it for now?

--
			Gleb.
Alexey Kardashevskiy Aug. 28, 2013, 12:51 a.m. UTC | #4
On 08/27/2013 08:58 PM, Gleb Natapov wrote:
> On Tue, Aug 27, 2013 at 06:42:18PM +1000, Alexey Kardashevskiy wrote:
>> On 08/27/2013 05:56 PM, Gleb Natapov wrote:
>>> On Thu, Aug 15, 2013 at 05:49:26PM +1000, Alexey Kardashevskiy wrote:
>>>> This is to reserve a capablity number for upcoming support
>>>> of VFIO-IOMMU DMA operations in real mode.
>>>>
>>>> The last ioctl in the group which KVM_CREATE_SPAPR_TCE_IOMMU is added to
>>>> is 0xac, the next two numbers are taken - 0xad for KVM_KVMCLOCK_CTRL and
>>> 0xac was also taken by KVM_SET_ONE_REG :(
>>>
>>>> 0xae for KVM_ARM_VCPU_INIT. So the KVM_CREATE_SPAPR_TCE_IOMMU ioclt gets
>>>> 0xaf.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>
>>>> ---
>>>> Changes:
>>>> 2013/08/15 v8:
>>>> * fixed comment again
>>>>
>>>> 2013/08/15:
>>>> * fixed mistype in comments
>>>> * fixed commit message which says what uses ioctls 0xad and 0xae
>>>>
>>>> 2013/07/16:
>>>> * changed the number
>>>>
>>>> 2013/07/11:
>>>> * changed order in a file, added comment about a gap in ioctl number
>>>> ---
>>>>  include/uapi/linux/kvm.h | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>>> index 99c2533..bd94127 100644
>>>> --- a/include/uapi/linux/kvm.h
>>>> +++ b/include/uapi/linux/kvm.h
>>>> @@ -668,6 +668,7 @@ struct kvm_ppc_smmu_info {
>>>>  #define KVM_CAP_IRQ_XICS 92
>>>>  #define KVM_CAP_ARM_EL1_32BIT 93
>>>>  #define KVM_CAP_SPAPR_MULTITCE 94
>>>> +#define KVM_CAP_SPAPR_TCE_IOMMU 95
>>>>  
>>>>  #ifdef KVM_CAP_IRQ_ROUTING
>>>>  
>>>> @@ -933,6 +934,11 @@ struct kvm_s390_ucas_mapping {
>>>>  #define KVM_ARM_SET_DEVICE_ADDR	  _IOW(KVMIO,  0xab, struct kvm_arm_device_addr)
>>>>  /* Available with KVM_CAP_PPC_RTAS */
>>>>  #define KVM_PPC_RTAS_DEFINE_TOKEN _IOW(KVMIO,  0xac, struct kvm_rtas_token_args)
>>>> +/* 0xad is taken by KVM_KVMCLOCK_CTRL */
>>>> +/* 0xae is taken by KVM_ARM_VCPU_INIT */
>>>> +/* Available with KVM_CAP_SPAPR_TCE_IOMMU */
>>>> +#define KVM_CREATE_SPAPR_TCE_IOMMU _IOW(KVMIO,  0xaf, \
>>>> +					struct kvm_create_spapr_tce_iommu)
>>>>  
>>> Why not use KVM_CREATE_DEVICE API for that?
>>
>>
>> Because when I came up with my ioctl first time, it was not in upstream and
>> since then nobody pointed me to this new ioctl :)
> Sorry about that :(. The ioctl exists for a while now, but with v8 patch I
> imaging your patch series predates it.

The ioctl I made up is basically a copy of KVM_CREATE_SPAPR_TCE which does
the same thing for emulated devices and it is there for quite a while but
it is not really extensible. And these two ioctls share some bits of code.
Now we will have 2 pieces of code which do almost the same thing but in a
different way. Kinda sucks :(

>> So my stuff is not going to upstream again. Heh. Ok. I'll implement it.
>>
> Thanks! Should I keep KVM_CAP_SPAPR_MULTITCE capability patch or can I
> drop it for now?

Please keep it, it is unrelated to the IOMMU-VFIO thing.
Benjamin Herrenschmidt Aug. 28, 2013, 1:26 a.m. UTC | #5
On Wed, 2013-08-28 at 10:51 +1000, Alexey Kardashevskiy wrote:
> The ioctl I made up is basically a copy of KVM_CREATE_SPAPR_TCE which does
> the same thing for emulated devices and it is there for quite a while but
> it is not really extensible. And these two ioctls share some bits of code.
> Now we will have 2 pieces of code which do almost the same thing but in a
> different way. Kinda sucks :(

Right. Thus the question, Gleb, we can either:

 - Keep Alexey patch as-is allowing us to *finally* merge that stuff
that's been around for monthes

 - Convert *both* existing TCE objects to the new 
KVM_CREATE_DEVICE, and have some backward compat code for the old one.

I don't think it makes sense to have the "emulated TCE" and "IOMMU TCE"
objects use a fundamentally different API and infrastructure.

> >> So my stuff is not going to upstream again. Heh. Ok. I'll implement it.
> >>
> > Thanks! Should I keep KVM_CAP_SPAPR_MULTITCE capability patch or can I
> > drop it for now?
> 
> Please keep it, it is unrelated to the IOMMU-VFIO thing.
Gleb Natapov Aug. 28, 2013, 6:38 a.m. UTC | #6
On Wed, Aug 28, 2013 at 11:26:31AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2013-08-28 at 10:51 +1000, Alexey Kardashevskiy wrote:
> > The ioctl I made up is basically a copy of KVM_CREATE_SPAPR_TCE which does
> > the same thing for emulated devices and it is there for quite a while but
> > it is not really extensible. And these two ioctls share some bits of code.
> > Now we will have 2 pieces of code which do almost the same thing but in a
> > different way. Kinda sucks :(
> 
> Right. Thus the question, Gleb, we can either:
> 
>  - Keep Alexey patch as-is allowing us to *finally* merge that stuff
> that's been around for monthes
> 
>  - Convert *both* existing TCE objects to the new 
> KVM_CREATE_DEVICE, and have some backward compat code for the old one.
> 
> I don't think it makes sense to have the "emulated TCE" and "IOMMU TCE"
> objects use a fundamentally different API and infrastructure.
> 
As a general rule we are not going to mandate converting old devices to
new API, but if it make sense to do here I would much prefer that over
adding another special ioctl

> > >> So my stuff is not going to upstream again. Heh. Ok. I'll implement it.
> > >>
> > > Thanks! Should I keep KVM_CAP_SPAPR_MULTITCE capability patch or can I
> > > drop it for now?
> > 
> > Please keep it, it is unrelated to the IOMMU-VFIO thing.
> 

--
			Gleb.
diff mbox

Patch

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 99c2533..bd94127 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -668,6 +668,7 @@  struct kvm_ppc_smmu_info {
 #define KVM_CAP_IRQ_XICS 92
 #define KVM_CAP_ARM_EL1_32BIT 93
 #define KVM_CAP_SPAPR_MULTITCE 94
+#define KVM_CAP_SPAPR_TCE_IOMMU 95
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -933,6 +934,11 @@  struct kvm_s390_ucas_mapping {
 #define KVM_ARM_SET_DEVICE_ADDR	  _IOW(KVMIO,  0xab, struct kvm_arm_device_addr)
 /* Available with KVM_CAP_PPC_RTAS */
 #define KVM_PPC_RTAS_DEFINE_TOKEN _IOW(KVMIO,  0xac, struct kvm_rtas_token_args)
+/* 0xad is taken by KVM_KVMCLOCK_CTRL */
+/* 0xae is taken by KVM_ARM_VCPU_INIT */
+/* Available with KVM_CAP_SPAPR_TCE_IOMMU */
+#define KVM_CREATE_SPAPR_TCE_IOMMU _IOW(KVMIO,  0xaf, \
+					struct kvm_create_spapr_tce_iommu)
 
 /* ioctl for vm fd */
 #define KVM_CREATE_DEVICE	  _IOWR(KVMIO,  0xe0, struct kvm_create_device)