Re: [PATCH 2/2] Add emulated NVMe to IsDiskPresent results

[Paul Durrant <xadimgnik@xxxxxxxxx>]

On 28/06/2021 13:58, Owen Smith wrote:
> IsDiskPresent currently only reports the presence of emulated IDE disks. When
> using emulated NVMe disks, its possible to start booting off the emulated disk,
> but have XenVbd 'take over' resulting in storage requests to the emulated NVMe
> disk timing out and failing. This results in a Windows error on boot
> "Status 0xc000000e. A required device isnt connected or can't be accessed"
>
> Query the CompatibleIDs and, if present, add the last CompatibleID to emulated
> objects of type PCI. When querying if a disk is preset, also check for PCI
> devices which match the CompatibleID "PCI\CC_0108". This will prevent XenVbd
> enumerating a PV disk which is has a matching emulated NVMe device.
>
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
Acked-by: Paul Durrant <paul@xxxxxxx>

> ---
>   src/xenfilt/emulated.c | 48 ++++++++++++++++++++++++++++++++++++++----
>   src/xenfilt/emulated.h |  1 +
>   src/xenfilt/pdo.c      | 24 ++++++++++++++++++++-
>   3 files changed, 68 insertions(+), 5 deletions(-)
>
> diff --git a/src/xenfilt/emulated.c b/src/xenfilt/emulated.c
> index 827c905..189b349 100644
> --- a/src/xenfilt/emulated.c
> +++ b/src/xenfilt/emulated.c
> @@ -46,6 +46,7 @@
>   typedef struct _XENFILT_EMULATED_DEVICE_DATA {
>       CHAR    DeviceID[MAXNAMELEN];
>       CHAR    InstanceID[MAXNAMELEN];
> +    CHAR    CompatibleID[MAXNAMELEN];
>   } XENFILT_EMULATED_DEVICE_DATA, *PXENFILT_EMULATED_DEVICE_DATA;
>   
>   typedef struct _XENFILT_EMULATED_DISK_DATA {
> @@ -92,9 +93,12 @@ EmulatedSetObjectDeviceData(
>       IN  PXENFILT_EMULATED_OBJECT        EmulatedObject,
>       IN  XENFILT_EMULATED_OBJECT_TYPE    Type,
>       IN  PCHAR                           DeviceID,
> -    IN  PCHAR                           InstanceID
> +    IN  PCHAR                           InstanceID,
> +    IN  PCHAR                           CompatibleIDs OPTIONAL
>       )
>   {
> +    ULONG                               Index;
> +    PCHAR                               LastMatch;
>       NTSTATUS                            status;
>   
>       status = STATUS_INVALID_PARAMETER;
> @@ -113,6 +117,30 @@ EmulatedSetObjectDeviceData(
>                                   InstanceID);
>       ASSERT(NT_SUCCESS(status));
>   
> +    if (CompatibleIDs == NULL)
> +        goto done;
> +
> +    Index = 0;
> +    LastMatch = CompatibleIDs;
> +    for (;;) {
> +        ULONG                           Length;
> +
> +        Length = (ULONG)strlen(&CompatibleIDs[Index]);
> +        if (Length == 0)
> +            break;
> +
> +        LastMatch = &CompatibleIDs[Index];
> +
> +        Index += Length + 1;
> +    }
> +
> +    status = RtlStringCbPrintfA(EmulatedObject->Data.Device.CompatibleID,
> +                                MAXNAMELEN,
> +                                "%s",
> +                                LastMatch);
> +    ASSERT(NT_SUCCESS(status));
> +
> +done:
>       return STATUS_SUCCESS;
>   
>   fail1:
> @@ -126,7 +154,8 @@ EmulatedSetObjectDiskData(
>       IN  PXENFILT_EMULATED_OBJECT        EmulatedObject,
>       IN  XENFILT_EMULATED_OBJECT_TYPE    Type,
>       IN  PCHAR                           DeviceID,
> -    IN  PCHAR                           InstanceID
> +    IN  PCHAR                           InstanceID,
> +    IN  PCHAR                           CompatibleIDs OPTIONAL
>       )
>   {
>       PCHAR                               End;
> @@ -136,6 +165,7 @@ EmulatedSetObjectDiskData(
>       NTSTATUS                            status;
>   
>       UNREFERENCED_PARAMETER(DeviceID);
> +    UNREFERENCED_PARAMETER(CompatibleIDs);
>   
>       status = STATUS_INVALID_PARAMETER;
>       if (Type != XENFILT_EMULATED_OBJECT_TYPE_IDE)
> @@ -194,6 +224,7 @@ EmulatedAddObject(
>       IN  PXENFILT_EMULATED_CONTEXT       Context,
>       IN  PCHAR                           DeviceID,
>       IN  PCHAR                           InstanceID,
> +    IN  PCHAR                           CompatibleIDs OPTIONAL,
>       IN  XENFILT_EMULATED_OBJECT_TYPE    Type,
>       OUT PXENFILT_EMULATED_OBJECT        *EmulatedObject
>       )
> @@ -214,14 +245,16 @@ EmulatedAddObject(
>           status = EmulatedSetObjectDeviceData(*EmulatedObject,
>                                                Type,
>                                                DeviceID,
> -                                             InstanceID);
> +                                             InstanceID,
> +                                             CompatibleIDs);
>           break;
>   
>       case XENFILT_EMULATED_OBJECT_TYPE_IDE:
>           status = EmulatedSetObjectDiskData(*EmulatedObject,
>                                              Type,
>                                              DeviceID,
> -                                           InstanceID);
> +                                           InstanceID,
> +                                           CompatibleIDs);
>           break;
>   
>       default:
> @@ -339,6 +372,13 @@ EmulatedIsDiskPresent(
>               break;
>           }
>   
> +        if (EmulatedObject->Type == XENFILT_EMULATED_OBJECT_TYPE_PCI &&
> +            _stricmp("PCI\\CC_0108", EmulatedObject->Data.Device.CompatibleID) == 0 
> &&
> +            Index <= 3) {
> +            Trace("FOUND\n");
> +            break;
> +        }
> +
>           ListEntry = ListEntry->Flink;
>       }
>   
> diff --git a/src/xenfilt/emulated.h b/src/xenfilt/emulated.h
> index 57edee2..049c0bb 100644
> --- a/src/xenfilt/emulated.h
> +++ b/src/xenfilt/emulated.h
> @@ -69,6 +69,7 @@ EmulatedAddObject(
>       IN  PXENFILT_EMULATED_CONTEXT       Context,
>       IN  PCHAR                           DeviceID,
>       IN  PCHAR                           InstanceID,
> +    IN  PCHAR                           CompatibleIDs OPTIONAL,
>       IN  XENFILT_EMULATED_OBJECT_TYPE    Type,
>       OUT PXENFILT_EMULATED_OBJECT        *EmulatedObject
>       );
> diff --git a/src/xenfilt/pdo.c b/src/xenfilt/pdo.c
> index 0f6e6ce..b3569a3 100644
> --- a/src/xenfilt/pdo.c
> +++ b/src/xenfilt/pdo.c
> @@ -376,6 +376,14 @@ __PdoGetInstanceID(
>              Dx->InstanceID : "";
>   }
>   
> +static FORCEINLINE XENFILT_EMULATED_OBJECT_TYPE
> +__PdoGetType(
> +    IN  PXENFILT_PDO    Pdo
> +    )
> +{
> +    return Pdo->Type;
> +}
> +
>   static FORCEINLINE PCHAR
>   __PdoGetLocationInformation(
>       IN  PXENFILT_PDO    Pdo
> @@ -2112,6 +2120,7 @@ PdoCreate(
>       PDEVICE_OBJECT                  FilterDeviceObject;
>       PXENFILT_DX                     Dx;
>       PXENFILT_PDO                    Pdo;
> +    PCHAR                           CompatibleIDs;
>       NTSTATUS                        status;
>   
>       ASSERT(Type != XENFILT_EMULATED_OBJECT_TYPE_UNKNOWN);
> @@ -2173,14 +2182,24 @@ PdoCreate(
>       if (!NT_SUCCESS(status))
>           goto fail6;
>   
> +    status = DriverQueryId(Pdo->LowerDeviceObject,
> +                           BusQueryCompatibleIDs,
> +                           &CompatibleIDs);
> +    if (!NT_SUCCESS(status))
> +        CompatibleIDs = NULL;
> +
>       status = EmulatedAddObject(DriverGetEmulatedContext(),
>                                  __PdoGetDeviceID(Pdo),
>                                  __PdoGetInstanceID(Pdo),
> -                               Pdo->Type,
> +                               CompatibleIDs,
> +                               __PdoGetType(Pdo),
>                                  &Pdo->EmulatedObject);
>       if (!NT_SUCCESS(status))
>           goto fail7;
>   
> +    if (CompatibleIDs)
> +        ExFreePool(CompatibleIDs);
> +
>       __PdoSetName(Pdo);
>   
>       Info("%p (%s) %s\n",
> @@ -2204,6 +2223,9 @@ PdoCreate(
>   fail7:
>       Error("fail7\n");
>   
> +    if (CompatibleIDs)
> +        ExFreePool(CompatibleIDs);
> +
>       PdoClearDeviceInformation(Pdo);
>   
>   fail6: