Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v2] KVM: SVM: Override default MMIO mask if memory
 encryption is enabled
To:     Sean Christopherson <sean.j.christopherson@intel.com>
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        Paolo Bonzini <pbonzini@redhat.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        Brijesh Singh <brijesh.singh@amd.com>
References: <d741b3a58769749b7873fea703c027a68b8e2e3d.1577462279.git.thomas.lendacky@amd.com>
 <20200106224931.GB12879@linux.intel.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
Message-ID: <f5c2e60c-536f-e0cd-98b9-86e6da82e48f@amd.com>
Date:   Mon, 6 Jan 2020 17:14:04 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.2.2
In-Reply-To: <20200106224931.GB12879@linux.intel.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
Received-SPF: None (protection.outlook.com: amd.com does not designate
 permitted sender hosts)
X-MS-Exchange-CrossTenant-Network-Message-Id: 361f76dd-5270-4363-23e6-08d792fe2112
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Jan 2020 23:14:07.3058
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: DqDE1n+Z0dv0dpkczapl7ljIfsrMblsMfO8fYcJP6dTRqlz/ITIZPrbsRGGwefQB5RWwjqOydiVzvUKPrql4Uw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB3913
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/6/20 4:49 PM, Sean Christopherson wrote:
> On Fri, Dec 27, 2019 at 09:58:00AM -0600, 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
>> a CPUID function to define the encryption bit position. Given this, it is
>> possible that these bits can conflict.
>>
>> Use svm_hardware_setup() to override the MMIO mask if memory encryption
>> support is enabled. When memory encryption support is enabled the physical
>> address width is reduced and the first bit after the last valid reduced
>> physical address bit will always be reserved. Use this bit as the MMIO
>> mask.
>>
>> Fixes: 28a1f3ac1d0c ("kvm: x86: Set highest physical address bits in non-present/reserved SPTEs")
>> Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>  arch/x86/kvm/svm.c | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> index 122d4ce3b1ab..2cb834b5982a 100644
>> --- a/arch/x86/kvm/svm.c
>> +++ b/arch/x86/kvm/svm.c
>> @@ -1361,6 +1361,32 @@ static __init int svm_hardware_setup(void)
>>  		}
>>  	}
>>  
>> +	/*
>> +	 * The default MMIO mask is a single bit (excluding the present bit),
>> +	 * which could conflict with the memory encryption bit. Check for
>> +	 * memory encryption support and override the default MMIO masks if
>> +	 * it is enabled.
>> +	 */
>> +	if (cpuid_eax(0x80000000) >= 0x8000001f) {
>> +		u64 msr, mask;
>> +
>> +		rdmsrl(MSR_K8_SYSCFG, msr);
>> +		if (msr & MSR_K8_SYSCFG_MEM_ENCRYPT)  {
>> +			/*
>> +			 * The physical addressing width is reduced. The first
>> +			 * bit above the new physical addressing limit will
>> +			 * always be reserved. Use this bit and the present bit
>> +			 * to generate a page fault with PFER.RSV = 1.
>> +			 */
>> +			mask = BIT_ULL(boot_cpu_data.x86_phys_bits);
> 
> This doesn't handle the case where x86_phys_bits _isn't_ reduced by SME/SEV
> on a future processor, i.e. x86_phys_bits==52.

Not sure I follow. If MSR_K8_SYSCFG_MEM_ENCRYPT is set then there will
always be a reduction in physical addressing (so I'm told). And if
MSR_K8_SYSCFG_MEM_ENCRYPT isn't set and x86_phys_bits == 52, then
everything should be fine with the existing kvm_set_mmio_spte_mask()
function where bit 51 is set but the present bit isn't, correct?

Thanks,
Tom

> 
> After staring at things for a while, I think we can handle this issue with
> minimal fuss by special casing MKTME in kvm_set_mmio_spte_mask().  I'll
> send a patch, I have a related bug fix for kvm_set_mmio_spte_mask() that
> touches the same code.
> 
>> +			mask |= BIT_ULL(0);
>> +
>> +			kvm_mmu_set_mmio_spte_mask(mask, mask,
>> +						   PT_WRITABLE_MASK |
>> +						   PT_USER_MASK);
>> +		}
>> +	}
>> +
>>  	for_each_possible_cpu(cpu) {
>>  		r = svm_cpu_init(cpu);
>>  		if (r)
>> -- 
>> 2.17.1
>>