[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] xen/pt: fix igd passthrough for pc machine with xen accelera
From: |
Stefano Stabellini |
Subject: |
Re: [PATCH] xen/pt: fix igd passthrough for pc machine with xen accelerator |
Date: |
Fri, 10 Feb 2023 15:27:52 -0800 (PST) |
User-agent: |
Alpine 2.22 (DEB 394 2020-01-19) |
On Tue, 7 Feb 2023, Chuck Zmudzinski wrote:
> Commit 998250e97661 ("xen, gfx passthrough: register host bridge specific
> to passthrough") uses the igd-passthrough-i440FX pci host device with
> the xenfv machine type and igd-passthru=on, but using it for the pc
> machine type, xen accelerator, and igd-passtru=on was omitted from that
> commit.
>
> The igd-passthru-i440FX pci host device is also needed for guests
> configured with the pc machine type, the xen accelerator, and
> igd-passthru=on. Specifically, tests show that not using the igd-specific
> pci host device with the Intel igd passed through to the guest results
> in slower startup performance and reduced resolution of the display
> during startup. This patch fixes this issue.
>
> To simplify the logic that is needed to support both the --enable-xen
> and the --disable-xen configure options, introduce the boolean symbol
> pc_xen_igd_gfx_pt_enabled() whose value is set appropriately in the
> sysemu/xen.h header file as the test to determine whether or not
> to use the igd-passthrough-i440FX pci host device instead of the
> normal i440FX pci host device.
>
> Fixes: 998250e97661 ("xen, gfx passthrough: register host bridge specific to
> passthrough")
> Signed-off-by: Chuck Zmudzinski <brchuckz@aol.com>
I think this is OK
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> This patch is intended to replace or complement a recently proposed
> patch that modifies slot_reserved_mask for the xenfv machine with
> igd-passthru=on in order to fix the problem of Qemu not reserving slot 2
> for the Intel IGD for the xenfv machine type. This patch provides a
> simple way to improve Qemu support for the Intel IGD with the xen
> accelerator without needing to change how slot_reserved_mask functions.
>
> For reference, the latest version of the patch to fix the xenfv machine
> using slot_reserved_mask is at:
>
> https://lore.kernel.org/qemu-devel/b1b4a21fe9a600b1322742dda55a40e9961daa57.1674346505.git.brchuckz@aol.com/
>
> Reason for introducing the new boolean pc_xen_igd_gfx_pt_enabled():
>
> It is also possible to use xen_igd_gfx_pt_enabled() directly to check
> if the igd-passthru-i440FX pci host device is needed in this patch,
> but in that case it would be necessary to implement it in
> accel/stubs/xen-stub.c like this:
>
> bool xen_igd_gfx_pt_enabled(void)
> {
> return false;
> }
>
> to cover the case when Qemu is configured with --disable-xen. I thought
> it was simpler to introduce the same boolean prefixed with pc_ and
> set it to 0 when --disable-xen is the configure option, and that explains
> why the proposed patch introduces pc_xen_igd_gfx_pt_enabled() instead of
> using xen_igd_gfx_pt_enabled() directly.
>
> Another reason to use pc_xen_igd_gfx_pt_enabled() is to distinguish it
> from xen_igd_gfx_pt_enabled() in hw/i386/pc_piix.c, because the use of
> xen_igd_gfx_pt_enabled() is guarded by CONFIG_XEN but this patch needs
> to place the boolean in a position that is not guarded by CONFIG_XEN.
> This approach will simplify any future effort to move the code in
> pc_piix.c that is not guarded by CONFIG_XEN to a xen-specific file.
>
> hw/i386/pc_piix.c | 2 ++
> include/sysemu/xen.h | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index df64dd8dcc..fd5b9ae1eb 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -433,6 +433,8 @@ static void pc_xen_hvm_init(MachineState *machine)
> compat(machine); \
> } \
> pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, \
> + pc_xen_igd_gfx_pt_enabled() ? \
> + TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : \
> TYPE_I440FX_PCI_DEVICE); \
> } \
> DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn)
> diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
> index 0ca25697e4..99ae41e619 100644
> --- a/include/sysemu/xen.h
> +++ b/include/sysemu/xen.h
> @@ -23,6 +23,7 @@
> extern bool xen_allowed;
>
> #define xen_enabled() (xen_allowed)
> +#define pc_xen_igd_gfx_pt_enabled() xen_igd_gfx_pt_enabled()
>
> #ifndef CONFIG_USER_ONLY
> void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
> @@ -33,6 +34,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
> #else /* !CONFIG_XEN_IS_POSSIBLE */
>
> #define xen_enabled() 0
> +#define pc_xen_igd_gfx_pt_enabled() 0
> #ifndef CONFIG_USER_ONLY
> static inline void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t
> length)
> {
> --
> 2.39.1
>