]> xenbits.xensource.com Git - pvdrivers/win/xenvif.git/log
pvdrivers/win/xenvif.git
3 years agoWindows PV drivers fail to set up RSS when vCPU index >= 8
Martin Harvey [Wed, 23 Mar 2022 10:33:47 +0000 (10:33 +0000)]
Windows PV drivers fail to set up RSS when vCPU index >= 8

The driver only supports at most 8 queues, however Windows
can decide to assign vCPU numbers starting from a non-zero
offset. E.g. vCPU 8,9,10,11 could get assigned to a device
if you have more than one NIC. The total number of vCPUs
used by a single device is still less than 8, but the vCPU
indexes themselves can be greater than 8. The code
previously incorrectly assumed that individual vCPU
indexes cannot exceed 8, however a 1:1 mapping between
vCPU indexes and queues seems to only exist when using
a single NIC.

Signed-off-by: Martin Harvey <martin.harvey@citrix.com>
Removed unncessary error handling and tidied up.

Signed-off-by: Paul Durrant <paul@xen.org>
3 years agoAdd /CETCOMPAT to linker options
Owen Smith [Mon, 29 Nov 2021 09:58:37 +0000 (09:58 +0000)]
Add /CETCOMPAT to linker options

Signed-off-by: Owen Smith <owen.smith@citrix.com>
3 years agoFix CodeQL warnings
Owen Smith [Thu, 12 Aug 2021 12:44:27 +0000 (13:44 +0100)]
Fix CodeQL warnings

- ExAllocatePoolWithTag has been deprecated with Win10 2004, use
    ExAllocatePoolUninitialized instead
- Check return value of _strtoui64 for error condition as indicated by
    _UI64_MAX

Signed-off-by: Owen Smith <owen.smith@citrix.com>
3 years agoFix SDV/CodeQL log generation
Owen Smith [Thu, 12 Aug 2021 12:44:26 +0000 (13:44 +0100)]
Fix SDV/CodeQL log generation

- sarif files need to be stored with SDV logs when generating the DVL file
- Disable PREFast and CodeAnalysis by default
- Run a seperate CodeAnalysis build after SDV, but before generating DVL file
    DVL file should contain multiple summary lines for SDV, at least 1 line
    for CodeAnalysis and at least 1 line for Semmle (CodeQL)

Signed-off-by: Owen Smith <owen.smith@citrix.com>
3 years agoDocument CodeQL build requirements
Owen Smith [Tue, 7 Sep 2021 08:10:46 +0000 (09:10 +0100)]
Document CodeQL build requirements

CodeQL requires an additional tool and rule set which are seperate from the
EWDK ISOs, and require manual installation and configuration

Signed-off-by: Owen Smith <owen.smith@citrix.com>
3 years agoFix build with later WDKs
Owen Smith [Thu, 12 Aug 2021 12:44:25 +0000 (13:44 +0100)]
Fix build with later WDKs

- Adds alias for GetProjectInfoForReference target to version.vcxproj
    Later kits seemed to have renamed the build target, and will fail without
    this alias target.
- Adds "/fd sha256" to signtool command line
    WDK 20344 and later require binaries signed with a SHA256 file digest, or
    the build outputs are deleted
- Disables warning 4061 - switch statement on enum types need to have a case
    for all values of the enumeration

Signed-off-by: Owen Smith <owen.smith@citrix.com>
- Cast enum types used as array indices to avoid bounds check complaint

Signed-off-by: Paul Durrant <paul@xen.org>
3 years agoFix semantics of ASSERT3[P|S|U]
Paul Durrant [Mon, 6 Sep 2021 08:43:46 +0000 (09:43 +0100)]
Fix semantics of ASSERT3[P|S|U]

These ASSERTions are supposed to cast their arguments to pointer, signed or
unsigned values (respectively) before applying the operator. This is not
done correctly; the test and __analysis_assume() directive in the underlying
ASSERT() macro are applied to the un-cast values. This patch rectifies the
situation.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
3 years agoAdd ring disconnect logging.
Martin Harvey [Tue, 17 Aug 2021 08:33:04 +0000 (09:33 +0100)]
Add ring disconnect logging.

Logging to note error cases which would otherwise be silent in release builds.
This aids greatly with customer debugging.

Signed-off-by: Martin Harvey <martin.harvey@citrix.com>
Cosmetic fixes.

Signed-off-by: Paul Durrant <paul@xen.org>
3 years agoReduce logging of Fdo->NotDisableable
Martin Harvey [Wed, 28 Jul 2021 14:02:47 +0000 (15:02 +0100)]
Reduce logging of Fdo->NotDisableable

Signed-off-by: Martin Harvey <martin.harvey@citrix.com>
3 years agoAssert that transmitter not being sent NULL MDL's
Martin Harvey [Wed, 28 Jul 2021 14:02:46 +0000 (15:02 +0100)]
Assert that transmitter not being sent NULL MDL's

An assertion to check that a corresponding fix in XenNet is preventing
NULL MDL's from being queued for transmission.

Signed-off-by: Martin Harvey <martin.harvey@citrix.com>
3 years agoLog waits for backend state changes.
Martin Harvey [Wed, 28 Jul 2021 14:02:45 +0000 (15:02 +0100)]
Log waits for backend state changes.

In addition to preceding changes to ring disconnects, and
associated logging, we also add some logging to check
whether state change notifications are being sent in a timely
manner between frontend and backend. Also a great assistance
to customer debugging.

Signed-off-by: Martin Harvey <martin.harvey@citrix.com>
3 years agoImprove transmitter disable.
Martin Harvey [Wed, 28 Jul 2021 14:02:43 +0000 (15:02 +0100)]
Improve transmitter disable.

