diff mbox series

[14/15] add a domU script to fetch overlays and applying them to linux

Message ID 20240424033449.168398-15-xin.wang2@amd.com (mailing list archive)
State Superseded
Headers show
Series Remaining patches for dynamic node programming using overlay dtbo | expand

Commit Message

Henry Wang April 24, 2024, 3:34 a.m. UTC 
From: Vikram Garhwal <fnu.vikram@xilinx.com>

Introduce a shell script that runs in the background and calls
get_overlay to retrive overlays and add them (or remove them) to Linux
device tree (running as a domU).

Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
Signed-off-by: Henry Wang <xin.wang2@amd.com>
---
 tools/helpers/Makefile       |  2 +-
 tools/helpers/get_overlay.sh | 81 ++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100755 tools/helpers/get_overlay.sh

Comments

Jan Beulich April 24, 2024, 6:16 a.m. UTC | #1 
On 24.04.2024 05:34, Henry Wang wrote:
> From: Vikram Garhwal <fnu.vikram@xilinx.com>
> 
> Introduce a shell script that runs in the background and calls
> get_overlay to retrive overlays and add them (or remove them) to Linux
> device tree (running as a domU).
> 
> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> Signed-off-by: Henry Wang <xin.wang2@amd.com>
> ---
>  tools/helpers/Makefile       |  2 +-
>  tools/helpers/get_overlay.sh | 81 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 82 insertions(+), 1 deletion(-)
>  create mode 100755 tools/helpers/get_overlay.sh

Besides the same naming issue as in the earlier patch, the script also
looks very Linux-ish. Yet ...

> --- a/tools/helpers/Makefile
> +++ b/tools/helpers/Makefile
> @@ -58,7 +58,6 @@ init-dom0less: $(INIT_DOM0LESS_OBJS)
>  get_overlay: $(SHARE_OVERLAY_OBJS)
>  	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenvchan) $(LDLIBS_libxenstore) $(LDLIBS_libxenctrl) $(LDLIBS_libxengnttab) $(APPEND_LDFLAGS)
>  
> -
>  .PHONY: install
>  install: all
>  	$(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
> @@ -67,6 +66,7 @@ install: all
>  .PHONY: uninstall
>  uninstall:
>  	for i in $(TARGETS); do rm -f $(DESTDIR)$(LIBEXEC_BIN)/$$i; done
> +	$(RM) $(DESTDIR)$(LIBEXEC_BIN)/get_overlay.sh
>  
>  .PHONY: clean
>  clean:

... you touching only the uninstall target, it's not even clear to me
how (and under what conditions) the script is going to make it into
$(DESTDIR)$(LIBEXEC_BIN)/. Did you mean to add to $(TARGETS), perhaps,
alongside the earlier added get-overlay binary?

Jan
Henry Wang April 25, 2024, 12:54 a.m. UTC | #2 
Hi Jan,

On 4/24/2024 2:16 PM, Jan Beulich wrote:
> On 24.04.2024 05:34, Henry Wang wrote:
>> From: Vikram Garhwal <fnu.vikram@xilinx.com>
>>
>> Introduce a shell script that runs in the background and calls
>> get_overlay to retrive overlays and add them (or remove them) to Linux
>> device tree (running as a domU).
>>
>> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
>> Signed-off-by: Henry Wang <xin.wang2@amd.com>
>> ---
>>   tools/helpers/Makefile       |  2 +-
>>   tools/helpers/get_overlay.sh | 81 ++++++++++++++++++++++++++++++++++++
>>   2 files changed, 82 insertions(+), 1 deletion(-)
>>   create mode 100755 tools/helpers/get_overlay.sh
> Besides the same naming issue as in the earlier patch, the script also
> looks very Linux-ish. Yet ...

I will fix the naming issue in v2. Would you mind elaborating a bit more 
about the "Linux-ish" concern? I guess this is because the original use 
case is on Linux, should I do anything about this?

>> --- a/tools/helpers/Makefile
>> +++ b/tools/helpers/Makefile
>> @@ -58,7 +58,6 @@ init-dom0less: $(INIT_DOM0LESS_OBJS)
>>   get_overlay: $(SHARE_OVERLAY_OBJS)
>>   	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenvchan) $(LDLIBS_libxenstore) $(LDLIBS_libxenctrl) $(LDLIBS_libxengnttab) $(APPEND_LDFLAGS)
>>   
>> -
>>   .PHONY: install
>>   install: all
>>   	$(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
>> @@ -67,6 +66,7 @@ install: all
>>   .PHONY: uninstall
>>   uninstall:
>>   	for i in $(TARGETS); do rm -f $(DESTDIR)$(LIBEXEC_BIN)/$$i; done
>> +	$(RM) $(DESTDIR)$(LIBEXEC_BIN)/get_overlay.sh
>>   
>>   .PHONY: clean
>>   clean:
> ... you touching only the uninstall target, it's not even clear to me
> how (and under what conditions) the script is going to make it into
> $(DESTDIR)$(LIBEXEC_BIN)/. Did you mean to add to $(TARGETS), perhaps,
> alongside the earlier added get-overlay binary?

You are right, I think the get-overlay binary and this script should be 
installed if DTB overlay is supported. Checking the code, I found 
LIBXL_HAVE_DT_OVERLAY which can indicate if we have this feature 
supported in libxl. Do you think it is a good idea to use it to install 
these two files in Makefile? Thanks.

Kind regards,
Henry

>
> Jan
Jan Beulich April 25, 2024, 6:46 a.m. UTC | #3 
On 25.04.2024 02:54, Henry Wang wrote:
> On 4/24/2024 2:16 PM, Jan Beulich wrote:
>> On 24.04.2024 05:34, Henry Wang wrote:
>>> From: Vikram Garhwal <fnu.vikram@xilinx.com>
>>>
>>> Introduce a shell script that runs in the background and calls
>>> get_overlay to retrive overlays and add them (or remove them) to Linux
>>> device tree (running as a domU).
>>>
>>> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>> Signed-off-by: Henry Wang <xin.wang2@amd.com>
>>> ---
>>>   tools/helpers/Makefile       |  2 +-
>>>   tools/helpers/get_overlay.sh | 81 ++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 82 insertions(+), 1 deletion(-)
>>>   create mode 100755 tools/helpers/get_overlay.sh
>> Besides the same naming issue as in the earlier patch, the script also
>> looks very Linux-ish. Yet ...
> 
> I will fix the naming issue in v2. Would you mind elaborating a bit more 
> about the "Linux-ish" concern? I guess this is because the original use 
> case is on Linux, should I do anything about this?

Well, the script won't work on other than Linux, will it? Therefore ...

>>> --- a/tools/helpers/Makefile
>>> +++ b/tools/helpers/Makefile
>>> @@ -58,7 +58,6 @@ init-dom0less: $(INIT_DOM0LESS_OBJS)
>>>   get_overlay: $(SHARE_OVERLAY_OBJS)
>>>   	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenvchan) $(LDLIBS_libxenstore) $(LDLIBS_libxenctrl) $(LDLIBS_libxengnttab) $(APPEND_LDFLAGS)
>>>   
>>> -
>>>   .PHONY: install
>>>   install: all
>>>   	$(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
>>> @@ -67,6 +66,7 @@ install: all
>>>   .PHONY: uninstall
>>>   uninstall:
>>>   	for i in $(TARGETS); do rm -f $(DESTDIR)$(LIBEXEC_BIN)/$$i; done
>>> +	$(RM) $(DESTDIR)$(LIBEXEC_BIN)/get_overlay.sh
>>>   
>>>   .PHONY: clean
>>>   clean:
>> ... you touching only the uninstall target, it's not even clear to me
>> how (and under what conditions) the script is going to make it into
>> $(DESTDIR)$(LIBEXEC_BIN)/. Did you mean to add to $(TARGETS), perhaps,
>> alongside the earlier added get-overlay binary?

... it first of needs to become clear under what conditions it is actually
going to be installed.

> You are right, I think the get-overlay binary and this script should be 
> installed if DTB overlay is supported. Checking the code, I found 
> LIBXL_HAVE_DT_OVERLAY which can indicate if we have this feature 
> supported in libxl. Do you think it is a good idea to use it to install 
> these two files in Makefile? Thanks.

Counter question: If it's not going to be installed, how are people going
to make use of it? If the script is intended for manual use only, I think
that would want saying in the description. Yet then I couldn't see why
the uninstall goal would need modifying.

As to LIBXL_HAVE_DT_OVERLAY - that's not accessible from a Makefile, I
guess?

Jan
Henry Wang April 25, 2024, 7:06 a.m. UTC | #4 
Hi Jan,

On 4/25/2024 2:46 PM, Jan Beulich wrote:
> On 25.04.2024 02:54, Henry Wang wrote:
>> On 4/24/2024 2:16 PM, Jan Beulich wrote:
>>> On 24.04.2024 05:34, Henry Wang wrote:
>>>> From: Vikram Garhwal <fnu.vikram@xilinx.com>
>>>>
>>>> Introduce a shell script that runs in the background and calls
>>>> get_overlay to retrive overlays and add them (or remove them) to Linux
>>>> device tree (running as a domU).
>>>>
>>>> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>>> Signed-off-by: Henry Wang <xin.wang2@amd.com>
>>>> ---
>>>>    tools/helpers/Makefile       |  2 +-
>>>>    tools/helpers/get_overlay.sh | 81 ++++++++++++++++++++++++++++++++++++
>>>>    2 files changed, 82 insertions(+), 1 deletion(-)
>>>>    create mode 100755 tools/helpers/get_overlay.sh
>>> Besides the same naming issue as in the earlier patch, the script also
>>> looks very Linux-ish. Yet ...
>> I will fix the naming issue in v2. Would you mind elaborating a bit more
>> about the "Linux-ish" concern? I guess this is because the original use
>> case is on Linux, should I do anything about this?
> Well, the script won't work on other than Linux, will it? Therefore ...
>
>>>> --- a/tools/helpers/Makefile
>>>> +++ b/tools/helpers/Makefile
>>>> @@ -58,7 +58,6 @@ init-dom0less: $(INIT_DOM0LESS_OBJS)
>>>>    get_overlay: $(SHARE_OVERLAY_OBJS)
>>>>    	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenvchan) $(LDLIBS_libxenstore) $(LDLIBS_libxenctrl) $(LDLIBS_libxengnttab) $(APPEND_LDFLAGS)
>>>>    
>>>> -
>>>>    .PHONY: install
>>>>    install: all
>>>>    	$(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
>>>> @@ -67,6 +66,7 @@ install: all
>>>>    .PHONY: uninstall
>>>>    uninstall:
>>>>    	for i in $(TARGETS); do rm -f $(DESTDIR)$(LIBEXEC_BIN)/$$i; done
>>>> +	$(RM) $(DESTDIR)$(LIBEXEC_BIN)/get_overlay.sh
>>>>    
>>>>    .PHONY: clean
>>>>    clean:
>>> ... you touching only the uninstall target, it's not even clear to me
>>> how (and under what conditions) the script is going to make it into
>>> $(DESTDIR)$(LIBEXEC_BIN)/. Did you mean to add to $(TARGETS), perhaps,
>>> alongside the earlier added get-overlay binary?
> ... it first of needs to become clear under what conditions it is actually
> going to be installed.
>
>> You are right, I think the get-overlay binary and this script should be
>> installed if DTB overlay is supported. Checking the code, I found
>> LIBXL_HAVE_DT_OVERLAY which can indicate if we have this feature
>> supported in libxl. Do you think it is a good idea to use it to install
>> these two files in Makefile? Thanks.
> Counter question: If it's not going to be installed, how are people going
> to make use of it? If the script is intended for manual use only, I think
> that would want saying in the description. Yet then I couldn't see why
> the uninstall goal would need modifying.

Checking the code again, I feel like this is a mistake actually. I think 
this script should be installed together with the get-overlay 
application as the script actually calls get-overlay. The uninstall goal 
should remain untouched. I will fix this in v2.

> As to LIBXL_HAVE_DT_OVERLAY - that's not accessible from a Makefile, I
> guess?

Yes.

Kind regards,
Henry

>
> Jan
diff mbox series

Patch

diff --git a/tools/helpers/Makefile b/tools/helpers/Makefile
index dfe17ef269..2d0558aeb8 100644
--- a/tools/helpers/Makefile
+++ b/tools/helpers/Makefile
@@ -58,7 +58,6 @@  init-dom0less: $(INIT_DOM0LESS_OBJS)
 get_overlay: $(SHARE_OVERLAY_OBJS)
 	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenvchan) $(LDLIBS_libxenstore) $(LDLIBS_libxenctrl) $(LDLIBS_libxengnttab) $(APPEND_LDFLAGS)
 
-
 .PHONY: install
 install: all
 	$(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
@@ -67,6 +66,7 @@  install: all
 .PHONY: uninstall
 uninstall:
 	for i in $(TARGETS); do rm -f $(DESTDIR)$(LIBEXEC_BIN)/$$i; done
+	$(RM) $(DESTDIR)$(LIBEXEC_BIN)/get_overlay.sh
 
 .PHONY: clean
 clean:
diff --git a/tools/helpers/get_overlay.sh b/tools/helpers/get_overlay.sh
new file mode 100755
index 0000000000..2e8c6ecefd
--- /dev/null
+++ b/tools/helpers/get_overlay.sh
@@ -0,0 +1,81 @@ 
+#!/bin/sh
+
+modprobe xen_gntalloc
+modprobe xen_gntdev
+
+while :
+do
+    overlay_node_name=""
+    type_overlay="normal"
+    is_partial_dtb=""
+
+    output=`/usr/lib/xen/bin/get_overlay 0`
+
+    if test $? -ne 0
+    then
+        echo error
+        exit 1
+    fi
+
+    if test -z "$output"
+    then
+        echo ""
+        exit 1
+    fi
+
+    # output: add overlay-name normal partial
+    operation=`echo $output | cut -d " " -f 1`
+    overlay_node_name=`echo $output | cut -d " " -f 2`
+    type_overlay=`echo $output | cut -d " " -f 3`
+    is_partial_dtb=`echo $output | cut -d " " -f 4`
+
+    if test -z "$operation" || test -z "$overlay_node_name"
+    then
+        echo "invalid ops"
+        exit 1
+    fi
+
+    if test $operation = "add"
+    then
+        echo "Overlay received"
+
+        if test "$type_overlay" = "normal"
+        then
+            final_path="/sys/kernel/config/device-tree/overlays/$overlay_node_name"
+            mkdir -p $final_path
+            cat overlay.dtbo > $final_path/dtbo
+        else
+            # fpga overlay
+            cp overlay.dtbo lib/firmware/
+            mkdir /configfs
+            mount -t configfs configfs /configfs
+            cd /configfs/device-tree/overlays/
+
+            if test "$is_partial_dtb"
+            then
+                mkdir partial
+                echo 1 > /sys/class/fpga_manager/fpga0/flags
+                echo -n "overlay.dtbo" > /configfs/device-tree/overlays/partial
+            else
+                mkdir full
+                echo -n "overlay.dtbo" > /configfs/device-tree/overlays/full
+            fi
+        fi
+    elif test $operation = "remove"
+    then
+        if test "$type_overlay" = "normal"
+        then
+            # implement remove
+            path=/sys/kernel/config/device-tree/overlays/$overlay_node_name/dtbo
+            if ! test -f $path
+            then
+                echo "error: path doesn't exist"
+                exit 1
+            fi
+            rm $path
+        fi
+    else
+        echo "operation unsupported"
+        exit 1
+    fi
+done