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

2019-06-14 Thread Tony Krowiak

On 6/13/19 9:54 AM, Cornelia Huck wrote:

On Fri, 7 Jun 2019 14:30:48 -0400
Tony Krowiak  wrote:


On 6/7/19 10:56 AM, Cornelia Huck wrote:

On Thu, 6 Jun 2019 12:45:29 -0400
Matthew Rosato  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  ap_adapters=0x4,0x5
# mdevctl set-additional-config  ap_domains=0x36
# mdevctl set-additional-config  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


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

2019-06-13 Thread Matthew Rosato
On 6/13/19 9:54 AM, Cornelia Huck wrote:
> On Fri, 7 Jun 2019 14:30:48 -0400
> Tony Krowiak  wrote:
> 
>> On 6/7/19 10:56 AM, Cornelia Huck wrote:
>>> On Thu, 6 Jun 2019 12:45:29 -0400
>>> Matthew Rosato  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  ap_adapters=0x4,0x5
 # mdevctl set-additional-config  ap_domains=0x36
 # mdevctl set-additional-config  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

2019-06-13 Thread Cornelia Huck
On Fri, 7 Jun 2019 14:30:48 -0400
Tony Krowiak  wrote:

> On 6/7/19 10:56 AM, Cornelia Huck wrote:
> > On Thu, 6 Jun 2019 12:45:29 -0400
> > Matthew Rosato  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  ap_adapters=0x4,0x5
> >> # mdevctl set-additional-config  ap_domains=0x36
> >> # mdevctl set-additional-config  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

2019-06-07 Thread Tony Krowiak

On 6/7/19 10:56 AM, Cornelia Huck wrote:

On Thu, 6 Jun 2019 12:45:29 -0400
Matthew Rosato  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  ap_adapters=0x4,0x5
# mdevctl set-additional-config  ap_domains=0x36
# mdevctl set-additional-config  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

2019-06-07 Thread Cornelia Huck
On Thu, 6 Jun 2019 12:45:29 -0400
Matthew Rosato  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  ap_adapters=0x4,0x5
> # mdevctl set-additional-config  ap_domains=0x36
> # mdevctl set-additional-config  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

2019-06-06 Thread Matthew Rosato
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  ap_adapters=0x4,0x5
# mdevctl set-additional-config  ap_domains=0x36
# mdevctl set-additional-config  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


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

2019-06-06 Thread Cornelia Huck
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