[Qemu-devel] [PATCH] virtio-vga: only enable for specific boards

Paolo Bonzini posted 1 patch 5 years, 1 month ago
Failed in applying to current master (apply log)
hw/display/Kconfig | 2 +-
hw/hppa/Kconfig    | 1 +
hw/i386/Kconfig    | 1 +
hw/ppc/Kconfig     | 1 +
4 files changed, 4 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH] virtio-vga: only enable for specific boards
Posted by Paolo Bonzini 5 years, 1 month ago
When virtio-vga was added, the intention was to only support it for
those machines where the firmware does not know about virtio-gpu,
and supported VGA legacy hardware before virtio-{gpu,vga} were
introduced.

The Kconfig switch however enabled virtio-vga for all machines with
a PCI bus, and libvirt then prefers it even on hardware where
virtio-gpu would be preferrable.  At least for now, only enable
virtio-vga for PC, hppa and pSeries machines, as was the case
before Kconfig dependencies were introduced.

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/display/Kconfig | 2 +-
 hw/hppa/Kconfig    | 1 +
 hw/i386/Kconfig    | 1 +
 hw/ppc/Kconfig     | 1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 86c1d544c5..72be57a403 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -100,7 +100,7 @@ config VIRTIO_GPU
 
 config VIRTIO_VGA
     bool
-    default y if PCI_DEVICES
+    # defaults to "N", enabled by specific boards
     depends on VIRTIO_PCI
     select VGA
 
diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
index 7334f57081..6e5d74a825 100644
--- a/hw/hppa/Kconfig
+++ b/hw/hppa/Kconfig
@@ -2,6 +2,7 @@ config DINO
     bool
     imply PCI_DEVICES
     imply E1000_PCI
+    imply VIRTIO_VGA
     select PCI
     select SERIAL
     select ISA_BUS
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 8e8444dbf9..a6aed7c131 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -19,6 +19,7 @@ config PC
     imply TPM_CRB
     imply TPM_TIS
     imply VGA_PCI
+    imply VIRTIO_VGA
     select FDC
     select I8259
     select I8254
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index ae07b4da63..a3465155f0 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -2,6 +2,7 @@ config PSERIES
     bool
     imply PCI_DEVICES
     imply TEST_DEVICES
+    imply VIRTIO_VGA
     select DIMM
     select PCI
     select SPAPR_VSCSI
-- 
2.20.1


Re: [Qemu-devel] [PATCH] virtio-vga: only enable for specific boards
Posted by Thomas Huth 5 years, 1 month ago
On 21/03/2019 15.29, Paolo Bonzini wrote:
> When virtio-vga was added, the intention was to only support it for
> those machines where the firmware does not know about virtio-gpu,
> and supported VGA legacy hardware before virtio-{gpu,vga} were
> introduced.
> 
> The Kconfig switch however enabled virtio-vga for all machines with
> a PCI bus, and libvirt then prefers it even on hardware where
> virtio-gpu would be preferrable.  At least for now, only enable
> virtio-vga for PC, hppa and pSeries machines, as was the case
> before Kconfig dependencies were introduced.
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/display/Kconfig | 2 +-
>  hw/hppa/Kconfig    | 1 +
>  hw/i386/Kconfig    | 1 +
>  hw/ppc/Kconfig     | 1 +
>  4 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index 86c1d544c5..72be57a403 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -100,7 +100,7 @@ config VIRTIO_GPU
>  
>  config VIRTIO_VGA
>      bool
> -    default y if PCI_DEVICES
> +    # defaults to "N", enabled by specific boards
>      depends on VIRTIO_PCI
>      select VGA
>  
> diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
> index 7334f57081..6e5d74a825 100644
> --- a/hw/hppa/Kconfig
> +++ b/hw/hppa/Kconfig
> @@ -2,6 +2,7 @@ config DINO
>      bool
>      imply PCI_DEVICES
>      imply E1000_PCI
> +    imply VIRTIO_VGA
>      select PCI
>      select SERIAL
>      select ISA_BUS
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index 8e8444dbf9..a6aed7c131 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -19,6 +19,7 @@ config PC
>      imply TPM_CRB
>      imply TPM_TIS
>      imply VGA_PCI
> +    imply VIRTIO_VGA
>      select FDC
>      select I8259
>      select I8254
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index ae07b4da63..a3465155f0 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -2,6 +2,7 @@ config PSERIES
>      bool
>      imply PCI_DEVICES
>      imply TEST_DEVICES
> +    imply VIRTIO_VGA
>      select DIMM
>      select PCI
>      select SPAPR_VSCSI

Reviewed-by: Thomas Huth <thuth@redhat.com>


Re: [Qemu-devel] [PATCH] virtio-vga: only enable for specific boards
Posted by Peter Maydell 5 years, 1 month ago
On Thu, 21 Mar 2019 at 14:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> When virtio-vga was added, the intention was to only support it for
> those machines where the firmware does not know about virtio-gpu,
> and supported VGA legacy hardware before virtio-{gpu,vga} were
> introduced.
>
> The Kconfig switch however enabled virtio-vga for all machines with
> a PCI bus, and libvirt then prefers it even on hardware where
> virtio-gpu would be preferrable.  At least for now, only enable
> virtio-vga for PC, hppa and pSeries machines, as was the case
> before Kconfig dependencies were introduced.

Do we have a reference to a bug report against libvirt for this?

> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/display/Kconfig | 2 +-
>  hw/hppa/Kconfig    | 1 +
>  hw/i386/Kconfig    | 1 +
>  hw/ppc/Kconfig     | 1 +
>  4 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index 86c1d544c5..72be57a403 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -100,7 +100,7 @@ config VIRTIO_GPU
>
>  config VIRTIO_VGA
>      bool
> -    default y if PCI_DEVICES
> +    # defaults to "N", enabled by specific boards
>      depends on VIRTIO_PCI
>      select VGA

Could we have a comment that says what the criterion for
a board needing to enable it is, so that if we add new
boards in future we know whether they need to enable it or not?
(If the answer is "never for any new board", that's useful
to say.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] virtio-vga: only enable for specific boards
Posted by Laszlo Ersek 5 years, 1 month ago
On 03/21/19 15:29, Paolo Bonzini wrote:
> When virtio-vga was added, the intention was to only support it for
> those machines where the firmware does not know about virtio-gpu,
> and supported VGA legacy hardware before virtio-{gpu,vga} were
> introduced.
> 
> The Kconfig switch however enabled virtio-vga for all machines with
> a PCI bus, and libvirt then prefers it even on hardware where
> virtio-gpu would be preferrable.  At least for now, only enable
> virtio-vga for PC, hppa and pSeries machines, as was the case
> before Kconfig dependencies were introduced.
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/display/Kconfig | 2 +-
>  hw/hppa/Kconfig    | 1 +
>  hw/i386/Kconfig    | 1 +
>  hw/ppc/Kconfig     | 1 +
>  4 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> index 86c1d544c5..72be57a403 100644
> --- a/hw/display/Kconfig
> +++ b/hw/display/Kconfig
> @@ -100,7 +100,7 @@ config VIRTIO_GPU
>  
>  config VIRTIO_VGA
>      bool
> -    default y if PCI_DEVICES
> +    # defaults to "N", enabled by specific boards
>      depends on VIRTIO_PCI
>      select VGA
>  
> diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
> index 7334f57081..6e5d74a825 100644
> --- a/hw/hppa/Kconfig
> +++ b/hw/hppa/Kconfig
> @@ -2,6 +2,7 @@ config DINO
>      bool
>      imply PCI_DEVICES
>      imply E1000_PCI
> +    imply VIRTIO_VGA
>      select PCI
>      select SERIAL
>      select ISA_BUS
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index 8e8444dbf9..a6aed7c131 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -19,6 +19,7 @@ config PC
>      imply TPM_CRB
>      imply TPM_TIS
>      imply VGA_PCI
> +    imply VIRTIO_VGA
>      select FDC
>      select I8259
>      select I8254
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index ae07b4da63..a3465155f0 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -2,6 +2,7 @@ config PSERIES
>      bool
>      imply PCI_DEVICES
>      imply TEST_DEVICES
> +    imply VIRTIO_VGA
>      select DIMM
>      select PCI
>      select SPAPR_VSCSI
> 

