diff mbox series

[V2] hw/arm: enable qxl for aarch64

Message ID 20230512093108.1180726-1-zenghao@kylinos.cn
State New
Headers show
Series [V2] hw/arm: enable qxl for aarch64 | expand

Commit Message

Hao Zeng May 12, 2023, 9:31 a.m. UTC
Qemu does not support qxl graphics cards in arm, it is recommended to enable

Signed-off-by: Hao Zeng <zenghao@kylinos.cn>
---
 hw/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Daniel P. Berrangé May 15, 2023, 8:52 a.m. UTC | #1
On Fri, May 12, 2023 at 05:31:08PM +0800, Hao Zeng wrote:
> Qemu does not support qxl graphics cards in arm, it is recommended to enable

Who recommends this and why ?

The recommendations from Gerd are what I tend to point poeple to
for display devices:

  https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/

and it had this to say about arm:

  "On arm systems display devices with a pci memory bar do
   not work, which reduces the choices alot. We are left with:

    virtio gpu, if your guest has drivers
    ramfb"

Not sure if anything has changed in this respect ?


The QXL graphics card is an incredibly complex device, offering
2d acceleration that is not very interesting for modern guest OS
desktops since they're largely focused on 3d acceleration. This
complexity is bad from a security POV.

It would also require a guest driver to take advantage of QXL
features and while I presume the Linux driver will build, it is
still mostly pointless because of lack of interest in 2d acceleration.
I'm not sure about status of the Windows QXL driver for aarch64 ?

Further QXL is only useful when combined with SPICE graphics and
the SPICE project is largely inactive.

Overall, IMHO, we should keep QXL restricted to as few build scenarios
as possible. Given the status of SPICE, possibly we'll even want to
deprecate it on x86 eventually, not add it to more arches. 

What are you seeing as the compelling use case that requires QXL to
exist on aarch64 ?



> 
> Signed-off-by: Hao Zeng <zenghao@kylinos.cn>
> ---
>  hw/arm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 0f42c556d7..d0bedf9347 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -32,6 +32,7 @@ config ARM_VIRT
>      select VIRTIO_MEM_SUPPORTED
>      select ACPI_CXL
>      select ACPI_HMAT
> +    select QXL
>  
>  config CHEETAH
>      bool
> -- 
> 2.37.2
> 
> 

With regards,
Daniel
Peter Maydell May 15, 2023, 9:41 a.m. UTC | #2
On Mon, 15 May 2023 at 09:52, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, May 12, 2023 at 05:31:08PM +0800, Hao Zeng wrote:
> > Qemu does not support qxl graphics cards in arm, it is recommended to enable
>
> Who recommends this and why ?
>
> The recommendations from Gerd are what I tend to point poeple to
> for display devices:
>
>   https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/
>
> and it had this to say about arm:
>
>   "On arm systems display devices with a pci memory bar do
>    not work, which reduces the choices alot. We are left with:
>
>     virtio gpu, if your guest has drivers
>     ramfb"
>
> Not sure if anything has changed in this respect ?

That statement was not exactly correct in 2019. The
accurate statement would be:

If:
 * you're using KVM on Arm (TCG is fine)
 * and your host CPU does not have FEAT_S2FWB (a feature
   which is mandatory from Armv8.4 onwards)
 * and the guest device driver maps the PCI memory BAR
   as non-cacheable memory, or as write-through cacheable
   (which a real-hw graphics card driver will do)

then the behaviour will not be correct.

The only change since 2019 is that systems with FEAT_S2FWB
are more likely to be actually available now :-)

This happens because QEMU and the guest disagree about
the cacheability attribute for the emulated frame buffer:
to QEMU this is always normal writeback cacheable memory,
because it's just memory we got from malloc() or mmap().
The guest of course thinks it's a real hw framebuffer and
wants to map it with the attributes a hw framebuffer would have.
FEAT_S2FWB allows the hypervisor to overrule the guest's
stage 1 cacheability settings and enforce that RAM from
the host is always cacheable, which is why it avoids the issue.

For TCG none of this is a problem because the guest's
emulated memory accesses to host memory via emulation and
what the guest sets the cacheable attributes to has no effect.

The simple approach for KVM if you need to run it on
pre-v8.4 host CPUs is to use a device where the guest
device driver treats the memory as normal cacheable,
which includes the virtio-gpu one, but not any real
VGA hardware devices. A virtualization-aware graphics
device likely has other useful features in the VM
usecase anyway.

