[PATCH v2] powerpc/pseries/mobility: ignore ibm, platform-facilities updates

[PATCH v2] powerpc/pseries/mobility: ignore ibm, platform-facilities updates

[Nathan Lynch]

On VMs with NX encryption, compression, and/or RNG offload, these
capabilities are described by nodes in the ibm,platform-facilities device
tree hierarchy:

  $ tree -d /sys/firmware/devicetree/base/ibm,platform-facilities/
  /sys/firmware/devicetree/base/ibm,platform-facilities/
  ├── ibm,compression-v1
  ├── ibm,random-v1
  └── ibm,sym-encryption-v1

  3 directories

The acceleration functions that these nodes describe are not disrupted by
live migration, not even temporarily.

But the post-migration ibm,update-nodes sequence firmware always sends
"delete" messages for this hierarchy, followed by an "add" directive to
reconstruct it via ibm,configure-connector (log with debugging statements
enabled in mobility.c):

  mobility: removing node /ibm,platform-facilities/ibm,random-v1:4294967285
  mobility: removing node /ibm,platform-facilities/ibm,compression-v1:4294967284
  mobility: removing node /ibm,platform-facilities/ibm,sym-encryption-v1:4294967283
  mobility: removing node /ibm,platform-facilities:4294967286
  ...
  mobility: added node /ibm,platform-facilities:4294967286

Note we receive a single "add" message for the entire hierarchy, and what
we receive from the ibm,configure-connector sequence is the top-level
platform-facilities node along with its three children. The debug message
simply reports the parent node and not the whole subtree.

Also, significantly, the nodes added are almost completely equivalent to
the ones removed; even phandles are unchanged. ibm,shared-interrupt-pool in
the leaf nodes is the only property I've observed to differ, and Linux does
not use that. So in practice, the sum of update messages Linux receives for
this hierarchy is equivalent to minor property updates.

We succeed in removing the original hierarchy from the device tree. But the
vio bus code is ignorant of this, and does not unbind or relinquish its
references. The leaf nodes, still reachable through sysfs, of course still
refer to the now-freed ibm,platform-facilities parent node, which makes
use-after-free possible:

  refcount_t: addition on 0; use-after-free.
  WARNING: CPU: 3 PID: 1706 at lib/refcount.c:25 refcount_warn_saturate+0x164/0x1f0
  refcount_warn_saturate+0x160/0x1f0 (unreliable)
  kobject_get+0xf0/0x100
  of_node_get+0x30/0x50
  of_get_parent+0x50/0xb0
  of_fwnode_get_parent+0x54/0x90
  fwnode_count_parents+0x50/0x150
  fwnode_full_name_string+0x30/0x110
  device_node_string+0x49c/0x790
  vsnprintf+0x1c0/0x4c0
  sprintf+0x44/0x60
  devspec_show+0x34/0x50
  dev_attr_show+0x40/0xa0
  sysfs_kf_seq_show+0xbc/0x200
  kernfs_seq_show+0x44/0x60
  seq_read_iter+0x2a4/0x740
  kernfs_fop_read_iter+0x254/0x2e0
  new_sync_read+0x120/0x190
  vfs_read+0x1d0/0x240

Moreover, the "new" replacement subtree is not correctly added to the
device tree, resulting in ibm,platform-facilities parent node without the
appropriate leaf nodes, and broken symlinks in the sysfs device hierarchy:

  $ tree -d /sys/firmware/devicetree/base/ibm,platform-facilities/
  /sys/firmware/devicetree/base/ibm,platform-facilities/

  0 directories

  $ cd /sys/devices/vio ; find . -xtype l -exec file {} +
  ./ibm,sym-encryption-v1/of_node: broken symbolic link to
    ../../../firmware/devicetree/base/ibm,platform-facilities/ibm,sym-encryption-v1
  ./ibm,random-v1/of_node:         broken symbolic link to
    ../../../firmware/devicetree/base/ibm,platform-facilities/ibm,random-v1
  ./ibm,compression-v1/of_node:    broken symbolic link to
    ../../../firmware/devicetree/base/ibm,platform-facilities/ibm,compression-v1

This is because add_dt_node() -> dlpar_attach_node() attaches only the
parent node returned from configure-connector, ignoring any children. This
should be corrected for the general case, but fixing that won't help with
the stale OF node references, which is the more urgent problem.

