Re: [PATCH v8 08/24] x86: refactor psr: set value: implement framework.

["Jan Beulich" <JBeulich@suse.com>]

>>> On 10.03.17 at 03:54, <yi.y.sun@linux.intel.com> wrote:
> On 17-03-08 09:07:10, Jan Beulich wrote:
>> >>> On 15.02.17 at 09:49, <yi.y.sun@linux.intel.com> wrote:
>> > +static int write_psr_msr(unsigned int socket, unsigned int cos,
>> > +                         const uint64_t *val)
>> > +{
>> > +    return -ENOENT;
>> > +}
>> 
>> Is this function intended you write just one MSR, or potentially many?
>> In the latter case the name would perhaps better be "write_psr_msrs()".
>> 
> For one feature, it does set one MSR.

In which case - why is the value being passed by pointer?

>> > +    ref[old_cos]--;
>> > +    spin_unlock(&info->ref_lock);
>> > +
>> > +    /*
>> > +     * Step 6:
>> > +     * Save the COS ID into current domain's psr_cos_ids[] so that we can know
>> > +     * which COS the domain is using on the socket. One domain can only use
>> > +     * one COS ID at same time on each socket.
>> > +     */
>> > +    d->arch.psr_cos_ids[socket] = cos;
>> 
>> So the domain has not been paused, i.e. some of its vCPU-s may
>> be running on other pCPU-s (including ones on the socket in
>> question). How come it is safe to update this value here?
>> 
> This is a domctl operation. It is protected by domctl_lock which is locked in
> do_domctl().

But that lock doesn't keep the subject domain's vCPU-s from
running on other pCPU-s at the same time.

>> >  /* Called with domain lock held, no extra lock needed for 'psr_cos_ids' */
>> >  static void psr_free_cos(struct domain *d)
>> >  {
>> > -    if( !d->arch.psr_cos_ids )
>> > +    unsigned int socket, cos;
>> > +
>> > +    if ( !d->arch.psr_cos_ids )
>> >          return;
>> 
>> As in an earlier patch I've asked for this check to be removed, I
>> think you will need to add a check on socket_info to be non-
>> NULL somewhere in this function.
>> 
> Ok, will do it in the loop.

Please don't - loop invariants should be put outside of loops.

Jan


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