Virtualization difficulty -- phandles

Hi, all!

The product that I work on supports physical device assignment (aka "pass-through") to virtual machines, which means we need to take portions of the physical FDT and include them in the guest FDT. This needs to happen automatically in software.

The problem is phandles, because we cannot identify them in the blob and therefore can't find any dependent devices/nodes that also need to be included in the guest FDT. So the process cannot be fully automated. We can't even advise the user what other devices should be assigned to the VM. The hypervisor runs or bare metal, so having to parse the source DTS files for this is very inconvenient.

Would it be possible to add metadata properties to the binary FDT format, which would identify other property cells that are in fact phandles? It could be a per-node property or a single root node property, up to you guys. DTC would then automatically generate the metadata property along with the phandle property when compiling the DTS.

Quoted 14 lines · Cyril Novikov

Hi, all!

The product that I work on supports physical device assignment (aka "pass-through") to virtual machines, which means we need to take portions of the physical FDT and include them in the guest FDT. This needs to happen automatically in software.

The problem is phandles, because we cannot identify them in the blob and therefore can't find any dependent devices/nodes that also need to be included in the guest FDT. So the process cannot be fully automated. We can't even advise the user what other devices should be assigned to the VM. The hypervisor runs or bare metal, so having to parse the source DTS files for this is very inconvenient.

Quoted 5 lines

Would it be possible to add metadata properties to the binary FDT format, which would identify other property cells that are in fact phandles? It could be a per-node property or a single root node property, up to you guys. DTC would then automatically generate the metadata property along with the phandle property when compiling the DTS.

phandles are already per-node properties, they are not quite different from normal properties actually. If a node is referred to by other nodes, the DTC compiler would add a phandle = <N> (and maybe even linux,phandle = <N> depending on options passed to it) where N is a global integer that keeps being incremented every time a new phandle is generated.

When you strip or create new nodes from a base blob for your virtual machine, either the node is still existing, in which case its phandle property (if existing) is still valid and can still be referenced to, or it is a new node and then you can control how to allocate new phandle integers.

I guess I am just not clear on what problem you are seeing with phandles based on your description of what you want to do?

Quoted 35 lines · Florian Fainelli

Quoted 14 lines · Cyril Novikov

Hi, all!

The product that I work on supports physical device assignment (aka "pass-through") to virtual machines, which means we need to take portions of the physical FDT and include them in the guest FDT. This needs to happen automatically in software.

The problem is phandles, because we cannot identify them in the blob and therefore can't find any dependent devices/nodes that also need to be included in the guest FDT. So the process cannot be fully automated. We can't even advise the user what other devices should be assigned to the VM. The hypervisor runs or bare metal, so having to parse the source DTS files for this is very inconvenient.

Quoted 6 lines

Would it be possible to add metadata properties to the binary FDT format, which would identify other property cells that are in fact phandles? It could be a per-node property or a single root node property, up to you guys. DTC would then automatically generate the metadata property along with the phandle property when compiling the DTS.

phandles are already per-node properties, they are not quite different from normal properties actually. If a node is referred to by other nodes, the DTC compiler would add a phandle = <N> (and maybe even linux,phandle = <N> depending on options passed to it) where N is a global integer that keeps being incremented every time a new phandle is generated.

When you strip or create new nodes from a base blob for your virtual machine, either the node is still existing, in which case its phandle property (if existing) is still valid and can still be referenced to, or it is a new node and then you can control how to allocate new phandle integers.

Quoted 2 lines

I guess I am just not clear on what problem you are seeing with phandles based on your description of what you want to do?

Maybe I should have worded it differently: the problem is with phandle references rather than phandle properties themselves, does it make it more clear?

There is no way to know that a certain aligned 4 byte sequence is a phandle that references another part of the FDT. You can for some standard properties whose format is known not only to the driver, but you can't in general. That makes it impossible to analyze and detect dependencies between different parts of the FDT automatically.

I think this situation is solvable with automatically DTC-generated metadata. I'm also interested if there are other hypervisor vendors that had to deal with this and how big is the demand for a solution. If we are the first one, at least let it register that there's a problem and interest in addressing it.

