Hi Bart,
sorry for not replying earlier, as I am very new to NVMe/UFS spec and was figuring out few details about them.
On 7/21/23 18:40, Shyam Saini wrote:
+config RPMB
- tristate "RPMB partition interface"
- help
Unified RPMB partition interface for RPMB capable devices such aseMMC and UFS. Provides interface for in kernel securitycontrollers to
access RPMB partition.If unsure, select N.Please also mention NVMe.
Sure,
Please change the word "partition" into "unit" to avoid confusion with the concept "LBA range partition".
sure, in next iteration
+static DEFINE_IDA(rpmb_ida);
How are accesses to this IDA serialized?
I will look into that.
+/**
- rpmb_get_capacity() - returns the capacity of the rpmb device
- @rdev: rpmb device
- Return:
capacity of the device in units of 128K, on success
-EINVAL on wrong parameters
-EOPNOTSUPP if device doesn't support the requested operation
< 0 if the operation fails- */
Why in units of 128 KiB?
I think UFS/eMMC RPMB spec suggests size of RPMB multiple of 128K and NVMe spec suggests RPMB Data Area to be multiple of 128K as well.
+/**
- rpmb_dev_find_by_device() - retrieve rpmb device from the parent
device
- @parent: parent device of the rpmb device
- @target: RPMB target/region within the physical device
- Return: NULL if there is no rpmb device associated with the parent
device
- */
Can an NVMe controller have multiple RPMB units? From the NVMe specification: "The controller may support multiple RPMB targets."
That we have to figure, I see NVMe device can have upto 7 RPMB targets/units
Can rpmb_dev_find_by_device() be used if multiple RPMB units are associated with a single controller?
That's not finalised yet, but we some ideas from Optee folks on the other replies.
+/**
- rpmb_dev_register - register RPMB partition with the RPMB subsystem
- @dev: storage device of the rpmb device
- @target: RPMB target/region within the physical device
- @ops: device specific operations
- Return: a pointer to rpmb device
- */
+struct rpmb_dev *rpmb_dev_register(struct device *dev, u8 target,
const struct rpmb_ops *ops)+{
- struct rpmb_dev *rdev;
- int id;
- int ret;
- if (!dev || !ops)
return ERR_PTR(-EINVAL);- if (!ops->program_key)
return ERR_PTR(-EINVAL);- if (!ops->get_capacity)
return ERR_PTR(-EINVAL);- if (!ops->get_write_counter)
return ERR_PTR(-EINVAL);- if (!ops->write_blocks)
return ERR_PTR(-EINVAL);- if (!ops->read_blocks)
return ERR_PTR(-EINVAL);- rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
- if (!rdev)
return ERR_PTR(-ENOMEM);- id = ida_simple_get(&rpmb_ida, 0, 0, GFP_KERNEL);
- if (id < 0) {
ret = id;goto exit;- }
- mutex_init(&rdev->lock);
- rdev->ops = ops;
- rdev->id = id;
- rdev->target = target;
- dev_set_name(&rdev->dev, "rpmb%d", id);
- rdev->dev.class = &rpmb_class;
- rdev->dev.parent = dev;
- rpmb_cdev_prepare(rdev);
- ret = device_register(&rdev->dev);
- if (ret)
goto exit;- rpmb_cdev_add(rdev);
- dev_dbg(&rdev->dev, "registered device\n");
- return rdev;
+exit:
- if (id >= 0)
ida_simple_remove(&rpmb_ida, id);- kfree(rdev);
- return ERR_PTR(ret);
+}
How is user space software supposed to map an NVMe RPMB target ID to an RPMB device name?
I am not sure, this driver aims to provide in kernel RPMB access APIs, user space support may be added later on, but i will look if the current RFC version has any implication on future user-space support.
+MODULE_AUTHOR("Intel Corporation");
Shouldn't this be the name of a person instead of the name of a company?
Thanks, I will address that in next iteration.
Please keep posted your reviews and feedback.
Best Regards, Shyam