Hello, I need access to some MSR values that are not currently being saved in struct hvm_hw_cpu. Among them are MSR_IA32_MC0_CTL, MSR_IA32_MISC_ENABLE and MSR_IA32_ENERGY_PERF_BIAS. The way I''m approaching this that I''ll patch xen/arch/x86/hvm/vmx/vmx.c and xen/arch/x86/hvm/svm/svm.c, and add this in vmx_save_cpu_state() and svm_save_cpu_state(), respectively: hvm_msr_read_intercept(MSR_IA32_MC0_CTL, &data->msr_mc0_ctl); and so on, for the other registers (after adding the msr_mc0_ctl member to struct hvm_hw_cpu, of course). I would also have to do the reverse operation (using hvm_msr_write_intercept()) in vmx_load_cpu_state(). My questions: 1. Does it seem architecturally sound to perform the described modifications? Can I use hvm_msr_xxx_intercept() for both the VMX and the SVM code? 2. It seems repetitive to have duplicated code in both vmx_save_cpu_state() and svm_save_cpu_state(), does it make more sense to have it like that anyway (in case, for example, the SVM way to retrieve that register could change in the future)? 3. Do I need to do additional things so that I won''t break anything else? 4. Is there a better way to achieve what I''m after? Thanks, Razvan Cojocaru
On 21/12/12 12:45, Razvan Cojocaru wrote:> Hello, > > I need access to some MSR values that are not currently being saved in > struct hvm_hw_cpu. Among them are MSR_IA32_MC0_CTL, MSR_IA32_MISC_ENABLE > and MSR_IA32_ENERGY_PERF_BIAS. > > The way I''m approaching this that I''ll patch xen/arch/x86/hvm/vmx/vmx.c > and xen/arch/x86/hvm/svm/svm.c, and add this in vmx_save_cpu_state() and > svm_save_cpu_state(), respectively: > > hvm_msr_read_intercept(MSR_IA32_MC0_CTL, &data->msr_mc0_ctl); > > and so on, for the other registers (after adding the msr_mc0_ctl member > to struct hvm_hw_cpu, of course). I would also have to do the reverse > operation (using hvm_msr_write_intercept()) in vmx_load_cpu_state(). > > My questions: > > 1. Does it seem architecturally sound to perform the described > modifications? Can I use hvm_msr_xxx_intercept() for both the VMX and > the SVM code? > > 2. It seems repetitive to have duplicated code in both > vmx_save_cpu_state() and svm_save_cpu_state(), does it make more sense > to have it like that anyway (in case, for example, the SVM way to > retrieve that register could change in the future)? > > 3. Do I need to do additional things so that I won''t break anything else? > > 4. Is there a better way to achieve what I''m after? > > Thanks, > Razvan Cojocaru >I''m not sure I understand what you are trying to achieve (nor am I convinced I know how to help you, but if I don''t understand the question suffiiciently, I certainly can''t advice you on what you can/should do or can''t/shouldn''t do), but what MSR''s are we talking about - the guest MSR''s or the host MSR''s? In general, Xen is responsible for "real" Machine Check interrupts (correctible ones), and will then forward this information to the guest if it has enabled MCE in it''s code (via the MCE_SOFTIRQ). Normally, reading MSR''s in usermode is not allowed on bare-metal, so not sure why you expect this to work in the guest (or Dom0) on top of Xen. But maybe you don''t actually mean userspace as opposed to "kernel mode"? -- Mats
Hello, thanks for the reply!> I''m not sure I understand what you are trying to achieve (nor am I > convinced I know how to help you, but if I don''t understand the question > suffiiciently, I certainly can''t advice you on what you can/should do or > can''t/shouldn''t do), but what MSR''s are we talking about - the guest > MSR''s or the host MSR''s?Sorry if I''ve not been clear. I want to access the MSRs of a Xen HVM guest, from a userspace application running in dom0, with the help of libxc. Libxc already allows me to inspect the values of several registers, including a handful of MSRs, if I call: xc_domain_hvm_getcontext_partial(xch, domain_id, HVM_SAVE_CODE(CPU), instance, &hw_ctxt, sizeof hw_ctxt); and then examine, for example, hw_ctxt.msr_lstar. What I''d like is to be able to check hw_ctxt.msr_mc0_ctl, for example, after the xc_domain_hvm_getcontext_partial().> Normally, reading MSR''s in usermode is not allowed on bare-metal, so not > sure why you expect this to work in the guest (or Dom0) on top of Xen. > But maybe you don''t actually mean userspace as opposed to "kernel mode"?I mean accessing a domU''s MSRs from dom0 userspace. Thanks, Razvan Cojocaru
>>> On 21.12.12 at 13:45, Razvan Cojocaru <rzvncj@gmail.com> wrote: > I need access to some MSR values that are not currently being saved in > struct hvm_hw_cpu. Among them are MSR_IA32_MC0_CTL, MSR_IA32_MISC_ENABLE > and MSR_IA32_ENERGY_PERF_BIAS.I can''t see why, and this is quite likely related to the reason for them not being accessible in the first place.> The way I''m approaching this that I''ll patch xen/arch/x86/hvm/vmx/vmx.c > and xen/arch/x86/hvm/svm/svm.c, and add this in vmx_save_cpu_state() and > svm_save_cpu_state(), respectively: > > hvm_msr_read_intercept(MSR_IA32_MC0_CTL, &data->msr_mc0_ctl); > > and so on, for the other registers (after adding the msr_mc0_ctl member > to struct hvm_hw_cpu, of course). I would also have to do the reverse > operation (using hvm_msr_write_intercept()) in vmx_load_cpu_state(). > > My questions: > > 1. Does it seem architecturally sound to perform the described > modifications?Not to me, no. These records are use for save/restore/migrate, and so far there hasn''t been a need to include here the MSRs you mention.> Can I use hvm_msr_xxx_intercept() for both the VMX and > the SVM code?For MSR_IA32_MC0_CTL, why not? These should be fine for anything that is architectural to x86. The other two MSRs are Intel specific iirc, and hence wouldn''t be validly dealt with in vendor independent code.> 2. It seems repetitive to have duplicated code in both > vmx_save_cpu_state() and svm_save_cpu_state(), does it make more sense > to have it like that anyway (in case, for example, the SVM way to > retrieve that register could change in the future)?Is what you want was explained well enough to become reasonable, and if the amount of code duplication would become significant, then consolidating what is common between both would certainly make sense. Jan
>> I need access to some MSR values that are not currently being saved in >> struct hvm_hw_cpu. Among them are MSR_IA32_MC0_CTL, MSR_IA32_MISC_ENABLE >> and MSR_IA32_ENERGY_PERF_BIAS. > > I can''t see why, and this is quite likely related to the reason for > them not being accessible in the first place.Because they offer heuristic information about the type of OS running in the domU domain, and there are classes of applications interested in that.> Not to me, no. These records are use for save/restore/migrate, > and so far there hasn''t been a need to include here the MSRs you > mention.Interesting. I''ve assumed that those would be saved (and perhaps ignored on restore if they became irrelevant) in a migration scenario.> For MSR_IA32_MC0_CTL, why not? These should be fine for > anything that is architectural to x86. > > The other two MSRs are Intel specific iirc, and hence wouldn''t > be validly dealt with in vendor independent code.I agree, however, as I''ve said before, _some_ applications _are_ interested in vendor-specific information for heuristic purposes. Whether this would make a interesting patch for Xen or not, I would appreciate any advice on how best to implement said functionality (if only for MSR_IA32_MC0_CTL). Thanks, Razvan Cojocaru
On 21/12/12 14:08, Razvan Cojocaru wrote:>>> I need access to some MSR values that are not currently being saved in >>> struct hvm_hw_cpu. Among them are MSR_IA32_MC0_CTL, MSR_IA32_MISC_ENABLE >>> and MSR_IA32_ENERGY_PERF_BIAS. >> I can''t see why, and this is quite likely related to the reason for >> them not being accessible in the first place. > Because they offer heuristic information about the type of OS running in > the domU domain, and there are classes of applications interested in that.The handling of MC0_CTL for guest is this: /xen/arch/x86/cpu/mcheck/vmce.c: line 14: case MSR_IA32_MC0_CTL: /* * if guest crazy clear any bit of MCi_CTL, * treat it as not implement and ignore write change it. */ break; That is, whatever the guest writes to MC0 is completely ignored. So, if you''re trying to figure out what guest is running by looking at MC0_CTL, then I guess tough - Xen doesn''t store that at all. Like I said before, MC Interrupt is handled in Xen, and then forwarded to the guest as an IRQ. (Obviously, MCE is generally fatal to the system, so guest isn''t at all involved in those). If you are to save some more Machine Check related information, it probably should go into: static int vmce_save_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h) in the same vmce.c file as above (although I''m not 100% sure this is part of "partial" saves). Currently, this saves MCI_CTL2 register values only.>> Not to me, no. These records are use for save/restore/migrate, >> and so far there hasn''t been a need to include here the MSRs you >> mention. > Interesting. I''ve assumed that those would be saved (and perhaps ignored > on restore if they became irrelevant) in a migration scenario.I presume guests would just be happy with the defaults Xen provides. Otherwise, we''d have bug reports... -- Mats>> For MSR_IA32_MC0_CTL, why not? These should be fine for >> anything that is architectural to x86. >> >> The other two MSRs are Intel specific iirc, and hence wouldn''t >> be validly dealt with in vendor independent code. > I agree, however, as I''ve said before, _some_ applications _are_ > interested in vendor-specific information for heuristic purposes. > > Whether this would make a interesting patch for Xen or not, I would > appreciate any advice on how best to implement said functionality (if > only for MSR_IA32_MC0_CTL). > > Thanks, > Razvan Cojocaru > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > >
>>> On 21.12.12 at 15:08, Razvan Cojocaru <rzvncj@gmail.com> wrote: > Whether this would make a interesting patch for Xen or not, I would > appreciate any advice on how best to implement said functionality (if > only for MSR_IA32_MC0_CTL).I''d make this a new hvm_op. Jan