Quoted 36 lines · Cyril Novikov

Quoted 35 lines · Florian Fainelli

Quoted 14 lines · Cyril Novikov

Hi, all!

The product that I work on supports physical device assignment (aka "pass-through") to virtual machines, which means we need to take portions of the physical FDT and include them in the guest FDT. This needs to happen automatically in software.

The problem is phandles, because we cannot identify them in the blob and therefore can't find any dependent devices/nodes that also need to be included in the guest FDT. So the process cannot be fully automated. We can't even advise the user what other devices should be assigned to the VM. The hypervisor runs or bare metal, so having to parse the source DTS files for this is very inconvenient.

Quoted 6 lines

Would it be possible to add metadata properties to the binary FDT format, which would identify other property cells that are in fact phandles? It could be a per-node property or a single root node property, up to you guys. DTC would then automatically generate the metadata property along with the phandle property when compiling the DTS.

phandles are already per-node properties, they are not quite different from normal properties actually. If a node is referred to by other nodes, the DTC compiler would add a phandle = <N> (and maybe even linux,phandle = <N> depending on options passed to it) where N is a global integer that keeps being incremented every time a new phandle is generated.

When you strip or create new nodes from a base blob for your virtual machine, either the node is still existing, in which case its phandle property (if existing) is still valid and can still be referenced to, or it is a new node and then you can control how to allocate new phandle integers.

Quoted 6 lines

Quoted 2 lines

I guess I am just not clear on what problem you are seeing with phandles based on your description of what you want to do?

Maybe I should have worded it differently: the problem is with phandle references rather than phandle properties themselves, does it make it more clear?

It does, thanks.

Quoted 2 lines

There is no way to know that a certain aligned 4 byte sequence is a phandle that references another part of the FDT.

You can

Quoted 3 lines

for some standard properties whose format is known not only to the driver, but you can't in general. That makes it impossible to analyze and detect dependencies between different parts of the FDT automatically.

I see what you mean now, there is indeed no way to tell whether a property that has an integer is just a normal integer versus an integer corresponding to a phandle.

You could argue that property that specify a phandle should have a name that suggests so, like "phy-handle" but then this stops working with e.g: gpios that are (at least with Linux) specified as e.g: reset-gpios.

In any case, you would have to have some sort of heuristic built into your FDT mangling code that tries to check if a given property is designating a phandle as opposed to having a more robust approach...

Quoted 5 lines

I think this situation is solvable with automatically DTC-generated metadata. I'm also interested if there are other hypervisor vendors that had to deal with this and how big is the demand for a solution. If we are the first one, at least let it register that there's a problem and interest in addressing it.

That would work, I have not heard of a similar problem with the hypervisor folks that I worked with, but AFAIR they were generating their DTS almost from scratch as opposed to mangling/passing through parts of an existing one.

But.. I'm very uneasy about the idea.

The first thing, is that this relies on the dts file containing the &whatever phandle reference syntax wherever there should be a phandle reference. That's normal, but nothing stops a user from simply putting an actual number there, manually assigning that number to another node's phandle and generating a valid dtb that way.

More importantly, it won't detect phandles if the tree comes from a source other than a dts - for example if you flatten /proc/device-tree with -I fs, or have code which flattens a tree presented by real Open Firmware it won't have that phandle metadata, just integer values.

Now those might not be something that happens in your use case, but I'm pretty concerned that if I added this functionality, people are going to forget that properties are all, fundamentally, bytestrings and get surprised when tools don't magically know where the phandles are in some cases.

That said, I'm open to persuasion if a good enough case is made for this functionality.

But then, as Mark Rutland says in his other replies, locating phandles is far from the only problem in trying to passthrough random DT nodes to a guest.

Quoted 8 lines · David Gibson

But.. I'm very uneasy about the idea.

The first thing, is that this relies on the dts file containing the &whatever phandle reference syntax wherever there should be a phandle reference. That's normal, but nothing stops a user from simply putting an actual number there, manually assigning that number to another node's phandle and generating a valid dtb that way.

