Page MenuHomeFreeBSD
Differential D3770

wpi(4): add support for TX fragmentation
ClosedPublic
Actions

Authored by avos on Oct 1 2015, 3:24 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 12, 9:16 PM
Unknown Object (File)
Tue, Apr 1, 9:15 PM
Unknown Object (File)
Feb 8 2025, 3:14 PM
Unknown Object (File)
Jan 16 2025, 8:07 PM
Unknown Object (File)
Jan 11 2025, 7:57 AM
Unknown Object (File)
Dec 21 2024, 12:41 PM
Unknown Object (File)
Nov 20 2024, 11:53 AM
Unknown Object (File)
Nov 20 2024, 7:01 AM
View All Files
Subscribers
imp

Details

Reviewers
adrian
Commits
rS289163: wpi(4): add support for TX fragmentation.
Test Plan

Tested with Intel 3945BG, HOSTAP and STA modes

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

avos updated this revision to Diff 9023.Oct 1 2015, 3:24 PM
avos retitled this revision from to wpi(4): add support for TX fragmentation.
avos updated this object.
avos edited the test plan for this revision. (Show Details)
avos added a reviewer: adrian.
avos set the repository for this revision to rS FreeBSD src repository - subversion.
Herald added a subscriber: imp. · View Herald TranscriptOct 1 2015, 3:24 PM
avos updated this revision to Diff 9024.Oct 1 2015, 3:29 PM

Fix order of operations.

avos added parent revisions: D3769: net80211: separate mbuf cleanup from ieee80211_fragment(), D3768: wpi(4): do not count failures twice for ic_raw_xmit(), D3767: wpi(4): move error handling upper (wpi_tx_data()/wpi_cmd2() -> wpi_raw_xmit()/wpi_transmit()), D3757: wpi(4): remove software queues.Oct 1 2015, 3:30 PM
avos updated this revision to Diff 9068.Oct 2 2015, 5:39 PM
  • Fix few races (TX/RX; deferred TX pointer commit w/ powersave enabled).
  • Improve error handling.
avos mentioned this in D3757: wpi(4): remove software queues.Oct 4 2015, 9:30 AM
Closed by commit rS289163: wpi(4): add support for TX fragmentation. (authored by adrian). · Explain WhyOct 12 2015, 4:05 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
dev/
wpi/
106 lines
2 lines
1 line

Diff 9023

View Options

sys/dev/wpi/if_wpi.c

Show First 20 Lines • Show All 81 Lines • ▼ Show 20 Lines echo " -I: Installation CDROM image location" \
"(default: ${DEFAULT_ISOFILE})" "(default: ${DEFAULT_ISOFILE})"
echo " -l: the OS loader to use (default: /boot/userboot.so)" echo " -l: the OS loader to use (default: /boot/userboot.so)"
echo " -L: IP address for UEFI GOP VNC server" \ echo " -L: IP address for UEFI GOP VNC server" \
"(default: ${DEFAULT_VNCHOST})" "(default: ${DEFAULT_VNCHOST})"
echo " -m: memory size (default: ${DEFAULT_MEMSIZE})" echo " -m: memory size (default: ${DEFAULT_MEMSIZE})"
echo " -n: network adapter emulation type" \ echo " -n: network adapter emulation type" \
"(default: ${DEFAULT_NIC})" "(default: ${DEFAULT_NIC})"
echo " -p: pass-through a host PCI device at bus/slot/func" \ echo " -p: pass-through a host PCI device at bus/slot/func" \
"(e.g. 10/0/0)" "(e.g. 10/0/0)"
markjUnsubmitted
Done
Inline Actions

In my experience, if you want to pass a device through to the VM, the host driver has to be detached, in which case it'll show up in pciconf -l output as none@pci.... So I'm not sure how this can work.

markj: In my experience, if you want to pass a device through to the VM, the host driver has to be…
christosAuthorUnsubmitted
Done
Inline Actions

The example is indeed incorrect, as the driver would have to be detached first. bhyve works by passing it a ppt driver, so none (although the script will still be able to fetch its bus/slot/func) isn't supposed to be used either.

https://wiki.freebsd.org/bhyve/pci_passthru

