| Message ID | 20230807162210.2528230-8-jingzhangos@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable writable for idregs DFR0,PFR0, MMFR{0,1,2,3} | expand |
On Mon, 07 Aug 2023 17:22:05 +0100, Jing Zhang <jingzhangos@google.com> wrote: > > All valid fields in ID_AA64PFR0_EL1 are writable from usrespace > with this change. userspace > > Signed-off-by: Jing Zhang <jingzhangos@google.com> > --- > 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 879004fd37e5..392613bec560 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -2041,7 +2041,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > .get_user = get_id_reg, > .set_user = set_id_reg, > .reset = read_sanitised_id_aa64pfr0_el1, > - .val = ID_AA64PFR0_EL1_CSV2_MASK | ID_AA64PFR0_EL1_CSV3_MASK, }, > + .val = GENMASK(63, 0), }, > ID_SANITISED(ID_AA64PFR1_EL1), > ID_UNALLOCATED(4,2), > ID_UNALLOCATED(4,3), Same remark as the previous patch. What makes it legal to make *everything* writable? For example, we don't expose the AMU. And yet you are telling userspace "sure, go ahead". Userspace will then try and restore *something*, and will eventually crap itself because the kernel won't allow it. Why do we bother describing the writable fields if userspace can't write to them? M.
Hi Marc, On Thu, Aug 17, 2023 at 8:53 AM Marc Zyngier <maz@kernel.org> wrote: > > On Mon, 07 Aug 2023 17:22:05 +0100, > Jing Zhang <jingzhangos@google.com> wrote: > > > > All valid fields in ID_AA64PFR0_EL1 are writable from usrespace > > with this change. > > userspace > > > > > Signed-off-by: Jing Zhang <jingzhangos@google.com> > > --- > > 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 879004fd37e5..392613bec560 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -2041,7 +2041,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > > .get_user = get_id_reg, > > .set_user = set_id_reg, > > .reset = read_sanitised_id_aa64pfr0_el1, > > - .val = ID_AA64PFR0_EL1_CSV2_MASK | ID_AA64PFR0_EL1_CSV3_MASK, }, > > + .val = GENMASK(63, 0), }, > > ID_SANITISED(ID_AA64PFR1_EL1), > > ID_UNALLOCATED(4,2), > > ID_UNALLOCATED(4,3), > > Same remark as the previous patch. What makes it legal to make > *everything* writable? For example, we don't expose the AMU. And yet > you are telling userspace "sure, go ahead". > > Userspace will then try and restore *something*, and will eventually > crap itself because the kernel won't allow it. > > Why do we bother describing the writable fields if userspace can't > write to them? I'll send out another version which wouldn't enable writable masks for RES0 fields and KVM hidden fields. Does it sound good to you? > > M. > > -- > Without deviation from the norm, progress is not possible. Thanks, Jing
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 879004fd37e5..392613bec560 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2041,7 +2041,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { .get_user = get_id_reg, .set_user = set_id_reg, .reset = read_sanitised_id_aa64pfr0_el1, - .val = ID_AA64PFR0_EL1_CSV2_MASK | ID_AA64PFR0_EL1_CSV3_MASK, }, + .val = GENMASK(63, 0), }, ID_SANITISED(ID_AA64PFR1_EL1), ID_UNALLOCATED(4,2), ID_UNALLOCATED(4,3),
All valid fields in ID_AA64PFR0_EL1 are writable from usrespace with this change. Signed-off-by: Jing Zhang <jingzhangos@google.com> --- arch/arm64/kvm/sys_regs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)