On 10.03.2026 18:08, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/Makefile
> +++ b/xen/arch/riscv/Makefile
> @@ -1,6 +1,7 @@
> obj-y += aplic.o
> obj-y += cpufeature.o
> obj-y += domain.o
> +obj-y += domain-build.o
> obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
> obj-y += entry.o
> obj-y += guestcopy.o
Doesn't this belong in patch 02? And then domain-build.init.o?
> --- a/xen/arch/riscv/domain-build.c
> +++ b/xen/arch/riscv/domain-build.c
> @@ -3,8 +3,10 @@
> #include <xen/fdt-domain-build.h>
> #include <xen/fdt-kernel.h>
> #include <xen/init.h>
> +#include <xen/libfdt/libfdt.h>
> #include <xen/sched.h>
>
> +#include <asm/cpufeature.h>
> #include <asm/current.h>
> #include <asm/guest_access.h>
>
> @@ -44,3 +46,109 @@ int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
>
> return 0;
> }
> +
> +int __init make_cpus_node(const struct domain *d, void *fdt)
> +{
> + int res;
> + const struct dt_device_node *cpus = dt_find_node_by_path("/cpus");
> + unsigned int cpu;
> + u32 timebase_frequency;
uint32_t please.
> + bool frequency_valid;
> + uint32_t *next_phandle = &((struct domain *)d)->arch.next_phandle;
No casting away of const, please.
> + dt_dprintk("Create cpus node\n");
> +
> + if ( !cpus )
> + {
> + dprintk(XENLOG_ERR, "Missing /cpus node in the device tree?\n");
> + return -ENOENT;
> + }
> +
> + frequency_valid = dt_property_read_u32(cpus, "timebase-frequency",
> + &timebase_frequency);
> +
> + res = fdt_begin_node(fdt, "cpus");
> + if ( res )
> + return res;
> +
> + res = fdt_property_cell(fdt, "#address-cells", 1);
> + if ( res )
> + return res;
> +
> + res = fdt_property_cell(fdt, "#size-cells", 0);
> + if ( res )
> + return res;
> +
> + if ( frequency_valid )
> + res = fdt_property_cell(fdt, "timebase-frequency", timebase_frequency);
Handing through a property directly makes me wonder how that's going to
fit with migration. I understand migration may not even be a mid-term
goal, but still.
> + for ( cpu = 0; cpu < d->max_vcpus; cpu++ )
> + {
> + char buf[64];
> + uint32_t reg = cpu_to_fdt32(cpu);
> +
> + snprintf(buf, sizeof(buf), "cpu@%u", cpu);
> + res = fdt_begin_node(fdt, buf);
> + if ( res )
> + return res;
> +
> + res = fdt_property(fdt, "reg", ®, sizeof(reg));
> + if ( res )
> + return res;
> +
> + res = fdt_property_string(fdt, "status", "okay");
> + if ( res )
> + return res;
> +
> + res = fdt_property_string(fdt, "compatible", "riscv");
> + if ( res )
> + return res;
> +
> + BUILD_BUG_ON((sizeof("riscv,") + sizeof_field(struct gstage_mode_desc, name) + 1) >= sizeof(buf));
Nit: Overlong line. Also, why +1? The name field has to include a nul, or
else ...
> + snprintf(buf, sizeof(buf), "riscv,%s", max_gstage_mode->name);
... you can't use plain %s here.
> + res = fdt_property_string(fdt, "mmu-type", buf);
> + if ( res )
> + return res;
> +
> + res = fdt_property_string(fdt, "riscv,isa", guest_isa_str);
> + if ( res )
> + return res;
> +
> + res = fdt_property_string(fdt, "device_type", "cpu");
> + if ( res )
> + return res;
> +
> + res = fdt_begin_node(fdt, "interrupt-controller");
> + if ( res )
> + return res;
> +
> + res = fdt_property_string(fdt, "compatible", "riscv,cpu-intc");
> + if ( res )
> + return res;
> +
> + res = fdt_property_cell(fdt, "#interrupt-cells", 1);
> + if ( res )
> + return res;
> +
> + res = fdt_property(fdt, "interrupt-controller", NULL, 0);
> + if ( res )
> + return res;
> +
> + res = fdt_property_u32(fdt, "phandle", (*next_phandle)++);
> + if ( res )
> + return res;
> +
> + /* end of interrupt-controller */
> + res = fdt_end_node(fdt);
> + if ( res )
> + return res;
> +
> + res = fdt_end_node(fdt);
> + if ( res )
> + return res;
> + }
> +
> + res = fdt_end_node(fdt);
> +
> + return res;
Simply "return fdt_end_node(fdt);"?
Jan