[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V5 14/22] arm/ioreq: Introduce arch specific bits for IOREQ/DM features



Hi Jan,

On 26/01/2021 09:15, Jan Beulich wrote:
On 25.01.2021 20:08, Oleksandr Tyshchenko wrote:
--- a/xen/include/xen/dm.h
+++ b/xen/include/xen/dm.h
@@ -19,6 +19,8 @@
#include <xen/sched.h> +#include <public/hvm/dm_op.h>
+
  struct dmop_args {
      domid_t domid;
      unsigned int nr_bufs;

How come this becomes necessary at this point in the series, when
nothing else in this header changes, and nothing changes in the
public headers at all? Is it perhaps a .c file that needs the
#include instead? Headers shouldn't pull in other headers without
clear need - as indicated in reply to a prior version, we have
way too many bad examples (causing headaches in certain cases),
and we'd like to avoid gaining more. (Oh, I notice you actually
have a post-commit-message remark about this, but then this
patch should be marked RFC until the issue was resolved.)

dm.h contains the following:

struct dmop_args {
    domid_t domid;
    unsigned int nr_bufs;
    /* Reserve enough buf elements for all current hypercalls. */
    struct xen_dm_op_buf buf[2];
};

The struct xen_dm_op_buf is defined public/hvm/dm_op.h. On x86, this will be indirectly included via:
   ->  xen/sched.h
    ->  xen/domain.h
     ->  asm/domain.h
      ->  asm/hvm/domain.h
       ->  public/hvm/dm_op.h

It looks like this was include from asm/hvm/domain.h because, before this series, NR_IO_RANGE_TYPES made use of a DMOP definition.

With this series, the type is now moved in ioreq.h. So I think we may be able to drop the include from asm/hvm/domain.h (this would avoid to include it everywhere...).

I also think we want to include public/hvm/dm_op.h in xen/dm.h because it is included directly by *.c.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.