On 11/2/2022 5:20 PM, Greg Kroah-Hartman wrote:
On Wed, Nov 02, 2022 at 11:44:51AM -0700, Elliot Berman wrote:
On 11/2/2022 12:31 AM, Arnd Bergmann wrote:
On Wed, Oct 26, 2022, at 20:58, Elliot Berman wrote:
+static const struct file_operations gh_vm_fops = {
+ .unlocked_ioctl = gh_vm_ioctl,
+ .release = gh_vm_release,
+ .llseek = noop_llseek,
+};
There should be a .compat_ioctl entry here, otherwise it is
impossible to use from 32-bit tasks. If all commands have
arguments passed through a pointer to a properly defined
structure, you can just set it to compat_ptr_ioctl.
Ack.
+static long gh_dev_ioctl_create_vm(unsigned long arg)
+{
+ struct gunyah_vm *ghvm;
+ struct file *file;
+ int fd, err;
+
+ /* arg reserved for future use. */
+ if (arg)
+ return -EINVAL;
Do you have something specific in mind here? If 'create'
is the only command you support, and it has no arguments,
it would be easier to do it implicitly during open() and
have each fd opened from /dev/gunyah represent a new VM.
I'd like the argument here to support different types of virtual machines. I
want to leave open what "different types" can be in case something new comes
up in the future, but immediately "different type" would correspond to a few
different authentication mechanisms for virtual machines that Gunyah
supports.
Please don't add code that does not actually do something now, as that
makes it impossible to review properly, _AND_ no one knows what is going
to happen in the future. In the future, you can just add a new ioctl
and all is good, no need to break working userspace by suddenly looking
at the arg value and doing something with it.
I think the argument does something today and it's documented to need to
be 0. If a userspace from the future provides non-zero value, Linux will
correctly reject it because it doesn't know how to interpret the
different VM types.
I can document it more clearly as the VM type field and support only the
one VM type today.
Creating new ioctl for each VM type feels to me like I didn't design
CREATE_VM ioctl right in first place.
Thanks,
Elliot
