| Message ID | 20180416062453.24743-2-andr2000@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16/04/18 08:24, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > Introduce skeleton of the para-virtualized Xen sound > frontend driver. > > Initial handling for Xen bus states: implement > Xen bus state machine for the frontend driver according to > the state diagram and recovery flow from sound para-virtualized > protocol: xen/interface/io/sndif.h. > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Only one minor nit (see below). With that addressed (or fixed when committing): Reviewed-by: Juergen Gross <jgross@suse.com> Juergen > --- > sound/Kconfig | 2 + > sound/Makefile | 2 +- > sound/xen/Kconfig | 10 +++ > sound/xen/Makefile | 5 ++ > sound/xen/xen_snd_front.c | 196 ++++++++++++++++++++++++++++++++++++++++++++++ > sound/xen/xen_snd_front.h | 18 +++++ > 6 files changed, 232 insertions(+), 1 deletion(-) > create mode 100644 sound/xen/Kconfig > create mode 100644 sound/xen/Makefile > create mode 100644 sound/xen/xen_snd_front.c > create mode 100644 sound/xen/xen_snd_front.h > > diff --git a/sound/xen/xen_snd_front.c b/sound/xen/xen_snd_front.c > new file mode 100644 > index 000000000000..f406a8f52c51 > --- /dev/null > +++ b/sound/xen/xen_snd_front.c > @@ -0,0 +1,196 @@ > +static void sndback_changed(struct xenbus_device *xb_dev, > + enum xenbus_state backend_state) > +{ > + struct xen_snd_front_info *front_info = dev_get_drvdata(&xb_dev->dev); > + int ret; > + > + dev_dbg(&xb_dev->dev, "Backend state is %s, front is %s\n", > + xenbus_strstate(backend_state), > + xenbus_strstate(xb_dev->state)); > + > + switch (backend_state) { > + case XenbusStateReconfiguring: > + /* fall through */ > + case XenbusStateReconfigured: > + /* fall through */ > + case XenbusStateInitialised: > + /* fall through */ > + break; > + > + case XenbusStateInitialising: > + /* recovering after backend unexpected closure */ > + sndback_disconnect(front_info); > + break; > + > + case XenbusStateInitWait: > + /* recovering after backend unexpected closure */ > + sndback_disconnect(front_info); > + > + ret = sndback_initwait(front_info); > + if (ret < 0) > + xenbus_dev_fatal(xb_dev, ret, "initializing frontend"); > + else > + xenbus_switch_state(xb_dev, XenbusStateInitialised); > + break; > + > + case XenbusStateConnected: > + if (xb_dev->state != XenbusStateInitialised) > + break; > + > + ret = sndback_connect(front_info); > + if (ret < 0) > + xenbus_dev_fatal(xb_dev, ret, "initializing frontend"); > + else > + xenbus_switch_state(xb_dev, XenbusStateConnected); > + break; > + > + case XenbusStateClosing: > + /* > + * in this state backend starts freeing resources, > + * so let it go into closed state first, so we can also > + * remove ours > + */ Please start the sentence with a capital letter and end it with a full stop. Juergen
On 04/16/2018 03:25 PM, Juergen Gross wrote: > On 16/04/18 08:24, Oleksandr Andrushchenko wrote: >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> >> Introduce skeleton of the para-virtualized Xen sound >> frontend driver. >> >> Initial handling for Xen bus states: implement >> Xen bus state machine for the frontend driver according to >> the state diagram and recovery flow from sound para-virtualized >> protocol: xen/interface/io/sndif.h. >> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > Only one minor nit (see below). With that addressed (or fixed when > committing): will fix > Reviewed-by: Juergen Gross <jgross@suse.com> Thank you > > Juergen > >> --- >> sound/Kconfig | 2 + >> sound/Makefile | 2 +- >> sound/xen/Kconfig | 10 +++ >> sound/xen/Makefile | 5 ++ >> sound/xen/xen_snd_front.c | 196 ++++++++++++++++++++++++++++++++++++++++++++++ >> sound/xen/xen_snd_front.h | 18 +++++ >> 6 files changed, 232 insertions(+), 1 deletion(-) >> create mode 100644 sound/xen/Kconfig >> create mode 100644 sound/xen/Makefile >> create mode 100644 sound/xen/xen_snd_front.c >> create mode 100644 sound/xen/xen_snd_front.h >> >> diff --git a/sound/xen/xen_snd_front.c b/sound/xen/xen_snd_front.c >> new file mode 100644 >> index 000000000000..f406a8f52c51 >> --- /dev/null >> +++ b/sound/xen/xen_snd_front.c >> @@ -0,0 +1,196 @@ >> +static void sndback_changed(struct xenbus_device *xb_dev, >> + enum xenbus_state backend_state) >> +{ >> + struct xen_snd_front_info *front_info = dev_get_drvdata(&xb_dev->dev); >> + int ret; >> + >> + dev_dbg(&xb_dev->dev, "Backend state is %s, front is %s\n", >> + xenbus_strstate(backend_state), >> + xenbus_strstate(xb_dev->state)); >> + >> + switch (backend_state) { >> + case XenbusStateReconfiguring: >> + /* fall through */ >> + case XenbusStateReconfigured: >> + /* fall through */ >> + case XenbusStateInitialised: >> + /* fall through */ >> + break; >> + >> + case XenbusStateInitialising: >> + /* recovering after backend unexpected closure */ >> + sndback_disconnect(front_info); >> + break; >> + >> + case XenbusStateInitWait: >> + /* recovering after backend unexpected closure */ >> + sndback_disconnect(front_info); >> + >> + ret = sndback_initwait(front_info); >> + if (ret < 0) >> + xenbus_dev_fatal(xb_dev, ret, "initializing frontend"); >> + else >> + xenbus_switch_state(xb_dev, XenbusStateInitialised); >> + break; >> + >> + case XenbusStateConnected: >> + if (xb_dev->state != XenbusStateInitialised) >> + break; >> + >> + ret = sndback_connect(front_info); >> + if (ret < 0) >> + xenbus_dev_fatal(xb_dev, ret, "initializing frontend"); >> + else >> + xenbus_switch_state(xb_dev, XenbusStateConnected); >> + break; >> + >> + case XenbusStateClosing: >> + /* >> + * in this state backend starts freeing resources, >> + * so let it go into closed state first, so we can also >> + * remove ours >> + */ > Please start the sentence with a capital letter and end it with a > full stop. > > > Juergen
On Mon, 16 Apr 2018 08:24:49 +0200, Oleksandr Andrushchenko wrote: > --- /dev/null > +++ b/sound/xen/Kconfig > @@ -0,0 +1,10 @@ > +# ALSA Xen drivers > + > +config SND_XEN_FRONTEND > + tristate "Xen para-virtualized sound frontend driver" > + depends on XEN && SND_PCM Please do select SND_PCM instead of depends. > + select XEN_XENBUS_FRONTEND > + default n "default n" is superfluous, drop it. thanks, Takashi
On 04/24/2018 04:55 PM, Takashi Iwai wrote: > On Mon, 16 Apr 2018 08:24:49 +0200, > Oleksandr Andrushchenko wrote: >> --- /dev/null >> +++ b/sound/xen/Kconfig >> @@ -0,0 +1,10 @@ >> +# ALSA Xen drivers >> + >> +config SND_XEN_FRONTEND >> + tristate "Xen para-virtualized sound frontend driver" >> + depends on XEN && SND_PCM > Please do select SND_PCM instead of depends. will do >> + select XEN_XENBUS_FRONTEND >> + default n > "default n" is superfluous, drop it. will drop > > thanks, > > Takashi Thank you, Oleksandr
diff --git a/sound/Kconfig b/sound/Kconfig index 6833db9002ec..1140e9988fc5 100644 --- a/sound/Kconfig +++ b/sound/Kconfig @@ -96,6 +96,8 @@ source "sound/x86/Kconfig" source "sound/synth/Kconfig" +source "sound/xen/Kconfig" + endif # SND endif # !UML diff --git a/sound/Makefile b/sound/Makefile index 99d8c31262c8..797ecdcd35e2 100644 --- a/sound/Makefile +++ b/sound/Makefile @@ -5,7 +5,7 @@ obj-$(CONFIG_SOUND) += soundcore.o obj-$(CONFIG_DMASOUND) += oss/dmasound/ obj-$(CONFIG_SND) += core/ i2c/ drivers/ isa/ pci/ ppc/ arm/ sh/ synth/ usb/ \ - firewire/ sparc/ spi/ parisc/ pcmcia/ mips/ soc/ atmel/ hda/ x86/ + firewire/ sparc/ spi/ parisc/ pcmcia/ mips/ soc/ atmel/ hda/ x86/ xen/ obj-$(CONFIG_SND_AOA) += aoa/ # This one must be compilable even if sound is configured out diff --git a/sound/xen/Kconfig b/sound/xen/Kconfig new file mode 100644 index 000000000000..7cad38be8317 --- /dev/null +++ b/sound/xen/Kconfig @@ -0,0 +1,10 @@ +# ALSA Xen drivers + +config SND_XEN_FRONTEND + tristate "Xen para-virtualized sound frontend driver" + depends on XEN && SND_PCM + select XEN_XENBUS_FRONTEND + default n + help + Choose this option if you want to enable a para-virtualized + frontend sound driver for Xen guest OSes. diff --git a/sound/xen/Makefile b/sound/xen/Makefile new file mode 100644 index 000000000000..4507ef3c27fd --- /dev/null +++ b/sound/xen/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 OR MIT + +snd_xen_front-objs := xen_snd_front.o + +obj-$(CONFIG_SND_XEN_FRONTEND) += snd_xen_front.o diff --git a/sound/xen/xen_snd_front.c b/sound/xen/xen_snd_front.c new file mode 100644 index 000000000000..f406a8f52c51 --- /dev/null +++ b/sound/xen/xen_snd_front.c @@ -0,0 +1,196 @@ +// SPDX-License-Identifier: GPL-2.0 OR MIT + +/* + * Xen para-virtual sound device + * + * Copyright (C) 2016-2018 EPAM Systems Inc. + * + * Author: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> + */ + +#include <linux/delay.h> +#include <linux/module.h> + +#include <xen/platform_pci.h> +#include <xen/xen.h> +#include <xen/xenbus.h> + +#include <xen/interface/io/sndif.h> + +#include "xen_snd_front.h" + +static void xen_snd_drv_fini(struct xen_snd_front_info *front_info) +{ +} + +static int sndback_initwait(struct xen_snd_front_info *front_info) +{ + return 0; +} + +static int sndback_connect(struct xen_snd_front_info *front_info) +{ + return 0; +} + +static void sndback_disconnect(struct xen_snd_front_info *front_info) +{ + xen_snd_drv_fini(front_info); + xenbus_switch_state(front_info->xb_dev, XenbusStateInitialising); +} + +static void sndback_changed(struct xenbus_device *xb_dev, + enum xenbus_state backend_state) +{ + struct xen_snd_front_info *front_info = dev_get_drvdata(&xb_dev->dev); + int ret; + + dev_dbg(&xb_dev->dev, "Backend state is %s, front is %s\n", + xenbus_strstate(backend_state), + xenbus_strstate(xb_dev->state)); + + switch (backend_state) { + case XenbusStateReconfiguring: + /* fall through */ + case XenbusStateReconfigured: + /* fall through */ + case XenbusStateInitialised: + /* fall through */ + break; + + case XenbusStateInitialising: + /* recovering after backend unexpected closure */ + sndback_disconnect(front_info); + break; + + case XenbusStateInitWait: + /* recovering after backend unexpected closure */ + sndback_disconnect(front_info); + + ret = sndback_initwait(front_info); + if (ret < 0) + xenbus_dev_fatal(xb_dev, ret, "initializing frontend"); + else + xenbus_switch_state(xb_dev, XenbusStateInitialised); + break; + + case XenbusStateConnected: + if (xb_dev->state != XenbusStateInitialised) + break; + + ret = sndback_connect(front_info); + if (ret < 0) + xenbus_dev_fatal(xb_dev, ret, "initializing frontend"); + else + xenbus_switch_state(xb_dev, XenbusStateConnected); + break; + + case XenbusStateClosing: + /* + * in this state backend starts freeing resources, + * so let it go into closed state first, so we can also + * remove ours + */ + break; + + case XenbusStateUnknown: + /* fall through */ + case XenbusStateClosed: + if (xb_dev->state == XenbusStateClosed) + break; + + sndback_disconnect(front_info); + break; + } +} + +static int xen_drv_probe(struct xenbus_device *xb_dev, + const struct xenbus_device_id *id) +{ + struct xen_snd_front_info *front_info; + + front_info = devm_kzalloc(&xb_dev->dev, + sizeof(*front_info), GFP_KERNEL); + if (!front_info) + return -ENOMEM; + + front_info->xb_dev = xb_dev; + dev_set_drvdata(&xb_dev->dev, front_info); + + return xenbus_switch_state(xb_dev, XenbusStateInitialising); +} + +static int xen_drv_remove(struct xenbus_device *dev) +{ + struct xen_snd_front_info *front_info = dev_get_drvdata(&dev->dev); + int to = 100; + + xenbus_switch_state(dev, XenbusStateClosing); + + /* + * On driver removal it is disconnected from XenBus, + * so no backend state change events come via .otherend_changed + * callback. This prevents us from exiting gracefully, e.g. + * signaling the backend to free event channels, waiting for its + * state to change to XenbusStateClosed and cleaning at our end. + * Normally when front driver removed backend will finally go into + * XenbusStateInitWait state. + * + * Workaround: read backend's state manually and wait with time-out. + */ + while ((xenbus_read_unsigned(front_info->xb_dev->otherend, "state", + XenbusStateUnknown) != XenbusStateInitWait) && + to--) + msleep(10); + + if (!to) { + unsigned int state; + + state = xenbus_read_unsigned(front_info->xb_dev->otherend, + "state", XenbusStateUnknown); + pr_err("Backend state is %s while removing driver\n", + xenbus_strstate(state)); + } + + xen_snd_drv_fini(front_info); + xenbus_frontend_closed(dev); + return 0; +} + +static const struct xenbus_device_id xen_drv_ids[] = { + { XENSND_DRIVER_NAME }, + { "" } +}; + +static struct xenbus_driver xen_driver = { + .ids = xen_drv_ids, + .probe = xen_drv_probe, + .remove = xen_drv_remove, + .otherend_changed = sndback_changed, +}; + +static int __init xen_drv_init(void) +{ + if (!xen_domain()) + return -ENODEV; + + if (!xen_has_pv_devices()) + return -ENODEV; + + pr_info("Initialising Xen " XENSND_DRIVER_NAME " frontend driver\n"); + return xenbus_register_frontend(&xen_driver); +} + +static void __exit xen_drv_fini(void) +{ + pr_info("Unregistering Xen " XENSND_DRIVER_NAME " frontend driver\n"); + xenbus_unregister_driver(&xen_driver); +} + +module_init(xen_drv_init); +module_exit(xen_drv_fini); + +MODULE_DESCRIPTION("Xen virtual sound device frontend"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("xen:" XENSND_DRIVER_NAME); +MODULE_SUPPORTED_DEVICE("{{ALSA,Virtual soundcard}}"); diff --git a/sound/xen/xen_snd_front.h b/sound/xen/xen_snd_front.h new file mode 100644 index 000000000000..4ae204b23d32 --- /dev/null +++ b/sound/xen/xen_snd_front.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 OR MIT */ + +/* + * Xen para-virtual sound device + * + * Copyright (C) 2016-2018 EPAM Systems Inc. + * + * Author: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> + */ + +#ifndef __XEN_SND_FRONT_H +#define __XEN_SND_FRONT_H + +struct xen_snd_front_info { + struct xenbus_device *xb_dev; +}; + +#endif /* __XEN_SND_FRONT_H */