The example should say ppt0 instead of ehci0.

christos: The example is indeed incorrect, as the driver would have to be detached first. bhyve works by…
markjUnsubmitted
Done
Inline Actions

Ah right, ok.

Can we please also preserve backward compatibility? That is, if the passthrough device is of the form a/b/c, avoid treating it as a device name?

markj: Ah right, ok. Can we please also preserve backward compatibility? That is, if the passthrough…
christosAuthorUnsubmitted
Done
Inline Actions

Sure.

christos: Sure.
echo " -P: UEFI GOP VNC port (default: ${DEFAULT_VNCPORT})" echo " -P: UEFI GOP VNC port (default: ${DEFAULT_VNCPORT})"
echo " -t: tap device for virtio-net (default: $DEFAULT_TAPDEV)" echo " -t: tap device for virtio-net (default: $DEFAULT_TAPDEV)"
echo " -T: Enable tablet device (for UEFI GOP)" echo " -T: Enable tablet device (for UEFI GOP)"
echo " -u: RTC keeps UTC time" echo " -u: RTC keeps UTC time"
echo " -v: Wait for VNC client connection before booting VM" echo " -v: Wait for VNC client connection before booting VM"
echo " -w: ignore unimplemented MSRs" echo " -w: ignore unimplemented MSRs"
echo "" echo ""
[ -n "$msg" ] && errmsg "$msg" [ -n "$msg" ] && errmsg "$msg"
▲ Show 20 Lines • Show All 247 Lines • ▼ Show 20 Lines while [ $i -lt $disk_total ] ; do
devargs="$devargs -s $nextslot:0,$disk_emulation,${disk}${opts} " devargs="$devargs -s $nextslot:0,$disk_emulation,${disk}${opts} "
nextslot=$(($nextslot + 1)) nextslot=$(($nextslot + 1))
i=$(($i + 1)) i=$(($i + 1))
done done
i=0 i=0
while [ $i -lt $pass_total ] ; do while [ $i -lt $pass_total ] ; do
eval "pass=\$pass_dev${i}" eval "pass=\$pass_dev${i}"
devargs="$devargs -s $nextslot:0,passthru,${pass} " passno="$(
pciconf -l | grep "${pass}@" | awk '{print $1}' | cut -f2 -d@ |
markjUnsubmitted
Done
Inline Actions

Sorry, I meant to write "bsf".

markj: Sorry, I meant to write "bsf".
christosAuthorUnsubmitted
Done
Inline Actions

Sorry, I meant to write "bsf".

I didn't think about it at all, thanks.

christos: > Sorry, I meant to write "bsf". I didn't think about it at all, thanks.
cut -f2- -d: | sed 's/:$//' | tr : /
)"
markjUnsubmitted
Done
Inline Actions

I think you can replace this pipe with passno=$(pciconf -l | awk -F: '{print $2"/"$3"/"$4}').

markj: I think you can replace this pipe with `passno=$(pciconf -l | awk -F: '{print $2"/"$3"/"$4}')`.
christosAuthorUnsubmitted
Done
Inline Actions

Thank you, definitely better this way

christos: Thank you, definitely better this way
markjUnsubmitted
Done
Inline Actions

Sorry, I meant to write pciconf -l $pass| awk -F: '{print $2"/"$3"/"$4}'.

I'd also name the variable something more accurate, maybe "bsl" (bus slot function).

markj: Sorry, I meant to write `pciconf -l $pass| awk -F: '{print $2"/"$3"/"$4}'`. I'd also name the…
impUnsubmitted
Not Done
Inline Actions

Don't we need to preserve the domain as well?

imp: Don't we need to preserve the domain as well?
christosAuthorUnsubmitted
Done
Inline Actions

bhyve expects bus/slot/entry as input, so the domain field is not needed in this case.

christos: bhyve expects bus/slot/entry as input, so the domain field is not needed in this case.
test -z "${passno}" && errmsg "${pass} is not a host PCI device"
devargs="$devargs -s $nextslot:0,passthru,${passno} "
markjUnsubmitted
Not Done
Inline Actions
test -z "${passno}" && errmsg "${pass} is not a host PCI device"
- devargs="$devargs -s $nextslot:0,passthru,${passno} "
+ devargs="$devargs -s ${nextslot}:0,passthru,${passno} "
nextslot=$(($nextslot + 1))
markj:
christosAuthorUnsubmitted
Done
Inline Actions

