Re: [PATCH v2 07/15] xen: generate hypercall interface related code

[Juergen Gross <jgross@xxxxxxxx>]

On 01.11.21 16:20, 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.
>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> V2:
> - split platform_op for doe and compat prefixes (Jan Beulich)
> - add "addline:" directive
> - add priorities to handlers (Jan Beulich)
> ---
>   .gitignore                    |   1 +
>   xen/Makefile                  |  10 ++
>   xen/include/hypercall-defs.c  | 285 +++++++++++++++++++++++++++++++
>   xen/scripts/gen_hypercall.awk | 306 ++++++++++++++++++++++++++++++++++
>   4 files changed, 602 insertions(+)
>   create mode 100644 xen/include/hypercall-defs.c
>   create mode 100644 xen/scripts/gen_hypercall.awk
...

> diff --git a/xen/scripts/gen_hypercall.awk b/xen/scripts/gen_hypercall.awk
> new file mode 100644
> index 0000000000..26017c0900
> --- /dev/null
> +++ b/xen/scripts/gen_hypercall.awk
...

> +    # Generate call sequences and args array contents
> +    for (ca in caller) {
> +        if (caller[ca] != 1)
> +            continue;
> +        for (pl = 1; pl <= n_prios[ca]; pl++)
> +            p_list[pl] = prio_list[ca, pl];
> +        asort(p_list, p_list, "@val_num_asc");
> +        need_mask = 0;
> +        # If any prio but the default one has more than 1 entry we need "mask"
> +        for (pl = 1; pl < n_prios[ca]; pl++) {
> +            if (prios[ca, p_list[pl]] > 1)
> +                need_mask = 1;
> +        }
> +        printf("\n");
> +        printf("#define call_handlers_%s(num, ret, a1, a2, a3, a4, a5) \\\n", 
> ca);
> +        printf("{ \\\n");
> +        if (need_mask)
> +            printf("    uint64_t mask = 1ULL << num; \\\n");
> +        for (pl = 1; pl <= n_prios[ca]; pl++) {
> +            if (prios[ca, p_list[pl]] > 1) {
> +                if (pl < n_prios[ca]) {
> +                    printf("if ( likely(mask & (%s)) ) \\\n", prio_mask[ca, 
> p_list[pl]]);
> +                    printf("    { \\\n");
> +                }
> +                do_switch(ca, p_list[pl]);
> +                if (pl < n_prios[ca])
> +                    printf("    } \\\n");
I've found another optimization for the case of 2 hypercalls with the
same priority: in this case "if ( ) ... else ..." is the better choice.

> +            } else {
> +                for (i = 1; i <= nc; i++)
> +                    if (call[i] == ca && call_prio[i] == p_list[pl]) {
> +                        printf("if ( likely(num == __HYPERVISOR_%s) ) \\\n", 
> fn[call_fn[i]]);
> +                        printf("    { \\\n");
> +                        do_call(call_fn[i], call_p[i]);
> +                        printf("    } \\\n");
> +                    }
> +            }
> +            if (pl < n_prios[ca] || prios[ca, pl] == 1)
> +                printf("    else ");
> +        }
> +        if (prios[ca, p_list[n_prios[ca]]] == 1) {
> +            printf("\\\n");
> +            printf("        ret = -ENOSYS; \\\n");
> +        }
> +        printf("}\n");
Here a "delete p_list;" is missing in order to avoid stale array
elements in the next round.

I'll send an update of this patch.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature