Received-SPF: pass (google.com: domain of linux-kernel+bounces-111788-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Subject: Re: [PATCH v1 0/4] virt: vmgenid: Add devicetree bindings support
Message-ID: <d790d5cc-a116-4d5b-97a4-da0d073ff3e3@amazon.co.uk>
Date: Fri, 22 Mar 2024 16:39:07 +0000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: David Woodhouse <dwmw2@infradead.org>, Rob Herring <robh@kernel.org>
CC: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, Sudan Landge
	<sudanl@amazon.com>, <tytso@mit.edu>, <Jason@zx2c4.com>,
	<krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
	<sathyanarayanan.kuppuswamy@linux.intel.com>, <thomas.lendacky@amd.com>,
	<dan.j.williams@intel.com>, <devicetree@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <graf@amazon.de>, <bchalios@amazon.es>,
	<xmarcalx@amazon.co.uk>, <ardb@kernel.org>, benh <benh@kernel.crashing.org>
References: <20240319143253.22317-1-sudanl@amazon.com>
 <23692c07-98bd-477d-b244-bba14c50352c@linaro.org>
 <38aad6c0e698c8e804694276d1762d61f2068ce8.camel@infradead.org>
 <20240320161531.GA1810860-robh@kernel.org>
 <60404403932a984d1f75d111ff53b9053af03579.camel@infradead.org>
 <20240321133250.GA1600070-robh@kernel.org>
 <db5a1027-93b7-4630-b679-8a654905dc48@amazon.co.uk>
 <17611183-f288-47fe-a5e1-91ee16168db0@linaro.org>
 <ee688cca986d95148a55e32fee48ceed8567f128.camel@infradead.org>
 <CAL_JsqJOn9yU_YC0+q=4DX9_=0+z8yTact0HAvOKbrUpcXHxkQ@mail.gmail.com>
 <0a83e174db16e15cb0f0d3ac37d6717c918ee78d.camel@infradead.org>
From: "Landge, Sudan" <sudanl@amazon.co.uk>
In-Reply-To: <0a83e174db16e15cb0f0d3ac37d6717c918ee78d.camel@infradead.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit


On 22/03/2024 14:27, David Woodhouse wrote:
> On Fri, 2024-03-22 at 08:22 -0500, Rob Herring wrote:
>>
>>>> What stops you from passing fw_cfg not to UEFI FW? BTW, no actual VM
>>>> name was used in your posting, but now suddenly it is a talk about QEMU.
> 
> (Forgot to address the second part of that last time. No specific VMM
> was mentioned in the first place because this isn't VMM-specific)
> 
QEMU is referenced to explain `vmgenid` which they are also using and 
have more documentation on it. We mentioned the hypervisor we tested the 
changes with in the cover letter which is 
https://github.com/firecracker-microvm/firecracker but this change isn't 
VMM specific.

>>> That would be possible. But not ideal.
>>
>> Why not ideal?
>>
>> To rephrase the question, why is it fine for UEFI to read the vmgenid
>> from fw_cfg, but the kernel can't use the same mechanism?
> 
> Because fw_cfg an incestuous way to get data from the VMM into the BIOS
> (both SeaBIOS and UEFI). It's the way we pass the ACPI tables and
> things like that.
> 
> It *isn't* designed as a general-purpose way of doing device discovery
> for use by various operating systems.
> 
> I'm also not sure Firecracker, which is the VMM Sudan is working on,
> even *has* fw_cfg. Especially on ARM. If we're going to be forced to
> add some complicated device with MMIO and DMA just to be able to
> advertise the existence of a simple memory region, that's just as bad
> as being forced to expose it as an emulated PCI device.
> 
> This is what DT is *for*.
> 
> 
>> The response
>> that you'd have to use UEFI to use fw_cfg makes no sense to me. The
>> only reason I can think of is just being lazy and wanting to have
>> minimal changes to some existing driver. It looks to me like you could
>> implement this entirely in userspace already with zero kernel or
>> binding changes. From a quick look, we already have a fw_cfg driver
>> exposing UUID (that's the same thing as vmgenid AIUI) to userspace,
>> and you can feed that back into the random pool.
>>
>> I am concerned that we already have a mechanism and you want to add a
>> second way. When do we ever think that's a good idea? What happens
>> on the next piece of fw_cfg data? We add yet another binding?
> 
> No, because fw_cfg is a way for the VMM to give configuration
> information to the firmware. There's a clue in the name. The firmware
> then sets up ACPI tables or DT to pass information in a more coherent
> and structured fashion to general-purpose operating systems.
> 
> And some VMMs *don't* use fw_cfg at all because for the minimal microvm
> case it's overkill.
> 
The hypervisor we work on 
(https://github.com/firecracker-microvm/firecracker) does not have 
fw_cfg, it loads kernel directly without the need for UEFI or any 
intermediate firmware. It is, as said, an overkill to enable UEFI and 
fw_cfg just to support `vmgenid` specially when there is an alternative 
available which could keep things simple for the vmm and for the linux 
driver.