Re: [PATCH V6 8/8] hv: vCAT: propagate vCBM to other vCPUs that share cache with vcpu


Eddie Dong
 

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of Dongsheng Zhang
Sent: Thursday, October 21, 2021 1:21 AM
To: acrn-dev@...
Subject: Re: [acrn-dev] [PATCH V6 8/8] hv: vCAT: propagate vCBM to other
vCPUs that share cache with vcpu



-----Original Message-----
From: acrn-dev@... <acrn-dev@...>
On Behalf Of Eddie Dong
Sent: Wednesday, October 20, 2021 6:11 AM
To: acrn-dev@...
Subject: Re: [acrn-dev] [PATCH V6 8/8] hv: vCAT: propagate vCBM to
other vCPUs that share cache with vcpu



-----Original Message-----
From: acrn-dev@...
<acrn-dev@...> On Behalf Of Dongsheng Zhang
Sent: Wednesday, October 20, 2021 6:42 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH V6 8/8] hv: vCAT: propagate vCBM to other
vCPUs that share cache with vcpu

From: dongshen <dongsheng.x.zhang@...>

Move the nearest_pow2() and get_cache_shift() functions from
hypercall.c to cpu_caps.c Store L2/L3 cat id shift in struct
cpuinfo_x86

Implement the propagate_vcbm() function:
Set vCBM to to all the vCPUs that share cache with vcpu
to mimic hardware CAT behavior

Tracked-On: #5917
Signed-off-by: dongshen <dongsheng.x.zhang@...>
---
hypervisor/arch/x86/cpu_caps.c | 55
+++++++++++++++++++
hypervisor/arch/x86/guest/vcat.c | 62
+++++++++++++++++++++-
hypervisor/common/hypercall.c | 50 ++---------------
hypervisor/include/arch/x86/asm/cpu_caps.h | 4 ++
4 files changed, 122 insertions(+), 49 deletions(-)

diff --git a/hypervisor/arch/x86/cpu_caps.c
b/hypervisor/arch/x86/cpu_caps.c index 89fd559b1..33ea3aa36 100644
--- a/hypervisor/arch/x86/cpu_caps.c
+++ b/hypervisor/arch/x86/cpu_caps.c
@@ -16,6 +16,7 @@
#include <errno.h>
#include <logmsg.h>
#include <asm/guest/vmcs.h>
+#include <asm/lib/bits.h>

/* TODO: add more capability per requirement */
/* APICv features */
@@ -322,6 +323,58 @@ static uint64_t get_address_mask(uint8_t limit)
return ((1UL << limit) - 1UL) & PAGE_MASK; }

+/*
+ * nearest_pow2(n) is the nearest power of 2 integer that is not
+less than n
+ * The last (most significant) bit set of (n*2-1) matches the above
+definition */ static uint32_t nearest_pow2(uint32_t n) {
+ uint32_t p = n;
+
+ if (n >= 2U) {
+ p = fls32(2U*n - 1U);
+ }
+
+ return p;
+}
+
+static void get_cat_id_shift(uint32_t *l2_cat_id_shift, uint32_t
+*l3_cat_id_shift) {
What does cat id mean?
Or are u defining a new term cache-id-shift?
Cannot understand.

I guess you are saying: cache_id? Cache_type? Cache_level? I am lost
Don: I mean l2/l3 cache id
If cache ID is an official term, why you need to invent a new name?


+ uint32_t subleaf;
+
+ *l2_cat_id_shift = 0U;
+ *l3_cat_id_shift = 0U;
+
+ for (subleaf = 0U;; subleaf++) {
+ uint32_t eax, ebx, ecx, edx;
+ uint32_t cache_type, cache_level, id, shift;
+
+ cpuid_subleaf(0x4U, subleaf, &eax, &ebx, &ecx, &edx);
+
+ cache_type = eax & 0x1fU;
+ cache_level = (eax >> 5U) & 0x7U;
+
+ /* Intel SDM Vol 2, CPUID 04H:
+ * EAX: bits 25 - 14: Maximum number of addressable IDs for
logical
processors sharing this cache.
+ * The nearest power-of-2 integer that is not smaller than (1
+
EAX[25:14]) is the number of unique
+ * initial APIC IDs reserved for addressing different logical
processors sharing this cache
+ */
+ id = (eax >> 14U) & 0xfffU;
+ shift = nearest_pow2(id + 1U);
+
+ /* No more caches */
+ if ((cache_type == 0U) || (cache_type >= 4U)) {
+ break;
+ }
+
+ if (cache_level == 2U) {
+ *l2_cat_id_shift = shift;
+ } else if (cache_level == 3U) {
+ *l3_cat_id_shift = shift;
+ }
+ }
+}
+
void init_pcpu_capabilities(void)
{
uint32_t eax, unused;
@@ -385,6 +438,8 @@ void init_pcpu_capabilities(void)
get_address_mask(boot_cpu_data.phys_bits);
}