I'll fix this in another commit because there are multiple other instances of this.

christos: I'll fix this in another commit because there are multiple other instances of this.
markjUnsubmitted
Done
Inline Actions
eval "pass=\$pass_dev${i}"
- if [ -z "$(echo "${pass}" | grep "^[0-9]*/[0-9]*/[0-9]*$")" ]; then
+ if [ -z "$(echo "${pass}" | grep "^[0-9]\+/[0-9]\+/[0-9]\+$")" ]; then
bsl="$(pciconf -l "${pass}" 2>/dev/null)"

[0-9]* matches the empty string, but we don't want that.

markj: [0-9]* matches the empty string, but we don't want that.
nextslot=$(($nextslot + 1)) nextslot=$(($nextslot + 1))
i=$(($i + 1)) i=$(($i + 1))
done done
efiargs="" efiargs=""
if [ ${efi_mode} -gt 0 ]; then if [ ${efi_mode} -gt 0 ]; then
efiargs="-s 29,fbuf,tcp=${vnchost}:${vncport}," efiargs="-s 29,fbuf,tcp=${vnchost}:${vncport},"
efiargs="${efiargs}${vncsize}${vncwait}" efiargs="${efiargs}${vncsize}${vncwait}"
Show All 36 Lines
View Options

sys/dev/wpi/if_wpireg.h

Show First 20 Lines • Show All 81 Lines • ▼ Show 20 Lines echo " -I: Installation CDROM image location" \
"(default: ${DEFAULT_ISOFILE})" "(default: ${DEFAULT_ISOFILE})"
echo " -l: the OS loader to use (default: /boot/userboot.so)" echo " -l: the OS loader to use (default: /boot/userboot.so)"
echo " -L: IP address for UEFI GOP VNC server" \ echo " -L: IP address for UEFI GOP VNC server" \
"(default: ${DEFAULT_VNCHOST})" "(default: ${DEFAULT_VNCHOST})"
echo " -m: memory size (default: ${DEFAULT_MEMSIZE})" echo " -m: memory size (default: ${DEFAULT_MEMSIZE})"
echo " -n: network adapter emulation type" \ echo " -n: network adapter emulation type" \
"(default: ${DEFAULT_NIC})" "(default: ${DEFAULT_NIC})"
echo " -p: pass-through a host PCI device at bus/slot/func" \ echo " -p: pass-through a host PCI device at bus/slot/func" \
"(e.g. 10/0/0)" "(e.g. 10/0/0)"
markjUnsubmitted
Done
Inline Actions

In my experience, if you want to pass a device through to the VM, the host driver has to be detached, in which case it'll show up in pciconf -l output as none@pci.... So I'm not sure how this can work.

markj: In my experience, if you want to pass a device through to the VM, the host driver has to be…
christosAuthorUnsubmitted
Done
Inline Actions

The example is indeed incorrect, as the driver would have to be detached first. bhyve works by passing it a ppt driver, so none (although the script will still be able to fetch its bus/slot/func) isn't supposed to be used either.

https://wiki.freebsd.org/bhyve/pci_passthru

The example should say ppt0 instead of ehci0.

christos: The example is indeed incorrect, as the driver would have to be detached first. bhyve works by…
markjUnsubmitted
Done
Inline Actions

Ah right, ok.

Can we please also preserve backward compatibility? That is, if the passthrough device is of the form a/b/c, avoid treating it as a device name?

markj: Ah right, ok. Can we please also preserve backward compatibility? That is, if the passthrough…
christosAuthorUnsubmitted
Done
Inline Actions

Sure.

christos: Sure.
echo " -P: UEFI GOP VNC port (default: ${DEFAULT_VNCPORT})" echo " -P: UEFI GOP VNC port (default: ${DEFAULT_VNCPORT})"
echo " -t: tap device for virtio-net (default: $DEFAULT_TAPDEV)" echo " -t: tap device for virtio-net (default: $DEFAULT_TAPDEV)"
echo " -T: Enable tablet device (for UEFI GOP)" echo " -T: Enable tablet device (for UEFI GOP)"
echo " -u: RTC keeps UTC time" echo " -u: RTC keeps UTC time"
echo " -v: Wait for VNC client connection before booting VM" echo " -v: Wait for VNC client connection before booting VM"
echo " -w: ignore unimplemented MSRs" echo " -w: ignore unimplemented MSRs"
echo "" echo ""
[ -n "$msg" ] && errmsg "$msg" [ -n "$msg" ] && errmsg "$msg"
▲ Show 20 Lines • Show All 247 Lines • ▼ Show 20 Lines while [ $i -lt $disk_total ] ; do
devargs="$devargs -s $nextslot:0,$disk_emulation,${disk}${opts} " devargs="$devargs -s $nextslot:0,$disk_emulation,${disk}${opts} "
nextslot=$(($nextslot + 1)) nextslot=$(($nextslot + 1))
i=$(($i + 1)) i=$(($i + 1))
done done
i=0 i=0
while [ $i -lt $pass_total ] ; do while [ $i -lt $pass_total ] ; do
eval "pass=\$pass_dev${i}" eval "pass=\$pass_dev${i}"
devargs="$devargs -s $nextslot:0,passthru,${pass} " passno="$(
pciconf -l | grep "${pass}@" | awk '{print $1}' | cut -f2 -d@ |
markjUnsubmitted
Done
Inline Actions

Sorry, I meant to write "bsf".

markj: Sorry, I meant to write "bsf".
christosAuthorUnsubmitted
Done
Inline Actions

Sorry, I meant to write "bsf".

I didn't think about it at all, thanks.

christos: > Sorry, I meant to write "bsf". I didn't think about it at all, thanks.
cut -f2- -d: | sed 's/:$//' | tr : /
)"
markjUnsubmitted
Done
Inline Actions

I think you can replace this pipe with passno=$(pciconf -l | awk -F: '{print $2"/"$3"/"$4}').

markj: I think you can replace this pipe with `passno=$(pciconf -l | awk -F: '{print $2"/"$3"/"$4}')`.
christosAuthorUnsubmitted
Done
Inline Actions

Thank you, definitely better this way

christos: Thank you, definitely better this way
markjUnsubmitted
Done
Inline Actions

Sorry, I meant to write pciconf -l $pass| awk -F: '{print $2"/"$3"/"$4}'.

I'd also name the variable something more accurate, maybe "bsl" (bus slot function).

markj: Sorry, I meant to write `pciconf -l $pass| awk -F: '{print $2"/"$3"/"$4}'`. I'd also name the…
impUnsubmitted
Not Done
Inline Actions

Don't we need to preserve the domain as well?

imp: Don't we need to preserve the domain as well?
christosAuthorUnsubmitted
Done
Inline Actions

bhyve expects bus/slot/entry as input, so the domain field is not needed in this case.

christos: bhyve expects bus/slot/entry as input, so the domain field is not needed in this case.
test -z "${passno}" && errmsg "${pass} is not a host PCI device"
devargs="$devargs -s $nextslot:0,passthru,${passno} "
markjUnsubmitted
Not Done
Inline Actions
test -z "${passno}" && errmsg "${pass} is not a host PCI device"
- devargs="$devargs -s $nextslot:0,passthru,${passno} "
+ devargs="$devargs -s ${nextslot}:0,passthru,${passno} "
nextslot=$(($nextslot + 1))
markj:
christosAuthorUnsubmitted
Done
Inline Actions

I'll fix this in another commit because there are multiple other instances of this.

christos: I'll fix this in another commit because there are multiple other instances of this.
markjUnsubmitted
Done
Inline Actions
eval "pass=\$pass_dev${i}"
- if [ -z "$(echo "${pass}" | grep "^[0-9]*/[0-9]*/[0-9]*$")" ]; then
+ if [ -z "$(echo "${pass}" | grep "^[0-9]\+/[0-9]\+/[0-9]\+$")" ]; then
bsl="$(pciconf -l "${pass}" 2>/dev/null)"

