All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen
@ 2017-10-08  7:23 Yi Sun
  2017-10-08  7:23 ` [PATCH v6 01/16] docs: create Memory Bandwidth Allocation (MBA) feature document Yi Sun
                   ` (15 more replies)
  0 siblings, 16 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Yi Sun, Konrad Rzeszutek Wilk, Andrew Cooper, Ian Jackson,
	Julien Grall, Jan Beulich, Chao Peng, Wei Liu, Daniel De Graaf,
	Roger Pau Monné

Hi, all,

We plan to bring a new PSR (Platform Shared Resource) feature called
Intel Memory Bandwidth Allocation (MBA) to Xen.

Besides the MBA enabling, we change some interfaces to make them more
general but not only for CAT.

Any comments are welcome!

You can find this series at:
https://github.com/yisun-git/xen_mba mba_v6

This version base on below pre-fix patch:
"x86: psr: support co-exist features' values setting"
https://lists.xen.org/archives/html/xen-devel/2017-10/msg00866.html

CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>
CC: Julien Grall <julien.grall@arm.com>

---
Acked and Reviewed list before V6:

a - Acked-by
r - Reviewed-by

  r  patch 1  - docs: create Memory Bandwidth Allocation (MBA) feature document
  r  patch 2  - Rename PSR sysctl/domctl interfaces and xsm policy to make them be general
  ar patch 3  - x86: rename 'cbm_type' to 'psr_type' to make it general
  r  patch 4  - x86: a few optimizations to psr codes
  r  patch 5  - x86: implement data structure and CPU init flow for MBA
  r  patch 6  - x86: implement get hw info flow for MBA
  ar patch 7  - x86: implement get value interface for MBA
  ar patch 9  - tools: create general interfaces to support psr allocation features
  ar patch 10 - tools: implement the new libxc get hw info interface
  ar patch 11 - tools: implement the new libxl get hw info interface
  ar patch 12 - tools: implement the new xl get hw info interface
  ar patch 13 - tools: rename 'xc_psr_cat_type' to 'xc_psr_type'
  ar patch 14 - tools: implement new generic get value interface and MBA get value command
  ar patch 15 - tools: implement new generic set value interface and MBA set value command
  ar patch 16 - docs: add MBA description in docs

---
V6 change list:

Patch 1:
    - fix some words.
      (suggested by Roger Pau Monné)
Patch 2:
    - move macro definition into the function and undefine it after use.
      (suggested by Roger Pau Monné)
    - do not bump sysctl version because it has been bumped for 4.10.
      (suggested by Roger Pau Monné)
Patch 4:
    - restore 'write_msr()' type to 'void'.
      (suggested by Jan Beulich and Roger Pau Monné)
    - change 'ebx' in 'psr_cpu_init' to 'feat_mask'.
      (suggested by Jan Beulich and Roger Pau Monné)
Patch 5:
    - restore type of 'mba_write_msr' to 'void'.
Patch 8:
    - split co-exist features' values setting flow to a new patch.
      (suggested by Jan Beulich)
    - restore codes related to 'mba_check_thrtl' and 'check_value'.
      (suggested by Jan Beulich)
Patch 10:
    - remove unnecessary spaces in brackets.
      (suggested by Wei Liu)
    - use assert to check input lvl.
      (suggested by Roger Pau Monné)
Patch 14:
    - fix one coding style issue.
      (suggested by Roger Pau Monné)
Patch 15:
    - move xc_type definition and value get out of the loop.
      (suggested by Roger Pau Monné)

Yi Sun (16):
  docs: create Memory Bandwidth Allocation (MBA) feature document
  Rename PSR sysctl/domctl interfaces and xsm policy to make them be
    general
  x86: rename 'cbm_type' to 'psr_type' to make it general
  x86: a few optimizations to psr codes
  x86: implement data structure and CPU init flow for MBA
  x86: implement get hw info flow for MBA
  x86: implement get value interface for MBA
  x86: implement set value flow for MBA
  tools: create general interfaces to support psr allocation features
  tools: implement the new libxc get hw info interface
  tools: implement the new libxl get hw info interface
  tools: implement the new xl get hw info interface
  tools: rename 'xc_psr_cat_type' to 'xc_psr_type'
  tools: implement new generic get value interface and MBA get value
    command
  tools: implement new generic set value interface and MBA set value
    command
  docs: add MBA description in docs

 docs/features/intel_psr_mba.pandoc  | 297 ++++++++++++++++++++++++++++++++++
 docs/man/xl.pod.1.in                |  33 ++++
 docs/misc/xl-psr.markdown           |  62 ++++++++
 tools/flask/policy/modules/dom0.te  |   4 +-
 tools/libxc/include/xenctrl.h       |  44 +++--
 tools/libxc/xc_psr.c                | 109 ++++++++-----
 tools/libxl/libxl.h                 |  37 +++++
 tools/libxl/libxl_psr.c             | 223 ++++++++++++++++++++------
 tools/libxl/libxl_types.idl         |  22 +++
 tools/xl/xl.h                       |   2 +
 tools/xl/xl_cmdtable.c              |  12 ++
 tools/xl/xl_psr.c                   | 279 +++++++++++++++++++++++++-------
 xen/arch/x86/domctl.c               |  87 +++++-----
 xen/arch/x86/psr.c                  | 310 +++++++++++++++++++++++++++---------
 xen/arch/x86/sysctl.c               |  53 ++++--
 xen/include/asm-x86/msr-index.h     |   1 +
 xen/include/asm-x86/psr.h           |  22 +--
 xen/include/public/domctl.h         |  26 +--
 xen/include/public/sysctl.h         |  20 ++-
 xen/xsm/flask/hooks.c               |   8 +-
 xen/xsm/flask/policy/access_vectors |   8 +-
 21 files changed, 1331 insertions(+), 328 deletions(-)
 create mode 100644 docs/features/intel_psr_mba.pandoc

