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

[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?

Dave.