| 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
| ||
|
Message-ID: <86cyffrlof.wl-maz@kernel.org>
Date: Tue, 18 Feb 2025 21:24:16 +0000
From: Marc Zyngier <maz@...nel.org>
To: Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>
Cc: Eric Auger <eauger@...hat.com>,
Oliver Upton <oliver.upton@...ux.dev>,
kvmarm <kvmarm@...ts.linux.dev>,
linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org,
christoffer.dall@....com,
suzuki.poulose@....com,
will@...nel.org,
catalin.marinas@....com,
coltonlewis@...gle.com,
joey.gouly@....com,
yuzenghui@...wei.com,
darren@...amperecomputing.com,
vishnu@...amperecomputing.com
Subject: Re: [PATCH] KVM: arm64: nv: Set ISTATUS for emulated timers, If timer expired
On Tue, 18 Feb 2025 07:33:11 +0000,
Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com> wrote:
>
>
> Hi Marc,
>
> On 15-02-2025 11:20 pm, Marc Zyngier wrote:
> > On Mon, 10 Feb 2025 18:26:48 +0000,
> > Eric Auger <eauger@...hat.com> wrote:
> >>
> >> Hi Marc,
> >>
> >> On 2/7/25 7:38 PM, Marc Zyngier wrote:
> >>> On Fri, 07 Feb 2025 18:09:58 +0000,
> >>> Oliver Upton <oliver.upton@...ux.dev> wrote:
> >>>>
> >>>> Hey,
> >>>>
> >>>> On Fri, Feb 07, 2025 at 05:45:33PM +0000, Marc Zyngier wrote:
> >>>>> I found at least one issue that could fail the migration. Before the
> >>>>> VM starts running, we limit the feature set to the subset we actually
> >>>>> support with NV.
> >>>>>
> >>>>> By doing this, we also change the value of IDreg fields that are not
> >>>>> writable, because they describe features that we don't support.
> >>>>> Obviously, that fails on restore.
> >>>>>
> >>>>> I need to have a think...
> >>>>
> >>>> We spoke about this a while ago (and I forgot til now), but I was
> >>>> wondering if we could use vCPU feature flags to describe NV, including
> >>>> the selection between FEAT_E2H0 and FEAT_VHE.
> >>>>
> >>>> I think this might match userspace expectations a bit more closely where
> >>>> the state of the ID registers after init gives the actual feature set
> >>>> supported by the VM.
> >>>
> >>> I'm not sure that's enough. Let me give you an example:
> >>>
> >>> My host has FEAT_XNX, described in ID_AA64MMFR1_EL1.XNX. For whatever
> >>> reason, we don't allow this field to be written to, even out of NV
> >>> context. This is odd, because for an EL1 VM, this field means nothing
> >>> at all.
> >> So the curprit fields for me look like
> >>
> >> - ID_AA64MMFR1_EL1.XNX
> >> - ID_AA64DFR0_EL1.DoubleLock
> >> - ID_AA64PFR0_EL1.RAS
> >>
> >> This is still based on your nv-next branch from Jan 9
> >> https://github.com/eauger/linux/tree/nv_next_jan9_2025
> >
> > I have now pushed out a new nv-next branch with the new and improved
> > UAPI. I expect migration to work a bit better, or at least not to
> > explode on ID register restore. You will notice that things have
> > changed a bit (extra flag and cap for FEAT_E2H0), but nothing really
> > major.
> >
>
> Tried nv-next branch and it is breaking(kernel Oops) for normal VM
> boot itself with qemu. Looks like this is happening since qemu is
> trying to write to ID_UNALLOCATED mapped registers as part of
> save-restore of registers.
My take on this problem ends up being more consolidation, and make
sure that the individual macros only override the default callbacks
for idregs.
Additionally, ID_UNALLOCATED gets a name matching the architectural
encoding.
M.
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index e6f4599dca48e..2e14562b5841f 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2261,24 +2261,26 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
* from userspace.
*/
-#define ID_DESC(name) \
- SYS_DESC(SYS_##name), \
+#define ID_DESC_DEFAULT_CALLBACKS \
.access = access_id_reg, \
.get_user = get_id_reg, \
+ .set_user = set_id_reg, \
+ .visibility = id_visibility, \
.reset = kvm_read_sanitised_id_reg
+#define ID_DESC(name) \
+ SYS_DESC(SYS_##name), \
+ ID_DESC_DEFAULT_CALLBACKS
+
/* sys_reg_desc initialiser for known cpufeature ID registers */
#define ID_SANITISED(name) { \
ID_DESC(name), \
- .set_user = set_id_reg, \
- .visibility = id_visibility, \
.val = 0, \
}
/* sys_reg_desc initialiser for known cpufeature ID registers */
#define AA32_ID_SANITISED(name) { \
ID_DESC(name), \
- .set_user = set_id_reg, \
.visibility = aa32_id_visibility, \
.val = 0, \
}
@@ -2286,8 +2288,6 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
/* sys_reg_desc initialiser for writable ID registers */
#define ID_WRITABLE(name, mask) { \
ID_DESC(name), \
- .set_user = set_id_reg, \
- .visibility = id_visibility, \
.val = mask, \
}
@@ -2295,7 +2295,6 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
#define ID_FILTERED(sysreg, name, mask) { \
ID_DESC(sysreg), \
.set_user = set_##name, \
- .visibility = id_visibility, \
.val = (mask), \
}
@@ -2305,10 +2304,9 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
* (1 <= crm < 8, 0 <= Op2 < 8).
*/
#define ID_UNALLOCATED(crm, op2) { \
+ .name = "S3_0_0_" #crm "_" #op2, \
Op0(3), Op1(0), CRn(0), CRm(crm), Op2(op2), \
- .access = access_id_reg, \
- .get_user = get_id_reg, \
- .set_user = set_id_reg, \
+ ID_DESC_DEFAULT_CALLBACKS, \
.visibility = raz_visibility, \
.val = 0, \
}
@@ -2320,7 +2318,6 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
*/
#define ID_HIDDEN(name) { \
ID_DESC(name), \
- .set_user = set_id_reg, \
.visibility = raz_visibility, \
.val = 0, \
}
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists