linux-kernel - Re: [PATCH v3] xen: Remove dependency between pciback and privcmd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3b83e1ea-f630-4259-bc9e-16497329f5a3@suse.com>
Date: Fri, 11 Oct 2024 13:12:48 +0200
From: Juergen Gross <jgross@...e.com>
To: Jan Beulich <jbeulich@...e.com>, "Chen, Jiqian" <Jiqian.Chen@....com>
Cc: "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 Marek Marczykowski-Górecki
 <marmarek@...isiblethingslab.com>,
 Stefano Stabellini <sstabellini@...nel.org>,
 Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>
Subject: Re: [PATCH v3] xen: Remove dependency between pciback and privcmd

On 11.10.24 12:10, Jan Beulich wrote:
> On 11.10.2024 11:33, Chen, Jiqian wrote:
>> On 2024/10/11 17:20, Chen, Jiqian wrote:
>>> On 2024/10/11 16:54, Jan Beulich wrote:
>>>> On 11.10.2024 05:42, Jiqian Chen wrote:
>>>>> @@ -1757,11 +1756,19 @@ static int __init xen_pcibk_init(void)
>>>>>   		bus_register_notifier(&pci_bus_type, &pci_stub_nb);
>>>>>   #endif
>>>>>   
>>>>> +#ifdef CONFIG_XEN_ACPI
>>>>> +	xen_acpi_register_get_gsi_func(pcistub_get_gsi_from_sbdf);
>>>>> +#endif
>>>>> +
>>>>>   	return err;
>>>>>   }
>>>>>   
>>>>>   static void __exit xen_pcibk_cleanup(void)
>>>>>   {
>>>>> +#ifdef CONFIG_XEN_ACPI
>>>>> +	xen_acpi_register_get_gsi_func(NULL);
>>>>> +#endif
>>>>
>>>> Just wondering - instead of these two #ifdef-s, ...
>>>>
>>>>> --- a/include/xen/acpi.h
>>>>> +++ b/include/xen/acpi.h
>>>>> @@ -91,13 +91,9 @@ static inline int xen_acpi_get_gsi_info(struct pci_dev *dev,
>>>>>   }
>>>>>   #endif
>>>>>   
>>>>> -#ifdef CONFIG_XEN_PCI_STUB
>>>>> -int pcistub_get_gsi_from_sbdf(unsigned int sbdf);
>>>>> -#else
>>>>> -static inline int pcistub_get_gsi_from_sbdf(unsigned int sbdf)
>>>>> -{
>>>>> -	return -1;
>>>>> -}
>>>>> -#endif
>>>>> +typedef int (*get_gsi_from_sbdf_t)(u32 sbdf);
>>>>> +
>>>>> +void xen_acpi_register_get_gsi_func(get_gsi_from_sbdf_t func);
>>>>> +int xen_acpi_get_gsi_from_sbdf(u32 sbdf);
>>>>
>>>> ... wouldn't a static inline stub (for the !XEN_ACPI case) aid overall readability?
>>> I'm not sure if other files do this. But for me, it feels a little strange to use "#ifdef CONFIG_XEN_ACPI #else" in apci.h, like self-containment.
>>> And "#include apci.h" in pic_stub.c is also wraped with CONFIG_XEN_ACPI.
>> OK, I saw other files also do this.
>> If you insist, I will make modifications in the next version.
> 
> Well, I'm not a maintainer of this code, so I can't very well insist.

In this case I'm in favor of having the #ifdefs at the calling site.

The amount of code isn't larger this way, while it is more clear when
reading the code that the calls are only done in the CONFIG_XEN_ACPI
case.


Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3684 bytes)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (496 bytes)