Page MenuHomeFreeBSD

ntb: Add Xeon Gen3 support
ClosedPublic

Authored by markj on Oct 5 2020, 2:52 PM.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj requested review of this revision.Oct 5 2020, 2:52 PM
markj created this revision.

Looks good to me.

sys/dev/ntb/ntb_hw/ntb_hw_intel.c
1306 ↗(On Diff #77900)

Might want to keep the comment and assert on default.

sys/dev/ntb/ntb_hw/ntb_hw_intel.c
1306 ↗(On Diff #77900)

Note that the NTB type is an enum, so I'm not sure that having a default case makes sense.

1316 ↗(On Diff #77900)

Isilon's patch and Linux and DPDK perform 8-byte reads and writes for doorbells.

  • A few fixups, switch on the NTB hardware type to make code a bit clearer.
  • Update the man page.

No objection. I don’t really want to be an NTB maintainer, fwiw. I skimmed maybe half of this and it seemed reasonable.

This revision is now accepted and ready to land.Oct 13 2020, 4:59 PM
In D26683#596653, @cem wrote:

No objection. I don’t really want to be an NTB maintainer, fwiw. I skimmed maybe half of this and it seemed reasonable.

Thanks for looking. If I don't get any feedback in the next few days I'll just go ahead and commit.

Thanks for looking. If I don't get any feedback in the next few days I'll just go ahead and commit.

👌

vangyzen added a subscriber: vangyzen.

I'm not an NTB expert, but LGTM.

sys/dev/ntb/ntb_hw/ntb_hw_intel.c
1306 ↗(On Diff #77900)

It would catch a scribbler earlier.

1146 ↗(On Diff #78164)

desired_vectors was just assigned to num_vectors, so this is always true.

1155 ↗(On Diff #78164)

Indentation?

1840 ↗(On Diff #78164)

Indentation?

2147–2148 ↗(On Diff #78164)

Indentation?

I assume this is a rare case, but adding the sizes to the error message could be helpful.

3253–3256 ↗(On Diff #78164)

You could remove the condition and keep only limit = base + size;. Or maybe a different condition is needed?

markj added inline comments.
sys/dev/ntb/ntb_hw/ntb_hw_intel.c
1155 ↗(On Diff #78164)

I think this can just be removed.

3253–3256 ↗(On Diff #78164)

I believe we can just remove the conditional.

markj marked an inline comment as done.

Address feedback.

This revision now requires review to proceed.Oct 17 2020, 3:16 PM
This revision is now accepted and ready to land.Oct 18 2020, 10:18 PM
This revision was automatically updated to reflect the committed changes.