Re: [PATCH V3 04/23] xen/ioreq: Make x86's IOREQ feature common

[Oleksandr <olekstysh@xxxxxxxxx>]

On 08.12.20 17:02, Jan Beulich wrote:

Hi Jan

> On 08.12.2020 14:56, Oleksandr wrote:
> > On 08.12.20 11:21, Jan Beulich wrote:
> >
> > Hi Jan
> >
> > > On 07.12.2020 20:43, Oleksandr wrote:
> > > > On 07.12.20 13:41, Jan Beulich wrote:
> > > > > On 30.11.2020 11:31, Oleksandr Tyshchenko wrote:
> > > > > > @@ -38,42 +37,6 @@ int arch_ioreq_server_get_type_addr(const struct domain *d,
> > > > > >                                         uint64_t *addr);
> > > > > >     void arch_ioreq_domain_init(struct domain *d);
> > > > > As already mentioned in an earlier reply: What about these? They
> > > > > shouldn't get declared once per arch. If anything, ones that
> > > > > want to be inline functions can / should remain in the per-arch
> > > > > header.
> > > > Don't entirely get a suggestion. Is the suggestion to make "simple" ones
> > > > inline? Why not, there are a few ones which probably want to be inline,
> > > > such as the following for example:
> > > > - arch_ioreq_domain_init
> > > > - arch_ioreq_server_destroy
> > > > - arch_ioreq_server_destroy_all
> > > > - arch_ioreq_server_map_mem_type (probably)
> > First of all, thank you for the clarification, now your point is clear
> > to me.
> >
> >
> > > Before being able to make a suggestion, I need to have my question
> > > answered: Why do the arch_*() declarations live in the arch header?
> > > They represent a common interface (between common and arch code)
> > > and hence should be declared in exactly one place.
> > I got it, I had a wrong assumption that arch hooks declarations should
> > live in arch code.
> >
> >
> > > It is only at
> > > the point where you/we _consider_ making some of them inline that
> > > moving those (back) to the arch header may make sense. Albeit even
> > > then I'd prefer if only the ones get moved which are expected to
> > > be inline for all arch-es. Others would better have the arch header
> > > indicate to the common one that no declaration is needed (such that
> > > the declaration still remains common for all arch-es using out-of-
> > > line functions).
> > I got it as well.
> >
> > Well, I think, in order to address your comments two options are available:
> > 1. All arch hooks are out-of-line: мove all arch hook declarations to
> > the common header here and modify
> > "[PATCH V3 14/23] arm/ioreq: Introduce arch specific bits for IOREQ/DM
> > features" to make all Arm variants
> > out-of-line (I made them inline since all of them are just stubs).
> > 2. Some of arch hooks are inline: consider which want to be inline (for
> > both arch-es) and place them into arch headers, other ones
> > should remain in the common header.
> >
> > My question is which option is more suitable?
> And the presumably very helpful to you answer is "Depends." It's a
> case by case judgement call in the end.
>
> Sorry, Jan
Thank you for the honest answer. I will use option #1 since all these 
arch hooks are for single-use purpose only.
If indeed there is a need to make some of them inline, I think it could 
be done later on.
--
Regards,

Oleksandr Tyshchenko