| Message ID | 20240306155511.974517-3-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Preserve TPM log across kexec | expand |
Stefan Berger <stefanb@linux.ibm.com> writes: > If linux,sml-log is available use it to get the TPM log rather than the > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > on Power where after a kexec the memory pointed to by linux,sml-base may > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > linux,sml-size on these two platforms. It would be good to mention that powernv platforms (sometimes called Open Power or bare metal) still use linux,sml-base/size, via skiboot. cheers > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- > 1 file changed, 11 insertions(+), 25 deletions(-) > > diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c > index 930fe43d5daf..e37196e64ef1 100644 > --- a/drivers/char/tpm/eventlog/of.c > +++ b/drivers/char/tpm/eventlog/of.c > @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > const u32 *sizep; > const u64 *basep; > struct tpm_bios_log *log; > + const void *logp; > u32 size; > - u64 base; > > log = &chip->log; > if (chip->dev.parent && chip->dev.parent->of_node) > @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) > if (of_property_read_bool(np, "powered-while-suspended")) > chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; > > - sizep = of_get_property(np, "linux,sml-size", NULL); > - basep = of_get_property(np, "linux,sml-base", NULL); > - if (sizep == NULL && basep == NULL) > - return tpm_read_log_memory_region(chip); > - if (sizep == NULL || basep == NULL) > - return -EIO; > - > - /* > - * For both vtpm/tpm, firmware has log addr and log size in big > - * endian format. But in case of vtpm, there is a method called > - * sml-handover which is run during kernel init even before > - * device tree is setup. This sml-handover function takes care > - * of endianness and writes to sml-base and sml-size in little > - * endian format. For this reason, vtpm doesn't need conversion > - * but physical tpm needs the conversion. > - */ > - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && > - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { > + logp = of_get_property(np, "linux,sml-log", &size); > + if (logp == NULL) { > + sizep = of_get_property(np, "linux,sml-size", NULL); > + basep = of_get_property(np, "linux,sml-base", NULL); > + if (sizep == NULL && basep == NULL) > + return tpm_read_log_memory_region(chip); > + if (sizep == NULL || basep == NULL) > + return -EIO; > + logp = __va(be64_to_cpup((__force __be64 *)basep)); > size = be32_to_cpup((__force __be32 *)sizep); > - base = be64_to_cpup((__force __be64 *)basep); > - } else { > - size = *sizep; > - base = *basep; > } > - > if (size == 0) { > dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); > return -EIO; > } > > - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); > + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); > if (!log->bios_event_log) > return -ENOMEM; > > -- > 2.43.0
On Wed, Mar 06, 2024 at 10:55:11AM -0500, Stefan Berger wrote: > If linux,sml-log is available use it to get the TPM log rather than the > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > on Power where after a kexec the memory pointed to by linux,sml-base may > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > linux,sml-size on these two platforms. Those two properties are documented, but linux,sml-log is not, nor can I find patches on the list documenting it. There should be a patch adding this to tmp-common.yaml. Cheers, Conor. > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- > 1 file changed, 11 insertions(+), 25 deletions(-) > > diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c > index 930fe43d5daf..e37196e64ef1 100644 > --- a/drivers/char/tpm/eventlog/of.c > +++ b/drivers/char/tpm/eventlog/of.c > @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > const u32 *sizep; > const u64 *basep; > struct tpm_bios_log *log; > + const void *logp; > u32 size; > - u64 base; > > log = &chip->log; > if (chip->dev.parent && chip->dev.parent->of_node) > @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) > if (of_property_read_bool(np, "powered-while-suspended")) > chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; > > - sizep = of_get_property(np, "linux,sml-size", NULL); > - basep = of_get_property(np, "linux,sml-base", NULL); > - if (sizep == NULL && basep == NULL) > - return tpm_read_log_memory_region(chip); > - if (sizep == NULL || basep == NULL) > - return -EIO; > - > - /* > - * For both vtpm/tpm, firmware has log addr and log size in big > - * endian format. But in case of vtpm, there is a method called > - * sml-handover which is run during kernel init even before > - * device tree is setup. This sml-handover function takes care > - * of endianness and writes to sml-base and sml-size in little > - * endian format. For this reason, vtpm doesn't need conversion > - * but physical tpm needs the conversion. > - */ > - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && > - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { > + logp = of_get_property(np, "linux,sml-log", &size); > + if (logp == NULL) { > + sizep = of_get_property(np, "linux,sml-size", NULL); > + basep = of_get_property(np, "linux,sml-base", NULL); > + if (sizep == NULL && basep == NULL) > + return tpm_read_log_memory_region(chip); > + if (sizep == NULL || basep == NULL) > + return -EIO; > + logp = __va(be64_to_cpup((__force __be64 *)basep)); > size = be32_to_cpup((__force __be32 *)sizep); > - base = be64_to_cpup((__force __be64 *)basep); > - } else { > - size = *sizep; > - base = *basep; > } > - > if (size == 0) { > dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); > return -EIO; > } > > - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); > + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); > if (!log->bios_event_log) > return -ENOMEM; > > -- > 2.43.0 >
On 3/7/24 05:42, Michael Ellerman wrote: > Stefan Berger <stefanb@linux.ibm.com> writes: >> If linux,sml-log is available use it to get the TPM log rather than the >> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >> on Power where after a kexec the memory pointed to by linux,sml-base may >> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >> linux,sml-size on these two platforms. > > It would be good to mention that powernv platforms (sometimes called > Open Power or bare metal) still use linux,sml-base/size, via skiboot. Will do in v2.
in short summary: s/Use/use/ On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > If linux,sml-log is available use it to get the TPM log rather than the > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > on Power where after a kexec the memory pointed to by linux,sml-base may > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > linux,sml-size on these two platforms. > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> So shouldn't this have a fixed tag, or not? > --- > drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- > 1 file changed, 11 insertions(+), 25 deletions(-) > > diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c > index 930fe43d5daf..e37196e64ef1 100644 > --- a/drivers/char/tpm/eventlog/of.c > +++ b/drivers/char/tpm/eventlog/of.c > @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) > const u32 *sizep; > const u64 *basep; > struct tpm_bios_log *log; > + const void *logp; > u32 size; > - u64 base; > > log = &chip->log; > if (chip->dev.parent && chip->dev.parent->of_node) > @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) > if (of_property_read_bool(np, "powered-while-suspended")) > chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; > > - sizep = of_get_property(np, "linux,sml-size", NULL); > - basep = of_get_property(np, "linux,sml-base", NULL); > - if (sizep == NULL && basep == NULL) > - return tpm_read_log_memory_region(chip); > - if (sizep == NULL || basep == NULL) > - return -EIO; > - > - /* > - * For both vtpm/tpm, firmware has log addr and log size in big > - * endian format. But in case of vtpm, there is a method called > - * sml-handover which is run during kernel init even before > - * device tree is setup. This sml-handover function takes care > - * of endianness and writes to sml-base and sml-size in little > - * endian format. For this reason, vtpm doesn't need conversion > - * but physical tpm needs the conversion. > - */ > - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && > - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { > + logp = of_get_property(np, "linux,sml-log", &size); > + if (logp == NULL) { > + sizep = of_get_property(np, "linux,sml-size", NULL); > + basep = of_get_property(np, "linux,sml-base", NULL); > + if (sizep == NULL && basep == NULL) > + return tpm_read_log_memory_region(chip); > + if (sizep == NULL || basep == NULL) > + return -EIO; > + logp = __va(be64_to_cpup((__force __be64 *)basep)); > size = be32_to_cpup((__force __be32 *)sizep); > - base = be64_to_cpup((__force __be64 *)basep); > - } else { > - size = *sizep; > - base = *basep; > } > - > if (size == 0) { > dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); > return -EIO; > } > > - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); > + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); > if (!log->bios_event_log) > return -ENOMEM; > Looks pretty good other than that. BR, Jarkko
On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: > in short summary: s/Use/use/ > > On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > > If linux,sml-log is available use it to get the TPM log rather than the > > pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > > on Power where after a kexec the memory pointed to by linux,sml-base may > > have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > > linux,sml-size on these two platforms. > > > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > > So shouldn't this have a fixed tag, or not? In English: do we want this to be backported to stable kernel releases or not? BR, Jarkko
On 3/7/24 15:00, Jarkko Sakkinen wrote: > On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: >> in short summary: s/Use/use/ >> >> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: >>> If linux,sml-log is available use it to get the TPM log rather than the >>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >>> on Power where after a kexec the memory pointed to by linux,sml-base may >>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >>> linux,sml-size on these two platforms. >>> >>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> >> So shouldn't this have a fixed tag, or not? > > In English: do we want this to be backported to stable kernel releases or not? Ideally, yes. v3 will have 3 patches and all 3 of them will have to be backported *together* and not applied otherwise if any one of them fails. Can this be 'guaranteed'? > > BR, Jarkko
On Fri Mar 8, 2024 at 2:17 PM EET, Stefan Berger wrote: > > > On 3/7/24 15:00, Jarkko Sakkinen wrote: > > On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: > >> in short summary: s/Use/use/ > >> > >> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > >>> If linux,sml-log is available use it to get the TPM log rather than the > >>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > >>> on Power where after a kexec the memory pointed to by linux,sml-base may > >>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > >>> linux,sml-size on these two platforms. > >>> > >>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > >> > >> So shouldn't this have a fixed tag, or not? > > > > In English: do we want this to be backported to stable kernel releases or not? > > Ideally, yes. v3 will have 3 patches and all 3 of them will have to be > backported *together* and not applied otherwise if any one of them > fails. Can this be 'guaranteed'? All of them will end up to stable if the following conditions hold: - All have a fixes tag. - All have "Cc: stable@vger.kernel.org". - We agree in the review process that they are all legit fixes. BR, Jarkko
Stefan Berger <stefanb@linux.ibm.com> writes: > On 3/7/24 15:00, Jarkko Sakkinen wrote: >> On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: >>> in short summary: s/Use/use/ >>> >>> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: >>>> If linux,sml-log is available use it to get the TPM log rather than the >>>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >>>> on Power where after a kexec the memory pointed to by linux,sml-base may >>>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >>>> linux,sml-size on these two platforms. >>>> >>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >>> >>> So shouldn't this have a fixed tag, or not? >> >> In English: do we want this to be backported to stable kernel releases or not? > > Ideally, yes. v3 will have 3 patches and all 3 of them will have to be > backported *together* and not applied otherwise if any one of them > fails. Can this be 'guaranteed'? You can use Depends-on: <previous commit SHA> to indicate the relationship. cheers
On Tue Mar 12, 2024 at 12:35 PM EET, Michael Ellerman wrote: > Stefan Berger <stefanb@linux.ibm.com> writes: > > On 3/7/24 15:00, Jarkko Sakkinen wrote: > >> On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: > >>> in short summary: s/Use/use/ > >>> > >>> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: > >>>> If linux,sml-log is available use it to get the TPM log rather than the > >>>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM > >>>> on Power where after a kexec the memory pointed to by linux,sml-base may > >>>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and > >>>> linux,sml-size on these two platforms. > >>>> > >>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > >>> > >>> So shouldn't this have a fixed tag, or not? > >> > >> In English: do we want this to be backported to stable kernel releases or not? > > > > Ideally, yes. v3 will have 3 patches and all 3 of them will have to be > > backported *together* and not applied otherwise if any one of them > > fails. Can this be 'guaranteed'? > > You can use Depends-on: <previous commit SHA> to indicate the relationship. > > cheers Thanks, I've missed depends-on tag. Stefan, please add also "Cc: stable@vger.kernel.org" just to make sure that I don't forget to add it. Right, and since these are so small scoped commits, and bug fixes in particular, it is also possible to do PR during the release cycle. BR, Jarkko
On 3/12/24 11:50, Jarkko Sakkinen wrote: > On Tue Mar 12, 2024 at 12:35 PM EET, Michael Ellerman wrote: >> Stefan Berger <stefanb@linux.ibm.com> writes: >>> On 3/7/24 15:00, Jarkko Sakkinen wrote: >>>> On Thu Mar 7, 2024 at 9:57 PM EET, Jarkko Sakkinen wrote: >>>>> in short summary: s/Use/use/ >>>>> >>>>> On Wed Mar 6, 2024 at 5:55 PM EET, Stefan Berger wrote: >>>>>> If linux,sml-log is available use it to get the TPM log rather than the >>>>>> pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM >>>>>> on Power where after a kexec the memory pointed to by linux,sml-base may >>>>>> have been corrupted. Also, linux,sml-log has replaced linux,sml-base and >>>>>> linux,sml-size on these two platforms. >>>>>> >>>>>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >>>>> >>>>> So shouldn't this have a fixed tag, or not? >>>> >>>> In English: do we want this to be backported to stable kernel releases or not? >>> >>> Ideally, yes. v3 will have 3 patches and all 3 of them will have to be >>> backported *together* and not applied otherwise if any one of them >>> fails. Can this be 'guaranteed'? >> >> You can use Depends-on: <previous commit SHA> to indicate the relationship. >> >> cheers > > Thanks, I've missed depends-on tag. > > Stefan, please add also "Cc: stable@vger.kernel.org" just to make sure > that I don't forget to add it. Yeah, once we know whether this is the way forward or not... I posted v2 as RFC to figure this out. v2's 2/3 patch will only apply to 6.8. To avoid any inconsistencies between code and bindings we cannot even go further back with this series (IFF it's the way forward at all). So I am inclined to remove the Fixes tags. I also find little under Documentation about the Depends-on tag and what it's supposed to be formatted like -- a commit hash of 1/3 appearing in 2/3 for example? The commit hash is not stable at this point so I couldn't created it. > Right, and since these are so small scoped commits, and bug fixes in > particular, it is also possible to do PR during the release cycle. > > BR, Jarkko
diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c index 930fe43d5daf..e37196e64ef1 100644 --- a/drivers/char/tpm/eventlog/of.c +++ b/drivers/char/tpm/eventlog/of.c @@ -54,8 +54,8 @@ int tpm_read_log_of(struct tpm_chip *chip) const u32 *sizep; const u64 *basep; struct tpm_bios_log *log; + const void *logp; u32 size; - u64 base; log = &chip->log; if (chip->dev.parent && chip->dev.parent->of_node) @@ -66,37 +66,23 @@ int tpm_read_log_of(struct tpm_chip *chip) if (of_property_read_bool(np, "powered-while-suspended")) chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED; - sizep = of_get_property(np, "linux,sml-size", NULL); - basep = of_get_property(np, "linux,sml-base", NULL); - if (sizep == NULL && basep == NULL) - return tpm_read_log_memory_region(chip); - if (sizep == NULL || basep == NULL) - return -EIO; - - /* - * For both vtpm/tpm, firmware has log addr and log size in big - * endian format. But in case of vtpm, there is a method called - * sml-handover which is run during kernel init even before - * device tree is setup. This sml-handover function takes care - * of endianness and writes to sml-base and sml-size in little - * endian format. For this reason, vtpm doesn't need conversion - * but physical tpm needs the conversion. - */ - if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0 && - of_property_match_string(np, "compatible", "IBM,vtpm20") < 0) { + logp = of_get_property(np, "linux,sml-log", &size); + if (logp == NULL) { + sizep = of_get_property(np, "linux,sml-size", NULL); + basep = of_get_property(np, "linux,sml-base", NULL); + if (sizep == NULL && basep == NULL) + return tpm_read_log_memory_region(chip); + if (sizep == NULL || basep == NULL) + return -EIO; + logp = __va(be64_to_cpup((__force __be64 *)basep)); size = be32_to_cpup((__force __be32 *)sizep); - base = be64_to_cpup((__force __be64 *)basep); - } else { - size = *sizep; - base = *basep; } - if (size == 0) { dev_warn(&chip->dev, "%s: Event log area empty\n", __func__); return -EIO; } - log->bios_event_log = devm_kmemdup(&chip->dev, __va(base), size, GFP_KERNEL); + log->bios_event_log = devm_kmemdup(&chip->dev, logp, size, GFP_KERNEL); if (!log->bios_event_log) return -ENOMEM;
If linux,sml-log is available use it to get the TPM log rather than the pointer found in linux,sml-base. This resolves an issue on PowerVM and KVM on Power where after a kexec the memory pointed to by linux,sml-base may have been corrupted. Also, linux,sml-log has replaced linux,sml-base and linux,sml-size on these two platforms. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- drivers/char/tpm/eventlog/of.c | 36 +++++++++++----------------------- 1 file changed, 11 insertions(+), 25 deletions(-)