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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Thu, 2023-12-21 at 08:58 +0100, Jan Beulich wrote:
> On 20.12.2023 21:05, Oleksii wrote:
> > 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?
> 
> I'm still curious about the reason here, but ...
> 
> > 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).
> 
> ... we can't easily remove any existing public header. We can only
> try to
> avoid making the same mistake (even if it's just a minor one) again.
> 
> > 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
> 
> Yes. Still awaiting Shawn's input here as well, though.
Perhaps you missed the email from Shawn:
https://lore.kernel.org/xen-devel/c2f3280e-2208-496b-a0b5-fda1a2076b3a@xxxxxxxxxxxxxxxxxxxxx/

> 
> > 3. Provide an asm-generic version of save.h for Arm, PPC, and RISC-
> > V
> > and use it in public/save.h.
> 
> That's not an option imo - what's under public/ needs to be self-
> contained.
> Stuff there isn't supposed to even know of asm-generic/.
In this case, this is not an option.

~ Oleksii