I tried to apply this for testing, on top of
c692931cda9dc6cbc16b89d5094a725a10dbb641, but git-am failed. The
conflicts are in "hw/hppa/Kconfig" and "hw/i386/Kconfig". In the former,
the conflict is due to E1000_PCI, in the latter, the conflict occurs due
to VGA_PCI -- upstream doesn't seem to have either of those.

Thanks
Laszlo

Re: [Qemu-devel] [PATCH] virtio-vga: only enable for specific boards
Posted by Paolo Bonzini 5 years, 1 month ago

----- Original Message -----
> From: "Laszlo Ersek" <lersek@redhat.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>, qemu-devel@nongnu.org
> Cc: thuth@redhat.com
> Sent: Thursday, March 21, 2019 8:34:46 PM
> Subject: Re: [PATCH] virtio-vga: only enable for specific boards
> 
> On 03/21/19 15:29, Paolo Bonzini wrote:
> > When virtio-vga was added, the intention was to only support it for
> > those machines where the firmware does not know about virtio-gpu,
> > and supported VGA legacy hardware before virtio-{gpu,vga} were
> > introduced.
> > 
> > The Kconfig switch however enabled virtio-vga for all machines with
> > a PCI bus, and libvirt then prefers it even on hardware where
> > virtio-gpu would be preferrable.  At least for now, only enable
> > virtio-vga for PC, hppa and pSeries machines, as was the case
> > before Kconfig dependencies were introduced.
> > 
> > Reported-by: Laszlo Ersek <lersek@redhat.com>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  hw/display/Kconfig | 2 +-
> >  hw/hppa/Kconfig    | 1 +
> >  hw/i386/Kconfig    | 1 +
> >  hw/ppc/Kconfig     | 1 +
> >  4 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/display/Kconfig b/hw/display/Kconfig
> > index 86c1d544c5..72be57a403 100644
> > --- a/hw/display/Kconfig
> > +++ b/hw/display/Kconfig
> > @@ -100,7 +100,7 @@ config VIRTIO_GPU
> >  
> >  config VIRTIO_VGA
> >      bool
> > -    default y if PCI_DEVICES
> > +    # defaults to "N", enabled by specific boards
> >      depends on VIRTIO_PCI
> >      select VGA
> >  
> > diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
> > index 7334f57081..6e5d74a825 100644
> > --- a/hw/hppa/Kconfig
> > +++ b/hw/hppa/Kconfig
> > @@ -2,6 +2,7 @@ config DINO
> >      bool
> >      imply PCI_DEVICES
> >      imply E1000_PCI
> > +    imply VIRTIO_VGA
> >      select PCI
> >      select SERIAL
> >      select ISA_BUS
> > diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> > index 8e8444dbf9..a6aed7c131 100644
> > --- a/hw/i386/Kconfig
> > +++ b/hw/i386/Kconfig
> > @@ -19,6 +19,7 @@ config PC
> >      imply TPM_CRB
> >      imply TPM_TIS
> >      imply VGA_PCI
> > +    imply VIRTIO_VGA
> >      select FDC
> >      select I8259
> >      select I8254
> > diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> > index ae07b4da63..a3465155f0 100644
> > --- a/hw/ppc/Kconfig
> > +++ b/hw/ppc/Kconfig
> > @@ -2,6 +2,7 @@ config PSERIES
> >      bool
> >      imply PCI_DEVICES
> >      imply TEST_DEVICES
> > +    imply VIRTIO_VGA
> >      select DIMM
> >      select PCI
> >      select SPAPR_VSCSI
> > 
> 
> I tried to apply this for testing, on top of
> c692931cda9dc6cbc16b89d5094a725a10dbb641, but git-am failed. The
> conflicts are in "hw/hppa/Kconfig" and "hw/i386/Kconfig". In the former,
> the conflict is due to E1000_PCI, in the latter, the conflict occurs due
> to VGA_PCI -- upstream doesn't seem to have either of those.

Yeah, it is on top of my tree.  You can find it as tag for-upstream at
https://github.com/bonzini/qemu, but it is not tested yet.

Paolo

