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

[Lucas De Marchi]

On Fri, May 23, 2025 at 06:32:43AM +1000, Dave Airlie wrote:
>On Fri, 23 May 2025 at 01:10, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>>
>> 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?
>
>oh interesting, my thoughts was of course nobody would want to use
>this for < 32-byte aligned ptrs :-)
>
>but I forgot about using the incr for stuff, I do wonder if the incr
>could be modelled on a base + offset, as I do think for a lot of stuff
>we'd want to retain the original vaddr for unmapping or other things,

for the parts of the code that want to update "inner blocks", the
approach is to copy the struct and operate on that. Example from
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:guc_prep_golden_context:

	info_map = IOSYS_MAP_INIT_OFFSET(&guc->ads_map,
					 offsetof(struct __guc_ads_blob, system_info));

then that function can manipulate its "local map" without affecting the
one used for really mapping the memory.

>
>I'll play around a bit more next week with at least protecting against bad uses.

thanks
Lucas De Marchi

>
>Dave.