Re: [PATCH 5/6] tools: add a dedicated header file for barrier definitions

["Anthony PERARD" <anthony.perard@vates.tech>]

On Wed, Oct 23, 2024 at 03:10:04PM +0200, Juergen Gross wrote:
> diff --git a/tools/include/xen-barrier.h b/tools/include/xen-barrier.h
> new file mode 100644
> index 0000000000..62036f528b
> --- /dev/null
> +++ b/tools/include/xen-barrier.h
> @@ -0,0 +1,51 @@
> +/******************************************************************************
> + * xen-barrier.h
> + *
> + * Definition of CPU barriers, part of libxenctrl.

Does it needs to be part of "libxenctrl" ? :-) Since the goal is to be
able to use the header without xenctrl.

> + *
> + * Copyright (c) 2003-2004, K A Fraser.

I'm not sure this copyright line is enough, looking at `git blame`.

Keir introduce xen_barrier macro in 2012, in
8d3f757328e1 ("libxc: Update rmb/wmb for x86.")
Stefano introduced the Arm macro in 2012, in daa314fe1938 ("arm: compile
libxc"), and Ian in 2013 in ae4b6f29a983 ("tools: libxc: arm64
support").
There's been a modification by Andrew in 2020, so Citrix copyright,
in de16a8fa0db7 ("x86: Use LOCK ADD instead of MFENCE for smp_mb()").

So overall, we probably want:
Copyright (C) 2003-2012, K A Fraser.
Copyright (C) 2012-2020 Citrix Systems, Inc.


> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation;
> + * version 2.1 of the License.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef XENBARRIER_H
> +#define XENBARRIER_H

With an extra '_' for the '-' in the header filename?
    XEN_BARRIER_H


Otherwise, the rest of the patch looks fine to me, even without the rest
of the series.

Cheers,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech