From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AEFFFC433ED for ; Fri, 30 Apr 2021 15:37:05 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id ECA7D6145D for ; Fri, 30 Apr 2021 15:37:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org ECA7D6145D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:46912 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lcVCR-0000ip-Mc for qemu-devel@archiver.kernel.org; Fri, 30 Apr 2021 11:37:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:45824) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lcV8p-0007HW-73 for qemu-devel@nongnu.org; Fri, 30 Apr 2021 11:33:20 -0400 Received: from mail-ed1-x529.google.com ([2a00:1450:4864:20::529]:36391) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lcV8l-0002vy-9v for qemu-devel@nongnu.org; Fri, 30 Apr 2021 11:33:18 -0400 Received: by mail-ed1-x529.google.com with SMTP id q6so23843806edr.3 for ; Fri, 30 Apr 2021 08:33:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=46moMcahEwdKY/7RLW7K0BiIlv4+W6IHXL2AV7DYEvE=; b=MV58OS0E9xmNwOHROFV5V0PDWjwTExQXzQ3lfHS1FYOBUxldfB44gzyTRT+xwmPLUj EK59Eo93Zks3n9+Rgrg0GX0IyREWUgJGyaVfX36uUwMeMsu6JXi+W7mZLBeTLdXoreQw pG1zG937qHVaA8W47Ip5a8V0ei6MhUnZMSc3aBrzVIePRjJwe0FGcX+0zMTcidsMOxpJ Cqres/ocTSyG17kjlw0TvHQ39EXCbPJ9EZxhrGjZrLVrvFQ5WiRLNq4aFlyRimoSKC55 1X3cO8GJRr2tu3Q7ud2IwECoJVqHnKTon3SJmvBRZIVQIGEQvuoEAneo4sgTATdEBvYo O6wQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=46moMcahEwdKY/7RLW7K0BiIlv4+W6IHXL2AV7DYEvE=; b=nIuptObZNyjJkhYsaH4GDabuNp561hWS9id44hGiRIzy217jjcoQ4OwUvRwd4es80v N12V903ALfjS8tRXiuvNatY1SF1iIY1ej5g3EMyt+07y9B2K90fvBB3NkXjufU4t2fZs 6KhmzaXAL3LolprAAhd9HCoondy2ZTQKZDWDILoHFeVY9ao8hhVyhBJqNHyifalmIbIi iKmVt3BX3mYSYd1UakKtT6X8vy6Zm+4d01ATpwOJJfsRjMPrqbZu9wHc9Gewn9Lk/5rl HQ+0PpHCfDxEtG2RM9kO/o3CnsSUsHhsTnE0Jp33+QlJ8B5Eann1JQE7Pu8wZhD3B9Vx 32ZA== X-Gm-Message-State: AOAM533tEoizCEHCtd8dW5Gr8wE60SXvW4GQ9Czs52rJ/828ZXYcxkfl OGSFqY7PgEc47X2tFX0GJFDBlZUcsZ9qLp/usnw= X-Google-Smtp-Source: ABdhPJw4apNhBPDCxU7H8Yhiry45I5pi5lUyPhQIs9ikDoKuQwrITyiVQYhBt8Vj+ld11FBO/zSJO0s876UbAluygIQ= X-Received: by 2002:a05:6402:48c:: with SMTP id k12mr6792465edv.237.1619796790335; Fri, 30 Apr 2021 08:33:10 -0700 (PDT) MIME-Version: 1.0 References: <20210430113547.1816178-1-kraxel@redhat.com> In-Reply-To: <20210430113547.1816178-1-kraxel@redhat.com> From: =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= Date: Fri, 30 Apr 2021 19:32:58 +0400 Message-ID: Subject: Re: [PATCH v2 00/16] virtio-gpu: split into two devices. To: Gerd Hoffmann Content-Type: multipart/alternative; boundary="0000000000004bde8c05c13254ba" Received-SPF: pass client-ip=2a00:1450:4864:20::529; envelope-from=marcandre.lureau@gmail.com; helo=mail-ed1-x529.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , "Michael S. Tsirkin" , Vivek Kasireddy , QEMU , Tina Zhang Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --0000000000004bde8c05c13254ba Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi On Fri, Apr 30, 2021 at 4:23 PM Gerd Hoffmann wrote: > Currently we have one virtio-gpu device. Problem with this approach is > that if you compile a full-featured qemu you'll get a virtio-gpu device > which depends on opengl and virgl, so these dependencies must be > installed and the libraries will be loaded into memory even if you don't > use virgl. Also the code is cluttered with #ifdefs and a bit messy. > > This patch series splits the virtio-gpu device into two: > > (1) virtio-gpu-device becomes the non-virgl device, same as > virtio-gpu-device,virgl=3Doff today. > (2) virtio-gpu-gl-device is the new virgl device, same as > virtio-gpu-device,virgl=3Don today. > > When compiling qemu without virglrenderer support virtio-gpu-device > behavior doesn't change. > > v2: > - rebase to latest master. > - move pci and vga wrappers to separate modules. > - fix ci failures. > The patch series looks good. But isn't that a breaking change? Any existing user of virtio-gpu/vga,virgl=3Don will no longer get a working setup. Right? Imho, = in this case (virgl VM being not very common) the benefit is worth it. However, I suggest to keep the 'virgl=3D' property, and print a deprecation= / replaced-by warning, falling back to non-virgl/2d mode. Or perhaps we could have more cleverness to have virgl=3Don aliasing to the new devices. > Gerd Hoffmann (16): > virtio-gpu: rename virgl source file. > virtio-gpu: add virtio-gpu-gl-device > virtio-gpu: move virgl realize + properties > virtio-gpu: move virgl reset > virtio-gpu: use class function for ctrl queue handlers > virtio-gpu: move virgl handle_ctrl > virtio-gpu: move virgl gl_flushed > virtio-gpu: move virgl process_cmd > virtio-gpu: move update_cursor_data > virtio-gpu: drop VIRGL() macro > virtio-gpu: move virtio-gpu-gl-device to separate module > virtio-gpu: drop use_virgl_renderer > virtio-gpu: move fields to struct VirtIOGPUGL > virtio-gpu: add virtio-gpu-gl-pci > modules: add have_vga > virtio-gpu: add virtio-vga-gl > > include/hw/display/vga.h | 6 + > include/hw/virtio/virtio-gpu.h | 31 +++- > hw/display/vga.c | 2 + > hw/display/virtio-gpu-base.c | 6 +- > hw/display/virtio-gpu-gl.c | 163 ++++++++++++++++++ > hw/display/virtio-gpu-pci-gl.c | 55 ++++++ > .../{virtio-gpu-3d.c =3D> virtio-gpu-virgl.c} | 0 > hw/display/virtio-gpu.c | 142 +++------------ > hw/display/virtio-vga-gl.c | 47 +++++ > util/module.c | 7 + > hw/display/meson.build | 19 +- > 11 files changed, 344 insertions(+), 134 deletions(-) > create mode 100644 hw/display/virtio-gpu-gl.c > create mode 100644 hw/display/virtio-gpu-pci-gl.c > rename hw/display/{virtio-gpu-3d.c =3D> virtio-gpu-virgl.c} (100%) > create mode 100644 hw/display/virtio-vga-gl.c > > -- > 2.30.2 > > > > --=20 Marc-Andr=C3=A9 Lureau --0000000000004bde8c05c13254ba Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Fri, Apr 30, 2021 at 4:23 PM Ger= d Hoffmann <kraxel@redhat.com&g= t; wrote:
Curren= tly we have one virtio-gpu device.=C2=A0 Problem with this approach is
that if you compile a full-featured qemu you'll get a virtio-gpu device=
which depends on opengl and virgl, so these dependencies must be
installed and the libraries will be loaded into memory even if you don'= t
use virgl.=C2=A0 Also the code is cluttered with #ifdefs and a bit messy.
This patch series splits the virtio-gpu device into two:

