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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

> 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/.

Jan