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;
Message-ID: <95a9f253-984a-14e0-7e01-f168452576c4@quicinc.com>
Date:   Fri, 4 Nov 2022 21:19:24 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH v6 13/21] gunyah: vm_mgr: Introduce basic VM Manager
Content-Language: en-US
To:     Elliot Berman <quic_eberman@quicinc.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC:     Bjorn Andersson <quic_bjorande@quicinc.com>,
        Murali Nalajala <quic_mnalajal@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>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.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>
References: <20221026185846.3983888-1-quic_eberman@quicinc.com>
 <20221026185846.3983888-14-quic_eberman@quicinc.com>
 <Y2H8oh7AvYDiMqKs@kroah.com>
 <722b05a1-4bf5-0837-baea-b1d0a9cc1e43@quicinc.com>
 <Y2MKWOihjAPxfl6v@kroah.com>
 <96238455-73b6-bead-0fdb-55ca68e5bf0b@quicinc.com>
 <9dd597d9-a3f3-48f2-8416-b5b097a230d5@app.fastmail.com>
 <980db147-794e-ecd9-9626-64ff81109bab@quicinc.com>
From:   Trilok Soni <quic_tsoni@quicinc.com>
In-Reply-To: <980db147-794e-ecd9-9626-64ff81109bab@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 11/4/2022 3:38 PM, Elliot Berman wrote:
> 
> 
> On 11/4/2022 1:10 AM, Arnd Bergmann wrote:
>> On Fri, Nov 4, 2022, at 01:11, Elliot Berman wrote:
>>> On 11/2/2022 5:24 PM, Greg Kroah-Hartman wrote:
>>>> On Wed, Nov 02, 2022 at 11:45:12AM -0700, Elliot Berman wrote:
>>>>
>>>> Even if you don't support it 1:1, at least for the ones that are the
>>>> same thing, pick the same numbers as that's a nicer thing to do, right?
>>>>
>>>
>>> Does same thing == interpretation of arguments is the same? For
>>> instance, GH_CREATE_VM and KVM_CREATE_VM interpret the arguments
>>> differently. Same for KVM_SET_USERSPACE_MEMORY. The high level
>>> functionality should be similar for most all hypervisors since they will
>>> all support creating a VM and probably sharing memory with that VM. The
>>> arguments for that will necessarily look similar, but they will probably
>>> be subtly different because the hypervisors support different features.
>>
>> I think in the ideal case, you should make the arguments and the
>> command codes the same for any command where that is possible. If
>> you come across a command that is shared with KVM but just needs
>> another flag, that would involve coordinating with the KVM maintainers
>> about sharing the definition so the same flag does not get reused
>> in an incompatible way.
>>
> 
> I think the converse also needs to be true; KVM would need to check that
> new flags don't get used in some incompatible way with Gunyah, even if
> one of us is just -EINVAL'ing. I don't think Gunyah and KVM should be
> reliant on the other reviewing shared ioctls.
> 
> The problem is a bit worse because "machine type" is architecture-
> dependent whereas the planned Gunyah flags are architecture-independent.
> KVM within itself re-uses flags between architectures so Gunyah would
> need to reserve some flags from all architectures that KVM supports.

I agree w/ Elliot. We would like to keep Gunyah independent and not rely 
on the existing KVM ioctls space. We should allow new hypervisor drivers 
interfaces addition in Linux kernel without them relying on KVM.

> 
>> For commands that cannot fit into the existing definition, there
>> should be a different command code, using your own namespace and
>> not the 0xAE block that KVM has. It still makes sense to follow
>> the argument structure roughly here, unless there is a technical
>> reason for making it different.
>>
>>> I don't think userspace that supports both KVM and Gunyah will benefit
>>> much from re-using the same numbers since those re-used ioctl calls
>>> still need to sit within the context of a Gunyah VM.
>>
>> One immediate benefit is for tools that work on running processes,
>> such as strace, gdb or qemu-user. If they encounter a known command,
>> they can correctly display the arguments etc.
>>
> 
> We can update these tools and anyway there will be different ioctls to
> get started. There are important ioctls that wouldn't be correctly
> displayed off the bat anyway; work would need to be done to support the
> Gunyah ioctls either way. Whereas tooling update is temporary, the
> coupling of KVM and Gunyah ioctls would be permanent.

Agree, tools can be updated and that is the easy part as we grow the s/w 
stack around Gunyah in userspace, like we already do w/ CrosVM (Virtual 
Machine Manager) and QEMU will be next followed by rust-vmm. All of them 
can be done without Gunyah ioctls relying anything on the KVM ioctls. 
Elliot has also explained very well that we don't to go to KVM 
maintainers for any of our additions and we also don't want them to come 
to us, since there is no interoperability testing. It is best that both 
Hypervisors and their Linux interfaces evolve independently.

---Trilok Soni