Re: [PATCH v2] bus: mhi: host: Allocate entire MHI control config once

[Jeff Hugo <jeff.hugo@oss.qualcomm.com>]

On 5/12/2025 7:43 PM, Baochen Qiang wrote:
> 
> 
> On 5/13/2025 2:31 AM, Jeff Hugo wrote:
>> On 4/27/2025 7:57 PM, Baochen Qiang wrote:
>>>
>>>
>>> On 4/26/2025 1:04 AM, Jeff Hugo wrote:
>>>> From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
>>>>    int mhi_init_mmio(struct mhi_controller *mhi_cntrl)
>>>> @@ -475,6 +497,7 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl)
>>>>        struct mhi_event *mhi_event;
>>>>        void __iomem *base = mhi_cntrl->regs;
>>>>        struct device *dev = &mhi_cntrl->mhi_dev->dev;
>>>> +    dma_addr_t mhi_ctrl_limit = mhi_cntrl->ctrl_config_dma + mhi_cntrl-
>>>>> ctrl_config_size - 1;
>>>
>>> but the range [ctrl_config_dma, mhi_ctrl_limit] could still be possbile to cross the 4GB
>>> boundary, no?
>>
>> Its possible, yes.  From a practical standpoint, that seems to be unlikely as the control
>> area is not terribly large on supported platforms.
> 
> although unlikely, we need to handle it, right?
> 
> IMO a possible solution is that you allocate (2 * ctrl_config_size  - 1) bytes and take
> either the former half or the latter half, depending on which one does not cross the boundary.
> 

Ick. That is going to double the memory consumption for the lifetime of 
the device, and these are dma_alloc_coherent() allocations which usually 
pulls from a special pool, making it a limited resource in my view. qaic 
has usecases for 128 devices in a system, meaning 128 allocations are 
now doubled. That feels excessive.

Trying to allocate again may introduce fragmentation, which is also 
something I'd like to avoid.

I'm working to get the spec updated to remove this restriction, which 
would then mean only "legacy" devices are a concern. As far as I know, 
the number of MHI devices that actually require this handling are low 
compared to the set of all MHI devices.  Therefore I am hoping for a 
balance.

-Jeff