Re: [Xen-devel] [PATCH v4 1/2] SVM: support data breakpoint extension registers

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 04/22/2014 10:04 AM, Jan Beulich wrote:
> > > > On 22.04.14 at 16:04, <boris.ostrovsky@xxxxxxxxxx> 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.ostrovsky@xxxxxxxxxx>



-boris

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