Enhance hax logger on Windows, and some cleanup. #215

coxuintel · 2019-06-21T06:40:42Z

Use hax logger instead of direct call to DbgPrint, so that platform
specific APIs are not called directly.
Specify DPFLTR_IHVDRIVER_ID to vDbgPrintExWithPrefix(), and assign
log level accordingly. DPFLTR_xxx level to HAX log level mapping:
HAX_LOGD >> DPFLTR_TRACE_LEVEL
HAX_LOGI >> DPFLTR_INFO_LEVEL
HAX_LOGW >> DPFLTR_WARNING_LEVEL
HAX_LOGE >> DPFLTR_ERROR_LEVEL
According to MSDN, non-OS component should use DPFLTR_IHVDRIVER_ID.
With proper level set, on-the-fly log level can be controlled by
component filter mask without recompiling the driver.
Check MSDN for details:
https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/reading-and-filtering-debugging-messages
Replace reduntant default_hax_log_level with HAX_LOG_DEFAULT in hax.h,
and prevent using instant value directly in logger.
Some spelling mistake and cleanup.

Signed-off-by: Colin Xu colin.xu@intel.com

krytarowski · 2019-06-21T13:22:49Z

platforms/windows/hax_wrapper.c


+    if (level >= HAX_LOG_DEFAULT)
+        vDbgPrintExWithPrefix("haxm: ", DPFLTR_IHVDRIVER_ID, DPFLTR_ERROR_LEVEL, fmt, arglist);


va_end() missed. Even if it is dummy on Windows.

That's right, it's better pair va_start() with va_end() even it's dummy. Thanks for point it out.

wcwang

It is recommended to unify the log functions invocation approach on Windows.

All log functions are the wrappers of hax_log_level().
hax_log_level() is no longer called individually, instead of calling the wrapper functions.
Replace the remaining places that call hax_log_level() with the corresponding wrapper functions.
./platforms/windows/hax_entry.c
Line 696 and 707: hax_error

wcwang · 2019-07-04T12:12:50Z

platforms/windows/hax_wrapper.c

-    if (HAX_LOGE >= default_hax_log_level)
-        vDbgPrintExWithPrefix("haxm_error:", -1, 0, fmt, arglist);
+
+    if (HAX_LOGE >= HAX_LOG_DEFAULT)


hax_log_level(HAX_LOGE, fmt, arglist);

Got your point. Let me revise and update.

wcwang · 2019-07-04T12:13:11Z

platforms/windows/hax_wrapper.c

 }

 void hax_warning(char *fmt, ...)
 {
    va_list arglist;
    va_start(arglist, fmt);

-    if (HAX_LOGW >= default_hax_log_level)
-        vDbgPrintExWithPrefix("haxm_warning:", -1, 0, fmt, arglist);
+    if (HAX_LOGW >= HAX_LOG_DEFAULT)


hax_log_level(HAX_LOGW, fmt, arglist);

wcwang · 2019-07-04T12:13:23Z

platforms/windows/hax_wrapper.c

 }

 void hax_info(char *fmt, ...)
 {
    va_list arglist;
    va_start(arglist, fmt);

-    if (HAX_LOGI >= default_hax_log_level)
-        vDbgPrintExWithPrefix("haxm_info:", -1, 0, fmt, arglist);
+    if (HAX_LOGI >= HAX_LOG_DEFAULT)


hax_log_level(HAX_LOGI, fmt, arglist);

wcwang · 2019-07-04T12:13:33Z

platforms/windows/hax_wrapper.c

 }

 void hax_debug(char *fmt, ...)
 {
    va_list arglist;
    va_start(arglist, fmt);

-    if (HAX_LOGD >= default_hax_log_level)
-        vDbgPrintExWithPrefix("haxm_debug:", -1, 0, fmt, arglist);
+    if (HAX_LOGD >= HAX_LOG_DEFAULT)


hax_log_level(HAX_LOGD, fmt, arglist);

wcwang · 2019-07-04T12:15:55Z

platforms/windows/hax_wrapper.c

@@ -237,47 +226,70 @@ void hax_disable_irq(void)
    asm_disable_irq();
 }

-void hax_error(char *fmt, ...)
+int hax_log_level(int level, const char *fmt,  ...)


int hax_log_level(int level, const char *fmt, va_list arglist)
{
const char *prefix[] = {
"",
"haxm_debug: ", // HAX_LOGD
"haxm_info: ", // HAX_LOGI
"haxm_warning: ", // HAX_LOGW
"haxm_error: " // HAX_LOGE
};
if (level >= HAX_LOG_DEFAULT)
vDbgPrintExWithPrefix(prefix[level], DPFLTR_IHVDRIVER_ID, DPFLTR_ERROR_LEVEL, fmt, arglist);
return 0;
}

