linux-kernel - Re: [PATCH v1 3/4] dt-bindings: Add bindings for vmgenid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <cfdb77c8-e893-41bf-965f-1013f3fc910e@linaro.org>
Date: Wed, 20 Mar 2024 11:24:07 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: "Landge, Sudan" <sudanl@...zon.co.uk>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 3/4] dt-bindings: Add bindings for vmgenid

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@...zon.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