public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] IntelFsp2Pkg: Add support for config editor to handle multiple UPD
@ 2021-10-15  0:38 Tung Lun
  2021-10-18  5:10 ` Chiu, Chasel
  0 siblings, 1 reply; 3+ messages in thread
From: Tung Lun @ 2021-10-15  0:38 UTC (permalink / raw)
  To: devel; +Cc: Loo Tung Lun, Maurice Ma, Nate DeSimone, Star Zeng, Chasel Chiu

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3692

In several use cases in bootloader, there are multiple instances of UPD
with same signature header. As such, using previous version of config
editor to edit those will result in only overriding the first found
instance. This patch provides the flexibility to modify the instance
specified.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>

Signed-off-by: Loo Tung Lun <tung.lun.loo@intel.com>
---
 IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 62 insertions(+), 32 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py b/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
index b593885807..91c4180085 100644
--- a/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
+++ b/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
@@ -1351,24 +1351,20 @@ option format '%s' !" % option)
                                 act_cfg['value']
                         option = act_cfg['option']
 
-                        cfg_val = ''
-                        bin_val = ''
                         for i in option.split(','):
                             if act_cfg['value'] in i:
-                                bin_val = i
+                                self.data_diff += \
+                                    '\n\nBinary:          ' \
+                                    + act_cfg['name'] + ': ' \
+                                    + i + '\n'
                             elif config_val in i:
-                                cfg_val = i
-                        if cfg_val != '' and bin_val != '':
-                            self.data_diff += '\n\nBinary:        ' \
-                                + act_cfg['name'] \
-                                + ': ' + bin_val.replace(' ', '') \
-                                + '\nConfig file:   ' \
-                                + act_cfg['name'] + ': ' \
-                                + cfg_val.replace(' ', '') + '\n'
+                                self.data_diff += \
+                                    '\nConfig file:     ' \
+                                    + act_cfg['name'] + ': ' + i
                     else:
-                        self.data_diff += '\n\nBinary:        ' \
+                        self.data_diff += '\n\nBinary:           ' \
                             + act_cfg['name'] + ': ' + act_cfg['value'] \
-                            + '\nConfig file:   ' + act_cfg['name'] \
+                            + '\nConfig file:     ' + act_cfg['name'] \
                             + ': ' + config_val + '\n'
 
     def set_field_value(self, top, value_bytes, force=False):
@@ -1477,33 +1473,67 @@ for '%s' !" % (act_cfg['value'], act_cfg['path']))
     def get_bin_segment(self, bin_data):
         cfg_segs = self.get_cfg_segment()
         bin_segs = []
+        fsp_instance = []
         for seg in cfg_segs:
             key = seg[0].encode()
+            print("key ", key)
             if key == 0:
                 bin_segs.append([seg[0], 0, len(bin_data)])
                 break
             pos = bin_data.find(key)
-            if pos >= 0:
-                # ensure no other match for the key
-                next_pos = bin_data.find(key, pos + len(seg[0]))
-                if next_pos >= 0:
-                    if key == b'$SKLFSP$' or key == b'$BSWFSP$':
-                        string = ('Warning: Multiple matches for %s in '
-                                  'binary!\n\nA workaround applied to such '
-                                  'FSP 1.x binary to use second'
-                                  ' match instead of first match!' % key)
-                        messagebox.showwarning('Warning!', string)
-                        pos = next_pos
-                    else:
-                        print("Warning: Multiple matches for '%s' "
-                              "in binary, the 1st instance will be used !"
-                              % seg[0])
-                bin_segs.append([seg[0], pos, seg[2]])
-                self.binseg_dict[seg[0]] = pos
-            else:
+            while pos != -1:
+                fsp_instance.append(pos)
+                pos = bin_data.find(key, pos + len(seg[0]))
+            if len(fsp_instance) <= 0:
                 bin_segs.append([seg[0], -1, seg[2]])
                 self.binseg_dict[seg[0]] = -1
-                continue
+
+            elif len(fsp_instance) == 1:
+                bin_segs.append([seg[0], fsp_instance[0], seg[2]])
+                self.binseg_dict[seg[0]] = fsp_instance[0]
+                fsp_instance.clear()
+
+            else:
+
+                fsp_instance_root = tkinter.Tk()
+
+                canvas1 = tkinter.Canvas(fsp_instance_root,
+                                         width=400, height=400)
+                canvas1.pack()
+
+                entry1 = tkinter.Entry(fsp_instance_root)
+                canvas1.create_window(200, 220, window=entry1)
+
+                text = "Multiple instances available for " +\
+                    seg[0] + "\n\nThe available instances are\n"
+                for edx, ins in enumerate(fsp_instance):
+                    text += "\nInstance" + str(edx + 1) + ' :offset  ' +\
+                         str(hex(ins))
+                text += "\n\nPlease enter the instance number between 1 and "\
+                    + str(len(fsp_instance))
+                label1 = tkinter.Label(
+                    fsp_instance_root,
+                    text=text, wraplength=380, justify='left')
+                canvas1.create_window(200, 90, window=label1)
+
+                def getfspinstance():
+                    x1 = entry1.get()
+                    fsp_instance_option = int(x1)
+                    if fsp_instance_option <= len(fsp_instance):
+                        bin_segs.append([seg[0],
+                                        fsp_instance[fsp_instance_option - 1],
+                                        seg[2]])
+                        self.binseg_dict[seg[0]] = fsp_instance[
+                            fsp_instance_option - 1]
+                    fsp_instance_root.destroy()
+
+                button2 = tkinter.Button(fsp_instance_root,
+                                         text='Enter the instance',
+                                         command=lambda: getfspinstance())
+                canvas1.create_window(200, 250, window=button2)
+                fsp_instance_root.wait_window(fsp_instance_root)
+
+                fsp_instance.clear()
 
         return bin_segs
 
-- 
2.26.2.windows.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] IntelFsp2Pkg: Add support for config editor to handle multiple UPD
  2021-10-15  0:38 [PATCH] IntelFsp2Pkg: Add support for config editor to handle multiple UPD Tung Lun
@ 2021-10-18  5:10 ` Chiu, Chasel
  2021-10-18  5:44   ` Tung Lun
  0 siblings, 1 reply; 3+ messages in thread
From: Chiu, Chasel @ 2021-10-18  5:10 UTC (permalink / raw)
  To: Loo, Tung Lun, devel@edk2.groups.io
  Cc: Ma, Maurice, Desimone, Nathaniel L, Zeng, Star


Hi Tung Lun,

Should we update ConfigEditorUserManual.md for this new function?

Thanks,
Chasel


> -----Original Message-----
> From: Loo, Tung Lun <tung.lun.loo@intel.com>
> Sent: Friday, October 15, 2021 8:39 AM
> To: devel@edk2.groups.io
> Cc: Loo, Tung Lun <tung.lun.loo@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Chiu,
> Chasel <chasel.chiu@intel.com>
> Subject: [PATCH] IntelFsp2Pkg: Add support for config editor to handle multiple
> UPD
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3692
> 
> In several use cases in bootloader, there are multiple instances of UPD with
> same signature header. As such, using previous version of config editor to edit
> those will result in only overriding the first found instance. This patch provides
> the flexibility to modify the instance specified.
> 
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> 
> Signed-off-by: Loo Tung Lun <tung.lun.loo@intel.com>
> ---
>  IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py | 94
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> ---------------------------
>  1 file changed, 62 insertions(+), 32 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
> b/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
> index b593885807..91c4180085 100644
> --- a/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
> +++ b/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
> @@ -1351,24 +1351,20 @@ option format '%s' !" % option)
>                                  act_cfg['value']                         option = act_cfg['option'] -
> cfg_val = ''-                        bin_val = ''                         for i in option.split(','):
> if act_cfg['value'] in i:-                                bin_val = i+
> self.data_diff += \+                                    '\n\nBinary:          ' \+                                    +
> act_cfg['name'] + ': ' \+                                    + i + '\n'                             elif
> config_val in i:-                                cfg_val = i-                        if cfg_val != '' and
> bin_val != '':-                            self.data_diff += '\n\nBinary:        ' \-
> + act_cfg['name'] \-                                + ': ' + bin_val.replace(' ', '') \-
> + '\nConfig file:   ' \-                                + act_cfg['name'] + ': ' \-
> + cfg_val.replace(' ', '') + '\n'+                                self.data_diff += \+
> '\nConfig file:     ' \+                                    + act_cfg['name'] + ': ' + i
> else:-                        self.data_diff += '\n\nBinary:        ' \+
> self.data_diff += '\n\nBinary:           ' \                             + act_cfg['name'] + ': ' +
> act_cfg['value'] \-                            + '\nConfig file:   ' + act_cfg['name'] \+
> + '\nConfig file:     ' + act_cfg['name'] \                             + ': ' + config_val + '\n'
> def set_field_value(self, top, value_bytes, force=False):@@ -1477,33 +1473,67
> @@ for '%s' !" % (act_cfg['value'], act_cfg['path']))
>      def get_bin_segment(self, bin_data):         cfg_segs = self.get_cfg_segment()
> bin_segs = []+        fsp_instance = []         for seg in cfg_segs:             key =
> seg[0].encode()+            print("key ", key)             if key == 0:
> bin_segs.append([seg[0], 0, len(bin_data)])                 break             pos =
> bin_data.find(key)-            if pos >= 0:-                # ensure no other match for the
> key-                next_pos = bin_data.find(key, pos + len(seg[0]))-                if
> next_pos >= 0:-                    if key == b'$SKLFSP$' or key == b'$BSWFSP$':-
> string = ('Warning: Multiple matches for %s in '-                                  'binary!\n\nA
> workaround applied to such '-                                  'FSP 1.x binary to use second'-
> ' match instead of first match!' % key)-
> messagebox.showwarning('Warning!', string)-                        pos = next_pos-
> else:-                        print("Warning: Multiple matches for '%s' "-
> "in binary, the 1st instance will be used !"-                              % seg[0])-
> bin_segs.append([seg[0], pos, seg[2]])-                self.binseg_dict[seg[0]] = pos-
> else:+            while pos != -1:+                fsp_instance.append(pos)+                pos =
> bin_data.find(key, pos + len(seg[0]))+            if len(fsp_instance) <= 0:
> bin_segs.append([seg[0], -1, seg[2]])                 self.binseg_dict[seg[0]] = -1-
> continue++            elif len(fsp_instance) == 1:+                bin_segs.append([seg[0],
> fsp_instance[0], seg[2]])+                self.binseg_dict[seg[0]] = fsp_instance[0]+
> fsp_instance.clear()++            else:++                fsp_instance_root = tkinter.Tk()++
> canvas1 = tkinter.Canvas(fsp_instance_root,+                                         width=400,
> height=400)+                canvas1.pack()++                entry1 =
> tkinter.Entry(fsp_instance_root)+                canvas1.create_window(200, 220,
> window=entry1)++                text = "Multiple instances available for " +\+
> seg[0] + "\n\nThe available instances are\n"+                for edx, ins in
> enumerate(fsp_instance):+                    text += "\nInstance" + str(edx + 1) +
> ' :offset  ' +\+                         str(hex(ins))+                text += "\n\nPlease enter the
> instance number between 1 and "\+                    + str(len(fsp_instance))+
> label1 = tkinter.Label(+                    fsp_instance_root,+                    text=text,
> wraplength=380, justify='left')+                canvas1.create_window(200, 90,
> window=label1)++                def getfspinstance():+                    x1 = entry1.get()+
> fsp_instance_option = int(x1)+                    if fsp_instance_option <=
> len(fsp_instance):+                        bin_segs.append([seg[0],+
> fsp_instance[fsp_instance_option - 1],+                                        seg[2]])+
> self.binseg_dict[seg[0]] = fsp_instance[+                            fsp_instance_option -
> 1]+                    fsp_instance_root.destroy()++                button2 =
> tkinter.Button(fsp_instance_root,+                                         text='Enter the
> instance',+                                         command=lambda: getfspinstance())+
> canvas1.create_window(200, 250, window=button2)+
> fsp_instance_root.wait_window(fsp_instance_root)++
> fsp_instance.clear()          return bin_segs --
> 2.26.2.windows.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] IntelFsp2Pkg: Add support for config editor to handle multiple UPD
  2021-10-18  5:10 ` Chiu, Chasel
