Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nvme: add support for CMB (Controller Memory Buffer) #29

Merged
merged 2 commits into from
Dec 24, 2024
Merged

nvme: add support for CMB (Controller Memory Buffer) #29

merged 2 commits into from
Dec 24, 2024

Conversation

minwooim
Copy link
Collaborator

CMB provides device memory buffer for driver context such as SQ/CQ and data buffer. nvme_configure_cmb() initializes CMB registers in the nvme controller and nvme_discard_cmb() will disable it and destroy the cmb attributes in @ctrl->cmb.

@minwooim minwooim requested a review from birkelund December 12, 2024 04:59
src/nvme/core.c Show resolved Hide resolved
src/nvme/core.c Show resolved Hide resolved
src/nvme/core.c Outdated
(ctrl->cmb.iova >> NVME_CMBMSC_CBA_SHIFT) <<
NVME_CMBMSC_CBA_SHIFT;

mmio_lh_write64(ctrl->regs + NVME_REG_CMBMSC, cpu_to_le64(cmbmsc));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be a mmio_hl_write64 (we want to make sure we write the CBA before setting CMSE).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my bad, actually I intended just like you're saying, but reversered ;) Thanks for catching this!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still the low-high variant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed that one. Applied, Thanks.

src/nvme/core.c Outdated
cmbloc = le32_to_cpu(mmio_read32(ctrl->regs + NVME_REG_CMBLOC));
bar = NVME_FIELD_GET(cmbloc, CMBLOC_BIR);

cmbmsc = le64_to_cpu(mmio_read64(ctrl->regs + NVME_REG_CMBMSC));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the register read is required to flush the CRE write, then this should be moved above the read to CMBLOC. And if it is required, a comment staging why would be nice :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right!

Fix sparse error to avoid leint64_t -> integer degradation.

    ../include/vfn/support/mmio.h:72:52: warning: restricted leint64_t degrades to integer

Signed-off-by: Minwoo Im <[email protected]>
@minwooim
Copy link
Collaborator Author

@birkelund ,

All addressed! Thanks,

CMB provides device memory buffer for driver context such as SQ/CQ and
data buffer.  nvme_configure_cmb() initializes CMB registers in the nvme
controller and nvme_discard_cmb() will disable it and destroy the cmb
attributes in @ctrl->cmb.

Signed-off-by: Minwoo Im <[email protected]>
@minwooim minwooim added the enhancement New feature or request label Dec 24, 2024
@birkelund birkelund merged commit 24fa832 into main Dec 24, 2024
25 checks passed
@birkelund birkelund deleted the cmb branch December 24, 2024 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants