xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tools: Move MASK_INSR to common-macros.h
@ 2023-06-09 10:11 Andrew Cooper
  2023-06-09 10:13 ` [PATCH v2] " Andrew Cooper
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Andrew Cooper @ 2023-06-09 10:11 UTC (permalink / raw)
  To: Xen-devel
  Cc: Andrew Cooper, Anthony PERARD, Juergen Gross, Luca Fancellu,
	Jan Beulich, Roger Pau Monné,
	Wei Liu

MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.

Drop the pair from x86-emulate.h which includes common-macros.h.

Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@citrix.com>
CC: Juergen Gross <jgross@suse.com>
CC: Luca Fancellu <luca.fancellu@arm.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>

v2:
 * Drop from x86-emulate.h too.
---
 tools/include/xen-tools/common-macros.h | 1 +
 tools/libs/light/libxl_internal.h       | 2 --
 tools/tests/x86_emulator/x86-emulate.h  | 3 ---
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h
index d53b88182560..168691be0e93 100644
--- a/tools/include/xen-tools/common-macros.h
+++ b/tools/include/xen-tools/common-macros.h
@@ -73,6 +73,7 @@
 #endif
 
 #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
+#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
 
 #ifndef __must_check
 #define __must_check __attribute__((__warn_unused_result__))
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index 8aba3e138909..61f4fe1dec55 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -132,8 +132,6 @@
 
 #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
 
-#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
-
 #define LIBXL__LOGGING_ENABLED
 
 #ifdef LIBXL__LOGGING_ENABLED
diff --git a/tools/tests/x86_emulator/x86-emulate.h b/tools/tests/x86_emulator/x86-emulate.h
index de1f82668010..aa1ed75ec890 100644
--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -51,9 +51,6 @@
 #define DEFINE_PER_CPU(type, var) type per_cpu_##var
 #define this_cpu(var) per_cpu_##var
 
-#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
-#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
-
 #define __init
 #define __maybe_unused __attribute__((__unused__))
 
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] tools: Move MASK_INSR to common-macros.h
  2023-06-09 10:11 [PATCH] tools: Move MASK_INSR to common-macros.h Andrew Cooper
@ 2023-06-09 10:13 ` Andrew Cooper
  2023-06-09 10:22 ` [PATCH] " Luca Fancellu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2023-06-09 10:13 UTC (permalink / raw)
  To: Xen-devel
  Cc: Anthony PERARD, Juergen Gross, Luca Fancellu, Jan Beulich,
	Roger Pau Monné,
	Wei Liu

Urgh, well I'm failing at email today.  This is v2.  Everything else in
the patch is as intended.

~Andrew

On 09/06/2023 11:11 am, Andrew Cooper wrote:
> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>
> Drop the pair from x86-emulate.h which includes common-macros.h.
>
> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Anthony PERARD <anthony.perard@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Luca Fancellu <luca.fancellu@arm.com>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
>
> v2:
>  * Drop from x86-emulate.h too.
> ---
>  tools/include/xen-tools/common-macros.h | 1 +
>  tools/libs/light/libxl_internal.h       | 2 --
>  tools/tests/x86_emulator/x86-emulate.h  | 3 ---
>  3 files changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h
> index d53b88182560..168691be0e93 100644
> --- a/tools/include/xen-tools/common-macros.h
> +++ b/tools/include/xen-tools/common-macros.h
> @@ -73,6 +73,7 @@
>  #endif
>  
>  #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> +#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
>  
>  #ifndef __must_check
>  #define __must_check __attribute__((__warn_unused_result__))
> diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
> index 8aba3e138909..61f4fe1dec55 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -132,8 +132,6 @@
>  
>  #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
>  
> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> -
>  #define LIBXL__LOGGING_ENABLED
>  
>  #ifdef LIBXL__LOGGING_ENABLED
> diff --git a/tools/tests/x86_emulator/x86-emulate.h b/tools/tests/x86_emulator/x86-emulate.h
> index de1f82668010..aa1ed75ec890 100644
> --- a/tools/tests/x86_emulator/x86-emulate.h
> +++ b/tools/tests/x86_emulator/x86-emulate.h
> @@ -51,9 +51,6 @@
>  #define DEFINE_PER_CPU(type, var) type per_cpu_##var
>  #define this_cpu(var) per_cpu_##var
>  
> -#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> -
>  #define __init
>  #define __maybe_unused __attribute__((__unused__))
>  



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools: Move MASK_INSR to common-macros.h
  2023-06-09 10:11 [PATCH] tools: Move MASK_INSR to common-macros.h Andrew Cooper
  2023-06-09 10:13 ` [PATCH v2] " Andrew Cooper