One way to address that would be to make the drivers respond to node
removal notifications, so that node references can be dropped
appropriately. But this would likely force the drivers to disrupt active
clients for no useful purpose: equivalent nodes are immediately re-added.
And recall that the acceleration capabilities described by the nodes remain
available throughout the whole process.

The solution I believe to be robust for this situation is to convert
remove+add of a node with an unchanged phandle to an update of the node's
properties in the Linux device tree structure. That would involve changing
and adding a fair amount of code, and may take several iterations to land.

Until that can be realized we have a confirmed use-after-free and the
possibility of memory corruption. So add a limited workaround that
discriminates on the node type, ignoring adds and removes. This should be
amenable to backporting in the meantime.

Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
Fixes: 410bccf97881 ("powerpc/pseries: Partition migration in the kernel")
Cc: stable@vger.kernel.org
---

Notes:
    Changes since v1:
    
    * Clarify that the vio bus code maintains references to removed nodes, per
      Tyrel.

 arch/powerpc/platforms/pseries/mobility.c | 34 +++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index e83e0891272d..210a37a065fb 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -63,6 +63,27 @@ static int mobility_rtas_call(int token, char *buf, s32 scope)
 
 static int delete_dt_node(struct device_node *dn)
 {
+	struct device_node *pdn;
+	bool is_platfac;
+
+	pdn = of_get_parent(dn);
+	is_platfac = of_node_is_type(dn, "ibm,platform-facilities") ||
+		     of_node_is_type(pdn, "ibm,platform-facilities");
+	of_node_put(pdn);
+
+	/*
+	 * The drivers that bind to nodes in the platform-facilities
+	 * hierarchy don't support node removal, and the removal directive
+	 * from firmware is always followed by an add of an equivalent
+	 * node. The capability (e.g. RNG, encryption, compression)
+	 * represented by the node is never interrupted by the migration.
+	 * So ignore changes to this part of the tree.
+	 */
+	if (is_platfac) {
+		pr_notice("ignoring remove operation for %pOFfp\n", dn);
+		return 0;
+	}
+
 	pr_debug("removing node %pOFfp\n", dn);
 	dlpar_detach_node(dn);
 	return 0;
@@ -222,6 +243,19 @@ static int add_dt_node(struct device_node *parent_dn, __be32 drc_index)
 	if (!dn)
 		return -ENOENT;
 
+	/*
+	 * Since delete_dt_node() ignores this node type, this is the
+	 * necessary counterpart. We also know that a platform-facilities
+	 * node returned from dlpar_configure_connector() has children
+	 * attached, and dlpar_attach_node() only adds the parent, leaking
+	 * the children. So ignore these on the add side for now.
+	 */
+	if (of_node_is_type(dn, "ibm,platform-facilities")) {
+		pr_notice("ignoring add operation for %pOF\n", dn);
+		dlpar_free_cc_nodes(dn);
+		return 0;
+	}
+
 	rc = dlpar_attach_node(dn, parent_dn);
 	if (rc)
 		dlpar_free_cc_nodes(dn);
-- 
2.31.1

Re: [PATCH v2] powerpc/pseries/mobility: ignore ibm, platform-facilities updates

[Daniel Axtens]

Hi Nathan,

Thanks for the detailed explanation.

I've not really worked with the partition migration code before I was
able to follow your logic.

> On VMs with NX encryption, compression, and/or RNG offload, these
> capabilities are described by nodes in the ibm,platform-facilities device
> tree hierarchy:
>
>   $ tree -d /sys/firmware/devicetree/base/ibm,platform-facilities/
>   /sys/firmware/devicetree/base/ibm,platform-facilities/
>   ├── ibm,compression-v1
>   ├── ibm,random-v1
>   └── ibm,sym-encryption-v1
>
>   3 directories
>
> The acceleration functions that these nodes describe are not disrupted by
> live migration, not even temporarily.
>
> But the post-migration ibm,update-nodes sequence firmware always sends
> "delete" messages for this hierarchy, followed by an "add" directive to
> reconstruct it via ibm,configure-connector (log with debugging statements
> enabled in mobility.c):
>
>   mobility: removing node /ibm,platform-facilities/ibm,random-v1:4294967285
>   mobility: removing node /ibm,platform-facilities/ibm,compression-v1:4294967284
>   mobility: removing node /ibm,platform-facilities/ibm,sym-encryption-v1:4294967283
>   mobility: removing node /ibm,platform-facilities:4294967286
>   ...
>   mobility: added node /ibm,platform-facilities:4294967286
>
> Note we receive a single "add" message for the entire hierarchy, and what
> we receive from the ibm,configure-connector sequence is the top-level
> platform-facilities node along with its three children. The debug message
> simply reports the parent node and not the whole subtree.

If I understand correctly, (and again, this is not my area at all!) we
still have to go out to the firmware and call the
ibm,configure-connector sequence in order to figure out that the node
we're supposed to add is the ibm,platform-facilites node, right? We
can't save enough information at delete time to avoid the trip out to
firmware?

> Also, significantly, the nodes added are almost completely equivalent to
> the ones removed; even phandles are unchanged. ibm,shared-interrupt-pool in
> the leaf nodes is the only property I've observed to differ, and Linux does
> not use that. So in practice, the sum of update messages Linux receives for
> this hierarchy is equivalent to minor property updates.
>
> We succeed in removing the original hierarchy from the device tree. But the
> vio bus code is ignorant of this, and does not unbind or relinquish its
> references. The leaf nodes, still reachable through sysfs, of course still
> refer to the now-freed ibm,platform-facilities parent node, which makes
> use-after-free possible:
>
>   refcount_t: addition on 0; use-after-free.
>   WARNING: CPU: 3 PID: 1706 at lib/refcount.c:25 refcount_warn_saturate+0x164/0x1f0
>   refcount_warn_saturate+0x160/0x1f0 (unreliable)
>   kobject_get+0xf0/0x100
>   of_node_get+0x30/0x50
>   of_get_parent+0x50/0xb0
>   of_fwnode_get_parent+0x54/0x90
>   fwnode_count_parents+0x50/0x150
>   fwnode_full_name_string+0x30/0x110
>   device_node_string+0x49c/0x790
>   vsnprintf+0x1c0/0x4c0
>   sprintf+0x44/0x60
>   devspec_show+0x34/0x50
>   dev_attr_show+0x40/0xa0
>   sysfs_kf_seq_show+0xbc/0x200
>   kernfs_seq_show+0x44/0x60
>   seq_read_iter+0x2a4/0x740
>   kernfs_fop_read_iter+0x254/0x2e0
>   new_sync_read+0x120/0x190
>   vfs_read+0x1d0/0x240
>
> Moreover, the "new" replacement subtree is not correctly added to the
> device tree, resulting in ibm,platform-facilities parent node without the
> appropriate leaf nodes, and broken symlinks in the sysfs device hierarchy:
>
>   $ tree -d /sys/firmware/devicetree/base/ibm,platform-facilities/
>   /sys/firmware/devicetree/base/ibm,platform-facilities/
>
>   0 directories
>
>   $ cd /sys/devices/vio ; find . -xtype l -exec file {} +
>   ./ibm,sym-encryption-v1/of_node: broken symbolic link to
>     ../../../firmware/devicetree/base/ibm,platform-facilities/ibm,sym-encryption-v1
>   ./ibm,random-v1/of_node:         broken symbolic link to
>     ../../../firmware/devicetree/base/ibm,platform-facilities/ibm,random-v1
>   ./ibm,compression-v1/of_node:    broken symbolic link to
>     ../../../firmware/devicetree/base/ibm,platform-facilities/ibm,compression-v1
>
> This is because add_dt_node() -> dlpar_attach_node() attaches only the
> parent node returned from configure-connector, ignoring any children. This
> should be corrected for the general case, but fixing that won't help with
> the stale OF node references, which is the more urgent problem.
>
> One way to address that would be to make the drivers respond to node
> removal notifications, so that node references can be dropped
> appropriately. But this would likely force the drivers to disrupt active
> clients for no useful purpose: equivalent nodes are immediately re-added.
> And recall that the acceleration capabilities described by the nodes remain
> available throughout the whole process.
>
> The solution I believe to be robust for this situation is to convert
> remove+add of a node with an unchanged phandle to an update of the node's
> properties in the Linux device tree structure. That would involve changing
> and adding a fair amount of code, and may take several iterations to land.
>
> Until that can be realized we have a confirmed use-after-free and the
> possibility of memory corruption. So add a limited workaround that
> discriminates on the node type, ignoring adds and removes. This should be
> amenable to backporting in the meantime.

Yeah it's an unpleasant situation to find ourselves in. It's a bit icky
but as I think you said in a previous email, at least this isn't worse:
in the common case it should now succeed and and if properties change
significantly it will still fail.

My one question (from more of a security point of view) is:
 1) Say you start using the facilities with a particular set of
    parameters.

 2) Say you then get migrated and the parameters change.

 3) If you keep using the platform facilities as if the original
    properties are still valid, can this cause any Interesting,
    unexpected or otherwise Bad consequences? Are we going to end up
    (for example) scribbling over random memory somehow?

Apart from that, the code seems to do what it says, it seems to solve a
real problem, the error and memory handling makes sense, you _put the DT
nodes that you _get, the comments are helpful and descriptive, and it
passes the automated tests on patchwork/snowpatch.

Kind regards,
Daniel

> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
> Fixes: 410bccf97881 ("powerpc/pseries: Partition migration in the kernel")
> Cc: stable@vger.kernel.org
> ---
>
> Notes:
>     Changes since v1:
>     
>     * Clarify that the vio bus code maintains references to removed nodes, per
>       Tyrel.
>
>  arch/powerpc/platforms/pseries/mobility.c | 34 +++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index e83e0891272d..210a37a065fb 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -63,6 +63,27 @@ static int mobility_rtas_call(int token, char *buf, s32 scope)
>  
>  static int delete_dt_node(struct device_node *dn)
>  {
> +	struct device_node *pdn;
> +	bool is_platfac;
> +
> +	pdn = of_get_parent(dn);
> +	is_platfac = of_node_is_type(dn, "ibm,platform-facilities") ||
> +		     of_node_is_type(pdn, "ibm,platform-facilities");
> +	of_node_put(pdn);
> +
> +	/*
> +	 * The drivers that bind to nodes in the platform-facilities
> +	 * hierarchy don't support node removal, and the removal directive
> +	 * from firmware is always followed by an add of an equivalent
> +	 * node. The capability (e.g. RNG, encryption, compression)
> +	 * represented by the node is never interrupted by the migration.
> +	 * So ignore changes to this part of the tree.
> +	 */
> +	if (is_platfac) {
> +		pr_notice("ignoring remove operation for %pOFfp\n", dn);
> +		return 0;
> +	}
> +
>  	pr_debug("removing node %pOFfp\n", dn);
>  	dlpar_detach_node(dn);
>  	return 0;
> @@ -222,6 +243,19 @@ static int add_dt_node(struct device_node *parent_dn, __be32 drc_index)
>  	if (!dn)
>  		return -ENOENT;
>  
> +	/*
> +	 * Since delete_dt_node() ignores this node type, this is the
> +	 * necessary counterpart. We also know that a platform-facilities
> +	 * node returned from dlpar_configure_connector() has children
> +	 * attached, and dlpar_attach_node() only adds the parent, leaking
> +	 * the children. So ignore these on the add side for now.
> +	 */
> +	if (of_node_is_type(dn, "ibm,platform-facilities")) {
> +		pr_notice("ignoring add operation for %pOF\n", dn);
> +		dlpar_free_cc_nodes(dn);
> +		return 0;
> +	}
> +
>  	rc = dlpar_attach_node(dn, parent_dn);
>  	if (rc)
>  		dlpar_free_cc_nodes(dn);
> -- 
> 2.31.1

Re: [PATCH v2] powerpc/pseries/mobility: ignore ibm, platform-facilities updates

[Nathan Lynch]

Daniel Axtens <dja@axtens.net> writes:
>> On VMs with NX encryption, compression, and/or RNG offload, these
>> capabilities are described by nodes in the ibm,platform-facilities device
>> tree hierarchy:
>>
>>   $ tree -d /sys/firmware/devicetree/base/ibm,platform-facilities/
>>   /sys/firmware/devicetree/base/ibm,platform-facilities/
>>   ├── ibm,compression-v1
>>   ├── ibm,random-v1
>>   └── ibm,sym-encryption-v1
>>
>>   3 directories
>>
>> The acceleration functions that these nodes describe are not disrupted by
>> live migration, not even temporarily.
>>
>> But the post-migration ibm,update-nodes sequence firmware always sends
>> "delete" messages for this hierarchy, followed by an "add" directive to
>> reconstruct it via ibm,configure-connector (log with debugging statements
>> enabled in mobility.c):
>>
>>   mobility: removing node /ibm,platform-facilities/ibm,random-v1:4294967285
>>   mobility: removing node /ibm,platform-facilities/ibm,compression-v1:4294967284
>>   mobility: removing node /ibm,platform-facilities/ibm,sym-encryption-v1:4294967283
>>   mobility: removing node /ibm,platform-facilities:4294967286
>>   ...
>>   mobility: added node /ibm,platform-facilities:4294967286
>>
>> Note we receive a single "add" message for the entire hierarchy, and what
>> we receive from the ibm,configure-connector sequence is the top-level
>> platform-facilities node along with its three children. The debug message
>> simply reports the parent node and not the whole subtree.
>
> If I understand correctly, (and again, this is not my area at all!) we
> still have to go out to the firmware and call the
> ibm,configure-connector sequence in order to figure out that the node
> we're supposed to add is the ibm,platform-facilites node, right? We
> can't save enough information at delete time to avoid the trip out to
> firmware?

That is right... but maybe I don't understand your angle here. Unsure
what avoiding the configure-connector sequence for the nodes would buy
us.


>> Until that can be realized we have a confirmed use-after-free and the
>> possibility of memory corruption. So add a limited workaround that
>> discriminates on the node type, ignoring adds and removes. This should be
>> amenable to backporting in the meantime.
>
> Yeah it's an unpleasant situation to find ourselves in. It's a bit icky
> but as I think you said in a previous email, at least this isn't worse:
> in the common case it should now succeed and and if properties change
> significantly it will still fail.
>
> My one question (from more of a security point of view) is:
>  1) Say you start using the facilities with a particular set of
>     parameters.
>
>  2) Say you then get migrated and the parameters change.
>
>  3) If you keep using the platform facilities as if the original
>     properties are still valid, can this cause any Interesting,
>     unexpected or otherwise Bad consequences? Are we going to end up
>     (for example) scribbling over random memory somehow?

