On 11.04.2025 04:51, Chen, Jiqian wrote: > On 2025/4/10 20:34, Jan Beulich wrote: >> On 09.04.2025 08:45, Jiqian Chen wrote: >>> --- a/xen/drivers/pci/pci.c >>> +++ b/xen/drivers/pci/pci.c >>> @@ -40,7 +40,7 @@ unsigned int pci_find_cap_offset(pci_sbdf_t sbdf, >>> unsigned int cap) >>> } >>> >>> unsigned int pci_find_next_cap_ttl(pci_sbdf_t sbdf, unsigned int pos, >>> - const unsigned int caps[], unsigned int >>> n, >>> + const unsigned int *caps, unsigned int >>> n, >> >> I don't follow the need for this change. > This changed is for my next patch "vpci/header: Emulate legacy capability > list for host". > Currently, vpci only emulates capability list for domU, not for dom0. > For domU, vpci exposes a fixed capability array which calls "supported_caps". > My changes want to emulate capability list for dom0. > I think vpci should expose all devices capabilities to dom0. > When I emulate it, I need to iterate over all capabilities without another > fixed array, > so I need this function to return the position of next capability directly > when passing a zero length array to this function.
This doesn't answer my question. The change you need for the next patch is the hunk below, not the one above. Aiui at least. >>> @@ -55,6 +55,10 @@ unsigned int pci_find_next_cap_ttl(pci_sbdf_t sbdf, >>> unsigned int pos, >>> >>> if ( id == 0xff ) >>> break; >>> + >>> + if ( !caps || n == 0 ) >>> + return pos; >> >> Checking n to be zero ought to suffice here? In that case it doesn't matter >> what caps is. Plus if n is non-zero, it clearly is an error if caps was NULL. > Two checking is to prevent null pointer errors. > But as you said, if checking n to be zero is enough, then I don't need to > change the definition of this function. > I will change in next version. If you really wanted to, you could add e.g. ASSERT(caps) after this if(). Jan
