From mboxrd@z Thu Jan 1 00:00:00 1970 From: Razvan Cojocaru Subject: Re: [PATCH] libxc: Add xc_domain_hvm_get_mtrr_type() call Date: Wed, 19 Dec 2012 13:49:37 +0200 Message-ID: <50D1A9D1.2020106@gmail.com> References: <1355850255.14620.277.camel@zakaz.uk.xensource.com> <50D0A6B1.30702@gmail.com> <1355912063.14620.286.camel@zakaz.uk.xensource.com> <50D19A2B.2050006@gmail.com> <1355916539.14620.332.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1355916539.14620.332.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: "Tim (Xen.org)" , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org > 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