@ 2023-06-09 10:22 ` Luca Fancellu
  2023-06-09 10:28   ` [PATCH v2] " Andrew Cooper
  2023-06-09 11:33 ` [PATCH] " Roger Pau Monné
  2023-06-09 12:27 ` Jason Andryuk
  3 siblings, 1 reply; 14+ messages in thread
From: Luca Fancellu @ 2023-06-09 10:22 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Xen-devel, Anthony PERARD, Juergen Gross, Jan Beulich,
	Roger Pau Monné,
	Wei Liu



> On 9 Jun 2023, at 11:11, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
> 
> Drop the pair from x86-emulate.h which includes common-macros.h.
> 
> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

I think you forgot to retain my R-by:

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>



> ---
> CC: Anthony PERARD <anthony.perard@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Luca Fancellu <luca.fancellu@arm.com>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> 
> v2:
> * Drop from x86-emulate.h too.
> ---
> tools/include/xen-tools/common-macros.h | 1 +
> tools/libs/light/libxl_internal.h       | 2 --
> tools/tests/x86_emulator/x86-emulate.h  | 3 ---
> 3 files changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h
> index d53b88182560..168691be0e93 100644
> --- a/tools/include/xen-tools/common-macros.h
> +++ b/tools/include/xen-tools/common-macros.h
> @@ -73,6 +73,7 @@
> #endif
> 
> #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> +#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> 
> #ifndef __must_check
> #define __must_check __attribute__((__warn_unused_result__))
> diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
> index 8aba3e138909..61f4fe1dec55 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -132,8 +132,6 @@
> 
> #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
> 
> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> -
> #define LIBXL__LOGGING_ENABLED
> 
> #ifdef LIBXL__LOGGING_ENABLED
> diff --git a/tools/tests/x86_emulator/x86-emulate.h b/tools/tests/x86_emulator/x86-emulate.h
> index de1f82668010..aa1ed75ec890 100644
> --- a/tools/tests/x86_emulator/x86-emulate.h
> +++ b/tools/tests/x86_emulator/x86-emulate.h
> @@ -51,9 +51,6 @@
> #define DEFINE_PER_CPU(type, var) type per_cpu_##var
> #define this_cpu(var) per_cpu_##var
> 
> -#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> -
> #define __init
> #define __maybe_unused __attribute__((__unused__))
> 
> -- 
> 2.30.2
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] tools: Move MASK_INSR to common-macros.h
  2023-06-09 10:22 ` [PATCH] " Luca Fancellu
@ 2023-06-09 10:28   ` Andrew Cooper
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2023-06-09 10:28 UTC (permalink / raw)
  To: xen-devel

On 09/06/2023 11:22 am, Luca Fancellu wrote:
>
>> On 9 Jun 2023, at 11:11, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>
>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>>
>> Drop the pair from x86-emulate.h which includes common-macros.h.
>>
>> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> I think you forgot to retain my R-by:
>
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

I did.  I'm very sorry.  I need more coffee.

Thanks,

~Andrew


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools: Move MASK_INSR to common-macros.h
  2023-06-09 10:11 [PATCH] tools: Move MASK_INSR to common-macros.h Andrew Cooper
  2023-06-09 10:13 ` [PATCH v2] " Andrew Cooper
  2023-06-09 10:22 ` [PATCH] " Luca Fancellu
@ 2023-06-09 11:33 ` Roger Pau Monné
  2023-06-09 12:27 ` Jason Andryuk
  3 siblings, 0 replies; 14+ messages in thread
From: Roger Pau Monné @ 2023-06-09 11:33 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Xen-devel, Anthony PERARD, Juergen Gross, Luca Fancellu,
	Jan Beulich, Wei Liu

On Fri, Jun 09, 2023 at 11:11:05AM +0100, Andrew Cooper wrote:
> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
> 
> Drop the pair from x86-emulate.h which includes common-macros.h.
> 
> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools: Move MASK_INSR to common-macros.h
  2023-06-09 10:11 [PATCH] tools: Move MASK_INSR to common-macros.h Andrew Cooper
                   ` (2 preceding siblings ...)
  2023-06-09 11:33 ` [PATCH] " Roger Pau Monné
