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

[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@xxxxxxxxxx>
> > > > ---
> > > >    .../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