Re: [PATCH 0/5] Intel Code/Data Prioritization(CDP) feature enabling

[He Chen <he.chen@linux.intel.com>]

On Wed, Sep 02, 2015 at 01:08:33PM +0100, Andrew Cooper wrote:
> On 02/09/15 09:27, He Chen wrote:
> > Hi all,
> >
> > Code/Data Prioritization(CDP) is offered in Intel Broadwell and later server
> > platforms, which is an extension of CAT. CDP enables isolation and separate
> > prioritization of code and data fetches to the L3 cache in a software
> > configurable manner, which can enable workload prioritization and tuning of
> > cache capacity to the characteristics of the workload. CDP extends Cache
> > Allocation Technology (CAT) by providing separate code and data capacity bit
> > masks(CBM) per Class of Service (COS). CDP is used on VM basis in the Xen
> > implementation.
> >
> > More information about CDP, please refer to Intel SDM, Volumn 3, section 17.16
> > http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf
> >
> > This patch series enables CDP feature in Xen based on CAT code, including
> > extending CBM operation functions and introducing new commands to enable/disable
> > CDP dynamically. For all the changes, please see in each patch.
> >
> > This patchset has been tested on Intel Broadwell server platform.
> >
> > To make this patchset better, any comment or suggestion is welcomed, I would
> > really appreciate it.
> 
> I have taken a look at patches 1-3.  For the most part, it looks good.
> 
> The main point I have is on patch 2, as to whether it is sensible to
> permit enabling/disabling cdp at runtime.  I suggest that it is not
> sensible, and should be a command line parameter instead.
> 
> If this is agreed as ok going forwards, patches 3 through 5 should
> become rather more simple.
> 
> ~Andrew

Thanks for your patient review and valuable suggestions.

About permitting enabling/disabling CDP at runtime, I agree with you to
use command line parameter instead, it really makes code simple and
reliable.

For caution's sake, hardware support confingure CDP dynamically at any
point during normal system operation according to Intel SDM (see section
17.16.2), and that is why I wrote patch 2.

Anyway, since there is few cases to change CDP at runtime, I think it is
better to make this a boot-time parameter. I would resend v2 patch soon
and thanks again~