Re: [PATCH v11 13/26] gunyah: vm_mgr: Add ioctls to support basic non-proxy VM boot

From: Elliot Berman <quic_eberman@quicinc.com>
To: Alex Elder <elder@linaro.org>,
	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	Prakruthi Deepak Heragu <quic_pheragu@quicinc.com>
Cc: Murali Nalajala <quic_mnalajal@quicinc.com>,
	Trilok Soni <quic_tsoni@quicinc.com>,
	Srivatsa Vaddagiri <quic_svaddagi@quicinc.com>,
	Carl van Schaik <quic_cvanscha@quicinc.com>,
	Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
	Bjorn Andersson <andersson@kernel.org>,
	"Konrad Dybcio" <konrad.dybcio@linaro.org>,
	Arnd Bergmann <arnd@arndb.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Bagas Sanjaya <bagasdotme@gmail.com>,
	Will Deacon <will@kernel.org>, Andy Gross <agross@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Jassi Brar <jassisinghbrar@gmail.com>,
	<linux-arm-msm@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-doc@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v11 13/26] gunyah: vm_mgr: Add ioctls to support basic non-proxy VM boot
Date: Tue, 11 Apr 2023 14:16:28 -0700	[thread overview]
Message-ID: <d736d53b-e289-85ca-ee78-e1f77ccc08c6@quicinc.com> (raw)
In-Reply-To: <3271fdf5-57cd-d962-fd20-01f5a5447232@linaro.org>

On 3/31/2023 7:26 AM, Alex Elder wrote:
> On 3/3/23 7:06 PM, Elliot Berman wrote:
>> diff --git a/include/linux/gunyah_rsc_mgr.h 
>> b/include/linux/gunyah_rsc_mgr.h
>> index 88a429dad09e..8b0b46f28e39 100644
>> --- a/include/linux/gunyah_rsc_mgr.h
>> +++ b/include/linux/gunyah_rsc_mgr.h
>> @@ -29,6 +29,12 @@ struct gh_rm_vm_exited_payload {
>>   #define GH_RM_NOTIFICATION_VM_EXITED         0x56100001
>>   enum gh_rm_vm_status {
>> +    /**
>> +     * RM doesn't have a state where load partially failed because
>> +     * only Linux
> 
> I have no idea what the comment above means...  Please fix.
> 
> Several of the values below are never explicitly assigned,
> and some are used but not assigned.  The others apparently
> might come back from the resource manager?  Why, for
> example, are the PAUSED, AUTH, and RESETTING statuses
> defined if we don't use them?
> 

I ended up no longer needing VM_STATUS_LOAD_FAILED.

The other status values are defined by Gunyah resource manager. RM will 
notify us about the state transitions.

Some of the state transitions can be inferred by Linux directly. For 
instance, gh_rm_vm_init() will transition the VM from 
GH_RM_VM_STATUS_INIT to GH_RM_VM_STATUS_READY iff it returns 
successfully. There is one instance where we wait for VM to exit during 
the VM teardown as well.

Thanks,
Elliot

>> +     */
>> +    GH_RM_VM_STATUS_LOAD_FAILED    = -1,
>> +
>>       GH_RM_VM_STATUS_NO_STATE    = 0,
>>       GH_RM_VM_STATUS_INIT        = 1,
>>       GH_RM_VM_STATUS_READY        = 2,
>> diff --git a/include/uapi/linux/gunyah.h b/include/uapi/linux/gunyah.h
>> index a19207e3e065..d6abd8605a2e 100644
>> --- a/include/uapi/linux/gunyah.h
>> +++ b/include/uapi/linux/gunyah.h
>> @@ -49,4 +49,17 @@ struct gh_userspace_memory_region {
>>   #define GH_VM_SET_USER_MEM_REGION    _IOW(GH_IOCTL_TYPE, 0x1, \
>>                           struct gh_userspace_memory_region)
>> +/**
>> + * struct gh_vm_dtb_config - Set the location of the VM's devicetree 
>> blob
>> + * @guest_phys_addr: Address of the VM's devicetree in guest memory.
>> + * @size: Maximum size of the devicetree.
>> + */
>> +struct gh_vm_dtb_config {
>> +    __u64 guest_phys_addr;
>> +    __u64 size;
>> +};
>> +#define GH_VM_SET_DTB_CONFIG    _IOW(GH_IOCTL_TYPE, 0x2, struct 
>> gh_vm_dtb_config)
>> +
>> +#define GH_VM_START        _IO(GH_IOCTL_TYPE, 0x3)
>> +
>>   #endif
> 