[Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Zhao Yan]

Commit 5a11d0f7 mistakenly converted a log message into an error
condition when irq map is failed for the pci device being
passed through. Revert that part of the commit.

Signed-off-by: Zhao Yan <yan.y.zhao@intel.com>
---
 hw/xen/xen_pt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index e5a6eff44f..840fd0f748 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -849,7 +849,7 @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
     machine_irq = s->real_device.irq;
     rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
     if (rc < 0) {
-        error_setg_errno(errp, errno, "Mapping machine irq %u to"
+        XEN_PT_ERR(d, "Mapping machine irq %u to"
                          " pirq %i failed", machine_irq, pirq);
 
         /* Disable PCI intx assertion (turn on bit10 of devctl) */
@@ -871,7 +871,7 @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
                                        PCI_SLOT(d->devfn),
                                        e_intx);
         if (rc < 0) {
-            error_setg_errno(errp, errno, "Binding of interrupt %u failed",
+            XEN_PT_ERR(d, "Binding of interrupt %u failed",
                              e_intx);
 
             /* Disable PCI intx assertion (turn on bit10 of devctl) */
-- 
2.17.1

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Zhao, Yan Y]

Hi
The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.

Error message is below:
libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted
libxl: error: libxl_pci.c:1306:libxl__add_pcidevs: Domain 2:libxl_device_pci_add failed: -3
libxl: error: libxl_create.c:1458:domcreate_attach_devices: Domain 2:unable to add pci devices
libxl: error: libxl_domain.c:1003:libxl__destroy_domid: Domain  2:Non-existant domain
libxl: error: libxl_domain.c:962:domain_destroy_callback: Domain  2:Unable to destroy guest
libxl: error: libxl_domain.c:889:domain_destroy_cb: Domain 2:Destruction of domain failed

After partially revert 5a11d0f7, the device can be passed through into domU and running quite well using msi mode.

> -----Original Message-----
> From: Zhao, Yan Y
> Sent: Tuesday, October 16, 2018 10:15 AM
> To: qemu-devel@nongnu.org; sstabellini@kernel.org;
> anthony.perard@citrix.com; xen-devel@lists.xenproject.org
> Cc: Zhao, Yan Y <yan.y.zhao@intel.com>
> Subject: [PATCH] Xen PCI passthrough: fix passthrough failure when irq map
> failure
> 
> Commit 5a11d0f7 mistakenly converted a log message into an error condition
> when irq map is failed for the pci device being passed through. Revert that part
> of the commit.
> 
> Signed-off-by: Zhao Yan <yan.y.zhao@intel.com>
> ---
>  hw/xen/xen_pt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index e5a6eff44f..840fd0f748
> 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -849,7 +849,7 @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
>      machine_irq = s->real_device.irq;
>      rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
>      if (rc < 0) {
> -        error_setg_errno(errp, errno, "Mapping machine irq %u to"
> +        XEN_PT_ERR(d, "Mapping machine irq %u to"
>                           " pirq %i failed", machine_irq, pirq);
> 
>          /* Disable PCI intx assertion (turn on bit10 of devctl) */ @@ -871,7 +871,7
> @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
>                                         PCI_SLOT(d->devfn),
>                                         e_intx);
>          if (rc < 0) {
> -            error_setg_errno(errp, errno, "Binding of interrupt %u failed",
> +            XEN_PT_ERR(d, "Binding of interrupt %u failed",
>                               e_intx);
> 
>              /* Disable PCI intx assertion (turn on bit10 of devctl) */
> --
> 2.17.1

Re: [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Zhao, Yan Y]

Hi
The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.

Error message is below:
libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted
libxl: error: libxl_pci.c:1306:libxl__add_pcidevs: Domain 2:libxl_device_pci_add failed: -3
libxl: error: libxl_create.c:1458:domcreate_attach_devices: Domain 2:unable to add pci devices
libxl: error: libxl_domain.c:1003:libxl__destroy_domid: Domain  2:Non-existant domain
libxl: error: libxl_domain.c:962:domain_destroy_callback: Domain  2:Unable to destroy guest
libxl: error: libxl_domain.c:889:domain_destroy_cb: Domain 2:Destruction of domain failed

After partially revert 5a11d0f7, the device can be passed through into domU and running quite well using msi mode.

> -----Original Message-----
> From: Zhao, Yan Y
> Sent: Tuesday, October 16, 2018 10:15 AM
> To: qemu-devel@nongnu.org; sstabellini@kernel.org;
> anthony.perard@citrix.com; xen-devel@lists.xenproject.org
> Cc: Zhao, Yan Y <yan.y.zhao@intel.com>
> Subject: [PATCH] Xen PCI passthrough: fix passthrough failure when irq map
> failure
> 
> Commit 5a11d0f7 mistakenly converted a log message into an error condition
> when irq map is failed for the pci device being passed through. Revert that part
> of the commit.
> 
> Signed-off-by: Zhao Yan <yan.y.zhao@intel.com>
> ---
>  hw/xen/xen_pt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index e5a6eff44f..840fd0f748
> 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -849,7 +849,7 @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
>      machine_irq = s->real_device.irq;
>      rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
>      if (rc < 0) {
> -        error_setg_errno(errp, errno, "Mapping machine irq %u to"
> +        XEN_PT_ERR(d, "Mapping machine irq %u to"
>                           " pirq %i failed", machine_irq, pirq);
> 
>          /* Disable PCI intx assertion (turn on bit10 of devctl) */ @@ -871,7 +871,7
> @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
>                                         PCI_SLOT(d->devfn),
>                                         e_intx);
>          if (rc < 0) {
> -            error_setg_errno(errp, errno, "Binding of interrupt %u failed",
> +            XEN_PT_ERR(d, "Binding of interrupt %u failed",
>                               e_intx);
> 
>              /* Disable PCI intx assertion (turn on bit10 of devctl) */
> --
> 2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Roger Pau Monné]

On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> Hi
> The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.
> 
> Error message is below:
> libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted

I'm having issues figuring out what's happening here.
s->real_device.irq is 0, yet the PCI config space read of
PCI_INTERRUPT_PIN returns something different than 0.

AFAICT this is due to some kind of error in Linux, so that even when
the device is supposed to have a valid IRQ the sysfs node it is set to
0, do you know the actual underlying cause of this?

Thanks, Roger.

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Roger Pau Monné]

On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> Hi
> The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.
> 
> Error message is below:
> libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted

I'm having issues figuring out what's happening here.
s->real_device.irq is 0, yet the PCI config space read of
PCI_INTERRUPT_PIN returns something different than 0.

AFAICT this is due to some kind of error in Linux, so that even when
the device is supposed to have a valid IRQ the sysfs node it is set to
0, do you know the actual underlying cause of this?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Zhao Yan]

On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
> On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> > Hi
> > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.
> > 
> > Error message is below:
> > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted
> 
> I'm having issues figuring out what's happening here.
> s->real_device.irq is 0, yet the PCI config space read of
> PCI_INTERRUPT_PIN returns something different than 0.
> 
> AFAICT this is due to some kind of error in Linux, so that even when
> the device is supposed to have a valid IRQ the sysfs node it is set to
> 0, do you know the actual underlying cause of this?
> 
> Thanks, Roger.
Hi Roger
Sorry for the later reply, I just missed this mail...
on my side, it's because the hardware actually does not support INTx mode,
but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
hardware bug, but previous version of qemu can tolerate it, switch to MSI
and make passthrough work.

Thanks
Yan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Zhao Yan]

On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
> On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> > Hi
> > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.
> > 
> > Error message is below:
> > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted
> 
> I'm having issues figuring out what's happening here.
> s->real_device.irq is 0, yet the PCI config space read of
> PCI_INTERRUPT_PIN returns something different than 0.
> 
> AFAICT this is due to some kind of error in Linux, so that even when
> the device is supposed to have a valid IRQ the sysfs node it is set to
> 0, do you know the actual underlying cause of this?
> 
> Thanks, Roger.
Hi Roger
Sorry for the later reply, I just missed this mail...
On my side, it's because the hardware actually does not support INTx mode,
but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
hardware bug, but previous version of qemu can tolerate it, switch to MSI
and make passthrough work.

Thanks
Yan

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Zhao Yan]

On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
> On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> > Hi
> > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.
> > 
> > Error message is below:
> > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted
> 
> I'm having issues figuring out what's happening here.
> s->real_device.irq is 0, yet the PCI config space read of
> PCI_INTERRUPT_PIN returns something different than 0.
> 
> AFAICT this is due to some kind of error in Linux, so that even when
> the device is supposed to have a valid IRQ the sysfs node it is set to
> 0, do you know the actual underlying cause of this?
> 
> Thanks, Roger.
Hi Roger
Sorry for the later reply, I just missed this mail...
On my side, it's because the hardware actually does not support INTx mode,
but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
hardware bug, but previous version of qemu can tolerate it, switch to MSI
and make passthrough work.

Thanks
Yan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Roger Pau Monné]

On Thu, Nov 22, 2018 at 08:11:20AM -0500, Zhao Yan wrote:
> On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
> > On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> > > Hi
> > > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> > > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.
> > > 
> > > Error message is below:
> > > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted
> > 
> > I'm having issues figuring out what's happening here.
> > s->real_device.irq is 0, yet the PCI config space read of
> > PCI_INTERRUPT_PIN returns something different than 0.
> > 
> > AFAICT this is due to some kind of error in Linux, so that even when
> > the device is supposed to have a valid IRQ the sysfs node it is set to
> > 0, do you know the actual underlying cause of this?
> > 
> > Thanks, Roger.
> Hi Roger
> Sorry for the later reply, I just missed this mail...
> On my side, it's because the hardware actually does not support INTx mode,
> but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
> hardware bug, but previous version of qemu can tolerate it, switch to MSI
> and make passthrough work.

Then I think it would be better to check both PCI_INTERRUPT_PIN and
s->real_device.irq before attempting to map the IRQ.

Making the error non-fatal would mean that a device with a valid IRQ
could fail to be setup correctly but the guest will still be created,
and things won't go as expected when the guest attempts to use it.

Thanks, Roger.

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Roger Pau Monné]

On Thu, Nov 22, 2018 at 08:11:20AM -0500, Zhao Yan wrote:
> On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
> > On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> > > Hi
> > > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> > > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.
> > > 
> > > Error message is below:
> > > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted
> > 
> > I'm having issues figuring out what's happening here.
> > s->real_device.irq is 0, yet the PCI config space read of
> > PCI_INTERRUPT_PIN returns something different than 0.
> > 
> > AFAICT this is due to some kind of error in Linux, so that even when
> > the device is supposed to have a valid IRQ the sysfs node it is set to
> > 0, do you know the actual underlying cause of this?
> > 
> > Thanks, Roger.
> Hi Roger
> Sorry for the later reply, I just missed this mail...
> On my side, it's because the hardware actually does not support INTx mode,
> but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
> hardware bug, but previous version of qemu can tolerate it, switch to MSI
> and make passthrough work.

Then I think it would be better to check both PCI_INTERRUPT_PIN and
s->real_device.irq before attempting to map the IRQ.

Making the error non-fatal would mean that a device with a valid IRQ
could fail to be setup correctly but the guest will still be created,
and things won't go as expected when the guest attempts to use it.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Zhao Yan]

On Thu, Nov 22, 2018 at 03:18:05PM +0100, Roger Pau Monné wrote:
> On Thu, Nov 22, 2018 at 08:11:20AM -0500, Zhao Yan wrote:
> > On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
> > > On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> > > > Hi
> > > > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> > > > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.
> > > > 
> > > > Error message is below:
> > > > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted
> > > 
> > > I'm having issues figuring out what's happening here.
> > > s->real_device.irq is 0, yet the PCI config space read of
> > > PCI_INTERRUPT_PIN returns something different than 0.
> > > 
> > > AFAICT this is due to some kind of error in Linux, so that even when
> > > the device is supposed to have a valid IRQ the sysfs node it is set to
> > > 0, do you know the actual underlying cause of this?
> > > 
> > > Thanks, Roger.
> > Hi Roger
> > Sorry for the later reply, I just missed this mail...
> > On my side, it's because the hardware actually does not support INTx mode,
> > but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
> > hardware bug, but previous version of qemu can tolerate it, switch to MSI
> > and make passthrough work.
> 
> Then I think it would be better to check both PCI_INTERRUPT_PIN and
> s->real_device.irq before attempting to map the IRQ.
> 
> Making the error non-fatal would mean that a device with a valid IRQ
> could fail to be setup correctly but the guest will still be created,
> and things won't go as expected when the guest attempts to use it.
> 
> Thanks, Roger.
hi roger
thanks for your sugguestion. it's right that "s->real_device.irq" is needed to be checked before mapping, like if it's 0.
but on the other hand, maybe xc_physdev_map_pirq() itself can serve as a checking of "s->real_device.irq" ?
like in our case, it will fail and return -EPERM.
then error hanling is still conducted ==>set INTX_DISABLE flag, eventhrough the error is not fatal.

    machine_irq = s->real_device.irq;
    rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
    if (rc < 0) {
        error_setg_errno(errp, errno, "Mapping machine irq %u to"
                         " pirq %i failed", machine_irq, pirq);

        /* Disable PCI intx assertion (turn on bit10 of devctl) */
        cmd |= PCI_COMMAND_INTX_DISABLE;
        machine_irq = 0;
        s->machine_irq = 0;
So, do you think it's all right just converting fatal error to non-fatal?


Thanks
Yan

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Zhao Yan]

On Thu, Nov 22, 2018 at 03:18:05PM +0100, Roger Pau Monné wrote:
> On Thu, Nov 22, 2018 at 08:11:20AM -0500, Zhao Yan wrote:
> > On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
> > > On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> > > > Hi
> > > > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> > > > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.
> > > > 
> > > > Error message is below:
> > > > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted
> > > 
> > > I'm having issues figuring out what's happening here.
> > > s->real_device.irq is 0, yet the PCI config space read of
> > > PCI_INTERRUPT_PIN returns something different than 0.
> > > 
> > > AFAICT this is due to some kind of error in Linux, so that even when
> > > the device is supposed to have a valid IRQ the sysfs node it is set to
> > > 0, do you know the actual underlying cause of this?
> > > 
> > > Thanks, Roger.
> > Hi Roger
> > Sorry for the later reply, I just missed this mail...
> > On my side, it's because the hardware actually does not support INTx mode,
> > but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
> > hardware bug, but previous version of qemu can tolerate it, switch to MSI
> > and make passthrough work.
> 
> Then I think it would be better to check both PCI_INTERRUPT_PIN and
> s->real_device.irq before attempting to map the IRQ.
> 
> Making the error non-fatal would mean that a device with a valid IRQ
> could fail to be setup correctly but the guest will still be created,
> and things won't go as expected when the guest attempts to use it.
> 
> Thanks, Roger.
hi roger
thanks for your sugguestion. it's right that "s->real_device.irq" is needed to be checked before mapping, like if it's 0.
but on the other hand, maybe xc_physdev_map_pirq() itself can serve as a checking of "s->real_device.irq" ?
like in our case, it will fail and return -EPERM.
then error hanling is still conducted ==>set INTX_DISABLE flag, eventhrough the error is not fatal.

    machine_irq = s->real_device.irq;
    rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
    if (rc < 0) {
        error_setg_errno(errp, errno, "Mapping machine irq %u to"
                         " pirq %i failed", machine_irq, pirq);

        /* Disable PCI intx assertion (turn on bit10 of devctl) */
        cmd |= PCI_COMMAND_INTX_DISABLE;
        machine_irq = 0;
        s->machine_irq = 0;
So, do you think it's all right just converting fatal error to non-fatal?


Thanks
Yan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Roger Pau Monné]

Adding Jan in case he has an opinion on my reply below.

On Fri, Nov 23, 2018 at 12:04:51AM -0500, Zhao Yan wrote:
> On Thu, Nov 22, 2018 at 03:18:05PM +0100, Roger Pau Monné wrote:
> > On Thu, Nov 22, 2018 at 08:11:20AM -0500, Zhao Yan wrote:
> > > On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
> > > > On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> > > > > Hi
> > > > > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> > > > > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.
> > > > > 
> > > > > Error message is below:
> > > > > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted
> > > > 
> > > > I'm having issues figuring out what's happening here.
> > > > s->real_device.irq is 0, yet the PCI config space read of
> > > > PCI_INTERRUPT_PIN returns something different than 0.
> > > > 
> > > > AFAICT this is due to some kind of error in Linux, so that even when
> > > > the device is supposed to have a valid IRQ the sysfs node it is set to
> > > > 0, do you know the actual underlying cause of this?
> > > > 
> > > > Thanks, Roger.
> > > Hi Roger
> > > Sorry for the later reply, I just missed this mail...
> > > On my side, it's because the hardware actually does not support INTx mode,
> > > but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
> > > hardware bug, but previous version of qemu can tolerate it, switch to MSI
> > > and make passthrough work.
> > 
> > Then I think it would be better to check both PCI_INTERRUPT_PIN and
> > s->real_device.irq before attempting to map the IRQ.
> > 
> > Making the error non-fatal would mean that a device with a valid IRQ
> > could fail to be setup correctly but the guest will still be created,
> > and things won't go as expected when the guest attempts to use it.
> > 
> > Thanks, Roger.
> hi roger
> thanks for your sugguestion. it's right that "s->real_device.irq" is needed to be checked before mapping, like if it's 0.
> but on the other hand, maybe xc_physdev_map_pirq() itself can serve as a checking of "s->real_device.irq" ?
> like in our case, it will fail and return -EPERM.
> then error hanling is still conducted ==>set INTX_DISABLE flag, eventhrough the error is not fatal.
> 
>     machine_irq = s->real_device.irq;
>     rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
>     if (rc < 0) {
>         error_setg_errno(errp, errno, "Mapping machine irq %u to"
>                          " pirq %i failed", machine_irq, pirq);
> 
>         /* Disable PCI intx assertion (turn on bit10 of devctl) */
>         cmd |= PCI_COMMAND_INTX_DISABLE;
>         machine_irq = 0;
>         s->machine_irq = 0;
> So, do you think it's all right just converting fatal error to non-fatal?

As I said above, I think it would be better to leave the error as
fatal and avoid attempting a xc_physdev_map_pirq with a machine_irq ==
0, which will fail.

If we really want to go down the route of making the error non-fatal,
I think you will also have to report PCI_INTERRUPT_PIN as 0 to the
guest, so that it's clear to the guest that the device doesn't have
legacy interrupt support.

Exposing a device with PCI_INTERRUPT_PIN != 0 but then not allowing
the guest to clear PCI_COMMAND_INTX_DISABLE is likely bogus.

Thanks, Roger.

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Roger Pau Monné]

Adding Jan in case he has an opinion on my reply below.

On Fri, Nov 23, 2018 at 12:04:51AM -0500, Zhao Yan wrote:
> On Thu, Nov 22, 2018 at 03:18:05PM +0100, Roger Pau Monné wrote:
> > On Thu, Nov 22, 2018 at 08:11:20AM -0500, Zhao Yan wrote:
> > > On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
> > > > On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> > > > > Hi
> > > > > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> > > > > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too rigorous.
> > > > > 
> > > > > Error message is below:
> > > > > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server: Mapping machine irq 0 to pirq -1 failed: Operation not permitted
> > > > 
> > > > I'm having issues figuring out what's happening here.
> > > > s->real_device.irq is 0, yet the PCI config space read of
> > > > PCI_INTERRUPT_PIN returns something different than 0.
> > > > 
> > > > AFAICT this is due to some kind of error in Linux, so that even when
> > > > the device is supposed to have a valid IRQ the sysfs node it is set to
> > > > 0, do you know the actual underlying cause of this?
> > > > 
> > > > Thanks, Roger.
> > > Hi Roger
> > > Sorry for the later reply, I just missed this mail...
> > > On my side, it's because the hardware actually does not support INTx mode,
> > > but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
> > > hardware bug, but previous version of qemu can tolerate it, switch to MSI
> > > and make passthrough work.
> > 
> > Then I think it would be better to check both PCI_INTERRUPT_PIN and
> > s->real_device.irq before attempting to map the IRQ.
> > 
> > Making the error non-fatal would mean that a device with a valid IRQ
> > could fail to be setup correctly but the guest will still be created,
> > and things won't go as expected when the guest attempts to use it.
> > 
> > Thanks, Roger.
> hi roger
> thanks for your sugguestion. it's right that "s->real_device.irq" is needed to be checked before mapping, like if it's 0.
> but on the other hand, maybe xc_physdev_map_pirq() itself can serve as a checking of "s->real_device.irq" ?
> like in our case, it will fail and return -EPERM.
> then error hanling is still conducted ==>set INTX_DISABLE flag, eventhrough the error is not fatal.
> 
>     machine_irq = s->real_device.irq;
>     rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
>     if (rc < 0) {
>         error_setg_errno(errp, errno, "Mapping machine irq %u to"
>                          " pirq %i failed", machine_irq, pirq);
> 
>         /* Disable PCI intx assertion (turn on bit10 of devctl) */
>         cmd |= PCI_COMMAND_INTX_DISABLE;
>         machine_irq = 0;
>         s->machine_irq = 0;
> So, do you think it's all right just converting fatal error to non-fatal?

As I said above, I think it would be better to leave the error as
fatal and avoid attempting a xc_physdev_map_pirq with a machine_irq ==
0, which will fail.

If we really want to go down the route of making the error non-fatal,
I think you will also have to report PCI_INTERRUPT_PIN as 0 to the
guest, so that it's clear to the guest that the device doesn't have
legacy interrupt support.

Exposing a device with PCI_INTERRUPT_PIN != 0 but then not allowing
the guest to clear PCI_COMMAND_INTX_DISABLE is likely bogus.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Jan Beulich]

>>> On 23.11.18 at 11:19, <roger.pau@citrix.com> wrote:
> Adding Jan in case he has an opinion on my reply below.

I agree, fwiw.

Jan

> On Fri, Nov 23, 2018 at 12:04:51AM -0500, Zhao Yan wrote:
>> On Thu, Nov 22, 2018 at 03:18:05PM +0100, Roger Pau Monné wrote:
>> > On Thu, Nov 22, 2018 at 08:11:20AM -0500, Zhao Yan wrote:
>> > > On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
>> > > > On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
>> > > > > Hi
>> > > > > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it
actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
>> > > > > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too
rigorous.
>> > > > > 
>> > > > > Error message is below:
>> > > > > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server:
Mapping machine irq 0 to pirq -1 failed: Operation not permitted
>> > > > 
>> > > > I'm having issues figuring out what's happening here.
>> > > > s->real_device.irq is 0, yet the PCI config space read of
>> > > > PCI_INTERRUPT_PIN returns something different than 0.
>> > > > 
>> > > > AFAICT this is due to some kind of error in Linux, so that even when
>> > > > the device is supposed to have a valid IRQ the sysfs node it is set to
>> > > > 0, do you know the actual underlying cause of this?
>> > > > 
>> > > > Thanks, Roger.
>> > > Hi Roger
>> > > Sorry for the later reply, I just missed this mail...
>> > > On my side, it's because the hardware actually does not support INTx mode,
>> > > but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
>> > > hardware bug, but previous version of qemu can tolerate it, switch to MSI
>> > > and make passthrough work.
>> > 
>> > Then I think it would be better to check both PCI_INTERRUPT_PIN and
>> > s->real_device.irq before attempting to map the IRQ.
>> > 
>> > Making the error non-fatal would mean that a device with a valid IRQ
>> > could fail to be setup correctly but the guest will still be created,
>> > and things won't go as expected when the guest attempts to use it.
>> > 
>> > Thanks, Roger.
>> hi roger
>> thanks for your sugguestion. it's right that "s->real_device.irq" is needed to be checked before mapping, like if
it's 0.
>> but on the other hand, maybe xc_physdev_map_pirq() itself can serve as a checking of "s->real_device.irq" ?
>> like in our case, it will fail and return -EPERM.
>> then error hanling is still conducted ==>set INTX_DISABLE flag, eventhrough the error is not fatal.
>> 
>>     machine_irq = s->real_device.irq;
>>     rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
>>     if (rc < 0) {
>>         error_setg_errno(errp, errno, "Mapping machine irq %u to"
>>                          " pirq %i failed", machine_irq, pirq);
>> 
>>         /* Disable PCI intx assertion (turn on bit10 of devctl) */
>>         cmd |= PCI_COMMAND_INTX_DISABLE;
>>         machine_irq = 0;
>>         s->machine_irq = 0;
>> So, do you think it's all right just converting fatal error to non-fatal?
> 
> As I said above, I think it would be better to leave the error as
> fatal and avoid attempting a xc_physdev_map_pirq with a machine_irq ==
> 0, which will fail.
> 
> If we really want to go down the route of making the error non-fatal,
> I think you will also have to report PCI_INTERRUPT_PIN as 0 to the
> guest, so that it's clear to the guest that the device doesn't have
> legacy interrupt support.
> 
> Exposing a device with PCI_INTERRUPT_PIN != 0 but then not allowing
> the guest to clear PCI_COMMAND_INTX_DISABLE is likely bogus.
> 
> Thanks, Roger.

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Jan Beulich]

>>> On 23.11.18 at 11:19, <roger.pau@citrix.com> wrote:
> Adding Jan in case he has an opinion on my reply below.

I agree, fwiw.

Jan

> On Fri, Nov 23, 2018 at 12:04:51AM -0500, Zhao Yan wrote:
>> On Thu, Nov 22, 2018 at 03:18:05PM +0100, Roger Pau Monné wrote:
>> > On Thu, Nov 22, 2018 at 08:11:20AM -0500, Zhao Yan wrote:
>> > > On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
>> > > > On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
>> > > > > Hi
>> > > > > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it
actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
>> > > > > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too
rigorous.
>> > > > > 
>> > > > > Error message is below:
>> > > > > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server:
Mapping machine irq 0 to pirq -1 failed: Operation not permitted
>> > > > 
>> > > > I'm having issues figuring out what's happening here.
>> > > > s->real_device.irq is 0, yet the PCI config space read of
>> > > > PCI_INTERRUPT_PIN returns something different than 0.
>> > > > 
>> > > > AFAICT this is due to some kind of error in Linux, so that even when
>> > > > the device is supposed to have a valid IRQ the sysfs node it is set to
>> > > > 0, do you know the actual underlying cause of this?
>> > > > 
>> > > > Thanks, Roger.
>> > > Hi Roger
>> > > Sorry for the later reply, I just missed this mail...
>> > > On my side, it's because the hardware actually does not support INTx mode,
>> > > but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
>> > > hardware bug, but previous version of qemu can tolerate it, switch to MSI
>> > > and make passthrough work.
>> > 
>> > Then I think it would be better to check both PCI_INTERRUPT_PIN and
>> > s->real_device.irq before attempting to map the IRQ.
>> > 
>> > Making the error non-fatal would mean that a device with a valid IRQ
>> > could fail to be setup correctly but the guest will still be created,
>> > and things won't go as expected when the guest attempts to use it.
>> > 
>> > Thanks, Roger.
>> hi roger
>> thanks for your sugguestion. it's right that "s->real_device.irq" is needed to be checked before mapping, like if
it's 0.
>> but on the other hand, maybe xc_physdev_map_pirq() itself can serve as a checking of "s->real_device.irq" ?
>> like in our case, it will fail and return -EPERM.
>> then error hanling is still conducted ==>set INTX_DISABLE flag, eventhrough the error is not fatal.
>> 
>>     machine_irq = s->real_device.irq;
>>     rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
>>     if (rc < 0) {
>>         error_setg_errno(errp, errno, "Mapping machine irq %u to"
>>                          " pirq %i failed", machine_irq, pirq);
>> 
>>         /* Disable PCI intx assertion (turn on bit10 of devctl) */
>>         cmd |= PCI_COMMAND_INTX_DISABLE;
>>         machine_irq = 0;
>>         s->machine_irq = 0;
>> So, do you think it's all right just converting fatal error to non-fatal?
> 
> As I said above, I think it would be better to leave the error as
> fatal and avoid attempting a xc_physdev_map_pirq with a machine_irq ==
> 0, which will fail.
> 
> If we really want to go down the route of making the error non-fatal,
> I think you will also have to report PCI_INTERRUPT_PIN as 0 to the
> guest, so that it's clear to the guest that the device doesn't have
> legacy interrupt support.
> 
> Exposing a device with PCI_INTERRUPT_PIN != 0 but then not allowing
> the guest to clear PCI_COMMAND_INTX_DISABLE is likely bogus.
> 
> Thanks, Roger.




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Zhao Yan]

Hi Roger and Jan,
Thanks for your review. You are right.
I'll try the way to report PCI_INTERRUPT_PIN as 0 to the guest.

Thanks
Yan

On Fri, Nov 23, 2018 at 03:26:44AM -0700, Jan Beulich wrote:
> >>> On 23.11.18 at 11:19, <roger.pau@citrix.com> wrote:
> > Adding Jan in case he has an opinion on my reply below.
> 
> I agree, fwiw.
> 
> Jan
> 
> > On Fri, Nov 23, 2018 at 12:04:51AM -0500, Zhao Yan wrote:
> >> On Thu, Nov 22, 2018 at 03:18:05PM +0100, Roger Pau Monné wrote:
> >> > On Thu, Nov 22, 2018 at 08:11:20AM -0500, Zhao Yan wrote:
> >> > > On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
> >> > > > On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> >> > > > > Hi
> >> > > > > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it
> actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> >> > > > > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too
> rigorous.
> >> > > > > 
> >> > > > > Error message is below:
> >> > > > > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server:
> Mapping machine irq 0 to pirq -1 failed: Operation not permitted
> >> > > > 
> >> > > > I'm having issues figuring out what's happening here.
> >> > > > s->real_device.irq is 0, yet the PCI config space read of
> >> > > > PCI_INTERRUPT_PIN returns something different than 0.
> >> > > > 
> >> > > > AFAICT this is due to some kind of error in Linux, so that even when
> >> > > > the device is supposed to have a valid IRQ the sysfs node it is set to
> >> > > > 0, do you know the actual underlying cause of this?
> >> > > > 
> >> > > > Thanks, Roger.
> >> > > Hi Roger
> >> > > Sorry for the later reply, I just missed this mail...
> >> > > On my side, it's because the hardware actually does not support INTx mode,
> >> > > but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
> >> > > hardware bug, but previous version of qemu can tolerate it, switch to MSI
> >> > > and make passthrough work.
> >> > 
> >> > Then I think it would be better to check both PCI_INTERRUPT_PIN and
> >> > s->real_device.irq before attempting to map the IRQ.
> >> > 
> >> > Making the error non-fatal would mean that a device with a valid IRQ
> >> > could fail to be setup correctly but the guest will still be created,
> >> > and things won't go as expected when the guest attempts to use it.
> >> > 
> >> > Thanks, Roger.
> >> hi roger
> >> thanks for your sugguestion. it's right that "s->real_device.irq" is needed to be checked before mapping, like if
> it's 0.
> >> but on the other hand, maybe xc_physdev_map_pirq() itself can serve as a checking of "s->real_device.irq" ?
> >> like in our case, it will fail and return -EPERM.
> >> then error hanling is still conducted ==>set INTX_DISABLE flag, eventhrough the error is not fatal.
> >> 
> >>     machine_irq = s->real_device.irq;
> >>     rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
> >>     if (rc < 0) {
> >>         error_setg_errno(errp, errno, "Mapping machine irq %u to"
> >>                          " pirq %i failed", machine_irq, pirq);
> >> 
> >>         /* Disable PCI intx assertion (turn on bit10 of devctl) */
> >>         cmd |= PCI_COMMAND_INTX_DISABLE;
> >>         machine_irq = 0;
> >>         s->machine_irq = 0;
> >> So, do you think it's all right just converting fatal error to non-fatal?
> > 
> > As I said above, I think it would be better to leave the error as
> > fatal and avoid attempting a xc_physdev_map_pirq with a machine_irq ==
> > 0, which will fail.
> > 
> > If we really want to go down the route of making the error non-fatal,
> > I think you will also have to report PCI_INTERRUPT_PIN as 0 to the
> > guest, so that it's clear to the guest that the device doesn't have
> > legacy interrupt support.
> > 
> > Exposing a device with PCI_INTERRUPT_PIN != 0 but then not allowing
> > the guest to clear PCI_COMMAND_INTX_DISABLE is likely bogus.
> > 
> > Thanks, Roger.
> 
> 
> 

Re: [Qemu-devel] [PATCH] Xen PCI passthrough: fix passthrough failure when irq map failure

[Zhao Yan]

Hi Roger and Jan,
Thanks for your review. You are right.
I'll try the way to report PCI_INTERRUPT_PIN as 0 to the guest.

Thanks
Yan

On Fri, Nov 23, 2018 at 03:26:44AM -0700, Jan Beulich wrote:
> >>> On 23.11.18 at 11:19, <roger.pau@citrix.com> wrote:
> > Adding Jan in case he has an opinion on my reply below.
> 
> I agree, fwiw.
> 
> Jan
> 
> > On Fri, Nov 23, 2018 at 12:04:51AM -0500, Zhao Yan wrote:
> >> On Thu, Nov 22, 2018 at 03:18:05PM +0100, Roger Pau Monné wrote:
> >> > On Thu, Nov 22, 2018 at 08:11:20AM -0500, Zhao Yan wrote:
> >> > > On Thu, Oct 18, 2018 at 03:56:36PM +0100, Roger Pau Monné wrote:
> >> > > > On Thu, Oct 18, 2018 at 08:22:41AM +0000, Zhao, Yan Y wrote:
> >> > > > > Hi
> >> > > > > The background for this patch is that: for some pci device, even it's PCI_INTERRUPT_PIN is not 0, it
> actually does not support INTx mode, so we should just report error, disable INTx mode and continue the passthrough.
> >> > > > > However, the commit 5a11d0f7 regards this as error condition and let qemu quit passthrough, which is too
> rigorous.
> >> > > > > 
> >> > > > > Error message is below:
> >> > > > > libxl: error: libxl_qmp.c:287:qmp_handle_error_response: Domain 2:received an error message from QMP server:
> Mapping machine irq 0 to pirq -1 failed: Operation not permitted
> >> > > > 
> >> > > > I'm having issues figuring out what's happening here.
> >> > > > s->real_device.irq is 0, yet the PCI config space read of
> >> > > > PCI_INTERRUPT_PIN returns something different than 0.
> >> > > > 
> >> > > > AFAICT this is due to some kind of error in Linux, so that even when
> >> > > > the device is supposed to have a valid IRQ the sysfs node it is set to
> >> > > > 0, do you know the actual underlying cause of this?
> >> > > > 
> >> > > > Thanks, Roger.
> >> > > Hi Roger
> >> > > Sorry for the later reply, I just missed this mail...
> >> > > On my side, it's because the hardware actually does not support INTx mode,
> >> > > but its configuration space does not report PCI_INTERRUPT_PIN to 0. It's a
> >> > > hardware bug, but previous version of qemu can tolerate it, switch to MSI
> >> > > and make passthrough work.
> >> > 
> >> > Then I think it would be better to check both PCI_INTERRUPT_PIN and
> >> > s->real_device.irq before attempting to map the IRQ.
> >> > 
> >> > Making the error non-fatal would mean that a device with a valid IRQ
> >> > could fail to be setup correctly but the guest will still be created,
> >> > and things won't go as expected when the guest attempts to use it.
> >> > 
> >> > Thanks, Roger.
> >> hi roger
> >> thanks for your sugguestion. it's right that "s->real_device.irq" is needed to be checked before mapping, like if
> it's 0.
> >> but on the other hand, maybe xc_physdev_map_pirq() itself can serve as a checking of "s->real_device.irq" ?
> >> like in our case, it will fail and return -EPERM.
> >> then error hanling is still conducted ==>set INTX_DISABLE flag, eventhrough the error is not fatal.
> >> 
> >>     machine_irq = s->real_device.irq;
> >>     rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
> >>     if (rc < 0) {
> >>         error_setg_errno(errp, errno, "Mapping machine irq %u to"
> >>                          " pirq %i failed", machine_irq, pirq);
> >> 
> >>         /* Disable PCI intx assertion (turn on bit10 of devctl) */
> >>         cmd |= PCI_COMMAND_INTX_DISABLE;
> >>         machine_irq = 0;
> >>         s->machine_irq = 0;
> >> So, do you think it's all right just converting fatal error to non-fatal?
> > 
> > As I said above, I think it would be better to leave the error as
> > fatal and avoid attempting a xc_physdev_map_pirq with a machine_irq ==
> > 0, which will fail.
> > 
> > If we really want to go down the route of making the error non-fatal,
> > I think you will also have to report PCI_INTERRUPT_PIN as 0 to the
> > guest, so that it's clear to the guest that the device doesn't have
> > legacy interrupt support.
> > 
> > Exposing a device with PCI_INTERRUPT_PIN != 0 but then not allowing
> > the guest to clear PCI_COMMAND_INTX_DISABLE is likely bogus.
> > 
> > Thanks, Roger.
> 
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel