|
From: | geoff |
Subject: | Re: [Qemu-devel] ivshmem Windows Driver |
Date: | Wed, 18 Oct 2017 17:56:13 +1100 |
User-agent: | Roundcube Webmail/1.2.3 |
On 2017-10-18 17:50, Ladi Prosek wrote:
On Wed, Oct 18, 2017 at 7:50 AM, <address@hidden> wrote:On 2017-10-18 16:31, Ladi Prosek wrote:Hi Geoff, On Mon, Oct 16, 2017 at 8:31 PM, <address@hidden> wrote:Hi Yan & Ladi.I have written an initial implementation that supports just the sharedmemorymapping at this time. I plan to add events also but before I go further Iwould like some feedback if possible on what I have implemented thus far. Please see: https://github.com/gnif/kvm-guest-drivers-windows/commit/8655cf12fbdd77b991f96d97bc20f967b5907c12Thank you, looks good overall. * Please don't use the 'vio' prefix for this driver. ivshmem is not a VirtIO device (i.e. not using the VirtIO protocol). Also the test program should live in a subdirectory, so maybe something like /ivshmem and /ivshmem/test.Noted, I will remove the prefix throughout and move the test application.* In VIOIVSHMEMEvtDevicePrepareHardware: I don't think that Windows guarantees that resources are enumerated in BAR order. In VirtIO drivers we read the PCI config space to identify the BAR index: https://github.com/virtio-win/kvm-guest-drivers-windows/blob/master/VirtIO/VirtIOPCICommon.c#L353The windows 'toaster' sample relies on the resource order, but as a belt and braces approach I will update the code to use the same approach.Interesting, thanks! If that's really the case then we can remove the code from VirtioLib. I have cloned the latest Windows-driver-samples but can't find this under general/toaster. Namely ToasterEvtDevicePrepareHardware just prints some info about all resources but does not do anything order-related. Can you point me to the right code?
Sorry, my mistake, it wasn't the toaster code but the kmdf driver, it assumes the BAR ordering to determine which is which. https://github.com/Microsoft/Windows-driver-samples/blob/aa6e0b36eb932099fa4eb950a6f5e289a23b6d6e/general/pcidrv/kmdf/HW/nic_init.c#L649
* IOCTL codes on Windows have a structure to them: https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/defining-i-o-control-codesThanks, I will fix this.* In VIOIVSHMEMEvtIoDeviceControl: The "only one mapping at a time is allowed" test has a race. I think that simply making the IO queue WdfIoQueueDispatchSequential instead of WdfIoQueueDispatchParallel will fix it.Good point, I will change this.* According to MSDN, MmMapLockedPagesSpecifyCache(UserMode) should be wrapped in try/except. Also, what happens if the file handle is inherited by a child process? Can it unmap the mapping in parent's address space? What if the parent exits? A possible solution is discussed in this article: http://www.osronline.com/article.cfm?article=39Noted re try/except. As for a child inheriting it, the owner is tracked by the WDFFILEOBJECT, which the child I believe will inherit also, which would mean that the child would gain the ability to issue IOCTLs to themapping.Thanks! LadiNo, thank you! I am grateful someone is willing to provide some feedbackon this. I have been working on adding MSI interrupt support to the driver also which is close to ready, just trying to figure out why the driverfails to start with STATUS_DEVICE_POWER_FAILURE when I try to setup theIRQs with WdfInterruptCreate. Thanks again, Geoff
[Prev in Thread] | Current Thread | [Next in Thread] |