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

[Srinivas Kandagatla]

On 23/02/2023 00:50, Elliot Berman wrote:
> > > +
> > > +    mem_handle = mapping->parcel.mem_handle;
> > > +    dtb_offset = ghvm->dtb_config.gpa - mapping->guest_phys_addr;
> > > +
> > > +    ret = gh_rm_vm_configure(ghvm->rm, ghvm->vmid, ghvm->auth, 
> > > mem_handle,
> >
> > where is authentication mechanism (auth) comming from? Who is supposed 
> > to set this value?
> >
> > Should it come from userspace? if so I do not see any UAPI facility to 
> > do that via VM_START ioctl.
>
> Right, we are only adding the support for unauthenticated VMs for now. 
> There would be further UAPI facilities to set the authentication type.
We have to be careful, please note that you can not change an existing 
UAPI to accommodate new features.

There are two ways to do this properly:

1. Design UAPI to accommodate features that will be part of this in very 
 soon or in future. This way the UAPI is stable and does not change 
over time when we add support this feature in driver.

In this particular case, vm authentication type is one that needs to 
come from user, rather than kernel assuming it, so definitely this need 
to be properly addressed by passing this info from userspace.
Or rename this IOCTl to something like VM_START_UNAUTH_VM to make this 
more explicit.


2. For each feature add new UAPI as and when its required, which is 
really the only option when we failed to design UAPIs correctly in the 
first place.

--srini


> > > +                0, 0, dtb_offset, ghvm->dtb_config.size);
> > > +    if (ret) {