Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Fri, May 03, 2019 at 08:02:18AM +0200, Ingo Molnar wrote:
>
> * Matthew Garrett wrote:
>
> > On Thu, May 2, 2019 at 12:15 AM Ard Biesheuvel
> > wrote:
> > >
> > > (+ Ingo)
> > >
> > > On Tue, 30 Apr 2019 at 21:52, Matthew Garrett wrote:
> > > >
> > > > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek
> > > > wrote:
> > > > >
> > > > > I may be a little late with this comment, but I've just tested these
> > > > > patches on aarch64 platform (from the top of jjs/master) and got
> > > > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > > > mail). I think there's problem with below call to
> > > > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > > > passed as (void *) and never remapped:
> > > >
> > > > Yes, it looks like this is just broken. Can you try with the attached
> > > > patch?
> > >
> > > I'm a bit uncomfortable with EFI code that is obviously broken and
> > > untested being queued for the next merge window in another tree.
> >
> > The patchset was Cc:ed to linux-efi@. Is there anything else I should
> > have done to ensure you picked it up rather than Jarkko?
>
> That's not the workflow rule the Linux kernel is using, if Cc:-ing a
> patchset was the only condition for upstream inclusion then we'd have a
> *LOT* of crap in the Linux kernel.
>
> Just applying those EFI changes without even as much as an Acked-by from
> the EFI maintainers is a *totally* unacceptable workflow.
>
> Please revert/rebase and re-try this on the proper submission channels.
>
> Meanwhile the broken code is NAK-ed by me:
>
>Nacked-by: Ingo Molnar
There must be some kind of misconception here. None of the changes have
been submitted so far. They are only in my master branch. They briefly
went to linux-next through my next branch but as soon as issues were
reported I wiped them off from there (which happened like 2-3 weeks
ago). They haven't been part off any of my PR's.
There is nothing to revert.
/Jarkko
Re: [PATCH 1/2 v2] efi: add a function to convert the status value to string
On Fri, 3 May 2019 at 08:15, joeyli wrote: > > On Thu, May 02, 2019 at 10:53:31AM +0200, Ard Biesheuvel wrote: > > On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi wrote: > > > > > > This function can be used to convert EFI status value to string > > > for printing out debug message. Using this function can improve > > > the readability of log. > > > > > > v2. > > > > Please move the changelog out of the commit log (move it below the --- > > further down) > > > > OK! I will moved the changelog out of the commit log. > > > > - Changed the wording in subject and description. > > > - Moved the marco immediately after the status value definitions. > > > - Turned into a proper function instead of inline. > > > > > > > You missed my point here. A proper function means the function in a .c > > file, and only the declaration in a .h file. This way, you are still > > duplicating the literal strings into every object file that references > > this function. > > > > Sorry for I missunderstood. I will move the function to load_uefi.c. > No, please move it to a file that is shared between all EFI code.
Re: [PATCH 1/2 v2] efi: add a function to convert the status value to string
On Thu, May 02, 2019 at 10:53:31AM +0200, Ard Biesheuvel wrote: > On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi wrote: > > > > This function can be used to convert EFI status value to string > > for printing out debug message. Using this function can improve > > the readability of log. > > > > v2. > > Please move the changelog out of the commit log (move it below the --- > further down) > OK! I will moved the changelog out of the commit log. > > - Changed the wording in subject and description. > > - Moved the marco immediately after the status value definitions. > > - Turned into a proper function instead of inline. > > > > You missed my point here. A proper function means the function in a .c > file, and only the declaration in a .h file. This way, you are still > duplicating the literal strings into every object file that references > this function. > Sorry for I missunderstood. I will move the function to load_uefi.c. Thanks a lot! Joey Lee
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
* Matthew Garrett wrote:
> On Thu, May 2, 2019 at 12:15 AM Ard Biesheuvel
> wrote:
> >
> > (+ Ingo)
> >
> > On Tue, 30 Apr 2019 at 21:52, Matthew Garrett wrote:
> > >
> > > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek
> > > wrote:
> > > >
> > > > I may be a little late with this comment, but I've just tested these
> > > > patches on aarch64 platform (from the top of jjs/master) and got
> > > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > > mail). I think there's problem with below call to
> > > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > > passed as (void *) and never remapped:
> > >
> > > Yes, it looks like this is just broken. Can you try with the attached
> > > patch?
> >
> > I'm a bit uncomfortable with EFI code that is obviously broken and
> > untested being queued for the next merge window in another tree.
>
> The patchset was Cc:ed to linux-efi@. Is there anything else I should
> have done to ensure you picked it up rather than Jarkko?
That's not the workflow rule the Linux kernel is using, if Cc:-ing a
patchset was the only condition for upstream inclusion then we'd have a
*LOT* of crap in the Linux kernel.
Just applying those EFI changes without even as much as an Acked-by from
the EFI maintainers is a *totally* unacceptable workflow.
Please revert/rebase and re-try this on the proper submission channels.
Meanwhile the broken code is NAK-ed by me:
Nacked-by: Ingo Molnar
Thanks,
Ingo
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Thu, May 02, 2019 at 11:03:08AM -0700, Matthew Garrett wrote:
> On Thu, May 2, 2019 at 1:32 AM Jarkko Sakkinen
> wrote:
> >
> > On Tue, Apr 30, 2019 at 03:07:09PM +0200, Bartosz Szczepanek wrote:
> > > I may be a little late with this comment, but I've just tested these
> > > patches on aarch64 platform (from the top of jjs/master) and got
> > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > mail). I think there's problem with below call to
> > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > passed as (void *) and never remapped:
> >
> > Not late. This is not part of any PR yet. Thank you for the
> > feedback!
> >
> > Matthew, can you send an updated version of the whole patch set
> > with fixes to this issue and also reordering of the includes?
>
> Yes, I'll resend and let's do this again for 5.3.
Agreed, better not rush but get it right once.
/Jarkko
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Thu, May 02, 2019 at 09:14:49AM +0200, Ard Biesheuvel wrote:
> (+ Ingo)
>
> On Tue, 30 Apr 2019 at 21:52, Matthew Garrett wrote:
> >
> > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek
> > wrote:
> > >
> > > I may be a little late with this comment, but I've just tested these
> > > patches on aarch64 platform (from the top of jjs/master) and got
> > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > mail). I think there's problem with below call to
> > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > passed as (void *) and never remapped:
> >
> > Yes, it looks like this is just broken. Can you try with the attached patch?
>
> I'm a bit uncomfortable with EFI code that is obviously broken and
> untested being queued for the next merge window in another tree.
>
> What is currently queued there? Can we revert this change for the time
> being, and resubmit it via the EFI tree for v5.3?
Nothing ATM. The broken code is in my master branch ATM. Nothing
in my next branch nor have I included anything to my PRs. Should
be nothing to worry about in that sense :-)
/Jarkko
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Thu, 2 May 2019 at 20:04, Matthew Garrett wrote:
>
> On Thu, May 2, 2019 at 12:15 AM Ard Biesheuvel
> wrote:
> >
> > (+ Ingo)
> >
> > On Tue, 30 Apr 2019 at 21:52, Matthew Garrett wrote:
> > >
> > > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek
> > > wrote:
> > > >
> > > > I may be a little late with this comment, but I've just tested these
> > > > patches on aarch64 platform (from the top of jjs/master) and got
> > > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > > mail). I think there's problem with below call to
> > > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > > passed as (void *) and never remapped:
> > >
> > > Yes, it looks like this is just broken. Can you try with the attached
> > > patch?
> >
> > I'm a bit uncomfortable with EFI code that is obviously broken and
> > untested being queued for the next merge window in another tree.
>
> The patchset was Cc:ed to linux-efi@. Is there anything else I should
> have done to ensure you picked it up rather than Jarkko?
No, I am not saying it was you who did anything wrong - Jarkko and I
should probably have aligned better. But my own testing wouldn't have
caught this particular issue either (I am still in the process of
getting access to ARM machines with a TPM), so it wouldn't have made a
huge difference in any case.
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
Sorry, how about this one? I was confused by why I wasn't hitting
this, but on closer examination it turns out that my system populates
the final event log with 0 events which means we never hit this
codepath :(
diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index 2ccaa6661aaf..db0fdaa9c666 100644
--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -28,6 +28,7 @@ static int tpm2_calc_event_log_size(void *data, int count, void *size_info)
if (event_size == 0)
return -1;
size += event_size;
+ count--;
}
return size;
@@ -41,6 +42,7 @@ int __init efi_tpm_eventlog_init(void)
struct linux_efi_tpm_eventlog *log_tbl;
struct efi_tcg2_final_events_table *final_tbl;
unsigned int tbl_size;
+ int ret = 0;
if (efi.tpm_log == EFI_INVALID_TABLE_ADDR) {
/*
@@ -60,10 +62,9 @@ int __init efi_tpm_eventlog_init(void)
tbl_size = sizeof(*log_tbl) + log_tbl->size;
memblock_reserve(efi.tpm_log, tbl_size);
- early_memunmap(log_tbl, sizeof(*log_tbl));
if (efi.tpm_final_log == EFI_INVALID_TABLE_ADDR)
- return 0;
+ goto out;
final_tbl = early_memremap(efi.tpm_final_log, sizeof(*final_tbl));
@@ -71,17 +72,20 @@ int __init efi_tpm_eventlog_init(void)
pr_err("Failed to map TPM Final Event Log table @ 0x%lx\n",
efi.tpm_final_log);
efi.tpm_final_log = EFI_INVALID_TABLE_ADDR;
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out;
}
tbl_size = tpm2_calc_event_log_size(final_tbl->events,
final_tbl->nr_events,
- (void *)efi.tpm_log);
+ log_tbl->log);
memblock_reserve((unsigned long)final_tbl,
tbl_size + sizeof(*final_tbl));
early_memunmap(final_tbl, sizeof(*final_tbl));
efi_tpm_final_log_size = tbl_size;
- return 0;
+out:
+ early_memunmap(log_tbl, sizeof(*log_tbl));
+ return ret;
}
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index dccc97e6135c..190a33968a91 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -158,7 +158,6 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
{
struct tcg_efi_specid_event_head *efispecid;
struct tcg_event_field *event_field;
- void *mapping = NULL;
int mapping_size;
void *marker;
void *marker_start;
@@ -176,9 +175,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
/* Map the event header */
if (do_mapping) {
mapping_size = marker - marker_start;
- mapping = early_memremap((unsigned long)marker_start,
+ event = early_memremap((unsigned long)marker_start,
mapping_size);
- if (!mapping) {
+ if (!event) {
size = 0;
goto out;
}
@@ -199,9 +198,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
if (do_mapping) {
early_memunmap(mapping, mapping_size);
mapping_size = marker - marker_start + halg_size;
- mapping = early_memremap((unsigned long)marker_start,
+ event = early_memremap((unsigned long)marker_start,
mapping_size);
- if (!mapping) {
+ if (!event) {
size = 0;
goto out;
}
@@ -219,9 +218,9 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
if (do_mapping) {
early_memunmap(mapping, mapping_size);
mapping_size = marker - marker_start;
- mapping = early_memremap((unsigned long)marker_start,
+ event = early_memremap((unsigned long)marker_start,
mapping_size);
- if (!mapping) {
+ if (!event) {
size = 0;
goto out;
}
@@ -243,11 +242,11 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
* we don't need to map it
*/
if (do_mapping) {
- early_memunmap(marker_start, mapping_size);
+ early_memunmap(event, mapping_size);
mapping_size += sizeof(event_field->event_size);
- mapping = early_memremap((unsigned long)marker_start,
- mapping_size);
- if (!mapping) {
+ event = early_memremap((unsigned long)marker_start,
+ mapping_size);
+ if (!event) {
size = 0;
goto out;
}
@@ -257,8 +256,6 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
+ event_field->event_size;
size = marker - marker_start;
- if ((event->event_type == 0) && (event_field->event_size == 0))
- size = 0;
out:
if (do_mapping)
early_memunmap(mapping, mapping_size);
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Thu, May 2, 2019 at 12:15 AM Ard Biesheuvel
wrote:
>
> (+ Ingo)
>
> On Tue, 30 Apr 2019 at 21:52, Matthew Garrett wrote:
> >
> > On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek
> > wrote:
> > >
> > > I may be a little late with this comment, but I've just tested these
> > > patches on aarch64 platform (from the top of jjs/master) and got
> > > kernel panic ("Unable to handle kernel read", full log at the end of
> > > mail). I think there's problem with below call to
> > > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > > passed as (void *) and never remapped:
> >
> > Yes, it looks like this is just broken. Can you try with the attached patch?
>
> I'm a bit uncomfortable with EFI code that is obviously broken and
> untested being queued for the next merge window in another tree.
The patchset was Cc:ed to linux-efi@. Is there anything else I should
have done to ensure you picked it up rather than Jarkko?
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Thu, May 2, 2019 at 1:32 AM Jarkko Sakkinen
wrote:
>
> On Tue, Apr 30, 2019 at 03:07:09PM +0200, Bartosz Szczepanek wrote:
> > I may be a little late with this comment, but I've just tested these
> > patches on aarch64 platform (from the top of jjs/master) and got
> > kernel panic ("Unable to handle kernel read", full log at the end of
> > mail). I think there's problem with below call to
> > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > passed as (void *) and never remapped:
>
> Not late. This is not part of any PR yet. Thank you for the
> feedback!
>
> Matthew, can you send an updated version of the whole patch set
> with fixes to this issue and also reordering of the includes?
Yes, I'll resend and let's do this again for 5.3.
Re: [PATCH] mm/pgtable: Drop pgtable_t variable from pte_fn_t functions
On Thu, 2 May 2019 06:46:23 -0700
Matthew Wilcox wrote:
> On Thu, May 02, 2019 at 06:48:46PM +0530, Anshuman Khandual wrote:
> > Drop the pgtable_t variable from all implementation for pte_fn_t as none of
> > them use it. apply_to_pte_range() should stop computing it as well. Should
> > help us save some cycles.
>
> You didn't add Martin Schwidefsky for some reason. He introduced
> it originally for s390 for sub-page page tables back in 2008 (commit
> 2f569afd9c). I think he should confirm that he no longer needs it.
With its 2K pte tables s390 can not deal with a (struct page *) as a reference
to a page table. But if there are no user of the apply_to_page_range() API
left which actually make use of the token argument we can safely drop it.
> > ---
> > - Boot tested on arm64 and x86 platforms.
> > - Build tested on multiple platforms with their defconfig
> >
> > arch/arm/kernel/efi.c | 3 +--
> > arch/arm/mm/dma-mapping.c | 3 +--
> > arch/arm/mm/pageattr.c | 3 +--
> > arch/arm64/kernel/efi.c| 3 +--
> > arch/arm64/mm/pageattr.c | 3 +--
> > arch/x86/xen/mmu_pv.c | 3 +--
> > drivers/gpu/drm/i915/i915_mm.c | 3 +--
> > drivers/xen/gntdev.c | 6 ++
> > drivers/xen/privcmd.c | 6 ++
> > drivers/xen/xlate_mmu.c| 3 +--
> > include/linux/mm.h | 3 +--
> > mm/memory.c| 5 +
> > mm/vmalloc.c | 2 +-
> > 13 files changed, 15 insertions(+), 31 deletions(-)
> >
> > diff --git a/arch/arm/kernel/efi.c b/arch/arm/kernel/efi.c
> > index 9f43ba012d10..b1f142a01f2f 100644
> > --- a/arch/arm/kernel/efi.c
> > +++ b/arch/arm/kernel/efi.c
> > @@ -11,8 +11,7 @@
> > #include
> > #include
> >
> > -static int __init set_permissions(pte_t *ptep, pgtable_t token,
> > - unsigned long addr, void *data)
> > +static int __init set_permissions(pte_t *ptep, unsigned long addr, void
> > *data)
> > {
> > efi_memory_desc_t *md = data;
> > pte_t pte = *ptep;
> > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> > index 43f46aa7ef33..739286511a18 100644
> > --- a/arch/arm/mm/dma-mapping.c
> > +++ b/arch/arm/mm/dma-mapping.c
> > @@ -496,8 +496,7 @@ void __init dma_contiguous_remap(void)
> > }
> > }
> >
> > -static int __dma_update_pte(pte_t *pte, pgtable_t token, unsigned long
> > addr,
> > - void *data)
> > +static int __dma_update_pte(pte_t *pte, unsigned long addr, void *data)
> > {
> > struct page *page = virt_to_page(addr);
> > pgprot_t prot = *(pgprot_t *)data;
> > diff --git a/arch/arm/mm/pageattr.c b/arch/arm/mm/pageattr.c
> > index 1403cb4a0c3d..c8b500940e1f 100644
> > --- a/arch/arm/mm/pageattr.c
> > +++ b/arch/arm/mm/pageattr.c
> > @@ -22,8 +22,7 @@ struct page_change_data {
> > pgprot_t clear_mask;
> > };
> >
> > -static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long
> > addr,
> > - void *data)
> > +static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> > {
> > struct page_change_data *cdata = data;
> > pte_t pte = *ptep;
> > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> > index 4f9acb5fbe97..230cff073a08 100644
> > --- a/arch/arm64/kernel/efi.c
> > +++ b/arch/arm64/kernel/efi.c
> > @@ -86,8 +86,7 @@ int __init efi_create_mapping(struct mm_struct *mm,
> > efi_memory_desc_t *md)
> > return 0;
> > }
> >
> > -static int __init set_permissions(pte_t *ptep, pgtable_t token,
> > - unsigned long addr, void *data)
> > +static int __init set_permissions(pte_t *ptep, unsigned long addr, void
> > *data)
> > {
> > efi_memory_desc_t *md = data;
> > pte_t pte = READ_ONCE(*ptep);
> > diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> > index 6cd645edcf35..0be077628b21 100644
> > --- a/arch/arm64/mm/pageattr.c
> > +++ b/arch/arm64/mm/pageattr.c
> > @@ -27,8 +27,7 @@ struct page_change_data {
> >
> > bool rodata_full __ro_after_init =
> > IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED);
> >
> > -static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long
> > addr,
> > - void *data)
> > +static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> > {
> > struct page_change_data *cdata = data;
> > pte_t pte = READ_ONCE(*ptep);
> > diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
> > index a21e1734fc1f..308a6195fd26 100644
> > --- a/arch/x86/xen/mmu_pv.c
> > +++ b/arch/x86/xen/mmu_pv.c
> > @@ -2702,8 +2702,7 @@ struct remap_data {
> > struct mmu_update *mmu_update;
> > };
> >
> > -static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token,
> > -unsigned long addr, void *data)
> > +static int remap_area_pfn_pte_fn(pte_t *ptep, unsigned long addr, void
> > *data)
> > {
> > struct remap_data *rmd = data;
> > pte_t pte = pte_mkspecial(mfn_
Re: [PATCH] mm/pgtable: Drop pgtable_t variable from pte_fn_t functions
On 05/02/2019 07:16 PM, Matthew Wilcox wrote: > On Thu, May 02, 2019 at 06:48:46PM +0530, Anshuman Khandual wrote: >> Drop the pgtable_t variable from all implementation for pte_fn_t as none of >> them use it. apply_to_pte_range() should stop computing it as well. Should >> help us save some cycles. > You didn't add Martin Schwidefsky for some reason. He introduced scripts/get_maintainer.pl did not list the email but anyways I should have added it from git blame. Thanks for adding his email to the thread.
Re: [PATCH] mm/pgtable: Drop pgtable_t variable from pte_fn_t functions
On Thu, May 02, 2019 at 06:48:46PM +0530, Anshuman Khandual wrote:
> Drop the pgtable_t variable from all implementation for pte_fn_t as none of
> them use it. apply_to_pte_range() should stop computing it as well. Should
> help us save some cycles.
You didn't add Martin Schwidefsky for some reason. He introduced
it originally for s390 for sub-page page tables back in 2008 (commit
2f569afd9c). I think he should confirm that he no longer needs it.
> ---
> - Boot tested on arm64 and x86 platforms.
> - Build tested on multiple platforms with their defconfig
>
> arch/arm/kernel/efi.c | 3 +--
> arch/arm/mm/dma-mapping.c | 3 +--
> arch/arm/mm/pageattr.c | 3 +--
> arch/arm64/kernel/efi.c| 3 +--
> arch/arm64/mm/pageattr.c | 3 +--
> arch/x86/xen/mmu_pv.c | 3 +--
> drivers/gpu/drm/i915/i915_mm.c | 3 +--
> drivers/xen/gntdev.c | 6 ++
> drivers/xen/privcmd.c | 6 ++
> drivers/xen/xlate_mmu.c| 3 +--
> include/linux/mm.h | 3 +--
> mm/memory.c| 5 +
> mm/vmalloc.c | 2 +-
> 13 files changed, 15 insertions(+), 31 deletions(-)
>
> diff --git a/arch/arm/kernel/efi.c b/arch/arm/kernel/efi.c
> index 9f43ba012d10..b1f142a01f2f 100644
> --- a/arch/arm/kernel/efi.c
> +++ b/arch/arm/kernel/efi.c
> @@ -11,8 +11,7 @@
> #include
> #include
>
> -static int __init set_permissions(pte_t *ptep, pgtable_t token,
> - unsigned long addr, void *data)
> +static int __init set_permissions(pte_t *ptep, unsigned long addr, void
> *data)
> {
> efi_memory_desc_t *md = data;
> pte_t pte = *ptep;
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 43f46aa7ef33..739286511a18 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -496,8 +496,7 @@ void __init dma_contiguous_remap(void)
> }
> }
>
> -static int __dma_update_pte(pte_t *pte, pgtable_t token, unsigned long addr,
> - void *data)
> +static int __dma_update_pte(pte_t *pte, unsigned long addr, void *data)
> {
> struct page *page = virt_to_page(addr);
> pgprot_t prot = *(pgprot_t *)data;
> diff --git a/arch/arm/mm/pageattr.c b/arch/arm/mm/pageattr.c
> index 1403cb4a0c3d..c8b500940e1f 100644
> --- a/arch/arm/mm/pageattr.c
> +++ b/arch/arm/mm/pageattr.c
> @@ -22,8 +22,7 @@ struct page_change_data {
> pgprot_t clear_mask;
> };
>
> -static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long
> addr,
> - void *data)
> +static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> {
> struct page_change_data *cdata = data;
> pte_t pte = *ptep;
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 4f9acb5fbe97..230cff073a08 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -86,8 +86,7 @@ int __init efi_create_mapping(struct mm_struct *mm,
> efi_memory_desc_t *md)
> return 0;
> }
>
> -static int __init set_permissions(pte_t *ptep, pgtable_t token,
> - unsigned long addr, void *data)
> +static int __init set_permissions(pte_t *ptep, unsigned long addr, void
> *data)
> {
> efi_memory_desc_t *md = data;
> pte_t pte = READ_ONCE(*ptep);
> diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
> index 6cd645edcf35..0be077628b21 100644
> --- a/arch/arm64/mm/pageattr.c
> +++ b/arch/arm64/mm/pageattr.c
> @@ -27,8 +27,7 @@ struct page_change_data {
>
> bool rodata_full __ro_after_init =
> IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED);
>
> -static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long
> addr,
> - void *data)
> +static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
> {
> struct page_change_data *cdata = data;
> pte_t pte = READ_ONCE(*ptep);
> diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
> index a21e1734fc1f..308a6195fd26 100644
> --- a/arch/x86/xen/mmu_pv.c
> +++ b/arch/x86/xen/mmu_pv.c
> @@ -2702,8 +2702,7 @@ struct remap_data {
> struct mmu_update *mmu_update;
> };
>
> -static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token,
> - unsigned long addr, void *data)
> +static int remap_area_pfn_pte_fn(pte_t *ptep, unsigned long addr, void *data)
> {
> struct remap_data *rmd = data;
> pte_t pte = pte_mkspecial(mfn_pte(*rmd->pfn, rmd->prot));
> diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
> index e4935dd1fd37..c23bb29e6d3e 100644
> --- a/drivers/gpu/drm/i915/i915_mm.c
> +++ b/drivers/gpu/drm/i915/i915_mm.c
> @@ -35,8 +35,7 @@ struct remap_pfn {
> pgprot_t prot;
> };
>
> -static int remap_pfn(pte_t *pte, pgtable_t token,
> - unsigned long addr, void *data)
> +static int remap_pfn(pte_t *pte, unsigned long addr, void *data)
> {
> str
[PATCH] mm/pgtable: Drop pgtable_t variable from pte_fn_t functions
Drop the pgtable_t variable from all implementation for pte_fn_t as none of
them use it. apply_to_pte_range() should stop computing it as well. Should
help us save some cycles.
Signed-off-by: Anshuman Khandual
Cc: Ard Biesheuvel
Cc: Russell King
Cc: Catalin Marinas
Cc: Will Deacon
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Andrew Morton
Cc: Michal Hocko
Cc: Matthew Wilcox
Cc: Logan Gunthorpe
Cc: "Kirill A. Shutemov"
Cc: Dan Williams
Cc:
Cc: Mike Rapoport
Cc: x...@kernel.org
Cc: linux-efi@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-ker...@vger.kernel.org
Cc: xen-de...@lists.xenproject.org
Cc: intel-...@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org
Cc: linux...@kvack.org
---
- Boot tested on arm64 and x86 platforms.
- Build tested on multiple platforms with their defconfig
arch/arm/kernel/efi.c | 3 +--
arch/arm/mm/dma-mapping.c | 3 +--
arch/arm/mm/pageattr.c | 3 +--
arch/arm64/kernel/efi.c| 3 +--
arch/arm64/mm/pageattr.c | 3 +--
arch/x86/xen/mmu_pv.c | 3 +--
drivers/gpu/drm/i915/i915_mm.c | 3 +--
drivers/xen/gntdev.c | 6 ++
drivers/xen/privcmd.c | 6 ++
drivers/xen/xlate_mmu.c| 3 +--
include/linux/mm.h | 3 +--
mm/memory.c| 5 +
mm/vmalloc.c | 2 +-
13 files changed, 15 insertions(+), 31 deletions(-)
diff --git a/arch/arm/kernel/efi.c b/arch/arm/kernel/efi.c
index 9f43ba012d10..b1f142a01f2f 100644
--- a/arch/arm/kernel/efi.c
+++ b/arch/arm/kernel/efi.c
@@ -11,8 +11,7 @@
#include
#include
-static int __init set_permissions(pte_t *ptep, pgtable_t token,
- unsigned long addr, void *data)
+static int __init set_permissions(pte_t *ptep, unsigned long addr, void *data)
{
efi_memory_desc_t *md = data;
pte_t pte = *ptep;
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 43f46aa7ef33..739286511a18 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -496,8 +496,7 @@ void __init dma_contiguous_remap(void)
}
}
-static int __dma_update_pte(pte_t *pte, pgtable_t token, unsigned long addr,
- void *data)
+static int __dma_update_pte(pte_t *pte, unsigned long addr, void *data)
{
struct page *page = virt_to_page(addr);
pgprot_t prot = *(pgprot_t *)data;
diff --git a/arch/arm/mm/pageattr.c b/arch/arm/mm/pageattr.c
index 1403cb4a0c3d..c8b500940e1f 100644
--- a/arch/arm/mm/pageattr.c
+++ b/arch/arm/mm/pageattr.c
@@ -22,8 +22,7 @@ struct page_change_data {
pgprot_t clear_mask;
};
-static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
- void *data)
+static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
{
struct page_change_data *cdata = data;
pte_t pte = *ptep;
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index 4f9acb5fbe97..230cff073a08 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -86,8 +86,7 @@ int __init efi_create_mapping(struct mm_struct *mm,
efi_memory_desc_t *md)
return 0;
}
-static int __init set_permissions(pte_t *ptep, pgtable_t token,
- unsigned long addr, void *data)
+static int __init set_permissions(pte_t *ptep, unsigned long addr, void *data)
{
efi_memory_desc_t *md = data;
pte_t pte = READ_ONCE(*ptep);
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 6cd645edcf35..0be077628b21 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -27,8 +27,7 @@ struct page_change_data {
bool rodata_full __ro_after_init =
IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED);
-static int change_page_range(pte_t *ptep, pgtable_t token, unsigned long addr,
- void *data)
+static int change_page_range(pte_t *ptep, unsigned long addr, void *data)
{
struct page_change_data *cdata = data;
pte_t pte = READ_ONCE(*ptep);
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index a21e1734fc1f..308a6195fd26 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -2702,8 +2702,7 @@ struct remap_data {
struct mmu_update *mmu_update;
};
-static int remap_area_pfn_pte_fn(pte_t *ptep, pgtable_t token,
-unsigned long addr, void *data)
+static int remap_area_pfn_pte_fn(pte_t *ptep, unsigned long addr, void *data)
{
struct remap_data *rmd = data;
pte_t pte = pte_mkspecial(mfn_pte(*rmd->pfn, rmd->prot));
diff --git a/drivers/gpu/drm/i915/i915_mm.c b/drivers/gpu/drm/i915/i915_mm.c
index e4935dd1fd37..c23bb29e6d3e 100644
--- a/drivers/gpu/drm/i915/i915_mm.c
+++ b/drivers/gpu/drm/i915/i915_mm.c
@@ -35,8 +35,7 @@ struct remap_pfn {
pgprot_t prot;
};
-static int remap_pfn(pte_t *pte, pgtable_t token,
-unsigned long addr, void *data)
+static
Re: [PATCH 2/2 v3] efi: print appropriate status message when loading certificates
On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi wrote:
>
> When loading certificates list from UEFI variable, the original error
> message direct shows the efi status code from UEFI firmware. It looks
> ugly:
>
> [2.335031] Couldn't get size: 0x800e
> [2.335032] Couldn't get UEFI MokListRT
> [2.339985] Couldn't get size: 0x800e
> [2.339987] Couldn't get UEFI dbx list
>
> So, this patch shows the status string instead of status code.
>
> On the other hand, the "Couldn't get UEFI" message doesn't need
> to be exposed when db/dbx/mok variable do not exist. So, this
> patch set the message level to debug.
>
> v3.
> - Print messages similar to db/mok when loading dbx hash to blacklist:
> [1.500952] EFI: Blacklisting hash of an executable: UEFI:dbx
> [1.501773] blacklist: Loaded blacklisting hash
> 'bin:80b4d96931bf0d02fd91a61e19d14f1da452e66db2408ca8604d411f92659f0a'
>
> - Setting messages for the existence of db/mok/dbx lists to debug level.
>
> v2.
> Setting the MODSIGN messages level to debug.
>
> Link:
> https://forums.opensuse.org/showthread.php/535324-MODSIGN-Couldn-t-get-UEFI-db-list?p=2897516#post2897516
> Cc: James Morris
> Cc: Serge E. Hallyn"
> Cc: David Howells
> Cc: Nayna Jain
> Cc: Josh Boyer
> Cc: Mimi Zohar
> Signed-off-by: "Lee, Chun-Yi"
> ---
> certs/blacklist.c | 3 +-
> security/integrity/platform_certs/load_uefi.c | 40
> +++
> 2 files changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index 3a507b9e2568..f91437e39e44 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c
> @@ -100,7 +100,8 @@ int mark_hash_blacklisted(const char *hash)
> if (IS_ERR(key)) {
> pr_err("Problem blacklisting hash (%ld)\n", PTR_ERR(key));
> return PTR_ERR(key);
> - }
> + } else
> + pr_notice("Loaded blacklisting hash '%s'\n", hash);
> return 0;
> }
>
> diff --git a/security/integrity/platform_certs/load_uefi.c
> b/security/integrity/platform_certs/load_uefi.c
> index 81b19c52832b..6b6996e5bc27 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -1,5 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
>
> +#define pr_fmt(fmt) "EFI: "fmt
> +
> #include
> #include
> #include
> @@ -35,6 +37,18 @@ static __init bool uefi_check_ignore_db(void)
> return status == EFI_SUCCESS;
> }
>
> +static void str16_to_str(efi_char16_t *str16, char *str, int str_size)
> +{
> + int i = 0;
> +
> + while (str16[i] != '\0' && i < (str_size - 1)) {
> + str[i] = str16[i];
> + i++;
> + }
> +
> + str[i] = '\0';
> +}
> +
> /*
> * Get a certificate list blob from the named EFI variable.
> */
> @@ -44,13 +58,20 @@ static __init void *get_cert_list(efi_char16_t *name,
> efi_guid_t *guid,
> efi_status_t status;
> unsigned long lsize = 4;
> unsigned long tmpdb[4];
> + char namestr[16];
> void *db;
>
> + str16_to_str(name, namestr, ARRAY_SIZE(namestr));
Please drop this (and the function above) - instead, just return NULL
if the variable is not found (without reporting an error).
> status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
> if (status != EFI_BUFFER_TOO_SMALL) {
> - pr_err("Couldn't get size: 0x%lx\n", status);
> + if (status == EFI_NOT_FOUND)
> + pr_debug("UEFI %s list doesn't exist\n", namestr);
> + else
> + pr_err("Couldn't get size for UEFI %s list: %s\n",
> + namestr, efi_status_to_str(status));
> return NULL;
> }
> + pr_debug("UEFI %s list exists\n", namestr);
>
> db = kmalloc(lsize, GFP_KERNEL);
> if (!db)
> @@ -59,7 +80,8 @@ static __init void *get_cert_list(efi_char16_t *name,
> efi_guid_t *guid,
> status = efi.get_variable(name, guid, NULL, &lsize, db);
> if (status != EFI_SUCCESS) {
> kfree(db);
> - pr_err("Error reading db var: 0x%lx\n", status);
> + pr_err("Error reading UEFI %s list: %s\n",
> + namestr, efi_status_to_str(status));
> return NULL;
> }
>
> @@ -95,6 +117,7 @@ static __init void uefi_blacklist_hash(const char *source,
> const void *data,
> static __init void uefi_blacklist_x509_tbs(const char *source,
>const void *data, size_t len)
> {
> + pr_info("Blacklisting X.509 TBS hash: %s\n", source);
> uefi_blacklist_hash(source, data, len, "tbs:", 4);
> }
>
> @@ -104,6 +127,7 @@ static __init void uefi_blacklist_x509_tbs(const char
> *source,
> static __init void uefi_blacklist_binary(const char *source,
> const void *d
Re: [PATCH 1/2 v2] efi: add a function to convert the status value to string
On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi wrote:
>
> This function can be used to convert EFI status value to string
> for printing out debug message. Using this function can improve
> the readability of log.
>
> v2.
Please move the changelog out of the commit log (move it below the ---
further down)
> - Changed the wording in subject and description.
> - Moved the marco immediately after the status value definitions.
> - Turned into a proper function instead of inline.
>
You missed my point here. A proper function means the function in a .c
file, and only the declaration in a .h file. This way, you are still
duplicating the literal strings into every object file that references
this function.
> Cc: Ard Biesheuvel
> Cc: Kees Cook
> Cc: Anton Vorontsov
> Cc: Colin Cross
> Cc: Tony Luck
> Signed-off-by: "Lee, Chun-Yi"
> ---
> include/linux/efi.h | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 54357a258b35..6f3f89a32eef 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -42,6 +42,34 @@
> #define EFI_ABORTED(21 | (1UL << (BITS_PER_LONG-1)))
> #define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1)))
>
> +#define EFI_STATUS_STR(_status) \
> +case EFI_##_status: \
> + return "EFI_" __stringify(_status);
> +
> +static __attribute__((unused)) char *
> +efi_status_to_str(unsigned long status)
> +{
> + switch (status) {
> + EFI_STATUS_STR(SUCCESS)
> + EFI_STATUS_STR(LOAD_ERROR)
> + EFI_STATUS_STR(INVALID_PARAMETER)
> + EFI_STATUS_STR(UNSUPPORTED)
> + EFI_STATUS_STR(BAD_BUFFER_SIZE)
> + EFI_STATUS_STR(BUFFER_TOO_SMALL)
> + EFI_STATUS_STR(NOT_READY)
> + EFI_STATUS_STR(DEVICE_ERROR)
> + EFI_STATUS_STR(WRITE_PROTECTED)
> + EFI_STATUS_STR(OUT_OF_RESOURCES)
> + EFI_STATUS_STR(NOT_FOUND)
> + EFI_STATUS_STR(ABORTED)
> + EFI_STATUS_STR(SECURITY_VIOLATION)
> + default:
> + pr_warn("Unknown efi status: 0x%lx", status);
> + }
> +
> + return "Unknown efi status";
> +}
> +
> typedef unsigned long efi_status_t;
> typedef u8 efi_bool_t;
> typedef u16 efi_char16_t; /* UNICODE character */
> --
> 2.16.4
>
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
On Tue, Apr 30, 2019 at 03:07:09PM +0200, Bartosz Szczepanek wrote:
> I may be a little late with this comment, but I've just tested these
> patches on aarch64 platform (from the top of jjs/master) and got
> kernel panic ("Unable to handle kernel read", full log at the end of
> mail). I think there's problem with below call to
> tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> passed as (void *) and never remapped:
Not late. This is not part of any PR yet. Thank you for the
feedback!
Matthew, can you send an updated version of the whole patch set
with fixes to this issue and also reordering of the includes?
/Jarkko
Re: [PATCH V5 2/4] tpm: Reserve the TPM final events table
(+ Ingo)
On Tue, 30 Apr 2019 at 21:52, Matthew Garrett wrote:
>
> On Tue, Apr 30, 2019 at 6:07 AM Bartosz Szczepanek wrote:
> >
> > I may be a little late with this comment, but I've just tested these
> > patches on aarch64 platform (from the top of jjs/master) and got
> > kernel panic ("Unable to handle kernel read", full log at the end of
> > mail). I think there's problem with below call to
> > tpm2_calc_event_log_size(), where physical address of efi.tpm_log is
> > passed as (void *) and never remapped:
>
> Yes, it looks like this is just broken. Can you try with the attached patch?
I'm a bit uncomfortable with EFI code that is obviously broken and
untested being queued for the next merge window in another tree.
What is currently queued there? Can we revert this change for the time
being, and resubmit it via the EFI tree for v5.3?
