Re: [PATCH] firewire: Use bitwise instead of arithmetic operator for flags
On Mar 09 Jiapeng Chong wrote:
> Fix the following coccicheck warnings:
>
> ./drivers/firewire/core-device.c:973:22-23: WARNING: sum of probable
> bitmasks, consider |.
>
> ./drivers/firewire/core-device.c:954:22-23: WARNING: sum of probable
> bitmasks, consider |.
>
> Reported-by: Abaci Robot
> Signed-off-by: Jiapeng Chong
> ---
> drivers/firewire/core-device.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c
> index 6821698..e04832d 100644
> --- a/drivers/firewire/core-device.c
> +++ b/drivers/firewire/core-device.c
> @@ -951,7 +951,7 @@ static void set_broadcast_channel(struct fw_device
> *device, int generation)
> if (device->bc_implemented == BC_UNKNOWN) {
> rcode = fw_run_transaction(card, TCODE_READ_QUADLET_REQUEST,
> device->node_id, generation, device->max_speed,
> - CSR_REGISTER_BASE + CSR_BROADCAST_CHANNEL,
> + CSR_REGISTER_BASE | CSR_BROADCAST_CHANNEL,
> , 4);
> switch (rcode) {
> case RCODE_COMPLETE:
> @@ -970,7 +970,7 @@ static void set_broadcast_channel(struct fw_device
> *device, int generation)
> BROADCAST_CHANNEL_VALID);
> fw_run_transaction(card, TCODE_WRITE_QUADLET_REQUEST,
> device->node_id, generation, device->max_speed,
> - CSR_REGISTER_BASE + CSR_BROADCAST_CHANNEL,
> + CSR_REGISTER_BASE | CSR_BROADCAST_CHANNEL,
> , 4);
> }
> }
It's "base address + address offset". The arithmetic operator is correct.
--
Stefan Richter
-==--=-= --== -===-
http://arcgraph.de/sr/
Re: [PATCH] firewire: prevent integer overflow on 32bit systems
On Mar 02 Dan Carpenter wrote:
> In TCODE_STREAM_DATA mode, on 32bit systems, the "sizeof(*e) +
> request->length" operation can overflow leading to memory corruption.
>
> Fixes: 18e9b10fcdc0 ("firewire: cdev: add closure to async stream ioctl")
> Signed-off-by: Dan Carpenter
> ---
> drivers/firewire/core-cdev.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
> index fb6c651214f3..314de0384035 100644
> --- a/drivers/firewire/core-cdev.c
> +++ b/drivers/firewire/core-cdev.c
> @@ -587,6 +587,9 @@ static int init_request(struct client *client,
> request->length < 4)
> return -EINVAL;
>
> + if (request->length > ULONG_MAX - sizeof(*e))
> + return -EINVAL;
> +
> e = kmalloc(sizeof(*e) + request->length, GFP_KERNEL);
> if (e == NULL)
> return -ENOMEM;
There is already a length check for asynchronous stream requests.
It happens in ioctl_send_stream_packet().
--
Stefan Richter
-==--=-= --== ---=-
http://arcgraph.de/sr/
Re: [PATCH v2] firewire-core: remove cast of function callback
a->channel, a->speed, a->header_size, cb, client);
> > if (IS_ERR(context))
> > return PTR_ERR(context);
> > if (client->version < FW_CDEV_VERSION_AUTO_FLUSH_ISO_OVERFLOW)
> > diff --git a/include/linux/firewire.h b/include/linux/firewire.h
> > index aec8f30ab200..bff08118baaf 100644
> > --- a/include/linux/firewire.h
> > +++ b/include/linux/firewire.h
> > @@ -453,6 +453,22 @@ struct fw_iso_context {
> > struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
> > int type, int channel, int speed, size_t header_size,
> > fw_iso_callback_t callback, void *callback_data);
> > +
> > +static inline struct fw_iso_context *fw_iso_mc_context_create(
> > + struct fw_card *card,
> > + fw_iso_mc_callback_t callback,
> > + void *callback_data)
> > +{
> > + struct fw_iso_context *ctx;
> > +
> > + ctx = fw_iso_context_create(card, FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL,
> > + 0, 0, 0, NULL, callback_data);
> > + if (!IS_ERR(ctx))
> > + ctx->callback.mc = callback;
> > +
> > + return ctx;
> > +}
>
> Why is this in a .h file? What's wrong with just putting it in the .c
> file as a static function? There's no need to make this an inline,
> right?
There is no need for this in a header.
Furthermore, I prefer the original switch statement over the nested if/else.
Here is another proposal; I will resend it later as a proper patch.
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 719791819c24..bece1b69b43f 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -957,7 +957,6 @@ static int ioctl_create_iso_context(struct client *client,
union ioctl_arg *arg)
{
struct fw_cdev_create_iso_context *a = >create_iso_context;
struct fw_iso_context *context;
- fw_iso_callback_t cb;
int ret;
BUILD_BUG_ON(FW_CDEV_ISO_CONTEXT_TRANSMIT != FW_ISO_CONTEXT_TRANSMIT ||
@@ -969,20 +968,15 @@ static int ioctl_create_iso_context(struct client
*client, union ioctl_arg *arg)
case FW_ISO_CONTEXT_TRANSMIT:
if (a->speed > SCODE_3200 || a->channel > 63)
return -EINVAL;
-
- cb = iso_callback;
break;
case FW_ISO_CONTEXT_RECEIVE:
if (a->header_size < 4 || (a->header_size & 3) ||
a->channel > 63)
return -EINVAL;
-
- cb = iso_callback;
break;
case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL:
- cb = (fw_iso_callback_t)iso_mc_callback;
break;
default:
@@ -990,9 +984,15 @@ static int ioctl_create_iso_context(struct client *client,
union ioctl_arg *arg)
}
context = fw_iso_context_create(client->device->card, a->type,
- a->channel, a->speed, a->header_size, cb, client);
+ a->channel, a->speed, a->header_size, NULL, client);
if (IS_ERR(context))
return PTR_ERR(context);
+
+ if (a->type == FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL)
+ context->callback.mc = iso_mc_callback;
+ else
+ context->callback.sc = iso_callback;
+
if (client->version < FW_CDEV_VERSION_AUTO_FLUSH_ISO_OVERFLOW)
context->drop_overflow_headers = true;
--
Stefan Richter
-==--=-- -=-= ==--=
http://arcgraph.de/sr/
Re: [PATCH] firewire: mark expected switch fall-throughs
(added Cc: Mathieu Malaterre, who sent a patch which is a subset of this)
On Feb 11 Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch
> cases where we are expecting to fall through.
>
> This patch fixes the following warnings:
>
> drivers/firewire/core-device.c: In function ‘set_broadcast_channel’:
> drivers/firewire/core-device.c:969:7: warning: this statement may fall
> through [-Wimplicit-fallthrough=]
> if (data & cpu_to_be32(1 << 31)) {
>^
> drivers/firewire/core-device.c:974:3: note: here
>case RCODE_ADDRESS_ERROR:
>^~~~
> drivers/firewire/core-iso.c: In function ‘manage_channel’:
> drivers/firewire/core-iso.c:308:7: warning: this statement may fall through
> [-Wimplicit-fallthrough=]
> if ((data[0] & bit) == (data[1] & bit))
>^
> drivers/firewire/core-iso.c:312:3: note: here
>default:
>^~~
> drivers/firewire/core-topology.c: In function ‘count_ports’:
> drivers/firewire/core-topology.c:69:23: warning: this statement may fall
> through [-Wimplicit-fallthrough=]
> (*child_port_count)++;
> ~~~^~
> drivers/firewire/core-topology.c:70:3: note: here
>case SELFID_PORT_PARENT:
>^~~~
>
> Warning level 3 was used: -Wimplicit-fallthrough=3
>
> Notice that in some cases, the code comment is modified in
> accordance with what GCC is expecting to find.
OK, I looked up
https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Warning-Options.html#index-Wimplicit-fallthrough
now. I am loudly sighing and rolling my eyes... Anyway; the last regex
listed at the Wimplicit-fallthrough=3 bullet point is obviously the one
you are wanting to match.
> This patch is part of the ongoing efforts to enable
> -Wimplicit-fallthrough.
>
> Signed-off-by: Gustavo A. R. Silva
> ---
> drivers/firewire/core-device.c | 2 +-
> drivers/firewire/core-iso.c | 2 +-
> drivers/firewire/core-topology.c | 1 +
> 3 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c
> index 7c2eed76011e..0c86548fa4a7 100644
> --- a/drivers/firewire/core-device.c
> +++ b/drivers/firewire/core-device.c
> @@ -970,7 +970,7 @@ static void set_broadcast_channel(struct fw_device
> *device, int generation)
> device->bc_implemented = BC_IMPLEMENTED;
> break;
> }
> - /* else fall through to case address error */
> + /* else, fall through - to case address error */
> case RCODE_ADDRESS_ERROR:
> device->bc_implemented = BC_UNIMPLEMENTED;
> }
> diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
> index 35e784cffc23..7e5c98840b80 100644
> --- a/drivers/firewire/core-iso.c
> +++ b/drivers/firewire/core-iso.c
> @@ -308,7 +308,7 @@ static int manage_channel(struct fw_card *card, int
> irm_id, int generation,
> if ((data[0] & bit) == (data[1] & bit))
> continue;
>
> - /* 1394-1995 IRM, fall through to retry. */
> + /* fall through - to retry for 1394-1995 IRM */
If you don't mind, I will apply your patch with a different wording of this
comment:
/* fall through - It's an 1394-1995 IRM, retry. */
I'll mark my modification to your patch in the signed-off-by-tag.
> default:
> if (retry) {
> retry--;
> diff --git a/drivers/firewire/core-topology.c
> b/drivers/firewire/core-topology.c
> index 7db234d3fbdd..82c67e900aad 100644
> --- a/drivers/firewire/core-topology.c
> +++ b/drivers/firewire/core-topology.c
> @@ -67,6 +67,7 @@ static u32 *count_ports(u32 *sid, int *total_port_count,
> int *child_port_count)
> switch (port_type) {
> case SELFID_PORT_CHILD:
> (*child_port_count)++;
> + /* fall through */
> case SELFID_PORT_PARENT:
> case SELFID_PORT_NCONN:
> (*total_port_count)++;
--
Stefan Richter
-==---== --== -===-
http://arcgraph.de/sr/
[git pull] FireWire (IEEE 1394) update post v4.20
Linus, please pull from the tag "firewire-update" at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git firewire-update to receive the following firewire subsystem patch: - remove an explicit dependency in Kconfig which is implied by another dependency Geert Uytterhoeven (1): firewire: Remove depends on HAS_DMA in case of platform dependency drivers/firewire/Kconfig | 1 - 1 file changed, 1 deletion(-) Thanks, -- Stefan Richter -==---== ---= --=-= http://arcgraph.de/sr/
Re: [PATCH v4 resend] firewire: Remove depends on HAS_DMA in case of platform dependency
On Dec 03 Geert Uytterhoeven wrote: > Remove dependencies on HAS_DMA where a Kconfig symbol depends on another > symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST". > In most cases this other symbol is an architecture or platform specific > symbol, or PCI. > > Generic symbols and drivers without platform dependencies keep their > dependencies on HAS_DMA, to prevent compiling subsystems or drivers that > cannot work anyway. > > This simplifies the dependencies, and allows to improve compile-testing. > > Signed-off-by: Geert Uytterhoeven > Reviewed-by: Mark Brown > Acked-by: Robin Murphy Thanks for resending. Applied to linux1394.git #for-next. > --- > v4: > - Rebase to v4.18-rc1 (applies to next-20180622, too), > > v3: > - Rebase to v4.17-rc1, > > v2: > - Add Reviewed-by, Acked-by, > - Drop RFC state, > - Split per subsystem. > --- > drivers/firewire/Kconfig | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig > index 145974f9662b63e6..4199849e37585181 100644 > --- a/drivers/firewire/Kconfig > +++ b/drivers/firewire/Kconfig > @@ -1,5 +1,4 @@ > menu "IEEE 1394 (FireWire) support" > - depends on HAS_DMA > depends on PCI || COMPILE_TEST > # firewire-core does not depend on PCI but is > # not useful without PCI controller driver -- Stefan Richter -==---=- ==-- ---== http://arcgraph.de/sr/
Re: [PATCH v4 resend] firewire: Remove depends on HAS_DMA in case of platform dependency
On Dec 03 Geert Uytterhoeven wrote: > Remove dependencies on HAS_DMA where a Kconfig symbol depends on another > symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST". > In most cases this other symbol is an architecture or platform specific > symbol, or PCI. > > Generic symbols and drivers without platform dependencies keep their > dependencies on HAS_DMA, to prevent compiling subsystems or drivers that > cannot work anyway. > > This simplifies the dependencies, and allows to improve compile-testing. > > Signed-off-by: Geert Uytterhoeven > Reviewed-by: Mark Brown > Acked-by: Robin Murphy Thanks for resending. Applied to linux1394.git #for-next. > --- > v4: > - Rebase to v4.18-rc1 (applies to next-20180622, too), > > v3: > - Rebase to v4.17-rc1, > > v2: > - Add Reviewed-by, Acked-by, > - Drop RFC state, > - Split per subsystem. > --- > drivers/firewire/Kconfig | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig > index 145974f9662b63e6..4199849e37585181 100644 > --- a/drivers/firewire/Kconfig > +++ b/drivers/firewire/Kconfig > @@ -1,5 +1,4 @@ > menu "IEEE 1394 (FireWire) support" > - depends on HAS_DMA > depends on PCI || COMPILE_TEST > # firewire-core does not depend on PCI but is > # not useful without PCI controller driver -- Stefan Richter -==---=- ==-- ---== http://arcgraph.de/sr/
Re: [PATCH] firewire: nosy: don't read packets bigger than requested
On Sep 03 Randy Dunlap wrote:
> On 09/03/2018 08:55 AM, Jann Horn wrote:
> > On Fri, Jul 6, 2018 at 5:16 PM Jann Horn wrote:
> >> In general, accessing userspace memory beyond the length of the supplied
> >> buffer in VFS read/write handlers can lead to both kernel memory corruption
> >> (via kernel_read()/kernel_write(), which can e.g. be triggered via
> >> sys_splice()) and privilege escalation inside userspace.
> >>
> >> Fixes: 286468210d83 ("firewire: new driver: nosy - IEEE 1394 traffic
> >> sniffer")
> >> Signed-off-by: Jann Horn
[...]
> >> drivers/firewire/nosy.c | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
[...]
> > Ping. I sent this about two months ago, I haven't received a reply,
> > and from what I can tell, it hasn't landed in any tree so far...
> >
>
> :(
> I have that same problem with some Firewire documentation patches.
> I plan to ask someone else to merge my patches.
Jann,
sorry for not responding in July (was buried in other work and been
effectively absent from maintainership for many months). And sorry for
missing your ping in September (it must have been misplaced into the spam
folder and I apparently overlooked it there).
This week is another one in which I will not be able to check your patch.
Next week I will have a vacation of sorts and will use it to (a) review
and merge your patch and (b) clean out my mailbox and update my mail
sorting filters (long overdue after my mail service provider changed
backends).
Again sorry, and thank you for your extraordinary patience.
--
Stefan Richter
-==---=- =--= =--=-
http://arcgraph.de/sr/
Re: [PATCH] firewire: nosy: don't read packets bigger than requested
On Sep 03 Randy Dunlap wrote:
> On 09/03/2018 08:55 AM, Jann Horn wrote:
> > On Fri, Jul 6, 2018 at 5:16 PM Jann Horn wrote:
> >> In general, accessing userspace memory beyond the length of the supplied
> >> buffer in VFS read/write handlers can lead to both kernel memory corruption
> >> (via kernel_read()/kernel_write(), which can e.g. be triggered via
> >> sys_splice()) and privilege escalation inside userspace.
> >>
> >> Fixes: 286468210d83 ("firewire: new driver: nosy - IEEE 1394 traffic
> >> sniffer")
> >> Signed-off-by: Jann Horn
[...]
> >> drivers/firewire/nosy.c | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
[...]
> > Ping. I sent this about two months ago, I haven't received a reply,
> > and from what I can tell, it hasn't landed in any tree so far...
> >
>
> :(
> I have that same problem with some Firewire documentation patches.
> I plan to ask someone else to merge my patches.
Jann,
sorry for not responding in July (was buried in other work and been
effectively absent from maintainership for many months). And sorry for
missing your ping in September (it must have been misplaced into the spam
folder and I apparently overlooked it there).
This week is another one in which I will not be able to check your patch.
Next week I will have a vacation of sorts and will use it to (a) review
and merge your patch and (b) clean out my mailbox and update my mail
sorting filters (long overdue after my mail service provider changed
backends).
Again sorry, and thank you for your extraordinary patience.
--
Stefan Richter
-==---=- =--= =--=-
http://arcgraph.de/sr/
[git pull] FireWire (IEEE 1394) updates post v4.15
Linus, please pull from the tag "firewire-updates" at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git firewire-updates to receive the following IEEE 1394 subsystem patches: - make JMicron JMB38x controllers work with IOMMU-equipped systems - IP-over-1394: allow user-configured MTU of up to 4096 bytes Hector Martin (1): firewire-ohci: work around oversized DMA reads on JMicron controllers Stefan Richter (1): firewire: net: max MTU off by one drivers/firewire/net.c | 7 ++- drivers/firewire/ohci.c | 8 +++- 2 files changed, 13 insertions(+), 2 deletions(-) Thanks, -- Stefan Richter -==---=- --=- ---=- http://arcgraph.de/sr/
[git pull] FireWire (IEEE 1394) updates post v4.15
Linus, please pull from the tag "firewire-updates" at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git firewire-updates to receive the following IEEE 1394 subsystem patches: - make JMicron JMB38x controllers work with IOMMU-equipped systems - IP-over-1394: allow user-configured MTU of up to 4096 bytes Hector Martin (1): firewire-ohci: work around oversized DMA reads on JMicron controllers Stefan Richter (1): firewire: net: max MTU off by one drivers/firewire/net.c | 7 ++- drivers/firewire/ohci.c | 8 +++- 2 files changed, 13 insertions(+), 2 deletions(-) Thanks, -- Stefan Richter -==---=- --=- ---=- http://arcgraph.de/sr/
Re: [PATCH] firewire-ohci: work around oversized DMA reads on JMicron controllers
On Jan 11 Hector Martin 'marcan' wrote: > On 2017-11-13 06:05, Stefan Richter wrote: > > Thanks Hector for the troubleshooting and for the patch. > > Thanks Clemens for the review. > > > > It's been a while since I last reviewed and tested kernel patches, and > > also my main FireWire equipped PC is currently tied up in work for which > > reboots aren't desirable. But I am updating a long unused secondary > > FireWire'd PC right now and give the patch some testing this week. (This > > one even has a JMicron controller, but not an IOMMU.) > > > Hi Stefan, did you ever get around to testing the patch? It doesn't seem > to have made it into any trees or percolated up to mainline yet. Pushed to linux1394.git:for-next now. Sorry that I took so long. Thanks once more for your effort. (Getting the test computer up to date was a PITA; no fun when there is almost no spare time...) -- Stefan Richter -==---=- ---= -==-= http://arcgraph.de/sr/
Re: [PATCH] firewire-ohci: work around oversized DMA reads on JMicron controllers
On Jan 11 Hector Martin 'marcan' wrote: > On 2017-11-13 06:05, Stefan Richter wrote: > > Thanks Hector for the troubleshooting and for the patch. > > Thanks Clemens for the review. > > > > It's been a while since I last reviewed and tested kernel patches, and > > also my main FireWire equipped PC is currently tied up in work for which > > reboots aren't desirable. But I am updating a long unused secondary > > FireWire'd PC right now and give the patch some testing this week. (This > > one even has a JMicron controller, but not an IOMMU.) > > > Hi Stefan, did you ever get around to testing the patch? It doesn't seem > to have made it into any trees or percolated up to mainline yet. Pushed to linux1394.git:for-next now. Sorry that I took so long. Thanks once more for your effort. (Getting the test computer up to date was a PITA; no fun when there is almost no spare time...) -- Stefan Richter -==---=- ---= -==-= http://arcgraph.de/sr/
Re: [PATCH] firewire-ohci: work around oversized DMA reads on JMicron controllers
On Nov 03 Clemens Ladisch wrote: > Hector Martin wrote: > > At least some JMicron controllers issue buggy oversized DMA reads when > > fetching context descriptors, always fetching 0x20 bytes at once for > > descriptors which are only 0x10 bytes long. This is often harmless, but > > can cause page faults on modern systems with IOMMUs: > > > > DMAR: [DMA Read] Request device [05:00.0] fault addr fff56000 [fault reason > > 06] PTE Read access is not set > > firewire_ohci :05:00.0: DMA context IT0 has stopped, error code: > > evt_descriptor_read > > > > This works around the problem by always leaving 0x10 padding bytes at > > the end of descriptor buffer pages, which should be harmless to do > > unconditionally for controllers in case others have the same behavior. > > > > Signed-off-by: Hector Martin <mar...@marcan.st> > > Reviewed-by: Clemens Ladisch <clem...@ladisch.de> Thanks Hector for the troubleshooting and for the patch. Thanks Clemens for the review. It's been a while since I last reviewed and tested kernel patches, and also my main FireWire equipped PC is currently tied up in work for which reboots aren't desirable. But I am updating a long unused secondary FireWire'd PC right now and give the patch some testing this week. (This one even has a JMicron controller, but not an IOMMU.) > > --- > > drivers/firewire/ohci.c | 8 +++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c > > index 8bf89267dc25..d731b413cb2c 100644 > > --- a/drivers/firewire/ohci.c > > +++ b/drivers/firewire/ohci.c > > @@ -1130,7 +1130,13 @@ static int context_add_buffer(struct context *ctx) > > return -ENOMEM; > > > > offset = (void *)>buffer - (void *)desc; > > - desc->buffer_size = PAGE_SIZE - offset; > > + /* > > +* Some controllers, like JMicron ones, always issue 0x20-byte DMA reads > > +* for descriptors, even 0x10-byte ones. This can cause page faults when > > +* an IOMMU is in use and the oversized read crosses a page boundary. > > + * Work around this by always leaving at least 0x10 bytes of padding. > > +*/ > > + desc->buffer_size = PAGE_SIZE - offset - 0x10; > > desc->buffer_bus = bus_addr + offset; > > desc->used = 0; > > -- Stefan Richter -=== =-== -==-- http://arcgraph.de/sr/
Re: [PATCH] firewire-ohci: work around oversized DMA reads on JMicron controllers
On Nov 03 Clemens Ladisch wrote: > Hector Martin wrote: > > At least some JMicron controllers issue buggy oversized DMA reads when > > fetching context descriptors, always fetching 0x20 bytes at once for > > descriptors which are only 0x10 bytes long. This is often harmless, but > > can cause page faults on modern systems with IOMMUs: > > > > DMAR: [DMA Read] Request device [05:00.0] fault addr fff56000 [fault reason > > 06] PTE Read access is not set > > firewire_ohci :05:00.0: DMA context IT0 has stopped, error code: > > evt_descriptor_read > > > > This works around the problem by always leaving 0x10 padding bytes at > > the end of descriptor buffer pages, which should be harmless to do > > unconditionally for controllers in case others have the same behavior. > > > > Signed-off-by: Hector Martin > > Reviewed-by: Clemens Ladisch Thanks Hector for the troubleshooting and for the patch. Thanks Clemens for the review. It's been a while since I last reviewed and tested kernel patches, and also my main FireWire equipped PC is currently tied up in work for which reboots aren't desirable. But I am updating a long unused secondary FireWire'd PC right now and give the patch some testing this week. (This one even has a JMicron controller, but not an IOMMU.) > > --- > > drivers/firewire/ohci.c | 8 +++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c > > index 8bf89267dc25..d731b413cb2c 100644 > > --- a/drivers/firewire/ohci.c > > +++ b/drivers/firewire/ohci.c > > @@ -1130,7 +1130,13 @@ static int context_add_buffer(struct context *ctx) > > return -ENOMEM; > > > > offset = (void *)>buffer - (void *)desc; > > - desc->buffer_size = PAGE_SIZE - offset; > > + /* > > +* Some controllers, like JMicron ones, always issue 0x20-byte DMA reads > > +* for descriptors, even 0x10-byte ones. This can cause page faults when > > +* an IOMMU is in use and the oversized read crosses a page boundary. > > +* Work around this by always leaving at least 0x10 bytes of padding. > > +*/ > > + desc->buffer_size = PAGE_SIZE - offset - 0x10; > > desc->buffer_bus = bus_addr + offset; > > desc->used = 0; > > -- Stefan Richter -=== =-== -==-- http://arcgraph.de/sr/
Re: [PATCH] tools: firewire: nosy-dump: fix a resource leak in main()
On Sep 13 Martin Kepplinger wrote:
> If option_input and option_output is true two files are opened
> consecutively. In case the second fopen() fails, let's fclose()
> the first one before returning early.
>
> Signed-off-by: Martin Kepplinger <mart...@posteo.de>
> ---
> tools/firewire/nosy-dump.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/firewire/nosy-dump.c b/tools/firewire/nosy-dump.c
> index 3179c711bd65..228be406f206 100644
> --- a/tools/firewire/nosy-dump.c
> +++ b/tools/firewire/nosy-dump.c
> @@ -960,6 +960,8 @@ int main(int argc, const char *argv[])
> output = fopen(option_output, "w");
> if (output == NULL) {
> fprintf(stderr, "Could not open %s, %m\n",
> option_output);
> + if (input)
> + fclose(input);
> return -1;
> }
> }
When we return from main(), all files are closed implicitly.
--
Stefan Richter
-=== =--= ==-=-
http://arcgraph.de/sr/
Re: [PATCH] tools: firewire: nosy-dump: fix a resource leak in main()
On Sep 13 Martin Kepplinger wrote:
> If option_input and option_output is true two files are opened
> consecutively. In case the second fopen() fails, let's fclose()
> the first one before returning early.
>
> Signed-off-by: Martin Kepplinger
> ---
> tools/firewire/nosy-dump.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/firewire/nosy-dump.c b/tools/firewire/nosy-dump.c
> index 3179c711bd65..228be406f206 100644
> --- a/tools/firewire/nosy-dump.c
> +++ b/tools/firewire/nosy-dump.c
> @@ -960,6 +960,8 @@ int main(int argc, const char *argv[])
> output = fopen(option_output, "w");
> if (output == NULL) {
> fprintf(stderr, "Could not open %s, %m\n",
> option_output);
> + if (input)
> + fclose(input);
> return -1;
> }
> }
When we return from main(), all files are closed implicitly.
--
Stefan Richter
-=== =--= ==-=-
http://arcgraph.de/sr/
Re: [PATCH 3.2 144/152] firewire: net: guard against rx buffer overflows
On Nov 14 Ben Hutchings wrote: > 3.2.84-rc1 review patch. If anyone has any objections, please let me know. > > -- > > From: Stefan Richter <stef...@s5r6.in-berlin.de> > > commit 667121ace9dbafb368618dbabcf07901c962ddac upstream. [...] > [bwh: Backported to 3.2: fwnet_receive_broadcast() never matches IPv6 packets] > Signed-off-by: Ben Hutchings <b...@decadent.org.uk> Backport looks correct to me; thanks. -- Stefan Richter -==- =-== -===- http://arcgraph.de/sr/
Re: [PATCH 3.2 144/152] firewire: net: guard against rx buffer overflows
On Nov 14 Ben Hutchings wrote: > 3.2.84-rc1 review patch. If anyone has any objections, please let me know. > > -- > > From: Stefan Richter > > commit 667121ace9dbafb368618dbabcf07901c962ddac upstream. [...] > [bwh: Backported to 3.2: fwnet_receive_broadcast() never matches IPv6 packets] > Signed-off-by: Ben Hutchings Backport looks correct to me; thanks. -- Stefan Richter -==- =-== -===- http://arcgraph.de/sr/
FireWire/ nosy compile error (was gre...@linuxfoundation.org linux-kernel@vger.kernel.org)
On Nov 14 Karatas Ozgur wrote: > Hi, > > I fixed compile error, not include to FIREWIRE variable and only PCI return > an error. > > > diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig > > index 145974f..60e5a8c 100644 > --- a/drivers/firewire/Kconfig > +++ b/drivers/firewire/Kconfig > @@ -57,7 +57,7 @@ config FIREWIRE_NET > > config FIREWIRE_NOSY > tristate "Nosy - a FireWire traffic sniffer for PCILynx cards" > - depends on PCI > + depends on FIREWIRE && PCI > help > Nosy is an IEEE 1394 packet sniffer that is used for protocol > analysis and in development of IEEE 1394 drivers, applications, This is wrong. The variable FIREWIRE enables the firewire-core driver. But nosy is a stand-alone driver which does not depend on firewire-core. Please tell us what your compile error was (and on what platform), so that we can find out which dependency was really missing, if any. -- Stefan Richter -==- =-== -===- http://arcgraph.de/sr/
FireWire/ nosy compile error (was gre...@linuxfoundation.org linux-kernel@vger.kernel.org)
On Nov 14 Karatas Ozgur wrote: > Hi, > > I fixed compile error, not include to FIREWIRE variable and only PCI return > an error. > > > diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig > > index 145974f..60e5a8c 100644 > --- a/drivers/firewire/Kconfig > +++ b/drivers/firewire/Kconfig > @@ -57,7 +57,7 @@ config FIREWIRE_NET > > config FIREWIRE_NOSY > tristate "Nosy - a FireWire traffic sniffer for PCILynx cards" > - depends on PCI > + depends on FIREWIRE && PCI > help > Nosy is an IEEE 1394 packet sniffer that is used for protocol > analysis and in development of IEEE 1394 drivers, applications, This is wrong. The variable FIREWIRE enables the firewire-core driver. But nosy is a stand-alone driver which does not depend on firewire-core. Please tell us what your compile error was (and on what platform), so that we can find out which dependency was really missing, if any. -- Stefan Richter -==- =-== -===- http://arcgraph.de/sr/
[git pull] FireWire fixes
Linus, please pull from the tag "firewire-fixes" at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git firewire-fixes to receive the following FireWire (IEEE 1394) subsystem fixes: - Add missing input validation to the firewire-net driver. Invalid IP-over-1394 encapsulation headers could trigger buffer overflows (CVE 2016-8633). - IP-over-1394 link fragmentation headers were read and written incorrectly, breaking fragmented RX/TX with other OS's stacks. Stefan Richter (2): firewire: net: guard against rx buffer overflows firewire: net: fix fragmented datagram_size off-by-one drivers/firewire/net.c | 59 -- 1 file changed, 39 insertions(+), 20 deletions(-) Thanks, -- Stefan Richter -==- =-== --=-= http://arcgraph.de/sr/
[git pull] FireWire fixes
Linus, please pull from the tag "firewire-fixes" at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git firewire-fixes to receive the following FireWire (IEEE 1394) subsystem fixes: - Add missing input validation to the firewire-net driver. Invalid IP-over-1394 encapsulation headers could trigger buffer overflows (CVE 2016-8633). - IP-over-1394 link fragmentation headers were read and written incorrectly, breaking fragmented RX/TX with other OS's stacks. Stefan Richter (2): firewire: net: guard against rx buffer overflows firewire: net: fix fragmented datagram_size off-by-one drivers/firewire/net.c | 59 -- 1 file changed, 39 insertions(+), 20 deletions(-) Thanks, -- Stefan Richter -==- =-== --=-= http://arcgraph.de/sr/
[PATCH 3/3] firewire: net: max MTU off by one
The latest max_mtu patch missed that datagram_size is actually one less
than the datagram's Total Length.
Fixes: 357f4aae859b ("firewire: net: really fix maximum possible MTU")
Signed-off-by: Stefan Richter <stef...@s5r6.in-berlin.de>
---
To be applied after net-next was merged.
drivers/firewire/net.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 5d3640264f2d..655c259e37fd 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1482,9 +1482,14 @@ static int fwnet_probe(struct fw_unit *unit,
goto out;
dev->local_fifo = dev->handler.offset;
+ /*
+* default MTU: RFC 2734 cl. 4, RFC 3146 cl. 4
+* maximum MTU: RFC 2734 cl. 4.2, fragment encapsulation header's
+* maximum possible datagram_size + 1 = 0xfff + 1
+*/
net->mtu = 1500U;
net->min_mtu = ETH_MIN_MTU;
- net->max_mtu = 0xfff;
+ net->max_mtu = 4096U;
/* Set our hardware address while we're at it */
ha = (union fwnet_hwaddr *)net->dev_addr;
--
Stefan Richter
-==- =-== ---=-
http://arcgraph.de/sr/
[PATCH 3/3] firewire: net: max MTU off by one
The latest max_mtu patch missed that datagram_size is actually one less
than the datagram's Total Length.
Fixes: 357f4aae859b ("firewire: net: really fix maximum possible MTU")
Signed-off-by: Stefan Richter
---
To be applied after net-next was merged.
drivers/firewire/net.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 5d3640264f2d..655c259e37fd 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1482,9 +1482,14 @@ static int fwnet_probe(struct fw_unit *unit,
goto out;
dev->local_fifo = dev->handler.offset;
+ /*
+* default MTU: RFC 2734 cl. 4, RFC 3146 cl. 4
+* maximum MTU: RFC 2734 cl. 4.2, fragment encapsulation header's
+* maximum possible datagram_size + 1 = 0xfff + 1
+*/
net->mtu = 1500U;
net->min_mtu = ETH_MIN_MTU;
- net->max_mtu = 0xfff;
+ net->max_mtu = 4096U;
/* Set our hardware address while we're at it */
ha = (union fwnet_hwaddr *)net->dev_addr;
--
Stefan Richter
-==- =-== ---=-
http://arcgraph.de/sr/
[PATCH 2/3] firewire: net: fix fragmented datagram_size off-by-one
Date: Sun, 30 Oct 2016 17:32:01 +0100
RFC 2734 defines the datagram_size field in fragment encapsulation
headers thus:
datagram_size: The encoded size of the entire IP datagram. The
value of datagram_size [...] SHALL be one less than the value of
Total Length in the datagram's IP header (see STD 5, RFC 791).
Accordingly, the eth1394 driver of Linux 2.6.36 and older set and got
this field with a -/+1 offset:
ether1394_tx() /* transmit */
ether1394_encapsulate_prep()
hdr->ff.dg_size = dg_size - 1;
ether1394_data_handler() /* receive */
if (hdr->common.lf == ETH1394_HDR_LF_FF)
dg_size = hdr->ff.dg_size + 1;
else
dg_size = hdr->sf.dg_size + 1;
Likewise, I observe OS X 10.4 and Windows XP Pro SP3 to transmit 1500
byte sized datagrams in fragments with datagram_size=1499 if link
fragmentation is required.
Only firewire-net sets and gets datagram_size without this offset. The
result is lacking interoperability of firewire-net with OS X, Windows
XP, and presumably Linux' eth1394. (I did not test with the latter.)
For example, FTP data transfers to a Linux firewire-net box with max_rec
smaller than the 1500 bytes MTU
- from OS X fail entirely,
- from Win XP start out with a bunch of fragmented datagrams which
time out, then continue with unfragmented datagrams because Win XP
temporarily reduces the MTU to 576 bytes.
So let's fix firewire-net's datagram_size accessors.
Note that firewire-net thereby loses interoperability with unpatched
firewire-net, but only if link fragmentation is employed. (This happens
with large broadcast datagrams, and with large datagrams on several
FireWire CardBus cards with smaller max_rec than equivalent PCI cards,
and it can be worked around by setting a small enough MTU.)
Cc: sta...@vger.kernel.org
Signed-off-by: Stefan Richter <stef...@s5r6.in-berlin.de>
---
drivers/firewire/net.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -73,13 +73,13 @@ struct rfc2734_header {
#define fwnet_get_hdr_lf(h)(((h)->w0 & 0xc000) >> 30)
#define fwnet_get_hdr_ether_type(h)(((h)->w0 & 0x))
-#define fwnet_get_hdr_dg_size(h) (((h)->w0 & 0x0fff) >> 16)
+#define fwnet_get_hdr_dg_size(h) h)->w0 & 0x0fff) >> 16) + 1)
#define fwnet_get_hdr_fg_off(h)(((h)->w0 & 0x0fff))
#define fwnet_get_hdr_dgl(h) (((h)->w1 & 0x) >> 16)
-#define fwnet_set_hdr_lf(lf) ((lf) << 30)
+#define fwnet_set_hdr_lf(lf) ((lf) << 30)
#define fwnet_set_hdr_ether_type(et) (et)
-#define fwnet_set_hdr_dg_size(dgs) ((dgs) << 16)
+#define fwnet_set_hdr_dg_size(dgs) (((dgs) - 1) << 16)
#define fwnet_set_hdr_fg_off(fgo) (fgo)
#define fwnet_set_hdr_dgl(dgl) ((dgl) << 16)
@@ -622,7 +622,7 @@ static int fwnet_incoming_packet(struct
fg_off = fwnet_get_hdr_fg_off();
}
datagram_label = fwnet_get_hdr_dgl();
- dg_size = fwnet_get_hdr_dg_size(); /* ??? + 1 */
+ dg_size = fwnet_get_hdr_dg_size();
if (fg_off + len > dg_size)
return 0;
--
Stefan Richter
-==- =-== ---=-
http://arcgraph.de/sr/
[PATCH 2/3] firewire: net: fix fragmented datagram_size off-by-one
Date: Sun, 30 Oct 2016 17:32:01 +0100
RFC 2734 defines the datagram_size field in fragment encapsulation
headers thus:
datagram_size: The encoded size of the entire IP datagram. The
value of datagram_size [...] SHALL be one less than the value of
Total Length in the datagram's IP header (see STD 5, RFC 791).
Accordingly, the eth1394 driver of Linux 2.6.36 and older set and got
this field with a -/+1 offset:
ether1394_tx() /* transmit */
ether1394_encapsulate_prep()
hdr->ff.dg_size = dg_size - 1;
ether1394_data_handler() /* receive */
if (hdr->common.lf == ETH1394_HDR_LF_FF)
dg_size = hdr->ff.dg_size + 1;
else
dg_size = hdr->sf.dg_size + 1;
Likewise, I observe OS X 10.4 and Windows XP Pro SP3 to transmit 1500
byte sized datagrams in fragments with datagram_size=1499 if link
fragmentation is required.
Only firewire-net sets and gets datagram_size without this offset. The
result is lacking interoperability of firewire-net with OS X, Windows
XP, and presumably Linux' eth1394. (I did not test with the latter.)
For example, FTP data transfers to a Linux firewire-net box with max_rec
smaller than the 1500 bytes MTU
- from OS X fail entirely,
- from Win XP start out with a bunch of fragmented datagrams which
time out, then continue with unfragmented datagrams because Win XP
temporarily reduces the MTU to 576 bytes.
So let's fix firewire-net's datagram_size accessors.
Note that firewire-net thereby loses interoperability with unpatched
firewire-net, but only if link fragmentation is employed. (This happens
with large broadcast datagrams, and with large datagrams on several
FireWire CardBus cards with smaller max_rec than equivalent PCI cards,
and it can be worked around by setting a small enough MTU.)
Cc: sta...@vger.kernel.org
Signed-off-by: Stefan Richter
---
drivers/firewire/net.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -73,13 +73,13 @@ struct rfc2734_header {
#define fwnet_get_hdr_lf(h)(((h)->w0 & 0xc000) >> 30)
#define fwnet_get_hdr_ether_type(h)(((h)->w0 & 0x))
-#define fwnet_get_hdr_dg_size(h) (((h)->w0 & 0x0fff) >> 16)
+#define fwnet_get_hdr_dg_size(h) h)->w0 & 0x0fff) >> 16) + 1)
#define fwnet_get_hdr_fg_off(h)(((h)->w0 & 0x0fff))
#define fwnet_get_hdr_dgl(h) (((h)->w1 & 0x) >> 16)
-#define fwnet_set_hdr_lf(lf) ((lf) << 30)
+#define fwnet_set_hdr_lf(lf) ((lf) << 30)
#define fwnet_set_hdr_ether_type(et) (et)
-#define fwnet_set_hdr_dg_size(dgs) ((dgs) << 16)
+#define fwnet_set_hdr_dg_size(dgs) (((dgs) - 1) << 16)
#define fwnet_set_hdr_fg_off(fgo) (fgo)
#define fwnet_set_hdr_dgl(dgl) ((dgl) << 16)
@@ -622,7 +622,7 @@ static int fwnet_incoming_packet(struct
fg_off = fwnet_get_hdr_fg_off();
}
datagram_label = fwnet_get_hdr_dgl();
- dg_size = fwnet_get_hdr_dg_size(); /* ??? + 1 */
+ dg_size = fwnet_get_hdr_dg_size();
if (fg_off + len > dg_size)
return 0;
--
Stefan Richter
-==- =-== ---=-
http://arcgraph.de/sr/
[PATCH 1/3] firewire: net: guard against rx buffer overflows
Date: Sat, 29 Oct 2016 21:28:18 +0200
The IP-over-1394 driver firewire-net lacked input validation when
handling incoming fragmented datagrams. A maliciously formed fragment
with a respectively large datagram_offset would cause a memcpy past the
datagram buffer.
So, drop any packets carrying a fragment with offset + length larger
than datagram_size.
In addition, ensure that
- GASP header, unfragmented encapsulation header, or fragment
encapsulation header actually exists before we access it,
- the encapsulated datagram or fragment is of nonzero size.
Reported-by: Eyal Itkin <eyal.it...@gmail.com>
Cc: sta...@vger.kernel.org
Signed-off-by: Stefan Richter <stef...@s5r6.in-berlin.de>
---
drivers/firewire/net.c | 51 -
1 file changed, 35 insertions(+), 16 deletions(-)
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -578,6 +578,9 @@ static int fwnet_incoming_packet(struct
int retval;
u16 ether_type;
+ if (len <= RFC2374_UNFRAG_HDR_SIZE)
+ return 0;
+
hdr.w0 = be32_to_cpu(buf[0]);
lf = fwnet_get_hdr_lf();
if (lf == RFC2374_HDR_UNFRAG) {
@@ -602,7 +605,12 @@ static int fwnet_incoming_packet(struct
return fwnet_finish_incoming_packet(net, skb, source_node_id,
is_broadcast, ether_type);
}
+
/* A datagram fragment has been received, now the fun begins. */
+
+ if (len <= RFC2374_FRAG_HDR_SIZE)
+ return 0;
+
hdr.w1 = ntohl(buf[1]);
buf += 2;
len -= RFC2374_FRAG_HDR_SIZE;
@@ -616,6 +624,9 @@ static int fwnet_incoming_packet(struct
datagram_label = fwnet_get_hdr_dgl();
dg_size = fwnet_get_hdr_dg_size(); /* ??? + 1 */
+ if (fg_off + len > dg_size)
+ return 0;
+
spin_lock_irqsave(>lock, flags);
peer = fwnet_peer_find_by_node_id(dev, source_node_id, generation);
@@ -722,6 +733,22 @@ static void fwnet_receive_packet(struct
fw_send_response(card, r, rcode);
}
+static int gasp_source_id(__be32 *p)
+{
+ return be32_to_cpu(p[0]) >> 16;
+}
+
+static u32 gasp_specifier_id(__be32 *p)
+{
+ return (be32_to_cpu(p[0]) & 0x) << 8 |
+ (be32_to_cpu(p[1]) & 0xff00) >> 24;
+}
+
+static u32 gasp_version(__be32 *p)
+{
+ return be32_to_cpu(p[1]) & 0xff;
+}
+
static void fwnet_receive_broadcast(struct fw_iso_context *context,
u32 cycle, size_t header_length, void *header, void *data)
{
@@ -731,9 +758,6 @@ static void fwnet_receive_broadcast(stru
__be32 *buf_ptr;
int retval;
u32 length;
- u16 source_node_id;
- u32 specifier_id;
- u32 ver;
unsigned long offset;
unsigned long flags;
@@ -750,22 +774,17 @@ static void fwnet_receive_broadcast(stru
spin_unlock_irqrestore(>lock, flags);
- specifier_id =(be32_to_cpu(buf_ptr[0]) & 0x) << 8
- | (be32_to_cpu(buf_ptr[1]) & 0xff00) >> 24;
- ver = be32_to_cpu(buf_ptr[1]) & 0xff;
- source_node_id = be32_to_cpu(buf_ptr[0]) >> 16;
-
- if (specifier_id == IANA_SPECIFIER_ID &&
- (ver == RFC2734_SW_VERSION
+ if (length > IEEE1394_GASP_HDR_SIZE &&
+ gasp_specifier_id(buf_ptr) == IANA_SPECIFIER_ID &&
+ (gasp_version(buf_ptr) == RFC2734_SW_VERSION
#if IS_ENABLED(CONFIG_IPV6)
-|| ver == RFC3146_SW_VERSION
+|| gasp_version(buf_ptr) == RFC3146_SW_VERSION
#endif
- )) {
- buf_ptr += 2;
- length -= IEEE1394_GASP_HDR_SIZE;
- fwnet_incoming_packet(dev, buf_ptr, length, source_node_id,
+ ))
+ fwnet_incoming_packet(dev, buf_ptr + 2,
+ length - IEEE1394_GASP_HDR_SIZE,
+ gasp_source_id(buf_ptr),
context->card->generation, true);
- }
packet.payload_length = dev->rcv_buffer_size;
packet.interrupt = 1;
--
Stefan Richter
-==- =-== ---=-
http://arcgraph.de/sr/
[PATCH 1/3] firewire: net: guard against rx buffer overflows
Date: Sat, 29 Oct 2016 21:28:18 +0200
The IP-over-1394 driver firewire-net lacked input validation when
handling incoming fragmented datagrams. A maliciously formed fragment
with a respectively large datagram_offset would cause a memcpy past the
datagram buffer.
So, drop any packets carrying a fragment with offset + length larger
than datagram_size.
In addition, ensure that
- GASP header, unfragmented encapsulation header, or fragment
encapsulation header actually exists before we access it,
- the encapsulated datagram or fragment is of nonzero size.
Reported-by: Eyal Itkin
Cc: sta...@vger.kernel.org
Signed-off-by: Stefan Richter
---
drivers/firewire/net.c | 51 -
1 file changed, 35 insertions(+), 16 deletions(-)
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -578,6 +578,9 @@ static int fwnet_incoming_packet(struct
int retval;
u16 ether_type;
+ if (len <= RFC2374_UNFRAG_HDR_SIZE)
+ return 0;
+
hdr.w0 = be32_to_cpu(buf[0]);
lf = fwnet_get_hdr_lf();
if (lf == RFC2374_HDR_UNFRAG) {
@@ -602,7 +605,12 @@ static int fwnet_incoming_packet(struct
return fwnet_finish_incoming_packet(net, skb, source_node_id,
is_broadcast, ether_type);
}
+
/* A datagram fragment has been received, now the fun begins. */
+
+ if (len <= RFC2374_FRAG_HDR_SIZE)
+ return 0;
+
hdr.w1 = ntohl(buf[1]);
buf += 2;
len -= RFC2374_FRAG_HDR_SIZE;
@@ -616,6 +624,9 @@ static int fwnet_incoming_packet(struct
datagram_label = fwnet_get_hdr_dgl();
dg_size = fwnet_get_hdr_dg_size(); /* ??? + 1 */
+ if (fg_off + len > dg_size)
+ return 0;
+
spin_lock_irqsave(>lock, flags);
peer = fwnet_peer_find_by_node_id(dev, source_node_id, generation);
@@ -722,6 +733,22 @@ static void fwnet_receive_packet(struct
fw_send_response(card, r, rcode);
}
+static int gasp_source_id(__be32 *p)
+{
+ return be32_to_cpu(p[0]) >> 16;
+}
+
+static u32 gasp_specifier_id(__be32 *p)
+{
+ return (be32_to_cpu(p[0]) & 0x) << 8 |
+ (be32_to_cpu(p[1]) & 0xff00) >> 24;
+}
+
+static u32 gasp_version(__be32 *p)
+{
+ return be32_to_cpu(p[1]) & 0xff;
+}
+
static void fwnet_receive_broadcast(struct fw_iso_context *context,
u32 cycle, size_t header_length, void *header, void *data)
{
@@ -731,9 +758,6 @@ static void fwnet_receive_broadcast(stru
__be32 *buf_ptr;
int retval;
u32 length;
- u16 source_node_id;
- u32 specifier_id;
- u32 ver;
unsigned long offset;
unsigned long flags;
@@ -750,22 +774,17 @@ static void fwnet_receive_broadcast(stru
spin_unlock_irqrestore(>lock, flags);
- specifier_id =(be32_to_cpu(buf_ptr[0]) & 0x) << 8
- | (be32_to_cpu(buf_ptr[1]) & 0xff00) >> 24;
- ver = be32_to_cpu(buf_ptr[1]) & 0xff;
- source_node_id = be32_to_cpu(buf_ptr[0]) >> 16;
-
- if (specifier_id == IANA_SPECIFIER_ID &&
- (ver == RFC2734_SW_VERSION
+ if (length > IEEE1394_GASP_HDR_SIZE &&
+ gasp_specifier_id(buf_ptr) == IANA_SPECIFIER_ID &&
+ (gasp_version(buf_ptr) == RFC2734_SW_VERSION
#if IS_ENABLED(CONFIG_IPV6)
-|| ver == RFC3146_SW_VERSION
+|| gasp_version(buf_ptr) == RFC3146_SW_VERSION
#endif
- )) {
- buf_ptr += 2;
- length -= IEEE1394_GASP_HDR_SIZE;
- fwnet_incoming_packet(dev, buf_ptr, length, source_node_id,
+ ))
+ fwnet_incoming_packet(dev, buf_ptr + 2,
+ length - IEEE1394_GASP_HDR_SIZE,
+ gasp_source_id(buf_ptr),
context->card->generation, true);
- }
packet.payload_length = dev->rcv_buffer_size;
packet.interrupt = 1;
--
Stefan Richter
-==- =-== ---=-
http://arcgraph.de/sr/
[PATCH 0/3] firewire: net: IP-over-1394 link fragmentation fixes
The following patches 1/3 firewire: net: guard against rx buffer overflows 2/3 firewire: net: fix fragmented datagram_size off-by-one 3/3 firewire: net: max MTU off by one fix a few long-standing bugs of the IP-over-1394 driver firewire-net related to reception and transmission of fragmented datagrams: - RX: Missing validation of fragment offset and size makes the driver vulnerable to buffer overflows, potentially leading to remote¹ code execution. Reported by Eyal Itkin. ¹) The vulnerability cannot be triggered by malformed IP datagrams, but by malformed IEEE 1394 packets sent from other FireWire nodes to the 1394 broadcast channel or to firewire-net's unicast FIFO, or can be sent from the local node to the unicast FIFO by sufficiently privileged userland. I.e. an attack can only originate from somewhere on the FireWire bus, not from another network that is bridged to the FireWire bus. - RX: Missing validation of unfragmented and fragmented datagrams for minimum packet size before looking at GASP header and encapsulation header. - RX and TX: The datagram_size field of fragmented datagrams was read and written incorrectly; an offset of +/-1 needs to be applied. This prevents fragmented traffic from/to nodes which run OS X, Windows XP, or Linux' older eth1394 driver. (Traffic from Win XP would eventually be retried with smaller MTU and possibly succeed slowly despite the bug.) Patch 1/3 is obviously urgent. Patch 2/3 is a bit of a bother because while it fixes fragmented RX/TX with OS X, Win XP, and eth1394, it does disrupt fragmented RX/TX with Linux nodes which run an unfixed firewire-net. Patch 3/3 will only apply in conjunction with changes that are queued up in the net-next git tree, hence this patch will wait until net-next was merged. Patches 1+2/3 are already pushed out to linux1394.git "testing" and "for-next" branches, but I still like to get review comments before I send a pull request. -- Stefan Richter -==- =-== ---=- http://arcgraph.de/sr/ pgpMMihP3nLbW.pgp Description: OpenPGP digital signature
[PATCH 0/3] firewire: net: IP-over-1394 link fragmentation fixes
The following patches 1/3 firewire: net: guard against rx buffer overflows 2/3 firewire: net: fix fragmented datagram_size off-by-one 3/3 firewire: net: max MTU off by one fix a few long-standing bugs of the IP-over-1394 driver firewire-net related to reception and transmission of fragmented datagrams: - RX: Missing validation of fragment offset and size makes the driver vulnerable to buffer overflows, potentially leading to remote¹ code execution. Reported by Eyal Itkin. ¹) The vulnerability cannot be triggered by malformed IP datagrams, but by malformed IEEE 1394 packets sent from other FireWire nodes to the 1394 broadcast channel or to firewire-net's unicast FIFO, or can be sent from the local node to the unicast FIFO by sufficiently privileged userland. I.e. an attack can only originate from somewhere on the FireWire bus, not from another network that is bridged to the FireWire bus. - RX: Missing validation of unfragmented and fragmented datagrams for minimum packet size before looking at GASP header and encapsulation header. - RX and TX: The datagram_size field of fragmented datagrams was read and written incorrectly; an offset of +/-1 needs to be applied. This prevents fragmented traffic from/to nodes which run OS X, Windows XP, or Linux' older eth1394 driver. (Traffic from Win XP would eventually be retried with smaller MTU and possibly succeed slowly despite the bug.) Patch 1/3 is obviously urgent. Patch 2/3 is a bit of a bother because while it fixes fragmented RX/TX with OS X, Win XP, and eth1394, it does disrupt fragmented RX/TX with Linux nodes which run an unfixed firewire-net. Patch 3/3 will only apply in conjunction with changes that are queued up in the net-next git tree, hence this patch will wait until net-next was merged. Patches 1+2/3 are already pushed out to linux1394.git "testing" and "for-next" branches, but I still like to get review comments before I send a pull request. -- Stefan Richter -==- =-== ---=- http://arcgraph.de/sr/ pgpMMihP3nLbW.pgp Description: OpenPGP digital signature
[PATCH net-next] firewire: net: really fix maximum possible MTU
The maximum unicast datagram size /without/ link fragmentation is
4096 - 4 = 4092 (max IEEE 1394 async payload size at >= S800 bus speed,
minus unfragmented encapssulation header). Max broadcast datagram size
without fragmentation is 8 bytes less than that (due to GASP header).
The maximum datagram size /with/ link fragmentation is 0xfff = 4095
for unicast and broadcast. This is because the RFC 2734 fragment
encapsulation header field for datagram size is only 12 bits wide.
Fixes: 5d48f00d836a('firewire: net: fix maximum possible MTU')
Signed-off-by: Stefan Richter <stef...@s5r6.in-berlin.de>
---
drivers/firewire/net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 03715e7d9d92..363fc5ec1a4e 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1465,7 +1465,7 @@ static int fwnet_probe(struct fw_unit *unit,
net->mtu = 1500U;
net->min_mtu = ETH_MIN_MTU;
- net->max_mtu = ETH_MAX_MTU;
+ net->max_mtu = 0xfff;
/* Set our hardware address while we're at it */
ha = (union fwnet_hwaddr *)net->dev_addr;
--
Stefan Richter
-==- =-=- ===-=
http://arcgraph.de/sr/
[PATCH net-next] firewire: net: really fix maximum possible MTU
The maximum unicast datagram size /without/ link fragmentation is
4096 - 4 = 4092 (max IEEE 1394 async payload size at >= S800 bus speed,
minus unfragmented encapssulation header). Max broadcast datagram size
without fragmentation is 8 bytes less than that (due to GASP header).
The maximum datagram size /with/ link fragmentation is 0xfff = 4095
for unicast and broadcast. This is because the RFC 2734 fragment
encapsulation header field for datagram size is only 12 bits wide.
Fixes: 5d48f00d836a('firewire: net: fix maximum possible MTU')
Signed-off-by: Stefan Richter
---
drivers/firewire/net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 03715e7d9d92..363fc5ec1a4e 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1465,7 +1465,7 @@ static int fwnet_probe(struct fw_unit *unit,
net->mtu = 1500U;
net->min_mtu = ETH_MIN_MTU;
- net->max_mtu = ETH_MAX_MTU;
+ net->max_mtu = 0xfff;
/* Set our hardware address while we're at it */
ha = (union fwnet_hwaddr *)net->dev_addr;
--
Stefan Richter
-==- =-=- ===-=
http://arcgraph.de/sr/
[PATCH net-next] ethernet: fix min/max MTU typos
Fixes: d894be57ca92('ethernet: use net core MTU range checking in more drivers')
CC: Jarod Wilson <ja...@redhat.com>
CC: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
Signed-off-by: Stefan Richter <stef...@s5r6.in-berlin.de>
---
drivers/net/ethernet/broadcom/sb1250-mac.c | 2 +-
drivers/net/ethernet/ibm/ibmveth.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/sb1250-mac.c
b/drivers/net/ethernet/broadcom/sb1250-mac.c
index cb312e4c89f4..435a2e4739d1 100644
--- a/drivers/net/ethernet/broadcom/sb1250-mac.c
+++ b/drivers/net/ethernet/broadcom/sb1250-mac.c
@@ -2219,7 +2219,7 @@ static int sbmac_init(struct platform_device *pldev, long
long base)
dev->netdev_ops = _netdev_ops;
dev->watchdog_timeo = TX_TIMEOUT;
- dev->max_mtu = 0;
+ dev->min_mtu = 0;
dev->max_mtu = ENET_PACKET_SIZE;
netif_napi_add(dev, >napi, sbmac_poll, 16);
diff --git a/drivers/net/ethernet/ibm/ibmveth.c
b/drivers/net/ethernet/ibm/ibmveth.c
index 29c05d0d79a9..4a81c892fc31 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1549,7 +1549,7 @@ static int ibmveth_probe(struct vio_dev *dev, const
struct vio_device_id *id)
}
netdev->min_mtu = IBMVETH_MIN_MTU;
- netdev->min_mtu = ETH_MAX_MTU;
+ netdev->max_mtu = ETH_MAX_MTU;
memcpy(netdev->dev_addr, mac_addr_p, ETH_ALEN);
--
Stefan Richter
-==- =-=- ==---
http://arcgraph.de/sr/
[PATCH net-next] ethernet: fix min/max MTU typos
Fixes: d894be57ca92('ethernet: use net core MTU range checking in more drivers')
CC: Jarod Wilson
CC: Thomas Falcon
Signed-off-by: Stefan Richter
---
drivers/net/ethernet/broadcom/sb1250-mac.c | 2 +-
drivers/net/ethernet/ibm/ibmveth.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/sb1250-mac.c
b/drivers/net/ethernet/broadcom/sb1250-mac.c
index cb312e4c89f4..435a2e4739d1 100644
--- a/drivers/net/ethernet/broadcom/sb1250-mac.c
+++ b/drivers/net/ethernet/broadcom/sb1250-mac.c
@@ -2219,7 +2219,7 @@ static int sbmac_init(struct platform_device *pldev, long
long base)
dev->netdev_ops = _netdev_ops;
dev->watchdog_timeo = TX_TIMEOUT;
- dev->max_mtu = 0;
+ dev->min_mtu = 0;
dev->max_mtu = ENET_PACKET_SIZE;
netif_napi_add(dev, >napi, sbmac_poll, 16);
diff --git a/drivers/net/ethernet/ibm/ibmveth.c
b/drivers/net/ethernet/ibm/ibmveth.c
index 29c05d0d79a9..4a81c892fc31 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1549,7 +1549,7 @@ static int ibmveth_probe(struct vio_dev *dev, const
struct vio_device_id *id)
}
netdev->min_mtu = IBMVETH_MIN_MTU;
- netdev->min_mtu = ETH_MAX_MTU;
+ netdev->max_mtu = ETH_MAX_MTU;
memcpy(netdev->dev_addr, mac_addr_p, ETH_ALEN);
--
Stefan Richter
-==- =-=- ==---
http://arcgraph.de/sr/
[PATCH net-next 2/2 v2] firewire: net: set initial MTU = 1500 unconditionally, fix IPv6 on some CardBus cards
firewire-net, like the older eth1394 driver, reduced the initial MTU to less than 1500 octets if the local link layer controller's asynchronous packet reception limit was lower. This is bogus, since this reception limit does not have anything to do with the transmission limit. Neither did this reduction affect the TX path positively, nor could it prevent link fragmentation at the RX path. Many FireWire CardBus cards have a max_rec of 9, causing an initial MTU of 1024 - 16 = 1008. RFC 2734 and RFC 3146 allow a minimum max_rec = 8, which would result in an initial MTU of 512 - 16 = 496. On such cards, IPv6 could only be employed if the MTU was manually increased to 1280 or more, i.e. IPv6 would not work without intervention from userland. We now always initialize the MTU to 1500, which is the default according to RFC 2734 and RFC 3146. On a VIA VT6316 based CardBus card which was affected by this, changing the MTU from 1008 to 1500 also increases TX bandwidth by 6 %. RX remains unaffected. CC: net...@vger.kernel.org CC: linux1394-de...@lists.sourceforge.net CC: Jarod Wilson <ja...@redhat.com> Signed-off-by: Stefan Richter <stef...@s5r6.in-berlin.de> --- v2: use ETH_DATA_LEN, add comment drivers/firewire/net.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c index 99379542b263..29dbcba38f59 100644 --- a/drivers/firewire/net.c +++ b/drivers/firewire/net.c @@ -1463,13 +1463,8 @@ static int fwnet_probe(struct fw_unit *unit, goto out; dev->local_fifo = dev->handler.offset; - /* -* Use the RFC 2734 default 1500 octets or the maximum payload -* as initial MTU -*/ - net->mtu = min(1500U, - (1U << (card->max_receive + 1)) - - RFC2374_FRAG_HDR_SIZE - IEEE1394_GASP_HDR_SIZE); + /* MTU range: 68 - 65535, RFC 2734 default: 1500 */ + net->mtu = ETH_DATA_LEN; net->min_mtu = ETH_MIN_MTU; net->max_mtu = ETH_MAX_MTU; -- 2.7.3 -- Stefan Richter -==- =-=- ==--- http://arcgraph.de/sr/
[PATCH net-next 2/2 v2] firewire: net: set initial MTU = 1500 unconditionally, fix IPv6 on some CardBus cards
firewire-net, like the older eth1394 driver, reduced the initial MTU to less than 1500 octets if the local link layer controller's asynchronous packet reception limit was lower. This is bogus, since this reception limit does not have anything to do with the transmission limit. Neither did this reduction affect the TX path positively, nor could it prevent link fragmentation at the RX path. Many FireWire CardBus cards have a max_rec of 9, causing an initial MTU of 1024 - 16 = 1008. RFC 2734 and RFC 3146 allow a minimum max_rec = 8, which would result in an initial MTU of 512 - 16 = 496. On such cards, IPv6 could only be employed if the MTU was manually increased to 1280 or more, i.e. IPv6 would not work without intervention from userland. We now always initialize the MTU to 1500, which is the default according to RFC 2734 and RFC 3146. On a VIA VT6316 based CardBus card which was affected by this, changing the MTU from 1008 to 1500 also increases TX bandwidth by 6 %. RX remains unaffected. CC: net...@vger.kernel.org CC: linux1394-de...@lists.sourceforge.net CC: Jarod Wilson Signed-off-by: Stefan Richter --- v2: use ETH_DATA_LEN, add comment drivers/firewire/net.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c index 99379542b263..29dbcba38f59 100644 --- a/drivers/firewire/net.c +++ b/drivers/firewire/net.c @@ -1463,13 +1463,8 @@ static int fwnet_probe(struct fw_unit *unit, goto out; dev->local_fifo = dev->handler.offset; - /* -* Use the RFC 2734 default 1500 octets or the maximum payload -* as initial MTU -*/ - net->mtu = min(1500U, - (1U << (card->max_receive + 1)) - - RFC2374_FRAG_HDR_SIZE - IEEE1394_GASP_HDR_SIZE); + /* MTU range: 68 - 65535, RFC 2734 default: 1500 */ + net->mtu = ETH_DATA_LEN; net->min_mtu = ETH_MIN_MTU; net->max_mtu = ETH_MAX_MTU; -- 2.7.3 -- Stefan Richter -==- =-=- ==--- http://arcgraph.de/sr/
[PATCH net-next 2/2] firewire: net: set initial MTU = 1500 unconditionally, fix IPv6 on some CardBus cards
firewire-net, like the older eth1394 driver, reduced the initial MTU to
less than 1500 octets if the local link layer controller's asynchronous
packet reception limit was lower.
This is bogus, since this reception limit does not have anything to do
with the transmission limit. Neither did this reduction affect the TX
path positively, nor could it prevent link fragmentation at the RX path.
Many FireWire CardBus cards have a max_rec of 9, causing an initial MTU
of 1024 - 16 = 1008. RFC 2734 and RFC 3146 allow a minimum max_rec = 8,
which would result in an initial MTU of 512 - 16 = 496. On such cards,
IPv6 could only be employed if the MTU was manually increased to 1280 or
more, i.e. IPv6 would not work without intervention from userland.
We now always initialize the MTU to 1500, which is the default according
to RFC 2734 and RFC 3146.
On a VIA VT6316 based CardBus card which was affected by this, changing
the MTU from 1008 to 1500 also increases TX bandwidth by 6 %.
RX remains unaffected.
CC: net...@vger.kernel.org
CC: linux1394-de...@lists.sourceforge.net
CC: Jarod Wilson <ja...@redhat.com>
Signed-off-by: Stefan Richter <stef...@s5r6.in-berlin.de>
---
drivers/firewire/net.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 99379542b263..03715e7d9d92 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1463,13 +1463,7 @@ static int fwnet_probe(struct fw_unit *unit,
goto out;
dev->local_fifo = dev->handler.offset;
- /*
-* Use the RFC 2734 default 1500 octets or the maximum payload
-* as initial MTU
-*/
- net->mtu = min(1500U,
- (1U << (card->max_receive + 1))
- - RFC2374_FRAG_HDR_SIZE - IEEE1394_GASP_HDR_SIZE);
+ net->mtu = 1500U;
net->min_mtu = ETH_MIN_MTU;
net->max_mtu = ETH_MAX_MTU;
--
Stefan Richter
-==- =-=- =-===
http://arcgraph.de/sr/
[PATCH net-next 2/2] firewire: net: set initial MTU = 1500 unconditionally, fix IPv6 on some CardBus cards
firewire-net, like the older eth1394 driver, reduced the initial MTU to less than 1500 octets if the local link layer controller's asynchronous packet reception limit was lower. This is bogus, since this reception limit does not have anything to do with the transmission limit. Neither did this reduction affect the TX path positively, nor could it prevent link fragmentation at the RX path. Many FireWire CardBus cards have a max_rec of 9, causing an initial MTU of 1024 - 16 = 1008. RFC 2734 and RFC 3146 allow a minimum max_rec = 8, which would result in an initial MTU of 512 - 16 = 496. On such cards, IPv6 could only be employed if the MTU was manually increased to 1280 or more, i.e. IPv6 would not work without intervention from userland. We now always initialize the MTU to 1500, which is the default according to RFC 2734 and RFC 3146. On a VIA VT6316 based CardBus card which was affected by this, changing the MTU from 1008 to 1500 also increases TX bandwidth by 6 %. RX remains unaffected. CC: net...@vger.kernel.org CC: linux1394-de...@lists.sourceforge.net CC: Jarod Wilson Signed-off-by: Stefan Richter --- drivers/firewire/net.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c index 99379542b263..03715e7d9d92 100644 --- a/drivers/firewire/net.c +++ b/drivers/firewire/net.c @@ -1463,13 +1463,7 @@ static int fwnet_probe(struct fw_unit *unit, goto out; dev->local_fifo = dev->handler.offset; - /* -* Use the RFC 2734 default 1500 octets or the maximum payload -* as initial MTU -*/ - net->mtu = min(1500U, - (1U << (card->max_receive + 1)) - - RFC2374_FRAG_HDR_SIZE - IEEE1394_GASP_HDR_SIZE); + net->mtu = 1500U; net->min_mtu = ETH_MIN_MTU; net->max_mtu = ETH_MAX_MTU; -- Stefan Richter -==- =-=- =-=== http://arcgraph.de/sr/
[PATCH net-next 1/2] firewire: net: fix maximum possible MTU
Commit b3e3893e1253 ("net: use core MTU range checking in misc drivers")
mistakenly introduced an upper limit for firewire-net's MTU based on the
local link layer controller's reception capability. Revert this. Neither
RFC 2734 nor our implementation impose any particular upper limit.
Actually, to be on the safe side and to make the code explicit, set
ETH_MAX_MTU = 65535 as upper limit now.
(I replaced sizeof(struct rfc2734_header) by the equivalent
RFC2374_FRAG_HDR_SIZE in order to avoid distracting long/int conversions.)
Fixes: b3e3893e1253('net: use core MTU range checking in misc drivers')
CC: net...@vger.kernel.org
CC: linux1394-de...@lists.sourceforge.net
CC: Jarod Wilson <ja...@redhat.com>
Signed-off-by: Stefan Richter <stef...@s5r6.in-berlin.de>
---
drivers/firewire/net.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 8430222151fc..99379542b263 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1467,10 +1467,11 @@ static int fwnet_probe(struct fw_unit *unit,
* Use the RFC 2734 default 1500 octets or the maximum payload
* as initial MTU
*/
- net->max_mtu = (1 << (card->max_receive + 1))
- - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
- net->mtu = min(1500U, net->max_mtu);
+ net->mtu = min(1500U,
+ (1U << (card->max_receive + 1))
+ - RFC2374_FRAG_HDR_SIZE - IEEE1394_GASP_HDR_SIZE);
net->min_mtu = ETH_MIN_MTU;
+ net->max_mtu = ETH_MAX_MTU;
/* Set our hardware address while we're at it */
ha = (union fwnet_hwaddr *)net->dev_addr;
--
Stefan Richter
-==- =-=- =-===
http://arcgraph.de/sr/
[PATCH net-next 1/2] firewire: net: fix maximum possible MTU
Commit b3e3893e1253 ("net: use core MTU range checking in misc drivers")
mistakenly introduced an upper limit for firewire-net's MTU based on the
local link layer controller's reception capability. Revert this. Neither
RFC 2734 nor our implementation impose any particular upper limit.
Actually, to be on the safe side and to make the code explicit, set
ETH_MAX_MTU = 65535 as upper limit now.
(I replaced sizeof(struct rfc2734_header) by the equivalent
RFC2374_FRAG_HDR_SIZE in order to avoid distracting long/int conversions.)
Fixes: b3e3893e1253('net: use core MTU range checking in misc drivers')
CC: net...@vger.kernel.org
CC: linux1394-de...@lists.sourceforge.net
CC: Jarod Wilson
Signed-off-by: Stefan Richter
---
drivers/firewire/net.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 8430222151fc..99379542b263 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1467,10 +1467,11 @@ static int fwnet_probe(struct fw_unit *unit,
* Use the RFC 2734 default 1500 octets or the maximum payload
* as initial MTU
*/
- net->max_mtu = (1 << (card->max_receive + 1))
- - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
- net->mtu = min(1500U, net->max_mtu);
+ net->mtu = min(1500U,
+ (1U << (card->max_receive + 1))
+ - RFC2374_FRAG_HDR_SIZE - IEEE1394_GASP_HDR_SIZE);
net->min_mtu = ETH_MIN_MTU;
+ net->max_mtu = ETH_MAX_MTU;
/* Set our hardware address while we're at it */
ha = (union fwnet_hwaddr *)net->dev_addr;
--
Stefan Richter
-==- =-=- =-===
http://arcgraph.de/sr/
Re: [PATCH net-next v2 7/9] net: use core MTU range checking in misc drivers
Adding Cc: linux1394-devel, dropping several Ccs, no additional comment.
On Oct 22 Stefan Richter wrote:
> On Oct 20 Jarod Wilson wrote:
> > firewire-net:
> > - set min/max_mtu
> > - remove fwnet_change_mtu
> [...]
> > --- a/drivers/firewire/net.c
> > +++ b/drivers/firewire/net.c
> > @@ -1349,15 +1349,6 @@ static netdev_tx_t fwnet_tx(struct sk_buff
> > *skb, struct net_device *net) return NETDEV_TX_OK;
> > }
> >
> > -static int fwnet_change_mtu(struct net_device *net, int new_mtu)
> > -{
> > - if (new_mtu < 68)
> > - return -EINVAL;
> > -
> > - net->mtu = new_mtu;
> > - return 0;
> > -}
> > -
> > static const struct ethtool_ops fwnet_ethtool_ops = {
> > .get_link = ethtool_op_get_link,
> > };
> > @@ -1366,7 +1357,6 @@ static const struct net_device_ops
> > fwnet_netdev_ops = { .ndo_open = fwnet_open,
> > .ndo_stop = fwnet_stop,
> > .ndo_start_xmit = fwnet_tx,
> > - .ndo_change_mtu = fwnet_change_mtu,
> > };
> >
> > static void fwnet_init_dev(struct net_device *net)
> > @@ -1435,7 +1425,6 @@ static int fwnet_probe(struct fw_unit *unit,
> > struct net_device *net;
> > bool allocated_netdev = false;
> > struct fwnet_device *dev;
> > - unsigned max_mtu;
> > int ret;
> > union fwnet_hwaddr *ha;
> >
> > @@ -1478,9 +1467,10 @@ static int fwnet_probe(struct fw_unit *unit,
> > * Use the RFC 2734 default 1500 octets or the maximum payload
> > * as initial MTU
> > */
> > - max_mtu = (1 << (card->max_receive + 1))
> > - - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> > - net->mtu = min(1500U, max_mtu);
> > + net->max_mtu = (1 << (card->max_receive + 1))
> > + - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> > + net->mtu = min(1500U, net->max_mtu);
> > + net->min_mtu = ETH_MIN_MTU;
> >
> > /* Set our hardware address while we're at it */
> > ha = (union fwnet_hwaddr *)net->dev_addr;
>
> Please preserve the current behavior, i.e. do not enforce any particular
> upper bound. (Especially none based on the local link layer controller's
> max_receive parameter.)
>
> BTW, after having read RFC 2734, RFC 3146, and the code, I am convinced
> that net->mtu should be initialized to 1500, not less. But such a change
> should be done in a separate patch.
--
Stefan Richter
-==- =-=- =-==-
http://arcgraph.de/sr/
Re: [PATCH net-next v2 7/9] net: use core MTU range checking in misc drivers
Adding Cc: linux1394-devel, dropping several Ccs, no additional comment.
On Oct 22 Stefan Richter wrote:
> On Oct 20 Jarod Wilson wrote:
> > firewire-net:
> > - set min/max_mtu
> > - remove fwnet_change_mtu
> [...]
> > --- a/drivers/firewire/net.c
> > +++ b/drivers/firewire/net.c
> > @@ -1349,15 +1349,6 @@ static netdev_tx_t fwnet_tx(struct sk_buff
> > *skb, struct net_device *net) return NETDEV_TX_OK;
> > }
> >
> > -static int fwnet_change_mtu(struct net_device *net, int new_mtu)
> > -{
> > - if (new_mtu < 68)
> > - return -EINVAL;
> > -
> > - net->mtu = new_mtu;
> > - return 0;
> > -}
> > -
> > static const struct ethtool_ops fwnet_ethtool_ops = {
> > .get_link = ethtool_op_get_link,
> > };
> > @@ -1366,7 +1357,6 @@ static const struct net_device_ops
> > fwnet_netdev_ops = { .ndo_open = fwnet_open,
> > .ndo_stop = fwnet_stop,
> > .ndo_start_xmit = fwnet_tx,
> > - .ndo_change_mtu = fwnet_change_mtu,
> > };
> >
> > static void fwnet_init_dev(struct net_device *net)
> > @@ -1435,7 +1425,6 @@ static int fwnet_probe(struct fw_unit *unit,
> > struct net_device *net;
> > bool allocated_netdev = false;
> > struct fwnet_device *dev;
> > - unsigned max_mtu;
> > int ret;
> > union fwnet_hwaddr *ha;
> >
> > @@ -1478,9 +1467,10 @@ static int fwnet_probe(struct fw_unit *unit,
> > * Use the RFC 2734 default 1500 octets or the maximum payload
> > * as initial MTU
> > */
> > - max_mtu = (1 << (card->max_receive + 1))
> > - - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> > - net->mtu = min(1500U, max_mtu);
> > + net->max_mtu = (1 << (card->max_receive + 1))
> > + - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> > + net->mtu = min(1500U, net->max_mtu);
> > + net->min_mtu = ETH_MIN_MTU;
> >
> > /* Set our hardware address while we're at it */
> > ha = (union fwnet_hwaddr *)net->dev_addr;
>
> Please preserve the current behavior, i.e. do not enforce any particular
> upper bound. (Especially none based on the local link layer controller's
> max_receive parameter.)
>
> BTW, after having read RFC 2734, RFC 3146, and the code, I am convinced
> that net->mtu should be initialized to 1500, not less. But such a change
> should be done in a separate patch.
--
Stefan Richter
-==- =-=- =-==-
http://arcgraph.de/sr/
Re: [PATCH net-next v2 7/9] net: use core MTU range checking in misc drivers
On Oct 20 Jarod Wilson wrote: > firewire-net: > - set min/max_mtu > - remove fwnet_change_mtu [...] > --- a/drivers/firewire/net.c > +++ b/drivers/firewire/net.c [...] > @@ -1478,9 +1467,10 @@ static int fwnet_probe(struct fw_unit *unit, >* Use the RFC 2734 default 1500 octets or the maximum payload >* as initial MTU >*/ > - max_mtu = (1 << (card->max_receive + 1)) > - - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE; > - net->mtu = min(1500U, max_mtu); > + net->max_mtu = (1 << (card->max_receive + 1)) > +- sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE; > + net->mtu = min(1500U, net->max_mtu); > + net->min_mtu = ETH_MIN_MTU; > > /* Set our hardware address while we're at it */ > ha = (union fwnet_hwaddr *)net->dev_addr; Please preserve the current behavior, i.e. do not enforce any particular upper bound. (Especially none based on the local link layer controller's max_receive parameter.) BTW, after having read RFC 2734, RFC 3146, and the code, I am convinced that net->mtu should be initialized to 1500, not less. But such a change should be done in a separate patch. -- Stefan Richter -==- =-=- =-==- http://arcgraph.de/sr/
Re: [PATCH net-next v2 7/9] net: use core MTU range checking in misc drivers
On Oct 20 Jarod Wilson wrote: > firewire-net: > - set min/max_mtu > - remove fwnet_change_mtu [...] > --- a/drivers/firewire/net.c > +++ b/drivers/firewire/net.c [...] > @@ -1478,9 +1467,10 @@ static int fwnet_probe(struct fw_unit *unit, >* Use the RFC 2734 default 1500 octets or the maximum payload >* as initial MTU >*/ > - max_mtu = (1 << (card->max_receive + 1)) > - - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE; > - net->mtu = min(1500U, max_mtu); > + net->max_mtu = (1 << (card->max_receive + 1)) > +- sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE; > + net->mtu = min(1500U, net->max_mtu); > + net->min_mtu = ETH_MIN_MTU; > > /* Set our hardware address while we're at it */ > ha = (union fwnet_hwaddr *)net->dev_addr; Please preserve the current behavior, i.e. do not enforce any particular upper bound. (Especially none based on the local link layer controller's max_receive parameter.) BTW, after having read RFC 2734, RFC 3146, and the code, I am convinced that net->mtu should be initialized to 1500, not less. But such a change should be done in a separate patch. -- Stefan Richter -==- =-=- =-==- http://arcgraph.de/sr/
Re: [PATCH net-next 6/6] net: use core MTU range checking in misc drivers
On Oct 22 Stefan Richter wrote: > On Oct 19 Jarod Wilson wrote: > > On Thu, Oct 20, 2016 at 12:38:46AM +0200, Stefan Richter wrote: > > > On Oct 19 Sabrina Dubroca wrote: > > > > 2016-10-18, 22:33:33 -0400, Jarod Wilson wrote: [...] > > > > > @@ -1481,6 +1471,8 @@ static int fwnet_probe(struct fw_unit *unit, > > > > > max_mtu = (1 << (card->max_receive + 1)) > > > > > - sizeof(struct rfc2734_header) - > > > > > IEEE1394_GASP_HDR_SIZE; > > > > > net->mtu = min(1500U, max_mtu); > > > > > + net->min_mtu = ETH_MIN_MTU; > > > > > + net->max_mtu = net->mtu; > > > > > > > > But that will now prevent increasing the MTU above the initial value? > > > > > > Indeed, therefore NAK. > > > > However, there's an explicit calculation for 'max_mtu' right there that I > > glazed right over. It would seem perhaps *that* should be used for > > net->max_mtu here, no? > > No. This 'max_mtu' here is not the absolute maximum. It is only an > initial MTU which has the property that link fragmentation is not > going to happen (if all other peers will at least as capable as this > node). Besides, card->max_receive is about what the card can receive (at the IEEE 1394 link layer), not about what the card can send. -- Stefan Richter -==- =-=- =-==- http://arcgraph.de/sr/
Re: [PATCH net-next 6/6] net: use core MTU range checking in misc drivers
On Oct 22 Stefan Richter wrote: > On Oct 19 Jarod Wilson wrote: > > On Thu, Oct 20, 2016 at 12:38:46AM +0200, Stefan Richter wrote: > > > On Oct 19 Sabrina Dubroca wrote: > > > > 2016-10-18, 22:33:33 -0400, Jarod Wilson wrote: [...] > > > > > @@ -1481,6 +1471,8 @@ static int fwnet_probe(struct fw_unit *unit, > > > > > max_mtu = (1 << (card->max_receive + 1)) > > > > > - sizeof(struct rfc2734_header) - > > > > > IEEE1394_GASP_HDR_SIZE; > > > > > net->mtu = min(1500U, max_mtu); > > > > > + net->min_mtu = ETH_MIN_MTU; > > > > > + net->max_mtu = net->mtu; > > > > > > > > But that will now prevent increasing the MTU above the initial value? > > > > > > Indeed, therefore NAK. > > > > However, there's an explicit calculation for 'max_mtu' right there that I > > glazed right over. It would seem perhaps *that* should be used for > > net->max_mtu here, no? > > No. This 'max_mtu' here is not the absolute maximum. It is only an > initial MTU which has the property that link fragmentation is not > going to happen (if all other peers will at least as capable as this > node). Besides, card->max_receive is about what the card can receive (at the IEEE 1394 link layer), not about what the card can send. -- Stefan Richter -==- =-=- =-==- http://arcgraph.de/sr/
Re: [PATCH net-next 09/15] ethernet/dlink: use core min/max MTU checking
On Oct 18 Jarod Wilson wrote:
> On Tue, Oct 18, 2016 at 04:45:51PM +0300, Denis Kirjanov wrote:
> > On 10/17/16, Jarod Wilson <ja...@redhat.com> wrote:
[...]
> > > --- a/drivers/net/ethernet/dlink/sundance.c
> > > +++ b/drivers/net/ethernet/dlink/sundance.c
> > > @@ -580,6 +580,10 @@ static int sundance_probe1(struct pci_dev *pdev,
> > > dev->ethtool_ops = _ops;
> > > dev->watchdog_timeo = TX_TIMEOUT;
> > >
> > > + /* MTU range: 68 - 8191 */
> > > + dev->min_mtu = ETH_MIN_MTU;
> > > + dev->max_mtu = 8191;
> > > +
> > ICPlus datasheet defines the max frame size like 0x1514 or 0x4491
> > based on the RcvLargeFrames bit in the MACCtrl0 register
>
> I do anticipate this patchset might bring to light some inaccuracies in
> min/max mtu values currently implemented in various drivers, but for the
> moment, I'm going for 100% identical behavior with what's currently in the
> driver, and if you'll look down below...
>
> > > pci_set_drvdata(pdev, dev);
> > >
> > > i = register_netdev(dev);
> > > @@ -713,8 +717,6 @@ static int sundance_probe1(struct pci_dev *pdev,
> > >
> > > static int change_mtu(struct net_device *dev, int new_mtu)
> > > {
> > > - if ((new_mtu < 68) || (new_mtu > 8191)) /* Set by RxDMAFrameLen */
>
>
> The 8191 was simply transplanted from right here. I think altering the
> value should be the subject of a separate patch.
You transplanted the value but dropped the comment. Though Denis' reply
sounds like the comment should have been /* FIXME, set by RxDMAFrameLen */.
--
Stefan Richter
-==- =-=- =-==-
http://arcgraph.de/sr/
Re: [PATCH net-next 09/15] ethernet/dlink: use core min/max MTU checking
On Oct 18 Jarod Wilson wrote:
> On Tue, Oct 18, 2016 at 04:45:51PM +0300, Denis Kirjanov wrote:
> > On 10/17/16, Jarod Wilson wrote:
[...]
> > > --- a/drivers/net/ethernet/dlink/sundance.c
> > > +++ b/drivers/net/ethernet/dlink/sundance.c
> > > @@ -580,6 +580,10 @@ static int sundance_probe1(struct pci_dev *pdev,
> > > dev->ethtool_ops = _ops;
> > > dev->watchdog_timeo = TX_TIMEOUT;
> > >
> > > + /* MTU range: 68 - 8191 */
> > > + dev->min_mtu = ETH_MIN_MTU;
> > > + dev->max_mtu = 8191;
> > > +
> > ICPlus datasheet defines the max frame size like 0x1514 or 0x4491
> > based on the RcvLargeFrames bit in the MACCtrl0 register
>
> I do anticipate this patchset might bring to light some inaccuracies in
> min/max mtu values currently implemented in various drivers, but for the
> moment, I'm going for 100% identical behavior with what's currently in the
> driver, and if you'll look down below...
>
> > > pci_set_drvdata(pdev, dev);
> > >
> > > i = register_netdev(dev);
> > > @@ -713,8 +717,6 @@ static int sundance_probe1(struct pci_dev *pdev,
> > >
> > > static int change_mtu(struct net_device *dev, int new_mtu)
> > > {
> > > - if ((new_mtu < 68) || (new_mtu > 8191)) /* Set by RxDMAFrameLen */
>
>
> The 8191 was simply transplanted from right here. I think altering the
> value should be the subject of a separate patch.
You transplanted the value but dropped the comment. Though Denis' reply
sounds like the comment should have been /* FIXME, set by RxDMAFrameLen */.
--
Stefan Richter
-==- =-=- =-==-
http://arcgraph.de/sr/
Re: [PATCH net-next 6/6] net: use core MTU range checking in misc drivers
On Oct 19 Jarod Wilson wrote:
> On Thu, Oct 20, 2016 at 12:38:46AM +0200, Stefan Richter wrote:
> > On Oct 19 Sabrina Dubroca wrote:
> > > 2016-10-18, 22:33:33 -0400, Jarod Wilson wrote:
> > > [...]
> > > > diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
> > > > index 309311b..b5f125c 100644
> > > > --- a/drivers/firewire/net.c
> > > > +++ b/drivers/firewire/net.c
> > > > @@ -1349,15 +1349,6 @@ static netdev_tx_t fwnet_tx(struct sk_buff *skb,
> > > > struct net_device *net)
> > > > return NETDEV_TX_OK;
> > > > }
> > > >
> > > > -static int fwnet_change_mtu(struct net_device *net, int new_mtu)
> > > > -{
> > > > - if (new_mtu < 68)
> > > > - return -EINVAL;
> > > > -
> > > > - net->mtu = new_mtu;
> > > > - return 0;
> > > > -}
> > > > -
> > >
> > > This doesn't do any upper bound checking.
> >
> > I need to check more closely, but I think the RFC 2734 encapsulation spec
> > and our implementation do not impose a particular upper limit. Though I
> > guess it's bad to let userland set arbitrarily large values here.
>
> In which case, that would suggest using IP_MAX_MTU (65535) here.
Probably. I (or somebody) need to check the spec and the code once more.
[...]
> > > > @@ -1481,6 +1471,8 @@ static int fwnet_probe(struct fw_unit *unit,
> > > > max_mtu = (1 << (card->max_receive + 1))
> > > > - sizeof(struct rfc2734_header) -
> > > > IEEE1394_GASP_HDR_SIZE;
> > > > net->mtu = min(1500U, max_mtu);
> > > > + net->min_mtu = ETH_MIN_MTU;
> > > > + net->max_mtu = net->mtu;
> > >
> > > But that will now prevent increasing the MTU above the initial value?
> >
> > Indeed, therefore NAK.
>
> However, there's an explicit calculation for 'max_mtu' right there that I
> glazed right over. It would seem perhaps *that* should be used for
> net->max_mtu here, no?
No. This 'max_mtu' here is not the absolute maximum. It is only an
initial MTU which has the property that link fragmentation is not
going to happen (if all other peers will at least as capable as this
node).
--
Stefan Richter
-==- =-=- =-==-
http://arcgraph.de/sr/
Re: [PATCH net-next 6/6] net: use core MTU range checking in misc drivers
On Oct 19 Jarod Wilson wrote:
> On Thu, Oct 20, 2016 at 12:38:46AM +0200, Stefan Richter wrote:
> > On Oct 19 Sabrina Dubroca wrote:
> > > 2016-10-18, 22:33:33 -0400, Jarod Wilson wrote:
> > > [...]
> > > > diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
> > > > index 309311b..b5f125c 100644
> > > > --- a/drivers/firewire/net.c
> > > > +++ b/drivers/firewire/net.c
> > > > @@ -1349,15 +1349,6 @@ static netdev_tx_t fwnet_tx(struct sk_buff *skb,
> > > > struct net_device *net)
> > > > return NETDEV_TX_OK;
> > > > }
> > > >
> > > > -static int fwnet_change_mtu(struct net_device *net, int new_mtu)
> > > > -{
> > > > - if (new_mtu < 68)
> > > > - return -EINVAL;
> > > > -
> > > > - net->mtu = new_mtu;
> > > > - return 0;
> > > > -}
> > > > -
> > >
> > > This doesn't do any upper bound checking.
> >
> > I need to check more closely, but I think the RFC 2734 encapsulation spec
> > and our implementation do not impose a particular upper limit. Though I
> > guess it's bad to let userland set arbitrarily large values here.
>
> In which case, that would suggest using IP_MAX_MTU (65535) here.
Probably. I (or somebody) need to check the spec and the code once more.
[...]
> > > > @@ -1481,6 +1471,8 @@ static int fwnet_probe(struct fw_unit *unit,
> > > > max_mtu = (1 << (card->max_receive + 1))
> > > > - sizeof(struct rfc2734_header) -
> > > > IEEE1394_GASP_HDR_SIZE;
> > > > net->mtu = min(1500U, max_mtu);
> > > > + net->min_mtu = ETH_MIN_MTU;
> > > > + net->max_mtu = net->mtu;
> > >
> > > But that will now prevent increasing the MTU above the initial value?
> >
> > Indeed, therefore NAK.
>
> However, there's an explicit calculation for 'max_mtu' right there that I
> glazed right over. It would seem perhaps *that* should be used for
> net->max_mtu here, no?
No. This 'max_mtu' here is not the absolute maximum. It is only an
initial MTU which has the property that link fragmentation is not
going to happen (if all other peers will at least as capable as this
node).
--
Stefan Richter
-==- =-=- =-==-
http://arcgraph.de/sr/
Re: [PATCH net-next 6/6] net: use core MTU range checking in misc drivers
On Oct 19 Sabrina Dubroca wrote:
> 2016-10-18, 22:33:33 -0400, Jarod Wilson wrote:
> [...]
> > diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
> > index 309311b..b5f125c 100644
> > --- a/drivers/firewire/net.c
> > +++ b/drivers/firewire/net.c
> > @@ -1349,15 +1349,6 @@ static netdev_tx_t fwnet_tx(struct sk_buff *skb,
> > struct net_device *net)
> > return NETDEV_TX_OK;
> > }
> >
> > -static int fwnet_change_mtu(struct net_device *net, int new_mtu)
> > -{
> > - if (new_mtu < 68)
> > - return -EINVAL;
> > -
> > - net->mtu = new_mtu;
> > - return 0;
> > -}
> > -
>
> This doesn't do any upper bound checking.
I need to check more closely, but I think the RFC 2734 encapsulation spec
and our implementation do not impose a particular upper limit. Though I
guess it's bad to let userland set arbitrarily large values here.
> > static const struct ethtool_ops fwnet_ethtool_ops = {
> > .get_link = ethtool_op_get_link,
> > };
> > @@ -1366,7 +1357,6 @@ static const struct net_device_ops fwnet_netdev_ops =
> > {
> > .ndo_open = fwnet_open,
> > .ndo_stop = fwnet_stop,
> > .ndo_start_xmit = fwnet_tx,
> > - .ndo_change_mtu = fwnet_change_mtu,
> > };
> >
> > static void fwnet_init_dev(struct net_device *net)
> > @@ -1481,6 +1471,8 @@ static int fwnet_probe(struct fw_unit *unit,
> > max_mtu = (1 << (card->max_receive + 1))
> > - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> > net->mtu = min(1500U, max_mtu);
> > + net->min_mtu = ETH_MIN_MTU;
> > + net->max_mtu = net->mtu;
>
> But that will now prevent increasing the MTU above the initial value?
Indeed, therefore NAK.
PS:
If the IP packet plus encapsulation header fits into IEEE 1394 packet
payload, it is transported without link fragmentation. If it does not
fit, link fragmentation occurs (which reduces bandwidth a bit and
consumes additional buffering resources at the transmitter and the
receiver).
Broadcast and multicast packets are transmitted via IEEE 1394 asynchronous
stream packets at a low bus speed (because our code does not attempt to
find the maximum speed and size that is supported by all potential
listeners). This limits the payload to 512 bytes.
Unicast packets are transmitted via IEEE 1394 asynchronous write request
packets at optimum speed. In most cases, this means that 2048 bytes
payload is possible, in some cases 4096 bytes. Many CardBus FireWire
cards support only 1024 bytes payload of these packets though.
Furthermore, some low-speed long-haul cablings may cap the bus speed and
thereby the payload size to 1024 or 512 bytes, but this is uncommon in
practice.
--
Stefan Richter
-==- =-=- =-=--
http://arcgraph.de/sr/
Re: [PATCH net-next 6/6] net: use core MTU range checking in misc drivers
On Oct 19 Sabrina Dubroca wrote:
> 2016-10-18, 22:33:33 -0400, Jarod Wilson wrote:
> [...]
> > diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
> > index 309311b..b5f125c 100644
> > --- a/drivers/firewire/net.c
> > +++ b/drivers/firewire/net.c
> > @@ -1349,15 +1349,6 @@ static netdev_tx_t fwnet_tx(struct sk_buff *skb,
> > struct net_device *net)
> > return NETDEV_TX_OK;
> > }
> >
> > -static int fwnet_change_mtu(struct net_device *net, int new_mtu)
> > -{
> > - if (new_mtu < 68)
> > - return -EINVAL;
> > -
> > - net->mtu = new_mtu;
> > - return 0;
> > -}
> > -
>
> This doesn't do any upper bound checking.
I need to check more closely, but I think the RFC 2734 encapsulation spec
and our implementation do not impose a particular upper limit. Though I
guess it's bad to let userland set arbitrarily large values here.
> > static const struct ethtool_ops fwnet_ethtool_ops = {
> > .get_link = ethtool_op_get_link,
> > };
> > @@ -1366,7 +1357,6 @@ static const struct net_device_ops fwnet_netdev_ops =
> > {
> > .ndo_open = fwnet_open,
> > .ndo_stop = fwnet_stop,
> > .ndo_start_xmit = fwnet_tx,
> > - .ndo_change_mtu = fwnet_change_mtu,
> > };
> >
> > static void fwnet_init_dev(struct net_device *net)
> > @@ -1481,6 +1471,8 @@ static int fwnet_probe(struct fw_unit *unit,
> > max_mtu = (1 << (card->max_receive + 1))
> > - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> > net->mtu = min(1500U, max_mtu);
> > + net->min_mtu = ETH_MIN_MTU;
> > + net->max_mtu = net->mtu;
>
> But that will now prevent increasing the MTU above the initial value?
Indeed, therefore NAK.
PS:
If the IP packet plus encapsulation header fits into IEEE 1394 packet
payload, it is transported without link fragmentation. If it does not
fit, link fragmentation occurs (which reduces bandwidth a bit and
consumes additional buffering resources at the transmitter and the
receiver).
Broadcast and multicast packets are transmitted via IEEE 1394 asynchronous
stream packets at a low bus speed (because our code does not attempt to
find the maximum speed and size that is supported by all potential
listeners). This limits the payload to 512 bytes.
Unicast packets are transmitted via IEEE 1394 asynchronous write request
packets at optimum speed. In most cases, this means that 2048 bytes
payload is possible, in some cases 4096 bytes. Many CardBus FireWire
cards support only 1024 bytes payload of these packets though.
Furthermore, some low-speed long-haul cablings may cap the bus speed and
thereby the payload size to 1024 or 512 bytes, but this is uncommon in
practice.
--
Stefan Richter
-==- =-=- =-=--
http://arcgraph.de/sr/
[git pull] FireWire (IEEE 1394) fixlet
Linus, please pull from the tag "firewire-update" at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git firewire-update to receive the following IEEE 1394 subsystem patch: - catch an initialization error in the packet sniffer nosy Alexey Khoroshilov (1): firewire: nosy: do not ignore errors in ioremap_nocache() drivers/firewire/nosy.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) Thanks. -- Stefan Richter -==- =-=- =---= http://arcgraph.de/sr/
[git pull] FireWire (IEEE 1394) fixlet
Linus, please pull from the tag "firewire-update" at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git firewire-update to receive the following IEEE 1394 subsystem patch: - catch an initialization error in the packet sniffer nosy Alexey Khoroshilov (1): firewire: nosy: do not ignore errors in ioremap_nocache() drivers/firewire/nosy.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) Thanks. -- Stefan Richter -==- =-=- =---= http://arcgraph.de/sr/
Re: [PATCH 3.4 027/125] firewire: ohci: fix JMicron JMB38x IT context discovery
On Oct 12 l...@kernel.org wrote:
[...]
> Signed-off-by: Stefan Richter <stef...@s5r6.in-berlin.de>
> [lizf: Backported to 3.4: use dev_notice() instead of ohci_notice()]
> Signed-off-by: Zefan Li <lize...@huawei.com>
> ---
> drivers/firewire/ohci.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
> index c1de4c3..4eedb07 100644
> --- a/drivers/firewire/ohci.c
> +++ b/drivers/firewire/ohci.c
> @@ -3620,6 +3620,11 @@ static int __devinit pci_probe(struct pci_dev *dev,
>
> reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, ~0);
> ohci->it_context_support = reg_read(ohci, OHCI1394_IsoXmitIntMaskSet);
> + /* JMicron JMB38x often shows 0 at first read, just ignore it */
> + if (!ohci->it_context_support) {
> + dev_notice(>dev, "overriding IsoXmitIntMask\n");
> + ohci->it_context_support = 0xf;
> + }
> reg_write(ohci, OHCI1394_IsoXmitIntMaskClear, ~0);
> ohci->it_context_mask = ohci->it_context_support;
> ohci->n_it = hweight32(ohci->it_context_mask);
Backport looks good to me. Thanks.
--
Stefan Richter
-==- =-=- -==--
http://arcgraph.de/sr/
Re: [PATCH 3.4 027/125] firewire: ohci: fix JMicron JMB38x IT context discovery
On Oct 12 l...@kernel.org wrote:
[...]
> Signed-off-by: Stefan Richter
> [lizf: Backported to 3.4: use dev_notice() instead of ohci_notice()]
> Signed-off-by: Zefan Li
> ---
> drivers/firewire/ohci.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
> index c1de4c3..4eedb07 100644
> --- a/drivers/firewire/ohci.c
> +++ b/drivers/firewire/ohci.c
> @@ -3620,6 +3620,11 @@ static int __devinit pci_probe(struct pci_dev *dev,
>
> reg_write(ohci, OHCI1394_IsoXmitIntMaskSet, ~0);
> ohci->it_context_support = reg_read(ohci, OHCI1394_IsoXmitIntMaskSet);
> + /* JMicron JMB38x often shows 0 at first read, just ignore it */
> + if (!ohci->it_context_support) {
> + dev_notice(>dev, "overriding IsoXmitIntMask\n");
> + ohci->it_context_support = 0xf;
> + }
> reg_write(ohci, OHCI1394_IsoXmitIntMaskClear, ~0);
> ohci->it_context_mask = ohci->it_context_support;
> ohci->n_it = hweight32(ohci->it_context_mask);
Backport looks good to me. Thanks.
--
Stefan Richter
-==- =-=- -==--
http://arcgraph.de/sr/
Re: [PATCH] firewire: nosy: do not ignore errors in ioremap_nocache()
On Sep 25 Alexey Khoroshilov wrote:
> There is no check if ioremap_nocache() returns a valid pointer.
> Potentially it can lead to null pointer dereference.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshi...@ispras.ru>
> ---
> drivers/firewire/nosy.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/firewire/nosy.c b/drivers/firewire/nosy.c
> index 631c977b0da5..f68a749f740b 100644
> --- a/drivers/firewire/nosy.c
> +++ b/drivers/firewire/nosy.c
> @@ -566,6 +566,11 @@ add_card(struct pci_dev *dev, const struct pci_device_id
> *unused)
>
> lynx->registers = ioremap_nocache(pci_resource_start(dev, 0),
> PCILYNX_MAX_REGISTER);
> + if (lynx->registers == NULL) {
> + dev_err(>dev, "Failed to map registers\n");
> + ret = -ENOMEM;
> + goto fail_deallocate2;
> + }
>
> lynx->rcv_start_pcl = pci_alloc_consistent(lynx->pci_device,
> sizeof(struct pcl), >rcv_start_pcl_bus);
> @@ -679,6 +684,8 @@ fail_deallocate:
> pci_free_consistent(lynx->pci_device, PAGE_SIZE,
> lynx->rcv_buffer, lynx->rcv_buffer_bus);
> iounmap(lynx->registers);
> +
> +fail_deallocate2:
> kfree(lynx);
>
> fail_disable:
Thanks. Committed to linux1394.git.
I folded the following cosmetic change into the commit:
--- a/drivers/firewire/nosy.c
+++ b/drivers/firewire/nosy.c
@@ -569,7 +569,7 @@ add_card(struct pci_dev *dev, const struct pci_device_id
*unused)
if (lynx->registers == NULL) {
dev_err(>dev, "Failed to map registers\n");
ret = -ENOMEM;
- goto fail_deallocate2;
+ goto fail_deallocate_lynx;
}
lynx->rcv_start_pcl = pci_alloc_consistent(lynx->pci_device,
@@ -583,7 +583,7 @@ add_card(struct pci_dev *dev, const struct pci_device_id
*unused)
lynx->rcv_buffer == NULL) {
dev_err(>dev, "Failed to allocate receive buffer\n");
ret = -ENOMEM;
- goto fail_deallocate;
+ goto fail_deallocate_buffers;
}
lynx->rcv_start_pcl->next = cpu_to_le32(lynx->rcv_pcl_bus);
lynx->rcv_pcl->next = cpu_to_le32(PCL_NEXT_INVALID);
@@ -646,7 +646,7 @@ add_card(struct pci_dev *dev, const struct pci_device_id
*unused)
dev_err(>dev,
"Failed to allocate shared interrupt %d\n", dev->irq);
ret = -EIO;
- goto fail_deallocate;
+ goto fail_deallocate_buffers;
}
lynx->misc.parent = >dev;
@@ -673,7 +673,7 @@ fail_free_irq:
reg_write(lynx, PCI_INT_ENABLE, 0);
free_irq(lynx->pci_device->irq, lynx);
-fail_deallocate:
+fail_deallocate_buffers:
if (lynx->rcv_start_pcl)
pci_free_consistent(lynx->pci_device, sizeof(struct pcl),
lynx->rcv_start_pcl, lynx->rcv_start_pcl_bus);
@@ -685,7 +685,7 @@ fail_deallocate:
lynx->rcv_buffer, lynx->rcv_buffer_bus);
iounmap(lynx->registers);
-fail_deallocate2:
+fail_deallocate_lynx:
kfree(lynx);
fail_disable:
--
Stefan Richter
-==- =-=- -=--=
http://arcgraph.de/sr/
Re: [PATCH] firewire: nosy: do not ignore errors in ioremap_nocache()
On Sep 25 Alexey Khoroshilov wrote:
> There is no check if ioremap_nocache() returns a valid pointer.
> Potentially it can lead to null pointer dereference.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov
> ---
> drivers/firewire/nosy.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/firewire/nosy.c b/drivers/firewire/nosy.c
> index 631c977b0da5..f68a749f740b 100644
> --- a/drivers/firewire/nosy.c
> +++ b/drivers/firewire/nosy.c
> @@ -566,6 +566,11 @@ add_card(struct pci_dev *dev, const struct pci_device_id
> *unused)
>
> lynx->registers = ioremap_nocache(pci_resource_start(dev, 0),
> PCILYNX_MAX_REGISTER);
> + if (lynx->registers == NULL) {
> + dev_err(>dev, "Failed to map registers\n");
> + ret = -ENOMEM;
> + goto fail_deallocate2;
> + }
>
> lynx->rcv_start_pcl = pci_alloc_consistent(lynx->pci_device,
> sizeof(struct pcl), >rcv_start_pcl_bus);
> @@ -679,6 +684,8 @@ fail_deallocate:
> pci_free_consistent(lynx->pci_device, PAGE_SIZE,
> lynx->rcv_buffer, lynx->rcv_buffer_bus);
> iounmap(lynx->registers);
> +
> +fail_deallocate2:
> kfree(lynx);
>
> fail_disable:
Thanks. Committed to linux1394.git.
I folded the following cosmetic change into the commit:
--- a/drivers/firewire/nosy.c
+++ b/drivers/firewire/nosy.c
@@ -569,7 +569,7 @@ add_card(struct pci_dev *dev, const struct pci_device_id
*unused)
if (lynx->registers == NULL) {
dev_err(>dev, "Failed to map registers\n");
ret = -ENOMEM;
- goto fail_deallocate2;
+ goto fail_deallocate_lynx;
}
lynx->rcv_start_pcl = pci_alloc_consistent(lynx->pci_device,
@@ -583,7 +583,7 @@ add_card(struct pci_dev *dev, const struct pci_device_id
*unused)
lynx->rcv_buffer == NULL) {
dev_err(>dev, "Failed to allocate receive buffer\n");
ret = -ENOMEM;
- goto fail_deallocate;
+ goto fail_deallocate_buffers;
}
lynx->rcv_start_pcl->next = cpu_to_le32(lynx->rcv_pcl_bus);
lynx->rcv_pcl->next = cpu_to_le32(PCL_NEXT_INVALID);
@@ -646,7 +646,7 @@ add_card(struct pci_dev *dev, const struct pci_device_id
*unused)
dev_err(>dev,
"Failed to allocate shared interrupt %d\n", dev->irq);
ret = -EIO;
- goto fail_deallocate;
+ goto fail_deallocate_buffers;
}
lynx->misc.parent = >dev;
@@ -673,7 +673,7 @@ fail_free_irq:
reg_write(lynx, PCI_INT_ENABLE, 0);
free_irq(lynx->pci_device->irq, lynx);
-fail_deallocate:
+fail_deallocate_buffers:
if (lynx->rcv_start_pcl)
pci_free_consistent(lynx->pci_device, sizeof(struct pcl),
lynx->rcv_start_pcl, lynx->rcv_start_pcl_bus);
@@ -685,7 +685,7 @@ fail_deallocate:
lynx->rcv_buffer, lynx->rcv_buffer_bus);
iounmap(lynx->registers);
-fail_deallocate2:
+fail_deallocate_lynx:
kfree(lynx);
fail_disable:
--
Stefan Richter
-==- =-=- -=--=
http://arcgraph.de/sr/
Re: [PATCH 02/10] firewire-net: Rename a jump label in fwnet_broadcast_start()
On Sep 18 SF Markus Elfring wrote:
> Adjust jump labels according to the current Linux coding style
> convention.
The current CodingStyle says: "Choose label names which say what the goto
does or why the goto exists." Given the choice between /what/ and /why/,
I for one lean towards /why/.
In this instance, the what and why is "clean up and exit after an error
occurred". 'failed' looks to me to be a better shorthand of this than
'stop_broadcast'. The latter merely says /how/ the cleanup is done.
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
> ---
> drivers/firewire/net.c | 13 ++---
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
> index 7911f13..89afed3 100644
> --- a/drivers/firewire/net.c
> +++ b/drivers/firewire/net.c
> @@ -1106,7 +1106,7 @@ static int fwnet_broadcast_start(struct
> fwnet_device *dev) ptrptr = kmalloc_array(num_packets, sizeof(*ptrptr),
> GFP_KERNEL); if (!ptrptr) {
> retval = -ENOMEM;
> - goto failed;
> + goto stop_broadcast;
> }
> dev->broadcast_rcv_buffer_ptrs = ptrptr;
>
> @@ -1116,13 +1116,13 @@ static int fwnet_broadcast_start(struct
> fwnet_device *dev) fwnet_receive_broadcast, dev);
> if (IS_ERR(context)) {
> retval = PTR_ERR(context);
> - goto failed;
> + goto stop_broadcast;
> }
[...]
> @@ -1166,8 +1166,7 @@ static int fwnet_broadcast_start(struct
> fwnet_device *dev) dev->broadcast_state = FWNET_BROADCAST_RUNNING;
>
> return 0;
> -
> - failed:
> + stop_broadcast:
> __fwnet_broadcast_stop(dev);
> return retval;
> }
I see no reason to remove the blank line between the return statement and
the label.
--
Stefan Richter
-==- =--= ==---
http://arcgraph.de/sr/
Re: [PATCH 02/10] firewire-net: Rename a jump label in fwnet_broadcast_start()
On Sep 18 SF Markus Elfring wrote:
> Adjust jump labels according to the current Linux coding style
> convention.
The current CodingStyle says: "Choose label names which say what the goto
does or why the goto exists." Given the choice between /what/ and /why/,
I for one lean towards /why/.
In this instance, the what and why is "clean up and exit after an error
occurred". 'failed' looks to me to be a better shorthand of this than
'stop_broadcast'. The latter merely says /how/ the cleanup is done.
> Signed-off-by: Markus Elfring
> ---
> drivers/firewire/net.c | 13 ++---
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
> index 7911f13..89afed3 100644
> --- a/drivers/firewire/net.c
> +++ b/drivers/firewire/net.c
> @@ -1106,7 +1106,7 @@ static int fwnet_broadcast_start(struct
> fwnet_device *dev) ptrptr = kmalloc_array(num_packets, sizeof(*ptrptr),
> GFP_KERNEL); if (!ptrptr) {
> retval = -ENOMEM;
> - goto failed;
> + goto stop_broadcast;
> }
> dev->broadcast_rcv_buffer_ptrs = ptrptr;
>
> @@ -1116,13 +1116,13 @@ static int fwnet_broadcast_start(struct
> fwnet_device *dev) fwnet_receive_broadcast, dev);
> if (IS_ERR(context)) {
> retval = PTR_ERR(context);
> - goto failed;
> + goto stop_broadcast;
> }
[...]
> @@ -1166,8 +1166,7 @@ static int fwnet_broadcast_start(struct
> fwnet_device *dev) dev->broadcast_state = FWNET_BROADCAST_RUNNING;
>
> return 0;
> -
> - failed:
> + stop_broadcast:
> __fwnet_broadcast_stop(dev);
> return retval;
> }
I see no reason to remove the blank line between the return statement and
the label.
--
Stefan Richter
-==- =--= ==---
http://arcgraph.de/sr/
Re: [PATCH 10/10] firewire-net: Adjust checks for null pointers in five functions
On Sep 18 SF Markus Elfring wrote: > The script "checkpatch.pl" can point information out like the following. A side note: checkpatch.pl is for authors to help prepare their submissions of new code (or their refactoring of existing code if for some reason a refactoring is undertaken). > Comparison to NULL could be written !… Yes it could, or it could remain as is. Both styles are used in drivers/firewire/net.c, so it is indeed preferable to normalize it to one of them. I keep your patch for the next occasion when related work is being done on drivers/firewire/net.c or drivers/firewire. > Thus fix the affected source code places. It is not a "fix", it is a stylistic change. > Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net> > --- > drivers/firewire/net.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c > index eb7ce5e..e313be3 100644 > --- a/drivers/firewire/net.c > +++ b/drivers/firewire/net.c > @@ -366,13 +366,13 @@ static struct fwnet_partial_datagram > *fwnet_pd_new(struct net_device *net, > > INIT_LIST_HEAD(>fi_list); > fi = fwnet_frag_new(new, frag_off, frag_len); > - if (fi == NULL) > + if (!fi) > goto free_new; [...] -- Stefan Richter -==- =--= ==--- http://arcgraph.de/sr/
Re: [PATCH 10/10] firewire-net: Adjust checks for null pointers in five functions
On Sep 18 SF Markus Elfring wrote: > The script "checkpatch.pl" can point information out like the following. A side note: checkpatch.pl is for authors to help prepare their submissions of new code (or their refactoring of existing code if for some reason a refactoring is undertaken). > Comparison to NULL could be written !… Yes it could, or it could remain as is. Both styles are used in drivers/firewire/net.c, so it is indeed preferable to normalize it to one of them. I keep your patch for the next occasion when related work is being done on drivers/firewire/net.c or drivers/firewire. > Thus fix the affected source code places. It is not a "fix", it is a stylistic change. > Signed-off-by: Markus Elfring > --- > drivers/firewire/net.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c > index eb7ce5e..e313be3 100644 > --- a/drivers/firewire/net.c > +++ b/drivers/firewire/net.c > @@ -366,13 +366,13 @@ static struct fwnet_partial_datagram > *fwnet_pd_new(struct net_device *net, > > INIT_LIST_HEAD(>fi_list); > fi = fwnet_frag_new(new, frag_off, frag_len); > - if (fi == NULL) > + if (!fi) > goto free_new; [...] -- Stefan Richter -==- =--= ==--- http://arcgraph.de/sr/
Re: [PATCH 01/] firewire-net: Use kmalloc_array() in fwnet_broadcast_start()
On Sep 18 SF Markus Elfring wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Sat, 17 Sep 2016 21:55:42 +0200
>
> * A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "kmalloc_array".
>
> This issue was detected by using the Coccinelle software.
>
> * Replace the specification of a data type by a pointer dereference
> to make the corresponding size determination a bit safer according to
> the Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
> ---
> drivers/firewire/net.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
> index 309311b..7911f13 100644
> --- a/drivers/firewire/net.c
> +++ b/drivers/firewire/net.c
> @@ -1103,8 +1103,7 @@ static int fwnet_broadcast_start(struct fwnet_device
> *dev)
>
> max_receive = 1U << (dev->card->max_receive + 1);
> num_packets = (FWNET_ISO_PAGE_COUNT * PAGE_SIZE) / max_receive;
> -
> - ptrptr = kmalloc(sizeof(void *) * num_packets, GFP_KERNEL);
> + ptrptr = kmalloc_array(num_packets, sizeof(*ptrptr), GFP_KERNEL);
> if (!ptrptr) {
> retval = -ENOMEM;
> goto failed;
Coccinelle enabled you to determine that kmalloc_array /could/ be used
here. But whether it /should/ be used here is another question, and it is
not addressed in your changelog. (You state that there is an "issue" but
do not explain.)
kmalloc_array is a kmalloc wrapper which adds an inline check for integer
overflow. So, can sizeof(void *) * num_packets ever overflow size_t?
If yes,
do we want a runtime check here (which kmalloc_array provides),
or do we want a compile-time check?
If no,
then the remaining benefit of the patch is that it is more obvious
to the reader that dev->broadcast_rcv_buffer_ptrs is an array,
but possibly at the cost of superfluous code. Is gcc's optimizer
able to resolve kmalloc_array's check at compile time as always
false, such that the superfluous code is eliminated as dead code?
I believe I know answers to this but prefer to hear what you as the patch
author think about it.
--
Stefan Richter
-==- =--= ==---
http://arcgraph.de/sr/
Re: [PATCH 01/] firewire-net: Use kmalloc_array() in fwnet_broadcast_start()
On Sep 18 SF Markus Elfring wrote:
> From: Markus Elfring
> Date: Sat, 17 Sep 2016 21:55:42 +0200
>
> * A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "kmalloc_array".
>
> This issue was detected by using the Coccinelle software.
>
> * Replace the specification of a data type by a pointer dereference
> to make the corresponding size determination a bit safer according to
> the Linux coding style convention.
>
> Signed-off-by: Markus Elfring
> ---
> drivers/firewire/net.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
> index 309311b..7911f13 100644
> --- a/drivers/firewire/net.c
> +++ b/drivers/firewire/net.c
> @@ -1103,8 +1103,7 @@ static int fwnet_broadcast_start(struct fwnet_device
> *dev)
>
> max_receive = 1U << (dev->card->max_receive + 1);
> num_packets = (FWNET_ISO_PAGE_COUNT * PAGE_SIZE) / max_receive;
> -
> - ptrptr = kmalloc(sizeof(void *) * num_packets, GFP_KERNEL);
> + ptrptr = kmalloc_array(num_packets, sizeof(*ptrptr), GFP_KERNEL);
> if (!ptrptr) {
> retval = -ENOMEM;
> goto failed;
Coccinelle enabled you to determine that kmalloc_array /could/ be used
here. But whether it /should/ be used here is another question, and it is
not addressed in your changelog. (You state that there is an "issue" but
do not explain.)
kmalloc_array is a kmalloc wrapper which adds an inline check for integer
overflow. So, can sizeof(void *) * num_packets ever overflow size_t?
If yes,
do we want a runtime check here (which kmalloc_array provides),
or do we want a compile-time check?
If no,
then the remaining benefit of the patch is that it is more obvious
to the reader that dev->broadcast_rcv_buffer_ptrs is an array,
but possibly at the cost of superfluous code. Is gcc's optimizer
able to resolve kmalloc_array's check at compile time as always
false, such that the superfluous code is eliminated as dead code?
I believe I know answers to this but prefer to hear what you as the patch
author think about it.
--
Stefan Richter
-==- =--= ==---
http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 06 Daniel Vetter wrote: > On Thu, May 05, 2016 at 10:45:31PM +0200, Stefan Richter wrote: [...] > > Subtest fbc-1p-primscrn-spr-indfb-fullscreen: FAIL (5.876s) > > This one failed in both runs. Can you please retest with just that using > > # kms_frontbuffer_tracking --run-subtest fbc-1p-primscrn-spr-indfb-fullscreen > > Also please boot with drm.debug=0xe and grab the full dmesg of just that > single subtest. There's definitely something going wrong here. I performed this test with - plain v4.6-rc6, - v4.6-rc6 patched with drm-intel-nightly (2016y-05m-06d-14h-29m-58s). On v4.6-rc6, the test failed thus: 8< Subtest fbc-1p-primscrn-spr-indfb-fullscreen failed. DEBUG (kms_frontbuffer_tracking:1914) DEBUG: Test requirement passed: fbc.can_test (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(width=2560, height=1440, format=0x34325258, tiling=0x101, size=14745600) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(handle=6, pitch=10240) (kms_frontbuffer_tracking:1914) DEBUG: Blue CRC: pipe:[2ca73d01 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(width=2560, height=1440, format=0x34325258, tiling=0x101, size=14745600) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(handle=6, pitch=10240) (kms_frontbuffer_tracking:1914) igt-draw-DEBUG: Test requirement passed: intel_gen(intel_get_drm_devid(fd)) >= 5 (kms_frontbuffer_tracking:1914) DEBUG: Rect 0 CRC: pipe:[febb8b20 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) igt-draw-DEBUG: Test requirement passed: intel_gen(intel_get_drm_devid(fd)) >= 5 (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[2ca73d01 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) DEBUG: Test requirement passed: !fbc_not_enough_stolen() (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[2ca73d01 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) igt-draw-DEBUG: Test requirement passed: intel_gen(intel_get_drm_devid(fd)) >= 5 (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[2ca73d01 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) DEBUG: Test requirement passed: !fbc_not_enough_stolen() (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[2ca73d01 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(width=2560, height=1440, format=0x34325258, tiling=0x101, size=14745600) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(handle=6, pitch=10240) (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[febb8b20 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) DEBUG: Test requirement passed: !fbc_not_enough_stolen() (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[febb8b20 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) igt-core-INFO: Timed out: CRC reading END IGT-Version: 1.14-gc03a8ae6bf2f (x86_64) (Linux: 4.6.0-rc6 x86_64) Primary screen: DP 2560x1440, crtc 26 FBC last action not supported Can't test PSR: no usable eDP screen. Sink CRC not supported: primary screen is not eDP Timed out: CRC reading Subtest fbc-1p-primscrn-spr-indfb-fullscreen: FAIL (5.806s) >8 On v4.6-rc6 plus drm-intel-nightly, the test apparently passed: 8< IGT-Version: 1.14-gc03a8ae6bf2f (x86_64) (Linux: 4.6.0-rc6+intel-drm-n
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 06 Daniel Vetter wrote: > On Thu, May 05, 2016 at 10:45:31PM +0200, Stefan Richter wrote: [...] > > Subtest fbc-1p-primscrn-spr-indfb-fullscreen: FAIL (5.876s) > > This one failed in both runs. Can you please retest with just that using > > # kms_frontbuffer_tracking --run-subtest fbc-1p-primscrn-spr-indfb-fullscreen > > Also please boot with drm.debug=0xe and grab the full dmesg of just that > single subtest. There's definitely something going wrong here. I performed this test with - plain v4.6-rc6, - v4.6-rc6 patched with drm-intel-nightly (2016y-05m-06d-14h-29m-58s). On v4.6-rc6, the test failed thus: 8< Subtest fbc-1p-primscrn-spr-indfb-fullscreen failed. DEBUG (kms_frontbuffer_tracking:1914) DEBUG: Test requirement passed: fbc.can_test (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(width=2560, height=1440, format=0x34325258, tiling=0x101, size=14745600) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(handle=6, pitch=10240) (kms_frontbuffer_tracking:1914) DEBUG: Blue CRC: pipe:[2ca73d01 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(width=2560, height=1440, format=0x34325258, tiling=0x101, size=14745600) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(handle=6, pitch=10240) (kms_frontbuffer_tracking:1914) igt-draw-DEBUG: Test requirement passed: intel_gen(intel_get_drm_devid(fd)) >= 5 (kms_frontbuffer_tracking:1914) DEBUG: Rect 0 CRC: pipe:[febb8b20 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) igt-draw-DEBUG: Test requirement passed: intel_gen(intel_get_drm_devid(fd)) >= 5 (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[2ca73d01 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) DEBUG: Test requirement passed: !fbc_not_enough_stolen() (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[2ca73d01 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) igt-draw-DEBUG: Test requirement passed: intel_gen(intel_get_drm_devid(fd)) >= 5 (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[2ca73d01 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) DEBUG: Test requirement passed: !fbc_not_enough_stolen() (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[2ca73d01 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(width=2560, height=1440, format=0x34325258, tiling=0x101, size=14745600) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) drmtest-DEBUG: Test requirement passed: is_i915_device(fd) && has_known_intel_chipset(fd) (kms_frontbuffer_tracking:1914) igt-fb-DEBUG: igt_create_fb_with_bo_size(handle=6, pitch=10240) (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[febb8b20 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) DEBUG: Test requirement passed: !fbc_not_enough_stolen() (kms_frontbuffer_tracking:1914) DEBUG: Calculated CRC: pipe:[febb8b20 ] sink:[unsupported!] (kms_frontbuffer_tracking:1914) igt-core-INFO: Timed out: CRC reading END IGT-Version: 1.14-gc03a8ae6bf2f (x86_64) (Linux: 4.6.0-rc6 x86_64) Primary screen: DP 2560x1440, crtc 26 FBC last action not supported Can't test PSR: no usable eDP screen. Sink CRC not supported: primary screen is not eDP Timed out: CRC reading Subtest fbc-1p-primscrn-spr-indfb-fullscreen: FAIL (5.806s) >8 On v4.6-rc6 plus drm-intel-nightly, the test apparently passed: 8< IGT-Version: 1.14-gc03a8ae6bf2f (x86_64) (Linux: 4.6.0-rc6+intel-drm-n
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 08 Stefan Richter wrote: > On May 05 Zanoni, Paulo R wrote: > > If you don't want to keep carrying a manual revert, you can just boot > > with i915.enable_fbc=0 for now (or write a /etc/modprobe.d file). Also, > > it would be good to know in case you still somehow see the machine > > hangs even with FBC disabled. > > As expected, i915.enable_fbc=0 works fine. > No freeze within 2.5 days uptime; tested on v4.6-rc6. Furthermore, I checked out drm-intel.git (v4.6-rc6-962-g91567024d358 "drm-intel-nightly: 2016y-05m-06d-14h-29m-58s UTC integration manifest") and applied "git diff v4.6-rc6..." on top of v4.6-rc6. I booted the result once with default i915.enable_fbc, i.e. FBC enabled, performed the test which Daniel asked for (I will post the results in another message), then started X11. - The good news: I was able to switch back and forth between the sddm greeter screen on tty7, the text consoles at tty1...6, and the logger at tty12 --- without getting any FIFO underrun messages and without getting stuck with a blank screen. - The bad news: Less than a minute after login into sddm, just after having started openbox + lxpanel + konsole, the kernel froze again without netconsole output. I am now on 4.6.0-rc6+intel-drm-nightly with i915.enable_fbc=0. This is running fine so far. (uptime is just 30 minutes now though, so that doesn't say a lot.) Again, switching between ttys works without FIFO underruns, unlike plain v4.6-rc6. Not sure if it is coincidence or if this is because somebody fixed something. Like v4.6-rc6 and older,4.6.0-rc6+intel-drm-nightly still exhibits the following behaviour: If I switch the displayport connected monitor off and on again, the following messages are logged when the monitor comes on: [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* uncleared fifo underrun on pipe A [drm:intel_cpu_fifo_underrun_irq_handler] *ERROR* CPU pipe A FIFO underrun Other than these messages, there is nothing extraordinary going on. -- Stefan Richter -==- -=-= -=--- http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 08 Stefan Richter wrote: > On May 05 Zanoni, Paulo R wrote: > > If you don't want to keep carrying a manual revert, you can just boot > > with i915.enable_fbc=0 for now (or write a /etc/modprobe.d file). Also, > > it would be good to know in case you still somehow see the machine > > hangs even with FBC disabled. > > As expected, i915.enable_fbc=0 works fine. > No freeze within 2.5 days uptime; tested on v4.6-rc6. Furthermore, I checked out drm-intel.git (v4.6-rc6-962-g91567024d358 "drm-intel-nightly: 2016y-05m-06d-14h-29m-58s UTC integration manifest") and applied "git diff v4.6-rc6..." on top of v4.6-rc6. I booted the result once with default i915.enable_fbc, i.e. FBC enabled, performed the test which Daniel asked for (I will post the results in another message), then started X11. - The good news: I was able to switch back and forth between the sddm greeter screen on tty7, the text consoles at tty1...6, and the logger at tty12 --- without getting any FIFO underrun messages and without getting stuck with a blank screen. - The bad news: Less than a minute after login into sddm, just after having started openbox + lxpanel + konsole, the kernel froze again without netconsole output. I am now on 4.6.0-rc6+intel-drm-nightly with i915.enable_fbc=0. This is running fine so far. (uptime is just 30 minutes now though, so that doesn't say a lot.) Again, switching between ttys works without FIFO underruns, unlike plain v4.6-rc6. Not sure if it is coincidence or if this is because somebody fixed something. Like v4.6-rc6 and older,4.6.0-rc6+intel-drm-nightly still exhibits the following behaviour: If I switch the displayport connected monitor off and on again, the following messages are logged when the monitor comes on: [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* uncleared fifo underrun on pipe A [drm:intel_cpu_fifo_underrun_irq_handler] *ERROR* CPU pipe A FIFO underrun Other than these messages, there is nothing extraordinary going on. -- Stefan Richter -==- -=-= -=--- http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 05 Zanoni, Paulo R wrote: > If you don't want to keep carrying a manual revert, you can just boot > with i915.enable_fbc=0 for now (or write a /etc/modprobe.d file). Also, > it would be good to know in case you still somehow see the machine > hangs even with FBC disabled. As expected, i915.enable_fbc=0 works fine. No freeze within 2.5 days uptime; tested on v4.6-rc6. -- Stefan Richter -==- -=-= -=--- http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 05 Zanoni, Paulo R wrote: > If you don't want to keep carrying a manual revert, you can just boot > with i915.enable_fbc=0 for now (or write a /etc/modprobe.d file). Also, > it would be good to know in case you still somehow see the machine > hangs even with FBC disabled. As expected, i915.enable_fbc=0 works fine. No freeze within 2.5 days uptime; tested on v4.6-rc6. -- Stefan Richter -==- -=-= -=--- http://arcgraph.de/sr/
Re: [Intel-gfx] Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 05 Daniel Vetter wrote: > Hm, if it's watermarks then testing with latest drm-intel-nightly would be > interesting. We finally managed to land atomic watermark updates (should > all be there in 4.7 too): > > https://cgit.freedesktop.org/drm-intel I will see if I can test this sometime soon. -- Stefan Richter -==- -=-= --==- http://arcgraph.de/sr/
Re: [Intel-gfx] Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 05 Daniel Vetter wrote: > Hm, if it's watermarks then testing with latest drm-intel-nightly would be > interesting. We finally managed to land atomic watermark updates (should > all be there in 4.7 too): > > https://cgit.freedesktop.org/drm-intel I will see if I can test this sometime soon. -- Stefan Richter -==- -=-= --==- http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 05 Zanoni, Paulo R wrote: > Em Qui, 2016-05-05 às 19:45 +0200, Stefan Richter escreveu: > > Oh, and in case you - the person reading this commit message - found > > this commit through git bisect, please do the following: > > - Check your dmesg and see if there are error messages mentioning > > underruns around the time your problem started happening. > > > > Well, I always had the followings lines in dmesg: > > [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* uncleared fifo underrun > > on pipe A > > [drm:intel_cpu_fifo_underrun_irq_handler] *ERROR* CPU pipe A FIFO underrun > > Oh, well... I had a patch that would just disable FBC in case we saw a > FIFO underrun, but it was rejected. Maybe this is the time to think > about it again? Otherwise, I can't think of much besides disabling FBC > on HSW until all the underruns and watermarks regressions are fixed > forever. Just to be clear though, I know that these messages are emitted when the monitor is switched on, and when sddm is being shut down --- but I do not know whether there is any sort of underrun when I get the FBC related freeze (since I just don't get any kernel messages at that point). Is there a chance that a serial console would fare better than netconsole? This board and another PC in its vicinity have got onboard serial ports but I don't have cables at the moment. > > - Download intel-gpu-tools, compile it, and run: > > $ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 > > | tee fbc.txt > > Then send us the fbc.txt file, especially if you get a failure. > > This will really maximize your chances of getting the bug fixed > > quickly. > > > > Do you need this while FBC is enabled, or can I run it while FBC is > > disabled? > > FBC enabled. Considering your description, my hope is that maybe some > specific subtest will be able to hang your machine, so testing this > again will require only running the specific subtest instead of waiting > 18 hours. The kms_frontbuffer_tracking runs from which I posted output two hours ago did not trigger a lockup. (I ran them while X11 was shut down because otherwise kms_frontbuffer_tracking would skip all tests with "Can't become DRM master, please check if no other DRM client is running.") > > PS: > > I am mentioning the following just in case that it has any relationship > > with the FBC related kernel freezes. Maybe it doesn't... There is > > another recent regression on this PC, but I have not yet figured out > > whether it was introduced by any particular kernel version. The > > regression is: When switching from X11 to text console by [Ctrl][Alt][Fx] > > or by shutting down sddm, I often only get a blank screen. I suspect > > that this regression was introduced when I replaced kdm by sddm, but > > I am not sure about that. > > Maybe there is some relationship, since this operation involves a mode > change. You can also try checking dmesg to see if there are underruns > right when you do the change. Yes, this is accompanied by [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* uncleared fifo underrun on pipe A [drm:intel_cpu_fifo_underrun_irq_handler] *ERROR* CPU pipe A FIFO underrun -- Stefan Richter -==- -=-= --=-= http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 05 Zanoni, Paulo R wrote: > Em Qui, 2016-05-05 às 19:45 +0200, Stefan Richter escreveu: > > Oh, and in case you - the person reading this commit message - found > > this commit through git bisect, please do the following: > > - Check your dmesg and see if there are error messages mentioning > > underruns around the time your problem started happening. > > > > Well, I always had the followings lines in dmesg: > > [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* uncleared fifo underrun > > on pipe A > > [drm:intel_cpu_fifo_underrun_irq_handler] *ERROR* CPU pipe A FIFO underrun > > Oh, well... I had a patch that would just disable FBC in case we saw a > FIFO underrun, but it was rejected. Maybe this is the time to think > about it again? Otherwise, I can't think of much besides disabling FBC > on HSW until all the underruns and watermarks regressions are fixed > forever. Just to be clear though, I know that these messages are emitted when the monitor is switched on, and when sddm is being shut down --- but I do not know whether there is any sort of underrun when I get the FBC related freeze (since I just don't get any kernel messages at that point). Is there a chance that a serial console would fare better than netconsole? This board and another PC in its vicinity have got onboard serial ports but I don't have cables at the moment. > > - Download intel-gpu-tools, compile it, and run: > > $ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 > > | tee fbc.txt > > Then send us the fbc.txt file, especially if you get a failure. > > This will really maximize your chances of getting the bug fixed > > quickly. > > > > Do you need this while FBC is enabled, or can I run it while FBC is > > disabled? > > FBC enabled. Considering your description, my hope is that maybe some > specific subtest will be able to hang your machine, so testing this > again will require only running the specific subtest instead of waiting > 18 hours. The kms_frontbuffer_tracking runs from which I posted output two hours ago did not trigger a lockup. (I ran them while X11 was shut down because otherwise kms_frontbuffer_tracking would skip all tests with "Can't become DRM master, please check if no other DRM client is running.") > > PS: > > I am mentioning the following just in case that it has any relationship > > with the FBC related kernel freezes. Maybe it doesn't... There is > > another recent regression on this PC, but I have not yet figured out > > whether it was introduced by any particular kernel version. The > > regression is: When switching from X11 to text console by [Ctrl][Alt][Fx] > > or by shutting down sddm, I often only get a blank screen. I suspect > > that this regression was introduced when I replaced kdm by sddm, but > > I am not sure about that. > > Maybe there is some relationship, since this operation involves a mode > change. You can also try checking dmesg to see if there are underruns > right when you do the change. Yes, this is accompanied by [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* uncleared fifo underrun on pipe A [drm:intel_cpu_fifo_underrun_irq_handler] *ERROR* CPU pipe A FIFO underrun -- Stefan Richter -==- -=-= --=-= http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 05 Stefan Richter wrote: > Quoting the changelog of the commit: [...] > - Download intel-gpu-tools, compile it, and run: >$ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 | > tee fbc.txt >Then send us the fbc.txt file, especially if you get a failure. Attached are results of kms_frontbuffer_tracking from current intel-gpu-tools.git (intel-gpu-tools-1.14-273-gb4b2ac346c92), taken on kernel v4.5.2 and on v4.6-rc5. -- Stefan Richter -==- -=-= --=-= http://arcgraph.de/sr/ IGT-Version: 1.14-g99e61ed66f65 (x86_64) (Linux: 4.5.2 x86_64) Primary screen: DP 2560x1440, crtc 21 FBC last action not supported Can't test PSR: no usable eDP screen. Sink CRC not supported: primary screen is not eDP Subtest fbc-1p-rte: SUCCESS (2.754s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-rte: SKIP (0.000s) Subtest fbc-1p-primscrn-pri-indfb-draw-mmap-cpu: SUCCESS (1.468s) Subtest fbc-1p-primscrn-pri-indfb-draw-mmap-gtt: SUCCESS (0.787s) Subtest fbc-1p-primscrn-pri-indfb-draw-mmap-wc: SUCCESS (0.774s) Subtest fbc-1p-primscrn-pri-indfb-draw-pwrite: SUCCESS (0.958s) Subtest fbc-1p-primscrn-pri-indfb-draw-blt: SUCCESS (0.990s) Subtest fbc-1p-primscrn-pri-indfb-draw-render: SUCCESS (0.984s) Subtest fbc-1p-primscrn-pri-shrfb-draw-mmap-cpu: SUCCESS (0.948s) Subtest fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt: SUCCESS (0.793s) Subtest fbc-1p-primscrn-pri-shrfb-draw-mmap-wc: SUCCESS (0.836s) Subtest fbc-1p-primscrn-pri-shrfb-draw-pwrite: SUCCESS (1.171s) Subtest fbc-1p-primscrn-pri-shrfb-draw-blt: SUCCESS (0.995s) Subtest fbc-1p-primscrn-pri-shrfb-draw-render: SUCCESS (0.977s) Subtest fbc-1p-primscrn-cur-indfb-draw-mmap-cpu: SUCCESS (1.403s) Subtest fbc-1p-primscrn-cur-indfb-draw-mmap-gtt: SUCCESS (0.964s) Subtest fbc-1p-primscrn-cur-indfb-draw-mmap-wc: SUCCESS (0.939s) Subtest fbc-1p-primscrn-cur-indfb-draw-pwrite: SUCCESS (0.965s) Subtest fbc-1p-primscrn-cur-indfb-draw-blt: SUCCESS (0.939s) Subtest fbc-1p-primscrn-cur-indfb-draw-render: SUCCESS (0.941s) Subtest fbc-1p-primscrn-spr-indfb-draw-mmap-cpu: SUCCESS (1.010s) Subtest fbc-1p-primscrn-spr-indfb-draw-mmap-gtt: SUCCESS (0.971s) Subtest fbc-1p-primscrn-spr-indfb-draw-mmap-wc: SUCCESS (0.961s) Subtest fbc-1p-primscrn-spr-indfb-draw-pwrite: SUCCESS (1.010s) Subtest fbc-1p-primscrn-spr-indfb-draw-blt: SUCCESS (0.956s) Subtest fbc-1p-primscrn-spr-indfb-draw-render: SUCCESS (1.010s) Subtest fbc-1p-offscren-pri-indfb-draw-mmap-cpu: SUCCESS (0.502s) Subtest fbc-1p-offscren-pri-indfb-draw-mmap-gtt: SUCCESS (0.500s) Subtest fbc-1p-offscren-pri-indfb-draw-mmap-wc: SUCCESS (0.493s) Subtest fbc-1p-offscren-pri-indfb-draw-pwrite: SUCCESS (0.507s) Subtest fbc-1p-offscren-pri-indfb-draw-blt: SUCCESS (0.498s) Subtest fbc-1p-offscren-pri-indfb-draw-render: SUCCESS (0.486s) Subtest fbc-1p-offscren-pri-shrfb-draw-mmap-cpu: SUCCESS (0.578s) Subtest fbc-1p-offscren-pri-shrfb-draw-mmap-gtt: SUCCESS (0.459s) Subtest fbc-1p-offscren-pri-shrfb-draw-mmap-wc: SUCCESS (0.530s) Subtest fbc-1p-offscren-pri-shrfb-draw-pwrite: SUCCESS (1.096s) Subtest fbc-1p-offscren-pri-shrfb-draw-blt: SUCCESS (0.660s) Subtest fbc-1p-offscren-pri-shrfb-draw-render: SUCCESS (0.661s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-mmap-cpu: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-mmap-gtt: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-mmap-wc: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-pwrite: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-blt: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-render: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 05 Stefan Richter wrote: > Quoting the changelog of the commit: [...] > - Download intel-gpu-tools, compile it, and run: >$ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 | > tee fbc.txt >Then send us the fbc.txt file, especially if you get a failure. Attached are results of kms_frontbuffer_tracking from current intel-gpu-tools.git (intel-gpu-tools-1.14-273-gb4b2ac346c92), taken on kernel v4.5.2 and on v4.6-rc5. -- Stefan Richter -==- -=-= --=-= http://arcgraph.de/sr/ IGT-Version: 1.14-g99e61ed66f65 (x86_64) (Linux: 4.5.2 x86_64) Primary screen: DP 2560x1440, crtc 21 FBC last action not supported Can't test PSR: no usable eDP screen. Sink CRC not supported: primary screen is not eDP Subtest fbc-1p-rte: SUCCESS (2.754s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-rte: SKIP (0.000s) Subtest fbc-1p-primscrn-pri-indfb-draw-mmap-cpu: SUCCESS (1.468s) Subtest fbc-1p-primscrn-pri-indfb-draw-mmap-gtt: SUCCESS (0.787s) Subtest fbc-1p-primscrn-pri-indfb-draw-mmap-wc: SUCCESS (0.774s) Subtest fbc-1p-primscrn-pri-indfb-draw-pwrite: SUCCESS (0.958s) Subtest fbc-1p-primscrn-pri-indfb-draw-blt: SUCCESS (0.990s) Subtest fbc-1p-primscrn-pri-indfb-draw-render: SUCCESS (0.984s) Subtest fbc-1p-primscrn-pri-shrfb-draw-mmap-cpu: SUCCESS (0.948s) Subtest fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt: SUCCESS (0.793s) Subtest fbc-1p-primscrn-pri-shrfb-draw-mmap-wc: SUCCESS (0.836s) Subtest fbc-1p-primscrn-pri-shrfb-draw-pwrite: SUCCESS (1.171s) Subtest fbc-1p-primscrn-pri-shrfb-draw-blt: SUCCESS (0.995s) Subtest fbc-1p-primscrn-pri-shrfb-draw-render: SUCCESS (0.977s) Subtest fbc-1p-primscrn-cur-indfb-draw-mmap-cpu: SUCCESS (1.403s) Subtest fbc-1p-primscrn-cur-indfb-draw-mmap-gtt: SUCCESS (0.964s) Subtest fbc-1p-primscrn-cur-indfb-draw-mmap-wc: SUCCESS (0.939s) Subtest fbc-1p-primscrn-cur-indfb-draw-pwrite: SUCCESS (0.965s) Subtest fbc-1p-primscrn-cur-indfb-draw-blt: SUCCESS (0.939s) Subtest fbc-1p-primscrn-cur-indfb-draw-render: SUCCESS (0.941s) Subtest fbc-1p-primscrn-spr-indfb-draw-mmap-cpu: SUCCESS (1.010s) Subtest fbc-1p-primscrn-spr-indfb-draw-mmap-gtt: SUCCESS (0.971s) Subtest fbc-1p-primscrn-spr-indfb-draw-mmap-wc: SUCCESS (0.961s) Subtest fbc-1p-primscrn-spr-indfb-draw-pwrite: SUCCESS (1.010s) Subtest fbc-1p-primscrn-spr-indfb-draw-blt: SUCCESS (0.956s) Subtest fbc-1p-primscrn-spr-indfb-draw-render: SUCCESS (1.010s) Subtest fbc-1p-offscren-pri-indfb-draw-mmap-cpu: SUCCESS (0.502s) Subtest fbc-1p-offscren-pri-indfb-draw-mmap-gtt: SUCCESS (0.500s) Subtest fbc-1p-offscren-pri-indfb-draw-mmap-wc: SUCCESS (0.493s) Subtest fbc-1p-offscren-pri-indfb-draw-pwrite: SUCCESS (0.507s) Subtest fbc-1p-offscren-pri-indfb-draw-blt: SUCCESS (0.498s) Subtest fbc-1p-offscren-pri-indfb-draw-render: SUCCESS (0.486s) Subtest fbc-1p-offscren-pri-shrfb-draw-mmap-cpu: SUCCESS (0.578s) Subtest fbc-1p-offscren-pri-shrfb-draw-mmap-gtt: SUCCESS (0.459s) Subtest fbc-1p-offscren-pri-shrfb-draw-mmap-wc: SUCCESS (0.530s) Subtest fbc-1p-offscren-pri-shrfb-draw-pwrite: SUCCESS (1.096s) Subtest fbc-1p-offscren-pri-shrfb-draw-blt: SUCCESS (0.660s) Subtest fbc-1p-offscren-pri-shrfb-draw-render: SUCCESS (0.661s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-mmap-cpu: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-mmap-gtt: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-mmap-wc: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-pwrite: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-blt: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:1816: Test requirement: scnd_mode_params.connector_id Can't test dual pipes with the current outputs Subtest fbc-2p-primscrn-pri-indfb-draw-render: SKIP (0.000s) Test requirement not met in function check_test_requirements, file kms_frontbuffer_tracking.c:
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 05 Stefan Richter wrote: > Quoting the changelog of the commit: [...] > - Boot with drm.debug=0xe, reproduce the problem, then send us the >dmesg file. > > I can try this, but I am skeptical about getting any useful kernel > messages from before the hang. I booted 4.6-rc5 with drm.debug=0xe. It hung after about 80 minutes uptime, and just like at all previous hangs, netconsole did not capture anything at the time when it froze. Here is "dmesg | grep -e :00:02.0 -e i915 -e drm" from that session. [0.00] Command line: BOOT_IMAGE=/vmlinuz-4.6.0-rc5 root=/dev/sda4 ro rootflags=subvol=@ drm.debug=0xe [0.00] Kernel command line: BOOT_IMAGE=/vmlinuz-4.6.0-rc5 root=/dev/sda4 ro rootflags=subvol=@ drm.debug=0xe [0.673659] pci :00:02.0: [8086:041a] type 00 class 0x03 [0.673666] pci :00:02.0: reg 0x10: [mem 0xf580-0xf5bf 64bit] [0.673670] pci :00:02.0: reg 0x18: [mem 0xe000-0xefff 64bit pref] [0.673673] pci :00:02.0: reg 0x20: [io 0xf000-0xf03f] [0.705036] vgaarb: setting as boot device: PCI::00:02.0 [0.705113] vgaarb: device added: PCI::00:02.0,decodes=io+mem,owns=io+mem,locks=none [0.705300] vgaarb: bridge control possible :00:02.0 [0.727542] pci :00:02.0: Video device with shadowed ROM at [mem 0x000c-0x000d] [0.766034] [drm] Initialized drm 1.1.0 20060810 [0.766222] [drm:i915_dump_device_info] i915 device info: gen=7, pciid=0x041a rev=0x06 flags=need_gfx_hws,is_haswell,has_fbc,has_hotplug,has_llc,has_ddi,has_fpga_dbg, [0.766229] [drm:intel_detect_pch] Found LynxPoint PCH [0.766320] [drm:i915_gem_init_stolen] Memory reserved for graphics device: 32768K, usable: 31744K [0.766321] [drm] Memory usable by graphics device = 2048M [0.766398] [drm:i915_gem_gtt_init] GMADR size = 256M [0.766399] [drm:i915_gem_gtt_init] GTT stolen size = 32M [0.766399] [drm:i915_gem_gtt_init] ppgtt mode: 1 [0.766400] [drm] Replacing VGA console driver [0.767158] [drm:intel_opregion_setup] graphic opregion physical addr: 0xd9509018 [0.767161] [drm:intel_opregion_setup] Public ACPI methods supported [0.767162] [drm:intel_opregion_setup] SWSCI supported [0.772643] [drm:swsci_setup] SWSCI GBDA callbacks 0cb3, SBCB callbacks 00300483 [0.772646] [drm:intel_opregion_setup] ASLE supported [0.772646] [drm:intel_opregion_setup] ASLE extension supported [0.772648] [drm:intel_opregion_setup] Found valid VBT in ACPI OpRegion (Mailbox #4) [0.772717] [drm:intel_device_info_runtime_init] slice total: 0 [0.772717] [drm:intel_device_info_runtime_init] subslice total: 0 [0.772718] [drm:intel_device_info_runtime_init] subslice per slice: 0 [0.772719] [drm:intel_device_info_runtime_init] EU total: 0 [0.772720] [drm:intel_device_info_runtime_init] EU per subslice: 0 [0.772720] [drm:intel_device_info_runtime_init] has slice power gating: n [0.772721] [drm:intel_device_info_runtime_init] has subslice power gating: n [0.772722] [drm:intel_device_info_runtime_init] has EU power gating: n [0.772722] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [0.772725] [drm] Driver supports precise vblank timestamp query. [0.772727] [drm:init_vbt_defaults] Set default to SSC at 12 kHz [0.772728] [drm:intel_bios_init] VBT signature "$VBT HASWELL", BDB version 170 [0.772730] [drm:parse_general_features] BDB_GENERAL_FEATURES int_tv_support 0 int_crt_support 1 lvds_use_ssc 0 lvds_ssc_freq 12 display_clock_mode 0 fdi_rx_polarity_inverted 0 [0.772731] [drm:parse_general_definitions] crt_ddc_bus_pin: 2 [0.772732] [drm:parse_lfp_panel_data] DRRS supported mode is static [0.772734] [drm:parse_lfp_panel_data] Found panel mode in BIOS VBT tables: [0.772735] [drm:drm_mode_debug_printmodeline] Modeline 0:"1024x768" 0 65000 1024 1048 1184 1344 768 771 777 806 0x8 0xa [0.772736] [drm:parse_lfp_panel_data] VBT initial LVDS value 300 [0.772738] [drm:parse_lfp_backlight] VBT backlight PWM modulation frequency 200 Hz, active high, min brightness 0, level 255 [0.772739] [drm:parse_sdvo_panel_data] Found SDVO panel mode in BIOS VBT tables: [0.772740] [drm:drm_mode_debug_printmodeline] Modeline 0:"1600x1200" 0 162000 1600 1664 1856 2160 1200 1201 1204 1250 0x8 0xa [0.772741] [drm:parse_sdvo_device_mapping] No SDVO device info is found in VBT [0.772742] [drm:parse_driver_features] DRRS State Enabled:1 [0.772743] [drm:parse_ddi_port] Port B VBT info: DP:1 HDMI:1 DVI:1 EDP:0 CRT:0 [0.772745] [drm:parse_ddi_port] VBT HDMI level shift for port B: 6 [0.772745] [drm:parse_ddi_port] Port C VBT info: DP:0 HDMI:1 DVI:1 EDP:0 CRT:0 [0.772746] [drm:parse_ddi_port] VBT HDMI level shift for port C: 6 [0.772747] [drm:parse_ddi_port] Port D VBT info: DP:1 HDMI:1 DVI:1 EDP:0 CRT:0 [0.77274
Re: Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On May 05 Stefan Richter wrote: > Quoting the changelog of the commit: [...] > - Boot with drm.debug=0xe, reproduce the problem, then send us the >dmesg file. > > I can try this, but I am skeptical about getting any useful kernel > messages from before the hang. I booted 4.6-rc5 with drm.debug=0xe. It hung after about 80 minutes uptime, and just like at all previous hangs, netconsole did not capture anything at the time when it froze. Here is "dmesg | grep -e :00:02.0 -e i915 -e drm" from that session. [0.00] Command line: BOOT_IMAGE=/vmlinuz-4.6.0-rc5 root=/dev/sda4 ro rootflags=subvol=@ drm.debug=0xe [0.00] Kernel command line: BOOT_IMAGE=/vmlinuz-4.6.0-rc5 root=/dev/sda4 ro rootflags=subvol=@ drm.debug=0xe [0.673659] pci :00:02.0: [8086:041a] type 00 class 0x03 [0.673666] pci :00:02.0: reg 0x10: [mem 0xf580-0xf5bf 64bit] [0.673670] pci :00:02.0: reg 0x18: [mem 0xe000-0xefff 64bit pref] [0.673673] pci :00:02.0: reg 0x20: [io 0xf000-0xf03f] [0.705036] vgaarb: setting as boot device: PCI::00:02.0 [0.705113] vgaarb: device added: PCI::00:02.0,decodes=io+mem,owns=io+mem,locks=none [0.705300] vgaarb: bridge control possible :00:02.0 [0.727542] pci :00:02.0: Video device with shadowed ROM at [mem 0x000c-0x000d] [0.766034] [drm] Initialized drm 1.1.0 20060810 [0.766222] [drm:i915_dump_device_info] i915 device info: gen=7, pciid=0x041a rev=0x06 flags=need_gfx_hws,is_haswell,has_fbc,has_hotplug,has_llc,has_ddi,has_fpga_dbg, [0.766229] [drm:intel_detect_pch] Found LynxPoint PCH [0.766320] [drm:i915_gem_init_stolen] Memory reserved for graphics device: 32768K, usable: 31744K [0.766321] [drm] Memory usable by graphics device = 2048M [0.766398] [drm:i915_gem_gtt_init] GMADR size = 256M [0.766399] [drm:i915_gem_gtt_init] GTT stolen size = 32M [0.766399] [drm:i915_gem_gtt_init] ppgtt mode: 1 [0.766400] [drm] Replacing VGA console driver [0.767158] [drm:intel_opregion_setup] graphic opregion physical addr: 0xd9509018 [0.767161] [drm:intel_opregion_setup] Public ACPI methods supported [0.767162] [drm:intel_opregion_setup] SWSCI supported [0.772643] [drm:swsci_setup] SWSCI GBDA callbacks 0cb3, SBCB callbacks 00300483 [0.772646] [drm:intel_opregion_setup] ASLE supported [0.772646] [drm:intel_opregion_setup] ASLE extension supported [0.772648] [drm:intel_opregion_setup] Found valid VBT in ACPI OpRegion (Mailbox #4) [0.772717] [drm:intel_device_info_runtime_init] slice total: 0 [0.772717] [drm:intel_device_info_runtime_init] subslice total: 0 [0.772718] [drm:intel_device_info_runtime_init] subslice per slice: 0 [0.772719] [drm:intel_device_info_runtime_init] EU total: 0 [0.772720] [drm:intel_device_info_runtime_init] EU per subslice: 0 [0.772720] [drm:intel_device_info_runtime_init] has slice power gating: n [0.772721] [drm:intel_device_info_runtime_init] has subslice power gating: n [0.772722] [drm:intel_device_info_runtime_init] has EU power gating: n [0.772722] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [0.772725] [drm] Driver supports precise vblank timestamp query. [0.772727] [drm:init_vbt_defaults] Set default to SSC at 12 kHz [0.772728] [drm:intel_bios_init] VBT signature "$VBT HASWELL", BDB version 170 [0.772730] [drm:parse_general_features] BDB_GENERAL_FEATURES int_tv_support 0 int_crt_support 1 lvds_use_ssc 0 lvds_ssc_freq 12 display_clock_mode 0 fdi_rx_polarity_inverted 0 [0.772731] [drm:parse_general_definitions] crt_ddc_bus_pin: 2 [0.772732] [drm:parse_lfp_panel_data] DRRS supported mode is static [0.772734] [drm:parse_lfp_panel_data] Found panel mode in BIOS VBT tables: [0.772735] [drm:drm_mode_debug_printmodeline] Modeline 0:"1024x768" 0 65000 1024 1048 1184 1344 768 771 777 806 0x8 0xa [0.772736] [drm:parse_lfp_panel_data] VBT initial LVDS value 300 [0.772738] [drm:parse_lfp_backlight] VBT backlight PWM modulation frequency 200 Hz, active high, min brightness 0, level 255 [0.772739] [drm:parse_sdvo_panel_data] Found SDVO panel mode in BIOS VBT tables: [0.772740] [drm:drm_mode_debug_printmodeline] Modeline 0:"1600x1200" 0 162000 1600 1664 1856 2160 1200 1201 1204 1250 0x8 0xa [0.772741] [drm:parse_sdvo_device_mapping] No SDVO device info is found in VBT [0.772742] [drm:parse_driver_features] DRRS State Enabled:1 [0.772743] [drm:parse_ddi_port] Port B VBT info: DP:1 HDMI:1 DVI:1 EDP:0 CRT:0 [0.772745] [drm:parse_ddi_port] VBT HDMI level shift for port B: 6 [0.772745] [drm:parse_ddi_port] Port C VBT info: DP:0 HDMI:1 DVI:1 EDP:0 CRT:0 [0.772746] [drm:parse_ddi_port] VBT HDMI level shift for port C: 6 [0.772747] [drm:parse_ddi_port] Port D VBT info: DP:1 HDMI:1 DVI:1 EDP:0 CRT:0 [0.77274
Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On Apr 30 Stefan Richter wrote: > On Apr 29 Stefan Richter wrote: > > On Apr 26 Stefan Richter wrote: > > > v4.6-rc solidly hangs after a short while after boot, login to X11, and > > > doing nothing much remarkable on the just brought up X desktop. > > > > > > Hardware: x86-64, E3-1245 v3 (Haswell), > > > mainboard Supermicro X10SAE, > > > using integrated Intel graphics (HD P4600, i915 driver), > > > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > > > Intel LAN (i217, igb driver), > > > several IEEE 1394 controllers, some of them behind > > > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > > > and one PCI-to-CardBus bridge (Ricoh) > > > > > > kernel.org kernel, Gentoo Linux userland > > > > > > 1. known good: v4.5-rc5 (gcc 4.9.3) > > >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > > > > > 2. known good: v4.5.2 (gcc 5.2.0) > > >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > > > > > I will send my linux-4.6-rc5/.config in a follow-up message. > > .config: http://www.spinics.net/lists/kernel/msg2243444.html >lspci: http://www.spinics.net/lists/kernel/msg2243447.html > > Some userland package versions, in case these have any bearing: > x11-base/xorg-drivers-1.17 > x11-base/xorg-server-1.17.4 > x11-bas/xorg-x11-7.4-r2 Furthermore, there is a single display hooked up via DisplayPort. > > After it proved impossible to capture an oops through netconsole, I > > started git bisect. This will apparently take almost a week, as git > > estimated 13 bisection steps and I will be allowing about 12 hours of > > uptime as a sign for a good kernel. (In my four or five tests of bad > > kernels before I started bisection, they hung after 3 minutes...5.5 hours > > uptime, with no discernible difference in workload. Maybe 12 h cutoff is > > even too short...) I took at least 18 hours uptime (usually 24 hours) as a sign for good kernels. During the bisection, bad kernels hung after 3 h, 2 h, 9 min, 45 min, and 4 min uptime. Thus I arrived at a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW" as the point where the hangs are introduced. Quoting the changelog of the commit: Oh, and in case you - the person reading this commit message - found this commit through git bisect, please do the following: - Check your dmesg and see if there are error messages mentioning underruns around the time your problem started happening. Well, I always had the followings lines in dmesg: [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* uncleared fifo underrun on pipe A [drm:intel_cpu_fifo_underrun_irq_handler] *ERROR* CPU pipe A FIFO underrun I always got these when I switch on the DisplayPort attached monitor. Recently I changed userland from kdm to sddm and noticed that I apparently get these when sddm shuts down. I am not aware of whether or not this also already happened with kdm. However, "around the time your problem started happening" there is nothing in dmesg, because "your problem" is a complete hang without possibility of disk IO and without netconsole output. - Download intel-gpu-tools, compile it, and run: $ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 | tee fbc.txt Then send us the fbc.txt file, especially if you get a failure. This will really maximize your chances of getting the bug fixed quickly. Do you need this while FBC is enabled, or can I run it while FBC is disabled? - Try to find a reliable way to reproduce the problem, and tell us. The reliable way is to just wait for the kernel to hang after about 3 minutes to 5.5 hours. I have not identified any special activity which would trigger the hang. - Boot with drm.debug=0xe, reproduce the problem, then send us the dmesg file. I can try this, but I am skeptical about getting any useful kernel messages from before the hang. PS: I am mentioning the following just in case that it has any relationship with the FBC related kernel freezes. Maybe it doesn't... There is another recent regression on this PC, but I have not yet figured out whether it was introduced by any particular kernel version. The regression is: When switching from X11 to text console by [Ctrl][Alt][Fx] or by shutting down sddm, I often only get a blank screen. I suspect that this regression was introduced when I replaced kdm by sddm, but I am not sure about that. -- Stefan Richter -==- -=-= --=-= http://arcgraph.de/sr/
Regression of v4.6-rc vs. v4.5 bisected: a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW"
On Apr 30 Stefan Richter wrote: > On Apr 29 Stefan Richter wrote: > > On Apr 26 Stefan Richter wrote: > > > v4.6-rc solidly hangs after a short while after boot, login to X11, and > > > doing nothing much remarkable on the just brought up X desktop. > > > > > > Hardware: x86-64, E3-1245 v3 (Haswell), > > > mainboard Supermicro X10SAE, > > > using integrated Intel graphics (HD P4600, i915 driver), > > > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > > > Intel LAN (i217, igb driver), > > > several IEEE 1394 controllers, some of them behind > > > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > > > and one PCI-to-CardBus bridge (Ricoh) > > > > > > kernel.org kernel, Gentoo Linux userland > > > > > > 1. known good: v4.5-rc5 (gcc 4.9.3) > > >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > > > > > 2. known good: v4.5.2 (gcc 5.2.0) > > >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > > > > > I will send my linux-4.6-rc5/.config in a follow-up message. > > .config: http://www.spinics.net/lists/kernel/msg2243444.html >lspci: http://www.spinics.net/lists/kernel/msg2243447.html > > Some userland package versions, in case these have any bearing: > x11-base/xorg-drivers-1.17 > x11-base/xorg-server-1.17.4 > x11-bas/xorg-x11-7.4-r2 Furthermore, there is a single display hooked up via DisplayPort. > > After it proved impossible to capture an oops through netconsole, I > > started git bisect. This will apparently take almost a week, as git > > estimated 13 bisection steps and I will be allowing about 12 hours of > > uptime as a sign for a good kernel. (In my four or five tests of bad > > kernels before I started bisection, they hung after 3 minutes...5.5 hours > > uptime, with no discernible difference in workload. Maybe 12 h cutoff is > > even too short...) I took at least 18 hours uptime (usually 24 hours) as a sign for good kernels. During the bisection, bad kernels hung after 3 h, 2 h, 9 min, 45 min, and 4 min uptime. Thus I arrived at a98ee79317b4 "drm/i915/fbc: enable FBC by default on HSW and BDW" as the point where the hangs are introduced. Quoting the changelog of the commit: Oh, and in case you - the person reading this commit message - found this commit through git bisect, please do the following: - Check your dmesg and see if there are error messages mentioning underruns around the time your problem started happening. Well, I always had the followings lines in dmesg: [drm:intel_set_cpu_fifo_underrun_reporting] *ERROR* uncleared fifo underrun on pipe A [drm:intel_cpu_fifo_underrun_irq_handler] *ERROR* CPU pipe A FIFO underrun I always got these when I switch on the DisplayPort attached monitor. Recently I changed userland from kdm to sddm and noticed that I apparently get these when sddm shuts down. I am not aware of whether or not this also already happened with kdm. However, "around the time your problem started happening" there is nothing in dmesg, because "your problem" is a complete hang without possibility of disk IO and without netconsole output. - Download intel-gpu-tools, compile it, and run: $ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 | tee fbc.txt Then send us the fbc.txt file, especially if you get a failure. This will really maximize your chances of getting the bug fixed quickly. Do you need this while FBC is enabled, or can I run it while FBC is disabled? - Try to find a reliable way to reproduce the problem, and tell us. The reliable way is to just wait for the kernel to hang after about 3 minutes to 5.5 hours. I have not identified any special activity which would trigger the hang. - Boot with drm.debug=0xe, reproduce the problem, then send us the dmesg file. I can try this, but I am skeptical about getting any useful kernel messages from before the hang. PS: I am mentioning the following just in case that it has any relationship with the FBC related kernel freezes. Maybe it doesn't... There is another recent regression on this PC, but I have not yet figured out whether it was introduced by any particular kernel version. The regression is: When switching from X11 to text console by [Ctrl][Alt][Fx] or by shutting down sddm, I often only get a blank screen. I suspect that this regression was introduced when I replaced kdm by sddm, but I am not sure about that. -- Stefan Richter -==- -=-= --=-= http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5: Merge tag 'drm-intel-next-2016-02-29'
On Apr 29 Stefan Richter wrote: > On Apr 26 Stefan Richter wrote: > > v4.6-rc solidly hangs after a short while after boot, login to X11, and > > doing nothing much remarkable on the just brought up X desktop. > > > > Hardware: x86-64, E3-1245 v3 (Haswell), > > mainboard Supermicro X10SAE, > > using integrated Intel graphics (HD P4600, i915 driver), > > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > > Intel LAN (i217, igb driver), > > several IEEE 1394 controllers, some of them behind > > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > > and one PCI-to-CardBus bridge (Ricoh) > > > > kernel.org kernel, Gentoo Linux userland > > > > 1. known good: v4.5-rc5 (gcc 4.9.3) > >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > > > 2. known good: v4.5.2 (gcc 5.2.0) > >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > > > I will send my linux-4.6-rc5/.config in a follow-up message. .config: http://www.spinics.net/lists/kernel/msg2243444.html lspci: http://www.spinics.net/lists/kernel/msg2243447.html Some userland package versions, in case these have any bearing: x11-base/xorg-drivers-1.17 x11-base/xorg-server-1.17.4 x11-bas/xorg-x11-7.4-r2 > After it proved impossible to capture an oops through netconsole, I > started git bisect. This will apparently take almost a week, as git > estimated 13 bisection steps and I will be allowing about 12 hours of > uptime as a sign for a good kernel. (In my four or five tests of bad > kernels before I started bisection, they hung after 3 minutes...5.5 hours > uptime, with no discernible difference in workload. Maybe 12 h cutoff is > even too short...) There are about 9 more bisection steps left to go. The first few steps sent me straight into DRM land. My current "git bisect log" with own annotations: git bisect start # bad: [9735a22799b9214d17d3c231fe377fc852f042e9] Linux 4.6-rc2 git bisect bad 9735a22799b9214d17d3c231fe377fc852f042e9 # good: [b562e44f507e863c6792946e4e1b1449fbbac85d] Linux 4.5 git bisect good b562e44f507e863c6792946e4e1b1449fbbac85d # good: [6b5f04b6cf8ebab9a65d9c0026c650bb2538fd0f] Merge branch 'for-4.6' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup # ++ still good after 18 h uptime git bisect good 6b5f04b6cf8ebab9a65d9c0026c650bb2538fd0f # good: [2c856e14dad8cb1b085ae1f30c5e125c6d46019b] Merge tag 'arm64-perf' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux # ++ still good after 24 h uptime git bisect good 2c856e14dad8cb1b085ae1f30c5e125c6d46019b # bad: [8bb7e27bbb9d0db7ca0e83d40810fb752381cdd5] staging: delete STE RMI4 hackish driver # -- hung after 3 h uptime git bisect bad 8bb7e27bbb9d0db7ca0e83d40810fb752381cdd5 # bad: [507d44a9e1bb01661c75b88fd866d2461ab41c9c] Merge tag 'drm-intel-next-2016-02-29' of git://anongit.freedesktop.org/drm-intel into drm-next # -- hung after 2 h uptime git bisect bad 507d44a9e1bb01661c75b88fd866d2461ab41c9c -- Stefan Richter -==- -=-- - http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5: Merge tag 'drm-intel-next-2016-02-29'
On Apr 29 Stefan Richter wrote: > On Apr 26 Stefan Richter wrote: > > v4.6-rc solidly hangs after a short while after boot, login to X11, and > > doing nothing much remarkable on the just brought up X desktop. > > > > Hardware: x86-64, E3-1245 v3 (Haswell), > > mainboard Supermicro X10SAE, > > using integrated Intel graphics (HD P4600, i915 driver), > > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > > Intel LAN (i217, igb driver), > > several IEEE 1394 controllers, some of them behind > > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > > and one PCI-to-CardBus bridge (Ricoh) > > > > kernel.org kernel, Gentoo Linux userland > > > > 1. known good: v4.5-rc5 (gcc 4.9.3) > >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > > > 2. known good: v4.5.2 (gcc 5.2.0) > >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > > > I will send my linux-4.6-rc5/.config in a follow-up message. .config: http://www.spinics.net/lists/kernel/msg2243444.html lspci: http://www.spinics.net/lists/kernel/msg2243447.html Some userland package versions, in case these have any bearing: x11-base/xorg-drivers-1.17 x11-base/xorg-server-1.17.4 x11-bas/xorg-x11-7.4-r2 > After it proved impossible to capture an oops through netconsole, I > started git bisect. This will apparently take almost a week, as git > estimated 13 bisection steps and I will be allowing about 12 hours of > uptime as a sign for a good kernel. (In my four or five tests of bad > kernels before I started bisection, they hung after 3 minutes...5.5 hours > uptime, with no discernible difference in workload. Maybe 12 h cutoff is > even too short...) There are about 9 more bisection steps left to go. The first few steps sent me straight into DRM land. My current "git bisect log" with own annotations: git bisect start # bad: [9735a22799b9214d17d3c231fe377fc852f042e9] Linux 4.6-rc2 git bisect bad 9735a22799b9214d17d3c231fe377fc852f042e9 # good: [b562e44f507e863c6792946e4e1b1449fbbac85d] Linux 4.5 git bisect good b562e44f507e863c6792946e4e1b1449fbbac85d # good: [6b5f04b6cf8ebab9a65d9c0026c650bb2538fd0f] Merge branch 'for-4.6' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup # ++ still good after 18 h uptime git bisect good 6b5f04b6cf8ebab9a65d9c0026c650bb2538fd0f # good: [2c856e14dad8cb1b085ae1f30c5e125c6d46019b] Merge tag 'arm64-perf' of git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux # ++ still good after 24 h uptime git bisect good 2c856e14dad8cb1b085ae1f30c5e125c6d46019b # bad: [8bb7e27bbb9d0db7ca0e83d40810fb752381cdd5] staging: delete STE RMI4 hackish driver # -- hung after 3 h uptime git bisect bad 8bb7e27bbb9d0db7ca0e83d40810fb752381cdd5 # bad: [507d44a9e1bb01661c75b88fd866d2461ab41c9c] Merge tag 'drm-intel-next-2016-02-29' of git://anongit.freedesktop.org/drm-intel into drm-next # -- hung after 2 h uptime git bisect bad 507d44a9e1bb01661c75b88fd866d2461ab41c9c -- Stefan Richter -==- -=-- - http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 26 Stefan Richter wrote: > v4.6-rc solidly hangs after a short while after boot, login to X11, and > doing nothing much remarkable on the just brought up X desktop. > > Hardware: x86-64, E3-1245 v3 (Haswell), > mainboard Supermicro X10SAE, > using integrated Intel graphics (HD P4600, i915 driver), > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > Intel LAN (i217, igb driver), > several IEEE 1394 controllers, some of them behind > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > and one PCI-to-CardBus bridge (Ricoh) > > kernel.org kernel, Gentoo Linux userland > > 1. known good: v4.5-rc5 (gcc 4.9.3) >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > 2. known good: v4.5.2 (gcc 5.2.0) >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > I will send my linux-4.6-rc5/.config in a follow-up message. After it proved impossible to capture an oops through netconsole, I started git bisect. This will apparently take almost a week, as git estimated 13 bisection steps and I will be allowing about 12 hours of uptime as a sign for a good kernel. (In my four or five tests of bad kernels before I started bisection, they hung after 3 minutes...5.5 hours uptime, with no discernible difference in workload. Maybe 12 h cutoff is even too short...) -- Stefan Richter -==- -=-- ===-= http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 26 Stefan Richter wrote: > v4.6-rc solidly hangs after a short while after boot, login to X11, and > doing nothing much remarkable on the just brought up X desktop. > > Hardware: x86-64, E3-1245 v3 (Haswell), > mainboard Supermicro X10SAE, > using integrated Intel graphics (HD P4600, i915 driver), > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > Intel LAN (i217, igb driver), > several IEEE 1394 controllers, some of them behind > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > and one PCI-to-CardBus bridge (Ricoh) > > kernel.org kernel, Gentoo Linux userland > > 1. known good: v4.5-rc5 (gcc 4.9.3) >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > 2. known good: v4.5.2 (gcc 5.2.0) >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > I will send my linux-4.6-rc5/.config in a follow-up message. After it proved impossible to capture an oops through netconsole, I started git bisect. This will apparently take almost a week, as git estimated 13 bisection steps and I will be allowing about 12 hours of uptime as a sign for a good kernel. (In my four or five tests of bad kernels before I started bisection, they hung after 3 minutes...5.5 hours uptime, with no discernible difference in workload. Maybe 12 h cutoff is even too short...) -- Stefan Richter -==- -=-- ===-= http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 27 Stefan Richter wrote: > On Apr 27 Stefan Richter wrote: > > Today I booted a 2nd time into v4.6-rc5, and loaded netconsole shortly > > after boot and xdm login to try capturing an oops. But throughout 5 hours > > uptime now, the hang was not reproduced. > > ...and 20 minutes after this post went out, the PC hang. > There was nothing logged over netconsole, alas. One more hang, now after 12 minutes uptime. Again no netconsole output. For the time being I can't investigate further. -- Stefan Richter -==- -=-- ==-== http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 27 Stefan Richter wrote: > On Apr 27 Stefan Richter wrote: > > Today I booted a 2nd time into v4.6-rc5, and loaded netconsole shortly > > after boot and xdm login to try capturing an oops. But throughout 5 hours > > uptime now, the hang was not reproduced. > > ...and 20 minutes after this post went out, the PC hang. > There was nothing logged over netconsole, alas. One more hang, now after 12 minutes uptime. Again no netconsole output. For the time being I can't investigate further. -- Stefan Richter -==- -=-- ==-== http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 27 Stefan Richter wrote: > Today I booted a 2nd time into v4.6-rc5, and loaded netconsole shortly > after boot and xdm login to try capturing an oops. But throughout 5 hours > uptime now, the hang was not reproduced. ...and 20 minutes after this post went out, the PC hang. There was nothing logged over netconsole, alas. -- Stefan Richter -==- -=-- ==-== http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 27 Stefan Richter wrote: > Today I booted a 2nd time into v4.6-rc5, and loaded netconsole shortly > after boot and xdm login to try capturing an oops. But throughout 5 hours > uptime now, the hang was not reproduced. ...and 20 minutes after this post went out, the PC hang. There was nothing logged over netconsole, alas. -- Stefan Richter -==- -=-- ==-== http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 26 Stefan Richter wrote: > v4.6-rc solidly hangs after a short while after boot, login to X11, and > doing nothing much remarkable on the just brought up X desktop. > > Hardware: x86-64, E3-1245 v3 (Haswell), > mainboard Supermicro X10SAE, > using integrated Intel graphics (HD P4600, i915 driver), > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > Intel LAN (i217, igb driver), > several IEEE 1394 controllers, some of them behind > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > and one PCI-to-CardBus bridge (Ricoh) > > kernel.org kernel, Gentoo Linux userland > > 1. known good: v4.5-rc5 (gcc 4.9.3) >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > 2. known good: v4.5.2 (gcc 5.2.0) >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > I will send my linux-4.6-rc5/.config in a follow-up message. > > In theory I could collect more info (simplify the hardware, run > netconsole, bisect). In practice I cannot do so for the the time being > due to lack of spare time. That's also the reason why I did not already > send a report when I tested v4.6-rc2, and why I did not boot v4.6-rc[25] > more than once yet. Today I booted a 2nd time into v4.6-rc5, and loaded netconsole shortly after boot and xdm login to try capturing an oops. But throughout 5 hours uptime now, the hang was not reproduced. -- Stefan Richter -==- -=-- ==-== http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 26 Stefan Richter wrote: > v4.6-rc solidly hangs after a short while after boot, login to X11, and > doing nothing much remarkable on the just brought up X desktop. > > Hardware: x86-64, E3-1245 v3 (Haswell), > mainboard Supermicro X10SAE, > using integrated Intel graphics (HD P4600, i915 driver), > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > Intel LAN (i217, igb driver), > several IEEE 1394 controllers, some of them behind > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > and one PCI-to-CardBus bridge (Ricoh) > > kernel.org kernel, Gentoo Linux userland > > 1. known good: v4.5-rc5 (gcc 4.9.3) >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > 2. known good: v4.5.2 (gcc 5.2.0) >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > I will send my linux-4.6-rc5/.config in a follow-up message. > > In theory I could collect more info (simplify the hardware, run > netconsole, bisect). In practice I cannot do so for the the time being > due to lack of spare time. That's also the reason why I did not already > send a report when I tested v4.6-rc2, and why I did not boot v4.6-rc[25] > more than once yet. Today I booted a 2nd time into v4.6-rc5, and loaded netconsole shortly after boot and xdm login to try capturing an oops. But throughout 5 hours uptime now, the hang was not reproduced. -- Stefan Richter -==- -=-- ==-== http://arcgraph.de/sr/
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 26 Stefan Richter wrote: > v4.6-rc solidly hangs after a short while after boot, login to X11, and > doing nothing much remarkable on the just brought up X desktop. > > Hardware: x86-64, E3-1245 v3 (Haswell), > mainboard Supermicro X10SAE, > using integrated Intel graphics (HD P4600, i915 driver), > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > Intel LAN (i217, igb driver), > several IEEE 1394 controllers, some of them behind > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > and one PCI-to-CardBus bridge (Ricoh) > > kernel.org kernel, Gentoo Linux userland > > 1. known good: v4.5-rc5 (gcc 4.9.3) >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > 2. known good: v4.5.2 (gcc 5.2.0) >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > I will send my linux-4.6-rc5/.config in a follow-up message. > > In theory I could collect more info (simplify the hardware, run > netconsole, bisect). In practice I cannot do so for the the time being > due to lack of spare time. That's also the reason why I did not already > send a report when I tested v4.6-rc2, and why I did not boot v4.6-rc[25] > more than once yet. Attached: lspci -vvnn, obtained while on the good v4.5.2 -- Stefan Richter -==- -=-- ==-=- http://arcgraph.de/sr/ 00:00.0 Host bridge [0600]: Intel Corporation Xeon E3-1200 v3 Processor DRAM Controller [8086:0c08] (rev 06) Subsystem: Super Micro Computer Inc Xeon E3-1200 v3 Processor DRAM Controller [15d9:0805] Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR- 00:01.0 PCI bridge [0604]: Intel Corporation Xeon E3-1200 v3/4th Gen Core Processor PCI Express x16 Controller [8086:0c01] (rev 06) (prog-if 00 [Normal decode]) Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- TAbort- Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- Capabilities: [88] Subsystem: Intel Corporation Xeon E3-1200 v3/4th Gen Core Processor PCI Express x16 Controller [8086:2010] Capabilities: [80] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit- Address: feeff00c Data: 4191 Capabilities: [a0] Express (v2) Root Port (Slot+), MSI 00 DevCap: MaxPayload 256 bytes, PhantFunc 0 ExtTag- RBE+ DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend- LnkCap: Port #2, Speed 8GT/s, Width x8, ASPM L0s L1, Exit Latency L0s <1us, L1 <8us ClockPM- Surprise- LLActRep- BwNot+ ASPMOptComp+ LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk- ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt+ ABWMgmt- SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise- Slot #1, PowerLimit 75.000W; Interlock- NoCompl+ SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock- SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet+ Interlock- Changed: MRL- PresDet- LinkState- RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna+ CRSVisible- RootCap: CRSVisible- RootSta: PME ReqID , PMEStatus- PMEPending- DevCap2: Completion Timeout: Not Supported, TimeoutDis-, LTR+, OBFF Via WAKE# ARIFwd- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR+, OBFF Via WAKE# ARIFwd- LnkCtl2: Target Link Speed: 8GT/s, EnterCompliance- SpeedDis- Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1- EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest- Capabilities: [100 v1] Vi
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 26 Stefan Richter wrote: > v4.6-rc solidly hangs after a short while after boot, login to X11, and > doing nothing much remarkable on the just brought up X desktop. > > Hardware: x86-64, E3-1245 v3 (Haswell), > mainboard Supermicro X10SAE, > using integrated Intel graphics (HD P4600, i915 driver), > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > Intel LAN (i217, igb driver), > several IEEE 1394 controllers, some of them behind > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > and one PCI-to-CardBus bridge (Ricoh) > > kernel.org kernel, Gentoo Linux userland > > 1. known good: v4.5-rc5 (gcc 4.9.3) >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > 2. known good: v4.5.2 (gcc 5.2.0) >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > I will send my linux-4.6-rc5/.config in a follow-up message. > > In theory I could collect more info (simplify the hardware, run > netconsole, bisect). In practice I cannot do so for the the time being > due to lack of spare time. That's also the reason why I did not already > send a report when I tested v4.6-rc2, and why I did not boot v4.6-rc[25] > more than once yet. Attached: lspci -vvnn, obtained while on the good v4.5.2 -- Stefan Richter -==- -=-- ==-=- http://arcgraph.de/sr/ 00:00.0 Host bridge [0600]: Intel Corporation Xeon E3-1200 v3 Processor DRAM Controller [8086:0c08] (rev 06) Subsystem: Super Micro Computer Inc Xeon E3-1200 v3 Processor DRAM Controller [15d9:0805] Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR- 00:01.0 PCI bridge [0604]: Intel Corporation Xeon E3-1200 v3/4th Gen Core Processor PCI Express x16 Controller [8086:0c01] (rev 06) (prog-if 00 [Normal decode]) Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- TAbort- Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- Capabilities: [88] Subsystem: Intel Corporation Xeon E3-1200 v3/4th Gen Core Processor PCI Express x16 Controller [8086:2010] Capabilities: [80] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit- Address: feeff00c Data: 4191 Capabilities: [a0] Express (v2) Root Port (Slot+), MSI 00 DevCap: MaxPayload 256 bytes, PhantFunc 0 ExtTag- RBE+ DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend- LnkCap: Port #2, Speed 8GT/s, Width x8, ASPM L0s L1, Exit Latency L0s <1us, L1 <8us ClockPM- Surprise- LLActRep- BwNot+ ASPMOptComp+ LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk- ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- BWMgmt+ ABWMgmt- SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise- Slot #1, PowerLimit 75.000W; Interlock- NoCompl+ SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock- SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet+ Interlock- Changed: MRL- PresDet- LinkState- RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna+ CRSVisible- RootCap: CRSVisible- RootSta: PME ReqID , PMEStatus- PMEPending- DevCap2: Completion Timeout: Not Supported, TimeoutDis-, LTR+, OBFF Via WAKE# ARIFwd- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR+, OBFF Via WAKE# ARIFwd- LnkCtl2: Target Link Speed: 8GT/s, EnterCompliance- SpeedDis- Transmit Margin: Normal Operating Range, EnterModifiedCompliance- ComplianceSOS- Compliance De-emphasis: -6dB LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1- EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest- Capabilities: [100 v1] Vi
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 26 Stefan Richter wrote: > Hardware: x86-64, E3-1245 v3 (Haswell), > mainboard Supermicro X10SAE, > using integrated Intel graphics (HD P4600, i915 driver), > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > Intel LAN (i217, igb driver), > several IEEE 1394 controllers, some of them behind > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > and one PCI-to-CardBus bridge (Ricoh) > > kernel.org kernel, Gentoo Linux userland > > 1. known good: v4.5-rc5 (gcc 4.9.3) >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > 2. known good: v4.5.2 (gcc 5.2.0) >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > I will send my linux-4.6-rc5/.config in a follow-up message. > > In theory I could collect more info (simplify the hardware, run > netconsole, bisect). In practice I cannot do so for the the time being > due to lack of spare time. That's also the reason why I did not already > send a report when I tested v4.6-rc2, and why I did not boot v4.6-rc[25] > more than once yet. Attached: linux-4.6-rc5/.config -- Stefan Richter -==- -=-- ==-=- http://arcgraph.de/sr/ # # Automatically generated file; DO NOT EDIT. # Linux/x86 4.6.0-rc5 Kernel Configuration # CONFIG_64BIT=y CONFIG_X86_64=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_PERF_EVENTS_INTEL_UNCORE=y CONFIG_OUTPUT_FORMAT="elf64-x86-64" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig" CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_MMU=y CONFIG_ARCH_MMAP_RND_BITS_MIN=28 CONFIG_ARCH_MMAP_RND_BITS_MAX=32 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16 CONFIG_NEED_DMA_MAP_STATE=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_BUG=y CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y CONFIG_GENERIC_HWEIGHT=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_ARCH_HAS_CPU_RELAX=y CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y CONFIG_HAVE_SETUP_PER_CPU_AREA=y CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y CONFIG_ARCH_HIBERNATION_POSSIBLE=y CONFIG_ARCH_SUSPEND_POSSIBLE=y CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y CONFIG_ARCH_WANT_GENERAL_HUGETLB=y CONFIG_ZONE_DMA32=y CONFIG_AUDIT_ARCH=y CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y CONFIG_X86_64_SMP=y CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx -fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 -fcall-saved-r11" CONFIG_ARCH_SUPPORTS_UPROBES=y CONFIG_FIX_EARLYCON_MEM=y CONFIG_DEBUG_RODATA=y CONFIG_PGTABLE_LEVELS=4 CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config" CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y # # General setup # CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_CROSS_COMPILE="" # CONFIG_COMPILE_TEST is not set CONFIG_LOCALVERSION="" # CONFIG_LOCALVERSION_AUTO is not set CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y CONFIG_KERNEL_GZIP=y # CONFIG_KERNEL_BZIP2 is not set # CONFIG_KERNEL_LZMA is not set # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set CONFIG_DEFAULT_HOSTNAME="(none)" CONFIG_SWAP=y CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y CONFIG_POSIX_MQUEUE=y CONFIG_POSIX_MQUEUE_SYSCTL=y CONFIG_CROSS_MEMORY_ATTACH=y CONFIG_FHANDLE=y # CONFIG_USELIB is not set # CONFIG_AUDIT is not set CONFIG_HAVE_ARCH_AUDITSYSCALL=y # # IRQ subsystem # CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_GENERIC_PENDING_IRQ=y CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_DOMAIN_HIERARCHY=y CONFIG_GENERIC_MSI_IRQ=y CONFIG_GENERIC_MSI_IRQ_DOMAIN=y CONFIG_IRQ_FORCED_THREADING=y CONFIG_SPARSE_IRQ=y CONFIG_CLOCKSOURCE_WATCHDOG=y CONFIG_ARCH_CLOCKSOURCE_DATA=y CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y CONFIG_GENERIC_CMOS_UPDATE=y # # Timers subsystem # CONFIG_TICK_ONESHOT=y CONFIG_NO_HZ_COMMON=y # CONFIG_HZ_PERIODIC is not set CONFIG_NO_HZ_IDLE=y # CONFIG_NO_HZ_FULL is not set CONFIG_NO_HZ=y CONFIG_HIGH_RES_TIMERS=y # # CPU/Task time and stats accounting # CONFIG_TICK_CPU_ACCOUNTING=y # CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set # CONFIG_IRQ_TIME_ACCOUNTING is not set CONFIG_BSD_PROCESS_ACCT=y # CONFIG_BSD_PROCESS_ACCT_V3 is not set # CONFIG_TASKSTATS is not set # # RCU Subsystem # CONFIG_PREEMPT_RCU=y # CONFIG_RCU_EXPERT is not set CONFIG_SRCU=y # CONFIG_TASKS_RCU is not set CONFIG_RCU_STALL_COMMON=y # CONFIG_TREE_RCU_TRACE is not set # CONFIG_RCU_EXPEDITE_BOOT is not set # CONFIG_BUILD_BIN2C is not set # CONFIG_IKCONFIG is not set CONFIG_LOG_BUF_SHIFT=18 CONFI
Re: Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
On Apr 26 Stefan Richter wrote: > Hardware: x86-64, E3-1245 v3 (Haswell), > mainboard Supermicro X10SAE, > using integrated Intel graphics (HD P4600, i915 driver), > C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, > Intel LAN (i217, igb driver), > several IEEE 1394 controllers, some of them behind > PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) > and one PCI-to-CardBus bridge (Ricoh) > > kernel.org kernel, Gentoo Linux userland > > 1. known good: v4.5-rc5 (gcc 4.9.3) >known bad: v4.6-rc2 (gcc 4.9.3), only tried one time > > 2. known good: v4.5.2 (gcc 5.2.0) >known bad: v4.6-rc5 (gcc 5.2.0), only tried one time > > I will send my linux-4.6-rc5/.config in a follow-up message. > > In theory I could collect more info (simplify the hardware, run > netconsole, bisect). In practice I cannot do so for the the time being > due to lack of spare time. That's also the reason why I did not already > send a report when I tested v4.6-rc2, and why I did not boot v4.6-rc[25] > more than once yet. Attached: linux-4.6-rc5/.config -- Stefan Richter -==- -=-- ==-=- http://arcgraph.de/sr/ # # Automatically generated file; DO NOT EDIT. # Linux/x86 4.6.0-rc5 Kernel Configuration # CONFIG_64BIT=y CONFIG_X86_64=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_PERF_EVENTS_INTEL_UNCORE=y CONFIG_OUTPUT_FORMAT="elf64-x86-64" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig" CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_MMU=y CONFIG_ARCH_MMAP_RND_BITS_MIN=28 CONFIG_ARCH_MMAP_RND_BITS_MAX=32 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16 CONFIG_NEED_DMA_MAP_STATE=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_BUG=y CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y CONFIG_GENERIC_HWEIGHT=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_ARCH_HAS_CPU_RELAX=y CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y CONFIG_HAVE_SETUP_PER_CPU_AREA=y CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y CONFIG_ARCH_HIBERNATION_POSSIBLE=y CONFIG_ARCH_SUSPEND_POSSIBLE=y CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y CONFIG_ARCH_WANT_GENERAL_HUGETLB=y CONFIG_ZONE_DMA32=y CONFIG_AUDIT_ARCH=y CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y CONFIG_X86_64_SMP=y CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx -fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 -fcall-saved-r11" CONFIG_ARCH_SUPPORTS_UPROBES=y CONFIG_FIX_EARLYCON_MEM=y CONFIG_DEBUG_RODATA=y CONFIG_PGTABLE_LEVELS=4 CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config" CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y # # General setup # CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_CROSS_COMPILE="" # CONFIG_COMPILE_TEST is not set CONFIG_LOCALVERSION="" # CONFIG_LOCALVERSION_AUTO is not set CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y CONFIG_KERNEL_GZIP=y # CONFIG_KERNEL_BZIP2 is not set # CONFIG_KERNEL_LZMA is not set # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set CONFIG_DEFAULT_HOSTNAME="(none)" CONFIG_SWAP=y CONFIG_SYSVIPC=y CONFIG_SYSVIPC_SYSCTL=y CONFIG_POSIX_MQUEUE=y CONFIG_POSIX_MQUEUE_SYSCTL=y CONFIG_CROSS_MEMORY_ATTACH=y CONFIG_FHANDLE=y # CONFIG_USELIB is not set # CONFIG_AUDIT is not set CONFIG_HAVE_ARCH_AUDITSYSCALL=y # # IRQ subsystem # CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_GENERIC_PENDING_IRQ=y CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_DOMAIN_HIERARCHY=y CONFIG_GENERIC_MSI_IRQ=y CONFIG_GENERIC_MSI_IRQ_DOMAIN=y CONFIG_IRQ_FORCED_THREADING=y CONFIG_SPARSE_IRQ=y CONFIG_CLOCKSOURCE_WATCHDOG=y CONFIG_ARCH_CLOCKSOURCE_DATA=y CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y CONFIG_GENERIC_CMOS_UPDATE=y # # Timers subsystem # CONFIG_TICK_ONESHOT=y CONFIG_NO_HZ_COMMON=y # CONFIG_HZ_PERIODIC is not set CONFIG_NO_HZ_IDLE=y # CONFIG_NO_HZ_FULL is not set CONFIG_NO_HZ=y CONFIG_HIGH_RES_TIMERS=y # # CPU/Task time and stats accounting # CONFIG_TICK_CPU_ACCOUNTING=y # CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set # CONFIG_IRQ_TIME_ACCOUNTING is not set CONFIG_BSD_PROCESS_ACCT=y # CONFIG_BSD_PROCESS_ACCT_V3 is not set # CONFIG_TASKSTATS is not set # # RCU Subsystem # CONFIG_PREEMPT_RCU=y # CONFIG_RCU_EXPERT is not set CONFIG_SRCU=y # CONFIG_TASKS_RCU is not set CONFIG_RCU_STALL_COMMON=y # CONFIG_TREE_RCU_TRACE is not set # CONFIG_RCU_EXPEDITE_BOOT is not set # CONFIG_BUILD_BIN2C is not set # CONFIG_IKCONFIG is not set CONFIG_LOG_BUF_SHIFT=18 CONFI
Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
Hi, v4.6-rc solidly hangs after a short while after boot, login to X11, and doing nothing much remarkable on the just brought up X desktop. Hardware: x86-64, E3-1245 v3 (Haswell), mainboard Supermicro X10SAE, using integrated Intel graphics (HD P4600, i915 driver), C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, Intel LAN (i217, igb driver), several IEEE 1394 controllers, some of them behind PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) and one PCI-to-CardBus bridge (Ricoh) kernel.org kernel, Gentoo Linux userland 1. known good: v4.5-rc5 (gcc 4.9.3) known bad: v4.6-rc2 (gcc 4.9.3), only tried one time 2. known good: v4.5.2 (gcc 5.2.0) known bad: v4.6-rc5 (gcc 5.2.0), only tried one time I will send my linux-4.6-rc5/.config in a follow-up message. In theory I could collect more info (simplify the hardware, run netconsole, bisect). In practice I cannot do so for the the time being due to lack of spare time. That's also the reason why I did not already send a report when I tested v4.6-rc2, and why I did not boot v4.6-rc[25] more than once yet. -- Stefan Richter -==- -=-- ==-=- http://arcgraph.de/sr/
Regression of v4.6-rc vs. v4.5: hangs after a few minutes after boot
Hi, v4.6-rc solidly hangs after a short while after boot, login to X11, and doing nothing much remarkable on the just brought up X desktop. Hardware: x86-64, E3-1245 v3 (Haswell), mainboard Supermicro X10SAE, using integrated Intel graphics (HD P4600, i915 driver), C226 PCH's AHCI and USB 2/3, ASMedia ASM1062 AHCI, Intel LAN (i217, igb driver), several IEEE 1394 controllers, some of them behind PCIe bridges (IDT, PLX) or PCIe-to-PCI bridges (TI, Tundra) and one PCI-to-CardBus bridge (Ricoh) kernel.org kernel, Gentoo Linux userland 1. known good: v4.5-rc5 (gcc 4.9.3) known bad: v4.6-rc2 (gcc 4.9.3), only tried one time 2. known good: v4.5.2 (gcc 5.2.0) known bad: v4.6-rc5 (gcc 5.2.0), only tried one time I will send my linux-4.6-rc5/.config in a follow-up message. In theory I could collect more info (simplify the hardware, run netconsole, bisect). In practice I cannot do so for the the time being due to lack of spare time. That's also the reason why I did not already send a report when I tested v4.6-rc2, and why I did not boot v4.6-rc[25] more than once yet. -- Stefan Richter -==- -=-- ==-=- http://arcgraph.de/sr/
[git pull] another FireWire (IEEE 1394) update
Linus, please pull from the tag "firewire-update2" at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git firewire-update2 to receive the following IEEE 1394 subsystem patch: - Occurrences of timeval were supposed to be eliminated last round, now remove a last forgotten one. Tina Ruchandani (1): firewire: nosy: Replace timeval with timespec64 drivers/firewire/nosy.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) -- Stefan Richter -==- --== ==--= http://arcgraph.de/sr/
[git pull] another FireWire (IEEE 1394) update
Linus, please pull from the tag "firewire-update2" at git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git firewire-update2 to receive the following IEEE 1394 subsystem patch: - Occurrences of timeval were supposed to be eliminated last round, now remove a last forgotten one. Tina Ruchandani (1): firewire: nosy: Replace timeval with timespec64 drivers/firewire/nosy.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) -- Stefan Richter -==- --== ==--= http://arcgraph.de/sr/
Re: [PATCH] firewire: nosy: Replace timeval with timespec64
On Mar 21 Arnd Bergmann wrote: >>> On Sunday 20 March 2016 22:59:11 Tina Ruchandani wrote: >>>> 'struct timeval' uses a 32 bit field for its 'seconds' value which >>>> will overflow in year 2038 and beyond. This patch replaces the use >>>> of timeval in nosy.c with timespec64 which doesn't suffer from y2038 >>>> issue. The code is correct as is - since it is only using the >>>> microseconds portion of timeval. However, this patch does the >>>> replacement as part of a larger effort to remove all instances of >>>> 'struct timeval' from the kernel (that would help identify cases >>>> where the code is actually broken). >>>> >>>> Signed-off-by: Tina Ruchandani <ruchandani.t...@gmail.com> [...] > Reviewed-by: Arnd Bergmann <a...@arndb.de> Committed to linux1394.git. I will try to get this pulled upstream in the next few days. -- Stefan Richter -==- --== =-==- http://arcgraph.de/sr/
