microsoft
Implement Drop for vfio_sys Device, unmapping msix for interrupts.
GDMA Driver allocates the HWC device interrupt in the new() method. However, there are multiple potential error paths that can result in retries. Any failure in new() causes the interrupt to leak, resulting in the HWC interrupt not getting unmapped. GDMA::New() can then fail on retry due to missing first response as a result of the leaked interrupt.
Log messages from a lab machine where I force a panic, causing calls to drop() -> map_msix()
[10.065737] mana_driver::gdma_driver: INFO worker_new{ name="UnderhillWorker" action="new"}:init:init/new_underhill_vm:new_mana_device: Created HWC with eq id: 0, msix: 0
[10.065969] mana_driver::gdma_driver: INFO worker_new{ name="UnderhillWorker" action="new"}:init:init/new_underhill_vm:new_mana_device: Max VF resources: GdmaQueryMaxResourcesResp { status: 0, max_sq: 15360, max_rq: 15360, max_cq: 15360, max_eq: 960, max_db: 4096, max_mst: 57344, max_cq_mod_ctx: 4, max_mod_cq: 32, max_msix: 6 }
[10.070091] vfio_sys: INFO worker_new{ name="UnderhillWorker" action="new"}:init:init/new_underhill_vm:new_mana_device: Dropping vfio_sys::Device
[10.070174] vfio_sys: INFO worker_new{ name="UnderhillWorker" action="new"}:init:init/new_underhill_vm:new_mana_device: vfio_sys::map_msix header.count 0 VFIO_IRQ_SET_DATA_NONE
[10.147768] underhill_core::emuplat::netvsp: ERROR worker_new{ name="UnderhillWorker" action="new"}:init:init/new_underhill_vm: failed to create mana device pci_id="af87:00:00.0" reset_method=NoReset err=failed to initialize mana device: injecting fake HW failure on 18 attempt
In FHR scenarios, the GDMA::new() can fail due to missing the first response as a result of a leaked interrupt.
Log messages from a lab machine where I perform UH servicing (FHR)
[71955.164522] state_unit: INFO servicing_save_vtl2{ correlation_id=4d9f7e7f-158c-46e4-b3a6-2445e35cc9dc}:save_units:state_change{ operation="save"}: state change complete duration=16.971171ms
[71955.166572] vfio_sys: INFO Dropping vfio_sys::Device
[71955.166632] vfio_sys: INFO vfio_sys::map_msix header.count 0 VFIO_IRQ_SET_DATA_NONE
It looks to me like Linux should already do this when the device is closed. Are we leaking the device fd somehow?
Are we leaking the device fd somehow?
I don't believe the fd gets leaked. But without this Drop implementation, there is no code to tell the VFIO device 'unmap_interrupt' / VFIO_IRQ_SET_DATA_NONE.
When the fd gets closed, the kernel does VFIO_IRQ_SET_DATA_NONE internally.
The failure occurs in the retry case. There is logic to perform an FLR and try again to create a hardware channel if the first try was not successful (for example if initial commands time out). Are we not closing the fd before the retry case and thus necessitating this interrupt unmapping?
We still observed a problem with the retry even with these changes. Back to the drawing bord.