Quoted 4 lines

More importantly, it won't detect phandles if the tree comes from a source other than a dts - for example if you flatten /proc/device-tree with -I fs, or have code which flattens a tree presented by real Open Firmware it won't have that phandle metadata, just integer values.

Yeah, so the metadata does not contain full information. That's fine because it's still better than nothing.

Quoted 8 lines

Now those might not be something that happens in your use case, but I'm pretty concerned that if I added this functionality, people are going to forget that properties are all, fundamentally, bytestrings and get surprised when tools don't magically know where the phandles are in some cases.

That said, I'm open to persuasion if a good enough case is made for this functionality.

So, what we need fundamentally is a way to detect dependencies in the devicetree. Mark is arguing that we cannot *automate* the assignment of dependent devices to the same VM. However, just detecting that there is a missing dependency goes a long way. What we have now is guest drivers failing in various manners, but what they cannot do is point the user to the specific device that they need, because they don't have the host FDT in their VM.

For example, a driver may print something like

[    2.035071] xuartps ff000000.serial: pclk clock not found.

To resolve this, the user needs to be proficient with the device tree. Of course, "pclk" is not a node name in the host FDT. However, if configuration software could detect this at the time the set of the VM's assigned physical devices was created, it could give the user the name of the device that *probably* also needs to be assigned to the same VM. The user wouldn't need to learn anything about devicetree, they would just need to do a few mouse clicks to assign the dependent device. And now it's the software's concern if that assignment is not trivial, as Mark warns us.

To summarize, full automation is unfeasible and it is understood, but just the dependency information is very helpful too.

Quoted 3 lines

But then, as Mark Rutland says in his other replies, locating phandles is far from the only problem in trying to passthrough random DT nodes to a guest.

Quoted 5 lines · Florian Fainelli

Quoted 4 lines · Cyril Novikov

Quoted 3 lines · Florian Fainelli

Quoted 2 lines · Cyril Novikov

Hi, all!