=C2=A0(1) virtio-gpu-device becomes the non-virgl device, same as
=C2=A0 =C2=A0 =C2=A0virtio-gpu-device,virgl=3Doff today.
=C2=A0(2) virtio-gpu-gl-device is the new virgl device, same as
=C2=A0 =C2=A0 =C2=A0virtio-gpu-device,virgl=3Don today.

When compiling qemu without virglrenderer support virtio-gpu-device
behavior doesn't change.

v2:
=C2=A0- rebase to latest master.
=C2=A0- move pci and vga wrappers to separate modules.
=C2=A0- fix ci failures.

The patch seri= es looks good.

But isn't that a breaking chang= e? Any existing user of virtio-gpu/vga,virgl=3Don will no longer get a work= ing setup. Right? Imho, in this case (virgl VM being not very common) the b= enefit is worth it. However, I suggest to keep the 'virgl=3D' prope= rty, and print a deprecation / replaced-by warning, falling back to non-vir= gl/2d mode. Or perhaps we could have more cleverness to have virgl=3Don ali= asing to the new devices.


Gerd Hoffmann (16):
=C2=A0 virtio-gpu: rename virgl source file.
=C2=A0 virtio-gpu: add virtio-gpu-gl-device
=C2=A0 virtio-gpu: move virgl realize + properties
=C2=A0 virtio-gpu: move virgl reset
=C2=A0 virtio-gpu: use class function for ctrl queue handlers
=C2=A0 virtio-gpu: move virgl handle_ctrl
=C2=A0 virtio-gpu: move virgl gl_flushed
=C2=A0 virtio-gpu: move virgl process_cmd
=C2=A0 virtio-gpu: move update_cursor_data
=C2=A0 virtio-gpu: drop VIRGL() macro
=C2=A0 virtio-gpu: move virtio-gpu-gl-device to separate module
=C2=A0 virtio-gpu: drop use_virgl_renderer
=C2=A0 virtio-gpu: move fields to struct VirtIOGPUGL
=C2=A0 virtio-gpu: add virtio-gpu-gl-pci
=C2=A0 modules: add have_vga
=C2=A0 virtio-gpu: add virtio-vga-gl

