From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 21A94EB64DA for ; Fri, 30 Jun 2023 22:30:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 95C0E8E005A; Fri, 30 Jun 2023 18:30:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 90BC98E0059; Fri, 30 Jun 2023 18:30:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7ACDB8E005A; Fri, 30 Jun 2023 18:30:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 6BAAB8E0059 for ; Fri, 30 Jun 2023 18:30:02 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 305151A0C91 for ; Fri, 30 Jun 2023 22:30:02 +0000 (UTC) X-FDA: 80960858244.26.A6924D0 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by imf19.hostedemail.com (Postfix) with ESMTP id A0E981A0006 for ; Fri, 30 Jun 2023 22:29:59 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=OnoIixjL; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf19.hostedemail.com: domain of dave.hansen@intel.com designates 134.134.136.100 as permitted sender) smtp.mailfrom=dave.hansen@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1688164200; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=ankkngrNxFJG3SMD/qcjmZmxuK51hwGkaVViSPjjApc=; b=FIjHMs/C7VtPGpcN9unr5J0SUkGqU+2MvKa3vnvj1msGsygyRL0lDP3KzKHJYVl2f/j6+Y MBPnG8lVd7jtIl+NywfukMFnP9BtCMqFNc36gjmDezMGpgwaIcuEvSXZuPXmL94JNGnznc V8BcyZTJ2+7GerRfKnWV/d6FqQaNLOE= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=OnoIixjL; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf19.hostedemail.com: domain of dave.hansen@intel.com designates 134.134.136.100 as permitted sender) smtp.mailfrom=dave.hansen@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1688164200; a=rsa-sha256; cv=none; b=5LKMwToV1JhxLEJeM3HXusCBUX5igvrq23SYQQsnMHSxl/rZDjAdrJ2JFPlvlmTpME7RCN B2599Pm+RO62RJMwZ7dKdgYXumKjBRSaOAKhz6QdAN9xLVMW9TlnJ3i2IURZhdU4jnyywh skZjRxclPelcWgb5cpotkThLZAGtmoE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1688164199; x=1719700199; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=kmxjEdY/oS9OgHs4b5rew7Czs0b9e7jAaSHy4XBU38o=; b=OnoIixjLHMA7sVQrYXM+1B4yL7R04ompewRmamQXA+q+yLZE4DPtqnpt tc6CjH13RUHUJkc3QpUlzhbiT3vITvGVd1zs5obK1IZsJs0G8ugp3rPho AQAhgFL1uUOzihU2Oz+7YT8LEiu0qasWvHmFUAFaYek733zUiTPIfWNTz LGFNhu59poJ6cs5Lb4TOQSJf5AeTIavKeoqSljmPDBt6G21ZAsrT8kHkq xfzRIi9FGCIho7OheUGI2Pa7qg0DZAiWl3bXIgayp9oPOCoBYFz8w2ATS jvfv7/kMfKHBkIaxXsajY8RD7/8Udq6BoPQUXI60sOgLq0lM03y+BF09W w==; X-IronPort-AV: E=McAfee;i="6600,9927,10757"; a="428552769" X-IronPort-AV: E=Sophos;i="6.01,172,1684825200"; d="scan'208";a="428552769" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Jun 2023 15:29:57 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10757"; a="747558521" X-IronPort-AV: E=Sophos;i="6.01,172,1684825200"; d="scan'208";a="747558521" Received: from amuruge1-mobl.amr.corp.intel.com (HELO [10.252.133.96]) ([10.252.133.96]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Jun 2023 15:29:55 -0700 Message-ID: <522b0954-749e-33be-59a7-4ce28e8c4d5c@intel.com> Date: Fri, 30 Jun 2023 15:29:54 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH RFC v9 09/51] x86/sev: Add RMP entry lookup helpers Content-Language: en-US To: Michael Roth Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, linux-crypto@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@redhat.com, jroedel@suse.de, thomas.lendacky@amd.com, hpa@zytor.com, ardb@kernel.org, pbonzini@redhat.com, seanjc@google.com, vkuznets@redhat.com, jmattson@google.com, luto@kernel.org, dave.hansen@linux.intel.com, slp@redhat.com, pgonda@google.com, peterz@infradead.org, srinivas.pandruvada@linux.intel.com, rientjes@google.com, dovmurik@linux.ibm.com, tobin@ibm.com, bp@alien8.de, vbabka@suse.cz, kirill@shutemov.name, ak@linux.intel.com, tony.luck@intel.com, marcorr@google.com, sathyanarayanan.kuppuswamy@linux.intel.com, alpergun@google.com, dgilbert@redhat.com, jarkko@kernel.org, ashish.kalra@amd.com, nikunj.dadhania@amd.com, liam.merwick@oracle.com, zhi.a.wang@intel.com, Brijesh Singh References: <20230612042559.375660-1-michael.roth@amd.com> <20230612042559.375660-10-michael.roth@amd.com> <59d5ca67-6a31-1929-8d2f-0e3314626893@intel.com> <20230630215709.owobzb5cr2wtkqhd@amd.com> From: Dave Hansen In-Reply-To: <20230630215709.owobzb5cr2wtkqhd@amd.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Stat-Signature: ai15qdqjxodqpcnj3ahupq1z3agcoxpz X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: A0E981A0006 X-HE-Tag: 1688164199-85517 X-HE-Meta: U2FsdGVkX18Wyp0cHjRkiHbgNLJLIqiiyyVMV+I35ihUaNNJ/ti9Fkzj1pk6aEhuvHrVGI0tc2mpLB9EgDnFOMfPS/91+6I6U8Hl3O1j67fekdAoZD+igMFYGiVInpLI4+NQ9tdLzReAzsIGQpQe46FLM9atml2FT4J2dSbbj/p9M3aBbw/P6i1YtYWZPQx+Z/MbO+LHFVuUL5XeD9AC4+IIbepkGngLE+IGtD+2KMeoUGINjK5YVerpgB1/e2BFio72eH+v/+LpgL6U3pN6RqUNILn42cjU3dZLcnPjYh0osYJ9banFwxKiPVsjT9yT30RfuPtW3VyDqSOSZiNOd1MfCwdLextmTG6HuUoZVUbj5z7TSf4qtxTEq20zHTs0am1eCzqw8w8S25mhdWiYJavyQyCQ8Haq1LhGolalQEXDKkXITTRPheZleXThxfKXVZONueBrH6w86ReFEUPPLTirXyGNPSS1oOM5+rqQbsafr37f+KQURdDKoxGH67LK+XnD5I7EMdO7kFXfEx+I2qg01Vp9GYpnBANMAZiZvTkzZh/fLVUPYc5IYh/kQNUjwWbtb2XVhNmGuKB49dQiQxQ9K2t6TLMNMT4XXPJUCsoiMAbv3jUeXBrDuU7kUSrjWtkUMpnqqCAu2HVX48wP5cYTF76cr/kM5mk4RSV573a96BxYfsknjm17FI2GEYw5KqitJmqMFjdYYMDOVGvqtrsKnPnojz+bxq8zcX6rDZaNsZkoOxQ6848U+G5EqCJUvDzKH9/KAiQTCcXsZJQW0VRa02KAkrbp7q4ySLeJe4EAqQLN/LbqeiKQUmn/QaAQQeFlBBUX7TgpuMHN4RWgE/l8/R1wfYFQAMnm8qj3BmlaIV4gRZX4TmLx8EzEwCHflEvCkoO8gQo8Yba/NijcJ2Uik6HRuvVas70MU4Yjm8P5uvwutTcBQQ2mbGozlBtc5B7bEjdm19b/zu8IkJS vx+D+YUm AFpPxeqSFJPv9LnSZRBbEGQc5txMb8jBa7LRbeBXWqcH6JyOLMObcxNH/PNOy80K3XO83I4KIBkvHWMNgQMHA7LduZL7pm4JNkCXSs23KV8gxPtGs+RzKGVYRU5UQan3eph6KFI0a4a26OHhdNwvKhiJTjw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 6/30/23 14:57, Michael Roth wrote: > On Mon, Jun 12, 2023 at 09:08:58AM -0700, Dave Hansen wrote: >> On 6/11/23 21:25, Michael Roth wrote: >>> +/* >>> + * The RMP entry format is not architectural. The format is defined in PPR >>> + * Family 19h Model 01h, Rev B1 processor. >>> + */ >>> +struct rmpentry { >>> + union { >>> + struct { >>> + u64 assigned : 1, >>> + pagesize : 1, >>> + immutable : 1, >>> + rsvd1 : 9, >>> + gpa : 39, >>> + asid : 10, >>> + vmsa : 1, >>> + validated : 1, >>> + rsvd2 : 1; >>> + } info; >>> + u64 low; >>> + }; >>> + u64 high; >>> +} __packed; >> >> What's 'high' used for? The PPR says it's reserved. Why not call it >> reserved? >> >> It _looks_ like it's only used for a debugging pr_info(). It makes the >> struct look kinda goofy. I'd much rather limit the goofiness to the >> "dumping" code, like: >> >> u64 *__e = (void *)e; >> .... >> pr_info("RMPEntry paddr 0x%llx: [high=0x%016llx low=0x%016llx]\n", >> pfn << PAGE_SHIFT, __e[0], __e[1]); >> >> BTW, why does it do any good to dump all these reserved fields? >> > > The reserved bits sometimes contain information that can be useful to > pass along to folks on the firmware side, so would definitely be helpful > to provide the full raw contents of the RMP entry. Ahh, OK. Could you include a comment to that effect, please? > So maybe something like this better captures the intended usage: > > struct rmpentry { > union { > struct { > u64 assigned : 1, > pagesize : 1, > immutable : 1, > rsvd1 : 9, > gpa : 39, > asid : 10, > vmsa : 1, > validated : 1, > rsvd2 : 1; > u64 rsvd3; > } info; > u64 data[2]; > }; > } __packed; > > But dropping the union and casting to u64[] locally in the debug/dumping > routine should work fine as well. Yeah, I'd suggest doing the nasty casting in the debug function. That makes it much more clear what the hardware is doing with the entries. The hardware doesn't treat the struct as 2*u64's at all. ... >>> + ret = rmptable_entry(paddr, entry); >>> + if (ret) >>> + return ret; >>> + >>> + /* Read a large RMP entry to get the correct page level used in RMP entry. */ >>> + ret = rmptable_entry(paddr & PMD_MASK, &large_entry); >>> + if (ret) >>> + return ret; >>> + >>> + *level = RMP_TO_X86_PG_LEVEL(rmpentry_pagesize(&large_entry)); >>> + >>> + return 0; >>> +} >> >> This is a bit weird. Should it say something like this? >> >> To do an 4k RMP lookup the hardware looks at two places in the RMP: > > I'd word this as: > > "To query all the relevant bit of an 4k RMP entry, the kernel must access > 2 entries in the RMP table:" > > Because it's possible hardware only looks at the 2M entry for > hardware-based lookups, depending on where the access is coming from, or > how the memory at the PFN range is mapped. > > But otherwise it seems like an accurate description. The wording you suggest is a bit imprecise. For a 2M-aligned 4k page, there is only *one* location, *one* entry. Also, we're not doing a lookup for an RMP entry. We're doing it for a _pfn_ that results in an RMP entry. How about this: /* * Find the authoritative RMP entry for a PFN. This can be either a 4k * RMP entry or a special large RMP entry that is authoritative for a * whole 2M area. */ ... >>> +#ifdef CONFIG_KVM_AMD_SEV >>> +int snp_lookup_rmpentry(u64 pfn, bool *assigned, int *level); >>> +#else >>> +static inline int snp_lookup_rmpentry(u64 pfn, bool *assigned, int *level) { return 0; } >>> +#endif >> >> Above, -ENXIO was returned when SEV-SNP was not supported. Here, 0 is >> returned when it is compiled out. That inconsistent. >> >> Is snp_lookup_rmpentry() acceptable when SEV-SNP is in play? I'd like >> to see consistency between when it is compiled out and when it is >> compiled in but unsupported on the CPU. > > I really don't think anything in the kernel should be calling > snp_lookup_rmpentry(), so I think it makes sense to adoption the -ENXIO > convention here and in any other stubs where that applies. Sounds good to me. Just please make them consistent.