On 2025/6/20 14:34, Jan Beulich wrote:
> On 19.06.2025 08:14, Chen, Jiqian wrote:
>> On 2025/6/18 22:33, Jan Beulich wrote:
>>> On 12.06.2025 11:29, Jiqian Chen wrote:
>>>> --- a/xen/include/xen/vpci.h
>>>> +++ b/xen/include/xen/vpci.h
>>>> @@ -13,11 +13,12 @@ typedef uint32_t vpci_read_t(const struct pci_dev
>>>> *pdev, unsigned int reg,
>>>> typedef void vpci_write_t(const struct pci_dev *pdev, unsigned int reg,
>>>> uint32_t val, void *data);
>>>>
>>>> -typedef int vpci_register_init_t(struct pci_dev *dev);
>>>> -
>>>> -#define VPCI_PRIORITY_HIGH "1"
>>>> -#define VPCI_PRIORITY_MIDDLE "5"
>>>> -#define VPCI_PRIORITY_LOW "9"
>>>> +typedef struct {
>>>> + unsigned int id;
>>>> + bool is_ext;
>>>> + int (*init)(struct pci_dev *pdev);
>>>> + int (*cleanup)(struct pci_dev *pdev);
>>>
>>> Is const really not possible to add to at least one of these two?
>> Will change to be :
>>
>> typedef struct {
>> unsigned int id;
>> bool is_ext;
>> int (* const init)(struct pci_dev *pdev);
>> int (* const cleanup)(struct pci_dev *pdev);
>> } vpci_capability_t;
>
> Ehm, no. The question was for the two function (pointer) parameters. "const"
> on struct fields themselves can be useful, too, but for an entirely different
> purpose.
OK, will add const both for the struct field and the function parameters.
>
>>>> +} vpci_capability_t;
>>>
>>> As you have it here, ...
>>>
>>>> @@ -29,9 +30,22 @@ typedef int vpci_register_init_t(struct pci_dev *dev);
>>>> */
>>>> #define VPCI_MAX_VIRT_DEV (PCI_SLOT(~0) + 1)
>>>>
>>>> -#define REGISTER_VPCI_INIT(x, p) \
>>>> - static vpci_register_init_t *const x##_entry \
>>>> - __used_section(".data.vpci." p) = (x)
>>>> +#define REGISTER_VPCI_CAPABILITY(cap, finit, fclean, ext) \
>>>> + static const vpci_capability_t finit##_t = { \
>>>
>>> ... _t suffixes generally designate types. I don't think we should abuse
>>> that suffix for an identifier of a variable.
>> What do you think I should change to?
>
> Well, if you take my other advice, this question won't need answering, as
> then you only need the ..._entry one.
>
> Btw, noticing only now - why is it finit that's used to derive the identifier?
> With that, it could as well be fclean (leaving aside the fact that that's
> optional). Imo the name would better be derived from cap, and it would better
> also reflect the purpose of the variable.
I considered this.
I think it is easier to use finit, and finit contains the cap type, and the
main purpose of this struct is to initialize the cap.
>
> Jan
--
Best regards,
Jiqian Chen.
