Hi, On Thursday, April 4, 2024 12:07:49 AM IST Eugenio Perez Martin wrote: > On Wed, Apr 3, 2024 at 4:36 PM Sahil <icegambi...@gmail.com> wrote: > [...] > > I would like to clarify one thing in the figure "Full two-entries > > descriptor table". The driver can only overwrite a used descriptor in the > > descriptor ring, right? > > Except for the first round, the driver can only write to used entries > in the descriptor table. In other words, their avail and used flags > must be equal. > > > And likewise for the device? > > Yes, but with avail descs. I think you got this already, but I want to > be as complete as possible here. > > > So in the figure, the driver will have to wait until descriptor[1] is > > used before it can overwrite it? > > Yes, but I think it is easier to think that both descriptor id 0 and 1 > are available already. The descriptor id must be less than virtqueue > size. > > An entry with a valid buffer and length must be invalid because of the > descriptor id in that situation, either because it is a number > vq > length or because it is a descriptor already available.
I didn't think of it in that way. This makes sense now. > > Suppose the device marks descriptor[0] as used. I think the driver will > > not be able to overwrite that descriptor entry because it has to go in > > order and is at descriptor[1]. Is that correct? > > The device must write one descriptor as used, either 0 or 1, at > descriptors[0] as all the descriptors are available. > > Now, it does not matter if the device marks as used one or the two > descriptors: the driver must write its next available descriptor at > descriptor[1]. This is not because descriptor[1] contains a special > field or data, but because the driver must write the avail descriptors > sequentially, so the device knows the address to poll or check after a > notification. > > In other words, descriptor[1] is just a buffer space from the driver > to communicate an available descriptor to the device. It does not > matter what it contained before the writing, as the driver must > process that information before writing the new available descriptor. > > > Is it possible for the driver > > to go "backwards" in the descriptor ring? > > Nope, under any circumstance. Understood. Thank you for the clarification. > [...] > > Q1. > > In the paragraph just above Figure 6, there is the following line: > > > the vhost kernel thread and QEMU may run in different CPU threads, > > > so these writes must be synchronized with QEMU cleaning of the dirty > > > bitmap, and this write must be seen strictly after the modifications of > > > the guest memory by the QEMU thread. > > > > I am not clear on the last part of the statement. The modification of > > guest memory is being done by the vhost device and not by the QEMU > > thread, right? > > QEMU also writes to the bitmap cleaning it, so it knows the memory > does not need to be resent. Oh, I thought, from figure 6, the bitmap is a part of QEMU's memory but is separate from the guest's memory. > Feel free to ask questions about this, but you don't need to interact > with the dirty bitmap in the project. Understood, I won't go off on a tangent in that case. > [...] > > Regarding the implementation of this project, can the project be broken > > down into two parts: > > 1. implementing packed virtqueues in QEMU, and > > Right, but let me expand on this: QEMU already supports packed > virtqueue in an emulated device (hw/virtio/virtio.c). The missing part > is the "driver" one, to be able to communicate with a vDPA device, at > hw/virtio/vhost-shadow-virtqueue.c. Got it. I'll take a look at "hw/virtio/virtio.c". > [...] > > My plan is to also understand how split virtqueue has been implemented > > in QEMU. I think that'll be helpful when moving the kernel's implementation > > to QEMU. > > Sure, the split virtqueue is implemented in the same file > vhost_shadow_virtqueue.c. If you deploy vhost_vdpa +vdpa_sim or > vp_vdpa [1][2], you can: > * Run QEMU with -netdev type=vhost-vdpa,x-svq=on > * Set GDB breakpoint in interesting functions like > vhost_handle_guest_kick and vhost_svq_flush. I'll set up this environment as well. Thanks, Sahil