In some cases, the backend may stop processing Tx ring
requests in a timely manner. If this happens at the same time as ring
disable, then the Tx code could spin forever at dispatch IRQL.

This patch introduces a hard limit on how long the code will spin,
allowing a device disable or power transition to complete, albeit
at the cost of Tx requests being dropped or the ring being in
an indeterminate state. This has normally been seen at guest shutdown
where final ring state is of little consequence.

Signed-off-by: Martin Harvey <martin.harvey@citrix.com>
3 years agoFail if multi-queue-max-queues is 0
Owen Smith [Wed, 14 Jul 2021 10:39:43 +0000 (11:39 +0100)]
Fail if multi-queue-max-queues is 0

Validate backend has correctly set multi-queue-max-queues, and add ASSERTions
to NumQueues and MaxQueues accessors to validate they are not 0

Signed-off-by: Owen Smith <owen.smith@citrix.com>
4 years agoAdd CodeQL build stage
Owen Smith [Fri, 5 Mar 2021 10:16:20 +0000 (10:16 +0000)]
Add CodeQL build stage

CodeQL logs will be required for future WHQL submissions. Add a stage
that generates the required SARIF files. CodeQL is a semantic code
analysis engine, which will highlight vunerabilities that will need
fixing.

In order to use CodeQL, the CodeQL binaries must be on the path and the
Windows-Driver-Developer-Supplemental-Tools must be on the path defined
by the CODEQL_QUERY_SUITE environment variable (if defined), or under
the parent folder (if CODEQL_QUERY_SUITE variable is not defined)

Note: Due to the way the codeql command line is built, using quotes in a
MSBuild command line is not possible, so generate a batch file to wrap
the command line.

Signed-off-by: Owen Smith <owen.smith@citrix.com>
4 years agoBring XENBUS interface versions up to date...
Paul Durrant [Mon, 22 Feb 2021 10:37:14 +0000 (10:37 +0000)]
Bring XENBUS interface versions up to date...

... and update binding accordlingly.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
4 years agoDon't assume queue numbers can't be larger than 99
Paul Durrant [Tue, 1 Dec 2020 13:37:16 +0000 (13:37 +0000)]
Don't assume queue numbers can't be larger than 99

The theoretical maximum number of receiver or transmitter queues is limited at
the number of vCPUs in the VM, and this can be as many as 128 (limited by
Xen).

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
4 years agoInherit versioning info from environment if present
Nicholas Tsirakis [Fri, 6 Nov 2020 20:48:44 +0000 (15:48 -0500)]
Inherit versioning info from environment if present

As the drivers stabilize and mature, there is an ever-growing
chance that other opensource virtualization projects will adopt
them. Allow external projects to inject their own versioning
into the drivers instead of hardcoding the latest winpv version.

Signed-off-by: Nicholas Tsirakis <tsirakisn@ainfosec.com>
Acked-by: Owen Smith <owen.smith@citrix.com>
4 years agoAllow user to specify desired build architecture
Nicholas Tsirakis [Fri, 6 Nov 2020 20:48:43 +0000 (15:48 -0500)]
Allow user to specify desired build architecture

Often times we only need to build a driver for a single
targeted architecture. Continue to build both by default,
but allow the user to specify one if desired.

Signed-off-by: Nicholas Tsirakis <tsirakisn@ainfosec.com>
Use [string]::IsNullOrEmpty($Arch)

Signed-off-by: Paul Durrant <paul@xen.org>
4 years agoDont mix Exclusive and Shared locking calls
Owen Smith [Tue, 4 Aug 2020 14:49:21 +0000 (15:49 +0100)]
Dont mix Exclusive and Shared locking calls

If the lock is acquired with ExAcquireSpinLockSharedAtDpcLevel(), it
should always be released with ExReleaseSpinLockSharedFromDpcLevel()
(unless the lock is converted to an exclusive lock)

Signed-off-by: Owen Smith <owen.smith@citrix.com>
4 years agoDon't pass MM_DONT_ZERO_ALLOCATION to MmAllocatePagesForMdlEx()...
Paul Durrant [Tue, 16 Jun 2020 12:48:27 +0000 (13:48 +0100)]
Don't pass MM_DONT_ZERO_ALLOCATION to MmAllocatePagesForMdlEx()...

...in __AllocatePages()

See commit 4f85d004 "Replace uses of MmAllocatePagesForMdlEx in
__AllocatePage" for more background.

In summary, it is to avoid BSOD 139 1e with a stack similar to the following:

nt!KeBugCheckEx
nt!KiBugCheckDispatch+0x69
nt!KiFastFailDispatch+0xd0
nt!KiRaiseSecurityCheckFailure+0x30e
nt!KiAcquireThreadStateLock+0x11fa90
nt!KeSetIdealProcessorThreadEx+0xd0
nt!MiZeroInParallelWorker+0x115016
nt!MiZeroInParallel+0x11c
nt!MiInitializeMdlBatchPages+0x2ae
nt!MiAllocatePagesForMdl+0x192
nt!MmAllocatePartitionNodePagesForMdlEx+0xc9
nt!MmAllocatePagesForMdlEx+0x4d

These bugchecks have been observed in recent updates of Server 2019.

This patch, rather than replacing calls to MmAllocatePagesForMdlEx() with
calls to MmMapLockedPagesSpecifyCache(), just avoids passing
MM_DONT_ZERO_ALLOCATION to work round the bug.

The patch instead passes MM_ALLOCATE_FULLY_REQUIRED, which arguably should
have always been passed for allocations larger than a single page. It also
fixes a formatting issue.

