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

From: Takashi Iwai
Date: Thu May 17 2018 - 03:29:13 EST


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