| Message ID | 20240906073737.493254-1-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio: kconfig: memory devices are PCI only | expand |
On 06.09.24 09:37, Paolo Bonzini wrote: > Virtio memory devices rely on PCI BARs to expose the contents of memory. > Because of this they cannot be used with virtio-mmio or virtio-ccw. In fact Guess what I am working on at this very the moment ;) > the code that is common to virtio-mem and virtio-pmem, which is in > hw/virtio/virtio-md-pci.c, is only included if CONFIG_VIRTIO_PCI is > set. Reproduce the same condition in the Kconfig file. > > Without this patch it is possible to create a configuration with > CONFIG_VIRTIO_PCI=n and CONFIG_VIRTIO_MEM=y, but that causes a > compilation failure. Right. > > Cc: David Hildenbrand <david@redhat.com> > Reported-by: Michael Tokarev <mjt@tls.msk.ru> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/virtio/Kconfig | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig > index aa63ff7fd41..7c554d230d8 100644 > --- a/hw/virtio/Kconfig > +++ b/hw/virtio/Kconfig > @@ -37,6 +37,7 @@ config VIRTIO_CRYPTO > > config VIRTIO_MD > bool > + depends on VIRTIO_PCI > select MEM_DEVICE > > config VIRTIO_PMEM_SUPPORTED > @@ -45,7 +46,7 @@ config VIRTIO_PMEM_SUPPORTED > config VIRTIO_PMEM > bool > default y > - depends on VIRTIO > + depends on VIRTIO_PCI depends on VIRTIO_MD ? > depends on VIRTIO_PMEM_SUPPORTED > select VIRTIO_MD > > @@ -55,7 +56,7 @@ config VIRTIO_MEM_SUPPORTED > config VIRTIO_MEM > bool > default y > - depends on VIRTIO > + depends on VIRTIO_PCI Same here. With CCW support, I can unlock VIRTIO_MD and VIRTIO_MEM_SUPPORTED and it should fly.
>> Cc: David Hildenbrand <david@redhat.com> >> Reported-by: Michael Tokarev <mjt@tls.msk.ru> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> hw/virtio/Kconfig | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig >> index aa63ff7fd41..7c554d230d8 100644 >> --- a/hw/virtio/Kconfig >> +++ b/hw/virtio/Kconfig >> @@ -37,6 +37,7 @@ config VIRTIO_CRYPTO >> >> config VIRTIO_MD >> bool >> + depends on VIRTIO_PCI >> select MEM_DEVICE >> >> config VIRTIO_PMEM_SUPPORTED >> @@ -45,7 +46,7 @@ config VIRTIO_PMEM_SUPPORTED >> config VIRTIO_PMEM >> bool >> default y >> - depends on VIRTIO >> + depends on VIRTIO_PCI > > depends on VIRTIO_MD ? (of course, removing the "select VIRTIO_MD")
On Fri, Sep 6, 2024 at 9:40 AM David Hildenbrand <david@redhat.com> wrote: > On 06.09.24 09:37, Paolo Bonzini wrote: > > Virtio memory devices rely on PCI BARs to expose the contents of memory. > > Because of this they cannot be used with virtio-mmio or virtio-ccw. In fact > > Guess what I am working on at this very the moment ;) Ok, then hardcoding VIRTIO_PCI is not nice. > > @@ -45,7 +46,7 @@ config VIRTIO_PMEM_SUPPORTED > > config VIRTIO_PMEM > > bool > > default y > > - depends on VIRTIO > > + depends on VIRTIO_PCI > > depends on VIRTIO_MD ? No, because VIRTIO_MD is "default n" (and anyway you don't want to enable it by hand in the --without-default-devices case). But something like this could be a good alternative if you plan to support virtio-ccw as well: diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig index aa63ff7fd41..253e7d3f90a 100644 --- a/hw/virtio/Kconfig +++ b/hw/virtio/Kconfig @@ -16,6 +16,7 @@ config VIRTIO_PCI default y if PCI_DEVICES depends on PCI select VIRTIO + select VIRTIO_MD_SUPPORTED config VIRTIO_MMIO bool @@ -35,8 +36,14 @@ config VIRTIO_CRYPTO default y depends on VIRTIO +# not all virtio transports support memory devices; if none does, +# no need to include the code +config VIRTIO_MD_SUPPORTED + bool + config VIRTIO_MD bool + depends on VIRTIO_MD_SUPPORTED select MEM_DEVICE config VIRTIO_PMEM_SUPPORTED @@ -46,6 +51,7 @@ config VIRTIO_PMEM bool default y depends on VIRTIO + depends on VIRTIO_MD_SUPPORTED depends on VIRTIO_PMEM_SUPPORTED select VIRTIO_MD @@ -57,6 +63,7 @@ config VIRTIO_MEM default y depends on VIRTIO depends on LINUX + depends on VIRTIO_MD_SUPPORTED depends on VIRTIO_MEM_SUPPORTED select VIRTIO_MD and then you just need to select VIRTIO_MD_SUPPORTED from VIRTIO_CCW. In the case of PCI there is some board support code as well, which is why VIRTIO_{MEM,PMEM}_SUPPORTED is selected from "config PC", but perhaps in the s390 code you can select those three from VIRTIO_CCW as well. If this looks good I'll send it as v2. Thanks, Paolo
On 06.09.24 10:18, Paolo Bonzini wrote: > On Fri, Sep 6, 2024 at 9:40 AM David Hildenbrand <david@redhat.com> wrote: >> On 06.09.24 09:37, Paolo Bonzini wrote: >>> Virtio memory devices rely on PCI BARs to expose the contents of memory. >>> Because of this they cannot be used with virtio-mmio or virtio-ccw. In fact >> >> Guess what I am working on at this very the moment ;) > > Ok, then hardcoding VIRTIO_PCI is not nice. > >>> @@ -45,7 +46,7 @@ config VIRTIO_PMEM_SUPPORTED >>> config VIRTIO_PMEM >>> bool >>> default y >>> - depends on VIRTIO >>> + depends on VIRTIO_PCI >> >> depends on VIRTIO_MD ? > > No, because VIRTIO_MD is "default n" (and anyway you don't want to > enable it by hand in the --without-default-devices case). Right, that's what I originally tried to achieve. > > But something like this could be a good alternative if you plan to > support virtio-ccw as well: > > diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig > index aa63ff7fd41..253e7d3f90a 100644 > --- a/hw/virtio/Kconfig > +++ b/hw/virtio/Kconfig > @@ -16,6 +16,7 @@ config VIRTIO_PCI > default y if PCI_DEVICES > depends on PCI > select VIRTIO > + select VIRTIO_MD_SUPPORTED > > config VIRTIO_MMIO > bool > @@ -35,8 +36,14 @@ config VIRTIO_CRYPTO > default y > depends on VIRTIO > > +# not all virtio transports support memory devices; if none does, > +# no need to include the code > +config VIRTIO_MD_SUPPORTED > + bool > + > config VIRTIO_MD > bool > + depends on VIRTIO_MD_SUPPORTED > select MEM_DEVICE > > config VIRTIO_PMEM_SUPPORTED > @@ -46,6 +51,7 @@ config VIRTIO_PMEM > bool > default y > depends on VIRTIO > + depends on VIRTIO_MD_SUPPORTED > depends on VIRTIO_PMEM_SUPPORTED > select VIRTIO_MD > > @@ -57,6 +63,7 @@ config VIRTIO_MEM > default y > depends on VIRTIO > depends on LINUX > + depends on VIRTIO_MD_SUPPORTED > depends on VIRTIO_MEM_SUPPORTED > select VIRTIO_MD > > > and then you just need to select VIRTIO_MD_SUPPORTED from VIRTIO_CCW. Sounds good. > In the case of PCI there is some board support code as well, which is > why VIRTIO_{MEM,PMEM}_SUPPORTED is selected from "config PC", but > perhaps in the s390 code you can select those three from VIRTIO_CCW as > well. Yes, only VIRTIO_MEM_SUPPORTED for now. > > If this looks good I'll send it as v2. Thanks!
diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig index aa63ff7fd41..7c554d230d8 100644 --- a/hw/virtio/Kconfig +++ b/hw/virtio/Kconfig @@ -37,6 +37,7 @@ config VIRTIO_CRYPTO config VIRTIO_MD bool + depends on VIRTIO_PCI select MEM_DEVICE config VIRTIO_PMEM_SUPPORTED @@ -45,7 +46,7 @@ config VIRTIO_PMEM_SUPPORTED config VIRTIO_PMEM bool default y - depends on VIRTIO + depends on VIRTIO_PCI depends on VIRTIO_PMEM_SUPPORTED select VIRTIO_MD @@ -55,7 +56,7 @@ config VIRTIO_MEM_SUPPORTED config VIRTIO_MEM bool default y - depends on VIRTIO + depends on VIRTIO_PCI depends on LINUX depends on VIRTIO_MEM_SUPPORTED select VIRTIO_MD
Virtio memory devices rely on PCI BARs to expose the contents of memory. Because of this they cannot be used with virtio-mmio or virtio-ccw. In fact the code that is common to virtio-mem and virtio-pmem, which is in hw/virtio/virtio-md-pci.c, is only included if CONFIG_VIRTIO_PCI is set. Reproduce the same condition in the Kconfig file. Without this patch it is possible to create a configuration with CONFIG_VIRTIO_PCI=n and CONFIG_VIRTIO_MEM=y, but that causes a compilation failure. Cc: David Hildenbrand <david@redhat.com> Reported-by: Michael Tokarev <mjt@tls.msk.ru> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/virtio/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)