Re: [PATCH v1 0/4] virt: vmgenid: Add devicetree bindings support

[Rob Herring]

On Wed, Mar 20, 2024 at 01:50:43PM +0000, David Woodhouse wrote:
> On Tue, 2024-03-19 at 16:24 +0100, Krzysztof Kozlowski wrote:
> > On 19/03/2024 15:32, Sudan Landge wrote:
> > > This small series of patches aims to add devicetree bindings support for
> > > the Virtual Machine Generation ID (vmgenid) driver.
> > > 
> > > 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.
> > > We would like to extend vmgenid to support devicetree bindings because:
> > > 1. A device should not be defined as an ACPI or DT only device.

This (and the binding patch) tells me nothing about what "Virtual 
Machine Generation ID driver" is and isn't really justification for 
"why".

> > 
> > Virtual stuff is not a device, so your first assumption or rationale is
> > not correct.
> > 
> > Virtual stuff can be ACPI only, because DT is not for Virtual stuff.
> 
> I strongly disagree with this.
> 
> Discovering things is what the device-tree is *for*.

DT/ACPI is for discovering what hardware folks failed to make 
discoverable. But here, both sides are software. Can't the software 
folks do better?

This is just the latest in $hypervisor bindings[1][2][3]. The value add 
must be hypervisors because every SoC vendor seems to be creating their 
own with their own interfaces.


> We don't want to add extra complexity and overhead on both host and
> guest side to make things discoverable in a *less* efficient way.
> 
> The device-tree isn't just a last-resort for when we can't possibly do
> things differently in a discoverable way. The device-tree is a first-
> class citizen and perfectly valid choice as a way to discover things.
> 
> We shouldn't be forcing people to turn things into PCI devices just to
> avoid adding DT bindings for them.
> 
> And we *certainly* shouldn't be directing people towards all the
> awfulness of ACPI, and in-kernel bytecode interpreters, and all that
> horridness, just because we don't want to use DT to... describe things.

I assume you have other calls into the hypervisor and notifications from 
the hypervisor? Are you going to add DT nodes for each one? I'd be more 
comfortable with DT describing THE communication channel with the 
hypervisor than what sounds like a singular function. Otherwise, what's 
the next binding?

Rob

[1] https://lore.kernel.org/all/20240222-gunyah-v17-2-1e9da6763d38@xxxxxxxxxxx/
[2] https://lore.kernel.org/all/20240129083302.26044-4-yi-de.wu@xxxxxxxxxxxx/
[3] https://lore.kernel.org/all/20240127004321.1902477-2-davidai@xxxxxxxxxx/