-- 
1.9.1


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v6 01/16] docs: create Memory Bandwidth Allocation (MBA) feature document
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-08  7:23 ` [PATCH v6 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general Yi Sun
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Yi Sun, Konrad Rzeszutek Wilk, Andrew Cooper, Ian Jackson,
	Julien Grall, Jan Beulich, Chao Peng, Wei Liu, Daniel De Graaf,
	Roger Pau Monné

This patch creates MBA feature document in doc/features/. It describes
key points to implement MBA which is described in details in Intel SDM
"Introduction to Memory Bandwidth Allocation".

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>
CC: Julien Grall <julien.grall@arm.com>

v6:
    - fix some words.
      (suggested by Roger Pau Monné)
v5:
    - correct some words.
      (suggested by Roger Pau Monné)
    - change 'xl psr-mba-set 1 0xa' to 'xl psr-mba-set 1 10'.
      (suggested by Roger Pau Monné)
v4:
    - add 'domain-name' as parameter of 'psr-mba-show/psr-mba-set'.
      (suggested by Roger Pau Monné)
    - fix some wordings.
      (suggested by Roger Pau Monné)
    - explain how user can know the MBA_MAX.
      (suggested by Roger Pau Monné)
    - move the description of 'Linear mode/Non-linear mode' into section
      of 'psr-mba-show'.
      (suggested by Roger Pau Monné)
    - change 'per-thread' to 'per-hyper-thread' to make it clearer.
      (suggested by Roger Pau Monné)
    - upgrade revision number.
v3:
    - remove 'closed-loop' related description.
      (suggested by Roger Pau Monné)
    - explain 'linear' and 'non-linear' before mentioning them.
      (suggested by Roger Pau Monné)
    - adjust desription of 'psr-mba-set'.
      (suggested by Roger Pau Monné)
    - explain 'MBA_MAX'.
      (suggested by Roger Pau Monné)
    - remove 'n<64'.
      (suggested by Roger Pau Monné)
    - fix some wordings.
      (suggested by Roger Pau Monné)
    - add context in 'Testing' part to make things more clear.
      (suggested by Roger Pau Monné)
v2:
    - declare 'HW' in Terminology.
      (suggested by Chao Peng)
    - replace 'COS ID of VCPU' to 'COS ID of domain'.
      (suggested by Chao Peng)
    - replace 'COS register' to 'Thrtl MSR'.
      (suggested by Chao Peng)
    - add description for 'psr-mba-show' to state that the decimal value is
      shown for linear mode but hexadecimal value is shown for non-linear mode.
      (suggested by Chao Peng)
    - remove content in 'Areas for improvement'.
      (suggested by Chao Peng)
    - use '<>' to specify mandatory argument to a command.
      (suggested by Wei Liu)
v1:
    - remove a special character to avoid the error when building pandoc.
---
 docs/features/intel_psr_mba.pandoc | 297 +++++++++++++++++++++++++++++++++++++
 1 file changed, 297 insertions(+)
 create mode 100644 docs/features/intel_psr_mba.pandoc

diff --git a/docs/features/intel_psr_mba.pandoc b/docs/features/intel_psr_mba.pandoc
new file mode 100644
index 0000000..86df661
--- /dev/null
+++ b/docs/features/intel_psr_mba.pandoc
@@ -0,0 +1,297 @@
+% Intel Memory Bandwidth Allocation (MBA) Feature
+% Revision 1.8
+
+\clearpage
+
+# Basics
+
+---------------- ----------------------------------------------------
+         Status: **Tech Preview**
+
+Architecture(s): Intel x86
+
+   Component(s): Hypervisor, toolstack
+
+       Hardware: MBA is supported on Skylake Server and beyond
+---------------- ----------------------------------------------------
+
+# Terminology
+
+* CAT         Cache Allocation Technology
+* CBM         Capacity BitMasks
+* CDP         Code and Data Prioritization
+* COS/CLOS    Class of Service
+* HW          Hardware
+* MBA         Memory Bandwidth Allocation
+* MSRs        Machine Specific Registers
+* PSR         Intel Platform Shared Resource
+* THRTL       Throttle value or delay value
+
+# Overview
+
+The Memory Bandwidth Allocation (MBA) feature provides indirect and approximate
+control over memory bandwidth available per-core. This feature provides OS/
+hypervisor the ability to slow misbehaving apps/domains by using a credit-based
+throttling mechanism.
+
+# User details
+
+* Feature Enabling:
+
+  Add "psr=mba" to boot line parameter to enable MBA feature.
+
+* xl interfaces:
+
+  1. `psr-mba-show [domain-id|domain-name]`:
+
+     Show memory bandwidth throttling for domain. Under different modes, it
+     shows different type of data.
+
+     There are two modes:
+     Linear mode: the input precision is defined as 100-(MBA_MAX). For instance,
+     if the MBA_MAX value is 90, the input precision is 10%. Values not an even
+     multiple of the precision (e.g., 12%) will be rounded down (e.g., to 10%
+     delay applied) by HW automatically. The response of throttling value is
+     linear.
+
+     Non-linear mode: input delay values are powers-of-two from zero to the
+     MBA_MAX value from CPUID. In this case any values not a power of two will
+     be rounded down the next nearest power of two by HW automatically. The
+     response of throttling value is non-linear.
+
+     For linear mode, it shows the decimal value. For non-linear mode, it shows
+     hexadecimal value.
+
+  2. `psr-mba-set [OPTIONS] <domain-id|domain-name> <throttling>`:
+
+     Set memory bandwidth throttling for domain.
+
+     Options:
+     '-s': Specify the socket to process, otherwise all sockets are processed.
+
+     Throttling value set in register implies the approximate amount of delaying
+     the traffic between core and memory. Higher throttling value result in
+     lower bandwidth. The max throttling value (MBA_MAX) supported can be
+     obtained through CPUID inside hypervisor. Users can fetch the MBA_MAX value
+     using the `psr-hwinfo` xl command.
+
+# Technical details
+
+MBA is a member of Intel PSR features, it shares the base PSR infrastructure
+in Xen.
+
+## Hardware perspective
+
+  MBA defines a range of MSRs to support specifying a delay value (Thrtl) per
+  COS, with details below.
+
+  ```
+   +----------------------------+----------------+
+   | MSR (per socket)           |    Address     |
+   +----------------------------+----------------+
+   | IA32_L2_QOS_Ext_BW_Thrtl_0 |     0xD50      |
+   +----------------------------+----------------+
+   | ...                        |  ...           |
+   +----------------------------+----------------+
+   | IA32_L2_QOS_Ext_BW_Thrtl_n |     0xD50+n    |
+   +----------------------------+----------------+
+  ```
+
+  When context switch happens, the COS ID of domain is written to per-hyper-
+  thread MSR `IA32_PQR_ASSOC`, and then hardware enforces bandwidth allocation
+  according to the throttling value stored in the Thrtl MSR register.
+
+## The relationship between MBA and CAT/CDP
+
+  Generally speaking, MBA is completely independent of CAT/CDP, and any
+  combination may be applied at any time, e.g. enabling MBA with CAT
+  disabled.
+
+  But it needs to be noticed that MBA shares COS infrastructure with CAT,
+  although MBA is enumerated by different CPUID leaf from CAT (which
+  indicates that the max COS of MBA may be different from CAT). In some
+  cases, a domain is permitted to have a COS that is beyond one (or more)
+  of PSR features but within the others. For instance, let's assume the max
+  COS of MBA is 8 but the max COS of L3 CAT is 16, when a domain is assigned
+  9 as COS, the L3 CAT CBM associated to COS 9 would be enforced, but for MBA,
+  the HW works as default value is set since COS 9 is beyond the max COS (8)
+  of MBA.
+
+## Design Overview
+
+* Core COS/Thrtl association
+
+  When enforcing Memory Bandwidth Allocation, all cores of domains have
+  the same default Thrtl MSR (COS0) which stores the same Thrtl (0). The
+  default Thrtl MSR is used only in hypervisor and is transparent to tool stack
+  and user.
+
+  System administrators can change PSR allocation policy at runtime by
+  using the tool stack. Since MBA shares COS ID with CAT/CDP, a COS ID
+  corresponds to a 2-tuple, like [CBM, Thrtl] with only-CAT enabled, when CDP
+  is enabled, the COS ID corresponds to a 3-tuple, like [Code_CBM, Data_CBM,
+  Thrtl]. If neither CAT nor CDP is enabled, things are easier, since one COS
+  ID corresponds to one Thrtl.
+
+* VCPU schedule
+
+  This part reuses CAT COS infrastructure.
+
+* Multi-sockets
+
+  Different sockets may have different MBA capabilities (like max COS)
+  although it is consistent on the same socket. So the capability
+  of per-socket MBA is specified.
+
+  This part reuses CAT COS infrastructure.
+
+## Implementation Description
+
+* Hypervisor interfaces:
+
+  1. Boot line param: "psr=mba" to enable the feature.
+
+  2. SYSCTL:
+          - XEN_SYSCTL_PSR_MBA_get_info: Get system MBA information.
+
+  3. DOMCTL:
+          - XEN_DOMCTL_PSR_MBA_OP_GET_THRTL: Get throttling for a domain.
+          - XEN_DOMCTL_PSR_MBA_OP_SET_THRTL: Set throttling for a domain.
+
+* xl interfaces:
+
+  1. psr-mba-show [domain-id]
+          Show system/domain runtime MBA throttling value. For linear mode,
+          it shows the decimal value. For non-linear mode, it shows hexadecimal
+          value.
+          => XEN_SYSCTL_PSR_MBA_get_info/XEN_DOMCTL_PSR_MBA_OP_GET_THRTL
+
+  2. psr-mba-set [OPTIONS] <domain-id> <throttling>
+          Set bandwidth throttling for a domain.
+          => XEN_DOMCTL_PSR_MBA_OP_SET_THRTL
+
+  3. psr-hwinfo
+          Show PSR HW information, including L3 CAT/CDP/L2 CAT/MBA.
+          => XEN_SYSCTL_PSR_MBA_get_info
+
+* Key data structure:
+
+  1. Feature HW info
+
+     ```
+     struct {
+         unsigned int thrtl_max;
+         bool linear;
+     } mba;
+
+     - Member `thrtl_max`
+
+       `thrtl_max` is the max throttling value to be set, i.e. MBA_MAX.
+
+     - Member `linear`
+
+       `linear` means the response of delay value is linear or not.
+
+     As mentioned above, MBA is a member of Intel PSR features, it shares the
+     base PSR infrastructure in Xen. For example, the 'cos_max' is a common HW
+     property for all features. So, for other data structure details, please
+     refer to 'intel_psr_cat_cdp.pandoc'.
+
+# Limitations
+
+MBA can only work on HW which supports it (check CPUID).
+
+# Testing
+
+We can execute these commands to verify MBA on different HWs supporting them.
+
+For example:
+  1. User can get the MBA hardware info through 'psr-hwinfo' command. From
+     result, user can know if this hardware works under linear mode or non-
+     linear mode, the max throttling value (MBA_MAX) and so on.
+
+    root@:~$ xl psr-hwinfo --mba
+    Memory Bandwidth Allocation (MBA):
+    Socket ID       : 0
+    Linear Mode     : Enabled
+    Maximum COS     : 7
+    Maximum Throttling Value: 90
+    Default Throttling Value: 0
+
+  2. Then, user can set a throttling value to a domain. For example, set '10',
+     i.e 10% delay.
+
+    root@:~$ xl psr-mba-set 1 10
+
+  3. User can check the current configuration of the domain through
+     'psr-mab-show'. For linear mode, the decimal value is shown.
+
+    root@:~$ xl psr-mba-show 1
+    Socket ID       : 0
+    Default THRTL   : 0
+       ID                     NAME            THRTL
+        1                 ubuntu14             10
+
+# Areas for improvement
+
+N/A
+
+# Known issues
+
+N/A
+
+# References
+
+"INTEL RESOURCE DIRECTOR TECHNOLOGY (INTEL RDT) ALLOCATION FEATURES" [Intel 64 and IA-32 Architectures Software Developer Manuals, vol3](http://www.intel.com/content/www/us/en/processors/architectures-software-developer-manuals.html)
+
+# History
+
+------------------------------------------------------------------------
+Date       Revision Version  Notes
+---------- -------- -------- -------------------------------------------
+2017-01-10 1.0      Xen 4.9  Design document written
+2017-07-10 1.1      Xen 4.10 Changes:
+                             1. Modify data structure according to latest
+                                codes;
+                             2. Add content for 'Areas for improvement';
+                             3. Other minor changes.
+2017-08-09 1.2      Xen 4.10 Changes:
+                             1. Remove a special character to avoid error when
+                                building pandoc.
+2017-08-15 1.3      Xen 4.10 Changes:
+                             1. Add terminology 'HW'.
+                             2. Change 'COS ID of VCPU' to 'COS ID of domain'.
+                             3. Change 'COS register' to 'Thrtl MSR'.
+                             4. Explain the value shown for 'psr-mba-show' under
+                                different modes.
+                             5. Remove content in 'Areas for improvement'.
+2017-08-16 1.4      Xen 4.10 Changes:
+                             1. Add '<>' for mandatory argument.
+2017-08-30 1.5      Xen 4.10 Changes:
+                             1. Modify words in 'Overview' to make it easier to
+                                understand.
+                             2. Explain 'linear/non-linear' modes before mention
+                                them.
+                             3. Explain throttling value more accurate.
+                             4. Explain 'MBA_MAX'.
+                             5. Correct some words in 'Design Overview'.
+                             6. Change 'mba_info' to 'mba' according to code
+                                changes. Also, modify contents of it.
+                             7. Add context in 'Testing' part to make things
+                                more clear.
+                             8. Remove 'n<64' to avoid out-of-sync.
+2017-09-21 1.6      Xen 4.10 Changes:
+                             1. Add 'domain-name' as parameter of 'psr-mba-show/
+                                psr-mba-set'.
+                             2. Fix some wordings.
+                             3. Explain how user can know the MBA_MAX.
+                             4. Move the description of 'Linear mode/Non-linear
+                                mode' into section of 'psr-mba-show'.
+                             5. Change 'per-thread' to 'per-hyper-thread'.
+2017-09-29 1.7      Xen 4.10 Changes:
+                             1. Correct some words.
+                             2. Change 'xl psr-mba-set 1 0xa' to
+                                'xl psr-mba-set 1 10'
+2017-10-08 1.8      Xen 4.10 Changes:
+                             1. Correct some words.
+---------- -------- -------- -------------------------------------------
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
  2017-10-08  7:23 ` [PATCH v6 01/16] docs: create Memory Bandwidth Allocation (MBA) feature document Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-10 14:59   ` Jan Beulich
  2017-10-08  7:23 ` [PATCH v6 03/16] x86: rename 'cbm_type' to 'psr_type' to make it general Yi Sun
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Yi Sun, Andrew Cooper, Ian Jackson, Jan Beulich, Chao Peng,
	Wei Liu, Daniel De Graaf, Roger Pau Monné

This patch renames PSR sysctl/domctl interfaces and related xsm policy to
make them be general for all resource allocation features but not only
for CAT. Then, we can resuse the interfaces for all allocation features.

Basically, it changes 'psr_cat_op' to 'psr_alloc', and remove 'CAT_' from some
macros. E.g.:
1. psr_cat_op -> psr_alloc
2. XEN_DOMCTL_psr_cat_op -> XEN_DOMCTL_psr_alloc
3. XEN_SYSCTL_psr_cat_op -> XEN_SYSCTL_psr_alloc
4. XEN_DOMCTL_PSR_CAT_SET_L3_CBM -> XEN_DOMCTL_PSR_SET_L3_CBM
5. XEN_SYSCTL_PSR_CAT_get_l3_info -> XEN_SYSCTL_PSR_get_l3_info

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v6:
    - move macro definition into the function and undefine it after use.
      (suggested by Roger Pau Monné)
    - do not bump sysctl version because it has been bumped for 4.10.
      (suggested by Roger Pau Monné)
v5:
    - remove domctl version number upgrade.
      (suggested by Jan Beulich)
    - restore 'XEN_SYSCTL_PSR_CAT_L3_CDP'.
      (suggested by Jan Beulich)
    - define a local macro to complete psr get value flow.
      (suggested by Roger Pau Monné)
    - remove 'Reviewed-by' and 'Acked-by'.
      (suggested by Wei Liu)
v4:
    - remove 'ALLOC_' from names.
      (suggested by Roger Pau Monné)
    - fix comments.
      (suggested by Roger Pau Monné)
v3:
    - remove 'op/OP' from names and modify some names from 'PSR_CAT' to
      'PSR_ALLOC'.
      (suggested by Roger Pau Monné)
v1:
    - add description about what to be changed in commit message.
      (suggested by Wei Liu)
    - bump sysctl/domctl version numbers.
      (suggested by Wei Liu)
---
 tools/flask/policy/modules/dom0.te  |  4 +--
 tools/libxc/xc_psr.c                | 50 +++++++++++++-------------
 xen/arch/x86/domctl.c               | 71 ++++++++++++++++++-------------------
 xen/arch/x86/sysctl.c               | 28 +++++++--------
 xen/include/public/domctl.h         | 24 ++++++-------
 xen/include/public/sysctl.h         | 12 +++----
 xen/xsm/flask/hooks.c               |  8 ++---
 xen/xsm/flask/policy/access_vectors |  8 ++---
 8 files changed, 102 insertions(+), 103 deletions(-)

diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te
index 1643b40..07de3d5 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -14,7 +14,7 @@ allow dom0_t xen_t:xen {
 	tmem_control getscheduler setscheduler
 };
 allow dom0_t xen_t:xen2 {
-	resource_op psr_cmt_op psr_cat_op pmu_ctrl get_symbol
+	resource_op psr_cmt_op psr_alloc pmu_ctrl get_symbol
 	get_cpu_levelling_caps get_cpu_featureset livepatch_op
 	gcov_op set_parameter
 };
@@ -39,7 +39,7 @@ allow dom0_t dom0_t:domain {
 };
 allow dom0_t dom0_t:domain2 {
 	set_cpuid gettsc settsc setscheduler set_max_evtchn set_vnumainfo
-	get_vnumainfo psr_cmt_op psr_cat_op set_gnttab_limits
+	get_vnumainfo psr_cmt_op psr_alloc set_gnttab_limits
 };
 allow dom0_t dom0_t:resource { add remove };
 
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 039b920..5c54a35 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -258,27 +258,27 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
     switch ( type )
     {
     case XC_PSR_CAT_L3_CBM:
-        cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM;
+        cmd = XEN_DOMCTL_PSR_SET_L3_CBM;
         break;
     case XC_PSR_CAT_L3_CBM_CODE:
-        cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE;
+        cmd = XEN_DOMCTL_PSR_SET_L3_CODE;
         break;
     case XC_PSR_CAT_L3_CBM_DATA:
-        cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA;
+        cmd = XEN_DOMCTL_PSR_SET_L3_DATA;
         break;
     case XC_PSR_CAT_L2_CBM:
-        cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L2_CBM;
+        cmd = XEN_DOMCTL_PSR_SET_L2_CBM;
         break;
     default:
         errno = EINVAL;
         return -1;
     }
 
-    domctl.cmd = XEN_DOMCTL_psr_cat_op;
+    domctl.cmd = XEN_DOMCTL_psr_alloc;
     domctl.domain = (domid_t)domid;
-    domctl.u.psr_cat_op.cmd = cmd;
-    domctl.u.psr_cat_op.target = target;
-    domctl.u.psr_cat_op.data = data;
+    domctl.u.psr_alloc.cmd = cmd;
+    domctl.u.psr_alloc.target = target;
+    domctl.u.psr_alloc.data = data;
 
     return do_domctl(xch, &domctl);
 }
@@ -294,31 +294,31 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
     switch ( type )
     {
     case XC_PSR_CAT_L3_CBM:
-        cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM;
+        cmd = XEN_DOMCTL_PSR_GET_L3_CBM;
         break;
     case XC_PSR_CAT_L3_CBM_CODE:
-        cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE;
+        cmd = XEN_DOMCTL_PSR_GET_L3_CODE;
         break;
     case XC_PSR_CAT_L3_CBM_DATA:
-        cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA;
+        cmd = XEN_DOMCTL_PSR_GET_L3_DATA;
         break;
     case XC_PSR_CAT_L2_CBM:
-        cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L2_CBM;
+        cmd = XEN_DOMCTL_PSR_GET_L2_CBM;
         break;
     default:
         errno = EINVAL;
         return -1;
     }
 
-    domctl.cmd = XEN_DOMCTL_psr_cat_op;
+    domctl.cmd = XEN_DOMCTL_psr_alloc;
     domctl.domain = (domid_t)domid;
-    domctl.u.psr_cat_op.cmd = cmd;
-    domctl.u.psr_cat_op.target = target;
+    domctl.u.psr_alloc.cmd = cmd;
+    domctl.u.psr_alloc.target = target;
 
     rc = do_domctl(xch, &domctl);
 
     if ( !rc )
-        *data = domctl.u.psr_cat_op.data;
+        *data = domctl.u.psr_alloc.data;
 
     return rc;
 }
@@ -329,29 +329,29 @@ int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, unsigned int lvl,
     int rc = -1;
     DECLARE_SYSCTL;
 
-    sysctl.cmd = XEN_SYSCTL_psr_cat_op;
-    sysctl.u.psr_cat_op.target = socket;
+    sysctl.cmd = XEN_SYSCTL_psr_alloc;
+    sysctl.u.psr_alloc.target = socket;
 
     switch ( lvl )
     {
     case 2:
-        sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l2_info;
+        sysctl.u.psr_alloc.cmd = XEN_SYSCTL_PSR_get_l2_info;
         rc = xc_sysctl(xch, &sysctl);
         if ( !rc )
         {
-            *cos_max = sysctl.u.psr_cat_op.u.cat_info.cos_max;
-            *cbm_len = sysctl.u.psr_cat_op.u.cat_info.cbm_len;
+            *cos_max = sysctl.u.psr_alloc.u.cat_info.cos_max;
+            *cbm_len = sysctl.u.psr_alloc.u.cat_info.cbm_len;
             *cdp_enabled = false;
         }
         break;
     case 3:
-        sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l3_info;
+        sysctl.u.psr_alloc.cmd = XEN_SYSCTL_PSR_get_l3_info;
         rc = xc_sysctl(xch, &sysctl);
         if ( !rc )
         {
-            *cos_max = sysctl.u.psr_cat_op.u.cat_info.cos_max;
-            *cbm_len = sysctl.u.psr_cat_op.u.cat_info.cbm_len;
-            *cdp_enabled = sysctl.u.psr_cat_op.u.cat_info.flags &
+            *cos_max = sysctl.u.psr_alloc.u.cat_info.cos_max;
+            *cbm_len = sysctl.u.psr_alloc.u.cat_info.cbm_len;
+            *cdp_enabled = sysctl.u.psr_alloc.u.cat_info.flags &
                            XEN_SYSCTL_PSR_CAT_L3_CDP;
         }
         break;
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 540ba08..8a6004b 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1438,67 +1438,66 @@ long arch_do_domctl(
         }
         break;
 
-    case XEN_DOMCTL_psr_cat_op:
-        switch ( domctl->u.psr_cat_op.cmd )
+    case XEN_DOMCTL_psr_alloc:
+#define domctl_psr_get_val(d, domctl, type, copyback) ({   \
+    uint32_t v;                                            \
+    int r = psr_get_val(d, domctl->u.psr_alloc.target,     \
+                        &v, type);                         \
+                                                           \
+    domctl->u.psr_alloc.data = v;                          \
+    copyback = true;                                       \
+    r;                                                     \
+})
+
+        switch ( domctl->u.psr_alloc.cmd )
         {
-            uint32_t val32;
-
-        case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
-            ret = psr_set_val(d, domctl->u.psr_cat_op.target,
-                              domctl->u.psr_cat_op.data,
+        case XEN_DOMCTL_PSR_SET_L3_CBM:
+            ret = psr_set_val(d, domctl->u.psr_alloc.target,
+                              domctl->u.psr_alloc.data,
                               PSR_CBM_TYPE_L3);
             break;
 
-        case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE:
-            ret = psr_set_val(d, domctl->u.psr_cat_op.target,
-                              domctl->u.psr_cat_op.data,
+        case XEN_DOMCTL_PSR_SET_L3_CODE:
+            ret = psr_set_val(d, domctl->u.psr_alloc.target,
+                              domctl->u.psr_alloc.data,
                               PSR_CBM_TYPE_L3_CODE);
             break;
 
-        case XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA:
-            ret = psr_set_val(d, domctl->u.psr_cat_op.target,
-                              domctl->u.psr_cat_op.data,
+        case XEN_DOMCTL_PSR_SET_L3_DATA:
+            ret = psr_set_val(d, domctl->u.psr_alloc.target,
+                              domctl->u.psr_alloc.data,
                               PSR_CBM_TYPE_L3_DATA);
             break;
 
-        case XEN_DOMCTL_PSR_CAT_OP_SET_L2_CBM:
-            ret = psr_set_val(d, domctl->u.psr_cat_op.target,
-                              domctl->u.psr_cat_op.data,
+        case XEN_DOMCTL_PSR_SET_L2_CBM:
+            ret = psr_set_val(d, domctl->u.psr_alloc.target,
+                              domctl->u.psr_alloc.data,
                               PSR_CBM_TYPE_L2);
             break;
 
-        case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM:
-            ret = psr_get_val(d, domctl->u.psr_cat_op.target,
-                              &val32, PSR_CBM_TYPE_L3);
-            domctl->u.psr_cat_op.data = val32;
-            copyback = true;
+        case XEN_DOMCTL_PSR_GET_L3_CBM:
+            ret = domctl_psr_get_val(d, domctl, PSR_CBM_TYPE_L3, copyback);
             break;
 
-        case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE:
-            ret = psr_get_val(d, domctl->u.psr_cat_op.target,
-                              &val32, PSR_CBM_TYPE_L3_CODE);
-            domctl->u.psr_cat_op.data = val32;
-            copyback = true;
+        case XEN_DOMCTL_PSR_GET_L3_CODE:
+            ret = domctl_psr_get_val(d, domctl, PSR_CBM_TYPE_L3_CODE, copyback);
             break;
 
-        case XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA:
-            ret = psr_get_val(d, domctl->u.psr_cat_op.target,
-                              &val32, PSR_CBM_TYPE_L3_DATA);
-            domctl->u.psr_cat_op.data = val32;
-            copyback = true;
+        case XEN_DOMCTL_PSR_GET_L3_DATA:
+            ret = domctl_psr_get_val(d, domctl, PSR_CBM_TYPE_L3_DATA, copyback);
             break;
 
-        case XEN_DOMCTL_PSR_CAT_OP_GET_L2_CBM:
-            ret = psr_get_val(d, domctl->u.psr_cat_op.target,
-                              &val32, PSR_CBM_TYPE_L2);
-            domctl->u.psr_cat_op.data = val32;
-            copyback = true;
+        case XEN_DOMCTL_PSR_GET_L2_CBM:
+            ret = domctl_psr_get_val(d, domctl, PSR_CBM_TYPE_L2, copyback);
             break;
 
         default:
             ret = -EOPNOTSUPP;
             break;
         }
+
+#undef domctl_psr_get_val
+
         break;
 
     case XEN_DOMCTL_disable_migrate:
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 6ba823c..8ae6747 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -171,45 +171,45 @@ long arch_do_sysctl(
 
         break;
 
-    case XEN_SYSCTL_psr_cat_op:
-        switch ( sysctl->u.psr_cat_op.cmd )
+    case XEN_SYSCTL_psr_alloc:
+        switch ( sysctl->u.psr_alloc.cmd )
         {
             uint32_t data[PSR_INFO_ARRAY_SIZE];
 
-        case XEN_SYSCTL_PSR_CAT_get_l3_info:
+        case XEN_SYSCTL_PSR_get_l3_info:
         {
-            ret = psr_get_info(sysctl->u.psr_cat_op.target,
+            ret = psr_get_info(sysctl->u.psr_alloc.target,
                                PSR_CBM_TYPE_L3, data, ARRAY_SIZE(data));
             if ( ret )
                 break;
 
-            sysctl->u.psr_cat_op.u.cat_info.cos_max =
+            sysctl->u.psr_alloc.u.cat_info.cos_max =
                                       data[PSR_INFO_IDX_COS_MAX];
-            sysctl->u.psr_cat_op.u.cat_info.cbm_len =
+            sysctl->u.psr_alloc.u.cat_info.cbm_len =
                                       data[PSR_INFO_IDX_CAT_CBM_LEN];
-            sysctl->u.psr_cat_op.u.cat_info.flags =
+            sysctl->u.psr_alloc.u.cat_info.flags =
                                       data[PSR_INFO_IDX_CAT_FLAG];
 
-            if ( __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) )
+            if ( __copy_field_to_guest(u_sysctl, sysctl, u.psr_alloc) )
                 ret = -EFAULT;
             break;
         }
 
-        case XEN_SYSCTL_PSR_CAT_get_l2_info:
+        case XEN_SYSCTL_PSR_get_l2_info:
         {
-            ret = psr_get_info(sysctl->u.psr_cat_op.target,
+            ret = psr_get_info(sysctl->u.psr_alloc.target,
                                PSR_CBM_TYPE_L2, data, ARRAY_SIZE(data));
             if ( ret )
                 break;
 
-            sysctl->u.psr_cat_op.u.cat_info.cos_max =
+            sysctl->u.psr_alloc.u.cat_info.cos_max =
                                       data[PSR_INFO_IDX_COS_MAX];
-            sysctl->u.psr_cat_op.u.cat_info.cbm_len =
+            sysctl->u.psr_alloc.u.cat_info.cbm_len =
                                       data[PSR_INFO_IDX_CAT_CBM_LEN];
-            sysctl->u.psr_cat_op.u.cat_info.flags =
+            sysctl->u.psr_alloc.u.cat_info.flags =
                                       data[PSR_INFO_IDX_CAT_FLAG];
 
-            if ( __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) )
+            if ( __copy_field_to_guest(u_sysctl, sysctl, u.psr_alloc) )
                 ret = -EFAULT;
             break;
         }
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index a6448ea..c099334 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1060,16 +1060,16 @@ struct xen_domctl_monitor_op {
     } u;
 };
 
-struct xen_domctl_psr_cat_op {
-#define XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM     0
-#define XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM     1
-#define XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE    2
-#define XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA    3
-#define XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE    4
-#define XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA    5
-#define XEN_DOMCTL_PSR_CAT_OP_SET_L2_CBM     6
-#define XEN_DOMCTL_PSR_CAT_OP_GET_L2_CBM     7
-    uint32_t cmd;       /* IN: XEN_DOMCTL_PSR_CAT_OP_* */
+struct xen_domctl_psr_alloc {
+#define XEN_DOMCTL_PSR_SET_L3_CBM     0
+#define XEN_DOMCTL_PSR_GET_L3_CBM     1
+#define XEN_DOMCTL_PSR_SET_L3_CODE    2
+#define XEN_DOMCTL_PSR_SET_L3_DATA    3
+#define XEN_DOMCTL_PSR_GET_L3_CODE    4
+#define XEN_DOMCTL_PSR_GET_L3_DATA    5
+#define XEN_DOMCTL_PSR_SET_L2_CBM     6
+#define XEN_DOMCTL_PSR_GET_L2_CBM     7
+    uint32_t cmd;       /* IN: XEN_DOMCTL_PSR_* */
     uint32_t target;    /* IN */
     uint64_t data;      /* IN/OUT */
 };
@@ -1154,7 +1154,7 @@ struct xen_domctl {
 #define XEN_DOMCTL_setvnumainfo                  74
 #define XEN_DOMCTL_psr_cmt_op                    75
 #define XEN_DOMCTL_monitor_op                    77
-#define XEN_DOMCTL_psr_cat_op                    78
+#define XEN_DOMCTL_psr_alloc                     78
 #define XEN_DOMCTL_soft_reset                    79
 #define XEN_DOMCTL_set_gnttab_limits             80
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
@@ -1218,7 +1218,7 @@ struct xen_domctl {
         struct xen_domctl_vnuma             vnuma;
         struct xen_domctl_psr_cmt_op        psr_cmt_op;
         struct xen_domctl_monitor_op        monitor_op;
-        struct xen_domctl_psr_cat_op        psr_cat_op;
+        struct xen_domctl_psr_alloc         psr_alloc;
         struct xen_domctl_set_gnttab_limits set_gnttab_limits;
         uint8_t                             pad[128];
     } u;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 6140f1a..a50e345 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -696,10 +696,10 @@ struct xen_sysctl_pcitopoinfo {
     XEN_GUEST_HANDLE_64(uint32) nodes;
 };
 
-#define XEN_SYSCTL_PSR_CAT_get_l3_info               0
-#define XEN_SYSCTL_PSR_CAT_get_l2_info               1
-struct xen_sysctl_psr_cat_op {
-    uint32_t cmd;       /* IN: XEN_SYSCTL_PSR_CAT_* */
+#define XEN_SYSCTL_PSR_get_l3_info               0
+#define XEN_SYSCTL_PSR_get_l2_info               1
+struct xen_sysctl_psr_alloc {
+    uint32_t cmd;       /* IN: XEN_SYSCTL_PSR_* */
     uint32_t target;    /* IN */
     union {
         struct {
@@ -1068,7 +1068,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_gcov_op                       20
 #define XEN_SYSCTL_psr_cmt_op                    21
 #define XEN_SYSCTL_pcitopoinfo                   22
-#define XEN_SYSCTL_psr_cat_op                    23
+#define XEN_SYSCTL_psr_alloc                     23
 #define XEN_SYSCTL_tmem_op                       24
 #define XEN_SYSCTL_get_cpu_levelling_caps        25
 #define XEN_SYSCTL_get_cpu_featureset            26
@@ -1097,7 +1097,7 @@ struct xen_sysctl {
         struct xen_sysctl_scheduler_op      scheduler_op;
         struct xen_sysctl_gcov_op           gcov_op;
         struct xen_sysctl_psr_cmt_op        psr_cmt_op;
-        struct xen_sysctl_psr_cat_op        psr_cat_op;
+        struct xen_sysctl_psr_alloc         psr_alloc;
         struct xen_sysctl_tmem_op           tmem_op;
         struct xen_sysctl_cpu_levelling_caps cpu_levelling_caps;
         struct xen_sysctl_cpu_featureset    cpu_featureset;
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 7b005af..50e103a 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -743,8 +743,8 @@ static int flask_domctl(struct domain *d, int cmd)
     case XEN_DOMCTL_psr_cmt_op:
         return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_CMT_OP);
 
-    case XEN_DOMCTL_psr_cat_op:
-        return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_CAT_OP);
+    case XEN_DOMCTL_psr_alloc:
+        return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_ALLOC);
 
     case XEN_DOMCTL_soft_reset:
         return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__SOFT_RESET);
@@ -810,9 +810,9 @@ static int flask_sysctl(int cmd)
     case XEN_SYSCTL_psr_cmt_op:
         return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
                                     XEN2__PSR_CMT_OP, NULL);
-    case XEN_SYSCTL_psr_cat_op:
+    case XEN_SYSCTL_psr_alloc:
         return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
-                                    XEN2__PSR_CAT_OP, NULL);
+                                    XEN2__PSR_ALLOC, NULL);
 
     case XEN_SYSCTL_tmem_op:
         return domain_has_xen(current->domain, XEN__TMEM_CONTROL);
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index 3a2d863..d0a1ec5 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -85,8 +85,8 @@ class xen2
     resource_op
 # XEN_SYSCTL_psr_cmt_op
     psr_cmt_op
-# XEN_SYSCTL_psr_cat_op
-    psr_cat_op
+# XEN_SYSCTL_psr_alloc
+    psr_alloc
 # XENPF_get_symbol
     get_symbol
 # PMU control
@@ -246,8 +246,8 @@ class domain2
     mem_paging
 # XENMEM_sharing_op
     mem_sharing
-# XEN_DOMCTL_psr_cat_op
-    psr_cat_op
+# XEN_DOMCTL_psr_alloc
+    psr_alloc
 # XEN_DOMCTL_set_gnttab_limits
     set_gnttab_limits
 }
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 03/16] x86: rename 'cbm_type' to 'psr_type' to make it general
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
  2017-10-08  7:23 ` [PATCH v6 01/16] docs: create Memory Bandwidth Allocation (MBA) feature document Yi Sun
  2017-10-08  7:23 ` [PATCH v6 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-08  7:23 ` [PATCH v6 04/16] x86: a few optimizations to psr codes Yi Sun
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Yi Sun, Andrew Cooper, Jan Beulich, Chao Peng,
	Roger Pau Monné

This patch renames 'cbm_type' to 'psr_type' to generalize it.
Then, we can reuse this for all psr allocation features.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v5:
    - correct character of reviewer's name.
      (suggested by Jan Beulich)
v4:
    - fix words in commit message.
      (suggested by Roger Pau Monné)
v3:
    - replace 'psr_val_type' to 'psr_type' and remove '_VAL' from the enum
      items.
      (suggested by Roger Pau Monné)
v2:
    - replace 'PSR_VAL_TYPE_{L3, L2}' to 'PSR_VAL_TYPE_{L3, L2}_CBM'.
      (suggested by Chao Peng)
---
 xen/arch/x86/domctl.c     | 16 ++++++------
 xen/arch/x86/psr.c        | 62 +++++++++++++++++++++++++----------------------
 xen/arch/x86/sysctl.c     |  4 +--
 xen/include/asm-x86/psr.h | 18 +++++++-------
 4 files changed, 52 insertions(+), 48 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 8a6004b..0cd18a6 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1454,41 +1454,41 @@ long arch_do_domctl(
         case XEN_DOMCTL_PSR_SET_L3_CBM:
             ret = psr_set_val(d, domctl->u.psr_alloc.target,
                               domctl->u.psr_alloc.data,
-                              PSR_CBM_TYPE_L3);
+                              PSR_TYPE_L3_CBM);
             break;
 
         case XEN_DOMCTL_PSR_SET_L3_CODE:
             ret = psr_set_val(d, domctl->u.psr_alloc.target,
                               domctl->u.psr_alloc.data,
-                              PSR_CBM_TYPE_L3_CODE);
+                              PSR_TYPE_L3_CODE);
             break;
 
         case XEN_DOMCTL_PSR_SET_L3_DATA:
             ret = psr_set_val(d, domctl->u.psr_alloc.target,
                               domctl->u.psr_alloc.data,
-                              PSR_CBM_TYPE_L3_DATA);
+                              PSR_TYPE_L3_DATA);
             break;
 
         case XEN_DOMCTL_PSR_SET_L2_CBM:
             ret = psr_set_val(d, domctl->u.psr_alloc.target,
                               domctl->u.psr_alloc.data,
-                              PSR_CBM_TYPE_L2);
+                              PSR_TYPE_L2_CBM);
             break;
 
         case XEN_DOMCTL_PSR_GET_L3_CBM:
-            ret = domctl_psr_get_val(d, domctl, PSR_CBM_TYPE_L3, copyback);
+            ret = domctl_psr_get_val(d, domctl, PSR_TYPE_L3_CBM, copyback);
             break;
 
         case XEN_DOMCTL_PSR_GET_L3_CODE:
-            ret = domctl_psr_get_val(d, domctl, PSR_CBM_TYPE_L3_CODE, copyback);
+            ret = domctl_psr_get_val(d, domctl, PSR_TYPE_L3_CODE, copyback);
             break;
 
         case XEN_DOMCTL_PSR_GET_L3_DATA:
-            ret = domctl_psr_get_val(d, domctl, PSR_CBM_TYPE_L3_DATA, copyback);
+            ret = domctl_psr_get_val(d, domctl, PSR_TYPE_L3_DATA, copyback);
             break;
 
         case XEN_DOMCTL_PSR_GET_L2_CBM:
-            ret = domctl_psr_get_val(d, domctl, PSR_CBM_TYPE_L2, copyback);
+            ret = domctl_psr_get_val(d, domctl, PSR_TYPE_L2_CBM, copyback);
             break;
 
         default:
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index dbf7a4c..263ddbe 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -100,24 +100,24 @@ static const struct feat_props {
     unsigned int cos_num;
 
     /*
-     * An array to save all 'enum cbm_type' values of the feature. It is
+     * An array to save all 'enum psr_type' values of the feature. It is
      * used with cos_num together to get/write a feature's COS registers
      * values one by one.
      */
-    enum cbm_type type[MAX_COS_NUM];
+    enum psr_type type[MAX_COS_NUM];
 
     /*
      * alt_type is 'alternative type'. When this 'alt_type' is input, the
      * feature does some special operations.
      */
-    enum cbm_type alt_type;
+    enum psr_type alt_type;
 
     /* get_feat_info is used to return feature HW info through sysctl. */
     bool (*get_feat_info)(const struct feat_node *feat,
                           uint32_t data[], unsigned int array_len);
 
     /* write_msr is used to write out feature MSR register. */
-    void (*write_msr)(unsigned int cos, uint32_t val, enum cbm_type type);
+    void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
 } *feat_props[FEAT_TYPE_NUM];
 
 /*
@@ -215,13 +215,13 @@ static void free_socket_resources(unsigned int socket)
     bitmap_zero(info->dom_set, DOMID_IDLE + 1);
 }
 
-static enum psr_feat_type psr_cbm_type_to_feat_type(enum cbm_type type)
+static enum psr_feat_type psr_type_to_feat_type(enum psr_type type)
 {
     enum psr_feat_type feat_type = FEAT_TYPE_UNKNOWN;
 
     switch ( type )
     {
-    case PSR_CBM_TYPE_L3:
+    case PSR_TYPE_L3_CBM:
         feat_type = FEAT_TYPE_L3_CAT;
 
         /*
@@ -233,12 +233,12 @@ static enum psr_feat_type psr_cbm_type_to_feat_type(enum cbm_type type)
 
         break;
 
-    case PSR_CBM_TYPE_L3_DATA:
-    case PSR_CBM_TYPE_L3_CODE:
+    case PSR_TYPE_L3_DATA:
+    case PSR_TYPE_L3_CODE:
         feat_type = FEAT_TYPE_L3_CDP;
         break;
 
-    case PSR_CBM_TYPE_L2:
+    case PSR_TYPE_L2_CBM:
         feat_type = FEAT_TYPE_L2_CAT;
         break;
 
@@ -362,15 +362,16 @@ static bool cat_get_feat_info(const struct feat_node *feat,
 }
 
 /* L3 CAT props */