Reported-by: Jan Bakuwel <jan.bakuwel@gmail.com>
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---

This fix will also be propogated to all other PV drivers.

5 years agoMake sure fixed size CHAR arrays can fit MAX_ULONGs
Owen Smith [Mon, 27 Apr 2020 10:38:47 +0000 (11:38 +0100)]
Make sure fixed size CHAR arrays can fit MAX_ULONGs

When formatting strings with %u, make sure the target buffer is large
enough to contain MAX_ULONG.

Signed-off-by: Owen Smith <owen.smith@citrix.com>
5 years agoHandle return codes from MSBuild
Owen Smith [Tue, 10 Mar 2020 10:47:03 +0000 (10:47 +0000)]
Handle return codes from MSBuild

If MSBuild fails, it returns a non-zero return value. Forward this
failure to the calling scripts to fail earlier.

Signed-off-by: Owen Smith <owen.smith@citrix.com>
5 years agoMAINTAINERS: Update my email address
Paul Durrant [Tue, 10 Mar 2020 10:56:55 +0000 (10:56 +0000)]
MAINTAINERS: Update my email address

My @amazon.com address is no longer convenient to use.

Signed-off-by: Paul Durrant <paul@xen.org>
5 years agobump version to 9.1.0
Paul Durrant [Fri, 6 Dec 2019 12:11:48 +0000 (12:11 +0000)]
bump version to 9.1.0

Now that 9.0 is branched (in staging-9.0) the master version needs to be
advanced.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
5 years agoMAINTAINERS: Update my email address staging-9.0 9.0.0 9.0.0-rc1
Paul Durrant [Thu, 14 Nov 2019 10:11:28 +0000 (10:11 +0000)]
MAINTAINERS: Update my email address

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
5 years agoReduce xenstore churn
Owen Smith [Wed, 13 Nov 2019 15:43:54 +0000 (15:43 +0000)]
Reduce xenstore churn

Only write the complete changes to the xenstore MAC address table. There
is no need to write the complete table for each addition or removal,
when additions and removals are processed in a single update.

Pushes the responsibility for writing the MAC table to
FrontendSetMulticastAddresses() once all changes are made, instead of
for each call to MacAddMulticastAddress() or MacRemoveMulticastAddress()

Signed-off-by: Owen Smith <owen.smith@citrix.com>
5 years agoAdd support for EWDK_19h1_release_svc_prod3_18362_190416-1111
Paul Durrant [Thu, 19 Sep 2019 10:49:42 +0000 (11:49 +0100)]
Add support for EWDK_19h1_release_svc_prod3_18362_190416-1111

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
5 years agoMAINTAINERS: Update my email address
Paul Durrant [Thu, 19 Sep 2019 10:20:32 +0000 (11:20 +0100)]
MAINTAINERS: Update my email address

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
5 years agoRemove the old python build scripts and document use of the EWDK
Paul Durrant [Thu, 19 Sep 2019 10:12:12 +0000 (11:12 +0100)]
Remove the old python build scripts and document use of the EWDK

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
5 years agoRemove erroneously committed file
Paul Durrant [Wed, 17 Jul 2019 12:27:15 +0000 (13:27 +0100)]
Remove erroneously committed file

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
5 years agoUpdate cache_interface header
Owen Smith [Wed, 17 Jul 2019 12:18:33 +0000 (13:18 +0100)]
Update cache_interface header

Requires bumping the INF revision and revision table

Signed-off-by: Owen Smith <owen.smith@citrix.com>
5 years agoUse genfiles.ps1 to generate inf date
Owen Smith [Wed, 17 Jul 2019 12:24:36 +0000 (13:24 +0100)]
Use genfiles.ps1 to generate inf date

stampinf is no longer used, so genfiles.ps1 must be able to write the
DriverVer field in the inf file.

Signed-off-by: Owen Smith <owen.smith@citrix.com>
5 years agoAdd PowerShell build scripts, version.vcxproj
Owen Smith [Fri, 14 Jun 2019 16:16:20 +0000 (17:16 +0100)]
Add PowerShell build scripts, version.vcxproj

Based on the sequence of commits to xenbus, add powershell scripts to
build the solution using the EWDK
version.vcxproj generates versioned files (version.h and xenvif.inf) using
 scripts/genfiles.ps1
Strips duplicated functionality from build.py to produce consistant
builds between python and powershell.

Signed-off-by: Owen Smith <owen.smith@citrix.com>
5 years agoAdd BUILD_NUMBER to reported version
Owen Smith [Fri, 14 Jun 2019 16:05:40 +0000 (17:05 +0100)]
Add BUILD_NUMBER to reported version

Signed-off-by: Owen Smith <owen.smith@citrix.com>
5 years agoStop lying about the lock state when calling into XENBUS_CACHE
Paul Durrant [Fri, 31 May 2019 09:53:45 +0000 (10:53 +0100)]
Stop lying about the lock state when calling into XENBUS_CACHE

Various cache access that used to be under protection of the reciver ring
lock are no longer subject to that lock, and thus the 'Locked' parameter
passed to the Get/Put methods needs to be FALSE rather than TRUE.

This bug was not spotted under recent changes applied to the implementation
of XENBUS_CACHE, which removed use of atomic bit operations.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoPort "Ignore unused devices when checking for compatability" from XENBUS
Paul Durrant [Thu, 20 Dec 2018 17:34:49 +0000 (17:34 +0000)]
Port "Ignore unused devices when checking for compatability" from XENBUS

