From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v4 1/2] SVM: support data breakpoint extension registers Date: Tue, 22 Apr 2014 10:17:34 -0400 Message-ID: <535679FE.1020802@oracle.com> References: <534EAE6402000078000098FB@nat28.tlf.novell.com> <534EB0EE020000780000996A@nat28.tlf.novell.com> <535676F3.9000408@oracle.com> <53569312020000780000AE01@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WcbSh-0006CV-K7 for xen-devel@lists.xenproject.org; Tue, 22 Apr 2014 14:14:15 +0000 In-Reply-To: <53569312020000780000AE01@nat28.tlf.novell.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: Jan Beulich Cc: Keir Fraser , Tim Deegan , Ian Jackson , Ian Campbell , Aravind Gopalakrishnan , suravee.suthikulpanit@amd.com, xen-devel List-Id: xen-devel@lists.xenproject.org On 04/22/2014 10:04 AM, Jan Beulich wrote: >>>> On 22.04.14 at 16:04, wrote: >> On 04/16/2014 10:33 AM, Jan Beulich wrote: >>> @@ -1526,6 +1628,21 @@ static int svm_msr_read_intercept(unsign >>> vpmu_do_rdmsr(msr, msr_content); >>> break; >>> >>> + case MSR_AMD64_DR0_ADDRESS_MASK: >>> + hvm_cpuid(0x80000001, NULL, NULL, &ecx, NULL); >>> + if ( !test_bit(X86_FEATURE_DBEXT & 31, &ecx) ) >>> + goto gpf; >>> + *msr_content = v->arch.hvm_svm.dr_mask[0]; >>> + break; >>> + >>> + case MSR_AMD64_DR1_ADDRESS_MASK ... MSR_AMD64_DR3_ADDRESS_MASK: >>> + hvm_cpuid(0x80000001, NULL, NULL, &ecx, NULL); >>> + if ( !test_bit(X86_FEATURE_DBEXT & 31, &ecx) ) >>> + goto gpf; >>> + *msr_content = >>> + v->arch.hvm_svm.dr_mask[msr - MSR_AMD64_DR1_ADDRESS_MASK + 1]; >>> + break; >>> + >> Can you merge these two cases (here and in write_intercept)? They are >> doing logically the same thing, the only difference is index calculation >> (and you can have a macro or something similar for that). > Do you really think this would be better - since the MSR numbers > aren't contiguous (and since one might want to consider adding > support for Fam15 supporting just the DR0 extension), I think they > would better be kept separate. These MSR are tied to CPUID bit so if they are going to change then it will require a new bit anyway. But it's a nit really so Reviewed-by: Boris Ostrovsky -boris