On 15.10.2021 14:51, Juergen Gross wrote: > Instead of repeating similar data multiple times use a single source > file and a generator script for producing prototypes and call sequences > of the hypercalls. > > As the script already knows the number of parameters used add generating > a macro for populating an array with the number of parameters per > hypercall.
Isn't that array intended to go away? > --- a/.gitignore > +++ b/.gitignore > @@ -332,10 +332,12 @@ xen/include/asm-x86/asm-macros.h > xen/include/compat/* > xen/include/config/ > xen/include/generated/ > +xen/include//hypercall-defs.i Nit: Stray double slash (unless this has a meaning I'm unaware of). Yet then I wonder: Shouldn't *.i be among the patterns at the top of the file, like *.o is? > @@ -466,6 +468,14 @@ include/asm-$(TARGET_ARCH)/asm-offsets.h: asm-offsets.s > echo ""; \ > echo "#endif") <$< >$@ > > +quiet_cmd_genhyp = GEN $@ > +define cmd_genhyp > + awk -f scripts/gen_hypercall.awk <$< >$@ > +endef > + > +include/xen/hypercall-defs.h: include/hypercall-defs.i > scripts/gen_hypercall.awk FORCE > + $(call if_changed,genhyp) As per patch 5 there are quite a few sources including xen/hypercall.h and hence (in one of the next patches) the header generated here. If this one gets re-generated for a benign reason (i.e. without changing the header), all dependents will get rebuilt for no reason. Use $(move-if-changed ...)? > +prefix: do > +set_timer_op(s_time_t timeout) > +console_io(unsigned int cmd, unsigned int count, char *buffer) > +vm_assist(unsigned int cmd, unsigned int type) > +event_channel_op(int cmd, void *arg) > +mmuext_op(mmuext_op_t *uops, unsigned int count, unsigned int *pdone, > unsigned int foreigndom) > +multicall(multicall_entry_t *call_list, unsigned int nr_calls) > +#ifdef CONFIG_PV > +mmu_update(mmu_update_t *ureqs, unsigned int count, unsigned int *pdone, > unsigned int foreigndom) > +stack_switch(unsigned long ss, unsigned long esp) > +fpu_taskswitch(int set) > +set_debugreg(int reg, unsigned long value) > +get_debugreg(int reg) > +set_segment_base(unsigned int which, unsigned long base) > +mca(xen_mc_t *u_xen_mc) > +set_trap_table(const_trap_info_t *traps) > +set_gdt(xen_ulong_t *frame_list, unsigned int entries) > +set_callbacks(unsigned long event_address, unsigned long failsafe_address, > unsigned long syscall_address) > +update_descriptor(uint64_t gaddr, seg_desc_t desc) > +update_va_mapping(unsigned long va, uint64_t val64, unsigned long flags) > +update_va_mapping_otherdomain(unsigned long va, uint64_t val64, unsigned > long flags, domid_t domid) > +#endif > +#ifdef CONFIG_IOREQ_SERVER > +dm_op(domid_t domid, unsigned int nr_bufs, xen_dm_op_buf_t *bufs) > +#endif > +#ifndef CONFIG_PV_SHIM_EXCLUSIVE > +sysctl(xen_sysctl_t *u_sysctl) > +domctl(xen_domctl_t *u_domctl) > +paging_domctl_cont(xen_domctl_t *u_domctl) > +#endif > +#ifdef CONFIG_HVM > +hvm_op(unsigned long op, void *arg) > +#endif > +#ifdef CONFIG_HYPFS > +hypfs_op(unsigned int cmd, const char *arg1, unsigned long arg2, void *arg3, > unsigned long arg4) > +#endif > +#ifdef CONFIG_X86 > +xenpmu_op(unsigned int op, xen_pmu_params_t *arg) > +#endif > + > +#ifdef CONFIG_PV > +caller: pv64 > +#ifdef CONFIG_PV32 > +caller: pv32 > +#endif > +#endif > +#ifdef CONFIG_HVM > +caller: hvm64 > +#ifdef CONFIG_COMPAT > +caller: hvm32 > +#endif HVM selects COMPAT, so I don't see a point in this inner conditional. > +#endif > +#ifdef CONFIG_ARM > +caller: arm > +#endif > + > +table: pv32 pv64 hvm32 hvm64 arm > +set_trap_table compat do - - - > +mmu_update do do - - - > +set_gdt compat do - - - > +stack_switch do do - - - > +set_callbacks compat do - - - > +fpu_taskswitch do do - - - > +sched_op_compat do do - - dep > +#ifndef CONFIG_PV_SHIM_EXCLUSIVE > +platform_op compat do compat do do > +#endif > +set_debugreg do do - - - > +get_debugreg do do - - - > +update_descriptor compat do - - - > +memory_op compat do hvm hvm do > +multicall compat do compat do do > +update_va_mapping compat do - - - > +set_timer_op compat do compat do - > +event_channel_op_compat do do - - dep > +xen_version compat do compat do do > +console_io do do do do do > +physdev_op_compat compat do - - dep > +#if defined(CONFIG_GRANT_TABLE) || defined(CONFIG_PV_SHIM) > +grant_table_op compat do hvm hvm do > +#endif > +vm_assist do do do do do > +update_va_mapping_otherdomain compat do - - - > +iret compat do - - - > +vcpu_op compat do compat do do > +set_segment_base do do - - - > +#ifdef CONFIG_PV > +mmuext_op compat do compat do - > +#endif > +xsm_op compat do compat do do > +nmi_op compat do - - - > +sched_op compat do compat do do > +callback_op compat do - - - > +#ifdef CONFIG_XENOPROF > +xenoprof_op compat do - - - > +#endif > +event_channel_op do do do do do > +physdev_op compat do hvm hvm do > +#ifdef CONFIG_HVM > +hvm_op do do do do do > +#endif > +#ifndef CONFIG_PV_SHIM_EXCLUSIVE > +sysctl do do do do do > +domctl do do do do do > +#endif > +#ifdef CONFIG_KEXEC > +kexec_op compat do - - - > +#endif > +tmem_op - - - - - > +#ifdef CONFIG_ARGO > +argo_op compat do compat do do > +#endif > +xenpmu_op do do do do - > +#ifdef CONFIG_IOREQ_SERVER > +dm_op compat do compat do do > +#endif > +#ifdef CONFIG_HYPFS > +hypfs_op do do do do do > +#endif > +mca do do - - - > +#ifndef CONFIG_PV_SHIM_EXCLUSIVE > +paging_domctl_cont do do do do - > +#endif I assume the intention here is to sort by hypercall number. This results in 3 PV_SHIM_EXCLUSIVE conditionals when one might do. Just a remark for consideration, not strictly a request to change anything. Jan
