See inline comment. This is very subtle stuff...
I didn't manage to trigger this in my brief testing, and the USB-HID
stall during USB Disk access is still not very difficult to reproduce.
So, this doesn't seem to have been the issue.
This is essentially a complete rewrite of the way TDs work, and a
significant change to how "normal" requests are submitted. In summation:
* There is now no such thing as a "descriptor chain". This was a concept
mostly carried over from EHCI and previous controllers that does not
really map to XHCI properly, as one "transfer descriptor" can contain
as many TRBs as are necessary to complete the transfer.
What we were really doing before was setting up multiple TRB sequences
owned by our xhci_td structure, and then linking them together with the
CHAIN bit set on each TRB, which in XHCI terms is *one* descriptor,
not multiple ones as we had it, and we were allocating the same amount
of memory anyway, so we might as well do it all in one structure.
So now xhci_td does not have a fixed size of TRBs, but rather a dynamic
one, and thus also a dynamic number of buffers. As part of this refactor,
xhci_td is now a data structure for our own use only, and so it is allocated
from the normal heap instead of as physical memory, and TRBs separately.
* Removing the distinction between "descriptor" and "descriptor chain" greatly
simplifies quite a lot of logic related to transfer handling, especially
in WriteDescriptor and ReadDescriptor, which no longer need to handle
writing to buffers across xhci_td boundaries.
* There is now a proper split between "trb_count" and "trb_used". The former
is the actual number of TRBs allocated in the data structure; the latter
is the number presently used in said data structure. This clarifies
quite a lot of code.
As part of this refactor, a number of other related issues were also cleaned up:
* TRB buffer sizes are now hard-coded to be 4x the Max Packet Size for the
given pipe. Previously they were 64KB, which is much larger than the spec
suggests. As we now size them exactly this way, we can set the endpoint's
Average TRB Length to this value also, which allows the controller to
better schedule transfers for us. This logic can probably be cleaned up
further in the future, even.
* We now write the "TD Size" field of Normal transfers (i.e. packet count,
not TRB count) properly, as it is now much easier to compute based on
our new TRB sizing logic.
* We now record the Max Packet Size for the endpoint in the endpoint
structure. (This will probably be more useful when we are dealing
with isochronous transfers later on.)
* Write the last cycle bit in _LinkDescriptorForPipe after everything else
has been written. This probably does not affect anything too seriously,
but it is technically more correct.
* Added section & page references to the specification in various comments.
* Added more error checking where applicable.
Some things still to be done that I noticed while working on this change:
* When we use a single large buffer and manually segment it rather than
calling AllocateChunk() for each one, there is a massive performance
gain (I saw 50MB/s -> 95MB/s in testing on some USB devices.) However,
we can't do this unconditionally, as the stack doesn't allocate physical
chunks larger than 32 * B_PAGE_SIZE, which is a problem for some filesystems
which can read/write in chunks of 1MB. I'll implement this in a later commit.
* Setting the IOC bit on the last TRB in a transfer is acceptable to find
out when the transfer is finished, but it isn't enough to get a proper
tally of how much data was transfered. This is what Event Status TRBs
are for; we should implement them.
* _LinkDescriptorForPipe has an overflow condition. I'll fix that in the
next commit.
Tested on a ThinkPad E550 (Intel Broadwell) with usb_disk and usb_hid.
Previously we were sending the hardware completely garbage values,
as CreateDescriptorChain clobbered the trbCount value it had set
by using it to determine how many more TRBs it needed to allocate,
and so it usually returned with the value set to 0.
Then SubmitNormalRequest() would immediately subtract 1, causing it
to become negative, and the loop would continue doing this, which also
violated the spec's notice that the field should be 31 if its "true
value" was >= 31, and of course 0 for the last TRB in the transfer.
The spec also says this should be the number of packets remaining, not
the number of TRBs remaining, which we also do not obey; but that
will be a problem for another time, so just add a TODO.
This clears out the area_ids, the pointers to them, and a variety
of other state information that will be invalid following the
deletion of the device.
Turns #12929 from a use-after-free into a NULL dereference panic.
It was removed before because bold text rendering was partially
broken. Now that Terminal uses fractional widths for character
sizes, we can restore it by default.
This matches the behavior of virtually all Linux terminal emulators
(they also appear to make bold text brighter.)
Suggested by korli in #11019. We pass this struct directly to the
BIOS, so pack it while we are here.
Checked against GRUB (it also does not have this field.)
Change-Id: I9a2708c44ff48fd702175053f2889443d6f7a003
Reviewed-on: https://review.haiku-os.org/c/1145
Reviewed-by: Adrien Destugues <pulkomandy@gmail.com>
These are simple structs, so hopefully GCC8 will be OK with us
memsetting them. We can't use the standard = {} route because
GCC2 does not support that.
Initialize each class members instead of memset()
for clearing PackageInfoAttributeValue.
Pointed out by gcc8.
Change-Id: I8bdb328e2271e49e840b1294dba9cca544805e72
Reviewed-on: https://review.haiku-os.org/c/1114
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
So that I can more easily import sparc headers from it.
NO_UNDERSCORES is gone and is now always the default.
Some previously arch-specific defines are now valid for all platforms.
Change-Id: If9876b241719559bdcb5cd9d8b1dc97e5e3d96b3
Reviewed-on: https://review.haiku-os.org/c/1141
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
Add empty implementation of timer, elf, vm, debugger support, to let the
kernel link.
Also add the kernel linker script.
Change-Id: If0795fa6554aea3df1ee544c25cc4832634ffd78
Reviewed-on: https://review.haiku-os.org/c/1108
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
Previous commit adding these was merged very quickly, so here's one
more...
Change-Id: I23c424db7631db1f0ec48e2d0ae47c8409ae6af2
Reviewed-on: https://review.haiku-os.org/c/1088
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
On x86 and x86_64, this warning is never emitted because it is perfectly
fine to do unaligned access. On sparc, such accesses are not supported
by the hardware and will generate a SIGBUS. This must be caught by a
trap handler, and the unaligned access performed there, slowly, using
byte by byte access.
However, making this a Werror is annoying because it will trigger
everytime one casts a byte pointer to something larger, even when
alignment is actually preserved. So, removing all such warnings would be
nearly impossible (for example, just for the mergesort function, there
is a whole GSoC project for it at FreeBSD).
Keep it as a warning for now. The warning can be silenced by using
BytePointer, if desired. We should also investigate where the SIGBUS
trap is triggered a lot and consider improving the alignment of data
where possible.
Change-Id: I6b90025e8c6d69ef1ccda3c10eee270ccc1ebd29
Reviewed-on: https://review.haiku-os.org/c/1103
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
I'm copypasting a lot from this, so I may as well clean it up while I'm
at it.
Change-Id: I9288c087abbf95475f980b5539f2fd19fad7f775
Reviewed-on: https://review.haiku-os.org/c/1136
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
During the compilation of LLVM version 8, the build failed because it depends
on a constant in this file. In hrev49042 all BSD headers were feature-gated by
_BSD_SOURCE. This is not done (for this file) in glibc and (obviously) not in
BSD's libc.
Since this is not common practise, I would propose removing the feature gate
for this header file, as it would mean that we would have to upstream patches
for ports of other software that depends on the availability of these
constants.
Change-Id: I486f0c2e87eff489ce92d03589a6299ef1be6ca5
Reviewed-on: https://review.haiku-os.org/c/1144
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
The tool "pkgman" was not showing it's help text
and this seems to be somehow related to the
initialization of constants such as
"kCommandCategoryPackages"; these values seems to
be coming through as empty-string for some reason.
I am changing those to be "#define" of regular
C-Strings and this seems to resolve the problem.
These values only seem to be used to group the
possible commands for production of the syntax or
help text - there do not seem to be any deeper
impacts beyond that functionality.
Change-Id: If9cd61462cd7f1f1b5ab2ece521bb3f00a1ba246
Reviewed-on: https://review.haiku-os.org/c/1139
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
Reviewed-by: Adrien Destugues <pulkomandy@gmail.com>
Operations that succeed should not take longer; ones that fail will.
Should get rid of ControllerReset() failed CMD_HCRST.
Change-Id: I4981a319bd64a076f2f404214a96d9909f0676de
Reviewed-on: https://review.haiku-os.org/c/1135
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
* use --no-as-needed on Linux as the default changed some time ago.
* adjust LD_LIBRARY_PATH/LIBRARY_PATH to load in the current directory.
* fix some builds of program with compile_lib.
Only dlopen_lookup_next1 fails on Ubuntu 18.04.02 x86_64.
Change-Id: I6ecf70f742f67ab24d7d00fa615baa209634d02c
Reviewed-on: https://review.haiku-os.org/c/1140
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
Some operating systems only ship with Python 3 and the
binary for this is 'python3' instead of 'python' which
causes the Jam build process to fail because it expects
to find 'python'. This change will mean that the
configure process will detect this case and configure
the build to use the correct binary name.
Fixes#14938
Change-Id: I30cd0df828792715a54d760b86dd79aee04e2b2f
Reviewed-on: https://review.haiku-os.org/c/1134
Reviewed-by: Adrien Destugues <pulkomandy@gmail.com>
In recent changes to HD's build files to generate
some sources at build-time there was a problem
with builds that were configured to run with some
element of concurrency. In this change, the steps
that should be in sequence are done with shell
commands to avoid race conditions.
Change-Id: I5b6fb4907d8ea5f3ca90de956ecce322cd89c685
Reviewed-on: https://review.haiku-os.org/c/1130
Reviewed-by: Adrien Destugues <pulkomandy@gmail.com>
Use the latest version of the compiler for bootstrap.
Change-Id: I43639b560de2d4f3dc3fed48c3d4bd32a544cb57
Reviewed-on: https://review.haiku-os.org/c/1104
Reviewed-by: Jérôme Duval <jerome.duval@gmail.com>
This is my first commit, so bear with me if I've violated any standards here!
I've bumped a few offsets to fix text clipping in the Get Info window. The proper
long-term fix is to recreate this window with the layout library, but that's
a substantially larger job.
Patch set 1
Before: https://i.imgur.com/S7Pl5Qv.png
After: https://i.imgur.com/bd3H1Kw.png
Patch set 3
French: https://i.imgur.com/rpmUb5T.png
German: https://i.imgur.com/ca9DecW.png
Portuguese: https://i.imgur.com/dE8sKFI.png
The font size in the Permissions drop-down is fixed. I had previously bumped it to
12, to be inline with the default font size present in a new Haiku install. However,
that produced text clipping for French and other locales. I reverted it back to 10,
and now longer strings fit as-is.
Change-Id: I7f4412b10074c76eb5b023a231bdb6b230c8f35a
Reviewed-on: https://review.haiku-os.org/c/1073
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
Remove memset(), since WebCamAddOn()::FillDefaultFlavorInfo()
at next line sets all flavor_info members except _reserved_[].
Change-Id: I41b4297a79303b61ccea914133b268b118b0d968
Reviewed-on: https://review.haiku-os.org/c/1118
Reviewed-by: Barrett17 <b.vitruvio@gmail.com>
Window must be locked or MidiRunThread crashes with a segment violation.
Change-Id: I9d5d0fa477475a9b5ba877aea3d6583690aacb2b
Reviewed-on: https://review.haiku-os.org/c/1080
Reviewed-by: Barrett17 <b.vitruvio@gmail.com>
We need to hold the endpoint lock while reading the TD list on
the endpoint, as otherwise we have no guarantee that the pointers
will not be modified while we are looking at them.
Since this is the only consumer of _UnlinkDescriptorForPipe, just
make that function assume a lock, and then do all locking within
HandleTransferComplete.
This went through review too fast, the wrong variable name was used so
the package name was not listed...
Change-Id: I81d4aa57fdb65297ae9f63ebf123d7a6395a99b6
Reviewed-on: https://review.haiku-os.org/c/1109
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
The Browse button was misplaced when the "Start up" setting was
introduced recently. Moving it back into the "Download folder" row.
Move the "Download folder" row to the bottom of the group.
Have the text field of the days-of-history BSpinner not stretch
the whole window width.
Change-Id: I00260126cf7594f435899fa821e8bf79bb475ba8
Reviewed-on: https://review.haiku-os.org/c/1110
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
