Sukadev Bhattiprolu <sukadev@xxxxxxxxxxxxxxxxxx> writes:
> Michael Ellerman [mpe@xxxxxxxxxxxxxx] wrote:
>>
>> eg. here.
>>
>> This is the fast path of context switch.
>>
>> That expands to:
>>
>> if (!(mfmsr() & MSR_S))
>> asm volatile("mfspr %0, SPRN_BESCR" : "=r" (rval));
>> if (!(mfmsr() & MSR_S))
>> asm volatile("mfspr %0, SPRN_EBBHR" : "=r" (rval));
>> if (!(mfmsr() & MSR_S))
>> asm volatile("mfspr %0, SPRN_EBBRR" : "=r" (rval));
>>
>
> Yes, should have optimized this at least :-)
>>
>> If the Ultravisor is going to disable EBB and BHRB then we need new
>> CPU_FTR bits for those, and the code that accesses those registers
>> needs to be put behind cpu_has_feature(EBB) etc.
>
> Will try the cpu_has_feature(). Would it be ok to use a single feature
> bit, like UV or make it per-register group as that could need more
> feature bits?
We already have a number of places using is_secure_guest():
arch/powerpc/include/asm/mem_encrypt.h: return is_secure_guest();
arch/powerpc/include/asm/mem_encrypt.h: return is_secure_guest();
arch/powerpc/include/asm/svm.h:#define get_dtl_cache_ctor() (is_secure_guest() ? dtl_cache_ctor : NULL)
arch/powerpc/kernel/machine_kexec_64.c: if (is_secure_guest() && !(image->preserve_context ||
arch/powerpc/kernel/paca.c: if (is_secure_guest())
arch/powerpc/kernel/sysfs.c: return sprintf(buf, "%u\n", is_secure_guest());
arch/powerpc/platforms/pseries/iommu.c: if (!is_secure_guest())
arch/powerpc/platforms/pseries/smp.c: if (cpu_has_feature(CPU_FTR_DBELL) && !is_secure_guest())
arch/powerpc/platforms/pseries/svm.c: if (!is_secure_guest())
Which could all (or mostly) be converted to use a cpu_has_feature(CPU_FTR_SVM).
So yeah I guess it makes sense to do that, create a CPU_FTR_SVM and set
it early in boot based on MSR_S.
You could argue it's a firmware feature, so should be FW_FEATURE_SVM,
but we don't use jump_labels for firmware features so they're not as
nice for hot-path code like register switching. Also the distinction
between CPU and firmware features is a bit arbitrary.
cheers
