Re: [PATCH v2 07/39] xen/riscv: introduce arch-riscv/hvm/save.h

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Tue, 2023-12-05 at 16:59 +0100, Jan Beulich wrote:
> On 24.11.2023 11:30, Oleksii Kurochko wrote:
> > --- /dev/null
> > +++ b/xen/include/public/arch-riscv/hvm/save.h
> > @@ -0,0 +1,20 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Structure definitions for HVM state that is held by Xen and
> > must
> > + * be saved along with the domain's memory and device-model state.
> > + */
> > +
> > +#ifndef __XEN_PUBLIC_HVM_SAVE_RISCV_H__
> > +#define __XEN_PUBLIC_HVM_SAVE_RISCV_H__
> > +
> > +#endif
> > +
> > +/*
> > + * Local variables:
> > + * mode: C
> > + * c-file-style: "BSD"
> > + * c-basic-offset: 4
> > + * tab-width: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
> 
> Seeing that Arm's is as empty, I wonder why we have it. Julien,
> Stefano?
It seems to exist to satisfy the 'install-tools-public-headers' target:
install: cannot stat 'xen/arch-arm/hvm/*.h': No such file or directory
Makefile:58: recipe for target 'install' failed
make[1]: *** [install] Error 1
make[1]: Leaving directory '/builds/xen-
project/people/olkur/xen/tools/include'
Makefile:44: recipe for target 'install-tools-public-headers' failed

>From tools/include/Makefile:
install: all
...
$(DESTDIR)$(includedir)/xen/arch-arm
        $(INSTALL_DATA) xen/arch-arm/hvm/*.h
$(DESTDIR)$(includedir)/xen/arch-arm/hvm
...

We have the following options:
1. Remove the line with $(INSTALL_DATA) xen/arch-arm/hvm/*.h (only
save.h is now in this folder, which is empty).
2. Don't touch the Arm part, but for PPC and RISC-V, do the following:
#if defined(__i386__) || defined(__x86_64__)
#include "../arch-x86/hvm/save.h"
#elif defined(__arm__) || defined(__aarch64__)
#include "../arch-arm/hvm/save.h"
+#elif defined(__powerpc64__) || defined(__riscv)
+/* no specific header to include */
#else
#error "unsupported architecture"
#endif

3. Provide an asm-generic version of save.h for Arm, PPC, and RISC-V
and use it in public/save.h.

In your opinion, which option would be better?

~ Oleksii
> ....
> > --- a/xen/include/public/hvm/save.h
> > +++ b/xen/include/public/hvm/save.h
> > @@ -91,6 +91,8 @@ DECLARE_HVM_SAVE_TYPE(END, 0, struct
> > hvm_save_end);
> >  #include "../arch-arm/hvm/save.h"
> >  #elif defined(__powerpc64__)
> >  #include "../arch-ppc.h"
> > +#elif defined(__riscv)
> > +#include "../arch-riscv/hvm/save.h"
> >  #else
> >  #error "unsupported architecture"
> >  #endif
> 
> The PPC part here looks bogus altogether. Shawn?
> 
> Jan