diff mbox series

[v8,05/11] KVM: arm64: Enable writable for ID_AA64DFR0_EL1 and ID_DFR0_EL1

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

Commit Message

Jing Zhang Aug. 7, 2023, 4:22 p.m. UTC
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(-)

Comments

Marc Zyngier Aug. 17, 2023, 3:43 p.m. UTC | #1
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.
Jing Zhang Aug. 21, 2023, 5:37 p.m. UTC | #2
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 mbox series

Patch

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),