@ 2021-10-18  5:44   ` Tung Lun
  0 siblings, 0 replies; 3+ messages in thread
From: Tung Lun @ 2021-10-18  5:44 UTC (permalink / raw)
  To: Chiu, Chasel, devel@edk2.groups.io
  Cc: Ma, Maurice, Desimone, Nathaniel L, Zeng, Star

Sure Chasel, thanks for the feedback. Will work on it.

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com> 
Sent: Monday, October 18, 2021 1:10 PM
To: Loo, Tung Lun <tung.lun.loo@intel.com>; devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH] IntelFsp2Pkg: Add support for config editor to handle multiple UPD


Hi Tung Lun,

Should we update ConfigEditorUserManual.md for this new function?

Thanks,
Chasel


> -----Original Message-----
> From: Loo, Tung Lun <tung.lun.loo@intel.com>
> Sent: Friday, October 15, 2021 8:39 AM
> To: devel@edk2.groups.io
> Cc: Loo, Tung Lun <tung.lun.loo@intel.com>; Ma, Maurice 
> <maurice.ma@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; 
> Chiu, Chasel <chasel.chiu@intel.com>
> Subject: [PATCH] IntelFsp2Pkg: Add support for config editor to handle 
> multiple UPD
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3692
> 
> In several use cases in bootloader, there are multiple instances of 
> UPD with same signature header. As such, using previous version of 
> config editor to edit those will result in only overriding the first 
> found instance. This patch provides the flexibility to modify the instance specified.
> 
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> 
> Signed-off-by: Loo Tung Lun <tung.lun.loo@intel.com>
> ---
>  IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py | 94
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> ---------------------------
>  1 file changed, 62 insertions(+), 32 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
> b/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
> index b593885807..91c4180085 100644
> --- a/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
> +++ b/IntelFsp2Pkg/Tools/ConfigEditor/GenYamlCfg.py
> @@ -1351,24 +1351,20 @@ option format '%s' !" % option)
>                                  act_cfg['value']                         option = act_cfg['option'] -
> cfg_val = ''-                        bin_val = ''                         for i in option.split(','):
> if act_cfg['value'] in i:-                                bin_val = i+
> self.data_diff += \+                                    '\n\nBinary:          ' \+                                    +
> act_cfg['name'] + ': ' \+                                    + i + '\n'                             elif
> config_val in i:-                                cfg_val = i-                        if cfg_val != '' and
> bin_val != '':-                            self.data_diff += '\n\nBinary:        ' \-
> + act_cfg['name'] \-                                + ': ' + bin_val.replace(' ', '') \-
> + '\nConfig file:   ' \-                                + act_cfg['name'] + ': ' \-
> + cfg_val.replace(' ', '') + '\n'+                                self.data_diff += \+
> '\nConfig file:     ' \+                                    + act_cfg['name'] + ': ' + i
> else:-                        self.data_diff += '\n\nBinary:        ' \+
> self.data_diff += '\n\nBinary:           ' \                             + act_cfg['name'] + ': ' +
> act_cfg['value'] \-                            + '\nConfig file:   ' + act_cfg['name'] \+
> + '\nConfig file:     ' + act_cfg['name'] \                             + ': ' + config_val + '\n'
> def set_field_value(self, top, value_bytes, force=False):@@ -1477,33 
> +1473,67 @@ for '%s' !" % (act_cfg['value'], act_cfg['path']))
>      def get_bin_segment(self, bin_data):         cfg_segs = self.get_cfg_segment()
> bin_segs = []+        fsp_instance = []         for seg in cfg_segs:             key =
> seg[0].encode()+            print("key ", key)             if key == 0:
> bin_segs.append([seg[0], 0, len(bin_data)])                 break             pos =
> bin_data.find(key)-            if pos >= 0:-                # ensure no other match for the
> key-                next_pos = bin_data.find(key, pos + len(seg[0]))-                if
> next_pos >= 0:-                    if key == b'$SKLFSP$' or key == b'$BSWFSP$':-
> string = ('Warning: Multiple matches for %s in '-                                  'binary!\n\nA
> workaround applied to such '-                                  'FSP 1.x binary to use second'-
> ' match instead of first match!' % key)-
> messagebox.showwarning('Warning!', string)-                        pos = next_pos-
> else:-                        print("Warning: Multiple matches for '%s' "-
> "in binary, the 1st instance will be used !"-                              % seg[0])-
> bin_segs.append([seg[0], pos, seg[2]])-                self.binseg_dict[seg[0]] = pos-
> else:+            while pos != -1:+                fsp_instance.append(pos)+                pos =
> bin_data.find(key, pos + len(seg[0]))+            if len(fsp_instance) <= 0:
> bin_segs.append([seg[0], -1, seg[2]])                 self.binseg_dict[seg[0]] = -1-
> continue++            elif len(fsp_instance) == 1:+                bin_segs.append([seg[0],
> fsp_instance[0], seg[2]])+                self.binseg_dict[seg[0]] = fsp_instance[0]+
> fsp_instance.clear()++            else:++                fsp_instance_root = tkinter.Tk()++
> canvas1 = tkinter.Canvas(fsp_instance_root,+                                         width=400,
> height=400)+                canvas1.pack()++                entry1 =
> tkinter.Entry(fsp_instance_root)+                canvas1.create_window(200, 220,
> window=entry1)++                text = "Multiple instances available for " +\+
> seg[0] + "\n\nThe available instances are\n"+                for edx, ins in
> enumerate(fsp_instance):+                    text += "\nInstance" + str(edx + 1) +
> ' :offset  ' +\+                         str(hex(ins))+                text += "\n\nPlease enter the
> instance number between 1 and "\+                    + str(len(fsp_instance))+
> label1 = tkinter.Label(+                    fsp_instance_root,+                    text=text,
> wraplength=380, justify='left')+                canvas1.create_window(200, 90,
> window=label1)++                def getfspinstance():+                    x1 = entry1.get()+
> fsp_instance_option = int(x1)+                    if fsp_instance_option <=
> len(fsp_instance):+                        bin_segs.append([seg[0],+
> fsp_instance[fsp_instance_option - 1],+                                        seg[2]])+
> self.binseg_dict[seg[0]] = fsp_instance[+                            fsp_instance_option -
> 1]+                    fsp_instance_root.destroy()++                button2 =
> tkinter.Button(fsp_instance_root,+                                         text='Enter the
> instance',+                                         command=lambda: getfspinstance())+
> canvas1.create_window(200, 250, window=button2)+ 
> fsp_instance_root.wait_window(fsp_instance_root)++
> fsp_instance.clear()          return bin_segs --
> 2.26.2.windows.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-10-18  5:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-15  0:38 [PATCH] IntelFsp2Pkg: Add support for config editor to handle multiple UPD Tung Lun
2021-10-18  5:10 ` Chiu, Chasel
2021-10-18  5:44   ` Tung Lun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox