Re: [PATCH v3] x86, efi: never relocate kernel below lowest acceptable address

2019-10-16 Thread Joe Perches
On Wed, 2019-10-16 at 17:48 +0200, Borislav Petkov wrote:
> On Wed, Oct 16, 2019 at 08:23:56AM -0700, Joe Perches wrote:
> > ?  examples please.
> 
> From this very thread:
> 
> \sEfi\s, \sefi\s, \seFI\s etc should be "EFI"
> 
> I'm thinking perhaps start conservatively and catch the most often
> misspelled ones in commit messages or comments. "CPU", "SMT", "MCE",
> "MCA", "PCI" etc come to mind.
> 
> > checkpatch has a db for misspellings, I supposed another for
> > acronyms could be added,
> 
> Doesn't have to be another one - established acronyms are part of the
> dictionary too.

Couldn't work.  The dictionary is case insensitive.




[PATCH 4.9 76/92] efivar/ssdt: Dont iterate over EFI vars if no SSDT override was specified

2019-10-16 Thread Greg Kroah-Hartman
From: Ard Biesheuvel 

commit c05f8f92b701576b615f30aac31fabdc0648649b upstream.

The kernel command line option efivar_ssdt= allows the name to be
specified of an EFI variable containing an ACPI SSDT table that should
be loaded into memory by the OS, and treated as if it was provided by
the firmware.

Currently, that code will always iterate over the EFI variables and
compare each name with the provided name, even if the command line
option wasn't set to begin with.

So bail early when no variable name was provided. This works around a
boot regression on the 2012 Mac Pro, as reported by Scott.

Tested-by: Scott Talbert 
Signed-off-by: Ard Biesheuvel 
Cc:  # v4.9+
Cc: Ben Dooks 
Cc: Dave Young 
Cc: Jarkko Sakkinen 
Cc: Jerry Snitselaar 
Cc: Linus Torvalds 
Cc: Lukas Wunner 
Cc: Lyude Paul 
Cc: Matthew Garrett 
Cc: Octavian Purdila 
Cc: Peter Jones 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: linux-efi@vger.kernel.org
Cc: linux-integr...@vger.kernel.org
Fixes: 475fb4e8b2f4 ("efi / ACPI: load SSTDs from EFI variables")
Link: https://lkml.kernel.org/r/20191002165904.8819-3-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/firmware/efi/efi.c |3 +++
 1 file changed, 3 insertions(+)

--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -243,6 +243,9 @@ static __init int efivar_ssdt_load(void)
void *data;
int ret;
 
+   if (!efivar_ssdt[0])
+   return 0;
+
ret = efivar_init(efivar_ssdt_iter, , true, );
 
