[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

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
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:



Oleksandr Tyshchenko



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