Received-SPF: Pass (protection.outlook.com: domain of amd.com designates
 165.204.84.17 as permitted sender) receiver=protection.outlook.com;
 client-ip=165.204.84.17; helo=SATLEXMB03.amd.com;
Date:   Mon, 13 Dec 2021 11:54:47 -0600
From:   Michael Roth <michael.roth@amd.com>
To:     Dave Hansen <dave.hansen@intel.com>
CC:     Brijesh Singh <brijesh.singh@amd.com>, <x86@kernel.org>,
        <linux-kernel@vger.kernel.org>, <kvm@vger.kernel.org>,
        <linux-efi@vger.kernel.org>, <platform-driver-x86@vger.kernel.org>,
        <linux-coco@lists.linux.dev>, <linux-mm@kvack.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Joerg Roedel <jroedel@suse.de>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Ard Biesheuvel <ardb@kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        "Vitaly Kuznetsov" <vkuznets@redhat.com>,
        Jim Mattson <jmattson@google.com>,
        "Andy Lutomirski" <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Sergio Lopez <slp@redhat.com>, Peter Gonda <pgonda@google.com>,
        "Peter Zijlstra" <peterz@infradead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        David Rientjes <rientjes@google.com>,
        Dov Murik <dovmurik@linux.ibm.com>,
        Tobin Feldman-Fitzthum <tobin@ibm.com>,
        Borislav Petkov <bp@alien8.de>,
        Vlastimil Babka <vbabka@suse.cz>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Andi Kleen <ak@linux.intel.com>,
        "Dr . David Alan Gilbert" <dgilbert@redhat.com>,
        <tony.luck@intel.com>, <marcorr@google.com>,
        <sathyanarayanan.kuppuswamy@linux.intel.com>
Subject: Re: [PATCH v8 33/40] x86/compressed/64: add identity mapping for
 Confidential Computing blob
Message-ID: <20211213175447.2wskpjxmrvqzzpjx@amd.com>
References: <20211210154332.11526-1-brijesh.singh@amd.com>
 <20211210154332.11526-34-brijesh.singh@amd.com>
 <14a7c745-b6c3-8d9c-144f-2d06b80159e6@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <14a7c745-b6c3-8d9c-144f-2d06b80159e6@intel.com>
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2021 18:08:36.5601
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 5c202b6f-46af-4dc3-c492-08d9be639554
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB03.amd.com]
X-MS-Exchange-CrossTenant-AuthSource: BN8NAM11FT063.eop-nam11.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR12MB4358
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Dec 10, 2021 at 11:52:28AM -0800, Dave Hansen wrote:
> On 12/10/21 7:43 AM, Brijesh Singh wrote:
> > +static void sev_prep_identity_maps(void)
> > +{
> > +	/*
> > +	 * The ConfidentialComputing blob is used very early in uncompressed
> > +	 * kernel to find the in-memory cpuid table to handle cpuid
> > +	 * instructions. Make sure an identity-mapping exists so it can be
> > +	 * accessed after switchover.
> > +	 */
> > +	if (sev_snp_enabled()) {
> > +		struct cc_blob_sev_info *cc_info =
> > +			(void *)(unsigned long)boot_params->cc_blob_address;
> > +
> > +		add_identity_map((unsigned long)cc_info,
> > +				 (unsigned long)cc_info + sizeof(*cc_info));
> > +		add_identity_map((unsigned long)cc_info->cpuid_phys,
> > +				 (unsigned long)cc_info->cpuid_phys + cc_info->cpuid_len);
> > +	}
> 
> The casting here is pretty ugly.  Also, isn't ->cpuid_phys already a
> u64?  Whats the idea behind casting it?
> 
> I also have a sneaking suspicion that a single "unsigned long cc_blob"
> could remove virtually all the casting.  Does this work?
> 
> 	unsigned long cc_blob = boot_params->cc_blob_addres;
> 	struct cc_blob_sev_info *cc_info;
> 
> 	add_identity_map(cc_blob, cc_blob + sizeof(*cc_info));
> 
> 	cc_info = (struct cc_blob_sev_info *)cc_blob;
> 	add_identity_map(cc_info->cpuid_phys,
> 			 cc_info->cpuid_phys + cc_info->cpuid_len);

Yes, the cc->cpuid_phys cast is not needed, and your suggested implementation
is clearer and compiles/runs without any issues. I'll implement it this way for
the next spin. Thanks!