Re: [PATCH] KVM: arm64: Fix S1/S2 combination when FWB==1 and S2 has Device memory type

[Oliver Upton <oliver.upton@linux.dev>]

On Mon, Nov 25, 2024 at 09:47:56AM +0000, Marc Zyngier wrote:
> The G.a revision of the ARM ARM had it pretty clear that HCR_EL2.FWB
> had no influence on "The way that stage 1 memory types and attributes
> are combined with stage 2 Device type and attributes." (D5.5.5).
> 
> However, this wording was lost in further revisions of the architecture.
> 
> Restore the intended behaviour, which is to take the strongest memory
> type of S1 and S2 in this case, as if FWB was 0. The specification is
> being fixed accordingly.

Since you're already asking for a spec fix, could you mention that the
column headers in DDI0487K.a Table D8-95 are incorrect? MemAttr[1:0] is
used twice, although I believe the first column is actually MemAttr[3:2].

> Fixes: be04cebf3e788 ("KVM: arm64: nv: Add emulation of AT S12E{0,1}{R,W}")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/at.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/at.c b/arch/arm64/kvm/at.c
> index 8c5d7990e5b31..98cb499fa4b11 100644
> --- a/arch/arm64/kvm/at.c
> +++ b/arch/arm64/kvm/at.c
> @@ -739,8 +739,15 @@ static u64 compute_par_s12(struct kvm_vcpu *vcpu, u64 s1_par,
>  			final_attr = s1_parattr;
>  			break;
>  		default:
> -			/* MemAttr[2]=0, Device from S2 */
> -			final_attr = s2_memattr & GENMASK(1,0) << 2;
> +			/*
> +			 * MemAttr[2]=0, Device from S2.
> +			 *
> +			 * FWB does not influence the way that stage 1
> + 			 * memory types and attributes are combined
> + 			 * with stage 2 Device type and attributes.
> +			 */
> +			final_attr = min(s2_memattr_to_attr(s2_memattr),
> +					 s1_parattr);

Otherwise, LGTM.

-- 
Thanks,
Oliver