Re: [PATCH v5 11/13] powerpc: Allow userspace to set device tree properties in kexec_file_load

[Thiago Jung Bauermann]

Hello Sam,

Thanks for the quick response.

Am Freitag, 12 August 2016, 10:45:00 schrieb Samuel Mendoza-Jonas:
> On Thu, 2016-08-11 at 20:08 -0300, Thiago Jung Bauermann wrote:
> > @@ -908,4 +909,245 @@ bool find_debug_console(const void *fdt, int
> > chosen_node) return false;
> >  }
> >  
> > +/**
> > + * struct allowed_node - a node in the whitelist and its allowed
> > properties. + * @name:              node name or full node path
> > + * @properties:                NULL-terminated array of names or
> > name=value pairs + *
> > + * If name starts with /, then the node has to be at the specified path
> > in + * the device tree (including unit addresses for all nodes in the
> > path). + * If it doesn't, then the node can be anywhere in the device
> > tree. + *
> > + * An entry in properties can specify a string value that the property
> > must + * have by using the "name=value" format. If the entry ends with
> > =, it means + * that the property must be empty.
> > + */
> > +static struct allowed_node {
> > +       const char *name;
> > +       const char *properties[9];
> > +} allowed_nodes[] = {
> > +       {
> > +               .name = "/chosen",
> > +               .properties = {
> > +                       "stdout-path",
> > +                       "linux,stdout-path",
> > +                       NULL,
> > +               }
> > +       },
> > +       {
> > +               .name = "vga",
> > +               .properties = {
> > +                       "device_type=display",
> > +                       "assigned-addresses",
> > +                       "width",
> > +                       "height",
> > +                       "depth",
> > +                       "little-endian=",
> > +                       "linux,opened=",
> > +                       "linux,boot-display=",ss
> > +                       NULL,
> > +               }
> > +       },
> > +};
> 
> Hi Thiago,
> 
> As much as this solves problems for *me*, I suspect adding 'vga' here
> might be the subject of some discussion. Having /chosen whitelisted makes
> sense on it's own, but 'vga' and its properties are very specific without
> much explanation.
> 
> If everyone's happy to have it there, cool! If not, I have the majority
> of a patch that handles the original reason for these property updates
> separately in the kernel rather than from userspace. If needed I'll clean
> it up and we can handle it that way.

Ok, that's good to know. I'm fine with it either way. In any case, 'vga' in 
this patch also serves a good real-life example of a non-trivial binding 
outside of /chosen that we might want to whitelist in the future.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center