Re: [PATCH] xen: remove

2023-11-10 Thread Oleksii
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

2023-10-31 Thread Julien Grall




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

2023-10-31 Thread Jan Beulich
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

2023-10-31 Thread Oleksii Kurochko
 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

2023-03-16 Thread Juergen Gross

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

2023-03-16 Thread Yu Zhe
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

2023-02-16 Thread Jan Beulich
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

2023-02-16 Thread Andrew Cooper
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

2023-02-16 Thread Andrew Cooper
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

2023-02-16 Thread Andrew Cooper
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

2023-01-11 Thread Jan Beulich
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

2023-01-11 Thread Luca Fancellu


> 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

2023-01-11 Thread Alistair Francis
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

2023-01-11 Thread Andrew Cooper
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

2023-01-11 Thread Julien Grall
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

2022-12-06 Thread Jan Beulich
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

2022-12-06 Thread Michal Orzel



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

2022-12-06 Thread Jan Beulich
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

2022-12-06 Thread Michal Orzel
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

2022-12-06 Thread Jan Beulich
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

2022-12-06 Thread Luca Fancellu



> 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

2022-12-06 Thread Michal Orzel
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

2021-09-08 Thread Boris Ostrovsky


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

2021-08-25 Thread Juergen Gross
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

2021-04-23 Thread Juergen Gross

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

2021-04-13 Thread Rafael J. Wysocki
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

2021-04-13 Thread Boris Ostrovsky
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

2020-12-31 Thread Andrew Cooper
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

2020-12-31 Thread Roger Pau Monné
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

2020-12-31 Thread Andrew Cooper
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

2020-12-31 Thread Roger Pau Monné
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

2020-12-30 Thread Andrew Cooper
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

2020-12-30 Thread Roger Pau Monne
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

2020-12-15 Thread Jürgen Groß

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

2020-12-02 Thread Jürgen Groß

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

2020-11-27 Thread trix
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

2020-09-30 Thread boris . ostrovsky


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

2020-09-22 Thread Jürgen Groß

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

2020-09-18 Thread Jing Xiangfeng
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

2020-06-11 Thread Colin King
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()

2020-02-11 Thread Andrew Cooper
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()

2020-02-11 Thread Juergen Gross
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

2019-09-05 Thread Andrew Cooper
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

2019-09-05 Thread Jan Beulich
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

2019-09-05 Thread Juergen Gross
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()

2019-05-28 Thread Juergen Gross
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()

2019-05-28 Thread Jan Beulich
>>> 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()

2019-05-28 Thread Juergen Gross
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

2018-11-28 Thread Wei Liu
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

2018-11-28 Thread Wei Liu
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

2018-11-28 Thread Juergen Gross
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

2018-11-28 Thread Jan Beulich
>>> 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

2018-11-28 Thread Wei Liu
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

2018-11-28 Thread Jan Beulich
>>> 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

2018-11-28 Thread Andrew Cooper
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

2018-11-28 Thread Wei Liu
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

2018-11-28 Thread Juergen Gross
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

2018-11-09 Thread Boris Ostrovsky
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

2018-11-08 Thread Juergen Gross
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

2018-11-02 Thread Juergen Gross
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

2018-11-02 Thread Jan Beulich
>>> 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

2018-11-01 Thread Juergen Gross
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

2018-11-01 Thread Jan Beulich
>>> 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

2018-11-01 Thread Andrew Cooper
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

2018-11-01 Thread Juergen Gross
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

2018-11-01 Thread Jan Beulich
>>> 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

2018-10-24 Thread Juergen Gross
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

2018-10-17 Thread Juergen Gross
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

2018-10-16 Thread Bartlomiej Zolnierkiewicz
'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

2018-09-10 Thread zhong jiang
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

2018-09-10 Thread Juergen Gross
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

2018-09-09 Thread zhong jiang
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

2018-09-06 Thread Wei Liu
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

2018-09-06 Thread Ding Xiang
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

2018-08-20 Thread Boris Ostrovsky
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

2018-08-20 Thread Juergen Gross
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

2018-08-01 Thread Paul Durrant
> -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

2018-08-01 Thread Andrew Cooper
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

2018-08-01 Thread Roger Pau Monné
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

2018-08-01 Thread Jan Beulich
>>> 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

2018-08-01 Thread Andrew Cooper
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

2018-07-10 Thread Boris Ostrovsky
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

2018-07-02 Thread Juergen Gross
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()

2018-06-22 Thread Roger Pau Monné
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()

2018-06-22 Thread Juergen Gross
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()

2018-06-22 Thread Juergen Gross
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()

2018-06-21 Thread Boris Ostrovsky
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