=C2=A0include/hw/display/vga.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2=A06 +
=C2=A0include/hw/virtio/virtio-gpu.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 |=C2=A0 31 +++-
=C2=A0hw/display/vga.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2=A02 +
=C2=A0hw/display/virtio-gpu-base.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2=A06 +-
=C2=A0hw/display/virtio-gpu-gl.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 163 ++++++++++++++++++
=C2=A0hw/display/virtio-gpu-pci-gl.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 |=C2=A0 55 ++++++
=C2=A0.../{virtio-gpu-3d.c =3D> virtio-gpu-virgl.c}=C2=A0 =C2=A0|=C2=A0 = =C2=A00
=C2=A0hw/display/virtio-gpu.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 142 +++------------
=C2=A0hw/display/virtio-vga-gl.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 47 +++++
=C2=A0util/module.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0 =C2= =A07 +
=C2=A0hw/display/meson.build=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 19 +-
=C2=A011 files changed, 344 insertions(+), 134 deletions(-)
=C2=A0create mode 100644 hw/display/virtio-gpu-gl.c
=C2=A0create mode 100644 hw/display/virtio-gpu-pci-gl.c
=C2=A0rename hw/display/{virtio-gpu-3d.c =3D> virtio-gpu-virgl.c} (100%)=
=C2=A0create mode 100644 hw/display/virtio-vga-gl.c

--
2.30.2





--
Marc-Andr=C3=A9 Lureau
--0000000000004bde8c05c13254ba--