Hi Rob, > On Oct 6, 2017, at 16:55 , Rob Herring <robherring2@xxxxxxxxx> wrote: > > On Tue, Oct 3, 2017 at 12:39 PM, Pantelis Antoniou > <pantelis.antoniou@xxxxxxxxxxxx> wrote: >> Hi Rob, >> >> On Tue, 2017-10-03 at 12:13 -0500, Rob Herring wrote: >>> On Tue, Oct 3, 2017 at 9:13 AM, Pantelis Antoniou >>> <pantelis.antoniou@xxxxxxxxxxxx> wrote: >>>> Hi Rob, >>>> >>>> On Tue, 2017-10-03 at 08:18 -0500, Rob Herring wrote: >>>>> On Mon, Oct 2, 2017 at 2:46 PM, Pantelis Antoniou >>>>> <pantelis.antoniou@xxxxxxxxxxxx> wrote: >>>>>> Hi Rob, >>>>>> >>>>>> On Sun, 2017-10-01 at 17:00 -0500, Rob Herring wrote: >>>>>>> On Thu, Sep 28, 2017 at 2:58 PM, Pantelis Antoniou >>>>>>> <pantelis.antoniou@xxxxxxxxxxxx> wrote: >>>>>>>> Hello again, >>>>>>>> >>>>>>>> Significant progress has been made on yamldt and is now capable of >>>>>>>> not only generating yaml from DTS source but also compiling DTS sources >>>>>>>> and being almost fully compatible with DTC. >>>>>>> >>>>>>> Can you quantify "almost"? >>>>>>> >>>>>>>> Compiling the kernel's DTBs using yamldt is as simple as using a >>>>>>>> DTC=yamldt. >>>>>>> >>>>>>> Good. >>>>>>> >>>>>>>> >>>>>>>> Error reporting is accurate and validation against a YAML based schema >>>>>>>> works as well. In a short while I will begin posting patches with >>>>>>>> fixes on bindings and DTS files in the kernel. >>>>>>> >>>>>>> What I would like to see is the schema format posted for review. >>>>>>> >>>>>> >>>>>> I'm including the skeleton.yaml binding which is the template for >>>>>> the bindings and a board-example.yaml binding for a top level binding. >>>>>> >>>>>>> I would also like to see the bindings for top-level compatible strings >>>>>>> (aka boards) as an example. That's something that's simple enough that >>>>>>> I'd think we could agree on a format and start moving towards defining >>>>>>> board bindings that way. >>>>>>> >>>>>> >>>>>> Note there is some line wrapping I'm including a link >>>>>> to the github repo of the files: >>>>>> >>>>>> >>>>>> The skeleton.yaml >>>>>> >>>>>> https://raw.githubusercontent.com/pantoniou/yamldt/master/validate/bindings/skeleton.yaml >>>>>> >>>>>> %YAML 1.1 >>>>>> --- >>>>>> # The name of the binding is first >>>>>> # The anchor is put there for use by others >>>>>> skeleton: &skeleton >>>>> >>>>> This and "id" seem redundant. >>>>> >>>> >>>> Indeed. >>> >>> One other thing, "skeleton" is a bit weird as a key. It can't be >>> validated. All the other keys are standard words. I could write >>> "skeloton" by mistake and I guess I'd only find the mistake when >>> something inherits it. That's somewhat true with id, but we can at >>> least check "id" is present and that it's value is unique among all >>> other id values. >>> >> >> We can keep id and check that it matches the name of the enclosing node. >> That way you can be sure that there's no error. >> >> But it's a bit weird cause this is similar to declaring a function name >> with a typo. You won't find out until you use it. >> >>>> >>>>>> version: 1 >>>>>> >>>>>> id: skel-device >>>>>> >>>>>> title: > >>>>>> Skeleton Device >>>>>> >>>>>> maintainer: >>>>>> name: Skeleton Person <skel@xxxxxxxxxx> >>>>>> >>>>>> description: > >>>>>> The Skeleton Device binding represents the SK11 device produced by >>>>>> the Skeleton Corporation. The binding can also support compatible >>>>>> clones made by second source vendors. >>>>>> >>>>>> # The class is an optional property that declares this >>>>>> # binding as part of a larger set >>>>>> # Multiple definitions are possible >>>>>> class: [ device, spi-device ] >>>>>> >>>>>> # This binding inherits property characteristics from the generic >>>>>> # spi-slave binding >>>>>> # Note that the notation is standard yaml reference >>>>>> inherits: *spi-slave >>>>>> >>>>>> # virtual bindings do not generate checkers >>>>>> virtual: true >>>>> >>>>> virtual is an overloaded term. >>>>> >>>> >>>> OK, what term should I use that this binding should not be instantiated >>>> as a checker, only be used by other bindings when inherited? >>> >>> checks: true? >>> >>> I'd really like to avoid having to decide and drop this, but I don't >>> really get why it is needed. >>> >> >> It is needed because otherwise checker filters will be generated for >> the template bindings that while they won't be executed they will be >> compiled and take up space in the schema. > > Size is not a problem I have. That can come later if needed. > >>>>>> # each property is defined by each name >>>>>> properties: >>>>>> >>>>>> # The compatible property is a reserved name. The type is always >>>>>> "string" >>>>>> # and should not be repeated device binding. >>>>>> compatible: >>>>>> category: required # required property >>>>>> type: strseq # is a sequence of strings >>>>>> >>>>>> description: > >>>>>> FX11 is a clone of the original SK11 device >>>>>> >>>>>> # v is always the name of the value of the property >>>>>> # np is passed to the checker and is the current >>>>>> # node pointer. We can access properties and call >>>>>> # methods that operate on them. >>>>>> # There can be multiple constraints, just put them >>>>>> # into a sequence. >>>>>> # Note that the BASE("skel,sk11") form from the previous >>>>>> # binding will have to be reworked. >>>>>> constraint: | >>>>>> anystreq(v, "skel,sk11") || >>>>>> anystreq(v, "faux,fx11") >>>>> >>>>> Constraints and logic ops were certainly not decided in the last >>>>> attempt and I think will be the hardest part to define. I see you are >>>>> using eBPF in the checker. Is that where anystreq comes from? >>>>> >>>> >>>> Yes. The ebpf environment declares a number of methods that are executed >>>> outside the ebpf sandbox. Check out >>>> >>>> https://raw.githubusercontent.com/pantoniou/yamldt/master/validate/schema/codegen.yaml >>>> https://github.com/pantoniou/yamldt/blob/master/ebpf_dt.c >>> >>> I looked at this some a while back. It wasn't clear to me what eBPF >>> gives us and what you have defined on top of it. What I'm really after >>> is documentation of the syntax and keywords here. >>> >> >> eBPF is portable, can be serialized after compiling in the schema file >> and can be executed in the kernel. > > Executing in the kernel is a non-goal for me. > >> By stripping out all documentation related properties and nodes keeping >> only the compiled filters you can generate a dtb blob that passed to >> kernel can be used for verification of all runtime changes in the >> kernel's live tree. eBPF is enforcing an execution model that is 'safe' >> so we can be sure that no foul play is possible. > > Humm, if you wanted to ensure dtb's are safe, I'd think that we just > sign them like you would for the kernel or modules. > That’s a problem when deploying; the runtime validation is for making sure developers don’t get bogged down chasing problems when working on their own platforms/drivers. We have absolutely zero checks for stopping badly configured DT blobs hanging the kernel. With runtime validation a bug that might take a few days to figure out can be cut down to a few minutes. >> That means that you can a) run it at boot-time, verifying the dtb blob >> passed by the bootloader for errors (potentially disabling devices >> that their nodes fail) and b) run it when applying overlays to reject >> any that result in an invalid tree. > > Let's get verification at build time working first, then we can worry > about something like this. > >> One other use that I'm thinking that might be possible too, like >> binding version check between what the driver is compiled against and >> what the passed blob is without using an explicit version property. >> >> The constraint format ofcourse is plain C, you can generate a .so using >> the host compiler and execute that if you are looking for performance. >> >> Syntax and keywords of the internals is in a bit of flux right now, >> since the binding format is not finalized yet. I'd be happy to >> discuss the internal at ELCE. > > I won't be there. > > I'm just trying to understand what eBPF defines if anything and what's > up for discussion. The eBPF defines an API that the constraints use to perform validation. Anything that can be expressed as a C method can be in the eBPF API. > > [...] > >>>>>> constraint: | >>>>>> get_depth(np) == 0 && ( >>>>> >>>>> Ahh, I guess this is how you are doing it. I don't think this works >>>>> for any value other than 0. An MFD could be at any level. >>>>> >>>> >>>> Well, I could've used a streq(get_name(get_parent(np)), "/") test but >>>> this is faster. It's up to you what would work best. >>> >>> Why not just a "parent" key? >>> >> >> Because the property/type prolog code would increase even for properties >> that don't need the parent key. While generating the parent key only for >> constraints that need it keeps the filter size smaller. > > One could argue that category and type are also constraints, but you > don't express them as constraints. I think constraints should be > limited to constraints on the value of the property which are not > generically expressible (like type). I haven't fully thought through > whether there's other cases. > Both type and category constraints work in what I’ve posted, using the eBPF APIs. Taking for instance the enabled status constraint from ‘device-enabled.yaml’ > %YAML 1.1 > --- > device-enabled: > title: Contraint for enabled devices > > class: constraint > virtual: true > > properties: > status: &device-status-enabled > category: optional > type: str > description: Marks device state as enabled > constraint: | > !exists || streq(v, "okay") || streq(v, "ok”) > It generates the following code fragment. > { > uint64_t flags; > const char *v = get_str(np, "status", &flags); > const bool badtype = !!(flags & BADTYPE); > const bool exists = !!(flags & EXISTS); > > if (badtype) > return -BADTYPE_BASE - 100002; > > if (!exists) > goto skip_100002; > > /* for status from device-compatible rule */ > if (!( > !exists || streq(v, "okay") || streq(v, "ok") > )) > return -PROPC_BASE - 100002; > skip_100002: > do { } while(0); /* fix goto that requires a statement */ > > } > Both type checking (BADTYPE) and category (EXISTS) flags are returned and checked. > Another problem is this constraint is not really a constraint for > compatible as you have it, but for the whole binding. > What exactly is the difference in actual use? AFAIKT is our use of bindings and special meaning of the compatible property. > [...] > >>>>> Overall, I think this format is a bit long for boards. We have >>>>> something like ~1000 boards in arch/arm last I checked. I want adding >>>>> a board binding to be very short and easy to review. It's often only a >>>>> 1 line change. The main issue I have is it is just each SoC (or SoC >>>>> family) does things their own way. >>>>> >>>> >>>> Well, this is a full featured example; you could declare this a >>>> 'virtual' (or what ever you want to call it binding) and use: >>>> >>>> board-example-foo: >>>> >>>> inherits: *board-example >>>> >>>> properties: >>>> compatible: ... >>>> >>>> It is not absolutely terse, but it's still YAML. But for what is worth, >>>> those YAML files can be generated using the C preprocessor. You could >>>> define a macro that cuts the churn, albeit you lose the ability to >>>> parse them as normal YAML files. >>> >>> C preprocessor doesn't sound like a great option to me. >>> >>> Perhaps it is by SoC compatible and boards are just an addition to the >>> constraints. That's kind of how things are organized already. >>> >> >> The code base supports multiple constraints so we could have multiple >> constraint properties. How would you like your SoC example to >> work ideally? > > Not sure exactly other than meeting the requirements that I gave. > > Rob Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html