Re: [PATCH 2/3] RDMA/hfi1, rdmavt: open-code rvt_set_ibdev_name()

[Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>]

On 3/23/26 4:48 AM, Arnd Bergmann wrote:
> I took a quick look at the hfi2 driver, and noticed a few things
> that that may be worth addressing before it gets merged, mostly
> stuff copied from hfi1:
> 
> - A few global functions with questionable namespacing:
>    user_event_ack, ctxt_reset, iowait_init, register_pinning_interface,
>    sc_{alloc,free,enable,disable}, pio_copy, acquire_hw_mutex,
>    load_firmware, cap_mask.
>    It would make sense to prefix all global identifiers with 'hfi2_',
>    both out of principle, and to allow building hfi1 and hfi2 into
>    an allyesconfig kernel without link failures.

Will address that stuff.

> - The use of INFINIBAND_RDMAVT seems unnecessary: right now
>    this is only used by hfi1, now shared with hfi2 but later to
>    be exclusive to the latter. Since it is unlikely to ever
>    be used by another driver again, this may be a good time
>    to drop the abstraction again and integrate it all into
>    hfi2, with the old version getting dropped along with hfi1.

Replied about RDMAVT stuff separately. Perhaps when we target removal of 
hfi1 that would be the time to get RDMAVT re-incorporated into the main 
hfi (hfi2) driver.

> 
> - The pio_copy() function is particularly slow since it uses
>    the full-barrier version of writeq() in a tight loop,
>    this should probably use __iowrite64_copy() etc to make it
>    work better on arm64 and other architectures

Will certainly look into this. Thanks for pointing it out.


> - The use of bitfields in drivers/infiniband/hw/hfi2/cport.h
>    makes the structures nonportable especially for big-endian
>    targets, if those describe device interfaces. Similarly
>    the use of __packed attributes in the same file seems
>    arbitrary and inconsistent, to the point where it
>    is likely to cause more harm than it can help. E.g.
>    in

Actually have a patch that addresses some of that. It's coming. We had 
previously only built on x86_64 but have plans to change that.


> +struct ib_cc_table_entry_shadow {
> +	u16 entry; /* shift:2, multiplier:14 */
> +};
> +
>>
> +struct ib_cc_table_attr_shadow {
> +	u16 ccti_limit; /* max CCTI for cc table */
> +	struct ib_cc_table_entry_shadow ccti_entries[IB_CCT_ENTRIES];
> +} __packed;
> 
>    the outer structure is unaligned while the inner one requires
>    alignment.
> 

Will take care of that one too.


-Denny