Re: [PATCH] powerpc: powermac: Use of_get_cpu_hwid() to read CPU node 'reg'
Christophe JAILLET writes: > Le 03/07/2023 à 07:26, Michael Ellerman a écrit : >> On Sun, 19 Mar 2023 09:59:31 -0500, Rob Herring wrote: >>> Replace open coded reading of CPU nodes' "reg" properties with >>> of_get_cpu_hwid() dedicated for this purpose. >>> >>> >> >> Applied to powerpc/next. >> >> [1/1] powerpc: powermac: Use of_get_cpu_hwid() to read CPU node 'reg' >> >> https://git.kernel.org/powerpc/c/bc1cf75027585f8d87f94e464ee5909acf885a8c >> >> cheers >> > > Hi, > > I guess, that it does not really matter, but shouldn't the > of_node_put() be *after* the "reset_io = *rst;" statements to be > absolutely safe? Technically yes. > (This change is in my backlog and I have apparently never proposed it) Can you rebase and resend? cheers
Re: [PATCH v8 1/2] powerpc/rtas: Rename rtas_error_rc to rtas_generic_errno
Mahesh Salgaonkar writes:
> rtas_generic_errno() function will convert the generic rtas return codes
> into errno.
I don't see the point of renaming it, it just creates unnecessary churn.
The existing name seems OK to me.
...
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 3abe15ac79db1..5572a0a2f6e18 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -202,7 +202,9 @@ typedef struct {
> #define RTAS_USER_REGION_SIZE (64 * 1024)
>
> /* RTAS return status codes */
> -#define RTAS_BUSY-2/* RTAS Busy */
> +#define RTAS_HARDWARE_ERROR (-1) /* Hardware Error */
> +#define RTAS_BUSY(-2) /* RTAS Busy */
Are the brackets necessary?
> +#define RTAS_INVALID_PARAMETER (-3) /* Invalid
> indicator/domain/sensor etc. */
> #define RTAS_EXTENDED_DELAY_MIN 9900
> #define RTAS_EXTENDED_DELAY_MAX 9905
>
> @@ -212,6 +214,11 @@ typedef struct {
> #define RTAS_THREADS_ACTIVE -9005 /* Multiple processor threads active */
> #define RTAS_OUTSTANDING_COPROC -9006 /* Outstanding coprocessor operations
> */
>
> +/* statuses specific to get-sensor-state */
> +#define RTAS_SLOT_UNISOLATED (-9000)
> +#define RTAS_SLOT_NOT_UNISOLATED (-9001)
> +#define RTAS_SLOT_NOT_USABLE (-9002)
These aren't specific to get-sensor-state.
They're used by at least: ibm,manage-flash-image, ibm,activate-firmware,
ibm,configure-connector, set-indicator etc.
They have different meanings for those calls. I think you're best to
just leave the constant values in rtas_error_rc().
> /* RTAS event classes */
> #define RTAS_INTERNAL_ERROR 0x8000 /* set bit 0 */
> #define RTAS_EPOW_WARNING0x4000 /* set bit 1 */
> @@ -425,6 +432,7 @@ extern int rtas_set_indicator(int indicator, int index,
> int new_value);
> extern int rtas_set_indicator_fast(int indicator, int index, int new_value);
> extern void rtas_progress(char *s, unsigned short hex);
> int rtas_ibm_suspend_me(int *fw_status);
> +int rtas_generic_errno(int rtas_rc);
>
> struct rtc_time;
> extern time64_t rtas_get_boot_time(void);
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index c087320ff..80b6099e8ce20 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -1330,33 +1330,34 @@ bool __ref rtas_busy_delay(int status)
> }
> EXPORT_SYMBOL_GPL(rtas_busy_delay);
>
> -static int rtas_error_rc(int rtas_rc)
> +int rtas_generic_errno(int rtas_rc)
> {
> int rc;
>
> switch (rtas_rc) {
> - case -1:/* Hardware Error */
> - rc = -EIO;
> - break;
> - case -3:/* Bad indicator/domain/etc */
> - rc = -EINVAL;
> - break;
> - case -9000: /* Isolation error */
> - rc = -EFAULT;
> - break;
> - case -9001: /* Outstanding TCE/PTE */
> - rc = -EEXIST;
> - break;
> - case -9002: /* No usable slot */
> - rc = -ENODEV;
> - break;
> - default:
> - pr_err("%s: unexpected error %d\n", __func__, rtas_rc);
> - rc = -ERANGE;
> - break;
> + case RTAS_HARDWARE_ERROR: /* Hardware Error */
> + rc = -EIO;
> + break;
> + case RTAS_INVALID_PARAMETER:/* Bad indicator/domain/etc */
> + rc = -EINVAL;
> + break;
> + case RTAS_SLOT_UNISOLATED: /* Isolation error */
> + rc = -EFAULT;
> + break;
> + case RTAS_SLOT_NOT_UNISOLATED: /* Outstanding TCE/PTE */
> + rc = -EEXIST;
> + break;
> + case RTAS_SLOT_NOT_USABLE: /* No usable slot */
> + rc = -ENODEV;
> + break;
> + default:
> + pr_err("%s: unexpected error %d\n", __func__, rtas_rc);
> + rc = -ERANGE;
> + break;
> }
> return rc;
> }
> +EXPORT_SYMBOL(rtas_generic_errno);
Should be GPL.
cheers
[powerpc:next-test] BUILD SUCCESS ad03be7b2fe5e79df74a7e6b2568c7366adf0402
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test branch HEAD: ad03be7b2fe5e79df74a7e6b2568c7366adf0402 ocxl: Use pci_dev_id() to simplify the code elapsed time: 860m configs tested: 310 configs skipped: 17 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alphaallyesconfig gcc alpha defconfig gcc alpharandconfig-r001-20230814 gcc alpharandconfig-r003-20230814 gcc alpharandconfig-r004-20230814 gcc alpharandconfig-r012-20230815 gcc alpharandconfig-r013-20230815 gcc alpharandconfig-r021-20230814 gcc alpharandconfig-r022-20230815 gcc alpharandconfig-r024-20230815 gcc alpharandconfig-r031-20230815 gcc alpharandconfig-r033-20230815 gcc alpharandconfig-r034-20230814 gcc alpharandconfig-r034-20230815 gcc arc allyesconfig gcc arc defconfig gcc arc randconfig-r001-20230815 gcc arc randconfig-r002-20230815 gcc arc randconfig-r003-20230815 gcc arc randconfig-r013-20230815 gcc arc randconfig-r023-20230815 gcc arc randconfig-r024-20230815 gcc arc randconfig-r043-20230814 gcc arc randconfig-r043-20230815 gcc arcvdk_hs38_defconfig gcc arm allmodconfig gcc arm allyesconfig gcc arm defconfig gcc arm imx_v6_v7_defconfig gcc armkeystone_defconfig gcc arm lpc18xx_defconfig gcc arm moxart_defconfig clang armmps2_defconfig gcc armmulti_v7_defconfig gcc armmvebu_v7_defconfig gcc arm mxs_defconfig clang arm pxa_defconfig gcc arm randconfig-r006-20230815 gcc arm randconfig-r014-20230814 gcc arm randconfig-r024-20230815 clang arm randconfig-r025-20230815 clang arm randconfig-r026-20230814 gcc arm randconfig-r034-20230815 gcc arm randconfig-r036-20230814 clang arm randconfig-r046-20230814 gcc arm randconfig-r046-20230815 clang armshmobile_defconfig gcc arm tegra_defconfig gcc arm64alldefconfig gcc arm64allyesconfig gcc arm64 defconfig gcc arm64randconfig-r003-20230814 gcc arm64randconfig-r012-20230815 gcc arm64randconfig-r014-20230815 gcc arm64randconfig-r021-20230815 gcc arm64randconfig-r022-20230815 gcc arm64randconfig-r025-20230815 gcc arm64randconfig-r026-20230815 gcc cskydefconfig gcc csky randconfig-r002-20230814 gcc csky randconfig-r013-20230815 gcc csky randconfig-r014-20230815 gcc csky randconfig-r015-20230815 gcc csky randconfig-r016-20230815 gcc csky randconfig-r031-20230815 gcc csky randconfig-r035-20230814 gcc hexagon randconfig-r026-20230815 clang hexagon randconfig-r041-20230814 clang hexagon randconfig-r041-20230815 clang hexagon randconfig-r045-20230814 clang hexagon randconfig-r045-20230815 clang i386 allyesconfig gcc i386 buildonly-randconfig-r004-20230814 gcc i386 buildonly-randconfig-r004-20230815 clang i386 buildonly-randconfig-r005-20230814 gcc i386 buildonly-randconfig-r005-20230815 clang i386 buildonly-randconfig-r006-20230814 gcc i386 buildonly-randconfig-r006-20230815 clang i386 debian-10.3 gcc i386defconfig gcc i386 randconfig-i001-20230814 gcc i386 randconfig-i001-20230815 clang i386 randconfig-i002-20230814 gcc i386 randconfig-i002-20230815 clang i386 randconfig-i003-20230814 gcc i386
Re: [PATCH v6 00/25] iommu: Make default_domain's mandatory
On 2023/8/15 1:30, Jason Gunthorpe wrote:
On Mon, Aug 14, 2023 at 04:43:23PM +0800, Baolu Lu wrote:
This is on github:https://github.com/jgunthorpe/linux/commits/iommu_all_defdom
It seems that after this series, all ARM iommu drivers are able to
support the IDENTITY default domain, hence perhaps we can remove below
code?
Yes, but this code is still used
If I remember it correctly, the background of this part of code is
that some arm drivers didn't support IDENTITY domain, so fall back to
DMA domain if IDENTITY domain allocation fails.
Not quite..
if (req_type)
return __iommu_group_alloc_default_domain(group, req_type);
req_type == 0 can still happen because it depends on what
def_domain_type returns, which is still 0 in alot of cases
/* The driver gave no guidance on what type to use, try the default */
dom = __iommu_group_alloc_default_domain(group, iommu_def_domain_type);
if (dom)
return dom;
So we try the default which might be IDENTITY/DMA/DMA_FQ - still have
to do this.
/* Otherwise IDENTITY and DMA_FQ defaults will try DMA */
if (iommu_def_domain_type == IOMMU_DOMAIN_DMA)
return NULL;
dom = __iommu_group_alloc_default_domain(group, IOMMU_DOMAIN_DMA);
if (!dom)
return NULL;
pr_warn("Failed to allocate default IOMMU domain of type %u for group %s -
Falling back to IOMMU_DOMAIN_DMA",
iommu_def_domain_type, group->name);
And this hunk is primarily a fallback in case the DMA_FQ didn't
work. Then we try normal DMA.
That it also protected against not implementing IDENTITY is a side
effect, so I think we have to keep all of this still.
Okay, fair enough. Thanks for the explanation.
Best regards,
baolu
Re: [PATCH v6 08/25] iommu: Reorganize iommu_get_default_domain_type() to respect def_domain_type()
On 2023/8/15 1:25, Jason Gunthorpe wrote:
Ah, I went over all this again and decided to try again, it is too
complicated. This patch can do what the commit message says and the
following patches are even simpler:
Yes. This method is more concise. Some nits below.
/*
* Combine the driver's choosen def_domain_type across all the devices in a
* group. Drivers must give a consistent result.
*/
static int iommu_get_def_domain_type(struct iommu_group *group,
struct device *dev, int cur_type)
{
const struct iommu_ops *ops = group_iommu_ops(group);
int type;
if (!ops->def_domain_type)
return cur_type;
type = ops->def_domain_type(dev);
if (!type || cur_type == type)
return cur_type;
if (!cur_type)
return type;
dev_err_ratelimited(
dev,
"IOMMU driver error, requesting conflicting def_domain_type, %s and
%s, for devices in group %u.\n",
iommu_domain_type_str(cur_type), iommu_domain_type_str(type),
group->id);
/*
* Try to recover, drivers are allowed to force IDENITY or DMA, IDENTITY
* takes precedence.
*/
if (cur_type || type == IOMMU_DOMAIN_IDENTITY)
return IOMMU_DOMAIN_IDENTITY;
No need to check cur_type. It already returned if cur_type is 0.
return cur_type;
}
/*
* A target_type of 0 will select the best domain type. 0 can be returned in
* this case meaning the global default should be used.
*/
static int iommu_get_default_domain_type(struct iommu_group *group,
int target_type)
{
struct device *untrusted = NULL;
struct group_device *gdev;
int driver_type = 0;
lockdep_assert_held(>mutex);
/*
* ARM32 drivers supporting CONFIG_ARM_DMA_USE_IOMMU can declare an
* identity_domain and it will automatically become their default
* domain. Later on ARM_DMA_USE_IOMMU will install its UNMANAGED domain.
* Override the selection to IDENTITY.
*/
if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
static_assert(!(IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) &&
IS_ENABLED(CONFIG_IOMMU_DMA)));
IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) is duplicate with the condition in
the if statement. So only
static_assert(!IS_ENABLED(CONFIG_IOMMU_DMA));
?
driver_type = IOMMU_DOMAIN_IDENTITY;
}
for_each_group_device(group, gdev) {
driver_type = iommu_get_def_domain_type(group, gdev->dev,
driver_type);
No need to call this in the loop body?
if (dev_is_pci(gdev->dev) && to_pci_dev(gdev->dev)->untrusted) {
/*
* No ARM32 using systems will set untrusted, it cannot
* work.
*/
if (WARN_ON(IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)))
return -1;
untrusted = gdev->dev;
}
}
if (untrusted) {
if (driver_type && driver_type != IOMMU_DOMAIN_DMA) {
dev_err_ratelimited(
untrusted,
"Device is not trusted, but driver is overriding
group %u to %s, refusing to probe.\n",
group->id, iommu_domain_type_str(driver_type));
return -1;
}
driver_type = IOMMU_DOMAIN_DMA;
}
if (target_type) {
if (driver_type && target_type != driver_type)
return -1;
return target_type;
}
return driver_type;
}
Best regards,
baolu
[powerpc:next] BUILD SUCCESS cd50430ceb3598957803934068531a274349bcf9
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next branch HEAD: cd50430ceb3598957803934068531a274349bcf9 macintosh/ams: mark ams_init() static elapsed time: 725m configs tested: 278 configs skipped: 13 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alphaallyesconfig gcc alpha defconfig gcc alpharandconfig-r001-20230814 gcc alpharandconfig-r004-20230814 gcc alpharandconfig-r012-20230815 gcc alpharandconfig-r013-20230815 gcc alpharandconfig-r023-20230814 gcc alpharandconfig-r024-20230815 gcc alpharandconfig-r031-20230815 gcc alpharandconfig-r033-20230815 gcc alpharandconfig-r034-20230815 gcc arc allyesconfig gcc arc defconfig gcc arc randconfig-r001-20230815 gcc arc randconfig-r002-20230815 gcc arc randconfig-r003-20230815 gcc arc randconfig-r013-20230815 gcc arc randconfig-r014-20230814 gcc arc randconfig-r023-20230815 gcc arc randconfig-r043-20230814 gcc arc randconfig-r043-20230815 gcc arcvdk_hs38_defconfig gcc arm allmodconfig gcc arm allyesconfig gcc arm defconfig gcc armkeystone_defconfig gcc arm lpc18xx_defconfig gcc arm moxart_defconfig clang armmps2_defconfig gcc armmulti_v7_defconfig gcc arm mxs_defconfig clang arm randconfig-r006-20230815 gcc arm randconfig-r015-20230814 gcc arm randconfig-r024-20230815 clang arm randconfig-r025-20230815 clang arm randconfig-r026-20230814 gcc arm randconfig-r032-20230814 clang arm randconfig-r034-20230815 gcc arm randconfig-r046-20230814 gcc arm randconfig-r046-20230815 clang armshmobile_defconfig gcc arm tegra_defconfig gcc arm64alldefconfig gcc arm64allyesconfig gcc arm64 defconfig gcc arm64randconfig-r003-20230814 gcc arm64randconfig-r012-20230815 gcc arm64randconfig-r021-20230815 gcc arm64randconfig-r022-20230815 gcc arm64randconfig-r025-20230815 gcc arm64randconfig-r026-20230815 gcc cskydefconfig gcc csky randconfig-r002-20230814 gcc csky randconfig-r013-20230815 gcc csky randconfig-r014-20230815 gcc csky randconfig-r015-20230815 gcc csky randconfig-r016-20230815 gcc csky randconfig-r022-20230814 gcc csky randconfig-r031-20230815 gcc csky randconfig-r035-20230814 gcc hexagon randconfig-r026-20230814 clang hexagon randconfig-r026-20230815 clang hexagon randconfig-r041-20230814 clang hexagon randconfig-r041-20230815 clang hexagon randconfig-r045-20230814 clang hexagon randconfig-r045-20230815 clang i386 allyesconfig gcc i386 buildonly-randconfig-r004-20230814 gcc i386 buildonly-randconfig-r004-20230815 clang i386 buildonly-randconfig-r005-20230814 gcc i386 buildonly-randconfig-r005-20230815 clang i386 buildonly-randconfig-r006-20230814 gcc i386 buildonly-randconfig-r006-20230815 clang i386 debian-10.3 gcc i386defconfig gcc i386 randconfig-i001-20230814 gcc i386 randconfig-i001-20230815 clang i386 randconfig-i002-20230814 gcc i386 randconfig-i002-20230815 clang i386 randconfig-i003-20230814 gcc i386 randconfig-i003-20230815 clang i386 randconfig-i004-20230814 gcc i386 randconfig-i004-20230815 clang i386 randconfig-i005-20230814 gcc i386 randconfig-i005-20230815 clang i386 randconfig-i006
[powerpc:merge] BUILD SUCCESS 4f894fe22343138f0dc0ce18b49abfa45cdfeb6b
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge branch HEAD: 4f894fe22343138f0dc0ce18b49abfa45cdfeb6b Automatic merge of 'master' into merge (2023-08-14 10:49) elapsed time: 725m configs tested: 195 configs skipped: 13 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alphaallyesconfig gcc alpha defconfig gcc alpharandconfig-r001-20230814 gcc alpharandconfig-r004-20230814 gcc alpharandconfig-r013-20230815 gcc alpharandconfig-r034-20230815 gcc arc allyesconfig gcc arc defconfig gcc arc randconfig-r002-20230815 gcc arc randconfig-r003-20230815 gcc arc randconfig-r023-20230815 gcc arc randconfig-r032-20230814 gcc arc randconfig-r033-20230814 gcc arc randconfig-r043-20230814 gcc arc randconfig-r043-20230815 gcc arm allmodconfig gcc arm allyesconfig gcc arm defconfig gcc arm randconfig-r006-20230815 gcc arm randconfig-r026-20230814 gcc arm randconfig-r036-20230814 clang arm randconfig-r046-20230814 gcc arm randconfig-r046-20230815 clang arm64allyesconfig gcc arm64 defconfig gcc arm64randconfig-r003-20230814 gcc arm64randconfig-r021-20230815 gcc arm64randconfig-r024-20230814 clang arm64randconfig-r025-20230814 clang arm64randconfig-r025-20230815 gcc arm64randconfig-r026-20230815 gcc cskydefconfig gcc csky randconfig-r002-20230814 gcc csky randconfig-r013-20230815 gcc csky randconfig-r031-20230815 gcc csky randconfig-r035-20230814 gcc hexagon randconfig-r041-20230814 clang hexagon randconfig-r041-20230815 clang hexagon randconfig-r045-20230814 clang hexagon randconfig-r045-20230815 clang i386 allyesconfig gcc i386 buildonly-randconfig-r004-20230814 gcc i386 buildonly-randconfig-r004-20230815 clang i386 buildonly-randconfig-r005-20230814 gcc i386 buildonly-randconfig-r005-20230815 clang i386 buildonly-randconfig-r006-20230814 gcc i386 buildonly-randconfig-r006-20230815 clang i386 debian-10.3 gcc i386defconfig gcc i386 randconfig-i001-20230814 gcc i386 randconfig-i001-20230815 clang i386 randconfig-i002-20230814 gcc i386 randconfig-i002-20230815 clang i386 randconfig-i003-20230814 gcc i386 randconfig-i003-20230815 clang i386 randconfig-i004-20230814 gcc i386 randconfig-i004-20230815 clang i386 randconfig-i005-20230814 gcc i386 randconfig-i005-20230815 clang i386 randconfig-i006-20230814 gcc i386 randconfig-i006-20230815 clang i386 randconfig-i011-20230814 clang i386 randconfig-i011-20230815 gcc i386 randconfig-i012-20230814 clang i386 randconfig-i012-20230815 gcc i386 randconfig-i013-20230814 clang i386 randconfig-i013-20230815 gcc i386 randconfig-i014-20230814 clang i386 randconfig-i014-20230815 gcc i386 randconfig-i015-20230814 clang i386 randconfig-i015-20230815 gcc i386 randconfig-i016-20230814 clang i386 randconfig-i016-20230815 gcc i386 randconfig-r004-20230815 clang i386 randconfig-r005-20230814 gcc i386 randconfig-r005-20230815 clang i386 randconfig-r012-20230814 clang i386 randconfig-r016-20230815 gcc i386 randconfig-r023-20230814 clang loongarchallmodconfig gcc loongarch allnoconfig gcc loongarch defconfig gcc loongarchrandconfig-r021-20230814 gcc loongarchrandconfig-r022-20230814 gcc m68k allmodconfig gcc m68k
[powerpc:fixes-test] BUILD SUCCESS 80e30aa4961e89111ae3e48885138a80add3e2ab
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test branch HEAD: 80e30aa4961e89111ae3e48885138a80add3e2ab powerpc/rtas_flash: allow user copy to flash block cache objects elapsed time: 725m configs tested: 274 configs skipped: 127 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alphaallyesconfig gcc alpha defconfig gcc alpharandconfig-r001-20230814 gcc alpharandconfig-r004-20230814 gcc alpharandconfig-r012-20230815 gcc alpharandconfig-r013-20230815 gcc alpharandconfig-r024-20230815 gcc alpharandconfig-r031-20230815 gcc alpharandconfig-r033-20230815 gcc alpharandconfig-r034-20230815 gcc arc allyesconfig gcc arc defconfig gcc arc randconfig-r001-20230815 gcc arc randconfig-r002-20230815 gcc arc randconfig-r003-20230815 gcc arc randconfig-r013-20230814 gcc arc randconfig-r013-20230815 gcc arc randconfig-r014-20230814 gcc arc randconfig-r022-20230814 gcc arc randconfig-r023-20230815 gcc arc randconfig-r043-20230815 gcc arcvdk_hs38_defconfig gcc arm allmodconfig gcc arm allyesconfig gcc arm defconfig gcc armkeystone_defconfig gcc arm lpc18xx_defconfig gcc arm moxart_defconfig clang armmps2_defconfig gcc armmulti_v7_defconfig gcc arm mxs_defconfig clang arm randconfig-r003-20230814 clang arm randconfig-r004-20230814 clang arm randconfig-r006-20230815 gcc arm randconfig-r024-20230815 clang arm randconfig-r025-20230815 clang arm randconfig-r026-20230814 gcc arm randconfig-r034-20230815 gcc arm randconfig-r046-20230815 clang armshmobile_defconfig gcc armspear3xx_defconfig gcc arm tegra_defconfig gcc arm64alldefconfig gcc arm64allyesconfig gcc arm64 defconfig gcc arm64randconfig-r003-20230814 gcc arm64randconfig-r012-20230815 gcc arm64randconfig-r021-20230815 gcc arm64randconfig-r022-20230815 gcc arm64randconfig-r025-20230815 gcc arm64randconfig-r026-20230815 gcc cskydefconfig gcc csky randconfig-r002-20230814 gcc csky randconfig-r013-20230815 gcc csky randconfig-r014-20230815 gcc csky randconfig-r015-20230815 gcc csky randconfig-r016-20230815 gcc csky randconfig-r031-20230815 gcc csky randconfig-r035-20230814 gcc hexagon randconfig-r026-20230815 clang hexagon randconfig-r041-20230814 clang hexagon randconfig-r041-20230815 clang hexagon randconfig-r045-20230814 clang hexagon randconfig-r045-20230815 clang i386 allyesconfig gcc i386 buildonly-randconfig-r004-20230814 gcc i386 buildonly-randconfig-r004-20230815 clang i386 buildonly-randconfig-r005-20230814 gcc i386 buildonly-randconfig-r005-20230815 clang i386 buildonly-randconfig-r006-20230814 gcc i386 buildonly-randconfig-r006-20230815 clang i386 debian-10.3 gcc i386defconfig gcc i386 randconfig-i001-20230814 gcc i386 randconfig-i001-20230815 clang i386 randconfig-i002-20230814 gcc i386 randconfig-i002-20230815 clang i386 randconfig-i003-20230814 gcc i386 randconfig-i003-20230815 clang i386 randconfig-i004-20230814 gcc i386 randconfig-i004-20230815 clang i386 randconfig-i005-20230814 gcc i386 randconfig-i005-20230815 clang i386 randconfig-i006-20230814 gcc i386 randconfig-i006-20230815 clang i386
Re: [PATCH v6 17/25] iommu/qcom_iommu: Add an IOMMU_IDENTITIY_DOMAIN
On 2023/8/14 23:36, Jason Gunthorpe wrote:
On Mon, Aug 14, 2023 at 02:32:33PM +0800, Baolu Lu wrote:
+ pm_runtime_get_sync(qcom_iommu->dev);
+ for (i = 0; i < fwspec->num_ids; i++) {
+ struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain,
fwspec->ids[i]);
+
+ /* Disable the context bank: */
+ iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0);
+
+ ctx->domain = NULL;
Does setting ctx->domain to NULL still match this semantics?
Yes, I did not try to fix this driver. NULL means identity in the
ctx->domain.
Okay.
Best regards,
baolu
Re: [PATCH v6 07/25] iommu/mtk_iommu_v1: Implement an IDENTITY domain
On 2023/8/14 22:34, Jason Gunthorpe wrote:
@@ -443,7 +459,7 @@ static int mtk_iommu_v1_create_mapping(struct device *dev,
struct of_phandle_arg
static int mtk_iommu_v1_def_domain_type(struct device *dev)
{
- return IOMMU_DOMAIN_UNMANAGED;
+ return IOMMU_DOMAIN_IDENTITY;
def_domain_type can't be used for this purpose. But this seems to be a
temporary code, as it will be removed in patch 09/25.
It looked OK when I checked it, mkt_v1 is really confusing what it
tries to do, but it should call probe_finalize and basically do the
same hacky thing as what UNMANAGED was trying to accomplish.
Did you see something else?
No.
Best regards,
baolu
Re: [PATCH] I2C: Explicitly include correct DT includes
On Fri, Jul 14, 2023 at 11:46:16AM -0600, Rob Herring wrote: > The DT of_device.h and of_platform.h date back to the separate > of_platform_bus_type before it as merged into the regular platform bus. > As part of that merge prepping Arm DT support 13 years ago, they > "temporarily" include each other. They also include platform_device.h > and of.h. As a result, there's a pretty much random mix of those include > files used throughout the tree. In order to detangle these headers and > replace the implicit includes with struct declarations, users need to > explicitly include the correct includes. > > Signed-off-by: Rob Herring Applied to for-next, thanks! signature.asc Description: PGP signature
Re: [RFC PATCH v11 08/29] KVM: Introduce per-page memory attributes
On 7/19/2023 7:44 AM, Sean Christopherson wrote:
From: Chao Peng
In confidential computing usages, whether a page is private or shared is
necessary information for KVM to perform operations like page fault
handling, page zapping etc. There are other potential use cases for
per-page memory attributes, e.g. to make memory read-only (or no-exec,
or exec-only, etc.) without having to modify memslots.
Introduce two ioctls (advertised by KVM_CAP_MEMORY_ATTRIBUTES) to allow
userspace to operate on the per-page memory attributes.
- KVM_SET_MEMORY_ATTRIBUTES to set the per-page memory attributes to
a guest memory range.
- KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES to return the KVM supported
memory attributes.
Use an xarray to store the per-page attributes internally, with a naive,
not fully optimized implementation, i.e. prioritize correctness over
performance for the initial implementation.
Because setting memory attributes is roughly analogous to mprotect() on
memory that is mapped into the guest, zap existing mappings prior to
updating the memory attributes. Opportunistically provide an arch hook
for the post-set path (needed to complete invalidation anyways) in
s/anyways/anyway
anticipation of x86 needing the hook to update metadata related to
determining whether or not a given gfn can be backed with various sizes
of hugepages.
It's possible that future usages may not require an invalidation, e.g.
if KVM ends up supporting RWX protections and userspace grants _more_
protections, but again opt for simplicity and punt optimizations to
if/when they are needed.
Suggested-by: Sean Christopherson
Link: https://lore.kernel.org/all/y2wb48kd0j4vg...@google.com
Cc: Fuad Tabba
Signed-off-by: Chao Peng
Co-developed-by: Sean Christopherson
Signed-off-by: Sean Christopherson
---
Documentation/virt/kvm/api.rst | 60
include/linux/kvm_host.h | 14 +++
include/uapi/linux/kvm.h | 14 +++
virt/kvm/Kconfig | 4 +
virt/kvm/kvm_main.c| 170 +
5 files changed, 262 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 34d4ce66e0c8..0ca8561775ac 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6068,6 +6068,56 @@ writes to the CNTVCT_EL0 and CNTPCT_EL0 registers using
the SET_ONE_REG
interface. No error will be returned, but the resulting offset will not be
applied.
+4.139 KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES
+-
+
+:Capability: KVM_CAP_MEMORY_ATTRIBUTES
+:Architectures: x86
+:Type: vm ioctl
+:Parameters: u64 memory attributes bitmask(out)
+:Returns: 0 on success, <0 on error
+
+Returns supported memory attributes bitmask. Supported memory attributes will
+have the corresponding bits set in u64 memory attributes bitmask.
+
+The following memory attributes are defined::
+
+ #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3)
+
+4.140 KVM_SET_MEMORY_ATTRIBUTES
+-
+
+:Capability: KVM_CAP_MEMORY_ATTRIBUTES
+:Architectures: x86
+:Type: vm ioctl
+:Parameters: struct kvm_memory_attributes(in/out)
+:Returns: 0 on success, <0 on error
+
+Sets memory attributes for pages in a guest memory range. Parameters are
+specified via the following structure::
+
+ struct kvm_memory_attributes {
+ __u64 address;
+ __u64 size;
+ __u64 attributes;
+ __u64 flags;
+ };
+
+The user sets the per-page memory attributes to a guest memory range indicated
+by address/size, and in return KVM adjusts address and size to reflect the
+actual pages of the memory range have been successfully set to the attributes.
+If the call returns 0, "address" is updated to the last successful address + 1
+and "size" is updated to the remaining address size that has not been set
+successfully. The user should check the return value as well as the size to
+decide if the operation succeeded for the whole range or not. The user may want
+to retry the operation with the returned address/size if the previous range was
+partially successful.
+
+Both address and size should be page aligned and the supported attributes can
be
+retrieved with KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES.
+
+The "flags" field may be used for future extensions and should be set to 0s.
+
5. The kvm_run structure
@@ -8494,6 +8544,16 @@ block sizes is exposed in KVM_CAP_ARM_SUPPORTED_BLOCK_SIZES as a
64-bit bitmap (each bit describing a block size). The default value is
0, to disable the eager page splitting.
+8.41 KVM_CAP_MEMORY_ATTRIBUTES
+--
+
+:Capability: KVM_CAP_MEMORY_ATTRIBUTES
+:Architectures: x86
+:Type: vm
+
+This capability indicates KVM supports per-page memory attributes and ioctls
+KVM_GET_SUPPORTED_MEMORY_ATTRIBUTES/KVM_SET_MEMORY_ATTRIBUTES are available.
+
9. Known KVM API problems
=
Re: [PATCH v6 15/25] iommufd/selftest: Make the mock iommu driver into a real driver
On Wed, Aug 02, 2023 at 09:08:02PM -0300, Jason Gunthorpe wrote:
> I've avoided doing this because there is no way to make this happen
> without an intrusion into the core code. Up till now this has avoided
> needing the core code's probe path with some hackery - but now that
> default domains are becoming mandatory it is unavoidable. The core probe
> path must be run to set the default_domain, only it can do it. Without
> a default domain iommufd can't use the group.
>
> Make it so that iommufd selftest can create a real iommu driver and bind
> it only to is own private bus. Add iommu_device_register_bus() as a core
> code helper to make this possible. It simply sets the right pointers and
> registers the notifier block. The mock driver then works like any normal
> driver should, with probe triggered by the bus ops
>
> When the bus->iommu_ops stuff is fully unwound we can probably do better
> here and remove this special case.
>
> Remove set_platform_dma_ops from selftest and make it use a BLOCKED
> default domain.
>
> Signed-off-by: Jason Gunthorpe
> ---
> drivers/iommu/iommu-priv.h | 16 +++
> drivers/iommu/iommu.c | 43 +++
> drivers/iommu/iommufd/iommufd_private.h | 5 +-
> drivers/iommu/iommufd/main.c| 8 +-
> drivers/iommu/iommufd/selftest.c| 149 +---
> 5 files changed, 152 insertions(+), 69 deletions(-)
> create mode 100644 drivers/iommu/iommu-priv.h
Since this series will miss this kernel again I've taken this patch
into the iommufd tree to fix the broken selftest.
It needed two edits to make it work out of the context of this series
The core code still requires empty free functions:
+@@ drivers/iommu/iommufd/selftest.c: static struct iommu_domain
*mock_domain_alloc(unsigned int iommu_domain_type)
+ if (iommu_domain_type == IOMMU_DOMAIN_BLOCKED)
+ return _blocking_domain;
--static void mock_domain_blocking_free(struct iommu_domain *domain)
--{
--}
--
- static int mock_domain_nop_attach(struct iommu_domain *domain,
- struct device *dev)
- {
+@@ drivers/iommu/iommufd/selftest.c: static void
mock_domain_set_plaform_dma_ops(struct device *dev)
+*/
}
- static const struct iommu_domain_ops mock_blocking_ops = {
-- .free = mock_domain_blocking_free,
- .attach_dev = mock_domain_nop_attach,
- };
-
And we can't use default_domain so rely on a NULL default domain and
set_platform_dma_ops:
-@@ drivers/iommu/iommufd/selftest.c: static int
mock_domain_nop_attach(struct iommu_domain *domain,
+- if (WARN_ON(iommu_domain_type != IOMMU_DOMAIN_UNMANAGED))
++ if (iommu_domain_type != IOMMU_DOMAIN_UNMANAGED)
+ return NULL;
+
+ mock = kzalloc(sizeof(*mock), GFP_KERNEL);
-@@ drivers/iommu/iommufd/selftest.c: 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)
+static struct iommu_device mock_iommu_device = {
+};
+
+static struct iommu_device *mock_probe_device(struct device *dev)
- {
-- /*
-- * mock doesn't setup default domains because we can't hook into the
-- * normal probe path
-- */
++{
+ return _iommu_device;
- }
-
++}
++
static const struct iommu_ops mock_ops = {
-+ /*
-+ * IOMMU_DOMAIN_BLOCKED cannot be returned from def_domain_type()
-+ * because it is zero.
-+ */
-+ .default_domain = _blocking_domain,
.owner = THIS_MODULE,
.pgsize_bitmap = MOCK_IO_PAGE_SIZE,
.domain_alloc = mock_domain_alloc,
.capable = mock_domain_capable,
-- .set_platform_dma_ops = mock_domain_set_plaform_dma_ops,
+ .set_platform_dma_ops = mock_domain_set_plaform_dma_ops,
+ .device_group = generic_device_group,
+ .probe_device = mock_probe_device,
.default_domain_ops =
Otherwise it works the same and solves the same problem. There is a
small merge conflict with the fixes in Joerg's tree around the
bus_iommu_probe
Thanks,
Jason
Re: [PATCH RFC 2/3] powerpc/ps3: refactor strncpy usage attempt 2
On Fri, Aug 11, 2023 at 09:19:20PM +, Justin Stitt wrote: > This approach tries to use `make_field` inside of `make_first_field`. > This comes with some weird implementation as to get the same result we > need to first subtract `index` from the `make_field` result whilst being > careful with order of operations. We then have to add index back. I think for readability, it's better to avoid the function composition. The index subtraction undoes the earlier addition -- I say just leave it separate. i.e. I like option 1 of 3 the best. -Kees -- Kees Cook
Re: [RFC PATCH v11 08/29] KVM: Introduce per-page memory attributes
On Mon, Aug 14, 2023, Binbin Wu wrote:
>
> On 7/19/2023 7:44 AM, Sean Christopherson wrote:
> > + struct kvm_mmu_notifier_range post_set_range = {
> > + .start = start,
> > + .end = end,
> > + .arg.attributes = attributes,
> > + .handler = kvm_arch_post_set_memory_attributes,
> > + .on_lock = (void *)kvm_null_fn,
> > + .on_unlock = kvm_mmu_invalidate_end,
> > + .may_block = true,
> > + };
> > + unsigned long i;
> > + void *entry;
> > + int r;
> > +
> > + entry = attributes ? xa_mk_value(attributes) : NULL;
> Why attributes of value 0 is considered not a value? Is it because 0 is not
> a valid value when RWX is considered in the future?
0 values don't require an entry in the xarray, i.e. don't need to be stored and
so don't consume memory. The potential conflict with a RWX=0 entry has already
been noted, but we'll cross that bridge when we get to it, e.g. KVM can easily
support RWX=0 by using an internal "valid" flag.
> Both the changelog and the document added mention that the address and size
> of attrs will be updated to
> "reflect the actual pages of the memory range have been successfully set to
> the attributes", but it doesn't.
Yeah, on the todo list, all of the changelogs are horribly stale.
[BUG] Re: [PATCH v3 10/13] mm/khugepaged: collapse_pte_mapped_thp() with mmap_read_lock()
On Wed, Jul 12, 2023 at 6:42 AM Hugh Dickins wrote:
> Bring collapse_and_free_pmd() back into collapse_pte_mapped_thp().
> It does need mmap_read_lock(), but it does not need mmap_write_lock(),
> nor vma_start_write() nor i_mmap lock nor anon_vma lock. All racing
> paths are relying on pte_offset_map_lock() and pmd_lock(), so use those.
We can still have a racing userfaultfd operation at the "/* step 4:
remove page table */" point that installs a new PTE before the page
table is removed.
To reproduce, patch a delay into the kernel like this:
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 9a6e0d507759..27cc8dfbf3a7 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1617,6 +1618,11 @@ int collapse_pte_mapped_thp(struct mm_struct
*mm, unsigned long addr,
}
/* step 4: remove page table */
+ if (strcmp(current->comm, "DELAYME") == 0) {
+ pr_warn("%s: BEGIN DELAY INJECTION\n", __func__);
+ mdelay(5000);
+ pr_warn("%s: END DELAY INJECTION\n", __func__);
+ }
/* Huge page lock is still held, so page table must remain empty */
pml = pmd_lock(mm, pmd);
And then run the attached reproducer against mm/mm-everything. You
should get this in dmesg:
[ 206.578096] BUG: Bad rss-counter state mm:0942ebea
type:MM_ANONPAGES val:1
// compile with "gcc -o khugepaged-vs-uffd khugepaged-vs-uffd.c -pthread"
#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#ifndef MADV_COLLAPSE
#define MADV_COLLAPSE 25
#endif
#ifndef UFFD_USER_MODE_ONLY
#define UFFD_USER_MODE_ONLY 1
#endif
#define SYSCHK(x) ({ \
typeof(x) __res = (x); \
if (__res == (typeof(x))-1) \
err(1, "SYSCHK(" #x ")"); \
__res; \
})
static void write_file(char *name, char *buf) {
int fd = SYSCHK(open(name, O_WRONLY));
if (write(fd, buf, strlen(buf)) != strlen(buf))
err(1, "write %s", name);
close(fd);
}
static void write_map(char *name, int outer_id) {
char buf[100];
sprintf(buf, "0 %d 1", outer_id);
write_file(name, buf);
}
static void *thread_fn(void *dummy) {
system("head -n50 /proc/$PPID/smaps;echo;echo");
SYSCHK(prctl(PR_SET_NAME, "DELAYME"));
SYSCHK(madvise((void*)0x20UL, 0x20, MADV_COLLAPSE));
SYSCHK(prctl(PR_SET_NAME, "thread"));
system("head -n50 /proc/$PPID/smaps");
return NULL;
}
int main(void) {
int outer_uid = getuid();
int outer_gid = getgid();
SYSCHK(unshare(CLONE_NEWNS|CLONE_NEWUSER));
SYSCHK(mount(NULL, "/", NULL, MS_PRIVATE|MS_REC, NULL));
write_file("/proc/self/setgroups", "deny");
write_map("/proc/self/uid_map", outer_uid);
write_map("/proc/self/gid_map", outer_gid);
SYSCHK(mount("none", "/tmp", "tmpfs", MS_NOSUID|MS_NODEV, "huge=always"));
int fd = SYSCHK(open("/tmp/a", O_RDWR|O_CREAT, 0600));
SYSCHK(ftruncate(fd, 0x20));
void *ptr = SYSCHK(mmap((void*)0x20UL, 0x10, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED_NOREPLACE, fd, 0));
*(volatile char *)ptr;
SYSCHK(mmap((void*)0x30UL, 0x10, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED_NOREPLACE, fd, 0x10));
for (int i=0; i<512; i++)
*(volatile char *)(0x20UL + 0x1000 * i);
int uffd = SYSCHK(syscall(__NR_userfaultfd, UFFD_USER_MODE_ONLY));
struct uffdio_api api = { .api = UFFD_API, .features = 0 };
SYSCHK(ioctl(uffd, UFFDIO_API, ));
struct uffdio_register reg = {
.range = { .start = 0x20, .len = 0x20 },
.mode = UFFDIO_REGISTER_MODE_MISSING
};
SYSCHK(ioctl(uffd, UFFDIO_REGISTER, ));
pthread_t thread;
if (pthread_create(, NULL, thread_fn, NULL))
errx(1, "pthread_create");
sleep(1);
unsigned char dummy_page[0x1000] = {1};
struct uffdio_copy copy = {
.dst = 0x201000,
.src = (unsigned long)dummy_page,
.len = 0x1000,
.mode = 0
};
SYSCHK(ioctl(uffd, UFFDIO_COPY, ));
if (pthread_join(thread, NULL))
errx(1, "pthread_join");
//system("cat /proc/$PPID/smaps");
}
Re: [PATCH] powerpc: Make virt_to_pfn() a static inline
On Mon, Aug 14, 2023 at 2:37 PM Michael Ellerman wrote:
> > +static inline const void *pfn_to_kaddr(unsigned long pfn)
> > +{
> > + return (const void *)(((unsigned long)__va(pfn)) << PAGE_SHIFT);
>
> Any reason to do it this way rather than:
>
> + return __va(pfn << PAGE_SHIFT);
>
> Seems to be equivalent and much cleaner?
I was afraid of changing the semantic in the original macro
which converts to a virtual address before shifting, instead
of shifting first, but you're right, I'm too cautious. I'll propose
the elegant solution from you & Christophe instead!
Yours,
Linus Walleij
Re: [PATCH v3 6/6] integrity: PowerVM support for loading third party code signing keys
On Sun Aug 13, 2023 at 5:15 AM EEST, Nayna Jain wrote:
> On secure boot enabled PowerVM LPAR, third party code signing keys are
> needed during early boot to verify signed third party modules. These
> third party keys are stored in moduledb object in the Platform
> KeyStore(PKS).
>
> Load third party code signing keys onto .secondary_trusted_keys keyring.
>
> Signed-off-by: Nayna Jain
> ---
> certs/system_keyring.c| 30 +++
> include/keys/system_keyring.h | 7 +
> security/integrity/integrity.h| 1 +
> .../platform_certs/keyring_handler.c | 8 +
> .../platform_certs/keyring_handler.h | 5
> .../integrity/platform_certs/load_powerpc.c | 18 ++-
> 6 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index b348e0898d34..e458d414918d 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -396,3 +396,33 @@ void __init set_platform_trusted_keys(struct key
> *keyring)
> platform_trusted_keys = keyring;
> }
> #endif
> +
> +/**
> + * add_to_secondary_keyring - Add to secondary keyring.
> + * @source: Source of key
> + * @data: The blob holding the key
> + * @len: The length of the data blob
> + *
> + * Add a key to the secondary keyring. The key must be vouched for by a key
> in the builtin,
> + * machine or secondary keyring itself.
> + */
> +void __init add_to_secondary_keyring(const char *source, const void *data,
> size_t len)
> +{
> + key_ref_t key;
> + key_perm_t perm;
> +
> + perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW;
> +
> + key = key_create_or_update(make_key_ref(secondary_trusted_keys, 1),
> +"asymmetric",
> +NULL, data, len, perm,
> +KEY_ALLOC_NOT_IN_QUOTA);
> + if (IS_ERR(key)) {
> + pr_err("Problem loading X.509 certificate from %s to secondary
> keyring %ld\n",
> +source, PTR_ERR(key));
> + return;
> + }
> +
> + pr_notice("Loaded X.509 cert '%s'\n", key_ref_to_ptr(key)->description);
> + key_ref_put(key);
> +}
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index 7e2583208820..4188f75d1bac 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -50,9 +50,16 @@ int restrict_link_by_digsig_builtin_and_secondary(struct
> key *keyring,
> const struct key_type *type,
> const union key_payload
> *payload,
> struct key *restriction_key);
> +void __init add_to_secondary_keyring(const char *source, const void *data,
> + size_t len);
> +
> #else
> #define restrict_link_by_builtin_and_secondary_trusted
> restrict_link_by_builtin_trusted
> #define restrict_link_by_digsig_builtin_and_secondary
> restrict_link_by_digsig_builtin
> +void __init add_to_secondary_keyring(const char *source, const void *data,
> + size_t len)
> +{
> +}
> #endif
>
> #ifdef CONFIG_INTEGRITY_MACHINE_KEYRING
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index d7553c93f5c0..efaa2eb789ad 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -228,6 +228,7 @@ static inline int __init integrity_load_cert(const
> unsigned int id,
> {
> return 0;
> }
> +
> #endif /* CONFIG_INTEGRITY_SIGNATURE */
>
> #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
> diff --git a/security/integrity/platform_certs/keyring_handler.c
> b/security/integrity/platform_certs/keyring_handler.c
> index 586027b9a3f5..13ea17207902 100644
> --- a/security/integrity/platform_certs/keyring_handler.c
> +++ b/security/integrity/platform_certs/keyring_handler.c
> @@ -78,6 +78,14 @@ __init efi_element_handler_t get_handler_for_ca_keys(const
> efi_guid_t *sig_type)
> return NULL;
> }
>
> +__init efi_element_handler_t get_handler_for_code_signing_keys(const
> efi_guid_t *sig_type)
> +{
> + if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0)
> + return add_to_secondary_keyring;
> +
> + return NULL;
> +}
> +
> /*
> * Return the appropriate handler for particular signature list types found
> in
> * the UEFI dbx and MokListXRT tables.
> diff --git a/security/integrity/platform_certs/keyring_handler.h
> b/security/integrity/platform_certs/keyring_handler.h
> index 6f15bb4cc8dc..f92895cc50f6 100644
> --- a/security/integrity/platform_certs/keyring_handler.h
> +++ b/security/integrity/platform_certs/keyring_handler.h
> @@ -34,6 +34,11 @@ efi_element_handler_t get_handler_for_mok(const efi_guid_t
> *sig_type);
> */
> efi_element_handler_t get_handler_for_ca_keys(const efi_guid_t *sig_type);
>
> +/*
> + *
Re: [PATCH v3 2/6] integrity: ignore keys failing CA restrictions on non-UEFI platform
On Sun Aug 13, 2023 at 5:15 AM EEST, Nayna Jain wrote: > On non-UEFI platforms, handle restrict_link_by_ca failures differently. > > Certificates which do not satisfy CA restrictions on non-UEFI platforms > are ignored. > > Signed-off-by: Nayna Jain > Reviewed-and-tested-by: Mimi Zohar > --- > security/integrity/platform_certs/machine_keyring.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/integrity/platform_certs/machine_keyring.c > b/security/integrity/platform_certs/machine_keyring.c > index 7aaed7950b6e..389a6e7c9245 100644 > --- a/security/integrity/platform_certs/machine_keyring.c > +++ b/security/integrity/platform_certs/machine_keyring.c > @@ -36,7 +36,7 @@ void __init add_to_machine_keyring(const char *source, > const void *data, size_t >* If the restriction check does not pass and the platform keyring >* is configured, try to add it into that keyring instead. >*/ > - if (rc && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) > + if (rc && efi_enabled(EFI_BOOT) && > IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) > rc = integrity_load_cert(INTEGRITY_KEYRING_PLATFORM, source, >data, len, perm); > > -- > 2.31.1 Acked-by: Jarkko Sakkinen BR, Jarkko
Re: [PATCH v3 1/6] integrity: PowerVM support for loading CA keys on machine keyring
On Sun Aug 13, 2023 at 5:15 AM EEST, Nayna Jain wrote:
> Keys that derive their trust from an entity such as a security officer,
> administrator, system owner, or machine owner are said to have "imputed
> trust". CA keys with imputed trust can be loaded onto the machine keyring.
> The mechanism for loading these keys onto the machine keyring is platform
> dependent.
>
> Load keys stored in the variable trustedcadb onto the .machine keyring
> on PowerVM platform.
>
> Signed-off-by: Nayna Jain
> Reviewed-and-tested-by: Mimi Zohar
> ---
> .../integrity/platform_certs/keyring_handler.c | 8
> .../integrity/platform_certs/keyring_handler.h | 5 +
> .../integrity/platform_certs/load_powerpc.c | 17 +
> 3 files changed, 30 insertions(+)
>
> diff --git a/security/integrity/platform_certs/keyring_handler.c
> b/security/integrity/platform_certs/keyring_handler.c
> index 8a1124e4d769..1649d047e3b8 100644
> --- a/security/integrity/platform_certs/keyring_handler.c
> +++ b/security/integrity/platform_certs/keyring_handler.c
> @@ -69,6 +69,14 @@ __init efi_element_handler_t get_handler_for_mok(const
> efi_guid_t *sig_type)
> return NULL;
> }
>
> +__init efi_element_handler_t get_handler_for_ca_keys(const efi_guid_t
> *sig_type)
> +{
> + if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0)
> + return add_to_machine_keyring;
> +
> + return NULL;
> +}
> +
> /*
> * Return the appropriate handler for particular signature list types found
> in
> * the UEFI dbx and MokListXRT tables.
> diff --git a/security/integrity/platform_certs/keyring_handler.h
> b/security/integrity/platform_certs/keyring_handler.h
> index 212d894a8c0c..6f15bb4cc8dc 100644
> --- a/security/integrity/platform_certs/keyring_handler.h
> +++ b/security/integrity/platform_certs/keyring_handler.h
> @@ -29,6 +29,11 @@ efi_element_handler_t get_handler_for_db(const efi_guid_t
> *sig_type);
> */
> efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type);
>
> +/*
> + * Return the handler for particular signature list types for CA keys.
> + */
> +efi_element_handler_t get_handler_for_ca_keys(const efi_guid_t *sig_type);
> +
> /*
> * Return the handler for particular signature list types found in the dbx.
> */
> diff --git a/security/integrity/platform_certs/load_powerpc.c
> b/security/integrity/platform_certs/load_powerpc.c
> index 170789dc63d2..6263ce3b3f1e 100644
> --- a/security/integrity/platform_certs/load_powerpc.c
> +++ b/security/integrity/platform_certs/load_powerpc.c
> @@ -59,6 +59,7 @@ static __init void *get_cert_list(u8 *key, unsigned long
> keylen, u64 *size)
> static int __init load_powerpc_certs(void)
> {
> void *db = NULL, *dbx = NULL, *data = NULL;
> + void *trustedca = NULL;
Could this be just "void *trustedca;" ?
> u64 dsize = 0;
> u64 offset = 0;
> int rc = 0;
> @@ -120,6 +121,22 @@ static int __init load_powerpc_certs(void)
> kfree(data);
> }
>
> + data = get_cert_list("trustedcadb", 12, );
> + if (!data) {
> + pr_info("Couldn't get trustedcadb list from firmware\n");
> + } else if (IS_ERR(data)) {
> + rc = PTR_ERR(data);
> + pr_err("Error reading trustedcadb from firmware: %d\n", rc);
> + } else {
> + extract_esl(trustedca, data, dsize, offset);
> +
> + rc = parse_efi_signature_list("powerpc:trustedca", trustedca,
> dsize,
> + get_handler_for_ca_keys);
> + if (rc)
> + pr_err("Couldn't parse trustedcadb signatures: %d\n",
> rc);
> + kfree(data);
> + }
> +
> return rc;
> }
> late_initcall(load_powerpc_certs);
> --
> 2.31.1
BR, Jarkko
Re: [PATCH v6 00/25] iommu: Make default_domain's mandatory
On Mon, Aug 14, 2023 at 04:43:23PM +0800, Baolu Lu wrote:
> > This is on
> > github:https://github.com/jgunthorpe/linux/commits/iommu_all_defdom
>
> It seems that after this series, all ARM iommu drivers are able to
> support the IDENTITY default domain, hence perhaps we can remove below
> code?
Yes, but this code is still used
> If I remember it correctly, the background of this part of code is
> that some arm drivers didn't support IDENTITY domain, so fall back to
> DMA domain if IDENTITY domain allocation fails.
Not quite..
if (req_type)
return __iommu_group_alloc_default_domain(group, req_type);
req_type == 0 can still happen because it depends on what
def_domain_type returns, which is still 0 in alot of cases
/* The driver gave no guidance on what type to use, try the default */
dom = __iommu_group_alloc_default_domain(group, iommu_def_domain_type);
if (dom)
return dom;
So we try the default which might be IDENTITY/DMA/DMA_FQ - still have
to do this.
/* Otherwise IDENTITY and DMA_FQ defaults will try DMA */
if (iommu_def_domain_type == IOMMU_DOMAIN_DMA)
return NULL;
dom = __iommu_group_alloc_default_domain(group, IOMMU_DOMAIN_DMA);
if (!dom)
return NULL;
pr_warn("Failed to allocate default IOMMU domain of type %u for group
%s - Falling back to IOMMU_DOMAIN_DMA",
iommu_def_domain_type, group->name);
And this hunk is primarily a fallback in case the DMA_FQ didn't
work. Then we try normal DMA.
That it also protected against not implementing IDENTITY is a side
effect, so I think we have to keep all of this still.
Jason
Re: KASAN debug kernel fails to boot at early stage when CONFIG_SMP=y is set (kernel 6.5-rc5, PowerMac G4 3,6)
On Mon, 14 Aug 2023 09:40:44 +
Christophe Leroy wrote:
> Interesting. That means we get stuck somewhere around MMU_init()
>
> We know that MMU_init_hw() is called and runs at least until:
>
> pr_info("Total memory = %lldMB; using %ldkB for hash table\n",
> (unsigned long long)(total_memory >> 20), Hash_size >> 10);
>
> But we never reach the print in setup_kuap() which is itself called by
> set_kup():
> pr_info("Activating Kernel Userspace Access Protection\n");
>
>
> Could you try to narrow more the issue by spreading pr_info() at places
> in the code below and/or the called functions ? Either we never come
> back from MMU_init_hw(), or one of mapin_ram() btext_unmap()
> kasan_mmu_init() fails.
>
> So the piece of code we are interested in is located in
> arch/powerpc/mm/init_32.c and is:
>
> /* Initialize the MMU hardware */
> if (ppc_md.progress)
> ppc_md.progress("MMU:hw init", 0x300);
> ==> MMU_init_hw();
>
> /* Map in all of RAM starting at KERNELBASE */
> if (ppc_md.progress)
> ppc_md.progress("MMU:mapin", 0x301);
> mapin_ram();
>
> /* Initialize early top-down ioremap allocator */
> ioremap_bot = IOREMAP_TOP;
>
> if (ppc_md.progress)
> ppc_md.progress("MMU:exit", 0x211);
>
> /* From now on, btext is no longer BAT mapped if it was at all */
> #ifdef CONFIG_BOOTX_TEXT
> btext_unmap();
> #endif
>
> kasan_mmu_init();
>
> ==> setup_kup();
I added a pr_info(); right after MMU_init_hw(); and another one right after
setup_kup();.
Output of PPC_EARLY_DEBUG changes so that I get an additional black blank line
after
[0.00] printk: bootconsole [udbg0] enabled
[0.00] Total memory = 2048MB; using 4096kB for hash table
and the freeze afterwards. So it looks like we return from MMU_init_hw() but
not from setup_kup().
The dmesg of a warm boot (after first booting with kernel 6.4.10) supports that
as it also shows the 1st blank line and the 2nd one just after activating
KUEP/KUAP:
[0.00] printk: bootconsole [udbg0] enabled
[0.00] Total memory = 2048MB; using 4096kB for hash table
[0.00]
[0.00] Activating Kernel Userspace Access Protection
[0.00] Activating Kernel Userspace Execution Prevention
[0.00]
[0.00] Linux version 6.5.0-rc6-PMacG4-dirty (root@T1000) (gcc (Gentoo
12.3.1_p20230526 p2) 12.3.1 20230526, GNU ld (Gentoo 2.40 p7) 2.40.0) #1 SMP
Mon Aug 14 18:05:17 CEST 2023
As the 2nd blank line from pr_info() is just after KUAP, KUEP initialization I
thought these might be a problem. But if I deactivate KUAP/KUAP in the kernel
.config I still get the freeze sot it must be something else.
Regards,
Erhard
dmesg_65-rc6_g4-
Description: Binary data
Re: [PATCH v6 08/25] iommu: Reorganize iommu_get_default_domain_type() to respect def_domain_type()
On Sat, Aug 12, 2023 at 10:15:42AM +0800, Baolu Lu wrote:
>
> How about consolidating above into a single helper?
>
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1787,6 +1787,21 @@ __iommu_group_alloc_default_domain(struct iommu_group
> *group, int req_type)
> return __iommu_group_domain_alloc(group, req_type);
> }
>
> +/*
> + * Returns the iommu_ops for the devices in an iommu group.
> + *
> + * It is assumed that all devices in an iommu group are managed by a single
> + * IOMMU unit. Therefore, this returns the dev_iommu_ops of the first
> device
> + * in the group.
> + */
> +static const struct iommu_ops *group_iommu_ops(struct iommu_group *group)
> +{
> + struct group_device *device;
> +
> + device = list_first_entry(>devices, struct group_device, list);
> + return dev_iommu_ops(device->dev);
> +}
Okay I did this, but it doesn't help as much..
> @@ -2124,13 +2134,9 @@ static struct iommu_domain
> *__iommu_domain_alloc(const struct iommu_ops *ops,
> static struct iommu_domain *
> __iommu_group_domain_alloc(struct iommu_group *group, unsigned int type)
> {
> - struct device *dev =
> - list_first_entry(>devices, struct group_device, list)
> - ->dev;
> -
> lockdep_assert_held(>mutex);
>
> - return __iommu_domain_alloc(dev_iommu_ops(dev), dev, type);
> + return __iommu_domain_alloc(group_iommu_ops(group), dev, type);
> }
Since this is all still needed to calculate dev
> > +err:
> > + if (target_type) {
> > + dev_err_ratelimited(
> > + gdev->dev,
> > + "Device cannot be in %s domain - it is forcing %s\n",
> > + iommu_domain_type_str(target_type),
> > + iommu_domain_type_str(type));
> > + return -1;
> > + }
> > +
> > + dev_warn(
> > + gdev->dev,
> > + "Device needs domain type %s, but device %s in the same iommu
> > group requires type %s - using default\n",
> > + iommu_domain_type_str(type), dev_name(last_dev),
> > + iommu_domain_type_str(best_type));
> > + return 0;
>
> This doesn't match the commit message, where it states:
>
> "Arrange things so that if the driver says it needs IDENTITY then
> iommu_get_default_domain_type() either fails or returns IDENTITY.
> "
>
> I saw that this change was made in the sequential patch. It is probably
> better to put that here?
Ah, I went over all this again and decided to try again, it is too
complicated. This patch can do what the commit message says and the
following patches are even simpler:
/*
* Combine the driver's choosen def_domain_type across all the devices in a
* group. Drivers must give a consistent result.
*/
static int iommu_get_def_domain_type(struct iommu_group *group,
struct device *dev, int cur_type)
{
const struct iommu_ops *ops = group_iommu_ops(group);
int type;
if (!ops->def_domain_type)
return cur_type;
type = ops->def_domain_type(dev);
if (!type || cur_type == type)
return cur_type;
if (!cur_type)
return type;
dev_err_ratelimited(
dev,
"IOMMU driver error, requesting conflicting def_domain_type, %s
and %s, for devices in group %u.\n",
iommu_domain_type_str(cur_type), iommu_domain_type_str(type),
group->id);
/*
* Try to recover, drivers are allowed to force IDENITY or DMA, IDENTITY
* takes precedence.
*/
if (cur_type || type == IOMMU_DOMAIN_IDENTITY)
return IOMMU_DOMAIN_IDENTITY;
return cur_type;
}
/*
* A target_type of 0 will select the best domain type. 0 can be returned in
* this case meaning the global default should be used.
*/
static int iommu_get_default_domain_type(struct iommu_group *group,
int target_type)
{
struct device *untrusted = NULL;
struct group_device *gdev;
int driver_type = 0;
lockdep_assert_held(>mutex);
/*
* ARM32 drivers supporting CONFIG_ARM_DMA_USE_IOMMU can declare an
* identity_domain and it will automatically become their default
* domain. Later on ARM_DMA_USE_IOMMU will install its UNMANAGED domain.
* Override the selection to IDENTITY.
*/
if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
static_assert(!(IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) &&
IS_ENABLED(CONFIG_IOMMU_DMA)));
driver_type = IOMMU_DOMAIN_IDENTITY;
}
for_each_group_device(group, gdev) {
driver_type = iommu_get_def_domain_type(group, gdev->dev,
driver_type);
if (dev_is_pci(gdev->dev) && to_pci_dev(gdev->dev)->untrusted) {
Re: [PATCH v6 24/25] iommu: Convert simple drivers with DOMAIN_DMA to domain_alloc_paging()
On Mon, Aug 14, 2023 at 02:58:14PM +0800, Baolu Lu wrote: > > diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c > > index 0bf08b120cf105..056832a367c2af 100644 > > --- a/drivers/iommu/sun50i-iommu.c > > +++ b/drivers/iommu/sun50i-iommu.c > > @@ -667,14 +667,11 @@ static phys_addr_t sun50i_iommu_iova_to_phys(struct > > iommu_domain *domain, > > sun50i_iova_get_page_offset(iova); > > } > > -static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type) > > +static struct iommu_domain * > > +sun50i_iommu_domain_alloc_paging(struct device *paging) > > Why not "struct device *dev"? > > Typo? Or anything I missed? Typo, I fixed it Thanks, Jason
Re: [PATCHv3 pci-next 1/2] PCI/AER: correctable error message as KERN_INFO
On Sat, Aug 12, 2023 at 5:45 PM David Heidelberg wrote: > > Tested-by: David Heidelberg Thanks David! > For PATCH v4 please fix the typo reported by the bot :) Sorry - I'll do that today. > Seeing messages as > > __aer_print_error: 72 callbacks suppressed > > but it still prints many errors on my laptop. Anyway, the log is less > filled with this patch, so great! Awesome! That's what I'm hoping for. :) cheers, grant > > > Thank you > David > > -- > David Heidelberg > Certified Linux Magician >
Re: [PATCH v6 20/25] iommu/sun50i: Add an IOMMU_IDENTITIY_DOMAIN
On Mon, Aug 14, 2023 at 02:44:50PM +0800, Baolu Lu wrote:
> On 2023/8/3 8:08, 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(-)
> >
> > diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
> > index 74c5cb93e90027..0bf08b120cf105 100644
> > --- a/drivers/iommu/sun50i-iommu.c
> > +++ b/drivers/iommu/sun50i-iommu.c
> > @@ -757,21 +757,32 @@ static void sun50i_iommu_detach_domain(struct
> > sun50i_iommu *iommu,
> > iommu->domain = NULL;
> > }
> > -static void sun50i_iommu_detach_device(struct iommu_domain *domain,
> > - struct device *dev)
> > +static int sun50i_iommu_identity_attach(struct iommu_domain
> > *identity_domain,
> > + struct device *dev)
> > {
> > - struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
> > struct sun50i_iommu *iommu = dev_iommu_priv_get(dev);
> > + struct sun50i_iommu_domain *sun50i_domain;
> > dev_dbg(dev, "Detaching from IOMMU domain\n");
> > - if (iommu->domain != domain)
> > - return;
> > + if (iommu->domain == identity_domain)
> > + return 0;
> > + sun50i_domain = to_sun50i_domain(iommu->domain);
> > if (refcount_dec_and_test(_domain->refcnt))
> > sun50i_iommu_detach_domain(iommu, sun50i_domain);
> > + return 0;
> > }
>
> Does iommu->domain need to be set to identity_domain before returning?
sun50i_iommu_detach_domain() does it.
This driver is confused because it uses generic_single_device_group
but also has this weird refcounting stuff. It should just make the
first attach alter the HW and have the remaining ones (eg new domain
== current domani) be NOPs. It should not refcount.
Jason
Re: [PATCH v6 17/25] iommu/qcom_iommu: Add an IOMMU_IDENTITIY_DOMAIN
On Mon, Aug 14, 2023 at 02:32:33PM +0800, Baolu Lu wrote:
> > + pm_runtime_get_sync(qcom_iommu->dev);
> > + for (i = 0; i < fwspec->num_ids; i++) {
> > + struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain,
> > fwspec->ids[i]);
> > +
> > + /* Disable the context bank: */
> > + iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0);
> > +
> > + ctx->domain = NULL;
>
> Does setting ctx->domain to NULL still match this semantics?
Yes, I did not try to fix this driver. NULL means identity in the
ctx->domain.
Thanks,
Jason
Re: [PATCH v6 12/25] iommu/tegra-smmu: Support DMA domains in tegra
On Mon, Aug 14, 2023 at 01:08:39PM +0800, Baolu Lu wrote:
> > @@ -989,6 +989,12 @@ static int tegra_smmu_def_domain_type(struct device
> > *dev)
> > }
> > static const struct iommu_ops tegra_smmu_ops = {
> > + /*
> > +* FIXME: For now we want to run all translation in IDENTITY mode,
> > +* better would be to have a def_domain_type op do this for just the
> > +* quirky device.
> > +*/
> > + .default_domain = _smmu_identity_domain,
>
> tegra_smmu_def_domain_type() has already forced the core to use
> ops->identity_domain, why do we still need ops->default_domain?
This looks like it is just some cruft from an earlier version that did
not have tegra_smmu_def_domain_type(), I deleted it
Thanks,
Jason
Re: [PATCH v6 07/25] iommu/mtk_iommu_v1: Implement an IDENTITY domain
On Mon, Aug 14, 2023 at 11:06:12AM +0800, Baolu Lu wrote:
> On 2023/8/3 8:07, 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.
>
> This code seems to be not working properly.
>
> * @def_domain_type: device default domain type, return value:
> * - IOMMU_DOMAIN_IDENTITY: must use an identity domain
> * - IOMMU_DOMAIN_DMA: must use a dma domain
> * - 0: use the default setting
>
> The core code is not ready to accept any other return value.
Right, it is not following the spec. The design does do what it is
supposed to though..
> > diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> > index 8a0a5e5d049f4a..cc3e7d53d33ad9 100644
> > --- a/drivers/iommu/mtk_iommu_v1.c
> > +++ b/drivers/iommu/mtk_iommu_v1.c
> > @@ -319,11 +319,27 @@ static int mtk_iommu_v1_attach_device(struct
> > iommu_domain *domain, struct device
> > return 0;
> > }
> > -static void mtk_iommu_v1_set_platform_dma(struct device *dev)
> > +static int mtk_iommu_v1_identity_attach(struct iommu_domain
> > *identity_domain,
> > + struct device *dev)
> > {
> > struct mtk_iommu_v1_data *data = dev_iommu_priv_get(dev);
> > mtk_iommu_v1_config(data, dev, false);
> > + return 0;
> > +}
> > +
> > +static struct iommu_domain_ops mtk_iommu_v1_identity_ops = {
> > + .attach_dev = mtk_iommu_v1_identity_attach,
> > +};
> > +
> > +static struct iommu_domain mtk_iommu_v1_identity_domain = {
> > + .type = IOMMU_DOMAIN_IDENTITY,
> > + .ops = _iommu_v1_identity_ops,
> > +};
> > +
> > +static void mtk_iommu_v1_set_platform_dma(struct device *dev)
> > +{
> > + mtk_iommu_v1_identity_attach(_iommu_v1_identity_domain, dev);
>
> This callback seems to be a dead code now.
Not yet in the series, it is still used. All this patch does is
introduce the identity domain.
> > @@ -443,7 +459,7 @@ static int mtk_iommu_v1_create_mapping(struct device
> > *dev, struct of_phandle_arg
> > static int mtk_iommu_v1_def_domain_type(struct device *dev)
> > {
> > - return IOMMU_DOMAIN_UNMANAGED;
> > + return IOMMU_DOMAIN_IDENTITY;
>
> def_domain_type can't be used for this purpose. But this seems to be a
> temporary code, as it will be removed in patch 09/25.
It looked OK when I checked it, mkt_v1 is really confusing what it
tries to do, but it should call probe_finalize and basically do the
same hacky thing as what UNMANAGED was trying to accomplish.
Did you see something else?
Jason
Re: [PATCH] powerpc: Make virt_to_pfn() a static inline
Le 14/08/2023 à 14:37, Michael Ellerman a écrit :
> Linus Walleij writes:
>> Making virt_to_pfn() a static inline taking a strongly typed
>> (const void *) makes the contract of a passing a pointer of that
>> type to the function explicit and exposes any misuse of the
>> macro virt_to_pfn() acting polymorphic and accepting many types
>> such as (void *), (unitptr_t) or (unsigned long) as arguments
>> without warnings.
> ...
>> diff --git a/arch/powerpc/include/asm/page.h
>> b/arch/powerpc/include/asm/page.h
>> index f2b6bf5687d0..9ee4b6d4a82a 100644
>> --- a/arch/powerpc/include/asm/page.h
>> +++ b/arch/powerpc/include/asm/page.h
>> @@ -9,6 +9,7 @@
>> #ifndef __ASSEMBLY__
>> #include
>> #include
>> +#include
>> #else
>> #include
>> #endif
>> @@ -119,16 +120,6 @@ extern long long virt_phys_offset;
>> #define ARCH_PFN_OFFSET((unsigned long)(MEMORY_START >>
>> PAGE_SHIFT))
>> #endif
>>
>> -#define virt_to_pfn(kaddr) (__pa(kaddr) >> PAGE_SHIFT)
>> -#define virt_to_page(kaddr) pfn_to_page(virt_to_pfn(kaddr))
>> -#define pfn_to_kaddr(pfn) __va((pfn) << PAGE_SHIFT)
>> -
>> -#define virt_addr_valid(vaddr) ({
>> \
>> -unsigned long _addr = (unsigned long)vaddr; \
>> -_addr >= PAGE_OFFSET && _addr < (unsigned long)high_memory && \
>> -pfn_valid(virt_to_pfn(_addr)); \
>> -})
>> -
>> /*
>>* On Book-E parts we need __va to parse the device tree and we can't
>>* determine MEMORY_START until then. However we can determine
>> PHYSICAL_START
>> @@ -233,6 +224,25 @@ extern long long virt_phys_offset;
>> #endif
>> #endif
>>
>> +#ifndef __ASSEMBLY__
>> +static inline unsigned long virt_to_pfn(const void *kaddr)
>> +{
>> +return __pa(kaddr) >> PAGE_SHIFT;
>> +}
>> +
>> +static inline const void *pfn_to_kaddr(unsigned long pfn)
>> +{
>> +return (const void *)(((unsigned long)__va(pfn)) << PAGE_SHIFT);
>
> Any reason to do it this way rather than:
>
> + return __va(pfn << PAGE_SHIFT);
Even cleaner:
return __va(PFN_PHYS(pfn));
>
> Seems to be equivalent and much cleaner?
>
> cheers
Re: [PATCH v6 02/25] iommu: Add IOMMU_DOMAIN_PLATFORM
On Sat, Aug 12, 2023 at 09:41:50AM +0800, Baolu Lu wrote:
> > index e05c93b6c37fba..87aebba474e093 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)
>
> Nit: As a default domain could be the type of IOMMU_DOMAIN_PLATFORM,
>
> static const char *iommu_domain_type_str(unsigned int t)
>
> needs to be updated, so that users can get a right string when reading
> /sys/.../[group_id]/type.
Yeah, I missed that, fix it
Thanks,
Jason
Re: [PATCH v6 02/25] iommu: Add IOMMU_DOMAIN_PLATFORM
On Sun, Aug 13, 2023 at 08:11:30PM +0800, Baolu Lu wrote:
> On 2023/8/12 19:28, Jason Gunthorpe wrote:
> > On Sat, Aug 12, 2023 at 09:36:33AM +0800, Baolu Lu wrote:
> > > > @@ -290,6 +295,7 @@ struct iommu_ops {
> > > > unsigned long pgsize_bitmap;
> > > > struct module *owner;
> > > > struct iommu_domain *identity_domain;
> > > > + struct iommu_domain *default_domain;
> > >
> > > I am imaging whether we can merge above two pointers into a single one.
> > > It is either an IDENTITY or PLATFORM domain and the core will choose it
> > > as the default domain of a group if iommu_group_alloc_default_domain()
> > > fails to allocate one through the iommu dev_ops.
> >
> > I think that would be the wrong direction..
> >
> > identity_domain is a pointer that is always, ALWAYS an identity
> > domain. It is the shortcut for drivers (and all drivers should do
> > this) that implement a global static identity domain.
>
> I see. I originally thought this was special for arm32.
No, everything should use it eventually. Eg you would convert the
Intel driver too. It makes the function always available in all error
unwind paths.
> > default_domain is a shortcut to avoid implementing the entire flow
> > around def_domain_type/domain_alloc for special cases. For this patch
> > the specialc ase is the IOMMU_DOMAIN_PLATFORM.
>
> I think this is special for drivers like s390. You don't want it to be
> used beyond those special drivers, right?
It ended up here:
arch/powerpc/kernel/iommu.c:.default_domain = _tce_platform_domain,
drivers/iommu/fsl_pamu_domain.c:.default_domain =
_pamu_platform_domain,
drivers/iommu/iommufd/selftest.c: .default_domain = _blocking_domain,
drivers/iommu/s390-iommu.c: .default_domain = _iommu_platform_domain,
drivers/iommu/tegra-smmu.c: .default_domain = _smmu_identity_domain,
And I have to try to remember why tegra-smmu had it.. So yes special
places only.
> If so, the naming of default_domain seems to be a bit generic. I can't
> think of a better one, hence I am fine if you keep as it-is. After all,
> the comment for this field has already explained it very clearly.
Me too
Thanks,
Jason
Re: [PATCH v6 10/25] iommu/exynos: Implement an IDENTITY domain
On Mon, Aug 14, 2023 at 12:52:34PM +0800, Baolu Lu wrote: > On 2023/8/3 8:07, 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(-) > > > > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c > > index c275fe71c4db32..5e12b85dfe8705 100644 > > --- a/drivers/iommu/exynos-iommu.c > > +++ b/drivers/iommu/exynos-iommu.c > > @@ -24,6 +24,7 @@ > > typedef u32 sysmmu_iova_t; > > typedef u32 sysmmu_pte_t; > > +static struct iommu_domain exynos_identity_domain; > > Is there a conflict between above and below? No, this is a C forward declaration. Jason
Re: [PATCH] powerpc: Make virt_to_pfn() a static inline
Linus Walleij writes:
> Making virt_to_pfn() a static inline taking a strongly typed
> (const void *) makes the contract of a passing a pointer of that
> type to the function explicit and exposes any misuse of the
> macro virt_to_pfn() acting polymorphic and accepting many types
> such as (void *), (unitptr_t) or (unsigned long) as arguments
> without warnings.
...
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index f2b6bf5687d0..9ee4b6d4a82a 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -9,6 +9,7 @@
> #ifndef __ASSEMBLY__
> #include
> #include
> +#include
> #else
> #include
> #endif
> @@ -119,16 +120,6 @@ extern long long virt_phys_offset;
> #define ARCH_PFN_OFFSET ((unsigned long)(MEMORY_START >>
> PAGE_SHIFT))
> #endif
>
> -#define virt_to_pfn(kaddr) (__pa(kaddr) >> PAGE_SHIFT)
> -#define virt_to_page(kaddr) pfn_to_page(virt_to_pfn(kaddr))
> -#define pfn_to_kaddr(pfn)__va((pfn) << PAGE_SHIFT)
> -
> -#define virt_addr_valid(vaddr) ({
> \
> - unsigned long _addr = (unsigned long)vaddr; \
> - _addr >= PAGE_OFFSET && _addr < (unsigned long)high_memory && \
> - pfn_valid(virt_to_pfn(_addr)); \
> -})
> -
> /*
> * On Book-E parts we need __va to parse the device tree and we can't
> * determine MEMORY_START until then. However we can determine
> PHYSICAL_START
> @@ -233,6 +224,25 @@ extern long long virt_phys_offset;
> #endif
> #endif
>
> +#ifndef __ASSEMBLY__
> +static inline unsigned long virt_to_pfn(const void *kaddr)
> +{
> + return __pa(kaddr) >> PAGE_SHIFT;
> +}
> +
> +static inline const void *pfn_to_kaddr(unsigned long pfn)
> +{
> + return (const void *)(((unsigned long)__va(pfn)) << PAGE_SHIFT);
Any reason to do it this way rather than:
+ return __va(pfn << PAGE_SHIFT);
Seems to be equivalent and much cleaner?
cheers
Re: [PATCH RFC 0/3] powerpc/ps3: refactor strncpy usage
Justin Stitt writes: > On Fri, Aug 11, 2023 at 2:19 PM Justin Stitt wrote: >> >> Within this RFC-series I want to get some comments on two ideas that I >> have for refactoring the current `strncpy` usage in repository.c. >> >> When looking at `make_first_field` we see a u64 is being used to store >> up to 8 bytes from a literal string. This is slightly suspect to me but >> it works? In regards to `strncpy` here, it makes the code needlessly >> complex imo. >> >> Please see my two ideas to change this and let me know if any other >> approaches are more reasonable. >> >> Link: https://github.com/KSPP/linux/issues/90 >> Signed-off-by: Justin Stitt >> --- >> Justin Stitt (3): >> [RFC] powerpc/ps3: refactor strncpy usage attempt 1 >> [RFC] powerpc/ps3: refactor strncpy usage attempt 2 >> [RFC] powerpc/ps3: refactor strncpy usage attempt 2.5 > Errhm, It looks like the diffs after attempt 1 came out poorly and > probably won't apply cleanly because they were inter-diffed with the > first patch. Is there a way to let b4 know I wanted each patch diff'd > against the same SHA and not each other sequentially? I don't think there is. It always assumes they're a series. cheers
Re: [PATCH RFC 1/3] powerpc/ps3: refactor strncpy usage attempt 1
Justin Stitt writes:
> This approach simply replicates the implementation of `make_field` which
> means we drop `strncpy` for `memcpy`.
>
> Signed-off-by: Justin Stitt
> ---
> arch/powerpc/platforms/ps3/repository.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/ps3/repository.c
> b/arch/powerpc/platforms/ps3/repository.c
> index 205763061a2d..1abe33fbe529 100644
> --- a/arch/powerpc/platforms/ps3/repository.c
> +++ b/arch/powerpc/platforms/ps3/repository.c
> @@ -73,9 +73,9 @@ static void _dump_node(unsigned int lpar_id, u64 n1, u64
> n2, u64 n3, u64 n4,
>
> static u64 make_first_field(const char *text, u64 index)
> {
> - u64 n;
> + u64 n = 0;
>
> - strncpy((char *), text, 8);
> + memcpy((char *), text, strnlen(text, sizeof(n)));
> return PS3_VENDOR_ID_NONE + (n >> 32) + index;
> }
I guess it's a slight improvement, because people generally know
memcpy's behaviour better than strncpy.
The change log should be a bit more verbose and mention that the result
is the same, including the NULL padding done my strncpy().
cheers
[PATCH 7/7] perf vendor events: Update metric events for power10 platform
Update JSON/events for power10 platform with additional metrics.
Signed-off-by: Kajol Jain
---
.../arch/powerpc/power10/metrics.json | 33 +++
1 file changed, 33 insertions(+)
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
b/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
index 182369076d95..4d66b75c6ad5 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
@@ -24,6 +24,12 @@
"MetricGroup": "CPI",
"MetricName": "DISPATCH_STALL_FLUSH_CPI"
},
+{
+"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled because Fetch was being held, so there was nothing in the
pipeline for this thread",
+"MetricExpr": "PM_DISP_STALL_FETCH / PM_RUN_INST_CMPL",
+"MetricGroup": "CPI",
+"MetricName": "DISPATCH_STALL_FETCH_CPI"
+},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled because the MMU was handling a translation miss",
"MetricExpr": "PM_DISP_STALL_TRANSLATION / PM_RUN_INST_CMPL",
@@ -394,6 +400,13 @@
"MetricName": "L1_LD_MISS_RATE",
"ScaleUnit": "1%"
},
+{
+"BriefDescription": "Percentage of completed instructions that were
stores that missed the L1",
+"MetricExpr": "PM_ST_MISS_L1 * 100 / PM_RUN_INST_CMPL",
+"MetricGroup": "Others",
+"MetricName": "L1_ST_MISS_RATE",
+"ScaleUnit": "1%"
+},
{
"BriefDescription": "Percentage of completed instructions when the
DPTEG required for the load/store instruction in execution was missing from the
TLB",
"MetricExpr": "PM_DTLB_MISS / PM_RUN_INST_CMPL * 100",
@@ -467,6 +480,13 @@
"MetricName": "DL1_RELOAD_FROM_L3_MISS_RATE",
"ScaleUnit": "1%"
},
+{
+"BriefDescription": "Percentage of ITLB misses per completed run
instruction",
+"MetricExpr": "PM_ITLB_MISS / PM_RUN_INST_CMPL * 100",
+"MetricGroup": "General",
+"MetricName": "ITLB_MISS_RATE",
+"ScaleUnit": "1%"
+},
{
"BriefDescription": "Percentage of DERAT misses with 4k page size per
completed instruction",
"MetricExpr": "PM_DERAT_MISS_4K / PM_RUN_INST_CMPL * 100",
@@ -622,6 +642,13 @@
"MetricName": "DERAT_16M_MISS_RATE",
"ScaleUnit": "1%"
},
+{
+"BriefDescription": "Percentage of DERAT misses with 1G page size per
completed run instruction",
+"MetricExpr": "PM_DERAT_MISS_1G * 100 / PM_RUN_INST_CMPL",
+"MetricGroup": "Translation",
+"MetricName": "DERAT_1G_MISS_RATE",
+"ScaleUnit": "1%"
+},
{
"BriefDescription": "DERAT miss ratio for 4K page size",
"MetricExpr": "PM_DERAT_MISS_4K / PM_DERAT_MISS",
@@ -640,6 +667,12 @@
"MetricGroup": "Translation",
"MetricName": "DERAT_16M_MISS_RATIO"
},
+{
+"BriefDescription": "DERAT miss ratio for 1G page size",
+"MetricExpr": "PM_DERAT_MISS_1G / PM_DERAT_MISS",
+"MetricGroup": "Translation",
+"MetricName": "DERAT_1G_MISS_RATIO"
+},
{
"BriefDescription": "DERAT miss ratio for 64K page size",
"MetricExpr": "PM_DERAT_MISS_64K / PM_DERAT_MISS",
--
2.39.3
[PATCH 6/7] perf vendor events: Update metric event names for power10 platform
Update metric event name for some of the JSON/metric events for
power10 platform.
Fixes: 3ca3af7d1f23 ("perf vendor events power10: Add metric events JSON file
for power10 platform")
Signed-off-by: Kajol Jain
---
.../arch/powerpc/power10/metrics.json | 50 +--
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
b/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
index e3087eb1ccff..182369076d95 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
@@ -16,133 +16,133 @@
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled for any reason",
"MetricExpr": "PM_DISP_STALL_CYC / PM_RUN_INST_CMPL",
"MetricGroup": "CPI;CPI_STALL_RATIO",
-"MetricName": "DISPATCHED_CPI"
+"MetricName": "DISPATCH_STALL_CPI"
},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled because there was a flush",
"MetricExpr": "PM_DISP_STALL_FLUSH / PM_RUN_INST_CMPL",
"MetricGroup": "CPI",
-"MetricName": "DISPATCHED_FLUSH_CPI"
+"MetricName": "DISPATCH_STALL_FLUSH_CPI"
},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled because the MMU was handling a translation miss",
"MetricExpr": "PM_DISP_STALL_TRANSLATION / PM_RUN_INST_CMPL",
"MetricGroup": "CPI",
-"MetricName": "DISPATCHED_TRANSLATION_CPI"
+"MetricName": "DISPATCH_STALL_TRANSLATION_CPI"
},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled waiting to resolve an instruction ERAT miss",
"MetricExpr": "PM_DISP_STALL_IERAT_ONLY_MISS / PM_RUN_INST_CMPL",
"MetricGroup": "CPI",
-"MetricName": "DISPATCHED_IERAT_ONLY_MISS_CPI"
+"MetricName": "DISPATCH_STALL_IERAT_ONLY_MISS_CPI"
},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled waiting to resolve an instruction TLB miss",
"MetricExpr": "PM_DISP_STALL_ITLB_MISS / PM_RUN_INST_CMPL",
"MetricGroup": "CPI",
-"MetricName": "DISPATCHED_ITLB_MISS_CPI"
+"MetricName": "DISPATCH_STALL_ITLB_MISS_CPI"
},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled due to an icache miss",
"MetricExpr": "PM_DISP_STALL_IC_MISS / PM_RUN_INST_CMPL",
"MetricGroup": "CPI",
-"MetricName": "DISPATCHED_IC_MISS_CPI"
+"MetricName": "DISPATCH_STALL_IC_MISS_CPI"
},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled while the instruction was fetched from the local L2",
"MetricExpr": "PM_DISP_STALL_IC_L2 / PM_RUN_INST_CMPL",
"MetricGroup": "CPI",
-"MetricName": "DISPATCHED_IC_L2_CPI"
+"MetricName": "DISPATCH_STALL_IC_L2_CPI"
},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled while the instruction was fetched from the local L3",
"MetricExpr": "PM_DISP_STALL_IC_L3 / PM_RUN_INST_CMPL",
"MetricGroup": "CPI",
-"MetricName": "DISPATCHED_IC_L3_CPI"
+"MetricName": "DISPATCH_STALL_IC_L3_CPI"
},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled while the instruction was fetched from any source beyond
the local L3",
"MetricExpr": "PM_DISP_STALL_IC_L3MISS / PM_RUN_INST_CMPL",
"MetricGroup": "CPI",
-"MetricName": "DISPATCHED_IC_L3MISS_CPI"
+"MetricName": "DISPATCH_STALL_IC_L3MISS_CPI"
},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled due to an icache miss after a branch mispredict",
"MetricExpr": "PM_DISP_STALL_BR_MPRED_ICMISS / PM_RUN_INST_CMPL",
"MetricGroup": "CPI",
-"MetricName": "DISPATCHED_BR_MPRED_ICMISS_CPI"
+"MetricName": "DISPATCH_STALL_BR_MPRED_ICMISS_CPI"
},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled while instruction was fetched from the local L2 after
suffering a branch mispredict",
"MetricExpr": "PM_DISP_STALL_BR_MPRED_IC_L2 / PM_RUN_INST_CMPL",
"MetricGroup": "CPI",
-"MetricName": "DISPATCHED_BR_MPRED_IC_L2_CPI"
+"MetricName": "DISPATCH_STALL_BR_MPRED_IC_L2_CPI"
},
{
"BriefDescription": "Average cycles per completed instruction when
dispatch was stalled while instruction was fetched from the local L3 after
suffering a branch mispredict",
"MetricExpr": "PM_DISP_STALL_BR_MPRED_IC_L3 / PM_RUN_INST_CMPL",
"MetricGroup": "CPI",
-"MetricName":
[PATCH 4/7] perf vendor events: Move JSON/events to appropriate files for power10 platform
Move some of the power10 JSON/events to appropriate files.
Fixes: 32daa5d7899e ("perf vendor events: Initial JSON/events list for power10
platform")
Signed-off-by: Kajol Jain
---
.../arch/powerpc/power10/cache.json | 45
.../arch/powerpc/power10/floating_point.json | 67 +
.../arch/powerpc/power10/frontend.json| 180 -
.../arch/powerpc/power10/marked.json | 186 ++---
.../arch/powerpc/power10/memory.json | 85 --
.../arch/powerpc/power10/others.json | 192 ++
.../arch/powerpc/power10/pipeline.json| 247 ++
.../pmu-events/arch/powerpc/power10/pmc.json | 193 +-
.../arch/powerpc/power10/translation.json | 35 ---
9 files changed, 616 insertions(+), 614 deletions(-)
create mode 100644
tools/perf/pmu-events/arch/powerpc/power10/floating_point.json
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/cache.json
b/tools/perf/pmu-events/arch/powerpc/power10/cache.json
index 9cb929bb64af..839ae26945fb 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/cache.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/cache.json
@@ -1,54 +1,9 @@
[
- {
-"EventCode": "0x1003C",
-"EventName": "PM_EXEC_STALL_DMISS_L2L3",
-"BriefDescription": "Cycles in which the oldest instruction in the
pipeline was waiting for a load miss to resolve from either the local L2 or
local L3."
- },
- {
-"EventCode": "0x1E054",
-"EventName": "PM_EXEC_STALL_DMISS_L21_L31",
-"BriefDescription": "Cycles in which the oldest instruction in the
pipeline was waiting for a load miss to resolve from another core's L2 or L3 on
the same chip."
- },
- {
-"EventCode": "0x34054",
-"EventName": "PM_EXEC_STALL_DMISS_L2L3_NOCONFLICT",
-"BriefDescription": "Cycles in which the oldest instruction in the
pipeline was waiting for a load miss to resolve from the local L2 or local L3,
without a dispatch conflict."
- },
- {
-"EventCode": "0x34056",
-"EventName": "PM_EXEC_STALL_LOAD_FINISH",
-"BriefDescription": "Cycles in which the oldest instruction in the
pipeline was finishing a load after its data was reloaded from a data source
beyond the local L1; cycles in which the LSU was processing an L1-hit; cycles
in which the next-to-finish (NTF) instruction merged with another load in the
LMQ; cycles in which the NTF instruction is waiting for a data reload for a
load miss, but the data comes back with a non-NTF instruction."
- },
- {
-"EventCode": "0x3006C",
-"EventName": "PM_RUN_CYC_SMT2_MODE",
-"BriefDescription": "Cycles when this thread's run latch is set and the
core is in SMT2 mode."
- },
{
"EventCode": "0x300F4",
"EventName": "PM_RUN_INST_CMPL_CONC",
"BriefDescription": "PowerPC instruction completed by this thread when all
threads in the core had the run-latch set."
},
- {
-"EventCode": "0x4C016",
-"EventName": "PM_EXEC_STALL_DMISS_L2L3_CONFLICT",
-"BriefDescription": "Cycles in which the oldest instruction in the
pipeline was waiting for a load miss to resolve from the local L2 or local L3,
with a dispatch conflict."
- },
- {
-"EventCode": "0x4D014",
-"EventName": "PM_EXEC_STALL_LOAD",
-"BriefDescription": "Cycles in which the oldest instruction in the
pipeline was a load instruction executing in the Load Store Unit."
- },
- {
-"EventCode": "0x4D016",
-"EventName": "PM_EXEC_STALL_PTESYNC",
-"BriefDescription": "Cycles in which the oldest instruction in the
pipeline was a PTESYNC instruction executing in the Load Store Unit."
- },
- {
-"EventCode": "0x401EA",
-"EventName": "PM_THRESH_EXC_128",
-"BriefDescription": "Threshold counter exceeded a value of 128."
- },
{
"EventCode": "0x400F6",
"EventName": "PM_BR_MPRED_CMPL",
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/floating_point.json
b/tools/perf/pmu-events/arch/powerpc/power10/floating_point.json
new file mode 100644
index ..e816cd10c129
--- /dev/null
+++ b/tools/perf/pmu-events/arch/powerpc/power10/floating_point.json
@@ -0,0 +1,67 @@
+[
+ {
+"EventCode": "0x100F4",
+"EventName": "PM_FLOP_CMPL",
+"BriefDescription": "Floating Point Operations Completed. Includes any
type. It counts once for each 1, 2, 4 or 8 flop instruction. Use
PM_1|2|4|8_FLOP_CMPL events to count flops."
+ },
+ {
+"EventCode": "0x45050",
+"EventName": "PM_1FLOP_CMPL",
+"BriefDescription": "One floating point instruction completed (fadd, fmul,
fsub, fcmp, fsel, fabs, fnabs, fres, fsqrte, fneg)."
+ },
+ {
+"EventCode": "0x45052",
+"EventName": "PM_4FLOP_CMPL",
+"BriefDescription": "Four floating point instruction completed (fadd,
fmul, fsub, fcmp, fsel, fabs, fnabs, fres, fsqrte, fneg)."
+ },
+ {
+"EventCode": "0x45054",
+"EventName": "PM_FMA_CMPL",
+"BriefDescription": "Two floating point instruction completed (FMA class
of
[PATCH 5/7] perf vendor events: Update JSON/events for power10 platform
Update JSON/events for power10 platform with additional events.
Signed-off-by: Kajol Jain
---
.../arch/powerpc/power10/frontend.json| 25 +
.../arch/powerpc/power10/marked.json | 30
.../arch/powerpc/power10/memory.json | 10 ++
.../arch/powerpc/power10/others.json | 5 +++
.../arch/powerpc/power10/pipeline.json| 35 +++
.../pmu-events/arch/powerpc/power10/pmc.json | 5 +++
.../arch/powerpc/power10/translation.json | 5 +++
7 files changed, 115 insertions(+)
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/frontend.json
b/tools/perf/pmu-events/arch/powerpc/power10/frontend.json
index dc0bb6c6338b..5977f5e64212 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/frontend.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/frontend.json
@@ -1,4 +1,14 @@
[
+ {
+"EventCode": "0x1D054",
+"EventName": "PM_DTLB_HIT_2M",
+"BriefDescription": "Data TLB hit (DERAT reload) page size 2M. Implies
radix translation. When MMCR1[16]=0 this event counts only DERAT reloads for
demand misses. When MMCR1[16]=1 this event includes demand misses and
prefetches."
+ },
+ {
+"EventCode": "0x1D058",
+"EventName": "PM_ITLB_HIT_64K",
+"BriefDescription": "Instruction TLB hit (IERAT reload) page size 64K.
When MMCR1[17]=0 this event counts only for demand misses. When MMCR1[17]=1
this event includes demand misses and prefetches."
+ },
{
"EventCode": "0x1F054",
"EventName": "PM_DTLB_HIT",
@@ -44,6 +54,11 @@
"EventName": "PM_ITLB_HIT_1G",
"BriefDescription": "Instruction TLB hit (IERAT reload) page size 1G,
which implies Radix Page Table translation is in use. When MMCR1[17]=0 this
event counts only for demand misses. When MMCR1[17]=1 this event includes
demand misses and prefetches."
},
+ {
+"EventCode": "0x3C05A",
+"EventName": "PM_DTLB_HIT_64K",
+"BriefDescription": "Data TLB hit (DERAT reload) page size 64K. When
MMCR1[16]=0 this event counts only for demand misses. When MMCR1[16]=1 this
event includes demand misses and prefetches."
+ },
{
"EventCode": "0x3E054",
"EventName": "PM_LD_MISS_L1",
@@ -63,5 +78,15 @@
"EventCode": "0x44056",
"EventName": "PM_VECTOR_ST_CMPL",
"BriefDescription": "Vector store instruction completed."
+ },
+ {
+"EventCode": "0x4E054",
+"EventName": "PM_DTLB_HIT_1G",
+"BriefDescription": "Data TLB hit (DERAT reload) page size 1G. Implies
radix translation. When MMCR1[16]=0 this event counts only for demand misses.
When MMCR1[16]=1 this event includes demand misses and prefetches."
+ },
+ {
+"EventCode": "0x400FC",
+"EventName": "PM_ITLB_MISS",
+"BriefDescription": "Instruction TLB reload (after a miss), all page
sizes. Includes only demand misses."
}
]
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/marked.json
b/tools/perf/pmu-events/arch/powerpc/power10/marked.json
index 913b6515b870..78f71a9eadfd 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/marked.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/marked.json
@@ -19,6 +19,11 @@
"EventName": "PM_MRK_XFER_FROM_SRC_CYC_PMC1",
"BriefDescription": "Cycles taken for a marked demand miss to reload a
line from the source specified in MMCR3[0:12]."
},
+ {
+"EventCode": "0x1D15C",
+"EventName": "PM_MRK_DTLB_MISS_1G",
+"BriefDescription": "Marked Data TLB reload (after a miss) page size 1G.
Implies radix translation was used. When MMCR1[16]=0 this event counts only for
demand misses. When MMCR1[16]=1 this event includes demand misses and
prefetches."
+ },
{
"EventCode": "0x1F150",
"EventName": "PM_MRK_ST_L2_CYC",
@@ -134,6 +139,11 @@
"EventName": "PM_MRK_L2_RC_DONE",
"BriefDescription": "L2 RC machine completed the transaction for the
marked instruction."
},
+ {
+"EventCode": "0x3012E",
+"EventName": "PM_MRK_DTLB_MISS_2M",
+"BriefDescription": "Marked Data TLB reload (after a miss) page size 2M,
which implies Radix Page Table translation was used. When MMCR1[16]=0 this
event counts only for demand misses. When MMCR1[16]=1 this event includes
demand misses and prefetches."
+ },
{
"EventCode": "0x30132",
"EventName": "PM_MRK_VSU_FIN",
@@ -184,6 +194,16 @@
"EventName": "PM_MRK_BR_MPRED_CMPL",
"BriefDescription": "Marked Branch Mispredicted. Includes direction and
target."
},
+ {
+"EventCode": "0x301E6",
+"EventName": "PM_MRK_DERAT_MISS",
+"BriefDescription": "Marked Erat Miss (Data TLB Access) All page sizes.
When MMCR1[16]=0 this event counts only DERAT reloads for demand misses. When
MMCR1[16]=1 this event includes demand misses and prefetches."
+ },
+ {
+"EventCode": "0x4010E",
+"EventName": "PM_MRK_TLBIE_FIN",
+"BriefDescription": "Marked TLBIE instruction finished. Includes TLBIE and
TLBIEL instructions."
+ },
{
"EventCode": "0x40116",
[PATCH 3/7] perf vendor events: Drop STORES_PER_INST metric event for power10 platform
Drop STORES_PER_INST metric event for the power10 platform, as the
metric expression of STORES_PER_INST metric event using dropped event
PM_ST_FIN.
Fixes: 3ca3af7d1f23 ("perf vendor events power10: Add metric events JSON file
for power10 platform")
Signed-off-by: Kajol Jain
---
tools/perf/pmu-events/arch/powerpc/power10/metrics.json | 6 --
1 file changed, 6 deletions(-)
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
b/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
index 6f53583a0c62..e3087eb1ccff 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/metrics.json
@@ -453,12 +453,6 @@
"MetricGroup": "General",
"MetricName": "LOADS_PER_INST"
},
-{
-"BriefDescription": "Average number of finished stores per completed
instruction",
-"MetricExpr": "PM_ST_FIN / PM_RUN_INST_CMPL",
-"MetricGroup": "General",
-"MetricName": "STORES_PER_INST"
-},
{
"BriefDescription": "Percentage of demand loads that reloaded from
beyond the L2 per completed instruction",
"MetricExpr": "PM_DATA_FROM_L2MISS / PM_RUN_INST_CMPL * 100",
--
2.39.3
[PATCH 2/7] perf vendor events: Drop some of the JSON/events for power10 platform
Drop some of the JSON/events for power10 platform due to counter
data mismatch.
Fixes: 32daa5d7899e ("perf vendor events: Initial JSON/events list for power10
platform")
Signed-off-by: Kajol Jain
---
.../arch/powerpc/power10/floating_point.json | 7 ---
tools/perf/pmu-events/arch/powerpc/power10/marked.json | 10 --
tools/perf/pmu-events/arch/powerpc/power10/others.json | 5 -
.../perf/pmu-events/arch/powerpc/power10/pipeline.json | 10 --
.../pmu-events/arch/powerpc/power10/translation.json | 5 -
5 files changed, 37 deletions(-)
delete mode 100644
tools/perf/pmu-events/arch/powerpc/power10/floating_point.json
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/floating_point.json
b/tools/perf/pmu-events/arch/powerpc/power10/floating_point.json
deleted file mode 100644
index 54acb55e2c8c..
--- a/tools/perf/pmu-events/arch/powerpc/power10/floating_point.json
+++ /dev/null
@@ -1,7 +0,0 @@
-[
- {
-"EventCode": "0x4016E",
-"EventName": "PM_THRESH_NOT_MET",
-"BriefDescription": "Threshold counter did not meet threshold."
- }
-]
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/marked.json
b/tools/perf/pmu-events/arch/powerpc/power10/marked.json
index 131f8d0e8831..f2436fc5537c 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/marked.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/marked.json
@@ -19,11 +19,6 @@
"EventName": "PM_MRK_BR_TAKEN_CMPL",
"BriefDescription": "Marked Branch Taken instruction completed."
},
- {
-"EventCode": "0x20112",
-"EventName": "PM_MRK_NTF_FIN",
-"BriefDescription": "The marked instruction became the oldest in the
pipeline before it finished. It excludes instructions that finish at dispatch."
- },
{
"EventCode": "0x2C01C",
"EventName": "PM_EXEC_STALL_DMISS_OFF_CHIP",
@@ -64,11 +59,6 @@
"EventName": "PM_L1_ICACHE_MISS",
"BriefDescription": "Demand instruction cache miss."
},
- {
-"EventCode": "0x30130",
-"EventName": "PM_MRK_INST_FIN",
-"BriefDescription": "marked instruction finished. Excludes instructions
that finish at dispatch. Note that stores always finish twice since the address
gets issued to the LSU and the data gets issued to the VSU."
- },
{
"EventCode": "0x34146",
"EventName": "PM_MRK_LD_CMPL",
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/others.json
b/tools/perf/pmu-events/arch/powerpc/power10/others.json
index a5319cdba89b..17c5424ef1ac 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/others.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/others.json
@@ -29,11 +29,6 @@
"EventName": "PM_DISP_SS0_2_INSTR_CYC",
"BriefDescription": "Cycles in which Superslice 0 dispatches either 1 or 2
instructions."
},
- {
-"EventCode": "0x1F15C",
-"EventName": "PM_MRK_STCX_L2_CYC",
-"BriefDescription": "Cycles spent in the nest portion of a marked Stcx
instruction. It starts counting when the operation starts to drain to the L2
and it stops counting when the instruction retires from the Instruction
Completion Table (ICT) in the Instruction Sequencing Unit (ISU)."
- },
{
"EventCode": "0x10066",
"EventName": "PM_ADJUNCT_CYC",
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/pipeline.json
b/tools/perf/pmu-events/arch/powerpc/power10/pipeline.json
index 449f57e8ba6a..799893c56f32 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/pipeline.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/pipeline.json
@@ -194,11 +194,6 @@
"EventName": "PM_TLBIE_FIN",
"BriefDescription": "TLBIE instruction finished in the LSU. Two TLBIEs can
finish each cycle. All will be counted."
},
- {
-"EventCode": "0x3D058",
-"EventName": "PM_SCALAR_FSQRT_FDIV_ISSUE",
-"BriefDescription": "Scalar versions of four floating point operations:
fdiv,fsqrt (xvdivdp, xvdivsp, xvsqrtdp, xvsqrtsp)."
- },
{
"EventCode": "0x30066",
"EventName": "PM_LSU_FIN",
@@ -269,11 +264,6 @@
"EventName": "PM_IC_MISS_CMPL",
"BriefDescription": "Non-speculative instruction cache miss, counted at
completion."
},
- {
-"EventCode": "0x4D050",
-"EventName": "PM_VSU_NON_FLOP_CMPL",
-"BriefDescription": "Non-floating point VSU instructions completed."
- },
{
"EventCode": "0x4D052",
"EventName": "PM_2FLOP_CMPL",
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/translation.json
b/tools/perf/pmu-events/arch/powerpc/power10/translation.json
index 3e47b804a0a8..961e2491e73f 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/translation.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/translation.json
@@ -4,11 +4,6 @@
"EventName": "PM_MRK_START_PROBE_NOP_CMPL",
"BriefDescription": "Marked Start probe nop (AND R0,R0,R0) completed."
},
- {
-"EventCode": "0x20016",
-"EventName": "PM_ST_FIN",
-"BriefDescription": "Store finish count. Includes speculative activity."
- },
[PATCH 1/7] perf vendor events: Update the JSON/events descriptions for power10 platform
Update the description for some of the JSON/events for power10 platform.
Fixes: 32daa5d7899e ("perf vendor events: Initial JSON/events list for power10
platform")
Signed-off-by: Kajol Jain
---
.../arch/powerpc/power10/cache.json | 4 +-
.../arch/powerpc/power10/frontend.json| 30 ++--
.../arch/powerpc/power10/marked.json | 20
.../arch/powerpc/power10/memory.json | 6 +--
.../arch/powerpc/power10/others.json | 48 +--
.../arch/powerpc/power10/pipeline.json| 20
.../pmu-events/arch/powerpc/power10/pmc.json | 4 +-
.../arch/powerpc/power10/translation.json | 6 +--
8 files changed, 69 insertions(+), 69 deletions(-)
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/cache.json
b/tools/perf/pmu-events/arch/powerpc/power10/cache.json
index 605be14f441c..9cb929bb64af 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/cache.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/cache.json
@@ -17,7 +17,7 @@
{
"EventCode": "0x34056",
"EventName": "PM_EXEC_STALL_LOAD_FINISH",
-"BriefDescription": "Cycles in which the oldest instruction in the
pipeline was finishing a load after its data was reloaded from a data source
beyond the local L1; cycles in which the LSU was processing an L1-hit; cycles
in which the NTF instruction merged with another load in the LMQ; cycles in
which the NTF instruction is waiting for a data reload for a load miss, but the
data comes back with a non-NTF instruction."
+"BriefDescription": "Cycles in which the oldest instruction in the
pipeline was finishing a load after its data was reloaded from a data source
beyond the local L1; cycles in which the LSU was processing an L1-hit; cycles
in which the next-to-finish (NTF) instruction merged with another load in the
LMQ; cycles in which the NTF instruction is waiting for a data reload for a
load miss, but the data comes back with a non-NTF instruction."
},
{
"EventCode": "0x3006C",
@@ -27,7 +27,7 @@
{
"EventCode": "0x300F4",
"EventName": "PM_RUN_INST_CMPL_CONC",
-"BriefDescription": "PowerPC instructions completed by this thread when
all threads in the core had the run-latch set."
+"BriefDescription": "PowerPC instruction completed by this thread when all
threads in the core had the run-latch set."
},
{
"EventCode": "0x4C016",
diff --git a/tools/perf/pmu-events/arch/powerpc/power10/frontend.json
b/tools/perf/pmu-events/arch/powerpc/power10/frontend.json
index 558f9530f54e..61e9e0222c87 100644
--- a/tools/perf/pmu-events/arch/powerpc/power10/frontend.json
+++ b/tools/perf/pmu-events/arch/powerpc/power10/frontend.json
@@ -7,7 +7,7 @@
{
"EventCode": "0x10006",
"EventName": "PM_DISP_STALL_HELD_OTHER_CYC",
-"BriefDescription": "Cycles in which the NTC instruction is held at
dispatch for any other reason."
+"BriefDescription": "Cycles in which the next-to-complete (NTC)
instruction is held at dispatch for any other reason."
},
{
"EventCode": "0x10010",
@@ -32,12 +32,12 @@
{
"EventCode": "0x1D05E",
"EventName": "PM_DISP_STALL_HELD_HALT_CYC",
-"BriefDescription": "Cycles in which the NTC instruction is held at
dispatch because of power management."
+"BriefDescription": "Cycles in which the next-to-complete (NTC)
instruction is held at dispatch because of power management."
},
{
"EventCode": "0x1E050",
"EventName": "PM_DISP_STALL_HELD_STF_MAPPER_CYC",
-"BriefDescription": "Cycles in which the NTC instruction is held at
dispatch because the STF mapper/SRB was full. Includes GPR (count, link, tar),
VSR, VMR, FPR."
+"BriefDescription": "Cycles in which the next-to-complete (NTC)
instruction is held at dispatch because the STF mapper/SRB was full. Includes
GPR (count, link, tar), VSR, VMR, FPR."
},
{
"EventCode": "0x1F054",
@@ -67,7 +67,7 @@
{
"EventCode": "0x100F6",
"EventName": "PM_IERAT_MISS",
-"BriefDescription": "IERAT Reloaded to satisfy an IERAT miss. All page
sizes are counted by this event."
+"BriefDescription": "IERAT Reloaded to satisfy an IERAT miss. All page
sizes are counted by this event. This event only counts instruction demand
access."
},
{
"EventCode": "0x100F8",
@@ -77,7 +77,7 @@
{
"EventCode": "0x20006",
"EventName": "PM_DISP_STALL_HELD_ISSQ_FULL_CYC",
-"BriefDescription": "Cycles in which the NTC instruction is held at
dispatch due to Issue queue full. Includes issue queue and branch queue."
+"BriefDescription": "Cycles in which the next-to-complete (NTC)
instruction is held at dispatch due to Issue queue full. Includes issue queue
and branch queue."
},
{
"EventCode": "0x20114",
@@ -102,7 +102,7 @@
{
"EventCode": "0x2D01A",
"EventName": "PM_DISP_STALL_IC_MISS",
-"BriefDescription": "Cycles when dispatch was stalled for this thread due
to an Icache Miss."
+
Re: [PATCH v4 10/10] powerpc/pseries: Honour current SMT state when DLPAR onlining CPUs
* Laurent Dufour [2023-07-05 16:51:43]: > From: Michael Ellerman > > Integrate with the generic SMT support, so that when a CPU is DLPAR > onlined it is brought up with the correct SMT mode. > Looks good to me. Reviewed-by: Srikar Dronamraju > Signed-off-by: Michael Ellerman > --- > arch/powerpc/platforms/pseries/hotplug-cpu.c | 8 > 1 file changed, 8 insertions(+) > -- Thanks and Regards Srikar Dronamraju
Re: [PATCH v4 09/10] powerpc: Add HOTPLUG_SMT support
* Laurent Dufour [2023-07-05 16:51:42]: > From: Michael Ellerman > > Add support for HOTPLUG_SMT, which enables the generic sysfs SMT support > files in /sys/devices/system/cpu/smt, as well as the "nosmt" boot > parameter. > > Implement the recently added hooks to allow partial SMT states, allow > any number of threads per core. > > Tie the config symbol to HOTPLUG_CPU, which enables it on the major > platforms that support SMT. If there are other platforms that want the > SMT support that can be tweaked in future. > > Signed-off-by: Michael Ellerman > [ldufour: pass current SMT level to cpu_smt_set_num_threads] > [ldufour: remove topology_smt_supported] > [ldufour: remove topology_smt_threads_supported] > [ldufour: select CONFIG_SMT_NUM_THREADS_DYNAMIC] > [ldufour: update kernel-parameters.txt] > Signed-off-by: Laurent Dufour Looks good to me. Reviewed-by: Srikar Dronamraju -- Thanks and Regards Srikar Dronamraju
Re: [PATCH v4 08/10] powerpc/pseries: Initialise CPU hotplug callbacks earlier
* Laurent Dufour [2023-07-05 16:51:41]:
> From: Michael Ellerman
>
> As part of the generic HOTPLUG_SMT code, there is support for disabling
> secondary SMT threads at boot time, by passing "nosmt" on the kernel
> command line.
>
> The way that is implemented is the secondary threads are brought partly
> online, and then taken back offline again. That is done to support x86
> CPUs needing certain initialisation done on all threads. However powerpc
> has similar needs, see commit d70a54e2d085 ("powerpc/powernv: Ignore
> smt-enabled on Power8 and later").
>
> For that to work the powerpc CPU hotplug callbacks need to be registered
> before secondary CPUs are brought online, otherwise __cpu_disable()
> fails due to smp_ops->cpu_disable being NULL.
>
> So split the basic initialisation into pseries_cpu_hotplug_init() which
> can be called early from setup_arch(). The DLPAR related initialisation
> can still be done later, because it needs to do allocations.
>
Looks good to me.
Reviewed-by: Srikar Dronamraju
> Signed-off-by: Michael Ellerman
--
Thanks and Regards
Srikar Dronamraju
Re: [kvm-unit-tests PATCH 2/2] Link with "-z noexecstack" to avoid warning from newer versions of ld
On 27/06/2023 00.34, Sean Christopherson wrote: On Fri, Jun 23, 2023, Thomas Huth wrote: On 23/06/2023 16.24, Sean Christopherson wrote: On Fri, Jun 23, 2023, Thomas Huth wrote: Newer versions of ld (from binutils 2.40) complain on s390x and x86: ld: warning: s390x/cpu.o: missing .note.GNU-stack section implies executable stack ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker We can silence these warnings by using "-z noexecstack" for linking (which should not have any real influence on the kvm-unit-tests since the information from the ELF header is not used here anyway, so it's just cosmetics). Signed-off-by: Thomas Huth --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 0e5d85a1..20f7137c 100644 --- a/Makefile +++ b/Makefile @@ -96,7 +96,7 @@ CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d -LDFLAGS += -nostdlib +LDFLAGS += -nostdlib -z noexecstack Drat, the pull request[1] I sent to Paolo yesterday only fixes x86[2]. ... Paolo, want me to redo the pull request to drop the x86-specific patch? I can also respin my patch on top of your series later ... the problem currently also only seems to happen on x86 and s390x, on ppc64 and aarch64, the linker does not complain ... so maybe it's even better to do it per-architecture only anyway? Opinions? I don't think it makes sense to do this per-arch, other architectures likely aren't problematic purely because of linker specific behavior, e.g. see https://patches.linaro.org/project/binutils/patch/1506025575-1559-1-git-send-email-jim.wil...@linaro.org Ok, I've pushed now my patches since other people were running into this issue, too (see https://lore.kernel.org/kvm/20230809091717.1549-1-...@linux.ibm.com/ ). Sean, could you please rebase your series now? Thanks, Thomas
Re: KASAN debug kernel fails to boot at early stage when CONFIG_SMP=y is set (kernel 6.5-rc5, PowerMac G4 3,6)
Le 13/08/2023 à 21:38, Erhard Furtner a écrit :
> On Fri, 11 Aug 2023 06:45:14 +
> Christophe Leroy wrote:
>
>> Le 11/08/2023 à 01:48, Erhard Furtner a écrit :
>>> I wanted to fire up my PowerMac G4 MDD (Dual CPU) with a KASAN debug build
>>> of kernel 6.5-rc5 for testing purposes. But the kernel fails to boot at a
>>> very early stage. I only get a white screen reading
>>> "done
>>> found display: /pci@f000/ATY,AlteracParent@10/ATY,Alterac_B@1,
>>> opening..."
>>
>> Can you try with CONFIG_PPC_EARLY_DEBUG and see if you get more
>> information on the screen ?
>
> With CONFIG_PPC_EARLY_DEBUG set booting continues and I get two more lines on
> a white screen:
>
> [0.00] printk: bootconsole [udbg0] enabled
> [0.00] Total memory = 2048MB; using 4096kB for hash table
>
> Afterwards the G4 freezes.
Interesting. That means we get stuck somewhere around MMU_init()
We know that MMU_init_hw() is called and runs at least until:
pr_info("Total memory = %lldMB; using %ldkB for hash table\n",
(unsigned long long)(total_memory >> 20), Hash_size >> 10);
But we never reach the print in setup_kuap() which is itself called by
set_kup():
pr_info("Activating Kernel Userspace Access Protection\n");
Could you try to narrow more the issue by spreading pr_info() at places
in the code below and/or the called functions ? Either we never come
back from MMU_init_hw(), or one of mapin_ram() btext_unmap()
kasan_mmu_init() fails.
So the piece of code we are interested in is located in
arch/powerpc/mm/init_32.c and is:
/* Initialize the MMU hardware */
if (ppc_md.progress)
ppc_md.progress("MMU:hw init", 0x300);
==> MMU_init_hw();
/* Map in all of RAM starting at KERNELBASE */
if (ppc_md.progress)
ppc_md.progress("MMU:mapin", 0x301);
mapin_ram();
/* Initialize early top-down ioremap allocator */
ioremap_bot = IOREMAP_TOP;
if (ppc_md.progress)
ppc_md.progress("MMU:exit", 0x211);
/* From now on, btext is no longer BAT mapped if it was at all */
#ifdef CONFIG_BOOTX_TEXT
btext_unmap();
#endif
kasan_mmu_init();
==> setup_kup();
Christophe
>
> By chane I found out another interesting thing: This only happens on cold
> boots.
>
> If I boot the G4 up with another kernel first and boot the SMP KASAN kernel
> afterwards it just boots up fine too! On further reboots the SMP KASAN kernel
> keeps booting up normally. Until I turn the machine off - and next time I
> turn it on the same SMP KASAN kernel fails booting again as described.
> Strange...
>
> This behaviour however is reproducible. Tried that procedure 15-20 times just
> to be sure. Hope you can make something out of it.
>
> Attached is a dmesg of a successful SMP KASAM kernel boot attempt when a
> working kernel was booted first.
>
> Regards,
> Erhard F.
Re: [PATCH v6 00/25] iommu: Make default_domain's mandatory
On 2023/8/3 8:07, Jason Gunthorpe wrote:
[ It would be good to get this in linux-next, we have some good test
coverage on the ARM side already, thanks! ]
It has been a long time coming, this series completes the default_domain
transition and makes it so that the core IOMMU code will always have a
non-NULL default_domain for every driver on every
platform. set_platform_dma_ops() turned out to be a bad idea, and so
completely remove it.
This is achieved by changing each driver to either:
1 - Convert the existing (or deleted) ops->detach_dev() into an
op->attach_dev() of an IDENTITY domain.
This is based on the theory that the ARM32 HW is able to function when
the iommu is turned off and so the turned off state is an IDENTITY
translation.
2 - Use a new PLATFORM domain type. This is a hack to accommodate drivers
that we don't really know WTF they do. S390 is legitimately using this
to switch to it's platform dma_ops implementation, which is where the
name comes from.
3 - Do #1 and force the default domain to be IDENTITY, this corrects
the tegra-smmu case where even an ARM64 system would have a NULL
default_domain.
Using this we can apply the rules:
a) ARM_DMA_USE_IOMMU mode always uses either the driver's
ops->default_domain, ops->def_domain_type(), or an IDENTITY domain.
All ARM32 drivers provide one of these three options.
b) dma-iommu.c mode uses either the driver's ops->default_domain,
ops->def_domain_type or the usual DMA API policy logic based on the
command line/etc to pick IDENTITY/DMA domain types
c) All other arch's (PPC/S390) use ops->default_domain always.
See the patch "Require a default_domain for all iommu drivers" for a
per-driver breakdown.
The conversion broadly teaches a bunch of ARM32 drivers that they can do
IDENTITY domains. There is some educated guessing involved that these are
actual IDENTITY domains. If this turns out to be wrong the driver can be
trivially changed to use a BLOCKING domain type instead. Further, the
domain type only matters for drivers using ARM64's dma-iommu.c mode as it
will select IDENTITY based on the command line and expect IDENTITY to
work. For ARM32 and other arch cases it is purely documentation.
Finally, based on all the analysis in this series, we can purge
IOMMU_DOMAIN_UNMANAGED/DMA constants from most of the drivers. This
greatly simplifies understanding the driver contract to the core
code. IOMMU drivers should not be involved in policy for how the DMA API
works, that should be a core core decision.
The main gain from this work is to remove alot of ARM_DMA_USE_IOMMU
specific code and behaviors from drivers. All that remains in iommu
drivers after this series is the calls to arm_iommu_create_mapping().
This is a step toward removing ARM_DMA_USE_IOMMU.
The IDENTITY domains added to the ARM64 supporting drivers can be tested
by booting in ARM64 mode and enabling CONFIG_IOMMU_DEFAULT_PASSTHROUGH. If
the system still boots then most likely the implementation is an IDENTITY
domain. If not we can trivially change it to BLOCKING or at worst PLATFORM
if there is no detail what is going on in the HW.
I think this is pretty safe for the ARM32 drivers as they don't really
change, the code that was in detach_dev continues to be called in the same
places it was called before.
This is on github:https://github.com/jgunthorpe/linux/commits/iommu_all_defdom
It seems that after this series, all ARM iommu drivers are able to
support the IDENTITY default domain, hence perhaps we can remove below
code?
If I remember it correctly, the background of this part of code is
that some arm drivers didn't support IDENTITY domain, so fall back to
DMA domain if IDENTITY domain allocation fails.
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index ddbba3ffbfbd..ee1fa63f0612 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1798,7 +1798,6 @@ iommu_group_alloc_default_domain(struct
iommu_group *group, int req_type)
list_first_entry(>devices, struct group_device, list)
->dev;
const struct iommu_ops *ops = dev_iommu_ops(dev);
- struct iommu_domain *dom;
lockdep_assert_held(>mutex);
@@ -1817,20 +1816,7 @@ iommu_group_alloc_default_domain(struct
iommu_group *group, int req_type)
return __iommu_group_alloc_default_domain(group, req_type);
/* The driver gave no guidance on what type to use, try the default */
- dom = __iommu_group_alloc_default_domain(group, iommu_def_domain_type);
- if (dom)
- return dom;
-
- /* Otherwise IDENTITY and DMA_FQ defaults will try DMA */
- if (iommu_def_domain_type == IOMMU_DOMAIN_DMA)
- return NULL;
- dom = __iommu_group_alloc_default_domain(group, IOMMU_DOMAIN_DMA);
- if (!dom)
- return NULL;
-
- pr_warn("Failed to allocate default IOMMU domain of type %u for group
%s - Falling back
Re: [PATCH] powerpc/radix: Move some functions into #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
Le 14/08/2023 à 08:24, Nicholas Piggin a écrit : > On Wed Aug 9, 2023 at 6:01 PM AEST, Christophe Leroy wrote: >> With skiboot_defconfig, Clang reports: >> >>CC arch/powerpc/mm/book3s64/radix_tlb.o >> arch/powerpc/mm/book3s64/radix_tlb.c:419:20: error: unused function >> '_tlbie_pid_lpid' [-Werror,-Wunused-function] >> static inline void _tlbie_pid_lpid(unsigned long pid, unsigned long lpid, >> ^ >> arch/powerpc/mm/book3s64/radix_tlb.c:663:20: error: unused function >> '_tlbie_va_range_lpid' [-Werror,-Wunused-function] >> static inline void _tlbie_va_range_lpid(unsigned long start, unsigned long >> end, >> ^ >> >> This is because those functions are only called from functions >> enclosed in a #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE >> >> Move below functions inside that #ifdef >> * __tlbie_pid_lpid(unsigned long pid, >> * __tlbie_va_lpid(unsigned long va, unsigned long pid, >> * fixup_tlbie_pid_lpid(unsigned long pid, unsigned long lpid) >> * _tlbie_pid_lpid(unsigned long pid, unsigned long lpid, >> * fixup_tlbie_va_range_lpid(unsigned long va, >> * __tlbie_va_range_lpid(unsigned long start, unsigned long end, >> * _tlbie_va_range_lpid(unsigned long start, unsigned long end, > > Thanks for doing this. Functions vaguely belong where they are, which > makes it slightly annoying to move them. Is it also annoying to add > ifdefs for each one where they are? Looking at it once more, we can even move all those functions out in a separate file that would only be built when CONFIG_KVM_BOOK3S_HV_POSSIBLE is selected. The only dependency with other part of the file is the static tlb_single_page_flush_ceiling, which can easily be made global. In principle we try to avoid #ifdefs in C-files. Why would we want to keep those functions at the current place and spreat several more #ifdefs inside the file ? Christophe
Re: [PATCH v3 2/6] KVM: PPC: Rename accessor generator macros
On Mon Aug 7, 2023 at 11:45 AM AEST, Jordan Niethe wrote:
> More "wrapper" style accessor generating macros will be introduced for
> the nestedv2 guest support. Rename the existing macros with more
> descriptive names now so there is a consistent naming convention.
>
> Signed-off-by: Jordan Niethe
> ---
> v3:
> - New to series
> ---
> arch/powerpc/include/asm/kvm_ppc.h | 60 +++---
> 1 file changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> b/arch/powerpc/include/asm/kvm_ppc.h
> index d16d80ad2ae4..b66084a81dd0 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -927,19 +927,19 @@ static inline bool kvmppc_shared_big_endian(struct
> kvm_vcpu *vcpu)
> #endif
> }
>
> -#define SPRNG_WRAPPER_GET(reg, bookehv_spr) \
> +#define KVMPPC_BOOKE_HV_SPRNG_ACESSOR_GET(reg, bookehv_spr) \
> static inline ulong kvmppc_get_##reg(struct kvm_vcpu *vcpu) \
> {\
> return mfspr(bookehv_spr); \
> }\
>
> -#define SPRNG_WRAPPER_SET(reg, bookehv_spr) \
> +#define KVMPPC_BOOKE_HV_SPRNG_ACESSOR_SET(reg, bookehv_spr) \
> static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, ulong val)
> \
> {\
> mtspr(bookehv_spr, val);
> \
> }\
>
> -#define SHARED_WRAPPER_GET(reg, size)
> \
> +#define KVMPPC_VCPU_SHARED_REGS_ACESSOR_GET(reg, size)
> \
> static inline u##size kvmppc_get_##reg(struct kvm_vcpu *vcpu)
> \
> {\
> if (kvmppc_shared_big_endian(vcpu)) \
> @@ -948,7 +948,7 @@ static inline u##size kvmppc_get_##reg(struct kvm_vcpu
> *vcpu) \
> return le##size##_to_cpu(vcpu->arch.shared->reg);\
> }\
>
> -#define SHARED_WRAPPER_SET(reg, size)
> \
> +#define KVMPPC_VCPU_SHARED_REGS_ACESSOR_SET(reg, size)
> \
> static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, u##size val)
> \
> {\
> if (kvmppc_shared_big_endian(vcpu)) \
> @@ -957,36 +957,36 @@ static inline void kvmppc_set_##reg(struct kvm_vcpu
> *vcpu, u##size val) \
> vcpu->arch.shared->reg = cpu_to_le##size(val); \
> }\
>
> -#define SHARED_WRAPPER(reg, size)\
> - SHARED_WRAPPER_GET(reg, size) \
> - SHARED_WRAPPER_SET(reg, size) \
> +#define KVMPPC_VCPU_SHARED_REGS_ACESSOR(reg, size)
> \
> + KVMPPC_VCPU_SHARED_REGS_ACESSOR_GET(reg, size)
> \
> + KVMPPC_VCPU_SHARED_REGS_ACESSOR_SET(reg, size)
> \
>
> -#define SPRNG_WRAPPER(reg, bookehv_spr)
> \
> - SPRNG_WRAPPER_GET(reg, bookehv_spr) \
> - SPRNG_WRAPPER_SET(reg, bookehv_spr) \
> +#define KVMPPC_BOOKE_HV_SPRNG_ACESSOR(reg, bookehv_spr)
> \
> + KVMPPC_BOOKE_HV_SPRNG_ACESSOR_GET(reg, bookehv_spr)
> \
> + KVMPPC_BOOKE_HV_SPRNG_ACESSOR_SET(reg, bookehv_spr)
> \
>
> #ifdef CONFIG_KVM_BOOKE_HV
>
> -#define SHARED_SPRNG_WRAPPER(reg, size, bookehv_spr) \
> - SPRNG_WRAPPER(reg, bookehv_spr) \
> +#define KVMPPC_BOOKE_HV_SPRNG_OR_VCPU_SHARED_REGS_ACCESSOR(reg, size,
> bookehv_spr) \
> + KVMPPC_BOOKE_HV_SPRNG_ACESSOR(reg, bookehv_spr) \
>
> #else
>
> -#define SHARED_SPRNG_WRAPPER(reg, size, bookehv_spr) \
> - SHARED_WRAPPER(reg, size) \
> +#define KVMPPC_BOOKE_HV_SPRNG_OR_VCPU_SHARED_REGS_ACCESSOR(reg, size,
> bookehv_spr) \
> + KVMPPC_VCPU_SHARED_REGS_ACESSOR(reg, size) \
Not the greatest name I've ever seen :D Hard to be concice and
consistent though, this is an odd one.
Reviewed-by: Nicholas Piggin
>
> #endif
>
> -SHARED_WRAPPER(critical, 64)
> -SHARED_SPRNG_WRAPPER(sprg0, 64, SPRN_GSPRG0)
>
RE: [PATCH v3 4/6] KVM: PPC: Book3s HV: Hold LPIDs in an unsigned long
From: Jordan Niethe > Sent: 07 August 2023 02:46 > > The LPID register is 32 bits long. The host keeps the lpids for each > guest in an unsigned word struct kvm_arch. Currently, LPIDs are already > limited by mmu_lpid_bits and KVM_MAX_NESTED_GUESTS_SHIFT. > > The nestedv2 API returns a 64 bit "Guest ID" to be used be the L1 host > for each L2 guest. This value is used as an lpid, e.g. it is the > parameter used by H_RPT_INVALIDATE. To minimize needless special casing > it makes sense to keep this "Guest ID" in struct kvm_arch::lpid. > > This means that struct kvm_arch::lpid is too small so prepare for this > and make it an unsigned long. This is not a problem for the KVM-HV and > nestedv1 cases as their lpid values are already limited to valid ranges > so in those contexts the lpid can be used as an unsigned word safely as > needed. Shouldn't it be changed to u64? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH v3 4/6] KVM: PPC: Book3s HV: Hold LPIDs in an unsigned long
On Mon Aug 7, 2023 at 11:45 AM AEST, Jordan Niethe wrote:
> The LPID register is 32 bits long. The host keeps the lpids for each
> guest in an unsigned word struct kvm_arch. Currently, LPIDs are already
> limited by mmu_lpid_bits and KVM_MAX_NESTED_GUESTS_SHIFT.
>
> The nestedv2 API returns a 64 bit "Guest ID" to be used be the L1 host
> for each L2 guest. This value is used as an lpid, e.g. it is the
> parameter used by H_RPT_INVALIDATE. To minimize needless special casing
> it makes sense to keep this "Guest ID" in struct kvm_arch::lpid.
>
> This means that struct kvm_arch::lpid is too small so prepare for this
> and make it an unsigned long. This is not a problem for the KVM-HV and
> nestedv1 cases as their lpid values are already limited to valid ranges
> so in those contexts the lpid can be used as an unsigned word safely as
> needed.
>
> In the PAPR, the H_RPT_INVALIDATE pid/lpid parameter is already
> specified as an unsigned long so change pseries_rpt_invalidate() to
> match that. Update the callers of pseries_rpt_invalidate() to also take
> an unsigned long if they take an lpid value.
I don't suppose it would be worth having an lpid_t.
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index 4adff4f1896d..229f0a1ffdd4 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -886,10 +886,10 @@ int kvmppc_xive_attach_escalation(struct kvm_vcpu
> *vcpu, u8 prio,
>
> if (single_escalation)
> name = kasprintf(GFP_KERNEL, "kvm-%d-%d",
> - vcpu->kvm->arch.lpid, xc->server_num);
> + (unsigned int)vcpu->kvm->arch.lpid,
> xc->server_num);
> else
> name = kasprintf(GFP_KERNEL, "kvm-%d-%d-%d",
> - vcpu->kvm->arch.lpid, xc->server_num, prio);
> + (unsigned int)vcpu->kvm->arch.lpid,
> xc->server_num, prio);
> if (!name) {
> pr_err("Failed to allocate escalation irq name for queue %d of
> VCPU %d\n",
> prio, xc->server_num);
I would have thought you'd keep the type and change the format.
Otherwise seems okay too.
Thanks,
Nick
Re: [PATCH v3 1/6] KVM: PPC: Use getters and setters for vcpu register state
On Mon Aug 7, 2023 at 11:45 AM AEST, Jordan Niethe wrote:
> There are already some getter and setter functions used for accessing
> vcpu register state, e.g. kvmppc_get_pc(). There are also more
> complicated examples that are generated by macros like
> kvmppc_get_sprg0() which are generated by the SHARED_SPRNG_WRAPPER()
> macro.
>
> In the new PAPR "Nestedv2" API for nested guest partitions the L1 is
> required to communicate with the L0 to modify and read nested guest
> state.
>
> Prepare to support this by replacing direct accesses to vcpu register
> state with wrapper functions. Follow the existing pattern of using
> macros to generate individual wrappers. These wrappers will
> be augmented for supporting Nestedv2 guests later.
>
> Signed-off-by: Gautam Menghani
> Signed-off-by: Jordan Niethe
> ---
> v3:
> - Do not add a helper for pvr
> - Use an expression when declaring variable in case
> - Squash in all getters and setters
> - Guatam: Pass vector registers by reference
> ---
> arch/powerpc/include/asm/kvm_book3s.h | 123 +-
> arch/powerpc/include/asm/kvm_booke.h | 10 ++
> arch/powerpc/kvm/book3s.c | 38 ++---
> arch/powerpc/kvm/book3s_64_mmu_hv.c| 4 +-
> arch/powerpc/kvm/book3s_64_mmu_radix.c | 9 +-
> arch/powerpc/kvm/book3s_64_vio.c | 4 +-
> arch/powerpc/kvm/book3s_hv.c | 220 +
> arch/powerpc/kvm/book3s_hv.h | 58 +++
> arch/powerpc/kvm/book3s_hv_builtin.c | 10 +-
> arch/powerpc/kvm/book3s_hv_p9_entry.c | 4 +-
> arch/powerpc/kvm/book3s_hv_ras.c | 5 +-
> arch/powerpc/kvm/book3s_hv_rm_mmu.c| 8 +-
> arch/powerpc/kvm/book3s_hv_rm_xics.c | 4 +-
> arch/powerpc/kvm/book3s_xive.c | 9 +-
> arch/powerpc/kvm/emulate_loadstore.c | 2 +-
> arch/powerpc/kvm/powerpc.c | 76 -
> 16 files changed, 395 insertions(+), 189 deletions(-)
>
[snip]
> +
> /* Expiry time of vcpu DEC relative to host TB */
> static inline u64 kvmppc_dec_expires_host_tb(struct kvm_vcpu *vcpu)
> {
> - return vcpu->arch.dec_expires - vcpu->arch.vcore->tb_offset;
> + return kvmppc_get_dec_expires(vcpu) - kvmppc_get_tb_offset_hv(vcpu);
> }
I don't see kvmppc_get_tb_offset_hv in this patch.
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index 7f765d5ad436..738f2ecbe9b9 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -347,7 +347,7 @@ static int kvmppc_mmu_book3s_64_hv_xlate(struct kvm_vcpu
> *vcpu, gva_t eaddr,
> unsigned long v, orig_v, gr;
> __be64 *hptep;
> long int index;
> - int virtmode = vcpu->arch.shregs.msr & (data ? MSR_DR : MSR_IR);
> + int virtmode = kvmppc_get_msr(vcpu) & (data ? MSR_DR : MSR_IR);
>
> if (kvm_is_radix(vcpu->kvm))
> return kvmppc_mmu_radix_xlate(vcpu, eaddr, gpte, data, iswrite);
So this isn't _only_ adding new accessors. This should be functionally a
noop, but I think it introduces a branch if PR is defined.
Shared page is a slight annoyance for HV, I'd like to get rid of it...
but that's another story. I think the pattern here would be to add a
kvmppc_get_msr_hv() accessor.
And as a nitpick, for anywhere employing existing access functions, gprs
and such, could that be split into its own patch?
Looks pretty good aside from those little things.
Thanks,
Nick
Re: [PATCH] ocxl: Use pci_dev_id() to simplify the code
On Fri, 2023-08-11 at 18:20 +0800, Zheng Zengkai wrote: > PCI core API pci_dev_id() can be used to get the BDF number for a pci > device. We don't need to compose it mannually. Use pci_dev_id() to > simplify the code a little bit. > > Signed-off-by: Zheng Zengkai Acked-by: Andrew Donnellan -- Andrew DonnellanOzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v6 25/25] iommu: Convert remaining simple drivers to domain_alloc_paging()
On 2023/8/3 8:08, 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 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: Lu Baolu Best regards, baolu
Re: [PATCH v6 24/25] iommu: Convert simple drivers with DOMAIN_DMA to domain_alloc_paging()
On 2023/8/3 8:08, 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
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(-)
[...]
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index 0bf08b120cf105..056832a367c2af 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -667,14 +667,11 @@ static phys_addr_t sun50i_iommu_iova_to_phys(struct
iommu_domain *domain,
sun50i_iova_get_page_offset(iova);
}
-static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type)
+static struct iommu_domain *
+sun50i_iommu_domain_alloc_paging(struct device *paging)
Why not "struct device *dev"?
Typo? Or anything I missed?
{
struct sun50i_iommu_domain *sun50i_domain;
- if (type != IOMMU_DOMAIN_DMA &&
- type != IOMMU_DOMAIN_UNMANAGED)
- return NULL;
-
sun50i_domain = kzalloc(sizeof(*sun50i_domain), GFP_KERNEL);
if (!sun50i_domain)
return NULL;
@@ -840,7 +837,7 @@ static const struct iommu_ops sun50i_iommu_ops = {
.identity_domain = _iommu_identity_domain,
.pgsize_bitmap = SZ_4K,
.device_group = sun50i_iommu_device_group,
- .domain_alloc = sun50i_iommu_domain_alloc,
+ .domain_alloc_paging = sun50i_iommu_domain_alloc_paging,
.of_xlate = sun50i_iommu_of_xlate,
.probe_device = sun50i_iommu_probe_device,
.default_domain_ops = &(const struct iommu_domain_ops) {
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 6cba034905edbf..69c40c191ce4f0 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -272,13 +272,10 @@ static void tegra_smmu_free_asid(struct tegra_smmu *smmu,
unsigned int id)
clear_bit(id, smmu->asids);
}
-static struct iommu_domain *tegra_smmu_domain_alloc(unsigned type)
+static struct iommu_domain *tegra_smmu_domain_alloc_paging(struct device *dev)
{
struct tegra_smmu_as *as;
- if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
- return NULL;
-
as = kzalloc(sizeof(*as), GFP_KERNEL);
if (!as)
return NULL;
@@ -997,7 +994,7 @@ static const struct iommu_ops tegra_smmu_ops = {
.default_domain = _smmu_identity_domain,
.identity_domain = _smmu_identity_domain,
.def_domain_type = _smmu_def_domain_type,
- .domain_alloc = tegra_smmu_domain_alloc,
+ .domain_alloc_paging = tegra_smmu_domain_alloc_paging,
.probe_device = tegra_smmu_probe_device,
.device_group = tegra_smmu_device_group,
.of_xlate = tegra_smmu_of_xlate,
Anyway,
Reviewed-by: Lu Baolu
Best regards,
baolu
Re: [PATCH v6 20/25] iommu/sun50i: Add an IOMMU_IDENTITIY_DOMAIN
On 2023/8/3 8:08, 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(-)
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index 74c5cb93e90027..0bf08b120cf105 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -757,21 +757,32 @@ static void sun50i_iommu_detach_domain(struct
sun50i_iommu *iommu,
iommu->domain = NULL;
}
-static void sun50i_iommu_detach_device(struct iommu_domain *domain,
- struct device *dev)
+static int sun50i_iommu_identity_attach(struct iommu_domain *identity_domain,
+ struct device *dev)
{
- struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
struct sun50i_iommu *iommu = dev_iommu_priv_get(dev);
+ struct sun50i_iommu_domain *sun50i_domain;
dev_dbg(dev, "Detaching from IOMMU domain\n");
- if (iommu->domain != domain)
- return;
+ if (iommu->domain == identity_domain)
+ return 0;
+ sun50i_domain = to_sun50i_domain(iommu->domain);
if (refcount_dec_and_test(_domain->refcnt))
sun50i_iommu_detach_domain(iommu, sun50i_domain);
+ return 0;
}
Does iommu->domain need to be set to identity_domain before returning?
Best regards,
baolu
Re: [PATCH v6 19/25] iommu/mtk_iommu: Add an IOMMU_IDENTITIY_DOMAIN
On 2023/8/3 8:08, 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.
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/mtk_iommu.c | 23 +++
1 file changed, 23 insertions(+)
Reviewed-by: Lu Baolu
Best regards,
baolu
Re: [PATCH v6 18/25] iommu/ipmmu: Add an IOMMU_IDENTITIY_DOMAIN
On 2023/8/3 8:08, 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()")
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/ipmmu-vmsa.c | 43 ++
1 file changed, 43 insertions(+)
Reviewed-by: Lu Baolu
Best regards,
baolu
Re: [PATCH v6 17/25] iommu/qcom_iommu: Add an IOMMU_IDENTITIY_DOMAIN
On 2023/8/3 8:08, 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.
Signed-off-by: Jason Gunthorpe
---
drivers/iommu/arm/arm-smmu/qcom_iommu.c | 39 +
1 file changed, 39 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index a503ed758ec302..9d7b9d8b4386d4 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -387,6 +387,44 @@ static int qcom_iommu_attach_dev(struct iommu_domain
*domain, struct device *dev
return 0;
}
+static int qcom_iommu_identity_attach(struct iommu_domain *identity_domain,
+ struct device *dev)
+{
+ struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
+ struct qcom_iommu_domain *qcom_domain;
+ struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+ struct qcom_iommu_dev *qcom_iommu = to_iommu(dev);
+ unsigned int i;
+
+ if (domain == identity_domain || !domain)
+ return 0;
+
+ qcom_domain = to_qcom_iommu_domain(domain);
+ if (WARN_ON(!qcom_domain->iommu))
+ return -EINVAL;
+
+ pm_runtime_get_sync(qcom_iommu->dev);
+ for (i = 0; i < fwspec->num_ids; i++) {
+ struct qcom_iommu_ctx *ctx = to_ctx(qcom_domain,
fwspec->ids[i]);
+
+ /* Disable the context bank: */
+ iommu_writel(ctx, ARM_SMMU_CB_SCTLR, 0);
+
+ ctx->domain = NULL;
Does setting ctx->domain to NULL still match this semantics?
+ }
+ pm_runtime_put_sync(qcom_iommu->dev);
+ return 0;
+}
+
+static struct iommu_domain_ops qcom_iommu_identity_ops = {
+ .attach_dev = qcom_iommu_identity_attach,
+};
+
+static struct iommu_domain qcom_iommu_identity_domain = {
+ .type = IOMMU_DOMAIN_IDENTITY,
+ .ops = _iommu_identity_ops,
+};
+
static int qcom_iommu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t pgsize, size_t pgcount,
int prot, gfp_t gfp, size_t *mapped)
@@ -553,6 +591,7 @@ static int qcom_iommu_of_xlate(struct device *dev, struct
of_phandle_args *args)
}
static const struct iommu_ops qcom_iommu_ops = {
+ .identity_domain = _iommu_identity_domain,
.capable= qcom_iommu_capable,
.domain_alloc = qcom_iommu_domain_alloc,
.probe_device = qcom_iommu_probe_device,
Anyway,
Reviewed-by: Lu Baolu
Best regards,
baolu
Re: [PATCH] powerpc/radix: Move some functions into #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
On Wed Aug 9, 2023 at 6:01 PM AEST, Christophe Leroy wrote: > With skiboot_defconfig, Clang reports: > > CC arch/powerpc/mm/book3s64/radix_tlb.o > arch/powerpc/mm/book3s64/radix_tlb.c:419:20: error: unused function > '_tlbie_pid_lpid' [-Werror,-Wunused-function] > static inline void _tlbie_pid_lpid(unsigned long pid, unsigned long lpid, >^ > arch/powerpc/mm/book3s64/radix_tlb.c:663:20: error: unused function > '_tlbie_va_range_lpid' [-Werror,-Wunused-function] > static inline void _tlbie_va_range_lpid(unsigned long start, unsigned long > end, >^ > > This is because those functions are only called from functions > enclosed in a #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > > Move below functions inside that #ifdef > * __tlbie_pid_lpid(unsigned long pid, > * __tlbie_va_lpid(unsigned long va, unsigned long pid, > * fixup_tlbie_pid_lpid(unsigned long pid, unsigned long lpid) > * _tlbie_pid_lpid(unsigned long pid, unsigned long lpid, > * fixup_tlbie_va_range_lpid(unsigned long va, > * __tlbie_va_range_lpid(unsigned long start, unsigned long end, > * _tlbie_va_range_lpid(unsigned long start, unsigned long end, Thanks for doing this. Functions vaguely belong where they are, which makes it slightly annoying to move them. Is it also annoying to add ifdefs for each one where they are? Thanks, Nick