Considering to define prefix as below to prevent this array from being created and destroyed over and over again, and add a space after colons:
static const char *prefix[] = {
"",
"haxm_debug: ", // HAX_LOGD
"haxm_info: ", // HAX_LOGI
"haxm_warning: ", // HAX_LOGW
"haxm_error: " // HAX_LOGE
};

I have refine the log accordingly.

wcwang · 2019-07-29T09:28:40Z

platforms/linux/hax_wrapper.c

+    KERN_INFO,
+    KERN_WARNING,
+    KERN_ERR,
+    KERN_ERR,


Is it better to remove the comma after KERN_ERR?
It is recommended to add comment for each level, such as '// HAX_LOGE'.

wcwang · 2019-07-29T09:36:50Z

platforms/windows/hax_wrapper.c

+    DPFLTR_INFO_LEVEL,
+    DPFLTR_WARNING_LEVEL,
+    DPFLTR_ERROR_LEVEL,
+    DPFLTR_ERROR_LEVEL,


Is it better to remove the comma as it is the last item?
It is recommended to add comments for each level.

Enhance hax logger on Windows: - Use hax logger instead of direct call to DbgPrint, so that platform specific APIs are not called directly. - Specify DPFLTR_IHVDRIVER_ID to vDbgPrintExWithPrefix(), and assign log level accordingly. DPFLTR_xxx level to HAX log level mapping: HAX_LOGD >> DPFLTR_TRACE_LEVEL HAX_LOGI >> DPFLTR_INFO_LEVEL HAX_LOGW >> DPFLTR_WARNING_LEVEL HAX_LOGE >> DPFLTR_ERROR_LEVEL According to MSDN, non-OS component should use DPFLTR_IHVDRIVER_ID. With proper level set, on-the-fly log level can be controlled by component filter mask without recompiling the driver. Check MSDN for details: https://docs.microsoft.com/en-us/windows-hardware/drivers/devtest/reading-and-filtering-debugging-messages - Replace reduntant default_hax_log_level with HAX_LOG_DEFAULT in hax.h, and prevent using instant value directly in logger. Retire other log interface except hax_log() with level input. Now all log will call hax_log() with expect log level defined in hax.h. Some spelling mistakes and cleanup. Signed-off-by: Colin Xu <colin.xu@intel.com>

krytarowski reviewed Jun 21, 2019

View reviewed changes

HaxmCI added CI:Build Pass CI:Build Pass CI:Mac Test Pass CI:Mac Test Pass labels Jun 24, 2019

coxuintel requested review from wcwang and hyuan3 July 4, 2019 05:38

wcwang reviewed Jul 4, 2019

View reviewed changes

wcwang reviewed Jul 29, 2019

View reviewed changes

wcwang approved these changes Aug 5, 2019

View reviewed changes

wcwang merged commit 25b1856 into intel:master Aug 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance hax logger on Windows, and some cleanup. #215

Enhance hax logger on Windows, and some cleanup. #215

coxuintel commented Jun 21, 2019

krytarowski Jun 21, 2019

coxuintel Jun 24, 2019

wcwang left a comment

wcwang Jul 4, 2019

coxuintel Jul 5, 2019

wcwang Jul 4, 2019

wcwang Jul 4, 2019

wcwang Jul 4, 2019

wcwang Jul 4, 2019 •

edited

Loading

wcwang Jul 5, 2019 •

edited

Loading

coxuintel Jul 24, 2019

wcwang Jul 29, 2019 •

edited

Loading

coxuintel Aug 2, 2019

wcwang Jul 29, 2019 •

edited

Loading

coxuintel Aug 2, 2019


		if (level >= HAX_LOG_DEFAULT)
		vDbgPrintExWithPrefix("haxm: ", DPFLTR_IHVDRIVER_ID, DPFLTR_ERROR_LEVEL, fmt, arglist);

Enhance hax logger on Windows, and some cleanup. #215

Enhance hax logger on Windows, and some cleanup. #215

Conversation

coxuintel commented Jun 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wcwang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wcwang Jul 4, 2019 • edited Loading

Choose a reason for hiding this comment

wcwang Jul 5, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wcwang Jul 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wcwang Jul 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wcwang Jul 4, 2019 •

edited

Loading

wcwang Jul 5, 2019 •

edited

Loading

wcwang Jul 29, 2019 •

edited

Loading

wcwang Jul 29, 2019 •

edited

Loading