[libvirt] [PATCH RFC 0/1] mdevctl: further config for vfio-ap

Cornelia Huck posted 1 patch 4 years, 10 months ago
Failed in applying to current master (apply log)
mdevctl.libexec | 25 ++++++++++++++++++++++
mdevctl.sbin    | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 80 insertions(+), 1 deletion(-)
[libvirt] [PATCH RFC 0/1] mdevctl: further config for vfio-ap
Posted by Cornelia Huck 4 years, 10 months ago
This patch adds a very rough implementation of additional config data
for mdev devices. The idea is to make it possible to specify some
type-specific key=value pairs in the config file for an mdev device.
If a device is started automatically, the device is stopped and restarted
after applying the config.

The code has still some problems, like not doing a lot of error handling
and being ugly in general; but most importantly, I can't really test it,
as I don't have the needed hardware. Feedback welcome; would be good to
know if the direction is sensible in general.

Also available at

https://github.com/cohuck/mdevctl conf-data

Cornelia Huck (1):
  allow to specify additional config data

 mdevctl.libexec | 25 ++++++++++++++++++++++
 mdevctl.sbin    | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 80 insertions(+), 1 deletion(-)

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 0/1] mdevctl: further config for vfio-ap
Posted by Matthew Rosato 4 years, 10 months ago
On 6/6/19 10:44 AM, Cornelia Huck wrote:
> This patch adds a very rough implementation of additional config data
> for mdev devices. The idea is to make it possible to specify some
> type-specific key=value pairs in the config file for an mdev device.
> If a device is started automatically, the device is stopped and restarted
> after applying the config.
> 
> The code has still some problems, like not doing a lot of error handling
> and being ugly in general; but most importantly, I can't really test it,
> as I don't have the needed hardware. Feedback welcome; would be good to
> know if the direction is sensible in general.

Hi Connie,

This is very similar to what I was looking to do in zdev (config via
key=value pairs), so I like your general approach.

I pulled your code and took it for a spin on an LPAR with access to
crypto cards:

# mdevctl create-mdev `uuidgen` matrix vfio_ap-passthrough
# mdevctl set-additional-config <uuid> ap_adapters=0x4,0x5
# mdevctl set-additional-config <uuid> ap_domains=0x36
# mdevctl set-additional-config <uuid> ap_control_domains=0x37

Assuming all valid inputs, this successfully creates the appropriate
mdev and what looks to be a valid mdevctl.d entry.  A subsequent reboot
successfully brings the same vfio_ap-passthrough device up again.

Matt

> 
> Also available at
> 
> https://github.com/cohuck/mdevctl conf-data
> 
> Cornelia Huck (1):
>   allow to specify additional config data
> 
>  mdevctl.libexec | 25 ++++++++++++++++++++++
>  mdevctl.sbin    | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 80 insertions(+), 1 deletion(-)
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 0/1] mdevctl: further config for vfio-ap
Posted by Cornelia Huck 4 years, 10 months ago
On Thu, 6 Jun 2019 12:45:29 -0400
Matthew Rosato <mjrosato@linux.ibm.com> wrote:

> On 6/6/19 10:44 AM, Cornelia Huck wrote:
> > This patch adds a very rough implementation of additional config data
> > for mdev devices. The idea is to make it possible to specify some
> > type-specific key=value pairs in the config file for an mdev device.
> > If a device is started automatically, the device is stopped and restarted
> > after applying the config.
> > 
> > The code has still some problems, like not doing a lot of error handling
> > and being ugly in general; but most importantly, I can't really test it,
> > as I don't have the needed hardware. Feedback welcome; would be good to
> > know if the direction is sensible in general.  
> 
> Hi Connie,
> 
> This is very similar to what I was looking to do in zdev (config via
> key=value pairs), so I like your general approach.
> 
> I pulled your code and took it for a spin on an LPAR with access to
> crypto cards:
> 
> # mdevctl create-mdev `uuidgen` matrix vfio_ap-passthrough
> # mdevctl set-additional-config <uuid> ap_adapters=0x4,0x5
> # mdevctl set-additional-config <uuid> ap_domains=0x36
> # mdevctl set-additional-config <uuid> ap_control_domains=0x37
> 
> Assuming all valid inputs, this successfully creates the appropriate
> mdev and what looks to be a valid mdevctl.d entry.  A subsequent reboot
> successfully brings the same vfio_ap-passthrough device up again.

Cool, thanks for checking!

> 
> Matt
> 
> > 
> > Also available at
> > 
> > https://github.com/cohuck/mdevctl conf-data
> > 
> > Cornelia Huck (1):
> >   allow to specify additional config data
> > 
> >  mdevctl.libexec | 25 ++++++++++++++++++++++
> >  mdevctl.sbin    | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 80 insertions(+), 1 deletion(-)
> >   
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 0/1] mdevctl: further config for vfio-ap
Posted by Tony Krowiak 4 years, 10 months ago
On 6/7/19 10:56 AM, Cornelia Huck wrote:
> On Thu, 6 Jun 2019 12:45:29 -0400
> Matthew Rosato <mjrosato@linux.ibm.com> wrote:
> 
>> On 6/6/19 10:44 AM, Cornelia Huck wrote:
>>> This patch adds a very rough implementation of additional config data
>>> for mdev devices. The idea is to make it possible to specify some
>>> type-specific key=value pairs in the config file for an mdev device.
>>> If a device is started automatically, the device is stopped and restarted
>>> after applying the config.
>>>
>>> The code has still some problems, like not doing a lot of error handling
>>> and being ugly in general; but most importantly, I can't really test it,
>>> as I don't have the needed hardware. Feedback welcome; would be good to
>>> know if the direction is sensible in general.
>>
>> Hi Connie,
>>
>> This is very similar to what I was looking to do in zdev (config via
>> key=value pairs), so I like your general approach.
>>
>> I pulled your code and took it for a spin on an LPAR with access to
>> crypto cards:
>>
>> # mdevctl create-mdev `uuidgen` matrix vfio_ap-passthrough
>> # mdevctl set-additional-config <uuid> ap_adapters=0x4,0x5
>> # mdevctl set-additional-config <uuid> ap_domains=0x36
>> # mdevctl set-additional-config <uuid> ap_control_domains=0x37
>>
>> Assuming all valid inputs, this successfully creates the appropriate
>> mdev and what looks to be a valid mdevctl.d entry.  A subsequent reboot
>> successfully brings the same vfio_ap-passthrough device up again.
> 
> Cool, thanks for checking!

I also confirmed this. I realize this is a very early proof of concept,
if you will, but error handling could be an interesting endeavor in
the case of vfio_ap given the layers of configuration involved; for
example:

* The necessity for the vfio_ap module to be installed
* The necessity that the /sys/bus/ap/apmask and /sys/bus/ap/aqmask must
   be appropriately configured

> 
>>
>> Matt
>>
>>>
>>> Also available at
>>>
>>> https://github.com/cohuck/mdevctl conf-data
>>>
>>> Cornelia Huck (1):
>>>    allow to specify additional config data
>>>
>>>   mdevctl.libexec | 25 ++++++++++++++++++++++
>>>   mdevctl.sbin    | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>>   2 files changed, 80 insertions(+), 1 deletion(-)
>>>    
>>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 0/1] mdevctl: further config for vfio-ap
Posted by Cornelia Huck 4 years, 10 months ago
On Fri, 7 Jun 2019 14:30:48 -0400
Tony Krowiak <akrowiak@linux.ibm.com> wrote:

> On 6/7/19 10:56 AM, Cornelia Huck wrote:
> > On Thu, 6 Jun 2019 12:45:29 -0400
> > Matthew Rosato <mjrosato@linux.ibm.com> wrote:
> >   
> >> On 6/6/19 10:44 AM, Cornelia Huck wrote:  
> >>> This patch adds a very rough implementation of additional config data
> >>> for mdev devices. The idea is to make it possible to specify some
> >>> type-specific key=value pairs in the config file for an mdev device.
> >>> If a device is started automatically, the device is stopped and restarted
> >>> after applying the config.
> >>>
> >>> The code has still some problems, like not doing a lot of error handling
> >>> and being ugly in general; but most importantly, I can't really test it,
> >>> as I don't have the needed hardware. Feedback welcome; would be good to
> >>> know if the direction is sensible in general.  
> >>
> >> Hi Connie,
> >>
> >> This is very similar to what I was looking to do in zdev (config via
> >> key=value pairs), so I like your general approach.
> >>
> >> I pulled your code and took it for a spin on an LPAR with access to
> >> crypto cards:
> >>
> >> # mdevctl create-mdev `uuidgen` matrix vfio_ap-passthrough
> >> # mdevctl set-additional-config <uuid> ap_adapters=0x4,0x5
> >> # mdevctl set-additional-config <uuid> ap_domains=0x36
> >> # mdevctl set-additional-config <uuid> ap_control_domains=0x37
> >>
> >> Assuming all valid inputs, this successfully creates the appropriate
> >> mdev and what looks to be a valid mdevctl.d entry.  A subsequent reboot
> >> successfully brings the same vfio_ap-passthrough device up again.  
> > 
> > Cool, thanks for checking!  
> 
> I also confirmed this. I realize this is a very early proof of concept,
> if you will, but error handling could be an interesting endeavor in
> the case of vfio_ap given the layers of configuration involved; 

