Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Nov 2022 01:20:48 +0100
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     Elliot Berman <quic_eberman@quicinc.com>
Cc:     Arnd Bergmann <arnd@arndb.de>,
        Bjorn Andersson <quic_bjorande@quicinc.com>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        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>,
        Prakruthi Deepak Heragu <quic_pheragu@quicinc.com>,
        Andy Gross <agross@kernel.org>,
        Jassi Brar <jassisinghbrar@gmail.com>,
        linux-arm-kernel@lists.infradead.org,
        Mark Rutland <mark.rutland@arm.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Marc Zyngier <maz@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Will Deacon <will@kernel.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
        Amol Maheshwari <amahesh@qti.qualcomm.com>,
        Kalle Valo <kvalo@kernel.org>, devicetree@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 13/21] gunyah: vm_mgr: Introduce basic VM Manager
Message-ID: <Y2MJYEqnJONvH0fY@kroah.com>
References: <20221026185846.3983888-1-quic_eberman@quicinc.com>
 <20221026185846.3983888-14-quic_eberman@quicinc.com>
 <c1f86c53-1d9f-4faf-9313-de86d33e3739@app.fastmail.com>
 <7c59a115-36c5-c954-5610-ef5ef1dbb83e@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <7c59a115-36c5-c954-5610-ef5ef1dbb83e@quicinc.com>
Precedence: bulk

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