lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fc80ffde-2d19-4bd6-9590-a80049302ad7@arm.com>
Date: Tue, 13 Aug 2024 12:46:26 +0100
From: Robin Murphy <robin.murphy@....com>
To: Mostafa Saleh <smostafa@...gle.com>, linux-kernel@...r.kernel.org,
 iommu@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
 will@...nel.org, joro@...tes.org
Cc: jgg@...pe.ca, nicolinc@...dia.com, mshavit@...gle.com
Subject: Re: [PATCH 1/2] iommu/arm-smmu-v3: Match Stall behaviour for S2

On 12/08/2024 9:52 pm, Mostafa Saleh wrote:
> S2S must be set when stall model is forced "ARM_SMMU_FEAT_STALL_FORCE".
> But at the moment the driver ignores that, instead of doing the minimum
> and only set S2S for “ARM_SMMU_FEAT_STALL_FORCE” we can just match what

This was highly confusing, until the 3rd reading when I realised that 
maybe "instead of..." does not in fact belong to the description of the 
current behaviour, and it does start making sense if you swap the 
previous comma and full stop with each other.

> S1 does which also set it for “ARM_SMMU_FEAT_STALL” and the master
> has requested stalls.
> This makes the driver more consistent when running on different SMMU
> instances with different supported stages.
> 
> Signed-off-by: Mostafa Saleh <smostafa@...gle.com>
> ---
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 +++++
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 +
>   2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index a31460f9f3d4..8d573d9ca93c 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1562,6 +1562,11 @@ void arm_smmu_make_cdtable_ste(struct arm_smmu_ste *target,
>   		(cd_table->cdtab_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
>   		FIELD_PREP(STRTAB_STE_0_S1CDMAX, cd_table->s1cdmax));
>   
> +	/* S2S is ignored if stage-2 exists but not enabled. */
> +	if (master->stall_enabled &&
> +	    smmu->features & ARM_SMMU_FEAT_TRANS_S2)
> +		target->data[0] |= FIELD_PREP(STRTAB_STE_2_S2S, 1);

In the middle of the ASID?

Thanks,
Robin.

> +
>   	target->data[1] = cpu_to_le64(
>   		FIELD_PREP(STRTAB_STE_1_S1DSS, s1dss) |
>   		FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 14bca41a981b..0dc7ad43c64c 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -267,6 +267,7 @@ struct arm_smmu_ste {
>   #define STRTAB_STE_2_S2AA64		(1UL << 51)
>   #define STRTAB_STE_2_S2ENDI		(1UL << 52)
>   #define STRTAB_STE_2_S2PTW		(1UL << 54)
> +#define STRTAB_STE_2_S2S		(1UL << 57)
>   #define STRTAB_STE_2_S2R		(1UL << 58)
>   
>   #define STRTAB_STE_3_S2TTB_MASK		GENMASK_ULL(51, 4)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