| Message ID | 20191015154736.19882-3-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iommu: fixes for interrupt remapping enabling | expand |
On Tue, Oct 15, 2019 at 05:47:34PM +0200, Roger Pau Monne wrote: > There's no need to save and restore the IO-APIC entries, the entries > prior to suspension have already been saved by ioapic_suspend, and > will be restored by ioapic_resume. Note that at the point where > resume_x2apic gets called the IO-APIC has not yet resumed, and hence > all entries should be masked. > > Note this shouldn't introduce any functional change. > > Suggested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> I've tried host suspend without any domU running and it works. I've tested just this patch without others in the series, does it matter? Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > --- > I'm Ccing Marek since I think he usually tests suspend/resume. Could > you give this patch a try? > --- > Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > Cc: Juergen Gross <jgross@suse.com> > --- > Changes since v2: > - New in this version. > --- > xen/arch/x86/apic.c | 27 --------------------------- > 1 file changed, 27 deletions(-) > > diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c > index 6cdb50cf41..0607eb92a8 100644 > --- a/xen/arch/x86/apic.c > +++ b/xen/arch/x86/apic.c > @@ -492,35 +492,8 @@ static void __enable_x2apic(void) > > static void resume_x2apic(void) > { > - struct IO_APIC_route_entry **ioapic_entries = NULL; > - > - ASSERT(x2apic_enabled); > - > - ioapic_entries = alloc_ioapic_entries(); > - if ( !ioapic_entries ) > - { > - printk("Allocate ioapic_entries failed\n"); > - goto out; > - } > - > - if ( save_IO_APIC_setup(ioapic_entries) ) > - { > - printk("Saving IO-APIC state failed\n"); > - goto out; > - } > - > - mask_8259A(); > - mask_IO_APIC_setup(ioapic_entries); > - > iommu_enable_x2apic(); > __enable_x2apic(); > - > - restore_IO_APIC_setup(ioapic_entries); > - unmask_8259A(); > - > -out: > - if ( ioapic_entries ) > - free_ioapic_entries(ioapic_entries); > } > > void setup_local_APIC(void)
On Thu, Oct 17, 2019 at 10:26:08AM +0200, Marek Marczykowski-Górecki wrote: > On Tue, Oct 15, 2019 at 05:47:34PM +0200, Roger Pau Monne wrote: > > There's no need to save and restore the IO-APIC entries, the entries > > prior to suspension have already been saved by ioapic_suspend, and > > will be restored by ioapic_resume. Note that at the point where > > resume_x2apic gets called the IO-APIC has not yet resumed, and hence > > all entries should be masked. > > > > Note this shouldn't introduce any functional change. > > > > Suggested-by: Jan Beulich <jbeulich@suse.com> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > I've tried host suspend without any domU running and it works. I've tested > just this patch without others in the series, does it matter? No that's fine. > Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Thanks.
On 15.10.2019 17:47, Roger Pau Monne wrote: > There's no need to save and restore the IO-APIC entries, the entries > prior to suspension have already been saved by ioapic_suspend, and > will be restored by ioapic_resume. Note that at the point where > resume_x2apic gets called the IO-APIC has not yet resumed, and hence > all entries should be masked. > > Note this shouldn't introduce any functional change. > > Suggested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com>
diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c index 6cdb50cf41..0607eb92a8 100644 --- a/xen/arch/x86/apic.c +++ b/xen/arch/x86/apic.c @@ -492,35 +492,8 @@ static void __enable_x2apic(void) static void resume_x2apic(void) { - struct IO_APIC_route_entry **ioapic_entries = NULL; - - ASSERT(x2apic_enabled); - - ioapic_entries = alloc_ioapic_entries(); - if ( !ioapic_entries ) - { - printk("Allocate ioapic_entries failed\n"); - goto out; - } - - if ( save_IO_APIC_setup(ioapic_entries) ) - { - printk("Saving IO-APIC state failed\n"); - goto out; - } - - mask_8259A(); - mask_IO_APIC_setup(ioapic_entries); - iommu_enable_x2apic(); __enable_x2apic(); - - restore_IO_APIC_setup(ioapic_entries); - unmask_8259A(); - -out: - if ( ioapic_entries ) - free_ioapic_entries(ioapic_entries); } void setup_local_APIC(void)
There's no need to save and restore the IO-APIC entries, the entries prior to suspension have already been saved by ioapic_suspend, and will be restored by ioapic_resume. Note that at the point where resume_x2apic gets called the IO-APIC has not yet resumed, and hence all entries should be masked. Note this shouldn't introduce any functional change. Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- I'm Ccing Marek since I think he usually tests suspend/resume. Could you give this patch a try? --- Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Cc: Juergen Gross <jgross@suse.com> --- Changes since v2: - New in this version. --- xen/arch/x86/apic.c | 27 --------------------------- 1 file changed, 27 deletions(-)