I agree; that's why I mostly ignored it for now :)

> for
> example:
> 
> * The necessity for the vfio_ap module to be installed
> * The necessity that the /sys/bus/ap/apmask and /sys/bus/ap/aqmask must
>    be appropriately configured

What do you think about outsourcing that configuration to some
s390-specific tool (probably something in s390-tools)? While we can
(and should) rely on driverctl for vfio-ccw, this does not look like
something that can be easily served by some generic tooling.

> 
> >   
> >>
> >> Matt
> >>  
> >>>
> >>> Also available at
> >>>
> >>> https://github.com/cohuck/mdevctl conf-data
> >>>
> >>> Cornelia Huck (1):
> >>>    allow to specify additional config data
> >>>
> >>>   mdevctl.libexec | 25 ++++++++++++++++++++++
> >>>   mdevctl.sbin    | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >>>   2 files changed, 80 insertions(+), 1 deletion(-)
> >>>      
> >>  
> >   
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 0/1] mdevctl: further config for vfio-ap
Posted by Matthew Rosato 4 years, 10 months ago
On 6/13/19 9:54 AM, Cornelia Huck wrote:
> On Fri, 7 Jun 2019 14:30:48 -0400
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
> 
>> On 6/7/19 10:56 AM, Cornelia Huck wrote:
>>> On Thu, 6 Jun 2019 12:45:29 -0400
>>> Matthew Rosato <mjrosato@linux.ibm.com> wrote:
>>>   
>>>> On 6/6/19 10:44 AM, Cornelia Huck wrote:  
>>>>> This patch adds a very rough implementation of additional config data
>>>>> for mdev devices. The idea is to make it possible to specify some
>>>>> type-specific key=value pairs in the config file for an mdev device.
>>>>> If a device is started automatically, the device is stopped and restarted
>>>>> after applying the config.
>>>>>
>>>>> The code has still some problems, like not doing a lot of error handling
>>>>> and being ugly in general; but most importantly, I can't really test it,
>>>>> as I don't have the needed hardware. Feedback welcome; would be good to
>>>>> know if the direction is sensible in general.  
>>>>
>>>> Hi Connie,
>>>>
>>>> This is very similar to what I was looking to do in zdev (config via
>>>> key=value pairs), so I like your general approach.
>>>>
>>>> I pulled your code and took it for a spin on an LPAR with access to
>>>> crypto cards:
>>>>
>>>> # mdevctl create-mdev `uuidgen` matrix vfio_ap-passthrough
>>>> # mdevctl set-additional-config <uuid> ap_adapters=0x4,0x5
>>>> # mdevctl set-additional-config <uuid> ap_domains=0x36
>>>> # mdevctl set-additional-config <uuid> ap_control_domains=0x37
>>>>
>>>> Assuming all valid inputs, this successfully creates the appropriate
>>>> mdev and what looks to be a valid mdevctl.d entry.  A subsequent reboot
>>>> successfully brings the same vfio_ap-passthrough device up again.  
>>>
>>> Cool, thanks for checking!  
>>
>> I also confirmed this. I realize this is a very early proof of concept,
>> if you will, but error handling could be an interesting endeavor in
>> the case of vfio_ap given the layers of configuration involved; 
> 
> I agree; that's why I mostly ignored it for now :)
> 
>> for
>> example:
>>
>> * The necessity for the vfio_ap module to be installed
>> * The necessity that the /sys/bus/ap/apmask and /sys/bus/ap/aqmask must
>>    be appropriately configured
> 
> What do you think about outsourcing that configuration to some
> s390-specific tool (probably something in s390-tools)? While we can
> (and should) rely on driverctl for vfio-ccw, this does not look like
> something that can be easily served by some generic tooling.
> 

