Re: [PATCH v2 10/11] vpci: Add initial support for virtual PCI bus topology

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.09.2021 15:16, Oleksandr Andrushchenko wrote:
> 
> On 29.09.21 15:54, Jan Beulich wrote:
>> On 29.09.2021 13:56, Oleksandr Andrushchenko wrote:
>>> On 29.09.21 12:09, Jan Beulich wrote:
>>>> On 29.09.2021 11:03, Oleksandr Andrushchenko wrote:
>>>>> Sorry for top posting, but this is a general question on this 
>>>>> patch/functionality.
>>>>>
>>>>> Do you see we need to gate all this with CONFIG_HAS_VPCI_GUEST_SUPPORT
>>>>> as this renders in somewhat dead code for x86 for now? I do think this 
>>>>> still
>>>>> needs to be in the common code though.
>>>> I agree it wants to live in common code, but I'd still like the code to
>>>> not bloat x86 binaries. Hence yes, I think there want to be
>>>> "if ( !IS_ENABLED() )" early bailout paths or, whenever this isn't
>>>> possible without breaking the build, respective #ifdef-s.
>>> Then it needs to be defined as (xen/drivers/Kconfig):
>>>
>>> config HAS_VPCI_GUEST_SUPPORT
>>>       # vPCI guest support is only enabled for Arm now
>>>       def_bool y if ARM
>>>       depends on HAS_VPCI
>>>
>>> Because it needs to be defined as "y" for Arm with vPCI support.
>>>
>>> Otherwise it breaks the PCI passthrough feature, e.g. it compiles,
>>>
>>> but the resulting binary behaves wrong.
>>>
>>> Do you see this as an acceptable solution?
>> Like all (or at least the majority) of other HAS_*, it ought to be
>> "select"-ed (by arm/Kconfig). Is there a reason this isn't possible?
>> (I don't mind the "depends on", as long as it's clear that it exists
>> solely to allow kconfig to warn about bogus "select"s.)
> 
> There is yet no Kconfig exists (even for Arm) that enables HAS_VPCI,
> 
> thus I can't do that at the moment even if I want to. Yes, this can be 
> deferred
> 
> to the later stage when we enable VPCI for Arm, bit this config option is 
> still
> 
> considered as "common code" as the functionality being added
> 
> to the common code is just gated with CONFIG_HAS_VPCI_GUEST_SUPPORT.
> 
> With this defined now and not later the code seems to be complete and more 
> clean
> 
> as it is seen what is this configuration option and how it is enabled and 
> used in the
> 
> code.
> 
> So, I see no problem if it is defined in this Kconfig and evaluates to "n" 
> for x86
> 
> and eventually will be "y" for Arm when it enables HAS_VPCI.

I'm afraid I don't view this as a reply reflecting that you have
understood what I'm asking for. The primary request is for there to
not be "def_bool y" but just "bool" accompanied by a "select" in
Arm's Kconfig. If HAS_VPCI doesn't exist as an option yet, simply
omit the (questionable) "depends on".

Jan

PS: The more replies I get from you, the more annoying I find the
insertion of blank lines between every two lines of text. Blank
lines are usually used to separate paragraphs. If it is your mail
program which inserts these, can you please try to do something
about this? Thanks.