[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y2MJYEqnJONvH0fY@kroah.com>
Date: Thu, 3 Nov 2022 01:20:48 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Elliot Berman <quic_eberman@...cinc.com>
Cc: Arnd Bergmann <arnd@...db.de>,
Bjorn Andersson <quic_bjorande@...cinc.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Murali Nalajala <quic_mnalajal@...cinc.com>,
Trilok Soni <quic_tsoni@...cinc.com>,
Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>,
Carl van Schaik <quic_cvanscha@...cinc.com>,
Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>,
Andy Gross <agross@...nel.org>,
Jassi Brar <jassisinghbrar@...il.com>,
linux-arm-kernel@...ts.infradead.org,
Mark Rutland <mark.rutland@....com>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Sudeep Holla <sudeep.holla@....com>,
Marc Zyngier <maz@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Jonathan Corbet <corbet@....net>,
Will Deacon <will@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
Amol Maheshwari <amahesh@....qualcomm.com>,
Kalle Valo <kvalo@...nel.org>, devicetree@...r.kernel.org,
linux-doc@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 13/21] gunyah: vm_mgr: Introduce basic VM Manager
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.
thanks,
greg k-h
Powered by blists - more mailing lists