Messages in this thread | | | Subject | Re: [PATCH v1 3/4] dt-bindings: Add bindings for vmgenid | Date | Wed, 20 Mar 2024 12:16:10 +0000 | From | "Landge, Sudan" <> |
| |
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 >
| |