linux-kernel - Re: [PATCH V2 1/2] xen: Update dm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 22 Jul 2023 15:03:05 +0000
From:   Oleksandr Tyshchenko <Oleksandr_Tyshchenko@...m.com>
To:     Viresh Kumar <viresh.kumar@...aro.org>
CC:     Vincent Guittot <vincent.guittot@...aro.org>,
        Alex Bennée <alex.bennee@...aro.org>,
        "stratos-dev@...lists.linaro.org" <stratos-dev@...lists.linaro.org>,
        Erik Schilling <erik.schilling@...aro.org>,
        Manos Pitsidianakis <manos.pitsidianakis@...aro.org>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Juergen Gross <jgross@...e.com>,
        "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Stefano Stabellini <sstabellini@...nel.org>
Subject: Re: [PATCH V2 1/2] xen: Update dm_op.h from Xen public header



On 20.07.23 12:30, Viresh Kumar wrote:

Hello Viresh


> Update the definitions in dm_op.h from Xen public header.

I think, it would be good to mention exact Xen version (commit) we are 
based on.

In general patch looks good to me, just a note.

I compared with Xen's public/hvm/dm_op.h and noticed differences. I 
understand, this cannot be 100% verbatim copy, because of headers 
location, emacs magics, GUEST_HANDLE vs XEN_GUEST_HANDLE. The Linux 
header doesn't contain any aliases the Xen header has for each "struct 
xen_dm_op_xxx", for example ...

[snip]


>
> +/*
> + * XEN_DMOP_create_ioreq_server: Instantiate a new IOREQ Server for a
> + *                               secondary emulator.
> + *
> + * The <id> handed back is unique for target domain. The valur of
> + * <handle_bufioreq> should be one of HVM_IOREQSRV_BUFIOREQ_* defined in
> + * hvm_op.h. If the value is HVM_IOREQSRV_BUFIOREQ_OFF then  the buffered
> + * ioreq ring will not be allocated and hence all emulation requests to
> + * this server will be synchronous.
> + */
> +#define XEN_DMOP_create_ioreq_server 1
> +
> +struct xen_dm_op_create_ioreq_server {
> +    /* IN - should server handle buffered ioreqs */
> +    uint8_t handle_bufioreq;
> +    uint8_t pad[3];
> +    /* OUT - server id */
> +    ioservid_t id;
> +};

... this one:

typedef struct xen_dm_op_create_ioreq_server 
xen_dm_op_create_ioreq_server_t;

And "struct xen_dm_op" down the file uses these aliases inside a union.

I assume, we have to diverge here in order to follow a recommendation
to avoid typedef'ing structs at [1], am I сorrect? Or is there another 
reason?

I think, it would be good to mention a reason in the description.

[1] https://www.kernel.org/doc/html/v6.4/process/coding-style.html#typedefs