[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.