The
phandle fixup information goes into the special __local_fixups__ and __fixups__ nodes (which have gratuitiously different format, but that's a rant for elsewhere).

Well, I don't want to invent a new encoding if we can possibly avoid it. The current encoding used for overlay generation looks like this

/ {
	target: node@0 {
	};
	node@1 {
		ref = <&target>;
	};
	__local_fixups__ = {
		node@1 {
			ref = <0>;
		};
	};
};

Basically, __local_fixups__ has a subtree which paralells the main tree. Each property found under __local_fixups__ is a list of offsets at which phandle references appear in the corresponding property in the main tree.

Hi David,

(Adding Pantelis and Tom, since I'm going somewhat off-topic from the original thread, and they are impacted by what I am asking.)

Quoted 6 lines · David Gibson

Quoted 5 lines · Florian Fainelli

Quoted 4 lines · Cyril Novikov

Quoted 3 lines · Florian Fainelli

Quoted 2 lines · Cyril Novikov

Hi, all!

< snip >

Quoted 4 lines

The
phandle fixup information goes into the special __local_fixups__ and __fixups__ nodes (which have gratuitiously different format, but that's a rant for elsewhere).

< snip >

And in another email, David describes the __local_fixups__ format nicely, so I'll just copy that here instead of re-inventing it:

Quoted 16 lines

Well, I don't want to invent a new encoding if we can possibly avoid it. The current encoding used for overlay generation looks like this

/ {
	target: node@0 {
	};
	node@1 {
		ref = <&target>;
	};
	__local_fixups__ = {
		node@1 {
			ref = <0>;
		};
	};
};

Quoted 4 lines

Basically, __local_fixups__ has a subtree which paralells the main tree. Each property found under __local_fixups__ is a list of offsets at which phandle references appear in the corresponding property in the main tree.

I share your desire to rant about the different formats between __local_fixups__ and __fixups__. But I have not come up with an alternate format for __local_fixups__ that makes me happy. The best format that I have come up with so far would be:

/ {
	target: node@0 {
	};
	node@1 {
		ref = <&target>;
                ref2 = <&target 42 &target_2>;
	};
        target_2: node@2 {
        };
	__local_fixups__ = {
		x1 = <"node@1/ref" 0>;
                x2 = <"node@1/ref2" 0 8>;
		};
	};
};

x1 and x2 are abitrary property names.
The format of each __local_fixups__ property is
- path of property referencing a phandle
- list of offsets of the phandle in the property

As another alternative, Grant was thinking about adding a new block to the FDT format to contain the phandle information. That would remove the need to come up with a convoluted dts syntax, but adds in the problem of bootloaders corrupting the new block if they were not aware of it. He had thoughts about versioning and checksums to detect the corruption it if did occur.

If we were starting from scratch, do you have any other approach that might be fruitful? It seems like maybe I am missing something that requires thinking outside the box.

Quoted 47 lines · Frank Rowand

Hi David,

(Adding Pantelis and Tom, since I'm going somewhat off-topic from the original thread, and they are impacted by what I am asking.)

Quoted 6 lines · David Gibson

Quoted 5 lines · Florian Fainelli

Quoted 4 lines · Cyril Novikov

Quoted 3 lines · Florian Fainelli

Quoted 2 lines · Cyril Novikov

Hi, all!

< snip >

Quoted 4 lines

The
phandle fixup information goes into the special __local_fixups__ and __fixups__ nodes (which have gratuitiously different format, but that's a rant for elsewhere).

< snip >

And in another email, David describes the __local_fixups__ format nicely, so I'll just copy that here instead of re-inventing it:

Quoted 20 lines

Well, I don't want to invent a new encoding if we can possibly avoid it. The current encoding used for overlay generation looks like this

/ {
	target: node@0 {
	};
	node@1 {
		ref = <&target>;
	};
	__local_fixups__ = {
		node@1 {
			ref = <0>;
		};
	};
};

Basically, __local_fixups__ has a subtree which paralells the main tree. Each property found under __local_fixups__ is a list of offsets at which phandle references appear in the corresponding property in the main tree.

Quoted 4 lines

I share your desire to rant about the different formats between __local_fixups__ and __fixups__. But I have not come up with an alternate format for __local_fixups__ that makes me happy. The best format that I have come up with so far would be:

Well to fix it minimally, I'd go the other way - make __fixups__ look like __local_fixups__ but augmented with labels. Strings that need parsing aren't a normal thing in the DT.

Quoted 31 lines

/ {
	target: node@0 {
	};
	node@1 {
		ref = <&target>;
                ref2 = <&target 42 &target_2>;
	};
        target_2: node@2 {
        };
	__local_fixups__ = {
		x1 = <"node@1/ref" 0>;
                x2 = <"node@1/ref2" 0 8>;
		};
	};
};

x1 and x2 are abitrary property names.
The format of each __local_fixups__ property is
- path of property referencing a phandle
- list of offsets of the phandle in the property

As another alternative, Grant was thinking about adding a new block to the FDT format to contain the phandle information. That would remove the need to come up with a convoluted dts syntax, but adds in the problem of bootloaders corrupting the new block if they were not aware of it. He had thoughts about versioning and checksums to detect the corruption it if did occur.

Quoted 4 lines

If we were starting from scratch, do you have any other approach that might be fruitful? It seems like maybe I am missing something that requires thinking outside the box.

I thought about this the other day a bit. If going from scratch, I think the way to do it would be to add a new FDT_REF tag to the structure block stream. After the FDT_PROP tag and its contents, you'd have an arbitrary number of FDT_REF tags, each giving an offset in the preceding property and a label to fix it up to match. Not sure if you'd want separate FDT_REF and FDT_LOCAL_REF or just use an empty label to describe a local ref.

This would also allow for extension to say FDT_PATH_REF to insert paths rather than phandles (i.e. a runtime equivalent of prop = &foo; rather than prop = < &foo >;).

For encoding the fragments of an overlay, I'd suggest giving them simply as separate subtrees in the structure block, all before the FDT_END tag. At the moment there has to be only a single subtree before the FDT_END, and the top-level FDT_BEGIN is expected to have an empty name. We can extend that to overlays by allowing multiple subtrees, and making the top-level "name" the target label instead.

Incidentally, I'd take "label" in all the above to be represented as an old-style OF path. That is, either an absolute path /foo/bar/baz, or a path relative to an alias, alias/foo/bar/baz. That means we can just use the existing defined /aliases, rather than re-inventing it as __symbols__.

Quoted 48 lines · David Gibson

Quoted 47 lines · Frank Rowand

Hi David,

(Adding Pantelis and Tom, since I'm going somewhat off-topic from the original thread, and they are impacted by what I am asking.)

Quoted 6 lines · David Gibson

Quoted 5 lines · Florian Fainelli

Quoted 4 lines · Cyril Novikov

Quoted 3 lines · Florian Fainelli

Quoted 2 lines · Cyril Novikov

Hi, all!

< snip >

Quoted 4 lines

The
phandle fixup information goes into the special __local_fixups__ and __fixups__ nodes (which have gratuitiously different format, but that's a rant for elsewhere).

< snip >

And in another email, David describes the __local_fixups__ format nicely, so I'll just copy that here instead of re-inventing it:

Quoted 20 lines

Well, I don't want to invent a new encoding if we can possibly avoid it. The current encoding used for overlay generation looks like this

/ {
	target: node@0 {
	};
	node@1 {
		ref = <&target>;
	};
	__local_fixups__ = {
		node@1 {
			ref = <0>;
		};
	};
};

Basically, __local_fixups__ has a subtree which paralells the main tree. Each property found under __local_fixups__ is a list of offsets at which phandle references appear in the corresponding property in the main tree.

Quoted 8 lines

Quoted 4 lines

I share your desire to rant about the different formats between __local_fixups__ and __fixups__. But I have not come up with an alternate format for __local_fixups__ that makes me happy. The best format that I have come up with so far would be:

Well to fix it minimally, I'd go the other way - make __fixups__ look like __local_fixups__ but augmented with labels. Strings that need parsing aren't a normal thing in the DT.

On the string parsing issue, I agree that string parsing is not normal in the DT. If changing format in other ways, I would maybe also change the __fixups__ format so that (for an example with two tuples), instead of

   "A:B:C", "D:E:F"

the format would be

   "A", "B", <C>, "D", "E", <F>.

Or a more concrete example, change:

   i2c1 = "/fragment@1:target:0";

to

   i2c1 = "/fragment@1", "target", <0>;

or (to bikeshed) even change the order to:

   i2c1 = <0>, "/fragment@1", "target">;

This may look a little awkward in source form, but in my version of what the world should look like, this would not be hand coded in a DTS source file, but instead created by dtc in a DTB. Of course it could still be viewed as DTS format by de-compiling the DTB.

I admit this may be a really bad idea from a human usability standpoint, because the source fragment (for example):

        __fixups__ {
                i2c1 = <0>, "/fragment@1", "target";
                i2c2 = <8>, "/fragment@1", "target";
                i2c3 = "/fragment@1", "target", <0>;
                i2c4 = "/fragment@1", "target", <8>;
        };

decompiles (via 'dtc -O dts') somewhat cryptically as:

	__fixups__ {
		i2c1 = "", "", "", "", "/fragment@1", "target";
		i2c2 = "", "", "", "\b/fragment@1", "target";
		i2c3 = "/fragment@1", "target", "", "", "", "";
		i2c4 = [2f 66 72 61 67 6d 65 6e 74 40 31 00 74 61 72 67 65 74 00 00 00 00 08];
	};

-Frank

Quoted 61 lines

Quoted 31 lines

/ {
	target: node@0 {
	};
	node@1 {
		ref = <&target>;
                ref2 = <&target 42 &target_2>;
	};
        target_2: node@2 {
        };
	__local_fixups__ = {
		x1 = <"node@1/ref" 0>;
                x2 = <"node@1/ref2" 0 8>;
		};
	};
};

x1 and x2 are abitrary property names.
The format of each __local_fixups__ property is
- path of property referencing a phandle
- list of offsets of the phandle in the property

As another alternative, Grant was thinking about adding a new block to the FDT format to contain the phandle information. That would remove the need to come up with a convoluted dts syntax, but adds in the problem of bootloaders corrupting the new block if they were not aware of it. He had thoughts about versioning and checksums to detect the corruption it if did occur.

Quoted 4 lines

If we were starting from scratch, do you have any other approach that might be fruitful? It seems like maybe I am missing something that requires thinking outside the box.

I thought about this the other day a bit. If going from scratch, I think the way to do it would be to add a new FDT_REF tag to the structure block stream. After the FDT_PROP tag and its contents, you'd have an arbitrary number of FDT_REF tags, each giving an offset in the preceding property and a label to fix it up to match. Not sure if you'd want separate FDT_REF and FDT_LOCAL_REF or just use an empty label to describe a local ref.

This would also allow for extension to say FDT_PATH_REF to insert paths rather than phandles (i.e. a runtime equivalent of prop = &foo; rather than prop = < &foo >;).

For encoding the fragments of an overlay, I'd suggest giving them simply as separate subtrees in the structure block, all before the FDT_END tag. At the moment there has to be only a single subtree before the FDT_END, and the top-level FDT_BEGIN is expected to have an empty name. We can extend that to overlays by allowing multiple subtrees, and making the top-level "name" the target label instead.

Incidentally, I'd take "label" in all the above to be represented as an old-style OF path. That is, either an absolute path /foo/bar/baz, or a path relative to an alias, alias/foo/bar/baz. That means we can just use the existing defined /aliases, rather than re-inventing it as __symbols__.

Quoted 48 lines · David Gibson

Quoted 47 lines · Frank Rowand

Hi David,

(Adding Pantelis and Tom, since I'm going somewhat off-topic from the original thread, and they are impacted by what I am asking.)

Quoted 6 lines · David Gibson

Quoted 5 lines · Florian Fainelli

Quoted 4 lines · Cyril Novikov

Quoted 3 lines · Florian Fainelli

Quoted 2 lines · Cyril Novikov

Hi, all!

< snip >

Quoted 4 lines

The
phandle fixup information goes into the special __local_fixups__ and __fixups__ nodes (which have gratuitiously different format, but that's a rant for elsewhere).

< snip >

And in another email, David describes the __local_fixups__ format nicely, so I'll just copy that here instead of re-inventing it:

Quoted 20 lines

Well, I don't want to invent a new encoding if we can possibly avoid it. The current encoding used for overlay generation looks like this

/ {
	target: node@0 {
	};
	node@1 {
		ref = <&target>;
	};
	__local_fixups__ = {
		node@1 {
			ref = <0>;
		};
	};
};

Basically, __local_fixups__ has a subtree which paralells the main tree. Each property found under __local_fixups__ is a list of offsets at which phandle references appear in the corresponding property in the main tree.

Quoted 8 lines

Quoted 4 lines

I share your desire to rant about the different formats between __local_fixups__ and __fixups__. But I have not come up with an alternate format for __local_fixups__ that makes me happy. The best format that I have come up with so far would be:

Well to fix it minimally, I'd go the other way - make __fixups__ look like __local_fixups__ but augmented with labels. Strings that need parsing aren't a normal thing in the DT.

On the string parsing issue, I agree that string parsing is not normal in the DT. If changing format in other ways, I would maybe also change the __fixups__ format so that (for an example with two tuples), instead of

   "A:B:C", "D:E:F"

the format would be

   "A", "B", <C>, "D", "E", <F>.

Or a more concrete example, change:

   i2c1 = "/fragment@1:target:0";

to

   i2c1 = "/fragment@1", "target", <0>;

or (to bikeshed) even change the order to:

   i2c1 = <0>, "/fragment@1", "target">;

This may look a little awkward in source form, but in my version of what the world should look like, this would not be hand coded in a DTS source file, but instead created by dtc in a DTB. Of course it could still be viewed as DTS format by de-compiling the DTB.

I admit this may be a really bad idea from a human usability standpoint, because the source fragment (for example):

        __fixups__ {
                i2c1 = <0>, "/fragment@1", "target";
                i2c2 = <8>, "/fragment@1", "target";
                i2c3 = "/fragment@1", "target", <0>;
                i2c4 = "/fragment@1", "target", <8>;
        };

decompiles (via 'dtc -O dts') somewhat cryptically as:

	__fixups__ {
		i2c1 = "", "", "", "", "/fragment@1", "target";
		i2c2 = "", "", "", "\b/fragment@1", "target";
		i2c3 = "/fragment@1", "target", "", "", "", "";
		i2c4 = [2f 66 72 61 67 6d 65 6e 74 40 31 00 74 61 72 67 65 74 00 00 00 00 08];
	};

-Frank

Quoted 61 lines

Quoted 31 lines

/ {
	target: node@0 {
	};
	node@1 {
		ref = <&target>;
                ref2 = <&target 42 &target_2>;
	};
        target_2: node@2 {
        };
	__local_fixups__ = {
		x1 = <"node@1/ref" 0>;
                x2 = <"node@1/ref2" 0 8>;
		};
	};
};

x1 and x2 are abitrary property names.
The format of each __local_fixups__ property is
- path of property referencing a phandle
- list of offsets of the phandle in the property

As another alternative, Grant was thinking about adding a new block to the FDT format to contain the phandle information. That would remove the need to come up with a convoluted dts syntax, but adds in the problem of bootloaders corrupting the new block if they were not aware of it. He had thoughts about versioning and checksums to detect the corruption it if did occur.

Quoted 4 lines

If we were starting from scratch, do you have any other approach that might be fruitful? It seems like maybe I am missing something that requires thinking outside the box.

I thought about this the other day a bit. If going from scratch, I think the way to do it would be to add a new FDT_REF tag to the structure block stream. After the FDT_PROP tag and its contents, you'd have an arbitrary number of FDT_REF tags, each giving an offset in the preceding property and a label to fix it up to match. Not sure if you'd want separate FDT_REF and FDT_LOCAL_REF or just use an empty label to describe a local ref.

This would also allow for extension to say FDT_PATH_REF to insert paths rather than phandles (i.e. a runtime equivalent of prop = &foo; rather than prop = < &foo >;).

For encoding the fragments of an overlay, I'd suggest giving them simply as separate subtrees in the structure block, all before the FDT_END tag. At the moment there has to be only a single subtree before the FDT_END, and the top-level FDT_BEGIN is expected to have an empty name. We can extend that to overlays by allowing multiple subtrees, and making the top-level "name" the target label instead.

Incidentally, I'd take "label" in all the above to be represented as an old-style OF path. That is, either an absolute path /foo/bar/baz, or a path relative to an alias, alias/foo/bar/baz. That means we can just use the existing defined /aliases, rather than re-inventing it as __symbols__.

Hi, all!

The product that I work on supports physical device assignment (aka "pass-through") to virtual machines, which means we need to take portions of the physical FDT and include them in the guest FDT. This needs to happen automatically in software.

The problem is phandles, because we cannot identify them in the blob and therefore can't find any dependent devices/nodes that also need to be included in the guest FDT. So the process cannot be fully automated. We can't even advise the user what other devices should be assigned to the VM. The hypervisor runs or bare metal, so having to parse the source DTS files for this is very inconvenient.

Would it be possible to add metadata properties to the binary FDT format, which would identify other property cells that are in fact phandles? It could be a per-node property or a single root node property, up to you guys. DTC would then automatically generate the metadata property along with the phandle property when compiling the DTS.

Quoted 6 lines

Would it be possible to add metadata properties to the binary FDT format, which would identify other property cells that are in fact phandles? It could be a per-node property or a single root node property, up to you guys. DTC would then automatically generate the metadata property along with the phandle property when compiling the DTS.

Unfortunately, even ignoring the above, this metadata isn't likely to be reliable, as after compilation, other agents (e.g. the bootloader) may modify the FDT, without updating the metadata that they are not aware of.

Quoted 11 lines · Mark Rutland

Quoted 6 lines

Would it be possible to add metadata properties to the binary FDT format, which would identify other property cells that are in fact phandles? It could be a per-node property or a single root node property, up to you guys. DTC would then automatically generate the metadata property along with the phandle property when compiling the DTS.

Unfortunately, even ignoring the above, this metadata isn't likely to be reliable, as after compilation, other agents (e.g. the bootloader) may modify the FDT, without updating the metadata that they are not aware of.

Well, it depends on the design of said metadata. For example, imagine a node property named ".dependencies", which simply lists all phandles referenced by other properties in the same node. I would be very surprised if the bootloader does something that breaks that. It would require a major intervention into the FDT.

Even if they do, like I said in my reply to David, we don't strive to achieve a full automation, so it's probably tolerable. Oh well, we've missed some dependencies. It's still better than what we have now.

Well, I don't want to invent a new encoding if we can possibly avoid it. The current encoding used for overlay generation looks like this

/ {
	target: node@0 {
	};
	node@1 {
		ref = <&target>;
	};
	__local_fixups__ = {
		node@1 {
			ref = <0>;
		};
	};
};

Basically, __local_fixups__ has a subtree which paralells the main tree. Each property found under __local_fixups__ is a list of offsets at which phandle references appear in the corresponding property in the main tree.

That seems like it would survive most likely bootloader transformations as well, I think.

Quoted 3 lines

Even if they do, like I said in my reply to David, we don't strive to achieve a full automation, so it's probably tolerable. Oh well, we've missed some dependencies. It's still better than what we have now.

Ok. I'm tentatively convinced that it's worth adding a switch to dtc to generate __local_fixups__even for a non-plugin source.

Next step is for someone to propose a concrete patch.

Quoted 28 lines · David Gibson

Well, I don't want to invent a new encoding if we can possibly avoid it. The current encoding used for overlay generation looks like this

/ {
	target: node@0 {
	};
	node@1 {
		ref = <&target>;
	};
	__local_fixups__ = {
		node@1 {
			ref = <0>;
		};
	};
};

Basically, __local_fixups__ has a subtree which paralells the main tree. Each property found under __local_fixups__ is a list of offsets at which phandle references appear in the corresponding property in the main tree.

That seems like it would survive most likely bootloader transformations as well, I think.

Quoted 3 lines

Even if they do, like I said in my reply to David, we don't strive to achieve a full automation, so it's probably tolerable. Oh well, we've missed some dependencies. It's still better than what we have now.

Quoted 4 lines

Ok. I'm tentatively convinced that it's worth adding a switch to dtc to generate __local_fixups__even for a non-plugin source.

Next step is for someone to propose a concrete patch.

Do you mean a patch to the DTC utility or a patch to the Devicetree Specification?

I suppose we want this specced, so that alternative incompatible implementations do not appear?

The engineer looking at it told
me it's only a few lines change for the DTC.

Hi, all!

The product that I work on supports physical device assignment (aka "pass-through") to virtual machines, which means we need to take portions of the physical FDT and include them in the guest FDT. This needs to happen automatically in software.

The problem is phandles, because we cannot identify them in the blob and therefore can't find any dependent devices/nodes that also need to be included in the guest FDT. So the process cannot be fully automated. We can't even advise the user what other devices should be assigned to the VM. The hypervisor runs or bare metal, so having to parse the source DTS files for this is very inconvenient.

Would it be possible to add metadata properties to the binary FDT format, which would identify other property cells that are in fact phandles? It could be a per-node property or a single root node property, up to you guys. DTC would then automatically generate the metadata property along with the phandle property when compiling the DTS.

-- Cyril

-- To unsubscribe from this list: send the line "unsubscribe devicetree-spec" in the body of a message to [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html .

Hi Cyril,

Quoted 6 lines · Cyril Novikov

Hi, all!

The product that I work on supports physical device assignment (aka "pass-through") to virtual machines, which means we need to take portions of the physical FDT and include them in the guest FDT. This needs to happen automatically in software.

Is this for Linux, or some other OS?

If Linux, do you get the FDT from /sys/firmware/fdt or some other method?