linux-kernel - Re: [RFC PATCH V3 01/11] x86/HV: Initialize GHCB page in Isolation VM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <37260f47-bd32-08f7-b006-f75f4d3c408a@gmail.com>
Date:   Mon, 7 Jun 2021 16:14:36 +0800
From:   Tianyu Lan <ltykernel@...il.com>
To:     Christoph Hellwig <hch@....de>
Cc:     kys@...rosoft.com, haiyangz@...rosoft.com, sthemmin@...rosoft.com,
        wei.liu@...nel.org, decui@...rosoft.com, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, x86@...nel.org, hpa@...or.com,
        arnd@...db.de, dave.hansen@...ux.intel.com, luto@...nel.org,
        peterz@...radead.org, akpm@...ux-foundation.org,
        kirill.shutemov@...ux.intel.com, rppt@...nel.org,
        hannes@...xchg.org, cai@....pw, krish.sadhukhan@...cle.com,
        saravanand@...com, Tianyu.Lan@...rosoft.com,
        konrad.wilk@...cle.com, m.szyprowski@...sung.com,
        robin.murphy@....com, boris.ostrovsky@...cle.com, jgross@...e.com,
        sstabellini@...nel.org, joro@...tes.org, will@...nel.org,
        xen-devel@...ts.xenproject.org, davem@...emloft.net,
        kuba@...nel.org, jejb@...ux.ibm.com, martin.petersen@...cle.com,
        iommu@...ts.linux-foundation.org, linux-arch@...r.kernel.org,
        linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-scsi@...r.kernel.org, netdev@...r.kernel.org,
        vkuznets@...hat.com, thomas.lendacky@....com,
        brijesh.singh@....com, sunilmut@...rosoft.com
Subject: Re: [RFC PATCH V3 01/11] x86/HV: Initialize GHCB page in Isolation VM

Hi Christoph:
	Thanks for your review.

On 6/7/2021 2:41 PM, Christoph Hellwig wrote:
> On Sun, May 30, 2021 at 11:06:18AM -0400, Tianyu Lan wrote:
>> +	if (ms_hyperv.ghcb_base) {
>> +		rdmsrl(MSR_AMD64_SEV_ES_GHCB, ghcb_gpa);
>> +
>> +		ghcb_va = ioremap_cache(ghcb_gpa, HV_HYP_PAGE_SIZE);
>> +		if (!ghcb_va)
>> +			return -ENOMEM;
> 
> Can you explain this a bit more?  We've very much deprecated
> ioremap_cache in favor of memremap.  Why yo you need a __iomem address
> here?  Why do we need the remap here at all? >

GHCB physical address is an address in extra address space which is 
above shared gpa boundary reported by Hyper-V CPUID. The addresses below
shared gpa boundary treated as encrypted and the one above is treated as 
decrypted. System memory is remapped in the extra address space and it 
starts from the boundary. The shared memory with host needs to use 
address in the extra address(pa + shared_gpa_boundary) in Linux guest.

Here is to map ghcb page for the communication operations with 
Hypervisor(e.g, hypercall and read/write MSR) via GHCB page.

memremap() will go through iomem_resource list and the address in extra 
address space will not be in the list. So I used ioremap_cache(). I will
memremap() instead of ioremap() here.

> Does the data structure at this address not have any types that we
> could use a struct for?

The struct will be added in the following patch. I will refresh the 
following patch and use the struct hv_ghcb for the mapped point.
> 
>> +
>> +		rdmsrl(MSR_AMD64_SEV_ES_GHCB, ghcb_gpa);
>> +		ghcb_va = ioremap_cache(ghcb_gpa, HV_HYP_PAGE_SIZE);
>> +		if (!ghcb_va) {
> 
> This seems to duplicate the above code.

The above is to map ghcb for BSP and here does the same thing for APs
Will add a new function to avoid the duplication.

> 
>> +bool hv_isolation_type_snp(void)
>> +{
>> +	return static_branch_unlikely(&isolation_type_snp);
>> +}
>> +EXPORT_SYMBOL_GPL(hv_isolation_type_snp);
> 
> This probably wants a kerneldoc explaining when it should be used. >

OK. I will add.