Messages in this thread Patch in this message | | | Date | Fri, 31 Mar 2023 09:26:46 -0500 | From | Alex Elder <> | Subject | Re: [PATCH v11 15/26] gunyah: rsc_mgr: Add platform ops on mem_lend/mem_reclaim |
| |
On 3/3/23 7:06 PM, Elliot Berman wrote: > On Qualcomm platforms, there is a firmware entity which controls access > to physical pages. In order to share memory with another VM, this entity > needs to be informed that the guest VM should have access to the memory.
Will Gunyah ever be used on something other than a Qualcomm platform?
Is there really any need to have these "platform hooks" conditionally compiled?
One other comment below.
-Alex
> Co-developed-by: Prakruthi Deepak Heragu <quic_pheragu@quicinc.com> > Signed-off-by: Prakruthi Deepak Heragu <quic_pheragu@quicinc.com> > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > --- > drivers/virt/gunyah/Kconfig | 4 ++ > drivers/virt/gunyah/Makefile | 1 + > drivers/virt/gunyah/gunyah_platform_hooks.c | 80 +++++++++++++++++++++ > drivers/virt/gunyah/rsc_mgr.h | 3 + > drivers/virt/gunyah/rsc_mgr_rpc.c | 18 ++++- > include/linux/gunyah_rsc_mgr.h | 17 +++++ > 6 files changed, 121 insertions(+), 2 deletions(-) > create mode 100644 drivers/virt/gunyah/gunyah_platform_hooks.c > > diff --git a/drivers/virt/gunyah/Kconfig b/drivers/virt/gunyah/Kconfig > index 1a737694c333..de815189dab6 100644 > --- a/drivers/virt/gunyah/Kconfig > +++ b/drivers/virt/gunyah/Kconfig > @@ -4,6 +4,7 @@ config GUNYAH > tristate "Gunyah Virtualization drivers" > depends on ARM64 > depends on MAILBOX > + select GUNYAH_PLATFORM_HOOKS > help > The Gunyah drivers are the helper interfaces that run in a guest VM > such as basic inter-VM IPC and signaling mechanisms, and higher level > @@ -11,3 +12,6 @@ config GUNYAH > > Say Y/M here to enable the drivers needed to interact in a Gunyah > virtual environment. > + > +config GUNYAH_PLATFORM_HOOKS > + tristate > diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile > index ff8bc4925392..6b8f84dbfe0d 100644 > --- a/drivers/virt/gunyah/Makefile > +++ b/drivers/virt/gunyah/Makefile > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0 > > obj-$(CONFIG_GUNYAH) += gunyah.o > +obj-$(CONFIG_GUNYAH_PLATFORM_HOOKS) += gunyah_platform_hooks.o > > gunyah_rsc_mgr-y += rsc_mgr.o rsc_mgr_rpc.o vm_mgr.o vm_mgr_mm.o > obj-$(CONFIG_GUNYAH) += gunyah_rsc_mgr.o > diff --git a/drivers/virt/gunyah/gunyah_platform_hooks.c b/drivers/virt/gunyah/gunyah_platform_hooks.c > new file mode 100644 > index 000000000000..60da0e154e98 > --- /dev/null > +++ b/drivers/virt/gunyah/gunyah_platform_hooks.c > @@ -0,0 +1,80 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/module.h> > +#include <linux/rwsem.h> > +#include <linux/gunyah_rsc_mgr.h> > + > +#include "rsc_mgr.h" > + > +static struct gh_rm_platform_ops *rm_platform_ops; > +static DECLARE_RWSEM(rm_platform_ops_lock); > + > +int gh_rm_platform_pre_mem_share(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel) > +{ > + int ret = 0; > + > + down_read(&rm_platform_ops_lock); > + if (rm_platform_ops && rm_platform_ops->pre_mem_share) > + ret = rm_platform_ops->pre_mem_share(rm, mem_parcel); > + up_read(&rm_platform_ops_lock); > + return ret; > +} > +EXPORT_SYMBOL_GPL(gh_rm_platform_pre_mem_share); > + > +int gh_rm_platform_post_mem_reclaim(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel) > +{ > + int ret = 0; > + > + down_read(&rm_platform_ops_lock); > + if (rm_platform_ops && rm_platform_ops->post_mem_reclaim) > + ret = rm_platform_ops->post_mem_reclaim(rm, mem_parcel); > + up_read(&rm_platform_ops_lock); > + return ret; > +} > +EXPORT_SYMBOL_GPL(gh_rm_platform_post_mem_reclaim); > + > +int gh_rm_register_platform_ops(struct gh_rm_platform_ops *platform_ops) > +{ > + int ret = 0; > + > + down_write(&rm_platform_ops_lock); > + if (!rm_platform_ops) > + rm_platform_ops = platform_ops; > + else > + ret = -EEXIST; > + up_write(&rm_platform_ops_lock); > + return ret; > +} > +EXPORT_SYMBOL_GPL(gh_rm_register_platform_ops); > + > +void gh_rm_unregister_platform_ops(struct gh_rm_platform_ops *platform_ops) > +{ > + down_write(&rm_platform_ops_lock); > + if (rm_platform_ops == platform_ops) > + rm_platform_ops = NULL; > + up_write(&rm_platform_ops_lock); > +} > +EXPORT_SYMBOL_GPL(gh_rm_unregister_platform_ops); > + > +static void _devm_gh_rm_unregister_platform_ops(void *data) > +{ > + gh_rm_unregister_platform_ops(data); > +} > + > +int devm_gh_rm_register_platform_ops(struct device *dev, struct gh_rm_platform_ops *ops) > +{ > + int ret; > + > + ret = gh_rm_register_platform_ops(ops); > + if (ret) > + return ret; > + > + return devm_add_action(dev, _devm_gh_rm_unregister_platform_ops, ops); > +} > +EXPORT_SYMBOL_GPL(devm_gh_rm_register_platform_ops); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("Gunyah Platform Hooks"); > diff --git a/drivers/virt/gunyah/rsc_mgr.h b/drivers/virt/gunyah/rsc_mgr.h > index 3665ebc7b020..6838e736f361 100644 > --- a/drivers/virt/gunyah/rsc_mgr.h > +++ b/drivers/virt/gunyah/rsc_mgr.h > @@ -13,4 +13,7 @@ struct gh_rm; > int gh_rm_call(struct gh_rm *rsc_mgr, u32 message_id, void *req_buff, size_t req_buf_size, > void **resp_buf, size_t *resp_buf_size); > > +int gh_rm_platform_pre_mem_share(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel); > +int gh_rm_platform_post_mem_reclaim(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel); > + > #endif > diff --git a/drivers/virt/gunyah/rsc_mgr_rpc.c b/drivers/virt/gunyah/rsc_mgr_rpc.c > index 3df15ad5b97d..733be4dc8dd2 100644 > --- a/drivers/virt/gunyah/rsc_mgr_rpc.c > +++ b/drivers/virt/gunyah/rsc_mgr_rpc.c > @@ -204,6 +204,12 @@ static int gh_rm_mem_lend_common(struct gh_rm *rm, u32 message_id, struct gh_rm_ > if (!msg) > return -ENOMEM; > > + ret = gh_rm_platform_pre_mem_share(rm, p); > + if (ret) { > + kfree(msg); > + return ret; > + } > + > req_header = msg; > acl_section = (void *)req_header + sizeof(*req_header); > mem_section = (void *)acl_section + struct_size(acl_section, entries, p->n_acl_entries); > @@ -227,8 +233,10 @@ static int gh_rm_mem_lend_common(struct gh_rm *rm, u32 message_id, struct gh_rm_ > ret = gh_rm_call(rm, message_id, msg, msg_size, (void **)&resp, &resp_size); > kfree(msg); > > - if (ret) > + if (ret) { > + gh_rm_platform_post_mem_reclaim(rm, p); > return ret; > + } > > p->mem_handle = le32_to_cpu(*resp); > > @@ -283,8 +291,14 @@ int gh_rm_mem_reclaim(struct gh_rm *rm, struct gh_rm_mem_parcel *parcel) > struct gh_rm_mem_release_req req = { > .mem_handle = cpu_to_le32(parcel->mem_handle), > }; > + int ret; > + > + ret = gh_rm_call(rm, GH_RM_RPC_MEM_RECLAIM, &req, sizeof(req), NULL, NULL); > + /* Do not call platform mem reclaim hooks: the reclaim didn't happen*/
Move the comment above the gh_rm_call() call, and rephrase, such as:
/* Only call platform mem reclaim hooks if... */
> + if (ret) > + return ret; > > - return gh_rm_call(rm, GH_RM_RPC_MEM_RECLAIM, &req, sizeof(req), NULL, NULL); > + return gh_rm_platform_post_mem_reclaim(rm, parcel); > } > > /** > diff --git a/include/linux/gunyah_rsc_mgr.h b/include/linux/gunyah_rsc_mgr.h > index 8b0b46f28e39..515087931a2b 100644 > --- a/include/linux/gunyah_rsc_mgr.h > +++ b/include/linux/gunyah_rsc_mgr.h > @@ -145,4 +145,21 @@ int gh_rm_get_hyp_resources(struct gh_rm *rm, u16 vmid, > struct gh_rm_hyp_resources **resources); > int gh_rm_get_vmid(struct gh_rm *rm, u16 *vmid); > > +struct gunyah_rm_platform_ops { > + int (*pre_mem_share)(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel); > + int (*post_mem_reclaim)(struct gh_rm *rm, struct gh_rm_mem_parcel *mem_parcel); > +}; > + > +#if IS_ENABLED(CONFIG_GUNYAH_PLATFORM_HOOKS) > +int gh_rm_register_platform_ops(struct gh_rm_platform_ops *platform_ops); > +void gh_rm_unregister_platform_ops(struct gh_rm_platform_ops *platform_ops); > +int devm_gh_rm_register_platform_ops(struct device *dev, struct gh_rm_platform_ops *ops); > +#else > +static inline int gh_rm_register_platform_ops(struct gh_rm_platform_ops *platform_ops) > + { return 0; } > +static inline void gh_rm_unregister_platform_ops(struct gh_rm_platform_ops *platform_ops) { } > +static inline int devm_gh_rm_register_platform_ops(struct device *dev, > + struct gh_rm_platform_ops *ops) { return 0; } > +#endif > + > #endif
| |