On 12/06/18 10:00, Jan Beulich wrote: >>>> On 12.06.18 at 10:51, <andrew.coop...@citrix.com> wrote: >> On 12/06/2018 09:15, Jan Beulich wrote: >>>>>> On 08.06.18 at 20:48, <andrew.coop...@citrix.com> wrote: >>>> @@ -3106,14 +3104,13 @@ static int vmx_msr_write_intercept(unsigned int >>>> msr, >> uint64_t msr_content) >>>> for ( ; (rc == 0) && lbr->count; lbr++ ) >>>> for ( i = 0; (rc == 0) && (i < lbr->count); i++ ) >>>> if ( (rc = vmx_add_guest_msr(v, lbr->base + i)) == 0 ) >>>> - { >>>> vmx_clear_msr_intercept(v, lbr->base + i, >> VMX_MSR_RW); >>>> - if ( lbr_tsx_fixup_needed ) >>>> - v->arch.hvm_vmx.lbr_fixup_enabled |= >> FIXUP_LBR_TSX; >>>> - if ( bdw_erratum_bdf14_fixup_needed ) >>>> - v->arch.hvm_vmx.lbr_fixup_enabled |= >>>> - FIXUP_BDW_ERRATUM_BDF14; >>>> - } >>>> + >>>> + v->arch.hvm_vmx.lbr_flags |= LBR_MSRS_INSERTED; >>>> + if ( lbr_tsx_fixup_needed ) >>>> + v->arch.hvm_vmx.lbr_flags |= LBR_FIXUP_TSX; >>>> + if ( bdw_erratum_bdf14_fixup_needed ) >>>> + v->arch.hvm_vmx.lbr_flags |= LBR_FIXUP_BDF14; >>> Note how the setting of the flags previously depended on >>> vmx_add_guest_msr() having returned success at least once. >> And? >> >> Unless this sequence returns fully successfully, we throw #MC into the >> guest without setting any kind of vMCE state. It might be the least bad >> option we have available, but its also not reasonable to expect the >> guest to survive. >> >> The two ways to fail are ENOMEM which E2BIG. The former is going to be >> causing other forms of chaos, and the latter isn't going to occur in >> practice because current codepaths in Xen use a maximum of ~40 or the >> 256 available slots. If in the unlikely case that we fail with ENOMEM >> on the first entry, all the fixup logic gets short circuited due to the >> missing memory allocation (so practically 0 extra overhead), and the >> guest will still malfunction. >> >> The error handling here is sufficiently poor that I'm not worried about >> changing one minor corner case. I'm actually debating whether it would >> be better to make the allocation at vmcs construction time, to avoid >> runtime out-of-memory issues. > With further improved MSR handling down the road, I assume we'll > have some entries in the list in almost all cases, so yes, I think that > would be desirable.
For performance reasons, we'll want to keep the size of the lists to an absolute minimum. On a closer inspection, the only uses we currently have for the load/save lists are this new EFER case (on Gen1 hardware), the Global Perf Ctl (for vPMU, and we really should be using the load/save support like EFER), and the LBR MSRs. Therefore, for on non-ancient hardware, a guest which doesn't touch MSR_DEBUGCTL is not going to need the memory allocation, so perhaps an up-front allocation isn't the wisest of options. I'll keep this in mind during the MSR work. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel