Il 17/06/2014 09:04, Jan Kiszka ha scritto:
+               default1 = vmx_ctl_msr[n].default1;
+               ok = (ctrl.set & default1) == default1 &&
+                       ((ctrl.set ^ ctrl.clr) & ~ctrl.clr) == 0;

Thanks, now I can understand what's going on. :) It can still be simplified though.

This is just (ctrl.set & ~ctrl.clr) == 0:

        (a ^ b) & ~b == 0
->   (a & ~b) ^ (b & ~b) == 0
->   a & ~b == 0

This expresses just as well the concept that set=1, clr=0 is an impossible combination.

Also, it's a bit easier to read if the second line is written as a separate statement:

        ok = ok && (ctrl.set & ~ctrl.clr) == 0;

+               if (ok && basic.ctrl) {
+                       true_ctrl.val = rdmsr(vmx_ctl_msr[n].true_index);
+                       ok = ctrl.clr == true_ctrl.clr &&
+                               ((ctrl.set ^ true_ctrl.set) & ~default1) == 0;

This is

        ((ctrl.set ^ true_ctrl.set) & ~default1 == 0
->   ((ctrl.set & ~default1) ^ (true_ctrl.set & ~default1) == 0
->   ((ctrl.set & ~default1) == (true_ctrl.set & ~default1)

A bit longer, but clearer: the difference between ctrl.set and true_ctrl.set is only that true_ctrl.set can clear some default-1 bits.

Or you can simplify it further:

->      (ctrl.set | default1) == (true_ctrl.set | default1)
->      ctrl.set == (true_ctrl.set | default1)

Also clearer: the difference between ctrl.set and true_ctrl.set is only that default-1 bits must be 1 in ctrl.set. Pick the one you prefer.

Again, using a separate statement makes it easier to read in my opinion; in fact I would also write both statements as

        ok = ok && ...

even though it's redundant for the clr test.

Can you submit v3 of this patch only?

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to