[PATCH v1 0/7] xentrace changes

[PATCH v1 0/7] xentrace changes

[Olaf Hering]

Olaf Hering (7):
  xentrace: remove unimplemented option from man page
  xentrace: use correct output format for pit and rtc
  xenalyze: sync with vmx.h, use EXIT_REASON_PENDING_VIRT_INTR
  xenalyze: sync with vmx.h, use EXIT_REASON_VMXOFF
  xenalyze: sync with vmx.h, use EXIT_REASON_VMXON
  xenalyze: sync with vmx.h, use EXIT_REASON_MCE_DURING_VMENTRY
  xenalyze: handle more potential exit reason values from vmx.h

 docs/man/xentrace.8.pod   |  4 ----
 tools/xentrace/formats    |  4 ++--
 tools/xentrace/xenalyze.c | 46 ++++++++++++++++++++++++++++-----------
 3 files changed, 35 insertions(+), 19 deletions(-)

[PATCH v1 1/7] xentrace: remove unimplemented option from man page

[Olaf Hering]

The documented option --usage worked because every unknown option
showed the help.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 docs/man/xentrace.8.pod | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/docs/man/xentrace.8.pod b/docs/man/xentrace.8.pod
index 69aef05f65..4c174a84c0 100644
--- a/docs/man/xentrace.8.pod
+++ b/docs/man/xentrace.8.pod
@@ -69,10 +69,6 @@ set event capture mask. If not specified the TRC_ALL will be used.
 
 =item B<-?>, B<--help>
 
-Give this help list
-
-=item B<--usage>
-
 Give a short usage message
 
 =item B<-V>, B<--version>

[PATCH v1 2/7] xentrace: use correct output format for pit and rtc

[Olaf Hering]

The input values were always 32bit.

Fixes 55ee5dea32 ("xentrace: add TRC_HVM_EMUL")

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/xentrace/formats | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/xentrace/formats b/tools/xentrace/formats
index 0fcc327a40..afb5ee0112 100644
--- a/tools/xentrace/formats
+++ b/tools/xentrace/formats
@@ -211,8 +211,8 @@
 0x00802008  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  do_irq [ irq = %(1)d, began = %(2)dus, ended = %(3)dus ]
 
 0x00084001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  hpet create [ tn = %(1)d, irq = %(2)d, delta = 0x%(4)08x%(3)08x, period = 0x%(6)08x%(5)08x ]
-0x00084002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  pit create [ delta = 0x%(1)016x, period = 0x%(2)016x ]
-0x00084003  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtc create [ delta = 0x%(1)016x , period = 0x%(2)016x ]
+0x00084002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  pit create [ delta = 0x%(1)08x, period = 0x%(2)08x ]
+0x00084003  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtc create [ delta = 0x%(1)08x, period = 0x%(2)08x ]
 0x00084004  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  vlapic create [ delta = 0x%(2)08x%(1)08x , period = 0x%(4)08x%(3)08x, irq = %(5)d ]
 0x00084005  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  hpet destroy [ tn = %(1)d ]
 0x00084006  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  pit destroy  [ ]

[PATCH v1 3/7] xenalyze: sync with vmx.h, use EXIT_REASON_PENDING_VIRT_INTR

