OpenChannelSSD / qemu-nvme

The LightNVM qemu implementation, based on NVMe
http://openchannelssd.readthedocs.org/en/latest/
Other
131 stars 67 forks source link

Namespace with non-zero metadata size - rw access rejected #14

Closed AndrewMeir closed 7 years ago

AndrewMeir commented 7 years ago

Hello OpenChannelSSD,

I've been using your fork of qemu-nvme for simulation of an NVMe device. I've noticed that configuring a device with non-zero metadata size causes subsequent access failures.

I've observed behaviour this whilst running quemu-nvme's NVMe device against the vanilla NVMe driver in Linux *** 4.10.0-26-generic #30-Ubuntu SMP Tue Jun 27 09:30:12 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux.

My qemu start-up options are: $ qemu-system-x86_64 -m 2048 -hda ./30GB.img -drive file=./4GB_oc.img,if=none,id=drv0 -device nvme,drive=drv0,serial=foo,id=drv0,nlbaf=4,extended=1,mc=3,meta=8,dpc=0x1f,dps=3 --enable-kvm -smp 2 -net nic -net user

If I attempt to read from /dev/nvme0n1, say using dd, I encounter a device I/O error. It all seems to stem from this check in the qemu's NVMe device layer:

`static uint16_t nvme_rw_check_req(NvmeCtrl n, NvmeNamespace ns, NvmeCmd cmd, NvmeRequest req, uint64_t slba, uint64_t elba, uint32_t nlb, uint16_t ctrl, uint64_t data_size, uint64_t meta_size) { : : if (meta_size) { nvme_set_error_page(n, req->sq->sqid, cmd->cid, NVME_INVALID_FIELD, offsetof(NvmeRwCmd, control), ctrl, ns->id); return NVME_INVALID_FIELD NVME_DNR; }

:` Removing the above if (meta_size) check seems to fix things. So far as I can tell, this check was originally inherited from Keith Busch's dev code but there seems to have been some ongoing work within this project's fork. Perhaps someone can confirm that removing this test is a sane thing to do.

Thanks, Andrew Meir

javigon commented 7 years ago

Hi Andrew,

I know that Keith removed the medatada filed from the qemu-nvme some time ago. We do not use it on the OCSSD, so I cannot say much about it. Since this seems to be a generic qemu-nvme question, I would remit you to Keith.

Removing the check might help, but you need to check if by doing that, you allow any command buffer to overflow. Most checks are on the prp sizes...

Thanks, Javier

AndrewMeir commented 7 years ago

Hi Javier, Thanks for your reply. Understood - I'll discuss this with Keith. Thanks also for the tip about checking for buffer overflow. It will help me in the short term! Andrew