[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add XENFILT_EMULATED_OBJECT_TYPE override
On 26/03/2021 08:58, Owen Smith wrote:
________________________________________
From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> on behalf of Paul
Durrant <xadimgnik@xxxxxxxxx>
Sent: 26 March 2021 08:10
To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH] Add XENFILT_EMULATED_OBJECT_TYPE override
[CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments
unless you have verified the sender and know the content is safe.
On 15/03/2021 11:25, Owen Smith wrote:
EMULATED(IsDiskPresent) currently only reports if IDE disks are present.
Add the option to override the emulated type of any given PDO to allow
the emulated NVMe device to report its emulated device type as NVME.
Adds the override for the QEMU emulated NVMe controller
(PCI\VEN_8086&DEV_5845) to indicate it is a NVMe controller, and
exposes disk 0.
Also extends XENFILT_EMULATED_OBJECT_TYPE to include NVME
This is to fix an issue where XenVbd can incorrectly identify that disk
0 is not present, when disk 0 is infact the present emulated NVMe
device. This is problematic if the VM boots off the emulated device, not
the PV device (due to the missing unplug) and then rebinds XENVBD which
is unable to determine the presense of the emulated disk and attempts to
connect to an in-use ring.
Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
---
src/xenbus.inf | 1 +
src/xenfilt/driver.c | 24 +++++++++++++++---
src/xenfilt/driver.h | 7 ++++++
src/xenfilt/emulated.c | 40 ++++++++++++++++++++++++++++++
src/xenfilt/emulated.h | 3 ++-
src/xenfilt/pdo.c | 55 ++++++++++++++++++++++++++++++++++++++++++
6 files changed, 125 insertions(+), 5 deletions(-)
diff --git a/src/xenbus.inf b/src/xenbus.inf
index f9a5475..d2f2477 100644
--- a/src/xenbus.inf
+++ b/src/xenbus.inf
@@ -119,6 +119,7 @@ AddReg = XenFilt_Parameters
HKR,"Parameters",,0x00000010
HKR,"Parameters","*PNP0A03",0x00000000,"PCI"
HKR,"Parameters","Internal_IDE_Channel",0x00000000,"IDE"
+HKR,"Parameters\Override","PCI\VEN_8086&DEV_5845&SUBSYS_11001AF4&REV_02",0x00000000,"NVME"
That's a very strict match. How confident are we that the QEMU device
won't need to deal with e.g. multiple revisions of the QEMU device?
Perhaps it would be better to look at class code or is there something
else that may be a more stable identifier?
Paul
My intention here was to make the match as specific as possible given the
information immidiately available at the time the override is applied. By
keeping it specific, its reducing the chances of wrongly overriding a device.
Fair enough, but I wonder how much this ties us to specfic versions of QEMU.
Using device class code may catch devices that are passed through, or are not
otherwise the emulated device that corresponds to the PV device.
Actually shouldn't we be installing XENFILT on the controller? It's
going to the thing enumerating the disk PDOs and (if they're anything
like the real NVMe device in my laptop) they have a more specific id
string in the hardware ids. That may allow us to install the filter on
any device with a class code of 010802 (i.e. mass storage/non-volatile
memory/NVMe) and then spot the actual QEMU NVMe disks.
Paul
Owen
[Monitor_Service]
DisplayName=%MonitorName%
diff --git a/src/xenfilt/driver.c b/src/xenfilt/driver.c
index e9e6673..eb2ca42 100644
--- a/src/xenfilt/driver.c
+++ b/src/xenfilt/driver.c
@@ -723,6 +723,25 @@ fail1:
return status;
}
+XENFILT_EMULATED_OBJECT_TYPE
+DriverParseEmulatedType(
+ IN PANSI_STRING Ansi
+ )
+{
+ XENFILT_EMULATED_OBJECT_TYPE Type;
+
+ Type = XENFILT_EMULATED_OBJECT_TYPE_UNKNOWN;
+
+ 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;
+ else if (_strnicmp(Ansi->Buffer, "NVME", Ansi->Length) == 0)
+ Type = XENFILT_EMULATED_OBJECT_TYPE_NVME;
+
+ return Type;
+}
+
static XENFILT_EMULATED_OBJECT_TYPE
DriverGetEmulatedType(
IN PCHAR Id
@@ -753,10 +772,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..2129054 100644
--- a/src/xenfilt/driver.h
+++ b/src/xenfilt/driver.h
@@ -32,6 +32,8 @@
#ifndef _XENFILT_DRIVER_H
#define _XENFILT_DRIVER_H
+#include "emulated.h"
+
extern PDRIVER_OBJECT
DriverGetDriverObject(
VOID
@@ -58,6 +60,11 @@ DriverGetActive(
OUT PCHAR *Value
);
+extern XENFILT_EMULATED_OBJECT_TYPE
+DriverParseEmulatedType(
+ IN PANSI_STRING Ansi
+ );
+
typedef enum _XENFILT_FILTER_STATE {
XENFILT_FILTER_ENABLED = 0,
XENFILT_FILTER_PENDING,
diff --git a/src/xenfilt/emulated.c b/src/xenfilt/emulated.c
index 827c905..97da61b 100644
--- a/src/xenfilt/emulated.c
+++ b/src/xenfilt/emulated.c
@@ -87,6 +87,33 @@ __EmulatedFree(
__FreePoolWithTag(Buffer, XENFILT_EMULATED_TAG);
}
+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.Disk.Index = 0;
+
+ return STATUS_SUCCESS;
+
+fail1:
+ Error("fail1 (%08x)\n", status);
+
+ return status;
+}
+
static NTSTATUS
EmulatedSetObjectDeviceData(
IN PXENFILT_EMULATED_OBJECT EmulatedObject,
@@ -210,6 +237,13 @@ EmulatedAddObject(
goto fail1;
switch (Type) {
+ case XENFILT_EMULATED_OBJECT_TYPE_NVME:
+ status = EmulatedSetObjectNvmeData(*EmulatedObject,
+ Type,
+ DeviceID,
+ InstanceID);
+ break;
+
case XENFILT_EMULATED_OBJECT_TYPE_PCI:
status = EmulatedSetObjectDeviceData(*EmulatedObject,
Type,
@@ -338,6 +372,12 @@ EmulatedIsDiskPresent(
Trace("FOUND\n");
break;
}
+ if (EmulatedObject->Type == XENFILT_EMULATED_OBJECT_TYPE_NVME &&
+ Index == EmulatedObject->Data.Disk.Index) {
+ 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..41b0b10 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,58 @@ __PdoGetFdo(
return Pdo->Fdo;
}
+static VOID
+__PdoGetEmulatedTypeOverride(
+ IN PXENFILT_PDO Pdo,
+ IN PCHAR DeviceID
+ )
+{
+ HANDLE ParametersKey;
+ HANDLE OverrideKey;
+ ULONG ValueType;
+ PANSI_STRING Ansi;
+ XENFILT_EMULATED_OBJECT_TYPE Type;
+ NTSTATUS status;
+
+ ParametersKey = DriverGetParametersKey();
+
+ status = RegistryOpenSubKey(ParametersKey,
+ "Override",
+ GENERIC_READ,
+ &OverrideKey);
+ if (!NT_SUCCESS(status))
+ goto fail1;
+
+ status = RegistryQuerySzValue(OverrideKey,
+ DeviceID,
+ &ValueType,
+ &Ansi);
+ if (!NT_SUCCESS(status))
+ goto fail2;
+
+ if (ValueType != REG_SZ)
+ goto fail3;
+
+ Type = DriverParseEmulatedType(Ansi);
+ if (Type != XENFILT_EMULATED_OBJECT_TYPE_UNKNOWN)
+ Pdo->Type = Type;
+
+ RegistryFreeSzValue(Ansi);
+
+ RegistryCloseKey(OverrideKey);
+
+ return;
+
+fail3:
+ RegistryFreeSzValue(Ansi);
+
+fail2:
+ RegistryCloseKey(OverrideKey);
+
+fail1:
+ return;
+}
+
static NTSTATUS
PdoSetDeviceInformation(
IN PXENFILT_PDO Pdo
@@ -310,6 +363,8 @@ PdoSetDeviceInformation(
LocationInformation = NULL;
}
+ __PdoGetEmulatedTypeOverride(Pdo, DeviceID);
+
Dx->DeviceID = DeviceID;
Dx->InstanceID = InstanceID;
Dx->LocationInformation = LocationInformation;
|
