From mboxrd@z Thu Jan  1 00:00:00 1970
From: Brijesh Singh <brijesh.singh@amd.com>
Subject: Re: [RFC Part2 PATCH v3 16/26] KVM: SVM: Add support for SEV
 LAUNCH_UPDATE_DATA command
Date: Wed, 13 Sep 2017 14:45:37 -0500
Message-ID: <6f38ed16-376e-6716-981e-20bdf31c58b3@amd.com>
References: <20170724200303.12197-1-brijesh.singh@amd.com>
 <20170724200303.12197-17-brijesh.singh@amd.com>
 <20170913175537.wlfk3tmipx3xtbun@pd.tnic>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: brijesh.singh@amd.com, linux-kernel@vger.kernel.org,
        x86@kernel.org, kvm@vger.kernel.org,
        Thomas Gleixner <tglx@linutronix.de>,
        Joerg Roedel <joro@8bytes.org>,
        "Michael S . Tsirkin" <mst@redhat.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        =?UTF-8?B?XCJSYWRpbSBLcsSNbcOhxZlcIg==?= <rkrcmar@redhat.com>,
        Tom Lendacky <thomas.lendacky@amd.com>
To: Borislav Petkov <bp@suse.de>
Return-path: <linux-kernel-owner@vger.kernel.org>
In-Reply-To: <20170913175537.wlfk3tmipx3xtbun@pd.tnic>
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
List-Id: kvm.vger.kernel.org



On 09/13/2017 12:55 PM, Borislav Petkov wrote:
...

>> +
>> +	/* pin the user virtual address */
>> +	pinned = get_user_pages_fast(uaddr, npages, write ? FOLL_WRITE : 0,
>> +					pages);
> 
> Let it stick out.


Will do.

...


>> +	vaddr = params.address;
>> +	size = params.length;
>> +	vaddr_end = vaddr + size;
>> +
>> +	/* lock the user memory */
>> +	inpages = sev_pin_memory(vaddr, size, &npages, 1);
> 
> This way user basically controls how many pages to pin and you need to
> limit that on the upper end.
> 

Actually I don't know what should be sane upper bound in this case --
typically we encrypt the guest BIOS using LAUNCH_UPDATE_DATA command.
I have heard that some user may want to create a pre-encrypted image
(which may contains guest BIOS + kernel + initrd) -- this can be huge.

For SEV guest, we have been needing to pin the memory hence how about if
we limit the number of pages to pin with rlimit ? The rlimit check can
also include the guest RAM pinning.


>> +	if (!inpages) {
>> +		ret = -ENOMEM;
>> +		goto e_free;
>> +	}
>> +
>> +	/*
>> +	 * invalidate the cache to ensure that DRAM has recent content before
> 
> recent content?


Cache access from the PSP are coherent with x86 but not other way around --
I will update the comments to reflect the true meaning.

...

> 
> Yah, let it stick out.
> 

Okay.