XENBUS commit caf35361 fixes an upgrade issue with dangling references
left in ENUM keys. This patch applies the same fix to XENVIF.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoAllow advertised 'wire' speed to be overridden by a registry value...
Paul Durrant [Fri, 2 Nov 2018 17:16:58 +0000 (17:16 +0000)]
Allow advertised 'wire' speed to be overridden by a registry value...

...and change the default to 100G.

This patch adds code to check for a new 'MacSpeed' XENVIF parameter
(sampled at initialization time), defaulting to a wire speed of 100G if
it is not present (rather than the 1G default prior to this patch).

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoAdd a xenstore watch for the "speed" key...
Paul Durrant [Fri, 2 Nov 2018 15:51:31 +0000 (15:51 +0000)]
Add a xenstore watch for the "speed" key...

...and allow the value to specify units.

The 'wire' speed advertised by XENNET to the Windows network stack can be
controlled by the value of a "speed" key in the xenstore frontend area.
Values of this key are currently interpreted as decimal gigabits-per-second.

This patch sets a watch on the key (meaning changes to it now take
immediate effect) and allows units to be specified: 'G/g' for gigabits-per-
second, 'M/m' for megabits-per-second, and 'K/k' for kilobit-per-second.
Thus, for example, '100M' means 100 megabits-per-second. If no unit is
specified then the value is assumed to be in gigabits-per-second, as before.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoSet packet transmit completion information outside of ring lock
Paul Durrant [Fri, 2 Nov 2018 14:16:40 +0000 (14:16 +0000)]
Set packet transmit completion information outside of ring lock

There is no need to hold the ring lock to update the packet transmit
completion information, or update the frontend statistics. This patch
creates a new __TransmitterSetCompletionInfo() function and calls this
just before returning each packet to XENNET, after the ring lock has been
dropped.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoDon't force event channel unmasking
Paul Durrant [Fri, 2 Nov 2018 11:49:06 +0000 (11:49 +0000)]
Don't force event channel unmasking

The effect of efbe65aa "Make use of possible XENBUS_EVTCHN Unmask failure"
was lost when the poller subsystem was reverted. This patch re-instates
equivalent functionality.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoGet rid of the 'Retry' exit from the ring poll routines
Paul Durrant [Fri, 2 Nov 2018 11:42:47 +0000 (11:42 +0000)]
Get rid of the 'Retry' exit from the ring poll routines

This was supposed to allow better interleaving of receive and transmit
polling, but this does not really seem to help so it really only makes
the code more complex than it needs to be.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoRe-instate 264bde12 "Introduce a threaded DPC into the receiver code"
Paul Durrant [Fri, 2 Nov 2018 11:34:39 +0000 (11:34 +0000)]
Re-instate 264bde12 "Introduce a threaded DPC into the receiver code"

This patch re-works 264bde12 such that it apply (after reversion of the
poller subsystem), including re-naming the unqualified 'Dpc' and 'Dpcs'
fields in the receiver and transmitter ring structures to 'PollDpc' and
'PollDpcs' to disambiguate them from the new threaded 'QueueDpc' and
associated 'QueueDpcs' counter.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoRemove KTIMERS from transmit and receive paths (again)
Paul Durrant [Fri, 2 Nov 2018 11:09:11 +0000 (11:09 +0000)]
Remove KTIMERS from transmit and receive paths (again)

Even though the poller subsystem has been reverted, these still need to
go away as they do not adequately serve the purpose for which they were
added: DPC watchdog avoidance.

A subsequent patch will re-instate the threaded DPC in the receiver to
avoid the DPC watchdog.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoRevert complete poller subsystem
Paul Durrant [Thu, 1 Nov 2018 14:14:38 +0000 (14:14 +0000)]
Revert complete poller subsystem

It seems to have a bad effect on performance that various attempts at
improvement just don't seem to be able fix, so this patch just gets rid
of the whole thing.

This patch reverts the following commits:

 264bde12 "Introduce a threaded DPC into the receiver code"
 129ad516 "Stop using a threaded DPC in the poller"
 5932938b "Don't bump the receiver event counter if the poller is going to retry"
 16002b8f "poller: fix event channels when backends do not support multi-queue"
 bc722edd "Don't use KTIMERs in receive path"
 dfaa68cc "Don't affinitize timer DPC"
 eac9a95a "Move the Receiver and Transmitter event and DPC processing..."
 40be5c12 "Add the boilerplate for a new Poller sub-system"

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoRevert "Replace uses of MmAllocatePagesForMdlEx in __AllocatePage"
Paul Durrant [Thu, 1 Nov 2018 16:43:32 +0000 (16:43 +0000)]
Revert "Replace uses of MmAllocatePagesForMdlEx in __AllocatePage"

This reverts commit 4f85d004. This patch was put in place to work around
a bug in Windows but seems to have a serious negative effect on
performance. The bug in Windows has since been fixed so the patch can now
be reverted to recover performance.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoRevert reversion of "Deferring transmit completion causes...
Paul Durrant [Thu, 1 Nov 2018 14:59:10 +0000 (14:59 +0000)]
Revert reversion of "Deferring transmit completion causes...

... MPE_Ethernet test failures""

This reverts commit 0f91d01d as it is very detrimental to performance. It
appears that deferring transmit completion events roughly halves TCP
throughput.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoAdd missing status to error log
Owen Smith [Fri, 28 Sep 2018 12:12:14 +0000 (13:12 +0100)]
Add missing status to error log

Signed-off-by: Owen Smith <owen.smith@citrix.com>
6 years agoIntroduce a threaded DPC into the receiver code
Paul Durrant [Tue, 18 Sep 2018 13:48:49 +0000 (14:48 +0100)]
Introduce a threaded DPC into the receiver code

