On 2/7/2023 5:15 AM, Srinivas Kandagatla wrote:
On 20/01/2023 22:46, Elliot Berman wrote:
Introduce a framework for Gunyah userspace to install VM functions. VM
functions are optional interfaces to the virtual machine. vCPUs,
ioeventfs, and irqfds are examples of such VM functions and are
implemented in subsequent patches.
A generic framework is implemented instead of individual ioctls to
create vCPUs, irqfds, etc., in order to simplify the VM manager core
implementation and allow dynamic loading of VM function modules.
Signed-off-by: Elliot Berman <quic_eberman@xxxxxxxxxxx>
---
[snip]
+#define DECLARE_GUNYAH_VM_FUNCTION(_name, _bind, _release) \
+ static struct gunyah_vm_function_driver _name = { \
+ .name = __stringify(_name), \
+ .mod = THIS_MODULE, \
+ .bind = _bind, \
+ .release = _release, \
+ }; \
+ MODULE_ALIAS("ghfunc:"__stringify(_name))
lets not over kill this by having DECLARE_GUNYAH_VM_FUNCTION, this will
make the drivers readable in a more familar way. let the driver define
this static struct.
+
+#define DECLARE_GUNYAH_VM_FUNCTION_INIT(_name, _bind, _release) \
+ DECLARE_GUNYAH_VM_FUNCTION(_name, _bind, _release); \
+ static int __init _name##_mod_init(void) \
+ { \
+ return gunyah_vm_function_register(&(_name)); \
+ } \
+ module_init(_name##_mod_init); \
+ static void __exit _name##_mod_exit(void) \
+ { \
+ gunyah_vm_function_unregister(&(_name)); \
+ } \
+ module_exit(_name##_mod_exit)
+
How about:
#define module_gunyah_function_driver(__gf_driver)
module_driver(__gf_driver, gunyah_vm_function_register, \
gunyah_vm_function_unregister)
Having relook at the patch, I think modeling the gunyah_vm_function as a
proper device and driver model will scale, you could leverage most of
this manual management to the existing driver model. May I suggest to
you take a look at include/linux/auxiliary_bus.h
with that you could model add_functions as
auxiliary_device_add and the respecitive drivers as
module_auxiliary_driver.
I'm not sure if device model can fit well here. I wanted to make sure
that the VM function actually bound to a driver when user requests it
and the driver to be able to return some info about it to the user --
vCPUs return a file descriptor for instance. I could probably make it
work with a device/driver model, but I'm not sure if it should be done
like that.
+#endif
diff --git a/include/uapi/linux/gunyah.h b/include/uapi/linux/gunyah.h
index 36359ad2175e..ec8da6fde045 100644
--- a/include/uapi/linux/gunyah.h
+++ b/include/uapi/linux/gunyah.h
@@ -50,4 +50,17 @@ struct gh_vm_dtb_config {
#define GH_VM_START _IO(GH_IOCTL_TYPE, 0x3)
+#define GUNYAH_FUNCTION_NAME_SIZE 32
+#define GUNYAH_FUNCTION_MAX_ARG_SIZE 1024
+
+struct gh_vm_function {
+ char name[GUNYAH_FUNCTION_NAME_SIZE];
+ union {
+ char data[GUNYAH_FUNCTION_MAX_ARG_SIZE];
Are we missing any thing here, its odd to see a single member union like
this.
if other memembers are part of another patch please move them to this
one as its confusing.
I can add a comment that members will be added as new functions are
added. If I put it in this patch, it raises questions about where those
other members are being used.
