Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 06 Apr 2018 15:23:30 +0200
Message-ID: <s5hpo3cjwod.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Stuart Hayes <stuart.w.hayes@gmail.com>
Cc:     linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v4] dell_rbu: make firmware payload memory uncachable
In-Reply-To: <eb0db565-5f8a-ab1e-2bf3-2def3ec3eddc@gmail.com>
References: <eb0db565-5f8a-ab1e-2bf3-2def3ec3eddc@gmail.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka)
 FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3
 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 06 Apr 2018 15:12:21 +0200,
Stuart Hayes wrote:
> 
> 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@gmail.com>

Looks good to me.

Reviewed-by: Takashi Iwai <tiwai@suse.de>

Andrew, could you pick this one?  The driver seems to have no
maintainer.


thanks,

Takashi

> ---
> 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
> 
> This driver has no maintainer.
> 
> 
> 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@dell.com>");
>  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);
>