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

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

[Paolo Bonzini]

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(-)

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
-- 
2.46.0

Re: [PATCH] virtio: kconfig: memory devices are PCI only

[David Hildenbrand]

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.

-- 
Cheers,

David / dhildenb

Re: [PATCH] virtio: kconfig: memory devices are PCI only

[David Hildenbrand]

>> 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")

-- 
Cheers,

David / dhildenb

Re: [PATCH] virtio: kconfig: memory devices are PCI only

[Paolo Bonzini]

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

Re: [PATCH] virtio: kconfig: memory devices are PCI only

[David Hildenbrand]

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!

-- 
Cheers,

David / dhildenb