Hi Jan, > On 3 May 2021, at 3:46 pm, Jan Beulich <jbeul...@suse.com> wrote: > > On 29.04.2021 16:46, Rahul Singh wrote: >> MSI code that implements MSI functionality to support MSI within XEN is >> not usable on ARM. Move the code under CONFIG_HAS_PCI_MSI_INTERCEPT flag >> to gate the code for ARM. >> >> Currently, we have no idea how MSI functionality will be supported for >> other architecture therefore we have decided to move the code under >> CONFIG_PCI_MSI_INTERCEPT. We know this is not the right flag to gate the >> code but to avoid an extra flag we decided to use this. > > My objection remains: Actively putting code under the wrong gating > CONFIG_* is imo quite a bit worse than keeping it under a too wide one > (e.g. CONFIG_X86), if introducing a separate CONFIG_HAS_PCI_MSI is > deemed undesirable for whatever reason. Otherwise every abuse of > CONFIG_PCI_MSI_INTERCEPT ought to get a comment to the effect of this > being an abuse, which in particular for code you move into > xen/drivers/passthrough/msi-intercept.c would end up sufficiently odd. > (As a minor extra remark, putting deliberately misplaced code at the > top of a file rather than at its bottom is likely to add to possible > confusion down the road.) >
I understand that this is not the correct flag to gate the code. If we choose to move the code under CONFIG_X86 there will be #ifdef in the common file "passthrough/pci.c” that I think will make code harder to understand. The only option left is to introduce the new CONFIG_HAS_PCI_MSI option and new non-arch files (msi.c, msi.h). Move all non-intercept-related code to those files. As I mention earlier also this is code and as of now we have no data on how MSI will be supported for non-x86 architecture that’s why we decided it is better to move the code under CONFIG_PCI_MSI_INTERCEPT and later point in time we can modify the code once non-x86 architecture implements MSI functionality in XEN if required. I will move the code at the bottom of the file to avoid confusion. Regards, Rahul > Jan
