diff mbox series

[v2] virtio: kconfig: memory devices are PCI only

Message ID 20240906101658.514470-1-pbonzini@redhat.com
State New
Headers show
Series [v2] virtio: kconfig: memory devices are PCI only | expand

Commit Message

Paolo Bonzini Sept. 6, 2024, 10:16 a.m. UTC
Virtio memory devices rely on PCI BARs to expose the contents of memory.
Because of this they cannot be used (yet) 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, only allowing
VIRTIO_MEM and VIRTIO_PMEM to be defined if the transport supports it.

Without this patch it is possible to create a configuration with
CONFIG_VIRTIO_PCI=n and CONFIG_VIRTIO_MEM=y, but that causes a
linking 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 | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

David Hildenbrand Sept. 6, 2024, 10:25 a.m. UTC | #1
On 06.09.24 12:16, Paolo Bonzini wrote:
> Virtio memory devices rely on PCI BARs to expose the contents of memory.
> Because of this they cannot be used (yet) 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, only allowing
> VIRTIO_MEM and VIRTIO_PMEM to be defined if the transport supports it.
> 
> Without this patch it is possible to create a configuration with
> CONFIG_VIRTIO_PCI=n and CONFIG_VIRTIO_MEM=y, but that causes a
> linking failure.
> 
> Cc: David Hildenbrand <david@redhat.com>
> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

Thanks!
David Hildenbrand Sept. 9, 2024, 12:01 p.m. UTC | #2
On 06.09.24 12:16, Paolo Bonzini wrote:
> Virtio memory devices rely on PCI BARs to expose the contents of memory.
> Because of this they cannot be used (yet) 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, only allowing
> VIRTIO_MEM and VIRTIO_PMEM to be defined if the transport supports it.
> 
> Without this patch it is possible to create a configuration with
> CONFIG_VIRTIO_PCI=n and CONFIG_VIRTIO_MEM=y, but that causes a
> linking failure.

I'll queue this to

https://github.com/davidhildenbrand/qemu.git mem-next

and drop it if someone beats me to up-streaming it :)
Pankaj Gupta Sept. 25, 2024, 7:39 a.m. UTC | #3
> Virtio memory devices rely on PCI BARs to expose the contents of memory.
> Because of this they cannot be used (yet) 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, only allowing
> VIRTIO_MEM and VIRTIO_PMEM to be defined if the transport supports it.
>
> Without this patch it is possible to create a configuration with
> CONFIG_VIRTIO_PCI=n and CONFIG_VIRTIO_MEM=y, but that causes a
> linking failure.

Just curious what is required to make virtio-mem & virtio-pmem compatible with
virtio-mmio?

Maybe late but still:
Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com>

Thanks,
Pankaj

>
> 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 | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
> index aa63ff7fd41..0afec2ae929 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,10 +36,17 @@ 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
>
> +# selected by the board if it has the required support code
>  config VIRTIO_PMEM_SUPPORTED
>      bool
>
> @@ -46,9 +54,11 @@ config VIRTIO_PMEM
>      bool
>      default y
>      depends on VIRTIO
> +    depends on VIRTIO_MD_SUPPORTED
>      depends on VIRTIO_PMEM_SUPPORTED
>      select VIRTIO_MD
>
> +# selected by the board if it has the required support code
>  config VIRTIO_MEM_SUPPORTED
>      bool
>
> @@ -57,6 +67,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
>
> --
> 2.46.0
>
>
David Hildenbrand Sept. 26, 2024, 8:51 a.m. UTC | #4
On 25.09.24 09:39, Pankaj Gupta wrote:
>> Virtio memory devices rely on PCI BARs to expose the contents of memory.
>> Because of this they cannot be used (yet) 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, only allowing
>> VIRTIO_MEM and VIRTIO_PMEM to be defined if the transport supports it.
>>
>> Without this patch it is possible to create a configuration with
>> CONFIG_VIRTIO_PCI=n and CONFIG_VIRTIO_MEM=y, but that causes a
>> linking failure.
> 
> Just curious what is required to make virtio-mem & virtio-pmem compatible with
> virtio-mmio?

I assume not that much: primarily implementing the virtio-md-mmio 
abstraction, and the virtio-mem-mmio/virtio-pmem-mmio proxy devices. 
Then, it needs to be wired up in the machine hotplug code.

I posted the virtio-ccw variant a couple of days ago [1].

> 
> Maybe late but still:
> Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com>

I already sent a merge request that includes this change. If I have to 
resend it, I'll include that. Thanks!


[1] https://lkml.kernel.org/r/20240910175809.2135596-1-david@redhat.com
diff mbox series

Patch

diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index aa63ff7fd41..0afec2ae929 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,10 +36,17 @@  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
 
+# selected by the board if it has the required support code
 config VIRTIO_PMEM_SUPPORTED
     bool
 
@@ -46,9 +54,11 @@  config VIRTIO_PMEM
     bool
     default y
     depends on VIRTIO
+    depends on VIRTIO_MD_SUPPORTED
     depends on VIRTIO_PMEM_SUPPORTED
     select VIRTIO_MD
 
+# selected by the board if it has the required support code
 config VIRTIO_MEM_SUPPORTED
     bool
 
@@ -57,6 +67,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