Re: [PATCH V3 09/23] xen/dm: Make x86's DM feature common

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.12.2020 21:23, Oleksandr wrote:
> On 07.12.20 14:08, Jan Beulich wrote:
>> On 30.11.2020 11:31, Oleksandr Tyshchenko wrote:
>>> From: Julien Grall <julien.grall@xxxxxxx>
>>>
>>> As a lot of x86 code can be re-used on Arm later on, this patch
>>> splits devicemodel support into common and arch specific parts.
>>>
>>> The common DM feature is supposed to be built with IOREQ_SERVER
>>> option enabled (as well as the IOREQ feature), which is selected
>>> for x86's config HVM for now.
>>>
>>> Also update XSM code a bit to let DM op be used on Arm.
>>>
>>> This support is going to be used on Arm to be able run device
>>> emulator outside of Xen hypervisor.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>>>
>>> ---
>>> Please note, this is a split/cleanup/hardening of Julien's PoC:
>>> "Add support for Guest IO forwarding to a device emulator"
>>>
>>> Changes RFC -> V1:
>>>     - update XSM, related changes were pulled from:
>>>       [RFC PATCH V1 04/12] xen/arm: Introduce arch specific bits for 
>>> IOREQ/DM features
>>>
>>> Changes V1 -> V2:
>>>     - update the author of a patch
>>>     - update patch description
>>>     - introduce xen/dm.h and move definitions here
>>>
>>> Changes V2 -> V3:
>>>     - no changes
>> And my concern regarding the common vs arch nesting also hasn't
>> changed.
> 
> 
> I am sorry, I might misread your comment, but I failed to see any 
> obvious to me request(s) for changes.
> I have just re-read previous discussion...
> So the question about considering doing it the other way around (top 
> level dm-op handling arch-specific
> and call into e.g. ioreq_server_dm_op() for otherwise unhandled ops) is 
> exactly a concern which I should have addressed?

Well, on v2 you replied you didn't consider the alternative. I would
have expected that you would at least go through this consideration
process, and see whether there are better reasons to stick with the
apparently backwards arrangement than to change to the more
conventional one. If there are such reasons, I would expect them to
be called out in reply and perhaps also in the commit message; the
latter because down the road more people may wonder why the more
narrow / special set of cases gets handled at a higher layer than
the wider set of remaining ones, and they would then be able to find
an explanation without having to resort to searching through list
archives.

Jan