Re: [Qemu-devel] [PATCH] virtio-vga: only enable for specific boards
Posted by Laszlo Ersek 5 years, 1 month ago
On 03/22/19 23:00, Paolo Bonzini wrote:
> 
> 
> ----- Original Message -----
>> From: "Laszlo Ersek" <lersek@redhat.com>
>> To: "Paolo Bonzini" <pbonzini@redhat.com>, qemu-devel@nongnu.org
>> Cc: thuth@redhat.com
>> Sent: Thursday, March 21, 2019 8:34:46 PM
>> Subject: Re: [PATCH] virtio-vga: only enable for specific boards
>>
>> On 03/21/19 15:29, Paolo Bonzini wrote:
>>> When virtio-vga was added, the intention was to only support it for
>>> those machines where the firmware does not know about virtio-gpu,
>>> and supported VGA legacy hardware before virtio-{gpu,vga} were
>>> introduced.
>>>
>>> The Kconfig switch however enabled virtio-vga for all machines with
>>> a PCI bus, and libvirt then prefers it even on hardware where
>>> virtio-gpu would be preferrable.  At least for now, only enable
>>> virtio-vga for PC, hppa and pSeries machines, as was the case
>>> before Kconfig dependencies were introduced.
>>>
>>> Reported-by: Laszlo Ersek <lersek@redhat.com>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>  hw/display/Kconfig | 2 +-
>>>  hw/hppa/Kconfig    | 1 +
>>>  hw/i386/Kconfig    | 1 +
>>>  hw/ppc/Kconfig     | 1 +
>>>  4 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/display/Kconfig b/hw/display/Kconfig
>>> index 86c1d544c5..72be57a403 100644
>>> --- a/hw/display/Kconfig
>>> +++ b/hw/display/Kconfig
>>> @@ -100,7 +100,7 @@ config VIRTIO_GPU
>>>  
>>>  config VIRTIO_VGA
>>>      bool
>>> -    default y if PCI_DEVICES
>>> +    # defaults to "N", enabled by specific boards
>>>      depends on VIRTIO_PCI
>>>      select VGA
>>>  
>>> diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
>>> index 7334f57081..6e5d74a825 100644
>>> --- a/hw/hppa/Kconfig
>>> +++ b/hw/hppa/Kconfig
>>> @@ -2,6 +2,7 @@ config DINO
>>>      bool
>>>      imply PCI_DEVICES
>>>      imply E1000_PCI
>>> +    imply VIRTIO_VGA
>>>      select PCI
>>>      select SERIAL
>>>      select ISA_BUS
>>> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
>>> index 8e8444dbf9..a6aed7c131 100644
>>> --- a/hw/i386/Kconfig
>>> +++ b/hw/i386/Kconfig
>>> @@ -19,6 +19,7 @@ config PC
>>>      imply TPM_CRB
>>>      imply TPM_TIS
>>>      imply VGA_PCI
>>> +    imply VIRTIO_VGA
>>>      select FDC
>>>      select I8259
>>>      select I8254
>>> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
>>> index ae07b4da63..a3465155f0 100644
>>> --- a/hw/ppc/Kconfig
>>> +++ b/hw/ppc/Kconfig
>>> @@ -2,6 +2,7 @@ config PSERIES
>>>      bool
>>>      imply PCI_DEVICES
>>>      imply TEST_DEVICES
>>> +    imply VIRTIO_VGA
>>>      select DIMM
>>>      select PCI
>>>      select SPAPR_VSCSI
>>>
>>
>> I tried to apply this for testing, on top of
>> c692931cda9dc6cbc16b89d5094a725a10dbb641, but git-am failed. The
>> conflicts are in "hw/hppa/Kconfig" and "hw/i386/Kconfig". In the former,
>> the conflict is due to E1000_PCI, in the latter, the conflict occurs due
>> to VGA_PCI -- upstream doesn't seem to have either of those.
> 
> Yeah, it is on top of my tree.  You can find it as tag for-upstream at
> https://github.com/bonzini/qemu, but it is not tested yet.

I've now tested this at your commit 938912a86611 ("virtio-vga: only
enable for specific boards", 2019-03-21).

Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thank you!
Laszlo