Re: [PATCH] drm/i915: prevent generating unusable gvt build which no mpt module is selected

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 25 May 2017, "Du, Changbin" <changbin.du@intel.com> wrote:
> Hi, Jani, just relized you are in i915 team. :)
>
>> > +menu "Intel GVT-g graphics virtualization host support"
>> > +	depends on DRM_I915
>> > +	depends on 64BIT
>> > +
>> >  config DRM_I915_GVT
>> > -        bool "Enable Intel GVT-g graphics virtualization host support"
>> > -        depends on DRM_I915
>> > -        depends on 64BIT
>> > -        default n
>> > -        help
>> > +	bool "Enable Intel GVT-g graphics virtualization host support"
>> > +	default n
>> > +	depends on DRM_I915_GVT_KVMGT
>> > +	help
>> 
>> With this change, you can't actually change this config option. It's
>> purely subject to CONFIG_DRM_I915_GVT_KVMGT. You need to enable KVMGT to
>> even see this option, but enabling it will enable this one too. You
>> can't disable this before you disable KVMGT, but then disabling KVMGT
>> will disable this one too. This config option becomes pointless as a
>> visible option. Which isn't all that bad, considering
>> Documentation/kbuild/kconfig-language.txt:
>>
> Jani, this is by design in this patch. We will add another xengt hypervisor glue
> layer to support XenGT. After that, enable DRM_I915_GVT only if at least one of
> KVMGT or XENGT enabled or both. Also it doesn't make sense that we only build
> KVMGT/XenGT module without DRM_I915_GVT.
>
> Such mechanism is not as straigforward as two simple 'choice', so I agree with
> 'choice' if you prefer it. As you said, it is not a big problem.
>
>>   Note:
>> 	select should be used with care. select will force
>> 	a symbol to a value without visiting the dependencies.
>> 	By abusing select you are able to select a symbol FOO even
>> 	if FOO depends on BAR that is not set.
>> 	In general use select only for non-visible symbols
>> 	(no prompts anywhere) and for symbols with no dependencies.
>> 	That will limit the usefulness but on the other hand avoid
>> 	the illegal configurations all over.
>> 
> Yes, we should always be carefull with 'select' and should not use it if
> possible. So here I must create a 'menu' to ensure its safety.

I'm trying to say, why do you make DRM_I915_GVT visible in menuconfig at
all when you can't actually change it in menuconfig?

BR,
Jani.



>
>> BR,
>> Jani.
>> 
>> >  	  Choose this option if you want to enable Intel GVT-g graphics
>> >  	  virtualization technology host support with integrated graphics.
>> >  	  With GVT-g, it's possible to have one integrated graphics
>> > @@ -116,13 +119,14 @@ config DRM_I915_GVT
>> >  
>> >  config DRM_I915_GVT_KVMGT
>> >  	tristate "Enable KVM/VFIO support for Intel GVT-g"
>> > -	depends on DRM_I915_GVT
>> > +	select DRM_I915_GVT
>> >  	depends on KVM
>> >  	depends on VFIO_MDEV && VFIO_MDEV_DEVICE
>> >  	default n
>> >  	help
>> >  	  Choose this option if you want to enable KVMGT support for
>> >  	  Intel GVT-g.
>> > +endmenu
>> >  
>> >  menu "drm/i915 Debugging"
>> >  depends on DRM_I915
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center
>> _______________________________________________
>> intel-gvt-dev mailing list
>> intel-gvt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx