[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [ImageBuilder 2/2] Add support for lopper to generate partial dts


  • To: Michal Orzel <michal.orzel@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Ayan Kumar Halder <ayankuma@xxxxxxx>
  • Date: Mon, 12 Sep 2022 17:41:28 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=BvdGQAgT8wcsWoOOAirWtXjeIJ1Wk2SHhu09KgIVbvs=; b=eOm94mW8jhczIVnK6Cxsu5Rz6q0LV5cT3ZdOPox6YOnKkAN/1bMH7W/f++iUmokh3pqFBnQhc4MdKJxjjdLe1iwQDfJwt+wQTYCV5CKUvqdLTb8CB3teR82o9JZJG3PjPkfjmA/4NSlscoi4Nuu+iCy0wGsNUlRPNZ/Fk2+bjGNWChvRcjgDM5HLuXOQW7lCcFL5cA4zv3tiNStLnXJTN015s5cCMJMYtrI7ymngu+ektRqYBgCNMr4klEYB1TihmwhfDkg97+O8vRycNKUN4mDt4LexMLVp3W0evYpQ3dU6tpkiCUI/LgizscPQOwvnfX5Fdg0ED8CmqWTnnvmejQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UnaZf5s3836ASX61JRso23FJAu3hzZ1J53SCl8yAcxIsrNOSOQcx19fLa7B1ijyAPiaHA56f7883Z3nQDYM2sW6xlGXXp0yP4dVKudPFqsZp2sEKbhdCRrr/ApQc3lF4zcs/AQVRHIwKT3xh84KzcSscTi7xxZXvZVhs7sDkCZaAYPJqiKQwwSSKXAqkwQlgcwZz1nsUsQY2L3MbYX1msArvOM9sHTGEgI9kSeaUJnDQIgnI6ApPCblsgaUkg2wvGpqwOthPILP8FHFXuhzGzjqVhT5NTLU7uBRtuji+PfLeolS5ebX6YykR4AXwUATT/taAi8M5sI4pVSqRLkXtWA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
  • Cc: sstabellini@xxxxxxxxxx
  • Delivery-date: Mon, 12 Sep 2022 16:41:46 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Michal,

On 12/09/2022 12:59, Michal Orzel wrote:
Currently ImageBuilder can compile and merge partial dts obtained from
a repository specified using PASSTHROUGH_DTS_REPO. With the recent
changes done in the lopper, we can use it to generate partial dts
automatically (to some extent as this is still an early support).

Introduce LOPPER_PATH option to specify a path to a lopper.py script,
that if set, will invoke lopper to generate partial dts for the
passthrough devices specified in DOMU_PASSTHROUGH_PATHS.

Introduce LOPPER_CMD option to specify custom command line arguments
(if needed) for lopper's extract assist.

Example usage:
LOPPER_PATH="/home/user/lopper/lopper.py"
DOMU_PASSTHROUGH_PATHS[0]="/axi/spi@ff0f0000 /axi/serial@ff010000"

Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
---
  README.md                | 22 ++++++++++++--
  scripts/common           | 64 ++++++++++++++++++++++++++++++----------
  scripts/uboot-script-gen | 17 +++++++++--
  3 files changed, 83 insertions(+), 20 deletions(-)

diff --git a/README.md b/README.md
index da9ba788a3bf..aaee0939b589 100644
--- a/README.md
+++ b/README.md
@@ -128,6 +128,19 @@ Where:
  - DT_OVERLAY[number] specifies the path to the hosts device tree overlays
    to be added at boot time in u-boot
+- LOPPER_PATH specifies the path to lopper.py script. This is optional.
+  However, if this is specified, then DOMU_PASSTHROUGH_PATHS[number] need
+  to be specified. uboot-script-gen will invoke lopper to generate the partial
+  device trees which have been specified in DOMU_PASSTHROUGH_PATHS[number].
+  This option is currently in experimental state as the corresponding lopper
+  changes are still in an early support state.
+
+- LOPPER_CMD specifies the command line arguments for lopper's extract assist.
+  This is optional and only applicable when LOPPER_PATH is specified. Only to 
be
+  used to specify which nodes to include (using -i <node_name>) and which
+  nodes/properties to exclude (using -x <regex>). If not set at all, the 
default
+  one is used applicable for ZynqMP MPSoC boards.

You are using some more arguments (besides -x and -i) :-

--permissive -f
-- extract -t
-- extract-xen -t $node -o

It will be good to have some explaination for these. See my comments below.

+
  - NUM_DOMUS specifies how many Dom0-less DomUs to load
- DOMU_KERNEL[number] specifies the DomU kernel to use.
@@ -140,7 +153,7 @@ Where:
  - DOMU_PASSTHROUGH_PATHS[number] specifies the passthrough devices (
    separated by spaces). It adds "xen,passthrough" to the corresponding
    dtb nodes in xen device tree blob.
-  This option is valid in the following two cases:
+  This option is valid in the following cases:
1. When PASSTHROUGH_DTS_REPO is provided.
    With this option, the partial device trees (corresponding to the
@@ -149,7 +162,12 @@ Where:
    Note it assumes that the names of the partial device trees will match
    to the names of the devices specified here.
- 2. When DOMU_NOBOOT[number] is provided. In this case, it will only
+  2. When LOPPER_PATH is provided.
+  With this option, the partial device trees (corresponding to the
+  passthrough devices) are generated by the lopper and then compiled and merged
+  by ImageBuilder to be used as DOMU[number] device tree blob.
+
+  3. When DOMU_NOBOOT[number] is provided. In this case, it will only
    add "xen,passthrough" as mentioned before.
- DOMU_PASSTHROUGH_DTB[number] specifies the passthrough device trees
diff --git a/scripts/common b/scripts/common
index ccad03d82b30..680c5090cd07 100644
--- a/scripts/common
+++ b/scripts/common
@@ -9,6 +9,9 @@
  # - NUM_DOMUS
  # - DOMU_PASSTHROUGH_PATHS
  # - DOMU_PASSTHROUGH_DTB
+# - LOPPER_PATH
+# - LOPPER_CMD
+# - DEVICE_TREE
tmp_files=()
  tmp_dirs=()
@@ -99,31 +102,41 @@ function compile_merge_partial_dts()
      local tmp
      local tmpdts
      local file
+    local node
      local i
      local j
- if [[ "$repo" =~ .*@.*:.* ]]
+    if test "$repo"
      then
-        tmp=`mktemp -d`
-        tmp_dirs+=($tmp)
-
-        echo "Cloning git repo \"$git_repo\""
-        git clone "$repo" $tmp
-        if test $? -ne 0
+        # Partial dts will be obtained from PASSTHROUGH_DTS_REPO
+        if [[ "$repo" =~ .*@.*:.* ]]
          then
-            echo "Error occurred while cloning \"$git_repo\""
-            return 1
-        fi
+            tmp=`mktemp -d`
+            tmp_dirs+=($tmp)
- repo=$tmp
-    fi
+            echo "Cloning git repo \"$git_repo\""
+            git clone "$repo" $tmp
+            if test $? -ne 0
+            then
+                echo "Error occurred while cloning \"$git_repo\""
+                return 1
+            fi
- if test -z "$dir"
-    then
-        dir="."
+            repo=$tmp
+        fi
+
+        if test -z "$dir"
+        then
+            dir="."
+        fi
+        partial_dts_dir="$repo"/"$dir"
+    else
+        # Partial dts will be generated by the lopper
+        tmp=`mktemp -d`
+        tmp_dirs+=($tmp)
+        partial_dts_dir="$tmp"
      fi
- partial_dts_dir="$repo"/"$dir"
      i=0
      while test $i -lt $NUM_DOMUS
      do
@@ -133,6 +146,25 @@ function compile_merge_partial_dts()
              return 1
          fi
+ if test -z "$repo"
+        then
+            # Generate partial dts using lopper
+            for devpath in ${DOMU_PASSTHROUGH_PATHS[$i]}
+            do
+                node=${devpath##*/}
+                file="$partial_dts_dir"/"$node".dts
+
+                $LOPPER_PATH --permissive -f $DEVICE_TREE \
+                -- extract -t $devpath $LOPPER_CMD \
+                -- extract-xen -t $node -o $file
See below comment. Applies here as well.
+
+                if test $? -ne 0
+                then
+                    return 1
+                fi
+            done
+        fi
+
          sanity_check_partial_dts "${DOMU_PASSTHROUGH_PATHS[$i]}" 
"$partial_dts_dir"
          if test $? -ne 0
          then
diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index 1f8ab5ffd193..84a68d6bd0b0 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -1138,10 +1138,23 @@ fi
  # tftp or move the files to a partition
  cd "$uboot_dir"
-if test "$PASSTHROUGH_DTS_REPO"
+# If both PASSTHROUGH_DTS_REPO and LOPPER_PATH options are specified,
+# the former takes precedence because the partial device trees are already
+# created (probably tested), hence the reliability is higher than using lopper.
+if test "$PASSTHROUGH_DTS_REPO" || test "$LOPPER_PATH"
  then
      output_dir=`mktemp -d "partial-dtbs-XXX"`
-    compile_merge_partial_dts $output_dir "$PASSTHROUGH_DTS_REPO"
+    if test "$PASSTHROUGH_DTS_REPO"
+    then
+        compile_merge_partial_dts $output_dir "$PASSTHROUGH_DTS_REPO"
+    else
+        if test -z "$LOPPER_CMD"
+        then
+            # Default for ZynqMP MPSoC
+            LOPPER_CMD="-i zynqmp-firmware -x interrupt-controller -x pinctrl -x 
power-domains -x resets -x current-speed"

It will be very useful, if you could provide the link to Lopper's README which explains the arguments used here, as a comment.

Even better if you can provide some explaination (as a comment) to what the command intends to do here.

- Ayan

+        fi
+        compile_merge_partial_dts $output_dir
+    fi
      if test $? -ne 0
      then
          # Remove the output dir holding the partial dtbs in case of any error



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.