| Message ID | 1464907946-19242-5-git-send-email-tamas@tklengyel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Tamas, On 02/06/16 23:52, Tamas K Lengyel wrote: > diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h > index 9270d52..7976080 100644 > --- a/xen/include/public/vm_event.h > +++ b/xen/include/public/vm_event.h > @@ -119,6 +119,8 @@ > #define VM_EVENT_REASON_SINGLESTEP 7 > /* An event has been requested via HVMOP_guest_request_vm_event. */ > #define VM_EVENT_REASON_GUEST_REQUEST 8 > +/* Privileged call executed (e.g. SMC) */ > +#define VM_EVENT_REASON_PRIVILEGED_CALL 9 > > /* Supported values for the vm_event_write_ctrlreg index. */ > #define VM_EVENT_X86_CR0 0 > @@ -212,6 +214,13 @@ struct vm_event_mov_to_msr { > uint64_t value; > }; > > +#define VM_EVENT_PRIVCALL_SMC 0 > + > +struct vm_event_privcall { > + uint32_t type; > + uint32_t vector; /* ESR_EL2.ISS for SMC calls */ How do you expect the introspection app to deal with it? As explained in a previous mail [1], the ISS encoding is different between ARMv7 32-bit and ARMv8 32-bit. The former is unknown (see B3-1431 in ARM DDI 0406C.c) whilst the latter contains fields related to the condition (see D7-1897 in ARM DDI 0406C.c). This is because on ARMv8, the conditional SMC issued in AArch32 state may trap even if the condition has failed. So the app would have to know whether the hypervisor is running on an ARMv7 or ARMv8 platform. But I am not aware of an easy way to differentiate it from the registers. Furthermore, I think the vm_event app should only received SMCs whose condition has succeeded, because they will be actual SMC. The others should just be ignored. IHMO, the vm_event should only contain the immediate. The rest only matters for the hypervisor. Regards, [1] http://lists.xenproject.org/archives/html/xen-devel/2016-06/msg00285.html
On Jun 3, 2016 03:49, "Julien Grall" <julien.grall@arm.com> wrote: > > Hello Tamas, > > > On 02/06/16 23:52, Tamas K Lengyel wrote: >> >> diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h >> index 9270d52..7976080 100644 >> --- a/xen/include/public/vm_event.h >> +++ b/xen/include/public/vm_event.h >> @@ -119,6 +119,8 @@ >> #define VM_EVENT_REASON_SINGLESTEP 7 >> /* An event has been requested via HVMOP_guest_request_vm_event. */ >> #define VM_EVENT_REASON_GUEST_REQUEST 8 >> +/* Privileged call executed (e.g. SMC) */ >> +#define VM_EVENT_REASON_PRIVILEGED_CALL 9 >> >> /* Supported values for the vm_event_write_ctrlreg index. */ >> #define VM_EVENT_X86_CR0 0 >> @@ -212,6 +214,13 @@ struct vm_event_mov_to_msr { >> uint64_t value; >> }; >> >> +#define VM_EVENT_PRIVCALL_SMC 0 >> + >> +struct vm_event_privcall { >> + uint32_t type; >> + uint32_t vector; /* ESR_EL2.ISS for SMC calls */ > > > How do you expect the introspection app to deal with it? As explained in a previous mail [1], the ISS encoding is different between ARMv7 32-bit and ARMv8 32-bit. The former is unknown (see B3-1431 in ARM DDI 0406C.c) whilst the latter contains fields related to the condition (see D7-1897 in ARM DDI 0406C.c). > > This is because on ARMv8, the conditional SMC issued in AArch32 state may trap even if the condition has failed. > > So the app would have to know whether the hypervisor is running on an ARMv7 or ARMv8 platform. But I am not aware of an easy way to differentiate it from the registers. The app can certainly run other checks to determine what the CPU version is, not being exclusively reliant on vm_event and running in a privileged domain. > > Furthermore, I think the vm_event app should only received SMCs whose condition has succeeded, because they will be actual SMC. The others should just be ignored. > > IHMO, the vm_event should only contain the immediate. The rest only matters for the hypervisor. Absolutely not! The primary usecase I have for SMC trapping is kernel execution monitoring by manually writing it into arbitrary kernel code locations and hiding them from the guest with mem_access. If some SMCs may silently get swallowed by the hypervisor the whole thing becomes unreliable. Tamas
Hello Tamas, On 03/06/16 14:40, Tamas K Lengyel wrote: > > On Jun 3, 2016 03:49, "Julien Grall" <julien.grall@arm.com > <mailto:julien.grall@arm.com>> wrote: > > > > Hello Tamas, > > > > > > On 02/06/16 23:52, Tamas K Lengyel wrote: > >> > >> diff --git a/xen/include/public/vm_event.h > b/xen/include/public/vm_event.h > >> index 9270d52..797608Burrington0 100644 > >> --- a/xen/include/public/vm_event.h > >> +++ b/xen/include/public/vm_event.h > >> @@ -119,6 +119,8 @@ > >> #define VM_EVENT_REASON_SINGLESTEP 7 > >> /* An event has been requested via HVMOP_guest_request_vm_event. */ > >> #define VM_EVENT_REASON_GUEST_REQUEST 8 > >> +/* Privileged call executed (e.g. SMC) */ > >> +#define VM_EVENT_REASON_PRIVILEGED_CALL 9 > >> > >> /* Supported values for the vm_event_write_ctrlreg index. */ > >> #define VM_EVENT_X86_CR0 0 > >> @@ -212,6 +214,13 @@ struct vm_event_mov_to_msr { > >> uint64_t value; > >> }; > >> > >> +#define VM_EVENT_PRIVCALL_SMC 0 > >> + > >> +struct vm_event_privcall { > >> + uint32_t type; > >> + uint32_t vector; /* ESR_EL2.ISS for SMC calls */ > > > > > > How do you expect the introspection app to deal with it? As explained ` > in a previous mail [1], the ISS encoding is different between ARMv7 > 32-bit and ARMv8 32-bit. The former is unknown (see B3-1431 in ARM DDI > 0406C.c) whilst the latter contains fields related to the condition (see > D7-1897 in ARM DDI 0406C.c). > > > > This is because on ARMv8, the conditional SMC issued in AArch32 state > may trap even if the condition has failed. > > > > So the app would have to know whether the hypervisor is running on an > ARMv7 or ARMv8 platform. But I am not aware of an easy way to > differentiate it from the registers. > > The app can certainly run other checks to determine what the CPU version > is, not being exclusively reliant on vm_event and running in a > privileged domain. Manufacturers are allowed to build their custom ARM processor based on the ARM ARM. The number of CPU version to check will likely be huge and you will not be future proof. ` > > > > > Furthermore, I think the vm_event app should only received SMCs whose > condition has succeeded, because they will be actual SMC. The others > should just be ignored. > > > > IHMO, the vm_event should only contain the immediate. The rest only > matters for the hypervisor. > > Absolutely not! The primary usecase I have for SMC trapping is kernel > execution monitoring by manually writing it into arbitrary kernel code > locations and hiding them from the guest with mem_access. If some SMCs > may silently get swallowed by the hypervisor the whole thing becomes > unreliable. Please read what I wrote, on ARMv8, a conditional SMC issued in AArch32 state *may* trap even if the condition has failed. I.e an implementer can design its CPU to not trap them (see D1-1506 on ARM DDI 0487A.i). On ARMv7, only unconditional SMC and conditional SMC *which pass the condition test* will be trapped. The others will be ignored. So even if the hypervisor send an event for each SMC trapped, you may not receive all the SMCs. This is already unreliable by the architecture. If you want something reliable, you will have to inject unconditional SMC or HVC which are always unconditional. If you want to also trap all the SMCs written by a guest, then you will have to live with the fact that some may be ignored. Although, I don't think that an introspection app should care about instructions that would be treated as a nop (for instance because the condition check has failed). Hence my suggestion to check in the hypervisor whether the condition has failed and provide processor-agnostic information (the ISS is different between ARMv7, ARMv8 and AArch32 and AArch64). Regards,
On Fri, Jun 3, 2016 at 8:43 AM, Julien Grall <julien.grall@arm.com> wrote: > Hello Tamas, > > On 03/06/16 14:40, Tamas K Lengyel wrote: >> >> >> On Jun 3, 2016 03:49, "Julien Grall" <julien.grall@arm.com >> <mailto:julien.grall@arm.com>> wrote: >> > >> > Hello Tamas, >> > >> > >> > On 02/06/16 23:52, Tamas K Lengyel wrote: >> >> >> >> diff --git a/xen/include/public/vm_event.h >> b/xen/include/public/vm_event.h >> >> index 9270d52..797608Burrington0 100644 >> >> >> --- a/xen/include/public/vm_event.h >> >> +++ b/xen/include/public/vm_event.h >> >> @@ -119,6 +119,8 @@ >> >> #define VM_EVENT_REASON_SINGLESTEP 7 >> >> /* An event has been requested via HVMOP_guest_request_vm_event. */ >> >> #define VM_EVENT_REASON_GUEST_REQUEST 8 >> >> +/* Privileged call executed (e.g. SMC) */ >> >> +#define VM_EVENT_REASON_PRIVILEGED_CALL 9 >> >> >> >> /* Supported values for the vm_event_write_ctrlreg index. */ >> >> #define VM_EVENT_X86_CR0 0 >> >> @@ -212,6 +214,13 @@ struct vm_event_mov_to_msr { >> >> uint64_t value; >> >> }; >> >> >> >> +#define VM_EVENT_PRIVCALL_SMC 0 >> >> + >> >> +struct vm_event_privcall { >> >> + uint32_t type; >> >> + uint32_t vector; /* ESR_EL2.ISS for SMC calls */ >> > >> > >> > How do you expect the introspection app to deal with it? As explained ` >> in a previous mail [1], the ISS encoding is different between ARMv7 >> 32-bit and ARMv8 32-bit. The former is unknown (see B3-1431 in ARM DDI >> 0406C.c) whilst the latter contains fields related to the condition (see >> D7-1897 in ARM DDI 0406C.c). >> > >> > This is because on ARMv8, the conditional SMC issued in AArch32 state >> may trap even if the condition has failed. >> > >> > So the app would have to know whether the hypervisor is running on an >> ARMv7 or ARMv8 platform. But I am not aware of an easy way to >> differentiate it from the registers. >> >> The app can certainly run other checks to determine what the CPU version >> is, not being exclusively reliant on vm_event and running in a >> privileged domain. > > > Manufacturers are allowed to build their custom ARM processor based on the > ARM ARM. The number of CPU version to check will likely be huge and you will > not be future proof. > ` If transmitting the ISS to the user this way is not enough in your opinion I may just leave this item up for a future user to implement as my use-case doesn't need it. Tamas
On 03/06/16 16:03, Tamas K Lengyel wrote: > If transmitting the ISS to the user this way is not enough in your > opinion I may just leave this item up for a future user to implement > as my use-case doesn't need it. It is up to you. However, it will likely mean to bump the interface version of the VM Event. BTW, I have not seen any change to the interface version within this series. But the size of the structure will change in patch #9 which will impact all the introspection app. Is it normal? Regards,
>> > Furthermore, I think the vm_event app should only received SMCs whose >> condition has succeeded, because they will be actual SMC. The others >> should just be ignored. >> > >> > IHMO, the vm_event should only contain the immediate. The rest only >> matters for the hypervisor. >> >> Absolutely not! The primary usecase I have for SMC trapping is kernel >> execution monitoring by manually writing it into arbitrary kernel code >> locations and hiding them from the guest with mem_access. If some SMCs >> may silently get swallowed by the hypervisor the whole thing becomes >> unreliable. > > > Please read what I wrote, on ARMv8, a conditional SMC issued in AArch32 > state *may* trap even if the condition has failed. I.e an implementer can > design its CPU to not trap them (see D1-1506 on ARM DDI 0487A.i). > > On ARMv7, only unconditional SMC and conditional SMC *which pass the > condition test* will be trapped. The others will be ignored. > > So even if the hypervisor send an event for each SMC trapped, you may not > receive all the SMCs. This is already unreliable by the architecture. > > If you want something reliable, you will have to inject unconditional SMC or > HVC which are always unconditional. Can you tell me how a conditional SMC would look like in memory? Would it be a variant of the instruction with the condition code mnemonic embedded in it, or the condition code is like another instruction following the SMC? From the ARM ARM it's not entirely clear to me (SMC{cond} #imm4). If it's the latter then we indeed need more work done during trapping since we would need to be aware of the context of where we are writing SMC and make sure the following condition check is also disabled. Otherwise we can just inject unconditional SMCs and case closed. Either way, we can swallow the SMCs with failed condition checks, but if it already trapped to the hypervisor, we might as well forward it to the vm_event subscriber if there is one and let it decide what it wants to do next (jump over the instruction or crash the domain being the only paths available). Thanks, Tamas
On Fri, Jun 3, 2016 at 9:27 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote: >>> > Furthermore, I think the vm_event app should only received SMCs whose >>> condition has succeeded, because they will be actual SMC. The others >>> should just be ignored. >>> > >>> > IHMO, the vm_event should only contain the immediate. The rest only >>> matters for the hypervisor. >>> >>> Absolutely not! The primary usecase I have for SMC trapping is kernel >>> execution monitoring by manually writing it into arbitrary kernel code >>> locations and hiding them from the guest with mem_access. If some SMCs >>> may silently get swallowed by the hypervisor the whole thing becomes >>> unreliable. >> >> >> Please read what I wrote, on ARMv8, a conditional SMC issued in AArch32 >> state *may* trap even if the condition has failed. I.e an implementer can >> design its CPU to not trap them (see D1-1506 on ARM DDI 0487A.i). >> >> On ARMv7, only unconditional SMC and conditional SMC *which pass the >> condition test* will be trapped. The others will be ignored. >> >> So even if the hypervisor send an event for each SMC trapped, you may not >> receive all the SMCs. This is already unreliable by the architecture. >> >> If you want something reliable, you will have to inject unconditional SMC or >> HVC which are always unconditional. > > Can you tell me how a conditional SMC would look like in memory? Would > it be a variant of the instruction with the condition code mnemonic > embedded in it, or the condition code is like another instruction > following the SMC? From the ARM ARM it's not entirely clear to me > (SMC{cond} #imm4). If it's the latter then we indeed need more work > done during trapping since we would need to be aware of the context of > where we are writing SMC and make sure the following condition check > is also disabled. Otherwise we can just inject unconditional SMCs and > case closed. Either way, we can swallow the SMCs with failed condition > checks, but if it already trapped to the hypervisor, we might as well > forward it to the vm_event subscriber if there is one and let it > decide what it wants to do next (jump over the instruction or crash > the domain being the only paths available). > Never mind, found the info "This condition is encoded in ARM instructions". So yes, we are always injecting unconditional SMCs for monitoring so SMCs with failed condition checks are of no interest. My comment above still stands though, we might as well forward these too if they trapped to the VMM. Tamas
On Fri, Jun 3, 2016 at 9:06 AM, Julien Grall <julien.grall@arm.com> wrote: > > > On 03/06/16 16:03, Tamas K Lengyel wrote: >> >> If transmitting the ISS to the user this way is not enough in your >> opinion I may just leave this item up for a future user to implement >> as my use-case doesn't need it. > > > It is up to you. However, it will likely mean to bump the interface version > of the VM Event. That's what it is for. > > BTW, I have not seen any change to the interface version within this series. > But the size of the structure will change in patch #9 which will impact all > the introspection app. Is it normal? > We are increasing the interface number later in the series. We only need to increase it once per every merge-window, i.e. we can keep morphing it under the umbrella of a single version bump until it's only in unstable. Tamas
On Fri, Jun 03, 2016 at 09:34:20AM -0600, Tamas K Lengyel wrote: > On Fri, Jun 3, 2016 at 9:27 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote: > >>> > Furthermore, I think the vm_event app should only received SMCs whose > >>> condition has succeeded, because they will be actual SMC. The others > >>> should just be ignored. > >>> > > >>> > IHMO, the vm_event should only contain the immediate. The rest only > >>> matters for the hypervisor. > >>> > >>> Absolutely not! The primary usecase I have for SMC trapping is kernel > >>> execution monitoring by manually writing it into arbitrary kernel code > >>> locations and hiding them from the guest with mem_access. If some SMCs > >>> may silently get swallowed by the hypervisor the whole thing becomes > >>> unreliable. > >> > >> > >> Please read what I wrote, on ARMv8, a conditional SMC issued in AArch32 > >> state *may* trap even if the condition has failed. I.e an implementer can > >> design its CPU to not trap them (see D1-1506 on ARM DDI 0487A.i). > >> > >> On ARMv7, only unconditional SMC and conditional SMC *which pass the > >> condition test* will be trapped. The others will be ignored. > >> > >> So even if the hypervisor send an event for each SMC trapped, you may not > >> receive all the SMCs. This is already unreliable by the architecture. > >> > >> If you want something reliable, you will have to inject unconditional SMC or > >> HVC which are always unconditional. > > > > Can you tell me how a conditional SMC would look like in memory? Would > > it be a variant of the instruction with the condition code mnemonic > > embedded in it, or the condition code is like another instruction > > following the SMC? From the ARM ARM it's not entirely clear to me > > (SMC{cond} #imm4). If it's the latter then we indeed need more work > > done during trapping since we would need to be aware of the context of > > where we are writing SMC and make sure the following condition check > > is also disabled. Otherwise we can just inject unconditional SMCs and > > case closed. Either way, we can swallow the SMCs with failed condition > > checks, but if it already trapped to the hypervisor, we might as well > > forward it to the vm_event subscriber if there is one and let it > > decide what it wants to do next (jump over the instruction or crash > > the domain being the only paths available). > > > > Never mind, found the info "This condition is encoded in ARM > instructions". So yes, we are always injecting unconditional SMCs for > monitoring so SMCs with failed condition checks are of no interest. My > comment above still stands though, we might as well forward these too > if they trapped to the VMM. > Hi, Forwarding SMC events for SMC insns that didn't pass the condition tests doesn't make any sense to me. It'll just make the receivers job harder. Why would a receiver want to do anything else than drop these? If it actually does look at them it'll be looking at implementation defined HW behaviour that may vary between CPU implementations. Cheers, Edgar
On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > On Fri, Jun 03, 2016 at 09:34:20AM -0600, Tamas K Lengyel wrote: >> On Fri, Jun 3, 2016 at 9:27 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote: >> >>> > Furthermore, I think the vm_event app should only received SMCs whose >> >>> condition has succeeded, because they will be actual SMC. The others >> >>> should just be ignored. >> >>> > >> >>> > IHMO, the vm_event should only contain the immediate. The rest only >> >>> matters for the hypervisor. >> >>> >> >>> Absolutely not! The primary usecase I have for SMC trapping is kernel >> >>> execution monitoring by manually writing it into arbitrary kernel code >> >>> locations and hiding them from the guest with mem_access. If some SMCs >> >>> may silently get swallowed by the hypervisor the whole thing becomes >> >>> unreliable. >> >> >> >> >> >> Please read what I wrote, on ARMv8, a conditional SMC issued in AArch32 >> >> state *may* trap even if the condition has failed. I.e an implementer can >> >> design its CPU to not trap them (see D1-1506 on ARM DDI 0487A.i). >> >> >> >> On ARMv7, only unconditional SMC and conditional SMC *which pass the >> >> condition test* will be trapped. The others will be ignored. >> >> >> >> So even if the hypervisor send an event for each SMC trapped, you may not >> >> receive all the SMCs. This is already unreliable by the architecture. >> >> >> >> If you want something reliable, you will have to inject unconditional SMC or >> >> HVC which are always unconditional. >> > >> > Can you tell me how a conditional SMC would look like in memory? Would >> > it be a variant of the instruction with the condition code mnemonic >> > embedded in it, or the condition code is like another instruction >> > following the SMC? From the ARM ARM it's not entirely clear to me >> > (SMC{cond} #imm4). If it's the latter then we indeed need more work >> > done during trapping since we would need to be aware of the context of >> > where we are writing SMC and make sure the following condition check >> > is also disabled. Otherwise we can just inject unconditional SMCs and >> > case closed. Either way, we can swallow the SMCs with failed condition >> > checks, but if it already trapped to the hypervisor, we might as well >> > forward it to the vm_event subscriber if there is one and let it >> > decide what it wants to do next (jump over the instruction or crash >> > the domain being the only paths available). >> > >> >> Never mind, found the info "This condition is encoded in ARM >> instructions". So yes, we are always injecting unconditional SMCs for >> monitoring so SMCs with failed condition checks are of no interest. My >> comment above still stands though, we might as well forward these too >> if they trapped to the VMM. >> > > Hi, > > Forwarding SMC events for SMC insns that didn't pass the condition tests > doesn't make any sense to me. It'll just make the receivers job harder. > Why would a receiver want to do anything else than drop these? > If it actually does look at them it'll be looking at implementation > defined HW behaviour that may vary between CPU implementations. If for no other purposes it may be useful to log them to be able to study the CPU implementation's behavior. Tamas
Hello, On 04/06/2016 18:40, Tamas K Lengyel wrote: > On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias > <edgar.iglesias@gmail.com> wrote: >> Forwarding SMC events for SMC insns that didn't pass the condition tests >> doesn't make any sense to me. It'll just make the receivers job harder. >> Why would a receiver want to do anything else than drop these? >> If it actually does look at them it'll be looking at implementation >> defined HW behaviour that may vary between CPU implementations. > > If for no other purposes it may be useful to log them to be able to > study the CPU implementation's behavior. I cannot see how you will be able to study ARM CPU implementation's behavior with VM event. Though I am not familiar with it. For now, it looks like to me that forwarding conditional SMC even if the condition check has failed will require a lot of code in each introspection applications, not to mention that they will need specific code to distinguish ARMv7 vs ARMv8. Anyway, I am planning to send a patch to ignore conditional SMCs if the condition check has failed because this is the right thing to do. Regards,
On Jun 6, 2016 04:08, "Julien Grall" <julien.grall@arm.com> wrote: > > Hello, > > > On 04/06/2016 18:40, Tamas K Lengyel wrote: >> >> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias >> <edgar.iglesias@gmail.com> wrote: >>> >>> Forwarding SMC events for SMC insns that didn't pass the condition tests >>> doesn't make any sense to me. It'll just make the receivers job harder. >>> Why would a receiver want to do anything else than drop these? >>> If it actually does look at them it'll be looking at implementation >>> defined HW behaviour that may vary between CPU implementations. >> >> >> If for no other purposes it may be useful to log them to be able to >> study the CPU implementation's behavior. > > > I cannot see how you will be able to study ARM CPU implementation's behavior with VM event. Though I am not familiar with it. > > For now, it looks like to me that forwarding conditional SMC even if the condition check has failed will require a lot of code in each introspection applications, not to mention that they will need specific code to distinguish ARMv7 vs ARMv8. Why would it require any more code? Right now the only thing the listener can do is to increment pc to jump over the SMC. That would be the same regardless of what type it was. As for checking whether it was v7 or v8, if that is of interest to the app it should implement the appropriate logic for it. I don't see a problem there either. > > Anyway, I am planning to send a patch to ignore conditional SMCs if the condition check has failed because this is the right thing to do. As I said I don't really care for these cases so fine by me. Tamas
Hello Tamas, On 06/06/16 16:24, Tamas K Lengyel wrote: > > On Jun 6, 2016 04:08, "Julien Grall" <julien.grall@arm.com > <mailto:julien.grall@arm.com>> wrote: > > > > Hello, > > > > > > On 04/06/2016 18:40, Tamas K Lengyel wrote: > >> > >> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias > >> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote: > >>> > >>> Forwarding SMC events for SMC insns that didn't pass the condition > tests > >>> doesn't make any sense to me. It'll just make the receivers job harder. > >>> Why would a receiver want to do anything else than drop these? > >>> If it actually does look at them it'll be looking at implementation > >>> defined HW behaviour that may vary between CPU implementations. > >> > >> > >> If for no other purposes it may be useful to log them to be able to > >> study the CPU implementation's behavior. > > > > > > I cannot see how you will be able to study ARM CPU implementation's > behavior with VM event. Though I am not familiar with it. > > > > For now, it looks like to me that forwarding conditional SMC even if > the condition check has failed will require a lot of code in each > introspection applications, not to mention that they will need specific > code to distinguish ARMv7 vs ARMv8. > > Why would it require any more code? Right now the only thing the > listener can do is to increment pc to jump over the SMC. That would be > the same regardless of what type it was. As for checking whether it was > v7 or v8, if that is of interest to the app it should implement the > appropriate logic for it. I don't see a problem there either. A listener can to do more then incrementing pc to jump over the SMC. The app may want to decode the instruction to get the immediate and then execute different actions depending on its value (for instance to emulate some SMC call). For this kind of app, it will be necessary to find out whether the condition check has failed or not before executing it. Regards,
On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote: > Hello Tamas, > > On 06/06/16 16:24, Tamas K Lengyel wrote: >> >> >> On Jun 6, 2016 04:08, "Julien Grall" <julien.grall@arm.com >> <mailto:julien.grall@arm.com>> wrote: >> > >> > Hello, >> > >> > >> > On 04/06/2016 18:40, Tamas K Lengyel wrote: >> >> >> >> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias >> >> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote: >> >>> >> >>> Forwarding SMC events for SMC insns that didn't pass the condition >> tests >> >>> doesn't make any sense to me. It'll just make the receivers job >> harder. >> >>> Why would a receiver want to do anything else than drop these? >> >>> If it actually does look at them it'll be looking at implementation >> >>> defined HW behaviour that may vary between CPU implementations. >> >> >> >> >> >> If for no other purposes it may be useful to log them to be able to >> >> study the CPU implementation's behavior. >> > >> > >> > I cannot see how you will be able to study ARM CPU implementation's >> behavior with VM event. Though I am not familiar with it. >> > >> > For now, it looks like to me that forwarding conditional SMC even if >> the condition check has failed will require a lot of code in each >> introspection applications, not to mention that they will need specific >> code to distinguish ARMv7 vs ARMv8. >> >> Why would it require any more code? Right now the only thing the >> listener can do is to increment pc to jump over the SMC. That would be >> the same regardless of what type it was. As for checking whether it was >> v7 or v8, if that is of interest to the app it should implement the >> appropriate logic for it. I don't see a problem there either. > > > A listener can to do more then incrementing pc to jump over the SMC. The app > may want to decode the instruction to get the immediate and then execute > different actions depending on its value (for instance to emulate some SMC > call). > > For this kind of app, it will be necessary to find out whether the condition > check has failed or not before executing it. > Sure, and if that need arises the extra information can also be forwarded. Especially if as you said you are planning to implement the decoding for Xen to see if it was a failed condition check or not. Tamas
On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote: > On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote: >> Hello Tamas, >> >> On 06/06/16 16:24, Tamas K Lengyel wrote: >>> >>> >>> On Jun 6, 2016 04:08, "Julien Grall" <julien.grall@arm.com >>> <mailto:julien.grall@arm.com>> wrote: >>> > >>> > Hello, >>> > >>> > >>> > On 04/06/2016 18:40, Tamas K Lengyel wrote: >>> >> >>> >> On Sat, Jun 4, 2016 at 3:03 AM, Edgar E. Iglesias >>> >> <edgar.iglesias@gmail.com <mailto:edgar.iglesias@gmail.com>> wrote: >>> >>> >>> >>> Forwarding SMC events for SMC insns that didn't pass the condition >>> tests >>> >>> doesn't make any sense to me. It'll just make the receivers job >>> harder. >>> >>> Why would a receiver want to do anything else than drop these? >>> >>> If it actually does look at them it'll be looking at implementation >>> >>> defined HW behaviour that may vary between CPU implementations. >>> >> >>> >> >>> >> If for no other purposes it may be useful to log them to be able to >>> >> study the CPU implementation's behavior. >>> > >>> > >>> > I cannot see how you will be able to study ARM CPU implementation's >>> behavior with VM event. Though I am not familiar with it. >>> > >>> > For now, it looks like to me that forwarding conditional SMC even if >>> the condition check has failed will require a lot of code in each >>> introspection applications, not to mention that they will need specific >>> code to distinguish ARMv7 vs ARMv8. >>> >>> Why would it require any more code? Right now the only thing the >>> listener can do is to increment pc to jump over the SMC. That would be >>> the same regardless of what type it was. As for checking whether it was >>> v7 or v8, if that is of interest to the app it should implement the >>> appropriate logic for it. I don't see a problem there either. >> >> >> A listener can to do more then incrementing pc to jump over the SMC. The app >> may want to decode the instruction to get the immediate and then execute >> different actions depending on its value (for instance to emulate some SMC >> call). >> >> For this kind of app, it will be necessary to find out whether the condition >> check has failed or not before executing it. >> > > Sure, and if that need arises the extra information can also be > forwarded. Especially if as you said you are planning to implement the > decoding for Xen to see if it was a failed condition check or not. > So either way, I don't see a technical reason why Xen should silently swallow any SMC trap if the vm_event user specifically asked them to be forwarded. Other then it being odd that some ARM chips have varying behavior regarding a subset of SMC instructions, it should not affect when the vm_event user gets the events. If the user requests that it wants to get notified any time an SMC is trapped to the VMM, it should, regardless of whether that makes sense for "us". Depending on the use-case of the user, indeed it may need extra information if it wants to do emulation. If that need arises, the interface can easily be extended to accommodate that usecase. We can also add a comment saying that the forwarded events may also include ones with failed condition checks depending on the CPU implementation. Also, it would also be possible in the future to add a monitor configuration bit where the user can specify if it wants the failed condition check SMCs ignored by default or not. At this time however I want to start simple and just forward all events, adding more bits and pieces only as needed. Tamas
Hi Tamas, On 06/06/16 17:14, Tamas K Lengyel wrote: > On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote: >> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote: > So either way, I don't see a technical reason why Xen should silently > swallow any SMC trap if the vm_event user specifically asked them to > be forwarded. Other then it being odd that some ARM chips have varying > behavior regarding a subset of SMC instructions, it should not affect > when the vm_event user gets the events. If the user requests that it > wants to get notified any time an SMC is trapped to the VMM, it > should, regardless of whether that makes sense for "us". Depending on > the use-case of the user, indeed it may need extra information if it > wants to do emulation. If that need arises, the interface can easily > be extended to accommodate that usecase. We can also add a comment > saying that the forwarded events may also include ones with failed > condition checks depending on the CPU implementation. Also, it would > also be possible in the future to add a monitor configuration bit > where the user can specify if it wants the failed condition check SMCs > ignored by default or not. At this time however I want to start simple > and just forward all events, adding more bits and pieces only as > needed. We disagree on what is a "starting simple". It easier to relax than restricting a behavior later one. Even if we decide to add a bit to ignore some SMC in a later version of Xen, the introspection app will need to carry the burden mentioned in lengthly way on the previous mails because they may want to support older version of Xen. It would not be that difficult to provide a clean interface from beginning, which would allow to support more than your usecase. Anyway, I am not gonna ack this patch for the modification in arch/arm/traps.c because I don't think this is the right way to go. I will let Stefano deciding on this one. Regards,
On Mon, Jun 6, 2016 at 10:38 AM, Julien Grall <julien.grall@arm.com> wrote: > Hi Tamas, > > On 06/06/16 17:14, Tamas K Lengyel wrote: >> >> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com> >> wrote: >>> >>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> >>> wrote: >> >> So either way, I don't see a technical reason why Xen should silently >> swallow any SMC trap if the vm_event user specifically asked them to >> be forwarded. Other then it being odd that some ARM chips have varying >> behavior regarding a subset of SMC instructions, it should not affect >> when the vm_event user gets the events. If the user requests that it >> wants to get notified any time an SMC is trapped to the VMM, it >> should, regardless of whether that makes sense for "us". Depending on >> the use-case of the user, indeed it may need extra information if it >> wants to do emulation. If that need arises, the interface can easily >> be extended to accommodate that usecase. We can also add a comment >> saying that the forwarded events may also include ones with failed >> condition checks depending on the CPU implementation. Also, it would >> also be possible in the future to add a monitor configuration bit >> where the user can specify if it wants the failed condition check SMCs >> ignored by default or not. At this time however I want to start simple >> and just forward all events, adding more bits and pieces only as >> needed. > > > We disagree on what is a "starting simple". It easier to relax than > restricting a behavior later one. > > Even if we decide to add a bit to ignore some SMC in a later version of Xen, > the introspection app will need to carry the burden mentioned in lengthly > way on the previous mails because they may want to support older version of > Xen. > > It would not be that difficult to provide a clean interface from beginning, > which would allow to support more than your usecase. > > Anyway, I am not gonna ack this patch for the modification in > arch/arm/traps.c because I don't think this is the right way to go. I will > let Stefano deciding on this one. > Sounds good to me, my use-case is fine either way so ultimately it's up to you guys. Tamas
>>> On 06.06.16 at 18:38, <julien.grall@arm.com> wrote: > On 06/06/16 17:14, Tamas K Lengyel wrote: >> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote: >>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote: >> So either way, I don't see a technical reason why Xen should silently >> swallow any SMC trap if the vm_event user specifically asked them to >> be forwarded. Other then it being odd that some ARM chips have varying >> behavior regarding a subset of SMC instructions, it should not affect >> when the vm_event user gets the events. If the user requests that it >> wants to get notified any time an SMC is trapped to the VMM, it >> should, regardless of whether that makes sense for "us". Depending on >> the use-case of the user, indeed it may need extra information if it >> wants to do emulation. If that need arises, the interface can easily >> be extended to accommodate that usecase. We can also add a comment >> saying that the forwarded events may also include ones with failed >> condition checks depending on the CPU implementation. Also, it would >> also be possible in the future to add a monitor configuration bit >> where the user can specify if it wants the failed condition check SMCs >> ignored by default or not. At this time however I want to start simple >> and just forward all events, adding more bits and pieces only as >> needed. > > We disagree on what is a "starting simple". It easier to relax than > restricting a behavior later one. > > Even if we decide to add a bit to ignore some SMC in a later version of > Xen, the introspection app will need to carry the burden mentioned in > lengthly way on the previous mails because they may want to support > older version of Xen. FWIW, I'm with Julien here given the information available so far on this thread. Some of the basic problem is that the original patch (and namely its modification to the public header) doesn't really make clear what's intended: To intercept all SMC instruction uses (aiui that's impossible on some hardware) or to intercept all privileged calls resulting from their use (in which case instances with the condition being false wouldn't count). What you, Tamas, want to get to seems to be some middle ground, which I don't see what use it would be to the consumer. Jan
On Tue, 7 Jun 2016, Jan Beulich wrote: > >>> On 06.06.16 at 18:38, <julien.grall@arm.com> wrote: > > On 06/06/16 17:14, Tamas K Lengyel wrote: > >> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote: > >>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote: > >> So either way, I don't see a technical reason why Xen should silently > >> swallow any SMC trap if the vm_event user specifically asked them to > >> be forwarded. Other then it being odd that some ARM chips have varying > >> behavior regarding a subset of SMC instructions, it should not affect > >> when the vm_event user gets the events. If the user requests that it > >> wants to get notified any time an SMC is trapped to the VMM, it > >> should, regardless of whether that makes sense for "us". Depending on > >> the use-case of the user, indeed it may need extra information if it > >> wants to do emulation. If that need arises, the interface can easily > >> be extended to accommodate that usecase. We can also add a comment > >> saying that the forwarded events may also include ones with failed > >> condition checks depending on the CPU implementation. Also, it would > >> also be possible in the future to add a monitor configuration bit > >> where the user can specify if it wants the failed condition check SMCs > >> ignored by default or not. At this time however I want to start simple > >> and just forward all events, adding more bits and pieces only as > >> needed. > > > > We disagree on what is a "starting simple". It easier to relax than > > restricting a behavior later one. > > > > Even if we decide to add a bit to ignore some SMC in a later version of > > Xen, the introspection app will need to carry the burden mentioned in > > lengthly way on the previous mails because they may want to support > > older version of Xen. > > FWIW, I'm with Julien here given the information available so far > on this thread. Some of the basic problem is that the original > patch (and namely its modification to the public header) doesn't > really make clear what's intended: To intercept all SMC instruction > uses (aiui that's impossible on some hardware) or to intercept all > privileged calls resulting from their use (in which case instances > with the condition being false wouldn't count). Right. I think that the first thing to do would be to write down in the public header file what is the intended behavior. Given the scope for confusion, this is necessary regardless of the chosen behavior. > What you, Tamas, want to get to seems to be some middle > ground, which I don't see what use it would be to the consumer. I think that forwarding SMC events only for unconditional SMCs and SMCs which succeeded the conditional check would make for a better interface. This would be my preference. If you really want to forward SMC events for SMCs which failed the conditional check, then please add to the SMC event struct all the necessary information so that the monitoring application can quickly find out whether the conditional check succeeded or failed without jumping through hoops.
On Jun 7, 2016 04:30, "Stefano Stabellini" <sstabellini@kernel.org> wrote: > > On Tue, 7 Jun 2016, Jan Beulich wrote: > > >>> On 06.06.16 at 18:38, <julien.grall@arm.com> wrote: > > > On 06/06/16 17:14, Tamas K Lengyel wrote: > > >> On Mon, Jun 6, 2016 at 9:56 AM, Tamas K Lengyel <tamas@tklengyel.com> wrote: > > >>> On Mon, Jun 6, 2016 at 9:54 AM, Julien Grall <julien.grall@arm.com> wrote: > > >> So either way, I don't see a technical reason why Xen should silently > > >> swallow any SMC trap if the vm_event user specifically asked them to > > >> be forwarded. Other then it being odd that some ARM chips have varying > > >> behavior regarding a subset of SMC instructions, it should not affect > > >> when the vm_event user gets the events. If the user requests that it > > >> wants to get notified any time an SMC is trapped to the VMM, it > > >> should, regardless of whether that makes sense for "us". Depending on > > >> the use-case of the user, indeed it may need extra information if it > > >> wants to do emulation. If that need arises, the interface can easily > > >> be extended to accommodate that usecase. We can also add a comment > > >> saying that the forwarded events may also include ones with failed > > >> condition checks depending on the CPU implementation. Also, it would > > >> also be possible in the future to add a monitor configuration bit > > >> where the user can specify if it wants the failed condition check SMCs > > >> ignored by default or not. At this time however I want to start simple > > >> and just forward all events, adding more bits and pieces only as > > >> needed. > > > > > > We disagree on what is a "starting simple". It easier to relax than > > > restricting a behavior later one. > > > > > > Even if we decide to add a bit to ignore some SMC in a later version of > > > Xen, the introspection app will need to carry the burden mentioned in > > > lengthly way on the previous mails because they may want to support > > > older version of Xen. > > > > FWIW, I'm with Julien here given the information available so far > > on this thread. Some of the basic problem is that the original > > patch (and namely its modification to the public header) doesn't > > really make clear what's intended: To intercept all SMC instruction > > uses (aiui that's impossible on some hardware) or to intercept all > > privileged calls resulting from their use (in which case instances > > with the condition being false wouldn't count). > > Right. I think that the first thing to do would be to write down in the > public header file what is the intended behavior. Given the scope for > confusion, this is necessary regardless of the chosen behavior. > > > > What you, Tamas, want to get to seems to be some middle > > ground, which I don't see what use it would be to the consumer. > > I think that forwarding SMC events only for unconditional SMCs and SMCs > which succeeded the conditional check would make for a better interface. > This would be my preference. > > If you really want to forward SMC events for SMCs which failed the > conditional check, then please add to the SMC event struct all the > necessary information so that the monitoring application can quickly > find out whether the conditional check succeeded or failed without > jumping through hoops. Ack. As I said I have no use for conditional SMCs at all so this is beyond what I am looking for. From my perspective it is just easier to forward every trap. So as for doing the actual filtering, Julien mentioned he is going to add a patch for that. I'll wait for that and then rebase on top. Thanks, Tamas
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index ead0cc0..344d3ad 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -41,6 +41,7 @@ obj-y += decode.o obj-y += processor.o obj-y += smc.o obj-$(CONFIG_XSPLICE) += xsplice.o +obj-y += monitor.o #obj-bin-y += ....o diff --git a/xen/arch/arm/monitor.c b/xen/arch/arm/monitor.c new file mode 100644 index 0000000..90a13dd --- /dev/null +++ b/xen/arch/arm/monitor.c @@ -0,0 +1,84 @@ +/* + * arch/arm/monitor.c + * + * Arch-specific monitor_op domctl handler. + * + * Copyright (c) 2016 Tamas K Lengyel (tamas@tklengyel.com) + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public + * License v2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public + * License along with this program; If not, see <http://www.gnu.org/licenses/>. + */ + +#include <asm/vm_event.h> +#include <public/vm_event.h> + +int arch_monitor_domctl_event(struct domain *d, + struct xen_domctl_monitor_op *mop) +{ + struct arch_domain *ad = &d->arch; + bool_t requested_status = (XEN_DOMCTL_MONITOR_OP_ENABLE == mop->op); + + switch ( mop->event ) + { + case XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL: + { + bool_t old_status = ad->monitor.privileged_call_enabled; + + if ( unlikely(old_status == requested_status) ) + return -EEXIST; + + domain_pause(d); + ad->monitor.privileged_call_enabled = requested_status; + domain_unpause(d); + break; + } + + default: + /* + * Should not be reached unless arch_monitor_get_capabilities() is + * not properly implemented. + */ + ASSERT_UNREACHABLE(); + return -EOPNOTSUPP; + } + + return 0; +} + +bool_t monitor_smc(unsigned long iss, const struct cpu_user_regs *regs) +{ + struct vcpu *curr = current; + + if ( curr->domain->arch.monitor.privileged_call_enabled ) + { + vm_event_request_t req = { 0 }; + + req.reason = VM_EVENT_REASON_PRIVILEGED_CALL; + req.vcpu_id = curr->vcpu_id; + req.u.privcall.type = VM_EVENT_PRIVCALL_SMC; + req.u.privcall.vector = iss; + + if ( vm_event_monitor_traps(curr, 1, &req) > 0 ) + return 1; + } + + return 0; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index aa3e3c2..965c151 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -42,6 +42,7 @@ #include <asm/mmio.h> #include <asm/cpufeature.h> #include <asm/flushtlb.h> +#include <asm/monitor.h> #include "decode.h" #include "vtimer.h" @@ -2506,6 +2507,14 @@ bad_data_abort: inject_dabt_exception(regs, info.gva, hsr.len); } +static void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr) +{ + bool_t handled = monitor_smc(hsr.iss, regs); + + if ( !handled ) + inject_undef_exception(regs, hsr); +} + static void enter_hypervisor_head(struct cpu_user_regs *regs) { if ( guest_mode(regs) ) @@ -2581,7 +2590,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs) */ GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr)); perfc_incr(trap_smc32); - inject_undef32_exception(regs); + do_trap_smc(regs, hsr); break; case HSR_EC_HVC32: GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr)); @@ -2614,7 +2623,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs) */ GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr)); perfc_incr(trap_smc64); - inject_undef64_exception(regs, hsr.len); + do_trap_smc(regs, hsr); break; case HSR_EC_SYSREG: GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr)); diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index 370cdeb..ed56fc9 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -127,6 +127,11 @@ struct arch_domain paddr_t efi_acpi_gpa; paddr_t efi_acpi_len; #endif + + /* Monitor options */ + struct { + uint8_t privileged_call_enabled : 1; + } monitor; } __cacheline_aligned; struct arch_vcpu diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h index 3fd3c9d..0097be2 100644 --- a/xen/include/asm-arm/monitor.h +++ b/xen/include/asm-arm/monitor.h @@ -3,7 +3,7 @@ * * Arch-specific monitor_op domctl handler. * - * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com) + * Copyright (c) 2015-2016 Tamas K Lengyel (tamas@tklengyel.com) * Copyright (c) 2016, Bitdefender S.R.L. * * This program is free software; you can redistribute it and/or @@ -32,27 +32,15 @@ int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop) return -EOPNOTSUPP; } -static inline int arch_monitor_domctl_event(struct domain *d, - struct xen_domctl_monitor_op *mop) -{ - /* - * No arch-specific monitor vm-events on ARM. - * - * Should not be reached unless arch_monitor_get_capabilities() is not - * properly implemented. - */ - ASSERT_UNREACHABLE(); - return -EOPNOTSUPP; -} + struct xen_domctl_monitor_op *mop); static inline uint32_t arch_monitor_get_capabilities(struct domain *d) { - uint32_t capabilities = 0; - - capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST); - - return capabilities; + return (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) | + (1U << XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL); } +bool_t monitor_smc(unsigned long iss, const struct cpu_user_regs *regs); + #endif /* __ASM_ARM_MONITOR_H__ */ diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 2457698..35adce2 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -1080,6 +1080,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t); #define XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP 2 #define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT 3 #define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST 4 +#define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL 5 struct xen_domctl_monitor_op { uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */ diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h index 9270d52..7976080 100644 --- a/xen/include/public/vm_event.h +++ b/xen/include/public/vm_event.h @@ -119,6 +119,8 @@ #define VM_EVENT_REASON_SINGLESTEP 7 /* An event has been requested via HVMOP_guest_request_vm_event. */ #define VM_EVENT_REASON_GUEST_REQUEST 8 +/* Privileged call executed (e.g. SMC) */ +#define VM_EVENT_REASON_PRIVILEGED_CALL 9 /* Supported values for the vm_event_write_ctrlreg index. */ #define VM_EVENT_X86_CR0 0 @@ -212,6 +214,13 @@ struct vm_event_mov_to_msr { uint64_t value; }; +#define VM_EVENT_PRIVCALL_SMC 0 + +struct vm_event_privcall { + uint32_t type; + uint32_t vector; /* ESR_EL2.ISS for SMC calls */ +}; + #define MEM_PAGING_DROP_PAGE (1 << 0) #define MEM_PAGING_EVICT_FAIL (1 << 1) @@ -249,6 +258,7 @@ typedef struct vm_event_st { struct vm_event_mov_to_msr mov_to_msr; struct vm_event_debug software_breakpoint; struct vm_event_debug singlestep; + struct vm_event_privcall privcall; } u; union {