Re: [RFC] drm: implement generic firmware eviction

Hey Rob!

On Wed, Aug 31, 2016 at 1:01 AM, Rob Herring [off-list ref] wrote:

On Tue, Aug 30, 2016 at 4:12 PM, David Herrmann [off-list ref] wrote:

quoted

Currently I see at least 3 paths that might add such nodes:

 - of_platform_populate()

If we assume the node is only under chosen, then that would require a
call to of_platform_populate with /chosen as the root which shouldn't
happen. More generally the assumption is of_platform_populate is only
called once from for the root node and only on sub-trees by the driver
of sub-tree's root device.

I see.

quoted

 - of_node_attach() (via the notifier)

A node getting attached would be harmless. Nothing really would happen
until a handler calls of_platform_populate. So this is the same as the
1st case.

drivers/of/platform.c has a notifier callback and registers the
platform devices when added. It has a check that the parent is a
populated bus, which I guess is what you refer to?

quoted

 - simplefb_init()

Actually, I'd prefer to move it out of there and into the core. I
don't think drivers should look at /chosen and only the core and arch
code should. Of course, the only enforcing of that is policy. For
overlays though, there's been some discussion on limiting where
overlays can be applied.

quoted

Should I just ignore anything but simplefb_init()? I understand that
it's the only one used by normal code-paths, but isn't it kinda ugly
to silently introduce race conditions if a node just happens to be
introduced via one of the other methods? Or are errors in the DT not
meant to be caught?

In general there's no limit to how wrong a DT could be. I could write
a DT that causes every DT enabled driver in the kernel to probe (that
would be an interesting test case). The kernel is limited in knowing
what is correct, and the whole point of DT is to move that information
out of the kernel.  This is case is just one compatible string out of
thousands and location in the tree is just one thing to check.

This is a major reason why there is not yet a userspace interface for
applying DT overlays as who knows what random crap could be in the DT.
We're nervous about what could happen from frying h/w to creating
security holes. Evidently the ACPI folks were not so nervous and added
an interface.

I see. Thanks a lot for the clarifications! I will put the
of_platform_device_destroy() right next to the x86 handler, making
sure it is properly locked. I'm not so sure about the of_chosen
instantiation, though. x86 has the instantiation in
arch/x86/kernel/sysfb_simplefb.c, so maybe an equivalent for arm would
be fine?

Thanks
David
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html