Re: [Xen-devel] [PATCH] libxc: Add xc_domain_hvm_get_mtrr_type() call

[Razvan Cojocaru <rzvncj@xxxxxxxxx>]

> There is a section in the Intel SDM (chapter 10.4 in my copy) which
> explains the meanings etc of the registers, which are what is exposed in
> hvm_hw_mtrr I think.
Thanks, I'll look that up.

> xen/arch/x86/hvm/mtrr.c:mtrr_def_type_msr_set() seems to be where it is
> written, and it is called with hw_mtrr.msr_mtrr_def_type so it looks
> like it can be derived from that value in hvm_hw_mtrr.
Indeed it is, but this happens there:

uint8_t def_type = msr_content & 0xff;
uint8_t enabled = (msr_content >> 10) & 0x3;

So what ends up being put in def_type is only one byte of msr_content, 
whereas enabled is some bits from another byte of msr_content. To make 
matters worse, hw_mtrr.msr_mtrr_def_type is an uint64_t, so would that 
mean that hw_mtrr.msr_mtrr_def_type is actually the whole of msr_content?
> > I'm also having a hard time figuring out how to map m->overlapped on the
> > hvm_hw_mtrr members.
>      m->overlapped = is_var_mtrr_overlapped(m);
>
> Looks like that function contains the necessary logic.
You're right, but what happens there is that that function depends on 
the get_mtrr_range() function, which in turn depends on the size_or_mask 
global variable, which is initialized in hvm_mtrr_pat_init(), which then 
depends on a global table, and so on. Putting that into libxc is pretty 
much putting the whole mtrr.c file there.
Is this amount of copy/paste code a good thing, and wouldn't it be less 
tedious and bug-prone to have that code in a single place, and just add 
overlapped and enabled to hw_mtrr before sending it out into userspace?
Thanks,
Razvan Cojocaru

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel