[After First Evaluation] Import the Xen grant-table bus_dma(9) handlers from OpenBSD

For summary after the second evaluation, check the new post here

June 18 - 19:

Commits:

• Removed the malloc tag M_XEN_DMAMAP
• Changed nsegments in bus_dma_tag_xen to max_segments
• Removed unneeded typecast
• Added some TODOs to keep track of the changes Roger suggested
• Moved the allocation of the refs array and the grant refs to a helper
• Removed gref_head from bus_dmamap_xen
• bus_dmamap_xen now also contains the dma tag used to create it
• Fixed grant refs not getting allocated from deferred context

Summary:

• Review with Roger.
• He suggested multiple changes. Had discussions about some other issues.
• Failing to allocate grant references should not be fatal. Find out a way to use the grant table free callbacks.
• Removed the extra malloc tag I introduced.
• The three _load functions have the same exact piece of code. Replaced it with a call to a helper.
• Removed gref_head from the Xen DMA map. No need for it. Use gnttab_end_foreign_access_references() instead.
• Realized a potential bug in the implementation. If the load is called with BUS_DMA_WAITOK and the load is deferred, the grant references were not getting allocated and loaded. This is because in case of error, we returned from without calling xen_load_helper(). So, when the allocation completed from a deferred context, the client callback was called with no grant references allocated. Fixed it (maybe) by passing a custom xen callback to the parent’s waitok().
• Fixed it.
• Failing to allocate grant references is no longer fatal. Using the free callbacks
• First stipend arrived. Yay!

June 20 - 21:

Commits:

• Failing to allocate grant references is no longer fatal
• Fixed typos and compilation errors

Summary:

• Fixed multiple bugs and errors.
• The client callback was not being called if the allocation of grant refs was deferred. Fixed it.
• Fixed some memory leaks.
• The code has gotten rather complicated after the introduction of xen_gnttab_free_callback(). Spent a lot of time reasoning about its correction.

June 22 - 23:

Commits:

• Removed un-needed typecast
• Moved KASSERT messages in the same line wherever possible
• Fixed memory leak in xen_load_helper()
• Restructured the code paths when there is a need to call the

Summary:

• The code was getting rather complicated. So time to overhaul it a bit.
• Roger suggested that I try to get the code in the same state before it reaches xen_gnttab_free_callback() so there is no need for that if.
• I fixed it by calling the parent’s map_complete() a little early. This means that we can save a copy of the segs array in xen_load_helper(), and the free callback has to simply use the copy in both cases: initial load deferred, and not deferred.
• We need to call the parent’s map_complete() anyway, so it’s not like we are doing anything wrong. This is because once the parent’s load is complete, its map_complete() should be called. But if the allocation of grant refs fails, we will sleep. It is better to complete the parent’s load cycle before waiting.

June 25:

Commits:

• Removed unused variable i from xen_bus_dmamap_load_ma()
• Changed M_WAITOK to M_NOWAIT in xen_dmamap_callback
• Moved the call to parent’s map_complete after allocation of temp_segs
• Moved more code from load*() functions to xen_load_helper()
• Fixed unreachable code in xen_dmamap_callback()

Summary:

• Roger had a great idea about the problem with moving more of the duplicated code to a single place. Since the loads use different types and number of arguments, he suggested that I use a union.
• Modified the loads to use the union and moved the duplicated code to xen_load_helper().
• Roger also pointed out that the code would be unreachable because I was doing:

  if (error == EINPROGRESS) {
  	return (error);
  }
  else {
  	return (error);
  }
  

• But I should instead have else if (error != 0) otherwise the code below would never run.

June 26 - 27:

Summary:

• The Xen-specific bus_dma(9) implementation is almost complete, other than some minor additions/fixes:
  • Memory leak and segmentation fault if unload is called before the load finishes.
  • The ability to reserve a pool of grant references, like netfront and blkfront do.
  • The ability to extend an already loaded map.
• Wrote the freebsd-xen@ and freebsd-arch@ mailing lists to ask if I should expect a map to be unloaded before the load completes. No responses so far.
• Looked at the netfront driver to understand how it works and convert it to use the bus_dma(9) implementation we wrote.
• Need to write tests for our implementation. Spent some time reading sys/dev/xen/netback/netback_unit_tests.c.
• Wrote to my Edward, Roger and Akshay about the tests. Roger says it is too much work, and testing of a busdma implementation has never been done before. It is probably a GSoC on its own.
• So scrap the plans about testing.
• Roger suggests I start with blkfront.
• Started reading through the blkfront code.

June 28-30:

Commits:

• Removed declaration of struct in the middle of the code
• Made the initialization of load_op explicit
• Added xen_get_dma_tag()
• Added bus_get_dma_tag() in xenpv

Summary:

• Lots of reading. Still going through the blkfront code.
• Did some grepping around to find how other drivers use bus_get_dma_tag().
• Not much success, there are a lot of results, and most don’t do anything interesting, they simple call the parent’s bus_get_dma_tag().
• Looked at how busdma_dmar does things.
• They have a function dmar_get_bus_dma_tag() that returns the ctx tag after some initialization.
• Looked into sys/x86/iommu/intel_ctx.c. They simply set the parameters to max.
• Also looked at sys/dev/acpica/acpi_pci.c. They use the dmar tag.
• Wrote xen_get_dma_tag() that can be used to get the xen-specific dma tag.
• Added xenpv_get_dma_tag(). It can be used by devices hanging off the xenpv bus to get the xen dma tag.

July 2-3:

Summary:

• Read an article about the Xen device drivers.
• Lots of code reading.
• Made some minor changes/fixes to xenpv.c

July 4-5:

Commits:

• Removed error check from xen_get_dma_tag()
• Added the ability to pass flags to the grant table methods
• The xbd command indirection pages now use the Xen-specific dma interface

Summary:

• Started converting the blkfront driver to use the Xen-specific bus_dma(9) implementation.
• There are two places grant refs are allocated or foreign access granted in the blkfront driver:
  • xbd_connect(): Here, foreign access for the xbd command indirection pages in granted.
  • xbd_mksegarray(): This function is called from the callback specified to the dma load called from xbd_queue_request(). The callbacks calls xbd_mksegarray(). This function more or less does what our busdma implementation does: get ds_addr and grant foreign access to that address. Converting it would be tougher than converting xbd_connect() because it chases a lot of functions and is more complex.
• Converted the xbd command indirection pages to use the busdma implementation.
• Will ask Roger for review, and then start converting the dma loads in xbd_queue_request().

July 6:

Commits:

• The xbd command indirection pages now use the Xen-spacific dma interface
• Exported xen-specific dma magic numbers to header busdma_xen.h
• Replaced the nseg equality check with a KASSERT()
• Replaced int with unsigned int in xbd_indirectpage_cb()

Summary:

• Read a blog post about Xen block device indirect pages.
• Review with Roger.
• He suggested some small changes. Did them.
• Discovered a bug in xen_bus_dma_tag_create() which would cause an infinite recursion if a driver creates a tag. This would happen because we call bus_dma_tag_create(parent, ...) from xen_bus_dma_tag_create(). The call to tag create would expand to parent->impl->tag_create(parent,...). Parent’s tag_create is xen_bus_dma_tag_create(), so this would result in an infinite recursion. Need to fix it.

July 9:

Commits:

• Fixed infinite recursion in xen_bus_dma_tag_create()
• Added refcount check before freeing xentag in xen_bus_dma_tag_destroy()

Summary:

• Fixed the bug in xen_bus_dma_tag_create() which would cause an infinite recursion if a driver creates a tag. Achieved this by passing parent to common_bus_dma_tag_create() instead, and pass the parent tag’s machine dependent tag to bus_dma_tag_create().
• This raises another problem: xen_get_dma_tag() calls xen_bus_dma_tag_create() with the machine dependent dma tag in the parent argument. So there is need for a way to signal to xen_bus_dma_tag_create() that this is a special case where we are “bootstrapping” the dma tag. Used BUS_DMA_BUS1 flag here, which is reserved for the busdma functions to use as they wish.
• Added a refcount check in xen_bus_dma_tag_destroy(), so that we don’t free an in-use tag.

July 10-11:

Commits:

• Shift ds_addr by PAGE_SHIFT before granting foreign access
• Fixed KASSERT condition in xbd_indirectpage_cb()
• Disable indirection pages if the load for them fails.
• Added the function xen_dmamap_get_grefs
• Fixed passing wrong tag to common_bus_dma_tag_create()
• Updated xbd_mksegarray() to use the xen-specific busdma implementation

Summary:

• Filled the second GSoC evaluation.
• Multiple minor fixes like disabling indirection pages if the load for them fails, shifting ds_addr by PAGE_SHIFT.
• Started updating xbd_mksegarray() to use the busdma implementation.
• There is a problem: xbd_mksegarray() uses ds_addr, but we replace it with the grant reference, so xbd_mksegarray() will not work properly without it.
• Introduced the function xen_dmamap_get_grefs() that client drivers can call to access the grant references. This makes it possible to access ds_addr and the grant refs simultaneously.
• Updated xbd_mksegarray(), xbd_queue_request(), and xbd_int() to use the busdma implementation.
• The blkfront driver is completely updated to use the Xen-specific busdma implementation now. What remains is to, of course, test it.
• I need to build the world and kernel, and it takes a really long time. I’ll probably leave it running overnight.
• Also need to figure out how to convert the source build into an ISO so I can run it as a guest.

July 12-13:

Summary:

• Time to test the code I wrote.
• Roger suggests I grab one of the pre-built VM images from the FreeBSD FTP server and run the build in them.
• Downloaded the image and tried to boot. The boot gets stuck somewhere in the early stages.
• After a lot of looking around, I decide to try the disk image in RAW format and not in VHD format.
• Voila! It boots.
• Now I need to set up networking so I can fetch the FreeBSD sources and build them.
• Turns out it is much tougher than I expected.
• The official Xen guide does not work. The VM detects xenbr0 but it does not correctly set up the network. I can’t SSH into the VM from dom0 or connect to the internet.
• Googled around more. No success.
• I caught a lucky break. As a last resort, I try the lxcbr0 bridge I set up for a VirtualBox VM some time ago, and I had it lying around in my network interfaces. It works!
• After some tinkering around with the DNS settings, networking finally works in the VM.
• Checked out the code from my repo and compiled.
• The kernel panics on boot.
• The panic comes from a failed assertion inxbd_indirectpage_cb(). After some looking around, I realise the assertion condition is wrong. I was using sc->xbd_max_request_segments and I should instead have used sc->xbd_max_indirectpages.
• Fixed it.
• The kernel now boots. But it keeps throwing some error messages every few seconds. Looked around, but couldn’t fix it. Need to ask Roger about it.

For summary after the second evaluation, check the new post here