Re: [PATCH] xen: remove
Hi all, On Tue, 2023-10-31 at 12:12 +0200, Oleksii Kurochko wrote: > only declares udelay() function so udelay() > declaration was moved to xen/delay.h. > > For x86, __udelay() was renamed to udelay() and removed > inclusion of in x86 code. > > Signed-off-by: Oleksii Kurochko > --- > xen/arch/arm/include/asm/delay.h | 14 -- > xen/arch/riscv/include/asm/delay.h | 13 - > xen/arch/x86/cpu/microcode/core.c | 2 +- > xen/arch/x86/delay.c | 2 +- > xen/arch/x86/include/asm/delay.h | 13 - > xen/include/xen/delay.h | 3 ++- > 6 files changed, 4 insertions(+), 43 deletions(-) > delete mode 100644 xen/arch/arm/include/asm/delay.h > delete mode 100644 xen/arch/riscv/include/asm/delay.h > delete mode 100644 xen/arch/x86/include/asm/delay.h > > diff --git a/xen/arch/arm/include/asm/delay.h > b/xen/arch/arm/include/asm/delay.h > deleted file mode 100644 > index 042907d9d5..00 > --- a/xen/arch/arm/include/asm/delay.h > +++ /dev/null > @@ -1,14 +0,0 @@ > -#ifndef _ARM_DELAY_H > -#define _ARM_DELAY_H > - > -extern void udelay(unsigned long usecs); > - > -#endif /* defined(_ARM_DELAY_H) */ > -/* > - * Local variables: > - * mode: C > - * c-file-style: "BSD" > - * c-basic-offset: 4 > - * indent-tabs-mode: nil > - * End: > - */ > diff --git a/xen/arch/riscv/include/asm/delay.h > b/xen/arch/riscv/include/asm/delay.h > deleted file mode 100644 > index 2d59622c75..00 > --- a/xen/arch/riscv/include/asm/delay.h > +++ /dev/null > @@ -1,13 +0,0 @@ > -/* SPDX-License-Identifier: GPL-2.0 */ > -/* > - * Copyright (C) 2009 Chen Liqin > - * Copyright (C) 2016 Regents of the University of California > - */ > - > -#ifndef _ASM_RISCV_DELAY_H > -#define _ASM_RISCV_DELAY_H > - > -#define udelay udelay > -extern void udelay(unsigned long usecs); > - > -#endif /* _ASM_RISCV_DELAY_H */ > diff --git a/xen/arch/x86/cpu/microcode/core.c > b/xen/arch/x86/cpu/microcode/core.c > index c3fee62906..48822360c0 100644 > --- a/xen/arch/x86/cpu/microcode/core.c > +++ b/xen/arch/x86/cpu/microcode/core.c > @@ -23,6 +23,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -35,7 +36,6 @@ > > #include > #include > -#include > #include > #include > #include > diff --git a/xen/arch/x86/delay.c b/xen/arch/x86/delay.c > index 2662c26272..b3a41881a1 100644 > --- a/xen/arch/x86/delay.c > +++ b/xen/arch/x86/delay.c > @@ -15,7 +15,7 @@ > #include > #include > > -void __udelay(unsigned long usecs) > +void udelay(unsigned long usecs) > { > unsigned long ticks = usecs * (cpu_khz / 1000); > unsigned long s, e; > diff --git a/xen/arch/x86/include/asm/delay.h > b/xen/arch/x86/include/asm/delay.h > deleted file mode 100644 > index 9be2f46590..00 > --- a/xen/arch/x86/include/asm/delay.h > +++ /dev/null > @@ -1,13 +0,0 @@ > -#ifndef _X86_DELAY_H > -#define _X86_DELAY_H > - > -/* > - * Copyright (C) 1993 Linus Torvalds > - * > - * Delay routines calling functions in arch/i386/lib/delay.c > - */ > - > -extern void __udelay(unsigned long usecs); > -#define udelay(n) __udelay(n) > - > -#endif /* defined(_X86_DELAY_H) */ > diff --git a/xen/include/xen/delay.h b/xen/include/xen/delay.h > index 9d70ef035f..a5189329c7 100644 > --- a/xen/include/xen/delay.h > +++ b/xen/include/xen/delay.h > @@ -3,8 +3,9 @@ > > /* Copyright (C) 1993 Linus Torvalds */ > > -#include > #define mdelay(n) (\ > {unsigned long msec=(n); while (msec--) udelay(1000);}) > > +void udelay(unsigned long usecs); > + > #endif /* defined(_LINUX_DELAY_H) */ I forgot to update xen/arch/{x86,arm,ppc}/include/asm/Makefile: generic-y += delay.h Should I send a new patch version or it can be updated durig merge? ~ Oleksii
Re: [PATCH] xen: remove
On 31/10/2023 10:12, Oleksii Kurochko wrote: only declares udelay() function so udelay() declaration was moved to xen/delay.h. For x86, __udelay() was renamed to udelay() and removed inclusion of in x86 code. Signed-off-by: Oleksii Kurochko For Arm: Acked-by: Julien Grall Cheers, -- Julien Grall
Re: [PATCH] xen: remove
On 31.10.2023 11:12, Oleksii Kurochko wrote: > only declares udelay() function so udelay() > declaration was moved to xen/delay.h. > > For x86, __udelay() was renamed to udelay() and removed > inclusion of in x86 code. > > Signed-off-by: Oleksii Kurochko > --- > xen/arch/arm/include/asm/delay.h | 14 -- > xen/arch/riscv/include/asm/delay.h | 13 - > xen/arch/x86/cpu/microcode/core.c | 2 +- > xen/arch/x86/delay.c | 2 +- > xen/arch/x86/include/asm/delay.h | 13 - > xen/include/xen/delay.h| 3 ++- > 6 files changed, 4 insertions(+), 43 deletions(-) > delete mode 100644 xen/arch/arm/include/asm/delay.h > delete mode 100644 xen/arch/riscv/include/asm/delay.h > delete mode 100644 xen/arch/x86/include/asm/delay.h What about xen/arch/ppc/include/asm/delay.h? With that also removed Reviewed-by: Jan Beulich (and maybe also Suggested-by:?) Jan
[PATCH] xen: remove
only declares udelay() function so udelay() declaration was moved to xen/delay.h. For x86, __udelay() was renamed to udelay() and removed inclusion of in x86 code. Signed-off-by: Oleksii Kurochko --- xen/arch/arm/include/asm/delay.h | 14 -- xen/arch/riscv/include/asm/delay.h | 13 - xen/arch/x86/cpu/microcode/core.c | 2 +- xen/arch/x86/delay.c | 2 +- xen/arch/x86/include/asm/delay.h | 13 - xen/include/xen/delay.h| 3 ++- 6 files changed, 4 insertions(+), 43 deletions(-) delete mode 100644 xen/arch/arm/include/asm/delay.h delete mode 100644 xen/arch/riscv/include/asm/delay.h delete mode 100644 xen/arch/x86/include/asm/delay.h diff --git a/xen/arch/arm/include/asm/delay.h b/xen/arch/arm/include/asm/delay.h deleted file mode 100644 index 042907d9d5..00 --- a/xen/arch/arm/include/asm/delay.h +++ /dev/null @@ -1,14 +0,0 @@ -#ifndef _ARM_DELAY_H -#define _ARM_DELAY_H - -extern void udelay(unsigned long usecs); - -#endif /* defined(_ARM_DELAY_H) */ -/* - * Local variables: - * mode: C - * c-file-style: "BSD" - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ diff --git a/xen/arch/riscv/include/asm/delay.h b/xen/arch/riscv/include/asm/delay.h deleted file mode 100644 index 2d59622c75..00 --- a/xen/arch/riscv/include/asm/delay.h +++ /dev/null @@ -1,13 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -/* - * Copyright (C) 2009 Chen Liqin - * Copyright (C) 2016 Regents of the University of California - */ - -#ifndef _ASM_RISCV_DELAY_H -#define _ASM_RISCV_DELAY_H - -#define udelay udelay -extern void udelay(unsigned long usecs); - -#endif /* _ASM_RISCV_DELAY_H */ diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c index c3fee62906..48822360c0 100644 --- a/xen/arch/x86/cpu/microcode/core.c +++ b/xen/arch/x86/cpu/microcode/core.c @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -35,7 +36,6 @@ #include #include -#include #include #include #include diff --git a/xen/arch/x86/delay.c b/xen/arch/x86/delay.c index 2662c26272..b3a41881a1 100644 --- a/xen/arch/x86/delay.c +++ b/xen/arch/x86/delay.c @@ -15,7 +15,7 @@ #include #include -void __udelay(unsigned long usecs) +void udelay(unsigned long usecs) { unsigned long ticks = usecs * (cpu_khz / 1000); unsigned long s, e; diff --git a/xen/arch/x86/include/asm/delay.h b/xen/arch/x86/include/asm/delay.h deleted file mode 100644 index 9be2f46590..00 --- a/xen/arch/x86/include/asm/delay.h +++ /dev/null @@ -1,13 +0,0 @@ -#ifndef _X86_DELAY_H -#define _X86_DELAY_H - -/* - * Copyright (C) 1993 Linus Torvalds - * - * Delay routines calling functions in arch/i386/lib/delay.c - */ - -extern void __udelay(unsigned long usecs); -#define udelay(n) __udelay(n) - -#endif /* defined(_X86_DELAY_H) */ diff --git a/xen/include/xen/delay.h b/xen/include/xen/delay.h index 9d70ef035f..a5189329c7 100644 --- a/xen/include/xen/delay.h +++ b/xen/include/xen/delay.h @@ -3,8 +3,9 @@ /* Copyright (C) 1993 Linus Torvalds */ -#include #define mdelay(n) (\ {unsigned long msec=(n); while (msec--) udelay(1000);}) +void udelay(unsigned long usecs); + #endif /* defined(_LINUX_DELAY_H) */ -- 2.41.0
Re: [PATCH] xen: remove unnecessary (void*) conversions
On 16.03.23 09:39, Yu Zhe wrote: Pointer variables of void * type do not require type cast. Signed-off-by: Yu Zhe Reviewed-by: Juergen Gross Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
[PATCH] xen: remove unnecessary (void*) conversions
Pointer variables of void * type do not require type cast. Signed-off-by: Yu Zhe --- drivers/xen/xenfs/xensyms.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/xen/xenfs/xensyms.c b/drivers/xen/xenfs/xensyms.c index c6c73a33c44d..b799bc759c15 100644 --- a/drivers/xen/xenfs/xensyms.c +++ b/drivers/xen/xenfs/xensyms.c @@ -64,7 +64,7 @@ static int xensyms_next_sym(struct xensyms *xs) static void *xensyms_start(struct seq_file *m, loff_t *pos) { - struct xensyms *xs = (struct xensyms *)m->private; + struct xensyms *xs = m->private; xs->op.u.symdata.symnum = *pos; @@ -76,7 +76,7 @@ static void *xensyms_start(struct seq_file *m, loff_t *pos) static void *xensyms_next(struct seq_file *m, void *p, loff_t *pos) { - struct xensyms *xs = (struct xensyms *)m->private; + struct xensyms *xs = m->private; xs->op.u.symdata.symnum = ++(*pos); @@ -88,7 +88,7 @@ static void *xensyms_next(struct seq_file *m, void *p, loff_t *pos) static int xensyms_show(struct seq_file *m, void *p) { - struct xensyms *xs = (struct xensyms *)m->private; + struct xensyms *xs = m->private; struct xenpf_symdata *symdata = >op.u.symdata; seq_printf(m, "%016llx %c %s\n", symdata->address, @@ -120,7 +120,7 @@ static int xensyms_open(struct inode *inode, struct file *file) return ret; m = file->private_data; - xs = (struct xensyms *)m->private; + xs = m->private; xs->namelen = XEN_KSYM_NAME_LEN + 1; xs->name = kzalloc(xs->namelen, GFP_KERNEL); @@ -138,7 +138,7 @@ static int xensyms_open(struct inode *inode, struct file *file) static int xensyms_release(struct inode *inode, struct file *file) { struct seq_file *m = file->private_data; - struct xensyms *xs = (struct xensyms *)m->private; + struct xensyms *xs = m->private; kfree(xs->name); return seq_release_private(inode, file); -- 2.11.0
Re: [PATCH] xen: Remove the use of K functions
On 17.02.2023 00:17, Andrew Cooper wrote: > On 16/02/2023 11:02 pm, Andrew Cooper wrote: >> On 16/02/2023 10:44 pm, Andrew Cooper wrote: >>> Clang-15 (as seen in the FreeBSD 14 tests) complains: >>> >>> arch/x86/time.c:1364:20: error: a function declaration without a >>> prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes] >>> s_time_t get_s_time() >>> ^ >>> void >>> >>> The error message is a bit confusing but appears to new as part of >>> -Wdeprecated-non-prototype which is part of supporting C2x which formally >>> removes K syntax. >>> >>> Either way, fix the offending functions. >>> >>> Signed-off-by: Andrew Cooper >>> --- >>> CC: Jan Beulich >>> CC: Roger Pau Monné >>> CC: Wei Liu >>> >>> These are all the examples found in a default build of Xen. I'm still >>> finding >>> toolstack violations. >> Apparently not. int cf_check vmx_cpu_up() too. > > Ok, finally got a clean Clang-15 build. I've folded this hunk into the > patch: > > diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c > index 09edbd23b399..e1c268789e7e 100644 > --- a/xen/arch/x86/hvm/vmx/vmcs.c > +++ b/xen/arch/x86/hvm/vmx/vmcs.c > @@ -781,7 +781,7 @@ static int _vmx_cpu_up(bool bsp) > return 0; > } > > -int cf_check vmx_cpu_up() > +int cf_check vmx_cpu_up(void) > { > return _vmx_cpu_up(false); > } > > > but am not intending to send a v2 given how trivial it is. Sure. Reviewed-by: Jan Beulich Jan
Re: [PATCH] xen: Remove the use of K functions
On 16/02/2023 11:02 pm, Andrew Cooper wrote: > On 16/02/2023 10:44 pm, Andrew Cooper wrote: >> Clang-15 (as seen in the FreeBSD 14 tests) complains: >> >> arch/x86/time.c:1364:20: error: a function declaration without a >> prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes] >> s_time_t get_s_time() >> ^ >> void >> >> The error message is a bit confusing but appears to new as part of >> -Wdeprecated-non-prototype which is part of supporting C2x which formally >> removes K syntax. >> >> Either way, fix the offending functions. >> >> Signed-off-by: Andrew Cooper >> --- >> CC: Jan Beulich >> CC: Roger Pau Monné >> CC: Wei Liu >> >> These are all the examples found in a default build of Xen. I'm still >> finding >> toolstack violations. > Apparently not. int cf_check vmx_cpu_up() too. Ok, finally got a clean Clang-15 build. I've folded this hunk into the patch: diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index 09edbd23b399..e1c268789e7e 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -781,7 +781,7 @@ static int _vmx_cpu_up(bool bsp) return 0; } -int cf_check vmx_cpu_up() +int cf_check vmx_cpu_up(void) { return _vmx_cpu_up(false); } but am not intending to send a v2 given how trivial it is. ~Andrew
Re: [PATCH] xen: Remove the use of K functions
On 16/02/2023 10:44 pm, Andrew Cooper wrote: > Clang-15 (as seen in the FreeBSD 14 tests) complains: > > arch/x86/time.c:1364:20: error: a function declaration without a > prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes] > s_time_t get_s_time() > ^ > void > > The error message is a bit confusing but appears to new as part of > -Wdeprecated-non-prototype which is part of supporting C2x which formally > removes K syntax. > > Either way, fix the offending functions. > > Signed-off-by: Andrew Cooper > --- > CC: Jan Beulich > CC: Roger Pau Monné > CC: Wei Liu > > These are all the examples found in a default build of Xen. I'm still finding > toolstack violations. Apparently not. int cf_check vmx_cpu_up() too. ~Andrew
[PATCH] xen: Remove the use of K functions
Clang-15 (as seen in the FreeBSD 14 tests) complains: arch/x86/time.c:1364:20: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes] s_time_t get_s_time() ^ void The error message is a bit confusing but appears to new as part of -Wdeprecated-non-prototype which is part of supporting C2x which formally removes K syntax. Either way, fix the offending functions. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Roger Pau Monné CC: Wei Liu These are all the examples found in a default build of Xen. I'm still finding toolstack violations. --- xen/arch/x86/time.c | 2 +- xen/drivers/passthrough/iommu.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c index 782b11c8a97b..4e44a43cc5e8 100644 --- a/xen/arch/x86/time.c +++ b/xen/arch/x86/time.c @@ -1361,7 +1361,7 @@ s_time_t get_s_time_fixed(u64 at_tsc) return t->stamp.local_stime + scale_delta(delta, >tsc_scale); } -s_time_t get_s_time() +s_time_t get_s_time(void) { return get_s_time_fixed(0); } diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index 921b71e81904..0e187f6ae33c 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -606,7 +606,7 @@ int __init iommu_setup(void) return rc; } -int iommu_suspend() +int iommu_suspend(void) { if ( iommu_enabled ) return iommu_call(iommu_get_ops(), suspend); @@ -614,7 +614,7 @@ int iommu_suspend() return 0; } -void iommu_resume() +void iommu_resume(void) { if ( iommu_enabled ) iommu_vcall(iommu_get_ops(), resume); -- 2.30.2
Re: [PATCH] xen: Remove the arch specific header init.h
On 11.01.2023 12:44, Julien Grall wrote: > From: Julien Grall > > Both x86 and (soon) RISC-V version of init.h are empty. On Arm, it contains > a structure that should not be used by any common code. > > The structure init_info is used to store information to setup the CPU > currently being brought-up. setup.h seems to be more suitable even though > the header is getting quite crowded. > > Looking through the history, was introduced at the same > time as the ia64 port because for some reasons most of the macros > where duplicated. This was changed in 72c07f413879 and I don't > foresee any reason to require arch specific definition for init.h > in the near future. > > Therefore remove asm/init.h for both x86 and arm (the only definition > is moved in setup.h). With that RISC-V will not need to introduce > an empty header. > > Suggested-by: Jan Beulich > Signed-off-by: Julien Grall Reviewed-by: Jan Beulich
Re: [PATCH] xen: Remove the arch specific header init.h
> On 11 Jan 2023, at 11:44, Julien Grall wrote: > > From: Julien Grall > > Both x86 and (soon) RISC-V version of init.h are empty. On Arm, it contains > a structure that should not be used by any common code. > > The structure init_info is used to store information to setup the CPU > currently being brought-up. setup.h seems to be more suitable even though > the header is getting quite crowded. > > Looking through the history, was introduced at the same > time as the ia64 port because for some reasons most of the macros > where duplicated. This was changed in 72c07f413879 and I don't > foresee any reason to require arch specific definition for init.h > in the near future. > > Therefore remove asm/init.h for both x86 and arm (the only definition > is moved in setup.h). With that RISC-V will not need to introduce > an empty header. > > Suggested-by: Jan Beulich > Signed-off-by: Julien Grall > Hi Julien, The arm part looks good to me, I’ve also built x86, arm32 and arm64 and no problems found. Reviewed-by: Luca Fancellu
Re: [PATCH] xen: Remove the arch specific header init.h
On Wed, Jan 11, 2023 at 9:44 PM Julien Grall wrote: > > From: Julien Grall > > Both x86 and (soon) RISC-V version of init.h are empty. On Arm, it contains > a structure that should not be used by any common code. > > The structure init_info is used to store information to setup the CPU > currently being brought-up. setup.h seems to be more suitable even though > the header is getting quite crowded. > > Looking through the history, was introduced at the same > time as the ia64 port because for some reasons most of the macros > where duplicated. This was changed in 72c07f413879 and I don't > foresee any reason to require arch specific definition for init.h > in the near future. > > Therefore remove asm/init.h for both x86 and arm (the only definition > is moved in setup.h). With that RISC-V will not need to introduce > an empty header. > > Suggested-by: Jan Beulich > Signed-off-by: Julien Grall Acked-by: Alistair Francis Alistair > > --- > cc: Oleksii Kurochko > --- > xen/arch/arm/arm32/asm-offsets.c | 1 + > xen/arch/arm/arm64/asm-offsets.c | 1 + > xen/arch/arm/include/asm/init.h | 20 > xen/arch/arm/include/asm/setup.h | 8 > xen/arch/x86/acpi/power.c| 1 - > xen/arch/x86/include/asm/init.h | 4 > xen/include/xen/init.h | 2 -- > 7 files changed, 10 insertions(+), 27 deletions(-) > delete mode 100644 xen/arch/arm/include/asm/init.h > delete mode 100644 xen/arch/x86/include/asm/init.h > > diff --git a/xen/arch/arm/arm32/asm-offsets.c > b/xen/arch/arm/arm32/asm-offsets.c > index 2116ba5b95bf..05c692bb2822 100644 > --- a/xen/arch/arm/arm32/asm-offsets.c > +++ b/xen/arch/arm/arm32/asm-offsets.c > @@ -11,6 +11,7 @@ > #include > #include > #include > +#include > > #define DEFINE(_sym, _val) \ > asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \ > diff --git a/xen/arch/arm/arm64/asm-offsets.c > b/xen/arch/arm/arm64/asm-offsets.c > index 280ddb55bfd4..7226cd9b2eb0 100644 > --- a/xen/arch/arm/arm64/asm-offsets.c > +++ b/xen/arch/arm/arm64/asm-offsets.c > @@ -10,6 +10,7 @@ > #include > #include > #include > +#include > #include > > #define DEFINE(_sym, _val) \ > diff --git a/xen/arch/arm/include/asm/init.h b/xen/arch/arm/include/asm/init.h > deleted file mode 100644 > index 5ac8cf8797d6.. > --- a/xen/arch/arm/include/asm/init.h > +++ /dev/null > @@ -1,20 +0,0 @@ > -#ifndef _XEN_ASM_INIT_H > -#define _XEN_ASM_INIT_H > - > -struct init_info > -{ > -/* Pointer to the stack, used by head.S when entering in C */ > -unsigned char *stack; > -/* Logical CPU ID, used by start_secondary */ > -unsigned int cpuid; > -}; > - > -#endif /* _XEN_ASM_INIT_H */ > -/* > - * Local variables: > - * mode: C > - * c-file-style: "BSD" > - * c-basic-offset: 4 > - * indent-tabs-mode: nil > - * End: > - */ > diff --git a/xen/arch/arm/include/asm/setup.h > b/xen/arch/arm/include/asm/setup.h > index fdbf68aadcaa..a926f30a2be4 100644 > --- a/xen/arch/arm/include/asm/setup.h > +++ b/xen/arch/arm/include/asm/setup.h > @@ -168,6 +168,14 @@ int map_range_to_domain(const struct dt_device_node *dev, > > extern const char __ro_after_init_start[], __ro_after_init_end[]; > > +struct init_info > +{ > +/* Pointer to the stack, used by head.S when entering in C */ > +unsigned char *stack; > +/* Logical CPU ID, used by start_secondary */ > +unsigned int cpuid; > +}; > + > #endif > /* > * Local variables: > diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c > index b76f673acb1a..d23335391c67 100644 > --- a/xen/arch/x86/acpi/power.c > +++ b/xen/arch/x86/acpi/power.c > @@ -17,7 +17,6 @@ > #include > #include > #include > -#include > #include > #include > #include > diff --git a/xen/arch/x86/include/asm/init.h b/xen/arch/x86/include/asm/init.h > deleted file mode 100644 > index 5295b35e6337.. > --- a/xen/arch/x86/include/asm/init.h > +++ /dev/null > @@ -1,4 +0,0 @@ > -#ifndef _XEN_ASM_INIT_H > -#define _XEN_ASM_INIT_H > - > -#endif /* _XEN_ASM_INIT_H */ > diff --git a/xen/include/xen/init.h b/xen/include/xen/init.h > index 0af0e234ec80..1d7c0216bc80 100644 > --- a/xen/include/xen/init.h > +++ b/xen/include/xen/init.h > @@ -1,8 +1,6 @@ > #ifndef _LINUX_INIT_H > #define _LINUX_INIT_H > > -#include > - > /* > * Mark functions and data as being only used at initialization > * or exit time. > -- > 2.38.1 > >
Re: [PATCH] xen: Remove the arch specific header init.h
On 11/01/2023 11:44 am, Julien Grall wrote: > From: Julien Grall > > Both x86 and (soon) RISC-V version of init.h are empty. On Arm, it contains > a structure that should not be used by any common code. > > The structure init_info is used to store information to setup the CPU > currently being brought-up. setup.h seems to be more suitable even though > the header is getting quite crowded. > > Looking through the history, was introduced at the same > time as the ia64 port because for some reasons most of the macros > where duplicated. This was changed in 72c07f413879 and I don't > foresee any reason to require arch specific definition for init.h > in the near future. > > Therefore remove asm/init.h for both x86 and arm (the only definition > is moved in setup.h). With that RISC-V will not need to introduce > an empty header. > > Suggested-by: Jan Beulich > Signed-off-by: Julien Grall Acked-by: Andrew Cooper
[PATCH] xen: Remove the arch specific header init.h
From: Julien Grall Both x86 and (soon) RISC-V version of init.h are empty. On Arm, it contains a structure that should not be used by any common code. The structure init_info is used to store information to setup the CPU currently being brought-up. setup.h seems to be more suitable even though the header is getting quite crowded. Looking through the history, was introduced at the same time as the ia64 port because for some reasons most of the macros where duplicated. This was changed in 72c07f413879 and I don't foresee any reason to require arch specific definition for init.h in the near future. Therefore remove asm/init.h for both x86 and arm (the only definition is moved in setup.h). With that RISC-V will not need to introduce an empty header. Suggested-by: Jan Beulich Signed-off-by: Julien Grall --- cc: Oleksii Kurochko --- xen/arch/arm/arm32/asm-offsets.c | 1 + xen/arch/arm/arm64/asm-offsets.c | 1 + xen/arch/arm/include/asm/init.h | 20 xen/arch/arm/include/asm/setup.h | 8 xen/arch/x86/acpi/power.c| 1 - xen/arch/x86/include/asm/init.h | 4 xen/include/xen/init.h | 2 -- 7 files changed, 10 insertions(+), 27 deletions(-) delete mode 100644 xen/arch/arm/include/asm/init.h delete mode 100644 xen/arch/x86/include/asm/init.h diff --git a/xen/arch/arm/arm32/asm-offsets.c b/xen/arch/arm/arm32/asm-offsets.c index 2116ba5b95bf..05c692bb2822 100644 --- a/xen/arch/arm/arm32/asm-offsets.c +++ b/xen/arch/arm/arm32/asm-offsets.c @@ -11,6 +11,7 @@ #include #include #include +#include #define DEFINE(_sym, _val) \ asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \ diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c index 280ddb55bfd4..7226cd9b2eb0 100644 --- a/xen/arch/arm/arm64/asm-offsets.c +++ b/xen/arch/arm/arm64/asm-offsets.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #define DEFINE(_sym, _val) \ diff --git a/xen/arch/arm/include/asm/init.h b/xen/arch/arm/include/asm/init.h deleted file mode 100644 index 5ac8cf8797d6.. --- a/xen/arch/arm/include/asm/init.h +++ /dev/null @@ -1,20 +0,0 @@ -#ifndef _XEN_ASM_INIT_H -#define _XEN_ASM_INIT_H - -struct init_info -{ -/* Pointer to the stack, used by head.S when entering in C */ -unsigned char *stack; -/* Logical CPU ID, used by start_secondary */ -unsigned int cpuid; -}; - -#endif /* _XEN_ASM_INIT_H */ -/* - * Local variables: - * mode: C - * c-file-style: "BSD" - * c-basic-offset: 4 - * indent-tabs-mode: nil - * End: - */ diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h index fdbf68aadcaa..a926f30a2be4 100644 --- a/xen/arch/arm/include/asm/setup.h +++ b/xen/arch/arm/include/asm/setup.h @@ -168,6 +168,14 @@ int map_range_to_domain(const struct dt_device_node *dev, extern const char __ro_after_init_start[], __ro_after_init_end[]; +struct init_info +{ +/* Pointer to the stack, used by head.S when entering in C */ +unsigned char *stack; +/* Logical CPU ID, used by start_secondary */ +unsigned int cpuid; +}; + #endif /* * Local variables: diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c index b76f673acb1a..d23335391c67 100644 --- a/xen/arch/x86/acpi/power.c +++ b/xen/arch/x86/acpi/power.c @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include diff --git a/xen/arch/x86/include/asm/init.h b/xen/arch/x86/include/asm/init.h deleted file mode 100644 index 5295b35e6337.. --- a/xen/arch/x86/include/asm/init.h +++ /dev/null @@ -1,4 +0,0 @@ -#ifndef _XEN_ASM_INIT_H -#define _XEN_ASM_INIT_H - -#endif /* _XEN_ASM_INIT_H */ diff --git a/xen/include/xen/init.h b/xen/include/xen/init.h index 0af0e234ec80..1d7c0216bc80 100644 --- a/xen/include/xen/init.h +++ b/xen/include/xen/init.h @@ -1,8 +1,6 @@ #ifndef _LINUX_INIT_H #define _LINUX_INIT_H -#include - /* * Mark functions and data as being only used at initialization * or exit time. -- 2.38.1
Re: [PATCH] xen: Remove trigraphs from comments
On 06.12.2022 15:05, Michal Orzel wrote: > On 06/12/2022 14:46, Jan Beulich wrote: >> On 06.12.2022 14:05, Michal Orzel wrote: >>> Also there is no functional change being made by this patch so it is ok >>> to change Xen and not Linux in this case (which has quite a lot of >>> trigraphs all over the place). >> >> Based on what criteria are you saying this is ok (unconditionally)? > > I said that it is ok to change Xen and not Linux because this file already > diverged, > so we cannot assume that future backports will apply cleanly. If we change a > file > that did not diverge, then we are required to modify the origin first and then > do the backport. A file might have diverged, yet commits to be pulled in from the original tree may still apply cleanly. That why, in the general case, we aim at limiting the amount of divergence, and we prefer to pull in changes bringing us back closer to the (meanwhile evolved) original file. Jan
Re: [PATCH] xen: Remove trigraphs from comments
On 06/12/2022 14:46, Jan Beulich wrote: > > > On 06.12.2022 14:05, Michal Orzel wrote: >> On 06/12/2022 13:42, Jan Beulich wrote: >>> On 06.12.2022 11:59, Michal Orzel wrote: --- a/xen/include/xen/pci_regs.h +++ b/xen/include/xen/pci_regs.h @@ -246,13 +246,13 @@ #define PCI_PM_CTRL_STATE_MASK 0x0003 /* Current power state (D0 to D3) */ #define PCI_PM_CTRL_NO_SOFT_RESET 0x0008 /* No reset for D3hot->D0 */ #define PCI_PM_CTRL_PME_ENABLE 0x0100 /* PME pin enable */ -#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (??) */ -#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (??) */ +#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (?) */ +#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (?) */ #define PCI_PM_CTRL_PME_STATUS 0x8000 /* PME pin status */ -#define PCI_PM_PPB_EXTENSIONS6 /* PPB support extensions (??) */ -#define PCI_PM_PPB_B2_B30x40/* Stop clock when in D3hot (??) */ -#define PCI_PM_BPCC_ENABLE 0x80/* Bus power/clock control enable (??) */ -#define PCI_PM_DATA_REGISTER 7 /* (??) */ +#define PCI_PM_PPB_EXTENSIONS6 /* PPB support extensions (?) */ +#define PCI_PM_PPB_B2_B30x40/* Stop clock when in D3hot (?) */ +#define PCI_PM_BPCC_ENABLE 0x80/* Bus power/clock control enable (?) */ +#define PCI_PM_DATA_REGISTER 7 /* (?) */ #define PCI_PM_SIZEOF8 >>> >>> We've inherited all of these from Linux, and I notice Linux still has it >>> this same way. Ideally Linux would change first, but I'd be okay with a >>> sentence added to the description clarifying that we knowingly accept >>> the divergence. >> This file already diverged and we are not in sync with Linux as well. > > Of course - that's the case for the majority of files we've taken from > somewhere. But a Linux patch dropping the (??) parts of the comment > (perhaps once whoever had put them there convinced themselves that the > names of the constants and/or the comments are valid / applicable) > would then no longer apply cleanly if we wanted to carry it over. > Hence my request for amending the description. I'm totally fine to add an extra sentence. > >> Also there is no functional change being made by this patch so it is ok >> to change Xen and not Linux in this case (which has quite a lot of trigraphs >> all over the place). > > Based on what criteria are you saying this is ok (unconditionally)? I said that it is ok to change Xen and not Linux because this file already diverged, so we cannot assume that future backports will apply cleanly. If we change a file that did not diverge, then we are required to modify the origin first and then do the backport. > > Jan ~Michal
Re: [PATCH] xen: Remove trigraphs from comments
On 06.12.2022 14:05, Michal Orzel wrote: > On 06/12/2022 13:42, Jan Beulich wrote: >> On 06.12.2022 11:59, Michal Orzel wrote: >>> --- a/xen/include/xen/pci_regs.h >>> +++ b/xen/include/xen/pci_regs.h >>> @@ -246,13 +246,13 @@ >>> #define PCI_PM_CTRL_STATE_MASK 0x0003 /* Current power state (D0 to >>> D3) */ >>> #define PCI_PM_CTRL_NO_SOFT_RESET 0x0008 /* No reset for D3hot->D0 */ >>> #define PCI_PM_CTRL_PME_ENABLE 0x0100 /* PME pin enable */ >>> -#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (??) */ >>> -#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (??) */ >>> +#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (?) */ >>> +#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (?) */ >>> #define PCI_PM_CTRL_PME_STATUS 0x8000 /* PME pin status */ >>> -#define PCI_PM_PPB_EXTENSIONS6 /* PPB support extensions >>> (??) */ >>> -#define PCI_PM_PPB_B2_B30x40/* Stop clock when in D3hot (??) */ >>> -#define PCI_PM_BPCC_ENABLE 0x80/* Bus power/clock control enable >>> (??) */ >>> -#define PCI_PM_DATA_REGISTER 7 /* (??) */ >>> +#define PCI_PM_PPB_EXTENSIONS6 /* PPB support extensions (?) >>> */ >>> +#define PCI_PM_PPB_B2_B30x40/* Stop clock when in D3hot (?) */ >>> +#define PCI_PM_BPCC_ENABLE 0x80/* Bus power/clock control enable (?) >>> */ >>> +#define PCI_PM_DATA_REGISTER 7 /* (?) */ >>> #define PCI_PM_SIZEOF8 >> >> We've inherited all of these from Linux, and I notice Linux still has it >> this same way. Ideally Linux would change first, but I'd be okay with a >> sentence added to the description clarifying that we knowingly accept >> the divergence. > This file already diverged and we are not in sync with Linux as well. Of course - that's the case for the majority of files we've taken from somewhere. But a Linux patch dropping the (??) parts of the comment (perhaps once whoever had put them there convinced themselves that the names of the constants and/or the comments are valid / applicable) would then no longer apply cleanly if we wanted to carry it over. Hence my request for amending the description. > Also there is no functional change being made by this patch so it is ok > to change Xen and not Linux in this case (which has quite a lot of trigraphs > all over the place). Based on what criteria are you saying this is ok (unconditionally)? Jan
Re: [PATCH] xen: Remove trigraphs from comments
Hi Jan, On 06/12/2022 13:42, Jan Beulich wrote: > > > On 06.12.2022 11:59, Michal Orzel wrote: >> --- a/xen/include/public/arch-x86_64.h >> +++ b/xen/include/public/arch-x86_64.h >> @@ -22,5 +22,5 @@ >> * A similar callback occurs if the segment selectors are invalid. >> * failsafe_address is used as the value of eip. >> * >> - * On x86_64, event_selector and failsafe_selector are ignored (???). >> + * On x86_64, event_selector and failsafe_selector are ignored (?). > > May I ask that this become (?!?) instead? Sure. > >> --- a/xen/include/xen/pci_regs.h >> +++ b/xen/include/xen/pci_regs.h >> @@ -246,13 +246,13 @@ >> #define PCI_PM_CTRL_STATE_MASK 0x0003 /* Current power state (D0 to >> D3) */ >> #define PCI_PM_CTRL_NO_SOFT_RESET 0x0008 /* No reset for D3hot->D0 */ >> #define PCI_PM_CTRL_PME_ENABLE 0x0100 /* PME pin enable */ >> -#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (??) */ >> -#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (??) */ >> +#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (?) */ >> +#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (?) */ >> #define PCI_PM_CTRL_PME_STATUS 0x8000 /* PME pin status */ >> -#define PCI_PM_PPB_EXTENSIONS6 /* PPB support extensions (??) >> */ >> -#define PCI_PM_PPB_B2_B30x40/* Stop clock when in D3hot (??) */ >> -#define PCI_PM_BPCC_ENABLE 0x80/* Bus power/clock control enable (??) >> */ >> -#define PCI_PM_DATA_REGISTER 7 /* (??) */ >> +#define PCI_PM_PPB_EXTENSIONS6 /* PPB support extensions (?) >> */ >> +#define PCI_PM_PPB_B2_B30x40/* Stop clock when in D3hot (?) */ >> +#define PCI_PM_BPCC_ENABLE 0x80/* Bus power/clock control enable (?) >> */ >> +#define PCI_PM_DATA_REGISTER 7 /* (?) */ >> #define PCI_PM_SIZEOF8 > > We've inherited all of these from Linux, and I notice Linux still has it > this same way. Ideally Linux would change first, but I'd be okay with a > sentence added to the description clarifying that we knowingly accept > the divergence. This file already diverged and we are not in sync with Linux as well. Also there is no functional change being made by this patch so it is ok to change Xen and not Linux in this case (which has quite a lot of trigraphs all over the place). > > With the adjustments: > Reviewed-by: Jan Beulich > > Jan ~Michal
Re: [PATCH] xen: Remove trigraphs from comments
On 06.12.2022 11:59, Michal Orzel wrote: > --- a/xen/include/public/arch-x86_64.h > +++ b/xen/include/public/arch-x86_64.h > @@ -22,5 +22,5 @@ > * A similar callback occurs if the segment selectors are invalid. > * failsafe_address is used as the value of eip. > * > - * On x86_64, event_selector and failsafe_selector are ignored (???). > + * On x86_64, event_selector and failsafe_selector are ignored (?). May I ask that this become (?!?) instead? > --- a/xen/include/xen/pci_regs.h > +++ b/xen/include/xen/pci_regs.h > @@ -246,13 +246,13 @@ > #define PCI_PM_CTRL_STATE_MASK 0x0003 /* Current power state (D0 to > D3) */ > #define PCI_PM_CTRL_NO_SOFT_RESET 0x0008 /* No reset for D3hot->D0 */ > #define PCI_PM_CTRL_PME_ENABLE 0x0100 /* PME pin enable */ > -#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (??) */ > -#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (??) */ > +#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (?) */ > +#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (?) */ > #define PCI_PM_CTRL_PME_STATUS 0x8000 /* PME pin status */ > -#define PCI_PM_PPB_EXTENSIONS6 /* PPB support extensions (??) > */ > -#define PCI_PM_PPB_B2_B30x40/* Stop clock when in D3hot (??) */ > -#define PCI_PM_BPCC_ENABLE 0x80/* Bus power/clock control enable (??) > */ > -#define PCI_PM_DATA_REGISTER 7 /* (??) */ > +#define PCI_PM_PPB_EXTENSIONS6 /* PPB support extensions (?) */ > +#define PCI_PM_PPB_B2_B30x40/* Stop clock when in D3hot (?) */ > +#define PCI_PM_BPCC_ENABLE 0x80/* Bus power/clock control enable (?) */ > +#define PCI_PM_DATA_REGISTER 7 /* (?) */ > #define PCI_PM_SIZEOF8 We've inherited all of these from Linux, and I notice Linux still has it this same way. Ideally Linux would change first, but I'd be okay with a sentence added to the description clarifying that we knowingly accept the divergence. With the adjustments: Reviewed-by: Jan Beulich Jan
Re: [PATCH] xen: Remove trigraphs from comments
> On 6 Dec 2022, at 10:59, Michal Orzel wrote: > > MISRA C rule 4.2 states that trigraphs (sequences of two question marks > followed by a specified third character [=/'()!<>-]) should not be used. > This applies to both code and comments. Thankfully, we do not use them > in the code, but still there are some comments where they are > accidentally used. Fix it. > > Signed-off-by: Michal Orzel Reviewed-by: Luca Fancellu > --- > xen/arch/x86/x86_emulate/x86_emulate.h | 2 +- > xen/include/public/arch-x86_64.h | 2 +- > xen/include/xen/pci_regs.h | 12 ++-- > 3 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h > b/xen/arch/x86/x86_emulate/x86_emulate.h > index 4732855c40ed..bb7af967ffee 100644 > --- a/xen/arch/x86/x86_emulate/x86_emulate.h > +++ b/xen/arch/x86/x86_emulate/x86_emulate.h > @@ -228,7 +228,7 @@ struct x86_emulate_ops > * All functions: > * @ctxt: [IN ] Emulation context info as passed to the emulator. > * All memory-access functions: > - * @seg: [IN ] Segment being dereferenced (specified as x86_seg_??). > + * @seg: [IN ] Segment being dereferenced (specified as x86_seg_?). > * @offset:[IN ] Offset within segment. > * @p_data:[IN ] Pointer to i/o data buffer (length is @bytes) > * Read functions: > diff --git a/xen/include/public/arch-x86_64.h > b/xen/include/public/arch-x86_64.h > index 5db52de69584..1c3567a20217 100644 > --- a/xen/include/public/arch-x86_64.h > +++ b/xen/include/public/arch-x86_64.h > @@ -22,5 +22,5 @@ > * A similar callback occurs if the segment selectors are invalid. > * failsafe_address is used as the value of eip. > * > - * On x86_64, event_selector and failsafe_selector are ignored (???). > + * On x86_64, event_selector and failsafe_selector are ignored (?). > */ > diff --git a/xen/include/xen/pci_regs.h b/xen/include/xen/pci_regs.h > index ee8e82be36b4..a90aff1712ba 100644 > --- a/xen/include/xen/pci_regs.h > +++ b/xen/include/xen/pci_regs.h > @@ -246,13 +246,13 @@ > #define PCI_PM_CTRL_STATE_MASK 0x0003 /* Current power state (D0 to D3) */ > #define PCI_PM_CTRL_NO_SOFT_RESET 0x0008 /* No reset for D3hot->D0 */ > #define PCI_PM_CTRL_PME_ENABLE 0x0100 /* PME pin enable */ > -#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (??) */ > -#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (??) */ > +#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (?) */ > +#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (?) */ > #define PCI_PM_CTRL_PME_STATUS 0x8000 /* PME pin status */ > -#define PCI_PM_PPB_EXTENSIONS 6 /* PPB support extensions (??) */ > -#define PCI_PM_PPB_B2_B3 0x40 /* Stop clock when in D3hot (??) */ > -#define PCI_PM_BPCC_ENABLE 0x80 /* Bus power/clock control enable (??) */ > -#define PCI_PM_DATA_REGISTER 7 /* (??) */ > +#define PCI_PM_PPB_EXTENSIONS 6 /* PPB support extensions (?) */ > +#define PCI_PM_PPB_B2_B3 0x40 /* Stop clock when in D3hot (?) */ > +#define PCI_PM_BPCC_ENABLE 0x80 /* Bus power/clock control enable (?) */ > +#define PCI_PM_DATA_REGISTER 7 /* (?) */ > #define PCI_PM_SIZEOF 8 > > /* AGP registers */ > -- > 2.25.1 > >
[PATCH] xen: Remove trigraphs from comments
MISRA C rule 4.2 states that trigraphs (sequences of two question marks followed by a specified third character [=/'()!<>-]) should not be used. This applies to both code and comments. Thankfully, we do not use them in the code, but still there are some comments where they are accidentally used. Fix it. Signed-off-by: Michal Orzel --- xen/arch/x86/x86_emulate/x86_emulate.h | 2 +- xen/include/public/arch-x86_64.h | 2 +- xen/include/xen/pci_regs.h | 12 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h index 4732855c40ed..bb7af967ffee 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.h +++ b/xen/arch/x86/x86_emulate/x86_emulate.h @@ -228,7 +228,7 @@ struct x86_emulate_ops * All functions: * @ctxt: [IN ] Emulation context info as passed to the emulator. * All memory-access functions: - * @seg: [IN ] Segment being dereferenced (specified as x86_seg_??). + * @seg: [IN ] Segment being dereferenced (specified as x86_seg_?). * @offset:[IN ] Offset within segment. * @p_data:[IN ] Pointer to i/o data buffer (length is @bytes) * Read functions: diff --git a/xen/include/public/arch-x86_64.h b/xen/include/public/arch-x86_64.h index 5db52de69584..1c3567a20217 100644 --- a/xen/include/public/arch-x86_64.h +++ b/xen/include/public/arch-x86_64.h @@ -22,5 +22,5 @@ * A similar callback occurs if the segment selectors are invalid. * failsafe_address is used as the value of eip. * - * On x86_64, event_selector and failsafe_selector are ignored (???). + * On x86_64, event_selector and failsafe_selector are ignored (?). */ diff --git a/xen/include/xen/pci_regs.h b/xen/include/xen/pci_regs.h index ee8e82be36b4..a90aff1712ba 100644 --- a/xen/include/xen/pci_regs.h +++ b/xen/include/xen/pci_regs.h @@ -246,13 +246,13 @@ #define PCI_PM_CTRL_STATE_MASK0x0003 /* Current power state (D0 to D3) */ #define PCI_PM_CTRL_NO_SOFT_RESET 0x0008 /* No reset for D3hot->D0 */ #define PCI_PM_CTRL_PME_ENABLE0x0100 /* PME pin enable */ -#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (??) */ -#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (??) */ +#define PCI_PM_CTRL_DATA_SEL_MASK 0x1e00 /* Data select (?) */ +#define PCI_PM_CTRL_DATA_SCALE_MASK 0x6000 /* Data scale (?) */ #define PCI_PM_CTRL_PME_STATUS0x8000 /* PME pin status */ -#define PCI_PM_PPB_EXTENSIONS 6 /* PPB support extensions (??) */ -#define PCI_PM_PPB_B2_B3 0x40/* Stop clock when in D3hot (??) */ -#define PCI_PM_BPCC_ENABLE0x80/* Bus power/clock control enable (??) */ -#define PCI_PM_DATA_REGISTER 7 /* (??) */ +#define PCI_PM_PPB_EXTENSIONS 6 /* PPB support extensions (?) */ +#define PCI_PM_PPB_B2_B3 0x40/* Stop clock when in D3hot (?) */ +#define PCI_PM_BPCC_ENABLE0x80/* Bus power/clock control enable (?) */ +#define PCI_PM_DATA_REGISTER 7 /* (?) */ #define PCI_PM_SIZEOF 8 /* AGP registers */ -- 2.25.1
Re: [PATCH] xen: remove stray preempt_disable() from PV AP startup code
On 8/25/21 7:31 AM, Juergen Gross wrote: > In cpu_bringup() there is a call of preempt_disable() without a paired > preempt_enable(). This is not needed as interrupts are off initially. > Additionally this will result in early boot messages like: > > BUG: scheduling while atomic: swapper/1/0/0x0002 > > Signed-off-by: Juergen Gross Reviewed-by: Boris Ostrovsky
[PATCH] xen: remove stray preempt_disable() from PV AP startup code
In cpu_bringup() there is a call of preempt_disable() without a paired preempt_enable(). This is not needed as interrupts are off initially. Additionally this will result in early boot messages like: BUG: scheduling while atomic: swapper/1/0/0x0002 Signed-off-by: Juergen Gross --- arch/x86/xen/smp_pv.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c index c2ac319f11a4..96afadf9878e 100644 --- a/arch/x86/xen/smp_pv.c +++ b/arch/x86/xen/smp_pv.c @@ -64,7 +64,6 @@ static void cpu_bringup(void) cr4_init(); cpu_init(); touch_softlockup_watchdog(); - preempt_disable(); /* PVH runs in ring 0 and allows us to do native syscalls. Yay! */ if (!xen_feature(XENFEAT_supervisor_mode_kernel)) { -- 2.26.2
Re: [PATCH] xen: Remove support for PV ACPI cpu/memory hotplug
On 13.04.21 19:52, Boris Ostrovsky wrote: Commit 76fc253723ad ("xen/acpi-stub: Disable it b/c the acpi_processor_add is no longer called.") declared as BROKEN support for Xen ACPI stub (which is required for xen-acpi-{cpu|memory}-hotplug) and suggested that this is temporary and will be soon fixed. This was in March 2013. Further, commit cfafae940381 ("xen: rename dom0_op to platform_op") renamed an interface used by memory hotplug code without updating that code (as it was BROKEN and therefore not compiled). This was in November 2015 and has gone unnoticed for over 5 year. It is now clear that this code is of no interest to anyone and therefore should be removed. Signed-off-by: Boris Ostrovsky Pushed to xen/tip.git for-linus-5.13 Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: application/pgp-keys OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH] xen: Remove support for PV ACPI cpu/memory hotplug
On Tue, Apr 13, 2021 at 7:53 PM Boris Ostrovsky wrote: > > Commit 76fc253723ad ("xen/acpi-stub: Disable it b/c the acpi_processor_add > is no longer called.") declared as BROKEN support for Xen ACPI stub (which > is required for xen-acpi-{cpu|memory}-hotplug) and suggested that this > is temporary and will be soon fixed. This was in March 2013. > > Further, commit cfafae940381 ("xen: rename dom0_op to platform_op") > renamed an interface used by memory hotplug code without updating that > code (as it was BROKEN and therefore not compiled). This was > in November 2015 and has gone unnoticed for over 5 year. > > It is now clear that this code is of no interest to anyone and therefore > should be removed. > > Signed-off-by: Boris Ostrovsky Acked-by: Rafael J. Wysocki Thanks for doing this! > --- > drivers/xen/Kconfig | 31 --- > drivers/xen/Makefile | 3 - > drivers/xen/pcpu.c| 35 --- > drivers/xen/xen-acpi-cpuhotplug.c | 446 --- > drivers/xen/xen-acpi-memhotplug.c | 475 > -- > drivers/xen/xen-stub.c| 90 > include/xen/acpi.h| 35 --- > 7 files changed, 1115 deletions(-) > delete mode 100644 drivers/xen/xen-acpi-cpuhotplug.c > delete mode 100644 drivers/xen/xen-acpi-memhotplug.c > delete mode 100644 drivers/xen/xen-stub.c > > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig > index ea0efd290c37..5f1ce59b44b9 100644 > --- a/drivers/xen/Kconfig > +++ b/drivers/xen/Kconfig > @@ -238,37 +238,6 @@ config XEN_PRIVCMD > depends on XEN > default m > > -config XEN_STUB > - bool "Xen stub drivers" > - depends on XEN && X86_64 && BROKEN > - help > - Allow kernel to install stub drivers, to reserve space for Xen > drivers, > - i.e. memory hotplug and cpu hotplug, and to block native drivers > loaded, > - so that real Xen drivers can be modular. > - > - To enable Xen features like cpu and memory hotplug, select Y here. > - > -config XEN_ACPI_HOTPLUG_MEMORY > - tristate "Xen ACPI memory hotplug" > - depends on XEN_DOM0 && XEN_STUB && ACPI > - help > - This is Xen ACPI memory hotplug. > - > - Currently Xen only support ACPI memory hot-add. If you want > - to hot-add memory at runtime (the hot-added memory cannot be > - removed until machine stop), select Y/M here, otherwise select N. > - > -config XEN_ACPI_HOTPLUG_CPU > - tristate "Xen ACPI cpu hotplug" > - depends on XEN_DOM0 && XEN_STUB && ACPI > - select ACPI_CONTAINER > - help > - Xen ACPI cpu enumerating and hotplugging > - > - For hotplugging, currently Xen only support ACPI cpu hotadd. > - If you want to hotadd cpu at runtime (the hotadded cpu cannot > - be removed until machine stop), select Y/M here. > - > config XEN_ACPI_PROCESSOR > tristate "Xen ACPI processor" > depends on XEN && XEN_DOM0 && X86 && ACPI_PROCESSOR && CPU_FREQ > diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile > index c3621b9f4012..3434593455b2 100644 > --- a/drivers/xen/Makefile > +++ b/drivers/xen/Makefile > @@ -26,9 +26,6 @@ obj-$(CONFIG_SWIOTLB_XEN) += swiotlb-xen.o > obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o > obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/ > obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o > -obj-$(CONFIG_XEN_STUB) += xen-stub.o > -obj-$(CONFIG_XEN_ACPI_HOTPLUG_MEMORY) += xen-acpi-memhotplug.o > -obj-$(CONFIG_XEN_ACPI_HOTPLUG_CPU) += xen-acpi-cpuhotplug.o > obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o > obj-$(CONFIG_XEN_EFI) += efi.o > obj-$(CONFIG_XEN_SCSI_BACKEND) += xen-scsiback.o > diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c > index cdc6daa7a9f6..1bcdd5227771 100644 > --- a/drivers/xen/pcpu.c > +++ b/drivers/xen/pcpu.c > @@ -345,41 +345,6 @@ static irqreturn_t xen_pcpu_interrupt(int irq, void > *dev_id) > return IRQ_HANDLED; > } > > -/* Sync with Xen hypervisor after cpu hotadded */ > -void xen_pcpu_hotplug_sync(void) > -{ > - schedule_work(_pcpu_work); > -} > -EXPORT_SYMBOL_GPL(xen_pcpu_hotplug_sync); > - > -/* > - * For hypervisor presented cpu, return logic cpu id; > - * For hypervisor non-presented cpu, return -ENODEV. > - */ > -int xen_pcpu_id(uint32_t acpi_id) > -{ > - int cpu_id = 0, max_id = 0; > - struct xen_platform_op op; > - > - op.cmd = XENPF_get_cpuinfo; > - while (cpu_id <= max_id) { > - op.u.pcpu_info.xen_cpuid = cpu_id; > - if (HYPERVISOR_platform_op()) { > - cpu_id++; > - continue; > - } > - > - if (acpi_id == op.u.pcpu_info.acpi_id) > - return cpu_id; > - if (op.u.pcpu_info.max_present > max_id) > -
[PATCH] xen: Remove support for PV ACPI cpu/memory hotplug
Commit 76fc253723ad ("xen/acpi-stub: Disable it b/c the acpi_processor_add is no longer called.") declared as BROKEN support for Xen ACPI stub (which is required for xen-acpi-{cpu|memory}-hotplug) and suggested that this is temporary and will be soon fixed. This was in March 2013. Further, commit cfafae940381 ("xen: rename dom0_op to platform_op") renamed an interface used by memory hotplug code without updating that code (as it was BROKEN and therefore not compiled). This was in November 2015 and has gone unnoticed for over 5 year. It is now clear that this code is of no interest to anyone and therefore should be removed. Signed-off-by: Boris Ostrovsky --- drivers/xen/Kconfig | 31 --- drivers/xen/Makefile | 3 - drivers/xen/pcpu.c| 35 --- drivers/xen/xen-acpi-cpuhotplug.c | 446 --- drivers/xen/xen-acpi-memhotplug.c | 475 -- drivers/xen/xen-stub.c| 90 include/xen/acpi.h| 35 --- 7 files changed, 1115 deletions(-) delete mode 100644 drivers/xen/xen-acpi-cpuhotplug.c delete mode 100644 drivers/xen/xen-acpi-memhotplug.c delete mode 100644 drivers/xen/xen-stub.c diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index ea0efd290c37..5f1ce59b44b9 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -238,37 +238,6 @@ config XEN_PRIVCMD depends on XEN default m -config XEN_STUB - bool "Xen stub drivers" - depends on XEN && X86_64 && BROKEN - help - Allow kernel to install stub drivers, to reserve space for Xen drivers, - i.e. memory hotplug and cpu hotplug, and to block native drivers loaded, - so that real Xen drivers can be modular. - - To enable Xen features like cpu and memory hotplug, select Y here. - -config XEN_ACPI_HOTPLUG_MEMORY - tristate "Xen ACPI memory hotplug" - depends on XEN_DOM0 && XEN_STUB && ACPI - help - This is Xen ACPI memory hotplug. - - Currently Xen only support ACPI memory hot-add. If you want - to hot-add memory at runtime (the hot-added memory cannot be - removed until machine stop), select Y/M here, otherwise select N. - -config XEN_ACPI_HOTPLUG_CPU - tristate "Xen ACPI cpu hotplug" - depends on XEN_DOM0 && XEN_STUB && ACPI - select ACPI_CONTAINER - help - Xen ACPI cpu enumerating and hotplugging - - For hotplugging, currently Xen only support ACPI cpu hotadd. - If you want to hotadd cpu at runtime (the hotadded cpu cannot - be removed until machine stop), select Y/M here. - config XEN_ACPI_PROCESSOR tristate "Xen ACPI processor" depends on XEN && XEN_DOM0 && X86 && ACPI_PROCESSOR && CPU_FREQ diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index c3621b9f4012..3434593455b2 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -26,9 +26,6 @@ obj-$(CONFIG_SWIOTLB_XEN) += swiotlb-xen.o obj-$(CONFIG_XEN_MCE_LOG) += mcelog.o obj-$(CONFIG_XEN_PCIDEV_BACKEND) += xen-pciback/ obj-$(CONFIG_XEN_PRIVCMD) += xen-privcmd.o -obj-$(CONFIG_XEN_STUB) += xen-stub.o -obj-$(CONFIG_XEN_ACPI_HOTPLUG_MEMORY) += xen-acpi-memhotplug.o -obj-$(CONFIG_XEN_ACPI_HOTPLUG_CPU) += xen-acpi-cpuhotplug.o obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o obj-$(CONFIG_XEN_EFI) += efi.o obj-$(CONFIG_XEN_SCSI_BACKEND) += xen-scsiback.o diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c index cdc6daa7a9f6..1bcdd5227771 100644 --- a/drivers/xen/pcpu.c +++ b/drivers/xen/pcpu.c @@ -345,41 +345,6 @@ static irqreturn_t xen_pcpu_interrupt(int irq, void *dev_id) return IRQ_HANDLED; } -/* Sync with Xen hypervisor after cpu hotadded */ -void xen_pcpu_hotplug_sync(void) -{ - schedule_work(_pcpu_work); -} -EXPORT_SYMBOL_GPL(xen_pcpu_hotplug_sync); - -/* - * For hypervisor presented cpu, return logic cpu id; - * For hypervisor non-presented cpu, return -ENODEV. - */ -int xen_pcpu_id(uint32_t acpi_id) -{ - int cpu_id = 0, max_id = 0; - struct xen_platform_op op; - - op.cmd = XENPF_get_cpuinfo; - while (cpu_id <= max_id) { - op.u.pcpu_info.xen_cpuid = cpu_id; - if (HYPERVISOR_platform_op()) { - cpu_id++; - continue; - } - - if (acpi_id == op.u.pcpu_info.acpi_id) - return cpu_id; - if (op.u.pcpu_info.max_present > max_id) - max_id = op.u.pcpu_info.max_present; - cpu_id++; - } - - return -ENODEV; -} -EXPORT_SYMBOL_GPL(xen_pcpu_id); - static int __init xen_pcpu_init(void) { int irq, ret; diff --git a/drivers/xen/xen-acpi-cpuhotplug.c b/drivers/xen/xen-acpi-cpuhotplug.c deleted file mode 100644 index
Re: [PATCH] xen: remove the usage of the P ar option
On 31/12/2020 16:05, Roger Pau Monné wrote: > On Thu, Dec 31, 2020 at 03:46:50PM +, Andrew Cooper wrote: >> On 31/12/2020 09:20, Roger Pau Monné wrote: >>> On Wed, Dec 30, 2020 at 06:32:58PM +, Andrew Cooper wrote: On 30/12/2020 17:34, Roger Pau Monne wrote: > It's not part of the POSIX standard [0] and as such non GNU ar > implementations don't usually have it. > > It's not relevant for the use case here anyway, as the archive file is > recreated every time due to the rm invocation before the ar call. No > file name matching should happen so matching using the full path name > or a relative one should yield the same result. > > This fixes the build on FreeBSD. > > Signed-off-by: Roger Pau Monné Acked-by: Andrew Cooper , although... We really need some kind of BSD build in CI. This kind of breakage shouldn't get into master to begin with. >>> Fully agree. I'm not that familiar with gitlab CI, but since we have >>> our own runners there, could we also launch VMs instead of just using >>> containers? >>> >>> It might be easier to integrate with osstest in the future, since >>> FreeBSD has now switched to git. >>> > [0] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ar.html > --- > I'm unsure whether the r and s options are also needed, since they > seem to only be relevant when updating a library, and Xen build system > always removes the old library prior to any ar call. ... I think r should be dropped, because we're not replacing any files. However, I expect the index file is still relevant, because without it, you've got to perform an O(n) search through the archive to find a file. >>> Right, the description of 's' between opengroup and the Linux man page >>> seems to be slightly different. From the opengroup description it seems >>> like s should be used to force the generation of a symbol table when >>> no changes are made to the archive, but otherwise ar should already >>> generate such symbol table. >> Ok - are you happy for me to commit with dropping the r and s? > So after testing this, I think we need at least the r option, as we > want to add new files to the archive (regardless of whether it exists > or not). That seems to be mandatory on FreeBSD, as calling 'ar c' is > not valid. > > I think s can be dropped, as ar will generate the symbol table by > default unless S is specified. s should just be used to force the > generation of a symbol table when not adding files and the archive has > been stripped AFAICT. > > If so would you mind adding: > > "While there also drop the s option, as ar will already generate a > symbol table by default when creating the archive." > > To the commit message if you end up dropping s? Will do. ~Andrew
Re: [PATCH] xen: remove the usage of the P ar option
On Thu, Dec 31, 2020 at 03:46:50PM +, Andrew Cooper wrote: > On 31/12/2020 09:20, Roger Pau Monné wrote: > > On Wed, Dec 30, 2020 at 06:32:58PM +, Andrew Cooper wrote: > >> On 30/12/2020 17:34, Roger Pau Monne wrote: > >>> It's not part of the POSIX standard [0] and as such non GNU ar > >>> implementations don't usually have it. > >>> > >>> It's not relevant for the use case here anyway, as the archive file is > >>> recreated every time due to the rm invocation before the ar call. No > >>> file name matching should happen so matching using the full path name > >>> or a relative one should yield the same result. > >>> > >>> This fixes the build on FreeBSD. > >>> > >>> Signed-off-by: Roger Pau Monné > >> Acked-by: Andrew Cooper , although... > >> > >> We really need some kind of BSD build in CI. This kind of breakage > >> shouldn't get into master to begin with. > > Fully agree. I'm not that familiar with gitlab CI, but since we have > > our own runners there, could we also launch VMs instead of just using > > containers? > > > > It might be easier to integrate with osstest in the future, since > > FreeBSD has now switched to git. > > > >>> [0] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ar.html > >>> --- > >>> I'm unsure whether the r and s options are also needed, since they > >>> seem to only be relevant when updating a library, and Xen build system > >>> always removes the old library prior to any ar call. > >> ... I think r should be dropped, because we're not replacing any files. > >> However, I expect the index file is still relevant, because without it, > >> you've got to perform an O(n) search through the archive to find a file. > > Right, the description of 's' between opengroup and the Linux man page > > seems to be slightly different. From the opengroup description it seems > > like s should be used to force the generation of a symbol table when > > no changes are made to the archive, but otherwise ar should already > > generate such symbol table. > > Ok - are you happy for me to commit with dropping the r and s? So after testing this, I think we need at least the r option, as we want to add new files to the archive (regardless of whether it exists or not). That seems to be mandatory on FreeBSD, as calling 'ar c' is not valid. I think s can be dropped, as ar will generate the symbol table by default unless S is specified. s should just be used to force the generation of a symbol table when not adding files and the archive has been stripped AFAICT. If so would you mind adding: "While there also drop the s option, as ar will already generate a symbol table by default when creating the archive." To the commit message if you end up dropping s? Thanks, Roger.
Re: [PATCH] xen: remove the usage of the P ar option
On 31/12/2020 09:20, Roger Pau Monné wrote: > On Wed, Dec 30, 2020 at 06:32:58PM +, Andrew Cooper wrote: >> On 30/12/2020 17:34, Roger Pau Monne wrote: >>> It's not part of the POSIX standard [0] and as such non GNU ar >>> implementations don't usually have it. >>> >>> It's not relevant for the use case here anyway, as the archive file is >>> recreated every time due to the rm invocation before the ar call. No >>> file name matching should happen so matching using the full path name >>> or a relative one should yield the same result. >>> >>> This fixes the build on FreeBSD. >>> >>> Signed-off-by: Roger Pau Monné >> Acked-by: Andrew Cooper , although... >> >> We really need some kind of BSD build in CI. This kind of breakage >> shouldn't get into master to begin with. > Fully agree. I'm not that familiar with gitlab CI, but since we have > our own runners there, could we also launch VMs instead of just using > containers? > > It might be easier to integrate with osstest in the future, since > FreeBSD has now switched to git. > >>> [0] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ar.html >>> --- >>> I'm unsure whether the r and s options are also needed, since they >>> seem to only be relevant when updating a library, and Xen build system >>> always removes the old library prior to any ar call. >> ... I think r should be dropped, because we're not replacing any files. >> However, I expect the index file is still relevant, because without it, >> you've got to perform an O(n) search through the archive to find a file. > Right, the description of 's' between opengroup and the Linux man page > seems to be slightly different. From the opengroup description it seems > like s should be used to force the generation of a symbol table when > no changes are made to the archive, but otherwise ar should already > generate such symbol table. Ok - are you happy for me to commit with dropping the r and s? ~Andrew
Re: [PATCH] xen: remove the usage of the P ar option
On Wed, Dec 30, 2020 at 06:32:58PM +, Andrew Cooper wrote: > On 30/12/2020 17:34, Roger Pau Monne wrote: > > It's not part of the POSIX standard [0] and as such non GNU ar > > implementations don't usually have it. > > > > It's not relevant for the use case here anyway, as the archive file is > > recreated every time due to the rm invocation before the ar call. No > > file name matching should happen so matching using the full path name > > or a relative one should yield the same result. > > > > This fixes the build on FreeBSD. > > > > Signed-off-by: Roger Pau Monné > > Acked-by: Andrew Cooper , although... > > We really need some kind of BSD build in CI. This kind of breakage > shouldn't get into master to begin with. Fully agree. I'm not that familiar with gitlab CI, but since we have our own runners there, could we also launch VMs instead of just using containers? It might be easier to integrate with osstest in the future, since FreeBSD has now switched to git. > > > > [0] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ar.html > > --- > > I'm unsure whether the r and s options are also needed, since they > > seem to only be relevant when updating a library, and Xen build system > > always removes the old library prior to any ar call. > > ... I think r should be dropped, because we're not replacing any files. > However, I expect the index file is still relevant, because without it, > you've got to perform an O(n) search through the archive to find a file. Right, the description of 's' between opengroup and the Linux man page seems to be slightly different. From the opengroup description it seems like s should be used to force the generation of a symbol table when no changes are made to the archive, but otherwise ar should already generate such symbol table. Thanks, Roger.
Re: [PATCH] xen: remove the usage of the P ar option
On 30/12/2020 17:34, Roger Pau Monne wrote: > It's not part of the POSIX standard [0] and as such non GNU ar > implementations don't usually have it. > > It's not relevant for the use case here anyway, as the archive file is > recreated every time due to the rm invocation before the ar call. No > file name matching should happen so matching using the full path name > or a relative one should yield the same result. > > This fixes the build on FreeBSD. > > Signed-off-by: Roger Pau Monné Acked-by: Andrew Cooper , although... We really need some kind of BSD build in CI. This kind of breakage shouldn't get into master to begin with. > > [0] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ar.html > --- > I'm unsure whether the r and s options are also needed, since they > seem to only be relevant when updating a library, and Xen build system > always removes the old library prior to any ar call. ... I think r should be dropped, because we're not replacing any files. However, I expect the index file is still relevant, because without it, you've got to perform an O(n) search through the archive to find a file. ~Andrew
[PATCH] xen: remove the usage of the P ar option
It's not part of the POSIX standard [0] and as such non GNU ar implementations don't usually have it. It's not relevant for the use case here anyway, as the archive file is recreated every time due to the rm invocation before the ar call. No file name matching should happen so matching using the full path name or a relative one should yield the same result. This fixes the build on FreeBSD. Signed-off-by: Roger Pau Monné [0] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ar.html --- I'm unsure whether the r and s options are also needed, since they seem to only be relevant when updating a library, and Xen build system always removes the old library prior to any ar call. --- xen/Rules.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/Rules.mk b/xen/Rules.mk index aba6ca2a90..8fcc98 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -71,7 +71,7 @@ cmd_ld = $(LD) $(XEN_LDFLAGS) -r -o $@ $(filter-out %.a,$(real-prereqs)) \ # --- quiet_cmd_ar = AR $@ -cmd_ar = rm -f $@; $(AR) cPrs $@ $(real-prereqs) +cmd_ar = rm -f $@; $(AR) crs $@ $(real-prereqs) # Objcopy # --- -- 2.29.2
Re: [PATCH] xen: remove trailing semicolon in macro definition
On 27.11.20 17:07, t...@redhat.com wrote: From: Tom Rix The macro use will already have a semicolon. Signed-off-by: Tom Rix Applied to: xen/tip.git for-linus-5.11 Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: application/pgp-keys OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH] xen: remove trailing semicolon in macro definition
On 27.11.20 17:07, t...@redhat.com wrote: From: Tom Rix The macro use will already have a semicolon. Signed-off-by: Tom Rix Reviewed-by: Juergen Gross Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: application/pgp-keys OpenPGP_signature Description: OpenPGP digital signature
[PATCH] xen: remove trailing semicolon in macro definition
From: Tom Rix The macro use will already have a semicolon. Signed-off-by: Tom Rix --- arch/x86/include/asm/xen/page.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h index 5941e18edd5a..1a162e559753 100644 --- a/arch/x86/include/asm/xen/page.h +++ b/arch/x86/include/asm/xen/page.h @@ -355,7 +355,7 @@ unsigned long arbitrary_virt_to_mfn(void *vaddr); void make_lowmem_page_readonly(void *vaddr); void make_lowmem_page_readwrite(void *vaddr); -#define xen_remap(cookie, size) ioremap((cookie), (size)); +#define xen_remap(cookie, size) ioremap((cookie), (size)) #define xen_unmap(cookie) iounmap((cookie)) static inline bool xen_arch_need_swiotlb(struct device *dev, -- 2.18.4
Re: [PATCH] xen: remove redundant initialization of variable ret
On 9/18/20 11:17 PM, Jing Xiangfeng wrote: > After commit 9f51c05dc41a ("pvcalls-front: Avoid > get_free_pages(GFP_KERNEL) under spinlock"), the variable ret is being > initialized with '-ENOMEM' that is meaningless. So remove it. > > Signed-off-by: Jing Xiangfeng Applied to for-linus-5.10 -boris
Re: [PATCH] xen: remove redundant initialization of variable ret
On 19.09.20 05:17, Jing Xiangfeng wrote: After commit 9f51c05dc41a ("pvcalls-front: Avoid get_free_pages(GFP_KERNEL) under spinlock"), the variable ret is being initialized with '-ENOMEM' that is meaningless. So remove it. Signed-off-by: Jing Xiangfeng Reviewed-by: Juergen Gross Juergen
[PATCH] xen: remove redundant initialization of variable ret
After commit 9f51c05dc41a ("pvcalls-front: Avoid get_free_pages(GFP_KERNEL) under spinlock"), the variable ret is being initialized with '-ENOMEM' that is meaningless. So remove it. Signed-off-by: Jing Xiangfeng --- drivers/xen/pvcalls-front.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index 72d725a0ab5c..7984645b5956 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -371,7 +371,7 @@ static int alloc_active_ring(struct sock_mapping *map) static int create_active(struct sock_mapping *map, evtchn_port_t *evtchn) { void *bytes; - int ret = -ENOMEM, irq = -1, i; + int ret, irq = -1, i; *evtchn = 0; init_waitqueue_head(>active.inflight_conn_req); -- 2.17.1
[PATCH] xen: remove redundant initialization of variable irq
From: Colin Ian King The variable irq is being initialized with a value that is never read and it is being updated later with a new value. The initialization is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King --- arch/x86/pci/xen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c index e3f1ca316068..9f9aad42ccff 100644 --- a/arch/x86/pci/xen.c +++ b/arch/x86/pci/xen.c @@ -62,7 +62,7 @@ static int xen_pcifront_enable_irq(struct pci_dev *dev) #ifdef CONFIG_ACPI static int xen_register_pirq(u32 gsi, int triggering, bool set_pirq) { - int rc, pirq = -1, irq = -1; + int rc, pirq = -1, irq; struct physdev_map_pirq map_irq; int shareable = 0; char *name; -- 2.27.0.rc0
Re: [Xen-devel] [PATCH] xen: remove empty softirq_init()
On 11/02/2020 12:37, Juergen Gross wrote: > softirq_init() is empty since Sen 4.1. Remove it together with its call > sites. > > Signed-off-by: Juergen Gross Acked-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen: remove empty softirq_init()
softirq_init() is empty since Sen 4.1. Remove it together with its call sites. Signed-off-by: Juergen Gross --- xen/arch/arm/setup.c | 2 -- xen/arch/x86/setup.c | 1 - xen/common/softirq.c | 4 xen/include/xen/softirq.h | 1 - 4 files changed, 8 deletions(-) diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 3c8ae11b73..7968cee47d 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -876,8 +876,6 @@ void __init start_xen(unsigned long boot_phys_offset, gic_init(); -softirq_init(); - tasklet_subsys_init(); if ( xsm_dt_init() != 1 ) diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index e50e1f86b3..3fbaee156d 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1533,7 +1533,6 @@ void __init noreturn __start_xen(unsigned long mbi_p) console_init_ring(); vesa_init(); -softirq_init(); tasklet_subsys_init(); paging_init(); diff --git a/xen/common/softirq.c b/xen/common/softirq.c index 2d66193203..b83ad96d6c 100644 --- a/xen/common/softirq.c +++ b/xen/common/softirq.c @@ -132,10 +132,6 @@ void raise_softirq(unsigned int nr) set_bit(nr, _pending(smp_processor_id())); } -void __init softirq_init(void) -{ -} - /* * Local variables: * mode: C diff --git a/xen/include/xen/softirq.h b/xen/include/xen/softirq.h index d7273b389b..b4724f5c8b 100644 --- a/xen/include/xen/softirq.h +++ b/xen/include/xen/softirq.h @@ -25,7 +25,6 @@ typedef void (*softirq_handler)(void); void do_softirq(void); void open_softirq(int nr, softirq_handler handler); -void softirq_init(void); void cpumask_raise_softirq(const cpumask_t *, unsigned int nr); void cpu_raise_softirq(unsigned int cpu, unsigned int nr); -- 2.16.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove sched-if.h includes from various sources
On 05/09/2019 08:21, Jan Beulich wrote: > On 05.09.2019 09:06, Juergen Gross wrote: >> xen/sched-if.h is included in multiple sources where it isn't directly >> needed. Remove those #include statements. >> >> Suggested-by: Jan Beulich >> Signed-off-by: Juergen Gross > Given the tag in place already I'm not sure this is needed, but just > in case: > Acked-by: Jan Beulich Acked-by: Andrew Cooper Last time I played this game (which was this dev cycle), I found that all includes were still necessary. Clearly something has changed in staging - any idea what? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove sched-if.h includes from various sources
On 05.09.2019 09:06, Juergen Gross wrote: > xen/sched-if.h is included in multiple sources where it isn't directly > needed. Remove those #include statements. > > Suggested-by: Jan Beulich > Signed-off-by: Juergen Gross Given the tag in place already I'm not sure this is needed, but just in case: Acked-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen: remove sched-if.h includes from various sources
xen/sched-if.h is included in multiple sources where it isn't directly needed. Remove those #include statements. Suggested-by: Jan Beulich Signed-off-by: Juergen Gross --- Carved out from patch 7 of my core scheduling series --- xen/arch/x86/acpi/cpu_idle.c | 1 - xen/arch/x86/cpu/mcheck/mce.c | 1 - xen/arch/x86/cpu/mcheck/mctelem.c | 1 - xen/arch/x86/setup.c | 1 - xen/arch/x86/smpboot.c| 1 - 5 files changed, 5 deletions(-) diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c index 8f7b6e9b8c..836f524ef4 100644 --- a/xen/arch/x86/acpi/cpu_idle.c +++ b/xen/arch/x86/acpi/cpu_idle.c @@ -38,7 +38,6 @@ #include #include #include -#include #include #include #include diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c index 28ad7dd659..4b2b6de191 100644 --- a/xen/arch/x86/cpu/mcheck/mce.c +++ b/xen/arch/x86/cpu/mcheck/mce.c @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include diff --git a/xen/arch/x86/cpu/mcheck/mctelem.c b/xen/arch/x86/cpu/mcheck/mctelem.c index 3bb13e5265..012a9b95e5 100644 --- a/xen/arch/x86/cpu/mcheck/mctelem.c +++ b/xen/arch/x86/cpu/mcheck/mctelem.c @@ -18,7 +18,6 @@ #include #include #include -#include #include #include diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index d0b35b0ce2..5a88ef368f 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -3,7 +3,6 @@ #include #include #include -#include #include #include #include diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c index 5c4254ac87..911416c1e1 100644 --- a/xen/arch/x86/smpboot.c +++ b/xen/arch/x86/smpboot.c @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include -- 2.16.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove on-stack cpumask from stop_machine_run()
On 28/05/2019 16:32, Jan Beulich wrote: On 28.05.19 at 15:08, wrote: >> --- a/xen/common/stop_machine.c >> +++ b/xen/common/stop_machine.c >> @@ -69,8 +69,8 @@ static void stopmachine_wait_state(void) >> >> int stop_machine_run(int (*fn)(void *), void *data, unsigned int cpu) >> { >> -cpumask_t allbutself; >> unsigned int i, nr_cpus; >> +unsigned int my_cpu = smp_processor_id(); > > Variables starting with my_ being commonly used in introductory > examples, I'd prefer to avoid such names. Elsewhere we use > "this_cpu", "me", or maybe "this" if plain "cpu" is already taken. Okay. > >> @@ -79,9 +79,7 @@ int stop_machine_run(int (*fn)(void *), void *data, >> unsigned int cpu) >> if ( !get_cpu_maps() ) >> return -EBUSY; >> >> -cpumask_andnot(, _online_map, >> - cpumask_of(smp_processor_id())); >> -nr_cpus = cpumask_weight(); >> +nr_cpus = cpumask_weight(_online_map) - 1; > > Having looked at a lot of CPU offlining code recently, I notice this > isn't strictly correct: You imply cpu_online(my_cpu) to produce > "true". I think at present this will always hold, but I'd prefer if we > could avoid gaining such a dependency. And it doesn't look overly > difficult to avoid it. Yes, I have thought about it. If you like it better I test for the running cpu to be in cpu_online_map. > Also please don't open-code num_online_cpus(). Ah, of course! > >> @@ -100,8 +98,9 @@ int stop_machine_run(int (*fn)(void *), void *data, >> unsigned int cpu) >> >> smp_wmb(); >> >> -for_each_cpu ( i, ) >> -tasklet_schedule_on_cpu(_cpu(stopmachine_tasklet, i), i); >> +for_each_cpu ( i, _online_map ) > > Same here for for_each_online_cpu(). Yes. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove on-stack cpumask from stop_machine_run()
>>> On 28.05.19 at 15:08, wrote: > --- a/xen/common/stop_machine.c > +++ b/xen/common/stop_machine.c > @@ -69,8 +69,8 @@ static void stopmachine_wait_state(void) > > int stop_machine_run(int (*fn)(void *), void *data, unsigned int cpu) > { > -cpumask_t allbutself; > unsigned int i, nr_cpus; > +unsigned int my_cpu = smp_processor_id(); Variables starting with my_ being commonly used in introductory examples, I'd prefer to avoid such names. Elsewhere we use "this_cpu", "me", or maybe "this" if plain "cpu" is already taken. > @@ -79,9 +79,7 @@ int stop_machine_run(int (*fn)(void *), void *data, > unsigned int cpu) > if ( !get_cpu_maps() ) > return -EBUSY; > > -cpumask_andnot(, _online_map, > - cpumask_of(smp_processor_id())); > -nr_cpus = cpumask_weight(); > +nr_cpus = cpumask_weight(_online_map) - 1; Having looked at a lot of CPU offlining code recently, I notice this isn't strictly correct: You imply cpu_online(my_cpu) to produce "true". I think at present this will always hold, but I'd prefer if we could avoid gaining such a dependency. And it doesn't look overly difficult to avoid it. Also please don't open-code num_online_cpus(). > @@ -100,8 +98,9 @@ int stop_machine_run(int (*fn)(void *), void *data, > unsigned int cpu) > > smp_wmb(); > > -for_each_cpu ( i, ) > -tasklet_schedule_on_cpu(_cpu(stopmachine_tasklet, i), i); > +for_each_cpu ( i, _online_map ) Same here for for_each_online_cpu(). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen: remove on-stack cpumask from stop_machine_run()
The "allbutself" cpumask in stop_machine_run() is not needed. Instead of allocating it on the stack it can easily be avoided. Signed-off-by: Juergen Gross --- xen/common/stop_machine.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/xen/common/stop_machine.c b/xen/common/stop_machine.c index ce6f5624c4..ccda2efa3e 100644 --- a/xen/common/stop_machine.c +++ b/xen/common/stop_machine.c @@ -69,8 +69,8 @@ static void stopmachine_wait_state(void) int stop_machine_run(int (*fn)(void *), void *data, unsigned int cpu) { -cpumask_t allbutself; unsigned int i, nr_cpus; +unsigned int my_cpu = smp_processor_id(); int ret; BUG_ON(!local_irq_is_enabled()); @@ -79,9 +79,7 @@ int stop_machine_run(int (*fn)(void *), void *data, unsigned int cpu) if ( !get_cpu_maps() ) return -EBUSY; -cpumask_andnot(, _online_map, - cpumask_of(smp_processor_id())); -nr_cpus = cpumask_weight(); +nr_cpus = cpumask_weight(_online_map) - 1; /* Must not spin here as the holder will expect us to be descheduled. */ if ( !spin_trylock(_lock) ) @@ -100,8 +98,9 @@ int stop_machine_run(int (*fn)(void *), void *data, unsigned int cpu) smp_wmb(); -for_each_cpu ( i, ) -tasklet_schedule_on_cpu(_cpu(stopmachine_tasklet, i), i); +for_each_cpu ( i, _online_map ) +if ( i != my_cpu ) +tasklet_schedule_on_cpu(_cpu(stopmachine_tasklet, i), i); stopmachine_set_state(STOPMACHINE_PREPARE); stopmachine_wait_state(); -- 2.16.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove trailing spaces from public headers
On Wed, Nov 28, 2018 at 02:17:55PM +0100, Juergen Gross wrote: > On 28/11/2018 13:52, Jan Beulich wrote: > On 28.11.18 at 13:32, wrote: > >> Several public header files have trailing spaces in them. This is > >> rather annoying when importing them into other projects as they might > >> be rejected not complying to coding style. > >> > >> Remove the trailing spaces in all headers below xen/include/public/. > >> > >> Signed-off-by: Juergen Gross > >> --- > >> I have omitted tmem.h in order to avoid a conflict with Wei's tmem > >> removal series. > > > > To be honest I'm not convinced removing the public header would > > be an appropriate step to take. > > In case it is kept I can easily send a followup patch to remove the two > trailing spaces in there. There is no need for a follow-up patch. I will handle that myself in my series. I plan to commit your patch rather soon. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove trailing spaces from public headers
On Wed, Nov 28, 2018 at 06:09:25AM -0700, Jan Beulich wrote: > >>> On 28.11.18 at 13:57, wrote: > > On Wed, Nov 28, 2018 at 05:52:57AM -0700, Jan Beulich wrote: > >> >>> On 28.11.18 at 13:32, wrote: > >> > Several public header files have trailing spaces in them. This is > >> > rather annoying when importing them into other projects as they might > >> > be rejected not complying to coding style. > >> > > >> > Remove the trailing spaces in all headers below xen/include/public/. > >> > > >> > Signed-off-by: Juergen Gross > >> > --- > >> > I have omitted tmem.h in order to avoid a conflict with Wei's tmem > >> > removal series. > >> > >> To be honest I'm not convinced removing the public header would > >> be an appropriate step to take. > > > > IIRC someone said tmem is never used, so what's the point of keeping > > tmem.h? Unless I'm misremembering? > > Well, "never used" is a fuzzy term. It is a fact that code exists in > Linux (which is about to be removed as well). We don't know who > else is carrying code built against our public tmem.h, which is > different from that code perhaps also being dead. The original > idea (or the way I've been understanding it all the years) with > the public interface was that we'd never break people building > against them (and on the assumption that they may take our > headers verbatim, rather than - like e.g. Linux - cloning them; > otherwise installing the headers would be an entirely pointless > part of the build process), as long as they adhere to some > simple rules (like suitably defining __XEN_INTERFACE_VERSION__). > > As a result > - tools only parts of the public interface may of course be > deleted, > - general parts ought to remain, but may get framed by a > __XEN_INTERFACE_VERSION__ conditional. > Fair enough. I will keep it around. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove trailing spaces from public headers
On 28/11/2018 13:52, Jan Beulich wrote: On 28.11.18 at 13:32, wrote: >> Several public header files have trailing spaces in them. This is >> rather annoying when importing them into other projects as they might >> be rejected not complying to coding style. >> >> Remove the trailing spaces in all headers below xen/include/public/. >> >> Signed-off-by: Juergen Gross >> --- >> I have omitted tmem.h in order to avoid a conflict with Wei's tmem >> removal series. > > To be honest I'm not convinced removing the public header would > be an appropriate step to take. In case it is kept I can easily send a followup patch to remove the two trailing spaces in there. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove trailing spaces from public headers
>>> On 28.11.18 at 13:57, wrote: > On Wed, Nov 28, 2018 at 05:52:57AM -0700, Jan Beulich wrote: >> >>> On 28.11.18 at 13:32, wrote: >> > Several public header files have trailing spaces in them. This is >> > rather annoying when importing them into other projects as they might >> > be rejected not complying to coding style. >> > >> > Remove the trailing spaces in all headers below xen/include/public/. >> > >> > Signed-off-by: Juergen Gross >> > --- >> > I have omitted tmem.h in order to avoid a conflict with Wei's tmem >> > removal series. >> >> To be honest I'm not convinced removing the public header would >> be an appropriate step to take. > > IIRC someone said tmem is never used, so what's the point of keeping > tmem.h? Unless I'm misremembering? Well, "never used" is a fuzzy term. It is a fact that code exists in Linux (which is about to be removed as well). We don't know who else is carrying code built against our public tmem.h, which is different from that code perhaps also being dead. The original idea (or the way I've been understanding it all the years) with the public interface was that we'd never break people building against them (and on the assumption that they may take our headers verbatim, rather than - like e.g. Linux - cloning them; otherwise installing the headers would be an entirely pointless part of the build process), as long as they adhere to some simple rules (like suitably defining __XEN_INTERFACE_VERSION__). As a result - tools only parts of the public interface may of course be deleted, - general parts ought to remain, but may get framed by a __XEN_INTERFACE_VERSION__ conditional. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove trailing spaces from public headers
On Wed, Nov 28, 2018 at 05:52:57AM -0700, Jan Beulich wrote: > >>> On 28.11.18 at 13:32, wrote: > > Several public header files have trailing spaces in them. This is > > rather annoying when importing them into other projects as they might > > be rejected not complying to coding style. > > > > Remove the trailing spaces in all headers below xen/include/public/. > > > > Signed-off-by: Juergen Gross > > --- > > I have omitted tmem.h in order to avoid a conflict with Wei's tmem > > removal series. > > To be honest I'm not convinced removing the public header would > be an appropriate step to take. IIRC someone said tmem is never used, so what's the point of keeping tmem.h? Unless I'm misremembering? Wei. > > Jan > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove trailing spaces from public headers
>>> On 28.11.18 at 13:32, wrote: > Several public header files have trailing spaces in them. This is > rather annoying when importing them into other projects as they might > be rejected not complying to coding style. > > Remove the trailing spaces in all headers below xen/include/public/. > > Signed-off-by: Juergen Gross > --- > I have omitted tmem.h in order to avoid a conflict with Wei's tmem > removal series. To be honest I'm not convinced removing the public header would be an appropriate step to take. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove trailing spaces from public headers
On 28/11/2018 12:36, Wei Liu wrote: > On Wed, Nov 28, 2018 at 01:32:36PM +0100, Juergen Gross wrote: >> Several public header files have trailing spaces in them. This is >> rather annoying when importing them into other projects as they might >> be rejected not complying to coding style. >> >> Remove the trailing spaces in all headers below xen/include/public/. >> >> Signed-off-by: Juergen Gross > Acked-by: Wei Liu Acked-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove trailing spaces from public headers
On Wed, Nov 28, 2018 at 01:32:36PM +0100, Juergen Gross wrote: > Several public header files have trailing spaces in them. This is > rather annoying when importing them into other projects as they might > be rejected not complying to coding style. > > Remove the trailing spaces in all headers below xen/include/public/. > > Signed-off-by: Juergen Gross Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen: remove trailing spaces from public headers
Several public header files have trailing spaces in them. This is rather annoying when importing them into other projects as they might be rejected not complying to coding style. Remove the trailing spaces in all headers below xen/include/public/. Signed-off-by: Juergen Gross --- I have omitted tmem.h in order to avoid a conflict with Wei's tmem removal series. --- xen/include/public/arch-x86/cpuid.h | 8 xen/include/public/arch-x86/hvm/save.h | 20 +-- xen/include/public/arch-x86/xen-x86_32.h | 4 ++-- xen/include/public/arch-x86/xen-x86_64.h | 4 ++-- xen/include/public/arch-x86/xen.h| 4 ++-- xen/include/public/arch-x86_32.h | 4 ++-- xen/include/public/arch-x86_64.h | 4 ++-- xen/include/public/dom0_ops.h| 4 ++-- xen/include/public/domctl.h | 8 xen/include/public/elfnote.h | 4 ++-- xen/include/public/features.h| 4 ++-- xen/include/public/hvm/hvm_info_table.h | 2 +- xen/include/public/hvm/ioreq.h | 8 xen/include/public/hvm/pvdrivers.h | 4 ++-- xen/include/public/hvm/save.h| 34 xen/include/public/io/console.h | 4 ++-- xen/include/public/io/fsif.h | 6 +++--- xen/include/public/io/protocols.h| 2 +- xen/include/public/io/ring.h | 34 xen/include/public/io/vscsiif.h | 24 +++--- xen/include/public/kexec.h | 14 ++--- xen/include/public/memory.h | 16 +++ xen/include/public/nmi.h | 4 ++-- xen/include/public/physdev.h | 6 +++--- xen/include/public/platform.h| 4 ++-- xen/include/public/sysctl.h | 16 +++ xen/include/public/trace.h | 2 +- xen/include/public/vcpu.h| 14 ++--- xen/include/public/version.h | 4 ++-- xen/include/public/xen-compat.h | 4 ++-- xen/include/public/xen.h | 22 ++--- xen/include/public/xenoprof.h| 4 ++-- 32 files changed, 148 insertions(+), 148 deletions(-) diff --git a/xen/include/public/arch-x86/cpuid.h b/xen/include/public/arch-x86/cpuid.h index 665c4b644d..ce46305bee 100644 --- a/xen/include/public/arch-x86/cpuid.h +++ b/xen/include/public/arch-x86/cpuid.h @@ -1,8 +1,8 @@ /** * arch-x86/cpuid.h - * + * * CPUID interface to Xen. - * + * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to * deal in the Software without restriction, including without limitation the @@ -20,9 +20,9 @@ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER * DEALINGS IN THE SOFTWARE. - * + * * Copyright (c) 2007 Citrix Systems, Inc. - * + * * Authors: *Keir Fraser */ diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h index 80e762c335..40be84ecda 100644 --- a/xen/include/public/arch-x86/hvm/save.h +++ b/xen/include/public/arch-x86/hvm/save.h @@ -1,7 +1,7 @@ -/* +/* * Structure definitions for HVM state that is held by Xen and must * be saved along with the domain's memory and device-model state. - * + * * Copyright (c) 2007 XenSource Ltd. * * Permission is hereby granted, free of charge, to any person obtaining a copy @@ -26,8 +26,8 @@ #ifndef __XEN_PUBLIC_HVM_SAVE_X86_H__ #define __XEN_PUBLIC_HVM_SAVE_X86_H__ -/* - * Save/restore header: general info about the save file. +/* + * Save/restore header: general info about the save file. */ #define HVM_FILE_MAGIC 0x54381286 @@ -85,7 +85,7 @@ struct hvm_hw_cpu { uint64_t dr2; uint64_t dr3; uint64_t dr6; -uint64_t dr7; +uint64_t dr7; uint32_t cs_sel; uint32_t ds_sel; @@ -199,7 +199,7 @@ struct hvm_hw_cpu_compat { uint64_t dr2; uint64_t dr3; uint64_t dr6; -uint64_t dr7; +uint64_t dr7; uint32_t cs_sel; uint32_t ds_sel; @@ -463,7 +463,7 @@ struct hvm_hw_pci_link { DECLARE_HVM_SAVE_TYPE(PCI_LINK, 9, struct hvm_hw_pci_link); -/* +/* * PIT */ @@ -489,9 +489,9 @@ struct hvm_hw_pit { DECLARE_HVM_SAVE_TYPE(PIT, 10, struct hvm_hw_pit); -/* +/* * RTC - */ + */ #define RTC_CMOS_SIZE 14 struct hvm_hw_rtc { @@ -639,7 +639,7 @@ struct hvm_msr { #define CPU_MSR_CODE 20 -/* +/* * Largest type-code in use */ #define HVM_SAVE_CODE_MAX 20 diff --git a/xen/include/public/arch-x86/xen-x86_32.h b/xen/include/public/arch-x86/xen-x86_32.h index aa388b7f32..19d7388633 100644 --- a/xen/include/public/arch-x86/xen-x86_32.h +++ b/xen/include/public/arch-x86/xen-x86_32.h @@ -1,8
Re: [Xen-devel] [PATCH] xen: remove size limit of privcmd-buf mapping interface
On 11/9/18 2:03 AM, Juergen Gross wrote: > Ping? > > Jan's remark regarding de-privileged qemu is no issue as the hypercall > node is being closed by the de-privilege library function. Reviewed-by: Boris Ostrovsky ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove size limit of privcmd-buf mapping interface
Ping? Jan's remark regarding de-privileged qemu is no issue as the hypercall node is being closed by the de-privilege library function. Juergen On 01/11/2018 13:33, Juergen Gross wrote: > Currently the size of hypercall buffers allocated via > /dev/xen/hypercall is limited to a default of 64 memory pages. For live > migration of guests this might be too small as the page dirty bitmask > needs to be sized according to the size of the guest. This means > migrating a 8GB sized guest is already exhausting the default buffer > size for the dirty bitmap. > > There is no sensible way to set a sane limit, so just remove it > completely. The device node's usage is limited to root anyway, so there > is no additional DOS scenario added by allowing unlimited buffers. > > While at it make the error path for the -ENOMEM case a little bit > cleaner by setting n_pages to the number of successfully allocated > pages instead of the target size. > > Fixes: c51b3c639e01f2 ("xen: add new hypercall buffer mapping device") > Cc: #4.18 > Signed-off-by: Juergen Gross > --- > drivers/xen/privcmd-buf.c | 22 -- > 1 file changed, 4 insertions(+), 18 deletions(-) > > diff --git a/drivers/xen/privcmd-buf.c b/drivers/xen/privcmd-buf.c > index df1ed37c3269..de01a6d0059d 100644 > --- a/drivers/xen/privcmd-buf.c > +++ b/drivers/xen/privcmd-buf.c > @@ -21,15 +21,9 @@ > > MODULE_LICENSE("GPL"); > > -static unsigned int limit = 64; > -module_param(limit, uint, 0644); > -MODULE_PARM_DESC(limit, "Maximum number of pages that may be allocated by " > - "the privcmd-buf device per open file"); > - > struct privcmd_buf_private { > struct mutex lock; > struct list_head list; > - unsigned int allocated; > }; > > struct privcmd_buf_vma_private { > @@ -60,13 +54,10 @@ static void privcmd_buf_vmapriv_free(struct > privcmd_buf_vma_private *vma_priv) > { > unsigned int i; > > - vma_priv->file_priv->allocated -= vma_priv->n_pages; > - > list_del(_priv->list); > > for (i = 0; i < vma_priv->n_pages; i++) > - if (vma_priv->pages[i]) > - __free_page(vma_priv->pages[i]); > + __free_page(vma_priv->pages[i]); > > kfree(vma_priv); > } > @@ -146,8 +137,7 @@ static int privcmd_buf_mmap(struct file *file, struct > vm_area_struct *vma) > unsigned int i; > int ret = 0; > > - if (!(vma->vm_flags & VM_SHARED) || count > limit || > - file_priv->allocated + count > limit) > + if (!(vma->vm_flags & VM_SHARED)) > return -EINVAL; > > vma_priv = kzalloc(sizeof(*vma_priv) + count * sizeof(void *), > @@ -155,19 +145,15 @@ static int privcmd_buf_mmap(struct file *file, struct > vm_area_struct *vma) > if (!vma_priv) > return -ENOMEM; > > - vma_priv->n_pages = count; > - count = 0; > - for (i = 0; i < vma_priv->n_pages; i++) { > + for (i = 0; i < count; i++) { > vma_priv->pages[i] = alloc_page(GFP_KERNEL | __GFP_ZERO); > if (!vma_priv->pages[i]) > break; > - count++; > + vma_priv->n_pages++; > } > > mutex_lock(_priv->lock); > > - file_priv->allocated += count; > - > vma_priv->file_priv = file_priv; > vma_priv->users = 1; > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove size limit of privcmd-buf mapping interface
On 02/11/2018 08:26, Jan Beulich wrote: On 01.11.18 at 17:27, wrote: >> On 01/11/2018 16:50, Jan Beulich wrote: >> Juergen Gross 11/01/18 3:23 PM >>> On 01/11/2018 15:18, Jan Beulich wrote: Juergen Gross 11/01/18 1:34 PM >>> >> Currently the size of hypercall buffers allocated via >> /dev/xen/hypercall is limited to a default of 64 memory pages. For live >> migration of guests this might be too small as the page dirty bitmask >> needs to be sized according to the size of the guest. This means >> migrating a 8GB sized guest is already exhausting the default buffer >> size for the dirty bitmap. >> >> There is no sensible way to set a sane limit, so just remove it >> completely. The device node's usage is limited to root anyway, so there >> is no additional DOS scenario added by allowing unlimited buffers. > > But is this setting of permissions what we want long term? What about a > de-privileged qemu, which still needs to be able to issue at least dm-op > hypercalls? Wouldn't that qemu have opened the node while still being privileged? >>> >>> Possibly, but how does this help? As soon as it's unprivileged it must not >>> be able to hog resources anymore. >>> >>> Anyway, with Andrew's reply my point may be irrelevant, but I have to >>> admit I'm not entirely sure. >> >> I guess we want Xen tools to close /dev/xen/hypercall (or more precise: >> don't dup2() it) when qemu is de-privileging itself. This will make it >> very clear that it can't hog memory via mmap(). >> >> When you are fine with that I'll send a Xen patch for this. > > If that doesn't prevent the process from making the hypercalls it > is permitted to do (I have to admit I don't recall if there are any > still needed besides the dmop ones), sure. Turns out that is already done: the restrict_all callback of libxencall will associate /dev/null with the file descriptor of /dev/xen/hypercall. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove size limit of privcmd-buf mapping interface
>>> On 01.11.18 at 17:27, wrote: > On 01/11/2018 16:50, Jan Beulich wrote: > Juergen Gross 11/01/18 3:23 PM >>> >>> On 01/11/2018 15:18, Jan Beulich wrote: >>> Juergen Gross 11/01/18 1:34 PM >>> > Currently the size of hypercall buffers allocated via > /dev/xen/hypercall is limited to a default of 64 memory pages. For live > migration of guests this might be too small as the page dirty bitmask > needs to be sized according to the size of the guest. This means > migrating a 8GB sized guest is already exhausting the default buffer > size for the dirty bitmap. > > There is no sensible way to set a sane limit, so just remove it > completely. The device node's usage is limited to root anyway, so there > is no additional DOS scenario added by allowing unlimited buffers. But is this setting of permissions what we want long term? What about a de-privileged qemu, which still needs to be able to issue at least dm-op hypercalls? >>> >>> Wouldn't that qemu have opened the node while still being privileged? >> >> Possibly, but how does this help? As soon as it's unprivileged it must not >> be able to hog resources anymore. >> >> Anyway, with Andrew's reply my point may be irrelevant, but I have to >> admit I'm not entirely sure. > > I guess we want Xen tools to close /dev/xen/hypercall (or more precise: > don't dup2() it) when qemu is de-privileging itself. This will make it > very clear that it can't hog memory via mmap(). > > When you are fine with that I'll send a Xen patch for this. If that doesn't prevent the process from making the hypercalls it is permitted to do (I have to admit I don't recall if there are any still needed besides the dmop ones), sure. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove size limit of privcmd-buf mapping interface
On 01/11/2018 16:50, Jan Beulich wrote: Juergen Gross 11/01/18 3:23 PM >>> >> On 01/11/2018 15:18, Jan Beulich wrote: >> Juergen Gross 11/01/18 1:34 PM >>> Currently the size of hypercall buffers allocated via /dev/xen/hypercall is limited to a default of 64 memory pages. For live migration of guests this might be too small as the page dirty bitmask needs to be sized according to the size of the guest. This means migrating a 8GB sized guest is already exhausting the default buffer size for the dirty bitmap. There is no sensible way to set a sane limit, so just remove it completely. The device node's usage is limited to root anyway, so there is no additional DOS scenario added by allowing unlimited buffers. >>> >>> But is this setting of permissions what we want long term? What about a >>> de-privileged qemu, which still needs to be able to issue at least dm-op >>> hypercalls? >> >> Wouldn't that qemu have opened the node while still being privileged? > > Possibly, but how does this help? As soon as it's unprivileged it must not > be able to hog resources anymore. > > Anyway, with Andrew's reply my point may be irrelevant, but I have to > admit I'm not entirely sure. I guess we want Xen tools to close /dev/xen/hypercall (or more precise: don't dup2() it) when qemu is de-privileging itself. This will make it very clear that it can't hog memory via mmap(). When you are fine with that I'll send a Xen patch for this. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove size limit of privcmd-buf mapping interface
>>> Juergen Gross 11/01/18 3:23 PM >>> >On 01/11/2018 15:18, Jan Beulich wrote: > Juergen Gross 11/01/18 1:34 PM >>> >>> Currently the size of hypercall buffers allocated via >>> /dev/xen/hypercall is limited to a default of 64 memory pages. For live >>> migration of guests this might be too small as the page dirty bitmask >>> needs to be sized according to the size of the guest. This means >>> migrating a 8GB sized guest is already exhausting the default buffer >>> size for the dirty bitmap. >>> >>> There is no sensible way to set a sane limit, so just remove it >>> completely. The device node's usage is limited to root anyway, so there >>> is no additional DOS scenario added by allowing unlimited buffers. >> >> But is this setting of permissions what we want long term? What about a >> de-privileged qemu, which still needs to be able to issue at least dm-op >> hypercalls? > >Wouldn't that qemu have opened the node while still being privileged? Possibly, but how does this help? As soon as it's unprivileged it must not be able to hog resources anymore. Anyway, with Andrew's reply my point may be irrelevant, but I have to admit I'm not entirely sure. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove size limit of privcmd-buf mapping interface
On 01/11/18 14:18, Jan Beulich wrote: Juergen Gross 11/01/18 1:34 PM >>> >> Currently the size of hypercall buffers allocated via >> /dev/xen/hypercall is limited to a default of 64 memory pages. For live >> migration of guests this might be too small as the page dirty bitmask >> needs to be sized according to the size of the guest. This means >> migrating a 8GB sized guest is already exhausting the default buffer >> size for the dirty bitmap. >> >> There is no sensible way to set a sane limit, so just remove it >> completely. The device node's usage is limited to root anyway, so there >> is no additional DOS scenario added by allowing unlimited buffers. > But is this setting of permissions what we want long term? What about a > de-privileged qemu, which still needs to be able to issue at least dm-op > hypercalls? dm-op very deliberately doesn't have hypercall bounce buffers like this. The kernel is responsible for ensuring that the buffer list passed in the ioctl is safe memory, whether this is bouncing the buffers itself, or simply ensuring that the underlying userspace memory won't disappear across the hypercall. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove size limit of privcmd-buf mapping interface
On 01/11/2018 15:18, Jan Beulich wrote: Juergen Gross 11/01/18 1:34 PM >>> >> Currently the size of hypercall buffers allocated via >> /dev/xen/hypercall is limited to a default of 64 memory pages. For live >> migration of guests this might be too small as the page dirty bitmask >> needs to be sized according to the size of the guest. This means >> migrating a 8GB sized guest is already exhausting the default buffer >> size for the dirty bitmap. >> >> There is no sensible way to set a sane limit, so just remove it >> completely. The device node's usage is limited to root anyway, so there >> is no additional DOS scenario added by allowing unlimited buffers. > > But is this setting of permissions what we want long term? What about a > de-privileged qemu, which still needs to be able to issue at least dm-op > hypercalls? Wouldn't that qemu have opened the node while still being privileged? Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove size limit of privcmd-buf mapping interface
>>> Juergen Gross 11/01/18 1:34 PM >>> >Currently the size of hypercall buffers allocated via >/dev/xen/hypercall is limited to a default of 64 memory pages. For live >migration of guests this might be too small as the page dirty bitmask >needs to be sized according to the size of the guest. This means >migrating a 8GB sized guest is already exhausting the default buffer >size for the dirty bitmap. > >There is no sensible way to set a sane limit, so just remove it >completely. The device node's usage is limited to root anyway, so there >is no additional DOS scenario added by allowing unlimited buffers. But is this setting of permissions what we want long term? What about a de-privileged qemu, which still needs to be able to issue at least dm-op hypercalls? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove redundant 'default n' from Kconfig
On 16/10/2018 16:33, Bartlomiej Zolnierkiewicz wrote: > 'default n' is the default value for any bool or tristate Kconfig > setting so there is no need to write it explicitly. > > Also since commit f467c5640c29 ("kconfig: only write '# CONFIG_FOO > is not set' for visible symbols") the Kconfig behavior is the same > regardless of 'default n' being present or not: > > ... > One side effect of (and the main motivation for) this change is making > the following two definitions behave exactly the same: > > config FOO > bool > > config FOO > bool > default n > > With this change, neither of these will generate a > '# CONFIG_FOO is not set' line (assuming FOO isn't selected/implied). > That might make it clearer to people that a bare 'default n' is > redundant. > ... > > Signed-off-by: Bartlomiej Zolnierkiewicz Pushed to xen.tip for-linus-4.20a Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove redundant 'default n' from Kconfig
On 16/10/2018 16:33, Bartlomiej Zolnierkiewicz wrote: > 'default n' is the default value for any bool or tristate Kconfig > setting so there is no need to write it explicitly. > > Also since commit f467c5640c29 ("kconfig: only write '# CONFIG_FOO > is not set' for visible symbols") the Kconfig behavior is the same > regardless of 'default n' being present or not: > > ... > One side effect of (and the main motivation for) this change is making > the following two definitions behave exactly the same: > > config FOO > bool > > config FOO > bool > default n > > With this change, neither of these will generate a > '# CONFIG_FOO is not set' line (assuming FOO isn't selected/implied). > That might make it clearer to people that a bare 'default n' is > redundant. > ... > > Signed-off-by: Bartlomiej Zolnierkiewicz Reviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen: remove redundant 'default n' from Kconfig
'default n' is the default value for any bool or tristate Kconfig setting so there is no need to write it explicitly. Also since commit f467c5640c29 ("kconfig: only write '# CONFIG_FOO is not set' for visible symbols") the Kconfig behavior is the same regardless of 'default n' being present or not: ... One side effect of (and the main motivation for) this change is making the following two definitions behave exactly the same: config FOO bool config FOO bool default n With this change, neither of these will generate a '# CONFIG_FOO is not set' line (assuming FOO isn't selected/implied). That might make it clearer to people that a bare 'default n' is redundant. ... Signed-off-by: Bartlomiej Zolnierkiewicz --- drivers/xen/Kconfig |8 1 file changed, 8 deletions(-) Index: b/drivers/xen/Kconfig === --- a/drivers/xen/Kconfig 2018-10-09 15:58:51.191123246 +0200 +++ b/drivers/xen/Kconfig 2018-10-16 16:32:13.387726147 +0200 @@ -12,7 +12,6 @@ config XEN_BALLOON config XEN_SELFBALLOONING bool "Dynamically self-balloon kernel memory to target" depends on XEN && XEN_BALLOON && CLEANCACHE && SWAP && XEN_TMEM - default n help Self-ballooning dynamically balloons available kernel memory driven by the current usage of anonymous memory ("committed AS") and @@ -27,7 +26,6 @@ config XEN_SELFBALLOONING config XEN_BALLOON_MEMORY_HOTPLUG bool "Memory hotplug support for Xen balloon driver" - default n depends on XEN_BALLOON && MEMORY_HOTPLUG help Memory hotplug support for Xen balloon driver allows expanding memory @@ -226,7 +224,6 @@ config XEN_PCIDEV_BACKEND config XEN_PVCALLS_FRONTEND tristate "XEN PV Calls frontend driver" depends on INET && XEN - default n select XEN_XENBUS_FRONTEND help Experimental frontend for the Xen PV Calls protocol @@ -237,7 +234,6 @@ config XEN_PVCALLS_FRONTEND config XEN_PVCALLS_BACKEND bool "XEN PV Calls backend driver" depends on INET && XEN && XEN_BACKEND - default n help Experimental backend for the Xen PV Calls protocol (https://xenbits.xen.org/docs/unstable/misc/pvcalls.html). It @@ -263,7 +259,6 @@ config XEN_PRIVCMD config XEN_STUB bool "Xen stub drivers" depends on XEN && X86_64 && BROKEN - default n help Allow kernel to install stub drivers, to reserve space for Xen drivers, i.e. memory hotplug and cpu hotplug, and to block native drivers loaded, @@ -274,7 +269,6 @@ config XEN_STUB config XEN_ACPI_HOTPLUG_MEMORY tristate "Xen ACPI memory hotplug" depends on XEN_DOM0 && XEN_STUB && ACPI - default n help This is Xen ACPI memory hotplug. @@ -286,7 +280,6 @@ config XEN_ACPI_HOTPLUG_CPU tristate "Xen ACPI cpu hotplug" depends on XEN_DOM0 && XEN_STUB && ACPI select ACPI_CONTAINER - default n help Xen ACPI cpu enumerating and hotplugging @@ -315,7 +308,6 @@ config XEN_ACPI_PROCESSOR config XEN_MCE_LOG bool "Xen platform mcelog" depends on XEN_DOM0 && X86_64 && X86_MCE - default n help Allow kernel fetching MCE error from Xen platform and converting it into Linux mcelog format for mcelog tools ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove unnecessary condition check before kfree
On 2018/9/10 17:52, Juergen Gross wrote: > On 08/09/18 16:18, zhong jiang wrote: >> kfree has taken null pointer into account. So just remove the >> condition check before kfree. >> >> Signed-off-by: zhong jiang >> --- >> drivers/xen/xen-acpi-processor.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/xen/xen-acpi-processor.c >> b/drivers/xen/xen-acpi-processor.c >> index fbb9137..7e1d49e 100644 >> --- a/drivers/xen/xen-acpi-processor.c >> +++ b/drivers/xen/xen-acpi-processor.c >> @@ -268,7 +268,7 @@ static int push_pxx_to_hypervisor(struct acpi_processor >> *_pr) >> pr_warn("(_PXX): Hypervisor error (%d) for ACPI CPU%u\n", >> ret, _pr->acpi_id); >> err_free: >> -if (!IS_ERR_OR_NULL(dst_states)) >> +if (!IS_ERR(dst_states)) > This is just a change of the condition, not a removal. > > I don't think change is worth it. > Fine, I just consider the duplication of function. Of course. make sense you have said. Maybe it will more clear to have the judgement. Thanks, zhong jiang > Juergen > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove unnecessary condition check before kfree
On 08/09/18 16:18, zhong jiang wrote: > kfree has taken null pointer into account. So just remove the > condition check before kfree. > > Signed-off-by: zhong jiang > --- > drivers/xen/xen-acpi-processor.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/xen/xen-acpi-processor.c > b/drivers/xen/xen-acpi-processor.c > index fbb9137..7e1d49e 100644 > --- a/drivers/xen/xen-acpi-processor.c > +++ b/drivers/xen/xen-acpi-processor.c > @@ -268,7 +268,7 @@ static int push_pxx_to_hypervisor(struct acpi_processor > *_pr) > pr_warn("(_PXX): Hypervisor error (%d) for ACPI CPU%u\n", > ret, _pr->acpi_id); > err_free: > - if (!IS_ERR_OR_NULL(dst_states)) > + if (!IS_ERR(dst_states)) This is just a change of the condition, not a removal. I don't think change is worth it. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen: remove unnecessary condition check before kfree
kfree has taken null pointer into account. So just remove the condition check before kfree. Signed-off-by: zhong jiang --- drivers/xen/xen-acpi-processor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c index fbb9137..7e1d49e 100644 --- a/drivers/xen/xen-acpi-processor.c +++ b/drivers/xen/xen-acpi-processor.c @@ -268,7 +268,7 @@ static int push_pxx_to_hypervisor(struct acpi_processor *_pr) pr_warn("(_PXX): Hypervisor error (%d) for ACPI CPU%u\n", ret, _pr->acpi_id); err_free: - if (!IS_ERR_OR_NULL(dst_states)) + if (!IS_ERR(dst_states)) kfree(dst_states); return ret; -- 1.7.12.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove redundant put_device
On Thu, Sep 06, 2018 at 02:33:10PM +0800, Ding Xiang wrote: > device_unregister will put device, do not need to do it one more time > > Signed-off-by: Ding Xiang > --- > drivers/xen/xenbus/xenbus_probe.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/xen/xenbus/xenbus_probe.c > b/drivers/xen/xenbus/xenbus_probe.c > index 5b47188..cfaa878 100644 > --- a/drivers/xen/xenbus/xenbus_probe.c > +++ b/drivers/xen/xenbus/xenbus_probe.c > @@ -369,7 +369,6 @@ static void xenbus_cleanup_devices(const char *path, > struct bus_type *bus) > bus_for_each_dev(bus, NULL, , cleanup_dev); > if (info.dev) { > device_unregister(>dev); > - put_device(>dev); This is to match the get_device call in cleanup_dev. It is not redundant IMHO. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen: remove redundant put_device
device_unregister will put device, do not need to do it one more time Signed-off-by: Ding Xiang --- drivers/xen/xenbus/xenbus_probe.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c index 5b47188..cfaa878 100644 --- a/drivers/xen/xenbus/xenbus_probe.c +++ b/drivers/xen/xenbus/xenbus_probe.c @@ -369,7 +369,6 @@ static void xenbus_cleanup_devices(const char *path, struct bus_type *bus) bus_for_each_dev(bus, NULL, , cleanup_dev); if (info.dev) { device_unregister(>dev); - put_device(>dev); } } while (info.dev); } -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove unused hypercall functions
On 08/20/2018 09:03 AM, Juergen Gross wrote: > Remove Xen hypercall functions which are used nowhere in the kernel. > > Signed-off-by: Juergen Gross > --- Applied to for-linus-19b. -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen: remove unused hypercall functions
Remove Xen hypercall functions which are used nowhere in the kernel. Signed-off-by: Juergen Gross --- arch/x86/include/asm/xen/hypercall.h | 118 --- 1 file changed, 118 deletions(-) diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h index 6b2f90a0b149..ef05bea7010d 100644 --- a/arch/x86/include/asm/xen/hypercall.h +++ b/arch/x86/include/asm/xen/hypercall.h @@ -197,17 +197,6 @@ extern struct { char _entry[32]; } hypercall_page[]; (type)__res;\ }) -#define _hypercall5(type, name, a1, a2, a3, a4, a5)\ -({ \ - __HYPERCALL_DECLS; \ - __HYPERCALL_5ARG(a1, a2, a3, a4, a5); \ - asm volatile (__HYPERCALL \ - : __HYPERCALL_5PARAM \ - : __HYPERCALL_ENTRY(name) \ - : __HYPERCALL_CLOBBER5); \ - (type)__res;\ -}) - static inline long xen_single_call(unsigned int call, unsigned long a1, unsigned long a2, @@ -267,47 +256,12 @@ HYPERVISOR_set_gdt(unsigned long *frame_list, int entries) } static inline int -HYPERVISOR_stack_switch(unsigned long ss, unsigned long esp) -{ - return _hypercall2(int, stack_switch, ss, esp); -} - -#ifdef CONFIG_X86_32 -static inline int -HYPERVISOR_set_callbacks(unsigned long event_selector, -unsigned long event_address, -unsigned long failsafe_selector, -unsigned long failsafe_address) -{ - return _hypercall4(int, set_callbacks, - event_selector, event_address, - failsafe_selector, failsafe_address); -} -#else /* CONFIG_X86_64 */ -static inline int -HYPERVISOR_set_callbacks(unsigned long event_address, - unsigned long failsafe_address, - unsigned long syscall_address) -{ - return _hypercall3(int, set_callbacks, - event_address, failsafe_address, - syscall_address); -} -#endif /* CONFIG_X86_{32,64} */ - -static inline int HYPERVISOR_callback_op(int cmd, void *arg) { return _hypercall2(int, callback_op, cmd, arg); } static inline int -HYPERVISOR_fpu_taskswitch(int set) -{ - return _hypercall1(int, fpu_taskswitch, set); -} - -static inline int HYPERVISOR_sched_op(int cmd, void *arg) { return _hypercall2(int, sched_op, cmd, arg); @@ -419,19 +373,6 @@ HYPERVISOR_grant_table_op(unsigned int cmd, void *uop, unsigned int count) } static inline int -HYPERVISOR_update_va_mapping_otherdomain(unsigned long va, pte_t new_val, -unsigned long flags, domid_t domid) -{ - if (sizeof(new_val) == sizeof(long)) - return _hypercall4(int, update_va_mapping_otherdomain, va, - new_val.pte, flags, domid); - else - return _hypercall5(int, update_va_mapping_otherdomain, va, - new_val.pte, new_val.pte >> 32, - flags, domid); -} - -static inline int HYPERVISOR_vm_assist(unsigned int cmd, unsigned int type) { return _hypercall2(int, vm_assist, cmd, type); @@ -465,12 +406,6 @@ HYPERVISOR_suspend(unsigned long start_info_mfn) return _hypercall3(int, sched_op, SCHEDOP_shutdown, , start_info_mfn); } -static inline int -HYPERVISOR_nmi_op(unsigned long op, unsigned long arg) -{ - return _hypercall2(int, nmi_op, op, arg); -} - static inline unsigned long __must_check HYPERVISOR_hvm_op(int op, void *arg) { @@ -529,39 +464,6 @@ MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va, } static inline void -MULTI_grant_table_op(struct multicall_entry *mcl, unsigned int cmd, -void *uop, unsigned int count) -{ - mcl->op = __HYPERVISOR_grant_table_op; - mcl->args[0] = cmd; - mcl->args[1] = (unsigned long)uop; - mcl->args[2] = count; - - trace_xen_mc_entry(mcl, 3); -} - -static inline void -MULTI_update_va_mapping_otherdomain(struct multicall_entry *mcl, unsigned long va, - pte_t new_val, unsigned long flags, - domid_t domid) -{ - mcl->op = __HYPERVISOR_update_va_mapping_otherdomain; - mcl->args[0] = va; - if (sizeof(new_val) == sizeof(long)) { - mcl->args[1] = new_val.pte; - mcl->args[2] = flags; - mcl->args[3] = domid; - } else { - mcl->args[1] = new_val.pte; - mcl->args[2] =
Re: [Xen-devel] [PATCH] xen: Remove domain_crash_synchronous() completely
> -Original Message- > From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf > Of Roger Pau Monné > Sent: 01 August 2018 14:50 > To: Andrew Cooper > Cc: Julien Grall ; Stefano Stabellini > ; Wei Liu ; Jan Beulich > ; Xen-devel > Subject: Re: [Xen-devel] [PATCH] xen: Remove > domain_crash_synchronous() completely > > On Wed, Aug 01, 2018 at 02:29:35PM +0100, Andrew Cooper wrote: > > domain_crash_synchronous() is unsafe to use in general as it may leave > > spinlocks held, temporary memory allocated, etc. > > > > With domain_crash_synchronous() removed from the ARM code in 4.11, > take the > > opportunity to remove the infrastructure completely by opencoding the > softirq > > loop in the remaining callsites, all of which are destined for deletion. > > > > None of these sites are at risk of having a pending ioreq to qemu, which > means > > that the vcpu_end_shutdown_deferral() isn't necessary. > > > > Signed-off-by: Andrew Cooper > > Reviewed-by: Roger Pau Monné > > This however removes the printk with the file an line number from > where the domain_crash was called. I don't think it's a big issue > because each call site already has a message. There are many places which call domain_crash() without any other logging so such crashes will certainly get harder to diagnose. Paul > > Roger. > > ___ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: Remove domain_crash_synchronous() completely
On 01/08/18 14:50, Roger Pau Monné wrote: > On Wed, Aug 01, 2018 at 02:29:35PM +0100, Andrew Cooper wrote: >> domain_crash_synchronous() is unsafe to use in general as it may leave >> spinlocks held, temporary memory allocated, etc. >> >> With domain_crash_synchronous() removed from the ARM code in 4.11, take the >> opportunity to remove the infrastructure completely by opencoding the softirq >> loop in the remaining callsites, all of which are destined for deletion. >> >> None of these sites are at risk of having a pending ioreq to qemu, which >> means >> that the vcpu_end_shutdown_deferral() isn't necessary. >> >> Signed-off-by: Andrew Cooper > Reviewed-by: Roger Pau Monné > > This however removes the printk with the file an line number from > where the domain_crash was called. I don't think it's a big issue > because each call site already has a message. Removing the line number was the basis of this work originally. It is a problem for livepatching, as it causes excessively large binary deltas. A followup which I've yet to refresh from its previous posting changes domain_crash() to take a string to force all users to provide an intelligent error, and swaps __LINE__ for __file__ ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: Remove domain_crash_synchronous() completely
On Wed, Aug 01, 2018 at 02:29:35PM +0100, Andrew Cooper wrote: > domain_crash_synchronous() is unsafe to use in general as it may leave > spinlocks held, temporary memory allocated, etc. > > With domain_crash_synchronous() removed from the ARM code in 4.11, take the > opportunity to remove the infrastructure completely by opencoding the softirq > loop in the remaining callsites, all of which are destined for deletion. > > None of these sites are at risk of having a pending ioreq to qemu, which means > that the vcpu_end_shutdown_deferral() isn't necessary. > > Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné This however removes the printk with the file an line number from where the domain_crash was called. I don't think it's a big issue because each call site already has a message. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: Remove domain_crash_synchronous() completely
>>> On 01.08.18 at 15:29, wrote: > domain_crash_synchronous() is unsafe to use in general as it may leave > spinlocks held, temporary memory allocated, etc. > > With domain_crash_synchronous() removed from the ARM code in 4.11, take the > opportunity to remove the infrastructure completely by opencoding the softirq > loop in the remaining callsites, all of which are destined for deletion. > > None of these sites are at risk of having a pending ioreq to qemu, which means > that the vcpu_end_shutdown_deferral() isn't necessary. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen: Remove domain_crash_synchronous() completely
domain_crash_synchronous() is unsafe to use in general as it may leave spinlocks held, temporary memory allocated, etc. With domain_crash_synchronous() removed from the ARM code in 4.11, take the opportunity to remove the infrastructure completely by opencoding the softirq loop in the remaining callsites, all of which are destined for deletion. None of these sites are at risk of having a pending ioreq to qemu, which means that the vcpu_end_shutdown_deferral() isn't necessary. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Stefano Stabellini CC: Julien Grall CC: Wei Liu CC: Roger Pau Monné asm_domain_crash_synchronous() will be removed by my "lift exception frame logic up into C" series, while the waitqueue callers will be removed when the vm_event ring mapping infrastructure is complete. --- xen/arch/x86/traps.c| 5 - xen/common/domain.c | 11 --- xen/common/wait.c | 13 ++--- xen/include/xen/sched.h | 10 -- 4 files changed, 14 insertions(+), 25 deletions(-) diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 789d7ff..ddff346 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -2215,7 +2215,10 @@ void asm_domain_crash_synchronous(unsigned long addr) printk("domain_crash_sync called from entry.S: fault at %p %pS\n", _p(addr), _p(addr)); -__domain_crash_synchronous(); +__domain_crash(current->domain); + +for ( ; ; ) +do_softirq(); } /* diff --git a/xen/common/domain.c b/xen/common/domain.c index 08ca4b1..749722b 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -700,17 +700,6 @@ void __domain_crash(struct domain *d) } -void __domain_crash_synchronous(void) -{ -__domain_crash(current->domain); - -vcpu_end_shutdown_deferral(current); - -for ( ; ; ) -do_softirq(); -} - - int domain_shutdown(struct domain *d, u8 reason) { struct vcpu *v; diff --git a/xen/common/wait.c b/xen/common/wait.c index a57bc10..4f830a1 100644 --- a/xen/common/wait.c +++ b/xen/common/wait.c @@ -20,6 +20,7 @@ */ #include +#include #include #include @@ -135,7 +136,10 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv) if ( vcpu_set_hard_affinity(curr, cpumask_of(wqv->wakeup_cpu)) ) { gdprintk(XENLOG_ERR, "Unable to set vcpu affinity\n"); -domain_crash_synchronous(); +domain_crash(current->domain); + +for ( ; ; ) +do_softirq(); } /* Hand-rolled setjmp(). */ @@ -166,7 +170,10 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv) if ( unlikely(wqv->esp == 0) ) { gdprintk(XENLOG_ERR, "Stack too large in %s\n", __func__); -domain_crash_synchronous(); +domain_crash(current->domain); + +for ( ; ; ) +do_softirq(); } cpu_info->guest_cpu_user_regs.entry_vector = entry_vector; @@ -196,7 +203,7 @@ void check_wakeup_from_wait(void) if ( vcpu_set_hard_affinity(curr, cpumask_of(wqv->wakeup_cpu)) ) { gdprintk(XENLOG_ERR, "Unable to set vcpu affinity\n"); -domain_crash_synchronous(); +domain_crash(current->domain); } wait(); /* takes us back into the scheduler */ } diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 851f11e..3c35473 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -617,16 +617,6 @@ void __domain_crash(struct domain *d); } while (0) /* - * Mark current domain as crashed and synchronously deschedule from the local - * processor. This function never returns. - */ -void noreturn __domain_crash_synchronous(void); -#define domain_crash_synchronous() do { \ -printk("domain_crash_sync called from %s:%d\n", __FILE__, __LINE__); \ -__domain_crash_synchronous(); \ -} while (0) - -/* * Called from assembly code, with an optional address to help indicate why * the crash occured. If addr is 0, look up address from last extable * redirection. -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: remove global bit from __default_kernel_pte_mask for pv guests
On 07/02/2018 06:00 AM, Juergen Gross wrote: > When removing the global bit from __supported_pte_mask do the same for > __default_kernel_pte_mask in order to avoid the WARN_ONCE() in > check_pgprot() when setting a kernel pte before having called > init_mem_mapping(). > > Cc: # 4.17 > Reported-by: Michael Young > Signed-off-by: Juergen Gross Reviewed-by: Boris Ostrovsky ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen: remove global bit from __default_kernel_pte_mask for pv guests
When removing the global bit from __supported_pte_mask do the same for __default_kernel_pte_mask in order to avoid the WARN_ONCE() in check_pgprot() when setting a kernel pte before having called init_mem_mapping(). Cc: # 4.17 Reported-by: Michael Young Signed-off-by: Juergen Gross --- arch/x86/xen/enlighten_pv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c index 0f4cd9e5bed4..cf7b13d3e911 100644 --- a/arch/x86/xen/enlighten_pv.c +++ b/arch/x86/xen/enlighten_pv.c @@ -1230,6 +1230,7 @@ asmlinkage __visible void __init xen_start_kernel(void) /* Prevent unwanted bits from being set in PTEs. */ __supported_pte_mask &= ~_PAGE_GLOBAL; + __default_kernel_pte_mask &= ~_PAGE_GLOBAL; /* * Prevent page tables from being allocated in highmem, even -- 2.13.7 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: Remove unnecessary BUG_ON from __unbind_from_irq()
On Thu, Jun 21, 2018 at 01:29:44PM -0400, Boris Ostrovsky wrote: > Commit 910f8befdf5b ("xen/pirq: fix error path cleanup when binding > MSIs") fixed a couple of errors in error cleanup path of > xen_bind_pirq_msi_to_irq(). This cleanup allowed a call to > __unbind_from_irq() with an unbound irq, which would result in > triggering the BUG_ON there. > > Since there is really no reason for the BUG_ON (xen_free_irq() can > operate on unbound irqs) we can remove it. > > Reported-by: Ben Hutchings > Signed-off-by: Boris Ostrovsky Reviewed-by: Roger Pau Monné Thanks, I had this on my queue of TODOs. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: Remove unnecessary BUG_ON from __unbind_from_irq()
On 21/06/18 19:29, Boris Ostrovsky wrote: > Commit 910f8befdf5b ("xen/pirq: fix error path cleanup when binding > MSIs") fixed a couple of errors in error cleanup path of > xen_bind_pirq_msi_to_irq(). This cleanup allowed a call to > __unbind_from_irq() with an unbound irq, which would result in > triggering the BUG_ON there. > > Since there is really no reason for the BUG_ON (xen_free_irq() can > operate on unbound irqs) we can remove it. > > Reported-by: Ben Hutchings > Signed-off-by: Boris Ostrovsky > Cc: sta...@vger.kernel.org Pushed to xen/tip.git for-linus-4.18 Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: Remove unnecessary BUG_ON from __unbind_from_irq()
On 21/06/18 19:29, Boris Ostrovsky wrote: > Commit 910f8befdf5b ("xen/pirq: fix error path cleanup when binding > MSIs") fixed a couple of errors in error cleanup path of > xen_bind_pirq_msi_to_irq(). This cleanup allowed a call to > __unbind_from_irq() with an unbound irq, which would result in > triggering the BUG_ON there. > > Since there is really no reason for the BUG_ON (xen_free_irq() can > operate on unbound irqs) we can remove it. > > Reported-by: Ben Hutchings > Signed-off-by: Boris Ostrovsky > Cc: sta...@vger.kernel.org Reviewed-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen: Remove unnecessary BUG_ON from __unbind_from_irq()
Commit 910f8befdf5b ("xen/pirq: fix error path cleanup when binding MSIs") fixed a couple of errors in error cleanup path of xen_bind_pirq_msi_to_irq(). This cleanup allowed a call to __unbind_from_irq() with an unbound irq, which would result in triggering the BUG_ON there. Since there is really no reason for the BUG_ON (xen_free_irq() can operate on unbound irqs) we can remove it. Reported-by: Ben Hutchings Signed-off-by: Boris Ostrovsky Cc: sta...@vger.kernel.org --- drivers/xen/events/events_base.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c index 762378f..08e4af0 100644 --- a/drivers/xen/events/events_base.c +++ b/drivers/xen/events/events_base.c @@ -628,8 +628,6 @@ static void __unbind_from_irq(unsigned int irq) xen_irq_info_cleanup(info); } - BUG_ON(info_for_irq(irq)->type == IRQT_UNBOUND); - xen_free_irq(irq); } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel