RE: [PATCH 3/6] Delete stornvme's StartOverride

[Owen Smith <owen.smith@xxxxxxxxxx>]

-----Original Message-----
From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
Owen Smith
Sent: Monday, June 21, 2021 4:33 PM
To: paul@xxxxxxx; win-pv-devel@xxxxxxxxxxxxxxxxxxxx
Subject: RE: [PATCH 3/6] Delete stornvme's StartOverride

[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 <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
Paul Durrant
Sent: Monday, June 21, 2021 2:48 PM
To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 3/6] Delete stornvme's StartOverride

[CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments 
unless you have verified the sender and know the content is safe.

On 17/06/2021 13:33, Owen Smith wrote:
> Using an emulated NVMe device for installation will not be able to 
> revert to emulated NVMe device once xenvbd has been used to boot. This 
> is due to stornvme creating a StartOverride value that overrides 
> stornvme's StartType to manual (from boot) when stornvme is not loaded 
> during boot. This occurs when xenbus has unplugged the emulated NVMe 
> controller at boot. Disabling the unplug is not sufficient to have 
> stornvme load at boot, which results in a 0x7B bugcheck.

Ick!

> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> ---
>   src/monitor/monitor.c | 60 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 60 insertions(+)
> 
> diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c

Not sure the monitor is the best place to do this; it seems a little too 
critical for that. Could we use this ancient interface instead?

https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/ntddk/nf-ntddk-ioregisterbootdriverreinitialization

   Paul


This looks doable, calling IoRegisterBootDriverReinitialization from XenVbd's 
DriverEntry and clearing the StartOverride from the reinitialize routine

Owen


After some investigation, the IoRegisterBootDriverReinitialization callback 
occurs too early and StorNvme adds the value later. It should only be required 
to ensure this value is removed before the VM is shutdown or rebooted, so its 
possible that removing the StartOverride in the unload/power down code would be 
appropriate.

Owen


> index 9adfb79..c858249 100644
> --- a/src/monitor/monitor.c
> +++ b/src/monitor/monitor.c
> @@ -1172,6 +1172,64 @@ fail1:
>       return FALSE;
>   }
>   
> +static BOOL
> +DeleteStartOverride(
> +    _In_    LPTSTR      DriverName
> +    )
> +{
> +    TCHAR               ServiceKeyName[MAX_PATH];
> +    HKEY                Key;
> +    HRESULT             Error;
> +
> +    Error = StringCbPrintf(ServiceKeyName,
> +                            MAX_PATH,
> +                            SERVICES_KEY "\\%s",
> +                            DriverName);
> +    if (!SUCCEEDED(Error))
> +        goto fail1;
> +
> +    Error = RegOpenKeyEx(HKEY_LOCAL_MACHINE,
> +                         ServiceKeyName,
> +                         0,
> +                         KEY_ALL_ACCESS,
> +                         &Key);
> +    if (Error != ERROR_SUCCESS) {
> +        SetLastError(Error);
> +        goto fail2;
> +    }
> +
> +    Error = RegDeleteTree(Key,
> +                          "StartOverride");
> +    if (Error != ERROR_SUCCESS) {
> +        SetLastError(Error);
> +        goto fail3;
> +    }
> +
> +    RegCloseKey(Key);
> +
> +    return TRUE;
> +
> +fail3:
> +    Log("fail3");
> +
> +    RegCloseKey(Key);
> +
> +fail2:
> +    Log("fail2");
> +
> +fail1:
> +    Error = GetLastError();
> +
> +    {
> +        PTCHAR  Message;
> +        Message = GetErrorMessage(Error);
> +        Log("fail1 (%s)", Message);
> +        LocalFree(Message);
> +    }
> +
> +    return FALSE;
> +}
> +
>   VOID WINAPI
>   MonitorMain(
>       _In_    DWORD       argc,
> @@ -1251,6 +1309,7 @@ MonitorMain(
>       if (!Success)
>           goto fail9;
>   
> +    (VOID) DeleteStartOverride("stornvme");
>       SetEvent(Context->RequestEvent);
>   
>       ReportStatus(SERVICE_RUNNING, NO_ERROR, 0); @@ -1285,6 +1344,7 
> @@ MonitorMain(
>       }
>   
>   done:
> +    (VOID) DeleteStartOverride("stornvme");
>       (VOID) RegDeleteTree(Context->RequestKey, NULL);
>   
>       free(Context->Question);
>