Re: [Xen-devel] [PATCH v10 05/25] x86: refactor psr: L3 CAT: implement CPU init and free flow.

[Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>]

On 17-04-10 11:27:04, Yi Sun wrote:
> On 17-04-07 03:46:42, Jan Beulich wrote:
> > >>> On 07.04.17 at 11:08, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> > > On 17-04-07 02:48:40, Jan Beulich wrote:
> > >> >>> On 07.04.17 at 07:17, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> > >> > On 17-04-06 08:02:15, Jan Beulich wrote:
> > >> >> Okay, so not the context switch path then, But you must be
> > >> >> changing the MSRs _somewhere_, and the question is why this
> > >> >> somewhere isn't sufficient.
> > >> >> 
> > >> > Besides the restore behavior in init process, I restore the MSRs when 
> > >> > ref[cos]
> > >> > is reduced to 0. This behavior happens in two scenarios:
> > >> > 1. In a value setting process, restore MSR if the ref[old_cos] is 
> > >> > reduced to 0.
> > >> > 2. When a domain is destroyed, its ref[cos] is reduced too.
> > >> > 
> > >> > Reason to restore is below:
> > >> >   For features,  e.g. CDP, which cos_num is more than 1, we have to
> > >> >   restore the old_cos value back to default when ref[old_cos] is 0.
> > >> >   Otherwise, user will see wrong values when this COS ID is reused. 
> > >> > E.g.
> > >> >   user wants to set DATA to 0x3ff for a new domain. He hopes to see the
> > >> >   DATA is set to 0x3ff and CODE should be the default value, 0x7ff. But
> > >> >   if the COS ID picked for this action is the one that has been used by
> > >> >   other domain and the CODE has been set to 0x1ff. Then, user will see
> > >> >   DATA: 0x3ff, CODE: 0x1ff. So, we have to restore COS values for 
> > >> > features
> > >> >   using multiple COSs.
> > >> 
> > >> I still don't feel my question being answered. Without a COS
> > >> ever allocated on a socket, how can values in MSRs other than
> > >> the first (index 0) one be used by anyone? I ask because if they
> > >> can't be used, their values don't matter (all you need to make
> > >> sure is to write them regardless of their currently cached value).
> > > 
> > > The COS ID using is managed by domain (d->arch.psr_cos_ids[socket]). Even 
> > > if a
> > > socket is offline, the COS ID saved in domain is still valid (domain is 
> > > alive).
> > > When this socket is online again, the domain may be scheduled onto it to 
> > > run.
> > > Then, the COS ID (e.g 2) will be used to get/set value for this domain. 
> > > If we
> > > don't restore MSRs on the socket, we may get an unknown value. This the 
> > > reason
> > > we have to restore MSRs in 'cat_init_feature' which is called when socket 
> > > is
> > > online.
> > 
> > No, it's not. At least that's not the only way to solve the problem:
> > As said, you could equally well make sure you always write the MSR
> > the first time a vCPU using a particular COS is being scheduled onto
> > the newly onlined pCPU. In fact most of the time it is better if
> > generic code can also deal with special cases than to introduce
> > special purpose code. Hence my insisting on you properly explaining
> > why generic code can't deal with the post-online situation here.
> > 
> Then, I propose another simple solution to handle this case. When writing
> MSRs, check if all values in val_array are same as old values (kept in
> 'cos_reg_val[]'). If a value is different, the MSR will be written and the
> corresponding member in 'cos_reg_val[]' will be updated. Still use above
> sample, user wants to set DATA to 0x3ff for a new domain. After value array
> assembling, the val_array will be val[DATA]=0x3ff, val[CODE]=0x7ff. The
> cos_reg_val[DATA]=0x7ff and cos_reg_val[DATA]=0x1ff. So, both of them will
> be updated.
> 
I want to explain more to make it clearer. The original codes are to solve the
issues when a COS ID is free and reused by other domain. With the newly
proposed method above, we can solve this issue too. E.g:
1. COS ID 1 is used by domain 1. Its CODE=0x1ff, DATA=0x7ff. They are kept in
   cos_reg_val[1 - CODE] and cos_reg_val[1 - DATA].
2. For some reason, COS ID 1 is free.
3. We want to set DATA of a new domain (8) to 0x3ff. Its old_cos is 0 so that
   val_array assembled is CODE=0x7ff, DATA=0x3ff. Then COS ID 1 is picked.
4. In write flow, iterate the feature's type array according to cos_num to
   set both CODE and DATA if val_array member is different with cos_reg_val
   member.

Below are codes to explain it clearly:

static void l3_cdp_write_msr(unsigned int cos, uint32_t val, ...)
{
    /* Data. For above case, get_cdp_data(..., cos) returns 0x7ff */
    if ( type == PSR_CBM_TYPE_L3_DATA && get_cdp_data(feat, cos) != val )
    {
        get_cdp_data(feat, cos) = val;
        wrmsrl(MSR_IA32_PSR_L3_MASK_DATA(cos), val);
    }
    /* Code. For above case, get_cdp_code(..., cos) returns 0x1ff */
    if ( type == PSR_CBM_TYPE_L3_CODE && get_cdp_code(feat, cos) != val )
    {
        get_cdp_code(feat, cos) = val;
        wrmsrl(MSR_IA32_PSR_L3_MASK_CODE(cos), val);
    }
}

struct cos_write_info
{
    unsigned int cos;
    struct feat_node *feature;
    /* All COS MSRs values of the feature. */
    uint32_t *val;
    unsigned int array_len;
    enum psr_feat_type feat_type;
};

static void do_write_psr_msr(void *data)
{
...
    /* Write all COS MSRs of the feature. */
    for ( i = 0; i < props[info->feat_type]->cos_num; i++ )
        props[info->feat_type]->write_msr(cos, info->val[i],
                                          props[info->feat_type]->type[i], 
feat);
}

static int write_psr_msr(unsigned int socket, unsigned int cos, ...)
{
...
    /* Skip to the feature's value head. */
    for ( i = 0; i < feat_type; i++ )
    {
        if ( !info->features[i] )
            continue;

        if ( array_len <= props[i].cos_num )
            return -ENOSPC;

        array_len -= props[i].cos_num;

        val += props[i].cos_num;
    }

    data.val = val;
    data.array_len = array_len;
...
}

int psr_set_val(struct domain *d, unsigned int socket, ...)
{
...
    /* Gather and insert value array. Array should be val[DATA]=0x3ff, 
val[CODE]=0x7ff. */
    gather_val_array(val_array,...);
    insert_val_to_array(val_array,...);
 
    /* Input val_array which has all COS MSRs values of all features. */
    ret = write_psr_msr(socket, cos, val_array, array_len, feat_type);
...
}

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

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