Re: [PATCH v1 00/29] Introduce stub headers necessary for full Xen build

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.09.2023 14:05, Oleksii wrote:
> On Mon, 2023-09-18 at 11:29 +0200, Jan Beulich wrote:
>> On 18.09.2023 10:51, Oleksii wrote:
>>> On Thu, 2023-09-14 at 17:08 +0200, Jan Beulich wrote:
>>>> On 14.09.2023 16:56, Oleksii Kurochko wrote:
>>>>> Based on two patch series [1] and [2], the idea of which is to
>>>>> provide minimal
>>>>> amount of things for a complete Xen build, a large amount of
>>>>> headers are the same
>>>>> or almost the same, so it makes sense to move them to asm-
>>>>> generic.
>>>>>
>>>>> Also, providing such stub headers should help future
>>>>> architectures
>>>>> to add
>>>>> a full Xen build.
>>>>>
>>>>> [1]
>>>>> https://lore.kernel.org/xen-devel/cover.1694543103.git.sanastasio@xxxxxxxxxxxxxxxxxxxxx/
>>>>> [2]
>>>>> https://lore.kernel.org/xen-devel/cover.1692181079.git.oleksii.kurochko@xxxxxxxxx/
>>>>>
>>>>> Oleksii Kurochko (29):
>>>>>   xen/asm-generic: introduce stub header spinlock.h
>>>>
>>>> At the example of this, personally I think this goes too far.
>>>> Headers
>>>> in
>>>> asm-generic should be for the case where an arch elects to not
>>>> implement
>>>> certain functionality. Clearly spinlocks are required uniformly.
>>> It makes sense. Then I will back to the option [2] where I
>>> introduced
>>> all this headers as part of RISC-V architecture. 
>>
>> You did see though that in a reply to my own mail I said I take back
>> the
>> comment, at least as far as this header (and perhaps several others)
>> are
>> concerned.
>>
> I missed that comment on the patch about spinlock.
> 
> Well, then, I don't fully understand the criteria.
> 
> What about empty headers or temporary empty headers?
> 
> For example, asm/xenoprof.h is empty for all arches except x86, so it
> is a good candidate for asm-generic.

That's an example where I think it is wrong (or at least unnecessary) for
the xen/ header to include the asm/ one irrespective of the controlling
CONFIG_* setting. From what I can tell common code would build fine with
the #include moved; x86 code may require an adjustment or two. IOW this
is a case where I think preferably presence of an arch header was
required only when XENOPROF can actually be yet to y in Kconfig.

> But asm/grant_table.h is empty for PPC and RISC-V for now but won't be
> empty in the future. Does it make sense to put them to asm-generic? The
> only benefit I see is that in future architecture if they follow the
> same way of adding support for the arch to Xen, they will face the same
> issue: building full Xen requires this empty header.

Here I can see different ways of looking at it. Personally I'd prefer
stub headers to be used only if, for the foreseeable future, they are
intended to remain in use. grant_table.h pretty clearly doesn't fall in
this category. (You may want to peek at what's being done on the PPC
side. Nevertheless some of what's done there could likely benefit from
what you're doing here.)

> So, should I wait for some time on other patches of the patch series?

Well, afaic I'd prefer if I got a chance to look over at least some more
of the patches in this series. But you're of course free to submit a v2
at any time.

Jan