2019-03-19 20:04:50

by Jennifer Herbert

[permalink] [raw]
Subject: [PATCH] xen/pv: Add PV specific legacy_pic struct to expose legacy IRQs.

The ACPI tables doesn't always contain all IRQs for legacy devices
such as RTC. Since no PIC controller is visible for a PV linux guest,
under Xen, legacy_pic currently defaults to the null_legacy_pic - with
reports no legacy IRQs. Since the commit "rtc: cmos: Do not assume
irq 8 for rtc when there are no legacy irqs" by Hans de Goede
(commit id: a1e23a42f1bdc00e32fc4869caef12e4e6272f26), the rtc now
incorrectly decides it has no irq it can use, for some hardware.

This patch rectifies the problem by providing a xen legacy_pic
struct, which is much like the null_legacy_pic except that it
reports NR_IRQS_LEGACY irqs.

Signed-off-by: Jennifer Herbert <[email protected]>
---
arch/x86/xen/enlighten_pv.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index c54a493..7644bdf 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -33,6 +33,7 @@
#include <linux/gfp.h>
#include <linux/edd.h>
#include <linux/frame.h>
+#include <linux/irq.h>

#include <xen/xen.h>
#include <xen/events.h>
@@ -49,6 +50,7 @@
#include <xen/acpi.h>

#include <asm/paravirt.h>
+#include <asm/i8259.h>
#include <asm/apic.h>
#include <asm/page.h>
#include <asm/xen/pci.h>
@@ -1188,6 +1190,41 @@ static void __init xen_dom0_set_legacy_features(void)
x86_platform.legacy.rtc = 1;
}

