On 16/5/25 15:47, Dan Williams wrote:
From: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
Enable PCI TSM/TSM core to support assigned device authentication in
CoCo VM. The main changes are:
- Add an ->accept() operation which also flags whether the TSM driver is
host or guest context.
- Re-purpose the 'connect' sysfs attribute in guest to lock down the
private configuration for the device.
- Add the 'accept' sysfs attribute for guest to accept the private
device into its TEE.
- Skip DOE setup/transfer for guest TSM managed devices.
All private capable assigned PCI devices (TDI) start as shared. CoCo VM
should authenticate some of these devices and accept them in its TEE for
private memory access. TSM supports this authentication in 3 steps:
Connect, Attest and Accept.
On Connect, CoCo VM requires hypervisor to finish all private
configurations to the device and put the device in TDISP CONFIG_LOCKED
state. Please note this verb has different meaning from host context. On
host, Connect means establish secure physical link (e.g. PCI IDE).
On Attest, CoCo VM retrieves evidence from device and decide if the
device is good for accept. The CoCo VM kernel provides evidence,
userspace decides if the evidence is good based on its own strategy.
On Accept, userspace has acknowledged the evidence and requires CoCo VM
kernel to enable private MMIO & DMA access. Usually it ends up by put
the device in TDISP RUN state.
Currently only implement Connect & Accept to enable a minimum flow for
device shared <-> private conversion. There is no evidence retrieval
interfaces, only to assume the device evidences are always good without
attestation.
The shared -> private conversion:
echo 1 > /sys/bus/pci/devices/<...>/tsm/connect
echo 1 > /sys/bus/pci/devices/<...>/tsm/accept
The private -> shared conversion:
echo 0 > /sys/bus/pci/devices/<...>/tsm/connect
Since the device's MMIO & DMA are all blocked after Connect & before
Accept, device drivers are not considered workable in this intermediate
state. The Connect and Accept transitions only proceed while the driver is
detached. Note this can be relaxed later with a callback to an enlightened
driver to coordinate the transition, but for now, require detachment.
Signed-off-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
Co-developed-by: Dan Williams <dan.j.williams@xxxxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
drivers/pci/tsm.c | 160 +++++++++++++++++++++++++++++++++++-----
include/linux/pci-tsm.h | 15 +++-
2 files changed, 152 insertions(+), 23 deletions(-)
diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
index 219e40c5d4e7..794de2f258c3 100644
--- a/drivers/pci/tsm.c
+++ b/drivers/pci/tsm.c
@@ -124,6 +124,29 @@ static int pci_tsm_disconnect(struct pci_dev *pdev)
return 0;
}
+/*
+ * TDISP locked state temporarily makes the device inaccessible, do not
+ * surprise live attached drivers
+ */
+static int __driver_idle_connect(struct pci_dev *pdev)
Do not need "__"...
+{
+ guard(device)(&pdev->dev);
+ if (pdev->dev.driver)
+ return -EBUSY;
+ return tsm_ops->connect(pdev);
+}
+
+/*
+ * When the registered ops support accept it indicates that this is a
+ * TVM-side (guest) TSM operations structure. In this mode ->connect()
+ * arranges for the TDI to enter TDISP LOCKED state, and ->accept()
+ * transitions the device to RUN state.
+ */
+static bool tvm_mode(void)
+{
+ return !!tsm_ops->accept;
tsm_ops->accept != NULL
+}
+
static int pci_tsm_connect(struct pci_dev *pdev)
{
struct pci_tsm_pf0 *tsm = to_pci_tsm_pf0(pdev->tsm);
@@ -138,13 +161,47 @@ static int pci_tsm_connect(struct pci_dev *pdev)
if (tsm->state >= PCI_TSM_CONNECT)
return 0;
- rc = tsm_ops->connect(pdev);
+ if (tvm_mode())
+ rc = __driver_idle_connect(pdev);
... or just open code it here?
+ else
+ rc = tsm_ops->connect(pdev);
if (rc)
return rc;
tsm->state = PCI_TSM_CONNECT;
return 0;
}
+static int pci_tsm_accept(struct pci_dev *pdev)
+{
+ struct pci_tsm_pf0 *tsm = to_pci_tsm_pf0(pdev->tsm);
+ int rc;
+
+ struct mutex *lock __free(tsm_ops_unlock) = tsm_ops_lock(tsm);
Add an empty line.
+ if (!lock)
+ return -EINTR;
+
+ if (tsm->state < PCI_TSM_CONNECT)
+ return -ENXIO;
+ if (tsm->state >= PCI_TSM_ACCEPT)
+ return 0;
+
+ /*
+ * "Accept" transitions a device to the run state, it is only suitable
+ * to make that transition from a known DMA-idle (no active mappings)
+ * state. The "driver detached" state is a coarse way to assert that
+ * requirement.
And then the userspace will modprobe the driver, which will enable BME and MSE which in turn will render the ERROR state, what is the plan here?
+ */
+ guard(device)(&pdev->dev);
+ if (pdev->dev.driver)
+ return -EBUSY;
+
+ rc = tsm_ops->accept(pdev);
+ if (rc)
+ return rc;
+ tsm->state = PCI_TSM_ACCEPT;
+ return 0;
+}
+
/* TODO: switch to ACQUIRE() and ACQUIRE_ERR() */
static struct rw_semaphore *tsm_read_lock(void)
{
@@ -196,6 +253,61 @@ static ssize_t connect_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RW(connect);
+static ssize_t accept_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ bool accept;
+ int rc;
+
+ rc = kstrtobool(buf, &accept);
+ if (rc)
+ return rc;
+
+ if (!accept)
+ return -EINVAL;
+
+ struct rw_semaphore *lock __free(tsm_read_unlock) = tsm_read_lock();
+ if (!lock)
+ return -EINTR;
+
+ rc = pci_tsm_accept(pdev);
+ if (rc)
+ return rc;
+
+ return len;
+}
+
+static ssize_t accept_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ struct pci_tsm_pf0 *tsm;
+
+ struct rw_semaphore *lock __free(tsm_read_unlock) = tsm_read_lock();
+ if (!lock)
+ return -EINTR;
+
+ if (!pdev->tsm)
+ return -ENXIO;
+
+ tsm = to_pci_tsm_pf0(pdev->tsm);
+ return sysfs_emit(buf, "%d\n", tsm->state >= PCI_TSM_ACCEPT);
+}
+static DEVICE_ATTR_RW(accept);
+
+static umode_t pci_tsm_pf0_attr_visible(struct kobject *kobj,
+ struct attribute *a, int n)
+{
+ if (!tvm_mode()) {
+ /* Host context, filter out guest only attributes */
+ if (a == &dev_attr_accept.attr)
+ return 0;
+ }
+
+ return a->mode;
+}
+
static bool pci_tsm_pf0_group_visible(struct kobject *kobj)
{
struct device *dev = kobj_to_dev(kobj);
@@ -205,10 +317,11 @@ static bool pci_tsm_pf0_group_visible(struct kobject *kobj)
return true;
return false;
}
-DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(pci_tsm_pf0);
+DEFINE_SYSFS_GROUP_VISIBLE(pci_tsm_pf0);
static struct attribute *pci_tsm_pf0_attrs[] = {
&dev_attr_connect.attr,
+ &dev_attr_accept.attr,
NULL
};
@@ -322,11 +435,15 @@ EXPORT_SYMBOL_GPL(pci_tsm_initialize);
int pci_tsm_pf0_initialize(struct pci_dev *pdev, struct pci_tsm_pf0 *tsm)
{
mutex_init(&tsm->lock);
- tsm->doe_mb = pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_PCI_SIG,
- PCI_DOE_PROTO_CMA);
- if (!tsm->doe_mb) {
- pci_warn(pdev, "TSM init failure, no CMA mailbox\n");
- return -ENODEV;
+
+ /* Assigned pci device in guest won't have IDE and DOE exposed. */
+ if (!tvm_mode()) {
+ tsm->doe_mb = pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_PCI_SIG,
+ PCI_DOE_PROTO_CMA);
+ if (!tsm->doe_mb) {
+ pci_warn(pdev, "TSM init failure, no CMA mailbox\n");
+ return -ENODEV;
+ }
}
tsm->state = PCI_TSM_INIT;
@@ -483,7 +600,7 @@ int pci_tsm_doe_transfer(struct pci_dev *pdev, enum pci_doe_proto type,
{
struct pci_tsm_pf0 *tsm;
- if (!pdev->tsm || !is_pci_tsm_pf0(pdev))
+ if (!pdev->tsm || !is_pci_tsm_pf0(pdev) || tvm_mode())
return -ENXIO;
tsm = to_pci_tsm_pf0(pdev->tsm);
@@ -495,8 +612,8 @@ int pci_tsm_doe_transfer(struct pci_dev *pdev, enum pci_doe_proto type,
}
EXPORT_SYMBOL_GPL(pci_tsm_doe_transfer);
-/* lookup the 'DSM' pf0 for @pdev */
-static struct pci_dev *tsm_pf0_get(struct pci_dev *pdev)
+/* lookup the Device Security Manager (DSM) pf0 for @pdev */
+static struct pci_dev *dsm_dev_get(struct pci_dev *pdev)
{
struct pci_dev *uport_pf0;
@@ -558,11 +675,11 @@ int pci_tsm_bind(struct pci_dev *pdev, struct kvm *kvm, u64 tdi_id)
if (!pdev->tsm)
return -EINVAL;
- struct pci_dev *pf0_dev __free(pci_dev_put) = tsm_pf0_get(pdev);
- if (!pf0_dev)
+ struct pci_dev *dsm_dev __free(pci_dev_put) = dsm_dev_get(pdev);
+ if (!dsm_dev)
return -EINVAL;
A bunch of changes like this one belong to 12/13.
- struct mutex *ops_lock __free(tdi_ops_unlock) = tdi_ops_lock(pf0_dev);
+ struct mutex *ops_lock __free(tdi_ops_unlock) = tdi_ops_lock(dsm_dev);
if (IS_ERR(ops_lock))
return PTR_ERR(ops_lock);
@@ -573,10 +690,13 @@ int pci_tsm_bind(struct pci_dev *pdev, struct kvm *kvm, u64 tdi_id)
return -EBUSY;
}
- tdi = tsm_ops->bind(pdev, pf0_dev, kvm, tdi_id);
+ tdi = tsm_ops->bind(pdev, dsm_dev, kvm, tdi_id);
if (!tdi)
return -ENXIO;
+ tdi->pdev = pdev;
+ tdi->dsm_dev = dsm_dev;
+ tdi->kvm = kvm;
pdev->tsm->tdi = tdi;
return 0;
@@ -592,11 +712,11 @@ static int __pci_tsm_unbind(struct pci_dev *pdev)
if (!pdev->tsm)
return -EINVAL;
- struct pci_dev *pf0_dev __free(pci_dev_put) = tsm_pf0_get(pdev);
- if (!pf0_dev)
+ struct pci_dev *dsm_dev __free(pci_dev_put) = dsm_dev_get(pdev);
+ if (!dsm_dev)
return -EINVAL;
- struct mutex *lock __free(tdi_ops_unlock) = tdi_ops_lock(pf0_dev);
+ struct mutex *lock __free(tdi_ops_unlock) = tdi_ops_lock(dsm_dev);
if (IS_ERR(lock))
return PTR_ERR(lock);
@@ -641,11 +761,11 @@ int pci_tsm_guest_req(struct pci_dev *pdev, struct pci_tsm_guest_req_info *info)
if (!pdev->tsm)
return -ENODEV;
- struct pci_dev *pf0_dev __free(pci_dev_put) = tsm_pf0_get(pdev);
- if (!pf0_dev)
+ struct pci_dev *dsm_dev __free(pci_dev_put) = dsm_dev_get(pdev);
+ if (!dsm_dev)
return -EINVAL;
- struct mutex *lock __free(tdi_ops_unlock) = tdi_ops_lock(pf0_dev);
+ struct mutex *lock __free(tdi_ops_unlock) = tdi_ops_lock(dsm_dev);
if (IS_ERR(lock))
return -ENODEV;
diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
index 2328037ae4d1..1920ca591a42 100644
--- a/include/linux/pci-tsm.h
+++ b/include/linux/pci-tsm.h
@@ -11,6 +11,7 @@ enum pci_tsm_state {
PCI_TSM_ERR = -1,
PCI_TSM_INIT,
PCI_TSM_CONNECT,
+ PCI_TSM_ACCEPT,
};
/**
@@ -32,12 +33,12 @@ enum pci_tsm_type {
/**
* struct pci_tdi - TDI context
* @pdev: host side representation of guest-side TDI
- * @dsm: PF0 PCI device that can modify TDISP state for the TDI
+ * @dsm_dev: PF0 PCI device that can modify TDISP state for the TDI
* @kvm: TEE VM context of bound TDI
*/
struct pci_tdi {
struct pci_dev *pdev;
- struct pci_dev *dsm;
+ struct pci_dev *dsm_dev;
Should be in 12/13.
struct kvm *kvm;
};
@@ -69,7 +70,12 @@ struct pci_tsm_pf0 {
struct pci_tsm tsm;
enum pci_tsm_state state;
struct mutex lock;
- struct pci_doe_mb *doe_mb;
+ union {
+ struct { /* host pf0 tsm */
+ struct pci_doe_mb *doe_mb;
+ };
+ /* To be added: guest tsm */
+ };
};
/* physical function0 and capable of 'connect' */
@@ -135,6 +141,8 @@ struct pci_tsm_guest_req_info {
* @bind: establish a secure binding with the TVM
* @unbind: teardown the secure binding
* @guest_req: handle the vendor specific requests from TVM when bound
+ * @accept: TVM-only operation to confirm that system policy allows
+ * device to access private memory and be mapped with private mmio.
*
* @probe and @remove run in pci_tsm_rwsem held for write context. All
* other ops run under the @pdev->tsm->lock mutex and pci_tsm_rwsem held
@@ -150,6 +158,7 @@ struct pci_tsm_ops {
void (*unbind)(struct pci_tdi *tdi);
int (*guest_req)(struct pci_dev *pdev,
struct pci_tsm_guest_req_info *info);
+ int (*accept)(struct pci_dev *pdev);
When I posted my v1, I got several comments to not put host and guest callbacks together which makes sense (as only really "connect" and "status" are shared, and I am not sure I like dual use of "connect") and so did I in v2 and yet you are pushing for one struct for all? Thanks,
};
enum pci_doe_proto {
--
Alexey