-static void l3_cat_write_msr(unsigned int cos, uint32_t val, enum cbm_type type)
+static void l3_cat_write_msr(unsigned int cos, uint32_t val,
+                             enum psr_type type)
 {
     wrmsrl(MSR_IA32_PSR_L3_MASK(cos), val);
 }
 
 static const struct feat_props l3_cat_props = {
     .cos_num = 1,
-    .type[0] = PSR_CBM_TYPE_L3,
-    .alt_type = PSR_CBM_TYPE_UNKNOWN,
+    .type[0] = PSR_TYPE_L3_CBM,
+    .alt_type = PSR_TYPE_UNKNOWN,
     .get_feat_info = cat_get_feat_info,
     .write_msr = l3_cat_write_msr,
 };
@@ -387,9 +388,10 @@ static bool l3_cdp_get_feat_info(const struct feat_node *feat,
     return true;
 }
 
-static void l3_cdp_write_msr(unsigned int cos, uint32_t val, enum cbm_type type)
+static void l3_cdp_write_msr(unsigned int cos, uint32_t val,
+                             enum psr_type type)
 {
-    wrmsrl(((type == PSR_CBM_TYPE_L3_DATA) ?
+    wrmsrl(((type == PSR_TYPE_L3_DATA) ?
             MSR_IA32_PSR_L3_MASK_DATA(cos) :
             MSR_IA32_PSR_L3_MASK_CODE(cos)),
            val);
@@ -397,23 +399,24 @@ static void l3_cdp_write_msr(unsigned int cos, uint32_t val, enum cbm_type type)
 
 static const struct feat_props l3_cdp_props = {
     .cos_num = 2,
-    .type[0] = PSR_CBM_TYPE_L3_DATA,
-    .type[1] = PSR_CBM_TYPE_L3_CODE,
-    .alt_type = PSR_CBM_TYPE_L3,
+    .type[0] = PSR_TYPE_L3_DATA,
+    .type[1] = PSR_TYPE_L3_CODE,
+    .alt_type = PSR_TYPE_L3_CBM,
     .get_feat_info = l3_cdp_get_feat_info,
     .write_msr = l3_cdp_write_msr,
 };
 
 /* L2 CAT props */
-static void l2_cat_write_msr(unsigned int cos, uint32_t val, enum cbm_type type)
+static void l2_cat_write_msr(unsigned int cos, uint32_t val,
+                             enum psr_type type)
 {
     wrmsrl(MSR_IA32_PSR_L2_MASK(cos), val);
 }
 
 static const struct feat_props l2_cat_props = {
     .cos_num = 1,
-    .type[0] = PSR_CBM_TYPE_L2,
-    .alt_type = PSR_CBM_TYPE_UNKNOWN,
+    .type[0] = PSR_TYPE_L2_CBM,
+    .alt_type = PSR_TYPE_UNKNOWN,
     .get_feat_info = cat_get_feat_info,
     .write_msr = l2_cat_write_msr,
 };
@@ -675,7 +678,7 @@ static struct psr_socket_info *get_socket_info(unsigned int socket)
     return socket_info + socket;
 }
 
-int psr_get_info(unsigned int socket, enum cbm_type type,
+int psr_get_info(unsigned int socket, enum psr_type type,
                  uint32_t data[], unsigned int array_len)
 {
     const struct psr_socket_info *info = get_socket_info(socket);
@@ -687,7 +690,7 @@ int psr_get_info(unsigned int socket, enum cbm_type type,
     if ( IS_ERR(info) )
         return PTR_ERR(info);
 
-    feat_type = psr_cbm_type_to_feat_type(type);
+    feat_type = psr_type_to_feat_type(type);
     if ( feat_type >= ARRAY_SIZE(info->features) )
         return -ENOENT;
 
@@ -708,7 +711,7 @@ int psr_get_info(unsigned int socket, enum cbm_type type,
 }
 
 int psr_get_val(struct domain *d, unsigned int socket,
-                uint32_t *val, enum cbm_type type)
+                uint32_t *val, enum psr_type type)
 {
     const struct psr_socket_info *info = get_socket_info(socket);
     const struct feat_node *feat;
@@ -720,7 +723,7 @@ int psr_get_val(struct domain *d, unsigned int socket,
     if ( IS_ERR(info) )
         return PTR_ERR(info);
 
-    feat_type = psr_cbm_type_to_feat_type(type);
+    feat_type = psr_type_to_feat_type(type);
     if ( feat_type >= ARRAY_SIZE(info->features) )
         return -ENOENT;
 
@@ -850,7 +853,7 @@ static int insert_val_into_array(uint32_t val[],
                                  unsigned int array_len,
                                  const struct psr_socket_info *info,
                                  enum psr_feat_type feat_type,
-                                 enum cbm_type type,
+                                 enum psr_type type,
                                  uint32_t new_val)
 {
     const struct feat_node *feat;
@@ -886,8 +889,9 @@ static int insert_val_into_array(uint32_t val[],
     /*
      * Value setting position is same as feature array.
      * For CDP, user may set both DATA and CODE to same value. For such case,
-     * user input 'PSR_CBM_TYPE_L3' as type. The alternative type of CDP is same
-     * as it. So we should set new_val to both of DATA and CODE under such case.
+     * user input 'PSR_TYPE_L3_CBM' as type. The alternative type of CDP is
+     * same as it. So we should set new_val to both of DATA and CODE under such
+     * case.
      */
     for ( i = 0; i < props->cos_num; i++ )
     {
@@ -1182,7 +1186,7 @@ static int write_psr_msrs(unsigned int socket, unsigned int cos,
 }
 
 int psr_set_val(struct domain *d, unsigned int socket,
-                uint64_t new_val, enum cbm_type type)
+                uint64_t new_val, enum psr_type type)
 {
     unsigned int old_cos, array_len;
     int cos, ret;
@@ -1198,7 +1202,7 @@ int psr_set_val(struct domain *d, unsigned int socket,
     if ( new_val != val )
         return -EINVAL;
 
-    feat_type = psr_cbm_type_to_feat_type(type);
+    feat_type = psr_type_to_feat_type(type);
     if ( feat_type >= ARRAY_SIZE(info->features) ||
          !info->features[feat_type] )
         return -ENOENT;
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 8ae6747..6867ee1 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -179,7 +179,7 @@ long arch_do_sysctl(
         case XEN_SYSCTL_PSR_get_l3_info:
         {
             ret = psr_get_info(sysctl->u.psr_alloc.target,
-                               PSR_CBM_TYPE_L3, data, ARRAY_SIZE(data));
+                               PSR_TYPE_L3_CBM, data, ARRAY_SIZE(data));
             if ( ret )
                 break;
 
@@ -198,7 +198,7 @@ long arch_do_sysctl(
         case XEN_SYSCTL_PSR_get_l2_info:
         {
             ret = psr_get_info(sysctl->u.psr_alloc.target,
-                               PSR_CBM_TYPE_L2, data, ARRAY_SIZE(data));
+                               PSR_TYPE_L2_CBM, data, ARRAY_SIZE(data));
             if ( ret )
                 break;
 
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index 18a42f3..cb3f067 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -53,12 +53,12 @@ struct psr_cmt {
     struct psr_cmt_l3 l3;
 };
 
-enum cbm_type {
-    PSR_CBM_TYPE_L3,
-    PSR_CBM_TYPE_L3_CODE,
-    PSR_CBM_TYPE_L3_DATA,
-    PSR_CBM_TYPE_L2,
-    PSR_CBM_TYPE_UNKNOWN,
+enum psr_type {
+    PSR_TYPE_L3_CBM,
+    PSR_TYPE_L3_CODE,
+    PSR_TYPE_L3_DATA,
+    PSR_TYPE_L2_CBM,
+    PSR_TYPE_UNKNOWN,
 };
 
 extern struct psr_cmt *psr_cmt;
@@ -72,12 +72,12 @@ int psr_alloc_rmid(struct domain *d);
 void psr_free_rmid(struct domain *d);
 void psr_ctxt_switch_to(struct domain *d);
 
-int psr_get_info(unsigned int socket, enum cbm_type type,
+int psr_get_info(unsigned int socket, enum psr_type type,
                  uint32_t data[], unsigned int array_len);
 int psr_get_val(struct domain *d, unsigned int socket,
-                uint32_t *val, enum cbm_type type);
+                uint32_t *val, enum psr_type type);
 int psr_set_val(struct domain *d, unsigned int socket,
-                uint64_t val, enum cbm_type type);
+                uint64_t val, enum psr_type type);
 
 void psr_domain_init(struct domain *d);
 void psr_domain_free(struct domain *d);
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 04/16] x86: a few optimizations to psr codes
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (2 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 03/16] x86: rename 'cbm_type' to 'psr_type' to make it general Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-10 15:01   ` Jan Beulich
  2017-10-08  7:23 ` [PATCH v6 05/16] x86: implement data structure and CPU init flow for MBA Yi Sun
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Yi Sun, Andrew Cooper, Jan Beulich, Chao Peng,
	Roger Pau Monné

This patch refines psr codes:
1. Change type of 'cat_init_feature' to 'bool' to remove the pointless
   returning of error code.
2. Move printk in 'cat_init_feature' to reduce a return path.
3. Define a local variable 'feat_mask' in 'psr_cpu_init' to reduce calling of
   'cpuid_count_leaf()'.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v6:
    - restore 'write_msr()' type to 'void'.
      (suggested by Jan Beulich and Roger Pau Monné)
    - change 'ebx' in 'psr_cpu_init' to 'feat_mask'.
      (suggested by Jan Beulich and Roger Pau Monné)
v5:
    - create this patch to make codes clearer.
      (suggested by Jan Beulich and Roger Pau Monné)
---
 xen/arch/x86/psr.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 263ddbe..5945efa 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -273,10 +273,10 @@ static bool psr_check_cbm(unsigned int cbm_len, unsigned long cbm)
 }
 
 /* CAT common functions implementation. */
-static int cat_init_feature(const struct cpuid_leaf *regs,
-                            struct feat_node *feat,
-                            struct psr_socket_info *info,
-                            enum psr_feat_type type)
+static bool cat_init_feature(const struct cpuid_leaf *regs,
+                             struct feat_node *feat,
+                             struct psr_socket_info *info,
+                             enum psr_feat_type type)
 {
     const char *const cat_feat_name[FEAT_TYPE_NUM] = {
         [FEAT_TYPE_L3_CAT] = "L3 CAT",
@@ -286,7 +286,7 @@ static int cat_init_feature(const struct cpuid_leaf *regs,
 
     /* No valid value so do not enable feature. */
     if ( !regs->a || !regs->d )
-        return -ENOENT;
+        return false;
 
     feat->cbm_len = (regs->a & CAT_CBM_LEN_MASK) + 1;
     feat->cos_max = min(opt_cos_max, regs->d & CAT_COS_MAX_MASK);
@@ -296,7 +296,7 @@ static int cat_init_feature(const struct cpuid_leaf *regs,
     case FEAT_TYPE_L3_CAT:
     case FEAT_TYPE_L2_CAT:
         if ( feat->cos_max < 1 )
-            return -ENOENT;
+            return false;
 
         /* We reserve cos=0 as default cbm (all bits within cbm_len are 1). */
         feat->cos_reg_val[0] = cat_default_val(feat->cbm_len);
@@ -313,7 +313,7 @@ static int cat_init_feature(const struct cpuid_leaf *regs,
         uint64_t val;
 
         if ( feat->cos_max < 3 )
-            return -ENOENT;
+            return false;
 
         /* Cut half of cos_max when CDP is enabled. */
         feat->cos_max = (feat->cos_max - 1) >> 1;
@@ -332,20 +332,18 @@ static int cat_init_feature(const struct cpuid_leaf *regs,
     }
 
     default:
-        return -ENOENT;
+        return false;
     }
 
     /* Add this feature into array. */
     info->features[type] = feat;
 
-    if ( !opt_cpu_info )
-        return 0;
-
-    printk(XENLOG_INFO "%s: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
-           cat_feat_name[type], cpu_to_socket(smp_processor_id()),
-           feat->cos_max, feat->cbm_len);
+    if ( opt_cpu_info )
+        printk(XENLOG_INFO "%s: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
+               cat_feat_name[type], cpu_to_socket(smp_processor_id()),
+               feat->cos_max, feat->cbm_len);
 
-    return 0;
+    return true;
 }
 
 static bool cat_get_feat_info(const struct feat_node *feat,
@@ -1416,6 +1414,7 @@ static void psr_cpu_init(void)
     unsigned int socket, cpu = smp_processor_id();
     struct feat_node *feat;
     struct cpuid_leaf regs;
+    uint32_t feat_mask;
 
     if ( !psr_alloc_feat_enabled() || !boot_cpu_has(X86_FEATURE_PQE) )
         goto assoc_init;
@@ -1434,7 +1433,8 @@ static void psr_cpu_init(void)
     spin_lock_init(&info->ref_lock);
 
     cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 0, &regs);
-    if ( regs.b & PSR_RESOURCE_TYPE_L3 )
+    feat_mask = regs.b;
+    if ( feat_mask & PSR_RESOURCE_TYPE_L3 )
     {
         cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 1, &regs);
 
@@ -1455,8 +1455,7 @@ static void psr_cpu_init(void)
         }
     }
 
-    cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 0, &regs);
-    if ( regs.b & PSR_RESOURCE_TYPE_L2 )
+    if ( feat_mask & PSR_RESOURCE_TYPE_L2 )
     {
         cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 2, &regs);
 
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 05/16] x86: implement data structure and CPU init flow for MBA
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (3 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 04/16] x86: a few optimizations to psr codes Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-10 15:26   ` Jan Beulich
  2017-10-08  7:23 ` [PATCH v6 06/16] x86: implement get hw info " Yi Sun
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Yi Sun, Andrew Cooper, Jan Beulich, Chao Peng,
	Roger Pau Monné

This patch implements main data structures of MBA.

Like CAT features, MBA HW info has cos_max which means the max thrtl
register number, and thrtl_max which means the max throttle value
(delay value). It also has a flag to represent if the throttle
value is linear or non-linear.

One thrtl register of MBA stores a throttle value for one or more
domains. The throttle value means the delay between L2 cache and next
cache level.

This patch also implements init flow for MBA and register stub
callback functions.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v6:
    - restore type of 'mba_write_msr' to 'void'.
v5:
    - move out some CAT codes optimization to a new patch.
      (suggested by Jan Beulich)
    - modify commit message.
      (suggested by Jan Beulich)
    - change print type of 'linear' to be %d.
      (suggested by Jan Beulich)
    - change type of 'mba_write_msr' to uint32_t.
    - move printk in 'mba_init_feature' to reduce one return path.
      (suggested by Roger Pau Monné)
    - move the MBA format string in printk to a new line.
      (suggested by Roger Pau Monné)
v4:
    - modify commit message.
      (suggested by Roger Pau Monné)
    - fix a comment.
      (suggested by Roger Pau Monné)
    - join two checks in a single if.
      (suggested by Roger Pau Monné)
    - remove redundant initialization of 'feat->cos_reg_val[0]'.
      (suggested by Roger Pau Monné)
    - change 'reg_b' to 'ebx'.
      (suggested by Jan Beulich)
    - change type of 'mba_init_feature' from 'int' to 'bool'.
      (suggested by Roger Pau Monné)
    - change type of 'cat_init_feature' from 'int' to 'bool'.
v3:
    - replace 'psr_val_type' to 'psr_type'. Also, change 'PSR_VAL_TYPE_MBA' to
      'PSR_TYPE_MBA_THRTL'.
      (suggested by Roger Pau Monné)
    - replace 'MBA_LINEAR' to 'MBA_LINEAR_MASK' to make the name more clear.
      (suggested by Roger Pau Monné)
    - replase 'cat_info'/'mba_info' to 'cat'/'mba' to make the names shorter.
      (suggested by Roger Pau Monné)
    - change type of 'linear' to 'bool'.
      (suggested by Roger Pau Monné)
    - make format string of printf in one line.
      (suggested by Roger Pau Monné)
v2:
    - modify commit message to replace 'cos register' to 'thrtl register' to
      make it accurate.
      (suggested by Chao Peng)
    - restore the place of the sentence to assign value to 'feat->cbm_len'
      because the MBA init flow is splitted out as a separate function in v1.
      (suggested by Chao Peng)
    - add comment to explain what the MBA thrtl defaul value '0' stands for.
      (suggested by Chao Peng)
    - check 'thrtl_max' under linear mode. It could not be euqal or larger than
      100.
      (suggested by Chao Peng)
v1:
    - rebase codes onto L2 CAT v15.
    - move comment to appropriate place.
      (suggested by Chao Peng)
    - implement 'mba_init_feature' and keep 'cat_init_feature'.
      (suggested by Chao Peng)
    - keep 'regs.b' into a local variable to avoid reading CPUID every time.
      (suggested by Chao Peng)
---
 xen/arch/x86/psr.c              | 135 ++++++++++++++++++++++++++++++++++------
 xen/include/asm-x86/msr-index.h |   1 +
 xen/include/asm-x86/psr.h       |   2 +
 3 files changed, 119 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 5945efa..157e11f 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -27,13 +27,16 @@
  * - CMT         Cache Monitoring Technology
  * - COS/CLOS    Class of Service. Also mean COS registers.
  * - COS_MAX     Max number of COS for the feature (minus 1)
+ * - MBA         Memory Bandwidth Allocation
  * - MSRs        Machine Specific Registers
  * - PSR         Intel Platform Shared Resource
+ * - THRTL_MAX   Max throttle value (delay value) of MBA
  */
 
 #define PSR_CMT        (1u << 0)
 #define PSR_CAT        (1u << 1)
 #define PSR_CDP        (1u << 2)
+#define PSR_MBA        (1u << 3)
 
 #define CAT_CBM_LEN_MASK 0x1f
 #define CAT_COS_MAX_MASK 0xffff
@@ -60,10 +63,14 @@
  */
 #define MAX_COS_NUM 2
 
+#define MBA_LINEAR_MASK    (1u << 2)
+#define MBA_THRTL_MAX_MASK 0xfff
+
 enum psr_feat_type {
     FEAT_TYPE_L3_CAT,
     FEAT_TYPE_L3_CDP,
     FEAT_TYPE_L2_CAT,
+    FEAT_TYPE_MBA,
     FEAT_TYPE_NUM,
     FEAT_TYPE_UNKNOWN,
 };
@@ -71,7 +78,6 @@ enum psr_feat_type {
 /*
  * This structure represents one feature.
  * cos_max     - The max COS registers number got through CPUID.
- * cbm_len     - The length of CBM got through CPUID.
  * cos_reg_val - Array to store the values of COS registers. One entry stores
  *               the value of one COS register.
  *               For L3 CAT and L2 CAT, one entry corresponds to one COS_ID.
@@ -80,9 +86,23 @@ enum psr_feat_type {
  *               cos_reg_val[1] (Code).
  */
 struct feat_node {
-    /* cos_max and cbm_len are common values for all features so far. */
+    /* cos_max is common among all features so far. */
     unsigned int cos_max;
-    unsigned int cbm_len;
+
+    /* Feature specific HW info. */
+    union {
+        struct {
+            /* The length of CBM got through CPUID. */
+            unsigned int cbm_len;
+        } cat;
+
+        struct {
+            /* The max throttling value got through CPUID. */
+            unsigned int thrtl_max;
+            bool linear;
+        } mba;
+    };
+
     uint32_t cos_reg_val[MAX_COS_REG_CNT];
 };
 
@@ -161,6 +181,7 @@ static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
  */
 static struct feat_node *feat_l3;
 static struct feat_node *feat_l2_cat;
+static struct feat_node *feat_mba;
 
 /* Common functions */
 #define cat_default_val(len) (0xffffffff >> (32 - (len)))
@@ -272,7 +293,7 @@ static bool psr_check_cbm(unsigned int cbm_len, unsigned long cbm)
     return true;
 }
 
-/* CAT common functions implementation. */
+/* Implementation of allocation features' functions. */
 static bool cat_init_feature(const struct cpuid_leaf *regs,
                              struct feat_node *feat,
                              struct psr_socket_info *info,
@@ -288,8 +309,8 @@ static bool cat_init_feature(const struct cpuid_leaf *regs,
     if ( !regs->a || !regs->d )
         return false;
 
-    feat->cbm_len = (regs->a & CAT_CBM_LEN_MASK) + 1;
     feat->cos_max = min(opt_cos_max, regs->d & CAT_COS_MAX_MASK);
+    feat->cat.cbm_len = (regs->a & CAT_CBM_LEN_MASK) + 1;
 
     switch ( type )
     {
@@ -299,12 +320,12 @@ static bool cat_init_feature(const struct cpuid_leaf *regs,
             return false;
 
         /* We reserve cos=0 as default cbm (all bits within cbm_len are 1). */
-        feat->cos_reg_val[0] = cat_default_val(feat->cbm_len);
+        feat->cos_reg_val[0] = cat_default_val(feat->cat.cbm_len);
 
         wrmsrl((type == FEAT_TYPE_L3_CAT ?
                 MSR_IA32_PSR_L3_MASK(0) :
                 MSR_IA32_PSR_L2_MASK(0)),
-               cat_default_val(feat->cbm_len));
+               cat_default_val(feat->cat.cbm_len));
 
         break;
 
@@ -319,11 +340,13 @@ static bool cat_init_feature(const struct cpuid_leaf *regs,
         feat->cos_max = (feat->cos_max - 1) >> 1;
 
         /* We reserve cos=0 as default cbm (all bits within cbm_len are 1). */
-        get_cdp_code(feat, 0) = cat_default_val(feat->cbm_len);
-        get_cdp_data(feat, 0) = cat_default_val(feat->cbm_len);
+        get_cdp_code(feat, 0) = cat_default_val(feat->cat.cbm_len);
+        get_cdp_data(feat, 0) = cat_default_val(feat->cat.cbm_len);
 
-        wrmsrl(MSR_IA32_PSR_L3_MASK(0), cat_default_val(feat->cbm_len));
-        wrmsrl(MSR_IA32_PSR_L3_MASK(1), cat_default_val(feat->cbm_len));
+        wrmsrl(MSR_IA32_PSR_L3_MASK(0),
+               cat_default_val(feat->cat.cbm_len));
+        wrmsrl(MSR_IA32_PSR_L3_MASK(1),
+               cat_default_val(feat->cat.cbm_len));
         rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
         wrmsrl(MSR_IA32_PSR_L3_QOS_CFG,
                val | (1ull << PSR_L3_QOS_CDP_ENABLE_BIT));
@@ -341,7 +364,44 @@ static bool cat_init_feature(const struct cpuid_leaf *regs,
     if ( opt_cpu_info )
         printk(XENLOG_INFO "%s: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
                cat_feat_name[type], cpu_to_socket(smp_processor_id()),
-               feat->cos_max, feat->cbm_len);
+               feat->cos_max, feat->cat.cbm_len);
+
+    return true;
+}
+
+static bool mba_init_feature(const struct cpuid_leaf *regs,
+                            struct feat_node *feat,
+                            struct psr_socket_info *info,
+                            enum psr_feat_type type)
+{
+    /* No valid value so do not enable feature. */
+    if ( !regs->a || !regs->d || type != FEAT_TYPE_MBA )
+        return false;
+
+    feat->cos_max = min(opt_cos_max, regs->d & CAT_COS_MAX_MASK);
+    if ( feat->cos_max < 1 )
+        return false;
+
+    feat->mba.thrtl_max = (regs->a & MBA_THRTL_MAX_MASK) + 1;
+
+    if ( regs->c & MBA_LINEAR_MASK )
+    {
+        feat->mba.linear = true;
+
+        if ( feat->mba.thrtl_max >= 100 )
+            return false;
+    }
+
+    wrmsrl(MSR_IA32_PSR_MBA_MASK(0), 0);
+
+    /* Add this feature into array. */
+    info->features[type] = feat;
+
+    if ( opt_cpu_info )
+        printk(XENLOG_INFO
+               "MBA: enabled on socket %u, cos_max:%u, thrtl_max:%u, linear:%d\n",
+               cpu_to_socket(smp_processor_id()),
+               feat->cos_max, feat->mba.thrtl_max, feat->mba.linear);
 
     return true;
 }
@@ -353,7 +413,7 @@ static bool cat_get_feat_info(const struct feat_node *feat,
         return false;
 
     data[PSR_INFO_IDX_COS_MAX] = feat->cos_max;
-    data[PSR_INFO_IDX_CAT_CBM_LEN] = feat->cbm_len;
+    data[PSR_INFO_IDX_CAT_CBM_LEN] = feat->cat.cbm_len;
     data[PSR_INFO_IDX_CAT_FLAG] = 0;
 
     return true;
@@ -419,6 +479,26 @@ static const struct feat_props l2_cat_props = {
     .write_msr = l2_cat_write_msr,
 };
 
+/* MBA props */
+static bool mba_get_feat_info(const struct feat_node *feat,
+                              uint32_t data[], unsigned int array_len)
+{
+    return false;
+}
+
+static void mba_write_msr(unsigned int cos, uint32_t val,
+                          enum psr_type type)
+{
+}
+
+static const struct feat_props mba_props = {
+    .cos_num = 1,
+    .type[0] = PSR_TYPE_MBA_THRTL,
+    .alt_type = PSR_TYPE_UNKNOWN,
+    .get_feat_info = mba_get_feat_info,
+    .write_msr = mba_write_msr,
+};
+
 static bool __init parse_psr_bool(const char *s, const char *delim,
                                   const char *ss, const char *feature,
                                   unsigned int mask)
@@ -477,7 +557,8 @@ static int __init parse_psr_param(const char *s)
         }
         else if ( !parse_psr_bool(s, val_delim, ss, "cmt", PSR_CMT) &&
                   !parse_psr_bool(s, val_delim, ss, "cat", PSR_CAT) &&
-                  !parse_psr_bool(s, val_delim, ss, "cdp", PSR_CDP) )
+                  !parse_psr_bool(s, val_delim, ss, "cdp", PSR_CDP) &&
+                  !parse_psr_bool(s, val_delim, ss, "mba", PSR_MBA) )
             rc = -EINVAL;
 
         s = ss + 1;
@@ -881,7 +962,7 @@ static int insert_val_into_array(uint32_t val[],
     if ( array_len < props->cos_num )
         return -ENOSPC;
 
-    if ( !psr_check_cbm(feat->cbm_len, new_val) )
+    if ( !psr_check_cbm(feat->cat.cbm_len, new_val) )
         return -EINVAL;
 
     /*
@@ -1405,6 +1486,10 @@ static int psr_cpu_prepare(void)
          (feat_l2_cat = xzalloc(struct feat_node)) == NULL )
         return -ENOMEM;
 
+    if ( feat_mba == NULL &&
+         (feat_mba = xzalloc(struct feat_node)) == NULL )
+        return -ENOMEM;
+
     return 0;
 }
 
@@ -1442,13 +1527,13 @@ static void psr_cpu_init(void)
         feat_l3 = NULL;
 
         if ( (regs.c & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) &&
-             !cat_init_feature(&regs, feat, info, FEAT_TYPE_L3_CDP) )
+             cat_init_feature(&regs, feat, info, FEAT_TYPE_L3_CDP) )
             feat_props[FEAT_TYPE_L3_CDP] = &l3_cdp_props;
 
         /* If CDP init fails, try to work as L3 CAT. */
         if ( !feat_props[FEAT_TYPE_L3_CDP] )
         {
-            if ( !cat_init_feature(&regs, feat, info, FEAT_TYPE_L3_CAT) )
+            if ( cat_init_feature(&regs, feat, info, FEAT_TYPE_L3_CAT) )
                 feat_props[FEAT_TYPE_L3_CAT] = &l3_cat_props;
             else
                 feat_l3 = feat;
@@ -1461,12 +1546,24 @@ static void psr_cpu_init(void)
 
         feat = feat_l2_cat;
         feat_l2_cat = NULL;
-        if ( !cat_init_feature(&regs, feat, info, FEAT_TYPE_L2_CAT) )
+        if ( cat_init_feature(&regs, feat, info, FEAT_TYPE_L2_CAT) )
             feat_props[FEAT_TYPE_L2_CAT] = &l2_cat_props;
         else
             feat_l2_cat = feat;
     }
 
+    if ( feat_mask & PSR_RESOURCE_TYPE_MBA )
+    {
+        cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 3, &regs);
+
+        feat = feat_mba;
+        feat_mba = NULL;
+        if ( mba_init_feature(&regs, feat, info, FEAT_TYPE_MBA) )
+            feat_props[FEAT_TYPE_MBA] = &mba_props;
+        else
+            feat_mba = feat;
+    }
+
     info->feat_init = true;
 
  assoc_init:
@@ -1526,7 +1623,7 @@ static int __init psr_presmp_init(void)
     if ( (opt_psr & PSR_CMT) && opt_rmid_max )
         init_psr_cmt(opt_rmid_max);
 
-    if ( opt_psr & (PSR_CAT | PSR_CDP) )
+    if ( opt_psr & (PSR_CAT | PSR_CDP | PSR_MBA) )
         init_psr();
 
     if ( psr_cpu_prepare() )
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index b99c623..a834f3b 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -348,6 +348,7 @@
 #define MSR_IA32_PSR_L3_MASK_CODE(n)	(0x00000c90 + (n) * 2 + 1)
 #define MSR_IA32_PSR_L3_MASK_DATA(n)	(0x00000c90 + (n) * 2)
 #define MSR_IA32_PSR_L2_MASK(n)		(0x00000d10 + (n))
+#define MSR_IA32_PSR_MBA_MASK(n)	(0x00000d50 + (n))
 
 /* Intel Model 6 */
 #define MSR_P6_PERFCTR(n)		(0x000000c1 + (n))
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index cb3f067..9d14264 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -24,6 +24,7 @@
 /* Resource Type Enumeration */
 #define PSR_RESOURCE_TYPE_L3            0x2
 #define PSR_RESOURCE_TYPE_L2            0x4
+#define PSR_RESOURCE_TYPE_MBA           0x8
 
 /* L3 Monitoring Features */
 #define PSR_CMT_L3_OCCUPANCY            0x1
@@ -58,6 +59,7 @@ enum psr_type {
     PSR_TYPE_L3_CODE,
     PSR_TYPE_L3_DATA,
     PSR_TYPE_L2_CBM,
+    PSR_TYPE_MBA_THRTL,
     PSR_TYPE_UNKNOWN,
 };
 
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 06/16] x86: implement get hw info flow for MBA
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (4 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 05/16] x86: implement data structure and CPU init flow for MBA Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-11 13:12   ` Jan Beulich
  2017-10-08  7:23 ` [PATCH v6 07/16] x86: implement get value interface " Yi Sun
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Yi Sun, Andrew Cooper, Jan Beulich, Chao Peng,
	Roger Pau Monné

This patch implements get HW info flow for MBA including its callback
function and sysctl interface.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v5:
    - use ASSERT in 'mba_get_feat_info'.
      (suggested by Roger Pau Monné)
    - correct initialization format of 'data[PSR_INFO_ARRAY_SIZE]'.
      (suggested by Roger Pau Monné and Jan Beulich)
v4:
    - remove 'ALLOC_' from macro names.
      (suggested by Roger Pau Monné)
    - initialize 'data[PSR_INFO_ARRAY_SIZE]' to 0 to prevent to leak stack data.
      (suggested by Roger Pau Monné)
v3:
    - replace 'PSR_VAL_TYPE_MBA' to 'PSR_TYPE_MBA_THRTL'.
      (suggested by Roger Pau Monné)
v2:
    - use 'XEN_SYSCTL_PSR_MBA_LINEAR' to set MBA feature HW info.
      (suggested by Chao Peng)
v1:
    - sort 'PSR_INFO_IDX_' macros as feature.
      (suggested by Chao Peng)
    - rename 'PSR_INFO_IDX_MBA_LINEAR' to 'PSR_INFO_IDX_MBA_FLAG'.
    - rename 'linear' in 'struct mba_info' to 'flags' for future extension.
      (suggested by Chao Peng)
---
 xen/arch/x86/psr.c          | 14 +++++++++++++-
 xen/arch/x86/sysctl.c       | 21 ++++++++++++++++++++-
 xen/include/asm-x86/psr.h   |  2 ++
 xen/include/public/sysctl.h |  8 ++++++++
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 157e11f..03f24c0 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -263,6 +263,10 @@ static enum psr_feat_type psr_type_to_feat_type(enum psr_type type)
         feat_type = FEAT_TYPE_L2_CAT;
         break;
 
+    case PSR_TYPE_MBA_THRTL:
+        feat_type = FEAT_TYPE_MBA;
+        break;
+
     default:
         ASSERT_UNREACHABLE();
     }
@@ -483,7 +487,15 @@ static const struct feat_props l2_cat_props = {
 static bool mba_get_feat_info(const struct feat_node *feat,
                               uint32_t data[], unsigned int array_len)
 {
-    return false;
+    ASSERT(array_len == PSR_INFO_ARRAY_SIZE);
+
+    data[PSR_INFO_IDX_COS_MAX] = feat->cos_max;
+    data[PSR_INFO_IDX_MBA_THRTL_MAX] = feat->mba.thrtl_max;
+
+    if ( feat->mba.linear )
+        data[PSR_INFO_IDX_MBA_FLAG] |= XEN_SYSCTL_PSR_MBA_LINEAR;
+
+    return true;
 }
 
 static void mba_write_msr(unsigned int cos, uint32_t val,
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 6867ee1..f48d6fd 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -174,7 +174,7 @@ long arch_do_sysctl(
     case XEN_SYSCTL_psr_alloc:
         switch ( sysctl->u.psr_alloc.cmd )
         {
-            uint32_t data[PSR_INFO_ARRAY_SIZE];
+            uint32_t data[PSR_INFO_ARRAY_SIZE] = { };
 
         case XEN_SYSCTL_PSR_get_l3_info:
         {
@@ -214,6 +214,25 @@ long arch_do_sysctl(
             break;
         }
 
+        case XEN_SYSCTL_PSR_get_mba_info:
+        {
+            ret = psr_get_info(sysctl->u.psr_alloc.target,
+                               PSR_TYPE_MBA_THRTL, data, ARRAY_SIZE(data));
+            if ( ret )
+                break;
+
+            sysctl->u.psr_alloc.u.mba_info.cos_max =
+                                      data[PSR_INFO_IDX_COS_MAX];
+            sysctl->u.psr_alloc.u.mba_info.thrtl_max =
+                                      data[PSR_INFO_IDX_MBA_THRTL_MAX];
+            sysctl->u.psr_alloc.u.mba_info.flags =
+                                      data[PSR_INFO_IDX_MBA_FLAG];
+
+            if ( __copy_field_to_guest(u_sysctl, sysctl, u.psr_alloc) )
+                ret = -EFAULT;
+            break;
+        }
+
         default:
             ret = -EOPNOTSUPP;
             break;
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index 9d14264..084ae97 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -39,6 +39,8 @@
 #define PSR_INFO_IDX_COS_MAX            0
 #define PSR_INFO_IDX_CAT_CBM_LEN        1
 #define PSR_INFO_IDX_CAT_FLAG           2
+#define PSR_INFO_IDX_MBA_THRTL_MAX      1
+#define PSR_INFO_IDX_MBA_FLAG           2
 #define PSR_INFO_ARRAY_SIZE             3
 
 struct psr_cmt_l3 {
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index a50e345..f7f26c3 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -698,6 +698,7 @@ struct xen_sysctl_pcitopoinfo {
 
 #define XEN_SYSCTL_PSR_get_l3_info               0
 #define XEN_SYSCTL_PSR_get_l2_info               1
+#define XEN_SYSCTL_PSR_get_mba_info              2
 struct xen_sysctl_psr_alloc {
     uint32_t cmd;       /* IN: XEN_SYSCTL_PSR_* */
     uint32_t target;    /* IN */
@@ -708,6 +709,13 @@ struct xen_sysctl_psr_alloc {
 #define XEN_SYSCTL_PSR_CAT_L3_CDP       (1u << 0)
             uint32_t flags;     /* OUT: CAT flags */
         } cat_info;
+
+        struct {
+            uint32_t thrtl_max; /* OUT: Maximum throttle */
+            uint32_t cos_max;   /* OUT: Maximum COS */
+#define XEN_SYSCTL_PSR_MBA_LINEAR      (1u << 0)
+            uint32_t flags;     /* OUT: MBA flags */
+        } mba_info;
     } u;
 };
 
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 07/16] x86: implement get value interface for MBA
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (5 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 06/16] x86: implement get hw info " Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-08  7:23 ` [PATCH v6 08/16] x86: implement set value flow " Yi Sun
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Yi Sun, Andrew Cooper, Jan Beulich, Chao Peng,
	Roger Pau Monné

This patch implements get value domctl interface for MBA.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v5:
    - use newly defined macro to get MBA thrtl.
      (suggested by Roger Pau Monné)
v4:
    - remove 'ALLOC_' from macro names.
      (suggested by Roger Pau Monné)
v3:
    - change 'PSR_VAL_TYPE_MBA' to 'PSR_TYPE_MBA_THRTL'.
      (suggested by Roger Pau Monné)
---
 xen/arch/x86/domctl.c       | 4 ++++
 xen/include/public/domctl.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 0cd18a6..17fd3ad 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1491,6 +1491,10 @@ long arch_do_domctl(
             ret = domctl_psr_get_val(d, domctl, PSR_TYPE_L2_CBM, copyback);
             break;
 
+        case XEN_DOMCTL_PSR_GET_MBA_THRTL:
+            ret = domctl_psr_get_val(d, domctl, PSR_TYPE_MBA_THRTL, copyback);
+            break;
+
         default:
             ret = -EOPNOTSUPP;
             break;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index c099334..e8f4c4c 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1069,6 +1069,7 @@ struct xen_domctl_psr_alloc {
 #define XEN_DOMCTL_PSR_GET_L3_DATA    5
 #define XEN_DOMCTL_PSR_SET_L2_CBM     6
 #define XEN_DOMCTL_PSR_GET_L2_CBM     7
+#define XEN_DOMCTL_PSR_GET_MBA_THRTL  9
     uint32_t cmd;       /* IN: XEN_DOMCTL_PSR_* */
     uint32_t target;    /* IN */
     uint64_t data;      /* IN/OUT */
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 08/16] x86: implement set value flow for MBA
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (6 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 07/16] x86: implement get value interface " Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-11 13:38   ` Jan Beulich
  2017-10-08  7:23 ` [PATCH v6 09/16] tools: create general interfaces to support psr allocation features Yi Sun
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Yi Sun, Andrew Cooper, Jan Beulich, Chao Peng,
	Roger Pau Monné

This patch implements set value flow for MBA including its callback
function and domctl interface.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v6:
    - split co-exist features' values setting flow to a new patch.
      (suggested by Jan Beulich)
    - restore codes related to 'mba_check_thrtl' and 'check_value'.
      (suggested by Jan Beulich)
v5:
    - adjust position of 'cat_check_cbm' to not to make changes so big.
      (suggested by Roger Pau Monné)
    - remove 'props' from 'struct cos_write_info'.
      (suggested by Roger Pau Monné)
    - make a single return statement in 'mba_check_thrtl'.
      (suggested by Jan Beulich)
v4:
    - remove 'ALLOC_' from macro names.
      (suggested by Roger Pau Monné)
    - join two checks into a single if.
      (suggested by Roger Pau Monné)
    - remove redundant local variable 'array_len'.
      (suggested by Roger Pau Monné)
v3:
    - modify commit message to make it clear.
      (suggested by Roger Pau Monné)
    - modify functionality of 'check_val' to make it simple to only check value.
      Change the last parameter type from 'unsigned long *' to 'unsigned long'.
      (suggested by Roger Pau Monné)
    - call rdmsrl to get value just written into MSR for MBA. Because HW can
      automatically change input value to what it wants.
      (suggested by Roger Pau Monné)
    - change type of 'write_msr' to 'uint32_t' to return the value actually
      written into MSR. Then, change 'do_write_psr_msrs' to set the returned
      value into 'cos_reg_val[]'
    - move the declaration of 'j' into loop in 'do_write_psr_msrs'.
      (suggested by Roger Pau Monné)
    - change 'mba_info' to 'mba'.
      (suggested by Roger Pau Monné)
    - change 'cat_info' to 'cat'.
      (suggested by Roger Pau Monné)
    - rename 'psr_cat/PSR_CAT' to 'psr_alloc/PSR_ALLOC' and remove 'op/OP'
      from name.
      (suggested by Roger Pau Monné)
    - change 'PSR_VAL_TYPE_MBA' to 'PSR_TYPE_MBA_THRTL'.
      (suggested by Roger Pau Monné)
v2:
    - remove linear mode 'thrtl_max' check in 'mba_check_thrtl' because it has
      been checked in 'mba_init_feature'.
      (suggested by Chao Peng)
    - for non-linear mode, check if '*thrtl' is not 0 in 'mba_check_thrtl'. If
      it is 0, we do not need to change it.
      (suggested by Chao Peng)
    - move comments to explain changes of 'cos_write_info' from psr.c to commit
      message.
      (suggested by Chao Peng)
---
 xen/arch/x86/domctl.c       |  6 ++++
 xen/arch/x86/psr.c          | 74 +++++++++++++++++++++++++++++++++++++--------
 xen/include/public/domctl.h |  1 +
 3 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 17fd3ad..bbfd76e 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1475,6 +1475,12 @@ long arch_do_domctl(
                               PSR_TYPE_L2_CBM);
             break;
 
+        case XEN_DOMCTL_PSR_SET_MBA_THRTL:
+            ret = psr_set_val(d, domctl->u.psr_alloc.target,
+                              domctl->u.psr_alloc.data,
+                              PSR_TYPE_MBA_THRTL);
+            break;
+
         case XEN_DOMCTL_PSR_GET_L3_CBM:
             ret = domctl_psr_get_val(d, domctl, PSR_TYPE_L3_CBM, copyback);
             break;
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 03f24c0..cffb377 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -138,6 +138,12 @@ static const struct feat_props {
 
     /* write_msr is used to write out feature MSR register. */
     void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
+
+    /*
+     * check_val is used to check if input val fulfills SDM requirement.
+     * Change it to valid value if SDM allows.
+     */
+    bool (*check_val)(const struct feat_node *feat, unsigned long *val);
 } *feat_props[FEAT_TYPE_NUM];
 
 /*
@@ -274,30 +280,30 @@ static enum psr_feat_type psr_type_to_feat_type(enum psr_type type)
     return feat_type;
 }
 
-static bool psr_check_cbm(unsigned int cbm_len, unsigned long cbm)
+/* Implementation of allocation features' functions. */
+static bool cat_check_cbm(const struct feat_node *feat, unsigned long *cbm)
 {
     unsigned int first_bit, zero_bit;
+    unsigned int cbm_len = feat->cat.cbm_len;
 
-    /* Set bits should only in the range of [0, cbm_len]. */
-    if ( cbm & (~0ul << cbm_len) )
-        return false;
-
-    /* At least one bit need to be set. */
-    if ( cbm == 0 )
+    /*
+     * Set bits should only in the range of [0, cbm_len].
+     * And, at least one bit need to be set.
+     */
+    if ( *cbm & (~0ul << cbm_len) || *cbm == 0 )
         return false;
 
-    first_bit = find_first_bit(&cbm, cbm_len);
-    zero_bit = find_next_zero_bit(&cbm, cbm_len, first_bit);
+    first_bit = find_first_bit(cbm, cbm_len);
+    zero_bit = find_next_zero_bit(cbm, cbm_len, first_bit);
 
     /* Set bits should be contiguous. */
     if ( zero_bit < cbm_len &&
-         find_next_bit(&cbm, cbm_len, zero_bit) < cbm_len )
+         find_next_bit(cbm, cbm_len, zero_bit) < cbm_len )
         return false;
 
     return true;
 }
 
-/* Implementation of allocation features' functions. */
 static bool cat_init_feature(const struct cpuid_leaf *regs,
                              struct feat_node *feat,
                              struct psr_socket_info *info,
@@ -436,6 +442,7 @@ static const struct feat_props l3_cat_props = {
     .alt_type = PSR_TYPE_UNKNOWN,
     .get_feat_info = cat_get_feat_info,
     .write_msr = l3_cat_write_msr,
+    .check_val = cat_check_cbm,
 };
 
 /* L3 CDP props */
@@ -466,6 +473,7 @@ static const struct feat_props l3_cdp_props = {
     .alt_type = PSR_TYPE_L3_CBM,
     .get_feat_info = l3_cdp_get_feat_info,
     .write_msr = l3_cdp_write_msr,
+    .check_val = cat_check_cbm,
 };
 
 /* L2 CAT props */
@@ -481,6 +489,7 @@ static const struct feat_props l2_cat_props = {
     .alt_type = PSR_TYPE_UNKNOWN,
     .get_feat_info = cat_get_feat_info,
     .write_msr = l2_cat_write_msr,
+    .check_val = cat_check_cbm,
 };
 
 /* MBA props */
@@ -501,6 +510,43 @@ static bool mba_get_feat_info(const struct feat_node *feat,
 static void mba_write_msr(unsigned int cos, uint32_t val,
                           enum psr_type type)
 {
+    wrmsrl(MSR_IA32_PSR_MBA_MASK(cos), val);
+}
+
+static bool mba_check_thrtl(const struct feat_node *feat, unsigned long *thrtl)
+{
+    if ( *thrtl > feat->mba.thrtl_max )
+        return false;
+
+    /*
+     * Per SDM (chapter "Memory Bandwidth Allocation Configuration"):
+     * 1. Linear mode: In the linear mode the input precision is defined
+     *    as 100-(MBA_MAX). For instance, if the MBA_MAX value is 90, the
+     *    input precision is 10%. Values not an even multiple of the
+     *    precision (e.g., 12%) will be rounded down (e.g., to 10% delay
+     *    applied).
+     * 2. Non-linear mode: Input delay values are powers-of-two from zero
+     *    to the MBA_MAX value from CPUID. In this case any values not a
+     *    power of two will be rounded down the next nearest power of two.
+     */
+    if ( feat->mba.linear )
+    {
+        unsigned int mod;
+
+        if ( feat->mba.thrtl_max >= 100 )
+            return false;
+
+        mod = *thrtl % (100 - feat->mba.thrtl_max);
+        *thrtl -= mod;
+    }
+    else
+    {
+        /* Not power of 2. */
+        if ( *thrtl & (*thrtl - 1) )
+            *thrtl = *thrtl & (1 << (flsl(*thrtl) - 1));
+    }
+
+    return true;
 }
 
 static const struct feat_props mba_props = {
@@ -509,6 +555,7 @@ static const struct feat_props mba_props = {
     .alt_type = PSR_TYPE_UNKNOWN,
     .get_feat_info = mba_get_feat_info,
     .write_msr = mba_write_msr,
+    .check_val = mba_check_thrtl,
 };
 
 static bool __init parse_psr_bool(const char *s, const char *delim,
@@ -950,6 +997,7 @@ static int insert_val_into_array(uint32_t val[],
     const struct feat_node *feat;
     const struct feat_props *props;
     unsigned int i;
+    unsigned long check_val = new_val;
     int ret;
 
     ASSERT(feat_type < FEAT_TYPE_NUM);
@@ -974,9 +1022,11 @@ static int insert_val_into_array(uint32_t val[],
     if ( array_len < props->cos_num )
         return -ENOSPC;
 
-    if ( !psr_check_cbm(feat->cat.cbm_len, new_val) )
+    if ( !props->check_val(feat, &check_val) )
         return -EINVAL;
 
+    new_val = check_val;
+
     /*
      * Value setting position is same as feature array.
      * For CDP, user may set both DATA and CODE to same value. For such case,
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index e8f4c4c..fb57e64 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1069,6 +1069,7 @@ struct xen_domctl_psr_alloc {
 #define XEN_DOMCTL_PSR_GET_L3_DATA    5
 #define XEN_DOMCTL_PSR_SET_L2_CBM     6
 #define XEN_DOMCTL_PSR_GET_L2_CBM     7
+#define XEN_DOMCTL_PSR_SET_MBA_THRTL  8
 #define XEN_DOMCTL_PSR_GET_MBA_THRTL  9
     uint32_t cmd;       /* IN: XEN_DOMCTL_PSR_* */
     uint32_t target;    /* IN */
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 09/16] tools: create general interfaces to support psr allocation features
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (7 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 08/16] x86: implement set value flow " Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-08  7:23 ` [PATCH v6 10/16] tools: implement the new libxc get hw info interface Yi Sun
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Chao Peng, Yi Sun, Ian Jackson, Roger Pau Monné

This patch creates general interfaces in libxl to support all psr
allocation features.

Add 'LIBXL_HAVE_PSR_GENERIC' to indicate interface change.

Please note, the functionality cannot work until later patches
are applied.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v5:
    - adjust parameters position in 'libxl_psr_get_hw_info'.
      (suggested by Roger Pau Monné)
v4:
    - add description for LIBXL_HAVE_PSR_GENERIC to mention newly added
      public functions.
      (suggested by Roger Pau Monné)
v3:
    - change 'LIBXL_HAVE_PSR_MBA' to 'LIBXL_HAVE_PSR_GENERIC'.
      (suggested by Roger Pau Monné)
    - 'typedef enum libxl_psr_cbm_type libxl_psr_type;' in libxl.h.
      (suggested by Roger Pau Monné and Wei Liu)
    - change 'libxl_psr_cbm_type' to 'libxl_psr_type' in newly defined
      interfaces.
      (suggested by Roger Pau Monné)
v2:
    - remove '_INFO' in 'libxl_psr_feat_type' and make corresponding
      changes in 'libxl_psr_hw_info'.
      (suggested by Chao Peng)
---
 tools/libxl/libxl.h         | 37 +++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_psr.c     | 25 +++++++++++++++++++++++++
 tools/libxl/libxl_types.idl | 22 ++++++++++++++++++++++
 3 files changed, 84 insertions(+)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 827272e..0d2dee8 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -967,6 +967,17 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
 #define LIBXL_HAVE_PSR_L2_CAT 1
 
 /*
+ * LIBXL_HAVE_PSR_GENERIC
+ *
+ * If this is defined, the Memory Bandwidth Allocation feature is supported.
+ * The following public functions are available:
+ *   libxl_psr_{set/get}_val
+ *   libxl_psr_get_hw_info
+ *   libxl_psr_hw_info_list_free
+ */
+#define LIBXL_HAVE_PSR_GENERIC 1
+
+/*
  * LIBXL_HAVE_MCA_CAPS
  *
  * If this is defined, setting MCA capabilities for HVM domain is supported.
@@ -2287,6 +2298,32 @@ int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
 int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
                               int *nr);
 void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr);
+
+typedef enum libxl_psr_cbm_type libxl_psr_type;
+
+/*
+ * Function to set a domain's value. It operates on a single or multiple
+ * target(s) defined in 'target_map'. 'target_map' specifies all the sockets
+ * to be operated on.
+ */
+int libxl_psr_set_val(libxl_ctx *ctx, uint32_t domid,
+                      libxl_psr_type type, libxl_bitmap *target_map,
+                      uint64_t val);
+/*
+ * Function to get a domain's cbm. It operates on a single 'target'.
+ * 'target' specifies which socket to be operated on.
+ */
+int libxl_psr_get_val(libxl_ctx *ctx, uint32_t domid,
+                      libxl_psr_type type, unsigned int target,
+                      uint64_t *val);
+/*
+ * On success, the function returns an array of elements in 'info',
+ * and the length in 'nr'.
+ */
+int libxl_psr_get_hw_info(libxl_ctx *ctx, libxl_psr_feat_type type,
+                          unsigned int lvl, unsigned int *nr,
+                          libxl_psr_hw_info **info);
+void libxl_psr_hw_info_list_free(libxl_psr_hw_info *list, unsigned int nr);
 #endif
 
 /* misc */
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 197505a..d4f5f67 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -428,6 +428,31 @@ void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr)
     free(list);
 }
 
+int libxl_psr_set_val(libxl_ctx *ctx, uint32_t domid,
+                      libxl_psr_type type, libxl_bitmap *target_map,
+                      uint64_t val)
+{
+    return ERROR_FAIL;
+}
+
+int libxl_psr_get_val(libxl_ctx *ctx, uint32_t domid,
+                      libxl_psr_type type, unsigned int target,
+                      uint64_t *val)
+{
+    return ERROR_FAIL;
+}
+
+int libxl_psr_get_hw_info(libxl_ctx *ctx, libxl_psr_feat_type type,
+                          unsigned int lvl, unsigned int *nr,
+                          libxl_psr_hw_info **info)
+{
+    return ERROR_FAIL;
+}
+
+void libxl_psr_hw_info_list_free(libxl_psr_hw_info *list, unsigned int nr)
+{
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index c2a1141..6f53b2d 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -1025,6 +1025,7 @@ libxl_psr_cbm_type = Enumeration("psr_cbm_type", [
     (2, "L3_CBM_CODE"),
     (3, "L3_CBM_DATA"),
     (4, "L2_CBM"),
+    (5, "MBA_THRTL"),
     ])
 
 libxl_psr_cat_info = Struct("psr_cat_info", [
@@ -1033,3 +1034,24 @@ libxl_psr_cat_info = Struct("psr_cat_info", [
     ("cbm_len", uint32),
     ("cdp_enabled", bool),
     ])
+
+libxl_psr_feat_type = Enumeration("psr_feat_type", [
+    (1, "CAT"),
+    (2, "MBA"),
+    ])
+
+libxl_psr_hw_info = Struct("psr_hw_info", [
+    ("id", uint32),
+    ("u", KeyedUnion(None, libxl_psr_feat_type, "type",
+          [("cat", Struct(None, [
+                                    ("cos_max",     uint32),
+                                    ("cbm_len",     uint32),
+                                    ("cdp_enabled", bool),
+                               ])),
+           ("mba", Struct(None, [
+                                    ("cos_max",     uint32),
+                                    ("thrtl_max",   uint32),
+                                    ("linear",      bool),
+                               ])),
+          ]))
+    ], dir=DIR_OUT)
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 10/16] tools: implement the new libxc get hw info interface
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (8 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 09/16] tools: create general interfaces to support psr allocation features Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-08  7:23 ` [PATCH v6 11/16] tools: implement the new libxl " Yi Sun
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Chao Peng, Yi Sun, Ian Jackson, Roger Pau Monné

This patch implements a new libxc get hw info interface and corresponding
data structures. It also changes libxl_psr.c to call this new interface.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v6:
    - remove unnecessary spaces in brackets.
      (suggested by Wei Liu)
    - use assert to check input lvl.
      (suggested by Roger Pau Monné)
v5:
    - directly define 'xc_psr_hw_info' as union type.
      (suggested by Roger Pau Monné)
    - converge L2 and L3 cases in 'xc_psr_get_hw_info'.
      (suggested by Roger Pau Monné)
v4:
    - remove 'ALLOC_' from macro names.
      (suggested by Roger Pau Monné)
    - remove 'XC_PSR_FEAT_UNKNOWN' which is not necessary.
      (suggested by Roger Pau Monné)
    - remove 'FEAT_' from enum item names.
      (suggested by Roger Pau Monné)
    - remove 'xc_' from struct name.
      (suggested by Roger Pau Monné)
    - adjust codes to reduce indentation.
      (suggested by Roger Pau Monné)
    - assert for not happened case.
      (suggested by Roger Pau Monné)
    - add LOGE to show errno.
      (suggested by Roger Pau Monné)
v3:
    - rename 'psr_cat/PSR_CAT' to 'psr_alloc/PSR_ALLOC' and remove 'op/OP'
      from name.
      (suggested by Roger Pau Monné)
    - remove 'info' from 'xc_cat_info' and 'xc_mba_info'.
      (suggested by Roger Pau Monné)
    - set errno in 'xc_psr_get_hw_info'.
      (suggested by Roger Pau Monné)
    - remove 'inline'.
      (suggested by Roger Pau Monné)
    - remove 'psr' from 'libxl__psr_feat_type_to_libxc_psr_feat_type' to make
      function name shorter.
      (suggested by Roger Pau Monné)
    - check 'xc_type' in 'libxl_psr_cat_get_info'.
      (suggested by Roger Pau Monné)
v2:
    - split this patch out from a big patch in v1.
      (suggested by Wei Liu)
    - change 'CAT_INFO' and 'MBA_INFO' to 'CAT' and 'MBA'.
      (suggested by Chao Peng)
---
 tools/libxc/include/xenctrl.h | 27 ++++++++++++++++++---
 tools/libxc/xc_psr.c          | 55 +++++++++++++++++++++++++++----------------
 tools/libxl/libxl_psr.c       | 38 ++++++++++++++++++++++++++++--
 3 files changed, 95 insertions(+), 25 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index b970905..2d977c8 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2474,6 +2474,28 @@ enum xc_psr_cat_type {
 };
 typedef enum xc_psr_cat_type xc_psr_cat_type;
 
+enum xc_psr_feat_type {
+    XC_PSR_CAT_L3,
+    XC_PSR_CAT_L2,
+    XC_PSR_MBA,
+};
+typedef enum xc_psr_feat_type xc_psr_feat_type;
+
+union xc_psr_hw_info {
+    struct {
+        uint32_t cos_max;
+        uint32_t cbm_len;
+        bool     cdp_enabled;
+    } cat;
+
+    struct {
+        uint32_t cos_max;
+        uint32_t thrtl_max;
+        bool     linear;
+    } mba;
+};
+typedef union xc_psr_hw_info xc_psr_hw_info;
+
 int xc_psr_cmt_attach(xc_interface *xch, uint32_t domid);
 int xc_psr_cmt_detach(xc_interface *xch, uint32_t domid);
 int xc_psr_cmt_get_domain_rmid(xc_interface *xch, uint32_t domid,
@@ -2495,9 +2517,8 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
 int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
                                xc_psr_cat_type type, uint32_t target,
                                uint64_t *data);
-int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, unsigned int lvl,
-                        uint32_t *cos_max, uint32_t *cbm_len,
-                        bool *cdp_enabled);
+int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
+                       xc_psr_feat_type type, xc_psr_hw_info *hw_info);
 
 int xc_get_cpu_levelling_caps(xc_interface *xch, uint32_t *caps);
 int xc_get_cpu_featureset(xc_interface *xch, uint32_t index,
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 5c54a35..2c605a7 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -323,37 +323,52 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
     return rc;
 }
 
-int xc_psr_cat_get_info(xc_interface *xch, uint32_t socket, unsigned int lvl,
-                        uint32_t *cos_max, uint32_t *cbm_len, bool *cdp_enabled)
+int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
+                       xc_psr_feat_type type, xc_psr_hw_info *hw_info)
 {
     int rc = -1;
     DECLARE_SYSCTL;
 
+    if ( !hw_info )
+    {
+        errno = EINVAL;
+        return rc;
+    }
+
     sysctl.cmd = XEN_SYSCTL_psr_alloc;
     sysctl.u.psr_alloc.target = socket;
 
-    switch ( lvl )
+    switch ( type )
     {
-    case 2:
-        sysctl.u.psr_alloc.cmd = XEN_SYSCTL_PSR_get_l2_info;
+    case XC_PSR_CAT_L2:
+    case XC_PSR_CAT_L3:
+        sysctl.u.psr_alloc.cmd = (type == XC_PSR_CAT_L2) ?
+                                 XEN_SYSCTL_PSR_get_l2_info :
+                                 XEN_SYSCTL_PSR_get_l3_info;
+
         rc = xc_sysctl(xch, &sysctl);
-        if ( !rc )
-        {
-            *cos_max = sysctl.u.psr_alloc.u.cat_info.cos_max;
-            *cbm_len = sysctl.u.psr_alloc.u.cat_info.cbm_len;
-            *cdp_enabled = false;
-        }
+        if ( rc )
+            break;
+
+        hw_info->cat.cos_max = sysctl.u.psr_alloc.u.cat_info.cos_max;
+        hw_info->cat.cbm_len = sysctl.u.psr_alloc.u.cat_info.cbm_len;
+        hw_info->cat.cdp_enabled = (type == XC_PSR_CAT_L2) ?
+                                   false :
+                                   (sysctl.u.psr_alloc.u.cat_info.flags &
+                                    XEN_SYSCTL_PSR_CAT_L3_CDP);
+
         break;
-    case 3:
-        sysctl.u.psr_alloc.cmd = XEN_SYSCTL_PSR_get_l3_info;
+    case XC_PSR_MBA:
+        sysctl.u.psr_alloc.cmd = XEN_SYSCTL_PSR_get_mba_info;
         rc = xc_sysctl(xch, &sysctl);
-        if ( !rc )
-        {
-            *cos_max = sysctl.u.psr_alloc.u.cat_info.cos_max;
-            *cbm_len = sysctl.u.psr_alloc.u.cat_info.cbm_len;
-            *cdp_enabled = sysctl.u.psr_alloc.u.cat_info.flags &
-                           XEN_SYSCTL_PSR_CAT_L3_CDP;
-        }
+        if ( rc )
+            break;
+
+        hw_info->mba.cos_max = sysctl.u.psr_alloc.u.mba_info.cos_max;
+        hw_info->mba.thrtl_max = sysctl.u.psr_alloc.u.mba_info.thrtl_max;
+        hw_info->mba.linear = sysctl.u.psr_alloc.u.mba_info.flags &
+                              XEN_SYSCTL_PSR_MBA_LINEAR;
+
         break;
     default:
         errno = EOPNOTSUPP;
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index d4f5f67..e1cc250 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -361,6 +361,31 @@ int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
     return rc;
 }
 
+static xc_psr_feat_type libxl__feat_type_to_libxc_feat_type(
+                            libxl_psr_feat_type type, unsigned int lvl)
+{
+    xc_psr_feat_type xc_type;
+
+    switch (type) {
+    case LIBXL_PSR_FEAT_TYPE_CAT:
+        assert(lvl == 3 || lvl == 2);
+
+        if (lvl == 3)
+            xc_type = XC_PSR_CAT_L3;
+        if (lvl == 2)
+            xc_type = XC_PSR_CAT_L2;
+        break;
+    case LIBXL_PSR_FEAT_TYPE_MBA:
+        xc_type = XC_PSR_MBA;
+        break;
+    default:
+        /* Could not happen */
+        assert(0);
+    }
+
+    return xc_type;
+}
+
 int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
                            unsigned int *nr, unsigned int lvl)
 {
@@ -369,6 +394,8 @@ int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
     int i = 0, socketid, nr_sockets;
     libxl_bitmap socketmap;
     libxl_psr_cat_info *ptr;
+    xc_psr_hw_info hw_info;
+    xc_psr_feat_type xc_type;
 
     libxl_bitmap_init(&socketmap);
 
@@ -385,16 +412,23 @@ int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
         goto out;
     }
 
+    xc_type = libxl__feat_type_to_libxc_feat_type(LIBXL_PSR_FEAT_TYPE_CAT, lvl);
+
     ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_cat_info));
 
     libxl_for_each_set_bit(socketid, socketmap) {
         ptr[i].id = socketid;
-        if (xc_psr_cat_get_info(ctx->xch, socketid, lvl, &ptr[i].cos_max,
-                                &ptr[i].cbm_len, &ptr[i].cdp_enabled)) {
+        if (xc_psr_get_hw_info(ctx->xch, socketid, xc_type, &hw_info)) {
+            LOGE(ERROR, "failed to get hw info");
             rc = ERROR_FAIL;
             free(ptr);
             goto out;
         }
+
+        ptr[i].cos_max = hw_info.cat.cos_max;
+        ptr[i].cbm_len = hw_info.cat.cbm_len;
+        ptr[i].cdp_enabled = hw_info.cat.cdp_enabled;
+
         i++;
     }
 
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 11/16] tools: implement the new libxl get hw info interface
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (9 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 10/16] tools: implement the new libxc get hw info interface Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-08  7:23 ` [PATCH v6 12/16] tools: implement the new xl " Yi Sun
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Chao Peng, Yi Sun, Ian Jackson, Roger Pau Monné

This patch implements the new libxl get hw info interface,
'libxl_psr_get_hw_info', which is suitable to all psr allocation
features. It also implements corresponding list free function,
'libxl_psr_hw_info_list_free' and makes 'libxl_psr_cat_get_info' call
'libxl_psr_get_hw_info' to avoid redundant code in libxl_psr.c.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v5:
    - change 'if (rc < 0)' to 'if (rc)'.
      (suggested by Roger Pau Monné)
v4:
    - remove 'xc_' from struct name.
      (suggested by Roger Pau Monné)
    - fix words in commit message.
      (suggested by Roger Pau Monné)
    - change type of 'libxl__hw_info_to_libxl_cat_info' to void and use
      assert to check invalid type. Then, remove check for
      'libxl__hw_info_to_libxl_cat_info'.
      (suggested by Roger Pau Monné)
    - change type of 'libxl__xc_hw_info_to_libxl_hw_info' to void and use
      assert to check invalid type. Then, remove check for
      'libxl__xc_hw_info_to_libxl_hw_info'.
      (suggested by Roger Pau Monné)
v3:
    - remove casting.
      (suggested by Roger Pau Monné)
    - remove inline.
      (suggested by Roger Pau Monné)
    - change 'libxc__psr_hw_info_to_libxl_psr_hw_info' to
      'libxl__xc_hw_info_to_libxl_hw_info'.
      (suggested by Roger Pau Monné)
    - remove '_hw' from parameter names.
      (suggested by Roger Pau Monné)
    - change some 'LOGE' to 'LOG'.
      (suggested by Roger Pau Monné)
    - check returned 'xc_type' and remove redundant 'lvl' check.
      (suggested by Roger Pau Monné)
v2:
    - split this patch out from a big patch in v1.
      (suggested by Wei Liu)
    - change 'CAT_INFO'/'MBA_INFO' to 'CAT' and 'MBA. Also the libxl structure
      name 'cat_info'/'mba_info' is changed to 'cat'/'mba'.
      (suggested by Chao Peng)
    - call 'libxl_psr_hw_info_list_free' in 'libxl_psr_cat_get_info' to free
      allocated resources.
      (suggested by Chao Peng)
---
 tools/libxl/libxl_psr.c | 131 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 93 insertions(+), 38 deletions(-)

diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index e1cc250..b053abd 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -386,56 +386,41 @@ static xc_psr_feat_type libxl__feat_type_to_libxc_feat_type(
     return xc_type;
 }
 
+static void libxl__hw_info_to_libxl_cat_info(
+                libxl_psr_feat_type type, libxl_psr_hw_info *hw_info,
+                libxl_psr_cat_info *cat_info)
+{
+    assert(type == LIBXL_PSR_FEAT_TYPE_CAT);
+
+    cat_info->id = hw_info->id;
+    cat_info->cos_max = hw_info->u.cat.cos_max;
+    cat_info->cbm_len = hw_info->u.cat.cbm_len;
+    cat_info->cdp_enabled = hw_info->u.cat.cdp_enabled;
+}
+
 int libxl_psr_cat_get_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
                            unsigned int *nr, unsigned int lvl)
 {
     GC_INIT(ctx);
     int rc;
-    int i = 0, socketid, nr_sockets;
-    libxl_bitmap socketmap;
+    unsigned int i;
+    libxl_psr_hw_info *hw_info;
     libxl_psr_cat_info *ptr;
-    xc_psr_hw_info hw_info;
-    xc_psr_feat_type xc_type;
-
-    libxl_bitmap_init(&socketmap);
-
-    rc = libxl__count_physical_sockets(gc, &nr_sockets);
-    if (rc) {
-        LOGE(ERROR, "failed to get system socket count");
-        goto out;
-    }
 
-    libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets);
-    rc = libxl_get_online_socketmap(ctx, &socketmap);
-    if (rc < 0) {
-        LOGE(ERROR, "failed to get available sockets");
+    rc = libxl_psr_get_hw_info(ctx, LIBXL_PSR_FEAT_TYPE_CAT, lvl, nr, &hw_info);
+    if (rc)
         goto out;
-    }
-
-    xc_type = libxl__feat_type_to_libxc_feat_type(LIBXL_PSR_FEAT_TYPE_CAT, lvl);
-
-    ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_cat_info));
-
-    libxl_for_each_set_bit(socketid, socketmap) {
-        ptr[i].id = socketid;
-        if (xc_psr_get_hw_info(ctx->xch, socketid, xc_type, &hw_info)) {
-            LOGE(ERROR, "failed to get hw info");
-            rc = ERROR_FAIL;
-            free(ptr);
-            goto out;
-        }
 
-        ptr[i].cos_max = hw_info.cat.cos_max;
-        ptr[i].cbm_len = hw_info.cat.cbm_len;
-        ptr[i].cdp_enabled = hw_info.cat.cdp_enabled;
+    ptr = libxl__malloc(NOGC, *nr * sizeof(libxl_psr_cat_info));
 
-        i++;
-    }
+    for (i = 0; i < *nr; i++)
+        libxl__hw_info_to_libxl_cat_info(LIBXL_PSR_FEAT_TYPE_CAT,
+                                         &hw_info[i],
+                                         &ptr[i]);
 
     *info = ptr;
-    *nr = i;
+    libxl_psr_hw_info_list_free(hw_info, *nr);
 out:
-    libxl_bitmap_dispose(&socketmap);
     GC_FREE;
     return rc;
 }
@@ -476,15 +461,85 @@ int libxl_psr_get_val(libxl_ctx *ctx, uint32_t domid,
     return ERROR_FAIL;
 }
 
+static void libxl__xc_hw_info_to_libxl_hw_info(
+                libxl_psr_feat_type type, xc_psr_hw_info *xc_info,
+                libxl_psr_hw_info *xl_info)
+{
+    switch (type) {
+    case LIBXL_PSR_FEAT_TYPE_CAT:
+        xl_info->u.cat.cos_max = xc_info->cat.cos_max;
+        xl_info->u.cat.cbm_len = xc_info->cat.cbm_len;
+        xl_info->u.cat.cdp_enabled = xc_info->cat.cdp_enabled;
+        break;
+    case LIBXL_PSR_FEAT_TYPE_MBA:
+        xl_info->u.mba.cos_max = xc_info->mba.cos_max;
+        xl_info->u.mba.thrtl_max = xc_info->mba.thrtl_max;
+        xl_info->u.mba.linear = xc_info->mba.linear;
+        break;
+    default:
+        assert(0);
+    }
+}
+
 int libxl_psr_get_hw_info(libxl_ctx *ctx, libxl_psr_feat_type type,
                           unsigned int lvl, unsigned int *nr,
                           libxl_psr_hw_info **info)
 {
-    return ERROR_FAIL;
+    GC_INIT(ctx);
+    int rc, nr_sockets;
+    unsigned int i = 0, socketid;
+    libxl_bitmap socketmap;
+    libxl_psr_hw_info *ptr;
+    xc_psr_feat_type xc_type;
+    xc_psr_hw_info hw_info;
+
+    libxl_bitmap_init(&socketmap);
+
+    xc_type = libxl__feat_type_to_libxc_feat_type(type, lvl);
+
+    rc = libxl__count_physical_sockets(gc, &nr_sockets);
+    if (rc) {
+        LOG(ERROR, "failed to get system socket count");
+        goto out;
+    }
+
+    libxl_socket_bitmap_alloc(ctx, &socketmap, nr_sockets);
+    rc = libxl_get_online_socketmap(ctx, &socketmap);
+    if (rc) {
+        LOGE(ERROR, "failed to get available sockets");
+        goto out;
+    }
+
+    ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_hw_info));
+
+    libxl_for_each_set_bit(socketid, socketmap) {
+        ptr[i].id = socketid;
+        if (xc_psr_get_hw_info(ctx->xch, socketid, xc_type, &hw_info)) {
+            rc = ERROR_FAIL;
+            free(ptr);
+            goto out;
+        }
+
+        libxl__xc_hw_info_to_libxl_hw_info(type, &hw_info, &ptr[i]);
+
+        i++;
+    }
+
+    *info = ptr;
+    *nr = i;
+out:
+    libxl_bitmap_dispose(&socketmap);
+    GC_FREE;
+    return rc;
 }
 
 void libxl_psr_hw_info_list_free(libxl_psr_hw_info *list, unsigned int nr)
 {
+    unsigned int i;
+
+    for (i = 0; i < nr; i++)
+        libxl_psr_hw_info_dispose(&list[i]);
+    free(list);
 }
 
 /*
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 12/16] tools: implement the new xl get hw info interface
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (10 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 11/16] tools: implement the new libxl " Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-08  7:23 ` [PATCH v6 13/16] tools: rename 'xc_psr_cat_type' to 'xc_psr_type' Yi Sun
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Chao Peng, Yi Sun, Ian Jackson, Roger Pau Monné

This patch implements a new xl get HW info interface. A new argument
is added for psr-hwinfo command to get and show MBA HW info.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v3:
    - change the format string of printf in 'psr_mba_hwinfo'.
      (suggested by Roger Pau Monné)
    - add 'const' for 'opts[]' in 'main_psr_hwinfo'.
      (suggested by Roger Pau Monné)
v2:
    - split out this patch from a big patch in v1.
      (suggested by Wei Liu)
    - change 'MBA_INFO' to 'MBA'. Also, change 'mba_info' to 'mba'.
      (suggested by Chao Peng)
---
 tools/xl/xl_cmdtable.c |  1 +
 tools/xl/xl_psr.c      | 39 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index c304a85..dbbfc02 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -549,6 +549,7 @@ struct cmd_spec cmd_table[] = {
       "[options]",
       "-m, --cmt       Show Cache Monitoring Technology (CMT) hardware info\n"
       "-a, --cat       Show Cache Allocation Technology (CAT) hardware info\n"
+      "-b, --mba       Show Memory Bandwidth Allocation (MBA) hardware info\n"
     },
     { "psr-cmt-attach",
       &main_psr_cmt_attach, 0, 1,
diff --git a/tools/xl/xl_psr.c b/tools/xl/xl_psr.c
index ef00048..ab47d96 100644
--- a/tools/xl/xl_psr.c
+++ b/tools/xl/xl_psr.c
@@ -475,6 +475,31 @@ static int psr_l2_cat_hwinfo(void)
     return rc;
 }
 
+static int psr_mba_hwinfo(void)
+{
+    int rc;
+    unsigned int i, nr;
+    libxl_psr_hw_info *info;
+
+    rc = libxl_psr_get_hw_info(ctx, LIBXL_PSR_FEAT_TYPE_MBA, 0, &nr, &info);
+    if (rc)
+        return rc;
+
+    printf("Memory Bandwidth Allocation (MBA):\n");
+
+    for (i = 0; i < nr; i++) {
+        printf("Socket ID               : %u\n", info[i].id);
+        printf("Linear Mode             : %s\n",
+               info[i].u.mba.linear ? "Enabled" : "Disabled");
+        printf("Maximum COS             : %u\n", info[i].u.mba.cos_max);
+        printf("Maximum Throttling Value: %u\n", info[i].u.mba.thrtl_max);
+        printf("Default Throttling Value: %u\n", 0);
+    }
+
+    libxl_psr_hw_info_list_free(info, nr);
+    return rc;
+}
+
 int main_psr_cat_cbm_set(int argc, char **argv)
 {
     uint32_t domid;
@@ -593,20 +618,24 @@ int main_psr_cat_show(int argc, char **argv)
 int main_psr_hwinfo(int argc, char **argv)
 {
     int opt, ret = 0;
-    bool all = true, cmt = false, cat = false;
-    static struct option opts[] = {
+    bool all = true, cmt = false, cat = false, mba = false;
+    static const struct option opts[] = {
         {"cmt", 0, 0, 'm'},
         {"cat", 0, 0, 'a'},
+        {"mba", 0, 0, 'b'},
         COMMON_LONG_OPTS
     };
 
-    SWITCH_FOREACH_OPT(opt, "ma", opts, "psr-hwinfo", 0) {
+    SWITCH_FOREACH_OPT(opt, "mab", opts, "psr-hwinfo", 0) {
     case 'm':
         all = false; cmt = true;
         break;
     case 'a':
         all = false; cat = true;
         break;
+    case 'b':
+        all = false; mba = true;
+        break;
     }
 
     if (!ret && (all || cmt))
@@ -619,6 +648,10 @@ int main_psr_hwinfo(int argc, char **argv)
     if (all || cat)
         ret = psr_l2_cat_hwinfo();
 
+    /* MBA is independent of CMT and CAT */
+    if (all || mba)
+        ret = psr_mba_hwinfo();
+
     return ret;
 }
 
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 13/16] tools: rename 'xc_psr_cat_type' to 'xc_psr_type'
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (11 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 12/16] tools: implement the new xl " Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-08  7:23 ` [PATCH v6 14/16] tools: implement new generic get value interface and MBA get value command Yi Sun
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Chao Peng, Yi Sun, Wei Liu, Roger Pau Monné

This patch renames 'xc_psr_cat_type' to 'xc_psr_type' so that
the structure name is common for all allocation features.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Chao Peng <chao.p.peng@linux.intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v5:
    - remove a duplicated ';'.
      (suggested by Roger Pau Monné)
v4:
    - move assignment of xc_type to its declaration place.
      (suggested by Roger Pau Monné)
v3:
    - change 'xc_psr_val_type' to 'xc_psr_type'.
      (suggested by Roger Pau Monné)
---
 tools/libxc/include/xenctrl.h |  8 ++++----
 tools/libxc/xc_psr.c          |  4 ++--
 tools/libxl/libxl_psr.c       | 11 +++++------
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 2d977c8..2736bc5 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2466,13 +2466,13 @@ enum xc_psr_cmt_type {
 };
 typedef enum xc_psr_cmt_type xc_psr_cmt_type;
 
-enum xc_psr_cat_type {
+enum xc_psr_type {
     XC_PSR_CAT_L3_CBM      = 1,
     XC_PSR_CAT_L3_CBM_CODE = 2,
     XC_PSR_CAT_L3_CBM_DATA = 3,
     XC_PSR_CAT_L2_CBM      = 4,
 };
-typedef enum xc_psr_cat_type xc_psr_cat_type;
+typedef enum xc_psr_type xc_psr_type;
 
 enum xc_psr_feat_type {
     XC_PSR_CAT_L3,
@@ -2512,10 +2512,10 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid, uint32_t cpu,
 int xc_psr_cmt_enabled(xc_interface *xch);
 
 int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
-                               xc_psr_cat_type type, uint32_t target,
+                               xc_psr_type type, uint32_t target,
                                uint64_t data);
 int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
-                               xc_psr_cat_type type, uint32_t target,
+                               xc_psr_type type, uint32_t target,
                                uint64_t *data);
 int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
                        xc_psr_feat_type type, xc_psr_hw_info *hw_info);
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 2c605a7..01f4ba7 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -249,7 +249,7 @@ int xc_psr_cmt_enabled(xc_interface *xch)
     return 0;
 }
 int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
-                               xc_psr_cat_type type, uint32_t target,
+                               xc_psr_type type, uint32_t target,
                                uint64_t data)
 {
     DECLARE_DOMCTL;
@@ -284,7 +284,7 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
 }
 
 int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
-                               xc_psr_cat_type type, uint32_t target,
+                               xc_psr_type type, uint32_t target,
                                uint64_t *data)
 {
     int rc;
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index b053abd..c54cb6f 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -303,11 +303,11 @@ out:
     return rc;
 }
 
-static inline xc_psr_cat_type libxl__psr_cbm_type_to_libxc_psr_cat_type(
+static inline xc_psr_type libxl__psr_cbm_type_to_libxc_psr_type(
     libxl_psr_cbm_type type)
 {
-    BUILD_BUG_ON(sizeof(libxl_psr_cbm_type) != sizeof(xc_psr_cat_type));
-    return (xc_psr_cat_type)type;
+    BUILD_BUG_ON(sizeof(libxl_psr_cbm_type) != sizeof(xc_psr_type));
+    return (xc_psr_type)type;
 }
 
 int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
@@ -325,12 +325,11 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
     }
 
     libxl_for_each_set_bit(socketid, *target_map) {
-        xc_psr_cat_type xc_type;
+        xc_psr_type xc_type = libxl__psr_cbm_type_to_libxc_psr_type(type);
 
         if (socketid >= nr_sockets)
             break;
 
-        xc_type = libxl__psr_cbm_type_to_libxc_psr_cat_type(type);
         if (xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type,
                                        socketid, cbm)) {
             libxl__psr_cat_log_err_msg(gc, errno);
@@ -349,7 +348,7 @@ int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
 {
     GC_INIT(ctx);
     int rc = 0;
-    xc_psr_cat_type xc_type = libxl__psr_cbm_type_to_libxc_psr_cat_type(type);
+    xc_psr_type xc_type = libxl__psr_cbm_type_to_libxc_psr_type(type);
 
     if (xc_psr_cat_get_domain_data(ctx->xch, domid, xc_type,
                                    target, cbm_r)) {
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 14/16] tools: implement new generic get value interface and MBA get value command
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (12 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 13/16] tools: rename 'xc_psr_cat_type' to 'xc_psr_type' Yi Sun
@ 2017-10-08  7:23 ` Yi Sun
  2017-10-08  7:24 ` [PATCH v6 15/16] tools: implement new generic set value interface and MBA set " Yi Sun
  2017-10-08  7:24 ` [PATCH v6 16/16] docs: add MBA description in docs Yi Sun
  15 siblings, 0 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:23 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Chao Peng, Yi Sun, Wei Liu, Roger Pau Monné

This patch implements generic get value interfaces in libxc and libxl.
It also refactors the get value flow in xl to make it be suitable for all
allocation features. Based on that, a new MBA get value command is added in xl.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v6:
    - fix one coding style issue.
      (suggested by Roger Pau Monné)
v5:
    - start a newline for "CDP" because it exceeds 80 characters.
      (suggested by Roger Pau Monné)
    - remove a duplicated ';'.
      (suggested by Roger Pau Monné)
    - remove a extra newline.
      (suggested by Roger Pau Monné)
    - correct words in log message.
      (suggested by Roger Pau Monné)
v4:
    - use designated initializers for 'feat_name[]'.
      (suggested by Roger Pau Monné)
    - use LOG in 'libxl__psr_alloc_log_err_msg'.
      (suggested by Roger Pau Monné)
v3:
    - replace 'libxl_psr_cbm_type' to 'libxl_psr_type' in newly defined
      interfaces.
      (suggested by Roger Pau Monné)
v2:
    - change 'CAT_INFO'/'MBA_INFO' to 'CAT'/'MBA'. The related structure names
      are changed too.
      (suggested by Chao Peng)
---
 tools/libxc/include/xenctrl.h |   7 +-
 tools/libxc/xc_psr.c          |   9 +-
 tools/libxl/libxl_psr.c       |  58 ++++++++-----
 tools/xl/xl.h                 |   1 +
 tools/xl/xl_cmdtable.c        |   5 ++
 tools/xl/xl_psr.c             | 185 ++++++++++++++++++++++++++++++------------
 6 files changed, 183 insertions(+), 82 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 2736bc5..dcea09e 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2471,6 +2471,7 @@ enum xc_psr_type {
     XC_PSR_CAT_L3_CBM_CODE = 2,
     XC_PSR_CAT_L3_CBM_DATA = 3,
     XC_PSR_CAT_L2_CBM      = 4,
+    XC_PSR_MBA_THRTL       = 5,
 };
 typedef enum xc_psr_type xc_psr_type;
 
@@ -2514,9 +2515,9 @@ int xc_psr_cmt_enabled(xc_interface *xch);
 int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
                                xc_psr_type type, uint32_t target,
                                uint64_t data);
-int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
-                               xc_psr_type type, uint32_t target,
-                               uint64_t *data);
+int xc_psr_get_domain_data(xc_interface *xch, uint32_t domid,
+                           xc_psr_type type, uint32_t target,
+                           uint64_t *data);
 int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
                        xc_psr_feat_type type, xc_psr_hw_info *hw_info);
 
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 01f4ba7..191de97 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -283,9 +283,9 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
     return do_domctl(xch, &domctl);
 }
 
-int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
-                               xc_psr_type type, uint32_t target,
-                               uint64_t *data)
+int xc_psr_get_domain_data(xc_interface *xch, uint32_t domid,
+                           xc_psr_type type, uint32_t target,
+                           uint64_t *data)
 {
     int rc;
     DECLARE_DOMCTL;
@@ -305,6 +305,9 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
     case XC_PSR_CAT_L2_CBM:
         cmd = XEN_DOMCTL_PSR_GET_L2_CBM;
         break;
+    case XC_PSR_MBA_THRTL:
+        cmd = XEN_DOMCTL_PSR_GET_MBA_THRTL;
+        break;
     default:
         errno = EINVAL;
         return -1;
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index c54cb6f..7c560bc 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -71,16 +71,30 @@ static void libxl__psr_cmt_log_err_msg(libxl__gc *gc, int err)
     LOGE(ERROR, "%s", msg);
 }
 
-static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
+static void libxl__psr_alloc_log_err_msg(libxl__gc *gc,
+                                         int err,
+                                         libxl_psr_type type)
 {
+    /*
+     * Index is 'libxl_psr_type' so we set two 'CDP' to correspond to
+     * DATA and CODE.
+     */
+    const char * const feat_name[] = {
+        [LIBXL_PSR_CBM_TYPE_UNKNOWN] = "UNKNOWN",
+        [LIBXL_PSR_CBM_TYPE_L3_CBM] = "L3 CAT",
+        [LIBXL_PSR_CBM_TYPE_L3_CBM_CODE...LIBXL_PSR_CBM_TYPE_L3_CBM_DATA] =
+                                      "CDP",
+        [LIBXL_PSR_CBM_TYPE_L2_CBM] = "L2 CAT",
+        [LIBXL_PSR_CBM_TYPE_MBA_THRTL] = "MBA",
+    };
     char *msg;
 
     switch (err) {
     case ENODEV:
-        msg = "CAT is not supported in this system";
+        msg = "is not supported in this system";
         break;
     case ENOENT:
-        msg = "CAT is not enabled on the socket";
+        msg = "is not enabled on the socket";
         break;
     case EOVERFLOW:
         msg = "no free COS available";
@@ -106,7 +120,7 @@ static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
         return;
     }
 
-    LOGE(ERROR, "%s", msg);
+    LOG(ERROR, "%s: %s", feat_name[type], msg);
 }
 
 static int libxl__pick_socket_cpu(libxl__gc *gc, uint32_t socketid)
@@ -303,10 +317,10 @@ out:
     return rc;
 }
 
-static inline xc_psr_type libxl__psr_cbm_type_to_libxc_psr_type(
-    libxl_psr_cbm_type type)
+static inline xc_psr_type libxl__psr_type_to_libxc_psr_type(
+    libxl_psr_type type)
 {
-    BUILD_BUG_ON(sizeof(libxl_psr_cbm_type) != sizeof(xc_psr_type));
+    BUILD_BUG_ON(sizeof(libxl_psr_type) != sizeof(xc_psr_type));
     return (xc_psr_type)type;
 }
 
@@ -325,14 +339,14 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
     }
 
     libxl_for_each_set_bit(socketid, *target_map) {
-        xc_psr_type xc_type = libxl__psr_cbm_type_to_libxc_psr_type(type);
+        xc_psr_type xc_type = libxl__psr_type_to_libxc_psr_type(type);
 
         if (socketid >= nr_sockets)
             break;
 
         if (xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type,
                                        socketid, cbm)) {
-            libxl__psr_cat_log_err_msg(gc, errno);
+            libxl__psr_alloc_log_err_msg(gc, errno, type);
             rc = ERROR_FAIL;
         }
     }
@@ -346,18 +360,7 @@ int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
                           libxl_psr_cbm_type type, uint32_t target,
                           uint64_t *cbm_r)
 {
-    GC_INIT(ctx);
-    int rc = 0;
-    xc_psr_type xc_type = libxl__psr_cbm_type_to_libxc_psr_type(type);
-
-    if (xc_psr_cat_get_domain_data(ctx->xch, domid, xc_type,
-                                   target, cbm_r)) {
-        libxl__psr_cat_log_err_msg(gc, errno);
-        rc = ERROR_FAIL;
-    }
-
-    GC_FREE;
-    return rc;
+    return libxl_psr_get_val(ctx, domid, type, target, cbm_r);
 }
 
 static xc_psr_feat_type libxl__feat_type_to_libxc_feat_type(
@@ -457,7 +460,18 @@ int libxl_psr_get_val(libxl_ctx *ctx, uint32_t domid,
                       libxl_psr_type type, unsigned int target,
                       uint64_t *val)
 {
-    return ERROR_FAIL;
+    GC_INIT(ctx);
+    int rc = 0;
+    xc_psr_type xc_type = libxl__psr_type_to_libxc_psr_type(type);
+
+    if (xc_psr_get_domain_data(ctx->xch, domid, xc_type,
+                               target, val)) {
+        libxl__psr_alloc_log_err_msg(gc, errno, type);
+        rc = ERROR_FAIL;
+    }
+
+    GC_FREE;
+    return rc;
 }
 
 static void libxl__xc_hw_info_to_libxl_hw_info(
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index 6b60d1d..a72458b 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -208,6 +208,7 @@ int main_psr_cmt_detach(int argc, char **argv);
 int main_psr_cmt_show(int argc, char **argv);
 int main_psr_cat_cbm_set(int argc, char **argv);
 int main_psr_cat_show(int argc, char **argv);
+int main_psr_mba_show(int argc, char **argv);
 #endif
 int main_qemu_monitor_command(int argc, char **argv);
 
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index dbbfc02..cac2014 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -585,6 +585,11 @@ struct cmd_spec cmd_table[] = {
       "[options] <Domain>",
       "-l <level>        Specify the cache level to process, otherwise L3 cache is processed\n"
     },
+    { "psr-mba-show",
+      &main_psr_mba_show, 0, 1,
+      "Show Memory Bandwidth Allocation information",
+      "<Domain>",
+    },
 #endif
     { "usbctrl-attach",
       &main_usbctrl_attach, 0, 1,
diff --git a/tools/xl/xl_psr.c b/tools/xl/xl_psr.c
index ab47d96..0eedbc7 100644
--- a/tools/xl/xl_psr.c
+++ b/tools/xl/xl_psr.c
@@ -327,19 +327,26 @@ out:
     return rc;
 }
 
-static void psr_cat_print_one_domain_cbm_type(uint32_t domid, uint32_t socketid,
-                                              libxl_psr_cbm_type type)
+static void psr_print_one_domain_val_type(uint32_t domid,
+                                          libxl_psr_hw_info *info,
+                                          libxl_psr_type type)
 {
-    uint64_t cbm;
+    uint64_t val;
 
-    if (!libxl_psr_cat_get_cbm(ctx, domid, type, socketid, &cbm))
-        printf("%#16"PRIx64, cbm);
+    if (!libxl_psr_get_val(ctx, domid, type, info->id, &val)) {
+        if (type == LIBXL_PSR_CBM_TYPE_MBA_THRTL && info->u.mba.linear)
+            printf("%16"PRIu64, val);
+        else
+            printf("%#16"PRIx64, val);
+    }
     else
         printf("%16s", "error");
 }
 
-static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid,
-                                         bool cdp_enabled, unsigned int lvl)
+static void psr_print_one_domain_val(uint32_t domid,
+                                     libxl_psr_hw_info *info,
+                                     libxl_psr_feat_type type,
+                                     unsigned int lvl)
 {
     char *domain_name;
 
@@ -347,106 +354,155 @@ static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socketid,
     printf("%5d%25s", domid, domain_name);
     free(domain_name);
 
-    switch (lvl) {
-    case 3:
-        if (!cdp_enabled) {
-            psr_cat_print_one_domain_cbm_type(domid, socketid,
+    switch (type) {
+    case LIBXL_PSR_FEAT_TYPE_CAT:
+        switch (lvl) {
+        case 3:
+            if (!info->u.cat.cdp_enabled) {
+                psr_print_one_domain_val_type(domid, info,
                                               LIBXL_PSR_CBM_TYPE_L3_CBM);
-        } else {
-            psr_cat_print_one_domain_cbm_type(domid, socketid,
+            } else {
+                psr_print_one_domain_val_type(domid, info,
                                               LIBXL_PSR_CBM_TYPE_L3_CBM_CODE);
-            psr_cat_print_one_domain_cbm_type(domid, socketid,
+                psr_print_one_domain_val_type(domid, info,
                                               LIBXL_PSR_CBM_TYPE_L3_CBM_DATA);
-        }
-        break;
-    case 2:
-        psr_cat_print_one_domain_cbm_type(domid, socketid,
+            }
+            break;
+
+        case 2:
+            psr_print_one_domain_val_type(domid, info,
                                           LIBXL_PSR_CBM_TYPE_L2_CBM);
+            break;
+
+        default:
+            printf("Input lvl %d is wrong!", lvl);
+        }
         break;
-    default:
-        printf("Input lvl %d is wrong!", lvl);
+
+    case LIBXL_PSR_FEAT_TYPE_MBA:
+        psr_print_one_domain_val_type(domid, info,
+                                      LIBXL_PSR_CBM_TYPE_MBA_THRTL);
         break;
     }
 
     printf("\n");
 }
 
-static int psr_cat_print_domain_cbm(uint32_t domid, uint32_t socketid,
-                                    bool cdp_enabled, unsigned int lvl)
+static int psr_print_domain_val(uint32_t domid,
+                                libxl_psr_hw_info *info,
+                                libxl_psr_feat_type type,
+                                unsigned int lvl)
 {
     int i, nr_domains;
     libxl_dominfo *list;
 
     if (domid != INVALID_DOMID) {
-        psr_cat_print_one_domain_cbm(domid, socketid, cdp_enabled, lvl);
+        psr_print_one_domain_val(domid, info, type, lvl);
         return 0;
     }
 
     if (!(list = libxl_list_domain(ctx, &nr_domains))) {
-        fprintf(stderr, "Failed to get domain list for cbm display\n");
-        return -1;
+        fprintf(stderr, "Failed to get domain list for value display\n");
+        return EXIT_FAILURE;
     }
 
     for (i = 0; i < nr_domains; i++)
-        psr_cat_print_one_domain_cbm(list[i].domid, socketid, cdp_enabled, lvl);
+        psr_print_one_domain_val(list[i].domid, info, type, lvl);
     libxl_dominfo_list_free(list, nr_domains);
 
     return 0;
 }
 
-static int psr_cat_print_socket(uint32_t domid, libxl_psr_cat_info *info,
-                                unsigned int lvl)
+static int psr_print_socket(uint32_t domid,
+                            libxl_psr_hw_info *info,
+                            libxl_psr_feat_type type,
+                            unsigned int lvl)
 {
-    int rc;
-    uint32_t l3_cache_size;
-
     printf("%-16s: %u\n", "Socket ID", info->id);
 
-    /* So far, CMT only supports L3 cache. */
-    if (lvl == 3) {
-        rc = libxl_psr_cmt_get_l3_cache_size(ctx, info->id, &l3_cache_size);
-        if (rc) {
-            fprintf(stderr, "Failed to get l3 cache size for socket:%d\n",
-                    info->id);
-            return -1;
+    switch (type) {
+    case LIBXL_PSR_FEAT_TYPE_CAT:
+    {
+        int rc;
+        uint32_t l3_cache_size;
+
+        /* So far, CMT only supports L3 cache. */
+        if (lvl == 3) {
+            rc = libxl_psr_cmt_get_l3_cache_size(ctx, info->id, &l3_cache_size);
+            if (rc) {
+                fprintf(stderr, "Failed to get l3 cache size for socket:%d\n",
+                        info->id);
+                return -1;
+            }
+            printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
         }
-        printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
+
+        printf("%-16s: %#llx\n", "Default CBM",
+               (1ull << info->u.cat.cbm_len) - 1);
+        if (info->u.cat.cdp_enabled)
+            printf("%5s%25s%16s%16s\n", "ID", "NAME", "CBM (code)", "CBM (data)");
+        else
+            printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
+
+        break;
     }
 
-    printf("%-16s: %#llx\n", "Default CBM", (1ull << info->cbm_len) - 1);
-    if (info->cdp_enabled)
-        printf("%5s%25s%16s%16s\n", "ID", "NAME", "CBM (code)", "CBM (data)");
-    else
-        printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
+    case LIBXL_PSR_FEAT_TYPE_MBA:
+        printf("%-16s: %u\n", "Default THRTL", 0);
+        printf("%5s%25s%16s\n", "ID", "NAME", "THRTL");
+        break;
 
-    return psr_cat_print_domain_cbm(domid, info->id, info->cdp_enabled, lvl);
+    default:
+        fprintf(stderr, "Input feature type %d is wrong\n", type);
+        return EXIT_FAILURE;
+    }
+
+    return psr_print_domain_val(domid, info, type, lvl);
 }
 
-static int psr_cat_show(uint32_t domid, unsigned int lvl)
+static int psr_val_show(uint32_t domid,
+                        libxl_psr_feat_type type,
+                        unsigned int lvl)
 {
     unsigned int i, nr;
     int rc;
-    libxl_psr_cat_info *info;
+    libxl_psr_hw_info *info;
 
-    if (lvl != 2 && lvl != 3) {
-        fprintf(stderr, "Input lvl %d is wrong\n", lvl);
+    switch (type) {
+    case LIBXL_PSR_FEAT_TYPE_CAT:
+        if (lvl != 2 && lvl != 3) {
+            fprintf(stderr, "Input lvl %d is wrong\n", lvl);
+            return EXIT_FAILURE;
+        }
+        break;
+
+    case LIBXL_PSR_FEAT_TYPE_MBA:
+        if (lvl) {
+            fprintf(stderr,
+                    "Unexpected lvl parameter %d for MBA feature\n", lvl);
+            return EXIT_FAILURE;
+        }
+        break;
+
+    default:
+        fprintf(stderr, "Input feature type %d is wrong\n", type);
         return EXIT_FAILURE;
     }
 
-    rc = libxl_psr_cat_get_info(ctx, &info, &nr, lvl);
+    rc = libxl_psr_get_hw_info(ctx, type, lvl, &nr, &info);
     if (rc) {
-        fprintf(stderr, "Failed to get %s cat info\n", (lvl == 3)?"L3":"L2");
+        fprintf(stderr, "Failed to get info\n");
         return rc;
     }
 
     for (i = 0; i < nr; i++) {
-        rc = psr_cat_print_socket(domid, info + i, lvl);
+        rc = psr_print_socket(domid, info + i, type, lvl);
         if (rc)
             goto out;
     }
 
 out:
-    libxl_psr_cat_info_list_free(info, nr);
+    libxl_psr_hw_info_list_free(info, nr);
     return rc;
 }
 
@@ -475,6 +531,27 @@ static int psr_l2_cat_hwinfo(void)
     return rc;
 }
 
+int main_psr_mba_show(int argc, char **argv)
+{
+    int opt;
+    uint32_t domid;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "psr-mba-show", 0) {
+        /* No options */
+    }
+
+    if (optind >= argc)
+        domid = INVALID_DOMID;
+    else if (optind == argc - 1)
+        domid = find_domain(argv[optind]);
+    else {
+        help("psr-mba-show");
+        return 2;
+    }
+
+    return psr_val_show(domid, LIBXL_PSR_FEAT_TYPE_MBA, 0);
+}
+
 static int psr_mba_hwinfo(void)
 {
     int rc;
@@ -612,7 +689,7 @@ int main_psr_cat_show(int argc, char **argv)
         return 2;
     }
 
-    return psr_cat_show(domid, lvl);
+    return psr_val_show(domid, LIBXL_PSR_FEAT_TYPE_CAT, lvl);
 }
 
 int main_psr_hwinfo(int argc, char **argv)
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 15/16] tools: implement new generic set value interface and MBA set value command
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (13 preceding siblings ...)
  2017-10-08  7:23 ` [PATCH v6 14/16] tools: implement new generic get value interface and MBA get value command Yi Sun
@ 2017-10-08  7:24 ` Yi Sun
  2017-10-08  7:24 ` [PATCH v6 16/16] docs: add MBA description in docs Yi Sun
  15 siblings, 0 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:24 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, Chao Peng, Yi Sun, Ian Jackson, Roger Pau Monné

This patch implements new generic set value interfaces in libxc and libxl.
These interfaces are suitable for all allocation features. It also adds a
new MBA set value command in xl.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
CC: Wei Liu <wei.liu2@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v5:
    - move xc_type definition and value get out of the loop.
      (suggested by Roger Pau Monné)
v4:
    - remove 'ALLOC_' from macro name.
      (suggested by Roger Pau Monné)
    - adjust place of argc check and return EXIT_FAILURE.
      (suggested by Roger Pau Monné)
    - fix indentation issue.
      (suggested by Roger Pau Monné)
    - move same type local variables declaration to a single line.
      (suggested by Roger Pau Monné)
v3:
    - add 'const' for 'opts[]' in 'main_psr_mba_set'.
      (suggested by Roger Pau Monné)
    - replace 'libxl_psr_cbm_type' to 'libxl_psr_type' for newly defined
      interfaces.
      (suggested by Roger Pau Monné)
---
 tools/libxc/include/xenctrl.h |  6 ++---
 tools/libxc/xc_psr.c          |  9 ++++---
 tools/libxl/libxl_psr.c       | 52 ++++++++++++++++++++--------------------
 tools/xl/xl.h                 |  1 +
 tools/xl/xl_cmdtable.c        |  6 +++++
 tools/xl/xl_psr.c             | 55 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 96 insertions(+), 33 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index dcea09e..f2463f9 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2512,9 +2512,9 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid, uint32_t cpu,
                         uint64_t *tsc);
 int xc_psr_cmt_enabled(xc_interface *xch);
 
-int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
-                               xc_psr_type type, uint32_t target,
-                               uint64_t data);
+int xc_psr_set_domain_data(xc_interface *xch, uint32_t domid,
+                           xc_psr_type type, uint32_t target,
+                           uint64_t data);
 int xc_psr_get_domain_data(xc_interface *xch, uint32_t domid,
                            xc_psr_type type, uint32_t target,
                            uint64_t *data);
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 191de97..1609185 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -248,9 +248,9 @@ int xc_psr_cmt_enabled(xc_interface *xch)
 
     return 0;
 }
-int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
-                               xc_psr_type type, uint32_t target,
-                               uint64_t data)
+int xc_psr_set_domain_data(xc_interface *xch, uint32_t domid,
+                           xc_psr_type type, uint32_t target,
+                           uint64_t data)
 {
     DECLARE_DOMCTL;
     uint32_t cmd;
@@ -269,6 +269,9 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
     case XC_PSR_CAT_L2_CBM:
         cmd = XEN_DOMCTL_PSR_SET_L2_CBM;
         break;
+    case XC_PSR_MBA_THRTL:
+        cmd = XEN_DOMCTL_PSR_SET_MBA_THRTL;
+        break;
     default:
         errno = EINVAL;
         return -1;
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 7c560bc..9ced7d1 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -328,32 +328,7 @@ int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
                           libxl_psr_cbm_type type, libxl_bitmap *target_map,
                           uint64_t cbm)
 {
-    GC_INIT(ctx);
-    int rc;
-    int socketid, nr_sockets;
-
-    rc = libxl__count_physical_sockets(gc, &nr_sockets);
-    if (rc) {
-        LOGED(ERROR, domid, "failed to get system socket count");
-        goto out;
-    }
-
-    libxl_for_each_set_bit(socketid, *target_map) {
-        xc_psr_type xc_type = libxl__psr_type_to_libxc_psr_type(type);
-
-        if (socketid >= nr_sockets)
-            break;
-
-        if (xc_psr_cat_set_domain_data(ctx->xch, domid, xc_type,
-                                       socketid, cbm)) {
-            libxl__psr_alloc_log_err_msg(gc, errno, type);
-            rc = ERROR_FAIL;
-        }
-    }
-
-out:
-    GC_FREE;
-    return rc;
+    return libxl_psr_set_val(ctx, domid, type, target_map, cbm);
 }
 
 int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
@@ -453,7 +428,30 @@ int libxl_psr_set_val(libxl_ctx *ctx, uint32_t domid,
                       libxl_psr_type type, libxl_bitmap *target_map,
                       uint64_t val)
 {
-    return ERROR_FAIL;
+    GC_INIT(ctx);
+    int rc, socketid, nr_sockets;
+    xc_psr_type xc_type = libxl__psr_type_to_libxc_psr_type(type);
+
+    rc = libxl__count_physical_sockets(gc, &nr_sockets);
+    if (rc) {
+        LOG(ERROR, "failed to get system socket count");
+        goto out;
+    }
+
+    libxl_for_each_set_bit(socketid, *target_map) {
+        if (socketid >= nr_sockets)
+            break;
+
+        if (xc_psr_set_domain_data(ctx->xch, domid, xc_type,
+                                   socketid, val)) {
+            libxl__psr_alloc_log_err_msg(gc, errno, type);
+            rc = ERROR_FAIL;
+        }
+    }
+
+out:
+    GC_FREE;
+    return rc;
 }
 
 int libxl_psr_get_val(libxl_ctx *ctx, uint32_t domid,
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index a72458b..4e784ff 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -208,6 +208,7 @@ int main_psr_cmt_detach(int argc, char **argv);
 int main_psr_cmt_show(int argc, char **argv);
 int main_psr_cat_cbm_set(int argc, char **argv);
 int main_psr_cat_show(int argc, char **argv);
+int main_psr_mba_set(int argc, char **argv);
 int main_psr_mba_show(int argc, char **argv);
 #endif
 int main_qemu_monitor_command(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index cac2014..3339311 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -585,6 +585,12 @@ struct cmd_spec cmd_table[] = {
       "[options] <Domain>",
       "-l <level>        Specify the cache level to process, otherwise L3 cache is processed\n"
     },
+    { "psr-mba-set",
+      &main_psr_mba_set, 0, 1,
+      "Set throttling value (THRTL) for a domain",
+      "[options] <Domain> <THRTL>",
+      "-s <socket>       Specify the socket to process, otherwise all sockets are processed\n"
+    },
     { "psr-mba-show",
       &main_psr_mba_show, 0, 1,
       "Show Memory Bandwidth Allocation information",
diff --git a/tools/xl/xl_psr.c b/tools/xl/xl_psr.c
index 0eedbc7..181d300 100644
--- a/tools/xl/xl_psr.c
+++ b/tools/xl/xl_psr.c
@@ -552,6 +552,61 @@ int main_psr_mba_show(int argc, char **argv)
     return psr_val_show(domid, LIBXL_PSR_FEAT_TYPE_MBA, 0);
 }
 
+int main_psr_mba_set(int argc, char **argv)
+{
+    uint32_t domid;
+    libxl_psr_type type;
+    uint64_t thrtl;
+    int ret, opt = 0;
+    libxl_bitmap target_map;
+    char *value;
+    libxl_string_list socket_list;
+    unsigned long start, end;
+    unsigned int i, j, len;
+
+    static const struct option opts[] = {
+        {"socket", 1, 0, 's'},
+        COMMON_LONG_OPTS
+    };
+
+    if (argc != optind + 2) {
+        help("psr-mba-set");
+        return EXIT_FAILURE;
+    }
+
+    libxl_socket_bitmap_alloc(ctx, &target_map, 0);
+    libxl_bitmap_set_none(&target_map);
+
+    SWITCH_FOREACH_OPT(opt, "s:", opts, "psr-mba-set", 0) {
+    case 's':
+        trim(isspace, optarg, &value);
+        split_string_into_string_list(value, ",", &socket_list);
+        len = libxl_string_list_length(&socket_list);
+        for (i = 0; i < len; i++) {
+            parse_range(socket_list[i], &start, &end);
+            for (j = start; j <= end; j++)
+                libxl_bitmap_set(&target_map, j);
+        }
+
+        libxl_string_list_dispose(&socket_list);
+        free(value);
+        break;
+    }
+
+    type = LIBXL_PSR_CBM_TYPE_MBA_THRTL;
+
+    if (libxl_bitmap_is_empty(&target_map))
+        libxl_bitmap_set_any(&target_map);
+
+    domid = find_domain(argv[optind]);
+    thrtl = strtoll(argv[optind + 1], NULL , 0);
+
+    ret = libxl_psr_set_val(ctx, domid, type, &target_map, thrtl);
+
+    libxl_bitmap_dispose(&target_map);
+    return ret;
+}
+
 static int psr_mba_hwinfo(void)
 {
     int rc;
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v6 16/16] docs: add MBA description in docs
  2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
                   ` (14 preceding siblings ...)
  2017-10-08  7:24 ` [PATCH v6 15/16] tools: implement new generic set value interface and MBA set " Yi Sun
@ 2017-10-08  7:24 ` Yi Sun
  15 siblings, 0 replies; 26+ messages in thread
From: Yi Sun @ 2017-10-08  7:24 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Chao Peng, Yi Sun, Wei Liu, Roger Pau Monné

This patch adds MBA description in related documents.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

v5:
    - remove 'closed-loop' in 'xl-psr.markdown'
      (suggested by Roger Pau Monné)
v4:
    - modify description of MBA in 'xl.pod.1.in' to be same as feature doc.
      (suggested by Roger Pau Monné)
    - fix words issue.
      (suggested by Roger Pau Monné)
v2:
    - state the value type shown by 'psr-mba-show'. For linear mode,
      it shows decimal value. For non-linear mode, it shows hexadecimal
      value.
      (suggested by Chao Peng)
---
 docs/man/xl.pod.1.in      | 33 +++++++++++++++++++++++++
 docs/misc/xl-psr.markdown | 62 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index cd8bb1c..324ef9e 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -1845,6 +1845,39 @@ processed.
 
 =back
 
+=head2 Memory Bandwidth Allocation
+
+Intel Skylake and later server platforms offer capabilities to configure and
+make use of the Memory Bandwidth Allocation (MBA) mechanisms, which provides
+OS/VMMs the ability to slow misbehaving apps/VMs by using a credit-based
+throttling mechanism. In the Xen implementation, MBA is used to control memory
+bandwidth on VM basis. To enforce bandwidth on a specific domain, just set
+throttling value (THRTL) for the domain.
+
+=over 4
+
+=item B<psr-mba-set> [I<OPTIONS>] I<domain-id> I<thrtl>
+
+Set throttling value (THRTL) for a domain. For how to specify I<thrtl>
+please refer to L<http://xenbits.xen.org/docs/unstable/misc/xl-psr.html>.
+
+B<OPTIONS>
+
+=over 4
+
+=item B<-s SOCKET>, B<--socket=SOCKET>
+
+Specify the socket to process, otherwise all sockets are processed.
+
+=back
+
+=item B<psr-mba-show> [I<domain-id>]
+
+Show MBA settings for a certain domain or all domains. For linear mode, it
+shows the decimal value. For non-linear mode, it shows hexadecimal value.
+
+=back
+
 =head1 IGNORED FOR COMPATIBILITY WITH XM
 
 xl is mostly command-line compatible with the old xm utility used with
diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
index 04dd957..3d196ed 100644
--- a/docs/misc/xl-psr.markdown
+++ b/docs/misc/xl-psr.markdown
@@ -186,6 +186,68 @@ Setting data CBM for a domain:
 Setting the same code and data CBM for a domain:
 `xl psr-cat-set <domid> <cbm>`
 
+## Memory Bandwidth Allocation (MBA)
+
+Memory Bandwidth Allocation (MBA) is a new feature available on Intel
+Skylake and later server platforms that allows an OS or Hypervisor/VMM to
+slow misbehaving apps/VMs by using a credit-based throttling mechanism. To
+enforce bandwidth on a specific domain, just set throttling value (THRTL)
+into Class of Service (COS). MBA provides two THRTL mode. One is linear mode
+and the other is non-linear mode.
+
+In the linear mode the input precision is defined as 100-(THRTL_MAX). Values
+not an even multiple of the precision (e.g., 12%) will be rounded down (e.g.,
+to 10% delay by the hardware).
+
+If linear values are not supported then input delay values are powers-of-two
+from zero to the THRTL_MAX value from CPUID. In this case any values not a power
+of two will be rounded down the next nearest power of two.
+
+For example, assuming a system with 2 domains:
+
+ * A THRTL of 0x0 for every domain means each domain can access the whole cache
+   without any delay. This is the default.
+
+ * Linear mode: Giving one domain a THRTL of 0xC and the other domain's 0 means
+   that the first domain gets 10% delay to access the cache and the other one
+   without any delay.
+
+ * Non-linear mode: Giving one domain a THRTL of 0xC and the other domain's 0
+   means that the first domain gets 8% delay to access the cache and the other
+   one without any delay.
+
+For more detailed information please refer to Intel SDM chapter
+"Introduction to Memory Bandwidth Allocation".
+
+In Xen's implementation, THRTL can be configured with libxl/xl interfaces but
+COS is maintained in hypervisor only. The cache partition granularity is per
+domain, each domain has COS=0 assigned by default, the corresponding THRTL is
+0, which means all the cache resource can be accessed without delay.
+
+### xl interfaces
+
+System MBA information such as maximum COS and maximum THRTL can be obtained by:
+
+`xl psr-hwinfo --mba`
+
+The simplest way to change a domain's THRTL from its default is running:
+
+`xl psr-mba-set  [OPTIONS] <domid> <thrtl>`
+
+In a multi-socket system, the same thrtl will be set on each socket by default.
+Per socket thrtl can be specified with the `--socket SOCKET` option.
+
+Setting the THRTL may not be successful if insufficient COS is available. In
+such case unused COS(es) may be freed by setting THRTL of all related domains to
+its default value(0).
+
+Per domain THRTL settings can be shown by:
+
+`xl psr-mba-show [OPTIONS] <domid>`
+
+For linear mode, it shows the decimal value. For non-linear mode, it shows
+hexadecimal value.
+
 ## Reference
 
 [1] Intel SDM
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v6 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general
  2017-10-08  7:23 ` [PATCH v6 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general Yi Sun
@ 2017-10-10 14:59   ` Jan Beulich
  0 siblings, 0 replies; 26+ messages in thread
From: Jan Beulich @ 2017-10-10 14:59 UTC (permalink / raw)
  To: Yi Sun
  Cc: Wei Liu, Andrew Cooper, Ian Jackson, xen-devel, Chao Peng,
	Daniel De Graaf, Roger Pau Monné

>>> On 08.10.17 at 09:23, <yi.y.sun@linux.intel.com> wrote:
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1438,67 +1438,66 @@ long arch_do_domctl(
>          }
>          break;
>  
> -    case XEN_DOMCTL_psr_cat_op:
> -        switch ( domctl->u.psr_cat_op.cmd )
> +    case XEN_DOMCTL_psr_alloc:
> +#define domctl_psr_get_val(d, domctl, type, copyback) ({   \
> +    uint32_t v;                                            \
> +    int r = psr_get_val(d, domctl->u.psr_alloc.target,     \

As indicated in reply to Roger's request to drop the double
underscores you previously had here, using entirely
"conventional" names isn't good either, as this risks conflicts
with variables declared in outer scopes. A single trailing
underscore would be advised here, or some other mechanism
to obviously disambiguate them.

Furthermore if already you pass in variables (which is fine
but not strictly necessary for a macro with such a restricted
scope), uses like domctl above need to have parentheses
added.

>          default:
>              ret = -EOPNOTSUPP;
>              break;
>          }
> +
> +#undef domctl_psr_get_val
> +
>          break;

Looking especially at this part I think the #define and #undef
aren't placed well. Such temporary helper macros should not
have wider scope than necessary - you really only need them
from the first GET case label unil immediately before default:.
At the very least I'd like to ask for the #define to be moved
inside the switch() statement, and the #undef ahead of
default:.

With these taken care of
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v6 04/16] x86: a few optimizations to psr codes
  2017-10-08  7:23 ` [PATCH v6 04/16] x86: a few optimizations to psr codes Yi Sun
@ 2017-10-10 15:01   ` Jan Beulich
  0 siblings, 0 replies; 26+ messages in thread
From: Jan Beulich @ 2017-10-10 15:01 UTC (permalink / raw)
  To: Yi Sun; +Cc: Andrew Cooper, xen-devel, Wei Liu, Chao Peng, Roger Pau Monné

>>> On 08.10.17 at 09:23, <yi.y.sun@linux.intel.com> wrote:
> This patch refines psr codes:
> 1. Change type of 'cat_init_feature' to 'bool' to remove the pointless
>    returning of error code.
> 2. Move printk in 'cat_init_feature' to reduce a return path.
> 3. Define a local variable 'feat_mask' in 'psr_cpu_init' to reduce calling 
> of
>    'cpuid_count_leaf()'.
> 
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v6 05/16] x86: implement data structure and CPU init flow for MBA
  2017-10-08  7:23 ` [PATCH v6 05/16] x86: implement data structure and CPU init flow for MBA Yi Sun
@ 2017-10-10 15:26   ` Jan Beulich
  0 siblings, 0 replies; 26+ messages in thread
From: Jan Beulich @ 2017-10-10 15:26 UTC (permalink / raw)
  To: Yi Sun; +Cc: Andrew Cooper, xen-devel, Wei Liu, Chao Peng, Roger Pau Monné

>>> On 08.10.17 at 09:23, <yi.y.sun@linux.intel.com> wrote:
> This patch implements main data structures of MBA.
> 
> Like CAT features, MBA HW info has cos_max which means the max thrtl
> register number, and thrtl_max which means the max throttle value
> (delay value). It also has a flag to represent if the throttle
> value is linear or non-linear.
> 
> One thrtl register of MBA stores a throttle value for one or more
> domains. The throttle value means the delay between L2 cache and next
> cache level.

It continues to be unclear to me what a delay between two
cache levels is. Perhaps "... the delay applied to traffic between
L2 cache and next cache level" (albeit the doc talks about core
and interconnect instead of cache levels).

> @@ -1442,13 +1527,13 @@ static void psr_cpu_init(void)
>          feat_l3 = NULL;
>  
>          if ( (regs.c & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) &&
> -             !cat_init_feature(&regs, feat, info, FEAT_TYPE_L3_CDP) )
> +             cat_init_feature(&regs, feat, info, FEAT_TYPE_L3_CDP) )
>              feat_props[FEAT_TYPE_L3_CDP] = &l3_cdp_props;
>  
>          /* If CDP init fails, try to work as L3 CAT. */
>          if ( !feat_props[FEAT_TYPE_L3_CDP] )
>          {
> -            if ( !cat_init_feature(&regs, feat, info, FEAT_TYPE_L3_CAT) )
> +            if ( cat_init_feature(&regs, feat, info, FEAT_TYPE_L3_CAT) )
>                  feat_props[FEAT_TYPE_L3_CAT] = &l3_cat_props;
>              else
>                  feat_l3 = feat;
> @@ -1461,12 +1546,24 @@ static void psr_cpu_init(void)
>  
>          feat = feat_l2_cat;
>          feat_l2_cat = NULL;
> -        if ( !cat_init_feature(&regs, feat, info, FEAT_TYPE_L2_CAT) )
> +        if ( cat_init_feature(&regs, feat, info, FEAT_TYPE_L2_CAT) )

All three changes above belong into patch 4 afaict.

Jan


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v6 06/16] x86: implement get hw info flow for MBA
  2017-10-08  7:23 ` [PATCH v6 06/16] x86: implement get hw info " Yi Sun
@ 2017-10-11 13:12   ` Jan Beulich
  0 siblings, 0 replies; 26+ messages in thread
From: Jan Beulich @ 2017-10-11 13:12 UTC (permalink / raw)
  To: Yi Sun; +Cc: Andrew Cooper, xen-devel, Wei Liu, Chao Peng, Roger Pau Monné

>>> On 08.10.17 at 09:23, <yi.y.sun@linux.intel.com> wrote:
> This patch implements get HW info flow for MBA including its callback
> function and sysctl interface.
> 
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with one further adjustment:

> --- a/xen/include/asm-x86/psr.h
> +++ b/xen/include/asm-x86/psr.h
> @@ -39,6 +39,8 @@
>  #define PSR_INFO_IDX_COS_MAX            0
>  #define PSR_INFO_IDX_CAT_CBM_LEN        1
>  #define PSR_INFO_IDX_CAT_FLAG           2
> +#define PSR_INFO_IDX_MBA_THRTL_MAX      1
> +#define PSR_INFO_IDX_MBA_FLAG           2

PSR_INFO_IDX_MBA_FLAGS please, even if right now there's only
one flag.

The CAT equivalent wants changing too (perhaps in an earlier
patch).

Jan

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v6 08/16] x86: implement set value flow for MBA
  2017-10-08  7:23 ` [PATCH v6 08/16] x86: implement set value flow " Yi Sun
@ 2017-10-11 13:38   ` Jan Beulich
  2017-10-12  4:33     ` Yi Sun
  0 siblings, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2017-10-11 13:38 UTC (permalink / raw)
  To: Yi Sun; +Cc: Andrew Cooper, xen-devel, Wei Liu, Chao Peng, Roger Pau Monné

>>> On 08.10.17 at 09:23, <yi.y.sun@linux.intel.com> wrote:
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -138,6 +138,12 @@ static const struct feat_props {
>  
>      /* write_msr is used to write out feature MSR register. */
>      void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
> +
> +    /*
> +     * check_val is used to check if input val fulfills SDM requirement.
> +     * Change it to valid value if SDM allows.
> +     */
> +    bool (*check_val)(const struct feat_node *feat, unsigned long *val);

I'm pretty sure I've said so before - "check" to me implies all r/o
inputs. Perhaps sanitize_val() or even just sanitize()?

And why unsigned long when the only caller has a uint32_t in its
hands?

> @@ -274,30 +280,30 @@ static enum psr_feat_type psr_type_to_feat_type(enum psr_type type)
>      return feat_type;
>  }
>  
> -static bool psr_check_cbm(unsigned int cbm_len, unsigned long cbm)
> +/* Implementation of allocation features' functions. */
> +static bool cat_check_cbm(const struct feat_node *feat, unsigned long *cbm)
>  {
>      unsigned int first_bit, zero_bit;
> +    unsigned int cbm_len = feat->cat.cbm_len;
>  
> -    /* Set bits should only in the range of [0, cbm_len]. */
> -    if ( cbm & (~0ul << cbm_len) )
> -        return false;
> -
> -    /* At least one bit need to be set. */
> -    if ( cbm == 0 )
> +    /*
> +     * Set bits should only in the range of [0, cbm_len].

As you alter the comment anyway, please also add the missing "be".
Also - isn't the upper bound of the range exclusive, i.e. shouldn't
this be [0, cbm_len)?

> +     * And, at least one bit need to be set.
> +     */
> +    if ( *cbm & (~0ul << cbm_len) || *cbm == 0 )

Parentheses missing for the left side operand of || and if you omit
!= 0 on the left part (which I appreciate) please also use ! instead
of == 0 on the right side.

> +static bool mba_check_thrtl(const struct feat_node *feat, unsigned long *thrtl)
> +{
> +    if ( *thrtl > feat->mba.thrtl_max )
> +        return false;
> +
> +    /*
> +     * Per SDM (chapter "Memory Bandwidth Allocation Configuration"):
> +     * 1. Linear mode: In the linear mode the input precision is defined
> +     *    as 100-(MBA_MAX). For instance, if the MBA_MAX value is 90, the
> +     *    input precision is 10%. Values not an even multiple of the
> +     *    precision (e.g., 12%) will be rounded down (e.g., to 10% delay
> +     *    applied).
> +     * 2. Non-linear mode: Input delay values are powers-of-two from zero
> +     *    to the MBA_MAX value from CPUID. In this case any values not a
> +     *    power of two will be rounded down the next nearest power of two.
> +     */
> +    if ( feat->mba.linear )
> +    {
> +        unsigned int mod;
> +
> +        if ( feat->mba.thrtl_max >= 100 )
> +            return false;

Don't you check this right after collecting CPUID output? If so,
this should be at most an ASSERT().

> +        mod = *thrtl % (100 - feat->mba.thrtl_max);
> +        *thrtl -= mod;

Do you really need the intermediate variable?

> +    }
> +    else
> +    {
> +        /* Not power of 2. */
> +        if ( *thrtl & (*thrtl - 1) )
> +            *thrtl = *thrtl & (1 << (flsl(*thrtl) - 1));

&= or even simply =.

> @@ -950,6 +997,7 @@ static int insert_val_into_array(uint32_t val[],
>      const struct feat_node *feat;
>      const struct feat_props *props;
>      unsigned int i;
> +    unsigned long check_val = new_val;
>      int ret;
>  
>      ASSERT(feat_type < FEAT_TYPE_NUM);
> @@ -974,9 +1022,11 @@ static int insert_val_into_array(uint32_t val[],
>      if ( array_len < props->cos_num )
>          return -ENOSPC;
>  
> -    if ( !psr_check_cbm(feat->cat.cbm_len, new_val) )
> +    if ( !props->check_val(feat, &check_val) )
>          return -EINVAL;
>  
> +    new_val = check_val;

When the function pointer's parameter changes to uint32_t *
you won't need the intermediate variable anymore afaict.

Jan


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v6 08/16] x86: implement set value flow for MBA
  2017-10-11 13:38   ` Jan Beulich
@ 2017-10-12  4:33     ` Yi Sun
  2017-10-12  9:43       ` Jan Beulich
  0 siblings, 1 reply; 26+ messages in thread
From: Yi Sun @ 2017-10-12  4:33 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Andrew Cooper, xen-devel, Wei Liu, Chao Peng, Roger Pau Monné

On 17-10-11 07:38:52, Jan Beulich wrote:
> >>> On 08.10.17 at 09:23, <yi.y.sun@linux.intel.com> wrote:
> > --- a/xen/arch/x86/psr.c
> > +++ b/xen/arch/x86/psr.c
> > @@ -138,6 +138,12 @@ static const struct feat_props {
> >  
> >      /* write_msr is used to write out feature MSR register. */
> >      void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
> > +
> > +    /*
> > +     * check_val is used to check if input val fulfills SDM requirement.
> > +     * Change it to valid value if SDM allows.
> > +     */
> > +    bool (*check_val)(const struct feat_node *feat, unsigned long *val);
> 
> I'm pretty sure I've said so before - "check" to me implies all r/o
> inputs. Perhaps sanitize_val() or even just sanitize()?
> 
> And why unsigned long when the only caller has a uint32_t in its
> hands?
> 
To be compatible with cat_check_cbm (old name is 'psr_check_cbm' in L2 series),
the last parameter type is 'unsigned long'. We have discussed it in L2 patch set
v9, patch 10.

> > @@ -274,30 +280,30 @@ static enum psr_feat_type psr_type_to_feat_type(enum psr_type type)
> >      return feat_type;
> >  }
> >  
> > -static bool psr_check_cbm(unsigned int cbm_len, unsigned long cbm)
> > +/* Implementation of allocation features' functions. */
> > +static bool cat_check_cbm(const struct feat_node *feat, unsigned long *cbm)
> >  {
> >      unsigned int first_bit, zero_bit;
> > +    unsigned int cbm_len = feat->cat.cbm_len;
> >  
> > -    /* Set bits should only in the range of [0, cbm_len]. */
> > -    if ( cbm & (~0ul << cbm_len) )
> > -        return false;
> > -
> > -    /* At least one bit need to be set. */
> > -    if ( cbm == 0 )
> > +    /*
> > +     * Set bits should only in the range of [0, cbm_len].
> 
> As you alter the comment anyway, please also add the missing "be".
> Also - isn't the upper bound of the range exclusive, i.e. shouldn't
> this be [0, cbm_len)?
> 
Thanks!

> > +     * And, at least one bit need to be set.
> > +     */
> > +    if ( *cbm & (~0ul << cbm_len) || *cbm == 0 )
> 
> Parentheses missing for the left side operand of || and if you omit
> != 0 on the left part (which I appreciate) please also use ! instead
> of == 0 on the right side.
> 
Got it.

> > +static bool mba_check_thrtl(const struct feat_node *feat, unsigned long *thrtl)
> > +{
> > +    if ( *thrtl > feat->mba.thrtl_max )
> > +        return false;
> > +
> > +    /*
> > +     * Per SDM (chapter "Memory Bandwidth Allocation Configuration"):
> > +     * 1. Linear mode: In the linear mode the input precision is defined
> > +     *    as 100-(MBA_MAX). For instance, if the MBA_MAX value is 90, the
> > +     *    input precision is 10%. Values not an even multiple of the
> > +     *    precision (e.g., 12%) will be rounded down (e.g., to 10% delay
> > +     *    applied).
> > +     * 2. Non-linear mode: Input delay values are powers-of-two from zero
> > +     *    to the MBA_MAX value from CPUID. In this case any values not a
> > +     *    power of two will be rounded down the next nearest power of two.
> > +     */
> > +    if ( feat->mba.linear )
> > +    {
> > +        unsigned int mod;
> > +
> > +        if ( feat->mba.thrtl_max >= 100 )
> > +            return false;
> 
> Don't you check this right after collecting CPUID output? If so,
> this should be at most an ASSERT().
> 
Yes, I have checked this in mba_init_feature. Will remove this check.

> > +        mod = *thrtl % (100 - feat->mba.thrtl_max);
> > +        *thrtl -= mod;
> 
> Do you really need the intermediate variable?
> 
Will remove 'mod'.

> > +    }
> > +    else
> > +    {
> > +        /* Not power of 2. */
> > +        if ( *thrtl & (*thrtl - 1) )
> > +            *thrtl = *thrtl & (1 << (flsl(*thrtl) - 1));
> 
> &= or even simply =.
> 
Ok, thanks!

> > @@ -950,6 +997,7 @@ static int insert_val_into_array(uint32_t val[],
> >      const struct feat_node *feat;
> >      const struct feat_props *props;
> >      unsigned int i;
> > +    unsigned long check_val = new_val;
> >      int ret;
> >  
> >      ASSERT(feat_type < FEAT_TYPE_NUM);
> > @@ -974,9 +1022,11 @@ static int insert_val_into_array(uint32_t val[],
> >      if ( array_len < props->cos_num )
> >          return -ENOSPC;
> >  
> > -    if ( !psr_check_cbm(feat->cat.cbm_len, new_val) )
> > +    if ( !props->check_val(feat, &check_val) )
> >          return -EINVAL;
> >  
> > +    new_val = check_val;
> 
> When the function pointer's parameter changes to uint32_t *
> you won't need the intermediate variable anymore afaict.
> 
> Jan

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v6 08/16] x86: implement set value flow for MBA
  2017-10-12  4:33     ` Yi Sun
@ 2017-10-12  9:43       ` Jan Beulich
  2017-10-13  2:02         ` Yi Sun
  0 siblings, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2017-10-12  9:43 UTC (permalink / raw)
  To: Yi Sun; +Cc: Andrew Cooper, xen-devel, Wei Liu, Chao Peng, Roger Pau Monné

>>> On 12.10.17 at 06:33, <yi.y.sun@linux.intel.com> wrote:
> On 17-10-11 07:38:52, Jan Beulich wrote:
>> >>> On 08.10.17 at 09:23, <yi.y.sun@linux.intel.com> wrote:
>> > --- a/xen/arch/x86/psr.c
>> > +++ b/xen/arch/x86/psr.c
>> > @@ -138,6 +138,12 @@ static const struct feat_props {
>> >  
>> >      /* write_msr is used to write out feature MSR register. */
>> >      void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
>> > +
>> > +    /*
>> > +     * check_val is used to check if input val fulfills SDM requirement.
>> > +     * Change it to valid value if SDM allows.
>> > +     */
>> > +    bool (*check_val)(const struct feat_node *feat, unsigned long *val);
>> 
>> I'm pretty sure I've said so before - "check" to me implies all r/o
>> inputs. Perhaps sanitize_val() or even just sanitize()?
>> 
>> And why unsigned long when the only caller has a uint32_t in its
>> hands?
>> 
> To be compatible with cat_check_cbm (old name is 'psr_check_cbm' in L2 series),
> the last parameter type is 'unsigned long'. We have discussed it in L2 patch set
> v9, patch 10.

Iirc (without checking the old thread) this was for calculations to
be done as unsigned long ones. If that's the only aspect here,
then imo this is not a valid reason for the hook's parameter type
to be unsigned long *.

Jan


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v6 08/16] x86: implement set value flow for MBA
  2017-10-12  9:43       ` Jan Beulich
@ 2017-10-13  2:02         ` Yi Sun
  2017-10-13  6:56           ` Jan Beulich
  0 siblings, 1 reply; 26+ messages in thread
From: Yi Sun @ 2017-10-13  2:02 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Andrew Cooper, xen-devel, Wei Liu, Chao Peng, Roger Pau Monné

On 17-10-12 03:43:26, Jan Beulich wrote:
> >>> On 12.10.17 at 06:33, <yi.y.sun@linux.intel.com> wrote:
> > On 17-10-11 07:38:52, Jan Beulich wrote:
> >> >>> On 08.10.17 at 09:23, <yi.y.sun@linux.intel.com> wrote:
> >> > --- a/xen/arch/x86/psr.c
> >> > +++ b/xen/arch/x86/psr.c
> >> > @@ -138,6 +138,12 @@ static const struct feat_props {
> >> >  
> >> >      /* write_msr is used to write out feature MSR register. */
> >> >      void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
> >> > +
> >> > +    /*
> >> > +     * check_val is used to check if input val fulfills SDM requirement.
> >> > +     * Change it to valid value if SDM allows.
> >> > +     */
> >> > +    bool (*check_val)(const struct feat_node *feat, unsigned long *val);
> >> 
> >> I'm pretty sure I've said so before - "check" to me implies all r/o
> >> inputs. Perhaps sanitize_val() or even just sanitize()?
> >> 
> >> And why unsigned long when the only caller has a uint32_t in its
> >> hands?
> >> 
> > To be compatible with cat_check_cbm (old name is 'psr_check_cbm' in L2 series),
> > the last parameter type is 'unsigned long'. We have discussed it in L2 patch set
> > v9, patch 10.
> 
> Iirc (without checking the old thread) this was for calculations to
> be done as unsigned long ones. If that's the only aspect here,
> then imo this is not a valid reason for the hook's parameter type
> to be unsigned long *.
> 
Because below macros used in cat_check_cbm require the input addr to be unsigned
long, we define the last parameter of cat_check_cbm to be unsigned long.
    find_first_bit
    find_next_zero_bit
    find_next_bit

If you think the unsigned long is not appropriate for 'check_val', I think I
have to define a local variable in cat_check_cbm to do the convertion.

> Jan

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v6 08/16] x86: implement set value flow for MBA
  2017-10-13  2:02         ` Yi Sun
@ 2017-10-13  6:56           ` Jan Beulich
  0 siblings, 0 replies; 26+ messages in thread
From: Jan Beulich @ 2017-10-13  6:56 UTC (permalink / raw)
  To: Yi Sun; +Cc: Andrew Cooper, xen-devel, Wei Liu, Chao Peng, Roger Pau Monné

>>> On 13.10.17 at 04:02, <yi.y.sun@linux.intel.com> wrote:
> On 17-10-12 03:43:26, Jan Beulich wrote:
>> >>> On 12.10.17 at 06:33, <yi.y.sun@linux.intel.com> wrote:
>> > On 17-10-11 07:38:52, Jan Beulich wrote:
>> >> >>> On 08.10.17 at 09:23, <yi.y.sun@linux.intel.com> wrote:
>> >> > --- a/xen/arch/x86/psr.c
>> >> > +++ b/xen/arch/x86/psr.c
>> >> > @@ -138,6 +138,12 @@ static const struct feat_props {
>> >> >  
>> >> >      /* write_msr is used to write out feature MSR register. */
>> >> >      void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
>> >> > +
>> >> > +    /*
>> >> > +     * check_val is used to check if input val fulfills SDM requirement.
>> >> > +     * Change it to valid value if SDM allows.
>> >> > +     */
>> >> > +    bool (*check_val)(const struct feat_node *feat, unsigned long *val);
>> >> 
>> >> I'm pretty sure I've said so before - "check" to me implies all r/o
>> >> inputs. Perhaps sanitize_val() or even just sanitize()?
>> >> 
>> >> And why unsigned long when the only caller has a uint32_t in its
>> >> hands?
>> >> 
>> > To be compatible with cat_check_cbm (old name is 'psr_check_cbm' in L2 series),
>> > the last parameter type is 'unsigned long'. We have discussed it in L2 patch set
>> > v9, patch 10.
>> 
>> Iirc (without checking the old thread) this was for calculations to
>> be done as unsigned long ones. If that's the only aspect here,
>> then imo this is not a valid reason for the hook's parameter type
>> to be unsigned long *.
>> 
> Because below macros used in cat_check_cbm require the input addr to be 
> unsigned
> long, we define the last parameter of cat_check_cbm to be unsigned long.
>     find_first_bit
>     find_next_zero_bit
>     find_next_bit
> 
> If you think the unsigned long is not appropriate for 'check_val', I think I
> have to define a local variable in cat_check_cbm to do the convertion.

Exactly - the use of unsigned long is specific to this function, not
generic for all implementations of the hook. The parameter type
change back then was only a simple way to avoid defining another
local variable.

Jan


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

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2017-10-13  6:56 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-08  7:23 [PATCH v6 00/16] Enable Memory Bandwidth Allocation in Xen Yi Sun
2017-10-08  7:23 ` [PATCH v6 01/16] docs: create Memory Bandwidth Allocation (MBA) feature document Yi Sun
2017-10-08  7:23 ` [PATCH v6 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general Yi Sun
2017-10-10 14:59   ` Jan Beulich
2017-10-08  7:23 ` [PATCH v6 03/16] x86: rename 'cbm_type' to 'psr_type' to make it general Yi Sun
2017-10-08  7:23 ` [PATCH v6 04/16] x86: a few optimizations to psr codes Yi Sun
2017-10-10 15:01   ` Jan Beulich
2017-10-08  7:23 ` [PATCH v6 05/16] x86: implement data structure and CPU init flow for MBA Yi Sun
2017-10-10 15:26   ` Jan Beulich
2017-10-08  7:23 ` [PATCH v6 06/16] x86: implement get hw info " Yi Sun
2017-10-11 13:12   ` Jan Beulich
2017-10-08  7:23 ` [PATCH v6 07/16] x86: implement get value interface " Yi Sun
2017-10-08  7:23 ` [PATCH v6 08/16] x86: implement set value flow " Yi Sun
2017-10-11 13:38   ` Jan Beulich
2017-10-12  4:33     ` Yi Sun
2017-10-12  9:43       ` Jan Beulich
2017-10-13  2:02         ` Yi Sun
2017-10-13  6:56           ` Jan Beulich
2017-10-08  7:23 ` [PATCH v6 09/16] tools: create general interfaces to support psr allocation features Yi Sun
2017-10-08  7:23 ` [PATCH v6 10/16] tools: implement the new libxc get hw info interface Yi Sun
2017-10-08  7:23 ` [PATCH v6 11/16] tools: implement the new libxl " Yi Sun
2017-10-08  7:23 ` [PATCH v6 12/16] tools: implement the new xl " Yi Sun
2017-10-08  7:23 ` [PATCH v6 13/16] tools: rename 'xc_psr_cat_type' to 'xc_psr_type' Yi Sun
2017-10-08  7:23 ` [PATCH v6 14/16] tools: implement new generic get value interface and MBA get value command Yi Sun
2017-10-08  7:24 ` [PATCH v6 15/16] tools: implement new generic set value interface and MBA set " Yi Sun
2017-10-08  7:24 ` [PATCH v6 16/16] docs: add MBA description in docs Yi Sun

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.