| Message ID | 1422839843-25622-1-git-send-email-tiejun.chen@intel.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 2015-02-02 at 09:17 +0800, Tiejun Chen wrote: > When we're working to support IGD GFX passthrough with qemu > upstream, instead of "-gfx_passthru" we'd like to make that > a machine option, "-machine xxx,-igd-passthru=on". This need > to bring a change on tool side. From which Qemu version is this new option accepted? What will a verison of qemu prior to then do when presented with the new option? (nb: you have an extra '-' in the description I think) > > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com> > --- > v2: > > * Based on some discussions with Wei we'd like to keep both old > option, -gfx_passthru, and new machine property option, > "-machine xxx,-igd-passthru=on" at the same time but deprecate > the old one. Then finally we remove the old one at that point > that to give downstream (in this case, Xen) time to cope with the > change. > > tools/libxl/libxl_dm.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index c2b0487..8405f0b 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -701,6 +701,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, > flexarray_append(dm_args, "-net"); > flexarray_append(dm_args, "none"); > } > + /* > + * Although we already introduce 'igd-passthru', but we'd like > + * to remove this until we give downstream time to cope with > + * the change. > + */ > if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { > flexarray_append(dm_args, "-gfx_passthru"); > } > @@ -748,6 +753,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, > machinearg, max_ram_below_4g); > } > } > + > + if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { > + machinearg = libxl__sprintf(gc, "%s,igd-passthru=on", machinearg); > + } > + > flexarray_append(dm_args, machinearg); > for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++) > flexarray_append(dm_args, b_info->extra_hvm[i]);
On Mon, Feb 02, 2015 at 09:17:23AM +0800, Tiejun Chen wrote: > When we're working to support IGD GFX passthrough with qemu > upstream, instead of "-gfx_passthru" we'd like to make that > a machine option, "-machine xxx,-igd-passthru=on". This need > to bring a change on tool side. > > Signed-off-by: Tiejun Chen <tiejun.chen@intel.com> > --- > v2: > > * Based on some discussions with Wei we'd like to keep both old > option, -gfx_passthru, and new machine property option, > "-machine xxx,-igd-passthru=on" at the same time but deprecate > the old one. Then finally we remove the old one at that point > that to give downstream (in this case, Xen) time to cope with the > change. > My suggestion has one premise -- if upstream QEMU has already released that -gfx_passthru option. If there is no "old one" (in upstream QEMU) at all, then there is nothing to keep and deprecate. > tools/libxl/libxl_dm.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index c2b0487..8405f0b 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -701,6 +701,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, Note this function is upstream QEMU specfic. > flexarray_append(dm_args, "-net"); > flexarray_append(dm_args, "none"); > } > + /* > + * Although we already introduce 'igd-passthru', but we'd like > + * to remove this until we give downstream time to cope with > + * the change. > + */ > if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { > flexarray_append(dm_args, "-gfx_passthru"); > } The comment contradicts what I know (or what I think I know). In our last email exchange you said there was no "-gfx_passthru" in any version of upstream QEMU. So, shouldn't you just remove this `if' statement? Wei. > @@ -748,6 +753,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, > machinearg, max_ram_below_4g); > } > } > + > + if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { > + machinearg = libxl__sprintf(gc, "%s,igd-passthru=on", machinearg); > + } > + > flexarray_append(dm_args, machinearg); > for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++) > flexarray_append(dm_args, b_info->extra_hvm[i]); > -- > 1.9.1
Wei Liu writes ("Re: [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough"):
> On Mon, Feb 02, 2015 at 09:17:23AM +0800, Tiejun Chen wrote:
> > When we're working to support IGD GFX passthrough with qemu
> > upstream, instead of "-gfx_passthru" we'd like to make that
> > a machine option, "-machine xxx,-igd-passthru=on". This need
> > to bring a change on tool side.
...
> My suggestion has one premise -- if upstream QEMU has already released
> that -gfx_passthru option. If there is no "old one" (in upstream QEMU)
> at all, then there is nothing to keep and deprecate.
I think the commit message of the xen.git commit should explain what
options are supported by which versions of qemu (including qemu
upstream's future plans).
That would provide (a) something which summarises the communication
etc. with qemu upstream and can be checked with them if necessary and
(b) something against which the libxl changes can be easily judged.
Thanks,
Ian.
On 2015/2/2 19:08, Ian Campbell wrote: > On Mon, 2015-02-02 at 09:17 +0800, Tiejun Chen wrote: >> When we're working to support IGD GFX passthrough with qemu >> upstream, instead of "-gfx_passthru" we'd like to make that >> a machine option, "-machine xxx,-igd-passthru=on". This need >> to bring a change on tool side. > >>From which Qemu version is this new option accepted? What will a verison > of qemu prior to then do when presented with the new option? Sorry, maybe I'm not describing this correctly. Actually qemu upstream never own this option, '-gfx_passthru' at all. This just exists alone in qemu-xen-traditional. So here I'm trying to introduce a new stuff that doesn't clash anything in qemu upstream. So I guess I should rephrase this as follows: libxl: add one machine property to support IGD GFX passthrough When we're working to support IGD GFX passthrough with qemu upstream, we'd like to introduce a machine option, "-machine xxx,igd-passthru=on", to enable/disable that feature. And we also remove that old option, "-gfx_passthru", just from the case of LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN since actually no any qemu stream version really need or use that. > > (nb: you have an extra '-' in the description I think) Yeah, I will remove that. Thanks Tiejun > >> >> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com> >> --- >> v2: >> >> * Based on some discussions with Wei we'd like to keep both old >> option, -gfx_passthru, and new machine property option, >> "-machine xxx,-igd-passthru=on" at the same time but deprecate >> the old one. Then finally we remove the old one at that point >> that to give downstream (in this case, Xen) time to cope with the >> change. >> >> tools/libxl/libxl_dm.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c >> index c2b0487..8405f0b 100644 >> --- a/tools/libxl/libxl_dm.c >> +++ b/tools/libxl/libxl_dm.c >> @@ -701,6 +701,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, >> flexarray_append(dm_args, "-net"); >> flexarray_append(dm_args, "none"); >> } >> + /* >> + * Although we already introduce 'igd-passthru', but we'd like >> + * to remove this until we give downstream time to cope with >> + * the change. >> + */ >> if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { >> flexarray_append(dm_args, "-gfx_passthru"); >> } >> @@ -748,6 +753,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, >> machinearg, max_ram_below_4g); >> } >> } >> + >> + if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { >> + machinearg = libxl__sprintf(gc, "%s,igd-passthru=on", machinearg); >> + } >> + >> flexarray_append(dm_args, machinearg); >> for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++) >> flexarray_append(dm_args, b_info->extra_hvm[i]); > > >
On 2015/2/2 20:54, Ian Jackson wrote: > Wei Liu writes ("Re: [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough"): >> On Mon, Feb 02, 2015 at 09:17:23AM +0800, Tiejun Chen wrote: >>> When we're working to support IGD GFX passthrough with qemu >>> upstream, instead of "-gfx_passthru" we'd like to make that >>> a machine option, "-machine xxx,-igd-passthru=on". This need >>> to bring a change on tool side. > ... >> My suggestion has one premise -- if upstream QEMU has already released >> that -gfx_passthru option. If there is no "old one" (in upstream QEMU) >> at all, then there is nothing to keep and deprecate. > > I think the commit message of the xen.git commit should explain what > options are supported by which versions of qemu (including qemu > upstream's future plans). > > That would provide (a) something which summarises the communication > etc. with qemu upstream and can be checked with them if necessary and > (b) something against which the libxl changes can be easily judged. > Sorry, looks I'm misleading this to everyone. Here I picked my reply from another email: Actually qemu upstream never own this option, '-gfx_passthru' at all. This just exists alone in qemu-xen-traditional. So here I'm trying to introduce a new stuff that doesn't clash anything in qemu upstream. So I guess I should rephrase this as follows: libxl: add one machine property to support IGD GFX passthrough When we're working to support IGD GFX passthrough with qemu upstream, we'd like to introduce a machine option, "-machine xxx,igd-passthru=on", to enable/disable that feature. And we also remove that old option, "-gfx_passthru", just from the case of LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN since actually no any qemu stream version really need or use that. So is it good? Thanks Tiejun
On Tue, 2015-02-03 at 09:04 +0800, Chen, Tiejun wrote: > > On 2015/2/2 20:54, Ian Jackson wrote: > > Wei Liu writes ("Re: [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough"): > >> On Mon, Feb 02, 2015 at 09:17:23AM +0800, Tiejun Chen wrote: > >>> When we're working to support IGD GFX passthrough with qemu > >>> upstream, instead of "-gfx_passthru" we'd like to make that > >>> a machine option, "-machine xxx,-igd-passthru=on". This need > >>> to bring a change on tool side. > > ... > >> My suggestion has one premise -- if upstream QEMU has already released > >> that -gfx_passthru option. If there is no "old one" (in upstream QEMU) > >> at all, then there is nothing to keep and deprecate. > > > > I think the commit message of the xen.git commit should explain what > > options are supported by which versions of qemu (including qemu > > upstream's future plans). > > > > That would provide (a) something which summarises the communication > > etc. with qemu upstream and can be checked with them if necessary and > > (b) something against which the libxl changes can be easily judged. > Sorry, looks I'm misleading this to everyone. > > Here I picked my reply from another email: > > Actually qemu upstream never own this option, '-gfx_passthru' at all. > This just exists alone in qemu-xen-traditional. So here I'm trying to > introduce a new stuff that doesn't clash anything in qemu upstream. > > So I guess I should rephrase this as follows: > > libxl: add one machine property to support IGD GFX passthrough > > When we're working to support IGD GFX passthrough with qemu > upstream, we'd like to introduce a machine option, Has this new option been acked/accepted into the upstream qemu code base yet? I think there should also be a reference to the relevant qemu.git changeset as well as to any useful conversations about it. > "-machine xxx,igd-passthru=on", to enable/disable that feature. > And we also remove that old option, "-gfx_passthru", just from > the case of LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN since actually > no any qemu stream version really need or use that. ^up ? What happens if you pass this new option to an older version of qemu upstream? I suppose it doesn't fail any worse than passing -gfx_passthru would have done. I have one more general concern, which is that hardcoding igd-passthru here may make it harder to support gfx_passthru of other cards in the future. Somehow something in the stack needs to either detect or be told which kind of card to passthrough. Automatic detection would be preferable, but maybe being told by the user is the only possibility. Is there any way, given gfx_passthru as a boolean that libxl can automatically figure out that IGD passthru is what is actually desired -- e.g. by scanning the set of PCI devices given to the guest perhaps? If not then that _might_ suggest we should deprecate the gdx_passthru option at the libxl interface and switch to something more expressive, such as an Enumeration of card types (with a singleton of IGD right now), but I'm not really very familiar with passthru nor the qemu side of this. What happens if you try to pass two different GFX cards to a guest? Ian.
On 2015/2/3 19:07, Ian Campbell wrote: > On Tue, 2015-02-03 at 09:04 +0800, Chen, Tiejun wrote: >> >> On 2015/2/2 20:54, Ian Jackson wrote: >>> Wei Liu writes ("Re: [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough"): >>>> On Mon, Feb 02, 2015 at 09:17:23AM +0800, Tiejun Chen wrote: >>>>> When we're working to support IGD GFX passthrough with qemu >>>>> upstream, instead of "-gfx_passthru" we'd like to make that >>>>> a machine option, "-machine xxx,-igd-passthru=on". This need >>>>> to bring a change on tool side. >>> ... >>>> My suggestion has one premise -- if upstream QEMU has already released >>>> that -gfx_passthru option. If there is no "old one" (in upstream QEMU) >>>> at all, then there is nothing to keep and deprecate. >>> >>> I think the commit message of the xen.git commit should explain what >>> options are supported by which versions of qemu (including qemu >>> upstream's future plans). >>> >>> That would provide (a) something which summarises the communication >>> etc. with qemu upstream and can be checked with them if necessary and >>> (b) something against which the libxl changes can be easily judged. > >> Sorry, looks I'm misleading this to everyone. >> >> Here I picked my reply from another email: >> >> Actually qemu upstream never own this option, '-gfx_passthru' at all. >> This just exists alone in qemu-xen-traditional. So here I'm trying to >> introduce a new stuff that doesn't clash anything in qemu upstream. >> >> So I guess I should rephrase this as follows: >> >> libxl: add one machine property to support IGD GFX passthrough >> >> When we're working to support IGD GFX passthrough with qemu >> upstream, we'd like to introduce a machine option, > > Has this new option been acked/accepted into the upstream qemu code base > yet? I think there should also be a reference to the relevant qemu.git Yes, Gerd just recommend this option and I guess this should be acked as well :) > changeset as well as to any useful conversations about it. I just post this patch prior to those qemu patch supporting IGD passthrough since I need this option is acked on Xen side. Then I can continue submitting all qemu patches. > >> "-machine xxx,igd-passthru=on", to enable/disable that feature. >> And we also remove that old option, "-gfx_passthru", just from >> the case of LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN since actually >> no any qemu stream version really need or use that. > ^up ? > > What happens if you pass this new option to an older version of qemu > upstream? I suppose it doesn't fail any worse than passing -gfx_passthru > would have done. Neither '-gfx_passthru' nor 'igd-passthrou' exists in any version of qemu upstream. As I mentioned previously, just now we're starting to support this in qemu upstream :) But you known, on Xen side we have two qemu versions, qemu-xen-traditional and qemu-xen. Although '-gfx_passthru' is adopted in both two versions, just qemu-xen-traditional supports IGD passthrough completely. For qemu-xen, we just have this term definition but without that IGD passthrough feature support actually. And now we're trying to support IGD passthrough in qemu stream. This mean we should set 'device_model_version="qemu-xen", right? Then libxl still pass '-gfx_passthru' to qemu upstream. But when I post other stuffs to qemu upstream community to support IGD passthrough. Gerd thought "-machine xxx,igd-passthru=on" is better than '-gfx_passthru'. So finally you see this change in Xen/tools/libxl. > > I have one more general concern, which is that hardcoding igd-passthru > here may make it harder to support gfx_passthru of other cards in the > future. Actually gfx_passthrou is introduced to just work for IGD passthrough since something specific to IGD is tricky, so we have to need such a flag to handle this precisely, like its fixed at 00:02.0, and expose some ISA bridge PCI config info and even host bridge PCI config info. So I don't thing other cards need this. > > Somehow something in the stack needs to either detect or be told which > kind of card to passthrough. Automatic detection would be preferable, > but maybe being told by the user is the only possibility. Based on the above explanation, something should be done before we detect to construct that real card , so its difficulty to achieve this goal currently. > > Is there any way, given gfx_passthru as a boolean that libxl can > automatically figure out that IGD passthru is what is actually desired > -- e.g. by scanning the set of PCI devices given to the guest perhaps? Sorry I don't understand what you mean here. Currently, we have to set something as follows, gfx_passthru=1 pci=["00:02.0"] This always works for qemu-xen-traditional. But you should know '00:02.0' doesn't mean we are passing IGD through. > > If not then that _might_ suggest we should deprecate the gdx_passthru > option at the libxl interface and switch to something more expressive, > such as an Enumeration of card types (with a singleton of IGD right > now), but I'm not really very familiar with passthru nor the qemu side > of this. > > What happens if you try to pass two different GFX cards to a guest? > Are you saying two IGDs? Its not possible since as I said above, IGD is tricky because it depends on something from ISA bridge and host bridge. So we can't provide two or more different setting configurations to own more IGDs just in one platform. Thanks Tiejun
On Wed, 2015-02-04 at 09:34 +0800, Chen, Tiejun wrote: > > > >> "-machine xxx,igd-passthru=on", to enable/disable that feature. > >> And we also remove that old option, "-gfx_passthru", just from > >> the case of LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN since actually > >> no any qemu stream version really need or use that. > > ^up ? > > > > What happens if you pass this new option to an older version of qemu > > upstream? I suppose it doesn't fail any worse than passing -gfx_passthru > > would have done. > > Neither '-gfx_passthru' nor 'igd-passthrou' exists in any version of > qemu upstream. As I mentioned previously, just now we're starting to > support this in qemu upstream :) > > But you known, on Xen side we have two qemu versions, > qemu-xen-traditional and qemu-xen. Although '-gfx_passthru' is adopted > in both two versions, just qemu-xen-traditional supports IGD passthrough > completely. For qemu-xen, we just have this term definition but without > that IGD passthrough feature support actually. I'm afraid I don't follow, you seem to be simultaneously saying that qemu-xen both does and does not support -gfx_passthru, which cannot be right. I think from the following paragraph that what you mean is that upstream qemu-xen has no support for any kind of -gfx_passthru command line option in any form in any existing version, including the git tree. Is that correct? (for the purposes of this conversation qemu-traditional is not of interest) > And now we're trying to support IGD passthrough in qemu stream. This > mean we should set 'device_model_version="qemu-xen", right? Then libxl > still pass '-gfx_passthru' to qemu upstream. But when I post other > stuffs to qemu upstream community to support IGD passthrough. Gerd > thought "-machine xxx,igd-passthru=on" is better than '-gfx_passthru'. > So finally you see this change in Xen/tools/libxl. > > > > > I have one more general concern, which is that hardcoding igd-passthru > > here may make it harder to support gfx_passthru of other cards in the > > future. > > Actually gfx_passthrou is introduced to just work for IGD passthrough > since something specific to IGD is tricky, so we have to need such a > flag to handle this precisely, like its fixed at 00:02.0, and expose > some ISA bridge PCI config info and even host bridge PCI config info. > > So I don't thing other cards need this. If one type VGA device needs these sorts of workaround it is not inconceivable that some other one will also need workarounds in the future. Even if you don't consider non-IGD, what about the possibility of a future IGD device which needs different (or additional, or fewer) workarounds? > > Somehow something in the stack needs to either detect or be told which > > kind of card to passthrough. Automatic detection would be preferable, > > but maybe being told by the user is the only possibility. > > Based on the above explanation, something should be done before we > detect to construct that real card , so its difficulty to achieve this > goal currently. > > > > > Is there any way, given gfx_passthru as a boolean that libxl can > > automatically figure out that IGD passthru is what is actually desired > > -- e.g. by scanning the set of PCI devices given to the guest perhaps? > > Sorry I don't understand what you mean here. "gfx_passthru" is a generically named option, but it is being implemented in an IGD specific way. We need to consider the possibility of other graphics devices needing special handling in the future and plan accordingly such that we can try and maintain our API guarantees when this happens. I think there are three ways to achieve that: * Make the libxl/xl option something which is not generic e.g. igd_passthru=1 * Add a second option to allow the user to configure the kind of graphics device being passed thru (e.g. gfx_passthru=1, passthru_device="igd"), or combine the two by making the gfx_passthru option a string instead of a boolean. * Make libxl detect the type of graphics device somehow and therefore automatically determine when gfx_passthru=1 => igd-passthru > Currently, we have to set > something as follows, > > gfx_passthru=1 > pci=["00:02.0"] > > This always works for qemu-xen-traditional. > > But you should know '00:02.0' doesn't mean we are passing IGD through. But by looking at the device 00:02.0 (e.g. its PCI vendor and device ID and other properties) we can unambiguously determine if it is an IGD device or not, can't we? > > If not then that _might_ suggest we should deprecate the gdx_passthru > > option at the libxl interface and switch to something more expressive, > > such as an Enumeration of card types (with a singleton of IGD right > > now), but I'm not really very familiar with passthru nor the qemu side > > of this. > > > > What happens if you try to pass two different GFX cards to a guest? > > > > Are you saying two IGDs? Yes, or any combination of two cards, perhaps from different vendors (AIUI some laptops have this with IGD and Nvidia or ATI?). > Its not possible since as I said above, IGD is > tricky because it depends on something from ISA bridge and host bridge. > So we can't provide two or more different setting configurations to own > more IGDs just in one platform. This is because IGD must be a "primary" VGA device? I understand that there can only be one of those in a system, but I think it is possible to have multiple secondary VGA devices or different types in one system. Ian.
On 2015/2/4 18:41, Ian Campbell wrote: > On Wed, 2015-02-04 at 09:34 +0800, Chen, Tiejun wrote: >>> >>>> "-machine xxx,igd-passthru=on", to enable/disable that feature. >>>> And we also remove that old option, "-gfx_passthru", just from >>>> the case of LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN since actually >>>> no any qemu stream version really need or use that. >>> ^up ? >>> >>> What happens if you pass this new option to an older version of qemu >>> upstream? I suppose it doesn't fail any worse than passing -gfx_passthru >>> would have done. >> >> Neither '-gfx_passthru' nor 'igd-passthrou' exists in any version of >> qemu upstream. As I mentioned previously, just now we're starting to >> support this in qemu upstream :) >> >> But you known, on Xen side we have two qemu versions, >> qemu-xen-traditional and qemu-xen. Although '-gfx_passthru' is adopted >> in both two versions, just qemu-xen-traditional supports IGD passthrough >> completely. For qemu-xen, we just have this term definition but without >> that IGD passthrough feature support actually. > > I'm afraid I don't follow, you seem to be simultaneously saying that > qemu-xen both does and does not support -gfx_passthru, which cannot be > right. I think from the following paragraph that what you mean is that > upstream qemu-xen has no support for any kind of -gfx_passthru command > line option in any form in any existing version, including the git tree. > Is that correct? Yes. > > (for the purposes of this conversation qemu-traditional is not of > interest) Yes. > >> And now we're trying to support IGD passthrough in qemu stream. This >> mean we should set 'device_model_version="qemu-xen", right? Then libxl >> still pass '-gfx_passthru' to qemu upstream. But when I post other >> stuffs to qemu upstream community to support IGD passthrough. Gerd >> thought "-machine xxx,igd-passthru=on" is better than '-gfx_passthru'. >> So finally you see this change in Xen/tools/libxl. >> >>> >>> I have one more general concern, which is that hardcoding igd-passthru >>> here may make it harder to support gfx_passthru of other cards in the >>> future. >> >> Actually gfx_passthrou is introduced to just work for IGD passthrough >> since something specific to IGD is tricky, so we have to need such a >> flag to handle this precisely, like its fixed at 00:02.0, and expose >> some ISA bridge PCI config info and even host bridge PCI config info. >> >> So I don't thing other cards need this. > > If one type VGA device needs these sorts of workaround it is not > inconceivable that some other one will also need workarounds in the > future. > Indeed this is not something workaround, and I think in any type of VGA devices, we'd like to diminish this sort of thing gradually, right? This mightn't come true in real world :) > Even if you don't consider non-IGD, what about the possibility of a > future IGD device which needs different (or additional, or fewer) > workarounds? As far as I know we're trying to drop off those dependencies on ISA bridge and host bridge in our IGD's roadmap. Because in any pass through cases, theoretically we should access those resources dedicated to that device. > >>> Somehow something in the stack needs to either detect or be told which >>> kind of card to passthrough. Automatic detection would be preferable, >>> but maybe being told by the user is the only possibility. >> >> Based on the above explanation, something should be done before we >> detect to construct that real card , so its difficulty to achieve this >> goal currently. >> >>> >>> Is there any way, given gfx_passthru as a boolean that libxl can >>> automatically figure out that IGD passthru is what is actually desired >>> -- e.g. by scanning the set of PCI devices given to the guest perhaps? >> >> Sorry I don't understand what you mean here. > > "gfx_passthru" is a generically named option, but it is being > implemented in an IGD specific way. We need to consider the possibility > of other graphics devices needing special handling in the future and > plan accordingly such that we can try and maintain our API guarantees > when this happens. Agreed. > > I think there are three ways to achieve that: > > * Make the libxl/xl option something which is not generic e.g. > igd_passthru=1 > * Add a second option to allow the user to configure the kind of > graphics device being passed thru (e.g. gfx_passthru=1, > passthru_device="igd"), or combine the two by making the > gfx_passthru option a string instead of a boolean. It makes more sense but this mean we're going to change that existing rule in qemu-traditional. But here I guess we shouldn't consider that case. > * Make libxl detect the type of graphics device somehow and > therefore automatically determine when gfx_passthru=1 => > igd-passthru This way confounds me all. Can libxl detect the graphics device *before* we intend to pass a parameter to active qemu? > >> Currently, we have to set >> something as follows, >> >> gfx_passthru=1 >> pci=["00:02.0"] >> >> This always works for qemu-xen-traditional. >> >> But you should know '00:02.0' doesn't mean we are passing IGD through. > > But by looking at the device 00:02.0 (e.g. its PCI vendor and device ID > and other properties) we can unambiguously determine if it is an IGD > device or not, can't we? Again, like what I said above, I'm not sure if its possible in my case. If I'm wrong please correct me. > >>> If not then that _might_ suggest we should deprecate the gdx_passthru >>> option at the libxl interface and switch to something more expressive, >>> such as an Enumeration of card types (with a singleton of IGD right >>> now), but I'm not really very familiar with passthru nor the qemu side >>> of this. >>> >>> What happens if you try to pass two different GFX cards to a guest? >>> >> >> Are you saying two IGDs? > > Yes, or any combination of two cards, perhaps from different vendors > (AIUI some laptops have this with IGD and Nvidia or ATI?). One IGD and multiple other type of Graphic display cards can coexist. > >> Its not possible since as I said above, IGD is >> tricky because it depends on something from ISA bridge and host bridge. >> So we can't provide two or more different setting configurations to own >> more IGDs just in one platform. > > This is because IGD must be a "primary" VGA device? I understand that No. I mean ISA bridge and host bridge just provide one set of IGD resource so its difficult to configure two or more IGDs. > there can only be one of those in a system, but I think it is possible > to have multiple secondary VGA devices or different types in one system. > What I'm saying is, its impossible to own two same IGDs in our current platform :) Thanks Tiejun
On Thu, 2015-02-05 at 09:22 +0800, Chen, Tiejun wrote: > Indeed this is not something workaround, and I think in any type of VGA > devices, we'd like to diminish this sort of thing gradually, right? > > This mightn't come true in real world :) It's not really something we can control, the h/w guys will do what they do, including integrating on-board graphics tightly with the N/S-bridges etc. > > > > I think there are three ways to achieve that: > > > > * Make the libxl/xl option something which is not generic e.g. > > igd_passthru=1 > > * Add a second option to allow the user to configure the kind of > > graphics device being passed thru (e.g. gfx_passthru=1, > > passthru_device="igd"), or combine the two by making the > > gfx_passthru option a string instead of a boolean. > > It makes more sense but this mean we're going to change that existing > rule in qemu-traditional. But here I guess we shouldn't consider that case. qemu-trad is frozen so we wouldn't be adding new features such as support for new graphics passthru devices, so we can ignore it's deficiencies in this area and just improve things in the qemu-xen case. (we may want to add a compat handling for any new option we add to libxl to translate to -gfx_passthru, but that's about it) > > * Make libxl detect the type of graphics device somehow and > > therefore automatically determine when gfx_passthru=1 => > > igd-passthru > > This way confounds me all. Can libxl detect the graphics device *before* > we intend to pass a parameter to active qemu? We know the BDF of all devices which we are going to pass to the guest and we can observe various properties of that device via /sys/bus/pci/devices/0000:B:D:F. > > > > >> Currently, we have to set > >> something as follows, > >> > >> gfx_passthru=1 > >> pci=["00:02.0"] > >> > >> This always works for qemu-xen-traditional. > >> > >> But you should know '00:02.0' doesn't mean we are passing IGD through. > > > > But by looking at the device 00:02.0 (e.g. its PCI vendor and device ID > > and other properties) we can unambiguously determine if it is an IGD > > device or not, can't we? > > Again, like what I said above, I'm not sure if its possible in my case. > If I'm wrong please correct me. Is my reply above sufficient? > >>> If not then that _might_ suggest we should deprecate the gdx_passthru > >>> option at the libxl interface and switch to something more expressive, > >>> such as an Enumeration of card types (with a singleton of IGD right > >>> now), but I'm not really very familiar with passthru nor the qemu side > >>> of this. > >>> > >>> What happens if you try to pass two different GFX cards to a guest? > >>> > >> > >> Are you saying two IGDs? > > > > Yes, or any combination of two cards, perhaps from different vendors > > (AIUI some laptops have this with IGD and Nvidia or ATI?). > > One IGD and multiple other type of Graphic display cards can coexist. ... but if they both need special handling then we need a way to communicate that. > >> Its not possible since as I said above, IGD is > >> tricky because it depends on something from ISA bridge and host bridge. > >> So we can't provide two or more different setting configurations to own > >> more IGDs just in one platform. > > > > This is because IGD must be a "primary" VGA device? I understand that > > No. I mean ISA bridge and host bridge just provide one set of IGD > resource so its difficult to configure two or more IGDs. > > > there can only be one of those in a system, but I think it is possible > > to have multiple secondary VGA devices or different types in one system. > > > > What I'm saying is, its impossible to own two same IGDs in our current > platform :) Understood, but systems needn't be homogeneous wrt graphics devices. Ian.
On 2015/2/5 17:52, Ian Campbell wrote: > On Thu, 2015-02-05 at 09:22 +0800, Chen, Tiejun wrote: >> Indeed this is not something workaround, and I think in any type of VGA >> devices, we'd like to diminish this sort of thing gradually, right? >> >> This mightn't come true in real world :) > > It's not really something we can control, the h/w guys will do what they > do, including integrating on-board graphics tightly with the N/S-bridges > etc. Yeah. > >>> >>> I think there are three ways to achieve that: >>> >>> * Make the libxl/xl option something which is not generic e.g. >>> igd_passthru=1 >>> * Add a second option to allow the user to configure the kind of >>> graphics device being passed thru (e.g. gfx_passthru=1, >>> passthru_device="igd"), or combine the two by making the >>> gfx_passthru option a string instead of a boolean. >> >> It makes more sense but this mean we're going to change that existing >> rule in qemu-traditional. But here I guess we shouldn't consider that case. > > qemu-trad is frozen so we wouldn't be adding new features such as > support for new graphics passthru devices, so we can ignore it's > deficiencies in this area and just improve things in the qemu-xen case. > (we may want to add a compat handling for any new option we add to libxl > to translate to -gfx_passthru, but that's about it) Understood. > >>> * Make libxl detect the type of graphics device somehow and >>> therefore automatically determine when gfx_passthru=1 => >>> igd-passthru >> >> This way confounds me all. Can libxl detect the graphics device *before* >> we intend to pass a parameter to active qemu? > > We know the BDF of all devices which we are going to pass to the guest > and we can observe various properties of that device > via /sys/bus/pci/devices/0000:B:D:F. So sounds what you're saying is Xen already have this sort of example in libxl. > >> >>> >>>> Currently, we have to set >>>> something as follows, >>>> >>>> gfx_passthru=1 >>>> pci=["00:02.0"] >>>> >>>> This always works for qemu-xen-traditional. >>>> >>>> But you should know '00:02.0' doesn't mean we are passing IGD through. >>> >>> But by looking at the device 00:02.0 (e.g. its PCI vendor and device ID >>> and other properties) we can unambiguously determine if it is an IGD >>> device or not, can't we? >> >> Again, like what I said above, I'm not sure if its possible in my case. >> If I'm wrong please correct me. > > Is my reply above sufficient? Yes, I can understand what you mean but I need to take close look at exactly what should be done in libxl :) In high level, this way may come out as follows, #1 libxl parse 'pci=[]' to get SBDF #2 Scan SBDF by accessing sysfs to get vendor/device IDs. #3 If this pair of IDs is identified to our target device, IGD, "igd-passthru" would be delivered to qemu. Right? > >>>>> If not then that _might_ suggest we should deprecate the gdx_passthru >>>>> option at the libxl interface and switch to something more expressive, >>>>> such as an Enumeration of card types (with a singleton of IGD right >>>>> now), but I'm not really very familiar with passthru nor the qemu side >>>>> of this. >>>>> >>>>> What happens if you try to pass two different GFX cards to a guest? >>>>> >>>> >>>> Are you saying two IGDs? >>> >>> Yes, or any combination of two cards, perhaps from different vendors >>> (AIUI some laptops have this with IGD and Nvidia or ATI?). >> >> One IGD and multiple other type of Graphic display cards can coexist. > > ... but if they both need special handling then we need a way to > communicate that. > >>>> Its not possible since as I said above, IGD is >>>> tricky because it depends on something from ISA bridge and host bridge. >>>> So we can't provide two or more different setting configurations to own >>>> more IGDs just in one platform. >>> >>> This is because IGD must be a "primary" VGA device? I understand that >> >> No. I mean ISA bridge and host bridge just provide one set of IGD >> resource so its difficult to configure two or more IGDs. >> >>> there can only be one of those in a system, but I think it is possible >>> to have multiple secondary VGA devices or different types in one system. >>> >> >> What I'm saying is, its impossible to own two same IGDs in our current >> platform :) > > Understood, but systems needn't be homogeneous wrt graphics devices. > Ian, Thanks for your kind discussion. Tiejun
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index c2b0487..8405f0b 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -701,6 +701,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, flexarray_append(dm_args, "-net"); flexarray_append(dm_args, "none"); } + /* + * Although we already introduce 'igd-passthru', but we'd like + * to remove this until we give downstream time to cope with + * the change. + */ if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { flexarray_append(dm_args, "-gfx_passthru"); } @@ -748,6 +753,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, machinearg, max_ram_below_4g); } } + + if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { + machinearg = libxl__sprintf(gc, "%s,igd-passthru=on", machinearg); + } + flexarray_append(dm_args, machinearg); for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++) flexarray_append(dm_args, b_info->extra_hvm[i]);
When we're working to support IGD GFX passthrough with qemu upstream, instead of "-gfx_passthru" we'd like to make that a machine option, "-machine xxx,-igd-passthru=on". This need to bring a change on tool side. Signed-off-by: Tiejun Chen <tiejun.chen@intel.com> --- v2: * Based on some discussions with Wei we'd like to keep both old option, -gfx_passthru, and new machine property option, "-machine xxx,-igd-passthru=on" at the same time but deprecate the old one. Then finally we remove the old one at that point that to give downstream (in this case, Xen) time to cope with the change. tools/libxl/libxl_dm.c | 10 ++++++++++ 1 file changed, 10 insertions(+)