[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v8 2/5] xen/arm: Enable the existing x86 virtual PCI support for ARM

On 15/10/2021 18:33, Bertrand Marquis wrote:
Hi Julien,

Hi Bertrand,

On 15 Oct 2021, at 18:25, Julien Grall <julien@xxxxxxx> wrote:

Hi Bertrand,

On 15/10/2021 17:51, Bertrand Marquis wrote:
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 3aa8c3175f..35e0190796 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -756,6 +756,19 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
      if ( !pdev->domain )
          pdev->domain = hardware_domain;
+#ifdef CONFIG_ARM
+        /*
+         * On ARM PCI devices discovery will be done by Dom0. Add vpci handler
+         * when Dom0 inform XEN to add the PCI devices in XEN.
+         */
+        ret = vpci_add_handlers(pdev);

I don't seem to find the code to remove __init_hwdom in this series. Are you 
intending to fix it separately?

Yes I think it is better to fix that in a new patch as it will require some 
discussion as it will impact the x86 code if I just remove the flag now.
For the future patch series, may I ask to keep track of outstanding issues in the commit message (if you don't plan to address them before commiting) or after --- (if they are meant to be addressed before commiting)?

In this case, the impact on Arm is this would result to an hypervisor crash if called. If we drop __init_hwdom, the impact on x86 is Xen text will slightly be bigger after the boot time.

AFAICT, the code is not reachable on Arm (?). Therefore, one could argue we this can wait after the week-end as this is a latent bug. Yet, I am not really comfortable to see knowningly buggy code merged.

Stefano, would you be willing to remove __init_hwdom while committing it? If not, can you update the commit message and mention this patch doesn't work as intended?


Julien Grall



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.