Received-SPF: pass (google.com: domain of linux-kernel+bounces-108899-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Subject: Re: [PATCH v1 3/4] dt-bindings: Add bindings for vmgenid
Message-ID: <7d8f7c42-02e9-46df-8ea5-ab9a4b9721dc@amazon.co.uk>
Date: Wed, 20 Mar 2024 12:16:10 +0000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, Sudan Landge
	<sudanl@amazon.com>, <tytso@mit.edu>, <Jason@zx2c4.com>,
	<robh+dt@kernel.org>, <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>
References: <20240319143253.22317-1-sudanl@amazon.com>
 <20240319143253.22317-4-sudanl@amazon.com>
 <ba8418ab-2829-416c-8e20-414f7818cab9@linaro.org>
 <f221da06-2a7c-4db3-a0de-870156865631@amazon.co.uk>
 <cfdb77c8-e893-41bf-965f-1013f3fc910e@linaro.org>
CC: <graf@amazon.de>, <dwmw@amazon.co.uk>, <bchalios@amazon.es>,
	<xmarcalx@amazon.co.uk>
From: "Landge, Sudan" <sudanl@amazon.co.uk>
In-Reply-To: <cfdb77c8-e893-41bf-965f-1013f3fc910e@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

Hi Krzysztof,


The recipient were removed by mistake. I have added them all back and 
fixed the email client to send mail in the right format. Sorry about that.
I'll revert after I have done more analysis and better data to explain.
Thank you for the quick feedback again.

Thanks and regards,
Sudan

On 20/03/2024 10:24, Krzysztof Kozlowski wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> On 20/03/2024 11:17, Landge, Sudan wrote:
>>
>> On 19/03/2024 15:28, Krzysztof Kozlowski wrote:
>>> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>>>
>>>
> 
> Why did you remove all the people from CC list?
> >>>
>>> On 19/03/2024 15:32, Sudan Landge wrote:
>>>> Virtual Machine Generation ID driver was introduced in commit af6b54e2b5ba
>>>> ("virt: vmgenid: notify RNG of VM fork and supply generation ID"), as an
>>>> ACPI only device.
>>> That's not a valid rationale. Second today... we do not add things to
>>> bindings just because someone added some crazy or not crazy idea to Linux.
>>>
>>> Bindings represent the hardware.
>>>
>>> Please come with real rationale. Even if this is accepted, above reason
>>> is just wrong and will be used as an excuse to promote more crap into
>>> bindings.
>>
>> Thank you for the quick review.
>>
>> I will add more details to the problem we are trying to fix with an
>> updated cover letter
>>
>> but to summarize the problem briefly:
>>
>> Firecracker is a minimalist feature hypervisor and we do not have ACPI
>> support
>>
>> for ARM yet. The vmgenid devicetree support looked a better option because
>>
>> supporting ACPI on ARM means supporting UEFI which adds a lot of complexity.
> 
> That does not convince me. Amount of work for you is not making virtual
> stuff real hardware. Come with some other discoverable protocol - you
> have full control of both sides of this thing.
> 
>>
>>> A nit, subject: drop second/last, redundant "bindings". The
>>> "dt-bindings" prefix is already stating that these are bindings.
>>> See also:
>>> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
>>>
>>> Please use subject prefixes matching the subsystem. You can get them for
>>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>>> your patch is touching.
>> Got it, thanks.
>>>> Add a devicetree binding support for vmgenid so that hypervisors
>>>> can support vmgenid without the need to support ACPI.
>>> Devicetree is not for virtual platforms. Virtual platform can define
>>> whatever interface they want (virtio, ACPI, "VTree" (just invented now)).
>> Sorry for my lack of experience in this area. I took reference of virtio
>> devices when I
>>
>> uploaded the patch. We would still like to support vmgenid via a
>> devicetree so I'll
>>
>> revert with a new approach.
> 
> There are other solutions, I think. This was discussed already multiple
> times.
> 
>>
>>>> Signed-off-by: Sudan Landge<sudanl@amazon.com>
>>>> ---
>>>>    .../devicetree/bindings/vmgenid/vmgenid.yaml  | 57 +++++++++++++++++++
>>> No, you do not get your own hardware subsystem. Use existing ones.
>>
>> Got it. The changes are related to the "rng" subsystem so I'll rethink
>> if that is the
>>
>> right place for this and revert.
> 
> Your wrapping is odd. Please use some decent email client.
> 
> Anyway, I am not discussing topics semi-private. Keep all maintainers in
> loop.
> 
> Best regards,
> Krzysztof
>