| Message ID | c61f930fed46e2312f460333401488af4b0adfc4.1693235841.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | introduce stub directory to storing empty/stub headers | expand |
On 28.08.2023 17:57, Oleksii Kurochko wrote: > asm/vm_event.h is common for ARM and RISC-V so it will be moved to > stubs dir. > > Original asm/vm_event.h from ARM was updated: > * use SPDX-License-Identifier. > * update comment messages of stubs. > * update #ifdef When generalizing such a header, more tidying wants doing imo: > --- /dev/null > +++ b/xen/include/stubs/asm/vm_event.h > @@ -0,0 +1,55 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * vm_event.h: stubs for architecture specific vm_event handling routines > + * > + * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com) > + */ > + > +#ifndef __ASM_STUB_VM_EVENT_H__ > +#define __ASM_STUB_VM_EVENT_H__ > + > +#include <xen/sched.h> > +#include <public/domctl.h> I can't spot why this is being included here. All that's needed ought to be public/vm_event.h, and even that only if we were to continue to use vm_event_response_t in the function definitions (which isn't really necessary). > +static inline int vm_event_init_domain(struct domain *d) > +{ > + /* Nothing to do. */ > + return 0; > +} > + > +static inline void vm_event_cleanup_domain(struct domain *d) > +{ > + memset(&d->monitor, 0, sizeof(d->monitor)); This looks to be the sole reason that xen/sched.h is needed. I question the existence of that field in the first place when this stub is being used. But I guess cleaning that up as well might be going too far. Jan
On Mon, 2023-08-28 at 18:05 +0200, Jan Beulich wrote: > On 28.08.2023 17:57, Oleksii Kurochko wrote: > > asm/vm_event.h is common for ARM and RISC-V so it will be moved to > > stubs dir. > > > > Original asm/vm_event.h from ARM was updated: > > * use SPDX-License-Identifier. > > * update comment messages of stubs. > > * update #ifdef > > When generalizing such a header, more tidying wants doing imo: > > > --- /dev/null > > +++ b/xen/include/stubs/asm/vm_event.h > > @@ -0,0 +1,55 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * vm_event.h: stubs for architecture specific vm_event handling > > routines > > + * > > + * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com) > > + */ > > + > > +#ifndef __ASM_STUB_VM_EVENT_H__ > > +#define __ASM_STUB_VM_EVENT_H__ > > + > > +#include <xen/sched.h> > > +#include <public/domctl.h> > > I can't spot why this is being included here. All that's needed ought > to > be public/vm_event.h, and even that only if we were to continue to You are right. I'll change public/domctl.h to public/vm_event.h. > use > vm_event_response_t in the function definitions (which isn't really > necessary). > > > +static inline int vm_event_init_domain(struct domain *d) > > +{ > > + /* Nothing to do. */ > > + return 0; > > +} > > + > > +static inline void vm_event_cleanup_domain(struct domain *d) > > +{ > > + memset(&d->monitor, 0, sizeof(d->monitor)); > > This looks to be the sole reason that xen/sched.h is needed. I > question > the existence of that field in the first place when this stub is > being > used. But I guess cleaning that up as well might be going too far. What do you mean by the existence of the field? Looking at declaration of struct domain, monitor field always exists. Could we leave initialisation of d->monitor for now? ~ Oleksii
On 29.08.2023 14:14, Oleksii wrote: > On Mon, 2023-08-28 at 18:05 +0200, Jan Beulich wrote: >> On 28.08.2023 17:57, Oleksii Kurochko wrote: >>> +static inline void vm_event_cleanup_domain(struct domain *d) >>> +{ >>> + memset(&d->monitor, 0, sizeof(d->monitor)); >> >> This looks to be the sole reason that xen/sched.h is needed. I >> question >> the existence of that field in the first place when this stub is >> being >> used. But I guess cleaning that up as well might be going too far. > What do you mean by the existence of the field? Looking at declaration > of struct domain, monitor field always exists. Right, and that's what I consider questionable. > Could we leave initialisation of d->monitor for now? As said, asking you to also do that aspect of cleanup is probably going too far, so yes, I guess you can leave that alone. Jan
diff --git a/xen/arch/arm/include/asm/vm_event.h b/xen/arch/arm/include/asm/vm_event.h deleted file mode 100644 index 4d861373b3..0000000000 --- a/xen/arch/arm/include/asm/vm_event.h +++ /dev/null @@ -1,66 +0,0 @@ -/* - * vm_event.h: architecture specific vm_event handling routines - * - * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com) - * - * This program is free software; you can redistribute it and/or modify it - * under the terms and conditions of the GNU General Public License, - * version 2, as published by the Free Software Foundation. - * - * This program is distributed in the hope it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along with - * this program; If not, see <http://www.gnu.org/licenses/>. - */ - -#ifndef __ASM_ARM_VM_EVENT_H__ -#define __ASM_ARM_VM_EVENT_H__ - -#include <xen/sched.h> -#include <public/domctl.h> - -static inline int vm_event_init_domain(struct domain *d) -{ - /* Nothing to do. */ - return 0; -} - -static inline void vm_event_cleanup_domain(struct domain *d) -{ - memset(&d->monitor, 0, sizeof(d->monitor)); -} - -static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v, - vm_event_response_t *rsp) -{ - /* Not supported on ARM. */ -} - -static inline -void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp) -{ - /* Not supported on ARM. */ -} - -static inline -void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp) -{ - /* Not supported on ARM. */ -} - -static inline -void vm_event_sync_event(struct vcpu *v, bool value) -{ - /* Not supported on ARM. */ -} - -static inline -void vm_event_reset_vmtrace(struct vcpu *v) -{ - /* Not supported on ARM. */ -} - -#endif /* __ASM_ARM_VM_EVENT_H__ */ diff --git a/xen/include/stubs/asm/vm_event.h b/xen/include/stubs/asm/vm_event.h new file mode 100644 index 0000000000..6bda6ce7df --- /dev/null +++ b/xen/include/stubs/asm/vm_event.h @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * vm_event.h: stubs for architecture specific vm_event handling routines + * + * Copyright (c) 2015 Tamas K Lengyel (tamas@tklengyel.com) + */ + +#ifndef __ASM_STUB_VM_EVENT_H__ +#define __ASM_STUB_VM_EVENT_H__ + +#include <xen/sched.h> +#include <public/domctl.h> + +static inline int vm_event_init_domain(struct domain *d) +{ + /* Nothing to do. */ + return 0; +} + +static inline void vm_event_cleanup_domain(struct domain *d) +{ + memset(&d->monitor, 0, sizeof(d->monitor)); +} + +static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v, + vm_event_response_t *rsp) +{ + /* Nothing to do. */ +} + +static inline +void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp) +{ + /* Nothing to do. */ +} + +static inline +void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp) +{ + /* Nothing to do. */ +} + +static inline +void vm_event_sync_event(struct vcpu *v, bool value) +{ + /* Nothing to do. */ +} + +static inline +void vm_event_reset_vmtrace(struct vcpu *v) +{ + /* Nothing to do. */ +} + +#endif /* __ASM_STUB_VM_EVENT_H__ */
asm/vm_event.h is common for ARM and RISC-V so it will be moved to stubs dir. Original asm/vm_event.h from ARM was updated: * use SPDX-License-Identifier. * update comment messages of stubs. * update #ifdef Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/arm/include/asm/vm_event.h | 66 ----------------------------- xen/include/stubs/asm/vm_event.h | 55 ++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 66 deletions(-) delete mode 100644 xen/arch/arm/include/asm/vm_event.h create mode 100644 xen/include/stubs/asm/vm_event.h