| Message ID | 20190606144417.1824-1-cohuck@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mdevctl: further config for vfio-ap | expand |
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(-) >
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(-) > > >
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(-) >>> >> >
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(-) > >>> > >> > > >
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(-) >>>>> >>>> >>> >> >
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(-) >>>>> >>>> >>> >> >