To avoid problems with DPC timeouts move the majority of the receiver's
work, and interaction with the network stack, into a threaded DPC. This
leaves the poll entry point (called from the now non-threaded poller DPC)
to simply service responses and build a local packet queue that is then
drained by the threaded DPC.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoAllow FrontendIncrementStatistic() to be called at < DISPATCH_LEVEL
Paul Durrant [Tue, 18 Sep 2018 13:34:23 +0000 (14:34 +0100)]
Allow FrontendIncrementStatistic() to be called at < DISPATCH_LEVEL

Swap the ASSERTion for a KeRaiseIrql().

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoUse a reader/writer lock in the mac module
Paul Durrant [Tue, 18 Sep 2018 13:30:21 +0000 (14:30 +0100)]
Use a reader/writer lock in the mac module

Each receiver thread needs to call MacApplyFilters() but the implementation
uses a normal kernel spin lock and hence the threads will all contend the
lock. Switch the mac code to use a reader/writer lock so that
MacApplyFilters() can acquire the lock as a reader and thus avoid causing
the contention.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoMake sure event counter is updated before finish receiver ring poll
Paul Durrant [Tue, 18 Sep 2018 13:24:15 +0000 (14:24 +0100)]
Make sure event counter is updated before finish receiver ring poll

There was a flaw in commit 5932938b "Don't bump the receiver event counter
if the poller is going to retry" in that it is possible to drop out of
poll without ever updating the event counter (if one attempt requests a
retry and the next attempt finds nothing to do). This patch fixes the
issue by using the RING_FINAL_CHECK_FOR_RESPONSES macro to update the
event counter, which checks for a race with new responses.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoStop using a threaded DPC in the poller
Paul Durrant [Tue, 18 Sep 2018 13:18:14 +0000 (14:18 +0100)]
Stop using a threaded DPC in the poller

The threaded DPC was introduced by commit bc722edd "Don't use KTIMERs in
receive path" but it appears to have too much of an impact on performance.
This patch reverts the poller to a normal DPC but does not introduce any
DPC timeout mitigation. That will be done by a subsequent patch.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoConditionally package DPInst
Owen Smith [Fri, 24 Aug 2018 16:46:43 +0000 (17:46 +0100)]
Conditionally package DPInst

Since DPInst.exe is not shipped with the Windows Driver Kit 10, an
environment variable must point to local copies. Make the inclusion of
DPInst conditional on DPINST_REDIST being defined and that path
existing. This simplifies building packages which do not require DPInst
for installation, and removes a required step to create a working build.

Signed-off-by: Owen Smith <owen.smith@citrix.com>
6 years agoDon't bump the receiver event counter if the poller is going to retry
Paul Durrant [Mon, 6 Aug 2018 12:11:39 +0000 (13:11 +0100)]
Don't bump the receiver event counter if the poller is going to retry

There is little point in bumping rsp_event (to trigger a new event from
the backend) if the poller is going to retry, so we can save modifying the
shared ring in this case.

This patch also adds extra debug code to the poller to make sure it never
exits from the main loop until either there are no retries pending or
the instance has been disabled.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoWork around bug in VS2017 SDV
Paul Durrant [Thu, 19 Jul 2018 09:25:03 +0000 (10:25 +0100)]
Work around bug in VS2017 SDV

XENBUS commit 868cd40f (of the same name) introduced a workaround for a
quoting bug in SDV. This commit applies a similar workaround for XENVIF.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoFix SDV release parameter
Paul Durrant [Wed, 18 Jul 2018 13:30:18 +0000 (14:30 +0100)]
Fix SDV release parameter

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoDisable warning about spectre mitigation
Marek Marczykowski-Górecki [Tue, 10 Jul 2018 13:23:20 +0000 (14:23 +0100)]
Disable warning about spectre mitigation

CL emits a warning about every place that will get spectre mitigation
when compiled with /Qspectre. Even if this option is already used. This
breaks the build if warnings are treated as errors.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Also disable the same warning in the co-installer build.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
6 years agoReplace uses of MmAllocatePagesForMdlEx in __AllocatePage
Ben Chalmers [Tue, 10 Jul 2018 10:09:59 +0000 (11:09 +0100)]
Replace uses of MmAllocatePagesForMdlEx in __AllocatePage

Windows appears to have an edge case bug in which zeroing
memory using MmAllocatePAgesForMdlEx (which in Win 10 1803
happens even if you specify MM_DONT_ZERO_ALLOCATION) can cause
a BSOD 139 1e.

This commit uses MmAllocateContinguousMemorySpecifyCache
to allocate memory instead, then builds and Mdl to wrap
it up.

__AllocatePages is left unchanged (as we don't want
to allocate multiple contiguous pages).  This issue
has not been seen outside of xenvif calls to
__AllocatePage and we expect a fix to the underlying
Windows problem in the near future

Signed-off-by: Ben.Chalmers <ben.chalmers@citrix.com>
6 years agoUpdate GNTTAB interface header
Owen Smith [Tue, 26 Jun 2018 11:56:26 +0000 (12:56 +0100)]
Update GNTTAB interface header

Also updates revision to 09000004

Signed-off-by: Owen Smith <owen.smith@citrix.com>
7 years agoCheck Mdl, not Controller->Mdl for allocation failure
Ben Chalmers [Mon, 23 Apr 2018 08:46:39 +0000 (09:46 +0100)]
Check Mdl, not Controller->Mdl for allocation failure

Ensure we fall through error handlers, rather than BSOD.