+/*
+ * The ACPI tables doesn't always contain all IRQ's for legacy devices
+ * such as RTC. Since no PIC controller is visible, we'd otherwise
+ * default to the null_legacy_pic - with no legacy IRQs. To allow drivers
+ * to use these IRQs despite this, provide a xen specific legacy_pic
+ * structure, which is noop, other then reporting NR_IRQS_LEGACY irqs.
+ */
+
+static void xen_legacy_pic_noop(void) { };
+static void xen_legacy_pic_uint_noop(unsigned int unused) { };
+static void xen_legacy_pic_int_noop(int unused) { };
+static int xen_legacy_pic_irq_pending_noop(unsigned int irq)
+{
+ return 0;
+}
+
+static int xen_legacy_pic_probe(void)
+{
+ pr_info("Using Xen legacy PIC\n");
+ return nr_legacy_irqs();
+}
+
+struct legacy_pic xen_legacy_pic = {
+ .nr_legacy_irqs = NR_IRQS_LEGACY,
+ .chip = &dummy_irq_chip,
+ .mask = xen_legacy_pic_uint_noop,
+ .unmask = xen_legacy_pic_uint_noop,
+ .mask_all = xen_legacy_pic_noop,
+ .restore_mask = xen_legacy_pic_noop,
+ .init = xen_legacy_pic_int_noop,
+ .probe = xen_legacy_pic_probe,
+ .irq_pending = xen_legacy_pic_irq_pending_noop,
+ .make_irq = xen_legacy_pic_uint_noop,
+};
+
/* First C function to be called on Xen boot */
asmlinkage __visible void __init xen_start_kernel(void)
{
@@ -1267,6 +1304,8 @@ asmlinkage __visible void __init xen_start_kernel(void)

xen_init_capabilities();

+ legacy_pic = &xen_legacy_pic;
+
#ifdef CONFIG_X86_LOCAL_APIC
/*
* set up the basic apic ops.
--
1.8.3.1



2019-03-19 23:10:13

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH] xen/pv: Add PV specific legacy_pic struct to expose legacy IRQs.

On 3/19/19 4:02 PM, Jennifer Herbert wrote:
> The ACPI tables doesn't always contain all IRQs for legacy devices
> such as RTC. Since no PIC controller is visible for a PV linux guest,
> under Xen, legacy_pic currently defaults to the null_legacy_pic - with
> reports no legacy IRQs. Since the commit "rtc: cmos: Do not assume
> irq 8 for rtc when there are no legacy irqs" by Hans de Goede
> (commit id: a1e23a42f1bdc00e32fc4869caef12e4e6272f26), the rtc now
> incorrectly decides it has no irq it can use, for some hardware.
>
> This patch rectifies the problem by providing a xen legacy_pic
> struct, which is much like the null_legacy_pic except that it
> reports NR_IRQS_LEGACY irqs.

I assume this is for dom0?

Could there be the same problem with PVH dom0? (and if yes then this
should probably go into arch/x86/xen/enlighten.c).

-boris


2019-03-21 17:50:57

by Jennifer Herbert

[permalink] [raw]
Subject: Re: [PATCH] xen/pv: Add PV specific legacy_pic struct to expose legacy IRQs.



On 19/03/19 23:06, Boris Ostrovsky wrote:
> On 3/19/19 4:02 PM, Jennifer Herbert wrote:
>> The ACPI tables doesn't always contain all IRQs for legacy devices
>> such as RTC. Since no PIC controller is visible for a PV linux guest,
>> under Xen, legacy_pic currently defaults to the null_legacy_pic - with
>> reports no legacy IRQs. Since the commit "rtc: cmos: Do not assume
>> irq 8 for rtc when there are no legacy irqs" by Hans de Goede
>> (commit id: a1e23a42f1bdc00e32fc4869caef12e4e6272f26), the rtc now
>> incorrectly decides it has no irq it can use, for some hardware.
>>
>> This patch rectifies the problem by providing a xen legacy_pic
>> struct, which is much like the null_legacy_pic except that it
>> reports NR_IRQS_LEGACY irqs.
> I assume this is for dom0?
>
> Could there be the same problem with PVH dom0? (and if yes then this
> should probably go into arch/x86/xen/enlighten.c).
>
> -boris
>

I am doing this to fix a problem with dom0. DomU doesn't seem to have
an RTC, and so it is unaffected.

I'm not familiar with PVH, but have now done some experiments. The RTC
on PVH seems broken - but not quite in the same way as PV. More research
is needed, however simply doing the same trick I did with PV will not
fix the issue.

I'll look further into it.

Cheer,

-jenny


2019-03-25 14:25:46

by Jennifer Herbert

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCH] xen/pv: Add PV specific legacy_pic struct to expose legacy IRQs.



On 21/03/19 17:49, Jennifer Herbert wrote:
>
>
> On 19/03/19 23:06, Boris Ostrovsky wrote:
>> On 3/19/19 4:02 PM, Jennifer Herbert wrote:
>>> The ACPI tables doesn't always contain all IRQs for legacy devices
>>> such as RTC. Since no PIC controller is visible for a PV linux guest,
>>> under Xen, legacy_pic currently defaults to the null_legacy_pic - with
>>> reports no legacy IRQs. Since the commit "rtc: cmos: Do not assume
>>> irq 8 for rtc when there are no legacy irqs" by Hans de Goede
>>> (commit id: a1e23a42f1bdc00e32fc4869caef12e4e6272f26), the rtc now
>>> incorrectly decides it has no irq it can use, for some hardware.
>>>
>>> This patch rectifies the problem by providing a xen legacy_pic
>>> struct, which is much like the null_legacy_pic except that it
>>> reports NR_IRQS_LEGACY irqs.
>> I assume this is for dom0?
>>
>> Could there be the same problem with PVH dom0? (and if yes then this
>> should probably go into arch/x86/xen/enlighten.c).
>>
>> -boris
>>
>
> I am doing this to fix a problem with dom0. DomU doesn't seem to have
> an RTC, and so it is unaffected.
>
> I'm not familiar with PVH, but have now done some experiments. The RTC
> on PVH seems broken - but not quite in the same way as PV. More
> research is needed, however simply doing the same trick I did with PV
> will not fix the issue.
>
> I'll look further into it.
>

The same problem does exist with PVH - however its worse with the
presence of the IO-APIC, as with my patch it tries to set up with IRQ,
and fails. I'm not sure how would be best to deal with this.
However, the RTC seems broken even for machines without the ACPI omission.
I can see fixing it for just PV doesn't seem too nice, but unsure how to
fix this for PVH. I'm open to suggestions, but otherwise I'll put this
on hold.




2019-03-25 14:46:56

by Paul Durrant

[permalink] [raw]
Subject: RE: [Xen-devel] [PATCH] xen/pv: Add PV specific legacy_pic struct to expose legacy IRQs.

> -----Original Message-----
> From: Xen-devel [mailto:[email protected]] On Behalf Of Jennifer Herbert
> Sent: 25 March 2019 14:24
> To: Boris Ostrovsky <[email protected]>; [email protected]; [email protected];
> [email protected]
> Cc: Juergen Gross <[email protected]>; Stefano Stabellini <[email protected]>; Ingo Molnar
> <[email protected]>; Borislav Petkov <[email protected]>; H. Peter Anvin <[email protected]>; Thomas Gleixner
> <[email protected]>
> Subject: Re: [Xen-devel] [PATCH] xen/pv: Add PV specific legacy_pic struct to expose legacy IRQs.
>
>
>
> On 21/03/19 17:49, Jennifer Herbert wrote:
> >
> >
> > On 19/03/19 23:06, Boris Ostrovsky wrote:
> >> On 3/19/19 4:02 PM, Jennifer Herbert wrote:
> >>> The ACPI tables doesn't always contain all IRQs for legacy devices
> >>> such as RTC. Since no PIC controller is visible for a PV linux guest,
> >>> under Xen, legacy_pic currently defaults to the null_legacy_pic - with
> >>> reports no legacy IRQs. Since the commit "rtc: cmos: Do not assume
> >>> irq 8 for rtc when there are no legacy irqs" by Hans de Goede
> >>> (commit id: a1e23a42f1bdc00e32fc4869caef12e4e6272f26), the rtc now
> >>> incorrectly decides it has no irq it can use, for some hardware.
> >>>
> >>> This patch rectifies the problem by providing a xen legacy_pic
> >>> struct, which is much like the null_legacy_pic except that it
> >>> reports NR_IRQS_LEGACY irqs.
> >> I assume this is for dom0?
> >>
> >> Could there be the same problem with PVH dom0? (and if yes then this
> >> should probably go into arch/x86/xen/enlighten.c).
> >>
> >> -boris
> >>
> >
> > I am doing this to fix a problem with dom0. DomU doesn't seem to have
> > an RTC, and so it is unaffected.
> >
> > I'm not familiar with PVH, but have now done some experiments. The RTC
> > on PVH seems broken - but not quite in the same way as PV. More
> > research is needed, however simply doing the same trick I did with PV
> > will not fix the issue.
> >
> > I'll look further into it.
> >
>
> The same problem does exist with PVH - however its worse with the
> presence of the IO-APIC, as with my patch it tries to set up with IRQ,
> and fails. I'm not sure how would be best to deal with this.
> However, the RTC seems broken even for machines without the ACPI omission.
> I can see fixing it for just PV doesn't seem too nice, but unsure how to
> fix this for PVH. I'm open to suggestions, but otherwise I'll put this
> on hold.

AFAICT from the code in libxl__arch_domain_prepare_config(), PVH domains don't get an RTC, just a local APIC.

Paul

>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> [email protected]
> https://lists.xenproject.org/mailman/listinfo/xen-devel

2019-03-27 14:58:19

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCH] xen/pv: Add PV specific legacy_pic struct to expose legacy IRQs.

On 3/25/19 10:40 AM, Paul Durrant wrote:
>> -----Original Message-----
>> From: Xen-devel [mailto:[email protected]] On Behalf Of Jennifer Herbert
>> Sent: 25 March 2019 14:24
>> To: Boris Ostrovsky <[email protected]>; [email protected]; [email protected];
>> [email protected]
>> Cc: Juergen Gross <[email protected]>; Stefano Stabellini <[email protected]>; Ingo Molnar
>> <[email protected]>; Borislav Petkov <[email protected]>; H. Peter Anvin <[email protected]>; Thomas Gleixner
>> <[email protected]>
>> Subject: Re: [Xen-devel] [PATCH] xen/pv: Add PV specific legacy_pic struct to expose legacy IRQs.
>>
>>
>>
>> On 21/03/19 17:49, Jennifer Herbert wrote:
>>>
>>> On 19/03/19 23:06, Boris Ostrovsky wrote:
>>>> On 3/19/19 4:02 PM, Jennifer Herbert wrote:
>>>>> The ACPI tables doesn't always contain all IRQs for legacy devices
>>>>> such as RTC. Since no PIC controller is visible for a PV linux guest,
>>>>> under Xen, legacy_pic currently defaults to the null_legacy_pic - with
>>>>> reports no legacy IRQs. Since the commit "rtc: cmos: Do not assume
>>>>> irq 8 for rtc when there are no legacy irqs" by Hans de Goede
>>>>> (commit id: a1e23a42f1bdc00e32fc4869caef12e4e6272f26), the rtc now
>>>>> incorrectly decides it has no irq it can use, for some hardware.
>>>>>
>>>>> This patch rectifies the problem by providing a xen legacy_pic
>>>>> struct, which is much like the null_legacy_pic except that it
>>>>> reports NR_IRQS_LEGACY irqs.
>>>> I assume this is for dom0?
>>>>
>>>> Could there be the same problem with PVH dom0? (and if yes then this
>>>> should probably go into arch/x86/xen/enlighten.c).
>>>>
>>>> -boris
>>>>
>>> I am doing this to fix a problem with dom0. DomU doesn't seem to have
>>> an RTC, and so it is unaffected.
>>>
>>> I'm not familiar with PVH, but have now done some experiments. The RTC
>>> on PVH seems broken - but not quite in the same way as PV. More
>>> research is needed, however simply doing the same trick I did with PV
>>> will not fix the issue.
>>>
>>> I'll look further into it.
>>>
>> The same problem does exist with PVH - however its worse with the
>> presence of the IO-APIC, as with my patch it tries to set up with IRQ,
>> and fails. I'm not sure how would be best to deal with this.
>> However, the RTC seems broken even for machines without the ACPI omission.
>> I can see fixing it for just PV doesn't seem too nice, but unsure how to
>> fix this for PVH. I'm open to suggestions, but otherwise I'll put this
>> on hold.
> AFAICT from the code in libxl__arch_domain_prepare_config(), PVH domains don't get an RTC, just a local APIC.
>


That's true for domU but not for PVH dom0 I believe. Roger?

-boris