Re: [PATCH RESENT v4] dell_rbu: make firmware payload memory uncachable
On Wed, 09 May 2018 17:17:45 +0200, Stuart Hayes wrote: > > > On 4/18/2018 12:46 AM, Takashi Iwai wrote: > > From: Stuart Hayes > > > > The dell_rbu driver takes firmware update payloads and puts them in memory > > so > > the system BIOS can find them after a reboot. This sometimes fails (though > > rarely), because the memory containing the payload is in the CPU cache but > > never gets written back to main memory before the system is rebooted (CPU > > cache contents are lost on reboot). > > > > With this patch, the payload memory will be changed to uncachable to ensure > > that the payload is actually in main memory before the system is rebooted. > > > > Signed-off-by: Stuart Hayes > > Reviewed-by: Takashi Iwai > > Signed-off-by: Takashi Iwai > > --- > > v2 Added include, removed extra parentheses > > v3 Corrected formatting and include line > > v4 Moved set_memory_uc() outside the while loop so that the memory is > >definitely allocated before it is set to uncachable > > > > Andrew, could you pick up this orphan one? Thanks! > > > > diff --git a/drivers/firmware/dell_rbu.c b/drivers/firmware/dell_rbu.c > > index 2f452f1f7c8a..53f27a6e2d76 100644 > > --- a/drivers/firmware/dell_rbu.c > > +++ b/drivers/firmware/dell_rbu.c > > @@ -45,6 +45,7 @@ > > #include > > #include > > #include > > +#include > > > > MODULE_AUTHOR("Abhay Salunke "); > > MODULE_DESCRIPTION("Driver for updating BIOS image on DELL systems"); > > @@ -181,6 +182,11 @@ static int create_packet(void *data, size_t length) > > packet_data_temp_buf = NULL; > > } > > } > > + /* > > +* set to uncachable or it may never get written back before reboot > > +*/ > > + set_memory_uc((unsigned long)packet_data_temp_buf, 1 << ordernum); > > + > > spin_lock(&rbu_data.lock); > > > > newpacket->data = packet_data_temp_buf; > > @@ -349,6 +355,8 @@ static void packet_empty_list(void) > > * to make sure there are no stale RBU packets left in memory > > */ > > memset(newpacket->data, 0, rbu_data.packetsize); > > + set_memory_wb((unsigned long)newpacket->data, > > + 1 << newpacket->ordernum); > > free_pages((unsigned long) newpacket->data, > > newpacket->ordernum); > > kfree(newpacket); > > > > Just a reminder... I don't think this has been picked up yet. Please let me > know if there's anything I could do. Another gentle reminder ping. Andrew, if it's not suitable to your tree, just let me know. If so, I'll either submit directly to Linus or create a new tree for the misc stuff. thanks, Takashi
Re: [PATCH RESENT v4] dell_rbu: make firmware payload memory uncachable
On 4/18/2018 12:46 AM, Takashi Iwai wrote: > From: Stuart Hayes > > The dell_rbu driver takes firmware update payloads and puts them in memory so > the system BIOS can find them after a reboot. This sometimes fails (though > rarely), because the memory containing the payload is in the CPU cache but > never gets written back to main memory before the system is rebooted (CPU > cache contents are lost on reboot). > > With this patch, the payload memory will be changed to uncachable to ensure > that the payload is actually in main memory before the system is rebooted. > > Signed-off-by: Stuart Hayes > Reviewed-by: Takashi Iwai > Signed-off-by: Takashi Iwai > --- > v2 Added include, removed extra parentheses > v3 Corrected formatting and include line > v4 Moved set_memory_uc() outside the while loop so that the memory is >definitely allocated before it is set to uncachable > > Andrew, could you pick up this orphan one? Thanks! > > diff --git a/drivers/firmware/dell_rbu.c b/drivers/firmware/dell_rbu.c > index 2f452f1f7c8a..53f27a6e2d76 100644 > --- a/drivers/firmware/dell_rbu.c > +++ b/drivers/firmware/dell_rbu.c > @@ -45,6 +45,7 @@ > #include > #include > #include > +#include > > MODULE_AUTHOR("Abhay Salunke "); > MODULE_DESCRIPTION("Driver for updating BIOS image on DELL systems"); > @@ -181,6 +182,11 @@ static int create_packet(void *data, size_t length) > packet_data_temp_buf = NULL; > } > } > + /* > + * set to uncachable or it may never get written back before reboot > + */ > + set_memory_uc((unsigned long)packet_data_temp_buf, 1 << ordernum); > + > spin_lock(&rbu_data.lock); > > newpacket->data = packet_data_temp_buf; > @@ -349,6 +355,8 @@ static void packet_empty_list(void) >* to make sure there are no stale RBU packets left in memory >*/ > memset(newpacket->data, 0, rbu_data.packetsize); > + set_memory_wb((unsigned long)newpacket->data, > + 1 << newpacket->ordernum); > free_pages((unsigned long) newpacket->data, > newpacket->ordernum); > kfree(newpacket); > Just a reminder... I don't think this has been picked up yet. Please let me know if there's anything I could do. Thanks! Stuart
[PATCH RESENT v4] dell_rbu: make firmware payload memory uncachable
From: Stuart Hayes The dell_rbu driver takes firmware update payloads and puts them in memory so the system BIOS can find them after a reboot. This sometimes fails (though rarely), because the memory containing the payload is in the CPU cache but never gets written back to main memory before the system is rebooted (CPU cache contents are lost on reboot). With this patch, the payload memory will be changed to uncachable to ensure that the payload is actually in main memory before the system is rebooted. Signed-off-by: Stuart Hayes Reviewed-by: Takashi Iwai Signed-off-by: Takashi Iwai --- v2 Added include, removed extra parentheses v3 Corrected formatting and include line v4 Moved set_memory_uc() outside the while loop so that the memory is definitely allocated before it is set to uncachable Andrew, could you pick up this orphan one? Thanks! diff --git a/drivers/firmware/dell_rbu.c b/drivers/firmware/dell_rbu.c index 2f452f1f7c8a..53f27a6e2d76 100644 --- a/drivers/firmware/dell_rbu.c +++ b/drivers/firmware/dell_rbu.c @@ -45,6 +45,7 @@ #include #include #include +#include MODULE_AUTHOR("Abhay Salunke "); MODULE_DESCRIPTION("Driver for updating BIOS image on DELL systems"); @@ -181,6 +182,11 @@ static int create_packet(void *data, size_t length) packet_data_temp_buf = NULL; } } + /* +* set to uncachable or it may never get written back before reboot +*/ + set_memory_uc((unsigned long)packet_data_temp_buf, 1 << ordernum); + spin_lock(&rbu_data.lock); newpacket->data = packet_data_temp_buf; @@ -349,6 +355,8 @@ static void packet_empty_list(void) * to make sure there are no stale RBU packets left in memory */ memset(newpacket->data, 0, rbu_data.packetsize); + set_memory_wb((unsigned long)newpacket->data, + 1 << newpacket->ordernum); free_pages((unsigned long) newpacket->data, newpacket->ordernum); kfree(newpacket);