| Message ID | cover.1634639117.git.bertrand.marquis@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fixes: PCI devices passthrough on Arm | expand |
Bertrand Marquis writes ("[PATCH 0/3] Fixes: PCI devices passthrough on Arm"):
> This patch serie is a follow-up after various findings on d59168dc05
> ("xen/arm: Enable the existing x86 virtual PCI support for ARM") as of
> agreed in [1].
>
> It does the following:
> - enable vpci_add_handlers on x86 and not only on arm
> - remove __hwdom_init flag for vpci_add_handlers
> - add missing vpci handler cleanup in error path during pci_device_add
> and pci_device_remove
Thanks. Roger, Jan, what do you think of this ?
I have no qualms from my RM POV other than that I want a fix
resolves the concenrs previously expressed by maintainers.
Thanks,
Ian.
On 19.10.2021 13:12, Ian Jackson wrote: > Bertrand Marquis writes ("[PATCH 0/3] Fixes: PCI devices passthrough on Arm"): >> This patch serie is a follow-up after various findings on d59168dc05 >> ("xen/arm: Enable the existing x86 virtual PCI support for ARM") as of >> agreed in [1]. >> >> It does the following: >> - enable vpci_add_handlers on x86 and not only on arm >> - remove __hwdom_init flag for vpci_add_handlers >> - add missing vpci handler cleanup in error path during pci_device_add >> and pci_device_remove > > Thanks. Roger, Jan, what do you think of this ? I'll get to this, but at the first glance I'd say that the change in patch 1 coming before what patch 2 does is already a problem. Jan
Hi Jan, > On 19 Oct 2021, at 12:25, Jan Beulich <jbeulich@suse.com> wrote: > > On 19.10.2021 13:12, Ian Jackson wrote: >> Bertrand Marquis writes ("[PATCH 0/3] Fixes: PCI devices passthrough on Arm"): >>> This patch serie is a follow-up after various findings on d59168dc05 >>> ("xen/arm: Enable the existing x86 virtual PCI support for ARM") as of >>> agreed in [1]. >>> >>> It does the following: >>> - enable vpci_add_handlers on x86 and not only on arm >>> - remove __hwdom_init flag for vpci_add_handlers >>> - add missing vpci handler cleanup in error path during pci_device_add >>> and pci_device_remove >> >> Thanks. Roger, Jan, what do you think of this ? > > I'll get to this, but at the first glance I'd say that the change in > patch 1 coming before what patch 2 does is already a problem. Because path 1 is actually introducing a bug solved by patch 2, I should have thought of that. I can either invert those 2 (or actually put patch 1 at the end) or merge patch 1 and 2 together. Bertrand > > Jan >
On 19.10.2021 12:40, Bertrand Marquis wrote: > This patch serie is a follow-up after various findings on d59168dc05 > ("xen/arm: Enable the existing x86 virtual PCI support for ARM") as of > agreed in [1]. > > It does the following: > - enable vpci_add_handlers on x86 and not only on arm > - remove __hwdom_init flag for vpci_add_handlers > - add missing vpci handler cleanup in error path during pci_device_add > and pci_device_remove > > [1] https://marc.info/?l=xen-devel&m=163455502020100&w=2 > > Bertrand Marquis (3): > xen/arm: call vpci_add_handlers on x86 > xen/vpci: Remove __hwdom_init for vpci_add_handlers > xen/pci: Add missing vpci handler cleanup Imo all three changes need to be in a single patch. An option might be to have first what is now patch 3, with CONFIG_ARM conditionals, which the subsequent patch would then delete. But what is now patch 1 cannot come before patch 2, and patch 2 alone would unduly impact x86 by moving code from .init.text to .text for no reason. (Still it could technically be done that way.) But let me also comment patches 1 and 2 individually (patch 3 looks okay, apart from the ordering issue). Jan
> On 19 Oct 2021, at 13:26, Jan Beulich <jbeulich@suse.com> wrote: > > On 19.10.2021 12:40, Bertrand Marquis wrote: >> This patch serie is a follow-up after various findings on d59168dc05 >> ("xen/arm: Enable the existing x86 virtual PCI support for ARM") as of >> agreed in [1]. >> >> It does the following: >> - enable vpci_add_handlers on x86 and not only on arm >> - remove __hwdom_init flag for vpci_add_handlers >> - add missing vpci handler cleanup in error path during pci_device_add >> and pci_device_remove >> >> [1] https://marc.info/?l=xen-devel&m=163455502020100&w=2 >> >> Bertrand Marquis (3): >> xen/arm: call vpci_add_handlers on x86 >> xen/vpci: Remove __hwdom_init for vpci_add_handlers >> xen/pci: Add missing vpci handler cleanup > > Imo all three changes need to be in a single patch. I will merge all changes in one patch. > An option might be > to have first what is now patch 3, with CONFIG_ARM conditionals, which > the subsequent patch would then delete. But what is now patch 1 cannot > come before patch 2, and patch 2 alone would unduly impact x86 by > moving code from .init.text to .text for no reason. (Still it could > technically be done that way.) > > But let me also comment patches 1 and 2 individually (patch 3 looks > okay, apart from the ordering issue). Thanks Bertrand > > Jan
This patch serie is a follow-up after various findings on d59168dc05 ("xen/arm: Enable the existing x86 virtual PCI support for ARM") as of agreed in [1]. It does the following: - enable vpci_add_handlers on x86 and not only on arm - remove __hwdom_init flag for vpci_add_handlers - add missing vpci handler cleanup in error path during pci_device_add and pci_device_remove [1] https://marc.info/?l=xen-devel&m=163455502020100&w=2 Bertrand Marquis (3): xen/arm: call vpci_add_handlers on x86 xen/vpci: Remove __hwdom_init for vpci_add_handlers xen/pci: Add missing vpci handler cleanup xen/drivers/passthrough/pci.c | 8 ++------ xen/drivers/vpci/vpci.c | 2 +- xen/include/xen/vpci.h | 2 ++ 3 files changed, 5 insertions(+), 7 deletions(-)