Thank you Jan for your feedback.

From what I understood, Andrew is planning some "ground" changes

on CPUID leaf discovery, so it probably will move here.  I'll wait for his

changes to apply your and Andrew's remarks and to re-post the patches.

Andrei.

On 4/18/24 10:13, Jan Beulich wrote:
On 10.04.2024 17:36, Andrei Semenov wrote:
Signed-off-by: Andrei Semenov <andrei.seme...@vates.fr>
A couple more nits on top of what Andrew said. First: Please no patches
(which aren't blindingly trivial) without description.

@@ -1030,6 +1031,54 @@ static void amd_check_erratum_1485(void)
        wrmsrl(MSR_AMD64_BP_CFG, val | chickenbit);
  }

+#ifdef CONFIG_HVM
+static void amd_enable_mem_encrypt(const struct cpuinfo_x86 *c)
+{
+       unsigned int  eax, ebx, ecx, edx;
+       uint64_t syscfg;
+
+       if (!smp_processor_id()) {
+
+               cpuid_count(0x80000000,0,&eax, &ebx, &ecx, &edx);
No blank line above here please.

+               if (eax <  0x8000001f)
+                       return;
+
+               cpuid_count(0x8000001f,0,&eax, &ebx, &ecx, &edx);
+
+               if (eax & 0x1)
+                       setup_force_cpu_cap(X86_FEATURE_SME);
+
+               if (eax & 0x2) {
+                       setup_force_cpu_cap(X86_FEATURE_SEV);
I guess this goes along with what Andrew said: Using synthetic features here
looks suspicious. These want to be recorded as an ordinary leaf.

+                       max_sev_asid = ecx;
+                       min_sev_asid = edx;
+               }
+
+               if (eax & 0x3)
+                       pte_c_bit_mask = 1UL << (ebx & 0x3f);
+       }
+
+       if (!(cpu_has_sme || cpu_has_sev))
+               return;
+
+       if (!smp_processor_id()) {
+               if (cpu_has_sev)
Two if()-s like these want folding, unless it is made clear that very
soon (see above as to the missing description) further content is going
to appear inside the outer one.

+                       printk(XENLOG_INFO "SEV: ASID range [0x%x - 0x%x]\n",
%#x is preferred over 0x%x.

+                       min_sev_asid, max_sev_asid);
+       }
+
+       rdmsrl(MSR_K8_SYSCFG, syscfg);
+
+       if (syscfg & SYSCFG_MEM_ENCRYPT) {
+               return;
+       }
No need for braces in cases like this one.

Jan


Reply via email to