| Message ID | 20200218154712.25490-1-jgross@suse.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 99bcd4a6e5b8ba201fdd252f1054689884899fee |
| Headers | show |
| Series | x86/ioperm: add new paravirt function update_io_bitmap | expand |
Juergen Gross <jgross@suse.com> writes: > Commit 111e7b15cf10f6 ("x86/ioperm: Extend IOPL config to control > ioperm() as well") reworked the iopl syscall to use I/O bitmaps. > > Unfortunately this broke Xen PV domains using that syscall as there > is currently no I/O bitmap support in PV domains. > > Add I/O bitmap support via a new paravirt function update_io_bitmap > which Xen PV domains can use to update their I/O bitmaps via a > hypercall. > > Fixes: 111e7b15cf10f6 ("x86/ioperm: Extend IOPL config to control ioperm() as well") > Reported-by: Jan Beulich <jbeulich@suse.com> > Cc: <stable@vger.kernel.org> # 5.5 > Signed-off-by: Juergen Gross <jgross@suse.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> > Tested-by: Jan Beulich <jbeulich@suse.com> Duh, sorry about that and thanks for fixing it. BTW, why isn't stuff like this not catched during next or at least before the final release? Is nothing running CI on upstream with all that XEN muck active? Thanks, tglx
On 18.02.20 22:03, Thomas Gleixner wrote: > Juergen Gross <jgross@suse.com> writes: >> Commit 111e7b15cf10f6 ("x86/ioperm: Extend IOPL config to control >> ioperm() as well") reworked the iopl syscall to use I/O bitmaps. >> >> Unfortunately this broke Xen PV domains using that syscall as there >> is currently no I/O bitmap support in PV domains. >> >> Add I/O bitmap support via a new paravirt function update_io_bitmap >> which Xen PV domains can use to update their I/O bitmaps via a >> hypercall. >> >> Fixes: 111e7b15cf10f6 ("x86/ioperm: Extend IOPL config to control ioperm() as well") >> Reported-by: Jan Beulich <jbeulich@suse.com> >> Cc: <stable@vger.kernel.org> # 5.5 >> Signed-off-by: Juergen Gross <jgross@suse.com> >> Reviewed-by: Jan Beulich <jbeulich@suse.com> >> Tested-by: Jan Beulich <jbeulich@suse.com> > > Duh, sorry about that and thanks for fixing it. > > BTW, why isn't stuff like this not catched during next or at least > before the final release? Is nothing running CI on upstream with all > that XEN muck active? This problem showed up by not being able to start the X server (probably not the freshest one) in dom0 on a moderate aged AMD system. Our CI tests tend do be more text console based for dom0. Juergen
On 19.02.2020 06:35, Jürgen Groß wrote: > On 18.02.20 22:03, Thomas Gleixner wrote: >> Juergen Gross <jgross@suse.com> writes: >>> Commit 111e7b15cf10f6 ("x86/ioperm: Extend IOPL config to control >>> ioperm() as well") reworked the iopl syscall to use I/O bitmaps. >>> >>> Unfortunately this broke Xen PV domains using that syscall as there >>> is currently no I/O bitmap support in PV domains. >>> >>> Add I/O bitmap support via a new paravirt function update_io_bitmap >>> which Xen PV domains can use to update their I/O bitmaps via a >>> hypercall. >>> >>> Fixes: 111e7b15cf10f6 ("x86/ioperm: Extend IOPL config to control ioperm() as well") >>> Reported-by: Jan Beulich <jbeulich@suse.com> >>> Cc: <stable@vger.kernel.org> # 5.5 >>> Signed-off-by: Juergen Gross <jgross@suse.com> >>> Reviewed-by: Jan Beulich <jbeulich@suse.com> >>> Tested-by: Jan Beulich <jbeulich@suse.com> >> >> Duh, sorry about that and thanks for fixing it. >> >> BTW, why isn't stuff like this not catched during next or at least >> before the final release? Is nothing running CI on upstream with all >> that XEN muck active? > > This problem showed up by not being able to start the X server (probably > not the freshest one) in dom0 on a moderate aged AMD system. Not the freshest one, yes, but also on a system where KMS would not be available (my success rate with KMS is rather low overall, and with newer Linux I see rather more systems to stop working than ones to become working, but I simply don't have the time to investigate). Jan
Jürgen Groß <jgross@suse.com> writes: > On 18.02.20 22:03, Thomas Gleixner wrote: >> BTW, why isn't stuff like this not catched during next or at least >> before the final release? Is nothing running CI on upstream with all >> that XEN muck active? > > This problem showed up by not being able to start the X server (probably > not the freshest one) in dom0 on a moderate aged AMD system. > > Our CI tests tend do be more text console based for dom0. tools/testing/selftests/x86/io[perm|pl] should have caught that as well, right? If not, we need to fix the selftests. Thanks, tglx
On 19.02.20 10:22, Thomas Gleixner wrote: > Jürgen Groß <jgross@suse.com> writes: >> On 18.02.20 22:03, Thomas Gleixner wrote: >>> BTW, why isn't stuff like this not catched during next or at least >>> before the final release? Is nothing running CI on upstream with all >>> that XEN muck active? >> >> This problem showed up by not being able to start the X server (probably >> not the freshest one) in dom0 on a moderate aged AMD system. >> >> Our CI tests tend do be more text console based for dom0. > > tools/testing/selftests/x86/io[perm|pl] should have caught that as well, > right? If not, we need to fix the selftests. Hmm, yes. Thanks for the pointer. Will ask our testing specialist what is done in this regard and how it can be enhanced. Juergen
Friendly ping... On 18.02.20 16:47, Juergen Gross wrote: > Commit 111e7b15cf10f6 ("x86/ioperm: Extend IOPL config to control > ioperm() as well") reworked the iopl syscall to use I/O bitmaps. > > Unfortunately this broke Xen PV domains using that syscall as there > is currently no I/O bitmap support in PV domains. > > Add I/O bitmap support via a new paravirt function update_io_bitmap > which Xen PV domains can use to update their I/O bitmaps via a > hypercall. > > Fixes: 111e7b15cf10f6 ("x86/ioperm: Extend IOPL config to control ioperm() as well") > Reported-by: Jan Beulich <jbeulich@suse.com> > Cc: <stable@vger.kernel.org> # 5.5 > Signed-off-by: Juergen Gross <jgross@suse.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> > Tested-by: Jan Beulich <jbeulich@suse.com> > --- > arch/x86/include/asm/io_bitmap.h | 9 ++++++++- > arch/x86/include/asm/paravirt.h | 7 +++++++ > arch/x86/include/asm/paravirt_types.h | 4 ++++ > arch/x86/kernel/paravirt.c | 5 +++++ > arch/x86/kernel/process.c | 2 +- > arch/x86/xen/enlighten_pv.c | 25 +++++++++++++++++++++++++ > 6 files changed, 50 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/io_bitmap.h b/arch/x86/include/asm/io_bitmap.h > index 02c6ef8f7667..07344d82e88e 100644 > --- a/arch/x86/include/asm/io_bitmap.h > +++ b/arch/x86/include/asm/io_bitmap.h > @@ -19,7 +19,14 @@ struct task_struct; > void io_bitmap_share(struct task_struct *tsk); > void io_bitmap_exit(void); > > -void tss_update_io_bitmap(void); > +void native_tss_update_io_bitmap(void); > + > +#ifdef CONFIG_PARAVIRT_XXL > +#include <asm/paravirt.h> > +#else > +#define tss_update_io_bitmap native_tss_update_io_bitmap > +#endif > + > #else > static inline void io_bitmap_share(struct task_struct *tsk) { } > static inline void io_bitmap_exit(void) { } > diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h > index 86e7317eb31f..694d8daf4983 100644 > --- a/arch/x86/include/asm/paravirt.h > +++ b/arch/x86/include/asm/paravirt.h > @@ -295,6 +295,13 @@ static inline void write_idt_entry(gate_desc *dt, int entry, const gate_desc *g) > PVOP_VCALL3(cpu.write_idt_entry, dt, entry, g); > } > > +#ifdef CONFIG_X86_IOPL_IOPERM > +static inline void tss_update_io_bitmap(void) > +{ > + PVOP_VCALL0(cpu.update_io_bitmap); > +} > +#endif > + > static inline void paravirt_activate_mm(struct mm_struct *prev, > struct mm_struct *next) > { > diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h > index 84812964d3dd..732f62e04ddb 100644 > --- a/arch/x86/include/asm/paravirt_types.h > +++ b/arch/x86/include/asm/paravirt_types.h > @@ -140,6 +140,10 @@ struct pv_cpu_ops { > > void (*load_sp0)(unsigned long sp0); > > +#ifdef CONFIG_X86_IOPL_IOPERM > + void (*update_io_bitmap)(void); > +#endif > + > void (*wbinvd)(void); > > /* cpuid emulation, mostly so that caps bits can be disabled */ > diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c > index 789f5e4f89de..c131ba4e70ef 100644 > --- a/arch/x86/kernel/paravirt.c > +++ b/arch/x86/kernel/paravirt.c > @@ -30,6 +30,7 @@ > #include <asm/timer.h> > #include <asm/special_insns.h> > #include <asm/tlb.h> > +#include <asm/io_bitmap.h> > > /* > * nop stub, which must not clobber anything *including the stack* to > @@ -341,6 +342,10 @@ struct paravirt_patch_template pv_ops = { > .cpu.iret = native_iret, > .cpu.swapgs = native_swapgs, > > +#ifdef CONFIG_X86_IOPL_IOPERM > + .cpu.update_io_bitmap = native_tss_update_io_bitmap, > +#endif > + > .cpu.start_context_switch = paravirt_nop, > .cpu.end_context_switch = paravirt_nop, > > diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c > index 839b5244e3b7..3053c85e0e42 100644 > --- a/arch/x86/kernel/process.c > +++ b/arch/x86/kernel/process.c > @@ -374,7 +374,7 @@ static void tss_copy_io_bitmap(struct tss_struct *tss, struct io_bitmap *iobm) > /** > * tss_update_io_bitmap - Update I/O bitmap before exiting to usermode > */ > -void tss_update_io_bitmap(void) > +void native_tss_update_io_bitmap(void) > { > struct tss_struct *tss = this_cpu_ptr(&cpu_tss_rw); > struct thread_struct *t = ¤t->thread; > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c > index 1f756ffffe8b..feaf2e68ee5c 100644 > --- a/arch/x86/xen/enlighten_pv.c > +++ b/arch/x86/xen/enlighten_pv.c > @@ -72,6 +72,9 @@ > #include <asm/mwait.h> > #include <asm/pci_x86.h> > #include <asm/cpu.h> > +#ifdef CONFIG_X86_IOPL_IOPERM > +#include <asm/io_bitmap.h> > +#endif > > #ifdef CONFIG_ACPI > #include <linux/acpi.h> > @@ -837,6 +840,25 @@ static void xen_load_sp0(unsigned long sp0) > this_cpu_write(cpu_tss_rw.x86_tss.sp0, sp0); > } > > +#ifdef CONFIG_X86_IOPL_IOPERM > +static void xen_update_io_bitmap(void) > +{ > + struct physdev_set_iobitmap iobitmap; > + struct tss_struct *tss = this_cpu_ptr(&cpu_tss_rw); > + > + native_tss_update_io_bitmap(); > + > + iobitmap.bitmap = (uint8_t *)(&tss->x86_tss) + > + tss->x86_tss.io_bitmap_base; > + if (tss->x86_tss.io_bitmap_base == IO_BITMAP_OFFSET_INVALID) > + iobitmap.nr_ports = 0; > + else > + iobitmap.nr_ports = IO_BITMAP_BITS; > + > + HYPERVISOR_physdev_op(PHYSDEVOP_set_iobitmap, &iobitmap); > +} > +#endif > + > static void xen_io_delay(void) > { > } > @@ -1046,6 +1068,9 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = { > .write_idt_entry = xen_write_idt_entry, > .load_sp0 = xen_load_sp0, > > +#ifdef CONFIG_X86_IOPL_IOPERM > + .update_io_bitmap = xen_update_io_bitmap, > +#endif > .io_delay = xen_io_delay, > > /* Xen takes care of %gs when switching to usermode for us */ >
Jürgen Groß <jgross@suse.com> writes:
> Friendly ping...
Ooops. I pick it up first thing tomorrow morning
diff --git a/arch/x86/include/asm/io_bitmap.h b/arch/x86/include/asm/io_bitmap.h index 02c6ef8f7667..07344d82e88e 100644 --- a/arch/x86/include/asm/io_bitmap.h +++ b/arch/x86/include/asm/io_bitmap.h @@ -19,7 +19,14 @@ struct task_struct; void io_bitmap_share(struct task_struct *tsk); void io_bitmap_exit(void); -void tss_update_io_bitmap(void); +void native_tss_update_io_bitmap(void); + +#ifdef CONFIG_PARAVIRT_XXL +#include <asm/paravirt.h> +#else +#define tss_update_io_bitmap native_tss_update_io_bitmap +#endif + #else static inline void io_bitmap_share(struct task_struct *tsk) { } static inline void io_bitmap_exit(void) { } diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h index 86e7317eb31f..694d8daf4983 100644 --- a/arch/x86/include/asm/paravirt.h +++ b/arch/x86/include/asm/paravirt.h @@ -295,6 +295,13 @@ static inline void write_idt_entry(gate_desc *dt, int entry, const gate_desc *g) PVOP_VCALL3(cpu.write_idt_entry, dt, entry, g); } +#ifdef CONFIG_X86_IOPL_IOPERM +static inline void tss_update_io_bitmap(void) +{ + PVOP_VCALL0(cpu.update_io_bitmap); +} +#endif + static inline void paravirt_activate_mm(struct mm_struct *prev, struct mm_struct *next) { diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h index 84812964d3dd..732f62e04ddb 100644 --- a/arch/x86/include/asm/paravirt_types.h +++ b/arch/x86/include/asm/paravirt_types.h @@ -140,6 +140,10 @@ struct pv_cpu_ops { void (*load_sp0)(unsigned long sp0); +#ifdef CONFIG_X86_IOPL_IOPERM + void (*update_io_bitmap)(void); +#endif + void (*wbinvd)(void); /* cpuid emulation, mostly so that caps bits can be disabled */ diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c index 789f5e4f89de..c131ba4e70ef 100644 --- a/arch/x86/kernel/paravirt.c +++ b/arch/x86/kernel/paravirt.c @@ -30,6 +30,7 @@ #include <asm/timer.h> #include <asm/special_insns.h> #include <asm/tlb.h> +#include <asm/io_bitmap.h> /* * nop stub, which must not clobber anything *including the stack* to @@ -341,6 +342,10 @@ struct paravirt_patch_template pv_ops = { .cpu.iret = native_iret, .cpu.swapgs = native_swapgs, +#ifdef CONFIG_X86_IOPL_IOPERM + .cpu.update_io_bitmap = native_tss_update_io_bitmap, +#endif + .cpu.start_context_switch = paravirt_nop, .cpu.end_context_switch = paravirt_nop, diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c index 839b5244e3b7..3053c85e0e42 100644 --- a/arch/x86/kernel/process.c +++ b/arch/x86/kernel/process.c @@ -374,7 +374,7 @@ static void tss_copy_io_bitmap(struct tss_struct *tss, struct io_bitmap *iobm) /** * tss_update_io_bitmap - Update I/O bitmap before exiting to usermode */ -void tss_update_io_bitmap(void) +void native_tss_update_io_bitmap(void) { struct tss_struct *tss = this_cpu_ptr(&cpu_tss_rw); struct thread_struct *t = ¤t->thread; diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c index 1f756ffffe8b..feaf2e68ee5c 100644 --- a/arch/x86/xen/enlighten_pv.c +++ b/arch/x86/xen/enlighten_pv.c @@ -72,6 +72,9 @@ #include <asm/mwait.h> #include <asm/pci_x86.h> #include <asm/cpu.h> +#ifdef CONFIG_X86_IOPL_IOPERM +#include <asm/io_bitmap.h> +#endif #ifdef CONFIG_ACPI #include <linux/acpi.h> @@ -837,6 +840,25 @@ static void xen_load_sp0(unsigned long sp0) this_cpu_write(cpu_tss_rw.x86_tss.sp0, sp0); } +#ifdef CONFIG_X86_IOPL_IOPERM +static void xen_update_io_bitmap(void) +{ + struct physdev_set_iobitmap iobitmap; + struct tss_struct *tss = this_cpu_ptr(&cpu_tss_rw); + + native_tss_update_io_bitmap(); + + iobitmap.bitmap = (uint8_t *)(&tss->x86_tss) + + tss->x86_tss.io_bitmap_base; + if (tss->x86_tss.io_bitmap_base == IO_BITMAP_OFFSET_INVALID) + iobitmap.nr_ports = 0; + else + iobitmap.nr_ports = IO_BITMAP_BITS; + + HYPERVISOR_physdev_op(PHYSDEVOP_set_iobitmap, &iobitmap); +} +#endif + static void xen_io_delay(void) { } @@ -1046,6 +1068,9 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = { .write_idt_entry = xen_write_idt_entry, .load_sp0 = xen_load_sp0, +#ifdef CONFIG_X86_IOPL_IOPERM + .update_io_bitmap = xen_update_io_bitmap, +#endif .io_delay = xen_io_delay, /* Xen takes care of %gs when switching to usermode for us */
Commit 111e7b15cf10f6 ("x86/ioperm: Extend IOPL config to control ioperm() as well") reworked the iopl syscall to use I/O bitmaps. Unfortunately this broke Xen PV domains using that syscall as there is currently no I/O bitmap support in PV domains. Add I/O bitmap support via a new paravirt function update_io_bitmap which Xen PV domains can use to update their I/O bitmaps via a hypercall. Fixes: 111e7b15cf10f6 ("x86/ioperm: Extend IOPL config to control ioperm() as well") Reported-by: Jan Beulich <jbeulich@suse.com> Cc: <stable@vger.kernel.org> # 5.5 Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Tested-by: Jan Beulich <jbeulich@suse.com> --- arch/x86/include/asm/io_bitmap.h | 9 ++++++++- arch/x86/include/asm/paravirt.h | 7 +++++++ arch/x86/include/asm/paravirt_types.h | 4 ++++ arch/x86/kernel/paravirt.c | 5 +++++ arch/x86/kernel/process.c | 2 +- arch/x86/xen/enlighten_pv.c | 25 +++++++++++++++++++++++++ 6 files changed, 50 insertions(+), 2 deletions(-)