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

Re: [RFC PATCH 1/2] Add ETW support



ARRAYSIZE vs sizeof - looks like I missed something here.
EtwExit vs EtwExit2 - specifically to reduce having to insert EtwExit(.. STATUS_SUCCESS) in functions that do not return a NTSTATUS

Owen 

On Thu, Feb 1, 2024 at 5:41 PM Paul Durrant <xadimgnik@xxxxxxxxx> wrote:
On 12/01/2024 12:59, Owen Smith wrote:
> Defines ETW schema with EtwEnter and EtwExit events, which can be used to track
> code-paths and produce flamegraphs of executions.
> Only uses EtwEnter and EtwExit in the driver.c file of XenBus, but more events
> will be added in subsequent patches.
>
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxx>
> ---
>   src/common/dbg_print.h       | 60 +++++++++++++++++++++++++++++++++++
>   src/xenbus/driver.c          | 11 +++++++
>   src/xenbus/xenbus_etw.xml    | 61 ++++++++++++++++++++++++++++++++++++
>   vs2019/xenbus/xenbus.vcxproj | 13 +++++++-
>   vs2022/xenbus/xenbus.vcxproj | 13 +++++++-
>   5 files changed, 156 insertions(+), 2 deletions(-)
>   create mode 100644 src/xenbus/xenbus_etw.xml
>
> diff --git a/src/common/dbg_print.h b/src/common/dbg_print.h
> index a0bd727..98c22cf 100644
> --- a/src/common/dbg_print.h
> +++ b/src/common/dbg_print.h
> @@ -42,6 +42,66 @@
>   
>   #pragma warning(disable:4127)   // conditional _expression_ is constant
>   
> +#ifdef ETW_HEADER
> +#include ETW_HEADER
> +
> +static __inline VOID
> +__EtwEnter(
> +    IN  const CHAR      *Module,
> +    IN  const CHAR      *Function,
> +    IN  ULONG           Line
> +    )
> +{
> +    CHAR                _Module[16];
> +    CHAR                _Function[32];
> +
> +    if (!EventEnabledEvtEnter())
> +        return;
> +
> +    RtlZeroMemory(_Module, sizeof(_Module));
> +    RtlZeroMemory(_Function, sizeof(_Function));
> +
> +    strncpy(_Module, Module, ARRAYSIZE(_Module) -1);

Missing space. I can fix.

> +    strncpy(_Function, Function, ARRAYSIZE(_Function) - 1);
> +
> +    EventWriteEvtEnter(NULL, _Module, _Function, Line);
> +}
> +
> +static __inline VOID
> +__EtwExit(
> +    IN  const CHAR      *Module,
> +    IN  const CHAR      *Function,
> +    IN  ULONG           Line,
> +    IN  NTSTATUS        Status
> +    )
> +{
> +    CHAR                _Module[16];
> +    CHAR                _Function[32];
> +
> +    if (!EventEnabledEvtExit())
> +        return;
> +
> +    RtlZeroMemory(_Module, sizeof(_Module));
> +    RtlZeroMemory(_Function, sizeof(_Function));
> +
> +    strncpy(_Module, Module, sizeof(_Module));

Why sizeof() here but ARRAYSIZE above?

> +    strncpy(_Function, Function, sizeof(_Function));
> +
> +    EventWriteEvtExit(NULL, _Module, _Function, Line, (ULONG)Status);
> +}
> +
> +#define EtwEnter()          __EtwEnter(__MODULE__, __FUNCTION__, __LINE__)
> +#define EtwExit()           __EtwExit(__MODULE__, __FUNCTION__, __LINE__, STATUS_SUCCESS)
> +#define EtwExit2(status)    __EtwExit(__MODULE__, __FUNCTION__, __LINE__, status)

Why this rather than requiring an explicit status for EtwExit()?

   Paul

> +
> +#else
> +
> +#define EtwEnter()          (VOID)0
> +#define EtwExit()           (VOID)0
> +#define EtwExit2(status)    (VOID)status
> +
> +#endif
> +
>   static __inline VOID
>   __Error(
>       IN  const CHAR  *Prefix,
> diff --git a/src/xenbus/driver.c b/src/xenbus/driver.c
> index 522acef..4443559 100644
> --- a/src/xenbus/driver.c
> +++ b/src/xenbus/driver.c
> @@ -708,6 +708,8 @@ DriverUnload(
>   
>       __DriverSetDriverObject(NULL);
>   
> +    EventUnregisterXenBus_Driver();
> +
>       ASSERT(IsZeroMemory(&Driver, sizeof (XENBUS_DRIVER)));
>   
>       Trace("<====\n");
> @@ -726,6 +728,7 @@ DriverAddDevice(
>   
>       ASSERT3P(DriverObject, ==, __DriverGetDriverObject());
>   
> +    EtwEnter();
>       Trace("====>\n");
>   
>       __DriverAcquireMutex();
> @@ -737,12 +740,14 @@ DriverAddDevice(
>       __DriverReleaseMutex();
>   
>       Trace("<====\n");
> +    EtwExit();
>   
>       return STATUS_SUCCESS;
>   
>   fail1:
>       __DriverReleaseMutex();
>   
> +    EtwExit2(status);
>       return status;
>   }
>   
> @@ -757,6 +762,8 @@ DriverDispatch(
>       PXENBUS_DX          Dx;
>       NTSTATUS            status;
>   
> +    EtwEnter();
> +
>       Dx = (PXENBUS_DX)DeviceObject->DeviceExtension;
>       ASSERT3P(Dx->DeviceObject, ==, DeviceObject);
>   
> @@ -800,6 +807,7 @@ DriverDispatch(
>       }
>   
>   done:
> +    EtwExit2(status);
>       return status;
>   }
>   
> @@ -819,6 +827,7 @@ DriverEntry(
>   
>       ASSERT3P(__DriverGetDriverObject(), ==, NULL);
>   
> +    EventRegisterXenBus_Driver();
>       ExInitializeDriverRuntime(DrvRtPoolNxOptIn);
>       WdmlibProcgrpInitialize();
>   
> @@ -915,6 +924,8 @@ fail1:
>   
>       __DriverSetDriverObject(NULL);
>   
> +    EventUnregisterXenBus_Driver();
> +
>       ASSERT(IsZeroMemory(&Driver, sizeof (XENBUS_DRIVER)));
>   
>       return status;
> diff --git a/src/xenbus/xenbus_etw.xml b/src/xenbus/xenbus_etw.xml
> new file mode 100644
> index 0000000..c73dfe6
> --- /dev/null
> +++ b/src/xenbus/xenbus_etw.xml
> @@ -0,0 +1,61 @@
> +<?xml version='1.0' encoding='utf-8' standalone='yes'?>
> +<instrumentationManifest
> +    xmlns="http://schemas.microsoft.com/win/2004/08/events"
> +    xmlns:win="http://manifests.microsoft.com/win/2004/08/windows/events"
> +    xmlns:xs="http://www.w3.org/2001/XMLSchema"
> +    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
> +    xsi:schemaLocation="http://schemas.microsoft.com/win/2004/08/events eventman.xsd" >
> +  <instrumentation>
> +    <events>
> +      <provider
> +          guid="{e580595b-a8a6-41b7-a9c1-1954d2138ffc}"
> +          messageFileName="%SystemDrive%\windows\system32\drivers\xenbus.sys"
> +          name="XenBus_Driver"
> +          resourceFileName="%SystemDrive%\windows\system32\drivers\xenbus.sys"
> +          symbol="DriverControlGuid" >
> +        <channels>
> +          <channel chid="XENBUS" name="XenBus" type="Analytic" enabled="true" />
> +        </channels>
> +        <templates>
> +          <template tid="tid_enter">
> +            <data name="Module" inType="win:Int8" count="16" outType="xs:string" />     <!-- 16 chars -->
> +            <data name="Function" inType="win:Int8" count="32" outType="xs:string" />   <!-- 32 chars -->
> +            <data name="Line" inType="win:UInt32" outType="xs:unsignedInt" />           <!-- line number -->
> +          </template>
> +          <template tid="tid_exit">
> +            <data name="Module" inType="win:Int8" count="16" outType="xs:string" />     <!-- 16 chars -->
> +            <data name="Function" inType="win:Int8" count="32" outType="xs:string" />   <!-- 32 chars -->
> +            <data name="Line" inType="win:UInt32" outType="xs:unsignedInt" />           <!-- line number -->
> +            <data name="Status" inType="win:UInt32" outType="xs:unsignedInt" />         <!-- NTSTATUS -->
> +          </template>
> +        </templates>
> +        <events>
> +          <event
> +            channel="XENBUS"
> +            level="win:Informational"
> +            message="$(string.EventTraceInfo.Enter)"
> +            opcode="win:Info"
> +            symbol="EvtEnter"
> +            template="tid_enter"
> +            value="10" />
> +          <event
> +            channel="XENBUS"
> +            level="win:Informational"
> +            message="$(string.EventTraceInfo.Exit)"
> +            opcode="win:Info"
> +            symbol="EvtExit"
> +            template="tid_exit"
> +            value="11" />
> +        </events>
> +      </provider>
> +    </events>
> +  </instrumentation>
> +  <localization xmlns="http://schemas.microsoft.com/win/2004/08/events">
> +    <resources culture="en-US">
> +      <stringTable>
> +        <string id="EventTraceInfo.Enter" value="[Enter]" />
> +        <string id="EventTraceInfo.Exit"  value="[Exit ]" />
> +      </stringTable>
> +    </resources>
> +  </localization>
> +</instrumentationManifest>
> diff --git a/vs2019/xenbus/xenbus.vcxproj b/vs2019/xenbus/xenbus.vcxproj
> index aa88980..e191527 100644
> --- a/vs2019/xenbus/xenbus.vcxproj
> +++ b/vs2019/xenbus/xenbus.vcxproj
> @@ -20,7 +20,7 @@
>     <ItemDefinitionGroup>
>       <ClCompile>
>         <AdditionalOptions>/ZH:SHA_256 %(AdditionalOptions)</AdditionalOptions>
> -      <PreprocessorDefinitions>PROJECT=$(ProjectName);POOL_NX_OPTIN=1;NT_PROCESSOR_GROUPS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
> +      <PreprocessorDefinitions>PROJECT=$(ProjectName);POOL_NX_OPTIN=1;NT_PROCESSOR_GROUPS;ETW_HEADER="xenbus_etw.h";%(PreprocessorDefinitions)</PreprocessorDefinitions>
>         <IntrinsicFunctions>true</IntrinsicFunctions>
>         <AdditionalIncludeDirectories>$(WindowsSdkDir)\include\km;..\..\include;..\..\include\xen;..\..\src\common;</AdditionalIncludeDirectories>
>         <WarningLevel>EnableAllWarnings</WarningLevel>
> @@ -38,6 +38,13 @@
>         <MapExports>true</MapExports>
>         <AdditionalOptions>/INTEGRITYCHECK %(AdditionalOptions)</AdditionalOptions>
>       </Link>
> +    <MessageCompile>
> +      <HeaderFilePath>..\..\include</HeaderFilePath>
> +      <GeneratedFilesBaseName>%(Filename)</GeneratedFilesBaseName>
> +      <RCFilePath>..\..\src\xenbus</RCFilePath>
> +      <GenerateKernelModeLoggingMacros>true</GenerateKernelModeLoggingMacros>
> +      <UseBaseNameOfInput>true</UseBaseNameOfInput>
> +    </MessageCompile>
>       <DriverSign>
>         <FileDigestAlgorithm>sha256</FileDigestAlgorithm>
>       </DriverSign>
> @@ -68,6 +75,7 @@
>     <ItemGroup>
>       <FilesToPackage Include="$(TargetPath)" />
>       <FilesToPackage Include="$(OutDir)$(TargetName).pdb" />
> +    <FilesToPackage Include="..\..\src\xenbus\xenbus_etw.xml" />
>     </ItemGroup>
>     <ItemGroup>
>       <ClCompile Include="..\..\src\common\registry.c" />
> @@ -97,5 +105,8 @@
>     <ItemGroup>
>       <ResourceCompile Include="..\..\src\xenbus\xenbus.rc" />
>     </ItemGroup>
> +  <ItemGroup>
> +    <MessageCompile Include="..\..\src\xenbus\xenbus_etw.xml" />
> +  </ItemGroup>
>     <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
>   </Project>
> diff --git a/vs2022/xenbus/xenbus.vcxproj b/vs2022/xenbus/xenbus.vcxproj
> index ce0526f..1322d15 100644
> --- a/vs2022/xenbus/xenbus.vcxproj
> +++ b/vs2022/xenbus/xenbus.vcxproj
> @@ -20,7 +20,7 @@
>     <ItemDefinitionGroup>
>       <ClCompile>
>         <AdditionalOptions>/ZH:SHA_256 %(AdditionalOptions)</AdditionalOptions>
> -      <PreprocessorDefinitions>PROJECT=$(ProjectName);POOL_NX_OPTIN=1;NT_PROCESSOR_GROUPS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
> +      <PreprocessorDefinitions>PROJECT=$(ProjectName);POOL_NX_OPTIN=1;NT_PROCESSOR_GROUPS;ETW_HEADER="xenbus_etw.h";%(PreprocessorDefinitions)</PreprocessorDefinitions>
>         <IntrinsicFunctions>true</IntrinsicFunctions>
>         <AdditionalIncludeDirectories>$(WindowsSdkDir)\include\km;..\..\include;..\..\include\xen;..\..\src\common;</AdditionalIncludeDirectories>
>         <WarningLevel>EnableAllWarnings</WarningLevel>
> @@ -38,6 +38,13 @@
>         <MapExports>true</MapExports>
>         <AdditionalOptions>/INTEGRITYCHECK %(AdditionalOptions)</AdditionalOptions>
>       </Link>
> +    <MessageCompile>
> +      <HeaderFilePath>..\..\include</HeaderFilePath>
> +      <GeneratedFilesBaseName>%(Filename)</GeneratedFilesBaseName>
> +      <RCFilePath>..\..\src\xenbus</RCFilePath>
> +      <GenerateKernelModeLoggingMacros>true</GenerateKernelModeLoggingMacros>
> +      <UseBaseNameOfInput>true</UseBaseNameOfInput>
> +    </MessageCompile>
>       <DriverSign>
>         <FileDigestAlgorithm>sha256</FileDigestAlgorithm>
>       </DriverSign>
> @@ -60,6 +67,7 @@
>     <ItemGroup>
>       <FilesToPackage Include="$(TargetPath)" />
>       <FilesToPackage Include="$(OutDir)$(TargetName).pdb" />
> +    <FilesToPackage Include="..\..\src\xenbus\xenbus_etw.xml" />
>     </ItemGroup>
>     <ItemGroup>
>       <ClCompile Include="..\..\src\common\registry.c" />
> @@ -89,5 +97,8 @@
>     <ItemGroup>
>       <ResourceCompile Include="..\..\src\xenbus\xenbus.rc" />
>     </ItemGroup>
> +  <ItemGroup>
> +    <MessageCompile Include="..\..\src\xenbus\xenbus_etw.xml" />
> +  </ItemGroup>
>     <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
>   </Project>



 


Rackspace

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