| Message ID | 1396015649-5886-1-git-send-email-ian.campbell@citrix.com |
|---|---|
| State | New |
| Headers | show |
On 03/28/2014 02:07 PM, Ian Campbell wrote: > I'm not sure what I was smoking at the time of 5d74ad1a082e "xen: arm: > implement do_multicall_call for both 32 and 64-bit" but it is obviously > insufficient since it doesn't actually wire up the hypercall. > > Before doing so we need to make the usual adjustments for ARM and turn the > unsigned longs into xen_ulong_t. There is no difference in the resulting > structure for x86. > > There are knock on changes to the trace interface, but again they are nops on > x86. I'm wondering if you also need to modify __trace_multicall_call in common/compat/multicall.c. (I know it's not used by ARM...). Regards,
On Fri, 2014-03-28 at 14:22 +0000, Julien Grall wrote: > On 03/28/2014 02:07 PM, Ian Campbell wrote: > > I'm not sure what I was smoking at the time of 5d74ad1a082e "xen: arm: > > implement do_multicall_call for both 32 and 64-bit" but it is obviously > > insufficient since it doesn't actually wire up the hypercall. > > > > Before doing so we need to make the usual adjustments for ARM and turn the > > unsigned longs into xen_ulong_t. There is no difference in the resulting > > structure for x86. > > > > There are knock on changes to the trace interface, but again they are nops on > > x86. > > I'm wondering if you also need to modify __trace_multicall_call in > common/compat/multicall.c. (I know it's not used by ARM...). I did build test x86_64, but perhaps I should make a change here anyway for consistency. George/Jan? Ian.
On Fri, 2014-03-28 at 14:07 +0000, Ian Campbell wrote:= > Before doing so we need to make the usual adjustments for ARM and turn > the unsigned longs into xen_ulong_t. After a discussion with Julien I'm wondering if the actual do_multicall_call dispatcher (reproduced below) should be explicitly truncating the args values from 64-bits to 32-bits for 32-bit guests, since that is the actual size of hypercall arguments for a 32-bit guest. When running on a 64-bit Xen the guest can only actually see the 32-bit rN registers so for a normal hypercall their is an implicit truncation in the h/w exception model. This interface exposes a full 64-bit sized set of arguments even to 32-bit guests. On 32-bit Xen this is truncated by the call() which takes register_t's, but this might hide latent issues in guest kernels. On 64-bit Xen those 64-bit values would be passed to the hypercall. My feeling is that any (exploitable or otherwise) issue due to this would be due to lack of proper error checking in the hypercall, and would be equally accessible by a 64-bit guest. I'm considering whether to add an #ifndef NDEBUG check here which will reject a multicall from a 32-bit guest where any of the arguments (arm_hypercall_table[nr].nr_args) are non-zero in their top 32-bit. I can't decide whether -EINVAL or domain_kill() would be more appropriate. I'm actually leaning towards the latter. Thoughts? Ian. void do_multicall_call(struct multicall_entry *multi) { arm_hypercall_fn_t call = NULL; if ( multi->op >= ARRAY_SIZE(arm_hypercall_table) ) { multi->result = -ENOSYS; return; } call = arm_hypercall_table[multi->op].fn; if ( call == NULL ) { multi->result = -ENOSYS; return; } multi->result = call(multi->args[0], multi->args[1], multi->args[2], multi->args[3], multi->args[4]); }
On 03/28/2014 02:45 PM, Ian Campbell wrote: > My feeling is that any (exploitable or otherwise) issue due to this > would be due to lack of proper error checking in the hypercall, and > would be equally accessible by a 64-bit guest. I don't think it's exploitable. IHMO, the main point is to give a useful debug to the user rather than an obscure error message because the given pointer is invalid (perhaps by mistake). > I'm considering whether to add an #ifndef NDEBUG check here which will > reject a multicall from a 32-bit guest where any of the arguments > (arm_hypercall_table[nr].nr_args) are non-zero in their top 32-bit. I > can't decide whether -EINVAL or domain_kill() would be more appropriate. > I'm actually leaning towards the latter. > > Thoughts? Killing the domain is a bit tough. But it seems that all failures in trap.c result to crash the domain. Regards,
>>> On 28.03.14 at 15:37, <Ian.Campbell@citrix.com> wrote: > On Fri, 2014-03-28 at 14:22 +0000, Julien Grall wrote: >> On 03/28/2014 02:07 PM, Ian Campbell wrote: >> > I'm not sure what I was smoking at the time of 5d74ad1a082e "xen: arm: >> > implement do_multicall_call for both 32 and 64-bit" but it is obviously >> > insufficient since it doesn't actually wire up the hypercall. >> > >> > Before doing so we need to make the usual adjustments for ARM and turn the >> > unsigned longs into xen_ulong_t. There is no difference in the resulting >> > structure for x86. >> > >> > There are knock on changes to the trace interface, but again they are nops > on >> > x86. >> >> I'm wondering if you also need to modify __trace_multicall_call in >> common/compat/multicall.c. (I know it's not used by ARM...). > > I did build test x86_64, but perhaps I should make a change here anyway > for consistency. George/Jan? It doesn't matter that much, but for consistency's sake it would seem desirable. Jan
On Fri, 2014-03-28 at 15:01 +0000, Julien Grall wrote: > On 03/28/2014 02:45 PM, Ian Campbell wrote: > > > My feeling is that any (exploitable or otherwise) issue due to this > > would be due to lack of proper error checking in the hypercall, and > > would be equally accessible by a 64-bit guest. > > I don't think it's exploitable. IHMO, the main point is to give a useful > debug to the user rather than an obscure error message because the given > pointer is invalid (perhaps by mistake). Right. We also want to avoid the case where it just happens to work in one configuration (i.e. 32-on-32) but fails in another (i.e. 32-on-64). > > > I'm considering whether to add an #ifndef NDEBUG check here which will > > reject a multicall from a 32-bit guest where any of the arguments > > (arm_hypercall_table[nr].nr_args) are non-zero in their top 32-bit. I > > can't decide whether -EINVAL or domain_kill() would be more appropriate. > > I'm actually leaning towards the latter. > > > > Thoughts? > > Killing the domain is a bit tough. Sure, but the point is to flush out 32-bit guests which make invalid assumptions which will be broken when they run on 64-bit vs. 32-bit Xen. Ian.
On 03/28/2014 03:07 PM, Ian Campbell wrote: > On Fri, 2014-03-28 at 15:01 +0000, Julien Grall wrote: >> On 03/28/2014 02:45 PM, Ian Campbell wrote: >> >>> My feeling is that any (exploitable or otherwise) issue due to this >>> would be due to lack of proper error checking in the hypercall, and >>> would be equally accessible by a 64-bit guest. >> I don't think it's exploitable. IHMO, the main point is to give a useful >> debug to the user rather than an obscure error message because the given >> pointer is invalid (perhaps by mistake). > Right. > > We also want to avoid the case where it just happens to work in one > configuration (i.e. 32-on-32) but fails in another (i.e. 32-on-64). > >>> I'm considering whether to add an #ifndef NDEBUG check here which will >>> reject a multicall from a 32-bit guest where any of the arguments >>> (arm_hypercall_table[nr].nr_args) are non-zero in their top 32-bit. I >>> can't decide whether -EINVAL or domain_kill() would be more appropriate. >>> I'm actually leaning towards the latter. >>> >>> Thoughts? >> Killing the domain is a bit tough. > Sure, but the point is to flush out 32-bit guests which make invalid > assumptions which will be broken when they run on 64-bit vs. 32-bit Xen. Perhaps return -EINVAL for NDEBUG, and kill if !NDEBUG? We generally don't want to be killing production VMs unless absolutely necessary. One would of course hope that we had caught any bugs during development, but things don't always work the way you'd think... -George
On 31/03/14 17:38, George Dunlap wrote: > On 03/28/2014 03:07 PM, Ian Campbell wrote: >> On Fri, 2014-03-28 at 15:01 +0000, Julien Grall wrote: >>> On 03/28/2014 02:45 PM, Ian Campbell wrote: >>> >>>> My feeling is that any (exploitable or otherwise) issue due to this >>>> would be due to lack of proper error checking in the hypercall, and >>>> would be equally accessible by a 64-bit guest. >>> I don't think it's exploitable. IHMO, the main point is to give a useful >>> debug to the user rather than an obscure error message because the given >>> pointer is invalid (perhaps by mistake). >> Right. >> >> We also want to avoid the case where it just happens to work in one >> configuration (i.e. 32-on-32) but fails in another (i.e. 32-on-64). >> >>>> I'm considering whether to add an #ifndef NDEBUG check here which will >>>> reject a multicall from a 32-bit guest where any of the arguments >>>> (arm_hypercall_table[nr].nr_args) are non-zero in their top 32-bit. I >>>> can't decide whether -EINVAL or domain_kill() would be more >>>> appropriate. >>>> I'm actually leaning towards the latter. >>>> >>>> Thoughts? >>> Killing the domain is a bit tough. >> Sure, but the point is to flush out 32-bit guests which make invalid >> assumptions which will be broken when they run on 64-bit vs. 32-bit Xen. > > Perhaps return -EINVAL for NDEBUG, and kill if !NDEBUG? > > We generally don't want to be killing production VMs unless absolutely > necessary. One would of course hope that we had caught any bugs during > development, but things don't always work the way you'd think... Out-of-context, I've noticed that most of trap failure will kill the domain. From the ARM ARM , if a coprocessor instruction is failing, we should generate an Undefined Instruction exception (see P.7.5). Regards,
On Tue, 2014-04-01 at 10:05 +0100, Julien Grall wrote: > > On 31/03/14 17:38, George Dunlap wrote: > > On 03/28/2014 03:07 PM, Ian Campbell wrote: > >> On Fri, 2014-03-28 at 15:01 +0000, Julien Grall wrote: > >>> On 03/28/2014 02:45 PM, Ian Campbell wrote: > >>> > >>>> My feeling is that any (exploitable or otherwise) issue due to this > >>>> would be due to lack of proper error checking in the hypercall, and > >>>> would be equally accessible by a 64-bit guest. > >>> I don't think it's exploitable. IHMO, the main point is to give a useful > >>> debug to the user rather than an obscure error message because the given > >>> pointer is invalid (perhaps by mistake). > >> Right. > >> > >> We also want to avoid the case where it just happens to work in one > >> configuration (i.e. 32-on-32) but fails in another (i.e. 32-on-64). > >> > >>>> I'm considering whether to add an #ifndef NDEBUG check here which will > >>>> reject a multicall from a 32-bit guest where any of the arguments > >>>> (arm_hypercall_table[nr].nr_args) are non-zero in their top 32-bit. I > >>>> can't decide whether -EINVAL or domain_kill() would be more > >>>> appropriate. > >>>> I'm actually leaning towards the latter. > >>>> > >>>> Thoughts? > >>> Killing the domain is a bit tough. > >> Sure, but the point is to flush out 32-bit guests which make invalid > >> assumptions which will be broken when they run on 64-bit vs. 32-bit Xen. > > > > Perhaps return -EINVAL for NDEBUG, and kill if !NDEBUG? > > > > We generally don't want to be killing production VMs unless absolutely > > necessary. One would of course hope that we had caught any bugs during > > development, but things don't always work the way you'd think... More of often than not I'd expect that the guest would shoot itself if one of these hypercalls failed. But the approach you suggest is probably the best compromise. > Out-of-context, I've noticed that most of trap failure will kill the > domain. From the ARM ARM , if a coprocessor instruction is failing, we > should generate an Undefined Instruction exception (see P.7.5). You mean HSR_EC_CP15_32, HSR_EC_CP15_64 and HSR_EC_SYSREG? I've not checked but I think we only ask for traps for things which we are supposed to be able to handle, so anything which is trapped which we can't handle is a bug and would indicate the guest doing something funky. Now maybe that is wrong and we are asking for traps which we don't handle, in which case we should either handle them or stop asking for them. Ian.
On 03/28/2014 03:03 PM, Jan Beulich wrote: >>>> On 28.03.14 at 15:37, <Ian.Campbell@citrix.com> wrote: >> On Fri, 2014-03-28 at 14:22 +0000, Julien Grall wrote: >>> On 03/28/2014 02:07 PM, Ian Campbell wrote: >>>> I'm not sure what I was smoking at the time of 5d74ad1a082e "xen: arm: >>>> implement do_multicall_call for both 32 and 64-bit" but it is obviously >>>> insufficient since it doesn't actually wire up the hypercall. >>>> >>>> Before doing so we need to make the usual adjustments for ARM and turn the >>>> unsigned longs into xen_ulong_t. There is no difference in the resulting >>>> structure for x86. >>>> >>>> There are knock on changes to the trace interface, but again they are nops >> on >>>> x86. >>> >>> I'm wondering if you also need to modify __trace_multicall_call in >>> common/compat/multicall.c. (I know it's not used by ARM...). >> >> I did build test x86_64, but perhaps I should make a change here anyway >> for consistency. George/Jan? > > It doesn't matter that much, but for consistency's sake it would seem > desirable. +1 -G
On 04/01/2014 10:28 AM, Ian Campbell wrote: > On Tue, 2014-04-01 at 10:05 +0100, Julien Grall wrote: >> Out-of-context, I've noticed that most of trap failure will kill the >> domain. From the ARM ARM , if a coprocessor instruction is failing, we >> should generate an Undefined Instruction exception (see P.7.5). > > You mean HSR_EC_CP15_32, HSR_EC_CP15_64 and HSR_EC_SYSREG? > > I've not checked but I think we only ask for traps for things which we > are supposed to be able to handle, so anything which is trapped which we > can't handle is a bug and would indicate the guest doing something > funky. Right, but if the emulation of the instruction fails (see vtimer_emulate, do_trap_psci,...), Xen will destroy the domain instead of sending an UNDEF exception.
On Tue, 2014-04-01 at 11:46 +0100, Julien Grall wrote: > On 04/01/2014 10:28 AM, Ian Campbell wrote: > > On Tue, 2014-04-01 at 10:05 +0100, Julien Grall wrote: > >> Out-of-context, I've noticed that most of trap failure will kill the > >> domain. From the ARM ARM , if a coprocessor instruction is failing, we > >> should generate an Undefined Instruction exception (see P.7.5). > > > > You mean HSR_EC_CP15_32, HSR_EC_CP15_64 and HSR_EC_SYSREG? > > > > I've not checked but I think we only ask for traps for things which we > > are supposed to be able to handle, so anything which is trapped which we > > can't handle is a bug and would indicate the guest doing something > > funky. > > Right, but if the emulation of the instruction fails (see > vtimer_emulate, do_trap_psci,...), Xen will destroy the domain instead > of sending an UNDEF exception. My point was that the emulation should never fail... Ian.
On 04/01/2014 11:49 AM, Ian Campbell wrote: > On Tue, 2014-04-01 at 11:46 +0100, Julien Grall wrote: >> On 04/01/2014 10:28 AM, Ian Campbell wrote: >>> On Tue, 2014-04-01 at 10:05 +0100, Julien Grall wrote: >>>> Out-of-context, I've noticed that most of trap failure will kill the >>>> domain. From the ARM ARM , if a coprocessor instruction is failing, we >>>> should generate an Undefined Instruction exception (see P.7.5). >>> >>> You mean HSR_EC_CP15_32, HSR_EC_CP15_64 and HSR_EC_SYSREG? >>> >>> I've not checked but I think we only ask for traps for things which we >>> are supposed to be able to handle, so anything which is trapped which we >>> can't handle is a bug and would indicate the guest doing something >>> funky. >> >> Right, but if the emulation of the instruction fails (see >> vtimer_emulate, do_trap_psci,...), Xen will destroy the domain instead >> of sending an UNDEF exception. > > My point was that the emulation should never fail... The emulation can fail if the guest decides to write on an RO register.
On Tue, 2014-04-01 at 12:00 +0100, Julien Grall wrote: > On 04/01/2014 11:49 AM, Ian Campbell wrote: > > On Tue, 2014-04-01 at 11:46 +0100, Julien Grall wrote: > >> On 04/01/2014 10:28 AM, Ian Campbell wrote: > >>> On Tue, 2014-04-01 at 10:05 +0100, Julien Grall wrote: > >>>> Out-of-context, I've noticed that most of trap failure will kill the > >>>> domain. From the ARM ARM , if a coprocessor instruction is failing, we > >>>> should generate an Undefined Instruction exception (see P.7.5). > >>> > >>> You mean HSR_EC_CP15_32, HSR_EC_CP15_64 and HSR_EC_SYSREG? > >>> > >>> I've not checked but I think we only ask for traps for things which we > >>> are supposed to be able to handle, so anything which is trapped which we > >>> can't handle is a bug and would indicate the guest doing something > >>> funky. > >> > >> Right, but if the emulation of the instruction fails (see > >> vtimer_emulate, do_trap_psci,...), Xen will destroy the domain instead > >> of sending an UNDEF exception. > > > > My point was that the emulation should never fail... > > The emulation can fail if the guest decides to write on an RO register. Hrm yes, I'd forgotten that case. Is that an undef or some other sort of exception? Perhaps it depends on the cp register whether it faults or is ignored? In either case that seems to suggest that it is up to the specific handler to inject the correct kind of exception and return an appropriate error code to the generic handler. Ian.
On 04/01/2014 12:15 PM, Ian Campbell wrote: > On Tue, 2014-04-01 at 12:00 +0100, Julien Grall wrote: >> On 04/01/2014 11:49 AM, Ian Campbell wrote: >>> On Tue, 2014-04-01 at 11:46 +0100, Julien Grall wrote: >>>> On 04/01/2014 10:28 AM, Ian Campbell wrote: >>>>> On Tue, 2014-04-01 at 10:05 +0100, Julien Grall wrote: >>>>>> Out-of-context, I've noticed that most of trap failure will kill the >>>>>> domain. From the ARM ARM , if a coprocessor instruction is failing, we >>>>>> should generate an Undefined Instruction exception (see P.7.5). >>>>> >>>>> You mean HSR_EC_CP15_32, HSR_EC_CP15_64 and HSR_EC_SYSREG? >>>>> >>>>> I've not checked but I think we only ask for traps for things which we >>>>> are supposed to be able to handle, so anything which is trapped which we >>>>> can't handle is a bug and would indicate the guest doing something >>>>> funky. >>>> >>>> Right, but if the emulation of the instruction fails (see >>>> vtimer_emulate, do_trap_psci,...), Xen will destroy the domain instead >>>> of sending an UNDEF exception. >>> >>> My point was that the emulation should never fail... >> >> The emulation can fail if the guest decides to write on an RO register. > > Hrm yes, I'd forgotten that case. > > Is that an undef or some other sort of exception? Perhaps it depends on > the cp register whether it faults or is ignored? In either case that > seems to suggest that it is up to the specific handler to inject the > correct kind of exception and return an appropriate error code to the > generic handler. The default exception is "undefined instruction". I didn't find any specific exception following the coprocessor. Actually the only ways to kill the domain in traps.c: - we try to access in read (resp. write) on WO (resp. RO) registers - the hypercall tags is wrong - the PSCI function is not implemented I think we can replace every (?) domain_crash_synchronous by inject_undef*_exception.
On Tue, 2014-04-01 at 13:05 +0100, Julien Grall wrote: > On 04/01/2014 12:15 PM, Ian Campbell wrote: > > On Tue, 2014-04-01 at 12:00 +0100, Julien Grall wrote: > >> On 04/01/2014 11:49 AM, Ian Campbell wrote: > >>> On Tue, 2014-04-01 at 11:46 +0100, Julien Grall wrote: > >>>> On 04/01/2014 10:28 AM, Ian Campbell wrote: > >>>>> On Tue, 2014-04-01 at 10:05 +0100, Julien Grall wrote: > >>>>>> Out-of-context, I've noticed that most of trap failure will kill the > >>>>>> domain. From the ARM ARM , if a coprocessor instruction is failing, we > >>>>>> should generate an Undefined Instruction exception (see P.7.5). > >>>>> > >>>>> You mean HSR_EC_CP15_32, HSR_EC_CP15_64 and HSR_EC_SYSREG? > >>>>> > >>>>> I've not checked but I think we only ask for traps for things which we > >>>>> are supposed to be able to handle, so anything which is trapped which we > >>>>> can't handle is a bug and would indicate the guest doing something > >>>>> funky. > >>>> > >>>> Right, but if the emulation of the instruction fails (see > >>>> vtimer_emulate, do_trap_psci,...), Xen will destroy the domain instead > >>>> of sending an UNDEF exception. > >>> > >>> My point was that the emulation should never fail... > >> > >> The emulation can fail if the guest decides to write on an RO register. > > > > Hrm yes, I'd forgotten that case. > > > > Is that an undef or some other sort of exception? Perhaps it depends on > > the cp register whether it faults or is ignored? In either case that > > seems to suggest that it is up to the specific handler to inject the > > correct kind of exception and return an appropriate error code to the > > generic handler. > > The default exception is "undefined instruction". I didn't find any > specific exception following the coprocessor. Actually the only ways to > kill the domain in traps.c: > - we try to access in read (resp. write) on WO (resp. RO) registers > - the hypercall tags is wrong > - the PSCI function is not implemented > > I think we can replace every (?) domain_crash_synchronous by > inject_undef*_exception. Not in a bulk substitution. Each change would need justifying. In the PSCI case I'd have thought the PSCI spec said what to do, probably return some error code in r0? Or maybe not. Ian.
On 04/01/2014 01:11 PM, Ian Campbell wrote: > In the PSCI case I'd have thought the PSCI spec said what to do, > probably return some error code in r0? Or maybe not. Do you have a link to the spec?
On Tue, 2014-04-01 at 13:16 +0100, Julien Grall wrote: > On 04/01/2014 01:11 PM, Ian Campbell wrote: > > > In the PSCI case I'd have thought the PSCI spec said what to do, > > probably return some error code in r0? Or maybe not. > > Do you have a link to the spec? It should be in the ARM infocenter. I think it's public. Ian.
On 04/01/2014 01:16 PM, Ian Campbell wrote: > On Tue, 2014-04-01 at 13:16 +0100, Julien Grall wrote: >> On 04/01/2014 01:11 PM, Ian Campbell wrote: >> >>> In the PSCI case I'd have thought the PSCI spec said what to do, >>> probably return some error code in r0? Or maybe not. >> >> Do you have a link to the spec? > > It should be in the ARM infocenter. I think it's public. Thanks, so with PSCI v0.{1,2}, we should return -1 (Function not implemented) if Xen doesn't support the call.
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index ec43e65..ca315af 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1011,6 +1011,7 @@ static arm_hypercall_t arm_hypercall_table[] = { HYPERCALL(sysctl, 2), HYPERCALL(hvm_op, 2), HYPERCALL(grant_table_op, 3), + HYPERCALL(multicall, 2), HYPERCALL_ARM(vcpu_op, 3), }; diff --git a/xen/common/multicall.c b/xen/common/multicall.c index e66c798..fa9d910 100644 --- a/xen/common/multicall.c +++ b/xen/common/multicall.c @@ -35,10 +35,10 @@ static void trace_multicall_call(multicall_entry_t *call) ret_t do_multicall( - XEN_GUEST_HANDLE_PARAM(multicall_entry_t) call_list, unsigned int nr_calls) + XEN_GUEST_HANDLE_PARAM(multicall_entry_t) call_list, uint32_t nr_calls) { struct mc_state *mcs = ¤t->mc_state; - unsigned int i; + uint32_t i; int rc = 0; if ( unlikely(__test_and_set_bit(_MCSF_in_multicall, &mcs->flags)) ) diff --git a/xen/common/trace.c b/xen/common/trace.c index 1814165..f651cf3 100644 --- a/xen/common/trace.c +++ b/xen/common/trace.c @@ -817,7 +817,7 @@ unlock: } void __trace_hypercall(uint32_t event, unsigned long op, - const unsigned long *args) + const xen_ulong_t *args) { struct __packed { uint32_t op; diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index 8c5697e..5bba3af 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -541,13 +541,13 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t); /* * ` enum neg_errnoval * ` HYPERVISOR_multicall(multicall_entry_t call_list[], - * ` unsigned int nr_calls); + * ` uint32_t nr_calls); * * NB. The fields are natural register size for this architecture. */ struct multicall_entry { - unsigned long op, result; - unsigned long args[6]; + xen_ulong_t op, result; + xen_ulong_t args[6]; }; typedef struct multicall_entry multicall_entry_t; DEFINE_XEN_GUEST_HANDLE(multicall_entry_t); diff --git a/xen/include/xen/trace.h b/xen/include/xen/trace.h index 3b8a7b3..12966ea 100644 --- a/xen/include/xen/trace.h +++ b/xen/include/xen/trace.h @@ -45,7 +45,7 @@ static inline void trace_var(u32 event, int cycles, int extra, } void __trace_hypercall(uint32_t event, unsigned long op, - const unsigned long *args); + const xen_ulong_t *args); /* Convenience macros for calling the trace function. */ #define TRACE_0D(_e) \
I'm not sure what I was smoking at the time of 5d74ad1a082e "xen: arm: implement do_multicall_call for both 32 and 64-bit" but it is obviously insufficient since it doesn't actually wire up the hypercall. Before doing so we need to make the usual adjustments for ARM and turn the unsigned longs into xen_ulong_t. There is no difference in the resulting structure for x86. There are knock on changes to the trace interface, but again they are nops on x86. In the interests of clarity and always using explicitly sized types change the unsigned int in the hypercall arguments to a uint32_t. There is no actual change here on any platform. We should consider backporting this to 4.4.1 in case a guest decides they want to use a multicall in common code e.g. I suggested such a thing while reviewing a netback change recently. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: keir@xen.org Cc: jbeulich@suse.com Cc: George Dunlap <george.dunlap@eu.citrix.com> --- xen/arch/arm/traps.c | 1 + xen/common/multicall.c | 4 ++-- xen/common/trace.c | 2 +- xen/include/public/xen.h | 6 +++--- xen/include/xen/trace.h | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-)