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] [day] [month] [year] [list]
Message-ID: <Zr9la0hy34IDgbGd@google.com>
Date: Fri, 16 Aug 2024 14:42:51 +0000
From: Mostafa Saleh <smostafa@...gle.com>
To: Jason Gunthorpe <jgg@...pe.ca>
Cc: linux-kernel@...r.kernel.org, iommu@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org, will@...nel.org,
	robin.murphy@....com, joro@...tes.org, jean-philippe@...aro.org,
	nicolinc@...dia.com, mshavit@...gle.com
Subject: Re: [PATCH v3] iommu/arm-smmu-v3: Match Stall behaviour for S2

On Fri, Aug 16, 2024 at 10:31:35AM -0300, Jason Gunthorpe wrote:
> On Fri, Aug 16, 2024 at 12:59:01PM +0000, Mostafa Saleh wrote:
> > According to the spec (ARM IHI 0070 F.b), in
> > "5.5 Fault configuration (A, R, S bits)":
> >     A STE with stage 2 translation enabled and STE.S2S == 0 is
> >     considered ILLEGAL if SMMU_IDR0.STALL_MODEL == 0b10.
> > 
> > Also described in the pseudocode “SteIllegal()”
> >     if STE.Config == '11x' then
> >         [..]
> >         if eff_idr0_stall_model == '10' && STE.S2S == '0' then
> >             // stall_model forcing stall, but S2S == 0
> >             return TRUE;
> > 
> > Which means, S2S must be set when stall model is
> > "ARM_SMMU_FEAT_STALL_FORCE", but at the moment the driver ignores that.
> > 
> > Although, the driver can do the minimum and only set S2S for
> > “ARM_SMMU_FEAT_STALL_FORCE”, it is more consistent to match S1
> > behaviour, which also sets it for “ARM_SMMU_FEAT_STALL” if the
> > master has requested stalls.
> > 
> > Also, since S2 stalls are enabled now, report them to the IOMMU layer
> > and for VFIO devices it will fail anyway as VFIO doesn’t register an
> > iopf handler.
> > 
> > Signed-off-by: Mostafa Saleh <smostafa@...gle.com>
> > 
> > ---
> > v3:
> > - Set S2S for s2 and not s1 domain
> > - Ignore ats check
> > 
> > v2:
> > - Fix index of the STE
> > - Fix conflict with ATS
> > - Squash the 2 patches and drop enable_nesting
> > ---
> >  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, 2 insertions(+), 4 deletions(-)
> 
> Reviewed-by: Jason Gunthorpe <jgg@...dia.com>

Actually this misses adding S2S to arm_smmu_get_ste_used() as it is
a new bit :/

And that is not detected by “CONFIG_ARM_SMMU_V3_KUNIT_TEST” as it
doesn’t test stall in masters, I will fix that and add coverage in
tests for stall for both stages and respin.

Thanks,
Mostafa

> 
> Jason

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