thanks
-- PMM
Hao Zeng May 15, 2023, 9:56 a.m. UTC | #3
On Mon, 2023-05-15 at 09:52 +0100, Daniel P. Berrangé wrote:
> On Fri, May 12, 2023 at 05:31:08PM +0800, Hao Zeng wrote:
> > Qemu does not support qxl graphics cards in arm, it is recommended
> > to enable
> 
> Who recommends this and why ?
> 
> The recommendations from Gerd are what I tend to point poeple to
> for display devices:
> 
>   https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/
> 
> and it had this to say about arm:
> 
>   "On arm systems display devices with a pci memory bar do
>    not work, which reduces the choices alot. We are left with:
> 
>     virtio gpu, if your guest has drivers
>     ramfb"
> 
> Not sure if anything has changed in this respect ?
> 
> 
> The QXL graphics card is an incredibly complex device, offering
> 2d acceleration that is not very interesting for modern guest OS
> desktops since they're largely focused on 3d acceleration. This
> complexity is bad from a security POV.
> 
> It would also require a guest driver to take advantage of QXL
> features and while I presume the Linux driver will build, it is
> still mostly pointless because of lack of interest in 2d
> acceleration.
> I'm not sure about status of the Windows QXL driver for aarch64 ?
> 
> Further QXL is only useful when combined with SPICE graphics and
> the SPICE project is largely inactive.
> 
> Overall, IMHO, we should keep QXL restricted to as few build
> scenarios
> as possible. Given the status of SPICE, possibly we'll even want to
> deprecate it on x86 eventually, not add it to more arches. 
> 
> What are you seeing as the compelling use case that requires QXL to
> exist on aarch64 ?
> 
hi Daniel:
   Thank you for your answer, it made me learn a lot. No use case, just
outside customer feedback on the ARM architecture qxl use has problems,
I compiled the community qemu, found that the default does not support
qxl display, so the submitted enablement.
  I agree with you, please ignore this commit.

I apologize for the disturbance to you all.

Best regards,
> Hao
> 
> 
> > 
> > Signed-off-by: Hao Zeng <zenghao@kylinos.cn>
> > ---
> >  hw/arm/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index 0f42c556d7..d0bedf9347 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -32,6 +32,7 @@ config ARM_VIRT
> >      select VIRTIO_MEM_SUPPORTED
> >      select ACPI_CXL
> >      select ACPI_HMAT
> > +    select QXL
> >  
> >  config CHEETAH
> >      bool
> > -- 
> > 2.37.2
> > 
> > 
> 
> With regards,
> Daniel
Peter Maydell May 15, 2023, 9:59 a.m. UTC | #4
On Mon, 15 May 2023 at 10:57, Hao Zeng <zenghao@kylinos.cn> wrote:
>
>
> On Mon, 2023-05-15 at 09:52 +0100, Daniel P. Berrangé wrote:
> > Overall, IMHO, we should keep QXL restricted to as few build
> > scenarios
> > as possible. Given the status of SPICE, possibly we'll even want to
> > deprecate it on x86 eventually, not add it to more arches.
> >
> > What are you seeing as the compelling use case that requires QXL to
> > exist on aarch64 ?

>    Thank you for your answer, it made me learn a lot. No use case, just
> outside customer feedback on the ARM architecture qxl use has problems,
> I compiled the community qemu, found that the default does not support
> qxl display, so the submitted enablement.
>   I agree with you, please ignore this commit.

I would still like to know why QXL isn't automatically
enabled like every other PCI device...

-- PMM
Daniel P. Berrangé May 15, 2023, 10:03 a.m. UTC | #5
On Mon, May 15, 2023 at 10:59:16AM +0100, Peter Maydell wrote:
> On Mon, 15 May 2023 at 10:57, Hao Zeng <zenghao@kylinos.cn> wrote:
> >
> >
> > On Mon, 2023-05-15 at 09:52 +0100, Daniel P. Berrangé wrote:
> > > Overall, IMHO, we should keep QXL restricted to as few build
> > > scenarios
> > > as possible. Given the status of SPICE, possibly we'll even want to
> > > deprecate it on x86 eventually, not add it to more arches.
> > >
> > > What are you seeing as the compelling use case that requires QXL to
> > > exist on aarch64 ?
> 
> >    Thank you for your answer, it made me learn a lot. No use case, just
> > outside customer feedback on the ARM architecture qxl use has problems,
> > I compiled the community qemu, found that the default does not support
> > qxl display, so the submitted enablement.
> >   I agree with you, please ignore this commit.
> 
> I would still like to know why QXL isn't automatically
> enabled like every other PCI device...