[0-9]* matches the empty string, but we don't want that.

markj: [0-9]* matches the empty string, but we don't want that.
nextslot=$(($nextslot + 1)) nextslot=$(($nextslot + 1))
i=$(($i + 1)) i=$(($i + 1))
done done
efiargs="" efiargs=""
if [ ${efi_mode} -gt 0 ]; then if [ ${efi_mode} -gt 0 ]; then
efiargs="-s 29,fbuf,tcp=${vnchost}:${vncport}," efiargs="-s 29,fbuf,tcp=${vnchost}:${vncport},"
efiargs="${efiargs}${vncsize}${vncwait}" efiargs="${efiargs}${vncsize}${vncwait}"
Show All 36 Lines
View Options

sys/dev/wpi/if_wpivar.h

Show First 20 Lines • Show All 81 Lines • ▼ Show 20 Lines echo " -I: Installation CDROM image location" \
"(default: ${DEFAULT_ISOFILE})" "(default: ${DEFAULT_ISOFILE})"
echo " -l: the OS loader to use (default: /boot/userboot.so)" echo " -l: the OS loader to use (default: /boot/userboot.so)"
echo " -L: IP address for UEFI GOP VNC server" \ echo " -L: IP address for UEFI GOP VNC server" \
"(default: ${DEFAULT_VNCHOST})" "(default: ${DEFAULT_VNCHOST})"
echo " -m: memory size (default: ${DEFAULT_MEMSIZE})" echo " -m: memory size (default: ${DEFAULT_MEMSIZE})"
echo " -n: network adapter emulation type" \ echo " -n: network adapter emulation type" \
"(default: ${DEFAULT_NIC})" "(default: ${DEFAULT_NIC})"
echo " -p: pass-through a host PCI device at bus/slot/func" \ echo " -p: pass-through a host PCI device at bus/slot/func" \
"(e.g. 10/0/0)" "(e.g. 10/0/0)"
markjUnsubmitted
Done
Inline Actions

In my experience, if you want to pass a device through to the VM, the host driver has to be detached, in which case it'll show up in pciconf -l output as none@pci.... So I'm not sure how this can work.

markj: In my experience, if you want to pass a device through to the VM, the host driver has to be…
christosAuthorUnsubmitted
Done
Inline Actions

The example is indeed incorrect, as the driver would have to be detached first. bhyve works by passing it a ppt driver, so none (although the script will still be able to fetch its bus/slot/func) isn't supposed to be used either.

https://wiki.freebsd.org/bhyve/pci_passthru

The example should say ppt0 instead of ehci0.

christos: The example is indeed incorrect, as the driver would have to be detached first. bhyve works by…
markjUnsubmitted
Done
Inline Actions

Ah right, ok.

Can we please also preserve backward compatibility? That is, if the passthrough device is of the form a/b/c, avoid treating it as a device name?

markj: Ah right, ok. Can we please also preserve backward compatibility? That is, if the passthrough…
christosAuthorUnsubmitted
Done
Inline Actions

Sure.

christos: Sure.
echo " -P: UEFI GOP VNC port (default: ${DEFAULT_VNCPORT})" echo " -P: UEFI GOP VNC port (default: ${DEFAULT_VNCPORT})"
echo " -t: tap device for virtio-net (default: $DEFAULT_TAPDEV)" echo " -t: tap device for virtio-net (default: $DEFAULT_TAPDEV)"
echo " -T: Enable tablet device (for UEFI GOP)" echo " -T: Enable tablet device (for UEFI GOP)"
echo " -u: RTC keeps UTC time" echo " -u: RTC keeps UTC time"
echo " -v: Wait for VNC client connection before booting VM" echo " -v: Wait for VNC client connection before booting VM"
echo " -w: ignore unimplemented MSRs" echo " -w: ignore unimplemented MSRs"
echo "" echo ""
[ -n "$msg" ] && errmsg "$msg" [ -n "$msg" ] && errmsg "$msg"
▲ Show 20 Lines • Show All 247 Lines • ▼ Show 20 Lines while [ $i -lt $disk_total ] ; do
devargs="$devargs -s $nextslot:0,$disk_emulation,${disk}${opts} " devargs="$devargs -s $nextslot:0,$disk_emulation,${disk}${opts} "
nextslot=$(($nextslot + 1)) nextslot=$(($nextslot + 1))
i=$(($i + 1)) i=$(($i + 1))
done done
i=0 i=0
while [ $i -lt $pass_total ] ; do while [ $i -lt $pass_total ] ; do
eval "pass=\$pass_dev${i}" eval "pass=\$pass_dev${i}"
devargs="$devargs -s $nextslot:0,passthru,${pass} " passno="$(
pciconf -l | grep "${pass}@" | awk '{print $1}' | cut -f2 -d@ |
markjUnsubmitted
Done
Inline Actions

Sorry, I meant to write "bsf".

markj: Sorry, I meant to write "bsf".
christosAuthorUnsubmitted
Done
Inline Actions

Sorry, I meant to write "bsf".

I didn't think about it at all, thanks.

christos: > Sorry, I meant to write "bsf". I didn't think about it at all, thanks.
cut -f2- -d: | sed 's/:$//' | tr : /
)"
markjUnsubmitted
Done
Inline Actions

I think you can replace this pipe with passno=$(pciconf -l | awk -F: '{print $2"/"$3"/"$4}').

markj: I think you can replace this pipe with `passno=$(pciconf -l | awk -F: '{print $2"/"$3"/"$4}')`.
christosAuthorUnsubmitted
Done
Inline Actions

Thank you, definitely better this way

christos: Thank you, definitely better this way
markjUnsubmitted
Done
Inline Actions

Sorry, I meant to write pciconf -l $pass| awk -F: '{print $2"/"$3"/"$4}'.

I'd also name the variable something more accurate, maybe "bsl" (bus slot function).

markj: Sorry, I meant to write `pciconf -l $pass| awk -F: '{print $2"/"$3"/"$4}'`. I'd also name the…
impUnsubmitted
Not Done
Inline Actions

Don't we need to preserve the domain as well?

imp: Don't we need to preserve the domain as well?
christosAuthorUnsubmitted
Done
Inline Actions

bhyve expects bus/slot/entry as input, so the domain field is not needed in this case.

christos: bhyve expects bus/slot/entry as input, so the domain field is not needed in this case.
test -z "${passno}" && errmsg "${pass} is not a host PCI device"
devargs="$devargs -s $nextslot:0,passthru,${passno} "
markjUnsubmitted
Not Done
Inline Actions
test -z "${passno}" && errmsg "${pass} is not a host PCI device"
- devargs="$devargs -s $nextslot:0,passthru,${passno} "
+ devargs="$devargs -s ${nextslot}:0,passthru,${passno} "
nextslot=$(($nextslot + 1))
markj:
christosAuthorUnsubmitted
Done
Inline Actions

I'll fix this in another commit because there are multiple other instances of this.

christos: I'll fix this in another commit because there are multiple other instances of this.
markjUnsubmitted
Done
Inline Actions
eval "pass=\$pass_dev${i}"
- if [ -z "$(echo "${pass}" | grep "^[0-9]*/[0-9]*/[0-9]*$")" ]; then
+ if [ -z "$(echo "${pass}" | grep "^[0-9]\+/[0-9]\+/[0-9]\+$")" ]; then
bsl="$(pciconf -l "${pass}" 2>/dev/null)"

[0-9]* matches the empty string, but we don't want that.

markj: [0-9]* matches the empty string, but we don't want that.
nextslot=$(($nextslot + 1)) nextslot=$(($nextslot + 1))
i=$(($i + 1)) i=$(($i + 1))
done done
efiargs="" efiargs=""
if [ ${efi_mode} -gt 0 ]; then if [ ${efi_mode} -gt 0 ]; then
efiargs="-s 29,fbuf,tcp=${vnchost}:${vncport}," efiargs="-s 29,fbuf,tcp=${vnchost}:${vncport},"
efiargs="${efiargs}${vncsize}${vncwait}" efiargs="${efiargs}${vncsize}${vncwait}"
Show All 36 Lines