+1 mdevctl should stick to manipulation of mdevs.  I think any config
layer above that isn't directly modifying mdev parameters, like in this
case the kernel ap/aq masks, should be separate (and for vfio_ap I agree
s390-tools is the right place for this -- it's on my todo list)

>>
>>>   
>>>>
>>>> Matt
>>>>  
>>>>>
>>>>> Also available at
>>>>>
>>>>> https://github.com/cohuck/mdevctl conf-data
>>>>>
>>>>> Cornelia Huck (1):
>>>>>    allow to specify additional config data
>>>>>
>>>>>   mdevctl.libexec | 25 ++++++++++++++++++++++
>>>>>   mdevctl.sbin    | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>>   2 files changed, 80 insertions(+), 1 deletion(-)
>>>>>      
>>>>  
>>>   
>>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 0/1] mdevctl: further config for vfio-ap
Posted by Tony Krowiak 4 years, 10 months ago
On 6/13/19 9:54 AM, Cornelia Huck wrote:
> On Fri, 7 Jun 2019 14:30:48 -0400
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
> 
>> On 6/7/19 10:56 AM, Cornelia Huck wrote:
>>> On Thu, 6 Jun 2019 12:45:29 -0400
>>> Matthew Rosato <mjrosato@linux.ibm.com> wrote:
>>>    
>>>> On 6/6/19 10:44 AM, Cornelia Huck wrote:
>>>>> This patch adds a very rough implementation of additional config data
>>>>> for mdev devices. The idea is to make it possible to specify some
>>>>> type-specific key=value pairs in the config file for an mdev device.
>>>>> If a device is started automatically, the device is stopped and restarted
>>>>> after applying the config.
>>>>>
>>>>> The code has still some problems, like not doing a lot of error handling
>>>>> and being ugly in general; but most importantly, I can't really test it,
>>>>> as I don't have the needed hardware. Feedback welcome; would be good to
>>>>> know if the direction is sensible in general.
>>>>
>>>> Hi Connie,
>>>>
>>>> This is very similar to what I was looking to do in zdev (config via
>>>> key=value pairs), so I like your general approach.
>>>>
>>>> I pulled your code and took it for a spin on an LPAR with access to
>>>> crypto cards:
>>>>
>>>> # mdevctl create-mdev `uuidgen` matrix vfio_ap-passthrough
>>>> # mdevctl set-additional-config <uuid> ap_adapters=0x4,0x5
>>>> # mdevctl set-additional-config <uuid> ap_domains=0x36
>>>> # mdevctl set-additional-config <uuid> ap_control_domains=0x37
>>>>
>>>> Assuming all valid inputs, this successfully creates the appropriate
>>>> mdev and what looks to be a valid mdevctl.d entry.  A subsequent reboot
>>>> successfully brings the same vfio_ap-passthrough device up again.
>>>
>>> Cool, thanks for checking!
>>
>> I also confirmed this. I realize this is a very early proof of concept,
>> if you will, but error handling could be an interesting endeavor in
>> the case of vfio_ap given the layers of configuration involved;
> 
> I agree; that's why I mostly ignored it for now :)
> 
>> for
>> example:
>>
>> * The necessity for the vfio_ap module to be installed
>> * The necessity that the /sys/bus/ap/apmask and /sys/bus/ap/aqmask must
>>     be appropriately configured
> 
> What do you think about outsourcing that configuration to some
> s390-specific tool (probably something in s390-tools)? While we can
> (and should) rely on driverctl for vfio-ccw, this does not look like
> something that can be easily served by some generic tooling.

I wasn't suggesting configuration of the AP bus masks etc. should be
part of this tool, I was merely pointing out that errors encountered
when creating and configuring an mdev can be related to items higher in
the stack, thus making it difficult to isolate the real problem. There
are plans in place for an s390 tool that I assume will sit on top of
mdevctl should it move forward.

> 
>>
>>>    
>>>>
>>>> Matt
>>>>   
>>>>>
>>>>> Also available at
>>>>>
>>>>> https://github.com/cohuck/mdevctl conf-data
>>>>>
>>>>> Cornelia Huck (1):
>>>>>     allow to specify additional config data
>>>>>
>>>>>    mdevctl.libexec | 25 ++++++++++++++++++++++
>>>>>    mdevctl.sbin    | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>>    2 files changed, 80 insertions(+), 1 deletion(-)
>>>>>       
>>>>   
>>>    
>>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list