Historical reasons ?  Originally both QXL and SPICE were x86 only and
SPICE was broken on big endian if you tried to build it. The orignal
QXL code in QEMU had a hard dependancy on SPICE until an enhancement
made it work with other backends.


With regards,
Daniel
Gerd Hoffmann May 15, 2023, 10:54 a.m. UTC | #6
> > > On Mon, 2023-05-15 at 09:52 +0100, Daniel P. Berrangé wrote:
> > > > Overall, IMHO, we should keep QXL restricted to as few build
> > > > scenarios
> > > > as possible. Given the status of SPICE, possibly we'll even want to
> > > > deprecate it on x86 eventually, not add it to more arches.
> > > >
> > > > What are you seeing as the compelling use case that requires QXL to
> > > > exist on aarch64 ?
> > 
> > >    Thank you for your answer, it made me learn a lot. No use case, just
> > > outside customer feedback on the ARM architecture qxl use has problems,
> > > I compiled the community qemu, found that the default does not support
> > > qxl display, so the submitted enablement.
> > >   I agree with you, please ignore this commit.
> > 
> > I would still like to know why QXL isn't automatically
> > enabled like every other PCI device...
> 
> Historical reasons ?

Yes.  Any display device with a pci memory bar is disabled on arm due
to the caching problems.  So virtio-gpu is the best option available
(and it IMHO still is, it works without strings attached).

With FEAT_S2FWB being available (not sure how widespread) we might
reconsider this.  One advantage would be that a pci vram device with
dirty tracking allows for GOP support, i.e. efifb works.  Needs some
changes in edk2 too (include the QemuVideoDxe driver in ArmVirt builds).

If someone wants that I'd recommend to enable bochs-display for arm.

VGA + QXL would add complexity (and attack surface) without benefits.
Nothing running arm is going to use the obsolete features of these
devices (2d accel, vga text mode, planar video modes, ...).

take care,
  Gerd
Peter Maydell May 15, 2023, 11:54 a.m. UTC | #7
On Mon, 15 May 2023 at 11:54, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > > > On Mon, 2023-05-15 at 09:52 +0100, Daniel P. Berrangé wrote:
> > > > > Overall, IMHO, we should keep QXL restricted to as few build
> > > > > scenarios
> > > > > as possible. Given the status of SPICE, possibly we'll even want to
> > > > > deprecate it on x86 eventually, not add it to more arches.
> > > > >
> > > > > What are you seeing as the compelling use case that requires QXL to
> > > > > exist on aarch64 ?
> > >
> > > >    Thank you for your answer, it made me learn a lot. No use case, just
> > > > outside customer feedback on the ARM architecture qxl use has problems,
> > > > I compiled the community qemu, found that the default does not support
> > > > qxl display, so the submitted enablement.
> > > >   I agree with you, please ignore this commit.
> > >
> > > I would still like to know why QXL isn't automatically
> > > enabled like every other PCI device...
> >
> > Historical reasons ?
>
> Yes.  Any display device with a pci memory bar is disabled on arm due
> to the caching problems.  So virtio-gpu is the best option available
> (and it IMHO still is, it works without strings attached).

They all still build as far as I can see: if you
run "qemu-system-aarch64 -device help" it lists cirrus-vga,
ati-vga, bochs-display and the rest. This is correct, too --
there's no reason to deny TCG users these devices just because
they don't work in KVM.

I'm not trying to recommend QXL particularly, I'm just
not clear (a) how technically we are arranging for it not
to be built just like any other PCI device (assuming Spice
support is present at all) or (b) whether we really want
it to not be the same as all the other available but not
necessarily recommended PCI devices. Plus however we decide
to do the Kconfig logic we should resolve the discrepancy
between what hw/i386/Kconfig does and what hw/mips/Kconfig does.
(I would tend to go for "if it can be made to work, have it
enabled, and let users decide what they want to use". We
have *lots* of things QEMU can do that are almost always
not the sensible choice...)

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 0f42c556d7..d0bedf9347 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -32,6 +32,7 @@  config ARM_VIRT
     select VIRTIO_MEM_SUPPORTED
     select ACPI_CXL
     select ACPI_HMAT
+    select QXL
 
 config CHEETAH
     bool