linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device"
@ 2017-01-11 23:36 Stefano Stabellini
  2017-01-12 18:27 ` Boris Ostrovsky
  0 siblings, 1 reply; 5+ messages in thread
From: Stefano Stabellini @ 2017-01-11 23:36 UTC (permalink / raw)
  To: boris.ostrovsky; +Cc: sstabellini, jgross, karahmed, linux-kernel, xen-devel

The following commit:

commit 72a9b186292d98494f222226cfd24a1621796209
Author: KarimAllah Ahmed <karahmed@amazon.de>
Date:   Fri Aug 26 23:55:36 2016 +0200

    xen: Remove event channel notification through Xen PCI platform device

broke Linux when booting as Dom0 on Xen in a nested Xen environment (Xen
installed inside a Xen VM). In this scenario, Linux is a PV guest, but
at the same time it uses the platform-pci driver to receive
notifications from L0 Xen. vector callbacks are not available because L1
Xen doesn't allow them.

Partially revert the offending commit, by restoring IRQ based
notifications for PV guests only. I restored only the code which is
strictly needed and replaced the xen_have_vector_callback checks within
it with xen_pv_domain() checks.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>

---
Alternatively, I could also restore the xen_have_vector_callback
checks. In general, it's best to have feature flag checks than umbrella
xen_pv/hvm_domain() checks.
---

diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
index b59c9455..1477f1d 100644
--- a/drivers/xen/platform-pci.c
+++ b/drivers/xen/platform-pci.c
@@ -42,6 +42,7 @@
 static unsigned long platform_mmio;
 static unsigned long platform_mmio_alloc;
 static unsigned long platform_mmiolen;
+static uint64_t callback_via;
 
 static unsigned long alloc_xen_mmio(unsigned long len)
 {
@@ -54,6 +55,51 @@ static unsigned long alloc_xen_mmio(unsigned long len)
 	return addr;
 }
 
+static uint64_t get_callback_via(struct pci_dev *pdev)
+{
+	u8 pin;
+	int irq;
+
+	irq = pdev->irq;
+	if (irq < 16)
+		return irq; /* ISA IRQ */
+
+	pin = pdev->pin;
+
+	/* We don't know the GSI. Specify the PCI INTx line instead. */
+	return ((uint64_t)0x01 << 56) | /* PCI INTx identifier */
+		((uint64_t)pci_domain_nr(pdev->bus) << 32) |
+		((uint64_t)pdev->bus->number << 16) |
+		((uint64_t)(pdev->devfn & 0xff) << 8) |
+		((uint64_t)(pin - 1) & 3);
+}
+
+static irqreturn_t do_hvm_evtchn_intr(int irq, void *dev_id)
+{
+	xen_hvm_evtchn_do_upcall();
+	return IRQ_HANDLED;
+}
+
+static int xen_allocate_irq(struct pci_dev *pdev)
+{
+	return request_irq(pdev->irq, do_hvm_evtchn_intr,
+			IRQF_NOBALANCING | IRQF_TRIGGER_RISING,
+			"xen-platform-pci", pdev);
+}
+
+static int platform_pci_resume(struct pci_dev *pdev)
+{
+	int err;
+	if (!xen_pv_domain())
+		return 0;
+	err = xen_set_callback_via(callback_via);
+	if (err) {
+		dev_err(&pdev->dev, "platform_pci_resume failure!\n");
+		return err;
+	}
+	return 0;
+}
+
 static int platform_pci_probe(struct pci_dev *pdev,
 			      const struct pci_device_id *ent)
 {
@@ -92,6 +138,21 @@ static int platform_pci_probe(struct pci_dev *pdev,
 	platform_mmio = mmio_addr;
 	platform_mmiolen = mmio_len;
 
+	if (xen_pv_domain()) {
+		ret = xen_allocate_irq(pdev);
+		if (ret) {
+			dev_warn(&pdev->dev, "request_irq failed err=%d\n", ret);
+			goto out;
+		}
+		callback_via = get_callback_via(pdev);
+		ret = xen_set_callback_via(callback_via);
+		if (ret) {
+			dev_warn(&pdev->dev, "Unable to set the evtchn callback "
+					 "err=%d\n", ret);
+			goto out;
+		}
+	}
+
 	max_nr_gframes = gnttab_max_grant_frames();
 	grant_frames = alloc_xen_mmio(PAGE_SIZE * max_nr_gframes);
 	ret = gnttab_setup_auto_xlat_frames(grant_frames);
@@ -123,6 +184,9 @@ static int platform_pci_probe(struct pci_dev *pdev,
 	.name =           DRV_NAME,
 	.probe =          platform_pci_probe,
 	.id_table =       platform_pci_tbl,
+#ifdef CONFIG_PM
+	.resume_early =   platform_pci_resume,
+#endif
 };
 
 static int __init platform_pci_init(void)

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device"
  2017-01-11 23:36 [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device" Stefano Stabellini
@ 2017-01-12 18:27 ` Boris Ostrovsky
  2017-01-12 21:33   ` Stefano Stabellini
  0 siblings, 1 reply; 5+ messages in thread
From: Boris Ostrovsky @ 2017-01-12 18:27 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: jgross, karahmed, linux-kernel, xen-devel, Konrad Rzeszutek Wilk

On 01/11/2017 06:36 PM, Stefano Stabellini wrote:
> The following commit:
>
> commit 72a9b186292d98494f222226cfd24a1621796209
> Author: KarimAllah Ahmed <karahmed@amazon.de>
> Date:   Fri Aug 26 23:55:36 2016 +0200
>
>     xen: Remove event channel notification through Xen PCI platform device
>
> broke Linux when booting as Dom0 on Xen in a nested Xen environment (Xen
> installed inside a Xen VM). In this scenario, Linux is a PV guest, but
> at the same time it uses the platform-pci driver to receive
> notifications from L0 Xen. vector callbacks are not available because L1
> Xen doesn't allow them.

(+Konrad who has been running nested)

>
> Partially revert the offending commit, by restoring IRQ based
> notifications for PV guests only. I restored only the code which is
> strictly needed and replaced the xen_have_vector_callback checks within
> it with xen_pv_domain() checks.
>
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
>
> ---
> Alternatively, I could also restore the xen_have_vector_callback
> checks. In general, it's best to have feature flag checks than umbrella
> xen_pv/hvm_domain() checks.

I don't think it's worth doing given that we know that HVM we cant' run
without this feature (we have
BUG_ON(!xen_feature(XENFEAT_hvm_callback_vector)) in xen_hvm_guest_init()).


> ---
>
> diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
> index b59c9455..1477f1d 100644
> --- a/drivers/xen/platform-pci.c
> +++ b/drivers/xen/platform-pci.c
> @@ -42,6 +42,7 @@
>  static unsigned long platform_mmio;
>  static unsigned long platform_mmio_alloc;
>  static unsigned long platform_mmiolen;
> +static uint64_t callback_via;
>  
>  static unsigned long alloc_xen_mmio(unsigned long len)
>  {
> @@ -54,6 +55,51 @@ static unsigned long alloc_xen_mmio(unsigned long len)
>  	return addr;
>  }
>  
> +static uint64_t get_callback_via(struct pci_dev *pdev)
> +{
> +	u8 pin;
> +	int irq;
> +
> +	irq = pdev->irq;
> +	if (irq < 16)
> +		return irq; /* ISA IRQ */
> +
> +	pin = pdev->pin;
> +
> +	/* We don't know the GSI. Specify the PCI INTx line instead. */
> +	return ((uint64_t)0x01 << 56) | /* PCI INTx identifier */

You can use HVM_CALLBACK_VIA_TYPE_SHIFT here.

-boris


> +		((uint64_t)pci_domain_nr(pdev->bus) << 32) |
> +		((uint64_t)pdev->bus->number << 16) |
> +		((uint64_t)(pdev->devfn & 0xff) << 8) |
> +		((uint64_t)(pin - 1) & 3);
> +}
> +
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device"
  2017-01-12 18:27 ` Boris Ostrovsky
@ 2017-01-12 21:33   ` Stefano Stabellini
  2017-01-12 21:37     ` Boris Ostrovsky
  0 siblings, 1 reply; 5+ messages in thread
From: Stefano Stabellini @ 2017-01-12 21:33 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: Stefano Stabellini, jgross, karahmed, linux-kernel, xen-devel,
	Konrad Rzeszutek Wilk

On Thu, 12 Jan 2017, Boris Ostrovsky wrote:
> On 01/11/2017 06:36 PM, Stefano Stabellini wrote:
> > The following commit:
> >
> > commit 72a9b186292d98494f222226cfd24a1621796209
> > Author: KarimAllah Ahmed <karahmed@amazon.de>
> > Date:   Fri Aug 26 23:55:36 2016 +0200
> >
> >     xen: Remove event channel notification through Xen PCI platform device
> >
> > broke Linux when booting as Dom0 on Xen in a nested Xen environment (Xen
> > installed inside a Xen VM). In this scenario, Linux is a PV guest, but
> > at the same time it uses the platform-pci driver to receive
> > notifications from L0 Xen. vector callbacks are not available because L1
> > Xen doesn't allow them.
> 
> (+Konrad who has been running nested)
> 
> >
> > Partially revert the offending commit, by restoring IRQ based
> > notifications for PV guests only. I restored only the code which is
> > strictly needed and replaced the xen_have_vector_callback checks within
> > it with xen_pv_domain() checks.
> >
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> >
> > ---
> > Alternatively, I could also restore the xen_have_vector_callback
> > checks. In general, it's best to have feature flag checks than umbrella
> > xen_pv/hvm_domain() checks.
> 
> I don't think it's worth doing given that we know that HVM we cant' run
> without this feature (we have
> BUG_ON(!xen_feature(XENFEAT_hvm_callback_vector)) in xen_hvm_guest_init()).

