Re: [PATCH v4 3/3] xen/pci: Refactor MSI code that implements MSI functionality within XEN

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Jan,

> On 3 May 2021, at 3:46 pm, Jan Beulich <jbeulich@xxxxxxxx> 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