From win-pv-devel-bounces@lists.xenproject.org Wed Jul 01 17:28:58 2020 Return-path: Envelope-to: archives@lists.xenproject.org Delivery-date: Wed, 01 Jul 2020 17:28:58 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jqgXY-0007mA-WB; Wed, 01 Jul 2020 17:28:56 +0000 Received: from winpvdrvbuild.xenproject.org ([162.242.160.44]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jqgXX-0007m4-GL for win-pv-devel@lists.xenproject.org; Wed, 01 Jul 2020 17:28:55 +0000 Date: Wed, 1 Jul 2020 17:28:56 +0000 (UTC) From: paul.durrant@xenproject.org To: win-pv-devel@lists.xenproject.org Message-ID: <29926528.1.1593624538979.JavaMail.WINPVDRVBUILD$@winpvdrvbuild> Subject: XENIFACE-master - Build #98 - Successful MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_0_21433952.1593624537682" X-Jenkins-Job: XENIFACE-master X-Jenkins-Result: SUCCESS X-BeenThere: win-pv-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Developer list for the Windows PV Drivers subproject List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: win-pv-devel-bounces@lists.xenproject.org Sender: "win-pv-devel" ------=_Part_0_21433952.1593624537682 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit The build system has completed build #98. Check console output at https://winpvdrvbuild.xenproject.org:8080/job/XENIFACE-master/98/console to view the results. ------=_Part_0_21433952.1593624537682-- From win-pv-devel-bounces@lists.xenproject.org Thu Jul 16 08:37:39 2020 Return-path: Envelope-to: archives@lists.xenproject.org Delivery-date: Thu, 16 Jul 2020 08:37:39 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jvzOb-0004q5-Ct; Thu, 16 Jul 2020 08:37:37 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jvzOZ-0004pz-RV for win-pv-devel@lists.xenproject.org; Thu, 16 Jul 2020 08:37:35 +0000 X-Inumbo-ID: 988fc862-c73f-11ea-948b-12813bfff9fa Received: from eddie3.eng.citrite.net (unknown [185.25.67.249]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTP id 988fc862-c73f-11ea-948b-12813bfff9fa; Thu, 16 Jul 2020 08:37:34 +0000 (UTC) Received: by eddie3.eng.citrite.net (Postfix, from userid 2105) id 8FD2E3F29A09; Thu, 16 Jul 2020 09:37:33 +0100 (BST) From: Owen Smith To: win-pv-devel@lists.xenproject.org Subject: [PATCH] Clear unplug keys if Active device is not the Vendor device Date: Thu, 16 Jul 2020 09:37:32 +0100 Message-Id: <1594888652-49120-1-git-send-email-owen.smith@citrix.com> X-Mailer: git-send-email 1.8.3.1 X-BeenThere: win-pv-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Developer list for the Windows PV Drivers subproject List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Owen Smith Errors-To: win-pv-devel-bounces@lists.xenproject.org Sender: "win-pv-devel" If the Vendor device is distributed by Windows Update, the update process will queue a reboot. If the vendor device is not the active device, then the unplug keys will not be removed before the next reboot. The reboot will also replace the driver on the active device which requires rebinding child devices, which cannot happen during the early boot phase. This will lead to a 7B bugcheck because XenVbd has not been rebound to the new version of XenBus Signed-off-by: Owen Smith --- src/coinst/coinst.c | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/coinst/coinst.c b/src/coinst/coinst.c index 7b96f59..8e57a10 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,21 +1793,25 @@ DifInstallPostProcess( { BOOLEAN NewBinding; BOOLEAN Active; + BOOLEAN VendorIsActive; Log("====>"); NewBinding = (BOOLEAN)(ULONG_PTR)Context->PrivateData; Active = TRUE; + VendorIsActive = FALSE; (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"); } -- 2.16.2.windows.1 From win-pv-devel-bounces@lists.xenproject.org Mon Jul 20 09:18:10 2020 Return-path: Envelope-to: archives@lists.xenproject.org Delivery-date: Mon, 20 Jul 2020 09:18:10 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jxRw1-0003Ms-3o; Mon, 20 Jul 2020 09:18:09 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jxRw0-0003Mj-Ak for win-pv-devel@lists.xenproject.org; Mon, 20 Jul 2020 09:18:08 +0000 X-Inumbo-ID: ebcea96c-ca69-11ea-847f-bc764e2007e4 Received: from mail-ej1-x644.google.com (unknown [2a00:1450:4864:20::644]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id ebcea96c-ca69-11ea-847f-bc764e2007e4; Mon, 20 Jul 2020 09:18:07 +0000 (UTC) Received: by mail-ej1-x644.google.com with SMTP id n22so14357557ejy.3 for ; Mon, 20 Jul 2020 02:18:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:reply-to:to:references:in-reply-to:subject:date:message-id :mime-version:content-transfer-encoding:thread-index :content-language; bh=r2WHakiczGeNy2YPTyPwJc2mld6wDK+YL0TKfMkiDak=; b=KVxExOat+4Wcub/j0Qp/CYyfxF0gUBrRNL6NukEsi5vgj4c+sIaxAe1NmZnOSy1ZCM N0MYDVRckYj2gTmYUWGCOTJIM3pdJrNchL9lGOpYhOLa5/nSRVU1DCM/OlzJxV54DR/x iYyXZhyh7j8poQ06BlRNJbCfkae2yBHEBLcUZi9cxv3ag3jQRhYPhv8DBTvbKZ4yjrqV HrxXcwpiNeap/bJQ4/S8nJFryQsAoVqZRfToSs52tHqdlacOc3ZiipmiDQpX1ANITwH0 ntawKuVL+sD1h0k7AntNOwsGK7bR0tU1uvxkX7RRcIoKbR2pJN1AIHjSo1Fh/xNKBp4C Ai+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:reply-to:to:references:in-reply-to:subject :date:message-id:mime-version:content-transfer-encoding:thread-index :content-language; bh=r2WHakiczGeNy2YPTyPwJc2mld6wDK+YL0TKfMkiDak=; b=Xlr/G5QfdY3LWo3Ox7+EEqgBizDObIK4Q7JM/IfCYQvv6UZfdPunPH/Zcld4YHD7vd IT9uYhztxv6nujzJYTxlfmm4A0iPziU/5FjFtrCW4F8EIMLSg2AO9zPRg3gCPrLGyphh +x1pQJg3MtuxRoUYPbjjGl3kWchbCv5gE+uPc+87wxLvdX1TBJ3misTAPIB/vZ9J0Wju ewKRbeYj000imy0VqTUAQRhVwVFiX3oBIzsaDjqPdF+GA2lx3JQRFTd2CNvO8LkPu34U Uu7Usj/VMG+sOOqgecp5fpRmEraQgiFFuQ/fWppY9/Pbz+DYqVChrTF88gg7IYlSgtnj iWaA== X-Gm-Message-State: AOAM533LS6t+2KnhkMMxaQYd5Pecr9bKxp+dWXfvH/1+wKaldU0bA4tx d3eC5PK7tMpRPEmLPhWIl4c= X-Google-Smtp-Source: ABdhPJxMPd0QcLGEDTe9HAK1nkDMAPbJwCxh6aRHVhqQupShkl2b8DK9kneKiNmJXvC0hhdQzITh+A== X-Received: by 2002:a17:906:4685:: with SMTP id a5mr19666980ejr.46.1595236686163; Mon, 20 Jul 2020 02:18:06 -0700 (PDT) Received: from CBGR90WXYV0 (54-240-197-226.amazon.com. [54.240.197.226]) by smtp.gmail.com with ESMTPSA id i10sm14769386edx.42.2020.07.20.02.18.05 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 20 Jul 2020 02:18:05 -0700 (PDT) From: Paul Durrant X-Google-Original-From: "Paul Durrant" To: "'Owen Smith'" , References: <1594888652-49120-1-git-send-email-owen.smith@citrix.com> In-Reply-To: <1594888652-49120-1-git-send-email-owen.smith@citrix.com> Subject: RE: [PATCH] Clear unplug keys if Active device is not the Vendor device Date: Mon, 20 Jul 2020 10:18:04 +0100 Message-ID: <001001d65e76$ad007ad0$07017070$@xen.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQHF93dT2nTZiWuMcifdvJkXYXXBYakw+3Yg Content-Language: en-gb X-BeenThere: win-pv-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Developer list for the Windows PV Drivers subproject List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Reply-To: paul@xen.org Errors-To: win-pv-devel-bounces@lists.xenproject.org Sender: "win-pv-devel" > -----Original Message----- > From: win-pv-devel On Behalf Of Owen Smith > Sent: 16 July 2020 09:38 > To: win-pv-devel@lists.xenproject.org > Cc: Owen Smith > Subject: [PATCH] Clear unplug keys if Active device is not the Vendor device > > If the Vendor device is distributed by Windows Update, the update > process will queue a reboot. Well, the device is not distributed by Windows Update. Do you mean "If a version of XENBUS supporting a vendor device"? (Bear in mind there could theoretically be vendor devices other than the Citrix one). > If the vendor device is not the active > device, then the unplug keys will not be removed before the next reboot. > The reboot will also replace the driver on the active device which > requires rebinding child devices, which cannot happen during the early > boot phase. This will lead to a 7B bugcheck because XenVbd has not been > rebound to the new version of XenBus What is *supposed* to happen is that the, when a vendor device appears in the system necessitating a new XENBUS binding, the co-installer clears the unplug keys because the PDOs beneath the non-vendor device will disappear at next boot because XENBUS's active device will change. This could equally happen if a vendor device subsequently disappears, which I don't think this patch covers. Paul > > Signed-off-by: Owen Smith > --- > src/coinst/coinst.c | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/src/coinst/coinst.c b/src/coinst/coinst.c > index 7b96f59..8e57a10 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,21 +1793,25 @@ DifInstallPostProcess( > { > BOOLEAN NewBinding; > BOOLEAN Active; > + BOOLEAN VendorIsActive; > > Log("====>"); > > NewBinding = (BOOLEAN)(ULONG_PTR)Context->PrivateData; > > Active = TRUE; > + VendorIsActive = FALSE; > > (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"); > } > -- > 2.16.2.windows.1 > From win-pv-devel-bounces@lists.xenproject.org Mon Jul 20 10:18:11 2020 Return-path: Envelope-to: archives@lists.xenproject.org Delivery-date: Mon, 20 Jul 2020 10:18:11 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jxSs5-0000Tr-4F; Mon, 20 Jul 2020 10:18:09 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jxSs3-0000Tm-7Y for win-pv-devel@lists.xenproject.org; Mon, 20 Jul 2020 10:18:07 +0000 X-Inumbo-ID: 4cf5ab84-ca72-11ea-9f77-12813bfff9fa Received: from esa4.hc3370-68.iphmx.com (unknown [216.71.155.144]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id 4cf5ab84-ca72-11ea-9f77-12813bfff9fa; Mon, 20 Jul 2020 10:18:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1595240286; h=from:to:subject:date:message-id:references:in-reply-to: content-transfer-encoding:mime-version; bh=URETgLmfakNzAaxcSRLr6YhhuN5fAm+F1BUjrYe13YY=; b=RLWBnHZ/pIs4bSyOP6i/7J02MRrVqMYMdF/z3oJApppzlpdBmc+mD+k2 JZhHBkkN2DEV3U0OGq6ce2HAzVv+lWUDijnZqfhylOSHk6nJdngnWZT2C V1gCHGRvkKj5YOrS4S1szYljPCr4U1kjxJ+bGdHTUEKz6xyxfvy+zcBGz o=; Authentication-Results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none IronPort-SDR: szQsvC0lSHIYAu/4KTqxA9A8+VsohWyyMh3sA5vU+jhQFkPbF2KF5FEmjthjtoTEQHjA3RVMac FAnhcpFi1W0PUKRTwy1bea2E3C6zy8XQsdBgjY1ZUFsdw82uWh985CkNZ9J3cXPJuX6qe35J9U bvGbKr3U1vZJp9ejoyvpE+yk2OdN/0i1C3lfrxiUZ4NM0And2kSivLjMUhcWL5jW8MldOrj753 CL0jd8rzrd0M+4ONuijhd5h/pNEy4nwvajVTpdxjPhzjkHMTx8BPvqAH3/dIwx96oqBJiCGPVm RxE= X-SBRS: 2.7 X-MesageID: 23589519 X-Ironport-Server: esa4.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.75,374,1589256000"; d="scan'208";a="23589519" From: Owen Smith To: "paul@xen.org" , "win-pv-devel@lists.xenproject.org" Subject: RE: [PATCH] Clear unplug keys if Active device is not the Vendor device Thread-Topic: [PATCH] Clear unplug keys if Active device is not the Vendor device Thread-Index: AQHWW0xb2bSZteM2kkW6+qiqDL6MI6kQFTgAgAApybA= Date: Mon, 20 Jul 2020 10:18:02 +0000 Message-ID: <0ad7a14cc1864c4a97a4665df24f5bde@AMSPEX02CL02.citrite.net> References: <1594888652-49120-1-git-send-email-owen.smith@citrix.com> <001001d65e76$ad007ad0$07017070$@xen.org> In-Reply-To: <001001d65e76$ad007ad0$07017070$@xen.org> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-BeenThere: win-pv-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Developer list for the Windows PV Drivers subproject List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: win-pv-devel-bounces@lists.xenproject.org Sender: "win-pv-devel" > -----Original Message----- > From: Paul Durrant [mailto:xadimgnik@gmail.com] > Sent: 20 July 2020 10:18 > To: Owen Smith ; win-pv- > devel@lists.xenproject.org > Subject: RE: [PATCH] Clear unplug keys if Active device is not the Vendor > device >=20 > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments > unless you have verified the sender and know the content is safe. >=20 > > -----Original Message----- > > From: win-pv-devel On > > Behalf Of Owen Smith > > Sent: 16 July 2020 09:38 > > To: win-pv-devel@lists.xenproject.org > > Cc: Owen Smith > > Subject: [PATCH] Clear unplug keys if Active device is not the Vendor > > device > > > > If the Vendor device is distributed by Windows Update, the update > > process will queue a reboot. >=20 > Well, the device is not distributed by Windows Update. Do you mean "If a > version of XENBUS supporting a vendor device"? (Bear in mind there could > theoretically be vendor devices other than the Citrix one). The message is a little unclear here. If a version of XenBus is distributed= by Windows Update for a given vendor device ID >=20 > > If the vendor device is not the active device, then the unplug keys > > will not be removed before the next reboot. > > The reboot will also replace the driver on the active device which > > requires rebinding child devices, which cannot happen during the early > > boot phase. This will lead to a 7B bugcheck because XenVbd has not > > been rebound to the new version of XenBus >=20 > What is *supposed* to happen is that the, when a vendor device appears in > the system necessitating a new XENBUS binding, the co-installer clears th= e > unplug keys because the PDOs beneath the non-vendor device will > disappear at next boot because XENBUS's active device will change. This > could equally happen if a vendor device subsequently disappears, which I > don't think this patch covers. >=20 If the XenBus bound to the vendor device is updated, this will replace the = driver on any other device, which will necessitate clearing of the unplug k= eys. This doesn't happen at the moment, so on the reboot to complete instal= lation of XenBus on the non-active vendor device ID, the unplug keys are no= t removed and the bindings of the child devices on the XenBus on the active= non-vendor device ID are not regenerated. This hits DriverSetActive's fail= 2 case, where __DriverIsDeviceLegacy(non-vendor-device) and __DriverIsVendo= rDevicePresent(). This patch doesn't cover the case where the vendor device disappears, prima= rily because I didn't hit this edge case trying to resolve the original iss= ue > Paul >=20 > > > > Signed-off-by: Owen Smith > > --- > > src/coinst/coinst.c | 25 ++++++++++++++++++++++--- > > 1 file changed, 22 insertions(+), 3 deletions(-) > > > > diff --git a/src/coinst/coinst.c b/src/coinst/coinst.c index > > 7b96f59..8e57a10 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 =3D (_stricmp(ActiveDeviceID, > DRIVER_VENDOR_DEVICE_ID) =3D=3D 0) ? > > + TRUE : > > + FALSE; > > + > > +#undef DRIVER_VENDOR_DEVICE_ID > > + > > +#else > > + *VendorIsActive =3D FALSE; > > +#endif > > + > > free(DeviceID); > > free(InstanceID); > > > > @@ -1778,21 +1793,25 @@ DifInstallPostProcess( { > > BOOLEAN NewBinding; > > BOOLEAN Active; > > + BOOLEAN VendorIsActive; > > > > Log("=3D=3D=3D=3D>"); > > > > NewBinding =3D (BOOLEAN)(ULONG_PTR)Context->PrivateData; > > > > Active =3D TRUE; > > + VendorIsActive =3D FALSE; > > > > (VOID) IsActiveDevice(DeviceInfoSet, > > DeviceInfoData, > > - &Active); > > + &Active, > > + &VendorIsActive); > > > > Log("Active =3D %s", Active ? "TRUE" : "FALSE"); > > Log("NewBinding =3D %s", NewBinding ? "TRUE" : "FALSE"); > > > > - if (Active && NewBinding) { > > + if ((Active && NewBinding) || > > + !VendorIsActive) { > > (VOID) ClearUnplugRequest("DISKS"); > > (VOID) ClearUnplugRequest("NICS"); > > } > > -- > > 2.16.2.windows.1 > > >=20 >=20 From win-pv-devel-bounces@lists.xenproject.org Mon Jul 20 13:18:40 2020 Return-path: Envelope-to: archives@lists.xenproject.org Delivery-date: Mon, 20 Jul 2020 13:18:40 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jxVgk-0000Jy-Hc; Mon, 20 Jul 2020 13:18:38 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1jxVgj-0000Jt-Er for win-pv-devel@lists.xenproject.org; Mon, 20 Jul 2020 13:18:37 +0000 X-Inumbo-ID: 84162652-ca8b-11ea-848e-bc764e2007e4 Received: from mail-ej1-x631.google.com (unknown [2a00:1450:4864:20::631]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 84162652-ca8b-11ea-848e-bc764e2007e4; Mon, 20 Jul 2020 13:18:35 +0000 (UTC) Received: by mail-ej1-x631.google.com with SMTP id a21so17964697ejj.10 for ; Mon, 20 Jul 2020 06:18:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:reply-to:to:references:in-reply-to:subject:date:message-id :mime-version:content-transfer-encoding:thread-index :content-language; bh=iZKEz3NqwM5PjBrT3zlH/+KJOu6L2I5Fv3vK0DmGICk=; b=ocAx28wJ/qh1KGMZncA80t2rfhPg8Lzp/S1WVUoxeR+PKu5IdRrcClBzOjnybuVHWW jUrNS5vB3VOidD7lIHlF0XQ2EBED9EmfD2lJACxpyI2gzfQf9atUWbMO/YBgsEAieF7O 6p6VF6HjZBRps19z4OsLle4Z056aS4w/mTVH6Gny/ywSISYch4a0Q68mxbu4HqrOg+bP 0DMZ9vc29FiTvHD6pkA9p7mFKacF2RXnpJoD/PXODTbZMIv5oWF1CaimeKmxztiYcUVa fsxhZTg/5vD+soYcfPE70mcSaQI8vZFqmx635BQIGVQIuMyI4RZVSW/onBJkd6bV2LNo 5vTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:reply-to:to:references:in-reply-to:subject :date:message-id:mime-version:content-transfer-encoding:thread-index :content-language; bh=iZKEz3NqwM5PjBrT3zlH/+KJOu6L2I5Fv3vK0DmGICk=; b=HAQlHa4gsNAVhlYCmODiDnnZlHgUZW+xo7y8nPNdxUIqhow4bCXMwGj1lB2Pmkd5hv zt50nqAds/MfZ/DYAJFZSZQzCyPjztphtUEYNUARfXMR3t4lWlRu8lAx5cyGxhZn+CRn 3gwKcFv++No/V7Hv/9bxpRuJIh8zpSmkMQ4hwq6ojs+tArzJzBEl9mbKsn75SRSOqLEf nEDvdoZU6F6s/O5aV8FPqXFYoeSGauXba/QcM2DLUBpjX/1qQkjIIqP0z0SzuZK8xwtY B93is3Eo7J7hnHCOGzeTcpSupnhlW8SXFrYVoL0cRnmUjahrmYXpY9RMzgMWemH+4QZ/ aivA== X-Gm-Message-State: AOAM5312uAGIIQEgYafGFkURcqzrIxc6f7iB4u2fUvalNGOocCoKdnbW JM9jmRI112ycNwSMhCLcj7k= X-Google-Smtp-Source: ABdhPJxf3q1MrZmB0kOmz0U+qsbPZcCRJVpk/mx92IAZputdCGq8nq46gHnBqTgL7R8htPaa05iZGQ== X-Received: by 2002:a17:906:970a:: with SMTP id k10mr18897420ejx.236.1595251114899; Mon, 20 Jul 2020 06:18:34 -0700 (PDT) Received: from CBGR90WXYV0 (54-240-197-226.amazon.com. [54.240.197.226]) by smtp.gmail.com with ESMTPSA id kt4sm14790334ejb.48.2020.07.20.06.18.34 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 20 Jul 2020 06:18:34 -0700 (PDT) From: Paul Durrant X-Google-Original-From: "Paul Durrant" To: "'Owen Smith'" , References: <1594888652-49120-1-git-send-email-owen.smith@citrix.com> <001001d65e76$ad007ad0$07017070$@xen.org> <0ad7a14cc1864c4a97a4665df24f5bde@AMSPEX02CL02.citrite.net> In-Reply-To: <0ad7a14cc1864c4a97a4665df24f5bde@AMSPEX02CL02.citrite.net> Subject: RE: [PATCH] Clear unplug keys if Active device is not the Vendor device Date: Mon, 20 Jul 2020 14:18:33 +0100 Message-ID: <002a01d65e98$4542e460$cfc8ad20$@xen.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQHF93dT2nTZiWuMcifdvJkXYXXBYQNLKmW6AO2qj8qpD2Mu4A== Content-Language: en-gb X-BeenThere: win-pv-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Developer list for the Windows PV Drivers subproject List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Reply-To: paul@xen.org Errors-To: win-pv-devel-bounces@lists.xenproject.org Sender: "win-pv-devel" > -----Original Message----- > From: Owen Smith > Sent: 20 July 2020 11:18 > To: paul@xen.org; win-pv-devel@lists.xenproject.org > Subject: RE: [PATCH] Clear unplug keys if Active device is not the Vendor device > > > > -----Original Message----- > > From: Paul Durrant [mailto:xadimgnik@gmail.com] > > Sent: 20 July 2020 10:18 > > To: Owen Smith ; win-pv- > > devel@lists.xenproject.org > > Subject: RE: [PATCH] Clear unplug keys if Active device is not the Vendor > > device > > > > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments > > unless you have verified the sender and know the content is safe. > > > > > -----Original Message----- > > > From: win-pv-devel On > > > Behalf Of Owen Smith > > > Sent: 16 July 2020 09:38 > > > To: win-pv-devel@lists.xenproject.org > > > Cc: Owen Smith > > > Subject: [PATCH] Clear unplug keys if Active device is not the Vendor > > > device > > > > > > If the Vendor device is distributed by Windows Update, the update > > > process will queue a reboot. > > > > Well, the device is not distributed by Windows Update. Do you mean "If a > > version of XENBUS supporting a vendor device"? (Bear in mind there could > > theoretically be vendor devices other than the Citrix one). > > The message is a little unclear here. If a version of XenBus is distributed by Windows Update for a > given vendor device ID > > > > > > If the vendor device is not the active device, then the unplug keys > > > will not be removed before the next reboot. > > > The reboot will also replace the driver on the active device which > > > requires rebinding child devices, which cannot happen during the early > > > boot phase. This will lead to a 7B bugcheck because XenVbd has not > > > been rebound to the new version of XenBus > > > > What is *supposed* to happen is that the, when a vendor device appears in > > the system necessitating a new XENBUS binding, the co-installer clears the > > unplug keys because the PDOs beneath the non-vendor device will > > disappear at next boot because XENBUS's active device will change. This > > could equally happen if a vendor device subsequently disappears, which I > > don't think this patch covers. > > > > If the XenBus bound to the vendor device is updated, this will replace the driver on any other device, > which will necessitate clearing of the unplug keys. This doesn't happen at the moment, so on the > reboot to complete installation of XenBus on the non-active vendor device ID, the unplug keys are not > removed and the bindings of the child devices on the XenBus on the active non-vendor device ID are not > regenerated. This hits DriverSetActive's fail2 case, where __DriverIsDeviceLegacy(non-vendor-device) > and __DriverIsVendorDevicePresent(). > Is that a problem in itself? If the vendor device is present but the legacy device is already active, then things should be ok, shouldn't they? Ah, maybe I see the issue... A new XENBUS comes from Windows Update but, for some reason, the vendor device was not present when the PV drivers were previously installed. So the PDO tree hangs off the legacy device and hence no driver apart from XENBUS ever gets Windows updates, and then we run into a lack of compatibility issue? > This patch doesn't cover the case where the vendor device disappears, primarily because I didn't hit > this edge case trying to resolve the original issue > It seems that, when XENBUS binds to the vendor device, we need to clear the active device as well as the unplug keys such that, on the next reboot, the PDOs move over onto the vendor device. Would that work? As for the vendor device disappearing, I guess the condition in DriverSetActive() will deal with making the legacy device active and I *think* DriverSetFilterState() will DTRT WRT to unplugging (or not). Paul > > Paul > > > > > > > > Signed-off-by: Owen Smith > > > --- > > > src/coinst/coinst.c | 25 ++++++++++++++++++++++--- > > > 1 file changed, 22 insertions(+), 3 deletions(-) > > > > > > diff --git a/src/coinst/coinst.c b/src/coinst/coinst.c index > > > 7b96f59..8e57a10 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,21 +1793,25 @@ DifInstallPostProcess( { > > > BOOLEAN NewBinding; > > > BOOLEAN Active; > > > + BOOLEAN VendorIsActive; > > > > > > Log("====>"); > > > > > > NewBinding = (BOOLEAN)(ULONG_PTR)Context->PrivateData; > > > > > > Active = TRUE; > > > + VendorIsActive = FALSE; > > > > > > (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"); > > > } > > > -- > > > 2.16.2.windows.1 > > > > > > >