bhyve: implement Timestamp get/set feature for NVMe controller

[wanpengqian_gmail.com]

Summary

this patch will implement Timestamp get/set feature for NVMe controller.

We can benefit this feature for test purpose when develop other tools such as nvmecontrol.

Test Plan

Within linux client, use nvme command to get/set this feature and check the output.

[wanpengqian_gmail.com]

create struct for each feature object

[wanpengqian_gmail.com]

fix the timestamp caculation.

[corvink]

usr.sbin/bhyve/pci_nvme.c

132

Unused.

165

Unused.

271

Global variables are initialized to 0. No need to explicitly initialize it.

329

Seems to be unused.

[wanpengqian_gmail.com]

update as reviewer suggested.

[wanpengqian_gmail.com]

Thank you!

The standard about timestamp is very hard to understand. I am not sure I am doing the right thing...

[corvink]

In D32903#750510, @wanpengqian_gmail.com wrote:

Thank you!

The standard about timestamp is very hard to understand. I am not sure I am doing the right thing...

I'm unfamiliar with nvme. I can't help you checking whether it's correctly implemented or not.
For that reason, I've only commented some general issues like unused variables.

Btw.: I'm not a FreeBSD committer. I can't merge your patch. However, I can't see any reason why this patch shouldn't be merged (if it's correctly implemented).

[chuck]

This does not appear to implement either Set or Get correctly. For example, it does not use the Timestamp Origin field on Get's.

Ignoring the implementation issues (which can be fixed), the Timestamp feature is useful if the Controller supports Persistent Logs and/or Events. But because the bhyve emulation doesn't implement either of these, it isn't immediately clear to me how this would get used. Can you offer some insight?

usr.sbin/bhyve/pci_nvme.c

246

I believe this change will break the compliance/interoperability tests

[wanpengqian_gmail.com]

In D32903#786080, @chuck wrote:

This does not appear to implement either Set or Get correctly. For example, it does not use the Timestamp Origin field on Get's.

I will do more inspections on this.

Ignoring the implementation issues (which can be fixed), the Timestamp feature is useful if the Controller supports Persistent Logs and/or Events. But because the bhyve emulation doesn't implement either of these, it isn't immediately clear to me how this would get used. Can you offer some insight?

From NVMe spec(1.4c), here is the quota:
https://nvmexpress.org/wp-content/uploads/NVM-Express-1_4c-2021.06.28-Ratified.pdf

Timestamp values should not be used for security applications. Other application use of the Timestamp is outside the scope of this specification.

The specification didn't say it relates to Persistent Logs and/or Events. so I just implement it as spec documents.

[chuck]

In D32903#786264, @wanpengqian_gmail.com wrote:

From NVMe spec(1.4c), here is the quota:
https://nvmexpress.org/wp-content/uploads/NVM-Express-1_4c-2021.06.28-Ratified.pdf

Timestamp values should not be used for security applications. Other application use of the Timestamp is outside the scope of this specification.

The specification didn't say it relates to Persistent Logs and/or Events. so I just implement it as spec documents.

You are quoting the specification correctly, but it may be instructive to look at what other parts of the specification reference timestamps.

If there are existing consumers of the timestamp functionality, I'd be interested in learning more about their requirements. But I'm reluctant to add functionality to the emulation that would not be used.