Re: [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.11.2023 23:21, Shawn Anastasio wrote:
> On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
>> --- a/xen/arch/ppc/include/asm/monitor.h
>> +++ /dev/null
>> @@ -1,43 +0,0 @@
>> -/* SPDX-License-Identifier: GPL-2.0-only */
>> -/* Derived from xen/arch/arm/include/asm/monitor.h */
>> -#ifndef __ASM_PPC_MONITOR_H__
>> -#define __ASM_PPC_MONITOR_H__
>> -
>> -#include <public/domctl.h>
>> -#include <xen/errno.h>
>> -
>> -static inline
>> -void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
>> -{
>> -}
>> -
>> -static inline
>> -int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op 
>> *mop)
>> -{
>> -    /* No arch-specific monitor ops on PPC. */
>> -    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 PPC. */
>> -    return 0;
>> -}
>> -
>> -static inline
>> -void arch_monitor_cleanup_domain(struct domain *d)
>> -{
>> -    /* No arch-specific domain cleanup on PPC. */
>> -}
>> -
>> -static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>> -{
>> -    BUG_ON("unimplemented");
> 
> I'm not sure how I feel about this assertion being dropped in the
> generic header. In general my philosophy when creating these stub
> headers was to insert as many of these assertions as possible so we
> don't end up in a scenario where during early bringup I miss a function
> that was originally stubbed but ought to be implemented eventually.
> 
> Looking at ARM's monitor.h too, it seems that this function is the only
> one that differs from the generic/stub version. I'm wondering if it
> would make sense to leave this function out of the generic header, to be
> defined by the arch similar to what you've done with some other
> functions in this series. That would also allow ARM to be brought in to
> using the generic variant.

Yet then where would that function live, if not in arch/*/include/asm/monitor.h?

Jan