Re: [PATCH v3 1/9] PCI/P2PDMA: Add CONFIG_PCI_P2PDMA_CORE

[Matt Evans <matt@ozlabs.org>]

Hi Robin, me,

On 23/06/2026 16:59, Matt Evans wrote:
> Heya Robin,
> 
> On 23/06/2026 16:48, Robin Murphy wrote:
>> On 12/06/2026 3:31 pm, Matt Evans wrote:
>>> Hi Kevin, Pranjal, (+Robin, hi!)
>>
>> Oh hey there! :)
>>
>>> On 12/06/2026 04:39, Tian, Kevin wrote:
>>>>> From: Pranjal Shrivastava <praan@google.com>
>>>>> Sent: Friday, June 12, 2026 2:38 AM
>>>>>
>>>>> On Wed, Jun 10, 2026 at 04:43:15PM +0100, Matt Evans wrote:
>>>>>> --- a/drivers/pci/Kconfig
>>>>>> +++ b/drivers/pci/Kconfig
>>>>>> @@ -206,11 +206,7 @@ config PCIE_TPH
>>>>>>   config PCI_P2PDMA
>>>>>>       bool "PCI peer-to-peer transfer support"
>>>>>>       depends on ZONE_DEVICE
>>>>>> -    #
>>>>>> -    # The need for the scatterlist DMA bus address flag means PCI
>>>>> P2PDMA
>>>>>> -    # requires 64bit
>>>>>> -    #
>>>>>> -    depends on 64BIT
>>>>>> +    select PCI_P2PDMA_CORE
>>>>>>       select GENERIC_ALLOCATOR
>>>>>>       select NEED_SG_DMA_FLAGS
>>>>>>       help
>>>>>
>>>>> Nit: Did we drop depends on 64BIT intentionally here? I guess the full
>>>>> PCI_P2PDMA stack still selects NEED_SG_DMA_FLAGS? IIRC,
>>>>> NEED_SG_DMA_FLAGS doesn't select 64BIT?
>>>>
>>>> seems that comment is stale. According to the commit msg:
>>>>
>>>> " it would make vfio-pci only available if CONFIG_ZONE_DEVICE is
>>>> present (e.g. 64-bit systems), "
>>>>
>>>> so it sounds a redundant dependency hence is removed.
>>>
>>> This was intentional.  In practice there is still a dependency on 64BIT
>>> for PCI_P2PDMA, but it is because of ZONE_DEVICE (and mem hotplug).  The
>>> key need is PCI_P2PDMA_CORE is available on !64BIT for VFIO, but I
>>> didn't see a requirement from PCI_P2PDMA itself (as opposed to its
>>> dependencies).  If I've missed one, I can put it back...
>>>
>>> But NEED_SG_DMA_FLAGS doesn't smell quite right; I see from comments in
>>>
>>>    af2880ec44021 ("scatterlist: add dedicated config for DMA flags")
>>>
>>> that it assumes 64BIT, but it seems to be missing a "depends on 64BIT".
>>>
>>> Robin -- should that depend on 64BIT?
>>
>> Indeed, looking at the history it seems like that was overlooked, but it
>> worked out at the time since the only selector of NEED_SG_DMA_FLAGS was
>> PCI_P2PDMA as you say. If we're now generalising then moving the
>> explicit 64BIT dependency to NEED_SG_DMA_FLAGS itself sounds like the
>> right thing to do.
> 
> Cheers for confirming.  I'll send a patch separate to this series (since
> the deps work out OK for PCI_P2PDMA for the reasons mentioned).

I think we were wrong, NEED_SG_DMA_FLAGS doesn't _need_ 64BIT.

Other than P2PDMA, the other consumer of NEED_SG_DMA_FLAGS is IOMMU_DMA,
and turns out if one builds an i386 kernel with INTEL_IOMMU (or some
other configs, like Xen) then NEED_SG_DMA_FLAGS is enabled on 32-bit
builds too.

The scatterlist.h comments af2880ec44021 touched are just saying that
_since_ P2PDMA depends on 64BIT, there _is_ circumstantial padding so
let's use it for flags.  It doesn't require 64BIT.

For example struct scatterlist isn't pushed over some special (e.g.
power-of-two) size when NEED_SG_DMA_FLAGS is enabled on 32-bit; I can't
find a reason it should be prevented on 32-bit builds (and found cases
above in which it is already enabled in them).

So I won't change the NEED_SG_DMA_FLAGS dependencies after all.  Sorry
for the noise -- as ever if I've missed something do please explain.

(I'll continue with removing the P2PDMA dependency on 64BIT because it
seems P2PDMA's dependencies rely on 64BIT, though P2PDMA itself doesn't.)

Thanks,


Matt