2009-03-06 17:46:51

by Cliff Wickman

[permalink] [raw]
Subject: [PATCH] x86: access to efi reserved memory type

From: Cliff Wickman <[email protected]>

(this patch dates back to 2008-11-06
http://marc.info/?l=linux-kernel&m=122600658522471&w=2
but has never been applied.)

Give drivers addresses of memory type EFI_RESERVED_TYPE.
This supports drivers that use vendor-specific memory, available
only to special devices.

The walk() function scans the EFI memory map and does a callback to a
specified function for each memory area of a specified type.
efi_memmap_walk_reserved() provides a scan for type EFI_RESERVED_TYPE.
(an earlier version of this patch had proposed a new EFI type, but
EFI_RESERVED_TYPE should be sufficient, given that the firmware follows
the standard and does not use such memory for its own purposes)

A UV driver will be posted to the community in the future that will use
these routines.

Tested on 2.6.29-rc7 (and many previous versions) running on a
UV hardware simulator.

Diffed against 2.6.29-rc7

Signed-off-by: Cliff Wickman <[email protected]>
---
arch/x86/kernel/efi.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

Index: linux/arch/x86/kernel/efi.c
===================================================================
--- linux.orig/arch/x86/kernel/efi.c
+++ linux/arch/x86/kernel/efi.c
@@ -579,3 +579,39 @@ u64 efi_mem_attributes(unsigned long phy
}
return 0;
}
+
+static void
+walk(efi_freemem_callback_t callback, void *arg, int type)
+{
+ efi_memory_desc_t *md;
+ void *p;
+ int size;
+
+ /*
+ * memmap.map is zeroed in efi_enter_virtual_mode()
+ * but we can use the physical address (phys_map)
+ */
+ size = memmap.nr_map*memmap.desc_size;
+ for (p = memmap.phys_map; p < memmap.phys_map+size;
+ p += memmap.desc_size) {
+ md = (efi_memory_desc_t *)__va(p);
+ if (md->type != type)
+ continue;
+ if ((*callback)(md->phys_addr,
+ md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - 1,
+ arg) < 0)
+ return;
+ }
+}
+
+/*
+ * Walk the EFI memory map and call "callback" once for each EFI memory
+ * descriptor of type EFI_RESERVED_TYPE.
+ */
+void
+efi_memmap_walk_reserved(efi_freemem_callback_t callback, void *arg)
+{
+ walk(callback, arg, EFI_RESERVED_TYPE);
+}
+
+EXPORT_SYMBOL(efi_memmap_walk_reserved);


2009-03-08 22:51:40

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] x86: access to efi reserved memory type

Cliff Wickman wrote:
> From: Cliff Wickman <[email protected]>
>
> (this patch dates back to 2008-11-06
> http://marc.info/?l=linux-kernel&m=122600658522471&w=2
> but has never been applied.)
>
> Give drivers addresses of memory type EFI_RESERVED_TYPE.
> This supports drivers that use vendor-specific memory, available
> only to special devices.
>
> The walk() function scans the EFI memory map and does a callback to a
> specified function for each memory area of a specified type.
> efi_memmap_walk_reserved() provides a scan for type EFI_RESERVED_TYPE.
> (an earlier version of this patch had proposed a new EFI type, but
> EFI_RESERVED_TYPE should be sufficient, given that the firmware follows
> the standard and does not use such memory for its own purposes)
>
> A UV driver will be posted to the community in the future that will use
> these routines.
>
> Tested on 2.6.29-rc7 (and many previous versions) running on a
> UV hardware simulator.
>

I have multiple issues with this patch.

FIRST, this is identical to a platform driver. I really don't
understand why it should need a special interface.

SECOND, the EFI-specific callback interface is just plain weird.

THIRD, saying "A UV driver will be posted to the community in the future
that will use these routines" is not exactly motivation. At no point
are you technically justifying this code.

In particular, I really want to know why a plain platform device is
insufficient, and look for a better solution than this, using the
generic memory map interfaces rather than something EFI-specific.

-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

2009-03-09 13:17:16

by Robin Holt

[permalink] [raw]
Subject: Re: [PATCH] x86: access to efi reserved memory type

On Sun, Mar 08, 2009 at 03:51:20PM -0700, H. Peter Anvin wrote:
> Cliff Wickman wrote:
...
> > The walk() function scans the EFI memory map and does a callback to a
> > specified function for each memory area of a specified type.
> > efi_memmap_walk_reserved() provides a scan for type EFI_RESERVED_TYPE.
> > (an earlier version of this patch had proposed a new EFI type, but
> > EFI_RESERVED_TYPE should be sufficient, given that the firmware follows
> > the standard and does not use such memory for its own purposes)
...
> In particular, I really want to know why a plain platform device is
> insufficient, and look for a better solution than this, using the
> generic memory map interfaces rather than something EFI-specific.

For the driver Cliff is referring to, we are attempting to provide EFI
reserved memory to a special hardware (drivers/misc/sgi-gru) device
which is capable of handling pages up to 1TB in size. Additionally, a
previously posted xpmem driver could make those pages available via
Numalink to other SSIs connected to the same fabric.

Excuse my ignorance, but what do you mean by a "plain platform device"?
By generic memory map interfaces, are you proposing we use order based
allocations? If so, how do you propose we zero these 1TB (worst case)
size pages. Our current estimate is this will take approx 20 minutes,
but newer processors might shorten that. What assurances can we get on
these large pages not getting fragmented and therefore becoming useless
and requiring a reboot to reclaim.

I believe we have a special case of memory needing to be reserved by
BIOS, passed to a special driver which can utilize the GRU for async
zero on allocation and to prevent unintended fragmentation. I agree we
probably made a small mistake in our submission in that the patch set
which adds the EFI walk should have included the special purpose driver.
We will work to correct that.

Thanks,
Robin