Hi Vladimir,
in some e-mail in this thread You wrote:
> Can this be computed from the device info rather than hardwired to 2K?
So, there is "draft" of patch which makes maybe little bit more
sophisticated limiting of bulk transfer length.
How it works:
I added new variable "max_bulk_tds" into USB controller structure
"grub_usb_controller_dev". This value is controller specific, i.e. it is
set inside each USB controller driver during its registering.
Meaning of this variable is simple: How much TDs can be maximally used
per one bulk transfer.
This value is used inside function grub_usb_bulk_read together with
value of max. packet length to calculate max. possible bulk transfer
length in bytes - which is used instead of previous constant
MAX_USB_TRANSFER_LEN.
(The constant MAX_USB_TRANSFER_LEN is still used as some "safe value" in
case when max_bulk_tds is set to zero from some reason. This "safety"
code cen be deleted.)
Main problem is how to calculate proper value of "max_bulk_tds".
There shouldn't be maximal count of all TDs - because some TDs should be
reserved for potential simultaneous transfers. Even the GRUB is single
threaded, there can happen some simultaneous transfers (e.g.
"interrupts" for hubs or keyboards, possibly bulk transfer related to
USB/RS232 converter etc.) - all these transfers uses the same "heap" of
TDs (but, fortunately, they are using only few TDs, usually approx. 1-8
per each such transfer).
There is a problem to estimate how much devices could simultaneously
communicate in all real cases.
The only one thing which I hope I can expect (according to actual GRUB
single-threading) is: There should be no more than one "large" (i.e.
disc related) bulk transfer at the same time. (If this will be not
valid, then it will be necessary to do something else to prevent TDs
"exhausting".)
So, initially I tried to use very simple calculating/estimating of
"max_bulk_tds" = max. count of TDs * 3/4.
As the count of all TDs is usually about 600 in each kind of USB driver,
I hope there will be more than sufficient amount of "reserved" TDs and
also relative big count of TDs for large bulk transfer.
But, unfortunately, we still have some hardwired numbers here - only
with another meaning (maybe potentially more reasonable - ?) and
controller-specific (if necessary).
What do you prefer - this "improved" bulk transfer limiting or the
simple original one?
BR,
Ales
diff -purB ./grub/grub-core/bus/usb/ehci.c ./grub_patched/grub-core/bus/usb/ehci.c
--- ./grub/grub-core/bus/usb/ehci.c 2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/ehci.c 2013-02-11 22:17:50.000000000 +0100
@@ -1902,7 +1897,9 @@ static struct grub_usb_controller_dev us
.cancel_transfer = grub_ehci_cancel_transfer,
.hubports = grub_ehci_hubports,
.portstatus = grub_ehci_portstatus,
- .detect_dev = grub_ehci_detect_dev
+ .detect_dev = grub_ehci_detect_dev,
+ /* estimated max. count of TDs for one bulk transfer */
+ .max_bulk_tds = GRUB_EHCI_N_TD * 3 / 4
};
GRUB_MOD_INIT (ehci)
diff -purB ./grub/grub-core/bus/usb/ohci.c ./grub_patched/grub-core/bus/usb/ohci.c
--- ./grub/grub-core/bus/usb/ohci.c 2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/ohci.c 2013-02-11 22:28:55.000000000 +0100
@@ -1431,7 +1431,9 @@ static struct grub_usb_controller_dev us
.cancel_transfer = grub_ohci_cancel_transfer,
.hubports = grub_ohci_hubports,
.portstatus = grub_ohci_portstatus,
- .detect_dev = grub_ohci_detect_dev
+ .detect_dev = grub_ohci_detect_dev,
+ /* estimated max. count of TDs for one bulk transfer */
+ .max_bulk_tds = GRUB_OHCI_TDS * 3 / 4
};
static struct grub_preboot *fini_hnd;
diff -purB ./grub/grub-core/bus/usb/uhci.c ./grub_patched/grub-core/bus/usb/uhci.c
--- ./grub/grub-core/bus/usb/uhci.c 2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/uhci.c 2013-02-11 22:28:37.000000000 +0100
@@ -823,7 +823,9 @@ static struct grub_usb_controller_dev us
.cancel_transfer = grub_uhci_cancel_transfer,
.hubports = grub_uhci_hubports,
.portstatus = grub_uhci_portstatus,
- .detect_dev = grub_uhci_detect_dev
+ .detect_dev = grub_uhci_detect_dev,
+ /* estimated max. count of TDs for one bulk transfer */
+ .max_bulk_tds = N_TD * 3 / 4
};
GRUB_MOD_INIT(uhci)
diff -purB ./grub/grub-core/bus/usb/usbtrans.c ./grub_patched/grub-core/bus/usb/usbtrans.c
--- ./grub/grub-core/bus/usb/usbtrans.c 2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/grub-core/bus/usb/usbtrans.c 2013-02-11 22:28:04.000000000 +0100
@@ -354,13 +354,36 @@ grub_usb_bulk_read (grub_usb_device_t de
grub_size_t actual, transferred;
grub_usb_err_t err;
grub_size_t current_size, position;
+ grub_size_t max_bulk_transfer_len = MAX_USB_TRANSFER_LEN;
+ grub_size_t max;
+
+ if (dev->controller.dev->max_bulk_tds)
+ {
+ /* Use the maximum packet size given in the endpoint descriptor. */
+ /* This must be the same "calculation" of "max" as inside function */
+ /* grub_usb_bulk_setup_readwrite (should we create some function */
+ /* for it to prevent problems?) */
+ if (dev->initialized)
+ {
+ struct grub_usb_desc_endp *endpdesc;
+ endpdesc = grub_usb_get_endpdescriptor (dev, endpoint);
+ if (endpdesc)
+ max = endpdesc->maxpacket;
+ else
+ max = 64;
+ }
+ else
+ max = 64;
+ /* Calculate max. possible length of bulk transfer */
+ max_bulk_transfer_len = dev->controller.dev->max_bulk_tds * max;
+ }
for (position = 0, transferred = 0;
- position < size; position += MAX_USB_TRANSFER_LEN)
+ position < size; position += max_bulk_transfer_len)
{
current_size = size - position;
- if (current_size >= MAX_USB_TRANSFER_LEN)
- current_size = MAX_USB_TRANSFER_LEN;
+ if (current_size >= max_bulk_transfer_len)
+ current_size = max_bulk_transfer_len;
err = grub_usb_bulk_readwrite (dev, endpoint, current_size,
&data[position], GRUB_USB_TRANSFER_TYPE_IN, 1000, &actual);
transferred += actual;
diff -purB ./grub/include/grub/usb.h ./grub_patched/include/grub/usb.h
--- ./grub/include/grub/usb.h 2013-01-27 22:12:17.000000000 +0100
+++ ./grub_patched/include/grub/usb.h 2013-02-11 22:15:51.000000000 +0100
@@ -124,6 +124,13 @@ struct grub_usb_controller_dev
/* Per controller flag - port reset pending, don't do another reset */
grub_uint64_t pending_reset;
+
+ /* Max. number of transfer descriptors used per one bulk transfer */
+ /* The reason is to prevent "exhausting" of TD by large bulk */
+ /* transfer - number of TD is limited in USB host driver */
+ /* Value is calculated/estimated in driver - some TDs should be */
+ /* reserved for posible concurrent control or "interrupt" transfers */
+ grub_size_t max_bulk_tds;
/* The next host controller. */
struct grub_usb_controller_dev *next;
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
