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

[Jan Beulich <jbeulich@xxxxxxxx>]

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