[Olaf Hering]

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/xentrace/xenalyze.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index a50538e9a8..1cae055ef4 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -447,7 +447,7 @@ struct {
 #define EXIT_REASON_SIPI                4
 #define EXIT_REASON_IO_SMI              5
 #define EXIT_REASON_OTHER_SMI           6
-#define EXIT_REASON_PENDING_INTERRUPT   7
+#define EXIT_REASON_PENDING_VIRT_INTR   7
 #define EXIT_REASON_PENDING_VIRT_NMI    8
 #define EXIT_REASON_TASK_SWITCH         9
 #define EXIT_REASON_CPUID               10
@@ -503,7 +503,7 @@ const char * hvm_vmx_exit_reason_name[HVM_VMX_EXIT_REASON_MAX] = {
     [EXIT_REASON_SIPI]="SIPI",
     [EXIT_REASON_IO_SMI]="IO_SMI",
     [EXIT_REASON_OTHER_SMI]="OTHER_SMI",
-    [EXIT_REASON_PENDING_INTERRUPT]="PENDING_INTERRUPT",
+    [EXIT_REASON_PENDING_VIRT_INTR]="PENDING_VIRT_INTR",
     [EXIT_REASON_PENDING_VIRT_NMI]="PENDING_VIRT_NMI",
     [EXIT_REASON_TASK_SWITCH]="TASK_SWITCH",
     [EXIT_REASON_CPUID]="CPUID",
@@ -4632,7 +4632,7 @@ void hvm_generic_postprocess(struct hvm_data *h)
             switch(h->exit_reason)
             {
                 /* These just need us to go through the return path */
-            case EXIT_REASON_PENDING_INTERRUPT:
+            case EXIT_REASON_PENDING_VIRT_INTR:
             case EXIT_REASON_TPR_BELOW_THRESHOLD:
                 /* Not much to log now; may need later */
             case EXIT_REASON_WBINVD:

[PATCH v1 4/7] xenalyze: sync with vmx.h, use EXIT_REASON_VMXOFF

[Olaf Hering]

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/xentrace/xenalyze.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index 1cae055ef4..d2e6c77590 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -466,7 +466,7 @@ struct {
 #define EXIT_REASON_VMREAD              23
 #define EXIT_REASON_VMRESUME            24
 #define EXIT_REASON_VMWRITE             25
-#define EXIT_REASON_VMOFF               26
+#define EXIT_REASON_VMXOFF              26
 #define EXIT_REASON_VMON                27
 #define EXIT_REASON_CR_ACCESS           28
 #define EXIT_REASON_DR_ACCESS           29
@@ -522,7 +522,7 @@ const char * hvm_vmx_exit_reason_name[HVM_VMX_EXIT_REASON_MAX] = {
     [EXIT_REASON_VMREAD]="VMREAD",
     [EXIT_REASON_VMRESUME]="VMRESUME",
     [EXIT_REASON_VMWRITE]="VMWRITE",
-    [EXIT_REASON_VMOFF]="VMOFF",
+    [EXIT_REASON_VMXOFF]="VMXOFF",
     [EXIT_REASON_VMON]="VMON",
     [EXIT_REASON_CR_ACCESS]="CR_ACCESS",
     [EXIT_REASON_DR_ACCESS]="DR_ACCESS",

[PATCH v1 5/7] xenalyze: sync with vmx.h, use EXIT_REASON_VMXON

[Olaf Hering]

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/xentrace/xenalyze.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index d2e6c77590..88c3d5f873 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -467,7 +467,7 @@ struct {
 #define EXIT_REASON_VMRESUME            24
 #define EXIT_REASON_VMWRITE             25
 #define EXIT_REASON_VMXOFF              26
-#define EXIT_REASON_VMON                27
+#define EXIT_REASON_VMXON               27
 #define EXIT_REASON_CR_ACCESS           28
 #define EXIT_REASON_DR_ACCESS           29
 #define EXIT_REASON_IO_INSTRUCTION      30
@@ -523,7 +523,7 @@ const char * hvm_vmx_exit_reason_name[HVM_VMX_EXIT_REASON_MAX] = {
     [EXIT_REASON_VMRESUME]="VMRESUME",
     [EXIT_REASON_VMWRITE]="VMWRITE",
     [EXIT_REASON_VMXOFF]="VMXOFF",
-    [EXIT_REASON_VMON]="VMON",
+    [EXIT_REASON_VMXON]="VMXON",
     [EXIT_REASON_CR_ACCESS]="CR_ACCESS",
     [EXIT_REASON_DR_ACCESS]="DR_ACCESS",
     [EXIT_REASON_IO_INSTRUCTION]="IO_INSTRUCTION",

[PATCH v1 6/7] xenalyze: sync with vmx.h, use EXIT_REASON_MCE_DURING_VMENTRY

[Olaf Hering]

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/xentrace/xenalyze.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index 88c3d5f873..9635ff453a 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -479,7 +479,7 @@ struct {
 #define EXIT_REASON_MONITOR_TRAP_FLAG   37
 #define EXIT_REASON_MONITOR_INSTRUCTION 39
 #define EXIT_REASON_PAUSE_INSTRUCTION   40
-#define EXIT_REASON_MACHINE_CHECK       41
+#define EXIT_REASON_MCE_DURING_VMENTRY  41
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
 #define EXIT_REASON_APIC_ACCESS         44
 #define EXIT_REASON_ACCESS_GDTR_OR_IDTR 46
@@ -535,7 +535,7 @@ const char * hvm_vmx_exit_reason_name[HVM_VMX_EXIT_REASON_MAX] = {
     [EXIT_REASON_MONITOR_TRAP_FLAG]="MONITOR_TRAP_FLAG",
     [EXIT_REASON_MONITOR_INSTRUCTION]="MONITOR_INSTRUCTION",
     [EXIT_REASON_PAUSE_INSTRUCTION]="PAUSE_INSTRUCTION",
-    [EXIT_REASON_MACHINE_CHECK]="MACHINE_CHECK",
+    [EXIT_REASON_MCE_DURING_VMENTRY]="MCE_DURING_VMENTRY",
     [EXIT_REASON_TPR_BELOW_THRESHOLD]="TPR_BELOW_THRESHOLD",
     [EXIT_REASON_APIC_ACCESS]="APIC_ACCESS",
     [EXIT_REASON_EPT_VIOLATION]="EPT_VIOLATION",

[PATCH v1 7/7] xenalyze: handle more potential exit reason values from vmx.h

[Olaf Hering]

Copy and use more constants from vmx.h, to turn numbers into strings.
Adjust the REASON_MAX value accordingly.
Remove the size constraint from string array, the compiler will grow it
as needed.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/xentrace/xenalyze.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
index 9635ff453a..9af17d45bf 100644
--- a/tools/xentrace/xenalyze.c
+++ b/tools/xentrace/xenalyze.c
@@ -482,6 +482,7 @@ struct {
 #define EXIT_REASON_MCE_DURING_VMENTRY  41
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
 #define EXIT_REASON_APIC_ACCESS         44
+#define EXIT_REASON_EOI_INDUCED         45
 #define EXIT_REASON_ACCESS_GDTR_OR_IDTR 46
 #define EXIT_REASON_ACCESS_LDTR_OR_TR   47
 #define EXIT_REASON_EPT_VIOLATION       48
@@ -492,10 +493,18 @@ struct {
 #define EXIT_REASON_INVVPID             53
 #define EXIT_REASON_WBINVD              54
 #define EXIT_REASON_XSETBV              55
-
-#define HVM_VMX_EXIT_REASON_MAX (EXIT_REASON_XSETBV+1)
-
-const char * hvm_vmx_exit_reason_name[HVM_VMX_EXIT_REASON_MAX] = {
+#define EXIT_REASON_APIC_WRITE          56
+#define EXIT_REASON_INVPCID             58
+#define EXIT_REASON_VMFUNC              59
+#define EXIT_REASON_PML_FULL            62
+#define EXIT_REASON_XSAVES              63
+#define EXIT_REASON_XRSTORS             64
+#define EXIT_REASON_BUS_LOCK            74
+#define EXIT_REASON_NOTIFY              75
+
+#define HVM_VMX_EXIT_REASON_MAX (EXIT_REASON_NOTIFY+1)
+
+const char * hvm_vmx_exit_reason_name[] = {
     [EXIT_REASON_EXCEPTION_NMI]="EXCEPTION_NMI",
     [EXIT_REASON_EXTERNAL_INTERRUPT]="EXTERNAL_INTERRUPT",
     [EXIT_REASON_TRIPLE_FAULT]="TRIPLE_FAULT",
@@ -538,6 +547,9 @@ const char * hvm_vmx_exit_reason_name[HVM_VMX_EXIT_REASON_MAX] = {
     [EXIT_REASON_MCE_DURING_VMENTRY]="MCE_DURING_VMENTRY",
     [EXIT_REASON_TPR_BELOW_THRESHOLD]="TPR_BELOW_THRESHOLD",
     [EXIT_REASON_APIC_ACCESS]="APIC_ACCESS",
+    [EXIT_REASON_EOI_INDUCED]="EOI_INDUCED",
+    [EXIT_REASON_ACCESS_GDTR_OR_IDTR]="ACCESS_GDTR_OR_IDTR",
+    [EXIT_REASON_ACCESS_LDTR_OR_TR]="ACCESS_LDTR_OR_TR",
     [EXIT_REASON_EPT_VIOLATION]="EPT_VIOLATION",
     [EXIT_REASON_EPT_MISCONFIG]="EPT_MISCONFIG",
     [EXIT_REASON_INVEPT]="INVEPT",
@@ -546,6 +558,14 @@ const char * hvm_vmx_exit_reason_name[HVM_VMX_EXIT_REASON_MAX] = {
     [EXIT_REASON_INVVPID]="INVVPID",
     [EXIT_REASON_WBINVD]="WBINVD",
     [EXIT_REASON_XSETBV]="XSETBV",
+    [EXIT_REASON_APIC_WRITE]="APIC_WRITE",
+    [EXIT_REASON_INVPCID]="INVPCID",
+    [EXIT_REASON_VMFUNC]="VMFUNC",
+    [EXIT_REASON_PML_FULL]="PML_FULL",
+    [EXIT_REASON_XSAVES]="XSAVES",
+    [EXIT_REASON_XRSTORS]="XRSTORS",
+    [EXIT_REASON_BUS_LOCK]="BUS_LOCK",
+    [EXIT_REASON_NOTIFY]="NOTIFY",
 };
 
 /* SVM data */

Re: [PATCH v1 2/7] xentrace: use correct output format for pit and rtc

[George Dunlap]

[-- Attachment #1: Type: text/plain, Size: 1793 bytes --]

Sorry for the delay here.

Just a couple of notes on the whole series.  First, you didn't CC me on 0/7
or 1/7.  Secondly, patch 7 seems to be corrupted (see e.g., Patchew's
attempt to apply the series [1]).

[1] https://patchew.org/Xen/20230601142742.15489-1-olaf@aepfle.de/

 -George

On Thu, Jun 1, 2023 at 3:28 PM Olaf Hering <olaf@aepfle.de> wrote:

> The input values were always 32bit.
>
> Fixes 55ee5dea32 ("xentrace: add TRC_HVM_EMUL")
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  tools/xentrace/formats | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/xentrace/formats b/tools/xentrace/formats
> index 0fcc327a40..afb5ee0112 100644
> --- a/tools/xentrace/formats
> +++ b/tools/xentrace/formats
> @@ -211,8 +211,8 @@
>  0x00802008  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  do_irq [ irq = %(1)d,
> began = %(2)dus, ended = %(3)dus ]
>
>  0x00084001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  hpet create [ tn = %(1)d,
> irq = %(2)d, delta = 0x%(4)08x%(3)08x, period = 0x%(6)08x%(5)08x ]
> -0x00084002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  pit create [ delta =
> 0x%(1)016x, period = 0x%(2)016x ]
> -0x00084003  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtc create [ delta =
> 0x%(1)016x , period = 0x%(2)016x ]
> +0x00084002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  pit create [ delta =
> 0x%(1)08x, period = 0x%(2)08x ]
> +0x00084003  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtc create [ delta =
> 0x%(1)08x, period = 0x%(2)08x ]
>  0x00084004  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  vlapic create [ delta =
> 0x%(2)08x%(1)08x , period = 0x%(4)08x%(3)08x, irq = %(5)d ]
>  0x00084005  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  hpet destroy [ tn = %(1)d
> ]
>  0x00084006  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  pit destroy  [ ]
>

[-- Attachment #2: Type: text/html, Size: 2360 bytes --]

Re: [PATCH v1 2/7] xentrace: use correct output format for pit and rtc

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]

Fri, 28 Jul 2023 17:47:11 +0100 George Dunlap <george.dunlap@cloud.com>:

> Just a couple of notes on the whole series.  First, you didn't CC me on 0/7
> or 1/7.  Secondly, patch 7 seems to be corrupted (see e.g., Patchew's
> attempt to apply the series [1]).

The cover letter can not be easily fixed, this is a bug in git send-email.
It should be able to collect all recipients and put them in the cover letter.
I think I already pointed that out a while ago on git-devel...

I think #1 can be fixed by adjusting the MAINTAINERS file.
Not sure why #7 fails to apply.

I will rebase and resend next week.


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 2/7] xentrace: use correct output format for pit and rtc

[George Dunlap]

[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]

On Fri, Jul 28, 2023 at 6:52 PM Olaf Hering <olaf@aepfle.de> wrote:

> Fri, 28 Jul 2023 17:47:11 +0100 George Dunlap <george.dunlap@cloud.com>:
>
> > Just a couple of notes on the whole series.  First, you didn't CC me on
> 0/7
> > or 1/7.  Secondly, patch 7 seems to be corrupted (see e.g., Patchew's
> > attempt to apply the series [1]).
>
> The cover letter can not be easily fixed, this is a bug in git send-email.
> It should be able to collect all recipients and put them in the cover
> letter.
> I think I already pointed that out a while ago on git-devel...
>
> I think #1 can be fixed by adjusting the MAINTAINERS file.
> Not sure why #7 fails to apply.
>

Running `git am --show-current-patch` reveals a tail that looks like this:

```
+    [EXIT_REASON_NOTIFY]=3D"NOTIFY",
 };
=20
```

This may again be a known issue with `git am` and base64-encoded files: I
think `mailsplit` knows how to deal with these sorts of line breaks; and
`git am` also knows how to decode base64; but it does them in the wrong
order, so base64-encoded mails don't get handled properly.  I reported it
to them several years ago [1], and nothing happened.

 -George

[1]
https://lore.kernel.org/git/c44c3958-b0eb-22bd-bc35-04982706162f@citrix.com/

[-- Attachment #2: Type: text/html, Size: 1946 bytes --]

Re: [PATCH v1 1/7] xentrace: remove unimplemented option from man page

[George Dunlap]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

On Thu, Jun 1, 2023 at 3:28 PM Olaf Hering <olaf@aepfle.de> wrote:

> The documented option --usage worked because every unknown option
> showed the help.
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>

Reviewed-by: George Dunlap <george.dunlap@cloud.com>

[-- Attachment #2: Type: text/html, Size: 717 bytes --]

Re: [PATCH v1 2/7] xentrace: use correct output format for pit and rtc

[George Dunlap]

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

On Thu, Jun 1, 2023 at 3:28 PM Olaf Hering <olaf@aepfle.de> wrote:

> The input values were always 32bit.
>
> Fixes 55ee5dea32 ("xentrace: add TRC_HVM_EMUL")
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  tools/xentrace/formats | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/xentrace/formats b/tools/xentrace/formats
> index 0fcc327a40..afb5ee0112 100644
> --- a/tools/xentrace/formats
> +++ b/tools/xentrace/formats
> @@ -211,8 +211,8 @@
>  0x00802008  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  do_irq [ irq = %(1)d,
> began = %(2)dus, ended = %(3)dus ]
>
>  0x00084001  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  hpet create [ tn = %(1)d,
> irq = %(2)d, delta = 0x%(4)08x%(3)08x, period = 0x%(6)08x%(5)08x ]
> -0x00084002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  pit create [ delta =
> 0x%(1)016x, period = 0x%(2)016x ]
> -0x00084003  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtc create [ delta =
> 0x%(1)016x , period = 0x%(2)016x ]
> +0x00084002  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  pit create [ delta =
> 0x%(1)08x, period = 0x%(2)08x ]
> +0x00084003  CPU%(cpu)d  %(tsc)d (+%(reltsc)8d)  rtc create [ delta =
> 0x%(1)08x, period = 0x%(2)08x ]
>

Looks good.  While we're here, any reason not to s/create/start/; to match
up with the trace definition?

Either way:

Reviewed-by: George Dunlap <george.dunlap@cloud.com>

[-- Attachment #2: Type: text/html, Size: 1919 bytes --]

Re: [PATCH v1 3/7] xenalyze: sync with vmx.h, use EXIT_REASON_PENDING_VIRT_INTR

[George Dunlap]

[-- Attachment #1: Type: text/plain, Size: 290 bytes --]

On Thu, Jun 1, 2023 at 3:28 PM Olaf Hering <olaf@aepfle.de> wrote:

> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>

Reviewed-by: George Dunlap <george.dunlap@cloud.com>

One can't help wondering if it would be better to arrange somehow not to
have these definitions duplicated.

[-- Attachment #2: Type: text/html, Size: 742 bytes --]

Re: [PATCH v1 6/7] xenalyze: sync with vmx.h, use EXIT_REASON_MCE_DURING_VMENTRY

[George Dunlap]

[-- Attachment #1: Type: text/plain, Size: 194 bytes --]

On Thu, Jun 1, 2023 at 3:28 PM Olaf Hering <olaf@aepfle.de> wrote:

> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>

Patches 4-7:

Reviewed-by: George Dunlap <george.dunlap@cloud.com>

[-- Attachment #2: Type: text/html, Size: 643 bytes --]

Re: [PATCH v1 7/7] xenalyze: handle more potential exit reason values from vmx.h

[George Dunlap]

[-- Attachment #1: Type: text/plain, Size: 928 bytes --]

On Thu, Jun 1, 2023 at 3:28 PM Olaf Hering <olaf@aepfle.de> wrote:

> Copy and use more constants from vmx.h, to turn numbers into strings.
> Adjust the REASON_MAX value accordingly.
> Remove the size constraint from string array, the compiler will grow it
> as needed.
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>

Thanks for doing this.

Everything looks good (including adding the missing strings), except for
the removal of the fixed array size.  Call me paranoid, but if we define it
as REASON_MAX length, then there will never be any way that a value less
than REASON_MAX causes a segfault (and if we assign a value higher than
REASON_MAX, the compiler will complain); whereas if we make it variable, we
leave open the possibility that someone won't update REASON_MAX properly,
resulting in either segfaults (if REASON_MAX is too high) or missing
strings (if REASON_MAX is too low).

 -George

[-- Attachment #2: Type: text/html, Size: 1373 bytes --]

Re: [PATCH v1 6/7] xenalyze: sync with vmx.h, use EXIT_REASON_MCE_DURING_VMENTRY

[George Dunlap]

[-- Attachment #1: Type: text/plain, Size: 433 bytes --]

On Fri, Jul 28, 2023 at 9:25 PM George Dunlap <george.dunlap@cloud.com>
wrote:

>
>
> On Thu, Jun 1, 2023 at 3:28 PM Olaf Hering <olaf@aepfle.de> wrote:
>
>> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>>
>
> Patches 4-7:
>
> Reviewed-by: George Dunlap <george.dunlap@cloud.com>
>

Sorry, this should have said patches 4-6.

At any rate, I've pushed patches 1-6 now.  Thanks again for the cleanup.

  -George

[-- Attachment #2: Type: text/html, Size: 1284 bytes --]

Re: [PATCH v1 7/7] xenalyze: handle more potential exit reason values from vmx.h

[Olaf Hering]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

Fri, 28 Jul 2023 21:35:54 +0100 George Dunlap <george.dunlap@cloud.com>:

> Everything looks good (including adding the missing strings), except for
> the removal of the fixed array size.  Call me paranoid, but if we define it
> as REASON_MAX length, then there will never be any way that a value less
> than REASON_MAX causes a segfault (and if we assign a value higher than
> REASON_MAX, the compiler will complain); whereas if we make it variable, we
> leave open the possibility that someone won't update REASON_MAX properly,
> resulting in either segfaults (if REASON_MAX is too high) or missing
> strings (if REASON_MAX is too low).

I think the code needs to be rearranged to work with "array_size" instead.


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]