| Message ID | 1493802603-4978-2-git-send-email-rcojocaru@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com> wrote: > @@ -483,67 +483,7 @@ void hvm_do_resume(struct vcpu *v) > if ( !handle_hvm_io_completion(v) ) > return; > > - if ( unlikely(v->arch.vm_event) ) > - { > - struct monitor_write_data *w = &v->arch.vm_event->write_data; > - > - if ( unlikely(v->arch.vm_event->emulate_flags) ) > - { > - enum emul_kind kind = EMUL_KIND_NORMAL; > - > - /* > - * Please observ the order here to match the flag descriptions > - * provided in public/vm_event.h > - */ > - if ( v->arch.vm_event->emulate_flags & > - VM_EVENT_FLAG_SET_EMUL_READ_DATA ) > - kind = EMUL_KIND_SET_CONTEXT_DATA; > - else if ( v->arch.vm_event->emulate_flags & > - VM_EVENT_FLAG_EMULATE_NOWRITE ) > - kind = EMUL_KIND_NOWRITE; > - else if ( v->arch.vm_event->emulate_flags & > - VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) > - kind = EMUL_KIND_SET_CONTEXT_INSN; > - > - hvm_emulate_one_vm_event(kind, TRAP_invalid_op, > - X86_EVENT_NO_EC); > - > - v->arch.vm_event->emulate_flags = 0; > - } > - > - if ( w->do_write.msr ) > - { > - if ( hvm_msr_write_intercept(w->msr, w->value, 0) == > - X86EMUL_EXCEPTION ) > - hvm_inject_hw_exception(TRAP_gp_fault, 0); > - > - w->do_write.msr = 0; > - } > - > - if ( w->do_write.cr0 ) > - { > - if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION ) > - hvm_inject_hw_exception(TRAP_gp_fault, 0); > - > - w->do_write.cr0 = 0; > - } > - > - if ( w->do_write.cr4 ) > - { > - if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION ) > - hvm_inject_hw_exception(TRAP_gp_fault, 0); > - > - w->do_write.cr4 = 0; > - } > - > - if ( w->do_write.cr3 ) > - { > - if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION ) > - hvm_inject_hw_exception(TRAP_gp_fault, 0); > - > - w->do_write.cr3 = 0; > - } > - } > + hvm_vm_event_do_resume(v); As indicated before, I think we want to keep if ( unlikely(v->arch.vm_event) ) here of in an inline wrapper, to avoid the actual function call in the common case. > --- /dev/null > +++ b/xen/arch/x86/hvm/vm_event.c > @@ -0,0 +1,101 @@ > +/* > + * arch/x86/hvm/vm_event.c > + * > + * HVM vm_event handling routines > + * > + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com) I'm notoriously bad when it comes to copyrights, but you just moving code makes me wonder whether this is appropriate. > +void hvm_vm_event_do_resume(struct vcpu *v) > +{ > + struct monitor_write_data *w; > + > + if ( likely(!v->arch.vm_event) ) > + return; > + > + w = &v->arch.vm_event->write_data; > + > + if ( unlikely(v->arch.vm_event->emulate_flags) ) > + { > + enum emul_kind kind = EMUL_KIND_NORMAL; > + > + /* > + * Please observe the order here to match the flag descriptions > + * provided in public/vm_event.h > + */ > + if ( v->arch.vm_event->emulate_flags & > + VM_EVENT_FLAG_SET_EMUL_READ_DATA ) > + kind = EMUL_KIND_SET_CONTEXT_DATA; > + else if ( v->arch.vm_event->emulate_flags & > + VM_EVENT_FLAG_EMULATE_NOWRITE ) > + kind = EMUL_KIND_NOWRITE; > + else if ( v->arch.vm_event->emulate_flags & > + VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) > + kind = EMUL_KIND_SET_CONTEXT_INSN; > + > + hvm_emulate_one_vm_event(kind, TRAP_invalid_op, > + X86_EVENT_NO_EC); > + > + v->arch.vm_event->emulate_flags = 0; > + } > + > + if ( w->do_write.cr0 ) > + { > + if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION ) > + hvm_inject_hw_exception(TRAP_gp_fault, 0); > + > + w->do_write.cr0 = 0; > + } > + > + if ( w->do_write.cr4 ) > + { > + if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION ) > + hvm_inject_hw_exception(TRAP_gp_fault, 0); > + > + w->do_write.cr4 = 0; > + } > + > + if ( w->do_write.cr3 ) > + { > + if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION ) > + hvm_inject_hw_exception(TRAP_gp_fault, 0); > + > + w->do_write.cr3 = 0; > + } > + > + if ( w->do_write.msr ) > + { > + if ( hvm_msr_write_intercept(w->msr, w->value, 0) == > + X86EMUL_EXCEPTION ) > + hvm_inject_hw_exception(TRAP_gp_fault, 0); > + > + w->do_write.msr = 0; > + } I wonder whether all of these outer if()-s wouldn't better have unlikely() too. Jan
On 05/03/17 12:51, Jan Beulich wrote: >>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com> wrote: >> @@ -483,67 +483,7 @@ void hvm_do_resume(struct vcpu *v) >> if ( !handle_hvm_io_completion(v) ) >> return; >> >> - if ( unlikely(v->arch.vm_event) ) >> - { >> - struct monitor_write_data *w = &v->arch.vm_event->write_data; >> - >> - if ( unlikely(v->arch.vm_event->emulate_flags) ) >> - { >> - enum emul_kind kind = EMUL_KIND_NORMAL; >> - >> - /* >> - * Please observ the order here to match the flag descriptions >> - * provided in public/vm_event.h >> - */ >> - if ( v->arch.vm_event->emulate_flags & >> - VM_EVENT_FLAG_SET_EMUL_READ_DATA ) >> - kind = EMUL_KIND_SET_CONTEXT_DATA; >> - else if ( v->arch.vm_event->emulate_flags & >> - VM_EVENT_FLAG_EMULATE_NOWRITE ) >> - kind = EMUL_KIND_NOWRITE; >> - else if ( v->arch.vm_event->emulate_flags & >> - VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) >> - kind = EMUL_KIND_SET_CONTEXT_INSN; >> - >> - hvm_emulate_one_vm_event(kind, TRAP_invalid_op, >> - X86_EVENT_NO_EC); >> - >> - v->arch.vm_event->emulate_flags = 0; >> - } >> - >> - if ( w->do_write.msr ) >> - { >> - if ( hvm_msr_write_intercept(w->msr, w->value, 0) == >> - X86EMUL_EXCEPTION ) >> - hvm_inject_hw_exception(TRAP_gp_fault, 0); >> - >> - w->do_write.msr = 0; >> - } >> - >> - if ( w->do_write.cr0 ) >> - { >> - if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION ) >> - hvm_inject_hw_exception(TRAP_gp_fault, 0); >> - >> - w->do_write.cr0 = 0; >> - } >> - >> - if ( w->do_write.cr4 ) >> - { >> - if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION ) >> - hvm_inject_hw_exception(TRAP_gp_fault, 0); >> - >> - w->do_write.cr4 = 0; >> - } >> - >> - if ( w->do_write.cr3 ) >> - { >> - if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION ) >> - hvm_inject_hw_exception(TRAP_gp_fault, 0); >> - >> - w->do_write.cr3 = 0; >> - } >> - } >> + hvm_vm_event_do_resume(v); > > As indicated before, I think we want to keep > > if ( unlikely(v->arch.vm_event) ) > > here of in an inline wrapper, to avoid the actual function call in the > common case. Will do. >> --- /dev/null >> +++ b/xen/arch/x86/hvm/vm_event.c >> @@ -0,0 +1,101 @@ >> +/* >> + * arch/x86/hvm/vm_event.c >> + * >> + * HVM vm_event handling routines >> + * >> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com) > > I'm notoriously bad when it comes to copyrights, but you just > moving code makes me wonder whether this is appropriate. To be honest I quite agree with you, and in the beginning I just meant to have no Copyright line in there at all - but I remembered a discussion a while back where a patch was I believe rejected because it lacked one. So I've just copied Tamas' file (vm_event.c) and only changed the copyright line because I didn't really know what else to put there. I'm quite happy to remove it altogether. Will that do? >> +void hvm_vm_event_do_resume(struct vcpu *v) >> +{ >> + struct monitor_write_data *w; >> + >> + if ( likely(!v->arch.vm_event) ) >> + return; >> + >> + w = &v->arch.vm_event->write_data; >> + >> + if ( unlikely(v->arch.vm_event->emulate_flags) ) >> + { >> + enum emul_kind kind = EMUL_KIND_NORMAL; >> + >> + /* >> + * Please observe the order here to match the flag descriptions >> + * provided in public/vm_event.h >> + */ >> + if ( v->arch.vm_event->emulate_flags & >> + VM_EVENT_FLAG_SET_EMUL_READ_DATA ) >> + kind = EMUL_KIND_SET_CONTEXT_DATA; >> + else if ( v->arch.vm_event->emulate_flags & >> + VM_EVENT_FLAG_EMULATE_NOWRITE ) >> + kind = EMUL_KIND_NOWRITE; >> + else if ( v->arch.vm_event->emulate_flags & >> + VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) >> + kind = EMUL_KIND_SET_CONTEXT_INSN; >> + >> + hvm_emulate_one_vm_event(kind, TRAP_invalid_op, >> + X86_EVENT_NO_EC); >> + >> + v->arch.vm_event->emulate_flags = 0; >> + } >> + >> + if ( w->do_write.cr0 ) >> + { >> + if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION ) >> + hvm_inject_hw_exception(TRAP_gp_fault, 0); >> + >> + w->do_write.cr0 = 0; >> + } >> + >> + if ( w->do_write.cr4 ) >> + { >> + if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION ) >> + hvm_inject_hw_exception(TRAP_gp_fault, 0); >> + >> + w->do_write.cr4 = 0; >> + } >> + >> + if ( w->do_write.cr3 ) >> + { >> + if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION ) >> + hvm_inject_hw_exception(TRAP_gp_fault, 0); >> + >> + w->do_write.cr3 = 0; >> + } >> + >> + if ( w->do_write.msr ) >> + { >> + if ( hvm_msr_write_intercept(w->msr, w->value, 0) == >> + X86EMUL_EXCEPTION ) >> + hvm_inject_hw_exception(TRAP_gp_fault, 0); >> + >> + w->do_write.msr = 0; >> + } > > I wonder whether all of these outer if()-s wouldn't better have > unlikely() too. It can't hurt, unless anyone objects I'll wrap them in unlikely()s. Thanks, Razvan
>>> On 03.05.17 at 12:37, <rcojocaru@bitdefender.com> wrote: > On 05/03/17 12:51, Jan Beulich wrote: >>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com> wrote: >>> --- /dev/null >>> +++ b/xen/arch/x86/hvm/vm_event.c >>> @@ -0,0 +1,101 @@ >>> +/* >>> + * arch/x86/hvm/vm_event.c >>> + * >>> + * HVM vm_event handling routines >>> + * >>> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com) >> >> I'm notoriously bad when it comes to copyrights, but you just >> moving code makes me wonder whether this is appropriate. > > To be honest I quite agree with you, and in the beginning I just meant > to have no Copyright line in there at all - but I remembered a > discussion a while back where a patch was I believe rejected because it > lacked one. So I've just copied Tamas' file (vm_event.c) and only > changed the copyright line because I didn't really know what else to put > there. > > I'm quite happy to remove it altogether. Will that do? Afaic - sure. But as said, I'm quite bad at such things ... Jan
On Wed, May 3, 2017 at 6:48 AM, Jan Beulich <JBeulich@suse.com> wrote: > >>> On 03.05.17 at 12:37, <rcojocaru@bitdefender.com> wrote: > > On 05/03/17 12:51, Jan Beulich wrote: > >>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com> wrote: > >>> --- /dev/null > >>> +++ b/xen/arch/x86/hvm/vm_event.c > >>> @@ -0,0 +1,101 @@ > >>> +/* > >>> + * arch/x86/hvm/vm_event.c > >>> + * > >>> + * HVM vm_event handling routines > >>> + * > >>> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com) > >> > >> I'm notoriously bad when it comes to copyrights, but you just > >> moving code makes me wonder whether this is appropriate. > > > > To be honest I quite agree with you, and in the beginning I just meant > > to have no Copyright line in there at all - but I remembered a > > discussion a while back where a patch was I believe rejected because it > > lacked one. So I've just copied Tamas' file (vm_event.c) and only > > changed the copyright line because I didn't really know what else to put > > there. > > > > I'm quite happy to remove it altogether. Will that do? > > Afaic - sure. But as said, I'm quite bad at such things ... > Since this is just code-movement from hvm.c to a separate file I would say it should retain the copyright lines from hvm.c. Other then that it looks good to me. Tamas
On 05/03/2017 11:05 PM, Tamas K Lengyel wrote: > > > On Wed, May 3, 2017 at 6:48 AM, Jan Beulich <JBeulich@suse.com > <mailto:JBeulich@suse.com>> wrote: > > >>> On 03.05.17 at 12:37, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote: > > On 05/03/17 12:51, Jan Beulich wrote: > >>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote: > >>> --- /dev/null > >>> +++ b/xen/arch/x86/hvm/vm_event.c > >>> @@ -0,0 +1,101 @@ > >>> +/* > >>> + * arch/x86/hvm/vm_event.c > >>> + * > >>> + * HVM vm_event handling routines > >>> + * > >>> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>) > >> > >> I'm notoriously bad when it comes to copyrights, but you just > >> moving code makes me wonder whether this is appropriate. > > > > To be honest I quite agree with you, and in the beginning I just meant > > to have no Copyright line in there at all - but I remembered a > > discussion a while back where a patch was I believe rejected because it > > lacked one. So I've just copied Tamas' file (vm_event.c) and only > > changed the copyright line because I didn't really know what else to put > > there. > > > > I'm quite happy to remove it altogether. Will that do? > > Afaic - sure. But as said, I'm quite bad at such things ... > > > Since this is just code-movement from hvm.c to a separate file I would > say it should retain the copyright lines from hvm.c. Other then that it > looks good to me. Actually the funny part about that is that while this is indeed only moved code, I have written all of that code in the first place, so I've moved my own code. :) But I have no problem with either removing the copyright line altogether or using the lines in hvm.c as suggested. Thanks, Razvan
On Wed, May 3, 2017 at 4:16 PM, Razvan Cojocaru <rcojocaru@bitdefender.com> wrote: > On 05/03/2017 11:05 PM, Tamas K Lengyel wrote: >> >> >> On Wed, May 3, 2017 at 6:48 AM, Jan Beulich <JBeulich@suse.com >> <mailto:JBeulich@suse.com>> wrote: >> >> >>> On 03.05.17 at 12:37, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote: >> > On 05/03/17 12:51, Jan Beulich wrote: >> >>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote: >> >>> --- /dev/null >> >>> +++ b/xen/arch/x86/hvm/vm_event.c >> >>> @@ -0,0 +1,101 @@ >> >>> +/* >> >>> + * arch/x86/hvm/vm_event.c >> >>> + * >> >>> + * HVM vm_event handling routines >> >>> + * >> >>> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>) >> >> >> >> I'm notoriously bad when it comes to copyrights, but you just >> >> moving code makes me wonder whether this is appropriate. >> > >> > To be honest I quite agree with you, and in the beginning I just meant >> > to have no Copyright line in there at all - but I remembered a >> > discussion a while back where a patch was I believe rejected because it >> > lacked one. So I've just copied Tamas' file (vm_event.c) and only >> > changed the copyright line because I didn't really know what else to put >> > there. >> > >> > I'm quite happy to remove it altogether. Will that do? >> >> Afaic - sure. But as said, I'm quite bad at such things ... >> >> >> Since this is just code-movement from hvm.c to a separate file I would >> say it should retain the copyright lines from hvm.c. Other then that it >> looks good to me. > > Actually the funny part about that is that while this is indeed only > moved code, I have written all of that code in the first place, so I've > moved my own code. :) Well, I've also added like two lines to it with VM_EVENT_FLAG_SET_EMUL_INSN_DATA ;) > > But I have no problem with either removing the copyright line altogether > or using the lines in hvm.c as suggested. Yeap, I'm fine with either route but the safe bet is to just use the one from hvm.c (and add yourself to it if you feel like). Tamas
On 05/03/2017 11:32 PM, Tamas K Lengyel wrote: > On Wed, May 3, 2017 at 4:16 PM, Razvan Cojocaru > <rcojocaru@bitdefender.com> wrote: >> On 05/03/2017 11:05 PM, Tamas K Lengyel wrote: >>> >>> >>> On Wed, May 3, 2017 at 6:48 AM, Jan Beulich <JBeulich@suse.com >>> <mailto:JBeulich@suse.com>> wrote: >>> >>> >>> On 03.05.17 at 12:37, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote: >>> > On 05/03/17 12:51, Jan Beulich wrote: >>> >>>>> On 03.05.17 at 11:10, <rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>> wrote: >>> >>> --- /dev/null >>> >>> +++ b/xen/arch/x86/hvm/vm_event.c >>> >>> @@ -0,0 +1,101 @@ >>> >>> +/* >>> >>> + * arch/x86/hvm/vm_event.c >>> >>> + * >>> >>> + * HVM vm_event handling routines >>> >>> + * >>> >>> + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.com <mailto:rcojocaru@bitdefender.com>) >>> >> >>> >> I'm notoriously bad when it comes to copyrights, but you just >>> >> moving code makes me wonder whether this is appropriate. >>> > >>> > To be honest I quite agree with you, and in the beginning I just meant >>> > to have no Copyright line in there at all - but I remembered a >>> > discussion a while back where a patch was I believe rejected because it >>> > lacked one. So I've just copied Tamas' file (vm_event.c) and only >>> > changed the copyright line because I didn't really know what else to put >>> > there. >>> > >>> > I'm quite happy to remove it altogether. Will that do? >>> >>> Afaic - sure. But as said, I'm quite bad at such things ... >>> >>> >>> Since this is just code-movement from hvm.c to a separate file I would >>> say it should retain the copyright lines from hvm.c. Other then that it >>> looks good to me. >> >> Actually the funny part about that is that while this is indeed only >> moved code, I have written all of that code in the first place, so I've >> moved my own code. :) > > Well, I've also added like two lines to it with > VM_EVENT_FLAG_SET_EMUL_INS_DATA ;) Right, my bad. :) >> But I have no problem with either removing the copyright line altogether >> or using the lines in hvm.c as suggested. > > Yeap, I'm fine with either route but the safe bet is to just use the > one from hvm.c (and add yourself to it if you feel like). Alright, I'll just copy it from hvm.c. Thanks, Razvan
diff --git a/MAINTAINERS b/MAINTAINERS index c345a50..10863bc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -404,6 +404,7 @@ S: Supported F: tools/tests/xen-access F: xen/arch/*/monitor.c F: xen/arch/*/vm_event.c +F: xen/arch/*/hvm/vm_event.c F: xen/arch/arm/mem_access.c F: xen/arch/x86/mm/mem_access.c F: xen/arch/x86/hvm/monitor.c @@ -413,6 +414,7 @@ F: xen/common/vm_event.c F: xen/include/*/mem_access.h F: xen/include/*/monitor.h F: xen/include/*/vm_event.h +F: xen/include/*/hvm/vm_event.h F: xen/include/asm-x86/hvm/monitor.h VTPM diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile index 0a3d0f4..02f0add 100644 --- a/xen/arch/x86/hvm/Makefile +++ b/xen/arch/x86/hvm/Makefile @@ -24,6 +24,7 @@ obj-y += stdvga.o obj-y += vioapic.o obj-y += viridian.o obj-y += vlapic.o +obj-y += vm_event.o obj-y += vmsi.o obj-y += vpic.o obj-y += vpt.o diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index a441955..f010394 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -34,7 +34,6 @@ #include <xen/wait.h> #include <xen/mem_access.h> #include <xen/rangeset.h> -#include <xen/vm_event.h> #include <xen/monitor.h> #include <xen/warning.h> #include <asm/shadow.h> @@ -61,6 +60,7 @@ #include <asm/hvm/nestedhvm.h> #include <asm/hvm/monitor.h> #include <asm/hvm/ioreq.h> +#include <asm/hvm/vm_event.h> #include <asm/altp2m.h> #include <asm/mtrr.h> #include <asm/apic.h> @@ -483,67 +483,7 @@ void hvm_do_resume(struct vcpu *v) if ( !handle_hvm_io_completion(v) ) return; - if ( unlikely(v->arch.vm_event) ) - { - struct monitor_write_data *w = &v->arch.vm_event->write_data; - - if ( unlikely(v->arch.vm_event->emulate_flags) ) - { - enum emul_kind kind = EMUL_KIND_NORMAL; - - /* - * Please observ the order here to match the flag descriptions - * provided in public/vm_event.h - */ - if ( v->arch.vm_event->emulate_flags & - VM_EVENT_FLAG_SET_EMUL_READ_DATA ) - kind = EMUL_KIND_SET_CONTEXT_DATA; - else if ( v->arch.vm_event->emulate_flags & - VM_EVENT_FLAG_EMULATE_NOWRITE ) - kind = EMUL_KIND_NOWRITE; - else if ( v->arch.vm_event->emulate_flags & - VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) - kind = EMUL_KIND_SET_CONTEXT_INSN; - - hvm_emulate_one_vm_event(kind, TRAP_invalid_op, - X86_EVENT_NO_EC); - - v->arch.vm_event->emulate_flags = 0; - } - - if ( w->do_write.msr ) - { - if ( hvm_msr_write_intercept(w->msr, w->value, 0) == - X86EMUL_EXCEPTION ) - hvm_inject_hw_exception(TRAP_gp_fault, 0); - - w->do_write.msr = 0; - } - - if ( w->do_write.cr0 ) - { - if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION ) - hvm_inject_hw_exception(TRAP_gp_fault, 0); - - w->do_write.cr0 = 0; - } - - if ( w->do_write.cr4 ) - { - if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION ) - hvm_inject_hw_exception(TRAP_gp_fault, 0); - - w->do_write.cr4 = 0; - } - - if ( w->do_write.cr3 ) - { - if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION ) - hvm_inject_hw_exception(TRAP_gp_fault, 0); - - w->do_write.cr3 = 0; - } - } + hvm_vm_event_do_resume(v); /* Inject pending hw/sw event */ if ( v->arch.hvm_vcpu.inject_event.vector >= 0 ) diff --git a/xen/arch/x86/hvm/vm_event.c b/xen/arch/x86/hvm/vm_event.c new file mode 100644 index 0000000..b35ee12 --- /dev/null +++ b/xen/arch/x86/hvm/vm_event.c @@ -0,0 +1,101 @@ +/* + * arch/x86/hvm/vm_event.c + * + * HVM vm_event handling routines + * + * Copyright (c) 2017 Razvan Cojocaru (rcojocaru@bitdefender.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 <xen/sched.h> +#include <xen/vm_event.h> +#include <asm/hvm/support.h> +#include <asm/vm_event.h> + +void hvm_vm_event_do_resume(struct vcpu *v) +{ + struct monitor_write_data *w; + + if ( likely(!v->arch.vm_event) ) + return; + + w = &v->arch.vm_event->write_data; + + if ( unlikely(v->arch.vm_event->emulate_flags) ) + { + enum emul_kind kind = EMUL_KIND_NORMAL; + + /* + * Please observe the order here to match the flag descriptions + * provided in public/vm_event.h + */ + if ( v->arch.vm_event->emulate_flags & + VM_EVENT_FLAG_SET_EMUL_READ_DATA ) + kind = EMUL_KIND_SET_CONTEXT_DATA; + else if ( v->arch.vm_event->emulate_flags & + VM_EVENT_FLAG_EMULATE_NOWRITE ) + kind = EMUL_KIND_NOWRITE; + else if ( v->arch.vm_event->emulate_flags & + VM_EVENT_FLAG_SET_EMUL_INSN_DATA ) + kind = EMUL_KIND_SET_CONTEXT_INSN; + + hvm_emulate_one_vm_event(kind, TRAP_invalid_op, + X86_EVENT_NO_EC); + + v->arch.vm_event->emulate_flags = 0; + } + + if ( w->do_write.cr0 ) + { + if ( hvm_set_cr0(w->cr0, 0) == X86EMUL_EXCEPTION ) + hvm_inject_hw_exception(TRAP_gp_fault, 0); + + w->do_write.cr0 = 0; + } + + if ( w->do_write.cr4 ) + { + if ( hvm_set_cr4(w->cr4, 0) == X86EMUL_EXCEPTION ) + hvm_inject_hw_exception(TRAP_gp_fault, 0); + + w->do_write.cr4 = 0; + } + + if ( w->do_write.cr3 ) + { + if ( hvm_set_cr3(w->cr3, 0) == X86EMUL_EXCEPTION ) + hvm_inject_hw_exception(TRAP_gp_fault, 0); + + w->do_write.cr3 = 0; + } + + if ( w->do_write.msr ) + { + if ( hvm_msr_write_intercept(w->msr, w->value, 0) == + X86EMUL_EXCEPTION ) + hvm_inject_hw_exception(TRAP_gp_fault, 0); + + w->do_write.msr = 0; + } +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/asm-x86/hvm/vm_event.h b/xen/include/asm-x86/hvm/vm_event.h new file mode 100644 index 0000000..515fa85 --- /dev/null +++ b/xen/include/asm-x86/hvm/vm_event.h @@ -0,0 +1,34 @@ +/* + * include/asm-x86/hvm/vm_event.h + * + * Hardware virtual machine vm_event abstractions. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope 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/>. + */ + +#ifndef __ASM_X86_HVM_VM_EVENT_H__ +#define __ASM_X86_HVM_VM_EVENT_H__ + +void hvm_vm_event_do_resume(struct vcpu *v); + +#endif /* __ASM_X86_HVM_MONITOR_H__ */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */
Created arch/x86/hvm/vm_event.c and include/asm-x86/hvm/vm_event.h, where HVM-specific vm_event-related code will live. This cleans up hvm_do_resume() and ensures that the vm_event maintainers are responsible for changes to that code. Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com> --- MAINTAINERS | 2 + xen/arch/x86/hvm/Makefile | 1 + xen/arch/x86/hvm/hvm.c | 64 +---------------------- xen/arch/x86/hvm/vm_event.c | 101 +++++++++++++++++++++++++++++++++++++ xen/include/asm-x86/hvm/vm_event.h | 34 +++++++++++++ 5 files changed, 140 insertions(+), 62 deletions(-) create mode 100644 xen/arch/x86/hvm/vm_event.c create mode 100644 xen/include/asm-x86/hvm/vm_event.h