| Message ID | 20230214212343.3311875-1-quic_eberman@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Drivers for Gunyah hypervisor | expand |
On Tue, Feb 14, 2023 at 01:23:42PM -0800, Elliot Berman wrote: > > Add Gunyah Resource Manager RPC to launch an unauthenticated VM. > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > --- > drivers/virt/gunyah/Makefile | 2 +- > drivers/virt/gunyah/rsc_mgr.h | 45 ++++++ > drivers/virt/gunyah/rsc_mgr_rpc.c | 226 ++++++++++++++++++++++++++++++ > include/linux/gunyah_rsc_mgr.h | 73 ++++++++++ > 4 files changed, 345 insertions(+), 1 deletion(-) > create mode 100644 drivers/virt/gunyah/rsc_mgr_rpc.c > > diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile > index cc864ff5abbb..de29769f2f3f 100644 > --- a/drivers/virt/gunyah/Makefile > +++ b/drivers/virt/gunyah/Makefile > @@ -2,5 +2,5 @@ > > obj-$(CONFIG_GUNYAH) += gunyah.o > > -gunyah_rsc_mgr-y += rsc_mgr.o > +gunyah_rsc_mgr-y += rsc_mgr.o rsc_mgr_rpc.o > obj-$(CONFIG_GUNYAH) += gunyah_rsc_mgr.o > diff --git a/drivers/virt/gunyah/rsc_mgr.h b/drivers/virt/gunyah/rsc_mgr.h > index d4e799a7526f..7406237bc66d 100644 > --- a/drivers/virt/gunyah/rsc_mgr.h > +++ b/drivers/virt/gunyah/rsc_mgr.h > @@ -74,4 +74,49 @@ struct gh_rm; > int gh_rm_call(struct gh_rm *rsc_mgr, u32 message_id, void *req_buff, size_t req_buff_size, > void **resp_buf, size_t *resp_buff_size); > > +/* Message IDs: VM Management */ > +#define GH_RM_RPC_VM_ALLOC_VMID 0x56000001 > +#define GH_RM_RPC_VM_DEALLOC_VMID 0x56000002 > +#define GH_RM_RPC_VM_START 0x56000004 > +#define GH_RM_RPC_VM_STOP 0x56000005 > +#define GH_RM_RPC_VM_RESET 0x56000006 > +#define GH_RM_RPC_VM_CONFIG_IMAGE 0x56000009 > +#define GH_RM_RPC_VM_INIT 0x5600000B > +#define GH_RM_RPC_VM_GET_HYP_RESOURCES 0x56000020 > +#define GH_RM_RPC_VM_GET_VMID 0x56000024 > + > +struct gh_rm_vm_common_vmid_req { > + __le16 vmid; > + __le16 reserved0; reserved for what? What is a valid value for this field? Should it be checked for 0? Same with other "reserved0" fields in this file. > +} __packed; > + > +/* Call: VM_ALLOC */ > +struct gh_rm_vm_alloc_vmid_resp { > + __le16 vmid; > + __le16 reserved0; > +} __packed; > + > +/* Call: VM_STOP */ > +struct gh_rm_vm_stop_req { > + __le16 vmid; > +#define GH_RM_VM_STOP_FLAG_FORCE_STOP BIT(0) > + u8 flags; > + u8 reserved; Why just "reserved" and not "reserved0"? Naming is hard :( thanks, greg k-h
On 2/15/2023 10:39 PM, Greg Kroah-Hartman wrote: > On Tue, Feb 14, 2023 at 01:23:42PM -0800, Elliot Berman wrote: >> >> Add Gunyah Resource Manager RPC to launch an unauthenticated VM. >> >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >> --- >> drivers/virt/gunyah/Makefile | 2 +- >> drivers/virt/gunyah/rsc_mgr.h | 45 ++++++ >> drivers/virt/gunyah/rsc_mgr_rpc.c | 226 ++++++++++++++++++++++++++++++ >> include/linux/gunyah_rsc_mgr.h | 73 ++++++++++ >> 4 files changed, 345 insertions(+), 1 deletion(-) >> create mode 100644 drivers/virt/gunyah/rsc_mgr_rpc.c >> >> diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile >> index cc864ff5abbb..de29769f2f3f 100644 >> --- a/drivers/virt/gunyah/Makefile >> +++ b/drivers/virt/gunyah/Makefile >> @@ -2,5 +2,5 @@ >> >> obj-$(CONFIG_GUNYAH) += gunyah.o >> >> -gunyah_rsc_mgr-y += rsc_mgr.o >> +gunyah_rsc_mgr-y += rsc_mgr.o rsc_mgr_rpc.o >> obj-$(CONFIG_GUNYAH) += gunyah_rsc_mgr.o >> diff --git a/drivers/virt/gunyah/rsc_mgr.h b/drivers/virt/gunyah/rsc_mgr.h >> index d4e799a7526f..7406237bc66d 100644 >> --- a/drivers/virt/gunyah/rsc_mgr.h >> +++ b/drivers/virt/gunyah/rsc_mgr.h >> @@ -74,4 +74,49 @@ struct gh_rm; >> int gh_rm_call(struct gh_rm *rsc_mgr, u32 message_id, void *req_buff, size_t req_buff_size, >> void **resp_buf, size_t *resp_buff_size); >> >> +/* Message IDs: VM Management */ >> +#define GH_RM_RPC_VM_ALLOC_VMID 0x56000001 >> +#define GH_RM_RPC_VM_DEALLOC_VMID 0x56000002 >> +#define GH_RM_RPC_VM_START 0x56000004 >> +#define GH_RM_RPC_VM_STOP 0x56000005 >> +#define GH_RM_RPC_VM_RESET 0x56000006 >> +#define GH_RM_RPC_VM_CONFIG_IMAGE 0x56000009 >> +#define GH_RM_RPC_VM_INIT 0x5600000B >> +#define GH_RM_RPC_VM_GET_HYP_RESOURCES 0x56000020 >> +#define GH_RM_RPC_VM_GET_VMID 0x56000024 >> + >> +struct gh_rm_vm_common_vmid_req { >> + __le16 vmid; >> + __le16 reserved0; > > reserved for what? What is a valid value for this field? Should it be > checked for 0? This struct is transmitted "over the wire" and RM makes all of its structures 4-byte aligned. The reserved fields are padding for this alignment and will be zero but don't need to be checked. Linux initializes the reserved fields to zero. > > Same with other "reserved0" fields in this file. > > >> +} __packed; >> + >> +/* Call: VM_ALLOC */ >> +struct gh_rm_vm_alloc_vmid_resp { >> + __le16 vmid; >> + __le16 reserved0; >> +} __packed; >> + >> +/* Call: VM_STOP */ >> +struct gh_rm_vm_stop_req { >> + __le16 vmid; >> +#define GH_RM_VM_STOP_FLAG_FORCE_STOP BIT(0) >> + u8 flags; >> + u8 reserved; > > Why just "reserved" and not "reserved0"? Naming is hard :( > Some fields have multiple reserved fields. I'll clean up so "reserved0" only appears when there are multiple padding fields. Thanks, Elliot
On 14/02/2023 21:23, Elliot Berman wrote: > > Add Gunyah Resource Manager RPC to launch an unauthenticated VM. > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > --- > drivers/virt/gunyah/Makefile | 2 +- > drivers/virt/gunyah/rsc_mgr.h | 45 ++++++ > drivers/virt/gunyah/rsc_mgr_rpc.c | 226 ++++++++++++++++++++++++++++++ > include/linux/gunyah_rsc_mgr.h | 73 ++++++++++ > 4 files changed, 345 insertions(+), 1 deletion(-) > create mode 100644 drivers/virt/gunyah/rsc_mgr_rpc.c > > diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile > index cc864ff5abbb..de29769f2f3f 100644 > --- a/drivers/virt/gunyah/Makefile > +++ b/drivers/virt/gunyah/Makefile > @@ -2,5 +2,5 @@ > > obj-$(CONFIG_GUNYAH) += gunyah.o > > -gunyah_rsc_mgr-y += rsc_mgr.o > +gunyah_rsc_mgr-y += rsc_mgr.o rsc_mgr_rpc.o > obj-$(CONFIG_GUNYAH) += gunyah_rsc_mgr.o > diff --git a/drivers/virt/gunyah/rsc_mgr.h b/drivers/virt/gunyah/rsc_mgr.h > index d4e799a7526f..7406237bc66d 100644 > --- a/drivers/virt/gunyah/rsc_mgr.h > +++ b/drivers/virt/gunyah/rsc_mgr.h > @@ -74,4 +74,49 @@ struct gh_rm; > int gh_rm_call(struct gh_rm *rsc_mgr, u32 message_id, void *req_buff, size_t req_buff_size, > void **resp_buf, size_t *resp_buff_size); > <---------------------------- > +/* Message IDs: VM Management */ > +#define GH_RM_RPC_VM_ALLOC_VMID 0x56000001 > +#define GH_RM_RPC_VM_DEALLOC_VMID 0x56000002 > +#define GH_RM_RPC_VM_START 0x56000004 > +#define GH_RM_RPC_VM_STOP 0x56000005 > +#define GH_RM_RPC_VM_RESET 0x56000006 > +#define GH_RM_RPC_VM_CONFIG_IMAGE 0x56000009 > +#define GH_RM_RPC_VM_INIT 0x5600000B > +#define GH_RM_RPC_VM_GET_HYP_RESOURCES 0x56000020 > +#define GH_RM_RPC_VM_GET_VMID 0x56000024 > + > +struct gh_rm_vm_common_vmid_req { > + __le16 vmid; > + __le16 reserved0; > +} __packed; > + > +/* Call: VM_ALLOC */ > +struct gh_rm_vm_alloc_vmid_resp { > + __le16 vmid; > + __le16 reserved0; > +} __packed; > + > +/* Call: VM_STOP */ > +struct gh_rm_vm_stop_req { > + __le16 vmid; > +#define GH_RM_VM_STOP_FLAG_FORCE_STOP BIT(0) > + u8 flags; > + u8 reserved; > +#define GH_RM_VM_STOP_REASON_FORCE_STOP 3 > + __le32 stop_reason; > +} __packed; > + > +/* Call: VM_CONFIG_IMAGE */ > +struct gh_rm_vm_config_image_req { > + __le16 vmid; > + __le16 auth_mech; > + __le32 mem_handle; > + __le64 image_offset; > + __le64 image_size; > + __le64 dtb_offset; > + __le64 dtb_size; > +} __packed; > + > +/* Call: GET_HYP_RESOURCES */ > + --------------------------------> All the above structures are very much internal to rsc_mgr_rpc.c and interface to the rsc_mgr_rpc is already abstracted with function arguments ex: int gh_rm_vm_configure(struct gh_rm *rm, u16 vmid, enum gh_rm_vm_auth_mechanism auth_mechanism, u32 mem_handle, u64 image_offset, u64 image_size, u64 dtb_offset, u64 dtb_size) So why do we need these structs and defines in header file at all? you should proabably consider moving them to the .c file. > #endif > diff --git a/drivers/virt/gunyah/rsc_mgr_rpc.c b/drivers/virt/gunyah/rsc_mgr_rpc.c > new file mode 100644 > index 000000000000..4515cdd80106 > --- /dev/null > +++ b/drivers/virt/gunyah/rsc_mgr_rpc.c > @@ -0,0 +1,226 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/gunyah_rsc_mgr.h> > + Why new line here? > +#include "rsc_mgr.h" > + > +/* ... > +int gh_rm_vm_configure(struct gh_rm *rm, u16 vmid, enum gh_rm_vm_auth_mechanism auth_mechanism, > + u32 mem_handle, u64 image_offset, u64 image_size, u64 dtb_offset, u64 dtb_size) > +{ > + struct gh_rm_vm_config_image_req req_payload = { 0 }; > + size_t resp_size; > + void *resp; > + > + req_payload.vmid = cpu_to_le16(vmid); > + req_payload.auth_mech = cpu_to_le16(auth_mechanism); > + req_payload.mem_handle = cpu_to_le32(mem_handle); > + req_payload.image_offset = cpu_to_le64(image_offset); > + req_payload.image_size = cpu_to_le64(image_size); > + req_payload.dtb_offset = cpu_to_le64(dtb_offset); > + req_payload.dtb_size = cpu_to_le64(dtb_size); > + > + return gh_rm_call(rm, GH_RM_RPC_VM_CONFIG_IMAGE, &req_payload, sizeof(req_payload), > + &resp, &resp_size); > +} > + --srini
On 2/14/23 3:23 PM, Elliot Berman wrote: > > Add Gunyah Resource Manager RPC to launch an unauthenticated VM. > > Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> > --- > drivers/virt/gunyah/Makefile | 2 +- > drivers/virt/gunyah/rsc_mgr.h | 45 ++++++ > drivers/virt/gunyah/rsc_mgr_rpc.c | 226 ++++++++++++++++++++++++++++++ > include/linux/gunyah_rsc_mgr.h | 73 ++++++++++ > 4 files changed, 345 insertions(+), 1 deletion(-) > create mode 100644 drivers/virt/gunyah/rsc_mgr_rpc.c > > diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile > index cc864ff5abbb..de29769f2f3f 100644 > --- a/drivers/virt/gunyah/Makefile > +++ b/drivers/virt/gunyah/Makefile > @@ -2,5 +2,5 @@ > > obj-$(CONFIG_GUNYAH) += gunyah.o > > -gunyah_rsc_mgr-y += rsc_mgr.o > +gunyah_rsc_mgr-y += rsc_mgr.o rsc_mgr_rpc.o > obj-$(CONFIG_GUNYAH) += gunyah_rsc_mgr.o > diff --git a/drivers/virt/gunyah/rsc_mgr.h b/drivers/virt/gunyah/rsc_mgr.h > index d4e799a7526f..7406237bc66d 100644 > --- a/drivers/virt/gunyah/rsc_mgr.h > +++ b/drivers/virt/gunyah/rsc_mgr.h > @@ -74,4 +74,49 @@ struct gh_rm; > int gh_rm_call(struct gh_rm *rsc_mgr, u32 message_id, void *req_buff, size_t req_buff_size, > void **resp_buf, size_t *resp_buff_size); > > +/* Message IDs: VM Management */ > +#define GH_RM_RPC_VM_ALLOC_VMID 0x56000001 > +#define GH_RM_RPC_VM_DEALLOC_VMID 0x56000002 > +#define GH_RM_RPC_VM_START 0x56000004 > +#define GH_RM_RPC_VM_STOP 0x56000005 > +#define GH_RM_RPC_VM_RESET 0x56000006 > +#define GH_RM_RPC_VM_CONFIG_IMAGE 0x56000009 > +#define GH_RM_RPC_VM_INIT 0x5600000B > +#define GH_RM_RPC_VM_GET_HYP_RESOURCES 0x56000020 > +#define GH_RM_RPC_VM_GET_VMID 0x56000024 > + > +struct gh_rm_vm_common_vmid_req { > + __le16 vmid; > + __le16 reserved0; > +} __packed; > + > +/* Call: VM_ALLOC */ > +struct gh_rm_vm_alloc_vmid_resp { > + __le16 vmid; > + __le16 reserved0; > +} __packed; > + > +/* Call: VM_STOP */ > +struct gh_rm_vm_stop_req { > + __le16 vmid; > +#define GH_RM_VM_STOP_FLAG_FORCE_STOP BIT(0) > + u8 flags; > + u8 reserved; > +#define GH_RM_VM_STOP_REASON_FORCE_STOP 3 I suggested this before and you honored it. Now I'll suggest it again, and ask you to do it throughout the driver. Please separate the definitions of constant values that certain fields can take on from the structure definition. I think doing it the way you have here makes it harder to understand the structure definition. You could define an anonymous enumerated type to hold the values meant to be held by each field. > + __le32 stop_reason; > +} __packed; > + > +/* Call: VM_CONFIG_IMAGE */ > +struct gh_rm_vm_config_image_req { > + __le16 vmid; > + __le16 auth_mech; > + __le32 mem_handle; > + __le64 image_offset; > + __le64 image_size; > + __le64 dtb_offset; > + __le64 dtb_size; > +} __packed; > + > +/* Call: GET_HYP_RESOURCES */ > + > #endif > diff --git a/drivers/virt/gunyah/rsc_mgr_rpc.c b/drivers/virt/gunyah/rsc_mgr_rpc.c > new file mode 100644 > index 000000000000..4515cdd80106 > --- /dev/null > +++ b/drivers/virt/gunyah/rsc_mgr_rpc.c > @@ -0,0 +1,226 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/gunyah_rsc_mgr.h> > + > +#include "rsc_mgr.h" > + > +/* > + * Several RM calls take only a VMID as a parameter and give only standard > + * response back. Deduplicate boilerplate code by using this common call. > + */ > +static int gh_rm_common_vmid_call(struct gh_rm *rm, u32 message_id, u16 vmid) > +{ > + struct gh_rm_vm_common_vmid_req req_payload = { > + .vmid = cpu_to_le16(vmid), > + }; > + size_t resp_size; > + void *resp; > + > + return gh_rm_call(rm, message_id, &req_payload, sizeof(req_payload), &resp, &resp_size); > +} > + > +/** > + * gh_rm_alloc_vmid() - Allocate a new VM in Gunyah. Returns the VM identifier. > + * @rm: Handle to a Gunyah resource manager > + * @vmid: Use GH_VMID_INVAL or 0 to dynamically allocate a VM. A reserved VMID can > + * be supplied to request allocation of a platform-defined VM. Honestly, I'd rather just see 0 (and *not* GH_VMID_INVAL) be the special value to mean "dynamically allocate the VMID." It seems 0 is a reserved VMID anyway, and GH_VMID_INVAL might as well be treated here as an invalid parameter. Is there any definitition of which VMIDs are reserved? Like, anything under 1024? That's it on this patch for now. -Alex > + * > + * Returns - the allocated VMID or negative value on error > + */ > +int gh_rm_alloc_vmid(struct gh_rm *rm, u16 vmid) > +{ > + struct gh_rm_vm_common_vmid_req req_payload = { 0 }; > + struct gh_rm_vm_alloc_vmid_resp *resp_payload; > + size_t resp_size; > + void *resp; > + int ret; . . .
On 2/23/2023 1:36 PM, Alex Elder wrote: > On 2/14/23 3:23 PM, Elliot Berman wrote: >> >> Add Gunyah Resource Manager RPC to launch an unauthenticated VM. >> >> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> >> --- >> drivers/virt/gunyah/Makefile | 2 +- >> drivers/virt/gunyah/rsc_mgr.h | 45 ++++++ >> drivers/virt/gunyah/rsc_mgr_rpc.c | 226 ++++++++++++++++++++++++++++++ >> include/linux/gunyah_rsc_mgr.h | 73 ++++++++++ >> 4 files changed, 345 insertions(+), 1 deletion(-) >> create mode 100644 drivers/virt/gunyah/rsc_mgr_rpc.c >> >> diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile >> index cc864ff5abbb..de29769f2f3f 100644 >> --- a/drivers/virt/gunyah/Makefile >> +++ b/drivers/virt/gunyah/Makefile >> @@ -2,5 +2,5 @@ >> obj-$(CONFIG_GUNYAH) += gunyah.o >> -gunyah_rsc_mgr-y += rsc_mgr.o >> +gunyah_rsc_mgr-y += rsc_mgr.o rsc_mgr_rpc.o >> obj-$(CONFIG_GUNYAH) += gunyah_rsc_mgr.o >> diff --git a/drivers/virt/gunyah/rsc_mgr.h >> b/drivers/virt/gunyah/rsc_mgr.h >> index d4e799a7526f..7406237bc66d 100644 >> --- a/drivers/virt/gunyah/rsc_mgr.h >> +++ b/drivers/virt/gunyah/rsc_mgr.h >> @@ -74,4 +74,49 @@ struct gh_rm; >> int gh_rm_call(struct gh_rm *rsc_mgr, u32 message_id, void >> *req_buff, size_t req_buff_size, >> void **resp_buf, size_t *resp_buff_size); >> +/* Message IDs: VM Management */ >> +#define GH_RM_RPC_VM_ALLOC_VMID 0x56000001 >> +#define GH_RM_RPC_VM_DEALLOC_VMID 0x56000002 >> +#define GH_RM_RPC_VM_START 0x56000004 >> +#define GH_RM_RPC_VM_STOP 0x56000005 >> +#define GH_RM_RPC_VM_RESET 0x56000006 >> +#define GH_RM_RPC_VM_CONFIG_IMAGE 0x56000009 >> +#define GH_RM_RPC_VM_INIT 0x5600000B >> +#define GH_RM_RPC_VM_GET_HYP_RESOURCES 0x56000020 >> +#define GH_RM_RPC_VM_GET_VMID 0x56000024 >> + >> +struct gh_rm_vm_common_vmid_req { >> + __le16 vmid; >> + __le16 reserved0; >> +} __packed; >> + >> +/* Call: VM_ALLOC */ >> +struct gh_rm_vm_alloc_vmid_resp { >> + __le16 vmid; >> + __le16 reserved0; >> +} __packed; >> + >> +/* Call: VM_STOP */ >> +struct gh_rm_vm_stop_req { >> + __le16 vmid; >> +#define GH_RM_VM_STOP_FLAG_FORCE_STOP BIT(0) >> + u8 flags; >> + u8 reserved; >> +#define GH_RM_VM_STOP_REASON_FORCE_STOP 3 > > I suggested this before and you honored it. Now I'll suggest > it again, and ask you to do it throughout the driver. > > Please separate the definitions of constant values that > certain fields can take on from the structure definition. > I think doing it the way you have here makes it harder to > understand the structure definition. > > You could define an anonymous enumerated type to hold > the values meant to be held by each field. > Done. >> + __le32 stop_reason; >> +} __packed; >> + >> +/* Call: VM_CONFIG_IMAGE */ >> +struct gh_rm_vm_config_image_req { >> + __le16 vmid; >> + __le16 auth_mech; >> + __le32 mem_handle; >> + __le64 image_offset; >> + __le64 image_size; >> + __le64 dtb_offset; >> + __le64 dtb_size; >> +} __packed; >> + >> +/* Call: GET_HYP_RESOURCES */ >> + >> #endif >> diff --git a/drivers/virt/gunyah/rsc_mgr_rpc.c >> b/drivers/virt/gunyah/rsc_mgr_rpc.c >> new file mode 100644 >> index 000000000000..4515cdd80106 >> --- /dev/null >> +++ b/drivers/virt/gunyah/rsc_mgr_rpc.c >> @@ -0,0 +1,226 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All >> rights reserved. >> + */ >> + >> +#include <linux/gunyah_rsc_mgr.h> >> + >> +#include "rsc_mgr.h" >> + >> +/* >> + * Several RM calls take only a VMID as a parameter and give only >> standard >> + * response back. Deduplicate boilerplate code by using this common >> call. >> + */ >> +static int gh_rm_common_vmid_call(struct gh_rm *rm, u32 message_id, >> u16 vmid) >> +{ >> + struct gh_rm_vm_common_vmid_req req_payload = { >> + .vmid = cpu_to_le16(vmid), >> + }; >> + size_t resp_size; >> + void *resp; >> + >> + return gh_rm_call(rm, message_id, &req_payload, >> sizeof(req_payload), &resp, &resp_size); >> +} >> + >> +/** >> + * gh_rm_alloc_vmid() - Allocate a new VM in Gunyah. Returns the VM >> identifier. >> + * @rm: Handle to a Gunyah resource manager >> + * @vmid: Use GH_VMID_INVAL or 0 to dynamically allocate a VM. A >> reserved VMID can >> + * be supplied to request allocation of a platform-defined VM. > > Honestly, I'd rather just see 0 (and *not* GH_VMID_INVAL) be the > special value to mean "dynamically allocate the VMID." It seems > 0 is a reserved VMID anyway, and GH_VMID_INVAL might as well be > treated here as an invalid parameter. Done. > > Is there any definitition of which VMIDs are reserved? Like, > anything under 1024? It's platform dependent. On Qualcomm platforms, VMIDs <= 63 (QCOM_SCM_MAX_MANAGED_VMID) are reserved. Of those reserved VMIDs, Gunyah only allows us to allocate the "special VMs" (today: TUIVM, CPUSYSVM, OEMVM). Passing any value except 0, tuivm_vmid, cpusysvm_vmid, or oemvm_vmid returns an error. On current non-Qualcomm platforms, there aren't any reserved VMIDs so passing anything but 0 returns an error. Thanks, Elliot > > That's it on this patch for now. > > -Alex > >> + * >> + * Returns - the allocated VMID or negative value on error >> + */ >> +int gh_rm_alloc_vmid(struct gh_rm *rm, u16 vmid) >> +{ >> + struct gh_rm_vm_common_vmid_req req_payload = { 0 }; >> + struct gh_rm_vm_alloc_vmid_resp *resp_payload; >> + size_t resp_size; >> + void *resp; >> + int ret; > > . . . >
diff --git a/drivers/virt/gunyah/Makefile b/drivers/virt/gunyah/Makefile index cc864ff5abbb..de29769f2f3f 100644 --- a/drivers/virt/gunyah/Makefile +++ b/drivers/virt/gunyah/Makefile @@ -2,5 +2,5 @@ obj-$(CONFIG_GUNYAH) += gunyah.o -gunyah_rsc_mgr-y += rsc_mgr.o +gunyah_rsc_mgr-y += rsc_mgr.o rsc_mgr_rpc.o obj-$(CONFIG_GUNYAH) += gunyah_rsc_mgr.o diff --git a/drivers/virt/gunyah/rsc_mgr.h b/drivers/virt/gunyah/rsc_mgr.h index d4e799a7526f..7406237bc66d 100644 --- a/drivers/virt/gunyah/rsc_mgr.h +++ b/drivers/virt/gunyah/rsc_mgr.h @@ -74,4 +74,49 @@ struct gh_rm; int gh_rm_call(struct gh_rm *rsc_mgr, u32 message_id, void *req_buff, size_t req_buff_size, void **resp_buf, size_t *resp_buff_size); +/* Message IDs: VM Management */ +#define GH_RM_RPC_VM_ALLOC_VMID 0x56000001 +#define GH_RM_RPC_VM_DEALLOC_VMID 0x56000002 +#define GH_RM_RPC_VM_START 0x56000004 +#define GH_RM_RPC_VM_STOP 0x56000005 +#define GH_RM_RPC_VM_RESET 0x56000006 +#define GH_RM_RPC_VM_CONFIG_IMAGE 0x56000009 +#define GH_RM_RPC_VM_INIT 0x5600000B +#define GH_RM_RPC_VM_GET_HYP_RESOURCES 0x56000020 +#define GH_RM_RPC_VM_GET_VMID 0x56000024 + +struct gh_rm_vm_common_vmid_req { + __le16 vmid; + __le16 reserved0; +} __packed; + +/* Call: VM_ALLOC */ +struct gh_rm_vm_alloc_vmid_resp { + __le16 vmid; + __le16 reserved0; +} __packed; + +/* Call: VM_STOP */ +struct gh_rm_vm_stop_req { + __le16 vmid; +#define GH_RM_VM_STOP_FLAG_FORCE_STOP BIT(0) + u8 flags; + u8 reserved; +#define GH_RM_VM_STOP_REASON_FORCE_STOP 3 + __le32 stop_reason; +} __packed; + +/* Call: VM_CONFIG_IMAGE */ +struct gh_rm_vm_config_image_req { + __le16 vmid; + __le16 auth_mech; + __le32 mem_handle; + __le64 image_offset; + __le64 image_size; + __le64 dtb_offset; + __le64 dtb_size; +} __packed; + +/* Call: GET_HYP_RESOURCES */ + #endif diff --git a/drivers/virt/gunyah/rsc_mgr_rpc.c b/drivers/virt/gunyah/rsc_mgr_rpc.c new file mode 100644 index 000000000000..4515cdd80106 --- /dev/null +++ b/drivers/virt/gunyah/rsc_mgr_rpc.c @@ -0,0 +1,226 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#include <linux/gunyah_rsc_mgr.h> + +#include "rsc_mgr.h" + +/* + * Several RM calls take only a VMID as a parameter and give only standard + * response back. Deduplicate boilerplate code by using this common call. + */ +static int gh_rm_common_vmid_call(struct gh_rm *rm, u32 message_id, u16 vmid) +{ + struct gh_rm_vm_common_vmid_req req_payload = { + .vmid = cpu_to_le16(vmid), + }; + size_t resp_size; + void *resp; + + return gh_rm_call(rm, message_id, &req_payload, sizeof(req_payload), &resp, &resp_size); +} + +/** + * gh_rm_alloc_vmid() - Allocate a new VM in Gunyah. Returns the VM identifier. + * @rm: Handle to a Gunyah resource manager + * @vmid: Use GH_VMID_INVAL or 0 to dynamically allocate a VM. A reserved VMID can + * be supplied to request allocation of a platform-defined VM. + * + * Returns - the allocated VMID or negative value on error + */ +int gh_rm_alloc_vmid(struct gh_rm *rm, u16 vmid) +{ + struct gh_rm_vm_common_vmid_req req_payload = { 0 }; + struct gh_rm_vm_alloc_vmid_resp *resp_payload; + size_t resp_size; + void *resp; + int ret; + + if (vmid == GH_VMID_INVAL) + vmid = 0; + + req_payload.vmid = vmid; + + ret = gh_rm_call(rm, GH_RM_RPC_VM_ALLOC_VMID, &req_payload, sizeof(req_payload), &resp, + &resp_size); + if (ret) + return ret; + + if (!vmid) { + resp_payload = resp; + ret = le16_to_cpu(resp_payload->vmid); + kfree(resp); + } + + return ret; +} + +/** + * gh_rm_dealloc_vmid() - Dispose the VMID + * @rm: Handle to a Gunyah resource manager + * @vmid: VM identifier allocated with gh_rm_alloc_vmid + */ +int gh_rm_dealloc_vmid(struct gh_rm *rm, u16 vmid) +{ + return gh_rm_common_vmid_call(rm, GH_RM_RPC_VM_DEALLOC_VMID, vmid); +} + +/** + * gh_rm_vm_reset() - Reset the VM's resources + * @rm: Handle to a Gunyah resource manager + * @vmid: VM identifier allocated with gh_rm_alloc_vmid + * + * While tearing down the VM, request RM to clean up all the VM resources + * associated with the VM. Only after this, Linux can clean up all the + * references it maintains to resources. + */ +int gh_rm_vm_reset(struct gh_rm *rm, u16 vmid) +{ + return gh_rm_common_vmid_call(rm, GH_RM_RPC_VM_RESET, vmid); +} + +/** + * gh_rm_vm_start() - Move the VM into "ready to run" state + * @rm: Handle to a Gunyah resource manager + * @vmid: VM identifier allocated with gh_rm_alloc_vmid + * + * On VMs which use proxy scheduling, vcpu_run is needed to actually run the VM. + * On VMs which use Gunyah's scheduling, the vCPUs start executing in accordance with Gunyah + * scheduling policies. + */ +int gh_rm_vm_start(struct gh_rm *rm, u16 vmid) +{ + return gh_rm_common_vmid_call(rm, GH_RM_RPC_VM_START, vmid); +} + +/** + * gh_rm_vm_stop() - Send a request to Resource Manager VM to forcibly stop a VM. + * @rm: Handle to a Gunyah resource manager + * @vmid: VM identifier allocated with gh_rm_alloc_vmid + */ +int gh_rm_vm_stop(struct gh_rm *rm, u16 vmid) +{ + struct gh_rm_vm_stop_req req_payload = { + .vmid = cpu_to_le16(vmid), + .flags = GH_RM_VM_STOP_FLAG_FORCE_STOP, + .stop_reason = cpu_to_le32(GH_RM_VM_STOP_REASON_FORCE_STOP), + }; + size_t resp_size; + void *resp; + + return gh_rm_call(rm, GH_RM_RPC_VM_STOP, &req_payload, sizeof(req_payload), + &resp, &resp_size); +} + +/** + * gh_rm_vm_configure() - Prepare a VM to start and provide the common + * configuration needed by RM to configure a VM + * @rm: Handle to a Gunyah resource manager + * @vmid: VM identifier allocated with gh_rm_alloc_vmid + * @auth_mechanism: Authentication mechanism used by resource manager to verify + * the virtual machine + * @mem_handle: Handle to a previously shared memparcel that contains all parts + * of the VM image subject to authentication. + * @image_offset: Start address of VM image, relative to the start of memparcel + * @image_size: Size of the VM image + * @dtb_offset: Start address of the devicetree binary with VM configuration, + * relative to start of memparcel. + * @dtb_size: Maximum size of devicetree binary. Resource manager applies + * an overlay to the DTB and dtb_size should include room for + * the overlay. + */ +int gh_rm_vm_configure(struct gh_rm *rm, u16 vmid, enum gh_rm_vm_auth_mechanism auth_mechanism, + u32 mem_handle, u64 image_offset, u64 image_size, u64 dtb_offset, u64 dtb_size) +{ + struct gh_rm_vm_config_image_req req_payload = { 0 }; + size_t resp_size; + void *resp; + + req_payload.vmid = cpu_to_le16(vmid); + req_payload.auth_mech = cpu_to_le16(auth_mechanism); + req_payload.mem_handle = cpu_to_le32(mem_handle); + req_payload.image_offset = cpu_to_le64(image_offset); + req_payload.image_size = cpu_to_le64(image_size); + req_payload.dtb_offset = cpu_to_le64(dtb_offset); + req_payload.dtb_size = cpu_to_le64(dtb_size); + + return gh_rm_call(rm, GH_RM_RPC_VM_CONFIG_IMAGE, &req_payload, sizeof(req_payload), + &resp, &resp_size); +} + +/** + * gh_rm_vm_init() - Move the VM to initialized state. + * @rm: Handle to a Gunyah resource manager + * @vmid: VM identifier + * + * RM will allocate needed resources for the VM. + */ +int gh_rm_vm_init(struct gh_rm *rm, u16 vmid) +{ + return gh_rm_common_vmid_call(rm, GH_RM_RPC_VM_INIT, vmid); +} + +/** + * gh_rm_get_hyp_resources() - Retrieve hypervisor resources (capabilities) associated with a VM + * @rm: Handle to a Gunyah resource manager + * @vmid: VMID of the other VM to get the resources of + * @resources: Set by gh_rm_get_hyp_resources and contains the returned hypervisor resources. + */ +int gh_rm_get_hyp_resources(struct gh_rm *rm, u16 vmid, + struct gh_rm_hyp_resources **resources) +{ + struct gh_rm_vm_common_vmid_req req_payload = { + .vmid = cpu_to_le16(vmid), + }; + struct gh_rm_hyp_resources *resp; + size_t resp_size; + int ret; + + ret = gh_rm_call(rm, GH_RM_RPC_VM_GET_HYP_RESOURCES, + &req_payload, sizeof(req_payload), + (void **)&resp, &resp_size); + if (ret) + return ret; + + if (!resp_size) + return -EBADMSG; + + if (resp_size < struct_size(resp, entries, 0) || + resp_size != struct_size(resp, entries, le32_to_cpu(resp->n_entries))) { + kfree(resp); + return -EBADMSG; + } + + *resources = resp; + return 0; +} + +/** + * gh_rm_get_vmid() - Retrieve VMID of this virtual machine + * @rm: Handle to a Gunyah resource manager + * @vmid: Filled with the VMID of this VM + */ +int gh_rm_get_vmid(struct gh_rm *rm, u16 *vmid) +{ + static u16 cached_vmid = GH_VMID_INVAL; + size_t resp_size; + __le32 *resp; + int ret; + + if (cached_vmid != GH_VMID_INVAL) { + *vmid = cached_vmid; + return 0; + } + + ret = gh_rm_call(rm, GH_RM_RPC_VM_GET_VMID, NULL, 0, (void **)&resp, &resp_size); + if (ret) + return ret; + + *vmid = cached_vmid = lower_16_bits(le32_to_cpu(*resp)); + kfree(resp); + + return ret; +} +EXPORT_SYMBOL_GPL(gh_rm_get_vmid); diff --git a/include/linux/gunyah_rsc_mgr.h b/include/linux/gunyah_rsc_mgr.h index c992b3188c8d..e7bd29f8be6e 100644 --- a/include/linux/gunyah_rsc_mgr.h +++ b/include/linux/gunyah_rsc_mgr.h @@ -21,4 +21,77 @@ int gh_rm_notifier_unregister(struct gh_rm *rm, struct notifier_block *nb); void get_gh_rm(struct gh_rm *rm); void put_gh_rm(struct gh_rm *rm); +struct gh_rm_vm_exited_payload { + __le16 vmid; + __le16 exit_type; + __le32 exit_reason_size; + u8 exit_reason[]; +} __packed; + +#define GH_RM_NOTIFICATION_VM_EXITED 0x56100001 + +enum gh_rm_vm_status { + GH_RM_VM_STATUS_NO_STATE = 0, + GH_RM_VM_STATUS_INIT = 1, + GH_RM_VM_STATUS_READY = 2, + GH_RM_VM_STATUS_RUNNING = 3, + GH_RM_VM_STATUS_PAUSED = 4, + GH_RM_VM_STATUS_LOAD = 5, + GH_RM_VM_STATUS_AUTH = 6, + GH_RM_VM_STATUS_INIT_FAILED = 8, + GH_RM_VM_STATUS_EXITED = 9, + GH_RM_VM_STATUS_RESETTING = 10, + GH_RM_VM_STATUS_RESET = 11, +}; + +struct gh_rm_vm_status_payload { + __le16 vmid; + u16 reserved; + u8 vm_status; + u8 os_status; + __le16 app_status; +} __packed; + +#define GH_RM_NOTIFICATION_VM_STATUS 0x56100008 + +/* RPC Calls */ +int gh_rm_alloc_vmid(struct gh_rm *rm, u16 vmid); +int gh_rm_dealloc_vmid(struct gh_rm *rm, u16 vmid); +int gh_rm_vm_reset(struct gh_rm *rm, u16 vmid); +int gh_rm_vm_start(struct gh_rm *rm, u16 vmid); +int gh_rm_vm_stop(struct gh_rm *rm, u16 vmid); + +enum gh_rm_vm_auth_mechanism { + GH_RM_VM_AUTH_NONE = 0, + GH_RM_VM_AUTH_QCOM_PIL_ELF = 1, + GH_RM_VM_AUTH_QCOM_ANDROID_PVM = 2, +}; + +int gh_rm_vm_configure(struct gh_rm *rm, u16 vmid, enum gh_rm_vm_auth_mechanism auth_mechanism, + u32 mem_handle, u64 image_offset, u64 image_size, + u64 dtb_offset, u64 dtb_size); +int gh_rm_vm_init(struct gh_rm *rm, u16 vmid); + +struct gh_rm_hyp_resource { + u8 type; + u8 reserved; + __le16 partner_vmid; + __le32 resource_handle; + __le32 resource_label; + __le64 cap_id; + __le32 virq_handle; + __le32 virq; + __le64 base; + __le64 size; +} __packed; + +struct gh_rm_hyp_resources { + __le32 n_entries; + struct gh_rm_hyp_resource entries[]; +} __packed; + +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); + #endif
Add Gunyah Resource Manager RPC to launch an unauthenticated VM. Signed-off-by: Elliot Berman <quic_eberman@quicinc.com> --- drivers/virt/gunyah/Makefile | 2 +- drivers/virt/gunyah/rsc_mgr.h | 45 ++++++ drivers/virt/gunyah/rsc_mgr_rpc.c | 226 ++++++++++++++++++++++++++++++ include/linux/gunyah_rsc_mgr.h | 73 ++++++++++ 4 files changed, 345 insertions(+), 1 deletion(-) create mode 100644 drivers/virt/gunyah/rsc_mgr_rpc.c