On 8/21/17 10:24 AM, Laszlo Ersek wrote:
[Snip]... > (25) I think you intended to add the empty line above the "ReleaseQueue" > label, not below it. > >> VirtioRingUninit (Dev->VirtIo, &Dev->Ring); >> >> Failed: > (26) You forgot to call UnmapSharedBuffer() in the VirtioBlkUninit() > function. Each ring must be unmapped in three places: > > - in the device init function, if there is an error and we're past > mapping the ring, > - in the device uninit function, > - in the ExitBootServices notify function. I will go through each of your comments and try to address them in v3 but I would like to understand this code flow a bit more. When we invoke PciIo->UnmapBuffer(...,Map) [1], it searches for Mapping in its internal list and if found then unmaps the buffer. So idea is each Map() should have exactly one Unmap(). So the question: is it possible that we may get called from both ExitBootService notify and device uninit functions ? If so, then we will have issue because now we will end up calling Unmap() twice (once from ExitBootServices then device uninit). In my debug statement I saw the call to ExitBootServices but was never able to trigger code path where the device uninit is called. I am wondering if you know any method to trigger the device uninit code flow so that I can verify both the path. [1] https://github.com/tianocore/edk2/blob/master/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c#L316 >> @@ -885,6 +974,12 @@ VirtioBlkExitBoot ( >> // >> Dev = Context; >> Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); >> + >> + // >> + // Unmap the ring buffer so that hypervisor can not get a readable data >> + // after device is reset. >> + // >> + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMapping); >> } >> >> /** >> > I think I'm skipping the rest of the series for now (except the last > patch, I have comments for that). > > Please rework the rest of the transport-independent virtio drivers (scsi > and net) based on my comments for this (blk) and the previous (rng) patch. > > Thanks! > Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
