All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Upton <oliver.upton@linux.dev>
To: Marc Zyngier <maz@kernel.org>
Cc: kvmarm@lists.linux.dev, kvm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Volodymyr Babchuk <volodymyr_babchuk@epam.com>,
	Joey Gouly <joey.gouly@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>
Subject: Re: [PATCH 2/2] KVM: arm64: Fix vcpu_{read,write}_sys_reg() accessors
Date: Tue, 12 Aug 2025 13:23:33 -0700	[thread overview]
Message-ID: <aJuixWlc87f2UlK0@linux.dev> (raw)
In-Reply-To: <20250809144811.2314038-3-maz@kernel.org>

On Sat, Aug 09, 2025 at 03:48:11PM +0100, Marc Zyngier wrote:
> @@ -144,125 +156,120 @@ static bool get_el2_to_el1_mapping(unsigned int reg,
>  		MAPPED_EL2_SYSREG(ZCR_EL2,     ZCR_EL1,     NULL	     );
>  		MAPPED_EL2_SYSREG(CONTEXTIDR_EL2, CONTEXTIDR_EL1, NULL	     );
>  		MAPPED_EL2_SYSREG(SCTLR2_EL2,  SCTLR2_EL1,  NULL	     );
> +	case CNTHCTL_EL2:
> +		/* CNTHCTL_EL2 is super special, until we support NV2.1 */
> +		loc->loc = ((is_hyp_ctxt(vcpu) && vcpu_el2_e2h_is_set(vcpu)) ?
> +			    SR_LOC_SPECIAL : SR_LOC_MEMORY);
> +		break;
> +	case TPIDR_EL0:
> +	case TPIDRRO_EL0:
> +	case TPIDR_EL1:
> +	case PAR_EL1:
> +		/* These registers are always loaded, no matter what */
> +		loc->loc = SR_LOC_LOADED;
> +		break;
>  	default:
> -		return false;
> +		/*
> +		 * Non-mapped EL2 registers are by definition in memory, but
> +		 * we don't need to distinguish them here, as the CPU
> +		 * register accessors will bail out and we'll end-up using
> +		 * the backing store.
> +		 *
> +		 * EL1 registers are, however, only loaded if we're
> +		 * not in hypervisor context.
> +		 */
> +		loc->loc = is_hyp_ctxt(vcpu) ? SR_LOC_MEMORY : SR_LOC_LOADED;

Hmm... I get the feeling that this flow is becoming even more subtle.
There's some implicit coupling between this switch statement and the
__vcpu_{read,write}_sys_reg_from_cpu() which feels like it could be
error prone. Especially since we're gonna lose the WARN() that would
inform us if an on-CPU register was actually redirected to memory.

I'm wondering if we need some macro hell containing the block of
registers we handle on-CPU and expand that can be expanded into this
triage switch case as well as the sysreg accessor.

What you have definitely seems correct, though. I'll twiddle a bit and
see if I come up with something, although I imagine what you have is
what we'll use in the end anyway.

Thanks,
Oliver

  reply	other threads:[~2025-08-12 20:23 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-09 14:48 [PATCH 0/2] KVM: arm64: AT + SR accessor fixes Marc Zyngier
2025-08-09 14:48 ` [PATCH 1/2] KVM: arm64: nv: Fix ATS12 handling of single-stage translation Marc Zyngier
2025-08-09 14:48 ` [PATCH 2/2] KVM: arm64: Fix vcpu_{read,write}_sys_reg() accessors Marc Zyngier
2025-08-12 20:23   ` Oliver Upton [this message]
2025-08-13  6:54     ` Marc Zyngier
2025-08-14 16:16     ` Marc Zyngier
2025-08-15 18:56       ` Oliver Upton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aJuixWlc87f2UlK0@linux.dev \
    --to=oliver.upton@linux.dev \
    --cc=joey.gouly@arm.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=maz@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --cc=volodymyr_babchuk@epam.com \
    --cc=yuzenghui@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.