Re: [PATCH v5 5/5] x86/kvm: Avoid dynamic allocation of pvclock data when SEV is active

[Brijesh Singh]

On 09/06/2018 09:07 AM, Sean Christopherson wrote:
...

+
+/* This should cover upto 512 VCPUS (first 64 are covered by hv_clock_boot[]). 
*/
+#define HVC_DECRYPTED_ARRAY_SIZE \
+       ((PAGE_SIZE * 7)  / sizeof(struct pvclock_vsyscall_time_info))

I think we can define the size relative to NR_CPUS rather than picking
an arbitrary number of pages, maybe with a BUILD_BUG_ON to make sure
the total size won't require a second 2mb page for __decrpyted.

#define HVC_DECRYPTED_ARRAY_SIZE  \
        PAGE_ALIGN((NR_CPUS - HVC_BOOT_ARRAY_SIZE) * \
                   sizeof(struct pvclock_vsyscall_time_info))

Sure works for me.

+static struct pvclock_vsyscall_time_info
+                       hv_clock_dec[HVC_DECRYPTED_ARRAY_SIZE] 
__decrypted_hvclock;
+
  static inline struct pvclock_vcpu_time_info *this_cpu_pvti(void)
  {
        return &this_cpu_read(hv_clock_per_cpu)->pvti;
@@ -267,10 +274,19 @@ static int kvmclock_setup_percpu(unsigned int cpu)
                return 0;

/* Use the static page for the first CPUs, allocate otherwise */

-       if (cpu < HVC_BOOT_ARRAY_SIZE)
+       if (cpu < HVC_BOOT_ARRAY_SIZE) {
                p = &hv_clock_boot[cpu];
-       else
-               p = kzalloc(sizeof(*p), GFP_KERNEL);
+       } else {
+               /*
+                * When SEV is active, use the static pages from
+                * .data..decrypted_hvclock section. The pages are already
+                * mapped with C=0.
+                */
+               if (sev_active())
+                       p = &hv_clock_dec[cpu - HVC_BOOT_ARRAY_SIZE];
+               else
+                       p = kzalloc(sizeof(*p), GFP_KERNEL);
+       }

Personal preference, but I think an if-elif-else with a single block
comment would be easier to read.

I can do with that. thanks for your feedback.

        /*
         * Blah blah blah
         */
        if (cpu < HVC_BOOT_ARRAY_SIZE)
                p = &hv_clock_boot[cpu];
        else if (sev_active())
                p = &hv_clock_dec[cpu - HVC_BOOT_ARRAY_SIZE];
        else
                p = kzalloc(sizeof(*p), GFP_KERNEL);