QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] WHPX: refactor load library
@ 2019-11-08 20:31 Sunil Muthuswamy
  2019-11-12 18:42 ` Sunil Muthuswamy
  2019-11-13 14:59 ` Paolo Bonzini
  0 siblings, 2 replies; 9+ messages in thread
From: Sunil Muthuswamy @ 2019-11-08 20:31 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Stefan Weil
  Cc: qemu-devel, Justin Terry (VM)

This refactors the load library of WHV libraries to make it more
modular. It makes a helper routine that can be called on demand.
This allows future expansion of load library/functions to support
functionality that is depenedent on some feature being available.

Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
---
 target/i386/whp-dispatch.h |  4 +++
 target/i386/whpx-all.c     | 84 +++++++++++++++++++++++++++++++---------------
 2 files changed, 61 insertions(+), 27 deletions(-)

diff --git a/target/i386/whp-dispatch.h b/target/i386/whp-dispatch.h
index 23791fbb47..87d049ceab 100644
--- a/target/i386/whp-dispatch.h
+++ b/target/i386/whp-dispatch.h
@@ -50,5 +50,9 @@ extern struct WHPDispatch whp_dispatch;
 
 bool init_whp_dispatch(void);
 
+typedef enum WHPFunctionList {
+    WINHV_PLATFORM_FNS_DEFAULT,
+    WINHV_EMULATION_FNS_DEFAULT,
+} WHPFunctionList;
 
 #endif /* WHP_DISPATCH_H */
diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c
index ed95105eae..4688f40a65 100644
--- a/target/i386/whpx-all.c
+++ b/target/i386/whpx-all.c
@@ -1356,6 +1356,57 @@ static void whpx_handle_interrupt(CPUState *cpu, int mask)
     }
 }
 
