Re: [PATCH v2 01/35] xen: introduce resource.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.12.2024 05:41, Denis Mukhin via B4 Relay wrote:
> --- /dev/null
> +++ b/xen/include/xen/resource.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

GPL-2.0-only

> +/*
> + * System resource description.
> + *
> + * Reference:
> + *   include/linux/ioport.h

I'm unsure of the usefulness of such a reference.

> + */
> +#if !defined(XEN__RESOURCE_H)

Nit: #ifdef / #ifndef please whenever possible (as long as not inconsistent
with adjacent code).

> +#define XEN__RESOURCE_H
> +
> +#define IORESOURCE_BITS         0x000000FFU      /* Bus-specific bits */
> +
> +#define IORESOURCE_TYPE_BITS    0x00001F00U      /* Resource type */
> +#define IORESOURCE_IO           0x00000100U      /* PCI/ISA I/O ports */
> +#define IORESOURCE_MEM          0x00000200U
> +#define IORESOURCE_REG          0x00000300U      /* Register offsets */
> +#define IORESOURCE_IRQ          0x00000400U
> +#define IORESOURCE_DMA          0x00000800U
> +#define IORESOURCE_BUS          0x00001000U
> +
> +#define IORESOURCE_PREFETCH     0x00002000U      /* No side effects */
> +#define IORESOURCE_READONLY     0x00004000U
> +#define IORESOURCE_CACHEABLE    0x00008000U
> +#define IORESOURCE_RANGELENGTH  0x00010000U
> +#define IORESOURCE_SHADOWABLE   0x00020000U
> +
> +#define IORESOURCE_UNKNOWN      (~0U)
> +
> +struct resource {
> +    paddr_t addr;
> +    paddr_t size;
> +    unsigned int type;
> +};
> +
> +#define resource_size(res) (res)->size;

The semicolon surely was wrong before and is wrong now. Plus Misra
demands that such macro expansions be parenthesized, I think.

> +#define foreach_resource(res) \
> +    for (; res && res->type != IORESOURCE_UNKNOWN; res++)

This one isn't being moved, but is being added. It's not used here,
which makes it difficult to judge its correctness. Perhaps better to
introduce this when its first needed, and then right away with the
required parentheses around uses of the macro parameter.

> +#endif /* #if !defined(XEN__RESOURCE_H) */

Just the guard identifier in the comment please.

Jan