| Message ID | 20231230161954.569267-19-michael.roth@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Initialization Support | expand |
On Sat, Dec 30, 2023 at 10:19:46AM -0600, Michael Roth wrote: > From: Brijesh Singh <brijesh.singh@amd.com> > > The behavior of legacy SEV commands is altered when the firmware is > initialized for SNP support. In that case, all command buffer memory > that may get written to by legacy SEV commands must be marked as > firmware-owned in the RMP table prior to issuing the command. > > Additionally, when a command buffer contains a system physical address > that points to additional buffers that firmware may write to, special > handling is needed depending on whether: > > 1) the system physical address points to guest memory > 2) the system physical address points to host memory > > To handle case #1, the pages of these buffers are changed to > firmware-owned in the RMP table before issuing the command, and restored > to after the command completes. > > For case #2, a bounce buffer is used instead of the original address. > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > Co-developed-by: Michael Roth <michael.roth@amd.com> > Signed-off-by: Michael Roth <michael.roth@amd.com> > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com> > --- > drivers/crypto/ccp/sev-dev.c | 421 ++++++++++++++++++++++++++++++++++- > drivers/crypto/ccp/sev-dev.h | 3 + > 2 files changed, 414 insertions(+), 10 deletions(-) Definitely better, thanks. Some cleanups ontop: --- diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c index 8cfb376ca2e7..7681c094c7ff 100644 --- a/drivers/crypto/ccp/sev-dev.c +++ b/drivers/crypto/ccp/sev-dev.c @@ -514,18 +514,21 @@ static void *sev_fw_alloc(unsigned long len) * struct cmd_buf_desc - descriptors for managing legacy SEV command address * parameters corresponding to buffers that may be written to by firmware. * - * @paddr_ptr: pointer the address parameter in the command buffer, which may - * need to be saved/restored depending on whether a bounce buffer is used. - * Must be NULL if this descriptor is only an end-of-list indicator. + * @paddr: address which may need to be saved/restored depending on whether + * a bounce buffer is used. Must be NULL if this descriptor is only an + * end-of-list indicator. + * * @paddr_orig: storage for the original address parameter, which can be used to - * restore the original value in @paddr_ptr in cases where it is replaced - * with the address of a bounce buffer. - * @len: length of buffer located at the address originally stored at @paddr_ptr + * restore the original value in @paddr in cases where it is replaced with + * the address of a bounce buffer. + * + * @len: length of buffer located at the address originally stored at @paddr + * * @guest_owned: true if the address corresponds to guest-owned pages, in which - * case bounce buffers are not needed. + * case bounce buffers are not needed. */ struct cmd_buf_desc { - u64 *paddr_ptr; + u64 paddr; u64 paddr_orig; u32 len; bool guest_owned; @@ -549,30 +552,30 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, case SEV_CMD_PDH_CERT_EXPORT: { struct sev_data_pdh_cert_export *data = cmd_buf; - desc_list[0].paddr_ptr = &data->pdh_cert_address; + desc_list[0].paddr = data->pdh_cert_address; desc_list[0].len = data->pdh_cert_len; - desc_list[1].paddr_ptr = &data->cert_chain_address; + desc_list[1].paddr = data->cert_chain_address; desc_list[1].len = data->cert_chain_len; break; } case SEV_CMD_GET_ID: { struct sev_data_get_id *data = cmd_buf; - desc_list[0].paddr_ptr = &data->address; + desc_list[0].paddr = data->address; desc_list[0].len = data->len; break; } case SEV_CMD_PEK_CSR: { struct sev_data_pek_csr *data = cmd_buf; - desc_list[0].paddr_ptr = &data->address; + desc_list[0].paddr = data->address; desc_list[0].len = data->len; break; } case SEV_CMD_LAUNCH_UPDATE_DATA: { struct sev_data_launch_update_data *data = cmd_buf; - desc_list[0].paddr_ptr = &data->address; + desc_list[0].paddr = data->address; desc_list[0].len = data->len; desc_list[0].guest_owned = true; break; @@ -580,7 +583,7 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, case SEV_CMD_LAUNCH_UPDATE_VMSA: { struct sev_data_launch_update_vmsa *data = cmd_buf; - desc_list[0].paddr_ptr = &data->address; + desc_list[0].paddr = data->address; desc_list[0].len = data->len; desc_list[0].guest_owned = true; break; @@ -588,14 +591,14 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, case SEV_CMD_LAUNCH_MEASURE: { struct sev_data_launch_measure *data = cmd_buf; - desc_list[0].paddr_ptr = &data->address; + desc_list[0].paddr = data->address; desc_list[0].len = data->len; break; } case SEV_CMD_LAUNCH_UPDATE_SECRET: { struct sev_data_launch_secret *data = cmd_buf; - desc_list[0].paddr_ptr = &data->guest_address; + desc_list[0].paddr = data->guest_address; desc_list[0].len = data->guest_len; desc_list[0].guest_owned = true; break; @@ -603,7 +606,7 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, case SEV_CMD_DBG_DECRYPT: { struct sev_data_dbg *data = cmd_buf; - desc_list[0].paddr_ptr = &data->dst_addr; + desc_list[0].paddr = data->dst_addr; desc_list[0].len = data->len; desc_list[0].guest_owned = true; break; @@ -611,7 +614,7 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, case SEV_CMD_DBG_ENCRYPT: { struct sev_data_dbg *data = cmd_buf; - desc_list[0].paddr_ptr = &data->dst_addr; + desc_list[0].paddr = data->dst_addr; desc_list[0].len = data->len; desc_list[0].guest_owned = true; break; @@ -619,39 +622,39 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, case SEV_CMD_ATTESTATION_REPORT: { struct sev_data_attestation_report *data = cmd_buf; - desc_list[0].paddr_ptr = &data->address; + desc_list[0].paddr = data->address; desc_list[0].len = data->len; break; } case SEV_CMD_SEND_START: { struct sev_data_send_start *data = cmd_buf; - desc_list[0].paddr_ptr = &data->session_address; + desc_list[0].paddr = data->session_address; desc_list[0].len = data->session_len; break; } case SEV_CMD_SEND_UPDATE_DATA: { struct sev_data_send_update_data *data = cmd_buf; - desc_list[0].paddr_ptr = &data->hdr_address; + desc_list[0].paddr = data->hdr_address; desc_list[0].len = data->hdr_len; - desc_list[1].paddr_ptr = &data->trans_address; + desc_list[1].paddr = data->trans_address; desc_list[1].len = data->trans_len; break; } case SEV_CMD_SEND_UPDATE_VMSA: { struct sev_data_send_update_vmsa *data = cmd_buf; - desc_list[0].paddr_ptr = &data->hdr_address; + desc_list[0].paddr = data->hdr_address; desc_list[0].len = data->hdr_len; - desc_list[1].paddr_ptr = &data->trans_address; + desc_list[1].paddr = data->trans_address; desc_list[1].len = data->trans_len; break; } case SEV_CMD_RECEIVE_UPDATE_DATA: { struct sev_data_receive_update_data *data = cmd_buf; - desc_list[0].paddr_ptr = &data->guest_address; + desc_list[0].paddr = data->guest_address; desc_list[0].len = data->guest_len; desc_list[0].guest_owned = true; break; @@ -659,7 +662,7 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, case SEV_CMD_RECEIVE_UPDATE_VMSA: { struct sev_data_receive_update_vmsa *data = cmd_buf; - desc_list[0].paddr_ptr = &data->guest_address; + desc_list[0].paddr = data->guest_address; desc_list[0].len = data->guest_len; desc_list[0].guest_owned = true; break; @@ -687,16 +690,16 @@ static int snp_map_cmd_buf_desc(struct cmd_buf_desc *desc) return -ENOMEM; } - desc->paddr_orig = *desc->paddr_ptr; - *desc->paddr_ptr = __psp_pa(page_to_virt(page)); + desc->paddr_orig = desc->paddr; + desc->paddr = __psp_pa(page_to_virt(page)); } - paddr = *desc->paddr_ptr; + paddr = desc->paddr; npages = PAGE_ALIGN(desc->len) >> PAGE_SHIFT; /* Transition the buffer to firmware-owned. */ if (rmp_mark_pages_firmware(paddr, npages, true)) { - pr_warn("Failed move pages to firmware-owned state for SEV legacy command.\n"); + pr_warn("Error moving pages to firmware-owned state for SEV legacy command.\n"); return -EFAULT; } @@ -705,31 +708,29 @@ static int snp_map_cmd_buf_desc(struct cmd_buf_desc *desc) static int snp_unmap_cmd_buf_desc(struct cmd_buf_desc *desc) { - unsigned long paddr; unsigned int npages; if (!desc->len) return 0; - paddr = *desc->paddr_ptr; npages = PAGE_ALIGN(desc->len) >> PAGE_SHIFT; /* Transition the buffers back to hypervisor-owned. */ - if (snp_reclaim_pages(paddr, npages, true)) { + if (snp_reclaim_pages(desc->paddr, npages, true)) { pr_warn("Failed to reclaim firmware-owned pages while issuing SEV legacy command.\n"); return -EFAULT; } /* Copy data from bounce buffer and then free it. */ if (!desc->guest_owned) { - void *bounce_buf = __va(__sme_clr(paddr)); + void *bounce_buf = __va(__sme_clr(desc->paddr)); void *dst_buf = __va(__sme_clr(desc->paddr_orig)); memcpy(dst_buf, bounce_buf, desc->len); __free_pages(virt_to_page(bounce_buf), get_order(desc->len)); /* Restore the original address in the command buffer. */ - *desc->paddr_ptr = desc->paddr_orig; + desc->paddr = desc->paddr_orig; } return 0; @@ -737,14 +738,14 @@ static int snp_unmap_cmd_buf_desc(struct cmd_buf_desc *desc) static int snp_map_cmd_buf_desc_list(int cmd, void *cmd_buf, struct cmd_buf_desc *desc_list) { - int i, n; + int i; snp_populate_cmd_buf_desc_list(cmd, cmd_buf, desc_list); for (i = 0; i < CMD_BUF_DESC_MAX; i++) { struct cmd_buf_desc *desc = &desc_list[i]; - if (!desc->paddr_ptr) + if (!desc->paddr) break; if (snp_map_cmd_buf_desc(desc)) @@ -754,8 +755,7 @@ static int snp_map_cmd_buf_desc_list(int cmd, void *cmd_buf, struct cmd_buf_desc return 0; err_unmap: - n = i; - for (i = 0; i < n; i++) + for (i--; i >= 0; i--) snp_unmap_cmd_buf_desc(&desc_list[i]); return -EFAULT; @@ -768,7 +768,7 @@ static int snp_unmap_cmd_buf_desc_list(struct cmd_buf_desc *desc_list) for (i = 0; i < CMD_BUF_DESC_MAX; i++) { struct cmd_buf_desc *desc = &desc_list[i]; - if (!desc->paddr_ptr) + if (!desc->paddr) break; if (snp_unmap_cmd_buf_desc(desc)) @@ -799,8 +799,8 @@ static bool sev_cmd_buf_writable(int cmd) } } -/* After SNP is INIT'ed, the behavior of legacy SEV commands is changed. */ -static bool snp_legacy_handling_needed(int cmd) +/* After SNP is initialized, the behavior of legacy SEV commands is changed. */ +static inline bool snp_legacy_handling_needed(int cmd) { struct sev_device *sev = psp_master->sev_data; @@ -891,7 +891,7 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) sev->cmd_buf_backup_active = true; } else { dev_err(sev->dev, - "SEV: too many firmware commands are in-progress, no command buffers available.\n"); + "SEV: too many firmware commands in progress, no command buffers available.\n"); return -EBUSY; } @@ -904,7 +904,7 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) ret = snp_prep_cmd_buf(cmd, cmd_buf, desc_list); if (ret) { dev_err(sev->dev, - "SEV: failed to prepare buffer for legacy command %#x. Error: %d\n", + "SEV: failed to prepare buffer for legacy command 0x%#x. Error: %d\n", cmd, ret); return ret; }
On 1/19/24 11:18, Borislav Petkov wrote: > On Sat, Dec 30, 2023 at 10:19:46AM -0600, Michael Roth wrote: >> From: Brijesh Singh <brijesh.singh@amd.com> >> >> The behavior of legacy SEV commands is altered when the firmware is >> initialized for SNP support. In that case, all command buffer memory >> that may get written to by legacy SEV commands must be marked as >> firmware-owned in the RMP table prior to issuing the command. >> >> Additionally, when a command buffer contains a system physical address >> that points to additional buffers that firmware may write to, special >> handling is needed depending on whether: >> >> 1) the system physical address points to guest memory >> 2) the system physical address points to host memory >> >> To handle case #1, the pages of these buffers are changed to >> firmware-owned in the RMP table before issuing the command, and restored >> to after the command completes. >> >> For case #2, a bounce buffer is used instead of the original address. >> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> Co-developed-by: Michael Roth <michael.roth@amd.com> >> Signed-off-by: Michael Roth <michael.roth@amd.com> >> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com> >> --- >> drivers/crypto/ccp/sev-dev.c | 421 ++++++++++++++++++++++++++++++++++- >> drivers/crypto/ccp/sev-dev.h | 3 + >> 2 files changed, 414 insertions(+), 10 deletions(-) > > Definitely better, thanks. > > Some cleanups ontop: > > --- > > @@ -904,7 +904,7 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) > ret = snp_prep_cmd_buf(cmd, cmd_buf, desc_list); > if (ret) { > dev_err(sev->dev, > - "SEV: failed to prepare buffer for legacy command %#x. Error: %d\n", > + "SEV: failed to prepare buffer for legacy command 0x%#x. Error: %d\n", Using %#x will produce the 0x in the output (except if the value is zero for some reason). So I would say make that 0x%x. Thanks, Tom > cmd, ret); > return ret; > } > >
On Fri, Jan 19, 2024 at 11:36:44AM -0600, Tom Lendacky wrote: > Using %#x will produce the 0x in the output (except if the value is zero for > some reason). printf-compatibility: # The value should be converted to an "alternate form". For o conversions, the first character of the output string is made zero (by prefixing a 0 if it was not zero already). For x and X conversions, a nonzero result has the string "0x" (or "0X" for X conversions) prepended to it. > So I would say make that 0x%x. Yap, better, unambiguous.
On Fri, Jan 19, 2024 at 06:18:30PM +0100, Borislav Petkov wrote: > On Sat, Dec 30, 2023 at 10:19:46AM -0600, Michael Roth wrote: > > From: Brijesh Singh <brijesh.singh@amd.com> > > > > The behavior of legacy SEV commands is altered when the firmware is > > initialized for SNP support. In that case, all command buffer memory > > that may get written to by legacy SEV commands must be marked as > > firmware-owned in the RMP table prior to issuing the command. > > > > Additionally, when a command buffer contains a system physical address > > that points to additional buffers that firmware may write to, special > > handling is needed depending on whether: > > > > 1) the system physical address points to guest memory > > 2) the system physical address points to host memory > > > > To handle case #1, the pages of these buffers are changed to > > firmware-owned in the RMP table before issuing the command, and restored > > to after the command completes. > > > > For case #2, a bounce buffer is used instead of the original address. > > > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > > Co-developed-by: Michael Roth <michael.roth@amd.com> > > Signed-off-by: Michael Roth <michael.roth@amd.com> > > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com> > > --- > > drivers/crypto/ccp/sev-dev.c | 421 ++++++++++++++++++++++++++++++++++- > > drivers/crypto/ccp/sev-dev.h | 3 + > > 2 files changed, 414 insertions(+), 10 deletions(-) > > Definitely better, thanks. > > Some cleanups ontop: > > --- > > diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c > index 8cfb376ca2e7..7681c094c7ff 100644 > --- a/drivers/crypto/ccp/sev-dev.c > +++ b/drivers/crypto/ccp/sev-dev.c > @@ -514,18 +514,21 @@ static void *sev_fw_alloc(unsigned long len) > * struct cmd_buf_desc - descriptors for managing legacy SEV command address > * parameters corresponding to buffers that may be written to by firmware. > * > - * @paddr_ptr: pointer the address parameter in the command buffer, which may > - * need to be saved/restored depending on whether a bounce buffer is used. > - * Must be NULL if this descriptor is only an end-of-list indicator. > + * @paddr: address which may need to be saved/restored depending on whether > + * a bounce buffer is used. Must be NULL if this descriptor is only an > + * end-of-list indicator. > + * > * @paddr_orig: storage for the original address parameter, which can be used to > - * restore the original value in @paddr_ptr in cases where it is replaced > - * with the address of a bounce buffer. > - * @len: length of buffer located at the address originally stored at @paddr_ptr > + * restore the original value in @paddr in cases where it is replaced with > + * the address of a bounce buffer. > + * > + * @len: length of buffer located at the address originally stored at @paddr > + * > * @guest_owned: true if the address corresponds to guest-owned pages, in which > - * case bounce buffers are not needed. > + * case bounce buffers are not needed. In v2 I've fixed up the alignments in accordance with Documentation/doc-guide/kernel-doc.rst, which asks for the starting column of a multi-line description to be the same as first line, and asked for newlines to not be inserted in between parameters/members. I've gone back and updated other occurences in the series as well. > */ > struct cmd_buf_desc { > - u64 *paddr_ptr; > + u64 paddr; Unfortunately the logic here doesn't really work with this approach. If a descriptor needs to be re-mapped to a bounce buffer, the command buffer that contains the original address of the buffer needs to be updated to point to it, so that's where the pointer comes into play, so I've kept this in place for v2, but worked on all your other suggestions. Thanks, Mike > u64 paddr_orig; > u32 len; > bool guest_owned; > @@ -549,30 +552,30 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, > case SEV_CMD_PDH_CERT_EXPORT: { > struct sev_data_pdh_cert_export *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->pdh_cert_address; > + desc_list[0].paddr = data->pdh_cert_address; > desc_list[0].len = data->pdh_cert_len; > - desc_list[1].paddr_ptr = &data->cert_chain_address; > + desc_list[1].paddr = data->cert_chain_address; > desc_list[1].len = data->cert_chain_len; > break; > } > case SEV_CMD_GET_ID: { > struct sev_data_get_id *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->address; > + desc_list[0].paddr = data->address; > desc_list[0].len = data->len; > break; > } > case SEV_CMD_PEK_CSR: { > struct sev_data_pek_csr *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->address; > + desc_list[0].paddr = data->address; > desc_list[0].len = data->len; > break; > } > case SEV_CMD_LAUNCH_UPDATE_DATA: { > struct sev_data_launch_update_data *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->address; > + desc_list[0].paddr = data->address; > desc_list[0].len = data->len; > desc_list[0].guest_owned = true; > break; > @@ -580,7 +583,7 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, > case SEV_CMD_LAUNCH_UPDATE_VMSA: { > struct sev_data_launch_update_vmsa *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->address; > + desc_list[0].paddr = data->address; > desc_list[0].len = data->len; > desc_list[0].guest_owned = true; > break; > @@ -588,14 +591,14 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, > case SEV_CMD_LAUNCH_MEASURE: { > struct sev_data_launch_measure *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->address; > + desc_list[0].paddr = data->address; > desc_list[0].len = data->len; > break; > } > case SEV_CMD_LAUNCH_UPDATE_SECRET: { > struct sev_data_launch_secret *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->guest_address; > + desc_list[0].paddr = data->guest_address; > desc_list[0].len = data->guest_len; > desc_list[0].guest_owned = true; > break; > @@ -603,7 +606,7 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, > case SEV_CMD_DBG_DECRYPT: { > struct sev_data_dbg *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->dst_addr; > + desc_list[0].paddr = data->dst_addr; > desc_list[0].len = data->len; > desc_list[0].guest_owned = true; > break; > @@ -611,7 +614,7 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, > case SEV_CMD_DBG_ENCRYPT: { > struct sev_data_dbg *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->dst_addr; > + desc_list[0].paddr = data->dst_addr; > desc_list[0].len = data->len; > desc_list[0].guest_owned = true; > break; > @@ -619,39 +622,39 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, > case SEV_CMD_ATTESTATION_REPORT: { > struct sev_data_attestation_report *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->address; > + desc_list[0].paddr = data->address; > desc_list[0].len = data->len; > break; > } > case SEV_CMD_SEND_START: { > struct sev_data_send_start *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->session_address; > + desc_list[0].paddr = data->session_address; > desc_list[0].len = data->session_len; > break; > } > case SEV_CMD_SEND_UPDATE_DATA: { > struct sev_data_send_update_data *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->hdr_address; > + desc_list[0].paddr = data->hdr_address; > desc_list[0].len = data->hdr_len; > - desc_list[1].paddr_ptr = &data->trans_address; > + desc_list[1].paddr = data->trans_address; > desc_list[1].len = data->trans_len; > break; > } > case SEV_CMD_SEND_UPDATE_VMSA: { > struct sev_data_send_update_vmsa *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->hdr_address; > + desc_list[0].paddr = data->hdr_address; > desc_list[0].len = data->hdr_len; > - desc_list[1].paddr_ptr = &data->trans_address; > + desc_list[1].paddr = data->trans_address; > desc_list[1].len = data->trans_len; > break; > } > case SEV_CMD_RECEIVE_UPDATE_DATA: { > struct sev_data_receive_update_data *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->guest_address; > + desc_list[0].paddr = data->guest_address; > desc_list[0].len = data->guest_len; > desc_list[0].guest_owned = true; > break; > @@ -659,7 +662,7 @@ static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, > case SEV_CMD_RECEIVE_UPDATE_VMSA: { > struct sev_data_receive_update_vmsa *data = cmd_buf; > > - desc_list[0].paddr_ptr = &data->guest_address; > + desc_list[0].paddr = data->guest_address; > desc_list[0].len = data->guest_len; > desc_list[0].guest_owned = true; > break; > @@ -687,16 +690,16 @@ static int snp_map_cmd_buf_desc(struct cmd_buf_desc *desc) > return -ENOMEM; > } > > - desc->paddr_orig = *desc->paddr_ptr; > - *desc->paddr_ptr = __psp_pa(page_to_virt(page)); > + desc->paddr_orig = desc->paddr; > + desc->paddr = __psp_pa(page_to_virt(page)); > } > > - paddr = *desc->paddr_ptr; > + paddr = desc->paddr; > npages = PAGE_ALIGN(desc->len) >> PAGE_SHIFT; > > /* Transition the buffer to firmware-owned. */ > if (rmp_mark_pages_firmware(paddr, npages, true)) { > - pr_warn("Failed move pages to firmware-owned state for SEV legacy command.\n"); > + pr_warn("Error moving pages to firmware-owned state for SEV legacy command.\n"); > return -EFAULT; > } > > @@ -705,31 +708,29 @@ static int snp_map_cmd_buf_desc(struct cmd_buf_desc *desc) > > static int snp_unmap_cmd_buf_desc(struct cmd_buf_desc *desc) > { > - unsigned long paddr; > unsigned int npages; > > if (!desc->len) > return 0; > > - paddr = *desc->paddr_ptr; > npages = PAGE_ALIGN(desc->len) >> PAGE_SHIFT; > > /* Transition the buffers back to hypervisor-owned. */ > - if (snp_reclaim_pages(paddr, npages, true)) { > + if (snp_reclaim_pages(desc->paddr, npages, true)) { > pr_warn("Failed to reclaim firmware-owned pages while issuing SEV legacy command.\n"); > return -EFAULT; > } > > /* Copy data from bounce buffer and then free it. */ > if (!desc->guest_owned) { > - void *bounce_buf = __va(__sme_clr(paddr)); > + void *bounce_buf = __va(__sme_clr(desc->paddr)); > void *dst_buf = __va(__sme_clr(desc->paddr_orig)); > > memcpy(dst_buf, bounce_buf, desc->len); > __free_pages(virt_to_page(bounce_buf), get_order(desc->len)); > > /* Restore the original address in the command buffer. */ > - *desc->paddr_ptr = desc->paddr_orig; > + desc->paddr = desc->paddr_orig; > } > > return 0; > @@ -737,14 +738,14 @@ static int snp_unmap_cmd_buf_desc(struct cmd_buf_desc *desc) > > static int snp_map_cmd_buf_desc_list(int cmd, void *cmd_buf, struct cmd_buf_desc *desc_list) > { > - int i, n; > + int i; > > snp_populate_cmd_buf_desc_list(cmd, cmd_buf, desc_list); > > for (i = 0; i < CMD_BUF_DESC_MAX; i++) { > struct cmd_buf_desc *desc = &desc_list[i]; > > - if (!desc->paddr_ptr) > + if (!desc->paddr) > break; > > if (snp_map_cmd_buf_desc(desc)) > @@ -754,8 +755,7 @@ static int snp_map_cmd_buf_desc_list(int cmd, void *cmd_buf, struct cmd_buf_desc > return 0; > > err_unmap: > - n = i; > - for (i = 0; i < n; i++) > + for (i--; i >= 0; i--) > snp_unmap_cmd_buf_desc(&desc_list[i]); > > return -EFAULT; > @@ -768,7 +768,7 @@ static int snp_unmap_cmd_buf_desc_list(struct cmd_buf_desc *desc_list) > for (i = 0; i < CMD_BUF_DESC_MAX; i++) { > struct cmd_buf_desc *desc = &desc_list[i]; > > - if (!desc->paddr_ptr) > + if (!desc->paddr) > break; > > if (snp_unmap_cmd_buf_desc(desc)) > @@ -799,8 +799,8 @@ static bool sev_cmd_buf_writable(int cmd) > } > } > > -/* After SNP is INIT'ed, the behavior of legacy SEV commands is changed. */ > -static bool snp_legacy_handling_needed(int cmd) > +/* After SNP is initialized, the behavior of legacy SEV commands is changed. */ > +static inline bool snp_legacy_handling_needed(int cmd) > { > struct sev_device *sev = psp_master->sev_data; > > @@ -891,7 +891,7 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) > sev->cmd_buf_backup_active = true; > } else { > dev_err(sev->dev, > - "SEV: too many firmware commands are in-progress, no command buffers available.\n"); > + "SEV: too many firmware commands in progress, no command buffers available.\n"); > return -EBUSY; > } > > @@ -904,7 +904,7 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) > ret = snp_prep_cmd_buf(cmd, cmd_buf, desc_list); > if (ret) { > dev_err(sev->dev, > - "SEV: failed to prepare buffer for legacy command %#x. Error: %d\n", > + "SEV: failed to prepare buffer for legacy command 0x%#x. Error: %d\n", > cmd, ret); > return ret; > } > > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette >
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c index dfe7f7afc411..8cfb376ca2e7 100644 --- a/drivers/crypto/ccp/sev-dev.c +++ b/drivers/crypto/ccp/sev-dev.c @@ -43,6 +43,15 @@ #define SNP_MIN_API_MAJOR 1 #define SNP_MIN_API_MINOR 51 +/* + * Maximum number of firmware-writable buffers that might be specified + * in the parameters of a legacy SEV command buffer. + */ +#define CMD_BUF_FW_WRITABLE_MAX 2 + +/* Leave room in the descriptor array for an end-of-list indicator. */ +#define CMD_BUF_DESC_MAX (CMD_BUF_FW_WRITABLE_MAX + 1) + static DEFINE_MUTEX(sev_cmd_mutex); static struct sev_misc_dev *misc_dev; @@ -501,13 +510,351 @@ static void *sev_fw_alloc(unsigned long len) return page_address(page); } +/** + * struct cmd_buf_desc - descriptors for managing legacy SEV command address + * parameters corresponding to buffers that may be written to by firmware. + * + * @paddr_ptr: pointer the address parameter in the command buffer, which may + * need to be saved/restored depending on whether a bounce buffer is used. + * Must be NULL if this descriptor is only an end-of-list indicator. + * @paddr_orig: storage for the original address parameter, which can be used to + * restore the original value in @paddr_ptr in cases where it is replaced + * with the address of a bounce buffer. + * @len: length of buffer located at the address originally stored at @paddr_ptr + * @guest_owned: true if the address corresponds to guest-owned pages, in which + * case bounce buffers are not needed. + */ +struct cmd_buf_desc { + u64 *paddr_ptr; + u64 paddr_orig; + u32 len; + bool guest_owned; +}; + +/* + * If a legacy SEV command parameter is a memory address, those pages in + * turn need to be transitioned to/from firmware-owned before/after + * executing the firmware command. + * + * Additionally, in cases where those pages are not guest-owned, a bounce + * buffer is needed in place of the original memory address parameter. + * + * A set of descriptors are used to keep track of this handling, and + * initialized here based on the specific commands being executed. + */ +static void snp_populate_cmd_buf_desc_list(int cmd, void *cmd_buf, + struct cmd_buf_desc *desc_list) +{ + switch (cmd) { + case SEV_CMD_PDH_CERT_EXPORT: { + struct sev_data_pdh_cert_export *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->pdh_cert_address; + desc_list[0].len = data->pdh_cert_len; + desc_list[1].paddr_ptr = &data->cert_chain_address; + desc_list[1].len = data->cert_chain_len; + break; + } + case SEV_CMD_GET_ID: { + struct sev_data_get_id *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->address; + desc_list[0].len = data->len; + break; + } + case SEV_CMD_PEK_CSR: { + struct sev_data_pek_csr *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->address; + desc_list[0].len = data->len; + break; + } + case SEV_CMD_LAUNCH_UPDATE_DATA: { + struct sev_data_launch_update_data *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->address; + desc_list[0].len = data->len; + desc_list[0].guest_owned = true; + break; + } + case SEV_CMD_LAUNCH_UPDATE_VMSA: { + struct sev_data_launch_update_vmsa *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->address; + desc_list[0].len = data->len; + desc_list[0].guest_owned = true; + break; + } + case SEV_CMD_LAUNCH_MEASURE: { + struct sev_data_launch_measure *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->address; + desc_list[0].len = data->len; + break; + } + case SEV_CMD_LAUNCH_UPDATE_SECRET: { + struct sev_data_launch_secret *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->guest_address; + desc_list[0].len = data->guest_len; + desc_list[0].guest_owned = true; + break; + } + case SEV_CMD_DBG_DECRYPT: { + struct sev_data_dbg *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->dst_addr; + desc_list[0].len = data->len; + desc_list[0].guest_owned = true; + break; + } + case SEV_CMD_DBG_ENCRYPT: { + struct sev_data_dbg *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->dst_addr; + desc_list[0].len = data->len; + desc_list[0].guest_owned = true; + break; + } + case SEV_CMD_ATTESTATION_REPORT: { + struct sev_data_attestation_report *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->address; + desc_list[0].len = data->len; + break; + } + case SEV_CMD_SEND_START: { + struct sev_data_send_start *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->session_address; + desc_list[0].len = data->session_len; + break; + } + case SEV_CMD_SEND_UPDATE_DATA: { + struct sev_data_send_update_data *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->hdr_address; + desc_list[0].len = data->hdr_len; + desc_list[1].paddr_ptr = &data->trans_address; + desc_list[1].len = data->trans_len; + break; + } + case SEV_CMD_SEND_UPDATE_VMSA: { + struct sev_data_send_update_vmsa *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->hdr_address; + desc_list[0].len = data->hdr_len; + desc_list[1].paddr_ptr = &data->trans_address; + desc_list[1].len = data->trans_len; + break; + } + case SEV_CMD_RECEIVE_UPDATE_DATA: { + struct sev_data_receive_update_data *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->guest_address; + desc_list[0].len = data->guest_len; + desc_list[0].guest_owned = true; + break; + } + case SEV_CMD_RECEIVE_UPDATE_VMSA: { + struct sev_data_receive_update_vmsa *data = cmd_buf; + + desc_list[0].paddr_ptr = &data->guest_address; + desc_list[0].len = data->guest_len; + desc_list[0].guest_owned = true; + break; + } + default: + break; + } +} + +static int snp_map_cmd_buf_desc(struct cmd_buf_desc *desc) +{ + unsigned long paddr; + unsigned int npages; + + if (!desc->len) + return 0; + + /* Allocate a bounce buffer if this isn't a guest owned page. */ + if (!desc->guest_owned) { + struct page *page; + + page = alloc_pages(GFP_KERNEL_ACCOUNT, get_order(desc->len)); + if (!page) { + pr_warn("Failed to allocate bounce buffer for SEV legacy command.\n"); + return -ENOMEM; + } + + desc->paddr_orig = *desc->paddr_ptr; + *desc->paddr_ptr = __psp_pa(page_to_virt(page)); + } + + paddr = *desc->paddr_ptr; + npages = PAGE_ALIGN(desc->len) >> PAGE_SHIFT; + + /* Transition the buffer to firmware-owned. */ + if (rmp_mark_pages_firmware(paddr, npages, true)) { + pr_warn("Failed move pages to firmware-owned state for SEV legacy command.\n"); + return -EFAULT; + } + + return 0; +} + +static int snp_unmap_cmd_buf_desc(struct cmd_buf_desc *desc) +{ + unsigned long paddr; + unsigned int npages; + + if (!desc->len) + return 0; + + paddr = *desc->paddr_ptr; + npages = PAGE_ALIGN(desc->len) >> PAGE_SHIFT; + + /* Transition the buffers back to hypervisor-owned. */ + if (snp_reclaim_pages(paddr, npages, true)) { + pr_warn("Failed to reclaim firmware-owned pages while issuing SEV legacy command.\n"); + return -EFAULT; + } + + /* Copy data from bounce buffer and then free it. */ + if (!desc->guest_owned) { + void *bounce_buf = __va(__sme_clr(paddr)); + void *dst_buf = __va(__sme_clr(desc->paddr_orig)); + + memcpy(dst_buf, bounce_buf, desc->len); + __free_pages(virt_to_page(bounce_buf), get_order(desc->len)); + + /* Restore the original address in the command buffer. */ + *desc->paddr_ptr = desc->paddr_orig; + } + + return 0; +} + +static int snp_map_cmd_buf_desc_list(int cmd, void *cmd_buf, struct cmd_buf_desc *desc_list) +{ + int i, n; + + snp_populate_cmd_buf_desc_list(cmd, cmd_buf, desc_list); + + for (i = 0; i < CMD_BUF_DESC_MAX; i++) { + struct cmd_buf_desc *desc = &desc_list[i]; + + if (!desc->paddr_ptr) + break; + + if (snp_map_cmd_buf_desc(desc)) + goto err_unmap; + } + + return 0; + +err_unmap: + n = i; + for (i = 0; i < n; i++) + snp_unmap_cmd_buf_desc(&desc_list[i]); + + return -EFAULT; +} + +static int snp_unmap_cmd_buf_desc_list(struct cmd_buf_desc *desc_list) +{ + int i; + + for (i = 0; i < CMD_BUF_DESC_MAX; i++) { + struct cmd_buf_desc *desc = &desc_list[i]; + + if (!desc->paddr_ptr) + break; + + if (snp_unmap_cmd_buf_desc(desc)) + return -EFAULT; + } + + return 0; +} + +static bool sev_cmd_buf_writable(int cmd) +{ + switch (cmd) { + case SEV_CMD_PLATFORM_STATUS: + case SEV_CMD_GUEST_STATUS: + case SEV_CMD_LAUNCH_START: + case SEV_CMD_RECEIVE_START: + case SEV_CMD_LAUNCH_MEASURE: + case SEV_CMD_SEND_START: + case SEV_CMD_SEND_UPDATE_DATA: + case SEV_CMD_SEND_UPDATE_VMSA: + case SEV_CMD_PEK_CSR: + case SEV_CMD_PDH_CERT_EXPORT: + case SEV_CMD_GET_ID: + case SEV_CMD_ATTESTATION_REPORT: + return true; + default: + return false; + } +} + +/* After SNP is INIT'ed, the behavior of legacy SEV commands is changed. */ +static bool snp_legacy_handling_needed(int cmd) +{ + struct sev_device *sev = psp_master->sev_data; + + return cmd < SEV_CMD_SNP_INIT && sev->snp_initialized; +} + +static int snp_prep_cmd_buf(int cmd, void *cmd_buf, struct cmd_buf_desc *desc_list) +{ + if (!snp_legacy_handling_needed(cmd)) + return 0; + + if (snp_map_cmd_buf_desc_list(cmd, cmd_buf, desc_list)) + return -EFAULT; + + /* + * Before command execution, the command buffer needs to be put into + * the firmware-owned state. + */ + if (sev_cmd_buf_writable(cmd)) { + if (rmp_mark_pages_firmware(__pa(cmd_buf), 1, true)) + return -EFAULT; + } + + return 0; +} + +static int snp_reclaim_cmd_buf(int cmd, void *cmd_buf, struct cmd_buf_desc *desc_list) +{ + if (!snp_legacy_handling_needed(cmd)) + return 0; + + /* + * After command completion, the command buffer needs to be put back + * into the hypervisor-owned state. + */ + if (sev_cmd_buf_writable(cmd)) + if (snp_reclaim_pages(__pa(cmd_buf), 1, true)) + return -EFAULT; + + if (snp_unmap_cmd_buf_desc_list(desc_list)) + return -EFAULT; + + return 0; +} + static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) { + struct cmd_buf_desc desc_list[CMD_BUF_DESC_MAX] = {0}; struct psp_device *psp = psp_master; struct sev_device *sev; unsigned int cmdbuff_hi, cmdbuff_lo; unsigned int phys_lsb, phys_msb; unsigned int reg, ret = 0; + void *cmd_buf; int buf_len; if (!psp || !psp->sev_data) @@ -527,12 +874,47 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) * work for some memory, e.g. vmalloc'd addresses, and @data may not be * physically contiguous. */ - if (data) - memcpy(sev->cmd_buf, data, buf_len); + if (data) { + /* + * Commands are generally issued one at a time and require the + * sev_cmd_mutex, but there could be recursive firmware requests + * due to SEV_CMD_SNP_PAGE_RECLAIM needing to be issued while + * preparing buffers for another command. This is the only known + * case of nesting in the current code, so exactly one + * additional command buffer is available for that purpose. + */ + if (!sev->cmd_buf_active) { + cmd_buf = sev->cmd_buf; + sev->cmd_buf_active = true; + } else if (!sev->cmd_buf_backup_active) { + cmd_buf = sev->cmd_buf_backup; + sev->cmd_buf_backup_active = true; + } else { + dev_err(sev->dev, + "SEV: too many firmware commands are in-progress, no command buffers available.\n"); + return -EBUSY; + } + + memcpy(cmd_buf, data, buf_len); + + /* + * The behavior of the SEV-legacy commands is altered when the + * SNP firmware is in the INIT state. + */ + ret = snp_prep_cmd_buf(cmd, cmd_buf, desc_list); + if (ret) { + dev_err(sev->dev, + "SEV: failed to prepare buffer for legacy command %#x. Error: %d\n", + cmd, ret); + return ret; + } + } else { + cmd_buf = sev->cmd_buf; + } /* Get the physical address of the command buffer */ - phys_lsb = data ? lower_32_bits(__psp_pa(sev->cmd_buf)) : 0; - phys_msb = data ? upper_32_bits(__psp_pa(sev->cmd_buf)) : 0; + phys_lsb = data ? lower_32_bits(__psp_pa(cmd_buf)) : 0; + phys_msb = data ? upper_32_bits(__psp_pa(cmd_buf)) : 0; dev_dbg(sev->dev, "sev command id %#x buffer 0x%08x%08x timeout %us\n", cmd, phys_msb, phys_lsb, psp_timeout); @@ -586,15 +968,32 @@ static int __sev_do_cmd_locked(int cmd, void *data, int *psp_ret) ret = sev_write_init_ex_file_if_required(cmd); } - print_hex_dump_debug("(out): ", DUMP_PREFIX_OFFSET, 16, 2, data, - buf_len, false); - /* * Copy potential output from the PSP back to data. Do this even on * failure in case the caller wants to glean something from the error. */ - if (data) - memcpy(data, sev->cmd_buf, buf_len); + if (data) { + /* + * Restore the page state after the command completes. + */ + ret = snp_reclaim_cmd_buf(cmd, cmd_buf, desc_list); + if (ret) { + dev_err(sev->dev, + "SEV: failed to reclaim buffer for legacy command %#x. Error: %d\n", + cmd, ret); + return ret; + } + + memcpy(data, cmd_buf, buf_len); + + if (sev->cmd_buf_backup_active) + sev->cmd_buf_backup_active = false; + else + sev->cmd_buf_active = false; + } + + print_hex_dump_debug("(out): ", DUMP_PREFIX_OFFSET, 16, 2, data, + buf_len, false); return ret; } @@ -1696,10 +2095,12 @@ int sev_dev_init(struct psp_device *psp) if (!sev) goto e_err; - sev->cmd_buf = (void *)devm_get_free_pages(dev, GFP_KERNEL, 0); + sev->cmd_buf = (void *)devm_get_free_pages(dev, GFP_KERNEL, 1); if (!sev->cmd_buf) goto e_sev; + sev->cmd_buf_backup = (uint8_t *)sev->cmd_buf + PAGE_SIZE; + psp->sev_data = sev; sev->dev = dev; diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h index 85506325051a..3e4e5574e88a 100644 --- a/drivers/crypto/ccp/sev-dev.h +++ b/drivers/crypto/ccp/sev-dev.h @@ -52,6 +52,9 @@ struct sev_device { u8 build; void *cmd_buf; + void *cmd_buf_backup; + bool cmd_buf_active; + bool cmd_buf_backup_active; bool snp_initialized; };