* [PATCH v4 01/14] xen/asm-generic: introduce stub header paging.h
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-28 21:16 ` Shawn Anastasio
2023-11-27 14:13 ` [PATCH v4 02/14] xen/asm-generic: introduce generic device.h Oleksii Kurochko
` (13 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Jan Beulich, Wei Liu, Shawn Anastasio,
Julien Grall
The patch introduces generic paging.h header for Arm, PPC and
RISC-V.
All mentioned above architectures use hardware virt extensions
and hardware pagetable extensions thereby it makes sense to set
paging_mode_translate and paging_mode_external by default.
Also in this patch Arm and PPC architectures are switched to
generic paging.h header.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
Changes in V4:
- Added Acked-by: Julien Grall <jgrall@amazon.com>.
---
Changes in V3:
- Sort xen/arch/{arm,ppc}/include/asm/Makefile alphabetically.
- Add Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Changes in V2:
- evaluate argument of macros
- covert macros to true
- use proper tabs
- switch Arm & PPC to generic paging.h
- update commit message
---
xen/arch/arm/include/asm/Makefile | 1 +
xen/arch/arm/include/asm/paging.h | 16 ----------------
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/paging.h | 7 -------
xen/include/asm-generic/paging.h | 19 +++++++++++++++++++
5 files changed, 21 insertions(+), 23 deletions(-)
delete mode 100644 xen/arch/arm/include/asm/paging.h
delete mode 100644 xen/arch/ppc/include/asm/paging.h
create mode 100644 xen/include/asm-generic/paging.h
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index 821addb0bf..ece7fa66dd 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
+generic-y += paging.h
generic-y += vm_event.h
diff --git a/xen/arch/arm/include/asm/paging.h b/xen/arch/arm/include/asm/paging.h
deleted file mode 100644
index 6d1a000246..0000000000
--- a/xen/arch/arm/include/asm/paging.h
+++ /dev/null
@@ -1,16 +0,0 @@
-#ifndef _XEN_PAGING_H
-#define _XEN_PAGING_H
-
-#define paging_mode_translate(d) (1)
-#define paging_mode_external(d) (1)
-
-#endif /* XEN_PAGING_H */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index 821addb0bf..ece7fa66dd 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
+generic-y += paging.h
generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/paging.h b/xen/arch/ppc/include/asm/paging.h
deleted file mode 100644
index eccacece29..0000000000
--- a/xen/arch/ppc/include/asm/paging.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef __ASM_PPC_PAGING_H__
-#define __ASM_PPC_PAGING_H__
-
-#define paging_mode_translate(d) (1)
-#define paging_mode_external(d) (1)
-
-#endif /* __ASM_PPC_PAGING_H__ */
diff --git a/xen/include/asm-generic/paging.h b/xen/include/asm-generic/paging.h
new file mode 100644
index 0000000000..8df534cfdc
--- /dev/null
+++ b/xen/include/asm-generic/paging.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_PAGING_H__
+#define __ASM_GENERIC_PAGING_H__
+
+#include <xen/stdbool.h>
+
+#define paging_mode_translate(d) ((void)(d), true)
+#define paging_mode_external(d) ((void)(d), true)
+
+#endif /* __ASM_GENERIC_PAGING_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 01/14] xen/asm-generic: introduce stub header paging.h
2023-11-27 14:13 ` [PATCH v4 01/14] xen/asm-generic: introduce stub header paging.h Oleksii Kurochko
@ 2023-11-28 21:16 ` Shawn Anastasio
0 siblings, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-28 21:16 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu, Julien Grall
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> The patch introduces generic paging.h header for Arm, PPC and
> RISC-V.
>
> All mentioned above architectures use hardware virt extensions
> and hardware pagetable extensions thereby it makes sense to set
> paging_mode_translate and paging_mode_external by default.
>
> Also in this patch Arm and PPC architectures are switched to
> generic paging.h header.
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
2023-11-27 14:13 ` [PATCH v4 01/14] xen/asm-generic: introduce stub header paging.h Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-27 14:31 ` Jan Beulich
2023-11-28 21:28 ` Shawn Anastasio
2023-11-27 14:13 ` [PATCH v4 03/14] xen/asm-generic: introduce generic hypercall.h Oleksii Kurochko
` (12 subsequent siblings)
14 siblings, 2 replies; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Jan Beulich, Wei Liu, Shawn Anastasio
Arm, PPC and RISC-V use the same device.h thereby device.h
was moved to asm-generic. Arm's device.h was taken as a base with
the following changes:
- #ifdef PCI related things.
- #ifdef ACPI related things.
- Rename #ifdef guards.
- Add SPDX tag.
- #ifdef CONFIG_HAS_DEVICE_TREE related things.
- #ifdef-ing iommu related things with CONFIG_HAS_PASSTHROUGH.
Also Arm and PPC are switched to asm-generic version of device.h
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V4:
- Updated the commit message
- Switched Arm and PPC to asm-generic version of device.h
- Replaced HAS_PCI with CONFIG_HAS_PCI
- ifdef-ing iommu filed of dev_archdata struct with CONFIG_HAS_PASSTHROUGH
- ifdef-ing iommu_fwspec of device struct with CONFIG_HAS_PASSTHROUGH
- ifdef-ing DT related things with CONFIG_HAS_DEVICE_TREE
- Updated the commit message ( remove a note with question about
if device.h should be in asm-generic or not )
- Replaced DEVICE_IC with DEVICE_INTERRUPT_CONTROLLER
- Rationalized usage of CONFIG_HAS_* in device.h
- Fixed indents for ACPI_DEVICE_START and ACPI_DEVICE_END
---
Changes in V3:
- ifdef device tree related things.
- update the commit message
---
Changes in V2:
- take ( as common ) device.h from Arm as PPC and RISC-V use it as a base.
- #ifdef PCI related things.
- #ifdef ACPI related things.
- rename DEVICE_GIC to DEVIC_IC.
- rename #ifdef guards.
- switch Arm and PPC to generic device.h
- add SPDX tag
- update the commit message
---
xen/arch/arm/device.c | 15 ++-
xen/arch/arm/domain_build.c | 2 +-
xen/arch/arm/gic-v2.c | 4 +-
xen/arch/arm/gic-v3.c | 6 +-
xen/arch/arm/gic.c | 4 +-
xen/arch/arm/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/device.h | 53 --------
xen/arch/ppc/include/asm/irq.h | 4 +
.../asm => include/asm-generic}/device.h | 125 +++++++++++-------
| 0
11 files changed, 106 insertions(+), 109 deletions(-)
delete mode 100644 xen/arch/ppc/include/asm/device.h
rename xen/{arch/arm/include/asm => include/asm-generic}/device.h (79%)
create mode 100644 xen/include/headers++.chk.new
diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
index 1f631d3274..affbe79f9a 100644
--- a/xen/arch/arm/device.c
+++ b/xen/arch/arm/device.c
@@ -16,7 +16,10 @@
#include <xen/lib.h>
extern const struct device_desc _sdevice[], _edevice[];
+
+#ifdef CONFIG_ACPI
extern const struct acpi_device_desc _asdevice[], _aedevice[];
+#endif
int __init device_init(struct dt_device_node *dev, enum device_class class,
const void *data)
@@ -45,6 +48,7 @@ int __init device_init(struct dt_device_node *dev, enum device_class class,
return -EBADF;
}
+#ifdef CONFIG_ACPI
int __init acpi_device_init(enum device_class class, const void *data, int class_type)
{
const struct acpi_device_desc *desc;
@@ -61,6 +65,7 @@ int __init acpi_device_init(enum device_class class, const void *data, int class
return -EBADF;
}
+#endif
enum device_class device_get_class(const struct dt_device_node *dev)
{
@@ -329,9 +334,13 @@ int handle_device(struct domain *d, struct dt_device_node *dev, p2m_type_t p2mt,
struct map_range_data mr_data = {
.d = d,
.p2mt = p2mt,
- .skip_mapping = !own_device ||
- (is_pci_passthrough_enabled() &&
- (device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE)),
+ .skip_mapping =
+ !own_device
+#ifdef CONFIG_HAS_PCI
+ || (is_pci_passthrough_enabled() &&
+ (device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE))
+#endif
+ ,
.iomem_ranges = iomem_ranges,
.irq_ranges = irq_ranges
};
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2dd2926b41..a26cbff68e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2361,7 +2361,7 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo,
* Replace these nodes with our own. Note that the original may be
* used_by DOMID_XEN so this check comes first.
*/
- if ( device_get_class(node) == DEVICE_GIC )
+ if ( device_get_class(node) == DEVICE_INTERRUPT_CONTROLLER )
return make_gic_node(d, kinfo->fdt, node);
if ( dt_match_node(timer_matches, node) )
return make_timer_node(kinfo);
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index cf392bfd1c..5d6885e389 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -1366,7 +1366,7 @@ static const struct dt_device_match gicv2_dt_match[] __initconst =
{ /* sentinel */ },
};
-DT_DEVICE_START(gicv2, "GICv2", DEVICE_GIC)
+DT_DEVICE_START(gicv2, "GICv2", DEVICE_INTERRUPT_CONTROLLER)
.dt_match = gicv2_dt_match,
.init = gicv2_dt_preinit,
DT_DEVICE_END
@@ -1381,7 +1381,7 @@ static int __init gicv2_acpi_preinit(const void *data)
return 0;
}
-ACPI_DEVICE_START(agicv2, "GICv2", DEVICE_GIC)
+ACPI_DEVICE_START(agicv2, "GICv2", DEVICE_INTERRUPT_CONTROLLER)
.class_type = ACPI_MADT_GIC_VERSION_V2,
.init = gicv2_acpi_preinit,
ACPI_DEVICE_END
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 9b35a8c8a7..d8f9aa0352 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1875,7 +1875,7 @@ static const struct dt_device_match gicv3_dt_match[] __initconst =
{ /* sentinel */ },
};
-DT_DEVICE_START(gicv3, "GICv3", DEVICE_GIC)
+DT_DEVICE_START(gicv3, "GICv3", DEVICE_INTERRUPT_CONTROLLER)
.dt_match = gicv3_dt_match,
.init = gicv3_dt_preinit,
DT_DEVICE_END
@@ -1890,12 +1890,12 @@ static int __init gicv3_acpi_preinit(const void *data)
return 0;
}
-ACPI_DEVICE_START(agicv3, "GICv3", DEVICE_GIC)
+ACPI_DEVICE_START(agicv3, "GICv3", DEVICE_INTERRUPT_CONTROLLER)
.class_type = ACPI_MADT_GIC_VERSION_V3,
.init = gicv3_acpi_preinit,
ACPI_DEVICE_END
-ACPI_DEVICE_START(agicv4, "GICv4", DEVICE_GIC)
+ACPI_DEVICE_START(agicv4, "GICv4", DEVICE_INTERRUPT_CONTROLLER)
.class_type = ACPI_MADT_GIC_VERSION_V4,
.init = gicv3_acpi_preinit,
ACPI_DEVICE_END
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index d922ea67aa..b5a9c8266c 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -234,7 +234,7 @@ static void __init gic_dt_preinit(void)
if ( !dt_get_parent(node) )
continue;
- rc = device_init(node, DEVICE_GIC, NULL);
+ rc = device_init(node, DEVICE_INTERRUPT_CONTROLLER, NULL);
if ( !rc )
{
/* NOTE: Only one GIC is supported */
@@ -262,7 +262,7 @@ static void __init gic_acpi_preinit(void)
dist = container_of(header, struct acpi_madt_generic_distributor, header);
- if ( acpi_device_init(DEVICE_GIC, NULL, dist->version) )
+ if ( acpi_device_init(DEVICE_INTERRUPT_CONTROLLER, NULL, dist->version) )
panic("Unable to find compatible GIC in the ACPI table\n");
}
#else
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index ece7fa66dd..df4c1ebb08 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
+generic-y += device.h
generic-y += paging.h
generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index ece7fa66dd..df4c1ebb08 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
+generic-y += device.h
generic-y += paging.h
generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/device.h b/xen/arch/ppc/include/asm/device.h
deleted file mode 100644
index 8253e61d51..0000000000
--- a/xen/arch/ppc/include/asm/device.h
+++ /dev/null
@@ -1,53 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_DEVICE_H__
-#define __ASM_PPC_DEVICE_H__
-
-enum device_type
-{
- DEV_DT,
- DEV_PCI,
-};
-
-struct device {
- enum device_type type;
-#ifdef CONFIG_HAS_DEVICE_TREE
- struct dt_device_node *of_node; /* Used by drivers imported from Linux */
-#endif
-};
-
-enum device_class
-{
- DEVICE_SERIAL,
- DEVICE_IOMMU,
- DEVICE_PCI_HOSTBRIDGE,
- /* Use for error */
- DEVICE_UNKNOWN,
-};
-
-struct device_desc {
- /* Device name */
- const char *name;
- /* Device class */
- enum device_class class;
- /* List of devices supported by this driver */
- const struct dt_device_match *dt_match;
- /*
- * Device initialization.
- *
- * -EAGAIN is used to indicate that device probing is deferred.
- */
- int (*init)(struct dt_device_node *dev, const void *data);
-};
-
-typedef struct device device_t;
-
-#define DT_DEVICE_START(name_, namestr_, class_) \
-static const struct device_desc __dev_desc_##name_ __used \
-__section(".dev.info") = { \
- .name = namestr_, \
- .class = class_, \
-
-#define DT_DEVICE_END \
-};
-
-#endif /* __ASM_PPC_DEVICE_H__ */
diff --git a/xen/arch/ppc/include/asm/irq.h b/xen/arch/ppc/include/asm/irq.h
index 5c37d0cf25..49193fddff 100644
--- a/xen/arch/ppc/include/asm/irq.h
+++ b/xen/arch/ppc/include/asm/irq.h
@@ -3,7 +3,9 @@
#define __ASM_PPC_IRQ_H__
#include <xen/lib.h>
+#ifdef CONFIG_HAS_DEVICE_TREE
#include <xen/device_tree.h>
+#endif
#include <public/device_tree_defs.h>
/* TODO */
@@ -25,9 +27,11 @@ static inline void arch_move_irqs(struct vcpu *v)
BUG_ON("unimplemented");
}
+#ifdef CONFIG_HAS_DEVICE_TREE
static inline int platform_get_irq(const struct dt_device_node *device, int index)
{
BUG_ON("unimplemented");
}
+#endif
#endif /* __ASM_PPC_IRQ_H__ */
diff --git a/xen/arch/arm/include/asm/device.h b/xen/include/asm-generic/device.h
similarity index 79%
rename from xen/arch/arm/include/asm/device.h
rename to xen/include/asm-generic/device.h
index b5d451e087..063c448c4e 100644
--- a/xen/arch/arm/include/asm/device.h
+++ b/xen/include/asm-generic/device.h
@@ -1,14 +1,37 @@
-#ifndef __ASM_ARM_DEVICE_H
-#define __ASM_ARM_DEVICE_H
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_DEVICE_H__
+#define __ASM_GENERIC_DEVICE_H__
+
+#include <xen/stdbool.h>
enum device_type
{
+#ifdef CONFIG_HAS_DEVICE_TREE
DEV_DT,
+#endif
+
+#ifdef CONFIG_HAS_PCI
DEV_PCI,
+#endif
+ DEV_TYPE_MAX,
+};
+
+enum device_class
+{
+ DEVICE_SERIAL,
+ DEVICE_IOMMU,
+ DEVICE_INTERRUPT_CONTROLLER,
+#ifdef CONFIG_HAS_PCI
+ DEVICE_PCI_HOSTBRIDGE,
+#endif
+ /* Use for error */
+ DEVICE_UNKNOWN,
};
struct dev_archdata {
+#ifdef CONFIG_HAS_PASSTHROUGH
void *iommu; /* IOMMU private data */
+#endif
};
/* struct device - The basic device structure */
@@ -19,31 +42,65 @@ struct device
struct dt_device_node *of_node; /* Used by drivers imported from Linux */
#endif
struct dev_archdata archdata;
+#ifdef CONFIG_HAS_PASSTHROUGH
struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU instance data */
+#endif
};
typedef struct device device_t;
+#ifdef CONFIG_HAS_DEVICE_TREE
+
#include <xen/device_tree.h>
-#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
#define dev_is_dt(dev) ((dev)->type == DEV_DT)
-enum device_class
-{
- DEVICE_SERIAL,
- DEVICE_IOMMU,
- DEVICE_GIC,
- DEVICE_PCI_HOSTBRIDGE,
- /* Use for error */
- DEVICE_UNKNOWN,
+/**
+ * device_init - Initialize a device
+ * @dev: device to initialize
+ * @class: class of the device (serial, network...)
+ * @data: specific data for initializing the device
+ *
+ * Return 0 on success.
+ */
+int device_init(struct dt_device_node *dev, enum device_class class,
+ const void *data);
+
+/**
+ * device_get_type - Get the type of the device
+ * @dev: device to match
+ *
+ * Return the device type on success or DEVICE_ANY on failure
+ */
+enum device_class device_get_class(const struct dt_device_node *dev);
+
+#define DT_DEVICE_START(_name, _namestr, _class) \
+static const struct device_desc __dev_desc_##_name __used \
+__section(".dev.info") = { \
+ .name = _namestr, \
+ .class = _class, \
+
+#define DT_DEVICE_END \
};
+#else /* !CONFIG_HAS_DEVICE_TREE */
+#define dev_is_dt(dev) ((void)(dev), false)
+#endif /* CONFIG_HAS_DEVICE_TREE */
+
+#ifdef CONFIG_HAS_PCI
+#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
+#else
+#define dev_is_pci(dev) ((void)(dev), false)
+#endif
+
struct device_desc {
/* Device name */
const char *name;
/* Device class */
enum device_class class;
+
+#ifdef CONFIG_HAS_DEVICE_TREE
+
/* List of devices supported by this driver */
const struct dt_device_match *dt_match;
/*
@@ -52,8 +109,12 @@ struct device_desc {
* -EAGAIN is used to indicate that device probing is deferred.
*/
int (*init)(struct dt_device_node *dev, const void *data);
+
+#endif
};
+#ifdef CONFIG_ACPI
+
struct acpi_device_desc {
/* Device name */
const char *name;
@@ -75,44 +136,18 @@ struct acpi_device_desc {
int acpi_device_init(enum device_class class,
const void *data, int class_type);
-/**
- * device_init - Initialize a device
- * @dev: device to initialize
- * @class: class of the device (serial, network...)
- * @data: specific data for initializing the device
- *
- * Return 0 on success.
- */
-int device_init(struct dt_device_node *dev, enum device_class class,
- const void *data);
-
-/**
- * device_get_type - Get the type of the device
- * @dev: device to match
- *
- * Return the device type on success or DEVICE_ANY on failure
- */
-enum device_class device_get_class(const struct dt_device_node *dev);
+#define ACPI_DEVICE_START(_name, _namestr, _class) \
+static const struct acpi_device_desc __dev_desc_##_name __used \
+__section(".adev.info") = { \
+ .name = _namestr, \
+ .class = _class, \
-#define DT_DEVICE_START(_name, _namestr, _class) \
-static const struct device_desc __dev_desc_##_name __used \
-__section(".dev.info") = { \
- .name = _namestr, \
- .class = _class, \
-
-#define DT_DEVICE_END \
+#define ACPI_DEVICE_END \
};
-#define ACPI_DEVICE_START(_name, _namestr, _class) \
-static const struct acpi_device_desc __dev_desc_##_name __used \
-__section(".adev.info") = { \
- .name = _namestr, \
- .class = _class, \
-
-#define ACPI_DEVICE_END \
-};
+#endif /* CONFIG_ACPI */
-#endif /* __ASM_ARM_DEVICE_H */
+#endif /* __ASM_GENERIC_DEVICE_H__ */
/*
* Local variables:
diff --git a/xen/include/headers++.chk.new b/xen/include/headers++.chk.new
new file mode 100644
index 0000000000..e69de29bb2
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
2023-11-27 14:13 ` [PATCH v4 02/14] xen/asm-generic: introduce generic device.h Oleksii Kurochko
@ 2023-11-27 14:31 ` Jan Beulich
2023-11-27 19:46 ` Oleksii
2023-11-28 21:28 ` Shawn Anastasio
1 sibling, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2023-11-27 14:31 UTC (permalink / raw)
To: Oleksii Kurochko
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On 27.11.2023 15:13, Oleksii Kurochko wrote:
> Arm, PPC and RISC-V use the same device.h thereby device.h
> was moved to asm-generic. Arm's device.h was taken as a base with
> the following changes:
> - #ifdef PCI related things.
> - #ifdef ACPI related things.
> - Rename #ifdef guards.
> - Add SPDX tag.
> - #ifdef CONFIG_HAS_DEVICE_TREE related things.
> - #ifdef-ing iommu related things with CONFIG_HAS_PASSTHROUGH.
>
> Also Arm and PPC are switched to asm-generic version of device.h
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in V4:
> - Updated the commit message
> - Switched Arm and PPC to asm-generic version of device.h
> - Replaced HAS_PCI with CONFIG_HAS_PCI
> - ifdef-ing iommu filed of dev_archdata struct with CONFIG_HAS_PASSTHROUGH
> - ifdef-ing iommu_fwspec of device struct with CONFIG_HAS_PASSTHROUGH
> - ifdef-ing DT related things with CONFIG_HAS_DEVICE_TREE
> - Updated the commit message ( remove a note with question about
> if device.h should be in asm-generic or not )
> - Replaced DEVICE_IC with DEVICE_INTERRUPT_CONTROLLER
> - Rationalized usage of CONFIG_HAS_* in device.h
> - Fixed indents for ACPI_DEVICE_START and ACPI_DEVICE_END
> ---
> Changes in V3:
> - ifdef device tree related things.
> - update the commit message
> ---
> Changes in V2:
> - take ( as common ) device.h from Arm as PPC and RISC-V use it as a base.
> - #ifdef PCI related things.
> - #ifdef ACPI related things.
> - rename DEVICE_GIC to DEVIC_IC.
> - rename #ifdef guards.
> - switch Arm and PPC to generic device.h
> - add SPDX tag
> - update the commit message
>
> ---
> xen/arch/arm/device.c | 15 ++-
> xen/arch/arm/domain_build.c | 2 +-
> xen/arch/arm/gic-v2.c | 4 +-
> xen/arch/arm/gic-v3.c | 6 +-
> xen/arch/arm/gic.c | 4 +-
> xen/arch/arm/include/asm/Makefile | 1 +
> xen/arch/ppc/include/asm/Makefile | 1 +
> xen/arch/ppc/include/asm/device.h | 53 --------
> xen/arch/ppc/include/asm/irq.h | 4 +
> .../asm => include/asm-generic}/device.h | 125 +++++++++++-------
> xen/include/headers++.chk.new | 0
> 11 files changed, 106 insertions(+), 109 deletions(-)
> delete mode 100644 xen/arch/ppc/include/asm/device.h
> rename xen/{arch/arm/include/asm => include/asm-generic}/device.h (79%)
> create mode 100644 xen/include/headers++.chk.new
Stray new file, presumably because of a missing entry in .gitignore?
Overall I think there are too many changes done all in one go here.
But it's mostly Arm which is affected, so I'll leave judging about that
to the Arm maintainers.
> --- a/xen/arch/ppc/include/asm/irq.h
> +++ b/xen/arch/ppc/include/asm/irq.h
> @@ -3,7 +3,9 @@
> #define __ASM_PPC_IRQ_H__
>
> #include <xen/lib.h>
> +#ifdef CONFIG_HAS_DEVICE_TREE
> #include <xen/device_tree.h>
> +#endif
> #include <public/device_tree_defs.h>
Why would this #ifdef not cover the public header as well? (Otherwise I'd
be inclined to ask that the conditional be moved inside that header.)
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
2023-11-27 14:31 ` Jan Beulich
@ 2023-11-27 19:46 ` Oleksii
2023-11-28 7:51 ` Jan Beulich
2023-11-28 21:36 ` Shawn Anastasio
0 siblings, 2 replies; 57+ messages in thread
From: Oleksii @ 2023-11-27 19:46 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On Mon, 2023-11-27 at 15:31 +0100, Jan Beulich wrote:
> On 27.11.2023 15:13, Oleksii Kurochko wrote:
> > Arm, PPC and RISC-V use the same device.h thereby device.h
> > was moved to asm-generic. Arm's device.h was taken as a base with
> > the following changes:
> > - #ifdef PCI related things.
> > - #ifdef ACPI related things.
> > - Rename #ifdef guards.
> > - Add SPDX tag.
> > - #ifdef CONFIG_HAS_DEVICE_TREE related things.
> > - #ifdef-ing iommu related things with CONFIG_HAS_PASSTHROUGH.
> >
> > Also Arm and PPC are switched to asm-generic version of device.h
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > ---
> > Changes in V4:
> > - Updated the commit message
> > - Switched Arm and PPC to asm-generic version of device.h
> > - Replaced HAS_PCI with CONFIG_HAS_PCI
> > - ifdef-ing iommu filed of dev_archdata struct with
> > CONFIG_HAS_PASSTHROUGH
> > - ifdef-ing iommu_fwspec of device struct with
> > CONFIG_HAS_PASSTHROUGH
> > - ifdef-ing DT related things with CONFIG_HAS_DEVICE_TREE
> > - Updated the commit message ( remove a note with question about
> > if device.h should be in asm-generic or not )
> > - Replaced DEVICE_IC with DEVICE_INTERRUPT_CONTROLLER
> > - Rationalized usage of CONFIG_HAS_* in device.h
> > - Fixed indents for ACPI_DEVICE_START and ACPI_DEVICE_END
> > ---
> > Changes in V3:
> > - ifdef device tree related things.
> > - update the commit message
> > ---
> > Changes in V2:
> > - take ( as common ) device.h from Arm as PPC and RISC-V
> > use it as a base.
> > - #ifdef PCI related things.
> > - #ifdef ACPI related things.
> > - rename DEVICE_GIC to DEVIC_IC.
> > - rename #ifdef guards.
> > - switch Arm and PPC to generic device.h
> > - add SPDX tag
> > - update the commit message
> >
> > ---
> > xen/arch/arm/device.c | 15 ++-
> > xen/arch/arm/domain_build.c | 2 +-
> > xen/arch/arm/gic-v2.c | 4 +-
> > xen/arch/arm/gic-v3.c | 6 +-
> > xen/arch/arm/gic.c | 4 +-
> > xen/arch/arm/include/asm/Makefile | 1 +
> > xen/arch/ppc/include/asm/Makefile | 1 +
> > xen/arch/ppc/include/asm/device.h | 53 --------
> > xen/arch/ppc/include/asm/irq.h | 4 +
> > .../asm => include/asm-generic}/device.h | 125 +++++++++++---
> > ----
> > xen/include/headers++.chk.new | 0
> > 11 files changed, 106 insertions(+), 109 deletions(-)
> > delete mode 100644 xen/arch/ppc/include/asm/device.h
> > rename xen/{arch/arm/include/asm => include/asm-generic}/device.h
> > (79%)
> > create mode 100644 xen/include/headers++.chk.new
>
> Stray new file, presumably because of a missing entry in .gitignore?
Yeah, I don't have such entry in .gitignore.
I will remove this file in next version of the patch.
>
> Overall I think there are too many changes done all in one go here.
> But it's mostly Arm which is affected, so I'll leave judging about
> that
> to the Arm maintainers.
>
> > --- a/xen/arch/ppc/include/asm/irq.h
> > +++ b/xen/arch/ppc/include/asm/irq.h
> > @@ -3,7 +3,9 @@
> > #define __ASM_PPC_IRQ_H__
> >
> > #include <xen/lib.h>
> > +#ifdef CONFIG_HAS_DEVICE_TREE
> > #include <xen/device_tree.h>
> > +#endif
> > #include <public/device_tree_defs.h>
>
> Why would this #ifdef not cover the public header as well? (Otherwise
> I'd
> be inclined to ask that the conditional be moved inside that header.)
In that header is defined only consts without additional header
inclusion. At that moment it looked to me pretty save to ifdef only
xen/device_tree.h but you are right we can move incluion of the public
header inside #ifdef OR just remove as xen/device_tree.h already
includes it.
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
2023-11-27 19:46 ` Oleksii
@ 2023-11-28 7:51 ` Jan Beulich
2023-11-28 21:36 ` Shawn Anastasio
1 sibling, 0 replies; 57+ messages in thread
From: Jan Beulich @ 2023-11-28 7:51 UTC (permalink / raw)
To: Oleksii
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On 27.11.2023 20:46, Oleksii wrote:
> On Mon, 2023-11-27 at 15:31 +0100, Jan Beulich wrote:
>> On 27.11.2023 15:13, Oleksii Kurochko wrote:
>>> --- a/xen/arch/ppc/include/asm/irq.h
>>> +++ b/xen/arch/ppc/include/asm/irq.h
>>> @@ -3,7 +3,9 @@
>>> #define __ASM_PPC_IRQ_H__
>>>
>>> #include <xen/lib.h>
>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>> #include <xen/device_tree.h>
>>> +#endif
>>> #include <public/device_tree_defs.h>
>>
>> Why would this #ifdef not cover the public header as well? (Otherwise
>> I'd
>> be inclined to ask that the conditional be moved inside that header.)
> In that header is defined only consts without additional header
> inclusion. At that moment it looked to me pretty save to ifdef only
> xen/device_tree.h but you are right we can move incluion of the public
> header inside #ifdef OR just remove as xen/device_tree.h already
> includes it.
Oh, yes, dropping the redundant #include would be even better then. Yet
that furthers the desire to have the #ifdef inside that other header, to
improve how things look as use sites like the one here.
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
2023-11-27 19:46 ` Oleksii
2023-11-28 7:51 ` Jan Beulich
@ 2023-11-28 21:36 ` Shawn Anastasio
1 sibling, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-28 21:36 UTC (permalink / raw)
To: Oleksii, Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
xen-devel
Hi Oleksii,
On 11/27/23 1:46 PM, Oleksii wrote:
> On Mon, 2023-11-27 at 15:31 +0100, Jan Beulich wrote:
>> On 27.11.2023 15:13, Oleksii Kurochko wrote:
>>> --- a/xen/arch/ppc/include/asm/irq.h
>>> +++ b/xen/arch/ppc/include/asm/irq.h
>>> @@ -3,7 +3,9 @@
>>> #define __ASM_PPC_IRQ_H__
>>>
>>> #include <xen/lib.h>
>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>> #include <xen/device_tree.h>
>>> +#endif
>>> #include <public/device_tree_defs.h>
>>
>> Why would this #ifdef not cover the public header as well? (Otherwise
>> I'd
>> be inclined to ask that the conditional be moved inside that header.)
> In that header is defined only consts without additional header
> inclusion. At that moment it looked to me pretty save to ifdef only
> xen/device_tree.h but you are right we can move incluion of the public
> header inside #ifdef OR just remove as xen/device_tree.h already
> includes it.
>
Removing the include of <xen/device_tree.h> from ppc's asm/irq.h breaks
the build, because of platform_get_irq's usage of the `struct
dt_device_node` type:
CC common/cpu.o
In file included from ./include/xen/irq.h:80,
from ./include/xen/pci.h:13,
from ./include/xen/iommu.h:25,
from ./include/xen/sched.h:12,
from ./include/xen/event.h:12,
from common/cpu.c:3:
./arch/ppc/include/asm/irq.h:28:49: error: ‘struct dt_device_node’
declared inside parameter list will not be visible outside of this
definition or declaration [-Werror]
28 | static inline int platform_get_irq(const struct dt_device_node
*device, int index)
I wouldn't be opposed to a forward declaration of that type, but as I
indicated in my other reply to this patch, I'd like to avoid the
addition of these essentially always-dead CONFIG_HAS_DEVICE_TREE checks
to PPC code anyways, so dropping this hunk entirely from the patch might
be the way forward.
> ~ Oleksii
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
2023-11-27 14:13 ` [PATCH v4 02/14] xen/asm-generic: introduce generic device.h Oleksii Kurochko
2023-11-27 14:31 ` Jan Beulich
@ 2023-11-28 21:28 ` Shawn Anastasio
2023-11-29 8:16 ` Jan Beulich
2023-11-29 12:49 ` Oleksii
1 sibling, 2 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-28 21:28 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
> index ece7fa66dd..df4c1ebb08 100644
> --- a/xen/arch/ppc/include/asm/Makefile
> +++ b/xen/arch/ppc/include/asm/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +generic-y += device.h
> generic-y += paging.h
> generic-y += vm_event.h
> diff --git a/xen/arch/ppc/include/asm/device.h b/xen/arch/ppc/include/asm/device.h
> deleted file mode 100644
> index 8253e61d51..0000000000
> --- a/xen/arch/ppc/include/asm/device.h
> +++ /dev/null
> @@ -1,53 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -#ifndef __ASM_PPC_DEVICE_H__
> -#define __ASM_PPC_DEVICE_H__
> -
> -enum device_type
> -{
> - DEV_DT,
> - DEV_PCI,
> -};
> -
> -struct device {
> - enum device_type type;
> -#ifdef CONFIG_HAS_DEVICE_TREE
> - struct dt_device_node *of_node; /* Used by drivers imported from Linux */
> -#endif
> -};
> -
> -enum device_class
> -{
> - DEVICE_SERIAL,
> - DEVICE_IOMMU,
> - DEVICE_PCI_HOSTBRIDGE,
> - /* Use for error */
> - DEVICE_UNKNOWN,
> -};
> -
> -struct device_desc {
> - /* Device name */
> - const char *name;
> - /* Device class */
> - enum device_class class;
> - /* List of devices supported by this driver */
> - const struct dt_device_match *dt_match;
> - /*
> - * Device initialization.
> - *
> - * -EAGAIN is used to indicate that device probing is deferred.
> - */
> - int (*init)(struct dt_device_node *dev, const void *data);
> -};
> -
> -typedef struct device device_t;
> -
> -#define DT_DEVICE_START(name_, namestr_, class_) \
> -static const struct device_desc __dev_desc_##name_ __used \
> -__section(".dev.info") = { \
> - .name = namestr_, \
> - .class = class_, \
> -
> -#define DT_DEVICE_END \
> -};
> -
> -#endif /* __ASM_PPC_DEVICE_H__ */
> diff --git a/xen/arch/ppc/include/asm/irq.h b/xen/arch/ppc/include/asm/irq.h
> index 5c37d0cf25..49193fddff 100644
> --- a/xen/arch/ppc/include/asm/irq.h
> +++ b/xen/arch/ppc/include/asm/irq.h
> @@ -3,7 +3,9 @@
> #define __ASM_PPC_IRQ_H__
>
> #include <xen/lib.h>
> +#ifdef CONFIG_HAS_DEVICE_TREE
I realize that you were likely following PPC's device.h which also
checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes sense to
check this conditional in PPC code at all -- we will always have
HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario where
this will ever not be the case.
Unless Jan (or someone else) disagrees, I'd rather this conditional be
dropped inside of PPC code.
> #include <xen/device_tree.h>
> +#endif
> #include <public/device_tree_defs.h>
>
> /* TODO */
> @@ -25,9 +27,11 @@ static inline void arch_move_irqs(struct vcpu *v)
> BUG_ON("unimplemented");
> }
>
> +#ifdef CONFIG_HAS_DEVICE_TREE
Ditto.
> static inline int platform_get_irq(const struct dt_device_node *device, int index)
> {
> BUG_ON("unimplemented");
> }
> +#endif
>
> #endif /* __ASM_PPC_IRQ_H__ */
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
2023-11-28 21:28 ` Shawn Anastasio
@ 2023-11-29 8:16 ` Jan Beulich
2023-11-29 12:49 ` Oleksii
1 sibling, 0 replies; 57+ messages in thread
From: Jan Beulich @ 2023-11-29 8:16 UTC (permalink / raw)
To: Shawn Anastasio
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Oleksii Kurochko, xen-devel
On 28.11.2023 22:28, Shawn Anastasio wrote:
> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
>> --- a/xen/arch/ppc/include/asm/irq.h
>> +++ b/xen/arch/ppc/include/asm/irq.h
>> @@ -3,7 +3,9 @@
>> #define __ASM_PPC_IRQ_H__
>>
>> #include <xen/lib.h>
>> +#ifdef CONFIG_HAS_DEVICE_TREE
>
> I realize that you were likely following PPC's device.h which also
> checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes sense to
> check this conditional in PPC code at all -- we will always have
> HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario where
> this will ever not be the case.
>
> Unless Jan (or someone else) disagrees, I'd rather this conditional be
> dropped inside of PPC code.
No, pointless #ifdef are indeed better avoided.
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
2023-11-28 21:28 ` Shawn Anastasio
2023-11-29 8:16 ` Jan Beulich
@ 2023-11-29 12:49 ` Oleksii
2023-11-29 19:18 ` Shawn Anastasio
1 sibling, 1 reply; 57+ messages in thread
From: Oleksii @ 2023-11-29 12:49 UTC (permalink / raw)
To: Shawn Anastasio, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu
On Tue, 2023-11-28 at 15:28 -0600, Shawn Anastasio wrote:
> Hi Oleksii,
>
> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> > diff --git a/xen/arch/ppc/include/asm/Makefile
> > b/xen/arch/ppc/include/asm/Makefile
> > index ece7fa66dd..df4c1ebb08 100644
> > --- a/xen/arch/ppc/include/asm/Makefile
> > +++ b/xen/arch/ppc/include/asm/Makefile
> > @@ -1,3 +1,4 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > +generic-y += device.h
> > generic-y += paging.h
> > generic-y += vm_event.h
> > diff --git a/xen/arch/ppc/include/asm/device.h
> > b/xen/arch/ppc/include/asm/device.h
> > deleted file mode 100644
> > index 8253e61d51..0000000000
> > --- a/xen/arch/ppc/include/asm/device.h
> > +++ /dev/null
> > @@ -1,53 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0-only */
> > -#ifndef __ASM_PPC_DEVICE_H__
> > -#define __ASM_PPC_DEVICE_H__
> > -
> > -enum device_type
> > -{
> > - DEV_DT,
> > - DEV_PCI,
> > -};
> > -
> > -struct device {
> > - enum device_type type;
> > -#ifdef CONFIG_HAS_DEVICE_TREE
> > - struct dt_device_node *of_node; /* Used by drivers imported
> > from Linux */
> > -#endif
> > -};
> > -
> > -enum device_class
> > -{
> > - DEVICE_SERIAL,
> > - DEVICE_IOMMU,
> > - DEVICE_PCI_HOSTBRIDGE,
> > - /* Use for error */
> > - DEVICE_UNKNOWN,
> > -};
> > -
> > -struct device_desc {
> > - /* Device name */
> > - const char *name;
> > - /* Device class */
> > - enum device_class class;
> > - /* List of devices supported by this driver */
> > - const struct dt_device_match *dt_match;
> > - /*
> > - * Device initialization.
> > - *
> > - * -EAGAIN is used to indicate that device probing is
> > deferred.
> > - */
> > - int (*init)(struct dt_device_node *dev, const void *data);
> > -};
> > -
> > -typedef struct device device_t;
> > -
> > -#define DT_DEVICE_START(name_, namestr_,
> > class_) \
> > -static const struct device_desc __dev_desc_##name_
> > __used \
> > -__section(".dev.info") =
> > { \
> > - .name =
> > namestr_, \
> > - .class =
> > class_, \
> > -
> > -#define
> > DT_DEVICE_END \
> > -};
> > -
> > -#endif /* __ASM_PPC_DEVICE_H__ */
> > diff --git a/xen/arch/ppc/include/asm/irq.h
> > b/xen/arch/ppc/include/asm/irq.h
> > index 5c37d0cf25..49193fddff 100644
> > --- a/xen/arch/ppc/include/asm/irq.h
> > +++ b/xen/arch/ppc/include/asm/irq.h
> > @@ -3,7 +3,9 @@
> > #define __ASM_PPC_IRQ_H__
> >
> > #include <xen/lib.h>
> > +#ifdef CONFIG_HAS_DEVICE_TREE
>
> I realize that you were likely following PPC's device.h which also
> checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes sense
> to
> check this conditional in PPC code at all -- we will always have
> HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario
> where
> this will ever not be the case.
What about case if ACPI is used? Does ACPI is supported by PPC?
But if you are sure that CONFIG_HAS_DEVICE_TREE will be always selected
then I am OK to remove this change.
>
> Unless Jan (or someone else) disagrees, I'd rather this conditional
> be
> dropped inside of PPC code.
I'll double check but I think I had a compilation issue if it isn't
defined.
>
> > #include <xen/device_tree.h>
> > +#endif
> > #include <public/device_tree_defs.h>
> >
> > /* TODO */
> > @@ -25,9 +27,11 @@ static inline void arch_move_irqs(struct vcpu
> > *v)
> > BUG_ON("unimplemented");
> > }
> >
> > +#ifdef CONFIG_HAS_DEVICE_TREE
>
> Ditto.
>
> > static inline int platform_get_irq(const struct dt_device_node
> > *device, int index)
> > {
> > BUG_ON("unimplemented");
> > }
> > +#endif
> >
> > #endif /* __ASM_PPC_IRQ_H__ */
Thanks.
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
2023-11-29 12:49 ` Oleksii
@ 2023-11-29 19:18 ` Shawn Anastasio
2023-11-30 12:48 ` Oleksii
0 siblings, 1 reply; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-29 19:18 UTC (permalink / raw)
To: Oleksii, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu
On 11/29/23 6:49 AM, Oleksii wrote:
> On Tue, 2023-11-28 at 15:28 -0600, Shawn Anastasio wrote:
>> Hi Oleksii,
>>
>> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
>>> diff --git a/xen/arch/ppc/include/asm/irq.h
>>> b/xen/arch/ppc/include/asm/irq.h
>>> index 5c37d0cf25..49193fddff 100644
>>> --- a/xen/arch/ppc/include/asm/irq.h
>>> +++ b/xen/arch/ppc/include/asm/irq.h
>>> @@ -3,7 +3,9 @@
>>> #define __ASM_PPC_IRQ_H__
>>>
>>> #include <xen/lib.h>
>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>
>> I realize that you were likely following PPC's device.h which also
>> checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes sense
>> to
>> check this conditional in PPC code at all -- we will always have
>> HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario
>> where
>> this will ever not be the case.
> What about case if ACPI is used? Does ACPI is supported by PPC?
>
> But if you are sure that CONFIG_HAS_DEVICE_TREE will be always selected
> then I am OK to remove this change.
>
ACPI isn't supported by any PPC platform, we always use device tree.
>>
>> Unless Jan (or someone else) disagrees, I'd rather this conditional
>> be
>> dropped inside of PPC code.
> I'll double check but I think I had a compilation issue if it isn't
> defined.
>
I'm not encountering any issues locally with the conditional dropped,
but if you are able to reproduce them then let me know.
> Thanks.
>
> ~ Oleksii
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
2023-11-29 19:18 ` Shawn Anastasio
@ 2023-11-30 12:48 ` Oleksii
0 siblings, 0 replies; 57+ messages in thread
From: Oleksii @ 2023-11-30 12:48 UTC (permalink / raw)
To: Shawn Anastasio, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu
Hi Shawn,
On Wed, 2023-11-29 at 13:18 -0600, Shawn Anastasio wrote:
> On 11/29/23 6:49 AM, Oleksii wrote:
> > On Tue, 2023-11-28 at 15:28 -0600, Shawn Anastasio wrote:
> > > Hi Oleksii,
> > >
> > > On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> > > > diff --git a/xen/arch/ppc/include/asm/irq.h
> > > > b/xen/arch/ppc/include/asm/irq.h
> > > > index 5c37d0cf25..49193fddff 100644
> > > > --- a/xen/arch/ppc/include/asm/irq.h
> > > > +++ b/xen/arch/ppc/include/asm/irq.h
> > > > @@ -3,7 +3,9 @@
> > > > #define __ASM_PPC_IRQ_H__
> > > >
> > > > #include <xen/lib.h>
> > > > +#ifdef CONFIG_HAS_DEVICE_TREE
> > >
> > > I realize that you were likely following PPC's device.h which
> > > also
> > > checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes
> > > sense
> > > to
> > > check this conditional in PPC code at all -- we will always have
> > > HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario
> > > where
> > > this will ever not be the case.
> > What about case if ACPI is used? Does ACPI is supported by PPC?
> >
> > But if you are sure that CONFIG_HAS_DEVICE_TREE will be always
> > selected
> > then I am OK to remove this change.
> >
>
> ACPI isn't supported by any PPC platform, we always use device tree.
Thanks for clarification.
Then we can remove useless #ifdef. I'll do it it in next patch version.
>
> > >
> > > Unless Jan (or someone else) disagrees, I'd rather this
> > > conditional
> > > be
> > > dropped inside of PPC code.
> > I'll double check but I think I had a compilation issue if it isn't
> > defined.
> >
>
> I'm not encountering any issues locally with the conditional dropped,
> but if you are able to reproduce them then let me know.
Sure. I am going to do it tomorrow or today evening if I'll face an
issue will write you.
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 03/14] xen/asm-generic: introduce generic hypercall.h
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
2023-11-27 14:13 ` [PATCH v4 01/14] xen/asm-generic: introduce stub header paging.h Oleksii Kurochko
2023-11-27 14:13 ` [PATCH v4 02/14] xen/asm-generic: introduce generic device.h Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-28 21:47 ` Shawn Anastasio
2023-11-27 14:13 ` [PATCH v4 04/14] xen/asm-generic: introduce generic header iocap.h Oleksii Kurochko
` (11 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Shawn Anastasio, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
Introduce an empty generic hypercall.h for archs which don't
implement it.
Drop PPC's hypercall.h and switch to generic one instead.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in V4:
- Nothing changed. Only rebase.
---
Changes in V3:
- Drop PPC's hypercall.h and switch to generic one.
- Update the commit message
- Add Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in V2:
- add check that <asm/hypercall.h> isn't included directly.
---
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/hypercall.h | 5 -----
xen/include/asm-generic/hypercall.h | 18 ++++++++++++++++++
3 files changed, 19 insertions(+), 5 deletions(-)
delete mode 100644 xen/arch/ppc/include/asm/hypercall.h
create mode 100644 xen/include/asm-generic/hypercall.h
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index df4c1ebb08..d5e369128c 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
generic-y += device.h
+generic-y += hypercall.h
generic-y += paging.h
generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/hypercall.h b/xen/arch/ppc/include/asm/hypercall.h
deleted file mode 100644
index 1e8ca0ce9c..0000000000
--- a/xen/arch/ppc/include/asm/hypercall.h
+++ /dev/null
@@ -1,5 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_HYPERCALL_H__
-#define __ASM_PPC_HYPERCALL_H__
-
-#endif /* __ASM_PPC_HYPERCALL_H__ */
diff --git a/xen/include/asm-generic/hypercall.h b/xen/include/asm-generic/hypercall.h
new file mode 100644
index 0000000000..7743b35c0d
--- /dev/null
+++ b/xen/include/asm-generic/hypercall.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __XEN_HYPERCALL_H__
+#error "asm/hypercall.h should not be included directly - include xen/hypercall.h instead"
+#endif
+
+#ifndef __ASM_GENERIC_HYPERCALL_H__
+#define __ASM_GENERIC_HYPERCALL_H__
+
+#endif /* __ASM_GENERIC_HYPERCALL_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 03/14] xen/asm-generic: introduce generic hypercall.h
2023-11-27 14:13 ` [PATCH v4 03/14] xen/asm-generic: introduce generic hypercall.h Oleksii Kurochko
@ 2023-11-28 21:47 ` Shawn Anastasio
0 siblings, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-28 21:47 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall,
Stefano Stabellini, Wei Liu
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> Introduce an empty generic hypercall.h for archs which don't
> implement it.
>
> Drop PPC's hypercall.h and switch to generic one instead.
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 04/14] xen/asm-generic: introduce generic header iocap.h
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (2 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 03/14] xen/asm-generic: introduce generic hypercall.h Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-28 21:50 ` Shawn Anastasio
2023-11-27 14:13 ` [PATCH v4 05/14] xen/asm-generic: introduce stub header <asm/random.h> Oleksii Kurochko
` (10 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Jan Beulich, Wei Liu, Shawn Anastasio,
Julien Grall
iocap.h is common for Arm, PPC and RISC-V architectures thereby
it was moved to asm-generic.
Also Arm and PPC were switched to asm-generic version of iocap.h.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
Changes in V4:
- Added Reviewed-by: Jan Beulich <jbeulich@suse.com>
- Acked-by: Julien Grall <jgrall@amazon.com>
---
Changes in V3:
- Drop Arm and PPC's iocap.h and switch to asm-generic's version.
- Update the commit message.
---
Changes in V2:
- update the commit message
---
xen/arch/arm/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/iocap.h | 8 --------
xen/{arch/arm/include/asm => include/asm-generic}/iocap.h | 7 ++++---
4 files changed, 6 insertions(+), 11 deletions(-)
delete mode 100644 xen/arch/ppc/include/asm/iocap.h
rename xen/{arch/arm/include/asm => include/asm-generic}/iocap.h (60%)
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index df4c1ebb08..2d036045e8 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
generic-y += device.h
+generic-y += iocap.h
generic-y += paging.h
generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index d5e369128c..209c3e74d8 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
generic-y += device.h
generic-y += hypercall.h
+generic-y += iocap.h
generic-y += paging.h
generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/iocap.h b/xen/arch/ppc/include/asm/iocap.h
deleted file mode 100644
index 76bf13a70f..0000000000
--- a/xen/arch/ppc/include/asm/iocap.h
+++ /dev/null
@@ -1,8 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_IOCAP_H__
-#define __ASM_PPC_IOCAP_H__
-
-#define cache_flush_permitted(d) \
- (!rangeset_is_empty((d)->iomem_caps))
-
-#endif /* __ASM_PPC_IOCAP_H__ */
diff --git a/xen/arch/arm/include/asm/iocap.h b/xen/include/asm-generic/iocap.h
similarity index 60%
rename from xen/arch/arm/include/asm/iocap.h
rename to xen/include/asm-generic/iocap.h
index 276fefbc59..dd7cb45488 100644
--- a/xen/arch/arm/include/asm/iocap.h
+++ b/xen/include/asm-generic/iocap.h
@@ -1,10 +1,11 @@
-#ifndef __X86_IOCAP_H__
-#define __X86_IOCAP_H__
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_IOCAP_H__
+#define __ASM_GENERIC_IOCAP_H__
#define cache_flush_permitted(d) \
(!rangeset_is_empty((d)->iomem_caps))
-#endif
+#endif /* __ASM_GENERIC_IOCAP_H__ */
/*
* Local variables:
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 04/14] xen/asm-generic: introduce generic header iocap.h
2023-11-27 14:13 ` [PATCH v4 04/14] xen/asm-generic: introduce generic header iocap.h Oleksii Kurochko
@ 2023-11-28 21:50 ` Shawn Anastasio
0 siblings, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-28 21:50 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu, Julien Grall
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> iocap.h is common for Arm, PPC and RISC-V architectures thereby
> it was moved to asm-generic.
>
> Also Arm and PPC were switched to asm-generic version of iocap.h.
>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 05/14] xen/asm-generic: introduce stub header <asm/random.h>
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (3 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 04/14] xen/asm-generic: introduce generic header iocap.h Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-28 21:51 ` Shawn Anastasio
2023-11-27 14:13 ` [PATCH v4 06/14] xen/asm-generic: introduce generic header percpu.h Oleksii Kurochko
` (9 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Jan Beulich, Wei Liu, Shawn Anastasio,
Julien Grall
<asm/random.h> is common for Arm, PPC and RISC-V thereby it
is moved to asm-generic.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
Changes in V4:
- Added Reviewed-by: Jan Beulich <jbeulich@suse.com>
- Added Acked-by: Julien Grall <jgrall@amazon.com>
---
Changes in V3:
- Drop Arm and PPC's random.h and switch to asm-generic verison.
---
Changes in V2:
- update the commit messages
---
xen/arch/arm/include/asm/Makefile | 1 +
xen/arch/arm/include/asm/random.h | 9 ---------
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/random.h | 9 ---------
xen/include/asm-generic/random.h | 19 +++++++++++++++++++
5 files changed, 21 insertions(+), 18 deletions(-)
delete mode 100644 xen/arch/arm/include/asm/random.h
delete mode 100644 xen/arch/ppc/include/asm/random.h
create mode 100644 xen/include/asm-generic/random.h
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index 2d036045e8..7593c4e8ee 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -2,4 +2,5 @@
generic-y += device.h
generic-y += iocap.h
generic-y += paging.h
+generic-y += random.h
generic-y += vm_event.h
diff --git a/xen/arch/arm/include/asm/random.h b/xen/arch/arm/include/asm/random.h
deleted file mode 100644
index b4acee276b..0000000000
--- a/xen/arch/arm/include/asm/random.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef __ASM_RANDOM_H__
-#define __ASM_RANDOM_H__
-
-static inline unsigned int arch_get_random(void)
-{
- return 0;
-}
-
-#endif /* __ASM_RANDOM_H__ */
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index 209c3e74d8..039b0eb5ee 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -3,4 +3,5 @@ generic-y += device.h
generic-y += hypercall.h
generic-y += iocap.h
generic-y += paging.h
+generic-y += random.h
generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/random.h b/xen/arch/ppc/include/asm/random.h
deleted file mode 100644
index 2f9e9bbae4..0000000000
--- a/xen/arch/ppc/include/asm/random.h
+++ /dev/null
@@ -1,9 +0,0 @@
-#ifndef __ASM_PPC_RANDOM_H__
-#define __ASM_PPC_RANDOM_H__
-
-static inline unsigned int arch_get_random(void)
-{
- return 0;
-}
-
-#endif /* __ASM_PPC_RANDOM_H__ */
diff --git a/xen/include/asm-generic/random.h b/xen/include/asm-generic/random.h
new file mode 100644
index 0000000000..d0d35dd217
--- /dev/null
+++ b/xen/include/asm-generic/random.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_RANDOM_H__
+#define __ASM_GENERIC_RANDOM_H__
+
+static inline unsigned int arch_get_random(void)
+{
+ return 0;
+}
+
+#endif /* __ASM_GENERIC_RANDOM_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: BSD
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 05/14] xen/asm-generic: introduce stub header <asm/random.h>
2023-11-27 14:13 ` [PATCH v4 05/14] xen/asm-generic: introduce stub header <asm/random.h> Oleksii Kurochko
@ 2023-11-28 21:51 ` Shawn Anastasio
0 siblings, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-28 21:51 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu, Julien Grall
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> <asm/random.h> is common for Arm, PPC and RISC-V thereby it
> is moved to asm-generic.
>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 06/14] xen/asm-generic: introduce generic header percpu.h
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (4 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 05/14] xen/asm-generic: introduce stub header <asm/random.h> Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-27 14:33 ` Jan Beulich
2023-11-28 21:58 ` Shawn Anastasio
2023-11-27 14:13 ` [PATCH v4 07/14] xen/asm-generic: introduce generalized hardirq.h Oleksii Kurochko
` (8 subsequent siblings)
14 siblings, 2 replies; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Jan Beulich, Wei Liu, Shawn Anastasio,
Roger Pau Monné,
Julien Grall
The patch introduces generic percpu.h which was based on Arm's version
with the following changes:
* makes __per_cpu_data_end[] constant
* introduce get_per_cpu_offset() for macros this_cpu() and this_cpu_ptr()
* add inclustion of <asm/current.h> as get_per_cpu_offset() is located there.
Also it was changed a place where <asm/percpu.h> is included in <xen/percpu.h>
because asm-generic version of percpu.h started to include <asm/current.h> which
requires definition of DECLARE_PER_CPU.
As well the patch switches Arm, PPC and x86 architectures to use asm-generic
version of percpu.h.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
Changes in V4:
- Added FIXME comment in ppc/current.h for get_per_cpu_offset()
- Added Acked-by: Julien Grall <jgrall@amazon.com>.
---
Changes in V3:
- switch all architectures to asm-generic version of percpu.h
- introduce get_per_cpu_offset() and implement it architectures.
- make __per_cpu_data_end constamt.
- update the commit message.
---
Changes in V2:
- use smp_processor_id() instead of get_processor_id().
- update commit message .
---
xen/arch/arm/include/asm/Makefile | 1 +
xen/arch/arm/include/asm/current.h | 3 +++
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/current.h | 6 +++++
xen/arch/ppc/include/asm/percpu.h | 24 -------------------
xen/arch/x86/include/asm/Makefile | 2 ++
xen/arch/x86/include/asm/current.h | 2 ++
xen/arch/x86/include/asm/percpu.h | 22 -----------------
.../asm => include/asm-generic}/percpu.h | 18 ++++++++------
xen/include/xen/percpu.h | 4 ++--
10 files changed, 28 insertions(+), 55 deletions(-)
delete mode 100644 xen/arch/ppc/include/asm/percpu.h
create mode 100644 xen/arch/x86/include/asm/Makefile
delete mode 100644 xen/arch/x86/include/asm/percpu.h
rename xen/{arch/arm/include/asm => include/asm-generic}/percpu.h (57%)
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index 7593c4e8ee..60bb78fc09 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -2,5 +2,6 @@
generic-y += device.h
generic-y += iocap.h
generic-y += paging.h
+generic-y += percpu.h
generic-y += random.h
generic-y += vm_event.h
diff --git a/xen/arch/arm/include/asm/current.h b/xen/arch/arm/include/asm/current.h
index 51d1c8efa8..0be7ad6ef9 100644
--- a/xen/arch/arm/include/asm/current.h
+++ b/xen/arch/arm/include/asm/current.h
@@ -5,6 +5,7 @@
#include <xen/percpu.h>
#include <asm/processor.h>
+#include <asm/sysregs.h>
/* Tell whether the guest vCPU enabled Workaround 2 (i.e variant 4) */
#define CPUINFO_WORKAROUND_2_FLAG_SHIFT 0
@@ -60,6 +61,8 @@ do { \
this_cpu(cpu_id) = (id); \
} while ( 0 )
+#define get_per_cpu_offset() READ_SYSREG(TPIDR_EL2)
+
#endif
#endif /* __ARM_CURRENT_H__ */
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index 039b0eb5ee..edef0fab78 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -3,5 +3,6 @@ generic-y += device.h
generic-y += hypercall.h
generic-y += iocap.h
generic-y += paging.h
+generic-y += percpu.h
generic-y += random.h
generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/current.h b/xen/arch/ppc/include/asm/current.h
index 0ca06033f9..e329a548e6 100644
--- a/xen/arch/ppc/include/asm/current.h
+++ b/xen/arch/ppc/include/asm/current.h
@@ -4,6 +4,8 @@
#include <xen/percpu.h>
+#include <asm/processor.h>
+
#ifndef __ASSEMBLY__
struct vcpu;
@@ -38,6 +40,10 @@ static inline struct cpu_info *get_cpu_info(void)
#define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
+#define smp_processor_id() 0 /* TODO: Fix this */
+
+#define get_per_cpu_offset() smp_processor_id() /* TODO: Fix this */
+
#endif /* __ASSEMBLY__ */
#endif /* __ASM_PPC_CURRENT_H__ */
diff --git a/xen/arch/ppc/include/asm/percpu.h b/xen/arch/ppc/include/asm/percpu.h
deleted file mode 100644
index e7c40c0f03..0000000000
--- a/xen/arch/ppc/include/asm/percpu.h
+++ /dev/null
@@ -1,24 +0,0 @@
-#ifndef __PPC_PERCPU_H__
-#define __PPC_PERCPU_H__
-
-#ifndef __ASSEMBLY__
-
-extern char __per_cpu_start[], __per_cpu_data_end[];
-extern unsigned long __per_cpu_offset[NR_CPUS];
-void percpu_init_areas(void);
-
-#define smp_processor_id() 0 /* TODO: Fix this */
-
-#define per_cpu(var, cpu) \
- (*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset[cpu]))
-#define this_cpu(var) \
- (*RELOC_HIDE(&per_cpu__##var, smp_processor_id()))
-
-#define per_cpu_ptr(var, cpu) \
- (*RELOC_HIDE(var, __per_cpu_offset[cpu]))
-#define this_cpu_ptr(var) \
- (*RELOC_HIDE(var, smp_processor_id()))
-
-#endif
-
-#endif /* __PPC_PERCPU_H__ */
diff --git a/xen/arch/x86/include/asm/Makefile b/xen/arch/x86/include/asm/Makefile
new file mode 100644
index 0000000000..874429ed30
--- /dev/null
+++ b/xen/arch/x86/include/asm/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+generic-y += percpu.h
diff --git a/xen/arch/x86/include/asm/current.h b/xen/arch/x86/include/asm/current.h
index 35cca5cbe4..10950f36cc 100644
--- a/xen/arch/x86/include/asm/current.h
+++ b/xen/arch/x86/include/asm/current.h
@@ -102,6 +102,8 @@ static inline struct cpu_info *get_cpu_info(void)
#define smp_processor_id() (get_cpu_info()->processor_id)
#define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
+#define get_per_cpu_offset() (get_cpu_info()->per_cpu_offset)
+
/*
* Get the bottom-of-stack, as stored in the per-CPU TSS. This actually points
* into the middle of cpu_info.guest_cpu_user_regs, at the section that
diff --git a/xen/arch/x86/include/asm/percpu.h b/xen/arch/x86/include/asm/percpu.h
deleted file mode 100644
index 2b0c29a233..0000000000
--- a/xen/arch/x86/include/asm/percpu.h
+++ /dev/null
@@ -1,22 +0,0 @@
-#ifndef __X86_PERCPU_H__
-#define __X86_PERCPU_H__
-
-#ifndef __ASSEMBLY__
-extern char __per_cpu_start[], __per_cpu_data_end[];
-extern unsigned long __per_cpu_offset[NR_CPUS];
-void percpu_init_areas(void);
-#endif
-
-/* var is in discarded region: offset to particular copy we want */
-#define per_cpu(var, cpu) \
- (*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset[cpu]))
-#define this_cpu(var) \
- (*RELOC_HIDE(&per_cpu__##var, get_cpu_info()->per_cpu_offset))
-
-#define this_cpu_ptr(var) \
- (*RELOC_HIDE(var, get_cpu_info()->per_cpu_offset))
-
-#define per_cpu_ptr(var, cpu) \
- (*RELOC_HIDE(var, __per_cpu_offset[cpu]))
-
-#endif /* __X86_PERCPU_H__ */
diff --git a/xen/arch/arm/include/asm/percpu.h b/xen/include/asm-generic/percpu.h
similarity index 57%
rename from xen/arch/arm/include/asm/percpu.h
rename to xen/include/asm-generic/percpu.h
index f1a8768080..60af4f9ff9 100644
--- a/xen/arch/arm/include/asm/percpu.h
+++ b/xen/include/asm-generic/percpu.h
@@ -1,28 +1,32 @@
-#ifndef __ARM_PERCPU_H__
-#define __ARM_PERCPU_H__
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_PERCPU_H__
+#define __ASM_GENERIC_PERCPU_H__
#ifndef __ASSEMBLY__
#include <xen/types.h>
-#include <asm/sysregs.h>
+#include <asm/current.h>
-extern char __per_cpu_start[], __per_cpu_data_end[];
+extern char __per_cpu_start[];
+extern const char __per_cpu_data_end[];
extern unsigned long __per_cpu_offset[NR_CPUS];
void percpu_init_areas(void);
#define per_cpu(var, cpu) \
(*RELOC_HIDE(&per_cpu__##var, __per_cpu_offset[cpu]))
+
#define this_cpu(var) \
- (*RELOC_HIDE(&per_cpu__##var, READ_SYSREG(TPIDR_EL2)))
+ (*RELOC_HIDE(&per_cpu__##var, get_per_cpu_offset()))
#define per_cpu_ptr(var, cpu) \
(*RELOC_HIDE(var, __per_cpu_offset[cpu]))
#define this_cpu_ptr(var) \
- (*RELOC_HIDE(var, READ_SYSREG(TPIDR_EL2)))
+ (*RELOC_HIDE(var, get_per_cpu_offset()))
#endif
-#endif /* __ARM_PERCPU_H__ */
+#endif /* __ASM_GENERIC_PERCPU_H__ */
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/xen/percpu.h b/xen/include/xen/percpu.h
index c7bf57cbcd..57522f346b 100644
--- a/xen/include/xen/percpu.h
+++ b/xen/include/xen/percpu.h
@@ -1,8 +1,6 @@
#ifndef __XEN_PERCPU_H__
#define __XEN_PERCPU_H__
-#include <asm/percpu.h>
-
#define DECLARE_PER_CPU(type, name) \
extern __typeof__(type) per_cpu__ ## name
@@ -29,6 +27,8 @@
#define get_per_cpu_var(var) (per_cpu__##var)
+#include <asm/percpu.h>
+
/* Linux compatibility. */
#define get_cpu_var(var) this_cpu(var)
#define put_cpu_var(var)
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 06/14] xen/asm-generic: introduce generic header percpu.h
2023-11-27 14:13 ` [PATCH v4 06/14] xen/asm-generic: introduce generic header percpu.h Oleksii Kurochko
@ 2023-11-27 14:33 ` Jan Beulich
2023-11-28 21:58 ` Shawn Anastasio
1 sibling, 0 replies; 57+ messages in thread
From: Jan Beulich @ 2023-11-27 14:33 UTC (permalink / raw)
To: Oleksii Kurochko
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, Roger Pau Monné,
Julien Grall, xen-devel
On 27.11.2023 15:13, Oleksii Kurochko wrote:
> The patch introduces generic percpu.h which was based on Arm's version
> with the following changes:
> * makes __per_cpu_data_end[] constant
> * introduce get_per_cpu_offset() for macros this_cpu() and this_cpu_ptr()
> * add inclustion of <asm/current.h> as get_per_cpu_offset() is located there.
>
> Also it was changed a place where <asm/percpu.h> is included in <xen/percpu.h>
> because asm-generic version of percpu.h started to include <asm/current.h> which
> requires definition of DECLARE_PER_CPU.
>
> As well the patch switches Arm, PPC and x86 architectures to use asm-generic
> version of percpu.h.
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 06/14] xen/asm-generic: introduce generic header percpu.h
2023-11-27 14:13 ` [PATCH v4 06/14] xen/asm-generic: introduce generic header percpu.h Oleksii Kurochko
2023-11-27 14:33 ` Jan Beulich
@ 2023-11-28 21:58 ` Shawn Anastasio
1 sibling, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-28 21:58 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu, Roger Pau Monné,
Julien Grall
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> The patch introduces generic percpu.h which was based on Arm's version
> with the following changes:
> * makes __per_cpu_data_end[] constant
> * introduce get_per_cpu_offset() for macros this_cpu() and this_cpu_ptr()
> * add inclustion of <asm/current.h> as get_per_cpu_offset() is located there.
>
> Also it was changed a place where <asm/percpu.h> is included in <xen/percpu.h>
> because asm-generic version of percpu.h started to include <asm/current.h> which
> requires definition of DECLARE_PER_CPU.
>
> As well the patch switches Arm, PPC and x86 architectures to use asm-generic
> version of percpu.h.
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Acked-by: Julien Grall <jgrall@amazon.com>
> ---
> Changes in V4:
> - Added FIXME comment in ppc/current.h for get_per_cpu_offset()
Sorry I missed the discussion on this in v3, but yes, the PPC
implementation of this will need to be fixed (this was also the case
before your patch in PPC's current percpu.h but it was conspicuously
missing a TODO comment).
In any case,
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 07/14] xen/asm-generic: introduce generalized hardirq.h
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (5 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 06/14] xen/asm-generic: introduce generic header percpu.h Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-28 22:01 ` Shawn Anastasio
2023-11-27 14:13 ` [PATCH v4 08/14] xen/asm-generic: introduce generic div64.h header Oleksii Kurochko
` (7 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Jan Beulich, Wei Liu, Shawn Anastasio,
Julien Grall
<asm/hardirq.h> is common through archs thereby it is moved
to asm-generic.
Arm and PPC were switched to asm generic verstion of hardirq.h.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
Changes in V4:
- Added Reviewed-by: Jan Beulich <jbeulich@suse.com>.
- Added Acked-by: Julien Grall <jgrall@amazon.com>.
---
Changes in V3:
- Drop Arm and PPC's hardirq.h
- Update the commit message.
---
Changes in V2:
- add #include <xen/cache.h>.
- update the commit message
---
xen/arch/arm/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/hardirq.h | 19 -------------------
.../asm => include/asm-generic}/hardirq.h | 8 +++++---
4 files changed, 7 insertions(+), 22 deletions(-)
delete mode 100644 xen/arch/ppc/include/asm/hardirq.h
rename xen/{arch/arm/include/asm => include/asm-generic}/hardirq.h (79%)
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index 60bb78fc09..f6f108bbb8 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
generic-y += device.h
+generic-y += hardirq.h
generic-y += iocap.h
generic-y += paging.h
generic-y += percpu.h
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index edef0fab78..3241236c64 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
generic-y += device.h
+generic-y += hardirq.h
generic-y += hypercall.h
generic-y += iocap.h
generic-y += paging.h
diff --git a/xen/arch/ppc/include/asm/hardirq.h b/xen/arch/ppc/include/asm/hardirq.h
deleted file mode 100644
index 343efc7e69..0000000000
--- a/xen/arch/ppc/include/asm/hardirq.h
+++ /dev/null
@@ -1,19 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_HARDIRQ_H__
-#define __ASM_PPC_HARDIRQ_H__
-
-#include <xen/cache.h>
-
-typedef struct {
- unsigned long __softirq_pending;
- unsigned int __local_irq_count;
-} __cacheline_aligned irq_cpustat_t;
-
-#include <xen/irq_cpustat.h> /* Standard mappings for irq_cpustat_t above */
-
-#define in_irq() (local_irq_count(smp_processor_id()) != 0)
-
-#define irq_enter() (local_irq_count(smp_processor_id())++)
-#define irq_exit() (local_irq_count(smp_processor_id())--)
-
-#endif /* __ASM_PPC_HARDIRQ_H__ */
diff --git a/xen/arch/arm/include/asm/hardirq.h b/xen/include/asm-generic/hardirq.h
similarity index 79%
rename from xen/arch/arm/include/asm/hardirq.h
rename to xen/include/asm-generic/hardirq.h
index 67b6a673db..ddccf460b9 100644
--- a/xen/arch/arm/include/asm/hardirq.h
+++ b/xen/include/asm-generic/hardirq.h
@@ -1,5 +1,6 @@
-#ifndef __ASM_HARDIRQ_H
-#define __ASM_HARDIRQ_H
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_HARDIRQ_H
+#define __ASM_GENERIC_HARDIRQ_H
#include <xen/cache.h>
#include <xen/smp.h>
@@ -16,7 +17,8 @@ typedef struct {
#define irq_enter() (local_irq_count(smp_processor_id())++)
#define irq_exit() (local_irq_count(smp_processor_id())--)
-#endif /* __ASM_HARDIRQ_H */
+#endif /* __ASM_GENERIC_HARDIRQ_H */
+
/*
* Local variables:
* mode: C
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 07/14] xen/asm-generic: introduce generalized hardirq.h
2023-11-27 14:13 ` [PATCH v4 07/14] xen/asm-generic: introduce generalized hardirq.h Oleksii Kurochko
@ 2023-11-28 22:01 ` Shawn Anastasio
0 siblings, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-28 22:01 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu, Julien Grall
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> <asm/hardirq.h> is common through archs thereby it is moved
> to asm-generic.
>
> Arm and PPC were switched to asm generic verstion of hardirq.h.
>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 08/14] xen/asm-generic: introduce generic div64.h header
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (6 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 07/14] xen/asm-generic: introduce generalized hardirq.h Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-28 22:07 ` Shawn Anastasio
2023-11-27 14:13 ` [PATCH v4 09/14] xen/asm-generic: introduce generic header altp2m.h Oleksii Kurochko
` (6 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Jan Beulich, Wei Liu, Shawn Anastasio,
Roger Pau Monné
All archs have the do_div implementation for BITS_PER_LONG == 64
so do_div64.h is moved to asm-generic.
x86 and PPC were switched to asm-generic version of div64.h.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in V4:
- Added Acked-by: Jan Beulich <jbeulich@suse.com>.
- include <asm-generic/div64.h> in Arm's div64.h for 64-bit case.
---
Changes in V3:
- Drop x86 and PPC's div64.h.
- Update the commit message.
---
Changes in V2:
- rename base to divisor
- add "#if BITS_PER_LONG == 64"
- fix code style
---
xen/arch/arm/include/asm/div64.h | 8 +-------
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/div64.h | 14 --------------
xen/arch/x86/include/asm/Makefile | 1 +
xen/arch/x86/include/asm/div64.h | 14 --------------
xen/include/asm-generic/div64.h | 27 +++++++++++++++++++++++++++
6 files changed, 30 insertions(+), 35 deletions(-)
delete mode 100644 xen/arch/ppc/include/asm/div64.h
delete mode 100644 xen/arch/x86/include/asm/div64.h
create mode 100644 xen/include/asm-generic/div64.h
diff --git a/xen/arch/arm/include/asm/div64.h b/xen/arch/arm/include/asm/div64.h
index fc667a80f9..0459d5cc01 100644
--- a/xen/arch/arm/include/asm/div64.h
+++ b/xen/arch/arm/include/asm/div64.h
@@ -24,13 +24,7 @@
#if BITS_PER_LONG == 64
-# define do_div(n,base) ({ \
- uint32_t __base = (base); \
- uint32_t __rem; \
- __rem = ((uint64_t)(n)) % __base; \
- (n) = ((uint64_t)(n)) / __base; \
- __rem; \
- })
+#include <asm-generic/div64.h>
#elif BITS_PER_LONG == 32
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index 3241236c64..5364bb1d59 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
generic-y += device.h
+generic-y += div64.h
generic-y += hardirq.h
generic-y += hypercall.h
generic-y += iocap.h
diff --git a/xen/arch/ppc/include/asm/div64.h b/xen/arch/ppc/include/asm/div64.h
deleted file mode 100644
index d213e50585..0000000000
--- a/xen/arch/ppc/include/asm/div64.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_DIV64_H__
-#define __ASM_PPC_DIV64_H__
-
-#include <xen/types.h>
-
-#define do_div(n, base) ({ \
- uint32_t base_ = (base); \
- uint32_t rem_ = (uint64_t)(n) % base_; \
- (n) = (uint64_t)(n) / base_; \
- rem_; \
-})
-
-#endif /* __ASM_PPC_DIV64_H__ */
diff --git a/xen/arch/x86/include/asm/Makefile b/xen/arch/x86/include/asm/Makefile
index 874429ed30..daab34ff0a 100644
--- a/xen/arch/x86/include/asm/Makefile
+++ b/xen/arch/x86/include/asm/Makefile
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
+generic-y += div64.h
generic-y += percpu.h
diff --git a/xen/arch/x86/include/asm/div64.h b/xen/arch/x86/include/asm/div64.h
deleted file mode 100644
index dd49f64a3b..0000000000
--- a/xen/arch/x86/include/asm/div64.h
+++ /dev/null
@@ -1,14 +0,0 @@
-#ifndef __X86_DIV64
-#define __X86_DIV64
-
-#include <xen/types.h>
-
-#define do_div(n,base) ({ \
- uint32_t __base = (base); \
- uint32_t __rem; \
- __rem = ((uint64_t)(n)) % __base; \
- (n) = ((uint64_t)(n)) / __base; \
- __rem; \
-})
-
-#endif
diff --git a/xen/include/asm-generic/div64.h b/xen/include/asm-generic/div64.h
new file mode 100644
index 0000000000..068d8a11ad
--- /dev/null
+++ b/xen/include/asm-generic/div64.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_DIV64
+#define __ASM_GENERIC_DIV64
+
+#include <xen/types.h>
+
+#if BITS_PER_LONG == 64
+
+#define do_div(n, divisor) ({ \
+ uint32_t divisor_ = (divisor); \
+ uint32_t rem_ = (uint64_t)(n) % divisor_; \
+ (n) = (uint64_t)(n) / divisor_; \
+ rem_; \
+})
+
+#endif /* BITS_PER_LONG */
+
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 08/14] xen/asm-generic: introduce generic div64.h header
2023-11-27 14:13 ` [PATCH v4 08/14] xen/asm-generic: introduce generic div64.h header Oleksii Kurochko
@ 2023-11-28 22:07 ` Shawn Anastasio
0 siblings, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-28 22:07 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu, Roger Pau Monné
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> All archs have the do_div implementation for BITS_PER_LONG == 64
> so do_div64.h is moved to asm-generic.
>
> x86 and PPC were switched to asm-generic version of div64.h.
>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 09/14] xen/asm-generic: introduce generic header altp2m.h
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (7 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 08/14] xen/asm-generic: introduce generic div64.h header Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-28 22:09 ` Shawn Anastasio
2023-11-27 14:13 ` [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h Oleksii Kurochko
` (5 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Jan Beulich, Wei Liu, Shawn Anastasio
<asm/alt2pm.h> is common between several archs so it is
moved to asm-generic.
Arm and PPC were switched to asm-generic version of altp2m.h.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Michal Orzel <michal.orzel@amd.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Michal asked the following:
Shouldn't this copyright be moved to generic header as well?
It seems to me that it is fine to leave the stub altp2m header
without the copyright "Copyright (c) 2014, Intel Corporation.".
If an absense of the copyright violates something I will be
happy to add it during merge.
---
Changes in V4:
- Update the commit message
- Added Acked-by: Michal Orzel <michal.orzel@amd.com>
- Added Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in V3:
- Drop Arm and PPC's altp2m.h
- Update the commit message.
---
Changes in V2:
- change uint16_t to unsigned int in declaration of altp2m_vcpu_idx
- update the commit message
---
xen/arch/arm/include/asm/Makefile | 1 +
xen/arch/arm/include/asm/altp2m.h | 39 -------------------------------
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/altp2m.h | 25 --------------------
xen/include/asm-generic/altp2m.h | 34 +++++++++++++++++++++++++++
5 files changed, 36 insertions(+), 64 deletions(-)
delete mode 100644 xen/arch/arm/include/asm/altp2m.h
delete mode 100644 xen/arch/ppc/include/asm/altp2m.h
create mode 100644 xen/include/asm-generic/altp2m.h
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index f6f108bbb8..55d153ed82 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
+generic-y += altp2m.h
generic-y += device.h
generic-y += hardirq.h
generic-y += iocap.h
diff --git a/xen/arch/arm/include/asm/altp2m.h b/xen/arch/arm/include/asm/altp2m.h
deleted file mode 100644
index df50cb2f09..0000000000
--- a/xen/arch/arm/include/asm/altp2m.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Alternate p2m
- *
- * Copyright (c) 2014, Intel Corporation.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef __ASM_ARM_ALTP2M_H
-#define __ASM_ARM_ALTP2M_H
-
-#include <xen/sched.h>
-
-/* Alternate p2m on/off per domain */
-static inline bool altp2m_active(const struct domain *d)
-{
- /* Not implemented on ARM. */
- return false;
-}
-
-/* Alternate p2m VCPU */
-static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
-{
- /* Not implemented on ARM, should not be reached. */
- BUG();
- return 0;
-}
-
-#endif /* __ASM_ARM_ALTP2M_H */
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index 5364bb1d59..319e90955b 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
+generic-y += altp2m.h
generic-y += device.h
generic-y += div64.h
generic-y += hardirq.h
diff --git a/xen/arch/ppc/include/asm/altp2m.h b/xen/arch/ppc/include/asm/altp2m.h
deleted file mode 100644
index bd5c9aa415..0000000000
--- a/xen/arch/ppc/include/asm/altp2m.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_ALTP2M_H__
-#define __ASM_PPC_ALTP2M_H__
-
-#include <xen/bug.h>
-
-struct domain;
-struct vcpu;
-
-/* Alternate p2m on/off per domain */
-static inline bool altp2m_active(const struct domain *d)
-{
- /* Not implemented on PPC. */
- return false;
-}
-
-/* Alternate p2m VCPU */
-static inline uint16_t altp2m_vcpu_idx(const struct vcpu *v)
-{
- /* Not implemented on PPC, should not be reached. */
- ASSERT_UNREACHABLE();
- return 0;
-}
-
-#endif /* __ASM_PPC_ALTP2M_H__ */
diff --git a/xen/include/asm-generic/altp2m.h b/xen/include/asm-generic/altp2m.h
new file mode 100644
index 0000000000..39865a842a
--- /dev/null
+++ b/xen/include/asm-generic/altp2m.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_ALTP2M_H
+#define __ASM_GENERIC_ALTP2M_H
+
+#include <xen/bug.h>
+
+struct domain;
+struct vcpu;
+
+/* Alternate p2m on/off per domain */
+static inline bool altp2m_active(const struct domain *d)
+{
+ /* Not implemented on GENERIC. */
+ return false;
+}
+
+/* Alternate p2m VCPU */
+static inline unsigned int altp2m_vcpu_idx(const struct vcpu *v)
+{
+ /* Not implemented on GENERIC, should not be reached. */
+ BUG();
+ return 0;
+}
+
+#endif /* __ASM_GENERIC_ALTP2M_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: BSD
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 09/14] xen/asm-generic: introduce generic header altp2m.h
2023-11-27 14:13 ` [PATCH v4 09/14] xen/asm-generic: introduce generic header altp2m.h Oleksii Kurochko
@ 2023-11-28 22:09 ` Shawn Anastasio
0 siblings, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-28 22:09 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> <asm/alt2pm.h> is common between several archs so it is
> moved to asm-generic.
>
> Arm and PPC were switched to asm-generic version of altp2m.h.
>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (8 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 09/14] xen/asm-generic: introduce generic header altp2m.h Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-28 22:21 ` Shawn Anastasio
2023-11-27 14:13 ` [PATCH v4 11/14] xen/asm-generic: introduce stub header numa.h Oleksii Kurochko
` (4 subsequent siblings)
14 siblings, 1 reply; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Shawn Anastasio, Tamas K Lengyel,
Alexandru Isaila, Petre Pircalabu, Jan Beulich
The header is shared between several archs so it is
moved to asm-generic.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>.
---
Changes in V4:
- Removed the double blank line.
- Added Acked-by: Jan Beulich <jbeulich@suse.com>.
- Update the commit message
---
Changes in V3:
- Use forward-declaration of struct domain instead of " #include <xen/sched.h> ".
- Add ' include <xen/errno.h> '
- Drop PPC's monitor.h.
---
Changes in V2:
- remove inclusion of "+#include <public/domctl.h>"
- add "struct xen_domctl_monitor_op;"
- remove one of SPDX tags.
---
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/monitor.h | 43 ---------------------
xen/include/asm-generic/monitor.h | 62 ++++++++++++++++++++++++++++++
3 files changed, 63 insertions(+), 43 deletions(-)
delete mode 100644 xen/arch/ppc/include/asm/monitor.h
create mode 100644 xen/include/asm-generic/monitor.h
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index 319e90955b..bcddcc181a 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -5,6 +5,7 @@ generic-y += div64.h
generic-y += hardirq.h
generic-y += hypercall.h
generic-y += iocap.h
+generic-y += monitor.h
generic-y += paging.h
generic-y += percpu.h
generic-y += random.h
diff --git a/xen/arch/ppc/include/asm/monitor.h b/xen/arch/ppc/include/asm/monitor.h
deleted file mode 100644
index e5b0282bf1..0000000000
--- a/xen/arch/ppc/include/asm/monitor.h
+++ /dev/null
@@ -1,43 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/* Derived from xen/arch/arm/include/asm/monitor.h */
-#ifndef __ASM_PPC_MONITOR_H__
-#define __ASM_PPC_MONITOR_H__
-
-#include <public/domctl.h>
-#include <xen/errno.h>
-
-static inline
-void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
-{
-}
-
-static inline
-int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
-{
- /* No arch-specific monitor ops on PPC. */
- return -EOPNOTSUPP;
-}
-
-int arch_monitor_domctl_event(struct domain *d,
- struct xen_domctl_monitor_op *mop);
-
-static inline
-int arch_monitor_init_domain(struct domain *d)
-{
- /* No arch-specific domain initialization on PPC. */
- return 0;
-}
-
-static inline
-void arch_monitor_cleanup_domain(struct domain *d)
-{
- /* No arch-specific domain cleanup on PPC. */
-}
-
-static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
-{
- BUG_ON("unimplemented");
- return 0;
-}
-
-#endif /* __ASM_PPC_MONITOR_H__ */
diff --git a/xen/include/asm-generic/monitor.h b/xen/include/asm-generic/monitor.h
new file mode 100644
index 0000000000..6be8614431
--- /dev/null
+++ b/xen/include/asm-generic/monitor.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * include/asm-GENERIC/monitor.h
+ *
+ * Arch-specific monitor_op domctl handler.
+ *
+ * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
+ * Copyright (c) 2016, Bitdefender S.R.L.
+ *
+ */
+
+#ifndef __ASM_GENERIC_MONITOR_H__
+#define __ASM_GENERIC_MONITOR_H__
+
+#include <xen/errno.h>
+
+struct domain;
+struct xen_domctl_monitor_op;
+
+static inline
+void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
+{
+}
+
+static inline
+int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
+{
+ /* No arch-specific monitor ops on GENERIC. */
+ return -EOPNOTSUPP;
+}
+
+int arch_monitor_domctl_event(struct domain *d,
+ struct xen_domctl_monitor_op *mop);
+
+static inline
+int arch_monitor_init_domain(struct domain *d)
+{
+ /* No arch-specific domain initialization on GENERIC. */
+ return 0;
+}
+
+static inline
+void arch_monitor_cleanup_domain(struct domain *d)
+{
+ /* No arch-specific domain cleanup on GENERIC. */
+}
+
+static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
+{
+ return 0;
+}
+
+#endif /* __ASM_GENERIC_MONITOR_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: BSD
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h
2023-11-27 14:13 ` [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h Oleksii Kurochko
@ 2023-11-28 22:21 ` Shawn Anastasio
2023-11-29 8:19 ` Jan Beulich
2023-11-29 12:39 ` Oleksii
0 siblings, 2 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-28 22:21 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu, Jan Beulich
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> The header is shared between several archs so it is
> moved to asm-generic.
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>.
> ---
> Changes in V4:
> - Removed the double blank line.
> - Added Acked-by: Jan Beulich <jbeulich@suse.com>.
> - Update the commit message
> ---
> Changes in V3:
> - Use forward-declaration of struct domain instead of " #include <xen/sched.h> ".
> - Add ' include <xen/errno.h> '
> - Drop PPC's monitor.h.
> ---
> Changes in V2:
> - remove inclusion of "+#include <public/domctl.h>"
> - add "struct xen_domctl_monitor_op;"
> - remove one of SPDX tags.
> ---
> xen/arch/ppc/include/asm/Makefile | 1 +
> xen/arch/ppc/include/asm/monitor.h | 43 ---------------------
> xen/include/asm-generic/monitor.h | 62 ++++++++++++++++++++++++++++++
> 3 files changed, 63 insertions(+), 43 deletions(-)
> delete mode 100644 xen/arch/ppc/include/asm/monitor.h
> create mode 100644 xen/include/asm-generic/monitor.h
>
> diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
> index 319e90955b..bcddcc181a 100644
> --- a/xen/arch/ppc/include/asm/Makefile
> +++ b/xen/arch/ppc/include/asm/Makefile
> @@ -5,6 +5,7 @@ generic-y += div64.h
> generic-y += hardirq.h
> generic-y += hypercall.h
> generic-y += iocap.h
> +generic-y += monitor.h
> generic-y += paging.h
> generic-y += percpu.h
> generic-y += random.h
> diff --git a/xen/arch/ppc/include/asm/monitor.h b/xen/arch/ppc/include/asm/monitor.h
> deleted file mode 100644
> index e5b0282bf1..0000000000
> --- a/xen/arch/ppc/include/asm/monitor.h
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/* Derived from xen/arch/arm/include/asm/monitor.h */
> -#ifndef __ASM_PPC_MONITOR_H__
> -#define __ASM_PPC_MONITOR_H__
> -
> -#include <public/domctl.h>
> -#include <xen/errno.h>
> -
> -static inline
> -void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
> -{
> -}
> -
> -static inline
> -int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
> -{
> - /* No arch-specific monitor ops on PPC. */
> - return -EOPNOTSUPP;
> -}
> -
> -int arch_monitor_domctl_event(struct domain *d,
> - struct xen_domctl_monitor_op *mop);
> -
> -static inline
> -int arch_monitor_init_domain(struct domain *d)
> -{
> - /* No arch-specific domain initialization on PPC. */
> - return 0;
> -}
> -
> -static inline
> -void arch_monitor_cleanup_domain(struct domain *d)
> -{
> - /* No arch-specific domain cleanup on PPC. */
> -}
> -
> -static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
> -{
> - BUG_ON("unimplemented");
I'm not sure how I feel about this assertion being dropped in the
generic header. In general my philosophy when creating these stub
headers was to insert as many of these assertions as possible so we
don't end up in a scenario where during early bringup I miss a function
that was originally stubbed but ought to be implemented eventually.
Looking at ARM's monitor.h too, it seems that this function is the only
one that differs from the generic/stub version. I'm wondering if it
would make sense to leave this function out of the generic header, to be
defined by the arch similar to what you've done with some other
functions in this series. That would also allow ARM to be brought in to
using the generic variant.
> - return 0;
> -}
> -
> -#endif /* __ASM_PPC_MONITOR_H__ */
> diff --git a/xen/include/asm-generic/monitor.h b/xen/include/asm-generic/monitor.h
> new file mode 100644
> index 0000000000..6be8614431
> --- /dev/null
> +++ b/xen/include/asm-generic/monitor.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * include/asm-GENERIC/monitor.h
> + *
> + * Arch-specific monitor_op domctl handler.
> + *
> + * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
> + * Copyright (c) 2016, Bitdefender S.R.L.
> + *
> + */
> +
> +#ifndef __ASM_GENERIC_MONITOR_H__
> +#define __ASM_GENERIC_MONITOR_H__
> +
> +#include <xen/errno.h>
> +
> +struct domain;
> +struct xen_domctl_monitor_op;
> +
> +static inline
> +void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
> +{
> +}
> +
> +static inline
> +int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
> +{
> + /* No arch-specific monitor ops on GENERIC. */
> + return -EOPNOTSUPP;
> +}
> +
> +int arch_monitor_domctl_event(struct domain *d,
> + struct xen_domctl_monitor_op *mop);
> +
> +static inline
> +int arch_monitor_init_domain(struct domain *d)
> +{
> + /* No arch-specific domain initialization on GENERIC. */
> + return 0;
> +}
> +
> +static inline
> +void arch_monitor_cleanup_domain(struct domain *d)
> +{
> + /* No arch-specific domain cleanup on GENERIC. */
> +}
> +
> +static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
> +{
See previous comment.
> + return 0;
> +}
> +
> +#endif /* __ASM_GENERIC_MONITOR_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: BSD
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h
2023-11-28 22:21 ` Shawn Anastasio
@ 2023-11-29 8:19 ` Jan Beulich
2023-11-29 8:21 ` Jan Beulich
2023-11-29 12:39 ` Oleksii
1 sibling, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2023-11-29 8:19 UTC (permalink / raw)
To: Shawn Anastasio
Cc: Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu,
Oleksii Kurochko, xen-devel
On 28.11.2023 23:21, Shawn Anastasio wrote:
> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
>> --- a/xen/arch/ppc/include/asm/monitor.h
>> +++ /dev/null
>> @@ -1,43 +0,0 @@
>> -/* SPDX-License-Identifier: GPL-2.0-only */
>> -/* Derived from xen/arch/arm/include/asm/monitor.h */
>> -#ifndef __ASM_PPC_MONITOR_H__
>> -#define __ASM_PPC_MONITOR_H__
>> -
>> -#include <public/domctl.h>
>> -#include <xen/errno.h>
>> -
>> -static inline
>> -void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
>> -{
>> -}
>> -
>> -static inline
>> -int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
>> -{
>> - /* No arch-specific monitor ops on PPC. */
>> - return -EOPNOTSUPP;
>> -}
>> -
>> -int arch_monitor_domctl_event(struct domain *d,
>> - struct xen_domctl_monitor_op *mop);
>> -
>> -static inline
>> -int arch_monitor_init_domain(struct domain *d)
>> -{
>> - /* No arch-specific domain initialization on PPC. */
>> - return 0;
>> -}
>> -
>> -static inline
>> -void arch_monitor_cleanup_domain(struct domain *d)
>> -{
>> - /* No arch-specific domain cleanup on PPC. */
>> -}
>> -
>> -static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>> -{
>> - BUG_ON("unimplemented");
>
> I'm not sure how I feel about this assertion being dropped in the
> generic header. In general my philosophy when creating these stub
> headers was to insert as many of these assertions as possible so we
> don't end up in a scenario where during early bringup I miss a function
> that was originally stubbed but ought to be implemented eventually.
>
> Looking at ARM's monitor.h too, it seems that this function is the only
> one that differs from the generic/stub version. I'm wondering if it
> would make sense to leave this function out of the generic header, to be
> defined by the arch similar to what you've done with some other
> functions in this series. That would also allow ARM to be brought in to
> using the generic variant.
Yet then where would that function live, if not in arch/*/include/asm/monitor.h?
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h
2023-11-29 8:19 ` Jan Beulich
@ 2023-11-29 8:21 ` Jan Beulich
2023-11-29 12:41 ` Oleksii
0 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2023-11-29 8:21 UTC (permalink / raw)
To: Shawn Anastasio
Cc: Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu,
Oleksii Kurochko, xen-devel
On 29.11.2023 09:19, Jan Beulich wrote:
> On 28.11.2023 23:21, Shawn Anastasio wrote:
>> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
>>> --- a/xen/arch/ppc/include/asm/monitor.h
>>> +++ /dev/null
>>> @@ -1,43 +0,0 @@
>>> -/* SPDX-License-Identifier: GPL-2.0-only */
>>> -/* Derived from xen/arch/arm/include/asm/monitor.h */
>>> -#ifndef __ASM_PPC_MONITOR_H__
>>> -#define __ASM_PPC_MONITOR_H__
>>> -
>>> -#include <public/domctl.h>
>>> -#include <xen/errno.h>
>>> -
>>> -static inline
>>> -void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
>>> -{
>>> -}
>>> -
>>> -static inline
>>> -int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
>>> -{
>>> - /* No arch-specific monitor ops on PPC. */
>>> - return -EOPNOTSUPP;
>>> -}
>>> -
>>> -int arch_monitor_domctl_event(struct domain *d,
>>> - struct xen_domctl_monitor_op *mop);
>>> -
>>> -static inline
>>> -int arch_monitor_init_domain(struct domain *d)
>>> -{
>>> - /* No arch-specific domain initialization on PPC. */
>>> - return 0;
>>> -}
>>> -
>>> -static inline
>>> -void arch_monitor_cleanup_domain(struct domain *d)
>>> -{
>>> - /* No arch-specific domain cleanup on PPC. */
>>> -}
>>> -
>>> -static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>>> -{
>>> - BUG_ON("unimplemented");
>>
>> I'm not sure how I feel about this assertion being dropped in the
>> generic header. In general my philosophy when creating these stub
>> headers was to insert as many of these assertions as possible so we
>> don't end up in a scenario where during early bringup I miss a function
>> that was originally stubbed but ought to be implemented eventually.
>>
>> Looking at ARM's monitor.h too, it seems that this function is the only
>> one that differs from the generic/stub version. I'm wondering if it
>> would make sense to leave this function out of the generic header, to be
>> defined by the arch similar to what you've done with some other
>> functions in this series. That would also allow ARM to be brought in to
>> using the generic variant.
>
> Yet then where would that function live, if not in arch/*/include/asm/monitor.h?
Hmm, maybe implicitly you're proposing that arch/*/include/asm/monitor.h
include include/asm-generic/monitor.h in such a case, and define this one
function on top?
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h
2023-11-29 8:21 ` Jan Beulich
@ 2023-11-29 12:41 ` Oleksii
0 siblings, 0 replies; 57+ messages in thread
From: Oleksii @ 2023-11-29 12:41 UTC (permalink / raw)
To: Jan Beulich, Shawn Anastasio
Cc: Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu, xen-devel
On Wed, 2023-11-29 at 09:21 +0100, Jan Beulich wrote:
> On 29.11.2023 09:19, Jan Beulich wrote:
> > On 28.11.2023 23:21, Shawn Anastasio wrote:
> > > On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> > > > --- a/xen/arch/ppc/include/asm/monitor.h
> > > > +++ /dev/null
> > > > @@ -1,43 +0,0 @@
> > > > -/* SPDX-License-Identifier: GPL-2.0-only */
> > > > -/* Derived from xen/arch/arm/include/asm/monitor.h */
> > > > -#ifndef __ASM_PPC_MONITOR_H__
> > > > -#define __ASM_PPC_MONITOR_H__
> > > > -
> > > > -#include <public/domctl.h>
> > > > -#include <xen/errno.h>
> > > > -
> > > > -static inline
> > > > -void arch_monitor_allow_userspace(struct domain *d, bool
> > > > allow_userspace)
> > > > -{
> > > > -}
> > > > -
> > > > -static inline
> > > > -int arch_monitor_domctl_op(struct domain *d, struct
> > > > xen_domctl_monitor_op *mop)
> > > > -{
> > > > - /* No arch-specific monitor ops on PPC. */
> > > > - return -EOPNOTSUPP;
> > > > -}
> > > > -
> > > > -int arch_monitor_domctl_event(struct domain *d,
> > > > - struct xen_domctl_monitor_op
> > > > *mop);
> > > > -
> > > > -static inline
> > > > -int arch_monitor_init_domain(struct domain *d)
> > > > -{
> > > > - /* No arch-specific domain initialization on PPC. */
> > > > - return 0;
> > > > -}
> > > > -
> > > > -static inline
> > > > -void arch_monitor_cleanup_domain(struct domain *d)
> > > > -{
> > > > - /* No arch-specific domain cleanup on PPC. */
> > > > -}
> > > > -
> > > > -static inline uint32_t arch_monitor_get_capabilities(struct
> > > > domain *d)
> > > > -{
> > > > - BUG_ON("unimplemented");
> > >
> > > I'm not sure how I feel about this assertion being dropped in the
> > > generic header. In general my philosophy when creating these stub
> > > headers was to insert as many of these assertions as possible so
> > > we
> > > don't end up in a scenario where during early bringup I miss a
> > > function
> > > that was originally stubbed but ought to be implemented
> > > eventually.
> > >
> > > Looking at ARM's monitor.h too, it seems that this function is
> > > the only
> > > one that differs from the generic/stub version. I'm wondering if
> > > it
> > > would make sense to leave this function out of the generic
> > > header, to be
> > > defined by the arch similar to what you've done with some other
> > > functions in this series. That would also allow ARM to be brought
> > > in to
> > > using the generic variant.
> >
> > Yet then where would that function live, if not in
> > arch/*/include/asm/monitor.h?
>
> Hmm, maybe implicitly you're proposing that
> arch/*/include/asm/monitor.h
> include include/asm-generic/monitor.h in such a case, and define this
> one
> function on top?
I think it can be a solution. The same I suggest in my direct reply to
Shawn message ( I didn't see your answer ).
If everyone is OK with such solution, I can apply it for the next
version of patch series.
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h
2023-11-28 22:21 ` Shawn Anastasio
2023-11-29 8:19 ` Jan Beulich
@ 2023-11-29 12:39 ` Oleksii
2023-11-29 19:27 ` Shawn Anastasio
1 sibling, 1 reply; 57+ messages in thread
From: Oleksii @ 2023-11-29 12:39 UTC (permalink / raw)
To: Shawn Anastasio, xen-devel
Cc: Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu, Jan Beulich
Hi Shawn,
On Tue, 2023-11-28 at 16:21 -0600, Shawn Anastasio wrote:
> Hi Oleksii,
>
> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> > The header is shared between several archs so it is
> > moved to asm-generic.
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > Acked-by: Jan Beulich <jbeulich@suse.com>.
> > ---
> > Changes in V4:
> > - Removed the double blank line.
> > - Added Acked-by: Jan Beulich <jbeulich@suse.com>.
> > - Update the commit message
> > ---
> > Changes in V3:
> > - Use forward-declaration of struct domain instead of " #include
> > <xen/sched.h> ".
> > - Add ' include <xen/errno.h> '
> > - Drop PPC's monitor.h.
> > ---
> > Changes in V2:
> > - remove inclusion of "+#include <public/domctl.h>"
> > - add "struct xen_domctl_monitor_op;"
> > - remove one of SPDX tags.
> > ---
> > xen/arch/ppc/include/asm/Makefile | 1 +
> > xen/arch/ppc/include/asm/monitor.h | 43 ---------------------
> > xen/include/asm-generic/monitor.h | 62
> > ++++++++++++++++++++++++++++++
> > 3 files changed, 63 insertions(+), 43 deletions(-)
> > delete mode 100644 xen/arch/ppc/include/asm/monitor.h
> > create mode 100644 xen/include/asm-generic/monitor.h
> >
> > diff --git a/xen/arch/ppc/include/asm/Makefile
> > b/xen/arch/ppc/include/asm/Makefile
> > index 319e90955b..bcddcc181a 100644
> > --- a/xen/arch/ppc/include/asm/Makefile
> > +++ b/xen/arch/ppc/include/asm/Makefile
> > @@ -5,6 +5,7 @@ generic-y += div64.h
> > generic-y += hardirq.h
> > generic-y += hypercall.h
> > generic-y += iocap.h
> > +generic-y += monitor.h
> > generic-y += paging.h
> > generic-y += percpu.h
> > generic-y += random.h
> > diff --git a/xen/arch/ppc/include/asm/monitor.h
> > b/xen/arch/ppc/include/asm/monitor.h
> > deleted file mode 100644
> > index e5b0282bf1..0000000000
> > --- a/xen/arch/ppc/include/asm/monitor.h
> > +++ /dev/null
> > @@ -1,43 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0-only */
> > -/* Derived from xen/arch/arm/include/asm/monitor.h */
> > -#ifndef __ASM_PPC_MONITOR_H__
> > -#define __ASM_PPC_MONITOR_H__
> > -
> > -#include <public/domctl.h>
> > -#include <xen/errno.h>
> > -
> > -static inline
> > -void arch_monitor_allow_userspace(struct domain *d, bool
> > allow_userspace)
> > -{
> > -}
> > -
> > -static inline
> > -int arch_monitor_domctl_op(struct domain *d, struct
> > xen_domctl_monitor_op *mop)
> > -{
> > - /* No arch-specific monitor ops on PPC. */
> > - return -EOPNOTSUPP;
> > -}
> > -
> > -int arch_monitor_domctl_event(struct domain *d,
> > - struct xen_domctl_monitor_op *mop);
> > -
> > -static inline
> > -int arch_monitor_init_domain(struct domain *d)
> > -{
> > - /* No arch-specific domain initialization on PPC. */
> > - return 0;
> > -}
> > -
> > -static inline
> > -void arch_monitor_cleanup_domain(struct domain *d)
> > -{
> > - /* No arch-specific domain cleanup on PPC. */
> > -}
> > -
> > -static inline uint32_t arch_monitor_get_capabilities(struct domain
> > *d)
> > -{
> > - BUG_ON("unimplemented");
>
> I'm not sure how I feel about this assertion being dropped in the
> generic header. In general my philosophy when creating these stub
> headers was to insert as many of these assertions as possible so we
> don't end up in a scenario where during early bringup I miss a
> function
> that was originally stubbed but ought to be implemented eventually.
>
> Looking at ARM's monitor.h too, it seems that this function is the
> only
> one that differs from the generic/stub version. I'm wondering if it
> would make sense to leave this function out of the generic header, to
> be
> defined by the arch similar to what you've done with some other
> functions in this series. That would also allow ARM to be brought in
> to
> using the generic variant.
Thanks for the comment.
For RISC-V, I did in the same way ( about BUG() and unimplemented for
time being functions ).
I agree that such implementation isn't correct for generic header. I
think the best one solution will be to include <asm-generic/monitor.h>
in <asm/monitor.h> whwere only this function will be implemented (
because implementation of other functions are the same for Arm, PPC and
RISC-V ).
>
> > - return 0;
> > -}
> > -
> > -#endif /* __ASM_PPC_MONITOR_H__ */
> > diff --git a/xen/include/asm-generic/monitor.h b/xen/include/asm-
> > generic/monitor.h
> > new file mode 100644
> > index 0000000000..6be8614431
> > --- /dev/null
> > +++ b/xen/include/asm-generic/monitor.h
> > @@ -0,0 +1,62 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * include/asm-GENERIC/monitor.h
> > + *
> > + * Arch-specific monitor_op domctl handler.
> > + *
> > + * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com)
> > + * Copyright (c) 2016, Bitdefender S.R.L.
> > + *
> > + */
> > +
> > +#ifndef __ASM_GENERIC_MONITOR_H__
> > +#define __ASM_GENERIC_MONITOR_H__
> > +
> > +#include <xen/errno.h>
> > +
> > +struct domain;
> > +struct xen_domctl_monitor_op;
> > +
> > +static inline
> > +void arch_monitor_allow_userspace(struct domain *d, bool
> > allow_userspace)
> > +{
> > +}
> > +
> > +static inline
> > +int arch_monitor_domctl_op(struct domain *d, struct
> > xen_domctl_monitor_op *mop)
> > +{
> > + /* No arch-specific monitor ops on GENERIC. */
> > + return -EOPNOTSUPP;
> > +}
> > +
> > +int arch_monitor_domctl_event(struct domain *d,
> > + struct xen_domctl_monitor_op *mop);
> > +
> > +static inline
> > +int arch_monitor_init_domain(struct domain *d)
> > +{
> > + /* No arch-specific domain initialization on GENERIC. */
> > + return 0;
> > +}
> > +
> > +static inline
> > +void arch_monitor_cleanup_domain(struct domain *d)
> > +{
> > + /* No arch-specific domain cleanup on GENERIC. */
> > +}
> > +
> > +static inline uint32_t arch_monitor_get_capabilities(struct domain
> > *d)
> > +{
>
> See previous comment.
>
> > + return 0;
> > +}
> > +
> > +#endif /* __ASM_GENERIC_MONITOR_H__ */
> > +
> > +/*
> > + * Local variables:
> > + * mode: C
> > + * c-file-style: BSD
> > + * c-basic-offset: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h
2023-11-29 12:39 ` Oleksii
@ 2023-11-29 19:27 ` Shawn Anastasio
0 siblings, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-29 19:27 UTC (permalink / raw)
To: Oleksii, xen-devel
Cc: Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu, Jan Beulich
On 11/29/23 6:39 AM, Oleksii wrote:
> Hi Shawn,
>
> On Tue, 2023-11-28 at 16:21 -0600, Shawn Anastasio wrote:
>> Hi Oleksii,
>>
>> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
>>> diff --git a/xen/arch/ppc/include/asm/Makefile
>>> b/xen/arch/ppc/include/asm/Makefile
>>> index 319e90955b..bcddcc181a 100644
>>> --- a/xen/arch/ppc/include/asm/Makefile
>>> +++ b/xen/arch/ppc/include/asm/Makefile
>>> @@ -5,6 +5,7 @@ generic-y += div64.h
>>> generic-y += hardirq.h
>>> generic-y += hypercall.h
>>> generic-y += iocap.h
>>> +generic-y += monitor.h
>>> generic-y += paging.h
>>> generic-y += percpu.h
>>> generic-y += random.h
>>> diff --git a/xen/arch/ppc/include/asm/monitor.h
>>> b/xen/arch/ppc/include/asm/monitor.h
>>> deleted file mode 100644
>>> index e5b0282bf1..0000000000
>>> --- a/xen/arch/ppc/include/asm/monitor.h
>>> +++ /dev/null
>>> @@ -1,43 +0,0 @@
>>> -/* SPDX-License-Identifier: GPL-2.0-only */
>>> -/* Derived from xen/arch/arm/include/asm/monitor.h */
>>> -#ifndef __ASM_PPC_MONITOR_H__
>>> -#define __ASM_PPC_MONITOR_H__
>>> -
>>> -#include <public/domctl.h>
>>> -#include <xen/errno.h>
>>> -
>>> -static inline
>>> -void arch_monitor_allow_userspace(struct domain *d, bool
>>> allow_userspace)
>>> -{
>>> -}
>>> -
>>> -static inline
>>> -int arch_monitor_domctl_op(struct domain *d, struct
>>> xen_domctl_monitor_op *mop)
>>> -{
>>> - /* No arch-specific monitor ops on PPC. */
>>> - return -EOPNOTSUPP;
>>> -}
>>> -
>>> -int arch_monitor_domctl_event(struct domain *d,
>>> - struct xen_domctl_monitor_op *mop);
>>> -
>>> -static inline
>>> -int arch_monitor_init_domain(struct domain *d)
>>> -{
>>> - /* No arch-specific domain initialization on PPC. */
>>> - return 0;
>>> -}
>>> -
>>> -static inline
>>> -void arch_monitor_cleanup_domain(struct domain *d)
>>> -{
>>> - /* No arch-specific domain cleanup on PPC. */
>>> -}
>>> -
>>> -static inline uint32_t arch_monitor_get_capabilities(struct domain
>>> *d)
>>> -{
>>> - BUG_ON("unimplemented");
>>
>> I'm not sure how I feel about this assertion being dropped in the
>> generic header. In general my philosophy when creating these stub
>> headers was to insert as many of these assertions as possible so we
>> don't end up in a scenario where during early bringup I miss a
>> function
>> that was originally stubbed but ought to be implemented eventually.
>>
>> Looking at ARM's monitor.h too, it seems that this function is the
>> only
>> one that differs from the generic/stub version. I'm wondering if it
>> would make sense to leave this function out of the generic header, to
>> be
>> defined by the arch similar to what you've done with some other
>> functions in this series. That would also allow ARM to be brought in
>> to
>> using the generic variant.
> Thanks for the comment.
>
> For RISC-V, I did in the same way ( about BUG() and unimplemented for
> time being functions ).
>
> I agree that such implementation isn't correct for generic header. I
> think the best one solution will be to include <asm-generic/monitor.h>
> in <asm/monitor.h> whwere only this function will be implemented (
> because implementation of other functions are the same for Arm, PPC and
> RISC-V ).
>
That approach sounds great to me.
> ~ Oleksii
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 11/14] xen/asm-generic: introduce stub header numa.h
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (9 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-27 14:35 ` Jan Beulich
2023-11-29 19:49 ` Shawn Anastasio
2023-11-27 14:13 ` [PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h Oleksii Kurochko
` (3 subsequent siblings)
14 siblings, 2 replies; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Jan Beulich, Wei Liu, Shawn Anastasio
<asm/numa.h> is common through some archs so it is moved
to asm-generic.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
---
Changes in V4:
- Updated guards name: *ARCH_GENERIC* -> *ASM_GENERIC*.
- Moved inclusion of xen/mm-frame.h under "#ifndef CONFIG_NUMA".
- Added Reviewed-by: Michal Orzel <michal.orzel@amd.com>.
---
Changes in V3:
- Remove old header inclusion in asm-generic numa.h and include
<xen/mm-frame.h> and <xen/stdint.h>
- Drop Arm and PPC's numa.h and use asm-generic version instead.
---
Changes in V2:
- update the commit message.
- change u8 to uint8_t.
- add ifnded CONFIG_NUMA.
---
xen/arch/arm/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/numa.h | 26 -------------------
.../asm => include/asm-generic}/numa.h | 14 ++++++----
4 files changed, 11 insertions(+), 31 deletions(-)
delete mode 100644 xen/arch/ppc/include/asm/numa.h
rename xen/{arch/arm/include/asm => include/asm-generic}/numa.h (76%)
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index 55d153ed82..b8e0763c77 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -3,6 +3,7 @@ generic-y += altp2m.h
generic-y += device.h
generic-y += hardirq.h
generic-y += iocap.h
+generic-y += numa.h
generic-y += paging.h
generic-y += percpu.h
generic-y += random.h
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index bcddcc181a..a4188dce1d 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -6,6 +6,7 @@ generic-y += hardirq.h
generic-y += hypercall.h
generic-y += iocap.h
generic-y += monitor.h
+generic-y += numa.h
generic-y += paging.h
generic-y += percpu.h
generic-y += random.h
diff --git a/xen/arch/ppc/include/asm/numa.h b/xen/arch/ppc/include/asm/numa.h
deleted file mode 100644
index 7fdf66c3da..0000000000
--- a/xen/arch/ppc/include/asm/numa.h
+++ /dev/null
@@ -1,26 +0,0 @@
-#ifndef __ASM_PPC_NUMA_H__
-#define __ASM_PPC_NUMA_H__
-
-#include <xen/types.h>
-#include <xen/mm.h>
-
-typedef uint8_t nodeid_t;
-
-/* Fake one node for now. See also node_online_map. */
-#define cpu_to_node(cpu) 0
-#define node_to_cpumask(node) (cpu_online_map)
-
-/*
- * TODO: make first_valid_mfn static when NUMA is supported on PPC, this
- * is required because the dummy helpers are using it.
- */
-extern mfn_t first_valid_mfn;
-
-/* XXX: implement NUMA support */
-#define node_spanned_pages(nid) (max_page - mfn_x(first_valid_mfn))
-#define node_start_pfn(nid) (mfn_x(first_valid_mfn))
-#define __node_distance(a, b) (20)
-
-#define arch_want_default_dmazone() (false)
-
-#endif /* __ASM_PPC_NUMA_H__ */
diff --git a/xen/arch/arm/include/asm/numa.h b/xen/include/asm-generic/numa.h
similarity index 76%
rename from xen/arch/arm/include/asm/numa.h
rename to xen/include/asm-generic/numa.h
index e2bee2bd82..b00fca4978 100644
--- a/xen/arch/arm/include/asm/numa.h
+++ b/xen/include/asm-generic/numa.h
@@ -1,12 +1,15 @@
-#ifndef __ARCH_ARM_NUMA_H
-#define __ARCH_ARM_NUMA_H
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_NUMA_H
+#define __ASM_GENERIC_NUMA_H
-#include <xen/mm.h>
+#include <xen/stdint.h>
-typedef u8 nodeid_t;
+typedef uint8_t nodeid_t;
#ifndef CONFIG_NUMA
+#include <xen/mm-frame.h>
+
/* Fake one node for now. See also node_online_map. */
#define cpu_to_node(cpu) 0
#define node_to_cpumask(node) (cpu_online_map)
@@ -26,7 +29,8 @@ extern mfn_t first_valid_mfn;
#define arch_want_default_dmazone() (false)
-#endif /* __ARCH_ARM_NUMA_H */
+#endif /* __ASM_GENERIC_NUMA_H */
+
/*
* Local variables:
* mode: C
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 11/14] xen/asm-generic: introduce stub header numa.h
2023-11-27 14:13 ` [PATCH v4 11/14] xen/asm-generic: introduce stub header numa.h Oleksii Kurochko
@ 2023-11-27 14:35 ` Jan Beulich
2023-11-29 19:49 ` Shawn Anastasio
1 sibling, 0 replies; 57+ messages in thread
From: Jan Beulich @ 2023-11-27 14:35 UTC (permalink / raw)
To: Oleksii Kurochko
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On 27.11.2023 15:13, Oleksii Kurochko wrote:
> <asm/numa.h> is common through some archs so it is moved
> to asm-generic.
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 11/14] xen/asm-generic: introduce stub header numa.h
2023-11-27 14:13 ` [PATCH v4 11/14] xen/asm-generic: introduce stub header numa.h Oleksii Kurochko
2023-11-27 14:35 ` Jan Beulich
@ 2023-11-29 19:49 ` Shawn Anastasio
2023-11-30 12:49 ` Oleksii
1 sibling, 1 reply; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-29 19:49 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> diff --git a/xen/arch/arm/include/asm/numa.h b/xen/include/asm-generic/numa.h
> similarity index 76%
> rename from xen/arch/arm/include/asm/numa.h
> rename to xen/include/asm-generic/numa.h
> index e2bee2bd82..b00fca4978 100644
> --- a/xen/arch/arm/include/asm/numa.h
> +++ b/xen/include/asm-generic/numa.h
> @@ -1,12 +1,15 @@
> -#ifndef __ARCH_ARM_NUMA_H
> -#define __ARCH_ARM_NUMA_H
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_GENERIC_NUMA_H
> +#define __ASM_GENERIC_NUMA_H
>
> -#include <xen/mm.h>
> +#include <xen/stdint.h>
>
> -typedef u8 nodeid_t;
> +typedef uint8_t nodeid_t;
>
> #ifndef CONFIG_NUMA
>
> +#include <xen/mm-frame.h>
> +
> /* Fake one node for now. See also node_online_map. */
> #define cpu_to_node(cpu) 0
> #define node_to_cpumask(node) (cpu_online_map)
> @@ -26,7 +29,8 @@ extern mfn_t first_valid_mfn;
Minor nit: in this part of the file (that wasn't included by git in this
diff), there's a comment that references Arm:
/*
* TODO: make first_valid_mfn static when NUMA is supported on Arm, this
* is required because the dummy helpers are using it.
*/
extern mfn_t first_valid_mfn;
This should probably be changed to generic/GENERIC as you've done
elsewhere in the series.
In any case,
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 11/14] xen/asm-generic: introduce stub header numa.h
2023-11-29 19:49 ` Shawn Anastasio
@ 2023-11-30 12:49 ` Oleksii
0 siblings, 0 replies; 57+ messages in thread
From: Oleksii @ 2023-11-30 12:49 UTC (permalink / raw)
To: Shawn Anastasio, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu
On Wed, 2023-11-29 at 13:49 -0600, Shawn Anastasio wrote:
> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> > diff --git a/xen/arch/arm/include/asm/numa.h b/xen/include/asm-
> > generic/numa.h
> > similarity index 76%
> > rename from xen/arch/arm/include/asm/numa.h
> > rename to xen/include/asm-generic/numa.h
> > index e2bee2bd82..b00fca4978 100644
> > --- a/xen/arch/arm/include/asm/numa.h
> > +++ b/xen/include/asm-generic/numa.h
> > @@ -1,12 +1,15 @@
> > -#ifndef __ARCH_ARM_NUMA_H
> > -#define __ARCH_ARM_NUMA_H
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef __ASM_GENERIC_NUMA_H
> > +#define __ASM_GENERIC_NUMA_H
> >
> > -#include <xen/mm.h>
> > +#include <xen/stdint.h>
> >
> > -typedef u8 nodeid_t;
> > +typedef uint8_t nodeid_t;
> >
> > #ifndef CONFIG_NUMA
> >
> > +#include <xen/mm-frame.h>
> > +
> > /* Fake one node for now. See also node_online_map. */
> > #define cpu_to_node(cpu) 0
> > #define node_to_cpumask(node) (cpu_online_map)
> > @@ -26,7 +29,8 @@ extern mfn_t first_valid_mfn;
>
> Minor nit: in this part of the file (that wasn't included by git in
> this
> diff), there's a comment that references Arm:
>
> /*
> * TODO: make first_valid_mfn static when NUMA is supported on Arm,
> this
> * is required because the dummy helpers are using it.
> */
> extern mfn_t first_valid_mfn;
>
> This should probably be changed to generic/GENERIC as you've done
> elsewhere in the series.
>
> In any case,
>
> Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
>
Thanks. I'll update in next patch version.
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (10 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 11/14] xen/asm-generic: introduce stub header numa.h Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-27 14:36 ` Jan Beulich
` (2 more replies)
2023-11-27 14:13 ` [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h> Oleksii Kurochko
` (2 subsequent siblings)
14 siblings, 3 replies; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Jan Beulich, Wei Liu, Shawn Anastasio
<asm/softirq.h> is common between Arm, PPC and RISC-V so it is
moved to asm-generic.
Drop Arm and PPC's softirq.h and use asm-generic version instead.
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>
Added Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in V4:
- Added Reviewed-by: Michal Orzel <michal.orzel@amd.com>
- Added Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in V3:
- Drop Arm and PPC's softirq.h
- Update the commit message.
---
Changes in V2:
- update the commit message.
---
xen/arch/arm/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/Makefile | 1 +
xen/arch/ppc/include/asm/softirq.h | 8 --------
.../arm/include/asm => include/asm-generic}/softirq.h | 7 ++++---
4 files changed, 6 insertions(+), 11 deletions(-)
delete mode 100644 xen/arch/ppc/include/asm/softirq.h
rename xen/{arch/arm/include/asm => include/asm-generic}/softirq.h (56%)
diff --git a/xen/arch/arm/include/asm/Makefile b/xen/arch/arm/include/asm/Makefile
index b8e0763c77..c3f4291ee2 100644
--- a/xen/arch/arm/include/asm/Makefile
+++ b/xen/arch/arm/include/asm/Makefile
@@ -7,4 +7,5 @@ generic-y += numa.h
generic-y += paging.h
generic-y += percpu.h
generic-y += random.h
+generic-y += softirq.h
generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/Makefile b/xen/arch/ppc/include/asm/Makefile
index a4188dce1d..4da0c70cc0 100644
--- a/xen/arch/ppc/include/asm/Makefile
+++ b/xen/arch/ppc/include/asm/Makefile
@@ -10,4 +10,5 @@ generic-y += numa.h
generic-y += paging.h
generic-y += percpu.h
generic-y += random.h
+generic-y += softirq.h
generic-y += vm_event.h
diff --git a/xen/arch/ppc/include/asm/softirq.h b/xen/arch/ppc/include/asm/softirq.h
deleted file mode 100644
index a0b28a5e51..0000000000
--- a/xen/arch/ppc/include/asm/softirq.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef __ASM_PPC_SOFTIRQ_H__
-#define __ASM_PPC_SOFTIRQ_H__
-
-#define NR_ARCH_SOFTIRQS 0
-
-#define arch_skip_send_event_check(cpu) 0
-
-#endif /* __ASM_PPC_SOFTIRQ_H__ */
diff --git a/xen/arch/arm/include/asm/softirq.h b/xen/include/asm-generic/softirq.h
similarity index 56%
rename from xen/arch/arm/include/asm/softirq.h
rename to xen/include/asm-generic/softirq.h
index 976e0ebd70..83be855e50 100644
--- a/xen/arch/arm/include/asm/softirq.h
+++ b/xen/include/asm-generic/softirq.h
@@ -1,11 +1,12 @@
-#ifndef __ASM_SOFTIRQ_H__
-#define __ASM_SOFTIRQ_H__
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_SOFTIRQ_H__
+#define __ASM_GENERIC_SOFTIRQ_H__
#define NR_ARCH_SOFTIRQS 0
#define arch_skip_send_event_check(cpu) 0
-#endif /* __ASM_SOFTIRQ_H__ */
+#endif /* __ASM_GENERIC_SOFTIRQ_H__ */
/*
* Local variables:
* mode: C
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h
2023-11-27 14:13 ` [PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h Oleksii Kurochko
@ 2023-11-27 14:36 ` Jan Beulich
2023-11-27 19:39 ` Oleksii
2023-11-29 19:54 ` Shawn Anastasio
2023-11-29 20:04 ` Shawn Anastasio
2 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2023-11-27 14:36 UTC (permalink / raw)
To: Oleksii Kurochko
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On 27.11.2023 15:13, Oleksii Kurochko wrote:
> <asm/softirq.h> is common between Arm, PPC and RISC-V so it is
> moved to asm-generic.
>
> Drop Arm and PPC's softirq.h and use asm-generic version instead.
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> Added Acked-by: Jan Beulich <jbeulich@suse.com>
In case a v5 is needed, please drop the stray "Added".
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h
2023-11-27 14:36 ` Jan Beulich
@ 2023-11-27 19:39 ` Oleksii
0 siblings, 0 replies; 57+ messages in thread
From: Oleksii @ 2023-11-27 19:39 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On Mon, 2023-11-27 at 15:36 +0100, Jan Beulich wrote:
> On 27.11.2023 15:13, Oleksii Kurochko wrote:
> > <asm/softirq.h> is common between Arm, PPC and RISC-V so it is
> > moved to asm-generic.
> >
> > Drop Arm and PPC's softirq.h and use asm-generic version instead.
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> > Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> > Added Acked-by: Jan Beulich <jbeulich@suse.com>
>
> In case a v5 is needed, please drop the stray "Added".
Thanks.
I'll do in case a v5 is needed.
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h
2023-11-27 14:13 ` [PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h Oleksii Kurochko
2023-11-27 14:36 ` Jan Beulich
@ 2023-11-29 19:54 ` Shawn Anastasio
2023-11-29 20:04 ` Shawn Anastasio
2 siblings, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-29 19:54 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> <asm/softirq.h> is common between Arm, PPC and RISC-V so it is
> moved to asm-generic.
>
> Drop Arm and PPC's softirq.h and use asm-generic version instead.
>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h
2023-11-27 14:13 ` [PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h Oleksii Kurochko
2023-11-27 14:36 ` Jan Beulich
2023-11-29 19:54 ` Shawn Anastasio
@ 2023-11-29 20:04 ` Shawn Anastasio
2 siblings, 0 replies; 57+ messages in thread
From: Shawn Anastasio @ 2023-11-29 20:04 UTC (permalink / raw)
To: Oleksii Kurochko, xen-devel
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Jan Beulich,
Wei Liu
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> <asm/softirq.h> is common between Arm, PPC and RISC-V so it is
> moved to asm-generic.
>
> Drop Arm and PPC's softirq.h and use asm-generic version instead.
>
Acked-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Thanks,
Shawn
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (11 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 12/14] xen/asm-generic: introduce stub header softirq.h Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-27 14:41 ` Jan Beulich
2023-11-27 14:13 ` [PATCH v4 14/14] xen/asm-generic: ifdef inclusion of <asm/mem_access.h> Oleksii Kurochko
2023-11-29 9:25 ` [PATCH v4 00/14] Introduce generic headers Jan Beulich
14 siblings, 1 reply; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Jan Beulich, Wei Liu, Shawn Anastasio
Ifdef-ing inclusion of <asm/grant_table.h> allows to avoid
generation of empty <asm/grant_table.h> for cases when
CONFIG_GRANT_TABLE is not enabled.
The following changes were done for Arm:
<asm/grant_table.h> should be included directly because it contains
gnttab_dom0_frames() macros which is unique for Arm and is used in
arch/arm/domain_build.c.
<asm/grant_table.h> is #ifdef-ed with CONFIG_GRANT_TABLE in
<xen/grant_table.h> so in case of !CONFIG_GRANT_TABLE gnttab_dom0_frames
won't be available for use in arch/arm/domain_build.c.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V4:
- Nothing changed. Only rebase.
---
Changes in V3:
- Remove unnecessary comment.
---
xen/arch/arm/domain_build.c | 1 +
xen/arch/ppc/include/asm/grant_table.h | 5 -----
xen/include/xen/grant_table.h | 3 +++
3 files changed, 4 insertions(+), 5 deletions(-)
delete mode 100644 xen/arch/ppc/include/asm/grant_table.h
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a26cbff68e..646862b10e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -34,6 +34,7 @@
#include <xen/irq.h>
#include <xen/grant_table.h>
+#include <asm/grant_table.h>
#include <xen/serial.h>
#define STATIC_EVTCHN_NODE_SIZE_CELLS 2
diff --git a/xen/arch/ppc/include/asm/grant_table.h b/xen/arch/ppc/include/asm/grant_table.h
deleted file mode 100644
index d0ff58dd3d..0000000000
--- a/xen/arch/ppc/include/asm/grant_table.h
+++ /dev/null
@@ -1,5 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_GRANT_TABLE_H__
-#define __ASM_PPC_GRANT_TABLE_H__
-
-#endif /* __ASM_PPC_GRANT_TABLE_H__ */
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 85fe6b7b5e..50edfecfb6 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -26,7 +26,10 @@
#include <xen/mm-frame.h>
#include <xen/rwlock.h>
#include <public/grant_table.h>
+
+#ifdef CONFIG_GRANT_TABLE
#include <asm/grant_table.h>
+#endif
struct grant_table;
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
2023-11-27 14:13 ` [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h> Oleksii Kurochko
@ 2023-11-27 14:41 ` Jan Beulich
2023-11-27 19:38 ` Oleksii
0 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2023-11-27 14:41 UTC (permalink / raw)
To: Oleksii Kurochko
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On 27.11.2023 15:13, Oleksii Kurochko wrote:
> --- a/xen/arch/ppc/include/asm/grant_table.h
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -#ifndef __ASM_PPC_GRANT_TABLE_H__
> -#define __ASM_PPC_GRANT_TABLE_H__
> -
> -#endif /* __ASM_PPC_GRANT_TABLE_H__ */
Removing this header would be correct only if GRANT_TABLE had a "depends on
!PPC", I'm afraid. Recall that the earlier randconfig adjustment in CI was
actually requested to be undone, at which point what an arch's defconfig
says isn't necessarily what a randconfig should use.
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
2023-11-27 14:41 ` Jan Beulich
@ 2023-11-27 19:38 ` Oleksii
2023-11-28 7:57 ` Jan Beulich
0 siblings, 1 reply; 57+ messages in thread
From: Oleksii @ 2023-11-27 19:38 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On Mon, 2023-11-27 at 15:41 +0100, Jan Beulich wrote:
> On 27.11.2023 15:13, Oleksii Kurochko wrote:
> > --- a/xen/arch/ppc/include/asm/grant_table.h
> > +++ /dev/null
> > @@ -1,5 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0-only */
> > -#ifndef __ASM_PPC_GRANT_TABLE_H__
> > -#define __ASM_PPC_GRANT_TABLE_H__
> > -
> > -#endif /* __ASM_PPC_GRANT_TABLE_H__ */
>
> Removing this header would be correct only if GRANT_TABLE had a
> "depends on
> !PPC", I'm afraid. Recall that the earlier randconfig adjustment in
> CI was
> actually requested to be undone, at which point what an arch's
> defconfig
> says isn't necessarily what a randconfig should use.
We can do depends on !PPC && !RISCV but shouldn't it be enough only to
turn CONFIG_GRANT_TABLE off in defconfig and set CONFIG_GRANT_TABLE=n
in EXTRA_XEN_CONFIG?
Some time ago I also tried to redefine "Config GRANT_TABLE" in arch-
specific Kconfig + defconfig + EXTRA_XEN_CONFIG and it works for me.
Could it be solution instead of "depends on..." ?
One more question I have do we really need this randconfig? On RISC-V
side, I launched several time this patch series ( from v1 to v4 + runs
during test of patch series ) and I haven't faced case
when CONFIG_GRANT_TABLE=n. ( but I turned the config off in defconfig +
EXTRA_XEN_CONFIG ). Also when it "Config GRANT_TABLE" was re-defined in
arch-specific KConfig I haven't face an issue with CONFIG_GRANT_TABLE
too.
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
2023-11-27 19:38 ` Oleksii
@ 2023-11-28 7:57 ` Jan Beulich
2023-11-28 9:28 ` Oleksii
0 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2023-11-28 7:57 UTC (permalink / raw)
To: Oleksii
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On 27.11.2023 20:38, Oleksii wrote:
> On Mon, 2023-11-27 at 15:41 +0100, Jan Beulich wrote:
>> On 27.11.2023 15:13, Oleksii Kurochko wrote:
>>> --- a/xen/arch/ppc/include/asm/grant_table.h
>>> +++ /dev/null
>>> @@ -1,5 +0,0 @@
>>> -/* SPDX-License-Identifier: GPL-2.0-only */
>>> -#ifndef __ASM_PPC_GRANT_TABLE_H__
>>> -#define __ASM_PPC_GRANT_TABLE_H__
>>> -
>>> -#endif /* __ASM_PPC_GRANT_TABLE_H__ */
>>
>> Removing this header would be correct only if GRANT_TABLE had a
>> "depends on
>> !PPC", I'm afraid. Recall that the earlier randconfig adjustment in
>> CI was
>> actually requested to be undone, at which point what an arch's
>> defconfig
>> says isn't necessarily what a randconfig should use.
> We can do depends on !PPC && !RISCV but shouldn't it be enough only to
> turn CONFIG_GRANT_TABLE off in defconfig and set CONFIG_GRANT_TABLE=n
> in EXTRA_XEN_CONFIG?
That _might_ be sufficient for CI, but we shouldn't take CI as the only
thing in the world. Personally I consider any kind of overriding for,
in particular, randconfig at bets bogus. Whatever may not be selected
(or must be selected) should be arranged for by Kconfig files themselves.
That said, there may be _rare_ exceptions. But what PPC's and RISC-V's
defconfig-s have right now is more than "rare" imo.
> Some time ago I also tried to redefine "Config GRANT_TABLE" in arch-
> specific Kconfig + defconfig + EXTRA_XEN_CONFIG and it works for me.
> Could it be solution instead of "depends on..." ?
Why would we want to re-define an setting? There wants to be one single
place where a common option is defined. Or maybe I don't understand
what you're suggesting ...
> One more question I have do we really need this randconfig? On RISC-V
> side, I launched several time this patch series ( from v1 to v4 + runs
> during test of patch series ) and I haven't faced case
> when CONFIG_GRANT_TABLE=n. ( but I turned the config off in defconfig +
> EXTRA_XEN_CONFIG ).
That override is why in CI you wouldn't see an issue. But as said - CI
isn't everything.
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
2023-11-28 7:57 ` Jan Beulich
@ 2023-11-28 9:28 ` Oleksii
2023-11-28 9:58 ` Jan Beulich
0 siblings, 1 reply; 57+ messages in thread
From: Oleksii @ 2023-11-28 9:28 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On Tue, 2023-11-28 at 08:57 +0100, Jan Beulich wrote:
> On 27.11.2023 20:38, Oleksii wrote:
> > On Mon, 2023-11-27 at 15:41 +0100, Jan Beulich wrote:
> > > On 27.11.2023 15:13, Oleksii Kurochko wrote:
> > > > --- a/xen/arch/ppc/include/asm/grant_table.h
> > > > +++ /dev/null
> > > > @@ -1,5 +0,0 @@
> > > > -/* SPDX-License-Identifier: GPL-2.0-only */
> > > > -#ifndef __ASM_PPC_GRANT_TABLE_H__
> > > > -#define __ASM_PPC_GRANT_TABLE_H__
> > > > -
> > > > -#endif /* __ASM_PPC_GRANT_TABLE_H__ */
> > >
> > > Removing this header would be correct only if GRANT_TABLE had a
> > > "depends on
> > > !PPC", I'm afraid. Recall that the earlier randconfig adjustment
> > > in
> > > CI was
> > > actually requested to be undone, at which point what an arch's
> > > defconfig
> > > says isn't necessarily what a randconfig should use.
> > We can do depends on !PPC && !RISCV but shouldn't it be enough only
> > to
> > turn CONFIG_GRANT_TABLE off in defconfig and set
> > CONFIG_GRANT_TABLE=n
> > in EXTRA_XEN_CONFIG?
>
> That _might_ be sufficient for CI, but we shouldn't take CI as the
> only
> thing in the world. Personally I consider any kind of overriding for,
> in particular, randconfig at bets bogus. Whatever may not be selected
> (or must be selected) should be arranged for by Kconfig files
> themselves.
> That said, there may be _rare_ exceptions. But what PPC's and RISC-
> V's
> defconfig-s have right now is more than "rare" imo.
>
> > Some time ago I also tried to redefine "Config GRANT_TABLE" in
> > arch-
> > specific Kconfig + defconfig + EXTRA_XEN_CONFIG and it works for
> > me.
> > Could it be solution instead of "depends on..." ?
>
> Why would we want to re-define an setting? There wants to be one
> single
> place where a common option is defined. Or maybe I don't understand
> what you're suggesting ...
I just thought this change is temporary because grant_table.h will be
introduced later or earlier, and it will be needed to remove "depends
on !PPC && !RISCV". And not to litter common KConfig with the change
which will be removed, it will be better to redefine it in arch-
specific Kconfig with default n.
But after your words about one place, I realized that it would be
better to update a place where a common option is defined.
The only thing I would like to change is probably it will be better to
do the opposite, add "depends on" arches that support
CONFIG_GRANT_TABLE now so it will not need to update "depends on" for
new arches or arches that don't support CONFIG_GRANT_TABLE.
>
> > One more question I have do we really need this randconfig? On
> > RISC-V
> > side, I launched several time this patch series ( from v1 to v4 +
> > runs
> > during test of patch series ) and I haven't faced case
> > when CONFIG_GRANT_TABLE=n. ( but I turned the config off in
> > defconfig +
> > EXTRA_XEN_CONFIG ).
>
> That override is why in CI you wouldn't see an issue. But as said -
> CI
> isn't everything.
From this point of view it will be better to add "depends on !PPC &&
!RISCV" to "Config GRANT_TABLE".
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
2023-11-28 9:28 ` Oleksii
@ 2023-11-28 9:58 ` Jan Beulich
2023-11-28 11:49 ` Oleksii
0 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2023-11-28 9:58 UTC (permalink / raw)
To: Oleksii
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On 28.11.2023 10:28, Oleksii wrote:
> On Tue, 2023-11-28 at 08:57 +0100, Jan Beulich wrote:
>> On 27.11.2023 20:38, Oleksii wrote:
>>> On Mon, 2023-11-27 at 15:41 +0100, Jan Beulich wrote:
>>>> On 27.11.2023 15:13, Oleksii Kurochko wrote:
>>>>> --- a/xen/arch/ppc/include/asm/grant_table.h
>>>>> +++ /dev/null
>>>>> @@ -1,5 +0,0 @@
>>>>> -/* SPDX-License-Identifier: GPL-2.0-only */
>>>>> -#ifndef __ASM_PPC_GRANT_TABLE_H__
>>>>> -#define __ASM_PPC_GRANT_TABLE_H__
>>>>> -
>>>>> -#endif /* __ASM_PPC_GRANT_TABLE_H__ */
>>>>
>>>> Removing this header would be correct only if GRANT_TABLE had a
>>>> "depends on
>>>> !PPC", I'm afraid. Recall that the earlier randconfig adjustment
>>>> in
>>>> CI was
>>>> actually requested to be undone, at which point what an arch's
>>>> defconfig
>>>> says isn't necessarily what a randconfig should use.
>>> We can do depends on !PPC && !RISCV but shouldn't it be enough only
>>> to
>>> turn CONFIG_GRANT_TABLE off in defconfig and set
>>> CONFIG_GRANT_TABLE=n
>>> in EXTRA_XEN_CONFIG?
>>
>> That _might_ be sufficient for CI, but we shouldn't take CI as the
>> only
>> thing in the world. Personally I consider any kind of overriding for,
>> in particular, randconfig at bets bogus. Whatever may not be selected
>> (or must be selected) should be arranged for by Kconfig files
>> themselves.
>> That said, there may be _rare_ exceptions. But what PPC's and RISC-
>> V's
>> defconfig-s have right now is more than "rare" imo.
>>
>>> Some time ago I also tried to redefine "Config GRANT_TABLE" in
>>> arch-
>>> specific Kconfig + defconfig + EXTRA_XEN_CONFIG and it works for
>>> me.
>>> Could it be solution instead of "depends on..." ?
>>
>> Why would we want to re-define an setting? There wants to be one
>> single
>> place where a common option is defined. Or maybe I don't understand
>> what you're suggesting ...
> I just thought this change is temporary because grant_table.h will be
> introduced later or earlier, and it will be needed to remove "depends
> on !PPC && !RISCV". And not to litter common KConfig with the change
> which will be removed, it will be better to redefine it in arch-
> specific Kconfig with default n.
Right. But if it's indeed temporary, what's the point of this patch?
The entire series is (supposed to be) to improve code structure for
longer term purposes. If a non-generic grant_table.h is going to
appear for PPC and RISC-V, I don't see why the present dummy would
need removing. That's only useful if an arch can be expected to live
with GRANT_TABLE=n even when otherwise feature-complete, and at that
point modifying the Kconfig dependencies would (imo) be appropriate.
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
2023-11-28 9:58 ` Jan Beulich
@ 2023-11-28 11:49 ` Oleksii
2023-11-28 12:53 ` Jan Beulich
0 siblings, 1 reply; 57+ messages in thread
From: Oleksii @ 2023-11-28 11:49 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On Tue, 2023-11-28 at 10:58 +0100, Jan Beulich wrote:
> On 28.11.2023 10:28, Oleksii wrote:
> > On Tue, 2023-11-28 at 08:57 +0100, Jan Beulich wrote:
> > > On 27.11.2023 20:38, Oleksii wrote:
> > > > On Mon, 2023-11-27 at 15:41 +0100, Jan Beulich wrote:
> > > > > On 27.11.2023 15:13, Oleksii Kurochko wrote:
> > > > > > --- a/xen/arch/ppc/include/asm/grant_table.h
> > > > > > +++ /dev/null
> > > > > > @@ -1,5 +0,0 @@
> > > > > > -/* SPDX-License-Identifier: GPL-2.0-only */
> > > > > > -#ifndef __ASM_PPC_GRANT_TABLE_H__
> > > > > > -#define __ASM_PPC_GRANT_TABLE_H__
> > > > > > -
> > > > > > -#endif /* __ASM_PPC_GRANT_TABLE_H__ */
> > > > >
> > > > > Removing this header would be correct only if GRANT_TABLE had
> > > > > a
> > > > > "depends on
> > > > > !PPC", I'm afraid. Recall that the earlier randconfig
> > > > > adjustment
> > > > > in
> > > > > CI was
> > > > > actually requested to be undone, at which point what an
> > > > > arch's
> > > > > defconfig
> > > > > says isn't necessarily what a randconfig should use.
> > > > We can do depends on !PPC && !RISCV but shouldn't it be enough
> > > > only
> > > > to
> > > > turn CONFIG_GRANT_TABLE off in defconfig and set
> > > > CONFIG_GRANT_TABLE=n
> > > > in EXTRA_XEN_CONFIG?
> > >
> > > That _might_ be sufficient for CI, but we shouldn't take CI as
> > > the
> > > only
> > > thing in the world. Personally I consider any kind of overriding
> > > for,
> > > in particular, randconfig at bets bogus. Whatever may not be
> > > selected
> > > (or must be selected) should be arranged for by Kconfig files
> > > themselves.
> > > That said, there may be _rare_ exceptions. But what PPC's and
> > > RISC-
> > > V's
> > > defconfig-s have right now is more than "rare" imo.
> > >
> > > > Some time ago I also tried to redefine "Config GRANT_TABLE" in
> > > > arch-
> > > > specific Kconfig + defconfig + EXTRA_XEN_CONFIG and it works
> > > > for
> > > > me.
> > > > Could it be solution instead of "depends on..." ?
> > >
> > > Why would we want to re-define an setting? There wants to be one
> > > single
> > > place where a common option is defined. Or maybe I don't
> > > understand
> > > what you're suggesting ...
> > I just thought this change is temporary because grant_table.h will
> > be
> > introduced later or earlier, and it will be needed to remove
> > "depends
> > on !PPC && !RISCV". And not to litter common KConfig with the
> > change
> > which will be removed, it will be better to redefine it in arch-
> > specific Kconfig with default n.
>
> Right. But if it's indeed temporary, what's the point of this patch?
> The entire series is (supposed to be) to improve code structure for
> longer term purposes. If a non-generic grant_table.h is going to
> appear for PPC and RISC-V, I don't see why the present dummy would
> need removing. That's only useful if an arch can be expected to live
> with GRANT_TABLE=n even when otherwise feature-complete, and at that
> point modifying the Kconfig dependencies would (imo) be appropriate.
I agree. Let's proceed by adding the dependency in the KConfig file.
So which one option will be better:
1. depends on !PPC && !RISCV
2. depends on ARM || X86
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
2023-11-28 11:49 ` Oleksii
@ 2023-11-28 12:53 ` Jan Beulich
2023-11-28 13:28 ` Oleksii
0 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2023-11-28 12:53 UTC (permalink / raw)
To: Oleksii
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On 28.11.2023 12:49, Oleksii wrote:
> On Tue, 2023-11-28 at 10:58 +0100, Jan Beulich wrote:
>> On 28.11.2023 10:28, Oleksii wrote:
>>> On Tue, 2023-11-28 at 08:57 +0100, Jan Beulich wrote:
>>>> On 27.11.2023 20:38, Oleksii wrote:
>>>>> On Mon, 2023-11-27 at 15:41 +0100, Jan Beulich wrote:
>>>>>> On 27.11.2023 15:13, Oleksii Kurochko wrote:
>>>>>>> --- a/xen/arch/ppc/include/asm/grant_table.h
>>>>>>> +++ /dev/null
>>>>>>> @@ -1,5 +0,0 @@
>>>>>>> -/* SPDX-License-Identifier: GPL-2.0-only */
>>>>>>> -#ifndef __ASM_PPC_GRANT_TABLE_H__
>>>>>>> -#define __ASM_PPC_GRANT_TABLE_H__
>>>>>>> -
>>>>>>> -#endif /* __ASM_PPC_GRANT_TABLE_H__ */
>>>>>>
>>>>>> Removing this header would be correct only if GRANT_TABLE had
>>>>>> a
>>>>>> "depends on
>>>>>> !PPC", I'm afraid. Recall that the earlier randconfig
>>>>>> adjustment
>>>>>> in
>>>>>> CI was
>>>>>> actually requested to be undone, at which point what an
>>>>>> arch's
>>>>>> defconfig
>>>>>> says isn't necessarily what a randconfig should use.
>>>>> We can do depends on !PPC && !RISCV but shouldn't it be enough
>>>>> only
>>>>> to
>>>>> turn CONFIG_GRANT_TABLE off in defconfig and set
>>>>> CONFIG_GRANT_TABLE=n
>>>>> in EXTRA_XEN_CONFIG?
>>>>
>>>> That _might_ be sufficient for CI, but we shouldn't take CI as
>>>> the
>>>> only
>>>> thing in the world. Personally I consider any kind of overriding
>>>> for,
>>>> in particular, randconfig at bets bogus. Whatever may not be
>>>> selected
>>>> (or must be selected) should be arranged for by Kconfig files
>>>> themselves.
>>>> That said, there may be _rare_ exceptions. But what PPC's and
>>>> RISC-
>>>> V's
>>>> defconfig-s have right now is more than "rare" imo.
>>>>
>>>>> Some time ago I also tried to redefine "Config GRANT_TABLE" in
>>>>> arch-
>>>>> specific Kconfig + defconfig + EXTRA_XEN_CONFIG and it works
>>>>> for
>>>>> me.
>>>>> Could it be solution instead of "depends on..." ?
>>>>
>>>> Why would we want to re-define an setting? There wants to be one
>>>> single
>>>> place where a common option is defined. Or maybe I don't
>>>> understand
>>>> what you're suggesting ...
>>> I just thought this change is temporary because grant_table.h will
>>> be
>>> introduced later or earlier, and it will be needed to remove
>>> "depends
>>> on !PPC && !RISCV". And not to litter common KConfig with the
>>> change
>>> which will be removed, it will be better to redefine it in arch-
>>> specific Kconfig with default n.
>>
>> Right. But if it's indeed temporary, what's the point of this patch?
>> The entire series is (supposed to be) to improve code structure for
>> longer term purposes. If a non-generic grant_table.h is going to
>> appear for PPC and RISC-V, I don't see why the present dummy would
>> need removing. That's only useful if an arch can be expected to live
>> with GRANT_TABLE=n even when otherwise feature-complete, and at that
>> point modifying the Kconfig dependencies would (imo) be appropriate.
> I agree. Let's proceed by adding the dependency in the KConfig file.
>
> So which one option will be better:
> 1. depends on !PPC && !RISCV
> 2. depends on ARM || X86
Agreeing and then making this suggestion contradicts one another. Unless
the long-term plan is for PPC and RISC-V to not use grant tables.
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
2023-11-28 12:53 ` Jan Beulich
@ 2023-11-28 13:28 ` Oleksii
0 siblings, 0 replies; 57+ messages in thread
From: Oleksii @ 2023-11-28 13:28 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, xen-devel
On Tue, 2023-11-28 at 13:53 +0100, Jan Beulich wrote:
> On 28.11.2023 12:49, Oleksii wrote:
> > On Tue, 2023-11-28 at 10:58 +0100, Jan Beulich wrote:
> > > On 28.11.2023 10:28, Oleksii wrote:
> > > > On Tue, 2023-11-28 at 08:57 +0100, Jan Beulich wrote:
> > > > > On 27.11.2023 20:38, Oleksii wrote:
> > > > > > On Mon, 2023-11-27 at 15:41 +0100, Jan Beulich wrote:
> > > > > > > On 27.11.2023 15:13, Oleksii Kurochko wrote:
> > > > > > > > --- a/xen/arch/ppc/include/asm/grant_table.h
> > > > > > > > +++ /dev/null
> > > > > > > > @@ -1,5 +0,0 @@
> > > > > > > > -/* SPDX-License-Identifier: GPL-2.0-only */
> > > > > > > > -#ifndef __ASM_PPC_GRANT_TABLE_H__
> > > > > > > > -#define __ASM_PPC_GRANT_TABLE_H__
> > > > > > > > -
> > > > > > > > -#endif /* __ASM_PPC_GRANT_TABLE_H__ */
> > > > > > >
> > > > > > > Removing this header would be correct only if GRANT_TABLE
> > > > > > > had
> > > > > > > a
> > > > > > > "depends on
> > > > > > > !PPC", I'm afraid. Recall that the earlier randconfig
> > > > > > > adjustment
> > > > > > > in
> > > > > > > CI was
> > > > > > > actually requested to be undone, at which point what an
> > > > > > > arch's
> > > > > > > defconfig
> > > > > > > says isn't necessarily what a randconfig should use.
> > > > > > We can do depends on !PPC && !RISCV but shouldn't it be
> > > > > > enough
> > > > > > only
> > > > > > to
> > > > > > turn CONFIG_GRANT_TABLE off in defconfig and set
> > > > > > CONFIG_GRANT_TABLE=n
> > > > > > in EXTRA_XEN_CONFIG?
> > > > >
> > > > > That _might_ be sufficient for CI, but we shouldn't take CI
> > > > > as
> > > > > the
> > > > > only
> > > > > thing in the world. Personally I consider any kind of
> > > > > overriding
> > > > > for,
> > > > > in particular, randconfig at bets bogus. Whatever may not be
> > > > > selected
> > > > > (or must be selected) should be arranged for by Kconfig files
> > > > > themselves.
> > > > > That said, there may be _rare_ exceptions. But what PPC's and
> > > > > RISC-
> > > > > V's
> > > > > defconfig-s have right now is more than "rare" imo.
> > > > >
> > > > > > Some time ago I also tried to redefine "Config GRANT_TABLE"
> > > > > > in
> > > > > > arch-
> > > > > > specific Kconfig + defconfig + EXTRA_XEN_CONFIG and it
> > > > > > works
> > > > > > for
> > > > > > me.
> > > > > > Could it be solution instead of "depends on..." ?
> > > > >
> > > > > Why would we want to re-define an setting? There wants to be
> > > > > one
> > > > > single
> > > > > place where a common option is defined. Or maybe I don't
> > > > > understand
> > > > > what you're suggesting ...
> > > > I just thought this change is temporary because grant_table.h
> > > > will
> > > > be
> > > > introduced later or earlier, and it will be needed to remove
> > > > "depends
> > > > on !PPC && !RISCV". And not to litter common KConfig with the
> > > > change
> > > > which will be removed, it will be better to redefine it in
> > > > arch-
> > > > specific Kconfig with default n.
> > >
> > > Right. But if it's indeed temporary, what's the point of this
> > > patch?
> > > The entire series is (supposed to be) to improve code structure
> > > for
> > > longer term purposes. If a non-generic grant_table.h is going to
> > > appear for PPC and RISC-V, I don't see why the present dummy
> > > would
> > > need removing. That's only useful if an arch can be expected to
> > > live
> > > with GRANT_TABLE=n even when otherwise feature-complete, and at
> > > that
> > > point modifying the Kconfig dependencies would (imo) be
> > > appropriate.
> > I agree. Let's proceed by adding the dependency in the KConfig
> > file.
> >
> > So which one option will be better:
> > 1. depends on !PPC && !RISCV
> > 2. depends on ARM || X86
>
> Agreeing and then making this suggestion contradicts one another.
> Unless
> the long-term plan is for PPC and RISC-V to not use grant tables.
On RISC-V side, I can run guests in dom0less and still don't use
grant_tables. And I am not sure when I'll start to implement it. Only
one thing I defined in grant_table.h is:
#define gnttab_dom0_frames()
\
min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext -
_stext))
But I think it can moved somewhere or dropped as it was defined because
of:
void __init create_dom0(void)
{
struct domain *dom0;
struct xen_domctl_createdomain dom0_cfg = {
.flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
.max_evtchn_port = -1,
.max_grant_frames = gnttab_dom0_frames(),
.max_maptrack_frames = -1,
.grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
};
...
Which was copied from Arm.
Taking into account that opt_max_grant_frames is 0 in case when
CONFIG_GRANT_TABLE=n then ".max_grant_frames =" will be 0 in case of
!CONFIG_GRANT_TABLE so for time being the macros gnttab_dom0_frames can
be dropped until CONFIG_GRANT_TABLE will be introduced.
But I am not aware of PPC plans of usage this config.
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH v4 14/14] xen/asm-generic: ifdef inclusion of <asm/mem_access.h>
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (12 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 13/14] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h> Oleksii Kurochko
@ 2023-11-27 14:13 ` Oleksii Kurochko
2023-11-27 14:43 ` Jan Beulich
2023-11-29 9:25 ` [PATCH v4 00/14] Introduce generic headers Jan Beulich
14 siblings, 1 reply; 57+ messages in thread
From: Oleksii Kurochko @ 2023-11-27 14:13 UTC (permalink / raw)
To: xen-devel
Cc: Oleksii Kurochko, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Michal Orzel, Volodymyr Babchuk,
Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu,
Shawn Anastasio, Jan Beulich
ifdefing inclusion of <asm/mem_access.h> in <xen/mem_access.h>
allows to avoid generation of empty <asm/mem_access.h> header
for the case when !CONFIG_MEM_ACCESS.
For Arm it was explicitly added inclusion of <asm/mem_access.h> for p2m.c
and traps.c because they require some functions from <asm/mem_access.h> which
aren't available in case of !CONFIG_MEM_ACCESS.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V4:
- Nothing changed. Only rebase.
---
Changes in V3:
- Remove unnecessary comment.
---
xen/arch/arm/p2m.c | 1 +
xen/arch/arm/traps.c | 1 +
xen/arch/ppc/include/asm/mem_access.h | 5 -----
xen/include/xen/mem_access.h | 2 ++
4 files changed, 4 insertions(+), 5 deletions(-)
delete mode 100644 xen/arch/ppc/include/asm/mem_access.h
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index b991b76ce4..2465c266e9 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -7,6 +7,7 @@
#include <asm/event.h>
#include <asm/flushtlb.h>
#include <asm/guest_walk.h>
+#include <asm/mem_access.h>
#include <asm/page.h>
#include <asm/traps.h>
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 8492e2b7bb..0ad056b89b 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -35,6 +35,7 @@
#include <asm/cpufeature.h>
#include <asm/event.h>
#include <asm/hsr.h>
+#include <asm/mem_access.h>
#include <asm/mmio.h>
#include <asm/regs.h>
#include <asm/smccc.h>
diff --git a/xen/arch/ppc/include/asm/mem_access.h b/xen/arch/ppc/include/asm/mem_access.h
deleted file mode 100644
index e7986dfdbd..0000000000
--- a/xen/arch/ppc/include/asm/mem_access.h
+++ /dev/null
@@ -1,5 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_MEM_ACCESS_H__
-#define __ASM_PPC_MEM_ACCESS_H__
-
-#endif /* __ASM_PPC_MEM_ACCESS_H__ */
diff --git a/xen/include/xen/mem_access.h b/xen/include/xen/mem_access.h
index 4e4811680d..87d93b31f6 100644
--- a/xen/include/xen/mem_access.h
+++ b/xen/include/xen/mem_access.h
@@ -33,7 +33,9 @@
*/
struct vm_event_st;
+#ifdef CONFIG_MEM_ACCESS
#include <asm/mem_access.h>
+#endif
/*
* Additional access types, which are used to further restrict
--
2.43.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH v4 14/14] xen/asm-generic: ifdef inclusion of <asm/mem_access.h>
2023-11-27 14:13 ` [PATCH v4 14/14] xen/asm-generic: ifdef inclusion of <asm/mem_access.h> Oleksii Kurochko
@ 2023-11-27 14:43 ` Jan Beulich
0 siblings, 0 replies; 57+ messages in thread
From: Jan Beulich @ 2023-11-27 14:43 UTC (permalink / raw)
To: Oleksii Kurochko
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Tamas K Lengyel, Alexandru Isaila,
Petre Pircalabu, Shawn Anastasio, xen-devel
On 27.11.2023 15:13, Oleksii Kurochko wrote:
> --- a/xen/arch/ppc/include/asm/mem_access.h
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -#ifndef __ASM_PPC_MEM_ACCESS_H__
> -#define __ASM_PPC_MEM_ACCESS_H__
> -
> -#endif /* __ASM_PPC_MEM_ACCESS_H__ */
Same as for the grant table change - if you want to remove this header
(and not put one in place for RISC-V), MEM_ACCESS needs to be impossible
to turn on for PPC and RISC-V.
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH v4 00/14] Introduce generic headers
2023-11-27 14:13 [PATCH v4 00/14] Introduce generic headers Oleksii Kurochko
` (13 preceding siblings ...)
2023-11-27 14:13 ` [PATCH v4 14/14] xen/asm-generic: ifdef inclusion of <asm/mem_access.h> Oleksii Kurochko
@ 2023-11-29 9:25 ` Jan Beulich
2023-11-29 12:32 ` Oleksii
14 siblings, 1 reply; 57+ messages in thread
From: Jan Beulich @ 2023-11-29 9:25 UTC (permalink / raw)
To: Oleksii Kurochko
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, Roger Pau Monné,
Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu, xen-devel
On 27.11.2023 15:13, Oleksii Kurochko wrote:
> Oleksii Kurochko (14):
> xen/asm-generic: introduce stub header paging.h
> xen/asm-generic: introduce generic device.h
> xen/asm-generic: introduce generic hypercall.h
> xen/asm-generic: introduce generic header iocap.h
> xen/asm-generic: introduce stub header <asm/random.h>
> xen/asm-generic: introduce generic header percpu.h
> xen/asm-generic: introduce generalized hardirq.h
> xen/asm-generic: introduce generic div64.h header
> xen/asm-generic: introduce generic header altp2m.h
> xen/asm-generic: introduce stub header monitor.h
> xen/asm-generic: introduce stub header numa.h
> xen/asm-generic: introduce stub header softirq.h
> xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>
> xen/asm-generic: ifdef inclusion of <asm/mem_access.h>
I've applied what I think was suitably ack-ed, re-basing over patches which
need further work. For the rest I'd like to ask that you do the necessary
re-basing, perhaps moving in particular the device.h change later in the
series (so that "easier" changes can go in without further tweaking).
Jan
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH v4 00/14] Introduce generic headers
2023-11-29 9:25 ` [PATCH v4 00/14] Introduce generic headers Jan Beulich
@ 2023-11-29 12:32 ` Oleksii
0 siblings, 0 replies; 57+ messages in thread
From: Oleksii @ 2023-11-29 12:32 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
Shawn Anastasio, Roger Pau Monné,
Tamas K Lengyel, Alexandru Isaila, Petre Pircalabu, xen-devel
On Wed, 2023-11-29 at 10:25 +0100, Jan Beulich wrote:
> On 27.11.2023 15:13, Oleksii Kurochko wrote:
> > Oleksii Kurochko (14):
> > xen/asm-generic: introduce stub header paging.h
> > xen/asm-generic: introduce generic device.h
> > xen/asm-generic: introduce generic hypercall.h
> > xen/asm-generic: introduce generic header iocap.h
> > xen/asm-generic: introduce stub header <asm/random.h>
> > xen/asm-generic: introduce generic header percpu.h
> > xen/asm-generic: introduce generalized hardirq.h
> > xen/asm-generic: introduce generic div64.h header
> > xen/asm-generic: introduce generic header altp2m.h
> > xen/asm-generic: introduce stub header monitor.h
> > xen/asm-generic: introduce stub header numa.h
> > xen/asm-generic: introduce stub header softirq.h
> > xen: ifdef inclusion of <asm/grant_table.h> in
> > <xen/grant_table.h>
> > xen/asm-generic: ifdef inclusion of <asm/mem_access.h>
>
> I've applied what I think was suitably ack-ed, re-basing over patches
> which
> need further work. For the rest I'd like to ask that you do the
> necessary
> re-basing, perhaps moving in particular the device.h change later in
> the
> series (so that "easier" changes can go in without further tweaking).
Thanks a lot. I'll take into account your advice.
~ Oleksii
^ permalink raw reply [flat|nested] 57+ messages in thread