On 2025/3/31 19:04, Roger Pau Monné wrote:
> On Mon, Mar 31, 2025 at 09:32:02AM +0000, Chen, Jiqian wrote:
>> So, I need to refactor the emulating PCI capability list codes of
>> init_header() to serve
>> for all domain(dom0+domUs) firstly, since current codes only emulate PCI
>> capability list for domUs, right?
>
> Yes, that would be my understanding. The current logic in
> init_header() needs to be expanded/generalized so it can be used for
> masking random PCI capabilities, plus adapted to work with PCIe
> capabilities also.
OK, I will try to refactor the logic in next version.
Hoping the next version will be more in line with your ideas.
Thanks!
>
>>>
>>>>
>>>>>
>>>>>> + /*
>>>>>> + * Capabilities with high priority like MSI-X need to
>>>>>> + * be initialized before header
>>>>>> + */
>>>>>> + rc = vpci_init_cap_with_priority(pdev, VPCI_PRIORITY_HIGH);
>>>>>> + if ( rc )
>>>>>> + goto out;
>>>>>
>>>>> I understand this is not introduced by this change, but I wonder if
>>>>> there could be a way to ditch the priority stuff for capabilities,
>>>>> specially now that we only have two "priorities": before or after PCI
>>>>> header initialization.
>>>> I have an idea, but it seems like a hake.
>>>> Can we add a flag(maybe name it "msix_initialized") to struct vpci{}?
>>>> Then in vpci_make_msix_hole(), we can first check that flag, if it is
>>>> false, we return an error to let modify_decoding() directly return in the
>>>> process of init_header.
>>>> And in the start of init_msix(), to set msix_initialized=true, in the end
>>>> of init_msix(), to call modify_decoding() to setup p2m.
>>>> Then we can remove the priorities.
>>>
>>> Maybe the initialization of the MSI-X capability could be done after
>>> the header, and call vpci_make_msix_hole()? There's a bit of
>>> redundancy here in that the BAR is first fully mapped, and then a hole
>>> is punched in place of the MSI-X related tables. Seems like the
>>> easier option to break the depedency of init_msix() in being called
>>> ahead of init_header().
>> You mean the sequence should be:
>> vpci_init_header()
>> vpci_init_capability() // all capabilities
>> vpci_make_msix_hole()
>>
>> Right?
>
> Yes, I think that would be my preference. The call to
> vpci_make_msix_hole() should be inside of init_msix().
Got it, will do in next version.
>
> Thanks, Roger.
--
Best regards,
Jiqian Chen.
