RE: Why does memblock only refer to E820 table and not EFI Memory Map?

> > On x86 platforms, there are two sources through which kernel learns > > about physical memory in the system namely E820 table and EFI Memory > > Map. Each table describes which regions of system memory is usable by > > kernel and which regions should be preserved (i.e. reserved regions > >

RE: [PATCH v2] x86/efi: fix a -Wtype-limits compilation warning

> Compiling a kernel with W=1 generates this warning, > > arch/x86/platform/efi/quirks.c:731:16: warning: comparison of unsigned > expression >= 0 is always true [-Wtype-limits] > > Fixes: 3425d934fc03 ("efi/x86: Handle page faults occurring while running EFI > runtime services") >

RE: kexec crash on OVMF i386 + x86_64 kernel (Re: [PATCH v4] x86/boot: Use efi_setup_data for searching RSDP on kexec-ed kernel)

> Added efi people. > > I remember previously Sai did some efi32 tests for kexec, but I'm not sure if > he > tested EFI32 + 64bit kernel. > > Kexec status is not certain because I'm not sure anyone tesed and reported > issues for that. No.. I didn't test kexec on EFI32 + 64bit kernel and I

RE: EFI reboot vs. ACPI reboot (was: Re: [tip:x86/urgent] x86/reboot, efi: Use EFI reboot for Acer TravelMate X514-51T)

> > >> That seems counter-intuitive to me: if no full ACPI hardware is > > >> implemented then we should assume reduced ACPI functionality, i.e. > > >> if > > >the > > >> EFI runtime is otherwise available we should default to it. > > > > > >It's a bit confusing, but my loose understanding is that

RE: [PATCH 02/10] x86/efi: Return error status if mapping EFI regions fail

> > > efi_map_region() creates VA mappings for an given EFI region using > > > any one of the two helper functions (namely __map_region() and > old_map_region()). > > > These helper functions *could* fail while creating mappings and > > > presently their return value is not checked. Not checking

RE: [PATCH] x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

> > Acked-by: Ard Biesheuvel > > > > with the sidenote that I won't be able to test this myself until > > monday at the earliest. > > OK, so I have tested both efi=old_map and mixed mode before and after > applying this patch, using QEMU/KVM with 64-bit and 32-bit builds of OVMF > [respectively]

RE: [PATCH] efi: memattr: don't bail on zero VA if it equals the region's PA

> The EFI memory attributes code cross-references the EFI memory map with the > more granular EFI memory attributes table to ensure that they are in sync > before > applying the strict permissions to the regions it describes. > > Since we always install virtual mappings for the EFI runtime

RE: [PATCH] x86/efi: Don't unmap EFI boot services code/data regions for EFI_OLD_MEMMAP and EFI_MIXED_MODE

> > * Sai Praneeth Prakhya wrote: > > > Commit d5052a7130a6 ("x86/efi: Unmap EFI boot services code/data > > regions from efi_pgd") forgets to take two EFI modes into > > consideration namely EFI_OLD_MEMMAP and EFI_MIXED_MODE. > > So the commit sha1 ended up being this one in tip:efi/core: >

RE: [PATCH V2 0/3] Fix EFI memory map leaks

> > Although majority of the changes are made to > > drivers/firmware/efi/memmap.c file (which is common across > > architectures), this bug is only limited to x86_64 machines and hence this > > patch > set shouldn't effect any other architectures. > > I will give this a test run on the arm64

RE: [PATCH V2 2/3] x86/efi: Fix EFI memory map leaks

> > -static void __init efi_clean_memmap(void) > > +void __init efi_clean_memmap(void) > > { > > - efi_memory_desc_t *out = efi.memmap.map; > > - const efi_memory_desc_t *in = out; > > - const efi_memory_desc_t *end = efi.memmap.map_end; > > - int i, n_removal; > > - > > - for (i =

RE: [PATCH V2 1/3] efi: Introduce efi_memmap_free() and efi_memmap_unmap_and_free()

> > +/** > > + * efi_memmap_free - Free memory pointed by new_memmap.map > > + * @new_memmap: Structure that describes EFI memory map. > > + * > > + * Memory is freed depending on the type of allocation performed. > > + */ > > +static void __init efi_memmap_free(struct efi_memory_map new_memmap) >

RE: [PATCH V2 0/3] Fix EFI memory map leaks

> Since we're being pedantic, it also makes sense to decide now whether 'area' > refers to all [discontiguous] regions or just one of them. I'd say the > former, and > use 'region' for the latter, i.e., an area may be made up of several regions, > but > only one exists of each type. > > >

RE: [PATCH V2 2/3] x86/efi: Fix EFI memory map leaks

> > if (efi_enabled(EFI_MEMMAP)) { > > + /* > > +* efi_clean_memmap() uses memblock_phys_alloc() to allocate > > +* memory for new EFI memmap and hence will work only after > > +* e820__memblock_setup() > > +*/ > > +

RE: [PATCH V2 1/3] efi: Introduce efi_memmap_free() and efi_memmap_unmap_and_free()

> > +static void __init efi_memmap_free(struct efi_memory_map new_memmap) > > +{ > > + phys_addr_t start, end; > > + unsigned long size = new_memmap.nr_map * new_memmap.desc_size; > > + unsigned int order = get_order(size); > > + > > + start = new_memmap.phys_map; > > + end = start +

RE: [GIT PULL 00/11] EFI updates

> On Thu, 29 Nov 2018 at 19:27, Prakhya, Sai Praneeth > wrote: > > > > Hi Ard, > > > > While building from next branch of efi tree, I noticed the below warning. > > Could > you please check the same on your side? > > > > CC lib/list_

RE: [GIT PULL 00/11] EFI updates

; Marc > Zyngier ; Nathan Chancellor > ; Peter Zijlstra ; Prakhya, > Sai Praneeth ; Sedat Dilek > ; YiFei Zhu > Subject: [GIT PULL 00/11] EFI updates > > The following changes since commit > 976b489120cdab2b1b3a41ffa14661db43d58190: > > efi: Prevent GICv3 WARN() by ma

RE: [PATCH V3 1/3] x86/mm/pageattr: Introduce helper function to unmap EFI boot services

> Ideally, after kernel assumes control of the platform, firmware shouldn't > access > EFI boot services code/data regions. But, it's noticed that this is not so > true in > many x86 platforms. Hence, during boot, kernel reserves EFI boot services > code/data regions [1] and maps [2] them to

RE: [PATCH V2 1/2] x86/efi: Unmap EFI boot services code/data regions from efi_pgd

> > /* > > * The testcases use internal knowledge of the implementation that > > shouldn't > > * be exposed to the rest of the kernel. Include these directly here. > > diff --git a/arch/x86/platform/efi/quirks.c > > b/arch/x86/platform/efi/quirks.c index 669babcaf245..fb1c44b11235 > > 100644

RE: [PATCH V2 1/2] x86/efi: Unmap EFI boot services code/data regions from efi_pgd

> > In general it's good to do on list because others can learn from the > > answers as well. > > So you somehow managed to keep everyone and the lists in CC nevertheless. > Did not pay attention to the CC list until I got my reply on the list :) Sorry! my bad.. yes, the conversation did happen

RE: [PATCH V2 1/2] x86/efi: Unmap EFI boot services code/data regions from efi_pgd

Hi Thomas and Peter, [off the mailing list because I wasn't sure if it's a good idea to spam others with my questions] > > > + /* > > > + * The typical sequence for unmapping is to find a pte through > > > + * lookup_address_in_pgd() (ideally, it should never return NULL > because > > > + *

RE: [PATCH V2 1/2] x86/efi: Unmap EFI boot services code/data regions from efi_pgd

> > +int kernel_unmap_pages_in_pgd(pgd_t *pgd, unsigned long address, > > + unsigned long numpages) > > +{ > > + int retval; > > + > > + /* > > +* The typical sequence for unmapping is to find a pte through > > +* lookup_address_in_pgd() (ideally, it should

RE: [PATCH] efi: Fix debugobjects warning on efi_rts_work

> -Original Message- > From: Waiman Long [mailto:long...@redhat.com] > Sent: Tuesday, October 23, 2018 7:18 AM > To: Ard Biesheuvel > Cc: linux-efi@vger.kernel.org; linux-ker...@vger.kernel.org; Prakhya, Sai > Praneeth ; Waiman Long > > Subject: [PATCH] efi:

RE: [PATCH 1/2] x86/efi: Unmap efi boot services code/data regions from efi_pgd

> On 10/21/2018 06:57 PM, Ingo Molnar wrote: > > Does the CPU _ever_ look look at the PFN if the page is > > !_PAGE_PRESENT, for example speculatively? If yes then what is the > > recommended value for the pfn - zero perhaps? > > I'll never say never. :) > > For L1TF[1], we know the CPU did

RE: [PATCH 1/2] x86/efi: Unmap efi boot services code/data regions from efi_pgd

> > The one bit that is odd is the cpa->pfn field - for unmapped pages > > that's totally uninteresting and I'm wondering whether setting it to 0 > > wouldn't be better. > > > > Does the CPU _ever_ look look at the PFN if the page is > > !_PAGE_PRESENT, for example speculatively? If yes then what

RE: [PATCH 1/2] x86/efi: Unmap efi boot services code/data regions from efi_pgd

> > +int kernel_unmap_pages_in_pgd(pgd_t *pgd, u64 pfn, unsigned long address, > > + unsigned long numpages) > > +{ > > + int retval; > > + > > + struct cpa_data cpa = { > > + .vaddr = , > > + .pfn = pfn, > > + .pgd = pgd, > > +

RE: [PATCH V6 2/2] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware

> On 12 September 2018 at 20:56, Thomas Gleixner wrote: > > On Tue, 11 Sep 2018, Sai Praneeth Prakhya wrote: > >> diff --git a/drivers/firmware/efi/runtime-wrappers.c > b/drivers/firmware/efi/runtime-wrappers.c > >> index b18b2d864c2c..7455277a3b65 100644 > >> ---

RE: [PATCH V6 0/2] Add efi page fault handler to recover from page

> > This issue was reported by Al Stone when he saw that reboot via EFI > > hangs the machine. Upon debugging, I found that it's > > efi_reset_system() that's touching memory regions which it shouldn't. > > To reproduce the same behavior, I have hacked OVMF and made > > efi_reset_system() buggy.

RE: [PATCH V5 0/2] Add efi page fault handler to recover from page

Hi All, > Changes from V4 to V5: > -- > 1. Drop config option that enables efi page fault handler, instead make >it default. > 2. Call schedule() in an infinite loop to account for spurious wake ups. > 3. Introduce "NONE" as an efi runtime service function identifier so

RE: [PATCH V4 0/3] Add efi page fault handler to recover from page

> Thanks Sai for this work. I think this a step in the right direction. > I tested this on qemu x86_64 with OVMF firmware modified to access some > random address in the EFI_Reserved_Region. I was able to reboot the qemu > instance successfully with the patches (see logs below) while without the >

RE: [PATCH V4 3/3] x86/efi: Introduce EFI_PAGE_FAULT_HANDLER

> > > Why make this optional? > > > > I made it as a config option in RFC because the page fault handler was > > complicated and touching many parts (it had lots of code change and I > > didn't want to break any existing functionality). Now that it's > > simple, I don't think we need the config

RE: [PATCH V4 0/3] Add efi page fault handler to recover from page

> > The efi page fault handler will check if the access is by > > efi_reset_system(). > > 1. If so, then the efi page fault handler will reboot the machine > >through BIOS and not through efi_reset_system(). > > 2. If not, then the efi page fault handler will freeze efi_rts_wq and > >

RE: [PATCH V4 2/3] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware

> > + if (efi_rts_work.efi_rts_id == RESET_SYSTEM) { > > + pr_info("efi_reset_system() buggy! Reboot through BIOS\n"); > > + machine_real_restart(MRR_BIOS); > > + return 0; > > + } > > + > > + /* Firmware has caused page fault, hence, freeze efi_rts_wq. */ > > +

RE: [PATCH V4 3/3] x86/efi: Introduce EFI_PAGE_FAULT_HANDLER

> > There may exist some buggy UEFI firmware implementations that might > > access efi regions other than EFI_RUNTIME_SERVICES_ even > > after the kernel has assumed control of the platform. This violates > > UEFI specification. > > > > If selected, this debug option will print a warning message

RE: [PATCH V3 3/5] x86/efi: Permanently save the EFI_MEMORY_MAP passed by the firmware

> >> I agree. Keep it simple. If the EFI crap fails, then assist with the > >> reboot and otherwise just kill it. > > > > The reasons for saving old memory map are (in my view, these are the > > less important ones because they are very unlikely to happen) > > > > 1. Make sure that a memory

RE: [PATCH V3 3/5] x86/efi: Permanently save the EFI_MEMORY_MAP passed by the firmware

> On Wed, 5 Sep 2018, Ard Biesheuvel wrote: > > On 5 September 2018 at 14:56, Peter Zijlstra wrote: > > > On Wed, Sep 05, 2018 at 02:27:49PM +0200, Ard Biesheuvel wrote: > > >> Would we still need to preserve the old memory map in that case? > > > > > > I thought the reason for having this was

RE: [PATCH V3 2/5] efi: Introduce __efi_init attribute

Hi Boris and Ard, > Buggy firmware could illegally access some efi regions even after the kernel > has > assumed control of the platform. When > "CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS" is enabled, the efi page fault > handler will detect and recover from these illegal accesses. >

RE: [PATCH V2 4/6] x86/efi: Add efi page fault handler to fixup/recover from page faults caused by firmware

> > The efi specific page fault handler offers us two advantages: > > 1. Avoid panics/hangs caused by buggy firmware. > > 2. Shout loud that the firmware is buggy and hence is not a kernel bug. > > > > Finally, this new mapping will not impact a reboot from kexec, as > > kexec is only concerned

RE: [PATCH V2 5/6] x86/mm: If in_atomic(), allocate pages without sleeping

> Thanks for CC'ing me. I wonder why the world and some more is on CC, but > x...@kernel.org is NOT. > Sorry! about that. I will CC you and x86 from V3. > > On Sun, Sep 02, 2018 at 02:46:33AM -0700, Sai Praneeth Prakhya wrote: > > > > @@ -926,7 +926,13 @@ static void unmap_pud_range(p4d_t *p4d,

RE: [PATCH V2 2/6] x86/efi: Remove __init attribute from memory mapping functions

> On Mon, Sep 03, 2018 at 05:03:56AM +0000, Prakhya, Sai Praneeth wrote: > > Hmm.. thought that __efi_init might be confusing with the normal > > __init attribute > > How would that be confusing? It has "__efi" prepended?! > > All I'm saying is, if you

RE: [PATCH V2 2/6] x86/efi: Remove __init attribute from memory mapping functions

> On Sun, Sep 02, 2018 at 02:46:30AM -0700, Sai Praneeth Prakhya wrote: > > In order to not keep these functions needlessly when > > "CONFIG_EFI_WARN_ON_ILLEGAL_ACCESS" is not selected, add a new > > __efi_init_fixup attribute whose value changes based on whether the > > Why not just

RE: [PATCH V1 6/6] x86/efi: Introduce EFI_WARN_ON_ILLEGAL_ACCESSES

> > +config EFI_WARN_ON_ILLEGAL_ACCESSES > > How about shortening the name to just 'EFI_WARN_ON_ILLEGAL_ACCESS'? > Makes sense.. I will shorten it. > > + bool "Warn about illegal memory accesses by firmware" > > + depends on EFI > > From a distribution p-o-v, I would suggest that

RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from

> > 1. Since we don't use "efi_rts_wq" for efi_reset_system(), the above > > solution doesn't work if efi_reset_system() is buggy. We cannot > > neglect it either because that's the issue faced by Al Stone and hence > > I started the patch set. So, I am thinking to use long jump technique > >

RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from

> > As discussed in previous mails, I have implemented the below: > > > > If kernel detects an illegal access by firmware to any efi region > > other than efi boot time code/data, a. It freezes "efi_rts_wq" (efi > > runtime services work queue). > > b. Signals the requested process that an error

RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from

Hi All, > > >> The main problem is that we have just merged Sai's code to use a > > >> work queue for invoking EFI services, and killing kworker threads > > >> is obviously not going to make EFI any new friends. > > >> > > >> So I guess we should probably rework that code to use a dedicated > >

RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from

> >> The main problem is that we have just merged Sai's code to use a work > >> queue for invoking EFI services, and killing kworker threads is > >> obviously not going to make EFI any new friends. > >> > >> So I guess we should probably rework that code to use a dedicated > >> kthread, and just

RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from

> > The basic idea is that, when you get an exception from a context that > > is busted (i.e. it wasn't user code with a signal handler or kernel > > code with a fixup), you can't return from the exception handler. That > > leaves two choices: > > > > 1. Kill the task without returning. You

RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from

I have added some x86/intel folks to cc. I am fine with these patches, and I think it is useful to be able to detect and recover from buggy UEFI implementations that use boot time regions at runtime. However, I need help from the x86 maintainers/developers to review this so please cc them on

RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from

> > AFAIK, efi runtime services are not reentrant. With this in mind, if > > something > like above happens, I have completely turned off EFI runtime services in > kernel. > Is that OK? Or should we keep them enabled hoping to catch further illegal > accesses (assuming that this feature is not

RE: [PATCH RFC 0/8] Add efi page fault handler to fix/recover from

> I have added some x86/intel folks to cc. > > I am fine with these patches, and I think it is useful to be able to detect > and > recover from buggy UEFI implementations that use boot time regions at > runtime. > > However, I need help from the x86 maintainers/developers to review this so >

RE: [RFC PATCH] x86/efi: drop task_lock() from efi_switch_mm()

> > Because local_save_flags() does not disable interrupts?? > > now that you say so, it does make sense… > > > > Anyway, I would still like to get rid of task_lock() in efi_switch_mm(). > > > Any objections to that? > > > > No, not at all. > okay. I don’t have any either and thanks for the

RE: [PATCH 2/8] efi/x86: Use non-blocking SetVariable() for efi_delete_dummy_variable()

> > diff --git a/arch/x86/platform/efi/quirks.c > > b/arch/x86/platform/efi/quirks.c index 36c1f8b9f7e0..6af39dc40325 > > 100644 > > --- a/arch/x86/platform/efi/quirks.c > > +++ b/arch/x86/platform/efi/quirks.c > > @@ -105,12 +105,11 @@ early_param("efi_no_storage_paranoia", > >

RE: [PATCH 0/6] efi/x86 mixed mode cleanups

> >> Patch #6 helps unused code paths to be optimized away on > >> configurations that don't need them (32-bit only and 64-bit only) > > > > I have tested this patch set on qemu and I see mixed mode kernel not > > booting. > > My test setup is: > > Running qemu-system-x86_64 with 32 bit OVMF and

RE: [PATCH 1/6] efi/x86: prevent reentrant firmware calls in mixed mode

> The UEFI spec does not permit runtime services to be called reentrantly, and > so > it is up to the OS to provide proper locking around such calls. > > For the native case, this was fixed a long time ago, but for the mixed mode > case, > no locking is done whatsoever. Note that the calls are

RE: [PATCH 0/6] efi/x86 mixed mode cleanups

Hi Ard, > This series contains some fixes and cleanups for mixed-mode UEFI on x86. > > Patch #1 adds the missing locking in the runtime service wrapper code for > mixed > mode. This was found by inspection rather than having been reported but could > be a candidate for -stable nonetheless. > >

RE: [PATCH 1/6] efi: Introduce efi_memmap_free() to free memory allocated by efi_memmap_alloc()

> > Signed-off-by: Sai Praneeth Prakhya > > Suggested-by: Ard Biesheuvel > > Cc: Lee Chun-Yi > > Cc: Dave Young > > Cc: Borislav Petkov > > Cc: Laszlo Ersek > > Cc: Jan Kiszka > > Cc: Dave Hansen > > Cc: Bhupesh Sharma > > Cc: Nicolai Stange > > Cc: Naresh Bhat > > Cc: Ricardo Neri > >

RE: [PATCH] efi: Free existing memory map before installing new memory map

> > Also, could you please clarify if there is any specific reason why > > memory allocated using memblock_reserve() shouldn't be freed. I mean, > > not with memblock_free() but I think we could make it _available_ > > using free_bootmem() (or something similar, please correct me if this is not >

RE: [kbuild-all] [PATCH] efi: Free existing memory map before installing new memory map

> >Since efi_memmap_free() is a recently introduced API [1], please note > >that the patch is based on efi tree > >@https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git > > I noticed the official efi tree recored in MAINTAINERS is >

RE: [PATCH] efi: Free existing memory map before installing new memory map

> > + /* Free the memory allocated to the existing memory map */ > > + efi_memmap_free(efi.memmap.phys_map, efi.memmap.nr_map, > > + efi.memmap.late); > > + > > data.phys_map = addr; > > data.size = efi.memmap.desc_size * nr_map; > > data.desc_version =

RE: [PATCH] efi: Free existing memory map before installing new memory map

> > Hi Sai, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on linus/master] > [also build test ERROR on v4.18-rc2 next-20180625] [if your patch is applied > to > the wrong git tree, please drop us a note to help improve the system] Since efi_memmap_free() is

RE: [PATCH V5 0/3] Use efi_rts_wq to invoke EFI Runtime Services

> >> I have tested V4 series on ThunderX2 Val1 and Saber platform by > >> integrating the patches with LUV project. > >> I will test V5 version soon and let you know the outcome. > > > > Thanks a lot! for taking time to test the patch set, Naresh. Will be > > waiting to > see the result. > >

RE: [PATCH V5 3/3] efi: Use efi_rts_wq to invoke EFI Runtime Services

> >> I noticed that -unsurprisingly- reboot no longer works with these changes. > >> > >> I will fix up the patch, and revert the efi_reset_system() change, > >> both here and below. > > > > Could you please let me know what the bug is here? I am unable to see > > it right away :( I have tested

RE: [PATCH V5 3/3] efi: Use efi_rts_wq to invoke EFI Runtime Services

> > + case RESET_SYSTEM: > > + __efi_call_virt(reset_system, *(int *)arg1, > > + *(efi_status_t *)arg2, > > + *(unsigned long *)arg3, > > + (efi_char16_t *)arg4); > > +

RE: [PATCH V5 0/3] Use efi_rts_wq to invoke EFI Runtime Services

> > Testing: > > > > Tested using LUV (Linux UEFI Validation) for x86_64, x86_32 and arm64 > > (qemu only). Will appreciate the effort if someone could test the > > patches on real ARM/ARM64 machines. > > LUV: https://01.org/linux-uefi-validation > > I have tested V4 series on ThunderX2

RE: [PATCH V5 0/3] Use efi_rts_wq to invoke EFI Runtime Services

> >> Testing: > >> > >> Tested using LUV (Linux UEFI Validation) for x86_64, x86_32 and arm64 > >> (qemu only). Will appreciate the effort if someone could test the > >> patches on real ARM/ARM64 machines. > > I would give the latest v5 a try on my ARM64 qualcomm board as well. > WIll

RE: [PATCH V4 0/3] Use efi_rts_wq to invoke EFI Runtime Services

> > Another follow on question is, does every firmware support both > > blocking and non-blocking variants (specially legacy EFI firmware)? I > > am worried about this because, presently efi_delete_dummy_variable() > > uses set_variable() and > > query_variable_info() but if I change

RE: [PATCH V4 0/3] Use efi_rts_wq to invoke EFI Runtime Services

> > One more question again, if we are sure that non-blocking variants > > will _always_ be called in atomic context, then, we got it covered. > > Because, in > > set_variable() and query_variable_info() (both blocking and > > non-blocking) we check for in_atomic() and if so, we don't use

RE: [PATCH V4 0/3] Use efi_rts_wq to invoke EFI Runtime Services

> > Assume some user requested to execute some non-blocking variant of > > efi_rts and the kernel hasn't called efi_call_virt() yet, but was > > scheduled out. IOW, even though user requests for non-blocking efi call, we > might still block. Am I right? > > > > No, that is the whole point. These

RE: [PATCH V4 0/3] Use efi_rts_wq to invoke EFI Runtime Services

> > Changes from V3 to V4: > > -- > > 1. As suggested by Peter, use completions instead of flush_work() as the > > former is cheaper > > 2. Call efi_delete_dummy_variable() from efisubsys_init(). Sorry! Ard, > > wasn't able to find a better alternative to keep this change

RE: [PATCH V3 2/3] efi: Introduce efi_queue_work() to queue any efi_runtime_service() on efi_rts_wq

> On Mon, May 21, 2018 at 08:13:03PM -0700, Sai Praneeth Prakhya wrote: > > + /* \ > > +* queue_work() returns 0 if work was already on queue, \ > > +* _ideally_ this should never happen. \ > >

RE: Query regarding SetVirtualAddressMap()

> > AFAIK, ExitBootServices() means that boot services are no longer > > needed by OS/bootloader and hence firmware can terminate them. Does it > > also mean that the system is in runtime mode..? (I don't think so, as, I > > didn't > find it in UEFI spec). > > > > Yes > > > Also, could you

RE: Query regarding SetVirtualAddressMap()

+ Ricardo > > > I recently noticed in UEFI spec v2.7, section 2.3.4, Calling > > > Conventions for X64, that “All selectors set to be flat with virtual > > > = physical address. If the UEFI OS loader or OS used > > > SetVirtualAddressMap() to relocate the runtime services in a virtual > > >

RE: Query regarding SetVirtualAddressMap()

> > I recently noticed in UEFI spec v2.7, section 2.3.4, Calling > > Conventions for X64, that “All selectors set to be flat with virtual = > > physical address. If the UEFI OS loader or OS used > > SetVirtualAddressMap() to relocate the runtime services in a virtual > > address space, then this

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

> > That's true! AFAIK, we don't have any issues handling NMI while in efi_pgd. > > We might have issues only when, we are already in efi_pgd, NMI comes > > along > > Can you trigger this? Or is it something hypothetical? > AFAIK, it's hypothetical. I did try to trigger the issue, but failed

RE: [PATCH 07/12] efi: Use efi_mm in x86 as well as ARM

> > diff --git a/include/linux/efi.h b/include/linux/efi.h index > > f5083aa72eae..f1b7d68ac460 100644 > > --- a/include/linux/efi.h > > +++ b/include/linux/efi.h > > @@ -966,6 +966,8 @@ extern struct efi { > > unsigned long flags; > > } efi; > > > > +extern struct mm_struct efi_mm; > > + > >

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

> > Another warning by checkpatch is "use of in_atomic() in drivers code" > > I'm assuming it warns because you're touching files in drivers/ but the efi > fun is > not really a driver... True! That makes sense :) > > But looking at patch 3, that thing looks like a real mess. Some of the

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

/* > >> > +* Since we process only one efi_runtime_service() at a time, an > >> > +* ordered workqueue (which creates only one execution context) > >> > +* should suffice all our needs. > >> > +*/ > >> > + efi_rts_wq =

RE: [PATCH V2 1/3] x86/efi: Call efi_delete_dummy_variable() during efi subsystem initialization

> > void __init efi_enter_virtual_mode(void) diff --git > > a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index > > cd42f66a7c85..838b8efe639c 100644 > > --- a/drivers/firmware/efi/efi.c > > +++ b/drivers/firmware/efi/efi.c > > @@ -328,6 +328,12 @@ static int __init

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

+Cc Miguel Ojeda > > > +({ > > > \ > > > + struct efi_runtime_work efi_rts_work; \ > > > + \ > > > + INIT_WORK_ONSTACK(_rts_work.work,

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

> > +({ \ > > + struct efi_runtime_work efi_rts_work; \ > > + \ > > + INIT_WORK_ONSTACK(_rts_work.work, efi_call_rts);\ > > +

RE: [PATCH V2 3/3] efi: Use efi_rts_workqueue to invoke EFI Runtime Services

> >> > pstore writes could potentially be invoked in interrupt context and > >> > it uses set_variable<>() and query_variable_info<>() to store logs. > >> > If we invoke efi_runtime_services() through efi_rts_wq while in > >> > atomic() kernel issues a warning ("scheduling wile in atomic") and >

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

> > +struct workqueue_struct *efi_rts_wq; > > + > > static bool disable_runtime; > > static int __init setup_noefi(char *arg) { @@ -329,6 +331,19 @@ > > static int __init efisubsys_init(void) > > return 0; > > > > /* > > +* Since we process only one

RE: [PATCH V2 3/3] efi: Use efi_rts_workqueue to invoke EFI Runtime Services

05, 2018 at 03:23:10PM -0800, Sai Praneeth Prakhya wrote: > > From: Sai Praneeth > > > > Presently, efi_runtime_services() are executed by firmware in process > > context. To execute efi_runtime_service(), kernel switches the page > > directory from swapper_pgd to

RE: [PATCH V2 2/3] efi: Introduce efi_rts_workqueue and some infrastructure to invoke all efi_runtime_services()

-0800, Sai Praneeth Prakhya wrote: > > @@ -329,6 +331,19 @@ static int __init efisubsys_init(void) > > return 0; > > > > /* > > +* Since we process only one efi_runtime_service() at a time, an > > +* ordered workqueue (which creates only one execution context) > > +*

RE: [PATCH V2 3/3] efi: Use efi_rts_workqueue to invoke EFI Runtime Services

> > Presently, efi_runtime_services() are executed by firmware in process > > context. To execute efi_runtime_service(), kernel switches the page > > directory from swapper_pgd to efi_pgd. However, efi_pgd doesn't have > > any user space mappings. A potential issue could be, for instance, an > >

RE: [PATCH V1 2/3] efi: Introduce efi_rts_workqueue and necessary infrastructure to invoke all efi_runtime_services()

> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c > > index ac5db5f8dbbf..4714b305ca90 100644 > > --- a/drivers/firmware/efi/efi.c > > +++ b/drivers/firmware/efi/efi.c > > @@ -76,6 +76,8 @@ static unsigned long *efi_tables[] = { > > _attr_table, > > }; > > > >

RE: [PATCH 0/3] Use mm_struct and switch_mm() instead of manually

> > Changes in V3: > > 1. When CPUMASK_OFFSTACK is enabled, switch_mm_irqs_off() sets cpumask > > by calling cpumask_set_cpu(). This panics kernel as efi_mm is not > > initialized, therefore initialize efi_mm in efi_alloc_page_tables(). > > Thanks for the v3. > > I confirmed that the issue I saw

RE: [PATCH V2 0/3] Use mm_struct and switch_mm() instead of manually

> So, in summary it seems that the primary kernel boot _fails_ with your > v2 patchset on the real hardware for me irrespective of whether I use Matt's > tree or Linus's tree: > > a) I would suggest that you perform some more checks on real hardware as > qemu boot tests sometimes do not expose

RE: [PATCH V2 0/3] Use mm_struct and switch_mm() instead of manually

> -Original Message- > From: Sai Praneeth Prakhya [mailto:sai.praneeth.prak...@intel.com] > Sent: Tuesday, September 5, 2017 7:43 PM > To: Bhupesh Sharma > Cc: linux-efi@vger.kernel.org; linux-ker...@vger.kernel.org; Matt Fleming > ; Ard

RE: [PATCH V2 0/3] Use mm_struct and switch_mm() instead of manually

> > > > Thanks for this v2. > > Introducing the 'efi_switch_mm() ' helper instead of manually > > twiddling with %cr3 seems more cleaner. > > > > I have tested this patchset on a x86_64 machine with the following > > configurations: > > > > 1. Primary kernel boot with efi=old_map 2. Primary kernel

RE: [PATCH] x86/efi-bgrt: Fix kernel panic when mapping BGRT data

>>On Thu, Dec 10, 2015 at 10:27:01AM -0800, Sai Praneeth Prakhya wrote: >> From: Sai Praneeth >> >> Starting with this commit 35eb8b81edd4 ("x86/efi: Build our own page >> table structures") efi regions have a separate page directory called >> "efi_pgd". In