On 27/06/18 10:58, Jan Beulich wrote: >>>> On 27.06.18 at 11:50, <andrew.coop...@citrix.com> wrote: >> On 27/06/18 10:12, Jan Beulich wrote: >>>>>> On 27.06.18 at 10:43, <andrew.coop...@citrix.com> wrote: >>>> The main purpose of this patch is to only ever insert the LBR MSRs into the >>>> guest load/save list once, as a future patch wants to change the behaviour >> of >>>> vmx_add_guest_msr(). >>>> >>>> The repeated processing of lbr_info and the guests MSR load/save list is >>>> redundant, and a guest using LBR itself will have to re-enable >>>> MSR_DEBUGCTL.LBR in its #DB handler, meaning that Xen will repeat this >>>> redundant processing every time the guest gets a debug exception. >>>> >>>> Rename lbr_fixup_enabled to lbr_flags to be a little more generic, and use >>>> one >>>> bit to indicate that the MSRs have been inserted into the load/save list. >>>> Shorten the existing FIXUP* identifiers to reduce code volume. >>>> >>>> Furthermore, handing the guest #MC on an error isn't a legitimate action. >>>> Two >>>> of the three failure cases are definitely hypervisor bugs, and the third >>>> is a >>>> boundary case which shouldn't occur in practice. The guest also won't >>>> execute >>>> correctly, so handle errors by cleanly crashing the guest. >>>> >>>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> >>> Reviewed-by: Jan Beulich <jbeul...@suse.com> >>> with one minor remark: >>> >>>> @@ -3097,30 +3098,64 @@ static int vmx_msr_write_intercept(unsigned int >>>> msr, uint64_t msr_content) >>>> if ( vpmu_do_wrmsr(msr, msr_content, supported) ) >>>> break; >>>> } >>>> - if ( msr_content & IA32_DEBUGCTLMSR_LBR ) >>>> + >>>> + /* >>>> + * When a guest first enables LBR, arrange to save and restore >>>> the LBR >>>> + * MSRs and allow the guest direct access. >>>> + * >>>> + * MSR_DEBUGCTL and LBR has existed almost long as MSRs have >>>> existed, >>> ... as long as ... >> Not quite. MSRs have existed since the P5, whereas LBR was introduced >> in the P6. The MSR wasn't architectural at the time, hence no CPUID >> bit, but the newer architectural declaration is compatible with the P6 >> (as far as LBR is concerned). > Perhaps a misunderstanding resulting from an ambiguity in my reply? I > should have said "... almost as long as ..."; I didn't mean for the "almost" > to be dropped.
Oops yes - you're right. I'll fix that. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
