[PATCH] x86/hvm: Fix non-onchangeonly CR write events logic

[PATCH] x86/hvm: Fix non-onchangeonly CR write events logic

[Razvan Cojocaru]

hvm_event_crX() already returns a bool_t to tell us whether an
event will be sent out or not, so the extra check that value != old
is not only useless, but also prevents non-onchangeonly events from
being sent.

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
 xen/arch/x86/hvm/hvm.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index c957610..707ad86 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3368,8 +3368,7 @@ int hvm_set_cr0(unsigned long value, bool_t may_defer)
     }
 
     if ( may_defer && unlikely(currad->monitor.write_ctrlreg_enabled &
-                               monitor_ctrlreg_bitmask(VM_EVENT_X86_CR0)) &&
-         value != old_value )
+                               monitor_ctrlreg_bitmask(VM_EVENT_X86_CR0)) )
     {
         ASSERT(currad->event_write_data != NULL);
 
@@ -3472,8 +3471,7 @@ int hvm_set_cr3(unsigned long value, bool_t may_defer)
     struct arch_domain *currad = &v->domain->arch;
 
     if ( may_defer && unlikely(currad->monitor.write_ctrlreg_enabled &
-                               monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3)) &&
-         value != old )
+                               monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3)) )
     {
         ASSERT(currad->event_write_data != NULL);
 
@@ -3546,8 +3544,7 @@ int hvm_set_cr4(unsigned long value, bool_t may_defer)
     }
 
     if ( may_defer && unlikely(currad->monitor.write_ctrlreg_enabled &
-                               monitor_ctrlreg_bitmask(VM_EVENT_X86_CR4)) &&
-         value != old_cr )
+                               monitor_ctrlreg_bitmask(VM_EVENT_X86_CR4)) )
     {
         ASSERT(currad->event_write_data != NULL);
 
-- 
1.7.9.5

Re: [PATCH] x86/hvm: Fix non-onchangeonly CR write events logic

[Andrew Cooper]

On 12/08/15 18:00, Razvan Cojocaru wrote:
> hvm_event_crX() already returns a bool_t to tell us whether an
> event will be sent out or not, so the extra check that value != old
> is not only useless, but also prevents non-onchangeonly events from
> being sent.
>
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

I presume this is a 4.6 candidate?  If so, you need to CC the release
manager as we are into the RC's

~Andrew

Re: [PATCH] x86/hvm: Fix non-onchangeonly CR write events logic

[Razvan Cojocaru]

On 08/12/2015 08:45 PM, Andrew Cooper wrote:
> On 12/08/15 18:00, Razvan Cojocaru wrote:
>> hvm_event_crX() already returns a bool_t to tell us whether an
>> event will be sent out or not, so the extra check that value != old
>> is not only useless, but also prevents non-onchangeonly events from
>> being sent.
>>
>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> I presume this is a 4.6 candidate?  If so, you need to CC the release
> manager as we are into the RC's

Yes, it is. I've now CCd Wei, thank you for pointing that out!
And sorry for missing this before.


Thanks,
Razvan

Re: [PATCH] x86/hvm: Fix non-onchangeonly CR write events logic

[Wei Liu]

On Wed, Aug 12, 2015 at 08:48:05PM +0300, Razvan Cojocaru wrote:
> On 08/12/2015 08:45 PM, Andrew Cooper wrote:
> > On 12/08/15 18:00, Razvan Cojocaru wrote:
> >> hvm_event_crX() already returns a bool_t to tell us whether an
> >> event will be sent out or not, so the extra check that value != old
> >> is not only useless, but also prevents non-onchangeonly events from
> >> being sent.
> >>
> >> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> > 
> > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > 
> > I presume this is a 4.6 candidate?  If so, you need to CC the release
> > manager as we are into the RC's
> 
> Yes, it is. I've now CCd Wei, thank you for pointing that out!
> And sorry for missing this before.
> 

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

> 
> Thanks,
> Razvan