Re: [PATCH v2 13/15] xen/asm-generic: introduce stub header monitor.h

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2023-11-15 at 14:54 +0200, Oleksii wrote:
> On Wed, 2023-11-15 at 11:00 +0100, Jan Beulich wrote:
> > On 10.11.2023 17:30, Oleksii Kurochko wrote:
> > > --- /dev/null
> > > +++ b/xen/include/asm-generic/monitor.h
> > > @@ -0,0 +1,62 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * include/asm-GENERIC/monitor.h
> > > + *
> > > + * Arch-specific monitor_op domctl handler.
> > > + *
> > > + * Copyright (c) 2015 Tamas K Lengyel (tamas@xxxxxxxxxxxxx)
> > > + * Copyright (c) 2016, Bitdefender S.R.L.
> > > + *
> > > + */
> > > +
> > > +#ifndef __ASM_GENERIC_MONITOR_H__
> > > +#define __ASM_GENERIC_MONITOR_H__
> > > +
> > > +#include <xen/sched.h>
> > 
> > What is this needed for? I expect ...
> > 
> > > +struct xen_domctl_monitor_op;
> > > +
> > > +static inline
> > > +void arch_monitor_allow_userspace(struct domain *d, bool
> > > allow_userspace)
> > 
> > ... struct domain, but since you never de-reference any such
> > pointer,
> > forward-
> > declaring that (just like struct xen_domctl_monitor_op) would do
> > here. Which
> > would leave you with needing at most xen/types.h, but maybe as
> > little
> > as
> > xen/stdbool.h and xen/stdint.h.
> Yes, the reason for " #include <xen/sched.h> " was ' struct domain '.
> Let's switch to forward-declaring.
> 
> Shouldn't it be included <xen/compiler.h> too for inline?
It should be added "#include <xen/errno.h>" because EOPNOTSUPP is used
in arch_monitor_domctl_op.
> 
> ~ Oleksii
> 
> > > +{
> > > +}
> > > +
> > > +static inline
> > > +int arch_monitor_domctl_op(struct domain *d, struct
> > > xen_domctl_monitor_op *mop)
> > > +{
> > > +    /* No arch-specific monitor ops on GENERIC. */
> > > +    return -EOPNOTSUPP;
> > > +}
> > > +
> > > +int arch_monitor_domctl_event(struct domain *d,
> > > +                              struct xen_domctl_monitor_op
> > > *mop);
> > > +
> > > +static inline
> > > +int arch_monitor_init_domain(struct domain *d)
> > > +{
> > > +    /* No arch-specific domain initialization on GENERIC. */
> > > +    return 0;
> > > +}
> > > +
> > > +static inline
> > > +void arch_monitor_cleanup_domain(struct domain *d)
> > > +{
> > > +    /* No arch-specific domain cleanup on GENERIC. */
> > > +}
> > > +
> > > +static inline uint32_t arch_monitor_get_capabilities(struct
> > > domain
> > > *d)
> > > +{
> > > +    return 0;
> > > +}
> > > +
> > > +#endif /* __ASM_GENERIC_MONITOR_H__ */
> > > +
> > > +
> > > +/*
> > > + * Local variables:
> > > + * mode: C
> > > + * c-file-style: BSD
> > > + * c-basic-offset: 4
> > > + * indent-tabs-mode: nil
> > > + * End:
> > > + */
> > 
>