|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 10/11] vpci: Add initial support for virtual PCI bus topology
On 29.09.21 17:07, Jan Beulich wrote: > On 29.09.2021 15:49, Oleksandr Andrushchenko wrote: >> >> On 29.09.21 16:23, Jan Beulich wrote: >>> 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". >> I understood that, but was trying to make sure we don't miss >> this option while enabling vPCI on Arm, but ok, I'll have the following: >> config HAS_VPCI >> bool >> >> config HAS_VPCI_GUEST_SUPPORT >> bool >> depends on HAS_VPCI >> and select it for Arm xen/arch/arm/Kconfig > Btw you could also have > > config HAS_VPCI > bool > > config HAS_VPCI_GUEST_SUPPORT > bool > select HAS_VPCI > > which would require arm/Kconfig to only select the latter, while > x86/Kconfig would only select the former. I'll probably leave it as I wrote before, because it then looks like a sub-feature enables the parent feature and doesn't seem right Although it may still look right... > >>> 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. >>> >> I first thought that this is how Thunderbird started showing >> my replies and was also curious about the distance between the lines >> which seemed to be as double-line, but I couldn't delete or edit >> those. I thought this is only visible to me... >> It came out that this was because of some Thunderbird settings which >> made my replies with those double-liners. Hope it is fixed now. > Indeed, thanks - I did not remove any blank lines from context above. > > Jan > > Thank you, Oleksandr
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |