| Message ID | 1467820469-13538-1-git-send-email-czuzu@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 06.07.16 at 17:54, <czuzu@bitdefender.com> wrote: > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -475,7 +475,7 @@ void hvm_do_resume(struct vcpu *v) > > if ( unlikely(v->arch.vm_event) ) > { > - if ( v->arch.vm_event->emulate_flags ) > + if ( unlikely(v->arch.vm_event->emulate_flags) ) > { > enum emul_kind kind = EMUL_KIND_NORMAL; > Acked-by: Jan Beulich <jbeulich@suse.com> > --- a/xen/arch/x86/vm_event.c > +++ b/xen/arch/x86/vm_event.c > @@ -96,14 +96,16 @@ void vm_event_register_write_resume(struct vcpu *v, > vm_event_response_t *rsp) > { > if ( rsp->flags & VM_EVENT_FLAG_DENY ) > { > - struct monitor_write_data *w = &v->arch.vm_event->write_data; > + struct monitor_write_data *w; > > - ASSERT(w); > + ASSERT(v->arch.vm_event); > > /* deny flag requires the vCPU to be paused */ > if ( !atomic_read(&v->vm_event_pause_count) ) > return; > > + w = &v->arch.vm_event->write_data; > + > switch ( rsp->reason ) > { > case VM_EVENT_REASON_MOV_TO_MSR: I'd have preferred for you to leave alone the initializer, but we'll see what the maintainers are going to say. Jan
On 7/7/2016 11:27 AM, Jan Beulich wrote: >>>> On 06.07.16 at 17:54, <czuzu@bitdefender.com> wrote: >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -475,7 +475,7 @@ void hvm_do_resume(struct vcpu *v) >> >> if ( unlikely(v->arch.vm_event) ) >> { >> - if ( v->arch.vm_event->emulate_flags ) >> + if ( unlikely(v->arch.vm_event->emulate_flags) ) >> { >> enum emul_kind kind = EMUL_KIND_NORMAL; >> > Acked-by: Jan Beulich <jbeulich@suse.com> > >> --- a/xen/arch/x86/vm_event.c >> +++ b/xen/arch/x86/vm_event.c >> @@ -96,14 +96,16 @@ void vm_event_register_write_resume(struct vcpu *v, >> vm_event_response_t *rsp) >> { >> if ( rsp->flags & VM_EVENT_FLAG_DENY ) >> { >> - struct monitor_write_data *w = &v->arch.vm_event->write_data; >> + struct monitor_write_data *w; >> >> - ASSERT(w); >> + ASSERT(v->arch.vm_event); >> >> /* deny flag requires the vCPU to be paused */ >> if ( !atomic_read(&v->vm_event_pause_count) ) >> return; >> >> + w = &v->arch.vm_event->write_data; >> + >> switch ( rsp->reason ) >> { >> case VM_EVENT_REASON_MOV_TO_MSR: > I'd have preferred for you to leave alone the initializer, but we'll see > what the maintainers are going to say. > > Jan It's 'cleaner' this way, doesn't it? Not assigning a pointer to a possibly invalid address... Anyway, I'm preparing a v4, I'll probably drop this change if you won't *subdue* to ack it (kidding). Thanks, Corneliu.
>>> On 07.07.16 at 10:35, <czuzu@bitdefender.com> wrote: > On 7/7/2016 11:27 AM, Jan Beulich wrote: >>>>> On 06.07.16 at 17:54, <czuzu@bitdefender.com> wrote: >>> --- a/xen/arch/x86/vm_event.c >>> +++ b/xen/arch/x86/vm_event.c >>> @@ -96,14 +96,16 @@ void vm_event_register_write_resume(struct vcpu *v, >>> vm_event_response_t *rsp) >>> { >>> if ( rsp->flags & VM_EVENT_FLAG_DENY ) >>> { >>> - struct monitor_write_data *w = &v->arch.vm_event->write_data; >>> + struct monitor_write_data *w; >>> >>> - ASSERT(w); >>> + ASSERT(v->arch.vm_event); >>> >>> /* deny flag requires the vCPU to be paused */ >>> if ( !atomic_read(&v->vm_event_pause_count) ) >>> return; >>> >>> + w = &v->arch.vm_event->write_data; >>> + >>> switch ( rsp->reason ) >>> { >>> case VM_EVENT_REASON_MOV_TO_MSR: >> I'd have preferred for you to leave alone the initializer, but we'll see >> what the maintainers are going to say. > > It's 'cleaner' this way, doesn't it? Not assigning a pointer to a > possibly invalid address... It's a matter of taste, hence subject to the maintainers' opinions. > Anyway, I'm preparing a v4, I'll probably drop this change if you won't > *subdue* to ack it (kidding). I can't ack it anyway. Jan
On Wed, Jul 6, 2016 at 9:54 AM, Corneliu ZUZU <czuzu@bitdefender.com> wrote: > Minor fixes: > > * vm_event_register_write_resume: ASSERT on non-NULL v->arch.vm_event instead of > &v->arch.vm_event->write_data. > * add 'unlikely' in if > > Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index ac6d9eb..091cabe 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -475,7 +475,7 @@ void hvm_do_resume(struct vcpu *v) if ( unlikely(v->arch.vm_event) ) { - if ( v->arch.vm_event->emulate_flags ) + if ( unlikely(v->arch.vm_event->emulate_flags) ) { enum emul_kind kind = EMUL_KIND_NORMAL; diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c index ff2ba92..e37d6d3 100644 --- a/xen/arch/x86/vm_event.c +++ b/xen/arch/x86/vm_event.c @@ -96,14 +96,16 @@ void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp) { if ( rsp->flags & VM_EVENT_FLAG_DENY ) { - struct monitor_write_data *w = &v->arch.vm_event->write_data; + struct monitor_write_data *w; - ASSERT(w); + ASSERT(v->arch.vm_event); /* deny flag requires the vCPU to be paused */ if ( !atomic_read(&v->vm_event_pause_count) ) return; + w = &v->arch.vm_event->write_data; + switch ( rsp->reason ) { case VM_EVENT_REASON_MOV_TO_MSR:
Minor fixes: * vm_event_register_write_resume: ASSERT on non-NULL v->arch.vm_event instead of &v->arch.vm_event->write_data. * add 'unlikely' in if Signed-off-by: Corneliu ZUZU <czuzu@bitdefender.com> --- Changed since v2: <nothing> --- xen/arch/x86/hvm/hvm.c | 2 +- xen/arch/x86/vm_event.c | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-)