Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 25 May 2023 19:50:49 +0000
In-Reply-To: <20230509142235.3284028-8-kristina.martsenko@arm.com>
Mime-Version: 1.0
Message-ID: <gsnta5xsb36u.fsf@coltonlewis-kvm.c.googlers.com>
Subject: RE: [PATCH v2 07/11] arm64: mops: handle MOPS exceptions
From:   Colton Lewis <coltonlewis@google.com>
To:     Kristina Martsenko <kristina.martsenko@arm.com>
Cc:     linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
        Oliver Upton <oliver.upton@linux.dev>,
        James Morse <james.morse@arm.com>,
        Suzuki K Poulose <suzuki.poulose@arm.com>,
        Zenghui Yu <yuzenghui@huawei.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Mark Brown <broonie@kernel.org>,
        Luis Machado <luis.machado@arm.com>,
        Vladimir Murzin <vladimir.murzin@arm.com>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"; format=flowed; delsp=yes
Precedence: bulk

> +	if (esr & ESR_ELx_MOPS_ISS_MEM_INST) {
> +		/* SET* instruction */
> +		if (option_a ^ wrong_option) {
> +			/* Format is from Option A; forward set */
> +			pt_regs_write_reg(regs, dstreg, dst + size);
> +			pt_regs_write_reg(regs, sizereg, -size);
> +		}
> +	} else {
> +		/* CPY* instruction */
> +		if (!(option_a ^ wrong_option)) {
> +			/* Format is from Option B */
> +			if (regs->pstate & PSR_N_BIT) {
> +				/* Backward copy */
> +				pt_regs_write_reg(regs, dstreg, dst - size);
> +				pt_regs_write_reg(regs, srcreg, src - size);
> +			}
> +		} else {
> +			/* Format is from Option A */
> +			if (size & BIT(63)) {
> +				/* Forward copy */
> +				pt_regs_write_reg(regs, dstreg, dst + size);
> +				pt_regs_write_reg(regs, srcreg, src + size);
> +				pt_regs_write_reg(regs, sizereg, -size);
> +			}
> +		}
> +	}

I can see an argument for styling things closely to the ARM manual as
you have done here, but Linux style recommends against deep nesting. In
this case it is unneeded. I believe this can be written as a single
if-else chain and that makes it easier to distinguish the three options.

if ((esr & ESR_ELx_MOPS_ISS_MEM_INST) && (option_a ^ wrong_option)) {
	/* Format is from Option A; forward set */
	pt_regs_write_reg(regs, dstreg, dst + size);
	pt_regs_write_reg(regs, sizereg, -size);
} else if ((option_a ^ wrong_option) && (size & BIT(63)) {
	/* Forward copy */
	pt_regs_write_reg(regs, dstreg, dst + size);
	pt_regs_write_reg(regs, srcreg, src + size);
	pt_regs_write_reg(regs, sizereg, -size);
} else if (regs-pstate & PSR_N_BIT) {
	/* Backward copy */
	pt_regs_write_reg(regs, dstreg, dst - size);
	pt_regs_write_reg(regs, srcreg, src - size);
}