2013-08-15 07:49:36

by Alexey Kardashevskiy

[permalink] [raw]
Subject: [PATCH v8] KVM: PPC: reserve a capability and ioctl numbers for realmode VFIO

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 <[email protected]>

---
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)

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


2013-08-27 07:56:15

by Gleb Natapov

[permalink] [raw]
Subject: Re: [PATCH v8] KVM: PPC: reserve a capability and ioctl numbers for realmode VFIO

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 <[email protected]>
>
> ---
> 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.

2013-08-27 08:42:29

by Alexey Kardashevskiy

[permalink] [raw]
Subject: Re: [PATCH v8] KVM: PPC: reserve a capability and ioctl numbers for realmode VFIO

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 <[email protected]>
>>
>> ---
>> 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)


--
Alexey

2013-08-27 10:58:47

by Gleb Natapov

[permalink] [raw]
Subject: Re: [PATCH v8] KVM: PPC: reserve a capability and ioctl numbers for realmode VFIO

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 <[email protected]>
> >>
> >> ---
> >> 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.

2013-08-28 00:51:12

by Alexey Kardashevskiy

[permalink] [raw]
Subject: Re: [PATCH v8] KVM: PPC: reserve a capability and ioctl numbers for realmode VFIO

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 <[email protected]>
>>>>
>>>> ---
>>>> 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.



--
Alexey

2013-08-28 01:27:06

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH v8] KVM: PPC: reserve a capability and ioctl numbers for realmode VFIO

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.

2013-08-28 06:38:17

by Gleb Natapov

[permalink] [raw]
Subject: Re: [PATCH v8] KVM: PPC: reserve a capability and ioctl numbers for realmode VFIO

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.