OK. I'll add an in-code comment.


> > ---
> >
> > diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
> > index b59c9455..1477f1d 100644
> > --- a/drivers/xen/platform-pci.c
> > +++ b/drivers/xen/platform-pci.c
> > @@ -42,6 +42,7 @@
> >  static unsigned long platform_mmio;
> >  static unsigned long platform_mmio_alloc;
> >  static unsigned long platform_mmiolen;
> > +static uint64_t callback_via;
> >  
> >  static unsigned long alloc_xen_mmio(unsigned long len)
> >  {
> > @@ -54,6 +55,51 @@ static unsigned long alloc_xen_mmio(unsigned long len)
> >  	return addr;
> >  }
> >  
> > +static uint64_t get_callback_via(struct pci_dev *pdev)
> > +{
> > +	u8 pin;
> > +	int irq;
> > +
> > +	irq = pdev->irq;
> > +	if (irq < 16)
> > +		return irq; /* ISA IRQ */
> > +
> > +	pin = pdev->pin;
> > +
> > +	/* We don't know the GSI. Specify the PCI INTx line instead. */
> > +	return ((uint64_t)0x01 << 56) | /* PCI INTx identifier */
> 
> You can use HVM_CALLBACK_VIA_TYPE_SHIFT here.

OK


> 
> > +		((uint64_t)pci_domain_nr(pdev->bus) << 32) |
> > +		((uint64_t)pdev->bus->number << 16) |
> > +		((uint64_t)(pdev->devfn & 0xff) << 8) |
> > +		((uint64_t)(pin - 1) & 3);
> > +}
> > +
> >
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device"
  2017-01-12 21:33   ` Stefano Stabellini
@ 2017-01-12 21:37     ` Boris Ostrovsky
  2017-01-13  1:45       ` Stefano Stabellini
  0 siblings, 1 reply; 5+ messages in thread
From: Boris Ostrovsky @ 2017-01-12 21:37 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: jgross, karahmed, linux-kernel, xen-devel, Konrad Rzeszutek Wilk

On 01/12/2017 04:33 PM, Stefano Stabellini wrote:
> On Thu, 12 Jan 2017, Boris Ostrovsky wrote:
>> On 01/11/2017 06:36 PM, Stefano Stabellini wrote:
>>> The following commit:
>>>
>>> commit 72a9b186292d98494f222226cfd24a1621796209
>>> Author: KarimAllah Ahmed <karahmed@amazon.de>
>>> Date:   Fri Aug 26 23:55:36 2016 +0200
>>>
>>>     xen: Remove event channel notification through Xen PCI platform device

Can you also replace this with

"Commit 72a9b186292d ("xen: Remove event channel notification through
Xen PCI platform device")" ... ?

Thanks.

-boris

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device"
  2017-01-12 21:37     ` Boris Ostrovsky
@ 2017-01-13  1:45       ` Stefano Stabellini
  0 siblings, 0 replies; 5+ messages in thread
From: Stefano Stabellini @ 2017-01-13  1:45 UTC (permalink / raw)
  To: Boris Ostrovsky
  Cc: Stefano Stabellini, jgross, karahmed, linux-kernel, xen-devel,
	Konrad Rzeszutek Wilk

On Thu, 12 Jan 2017, Boris Ostrovsky wrote:
> On 01/12/2017 04:33 PM, Stefano Stabellini wrote:
> > On Thu, 12 Jan 2017, Boris Ostrovsky wrote:
> >> On 01/11/2017 06:36 PM, Stefano Stabellini wrote:
> >>> The following commit:
> >>>
> >>> commit 72a9b186292d98494f222226cfd24a1621796209
> >>> Author: KarimAllah Ahmed <karahmed@amazon.de>
> >>> Date:   Fri Aug 26 23:55:36 2016 +0200
> >>>
> >>>     xen: Remove event channel notification through Xen PCI platform device
> 
> Can you also replace this with
> 
> "Commit 72a9b186292d ("xen: Remove event channel notification through
> Xen PCI platform device")" ... ?

Too late. But if you are Ok with the patch, please fix the message while
committing.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-01-13  1:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-11 23:36 [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device" Stefano Stabellini
2017-01-12 18:27 ` Boris Ostrovsky
2017-01-12 21:33   ` Stefano Stabellini
2017-01-12 21:37     ` Boris Ostrovsky
2017-01-13  1:45       ` Stefano Stabellini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).