Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1602178 > unrolled thread

Re: [RFC PATCH v2 30/32] kvm: svm: Add support for SEV DEBUG_ENCRYPT command

Started byPaolo Bonzini <pbonzini@redhat.com>
First post2017-03-16 12:10 +0100
Last post2017-03-16 12:10 +0100
Articles 1 — 1 participant

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [RFC PATCH v2 30/32] kvm: svm: Add support for SEV DEBUG_ENCRYPT  command Paolo Bonzini <pbonzini@redhat.com> - 2017-03-16 12:10 +0100

#1602178 — Re: [RFC PATCH v2 30/32] kvm: svm: Add support for SEV DEBUG_ENCRYPT command

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-03-16 12:10 +0100
SubjectRe: [RFC PATCH v2 30/32] kvm: svm: Add support for SEV DEBUG_ENCRYPT command
Message-ID<tlBs6-5QN-23@gated-at.bofh.it>

On 02/03/2017 16:18, Brijesh Singh wrote:
> +	data = (void *) get_zeroed_page(GFP_KERNEL);

The page does not need to be zeroed, does it?

> +
> +	if ((len & 15) || (dst_addr & 15)) {
> +		/* if destination address and length are not 16-byte
> +		 * aligned then:
> +		 * a) decrypt destination page into temporary buffer
> +		 * b) copy source data into temporary buffer at correct offset
> +		 * c) encrypt temporary buffer
> +		 */
> +		ret = __sev_dbg_decrypt_page(kvm, dst_addr, data, &argp->error);

Ah, I see now you're using this function here for read-modify-write.
data is already pinned here, so even if you keep the function it makes
sense to push pinning out of __sev_dbg_decrypt_page and into
sev_dbg_decrypt.

> +		if (ret)
> +			goto err_3;
> +		d_off = dst_addr & (PAGE_SIZE - 1);
> +
> +		if (copy_from_user(data + d_off,
> +					(uint8_t *)debug.src_addr, len)) {
> +			ret = -EFAULT;
> +			goto err_3;
> +		}
> +
> +		encrypt->length = PAGE_SIZE;

Why decrypt/re-encrypt all the page instead of just the 16 byte area
around the [dst_addr, dst_addr+len) range?

> +		encrypt->src_addr = __psp_pa(data);
> +		encrypt->dst_addr =  __sev_page_pa(inpages[0]);
> +	} else {
> +		if (copy_from_user(data, (uint8_t *)debug.src_addr, len)) {
> +			ret = -EFAULT;
> +			goto err_3;
> +		}

Do you need copy_from_user, or can you just pin/unpin memory as for
DEBUG_DECRYPT?

Paolo

> +		d_off = dst_addr & (PAGE_SIZE - 1);
> +		encrypt->length = len;
> +		encrypt->src_addr = __psp_pa(data);
> +		encrypt->dst_addr = __sev_page_pa(inpages[0]);
> +		encrypt->dst_addr += d_off;
> +	}
> +
> +	encrypt->handle = sev_get_handle(kvm);
> +	ret = sev_issue_cmd(kvm, SEV_CMD_DBG_ENCRYPT, encrypt, &argp->error);

[toc] | [standalone]


Back to top | Article view | linux.kernel


csiph-web