https://bugzilla.redhat.com/show_bug.cgi?id=1716248
Previous commit 2eea7a77b7 sets timeout on waiting
for response on control command. The command stops waiting
after 1,000 attempts to get the response with 1 microsecond
wait between attempts. The intention was to prevent infinite
wait in case of unexpected IOCTL after the miniport received
shutdown indication (note that NDIS should not call adapter's
callbacks after that, see
https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/ndis/nc-ndis-miniport_shutdown).
In this case the control queue is not initialized and
the QEMU does not receive the command and does not send
the response. But in case the limit is not sufficient and
the timeout happens during regular initialization flow and
QEMU does send the response, further commands might be
executed incorrectly and cause unpredictable result and
loss of functionality. Example of such case is sending
VIRTIO_NET_CTRL_MQ when the VM uses only part of queues (the
QEMU issues rollback, prints error message etc).
We increase the limit to 500,000 attempts.
Signed-off-by: Yuri Benditovich <[email protected]>
Removed support for RSC implemented in private qemu patch.
Now the driver supports RSC according to the virtio spec
and upstream qemu code. The RSC feature is supported for
V1 device and for legacy device as well.
Signed-off-by: Yuri Benditovich <[email protected]>
Virtio-net header in TX direction is at (pages start)+ 4 bytes.
This causes misaligment exception when preparing the packet.
Even this is probably bug of ARM64 code generation, the simplest
solution is to make the virtio-net header to be always at +8 bytes.
Signed-off-by: Yuri Benditovich <[email protected]>
The macro ETH_COMPARE_NETWORK_ADDRESSES_EQ causes unaligned
access exception on ARM64. To solve this, we will use this
macro on non-ARM systems and on ARM64 systems will use
byte-by-byte comparison.
The SDV build (msbuild /t:sdv /inputs:"/check") crashes
on this line. This looks like a bug of SDV, but we change
it in order to make transition to 2017 faster.
Signed-off-by: Yuri Benditovich <[email protected]>
https://bugzilla.redhat.com/show_bug.cgi?id=1666940
Set clear and simple rule for assignment of vectors to queues
and translation of MSIX messages to queues:
in case of one or two vectors they are assigned
to TX/RX queues and CX reuses the last vector. In this case
we do not spawn DPC for CX and process it during regular
RX/TX processing. For case of 3 and more vectors the processing
is the same as before.
Signed-off-by: Yuri Benditovich <[email protected]>
Windows with NetKVM has a little chance hang during shutdown.
If the NetKVM has not initialized yet (or reset), QEMU will ignore
the control message. It will cause driver hanging in infinite while loop
for waiting response (which leads to windows hang in shutdown). In our
test, there is a little chance to receive an OID for setting multicast list
after device reset. To make symptom more clear, I have added some log.
Below is the log and call stack when this situation happens:
Logs:
[ParaNdis_OnShutdown]=>
Common [ParaNdis_ResetVirtIONetDevice] Done
[ParaNdis_OnOidSetMulticastList] Go
[ParaNdis_UpdateDeviceFilters] Do
[CParaNdisCX::SendControlMessage] enter: 4277 - cls: 0, cmd: 0
CParaNdisCX::SendControlMessage - ERROR: in while
Assertion: *** DPC watchdog timeout
This is NOT a break in update time
This is most likely a BUG in an ISR
Perform a stack trace to find the culprit
The period will be doubled on continuation
Use gh to continue!!
Call stack:
kd> k
ChildEBP RetAddr
8b582530 83ab50be nt!KeAccumulateTicks+0x3c5
8b582570 83ab4f6b nt!KeUpdateRunTime+0x145
8b5825c8 83ab9c17 nt!KeUpdateSystemTime+0x613
8b5825c8 83a10122 nt!KeUpdateSystemTimeAssist+0x13
8b582680 8e16db70 hal!HalpTscStallExecutionProcessor+0xc0
WARNING: Stack unwind information not available. Following frames may be
wrong.
8b582700 8e16b216 netkvm+0x4b70
8b582730 8e16c418 netkvm+0x2216
8b582740 8e16f219 netkvm+0x3418
8b582750 8e17740f netkvm+0x6219
8b5827a8 880fc32b netkvm+0xe40f
8b5827f0 880a8ce3 ndis!ndisMDoOidRequest+0x3a1
8b582818 880a87ff ndis!ndisFQueueRequestOnNext+0x21e
8b582880 8df21086 ndis!NdisFOidRequest+0x181
8b5828a0 880fc961 pacer!PcFilterRequest+0x78
8b582900 880a8c5c ndis!ndisFDoOidRequest+0x1a2
8b582928 880a87ff ndis!ndisFQueueRequestOnNext+0x197
8b582990 8df15b22 ndis!NdisFOidRequest+0x181
8b5829b0 880fc961 wfplwf!FilterOidRequest+0x50
8b582a10 880a7799 ndis!ndisFDoOidRequest+0x1a2
8b582a3c 8810d602 ndis!ndisQueueRequestOnTop+0x224
8b582ab0 8810d686 ndis!ndisMOidRequest+0x1dd
8b582ac0 882a4646 ndis!NdisOidRequest+0xf
8b582b7c 88254d1d tcpip!FlpNdisRequestUnderReference+0x7d
8b582bb8 8825523b tcpip!FlpMulticastListSet+0xa1
8b582bc4 88251a2e tcpip!FlpSerializedNdisDeleteGroupWorker+0xd
8b582bec 83c4f460 tcpip!FlpSerializedNdisRequestWorkerRoutine+0x41
8b582c00 83ab7aab nt!IopProcessWorkItem+0x23
8b582c50 83c43f5e nt!ExpWorkerThread+0x10d
8b582c90 83aeb219 nt!PspSystemThreadStartup+0x9e
00000000 00000000 nt!KiThreadStartup+0x19
Signed-off-by: yuchenlin <[email protected]>
Upon surprise removal all the packets in use must be
collected and completed and further regular releases
disallowed even if the procedure will be called by
misunderstanding. Current commit adds the queue interface
that allows collecting used TX descriptors without using
GetBuf as these packets are not completed by the device.
Signed-off-by: Yuri Benditovich <[email protected]>
WHCK requires adapter speed to be equal to maximal speed, so
it fails with default run of qemu.
In addition, it is easy to have meaningless speed report
with parameters provided in qemu command line, as there is
almost no validation of values received from the configuration.
Current commit sets following order of assignment and report:
At the beginning we set adapter speed as default=100 Gbps.
We do not assign invalid values from device configuration.
We always use the same value for reports of current speed
and of maximal speed.
Signed-off-by: Yuri Benditovich <[email protected]>
SDV code analysis threw warning C28023: The function being assigned or
passed should have a _Function_class_ annotation for at least one of the
class(es) in: 'KDEFERRED_ROUTINE' line 47 in file netkvm\common\parandis-cx.cpp
The warning was generated because KeInitializeDPc function requires its
second argument to be a function with KDEFERRED_ROUTINE class, and
MiniportMSIInterruptCXDpc function that is passed as the second argument
was not declared with that function class.
so proper declaration was added to the function.
Signed-off-by: Basil Salman <[email protected]>
SDV code analysis threw warning C26110: Caller failing to hold
lock LockedContext line 324 before it being destructed at the
end of the block, The lock is actually being held locked but
SDV does not seem to recognize that due to encapsulation.
so the warning was suppressed.
Signed-off-by: Basil Salman <[email protected]>
SDV code analysis threw a warning regading using uninitialized memory
'addr' line 384 in parandis-virtio.cpp.
line 384 prints the address of the block that was freed using
NdisFreeMemoryWithTagPriority the line before, this was not a problem
since it only prints the address that pointer addr points to. and since
NdisFreeMemoryWithTagPriority is a void function that can't fail,
switching the lines and printing before the actual free, shall eliminate
the warning without affecting the code's functionality.
Signed-off-by: Basil Salman <[email protected]>
If power on procedure fails, it still must resume the
state machine, otherwise following pause flow never
completes.
Signed-off-by: Yuri Benditovich <[email protected]>
Now that in MSI interrupts we have a separate DPC for CX, we don't need
to check if an interrupt was reported or report an interrupt for any of
the paths as we already used this flag for CX only when MSI interrupts
are used.
For legacy interrupt we always set the report interrupt flag and then
clear it in CX handling, this work is superfluous as we always want to
check if CX has any work to do with legacy interrupts. Setting and
unsetting this flag constantly is a good example of race conditions that
might lead us to miss some interrupts for cx.
These race conditions were initially reported and discussed in the
following pull request:
https://github.com/virtio-win/kvm-guest-drivers-windows/pull/281
Signed-off-by: Sameeh Jubran <[email protected]>
This commit separates the CX path handeling to a standalone DPC, the
motive to do this is that sometimes we can miss Control interrupts due to
the way it is implemented today.
The issue is disscussed in great detail here:
https://github.com/virtio-win/kvm-guest-drivers-windows/pull/281
Signed-off-by: Sameeh Jubran <[email protected]>
The VIRTIO 1.0 spec adds the DEVICE_NEEDS_RESET bit to the device's
status. This commit enables the driver to respods appropriately to
RESET request by the device.
Upon receiving such notification from the device in the control queue we
do the following steps in the following order:
* On Control interrupt read the device status and check that
VIRTIO_CONFIG_S_NEEDS_RESET bit is set and VIRTIO 1.0 is supported.
* Update the state machine that the device needs a reset, the state
machine updates all queues using the obervee/oberver logic that they
can't touch the hardware.
* Lastly the flag bDeviceNeedsReset is set to true. By setting this flag
to true we inform Windows that the ParaNdis6_Reset should be called by
NDIS. This happens beacuse in ParaNdis6_CheckForHang we return the value
of the bDeviceNeedsReset flag which if set instructs Windows to call
reset function. The ParaNdis6_CheckForHang is called periodically by
NDIS every few seconds.
Signed-off-by: Sameeh Jubran <[email protected]>
Signed-off-by: Basil Salman <[email protected]>
This patch enables the state machine to notify all of it's observers
when the device needs a reset or when the device has been powered on.
Signed-off-by: Sameeh Jubran <[email protected]>
In the CX DPC path we execute the GuestAnnounce feature without checking
if CX was actually acked. This commit adds this check.
Signed-off-by: Sameeh Jubran <[email protected]>
Currently we store only the last time stamp for an interrupt which is okay
for legacy interrupts but not okay for msi as it neglects the fact that
interrupts are tide to paths.
Signed-off-by: Sameeh Jubran <[email protected]>
The Add method should return the size of list after the element has been
added. This method will always return 0 when the list's typename is
CNonCountingObject.
Signed-off-by: Sameeh Jubran <[email protected]>
In commit e8f319fdc0 the lock free queue was
introduced as an alternative implementation to the previous one which
required acquiring the TX's path lock for NBLs sent by Windows. The
intention for the implementation introduced was that this lock acquirement
would be dropped in favour of scheduling a DPC upon sending an NBL instead.
While this DPC scheduling approach works it does introduce a heavy DPC
scheduling on the environment, the overhead can be as high as executing a
DPC for each NBL sent by Windows in the worst case scenario. In general
Windows should drop duplicate DPCs but it seems that it is not doing so
efficiently or else it is executing a DPC upon each schedule request by us
in a heavy load environment.
An alternative implementation which would probably allow us to keep the
DPC scheduling implementation is adding a mechanism to identify bursts and
forecast when we are receiving a high packet rate and adapt (lower) the
DPC scheduling accordingly.
This commit enables the HCK/HLK 1c_Mini6PerfSend test to pass as it was
stuck due to this heavy DPC load environment.
Signed-off-by: Sameeh Jubran <[email protected]>
VIRTIO_NET_F_STATUS off misbehaviours fixed:
1. Reading the current link state,
fix: ReadLinkState is not to be called in this case.
2. Link state is not initialized,
fix: pContext->bConnected initialized to TRUE.
Signed-off-by: Bishara AbuHattoum <[email protected]>
Currently when we recieve a suprise remove IRP from Windows we touch
the hardware of the device which is an invalid memory. This doesn't
cause BSOD in HCK/HLK tests as the device is still in qemu and memory is
still valid.
However, when performing hibernate on a given client and
then - while the client is off - we remove the virtio-net from the
command line and boot the client, a BSOD occurs as a result of this
illegal access to memory.
This commit solves this issue by registering the paths for notifications from
the state machine and preventing the driver from touching hardware upon a
suprise removal notification from the state machine.
Signed-off-by: Sameeh Jubran <[email protected]>
This commit integrate the observer / observee design pattern between the
State Machine and the paths. Now paths are able to register for
notifications from the state machine.
Signed-off-by: Sameeh Jubran <[email protected]>
Clearing the variables is important as calling the Dotr twice on the
same object will cause BSODs as a result of a double free for example.
Signed-off-by: Sameeh Jubran <[email protected]>
Yuri Benditovich