Paul Durrant [Thu, 25 Jan 2018 13:37:22 +0000 (13:37 +0000)]
Make sure Mdl->StartVa is set by __AllocatePages()
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
//
Hence it is important that a mapped MDL has a valid StartVa field as well
as a valid MappedSystemVa field. Unfortunately, for reasons best known to
Microsoft, MmMapLockedPagesSpecifyCache() does not ensure this is the
case, so it needs to be fixed up by __AllocatePages() itself.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Owen Smith [Mon, 23 Oct 2017 16:22:38 +0000 (17:22 +0100)]
Remove RingReQueueRequests
Since the Frontend transitions the ring through Closed and back to
Enabled on a resume from suspend, which aborts all queued and
prepared requests, de-queueing and re-queuing all requests from the
Target suspend callback is unneccessary. Also removes the Target
suspend callback and interface which is now unused.
Paul Durrant [Mon, 2 Oct 2017 10:07:45 +0000 (11:07 +0100)]
Fix BSOD on shutdown due to freeing NULL pointer
Unfortunately ExFreePoolWithTag() is not NULL-safe, and Base64Free() is
coded assuming it is. This patch adds an extra check for NULL to fix the
problem.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Owen Smith [Fri, 29 Sep 2017 16:23:31 +0000 (17:23 +0100)]
Use CACHE interface for REQUESTs, SEGMENTs and INDIRECTs
Replaces the ExLookasideList* calls with CACHE interface.
This allows the INDIRECT object constructor to allocate the indirect
page, and not allocate the indirect page on every object retrieval.
Also defers the indirect page free until the object is destroyed, and
not on every object put.
Owen Smith [Fri, 29 Sep 2017 15:58:13 +0000 (16:58 +0100)]
Add override for MaxRingPageOrder
Allow a Windows admin to limit the ring page count without
affecting any other VMs on the host. The override can be used
to restrict blkback's generous 16 page ring to a smaller value.
Owen Smith [Fri, 29 Sep 2017 15:45:27 +0000 (16:45 +0100)]
Use CACHE interface for bounce buffers
When the incomming read/write buffer is not aligned, the data needs
bouncing via a page-aligned buffer. Use the CACHE interface to provide
this buffer instead of a custom lookaside collection. Also moves the
storage for mapping the source buffers to the bounce buffer structure,
which will reduce the allocation size of the segments used on the fast
path (aligned buffers). The Indirect pages are also pre-allocated in the
object construction, and not for every object retrieval, and likewise
indirect pages are only freed on object destruction, and not for every
object put.
This will remove the log spam caused by the custom implementation.
Owen Smith [Fri, 22 Sep 2017 12:18:34 +0000 (13:18 +0100)]
Move Inquiry handling into target.c
Removes pdoinquiry.[ch] and its opaque inquiry pointer in favour of
making target.c the location of all target SRB handlers.
Removes a XenServer specific extension to the page83 inquiry data.
Owen Smith [Mon, 21 Aug 2017 14:37:00 +0000 (15:37 +0100)]
Add DPC timeout check
Check the DPC has not exceeded 1/2 its alloted time every
* 1/4 ring of responses processed
* all outstanding prepared requests submitted
* 1 queued SRB prepared
Eric Mackay [Wed, 28 Jun 2017 13:54:56 +0000 (14:54 +0100)]
Added a function called from DriverEntry where...
we can safely read registry keys and convert strings at PASSIVE_LEVEL
The MSDN documentation for various registry key access and string
conversion functions requires the caller to be at PASSIVE_LEVEL.
One of the reasons for this is that the string conversion tables used by
functions such as RtlAnsiStringToUnicodeString are stored in paged pool
memory. Both the page fault handler and the process scheduler run at
DISPATCH_LEVEL, therefore you must not touch memory at DISPATCH_LEVEL that
could be paged out. A process running at DISPATCH_LEVEL cannot be
preempted for the page fault handler to run.
To aid Static Driver Verifier code analysis and inform developers, I have
added SAL annotations that indicate PASSIVE_LEVEL is required on certain
registry access functions.
Signed-off-by: Eric Mackay <mackayem@amazon.com>
Re-based onto master and adjusted for style. Note this involved fixing
whitespace issues in frontend.c.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
v2:
- Remove unused FeatureName array
- Make more use of const qualifier for override name
Paul Durrant [Wed, 28 Jun 2017 15:32:45 +0000 (16:32 +0100)]
Make sure AdapterInitialize is called at PASSIVE_LEVEL
AdapterHwFindAdapter() can be called at DISPATCH_LEVEL and therefore it
is not safe to call AdapterInitialize() directly from there.
This patch, instead, calls StorPortEnablePassiveInitialization() from
AdapterHwInitialize() to request a PASSIVE_LEVEL initialization routine
is called immediately after AdapterHwFindAdapter() so that
AdapterInitialize() can be called from there.
Suggested-by: Owen Smith <owen.smith@citrix.com> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Mon, 8 May 2017 16:09:02 +0000 (17:09 +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>
Paul Durrant [Mon, 20 Feb 2017 14:03:50 +0000 (14:03 +0000)]
Fix race setting SrbStatus
SrbStatus needs to be set to PENDING right at the beginning of the
prepare functions, before the SRB has been added to any queue, otherwise
we risk a race with the completion code.
Reported-by: Andreas Kinzler <ml-ak@posteo.de> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Owen Smith [Fri, 17 Feb 2017 15:29:35 +0000 (15:29 +0000)]
Fix race calculating SrbExt->Count
It is possible under heavy loads for the backend to
start completing sub-requests of a Srb before the
SrbExt Count is set. This would leave the count unable
to reach 0 (as 1 or more requests are skipped by count
being overridden)
Owen Smith [Fri, 17 Feb 2017 15:27:03 +0000 (15:27 +0000)]
Ensure event channel is unmasked
If the Pdo is paused, the notifier dpc will not
unmask the event channel. When the Pdo is unpaused, the
event channel remains masked, so no more interrupts
get delivered.
Owen Smith [Fri, 3 Feb 2017 14:03:52 +0000 (14:03 +0000)]
Add registry overrides for features
Add a REG_DWORD to XenVbd's Parameters key, with names based
on the features, as per xenstore value, and set to 0 to
override use of that feature.
For example,
"HKLM\System\CCS\Services\XenVbd\Parameters" "feature-barrier" 0
will prevent the frontend issuing BLKIF_OP_WRITE_BARRIERs
"HKLM\System\CCS\Services\XenVbd\Parameters" "feature-discard" 0
will prevent the frontend issuing BLKIF_OP_DISCARDs
Owen Smith [Fri, 6 Jan 2017 12:02:55 +0000 (12:02 +0000)]
Fix pool leaks exposed by DriverVerifier
* RegistryCloseKey was not called in DriverRequestReboot
* RegistryTeardown was not being called in DriverUnload
* __RegistryFree was not being called in RegistryCreateKey
* Reordered DriverEntry slightly for improved code consistancy
Paul Durrant [Wed, 14 Dec 2016 16:13:10 +0000 (16:13 +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>
Paul Durrant [Tue, 13 Dec 2016 14:55:25 +0000 (14:55 +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>
Owen Smith [Mon, 5 Dec 2016 15:58:46 +0000 (15:58 +0000)]
Succeed SRBs that the backend doesnt support
If the backend does not support BLKIF_OP_DISCARD, BLKIF_OP_WRITE_BARRIER,
or BLKIF_OP_FLUSH_DISKCACHE, the first request will be failed and
subsiquent requests will be succeeded instead of being passed to the
backend. Bring the SRB status into line in both of these situations.
Signed-off-by: Owen Smith <owen.smith@citrix.com>
Remove rather scary warning and instead log any features being disabled.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Owen Smith [Tue, 22 Nov 2016 15:09:55 +0000 (15:09 +0000)]
Send BLKIF_OP_FLUSH_DISKCACHE
If the backend supports flush, send BLKIF_OP_FLUSH_DISKCACHE in
response to SCSIOP_SYNCHRONIZE_CACHE, and advertise the cache
to Windows. Also tracks count of cache operations. If the backend
does not support flush, but supports barrier, send
BLKIF_OP_WRITE_BARRIER instead, but do not advertise the cache to
Windows, as its assumed the backend will guarantee writes are
flushed if the storage requires flushing.
Paul Durrant [Thu, 6 Oct 2016 13:48:09 +0000 (14:48 +0100)]
Make use of the 'physical-sector-size' reported by the backend
blkif backends can report a 'physical-sector-size' as will as
'sector-size'. XENVBD currently reads this xenstore key but does
nothing with the value. This patch makes use of the value to set the
LogicalPerPhysicalExponent field in the READ_CAPACITY16_DATA structure
which is populated in response to a SCSIOP_READ_CAPACITY16 CDB.
The patch also modifies XENDISK to discover the logical block size by
sending a SCSIOP_READ_CAPACITY16 rather than relying on an IOCTL to
query StorageAccessAlignmentProperty being sent down the stack and
succeeded by STORPORT (which does not seem to be the case for Windows 10).
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Owen Smith [Mon, 3 Oct 2016 07:31:44 +0000 (08:31 +0100)]
Don't include null terminator in synthesized VendorId identifier
The VendorId identifier on SCSI page 83 is 16 bytes long (+header).
When synthesizing the inquiry data, either by global flag, or missing
xenstore data (sm-data/scsi/0x12/0x83), the NULL terminator on the
VendorId should not be included in the field. When this happens, any
query will decode 3 identifiers (VendorId, EUI64, VendorSpecific) instead
of the intended 2 (VendorId, VendorSpecific). This breaks the XenServer
VSS provider that uses the VendorSpecific identifier to retrieve the
vdi-uuid. This is only an issue when the inquiry data is synthesized and
additional identifiers are required, which is not a common use case.
Paul Durrant [Tue, 9 Aug 2016 14:19:29 +0000 (15:19 +0100)]
Use new monitor request key
The monitor service now uses a request key in registry under HKLM/SOFTWARE.
This patch modifies __DriverRequestReboot() to use the new key, which is
now set as a parameter by the INF file.
This patch also takes this opportunity to update the registry source module
to being it up to date with the XENBUS source base, and make wider use of it.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Paul Durrant [Tue, 2 Aug 2016 09:56:23 +0000 (10:56 +0100)]
Don't send a NUL terminator to QEMU's debug port
Logging a NUL terminator via QEMU's debug port seems to upset upstream
QEMU. It's also unnecessary anyway as QEMU will break log lines at a
newline character.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
It appears that this code is called when determining if Windows
can remove a PV disk. Without it Windows will not know the
disk is removable and will refuse to allow it to be removed
Ben Chalmers [Mon, 11 Jul 2016 13:29:04 +0000 (14:29 +0100)]
Only break FrontendBackend thread loop when alerted
Returns a missed '!' character without which the FrontendBackend
thread exits immediately - rather than when we Alert the thread
that we wish it to exit
Owen Smith [Fri, 22 Apr 2016 10:29:31 +0000 (11:29 +0100)]
Remove logging from failed Pdo Pnp IRPs
Logging fail codes for unhandled Pnp IRPs is unneccesary and adds
significant noise with multiple VBDs. Its unlikely the return value
from storport is going to add significant insight to diagnose
problems.
Owen Smith [Tue, 19 Apr 2016 15:50:08 +0000 (16:50 +0100)]
Create a seperate thread per VBD to handle backend change watches
Using a single thread for all backend watch events pust unneccesary
strain on xenstore when multiple vbds are in use, as each change
causes XenVbd to check several values in all backend areas.
Paul Durrant [Mon, 29 Feb 2016 10:20:50 +0000 (10:20 +0000)]
Override PartMgr SanPolicy
This patch makes sure that PartMgr's SanPolicy is 1 after XENVBD
installation, which means that newly plugged VBDs will come online as most
admins will expect.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Dave Buches [Fri, 22 Jan 2016 04:30:03 +0000 (20:30 -0800)]
Fixed improper SCSI UNMAP request implementation
The XenDisk disk class filter driver was generating requests
that did not adhere to the t10 SBC-3 SCSI specification for UNMAP
operations. Specifically, the data length and block descriptor data
length fields were not populated properly per the spec.
Although the XenVBD miniport driver handled these malformed
requests correctly, it failed to handle *properly* formed requests,
which would result in unpredictable behavior.
Additionally, the XenDisk filter wasn't properly responding to
StorageDeviceTrimProperty queries per the MSDN spec. Specifically,
the DEVICE_TRIM_DESCRIPTOR::Version structure member needs to be set
to sizeof(DEVICE_TRIM_DESCRIPTOR).