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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

> But, if we return x86 ioreq.c back I can move arch_io_completion() to it 
> as well as "non-online" ioreq_complete_mmio().
> This will avoid including hvm/emulate.h here. Or I missed something?

I suppose an out-of-line function as kind of a last resort solution
is what Paul had in mind. To be honest I'd prefer to avoid the
extra call layer though, if possible.

Jan