list_for_each_entry_safe(entry, aux, , list) {




[PATCH 4.14 43/65] efivar/ssdt: Dont iterate over EFI vars if no SSDT override was specified

2019-10-16 Thread Greg Kroah-Hartman
From: Ard Biesheuvel 

commit c05f8f92b701576b615f30aac31fabdc0648649b upstream.

The kernel command line option efivar_ssdt= allows the name to be
specified of an EFI variable containing an ACPI SSDT table that should
be loaded into memory by the OS, and treated as if it was provided by
the firmware.

Currently, that code will always iterate over the EFI variables and
compare each name with the provided name, even if the command line
option wasn't set to begin with.

So bail early when no variable name was provided. This works around a
boot regression on the 2012 Mac Pro, as reported by Scott.

Tested-by: Scott Talbert 
Signed-off-by: Ard Biesheuvel 
Cc:  # v4.9+
Cc: Ben Dooks 
Cc: Dave Young 
Cc: Jarkko Sakkinen 
Cc: Jerry Snitselaar 
Cc: Linus Torvalds 
Cc: Lukas Wunner 
Cc: Lyude Paul 
Cc: Matthew Garrett 
Cc: Octavian Purdila 
Cc: Peter Jones 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: linux-efi@vger.kernel.org
Cc: linux-integr...@vger.kernel.org
Fixes: 475fb4e8b2f4 ("efi / ACPI: load SSTDs from EFI variables")
Link: https://lkml.kernel.org/r/20191002165904.8819-3-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/firmware/efi/efi.c |3 +++
 1 file changed, 3 insertions(+)

--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -266,6 +266,9 @@ static __init int efivar_ssdt_load(void)
void *data;
int ret;
 
+   if (!efivar_ssdt[0])
+   return 0;
+
ret = efivar_init(efivar_ssdt_iter, , true, );
 
list_for_each_entry_safe(entry, aux, , list) {




[PATCH 5.3 063/112] efi/tpm: Dont access event->count when it isnt mapped

2019-10-16 Thread Greg Kroah-Hartman
From: Peter Jones 

commit 047d50aee341d940350897c85799e56ae57c3849 upstream.

Some machines generate a lot of event log entries.  When we're
iterating over them, the code removes the old mapping and adds a
new one, so once we cross the page boundary we're unmapping the page
with the count on it.  Hilarity ensues.

This patch keeps the info from the header in local variables so we don't
need to access that page again or keep track of if it's mapped.

Tested-by: Lyude Paul 
Signed-off-by: Peter Jones 
Signed-off-by: Jarkko Sakkinen 
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Jarkko Sakkinen 
Acked-by: Matthew Garrett 
Acked-by: Ard Biesheuvel 
Cc: Ben Dooks 
Cc: Dave Young 
Cc: Jerry Snitselaar 
Cc: Linus Torvalds 
Cc: Lukas Wunner 
Cc: Octavian Purdila 
Cc: Peter Zijlstra 
Cc: Scott Talbert 
Cc: Thomas Gleixner 
Cc: linux-efi@vger.kernel.org
Cc: linux-integr...@vger.kernel.org
Cc: sta...@vger.kernel.org
Fixes: 44038bc514a2 ("tpm: Abstract crypto agile event size calculations")
Link: https://lkml.kernel.org/r/20191002165904.8819-4-ard.biesheu...@linaro.org
[ Minor edits. ]
Signed-off-by: Ingo Molnar 
Signed-off-by: Greg Kroah-Hartman 

---
 include/linux/tpm_eventlog.h |   14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -170,6 +170,7 @@ static inline int __calc_tpm2_event_size
u16 halg;
int i;
int j;
+   u32 count, event_type;
 
marker = event;
marker_start = marker;
@@ -190,16 +191,22 @@ static inline int __calc_tpm2_event_size
}
 
event = (struct tcg_pcr_event2_head *)mapping;
+   /*
+* The loop below will unmap these fields if the log is larger than
+* one page, so save them here for reference:
+*/
+   count = READ_ONCE(event->count);
+   event_type = READ_ONCE(event->event_type);
 
efispecid = (struct tcg_efi_specid_event_head *)event_header->event;
 
/* Check if event is malformed. */
-   if (event->count > efispecid->num_algs) {
+   if (count > efispecid->num_algs) {
size = 0;
goto out;
}
 
-   for (i = 0; i < event->count; i++) {
+   for (i = 0; i < count; i++) {
halg_size = sizeof(event->digests[i].alg_id);
 
/* Map the digest's algorithm identifier */
@@ -256,8 +263,9 @@ static inline int __calc_tpm2_event_size
+ event_field->event_size;
size = marker - marker_start;
 
-   if ((event->event_type == 0) && (event_field->event_size == 0))
+   if (event_type == 0 && event_field->event_size == 0)
size = 0;
+
 out:
if (do_mapping)
TPM_MEMUNMAP(mapping, mapping_size);




[PATCH 5.3 065/112] efi/tpm: Only set efi_tpm_final_log_size after successful event log parsing

2019-10-16 Thread Greg Kroah-Hartman
From: Jerry Snitselaar 

commit e658c82be5561412c5e83b5e74e9da4830593f3e upstream.

If __calc_tpm2_event_size() fails to parse an event it will return 0,
resulting tpm2_calc_event_log_size() returning -1. Currently there is
no check of this return value, and 'efi_tpm_final_log_size' can end up
being set to this negative value resulting in a crash like this one:

  BUG: unable to handle page fault for address: bc8fc00866ad
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x) - not-present page

  RIP: 0010:memcpy_erms+0x6/0x10
  Call Trace:
   tpm_read_log_efi()
   tpm_bios_log_setup()
   tpm_chip_register()
   tpm_tis_core_init.cold.9+0x28c/0x466
   tpm_tis_plat_probe()
   platform_drv_probe()
   ...

Also __calc_tpm2_event_size() returns a size of 0 when it fails
to parse an event, so update function documentation to reflect this.

The root cause of the issue that caused the failure of event parsing
in this case is resolved by Peter Jone's patchset dealing with large
event logs where crossing over a page boundary causes the page with
the event count to be unmapped.

Signed-off-by: Jerry Snitselaar 
Signed-off-by: Ard Biesheuvel 
Cc: Ben Dooks 
Cc: Dave Young 
Cc: Jarkko Sakkinen 
Cc: Linus Torvalds 
Cc: Lukas Wunner 
Cc: Lyude Paul 
Cc: Matthew Garrett 
Cc: Octavian Purdila 
Cc: Peter Jones 
Cc: Peter Zijlstra 
Cc: Scott Talbert 
Cc: Thomas Gleixner 
Cc: linux-efi@vger.kernel.org
Cc: linux-integr...@vger.kernel.org
Cc: sta...@vger.kernel.org
Fixes: c46f3405692de ("tpm: Reserve the TPM final events table")
Link: https://lkml.kernel.org/r/20191002165904.8819-6-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/firmware/efi/tpm.c   |9 -
 include/linux/tpm_eventlog.h |2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -85,11 +85,18 @@ int __init efi_tpm_eventlog_init(void)
final_tbl->nr_events,
log_tbl->log);
}
+
+   if (tbl_size < 0) {
+   pr_err(FW_BUG "Failed to parse event in TPM Final Events 
Log\n");
+   goto out_calc;
+   }
+
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;
 
+out_calc:
+   early_memunmap(final_tbl, sizeof(*final_tbl));
 out:
early_memunmap(log_tbl, sizeof(*log_tbl));
return ret;
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -152,7 +152,7 @@ struct tcg_algorithm_info {
  * total. Once we've done this we know the offset of the data length field,
  * and can calculate the total size of the event.
  *
- * Return: size of the event on success, <0 on failure
+ * Return: size of the event on success, 0 on failure
  */
 
 static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,




[PATCH 5.3 064/112] efi/tpm: Dont traverse an event log with no events

2019-10-16 Thread Greg Kroah-Hartman
From: Peter Jones 

commit 05c8c1ff81ed2eb9bad7c27cf92e55c864c16df8 upstream.

When there are no entries to put into the final event log, some machines
will return the template they would have populated anyway.  In this case
the nr_events field is 0, but the rest of the log is just garbage.

This patch stops us from trying to iterate the table with
__calc_tpm2_event_size() when the number of events in the table is 0.

Tested-by: Lyude Paul 
Signed-off-by: Peter Jones 
Signed-off-by: Jarkko Sakkinen 
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Jarkko Sakkinen 
Acked-by: Matthew Garrett 
Acked-by: Ard Biesheuvel 
Cc: Ben Dooks 
Cc: Dave Young 
Cc: Jerry Snitselaar 
Cc: Linus Torvalds 
Cc: Lukas Wunner 
Cc: Octavian Purdila 
Cc: Peter Zijlstra 
Cc: Scott Talbert 
Cc: Thomas Gleixner 
Cc: linux-efi@vger.kernel.org
Cc: linux-integr...@vger.kernel.org
Cc: sta...@vger.kernel.org
Fixes: c46f3405692d ("tpm: Reserve the TPM final events table")
Link: https://lkml.kernel.org/r/20191002165904.8819-5-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/firmware/efi/tpm.c |   15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -75,11 +75,16 @@ int __init efi_tpm_eventlog_init(void)
goto out;
}
 
-   tbl_size = tpm2_calc_event_log_size((void *)efi.tpm_final_log
-   + sizeof(final_tbl->version)
-   + sizeof(final_tbl->nr_events),
-   final_tbl->nr_events,
-   log_tbl->log);
+   tbl_size = 0;
+   if (final_tbl->nr_events != 0) {
+   void *events = (void *)efi.tpm_final_log
+   + sizeof(final_tbl->version)
+   + sizeof(final_tbl->nr_events);
+
+   tbl_size = tpm2_calc_event_log_size(events,
+   final_tbl->nr_events,
+   log_tbl->log);
+   }
memblock_reserve((unsigned long)final_tbl,
 tbl_size + sizeof(*final_tbl));
early_memunmap(final_tbl, sizeof(*final_tbl));




[PATCH 5.3 062/112] efivar/ssdt: Dont iterate over EFI vars if no SSDT override was specified

2019-10-16 Thread Greg Kroah-Hartman
From: Ard Biesheuvel 

commit c05f8f92b701576b615f30aac31fabdc0648649b upstream.

The kernel command line option efivar_ssdt= allows the name to be
specified of an EFI variable containing an ACPI SSDT table that should
be loaded into memory by the OS, and treated as if it was provided by
the firmware.

Currently, that code will always iterate over the EFI variables and
compare each name with the provided name, even if the command line
option wasn't set to begin with.

So bail early when no variable name was provided. This works around a
boot regression on the 2012 Mac Pro, as reported by Scott.

Tested-by: Scott Talbert 
Signed-off-by: Ard Biesheuvel 
Cc:  # v4.9+
Cc: Ben Dooks 
Cc: Dave Young 
Cc: Jarkko Sakkinen 
Cc: Jerry Snitselaar 
Cc: Linus Torvalds 
Cc: Lukas Wunner 
Cc: Lyude Paul 
Cc: Matthew Garrett 
Cc: Octavian Purdila 
Cc: Peter Jones 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: linux-efi@vger.kernel.org
Cc: linux-integr...@vger.kernel.org
Fixes: 475fb4e8b2f4 ("efi / ACPI: load SSTDs from EFI variables")
Link: https://lkml.kernel.org/r/20191002165904.8819-3-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/firmware/efi/efi.c |3 +++
 1 file changed, 3 insertions(+)

--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -282,6 +282,9 @@ static __init int efivar_ssdt_load(void)
void *data;
int ret;
 
+   if (!efivar_ssdt[0])
+   return 0;
+
ret = efivar_init(efivar_ssdt_iter, , true, );
 
list_for_each_entry_safe(entry, aux, , list) {




Re: [PATCH v3] x86, efi: never relocate kernel below lowest acceptable address

2019-10-16 Thread Joe Perches
On Wed, 2019-10-16 at 19:27 +0300, Jarkko Sakkinen wrote:
> On Wed, Oct 16, 2019 at 08:23:56AM -0700, Joe Perches wrote:
> > On Wed, 2019-10-16 at 18:20 +0300, Jarkko Sakkinen wrote: > > On Mon, Oct 
> > 14, 2019 at 11:18:25PM +0200, Borislav Petkov wrote:
> > > > On Mon, Oct 14, 2019 at 11:21:11PM +0300, Jarkko Sakkinen wrote:
> > > > > Was there a section in the patch submission documentation to point out
> > > > > when people send patches with all the possible twists for an acronym?
> > > > 
> > > > I don't think so.
> > > > 
> > > > > This is giving me constantly gray hairs with TPM patches.
> > > > 
> > > > Well, I'm slowly getting tired of repeating the same crap over and over
> > > > again about how important it is to document one's changes and to write
> > > > good commit messages. The most repeated answers I'm simply putting into
> > > > canned reply templates because, well, saying it once or twice is not
> > > > enough anymore. :-\
> > > > 
> > > > And yeah, I see your pain. Same here, actually.
> > > > 
> > > > In the acronym case, I'd probably add a regex to my patch massaging
> > > > script and convert those typos automatically and be done with it.
> > > 
> > > Wonder if checkpatch.pl could be extended to know acronyms e.g. have a
> > > db of known acronyms.
> > 
> > ?  examples please.
> > 
> > checkpatch has a db for misspellings, I supposed another for
> > acronyms could be added, but how would false positives be avoided?
> 
> TPM should be always TPM, e.g. not tpm. EFI should be always, e.g.
> not efi.

I think it's not possible to distinguish between
proper and improper uses.  For instance:

$ git grep -w tpm | wc -l
328
$ git grep -w TPM | wc -l
566

$ git grep -w efi | wc -l
851
$ git grep -w EFI | wc -l
915




Re: [PATCH v3] x86, efi: never relocate kernel below lowest acceptable address

2019-10-16 Thread Jarkko Sakkinen
On Wed, Oct 16, 2019 at 08:23:56AM -0700, Joe Perches wrote:
> On Wed, 2019-10-16 at 18:20 +0300, Jarkko Sakkinen wrote: > > On Mon, Oct 14, 
> 2019 at 11:18:25PM +0200, Borislav Petkov wrote:
> > > On Mon, Oct 14, 2019 at 11:21:11PM +0300, Jarkko Sakkinen wrote:
> > > > Was there a section in the patch submission documentation to point out
> > > > when people send patches with all the possible twists for an acronym?
> > > 
> > > I don't think so.
> > > 
> > > > This is giving me constantly gray hairs with TPM patches.
> > > 
> > > Well, I'm slowly getting tired of repeating the same crap over and over
> > > again about how important it is to document one's changes and to write
> > > good commit messages. The most repeated answers I'm simply putting into
> > > canned reply templates because, well, saying it once or twice is not
> > > enough anymore. :-\
> > > 
> > > And yeah, I see your pain. Same here, actually.
> > > 
> > > In the acronym case, I'd probably add a regex to my patch massaging
> > > script and convert those typos automatically and be done with it.
> > 
> > Wonder if checkpatch.pl could be extended to know acronyms e.g. have a
> > db of known acronyms.
> 
> ?  examples please.
> 
> checkpatch has a db for misspellings, I supposed another for
> acronyms could be added, but how would false positives be avoided?

TPM should be always TPM, e.g. not tpm. EFI should be always, e.g.
not efi.

/Jarkko


Re: [PATCH v3] x86, efi: never relocate kernel below lowest acceptable address

2019-10-16 Thread Borislav Petkov
On Wed, Oct 16, 2019 at 08:23:56AM -0700, Joe Perches wrote:
> ?  examples please.

>From this very thread:

\sEfi\s, \sefi\s, \seFI\s etc should be "EFI"

I'm thinking perhaps start conservatively and catch the most often
misspelled ones in commit messages or comments. "CPU", "SMT", "MCE",
"MCA", "PCI" etc come to mind.

> checkpatch has a db for misspellings, I supposed another for
> acronyms could be added,

Doesn't have to be another one - established acronyms are part of the
dictionary too.

> but how would false positives be avoided?

Perhaps delimited with spaces or non-word chars (\W) and when they're
part of a comment or the commit message...

-- 
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette


Re: [PATCH v7 00/12] EFI Specific Purpose Memory Support

2019-10-16 Thread Dan Williams
On Tue, Oct 15, 2019 at 11:55 PM Ard Biesheuvel
 wrote:
>
> On Wed, 16 Oct 2019 at 03:13, Dan Williams  wrote:
> >
> > Changes since v6 [1]:
> > - Collect Ard's ack / review on patches 5-7, but not on patch 4 since it
> >   needed a non-trivial rework for linker error reported by the 0day robot.
> >
> > - Fixup "efi: Common enable/disable infrastructure for EFI soft
> >   reservation" with a new dependency on CONFIG_EFI_STUB for
> >   CONFIG_EFI_SOFT_RESERVE since the efi_soft_reserve_enabled() helper is
> >   only built with EFI_STUB=y and the support depends on early reservations
> >   to keep the kernel text from landing in the reservation.
>
> As far as I know, GRUB on x86 still boots without the EFI stub by
> default (i.e., using the 'linux' command instead of the 'linuxefi'
> command), so even if you build the stub, it is not going to be called
> in many cases. Is that going to be a problem?

It only becomes a problem if kaslr decides to land the kernel on top
of the soft-reservation. However, I think it's ok to say that if you
need the reservation to be honored in all circumstances, arrange to
boot in EFI mode.

>
> > This also
> >   moved the IS_ENABLED(CONFIG_EFI_SOFT_RESERVE) check into the header so
> >   that the stub does not try to link to __efi_soft_reserve_enabled() in
> >   the EFI_STUB=n case.
> >
> > - Rework "x86/efi: EFI soft reservation to E820 enumeration" to always
> >   add the full EFI memory map when EFI_MEMORY_SP ranges are found. This
> >   simplifies the logic to just add the full EFI map rather than try to
> >   tease out just the EFI_MEMORY_SP ranges. (Ard)
> >
> > [1]: 
> > https://lore.kernel.org/lkml/157066227329.1059972.5659620631541203458.st...@dwillia2-desk3.amr.corp.intel.com/
> >
> > ---
> > Merge notes:
> >
> > Hi Ingo,
> >
> > I'm still looking for Ard's ack on the revised patch 4, but otherwise
> > feel like this is ready for your consideration.
> >
>
> Patch 4 looks fine to me,
>
> Reviewed-by: Ard Biesheuvel 

Thanks for the help.


Re: [PATCH v3] x86, efi: never relocate kernel below lowest acceptable address

2019-10-16 Thread Joe Perches
On Wed, 2019-10-16 at 18:20 +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 14, 2019 at 11:18:25PM +0200, Borislav Petkov wrote:
> > On Mon, Oct 14, 2019 at 11:21:11PM +0300, Jarkko Sakkinen wrote:
> > > Was there a section in the patch submission documentation to point out
> > > when people send patches with all the possible twists for an acronym?
> > 
> > I don't think so.
> > 
> > > This is giving me constantly gray hairs with TPM patches.
> > 
> > Well, I'm slowly getting tired of repeating the same crap over and over
> > again about how important it is to document one's changes and to write
> > good commit messages. The most repeated answers I'm simply putting into
> > canned reply templates because, well, saying it once or twice is not
> > enough anymore. :-\
> > 
> > And yeah, I see your pain. Same here, actually.
> > 
> > In the acronym case, I'd probably add a regex to my patch massaging
> > script and convert those typos automatically and be done with it.
> 
> Wonder if checkpatch.pl could be extended to know acronyms e.g. have a
> db of known acronyms.

?  examples please.

checkpatch has a db for misspellings, I supposed another for
acronyms could be added, but how would false positives be avoided?




Re: [PATCH v3] x86, efi: never relocate kernel below lowest acceptable address

2019-10-16 Thread Jarkko Sakkinen
On Mon, Oct 14, 2019 at 11:18:25PM +0200, Borislav Petkov wrote:
> On Mon, Oct 14, 2019 at 11:21:11PM +0300, Jarkko Sakkinen wrote:
> > Was there a section in the patch submission documentation to point out
> > when people send patches with all the possible twists for an acronym?
> 
> I don't think so.
> 
> > This is giving me constantly gray hairs with TPM patches.
> 
> Well, I'm slowly getting tired of repeating the same crap over and over
> again about how important it is to document one's changes and to write
> good commit messages. The most repeated answers I'm simply putting into
> canned reply templates because, well, saying it once or twice is not
> enough anymore. :-\
> 
> And yeah, I see your pain. Same here, actually.
> 
> In the acronym case, I'd probably add a regex to my patch massaging
> script and convert those typos automatically and be done with it.

Wonder if checkpatch.pl could be extended to know acronyms e.g. have a
db of known acronyms.

/Jarkko


Re: [PATCH v3 0/3] x86/boot: Introduce the kernel_info et consortes

2019-10-16 Thread Daniel Kiper
On Wed, Oct 09, 2019 at 12:53:55PM +0200, Daniel Kiper wrote:
> Hi,
>
> Due to very limited space in the setup_header this patch series introduces new
> kernel_info struct which will be used to convey information from the kernel to
> the bootloader. This way the boot protocol can be extended regardless of the
> setup_header limitations. Additionally, the patch series introduces some
> convenience features like the setup_indirect struct and the
> kernel_info.setup_type_max field.
>
> Daniel
>
>  Documentation/x86/boot.rst | 168 
> ++
>  arch/x86/boot/Makefile |   2 +-
>  arch/x86/boot/compressed/Makefile  |   4 +-
>  arch/x86/boot/compressed/kaslr.c   |  12 ++
>  arch/x86/boot/compressed/kernel_info.S |  22 +++
>  arch/x86/boot/header.S |   3 +-
>  arch/x86/boot/tools/build.c|   5 +++
>  arch/x86/include/uapi/asm/bootparam.h  |  16 +++-
>  arch/x86/kernel/e820.c |  11 ++
>  arch/x86/kernel/kdebugfs.c |  20 --
>  arch/x86/kernel/ksysfs.c   |  30 ++
>  arch/x86/kernel/setup.c|   4 ++
>  arch/x86/mm/ioremap.c  |  11 ++
>  13 files changed, 292 insertions(+), 16 deletions(-)
>
> Daniel Kiper (3):
>   x86/boot: Introduce the kernel_info
>   x86/boot: Introduce the kernel_info.setup_type_max
>   x86/boot: Introduce the setup_indirect

hpa, ping?

Daniel


Re: [PATCH] efi/tpm: return -EINVAL when determining tpm final events log size fails

2019-10-16 Thread Jarkko Sakkinen
On Mon, Oct 14, 2019 at 10:21:45AM -0700, Jerry Snitselaar wrote:
> Currently nothing checks the return value of efi_tpm_eventlog_init,
> but in case that changes in the future make sure an error is
> returned when it fails to determine the tpm final events log
> size.
> 
> Cc: Ard Biesheuvel 
> Cc: Jarkko Sakkinen 
> Cc: linux-efi@vger.kernel.org
> Cc: linux-integr...@vger.kernel.org
> Fixes: e658c82be556 ("efi/tpm: Only set 'efi_tpm_final_log_size' after 
> successful event log parsing")
> Suggested-by: Dan Carpenter 
> Signed-off-by: Jerry Snitselaar 

Reviewed-by: Jarkko Sakkinen 

/Jarkko


Re: [PATCH v7 00/12] EFI Specific Purpose Memory Support

2019-10-16 Thread Ard Biesheuvel
On Wed, 16 Oct 2019 at 03:13, Dan Williams  wrote:
>
> Changes since v6 [1]:
> - Collect Ard's ack / review on patches 5-7, but not on patch 4 since it
>   needed a non-trivial rework for linker error reported by the 0day robot.
>
> - Fixup "efi: Common enable/disable infrastructure for EFI soft
>   reservation" with a new dependency on CONFIG_EFI_STUB for
>   CONFIG_EFI_SOFT_RESERVE since the efi_soft_reserve_enabled() helper is
>   only built with EFI_STUB=y and the support depends on early reservations
>   to keep the kernel text from landing in the reservation.

As far as I know, GRUB on x86 still boots without the EFI stub by
default (i.e., using the 'linux' command instead of the 'linuxefi'
command), so even if you build the stub, it is not going to be called
in many cases. Is that going to be a problem?

> This also
>   moved the IS_ENABLED(CONFIG_EFI_SOFT_RESERVE) check into the header so
>   that the stub does not try to link to __efi_soft_reserve_enabled() in
>   the EFI_STUB=n case.
>
> - Rework "x86/efi: EFI soft reservation to E820 enumeration" to always
>   add the full EFI memory map when EFI_MEMORY_SP ranges are found. This
>   simplifies the logic to just add the full EFI map rather than try to
>   tease out just the EFI_MEMORY_SP ranges. (Ard)
>
> [1]: 
> https://lore.kernel.org/lkml/157066227329.1059972.5659620631541203458.st...@dwillia2-desk3.amr.corp.intel.com/
>
> ---
> Merge notes:
>
> Hi Ingo,
>
> I'm still looking for Ard's ack on the revised patch 4, but otherwise
> feel like this is ready for your consideration.
>

Patch 4 looks fine to me,

Reviewed-by: Ard Biesheuvel 


> ---
>
> The EFI 2.8 Specification [2] introduces the EFI_MEMORY_SP ("specific
> purpose") memory attribute. This attribute bit replaces the deprecated
> ACPI HMAT "reservation hint" that was introduced in ACPI 6.2 and removed
> in ACPI 6.3.
>
> Given the increasing diversity of memory types that might be advertised
> to the operating system, there is a need for platform firmware to hint
> which memory ranges are free for the OS to use as general purpose memory
> and which ranges are intended for application specific usage. For
> example, an application with prior knowledge of the platform may expect
> to be able to exclusively allocate a precious / limited pool of high
> bandwidth memory. Alternatively, for the general purpose case, the
> operating system may want to make the memory available on a best effort
> basis as a unique numa-node with performance properties by the new
> CONFIG_HMEM_REPORTING [3] facility.
>
> In support of optionally allowing either application-exclusive and
> core-kernel-mm managed access to differentiated memory, claim
> EFI_MEMORY_SP ranges for exposure as "soft reserved" and assigned to a
> device-dax instance by default. Such instances can be directly owned /
> mapped by a platform-topology-aware application. Alternatively, with the
> new kmem facility [4], the administrator has the option to instead
> designate that those memory ranges be hot-added to the core-kernel-mm as
> a unique memory numa-node. In short, allow for the decision about what
> software agent manages soft-reserved memory to be made at runtime.
>
> The patches build on the new HMAT+HMEM_REPORTING facilities merged
> for v5.2-rc1. The implementation is tested with qemu emulation of HMAT
> [5] plus the efi_fake_mem facility for applying the EFI_MEMORY_SP
> attribute. Specific details on reproducing the test configuration are in
> patch 12.
>
> [2]: https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_final.pdf
> [3]: 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e1cf33aafb84
> [4]: 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c221c0b0308f
> [5]: http://patchwork.ozlabs.org/cover/1096737/
>
> ---
>
> Dan Williams (12):
>   acpi/numa: Establish a new drivers/acpi/numa/ directory
>   efi: Enumerate EFI_MEMORY_SP
>   x86/efi: Push EFI_MEMMAP check into leaf routines
>   efi: Common enable/disable infrastructure for EFI soft reservation
>   x86/efi: EFI soft reservation to E820 enumeration
>   arm/efi: EFI soft reservation to memblock
>   x86/efi: Add efi_fake_mem support for EFI_MEMORY_SP
>   lib: Uplevel the pmem "region" ida to a global allocator
>   dax: Fix alloc_dax_region() compile warning
>   device-dax: Add a driver for "hmem" devices
>   acpi/numa/hmat: Register HMAT at device_initcall level
>   acpi/numa/hmat: Register "soft reserved" memory as an "hmem" device
>
>
>  Documentation/admin-guide/kernel-parameters.txt |   19 +++
>  arch/arm64/mm/mmu.c |2
>  arch/x86/boot/compressed/eboot.c|6 +
>  arch/x86/boot/compressed/kaslr.c|   46 +++-
>  arch/x86/include/asm/e820/types.h   |8 +
>  arch/x86/include/asm/efi.h  |   17 +++
>  arch/x86/kernel/e820.c  |