| Message ID | 20200415024356.23751-1-jasowang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V2] vhost: do not enable VHOST_MENU by default | expand |
On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: > We try to keep the defconfig untouched after decoupling CONFIG_VHOST > out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a > ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by > default. Then the defconfigs can keep enabling CONFIG_VHOST_NET > without the caring of CONFIG_VHOST. > > But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even > for the ones that doesn't want vhost. So it actually shifts the > burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is > not set". So this patch tries to enable CONFIG_VHOST explicitly in > defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> (s390) > Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc) > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Michael Ellerman <mpe@ellerman.id.au> > Cc: Heiko Carstens <heiko.carstens@de.ibm.com> > Cc: Vasily Gorbik <gor@linux.ibm.com> > Cc: Christian Borntraeger <borntraeger@de.ibm.com> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > Signed-off-by: Jason Wang <jasowang@redhat.com> I rebased this on top of OABI fix since that seems more orgent to fix. Pushed to my vhost branch pls take a look and if possible test. Thanks! > --- > Change since V1: > - depends on EVENTFD for VHOST > --- > arch/mips/configs/malta_kvm_defconfig | 1 + > arch/powerpc/configs/powernv_defconfig | 1 + > arch/powerpc/configs/ppc64_defconfig | 1 + > arch/powerpc/configs/pseries_defconfig | 1 + > arch/s390/configs/debug_defconfig | 1 + > arch/s390/configs/defconfig | 1 + > drivers/vhost/Kconfig | 26 +++++++++----------------- > 7 files changed, 15 insertions(+), 17 deletions(-) > > diff --git a/arch/mips/configs/malta_kvm_defconfig b/arch/mips/configs/malta_kvm_defconfig > index 8ef612552a19..06f0c7a0ca87 100644 > --- a/arch/mips/configs/malta_kvm_defconfig > +++ b/arch/mips/configs/malta_kvm_defconfig > @@ -18,6 +18,7 @@ CONFIG_PCI=y > CONFIG_VIRTUALIZATION=y > CONFIG_KVM=m > CONFIG_KVM_MIPS_DEBUG_COP0_COUNTERS=y > +CONFIG_VHOST=m > CONFIG_VHOST_NET=m > CONFIG_MODULES=y > CONFIG_MODULE_UNLOAD=y > diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig > index 71749377d164..404245b4594d 100644 > --- a/arch/powerpc/configs/powernv_defconfig > +++ b/arch/powerpc/configs/powernv_defconfig > @@ -346,5 +346,6 @@ CONFIG_CRYPTO_DEV_VMX=y > CONFIG_VIRTUALIZATION=y > CONFIG_KVM_BOOK3S_64=m > CONFIG_KVM_BOOK3S_64_HV=m > +CONFIG_VHOST=m > CONFIG_VHOST_NET=m > CONFIG_PRINTK_TIME=y > diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig > index 7e68cb222c7b..4599fc7be285 100644 > --- a/arch/powerpc/configs/ppc64_defconfig > +++ b/arch/powerpc/configs/ppc64_defconfig > @@ -61,6 +61,7 @@ CONFIG_ELECTRA_CF=y > CONFIG_VIRTUALIZATION=y > CONFIG_KVM_BOOK3S_64=m > CONFIG_KVM_BOOK3S_64_HV=m > +CONFIG_VHOST=m > CONFIG_VHOST_NET=m > CONFIG_OPROFILE=m > CONFIG_KPROBES=y > diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig > index 6b68109e248f..4cad3901b5de 100644 > --- a/arch/powerpc/configs/pseries_defconfig > +++ b/arch/powerpc/configs/pseries_defconfig > @@ -321,5 +321,6 @@ CONFIG_CRYPTO_DEV_VMX=y > CONFIG_VIRTUALIZATION=y > CONFIG_KVM_BOOK3S_64=m > CONFIG_KVM_BOOK3S_64_HV=m > +CONFIG_VHOST=m > CONFIG_VHOST_NET=m > CONFIG_PRINTK_TIME=y > diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig > index 0c86ba19fa2b..6ec6e69630d1 100644 > --- a/arch/s390/configs/debug_defconfig > +++ b/arch/s390/configs/debug_defconfig > @@ -57,6 +57,7 @@ CONFIG_PROTECTED_VIRTUALIZATION_GUEST=y > CONFIG_CMM=m > CONFIG_APPLDATA_BASE=y > CONFIG_KVM=m > +CONFIG_VHOST=m > CONFIG_VHOST_NET=m > CONFIG_VHOST_VSOCK=m > CONFIG_OPROFILE=m > diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig > index 6b27d861a9a3..d1b3bf83d687 100644 > --- a/arch/s390/configs/defconfig > +++ b/arch/s390/configs/defconfig > @@ -57,6 +57,7 @@ CONFIG_PROTECTED_VIRTUALIZATION_GUEST=y > CONFIG_CMM=m > CONFIG_APPLDATA_BASE=y > CONFIG_KVM=m > +CONFIG_VHOST=m > CONFIG_VHOST_NET=m > CONFIG_VHOST_VSOCK=m > CONFIG_OPROFILE=m > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig > index e79cbbdfea45..29f171a53d8a 100644 > --- a/drivers/vhost/Kconfig > +++ b/drivers/vhost/Kconfig > @@ -12,23 +12,19 @@ config VHOST_RING > This option is selected by any driver which needs to access > the host side of a virtio ring. > > -config VHOST > - tristate > +menuconfig VHOST > + tristate "Vhost Devices" > + depends on EVENTFD > select VHOST_IOTLB > help > - This option is selected by any driver which needs to access > - the core of vhost. > + Enable option to support host kernel or hardware accelerator > + for virtio device. > > -menuconfig VHOST_MENU > - bool "VHOST drivers" > - default y > - > -if VHOST_MENU > +if VHOST > > config VHOST_NET > tristate "Host kernel accelerator for virtio net" > - depends on NET && EVENTFD && (TUN || !TUN) && (TAP || !TAP) > - select VHOST > + depends on NET && (TUN || !TUN) && (TAP || !TAP) > ---help--- > This kernel module can be loaded in host kernel to accelerate > guest networking with virtio_net. Not to be confused with virtio_net > @@ -39,8 +35,7 @@ config VHOST_NET > > config VHOST_SCSI > tristate "VHOST_SCSI TCM fabric driver" > - depends on TARGET_CORE && EVENTFD > - select VHOST > + depends on TARGET_CORE > default n > ---help--- > Say M here to enable the vhost_scsi TCM fabric module > @@ -48,8 +43,7 @@ config VHOST_SCSI > > config VHOST_VSOCK > tristate "vhost virtio-vsock driver" > - depends on VSOCKETS && EVENTFD > - select VHOST > + depends on VSOCKETS > select VIRTIO_VSOCKETS_COMMON > default n > ---help--- > @@ -62,8 +56,6 @@ config VHOST_VSOCK > > config VHOST_VDPA > tristate "Vhost driver for vDPA-based backend" > - depends on EVENTFD > - select VHOST > depends on VDPA > help > This kernel module can be loaded in host kernel to accelerate > -- > 2.20.1
On 2020/4/17 上午6:55, Michael S. Tsirkin wrote: > On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: >> We try to keep the defconfig untouched after decoupling CONFIG_VHOST >> out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a >> ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by >> default. Then the defconfigs can keep enabling CONFIG_VHOST_NET >> without the caring of CONFIG_VHOST. >> >> But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even >> for the ones that doesn't want vhost. So it actually shifts the >> burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is >> not set". So this patch tries to enable CONFIG_VHOST explicitly in >> defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. >> >> Acked-by: Christian Borntraeger<borntraeger@de.ibm.com> (s390) >> Acked-by: Michael Ellerman<mpe@ellerman.id.au> (powerpc) >> Cc: Thomas Bogendoerfer<tsbogend@alpha.franken.de> >> Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org> >> Cc: Paul Mackerras<paulus@samba.org> >> Cc: Michael Ellerman<mpe@ellerman.id.au> >> Cc: Heiko Carstens<heiko.carstens@de.ibm.com> >> Cc: Vasily Gorbik<gor@linux.ibm.com> >> Cc: Christian Borntraeger<borntraeger@de.ibm.com> >> Reported-by: Geert Uytterhoeven<geert@linux-m68k.org> >> Signed-off-by: Jason Wang<jasowang@redhat.com> > I rebased this on top of OABI fix since that > seems more orgent to fix. > Pushed to my vhost branch pls take a look and > if possible test. > Thanks! I test this patch by generating the defconfigs that wants vhost_net or vhost_vsock. All looks fine. But having CONFIG_VHOST_DPN=y may end up with the similar situation that this patch want to address. Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another menuconfig for VHOST_RING and do something similar? Thanks
On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: > > On 2020/4/17 上午6:55, Michael S. Tsirkin wrote: > > On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: > > > We try to keep the defconfig untouched after decoupling CONFIG_VHOST > > > out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a > > > ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by > > > default. Then the defconfigs can keep enabling CONFIG_VHOST_NET > > > without the caring of CONFIG_VHOST. > > > > > > But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even > > > for the ones that doesn't want vhost. So it actually shifts the > > > burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is > > > not set". So this patch tries to enable CONFIG_VHOST explicitly in > > > defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. > > > > > > Acked-by: Christian Borntraeger<borntraeger@de.ibm.com> (s390) > > > Acked-by: Michael Ellerman<mpe@ellerman.id.au> (powerpc) > > > Cc: Thomas Bogendoerfer<tsbogend@alpha.franken.de> > > > Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org> > > > Cc: Paul Mackerras<paulus@samba.org> > > > Cc: Michael Ellerman<mpe@ellerman.id.au> > > > Cc: Heiko Carstens<heiko.carstens@de.ibm.com> > > > Cc: Vasily Gorbik<gor@linux.ibm.com> > > > Cc: Christian Borntraeger<borntraeger@de.ibm.com> > > > Reported-by: Geert Uytterhoeven<geert@linux-m68k.org> > > > Signed-off-by: Jason Wang<jasowang@redhat.com> > > I rebased this on top of OABI fix since that > > seems more orgent to fix. > > Pushed to my vhost branch pls take a look and > > if possible test. > > Thanks! > > > I test this patch by generating the defconfigs that wants vhost_net or > vhost_vsock. All looks fine. > > But having CONFIG_VHOST_DPN=y may end up with the similar situation that > this patch want to address. > Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another > menuconfig for VHOST_RING and do something similar? > > Thanks Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just an internal variable for the OABI fix. I kept it separate so it's easy to revert for 5.8. Yes we could squash it into VHOST directly but I don't see how that changes logic at all.
On 2020/4/17 下午2:33, Michael S. Tsirkin wrote: > On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: >> On 2020/4/17 上午6:55, Michael S. Tsirkin wrote: >>> On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: >>>> We try to keep the defconfig untouched after decoupling CONFIG_VHOST >>>> out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a >>>> ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by >>>> default. Then the defconfigs can keep enabling CONFIG_VHOST_NET >>>> without the caring of CONFIG_VHOST. >>>> >>>> But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even >>>> for the ones that doesn't want vhost. So it actually shifts the >>>> burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is >>>> not set". So this patch tries to enable CONFIG_VHOST explicitly in >>>> defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. >>>> >>>> Acked-by: Christian Borntraeger<borntraeger@de.ibm.com> (s390) >>>> Acked-by: Michael Ellerman<mpe@ellerman.id.au> (powerpc) >>>> Cc: Thomas Bogendoerfer<tsbogend@alpha.franken.de> >>>> Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org> >>>> Cc: Paul Mackerras<paulus@samba.org> >>>> Cc: Michael Ellerman<mpe@ellerman.id.au> >>>> Cc: Heiko Carstens<heiko.carstens@de.ibm.com> >>>> Cc: Vasily Gorbik<gor@linux.ibm.com> >>>> Cc: Christian Borntraeger<borntraeger@de.ibm.com> >>>> Reported-by: Geert Uytterhoeven<geert@linux-m68k.org> >>>> Signed-off-by: Jason Wang<jasowang@redhat.com> >>> I rebased this on top of OABI fix since that >>> seems more orgent to fix. >>> Pushed to my vhost branch pls take a look and >>> if possible test. >>> Thanks! >> >> I test this patch by generating the defconfigs that wants vhost_net or >> vhost_vsock. All looks fine. >> >> But having CONFIG_VHOST_DPN=y may end up with the similar situation that >> this patch want to address. >> Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another >> menuconfig for VHOST_RING and do something similar? >> >> Thanks > Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just > an internal variable for the OABI fix. I kept it separate > so it's easy to revert for 5.8. Yes we could squash it into > VHOST directly but I don't see how that changes logic at all. Sorry for being unclear. I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left in the defconfigs. This requires the arch maintainers to add "CONFIG_VHOST_VDPN is not set". (Geert complains about this) Thanks >
On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: > > On 2020/4/17 下午2:33, Michael S. Tsirkin wrote: > > On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: > > > On 2020/4/17 上午6:55, Michael S. Tsirkin wrote: > > > > On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: > > > > > We try to keep the defconfig untouched after decoupling CONFIG_VHOST > > > > > out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a > > > > > ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by > > > > > default. Then the defconfigs can keep enabling CONFIG_VHOST_NET > > > > > without the caring of CONFIG_VHOST. > > > > > > > > > > But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even > > > > > for the ones that doesn't want vhost. So it actually shifts the > > > > > burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is > > > > > not set". So this patch tries to enable CONFIG_VHOST explicitly in > > > > > defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. > > > > > > > > > > Acked-by: Christian Borntraeger<borntraeger@de.ibm.com> (s390) > > > > > Acked-by: Michael Ellerman<mpe@ellerman.id.au> (powerpc) > > > > > Cc: Thomas Bogendoerfer<tsbogend@alpha.franken.de> > > > > > Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org> > > > > > Cc: Paul Mackerras<paulus@samba.org> > > > > > Cc: Michael Ellerman<mpe@ellerman.id.au> > > > > > Cc: Heiko Carstens<heiko.carstens@de.ibm.com> > > > > > Cc: Vasily Gorbik<gor@linux.ibm.com> > > > > > Cc: Christian Borntraeger<borntraeger@de.ibm.com> > > > > > Reported-by: Geert Uytterhoeven<geert@linux-m68k.org> > > > > > Signed-off-by: Jason Wang<jasowang@redhat.com> > > > > I rebased this on top of OABI fix since that > > > > seems more orgent to fix. > > > > Pushed to my vhost branch pls take a look and > > > > if possible test. > > > > Thanks! > > > > > > I test this patch by generating the defconfigs that wants vhost_net or > > > vhost_vsock. All looks fine. > > > > > > But having CONFIG_VHOST_DPN=y may end up with the similar situation that > > > this patch want to address. > > > Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another > > > menuconfig for VHOST_RING and do something similar? > > > > > > Thanks > > Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just > > an internal variable for the OABI fix. I kept it separate > > so it's easy to revert for 5.8. Yes we could squash it into > > VHOST directly but I don't see how that changes logic at all. > > > Sorry for being unclear. > > I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left > in the defconfigs. But who cares? That does not add any code, does it? > This requires the arch maintainers to add > "CONFIG_VHOST_VDPN is not set". (Geert complains about this) > > Thanks > > > >
On 2020/4/17 下午4:29, Michael S. Tsirkin wrote: > On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: >> On 2020/4/17 下午2:33, Michael S. Tsirkin wrote: >>> On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: >>>> On 2020/4/17 上午6:55, Michael S. Tsirkin wrote: >>>>> On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: >>>>>> We try to keep the defconfig untouched after decoupling CONFIG_VHOST >>>>>> out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a >>>>>> ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by >>>>>> default. Then the defconfigs can keep enabling CONFIG_VHOST_NET >>>>>> without the caring of CONFIG_VHOST. >>>>>> >>>>>> But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even >>>>>> for the ones that doesn't want vhost. So it actually shifts the >>>>>> burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is >>>>>> not set". So this patch tries to enable CONFIG_VHOST explicitly in >>>>>> defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. >>>>>> >>>>>> Acked-by: Christian Borntraeger<borntraeger@de.ibm.com> (s390) >>>>>> Acked-by: Michael Ellerman<mpe@ellerman.id.au> (powerpc) >>>>>> Cc: Thomas Bogendoerfer<tsbogend@alpha.franken.de> >>>>>> Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org> >>>>>> Cc: Paul Mackerras<paulus@samba.org> >>>>>> Cc: Michael Ellerman<mpe@ellerman.id.au> >>>>>> Cc: Heiko Carstens<heiko.carstens@de.ibm.com> >>>>>> Cc: Vasily Gorbik<gor@linux.ibm.com> >>>>>> Cc: Christian Borntraeger<borntraeger@de.ibm.com> >>>>>> Reported-by: Geert Uytterhoeven<geert@linux-m68k.org> >>>>>> Signed-off-by: Jason Wang<jasowang@redhat.com> >>>>> I rebased this on top of OABI fix since that >>>>> seems more orgent to fix. >>>>> Pushed to my vhost branch pls take a look and >>>>> if possible test. >>>>> Thanks! >>>> I test this patch by generating the defconfigs that wants vhost_net or >>>> vhost_vsock. All looks fine. >>>> >>>> But having CONFIG_VHOST_DPN=y may end up with the similar situation that >>>> this patch want to address. >>>> Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another >>>> menuconfig for VHOST_RING and do something similar? >>>> >>>> Thanks >>> Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just >>> an internal variable for the OABI fix. I kept it separate >>> so it's easy to revert for 5.8. Yes we could squash it into >>> VHOST directly but I don't see how that changes logic at all. >> >> Sorry for being unclear. >> >> I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left >> in the defconfigs. > But who cares? FYI, please see https://www.spinics.net/lists/kvm/msg212685.html > That does not add any code, does it? It doesn't. Thanks > >> This requires the arch maintainers to add >> "CONFIG_VHOST_VDPN is not set". (Geert complains about this) >> >> Thanks >> >>
On Fri, Apr 17, 2020 at 04:39:49PM +0800, Jason Wang wrote: > > On 2020/4/17 下午4:29, Michael S. Tsirkin wrote: > > On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: > > > On 2020/4/17 下午2:33, Michael S. Tsirkin wrote: > > > > On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: > > > > > On 2020/4/17 上午6:55, Michael S. Tsirkin wrote: > > > > > > On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: > > > > > > > We try to keep the defconfig untouched after decoupling CONFIG_VHOST > > > > > > > out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a > > > > > > > ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by > > > > > > > default. Then the defconfigs can keep enabling CONFIG_VHOST_NET > > > > > > > without the caring of CONFIG_VHOST. > > > > > > > > > > > > > > But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even > > > > > > > for the ones that doesn't want vhost. So it actually shifts the > > > > > > > burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is > > > > > > > not set". So this patch tries to enable CONFIG_VHOST explicitly in > > > > > > > defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. > > > > > > > > > > > > > > Acked-by: Christian Borntraeger<borntraeger@de.ibm.com> (s390) > > > > > > > Acked-by: Michael Ellerman<mpe@ellerman.id.au> (powerpc) > > > > > > > Cc: Thomas Bogendoerfer<tsbogend@alpha.franken.de> > > > > > > > Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org> > > > > > > > Cc: Paul Mackerras<paulus@samba.org> > > > > > > > Cc: Michael Ellerman<mpe@ellerman.id.au> > > > > > > > Cc: Heiko Carstens<heiko.carstens@de.ibm.com> > > > > > > > Cc: Vasily Gorbik<gor@linux.ibm.com> > > > > > > > Cc: Christian Borntraeger<borntraeger@de.ibm.com> > > > > > > > Reported-by: Geert Uytterhoeven<geert@linux-m68k.org> > > > > > > > Signed-off-by: Jason Wang<jasowang@redhat.com> > > > > > > I rebased this on top of OABI fix since that > > > > > > seems more orgent to fix. > > > > > > Pushed to my vhost branch pls take a look and > > > > > > if possible test. > > > > > > Thanks! > > > > > I test this patch by generating the defconfigs that wants vhost_net or > > > > > vhost_vsock. All looks fine. > > > > > > > > > > But having CONFIG_VHOST_DPN=y may end up with the similar situation that > > > > > this patch want to address. > > > > > Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another > > > > > menuconfig for VHOST_RING and do something similar? > > > > > > > > > > Thanks > > > > Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just > > > > an internal variable for the OABI fix. I kept it separate > > > > so it's easy to revert for 5.8. Yes we could squash it into > > > > VHOST directly but I don't see how that changes logic at all. > > > > > > Sorry for being unclear. > > > > > > I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left > > > in the defconfigs. > > But who cares? > > > FYI, please see https://www.spinics.net/lists/kvm/msg212685.html The complaint was not about the symbol IIUC. It was that we caused everyone to build vhost unless they manually disabled it. > > > That does not add any code, does it? > > > It doesn't. > > Thanks > > > > > > > This requires the arch maintainers to add > > > "CONFIG_VHOST_VDPN is not set". (Geert complains about this) > > > > > > Thanks > > > > > >
On 2020/4/17 下午4:46, Michael S. Tsirkin wrote: > On Fri, Apr 17, 2020 at 04:39:49PM +0800, Jason Wang wrote: >> On 2020/4/17 下午4:29, Michael S. Tsirkin wrote: >>> On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: >>>> On 2020/4/17 下午2:33, Michael S. Tsirkin wrote: >>>>> On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: >>>>>> On 2020/4/17 上午6:55, Michael S. Tsirkin wrote: >>>>>>> On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: >>>>>>>> We try to keep the defconfig untouched after decoupling CONFIG_VHOST >>>>>>>> out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a >>>>>>>> ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by >>>>>>>> default. Then the defconfigs can keep enabling CONFIG_VHOST_NET >>>>>>>> without the caring of CONFIG_VHOST. >>>>>>>> >>>>>>>> But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even >>>>>>>> for the ones that doesn't want vhost. So it actually shifts the >>>>>>>> burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is >>>>>>>> not set". So this patch tries to enable CONFIG_VHOST explicitly in >>>>>>>> defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. >>>>>>>> >>>>>>>> Acked-by: Christian Borntraeger<borntraeger@de.ibm.com> (s390) >>>>>>>> Acked-by: Michael Ellerman<mpe@ellerman.id.au> (powerpc) >>>>>>>> Cc: Thomas Bogendoerfer<tsbogend@alpha.franken.de> >>>>>>>> Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org> >>>>>>>> Cc: Paul Mackerras<paulus@samba.org> >>>>>>>> Cc: Michael Ellerman<mpe@ellerman.id.au> >>>>>>>> Cc: Heiko Carstens<heiko.carstens@de.ibm.com> >>>>>>>> Cc: Vasily Gorbik<gor@linux.ibm.com> >>>>>>>> Cc: Christian Borntraeger<borntraeger@de.ibm.com> >>>>>>>> Reported-by: Geert Uytterhoeven<geert@linux-m68k.org> >>>>>>>> Signed-off-by: Jason Wang<jasowang@redhat.com> >>>>>>> I rebased this on top of OABI fix since that >>>>>>> seems more orgent to fix. >>>>>>> Pushed to my vhost branch pls take a look and >>>>>>> if possible test. >>>>>>> Thanks! >>>>>> I test this patch by generating the defconfigs that wants vhost_net or >>>>>> vhost_vsock. All looks fine. >>>>>> >>>>>> But having CONFIG_VHOST_DPN=y may end up with the similar situation that >>>>>> this patch want to address. >>>>>> Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another >>>>>> menuconfig for VHOST_RING and do something similar? >>>>>> >>>>>> Thanks >>>>> Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just >>>>> an internal variable for the OABI fix. I kept it separate >>>>> so it's easy to revert for 5.8. Yes we could squash it into >>>>> VHOST directly but I don't see how that changes logic at all. >>>> Sorry for being unclear. >>>> >>>> I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left >>>> in the defconfigs. >>> But who cares? >> FYI, please seehttps://www.spinics.net/lists/kvm/msg212685.html > The complaint was not about the symbol IIUC. It was that we caused > everyone to build vhost unless they manually disabled it. There could be some misunderstanding here. I thought it's somehow similar: a CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is not set. Thanks >
On Fri, Apr 17, 2020 at 04:51:19PM +0800, Jason Wang wrote: > > On 2020/4/17 下午4:46, Michael S. Tsirkin wrote: > > On Fri, Apr 17, 2020 at 04:39:49PM +0800, Jason Wang wrote: > > > On 2020/4/17 下午4:29, Michael S. Tsirkin wrote: > > > > On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: > > > > > On 2020/4/17 下午2:33, Michael S. Tsirkin wrote: > > > > > > On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: > > > > > > > On 2020/4/17 上午6:55, Michael S. Tsirkin wrote: > > > > > > > > On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: > > > > > > > > > We try to keep the defconfig untouched after decoupling CONFIG_VHOST > > > > > > > > > out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a > > > > > > > > > ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by > > > > > > > > > default. Then the defconfigs can keep enabling CONFIG_VHOST_NET > > > > > > > > > without the caring of CONFIG_VHOST. > > > > > > > > > > > > > > > > > > But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even > > > > > > > > > for the ones that doesn't want vhost. So it actually shifts the > > > > > > > > > burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is > > > > > > > > > not set". So this patch tries to enable CONFIG_VHOST explicitly in > > > > > > > > > defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. > > > > > > > > > > > > > > > > > > Acked-by: Christian Borntraeger<borntraeger@de.ibm.com> (s390) > > > > > > > > > Acked-by: Michael Ellerman<mpe@ellerman.id.au> (powerpc) > > > > > > > > > Cc: Thomas Bogendoerfer<tsbogend@alpha.franken.de> > > > > > > > > > Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org> > > > > > > > > > Cc: Paul Mackerras<paulus@samba.org> > > > > > > > > > Cc: Michael Ellerman<mpe@ellerman.id.au> > > > > > > > > > Cc: Heiko Carstens<heiko.carstens@de.ibm.com> > > > > > > > > > Cc: Vasily Gorbik<gor@linux.ibm.com> > > > > > > > > > Cc: Christian Borntraeger<borntraeger@de.ibm.com> > > > > > > > > > Reported-by: Geert Uytterhoeven<geert@linux-m68k.org> > > > > > > > > > Signed-off-by: Jason Wang<jasowang@redhat.com> > > > > > > > > I rebased this on top of OABI fix since that > > > > > > > > seems more orgent to fix. > > > > > > > > Pushed to my vhost branch pls take a look and > > > > > > > > if possible test. > > > > > > > > Thanks! > > > > > > > I test this patch by generating the defconfigs that wants vhost_net or > > > > > > > vhost_vsock. All looks fine. > > > > > > > > > > > > > > But having CONFIG_VHOST_DPN=y may end up with the similar situation that > > > > > > > this patch want to address. > > > > > > > Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another > > > > > > > menuconfig for VHOST_RING and do something similar? > > > > > > > > > > > > > > Thanks > > > > > > Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just > > > > > > an internal variable for the OABI fix. I kept it separate > > > > > > so it's easy to revert for 5.8. Yes we could squash it into > > > > > > VHOST directly but I don't see how that changes logic at all. > > > > > Sorry for being unclear. > > > > > > > > > > I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left > > > > > in the defconfigs. > > > > But who cares? > > > FYI, please seehttps://www.spinics.net/lists/kvm/msg212685.html > > The complaint was not about the symbol IIUC. It was that we caused > > everyone to build vhost unless they manually disabled it. > > > There could be some misunderstanding here. I thought it's somehow similar: a > CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is > not set. > > Thanks Hmm. So looking at Documentation/kbuild/kconfig-language.rst : Things that merit "default y/m" include: a) A new Kconfig option for something that used to always be built should be "default y". b) A new gatekeeping Kconfig option that hides/shows other Kconfig options (but does not generate any code of its own), should be "default y" so people will see those other options. c) Sub-driver behavior or similar options for a driver that is "default n". This allows you to provide sane defaults. So it looks like VHOST_MENU is actually matching rule b). So what's the problem we are trying to solve with this patch, exactly? Geert could you clarify pls? > > >
On Fri, Apr 17, 2020 at 04:51:19PM +0800, Jason Wang wrote: > > On 2020/4/17 下午4:46, Michael S. Tsirkin wrote: > > On Fri, Apr 17, 2020 at 04:39:49PM +0800, Jason Wang wrote: > > > On 2020/4/17 下午4:29, Michael S. Tsirkin wrote: > > > > On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: > > > > > On 2020/4/17 下午2:33, Michael S. Tsirkin wrote: > > > > > > On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: > > > > > > > On 2020/4/17 上午6:55, Michael S. Tsirkin wrote: > > > > > > > > On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: > > > > > > > > > We try to keep the defconfig untouched after decoupling CONFIG_VHOST > > > > > > > > > out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a > > > > > > > > > ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by > > > > > > > > > default. Then the defconfigs can keep enabling CONFIG_VHOST_NET > > > > > > > > > without the caring of CONFIG_VHOST. > > > > > > > > > > > > > > > > > > But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even > > > > > > > > > for the ones that doesn't want vhost. So it actually shifts the > > > > > > > > > burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is > > > > > > > > > not set". So this patch tries to enable CONFIG_VHOST explicitly in > > > > > > > > > defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. > > > > > > > > > > > > > > > > > > Acked-by: Christian Borntraeger<borntraeger@de.ibm.com> (s390) > > > > > > > > > Acked-by: Michael Ellerman<mpe@ellerman.id.au> (powerpc) > > > > > > > > > Cc: Thomas Bogendoerfer<tsbogend@alpha.franken.de> > > > > > > > > > Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org> > > > > > > > > > Cc: Paul Mackerras<paulus@samba.org> > > > > > > > > > Cc: Michael Ellerman<mpe@ellerman.id.au> > > > > > > > > > Cc: Heiko Carstens<heiko.carstens@de.ibm.com> > > > > > > > > > Cc: Vasily Gorbik<gor@linux.ibm.com> > > > > > > > > > Cc: Christian Borntraeger<borntraeger@de.ibm.com> > > > > > > > > > Reported-by: Geert Uytterhoeven<geert@linux-m68k.org> > > > > > > > > > Signed-off-by: Jason Wang<jasowang@redhat.com> > > > > > > > > I rebased this on top of OABI fix since that > > > > > > > > seems more orgent to fix. > > > > > > > > Pushed to my vhost branch pls take a look and > > > > > > > > if possible test. > > > > > > > > Thanks! > > > > > > > I test this patch by generating the defconfigs that wants vhost_net or > > > > > > > vhost_vsock. All looks fine. > > > > > > > > > > > > > > But having CONFIG_VHOST_DPN=y may end up with the similar situation that > > > > > > > this patch want to address. > > > > > > > Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another > > > > > > > menuconfig for VHOST_RING and do something similar? > > > > > > > > > > > > > > Thanks > > > > > > Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just > > > > > > an internal variable for the OABI fix. I kept it separate > > > > > > so it's easy to revert for 5.8. Yes we could squash it into > > > > > > VHOST directly but I don't see how that changes logic at all. > > > > > Sorry for being unclear. > > > > > > > > > > I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left > > > > > in the defconfigs. > > > > But who cares? > > > FYI, please seehttps://www.spinics.net/lists/kvm/msg212685.html > > The complaint was not about the symbol IIUC. It was that we caused > > everyone to build vhost unless they manually disabled it. > > > There could be some misunderstanding here. I thought it's somehow similar: a > CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is > not set. > > Thanks > BTW do entries with no prompt actually appear in defconfig? > >
Hi Michael, On Fri, Apr 17, 2020 at 10:57 AM Michael S. Tsirkin <mst@redhat.com> wrote: > On Fri, Apr 17, 2020 at 04:51:19PM +0800, Jason Wang wrote: > > On 2020/4/17 下午4:46, Michael S. Tsirkin wrote: > > > On Fri, Apr 17, 2020 at 04:39:49PM +0800, Jason Wang wrote: > > > > On 2020/4/17 下午4:29, Michael S. Tsirkin wrote: > > > > > On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: > > > > > > On 2020/4/17 下午2:33, Michael S. Tsirkin wrote: > > > > > > > On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: > > > > > > > > On 2020/4/17 上午6:55, Michael S. Tsirkin wrote: > > > > > > > > > On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: > > > > > > > > > > We try to keep the defconfig untouched after decoupling CONFIG_VHOST > > > > > > > > > > out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a > > > > > > > > > > ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by > > > > > > > > > > default. Then the defconfigs can keep enabling CONFIG_VHOST_NET > > > > > > > > > > without the caring of CONFIG_VHOST. > > > > > > > > > > > > > > > > > > > > But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even > > > > > > > > > > for the ones that doesn't want vhost. So it actually shifts the > > > > > > > > > > burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is > > > > > > > > > > not set". So this patch tries to enable CONFIG_VHOST explicitly in > > > > > > > > > > defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. > > > > > > > > > > > > > > > > > > > > Acked-by: Christian Borntraeger<borntraeger@de.ibm.com> (s390) > > > > > > > > > > Acked-by: Michael Ellerman<mpe@ellerman.id.au> (powerpc) > > > > > > > > > > Cc: Thomas Bogendoerfer<tsbogend@alpha.franken.de> > > > > > > > > > > Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org> > > > > > > > > > > Cc: Paul Mackerras<paulus@samba.org> > > > > > > > > > > Cc: Michael Ellerman<mpe@ellerman.id.au> > > > > > > > > > > Cc: Heiko Carstens<heiko.carstens@de.ibm.com> > > > > > > > > > > Cc: Vasily Gorbik<gor@linux.ibm.com> > > > > > > > > > > Cc: Christian Borntraeger<borntraeger@de.ibm.com> > > > > > > > > > > Reported-by: Geert Uytterhoeven<geert@linux-m68k.org> > > > > > > > > > > Signed-off-by: Jason Wang<jasowang@redhat.com> > > > > > > > > > I rebased this on top of OABI fix since that > > > > > > > > > seems more orgent to fix. > > > > > > > > > Pushed to my vhost branch pls take a look and > > > > > > > > > if possible test. > > > > > > > > > Thanks! > > > > > > > > I test this patch by generating the defconfigs that wants vhost_net or > > > > > > > > vhost_vsock. All looks fine. > > > > > > > > > > > > > > > > But having CONFIG_VHOST_DPN=y may end up with the similar situation that > > > > > > > > this patch want to address. > > > > > > > > Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another > > > > > > > > menuconfig for VHOST_RING and do something similar? > > > > > > > > > > > > > > > > Thanks > > > > > > > Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just > > > > > > > an internal variable for the OABI fix. I kept it separate > > > > > > > so it's easy to revert for 5.8. Yes we could squash it into > > > > > > > VHOST directly but I don't see how that changes logic at all. > > > > > > Sorry for being unclear. > > > > > > > > > > > > I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left > > > > > > in the defconfigs. > > > > > But who cares? > > > > FYI, please seehttps://www.spinics.net/lists/kvm/msg212685.html > > > The complaint was not about the symbol IIUC. It was that we caused > > > everyone to build vhost unless they manually disabled it. > > > > There could be some misunderstanding here. I thought it's somehow similar: a > > CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is > > not set. > > > > Thanks > > Hmm. So looking at Documentation/kbuild/kconfig-language.rst : > > Things that merit "default y/m" include: > > a) A new Kconfig option for something that used to always be built > should be "default y". > > b) A new gatekeeping Kconfig option that hides/shows other Kconfig > options (but does not generate any code of its own), should be > "default y" so people will see those other options. > > c) Sub-driver behavior or similar options for a driver that is > "default n". This allows you to provide sane defaults. > > > So it looks like VHOST_MENU is actually matching rule b). > So what's the problem we are trying to solve with this patch, exactly? > > Geert could you clarify pls? I can confirm VHOST_MENU is matching rule b), so it is safe to always enable it. Gr{oetje,eeting}s, Geert
On 2020/4/17 下午5:25, Geert Uytterhoeven wrote: > Hi Michael, > > On Fri, Apr 17, 2020 at 10:57 AM Michael S. Tsirkin<mst@redhat.com> wrote: >> On Fri, Apr 17, 2020 at 04:51:19PM +0800, Jason Wang wrote: >>> On 2020/4/17 下午4:46, Michael S. Tsirkin wrote: >>>> On Fri, Apr 17, 2020 at 04:39:49PM +0800, Jason Wang wrote: >>>>> On 2020/4/17 下午4:29, Michael S. Tsirkin wrote: >>>>>> On Fri, Apr 17, 2020 at 03:36:52PM +0800, Jason Wang wrote: >>>>>>> On 2020/4/17 下午2:33, Michael S. Tsirkin wrote: >>>>>>>> On Fri, Apr 17, 2020 at 11:12:14AM +0800, Jason Wang wrote: >>>>>>>>> On 2020/4/17 上午6:55, Michael S. Tsirkin wrote: >>>>>>>>>> On Wed, Apr 15, 2020 at 10:43:56AM +0800, Jason Wang wrote: >>>>>>>>>>> We try to keep the defconfig untouched after decoupling CONFIG_VHOST >>>>>>>>>>> out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a >>>>>>>>>>> ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by >>>>>>>>>>> default. Then the defconfigs can keep enabling CONFIG_VHOST_NET >>>>>>>>>>> without the caring of CONFIG_VHOST. >>>>>>>>>>> >>>>>>>>>>> But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even >>>>>>>>>>> for the ones that doesn't want vhost. So it actually shifts the >>>>>>>>>>> burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is >>>>>>>>>>> not set". So this patch tries to enable CONFIG_VHOST explicitly in >>>>>>>>>>> defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. >>>>>>>>>>> >>>>>>>>>>> Acked-by: Christian Borntraeger<borntraeger@de.ibm.com> (s390) >>>>>>>>>>> Acked-by: Michael Ellerman<mpe@ellerman.id.au> (powerpc) >>>>>>>>>>> Cc: Thomas Bogendoerfer<tsbogend@alpha.franken.de> >>>>>>>>>>> Cc: Benjamin Herrenschmidt<benh@kernel.crashing.org> >>>>>>>>>>> Cc: Paul Mackerras<paulus@samba.org> >>>>>>>>>>> Cc: Michael Ellerman<mpe@ellerman.id.au> >>>>>>>>>>> Cc: Heiko Carstens<heiko.carstens@de.ibm.com> >>>>>>>>>>> Cc: Vasily Gorbik<gor@linux.ibm.com> >>>>>>>>>>> Cc: Christian Borntraeger<borntraeger@de.ibm.com> >>>>>>>>>>> Reported-by: Geert Uytterhoeven<geert@linux-m68k.org> >>>>>>>>>>> Signed-off-by: Jason Wang<jasowang@redhat.com> >>>>>>>>>> I rebased this on top of OABI fix since that >>>>>>>>>> seems more orgent to fix. >>>>>>>>>> Pushed to my vhost branch pls take a look and >>>>>>>>>> if possible test. >>>>>>>>>> Thanks! >>>>>>>>> I test this patch by generating the defconfigs that wants vhost_net or >>>>>>>>> vhost_vsock. All looks fine. >>>>>>>>> >>>>>>>>> But having CONFIG_VHOST_DPN=y may end up with the similar situation that >>>>>>>>> this patch want to address. >>>>>>>>> Maybe we can let CONFIG_VHOST depends on !ARM || AEABI then add another >>>>>>>>> menuconfig for VHOST_RING and do something similar? >>>>>>>>> >>>>>>>>> Thanks >>>>>>>> Sorry I don't understand. After this patch CONFIG_VHOST_DPN is just >>>>>>>> an internal variable for the OABI fix. I kept it separate >>>>>>>> so it's easy to revert for 5.8. Yes we could squash it into >>>>>>>> VHOST directly but I don't see how that changes logic at all. >>>>>>> Sorry for being unclear. >>>>>>> >>>>>>> I meant since it was enabled by default, "CONFIG_VHOST_DPN=y" will be left >>>>>>> in the defconfigs. >>>>>> But who cares? >>>>> FYI, please seehttps://www.spinics.net/lists/kvm/msg212685.html >>>> The complaint was not about the symbol IIUC. It was that we caused >>>> everyone to build vhost unless they manually disabled it. >>> There could be some misunderstanding here. I thought it's somehow similar: a >>> CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is >>> not set. >>> >>> Thanks >> Hmm. So looking at Documentation/kbuild/kconfig-language.rst : >> >> Things that merit "default y/m" include: >> >> a) A new Kconfig option for something that used to always be built >> should be "default y". >> >> b) A new gatekeeping Kconfig option that hides/shows other Kconfig >> options (but does not generate any code of its own), should be >> "default y" so people will see those other options. >> >> c) Sub-driver behavior or similar options for a driver that is >> "default n". This allows you to provide sane defaults. >> >> >> So it looks like VHOST_MENU is actually matching rule b). >> So what's the problem we are trying to solve with this patch, exactly? >> >> Geert could you clarify pls? > I can confirm VHOST_MENU is matching rule b), so it is safe to always > enable it. > > Gr{oetje,eeting}s, > > Geert Right, so I think we can drop this patch. Thanks
On 2020/4/17 下午5:01, Michael S. Tsirkin wrote: >> There could be some misunderstanding here. I thought it's somehow similar: a >> CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is >> not set. >> >> Thanks >> > BTW do entries with no prompt actually appear in defconfig? > Yes. I can see CONFIG_VHOST_DPN=y after make ARCH=m68k defconfig Thanks
On Fri, Apr 17, 2020 at 05:33:56PM +0800, Jason Wang wrote: > > On 2020/4/17 下午5:01, Michael S. Tsirkin wrote: > > > There could be some misunderstanding here. I thought it's somehow similar: a > > > CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is > > > not set. > > > > > > Thanks > > > > > BTW do entries with no prompt actually appear in defconfig? > > > > Yes. I can see CONFIG_VHOST_DPN=y after make ARCH=m68k defconfig You see it in .config right? So that's harmless right?
On 2020/4/17 下午5:38, Michael S. Tsirkin wrote: > On Fri, Apr 17, 2020 at 05:33:56PM +0800, Jason Wang wrote: >> On 2020/4/17 下午5:01, Michael S. Tsirkin wrote: >>>> There could be some misunderstanding here. I thought it's somehow similar: a >>>> CONFIG_VHOST_MENU=y will be left in the defconfigs even if CONFIG_VHOST is >>>> not set. >>>> >>>> Thanks >>>> >>> BTW do entries with no prompt actually appear in defconfig? >>> >> Yes. I can see CONFIG_VHOST_DPN=y after make ARCH=m68k defconfig > You see it in .config right? So that's harmless right? Yes. Thanks
diff --git a/arch/mips/configs/malta_kvm_defconfig b/arch/mips/configs/malta_kvm_defconfig index 8ef612552a19..06f0c7a0ca87 100644 --- a/arch/mips/configs/malta_kvm_defconfig +++ b/arch/mips/configs/malta_kvm_defconfig @@ -18,6 +18,7 @@ CONFIG_PCI=y CONFIG_VIRTUALIZATION=y CONFIG_KVM=m CONFIG_KVM_MIPS_DEBUG_COP0_COUNTERS=y +CONFIG_VHOST=m CONFIG_VHOST_NET=m CONFIG_MODULES=y CONFIG_MODULE_UNLOAD=y diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig index 71749377d164..404245b4594d 100644 --- a/arch/powerpc/configs/powernv_defconfig +++ b/arch/powerpc/configs/powernv_defconfig @@ -346,5 +346,6 @@ CONFIG_CRYPTO_DEV_VMX=y CONFIG_VIRTUALIZATION=y CONFIG_KVM_BOOK3S_64=m CONFIG_KVM_BOOK3S_64_HV=m +CONFIG_VHOST=m CONFIG_VHOST_NET=m CONFIG_PRINTK_TIME=y diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig index 7e68cb222c7b..4599fc7be285 100644 --- a/arch/powerpc/configs/ppc64_defconfig +++ b/arch/powerpc/configs/ppc64_defconfig @@ -61,6 +61,7 @@ CONFIG_ELECTRA_CF=y CONFIG_VIRTUALIZATION=y CONFIG_KVM_BOOK3S_64=m CONFIG_KVM_BOOK3S_64_HV=m +CONFIG_VHOST=m CONFIG_VHOST_NET=m CONFIG_OPROFILE=m CONFIG_KPROBES=y diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig index 6b68109e248f..4cad3901b5de 100644 --- a/arch/powerpc/configs/pseries_defconfig +++ b/arch/powerpc/configs/pseries_defconfig @@ -321,5 +321,6 @@ CONFIG_CRYPTO_DEV_VMX=y CONFIG_VIRTUALIZATION=y CONFIG_KVM_BOOK3S_64=m CONFIG_KVM_BOOK3S_64_HV=m +CONFIG_VHOST=m CONFIG_VHOST_NET=m CONFIG_PRINTK_TIME=y diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig index 0c86ba19fa2b..6ec6e69630d1 100644 --- a/arch/s390/configs/debug_defconfig +++ b/arch/s390/configs/debug_defconfig @@ -57,6 +57,7 @@ CONFIG_PROTECTED_VIRTUALIZATION_GUEST=y CONFIG_CMM=m CONFIG_APPLDATA_BASE=y CONFIG_KVM=m +CONFIG_VHOST=m CONFIG_VHOST_NET=m CONFIG_VHOST_VSOCK=m CONFIG_OPROFILE=m diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig index 6b27d861a9a3..d1b3bf83d687 100644 --- a/arch/s390/configs/defconfig +++ b/arch/s390/configs/defconfig @@ -57,6 +57,7 @@ CONFIG_PROTECTED_VIRTUALIZATION_GUEST=y CONFIG_CMM=m CONFIG_APPLDATA_BASE=y CONFIG_KVM=m +CONFIG_VHOST=m CONFIG_VHOST_NET=m CONFIG_VHOST_VSOCK=m CONFIG_OPROFILE=m diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index e79cbbdfea45..29f171a53d8a 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -12,23 +12,19 @@ config VHOST_RING This option is selected by any driver which needs to access the host side of a virtio ring. -config VHOST - tristate +menuconfig VHOST + tristate "Vhost Devices" + depends on EVENTFD select VHOST_IOTLB help - This option is selected by any driver which needs to access - the core of vhost. + Enable option to support host kernel or hardware accelerator + for virtio device. -menuconfig VHOST_MENU - bool "VHOST drivers" - default y - -if VHOST_MENU +if VHOST config VHOST_NET tristate "Host kernel accelerator for virtio net" - depends on NET && EVENTFD && (TUN || !TUN) && (TAP || !TAP) - select VHOST + depends on NET && (TUN || !TUN) && (TAP || !TAP) ---help--- This kernel module can be loaded in host kernel to accelerate guest networking with virtio_net. Not to be confused with virtio_net @@ -39,8 +35,7 @@ config VHOST_NET config VHOST_SCSI tristate "VHOST_SCSI TCM fabric driver" - depends on TARGET_CORE && EVENTFD - select VHOST + depends on TARGET_CORE default n ---help--- Say M here to enable the vhost_scsi TCM fabric module @@ -48,8 +43,7 @@ config VHOST_SCSI config VHOST_VSOCK tristate "vhost virtio-vsock driver" - depends on VSOCKETS && EVENTFD - select VHOST + depends on VSOCKETS select VIRTIO_VSOCKETS_COMMON default n ---help--- @@ -62,8 +56,6 @@ config VHOST_VSOCK config VHOST_VDPA tristate "Vhost driver for vDPA-based backend" - depends on EVENTFD - select VHOST depends on VDPA help This kernel module can be loaded in host kernel to accelerate
We try to keep the defconfig untouched after decoupling CONFIG_VHOST out of CONFIG_VIRTUALIZATION in commit 20c384f1ea1a ("vhost: refine vhost and vringh kconfig") by enabling VHOST_MENU by default. Then the defconfigs can keep enabling CONFIG_VHOST_NET without the caring of CONFIG_VHOST. But this will leave a "CONFIG_VHOST_MENU=y" in all defconfigs and even for the ones that doesn't want vhost. So it actually shifts the burdens to the maintainers of all other to add "CONFIG_VHOST_MENU is not set". So this patch tries to enable CONFIG_VHOST explicitly in defconfigs that enables CONFIG_VHOST_NET and CONFIG_VHOST_VSOCK. Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> (s390) Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc) Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Heiko Carstens <heiko.carstens@de.ibm.com> Cc: Vasily Gorbik <gor@linux.ibm.com> Cc: Christian Borntraeger <borntraeger@de.ibm.com> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> Signed-off-by: Jason Wang <jasowang@redhat.com> --- Change since V1: - depends on EVENTFD for VHOST --- arch/mips/configs/malta_kvm_defconfig | 1 + arch/powerpc/configs/powernv_defconfig | 1 + arch/powerpc/configs/ppc64_defconfig | 1 + arch/powerpc/configs/pseries_defconfig | 1 + arch/s390/configs/debug_defconfig | 1 + arch/s390/configs/defconfig | 1 + drivers/vhost/Kconfig | 26 +++++++++----------------- 7 files changed, 15 insertions(+), 17 deletions(-)