[prev in list] [next in list] [prev in thread] [next in thread]
List: win-pv-devel
Subject: Re: [PATCH 0/6] Remove CoInstallers
From: Owen Smith <owen.smith () cloud ! com>
Date: 2023-08-31 7:45:10
Message-ID: CAC_UY88F7EA10v3Nriuf4Mt-+Z7giD1diN3gYTOwwo4neRNJhw () mail ! gmail ! com
[Download RAW message or body]
I should have tagged this patch series as RFC
On Thu, Aug 31, 2023 at 8:29 AM Owen Smith <owen.smith@cloud.com> wrote:
> Windows 11 22H2 WHQL testing has highlighted the need to remove
> CoInstallers.
> The current CoInstaller handle several edge cases that will need
> replacement
> implementations. This patch series for most drivers offers a potential
> solution
> to the removal of CoInstallers and the required changes to avoid upgrade
> issues.
>
> CoInstallers have several uses currently:
> - Adds an upgrade block function with the "AllowUpgrade" value blocking
> driver
> upgrades when set.
> - Adds a check to prevent XenBus upgrades from a different vendor / brand.
> This checks the current device name (if it exists) matches the new
> device name.
> - Checks all currently installed child devices have a supported set of
> interfaces
> common with the new parent device.
> - Clears the Unplug keys only when required.
> - Correctly cleans up user mode services on uninstall.
>
> The AllowUpgrade function has not been replaced - I have not seen this
> used in
> the current implementations of the drivers.
>
> The vendor naming check is specific to XenBus, and I dont think there is a
> reliable replacement, and was intended to prevent different vendor drivers
> from
> attempting to coexist, or upgrade cases from particularly old versions
> which had
> a different naming scheme.
>
> The interface compatability checks can be replaced with the matching if
> RevisionIDs
> in the INF files, so that child devices will only install on parent
> devices that
> expose a matching HardwareID or CompatibleID. This does leave the case
> where the
> new parent driver could drop support for an interface version that is
> currently in
> use. This will mandate a further driver upgrade, and should remove the
> Unplug keys
> and mandate a reboot where the emulated devices are present, to ensure
> continued
> operation. After this reboot using emulated devices, updates to the child
> devices
> should be detected and installed, requiring another reboot to revert to PV
> devices.
>
> Unplug keys getting cleared on all upgrades should allow the next boot to
> use emulated
> devices, unless a subsequent install updates dependent drivers to use a
> working set
> of new interfaces. The requirement here is that updating any driver should
> always
> allow a reboot with emulated devices, should any further issues be
> detected.
>
> I believe that, going forward, changing the DeviceID to remove the
> RevisionID will
> avoid the need to set network IDs and copy settings from the previous
> network
> connection to the new network connection. This change will mean the driver
> selection
> during installation/upgrade will use the HardwareIDs (with RevisionIDs) to
> match the
> ID in the DDInstall INF section (with RevisionID), and use the DeviceID to
> generate
> the Matching Device ID and Instance ID. This means that once drivers have
> been
> upgraded, the InstanceIDs should not change, and Windows should not
> generate a new
> network connection (with default settings) for the upgraded driver set -
> meaning the
> correct network settings are persisted.
>
> I dont have a good replacement for cleaning up user mode services
> (xenbus_monitor
> xenagent and xencons_monitor) on device uninstall. This *should* be
> handled correctly
> by the PnP manager, but failing to stop either service should not be
> detremental to
> the operation of the VM. Selecting the "Delete Drivers" option during
> uninstall will
> stop and disable these services, not selecting this option will leave
> these services
> running.
>
> Owen Smith (6):
> Remove CoInstaller from INF
> Reset StorNvme's StartOverride
> Delete CoInstaller code
> Fix Length calculation in PdoQueryId
> Remove REV from DeviceID
> Add Unplug v2 interface (REV_0900000A)
>
> include/revision.h | 3 +-
> include/unplug_interface.h | 30 +-
> include/xen.h | 7 +
> src/coinst/coinst.c | 2136 -----------------
> src/coinst/xenbus_coinst.def | 38 -
> src/coinst/xenbus_coinst.rc | 57 -
> src/monitor/monitor.c | 63 +
> src/xen/unplug.c | 15 +
> src/xenbus.inf | 20 +-
> src/xenbus/pdo.c | 14 +-
> src/xenbus/unplug.c | 61 +
> vs2015/package/package.vcxproj | 3 -
> vs2015/xenbus.sln | 22 -
> vs2015/xenbus_coinst/xenbus_coinst.vcxproj | 64 -
> .../xenbus_coinst/xenbus_coinst.vcxproj.user | 8 -
> vs2017/package/package.vcxproj | 3 -
> vs2017/xenbus.sln | 22 -
> vs2017/xenbus_coinst/xenbus_coinst.vcxproj | 64 -
> .../xenbus_coinst/xenbus_coinst.vcxproj.user | 8 -
> vs2019/package/package.vcxproj | 3 -
> vs2019/xenbus.sln | 22 -
> vs2019/xenbus_coinst/xenbus_coinst.vcxproj | 70 -
> .../xenbus_coinst/xenbus_coinst.vcxproj.user | 8 -
> vs2022/package/package.vcxproj | 3 -
> vs2022/xenbus.sln | 22 -
> vs2022/xenbus_coinst/xenbus_coinst.vcxproj | 70 -
> .../xenbus_coinst/xenbus_coinst.vcxproj.user | 8 -
> 27 files changed, 187 insertions(+), 2657 deletions(-)
> delete mode 100644 src/coinst/coinst.c
> delete mode 100644 src/coinst/xenbus_coinst.def
> delete mode 100644 src/coinst/xenbus_coinst.rc
> delete mode 100644 vs2015/xenbus_coinst/xenbus_coinst.vcxproj
> delete mode 100644 vs2015/xenbus_coinst/xenbus_coinst.vcxproj.user
> delete mode 100644 vs2017/xenbus_coinst/xenbus_coinst.vcxproj
> delete mode 100644 vs2017/xenbus_coinst/xenbus_coinst.vcxproj.user
> delete mode 100644 vs2019/xenbus_coinst/xenbus_coinst.vcxproj
> delete mode 100644 vs2019/xenbus_coinst/xenbus_coinst.vcxproj.user
> delete mode 100644 vs2022/xenbus_coinst/xenbus_coinst.vcxproj
> delete mode 100644 vs2022/xenbus_coinst/xenbus_coinst.vcxproj.user
>
> --
> 2.41.0.windows.3
>
>
[Attachment #3 (text/html)]
<div dir="ltr"><div class="gmail_default" style="font-family:monospace">I should have \
tagged this patch series as RFC</div></div><br><div class="gmail_quote"><div \
dir="ltr" class="gmail_attr">On Thu, Aug 31, 2023 at 8:29 AM Owen Smith <<a \
href="mailto:owen.smith@cloud.com">owen.smith@cloud.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Windows 11 22H2 WHQL \
testing has highlighted the need to remove CoInstallers.<br> The current CoInstaller \
handle several edge cases that will need replacement<br> implementations. This patch \
series for most drivers offers a potential solution<br> to the removal of \
CoInstallers and the required changes to avoid upgrade issues.<br> <br>
CoInstallers have several uses currently:<br>
- Adds an upgrade block function with the "AllowUpgrade" value blocking \
driver<br> upgrades when set.<br>
- Adds a check to prevent XenBus upgrades from a different vendor / brand.<br>
This checks the current device name (if it exists) matches the new device \
name.<br>
- Checks all currently installed child devices have a supported set of interfaces<br>
common with the new parent device.<br>
- Clears the Unplug keys only when required.<br>
- Correctly cleans up user mode services on uninstall.<br>
<br>
The AllowUpgrade function has not been replaced - I have not seen this used in<br>
the current implementations of the drivers.<br>
<br>
The vendor naming check is specific to XenBus, and I dont think there is a<br>
reliable replacement, and was intended to prevent different vendor drivers from<br>
attempting to coexist, or upgrade cases from particularly old versions which had<br>
a different naming scheme.<br>
<br>
The interface compatability checks can be replaced with the matching if \
RevisionIDs<br> in the INF files, so that child devices will only install on parent \
devices that<br> expose a matching HardwareID or CompatibleID. This does leave the \
case where the<br> new parent driver could drop support for an interface version that \
is currently in<br> use. This will mandate a further driver upgrade, and should \
remove the Unplug keys<br> and mandate a reboot where the emulated devices are \
present, to ensure continued<br> operation. After this reboot using emulated devices, \
updates to the child devices<br> should be detected and installed, requiring another \
reboot to revert to PV devices.<br> <br>
Unplug keys getting cleared on all upgrades should allow the next boot to use \
emulated<br> devices, unless a subsequent install updates dependent drivers to use a \
working set<br> of new interfaces. The requirement here is that updating any driver \
should always<br> allow a reboot with emulated devices, should any further issues be \
detected.<br> <br>
I believe that, going forward, changing the DeviceID to remove the RevisionID \
will<br> avoid the need to set network IDs and copy settings from the previous \
network<br> connection to the new network connection. This change will mean the \
driver selection<br> during installation/upgrade will use the HardwareIDs (with \
RevisionIDs) to match the<br> ID in the DDInstall INF section (with RevisionID), and \
use the DeviceID to generate<br> the Matching Device ID and Instance ID. This means \
that once drivers have been<br> upgraded, the InstanceIDs should not change, and \
Windows should not generate a new<br> network connection (with default settings) for \
the upgraded driver set - meaning the<br> correct network settings are persisted.<br>
<br>
I dont have a good replacement for cleaning up user mode services (xenbus_monitor<br>
xenagent and xencons_monitor) on device uninstall. This *should* be handled \
correctly<br> by the PnP manager, but failing to stop either service should not be \
detremental to<br> the operation of the VM. Selecting the "Delete Drivers" \
option during uninstall will<br> stop and disable these services, not selecting this \
option will leave these services<br> running.<br>
<br>
Owen Smith (6):<br>
Remove CoInstaller from INF<br>
Reset StorNvme's StartOverride<br>
Delete CoInstaller code<br>
Fix Length calculation in PdoQueryId<br>
Remove REV from DeviceID<br>
Add Unplug v2 interface (REV_0900000A)<br>
<br>
include/revision.h | 3 +-<br>
include/unplug_interface.h | 30 +-<br>
include/xen.h | 7 +<br>
src/coinst/coinst.c | 2136 \
-----------------<br> src/coinst/xenbus_coinst.def | \
38 -<br> src/coinst/xenbus_coinst.rc | 57 -<br>
src/monitor/monitor.c | 63 +<br>
src/xen/unplug.c | 15 +<br>
src/xenbus.inf | 20 +-<br>
src/xenbus/pdo.c | 14 +-<br>
src/xenbus/unplug.c | 61 +<br>
vs2015/package/package.vcxproj | 3 -<br>
vs2015/xenbus.sln | 22 -<br>
vs2015/xenbus_coinst/xenbus_coinst.vcxproj | 64 -<br>
.../xenbus_coinst/xenbus_coinst.vcxproj.user | 8 -<br>
vs2017/package/package.vcxproj | 3 -<br>
vs2017/xenbus.sln | 22 -<br>
vs2017/xenbus_coinst/xenbus_coinst.vcxproj | 64 -<br>
.../xenbus_coinst/xenbus_coinst.vcxproj.user | 8 -<br>
vs2019/package/package.vcxproj | 3 -<br>
vs2019/xenbus.sln | 22 -<br>
vs2019/xenbus_coinst/xenbus_coinst.vcxproj | 70 -<br>
.../xenbus_coinst/xenbus_coinst.vcxproj.user | 8 -<br>
vs2022/package/package.vcxproj | 3 -<br>
vs2022/xenbus.sln | 22 -<br>
vs2022/xenbus_coinst/xenbus_coinst.vcxproj | 70 -<br>
.../xenbus_coinst/xenbus_coinst.vcxproj.user | 8 -<br>
27 files changed, 187 insertions(+), 2657 deletions(-)<br>
delete mode 100644 src/coinst/coinst.c<br>
delete mode 100644 src/coinst/xenbus_coinst.def<br>
delete mode 100644 src/coinst/xenbus_coinst.rc<br>
delete mode 100644 vs2015/xenbus_coinst/xenbus_coinst.vcxproj<br>
delete mode 100644 vs2015/xenbus_coinst/xenbus_coinst.vcxproj.user<br>
delete mode 100644 vs2017/xenbus_coinst/xenbus_coinst.vcxproj<br>
delete mode 100644 vs2017/xenbus_coinst/xenbus_coinst.vcxproj.user<br>
delete mode 100644 vs2019/xenbus_coinst/xenbus_coinst.vcxproj<br>
delete mode 100644 vs2019/xenbus_coinst/xenbus_coinst.vcxproj.user<br>
delete mode 100644 vs2022/xenbus_coinst/xenbus_coinst.vcxproj<br>
delete mode 100644 vs2022/xenbus_coinst/xenbus_coinst.vcxproj.user<br>
<br>
-- <br>
2.41.0.windows.3<br>
<br>
</blockquote></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic