Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DMF_CONTEXT_GET and DMF_CONFIG_GET issue #42

Closed
laengand opened this issue Dec 14, 2018 · 11 comments
Closed

DMF_CONTEXT_GET and DMF_CONFIG_GET issue #42

laengand opened this issue Dec 14, 2018 · 11 comments

Comments

@laengand
Copy link

So I have written and declared 2 modules: Dmf_HostController and Dmf_UsbDevice

Each Module is based on the Dmf_Template.h/c

I noticed that whenever Dmf_UsbDevice called its DMF_CONTEXT_GET, it would call the DMF_CONTEXT_GET of the Dmf_HostController instead. If I remove the Dmf_HostController from the project the Dmf_UsbDevice calls the correct DMF_CONTEXT_GET.

I looked through DmfModule.h and found DMF_MODULE_DECLARE_CONTEXT. If I append the static keyword like this:

static __forceinline
DMF_CONTEXT_##ModuleName##* 
DMF_CONTEXT_GET(          ... 

Then Dmf_UsbDevice calls the correct DMF_CONTEXT_GET.
The same issue arises when calling DMF_CONFIG_GET.

So it seems the compiler inserts the wrong function when calling DMF_CONTEXT_GET and DMF_CONFIG_GET

Kind regards

@samtertzakian
Copy link
Member

Hello, Laengand,

Something is wrong. It looks to me like you are passing the wrong DMFMODULE handle to DMF_CONTEXT_GET().

Please note that when a Child Module callback happens, the callback always receives the DMFMODULE of the Child Module, not the Parent Module. This is the same as how WDF works. For example, when a WDFTIMER callback happens, it receives the WDFTIMER of the timer object.

So, the when a callback happens you need to use the following pattern:

VOID Callback(DMFMODULE ChildModule, ...)
{
     DMFMODULE ParentModule;
     DMF_CONTEXT_Parent* moduleContext;
     ParentModule = DMF_GetParentModule(ChildModule);
     moduleContext = DMF_CONTEXT_GET(ParentModule);
     …
}

I am assuming that perhaps you are not getting the Parent Module.

It is invalid to do the following:

VOID Callback(DMFMODULE ChildModule, ...)
{
     DMFMODULE ParentModule;
     DMF_CONTEXT_Parent* moduleContext;
     moduleContext = DMF_CONTEXT_GET(ChildModule);
     …
}

DMF_CONTEXT_GET() is always local and private to the Module. That is why it is declared only in the Module's .c.

Please let me know if that is the cause of your problem. If not, I am happy to look at your code if you want to send me a function where this is happening.

You can also look investigate the DMFMODULE by doing this in the debugger:

!wdfkd.load
!wdfhandle
then look at the associated buffer and do
dt DMF_OBJECT

There you can look at the Module handle data and see the name of the Module (ModuleInstanceName).

@laengand laengand reopened this Dec 17, 2018
@laengand
Copy link
Author

Hi
The thing is that I don't have child modules. The Dmf_HostController and Dmf_UsbDevice are completely separate (at least for now).
I've attached a video of the issue and source files for the modules. There's a lot of commented code at the moment.

The video shows the Open function of Dmf_UsbDevice. which is called by its Create function. Stepping into DMF_CONTEXT_GET reveals that the HostController function is called.

2018-12-17_10h22_54_Trim.zip

@samtertzakian
Copy link
Member

Hi, laengand,
We are looking at your package.

While looking, I would like to ask a question:

Is there a reason you are creating Dmf_UsbDevice as a Dynamic Module? I think a better approach is that Dmf_UsbDevice is set to DMF_MODULE_OPEN_OPTION_OPEN_PrepareHardware instead of DMF_MODULE_OPEN_OPTION_OPEN_Create. Then, in Dmf_HostController's Create function set ChildModulesAdd() callback. Then, it that callback make Dmf_UsbDevice a Child Module of Dmf_HostController. This way both Modules can support all the WDF callbacks. In this case, you do not call Dmf_UsbDevice_Create() in Dmf_HostController_Open(). All of that will happen automatically and the binding of the Parent/Child Modules is strong.

If you do that, I am sure it will resolve your issue. For an example, please see Dmf_ToasterBus.c to see how it creates Child Modules. See its DMF_ToasterBus_Create() and DMF_ToasterBus_ChildModulesAdd().

Having said that, we are going to investigate the strange behavior your a seeing and get back to you.

@laengand
Copy link
Author

Hi
I'll try and elaborate a bit on what i'm trying to achieve with the dynamic modules. The Dmf_UsbDevice is going to be an emulated usb device, by using the UDE. The Dmf_HostController is going to be able to "attach" one or more of these emulated usb devices on demand. So that's why i thought of using dynamic modules.

I agree that it would make sense to let the Dmf_HostController have child modules in the form of Dmf_UsbDevice, but is it possible to have child modules that are dynamic?

I'll take a look at the toasterbus example.

@samtertzakian
Copy link
Member

samtertzakian commented Dec 19, 2018

Hi, laengand,
I saw your video and see that there is an issue. I took your files and I see that the way you are including DMF is not quite correct. I updated the files to include DMF correctly. Then, I instantiated your Dmf_HostController Module and its Dmf_UsbDevice. When I do so, the handle is correct and everything is fine. DMF_MODULE_CONTEXT is correct. You can see it here:

3: kd> !wdfkd.load
3: kd> !wdfhandle 0x000050f4`ba4103a8
Treating handle as a KMDF handle!

Dumping WDFHANDLE 0x000050f4ba4103a8
=============================
Handle type is WDFMEMORY [DMFMODULE_TYPE]
Refcount: 1
Contexts:
    <typeless context>
    EvtCleanupCallback fffff803c22bab00 osrusbfx2dmf4!DmfEvtDynamicModuleCleanupCallback

    context:  dt 0xffffaf0b4bd55bb0 osrusbfx2dmf4!DMF_CONTEXT_UsbDevice (size is 0x40 bytes)
    <no associated attribute callbacks>

    context:  dt 0xffffaf0b44e0e8f0 osrusbfx2dmf4!WdfCustomType_DMFMODULE_TYPE (size is 0x10 bytes)
    <no associated attribute callbacks>

Parent: !wdfhandle 0x000050f4b4b64948, type is WDFDRIVER

!wdfobject 0xffffaf0b45befc50
3: kd> dt 0xffffaf0b4bd55bb0 osrusbfx2dmf4!DMF_CONTEXT_UsbDevice
   +0x000 WdfHostController : (null) 
   +0x008 UdeUsbDeviceInit : (null) 
   +0x010 UdeUsbDevice     : (null) 
   +0x018 ControlEndpoint  : (null) 
   +0x020 BulkOutEndpoint  : (null) 
   +0x028 BulkInEndpoint   : (null) 
   +0x030 InterruptInEndpoint : (null) 
   +0x038 IsAwake          : 0 ''
3: kd> p

00 ffffd58a`89416640 fffff803`c22c92c0 osrusbfx2dmf4!DMF_UsbDevice_Open+0xee [c:\users\satertza\source\repos\dmf\dmf\modules.template\dmf_usbdevice.c @ 1046] 
01 ffffd58a`89416700 fffff803`c22b9ac6 osrusbfx2dmf4!DMF_Internal_Open+0x170 [c:\users\satertza\source\repos\dmf\dmf\framework\dmfinternal.c @ 1562] 
02 ffffd58a`89416780 fffff803`c22f0c73 osrusbfx2dmf4!DMF_Module_OpenOrRegisterNotificationOnCreate+0xa6 [c:\users\satertza\source\repos\dmf\dmf\framework\dmfcall.c @ 2470] 
03 ffffd58a`894167d0 fffff803`c22feec8 osrusbfx2dmf4!DMF_ModuleCreate+0x33d3 [c:\users\satertza\source\repos\dmf\dmf\framework\dmfcore.c @ 1174] 
04 ffffd58a`89416ba0 fffff803`c22fd96a osrusbfx2dmf4!DMF_UsbDevice_Create+0x2e8 [c:\users\satertza\source\repos\dmf\dmf\modules.template\dmf_usbdevice.c @ 1203] 
05 ffffd58a`89416c40 fffff803`c22c92c0 osrusbfx2dmf4!DMF_HostController_Open+0x13a [c:\users\satertza\source\repos\dmf\dmf\modules.template\dmf_hostcontroller.c @ 1124] 
06 ffffd58a`89416d60 fffff803`c22ebb4b osrusbfx2dmf4!DMF_Internal_Open+0x170 [c:\users\satertza\source\repos\dmf\dmf\framework\dmfinternal.c @ 1562] 
07 ffffd58a`89416de0 fffff803`c22b9c71 osrusbfx2dmf4!DMF_Generic_ModulePrepareHardware+0x19b [c:\users\satertza\source\repos\dmf\dmf\framework\dmfgeneric.c @ 297] 
08 ffffd58a`89416e70 fffff803`c22dcf43 osrusbfx2dmf4!DMF_Module_PrepareHardware+0xe1 [c:\users\satertza\source\repos\dmf\dmf\framework\dmfcall.c @ 548] 
09 ffffd58a`89416ed0 fffff803`c22e2176 osrusbfx2dmf4!DMF_ModuleCollectionPrepareHardware+0x223 [c:\users\satertza\source\repos\dmf\dmf\framework\dmfmodulecollection.c @ 750] 
0a ffffd58a`89416f60 fffff80e`4d9e4a08 osrusbfx2dmf4!DmfContainerEvtDevicePrepareHardware+0xf6 [c:\users\satertza\source\repos\dmf\dmf\framework\dmfcontainer.c @ 109] 

The only changes I made were to how the include DMF is included. It is not clear to me yet exactly what is wrong about how you included but I will try to track that down. For now, I have attached the corrected files for you here.

Note that you are not supposed to include Dmf_UsbDevice.h in the Parent Module. Instead, it is supposed to be included in the Module Library include file. In this case, it is DmfModules.Template.h.

This is how your includes should look (All Module should include files in this way)
Modules.Template.zip
:
Dmf_HostController.c

#include "DmfModule.h"
#include "DmfModules.Template.h"
#include "DmfModules.Template.Trace.h"

#include "Dmf_HostController.tmh"

///////////////////////////////////////////////////////////////////////////////////////////////////////
// Module Private Enumerations and Structures
///////////////////////////////////////////////////////////////////////////////////////////////////////
//
#include <ude/1.0/UdeCx.h>

Dmf_UsbDevice.c

#include "DmfModule.h"
#include "DmfModules.Template.h"
#include "DmfModules.Template.Trace.h"

#include "Dmf_UsbDevice.tmh"

///////////////////////////////////////////////////////////////////////////////////////////////////////
// Module Private Enumerations and Structures
///////////////////////////////////////////////////////////////////////////////////////////////////////
//

#include <ude/1.0/UdeCx.h>

(You can replace "Template" with "Library". I added them to the Template library for the test.) Please do a diff between your files and my files.

It looks to me like when you include Dmf_UsbDevice.h inside of Dmf_HostController.h it causes the issue you are seeing because the compiler makes DMF_UsbDevice_Create() inline. We will try to resolve the issue by making macros DMF_CONTEXT_GET() and DMF_CONFIG_GET() as static to prevent this possibility.

@samtertzakian
Copy link
Member

Hi, laendand,

Please see the files in this .zip file (and see the my previous comment):
Modules.Template.zip

This is the correct way to add Modules to a Module Library. In the Client driver you are only supposed to include the Module Library's Module Include file. In my case, it is DmfModule.Template.h. Note how I have modified Dmf_Template.h and Dmf_Template_Public.h.

That is the only place that Dmf_HostController.h and Dmf_UsbDevice.h should be included. (A single time for both Modules and Client drivers.)

In the Client driver you only need to include the following:

#include <DmfModules.Template.h>

In the Modules, you only should include the following:

#include "DmfModule.h"
#include "DmfModules.Template.h"
#include "DmfModules.Template.Trace.h"

#include "Dmf_HostController.tmh"

(Note: Do not include Trace.h.)

That will fix your issue in the correct way. Having said that, we will at a later time, update the macros DMF_CONTEXT_GET() and DMF_CONFIG_GET() to remove make them static.

One more thing: Note that Modules are agnositic about their parents. It is generally not correct that you DMF_UsbDevice's Parent is DMF_HostController. It seems like you are doing that.

Let me know if you have questions.

@samtertzakian
Copy link
Member

Hi, laengand,

I have spent a lot of time looking at this issue and I am unable to repro it exactly.

One thing I want to tell you is:

You are not supposed to include the Module's .h files directly in your driver. The Module's .h file is only supposed to be included in a single place: The Module Library's Include file. Then, your driver is only supposed to include the Module Library's Include file.

So, in my example, I added your Module to Template Library. Then, in the Template's Include file I added your Module include files. Then, in your driver you are only supposed to include Modules.Template.h. Then, I link to Template Library. Note that you are not supposed to add Module directly to your driver. You add them to the Library so that all the dependencies exist (both compile and link) in the Library. Then, you link your driver to that Library.

When I do not include the files as I state above, I see some similar behavior (but not exacty) to what you describe.

I also added "static" as you did and I did not see a difference.

@samtertzakian
Copy link
Member

Hi, laengand,

No matter what I do, I am unable to repro this issue. It is my belief that you are including the Modules directly into your driver which is not correct. I tried doing that also to repro your issue, but I was still unable to do so.

I am going to close this issue with out a fix because I don't want to make a change for the wrong reason.

If you can work with me to help me repro the issue (even if I can see it remotely), I am happy to do so.
Please contact me if you are interested pursuing it further.

To be clear, Modules should be added to a Module Library. Then, the Client driver includes that Library's path in the #include directory and that Module's Library in the Link directory. The include files of every Module should be the same in each Library.

Thank your for brining this issue to our attention. Please contact us again or open this issue again if you wish pursue it further.

@laengand
Copy link
Author

Hi again
Thank you for looking into my issue. I'm currently on holiday. I'll look into your suggestions when I get back to work on the Jan. 2.

@laengand
Copy link
Author

laengand commented Jan 2, 2019

Hi samtertzakian
I followed your suggestions and it seems to work now.
Heres what i did:

  1. Create a static library project for the module library.
  2. In each module I include the following:
    #include "DmfModule.h"
    #include "DmfModules.Ude.h"
    #include "DmfModules.Ude.Trace.h"
    #include "Dmf_<Modulename>.tmh"
    Where Ude is the name of the library
  3. Include the modules in this library
  4. In my driver project I link to the Ude library instead of including the modules directly

Doing this removed the need for using the static DMF_CONTEXT_GET and DMF_CONFIG_GET
So I guess that my issue was as you stated : "It is my belief that you are including the Modules directly into your driver which is not correct. "

Thank you for your assistance

@samtertzakian
Copy link
Member

Hi laendand,

It is great news that you were able to make things work. Thank you for taking the time to reorganize your code.

The reason that Client drivers should link to Libraries is that each Module's dependencies (on other Modules) is already encoded in the Library. Regardless of how the a Module uses other Modules (or other Modules use it), when you link to a Module in a Library, you automatically get all its dependencies (even if they change in the future).

The include files assume programmers will follow that model. The Include files are designed to hide DMF APIs used only by Modules from Client drivers. So, it is best to include the DMF include files just like the samples do. 100% of our drivers do the same.

I am planning on adding a section in the documentation to address this issue (Client drivers to should link to Libraries not Modules) and other similar issues.

Thank you again for reporting this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants