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

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




Hi Jan.


On 07.01.21 18:49, Oleksandr wrote:

On 07.01.21 17:01, Jan Beulich wrote:

Hi Jan, all

On 07.01.2021 15:38, Oleksandr wrote:
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.
Ah, will investigate. Sorry for not paying enough attention to it.
Yes, IOREQ (I mean "common") ops are 7 out of 18 right now. The
subsequent patch is adding one more DM op - XEN_DMOP_set_irq_level.
There are several PCI related ops which might want to be common in the
future (but I am not sure).
I think, I can say that I have considered the alternative (doing it the
other way around), of course if I got your suggestion for V2 correctly.
Agree, the alternative is more natural, also compat_dm_op() was left in
x86 code. For me the downside is in code duplication. With the
alternative both arches have to duplicate do_dm_op() and "common" part
of dm_op()
(only "switch ( op.op )" is unique).
Yes, this duplication is the main downside.

Now the question is which approach to take ("current" or "alternative")
for me to prepare for V4. Personally, I would be OK with the both (with
a little preference for "alternative").
Same here. I don't think I've seen anyone else voice an opinion.

Well, let's wait a bit for other opinions... @Julien, @Paul what do you think on that?



Also, If we decide to go with the alternative, should the common files
still be named dm.*?
I think this should live in ioreq.c, just like e.g.
iommu_do_pci_domctl() lives in passthrough/pci.c. This would then
allow quite a few things to become static in that file, I believe.

I got it. It seems yes, at least the following could became static:

ioreq_server_create
ioreq_server_get_info
ioreq_server_map_io_range
ioreq_server_unmap_io_range
ioreq_server_set_state
ioreq_server_destroy


Well, I have already pushed V4 with this "alternative" approach, let's discuss there [1].


[1] https://lore.kernel.org/xen-devel/1610488352-18494-10-git-send-email-olekstysh@xxxxxxxxx/T/#mb08f75657f43df869596c5c9c30e2395a9e35c7a


--
Regards,

Oleksandr Tyshchenko




 


Rackspace

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