Re: [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device"
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 72a9b186292d98494f26cfd24a1621796209
> >>> Author: KarimAllah Ahmed
> >>> 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.
Re: [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device"
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 72a9b186292d98494f26cfd24a1621796209
>>> Author: KarimAllah Ahmed
>>> 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
Re: [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device"
On Thu, 12 Jan 2017, Boris Ostrovsky wrote:
> On 01/11/2017 06:36 PM, Stefano Stabellini wrote:
> > The following commit:
> >
> > commit 72a9b186292d98494f26cfd24a1621796209
> > Author: KarimAllah Ahmed
> > 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
> >
> > ---
> > 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);
> > +}
> > +
> >
>
>
Re: [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device"
On 01/11/2017 06:36 PM, Stefano Stabellini wrote:
> The following commit:
>
> commit 72a9b186292d98494f26cfd24a1621796209
> Author: KarimAllah Ahmed
> 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
>
> ---
> 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);
> +}
> +
>