+/*
+ * Load the functions from the given library, using the given handle. If a
+ * handle is provided, it is used, otherwise the library is opened. The
+ * handle will be updated on return with the opened one.
+ */
+static bool load_whp_dipatch_fns(HMODULE *handle, WHPFunctionList function_list)
+{
+    HMODULE hLib = *handle;
+
+    #define WINHV_PLATFORM_DLL "WinHvPlatform.dll"
+    #define WINHV_EMULATION_DLL "WinHvEmulation.dll"
+    #define WHP_LOAD_FIELD(return_type, function_name, signature) \
+        whp_dispatch.function_name = \
+            (function_name ## _t)GetProcAddress(hLib, #function_name); \
+        if (!whp_dispatch.function_name) { \
+            error_report("Could not load function %s", #function_name); \
+            goto error; \
+        } \
+
+    #define WHP_LOAD_LIB(lib_name, handle_lib) \
+    if (!handle_lib) { \
+        handle_lib = LoadLibrary(lib_name); \
+        if (!handle_lib) { \
+            error_report("Could not load library %s.", lib_name); \
+            goto error; \
+        } \
+    } \
+
+    switch (function_list) {
+    case WINHV_PLATFORM_FNS_DEFAULT:
+        WHP_LOAD_LIB(WINHV_PLATFORM_DLL, hLib)
+        LIST_WINHVPLATFORM_FUNCTIONS(WHP_LOAD_FIELD)
+        break;
+
+    case WINHV_EMULATION_FNS_DEFAULT:
+        WHP_LOAD_LIB(WINHV_EMULATION_DLL, hLib)
+        LIST_WINHVEMULATION_FUNCTIONS(WHP_LOAD_FIELD)
+        break;
+    }
+
+    *handle = hLib;
+    return true;
+
+error:
+    if (hLib) {
+        FreeLibrary(hWinHvEmulation);
+    }
+
+    return false;
+}
+
 /*
  * Partition support
  */
@@ -1491,51 +1542,30 @@ static void whpx_type_init(void)
 
 bool init_whp_dispatch(void)
 {
-    const char *lib_name;
-    HMODULE hLib;
-
     if (whp_dispatch_initialized) {
         return true;
     }
 
-    #define WHP_LOAD_FIELD(return_type, function_name, signature) \
-        whp_dispatch.function_name = \
-            (function_name ## _t)GetProcAddress(hLib, #function_name); \
-        if (!whp_dispatch.function_name) { \
-            error_report("Could not load function %s from library %s.", \
-                         #function_name, lib_name); \
-            goto error; \
-        } \
-
-    lib_name = "WinHvPlatform.dll";
-    hWinHvPlatform = LoadLibrary(lib_name);
-    if (!hWinHvPlatform) {
-        error_report("Could not load library %s.", lib_name);
+    if (!load_whp_dipatch_fns(&hWinHvPlatform, WINHV_PLATFORM_FNS_DEFAULT)) {
         goto error;
     }
-    hLib = hWinHvPlatform;
-    LIST_WINHVPLATFORM_FUNCTIONS(WHP_LOAD_FIELD)
 
-    lib_name = "WinHvEmulation.dll";
-    hWinHvEmulation = LoadLibrary(lib_name);
-    if (!hWinHvEmulation) {
-        error_report("Could not load library %s.", lib_name);
+    if (!load_whp_dipatch_fns(&hWinHvEmulation, WINHV_EMULATION_FNS_DEFAULT)) {
         goto error;
     }
-    hLib = hWinHvEmulation;
-    LIST_WINHVEMULATION_FUNCTIONS(WHP_LOAD_FIELD)
 
     whp_dispatch_initialized = true;
-    return true;
-
-    error:
 
+    return true;
+error:
     if (hWinHvPlatform) {
         FreeLibrary(hWinHvPlatform);
     }
+
     if (hWinHvEmulation) {
         FreeLibrary(hWinHvEmulation);
     }
+
     return false;
 }
 
-- 
2.16.4



^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] WHPX: refactor load library
  2019-11-08 20:31 [PATCH] WHPX: refactor load library Sunil Muthuswamy
@ 2019-11-12 18:42 ` Sunil Muthuswamy
  2019-11-12 19:47   ` Eduardo Habkost
  2019-11-13 14:59 ` Paolo Bonzini
  1 sibling, 1 reply; 9+ messages in thread
From: Sunil Muthuswamy @ 2019-11-12 18:42 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Stefan Weil
  Cc: qemu-devel, Justin Terry (VM)



> -----Original Message-----
> From: Sunil Muthuswamy
> Sent: Friday, November 8, 2019 12:32 PM
> To: 'Paolo Bonzini' <pbonzini@redhat.com>; 'Richard Henderson' <rth@twiddle.net>; 'Eduardo Habkost' <ehabkost@redhat.com>; 'Stefan
> Weil' <sw@weilnetz.de>
> Cc: 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>; Justin Terry (VM) <juterry@microsoft.com>
> Subject: [PATCH] WHPX: refactor load library
> 
> This refactors the load library of WHV libraries to make it more
> modular. It makes a helper routine that can be called on demand.
> This allows future expansion of load library/functions to support
> functionality that is depenedent on some feature being available.
> 
> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> ---

Can I possibly get some eyes on this?



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] WHPX: refactor load library
  2019-11-12 18:42 ` Sunil Muthuswamy
@ 2019-11-12 19:47   ` Eduardo Habkost
  2019-11-13 16:35     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 9+ messages in thread
From: Eduardo Habkost @ 2019-11-12 19:47 UTC (permalink / raw)
  To: Sunil Muthuswamy
  Cc: Paolo Bonzini, Stefan Weil, Justin Terry \(VM\),
	qemu-devel, Richard Henderson

On Tue, Nov 12, 2019 at 06:42:00PM +0000, Sunil Muthuswamy wrote:
> 
> 
> > -----Original Message-----
> > From: Sunil Muthuswamy
> > Sent: Friday, November 8, 2019 12:32 PM
> > To: 'Paolo Bonzini' <pbonzini@redhat.com>; 'Richard Henderson' <rth@twiddle.net>; 'Eduardo Habkost' <ehabkost@redhat.com>; 'Stefan
> > Weil' <sw@weilnetz.de>
> > Cc: 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>; Justin Terry (VM) <juterry@microsoft.com>
> > Subject: [PATCH] WHPX: refactor load library
> > 
> > This refactors the load library of WHV libraries to make it more
> > modular. It makes a helper routine that can be called on demand.
> > This allows future expansion of load library/functions to support
> > functionality that is depenedent on some feature being available.
> > 
> > Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> > ---
> 
> Can I possibly get some eyes on this?

I'd be glad to queue the patch if we get a Reviewed-by line from
somebody who understands Windows and WHPX.  Maybe Justin?

Sunil, Justin, would you like to be listed as maintainers or
designated reviewers for the WHPX code in QEMU?

-- 
Eduardo



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] WHPX: refactor load library
  2019-11-08 20:31 [PATCH] WHPX: refactor load library Sunil Muthuswamy
  2019-11-12 18:42 ` Sunil Muthuswamy
@ 2019-11-13 14:59 ` Paolo Bonzini
  2019-11-13 16:59   ` Sunil Muthuswamy
  1 sibling, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2019-11-13 14:59 UTC (permalink / raw)
  To: Sunil Muthuswamy, Richard Henderson, Eduardo Habkost, Stefan Weil
  Cc: qemu-devel, Justin Terry \(VM\)

On 08/11/19 21:31, Sunil Muthuswamy wrote:
> 
> +typedef enum WHPFunctionList {
> +    WINHV_PLATFORM_FNS_DEFAULT,
> +    WINHV_EMULATION_FNS_DEFAULT,
> +} WHPFunctionList;
>  

What does "default" stand for?  I assume you have more changes to this
function in the future.

> + * Load the functions from the given library, using the given handle. If a
> + * handle is provided, it is used, otherwise the library is opened. The
> + * handle will be updated on return with the opened one.
> + */
> +static bool load_whp_dipatch_fns(HMODULE *handle, WHPFunctionList function_list)
> +{

Typo, "dipatch" instead of "dispatch".
> 
> +    if (hLib) {
> +        FreeLibrary(hWinHvEmulation);
> +    }

The argument to FreeLibrary should be hLib.

Paolo



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] WHPX: refactor load library
  2019-11-12 19:47   ` Eduardo Habkost
@ 2019-11-13 16:35     ` Philippe Mathieu-Daudé
  2019-11-13 16:49       ` Eduardo Habkost
  2019-11-13 17:12       ` Sunil Muthuswamy
  0 siblings, 2 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-13 16:35 UTC (permalink / raw)
  To: Eduardo Habkost, Sunil Muthuswamy
  Cc: Paolo Bonzini, Richard Henderson, Justin Terry \(VM\),
	qemu-devel, Stefan Weil

On 11/12/19 8:47 PM, Eduardo Habkost wrote:
> On Tue, Nov 12, 2019 at 06:42:00PM +0000, Sunil Muthuswamy wrote:
>>
>>
>>> -----Original Message-----
>>> From: Sunil Muthuswamy
>>> Sent: Friday, November 8, 2019 12:32 PM
>>> To: 'Paolo Bonzini' <pbonzini@redhat.com>; 'Richard Henderson' <rth@twiddle.net>; 'Eduardo Habkost' <ehabkost@redhat.com>; 'Stefan
>>> Weil' <sw@weilnetz.de>
>>> Cc: 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>; Justin Terry (VM) <juterry@microsoft.com>
>>> Subject: [PATCH] WHPX: refactor load library
>>>
>>> This refactors the load library of WHV libraries to make it more
>>> modular. It makes a helper routine that can be called on demand.
>>> This allows future expansion of load library/functions to support
>>> functionality that is depenedent on some feature being available.
>>>
>>> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
>>> ---
>>
>> Can I possibly get some eyes on this?
> 
> I'd be glad to queue the patch if we get a Reviewed-by line from
> somebody who understands Windows and WHPX.  Maybe Justin?

Can we wait for approval from the Microsoft legal department first?
So we can start testing WHPX builds, and reduce the possibilities to 
introduce regressions.

Testing is ready, we are waiting for Microsoft to merge, see:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg646351.html

> 
> Sunil, Justin, would you like to be listed as maintainers or
> designated reviewers for the WHPX code in QEMU?

Great idea!



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] WHPX: refactor load library
  2019-11-13 16:35     ` Philippe Mathieu-Daudé
@ 2019-11-13 16:49       ` Eduardo Habkost
  2019-11-13 17:21         ` Sunil Muthuswamy
  2019-11-13 17:12       ` Sunil Muthuswamy
  1 sibling, 1 reply; 9+ messages in thread
From: Eduardo Habkost @ 2019-11-13 16:49 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Stefan Weil, qemu-devel, Justin Terry \(VM\),
	Paolo Bonzini, Sunil Muthuswamy, Richard Henderson

On Wed, Nov 13, 2019 at 05:35:59PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/12/19 8:47 PM, Eduardo Habkost wrote:
> > On Tue, Nov 12, 2019 at 06:42:00PM +0000, Sunil Muthuswamy wrote:
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: Sunil Muthuswamy
> > > > Sent: Friday, November 8, 2019 12:32 PM
> > > > To: 'Paolo Bonzini' <pbonzini@redhat.com>; 'Richard Henderson' <rth@twiddle.net>; 'Eduardo Habkost' <ehabkost@redhat.com>; 'Stefan
> > > > Weil' <sw@weilnetz.de>
> > > > Cc: 'qemu-devel@nongnu.org' <qemu-devel@nongnu.org>; Justin Terry (VM) <juterry@microsoft.com>
> > > > Subject: [PATCH] WHPX: refactor load library
> > > > 
> > > > This refactors the load library of WHV libraries to make it more
> > > > modular. It makes a helper routine that can be called on demand.
> > > > This allows future expansion of load library/functions to support
> > > > functionality that is depenedent on some feature being available.
> > > > 
> > > > Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> > > > ---
> > > 
> > > Can I possibly get some eyes on this?
> > 
> > I'd be glad to queue the patch if we get a Reviewed-by line from
> > somebody who understands Windows and WHPX.  Maybe Justin?
> 
> Can we wait for approval from the Microsoft legal department first?
> So we can start testing WHPX builds, and reduce the possibilities to
> introduce regressions.
> 
> Testing is ready, we are waiting for Microsoft to merge, see:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg646351.html

Making it easier for other people to test WHPX would be nice.
But in case this is not sorted out soon, I don't see a reason to
not merge WHPX changes if they are reviewed and approved by the
main author of that code (Justin).

-- 
Eduardo



^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] WHPX: refactor load library
  2019-11-13 14:59 ` Paolo Bonzini
@ 2019-11-13 16:59   ` Sunil Muthuswamy
  0 siblings, 0 replies; 9+ messages in thread
From: Sunil Muthuswamy @ 2019-11-13 16:59 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson, Eduardo Habkost, Stefan Weil
  Cc: qemu-devel, Justin Terry (VM)



> -----Original Message-----
> From: Paolo Bonzini <pbonzini@redhat.com>
> Sent: Wednesday, November 13, 2019 7:00 AM
> To: Sunil Muthuswamy <sunilmut@microsoft.com>; Richard Henderson <rth@twiddle.net>; Eduardo Habkost <ehabkost@redhat.com>;
> Stefan Weil <sw@weilnetz.de>
> Cc: qemu-devel@nongnu.org; Justin Terry (VM) <juterry@microsoft.com>
> Subject: Re: [PATCH] WHPX: refactor load library
> 
> On 08/11/19 21:31, Sunil Muthuswamy wrote:
> >
> > +typedef enum WHPFunctionList {
> > +    WINHV_PLATFORM_FNS_DEFAULT,
> > +    WINHV_EMULATION_FNS_DEFAULT,
> > +} WHPFunctionList;
> >
> 
> What does "default" stand for?  I assume you have more changes to this
> function in the future.
> 
Yes, there are more functions coming, such as for XSAVE. I used "default" to represent
whatever is there currently, for lack of a better term.

> > + * Load the functions from the given library, using the given handle. If a
> > + * handle is provided, it is used, otherwise the library is opened. The
> > + * handle will be updated on return with the opened one.
> > + */
> > +static bool load_whp_dipatch_fns(HMODULE *handle, WHPFunctionList function_list)
> > +{
> 
> Typo, "dipatch" instead of "dispatch".
> >
> > +    if (hLib) {
> > +        FreeLibrary(hWinHvEmulation);
> > +    }
> 
> The argument to FreeLibrary should be hLib.
> 

Thanks, will fix these in the next version.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] WHPX: refactor load library
  2019-11-13 16:35     ` Philippe Mathieu-Daudé
  2019-11-13 16:49       ` Eduardo Habkost
@ 2019-11-13 17:12       ` Sunil Muthuswamy
  1 sibling, 0 replies; 9+ messages in thread
From: Sunil Muthuswamy @ 2019-11-13 17:12 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Eduardo Habkost
  Cc: Paolo Bonzini, Stefan Weil, Justin Terry (VM),
	qemu-devel, Richard Henderson



> Can we wait for approval from the Microsoft legal department first?
> So we can start testing WHPX builds, and reduce the possibilities to
> introduce regressions.
> 
> Testing is ready, we are waiting for Microsoft to merge, see:
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.mail-archive.com%2Fqemu-
> devel%40nongnu.org%2Fmsg646351.html&amp;data=02%7C01%7Csunilmut%40microsoft.com%7C41ce65aedecb47c7bd0d08d76857937d
> %7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637092597657121219&amp;sdata=tu0zZDIzlG%2F9lEU4SJi11%2B%2FX1JdHUt6PD
> 2teeYCMZ%2B8%3D&amp;reserved=0
> 

Yes, we have escalated this to the right set of teams, including the legal
team. We are working through the processes here internally and will
update once we have something. Meanwhile, it would be good to see if
we can get these patches in.

> >
> > Sunil, Justin, would you like to be listed as maintainers or
> > designated reviewers for the WHPX code in QEMU?
> 
> Great idea!
It's a valid and good point. I am discussing this internally here and
will get back.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] WHPX: refactor load library
  2019-11-13 16:49       ` Eduardo Habkost
@ 2019-11-13 17:21         ` Sunil Muthuswamy
  0 siblings, 0 replies; 9+ messages in thread
From: Sunil Muthuswamy @ 2019-11-13 17:21 UTC (permalink / raw)
  To: Eduardo Habkost, Philippe Mathieu-Daudé
  Cc: Paolo Bonzini, Stefan Weil, Justin Terry (VM),
	qemu-devel, Richard Henderson


> Making it easier for other people to test WHPX would be nice.

Yes, we understand the concerns and I generally agree here. I am
trying to connect the different teams involved here (legal, SDK here)
and connect the dots for them, to see what can be done here.

> But in case this is not sorted out soon, I don't see a reason to
> not merge WHPX changes if they are reviewed and approved by the
> main author of that code (Justin).
> 

Justin is ready to review it, but is out for another week. Will have him
review once he is back.



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08 20:31 [PATCH] WHPX: refactor load library Sunil Muthuswamy
2019-11-12 18:42 ` Sunil Muthuswamy
2019-11-12 19:47   ` Eduardo Habkost
2019-11-13 16:35     ` Philippe Mathieu-Daudé
2019-11-13 16:49       ` Eduardo Habkost
2019-11-13 17:21         ` Sunil Muthuswamy
2019-11-13 17:12       ` Sunil Muthuswamy
2019-11-13 14:59 ` Paolo Bonzini
2019-11-13 16:59   ` Sunil Muthuswamy

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git