[PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[Marcus Granado]

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

xenoprof: Make the escape code consistent across 32 and 64-bit xen

At the moment, the xenoprof escape code is defined as "~0UL".
Unfortunately, this expands to 0xffffffff on 32-bit systems
and 0xffffffffffffffff on 64-bit systems; with the result that
while 32-on-32 and 64-in-64 work fine, 32-on-64 (also known as
"compat mode") is broken.

This patch makes the definition consistent across architectures.
In so doing, it will break old-32-bit-on-new-Xen, and vice versa;
but this was seen as an acceptable thing to do.

Signed-off-by: Marcus Granado <marcus.granado@eu.citrix.com>
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>


diff -r 870db1ebc0df -r bcd2c4f0c502 xen/include/public/xenoprof.h
--- a/xen/include/public/xenoprof.h    Wed Jan 18 17:39:19 2012 +0000
+++ b/xen/include/public/xenoprof.h    Wed Jan 18 17:46:28 2012 +0000
@@ -68,7 +68,7 @@ struct event_log {
  };

  /* PC value that indicates a special code */
-#define XENOPROF_ESCAPE_CODE ~0UL
+#define XENOPROF_ESCAPE_CODE (~0ULL)
  /* Transient events for the xenoprof->oprofile cpu buf */
  #define XENOPROF_TRACE_BEGIN 1



[-- Attachment #2: xenoprof-64bit-escape-code.diff --]
[-- Type: text/x-patch, Size: 1101 bytes --]

xenoprof: Make the escape code consistent across 32 and 64-bit xen

At the moment, the xenoprof escape code is defined as "~0UL".
Unfortunately, this expands to 0xffffffff on 32-bit systems
and 0xffffffffffffffff on 64-bit systems; with the result that
while 32-on-32 and 64-in-64 work fine, 32-on-64 (also known as
"compat mode") is broken.

This patch makes the definition consistent across architectures.
In so doing, it will break old-32-bit-on-new-Xen, and vice versa;
but this was seen as an acceptable thing to do.

Signed-off-by: Marcus Granado <marcus.granado@eu.citrix.com>
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>


diff -r 870db1ebc0df -r bcd2c4f0c502 xen/include/public/xenoprof.h
--- a/xen/include/public/xenoprof.h	Wed Jan 18 17:39:19 2012 +0000
+++ b/xen/include/public/xenoprof.h	Wed Jan 18 17:46:28 2012 +0000
@@ -68,7 +68,7 @@ struct event_log {
 };
 
 /* PC value that indicates a special code */
-#define XENOPROF_ESCAPE_CODE ~0UL
+#define XENOPROF_ESCAPE_CODE (~0ULL)
 /* Transient events for the xenoprof->oprofile cpu buf */
 #define XENOPROF_TRACE_BEGIN 1
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[Jan Beulich]

>>> On 20.01.12 at 19:45, Marcus Granado <marcus.granado@citrix.com> wrote:
> xenoprof: Make the escape code consistent across 32 and 64-bit xen
> 
> At the moment, the xenoprof escape code is defined as "~0UL".
> Unfortunately, this expands to 0xffffffff on 32-bit systems
> and 0xffffffffffffffff on 64-bit systems; with the result that
> while 32-on-32 and 64-in-64 work fine, 32-on-64 (also known as
> "compat mode") is broken.
> 
> This patch makes the definition consistent across architectures.
> In so doing, it will break old-32-bit-on-new-Xen, and vice versa;
> but this was seen as an acceptable thing to do.
> 
> Signed-off-by: Marcus Granado <marcus.granado@eu.citrix.com>
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

NAK.

> diff -r 870db1ebc0df -r bcd2c4f0c502 xen/include/public/xenoprof.h
> --- a/xen/include/public/xenoprof.h    Wed Jan 18 17:39:19 2012 +0000
> +++ b/xen/include/public/xenoprof.h    Wed Jan 18 17:46:28 2012 +0000
> @@ -68,7 +68,7 @@ struct event_log {
>   };
> 
>   /* PC value that indicates a special code */
> -#define XENOPROF_ESCAPE_CODE ~0UL
> +#define XENOPROF_ESCAPE_CODE (~0ULL)

You cannot just go and change a public definition, as the consuming
side will break. You need to introduce a new escape code, and
provide a mechanism to negotiate (between hypervisor and kernel)
which one to use.

I'd also be curious to see the kernel side change accompanying this
(if trivial, it might be a good idea to commit this to the old 2.6.18
tree for future reference).

Jan

>   /* Transient events for the xenoprof->oprofile cpu buf */
>   #define XENOPROF_TRACE_BEGIN 1

Re: [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[Ian Campbell]

On Mon, 2012-01-23 at 09:57 +0000, Jan Beulich wrote:
> >>> On 20.01.12 at 19:45, Marcus Granado <marcus.granado@citrix.com> wrote:
> > xenoprof: Make the escape code consistent across 32 and 64-bit xen
> > 
> > At the moment, the xenoprof escape code is defined as "~0UL".
> > Unfortunately, this expands to 0xffffffff on 32-bit systems
> > and 0xffffffffffffffff on 64-bit systems; with the result that
> > while 32-on-32 and 64-in-64 work fine, 32-on-64 (also known as
> > "compat mode") is broken.
> > 
> > This patch makes the definition consistent across architectures.
> > In so doing, it will break old-32-bit-on-new-Xen, and vice versa;
> > but this was seen as an acceptable thing to do.
> > 
> > Signed-off-by: Marcus Granado <marcus.granado@eu.citrix.com>
> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> NAK.
> 
> > diff -r 870db1ebc0df -r bcd2c4f0c502 xen/include/public/xenoprof.h
> > --- a/xen/include/public/xenoprof.h    Wed Jan 18 17:39:19 2012 +0000
> > +++ b/xen/include/public/xenoprof.h    Wed Jan 18 17:46:28 2012 +0000
> > @@ -68,7 +68,7 @@ struct event_log {
> >   };
> > 
> >   /* PC value that indicates a special code */
> > -#define XENOPROF_ESCAPE_CODE ~0UL
> > +#define XENOPROF_ESCAPE_CODE (~0ULL)
> 
> You cannot just go and change a public definition, as the consuming
> side will break. You need to introduce a new escape code, and
> provide a mechanism to negotiate (between hypervisor and kernel)
> which one to use.

This seems more like a bug fix to me than an interface change, the fact
that both producer and consumer had the bug notwithstanding.

Since this is a developer oriented interface I think we can be a little
more relaxed than we would be for other interface changes. In this case
we are fixing 32on64 (quite common) at the expense of 32on32 (very
uncommon these days, especially for the suybset of developers wanting to
do profiling) and leaving 64on64 (quite common) as it is . That seems
like a worthwhile tradeoff to me.


Ian.

> I'd also be curious to see the kernel side change accompanying this
> (if trivial, it might be a good idea to commit this to the old 2.6.18
> tree for future reference).
> 
> Jan
> 
> >   /* Transient events for the xenoprof->oprofile cpu buf */
> >   #define XENOPROF_TRACE_BEGIN 1
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[Jan Beulich]

>>> On 23.01.12 at 11:16, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Mon, 2012-01-23 at 09:57 +0000, Jan Beulich wrote:
>> >>> On 20.01.12 at 19:45, Marcus Granado <marcus.granado@citrix.com> wrote:
>> > --- a/xen/include/public/xenoprof.h    Wed Jan 18 17:39:19 2012 +0000
>> > +++ b/xen/include/public/xenoprof.h    Wed Jan 18 17:46:28 2012 +0000
>> > @@ -68,7 +68,7 @@ struct event_log {
>> >   };
>> > 
>> >   /* PC value that indicates a special code */
>> > -#define XENOPROF_ESCAPE_CODE ~0UL
>> > +#define XENOPROF_ESCAPE_CODE (~0ULL)
>> 
>> You cannot just go and change a public definition, as the consuming
>> side will break. You need to introduce a new escape code, and
>> provide a mechanism to negotiate (between hypervisor and kernel)
>> which one to use.
> 
> This seems more like a bug fix to me than an interface change, the fact
> that both producer and consumer had the bug notwithstanding.
> 
> Since this is a developer oriented interface I think we can be a little
> more relaxed than we would be for other interface changes. In this case
> we are fixing 32on64 (quite common) at the expense of 32on32 (very
> uncommon these days, especially for the suybset of developers wanting to
> do profiling) and leaving 64on64 (quite common) as it is . That seems
> like a worthwhile tradeoff to me.

I see your point. However, I'd still like to be careful with this - what
we think things ought to be used for doesn't necessarily cover all
cases that they are in reality.

Additionally, making as exception here (where the developer nature
of the change is deemed obvious) may get us to the question of
making a similar adjustment in not as clear a situation at some point
in the future.

As negotiation isn't really difficult to implement (e.g. a new ELF note)
I don't really see why we shouldn't remain on the safe side here. But
Keir would have the final word anyway.

Jan

Re: [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[Jan Beulich]

>>> On 23.01.12 at 11:47, "Jan Beulich" <JBeulich@suse.com> wrote:
>>>> On 23.01.12 at 11:16, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> On Mon, 2012-01-23 at 09:57 +0000, Jan Beulich wrote:
>>> >>> On 20.01.12 at 19:45, Marcus Granado <marcus.granado@citrix.com> wrote:
>>> > --- a/xen/include/public/xenoprof.h    Wed Jan 18 17:39:19 2012 +0000
>>> > +++ b/xen/include/public/xenoprof.h    Wed Jan 18 17:46:28 2012 +0000
>>> > @@ -68,7 +68,7 @@ struct event_log {
>>> >   };
>>> > 
>>> >   /* PC value that indicates a special code */
>>> > -#define XENOPROF_ESCAPE_CODE ~0UL
>>> > +#define XENOPROF_ESCAPE_CODE (~0ULL)
>>> 
>>> You cannot just go and change a public definition, as the consuming
>>> side will break. You need to introduce a new escape code, and
>>> provide a mechanism to negotiate (between hypervisor and kernel)
>>> which one to use.
>> 
>> This seems more like a bug fix to me than an interface change, the fact
>> that both producer and consumer had the bug notwithstanding.
>> 
>> Since this is a developer oriented interface I think we can be a little
>> more relaxed than we would be for other interface changes. In this case
>> we are fixing 32on64 (quite common) at the expense of 32on32 (very
>> uncommon these days, especially for the suybset of developers wanting to
>> do profiling) and leaving 64on64 (quite common) as it is . That seems
>> like a worthwhile tradeoff to me.
> 
> I see your point. However, I'd still like to be careful with this - what
> we think things ought to be used for doesn't necessarily cover all
> cases that they are in reality.

I see that it got applied unchanged. It introduces two warnings in
the 32-bit build, which not only fails that build, but also indicates
that the situation likely wasn't fully analyzed.

Jan

Re: [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[George Dunlap]

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

On Mon, 2012-01-23 at 14:04 +0000, Jan Beulich wrote:
> >>> On 23.01.12 at 11:47, "Jan Beulich" <JBeulich@suse.com> wrote:
> >>>> On 23.01.12 at 11:16, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> On Mon, 2012-01-23 at 09:57 +0000, Jan Beulich wrote:
> >>> >>> On 20.01.12 at 19:45, Marcus Granado <marcus.granado@citrix.com> wrote:
> >>> > --- a/xen/include/public/xenoprof.h    Wed Jan 18 17:39:19 2012 +0000
> >>> > +++ b/xen/include/public/xenoprof.h    Wed Jan 18 17:46:28 2012 +0000
> >>> > @@ -68,7 +68,7 @@ struct event_log {
> >>> >   };
> >>> > 
> >>> >   /* PC value that indicates a special code */
> >>> > -#define XENOPROF_ESCAPE_CODE ~0UL
> >>> > +#define XENOPROF_ESCAPE_CODE (~0ULL)
> >>> 
> >>> You cannot just go and change a public definition, as the consuming
> >>> side will break. You need to introduce a new escape code, and
> >>> provide a mechanism to negotiate (between hypervisor and kernel)
> >>> which one to use.
> >> 
> >> This seems more like a bug fix to me than an interface change, the fact
> >> that both producer and consumer had the bug notwithstanding.
> >> 
> >> Since this is a developer oriented interface I think we can be a little
> >> more relaxed than we would be for other interface changes. In this case
> >> we are fixing 32on64 (quite common) at the expense of 32on32 (very
> >> uncommon these days, especially for the suybset of developers wanting to
> >> do profiling) and leaving 64on64 (quite common) as it is . That seems
> >> like a worthwhile tradeoff to me.
> > 
> > I see your point. However, I'd still like to be careful with this - what
> > we think things ought to be used for doesn't necessarily cover all
> > cases that they are in reality.
> 
> I see that it got applied unchanged. It introduces two warnings in
> the 32-bit build, which not only fails that build, but also indicates
> that the situation likely wasn't fully analyzed.

The attached patch fixes the build (with the original patch un-reverted
of course), by making the internal calls explicitly take 64-bit values
for eip, rather than "unsigned long".  Will that suffice?

 -George

[-- Attachment #2: xenoprof-explicit-u64.diff --]
[-- Type: text/x-patch, Size: 3033 bytes --]

xenoprof: Use uint64_t explicitly for internal calls

A recent changeset to make XENOPROF_ESCAPE_CODE consistent across
32- and 64-bit builds caused a build failure, because values were
passed through functions as "unsigned long".  Replace these with
uint64_t explicitly.

Also remove redundant function prototype from perfmon.c, now that
it's in a header file.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff -r 137c16a83e40 xen/arch/ia64/xen/oprofile/perfmon.c
--- a/xen/arch/ia64/xen/oprofile/perfmon.c	Mon Jan 23 09:42:12 2012 +0000
+++ b/xen/arch/ia64/xen/oprofile/perfmon.c	Mon Jan 23 15:58:36 2012 +0000
@@ -38,8 +38,6 @@
 #include <asm/vmx.h>    /* for vmx_user_mode() */
 
 // XXX move them to an appropriate header file
-extern void xenoprof_log_event(struct vcpu *vcpu, struct pt_regs * regs,
-                               unsigned long eip, int mode, int event);
 extern int is_active(struct domain *d);
 
 static int allow_virq;
diff -r 137c16a83e40 xen/common/xenoprof.c
--- a/xen/common/xenoprof.c	Mon Jan 23 09:42:12 2012 +0000
+++ b/xen/common/xenoprof.c	Mon Jan 23 15:58:36 2012 +0000
@@ -475,7 +475,7 @@ static int xenoprof_buf_space(struct dom
 
 /* Check for space and add a sample. Return 1 if successful, 0 otherwise. */
 static int xenoprof_add_sample(struct domain *d, xenoprof_buf_t *buf,
-                               unsigned long eip, int mode, int event)
+                               uint64_t eip, int mode, int event)
 {
     int head, tail, size;
 
@@ -512,7 +512,7 @@ static int xenoprof_add_sample(struct do
 }
 
 int xenoprof_add_trace(struct domain *d, struct vcpu *vcpu,
-                       unsigned long eip, int mode)
+                       uint64_t eip, int mode)
 {
     xenoprof_buf_t *buf = d->xenoprof->vcpu[vcpu->vcpu_id].buffer;
 
@@ -527,7 +527,7 @@ int xenoprof_add_trace(struct domain *d,
 }
 
 void xenoprof_log_event(struct vcpu *vcpu, 
-                        struct cpu_user_regs * regs, unsigned long eip, 
+                        struct cpu_user_regs * regs, uint64_t eip, 
                         int mode, int event)
 {
     struct domain *d = vcpu->domain;
diff -r 137c16a83e40 xen/include/xen/xenoprof.h
--- a/xen/include/xen/xenoprof.h	Mon Jan 23 09:42:12 2012 +0000
+++ b/xen/include/xen/xenoprof.h	Mon Jan 23 15:58:36 2012 +0000
@@ -69,7 +69,7 @@ int is_passive(struct domain *d);
 void free_xenoprof_pages(struct domain *d);
 
 int xenoprof_add_trace(struct domain *d, struct vcpu *v, 
-                       unsigned long eip, int mode);
+                       uint64_t eip, int mode);
 
 #define PMU_OWNER_NONE          0
 #define PMU_OWNER_XENOPROF      1
@@ -78,7 +78,7 @@ int acquire_pmu_ownship(int pmu_ownershi
 void release_pmu_ownship(int pmu_ownership);
 
 void xenoprof_log_event(struct vcpu *vcpu,
-                        struct cpu_user_regs * regs, unsigned long eip,
+                        struct cpu_user_regs * regs, uint64_t eip,
                         int mode, int event);
 
 #endif  /* __XEN__XENOPROF_H__ */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[Jan Beulich]

>>> On 23.01.12 at 17:01, George Dunlap <george.dunlap@citrix.com> wrote:
> On Mon, 2012-01-23 at 14:04 +0000, Jan Beulich wrote:
>> >>> On 23.01.12 at 11:47, "Jan Beulich" <JBeulich@suse.com> wrote:
>> >>>> On 23.01.12 at 11:16, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> >> On Mon, 2012-01-23 at 09:57 +0000, Jan Beulich wrote:
>> >>> >>> On 20.01.12 at 19:45, Marcus Granado <marcus.granado@citrix.com> wrote:
>> >>> > --- a/xen/include/public/xenoprof.h    Wed Jan 18 17:39:19 2012 +0000
>> >>> > +++ b/xen/include/public/xenoprof.h    Wed Jan 18 17:46:28 2012 +0000
>> >>> > @@ -68,7 +68,7 @@ struct event_log {
>> >>> >   };
>> >>> > 
>> >>> >   /* PC value that indicates a special code */
>> >>> > -#define XENOPROF_ESCAPE_CODE ~0UL
>> >>> > +#define XENOPROF_ESCAPE_CODE (~0ULL)
>> >>> 
>> >>> You cannot just go and change a public definition, as the consuming
>> >>> side will break. You need to introduce a new escape code, and
>> >>> provide a mechanism to negotiate (between hypervisor and kernel)
>> >>> which one to use.
>> >> 
>> >> This seems more like a bug fix to me than an interface change, the fact
>> >> that both producer and consumer had the bug notwithstanding.
>> >> 
>> >> Since this is a developer oriented interface I think we can be a little
>> >> more relaxed than we would be for other interface changes. In this case
>> >> we are fixing 32on64 (quite common) at the expense of 32on32 (very
>> >> uncommon these days, especially for the suybset of developers wanting to
>> >> do profiling) and leaving 64on64 (quite common) as it is . That seems
>> >> like a worthwhile tradeoff to me.
>> > 
>> > I see your point. However, I'd still like to be careful with this - what
>> > we think things ought to be used for doesn't necessarily cover all
>> > cases that they are in reality.
>> 
>> I see that it got applied unchanged. It introduces two warnings in
>> the 32-bit build, which not only fails that build, but also indicates
>> that the situation likely wasn't fully analyzed.
> 
> The attached patch fixes the build (with the original patch un-reverted
> of course), by making the internal calls explicitly take 64-bit values
> for eip, rather than "unsigned long".  Will that suffice?

Looks correct and sufficient, but with the original patch already
reverted folding the changes here into the original and re-submitting
would probably the best route to go.

Jan

Re: [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[George Dunlap]

On Mon, 2012-01-23 at 16:30 +0000, Jan Beulich wrote:
> > The attached patch fixes the build (with the original patch un-reverted
> > of course), by making the internal calls explicitly take 64-bit values
> > for eip, rather than "unsigned long".  Will that suffice?
> 
> Looks correct and sufficient, but with the original patch already
> reverted folding the changes here into the original and re-submitting
> would probably the best route to go.

I really prefer to keep patches with no functional change separate from
those with a pretty major functional change; even if the major
functional change is only one line. :-)

 -George

Re: [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[Jan Beulich]

>>> On 23.01.12 at 17:51, George Dunlap <george.dunlap@citrix.com> wrote:
> On Mon, 2012-01-23 at 16:30 +0000, Jan Beulich wrote:
>> > The attached patch fixes the build (with the original patch un-reverted
>> > of course), by making the internal calls explicitly take 64-bit values
>> > for eip, rather than "unsigned long".  Will that suffice?
>> 
>> Looks correct and sufficient, but with the original patch already
>> reverted folding the changes here into the original and re-submitting
>> would probably the best route to go.
> 
> I really prefer to keep patches with no functional change separate from
> those with a pretty major functional change; even if the major
> functional change is only one line. :-)

Not sure I follow - the (now reverted) patch was buggy, so why not
fix the patch and re-submit? And besides - the fixup patch is certainly
not "no functional change", at least not on 32-bits.

Jan

Re: [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[Keir Fraser]

On 23/01/2012 16:51, "George Dunlap" <george.dunlap@citrix.com> wrote:

> On Mon, 2012-01-23 at 16:30 +0000, Jan Beulich wrote:
>>> The attached patch fixes the build (with the original patch un-reverted
>>> of course), by making the internal calls explicitly take 64-bit values
>>> for eip, rather than "unsigned long".  Will that suffice?
>> 
>> Looks correct and sufficient, but with the original patch already
>> reverted folding the changes here into the original and re-submitting
>> would probably the best route to go.
> 
> I really prefer to keep patches with no functional change separate from
> those with a pretty major functional change; even if the major
> functional change is only one line. :-)

The one-line major functional change didn't work. Obviously the other
changes to make it build must be part of the same logical patch.

 -- Keir

Re: [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[George Dunlap]

On Mon, 2012-01-23 at 17:31 +0000, Keir Fraser wrote:
> > I really prefer to keep patches with no functional change separate from
> > those with a pretty major functional change; even if the major
> > functional change is only one line. :-)
> 
> The one-line major functional change didn't work. Obviously the other
> changes to make it build must be part of the same logical patch.

Perhaps "functional" change isn't what I meant, so much as
"guest-visible behavior change".

If I were writing this patch series from the beginning, I'd have one
patch which "fixed" the various internal functions to use 64-bits for
EIP, since even in 32-bit that value in the struct can be 64-bit.  That
patch by itself could have a bug in it, and so as a reviewer /
archaeologist, *I* would have preferred it did just that one thing, so I
didn't have to figure out what each individual line was supposed to be
doing.  Then the guest-visible functional change would have been its own
patch.

In any case, I'll send a combined patch tomorrow, since that seems
preferred.

 -George

Re: [PATCH 3/3] xenoprof: Make the escape code consistent across 32 and 64-bit xen

[Jan Beulich]

>>> On 23.01.12 at 18:40, George Dunlap <george.dunlap@citrix.com> wrote:
> If I were writing this patch series from the beginning, I'd have one
> patch which "fixed" the various internal functions to use 64-bits for
> EIP, since even in 32-bit that value in the struct can be 64-bit.  That
> patch by itself could have a bug in it, and so as a reviewer /
> archaeologist, *I* would have preferred it did just that one thing, so I
> didn't have to figure out what each individual line was supposed to be
> doing.  Then the guest-visible functional change would have been its own
> patch.

This way round it would seem a reasonable split to me. Yet that
prerequisite patch wasn't sent as such, but instead as a follow up
one.

Jan