If drivers are safely handling errors from H_COP_OP etc, then no. (I
know, this looks like a Well That Would Be a Driver Bug dismissal, but
that's not my attitude.) And again this is a case where the change
cannot make things worse.

In the current design of the pseries LPM implementation, user space and
other normal system activity resume as soon as we return from the
stop_machine() call which suspends the partition, executing concurrently
with any device tree updates. So even if we had code in place to
correctly resolve the DT changes and the drivers were able to respond to
the changes, there would still be a window of exposure to the kind of
problem you describe: the changed characteristics, if any, of the
destination obtain as soon as execution resumes, regardless of when the
OS initiates the update-nodes sequence.

The way out of that mess is to use the Linux suspend framework, or
otherwise prevent user space from executing until the destination
system's characteristics have been appropriately propagated out to the
necessary drivers etc. I'm trying to get there.


> Apart from that, the code seems to do what it says, it seems to solve a
> real problem, the error and memory handling makes sense, you _put the DT
> nodes that you _get, the comments are helpful and descriptive, and it
> passes the automated tests on patchwork/snowpatch.

I appreciate your review!

Re: [PATCH v2] powerpc/pseries/mobility: ignore ibm, platform-facilities updates

[Daniel Axtens]

Nathan Lynch <nathanl@linux.ibm.com> writes:

> Daniel Axtens <dja@axtens.net> writes:
>>> On VMs with NX encryption, compression, and/or RNG offload, these
>>> capabilities are described by nodes in the ibm,platform-facilities device
>>> tree hierarchy:
>>>
>>>   $ tree -d /sys/firmware/devicetree/base/ibm,platform-facilities/
>>>   /sys/firmware/devicetree/base/ibm,platform-facilities/
>>>   ├── ibm,compression-v1
>>>   ├── ibm,random-v1
>>>   └── ibm,sym-encryption-v1
>>>
>>>   3 directories
>>>
>>> The acceleration functions that these nodes describe are not disrupted by
>>> live migration, not even temporarily.
>>>
>>> But the post-migration ibm,update-nodes sequence firmware always sends
>>> "delete" messages for this hierarchy, followed by an "add" directive to
>>> reconstruct it via ibm,configure-connector (log with debugging statements
>>> enabled in mobility.c):
>>>
>>>   mobility: removing node /ibm,platform-facilities/ibm,random-v1:4294967285
>>>   mobility: removing node /ibm,platform-facilities/ibm,compression-v1:4294967284
>>>   mobility: removing node /ibm,platform-facilities/ibm,sym-encryption-v1:4294967283
>>>   mobility: removing node /ibm,platform-facilities:4294967286
>>>   ...
>>>   mobility: added node /ibm,platform-facilities:4294967286
>>>
>>> Note we receive a single "add" message for the entire hierarchy, and what
>>> we receive from the ibm,configure-connector sequence is the top-level
>>> platform-facilities node along with its three children. The debug message
>>> simply reports the parent node and not the whole subtree.
>>
>> If I understand correctly, (and again, this is not my area at all!) we
>> still have to go out to the firmware and call the
>> ibm,configure-connector sequence in order to figure out that the node
>> we're supposed to add is the ibm,platform-facilites node, right? We
>> can't save enough information at delete time to avoid the trip out to
>> firmware?
>
> That is right... but maybe I don't understand your angle here. Unsure
> what avoiding the configure-connector sequence for the nodes would buy
> us.

It's not meant to be a tricky question, so the simple answer is probably
the right one. Just wondering if there was a marginal efficiency gain -
although I believe it's not really a hot path anyway.

>
>
>>> Until that can be realized we have a confirmed use-after-free and the
>>> possibility of memory corruption. So add a limited workaround that
>>> discriminates on the node type, ignoring adds and removes. This should be
>>> amenable to backporting in the meantime.
>>
>> Yeah it's an unpleasant situation to find ourselves in. It's a bit icky
>> but as I think you said in a previous email, at least this isn't worse:
>> in the common case it should now succeed and and if properties change
>> significantly it will still fail.
>>
>> My one question (from more of a security point of view) is:
>>  1) Say you start using the facilities with a particular set of
>>     parameters.
>>
>>  2) Say you then get migrated and the parameters change.
>>
>>  3) If you keep using the platform facilities as if the original
>>     properties are still valid, can this cause any Interesting,
>>     unexpected or otherwise Bad consequences? Are we going to end up
>>     (for example) scribbling over random memory somehow?
>
> If drivers are safely handling errors from H_COP_OP etc, then no. (I
> know, this looks like a Well That Would Be a Driver Bug dismissal, but
> that's not my attitude.) And again this is a case where the change
> cannot make things worse.
>
> In the current design of the pseries LPM implementation, user space and
> other normal system activity resume as soon as we return from the
> stop_machine() call which suspends the partition, executing concurrently
> with any device tree updates. So even if we had code in place to
> correctly resolve the DT changes and the drivers were able to respond to
> the changes, there would still be a window of exposure to the kind of
> problem you describe: the changed characteristics, if any, of the
> destination obtain as soon as execution resumes, regardless of when the
> OS initiates the update-nodes sequence.
>
> The way out of that mess is to use the Linux suspend framework, or
> otherwise prevent user space from executing until the destination
> system's characteristics have been appropriately propagated out to the
> necessary drivers etc. I'm trying to get there.

Fair enough. I do appreciate the perfect not being the enemy of the good
especially in areas of the codebase like this where there is scope to
improve things but there is also a lot of complexity that we cannot
really get away from because the underlying problem domain is itself
just plain complex. (I think EEH is the other obvious example in
arch/powerpc.)

>> Apart from that, the code seems to do what it says, it seems to solve a
>> real problem, the error and memory handling makes sense, you _put the DT
>> nodes that you _get, the comments are helpful and descriptive, and it
>> passes the automated tests on patchwork/snowpatch.
>
> I appreciate your review!

With those questions answered, and with the caveats above and noting my
complete inability to test the code:

Reviewed-by: Daniel Axtens <dja@axtens.net>

Kind regards,
Daniel

Re: [PATCH v2] powerpc/pseries/mobility: ignore ibm, platform-facilities updates

[Michael Ellerman]

On Wed, 20 Oct 2021 14:47:03 -0500, Nathan Lynch wrote:
> On VMs with NX encryption, compression, and/or RNG offload, these
> capabilities are described by nodes in the ibm,platform-facilities device
> tree hierarchy:
> 
>   $ tree -d /sys/firmware/devicetree/base/ibm,platform-facilities/
>   /sys/firmware/devicetree/base/ibm,platform-facilities/
>   ├── ibm,compression-v1
>   ├── ibm,random-v1
>   └── ibm,sym-encryption-v1
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/pseries/mobility: ignore ibm, platform-facilities updates
      https://git.kernel.org/powerpc/c/319fa1a52e438a6e028329187783a25ad498c4e6

cheers