Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2015-03-02 at 09:20 +0800, Chen, Tiejun wrote:
> Is this expected?

Yes. Can you post it as a proper patch please.

I suggest you split the basic stuff and the kind override discussed
below in to two patches.

> >> +            (b_info->u.hvm.gfx_passthru &&
> >> +             strncmp(b_info->u.hvm.gfx_passthru, "igd", 3) == 0) ) {
> 
> But as you mentioned previously,
> 
> "
> You might like to optionally consider add a forcing option somehow so
> that people with new devices not in the list can control things without
> the need to recompile (e.g. gfx_passthru_kind_override?).
> "

> 
> Here I was trying to convert "gfx_passthru" to address this thing. 
> According to your comment right now, you prefer we should introduce a 
> new field instead of the original 'gfx_passthru' to enumerate such a 
> type. So what about this?
> 
> libxl_gfx_passthru_kind_type = Enumeration("gfx_passthru_kind_type", [

"kind_type" is redundant. I think just "kind" will do.

>      (0, "unknown"),

"default" I think is better, i.e. if gfx_passthru is enabled then do the
probing from the PCI ID list thing.

>      (1, "igd"),
>      ])

You would then need to add a field of this type next to the gfx_passthru
one in b_config, lets say it's called gfx_passthru_kind.

> Then if we want to override this, just submit the following line into .cfg:
> 
> gfx_passthru_kind_override = "igd"

So, while we cannot change the defbool in the libxl interface we do
actually have a little more freedom in the xl cfg parsing because we can
detect bool/integer vs string.

So I think it should be possible to arrange to support any of
    gfx_passthru = 0      => sets build_info.u.gfx_passthru to false
    gfx_passthru = 1      => sets build_info.u.gfx_passthru to false and
                             build_info.u.gfx_passthru_kind to DEFAULT
    gfx_passthru = "igd"  => sets build_info.u.gfx_passthru to false and
                             build_info.u.gfx_passthru_kind to IGD

Take a look at how the "timer_mode" option is handled in xl_cmdimpl.c
(except none of these variants are deprecated. You should be able to use
the libxl_..._from_string enum helper to do the parsing in the latter
case.

Ian.