@ 2023-06-09 12:27 ` Jason Andryuk
  3 siblings, 0 replies; 14+ messages in thread
From: Jason Andryuk @ 2023-06-09 12:27 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Xen-devel, Anthony PERARD, Juergen Gross, Luca Fancellu,
	Jan Beulich, Roger Pau Monné,
	Wei Liu

On Fri, Jun 9, 2023 at 6:11 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>
> Drop the pair from x86-emulate.h which includes common-macros.h.
>
> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jason Andryuk <jandryuk@gmail.com>

I have (and will now be dropping) an equivalent patch for my
forthcoming HWP v4 series.

Thanks,
Jason


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools: Move MASK_INSR to common-macros.h
  2023-06-09  9:33   ` Andrew Cooper
@ 2023-06-09 10:10     ` Andrew Cooper
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Cooper @ 2023-06-09 10:10 UTC (permalink / raw)
  To: Jan Beulich, Luca Fancellu; +Cc: Anthony PERARD, Juergen Gross, Xen-devel

On 09/06/2023 10:33 am, Andrew Cooper wrote:
> On 09/06/2023 10:31 am, Jan Beulich wrote:
>> On 08.06.2023 19:40, Andrew Cooper wrote:
>>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>> Right, that's one thing which should have been done right away.
>> The other is that both macros should have been purged from
>> tools/tests/x86_emulator/x86-emulate.h (which includes
>> xen-tools/common-macros.h). Luca?
> Hmm - I explicitly checked that, and concluded they didn't...  Happy to
> purge them if I'm wrong.

I think I was looking in an old branch.  Fixed for v2.

~Andrew


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools: Move MASK_INSR to common-macros.h
  2023-06-09 10:06   ` Andrew Cooper
@ 2023-06-09 10:10     ` Luca Fancellu
  0 siblings, 0 replies; 14+ messages in thread
From: Luca Fancellu @ 2023-06-09 10:10 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel, Anthony PERARD, Juergen Gross



> On 9 Jun 2023, at 11:06, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> On 08/06/2023 8:37 pm, Luca Fancellu wrote:
>>> On 8 Jun 2023, at 18:40, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>> 
>>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>>> 
>>> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
>> I don’t think this patch is fixing a bug:
>> 
>> ### Fixes:
>> 
>> If your patch fixes a bug in a specific commit, e.g. you found an issue using
>> ``git bisect``, please use the `Fixes:` tag with the first 12 characters of
>> the commit id, and the one line summary.
> 
> That a poor explanation...
> 
> Fixes: is about corrections to the patch, not bugs.
> 
> 56a7aaa16bfe is unlikely to be backported, but if a downstream were to
> backport your SVE patches, Fixes: identify all other patches they need
> to take.
> 
> Fixes: was specifically invented to let tooling (partially) automate the
> task if finding new patches to backport, based on what had already been
> backported.

Ok this makes sense, so that a tool can easily understand where to put the focus.

> 
> Concrete bugs are the majority reason for a Fixes tag, sure, but not the
> only reason.  In this case, a downstream absolutely doesn't want to get
> into a position where these macros aren't together in a pair, because it
> there will be a case in the future where it causes a build error.
> 
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> But it makes sense, so 
>> 
>> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
> 
> Thanks.  As you've already indicated that you're ok with fixing up
> x86-emulate.h in v2, I'll retain this.

sure, thanks for fixing it

> 
> ~Andrew


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools: Move MASK_INSR to common-macros.h
  2023-06-08 19:37 ` Luca Fancellu
@ 2023-06-09 10:06   ` Andrew Cooper
  2023-06-09 10:10     ` Luca Fancellu
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2023-06-09 10:06 UTC (permalink / raw)
  To: Luca Fancellu; +Cc: Xen-devel, Anthony PERARD, Juergen Gross

On 08/06/2023 8:37 pm, Luca Fancellu wrote:
>> On 8 Jun 2023, at 18:40, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>
>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
>>
>> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
> I don’t think this patch is fixing a bug:
>
> ### Fixes:
>
> If your patch fixes a bug in a specific commit, e.g. you found an issue using
> ``git bisect``, please use the `Fixes:` tag with the first 12 characters of
> the commit id, and the one line summary.

That a poor explanation...

Fixes: is about corrections to the patch, not bugs.

56a7aaa16bfe is unlikely to be backported, but if a downstream were to
backport your SVE patches, Fixes: identify all other patches they need
to take.

Fixes: was specifically invented to let tooling (partially) automate the
task if finding new patches to backport, based on what had already been
backported.

Concrete bugs are the majority reason for a Fixes tag, sure, but not the
only reason.  In this case, a downstream absolutely doesn't want to get
into a position where these macros aren't together in a pair, because it
there will be a case in the future where it causes a build error.

>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> But it makes sense, so 
>
> Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

Thanks.  As you've already indicated that you're ok with fixing up
x86-emulate.h in v2, I'll retain this.

~Andrew


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools: Move MASK_INSR to common-macros.h
  2023-06-09  9:31 ` Jan Beulich
  2023-06-09  9:33   ` Andrew Cooper
@ 2023-06-09  9:37   ` Luca Fancellu
  1 sibling, 0 replies; 14+ messages in thread
From: Luca Fancellu @ 2023-06-09  9:37 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Anthony PERARD, Juergen Gross, Xen-devel



> On 9 Jun 2023, at 10:31, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 08.06.2023 19:40, Andrew Cooper wrote:
>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
> 
> Right, that's one thing which should have been done right away.
> The other is that both macros should have been purged from
> tools/tests/x86_emulator/x86-emulate.h (which includes
> xen-tools/common-macros.h). Luca?

mmm right I’ve missed that, if Andrew can handle it in this patch I’m ok, if you
want me to send a patch I can do it

> 
> Jan
> 
>> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Anthony PERARD <anthony.perard@citrix.com>
>> CC: Juergen Gross <jgross@suse.com>
>> CC: Luca Fancellu <luca.fancellu@arm.com>
>> ---
>> tools/include/xen-tools/common-macros.h | 1 +
>> tools/libs/light/libxl_internal.h       | 2 --
>> 2 files changed, 1 insertion(+), 2 deletions(-)
>> 
>> diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h
>> index d53b88182560..168691be0e93 100644
>> --- a/tools/include/xen-tools/common-macros.h
>> +++ b/tools/include/xen-tools/common-macros.h
>> @@ -73,6 +73,7 @@
>> #endif
>> 
>> #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
>> +#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
>> 
>> #ifndef __must_check
>> #define __must_check __attribute__((__warn_unused_result__))
>> diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
>> index 8aba3e138909..61f4fe1dec55 100644
>> --- a/tools/libs/light/libxl_internal.h
>> +++ b/tools/libs/light/libxl_internal.h
>> @@ -132,8 +132,6 @@
>> 
>> #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
>> 
>> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
>> -
>> #define LIBXL__LOGGING_ENABLED
>> 
>> #ifdef LIBXL__LOGGING_ENABLED
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools: Move MASK_INSR to common-macros.h
  2023-06-09  9:31 ` Jan Beulich
@ 2023-06-09  9:33   ` Andrew Cooper
  2023-06-09 10:10     ` Andrew Cooper
  2023-06-09  9:37   ` Luca Fancellu
  1 sibling, 1 reply; 14+ messages in thread
From: Andrew Cooper @ 2023-06-09  9:33 UTC (permalink / raw)
  To: Jan Beulich, Luca Fancellu; +Cc: Anthony PERARD, Juergen Gross, Xen-devel

On 09/06/2023 10:31 am, Jan Beulich wrote:
> On 08.06.2023 19:40, Andrew Cooper wrote:
>> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
> Right, that's one thing which should have been done right away.
> The other is that both macros should have been purged from
> tools/tests/x86_emulator/x86-emulate.h (which includes
> xen-tools/common-macros.h). Luca?

Hmm - I explicitly checked that, and concluded they didn't...  Happy to
purge them if I'm wrong.

~Andrew


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools: Move MASK_INSR to common-macros.h
  2023-06-08 17:40 Andrew Cooper
  2023-06-08 19:37 ` Luca Fancellu
@ 2023-06-09  9:31 ` Jan Beulich
  2023-06-09  9:33   ` Andrew Cooper
  2023-06-09  9:37   ` Luca Fancellu
  1 sibling, 2 replies; 14+ messages in thread
From: Jan Beulich @ 2023-06-09  9:31 UTC (permalink / raw)
  To: Andrew Cooper, Luca Fancellu; +Cc: Anthony PERARD, Juergen Gross, Xen-devel

On 08.06.2023 19:40, Andrew Cooper wrote:
> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.

Right, that's one thing which should have been done right away.
The other is that both macros should have been purged from
tools/tests/x86_emulator/x86-emulate.h (which includes
xen-tools/common-macros.h). Luca?

Jan

> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Anthony PERARD <anthony.perard@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Luca Fancellu <luca.fancellu@arm.com>
> ---
>  tools/include/xen-tools/common-macros.h | 1 +
>  tools/libs/light/libxl_internal.h       | 2 --
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h
> index d53b88182560..168691be0e93 100644
> --- a/tools/include/xen-tools/common-macros.h
> +++ b/tools/include/xen-tools/common-macros.h
> @@ -73,6 +73,7 @@
>  #endif
>  
>  #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> +#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
>  
>  #ifndef __must_check
>  #define __must_check __attribute__((__warn_unused_result__))
> diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
> index 8aba3e138909..61f4fe1dec55 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -132,8 +132,6 @@
>  
>  #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
>  
> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> -
>  #define LIBXL__LOGGING_ENABLED
>  
>  #ifdef LIBXL__LOGGING_ENABLED



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tools: Move MASK_INSR to common-macros.h
  2023-06-08 17:40 Andrew Cooper
@ 2023-06-08 19:37 ` Luca Fancellu
  2023-06-09 10:06   ` Andrew Cooper
  2023-06-09  9:31 ` Jan Beulich
  1 sibling, 1 reply; 14+ messages in thread
