On 10/16/17 5:24 PM, Borislav Petkov wrote:
...
>>
>> +static inline void __set_percpu_decrypted(void *ptr, unsigned long size)
>> +{
>> + early_set_memory_decrypted(slow_virt_to_phys(ptr), size);
>> +}
> Ok, so this looks like useless conversion:
>
> you pass in a virtual address, it gets converted to a physical address
> with slow_virt_to_phys() and then in early_set_memory_enc_dec() gets
> converted to a virtual address again.
>
> Why? Why not pass the virtual address directly?
Actually, I worked to enable the kvmclock support before the
kvm-stealtime, eoi and apf_reason. The kvmclock uses memblock_alloc() to
allocate the shared memory and since the memblock_alloc() returns the
physical address hence I used the same input type as a argument to the
early_set_memory_decrypted(). If you want me to change the input to
accept the virtual address then I have no issue doing so. But the
changes need to propagated to kvmclock (i.e PATCH 17/17) to use __va().
Please let me know if you want me to pass the virtual address.
>> +/*
>> + * Iterate through all possible CPUs and map the memory region pointed
>> + * by apf_reason, steal_time and kvm_apic_eoi as decrypted at once.
>> + *
>> + * Note: we iterate through all possible CPUs to ensure that CPUs
>> + * hotplugged will have their per-cpu variable already mapped as
>> + * decrypted.
>> + */
>> +static void __init sev_map_percpu_data(void)
>> +{
>> + int cpu;
>> +
>> + if (!sev_active())
>> + return;
>> +
>> + for_each_possible_cpu(cpu) {
>> + __set_percpu_decrypted(&per_cpu(apf_reason, cpu), sizeof(apf_reason));
>> + __set_percpu_decrypted(&per_cpu(steal_time, cpu), sizeof(steal_time));
>> + __set_percpu_decrypted(&per_cpu(kvm_apic_eoi, cpu), sizeof(kvm_apic_eoi));
>> + }
>> +}
>> +
>> #ifdef CONFIG_SMP
>> static void __init kvm_smp_prepare_boot_cpu(void)
>> {
>> + sev_map_percpu_data();
>> kvm_guest_cpu_init();
>> native_smp_prepare_boot_cpu();
>> kvm_spinlock_init();
>> @@ -496,6 +524,7 @@ void __init kvm_guest_init(void)
>> kvm_cpu_online, kvm_cpu_down_prepare) < 0)
>> pr_err("kvm_guest: Failed to install cpu hotplug callbacks\n");
>> #else
>> + sev_map_percpu_data();
>> kvm_guest_cpu_init();
>> #endif
> Why isn't it enough to call
>
> sev_map_percpu_data()
>
> at the end of kvm_guest_init() only but you have to call it in
> kvm_smp_prepare_boot_cpu() too? I mean, once you map those things
> decrypted, there's no need to do them again...
>
IIRC, we tried clearing C bit in kvm_guest_init() but since the
kvm_guest_init() is called before setup_per_cpu_areas() hence
per_cpu_ptr(var, cpu_id) was not able to get another processors copy of
the variable.
