[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Use UNPLUG v3



Thanks for the fix.

On 30/06/2025 12:41, Owen Smith wrote:
> Uses UnplugReboot to request a reboot from xenbus_monitor, rather than
> writing values into an absolute registry path, which is a driver verifier
> violation during Server 2025 WHQL testing.
> Also removes the RequestKey from the INF file.
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxx>

Reviewed-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>

> ---
>   include/revision.h         |  3 +-
>   include/unplug_interface.h | 40 +++++++++++++++-
>   src/xenvif.inf             |  8 ++--
>   src/xenvif/driver.c        | 93 --------------------------------------
>   src/xenvif/driver.h        |  5 --
>   src/xenvif/pdo.c           | 20 +++++++-
>   6 files changed, 62 insertions(+), 107 deletions(-)
> 
> diff --git a/include/revision.h b/include/revision.h
> index 2f4ac80..656f171 100644
> --- a/include/revision.h
> +++ b/include/revision.h
> @@ -46,6 +46,7 @@
>       DEFINE_REVISION(0x09000001,  2,  8,   2,  1,   1),  \
>       DEFINE_REVISION(0x09000002,  2,  9,   2,  1,   1),  \
>       DEFINE_REVISION(0x09000003,  2,  10,  2,  1,   1),  \
> -    DEFINE_REVISION(0x09000004,  2,  10,  2,  1,   2)
> +    DEFINE_REVISION(0x09000004,  2,  10,  2,  1,   2),  \
> +    DEFINE_REVISION(0x09000005,  2,  10,  2,  1,   3)
>   
>   #endif  // _REVISION_H
> diff --git a/include/unplug_interface.h b/include/unplug_interface.h
> index dbdc76d..dad3afc 100644
> --- a/include/unplug_interface.h
> +++ b/include/unplug_interface.h
> @@ -99,6 +99,28 @@ typedef BOOLEAN
>       IN  XENBUS_UNPLUG_DEVICE_TYPE   Type
>       );
>   
> +/*! \typedef XENBUS_UNPLUG_BOOT_EMULATED
> +    \brief Should the boot disk be emulated
> +
> +    \param Interface The interface header
> +*/
> +typedef BOOLEAN
> +(*XENBUS_UNPLUG_BOOT_EMULATED)(
> +    IN  PINTERFACE                  Interface
> +    );
> +
> +/*! \typedef XENBUS_UNPLUG_REBOOT
> +    \brief Request a reboot to complete setup
> +
> +    \param Interface The interface header
> +    \param Module The module name requesting a reboot
> +*/
> +typedef VOID
> +(*XENBUS_UNPLUG_REBOOT)(
> +    IN  PINTERFACE                  Interface,
> +    IN  PCHAR                       Module
> +    );
> +
>   // {73db6517-3d06-4937-989f-199b7501e229}
>   DEFINE_GUID(GUID_XENBUS_UNPLUG_INTERFACE,
>   0x73db6517, 0x3d06, 0x4937, 0x98, 0x9f, 0x19, 0x9b, 0x75, 0x01, 0xe2, 0x29);
> @@ -126,7 +148,21 @@ struct _XENBUS_UNPLUG_INTERFACE_V2 {
>       XENBUS_UNPLUG_IS_REQUESTED  UnplugIsRequested;
>   };
>   
> -typedef struct _XENBUS_UNPLUG_INTERFACE_V2 XENBUS_UNPLUG_INTERFACE, 
> *PXENBUS_UNPLUG_INTERFACE;
> +/*! \struct _XENBUS_UNPLUG_INTERFACE_V3
> +    \brief UNPLUG interface version 3
> +    \ingroup interfaces
> +*/
> +struct _XENBUS_UNPLUG_INTERFACE_V3 {
> +    INTERFACE                   Interface;
> +    XENBUS_UNPLUG_ACQUIRE       UnplugAcquire;
> +    XENBUS_UNPLUG_RELEASE       UnplugRelease;
> +    XENBUS_UNPLUG_REQUEST       UnplugRequest;
> +    XENBUS_UNPLUG_IS_REQUESTED  UnplugIsRequested;
> +    XENBUS_UNPLUG_BOOT_EMULATED UnplugBootEmulated;
> +    XENBUS_UNPLUG_REBOOT        UnplugReboot;
> +};
> +
> +typedef struct _XENBUS_UNPLUG_INTERFACE_V3 XENBUS_UNPLUG_INTERFACE, 
> *PXENBUS_UNPLUG_INTERFACE;
>   
>   /*! \def XENBUS_UNPLUG
>       \brief Macro at assist in method invocation
> @@ -137,6 +173,6 @@ typedef struct _XENBUS_UNPLUG_INTERFACE_V2 
> XENBUS_UNPLUG_INTERFACE, *PXENBUS_UNP
>   #endif  // _WINDLL
>   
>   #define XENBUS_UNPLUG_INTERFACE_VERSION_MIN  1
> -#define XENBUS_UNPLUG_INTERFACE_VERSION_MAX  2
> +#define XENBUS_UNPLUG_INTERFACE_VERSION_MAX  3
>   
>   #endif  // _XENBUS_UNPLUG_INTERFACE_H
> diff --git a/src/xenvif.inf b/src/xenvif.inf
> index 8bfc37c..0430f04 100644
> --- a/src/xenvif.inf
> +++ b/src/xenvif.inf
> @@ -56,9 +56,9 @@ xenvif.sys=0,,
>   ; DisplayName               Section         DeviceID
>   ; -----------               -------         --------
>   
> -%XenVifName%         =XenVif_Inst,   
> XENBUS\VEN_@VENDOR_PREFIX@@VENDOR_DEVICE_ID@&DEV_VIF&REV_0900000A
> -%XenVifName%         =XenVif_Inst,   
> XENBUS\VEN_@VENDOR_PREFIX@0001&DEV_VIF&REV_0900000A
> -%XenVifName%         =XenVif_Inst,   
> XENBUS\VEN_@VENDOR_PREFIX@0002&DEV_VIF&REV_0900000A
> +%XenVifName%         =XenVif_Inst,   
> XENBUS\VEN_@VENDOR_PREFIX@@VENDOR_DEVICE_ID@&DEV_VIF&REV_0900000B
> +%XenVifName%         =XenVif_Inst,   
> XENBUS\VEN_@VENDOR_PREFIX@0001&DEV_VIF&REV_0900000B
> +%XenVifName%         =XenVif_Inst,   
> XENBUS\VEN_@VENDOR_PREFIX@0002&DEV_VIF&REV_0900000B
>   
>   [XenVif_Inst]
>   CopyFiles=XenVif_Copyfiles
> @@ -83,7 +83,6 @@ HKR,,"BootFlags",0x00010003,0x81
>   
>   [XenVif_Parameters]
>   HKR,"Parameters",,0x00000010
> -HKR,"Parameters","RequestKey",0x00000000,%RequestKey%
>   HKR,"Parameters","FrontendMaxQueues",0x00010001,0x00000008
>   
>   [XenVif_Unplug]
> @@ -94,7 +93,6 @@ HKLM,%UnplugKey%,"NICS",0x00010001,0
>   Vendor="@VENDOR_NAME@"
>   DiskDesc="@PRODUCT_NAME@ PV Network Class Package"
>   XenVifName="@PRODUCT_NAME@ PV Network Class"
> -RequestKey="SYSTEM\CurrentControlSet\Services\xenbus_monitor\Request"
>   UnplugKey="SYSTEM\CurrentControlSet\Services\XEN\Unplug"
>   
>   SERVICE_BOOT_START=0x0
> diff --git a/src/xenvif/driver.c b/src/xenvif/driver.c
> index 3345607..438c21a 100644
> --- a/src/xenvif/driver.c
> +++ b/src/xenvif/driver.c
> @@ -49,7 +49,6 @@ typedef struct _XENVIF_DRIVER {
>       HANDLE              ParametersKey;
>       HANDLE              AddressesKey;
>       HANDLE              SettingsKey;
> -    BOOLEAN             NeedReboot;
>   } XENVIF_DRIVER, *PXENVIF_DRIVER;
>   
>   static XENVIF_DRIVER    Driver;
> @@ -168,96 +167,6 @@ DriverGetSettingsKey(
>       return __DriverGetSettingsKey();
>   }
>   
> -#define MAXNAMELEN  256
> -
> -static FORCEINLINE VOID
> -__DriverRequestReboot(
> -    VOID
> -    )
> -{
> -    PANSI_STRING    Ansi;
> -    CHAR            RequestKeyName[MAXNAMELEN];
> -    HANDLE          RequestKey;
> -    HANDLE          SubKey;
> -    NTSTATUS        status;
> -
> -    Info("====>\n");
> -
> -    ASSERT3U(KeGetCurrentIrql(), ==, PASSIVE_LEVEL);
> -
> -    status = RegistryQuerySzValue(__DriverGetParametersKey(),
> -                                  "RequestKey",
> -                                  NULL,
> -                                  &Ansi);
> -    if (!NT_SUCCESS(status))
> -        goto fail1;
> -
> -    status = RtlStringCbPrintfA(RequestKeyName,
> -                                MAXNAMELEN,
> -                                "\\Registry\\Machine\\%Z",
> -                                &Ansi[0]);
> -    ASSERT(NT_SUCCESS(status));
> -
> -    status = RegistryCreateSubKey(NULL,
> -                                  RequestKeyName,
> -                                  REG_OPTION_NON_VOLATILE,
> -                                  &RequestKey);
> -    if (!NT_SUCCESS(status))
> -        goto fail2;
> -
> -    status = RegistryCreateSubKey(RequestKey,
> -                                  __MODULE__,
> -                                  REG_OPTION_VOLATILE,
> -                                  &SubKey);
> -    if (!NT_SUCCESS(status))
> -        goto fail3;
> -
> -    status = RegistryUpdateDwordValue(SubKey,
> -                                      "Reboot",
> -                                      1);
> -    if (!NT_SUCCESS(status))
> -        goto fail4;
> -
> -    RegistryCloseKey(SubKey);
> -
> -    RegistryFreeSzValue(Ansi);
> -
> -    Info("<====\n");
> -
> -    return;
> -
> -fail4:
> -    Error("fail4\n");
> -
> -    RegistryCloseKey(SubKey);
> -
> -fail3:
> -    Error("fail3\n");
> -
> -    RegistryCloseKey(RequestKey);
> -
> -fail2:
> -    Error("fail2\n");
> -
> -    RegistryFreeSzValue(Ansi);
> -
> -fail1:
> -    Error("fail1 (%08x)\n", status);
> -}
> -
> -VOID
> -DriverRequestReboot(
> -    VOID
> -    )
> -{
> -    if (Driver.NeedReboot)
> -        return;
> -
> -    __DriverRequestReboot();
> -
> -    Driver.NeedReboot = TRUE;
> -}
> -
>   DRIVER_UNLOAD       DriverUnload;
>   
>   VOID
> @@ -273,8 +182,6 @@ DriverUnload(
>   
>       Trace("====>\n");
>   
> -    Driver.NeedReboot = FALSE;
> -
>       SettingsKey = __DriverGetSettingsKey();
>       __DriverSetSettingsKey(NULL);
>   
> diff --git a/src/xenvif/driver.h b/src/xenvif/driver.h
> index b5b9a3d..96be6df 100644
> --- a/src/xenvif/driver.h
> +++ b/src/xenvif/driver.h
> @@ -58,11 +58,6 @@ DriverGetSettingsKey(
>       VOID
>       );
>   
> -extern VOID
> -DriverRequestReboot(
> -    VOID
> -    );
> -
>   typedef struct _XENVIF_PDO  XENVIF_PDO, *PXENVIF_PDO;
>   typedef struct _XENVIF_FDO  XENVIF_FDO, *PXENVIF_FDO;
>   
> diff --git a/src/xenvif/pdo.c b/src/xenvif/pdo.c
> index a3ae061..28121ac 100644
> --- a/src/xenvif/pdo.c
> +++ b/src/xenvif/pdo.c
> @@ -1253,6 +1253,24 @@ PdoUnplugRequest(
>       XENBUS_UNPLUG(Release, &Pdo->UnplugInterface);
>   }
>   
> +static VOID
> +PdoRequestReboot(
> +    IN  PXENVIF_PDO Pdo
> +    )
> +{
> +    NTSTATUS        status;
> +
> +    status = XENBUS_UNPLUG(Acquire, &Pdo->UnplugInterface);
> +    if (!NT_SUCCESS(status))
> +        return;
> +
> +    XENBUS_UNPLUG(Reboot,
> +                  &Pdo->UnplugInterface,
> +                  __MODULE__);
> +
> +    XENBUS_UNPLUG(Release, &Pdo->UnplugInterface);
> +}
> +
>   static BOOLEAN
>   PdoUnplugRequested(
>       IN  PXENVIF_PDO Pdo
> @@ -1439,7 +1457,7 @@ PdoStartDevice(
>       status = PdoParseMibTable(Pdo, SoftwareKey);
>       if (status == STATUS_PNP_REBOOT_REQUIRED || !PdoUnplugRequested(Pdo)) {
>           PdoUnplugRequest(Pdo, TRUE);
> -        DriverRequestReboot();
> +        PdoRequestReboot(Pdo);
>   
>           status = STATUS_PNP_REBOOT_REQUIRED;
>           goto fail5;



Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.