2019-12-24 18:21:56

by Tom Lendacky

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] KVM: x86/mmu: Allow for overriding MMIO SPTE mask

On 12/18/19 3:18 PM, Tom Lendacky wrote:
> On 12/18/19 2:27 PM, Sean Christopherson wrote:
>> On Wed, Dec 18, 2019 at 01:51:23PM -0600, Tom Lendacky wrote:
>>> On 12/18/19 1:45 PM, Tom Lendacky wrote:
>>>> The KVM MMIO support uses bit 51 as the reserved bit to cause nested page
>>>> faults when a guest performs MMIO. The AMD memory encryption support uses
>>>> CPUID functions to define the encryption bit position. Given this, KVM
>>>> can't assume that bit 51 will be safe all the time.
>>>>
>>>> Add a callback to return a reserved bit(s) mask that can be used for the
>>>> MMIO pagetable entries. The callback is not responsible for setting the
>>>> present bit.
>>>>
>>>> If a callback is registered:
>>>> - any non-zero mask returned is updated with the present bit and used
>>>> as the MMIO SPTE mask.
>>>> - a zero mask returned results in a mask with only bit 51 set (i.e. no
>>>> present bit) as the MMIO SPTE mask, similar to the way 52-bit physical
>>>> addressing is handled.
>>>>
>>>> If no callback is registered, the current method of setting the MMIO SPTE
>>>> mask is used.
>>>>
>>>> Fixes: 28a1f3ac1d0c ("kvm: x86: Set highest physical address bits in non-present/reserved SPTEs")
>>>> Signed-off-by: Tom Lendacky <[email protected]>
>>>> ---
>>>> arch/x86/include/asm/kvm_host.h | 4 ++-
>>>> arch/x86/kvm/mmu/mmu.c | 54 +++++++++++++++++++++------------
>>>> arch/x86/kvm/x86.c | 2 +-
>>>> 3 files changed, 38 insertions(+), 22 deletions(-)
>>>
>>> This patch has some extra churn because kvm_x86_ops isn't set yet when the
>>> call to kvm_set_mmio_spte_mask() is made. If it's not a problem to move
>>> setting kvm_x86_ops just a bit earlier in kvm_arch_init(), some of the
>>> churn can be avoided.
>>
>> As a completely different alternative, what about handling this purely
>> within SVM code by overriding the masks during svm_hardware_setup(),
>> similar to how VMX handles EPT's custom masks, e.g.:
>>
>> /*
>> * Override the MMIO masks if memory encryption support is enabled:
>> * The physical addressing width is reduced. The first bit above the
>> * new physical addressing limit will always be reserved.
>> */
>> if (cpuid_eax(0x80000000) >= 0x8000001f) {
>> rdmsrl(MSR_K8_SYSCFG, msr);
>> if (msr & MSR_K8_SYSCFG_MEM_ENCRYPT) {
>> mask = BIT_ULL(boot_cpu_data.x86_phys_bits) | BIT_ULL(0);
>> kvm_mmu_set_mmio_spte_mask(mask, mask,
>> ACC_WRITE_MASK | ACC_USER_MASK);
>> }
>> }
>
> Works for me if no one has objections to doing it that way (and will
> actually make going into stable much easier).

No objections, so I'll re-submit using Sean's suggested override method.

Thanks,
Tom

>
> Thanks,
> Tom
>
>>