Hi Jan, > On 18 Oct 2021, at 08:51, Jan Beulich <[email protected]> wrote: > > 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.
This was an honest mistake, we did build locally but without VPCI activated. Once I discovered this by rerunning all tests (including one modifying the code to activate VPCI), I signalled it on the mailing list and it was fixed in v8. We did a lot of tests and tried to be as careful as possible but on the last rush before the feature freeze deadline those can happen. Regards Bertrand > > 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,