Signed-off-by: Ben Chalmers <ben.chalmers@citrix.com>
Also fix another instance of the incorrect check.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agopoller: fix event channels when backends do not support multi-queue
Chris Patterson [Wed, 21 Feb 2018 09:54:57 +0000 (09:54 +0000)]
poller: fix event channels when backends do not support multi-queue

The event channels for rx & tx are written to a multi-queue formatted
path even when multiple queues are not supported. This results in a hung
VM with the following logs:
XENBUS|EvtchnWait: TIMED OUT: Count = 00000001 Channel->Count = 00000000
...

This can be reproduced by having a Linux VM network backend with 1 vCPU.

If FrontendGetNumQueues() is 1 and multiple queues are not supported,
the following paths are used for the poller event channel:
    device/vif/1/queue-0/event-channel-[rx|tx]

However, the proper xenstore path in this case is:
    device/vif/1/event-channel-[rx|tx]

PollerInstanceInitialize() sets its path using FrontendFormatPath(),
which assumes a multi-queue path layout.  This is done in a fashion
similar to the transmitter and receiver rings.  However, the tx/rx rings
check for the mutually supported number of queues to determine the
actual path written to xenstore, using FrontendGetNumQueues(). See
__TransmitterRingStoreWrite() and __ReceiverRingStoreWrite().  This patch
adds a similar procedure for the poller to write to the appropriate path
in xenstore.

Signed-off-by: Chris Patterson <cjp256@gmail.com>
7 years agoDo more optimization in release builds and add /Qspectre flag
Paul Durrant [Mon, 29 Jan 2018 15:48:17 +0000 (15:48 +0000)]
Do more optimization in release builds and add /Qspectre flag

Spectre mitigations apparently only work on optimized code.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agoAdd support for building with Visual Studio 2017
Paul Durrant [Mon, 29 Jan 2018 15:37:56 +0000 (15:37 +0000)]
Add support for building with Visual Studio 2017

Also remove mappings for obsolete versions of VS in build.py.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agoDon't use KTIMERs in receive path
Paul Durrant [Fri, 26 Jan 2018 15:08:15 +0000 (15:08 +0000)]
Don't use KTIMERs in receive path

They appear to always defer by at least one timer tick, which is about 16ms
by default... just too long.

Instead, to avoid DPC watchdog issues in the MPE_Ethernet test, use a
threaded DPC. This is a real-time thread that can be pre-empted by normal
DPCs but not by standard system threads.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agoFix NDISTest6.5 CheckConnectivity and others
Paul Durrant [Wed, 24 Jan 2018 14:40:53 +0000 (14:40 +0000)]
Fix NDISTest6.5 CheckConnectivity and others

Commit e5e64b57 "Don't try to __FreePages() with local copy of MDL"
introduced subtle breakage into the receive path.

Prior to this patch, for the MDLs embedded in receiver packets, it was
always true that Mdl->MappedSystemVa == Mdl->StartVa + Mdl->ByteOffset.
However, after the commit, Mdl->StartVa was left as NULL in the assumption
that Mdl->MappedSystemVa would always be used to access the buffer. Sadly
this appears not to always be the case and in fact, wdm.h carries this
comment:

// Notice that while in the context of the subject thread, the base virtual
// address of a buffer mapped by an MDL may be referenced using the
// following:
//
//      Mdl->StartVa | Mdl->ByteOffset
//

So the the previous setting of StartVa is indeed documented, and clearly
something in the network tests is relying on it.

This patch modifies the MDL allocator code in util.h to alwasy set StartVa,
and modifies the receiver code to carry this field forward into the
embedded MDLs. It also removes some code duplication in ReceiverPacketCtor()
and __ReceiverRingPutPacket().

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agoDon't affinitize timer DPC
Paul Durrant [Fri, 3 Nov 2017 16:09:38 +0000 (16:09 +0000)]
Don't affinitize timer DPC

Instead just use it to schedule the normal DPC from wherever it is run.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agoRevert "Deferring transmit completion causes MPE_Ethernet test failures"
Paul Durrant [Thu, 2 Nov 2017 15:27:18 +0000 (15:27 +0000)]
Revert "Deferring transmit completion causes MPE_Ethernet test failures"

This reverts commit 761e6abe, but also adds a low frequency transmit
poller thread to try to avoid an unbounded transmit completion delay.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agoMake use of possible XENBUS_EVTCHN Unmask failure
Paul Durrant [Fri, 20 Oct 2017 17:14:28 +0000 (18:14 +0100)]
Make use of possible XENBUS_EVTCHN Unmask failure

Bump up to the latest version of XENBUS_EVTCHN and specify the option
to Unmask that allows it to fail. In this circumstance it is possible to
avoid dropping out of the DPC and thus avoid a context switch and re-queue.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agoMove the Receiver and Transmitter event and DPC processing...
Paul Durrant [Wed, 18 Oct 2017 14:02:52 +0000 (15:02 +0100)]
Move the Receiver and Transmitter event and DPC processing...

...into the new Poller sub-system.

For efficiency it is desirable to have a single DPC handle both Receiver
and Transmitter polling, even if there are separate event channels for
the shared rings.

This patch moves all the basic event and DPC code into the Poller
subsystem, which calls back into the Transmitter and Receiver sub-systems
(to poll the shared rings) from its single per-CPU DPC.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agoAdd the boilerplate for a new Poller sub-system
Paul Durrant [Mon, 16 Oct 2017 10:20:17 +0000 (11:20 +0100)]
Add the boilerplate for a new Poller sub-system

