diff mbox series

[7/8] efi/capsule-loader: use page addresses rather than struct page pointers

Message ID 20170405092317.27921-8-ard.biesheuvel@linaro.org
State Superseded
Headers show
Series efi: add support for non-standard capsule headers | expand

Commit Message

Ard Biesheuvel April 5, 2017, 9:23 a.m. UTC 
To give some leeway to code that handles non-standard capsule headers,
let's keep an array of page addresses rather than struct page pointers.

This gives special implementations of efi_capsule_setup_info() the
opportunity to mangle the payload a bit before it is presented to the
firmware, without putting any knowledge of the nature of such quirks
into the generic code.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 drivers/firmware/efi/capsule-loader.c | 12 ++++++++----
 drivers/firmware/efi/capsule.c        |  7 ++++---
 include/linux/efi.h                   |  4 ++--
 3 files changed, 14 insertions(+), 9 deletions(-)

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Matt Fleming April 18, 2017, 12:56 p.m. UTC | #1 
On Wed, 05 Apr, at 10:23:16AM, Ard Biesheuvel wrote:
> To give some leeway to code that handles non-standard capsule headers,

> let's keep an array of page addresses rather than struct page pointers.

> 

> This gives special implementations of efi_capsule_setup_info() the

> opportunity to mangle the payload a bit before it is presented to the

> firmware, without putting any knowledge of the nature of such quirks

> into the generic code.

> 

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  drivers/firmware/efi/capsule-loader.c | 12 ++++++++----

>  drivers/firmware/efi/capsule.c        |  7 ++++---

>  include/linux/efi.h                   |  4 ++--

>  3 files changed, 14 insertions(+), 9 deletions(-)

> 

> diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c

> index d68a1ecebbf3..22b2bb73176c 100644

> --- a/drivers/firmware/efi/capsule-loader.c

> +++ b/drivers/firmware/efi/capsule-loader.c

> @@ -20,6 +20,10 @@

>  

>  #define NO_FURTHER_WRITE_ACTION -1

>  

> +#ifndef phys_to_page

> +#define phys_to_page(x)		virt_to_page((unsigned long)__va(x))

> +#endif


Is this going to work with highmem pages, which presumably, is a
possibility for the 32-bit Quark?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel April 18, 2017, 1:01 p.m. UTC | #2 
On 18 April 2017 at 13:56, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Wed, 05 Apr, at 10:23:16AM, Ard Biesheuvel wrote:

>> To give some leeway to code that handles non-standard capsule headers,

>> let's keep an array of page addresses rather than struct page pointers.

>>

>> This gives special implementations of efi_capsule_setup_info() the

>> opportunity to mangle the payload a bit before it is presented to the

>> firmware, without putting any knowledge of the nature of such quirks

>> into the generic code.

>>

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> ---

>>  drivers/firmware/efi/capsule-loader.c | 12 ++++++++----

>>  drivers/firmware/efi/capsule.c        |  7 ++++---

>>  include/linux/efi.h                   |  4 ++--

>>  3 files changed, 14 insertions(+), 9 deletions(-)

>>

>> diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c

>> index d68a1ecebbf3..22b2bb73176c 100644

>> --- a/drivers/firmware/efi/capsule-loader.c

>> +++ b/drivers/firmware/efi/capsule-loader.c

>> @@ -20,6 +20,10 @@

>>

>>  #define NO_FURTHER_WRITE_ACTION -1

>>

>> +#ifndef phys_to_page

>> +#define phys_to_page(x)              virt_to_page((unsigned long)__va(x))

>> +#endif

>

> Is this going to work with highmem pages, which presumably, is a

> possibility for the 32-bit Quark?


Good point. Given that we don't really care about the virtual address
anyway, what is the best way to translate physical addresses to struct
page pointers on x86? i suppose pfn_to_page(pa >> PAGE_SHIFT) always
does the trick?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matt Fleming April 18, 2017, 1:42 p.m. UTC | #3 
On Tue, 18 Apr, at 02:01:21PM, Ard Biesheuvel wrote:
> On 18 April 2017 at 13:56, Matt Fleming <matt@codeblueprint.co.uk> wrote:

> > On Wed, 05 Apr, at 10:23:16AM, Ard Biesheuvel wrote:

> >> To give some leeway to code that handles non-standard capsule headers,

> >> let's keep an array of page addresses rather than struct page pointers.

> >>

> >> This gives special implementations of efi_capsule_setup_info() the

> >> opportunity to mangle the payload a bit before it is presented to the

> >> firmware, without putting any knowledge of the nature of such quirks

> >> into the generic code.

> >>

> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >> ---

> >>  drivers/firmware/efi/capsule-loader.c | 12 ++++++++----

> >>  drivers/firmware/efi/capsule.c        |  7 ++++---

> >>  include/linux/efi.h                   |  4 ++--

> >>  3 files changed, 14 insertions(+), 9 deletions(-)

> >>

> >> diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c

> >> index d68a1ecebbf3..22b2bb73176c 100644

> >> --- a/drivers/firmware/efi/capsule-loader.c

> >> +++ b/drivers/firmware/efi/capsule-loader.c

> >> @@ -20,6 +20,10 @@

> >>

> >>  #define NO_FURTHER_WRITE_ACTION -1

> >>

> >> +#ifndef phys_to_page

> >> +#define phys_to_page(x)              virt_to_page((unsigned long)__va(x))

> >> +#endif

> >

> > Is this going to work with highmem pages, which presumably, is a

> > possibility for the 32-bit Quark?

> 

> Good point. Given that we don't really care about the virtual address

> anyway, what is the best way to translate physical addresses to struct

> page pointers on x86? i suppose pfn_to_page(pa >> PAGE_SHIFT) always

> does the trick?


Yep, I think that's the way to do it.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c
index d68a1ecebbf3..22b2bb73176c 100644
--- a/drivers/firmware/efi/capsule-loader.c
+++ b/drivers/firmware/efi/capsule-loader.c
@@ -20,6 +20,10 @@ 
 
 #define NO_FURTHER_WRITE_ACTION -1
 
+#ifndef phys_to_page
+#define phys_to_page(x)		virt_to_page((unsigned long)__va(x))
+#endif
+
 /**
  * efi_free_all_buff_pages - free all previous allocated buffer pages
  * @cap_info: pointer to current instance of capsule_info structure
@@ -31,7 +35,7 @@ 
 static void efi_free_all_buff_pages(struct capsule_info *cap_info)
 {
 	while (cap_info->index > 0)
-		__free_page(cap_info->pages[--cap_info->index]);
+		__free_page(phys_to_page(cap_info->pages[--cap_info->index]));
 
 	cap_info->index = NO_FURTHER_WRITE_ACTION;
 }
@@ -157,12 +161,12 @@  static ssize_t efi_capsule_write(struct file *file, const char __user *buff,
 			goto failed;
 		}
 
-		cap_info->pages[cap_info->index++] = page;
+		cap_info->pages[cap_info->index++] = page_to_phys(page);
 		cap_info->page_bytes_remain = PAGE_SIZE;
+	} else {
+		page = phys_to_page(cap_info->pages[cap_info->index - 1]);
 	}
 
-	page = cap_info->pages[cap_info->index - 1];
-
 	kbuff = kmap(page);
 	if (!kbuff) {
 		ret = -ENOMEM;
diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
index 6eedff45e6d7..57f85256feb2 100644
--- a/drivers/firmware/efi/capsule.c
+++ b/drivers/firmware/efi/capsule.c
@@ -214,7 +214,7 @@  efi_capsule_update_locked(efi_capsule_header_t *capsule,
  *
  * Return 0 on success, a converted EFI status code on failure.
  */
-int efi_capsule_update(efi_capsule_header_t *capsule, struct page **pages)
+int efi_capsule_update(efi_capsule_header_t *capsule, phys_addr_t *pages)
 {
 	u32 imagesize = capsule->imagesize;
 	efi_guid_t guid = capsule->guid;
@@ -253,10 +253,11 @@  int efi_capsule_update(efi_capsule_header_t *capsule, struct page **pages)
 		}
 
 		for (j = 0; j < SGLIST_PER_PAGE && count > 0; j++) {
-			u64 sz = min_t(u64, imagesize, PAGE_SIZE);
+			u64 sz = min_t(u64, imagesize,
+				       PAGE_SIZE - (u64)*pages % PAGE_SIZE);
 
 			sglist[j].length = sz;
-			sglist[j].data = page_to_phys(*pages++);
+			sglist[j].data = *pages++;
 
 			imagesize -= sz;
 			count--;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index a7379a2b5680..8269bcb8ccf7 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -143,7 +143,7 @@  struct capsule_info {
 	long			index;
 	size_t			count;
 	size_t			total_size;
-	struct page		**pages;
+	phys_addr_t		*pages;
 	size_t			page_bytes_remain;
 };
 
@@ -1415,7 +1415,7 @@  extern int efi_capsule_supported(efi_guid_t guid, u32 flags,
 				 size_t size, int *reset);
 
 extern int efi_capsule_update(efi_capsule_header_t *capsule,
-			      struct page **pages);
+			      phys_addr_t *pages);
 
 #ifdef CONFIG_EFI_RUNTIME_MAP
 int efi_runtime_map_init(struct kobject *);