Re: [PATCH v5 5/7] xen: ifdef inclusion of <asm/grant_table.h> in <xen/grant_table.h>

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Mon, 2023-12-04 at 10:46 +0100, Jan Beulich wrote:
> On 04.12.2023 10:39, Oleksii wrote:
> > On Mon, 2023-12-04 at 09:41 +0100, Jan Beulich wrote:
> > > On 01.12.2023 21:48, Oleksii Kurochko wrote:
> > > > Ifdef-ing inclusion of <asm/grant_table.h> allows to avoid
> > > > generation of empty <asm/grant_table.h> for cases when
> > > > CONFIG_GRANT_TABLE is not enabled.
> > > > 
> > > > The following changes were done for Arm:
> > > > <asm/grant_table.h> should be included directly because it
> > > > contains
> > > > gnttab_dom0_frames() macros which is unique for Arm and is used
> > > > in
> > > > arch/arm/domain_build.c.
> > > > <asm/grant_table.h> is #ifdef-ed with CONFIG_GRANT_TABLE in
> > > > <xen/grant_table.h> so in case of !CONFIG_GRANT_TABLE
> > > > gnttab_dom0_frames
> > > > won't be available for use in arch/arm/domain_build.c.
> > > > 
> > > > Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > 
> > > Not really, no: In particular ...
If it is comment was addressed to Suggested-by. Then it was added when
we didn't have a discussion about config GRANT_TABLE and depends on
things.
Probably I had to remove it after I started to update Kconfig things.

I am really sorry if I had to remove that before send this patch
version.

> > > 
> > > > --- a/xen/common/Kconfig
> > > > +++ b/xen/common/Kconfig
> > > > @@ -15,6 +15,7 @@ config CORE_PARKING
> > > >  config GRANT_TABLE
> > > >         bool "Grant table support" if EXPERT
> > > >         default y
> > > > +       depends on ARM || X86
> > > 
> > > ... this I explicitly said I consider wrong to add.
> > Then I misunderstood you.
> > 
> > What about to do the same as with MEM_ACCESS config and introduce
> > HAS_GRANT_TABLE?
> 
> That's an option, provided (and I put that under question before)
> there
> realistically can be ports which don't mean to support grant tables.
> You mentioned that things are fine for the dom0less setup you're
> testing,
> but I don't think a fully-functional Xen port makes sense to only
> support
> dom0less. But of course I'm willing to hear arguments to the
> contrary.
Unfortunately, I am not experienced in the depths of Xen, but I used
grant tables in Arm to share frames between dom0 and guest in PV
drivers. It should be another usage of grant tables.

I assume it is possible not to use grant tables and come up with
another solution, but it isn't the best idea, and I don't have any
reason not to use what already exists and works. Are there any cases
where something else, instead of grant tables, is used?

Therefore, I agree that a fully functional Xen port will support only
dom0less, but it can live for a long time only with dom0less. And in
non-dom0less grant tables will be used somewhere sooner or later.

> 
> > Or would it be better just update "depends on" to !RISCV && !PPC?
> 
> Definitely not.
So we have a "weird" situation.

Considering the message above, grant tables are likely to be used
anyway. From this point of view, there is not a lot of sense in
introducing "temporary" HAS_GRANT_TABLE as, at some point, every
architecture will use grant tables ( the same is with "depends on
!RISCV && &!PPC or any other combination ), and HAS_GRANT_TABLE won't
be needed any more except the time when support for new architecture
will be started and it will live without grant tables for some time.

But an introduction of HAS_GRANT_TABLE makes sense ( at least, when the
work on support for new architectures will be started ) for me.

If you ( or anyone else ) don't mind, I'll update the patch with an
introduction of HAS_GRANT_TABLE.

~ Oleksii