[PATCH 08/22] gdbstub: Add num_regs member to GDBFeature

Alex Bennée posted 22 patches 1 year ago
Maintainers: Laurent Vivier <laurent@vivier.eu>, Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, Richard Henderson <richard.henderson@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, Chris Wulff <crwulff@gmail.com>, Marek Vasut <marex@denx.de>
[PATCH 08/22] gdbstub: Add num_regs member to GDBFeature
Posted by Alex Bennée 1 year ago
From: Akihiko Odaki <akihiko.odaki@daynix.com>

Currently the number of registers exposed to GDB is written as magic
numbers in code. Derive the number of registers GDB actually see from
XML files to replace the magic numbers in code later.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231025093128.33116-2-akihiko.odaki@daynix.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20231103195956.1998255-9-alex.bennee@linaro.org>
---
 include/exec/gdbstub.h  |  1 +
 scripts/feature_to_c.py | 46 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 1a01c35f8e..a43aa34dad 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -13,6 +13,7 @@
 typedef struct GDBFeature {
     const char *xmlname;
     const char *xml;
+    int num_regs;
 } GDBFeature;
 
 
diff --git a/scripts/feature_to_c.py b/scripts/feature_to_c.py
index bcbcb83beb..e04d6b2df7 100644
--- a/scripts/feature_to_c.py
+++ b/scripts/feature_to_c.py
@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 # SPDX-License-Identifier: GPL-2.0-or-later
 
-import os, sys
+import os, sys, xml.etree.ElementTree
 
 def writeliteral(indent, bytes):
     sys.stdout.write(' ' * indent)
@@ -39,10 +39,52 @@ def writeliteral(indent, bytes):
     with open(input, 'rb') as file:
         read = file.read()
 
+    parser = xml.etree.ElementTree.XMLPullParser(['start', 'end'])
+    parser.feed(read)
+    events = parser.read_events()
+    event, element = next(events)
+    if event != 'start':
+        sys.stderr.write(f'unexpected event: {event}\n')
+        exit(1)
+    if element.tag != 'feature':
+        sys.stderr.write(f'unexpected start tag: {element.tag}\n')
+        exit(1)
+
+    regnum = 0
+    regnums = []
+    tags = ['feature']
+    for event, element in events:
+        if event == 'end':
+            if element.tag != tags[len(tags) - 1]:
+                sys.stderr.write(f'unexpected end tag: {element.tag}\n')
+                exit(1)
+
+            tags.pop()
+            if element.tag == 'feature':
+                break
+        elif event == 'start':
+            if len(tags) < 2 and element.tag == 'reg':
+                if 'regnum' in element.attrib:
+                    regnum = int(element.attrib['regnum'])
+
+                regnums.append(regnum)
+                regnum += 1
+
+            tags.append(element.tag)
+        else:
+            raise Exception(f'unexpected event: {event}\n')
+
+    if len(tags):
+        sys.stderr.write('unterminated feature tag\n')
+        exit(1)
+
+    base_reg = min(regnums)
+    num_regs = max(regnums) - base_reg + 1 if len(regnums) else 0
+
     sys.stdout.write('    {\n')
     writeliteral(8, bytes(os.path.basename(input), 'utf-8'))
     sys.stdout.write(',\n')
     writeliteral(8, read)
-    sys.stdout.write('\n    },\n')
+    sys.stdout.write(f',\n        {num_regs},\n    }},\n')
 
 sys.stdout.write('    { NULL }\n};\n')
-- 
2.39.2


Re: [PATCH 08/22] gdbstub: Add num_regs member to GDBFeature
Posted by Philippe Mathieu-Daudé 1 year ago
Hi Alex,

On 6/11/23 19:50, Alex Bennée wrote:
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> 
> Currently the number of registers exposed to GDB is written as magic
> numbers in code. Derive the number of registers GDB actually see from
> XML files to replace the magic numbers in code later.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231025093128.33116-2-akihiko.odaki@daynix.com>

Something in your workflow is odd, you should keep this Message-Id,

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231103195956.1998255-9-alex.bennee@linaro.org>

and not propagate this one, IMO.

> ---
>   include/exec/gdbstub.h  |  1 +
>   scripts/feature_to_c.py | 46 +++++++++++++++++++++++++++++++++++++++--
>   2 files changed, 45 insertions(+), 2 deletions(-)



Re: [PATCH 08/22] gdbstub: Add num_regs member to GDBFeature
Posted by Alex Bennée 1 year ago
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Hi Alex,
>
> On 6/11/23 19:50, Alex Bennée wrote:
>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>> Currently the number of registers exposed to GDB is written as magic
>> numbers in code. Derive the number of registers GDB actually see from
>> XML files to replace the magic numbers in code later.
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20231025093128.33116-2-akihiko.odaki@daynix.com>
>
> Something in your workflow is odd, you should keep this Message-Id,
>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20231103195956.1998255-9-alex.bennee@linaro.org>
>
> and not propagate this one, IMO.

Why not - it tracks all the review stuff. I explicitly keep on
Message-Id per domain so we see the original posting and the last time
it was posted (which you can then follow the chain of reviews from
there).

If we want to have an explicit policy on which Message-Id's to keep then
we should document it.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH 08/22] gdbstub: Add num_regs member to GDBFeature
Posted by Philippe Mathieu-Daudé 1 year ago
On 7/11/23 11:24, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Hi Alex,
>>
>> On 6/11/23 19:50, Alex Bennée wrote:
>>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> Currently the number of registers exposed to GDB is written as magic
>>> numbers in code. Derive the number of registers GDB actually see from
>>> XML files to replace the magic numbers in code later.
>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>> Message-Id: <20231025093128.33116-2-akihiko.odaki@daynix.com>
>>
>> Something in your workflow is odd, you should keep this Message-Id,
>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Message-Id: <20231103195956.1998255-9-alex.bennee@linaro.org>
>>
>> and not propagate this one, IMO.
> 
> Why not - it tracks all the review stuff. I explicitly keep on
> Message-Id per domain so we see the original posting and the last time
> it was posted (which you can then follow the chain of reviews from
> there).
> 
> If we want to have an explicit policy on which Message-Id's to keep then
> we should document it.

Fair enough :)