Re: [Xen-devel] [PATCH v13 13/23] x86: refactor psr: CDP: implement CPU init flow.

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 07/13/17 5:04 AM >>>
>On 17-07-12 13:52:35, Jan Beulich wrote:
>> >>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 07/06/17 4:07 AM >>>
>> >This patch implements the CPU init flow for CDP. The flow is almost
>> >same as L3 CAT.
>> >
>> >Note: CDP does NOT work until you apply the later patches of CDP.
>> >"x86: refactor psr: CDP: implement get hw info flow."
>> >"x86: refactor psr: CDP: implement set value callback function."
>> 
>> This is _not_ what I did ask for in what I assume ...
>> 
>> >v13:
>>     >- add commit message.
>> 
>> ... this refers to: The problem isn't that it won't work, but that it'll 
>> crash the
>> hypervisor. So what I had expected you to add is a note _after_ the first
>> --- separator that this patch should not be applied without the other two
>> ones. Or alternatively for this one to add stubs which the subsequent
>> patches would then fill.
>> 
>Oh, ok. I will move the comments under the first '---'. Furthermore, I'd like
>to add a 'stub_write_msr()' here to avoid crash. Will replace the stub function
>to real function when implementing the write flow.

Just to avoid any misunderstanding: I hope you don't mean to indeed
introduce and then replace a function by that name. Just like you've done
in earlier patches, introduce the function with it final name right away, and
just fill its body in the later patch.

>> >+        if ( (regs.c & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
>> >+        {
>> >+            feat = feat_l3_cdp;
>> >+            feat_l3_cdp = NULL;
>> >+            if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CDP) )
>> >+                feat_props[FEAT_TYPE_L3_CDP] = &l3_cdp_props;
>> >+        }
>> >+        else
>> >+        {
>> >+            feat = feat_l3_cat;
>> >+            feat_l3_cat = NULL;
>> >+            if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CAT) )
>> >+                feat_props[FEAT_TYPE_L3_CAT] = &l3_cat_props;
>> >+        }
>> 
>> ... wouldn't it be a good idea to then try to setup plain L3 CAT here?
>> 
>DYM if CDP init fails, enter CAT init flow to make CAT work at least?

Yes.

Jan


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