| Message ID | 1451378763-25398-1-git-send-email-caoj.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
the screenshots in attachment shows: 1. this patch don`t affect the device realization. 2. also shows what that bug looks like, which is described in my previous patch: "bugfix: passing reference instead of value" On 12/29/2015 04:46 PM, Cao jin wrote: > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > --- > changelog v4: > 1. strip the bugfix code, but I guess this patch should be applied after > that bugfix patch. > 2. Other little fix as per previous review > > Some explanation to previous review: > 1. error_setg_errno() I use in this patch will print the strerror(errno), so I > guess it will be informative enough? example: > > error_setg_errno(errp, errno, "open %s fail", path); will print: > "open bluhbluh fail: string_from_strerrer(errno)" > > 2. snprintf() has tiny tiny chance to fail, I don`t assert here, because this > device is on-board, it init during machine initialization, in case it fails, > the errp we set will results in error_report(err) and exit(1), at least can > let user know why fail. see the call-chain: > > pc_xen_hvm_init_pci->pc_init1->i440fx_init->pci_create_simple-> pci_create_simple_multifunction->qdev_init_nofail > > About test: > 1. Compiled > 2. Did a dirty hack to force create/realize this device in pc_init1(), prove > that the realizing process is ok, I will reply this mail later to attch the > screenshot evidence > > hw/pci-host/piix.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > index a9cb983..e91570f 100644 > --- a/hw/pci-host/piix.c > +++ b/hw/pci-host/piix.c > @@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = { > {0xa8, 4}, /* SNB: base of GTT stolen memory */ > }; > > -static int host_pci_config_read(int pos, int len, uint32_t *val) > +static void host_pci_config_read(int pos, int len, uint32_t *val, Error **errp) > { > char path[PATH_MAX]; > int config_fd; > @@ -769,19 +769,19 @@ static int host_pci_config_read(int pos, int len, uint32_t *val) > /* Access real host bridge. */ > int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", > 0, 0, 0, 0, "config"); > - int ret = 0; > > if (rc >= size || rc < 0) { > - return -ENODEV; > + error_setg_errno(errp, errno, "snprintf err"); > } > > config_fd = open(path, O_RDWR); > if (config_fd < 0) { > - return -ENODEV; > + error_setg_errno(errp, errno, "open %s fail", path); > + return; > } > > if (lseek(config_fd, pos, SEEK_SET) != pos) { > - ret = -errno; > + error_setg_errno(errp, errno, "lseek err"); > goto out; > } > > @@ -789,32 +789,32 @@ static int host_pci_config_read(int pos, int len, uint32_t *val) > rc = read(config_fd, (uint8_t *)val, len); > } while (rc < 0 && (errno == EINTR || errno == EAGAIN)); > if (rc != len) { > - ret = -errno; > + error_setg_errno(errp, errno, "read err"); > } > > out: > close(config_fd); > - return ret; > } > > -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) > +static void igd_pt_i440fx_realize(struct PCIDevice *pci_dev, Error **errp) > { > uint32_t val = 0; > - int rc, i, num; > + int i, num; > int pos, len; > + Error *local_err = NULL; > > num = ARRAY_SIZE(igd_host_bridge_infos); > for (i = 0; i < num; i++) { > pos = igd_host_bridge_infos[i].offset; > len = igd_host_bridge_infos[i].len; > - rc = host_pci_config_read(pos, len, &val); > - if (rc) { > - return -ENODEV; > + > + host_pci_config_read(pos, len, &val, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > } > pci_default_write_config(pci_dev, pos, cpu_to_le32(val), len); > } > - > - return 0; > } > > static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) > @@ -822,7 +822,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > - k->init = igd_pt_i440fx_initfn; > + k->realize = igd_pt_i440fx_realize; > dc->desc = "IGD Passthrough Host bridge"; > } > >
On Tue, Dec 29, 2015 at 04:46:03PM +0800, Cao jin wrote: > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > --- > changelog v4: > 1. strip the bugfix code, but I guess this patch should be applied after > that bugfix patch. > 2. Other little fix as per previous review > > Some explanation to previous review: > 1. error_setg_errno() I use in this patch will print the strerror(errno), so I > guess it will be informative enough? example: > > error_setg_errno(errp, errno, "open %s fail", path); will print: > "open bluhbluh fail: string_from_strerrer(errno)" I'd prefer some mention of why this is attempted too. E.g. "igd passthrough : open bluhbluh fail" > 2. snprintf() has tiny tiny chance to fail, It just formats a string into a buffer. the buffer is pre-allocated, and we know it's large enough for the data. How can it fail? > I don`t assert here, because this > device is on-board, it init during machine initialization, in case it fails, > the errp we set will results in error_report(err) and exit(1), at least can > let user know why fail. see the call-chain: > > pc_xen_hvm_init_pci->pc_init1->i440fx_init->pci_create_simple-> pci_create_simple_multifunction->qdev_init_nofail unlike open failing, there's nothing user can do, this means there's a coding bug somewhere, and assert is easier to debug. > About test: > 1. Compiled > 2. Did a dirty hack to force create/realize this device in pc_init1(), prove > that the realizing process is ok, I will reply this mail later to attch the > screenshot evidence Good job, thanks! You should also Cc Tiejun Chen <tiejun.chen@intel.com> who wrote this code, presumably with access to hardware to test on. > hw/pci-host/piix.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > index a9cb983..e91570f 100644 > --- a/hw/pci-host/piix.c > +++ b/hw/pci-host/piix.c > @@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = { > {0xa8, 4}, /* SNB: base of GTT stolen memory */ > }; > > -static int host_pci_config_read(int pos, int len, uint32_t *val) > +static void host_pci_config_read(int pos, int len, uint32_t *val, Error **errp) > { > char path[PATH_MAX]; > int config_fd; > @@ -769,19 +769,19 @@ static int host_pci_config_read(int pos, int len, uint32_t *val) > /* Access real host bridge. */ > int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", > 0, 0, 0, 0, "config"); > - int ret = 0; > > if (rc >= size || rc < 0) { > - return -ENODEV; > + error_setg_errno(errp, errno, "snprintf err"); > } > > config_fd = open(path, O_RDWR); > if (config_fd < 0) { > - return -ENODEV; > + error_setg_errno(errp, errno, "open %s fail", path); > + return; > } > > if (lseek(config_fd, pos, SEEK_SET) != pos) { > - ret = -errno; > + error_setg_errno(errp, errno, "lseek err"); > goto out; > } > > @@ -789,32 +789,32 @@ static int host_pci_config_read(int pos, int len, uint32_t *val) > rc = read(config_fd, (uint8_t *)val, len); > } while (rc < 0 && (errno == EINTR || errno == EAGAIN)); > if (rc != len) { > - ret = -errno; > + error_setg_errno(errp, errno, "read err"); > } > > out: > close(config_fd); > - return ret; > } > > -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) > +static void igd_pt_i440fx_realize(struct PCIDevice *pci_dev, Error **errp) > { > uint32_t val = 0; > - int rc, i, num; > + int i, num; > int pos, len; > + Error *local_err = NULL; > > num = ARRAY_SIZE(igd_host_bridge_infos); > for (i = 0; i < num; i++) { > pos = igd_host_bridge_infos[i].offset; > len = igd_host_bridge_infos[i].len; > - rc = host_pci_config_read(pos, len, &val); > - if (rc) { > - return -ENODEV; > + > + host_pci_config_read(pos, len, &val, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > } > pci_default_write_config(pci_dev, pos, cpu_to_le32(val), len); > } > - > - return 0; > } > > static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) > @@ -822,7 +822,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > - k->init = igd_pt_i440fx_initfn; > + k->realize = igd_pt_i440fx_realize; > dc->desc = "IGD Passthrough Host bridge"; > } > > -- > 2.1.0 > >
Agree with your review point. Since can`t get contact with author tiejun.chen@intel.com, maybe only can ask for test help from Stefano, I will send next version when we get in touch with stefano. On 12/29/2015 07:25 PM, Michael S. Tsirkin wrote: > On Tue, Dec 29, 2015 at 04:46:03PM +0800, Cao jin wrote: >> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> >> --- >> changelog v4: >> 1. strip the bugfix code, but I guess this patch should be applied after >> that bugfix patch. >> 2. Other little fix as per previous review >> >> Some explanation to previous review: >> 1. error_setg_errno() I use in this patch will print the strerror(errno), so I >> guess it will be informative enough? example: >> >> error_setg_errno(errp, errno, "open %s fail", path); will print: >> "open bluhbluh fail: string_from_strerrer(errno)" > > I'd prefer some mention of why this is attempted too. > E.g. "igd passthrough : open bluhbluh fail" > >> 2. snprintf() has tiny tiny chance to fail, > > It just formats a string into a buffer. the buffer > is pre-allocated, and we know it's large enough for the data. > How can it fail? > > >> I don`t assert here, because this >> device is on-board, it init during machine initialization, in case it fails, >> the errp we set will results in error_report(err) and exit(1), at least can >> let user know why fail. see the call-chain: >> >> pc_xen_hvm_init_pci->pc_init1->i440fx_init->pci_create_simple-> pci_create_simple_multifunction->qdev_init_nofail > > unlike open failing, there's nothing user can do, this means there's > a coding bug somewhere, and assert is easier to debug. > >> About test: >> 1. Compiled >> 2. Did a dirty hack to force create/realize this device in pc_init1(), prove >> that the realizing process is ok, I will reply this mail later to attch the >> screenshot evidence > > Good job, thanks! > You should also Cc Tiejun Chen <tiejun.chen@intel.com> who wrote this > code, presumably with access to hardware to test on. > > >> hw/pci-host/piix.c | 30 +++++++++++++++--------------- >> 1 file changed, 15 insertions(+), 15 deletions(-) >> >> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c >> index a9cb983..e91570f 100644 >> --- a/hw/pci-host/piix.c >> +++ b/hw/pci-host/piix.c >> @@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = { >> {0xa8, 4}, /* SNB: base of GTT stolen memory */ >> }; >> >> -static int host_pci_config_read(int pos, int len, uint32_t *val) >> +static void host_pci_config_read(int pos, int len, uint32_t *val, Error **errp) >> { >> char path[PATH_MAX]; >> int config_fd; >> @@ -769,19 +769,19 @@ static int host_pci_config_read(int pos, int len, uint32_t *val) >> /* Access real host bridge. */ >> int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", >> 0, 0, 0, 0, "config"); >> - int ret = 0; >> >> if (rc >= size || rc < 0) { >> - return -ENODEV; >> + error_setg_errno(errp, errno, "snprintf err"); >> } >> >> config_fd = open(path, O_RDWR); >> if (config_fd < 0) { >> - return -ENODEV; >> + error_setg_errno(errp, errno, "open %s fail", path); >> + return; >> } >> >> if (lseek(config_fd, pos, SEEK_SET) != pos) { >> - ret = -errno; >> + error_setg_errno(errp, errno, "lseek err"); >> goto out; >> } >> >> @@ -789,32 +789,32 @@ static int host_pci_config_read(int pos, int len, uint32_t *val) >> rc = read(config_fd, (uint8_t *)val, len); >> } while (rc < 0 && (errno == EINTR || errno == EAGAIN)); >> if (rc != len) { >> - ret = -errno; >> + error_setg_errno(errp, errno, "read err"); >> } >> >> out: >> close(config_fd); >> - return ret; >> } >> >> -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) >> +static void igd_pt_i440fx_realize(struct PCIDevice *pci_dev, Error **errp) >> { >> uint32_t val = 0; >> - int rc, i, num; >> + int i, num; >> int pos, len; >> + Error *local_err = NULL; >> >> num = ARRAY_SIZE(igd_host_bridge_infos); >> for (i = 0; i < num; i++) { >> pos = igd_host_bridge_infos[i].offset; >> len = igd_host_bridge_infos[i].len; >> - rc = host_pci_config_read(pos, len, &val); >> - if (rc) { >> - return -ENODEV; >> + >> + host_pci_config_read(pos, len, &val, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return; >> } >> pci_default_write_config(pci_dev, pos, cpu_to_le32(val), len); >> } >> - >> - return 0; >> } >> >> static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) >> @@ -822,7 +822,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) >> DeviceClass *dc = DEVICE_CLASS(klass); >> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >> >> - k->init = igd_pt_i440fx_initfn; >> + k->realize = igd_pt_i440fx_realize; >> dc->desc = "IGD Passthrough Host bridge"; >> } >> >> -- >> 2.1.0 >> >> > > > . >
Unfortunately I don't have a setup to test this either. Maybe Lars can find out who should be involved on the Intel side on this. In any case, the patch looks good to me. It would be nice to get feedback from somebody at Intel before applying it, but if we don't get any to your next version of the patch, I'll probably apply it anyway to my next branch. On Tue, 29 Dec 2015, Cao jin wrote: > Agree with your review point. Since can`t get contact with author > tiejun.chen@intel.com, maybe only can ask for test help from Stefano, I will > send next version when we get in touch with stefano. > > On 12/29/2015 07:25 PM, Michael S. Tsirkin wrote: > > On Tue, Dec 29, 2015 at 04:46:03PM +0800, Cao jin wrote: > > > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > > > --- > > > changelog v4: > > > 1. strip the bugfix code, but I guess this patch should be applied after > > > that bugfix patch. > > > 2. Other little fix as per previous review > > > > > > Some explanation to previous review: > > > 1. error_setg_errno() I use in this patch will print the strerror(errno), > > > so I > > > guess it will be informative enough? example: > > > > > > error_setg_errno(errp, errno, "open %s fail", path); will print: > > > "open bluhbluh fail: string_from_strerrer(errno)" > > > > I'd prefer some mention of why this is attempted too. > > E.g. "igd passthrough : open bluhbluh fail" > > > > > 2. snprintf() has tiny tiny chance to fail, > > > > It just formats a string into a buffer. the buffer > > is pre-allocated, and we know it's large enough for the data. > > How can it fail? > > > > > > > I don`t assert here, because this > > > device is on-board, it init during machine initialization, in case it > > > fails, > > > the errp we set will results in error_report(err) and exit(1), at > > > least can > > > let user know why fail. see the call-chain: > > > > > > pc_xen_hvm_init_pci->pc_init1->i440fx_init->pci_create_simple-> > > > pci_create_simple_multifunction->qdev_init_nofail > > > > unlike open failing, there's nothing user can do, this means there's > > a coding bug somewhere, and assert is easier to debug. > > > > > About test: > > > 1. Compiled > > > 2. Did a dirty hack to force create/realize this device in pc_init1(), > > > prove > > > that the realizing process is ok, I will reply this mail later to > > > attch the > > > screenshot evidence > > > > Good job, thanks! > > You should also Cc Tiejun Chen <tiejun.chen@intel.com> who wrote this > > code, presumably with access to hardware to test on. > > > > > > > hw/pci-host/piix.c | 30 +++++++++++++++--------------- > > > 1 file changed, 15 insertions(+), 15 deletions(-) > > > > > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > > > index a9cb983..e91570f 100644 > > > --- a/hw/pci-host/piix.c > > > +++ b/hw/pci-host/piix.c > > > @@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = { > > > {0xa8, 4}, /* SNB: base of GTT stolen memory */ > > > }; > > > > > > -static int host_pci_config_read(int pos, int len, uint32_t *val) > > > +static void host_pci_config_read(int pos, int len, uint32_t *val, Error > > > **errp) > > > { > > > char path[PATH_MAX]; > > > int config_fd; > > > @@ -769,19 +769,19 @@ static int host_pci_config_read(int pos, int len, > > > uint32_t *val) > > > /* Access real host bridge. */ > > > int rc = snprintf(path, size, > > > "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", > > > 0, 0, 0, 0, "config"); > > > - int ret = 0; > > > > > > if (rc >= size || rc < 0) { > > > - return -ENODEV; > > > + error_setg_errno(errp, errno, "snprintf err"); > > > } > > > > > > config_fd = open(path, O_RDWR); > > > if (config_fd < 0) { > > > - return -ENODEV; > > > + error_setg_errno(errp, errno, "open %s fail", path); > > > + return; > > > } > > > > > > if (lseek(config_fd, pos, SEEK_SET) != pos) { > > > - ret = -errno; > > > + error_setg_errno(errp, errno, "lseek err"); > > > goto out; > > > } > > > > > > @@ -789,32 +789,32 @@ static int host_pci_config_read(int pos, int len, > > > uint32_t *val) > > > rc = read(config_fd, (uint8_t *)val, len); > > > } while (rc < 0 && (errno == EINTR || errno == EAGAIN)); > > > if (rc != len) { > > > - ret = -errno; > > > + error_setg_errno(errp, errno, "read err"); > > > } > > > > > > out: > > > close(config_fd); > > > - return ret; > > > } > > > > > > -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) > > > +static void igd_pt_i440fx_realize(struct PCIDevice *pci_dev, Error > > > **errp) > > > { > > > uint32_t val = 0; > > > - int rc, i, num; > > > + int i, num; > > > int pos, len; > > > + Error *local_err = NULL; > > > > > > num = ARRAY_SIZE(igd_host_bridge_infos); > > > for (i = 0; i < num; i++) { > > > pos = igd_host_bridge_infos[i].offset; > > > len = igd_host_bridge_infos[i].len; > > > - rc = host_pci_config_read(pos, len, &val); > > > - if (rc) { > > > - return -ENODEV; > > > + > > > + host_pci_config_read(pos, len, &val, &local_err); > > > + if (local_err) { > > > + error_propagate(errp, local_err); > > > + return; > > > } > > > pci_default_write_config(pci_dev, pos, cpu_to_le32(val), len); > > > } > > > - > > > - return 0; > > > } > > > > > > static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void > > > *data) > > > @@ -822,7 +822,7 @@ static void > > > igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) > > > DeviceClass *dc = DEVICE_CLASS(klass); > > > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > > > > > - k->init = igd_pt_i440fx_initfn; > > > + k->realize = igd_pt_i440fx_realize; > > > dc->desc = "IGD Passthrough Host bridge"; > > > } > > > > > > -- > > > 2.1.0 > > > > > > > > > > > > . > > > > -- > Yours Sincerely, > > Cao Jin > >
On 04/01/2016 14:47, "Stefano Stabellini" <stefano.stabellini@eu.citrix.com> wrote: >Unfortunately I don't have a setup to test this either. Maybe Lars can >find out who should be involved on the Intel side on this. I can certainly help to this and get back to you. What exactly are we asking Intel to do? It is not clear to me from this email thread Regards Lars
On Mon, 4 Jan 2016, Lars Kurth wrote: > On 04/01/2016 14:47, "Stefano Stabellini" > <stefano.stabellini@eu.citrix.com> wrote: > > >Unfortunately I don't have a setup to test this either. Maybe Lars can > >find out who should be involved on the Intel side on this. > > I can certainly help to this and get back to you. What exactly are we > asking Intel to do? > It is not clear to me from this email thread Tiejun Chen, the author of the Intel graphic card passthrough patches for QEMU, seems to have left the company. It would be nice if somebody else tested this patch with an intel graphic card assigned to a guest VM.
Hi folks, let me introduce you to Xudong from Intel, who is willing to help out. Best Regards Lars > On 4 Jan 2016, at 15:41, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > > On Mon, 4 Jan 2016, Lars Kurth wrote: >> On 04/01/2016 14:47, "Stefano Stabellini" >> <stefano.stabellini@eu.citrix.com> wrote: >> >>> Unfortunately I don't have a setup to test this either. Maybe Lars can >>> find out who should be involved on the Intel side on this. >> >> I can certainly help to this and get back to you. What exactly are we >> asking Intel to do? >> It is not clear to me from this email thread > > Tiejun Chen, the author of the Intel graphic card passthrough patches > for QEMU, seems to have left the company. It would be nice if somebody > else tested this patch with an intel graphic card assigned to a guest > VM. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Hello Xudong, please test this patch: http://marc.info/?l=qemu-devel&m=145137863501079 with an intel graphic card assigned to a Xen guest. If everything still works as expected, please reply with your Tested-by. Thanks, Stefano On Wed, 6 Jan 2016, Lars Kurth wrote: > Hi folks, > let me introduce you to Xudong from Intel, who is willing to help out. > Best Regards > Lars > > > On 4 Jan 2016, at 15:41, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > > > > On Mon, 4 Jan 2016, Lars Kurth wrote: > >> On 04/01/2016 14:47, "Stefano Stabellini" > >> <stefano.stabellini@eu.citrix.com> wrote: > >> > >>> Unfortunately I don't have a setup to test this either. Maybe Lars can > >>> find out who should be involved on the Intel side on this. > >> > >> I can certainly help to this and get back to you. What exactly are we > >> asking Intel to do? > >> It is not clear to me from this email thread > > > > Tiejun Chen, the author of the Intel graphic card passthrough patches > > for QEMU, seems to have left the company. It would be nice if somebody > > else tested this patch with an intel graphic card assigned to a guest > > VM. > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel >
Sure. I'll test it soon. Thanks, -Xudong > -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: Wednesday, January 6, 2016 8:18 PM > To: Lars Kurth <lars.kurth.xen@gmail.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Hao, Xudong > <xudong.hao@intel.com>; Lars Kurth <lars.kurth@citrix.com>; Cao jin > <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; Stefano Stabellini > <Stefano.Stabellini@citrix.com>; qemu-devel@nongnu.org; Michael S. Tsirkin > <mst@redhat.com> > Subject: Re: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert to realize() > > Hello Xudong, > > please test this patch: > > http://marc.info/?l=qemu-devel&m=145137863501079 > > with an intel graphic card assigned to a Xen guest. If everything still works as > expected, please reply with your Tested-by. > > Thanks, > > Stefano > > On Wed, 6 Jan 2016, Lars Kurth wrote: > > Hi folks, > > let me introduce you to Xudong from Intel, who is willing to help out. > > Best Regards > > Lars > > > > > On 4 Jan 2016, at 15:41, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > wrote: > > > > > > On Mon, 4 Jan 2016, Lars Kurth wrote: > > >> On 04/01/2016 14:47, "Stefano Stabellini" > > >> <stefano.stabellini@eu.citrix.com> wrote: > > >> > > >>> Unfortunately I don't have a setup to test this either. Maybe Lars > > >>> can find out who should be involved on the Intel side on this. > > >> > > >> I can certainly help to this and get back to you. What exactly are > > >> we asking Intel to do? > > >> It is not clear to me from this email thread > > > > > > Tiejun Chen, the author of the Intel graphic card passthrough > > > patches for QEMU, seems to have left the company. It would be nice > > > if somebody else tested this patch with an intel graphic card > > > assigned to a guest VM. > > > > > > _______________________________________________ > > > Xen-devel mailing list > > > Xen-devel@lists.xen.org > > > http://lists.xen.org/xen-devel > >
Hi
Since Gerd has a whole patchset that covers this, Shall I continue
with a new version of this?
On 12/29/2015 07:25 PM, Michael S. Tsirkin wrote:
> On Tue, Dec 29, 2015 at 04:46:03PM +0800, Cao jin wrote:
>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>> ---
>> changelog v4:
>> 1. strip the bugfix code, but I guess this patch should be applied after
>> that bugfix patch.
>> 2. Other little fix as per previous review
>>
>> Some explanation to previous review:
>> 1. error_setg_errno() I use in this patch will print the strerror(errno), so I
>> guess it will be informative enough? example:
>>
>> error_setg_errno(errp, errno, "open %s fail", path); will print:
>> "open bluhbluh fail: string_from_strerrer(errno)"
>
> I'd prefer some mention of why this is attempted too.
> E.g. "igd passthrough : open bluhbluh fail"
>
>> 2. snprintf() has tiny tiny chance to fail,
>
> It just formats a string into a buffer. the buffer
> is pre-allocated, and we know it's large enough for the data.
> How can it fail?
>
>
>> I don`t assert here, because this
>> device is on-board, it init during machine initialization, in case it fails,
>> the errp we set will results in error_report(err) and exit(1), at least can
>> let user know why fail. see the call-chain:
>>
>> pc_xen_hvm_init_pci->pc_init1->i440fx_init->pci_create_simple-> pci_create_simple_multifunction->qdev_init_nofail
>
> unlike open failing, there's nothing user can do, this means there's
> a coding bug somewhere, and assert is easier to debug.
>
>> About test:
>> 1. Compiled
>> 2. Did a dirty hack to force create/realize this device in pc_init1(), prove
>> that the realizing process is ok, I will reply this mail later to attch the
>> screenshot evidence
>
> Good job, thanks!
> You should also Cc Tiejun Chen <tiejun.chen@intel.com> who wrote this
> code, presumably with access to hardware to test on.
>
>
>> hw/pci-host/piix.c | 30 +++++++++++++++---------------
>> 1 file changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index a9cb983..e91570f 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
>> {0xa8, 4}, /* SNB: base of GTT stolen memory */
>> };
>>
>> -static int host_pci_config_read(int pos, int len, uint32_t *val)
>> +static void host_pci_config_read(int pos, int len, uint32_t *val, Error **errp)
>> {
>> char path[PATH_MAX];
>> int config_fd;
>> @@ -769,19 +769,19 @@ static int host_pci_config_read(int pos, int len, uint32_t *val)
>> /* Access real host bridge. */
>> int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
>> 0, 0, 0, 0, "config");
>> - int ret = 0;
>>
>> if (rc >= size || rc < 0) {
>> - return -ENODEV;
>> + error_setg_errno(errp, errno, "snprintf err");
>> }
>>
>> config_fd = open(path, O_RDWR);
>> if (config_fd < 0) {
>> - return -ENODEV;
>> + error_setg_errno(errp, errno, "open %s fail", path);
>> + return;
>> }
>>
>> if (lseek(config_fd, pos, SEEK_SET) != pos) {
>> - ret = -errno;
>> + error_setg_errno(errp, errno, "lseek err");
>> goto out;
>> }
>>
>> @@ -789,32 +789,32 @@ static int host_pci_config_read(int pos, int len, uint32_t *val)
>> rc = read(config_fd, (uint8_t *)val, len);
>> } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
>> if (rc != len) {
>> - ret = -errno;
>> + error_setg_errno(errp, errno, "read err");
>> }
>>
>> out:
>> close(config_fd);
>> - return ret;
>> }
>>
>> -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
>> +static void igd_pt_i440fx_realize(struct PCIDevice *pci_dev, Error **errp)
>> {
>> uint32_t val = 0;
>> - int rc, i, num;
>> + int i, num;
>> int pos, len;
>> + Error *local_err = NULL;
>>
>> num = ARRAY_SIZE(igd_host_bridge_infos);
>> for (i = 0; i < num; i++) {
>> pos = igd_host_bridge_infos[i].offset;
>> len = igd_host_bridge_infos[i].len;
>> - rc = host_pci_config_read(pos, len, &val);
>> - if (rc) {
>> - return -ENODEV;
>> +
>> + host_pci_config_read(pos, len, &val, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + return;
>> }
>> pci_default_write_config(pci_dev, pos, cpu_to_le32(val), len);
>> }
>> -
>> - return 0;
>> }
>>
>> static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
>> @@ -822,7 +822,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
>> DeviceClass *dc = DEVICE_CLASS(klass);
>> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>>
>> - k->init = igd_pt_i440fx_initfn;
>> + k->realize = igd_pt_i440fx_realize;
>> dc->desc = "IGD Passthrough Host bridge";
>> }
>>
>> --
>> 2.1.0
>>
>>
>
>
> .
>
Your patch is simpler and more mature. Maybe it should go in before Gerd's series? On Thu, 7 Jan 2016, Cao jin wrote: > Hi > Since Gerd has a whole patchset that covers this, Shall I continue with a > new version of this? > > On 12/29/2015 07:25 PM, Michael S. Tsirkin wrote: > > On Tue, Dec 29, 2015 at 04:46:03PM +0800, Cao jin wrote: > > > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > > > --- > > > changelog v4: > > > 1. strip the bugfix code, but I guess this patch should be applied after > > > that bugfix patch. > > > 2. Other little fix as per previous review > > > > > > Some explanation to previous review: > > > 1. error_setg_errno() I use in this patch will print the strerror(errno), > > > so I > > > guess it will be informative enough? example: > > > > > > error_setg_errno(errp, errno, "open %s fail", path); will print: > > > "open bluhbluh fail: string_from_strerrer(errno)" > > > > I'd prefer some mention of why this is attempted too. > > E.g. "igd passthrough : open bluhbluh fail" > > > > > 2. snprintf() has tiny tiny chance to fail, > > > > It just formats a string into a buffer. the buffer > > is pre-allocated, and we know it's large enough for the data. > > How can it fail? > > > > > > > I don`t assert here, because this > > > device is on-board, it init during machine initialization, in case it > > > fails, > > > the errp we set will results in error_report(err) and exit(1), at > > > least can > > > let user know why fail. see the call-chain: > > > > > > pc_xen_hvm_init_pci->pc_init1->i440fx_init->pci_create_simple-> > > > pci_create_simple_multifunction->qdev_init_nofail > > > > unlike open failing, there's nothing user can do, this means there's > > a coding bug somewhere, and assert is easier to debug. > > > > > About test: > > > 1. Compiled > > > 2. Did a dirty hack to force create/realize this device in pc_init1(), > > > prove > > > that the realizing process is ok, I will reply this mail later to > > > attch the > > > screenshot evidence > > > > Good job, thanks! > > You should also Cc Tiejun Chen <tiejun.chen@intel.com> who wrote this > > code, presumably with access to hardware to test on. > > > > > > > hw/pci-host/piix.c | 30 +++++++++++++++--------------- > > > 1 file changed, 15 insertions(+), 15 deletions(-) > > > > > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c > > > index a9cb983..e91570f 100644 > > > --- a/hw/pci-host/piix.c > > > +++ b/hw/pci-host/piix.c > > > @@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = { > > > {0xa8, 4}, /* SNB: base of GTT stolen memory */ > > > }; > > > > > > -static int host_pci_config_read(int pos, int len, uint32_t *val) > > > +static void host_pci_config_read(int pos, int len, uint32_t *val, Error > > > **errp) > > > { > > > char path[PATH_MAX]; > > > int config_fd; > > > @@ -769,19 +769,19 @@ static int host_pci_config_read(int pos, int len, > > > uint32_t *val) > > > /* Access real host bridge. */ > > > int rc = snprintf(path, size, > > > "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", > > > 0, 0, 0, 0, "config"); > > > - int ret = 0; > > > > > > if (rc >= size || rc < 0) { > > > - return -ENODEV; > > > + error_setg_errno(errp, errno, "snprintf err"); > > > } > > > > > > config_fd = open(path, O_RDWR); > > > if (config_fd < 0) { > > > - return -ENODEV; > > > + error_setg_errno(errp, errno, "open %s fail", path); > > > + return; > > > } > > > > > > if (lseek(config_fd, pos, SEEK_SET) != pos) { > > > - ret = -errno; > > > + error_setg_errno(errp, errno, "lseek err"); > > > goto out; > > > } > > > > > > @@ -789,32 +789,32 @@ static int host_pci_config_read(int pos, int len, > > > uint32_t *val) > > > rc = read(config_fd, (uint8_t *)val, len); > > > } while (rc < 0 && (errno == EINTR || errno == EAGAIN)); > > > if (rc != len) { > > > - ret = -errno; > > > + error_setg_errno(errp, errno, "read err"); > > > } > > > > > > out: > > > close(config_fd); > > > - return ret; > > > } > > > > > > -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) > > > +static void igd_pt_i440fx_realize(struct PCIDevice *pci_dev, Error > > > **errp) > > > { > > > uint32_t val = 0; > > > - int rc, i, num; > > > + int i, num; > > > int pos, len; > > > + Error *local_err = NULL; > > > > > > num = ARRAY_SIZE(igd_host_bridge_infos); > > > for (i = 0; i < num; i++) { > > > pos = igd_host_bridge_infos[i].offset; > > > len = igd_host_bridge_infos[i].len; > > > - rc = host_pci_config_read(pos, len, &val); > > > - if (rc) { > > > - return -ENODEV; > > > + > > > + host_pci_config_read(pos, len, &val, &local_err); > > > + if (local_err) { > > > + error_propagate(errp, local_err); > > > + return; > > > } > > > pci_default_write_config(pci_dev, pos, cpu_to_le32(val), len); > > > } > > > - > > > - return 0; > > > } > > > > > > static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void > > > *data) > > > @@ -822,7 +822,7 @@ static void > > > igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) > > > DeviceClass *dc = DEVICE_CLASS(klass); > > > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > > > > > - k->init = igd_pt_i440fx_initfn; > > > + k->realize = igd_pt_i440fx_realize; > > > dc->desc = "IGD Passthrough Host bridge"; > > > } > > > > > > -- > > > 2.1.0 > > > > > > > > > > > > . > > > > -- > Yours Sincerely, > > Cao jin > > >
On Do, 2016-01-07 at 15:01 +0000, Stefano Stabellini wrote: > Your patch is simpler and more mature. Maybe it should go in before Gerd's > series? My series would throw away most of the error handling fixes here, so IMHO that would be pointless churn ... And I don't think my series is far from being mergeable. The new patch #11 (added in v3) is the most problematic one and possibly requires some coordination with xen (depending on how we decide to tackle the problems). But we can split the series up and go ahead merging patches 1-10. cheers, Gerd
On Thu, 7 Jan 2016, Gerd Hoffmann wrote: > On Do, 2016-01-07 at 15:01 +0000, Stefano Stabellini wrote: > > Your patch is simpler and more mature. Maybe it should go in before Gerd's > > series? > > My series would throw away most of the error handling fixes here, so > IMHO that would be pointless churn ... > > And I don't think my series is far from being mergeable. The new patch > #11 (added in v3) is the most problematic one and possibly requires some > coordination with xen (depending on how we decide to tackle the > problems). But we can split the series up and go ahead merging patches > 1-10. That's OK for me.
Since you are at it, could you please let me know how well igd passthrough works without this bugfix: http://marc.info/?l=qemu-devel&m=145172165010604 which is about to land in QEMU. I guess it doesn't work at all? I am asking because I would like to know the level of support we need to provide to igd passthrough with the latest QEMU release (2.5). On Thu, 7 Jan 2016, Hao, Xudong wrote: > Sure. I'll test it soon. > > Thanks, > -Xudong > > > -----Original Message----- > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > Sent: Wednesday, January 6, 2016 8:18 PM > > To: Lars Kurth <lars.kurth.xen@gmail.com> > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Hao, Xudong > > <xudong.hao@intel.com>; Lars Kurth <lars.kurth@citrix.com>; Cao jin > > <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; Stefano Stabellini > > <Stefano.Stabellini@citrix.com>; qemu-devel@nongnu.org; Michael S. Tsirkin > > <mst@redhat.com> > > Subject: Re: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert to realize() > > > > Hello Xudong, > > > > please test this patch: > > > > http://marc.info/?l=qemu-devel&m=145137863501079 > > > > with an intel graphic card assigned to a Xen guest. If everything still works as > > expected, please reply with your Tested-by. > > > > Thanks, > > > > Stefano > > > > On Wed, 6 Jan 2016, Lars Kurth wrote: > > > Hi folks, > > > let me introduce you to Xudong from Intel, who is willing to help out. > > > Best Regards > > > Lars > > > > > > > On 4 Jan 2016, at 15:41, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > wrote: > > > > > > > > On Mon, 4 Jan 2016, Lars Kurth wrote: > > > >> On 04/01/2016 14:47, "Stefano Stabellini" > > > >> <stefano.stabellini@eu.citrix.com> wrote: > > > >> > > > >>> Unfortunately I don't have a setup to test this either. Maybe Lars > > > >>> can find out who should be involved on the Intel side on this. > > > >> > > > >> I can certainly help to this and get back to you. What exactly are > > > >> we asking Intel to do? > > > >> It is not clear to me from this email thread > > > > > > > > Tiejun Chen, the author of the Intel graphic card passthrough > > > > patches for QEMU, seems to have left the company. It would be nice > > > > if somebody else tested this patch with an intel graphic card > > > > assigned to a guest VM. > > > > > > > > _______________________________________________ > > > > Xen-devel mailing list > > > > Xen-devel@lists.xen.org > > > > http://lists.xen.org/xen-devel > > > >
Qemu with the patch can't boot VM with IGD pass-through, I'm checking if it works w/o this patch to eliminate the environment influence. Thanks, -Xudong > -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: Friday, January 8, 2016 7:57 PM > To: Hao, Xudong <xudong.hao@intel.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Lars Kurth > <lars.kurth.xen@gmail.com>; Lars Kurth <lars.kurth@citrix.com>; Cao jin > <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; Stefano Stabellini > <Stefano.Stabellini@citrix.com>; qemu-devel@nongnu.org; Michael S. Tsirkin > <mst@redhat.com> > Subject: RE: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert to realize() > > Since you are at it, could you please let me know how well igd passthrough > works without this bugfix: > > http://marc.info/?l=qemu-devel&m=145172165010604 > > which is about to land in QEMU. I guess it doesn't work at all? > > I am asking because I would like to know the level of support we need to provide > to igd passthrough with the latest QEMU release (2.5). > > > On Thu, 7 Jan 2016, Hao, Xudong wrote: > > Sure. I'll test it soon. > > > > Thanks, > > -Xudong > > > > > -----Original Message----- > > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > > Sent: Wednesday, January 6, 2016 8:18 PM > > > To: Lars Kurth <lars.kurth.xen@gmail.com> > > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Hao, > > > Xudong <xudong.hao@intel.com>; Lars Kurth <lars.kurth@citrix.com>; > > > Cao jin <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; > > > Stefano Stabellini <Stefano.Stabellini@citrix.com>; > > > qemu-devel@nongnu.org; Michael S. Tsirkin <mst@redhat.com> > > > Subject: Re: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert > > > to realize() > > > > > > Hello Xudong, > > > > > > please test this patch: > > > > > > http://marc.info/?l=qemu-devel&m=145137863501079 > > > > > > with an intel graphic card assigned to a Xen guest. If everything > > > still works as expected, please reply with your Tested-by. > > > > > > Thanks, > > > > > > Stefano > > > > > > On Wed, 6 Jan 2016, Lars Kurth wrote: > > > > Hi folks, > > > > let me introduce you to Xudong from Intel, who is willing to help out. > > > > Best Regards > > > > Lars > > > > > > > > > On 4 Jan 2016, at 15:41, Stefano Stabellini > > > > > <stefano.stabellini@eu.citrix.com> > > > wrote: > > > > > > > > > > On Mon, 4 Jan 2016, Lars Kurth wrote: > > > > >> On 04/01/2016 14:47, "Stefano Stabellini" > > > > >> <stefano.stabellini@eu.citrix.com> wrote: > > > > >> > > > > >>> Unfortunately I don't have a setup to test this either. Maybe > > > > >>> Lars can find out who should be involved on the Intel side on this. > > > > >> > > > > >> I can certainly help to this and get back to you. What exactly > > > > >> are we asking Intel to do? > > > > >> It is not clear to me from this email thread > > > > > > > > > > Tiejun Chen, the author of the Intel graphic card passthrough > > > > > patches for QEMU, seems to have left the company. It would be > > > > > nice if somebody else tested this patch with an intel graphic > > > > > card assigned to a guest VM. > > > > > > > > > > _______________________________________________ > > > > > Xen-devel mailing list > > > > > Xen-devel@lists.xen.org > > > > > http://lists.xen.org/xen-devel > > > > > >
Stefano, Patch http://marc.info/?l=qemu-devel&m=145137863501079 don't works for qemu at all, some conflict when git apply. Patch http://marc.info/?l=qemu-devel&m=145137863501079 is based on patch http://marc.info/?l=qemu-devel&m=145172165010604, right? I can boot up Linux VM with IGD pass-through with latest qemu (without any additional patch), guest run 3D "nexuiz" and get 180fps. Will try the two patch together later. Thanks, -Xudong > -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: Friday, January 8, 2016 7:57 PM > To: Hao, Xudong <xudong.hao@intel.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Lars Kurth > <lars.kurth.xen@gmail.com>; Lars Kurth <lars.kurth@citrix.com>; Cao jin > <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; Stefano Stabellini > <Stefano.Stabellini@citrix.com>; qemu-devel@nongnu.org; Michael S. Tsirkin > <mst@redhat.com> > Subject: RE: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert to realize() > > Since you are at it, could you please let me know how well igd passthrough > works without this bugfix: > > http://marc.info/?l=qemu-devel&m=145172165010604 > > which is about to land in QEMU. I guess it doesn't work at all? > > I am asking because I would like to know the level of support we need to provide > to igd passthrough with the latest QEMU release (2.5). > > > On Thu, 7 Jan 2016, Hao, Xudong wrote: > > Sure. I'll test it soon. > > > > Thanks, > > -Xudong > > > > > -----Original Message----- > > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > > Sent: Wednesday, January 6, 2016 8:18 PM > > > To: Lars Kurth <lars.kurth.xen@gmail.com> > > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Hao, > > > Xudong <xudong.hao@intel.com>; Lars Kurth <lars.kurth@citrix.com>; > > > Cao jin <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; > > > Stefano Stabellini <Stefano.Stabellini@citrix.com>; > > > qemu-devel@nongnu.org; Michael S. Tsirkin <mst@redhat.com> > > > Subject: Re: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert > > > to realize() > > > > > > Hello Xudong, > > > > > > please test this patch: > > > > > > http://marc.info/?l=qemu-devel&m=145137863501079 > > > > > > with an intel graphic card assigned to a Xen guest. If everything > > > still works as expected, please reply with your Tested-by. > > > > > > Thanks, > > > > > > Stefano > > > > > > On Wed, 6 Jan 2016, Lars Kurth wrote: > > > > Hi folks, > > > > let me introduce you to Xudong from Intel, who is willing to help out. > > > > Best Regards > > > > Lars > > > > > > > > > On 4 Jan 2016, at 15:41, Stefano Stabellini > > > > > <stefano.stabellini@eu.citrix.com> > > > wrote: > > > > > > > > > > On Mon, 4 Jan 2016, Lars Kurth wrote: > > > > >> On 04/01/2016 14:47, "Stefano Stabellini" > > > > >> <stefano.stabellini@eu.citrix.com> wrote: > > > > >> > > > > >>> Unfortunately I don't have a setup to test this either. Maybe > > > > >>> Lars can find out who should be involved on the Intel side on this. > > > > >> > > > > >> I can certainly help to this and get back to you. What exactly > > > > >> are we asking Intel to do? > > > > >> It is not clear to me from this email thread > > > > > > > > > > Tiejun Chen, the author of the Intel graphic card passthrough > > > > > patches for QEMU, seems to have left the company. It would be > > > > > nice if somebody else tested this patch with an intel graphic > > > > > card assigned to a guest VM. > > > > > > > > > > _______________________________________________ > > > > > Xen-devel mailing list > > > > > Xen-devel@lists.xen.org > > > > > http://lists.xen.org/xen-devel > > > > > >
Hi, > I can boot up Linux VM with IGD pass-through with latest qemu (without > any additional patch), guest run 3D "nexuiz" and get 180fps. That is a pretty recent linux guest I assume? Tried older kernels too, possibly even the old userspace xorg driver? Do windows guest work as well? cheers, Gerd
On Mon, 11 Jan 2016, Hao, Xudong wrote: > Stefano, > > Patch http://marc.info/?l=qemu-devel&m=145137863501079 don't works for qemu at all, some conflict when git apply. > Patch http://marc.info/?l=qemu-devel&m=145137863501079 is based on patch http://marc.info/?l=qemu-devel&m=145172165010604, right? > > I can boot up Linux VM with IGD pass-through with latest qemu (without any additional patch), guest run 3D "nexuiz" and get 180fps. Very interesting, thanks for testing. > Will try the two patch together later. That would be useful > > -----Original Message----- > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > Sent: Friday, January 8, 2016 7:57 PM > > To: Hao, Xudong <xudong.hao@intel.com> > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Lars Kurth > > <lars.kurth.xen@gmail.com>; Lars Kurth <lars.kurth@citrix.com>; Cao jin > > <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; Stefano Stabellini > > <Stefano.Stabellini@citrix.com>; qemu-devel@nongnu.org; Michael S. Tsirkin > > <mst@redhat.com> > > Subject: RE: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert to realize() > > > > Since you are at it, could you please let me know how well igd passthrough > > works without this bugfix: > > > > http://marc.info/?l=qemu-devel&m=145172165010604 > > > > which is about to land in QEMU. I guess it doesn't work at all? > > > > I am asking because I would like to know the level of support we need to provide > > to igd passthrough with the latest QEMU release (2.5). > > > > > > On Thu, 7 Jan 2016, Hao, Xudong wrote: > > > Sure. I'll test it soon. > > > > > > Thanks, > > > -Xudong > > > > > > > -----Original Message----- > > > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > > > Sent: Wednesday, January 6, 2016 8:18 PM > > > > To: Lars Kurth <lars.kurth.xen@gmail.com> > > > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Hao, > > > > Xudong <xudong.hao@intel.com>; Lars Kurth <lars.kurth@citrix.com>; > > > > Cao jin <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; > > > > Stefano Stabellini <Stefano.Stabellini@citrix.com>; > > > > qemu-devel@nongnu.org; Michael S. Tsirkin <mst@redhat.com> > > > > Subject: Re: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert > > > > to realize() > > > > > > > > Hello Xudong, > > > > > > > > please test this patch: > > > > > > > > http://marc.info/?l=qemu-devel&m=145137863501079 > > > > > > > > with an intel graphic card assigned to a Xen guest. If everything > > > > still works as expected, please reply with your Tested-by. > > > > > > > > Thanks, > > > > > > > > Stefano > > > > > > > > On Wed, 6 Jan 2016, Lars Kurth wrote: > > > > > Hi folks, > > > > > let me introduce you to Xudong from Intel, who is willing to help out. > > > > > Best Regards > > > > > Lars > > > > > > > > > > > On 4 Jan 2016, at 15:41, Stefano Stabellini > > > > > > <stefano.stabellini@eu.citrix.com> > > > > wrote: > > > > > > > > > > > > On Mon, 4 Jan 2016, Lars Kurth wrote: > > > > > >> On 04/01/2016 14:47, "Stefano Stabellini" > > > > > >> <stefano.stabellini@eu.citrix.com> wrote: > > > > > >> > > > > > >>> Unfortunately I don't have a setup to test this either. Maybe > > > > > >>> Lars can find out who should be involved on the Intel side on this. > > > > > >> > > > > > >> I can certainly help to this and get back to you. What exactly > > > > > >> are we asking Intel to do? > > > > > >> It is not clear to me from this email thread > > > > > > > > > > > > Tiejun Chen, the author of the Intel graphic card passthrough > > > > > > patches for QEMU, seems to have left the company. It would be > > > > > > nice if somebody else tested this patch with an intel graphic > > > > > > card assigned to a guest VM. > > > > > > > > > > > > _______________________________________________ > > > > > > Xen-devel mailing list > > > > > > Xen-devel@lists.xen.org > > > > > > http://lists.xen.org/xen-devel > > > > > > > > >
On Mon, Jan 11, 2016 at 10:46:20AM +0000, Stefano Stabellini wrote: > On Mon, 11 Jan 2016, Hao, Xudong wrote: > > Stefano, > > > > Patch http://marc.info/?l=qemu-devel&m=145137863501079 don't works for qemu at all, some conflict when git apply. > > Patch http://marc.info/?l=qemu-devel&m=145137863501079 is based on patch http://marc.info/?l=qemu-devel&m=145172165010604, right? > > > > I can boot up Linux VM with IGD pass-through with latest qemu (without any additional patch), guest run 3D "nexuiz" and get 180fps. > > Very interesting, thanks for testing. Could windows VM be tested too please? It might be that the host pci read hacks are only needed for the benefit of the windows guests. > > > Will try the two patch together later. > > That would be useful > > > > > -----Original Message----- > > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > > Sent: Friday, January 8, 2016 7:57 PM > > > To: Hao, Xudong <xudong.hao@intel.com> > > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Lars Kurth > > > <lars.kurth.xen@gmail.com>; Lars Kurth <lars.kurth@citrix.com>; Cao jin > > > <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; Stefano Stabellini > > > <Stefano.Stabellini@citrix.com>; qemu-devel@nongnu.org; Michael S. Tsirkin > > > <mst@redhat.com> > > > Subject: RE: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert to realize() > > > > > > Since you are at it, could you please let me know how well igd passthrough > > > works without this bugfix: > > > > > > http://marc.info/?l=qemu-devel&m=145172165010604 > > > > > > which is about to land in QEMU. I guess it doesn't work at all? > > > > > > I am asking because I would like to know the level of support we need to provide > > > to igd passthrough with the latest QEMU release (2.5). > > > > > > > > > On Thu, 7 Jan 2016, Hao, Xudong wrote: > > > > Sure. I'll test it soon. > > > > > > > > Thanks, > > > > -Xudong > > > > > > > > > -----Original Message----- > > > > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > > > > Sent: Wednesday, January 6, 2016 8:18 PM > > > > > To: Lars Kurth <lars.kurth.xen@gmail.com> > > > > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Hao, > > > > > Xudong <xudong.hao@intel.com>; Lars Kurth <lars.kurth@citrix.com>; > > > > > Cao jin <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; > > > > > Stefano Stabellini <Stefano.Stabellini@citrix.com>; > > > > > qemu-devel@nongnu.org; Michael S. Tsirkin <mst@redhat.com> > > > > > Subject: Re: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert > > > > > to realize() > > > > > > > > > > Hello Xudong, > > > > > > > > > > please test this patch: > > > > > > > > > > http://marc.info/?l=qemu-devel&m=145137863501079 > > > > > > > > > > with an intel graphic card assigned to a Xen guest. If everything > > > > > still works as expected, please reply with your Tested-by. > > > > > > > > > > Thanks, > > > > > > > > > > Stefano > > > > > > > > > > On Wed, 6 Jan 2016, Lars Kurth wrote: > > > > > > Hi folks, > > > > > > let me introduce you to Xudong from Intel, who is willing to help out. > > > > > > Best Regards > > > > > > Lars > > > > > > > > > > > > > On 4 Jan 2016, at 15:41, Stefano Stabellini > > > > > > > <stefano.stabellini@eu.citrix.com> > > > > > wrote: > > > > > > > > > > > > > > On Mon, 4 Jan 2016, Lars Kurth wrote: > > > > > > >> On 04/01/2016 14:47, "Stefano Stabellini" > > > > > > >> <stefano.stabellini@eu.citrix.com> wrote: > > > > > > >> > > > > > > >>> Unfortunately I don't have a setup to test this either. Maybe > > > > > > >>> Lars can find out who should be involved on the Intel side on this. > > > > > > >> > > > > > > >> I can certainly help to this and get back to you. What exactly > > > > > > >> are we asking Intel to do? > > > > > > >> It is not clear to me from this email thread > > > > > > > > > > > > > > Tiejun Chen, the author of the Intel graphic card passthrough > > > > > > > patches for QEMU, seems to have left the company. It would be > > > > > > > nice if somebody else tested this patch with an intel graphic > > > > > > > card assigned to a guest VM. > > > > > > > > > > > > > > _______________________________________________ > > > > > > > Xen-devel mailing list > > > > > > > Xen-devel@lists.xen.org > > > > > > > http://lists.xen.org/xen-devel > > > > > > > > > > > >
Hi On 01/11/2016 05:53 PM, Hao, Xudong wrote: > Stefano, > > Patch http://marc.info/?l=qemu-devel&m=145137863501079 don't works for qemu at all, some conflict when git apply. > Patch http://marc.info/?l=qemu-devel&m=145137863501079 is based on patch http://marc.info/?l=qemu-devel&m=145172165010604, right? > > I can boot up Linux VM with IGD pass-through with latest qemu (without any additional patch), guest run 3D "nexuiz" and get 180fps. > Because commit: 349a3b1cc is already in the latest qemu(maybe the day before yesterday?), so I guess that is why latest qemu works well with igd-passthru and also the same reason for git apply conflict? > Will try the two patch together later. > > Thanks, > -Xudong > > >> -----Original Message----- >> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] >> Sent: Friday, January 8, 2016 7:57 PM >> To: Hao, Xudong <xudong.hao@intel.com> >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Lars Kurth >> <lars.kurth.xen@gmail.com>; Lars Kurth <lars.kurth@citrix.com>; Cao jin >> <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; Stefano Stabellini >> <Stefano.Stabellini@citrix.com>; qemu-devel@nongnu.org; Michael S. Tsirkin >> <mst@redhat.com> >> Subject: RE: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert to realize() >> >> Since you are at it, could you please let me know how well igd passthrough >> works without this bugfix: >> >> http://marc.info/?l=qemu-devel&m=145172165010604 >> >> which is about to land in QEMU. I guess it doesn't work at all? >> >> I am asking because I would like to know the level of support we need to provide >> to igd passthrough with the latest QEMU release (2.5). >> >> >> On Thu, 7 Jan 2016, Hao, Xudong wrote: >>> Sure. I'll test it soon. >>> >>> Thanks, >>> -Xudong >>> >>>> -----Original Message----- >>>> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] >>>> Sent: Wednesday, January 6, 2016 8:18 PM >>>> To: Lars Kurth <lars.kurth.xen@gmail.com> >>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Hao, >>>> Xudong <xudong.hao@intel.com>; Lars Kurth <lars.kurth@citrix.com>; >>>> Cao jin <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; >>>> Stefano Stabellini <Stefano.Stabellini@citrix.com>; >>>> qemu-devel@nongnu.org; Michael S. Tsirkin <mst@redhat.com> >>>> Subject: Re: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert >>>> to realize() >>>> >>>> Hello Xudong, >>>> >>>> please test this patch: >>>> >>>> http://marc.info/?l=qemu-devel&m=145137863501079 >>>> >>>> with an intel graphic card assigned to a Xen guest. If everything >>>> still works as expected, please reply with your Tested-by. >>>> >>>> Thanks, >>>> >>>> Stefano >>>> >>>> On Wed, 6 Jan 2016, Lars Kurth wrote: >>>>> Hi folks, >>>>> let me introduce you to Xudong from Intel, who is willing to help out. >>>>> Best Regards >>>>> Lars >>>>> >>>>>> On 4 Jan 2016, at 15:41, Stefano Stabellini >>>>>> <stefano.stabellini@eu.citrix.com> >>>> wrote: >>>>>> >>>>>> On Mon, 4 Jan 2016, Lars Kurth wrote: >>>>>>> On 04/01/2016 14:47, "Stefano Stabellini" >>>>>>> <stefano.stabellini@eu.citrix.com> wrote: >>>>>>> >>>>>>>> Unfortunately I don't have a setup to test this either. Maybe >>>>>>>> Lars can find out who should be involved on the Intel side on this. >>>>>>> >>>>>>> I can certainly help to this and get back to you. What exactly >>>>>>> are we asking Intel to do? >>>>>>> It is not clear to me from this email thread >>>>>> >>>>>> Tiejun Chen, the author of the Intel graphic card passthrough >>>>>> patches for QEMU, seems to have left the company. It would be >>>>>> nice if somebody else tested this patch with an intel graphic >>>>>> card assigned to a guest VM. >>>>>> >>>>>> _______________________________________________ >>>>>> Xen-devel mailing list >>>>>> Xen-devel@lists.xen.org >>>>>> http://lists.xen.org/xen-devel >>>>> >>> > > > . >
I used 6bb9ead762bf749af11ea225fc2a74db1b93c105 yesterday, this version don't include commit 349a3b1cc. Thanks, -Xudong > -----Original Message----- > From: Cao jin [mailto:caoj.fnst@cn.fujitsu.com] > Sent: Tuesday, January 12, 2016 10:01 AM > To: Hao, Xudong <xudong.hao@intel.com>; Stefano Stabellini > <stefano.stabellini@eu.citrix.com> > Cc: Lars Kurth <lars.kurth.xen@gmail.com>; Lars Kurth <lars.kurth@citrix.com>; > xen-devel@lists.xensource.com; Stefano Stabellini > <Stefano.Stabellini@citrix.com>; qemu-devel@nongnu.org; Michael S. Tsirkin > <mst@redhat.com> > Subject: Re: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert to realize() > > Hi > > On 01/11/2016 05:53 PM, Hao, Xudong wrote: > > Stefano, > > > > Patch http://marc.info/?l=qemu-devel&m=145137863501079 don't works for > qemu at all, some conflict when git apply. > > Patch http://marc.info/?l=qemu-devel&m=145137863501079 is based on > patch http://marc.info/?l=qemu-devel&m=145172165010604, right? > > > > I can boot up Linux VM with IGD pass-through with latest qemu (without any > additional patch), guest run 3D "nexuiz" and get 180fps. > > > > Because commit: 349a3b1cc is already in the latest qemu(maybe the day before > yesterday?), so I guess that is why latest qemu works well with igd-passthru and > also the same reason for git apply conflict? > > > Will try the two patch together later. > > > > Thanks, > > -Xudong > > > > > >> -----Original Message----- > >> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > >> Sent: Friday, January 8, 2016 7:57 PM > >> To: Hao, Xudong <xudong.hao@intel.com> > >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Lars Kurth > >> <lars.kurth.xen@gmail.com>; Lars Kurth <lars.kurth@citrix.com>; Cao > >> jin <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; > >> Stefano Stabellini <Stefano.Stabellini@citrix.com>; > >> qemu-devel@nongnu.org; Michael S. Tsirkin <mst@redhat.com> > >> Subject: RE: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert > >> to realize() > >> > >> Since you are at it, could you please let me know how well igd > >> passthrough works without this bugfix: > >> > >> http://marc.info/?l=qemu-devel&m=145172165010604 > >> > >> which is about to land in QEMU. I guess it doesn't work at all? > >> > >> I am asking because I would like to know the level of support we need > >> to provide to igd passthrough with the latest QEMU release (2.5). > >> > >> > >> On Thu, 7 Jan 2016, Hao, Xudong wrote: > >>> Sure. I'll test it soon. > >>> > >>> Thanks, > >>> -Xudong > >>> > >>>> -----Original Message----- > >>>> From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > >>>> Sent: Wednesday, January 6, 2016 8:18 PM > >>>> To: Lars Kurth <lars.kurth.xen@gmail.com> > >>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Hao, > >>>> Xudong <xudong.hao@intel.com>; Lars Kurth <lars.kurth@citrix.com>; > >>>> Cao jin <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; > >>>> Stefano Stabellini <Stefano.Stabellini@citrix.com>; > >>>> qemu-devel@nongnu.org; Michael S. Tsirkin <mst@redhat.com> > >>>> Subject: Re: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert > >>>> to realize() > >>>> > >>>> Hello Xudong, > >>>> > >>>> please test this patch: > >>>> > >>>> http://marc.info/?l=qemu-devel&m=145137863501079 > >>>> > >>>> with an intel graphic card assigned to a Xen guest. If everything > >>>> still works as expected, please reply with your Tested-by. > >>>> > >>>> Thanks, > >>>> > >>>> Stefano > >>>> > >>>> On Wed, 6 Jan 2016, Lars Kurth wrote: > >>>>> Hi folks, > >>>>> let me introduce you to Xudong from Intel, who is willing to help out. > >>>>> Best Regards > >>>>> Lars > >>>>> > >>>>>> On 4 Jan 2016, at 15:41, Stefano Stabellini > >>>>>> <stefano.stabellini@eu.citrix.com> > >>>> wrote: > >>>>>> > >>>>>> On Mon, 4 Jan 2016, Lars Kurth wrote: > >>>>>>> On 04/01/2016 14:47, "Stefano Stabellini" > >>>>>>> <stefano.stabellini@eu.citrix.com> wrote: > >>>>>>> > >>>>>>>> Unfortunately I don't have a setup to test this either. Maybe > >>>>>>>> Lars can find out who should be involved on the Intel side on this. > >>>>>>> > >>>>>>> I can certainly help to this and get back to you. What exactly > >>>>>>> are we asking Intel to do? > >>>>>>> It is not clear to me from this email thread > >>>>>> > >>>>>> Tiejun Chen, the author of the Intel graphic card passthrough > >>>>>> patches for QEMU, seems to have left the company. It would be > >>>>>> nice if somebody else tested this patch with an intel graphic > >>>>>> card assigned to a guest VM. > >>>>>> > >>>>>> _______________________________________________ > >>>>>> Xen-devel mailing list > >>>>>> Xen-devel@lists.xen.org > >>>>>> http://lists.xen.org/xen-devel > >>>>> > >>> > > > > > > . > > > > -- > Yours Sincerely, > > Cao jin >
Yes, Linux VM update to a 3.18 kernel. The RHEL7.2 default kernel (should be 3.10) VM don't boot up with IGD pass-through, and Windows can't boot up either. Thanks, -Xudong > -----Original Message----- > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > Sent: Monday, January 11, 2016 6:32 PM > To: Hao, Xudong <xudong.hao@intel.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Lars Kurth > <lars.kurth@citrix.com>; xen-devel@lists.xensource.com; Michael S. Tsirkin > <mst@redhat.com>; Lars Kurth <lars.kurth.xen@gmail.com>; qemu- > devel@nongnu.org; Cao jin <caoj.fnst@cn.fujitsu.com>; Stefano Stabellini > <Stefano.Stabellini@citrix.com> > Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v4] igd-passthrough-i440FX: > convert to realize() > > Hi, > > > I can boot up Linux VM with IGD pass-through with latest qemu (without > > any additional patch), guest run 3D "nexuiz" and get 180fps. > > That is a pretty recent linux guest I assume? > Tried older kernels too, possibly even the old userspace xorg driver? > Do windows guest work as well? > > cheers, > Gerd
OK - it's possible that this patch commit 349a3b1cc9023f67f8fa336cb3c4a8f21a4aaaf3 Author: Cao jin <caoj.fnst@cn.fujitsu.com> Date: Sat Jan 2 16:02:20 2016 +0800 igd-passthrough: fix use of host_pci_config_read is required for older guests. This patch just went it - could you test latest master please? On Tue, Jan 12, 2016 at 08:41:13AM +0000, Hao, Xudong wrote: > Yes, Linux VM update to a 3.18 kernel. > The RHEL7.2 default kernel (should be 3.10) VM don't boot up with IGD pass-through, and Windows can't boot up either. > > Thanks, > -Xudong > > > > -----Original Message----- > > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > > Sent: Monday, January 11, 2016 6:32 PM > > To: Hao, Xudong <xudong.hao@intel.com> > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Lars Kurth > > <lars.kurth@citrix.com>; xen-devel@lists.xensource.com; Michael S. Tsirkin > > <mst@redhat.com>; Lars Kurth <lars.kurth.xen@gmail.com>; qemu- > > devel@nongnu.org; Cao jin <caoj.fnst@cn.fujitsu.com>; Stefano Stabellini > > <Stefano.Stabellini@citrix.com> > > Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v4] igd-passthrough-i440FX: > > convert to realize() > > > > Hi, > > > > > I can boot up Linux VM with IGD pass-through with latest qemu (without > > > any additional patch), guest run 3D "nexuiz" and get 180fps. > > > > That is a pretty recent linux guest I assume? > > Tried older kernels too, possibly even the old userspace xorg driver? > > Do windows guest work as well? > > > > cheers, > > Gerd >
With latest qemu 7b8a354d4716, RHEL7.2 (with default kernel) VM still can't boot up with IGD. Thanks, -Xudong > -----Original Message----- > From: Michael S. Tsirkin [mailto:mst@redhat.com] > Sent: Tuesday, January 12, 2016 4:48 PM > To: Hao, Xudong <xudong.hao@intel.com> > Cc: Gerd Hoffmann <kraxel@redhat.com>; Stefano Stabellini > <stefano.stabellini@eu.citrix.com>; Lars Kurth <lars.kurth@citrix.com>; xen- > devel@lists.xensource.com; Lars Kurth <lars.kurth.xen@gmail.com>; qemu- > devel@nongnu.org; Cao jin <caoj.fnst@cn.fujitsu.com>; Stefano Stabellini > <Stefano.Stabellini@citrix.com> > Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v4] igd-passthrough-i440FX: > convert to realize() > > OK - it's possible that this patch > commit 349a3b1cc9023f67f8fa336cb3c4a8f21a4aaaf3 > Author: Cao jin <caoj.fnst@cn.fujitsu.com> > Date: Sat Jan 2 16:02:20 2016 +0800 > > igd-passthrough: fix use of host_pci_config_read > > is required for older guests. > This patch just went it - could you test latest master please? > > On Tue, Jan 12, 2016 at 08:41:13AM +0000, Hao, Xudong wrote: > > Yes, Linux VM update to a 3.18 kernel. > > The RHEL7.2 default kernel (should be 3.10) VM don't boot up with IGD pass- > through, and Windows can't boot up either. > > > > Thanks, > > -Xudong > > > > > > > -----Original Message----- > > > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > > > Sent: Monday, January 11, 2016 6:32 PM > > > To: Hao, Xudong <xudong.hao@intel.com> > > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Lars > > > Kurth <lars.kurth@citrix.com>; xen-devel@lists.xensource.com; > > > Michael S. Tsirkin <mst@redhat.com>; Lars Kurth > > > <lars.kurth.xen@gmail.com>; qemu- devel@nongnu.org; Cao jin > > > <caoj.fnst@cn.fujitsu.com>; Stefano Stabellini > > > <Stefano.Stabellini@citrix.com> > > > Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v4] igd-passthrough-i440FX: > > > convert to realize() > > > > > > Hi, > > > > > > > I can boot up Linux VM with IGD pass-through with latest qemu > > > > (without any additional patch), guest run 3D "nexuiz" and get 180fps. > > > > > > That is a pretty recent linux guest I assume? > > > Tried older kernels too, possibly even the old userspace xorg driver? > > > Do windows guest work as well? > > > > > > cheers, > > > Gerd > >
> -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: Monday, January 11, 2016 6:46 PM > To: Hao, Xudong <xudong.hao@intel.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Lars Kurth > <lars.kurth.xen@gmail.com>; Lars Kurth <lars.kurth@citrix.com>; Cao jin > <caoj.fnst@cn.fujitsu.com>; xen-devel@lists.xensource.com; Stefano Stabellini > <Stefano.Stabellini@citrix.com>; qemu-devel@nongnu.org; Michael S. Tsirkin > <mst@redhat.com> > Subject: RE: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert to realize() > > On Mon, 11 Jan 2016, Hao, Xudong wrote: > > Stefano, > > > > Patch http://marc.info/?l=qemu-devel&m=145137863501079 don't works for > qemu at all, some conflict when git apply. > > Patch http://marc.info/?l=qemu-devel&m=145137863501079 is based on > patch http://marc.info/?l=qemu-devel&m=145172165010604, right? > > > > I can boot up Linux VM with IGD pass-through with latest qemu (without any > additional patch), guest run 3D "nexuiz" and get 180fps. > > Very interesting, thanks for testing. > The Linux VM's kernel is 3.18. > > > Will try the two patch together later. > > That would be useful > With the two patch, Linux VM (with 3.18) boot up successfully. RHEL7.2(default kernel 3.10) and Windows8.1 VM boot up fail with IGD pass-through. The result is same w/o these two patches. > > > > -----Original Message----- > > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > > Sent: Friday, January 8, 2016 7:57 PM > > > To: Hao, Xudong <xudong.hao@intel.com> > > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Lars > > > Kurth <lars.kurth.xen@gmail.com>; Lars Kurth > > > <lars.kurth@citrix.com>; Cao jin <caoj.fnst@cn.fujitsu.com>; > > > xen-devel@lists.xensource.com; Stefano Stabellini > > > <Stefano.Stabellini@citrix.com>; qemu-devel@nongnu.org; Michael S. > > > Tsirkin <mst@redhat.com> > > > Subject: RE: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: convert > > > to realize() > > > > > > Since you are at it, could you please let me know how well igd > > > passthrough works without this bugfix: > > > > > > http://marc.info/?l=qemu-devel&m=145172165010604 > > > > > > which is about to land in QEMU. I guess it doesn't work at all? > > > > > > I am asking because I would like to know the level of support we > > > need to provide to igd passthrough with the latest QEMU release (2.5). > > > > > > > > > On Thu, 7 Jan 2016, Hao, Xudong wrote: > > > > Sure. I'll test it soon. > > > > > > > > Thanks, > > > > -Xudong > > > > > > > > > -----Original Message----- > > > > > From: Stefano Stabellini > > > > > [mailto:stefano.stabellini@eu.citrix.com] > > > > > Sent: Wednesday, January 6, 2016 8:18 PM > > > > > To: Lars Kurth <lars.kurth.xen@gmail.com> > > > > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>; Hao, > > > > > Xudong <xudong.hao@intel.com>; Lars Kurth > > > > > <lars.kurth@citrix.com>; Cao jin <caoj.fnst@cn.fujitsu.com>; > > > > > xen-devel@lists.xensource.com; Stefano Stabellini > > > > > <Stefano.Stabellini@citrix.com>; qemu-devel@nongnu.org; Michael > > > > > S. Tsirkin <mst@redhat.com> > > > > > Subject: Re: [Xen-devel] [PATCH v4] igd-passthrough-i440FX: > > > > > convert to realize() > > > > > > > > > > Hello Xudong, > > > > > > > > > > please test this patch: > > > > > > > > > > http://marc.info/?l=qemu-devel&m=145137863501079 > > > > > > > > > > with an intel graphic card assigned to a Xen guest. If > > > > > everything still works as expected, please reply with your Tested-by. > > > > > > > > > > Thanks, > > > > > > > > > > Stefano > > > > > > > > > > On Wed, 6 Jan 2016, Lars Kurth wrote: > > > > > > Hi folks, > > > > > > let me introduce you to Xudong from Intel, who is willing to help out. > > > > > > Best Regards > > > > > > Lars > > > > > > > > > > > > > On 4 Jan 2016, at 15:41, Stefano Stabellini > > > > > > > <stefano.stabellini@eu.citrix.com> > > > > > wrote: > > > > > > > > > > > > > > On Mon, 4 Jan 2016, Lars Kurth wrote: > > > > > > >> On 04/01/2016 14:47, "Stefano Stabellini" > > > > > > >> <stefano.stabellini@eu.citrix.com> wrote: > > > > > > >> > > > > > > >>> Unfortunately I don't have a setup to test this either. > > > > > > >>> Maybe Lars can find out who should be involved on the Intel side > on this. > > > > > > >> > > > > > > >> I can certainly help to this and get back to you. What > > > > > > >> exactly are we asking Intel to do? > > > > > > >> It is not clear to me from this email thread > > > > > > > > > > > > > > Tiejun Chen, the author of the Intel graphic card > > > > > > > passthrough patches for QEMU, seems to have left the > > > > > > > company. It would be nice if somebody else tested this patch > > > > > > > with an intel graphic card assigned to a guest VM. > > > > > > > > > > > > > > _______________________________________________ > > > > > > > Xen-devel mailing list > > > > > > > Xen-devel@lists.xen.org > > > > > > > http://lists.xen.org/xen-devel > > > > > > > > > > > >
On Di, 2016-01-12 at 09:50 +0000, Hao, Xudong wrote:
> With latest qemu 7b8a354d4716, RHEL7.2 (with default kernel) VM still can't boot up with IGD.
There is another bug, using pci_default_write_config() doesn't fly as
this checks writes against wmask and the registers in question are not
whitelisted ...
I've just rebased my igd patch series which (among other stuff) fixes
that bug:
https://www.kraxel.org/cgit/qemu/log/?h=work/igd
git://git.kraxel.org/qemu work/igd
Can you give it a spin?
Also: what does "can't boot up" mean exactly? Guest doesn't boot at
all? Guest boots, but igd/console/Xorg doesn't work? In case of the
latter: Any chance to login via network and get a kernel log?
thanks,
Gerd
"can't boot up" means guest doesn't boot at all, guest will stop to booting after adding vga device, detail log in attachment. Thanks, -Xudong > -----Original Message----- > From: qemu-devel-bounces+xudong.hao=intel.com@nongnu.org [mailto:qemu- > devel-bounces+xudong.hao=intel.com@nongnu.org] On Behalf Of Gerd > Hoffmann > Sent: Tuesday, January 12, 2016 6:25 PM > To: Hao, Xudong <xudong.hao@intel.com> > Cc: Lars Kurth <lars.kurth@citrix.com>; xen-devel@lists.xensource.com; Stefano > Stabellini <stefano.stabellini@eu.citrix.com>; Lars Kurth > <lars.kurth.xen@gmail.com>; Michael S. Tsirkin <mst@redhat.com>; qemu- > devel@nongnu.org; Cao jin <caoj.fnst@cn.fujitsu.com>; Stefano Stabellini > <Stefano.Stabellini@citrix.com> > Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v4] igd-passthrough-i440FX: > convert to realize() > > On Di, 2016-01-12 at 09:50 +0000, Hao, Xudong wrote: > > With latest qemu 7b8a354d4716, RHEL7.2 (with default kernel) VM still can't > boot up with IGD. > > There is another bug, using pci_default_write_config() doesn't fly as this checks > writes against wmask and the registers in question are not whitelisted ... > > I've just rebased my igd patch series which (among other stuff) fixes that bug: > https://www.kraxel.org/cgit/qemu/log/?h=work/igd > git://git.kraxel.org/qemu work/igd > > Can you give it a spin? > > Also: what does "can't boot up" mean exactly? Guest doesn't boot at all? Guest > boots, but igd/console/Xorg doesn't work? In case of the > latter: Any chance to login via network and get a kernel log? > > thanks, > Gerd >
> -----Original Message----- > From: qemu-devel-bounces+xudong.hao=intel.com@nongnu.org [mailto:qemu- > devel-bounces+xudong.hao=intel.com@nongnu.org] On Behalf Of Gerd > Hoffmann > Sent: Tuesday, January 12, 2016 6:25 PM > To: Hao, Xudong <xudong.hao@intel.com> > Cc: Lars Kurth <lars.kurth@citrix.com>; xen-devel@lists.xensource.com; Stefano > Stabellini <stefano.stabellini@eu.citrix.com>; Lars Kurth > <lars.kurth.xen@gmail.com>; Michael S. Tsirkin <mst@redhat.com>; qemu- > devel@nongnu.org; Cao jin <caoj.fnst@cn.fujitsu.com>; Stefano Stabellini > <Stefano.Stabellini@citrix.com> > Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v4] igd-passthrough-i440FX: > convert to realize() > > On Di, 2016-01-12 at 09:50 +0000, Hao, Xudong wrote: > > With latest qemu 7b8a354d4716, RHEL7.2 (with default kernel) VM still can't > boot up with IGD. > > There is another bug, using pci_default_write_config() doesn't fly as this checks > writes against wmask and the registers in question are not whitelisted ... > > I've just rebased my igd patch series which (among other stuff) fixes that bug: > https://www.kraxel.org/cgit/qemu/log/?h=work/igd > git://git.kraxel.org/qemu work/igd > > Can you give it a spin? The issue persist on this branch of tree (commit 1a0d06ce6), the VM kernel log attached in another mail while answered the following question. > > Also: what does "can't boot up" mean exactly? Guest doesn't boot at all? Guest > boots, but igd/console/Xorg doesn't work? In case of the > latter: Any chance to login via network and get a kernel log? > > thanks, > Gerd >
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index a9cb983..e91570f 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = { {0xa8, 4}, /* SNB: base of GTT stolen memory */ }; -static int host_pci_config_read(int pos, int len, uint32_t *val) +static void host_pci_config_read(int pos, int len, uint32_t *val, Error **errp) { char path[PATH_MAX]; int config_fd; @@ -769,19 +769,19 @@ static int host_pci_config_read(int pos, int len, uint32_t *val) /* Access real host bridge. */ int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", 0, 0, 0, 0, "config"); - int ret = 0; if (rc >= size || rc < 0) { - return -ENODEV; + error_setg_errno(errp, errno, "snprintf err"); } config_fd = open(path, O_RDWR); if (config_fd < 0) { - return -ENODEV; + error_setg_errno(errp, errno, "open %s fail", path); + return; } if (lseek(config_fd, pos, SEEK_SET) != pos) { - ret = -errno; + error_setg_errno(errp, errno, "lseek err"); goto out; } @@ -789,32 +789,32 @@ static int host_pci_config_read(int pos, int len, uint32_t *val) rc = read(config_fd, (uint8_t *)val, len); } while (rc < 0 && (errno == EINTR || errno == EAGAIN)); if (rc != len) { - ret = -errno; + error_setg_errno(errp, errno, "read err"); } out: close(config_fd); - return ret; } -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) +static void igd_pt_i440fx_realize(struct PCIDevice *pci_dev, Error **errp) { uint32_t val = 0; - int rc, i, num; + int i, num; int pos, len; + Error *local_err = NULL; num = ARRAY_SIZE(igd_host_bridge_infos); for (i = 0; i < num; i++) { pos = igd_host_bridge_infos[i].offset; len = igd_host_bridge_infos[i].len; - rc = host_pci_config_read(pos, len, &val); - if (rc) { - return -ENODEV; + + host_pci_config_read(pos, len, &val, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; } pci_default_write_config(pci_dev, pos, cpu_to_le32(val), len); } - - return 0; } static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) @@ -822,7 +822,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - k->init = igd_pt_i440fx_initfn; + k->realize = igd_pt_i440fx_realize; dc->desc = "IGD Passthrough Host bridge"; }
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> --- changelog v4: 1. strip the bugfix code, but I guess this patch should be applied after that bugfix patch. 2. Other little fix as per previous review Some explanation to previous review: 1. error_setg_errno() I use in this patch will print the strerror(errno), so I guess it will be informative enough? example: error_setg_errno(errp, errno, "open %s fail", path); will print: "open bluhbluh fail: string_from_strerrer(errno)" 2. snprintf() has tiny tiny chance to fail, I don`t assert here, because this device is on-board, it init during machine initialization, in case it fails, the errp we set will results in error_report(err) and exit(1), at least can let user know why fail. see the call-chain: pc_xen_hvm_init_pci->pc_init1->i440fx_init->pci_create_simple-> pci_create_simple_multifunction->qdev_init_nofail About test: 1. Compiled 2. Did a dirty hack to force create/realize this device in pc_init1(), prove that the realizing process is ok, I will reply this mail later to attch the screenshot evidence hw/pci-host/piix.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)