Hi Oliver,
On Tue, Dec 14, 2021 at 05:28:07PM +0000, Oliver Upton wrote:
> Any valid implementation of the architecture should generate an
> undefined exception for writes to a read-only register, such as
> OSLSR_EL1. Nonetheless, the KVM handler actually implements write-ignore
> behavior.
>
> Align the trap handler for OSLSR_EL1 with hardware behavior. If such a
> write ever traps to EL2, inject an undef into the guest and print a
> warning.
I think this can still be read amibguously, since we don't explicitly state
that writes to OSLSR_EL1 should never trap (and the implications of being
UNDEFINED are subtle). How about:
| Writes to OSLSR_EL1 are UNDEFINED and should never trap from EL1 to EL2, but
| the KVM trap handler for OSLSR_EL1 handlees writes via ignore_write(). This
| is confusing to readers of the code, but shouldn't have any functional impact.
|
| For clarity, use write_to_read_only() rather than ignore_write(). If a trap
| is unexpectedly taken to EL2 in violation of the architecture, this will
| WARN_ONCE() and inject an undef into the guest.
With that:
Reviewed-by: Mark Rutland <mark.rutland@xxxxxxx>
Mark.
> Reviewed-by: Reiji Watanabe <reijiw@xxxxxxxxxx>
> Signed-off-by: Oliver Upton <oupton@xxxxxxxxxx>
> ---
> arch/arm64/kvm/sys_regs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index e3ec1a44f94d..11b4212c2036 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -292,7 +292,7 @@ static bool trap_oslsr_el1(struct kvm_vcpu *vcpu,
> const struct sys_reg_desc *r)
> {
> if (p->is_write) {
> - return ignore_write(vcpu, p);
> + return write_to_read_only(vcpu, p, r);
> } else {
> p->regval = (1 << 3);
> return true;
> --
> 2.34.1.173.g76aa8bc2d0-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@xxxxxxxxxxxxxxxxxxxxx
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
