Hi Michal,
On 18/06/2025 08:06, Orzel, Michal wrote:
On 17/06/2025 19:13, Alejandro Vallejo wrote:
The DT spec declares only two number types for a property: u32 and u64,
as per Table 2.3 in Section 2.2.4. Remove unbounded loop and replace
with a switch statement. Default to a size of 1 cell in the nonsensical
size case, with a warning printed on the Xen console.
Suggested-by: Daniel P. Smith" <dpsm...@apertussolutions.com>
Signed-off-by: Alejandro Vallejo <agarc...@amd.com>
---
v2:
* Added missing `break` on the `case 2:` branch and added
ASSERT_UNREACHABLE() to the deafult path
---
xen/include/xen/device_tree.h | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 75017e4266..2ec668b94a 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -261,10 +261,21 @@ void intc_dt_preinit(void);
/* Helper to read a big number; size is in cells (not bytes) */
static inline u64 dt_read_number(const __be32 *cell, int size)
{
- u64 r = 0;
+ u64 r = be32_to_cpu(*cell);
+
+ switch ( size )
+ {
+ case 1:
+ break;
+ case 2:
+ r = (r << 32) | be32_to_cpu(cell[1]);
+ break;
+ default:
+ // Nonsensical size. default to 1.
I wonder why there are so many examples of device trees in Linux with
#address-cells = <3>? Also, libfdt defines FDT_MAX_NCELLS as 4 with comment:
"maximum value for #address-cells and #size-cells" but I guess it follows the
IEE1275 standard and DT spec "is loosely related" to it.
A lot of the use seems to be related to PCI. Below an example from [1]:
pcie {
#address-cells = <3>;
#size-cells = <2>;
pcie@0 {
device_type = "pci";
reg = <0x0 0x0 0x0 0x0 0x0>;
#address-cells = <3>;
#size-cells = <2>;
ranges;
wifi@0 {
compatible = "pci17cb,1109";
reg = <0x0 0x0 0x0 0x0 0x0>;
qcom,calibration-variant = "RDP433_1";
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
wifi1_wsi_tx: endpoint {
remote-endpoint = <&wifi2_wsi_rx>;
};
};
port@1 {
reg = <1>;
wifi1_wsi_rx: endpoint {
remote-endpoint = <&wifi3_wsi_tx>;
};
};
};
};
};
"reg" has effectively 5 cells (3 for address and 2 for size).
From a very brief look, I couldn't find how this is interpreted. But I
don't see how dt_read_number() can be used in this case. So I think it
makes sense to restrict. The question though is whether it is a good
idea to cap the value and behave differently from Linux.
Speaking of which there are another fun different behavior between Linux
and Xen in DT. The default size of the root #address is 2 in Xen (see
DT_ROOT_NODE_ADDR_CELLS_DEFAULT) which is spec compliant. But Linux will
use 1 (see OF_ROOT_NODE_ADDR_CELLS_DEFAULT). I haven't seen any issue so
far, but only notice recently when working on a separate project
recently. I am still undecided for this one whether Xen should change
because technically a Device-Tree should nowadays always provide
#address-cells and #size-cells.
Cheers,
[1] devicetree/bindings/net/wireless/qcom,ath12k-wsi.yaml
--
Julien Grall
