On 15.10.2021 20:38, Julien Grall wrote: > > > On 15/10/2021 18:33, Bertrand Marquis wrote: >> Hi Julien, > > Hi Bertrand, > >> >>> On 15 Oct 2021, at 18:25, Julien Grall <[email protected]> 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 (?).
Which re-raises my question towards testing of what is being added in this series. Supported also by the typo in v7 patch 1, which suggests that version wasn't even build-tested. Jan > 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? > > Cheers, >