The intention is that the separate Receiver and Transmitter event and DPC
handling be combined into a single sub-system. This patch lays the ground-
work for that to be done.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agoRemove ZwFlushKey() from registry code
Paul Durrant [Thu, 3 Aug 2017 10:14:11 +0000 (11:14 +0100)]
Remove ZwFlushKey() from registry code

Attempting to flush registry keys early in boot causes an error to be
logged.

This patch therefore removes the explicit flushes from the registry code.
There is no option but to trust Windows lazy flush.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agoMake use of updated XENBUS_EVTCHN interface
Paul Durrant [Tue, 30 May 2017 15:24:45 +0000 (16:24 +0100)]
Make use of updated XENBUS_EVTCHN interface

The latest version of the interface has modified the semantics of the
Wait method to avoid races. The patch takes advantage of these new
semantics.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
7 years agoRemove VS2012 and VS2013 build scripts
Paul Durrant [Thu, 18 May 2017 13:52:47 +0000 (14:52 +0100)]
Remove VS2012 and VS2013 build scripts

This patch removes the scripts for building under VS2013 and VS2013 and
also fixes the package destination when building using VS2015.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoReboot request keys should be volatile
Paul Durrant [Mon, 8 May 2017 16:10:48 +0000 (17:10 +0100)]
Reboot request keys should be volatile

When a driver makes a reboot request it should use a volatile registry key.
The monitor service will explicitly remove the key prior to reboot but,
if the reboot is initiated in some other way and the key is non-volatile,
the monitor service will then needlessly prompt for a second reboot.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoFix SDV
Paul Durrant [Tue, 10 Jan 2017 17:15:58 +0000 (17:15 +0000)]
Fix SDV

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoDon't use Packet->Offset when stripping VLAN tags
Paul Durrant [Tue, 10 Jan 2017 16:53:08 +0000 (16:53 +0000)]
Don't use Packet->Offset when stripping VLAN tags

The WHQL tests have always been buggy when dealing with packet data that
is offset into the NET_BUFFER MDL chain. Instead, adjust the MappedSystemVa
of the initial MDL.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoDeferring transmit completion causes MPE_Ethernet test failures
Paul Durrant [Tue, 10 Jan 2017 16:51:12 +0000 (16:51 +0000)]
Deferring transmit completion causes MPE_Ethernet test failures

The test imposes a strict timeout on transmit completion so unfortunately
we have to complete eagerly.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoFix kdfiles.py
Paul Durrant [Tue, 10 Jan 2017 11:15:09 +0000 (11:15 +0000)]
Fix kdfiles.py

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoClean up use of MmGetSystemAddressForMdlSafe()
Paul Durrant [Tue, 10 Jan 2017 11:10:32 +0000 (11:10 +0000)]
Clean up use of MmGetSystemAddressForMdlSafe()

It is not necessary in most cases, because we can ASSERT that the MDL
is already mapped to a system address for anything locally allocated.

Also this patch substitutes use of local variable name StartVa with BaseVa,
to avoid confusion with Mdl->StartVa.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoDon't try to __FreePages() with local copy of MDL
Paul Durrant [Tue, 10 Jan 2017 10:29:53 +0000 (10:29 +0000)]
Don't try to __FreePages() with local copy of MDL

The XENVIF_RECEIVER_PACKET structure contains an embedded MDL so that
multi-fragment packets can be properly chained together. However this
structure should not be passed to __FreePages() as:

a) It appears to create problems with system PTE tracking
b) It results in memory corruption now that __FreePages() calls
   ExFreePool()

This patch therefore extends the packet structure with a pointer to the
original system MDL such that it can be passed to __FreePages() when the
packet destructor is called.

The patch also bypasses some calls to MmGetSystemAddressForMdlSafe() since
we can ASSERT that the MDL is already mapped to a system address.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoRemove the explicit ExFreePool(Mdl) in TransmitterBufferDtor()
Paul Durrant [Mon, 9 Jan 2017 11:45:21 +0000 (11:45 +0000)]
Remove the explicit ExFreePool(Mdl) in TransmitterBufferDtor()

It should no longer be there as of commit f529058c since it now
constitutes a double-free.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoFix memory leak in __FreePage()
Paul Durrant [Fri, 6 Jan 2017 15:44:00 +0000 (15:44 +0000)]
Fix memory leak in __FreePage()

The pool memory for the MDL also needs to be freed.

Also, generalise __AllocatePage() and __FreePage() to __AllocatePages()
and __FreePages() to allow for multi-page allocations in future.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoRemove unnecessary complexity from the controller frontend
Paul Durrant [Thu, 15 Dec 2016 14:00:42 +0000 (14:00 +0000)]
Remove unnecessary complexity from the controller frontend

The controller ring is driven much like the store ring in XENBUS for
request/response but, unlike the store ring, there are no asynchronous
events (like watches) so we really don't need a DPC and an asynchronous
poll, or a watchdog.

This patch removes that code, and also shortens the poll timeout when
waiting for a response since use of XENBUS_EVTCHN(...Wait...) is
inherently racey.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoDrop all pre-8.2 revisions
Paul Durrant [Thu, 15 Dec 2016 11:56:07 +0000 (11:56 +0000)]
Drop all pre-8.2 revisions

Version 9.0 drivers only need to be backwards compatible as far as 8.2, so
we can drop any interface versions that pre-date the 8.2 branch.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoUpdate BUILD.md with VS2015/WDK10 information
Paul Durrant [Wed, 14 Dec 2016 16:32:58 +0000 (16:32 +0000)]
Update BUILD.md with VS2015/WDK10 information

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoFixes for VS2015/WDK10 build
Paul Durrant [Wed, 14 Dec 2016 15:52:52 +0000 (15:52 +0000)]
Fixes for VS2015/WDK10 build

