| Message ID | 20230807162210.2528230-6-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:03 +0100, Jing Zhang <jingzhangos@google.com> wrote: > > All valid fields in ID_AA64DFR0_EL1 and ID_DFR0_EL1 are writable > from usrespace with this change. nit: userspace > > Signed-off-by: Jing Zhang <jingzhangos@google.com> > --- > arch/arm64/kvm/sys_regs.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index afade7186675..5f6c2be12e44 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -2006,7 +2006,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > .set_user = set_id_dfr0_el1, > .visibility = aa32_id_visibility, > .reset = read_sanitised_id_dfr0_el1, > - .val = ID_DFR0_EL1_PerfMon_MASK, }, > + .val = GENMASK(63, 0), }, For obvious reasons, this cannot be a 64 bit mask... > ID_HIDDEN(ID_AFR0_EL1), > AA32_ID_SANITISED(ID_MMFR0_EL1), > AA32_ID_SANITISED(ID_MMFR1_EL1), > @@ -2055,7 +2055,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > .get_user = get_id_reg, > .set_user = set_id_aa64dfr0_el1, > .reset = read_sanitised_id_aa64dfr0_el1, > - .val = ID_AA64DFR0_EL1_PMUVer_MASK, }, > + .val = GENMASK(63, 0), }, What is the actual justification to go from "only the PMU version is writable" to "everything is writable"? Also, what about the RES0 fields? Thanks, M.
Hi Marc, On Thu, Aug 17, 2023 at 8:43 AM Marc Zyngier <maz@kernel.org> wrote: > > On Mon, 07 Aug 2023 17:22:03 +0100, > Jing Zhang <jingzhangos@google.com> wrote: > > > > All valid fields in ID_AA64DFR0_EL1 and ID_DFR0_EL1 are writable > > from usrespace with this change. > > nit: userspace > Fixed. > > > > Signed-off-by: Jing Zhang <jingzhangos@google.com> > > --- > > arch/arm64/kvm/sys_regs.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index afade7186675..5f6c2be12e44 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -2006,7 +2006,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > > .set_user = set_id_dfr0_el1, > > .visibility = aa32_id_visibility, > > .reset = read_sanitised_id_dfr0_el1, > > - .val = ID_DFR0_EL1_PerfMon_MASK, }, > > + .val = GENMASK(63, 0), }, > > For obvious reasons, this cannot be a 64 bit mask... > > > ID_HIDDEN(ID_AFR0_EL1), > > AA32_ID_SANITISED(ID_MMFR0_EL1), > > AA32_ID_SANITISED(ID_MMFR1_EL1), > > @@ -2055,7 +2055,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > > .get_user = get_id_reg, > > .set_user = set_id_aa64dfr0_el1, > > .reset = read_sanitised_id_aa64dfr0_el1, > > - .val = ID_AA64DFR0_EL1_PMUVer_MASK, }, > > + .val = GENMASK(63, 0), }, > > What is the actual justification to go from "only the PMU version is > writable" to "everything is writable"? > > Also, what about the RES0 fields? You're right. We should not mark RES0 fields and those fields KVM hide from userspace as writable. So, I think all the other fields can be writable? > > Thanks, > > 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 afade7186675..5f6c2be12e44 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2006,7 +2006,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { .set_user = set_id_dfr0_el1, .visibility = aa32_id_visibility, .reset = read_sanitised_id_dfr0_el1, - .val = ID_DFR0_EL1_PerfMon_MASK, }, + .val = GENMASK(63, 0), }, ID_HIDDEN(ID_AFR0_EL1), AA32_ID_SANITISED(ID_MMFR0_EL1), AA32_ID_SANITISED(ID_MMFR1_EL1), @@ -2055,7 +2055,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { .get_user = get_id_reg, .set_user = set_id_aa64dfr0_el1, .reset = read_sanitised_id_aa64dfr0_el1, - .val = ID_AA64DFR0_EL1_PMUVer_MASK, }, + .val = GENMASK(63, 0), }, ID_SANITISED(ID_AA64DFR1_EL1), ID_UNALLOCATED(5,2), ID_UNALLOCATED(5,3),
All valid fields in ID_AA64DFR0_EL1 and ID_DFR0_EL1 are writable from usrespace with this change. Signed-off-by: Jing Zhang <jingzhangos@google.com> --- arch/arm64/kvm/sys_regs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)