Re: [PATCH 1/6] Clear unplug keys if Active device is not the Vendor device

[Paul Durrant <xadimgnik@xxxxxxxxx>]

On 17/06/2021 13:33, Owen Smith wrote:
> When a VM has both Vendor device and the standard device, upgrades can be made
> for XenBus on the inactive device. In this case, the driver binaries are
> replaced but the coinstaller is not executed for the Active device, leading to
> the unplug keys remaining. When the VM is rebooted to complete the driver
> installation, both the Active and Inactive devices will use the new driver
> binaries, but the Active device will require the child devices rebinding to the
> potentially new hardware IDs exposed by the newer binary. This is not possible
> during early boot, and the absence of an emulated disk and not being able to
> enumerate the PV disk will result in a 0x7B bugcheck.
> The Vendor device is designed to be the prefered device, but is not required
> to be the active device (this is the case if the VMs configuration is changed
> after the drivers have been installed).
> It is possible to detect if the Active device is not the Vendor device during
> the Active device coinstaller, and clear the Unplug keys to avoid the problem
> where the VM will attempt to boot with unplugged emulated disks and PV disks
> that require rebinding, which results in a 0x7B bugcheck.
>
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
Acked-by: Paul Durrant <paul@xxxxxxx>

> ---
>   src/coinst/coinst.c | 23 ++++++++++++++++++++---
>   1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/src/coinst/coinst.c b/src/coinst/coinst.c
> index b269df0..c29ce59 100644
> --- a/src/coinst/coinst.c
> +++ b/src/coinst/coinst.c
> @@ -1361,7 +1361,8 @@ static BOOLEAN
>   IsActiveDevice(
>       IN  HDEVINFO            DeviceInfoSet,
>       IN  PSP_DEVINFO_DATA    DeviceInfoData,
> -    OUT PBOOLEAN            ActiveDevice
> +    OUT PBOOLEAN            ActiveDevice,
> +    OUT PBOOLEAN            VendorIsActive
>       )
>   {
>       PTCHAR                  ActiveDeviceID;
> @@ -1392,6 +1393,20 @@ IsActiveDevice(
>           TRUE :
>           FALSE;
>   
> +#ifdef VENDOR_DEVICE_ID_STR
> +
> +#define DRIVER_VENDOR_DEVICE_ID "PCI\\VEN_5853&DEV_" ## VENDOR_DEVICE_ID_STR ## 
> "&SUBSYS_C0005853&REV_01"
> +
> +    *VendorIsActive = (_stricmp(ActiveDeviceID, DRIVER_VENDOR_DEVICE_ID) == 0) 
> ?
> +        TRUE :
> +        FALSE;
> +
> +#undef DRIVER_VENDOR_DEVICE_ID
> +
> +#else
> +    *VendorIsActive = FALSE;
> +#endif
> +
>       free(DeviceID);
>       free(InstanceID);
>   
> @@ -1778,6 +1793,7 @@ DifInstallPostProcess(
>   {
>       BOOLEAN                         NewBinding;
>       BOOLEAN                         Active;
> +    BOOLEAN                         VendorIsActive;
>   
>       Log("====>");
>   
> @@ -1787,12 +1803,13 @@ DifInstallPostProcess(
>   
>       (VOID) IsActiveDevice(DeviceInfoSet,
>                             DeviceInfoData,
> -                          &Active);
> +                          &Active,
> +                          &VendorIsActive);
>   
>       Log("Active = %s", Active ? "TRUE" : "FALSE");
>       Log("NewBinding = %s", NewBinding ? "TRUE" : "FALSE");
>   
> -    if (Active && NewBinding) {
> +    if ((Active && NewBinding) || !VendorIsActive) {
>           (VOID) ClearUnplugRequest("DISKS");
>           (VOID) ClearUnplugRequest("NICS");
>       }