The package build was not working correctly and caused the overall build
to fail.
At least part of the reason for this is that Microsoft, in their infinite
wisdom, have removed the DIFx redist from WDK10. This patch makes use of
a new environment variable 'DPINST_REDIST' to find the copy of dpinst.exe
to package such that this can be pointed at an older WDK or alternative
location where dpinst.exe can be found.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoAdd support for building under VS2015/WDK10
Paul Durrant [Mon, 12 Dec 2016 16:31:50 +0000 (16:31 +0000)]
Add support for building under VS2015/WDK10

Moving to the new toolchain also threw up a few new warnings, which this
patch either fixes or squashes. Also, SDV appears to be fragile in new
ways (and whinge about some new things) so there are fixes for that too.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoUpdate driver version from 8.2.0 to 9.0.0
Paul Durrant [Mon, 12 Dec 2016 15:51:13 +0000 (15:51 +0000)]
Update driver version from 8.2.0 to 9.0.0

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoFix last commit... 8.2.0-rc1
Paul Durrant [Mon, 7 Nov 2016 16:46:59 +0000 (16:46 +0000)]
Fix last commit...

Missing 'git add'.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoAvoid calculating a hash if it is not necessary
Paul Durrant [Mon, 7 Nov 2016 16:20:16 +0000 (16:20 +0000)]
Avoid calculating a hash if it is not necessary

If there is only a single queue then we do not need to calculate a hash.
There is only one choice in how to steer the packet!

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoDisallow RSS configuration if there is only a single vCPU
Paul Durrant [Mon, 7 Nov 2016 16:15:24 +0000 (16:15 +0000)]
Disallow RSS configuration if there is only a single vCPU

It's not going to be useful and having RSS enabled in a single vCPU
VM seems to confuse a Windows Domain Controller installed in that VM.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoProvide registry override for disabling RSS
Paul Durrant [Mon, 7 Nov 2016 11:35:09 +0000 (11:35 +0000)]
Provide registry override for disabling RSS

For diagnostic purposes it is useful to be able to simulate the situation
where XENVIF does not support RSS (because the backend does not support
it).

This patch adds code to check a REG_DWORD value called
'FrontendDisableToeplitz' and will not allow the Toeplitz hash algorithm
to be configured if the value is non-zero. This prevents XENNET from
advertizing the RSS capability to the network stack.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoAdd more diagnostic messages
Paul Durrant [Fri, 4 Nov 2016 16:15:09 +0000 (16:15 +0000)]
Add more diagnostic messages

Log a message at start and end of both transmitter and receiver
ring enable and disable functions. Testing has thrown up some hangs
that appear to be in one of these functions.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoAdd registry override to disable multicast control
Paul Durrant [Fri, 4 Nov 2016 15:13:25 +0000 (15:13 +0000)]
Add registry override to disable multicast control

The patch adds a registry parameter 'TransmitterDisableMulticastControl'.
This is a REG_DWORD which, if set to a non-zero value, will prevent
XENVIF from using the dynamic-multicast-control feature of the backend.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoAlways select queue using the packet hash algorithm
Paul Durrant [Fri, 4 Nov 2016 10:51:54 +0000 (10:51 +0000)]
Always select queue using the packet hash algorithm

There may be a mismatch between the configured receive hash algorithm
and the actual algorithm present in a transmit-side packet.
E.g. Toeplitz may be configured but a transmitted packet may have no
hash information.

It makes no sense to use a hash mapping table configured for a Toeplitz
hash if the packet hash is not Toeplitz, therefore the code should pass
the actual packet hash algorithm into the FrontendGetQueue(). This patch
makes the that change.

Suggested-by: Owen Smith <owen.smith@citrite.net>
This patch also makes sure we cannot attempt to indirect through a zero-
sized mapping table (thereby incurring a divide-by-zero exception).

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoFix build warnings
Paul Durrant [Thu, 3 Nov 2016 10:11:05 +0000 (10:11 +0000)]
Fix build warnings

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoChange where transmitter packet cache is created
Paul Durrant [Thu, 3 Nov 2016 09:48:23 +0000 (09:48 +0000)]
Change where transmitter packet cache is created

The transmitter packet cache suffers from a similar issue as the
receiver packet cache did before commit 0dda5aa8 "Partially revert
commit ab655bb1...". If the VM goes through a suspend/resume cycle
with queued transmits then the cache would be torn down before all
objects had been freed, leading to ASSERTion failures in checked
builds and memory corruption in free builds.

This patch moves creation of the packet cache from the 'connect'
phase to the 'initialization' phase to avoid this problem.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoAdd Ring Index to ring-related Info messages
Paul Durrant [Wed, 2 Nov 2016 12:58:18 +0000 (12:58 +0000)]
Add Ring Index to ring-related Info messages

This patch adds the Ring Index to all calls to Info() issued in ring-
related functions so that the specific Ring is identified in the log.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoLog how many packets are aborted
Paul Durrant [Wed, 2 Nov 2016 12:02:37 +0000 (12:02 +0000)]
Log how many packets are aborted

When the transmitter rings are shut down, packets in the queues are
aborted. This patch adds a log line saying how many packets were
aborted in each queue.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
8 years agoAdd an extra transmitter ring poll
Paul Durrant [Wed, 2 Nov 2016 11:51:54 +0000 (11:51 +0000)]
Add an extra transmitter ring poll

If we are trying to post fragments into the ring and find it full then
do a single poll to try to free up the space, rather than bailing and
waiting for the DPC to run.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>