Re: [PATCH RESENT v4] dell_rbu: make firmware payload memory uncachable

[Takashi Iwai]

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 <stuart.w.hayes@xxxxxxxxx>
> > 
> > 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 <stuart.w.hayes@xxxxxxxxx>
> > Reviewed-by: Takashi Iwai <tiwai@xxxxxxx>
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > ---
> > 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 <linux/moduleparam.h>
> >  #include <linux/firmware.h>
> >  #include <linux/dma-mapping.h>
> > +#include <asm/set_memory.h>
> >  
> >  MODULE_AUTHOR("Abhay Salunke <abhay_salunke@xxxxxxxx>");
> >  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