[PATCH 9/9] iosys_map: embed the is_iomem bit into the pointer.

[Thomas Zimmermann]

Hi

Am 26.05.25 um 09:58 schrieb Dave Airlie:
> On Mon, 26 May 2025 at 16:39, Thomas Zimmermann <tzimmermann at suse.de> wrote:
>> Hi
>>
>> Am 22.05.25 um 17:09 schrieb Lucas De Marchi:
>>> On Thu, May 22, 2025 at 04:52:18PM +1000, Dave Airlie wrote:
>>>> From: Dave Airlie <airlied at redhat.com>
>>>>
>>>> This reduces this struct from 16 to 8 bytes, and it gets embedded
>>>> into a lot of things.
>>>>
>>>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>>> Replied too early on cover. Chatting with Michal Wajdeczko today, this
>>> may break things as we then can't byte-address anymore. It seems
>>> particularly dangerous when using the iosys_map_wr/iosys_map_rd as
>>> there's nothing preventing an unaligned address and we increment the map
>>> with the sizeof() of a struct that could be __packed. Example: in
>>> xe_guc_ads.c we use it to write packed structs like guc_gt_system_info.
>>> In this particular case it doesn't give unaligned address, but we should
>>> probably then protect iosys_map from doing the wrong thing.
>>>
>>> So, if we are keeping this patch, we should probably protect
>>> initially-unaligned addresses and the iosys_map_incr() call?
>> That sounds like a blocker to me. And there's another thing to keep in
>> mind. We have use cases where we need to know the caching of the memory
>> area. I never got to fully implement this, but it would be stored in the
>> iosys-map struct as well. We'd need 2 additional bits to represent UC,
>> WC and WT caching. If we don't have at least 3-bit alignment, shrinking
>> iosys-map might not be feasible anyway.
> I've played around a bit, and it's starting to seem like it might be
> difficult to get this across the line.
>
> I can add the iter stuff separately to fix the sub-dword offsets if
> needed, we probably don't want to be doing 8-bit iomem accesses
> anyways.
>
> But if we need 3-bits then it's messier, what's the use case out of
> interest to store the info? do you need all 3 states?

Various drivers perform format conversion or a memcpy of BO data as part 
of their display update. For buffers without caching, it's fastest to 
read the buffer once into system memory and do any further processing 
from there. If the buffer is already in cached memory, this would be 
overhead. Due to the lack of caching information, the gud driver treats 
all imported BOs as uncached. (Fourth argument at [1]). Storing the 
caching info in the iosys-map would benefit this driver and several others.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v6.15-rc7/source/drivers/gpu/drm/gud/gud_pipe.c#L446

>
> Dave.

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)