Alexey Kardashevskiy [off-list ref] writes:
On 29/10/26 02:26, Aneesh Kumar K.V (Arm) wrote:
quoted
AMD SEV-TIO currently establishes its SPDM session and enables IDE during
connect, before userspace can retrieve or accept the device identity.
Move AMD link setup behind its evidence_accept callback.
[ ... 49 lines skipped ... ]
quoted
dev_data->req.sla = 0;
dev_data->resp.sla = 0;
dev_data->scratch.sla = 0;
dev_data->respbuf = NULL;
dev_data->reqbuf = NULL;
- sla_free(dev_data->output, tio_status->spdm_out_size_max, true);
+ sla_free(dev_data->output, dev_data->output_len, true);
+ dev_data->output = SLA_NULL;
This is a bugfix (thanks for pointing!) and does not belong here. Want to repost, or I can (and should, really)?
quoted
}
static int spdm_ctrl_alloc(struct tsm_dsm_tio *dev_data)
@@ -674,7 +672,7 @@ static int spdm_ctrl_alloc(struct tsm_dsm_tio *dev_data)
dev_data->output = sla_alloc(dev_data->output_len, true);
if (IS_SLA_NULL(dev_data->req) || IS_SLA_NULL(dev_data->resp) ||
- IS_SLA_NULL(dev_data->scratch) || IS_SLA_NULL(dev_data->dev_ctx)) {
+ IS_SLA_NULL(dev_data->scratch) || IS_SLA_NULL(dev_data->output)) {
the same.
quoted
ret = -ENOMEM;
goto free_spdm_exit;
}
@@ -773,7 +771,8 @@ int sev_tio_dev_create(struct tsm_dsm_tio *dev_data, u16 device_id,
free_data_pg_exit:
snp_free_firmware_page(data_pg);
free_ctx_exit:
- sla_free(create.dev_ctx_sla, tio_status->devctx_size, true);
+ sla_free(dev_data->dev_ctx, tio_status->devctx_size, true);
and this. Is this codex? :)
Yes.
quoted
+ dev_data->dev_ctx = SLA_NULL;
return ret;
}
[ ... 179 lines skipped ... ]
quoted
+
+ /* Reserve TC0's stream ID without programming or enabling IDE. */
+ memset(ids, 0xff, sizeof(ids));
+ ids[0] = dev_data->ide[0]->stream_id;
+ dev_data->cert_slot = 0;
+ ret = sev_tio_dev_connect(dev_data, BIT(0), ids, dev_data->cert_slot);
+ ret = dsm_collect_identity(dsm, ret);
Cannot librspdm do that collection, directly via DOE? Ideally in the
generic code? There nothing special for the PSP to do and stopping
SEV-TIO's connect half way is weird.
This patch does not use rust-spdm and avoids introducing that
dependency. It uses slot 0 to initiate the session, retrieve the
certificate chain, and return it to the host. After host approval, it
resumes the session and enables IDE. Only slot 0 is supported, and the
host can reject the device identity before IDE is enabled.
quoted
+ if (!ret)
+ return 0;
+
+disconnect:
+ dsm_disconnect(pdev);
+ return ret;
+}
- tc_mask = streams_setup(dev_data->ide, ids);
+static int dsm_evidence_accept(struct pci_dev *pdev)
+{
+ struct tio_dsm *dsm = pdev_to_tio_dsm(pdev);
+ struct tsm_dsm_tio *dev_data = &dsm->data;
+ u8 ids[TIO_IDE_MAX_TC];
+ int ret;
- dev_data->cert_slot = pdev->tsm->evidence.slot;
- ret = sev_tio_dev_connect(dev_data, tc_mask, ids, dev_data->cert_slot);
- ret = sev_tio_spdm_cmd(dsm, ret);
- if (ret)
- goto free_exit;
+ if (pdev->tsm->evidence.slot)
+ return -EOPNOTSUPP;
+ if (!dsm->pending_spdm)
+ return -ENODATA;
- streams_enable(dev_data->ide);
+ streams_setup(dev_data->ide, ids);
I kinda get the appeal of not enabling IDE before accepting but in
practice the host still has to have a knob in IOMMU to prevent a
malicious device from DMAing T=1 trafic so accept() would trigger
enable_dma() hook.
This enables IDE; it is not the TDISP RUN transition. Am I missing
something here?
quoted
+ ret = sev_tio_spdm_cmd(dsm, dsm->pending_spdm);
+ dsm->pending_spdm = 0;
+ if (ret)
+ goto err_session;
+ dsm->session_connected = true;
+ ret = streams_enable(dev_data->ide);
+ if (ret)
+ goto err_session;
ret = streams_register(dev_data->ide);
if (ret)
- goto free_exit;
-
+ goto err_session;
return 0;
-free_exit:
- sev_tio_dev_reclaim(dev_data);
-
- streams_disable(dev_data->ide);
-ide_free_exit:
-
- streams_teardown(dev_data->ide);
-
+err_session:
+ /* Disconnect owns cleanup after firmware or IDE setup has started. */
+ pdev->tsm->error = true;
return ret;
}
@@ -339,14 +468,20 @@ static void dsm_disconnect(struct pci_dev *pdev)
struct tsm_dsm_tio *dev_data = &dsm->data;
int ret;
- ret = sev_tio_dev_disconnect(dev_data, force);
- ret = sev_tio_spdm_cmd(dsm, ret);
- if (ret && !force) {
- ret = sev_tio_dev_disconnect(dev_data, true);
- sev_tio_spdm_cmd(dsm, ret);
+ /* A paused or failed connect needs forced firmware cleanup. */
+ if (dev_data->reqbuf && dev_data->respbuf) {
+ force |= !dsm->session_connected;
+ ret = sev_tio_dev_disconnect(dev_data, force);
+ ret = sev_tio_spdm_cmd(dsm, ret);
+ if (ret && !force) {
+ ret = sev_tio_dev_disconnect(dev_data, true);
+ sev_tio_spdm_cmd(dsm, ret);
+ }
}
sev_tio_dev_reclaim(dev_data);
+ dsm->session_connected = false;
+ dsm->pending_spdm = 0;
streams_disable(dev_data->ide);
streams_unregister(dev_data->ide);@@ -357,6 +492,8 @@ static struct pci_tsm_ops sev_tsm_ops = {
.probe = dsm_probe,
.remove = dsm_remove,
.connect = dsm_connect,
+ .evidence_accept = dsm_evidence_accept,
I saw in your branch you are ditching the connect() hook, is that the plan? Thanks,
Yes. In the full branch, once rust-spdm is integrated, this entire flow
moves to the SPDM accept/evidence_accept phase.
quoted
+ .evidence_types = BIT(PCI_TSM_EVIDENCE_TYPE_CERT0),
.disconnect = dsm_disconnect,
};
-aneesh