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

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2023-09-18 at 14:38 +0200, Jan Beulich wrote:
> 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.
I think that it will be better to wait for some time not to produce
unnecessary patches.

~ Oleksii