linux-kernel - Re: [PATCH] linux/pci.h: add a dummy implement for pci_clear

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]

Date:   Thu, 1 Jun 2023 01:37:39 +0800
From:   Sui Jingfeng <suijingfeng@...ngson.cn>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, loongson-kernel@...ts.loongnix.cn,
        kernel test robot <lkp@...el.com>
Subject: Re: [PATCH] linux/pci.h: add a dummy implement for pci_clear_master()

Hi,

On 2023/6/1 01:16, Bjorn Helgaas wrote:
> On Wed, May 31, 2023 at 12:25:10PM +0800, Sui Jingfeng wrote:
>> On 2023/5/31 04:11, Bjorn Helgaas wrote:
>>> On Tue, May 30, 2023 at 06:16:55PM +0800, Sui Jingfeng wrote:
>>>> As some arch(m68k for example) doesn't have config_pci enabled, drivers[1]
>>>> call pci_clear_master() without config_pci guard can not built.
>>>>
>>>>      drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c:
>>>>      In function 'etnaviv_gpu_pci_fini':
>>>>>> drivers/gpu/drm/etnaviv/etnaviv_pci_drv.c:32:9:
>>>>      error: implicit declaration of function 'pci_clear_master';
>>>>      did you mean 'pci_set_master'? [-Werror=implicit-function-declaration]
>>>>         32 |         pci_clear_master(pdev);
>>>>            |         ^~~~~~~~~~~~~~~~
>>>>            |         pci_set_master
>>>>      cc1: some warnings being treated as errors
>>>>
>>>> [1] https://patchwork.freedesktop.org/patch/539977/?series=118522&rev=1
>>> I don't mind adding a stub if it's needed, but I don't understand why
>>> it's needed here.
>> For a single driver that supports both platform devices and PCI devices,
>>
>> Sometimes there is no way to separate the PCI driver part and the platform
>> driver part cleanly and clearly.
>>
>> For example, the module_init() and module_exit() functions,
>>
>> where we have to register PCI drivers and platform drivers there.
>>
>> We can't simply let the entire driver depend on PCI in Kconfig,
>>
>> This will make this driver unable to compile, which it's originally could.
>>
>> The PCI core could do such a thing for us, and
>>
>> There is no need to introduce a driver-specific guard then.
>>
>>
>> There is already a dummy stub for pci_set_master().
>>
>> Therefore, pci_clear_master() should also have a counterpart.
>>
>> They should emerge in pairs.
>>
>> This could probably eliminate pain for PCI driver writers,
>>
>> This patch is still useful.
>>
>>
>>>    The caller is in etnaviv_pci_drv.c, and if I
>>> understand the patch at [1], etnaviv_pci_drv.c is only compiled when
>>> CONFIG_PCI=y.
>> Yes, you are right. This is the right thing to do for the driver, though.
>>
>> Pure PCI device driver does not need to worry about this.
>>
>> Like drm/ast, drm/amdgpu, drm/radeon, etc.
>>
>> But drm/etnaviv is special; it's a platform driver that could pass the
>> compile test originally.
>>
>>
>> When patching it (Etnaviv) with PCI device driver support,
>>
>> This forces the PCI driver writer to add another config option.
>>
>> (which depends on the PCI config option.) in the Kconfig.
>>
>> For my case, it's theDRM_ETNAVIV_PCI_DRIVER config option.
> So if I understand correctly, you would prefer not to add the
> DRM_ETNAVIV_PCI_DRIVER config option, and if we add this stub, you
> won't need to add it?
>
> That's a good reason to add this patch.

Yes, please add this patch.

Otherwise, other people may suffer from the same issue someday.

After  this patch added,  I will try respin my etnaviv patch set.

If the PCI core could shield the hazard for us.

I would prefer my etnaviv don't introduce any config option.

Compile anywhere at any case.

> Bjorn

-- 
Jingfeng