+ get_cat_id_shift(&boot_cpu_data.l2_cat_id_shift,
+&boot_cpu_data.l3_cat_id_shift);
+
detect_pcpu_cap();
}

diff --git a/hypervisor/arch/x86/guest/vcat.c
b/hypervisor/arch/x86/guest/vcat.c
index 1dda6b61f..9e302b25e 100644
--- a/hypervisor/arch/x86/guest/vcat.c
+++ b/hypervisor/arch/x86/guest/vcat.c
@@ -410,6 +410,55 @@ static uint32_t vmsr_to_pmsr(const struct
acrn_vm
*vm, uint32_t vmsr, int res)
return pmsr;
}

+static void get_vcat_id_shift(uint32_t *l2_shift, uint32_t *l3_shift) {
+ struct cpuinfo_x86 *cpu_info = get_pcpu_info();
+
+ /* Assume that virtual cat id shift is equal to physical cat id
+shift for now
*/
+ *l2_shift = cpu_info->l2_cat_id_shift;
+ *l3_shift = cpu_info->l3_cat_id_shift; }
+
+/**
+ * @brief Propagate vCBM to other vCPUs that share cache with vcpu
+ * @pre vcpu != NULL && vcpu->vm != NULL */ static void
+propagate_vcbm(struct acrn_vcpu *vcpu, uint32_t vmsr, uint64_t val) {
+ uint16_t i;
+ struct acrn_vcpu *tmp_vcpu;
+ uint32_t l2_shift, l3_shift, l2_id, l3_id;
+ struct acrn_vm *vm = vcpu->vm;
+ uint32_t apicid = vlapic_get_apicid(vcpu_vlapic(vcpu));
+
+ get_vcat_id_shift(&l2_shift, &l3_shift);


+ /* Intel SDM Vol 2, CPUID 04H:
+ * EAX: bits 25 - 14: Maximum number of addressable IDs for
+logical
processors sharing this cache.
+ *
+ * l2_shift/l3_shift: the nearest power-of-2 integer that is not
+smaller
than (1 + EAX[25:14])
+ * is the number of unique initial APIC IDs reserved for
+addressing
different logical processors
+ * sharing this cache
+ */
+ l2_id = apicid >> l2_shift;
+ l3_id = apicid >> l3_shift;
+
+ /*
+ * Determine which logical processors share an MSR (for instance
local
+ * to a core, or shared across multiple cores) by checking if they
+have the
same
+ * L2/L3 cache id
+ */
+ foreach_vcpu(i, vm, tmp_vcpu) {
+ uint32_t tmp_apicid =
vlapic_get_apicid(vcpu_vlapic(tmp_vcpu));
+ uint32_t tmp_l2_id = tmp_apicid >> l2_shift;
+ uint32_t tmp_l3_id = tmp_apicid >> l3_shift;
+
This patch is becoming worse than previous version.

We want to propagate among VCPUs with same cache ID. All what we
compare is among virtual cache ID.
However this code is comparing between P and V.
Don: this is following our email discussion, it is compare V to V:
Maybe we should add one API to get virtual cache shift in vcat.c, like
get_vcache_shift, you can make it call get_cache_shift directly.
And get_cache_shift should be one low level API, maybe cpu_caps.c should be
a good place?

get_vcat_id_shift(): returns the virtual cache shift (internally we assume
virtual cache shift is equal to physical cache shift), so it is just a wrapper.
Rollback to previous version, and carefully fix the comment of previous version.






+ if ((is_l2_vcbm_msr(vm, vmsr) && (l2_id == tmp_l2_id))
+ || (is_l3_vcbm_msr(vm, vmsr) && (l3_id ==
tmp_l3_id))) {
+ vcpu_set_guest_msr(tmp_vcpu, vmsr, val);
+ }
+ }
+}
+
static void write_pcbm(uint32_t pmsr, uint64_t pcbm) {
/* Preserve reserved bits, and only set the pCBM bits */ @@ -461,8
+510,17 @@ static int32_t write_vcbm(struct acrn_vcpu *vcpu,
+uint32_t
vmsr, uint64_t val, i
uint32_t pmsr;
uint64_t pcbm;

- /* Write vCBM first */
- vcpu_set_guest_msr(vcpu, vmsr, masked_vcbm | (val &
0xFFFFFFFF00000000UL));
+ /*
+ * Write vCBM first:
+ * The L2 mask MSRs are scoped at the same level as the L2
cache
(similarly,
+ * the L3 mask MSRs are scoped at the same level as the L3
cache).
+ *
+ * For example, the MSR_IA32_L3_MASK_n MSRs are scoped
at
socket level, which means if
+ * we program MSR_IA32_L3_MASK_n on one cpu and the
same
MSR_IA32_L3_MASK_n on all other cpus
+ * of the same socket will also get the change!
+ * Set vcbm to all the vCPUs that share cache with vcpu to
mimic
this hardware behavior.
+ */
+ propagate_vcbm(vcpu, vmsr, masked_vcbm | (val &
+0xFFFFFFFF00000000UL));

/* Write pCBM: */
pmsr = vmsr_to_pmsr(vcpu->vm, vmsr, res); diff --git
a/hypervisor/common/hypercall.c b/hypervisor/common/hypercall.c
index
fb19fe4a0..3f5134d45 100644
--- a/hypervisor/common/hypercall.c
+++ b/hypervisor/common/hypercall.c
@@ -133,52 +133,6 @@ int32_t hcall_get_api_version(struct acrn_vcpu
*vcpu, __unused struct acrn_vm *t
return copy_to_gpa(vcpu->vm, &version, param1, sizeof(version));
}

-/*
- * nearest_pow2(n) is the nearest power of 2 integer that is not
less than n
- * The last (most significant) bit set of (n*2-1) matches the above
definition
- */
-static uint32_t nearest_pow2(uint32_t n) -{
- uint32_t p = n;
-
- if (n >= 2U) {
- p = fls32(2U*n - 1U);
- }
-
- return p;
-}
-
-static void get_cache_shift(uint32_t *l2_shift, uint32_t *l3_shift) -{
- uint32_t subleaf;
-
- *l2_shift = 0U;
- *l3_shift = 0U;
-
- for (subleaf = 0U;; subleaf++) {
- uint32_t eax, ebx, ecx, edx;
- uint32_t cache_type, cache_level, id, shift;
-
- cpuid_subleaf(0x4U, subleaf, &eax, &ebx, &ecx, &edx);
-
- cache_type = eax & 0x1fU;
- cache_level = (eax >> 5U) & 0x7U;
- id = (eax >> 14U) & 0xfffU;
- shift = nearest_pow2(id + 1U);
-
- /* No more caches */
- if ((cache_type == 0U) || (cache_type >= 4U)) {
- break;
- }
-
- if (cache_level == 2U) {
- *l2_shift = shift;
- } else if (cache_level == 3U) {
- *l3_shift = shift;
- }
- }
-}
-
/**
* @brief Get basic platform information.
*
@@ -204,8 +158,10 @@ int32_t hcall_get_platform_info(struct
acrn_vcpu *vcpu, __unused struct acrn_vm
if (ret == 0) {
uint16_t i;
uint16_t pcpu_nums = get_pcpu_nums();
+ struct cpuinfo_x86 *cpu_info = get_pcpu_info();

- get_cache_shift(&pi.hw.l2_cat_shift, &pi.hw.l3_cat_shift);
+ pi.hw.l2_cat_shift = cpu_info->l2_cat_id_shift;
+ pi.hw.l3_cat_shift = cpu_info->l3_cat_id_shift;

for (i = 0U; i < min(pcpu_nums,
ACRN_PLATFORM_LAPIC_IDS_MAX);
i++) {
pi.hw.lapic_ids[i] = per_cpu(lapic_id, i); diff --git
a/hypervisor/include/arch/x86/asm/cpu_caps.h
b/hypervisor/include/arch/x86/asm/cpu_caps.h
index 4f679f209..c22e306a0 100644
--- a/hypervisor/include/arch/x86/asm/cpu_caps.h
+++ b/hypervisor/include/arch/x86/asm/cpu_caps.h
@@ -43,6 +43,10 @@ struct cpuinfo_x86 {
uint64_t physical_address_mask;
uint32_t cpuid_leaves[FEATURE_WORDS];
char model_name[64];
+ /* Right-shift count that will allow software to extract part of
+APIC ID to
distinguish L2 CAT ID */
+ uint32_t l2_cat_id_shift;
+ /* Right-shift count that will allow software to extract part of
+APIC ID to
distinguish L3 CAT ID */
+ uint32_t l3_cat_id_shift;
};

bool has_monitor_cap(void);
--
2.25.1










Join acrn-dev@lists.projectacrn.org to automatically receive all group messages.