Re: [PATCH 4/6] Add emulated type override per device ID

[Paul Durrant <xadimgnik@xxxxxxxxx>]

On 17/06/2021 13:33, Owen Smith wrote:
> Add the option to override a device nodes emulated type for a specific
> device ID, and adds the NVME type.
> Specific HardwareIDs may be associated with a more specific type than 'Device'.
> Add a way of specifying a particular HardwareID as an emulated NVMe controller
> so that when XenVbd queries if a disk is present, XenFilt can respond correctly
> if either its emulated IDE disk or emulated NVMe disk is present.
> Certain upgrade situations could leave the unplug key empty (so the emulated
> NVMe device is in use), and this could lead to XenVbd not identifying the
> emulated NVMe disk and proceeding to connect to the backend (which could already
> be in use by the emulated NVMe disk). This situation leads to a recovery prompt
> "A required device isn't connected or can't be accessed"
>
> Note: this patch doesnt define the HardwareID(s) associated with emulated NVMe
> devices, as this is dependent on the VM configuration.
Could we not use CompatibleIDs and look for "PCI\\CC_010802" (or perhaps 
even drop the 02)?
  Paul

> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
> ---
>   src/xenfilt/driver.c   | 20 +++++++++++++----
>   src/xenfilt/driver.h   |  5 +++++
>   src/xenfilt/emulated.c | 48 ++++++++++++++++++++++++++++++++++++++++
>   src/xenfilt/emulated.h |  3 ++-
>   src/xenfilt/pdo.c      | 50 ++++++++++++++++++++++++++++++++++++++++++
>   5 files changed, 121 insertions(+), 5 deletions(-)
>
> diff --git a/src/xenfilt/driver.c b/src/xenfilt/driver.c
> index e9e6673..e25c208 100644
> --- a/src/xenfilt/driver.c
> +++ b/src/xenfilt/driver.c
> @@ -723,6 +723,21 @@ fail1:
>       return status;
>   }
>   
> +XENFILT_EMULATED_OBJECT_TYPE
> +DriverParseEmulatedType(
> +    IN  PANSI_STRING                Ansi
> +    )
> +{
> +    if (_strnicmp(Ansi->Buffer, "PCI", Ansi->Length) == 0)
> +        return XENFILT_EMULATED_OBJECT_TYPE_PCI;
> +    else if (_strnicmp(Ansi->Buffer, "IDE", Ansi->Length) == 0)
> +        return XENFILT_EMULATED_OBJECT_TYPE_IDE;
> +    else if (_strnicmp(Ansi->Buffer, "NVME", Ansi->Length) == 0)
> +        return XENFILT_EMULATED_OBJECT_TYPE_NVME;
> +
> +    return XENFILT_EMULATED_OBJECT_TYPE_UNKNOWN;
> +}
> +
>   static XENFILT_EMULATED_OBJECT_TYPE
>   DriverGetEmulatedType(
>       IN  PCHAR                       Id
> @@ -753,10 +768,7 @@ DriverGetEmulatedType(
>           if (NT_SUCCESS(status)) {
>               Info("MATCH: %s -> %Z\n", &Id[Index], Ansi);
>   
> -            if (_strnicmp(Ansi->Buffer, "PCI", Ansi->Length) == 0)
> -                Type = XENFILT_EMULATED_OBJECT_TYPE_PCI;
> -            else if (_strnicmp(Ansi->Buffer, "IDE", Ansi->Length) == 0)
> -                Type = XENFILT_EMULATED_OBJECT_TYPE_IDE;
> +            Type = DriverParseEmulatedType(Ansi);
>   
>               RegistryFreeSzValue(Ansi);
>           } else {
> diff --git a/src/xenfilt/driver.h b/src/xenfilt/driver.h
> index 286580b..ee656c8 100644
> --- a/src/xenfilt/driver.h
> +++ b/src/xenfilt/driver.h
> @@ -90,6 +90,11 @@ DriverQueryDeviceText(
>   
>   #include "emulated.h"
>   
> +XENFILT_EMULATED_OBJECT_TYPE
> +DriverParseEmulatedType(
> +    IN  PANSI_STRING                Ansi
> +    );
> +
>   PXENFILT_EMULATED_CONTEXT
>   DriverGetEmulatedContext(
>       VOID
> diff --git a/src/xenfilt/emulated.c b/src/xenfilt/emulated.c
> index 827c905..df8cbaf 100644
> --- a/src/xenfilt/emulated.c
> +++ b/src/xenfilt/emulated.c
> @@ -52,9 +52,15 @@ typedef struct _XENFILT_EMULATED_DISK_DATA {
>       ULONG   Index;
>   } XENFILT_EMULATED_DISK_DATA, *PXENFILT_EMULATED_DISK_DATA;
>   
> +typedef struct _XENFILT_EMULATED_NVME_DATA {
> +    ULONG   StartIndex;
> +    ULONG   EndIndex;
> +} XENFILT_EMULATED_NVME_DATA, *PXENFILT_EMULATED_NVME_DATA;
> +
>   typedef union _XENFILT_EMULATED_OBJECT_DATA {
>       XENFILT_EMULATED_DEVICE_DATA Device;
>       XENFILT_EMULATED_DISK_DATA   Disk;
> +    XENFILT_EMULATED_NVME_DATA   Nvme;
>   } XENFILT_EMULATED_OBJECT_DATA, *PXENFILT_EMULATED_OBJECT_DATA;
>   
>   struct _XENFILT_EMULATED_OBJECT {
> @@ -189,6 +195,34 @@ fail1:
>       return status;
>   }
>   
> +static NTSTATUS
> +EmulatedSetObjectNvmeData(
> +    IN  PXENFILT_EMULATED_OBJECT        EmulatedObject,
> +    IN  XENFILT_EMULATED_OBJECT_TYPE    Type,
> +    IN  PCHAR                           DeviceID,
> +    IN  PCHAR                           InstanceID
> +    )
> +{
> +    NTSTATUS                            status;
> +
> +    UNREFERENCED_PARAMETER(DeviceID);
> +    UNREFERENCED_PARAMETER(InstanceID);
> +
> +    status = STATUS_INVALID_PARAMETER;
> +    if (Type != XENFILT_EMULATED_OBJECT_TYPE_NVME)
> +        goto fail1;
> +
> +    EmulatedObject->Data.Nvme.StartIndex = 0;
> +    EmulatedObject->Data.Nvme.EndIndex = 3;
> +
> +    return STATUS_SUCCESS;
> +
> +fail1:
> +    Error("fail1 (%08x)\n", status);
> +
> +    return status;
> +}
> +
>   NTSTATUS
>   EmulatedAddObject(
>       IN  PXENFILT_EMULATED_CONTEXT       Context,
> @@ -224,6 +258,13 @@ EmulatedAddObject(
>                                              InstanceID);
>           break;
>   
> +    case XENFILT_EMULATED_OBJECT_TYPE_NVME:
> +        status = EmulatedSetObjectNvmeData(*EmulatedObject,
> +                                           Type,
> +                                           DeviceID,
> +                                           InstanceID);
> +        break;
> +
>       default:
>           status = STATUS_INVALID_PARAMETER;
>           break;
> @@ -339,6 +380,13 @@ EmulatedIsDiskPresent(
>               break;
>           }
>   
> +        if (EmulatedObject->Type == XENFILT_EMULATED_OBJECT_TYPE_NVME &&
> +            Index >= EmulatedObject->Data.Nvme.StartIndex &&
> +            Index <= EmulatedObject->Data.Nvme.EndIndex) {
> +            Trace("FOUND\n");
> +            break;
> +        }
> +
>           ListEntry = ListEntry->Flink;
>       }
>   
> diff --git a/src/xenfilt/emulated.h b/src/xenfilt/emulated.h
> index 57edee2..499e43c 100644
> --- a/src/xenfilt/emulated.h
> +++ b/src/xenfilt/emulated.h
> @@ -41,7 +41,8 @@ typedef struct _XENFILT_EMULATED_CONTEXT 
> XENFILT_EMULATED_CONTEXT, *PXENFILT_EMU
>   typedef enum _XENFILT_EMULATED_OBJECT_TYPE {
>       XENFILT_EMULATED_OBJECT_TYPE_UNKNOWN = 0,
>       XENFILT_EMULATED_OBJECT_TYPE_PCI,
> -    XENFILT_EMULATED_OBJECT_TYPE_IDE
> +    XENFILT_EMULATED_OBJECT_TYPE_IDE,
> +    XENFILT_EMULATED_OBJECT_TYPE_NVME
>   } XENFILT_EMULATED_OBJECT_TYPE, *PXENFILT_EMULATED_OBJECT_TYPE;
>   
>   typedef struct _XENFILT_EMULATED_OBJECT XENFILT_EMULATED_OBJECT, *PXENFILT_EMULATED_OBJECT;
> diff --git a/src/xenfilt/pdo.c b/src/xenfilt/pdo.c
> index 0f6e6ce..b0e0e0f 100644
> --- a/src/xenfilt/pdo.c
> +++ b/src/xenfilt/pdo.c
> @@ -42,6 +42,7 @@
>   #include "pdo.h"
>   #include "thread.h"
>   #include "driver.h"
> +#include "registry.h"
>   #include "dbg_print.h"
>   #include "assert.h"
>   #include "util.h"
> @@ -251,6 +252,54 @@ __PdoGetFdo(
>       return Pdo->Fdo;
>   }
>   
> +
> +static FORCEINLINE NTSTATUS
> +__PdoGetEmulatedTypeOverride(
> +    IN  PXENFILT_PDO                Pdo,
> +    IN  PCHAR                       DeviceID
> +    )
> +{
> +    XENFILT_EMULATED_OBJECT_TYPE    Type;
> +    PANSI_STRING                    Ansi;
> +    HANDLE                          ParametersKey;
> +    HANDLE                          OverrideKey;
> +    NTSTATUS                        status;
> +
> +    ParametersKey = DriverGetParametersKey();
> +
> +    status = RegistryOpenSubKey(ParametersKey,
> +                                "Override",
> +                                GENERIC_READ,
> +                                &OverrideKey);
> +    if (!NT_SUCCESS(status))
> +        goto fail1;
> +
> +    status = RegistryQuerySzValue(OverrideKey,
> +                                  DeviceID,
> +                                  NULL,
> +                                  &Ansi);
> +    if (!NT_SUCCESS(status))
> +        goto fail2;
> +
> +    Type = DriverParseEmulatedType(Ansi);
> +    if (Type != XENFILT_EMULATED_OBJECT_TYPE_UNKNOWN) {
> +        Info("%s = %Z\n", DeviceID, Ansi);
> +        Pdo->Type = Type;
> +    }
> +
> +    RegistryFreeSzValue(Ansi);
> +
> +    RegistryCloseKey(OverrideKey);
> +
> +    return STATUS_SUCCESS;
> +
> +fail2:
> +    RegistryCloseKey(OverrideKey);
> +
> +fail1:
> +    return status;
> +}
> +
>   static NTSTATUS
>   PdoSetDeviceInformation(
>       IN  PXENFILT_PDO    Pdo
> @@ -313,6 +362,7 @@ PdoSetDeviceInformation(
>       Dx->DeviceID = DeviceID;
>       Dx->InstanceID = InstanceID;
>       Dx->LocationInformation = LocationInformation;
> +    (VOID) __PdoGetEmulatedTypeOverride(Pdo, DeviceID);
>   
>       return STATUS_SUCCESS;
>