* [PATCH V5 1/6] libxl: Add support for Virtio I2C device
2022-08-22 9:15 [PATCH V5 0/6] Virtio toolstack support for I2C and GPIO on Arm Viresh Kumar
@ 2022-08-22 9:15 ` Viresh Kumar
2022-09-06 16:15 ` Anthony PERARD
2022-08-22 9:15 ` [PATCH V5 2/6] libxl: Add support for Virtio GPIO device Viresh Kumar
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Viresh Kumar @ 2022-08-22 9:15 UTC (permalink / raw)
To: xen-devel
Cc: Viresh Kumar, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall,
anthony.perard
This patch adds basic support for configuring and assisting virtio-mmio
based virtio-i2c backend (emualator) which is intended to run out of
Qemu and could be run in any domain.
An example of domain configuration for Virtio I2c:
i2c = [ "" ]
Please note, this patch is not enough for virtio-i2c to work on Xen
(Arm), as for every Virtio device we need to allocate Virtio MMIO params
(IRQ and memory region) and pass them to the backend, also update Guest
device-tree. A subsequent patch will add these missing bits. For the
current patch, the default "irq" and "base" are just written to the
Xenstore.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
tools/golang/xenlight/helpers.gen.go | 108 +++++++++++
tools/golang/xenlight/types.gen.go | 27 +++
tools/include/libxl.h | 32 +++
tools/include/libxl_utils.h | 3 +
tools/libs/light/Makefile | 1 +
tools/libs/light/libxl_create.c | 13 ++
tools/libs/light/libxl_dm.c | 19 +-
tools/libs/light/libxl_i2c.c | 226 ++++++++++++++++++++++
tools/libs/light/libxl_internal.h | 1 +
tools/libs/light/libxl_types.idl | 24 +++
tools/libs/light/libxl_types_internal.idl | 1 +
tools/ocaml/libs/xl/genwrap.py | 1 +
tools/ocaml/libs/xl/xenlight_stubs.c | 1 +
tools/xl/Makefile | 2 +-
tools/xl/xl.h | 3 +
tools/xl/xl_cmdtable.c | 15 ++
tools/xl/xl_i2c.c | 142 ++++++++++++++
tools/xl/xl_parse.c | 80 ++++++++
tools/xl/xl_parse.h | 1 +
tools/xl/xl_sxp.c | 2 +
20 files changed, 699 insertions(+), 3 deletions(-)
create mode 100644 tools/libs/light/libxl_i2c.c
create mode 100644 tools/xl/xl_i2c.c
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index fa3cf2ab7658..4c7b60439bf0 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1221,6 +1221,9 @@ x.Usbdevice = C.GoString(tmp.usbdevice)
if err := x.VkbDevice.fromC(&tmp.vkb_device);err != nil {
return fmt.Errorf("converting field VkbDevice: %v", err)
}
+if err := x.I2cDevice.fromC(&tmp.i2c_device);err != nil {
+return fmt.Errorf("converting field I2cDevice: %v", err)
+}
x.Soundhw = C.GoString(tmp.soundhw)
if err := x.XenPlatformPci.fromC(&tmp.xen_platform_pci);err != nil {
return fmt.Errorf("converting field XenPlatformPci: %v", err)
@@ -1538,6 +1541,9 @@ hvm.usbdevice = C.CString(tmp.Usbdevice)}
if err := tmp.VkbDevice.toC(&hvm.vkb_device); err != nil {
return fmt.Errorf("converting field VkbDevice: %v", err)
}
+if err := tmp.I2cDevice.toC(&hvm.i2c_device); err != nil {
+return fmt.Errorf("converting field I2cDevice: %v", err)
+}
if tmp.Soundhw != "" {
hvm.soundhw = C.CString(tmp.Soundhw)}
if err := tmp.XenPlatformPci.toC(&hvm.xen_platform_pci); err != nil {
@@ -1734,6 +1740,46 @@ xc.multi_touch_num_contacts = C.uint32_t(x.MultiTouchNumContacts)
return nil
}
+// NewDeviceI2c returns an instance of DeviceI2c initialized with defaults.
+func NewDeviceI2c() (*DeviceI2c, error) {
+var (
+x DeviceI2c
+xc C.libxl_device_i2c)
+
+C.libxl_device_i2c_init(&xc)
+defer C.libxl_device_i2c_dispose(&xc)
+
+if err := x.fromC(&xc); err != nil {
+return nil, err }
+
+return &x, nil}
+
+func (x *DeviceI2c) fromC(xc *C.libxl_device_i2c) error {
+ x.BackendDomid = Domid(xc.backend_domid)
+x.BackendDomname = C.GoString(xc.backend_domname)
+x.Devid = Devid(xc.devid)
+x.BackendType = I2cBackend(xc.backend_type)
+x.Irq = uint32(xc.irq)
+x.Base = uint64(xc.base)
+
+ return nil}
+
+func (x *DeviceI2c) toC(xc *C.libxl_device_i2c) (err error){defer func(){
+if err != nil{
+C.libxl_device_i2c_dispose(xc)}
+}()
+
+xc.backend_domid = C.libxl_domid(x.BackendDomid)
+if x.BackendDomname != "" {
+xc.backend_domname = C.CString(x.BackendDomname)}
+xc.devid = C.libxl_devid(x.Devid)
+xc.backend_type = C.libxl_i2c_backend(x.BackendType)
+xc.irq = C.uint32_t(x.Irq)
+xc.base = C.uint64_t(x.Base)
+
+ return nil
+ }
+
// NewDeviceDisk returns an instance of DeviceDisk initialized with defaults.
func NewDeviceDisk() (*DeviceDisk, error) {
var (
@@ -2867,6 +2913,15 @@ if err := x.Vkbs[i].fromC(&v); err != nil {
return fmt.Errorf("converting field Vkbs: %v", err) }
}
}
+x.I2cs = nil
+if n := int(xc.num_i2cs); n > 0 {
+cI2cs := (*[1<<28]C.libxl_device_i2c)(unsafe.Pointer(xc.i2cs))[:n:n]
+x.I2cs = make([]DeviceI2c, n)
+for i, v := range cI2cs {
+if err := x.I2cs[i].fromC(&v); err != nil {
+return fmt.Errorf("converting field I2cs: %v", err) }
+}
+}
x.Vtpms = nil
if n := int(xc.num_vtpms); n > 0 {
cVtpms := (*[1<<28]C.libxl_device_vtpm)(unsafe.Pointer(xc.vtpms))[:n:n]
@@ -3028,6 +3083,16 @@ return fmt.Errorf("converting field Vkbs: %v", err)
}
}
}
+if numI2cs := len(x.I2cs); numI2cs > 0 {
+xc.i2cs = (*C.libxl_device_i2c)(C.malloc(C.ulong(numI2cs)*C.sizeof_libxl_device_i2c))
+xc.num_i2cs = C.int(numI2cs)
+cI2cs := (*[1<<28]C.libxl_device_i2c)(unsafe.Pointer(xc.i2cs))[:numI2cs:numI2cs]
+for i,v := range x.I2cs {
+if err := v.toC(&cI2cs[i]); err != nil {
+return fmt.Errorf("converting field I2cs: %v", err)
+}
+}
+}
if numVtpms := len(x.Vtpms); numVtpms > 0 {
xc.vtpms = (*C.libxl_device_vtpm)(C.malloc(C.ulong(numVtpms)*C.sizeof_libxl_device_vtpm))
xc.num_vtpms = C.int(numVtpms)
@@ -3733,6 +3798,49 @@ xc.rref = C.int(x.Rref)
return nil
}
+// NewI2cinfo returns an instance of I2cinfo initialized with defaults.
+func NewI2cinfo() (*I2cinfo, error) {
+var (
+x I2cinfo
+xc C.libxl_i2cinfo)
+
+C.libxl_i2cinfo_init(&xc)
+defer C.libxl_i2cinfo_dispose(&xc)
+
+if err := x.fromC(&xc); err != nil {
+return nil, err }
+
+return &x, nil}
+
+func (x *I2cinfo) fromC(xc *C.libxl_i2cinfo) error {
+ x.Backend = C.GoString(xc.backend)
+x.BackendId = uint32(xc.backend_id)
+x.Frontend = C.GoString(xc.frontend)
+x.FrontendId = uint32(xc.frontend_id)
+x.Devid = Devid(xc.devid)
+x.State = int(xc.state)
+x.Evtch = int(xc.evtch)
+x.Rref = int(xc.rref)
+
+ return nil}
+
+func (x *I2cinfo) toC(xc *C.libxl_i2cinfo) (err error){defer func(){
+if err != nil{
+C.libxl_i2cinfo_dispose(xc)}
+}()
+
+if x.Backend != "" {
+xc.backend = C.CString(x.Backend)}
+xc.backend_id = C.uint32_t(x.BackendId)
+if x.Frontend != "" {
+xc.frontend = C.CString(x.Frontend)}
+xc.frontend_id = C.uint32_t(x.FrontendId)
+xc.devid = C.libxl_devid(x.Devid)
+xc.state = C.int(x.State)
+
+ return nil
+ }
+
// NewNumainfo returns an instance of Numainfo initialized with defaults.
func NewNumainfo() (*Numainfo, error) {
var (
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index a0be7ada8c63..f8d7e84d6180 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -255,6 +255,11 @@ VkbBackendQemu VkbBackend = 1
VkbBackendLinux VkbBackend = 2
)
+type I2cBackend int
+const(
+I2cBackendVirtio I2cBackend = 0
+)
+
type Passthrough int
const(
PassthroughDefault Passthrough = 0
@@ -584,6 +589,7 @@ Usb Defbool
Usbversion int
Usbdevice string
VkbDevice Defbool
+I2cDevice Defbool
Soundhw string
XenPlatformPci Defbool
UsbdeviceList StringList
@@ -646,6 +652,15 @@ MultiTouchHeight uint32
MultiTouchNumContacts uint32
}
+type DeviceI2c struct {
+BackendDomid Domid
+BackendDomname string
+Devid Devid
+BackendType I2cBackend
+Irq uint32
+Base uint64
+}
+
type DeviceDisk struct {
BackendDomid Domid
BackendDomname string
@@ -935,6 +950,7 @@ Rdms []DeviceRdm
Dtdevs []DeviceDtdev
Vfbs []DeviceVfb
Vkbs []DeviceVkb
+I2cs []DeviceI2c
Vtpms []DeviceVtpm
P9S []DeviceP9
Pvcallsifs []DevicePvcallsif
@@ -1091,6 +1107,17 @@ Evtch int
Rref int
}
+type I2cinfo struct {
+Backend string
+BackendId uint32
+Frontend string
+FrontendId uint32
+Devid Devid
+State int
+Evtch int
+Rref int
+}
+
type Numainfo struct {
Size uint64
Free uint64
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 2321a648a59a..ab18c0b8c794 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -829,6 +829,15 @@ typedef struct libxl__ctx libxl_ctx;
*/
#define LIBXL_HAVE_BUILDINFO_VKB_DEVICE 1
+/*
+ * LIBXL_HAVE_BUILDINFO_I2C_DEVICE
+ *
+ * If this is defined, then the libxl_domain_build_info structure will
+ * contain a boolean hvm.i2c_device which instructs libxl whether to include
+ * a i2c at build time or not.
+ */
+#define LIBXL_HAVE_BUILDINFO_I2C_DEVICE 1
+
/*
* LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
*
@@ -2397,6 +2406,29 @@ int libxl_device_vkb_getinfo(libxl_ctx *ctx, uint32_t domid,
libxl_vkbinfo *vkbinfo)
LIBXL_EXTERNAL_CALLERS_ONLY;
+/* I2C */
+int libxl_device_i2c_add(libxl_ctx *ctx, uint32_t domid, libxl_device_i2c *i2c,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_i2c_remove(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_i2c *i2c,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_i2c_destroy(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_i2c *i2c,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+libxl_device_i2c *libxl_device_i2c_list(libxl_ctx *ctx,
+ uint32_t domid, int *num)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+void libxl_device_i2c_list_free(libxl_device_i2c* list, int num)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_i2c_getinfo(libxl_ctx *ctx, uint32_t domid,
+ const libxl_device_i2c *i2c,
+ libxl_i2cinfo *i2cinfo)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
/* Framebuffer */
int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb,
const libxl_asyncop_how *ao_how)
diff --git a/tools/include/libxl_utils.h b/tools/include/libxl_utils.h
index 46918aea847f..72b7796b7e4b 100644
--- a/tools/include/libxl_utils.h
+++ b/tools/include/libxl_utils.h
@@ -83,6 +83,9 @@ int libxl_devid_to_device_usbctrl(libxl_ctx *ctx, uint32_t domid,
int libxl_devid_to_device_vkb(libxl_ctx *ctx, uint32_t domid,
int devid, libxl_device_vkb *vkb);
+int libxl_devid_to_device_i2c(libxl_ctx *ctx, uint32_t domid,
+ int devid, libxl_device_i2c *i2c);
+
int libxl_devid_to_device_vdispl(libxl_ctx *ctx, uint32_t domid,
int devid, libxl_device_vdispl *vdispl);
diff --git a/tools/libs/light/Makefile b/tools/libs/light/Makefile
index 13545654c2fc..961bdd33297b 100644
--- a/tools/libs/light/Makefile
+++ b/tools/libs/light/Makefile
@@ -112,6 +112,7 @@ OBJS-y += libxl_vdispl.o
OBJS-y += libxl_pvcalls.o
OBJS-y += libxl_vsnd.o
OBJS-y += libxl_vkb.o
+OBJS-y += libxl_i2c.o
OBJS-y += libxl_genid.o
OBJS-y += _libxl_types.o
OBJS-y += libxl_flask.o
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index b9dd2deedf13..84fe9f80c8fe 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -375,6 +375,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_defbool_setdefault(&b_info->u.hvm.altp2m, false);
libxl_defbool_setdefault(&b_info->u.hvm.usb, false);
libxl_defbool_setdefault(&b_info->u.hvm.vkb_device, true);
+ libxl_defbool_setdefault(&b_info->u.hvm.i2c_device, true);
libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
@@ -1753,6 +1754,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
libxl__device_console console;
libxl__device device;
libxl_device_vkb vkb;
+ libxl_device_i2c i2c;
init_console_info(gc, &console, 0);
console.backend_domid = state->console_domid;
@@ -1765,6 +1767,12 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
libxl_device_vkb_dispose(&vkb);
}
+ if (libxl_defbool_val(d_config->b_info.u.hvm.i2c_device)) {
+ libxl_device_i2c_init(&i2c);
+ libxl__device_add(gc, domid, &libxl__i2c_devtype, &i2c);
+ libxl_device_i2c_dispose(&i2c);
+ }
+
dcs->sdss.dm.guest_domid = domid;
if (libxl_defbool_val(d_config->b_info.device_model_stubdomain))
libxl__spawn_stub_dm(egc, &dcs->sdss);
@@ -1797,6 +1805,11 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
&d_config->vkbs[i]);
}
+ for (i = 0; i < d_config->num_i2cs; i++) {
+ libxl__device_add(gc, domid, &libxl__i2c_devtype,
+ &d_config->i2cs[i]);
+ }
+
if (d_config->b_info.arch_arm.vuart == LIBXL_VUART_TYPE_SBSA_UART) {
init_console_info(gc, &vuart, 0);
vuart.backend_domid = state->console_domid;
diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index fc264a3a13a6..362c0596f497 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -2112,7 +2112,8 @@ static void libxl__dm_vifs_from_hvm_guest_config(libxl__gc *gc,
static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
const libxl_domain_config *guest_config,
libxl_device_vfb *vfb,
- libxl_device_vkb *vkb)
+ libxl_device_vkb *vkb,
+ libxl_device_i2c *i2c)
{
const libxl_domain_build_info *b_info = &guest_config->b_info;
@@ -2121,6 +2122,7 @@ static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
libxl_device_vfb_init(vfb);
libxl_device_vkb_init(vkb);
+ libxl_device_i2c_init(i2c);
vfb->backend_domid = 0;
vfb->devid = 0;
@@ -2131,6 +2133,9 @@ static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
vkb->backend_domid = 0;
vkb->devid = 0;
+ i2c->backend_domid = 0;
+ i2c->devid = 0;
+
return 0;
}
@@ -2293,6 +2298,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
int ret;
libxl_device_vfb *vfb;
libxl_device_vkb *vkb;
+ libxl_device_i2c *i2c;
char **args;
struct xs_permissions perm[2];
xs_transaction_t t;
@@ -2365,11 +2371,14 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
|| libxl_defbool_val(guest_config->b_info.u.hvm.sdl.enable)) {
GCNEW(vfb);
GCNEW(vkb);
- libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, vkb);
+ GCNEW(i2c);
+ libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, vkb, i2c);
dm_config->vfbs = vfb;
dm_config->num_vfbs = 1;
dm_config->vkbs = vkb;
dm_config->num_vkbs = 1;
+ dm_config->i2cs = i2c;
+ dm_config->num_i2cs = 1;
}
if (guest_config->b_info.stubdomain_kernel &&
@@ -2511,6 +2520,12 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
if (ret) goto out;
}
+ if (dm_config->num_i2cs) {
+ ret = libxl__device_add(gc, dm_domid, &libxl__i2c_devtype,
+ &dm_config->i2cs[0]);
+ if (ret) goto out;
+ }
+
if (guest_config->b_info.u.hvm.serial) {
num_console++;
} else if (guest_config->b_info.u.hvm.serial_list) {
diff --git a/tools/libs/light/libxl_i2c.c b/tools/libs/light/libxl_i2c.c
new file mode 100644
index 000000000000..2c46351ce3a4
--- /dev/null
+++ b/tools/libs/light/libxl_i2c.c
@@ -0,0 +1,226 @@
+/*
+ * Copyright (C) 2022 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_internal.h"
+
+static int libxl__device_i2c_setdefault(libxl__gc *gc, uint32_t domid,
+ libxl_device_i2c *i2c, bool hotplug)
+{
+ if (i2c->backend_type != LIBXL_I2C_BACKEND_VIRTIO) {
+ i2c->backend_type = LIBXL_I2C_BACKEND_VIRTIO;
+ }
+
+ return libxl__resolve_domid(gc, i2c->backend_domname, &i2c->backend_domid);
+}
+
+static int libxl__device_i2c_dm_needed(void *e, uint32_t domid)
+{
+ libxl_device_i2c *elem = e;
+
+ return elem->backend_type == LIBXL_I2C_BACKEND_VIRTIO;
+}
+
+static int libxl__set_xenstore_i2c(libxl__gc *gc, uint32_t domid,
+ libxl_device_i2c *i2c,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front)
+{
+ flexarray_append_pair(back, "irq", GCSPRINTF("%u", i2c->irq));
+ flexarray_append_pair(back, "base", GCSPRINTF("%lu", i2c->base));
+
+ flexarray_append_pair(front, "irq", GCSPRINTF("%u", i2c->irq));
+ flexarray_append_pair(front, "base", GCSPRINTF("%lu", i2c->base));
+
+ return 0;
+}
+
+static int libxl__i2c_from_xenstore(libxl__gc *gc, const char *libxl_path,
+ libxl_devid devid,
+ libxl_device_i2c *i2c)
+{
+ const char *be_path, *fe_path, *tmp;
+ libxl__device dev;
+ int rc;
+
+ i2c->devid = devid;
+
+ rc = libxl__xs_read_mandatory(gc, XBT_NULL,
+ GCSPRINTF("%s/backend", libxl_path),
+ &be_path);
+ if (rc) goto out;
+
+ rc = libxl__xs_read_mandatory(gc, XBT_NULL,
+ GCSPRINTF("%s/frontend", libxl_path),
+ &fe_path);
+ if (rc) goto out;
+
+ rc = libxl__backendpath_parse_domid(gc, be_path, &i2c->backend_domid);
+ if (rc) goto out;
+
+ rc = libxl__parse_backend_path(gc, be_path, &dev);
+ if (rc) goto out;
+
+ i2c->backend_type = LIBXL_I2C_BACKEND_VIRTIO;
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/irq", be_path), &tmp);
+ if (rc) goto out;
+
+ if (tmp) {
+ i2c->irq = strtoul(tmp, NULL, 0);
+ }
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/base", be_path), &tmp);
+ if (rc) goto out;
+
+ if (tmp) {
+ i2c->base = strtoul(tmp, NULL, 0);
+ }
+
+ rc = 0;
+
+out:
+
+ return rc;
+}
+
+static int libxl__device_from_i2c(libxl__gc *gc, uint32_t domid,
+ libxl_device_i2c *type, libxl__device *device)
+{
+ device->backend_devid = type->devid;
+ device->backend_domid = type->backend_domid;
+ device->backend_kind = LIBXL__DEVICE_KIND_I2C;
+ device->devid = type->devid;
+ device->domid = domid;
+ device->kind = LIBXL__DEVICE_KIND_I2C;
+
+ return 0;
+}
+
+int libxl_device_i2c_add(libxl_ctx *ctx, uint32_t domid, libxl_device_i2c *i2c,
+ const libxl_asyncop_how *ao_how)
+{
+ AO_CREATE(ctx, domid, ao_how);
+ int rc;
+
+ rc = libxl__device_add(gc, domid, &libxl__i2c_devtype, i2c);
+ if (rc) {
+ LOGD(ERROR, domid, "Unable to add i2c device");
+ goto out;
+ }
+
+out:
+ libxl__ao_complete(egc, ao, rc);
+ return AO_INPROGRESS;
+}
+
+int libxl_devid_to_device_i2c(libxl_ctx *ctx, uint32_t domid,
+ int devid, libxl_device_i2c *i2c)
+{
+ GC_INIT(ctx);
+
+ libxl_device_i2c *i2cs = NULL;
+ int n, i;
+ int rc;
+
+ libxl_device_i2c_init(i2c);
+
+ i2cs = libxl__device_list(gc, &libxl__i2c_devtype, domid, &n);
+
+ if (!i2cs) { rc = ERROR_NOTFOUND; goto out; }
+
+ for (i = 0; i < n; ++i) {
+ if (devid == i2cs[i].devid) {
+ libxl_device_i2c_copy(ctx, i2c, &i2cs[i]);
+ rc = 0;
+ goto out;
+ }
+ }
+
+ rc = ERROR_NOTFOUND;
+
+out:
+
+ if (i2cs)
+ libxl__device_list_free(&libxl__i2c_devtype, i2cs, n);
+
+ GC_FREE;
+ return rc;
+}
+
+int libxl_device_i2c_getinfo(libxl_ctx *ctx, uint32_t domid,
+ const libxl_device_i2c *i2c,
+ libxl_i2cinfo *info)
+{
+ GC_INIT(ctx);
+ char *libxl_path, *dompath, *devpath;
+ char *val;
+ int rc;
+
+ libxl_i2cinfo_init(info);
+ dompath = libxl__xs_get_dompath(gc, domid);
+ info->devid = i2c->devid;
+
+ devpath = libxl__domain_device_frontend_path(gc, domid, info->devid,
+ LIBXL__DEVICE_KIND_I2C);
+ libxl_path = libxl__domain_device_libxl_path(gc, domid, info->devid,
+ LIBXL__DEVICE_KIND_I2C);
+
+ info->backend = xs_read(ctx->xsh, XBT_NULL,
+ GCSPRINTF("%s/backend", libxl_path),
+ NULL);
+ if (!info->backend) { rc = ERROR_FAIL; goto out; }
+
+ rc = libxl__backendpath_parse_domid(gc, info->backend, &info->backend_id);
+ if (rc) goto out;
+
+ val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/state", devpath));
+ info->state = val ? strtoul(val, NULL, 10) : -1;
+
+ info->frontend = xs_read(ctx->xsh, XBT_NULL,
+ GCSPRINTF("%s/frontend", libxl_path),
+ NULL);
+ info->frontend_id = domid;
+
+ rc = 0;
+
+out:
+ GC_FREE;
+ return rc;
+}
+
+static LIBXL_DEFINE_UPDATE_DEVID(i2c)
+
+#define libxl__add_i2cs NULL
+#define libxl_device_i2c_compare NULL
+
+LIBXL_DEFINE_DEVICE_LIST(i2c)
+LIBXL_DEFINE_DEVICE_REMOVE(i2c)
+
+DEFINE_DEVICE_TYPE_STRUCT(i2c, I2C, i2cs,
+ .skip_attach = 1,
+ .dm_needed = libxl__device_i2c_dm_needed,
+ .set_xenstore_config = (device_set_xenstore_config_fn_t)
+ libxl__set_xenstore_i2c,
+ .from_xenstore = (device_from_xenstore_fn_t)libxl__i2c_from_xenstore
+);
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index cb9e8b3b8b5a..a8904cfea427 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -4003,6 +4003,7 @@ static inline int *libxl__device_type_get_num(
extern const libxl__device_type libxl__vfb_devtype;
extern const libxl__device_type libxl__vkb_devtype;
+extern const libxl__device_type libxl__i2c_devtype;
extern const libxl__device_type libxl__disk_devtype;
extern const libxl__device_type libxl__nic_devtype;
extern const libxl__device_type libxl__vtpm_devtype;
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index d634f304cda2..014a3ea8364c 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -278,6 +278,10 @@ libxl_vkb_backend = Enumeration("vkb_backend", [
(2, "LINUX")
])
+libxl_i2c_backend = Enumeration("i2c_backend", [
+ (0, "VIRTIO")
+ ])
+
libxl_passthrough = Enumeration("passthrough", [
(0, "default"),
(1, "disabled"),
@@ -626,6 +630,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
# - "mouse" for PS/2 protocol relative mouse
("usbdevice", string),
("vkb_device", libxl_defbool),
+ ("i2c_device", libxl_defbool),
("soundhw", string),
("xen_platform_pci", libxl_defbool),
("usbdevice_list", libxl_string_list),
@@ -705,6 +710,15 @@ libxl_device_vkb = Struct("device_vkb", [
("multi_touch_num_contacts", uint32)
])
+libxl_device_i2c = Struct("device_i2c", [
+ ("backend_domid", libxl_domid),
+ ("backend_domname", string),
+ ("devid", libxl_devid),
+ ("backend_type", libxl_i2c_backend),
+ ("irq", uint32),
+ ("base", uint64)
+ ])
+
libxl_device_disk = Struct("device_disk", [
("backend_domid", libxl_domid),
("backend_domname", string),
@@ -982,6 +996,7 @@ libxl_domain_config = Struct("domain_config", [
("dtdevs", Array(libxl_device_dtdev, "num_dtdevs")),
("vfbs", Array(libxl_device_vfb, "num_vfbs")),
("vkbs", Array(libxl_device_vkb, "num_vkbs")),
+ ("i2cs", Array(libxl_device_i2c, "num_i2cs")),
("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
("p9s", Array(libxl_device_p9, "num_p9s")),
("pvcallsifs", Array(libxl_device_pvcallsif, "num_pvcallsifs")),
@@ -1145,6 +1160,15 @@ libxl_vkbinfo = Struct("vkbinfo", [
("rref", integer)
], dir=DIR_OUT)
+libxl_i2cinfo = Struct("i2cinfo", [
+ ("backend", string),
+ ("backend_id", uint32),
+ ("frontend", string),
+ ("frontend_id", uint32),
+ ("devid", libxl_devid),
+ ("state", integer),
+ ], dir=DIR_OUT)
+
# NUMA node characteristics: size and free are how much memory it has, and how
# much of it is free, respectively. dists is an array of distances from this
# node to each other node.
diff --git a/tools/libs/light/libxl_types_internal.idl b/tools/libs/light/libxl_types_internal.idl
index fb0f4f23d7c2..b1a94a963dfe 100644
--- a/tools/libs/light/libxl_types_internal.idl
+++ b/tools/libs/light/libxl_types_internal.idl
@@ -33,6 +33,7 @@ libxl__device_kind = Enumeration("device_kind", [
(15, "VSND"),
(16, "VINPUT"),
(17, "VIRTIO_DISK"),
+ (18, "I2C"),
])
libxl__console_backend = Enumeration("console_backend", [
diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py
index 7bf26bdcd831..a9db0b97d80f 100644
--- a/tools/ocaml/libs/xl/genwrap.py
+++ b/tools/ocaml/libs/xl/genwrap.py
@@ -36,6 +36,7 @@ DEVICE_LIST = [ ("list", ["ctx", "domid", "t list"]),
functions = { # ( name , [type1,type2,....] )
"device_vfb": DEVICE_FUNCTIONS,
"device_vkb": DEVICE_FUNCTIONS,
+ "device_i2c": DEVICE_FUNCTIONS,
"device_disk": DEVICE_FUNCTIONS + DEVICE_LIST +
[ ("insert", ["ctx", "t", "domid", "?async:'a", "unit", "unit"]),
("of_vdev", ["ctx", "domid", "string", "t"]),
diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c
index 45b8af61c74a..cdf473f4ed57 100644
--- a/tools/ocaml/libs/xl/xenlight_stubs.c
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c
@@ -707,6 +707,7 @@ DEVICE_ADDREMOVE(disk)
DEVICE_ADDREMOVE(nic)
DEVICE_ADDREMOVE(vfb)
DEVICE_ADDREMOVE(vkb)
+DEVICE_ADDREMOVE(i2c)
DEVICE_ADDREMOVE(pci)
_DEVICE_ADDREMOVE(disk, cdrom, insert)
diff --git a/tools/xl/Makefile b/tools/xl/Makefile
index b7f439121a3a..06801962f11e 100644
--- a/tools/xl/Makefile
+++ b/tools/xl/Makefile
@@ -23,7 +23,7 @@ XL_OBJS += xl_vtpm.o xl_block.o xl_nic.o xl_usb.o
XL_OBJS += xl_sched.o xl_pci.o xl_vcpu.o xl_cdrom.o xl_mem.o
XL_OBJS += xl_info.o xl_console.o xl_misc.o
XL_OBJS += xl_vmcontrol.o xl_saverestore.o xl_migrate.o
-XL_OBJS += xl_vdispl.o xl_vsnd.o xl_vkb.o
+XL_OBJS += xl_vdispl.o xl_vsnd.o xl_vkb.o xl_i2c.o
$(XL_OBJS): CFLAGS += $(CFLAGS_libxentoollog)
$(XL_OBJS): CFLAGS += $(CFLAGS_XL)
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index 7c9aff6ad7a6..28618326149e 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -176,6 +176,9 @@ int main_vsnddetach(int argc, char **argv);
int main_vkbattach(int argc, char **argv);
int main_vkblist(int argc, char **argv);
int main_vkbdetach(int argc, char **argv);
+int main_i2cattach(int argc, char **argv);
+int main_i2clist(int argc, char **argv);
+int main_i2cdetach(int argc, char **argv);
int main_usbctrl_attach(int argc, char **argv);
int main_usbctrl_detach(int argc, char **argv);
int main_usbdev_attach(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 35182ca19630..c409ab1578d4 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -406,6 +406,21 @@ const struct cmd_spec cmd_table[] = {
"Destroy a domain's virtual keyboard device",
"<Domain> <DevId>",
},
+ { "i2c-attach",
+ &main_i2cattach, 1, 1,
+ "Create a new virtual i2c device",
+ "<Domain> <i2c-spec-component(s)>...",
+ },
+ { "i2c-list",
+ &main_i2clist, 0, 0,
+ "List virtual i2c devices for a domain",
+ "<Domain(s)>",
+ },
+ { "i2c-detach",
+ &main_i2cdetach, 0, 1,
+ "Destroy a domain's virtual i2c device",
+ "<Domain> <DevId>",
+ },
{ "vdispl-attach",
&main_vdisplattach, 1, 1,
"Create a new virtual display device",
diff --git a/tools/xl/xl_i2c.c b/tools/xl/xl_i2c.c
new file mode 100644
index 000000000000..463fac2d5836
--- /dev/null
+++ b/tools/xl/xl_i2c.c
@@ -0,0 +1,142 @@
+/*
+ * Copyright (C) 2022 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include <stdlib.h>
+
+#include <libxl.h>
+#include <libxl_utils.h>
+#include <libxlutil.h>
+
+#include "xl.h"
+#include "xl_utils.h"
+#include "xl_parse.h"
+
+int main_i2cattach(int argc, char **argv)
+{
+ int opt;
+ int rc;
+ uint32_t domid;
+ libxl_device_i2c i2c;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "i2c-attach", 2) {
+ /* No options */
+ }
+
+ libxl_device_i2c_init(&i2c);
+ domid = find_domain(argv[optind++]);
+
+ for (argv += optind, argc -= optind; argc > 0; ++argv, --argc) {
+ rc = parse_i2c_config(&i2c, *argv);
+ if (rc) goto out;
+ }
+
+ if (dryrun_only) {
+ char *json = libxl_device_i2c_to_json(ctx, &i2c);
+ printf("i2c: %s\n", json);
+ free(json);
+ goto done;
+ }
+
+ if (libxl_device_i2c_add(ctx, domid, &i2c, 0)) {
+ fprintf(stderr, "libxl_device_i2c_add failed.\n");
+ rc = ERROR_FAIL; goto out;
+ }
+
+done:
+ rc = 0;
+
+out:
+ libxl_device_i2c_dispose(&i2c);
+ return rc;
+}
+
+int main_i2clist(int argc, char **argv)
+{
+ int opt;
+ libxl_device_i2c *i2cs;
+ libxl_i2cinfo i2cinfo;
+ int nb, i;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "i2c-list", 1) {
+ /* No options */
+ }
+
+ /* Idx BE Hdl Sta evch ref ID BE-type BE-path */
+ printf("%-3s %-2s %-6s %-5s %-6s %6s %-10s %-10s %-30s\n",
+ "Idx", "BE", "handle", "state", "evt-ch", "ref",
+ "ID", "BE-type", "BE-path");
+ for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
+ uint32_t domid = find_domain(*argv);
+ i2cs = libxl_device_i2c_list(ctx, domid, &nb);
+ if (!i2cs) {
+ continue;
+ }
+ for (i = 0; i < nb; ++i) {
+ if (libxl_device_i2c_getinfo(ctx, domid, &i2cs[i], &i2cinfo) == 0) {
+ printf("%-3d %-2d %6d %5d %-10s %-30s\n",
+ i2cinfo.devid, i2cinfo.backend_id,
+ i2cinfo.devid, i2cinfo.state,
+ libxl_i2c_backend_to_string(i2cs[i].backend_type),
+ i2cinfo.backend);
+ libxl_i2cinfo_dispose(&i2cinfo);
+ }
+ }
+ libxl_device_i2c_list_free(i2cs, nb);
+ }
+ return 0;
+}
+
+int main_i2cdetach(int argc, char **argv)
+{
+ uint32_t domid, devid;
+ int opt, rc;
+ libxl_device_i2c i2c;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "i2c-detach", 2) {
+ /* No options */
+ }
+
+ domid = find_domain(argv[optind++]);
+ devid = atoi(argv[optind++]);
+
+ libxl_device_i2c_init(&i2c);
+
+ if (libxl_devid_to_device_i2c(ctx, domid, devid, &i2c)) {
+ fprintf(stderr, "Error: Device %d not connected.\n", devid);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = libxl_device_i2c_remove(ctx, domid, &i2c, 0);
+ if (rc) {
+ fprintf(stderr, "libxl_device_i2c_remove failed.\n");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = 0;
+
+out:
+ libxl_device_i2c_dispose(&i2c);
+ return rc;
+}
+
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 1b5381cef033..b91ffba14d40 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1208,6 +1208,74 @@ static void parse_vkb_list(const XLU_Config *config,
if (rc) exit(EXIT_FAILURE);
}
+int parse_i2c_config(libxl_device_i2c *i2c, char *token)
+{
+ char *oparg;
+
+ if (MATCH_OPTION("backend", token, oparg)) {
+ i2c->backend_domname = strdup(oparg);
+ } else if (MATCH_OPTION("backend-type", token, oparg)) {
+ libxl_i2c_backend backend_type;
+ if (libxl_i2c_backend_from_string(oparg, &backend_type)) {
+ fprintf(stderr, "Unknown backend_type \"%s\" in i2c spec\n",
+ oparg);
+ return -1;
+ }
+ i2c->backend_type = backend_type;
+ } else if (MATCH_OPTION("irq", token, oparg)) {
+ i2c->irq = strtoul(oparg, NULL, 0);
+ } else if (MATCH_OPTION("base", token, oparg)) {
+ i2c->base = strtoul(oparg, NULL, 0);
+ } else {
+ fprintf(stderr, "Unknown string \"%s\" in i2c spec\n", token);
+ return -1;
+ }
+
+ return 0;
+}
+
+static void parse_i2c_list(const XLU_Config *config,
+ libxl_domain_config *d_config)
+{
+ XLU_ConfigList *i2cs;
+ const char *item;
+ char *buf = NULL;
+ int rc;
+
+ if (!xlu_cfg_get_list (config, "i2c", &i2cs, 0, 0)) {
+ int entry = 0;
+ while ((item = xlu_cfg_get_listitem(i2cs, entry)) != NULL) {
+ libxl_device_i2c *i2c;
+ char *p;
+
+ i2c = ARRAY_EXTEND_INIT(d_config->i2cs,
+ d_config->num_i2cs,
+ libxl_device_i2c_init);
+
+ buf = strdup(item);
+
+ p = strtok (buf, ",");
+ while (p != NULL)
+ {
+ while (*p == ' ') p++;
+
+ rc = parse_i2c_config(i2c, p);
+ if (rc) goto out;
+
+ p = strtok (NULL, ",");
+ }
+
+ entry++;
+ }
+ }
+
+ rc = 0;
+
+out:
+ free(buf);
+ if (rc) exit(EXIT_FAILURE);
+}
+
void parse_config_data(const char *config_source,
const char *config_data,
int config_len,
@@ -2309,13 +2377,16 @@ void parse_config_data(const char *config_source,
d_config->num_vfbs = 0;
d_config->num_vkbs = 0;
+ d_config->num_i2cs = 0;
d_config->vfbs = NULL;
d_config->vkbs = NULL;
+ d_config->i2cs = NULL;
if (!xlu_cfg_get_list (config, "vfb", &cvfbs, 0, 0)) {
while ((buf = xlu_cfg_get_listitem (cvfbs, d_config->num_vfbs)) != NULL) {
libxl_device_vfb *vfb;
libxl_device_vkb *vkb;
+ libxl_device_i2c *i2c;
char *buf2 = strdup(buf);
char *p, *p2;
@@ -2326,6 +2397,9 @@ void parse_config_data(const char *config_source,
vkb = ARRAY_EXTEND_INIT(d_config->vkbs, d_config->num_vkbs,
libxl_device_vkb_init);
+ i2c = ARRAY_EXTEND_INIT(d_config->i2cs, d_config->num_i2cs,
+ libxl_device_i2c_init);
+
p = strtok(buf2, ",");
if (!p)
goto skip_vfb;
@@ -2583,6 +2657,7 @@ void parse_config_data(const char *config_source,
if (vnc_enabled) {
libxl_device_vfb *vfb;
libxl_device_vkb *vkb;
+ libxl_device_i2c *i2c;
vfb = ARRAY_EXTEND_INIT(d_config->vfbs, d_config->num_vfbs,
libxl_device_vfb_init);
@@ -2590,6 +2665,9 @@ void parse_config_data(const char *config_source,
vkb = ARRAY_EXTEND_INIT(d_config->vkbs, d_config->num_vkbs,
libxl_device_vkb_init);
+ i2c = ARRAY_EXTEND_INIT(d_config->i2cs, d_config->num_i2cs,
+ libxl_device_i2c_init);
+
parse_top_level_vnc_options(config, &vfb->vnc);
parse_top_level_sdl_options(config, &vfb->sdl);
xlu_cfg_replace_string (config, "keymap", &vfb->keymap, 0);
@@ -2704,6 +2782,7 @@ void parse_config_data(const char *config_source,
exit(-ERROR_FAIL);
}
xlu_cfg_get_defbool(config, "vkb_device", &b_info->u.hvm.vkb_device, 0);
+ xlu_cfg_get_defbool(config, "i2c_device", &b_info->u.hvm.i2c_device, 0);
xlu_cfg_replace_string (config, "soundhw", &b_info->u.hvm.soundhw, 0);
xlu_cfg_get_defbool(config, "xen_platform_pci",
&b_info->u.hvm.xen_platform_pci, 0);
@@ -2752,6 +2831,7 @@ void parse_config_data(const char *config_source,
}
parse_vkb_list(config, d_config);
+ parse_i2c_list(config, d_config);
xlu_cfg_get_defbool(config, "xend_suspend_evtchn_compat",
&c_info->xend_suspend_evtchn_compat, 0);
diff --git a/tools/xl/xl_parse.h b/tools/xl/xl_parse.h
index bab2861f8c3e..4b972d525199 100644
--- a/tools/xl/xl_parse.h
+++ b/tools/xl/xl_parse.h
@@ -36,6 +36,7 @@ int parse_nic_config(libxl_device_nic *nic, XLU_Config **config, char *token);
int parse_vdispl_config(libxl_device_vdispl *vdispl, char *token);
int parse_vsnd_item(libxl_device_vsnd *vsnd, const char *spec);
int parse_vkb_config(libxl_device_vkb *vkb, char *token);
+int parse_i2c_config(libxl_device_i2c *i2c, char *token);
int match_option_size(const char *prefix, size_t len,
char *arg, char **argopt);
diff --git a/tools/xl/xl_sxp.c b/tools/xl/xl_sxp.c
index 359a0015709e..a44c765aa515 100644
--- a/tools/xl/xl_sxp.c
+++ b/tools/xl/xl_sxp.c
@@ -140,6 +140,8 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh)
fprintf(fh, "\t\t\t(usbdevice %s)\n", b_info->u.hvm.usbdevice);
fprintf(fh, "\t\t\t(vkb_device %s)\n",
libxl_defbool_to_string(b_info->u.hvm.vkb_device));
+ fprintf(fh, "\t\t\t(i2c_device %s)\n",
+ libxl_defbool_to_string(b_info->u.hvm.i2c_device));
fprintf(fh, "\t\t)\n");
break;
case LIBXL_DOMAIN_TYPE_PV:
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V5 1/6] libxl: Add support for Virtio I2C device
2022-08-22 9:15 ` [PATCH V5 1/6] libxl: Add support for Virtio I2C device Viresh Kumar
@ 2022-09-06 16:15 ` Anthony PERARD
2022-09-07 12:32 ` Viresh Kumar
0 siblings, 1 reply; 20+ messages in thread
From: Anthony PERARD @ 2022-09-06 16:15 UTC (permalink / raw)
To: Viresh Kumar
Cc: xen-devel, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall
Hi Viresh,
On Mon, Aug 22, 2022 at 02:45:13PM +0530, Viresh Kumar wrote:
> This patch adds basic support for configuring and assisting virtio-mmio
> based virtio-i2c backend (emualator) which is intended to run out of
> Qemu and could be run in any domain.
"to run out of Qemu" -> this is ambiguous. It can both mean that
virtio-i2c is provided by QEMU and also that virtio-i2c is provided by a
different piece of software.
> An example of domain configuration for Virtio I2c:
I believe a proper spelling is "I2C", not "I2c" which looks weird to me.
> i2c = [ "" ]
Is this doing something meaningful (with the whole series applied)?
> Please note, this patch is not enough for virtio-i2c to work on Xen
> (Arm), as for every Virtio device we need to allocate Virtio MMIO params
> (IRQ and memory region) and pass them to the backend, also update Guest
> device-tree. A subsequent patch will add these missing bits. For the
> current patch, the default "irq" and "base" are just written to the
> Xenstore.
Is having irq/base set in a different patch still useful? While it was
probably useful to do this way on the virtio-disk series, it doesn't
seems useful anymore.
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> tools/golang/xenlight/helpers.gen.go | 108 +++++++++++
> tools/golang/xenlight/types.gen.go | 27 +++
Those .gen.go files aren't up-to-date. You could just add a note after
a '---' line to tell the committer to regenerate them, or make sure to
regenerate them before a new submission.
> tools/include/libxl.h | 32 +++
> tools/include/libxl_utils.h | 3 +
> tools/libs/light/Makefile | 1 +
> tools/libs/light/libxl_create.c | 13 ++
> tools/libs/light/libxl_dm.c | 19 +-
> tools/libs/light/libxl_i2c.c | 226 ++++++++++++++++++++++
> tools/libs/light/libxl_internal.h | 1 +
> tools/libs/light/libxl_types.idl | 24 +++
> tools/libs/light/libxl_types_internal.idl | 1 +
> tools/ocaml/libs/xl/genwrap.py | 1 +
> tools/ocaml/libs/xl/xenlight_stubs.c | 1 +
> tools/xl/Makefile | 2 +-
Could you take care of the change in `xl` in a separate patch?
Also, we will want documentation, at least changes in the man pages about
the new commands and configurations.
> tools/xl/xl.h | 3 +
> tools/xl/xl_cmdtable.c | 15 ++
> tools/xl/xl_i2c.c | 142 ++++++++++++++
> tools/xl/xl_parse.c | 80 ++++++++
> tools/xl/xl_parse.h | 1 +
> tools/xl/xl_sxp.c | 2 +
> 20 files changed, 699 insertions(+), 3 deletions(-)
> create mode 100644 tools/libs/light/libxl_i2c.c
> create mode 100644 tools/xl/xl_i2c.c
>
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index 2321a648a59a..ab18c0b8c794 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -829,6 +829,15 @@ typedef struct libxl__ctx libxl_ctx;
> */
> #define LIBXL_HAVE_BUILDINFO_VKB_DEVICE 1
>
> +/*
> + * LIBXL_HAVE_BUILDINFO_I2C_DEVICE
> + *
> + * If this is defined, then the libxl_domain_build_info structure will
> + * contain a boolean hvm.i2c_device which instructs libxl whether to include
> + * a i2c at build time or not.
"at built time" ? Maybe "at guest creation time" would be a bit more
descriptive, or "domain build time".
Could you add that "i2cs" is available in "libxl_domain_config" as well?
Or maybe a more generic description and macro name speaking about the
availability of I2C.
> + */
> +#define LIBXL_HAVE_BUILDINFO_I2C_DEVICE 1
Also, can this be put at the end of one of the LIBXL_HAVE_* list? Around
line 1444, just before typedef **libxl_string_list would probably be
better.
> +
> /*
> * LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
> *
> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> index b9dd2deedf13..84fe9f80c8fe 100644
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -375,6 +375,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> libxl_defbool_setdefault(&b_info->u.hvm.altp2m, false);
> libxl_defbool_setdefault(&b_info->u.hvm.usb, false);
> libxl_defbool_setdefault(&b_info->u.hvm.vkb_device, true);
> + libxl_defbool_setdefault(&b_info->u.hvm.i2c_device, true);
> libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
>
> libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
> @@ -1753,6 +1754,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
> libxl__device_console console;
> libxl__device device;
> libxl_device_vkb vkb;
> + libxl_device_i2c i2c;
>
> init_console_info(gc, &console, 0);
> console.backend_domid = state->console_domid;
> @@ -1765,6 +1767,12 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
> libxl_device_vkb_dispose(&vkb);
> }
>
> + if (libxl_defbool_val(d_config->b_info.u.hvm.i2c_device)) {
> + libxl_device_i2c_init(&i2c);
> + libxl__device_add(gc, domid, &libxl__i2c_devtype, &i2c);
> + libxl_device_i2c_dispose(&i2c);
So, every HVM guest are going to get an I2C device? I don't think that
make sense, especially on x86, or even on Arm.
> + }
> +
> dcs->sdss.dm.guest_domid = domid;
> if (libxl_defbool_val(d_config->b_info.device_model_stubdomain))
> libxl__spawn_stub_dm(egc, &dcs->sdss);
> @@ -1797,6 +1805,11 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
> &d_config->vkbs[i]);
> }
>
> + for (i = 0; i < d_config->num_i2cs; i++) {
> + libxl__device_add(gc, domid, &libxl__i2c_devtype,
> + &d_config->i2cs[i]);
> + }
> +
> if (d_config->b_info.arch_arm.vuart == LIBXL_VUART_TYPE_SBSA_UART) {
> init_console_info(gc, &vuart, 0);
> vuart.backend_domid = state->console_domid;
> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> index fc264a3a13a6..362c0596f497 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> @@ -2112,7 +2112,8 @@ static void libxl__dm_vifs_from_hvm_guest_config(libxl__gc *gc,
> static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
> const libxl_domain_config *guest_config,
> libxl_device_vfb *vfb,
> - libxl_device_vkb *vkb)
> + libxl_device_vkb *vkb,
> + libxl_device_i2c *i2c)
That function is about taking care of adding Xen PV virtual framebuffer
and keyboard to a guest. What is the relationship with between that and
I2C?
> {
> const libxl_domain_build_info *b_info = &guest_config->b_info;
>
> diff --git a/tools/libs/light/libxl_i2c.c b/tools/libs/light/libxl_i2c.c
> new file mode 100644
> index 000000000000..2c46351ce3a4
> --- /dev/null
> +++ b/tools/libs/light/libxl_i2c.c
> @@ -0,0 +1,226 @@
> +/*
> + * Copyright (C) 2022 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +#include "libxl_internal.h"
> +
> +static int libxl__device_i2c_setdefault(libxl__gc *gc, uint32_t domid,
> + libxl_device_i2c *i2c, bool hotplug)
> +{
> + if (i2c->backend_type != LIBXL_I2C_BACKEND_VIRTIO) {
> + i2c->backend_type = LIBXL_I2C_BACKEND_VIRTIO;
> + }
> +
> + return libxl__resolve_domid(gc, i2c->backend_domname, &i2c->backend_domid);
> +}
> +
> +static int libxl__device_i2c_dm_needed(void *e, uint32_t domid)
> +{
> + libxl_device_i2c *elem = e;
> +
> + return elem->backend_type == LIBXL_I2C_BACKEND_VIRTIO;
I'm not sure I understand the result of this function. Is QEMU needed
to present VirtIO I2C devices to a guest?
> diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
> index d634f304cda2..014a3ea8364c 100644
> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -278,6 +278,10 @@ libxl_vkb_backend = Enumeration("vkb_backend", [
> (2, "LINUX")
> ])
>
> +libxl_i2c_backend = Enumeration("i2c_backend", [
> + (0, "VIRTIO")
Could you start with 1 rather than 0? This will allow libxl to find out
if the backend have been set or if an application let libxl choose which
backend is more appropriate.
> + ])
> +
> libxl_passthrough = Enumeration("passthrough", [
> (0, "default"),
> (1, "disabled"),
> @@ -626,6 +630,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> # - "mouse" for PS/2 protocol relative mouse
> ("usbdevice", string),
> ("vkb_device", libxl_defbool),
> + ("i2c_device", libxl_defbool),
How is "i2c_device" useful? Would "i2cs" in libxl_domain_config be
enough?
> ("soundhw", string),
> ("xen_platform_pci", libxl_defbool),
> ("usbdevice_list", libxl_string_list),
> diff --git a/tools/xl/Makefile b/tools/xl/Makefile
> index b7f439121a3a..06801962f11e 100644
> --- a/tools/xl/Makefile
> +++ b/tools/xl/Makefile
> @@ -23,7 +23,7 @@ XL_OBJS += xl_vtpm.o xl_block.o xl_nic.o xl_usb.o
> XL_OBJS += xl_sched.o xl_pci.o xl_vcpu.o xl_cdrom.o xl_mem.o
> XL_OBJS += xl_info.o xl_console.o xl_misc.o
> XL_OBJS += xl_vmcontrol.o xl_saverestore.o xl_migrate.o
> -XL_OBJS += xl_vdispl.o xl_vsnd.o xl_vkb.o
> +XL_OBJS += xl_vdispl.o xl_vsnd.o xl_vkb.o xl_i2c.o
Could you add xl_i2c.o in a new line instead?
>
> $(XL_OBJS): CFLAGS += $(CFLAGS_libxentoollog)
> $(XL_OBJS): CFLAGS += $(CFLAGS_XL)
> diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> index 35182ca19630..c409ab1578d4 100644
> --- a/tools/xl/xl_cmdtable.c
> +++ b/tools/xl/xl_cmdtable.c
> @@ -406,6 +406,21 @@ const struct cmd_spec cmd_table[] = {
> "Destroy a domain's virtual keyboard device",
> "<Domain> <DevId>",
> },
> + { "i2c-attach",
> + &main_i2cattach, 1, 1,
> + "Create a new virtual i2c device",
> + "<Domain> <i2c-spec-component(s)>...",
Is i2c-attach going to work? It seems that an i2c device is presented to
the guest via the device tree, but I don't think it can be modified on
the fly once the guest have booted. Is there i2c device hotplug?
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 1b5381cef033..b91ffba14d40 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -2583,6 +2657,7 @@ void parse_config_data(const char *config_source,
> if (vnc_enabled) {
> libxl_device_vfb *vfb;
> libxl_device_vkb *vkb;
> + libxl_device_i2c *i2c;
How is I2C related to VNC?
>
> vfb = ARRAY_EXTEND_INIT(d_config->vfbs, d_config->num_vfbs,
> libxl_device_vfb_init);
> diff --git a/tools/xl/xl_sxp.c b/tools/xl/xl_sxp.c
> index 359a0015709e..a44c765aa515 100644
> --- a/tools/xl/xl_sxp.c
> +++ b/tools/xl/xl_sxp.c
> @@ -140,6 +140,8 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh)
> fprintf(fh, "\t\t\t(usbdevice %s)\n", b_info->u.hvm.usbdevice);
> fprintf(fh, "\t\t\t(vkb_device %s)\n",
> libxl_defbool_to_string(b_info->u.hvm.vkb_device));
> + fprintf(fh, "\t\t\t(i2c_device %s)\n",
> + libxl_defbool_to_string(b_info->u.hvm.i2c_device));
There is a notice on the printf_info_sexp() function:
/* In general you should not add new output to this function since it
* is intended only for legacy use.
*/
void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh)
Could you explain why this function is been modified? If not, there's
probably no modification needed.
> fprintf(fh, "\t\t)\n");
> break;
> case LIBXL_DOMAIN_TYPE_PV:
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V5 1/6] libxl: Add support for Virtio I2C device
2022-09-06 16:15 ` Anthony PERARD
@ 2022-09-07 12:32 ` Viresh Kumar
2022-09-07 17:49 ` Julien Grall
2022-09-08 13:49 ` Anthony PERARD
0 siblings, 2 replies; 20+ messages in thread
From: Viresh Kumar @ 2022-09-07 12:32 UTC (permalink / raw)
To: Anthony PERARD
Cc: xen-devel, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall
Hi Anthony,
Thanks a lot for the in-depth review, it is really helpful.
I don't have much knowledge of the Xen code and wanted this code for I2C and
GPIO to be tested on Xen for the work we are doing around hypervisor agnostic
backends [1].
I started looking for a simple device's implementation and began by (blindly)
copying code from Keyboard device and so much of wasted code in here, which
isn't really required.
On 06-09-22, 17:15, Anthony PERARD wrote:
> On Mon, Aug 22, 2022 at 02:45:13PM +0530, Viresh Kumar wrote:
> > An example of domain configuration for Virtio I2c:
> > i2c = [ "" ]
>
> Is this doing something meaningful (with the whole series applied)?
If I am not wrong, this is required by parse_i2c_list()'s implementation.
Without this I don't get the I2C device populated in the guest.
Is there another way to achieve it ?
I have removed a lot of code now and what I have left is pasted at the end of
this email, does this look better now ? And yes, I will update documentation and
look again at any formatting issues before sending it.
Thanks.
--
viresh
[1] https://lore.kernel.org/all/20220414092358.kepxbmnrtycz7mhe@vireshk-i7/
-------------------------8<-------------------------
Subject: [PATCH] libxl: Add support for Virtio I2C device
This patch adds basic support for configuring and assisting virtio-mmio
based virtio-i2c backend (a hypervisor independent emulator) which could
run in any domain.
An example of domain configuration for Virtio I2C:
i2c = [ "" ]
Also to make this work on Arm, allocate Virtio MMIO params (IRQ and
memory region) and pass them to the backend. Update guest device-tree as
wellto create a DT node for the virtio I2C device.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
tools/libs/light/Makefile | 1 +
tools/libs/light/libxl_arm.c | 39 ++++++++
tools/libs/light/libxl_create.c | 5 +
tools/libs/light/libxl_i2c.c | 117 ++++++++++++++++++++++
tools/libs/light/libxl_internal.h | 1 +
tools/libs/light/libxl_types.idl | 22 ++++
tools/libs/light/libxl_types_internal.idl | 1 +
tools/ocaml/libs/xl/genwrap.py | 1 +
tools/ocaml/libs/xl/xenlight_stubs.c | 1 +
tools/xl/xl_parse.c | 63 ++++++++++++
tools/xl/xl_parse.h | 1 +
11 files changed, 252 insertions(+)
create mode 100644 tools/libs/light/libxl_i2c.c
diff --git a/tools/libs/light/Makefile b/tools/libs/light/Makefile
index 13545654c2fc..961bdd33297b 100644
--- a/tools/libs/light/Makefile
+++ b/tools/libs/light/Makefile
@@ -112,6 +112,7 @@ OBJS-y += libxl_vdispl.o
OBJS-y += libxl_pvcalls.o
OBJS-y += libxl_vsnd.o
OBJS-y += libxl_vkb.o
+OBJS-y += libxl_i2c.o
OBJS-y += libxl_genid.o
OBJS-y += _libxl_types.o
OBJS-y += libxl_flask.o
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 2637acafa358..c23ecbcd8528 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -113,6 +113,15 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
}
}
+ for (i = 0; i < d_config->num_i2cs; i++) {
+ libxl_device_i2c *i2c = &d_config->i2cs[i];
+ rc = alloc_virtio_mmio_params(gc, &i2c->base, &i2c->irq,
+ &virtio_mmio_base, &virtio_mmio_irq);
+
+ if (rc)
+ return rc;
+ }
+
/*
* Every virtio-mmio device uses one emulated SPI. If Virtio devices are
* present, make sure that we allocate enough SPIs for them.
@@ -956,6 +965,26 @@ static int make_virtio_mmio_node(libxl__gc *gc, void *fdt, uint64_t base,
return fdt_end_node(fdt);
}
+static int make_virtio_mmio_node_i2c(libxl__gc *gc, void *fdt, uint64_t base,
+ uint32_t irq, uint32_t backend_domid)
+{
+ int res;
+
+ res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid);
+ if (res) return res;
+
+ res = fdt_begin_node(fdt, "i2c");
+ if (res) return res;
+
+ res = fdt_property_compat(gc, fdt, 1, "virtio,device22");
+ if (res) return res;
+
+ res = fdt_end_node(fdt);
+ if (res) return res;
+
+ return fdt_end_node(fdt);
+}
+
static const struct arch_info *get_arch_info(libxl__gc *gc,
const struct xc_dom_image *dom)
{
@@ -1278,6 +1307,16 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
}
}
+ for (i = 0; i < d_config->num_i2cs; i++) {
+ libxl_device_i2c *i2c = &d_config->i2cs[i];
+
+ if (i2c->backend_domid != LIBXL_TOOLSTACK_DOMID)
+ iommu_needed = true;
+
+ FDT( make_virtio_mmio_node_i2c(gc, fdt, i2c->base, i2c->irq,
+ i2c->backend_domid) );
+ }
+
/*
* Note, this should be only called after creating all virtio-mmio
* device nodes
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index b9dd2deedf13..f18a7f829703 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -1797,6 +1797,11 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
&d_config->vkbs[i]);
}
+ for (i = 0; i < d_config->num_i2cs; i++) {
+ libxl__device_add(gc, domid, &libxl__i2c_devtype,
+ &d_config->i2cs[i]);
+ }
+
if (d_config->b_info.arch_arm.vuart == LIBXL_VUART_TYPE_SBSA_UART) {
init_console_info(gc, &vuart, 0);
vuart.backend_domid = state->console_domid;
diff --git a/tools/libs/light/libxl_i2c.c b/tools/libs/light/libxl_i2c.c
new file mode 100644
index 000000000000..ffbde41cc62b
--- /dev/null
+++ b/tools/libs/light/libxl_i2c.c
@@ -0,0 +1,117 @@
+/*
+ * Copyright (C) 2022 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_internal.h"
+
+static int libxl__device_i2c_setdefault(libxl__gc *gc, uint32_t domid,
+ libxl_device_i2c *i2c, bool hotplug)
+{
+ return libxl__resolve_domid(gc, i2c->backend_domname, &i2c->backend_domid);
+}
+
+static int libxl__set_xenstore_i2c(libxl__gc *gc, uint32_t domid,
+ libxl_device_i2c *i2c,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front)
+{
+ flexarray_append_pair(back, "irq", GCSPRINTF("%u", i2c->irq));
+ flexarray_append_pair(back, "base", GCSPRINTF("%lu", i2c->base));
+
+ flexarray_append_pair(front, "irq", GCSPRINTF("%u", i2c->irq));
+ flexarray_append_pair(front, "base", GCSPRINTF("%lu", i2c->base));
+
+ return 0;
+}
+
+static int libxl__i2c_from_xenstore(libxl__gc *gc, const char *libxl_path,
+ libxl_devid devid,
+ libxl_device_i2c *i2c)
+{
+ const char *be_path, *fe_path, *tmp;
+ libxl__device dev;
+ int rc;
+
+ i2c->devid = devid;
+
+ rc = libxl__xs_read_mandatory(gc, XBT_NULL,
+ GCSPRINTF("%s/backend", libxl_path),
+ &be_path);
+ if (rc) goto out;
+
+ rc = libxl__xs_read_mandatory(gc, XBT_NULL,
+ GCSPRINTF("%s/frontend", libxl_path),
+ &fe_path);
+ if (rc) goto out;
+
+ rc = libxl__backendpath_parse_domid(gc, be_path, &i2c->backend_domid);
+ if (rc) goto out;
+
+ rc = libxl__parse_backend_path(gc, be_path, &dev);
+ if (rc) goto out;
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/irq", be_path), &tmp);
+ if (rc) goto out;
+
+ if (tmp) {
+ i2c->irq = strtoul(tmp, NULL, 0);
+ }
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/base", be_path), &tmp);
+ if (rc) goto out;
+
+ if (tmp) {
+ i2c->base = strtoul(tmp, NULL, 0);
+ }
+
+ rc = 0;
+
+out:
+
+ return rc;
+}
+
+static int libxl__device_from_i2c(libxl__gc *gc, uint32_t domid,
+ libxl_device_i2c *type, libxl__device *device)
+{
+ device->backend_devid = type->devid;
+ device->backend_domid = type->backend_domid;
+ device->backend_kind = LIBXL__DEVICE_KIND_I2C;
+ device->devid = type->devid;
+ device->domid = domid;
+ device->kind = LIBXL__DEVICE_KIND_I2C;
+
+ return 0;
+}
+
+static LIBXL_DEFINE_UPDATE_DEVID(i2c)
+
+#define libxl__add_i2cs NULL
+#define libxl_device_i2c_compare NULL
+
+DEFINE_DEVICE_TYPE_STRUCT(i2c, I2C, i2cs,
+ .skip_attach = 1,
+ .set_xenstore_config = (device_set_xenstore_config_fn_t)
+ libxl__set_xenstore_i2c,
+ .from_xenstore = (device_from_xenstore_fn_t)libxl__i2c_from_xenstore
+);
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index cb9e8b3b8b5a..a8904cfea427 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -4003,6 +4003,7 @@ static inline int *libxl__device_type_get_num(
extern const libxl__device_type libxl__vfb_devtype;
extern const libxl__device_type libxl__vkb_devtype;
+extern const libxl__device_type libxl__i2c_devtype;
extern const libxl__device_type libxl__disk_devtype;
extern const libxl__device_type libxl__nic_devtype;
extern const libxl__device_type libxl__vtpm_devtype;
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index d634f304cda2..45e3b2755352 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -278,6 +278,10 @@ libxl_vkb_backend = Enumeration("vkb_backend", [
(2, "LINUX")
])
+libxl_i2c_backend = Enumeration("i2c_backend", [
+ (1, "VIRTIO")
+ ])
+
libxl_passthrough = Enumeration("passthrough", [
(0, "default"),
(1, "disabled"),
@@ -705,6 +709,14 @@ libxl_device_vkb = Struct("device_vkb", [
("multi_touch_num_contacts", uint32)
])
+libxl_device_i2c = Struct("device_i2c", [
+ ("backend_domid", libxl_domid),
+ ("backend_domname", string),
+ ("devid", libxl_devid),
+ ("irq", uint32),
+ ("base", uint64)
+ ])
+
libxl_device_disk = Struct("device_disk", [
("backend_domid", libxl_domid),
("backend_domname", string),
@@ -982,6 +994,7 @@ libxl_domain_config = Struct("domain_config", [
("dtdevs", Array(libxl_device_dtdev, "num_dtdevs")),
("vfbs", Array(libxl_device_vfb, "num_vfbs")),
("vkbs", Array(libxl_device_vkb, "num_vkbs")),
+ ("i2cs", Array(libxl_device_i2c, "num_i2cs")),
("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
("p9s", Array(libxl_device_p9, "num_p9s")),
("pvcallsifs", Array(libxl_device_pvcallsif, "num_pvcallsifs")),
@@ -1145,6 +1158,15 @@ libxl_vkbinfo = Struct("vkbinfo", [
("rref", integer)
], dir=DIR_OUT)
+libxl_i2cinfo = Struct("i2cinfo", [
+ ("backend", string),
+ ("backend_id", uint32),
+ ("frontend", string),
+ ("frontend_id", uint32),
+ ("devid", libxl_devid),
+ ("state", integer),
+ ], dir=DIR_OUT)
+
# NUMA node characteristics: size and free are how much memory it has, and how
# much of it is free, respectively. dists is an array of distances from this
# node to each other node.
diff --git a/tools/libs/light/libxl_types_internal.idl b/tools/libs/light/libxl_types_internal.idl
index fb0f4f23d7c2..b1a94a963dfe 100644
--- a/tools/libs/light/libxl_types_internal.idl
+++ b/tools/libs/light/libxl_types_internal.idl
@@ -33,6 +33,7 @@ libxl__device_kind = Enumeration("device_kind", [
(15, "VSND"),
(16, "VINPUT"),
(17, "VIRTIO_DISK"),
+ (18, "I2C"),
])
libxl__console_backend = Enumeration("console_backend", [
diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py
index 7bf26bdcd831..a9db0b97d80f 100644
--- a/tools/ocaml/libs/xl/genwrap.py
+++ b/tools/ocaml/libs/xl/genwrap.py
@@ -36,6 +36,7 @@ DEVICE_LIST = [ ("list", ["ctx", "domid", "t list"]),
functions = { # ( name , [type1,type2,....] )
"device_vfb": DEVICE_FUNCTIONS,
"device_vkb": DEVICE_FUNCTIONS,
+ "device_i2c": DEVICE_FUNCTIONS,
"device_disk": DEVICE_FUNCTIONS + DEVICE_LIST +
[ ("insert", ["ctx", "t", "domid", "?async:'a", "unit", "unit"]),
("of_vdev", ["ctx", "domid", "string", "t"]),
diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c
index 45b8af61c74a..cdf473f4ed57 100644
--- a/tools/ocaml/libs/xl/xenlight_stubs.c
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c
@@ -707,6 +707,7 @@ DEVICE_ADDREMOVE(disk)
DEVICE_ADDREMOVE(nic)
DEVICE_ADDREMOVE(vfb)
DEVICE_ADDREMOVE(vkb)
+DEVICE_ADDREMOVE(i2c)
DEVICE_ADDREMOVE(pci)
_DEVICE_ADDREMOVE(disk, cdrom, insert)
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 1b5381cef033..243c08aa5f36 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1208,6 +1208,66 @@ static void parse_vkb_list(const XLU_Config *config,
if (rc) exit(EXIT_FAILURE);
}
+int parse_i2c_config(libxl_device_i2c *i2c, char *token)
+{
+ char *oparg;
+
+ if (MATCH_OPTION("backend", token, oparg)) {
+ i2c->backend_domname = strdup(oparg);
+ } else if (MATCH_OPTION("irq", token, oparg)) {
+ i2c->irq = strtoul(oparg, NULL, 0);
+ } else if (MATCH_OPTION("base", token, oparg)) {
+ i2c->base = strtoul(oparg, NULL, 0);
+ } else {
+ fprintf(stderr, "Unknown string \"%s\" in i2c spec\n", token);
+ return -1;
+ }
+
+ return 0;
+}
+
+static void parse_i2c_list(const XLU_Config *config,
+ libxl_domain_config *d_config)
+{
+ XLU_ConfigList *i2cs;
+ const char *item;
+ char *buf = NULL;
+ int rc;
+
+ if (!xlu_cfg_get_list (config, "i2c", &i2cs, 0, 0)) {
+ int entry = 0;
+ while ((item = xlu_cfg_get_listitem(i2cs, entry)) != NULL) {
+ libxl_device_i2c *i2c;
+ char *p;
+
+ i2c = ARRAY_EXTEND_INIT(d_config->i2cs,
+ d_config->num_i2cs,
+ libxl_device_i2c_init);
+
+ buf = strdup(item);
+
+ p = strtok (buf, ",");
+ while (p != NULL)
+ {
+ while (*p == ' ') p++;
+
+ rc = parse_i2c_config(i2c, p);
+ if (rc) goto out;
+
+ p = strtok (NULL, ",");
+ }
+
+ entry++;
+ }
+ }
+
+ rc = 0;
+
+out:
+ free(buf);
+ if (rc) exit(EXIT_FAILURE);
+}
+
void parse_config_data(const char *config_source,
const char *config_data,
int config_len,
@@ -2309,8 +2369,10 @@ void parse_config_data(const char *config_source,
d_config->num_vfbs = 0;
d_config->num_vkbs = 0;
+ d_config->num_i2cs = 0;
d_config->vfbs = NULL;
d_config->vkbs = NULL;
+ d_config->i2cs = NULL;
if (!xlu_cfg_get_list (config, "vfb", &cvfbs, 0, 0)) {
while ((buf = xlu_cfg_get_listitem (cvfbs, d_config->num_vfbs)) != NULL) {
@@ -2752,6 +2814,7 @@ void parse_config_data(const char *config_source,
}
parse_vkb_list(config, d_config);
+ parse_i2c_list(config, d_config);
xlu_cfg_get_defbool(config, "xend_suspend_evtchn_compat",
&c_info->xend_suspend_evtchn_compat, 0);
diff --git a/tools/xl/xl_parse.h b/tools/xl/xl_parse.h
index bab2861f8c3e..4b972d525199 100644
--- a/tools/xl/xl_parse.h
+++ b/tools/xl/xl_parse.h
@@ -36,6 +36,7 @@ int parse_nic_config(libxl_device_nic *nic, XLU_Config **config, char *token);
int parse_vdispl_config(libxl_device_vdispl *vdispl, char *token);
int parse_vsnd_item(libxl_device_vsnd *vsnd, const char *spec);
int parse_vkb_config(libxl_device_vkb *vkb, char *token);
+int parse_i2c_config(libxl_device_i2c *i2c, char *token);
int match_option_size(const char *prefix, size_t len,
char *arg, char **argopt);
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V5 1/6] libxl: Add support for Virtio I2C device
2022-09-07 12:32 ` Viresh Kumar
@ 2022-09-07 17:49 ` Julien Grall
2022-09-08 4:24 ` Viresh Kumar
2022-09-08 13:49 ` Anthony PERARD
1 sibling, 1 reply; 20+ messages in thread
From: Julien Grall @ 2022-09-07 17:49 UTC (permalink / raw)
To: Viresh Kumar, Anthony PERARD
Cc: xen-devel, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross
Hi Viresh,
Jumping in the conversation.
On 07/09/2022 13:32, Viresh Kumar wrote:
> I don't have much knowledge of the Xen code and wanted this code for I2C and
> GPIO to be tested on Xen for the work we are doing around hypervisor agnostic
> backends [1].
>
> I started looking for a simple device's implementation and began by (blindly)
> copying code from Keyboard device and so much of wasted code in here, which
> isn't really required.
>
> On 06-09-22, 17:15, Anthony PERARD wrote:
>> On Mon, Aug 22, 2022 at 02:45:13PM +0530, Viresh Kumar wrote:
>>> An example of domain configuration for Virtio I2c:
>>> i2c = [ "" ]
>>
>> Is this doing something meaningful (with the whole series applied)?
>
> If I am not wrong, this is required by parse_i2c_list()'s implementation.
> Without this I don't get the I2C device populated in the guest.
>
> Is there another way to achieve it ?
Looking at this series, you will add ~250 lines (assuming your new
patch) for the i2c and then likely the same amount for GPIO.
I am assuming that for every new virtio device (e.g. gps, sound,
display...), we would also need to 250 lines of code. I am worry that we
will end up to bloat libxl with duplicated code and or for device that
are barely used.
I think it would be better to find a generic way to add new virtio
device without adding code (very limited) in libxl. The advantage is
someone will be able to create a new virtio device with less effort.
The approach I can think of is something along the lines:
virtio = ["type=<compatible>,transport=<transport>,..."]
where the compatible is the one that should be written in the DT and
transport is mmio or pci. the [...] refers to specific parameters that
would need to be passed to the backend (it is not clear how you provide
them today?).
AFAICT, the GPIO one may need some tweaking because it requires specific
properties. I think it would be more acceptable as this will be only a
few lines (compare to 250 lines today).
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V5 1/6] libxl: Add support for Virtio I2C device
2022-09-07 17:49 ` Julien Grall
@ 2022-09-08 4:24 ` Viresh Kumar
2022-09-08 6:50 ` Juergen Gross
0 siblings, 1 reply; 20+ messages in thread
From: Viresh Kumar @ 2022-09-08 4:24 UTC (permalink / raw)
To: Julien Grall
Cc: Anthony PERARD, xen-devel, Vincent Guittot, stratos-dev,
Alex Bennée, Stefano Stabellini, Mathieu Poirier,
Mike Holmes, Oleksandr Tyshchenko, Wei Liu, Juergen Gross
On 07-09-22, 18:49, Julien Grall wrote:
> Looking at this series, you will add ~250 lines (assuming your new patch)
> for the i2c and then likely the same amount for GPIO.
>
> I am assuming that for every new virtio device (e.g. gps, sound,
> display...), we would also need to 250 lines of code. I am worry that we
> will end up to bloat libxl with duplicated code and or for device that are
> barely used.
I agree.
> I think it would be better to find a generic way to add new virtio device
> without adding code (very limited) in libxl. The advantage is someone will
> be able to create a new virtio device with less effort.
>
> The approach I can think of is something along the lines:
>
> virtio = ["type=<compatible>,transport=<transport>,..."]
It sounds good and yeah it will save a lot of trouble.
> where the compatible is the one that should be written in the DT and
> transport is mmio or pci. the [...] refers to specific parameters that would
> need to be passed to the backend (it is not clear how you provide them
> today?).
The backend doesn't need lot of parameters to be passed right now, the host
specific ones (like which devices on a bus to share to the guests) are passed by
its command line. The backends in our case are hypervisor agnostic and are run
independently as daemons, they just need to know base/irq, which we get from xen
libraries.
> AFAICT, the GPIO one may need some tweaking because it requires specific
> properties. I think it would be more acceptable as this will be only a few
> lines (compare to 250 lines today).
Right.
If everyone agrees, I will be happy to work towards this.
--
viresh
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V5 1/6] libxl: Add support for Virtio I2C device
2022-09-08 4:24 ` Viresh Kumar
@ 2022-09-08 6:50 ` Juergen Gross
0 siblings, 0 replies; 20+ messages in thread
From: Juergen Gross @ 2022-09-08 6:50 UTC (permalink / raw)
To: Viresh Kumar, Julien Grall
Cc: Anthony PERARD, xen-devel, Vincent Guittot, stratos-dev,
Alex Bennée, Stefano Stabellini, Mathieu Poirier,
Mike Holmes, Oleksandr Tyshchenko, Wei Liu
[-- Attachment #1.1.1: Type: text/plain, Size: 2197 bytes --]
On 08.09.22 06:24, Viresh Kumar wrote:
> On 07-09-22, 18:49, Julien Grall wrote:
>> Looking at this series, you will add ~250 lines (assuming your new patch)
>> for the i2c and then likely the same amount for GPIO.
>>
>> I am assuming that for every new virtio device (e.g. gps, sound,
>> display...), we would also need to 250 lines of code. I am worry that we
>> will end up to bloat libxl with duplicated code and or for device that are
>> barely used.
>
> I agree.
>
>> I think it would be better to find a generic way to add new virtio device
>> without adding code (very limited) in libxl. The advantage is someone will
>> be able to create a new virtio device with less effort.
>>
>> The approach I can think of is something along the lines:
>>
>> virtio = ["type=<compatible>,transport=<transport>,..."]
>
> It sounds good and yeah it will save a lot of trouble.
Yes, I'd like that, too.
>
>> where the compatible is the one that should be written in the DT and
>> transport is mmio or pci. the [...] refers to specific parameters that would
>> need to be passed to the backend (it is not clear how you provide them
>> today?).
>
> The backend doesn't need lot of parameters to be passed right now, the host
> specific ones (like which devices on a bus to share to the guests) are passed by
> its command line. The backends in our case are hypervisor agnostic and are run
> independently as daemons, they just need to know base/irq, which we get from xen
> libraries.
When using qemu as a backend this will be quite different.
For my prototype "virtio using grants" I needed to add the following parameters
when starting qemu just for one virtio disk:
-drive file=/home/vm/sle15sp1/image,if=none,id=drive-virtio-disk0,format=raw'
-device
virtio-blk-pci,scsi=off,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=0,disable-legacy=on
As in commit 66dd1c62b2a3c70 I'd suggest to add "backendtype=" in order to
support further special handling for e.g. qemu. I guess your backendtype would
probably be "standalone", but in case you need further parameters from xl/libxl
you could add another type suiting your needs.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH V5 1/6] libxl: Add support for Virtio I2C device
2022-09-07 12:32 ` Viresh Kumar
2022-09-07 17:49 ` Julien Grall
@ 2022-09-08 13:49 ` Anthony PERARD
1 sibling, 0 replies; 20+ messages in thread
From: Anthony PERARD @ 2022-09-08 13:49 UTC (permalink / raw)
To: Viresh Kumar
Cc: xen-devel, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall
On Wed, Sep 07, 2022 at 06:02:24PM +0530, Viresh Kumar wrote:
> Hi Anthony,
>
> Thanks a lot for the in-depth review, it is really helpful.
>
> I don't have much knowledge of the Xen code and wanted this code for I2C and
> GPIO to be tested on Xen for the work we are doing around hypervisor agnostic
> backends [1].
>
> I started looking for a simple device's implementation and began by (blindly)
> copying code from Keyboard device and so much of wasted code in here, which
> isn't really required.
>
> On 06-09-22, 17:15, Anthony PERARD wrote:
> > On Mon, Aug 22, 2022 at 02:45:13PM +0530, Viresh Kumar wrote:
> > > An example of domain configuration for Virtio I2c:
> > > i2c = [ "" ]
> >
> > Is this doing something meaningful (with the whole series applied)?
>
> If I am not wrong, this is required by parse_i2c_list()'s implementation.
> Without this I don't get the I2C device populated in the guest.
Sorry, I should have added more context to may question. In my mind, I2C
is just a communication protocol between a device and a CPU, like USB or
PCI. So to me there is missing information about which device or kind of
device to present to a guest in this example. For example, for PCI we
give a path to the device (via bdf=), and for USB it can be a kind of
device to emulate or select a device based on the path to it (bus,
addr).
Cheers,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V5 2/6] libxl: Add support for Virtio GPIO device
2022-08-22 9:15 [PATCH V5 0/6] Virtio toolstack support for I2C and GPIO on Arm Viresh Kumar
2022-08-22 9:15 ` [PATCH V5 1/6] libxl: Add support for Virtio I2C device Viresh Kumar
@ 2022-08-22 9:15 ` Viresh Kumar
2022-09-06 16:16 ` Anthony PERARD
2022-08-22 9:15 ` [PATCH V5 3/6] libxl: arm: Create alloc_virtio_mmio_params() Viresh Kumar
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Viresh Kumar @ 2022-08-22 9:15 UTC (permalink / raw)
To: xen-devel
Cc: Viresh Kumar, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall,
anthony.perard
This patch adds basic support for configuring and assisting virtio-mmio
based virtio-gpio backend (emualator) which is intended to run out of
Qemu and could be run in any domain.
An example of domain configuration for Virtio Gpio:
gpio = [ "" ]
Please note, this patch is not enough for virtio-gpio to work on Xen
(Arm), as for every Virtio device we need to allocate Virtio MMIO params
(IRQ and memory region) and pass them to the backend, also update Guest
device-tree. A subsequent patch will add these missing bits. For the
current patch, the default "irq" and "base" are just written to the
Xenstore.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
tools/golang/xenlight/helpers.gen.go | 108 ++++++++++-
tools/golang/xenlight/types.gen.go | 27 +++
tools/include/libxl.h | 32 +++
tools/include/libxl_utils.h | 3 +
tools/libs/light/Makefile | 1 +
tools/libs/light/libxl_create.c | 13 ++
tools/libs/light/libxl_dm.c | 17 +-
tools/libs/light/libxl_gpio.c | 226 ++++++++++++++++++++++
tools/libs/light/libxl_internal.h | 1 +
tools/libs/light/libxl_types.idl | 24 +++
tools/libs/light/libxl_types_internal.idl | 1 +
tools/ocaml/libs/xl/genwrap.py | 1 +
tools/ocaml/libs/xl/xenlight_stubs.c | 1 +
tools/xl/Makefile | 2 +-
tools/xl/xl.h | 3 +
tools/xl/xl_cmdtable.c | 15 ++
tools/xl/xl_gpio.c | 142 ++++++++++++++
tools/xl/xl_parse.c | 80 ++++++++
tools/xl/xl_parse.h | 1 +
tools/xl/xl_sxp.c | 2 +
20 files changed, 696 insertions(+), 4 deletions(-)
create mode 100644 tools/libs/light/libxl_gpio.c
create mode 100644 tools/xl/xl_gpio.c
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index 4c7b60439bf0..2f41ace71c61 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1221,6 +1221,9 @@ x.Usbdevice = C.GoString(tmp.usbdevice)
if err := x.VkbDevice.fromC(&tmp.vkb_device);err != nil {
return fmt.Errorf("converting field VkbDevice: %v", err)
}
+if err := x.GpioDevice.fromC(&tmp.gpio_device);err != nil {
+return fmt.Errorf("converting field GpioDevice: %v", err)
+}
if err := x.I2cDevice.fromC(&tmp.i2c_device);err != nil {
return fmt.Errorf("converting field I2cDevice: %v", err)
}
@@ -1541,6 +1544,9 @@ hvm.usbdevice = C.CString(tmp.Usbdevice)}
if err := tmp.VkbDevice.toC(&hvm.vkb_device); err != nil {
return fmt.Errorf("converting field VkbDevice: %v", err)
}
+if err := tmp.GpioDevice.toC(&hvm.gpio_device); err != nil {
+return fmt.Errorf("converting field GpioDevice: %v", err)
+}
if err := tmp.I2cDevice.toC(&hvm.i2c_device); err != nil {
return fmt.Errorf("converting field I2cDevice: %v", err)
}
@@ -1740,6 +1746,46 @@ xc.multi_touch_num_contacts = C.uint32_t(x.MultiTouchNumContacts)
return nil
}
+// NewDeviceGpio returns an instance of DeviceGpio initialized with defaults.
+func NewDeviceGpio() (*DeviceGpio, error) {
+var (
+x DeviceGpio
+xc C.libxl_device_gpio)
+
+C.libxl_device_gpio_init(&xc)
+defer C.libxl_device_gpio_dispose(&xc)
+
+if err := x.fromC(&xc); err != nil {
+return nil, err }
+
+return &x, nil}
+
+func (x *DeviceGpio) fromC(xc *C.libxl_device_gpio) error {
+ x.BackendDomid = Domid(xc.backend_domid)
+x.BackendDomname = C.GoString(xc.backend_domname)
+x.Devid = Devid(xc.devid)
+x.BackendType = GpioBackend(xc.backend_type)
+x.Irq = uint32(xc.irq)
+x.Base = uint64(xc.base)
+
+ return nil}
+
+func (x *DeviceGpio) toC(xc *C.libxl_device_gpio) (err error){defer func(){
+if err != nil{
+C.libxl_device_gpio_dispose(xc)}
+}()
+
+xc.backend_domid = C.libxl_domid(x.BackendDomid)
+if x.BackendDomname != "" {
+xc.backend_domname = C.CString(x.BackendDomname)}
+xc.devid = C.libxl_devid(x.Devid)
+xc.backend_type = C.libxl_gpio_backend(x.BackendType)
+xc.irq = C.uint32_t(x.Irq)
+xc.base = C.uint64_t(x.Base)
+
+ return nil
+ }
+
// NewDeviceI2c returns an instance of DeviceI2c initialized with defaults.
func NewDeviceI2c() (*DeviceI2c, error) {
var (
@@ -2913,6 +2959,15 @@ if err := x.Vkbs[i].fromC(&v); err != nil {
return fmt.Errorf("converting field Vkbs: %v", err) }
}
}
+x.Gpios = nil
+if n := int(xc.num_gpios); n > 0 {
+cGpios := (*[1<<28]C.libxl_device_gpio)(unsafe.Pointer(xc.gpios))[:n:n]
+x.Gpios = make([]DeviceGpio, n)
+for i, v := range cGpios {
+if err := x.Gpios[i].fromC(&v); err != nil {
+return fmt.Errorf("converting field Gpios: %v", err) }
+}
+}
x.I2cs = nil
if n := int(xc.num_i2cs); n > 0 {
cI2cs := (*[1<<28]C.libxl_device_i2c)(unsafe.Pointer(xc.i2cs))[:n:n]
@@ -3083,6 +3138,16 @@ return fmt.Errorf("converting field Vkbs: %v", err)
}
}
}
+if numGpios := len(x.Gpios); numGpios > 0 {
+xc.gpios = (*C.libxl_device_gpio)(C.malloc(C.ulong(numGpios)*C.sizeof_libxl_device_gpio))
+xc.num_gpios = C.int(numGpios)
+cGpios := (*[1<<28]C.libxl_device_gpio)(unsafe.Pointer(xc.gpios))[:numGpios:numGpios]
+for i,v := range x.Gpios {
+if err := v.toC(&cGpios[i]); err != nil {
+return fmt.Errorf("converting field Gpios: %v", err)
+}
+}
+}
if numI2cs := len(x.I2cs); numI2cs > 0 {
xc.i2cs = (*C.libxl_device_i2c)(C.malloc(C.ulong(numI2cs)*C.sizeof_libxl_device_i2c))
xc.num_i2cs = C.int(numI2cs)
@@ -3798,6 +3863,47 @@ xc.rref = C.int(x.Rref)
return nil
}
+// NewGpioinfo returns an instance of Gpioinfo initialized with defaults.
+func NewGpioinfo() (*Gpioinfo, error) {
+var (
+x Gpioinfo
+xc C.libxl_gpioinfo)
+
+C.libxl_gpioinfo_init(&xc)
+defer C.libxl_gpioinfo_dispose(&xc)
+
+if err := x.fromC(&xc); err != nil {
+return nil, err }
+
+return &x, nil}
+
+func (x *Gpioinfo) fromC(xc *C.libxl_gpioinfo) error {
+ x.Backend = C.GoString(xc.backend)
+x.BackendId = uint32(xc.backend_id)
+x.Frontend = C.GoString(xc.frontend)
+x.FrontendId = uint32(xc.frontend_id)
+x.Devid = Devid(xc.devid)
+x.State = int(xc.state)
+
+ return nil}
+
+func (x *Gpioinfo) toC(xc *C.libxl_gpioinfo) (err error){defer func(){
+if err != nil{
+C.libxl_gpioinfo_dispose(xc)}
+}()
+
+if x.Backend != "" {
+xc.backend = C.CString(x.Backend)}
+xc.backend_id = C.uint32_t(x.BackendId)
+if x.Frontend != "" {
+xc.frontend = C.CString(x.Frontend)}
+xc.frontend_id = C.uint32_t(x.FrontendId)
+xc.devid = C.libxl_devid(x.Devid)
+xc.state = C.int(x.State)
+
+ return nil
+ }
+
// NewI2cinfo returns an instance of I2cinfo initialized with defaults.
func NewI2cinfo() (*I2cinfo, error) {
var (
@@ -3819,8 +3925,6 @@ x.Frontend = C.GoString(xc.frontend)
x.FrontendId = uint32(xc.frontend_id)
x.Devid = Devid(xc.devid)
x.State = int(xc.state)
-x.Evtch = int(xc.evtch)
-x.Rref = int(xc.rref)
return nil}
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index f8d7e84d6180..5a4656035cd1 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -255,6 +255,11 @@ VkbBackendQemu VkbBackend = 1
VkbBackendLinux VkbBackend = 2
)
+type GpioBackend int
+const(
+GpioBackendVirtio GpioBackend = 0
+)
+
type I2cBackend int
const(
I2cBackendVirtio I2cBackend = 0
@@ -589,6 +594,7 @@ Usb Defbool
Usbversion int
Usbdevice string
VkbDevice Defbool
+GpioDevice Defbool
I2cDevice Defbool
Soundhw string
XenPlatformPci Defbool
@@ -652,6 +658,15 @@ MultiTouchHeight uint32
MultiTouchNumContacts uint32
}
+type DeviceGpio struct {
+BackendDomid Domid
+BackendDomname string
+Devid Devid
+BackendType GpioBackend
+Irq uint32
+Base uint64
+}
+
type DeviceI2c struct {
BackendDomid Domid
BackendDomname string
@@ -950,6 +965,7 @@ Rdms []DeviceRdm
Dtdevs []DeviceDtdev
Vfbs []DeviceVfb
Vkbs []DeviceVkb
+Gpios []DeviceGpio
I2cs []DeviceI2c
Vtpms []DeviceVtpm
P9S []DeviceP9
@@ -1107,6 +1123,17 @@ Evtch int
Rref int
}
+type Gpioinfo struct {
+Backend string
+BackendId uint32
+Frontend string
+FrontendId uint32
+Devid Devid
+State int
+Evtch int
+Rref int
+}
+
type I2cinfo struct {
Backend string
BackendId uint32
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index ab18c0b8c794..3bbb6d031372 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -829,6 +829,15 @@ typedef struct libxl__ctx libxl_ctx;
*/
#define LIBXL_HAVE_BUILDINFO_VKB_DEVICE 1
+/*
+ * LIBXL_HAVE_BUILDINFO_GPIO_DEVICE
+ *
+ * If this is defined, then the libxl_domain_build_info structure will
+ * contain a boolean hvm.gpio_device which instructs libxl whether to include
+ * a gpio at build time or not.
+ */
+#define LIBXL_HAVE_BUILDINFO_GPIO_DEVICE 1
+
/*
* LIBXL_HAVE_BUILDINFO_I2C_DEVICE
*
@@ -2406,6 +2415,29 @@ int libxl_device_vkb_getinfo(libxl_ctx *ctx, uint32_t domid,
libxl_vkbinfo *vkbinfo)
LIBXL_EXTERNAL_CALLERS_ONLY;
+/* GPIO */
+int libxl_device_gpio_add(libxl_ctx *ctx, uint32_t domid, libxl_device_gpio *gpio,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_gpio_remove(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_gpio *gpio,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_gpio_destroy(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_gpio *gpio,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+libxl_device_gpio *libxl_device_gpio_list(libxl_ctx *ctx,
+ uint32_t domid, int *num)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+void libxl_device_gpio_list_free(libxl_device_gpio* list, int num)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_gpio_getinfo(libxl_ctx *ctx, uint32_t domid,
+ const libxl_device_gpio *gpio,
+ libxl_gpioinfo *gpioinfo)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
/* I2C */
int libxl_device_i2c_add(libxl_ctx *ctx, uint32_t domid, libxl_device_i2c *i2c,
const libxl_asyncop_how *ao_how)
diff --git a/tools/include/libxl_utils.h b/tools/include/libxl_utils.h
index 72b7796b7e4b..91f5c68172cb 100644
--- a/tools/include/libxl_utils.h
+++ b/tools/include/libxl_utils.h
@@ -83,6 +83,9 @@ int libxl_devid_to_device_usbctrl(libxl_ctx *ctx, uint32_t domid,
int libxl_devid_to_device_vkb(libxl_ctx *ctx, uint32_t domid,
int devid, libxl_device_vkb *vkb);
+int libxl_devid_to_device_gpio(libxl_ctx *ctx, uint32_t domid,
+ int devid, libxl_device_gpio *gpio);
+
int libxl_devid_to_device_i2c(libxl_ctx *ctx, uint32_t domid,
int devid, libxl_device_i2c *i2c);
diff --git a/tools/libs/light/Makefile b/tools/libs/light/Makefile
index 961bdd33297b..7fae83100829 100644
--- a/tools/libs/light/Makefile
+++ b/tools/libs/light/Makefile
@@ -113,6 +113,7 @@ OBJS-y += libxl_pvcalls.o
OBJS-y += libxl_vsnd.o
OBJS-y += libxl_vkb.o
OBJS-y += libxl_i2c.o
+OBJS-y += libxl_gpio.o
OBJS-y += libxl_genid.o
OBJS-y += _libxl_types.o
OBJS-y += libxl_flask.o
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 84fe9f80c8fe..98a27ff8c005 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -375,6 +375,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_defbool_setdefault(&b_info->u.hvm.altp2m, false);
libxl_defbool_setdefault(&b_info->u.hvm.usb, false);
libxl_defbool_setdefault(&b_info->u.hvm.vkb_device, true);
+ libxl_defbool_setdefault(&b_info->u.hvm.gpio_device, true);
libxl_defbool_setdefault(&b_info->u.hvm.i2c_device, true);
libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
@@ -1754,6 +1755,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
libxl__device_console console;
libxl__device device;
libxl_device_vkb vkb;
+ libxl_device_gpio gpio;
libxl_device_i2c i2c;
init_console_info(gc, &console, 0);
@@ -1767,6 +1769,12 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
libxl_device_vkb_dispose(&vkb);
}
+ if (libxl_defbool_val(d_config->b_info.u.hvm.gpio_device)) {
+ libxl_device_gpio_init(&gpio);
+ libxl__device_add(gc, domid, &libxl__gpio_devtype, &gpio);
+ libxl_device_gpio_dispose(&gpio);
+ }
+
if (libxl_defbool_val(d_config->b_info.u.hvm.i2c_device)) {
libxl_device_i2c_init(&i2c);
libxl__device_add(gc, domid, &libxl__i2c_devtype, &i2c);
@@ -1805,6 +1813,11 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
&d_config->vkbs[i]);
}
+ for (i = 0; i < d_config->num_gpios; i++) {
+ libxl__device_add(gc, domid, &libxl__gpio_devtype,
+ &d_config->gpios[i]);
+ }
+
for (i = 0; i < d_config->num_i2cs; i++) {
libxl__device_add(gc, domid, &libxl__i2c_devtype,
&d_config->i2cs[i]);
diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 362c0596f497..95d5606870c1 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -2113,6 +2113,7 @@ static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
const libxl_domain_config *guest_config,
libxl_device_vfb *vfb,
libxl_device_vkb *vkb,
+ libxl_device_gpio *gpio,
libxl_device_i2c *i2c)
{
const libxl_domain_build_info *b_info = &guest_config->b_info;
@@ -2122,6 +2123,7 @@ static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
libxl_device_vfb_init(vfb);
libxl_device_vkb_init(vkb);
+ libxl_device_gpio_init(gpio);
libxl_device_i2c_init(i2c);
vfb->backend_domid = 0;
@@ -2133,6 +2135,9 @@ static int libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
vkb->backend_domid = 0;
vkb->devid = 0;
+ gpio->backend_domid = 0;
+ gpio->devid = 0;
+
i2c->backend_domid = 0;
i2c->devid = 0;
@@ -2298,6 +2303,7 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
int ret;
libxl_device_vfb *vfb;
libxl_device_vkb *vkb;
+ libxl_device_gpio *gpio;
libxl_device_i2c *i2c;
char **args;
struct xs_permissions perm[2];
@@ -2371,12 +2377,15 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
|| libxl_defbool_val(guest_config->b_info.u.hvm.sdl.enable)) {
GCNEW(vfb);
GCNEW(vkb);
+ GCNEW(gpio);
GCNEW(i2c);
- libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, vkb, i2c);
+ libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, vkb, gpio, i2c);
dm_config->vfbs = vfb;
dm_config->num_vfbs = 1;
dm_config->vkbs = vkb;
dm_config->num_vkbs = 1;
+ dm_config->gpios = gpio;
+ dm_config->num_gpios = 1;
dm_config->i2cs = i2c;
dm_config->num_i2cs = 1;
}
@@ -2520,6 +2529,12 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
if (ret) goto out;
}
+ if (dm_config->num_gpios) {
+ ret = libxl__device_add(gc, dm_domid, &libxl__gpio_devtype,
+ &dm_config->gpios[0]);
+ if (ret) goto out;
+ }
+
if (dm_config->num_i2cs) {
ret = libxl__device_add(gc, dm_domid, &libxl__i2c_devtype,
&dm_config->i2cs[0]);
diff --git a/tools/libs/light/libxl_gpio.c b/tools/libs/light/libxl_gpio.c
new file mode 100644
index 000000000000..1597f0ac7cea
--- /dev/null
+++ b/tools/libs/light/libxl_gpio.c
@@ -0,0 +1,226 @@
+/*
+ * Copyright (C) 2022 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_internal.h"
+
+static int libxl__device_gpio_setdefault(libxl__gc *gc, uint32_t domid,
+ libxl_device_gpio *gpio, bool hotplug)
+{
+ if (gpio->backend_type != LIBXL_GPIO_BACKEND_VIRTIO) {
+ gpio->backend_type = LIBXL_GPIO_BACKEND_VIRTIO;
+ }
+
+ return libxl__resolve_domid(gc, gpio->backend_domname, &gpio->backend_domid);
+}
+
+static int libxl__device_gpio_dm_needed(void *e, uint32_t domid)
+{
+ libxl_device_gpio *elem = e;
+
+ return elem->backend_type == LIBXL_GPIO_BACKEND_VIRTIO;
+}
+
+static int libxl__set_xenstore_gpio(libxl__gc *gc, uint32_t domid,
+ libxl_device_gpio *gpio,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front)
+{
+ flexarray_append_pair(back, "irq", GCSPRINTF("%u", gpio->irq));
+ flexarray_append_pair(back, "base", GCSPRINTF("%lu", gpio->base));
+
+ flexarray_append_pair(front, "irq", GCSPRINTF("%u", gpio->irq));
+ flexarray_append_pair(front, "base", GCSPRINTF("%lu", gpio->base));
+
+ return 0;
+}
+
+static int libxl__gpio_from_xenstore(libxl__gc *gc, const char *libxl_path,
+ libxl_devid devid,
+ libxl_device_gpio *gpio)
+{
+ const char *be_path, *fe_path, *tmp;
+ libxl__device dev;
+ int rc;
+
+ gpio->devid = devid;
+
+ rc = libxl__xs_read_mandatory(gc, XBT_NULL,
+ GCSPRINTF("%s/backend", libxl_path),
+ &be_path);
+ if (rc) goto out;
+
+ rc = libxl__xs_read_mandatory(gc, XBT_NULL,
+ GCSPRINTF("%s/frontend", libxl_path),
+ &fe_path);
+ if (rc) goto out;
+
+ rc = libxl__backendpath_parse_domid(gc, be_path, &gpio->backend_domid);
+ if (rc) goto out;
+
+ rc = libxl__parse_backend_path(gc, be_path, &dev);
+ if (rc) goto out;
+
+ gpio->backend_type = LIBXL_GPIO_BACKEND_VIRTIO;
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/irq", be_path), &tmp);
+ if (rc) goto out;
+
+ if (tmp) {
+ gpio->irq = strtoul(tmp, NULL, 0);
+ }
+
+ rc = libxl__xs_read_checked(gc, XBT_NULL,
+ GCSPRINTF("%s/base", be_path), &tmp);
+ if (rc) goto out;
+
+ if (tmp) {
+ gpio->base = strtoul(tmp, NULL, 0);
+ }
+
+ rc = 0;
+
+out:
+
+ return rc;
+}
+
+static int libxl__device_from_gpio(libxl__gc *gc, uint32_t domid,
+ libxl_device_gpio *type, libxl__device *device)
+{
+ device->backend_devid = type->devid;
+ device->backend_domid = type->backend_domid;
+ device->backend_kind = LIBXL__DEVICE_KIND_GPIO;
+ device->devid = type->devid;
+ device->domid = domid;
+ device->kind = LIBXL__DEVICE_KIND_GPIO;
+
+ return 0;
+}
+
+int libxl_device_gpio_add(libxl_ctx *ctx, uint32_t domid, libxl_device_gpio *gpio,
+ const libxl_asyncop_how *ao_how)
+{
+ AO_CREATE(ctx, domid, ao_how);
+ int rc;
+
+ rc = libxl__device_add(gc, domid, &libxl__gpio_devtype, gpio);
+ if (rc) {
+ LOGD(ERROR, domid, "Unable to add gpio device");
+ goto out;
+ }
+
+out:
+ libxl__ao_complete(egc, ao, rc);
+ return AO_INPROGRESS;
+}
+
+int libxl_devid_to_device_gpio(libxl_ctx *ctx, uint32_t domid,
+ int devid, libxl_device_gpio *gpio)
+{
+ GC_INIT(ctx);
+
+ libxl_device_gpio *gpios = NULL;
+ int n, i;
+ int rc;
+
+ libxl_device_gpio_init(gpio);
+
+ gpios = libxl__device_list(gc, &libxl__gpio_devtype, domid, &n);
+
+ if (!gpios) { rc = ERROR_NOTFOUND; goto out; }
+
+ for (i = 0; i < n; ++i) {
+ if (devid == gpios[i].devid) {
+ libxl_device_gpio_copy(ctx, gpio, &gpios[i]);
+ rc = 0;
+ goto out;
+ }
+ }
+
+ rc = ERROR_NOTFOUND;
+
+out:
+
+ if (gpios)
+ libxl__device_list_free(&libxl__gpio_devtype, gpios, n);
+
+ GC_FREE;
+ return rc;
+}
+
+int libxl_device_gpio_getinfo(libxl_ctx *ctx, uint32_t domid,
+ const libxl_device_gpio *gpio,
+ libxl_gpioinfo *info)
+{
+ GC_INIT(ctx);
+ char *libxl_path, *dompath, *devpath;
+ char *val;
+ int rc;
+
+ libxl_gpioinfo_init(info);
+ dompath = libxl__xs_get_dompath(gc, domid);
+ info->devid = gpio->devid;
+
+ devpath = libxl__domain_device_frontend_path(gc, domid, info->devid,
+ LIBXL__DEVICE_KIND_GPIO);
+ libxl_path = libxl__domain_device_libxl_path(gc, domid, info->devid,
+ LIBXL__DEVICE_KIND_GPIO);
+
+ info->backend = xs_read(ctx->xsh, XBT_NULL,
+ GCSPRINTF("%s/backend", libxl_path),
+ NULL);
+ if (!info->backend) { rc = ERROR_FAIL; goto out; }
+
+ rc = libxl__backendpath_parse_domid(gc, info->backend, &info->backend_id);
+ if (rc) goto out;
+
+ val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/state", devpath));
+ info->state = val ? strtoul(val, NULL, 10) : -1;
+
+ info->frontend = xs_read(ctx->xsh, XBT_NULL,
+ GCSPRINTF("%s/frontend", libxl_path),
+ NULL);
+ info->frontend_id = domid;
+
+ rc = 0;
+
+out:
+ GC_FREE;
+ return rc;
+}
+
+static LIBXL_DEFINE_UPDATE_DEVID(gpio)
+
+#define libxl__add_gpios NULL
+#define libxl_device_gpio_compare NULL
+
+LIBXL_DEFINE_DEVICE_LIST(gpio)
+LIBXL_DEFINE_DEVICE_REMOVE(gpio)
+
+DEFINE_DEVICE_TYPE_STRUCT(gpio, GPIO, gpios,
+ .skip_attach = 1,
+ .dm_needed = libxl__device_gpio_dm_needed,
+ .set_xenstore_config = (device_set_xenstore_config_fn_t)
+ libxl__set_xenstore_gpio,
+ .from_xenstore = (device_from_xenstore_fn_t)libxl__gpio_from_xenstore
+);
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index a8904cfea427..a64d0cb63179 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -4003,6 +4003,7 @@ static inline int *libxl__device_type_get_num(
extern const libxl__device_type libxl__vfb_devtype;
extern const libxl__device_type libxl__vkb_devtype;
+extern const libxl__device_type libxl__gpio_devtype;
extern const libxl__device_type libxl__i2c_devtype;
extern const libxl__device_type libxl__disk_devtype;
extern const libxl__device_type libxl__nic_devtype;
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 014a3ea8364c..4c476a325e9d 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -278,6 +278,10 @@ libxl_vkb_backend = Enumeration("vkb_backend", [
(2, "LINUX")
])
+libxl_gpio_backend = Enumeration("gpio_backend", [
+ (0, "VIRTIO")
+ ])
+
libxl_i2c_backend = Enumeration("i2c_backend", [
(0, "VIRTIO")
])
@@ -630,6 +634,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
# - "mouse" for PS/2 protocol relative mouse
("usbdevice", string),
("vkb_device", libxl_defbool),
+ ("gpio_device", libxl_defbool),
("i2c_device", libxl_defbool),
("soundhw", string),
("xen_platform_pci", libxl_defbool),
@@ -710,6 +715,15 @@ libxl_device_vkb = Struct("device_vkb", [
("multi_touch_num_contacts", uint32)
])
+libxl_device_gpio = Struct("device_gpio", [
+ ("backend_domid", libxl_domid),
+ ("backend_domname", string),
+ ("devid", libxl_devid),
+ ("backend_type", libxl_gpio_backend),
+ ("irq", uint32),
+ ("base", uint64)
+ ])
+
libxl_device_i2c = Struct("device_i2c", [
("backend_domid", libxl_domid),
("backend_domname", string),
@@ -996,6 +1010,7 @@ libxl_domain_config = Struct("domain_config", [
("dtdevs", Array(libxl_device_dtdev, "num_dtdevs")),
("vfbs", Array(libxl_device_vfb, "num_vfbs")),
("vkbs", Array(libxl_device_vkb, "num_vkbs")),
+ ("gpios", Array(libxl_device_gpio, "num_gpios")),
("i2cs", Array(libxl_device_i2c, "num_i2cs")),
("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
("p9s", Array(libxl_device_p9, "num_p9s")),
@@ -1160,6 +1175,15 @@ libxl_vkbinfo = Struct("vkbinfo", [
("rref", integer)
], dir=DIR_OUT)
+libxl_gpioinfo = Struct("gpioinfo", [
+ ("backend", string),
+ ("backend_id", uint32),
+ ("frontend", string),
+ ("frontend_id", uint32),
+ ("devid", libxl_devid),
+ ("state", integer),
+ ], dir=DIR_OUT)
+
libxl_i2cinfo = Struct("i2cinfo", [
("backend", string),
("backend_id", uint32),
diff --git a/tools/libs/light/libxl_types_internal.idl b/tools/libs/light/libxl_types_internal.idl
index b1a94a963dfe..b57469b010ce 100644
--- a/tools/libs/light/libxl_types_internal.idl
+++ b/tools/libs/light/libxl_types_internal.idl
@@ -34,6 +34,7 @@ libxl__device_kind = Enumeration("device_kind", [
(16, "VINPUT"),
(17, "VIRTIO_DISK"),
(18, "I2C"),
+ (19, "GPIO"),
])
libxl__console_backend = Enumeration("console_backend", [
diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py
index a9db0b97d80f..ffab4b362d2a 100644
--- a/tools/ocaml/libs/xl/genwrap.py
+++ b/tools/ocaml/libs/xl/genwrap.py
@@ -36,6 +36,7 @@ DEVICE_LIST = [ ("list", ["ctx", "domid", "t list"]),
functions = { # ( name , [type1,type2,....] )
"device_vfb": DEVICE_FUNCTIONS,
"device_vkb": DEVICE_FUNCTIONS,
+ "device_gpio": DEVICE_FUNCTIONS,
"device_i2c": DEVICE_FUNCTIONS,
"device_disk": DEVICE_FUNCTIONS + DEVICE_LIST +
[ ("insert", ["ctx", "t", "domid", "?async:'a", "unit", "unit"]),
diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c
index cdf473f4ed57..0114cc22a1b6 100644
--- a/tools/ocaml/libs/xl/xenlight_stubs.c
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c
@@ -707,6 +707,7 @@ DEVICE_ADDREMOVE(disk)
DEVICE_ADDREMOVE(nic)
DEVICE_ADDREMOVE(vfb)
DEVICE_ADDREMOVE(vkb)
+DEVICE_ADDREMOVE(gpio)
DEVICE_ADDREMOVE(i2c)
DEVICE_ADDREMOVE(pci)
_DEVICE_ADDREMOVE(disk, cdrom, insert)
diff --git a/tools/xl/Makefile b/tools/xl/Makefile
index 06801962f11e..34ff203bfd86 100644
--- a/tools/xl/Makefile
+++ b/tools/xl/Makefile
@@ -23,7 +23,7 @@ XL_OBJS += xl_vtpm.o xl_block.o xl_nic.o xl_usb.o
XL_OBJS += xl_sched.o xl_pci.o xl_vcpu.o xl_cdrom.o xl_mem.o
XL_OBJS += xl_info.o xl_console.o xl_misc.o
XL_OBJS += xl_vmcontrol.o xl_saverestore.o xl_migrate.o
-XL_OBJS += xl_vdispl.o xl_vsnd.o xl_vkb.o xl_i2c.o
+XL_OBJS += xl_vdispl.o xl_vsnd.o xl_vkb.o xl_gpio.o xl_i2c.o
$(XL_OBJS): CFLAGS += $(CFLAGS_libxentoollog)
$(XL_OBJS): CFLAGS += $(CFLAGS_XL)
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index 28618326149e..dfa557077631 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -176,6 +176,9 @@ int main_vsnddetach(int argc, char **argv);
int main_vkbattach(int argc, char **argv);
int main_vkblist(int argc, char **argv);
int main_vkbdetach(int argc, char **argv);
+int main_gpioattach(int argc, char **argv);
+int main_gpiolist(int argc, char **argv);
+int main_gpiodetach(int argc, char **argv);
int main_i2cattach(int argc, char **argv);
int main_i2clist(int argc, char **argv);
int main_i2cdetach(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index c409ab1578d4..0c0d32b2e25b 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -406,6 +406,21 @@ const struct cmd_spec cmd_table[] = {
"Destroy a domain's virtual keyboard device",
"<Domain> <DevId>",
},
+ { "gpio-attach",
+ &main_gpioattach, 1, 1,
+ "Create a new virtual gpio device",
+ "<Domain> <gpio-spec-component(s)>...",
+ },
+ { "gpio-list",
+ &main_gpiolist, 0, 0,
+ "List virtual gpio devices for a domain",
+ "<Domain(s)>",
+ },
+ { "gpio-detach",
+ &main_gpiodetach, 0, 1,
+ "Destroy a domain's virtual gpio device",
+ "<Domain> <DevId>",
+ },
{ "i2c-attach",
&main_i2cattach, 1, 1,
"Create a new virtual i2c device",
diff --git a/tools/xl/xl_gpio.c b/tools/xl/xl_gpio.c
new file mode 100644
index 000000000000..fa63b01ab180
--- /dev/null
+++ b/tools/xl/xl_gpio.c
@@ -0,0 +1,142 @@
+/*
+ * Copyright (C) 2022 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include <stdlib.h>
+
+#include <libxl.h>
+#include <libxl_utils.h>
+#include <libxlutil.h>
+
+#include "xl.h"
+#include "xl_utils.h"
+#include "xl_parse.h"
+
+int main_gpioattach(int argc, char **argv)
+{
+ int opt;
+ int rc;
+ uint32_t domid;
+ libxl_device_gpio gpio;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "gpio-attach", 2) {
+ /* No options */
+ }
+
+ libxl_device_gpio_init(&gpio);
+ domid = find_domain(argv[optind++]);
+
+ for (argv += optind, argc -= optind; argc > 0; ++argv, --argc) {
+ rc = parse_gpio_config(&gpio, *argv);
+ if (rc) goto out;
+ }
+
+ if (dryrun_only) {
+ char *json = libxl_device_gpio_to_json(ctx, &gpio);
+ printf("gpio: %s\n", json);
+ free(json);
+ goto done;
+ }
+
+ if (libxl_device_gpio_add(ctx, domid, &gpio, 0)) {
+ fprintf(stderr, "libxl_device_gpio_add failed.\n");
+ rc = ERROR_FAIL; goto out;
+ }
+
+done:
+ rc = 0;
+
+out:
+ libxl_device_gpio_dispose(&gpio);
+ return rc;
+}
+
+int main_gpiolist(int argc, char **argv)
+{
+ int opt;
+ libxl_device_gpio *gpios;
+ libxl_gpioinfo gpioinfo;
+ int nb, i;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "gpio-list", 1) {
+ /* No options */
+ }
+
+ /* Idx BE Hdl Sta evch ref ID BE-type BE-path */
+ printf("%-3s %-2s %-6s %-5s %-6s %6s %-10s %-10s %-30s\n",
+ "Idx", "BE", "handle", "state", "evt-ch", "ref",
+ "ID", "BE-type", "BE-path");
+ for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
+ uint32_t domid = find_domain(*argv);
+ gpios = libxl_device_gpio_list(ctx, domid, &nb);
+ if (!gpios) {
+ continue;
+ }
+ for (i = 0; i < nb; ++i) {
+ if (libxl_device_gpio_getinfo(ctx, domid, &gpios[i], &gpioinfo) == 0) {
+ printf("%-3d %-2d %6d %5d %-10s %-30s\n",
+ gpioinfo.devid, gpioinfo.backend_id,
+ gpioinfo.devid, gpioinfo.state,
+ libxl_gpio_backend_to_string(gpios[i].backend_type),
+ gpioinfo.backend);
+ libxl_gpioinfo_dispose(&gpioinfo);
+ }
+ }
+ libxl_device_gpio_list_free(gpios, nb);
+ }
+ return 0;
+}
+
+int main_gpiodetach(int argc, char **argv)
+{
+ uint32_t domid, devid;
+ int opt, rc;
+ libxl_device_gpio gpio;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "gpio-detach", 2) {
+ /* No options */
+ }
+
+ domid = find_domain(argv[optind++]);
+ devid = atoi(argv[optind++]);
+
+ libxl_device_gpio_init(&gpio);
+
+ if (libxl_devid_to_device_gpio(ctx, domid, devid, &gpio)) {
+ fprintf(stderr, "Error: Device %d not connected.\n", devid);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = libxl_device_gpio_remove(ctx, domid, &gpio, 0);
+ if (rc) {
+ fprintf(stderr, "libxl_device_gpio_remove failed.\n");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = 0;
+
+out:
+ libxl_device_gpio_dispose(&gpio);
+ return rc;
+}
+
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index b91ffba14d40..181379cd5d19 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1208,6 +1208,74 @@ static void parse_vkb_list(const XLU_Config *config,
if (rc) exit(EXIT_FAILURE);
}
+int parse_gpio_config(libxl_device_gpio *gpio, char *token)
+{
+ char *oparg;
+
+ if (MATCH_OPTION("backend", token, oparg)) {
+ gpio->backend_domname = strdup(oparg);
+ } else if (MATCH_OPTION("backend-type", token, oparg)) {
+ libxl_gpio_backend backend_type;
+ if (libxl_gpio_backend_from_string(oparg, &backend_type)) {
+ fprintf(stderr, "Unknown backend_type \"%s\" in gpio spec\n",
+ oparg);
+ return -1;
+ }
+ gpio->backend_type = backend_type;
+ } else if (MATCH_OPTION("irq", token, oparg)) {
+ gpio->irq = strtoul(oparg, NULL, 0);
+ } else if (MATCH_OPTION("base", token, oparg)) {
+ gpio->base = strtoul(oparg, NULL, 0);
+ } else {
+ fprintf(stderr, "Unknown string \"%s\" in gpio spec\n", token);
+ return -1;
+ }
+
+ return 0;
+}
+
+static void parse_gpio_list(const XLU_Config *config,
+ libxl_domain_config *d_config)
+{
+ XLU_ConfigList *gpios;
+ const char *item;
+ char *buf = NULL;
+ int rc;
+
+ if (!xlu_cfg_get_list (config, "gpio", &gpios, 0, 0)) {
+ int entry = 0;
+ while ((item = xlu_cfg_get_listitem(gpios, entry)) != NULL) {
+ libxl_device_gpio *gpio;
+ char *p;
+
+ gpio = ARRAY_EXTEND_INIT(d_config->gpios,
+ d_config->num_gpios,
+ libxl_device_gpio_init);
+
+ buf = strdup(item);
+
+ p = strtok (buf, ",");
+ while (p != NULL)
+ {
+ while (*p == ' ') p++;
+
+ rc = parse_gpio_config(gpio, p);
+ if (rc) goto out;
+
+ p = strtok (NULL, ",");
+ }
+
+ entry++;
+ }
+ }
+
+ rc = 0;
+
+out:
+ free(buf);
+ if (rc) exit(EXIT_FAILURE);
+}
+
int parse_i2c_config(libxl_device_i2c *i2c, char *token)
{
char *oparg;
@@ -2377,15 +2445,18 @@ void parse_config_data(const char *config_source,
d_config->num_vfbs = 0;
d_config->num_vkbs = 0;
+ d_config->num_gpios = 0;
d_config->num_i2cs = 0;
d_config->vfbs = NULL;
d_config->vkbs = NULL;
+ d_config->gpios = NULL;
d_config->i2cs = NULL;
if (!xlu_cfg_get_list (config, "vfb", &cvfbs, 0, 0)) {
while ((buf = xlu_cfg_get_listitem (cvfbs, d_config->num_vfbs)) != NULL) {
libxl_device_vfb *vfb;
libxl_device_vkb *vkb;
+ libxl_device_gpio *gpio;
libxl_device_i2c *i2c;
char *buf2 = strdup(buf);
@@ -2397,6 +2468,9 @@ void parse_config_data(const char *config_source,
vkb = ARRAY_EXTEND_INIT(d_config->vkbs, d_config->num_vkbs,
libxl_device_vkb_init);
+ gpio = ARRAY_EXTEND_INIT(d_config->gpios, d_config->num_gpios,
+ libxl_device_gpio_init);
+
i2c = ARRAY_EXTEND_INIT(d_config->i2cs, d_config->num_i2cs,
libxl_device_i2c_init);
@@ -2657,6 +2731,7 @@ void parse_config_data(const char *config_source,
if (vnc_enabled) {
libxl_device_vfb *vfb;
libxl_device_vkb *vkb;
+ libxl_device_gpio *gpio;
libxl_device_i2c *i2c;
vfb = ARRAY_EXTEND_INIT(d_config->vfbs, d_config->num_vfbs,
@@ -2665,6 +2740,9 @@ void parse_config_data(const char *config_source,
vkb = ARRAY_EXTEND_INIT(d_config->vkbs, d_config->num_vkbs,
libxl_device_vkb_init);
+ gpio = ARRAY_EXTEND_INIT(d_config->gpios, d_config->num_gpios,
+ libxl_device_gpio_init);
+
i2c = ARRAY_EXTEND_INIT(d_config->i2cs, d_config->num_i2cs,
libxl_device_i2c_init);
@@ -2782,6 +2860,7 @@ void parse_config_data(const char *config_source,
exit(-ERROR_FAIL);
}
xlu_cfg_get_defbool(config, "vkb_device", &b_info->u.hvm.vkb_device, 0);
+ xlu_cfg_get_defbool(config, "gpio_device", &b_info->u.hvm.gpio_device, 0);
xlu_cfg_get_defbool(config, "i2c_device", &b_info->u.hvm.i2c_device, 0);
xlu_cfg_replace_string (config, "soundhw", &b_info->u.hvm.soundhw, 0);
xlu_cfg_get_defbool(config, "xen_platform_pci",
@@ -2831,6 +2910,7 @@ void parse_config_data(const char *config_source,
}
parse_vkb_list(config, d_config);
+ parse_gpio_list(config, d_config);
parse_i2c_list(config, d_config);
xlu_cfg_get_defbool(config, "xend_suspend_evtchn_compat",
diff --git a/tools/xl/xl_parse.h b/tools/xl/xl_parse.h
index 4b972d525199..6e041abe5d50 100644
--- a/tools/xl/xl_parse.h
+++ b/tools/xl/xl_parse.h
@@ -36,6 +36,7 @@ int parse_nic_config(libxl_device_nic *nic, XLU_Config **config, char *token);
int parse_vdispl_config(libxl_device_vdispl *vdispl, char *token);
int parse_vsnd_item(libxl_device_vsnd *vsnd, const char *spec);
int parse_vkb_config(libxl_device_vkb *vkb, char *token);
+int parse_gpio_config(libxl_device_gpio *gpio, char *token);
int parse_i2c_config(libxl_device_i2c *i2c, char *token);
int match_option_size(const char *prefix, size_t len,
diff --git a/tools/xl/xl_sxp.c b/tools/xl/xl_sxp.c
index a44c765aa515..632e1f338d09 100644
--- a/tools/xl/xl_sxp.c
+++ b/tools/xl/xl_sxp.c
@@ -140,6 +140,8 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh)
fprintf(fh, "\t\t\t(usbdevice %s)\n", b_info->u.hvm.usbdevice);
fprintf(fh, "\t\t\t(vkb_device %s)\n",
libxl_defbool_to_string(b_info->u.hvm.vkb_device));
+ fprintf(fh, "\t\t\t(gpio_device %s)\n",
+ libxl_defbool_to_string(b_info->u.hvm.gpio_device));
fprintf(fh, "\t\t\t(i2c_device %s)\n",
libxl_defbool_to_string(b_info->u.hvm.i2c_device));
fprintf(fh, "\t\t)\n");
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V5 2/6] libxl: Add support for Virtio GPIO device
2022-08-22 9:15 ` [PATCH V5 2/6] libxl: Add support for Virtio GPIO device Viresh Kumar
@ 2022-09-06 16:16 ` Anthony PERARD
0 siblings, 0 replies; 20+ messages in thread
From: Anthony PERARD @ 2022-09-06 16:16 UTC (permalink / raw)
To: Viresh Kumar
Cc: xen-devel, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall
On Mon, Aug 22, 2022 at 02:45:14PM +0530, Viresh Kumar wrote:
> This patch adds basic support for configuring and assisting virtio-mmio
> based virtio-gpio backend (emualator) which is intended to run out of
> Qemu and could be run in any domain.
>
> An example of domain configuration for Virtio Gpio:
> gpio = [ "" ]
>
> Please note, this patch is not enough for virtio-gpio to work on Xen
> (Arm), as for every Virtio device we need to allocate Virtio MMIO params
> (IRQ and memory region) and pass them to the backend, also update Guest
> device-tree. A subsequent patch will add these missing bits. For the
> current patch, the default "irq" and "base" are just written to the
> Xenstore.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Most comment on the previous patch would apply to this one as well.
Cheers,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V5 3/6] libxl: arm: Create alloc_virtio_mmio_params()
2022-08-22 9:15 [PATCH V5 0/6] Virtio toolstack support for I2C and GPIO on Arm Viresh Kumar
2022-08-22 9:15 ` [PATCH V5 1/6] libxl: Add support for Virtio I2C device Viresh Kumar
2022-08-22 9:15 ` [PATCH V5 2/6] libxl: Add support for Virtio GPIO device Viresh Kumar
@ 2022-08-22 9:15 ` Viresh Kumar
2022-09-06 16:29 ` Anthony PERARD
2022-08-22 9:15 ` [PATCH V5 4/6] libxl: arm: Split make_virtio_mmio_node() Viresh Kumar
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Viresh Kumar @ 2022-08-22 9:15 UTC (permalink / raw)
To: xen-devel
Cc: Viresh Kumar, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall,
anthony.perard, Oleksandr Tyshchenko
Create a separate routine to allocate base and irq for a device as the
same code will be required for each device type.
Suggested-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
tools/libs/light/libxl_arm.c | 46 +++++++++++++++++++++++++-----------
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 1a3ac1646e94..0e3051bac5e4 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -48,6 +48,24 @@ static uint32_t alloc_virtio_mmio_irq(libxl__gc *gc, uint32_t *virtio_mmio_irq)
return irq;
}
+static int alloc_virtio_mmio_params(libxl__gc *gc, uint64_t *base,
+ uint32_t *irq, uint64_t *virtio_mmio_base,
+ uint32_t *virtio_mmio_irq)
+{
+ *base = alloc_virtio_mmio_base(gc, virtio_mmio_base);
+ if (!*base)
+ return ERROR_FAIL;
+
+ *irq = alloc_virtio_mmio_irq(gc, virtio_mmio_irq);
+ if (!*irq)
+ return ERROR_FAIL;
+
+ LOG(DEBUG, "Allocate Virtio MMIO params: IRQ %u BASE 0x%"PRIx64, *irq,
+ *base);
+
+ return 0;
+}
+
static const char *gicv_to_string(libxl_gic_version gic_version)
{
switch (gic_version) {
@@ -85,20 +103,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
libxl_device_disk *disk = &d_config->disks[i];
if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) {
- disk->base = alloc_virtio_mmio_base(gc, &virtio_mmio_base);
- if (!disk->base)
- return ERROR_FAIL;
-
- disk->irq = alloc_virtio_mmio_irq(gc, &virtio_mmio_irq);
- if (!disk->irq)
- return ERROR_FAIL;
+ int rc = alloc_virtio_mmio_params(gc, &disk->base, &disk->irq,
+ &virtio_mmio_base,
+ &virtio_mmio_irq);
- if (virtio_irq < disk->irq)
- virtio_irq = disk->irq;
- virtio_enabled = true;
-
- LOG(DEBUG, "Allocate Virtio MMIO params for Vdev %s: IRQ %u BASE 0x%"PRIx64,
- disk->vdev, disk->irq, disk->base);
+ if (rc)
+ return rc;
}
}
@@ -107,8 +117,16 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
* present, make sure that we allocate enough SPIs for them.
* The resulting "nr_spis" needs to cover the highest possible SPI.
*/
- if (virtio_enabled)
+ if (virtio_mmio_irq != GUEST_VIRTIO_MMIO_SPI_FIRST) {
+ virtio_enabled = true;
+
+ /*
+ * Assumes that "virtio_mmio_irq" is the highest allocated irq, which is
+ * updated from alloc_virtio_mmio_irq() currently.
+ */
+ virtio_irq = virtio_mmio_irq - 1;
nr_spis = max(nr_spis, virtio_irq - 32 + 1);
+ }
for (i = 0; i < d_config->b_info.num_irqs; i++) {
uint32_t irq = d_config->b_info.irqs[i];
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V5 3/6] libxl: arm: Create alloc_virtio_mmio_params()
2022-08-22 9:15 ` [PATCH V5 3/6] libxl: arm: Create alloc_virtio_mmio_params() Viresh Kumar
@ 2022-09-06 16:29 ` Anthony PERARD
0 siblings, 0 replies; 20+ messages in thread
From: Anthony PERARD @ 2022-09-06 16:29 UTC (permalink / raw)
To: Viresh Kumar
Cc: xen-devel, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall,
Oleksandr Tyshchenko
On Mon, Aug 22, 2022 at 02:45:15PM +0530, Viresh Kumar wrote:
> Create a separate routine to allocate base and irq for a device as the
> same code will be required for each device type.
>
> Suggested-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> tools/libs/light/libxl_arm.c | 46 +++++++++++++++++++++++++-----------
> 1 file changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 1a3ac1646e94..0e3051bac5e4 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -85,20 +103,12 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> libxl_device_disk *disk = &d_config->disks[i];
>
> if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) {
> - disk->base = alloc_virtio_mmio_base(gc, &virtio_mmio_base);
> - if (!disk->base)
> - return ERROR_FAIL;
> -
> - disk->irq = alloc_virtio_mmio_irq(gc, &virtio_mmio_irq);
> - if (!disk->irq)
> - return ERROR_FAIL;
> + int rc = alloc_virtio_mmio_params(gc, &disk->base, &disk->irq,
> + &virtio_mmio_base,
> + &virtio_mmio_irq);
>
> - if (virtio_irq < disk->irq)
> - virtio_irq = disk->irq;
> - virtio_enabled = true;
This change to virtio_irq and virtio_enabled, as well as the chunck
below, isn't explained in the patch description.
> -
> - LOG(DEBUG, "Allocate Virtio MMIO params for Vdev %s: IRQ %u BASE 0x%"PRIx64,
> - disk->vdev, disk->irq, disk->base);
> + if (rc)
> + return rc;
> }
> }
>
> @@ -107,8 +117,16 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> * present, make sure that we allocate enough SPIs for them.
> * The resulting "nr_spis" needs to cover the highest possible SPI.
> */
> - if (virtio_enabled)
> + if (virtio_mmio_irq != GUEST_VIRTIO_MMIO_SPI_FIRST) {
> + virtio_enabled = true;
> +
> + /*
> + * Assumes that "virtio_mmio_irq" is the highest allocated irq, which is
> + * updated from alloc_virtio_mmio_irq() currently.
> + */
> + virtio_irq = virtio_mmio_irq - 1;
> nr_spis = max(nr_spis, virtio_irq - 32 + 1);
> + }
>
> for (i = 0; i < d_config->b_info.num_irqs; i++) {
> uint32_t irq = d_config->b_info.irqs[i];
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V5 4/6] libxl: arm: Split make_virtio_mmio_node()
2022-08-22 9:15 [PATCH V5 0/6] Virtio toolstack support for I2C and GPIO on Arm Viresh Kumar
` (2 preceding siblings ...)
2022-08-22 9:15 ` [PATCH V5 3/6] libxl: arm: Create alloc_virtio_mmio_params() Viresh Kumar
@ 2022-08-22 9:15 ` Viresh Kumar
2022-09-06 16:32 ` Anthony PERARD
2022-08-22 9:15 ` [PATCH V5 5/6] libxl: Allocate MMIO params for I2c device and update DT Viresh Kumar
2022-08-22 9:15 ` [PATCH V5 6/6] libxl: Allocate MMIO params for GPIO " Viresh Kumar
5 siblings, 1 reply; 20+ messages in thread
From: Viresh Kumar @ 2022-08-22 9:15 UTC (permalink / raw)
To: xen-devel
Cc: Viresh Kumar, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall,
anthony.perard, Oleksandr Tyshchenko
make_virtio_mmio_node() creates the DT node for simple MMIO devices
currently, i.e. the ones that don't require any additional properties.
In order to allow using it for other complex device types, split the
functionality into two, one where the fdt node isn't closed and the
other one to create a simple DT node.
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
tools/libs/light/libxl_arm.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 0e3051bac5e4..4d0469f8166a 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -906,9 +906,9 @@ static int make_xen_iommu_node(libxl__gc *gc, void *fdt)
return 0;
}
-static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
- uint64_t base, uint32_t irq,
- uint32_t backend_domid)
+/* The caller is responsible to complete / close the fdt node */
+static int make_virtio_mmio_node_common(libxl__gc *gc, void *fdt, uint64_t base,
+ uint32_t irq, uint32_t backend_domid)
{
int res;
gic_interrupt intr;
@@ -941,10 +941,18 @@ static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
if (res) return res;
}
- res = fdt_end_node(fdt);
+ return res;
+}
+
+static int make_virtio_mmio_node_simple(libxl__gc *gc, void *fdt, uint64_t base,
+ uint32_t irq, uint32_t backend_domid)
+{
+ int res;
+
+ res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid);
if (res) return res;
- return 0;
+ return fdt_end_node(fdt);
}
static const struct arch_info *get_arch_info(libxl__gc *gc,
@@ -1267,8 +1275,9 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
iommu_created = true;
}
- FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq,
- disk->backend_domid) );
+ FDT( make_virtio_mmio_node_simple(gc, fdt, disk->base,
+ disk->irq,
+ disk->backend_domid) );
}
}
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V5 4/6] libxl: arm: Split make_virtio_mmio_node()
2022-08-22 9:15 ` [PATCH V5 4/6] libxl: arm: Split make_virtio_mmio_node() Viresh Kumar
@ 2022-09-06 16:32 ` Anthony PERARD
0 siblings, 0 replies; 20+ messages in thread
From: Anthony PERARD @ 2022-09-06 16:32 UTC (permalink / raw)
To: Viresh Kumar
Cc: xen-devel, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall,
Oleksandr Tyshchenko
On Mon, Aug 22, 2022 at 02:45:16PM +0530, Viresh Kumar wrote:
> make_virtio_mmio_node() creates the DT node for simple MMIO devices
> currently, i.e. the ones that don't require any additional properties.
>
> In order to allow using it for other complex device types, split the
> functionality into two, one where the fdt node isn't closed and the
> other one to create a simple DT node.
>
> Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 0e3051bac5e4..4d0469f8166a 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -1267,8 +1275,9 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
> iommu_created = true;
> }
>
> - FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq,
> - disk->backend_domid) );
> + FDT( make_virtio_mmio_node_simple(gc, fdt, disk->base,
Renaming that function to add the suffix "_simple" doesn't meaning full,
could you keep the same name?
> + disk->irq,
> + disk->backend_domid) );
> }
> }
>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V5 5/6] libxl: Allocate MMIO params for I2c device and update DT
2022-08-22 9:15 [PATCH V5 0/6] Virtio toolstack support for I2C and GPIO on Arm Viresh Kumar
` (3 preceding siblings ...)
2022-08-22 9:15 ` [PATCH V5 4/6] libxl: arm: Split make_virtio_mmio_node() Viresh Kumar
@ 2022-08-22 9:15 ` Viresh Kumar
2022-09-06 16:38 ` Anthony PERARD
2022-08-22 9:15 ` [PATCH V5 6/6] libxl: Allocate MMIO params for GPIO " Viresh Kumar
5 siblings, 1 reply; 20+ messages in thread
From: Viresh Kumar @ 2022-08-22 9:15 UTC (permalink / raw)
To: xen-devel
Cc: Viresh Kumar, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall,
anthony.perard, Oleksandr Tyshchenko
This patch allocates Virtio MMIO params (IRQ and memory region) and pass
them to the backend, also update Guest device-tree based on Virtio I2C
DT bindings [1].
This also relocates the code responsible to call make_xen_iommu_node()
towards the end of the routine, so we create the node only once.
[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
tools/libs/light/libxl_arm.c | 57 +++++++++++++++++++++++++++++++-----
1 file changed, 50 insertions(+), 7 deletions(-)
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 4d0469f8166a..4c1012e56893 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -112,6 +112,15 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
}
}
+ for (i = 0; i < d_config->num_i2cs; i++) {
+ libxl_device_i2c *i2c = &d_config->i2cs[i];
+ int rc = alloc_virtio_mmio_params(gc, &i2c->base, &i2c->irq,
+ &virtio_mmio_base, &virtio_mmio_irq);
+
+ if (rc)
+ return rc;
+ }
+
/*
* Every virtio-mmio device uses one emulated SPI. If Virtio devices are
* present, make sure that we allocate enough SPIs for them.
@@ -955,6 +964,26 @@ static int make_virtio_mmio_node_simple(libxl__gc *gc, void *fdt, uint64_t base,
return fdt_end_node(fdt);
}
+static int make_virtio_mmio_node_i2c(libxl__gc *gc, void *fdt, uint64_t base,
+ uint32_t irq, uint32_t backend_domid)
+{
+ int res;
+
+ res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid);
+ if (res) return res;
+
+ res = fdt_begin_node(fdt, "i2c");
+ if (res) return res;
+
+ res = fdt_property_compat(gc, fdt, 1, "virtio,device22");
+ if (res) return res;
+
+ res = fdt_end_node(fdt);
+ if (res) return res;
+
+ return fdt_end_node(fdt);
+}
+
static const struct arch_info *get_arch_info(libxl__gc *gc,
const struct xc_dom_image *dom)
{
@@ -1156,7 +1185,7 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
size_t fdt_size = 0;
int pfdt_size = 0;
libxl_domain_build_info *const info = &d_config->b_info;
- bool iommu_created;
+ bool iommu_needed;
unsigned int i;
const libxl_version_info *vers;
@@ -1264,16 +1293,13 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
if (d_config->num_pcidevs)
FDT( make_vpci_node(gc, fdt, ainfo, dom) );
- iommu_created = false;
+ iommu_needed = false;
for (i = 0; i < d_config->num_disks; i++) {
libxl_device_disk *disk = &d_config->disks[i];
if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) {
- if (disk->backend_domid != LIBXL_TOOLSTACK_DOMID &&
- !iommu_created) {
- FDT( make_xen_iommu_node(gc, fdt) );
- iommu_created = true;
- }
+ if (disk->backend_domid != LIBXL_TOOLSTACK_DOMID)
+ iommu_needed = true;
FDT( make_virtio_mmio_node_simple(gc, fdt, disk->base,
disk->irq,
@@ -1281,6 +1307,23 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
}
}
+ for (i = 0; i < d_config->num_i2cs; i++) {
+ libxl_device_i2c *i2c = &d_config->i2cs[i];
+
+ if (i2c->backend_domid != LIBXL_TOOLSTACK_DOMID)
+ iommu_needed = true;
+
+ FDT( make_virtio_mmio_node_i2c(gc, fdt, i2c->base, i2c->irq,
+ i2c->backend_domid) );
+ }
+
+ /*
+ * Note, this should be only called after creating all virtio-mmio
+ * device nodes
+ */
+ if (iommu_needed)
+ FDT( make_xen_iommu_node(gc, fdt) );
+
if (pfdt)
FDT( copy_partial_fdt(gc, fdt, pfdt) );
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V5 5/6] libxl: Allocate MMIO params for I2c device and update DT
2022-08-22 9:15 ` [PATCH V5 5/6] libxl: Allocate MMIO params for I2c device and update DT Viresh Kumar
@ 2022-09-06 16:38 ` Anthony PERARD
0 siblings, 0 replies; 20+ messages in thread
From: Anthony PERARD @ 2022-09-06 16:38 UTC (permalink / raw)
To: Viresh Kumar
Cc: xen-devel, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall,
Oleksandr Tyshchenko
On Mon, Aug 22, 2022 at 02:45:17PM +0530, Viresh Kumar wrote:
> This also relocates the code responsible to call make_xen_iommu_node()
> towards the end of the routine, so we create the node only once.
Could you do this part in a separate patch? Then, I believe the rest of
the patch could be squashed into the patch that adds i2c support into
libxl.
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH V5 6/6] libxl: Allocate MMIO params for GPIO device and update DT
2022-08-22 9:15 [PATCH V5 0/6] Virtio toolstack support for I2C and GPIO on Arm Viresh Kumar
` (4 preceding siblings ...)
2022-08-22 9:15 ` [PATCH V5 5/6] libxl: Allocate MMIO params for I2c device and update DT Viresh Kumar
@ 2022-08-22 9:15 ` Viresh Kumar
2022-09-06 16:43 ` Anthony PERARD
2022-09-07 18:02 ` Julien Grall
5 siblings, 2 replies; 20+ messages in thread
From: Viresh Kumar @ 2022-08-22 9:15 UTC (permalink / raw)
To: xen-devel
Cc: Viresh Kumar, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall,
anthony.perard, Oleksandr Tyshchenko
This patch allocates Virtio MMIO params (IRQ and memory region) and pass
them to the backend, also update Guest device-tree based on Virtio GPIO
DT bindings [1].
[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
tools/libs/light/libxl_arm.c | 51 ++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 4c1012e56893..86c1e560900f 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -121,6 +121,15 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
return rc;
}
+ for (i = 0; i < d_config->num_gpios; i++) {
+ libxl_device_gpio *gpio = &d_config->gpios[i];
+ int rc = alloc_virtio_mmio_params(gc, &gpio->base, &gpio->irq,
+ &virtio_mmio_base, &virtio_mmio_irq);
+
+ if (rc)
+ return rc;
+ }
+
/*
* Every virtio-mmio device uses one emulated SPI. If Virtio devices are
* present, make sure that we allocate enough SPIs for them.
@@ -984,6 +993,38 @@ static int make_virtio_mmio_node_i2c(libxl__gc *gc, void *fdt, uint64_t base,
return fdt_end_node(fdt);
}
+static int make_virtio_mmio_node_gpio(libxl__gc *gc, void *fdt, uint64_t base,
+ uint32_t irq, uint32_t backend_domid)
+{
+ int res;
+
+ res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid);
+ if (res) return res;
+
+ res = fdt_begin_node(fdt, "gpio");
+ if (res) return res;
+
+ res = fdt_property_compat(gc, fdt, 1, "virtio,device29");
+ if (res) return res;
+
+ res = fdt_property(fdt, "gpio-controller", NULL, 0);
+ if (res) return res;
+
+ res = fdt_property_cell(fdt, "#gpio-cells", 2);
+ if (res) return res;
+
+ res = fdt_property(fdt, "interrupt-controller", NULL, 0);
+ if (res) return res;
+
+ res = fdt_property_cell(fdt, "#interrupt-cells", 2);
+ if (res) return res;
+
+ res = fdt_end_node(fdt);
+ if (res) return res;
+
+ return fdt_end_node(fdt);
+}
+
static const struct arch_info *get_arch_info(libxl__gc *gc,
const struct xc_dom_image *dom)
{
@@ -1317,6 +1358,16 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
i2c->backend_domid) );
}
+ for (i = 0; i < d_config->num_gpios; i++) {
+ libxl_device_gpio *gpio = &d_config->gpios[i];
+
+ if (gpio->backend_domid != LIBXL_TOOLSTACK_DOMID)
+ iommu_needed = true;
+
+ FDT( make_virtio_mmio_node_gpio(gc, fdt, gpio->base, gpio->irq,
+ gpio->backend_domid) );
+ }
+
/*
* Note, this should be only called after creating all virtio-mmio
* device nodes
--
2.31.1.272.g89b43f80a514
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH V5 6/6] libxl: Allocate MMIO params for GPIO device and update DT
2022-08-22 9:15 ` [PATCH V5 6/6] libxl: Allocate MMIO params for GPIO " Viresh Kumar
@ 2022-09-06 16:43 ` Anthony PERARD
2022-09-07 18:02 ` Julien Grall
1 sibling, 0 replies; 20+ messages in thread
From: Anthony PERARD @ 2022-09-06 16:43 UTC (permalink / raw)
To: Viresh Kumar
Cc: xen-devel, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, Julien Grall,
Oleksandr Tyshchenko
On Mon, Aug 22, 2022 at 02:45:18PM +0530, Viresh Kumar wrote:
> This patch allocates Virtio MMIO params (IRQ and memory region) and pass
> them to the backend, also update Guest device-tree based on Virtio GPIO
> DT bindings [1].
>
> [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
>
> Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> tools/libs/light/libxl_arm.c | 51 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 4c1012e56893..86c1e560900f 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -121,6 +121,15 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> return rc;
> }
>
> + for (i = 0; i < d_config->num_gpios; i++) {
> + libxl_device_gpio *gpio = &d_config->gpios[i];
> + int rc = alloc_virtio_mmio_params(gc, &gpio->base, &gpio->irq,
Indentation seems wrong here.
Also, you could declare "rc" (without an initial value) for the whole
function rather than declaring it in each for loop scope.
Then, this patch could be squash into the one that adds GPIO support to
libxl.
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V5 6/6] libxl: Allocate MMIO params for GPIO device and update DT
2022-08-22 9:15 ` [PATCH V5 6/6] libxl: Allocate MMIO params for GPIO " Viresh Kumar
2022-09-06 16:43 ` Anthony PERARD
@ 2022-09-07 18:02 ` Julien Grall
2022-09-08 5:44 ` Viresh Kumar
1 sibling, 1 reply; 20+ messages in thread
From: Julien Grall @ 2022-09-07 18:02 UTC (permalink / raw)
To: Viresh Kumar, xen-devel
Cc: Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, anthony.perard,
Oleksandr Tyshchenko, Bertrand Marquis
(+ Bertrand)
Hi Viresh,
On 22/08/2022 10:15, Viresh Kumar wrote:
> This patch allocates Virtio MMIO params (IRQ and memory region) and pass
> them to the backend, also update Guest device-tree based on Virtio GPIO
> DT bindings [1].
>
> [1] https://www.kernel.org/doc/Documentation/devicetree/bindings/gpio/gpio-virtio.yaml
>
> Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> tools/libs/light/libxl_arm.c | 51 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index 4c1012e56893..86c1e560900f 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -121,6 +121,15 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
> return rc;
> }
>
> + for (i = 0; i < d_config->num_gpios; i++) {
> + libxl_device_gpio *gpio = &d_config->gpios[i];
> + int rc = alloc_virtio_mmio_params(gc, &gpio->base, &gpio->irq,
> + &virtio_mmio_base, &virtio_mmio_irq);
> +
> + if (rc)
> + return rc;
> + }
> +
> /*
> * Every virtio-mmio device uses one emulated SPI. If Virtio devices are
> * present, make sure that we allocate enough SPIs for them.
> @@ -984,6 +993,38 @@ static int make_virtio_mmio_node_i2c(libxl__gc *gc, void *fdt, uint64_t base,
> return fdt_end_node(fdt);
> }
>
> +static int make_virtio_mmio_node_gpio(libxl__gc *gc, void *fdt, uint64_t base,
> + uint32_t irq, uint32_t backend_domid)
> +{
> + int res;
> +
> + res = make_virtio_mmio_node_common(gc, fdt, base, irq, backend_domid);
> + if (res) return res;
> +
> + res = fdt_begin_node(fdt, "gpio");
> + if (res) return res;
> +
> + res = fdt_property_compat(gc, fdt, 1, "virtio,device29");
It is a very descriptive compatible :). And yes I realize this is the
compatible chosen by upstream.
> + if (res) return res;
> +
> + res = fdt_property(fdt, "gpio-controller", NULL, 0);
> + if (res) return res;
> +
> + res = fdt_property_cell(fdt, "#gpio-cells", 2);
> + if (res) return res;
> +
> + res = fdt_property(fdt, "interrupt-controller", NULL, 0);
> + if (res) return res;
> +
> + res = fdt_property_cell(fdt, "#interrupt-cells", 2);
> + if (res) return res;
> +
> + res = fdt_end_node(fdt);
> + if (res) return res;
> +
> + return fdt_end_node(fdt);
> +}
> +
> static const struct arch_info *get_arch_info(libxl__gc *gc,
> const struct xc_dom_image *dom)
> {
> @@ -1317,6 +1358,16 @@ static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_config *d_config,
> i2c->backend_domid) );
> }
>
> + for (i = 0; i < d_config->num_gpios; i++) {
> + libxl_device_gpio *gpio = &d_config->gpios[i];
> +
> + if (gpio->backend_domid != LIBXL_TOOLSTACK_DOMID)
> + iommu_needed = true;
> +
> + FDT( make_virtio_mmio_node_gpio(gc, fdt, gpio->base, gpio->irq,
> + gpio->backend_domid) );
So this is exposing a GPIO interrupt controller but IIUC we will also
need to describe the devices in the device-tree. Can you outline how you
would expect it to work?
To put some context, this is related to what I wrote in patch #1. How
much code will be necessary in libxl to add new device?
I am not the maintainer of this code, but I would expect some example
how this can be used once this merged. This would help us to understand
whether the interface is correct.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH V5 6/6] libxl: Allocate MMIO params for GPIO device and update DT
2022-09-07 18:02 ` Julien Grall
@ 2022-09-08 5:44 ` Viresh Kumar
0 siblings, 0 replies; 20+ messages in thread
From: Viresh Kumar @ 2022-09-08 5:44 UTC (permalink / raw)
To: Julien Grall
Cc: xen-devel, Vincent Guittot, stratos-dev, Alex Bennée,
Stefano Stabellini, Mathieu Poirier, Mike Holmes,
Oleksandr Tyshchenko, Wei Liu, Juergen Gross, anthony.perard,
Oleksandr Tyshchenko, Bertrand Marquis
On 07-09-22, 19:02, Julien Grall wrote:
> It is a very descriptive compatible :). And yes I realize this is the
> compatible chosen by upstream.
:)
> So this is exposing a GPIO interrupt controller but IIUC we will also need
> to describe the devices in the device-tree. Can you outline how you would
> expect it to work?
The same is true for busses like I2C and SPI, which may have client devices in
the DT as well.
Though I don't use them currently in my setup where I need to test the backends.
I am able to test both I2C and GPIO dynamically from the guest, without any
client devices in the DT.
> To put some context, this is related to what I wrote in patch #1. How much
> code will be necessary in libxl to add new device?
>
> I am not the maintainer of this code, but I would expect some example how
> this can be used once this merged. This would help us to understand whether
> the interface is correct.
I understand what you are looking for and that we may be required to pass the
relation of a client device with the GPIO controller via the "virtio" entry you
suggested for a common device.
I don't have an answer for that at the moment.
--
viresh
^ permalink raw reply [flat|nested] 20+ messages in thread