Il 29/07/2013 14:34, Gleb Natapov ha scritto: >>>> But I think what you _really_ want is not avoiding conditional branches. >>> The idea is that it is hard for branch prediction to predict correct >>> result when correct result depends on guest's page table that can >>> contain anything, so in some places shadow paging code uses boolean >>> logic to avoid branches, in this case it is hard to avoid if() anyway >>> since the function invocation is in the if(). >> >> Yes, I get the idea, but is_rsvd_bits_set should be predicted unlikely, >> no? If the guest has to run, it must use mostly valid ptes. :) >> > You see, you are confused and you want branch prediction not to be? :) > If your guest is KVM is_rsvd_bits_set() will be likely much more then > unlikely because KVM misconfigures EPT entries to cache MMIO addresses, > so all the "unlikely" cases will be fixed by shadow pages and will not > reappear (until shadow pages are zapped), but misconfigured entries will > continue to produces violations. But then: 1) MMIO is a slow path anyway, losing 10 cycles on a mispredicted branch is not going to help much. Fast page faults are all I would optimize for. 2) in cases like this you just do not use likely/unlikely; the branch will be very unlikely in the beginning, and very likely once shadow pages are filled or in the no-EPT case. Just let the branch predictor adjust, it will probably do better than boolean tricks. >> Especially if you change prefetch_invalid_gpte to do the reserved bits >> test after the present test (so that is_rsvd_bits_set is only called on >> present pagetables), is_rsvd_bits_set's result should be really >> well-predicted. > Nope, for ept page tables present is not a single bit, it is three bits > which by themselves can have invalid values. We're not checking the validity of the bits in the is_present_gpte test, we're checking it in the is_rsvd_bits_set test (is_present_gpte is doing just "(pte & 7) != 0"). It doesn't change anything in the outcome of prefetch_invalid_gpte, and it makes the ordering consistent with walk_addr_generic which already tests presence before reserved bits. So doing this swap should be a win anyway. >> At this point (and especially since function invocation >> is always in "if"s), using boolean logic to avoid branches does not make >> much sense anymore for this function. > > That's true. So are you going to change to "if"s? Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