From: Luca Fancellu @ 2023-06-08 19:37 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel, Anthony PERARD, Juergen Gross



> On 8 Jun 2023, at 18:40, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.
> 
> Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")

I don’t think this patch is fixing a bug:

### Fixes:

If your patch fixes a bug in a specific commit, e.g. you found an issue using
``git bisect``, please use the `Fixes:` tag with the first 12 characters of
the commit id, and the one line summary.

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

But it makes sense, so 

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

> ---
> CC: Anthony PERARD <anthony.perard@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Luca Fancellu <luca.fancellu@arm.com>
> ---
> tools/include/xen-tools/common-macros.h | 1 +
> tools/libs/light/libxl_internal.h       | 2 --
> 2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h
> index d53b88182560..168691be0e93 100644
> --- a/tools/include/xen-tools/common-macros.h
> +++ b/tools/include/xen-tools/common-macros.h
> @@ -73,6 +73,7 @@
> #endif
> 
> #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> +#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> 
> #ifndef __must_check
> #define __must_check __attribute__((__warn_unused_result__))
> diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
> index 8aba3e138909..61f4fe1dec55 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -132,8 +132,6 @@
> 
> #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
> 
> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> -
> #define LIBXL__LOGGING_ENABLED
> 
> #ifdef LIBXL__LOGGING_ENABLED
> -- 
> 2.30.2
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] tools: Move MASK_INSR to common-macros.h
@ 2023-06-08 17:40 Andrew Cooper
  2023-06-08 19:37 ` Luca Fancellu
  2023-06-09  9:31 ` Jan Beulich
  0 siblings, 2 replies; 14+ messages in thread
From: Andrew Cooper @ 2023-06-08 17:40 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Anthony PERARD, Juergen Gross, Luca Fancellu

MASK_EXTR() and MASK_INSR() are a matching pair.  Keep them together.

Fixes: 56a7aaa16bfe ("tools: add physinfo arch_capabilities handling for Arm")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@citrix.com>
CC: Juergen Gross <jgross@suse.com>
CC: Luca Fancellu <luca.fancellu@arm.com>
---
 tools/include/xen-tools/common-macros.h | 1 +
 tools/libs/light/libxl_internal.h       | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/include/xen-tools/common-macros.h b/tools/include/xen-tools/common-macros.h
index d53b88182560..168691be0e93 100644
--- a/tools/include/xen-tools/common-macros.h
+++ b/tools/include/xen-tools/common-macros.h
@@ -73,6 +73,7 @@
 #endif
 
 #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
+#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
 
 #ifndef __must_check
 #define __must_check __attribute__((__warn_unused_result__))
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index 8aba3e138909..61f4fe1dec55 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -132,8 +132,6 @@
 
 #define DIV_ROUNDUP(n, d) (((n) + (d) - 1) / (d))
 
-#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
-
 #define LIBXL__LOGGING_ENABLED
 
 #ifdef LIBXL__LOGGING_ENABLED
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-06-09 12:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-09 10:11 [PATCH] tools: Move MASK_INSR to common-macros.h Andrew Cooper
2023-06-09 10:13 ` [PATCH v2] " Andrew Cooper
2023-06-09 10:22 ` [PATCH] " Luca Fancellu
2023-06-09 10:28   ` [PATCH v2] " Andrew Cooper
2023-06-09 11:33 ` [PATCH] " Roger Pau Monné
2023-06-09 12:27 ` Jason Andryuk
  -- strict thread matches above, loose matches on Subject: below --
2023-06-08 17:40 Andrew Cooper
2023-06-08 19:37 ` Luca Fancellu
2023-06-09 10:06   ` Andrew Cooper
2023-06-09 10:10     ` Luca Fancellu
2023-06-09  9:31 ` Jan Beulich
2023-06-09  9:33   ` Andrew Cooper
2023-06-09 10:10     ` Andrew Cooper
2023-06-09  9:37   ` Luca Fancellu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).