Re: [PATCH V2 04/23] xen/ioreq: Provide alias for the handle_mmio()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.11.2020 09:09, Oleksandr wrote:
> 
> On 11.11.20 09:27, Jan Beulich wrote:
> 
> Hi Jan
> 
>> On 10.11.2020 20:44, Oleksandr wrote:
>>> On 20.10.20 13:38, Paul Durrant wrote:
>>>
>>> Hi Jan, Paul
>>>
>>> Sorry for the late response.
>>>
>>>>> -----Original Message-----
>>>>> From: Jan Beulich <jbeulich@xxxxxxxx>
>>>>> Sent: 20 October 2020 11:05
>>>>> To: paul@xxxxxxx
>>>>> Cc: 'Oleksandr Tyshchenko' <olekstysh@xxxxxxxxx>; 
>>>>> xen-devel@xxxxxxxxxxxxxxxxxxxx; 'Oleksandr
>>>>> Tyshchenko' <oleksandr_tyshchenko@xxxxxxxx>; 'Andrew Cooper' 
>>>>> <andrew.cooper3@xxxxxxxxxx>; 'Roger Pau
>>>>> Monné' <roger.pau@xxxxxxxxxx>; 'Wei Liu' <wl@xxxxxxx>; 'Julien Grall' 
>>>>> <julien@xxxxxxx>; 'Stefano
>>>>> Stabellini' <sstabellini@xxxxxxxxxx>; 'Julien Grall' 
>>>>> <julien.grall@xxxxxxx>
>>>>> Subject: Re: [PATCH V2 04/23] xen/ioreq: Provide alias for the 
>>>>> handle_mmio()
>>>>>
>>>>> On 20.10.2020 11:14, Paul Durrant wrote:
>>>>>>> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of 
>>>>>>> Oleksandr Tyshchenko
>>>>>>> Sent: 15 October 2020 17:44
>>>>>>>
>>>>>>> --- a/xen/include/asm-x86/hvm/ioreq.h
>>>>>>> +++ b/xen/include/asm-x86/hvm/ioreq.h
>>>>>>> @@ -181,6 +181,8 @@ static inline bool arch_hvm_ioreq_destroy(struct 
>>>>>>> domain *d)
>>>>>>>    #define IOREQ_STATUS_UNHANDLED   X86EMUL_UNHANDLEABLE
>>>>>>>    #define IOREQ_STATUS_RETRY       X86EMUL_RETRY
>>>>>>>
>>>>>>> +#define ioreq_complete_mmio   handle_mmio
>>>>>>> +
>>>>>> A #define? Really? Can we not have a static inline?
>>>>> I guess this would require further shuffling: handle_mmio() is
>>>>> an inline function in hvm/emulate.h, and hvm/ioreq.h has no
>>>>> need to include the former (and imo it also shouldn't have).
>>>>>
>>>> I see. I think we need an x86 ioreq.c anyway, to deal with the legacy use 
>>>> of magic pages, so it could be dealt with there instead.
>>> I am afraid I don't entirely understand the required changes. Could you
>>> please clarify where the "inline(?)" ioreq_complete_mmio() should
>>> live? I included hvm/emulate.h here not for the "handle_mmio()" reason
>>> only, but for "struct hvm_emulate_ctxt" also (see arch_io_completion()).
>> I'm sorry, but in the context of this patch there's no use of any
>> struct hvm_emulate_ctxt instance. I'm not going to wade through 23
>> patches to find what you mean.
> 
> Sorry for not being precise here. I meant arch_io_completion() added at [1]

At least some of the inlines you add there are way too large to be
inline functions, imo. But consensus appears to be now to retain a
per-arch ioreq.c anyway.

Jan

> [1] 
> https://patchwork.kernel.org/project/xen-devel/patch/1602780274-29141-2-git-send-email-olekstysh@xxxxxxxxx/
>