[PATCH] drm/i915/gvt: Add gvt_debug to classify GVT-g log messages

[Shuo Liu]

On Fri  1.Sep'17 at 10:36:59 +0800, Zhenyu Wang wrote:
>On 2017.08.31 20:41:00 +0800, Liu Shuo wrote:
>> > > diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
>> > > index f5486cb9..2b5955a 100644
>> > > --- a/drivers/gpu/drm/i915/gvt/Makefile
>> > > +++ b/drivers/gpu/drm/i915/gvt/Makefile
>> > > @@ -6,3 +6,4 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
>> > >  ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
>> > >  i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
>> > >  obj-$(CONFIG_DRM_I915_GVT_KVMGT)	+= $(GVT_DIR)/kvmgt.o
>> > > +obj-$(CONFIG_DRM_I915_GVT_DEBUG)	+= $(GVT_DIR)/gvt_debug.o
>> >
>> > Let's align file naming, just use "debug.c".
>> Changing name is just for the output module. gvt_debug.ko is more easy
>> to understand.
>
>oh, no, don't like for extra module, not necessary, debug.c should be in GVT_SOURCE
>if debug option is on.
Got your point. Thanks. But the module or not is the key. Seem below
problem.
>
>> > > diff --git a/drivers/gpu/drm/i915/gvt/gvt_debug.c b/drivers/gpu/drm/i915/gvt/gvt_debug.c
>> > > new file mode 100644
>> > > index 0000000..8fe8b2f
>> > > --- /dev/null
>> > > +++ b/drivers/gpu/drm/i915/gvt/gvt_debug.c
>> > > @@ -0,0 +1,41 @@
>> > > +#include <linux/module.h>
>> > > +#include "debug.h"
>> > > +
>> > > +static unsigned int gvt_debug;
>> > > +
>> > > +module_param_named(debug, gvt_debug, int, 0600);
>> > > +MODULE_PARM_DESC(debug, "Enable Gvt-g debug output, where each bit enables a category.\n"
>> > > +		"\t\tBit 0 (0x01) will enable CORE messages (GVT-g core message)\n"
>> > > +		"\t\tBit 1 (0x02) will enable IRQ messages (GVT-g interrupt message)\n"
>> > > +		"\t\tBit 2 (0x04) will enable MM messages (GVT-g memory management message)\n"
>> > > +		"\t\tBit 3 (0x08) will enable MMIO messages (GVT-g MMIO message)\n"
>> > > +		"\t\tBit 4 (0x10) will enable DPY messages (GVT-g display message)\n"
>> > > +		"\t\tBit 5 (0x20) will enable EL messages (GVT-g execlist message)\n"
>> > > +		"\t\tBit 6 (0x40) will enable SCHED messages (GVT-g schedule message)\n"
>> > > +		"\t\tBit 7 (0x80) will enable RENDER messages (GVT-g render message)\n"
>> > > +		"\t\tBit 8 (0x100) will enable CMD messages (GVT-g command message)");
>> > > +
>> > > +void gvt_printk(const char *level, unsigned int category,
>> > > +		const char *format, ...)
>> > > +{
>> > > +	struct va_format vaf;
>> > > +	va_list args;
>> > > +
>> > > +	if (category != GVT_MSG_NONE && !(gvt_debug & category))
>> > > +		return;
>> > > +
>> > > +	va_start(args, format);
>> > > +	vaf.fmt = format;
>> > > +	vaf.va = &args;
>> > > +
>> > > +	printk("%s[gvt:%ps]%s %pV",
>> > > +		level, __builtin_return_address(0),
>> > > +		strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
>> > > +
>> > > +	va_end(args);
>> > > +}
>> > > +EXPORT_SYMBOL_GPL(gvt_printk);
>> >
>> > Not required for other module, this is only for gvt internal usage.
>> Fully agree with you to build it in gvt module.
>> But here is limitation:
>> 1) gvt in i915 module, while MPT in another module kvmgt 2) i915 module with
>> gvt code has dependency on kvmgt
>> 3) both i915 module with gvt and kvmgt has dependency on gvt_printk
>> If we build gvt_printk into i915, it will deadlock while i915 modprobe.
>> Any comments on this?
>
>I don't think 2) is true and yes for 3), so you can export gvt_printk
>for kvmgt module and use same debug.h for it. You can just add gvt_debug
>option in i915 param.
It sounds like my original version. The problem is:
  i915.ko probe and init gvt -->
    try to load kvmgt.ko by __request_module -->
      modeprobe will wait i915.ko(gvt_print dependency) init complete (but not as we are in it) -->
        the module load timeout and fail.
>
>-- 
>Open Source Technology Center, Intel ltd.
>
>$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827