Re: char/tpm: Improve a size determination in nine functions
On Wed Oct 18 17, SF Markus Elfring wrote: For 1/4 and 2/4: explain why the message can be omitted. Why did you not reply directly with this request for the update steps with the subject “Delete an error message for a failed memory allocation in tpm_…()”? https://patchwork.kernel.org/patch/10009405/ https://patchwork.kernel.org/patch/10009415/ I find that there can be difficulty to show an appropriate information source for the reasonable explanation of this change pattern. Shouldn't this information source for the explanation be the submitter? I'd hope they understand what it is they are submitting. Remove sentence about Coccinelle. I got the impression that there is a bit of value in such a kind of attribution. That's all. I assume that there might be also some communication challenges involved. 3/4: definitive NAK, too much noise compared to value. I tried to reduce deviations from the Linux coding style again. You do not like such an attempt for this software area so far. 4/4: this a good commit message. Why did you not reply directly with this feedback for the update step “[PATCH 4/4] char/tpm: Less checks in tpm_ibmvtpm_probe() after error detection”? https://patchwork.kernel.org/patch/10009429/ https://lkml.kernel.org/r/<09a2c3a1-1b10-507d-a866-258b570f6...@users.sourceforge.net> Requires a Tested-by before can be accepted, which I'm not able to give. I am curious on how this detail will evolve. Regards, Markus
Re: [PATCHv2] tpm: ibmvtpm: Wait for ready buffer before probing for TPM2 attributes
On Fri Jun 19 20, David Gibson wrote:
The tpm2_get_cc_attrs_tbl() call will result in TPM commands being issued,
which will need the use of the internal command/response buffer. But,
we're issuing this *before* we've waited to make sure that buffer is
allocated.
This can result in intermittent failures to probe if the hypervisor / TPM
implementation doesn't respond quickly enough. I find it fails almost
every time with an 8 vcpu guest under KVM with software emulated TPM.
To fix it, just move the tpm2_get_cc_attrs_tlb() call after the
existing code to wait for initialization, which will ensure the buffer
is allocated.
Fixes: 18b3670d79ae9 ("tpm: ibmvtpm: Add support for TPM2")
Signed-off-by: David Gibson
---
Reviewed-by: Jerry Snitselaar
Changes from v1:
* Fixed a formatting error in the commit message
* Added some more detail to the commit message
drivers/char/tpm/tpm_ibmvtpm.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 09fe45246b8cc..994385bf37c0c 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -683,13 +683,6 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
if (rc)
goto init_irq_cleanup;
- if (!strcmp(id->compat, "IBM,vtpm20")) {
- chip->flags |= TPM_CHIP_FLAG_TPM2;
- rc = tpm2_get_cc_attrs_tbl(chip);
- if (rc)
- goto init_irq_cleanup;
- }
-
if (!wait_event_timeout(ibmvtpm->crq_queue.wq,
ibmvtpm->rtce_buf != NULL,
HZ)) {
@@ -697,6 +690,13 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
goto init_irq_cleanup;
}
+ if (!strcmp(id->compat, "IBM,vtpm20")) {
+ chip->flags |= TPM_CHIP_FLAG_TPM2;
+ rc = tpm2_get_cc_attrs_tbl(chip);
+ if (rc)
+ goto init_irq_cleanup;
+ }
+
return tpm_chip_register(chip);
init_irq_cleanup:
do {
--
2.26.2
Re: [PATCH 00/13] iommu: Remove usage of dev->archdata.iommu
On Thu Jun 25 20, Joerg Roedel wrote: From: Joerg Roedel Hi, here is a patch-set to remove the usage of dev->archdata.iommu from the IOMMU code in the kernel and replace its uses by the iommu per-device private data field. The changes also remove the field entirely from the architectures which no longer need it. On PowerPC the field is called dev->archdata.iommu_domain and was only used by the PAMU IOMMU driver. It gets removed as well. The patches have been runtime tested on Intel VT-d and compile tested with allyesconfig for: * x86 (32 and 64 bit) * arm and arm64 * ia64 (only drivers/ because build failed for me in arch/ia64) * PPC64 Besides that the changes also survived my IOMMU tree compile tests. Please review. Regards, Joerg Joerg Roedel (13): iommu/exynos: Use dev_iommu_priv_get/set() iommu/vt-d: Use dev_iommu_priv_get/set() iommu/msm: Use dev_iommu_priv_get/set() iommu/omap: Use dev_iommu_priv_get/set() iommu/rockchip: Use dev_iommu_priv_get/set() iommu/tegra: Use dev_iommu_priv_get/set() iommu/pamu: Use dev_iommu_priv_get/set() iommu/mediatek: Do no use dev->archdata.iommu x86: Remove dev->archdata.iommu pointer ia64: Remove dev->archdata.iommu pointer arm: Remove dev->archdata.iommu pointer arm64: Remove dev->archdata.iommu pointer powerpc/dma: Remove dev->archdata.iommu_domain arch/arm/include/asm/device.h | 3 --- arch/arm64/include/asm/device.h | 3 --- arch/ia64/include/asm/device.h| 3 --- arch/powerpc/include/asm/device.h | 3 --- arch/x86/include/asm/device.h | 3 --- .../gpu/drm/i915/selftests/mock_gem_device.c | 10 -- drivers/iommu/exynos-iommu.c | 20 +-- drivers/iommu/fsl_pamu_domain.c | 8 drivers/iommu/intel/iommu.c | 18 - drivers/iommu/msm_iommu.c | 4 ++-- drivers/iommu/mtk_iommu.h | 2 ++ drivers/iommu/mtk_iommu_v1.c | 10 -- drivers/iommu/omap-iommu.c| 20 +-- drivers/iommu/rockchip-iommu.c| 8 drivers/iommu/tegra-gart.c| 8 drivers/iommu/tegra-smmu.c| 8 .../media/platform/s5p-mfc/s5p_mfc_iommu.h| 4 +++- 17 files changed, 64 insertions(+), 71 deletions(-) -- 2.27.0 Reviewed-by: Jerry Snitselaar
Re: [RFC PATCH v2 1/3] powerpc/prom_init: Replace linux,sml-base/sml-size with linux,sml-log
On Mon, Mar 11, 2024 at 09:20:28AM -0400, Stefan Berger wrote:
> linux,sml-base holds the address of a buffer with the TPM log. This
> buffer may become invalid after a kexec. To avoid accessing an invalid
> address or corrupted buffer, embed the whole TPM log in the device tree
> property linux,sml-log. This helps to protect the log since it is
> properly carried across a kexec soft reboot with both of the kexec
> syscalls.
>
> Avoid having the firmware ingest the whole TPM log when calling
> prom_setprop but only create the linux,sml-log property as a place holder.
> Insert the actual TPM log during the tree flattening phase.
>
> Fixes: 4a727429abec ("PPC64: Add support for instantiating SML from Open
> Firmware")
> Suggested-by: Michael Ellerman
> Signed-off-by: Stefan Berger
> ---
> arch/powerpc/kernel/prom_init.c | 27 +++
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index e67effdba85c..6f7ca72013c2 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -211,6 +211,8 @@ static cell_t __prombss regbuf[1024];
>
> static bool __prombss rtas_has_query_cpu_stopped;
>
> +static u64 __prombss sml_base;
> +static u32 __prombss sml_size;
Should inside an #ifdef CONFIG_PPC64 since prom_instantiate_sml() is?
>
> /*
> * Error results ... some OF calls will return "-1" on error, some
> @@ -1954,17 +1956,15 @@ static void __init prom_instantiate_sml(void)
> }
> prom_printf(" done\n");
>
> - reserve_mem(base, size);
> -
> - prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-base",
> - &base, sizeof(base));
> - prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-size",
> - &size, sizeof(size));
> -
> - prom_debug("sml base = 0x%llx\n", base);
> + /* Add property now, defer adding log to tree flattening phase */
> + prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-log",
> + NULL, 0);
> prom_debug("sml size = 0x%x\n", size);
>
> prom_debug("prom_instantiate_sml: end...\n");
> +
> + sml_base = base;
> + sml_size = size;
> }
>
> /*
> @@ -2645,6 +2645,17 @@ static void __init scan_dt_build_struct(phandle node,
> unsigned long *mem_start,
> }
> prev_name = sstart + soff;
>
> + if (!prom_strcmp("linux,sml-log", pname)) {
> + /* push property head */
> + dt_push_token(OF_DT_PROP, mem_start, mem_end);
> + dt_push_token(sml_size, mem_start, mem_end);
> + dt_push_token(soff, mem_start, mem_end);
> + /* push property content */
> + valp = make_room(mem_start, mem_end, sml_size, 1);
> + memcpy(valp, (void *)sml_base, sml_size);
> + *mem_start = ALIGN(*mem_start, 4);
> + continue;
> + }
Same question as above.
Regards,
Jerry
> /* get length */
> l = call_prom("getproplen", 2, 1, node, pname);
>
> --
> 2.43.0
>
Re: [PATCH v7 01/24] iommu: Add iommu_ops->identity_domain
On Wed, Aug 23, 2023 at 01:47:15PM -0300, Jason Gunthorpe wrote: > This allows a driver to set a global static to an IDENTITY domain and > the core code will automatically use it whenever an IDENTITY domain > is requested. > > By making it always available it means the IDENTITY can be used in error > handling paths to force the iommu driver into a known state. Devices > implementing global static identity domains should avoid failing their > attach_dev ops. > > To make global static domains simpler allow drivers to omit their free > function and update the iommufd selftest. > > Convert rockchip to use the new mechanism. > > Tested-by: Steven Price > Tested-by: Marek Szyprowski > Tested-by: Nicolin Chen > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 02/24] iommu: Add IOMMU_DOMAIN_PLATFORM
On Wed, Aug 23, 2023 at 01:47:16PM -0300, Jason Gunthorpe wrote:
> This is used when the iommu driver is taking control of the dma_ops,
> currently only on S390 and power spapr. It is designed to preserve the
> original ops->detach_dev() semantic that these S390 was built around.
>
> Provide an opaque domain type and a 'default_domain' ops value that allows
> the driver to trivially force any single domain as the default domain.
>
> Update iommufd selftest to use this instead of set_platform_dma_ops
>
> Reviewed-by: Lu Baolu
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/iommu/iommu.c| 13 +
> drivers/iommu/iommufd/selftest.c | 14 +-
> include/linux/iommu.h| 6 ++
> 3 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 33bd1107090720..7cedb0640290c8 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -184,6 +184,8 @@ static const char *iommu_domain_type_str(unsigned int t)
> case IOMMU_DOMAIN_DMA:
> case IOMMU_DOMAIN_DMA_FQ:
> return "Translated";
> + case IOMMU_DOMAIN_PLATFORM:
> + return "Platform";
> default:
> return "Unknown";
> }
> @@ -1752,6 +1754,17 @@ iommu_group_alloc_default_domain(struct iommu_group
> *group, int req_type)
>
> lockdep_assert_held(&group->mutex);
>
> + /*
> + * Allow legacy drivers to specify the domain that will be the default
> + * domain. This should always be either an IDENTITY or PLATFORM domain.
> + * Do not use in new drivers.
> + */
Would it be worthwhile to mention this in iommu.h for the iommu_ops
default_domain?
> + if (bus->iommu_ops->default_domain) {
> + if (req_type)
> + return ERR_PTR(-EINVAL);
> + return bus->iommu_ops->default_domain;
> + }
> +
> if (req_type)
> return __iommu_group_alloc_default_domain(bus, group, req_type);
>
> diff --git a/drivers/iommu/iommufd/selftest.c
> b/drivers/iommu/iommufd/selftest.c
> index d48a202a7c3b81..fb981ba97c4e87 100644
> --- a/drivers/iommu/iommufd/selftest.c
> +++ b/drivers/iommu/iommufd/selftest.c
> @@ -281,14 +281,6 @@ static bool mock_domain_capable(struct device *dev, enum
> iommu_cap cap)
> return cap == IOMMU_CAP_CACHE_COHERENCY;
> }
>
> -static void mock_domain_set_plaform_dma_ops(struct device *dev)
> -{
> - /*
> - * mock doesn't setup default domains because we can't hook into the
> - * normal probe path
> - */
> -}
> -
> static struct iommu_device mock_iommu_device = {
> };
>
> @@ -298,12 +290,16 @@ static struct iommu_device *mock_probe_device(struct
> device *dev)
> }
>
> static const struct iommu_ops mock_ops = {
> + /*
> + * IOMMU_DOMAIN_BLOCKED cannot be returned from def_domain_type()
> + * because it is zero.
> + */
> + .default_domain = &mock_blocking_domain,
> .owner = THIS_MODULE,
> .pgsize_bitmap = MOCK_IO_PAGE_SIZE,
> .hw_info = mock_domain_hw_info,
> .domain_alloc = mock_domain_alloc,
> .capable = mock_domain_capable,
> - .set_platform_dma_ops = mock_domain_set_plaform_dma_ops,
> .device_group = generic_device_group,
> .probe_device = mock_probe_device,
> .default_domain_ops =
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index d0920b2a9f1c0e..48a18b6e07abff 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -64,6 +64,7 @@ struct iommu_domain_geometry {
> #define __IOMMU_DOMAIN_DMA_FQ(1U << 3) /* DMA-API uses flush queue
> */
>
> #define __IOMMU_DOMAIN_SVA (1U << 4) /* Shared process address space */
> +#define __IOMMU_DOMAIN_PLATFORM (1U << 5)
>
> #define IOMMU_DOMAIN_ALLOC_FLAGS ~__IOMMU_DOMAIN_DMA_FQ
> /*
> @@ -81,6 +82,8 @@ struct iommu_domain_geometry {
> * invalidation.
> * IOMMU_DOMAIN_SVA- DMA addresses are shared process addresses
> * represented by mm_struct's.
> + * IOMMU_DOMAIN_PLATFORM - Legacy domain for drivers that do their own
> + * dma_api stuff. Do not use in new drivers.
> */
> #define IOMMU_DOMAIN_BLOCKED (0U)
> #define IOMMU_DOMAIN_IDENTITY(__IOMMU_DOMAIN_PT)
> @@ -91,6 +94,7 @@ struct iommu_domain_geometry {
>__IOMMU_DOMAIN_DMA_API | \
>__IOMMU_DOMAIN_DMA_FQ)
> #define IOMMU_DOMAIN_SVA (__IOMMU_DOMAIN_SVA)
> +#define IOMMU_DOMAIN_PLATFORM(__IOMMU_DOMAIN_PLATFORM)
>
> struct iommu_domain {
> unsigned type;
> @@ -262,6 +266,7 @@ struct iommu_iotlb_gather {
> * @owner: Driver module providing these ops
> * @identity_domain: An always available, always attachable identity
> * translation.
> + * @default_domain: If not NULL this will always be set as the
Re: [PATCH v7 02/24] iommu: Add IOMMU_DOMAIN_PLATFORM
On Fri, Aug 25, 2023 at 02:40:10PM -0300, Jason Gunthorpe wrote:
> On Thu, Aug 24, 2023 at 06:51:48PM -0700, Jerry Snitselaar wrote:
>
> > > + /*
> > > + * Allow legacy drivers to specify the domain that will be the default
> > > + * domain. This should always be either an IDENTITY or PLATFORM domain.
> > > + * Do not use in new drivers.
> > > + */
> >
> > Would it be worthwhile to mention this in iommu.h for the iommu_ops
> > default_domain?
>
> I did this:
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 11d47f9ac9b345..7fa53d28feca87 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1757,8 +1757,8 @@ iommu_group_alloc_default_domain(struct iommu_group
> *group, int req_type)
>
> /*
> * Allow legacy drivers to specify the domain that will be the default
> -* domain. This should always be either an IDENTITY or PLATFORM
> domain.
> -* Do not use in new drivers.
> +* domain. This should always be either an IDENTITY/BLOCKED/PLATFORM
> +* domain. Do not use in new drivers.
> */
> if (ops->default_domain) {
> if (req_type)
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 7e9d94a56f473e..6f9e0aacc4431a 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -267,6 +267,8 @@ struct iommu_iotlb_gather {
> * @blocked_domain: An always available, always attachable blocking
> * translation.
> * @default_domain: If not NULL this will always be set as the default
> domain.
> + * This should be an IDENTITY/BLOCKED/PLATFORM domain.
> + * Do not use in new drivers.
> */
> struct iommu_ops {
> bool (*capable)(struct device *dev, enum iommu_cap);
>
> Thanks,
> Jason
>
For all of 02/24
Reviewed-by: Jerry Snitselaar
> ___
> Linux-rockchip mailing list
> linux-rockc...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH v7 03/24] powerpc/iommu: Setup a default domain and remove set_platform_dma_ops
On Wed, Aug 23, 2023 at 01:47:17PM -0300, Jason Gunthorpe wrote: > POWER is using the set_platform_dma_ops() callback to hook up its private > dma_ops, but this is buired under some indirection and is weirdly > happening for a BLOCKED domain as well. > > For better documentation create a PLATFORM domain to manage the dma_ops, > since that is what it is for, and make the BLOCKED domain an alias for > it. BLOCKED is required for VFIO. > > Also removes the leaky allocation of the BLOCKED domain by using a global > static. > > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe > --- > arch/powerpc/kernel/iommu.c | 38 + > 1 file changed, 17 insertions(+), 21 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 04/24] iommu: Add IOMMU_DOMAIN_PLATFORM for S390
On Wed, Aug 23, 2023 at 01:47:18PM -0300, Jason Gunthorpe wrote: > The PLATFORM domain will be set as the default domain and attached as > normal during probe. The driver will ignore the initial attach from a NULL > domain to the PLATFORM domain. > > After this, the PLATFORM domain's attach_dev will be called whenever we > detach from an UNMANAGED domain (eg for VFIO). This is the same time the > original design would have called op->detach_dev(). > > This is temporary until the S390 dma-iommu.c conversion is merged. > > Tested-by: Heiko Stuebner > Tested-by: Niklas Schnelle > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 06/24] iommu/tegra-gart: Remove tegra-gart
On Wed, Aug 23, 2023 at 01:47:20PM -0300, Jason Gunthorpe wrote: > Thierry says this is not used anymore, and doesn't think it makes sense as > an iommu driver. The HW it supports is about 10 years old now and newer HW > uses different IOMMU drivers. > > As this is the only driver with a GART approach, and it doesn't really > meet the driver expectations from the IOMMU core, let's just remove it > so we don't have to think about how to make it fit in. > > It has a number of identified problems: > - The assignment of iommu_groups doesn't match the HW behavior > > - It claims to have an UNMANAGED domain but it is really an IDENTITY >domain with a translation aperture. This is inconsistent with the core >expectation for security sensitive operations > > - It doesn't implement a SW page table under struct iommu_domain so >* It can't accept a map until the domain is attached >* It forgets about all maps after the domain is detached >* It doesn't clear the HW of maps once the domain is detached > (made worse by having the wrong groups) > > Cc: Thierry Reding > Cc: Dmitry Osipenko > Acked-by: Thierry Reding > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 05/24] iommu/fsl_pamu: Implement a PLATFORM domain
On Wed, Aug 23, 2023 at 01:47:19PM -0300, Jason Gunthorpe wrote: > This driver is nonsensical. To not block migrating the core API away from > NULL default_domains give it a hacky of a PLATFORM domain that keeps it > working exactly as it always did. > > Leave some comments around to warn away any future people looking at this. > > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 07/24] iommu/mtk_iommu_v1: Implement an IDENTITY domain
On Wed, Aug 23, 2023 at 01:47:21PM -0300, Jason Gunthorpe wrote:
> What mtk does during mtk_iommu_v1_set_platform_dma() is actually putting
> the iommu into identity mode. Make this available as a proper IDENTITY
> domain.
>
> The mtk_iommu_v1_def_domain_type() from
> commit 8bbe13f52cb7 ("iommu/mediatek-v1: Add def_domain_type") explains
> this was needed to allow probe_finalize() to be called, but now the
> IDENTITY domain will do the same job so change the returned
> def_domain_type.
>
> mkt_v1 is the only driver that returns IOMMU_DOMAIN_UNMANAGED from
> def_domain_type(). This allows the next patch to enforce an IDENTITY
> domain policy for this driver.
>
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/iommu/mtk_iommu_v1.c | 21 +++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 08/24] iommu: Reorganize iommu_get_default_domain_type() to respect def_domain_type()
On Wed, Aug 23, 2023 at 01:47:22PM -0300, Jason Gunthorpe wrote: > Except for dart (which forces IOMMU_DOMAIN_DMA) every driver returns 0 or > IDENTITY from ops->def_domain_type(). > > The drivers that return IDENTITY have some kind of good reason, typically > that quirky hardware really can't support anything other than IDENTITY. > > Arrange things so that if the driver says it needs IDENTITY then > iommu_get_default_domain_type() either fails or returns IDENTITY. It will > not ignore the driver's override to IDENTITY. > > Split the function into two steps, reducing the group device list to the > driver's def_domain_type() and the untrusted flag. > > Then compute the result based on those two reduced variables. Fully reject > combining untrusted with IDENTITY. > > Remove the debugging print on the iommu_group_store_type() failure path, > userspace should not be able to trigger kernel prints. > > This makes the next patch cleaner that wants to force IDENTITY always for > ARM_IOMMU because there is no support for DMA. > > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/iommu.c | 117 ++++-- > 1 file changed, 79 insertions(+), 38 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 09/24] iommu: Allow an IDENTITY domain as the default_domain in ARM32
On Wed, Aug 23, 2023 at 01:47:23PM -0300, Jason Gunthorpe wrote: > Even though dma-iommu.c and CONFIG_ARM_DMA_USE_IOMMU do approximately the > same stuff, the way they relate to the IOMMU core is quiet different. > > dma-iommu.c expects the core code to setup an UNMANAGED domain (of type > IOMMU_DOMAIN_DMA) and then configures itself to use that domain. This > becomes the default_domain for the group. > > ARM_DMA_USE_IOMMU does not use the default_domain, instead it directly > allocates an UNMANAGED domain and operates it just like an external > driver. In this case group->default_domain is NULL. > > If the driver provides a global static identity_domain then automatically > use it as the default_domain when in ARM_DMA_USE_IOMMU mode. > > This allows drivers that implemented default_domain == NULL as an IDENTITY > translation to trivially get a properly labeled non-NULL default_domain on > ARM32 configs. > > With this arrangment when ARM_DMA_USE_IOMMU wants to disconnect from the > device the normal detach_domain flow will restore the IDENTITY domain as > the default domain. Overall this makes attach_dev() of the IDENTITY domain > called in the same places as detach_dev(). > > This effectively migrates these drivers to default_domain mode. For > drivers that support ARM64 they will gain support for the IDENTITY > translation mode for the dma_api and behave in a uniform way. > > Drivers use this by setting ops->identity_domain to a static singleton > iommu_domain that implements the identity attach. If the core detects > ARM_DMA_USE_IOMMU mode then it automatically attaches the IDENTITY domain > during probe. > > Drivers can continue to prevent the use of DMA translation by returning > IOMMU_DOMAIN_IDENTITY from def_domain_type, this will completely prevent > IOMMU_DMA from running but will not impact ARM_DMA_USE_IOMMU. > > This allows removing the set_platform_dma_ops() from every remaining > driver. > > Remove the set_platform_dma_ops from rockchip and mkt_v1 as all it does > is set an existing global static identity domain. mkt_v1 does not support > IOMMU_DOMAIN_DMA and it does not compile on ARM64 so this transformation > is safe. > > Tested-by: Steven Price > Tested-by: Marek Szyprowski > Tested-by: Nicolin Chen > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/iommu.c | 21 - > drivers/iommu/mtk_iommu_v1.c | 12 > drivers/iommu/rockchip-iommu.c | 10 -- > 3 files changed, 20 insertions(+), 23 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 10/24] iommu/exynos: Implement an IDENTITY domain
On Wed, Aug 23, 2023 at 01:47:24PM -0300, Jason Gunthorpe wrote: > What exynos calls exynos_iommu_detach_device is actually putting the iommu > into identity mode. > > Move to the new core support for ARM_DMA_USE_IOMMU by defining > ops->identity_domain. > > Tested-by: Marek Szyprowski > Acked-by: Marek Szyprowski > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/exynos-iommu.c | 66 +--- > 1 file changed, 32 insertions(+), 34 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 11/24] iommu/tegra-smmu: Implement an IDENTITY domain
On Wed, Aug 23, 2023 at 01:47:25PM -0300, Jason Gunthorpe wrote: > What tegra-smmu does during tegra_smmu_set_platform_dma() is actually > putting the iommu into identity mode. > > Move to the new core support for ARM_DMA_USE_IOMMU by defining > ops->identity_domain. > > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/tegra-smmu.c | 37 - > 1 file changed, 32 insertions(+), 5 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 12/24] iommu/tegra-smmu: Support DMA domains in tegra
On Wed, Aug 23, 2023 at 01:47:26PM -0300, Jason Gunthorpe wrote: > All ARM64 iommu drivers should support IOMMU_DOMAIN_DMA to enable > dma-iommu.c. > > tegra is blocking dma-iommu usage, and also default_domain's, because it > wants an identity translation. This is needed for some device quirk. The > correct way to do this is to support IDENTITY domains and use > ops->def_domain_type() to return IOMMU_DOMAIN_IDENTITY for only the quirky > devices. > > Add support for IOMMU_DOMAIN_DMA and force IOMMU_DOMAIN_IDENTITY mode for > everything so no behavior changes. > > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/tegra-smmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 13/24] iommu/omap: Implement an IDENTITY domain
On Wed, Aug 23, 2023 at 01:47:27PM -0300, Jason Gunthorpe wrote: > What omap does during omap_iommu_set_platform_dma() is actually putting > the iommu into identity mode. > > Move to the new core support for ARM_DMA_USE_IOMMU by defining > ops->identity_domain. > > This driver does not support IOMMU_DOMAIN_DMA, however it cannot be > compiled on ARM64 either. Most likely it is fine to support dma-iommu.c > > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/omap-iommu.c | 21 ++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 14/24] iommu/msm: Implement an IDENTITY domain
On Wed, Aug 23, 2023 at 01:47:28PM -0300, Jason Gunthorpe wrote: > What msm does during msm_iommu_set_platform_dma() is actually putting the > iommu into identity mode. > > Move to the new core support for ARM_DMA_USE_IOMMU by defining > ops->identity_domain. > > This driver does not support IOMMU_DOMAIN_DMA, however it cannot be > compiled on ARM64 either. Most likely it is fine to support dma-iommu.c > > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/msm_iommu.c | 23 +++ > 1 file changed, 19 insertions(+), 4 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 15/24] iommu: Remove ops->set_platform_dma_ops()
On Wed, Aug 23, 2023 at 01:47:29PM -0300, Jason Gunthorpe wrote: > All drivers are now using IDENTITY or PLATFORM domains for what this did, > we can remove it now. It is no longer possible to attach to a NULL domain. > > Tested-by: Heiko Stuebner > Tested-by: Niklas Schnelle > Tested-by: Steven Price > Tested-by: Marek Szyprowski > Tested-by: Nicolin Chen > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/iommu.c | 30 +- > include/linux/iommu.h | 4 > 2 files changed, 5 insertions(+), 29 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 16/24] iommu/qcom_iommu: Add an IOMMU_IDENTITIY_DOMAIN
On Wed, Aug 23, 2023 at 01:47:30PM -0300, Jason Gunthorpe wrote:
> This brings back the ops->detach_dev() code that commit
> 1b932ceddd19 ("iommu: Remove detach_dev callbacks") deleted and turns it
> into an IDENTITY domain.
>
> Reviewed-by: Lu Baolu
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/iommu/arm/arm-smmu/qcom_iommu.c | 39 +
> 1 file changed, 39 insertions(+)
>
Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 17/24] iommu/ipmmu: Add an IOMMU_IDENTITIY_DOMAIN
On Wed, Aug 23, 2023 at 01:47:31PM -0300, Jason Gunthorpe wrote:
> This brings back the ops->detach_dev() code that commit
> 1b932ceddd19 ("iommu: Remove detach_dev callbacks") deleted and turns it
> into an IDENTITY domain.
>
> Also reverts commit 584d334b1393 ("iommu/ipmmu-vmsa: Remove
> ipmmu_utlb_disable()")
>
> Reviewed-by: Lu Baolu
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/iommu/ipmmu-vmsa.c | 43 ++
> 1 file changed, 43 insertions(+)
>
Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 18/24] iommu/mtk_iommu: Add an IOMMU_IDENTITIY_DOMAIN
On Wed, Aug 23, 2023 at 01:47:32PM -0300, Jason Gunthorpe wrote:
> This brings back the ops->detach_dev() code that commit
> 1b932ceddd19 ("iommu: Remove detach_dev callbacks") deleted and turns it
> into an IDENTITY domain.
>
> Reviewed-by: Lu Baolu
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/iommu/mtk_iommu.c | 23 +++
> 1 file changed, 23 insertions(+)
>
Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 19/24] iommu/sun50i: Add an IOMMU_IDENTITIY_DOMAIN
On Wed, Aug 23, 2023 at 01:47:33PM -0300, Jason Gunthorpe wrote:
> Prior to commit 1b932ceddd19 ("iommu: Remove detach_dev callbacks") the
> sun50i_iommu_detach_device() function was being called by
> ops->detach_dev().
>
> This is an IDENTITY domain so convert sun50i_iommu_detach_device() into
> sun50i_iommu_identity_attach() and a full IDENTITY domain and thus hook it
> back up the same was as the old ops->detach_dev().
>
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/iommu/sun50i-iommu.c | 26 +++---
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 20/24] iommu: Require a default_domain for all iommu drivers
On Wed, Aug 23, 2023 at 01:47:34PM -0300, Jason Gunthorpe wrote: > At this point every iommu driver will cause a default_domain to be > selected, so we can finally remove this gap from the core code. > > The following table explains what each driver supports and what the > resulting default_domain will be: > > ops->defaut_domain > IDENTITY DMA PLATFORMv ARM32 > dma-iommu ARCH > amd/iommu.c Y Y N/A either > apple-dart.cY Y N/A either > arm-smmu.c Y Y IDENTITYeither > qcom_iommu.cG Y IDENTITYeither > arm-smmu-v3.c Y Y N/A either > exynos-iommu.c G Y IDENTITYeither > fsl_pamu_domain.c Y Y N/A N/A > PLATFORM > intel/iommu.c Y Y N/A either > ipmmu-vmsa.cG Y IDENTITYeither > msm_iommu.c G IDENTITYN/A > mtk_iommu.c G Y IDENTITYeither > mtk_iommu_v1.c G IDENTITYN/A > omap-iommu.cG IDENTITYN/A > rockchip-iommu.cG Y IDENTITYeither > s390-iommu.cY Y N/A N/A > PLATFORM > sprd-iommu.cY N/A DMA > sun50i-iommu.c G Y IDENTITYeither > tegra-smmu.cG Y IDENTITY > IDENTITY > virtio-iommu.c Y Y N/A either > spapr Y Y N/A N/A > PLATFORM > * G means ops->identity_domain is used > * N/A means the driver will not compile in this configuration > > ARM32 drivers select an IDENTITY default domain through either the > ops->identity_domain or directly requesting an IDENTIY domain through > alloc_domain(). > > In ARM64 mode tegra-smmu will still block the use of dma-iommu.c and > forces an IDENTITY domain. > > S390 uses a PLATFORM domain to represent when the dma_ops are set to the > s390 iommu code. > > fsl_pamu uses an PLATFORM domain. > > POWER SPAPR uses PLATFORM and blocking to enable its weird VFIO mode. > > The x86 drivers continue unchanged. > > After this patch group->default_domain is only NULL for a short period > during bus iommu probing while all the groups are constituted. Otherwise > it is always !NULL. > > This completes changing the iommu subsystem driver contract to a system > where the current iommu_domain always represents some form of translation > and the driver is continuously asserting a definable translation mode. > > It resolves the confusion that the original ops->detach_dev() caused > around what translation, exactly, is the IOMMU performing after > detach. There were at least three different answers to that question in > the tree, they are all now clearly named with domain types. > > Tested-by: Heiko Stuebner > Tested-by: Niklas Schnelle > Tested-by: Steven Price > Tested-by: Marek Szyprowski > Tested-by: Nicolin Chen > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/iommu.c | 22 +++--- > 1 file changed, 7 insertions(+), 15 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 21/24] iommu: Add __iommu_group_domain_alloc()
On Wed, Aug 23, 2023 at 01:47:35PM -0300, Jason Gunthorpe wrote: > Allocate a domain from a group. Automatically obtains the iommu_ops to use > from the device list of the group. Convert the internal callers to use it. > > Tested-by: Steven Price > Tested-by: Marek Szyprowski > Tested-by: Nicolin Chen > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe > --- Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 22/24] iommu: Add ops->domain_alloc_paging()
On Wed, Aug 23, 2023 at 01:47:36PM -0300, Jason Gunthorpe wrote: > This callback requests the driver to create only a __IOMMU_DOMAIN_PAGING > domain, so it saves a few lines in a lot of drivers needlessly checking > the type. > > More critically, this allows us to sweep out all the > IOMMU_DOMAIN_UNMANAGED and IOMMU_DOMAIN_DMA checks from a lot of the > drivers, simplifying what is going on in the code and ultimately removing > the now-unused special cases in drivers where they did not support > IOMMU_DOMAIN_DMA. > > domain_alloc_paging() should return a struct iommu_domain that is > functionally compatible with ARM_DMA_USE_IOMMU, dma-iommu.c and iommufd. > > Be forwards looking and pass in a 'struct device *' argument. We can > provide this when allocating the default_domain. No drivers will look at > this. > > Tested-by: Steven Price > Tested-by: Marek Szyprowski > Tested-by: Nicolin Chen > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/iommu.c | 17 ++--- > include/linux/iommu.h | 3 +++ > 2 files changed, 17 insertions(+), 3 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 23/24] iommu: Convert simple drivers with DOMAIN_DMA to domain_alloc_paging()
On Wed, Aug 23, 2023 at 01:47:37PM -0300, Jason Gunthorpe wrote: > These drivers are all trivially converted since the function is only > called if the domain type is going to be > IOMMU_DOMAIN_UNMANAGED/DMA. > > Tested-by: Heiko Stuebner > Tested-by: Steven Price > Tested-by: Marek Szyprowski > Tested-by: Nicolin Chen > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/arm/arm-smmu/qcom_iommu.c | 6 ++ > drivers/iommu/exynos-iommu.c| 7 ++- > drivers/iommu/ipmmu-vmsa.c | 7 ++- > drivers/iommu/mtk_iommu.c | 7 ++- > drivers/iommu/rockchip-iommu.c | 7 ++- > drivers/iommu/sprd-iommu.c | 7 ++- > drivers/iommu/sun50i-iommu.c| 9 +++-- > drivers/iommu/tegra-smmu.c | 7 ++- > 8 files changed, 17 insertions(+), 40 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v7 24/24] iommu: Convert remaining simple drivers to domain_alloc_paging()
On Wed, Aug 23, 2023 at 01:47:38PM -0300, Jason Gunthorpe wrote: > These drivers don't support IOMMU_DOMAIN_DMA, so this commit effectively > allows them to support that mode. > > The prior work to require default_domains makes this safe because every > one of these drivers is either compilation incompatible with dma-iommu.c, > or already establishing a default_domain. In both cases alloc_domain() > will never be called with IOMMU_DOMAIN_DMA for these drivers so it is safe > to drop the test. > > Removing these tests clarifies that the domain allocation path is only > about the functionality of a paging domain and has nothing to do with > policy of how the paging domain is used for UNMANAGED/DMA/DMA_FQ. > > Tested-by: Niklas Schnelle > Tested-by: Steven Price > Tested-by: Marek Szyprowski > Tested-by: Nicolin Chen > Reviewed-by: Lu Baolu > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/msm_iommu.c| 7 ++- > drivers/iommu/mtk_iommu_v1.c | 7 ++- > drivers/iommu/omap-iommu.c | 7 ++- > drivers/iommu/s390-iommu.c | 7 ++- > 4 files changed, 8 insertions(+), 20 deletions(-) > Reviewed-by: Jerry Snitselaar
Re: [PATCH v9 1/4] drivers: of: kexec ima: Support 32-bit platforms
On Tue, Apr 18, 2023 at 09:44:06AM -0400, Stefan Berger wrote:
> From: Palmer Dabbelt
>
> RISC-V recently added kexec_file() support, which uses enables kexec
> IMA. We're the first 32-bit platform to support this, so we found a
> build bug.
>
> Acked-by: Rob Herring
> Signed-off-by: Palmer Dabbelt
> Reviewed-by: Mimi Zohar
Reviewed-by: Jerry Snitselaar
> ---
> drivers/of/kexec.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> index f26d2ba8a371..1373d7e0a9b3 100644
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
> @@ -250,8 +250,8 @@ static int setup_ima_buffer(const struct kimage *image,
> void *fdt,
> if (ret)
> return -EINVAL;
>
> - pr_debug("IMA buffer at 0x%llx, size = 0x%zx\n",
> - image->ima_buffer_addr, image->ima_buffer_size);
> + pr_debug("IMA buffer at 0x%pa, size = 0x%zx\n",
> + &image->ima_buffer_addr, image->ima_buffer_size);
>
> return 0;
> }
> --
> 2.38.1
>
Re: [PATCH v9 2/4] tpm: of: Make of-tree specific function commonly available
On Tue, Apr 18, 2023 at 09:44:07AM -0400, Stefan Berger wrote:
> Simplify tpm_read_log_of() by moving reusable parts of the code into
> an inline function that makes it commonly available so it can be
> used also for kexec support. Call the new of_tpm_get_sml_parameters()
> function from the TPM Open Firmware driver.
>
> Signed-off-by: Stefan Berger
> Cc: Jarkko Sakkinen
> Cc: Jason Gunthorpe
> Cc: Rob Herring
> Cc: Frank Rowand
> Reviewed-by: Mimi Zohar
> Tested-by: Nageswara R Sastry
> Tested-by: Coiby Xu
> Acked-by: Jarkko Sakkinen
>
Reviewed-by: Jerry Snitselaar
> ---
> v9:
> - Rebased on 6.3-rc7; call tpm_read_log_memory_region if -ENODEV returned
>
> v7:
> - Added original comment back into inlined function
>
> v4:
> - converted to inline function
> ---
> drivers/char/tpm/eventlog/of.c | 30 +---
> include/linux/tpm.h| 36 ++
> 2 files changed, 41 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c
> index 930fe43d5daf..41688d9cbd3b 100644
> --- a/drivers/char/tpm/eventlog/of.c
> +++ b/drivers/char/tpm/eventlog/of.c
> @@ -51,11 +51,10 @@ static int tpm_read_log_memory_region(struct tpm_chip
> *chip)
> int tpm_read_log_of(struct tpm_chip *chip)
> {
> struct device_node *np;
> - const u32 *sizep;
> - const u64 *basep;
> struct tpm_bios_log *log;
> u32 size;
> u64 base;
> + int ret;
>
> log = &chip->log;
> if (chip->dev.parent && chip->dev.parent->of_node)
> @@ -66,30 +65,11 @@ 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)
> + ret = of_tpm_get_sml_parameters(np, &base, &size);
> + if (ret == -ENODEV)
> 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) {
> - size = be32_to_cpup((__force __be32 *)sizep);
> - base = be64_to_cpup((__force __be64 *)basep);
> - } else {
> - size = *sizep;
> - base = *basep;
> - }
> + if (ret < 0)
> + return ret;
>
> if (size == 0) {
> dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 4dc97b9f65fb..dd9a376a1dbb 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -461,4 +461,40 @@ static inline struct tpm_chip *tpm_default_chip(void)
> return NULL;
> }
> #endif
> +
> +#ifdef CONFIG_OF
> +static inline int of_tpm_get_sml_parameters(struct device_node *np,
> + u64 *base, u32 *size)
> +{
> + const u32 *sizep;
> + const u64 *basep;
> +
> + sizep = of_get_property(np, "linux,sml-size", NULL);
> + basep = of_get_property(np, "linux,sml-base", NULL);
> + if (sizep == NULL && basep == NULL)
> + return -ENODEV;
> + 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) {
> + *size = be32_to_cpup((__force __be32 *)sizep);
> + *base = be64_to_cpup((__force __be64 *)basep);
> + } else {
> + *size = *sizep;
> + *base = *basep;
> + }
> + return 0;
> +}
> +#endif
> +
> #endif
> --
> 2.38.1
>
Re: [PATCH v9 3/4] of: kexec: Refactor IMA buffer related functions to make them reusable
On Tue, Apr 18, 2023 at 09:44:08AM -0400, Stefan Berger wrote:
> Refactor IMA buffer related functions to make them reusable for carrying
> TPM logs across kexec.
>
> Signed-off-by: Stefan Berger
> Cc: Rob Herring
> Cc: Frank Rowand
> Cc: Mimi Zohar
> Reviewed-by: Mimi Zohar
> Reviewed-by: Rob Herring
> Tested-by: Nageswara R Sastry
> Tested-by: Coiby Xu
>
Reviewed-by: Jerry Snitselaar
> ---
> v6:
> - Add __init to get_kexec_buffer as suggested by Jonathan
>
> v5:
> - Rebased on Jonathan McDowell's commit "b69a2afd5afc x86/kexec: Carry
>forward IMA measurement log on kexec"
> v4:
> - Move debug output into setup_buffer()
> ---
> drivers/of/kexec.c | 126 ++---
> 1 file changed, 74 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> index 1373d7e0a9b3..fa8c0c75adf9 100644
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
> @@ -117,45 +117,57 @@ static int do_get_kexec_buffer(const void *prop, int
> len, unsigned long *addr,
> }
>
> #ifdef CONFIG_HAVE_IMA_KEXEC
> -/**
> - * ima_get_kexec_buffer - get IMA buffer from the previous kernel
> - * @addr:On successful return, set to point to the buffer contents.
> - * @size:On successful return, set to the buffer size.
> - *
> - * Return: 0 on success, negative errno on error.
> - */
> -int __init ima_get_kexec_buffer(void **addr, size_t *size)
> +static int __init get_kexec_buffer(const char *name, unsigned long *addr,
> +size_t *size)
> {
> int ret, len;
> - unsigned long tmp_addr;
> unsigned long start_pfn, end_pfn;
> - size_t tmp_size;
> const void *prop;
>
> - prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", &len);
> + prop = of_get_property(of_chosen, name, &len);
> if (!prop)
> return -ENOENT;
>
> - ret = do_get_kexec_buffer(prop, len, &tmp_addr, &tmp_size);
> + ret = do_get_kexec_buffer(prop, len, addr, size);
> if (ret)
> return ret;
>
> - /* Do some sanity on the returned size for the ima-kexec buffer */
> - if (!tmp_size)
> + /* Do some sanity on the returned size for the kexec buffer */
> + if (!*size)
> return -ENOENT;
>
> /*
>* Calculate the PFNs for the buffer and ensure
>* they are with in addressable memory.
>*/
> - start_pfn = PHYS_PFN(tmp_addr);
> - end_pfn = PHYS_PFN(tmp_addr + tmp_size - 1);
> + start_pfn = PHYS_PFN(*addr);
> + end_pfn = PHYS_PFN(*addr + *size - 1);
> if (!page_is_ram(start_pfn) || !page_is_ram(end_pfn)) {
> - pr_warn("IMA buffer at 0x%lx, size = 0x%zx beyond memory\n",
> - tmp_addr, tmp_size);
> + pr_warn("%s buffer at 0x%lx, size = 0x%zx beyond memory\n",
> + name, *addr, *size);
> return -EINVAL;
> }
>
> + return 0;
> +}
> +
> +/**
> + * ima_get_kexec_buffer - get IMA buffer from the previous kernel
> + * @addr:On successful return, set to point to the buffer contents.
> + * @size:On successful return, set to the buffer size.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int __init ima_get_kexec_buffer(void **addr, size_t *size)
> +{
> + int ret;
> + unsigned long tmp_addr;
> + size_t tmp_size;
> +
> + ret = get_kexec_buffer("linux,ima-kexec-buffer", &tmp_addr, &tmp_size);
> + if (ret)
> + return ret;
> +
> *addr = __va(tmp_addr);
> *size = tmp_size;
>
> @@ -188,72 +200,82 @@ int __init ima_free_kexec_buffer(void)
> }
> #endif
>
> -/**
> - * remove_ima_buffer - remove the IMA buffer property and reservation from
> @fdt
> - *
> - * @fdt: Flattened Device Tree to update
> - * @chosen_node: Offset to the chosen node in the device tree
> - *
> - * The IMA measurement buffer is of no use to a subsequent kernel, so we
> always
> - * remove it from the device tree.
> - */
> -static void remove_ima_buffer(void *fdt, int chosen_node)
> +static int remove_buffer(void *fdt, int chosen_node, const char *name)
> {
> int ret, len;
> unsigned long addr;
> size_t size;
> const void *prop;
>
> - if (!IS_ENABLED(CONFIG_HAVE_IMA_KEXEC))
> - return;
> -
> - prop = fdt_getprop(fdt, chosen_node, "linux,ima-kexec-buffer", &len);
> + prop = fdt_getprop(fdt, chosen_node, name, &len);
> if (!prop)
>
Re: [6.4-rc6] Crash during a kexec operation (tpm_amd_is_rng_defective)
On Thu, Jun 22, 2023 at 09:38:04AM -0500, Limonciello, Mario wrote: > > On 6/22/2023 7:36 AM, Michael Ellerman wrote: > > "Linux regression tracking (Thorsten Leemhuis)" > > writes: > > > Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting > > > for once, to make this easily accessible to everyone. > > > > > > As Linus will likely release 6.4 on this or the following Sunday a quick > > > question: is there any hope this regression might be fixed any time > > > soon? > > No. > > > > I have added the author of the commit to Cc, maybe they can help? > > > > The immediate question is, is it expected for chip->ops to be NULL in > > this path? Obviously on actual AMD systems that isn't the case, > > otherwise the code would crash there. But is the fact that chip->ops is > > NULL a bug in the ibmvtpm driver, or a possibility that has been > > overlooked by the checking code. > > > > cheers > > All that code assumes that the TPM is still functional which > seems not to be the case for your TPM. > > This should fix it: > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > index 5be91591cb3b..7082b031741e 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -525,6 +525,9 @@ static bool tpm_amd_is_rng_defective(struct tpm_chip > *chip) > u64 version; > int ret; > > + if (!chip->ops) > + return false; > + > if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) > return false; Should tpm_amd_is_rng_defective compile to nothing on non-x86 architectures? This code is all about working around an issue with the AMD fTPM, right? Regards, Jerry
Re: [6.4-rc6] Crash during a kexec operation (tpm_amd_is_rng_defective)
On Thu, Jun 29, 2023 at 05:28:58PM +, Limonciello, Mario wrote:
> [Public]
>
> > -Original Message-
> > From: Jerry Snitselaar
> > Sent: Thursday, June 29, 2023 12:07 PM
> > To: Limonciello, Mario
> > Cc: Michael Ellerman ; Linux regressions mailing list
> > ; Sachin Sant ; open
> > list ; linuxppc-dev > d...@lists.ozlabs.org>; jar...@kernel.org; linux-integr...@vger.kernel.org
> > Subject: Re: [6.4-rc6] Crash during a kexec operation
> > (tpm_amd_is_rng_defective)
> >
> > On Thu, Jun 22, 2023 at 09:38:04AM -0500, Limonciello, Mario wrote:
> > >
> > > On 6/22/2023 7:36 AM, Michael Ellerman wrote:
> > > > "Linux regression tracking (Thorsten Leemhuis)"
> > writes:
> > > > > Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> > > > > for once, to make this easily accessible to everyone.
> > > > >
> > > > > As Linus will likely release 6.4 on this or the following Sunday a
> > > > > quick
> > > > > question: is there any hope this regression might be fixed any time
> > > > > soon?
> > > > No.
> > > >
> > > > I have added the author of the commit to Cc, maybe they can help?
> > > >
> > > > The immediate question is, is it expected for chip->ops to be NULL in
> > > > this path? Obviously on actual AMD systems that isn't the case,
> > > > otherwise the code would crash there. But is the fact that chip->ops is
> > > > NULL a bug in the ibmvtpm driver, or a possibility that has been
> > > > overlooked by the checking code.
> > > >
> > > > cheers
> > >
> > > All that code assumes that the TPM is still functional which
> > > seems not to be the case for your TPM.
> > >
> > > This should fix it:
> > >
> > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > > index 5be91591cb3b..7082b031741e 100644
> > > --- a/drivers/char/tpm/tpm-chip.c
> > > +++ b/drivers/char/tpm/tpm-chip.c
> > > @@ -525,6 +525,9 @@ static bool tpm_amd_is_rng_defective(struct
> > tpm_chip
> > > *chip)
> > > u64 version;
> > > int ret;
> > >
> > > + if (!chip->ops)
> > > + return false;
> > > +
> > > if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> > > return false;
> >
> >
> > Should tpm_amd_is_rng_defective compile to nothing on non-x86
> > architectures? This code is all about
> > working around an issue with the AMD fTPM, right?
> >
>
> That's a good point. Yes it could and that would also solve this problem.
>
Or I guess more accurately for non-x86 it should be:
static bool tpm_amd_is_rng_defective(struct tpm_chip *chip)
{
return false;
}
